Re: [PATCH v3 05/11] mfd: lm3533: Convert to use OF bindings

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Tue, Jun 02, 2026 at 01:31:44PM +0300, Svyatoslav Ryhel wrote:
> вт, 2 черв. 2026 р. о 11:24 Andy Shevchenko <andriy.shevchenko@intel.com> пише:
> > On Mon, Jun 01, 2026 at 06:18:25PM +0300, Svyatoslav Ryhel wrote:


...

> > > +     device_for_each_child_node_scoped(lm3533->dev, child) {
> >
> > > +             if (!fwnode_device_is_available(child))
> > > +                     continue;
> >
> > Do we need this check?
> 
> This is nice to have if the node is disabled. If we assume that there
> are no disabled nodes, I can remove it.

It's already implied. See

static struct fwnode_handle *
of_fwnode_get_next_child_node(const struct fwnode_handle *fwnode, struct fwnode_handle *child)
{
        return of_fwnode_handle(of_get_next_available_child(to_of_node(fwnode), to_of_node(child)));
}

And I believe it's written somewhere in the documentation (if not, feel free to
patch that).

...

> > > +     ret = sysfs_create_group(&dev->kobj, &lm3533_attribute_group);
> >
> > No way. You should use .dev_groups.
> 
> I did not change how driver does this, just swapped lm3533->dev to
> dev. I will set is back as it was.

This is a serious race condition that needs to be addressed. Since you are
touching this driver the fixes against known issues probably are the first
things that have to be done.

> > > +     if (ret) {
> > > +             dev_err(dev, "failed to create sysfs attributes\n");
> > >               goto err_unregister;
> > >       }

...

> > Can you think on how to split this change to smaller steps? I believe it's
> > possible.
> 
> No, I am done with tinkering with this patchset. It is broken enough
> and it has inflated enough.

Probably you don't want this to be reviewed then? I believe other kernel
developers and maintainers will ask you the same.

-- 
With Best Regards,
Andy Shevchenko