Re: [PATCH v3 5/6] of: dynamic: Move dead property list check into property add/update functions

[Rob Herring <robh@kernel.org>]

On Mon, Aug 21, 2023 at 5:49 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, Aug 18, 2023 at 03:41:00PM -0500, Rob Herring wrote:
> > The changeset code checks for a property in the deadprops list when
> > adding/updating a property, but of_add_property() and
> > of_update_property() do not. As the users of these functions are pretty
> > simple, they have not hit this scenario or else the property lists
> > would get corrupted.
> >
> > With this there are 3 cases of removing a property from either deadprops
> > or properties lists, so add a helper to find and remove a matching
> > property.
>
> ...
>
> > v3:
> >  - Keep existing style in deadprops loop
>
> Not sure where exactly in the code that one, but...

That was your previous comment...

>
> ...
>
> >  int __of_remove_property(struct device_node *np, struct property *prop)
> >  {
> > -     struct property **next;
> > -
> > -     for (next = &np->properties; *next; next = &(*next)->next) {
> > -             if (*next == prop)
> > -                     break;
> > +     if (__of_remove_property_from_list(&np->properties, prop)) {
> > +             /* Found the property, add it to deadprops list */
> > +             prop->next = np->deadprops;
> > +             np->deadprops = prop;
> > +             return 0;
> >       }
> > -     if (*next == NULL)
> > -             return -ENODEV;
> > -
> > -     /* found the node */
> > -     *next = prop->next;
> > -     prop->next = np->deadprops;
> > -     np->deadprops = prop;
> >
> > -     return 0;
> > +     return -ENODEV;
> >  }
>
>
> ...if it's this one, I don't see how it's better than
>
>         if (!__of_remove_property_from_list(&np->properties, prop))
>                 return -ENODEV;

Because this way doesn't work well when we move the spinlock in here.
Maybe cleanup.h will help, but I'm not going to do that now. If we do,
then I'll do it for the whole subsystem/file.

Rob