Re: [PATCH V2] firmware: arm_scmi: Make scmi core independent of transport type

On 13-01-20, 12:36, Arnd Bergmann wrote:

On Mon, Jan 13, 2020 at 7:42 AM Viresh Kumar [off-list ref] wrote:

quoted

On 10-01-20, 12:15, Arnd Bergmann wrote:

quoted

On Fri, Jan 10, 2020 at 10:43 AM Viresh Kumar [off-list ref] wrote:

Simply dropping the __iomem isn't much better, now you get other
type mismatches.

Right. So what exactly do you suggest I should do now? Drop __iomem
from the structure's payload field but keep all local variables and
function arguments with __iomem ?

quoted

quoted

quoted

+struct scmi_chan_info {
+       void *payload;
+       struct device *dev;
+       struct scmi_handle *handle;
+       void *transport_info;
+};

Maybe you can wrap the scmi_chan_info inside of another
structure that contains  the payload pointer, and use container_of
to convert between them?

We don't need to convert between the two of them, isn't it ? Are you
referring some other field here ?

quoted

quoted

It's not obvious which parts of the structure should be shared and
which are transport specific.

All transport specific information is kept in the transport specific
structure which is saved here in the transport_info field. Is there
something else that isn't clear ?

To answer all three, what I meant is that the payload pointer appears
to be transport specific and

I am not sure if I understood the below statement properly. Is there
something missing from it ?

should not be part of the common
structure if there is generic way to access it.

The scmi protocol requires a block of shared memory which is
represented by struct scmi_shared_mem, and payload is this memory
block itself. This block of memory is accessed throughout driver.c
file using ioread/write commands. If payload is transport specific, so
will be those accesses, isn't it ? Are you suggesting to move all this
to mailbox.c (the transport specific file) instead ? I am sorry, but I
am not able to understand how exactly you want me to reorder code here
:(

@Sudeep: I had a question for you though. Looks like we are doing
ioremap() of this payload for every channel's tx/rx, why ? Why is the
same memory area mapped that way ? Can we just map the area once for
scmi block ?

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel