Messages in this thread | | | Date | Tue, 14 Jan 2020 14:56:15 +0530 | From | Viresh Kumar <> | Subject | 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 <viresh.kumar@linaro.org> wrote: > > On 10-01-20, 12:15, Arnd Bergmann wrote: > > > On Fri, Jan 10, 2020 at 10:43 AM Viresh Kumar <viresh.kumar@linaro.org> 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 ? > > > > > +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 ? > > > > 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
| |