linux-kernel - Re: [PATCH] firmware: arm_scmi: Make scmi core independent of 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: <20191210053448.ugjzbp2puzvnm37f@vireshk-i7>
Date:   Tue, 10 Dec 2019 11:04:48 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     Cristian Marussi <cristian.marussi@....com>
Cc:     Sudeep Holla <sudeep.holla@....com>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Arnd Bergmann <arnd@...db.de>,
        Jassi Brar <jassisinghbrar@...il.com>,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] firmware: arm_scmi: Make scmi core independent of
 transport type

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