Re: [PATCH v6 08/14] firmware: scmi: add a version check against base protocol

[Etienne CARRIERE - foss <etienne.carriere@foss.st.com>]

> From: U-Boot <u-boot-bounces@lists.denx.de> on behalf of AKASHI Takahiro <takahiro.akashi@linaro.org>
> Sent: Wednesday, October 11, 2023 12:07 PM
> 
> In SCMI base protocol version 2 (0x20000), new interfaces,
> BASE_SET_DEVICE_PERMISSIONS/BASE_SET_PROTOCOL_PERMISSIONS/
> BASE_RESET_AGENT_CONFIGURATION, were added. Moreover, the api of
> BASE_DISCOVER_AGENT was changed to support self-agent discovery.
> 
> So the driver expects SCMI firmware support version 2 of base protocol.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> v6
> * new commit
> ---
>  drivers/firmware/scmi/base.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/firmware/scmi/base.c b/drivers/firmware/scmi/base.c
> index ee84e261945a..1d41a8a98fc6 100644
> --- a/drivers/firmware/scmi/base.c
> +++ b/drivers/firmware/scmi/base.c
> @@ -481,6 +481,7 @@ static int scmi_base_reset_agent_configuration_int(struct udevice *dev,
>   */
>  static int scmi_base_probe(struct udevice *dev)
>  {
> +       u32 version;
>         int ret;
> 
>         ret = devm_scmi_of_get_channel(dev);
> @@ -488,6 +489,13 @@ static int scmi_base_probe(struct udevice *dev)
>                 dev_err(dev, "get_channel failed\n");
>                 return ret;
>         }
> +       ret = scmi_base_protocol_version_int(dev, &version);
> +       if (ret) {
> +               dev_err(dev, "getting protocol version failed\n");
> +               return ret;
> +       }
> +       if (version < SCMI_BASE_PROTOCOL_VERSION)
> +               return -EINVAL;

LGTM. The open source SCMI server implementations I'm aware of (scp-firmware, tf-a and optee-os) all report SCMI Base protocol version v2.0.
Reviewed-by: Etienne Carriere <etienne.carriere@foss.st.com>

That said, maybe a more flexible implementation would support both version v1.0 (0x10000) and v2.0
but disable permission commands for v1.0 compliant servers. Maybe in a later change, if there is a need for.
I fear using such strict minima protocol version values for other SCMI procotols make U-Boot client too restrictive.

BR,
Etienne

> 
>         return ret;
>  }
> --
> 2.34.1
> 
>