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

From: Jeffrey Hugo
Date: Sun Jan 26 2020 - 16:00:29 EST


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.