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

Viresh Kumar
2019-12-10T05:34:53Z

On 09-12-19, 18:13, Cristian Marussi wrote:
> On 29/11/2019 09:31, Viresh Kumar wrote:
> > /**
> > * struct scmi_info - Structure representing a SCMI instance
> > *
> > @@ -128,6 +109,7 @@ struct scmi_chan_info {
> > struct scmi_info {
> > struct device *dev;
> > const struct scmi_desc *desc;
> > + struct scmi_transport_ops *transport_ops;
> > struct scmi_revision_info version;
> > struct scmi_handle handle;
> > struct scmi_xfers_info tx_minfo;
> > @@ -138,7 +120,6 @@ struct scmi_info {
> > int users;
> > };
> >
>
> Could we add also the related @transport_ops in the above comment block ?

Ah, I missed that.

> > desc = of_device_get_match_data(dev);
> > if (!desc)
> > return -EINVAL;
>
> This scmi_desc struct descriptor is retrieved from of_match_table .data and points to
> the driver-provided scmi_generic_desc
>
> static const struct scmi_desc scmi_generic_desc = {
> .max_rx_timeout_ms = 30, /* We may increase this if required */
> .max_msg = 20, /* Limited by MBOX_TX_QUEUE_LEN */
> .max_msg_size = 128,
> };
>
> Is not this kind of information possibly (maybe partially) related to the selected
> transport, and as such it should be also provided dynamically by the chosen transport
> layer at probe time, like the transport_ops, instead of being hard-coded in
> this driver ?

I had my doubts about this thing and I missed checking it out.

@Sudeep: Is this information completely mailbox specific ? Should I move it to
mailbox.c here ?

--
viresh