Re: [PATCH 07/10] firmware: scmi: mailbox: Support arm,max_rx_timeout_ms

[Tom Rini]

On Sat, Sep 27, 2025 at 12:06:20AM +0800, Peng Fan (OSS) wrote:

> From: Peng Fan <peng....@nxp.com>
> 
> Per devicetree bindings:
> arm,max-rx-timeout-ms indicates an optional time value, expressed in
> milliseconds, representing the transport maximum timeout value for the
> receive channel. The value should be a non-zero value if set.

This is what dts/upstream/Bindings/firmware/arm,scmi.yaml says, along
with a minimum value of 1, implying that yes, number of milliseconds and
not value in milliseconds.

> 
> Support this property if platform set it to a non-default value. This
> property is a per SCMI property, so all channels share same value.
> 
> Signed-off-by: Peng Fan <peng....@nxp.com>
> ---
>  drivers/firmware/scmi/mailbox_agent.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/firmware/scmi/mailbox_agent.c 
> b/drivers/firmware/scmi/mailbox_agent.c
> index 
> 32b0fd9f589a30c2bb8bdbe8d655c3d47d0abd5a..16a82f55ab7f7550e7744066cd640b0c439f75eb
>  100644
> --- a/drivers/firmware/scmi/mailbox_agent.c
> +++ b/drivers/firmware/scmi/mailbox_agent.c
> @@ -16,7 +16,7 @@
>  
>  #include "smt.h"
>  
> -#define TIMEOUT_US_30MS                      30000
> +#define TIMEOUT_30MS                 30
>  
>  /**
>   * struct scmi_mbox_channel - Description of an SCMI mailbox transport
> @@ -73,6 +73,7 @@ out:
>  
>  static int setup_channel(struct udevice *dev, struct scmi_mbox_channel *chan)
>  {
> +     struct scmi_mbox_channel *base_chan = dev_get_plat(dev);
>       int ret;
>  
>       ret = mbox_get_by_index(dev, 0, &chan->mbox);
> @@ -87,7 +88,7 @@ static int setup_channel(struct udevice *dev, struct 
> scmi_mbox_channel *chan)
>               return ret;
>       }
>  
> -     chan->timeout_us = TIMEOUT_US_30MS;
> +     chan->timeout_us = base_chan->timeout_us;
>  
>       return 0;
>  }
> @@ -127,6 +128,9 @@ int scmi_mbox_of_to_plat(struct udevice *dev)
>  {
>       struct scmi_mbox_channel *chan = dev_get_plat(dev);
>  
> +     chan->timeout_us = dev_read_u32_default(dev, "arm,max-rx-timeout-ms",
> +                                             TIMEOUT_30MS) * 1000;
> +
>       return setup_channel(dev, chan);
>  }

So the rest of this makes sense.

Reviewed-by: Tom Rini <tr...@konsulko.com>

However:
$ rg -g *.dtsi arm,max-rx-timeout-ms
dts/upstream/src/arm64/freescale/imx95.dtsi:                    
arm,max-rx-timeout-ms = <5000>;
dts/upstream/src/arm64/freescale/imx94.dtsi:                    
arm,max-rx-timeout-ms = <5000>;

Is the timeout really 5 seconds on these platforms?

-- 
Tom

signature.asc
Description: PGP signature