Messages in this thread Patch in this message | | | From | Etienne Carriere <> | Date | Thu, 30 Jan 2020 10:25:58 +0100 | Subject | Re: [PATCH V5] firmware: arm_scmi: Make scmi core independent of the transport type |
| |
Hello Viresh,
On Tue, Jan 28, 2020 at 04:24:19PM +0530, Viresh Kumar wrote: > The SCMI specification is fairly independent of the transport protocol, > which can be a simple mailbox (already implemented) or anything else. > The current Linux implementation however is very much dependent on the > mailbox transport layer. > > This patch makes the SCMI core code (driver.c) independent of the > mailbox transport layer and moves all mailbox related code to a new > file: mailbox.c. > > We can now implement more transport protocols to transport SCMI > messages. > > The transport protocols just need to provide struct scmi_transport_ops, > with its version of the callbacks to enable exchange of SCMI messages. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > @Sudeep: Please help getting this tested as well :) > > V4->V5: > - struct scmi_shared_mem is moved to mailbox.c and it is completely > handled by transport layer now. > - And so lots of ops change due to this. > - Fixed a bug from previous version where wrong dev structure was > getting passed to devm_kzalloc(). >
Hello Viresh, Sudeep, I've made a first port (draft) for adding new transport channels, next to existing mailbox channel, on top of your change. You can find it here: https://github.com/etienne-lms/linux/pull/1.
I don't have specific comments on your change but the one below. I think SMT header should move out of mailbox.c, but that may be a bit out of the scope of your change.
> (...) > > @@ -470,13 +310,7 @@ int scmi_do_xfer(const struct scmi_handle *handle, struct scmi_xfer *xfer) > if (!ret && xfer->hdr.status) > ret = scmi_to_linux_errno(xfer->hdr.status); > > - /* > - * NOTE: we might prefer not to need the mailbox ticker to manage the > - * transfer queueing since the protocol layer queues things by itself. > - * Unfortunately, we have to kick the mailbox framework after we have > - * received our message. > - */ > - mbox_client_txdone(cinfo->chan, ret); > + info->desc->ops->mark_txdone(cinfo, ret); > > return ret; > }
I would prefer an optional mak_txdone callback:
if (info->desc->ops->mark_txdone) info->desc->ops->mark_txdone(cinfo, ret);
> @@ -713,29 +547,18 @@ static int scmi_xfer_info_init(struct scmi_info *sinfo) > return 0; > } > > -static int scmi_mailbox_check(struct device_node *np, int idx) > -{ > - return of_parse_phandle_with_args(np, "mboxes", "#mbox-cells", > - idx, NULL); > -} > - > -static int scmi_mbox_chan_setup(struct scmi_info *info, struct device *dev, > - int prot_id, bool tx) > +static int scmi_chan_setup(struct scmi_info *info, struct device *dev, > + int prot_id, bool tx) > > (...)
Regards, Etienne
| |