Re: [PATCH v3 12/25] bus: mhi: ep: Add support for ring management

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

 



On 2/18/22 9:23 AM, Manivannan Sadhasivam wrote:

I'm pretty sure I mentioned this before...  I don't really like these
"DWORD" macros that simply write compute register values to write
out to the TREs.  A TRE is a structure, not a set of registers.  And
a whole TRE can be written or read in a single ARM instruction in
some cases--but most likely you need to define it as a structure
for that to happen.

struct mhi_tre {
	__le64 addr;
	__le16 len_opcode
	__le16 reserved;
	__le32 flags;
};
Changing the TRE structure requires changes to both host and endpoint
stack. So I'll tackle this as an improvement later.

Added to TODO list.
Just did a comparision w/ IPA code and I convinced myself that this conversion
should happen now itself. So please ignore my above comment.

This might not be that much work, but if it is, I somewhat
apologize for that.  Still, I believe the code will be better
as a result, so I'm not *that* sorry.

If you do this though, I would recommend you do it as a
separate, prerequisite bit of work.  Your series is too
long, and making it longer by adding this will just delay
*everything* a bit more.  So, I'd advise updating the
existing host code this way first, then adapt your patch
series to do things the new way.

Alternatively, do this later (as you earlier said you would),
and don't delay this series any more.  If it works, it works,
and you can always improve it in the future.

And now that your series is getting closer to golden, maybe
you can break it into a few smaller series?  I don't know,
that also can lead to some confusion, so I won't strongly
advocate that.  But it's something to consider for future
work regardless.

					-Alex



[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