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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

In the call back, the first thing the client is likely to do is:
struct my_struct *s = buf_addr;

This works great when its a void *. If buf_addr is a u8 *, that's not valid, and will result in a compiler error (at-least per gcc 5.4.0).
With u8 *, the client has to do:
struct my_struct *s = (struct my_struct *)buf_addr;

I don't see a benefit to u8 * over void * in this case.

The only possibly benefit I might see is if the client wants to use buf_addr as an array to poke into it and maybe check a magic number, but that assumes said magic number is a u8. Otherwise the client has to do an explicit cast. It seems like such a small amount of the time that usecase would be valid, that its not worth it to cater to it.

rpmsg, as one example, does the exact same thing where the received buffer is a void *, and there is a size parameter.

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



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux