Re: [PATCH] drivers: led: bcm6858: do not use null label to find the top

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 7024 bytes --]

On Tue, Jul 18, 2023 at 11:20:52AM +0200, Philippe REYNES wrote:
> Hi Tom,
> 
> For this patch, I see that it is accepted on patchwork:
> https://patchwork.ozlabs.org/project/uboot/patch/20230623163642.241034-1-philippe.reynes@softathome.com/
> But I don't see it in master/next.
> 
> Is it a "miss" or I have to update it please ?

This commit:
https://source.denx.de/u-boot/u-boot/-/commit/910b01c27c0499ae8f84104a0db745dd3a056c04

is that patch, yes?

> 
> Regards,
> Philippe
> 
> 
> 
> Le 23/06/2023 à 18:36, Philippe Reynes a écrit :
> > This driver considers that a node with an empty label is the top.
> > But the led class has changed, if a label is not provided for a led,
> > the label is filed with the node name. So we update this driver
> > to use a wrapper to manage the top led node.
> > 
> > Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
> > ---
> >   drivers/led/led_bcm6858.c | 122 ++++++++++++++++++++------------------
> >   1 file changed, 64 insertions(+), 58 deletions(-)
> > 
> > diff --git a/drivers/led/led_bcm6858.c b/drivers/led/led_bcm6858.c
> > index 6b3698674b..397dc0d869 100644
> > --- a/drivers/led/led_bcm6858.c
> > +++ b/drivers/led/led_bcm6858.c
> > @@ -180,63 +180,71 @@ static const struct led_ops bcm6858_led_ops = {
> > 
> >   static int bcm6858_led_probe(struct udevice *dev)
> >   {
> > -     struct led_uc_plat *uc_plat = dev_get_uclass_plat(dev);
> > -
> > -     /* Top-level LED node */
> > -     if (!uc_plat->label) {
> > -             void __iomem *regs;
> > -             u32 set_bits = 0;
> > -
> > -             regs = dev_remap_addr(dev);
> > -             if (!regs)
> > -                     return -EINVAL;
> > -
> > -             if (dev_read_bool(dev, "brcm,serial-led-msb-first"))
> > -                     set_bits |= LED_CTRL_SERIAL_LED_MSB_FIRST;
> > -             if (dev_read_bool(dev, "brcm,serial-led-en-pol"))
> > -                     set_bits |= LED_CTRL_SERIAL_LED_EN_POL;
> > -             if (dev_read_bool(dev, "brcm,serial-led-clk-pol"))
> > -                     set_bits |= LED_CTRL_SERIAL_LED_CLK_POL;
> > -             if (dev_read_bool(dev, "brcm,serial-led-data-ppol"))
> > -                     set_bits |= LED_CTRL_SERIAL_LED_DATA_PPOL;
> > -             if (dev_read_bool(dev, "brcm,led-test-mode"))
> > -                     set_bits |= LED_CTRL_LED_TEST_MODE;
> > -
> > -             clrsetbits_32(regs + LED_CTRL_REG, ~0, set_bits);
> > -     } else {
> > -             struct bcm6858_led_priv *priv = dev_get_priv(dev);
> > -             void __iomem *regs;
> > -             unsigned int pin, brightness;
> > -
> > -             regs = dev_remap_addr(dev_get_parent(dev));
> > -             if (!regs)
> > -                     return -EINVAL;
> > -
> > -             pin = dev_read_u32_default(dev, "reg", LEDS_MAX);
> > -             if (pin >= LEDS_MAX)
> > -                     return -EINVAL;
> > -
> > -             priv->regs = regs;
> > -             priv->pin = pin;
> > -
> > -             /* this led is managed by software */
> > -             clrbits_32(regs + LED_HW_LED_EN_REG, 1 << pin);
> > -
> > -             /* configure the polarity */
> > -             if (dev_read_bool(dev, "active-low"))
> > -                     clrbits_32(regs + LED_PLED_OP_PPOL_REG, 1 << pin);
> > -             else
> > -                     setbits_32(regs + LED_PLED_OP_PPOL_REG, 1 << pin);
> > +     struct bcm6858_led_priv *priv = dev_get_priv(dev);
> > +     void __iomem *regs;
> > +     unsigned int pin, brightness;
> > +
> > +     regs = dev_remap_addr(dev_get_parent(dev));
> > +     if (!regs)
> > +             return -EINVAL;
> > +
> > +     pin = dev_read_u32_default(dev, "reg", LEDS_MAX);
> > +     if (pin >= LEDS_MAX)
> > +             return -EINVAL;
> > +
> > +     priv->regs = regs;
> > +     priv->pin = pin;
> > +
> > +     /* this led is managed by software */
> > +     clrbits_32(regs + LED_HW_LED_EN_REG, 1 << pin);
> > 
> > -             brightness = dev_read_u32_default(dev, "default-brightness",
> > +     /* configure the polarity */
> > +     if (dev_read_bool(dev, "active-low"))
> > +             clrbits_32(regs + LED_PLED_OP_PPOL_REG, 1 << pin);
> > +     else
> > +             setbits_32(regs + LED_PLED_OP_PPOL_REG, 1 << pin);
> > +
> > +     brightness = dev_read_u32_default(dev, "default-brightness",
> >                                                 LEDS_MAX_BRIGHTNESS);
> > -             led_set_brightness(dev, brightness);
> > -     }
> > +     led_set_brightness(dev, brightness);
> > 
> >       return 0;
> >   }
> > 
> > -static int bcm6858_led_bind(struct udevice *parent)
> > +U_BOOT_DRIVER(bcm6858_led) = {
> > +     .name = "bcm6858-led",
> > +     .id = UCLASS_LED,
> > +     .probe = bcm6858_led_probe,
> > +     .priv_auto      = sizeof(struct bcm6858_led_priv),
> > +     .ops = &bcm6858_led_ops,
> > +};
> > +
> > +static int bcm6858_led_wrap_probe(struct udevice *dev)
> > +{
> > +     void __iomem *regs;
> > +     u32 set_bits = 0;
> > +
> > +     regs = dev_remap_addr(dev);
> > +     if (!regs)
> > +             return -EINVAL;
> > +
> > +     if (dev_read_bool(dev, "brcm,serial-led-msb-first"))
> > +             set_bits |= LED_CTRL_SERIAL_LED_MSB_FIRST;
> > +     if (dev_read_bool(dev, "brcm,serial-led-en-pol"))
> > +             set_bits |= LED_CTRL_SERIAL_LED_EN_POL;
> > +     if (dev_read_bool(dev, "brcm,serial-led-clk-pol"))
> > +             set_bits |= LED_CTRL_SERIAL_LED_CLK_POL;
> > +     if (dev_read_bool(dev, "brcm,serial-led-data-ppol"))
> > +             set_bits |= LED_CTRL_SERIAL_LED_DATA_PPOL;
> > +     if (dev_read_bool(dev, "brcm,led-test-mode"))
> > +             set_bits |= LED_CTRL_LED_TEST_MODE;
> > +
> > +     clrsetbits_32(regs + LED_CTRL_REG, ~0, set_bits);
> > +
> > +     return 0;
> > +}
> > +
> > +static int bcm6858_led_wrap_bind(struct udevice *parent)
> >   {
> >       ofnode node;
> > 
> > @@ -259,12 +267,10 @@ static const struct udevice_id bcm6858_led_ids[] = {
> >       { /* sentinel */ }
> >   };
> > 
> > -U_BOOT_DRIVER(bcm6858_led) = {
> > -     .name = "bcm6858-led",
> > -     .id = UCLASS_LED,
> > +U_BOOT_DRIVER(bcm6858_led_wrap) = {
> > +     .name   = "bcm6858_led_wrap",
> > +     .id     = UCLASS_NOP,
> >       .of_match = bcm6858_led_ids,
> > -     .bind = bcm6858_led_bind,
> > -     .probe = bcm6858_led_probe,
> > -     .priv_auto      = sizeof(struct bcm6858_led_priv),
> > -     .ops = &bcm6858_led_ops,
> > +     .probe = bcm6858_led_wrap_probe,
> > +     .bind = bcm6858_led_wrap_bind,
> >   };
> -- This message and any attachments herein are confidential, intended solely for the addressees and are SoftAtHome’s ownership. Any unauthorized use or dissemination is prohibited. If you are not the intended addressee of this message, please cancel it immediately and inform the sender.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]