[U-Boot] [PATCH] syscon: update syscon_node_to_regmap to use the DM functions

[Patrick DELAUNAY]

Hi Simon,

> From: sjg at google.com <sjg at google.com> On Behalf Of Simon Glass
> Sent: mercredi 31 octobre 2018 01:11
> 
> HI Patrick,
> 
> On 30 October 2018 at 07:44, Patrick Delaunay <patrick.delaunay at st.com>
> wrote:
> >
> > + Update the function syscon_node_to_regmap() to force bound on
> >   syscon uclass and directly use the list of device from DM.
> > + Remove the static list syscon_list.
> >
> > This patch avoid issue (crash) when syscon_node_to_regmap() is called
> > before and after reallocation (list content is invalid).
> >
> > Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
> >
> >
> >  drivers/core/syscon-uclass.c | 55
> > ++++++++++++++------------------------------
> >  1 file changed, 17 insertions(+), 38 deletions(-)
> >
> > diff --git a/drivers/core/syscon-uclass.c
> > b/drivers/core/syscon-uclass.c index 303e166..aeb3583 100644
> > --- a/drivers/core/syscon-uclass.c
> > +++ b/drivers/core/syscon-uclass.c
> > @@ -123,52 +123,31 @@ U_BOOT_DRIVER(generic_syscon) = {
> >   * The syscon node can be bound to another driver, but still works
> >   * as a syscon provider.
> >   */
> > -static LIST_HEAD(syscon_list);
> > -
> > -struct syscon {
> > -       ofnode node;
> > -       struct regmap *regmap;
> > -       struct list_head list;
> > -};
> > -
> > -static struct syscon *of_syscon_register(ofnode node)
> > +struct regmap *syscon_node_to_regmap(ofnode node)
> >  {
> > -       struct syscon *syscon;
> > +       struct udevice *dev, *parent;
> > +       ofnode ofnode = node;
> 
> What is the purpose of this variable if it is always set to 'node'?

No need, I remove it.

> 
> >         int ret;
> >
> > +       if (!uclass_get_device_by_ofnode(UCLASS_SYSCON, node, &dev))
> > +               return syscon_get_regmap(dev);
> > +
> >         if (!ofnode_device_is_compatible(node, "syscon"))
> >                 return ERR_PTR(-EINVAL);
> >
> > -       syscon = malloc(sizeof(*syscon));
> > -       if (!syscon)
> > -               return ERR_PTR(-ENOMEM);
> > +       if (device_find_global_by_ofnode(ofnode, &parent))
> > +               parent = dm_root();
> 
> So if you fail to find the node, you use the root? Is this so that you have
> somewhere to 'hang' the device? I think this code could all use a few comments.

Yes exactly.
 it is to avoid binding error when the node with "syscon" compatible is not associated to a other U-Boot driver.
But It is more a protection,  normally if the driver is not bound to other U-boot driver, it should be bound to "syscon" at least.

I will add comment.

> >
> > -       ret = regmap_init_mem(node, &syscon->regmap);
> > -       if (ret) {
> > -               free(syscon);
> > +       /* force bound to syscon class */
> > +       ret = device_bind_driver_to_node(parent, "syscon",
> > +                                        ofnode_get_name(node),
> > +                                        node, &dev);
> > +       if (ret)
> >                 return ERR_PTR(ret);
> > -       }
> > -
> > -       list_add_tail(&syscon->list, &syscon_list);
> > -
> > -       return syscon;
> > -}
> >
> > -struct regmap *syscon_node_to_regmap(ofnode node) -{
> > -       struct syscon *entry, *syscon = NULL;
> > -
> > -       list_for_each_entry(entry, &syscon_list, list)
> > -               if (ofnode_equal(entry->node, node)) {
> > -                       syscon = entry;
> > -                       break;
> > -               }
> > -
> > -       if (!syscon)
> > -               syscon = of_syscon_register(node);
> > -
> > -       if (IS_ERR(syscon))
> > -               return ERR_CAST(syscon);
> > +       ret = device_probe(dev);
> > +       if (ret)
> > +               return ERR_PTR(ret);
> >
> > -       return syscon->regmap;
> > +       return syscon_get_regmap(dev);
> >  }
> > --
> > 2.7.4
> >
> 
> Regards,
> Simon

Regards

Patrick