linux-kernel - Re: [PATCH V3] firmware: arm_scmi: Make scmi core independent of the transport type

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200123023924.roqc2iyx4wmukk4p@vireshk-i7>
Date:   Thu, 23 Jan 2020 08:09:24 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     Cristian Marussi <cristian.marussi@....com>
Cc:     arnd@...db.de, Sudeep Holla <sudeep.holla@....com>,
        jassisinghbrar@...il.com, peng.fan@....com,
        peter.hilber@...nsynergy.com, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH V3] firmware: arm_scmi: Make scmi core independent of the
 transport type

On 22-01-20, 12:44, Cristian Marussi wrote:
> On 21/01/2020 08:27, Viresh Kumar wrote:
> commment is obsolete

Right, they need to be checked everywhere again. Sorry for missing
that earlier.

> > +struct scmi_chan_info {
> > +	struct scmi_info *info;
> > +	struct device *dev;
> > +	struct scmi_handle *handle;
> > +	void *transport_info;
> > +};
> > +
> > +/**
> > + * struct scmi_transport_ops - Structure representing a SCMI transport ops
> > + *
> > + * @send_message: Callback to send a message
> > + * @mark_txdone: Callback to mark tx as done
> > + * @chan_setup: Callback to allocate and setup a channel
> > + * @chan_free: Callback to free a channel
> > + */
> commment is obsolete but I would also ask: are all of these operations supposed to be mandatory supported
> on any possible foreseeable transport ? (beside the obviously needed like send_message)
> 
> I'm asking because they are all called straight away from the driver core without any NULL check
> so that if a new transport should not need one of them it will be forced to anyway implement a dummy one
> to comply, which it will be needlessly invoked every time.

They are kept as mandatory for now as we don't really know how it
will look for other transport types. Lets make them optional only when
someone don't need to define them. It would be a simple change anyway.

> >  /* Each compatible listed below must have descriptor associated with it */
> >  static const struct of_device_id scmi_of_match[] = {
> > -	{ .compatible = "arm,scmi", .data = &scmi_generic_desc },
> > +	{ .compatible = "arm,scmi", .data = &scmi_mailbox_desc },
> >  	{ /* Sentinel */ },
> >  };
> 
> minor thing: shouldn't the chosen transport being configurable at compile time with some
> option like CONFIG_SCMI_TRANSPORT_MBOX ? or via DT ?

It is configurable via DT. The compatible should look different in
that case, something like: "arm,scmi-<newtranport>".

-- 
viresh