Hi Simon,
> From: s...@google.com <s...@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.delau...@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.delau...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
