回复: [EXT] Re: [PATCH v4 10/20] scmi_protocols: update struct scmi_base_discover_list_protocols_out

[Alice Guo (OSS)]

> -----邮件原件-----
> 发件人: Marek Vasut <marex at denx.de>
> 发送时间: 2025年1月24日 22:38
> 收件人: Alice Guo (OSS) <alice.guo at oss.nxp.com>; Tom Rini
> <trini at konsulko.com>; Stefano Babic <sbabic at denx.de>; Fabio Estevam
> <festevam at gmail.com>; dl-uboot-imx <uboot-imx at nxp.com>; Lukasz
> Majewski <lukma at denx.de>; Sean Anderson <seanga2 at gmail.com>; Simon
> Glass <sjg at chromium.org>; Alper Nebi Yasak <alpernebiyasak at gmail.com>
> 抄送: u-boot at lists.denx.de; tharvey at gateworks.com; Alice Guo
> <alice.guo at nxp.com>; Ye Li <ye.li at nxp.com>
> 主题: [EXT] Re: [PATCH v4 10/20] scmi_protocols: update struct
> scmi_base_discover_list_protocols_out
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report this
> email' button
> 
> 
> On 1/15/25 2:28 PM, Alice Guo wrote:
> > From: Ye Li <ye.li at nxp.com>
> >
> > @protocols is an array of protocol identifiers that are implemented,
> > excluding the Base protocol. The number of elements of @protocols is
> > specified by callee-side. Currently, set it to 4 is enough for i.MX95.
> Can you please try something like this instead ? That should be
> future-proof:
> 
> diff --git a/drivers/firmware/scmi/base.c b/drivers/firmware/scmi/base.c index
> f4e3974ff5b..92d278edfaf 100644
> --- a/drivers/firmware/scmi/base.c
> +++ b/drivers/firmware/scmi/base.c
> @@ -258,17 +258,16 @@ static int
> scmi_base_discover_impl_version_int(struct udevice *dev,
>   static int scmi_base_discover_list_protocols_int(struct udevice *dev,
>                                                  u8 **protocols)
>   {
> -       struct scmi_base_discover_list_protocols_out out;
> +       struct scmi_base_discover_list_protocols_out *out;
>         int cur;
>         struct scmi_msg msg = {
>                 .protocol_id = SCMI_PROTOCOL_ID_BASE,
>                 .message_id = SCMI_BASE_DISCOVER_LIST_PROTOCOLS,
>                 .in_msg = (u8 *)&cur,
>                 .in_msg_sz = sizeof(cur),
> -               .out_msg = (u8 *)&out,
> -               .out_msg_sz = sizeof(out),

The pointer and byte size of the buffer where the response message is stored should be provided before sending a BASE_DISCOVER_LIST PROTOCOLS message.

There is a more reasonable way. Number of protocols that are implemented, excluding the Base protocol, can be obtained by sending a PROTOCOL_ATTRIBUTES message. But now this command is not supported in U-Boot. In this patch, ec8727b7e1 ("firmware: scmi: implement SCMI base protocol"), "u32 protocols[3];" is used. So, I think setting the array of packed SCMI protocol ID's to a fixed size is acceptable.

Best Regards,
Alice Guo

>         };
>         u32 num_agents, num_protocols;
> +       int size;
>         u8 *buf;
>         int i, ret;
> 
> @@ -276,29 +275,34 @@ static int
> scmi_base_discover_list_protocols_int(struct udevice *dev,
>         if (ret)
>                 return ret;
> 
> -       buf = calloc(sizeof(u8), num_protocols);
> -       if (!buf)
> +       size = sizeof(*out) + sizeof(u8) * num_protocols;
> +       out = calloc(1, size);
> +       if (!out)
>                 return -ENOMEM;
> 
> +       msg.out_msg = (u8 *)out;
> +       msg.out_msg_sz = size;
> +       buf = (u8 *)out->protocols;
> +
>         cur = 0;
>         do {
>                 ret = devm_scmi_process_msg(dev, &msg);
>                 if (ret)
>                         goto err;
> -               if (out.status) {
> -                       ret = scmi_to_linux_errno(out.status);
> +               if (out->status) {
> +                       ret = scmi_to_linux_errno(out->status);
>                         goto err;
>                 }
> 
> -               for (i = 0; i < out.num_protocols; i++, cur++)
> -                       buf[cur] = out.protocols[i / 4] >> ((i % 4) * 8);
> +               for (i = 0; i < out->num_protocols; i++, cur++)
> +                       buf[cur] = out->protocols[i / 4] >> ((i % 4) *
> + 8);
>         } while (cur < num_protocols);
> 
>         *protocols = buf;
> 
>         return num_protocols;
>   err:
> -       free(buf);
> +       free(out);
> 
>         return ret;
>   }
> diff --git a/include/scmi_protocols.h b/include/scmi_protocols.h index
> a51234eda5c..29a7fd85cc0 100644
> --- a/include/scmi_protocols.h
> +++ b/include/scmi_protocols.h
> @@ -148,7 +148,7 @@ struct scmi_base_discover_impl_version_out {
>   struct scmi_base_discover_list_protocols_out {
>         s32 status;
>         u32 num_protocols;
> -       u32 protocols[4];
> +       u32 protocols[];
>   };
> 
>   /**