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

[Marek Vasut]

On 3/4/25 10:04 AM, Alice Guo (OSS) wrote:
>> -----邮件原件-----
>> 发件人: Marek Vasut <marex at denx.de>
>> 发送时间: 2025年3月4日 4:19
>> 收件人: 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>
>> 主题: Re: 回复: [EXT] Re: [PATCH v4 10/20] scmi_protocols: update struct
>> scmi_base_discover_list_protocols_out
>>
>> On 3/3/25 11:37 AM, Alice Guo (OSS) wrote:
>>
>> Hi,
>>
>>>>> @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.
>> NAK, let's not perpetuate this suboptimal implementation, please implement
>> PROTOCOL_ATTRIBUTES if that's the way to find out the amount of supported
>> protocols dynamically.
> 
> OK. Will do it according to your comment.

Thank you !