linux-kernel - Re: [PATCH v1 3/6] firmware: arm_scmi: Use max-rx-timeout-ms from devicetree

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: <1ea7fff0-149d-408a-b5a7-1b223e8509d0@gmail.com>
Date: Thu, 12 Sep 2024 14:05:53 -0700
From: Florian Fainelli <f.fainelli@...il.com>
To: Cristian Marussi <cristian.marussi@....com>,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 arm-scmi@...r.kernel.org, devicetree@...r.kernel.org
Cc: sudeep.holla@....com, james.quinlan@...adcom.com,
 vincent.guittot@...aro.org, etienne.carriere@...com, peng.fan@....nxp.com,
 michal.simek@....com, quic_sibis@...cinc.com, quic_nkela@...cinc.com,
 dan.carpenter@...aro.org, souvik.chakravarty@....com, robh@...nel.org,
 krzk+dt@...nel.org, Peng Fan <peng.fan@....com>
Subject: Re: [PATCH v1 3/6] firmware: arm_scmi: Use max-rx-timeout-ms from
 devicetree

On 7/30/24 07:47, Cristian Marussi wrote:
> Override default maximum RX timeout with the value picked from the
> devicetree, when provided.
> 
> Suggested-by: Peng Fan <peng.fan@....com>
> Signed-off-by: Cristian Marussi <cristian.marussi@....com>
> ---
>   drivers/firmware/arm_scmi/driver.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> index 332cd5207bbc..e7dab0eea540 100644
> --- a/drivers/firmware/arm_scmi/driver.c
> +++ b/drivers/firmware/arm_scmi/driver.c
> @@ -2964,6 +2964,7 @@ static int scmi_debugfs_raw_mode_setup(struct scmi_info *info)
>   static const struct scmi_desc *scmi_transport_setup(struct device *dev)
>   {
>   	struct scmi_transport *trans;
> +	int ret;
>   
>   	trans = dev_get_platdata(dev);
>   	if (!trans || !trans->desc || !trans->supplier || !trans->core_ops)
> @@ -2980,6 +2981,14 @@ static const struct scmi_desc *scmi_transport_setup(struct device *dev)
>   
>   	dev_info(dev, "Using %s\n", dev_driver_string(trans->supplier));
>   
> +	ret = of_property_read_u32(dev->of_node, "max-rx-timeout-ms",
> +				   &trans->desc->max_rx_timeout_ms);
> +	if (ret && ret != -EINVAL)
> +		dev_err(dev, "Malformed max-rx-timeout-ms DT property.\n");
> +
> +	dev_info(dev, "SCMI max-rx-timeout: %dms\n",
> +		 trans->desc->max_rx_timeout_ms);

I am bit on the fence on that change, it is useful, and we have done 
similar things before using a command line parameter.

This is definitively useful when bringing up new systems where you might 
be sprinkling enough debugging messages that this pushes your message 
processing logic too close to the default 30ms timeout. For normal use 
cases, we really want the message timeout to be as small as possible for 
most SCMI traffic but if we want the timeout to be configurable, that 
might have have to be on a per-message basis.
-- 
Florian