[U-Boot] [PATCH 1/5] dm: led: add BCM6328 led driver

[Simon Glass]

On 3 May 2017 at 07:08, Álvaro Fernández Rojas <noltari at gmail.com> wrote:
> This driver is a simplified version of linux/drivers/leds/leds-bcm6328.c,
> simplified to remove HW leds and blink fallbacks.
>
> Signed-off-by: Álvaro Fernández Rojas <noltari at gmail.com>
> ---
>  drivers/led/Kconfig       |   7 ++
>  drivers/led/Makefile      |   1 +
>  drivers/led/led_bcm6328.c | 262 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 270 insertions(+)
>  create mode 100644 drivers/led/led_bcm6328.c
>

Reviewed-by: Simon Glass <sjg at chromium.org>

But please see below.

Can you a DT binding file for this?

> diff --git a/drivers/led/Kconfig b/drivers/led/Kconfig
> index 309372a..50b27bd 100644
> --- a/drivers/led/Kconfig
> +++ b/drivers/led/Kconfig
> @@ -9,6 +9,13 @@ config LED
>           can provide access to board-specific LEDs. Use of the device tree
>           for configuration is encouraged.
>
> +config LED_BCM6328
> +       bool "LED Support for BCM6328"
> +       depends on LED && ARCH_BMIPS
> +       help
> +         This option enables support for LEDs connected to the BCM6328
> +         LED HW controller accessed via MMIO registers.

What features does it support? Flashing? How many LEDs?

[...]

> +static int bcm6328_led_probe(struct udevice *dev)
> +{
> +       struct led_uc_plat *uc_plat = dev_get_uclass_platdata(dev);
> +       fdt_addr_t addr;
> +       fdt_size_t size;
> +
> +       /* Top-level LED node */
> +       if (!uc_plat->label) {
> +               void __iomem *regs;
> +               u32 set_bits = 0;
> +
> +               addr = dev_get_addr_size_index(dev, 0, &size);
> +               if (addr == FDT_ADDR_T_NONE)
> +                       return -EINVAL;
> +
> +               regs = ioremap(addr, size);
> +
> +               if (fdt_getprop(gd->fdt_blob, dev_of_offset(dev),
> +                               "brcm,serial-leds", NULL))

Can you use fdtdec_get_bool() ?

> +                       set_bits |= LED_INIT_SLEDEN_MASK;
> +               if (fdt_getprop(gd->fdt_blob, dev_of_offset(dev),
> +                               "brcm,serial-mux", NULL))
> +                       set_bits |= LED_INIT_SLEDMUX_MASK;
> +               if (fdt_getprop(gd->fdt_blob, dev_of_offset(dev),
> +                               "brcm,serial-clk-low", NULL))
> +                       set_bits |= LED_INIT_SLEDCLKNPOL_MASK;
> +               if (!fdt_getprop(gd->fdt_blob, dev_of_offset(dev),
> +                                "brcm,serial-dat-low", NULL))
> +                       set_bits |= LED_INIT_SLEDDATANPOL_MASK;
> +               if (!fdt_getprop(gd->fdt_blob, dev_of_offset(dev),
> +                                "brcm,serial-shift-inv", NULL))
> +                       set_bits |= LED_INIT_SLEDSHIFTDIR_MASK;
> +
> +               clrsetbits_be32(regs + LED_INIT_REG, ~0, set_bits);
> +       } else {
> +               struct bcm6328_led_priv *priv = dev_get_priv(dev);
> +               unsigned int pin;
> +
> +               addr = dev_get_addr_size_index(dev_get_parent(dev), 0, &size);
> +               if (addr == FDT_ADDR_T_NONE)
> +                       return -EINVAL;
> +
> +               pin = fdtdec_get_uint(gd->fdt_blob, dev_of_offset(dev), "reg",
> +                                     LEDS_MAX);
> +               if (pin >= LEDS_MAX)
> +                       return -EINVAL;
> +
> +               priv->regs = ioremap(addr, size);
> +               if (pin < 8) {
> +                       /* LEDs 0-7 (bits 47:32) */
> +                       priv->mode = priv->regs + LED_MODE_REG_HI;
> +                       priv->shift = (pin << 1);
> +               } else {
> +                       /* LEDs 8-23 (bits 31:0) */
> +                       priv->mode = priv->regs + LED_MODE_REG_LO;
> +                       priv->shift = ((pin - 8) << 1);
> +               }
> +
> +               if (fdt_getprop(gd->fdt_blob, dev_of_offset(dev), "active-low",
> +                               NULL))
> +                       priv->active_low = true;
> +       }
> +
> +       return 0;
> +}
> +

[..]

Regards,
Simon