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

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

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