[RFC 3/3] firmware: scmi: add a sanity check against protocol version

[Etienne CARRIERE]

> From: AKASHI Takahiro <takahiro.akashi at linaro.org>
> Sent: Friday, July 28, 2023 02:33
> Subject: [RFC 3/3] firmware: scmi: add a sanity check against protocol version
> 
> SCMI_PROTOCOL_VERSION is a mandatory command for all the SCMI protocols.
> With this patch, this command is implemented on each protocol.
> Then, we can assure that the feature set implemented by SCMI Server
> (firmware) is compatible with U-Boot, that is, each protocol's version
> must be equal to or higher than the one that U-Boot's corresponding driver
> expects.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> ---

This change enforces some protocol versions that may not be reported
by known open source scmi server implementations: scp-firmware, tf-a
and op-tee. 

Latest scp-firwmare/op-tee/tf-a report the following verisons numbers:
* base protocol : 0x20000 all
* clock protocol: 0x10000 for scp-firwmare / 0x20000 for op-tee and tf-a.
* reset-dom: 0x1000 for all
* voltage-dom: 0x10000 for scp-firmware / 0x30000 for op-tee / not used in tf-a.
* power-dom: 0x20000 for scp-firmware / 0x21000 for tf-a / not used in op-tee.

Regarding the SCMI specification, these protocol versions evolve with the specification version:
---------- ---base -powerD --clock -resetD --voltD
SCMI v1.0: 0x10000 0x10000 0x10000 ------- -------
SCMI v2.0: 0x20000 0x20000 0x10000 0x10000 -------
SCMI v3.0: 0x20000 0x21000 0x10000 0x20000 0x10000
SCMI v3.1: 0x20000 0x30000 0x30000 0x30000 0x20000
SCMI v3.2: 0x20000 0x30000 0x30000 0x30000 0x20000

This patch enforces the versions defined in SCMI v3.2.
Being strict in protocol version is not ideal I think.

BR,
etienne

>  drivers/clk/clk_scmi.c                   |  6 ++++++
>  drivers/power/regulator/scmi_regulator.c |  6 ++++++
>  drivers/reset/reset-scmi.c               | 14 +++++++++++++-
>  include/scmi_protocols.h                 |  6 ++++++
>  4 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
> index 34a49363a51a..3591acb6d3a9 100644
> --- a/drivers/clk/clk_scmi.c
> +++ b/drivers/clk/clk_scmi.c
> @@ -138,12 +138,18 @@ static int scmi_clk_probe(struct udevice *dev)
>  {
>         struct clk *clk;
>         size_t num_clocks, i;
> +       u32 version;
>         int ret;
> 
>         ret = devm_scmi_of_get_channel(dev);
>         if (ret)
>                 return ret;
> 
> +       ret = scmi_generic_protocol_version(dev, SCMI_PROTOCOL_ID_CLOCK,
> +                                           &version);
> +       if (ret || version < SCMI_CLOCK_PROTOCOL_VERSION)
> +               return -EINVAL;
> +
>         if (!CONFIG_IS_ENABLED(CLK_CCF))
>                 return 0;
> 
> diff --git a/drivers/power/regulator/scmi_regulator.c b/drivers/power/regulator/scmi_regulator.c
> index 08918b20872c..936768d0eeeb 100644
> --- a/drivers/power/regulator/scmi_regulator.c
> +++ b/drivers/power/regulator/scmi_regulator.c
> @@ -138,12 +138,18 @@ static int scmi_regulator_probe(struct udevice *dev)
>                 .out_msg = (u8 *)&out,
>                 .out_msg_sz = sizeof(out),
>         };
> +       u32 version;
>         int ret;
> 
>         ret = devm_scmi_of_get_channel(dev);
>         if (ret)
>                 return ret;
> 
> +       ret = scmi_generic_protocol_version(dev, SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN,
> +                                           &version);
> +       if (ret || version < SCMI_VOLTDOM_PROTOCOL_VERSION)
> +               return -EINVAL;
> +
>         /* Check voltage domain is known from SCMI server */
>         in.domain_id = pdata->domain_id;
> 
> diff --git a/drivers/reset/reset-scmi.c b/drivers/reset/reset-scmi.c
> index b76711f0a8fb..dbd98dbdbc4f 100644
> --- a/drivers/reset/reset-scmi.c
> +++ b/drivers/reset/reset-scmi.c
> @@ -73,7 +73,19 @@ static const struct reset_ops scmi_reset_domain_ops = {
> 
>  static int scmi_reset_probe(struct udevice *dev)
>  {
> -       return devm_scmi_of_get_channel(dev);
> +       u32 version;
> +       int ret;
> +
> +       ret = devm_scmi_of_get_channel(dev);
> +       if (ret)
> +               return ret;
> +
> +       ret = scmi_generic_protocol_version(dev, SCMI_PROTOCOL_ID_RESET_DOMAIN,
> +                                           &version);
> +       if (ret || version < SCMI_RESETDOM_PROTOCOL_VERSION)
> +               return -EINVAL;
> +
> +       return 0;
>  }
> 
>  U_BOOT_DRIVER(scmi_reset_domain) = {
> diff --git a/include/scmi_protocols.h b/include/scmi_protocols.h
> index 64fd740472b5..6ab16efd49cc 100644
> --- a/include/scmi_protocols.h
> +++ b/include/scmi_protocols.h
> @@ -398,6 +398,8 @@ int scmi_generic_protocol_version(struct udevice *dev,
>   * SCMI Clock Protocol
>   */
> 
> +#define SCMI_CLOCK_PROTOCOL_VERSION 0x20000

TF-A and OP-TEE scmi servers report version 0x20000 for clock protocol but SCP-firmware reports version 0x10000:
https://github.com/ARM-software/SCP-firmware/blob/master/module/scmi_clock/include/internal/scmi_clock.h#L16


> +
>  enum scmi_clock_message_id {
>         SCMI_CLOCK_ATTRIBUTES = 0x3,
>         SCMI_CLOCK_RATE_SET = 0x5,
> @@ -509,6 +511,8 @@ struct scmi_clk_rate_set_out {
>   * SCMI Reset Domain Protocol
>   */
> 
> +#define SCMI_RESETDOM_PROTOCOL_VERSION 0x30000
> +
>  enum scmi_reset_domain_message_id {
>         SCMI_RESET_DOMAIN_ATTRIBUTES = 0x3,
>         SCMI_RESET_DOMAIN_RESET = 0x4,
> @@ -569,6 +573,8 @@ struct scmi_rd_reset_out {
>   * SCMI Voltage Domain Protocol
>   */
> 
> +#define SCMI_VOLTDOM_PROTOCOL_VERSION 0x20000
> +
>  enum scmi_voltage_domain_message_id {
>         SCMI_VOLTAGE_DOMAIN_ATTRIBUTES = 0x3,
>         SCMI_VOLTAGE_DOMAIN_CONFIG_SET = 0x5,
> --
> 2.41.0
> 
>