Re: [RFC 1/6] firmware: scmi: fix protocol enumeration logic

Hi Simon,

On Sun, Sep 10, 2023 at 01:13:26PM -0600, Simon Glass wrote:

Hi AKASHI,

On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro takahiro.akashi@linaro.org wrote:

...

The original logic in enumerating all the protocols accidentally modifies a *loop* variable, node, at Voltage domain protocol. So subsequent protocol nodes in a device tree won't be detected.

Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org

drivers/firmware/scmi/scmi_agent-uclass.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c b/drivers/firmware/scmi/scmi_agent-uclass.c index 46a2933d51a4..79584c00a066 100644 --- a/drivers/firmware/scmi/scmi_agent-uclass.c +++ b/drivers/firmware/scmi/scmi_agent-uclass.c @@ -422,8 +422,11 @@ static int scmi_bind_protocols(struct udevice *dev) case SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN: if (IS_ENABLED(CONFIG_DM_REGULATOR_SCMI) && scmi_protocol_is_supported(dev, protocol_id)) {

•                           node = ofnode_find_subnode(node, "regulators");
  

•                           if (!ofnode_valid(node)) {
  

•                           ofnode sub_node;
  

•                           sub_node = ofnode_find_subnode(node,
  

•                                                          "regulators");
  

•                           if (!ofnode_valid(sub_node)) {
                                    dev_err(dev, "no regulators node\n");
                                    return -ENXIO;
                            }
  

-- 2.34.1

This function is very ugly...could we instead rely on driver model to

I think you are talking about scmi_bind_protocols() itself, not the hunk above in my patch, which is merely a bug fix in the existing code. If so,

bind the devices? It seems unfortunate to have to write code for something which could be done with no extra code.

We will discuss in the threads on your comment [1].

[1] https://lists.denx.de/pipermail/u-boot/2023-September/530200.html

-Takahiro Akashi

Regards, Simon