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

On 3 May 2017 at 07:08, Álvaro Fernández Rojas noltari@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@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@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