Re: [PATCH 02/16] bus: mhi: core: Add support for registering MHI controllers

Jeffrey Hugo
2020-01-26T21:00:21Z

On 1/25/2020 6:26 AM, Greg KH wrote:
> On Fri, Jan 24, 2020 at 11:12:57AM -0700, Jeffrey Hugo wrote:
>> On 1/24/2020 10:47 AM, Greg KH wrote:
>>> On Fri, Jan 24, 2020 at 07:24:43AM -0700, Jeffrey Hugo wrote:
>>>>>> +/**
>>>>>> + * struct mhi_result - Completed buffer information
>>>>>> + * @buf_addr: Address of data buffer
>>>>>> + * @dir: Channel direction
>>>>>> + * @bytes_xfer: # of bytes transferred
>>>>>> + * @transaction_status: Status of last transaction
>>>>>> + */
>>>>>> +struct mhi_result {
>>>>>> + void *buf_addr;
>>>>>
>>>>> Why void *?
>>>>
>>>> Because its not possible to resolve this more clearly. The client provides
>>>> the buffer and knows what the structure is. The bus does not. Its just an
>>>> opaque pointer (hence void *) to the bus, and the client needs to decode it.
>>>> This is the struct that is handed to the client to allow them to decode the
>>>> activity (either a received buf, or a confirmation that a transmitted buf
>>>> has been consumed).
>>>
>>> Then shouldn't this be a "u8 *" instead as you are saying how many bytes
>>> are here?
>>
>> I'm sorry, I don't see the benefit of that. Can you elaborate on why you
>> think that u8 * is a better type?
>>
>> Sure, its an arbitrary byte stream from the perspective of the bus, but to
>> the client, 99% of the time its going to have some structure.
>
> So which side is in control here, the "bus" or the "client"? For the
> bus to care, it's a bytestream and should be represented as such (like
> you have) with a number of bytes in the "packet". >
> If you already know the structure types, just make a union of all of the
> valid ones and be done with it. In other words, try to avoid using void
> * as much as is ever possible please.

The client is in control. Perhaps if you think of this like a NIC - the
NIC is a dumb pipe that you shove bytes into and get bytes out of. The
NIC doesn't know or care what the bytes are, only that it performs its
responsibilities of successfully moving those bytes through the pipe.
The bytes could be a TCP packet, UDP packet, raw IP packet, or something
entirely different. The NIC doesn't need to know, nor care.

MHI is a little one sided because its designed so that the Host is in
control for the most part.

In the transmit path, the client on the Host gives the bus a stream of
bytes. The DMA-able address of that stream of bytes is put into the bus
structures, and the doorbell is rung. Then the device pulls in the data.

In the receive path, the client on the host gives the bus a receive
buffer. The DMA-able address of that buffer is put into the bus
structures. When the device wants to send data to the host, it picks up
the buffer address, copies the data into it, and then flags an event to
the Host.

This structure we are discussing is used in the callback from the bus to
the client to either signal that the TX buffer has been consumed by the
device and is now back under the control of the client, or that the
device has consumed a queued RX buffer, and now the buffer is back under
the control of the client and can be read to determine what data the
device sent.

In both cases, its impossible for the bus to know the structure or
content of the data. All the bus knows or cares about is the location
and size of the buffer. Its entirely up to the control of the client.
The client could be the network stack, in which case the data is
probably an IP packet. The client could be entirely something else
where the client protocol running over MHI is entirely unique to that
client.

Since MHI supports arbitrary clients, its impossible to come up with
some kind of union that describes every possible client's structure
definitions from now until the end of time.

void * is the only type that makes realistic sense.

--
Jeffrey Hugo
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.