Re: [PATCH v1 2/6] mfd: lm3533: Convert to use OF bindings

[Lee Jones <lee@kernel.org>]

On Sun, 17 May 2026, Svyatoslav Ryhel wrote:

> нд, 17 трав. 2026 р. о 10:43 Svyatoslav Ryhel <clamor95@gmail.com> пише:
> >
> > Since there are no users of this driver via platform data, remove the
> > platform data support and switch to using Device Tree bindings.
> > Additionally, optimize functions used only by platform data.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> >  drivers/iio/light/lm3533-als.c      | 123 +++++--------
> >  drivers/leds/leds-lm3533.c          |  60 ++++---
> >  drivers/mfd/lm3533-core.c           | 257 +++++++++-------------------
> >  drivers/video/backlight/lm3533_bl.c |  52 ++++--
> >  include/linux/mfd/lm3533.h          |  51 +-----
> >  5 files changed, 202 insertions(+), 341 deletions(-)

Please snip replies.

[...]

> > -static int lm3533_device_led_init(struct lm3533 *lm3533)
> > -{
> > -       struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
> > -       int i;
> > -       int ret;
> > -
> > -       if (!pdata->leds || pdata->num_leds == 0)
> > -               return 0;
> > -
> > -       if (pdata->num_leds > ARRAY_SIZE(lm3533_led_devs))
> > -               pdata->num_leds = ARRAY_SIZE(lm3533_led_devs);
> > -
> > -       for (i = 0; i < pdata->num_leds; ++i) {
> > -               lm3533_led_devs[i].platform_data = &pdata->leds[i];
> > -               lm3533_led_devs[i].pdata_size = sizeof(pdata->leds[i]);
> > +               dev_err(dev, "failed to set boost ovp\n");
> > +               goto err_disable;
> >         }
> >
> > -       ret = mfd_add_devices(lm3533->dev, 0, lm3533_led_devs,
> > -                             pdata->num_leds, NULL, 0, NULL);
> > +       ret = devm_mfd_add_devices(dev, 0, lm3533_child_devices,
> > +                                  ARRAY_SIZE(lm3533_child_devices),
> > +                                  NULL, 0, NULL);
> 
> Question to Lee Jones. Would you find acceptable if the driver will
> build cell list dynamically based on the nodes in the device tree?
> This is LED controller after all, not all leds can be populated and
> same LED control bank can be linked to all LVLEDs for example.
> 
> If you are ok, would this implementation satisfy you?

Generally not.  Create the non-dynamical information statically
(obviously not 'const'), then you can add dynamic data as you go.

>         struct mfd_cell lm3533_cells[LM3533_CELLS_MAX];
>         u32 count = 0, reg;
>         int ret;
> 
>         device_for_each_child_node_scoped(lm3533->dev, child) {
>                 if (!fwnode_device_is_available(child))
>                         continue;
> 
>                 if (count >= LM3533_CELLS_MAX)
>                         break;
> 
>                 if (fwnode_device_is_compatible(child, "ti,lm3533-als")) {
>                         lm3533_cells[count].name = "lm3533-als";
>                         lm3533_cells[count].id = PLATFORM_DEVID_NONE;
>                         lm3533_cells[count].of_compatible = "ti,lm3533-als";
> 
>                         lm3533->have_als = true;
>                 }
> 
>                 if (fwnode_device_is_compatible(child, "ti,lm3533-backlight")) {
>                         ret = fwnode_property_read_u32(child, "reg", &reg);
>                         if (ret || reg > LM3533_HVLED_ID_MAX) {
>                                 dev_err(dev, "invalid backlight reg %d\n", reg);
>                                 continue;
>                         }
> 
>                         lm3533_cells[count].name = "lm3533-backlight";
>                         lm3533_cells[count].id = reg;
>                         lm3533_cells[count].of_compatible =
> "ti,lm3533-backlight";
> 
>                         lm3533->have_backlights = true;
>                 }
> 
>                 if (fwnode_device_is_compatible(child, "ti,lm3533-leds")) {
>                         ret = fwnode_property_read_u32(child, "reg", &reg);
>                         if (ret || reg < LM3533_HVLED_ID_MAX ||
>                             reg > LM3533_LVLED_ID_MAX) {
>                                 dev_err(dev, "invalid LED reg %d\n", reg);
>                                 continue;
>                         }
> 
>                         lm3533_cells[count].name = "lm3533-leds";
>                         lm3533_cells[count].id = reg - LM3533_HVLED_ID_MAX;
>                         lm3533_cells[count].of_compatible = "ti,lm3533-leds";
> 
>                         lm3533->have_leds = true;
>                 }
> 
>                 count++;
>         }
> 
> >         if (ret) {
> > -               dev_err(lm3533->dev, "failed to add LED devices\n");
> > -               return ret;
> > -       }
> > -
> > -       lm3533->have_leds = 1;
> > -
> > -       return 0;
> > -}

[...]

-- 
Lee Jones