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

[Florian Fainelli]

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@xxxxxxx>
> Signed-off-by: Cristian Marussi <cristian.marussi@xxxxxxx>
> ---
>   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