lkml.org 
[lkml]   [2020]   [Jan]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    SubjectRe: [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

    \
     
     \ /
      Last update: 2020-01-30 10:26    [W:4.466 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site