All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] BCM6328 LED driver
@ 2015-04-02 11:54 Álvaro Fernández Rojas
  2015-04-02 11:54 ` [PATCH 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
                   ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Álvaro Fernández Rojas @ 2015-04-02 11:54 UTC (permalink / raw)
  To: linux-leds, cooloney, jogo, f.fainelli, cernekee
  Cc: Álvaro Fernández Rojas

These patches add support and documentation for the BCM6328 LED driver, which is present on BCM6318, BCM6328, BCM6362 and BCM63268.
In these SoCs it's possible to control LEDs both as GPIOs or by hardware.
However, on some devices there are Serial LEDs (LEDs connected to a 74hc controller), which can either be controlled by software (exporting the 74hc as spi-gpio) or hardware using this driver.
The problem is some of these serial LEDs are hardware controlled (ethernet LEDs) and exporting the 74hc as spi-gpio prevents those LEDs to be hardware controlled, so the only chance to keep them working is by using this driver. 

Álvaro Fernández Rojas (2):
  leds: add DT binding for BCM6328 LED controller
  leds: add BCM6328 LED driver

 .../devicetree/bindings/leds/leds-bcm6328.txt      | 173 +++++++++
 drivers/leds/Kconfig                               |   8 +
 drivers/leds/Makefile                              |   1 +
 drivers/leds/leds-bcm6328.c                        | 411 +++++++++++++++++++++
 4 files changed, 593 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-bcm6328.txt
 create mode 100644 drivers/leds/leds-bcm6328.c

-- 
1.9.1

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/2] leds: add DT binding for BCM6328 LED controller
  2015-04-02 11:54 [PATCH 0/2] BCM6328 LED driver Álvaro Fernández Rojas
@ 2015-04-02 11:54 ` Álvaro Fernández Rojas
  2015-04-03  2:06   ` Florian Fainelli
  2015-04-02 11:54 ` [PATCH 2/2] leds: add BCM6328 LED driver Álvaro Fernández Rojas
  2015-04-05 15:08 ` [PATCH v2 0/2] " Álvaro Fernández Rojas
  2 siblings, 1 reply; 26+ messages in thread
From: Álvaro Fernández Rojas @ 2015-04-02 11:54 UTC (permalink / raw)
  To: linux-leds, cooloney, jogo, f.fainelli, cernekee
  Cc: Álvaro Fernández Rojas

This adds device tree binding documentation for the Broadcom BCM6328 LED
controller.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Signed-off-by: Jonas Gorski <jogo@openwrt.org>
---
 .../devicetree/bindings/leds/leds-bcm6328.txt      | 173 +++++++++++++++++++++
 1 file changed, 173 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-bcm6328.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-bcm6328.txt b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
new file mode 100644
index 0000000..e63d27f
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
@@ -0,0 +1,173 @@
+LEDs connected to Broadcom BCM6328 controller
+
+Required properties:
+- compatible : should be : "brcm,bcm6328-leds".
+- #address-cells: must be 1
+- #size-cells: must be 0
+- reg: BCM6328 LED controller address and size.
+
+Optional properties:
+- brcm,serial-leds: enable Serial LEDs.
+
+Each led is represented as a sub-node of the brcm,bcm6328-leds device.
+
+LED sub-node properties:
+- reg : LED pin number (could be from 0 to 23).
+- compatible : should be : "brcm,bcm6328-led".
+
+Normal LED:
+- label (optional) : see Documentation/devicetree/bindings/leds/common.txt
+- active-low (optional) : LED is active low.
+- default-state (optional): see
+  Documentation/devicetree/bindings/leds/leds-gpio.txt
+- linux,default-trigger (optional): see
+  Documentation/devicetree/bindings/leds/common.txt
+
+Hardware controlled LED:
+- brcm,hardware-controlled (optional) : LED is hardware controlled.
+- brcm,link-selection (optional) : LED link selection values.
+- brcm,activity-selection (optional) : LED activity selection values.
+
+example 1) BCM6328
+
+leds0: led-controller@10000800 {
+	compatible = "brcm,bcm6328-leds";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reg = <0x10000800 0x24>;
+
+	alarm_red@2 {
+		compatible = "brcm,bcm6328-led";
+		reg = <2>;
+		active-low;
+		label = "red:alarm";
+	};
+	inet_green@3 {
+		compatible = "brcm,bcm6328-led";
+		reg = <3>;
+		active-low;
+		label = "green:inet";
+	};
+	power_green@4 {
+		compatible = "brcm,bcm6328-led";
+		reg = <4>;
+		active-low;
+		label = "green:power";
+		default-state = "on";
+	};
+	ephy0_spd@17 {
+		compatible = "brcm,bcm6328-led";
+		reg = <17>;
+		brcm,hardware-controlled;
+	};
+	ephy1_spd@18 {
+		compatible = "brcm,bcm6328-led";
+		reg = <18>;
+		brcm,hardware-controlled;
+	};
+	ephy2_spd@19 {
+		compatible = "brcm,bcm6328-led";
+		reg = <19>;
+		brcm,hardware-controlled;
+	};
+	ephy3_spd@20 {
+		compatible = "brcm,bcm6328-led";
+		reg = <20>;
+		brcm,hardware-controlled;
+	};
+};
+
+example 2) BCM63268
+
+leds0: led-controller@10001900 {
+	compatible = "brcm,bcm6328-leds";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reg = <0x10001900 0x24>;
+	brcm,serial-leds;
+
+	gphy0_spd0@0 {
+		compatible = "brcm,bcm6328-led";
+		reg = <0>;
+		brcm,hardware-controlled;
+		brcm,link-selection = <0>;
+	};
+	gphy0_spd1@1 {
+		compatible = "brcm,bcm6328-led";
+		reg = <1>;
+		brcm,hardware-controlled;
+		brcm,link-selection = <1>;
+	};
+	inet_red@2 {
+		compatible = "brcm,bcm6328-led";
+		reg = <2>;
+		active-low;
+		label = "red:inet";
+	};
+	dsl_green@3 {
+		compatible = "brcm,bcm6328-led";
+		reg = <3>;
+		active-low;
+		label = "green:dsl";
+	};
+	usb_green@4 {
+		compatible = "brcm,bcm6328-led";
+		reg = <4>;
+		active-low;
+		label = "green:usb";
+	};
+	wps_green@7 {
+		compatible = "brcm,bcm6328-led";
+		reg = <7>;
+		active-low;
+		label = "green:wps";
+	};
+	inet_green@8 {
+		compatible = "brcm,bcm6328-led";
+		reg = <8>;
+		active-low;
+		label = "green:inet";
+	};
+	ephy0_act@9 {
+		compatible = "brcm,bcm6328-led";
+		reg = <9>;
+		brcm,hardware-controlled;
+	};
+	ephy1_act@10 {
+		compatible = "brcm,bcm6328-led";
+		reg = <10>;
+		brcm,hardware-controlled;
+	};
+	ephy2_act@11 {
+		compatible = "brcm,bcm6328-led";
+		reg = <11>;
+		brcm,hardware-controlled;
+	};
+	gphy0_act@12 {
+		compatible = "brcm,bcm6328-led";
+		reg = <12>;
+		brcm,hardware-controlled;
+	};
+	ephy0_spd@13 {
+		compatible = "brcm,bcm6328-led";
+		reg = <13>;
+		brcm,hardware-controlled;
+	};
+	ephy1_spd@14 {
+		compatible = "brcm,bcm6328-led";
+		reg = <14>;
+		brcm,hardware-controlled;
+	};
+	ephy2_spd@15 {
+		compatible = "brcm,bcm6328-led";
+		reg = <15>;
+		brcm,hardware-controlled;
+	};
+	power_green@20 {
+		compatible = "brcm,bcm6328-led";
+		reg = <20>;
+		active-low;
+		label = "green:power";
+		default-state = "on";
+	};
+};
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 2/2] leds: add BCM6328 LED driver
  2015-04-02 11:54 [PATCH 0/2] BCM6328 LED driver Álvaro Fernández Rojas
  2015-04-02 11:54 ` [PATCH 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
@ 2015-04-02 11:54 ` Álvaro Fernández Rojas
  2015-04-05 15:08 ` [PATCH v2 0/2] " Álvaro Fernández Rojas
  2 siblings, 0 replies; 26+ messages in thread
From: Álvaro Fernández Rojas @ 2015-04-02 11:54 UTC (permalink / raw)
  To: linux-leds, cooloney, jogo, f.fainelli, cernekee
  Cc: Álvaro Fernández Rojas

This adds support for the LED controller on Broadcom's BCM6328.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Signed-off-by: Jonas Gorski <jogo@openwrt.org>
---
 drivers/leds/Kconfig        |   8 +
 drivers/leds/Makefile       |   1 +
 drivers/leds/leds-bcm6328.c | 411 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 420 insertions(+)
 create mode 100644 drivers/leds/leds-bcm6328.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 25b320d..1cdaf58 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -42,6 +42,14 @@ config LEDS_88PM860X
 	  This option enables support for on-chip LED drivers found on Marvell
 	  Semiconductor 88PM8606 PMIC.
 
+config LEDS_BCM6328
+	tristate "Serial LEDs support for BCM6328 chip"
+	depends on LEDS_CLASS
+	depends on OF
+	help
+	  This option enables support for LEDs connected to the BCM6328
+	  LED HW controller accessed via MMIO registers.
+
 config LEDS_LM3530
 	tristate "LCD Backlight driver for LM3530"
 	depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index cbba921..bf69f49 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_LEDS_TRIGGERS)		+= led-triggers.o
 
 # LED Platform Drivers
 obj-$(CONFIG_LEDS_88PM860X)		+= leds-88pm860x.o
+obj-$(CONFIG_LEDS_BCM6328)		+= leds-bcm6328.o
 obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
 obj-$(CONFIG_LEDS_LOCOMO)		+= leds-locomo.o
 obj-$(CONFIG_LEDS_LM3530)		+= leds-lm3530.o
diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
new file mode 100644
index 0000000..29acf7c
--- /dev/null
+++ b/drivers/leds/leds-bcm6328.c
@@ -0,0 +1,411 @@
+/*
+ * Driver for BCM6328 memory-mapped LEDs, based on leds-syscon.c
+ *
+ * Copyright 2015 Álvaro Fernández Rojas <noltari@gmail.com>
+ * Copyright 2015 Jonas Gorski <jogo@openwrt.org>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+#include <linux/io.h>
+#include <linux/leds.h>
+#include <linux/of_device.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/spinlock.h>
+#include <linux/slab.h>
+#include <linux/stat.h>
+
+#define REG_INIT		0x00
+#define REG_MODE_HI		0x04
+#define REG_MODE_LO		0x08
+#define REG_HWDIS		0x0c
+#define REG_STROBE		0x10
+#define REG_LNKACTSEL_HI	0x14
+#define REG_LNKACTSEL_LO	0x18
+#define REG_RBACK		0x1c
+#define REG_SERMUX		0x20
+
+#define LED_MAX_COUNT		24
+#define LED_DEF_DELAY		500
+#define LED_INTERVAL_MS		20
+
+#define LED_INTV_MASK		0x3f
+#define LED_FAST_INTV_SHIFT	6
+#define LED_FAST_INTV_MASK	(LED_INTV_MASK << LED_FAST_INTV_SHIFT)
+#define SERIAL_LED_EN		BIT(12)
+#define SERIAL_LED_MUX		BIT(13)
+#define SERIAL_LED_CLK_NPOL	BIT(14)
+#define SERIAL_LED_DATA_PPOL	BIT(15)
+#define SERIAL_LED_SHIFT_DIR	BIT(16)
+#define LED_SHIFT_TEST		BIT(30)
+#define LED_TEST		BIT(31)
+
+#define LED_MODE_MASK		3
+#define LED_MODE_OFF		0
+#define LED_MODE_FAST		1
+#define LED_MODE_BLINK		2
+#define LED_MODE_ON		3
+#define LED_SHIFT(X)		((X) << 1)
+
+/**
+ * struct bcm6328_led - state container for bcm6328 based LEDs
+ * @cdev: LED class device for this LED
+ * @mem: memory resource
+ * @lock: memory lock
+ * @pin: LED pin number
+ * @blink_leds: blinking LEDs
+ * @blink_del: blinking delay
+ * @active_low: LED is active low
+ */
+struct bcm6328_led {
+	struct led_classdev cdev;
+	void __iomem *mem;
+	spinlock_t *lock;
+	unsigned long pin;
+	unsigned long *blink_leds;
+	unsigned long *blink_del;
+	bool active_low;
+};
+
+static void bcm6328_led_write(void __iomem *reg, unsigned long data)
+{
+	iowrite32be(data, reg);
+}
+
+static unsigned long bcm6328_led_read(void __iomem *reg)
+{
+	return ioread32be(reg);
+}
+
+/**
+ * LEDMode 64 bits / 24 LEDs
+ * bits [31:0] -> LEDs 8-23
+ * bits [47:32] -> LEDs 0-7
+ * bits [63:48] -> unused
+ */
+static unsigned long bcm6328_pin2shift(unsigned long pin)
+{
+	if (pin < 8)
+		return pin + 16; /* LEDs 0-7 (bits 47:32) */
+	else
+		return pin - 8; /* LEDs 8-23 (bits 31:0) */
+}
+
+static void bcm6328_led_mode(struct bcm6328_led *led, unsigned long value)
+{
+	void __iomem *mode;
+	unsigned long val, shift;
+
+	shift = bcm6328_pin2shift(led->pin);
+	if (shift / 16)
+		mode = led->mem + REG_MODE_HI;
+	else
+		mode = led->mem + REG_MODE_LO;
+
+	val = bcm6328_led_read(mode);
+	val &= ~(LED_MODE_MASK << LED_SHIFT(shift % 16));
+	val |= (value << LED_SHIFT(shift % 16));
+	bcm6328_led_write(mode, val);
+}
+
+static void bcm6328_led_set(struct led_classdev *led_cdev,
+	enum led_brightness value)
+{
+	struct bcm6328_led *led =
+		container_of(led_cdev, struct bcm6328_led, cdev);
+	unsigned long flags;
+
+	spin_lock_irqsave(led->lock, flags);
+	*(led->blink_leds) &= ~BIT(led->pin);
+	if ((led->active_low && value == LED_OFF) ||
+	    (!led->active_low && value != LED_OFF))
+		bcm6328_led_mode(led, LED_MODE_OFF);
+	else
+		bcm6328_led_mode(led, LED_MODE_ON);
+	spin_unlock_irqrestore(led->lock, flags);
+}
+
+static int bcm6328_blink_set(struct led_classdev *led_cdev,
+			     unsigned long *delay_on, unsigned long *delay_off)
+{
+	struct bcm6328_led *led =
+		container_of(led_cdev, struct bcm6328_led, cdev);
+	unsigned long delay, flags;
+
+	if (!*delay_on)
+		*delay_on = LED_DEF_DELAY;
+	if (!*delay_off)
+		*delay_off = LED_DEF_DELAY;
+
+	if (*delay_on != *delay_off) {
+		dev_dbg(led_cdev->dev,
+			"fallback to soft blinking (delay_on != delay_off)\n");
+		return -EINVAL;
+	}
+
+	delay = *delay_on / LED_INTERVAL_MS;
+	if (delay == 0)
+		delay = 1;
+	else if (delay > LED_INTV_MASK) {
+		dev_dbg(led_cdev->dev,
+			"fallback to soft blinking (delay > %ums)\n",
+			LED_INTV_MASK * LED_INTERVAL_MS);
+		return -EINVAL;
+	}
+
+	spin_lock_irqsave(led->lock, flags);
+	if (*(led->blink_leds) == 0 ||
+	    *(led->blink_leds) == BIT(led->pin) ||
+	    *(led->blink_del) == delay) {
+		unsigned long val;
+
+		*(led->blink_leds) = BIT(led->pin);
+		*(led->blink_del) = delay;
+
+		val = bcm6328_led_read(led->mem + REG_INIT);
+		val &= ~LED_FAST_INTV_MASK;
+		val |= (delay << LED_FAST_INTV_SHIFT);
+		bcm6328_led_write(led->mem + REG_INIT, val);
+
+		bcm6328_led_mode(led, LED_MODE_BLINK);
+
+		spin_unlock_irqrestore(led->lock, flags);
+	} else {
+		spin_unlock_irqrestore(led->lock, flags);
+		dev_dbg(led_cdev->dev,
+			"fallback to soft blinking (delay already set)\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int bcm6328_hwled(struct device *dev, struct device_node *nc, u32 reg,
+			 void __iomem *mem, spinlock_t *lock)
+{
+	int i, cnt;
+	unsigned long flags, val;
+
+	spin_lock_irqsave(lock, flags);
+	val = bcm6328_led_read(mem + REG_HWDIS);
+	val &= ~BIT(reg);
+	bcm6328_led_write(mem + REG_HWDIS, val);
+	spin_unlock_irqrestore(lock, flags);
+
+	/* Only LEDs 0-7 can be activity/link controlled */
+	if (reg >= 8)
+		return 0;
+
+	cnt = of_property_count_elems_of_size(nc, "brcm,link-selection",
+					      sizeof(u32));
+	for (i = 0; i < cnt; i++) {
+		u32 sel;
+		void __iomem *addr;
+
+		if (reg < 4)
+			addr = mem + REG_LNKACTSEL_LO;
+		else
+			addr = mem + REG_LNKACTSEL_HI;
+
+		of_property_read_u32_index(nc, "brcm,link-selection", i, &sel);
+
+		if (reg / 4 != sel / 4) {
+			dev_warn(dev, "invalid link selection\n");
+			continue;
+		}
+
+		spin_lock_irqsave(lock, flags);
+		val = bcm6328_led_read(addr);
+		val |= (BIT(reg) << (((sel % 4) * 4) + 16));
+		bcm6328_led_write(addr, val);
+		spin_unlock_irqrestore(lock, flags);
+	}
+
+	cnt = of_property_count_elems_of_size(nc, "brcm,activity-selection",
+					      sizeof(u32));
+	for (i = 0; i < cnt; i++) {
+		u32 sel;
+		void __iomem *addr;
+
+		if (reg < 4)
+			addr = mem + REG_LNKACTSEL_LO;
+		else
+			addr = mem + REG_LNKACTSEL_HI;
+
+		of_property_read_u32_index(nc, "brcm,activity-selection", i,
+					   &sel);
+
+		if (reg / 4 != sel / 4) {
+			dev_warn(dev, "invalid activity selection\n");
+			continue;
+		}
+
+		spin_lock_irqsave(lock, flags);
+		val = bcm6328_led_read(addr);
+		val |= (BIT(reg) << ((sel % 4) * 4));
+		bcm6328_led_write(addr, val);
+		spin_unlock_irqrestore(lock, flags);
+	}
+
+	return 0;
+}
+
+static int bcm6328_led(struct device *dev, struct device_node *nc, u32 reg,
+		       void __iomem *mem, spinlock_t *lock,
+		       unsigned long *blink_leds, unsigned long *blink_del)
+{
+	struct bcm6328_led *led;
+	unsigned long flags;
+	const char *state;
+	int ret;
+
+	led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+
+	led->pin = reg;
+	led->mem = mem;
+	led->lock = lock;
+	led->blink_leds = blink_leds;
+	led->blink_del = blink_del;
+
+	if (of_get_property(nc, "active-low", NULL))
+		led->active_low = 1;
+
+	led->cdev.name = of_get_property(nc, "label", NULL) ? : nc->name;
+	led->cdev.default_trigger = of_get_property(nc,
+						    "linux,default-trigger",
+						    NULL);
+
+	if (!of_property_read_string(nc, "default-state", &state)) {
+		spin_lock_irqsave(lock, flags);
+		if (!strcmp(state, "on")) {
+			led->cdev.brightness = LED_FULL;
+			bcm6328_led_mode(led, LED_MODE_ON);
+		} else if (!strcmp(state, "keep")) {
+			void __iomem *mode;
+			unsigned long val, shift;
+
+			shift = bcm6328_pin2shift(led->pin);
+			if (shift / 16)
+				mode = mem + REG_MODE_HI;
+			else
+				mode = mem + REG_MODE_LO;
+
+			val = bcm6328_led_read(mode) >> (shift % 16);
+			val &= LED_MODE_MASK;
+			if (val == LED_MODE_ON)
+				led->cdev.brightness = LED_FULL;
+			else {
+				led->cdev.brightness = LED_OFF;
+				bcm6328_led_mode(led, LED_MODE_OFF);
+			}
+		} else {
+			led->cdev.brightness = LED_OFF;
+			bcm6328_led_mode(led, LED_MODE_OFF);
+		}
+		spin_unlock_irqrestore(lock, flags);
+	}
+
+	led->cdev.brightness_set = bcm6328_led_set;
+	led->cdev.blink_set = bcm6328_blink_set;
+
+	ret = led_classdev_register(dev, &led->cdev);
+	if (ret < 0)
+		return ret;
+
+	dev_info(dev, "registered LED %s\n", led->cdev.name);
+
+	return 0;
+}
+
+static int bcm6328_leds_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *child;
+	struct resource *mem_r;
+	void __iomem *mem;
+	spinlock_t *lock;
+	unsigned long val, *blink_leds, *blink_del;
+
+	mem_r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mem_r)
+		return -EINVAL;
+
+	mem = devm_ioremap_resource(dev, mem_r);
+	if (IS_ERR(mem))
+		return PTR_ERR(mem);
+
+	lock = devm_kzalloc(dev, sizeof(*lock), GFP_KERNEL);
+	if (!lock)
+		return -ENOMEM;
+
+	blink_leds = devm_kzalloc(dev, sizeof(*blink_leds), GFP_KERNEL);
+	if (!blink_leds)
+		return -ENOMEM;
+
+	blink_del = devm_kzalloc(dev, sizeof(*blink_del), GFP_KERNEL);
+	if (!blink_del)
+		return -ENOMEM;
+
+	spin_lock_init(lock);
+
+	bcm6328_led_write(mem + REG_HWDIS, ~0);
+	bcm6328_led_write(mem + REG_LNKACTSEL_HI, 0);
+	bcm6328_led_write(mem + REG_LNKACTSEL_LO, 0);
+
+	val = bcm6328_led_read(mem + REG_INIT);
+	val &= ~SERIAL_LED_EN;
+	if (of_get_property(np, "brcm,serial-leds", NULL))
+		val |= SERIAL_LED_EN;
+	bcm6328_led_write(mem + REG_INIT, val);
+
+	for_each_available_child_of_node(np, child) {
+		int ret;
+		u32 reg;
+
+		if (!of_device_is_compatible(child, "brcm,bcm6328-led") ||
+		    of_property_read_u32(child, "reg", &reg))
+			continue;
+
+		if (reg >= LED_MAX_COUNT) {
+			dev_err(dev, "invalid LED (>= %d)\n", LED_MAX_COUNT);
+			continue;
+		}
+
+		if (of_get_property(child, "brcm,hardware-controlled", NULL))
+			ret = bcm6328_hwled(dev, child, reg, mem, lock);
+		else
+			ret = bcm6328_led(dev, child, reg, mem, lock,
+					  blink_leds, blink_del);
+
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id bcm6328_leds_of_match[] = {
+	{ .compatible = "brcm,bcm6328-leds", },
+	{ },
+};
+
+static struct platform_driver bcm6328_leds_driver = {
+	.probe = bcm6328_leds_probe,
+	.driver = {
+		.name = "bcm6328-leds",
+		.of_match_table = bcm6328_leds_of_match,
+	},
+};
+
+module_platform_driver(bcm6328_leds_driver);
+
+MODULE_DESCRIPTION("bcm6328 led driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:bcm6328-leds");
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/2] leds: add DT binding for BCM6328 LED controller
  2015-04-02 11:54 ` [PATCH 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
@ 2015-04-03  2:06   ` Florian Fainelli
  2015-04-03 12:19     ` Álvaro Fernández Rojas
  0 siblings, 1 reply; 26+ messages in thread
From: Florian Fainelli @ 2015-04-03  2:06 UTC (permalink / raw)
  To: Álvaro Fernández Rojas, linux-leds, cooloney, jogo, cernekee

On 02/04/15 04:54, Álvaro Fernández Rojas wrote:
> This adds device tree binding documentation for the Broadcom BCM6328 LED
> controller.

Looks pretty good to me, few comments below:

> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> Signed-off-by: Jonas Gorski <jogo@openwrt.org>
> ---
>  .../devicetree/bindings/leds/leds-bcm6328.txt      | 173 +++++++++++++++++++++
>  1 file changed, 173 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-bcm6328.txt
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-bcm6328.txt b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
> new file mode 100644
> index 0000000..e63d27f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
> @@ -0,0 +1,173 @@
> +LEDs connected to Broadcom BCM6328 controller
> +
> +Required properties:
> +- compatible : should be : "brcm,bcm6328-leds".

Something like "brcm,bcm6328-leds-ctrl" might be a bit more descriptive.

> +- #address-cells: must be 1
> +- #size-cells: must be 0
> +- reg: BCM6328 LED controller address and size.
> +
> +Optional properties:
> +- brcm,serial-leds: enable Serial LEDs.
> +
> +Each led is represented as a sub-node of the brcm,bcm6328-leds device.
> +
> +LED sub-node properties:
> +- reg : LED pin number (could be from 0 to 23).
> +- compatible : should be : "brcm,bcm6328-led".

Do we really need to strongly type the LED child nodes here with a
compatible string? It is somewhat implicit that if your parent is
"brcm,bcm6328-leds", the child nodes are of the same "type"?

> +
> +Normal LED:
> +- label (optional) : see Documentation/devicetree/bindings/leds/common.txt
> +- active-low (optional) : LED is active low.

If it is an optional property, you would want to clarify that active
high is the default.

> +- default-state (optional): see
> +  Documentation/devicetree/bindings/leds/leds-gpio.txt
> +- linux,default-trigger (optional): see
> +  Documentation/devicetree/bindings/leds/common.txt
> +
> +Hardware controlled LED:
> +- brcm,hardware-controlled (optional) : LED is hardware controlled.

The type of this property is not strictly defined, but it would appear
to be a boolean. Reading through the driver though, it looks like the
number of LEDs "hardware" controlled should be something customizable at
some point.

> +- brcm,link-selection (optional) : LED link selection values.

Here you should specify exactly which values are accepted, if this is a
value that is directly mapping to a hardware register value, we might
want to create a header file for that.

> +- brcm,activity-selection (optional) : LED activity selection values.

Ditto.
-- 
Florian

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/2] leds: add DT binding for BCM6328 LED controller
  2015-04-03  2:06   ` Florian Fainelli
@ 2015-04-03 12:19     ` Álvaro Fernández Rojas
  2015-04-03 14:46       ` Jonas Gorski
  0 siblings, 1 reply; 26+ messages in thread
From: Álvaro Fernández Rojas @ 2015-04-03 12:19 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: linux-leds, cooloney, jogo, cernekee


> El 3/4/2015, a las 4:06, Florian Fainelli <f.fainelli@gmail.com> escribió:
> 
>> On 02/04/15 04:54, Álvaro Fernández Rojas wrote:
>> This adds device tree binding documentation for the Broadcom BCM6328 LED
>> controller.
> 
> Looks pretty good to me, few comments below:
> 
>> 
>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
>> Signed-off-by: Jonas Gorski <jogo@openwrt.org>
>> ---
>> .../devicetree/bindings/leds/leds-bcm6328.txt      | 173 +++++++++++++++++++++
>> 1 file changed, 173 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/leds/leds-bcm6328.txt
>> 
>> diff --git a/Documentation/devicetree/bindings/leds/leds-bcm6328.txt b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
>> new file mode 100644
>> index 0000000..e63d27f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
>> @@ -0,0 +1,173 @@
>> +LEDs connected to Broadcom BCM6328 controller
>> +
>> +Required properties:
>> +- compatible : should be : "brcm,bcm6328-leds".
> 
> Something like "brcm,bcm6328-leds-ctrl" might be a bit more descriptive.

Yeah you're right, bcm6328-leds-ctrl is better.

> 
>> +- #address-cells: must be 1
>> +- #size-cells: must be 0
>> +- reg: BCM6328 LED controller address and size.
>> +
>> +Optional properties:
>> +- brcm,serial-leds: enable Serial LEDs.
>> +
>> +Each led is represented as a sub-node of the brcm,bcm6328-leds device.
>> +
>> +LED sub-node properties:
>> +- reg : LED pin number (could be from 0 to 23).
>> +- compatible : should be : "brcm,bcm6328-led".
> 
> Do we really need to strongly type the LED child nodes here with a
> compatible string? It is somewhat implicit that if your parent is
> "brcm,bcm6328-leds", the child nodes are of the same "type"?

This is just a sanity check inherited from other LED controllers, but I can remove it. In fact I prefer it, because adding compatible strings on each subnode can be a bit annoying, specially if there are a lot of LEDs.

> 
>> +
>> +Normal LED:
>> +- label (optional) : see Documentation/devicetree/bindings/leds/common.txt
>> +- active-low (optional) : LED is active low.
> 
> If it is an optional property, you would want to clarify that active
> high is the default.

Ok, I will.

> 
>> +- default-state (optional): see
>> +  Documentation/devicetree/bindings/leds/leds-gpio.txt
>> +- linux,default-trigger (optional): see
>> +  Documentation/devicetree/bindings/leds/common.txt
>> +
>> +Hardware controlled LED:
>> +- brcm,hardware-controlled (optional) : LED is hardware controlled.
> 
> The type of this property is not strictly defined, but it would appear
> to be a boolean. Reading through the driver though, it looks like the
> number of LEDs "hardware" controlled should be something customizable at
> some point.

Actually the number of hardware controlled LEDs depends on each SoC, but it can also depend on the specific board. There's a 32 bit register (HWDIS) which controls which LEDs are hardware controlled.

> 
>> +- brcm,link-selection (optional) : LED link selection values.
> 
> Here you should specify exactly which values are accepted, if this is a
> value that is directly mapping to a hardware register value, we might
> want to create a header file for that.

The problem of this is that it depends on each SoC but also on each board, since this controls which LED from LEDs 0-7 is controlled by each ethernet activity and speed "event". For example, Jonas managed to set a specific LED to be controlled by every ethernet port on the board, so there may be boards in which Ethernet ports may not be directly assigned to the Ethernet LEDs, but swapped (it's not the most likely situtation, but since there isn't much documentation from Broadcom available it seems sensible to me to offer that configuration posibility).

> 
>> +- brcm,activity-selection (optional) : LED activity selection values.
> 
> Ditto.
> -- 
> Florian

I will wait some time before sending the v2 with the changes suggested by Florian, so everyone can share their opinion (just in case there are other things to modify).

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/2] leds: add DT binding for BCM6328 LED controller
  2015-04-03 12:19     ` Álvaro Fernández Rojas
@ 2015-04-03 14:46       ` Jonas Gorski
  0 siblings, 0 replies; 26+ messages in thread
From: Jonas Gorski @ 2015-04-03 14:46 UTC (permalink / raw)
  To: Álvaro Fernández Rojas, Florian Fainelli
  Cc: linux-leds, cooloney, cernekee

On 03.04.2015 14:19, Álvaro Fernández Rojas wrote:
> 
>> El 3/4/2015, a las 4:06, Florian Fainelli <f.fainelli@gmail.com> escribió:
>>
>>> On 02/04/15 04:54, Álvaro Fernández Rojas wrote:
>>> This adds device tree binding documentation for the Broadcom BCM6328 LED
>>> controller.
>>
>> Looks pretty good to me, few comments below:
>>
>>>
>>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
>>> Signed-off-by: Jonas Gorski <jogo@openwrt.org>
>>> ---
>>> .../devicetree/bindings/leds/leds-bcm6328.txt      | 173 +++++++++++++++++++++
>>> 1 file changed, 173 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/leds/leds-bcm6328.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/leds/leds-bcm6328.txt b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
>>> new file mode 100644
>>> index 0000000..e63d27f
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
>>> @@ -0,0 +1,173 @@
>>> +LEDs connected to Broadcom BCM6328 controller
>>> +
>>> +Required properties:
>>> +- compatible : should be : "brcm,bcm6328-leds".
>>
>> Something like "brcm,bcm6328-leds-ctrl" might be a bit more descriptive.
> 
> Yeah you're right, bcm6328-leds-ctrl is better.
> 
>>
>>> +- #address-cells: must be 1
>>> +- #size-cells: must be 0
>>> +- reg: BCM6328 LED controller address and size.
>>> +
>>> +Optional properties:
>>> +- brcm,serial-leds: enable Serial LEDs.
>>> +
>>> +Each led is represented as a sub-node of the brcm,bcm6328-leds device.
>>> +
>>> +LED sub-node properties:
>>> +- reg : LED pin number (could be from 0 to 23).

I think this should be a bit more strict: "Only leds 0 to 23 are valid".

>>> +- compatible : should be : "brcm,bcm6328-led".
>>
>> Do we really need to strongly type the LED child nodes here with a
>> compatible string? It is somewhat implicit that if your parent is
>> "brcm,bcm6328-leds", the child nodes are of the same "type"?
> 
> This is just a sanity check inherited from other LED controllers, but I can remove it. In fact I prefer it, because adding compatible strings on each subnode can be a bit annoying, specially if there are a lot of LEDs.

Well, you might have/want other subnodes like pinctrl configurations.
But since we have a reg-property, the existence of it is probably enough
to tell it apart from non-led-configuration nodes.

> 
>>
>>> +
>>> +Normal LED:
>>> +- label (optional) : see Documentation/devicetree/bindings/leds/common.txt
>>> +- active-low (optional) : LED is active low.
>>
>> If it is an optional property, you would want to clarify that active
>> high is the default.
> 
> Ok, I will.
> 
>>
>>> +- default-state (optional): see
>>> +  Documentation/devicetree/bindings/leds/leds-gpio.txt
>>> +- linux,default-trigger (optional): see
>>> +  Documentation/devicetree/bindings/leds/common.txt
>>> +
>>> +Hardware controlled LED:
>>> +- brcm,hardware-controlled (optional) : LED is hardware controlled.
>>
>> The type of this property is not strictly defined, but it would appear
>> to be a boolean. Reading through the driver though, it looks like the
>> number of LEDs "hardware" controlled should be something customizable at
>> some point.
> 
> Actually the number of hardware controlled LEDs depends on each SoC, but it can also depend on the specific board. There's a 32 bit register (HWDIS) which controls which LEDs are hardware controlled.

Actually this bit toggles whether the led should be controlled by the
appropriate hardware signal, or may be controlled by software through
the appropriate registers. You can enable it for all leds, but only a
subset is actually wired to an actual function.

The hardware signals aren't consecutive. E.g. on BCM6362 0 is the USB
led, 1 is the "internet" led, and 4 to 7 are the ephy leds. 2 or 3 do
not have a defined function, at least in the public sources. BCM6328 has
its ephy activity leds at 17~20, etc.

So it's technically a configuration value, but allowing this to be
changed seems quite a challenge from the current linux led apis. AFAICT
triggers have no way of only working on certain led controllers, and led
controllers have no way of knowing that a led is controlled by a certain
trigger.

Which functions maps to which led is probably something better described
in header files or default led-nodes in the dtsi files.

>>> +- brcm,link-selection (optional) : LED link selection values.
>>
>> Here you should specify exactly which values are accepted, if this is a
>> value that is directly mapping to a hardware register value, we might
>> want to create a header file for that.
> 
> The problem of this is that it depends on each SoC but also on each board, since this controls which LED from LEDs 0-7 is controlled by each ethernet activity and speed "event". For example, Jonas managed to set a specific LED to be controlled by every ethernet port on the board, so there may be boards in which Ethernet ports may not be directly assigned to the Ethernet LEDs, but swapped (it's not the most likely situtation, but since there isn't much documentation from Broadcom available it seems sensible to me to offer that configuration posibility).

A potential use case for using more than one source for a led is if you
have only one "lan" led and want to mux all four phy activity/link
signals onto it.

A better name and description might be

- brcm,link-signal-sources (optional): An array of hardware link
  signal sources. Up to four link hardware signals can get muxed into
  this leds. Only valid for leds 0 to 7, where led signals 0 to 3 may
  be muxed to leds 0 to 3, and signals 4 to 7 may be muxed to leds
  4 to 7. A signal can be muxed to more than one led, and one led can
  have more than one source signal.

> 
>>
>>> +- brcm,activity-selection (optional) : LED activity selection values.
>>
>> Ditto.

And this one called "brcm,activity-signal-sources".


Regards
Jonas

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 0/2] BCM6328 LED driver
  2015-04-02 11:54 [PATCH 0/2] BCM6328 LED driver Álvaro Fernández Rojas
  2015-04-02 11:54 ` [PATCH 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
  2015-04-02 11:54 ` [PATCH 2/2] leds: add BCM6328 LED driver Álvaro Fernández Rojas
@ 2015-04-05 15:08 ` Álvaro Fernández Rojas
  2015-04-05 15:08   ` [PATCH v2 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
                     ` (2 more replies)
  2 siblings, 3 replies; 26+ messages in thread
From: Álvaro Fernández Rojas @ 2015-04-05 15:08 UTC (permalink / raw)
  To: linux-leds, cooloney, jogo, f.fainelli, cernekee
  Cc: Álvaro Fernández Rojas

These patches add support and documentation for the BCM6328 LED driver, which is present on BCM6318, BCM6328, BCM6362 and BCM63268.
In these SoCs it's possible to control LEDs both as GPIOs or by hardware.
However, on some devices there are Serial LEDs (LEDs connected to a 74hc controller), which can either be controlled by software (exporting the 74hc as spi-gpio) or hardware using this driver.
The problem is some of these serial LEDs are hardware controlled (ethernet LEDs) and exporting the 74hc as spi-gpio prevents those LEDs to be hardware controlled, so the only chance to keep them working is by using this driver.

v1->v2: Introduce changes suggested by Florian and Jonas.

Álvaro Fernández Rojas (2):
  leds: add DT binding for BCM6328 LED controller
  leds: add BCM6328 LED driver

 .../devicetree/bindings/leds/leds-bcm6328.txt      | 162 ++++++++
 drivers/leds/Kconfig                               |   8 +
 drivers/leds/Makefile                              |   1 +
 drivers/leds/leds-bcm6328.c                        | 409 +++++++++++++++++++++
 4 files changed, 580 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-bcm6328.txt
 create mode 100644 drivers/leds/leds-bcm6328.c

-- 
1.9.1

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 1/2] leds: add DT binding for BCM6328 LED controller
  2015-04-05 15:08 ` [PATCH v2 0/2] " Álvaro Fernández Rojas
@ 2015-04-05 15:08   ` Álvaro Fernández Rojas
  2015-04-16 14:39     ` Jacek Anaszewski
  2015-04-05 15:08   ` [PATCH 2/2] leds: add BCM6328 LED driver Álvaro Fernández Rojas
  2015-04-24 17:06   ` [PATCH v3 0/2] " Álvaro Fernández Rojas
  2 siblings, 1 reply; 26+ messages in thread
From: Álvaro Fernández Rojas @ 2015-04-05 15:08 UTC (permalink / raw)
  To: linux-leds, cooloney, jogo, f.fainelli, cernekee
  Cc: Álvaro Fernández Rojas

This adds device tree binding documentation for the Broadcom BCM6328 LED
controller.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Signed-off-by: Jonas Gorski <jogo@openwrt.org>
---
v2: Introduce changes suggested by Florian and Jonas.
 - Change compatible string to "brcm,bcm6328-leds-ctrl".
 - Make valid LEDs statement more strict.
 - Remove compatible strings from LED subnodes.
 - Clarify that LEDs are active high by default.
 - Add a better description for brcm,link-signal-sources and brcm,activity-signal-sources.

 .../devicetree/bindings/leds/leds-bcm6328.txt      | 162 +++++++++++++++++++++
 1 file changed, 162 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-bcm6328.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-bcm6328.txt b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
new file mode 100644
index 0000000..6e4f5563a
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
@@ -0,0 +1,162 @@
+LEDs connected to Broadcom BCM6328 controller
+
+Required properties:
+- compatible: should be : "brcm,bcm6328-leds-ctrl".
+- #address-cells: must be 1
+- #size-cells: must be 0
+- reg: BCM6328 LED controller address and size.
+
+Optional properties:
+- brcm,serial-leds: boolean which enables Serial LEDs.
+
+Each LED is represented as a sub-node of the brcm,bcm6328-leds device.
+
+LED sub-node properties:
+- reg: LED pin number (only LEDs 0 to 23 are valid).
+
+Normal LED:
+LEDs are active high by default.
+- label (optional): see Documentation/devicetree/bindings/leds/common.txt
+- active-low (optional): boolean that makes LED active low.
+- default-state (optional): see
+  Documentation/devicetree/bindings/leds/leds-gpio.txt
+- linux,default-trigger (optional): see
+  Documentation/devicetree/bindings/leds/common.txt
+
+Hardware controlled LED:
+- brcm,hardware-controlled (optional): boolean that makes this LED hardware
+  controlled.
+- brcm,link-signal-sources (optional): An array of hardware link
+  signal sources. Up to four link hardware signals can get muxed into
+  these LEDs. Only valid for LEDs 0 to 7, where LED signals 0 to 3 may
+  be muxed to LEDs 0 to 3, and signals 4 to 7 may be muxed to LEDs
+  4 to 7. A signal can be muxed to more than one LED, and one LED can
+  have more than one source signal.
+- brcm,activity-signal-sources (optional): An array of hardware activity
+  signal sources. Up to four activity hardware signals can get muxed into
+  these LEDs. Only valid for LEDs 0 to 7, where LED signals 0 to 3 may
+  be muxed to LEDs 0 to 3, and signals 4 to 7 may be muxed to LEDs
+  4 to 7. A signal can be muxed to more than one LED, and one LED can
+  have more than one source signal.
+
+example 1) BCM6328
+
+leds0: led-controller@10000800 {
+	compatible = "brcm,bcm6328-leds-ctrl";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reg = <0x10000800 0x24>;
+
+	alarm_red@2 {
+		reg = <2>;
+		active-low;
+		label = "red:alarm";
+	};
+	inet_green@3 {
+		reg = <3>;
+		active-low;
+		label = "green:inet";
+	};
+	power_green@4 {
+		reg = <4>;
+		active-low;
+		label = "green:power";
+		default-state = "on";
+	};
+	ephy0_spd@17 {
+		reg = <17>;
+		brcm,hardware-controlled;
+	};
+	ephy1_spd@18 {
+		reg = <18>;
+		brcm,hardware-controlled;
+	};
+	ephy2_spd@19 {
+		reg = <19>;
+		brcm,hardware-controlled;
+	};
+	ephy3_spd@20 {
+		reg = <20>;
+		brcm,hardware-controlled;
+	};
+};
+
+example 2) BCM63268
+
+leds0: led-controller@10001900 {
+	compatible = "brcm,bcm6328-leds-ctrl";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reg = <0x10001900 0x24>;
+	brcm,serial-leds;
+
+	gphy0_spd0@0 {
+		reg = <0>;
+		brcm,hardware-controlled;
+		brcm,link-signal-sources = <0>;
+	};
+	gphy0_spd1@1 {
+		reg = <1>;
+		brcm,hardware-controlled;
+		brcm,link-signal-sources = <1>;
+	};
+	inet_red@2 {
+		reg = <2>;
+		active-low;
+		label = "red:inet";
+	};
+	dsl_green@3 {
+		reg = <3>;
+		active-low;
+		label = "green:dsl";
+	};
+	usb_green@4 {
+		reg = <4>;
+		active-low;
+		label = "green:usb";
+	};
+	wps_green@7 {
+		reg = <7>;
+		active-low;
+		label = "green:wps";
+	};
+	inet_green@8 {
+		reg = <8>;
+		active-low;
+		label = "green:inet";
+	};
+	ephy0_act@9 {
+		reg = <9>;
+		brcm,hardware-controlled;
+	};
+	ephy1_act@10 {
+		reg = <10>;
+		brcm,hardware-controlled;
+	};
+	ephy2_act@11 {
+		reg = <11>;
+		brcm,hardware-controlled;
+	};
+	gphy0_act@12 {
+		reg = <12>;
+		brcm,hardware-controlled;
+	};
+	ephy0_spd@13 {
+		reg = <13>;
+		brcm,hardware-controlled;
+	};
+	ephy1_spd@14 {
+		reg = <14>;
+		brcm,hardware-controlled;
+	};
+	ephy2_spd@15 {
+		reg = <15>;
+		brcm,hardware-controlled;
+	};
+	power_green@20 {
+		reg = <20>;
+		active-low;
+		label = "green:power";
+		default-state = "on";
+	};
+};
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 2/2] leds: add BCM6328 LED driver
  2015-04-05 15:08 ` [PATCH v2 0/2] " Álvaro Fernández Rojas
  2015-04-05 15:08   ` [PATCH v2 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
@ 2015-04-05 15:08   ` Álvaro Fernández Rojas
  2015-04-16 14:37     ` Jacek Anaszewski
  2015-04-24 17:06   ` [PATCH v3 0/2] " Álvaro Fernández Rojas
  2 siblings, 1 reply; 26+ messages in thread
From: Álvaro Fernández Rojas @ 2015-04-05 15:08 UTC (permalink / raw)
  To: linux-leds, cooloney, jogo, f.fainelli, cernekee
  Cc: Álvaro Fernández Rojas

This adds support for the LED controller on Broadcom's BCM6328.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Signed-off-by: Jonas Gorski <jogo@openwrt.org>
---
v2: introduce changes suggested by Florian and Jonas.
 - Improve Kconfig description.
 - Remove unused imports.
 - Fix bug in blink_set so it works for multiple LEDs.
 - "brcm,link-selection" -> "brcm,link-signal-sources".
 - "brcm,activity-selection" -> "brcm,activity-signal-sources".
 - Use of_property_read_bool instead of of_get_property for booleans.
 - Add MODULE_AUTHOR strings.

 drivers/leds/Kconfig        |   8 +
 drivers/leds/Makefile       |   1 +
 drivers/leds/leds-bcm6328.c | 409 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 418 insertions(+)
 create mode 100644 drivers/leds/leds-bcm6328.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 25b320d..64a12e4 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -42,6 +42,14 @@ config LEDS_88PM860X
 	  This option enables support for on-chip LED drivers found on Marvell
 	  Semiconductor 88PM8606 PMIC.
 
+config LEDS_BCM6328
+	tristate "LED Support for Broadcom BCM6328"
+	depends on LEDS_CLASS
+	depends on OF
+	help
+	  This option enables support for LEDs connected to the BCM6328
+	  LED HW controller accessed via MMIO registers.
+
 config LEDS_LM3530
 	tristate "LCD Backlight driver for LM3530"
 	depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index cbba921..bf69f49 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_LEDS_TRIGGERS)		+= led-triggers.o
 
 # LED Platform Drivers
 obj-$(CONFIG_LEDS_88PM860X)		+= leds-88pm860x.o
+obj-$(CONFIG_LEDS_BCM6328)		+= leds-bcm6328.o
 obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
 obj-$(CONFIG_LEDS_LOCOMO)		+= leds-locomo.o
 obj-$(CONFIG_LEDS_LM3530)		+= leds-lm3530.o
diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
new file mode 100644
index 0000000..6a63f9c
--- /dev/null
+++ b/drivers/leds/leds-bcm6328.c
@@ -0,0 +1,409 @@
+/*
+ * Driver for BCM6328 memory-mapped LEDs, based on leds-syscon.c
+ *
+ * Copyright 2015 Álvaro Fernández Rojas <noltari@gmail.com>
+ * Copyright 2015 Jonas Gorski <jogo@openwrt.org>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+#include <linux/io.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+
+#define REG_INIT		0x00
+#define REG_MODE_HI		0x04
+#define REG_MODE_LO		0x08
+#define REG_HWDIS		0x0c
+#define REG_STROBE		0x10
+#define REG_LNKACTSEL_HI	0x14
+#define REG_LNKACTSEL_LO	0x18
+#define REG_RBACK		0x1c
+#define REG_SERMUX		0x20
+
+#define LED_MAX_COUNT		24
+#define LED_DEF_DELAY		500
+#define LED_INTERVAL_MS		20
+
+#define LED_INTV_MASK		0x3f
+#define LED_FAST_INTV_SHIFT	6
+#define LED_FAST_INTV_MASK	(LED_INTV_MASK << LED_FAST_INTV_SHIFT)
+#define SERIAL_LED_EN		BIT(12)
+#define SERIAL_LED_MUX		BIT(13)
+#define SERIAL_LED_CLK_NPOL	BIT(14)
+#define SERIAL_LED_DATA_PPOL	BIT(15)
+#define SERIAL_LED_SHIFT_DIR	BIT(16)
+#define LED_SHIFT_TEST		BIT(30)
+#define LED_TEST		BIT(31)
+
+#define LED_MODE_MASK		3
+#define LED_MODE_OFF		0
+#define LED_MODE_FAST		1
+#define LED_MODE_BLINK		2
+#define LED_MODE_ON		3
+#define LED_SHIFT(X)		((X) << 1)
+
+/**
+ * struct bcm6328_led - state container for bcm6328 based LEDs
+ * @cdev: LED class device for this LED
+ * @mem: memory resource
+ * @lock: memory lock
+ * @pin: LED pin number
+ * @blink_leds: blinking LEDs
+ * @blink_del: blinking delay
+ * @active_low: LED is active low
+ */
+struct bcm6328_led {
+	struct led_classdev cdev;
+	void __iomem *mem;
+	spinlock_t *lock;
+	unsigned long pin;
+	unsigned long *blink_leds;
+	unsigned long *blink_del;
+	bool active_low;
+};
+
+static void bcm6328_led_write(void __iomem *reg, unsigned long data)
+{
+	iowrite32be(data, reg);
+}
+
+static unsigned long bcm6328_led_read(void __iomem *reg)
+{
+	return ioread32be(reg);
+}
+
+/**
+ * LEDMode 64 bits / 24 LEDs
+ * bits [31:0] -> LEDs 8-23
+ * bits [47:32] -> LEDs 0-7
+ * bits [63:48] -> unused
+ */
+static unsigned long bcm6328_pin2shift(unsigned long pin)
+{
+	if (pin < 8)
+		return pin + 16; /* LEDs 0-7 (bits 47:32) */
+	else
+		return pin - 8; /* LEDs 8-23 (bits 31:0) */
+}
+
+static void bcm6328_led_mode(struct bcm6328_led *led, unsigned long value)
+{
+	void __iomem *mode;
+	unsigned long val, shift;
+
+	shift = bcm6328_pin2shift(led->pin);
+	if (shift / 16)
+		mode = led->mem + REG_MODE_HI;
+	else
+		mode = led->mem + REG_MODE_LO;
+
+	val = bcm6328_led_read(mode);
+	val &= ~(LED_MODE_MASK << LED_SHIFT(shift % 16));
+	val |= (value << LED_SHIFT(shift % 16));
+	bcm6328_led_write(mode, val);
+}
+
+static void bcm6328_led_set(struct led_classdev *led_cdev,
+	enum led_brightness value)
+{
+	struct bcm6328_led *led =
+		container_of(led_cdev, struct bcm6328_led, cdev);
+	unsigned long flags;
+
+	spin_lock_irqsave(led->lock, flags);
+	*(led->blink_leds) &= ~BIT(led->pin);
+	if ((led->active_low && value == LED_OFF) ||
+	    (!led->active_low && value != LED_OFF))
+		bcm6328_led_mode(led, LED_MODE_OFF);
+	else
+		bcm6328_led_mode(led, LED_MODE_ON);
+	spin_unlock_irqrestore(led->lock, flags);
+}
+
+static int bcm6328_blink_set(struct led_classdev *led_cdev,
+			     unsigned long *delay_on, unsigned long *delay_off)
+{
+	struct bcm6328_led *led =
+		container_of(led_cdev, struct bcm6328_led, cdev);
+	unsigned long delay, flags;
+
+	if (!*delay_on)
+		*delay_on = LED_DEF_DELAY;
+	if (!*delay_off)
+		*delay_off = LED_DEF_DELAY;
+
+	if (*delay_on != *delay_off) {
+		dev_dbg(led_cdev->dev,
+			"fallback to soft blinking (delay_on != delay_off)\n");
+		return -EINVAL;
+	}
+
+	delay = *delay_on / LED_INTERVAL_MS;
+	if (delay == 0)
+		delay = 1;
+	else if (delay > LED_INTV_MASK) {
+		dev_dbg(led_cdev->dev,
+			"fallback to soft blinking (delay > %ums)\n",
+			LED_INTV_MASK * LED_INTERVAL_MS);
+		return -EINVAL;
+	}
+
+	spin_lock_irqsave(led->lock, flags);
+	if (*(led->blink_leds) == 0 ||
+	    *(led->blink_leds) == BIT(led->pin) ||
+	    *(led->blink_del) == delay) {
+		unsigned long val;
+
+		*(led->blink_leds) |= BIT(led->pin);
+		*(led->blink_del) = delay;
+
+		val = bcm6328_led_read(led->mem + REG_INIT);
+		val &= ~LED_FAST_INTV_MASK;
+		val |= (delay << LED_FAST_INTV_SHIFT);
+		bcm6328_led_write(led->mem + REG_INIT, val);
+
+		bcm6328_led_mode(led, LED_MODE_BLINK);
+
+		spin_unlock_irqrestore(led->lock, flags);
+	} else {
+		spin_unlock_irqrestore(led->lock, flags);
+		dev_dbg(led_cdev->dev,
+			"fallback to soft blinking (delay already set)\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int bcm6328_hwled(struct device *dev, struct device_node *nc, u32 reg,
+			 void __iomem *mem, spinlock_t *lock)
+{
+	int i, cnt;
+	unsigned long flags, val;
+
+	spin_lock_irqsave(lock, flags);
+	val = bcm6328_led_read(mem + REG_HWDIS);
+	val &= ~BIT(reg);
+	bcm6328_led_write(mem + REG_HWDIS, val);
+	spin_unlock_irqrestore(lock, flags);
+
+	/* Only LEDs 0-7 can be activity/link controlled */
+	if (reg >= 8)
+		return 0;
+
+	cnt = of_property_count_elems_of_size(nc, "brcm,link-signal-sources",
+					      sizeof(u32));
+	for (i = 0; i < cnt; i++) {
+		u32 sel;
+		void __iomem *addr;
+
+		if (reg < 4)
+			addr = mem + REG_LNKACTSEL_LO;
+		else
+			addr = mem + REG_LNKACTSEL_HI;
+
+		of_property_read_u32_index(nc, "brcm,link-signal-sources", i, &sel);
+
+		if (reg / 4 != sel / 4) {
+			dev_warn(dev, "invalid link signal source\n");
+			continue;
+		}
+
+		spin_lock_irqsave(lock, flags);
+		val = bcm6328_led_read(addr);
+		val |= (BIT(reg) << (((sel % 4) * 4) + 16));
+		bcm6328_led_write(addr, val);
+		spin_unlock_irqrestore(lock, flags);
+	}
+
+	cnt = of_property_count_elems_of_size(nc, "brcm,activity-signal-sources",
+					      sizeof(u32));
+	for (i = 0; i < cnt; i++) {
+		u32 sel;
+		void __iomem *addr;
+
+		if (reg < 4)
+			addr = mem + REG_LNKACTSEL_LO;
+		else
+			addr = mem + REG_LNKACTSEL_HI;
+
+		of_property_read_u32_index(nc, "brcm,activity-signal-sources", i,
+					   &sel);
+
+		if (reg / 4 != sel / 4) {
+			dev_warn(dev, "invalid activity signal source\n");
+			continue;
+		}
+
+		spin_lock_irqsave(lock, flags);
+		val = bcm6328_led_read(addr);
+		val |= (BIT(reg) << ((sel % 4) * 4));
+		bcm6328_led_write(addr, val);
+		spin_unlock_irqrestore(lock, flags);
+	}
+
+	return 0;
+}
+
+static int bcm6328_led(struct device *dev, struct device_node *nc, u32 reg,
+		       void __iomem *mem, spinlock_t *lock,
+		       unsigned long *blink_leds, unsigned long *blink_del)
+{
+	struct bcm6328_led *led;
+	unsigned long flags;
+	const char *state;
+	int rc;
+
+	led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+
+	led->pin = reg;
+	led->mem = mem;
+	led->lock = lock;
+	led->blink_leds = blink_leds;
+	led->blink_del = blink_del;
+
+	if (of_property_read_bool(nc, "active-low"))
+		led->active_low = 1;
+
+	led->cdev.name = of_get_property(nc, "label", NULL) ? : nc->name;
+	led->cdev.default_trigger = of_get_property(nc,
+						    "linux,default-trigger",
+						    NULL);
+
+	if (!of_property_read_string(nc, "default-state", &state)) {
+		spin_lock_irqsave(lock, flags);
+		if (!strcmp(state, "on")) {
+			led->cdev.brightness = LED_FULL;
+			bcm6328_led_mode(led, LED_MODE_ON);
+		} else if (!strcmp(state, "keep")) {
+			void __iomem *mode;
+			unsigned long val, shift;
+
+			shift = bcm6328_pin2shift(led->pin);
+			if (shift / 16)
+				mode = mem + REG_MODE_HI;
+			else
+				mode = mem + REG_MODE_LO;
+
+			val = bcm6328_led_read(mode) >> (shift % 16);
+			val &= LED_MODE_MASK;
+			if (val == LED_MODE_ON)
+				led->cdev.brightness = LED_FULL;
+			else {
+				led->cdev.brightness = LED_OFF;
+				bcm6328_led_mode(led, LED_MODE_OFF);
+			}
+		} else {
+			led->cdev.brightness = LED_OFF;
+			bcm6328_led_mode(led, LED_MODE_OFF);
+		}
+		spin_unlock_irqrestore(lock, flags);
+	}
+
+	led->cdev.brightness_set = bcm6328_led_set;
+	led->cdev.blink_set = bcm6328_blink_set;
+
+	rc = led_classdev_register(dev, &led->cdev);
+	if (rc < 0)
+		return rc;
+
+	dev_dbg(dev, "registered LED %s\n", led->cdev.name);
+
+	return 0;
+}
+
+static int bcm6328_leds_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *child;
+	struct resource *mem_r;
+	void __iomem *mem;
+	spinlock_t *lock;
+	unsigned long val, *blink_leds, *blink_del;
+
+	mem_r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mem_r)
+		return -EINVAL;
+
+	mem = devm_ioremap_resource(dev, mem_r);
+	if (IS_ERR(mem))
+		return PTR_ERR(mem);
+
+	lock = devm_kzalloc(dev, sizeof(*lock), GFP_KERNEL);
+	if (!lock)
+		return -ENOMEM;
+
+	blink_leds = devm_kzalloc(dev, sizeof(*blink_leds), GFP_KERNEL);
+	if (!blink_leds)
+		return -ENOMEM;
+
+	blink_del = devm_kzalloc(dev, sizeof(*blink_del), GFP_KERNEL);
+	if (!blink_del)
+		return -ENOMEM;
+
+	spin_lock_init(lock);
+
+	bcm6328_led_write(mem + REG_HWDIS, ~0);
+	bcm6328_led_write(mem + REG_LNKACTSEL_HI, 0);
+	bcm6328_led_write(mem + REG_LNKACTSEL_LO, 0);
+
+	val = bcm6328_led_read(mem + REG_INIT);
+	val &= ~SERIAL_LED_EN;
+	if (of_property_read_bool(np, "brcm,serial-leds"))
+		val |= SERIAL_LED_EN;
+	bcm6328_led_write(mem + REG_INIT, val);
+
+	for_each_available_child_of_node(np, child) {
+		int rc;
+		u32 reg;
+
+		if (of_property_read_u32(child, "reg", &reg))
+			continue;
+
+		if (reg >= LED_MAX_COUNT) {
+			dev_err(dev, "invalid LED (>= %d)\n", LED_MAX_COUNT);
+			continue;
+		}
+
+		if (of_property_read_bool(child, "brcm,hardware-controlled"))
+			rc = bcm6328_hwled(dev, child, reg, mem, lock);
+		else
+			rc = bcm6328_led(dev, child, reg, mem, lock,
+					  blink_leds, blink_del);
+
+		if (rc < 0)
+			return rc;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id bcm6328_leds_of_match[] = {
+	{ .compatible = "brcm,bcm6328-leds-ctrl", },
+	{ },
+};
+
+static struct platform_driver bcm6328_leds_driver = {
+	.probe = bcm6328_leds_probe,
+	.driver = {
+		.name = "leds-bcm6328",
+		.of_match_table = bcm6328_leds_of_match,
+	},
+};
+
+module_platform_driver(bcm6328_leds_driver);
+
+MODULE_AUTHOR("Álvaro Fernández Rojas <noltari@gmail.com>");
+MODULE_AUTHOR("Jonas Gorski <jogo@openwrt.org>");
+MODULE_DESCRIPTION("LED driver for BCM6328 controllers");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:leds-bcm6328");
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] leds: add BCM6328 LED driver
  2015-04-05 15:08   ` [PATCH 2/2] leds: add BCM6328 LED driver Álvaro Fernández Rojas
@ 2015-04-16 14:37     ` Jacek Anaszewski
  2015-04-18  9:14       ` Jonas Gorski
  0 siblings, 1 reply; 26+ messages in thread
From: Jacek Anaszewski @ 2015-04-16 14:37 UTC (permalink / raw)
  To: Álvaro Fernández Rojas
  Cc: linux-leds, cooloney, jogo, f.fainelli, cernekee

Hi Alvaro,

On 04/05/2015 05:08 PM, Álvaro Fernández Rojas wrote:
> This adds support for the LED controller on Broadcom's BCM6328.
>
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> Signed-off-by: Jonas Gorski <jogo@openwrt.org>
> ---
> v2: introduce changes suggested by Florian and Jonas.
>   - Improve Kconfig description.
>   - Remove unused imports.
>   - Fix bug in blink_set so it works for multiple LEDs.
>   - "brcm,link-selection" -> "brcm,link-signal-sources".
>   - "brcm,activity-selection" -> "brcm,activity-signal-sources".
>   - Use of_property_read_bool instead of of_get_property for booleans.
>   - Add MODULE_AUTHOR strings.
>
>   drivers/leds/Kconfig        |   8 +
>   drivers/leds/Makefile       |   1 +
>   drivers/leds/leds-bcm6328.c | 409 ++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 418 insertions(+)
>   create mode 100644 drivers/leds/leds-bcm6328.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 25b320d..64a12e4 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -42,6 +42,14 @@ config LEDS_88PM860X
>   	  This option enables support for on-chip LED drivers found on Marvell
>   	  Semiconductor 88PM8606 PMIC.
>
> +config LEDS_BCM6328
> +	tristate "LED Support for Broadcom BCM6328"
> +	depends on LEDS_CLASS
> +	depends on OF
> +	help
> +	  This option enables support for LEDs connected to the BCM6328
> +	  LED HW controller accessed via MMIO registers.
> +
>   config LEDS_LM3530
>   	tristate "LCD Backlight driver for LM3530"
>   	depends on LEDS_CLASS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index cbba921..bf69f49 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_LEDS_TRIGGERS)		+= led-triggers.o
>
>   # LED Platform Drivers
>   obj-$(CONFIG_LEDS_88PM860X)		+= leds-88pm860x.o
> +obj-$(CONFIG_LEDS_BCM6328)		+= leds-bcm6328.o
>   obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
>   obj-$(CONFIG_LEDS_LOCOMO)		+= leds-locomo.o
>   obj-$(CONFIG_LEDS_LM3530)		+= leds-lm3530.o
> diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
> new file mode 100644
> index 0000000..6a63f9c
> --- /dev/null
> +++ b/drivers/leds/leds-bcm6328.c
> @@ -0,0 +1,409 @@
> +/*
> + * Driver for BCM6328 memory-mapped LEDs, based on leds-syscon.c
> + *
> + * Copyright 2015 Álvaro Fernández Rojas <noltari@gmail.com>
> + * Copyright 2015 Jonas Gorski <jogo@openwrt.org>
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */
> +#include <linux/io.h>
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +
> +#define REG_INIT		0x00
> +#define REG_MODE_HI		0x04
> +#define REG_MODE_LO		0x08
> +#define REG_HWDIS		0x0c
> +#define REG_STROBE		0x10
> +#define REG_LNKACTSEL_HI	0x14
> +#define REG_LNKACTSEL_LO	0x18
> +#define REG_RBACK		0x1c
> +#define REG_SERMUX		0x20
> +
> +#define LED_MAX_COUNT		24
> +#define LED_DEF_DELAY		500
> +#define LED_INTERVAL_MS		20
> +
> +#define LED_INTV_MASK		0x3f
> +#define LED_FAST_INTV_SHIFT	6
> +#define LED_FAST_INTV_MASK	(LED_INTV_MASK << LED_FAST_INTV_SHIFT)
> +#define SERIAL_LED_EN		BIT(12)
> +#define SERIAL_LED_MUX		BIT(13)
> +#define SERIAL_LED_CLK_NPOL	BIT(14)
> +#define SERIAL_LED_DATA_PPOL	BIT(15)
> +#define SERIAL_LED_SHIFT_DIR	BIT(16)
> +#define LED_SHIFT_TEST		BIT(30)
> +#define LED_TEST		BIT(31)
> +
> +#define LED_MODE_MASK		3
> +#define LED_MODE_OFF		0
> +#define LED_MODE_FAST		1
> +#define LED_MODE_BLINK		2
> +#define LED_MODE_ON		3
> +#define LED_SHIFT(X)		((X) << 1)

Please add BCM6328_ namespacing prefix to the macros above.

> +
> +/**
> + * struct bcm6328_led - state container for bcm6328 based LEDs
> + * @cdev: LED class device for this LED
> + * @mem: memory resource
> + * @lock: memory lock
> + * @pin: LED pin number
> + * @blink_leds: blinking LEDs
> + * @blink_del: blinking delay
> + * @active_low: LED is active low
> + */
> +struct bcm6328_led {
> +	struct led_classdev cdev;
> +	void __iomem *mem;
> +	spinlock_t *lock;
> +	unsigned long pin;
> +	unsigned long *blink_leds;
> +	unsigned long *blink_del;

I would go for blink_delay, as "del" suggests "delete".

> +	bool active_low;
> +};
> +
> +static void bcm6328_led_write(void __iomem *reg, unsigned long data)
> +{
> +	iowrite32be(data, reg);
> +}
> +
> +static unsigned long bcm6328_led_read(void __iomem *reg)
> +{
> +	return ioread32be(reg);
> +}
> +
> +/**
> + * LEDMode 64 bits / 24 LEDs
> + * bits [31:0] -> LEDs 8-23
> + * bits [47:32] -> LEDs 0-7
> + * bits [63:48] -> unused
> + */
> +static unsigned long bcm6328_pin2shift(unsigned long pin)
> +{
> +	if (pin < 8)
> +		return pin + 16; /* LEDs 0-7 (bits 47:32) */
> +	else
> +		return pin - 8; /* LEDs 8-23 (bits 31:0) */
> +}
> +
> +static void bcm6328_led_mode(struct bcm6328_led *led, unsigned long value)
> +{
> +	void __iomem *mode;
> +	unsigned long val, shift;
> +
> +	shift = bcm6328_pin2shift(led->pin);
> +	if (shift / 16)
> +		mode = led->mem + REG_MODE_HI;
> +	else
> +		mode = led->mem + REG_MODE_LO;
> +
> +	val = bcm6328_led_read(mode);
> +	val &= ~(LED_MODE_MASK << LED_SHIFT(shift % 16));
> +	val |= (value << LED_SHIFT(shift % 16));
> +	bcm6328_led_write(mode, val);
> +}
> +
> +static void bcm6328_led_set(struct led_classdev *led_cdev,
> +	enum led_brightness value)
> +{
> +	struct bcm6328_led *led =
> +		container_of(led_cdev, struct bcm6328_led, cdev);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(led->lock, flags);
> +	*(led->blink_leds) &= ~BIT(led->pin);
> +	if ((led->active_low && value == LED_OFF) ||
> +	    (!led->active_low && value != LED_OFF))
> +		bcm6328_led_mode(led, LED_MODE_OFF);
> +	else
> +		bcm6328_led_mode(led, LED_MODE_ON);
> +	spin_unlock_irqrestore(led->lock, flags);
> +}
> +
> +static int bcm6328_blink_set(struct led_classdev *led_cdev,
> +			     unsigned long *delay_on, unsigned long *delay_off)
> +{
> +	struct bcm6328_led *led =
> +		container_of(led_cdev, struct bcm6328_led, cdev);
> +	unsigned long delay, flags;
> +
> +	if (!*delay_on)
> +		*delay_on = LED_DEF_DELAY;
> +	if (!*delay_off)
> +		*delay_off = LED_DEF_DELAY;
> +
> +	if (*delay_on != *delay_off) {
> +		dev_dbg(led_cdev->dev,
> +			"fallback to soft blinking (delay_on != delay_off)\n");
> +		return -EINVAL;
> +	}
> +
> +	delay = *delay_on / LED_INTERVAL_MS;
> +	if (delay == 0)
> +		delay = 1;
> +	else if (delay > LED_INTV_MASK) {
> +		dev_dbg(led_cdev->dev,
> +			"fallback to soft blinking (delay > %ums)\n",
> +			LED_INTV_MASK * LED_INTERVAL_MS);
> +		return -EINVAL;
> +	}
> +
> +	spin_lock_irqsave(led->lock, flags);
> +	if (*(led->blink_leds) == 0 ||
> +	    *(led->blink_leds) == BIT(led->pin) ||
> +	    *(led->blink_del) == delay) {
> +		unsigned long val;
> +
> +		*(led->blink_leds) |= BIT(led->pin);
> +		*(led->blink_del) = delay;
> +
> +		val = bcm6328_led_read(led->mem + REG_INIT);
> +		val &= ~LED_FAST_INTV_MASK;
> +		val |= (delay << LED_FAST_INTV_SHIFT);
> +		bcm6328_led_write(led->mem + REG_INIT, val);
> +
> +		bcm6328_led_mode(led, LED_MODE_BLINK);
> +
> +		spin_unlock_irqrestore(led->lock, flags);
> +	} else {
> +		spin_unlock_irqrestore(led->lock, flags);
> +		dev_dbg(led_cdev->dev,
> +			"fallback to soft blinking (delay already set)\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int bcm6328_hwled(struct device *dev, struct device_node *nc, u32 reg,
> +			 void __iomem *mem, spinlock_t *lock)
> +{
> +	int i, cnt;
> +	unsigned long flags, val;
> +
> +	spin_lock_irqsave(lock, flags);
> +	val = bcm6328_led_read(mem + REG_HWDIS);
> +	val &= ~BIT(reg);
> +	bcm6328_led_write(mem + REG_HWDIS, val);

What is the purpose of REG_HWDIS register? Does it activate the LED or
so?

> +	spin_unlock_irqrestore(lock, flags);
> +
> +	/* Only LEDs 0-7 can be activity/link controlled */
> +	if (reg >= 8)
> +		return 0;
> +
> +	cnt = of_property_count_elems_of_size(nc, "brcm,link-signal-sources",
> +					      sizeof(u32));
> +	for (i = 0; i < cnt; i++) {
> +		u32 sel;
> +		void __iomem *addr;
> +
> +		if (reg < 4)
> +			addr = mem + REG_LNKACTSEL_LO;
> +		else
> +			addr = mem + REG_LNKACTSEL_HI;
> +
> +		of_property_read_u32_index(nc, "brcm,link-signal-sources", i, &sel);
> +
> +		if (reg / 4 != sel / 4) {
> +			dev_warn(dev, "invalid link signal source\n");
> +			continue;
> +		}
> +
> +		spin_lock_irqsave(lock, flags);
> +		val = bcm6328_led_read(addr);
> +		val |= (BIT(reg) << (((sel % 4) * 4) + 16));
> +		bcm6328_led_write(addr, val);
> +		spin_unlock_irqrestore(lock, flags);
> +	}
> +
> +	cnt = of_property_count_elems_of_size(nc, "brcm,activity-signal-sources",
> +					      sizeof(u32));
> +	for (i = 0; i < cnt; i++) {
> +		u32 sel;
> +		void __iomem *addr;
> +
> +		if (reg < 4)
> +			addr = mem + REG_LNKACTSEL_LO;
> +		else
> +			addr = mem + REG_LNKACTSEL_HI;
> +
> +		of_property_read_u32_index(nc, "brcm,activity-signal-sources", i,
> +					   &sel);
> +
> +		if (reg / 4 != sel / 4) {
> +			dev_warn(dev, "invalid activity signal source\n");
> +			continue;
> +		}
> +
> +		spin_lock_irqsave(lock, flags);
> +		val = bcm6328_led_read(addr);
> +		val |= (BIT(reg) << ((sel % 4) * 4));
> +		bcm6328_led_write(addr, val);
> +		spin_unlock_irqrestore(lock, flags);
> +	}
> +
> +	return 0;
> +}
> +
> +static int bcm6328_led(struct device *dev, struct device_node *nc, u32 reg,
> +		       void __iomem *mem, spinlock_t *lock,
> +		       unsigned long *blink_leds, unsigned long *blink_del)
> +{
> +	struct bcm6328_led *led;
> +	unsigned long flags;
> +	const char *state;
> +	int rc;
> +
> +	led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
> +	if (!led)
> +		return -ENOMEM;
> +
> +	led->pin = reg;
> +	led->mem = mem;
> +	led->lock = lock;
> +	led->blink_leds = blink_leds;
> +	led->blink_del = blink_del;
> +
> +	if (of_property_read_bool(nc, "active-low"))
> +		led->active_low = 1;
> +
> +	led->cdev.name = of_get_property(nc, "label", NULL) ? : nc->name;
> +	led->cdev.default_trigger = of_get_property(nc,
> +						    "linux,default-trigger",
> +						    NULL);
> +
> +	if (!of_property_read_string(nc, "default-state", &state)) {
> +		spin_lock_irqsave(lock, flags);
> +		if (!strcmp(state, "on")) {
> +			led->cdev.brightness = LED_FULL;
> +			bcm6328_led_mode(led, LED_MODE_ON);
> +		} else if (!strcmp(state, "keep")) {
> +			void __iomem *mode;
> +			unsigned long val, shift;
> +
> +			shift = bcm6328_pin2shift(led->pin);
> +			if (shift / 16)
> +				mode = mem + REG_MODE_HI;
> +			else
> +				mode = mem + REG_MODE_LO;
> +
> +			val = bcm6328_led_read(mode) >> (shift % 16);
> +			val &= LED_MODE_MASK;
> +			if (val == LED_MODE_ON)
> +				led->cdev.brightness = LED_FULL;
> +			else {
> +				led->cdev.brightness = LED_OFF;
> +				bcm6328_led_mode(led, LED_MODE_OFF);
> +			}
> +		} else {
> +			led->cdev.brightness = LED_OFF;
> +			bcm6328_led_mode(led, LED_MODE_OFF);
> +		}
> +		spin_unlock_irqrestore(lock, flags);
> +	}
> +
> +	led->cdev.brightness_set = bcm6328_led_set;
> +	led->cdev.blink_set = bcm6328_blink_set;
> +
> +	rc = led_classdev_register(dev, &led->cdev);
> +	if (rc < 0)
> +		return rc;
> +
> +	dev_dbg(dev, "registered LED %s\n", led->cdev.name);
> +
> +	return 0;
> +}
> +
> +static int bcm6328_leds_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *child;
> +	struct resource *mem_r;
> +	void __iomem *mem;
> +	spinlock_t *lock;
> +	unsigned long val, *blink_leds, *blink_del;
> +
> +	mem_r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!mem_r)
> +		return -EINVAL;
> +
> +	mem = devm_ioremap_resource(dev, mem_r);
> +	if (IS_ERR(mem))
> +		return PTR_ERR(mem);
> +
> +	lock = devm_kzalloc(dev, sizeof(*lock), GFP_KERNEL);
> +	if (!lock)
> +		return -ENOMEM;
> +
> +	blink_leds = devm_kzalloc(dev, sizeof(*blink_leds), GFP_KERNEL);
> +	if (!blink_leds)
> +		return -ENOMEM;
> +
> +	blink_del = devm_kzalloc(dev, sizeof(*blink_del), GFP_KERNEL);
> +	if (!blink_del)
> +		return -ENOMEM;
> +
> +	spin_lock_init(lock);
> +
> +	bcm6328_led_write(mem + REG_HWDIS, ~0);
> +	bcm6328_led_write(mem + REG_LNKACTSEL_HI, 0);
> +	bcm6328_led_write(mem + REG_LNKACTSEL_LO, 0);
> +
> +	val = bcm6328_led_read(mem + REG_INIT);
> +	val &= ~SERIAL_LED_EN;
> +	if (of_property_read_bool(np, "brcm,serial-leds"))
> +		val |= SERIAL_LED_EN;
> +	bcm6328_led_write(mem + REG_INIT, val);
> +
> +	for_each_available_child_of_node(np, child) {
> +		int rc;
> +		u32 reg;
> +
> +		if (of_property_read_u32(child, "reg", &reg))
> +			continue;
> +
> +		if (reg >= LED_MAX_COUNT) {
> +			dev_err(dev, "invalid LED (>= %d)\n", LED_MAX_COUNT);
> +			continue;
> +		}
> +
> +		if (of_property_read_bool(child, "brcm,hardware-controlled"))
> +			rc = bcm6328_hwled(dev, child, reg, mem, lock);
> +		else
> +			rc = bcm6328_led(dev, child, reg, mem, lock,
> +					  blink_leds, blink_del);
> +
> +		if (rc < 0)
> +			return rc;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id bcm6328_leds_of_match[] = {
> +	{ .compatible = "brcm,bcm6328-leds-ctrl", },
> +	{ },
> +};
> +
> +static struct platform_driver bcm6328_leds_driver = {
> +	.probe = bcm6328_leds_probe,
> +	.driver = {
> +		.name = "leds-bcm6328",
> +		.of_match_table = bcm6328_leds_of_match,
> +	},
> +};
> +
> +module_platform_driver(bcm6328_leds_driver);
> +
> +MODULE_AUTHOR("Álvaro Fernández Rojas <noltari@gmail.com>");
> +MODULE_AUTHOR("Jonas Gorski <jogo@openwrt.org>");
> +MODULE_DESCRIPTION("LED driver for BCM6328 controllers");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:leds-bcm6328");
>


-- 
Best Regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 1/2] leds: add DT binding for BCM6328 LED controller
  2015-04-05 15:08   ` [PATCH v2 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
@ 2015-04-16 14:39     ` Jacek Anaszewski
  0 siblings, 0 replies; 26+ messages in thread
From: Jacek Anaszewski @ 2015-04-16 14:39 UTC (permalink / raw)
  To: Álvaro Fernández Rojas
  Cc: linux-leds, cooloney, jogo, f.fainelli, cernekee, devicetree

Hi Alvaro,

On 04/05/2015 05:08 PM, Álvaro Fernández Rojas wrote:
> This adds device tree binding documentation for the Broadcom BCM6328 LED
> controller.
>
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> Signed-off-by: Jonas Gorski <jogo@openwrt.org>
> ---
> v2: Introduce changes suggested by Florian and Jonas.
>   - Change compatible string to "brcm,bcm6328-leds-ctrl".
>   - Make valid LEDs statement more strict.
>   - Remove compatible strings from LED subnodes.
>   - Clarify that LEDs are active high by default.
>   - Add a better description for brcm,link-signal-sources and brcm,activity-signal-sources.
>
>   .../devicetree/bindings/leds/leds-bcm6328.txt      | 162 +++++++++++++++++++++
>   1 file changed, 162 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/leds/leds-bcm6328.txt
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-bcm6328.txt b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
> new file mode 100644
> index 0000000..6e4f5563a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
> @@ -0,0 +1,162 @@
> +LEDs connected to Broadcom BCM6328 controller

Part of description from the cover letter could find its way here.
It could be more precise though. I am wondering who controls the LEDs
through spi-gpio interface.

> +
> +Required properties:
> +- compatible: should be : "brcm,bcm6328-leds-ctrl".

I liked brcm,bcm6328-leds more. It is obvious that all LED subsystem
drivers are for LED *controllers*.

> +- #address-cells: must be 1
> +- #size-cells: must be 0
> +- reg: BCM6328 LED controller address and size.
> +
> +Optional properties:
> +- brcm,serial-leds: boolean which enables Serial LEDs.
> +
> +Each LED is represented as a sub-node of the brcm,bcm6328-leds device.
> +
> +LED sub-node properties:
> +- reg: LED pin number (only LEDs 0 to 23 are valid).
> +
> +Normal LED:

Please give it more meaningful description. Software controlled?

> +LEDs are active high by default.

This should be put next to active-low property, like:

active-low: boolean that makes LED active low.
             Default: false

> +- label (optional): see Documentation/devicetree/bindings/leds/common.txt
> +- active-low (optional): boolean that makes LED active low.
> +- default-state (optional): see
> +  Documentation/devicetree/bindings/leds/leds-gpio.txt
> +- linux,default-trigger (optional): see
> +  Documentation/devicetree/bindings/leds/common.txt
> +
> +Hardware controlled LED:
> +- brcm,hardware-controlled (optional): boolean that makes this LED hardware
> +  controlled.
> +- brcm,link-signal-sources (optional): An array of hardware link
> +  signal sources. Up to four link hardware signals can get muxed into
> +  these LEDs. Only valid for LEDs 0 to 7, where LED signals 0 to 3 may
> +  be muxed to LEDs 0 to 3, and signals 4 to 7 may be muxed to LEDs
> +  4 to 7. A signal can be muxed to more than one LED, and one LED can
> +  have more than one source signal.
> +- brcm,activity-signal-sources (optional): An array of hardware activity
> +  signal sources. Up to four activity hardware signals can get muxed into
> +  these LEDs. Only valid for LEDs 0 to 7, where LED signals 0 to 3 may
> +  be muxed to LEDs 0 to 3, and signals 4 to 7 may be muxed to LEDs
> +  4 to 7. A signal can be muxed to more than one LED, and one LED can
> +  have more than one source signal.

It is more common to group properties in sections - in this case:
"Required properties for child nodes" and "Optional properties for
child nodes".


> +example 1) BCM6328
> +
> +leds0: led-controller@10000800 {
> +	compatible = "brcm,bcm6328-leds-ctrl";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	reg = <0x10000800 0x24>;
> +
> +	alarm_red@2 {
> +		reg = <2>;
> +		active-low;
> +		label = "red:alarm";
> +	};
> +	inet_green@3 {
> +		reg = <3>;
> +		active-low;
> +		label = "green:inet";
> +	};
> +	power_green@4 {
> +		reg = <4>;
> +		active-low;
> +		label = "green:power";
> +		default-state = "on";
> +	};
> +	ephy0_spd@17 {
> +		reg = <17>;
> +		brcm,hardware-controlled;

If there is no LED class device created for hardware controlled LED,
then why does it need the node if has no brcm,link-signal-sources
or brcm,activity-signal-sources properties? There seems to be nothing
to configure then.

> +	};
> +	ephy1_spd@18 {
> +		reg = <18>;
> +		brcm,hardware-controlled;
> +	};
> +	ephy2_spd@19 {
> +		reg = <19>;
> +		brcm,hardware-controlled;
> +	};
> +	ephy3_spd@20 {
> +		reg = <20>;
> +		brcm,hardware-controlled;
> +	};
> +};
> +
> +example 2) BCM63268
> +
> +leds0: led-controller@10001900 {
> +	compatible = "brcm,bcm6328-leds-ctrl";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	reg = <0x10001900 0x24>;
> +	brcm,serial-leds;
> +
> +	gphy0_spd0@0 {
> +		reg = <0>;
> +		brcm,hardware-controlled;
> +		brcm,link-signal-sources = <0>;

More valid examples with brcm,link-signal-sources and
brcm,activity-signal-sources properties would be useful. This is
especially vital taking into account limitations mentioned in the
description of these properties. The examples should be as
comprehensive as possible, i.e. the lists should contain more than
one element.

> +	};
> +	gphy0_spd1@1 {
> +		reg = <1>;
> +		brcm,hardware-controlled;
> +		brcm,link-signal-sources = <1>;
> +	};
> +	inet_red@2 {
> +		reg = <2>;
> +		active-low;
> +		label = "red:inet";
> +	};
> +	dsl_green@3 {
> +		reg = <3>;
> +		active-low;
> +		label = "green:dsl";
> +	};
> +	usb_green@4 {
> +		reg = <4>;
> +		active-low;
> +		label = "green:usb";
> +	};
> +	wps_green@7 {
> +		reg = <7>;
> +		active-low;
> +		label = "green:wps";
> +	};
> +	inet_green@8 {
> +		reg = <8>;
> +		active-low;
> +		label = "green:inet";
> +	};
> +	ephy0_act@9 {
> +		reg = <9>;
> +		brcm,hardware-controlled;
> +	};
> +	ephy1_act@10 {
> +		reg = <10>;
> +		brcm,hardware-controlled;
> +	};
> +	ephy2_act@11 {
> +		reg = <11>;
> +		brcm,hardware-controlled;
> +	};
> +	gphy0_act@12 {
> +		reg = <12>;
> +		brcm,hardware-controlled;
> +	};
> +	ephy0_spd@13 {
> +		reg = <13>;
> +		brcm,hardware-controlled;
> +	};
> +	ephy1_spd@14 {
> +		reg = <14>;
> +		brcm,hardware-controlled;
> +	};
> +	ephy2_spd@15 {
> +		reg = <15>;
> +		brcm,hardware-controlled;
> +	};
> +	power_green@20 {
> +		reg = <20>;
> +		active-low;
> +		label = "green:power";
> +		default-state = "on";
> +	};
> +};
>


-- 
Best Regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] leds: add BCM6328 LED driver
  2015-04-16 14:37     ` Jacek Anaszewski
@ 2015-04-18  9:14       ` Jonas Gorski
  2015-04-20  7:12         ` Jacek Anaszewski
  0 siblings, 1 reply; 26+ messages in thread
From: Jonas Gorski @ 2015-04-18  9:14 UTC (permalink / raw)
  To: Jacek Anaszewski, Álvaro Fernández Rojas
  Cc: linux-leds, cooloney, f.fainelli, cernekee

Hi Jacek,

On 16.04.2015 16:37, Jacek Anaszewski wrote:
> Hi Alvaro,
> 
> On 04/05/2015 05:08 PM, Álvaro Fernández Rojas wrote:

(snip)

>> +static int bcm6328_hwled(struct device *dev, struct device_node *nc, u32 reg,
>> +			 void __iomem *mem, spinlock_t *lock)
>> +{
>> +	int i, cnt;
>> +	unsigned long flags, val;
>> +
>> +	spin_lock_irqsave(lock, flags);
>> +	val = bcm6328_led_read(mem + REG_HWDIS);
>> +	val &= ~BIT(reg);
>> +	bcm6328_led_write(mem + REG_HWDIS, val);
> 
> What is the purpose of REG_HWDIS register? Does it activate the LED or
> so?

The HWDIS register controls whether the led will be controlled by a
hardware signal, with 0 meaning hardware control enabled and 1 hardware
control disabled. This is usually 1:1 for hardware to led signals, but
through the activity/link registers you have some limited control over
rerouting it. Note that even with a bit cleared there is limited control
over the led; you can still let it blink and light it up if it isn't,
but not turn it off if the hardware decided to light it up.

The default is to disable all hardware signals, and only enable them to
be driven by hardware if requested.


Regards
Jonas

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] leds: add BCM6328 LED driver
  2015-04-18  9:14       ` Jonas Gorski
@ 2015-04-20  7:12         ` Jacek Anaszewski
  0 siblings, 0 replies; 26+ messages in thread
From: Jacek Anaszewski @ 2015-04-20  7:12 UTC (permalink / raw)
  To: Jonas Gorski
  Cc: Álvaro Fernández Rojas, linux-leds, cooloney,
	f.fainelli, cernekee

Hi Jonas,

On 04/18/2015 11:14 AM, Jonas Gorski wrote:
> Hi Jacek,
>
> On 16.04.2015 16:37, Jacek Anaszewski wrote:
>> Hi Alvaro,
>>
>> On 04/05/2015 05:08 PM, Álvaro Fernández Rojas wrote:
>
> (snip)
>
>>> +static int bcm6328_hwled(struct device *dev, struct device_node *nc, u32 reg,
>>> +			 void __iomem *mem, spinlock_t *lock)
>>> +{
>>> +	int i, cnt;
>>> +	unsigned long flags, val;
>>> +
>>> +	spin_lock_irqsave(lock, flags);
>>> +	val = bcm6328_led_read(mem + REG_HWDIS);
>>> +	val &= ~BIT(reg);
>>> +	bcm6328_led_write(mem + REG_HWDIS, val);
>>
>> What is the purpose of REG_HWDIS register? Does it activate the LED or
>> so?
>
> The HWDIS register controls whether the led will be controlled by a
> hardware signal, with 0 meaning hardware control enabled and 1 hardware
> control disabled. This is usually 1:1 for hardware to led signals, but
> through the activity/link registers you have some limited control over
> rerouting it. Note that even with a bit cleared there is limited control
> over the led; you can still let it blink and light it up if it isn't,
> but not turn it off if the hardware decided to light it up.
>
> The default is to disable all hardware signals, and only enable them to
> be driven by hardware if requested.

Thanks for the explanation. In the associated DT documentation there are
provided examples with brcm,hardware-controlled property, but no related
brcm,link-signal-sources and brcm,activity-signal-sources properties.
What is the default routing then? It should be mentioned in the
documentation as well.

-- 
Best Regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v3 0/2] BCM6328 LED driver
  2015-04-05 15:08 ` [PATCH v2 0/2] " Álvaro Fernández Rojas
  2015-04-05 15:08   ` [PATCH v2 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
  2015-04-05 15:08   ` [PATCH 2/2] leds: add BCM6328 LED driver Álvaro Fernández Rojas
@ 2015-04-24 17:06   ` Álvaro Fernández Rojas
  2015-04-24 17:06     ` [PATCH v3 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
                       ` (2 more replies)
  2 siblings, 3 replies; 26+ messages in thread
From: Álvaro Fernández Rojas @ 2015-04-24 17:06 UTC (permalink / raw)
  To: linux-leds, cooloney, jogo, f.fainelli, cernekee, j.anaszewski
  Cc: Álvaro Fernández Rojas

These patches add support and documentation for the BCM6328 LED driver, which is present on BCM6318, BCM6328, BCM6362 and BCM63268.
In these SoCs it's possible to control LEDs both as GPIOs or by hardware.
However, on some devices there are Serial LEDs (LEDs connected to a 74hc controller), which can either be controlled by software (exporting the 74hc as spi-gpio) or hardware using this driver.
The problem is some of these serial LEDs are hardware controlled (ethernet LEDs) and exporting the 74hc as spi-gpio prevents those LEDs to be hardware controlled, so the only chance to keep them working is by using this driver.

v2->v3: Introduce changes suggested by Jacek.
v1->v2: Introduce changes suggested by Florian and Jonas.

Álvaro Fernández Rojas (2):
  leds: add DT binding for BCM6328 LED controller
  leds: add BCM6328 LED driver

 .../devicetree/bindings/leds/leds-bcm6328.txt      | 311 ++++++++++++++++
 drivers/leds/Kconfig                               |   8 +
 drivers/leds/Makefile                              |   1 +
 drivers/leds/leds-bcm6328.c                        | 413 +++++++++++++++++++++
 4 files changed, 733 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-bcm6328.txt
 create mode 100644 drivers/leds/leds-bcm6328.c

-- 
1.9.1

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v3 1/2] leds: add DT binding for BCM6328 LED controller
  2015-04-24 17:06   ` [PATCH v3 0/2] " Álvaro Fernández Rojas
@ 2015-04-24 17:06     ` Álvaro Fernández Rojas
  2015-04-26 11:09       ` Jacek Anaszewski
  2015-04-24 17:06     ` [PATCH v3 2/2] leds: add BCM6328 LED driver Álvaro Fernández Rojas
  2015-04-26 12:15     ` [PATCH v4 0/2] " Álvaro Fernández Rojas
  2 siblings, 1 reply; 26+ messages in thread
From: Álvaro Fernández Rojas @ 2015-04-24 17:06 UTC (permalink / raw)
  To: linux-leds, cooloney, jogo, f.fainelli, cernekee, j.anaszewski
  Cc: Álvaro Fernández Rojas

This adds device tree binding documentation for the Broadcom BCM6328 LED controller.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Signed-off-by: Jonas Gorski <jogo@openwrt.org>
---
 v3: introduce changes suggested by Jacek
 - Revert compatible string to "brcm,bcm6328-leds".
 - Improved description.
 - Renamed normal LEDs to hardware controlled.
 - Explain that LEDs are active high by default on active-low optional property.
 - Properties are now grouped depending on the type of LED and optional requirement.
 - Added more examples to explain hardware controlled LEDs and activity/link selection.
 v2: Introduce changes suggested by Florian and Jonas.
 - Change compatible string to "brcm,bcm6328-leds-ctrl".
 - Make valid LEDs statement more strict.
 - Remove compatible strings from LED subnodes.
 - Clarify that LEDs are active high by default.
 - Add a better description for brcm,link-signal-sources and brcm,activity-signal-sources.

 .../devicetree/bindings/leds/leds-bcm6328.txt      | 311 +++++++++++++++++++++
 1 file changed, 311 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-bcm6328.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-bcm6328.txt b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
new file mode 100644
index 0000000..6d838f8
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
@@ -0,0 +1,311 @@
+LEDs connected to Broadcom BCM6328 controller
+
+This controller is present on BCM6318, BCM6328, BCM6362 and BCM63268.
+In these SoCs it's possible to control LEDs both as GPIOs or by hardware.
+However, on some devices there are Serial LEDs (LEDs connected to a 74x164
+controller), which can either be controlled by software (exporting the 74x164
+as spi-gpio. See Documentation/devicetree/bindings/gpio/gpio-74x164.txt), or
+by hardware using this driver.
+Some of these Serial LEDs are hardware controlled (e.g. ethernet LEDs) and
+exporting the 74x164 as spi-gpio prevents those LEDs to be hardware
+controlled, so the only chance to keep them working is by using this driver.
+
+BCM6328 LED controller has a HWDIS register, which controls whether a LED
+should be controlled by a hardware signal instead of the MODE register value,
+with 0 meaning hardware control enabled and 1 hardware control disabled. This
+is usually 1:1 for hardware to LED signals, but through the activity/link
+registers you have some limited control over rerouting the LEDs (as
+explained later in brcm,link-signal-sources). Even if a LED is hardware
+controlled you are still able to make it blink or light it up if it isn't,
+but you can't turn it off if the hardware decides to light it up. For this
+reason, hardware controlled LEDs aren't registered as LED class devices.
+
+Required properties:
+- compatible: should be : "brcm,bcm6328-leds".
+- #address-cells: must be 1
+- #size-cells: must be 0
+- reg: BCM6328 LED controller address and size.
+
+Optional properties:
+- brcm,serial-leds: boolean which enables Serial LEDs.
+
+Each LED is represented as a sub-node of the brcm,bcm6328-leds device.
+
+LED sub-node required properties:
+- reg: LED pin number (only LEDs 0 to 23 are valid).
+
+Software controlled LED optional properties:
+- label (optional): see Documentation/devicetree/bindings/leds/common.txt
+- active-low (optional): boolean that makes LED active low.
+  Default: false
+- default-state (optional): see
+  Documentation/devicetree/bindings/leds/leds-gpio.txt
+- linux,default-trigger (optional): see
+  Documentation/devicetree/bindings/leds/common.txt
+
+Hardware controlled LED optional properties:
+- brcm,hardware-controlled: boolean that makes this LED hardware
+  controlled.
+- brcm,link-signal-sources: An array of hardware link
+  signal sources. Up to four link hardware signals can get muxed into
+  these LEDs. Only valid for LEDs 0 to 7, where LED signals 0 to 3 may
+  be muxed to LEDs 0 to 3, and signals 4 to 7 may be muxed to LEDs
+  4 to 7. A signal can be muxed to more than one LED, and one LED can
+  have more than one source signal.
+- brcm,activity-signal-sources: An array of hardware activity
+  signal sources. Up to four activity hardware signals can get muxed into
+  these LEDs. Only valid for LEDs 0 to 7, where LED signals 0 to 3 may
+  be muxed to LEDs 0 to 3, and signals 4 to 7 may be muxed to LEDs
+  4 to 7. A signal can be muxed to more than one LED, and one LED can
+  have more than one source signal.
+
+example 1) BCM6328 with 4 EPHY LEDs
+
+leds0: led-controller@10000800 {
+	compatible = "brcm,bcm6328-leds";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reg = <0x10000800 0x24>;
+
+	alarm_red@2 {
+		reg = <2>;
+		active-low;
+		label = "red:alarm";
+	};
+	inet_green@3 {
+		reg = <3>;
+		active-low;
+		label = "green:inet";
+	};
+	power_green@4 {
+		reg = <4>;
+		active-low;
+		label = "green:power";
+		default-state = "on";
+	};
+	ephy0_spd@17 {
+		reg = <17>;
+		brcm,hardware-controlled;
+	};
+	ephy1_spd@18 {
+		reg = <18>;
+		brcm,hardware-controlled;
+	};
+	ephy2_spd@19 {
+		reg = <19>;
+		brcm,hardware-controlled;
+	};
+	ephy3_spd@20 {
+		reg = <20>;
+		brcm,hardware-controlled;
+	};
+};
+
+example 2) BCM63268 with Serial/GPHY0 LEDs
+
+leds0: led-controller@10001900 {
+	compatible = "brcm,bcm6328-leds";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reg = <0x10001900 0x24>;
+	brcm,serial-leds;
+
+	gphy0_spd0@0 {
+		reg = <0>;
+		brcm,hardware-controlled;
+		brcm,link-signal-sources = <0>;
+	};
+	gphy0_spd1@1 {
+		reg = <1>;
+		brcm,hardware-controlled;
+		brcm,link-signal-sources = <1>;
+	};
+	inet_red@2 {
+		reg = <2>;
+		active-low;
+		label = "red:inet";
+	};
+	dsl_green@3 {
+		reg = <3>;
+		active-low;
+		label = "green:dsl";
+	};
+	usb_green@4 {
+		reg = <4>;
+		active-low;
+		label = "green:usb";
+	};
+	wps_green@7 {
+		reg = <7>;
+		active-low;
+		label = "green:wps";
+	};
+	inet_green@8 {
+		reg = <8>;
+		active-low;
+		label = "green:inet";
+	};
+	ephy0_act@9 {
+		reg = <9>;
+		brcm,hardware-controlled;
+	};
+	ephy1_act@10 {
+		reg = <10>;
+		brcm,hardware-controlled;
+	};
+	ephy2_act@11 {
+		reg = <11>;
+		brcm,hardware-controlled;
+	};
+	gphy0_act@12 {
+		reg = <12>;
+		brcm,hardware-controlled;
+	};
+	ephy0_spd@13 {
+		reg = <13>;
+		brcm,hardware-controlled;
+	};
+	ephy1_spd@14 {
+		reg = <14>;
+		brcm,hardware-controlled;
+	};
+	ephy2_spd@15 {
+		reg = <15>;
+		brcm,hardware-controlled;
+	};
+	power_green@20 {
+		reg = <20>;
+		active-low;
+		label = "green:power";
+		default-state = "on";
+	};
+};
+
+example 3) BCM6362 with 1 LED for each EPHY
+
+leds0: led-controller@10001900 {
+	compatible = "brcm,bcm6328-leds";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reg = <0x10001900 0x24>;
+
+	usb@0 {
+		reg = <0>;
+		brcm,hardware-controlled;
+		brcm,link-signal-sources = <0>;
+		brcm,activity-signal-sources = <0>;
+		/* USB link/activity routed to USB LED */
+	};
+	inet@1 {
+		reg = <1>;
+		brcm,hardware-controlled;
+		brcm,activity-signal-sources = <1>;
+		/* INET activity routed to INET LED */
+	};
+	ephy0@4 {
+		reg = <4>;
+		brcm,hardware-controlled;
+		brcm,link-signal-sources = <4>;
+		/* EPHY0 link routed to EPHY0 LED */
+	};
+	ephy1@5 {
+		reg = <5>;
+		brcm,hardware-controlled;
+		brcm,link-signal-sources = <5>;
+		/* EPHY1 link routed to EPHY1 LED */
+	};
+	ephy2@6 {
+		reg = <6>;
+		brcm,hardware-controlled;
+		brcm,link-signal-sources = <6>;
+		/* EPHY2 link routed to EPHY2 LED */
+	};
+	ephy3@7 {
+		reg = <7>;
+		brcm,hardware-controlled;
+		brcm,link-signal-sources = <7>;
+		/* EPHY3 link routed to EPHY3 LED */
+	};
+	power_green@20 {
+		reg = <20>;
+		active-low;
+		label = "green:power";
+		default-state = "on";
+	};
+};
+
+example 4) BCM6362 with 1 LED for all EPHYs
+
+leds0: led-controller@10001900 {
+	compatible = "brcm,bcm6328-leds";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reg = <0x10001900 0x24>;
+
+	usb@0 {
+		reg = <0>;
+		brcm,hardware-controlled;
+		brcm,link-signal-sources = <0 1>;
+		brcm,activity-signal-sources = <0 1>;
+		/* USB/INET link/activity routed to USB LED */
+	};
+	ephy@4 {
+		reg = <4>;
+		brcm,hardware-controlled;
+		brcm,link-signal-sources = <4 5 6 7>;
+		/* EPHY0/1/2/3 link routed to EPHY0 LED */
+	};
+	power_green@20 {
+		reg = <20>;
+		active-low;
+		label = "green:power";
+		default-state = "on";
+	};
+};
+
+example 5) BCM6362 with EPHY LEDs swapped
+
+leds0: led-controller@10001900 {
+	compatible = "brcm,bcm6328-leds";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reg = <0x10001900 0x24>;
+
+	usb@0 {
+		reg = <0>;
+		brcm,hardware-controlled;
+		brcm,link-signal-sources = <0>;
+		brcm,activity-signal-sources = <0 1>;
+		/* USB link/activity & INET activity routed to USB LED */
+	};
+	ephy0@4 {
+		reg = <4>;
+		brcm,hardware-controlled;
+		brcm,link-signal-sources = <7>;
+		/* EPHY3 link routed to EPHY0 LED */
+	};
+	ephy1@5 {
+		reg = <5>;
+		brcm,hardware-controlled;
+		brcm,link-signal-sources = <6>;
+		/* EPHY2 link routed to EPHY1 LED */
+	};
+	ephy2@6 {
+		reg = <6>;
+		brcm,hardware-controlled;
+		brcm,link-signal-sources = <5>;
+		/* EPHY1 link routed to EPHY2 LED */
+	};
+	ephy3@7 {
+		reg = <7>;
+		brcm,hardware-controlled;
+		brcm,link-signal-sources = <4>;
+		/* EPHY0 link routed to EPHY3 LED */
+	};
+	power_green@20 {
+		reg = <20>;
+		active-low;
+		label = "green:power";
+		default-state = "on";
+	};
+};
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v3 2/2] leds: add BCM6328 LED driver
  2015-04-24 17:06   ` [PATCH v3 0/2] " Álvaro Fernández Rojas
  2015-04-24 17:06     ` [PATCH v3 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
@ 2015-04-24 17:06     ` Álvaro Fernández Rojas
  2015-04-26 12:15     ` [PATCH v4 0/2] " Álvaro Fernández Rojas
  2 siblings, 0 replies; 26+ messages in thread
From: Álvaro Fernández Rojas @ 2015-04-24 17:06 UTC (permalink / raw)
  To: linux-leds, cooloney, jogo, f.fainelli, cernekee, j.anaszewski
  Cc: Álvaro Fernández Rojas

This adds support for the LED controller on Broadcom's BCM6328.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Signed-off-by: Jonas Gorski <jogo@openwrt.org>
---
 v3: introduce changes suggested by Jacek
 - Revert compatible string to "brcm,bcm6328-leds".
 - Add BCM6328_ prefix to macros.
 - Rename blink_del to blink_delay.
 v2: introduce changes suggested by Florian and Jonas.
 - Improve Kconfig description.
 - Remove unused imports.
 - Fix bug in blink_set so it works for multiple LEDs.
 - "brcm,link-selection" -> "brcm,link-signal-sources".
 - "brcm,activity-selection" -> "brcm,activity-signal-sources".
 - Use of_property_read_bool instead of of_get_property for booleans.
 - Add MODULE_AUTHOR strings.

 drivers/leds/Kconfig        |   8 +
 drivers/leds/Makefile       |   1 +
 drivers/leds/leds-bcm6328.c | 413 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 422 insertions(+)
 create mode 100644 drivers/leds/leds-bcm6328.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 966b960..caa8ee6 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -42,6 +42,14 @@ config LEDS_88PM860X
 	  This option enables support for on-chip LED drivers found on Marvell
 	  Semiconductor 88PM8606 PMIC.
 
+config LEDS_BCM6328
+	tristate "LED Support for Broadcom BCM6328"
+	depends on LEDS_CLASS
+	depends on OF
+	help
+	  This option enables support for LEDs connected to the BCM6328
+	  LED HW controller accessed via MMIO registers.
+
 config LEDS_LM3530
 	tristate "LCD Backlight driver for LM3530"
 	depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index bf46093..309a46b 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_LEDS_TRIGGERS)		+= led-triggers.o
 
 # LED Platform Drivers
 obj-$(CONFIG_LEDS_88PM860X)		+= leds-88pm860x.o
+obj-$(CONFIG_LEDS_BCM6328)		+= leds-bcm6328.o
 obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
 obj-$(CONFIG_LEDS_LOCOMO)		+= leds-locomo.o
 obj-$(CONFIG_LEDS_LM3530)		+= leds-lm3530.o
diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
new file mode 100644
index 0000000..d2bcbc3
--- /dev/null
+++ b/drivers/leds/leds-bcm6328.c
@@ -0,0 +1,413 @@
+/*
+ * Driver for BCM6328 memory-mapped LEDs, based on leds-syscon.c
+ *
+ * Copyright 2015 Álvaro Fernández Rojas <noltari@gmail.com>
+ * Copyright 2015 Jonas Gorski <jogo@openwrt.org>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+#include <linux/io.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+
+#define BCM6328_REG_INIT		0x00
+#define BCM6328_REG_MODE_HI		0x04
+#define BCM6328_REG_MODE_LO		0x08
+#define BCM6328_REG_HWDIS		0x0c
+#define BCM6328_REG_STROBE		0x10
+#define BCM6328_REG_LNKACTSEL_HI	0x14
+#define BCM6328_REG_LNKACTSEL_LO	0x18
+#define BCM6328_REG_RBACK		0x1c
+#define BCM6328_REG_SERMUX		0x20
+
+#define BCM6328_LED_MAX_COUNT		24
+#define BCM6328_LED_DEF_DELAY		500
+#define BCM6328_LED_INTERVAL_MS		20
+
+#define BCM6328_LED_INTV_MASK		0x3f
+#define BCM6328_LED_FAST_INTV_SHIFT	6
+#define BCM6328_LED_FAST_INTV_MASK	(BCM6328_LED_INTV_MASK << \
+					 BCM6328_LED_FAST_INTV_SHIFT)
+#define BCM6328_SERIAL_LED_EN		BIT(12)
+#define BCM6328_SERIAL_LED_MUX		BIT(13)
+#define BCM6328_SERIAL_LED_CLK_NPOL	BIT(14)
+#define BCM6328_SERIAL_LED_DATA_PPOL	BIT(15)
+#define BCM6328_SERIAL_LED_SHIFT_DIR	BIT(16)
+#define BCM6328_LED_SHIFT_TEST		BIT(30)
+#define BCM6328_LED_TEST		BIT(31)
+
+#define BCM6328_LED_MODE_MASK		3
+#define BCM6328_LED_MODE_OFF		0
+#define BCM6328_LED_MODE_FAST		1
+#define BCM6328_LED_MODE_BLINK		2
+#define BCM6328_LED_MODE_ON		3
+#define BCM6328_LED_SHIFT(X)		((X) << 1)
+
+/**
+ * struct bcm6328_led - state container for bcm6328 based LEDs
+ * @cdev: LED class device for this LED
+ * @mem: memory resource
+ * @lock: memory lock
+ * @pin: LED pin number
+ * @blink_leds: blinking LEDs
+ * @blink_delay: blinking delay
+ * @active_low: LED is active low
+ */
+struct bcm6328_led {
+	struct led_classdev cdev;
+	void __iomem *mem;
+	spinlock_t *lock;
+	unsigned long pin;
+	unsigned long *blink_leds;
+	unsigned long *blink_delay;
+	bool active_low;
+};
+
+static void bcm6328_led_write(void __iomem *reg, unsigned long data)
+{
+	iowrite32be(data, reg);
+}
+
+static unsigned long bcm6328_led_read(void __iomem *reg)
+{
+	return ioread32be(reg);
+}
+
+/**
+ * LEDMode 64 bits / 24 LEDs
+ * bits [31:0] -> LEDs 8-23
+ * bits [47:32] -> LEDs 0-7
+ * bits [63:48] -> unused
+ */
+static unsigned long bcm6328_pin2shift(unsigned long pin)
+{
+	if (pin < 8)
+		return pin + 16; /* LEDs 0-7 (bits 47:32) */
+	else
+		return pin - 8; /* LEDs 8-23 (bits 31:0) */
+}
+
+static void bcm6328_led_mode(struct bcm6328_led *led, unsigned long value)
+{
+	void __iomem *mode;
+	unsigned long val, shift;
+
+	shift = bcm6328_pin2shift(led->pin);
+	if (shift / 16)
+		mode = led->mem + BCM6328_REG_MODE_HI;
+	else
+		mode = led->mem + BCM6328_REG_MODE_LO;
+
+	val = bcm6328_led_read(mode);
+	val &= ~(BCM6328_LED_MODE_MASK << BCM6328_LED_SHIFT(shift % 16));
+	val |= (value << BCM6328_LED_SHIFT(shift % 16));
+	bcm6328_led_write(mode, val);
+}
+
+static void bcm6328_led_set(struct led_classdev *led_cdev,
+			    enum led_brightness value)
+{
+	struct bcm6328_led *led =
+		container_of(led_cdev, struct bcm6328_led, cdev);
+	unsigned long flags;
+
+	spin_lock_irqsave(led->lock, flags);
+	*(led->blink_leds) &= ~BIT(led->pin);
+	if ((led->active_low && value == LED_OFF) ||
+	    (!led->active_low && value != LED_OFF))
+		bcm6328_led_mode(led, BCM6328_LED_MODE_OFF);
+	else
+		bcm6328_led_mode(led, BCM6328_LED_MODE_ON);
+	spin_unlock_irqrestore(led->lock, flags);
+}
+
+static int bcm6328_blink_set(struct led_classdev *led_cdev,
+			     unsigned long *delay_on, unsigned long *delay_off)
+{
+	struct bcm6328_led *led =
+		container_of(led_cdev, struct bcm6328_led, cdev);
+	unsigned long delay, flags;
+
+	if (!*delay_on)
+		*delay_on = BCM6328_LED_DEF_DELAY;
+	if (!*delay_off)
+		*delay_off = BCM6328_LED_DEF_DELAY;
+
+	if (*delay_on != *delay_off) {
+		dev_dbg(led_cdev->dev,
+			"fallback to soft blinking (delay_on != delay_off)\n");
+		return -EINVAL;
+	}
+
+	delay = *delay_on / BCM6328_LED_INTERVAL_MS;
+	if (delay == 0)
+		delay = 1;
+	else if (delay > BCM6328_LED_INTV_MASK) {
+		dev_dbg(led_cdev->dev,
+			"fallback to soft blinking (delay > %ums)\n",
+			BCM6328_LED_INTV_MASK * BCM6328_LED_INTERVAL_MS);
+		return -EINVAL;
+	}
+
+	spin_lock_irqsave(led->lock, flags);
+	if (*(led->blink_leds) == 0 ||
+	    *(led->blink_leds) == BIT(led->pin) ||
+	    *(led->blink_delay) == delay) {
+		unsigned long val;
+
+		*(led->blink_leds) |= BIT(led->pin);
+		*(led->blink_delay) = delay;
+
+		val = bcm6328_led_read(led->mem + BCM6328_REG_INIT);
+		val &= ~BCM6328_LED_FAST_INTV_MASK;
+		val |= (delay << BCM6328_LED_FAST_INTV_SHIFT);
+		bcm6328_led_write(led->mem + BCM6328_REG_INIT, val);
+
+		bcm6328_led_mode(led, BCM6328_LED_MODE_BLINK);
+
+		spin_unlock_irqrestore(led->lock, flags);
+	} else {
+		spin_unlock_irqrestore(led->lock, flags);
+		dev_dbg(led_cdev->dev,
+			"fallback to soft blinking (delay already set)\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int bcm6328_hwled(struct device *dev, struct device_node *nc, u32 reg,
+			 void __iomem *mem, spinlock_t *lock)
+{
+	int i, cnt;
+	unsigned long flags, val;
+
+	spin_lock_irqsave(lock, flags);
+	val = bcm6328_led_read(mem + BCM6328_REG_HWDIS);
+	val &= ~BIT(reg);
+	bcm6328_led_write(mem + BCM6328_REG_HWDIS, val);
+	spin_unlock_irqrestore(lock, flags);
+
+	/* Only LEDs 0-7 can be activity/link controlled */
+	if (reg >= 8)
+		return 0;
+
+	cnt = of_property_count_elems_of_size(nc, "brcm,link-signal-sources",
+					      sizeof(u32));
+	for (i = 0; i < cnt; i++) {
+		u32 sel;
+		void __iomem *addr;
+
+		if (reg < 4)
+			addr = mem + BCM6328_REG_LNKACTSEL_LO;
+		else
+			addr = mem + BCM6328_REG_LNKACTSEL_HI;
+
+		of_property_read_u32_index(nc, "brcm,link-signal-sources", i,
+					   &sel);
+
+		if (reg / 4 != sel / 4) {
+			dev_warn(dev, "invalid link signal source\n");
+			continue;
+		}
+
+		spin_lock_irqsave(lock, flags);
+		val = bcm6328_led_read(addr);
+		val |= (BIT(reg) << (((sel % 4) * 4) + 16));
+		bcm6328_led_write(addr, val);
+		spin_unlock_irqrestore(lock, flags);
+	}
+
+	cnt = of_property_count_elems_of_size(nc,
+					      "brcm,activity-signal-sources",
+					      sizeof(u32));
+	for (i = 0; i < cnt; i++) {
+		u32 sel;
+		void __iomem *addr;
+
+		if (reg < 4)
+			addr = mem + BCM6328_REG_LNKACTSEL_LO;
+		else
+			addr = mem + BCM6328_REG_LNKACTSEL_HI;
+
+		of_property_read_u32_index(nc, "brcm,activity-signal-sources",
+					   i, &sel);
+
+		if (reg / 4 != sel / 4) {
+			dev_warn(dev, "invalid activity signal source\n");
+			continue;
+		}
+
+		spin_lock_irqsave(lock, flags);
+		val = bcm6328_led_read(addr);
+		val |= (BIT(reg) << ((sel % 4) * 4));
+		bcm6328_led_write(addr, val);
+		spin_unlock_irqrestore(lock, flags);
+	}
+
+	return 0;
+}
+
+static int bcm6328_led(struct device *dev, struct device_node *nc, u32 reg,
+		       void __iomem *mem, spinlock_t *lock,
+		       unsigned long *blink_leds, unsigned long *blink_delay)
+{
+	struct bcm6328_led *led;
+	unsigned long flags;
+	const char *state;
+	int rc;
+
+	led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+
+	led->pin = reg;
+	led->mem = mem;
+	led->lock = lock;
+	led->blink_leds = blink_leds;
+	led->blink_delay = blink_delay;
+
+	if (of_property_read_bool(nc, "active-low"))
+		led->active_low = 1;
+
+	led->cdev.name = of_get_property(nc, "label", NULL) ? : nc->name;
+	led->cdev.default_trigger = of_get_property(nc,
+						    "linux,default-trigger",
+						    NULL);
+
+	if (!of_property_read_string(nc, "default-state", &state)) {
+		spin_lock_irqsave(lock, flags);
+		if (!strcmp(state, "on")) {
+			led->cdev.brightness = LED_FULL;
+			bcm6328_led_mode(led, BCM6328_LED_MODE_ON);
+		} else if (!strcmp(state, "keep")) {
+			void __iomem *mode;
+			unsigned long val, shift;
+
+			shift = bcm6328_pin2shift(led->pin);
+			if (shift / 16)
+				mode = mem + BCM6328_REG_MODE_HI;
+			else
+				mode = mem + BCM6328_REG_MODE_LO;
+
+			val = bcm6328_led_read(mode) >> (shift % 16);
+			val &= BCM6328_LED_MODE_MASK;
+			if (val == BCM6328_LED_MODE_ON)
+				led->cdev.brightness = LED_FULL;
+			else {
+				led->cdev.brightness = LED_OFF;
+				bcm6328_led_mode(led, BCM6328_LED_MODE_OFF);
+			}
+		} else {
+			led->cdev.brightness = LED_OFF;
+			bcm6328_led_mode(led, BCM6328_LED_MODE_OFF);
+		}
+		spin_unlock_irqrestore(lock, flags);
+	}
+
+	led->cdev.brightness_set = bcm6328_led_set;
+	led->cdev.blink_set = bcm6328_blink_set;
+
+	rc = led_classdev_register(dev, &led->cdev);
+	if (rc < 0)
+		return rc;
+
+	dev_dbg(dev, "registered LED %s\n", led->cdev.name);
+
+	return 0;
+}
+
+static int bcm6328_leds_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *child;
+	struct resource *mem_r;
+	void __iomem *mem;
+	spinlock_t *lock;
+	unsigned long val, *blink_leds, *blink_delay;
+
+	mem_r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mem_r)
+		return -EINVAL;
+
+	mem = devm_ioremap_resource(dev, mem_r);
+	if (IS_ERR(mem))
+		return PTR_ERR(mem);
+
+	lock = devm_kzalloc(dev, sizeof(*lock), GFP_KERNEL);
+	if (!lock)
+		return -ENOMEM;
+
+	blink_leds = devm_kzalloc(dev, sizeof(*blink_leds), GFP_KERNEL);
+	if (!blink_leds)
+		return -ENOMEM;
+
+	blink_delay = devm_kzalloc(dev, sizeof(*blink_delay), GFP_KERNEL);
+	if (!blink_delay)
+		return -ENOMEM;
+
+	spin_lock_init(lock);
+
+	bcm6328_led_write(mem + BCM6328_REG_HWDIS, ~0);
+	bcm6328_led_write(mem + BCM6328_REG_LNKACTSEL_HI, 0);
+	bcm6328_led_write(mem + BCM6328_REG_LNKACTSEL_LO, 0);
+
+	val = bcm6328_led_read(mem + BCM6328_REG_INIT);
+	val &= ~BCM6328_SERIAL_LED_EN;
+	if (of_property_read_bool(np, "brcm,serial-leds"))
+		val |= BCM6328_SERIAL_LED_EN;
+	bcm6328_led_write(mem + BCM6328_REG_INIT, val);
+
+	for_each_available_child_of_node(np, child) {
+		int rc;
+		u32 reg;
+
+		if (of_property_read_u32(child, "reg", &reg))
+			continue;
+
+		if (reg >= BCM6328_LED_MAX_COUNT) {
+			dev_err(dev, "invalid LED (>= %d)\n",
+				BCM6328_LED_MAX_COUNT);
+			continue;
+		}
+
+		if (of_property_read_bool(child, "brcm,hardware-controlled"))
+			rc = bcm6328_hwled(dev, child, reg, mem, lock);
+		else
+			rc = bcm6328_led(dev, child, reg, mem, lock,
+					 blink_leds, blink_delay);
+
+		if (rc < 0)
+			return rc;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id bcm6328_leds_of_match[] = {
+	{ .compatible = "brcm,bcm6328-leds", },
+	{ },
+};
+
+static struct platform_driver bcm6328_leds_driver = {
+	.probe = bcm6328_leds_probe,
+	.driver = {
+		.name = "leds-bcm6328",
+		.of_match_table = bcm6328_leds_of_match,
+	},
+};
+
+module_platform_driver(bcm6328_leds_driver);
+
+MODULE_AUTHOR("Álvaro Fernández Rojas <noltari@gmail.com>");
+MODULE_AUTHOR("Jonas Gorski <jogo@openwrt.org>");
+MODULE_DESCRIPTION("LED driver for BCM6328 controllers");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:leds-bcm6328");
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 1/2] leds: add DT binding for BCM6328 LED controller
  2015-04-24 17:06     ` [PATCH v3 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
@ 2015-04-26 11:09       ` Jacek Anaszewski
  0 siblings, 0 replies; 26+ messages in thread
From: Jacek Anaszewski @ 2015-04-26 11:09 UTC (permalink / raw)
  To: Álvaro Fernández Rojas, linux-leds, cooloney, jogo,
	f.fainelli, cernekee, j.anaszewski, devicetree

Hi Alvaro,

Thanks for the update. Since this patch will require also DT ack,
please add devicetree@vger.kernel.org on cc as you'll be posting
next version

On Fri, 24 Apr 2015 19:06:15 +0200
Álvaro Fernández Rojas  <noltari@gmail.com> wrote:

> This adds device tree binding documentation for the Broadcom BCM6328 LED controller.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> Signed-off-by: Jonas Gorski <jogo@openwrt.org>
> ---
>  v3: introduce changes suggested by Jacek
>  - Revert compatible string to "brcm,bcm6328-leds".
>  - Improved description.
>  - Renamed normal LEDs to hardware controlled.
>  - Explain that LEDs are active high by default on active-low optional property.
>  - Properties are now grouped depending on the type of LED and optional requirement.
>  - Added more examples to explain hardware controlled LEDs and activity/link selection.
>  v2: Introduce changes suggested by Florian and Jonas.
>  - Change compatible string to "brcm,bcm6328-leds-ctrl".
>  - Make valid LEDs statement more strict.
>  - Remove compatible strings from LED subnodes.
>  - Clarify that LEDs are active high by default.
>  - Add a better description for brcm,link-signal-sources and brcm,activity-signal-sources.
> 
>  .../devicetree/bindings/leds/leds-bcm6328.txt      | 311 +++++++++++++++++++++
>  1 file changed, 311 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-bcm6328.txt
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-bcm6328.txt b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
> new file mode 100644
> index 0000000..6d838f8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
> @@ -0,0 +1,311 @@
> +LEDs connected to Broadcom BCM6328 controller
> +
> +This controller is present on BCM6318, BCM6328, BCM6362 and BCM63268.
> +In these SoCs it's possible to control LEDs both as GPIOs or by hardware.
> +However, on some devices there are Serial LEDs (LEDs connected to a 74x164
> +controller), which can either be controlled by software (exporting the 74x164
> +as spi-gpio. See Documentation/devicetree/bindings/gpio/gpio-74x164.txt), or
> +by hardware using this driver.
> +Some of these Serial LEDs are hardware controlled (e.g. ethernet LEDs) and
> +exporting the 74x164 as spi-gpio prevents those LEDs to be hardware
> +controlled, so the only chance to keep them working is by using this driver.
> +
> +BCM6328 LED controller has a HWDIS register, which controls whether a LED
> +should be controlled by a hardware signal instead of the MODE register value,
> +with 0 meaning hardware control enabled and 1 hardware control disabled. This
> +is usually 1:1 for hardware to LED signals, but through the activity/link
> +registers you have some limited control over rerouting the LEDs (as
> +explained later in brcm,link-signal-sources). Even if a LED is hardware
> +controlled you are still able to make it blink or light it up if it isn't,
> +but you can't turn it off if the hardware decides to light it up. For this
> +reason, hardware controlled LEDs aren't registered as LED class devices.
> +
> +Required properties:
> +- compatible: should be : "brcm,bcm6328-leds".

Please drop the colon after "should be".
Also it is more common to insert a space before the colon character.
e.g.:

compatible : should be ...

Please apply this rule to each property.

> +- #address-cells: must be 1
> +- #size-cells: must be 0
> +- reg: BCM6328 LED controller address and size.
>
> +
> +Optional properties:
> +- brcm,serial-leds: boolean which enables Serial LEDs.

How about:

brcm,serial-leds: Boolean, enables Serial LEDs.

Boolean should begin with capital letter. Please refer to the other
bindings in Documentation/devicetree/bindings to keep the style of
DT documentation consistent.
Also default value should be mentioned.

> +Each LED is represented as a sub-node of the brcm,bcm6328-leds device.
> +
> +LED sub-node required properties:

How about:

Required properties for sub-nodes:

> +- reg: LED pin number (only LEDs 0 to 23 are valid).
> +
> +Software controlled LED optional properties:

How about:

Optional properties for sub-nodes related to software controlled LEDs:

> +- label (optional): see Documentation/devicetree/bindings/leds/common.txt
> +- active-low (optional): boolean that makes LED active low.
> +  Default: false
> +- default-state (optional): see
> +  Documentation/devicetree/bindings/leds/leds-gpio.txt
> +- linux,default-trigger (optional): see
> +  Documentation/devicetree/bindings/leds/common.txt

Please drop "(optional)" as you are already mentioned this in the
section header.

> +Hardware controlled LED optional properties:

How about:

Optional properties for sub-nodes related to hardware controlled LEDs:

> +- brcm,hardware-controlled: boolean that makes this LED hardware
> +  controlled.
> +- brcm,link-signal-sources: An array of hardware link
> +  signal sources. Up to four link hardware signals can get muxed into
> +  these LEDs. Only valid for LEDs 0 to 7, where LED signals 0 to 3 may
> +  be muxed to LEDs 0 to 3, and signals 4 to 7 may be muxed to LEDs
> +  4 to 7. A signal can be muxed to more than one LED, and one LED can
> +  have more than one source signal.
> +- brcm,activity-signal-sources: An array of hardware activity
> +  signal sources. Up to four activity hardware signals can get muxed into
> +  these LEDs. Only valid for LEDs 0 to 7, where LED signals 0 to 3 may
> +  be muxed to LEDs 0 to 3, and signals 4 to 7 may be muxed to LEDs
> +  4 to 7. A signal can be muxed to more than one LED, and one LED can
> +  have more than one source signal.
> +
> +example 1) BCM6328 with 4 EPHY LEDs
> +
> +leds0: led-controller@10000800 {
> +	compatible = "brcm,bcm6328-leds";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	reg = <0x10000800 0x24>;
> +
> +	alarm_red@2 {
> +		reg = <2>;
> +		active-low;
> +		label = "red:alarm";
> +	};
> +	inet_green@3 {
> +		reg = <3>;
> +		active-low;
> +		label = "green:inet";
> +	};
> +	power_green@4 {
> +		reg = <4>;
> +		active-low;
> +		label = "green:power";
> +		default-state = "on";
> +	};
> +	ephy0_spd@17 {
> +		reg = <17>;
> +		brcm,hardware-controlled;
> +	};
> +	ephy1_spd@18 {
> +		reg = <18>;
> +		brcm,hardware-controlled;
> +	};
> +	ephy2_spd@19 {
> +		reg = <19>;
> +		brcm,hardware-controlled;
> +	};
> +	ephy3_spd@20 {
> +		reg = <20>;
> +		brcm,hardware-controlled;
> +	};
> +};
> +
> +example 2) BCM63268 with Serial/GPHY0 LEDs
> +
> +leds0: led-controller@10001900 {
> +	compatible = "brcm,bcm6328-leds";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	reg = <0x10001900 0x24>;
> +	brcm,serial-leds;
> +
> +	gphy0_spd0@0 {
> +		reg = <0>;
> +		brcm,hardware-controlled;
> +		brcm,link-signal-sources = <0>;
> +	};
> +	gphy0_spd1@1 {
> +		reg = <1>;
> +		brcm,hardware-controlled;
> +		brcm,link-signal-sources = <1>;
> +	};
> +	inet_red@2 {
> +		reg = <2>;
> +		active-low;
> +		label = "red:inet";
> +	};
> +	dsl_green@3 {
> +		reg = <3>;
> +		active-low;
> +		label = "green:dsl";
> +	};
> +	usb_green@4 {
> +		reg = <4>;
> +		active-low;
> +		label = "green:usb";
> +	};
> +	wps_green@7 {
> +		reg = <7>;
> +		active-low;
> +		label = "green:wps";
> +	};
> +	inet_green@8 {
> +		reg = <8>;
> +		active-low;
> +		label = "green:inet";
> +	};
> +	ephy0_act@9 {
> +		reg = <9>;
> +		brcm,hardware-controlled;
> +	};
> +	ephy1_act@10 {
> +		reg = <10>;
> +		brcm,hardware-controlled;
> +	};
> +	ephy2_act@11 {
> +		reg = <11>;
> +		brcm,hardware-controlled;
> +	};
> +	gphy0_act@12 {
> +		reg = <12>;
> +		brcm,hardware-controlled;
> +	};
> +	ephy0_spd@13 {
> +		reg = <13>;
> +		brcm,hardware-controlled;
> +	};
> +	ephy1_spd@14 {
> +		reg = <14>;
> +		brcm,hardware-controlled;
> +	};
> +	ephy2_spd@15 {
> +		reg = <15>;
> +		brcm,hardware-controlled;
> +	};
> +	power_green@20 {
> +		reg = <20>;
> +		active-low;
> +		label = "green:power";
> +		default-state = "on";
> +	};
> +};
> +
> +example 3) BCM6362 with 1 LED for each EPHY
> +
> +leds0: led-controller@10001900 {
> +	compatible = "brcm,bcm6328-leds";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	reg = <0x10001900 0x24>;
> +
> +	usb@0 {
> +		reg = <0>;
> +		brcm,hardware-controlled;
> +		brcm,link-signal-sources = <0>;
> +		brcm,activity-signal-sources = <0>;
> +		/* USB link/activity routed to USB LED */
> +	};
> +	inet@1 {
> +		reg = <1>;
> +		brcm,hardware-controlled;
> +		brcm,activity-signal-sources = <1>;
> +		/* INET activity routed to INET LED */
> +	};
> +	ephy0@4 {
> +		reg = <4>;
> +		brcm,hardware-controlled;
> +		brcm,link-signal-sources = <4>;
> +		/* EPHY0 link routed to EPHY0 LED */
> +	};
> +	ephy1@5 {
> +		reg = <5>;
> +		brcm,hardware-controlled;
> +		brcm,link-signal-sources = <5>;
> +		/* EPHY1 link routed to EPHY1 LED */
> +	};
> +	ephy2@6 {
> +		reg = <6>;
> +		brcm,hardware-controlled;
> +		brcm,link-signal-sources = <6>;
> +		/* EPHY2 link routed to EPHY2 LED */
> +	};
> +	ephy3@7 {
> +		reg = <7>;
> +		brcm,hardware-controlled;
> +		brcm,link-signal-sources = <7>;
> +		/* EPHY3 link routed to EPHY3 LED */
> +	};
> +	power_green@20 {
> +		reg = <20>;
> +		active-low;
> +		label = "green:power";
> +		default-state = "on";
> +	};
> +};
> +
> +example 4) BCM6362 with 1 LED for all EPHYs
> +
> +leds0: led-controller@10001900 {
> +	compatible = "brcm,bcm6328-leds";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	reg = <0x10001900 0x24>;
> +
> +	usb@0 {
> +		reg = <0>;
> +		brcm,hardware-controlled;
> +		brcm,link-signal-sources = <0 1>;
> +		brcm,activity-signal-sources = <0 1>;
> +		/* USB/INET link/activity routed to USB LED */
> +	};
> +	ephy@4 {
> +		reg = <4>;
> +		brcm,hardware-controlled;
> +		brcm,link-signal-sources = <4 5 6 7>;
> +		/* EPHY0/1/2/3 link routed to EPHY0 LED */
> +	};
> +	power_green@20 {
> +		reg = <20>;
> +		active-low;
> +		label = "green:power";
> +		default-state = "on";
> +	};
> +};
> +
> +example 5) BCM6362 with EPHY LEDs swapped
> +
> +leds0: led-controller@10001900 {
> +	compatible = "brcm,bcm6328-leds";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	reg = <0x10001900 0x24>;
> +
> +	usb@0 {
> +		reg = <0>;
> +		brcm,hardware-controlled;
> +		brcm,link-signal-sources = <0>;
> +		brcm,activity-signal-sources = <0 1>;
> +		/* USB link/activity & INET activity routed to USB LED */
> +	};
> +	ephy0@4 {
> +		reg = <4>;
> +		brcm,hardware-controlled;
> +		brcm,link-signal-sources = <7>;
> +		/* EPHY3 link routed to EPHY0 LED */
> +	};
> +	ephy1@5 {
> +		reg = <5>;
> +		brcm,hardware-controlled;
> +		brcm,link-signal-sources = <6>;
> +		/* EPHY2 link routed to EPHY1 LED */
> +	};
> +	ephy2@6 {
> +		reg = <6>;
> +		brcm,hardware-controlled;
> +		brcm,link-signal-sources = <5>;
> +		/* EPHY1 link routed to EPHY2 LED */
> +	};
> +	ephy3@7 {
> +		reg = <7>;
> +		brcm,hardware-controlled;
> +		brcm,link-signal-sources = <4>;
> +		/* EPHY0 link routed to EPHY3 LED */
> +	};
> +	power_green@20 {
> +		reg = <20>;
> +		active-low;
> +		label = "green:power";
> +		default-state = "on";
> +	};
> +};


-- 
Best Regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v4 0/2] BCM6328 LED driver
  2015-04-24 17:06   ` [PATCH v3 0/2] " Álvaro Fernández Rojas
  2015-04-24 17:06     ` [PATCH v3 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
  2015-04-24 17:06     ` [PATCH v3 2/2] leds: add BCM6328 LED driver Álvaro Fernández Rojas
@ 2015-04-26 12:15     ` Álvaro Fernández Rojas
  2015-04-26 12:15       ` [PATCH v4 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
                         ` (2 more replies)
  2 siblings, 3 replies; 26+ messages in thread
From: Álvaro Fernández Rojas @ 2015-04-26 12:15 UTC (permalink / raw)
  To: linux-leds, devicetree, cooloney, jogo, f.fainelli, cernekee,
	j.anaszewski
  Cc: Álvaro Fernández Rojas

These patches add support and documentation for the BCM6328 LED driver, which is present on BCM6318, BCM6328, BCM6362 and BCM63268.
In these SoCs it's possible to control LEDs both as GPIOs or by hardware.
However, on some devices there are Serial LEDs (LEDs connected to a 74hc controller), which can either be controlled by software (exporting the 74hc as spi-gpio) or hardware using this driver.
The problem is some of these serial LEDs are hardware controlled (ethernet LEDs) and exporting the 74hc as spi-gpio prevents those LEDs to be hardware controlled, so the only chance to keep them working is by using this driver.

v3->v4: Introduce changes suggested by Jacek.
v2->v3: Introduce changes suggested by Jacek.
v1->v2: Introduce changes suggested by Florian and Jonas.

Álvaro Fernández Rojas (2):
  leds: add DT binding for BCM6328 LED controller
  leds: add BCM6328 LED driver

 .../devicetree/bindings/leds/leds-bcm6328.txt      | 309 +++++++++++++++
 drivers/leds/Kconfig                               |   8 +
 drivers/leds/Makefile                              |   1 +
 drivers/leds/leds-bcm6328.c                        | 413 +++++++++++++++++++++
 4 files changed, 731 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-bcm6328.txt
 create mode 100644 drivers/leds/leds-bcm6328.c

-- 
1.9.1

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v4 1/2] leds: add DT binding for BCM6328 LED controller
  2015-04-26 12:15     ` [PATCH v4 0/2] " Álvaro Fernández Rojas
@ 2015-04-26 12:15       ` Álvaro Fernández Rojas
  2015-04-28  8:18         ` Jacek Anaszewski
  2015-04-26 12:15       ` [PATCH v4 2/2] leds: add BCM6328 LED driver Álvaro Fernández Rojas
  2015-04-28 16:50       ` [PATCH v5 0/2] " Álvaro Fernández Rojas
  2 siblings, 1 reply; 26+ messages in thread
From: Álvaro Fernández Rojas @ 2015-04-26 12:15 UTC (permalink / raw)
  To: linux-leds, devicetree, cooloney, jogo, f.fainelli, cernekee,
	j.anaszewski
  Cc: Álvaro Fernández Rojas

This adds device tree binding documentation for the Broadcom BCM6328 LED controller.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Signed-off-by: Jonas Gorski <jogo@openwrt.org>
---
 v4: introduce changes suggested by Jacek
 - Follow Documentation convention.
 v3: introduce changes suggested by Jacek
 - Revert compatible string to "brcm,bcm6328-leds".
 - Improved description.
 - Renamed normal LEDs to hardware controlled.
 - Explain that LEDs are active high by default on active-low optional property.
 - Properties are now grouped depending on the type of LED and optional requirement.
 - Added more examples to explain hardware controlled LEDs and activity/link selection.
 v2: Introduce changes suggested by Florian and Jonas.
 - Change compatible string to "brcm,bcm6328-leds-ctrl".
 - Make valid LEDs statement more strict.
 - Remove compatible strings from LED subnodes.
 - Clarify that LEDs are active high by default.
 - Add a better description for brcm,link-signal-sources and brcm,activity-signal-sources.

 .../devicetree/bindings/leds/leds-bcm6328.txt      | 309 +++++++++++++++++++++
 1 file changed, 309 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-bcm6328.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-bcm6328.txt b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
new file mode 100644
index 0000000..f9e36ad
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
@@ -0,0 +1,309 @@
+LEDs connected to Broadcom BCM6328 controller
+
+This controller is present on BCM6318, BCM6328, BCM6362 and BCM63268.
+In these SoCs it's possible to control LEDs both as GPIOs or by hardware.
+However, on some devices there are Serial LEDs (LEDs connected to a 74x164
+controller), which can either be controlled by software (exporting the 74x164
+as spi-gpio. See Documentation/devicetree/bindings/gpio/gpio-74x164.txt), or
+by hardware using this driver.
+Some of these Serial LEDs are hardware controlled (e.g. ethernet LEDs) and
+exporting the 74x164 as spi-gpio prevents those LEDs to be hardware
+controlled, so the only chance to keep them working is by using this driver.
+
+BCM6328 LED controller has a HWDIS register, which controls whether a LED
+should be controlled by a hardware signal instead of the MODE register value,
+with 0 meaning hardware control enabled and 1 hardware control disabled. This
+is usually 1:1 for hardware to LED signals, but through the activity/link
+registers you have some limited control over rerouting the LEDs (as
+explained later in brcm,link-signal-sources). Even if a LED is hardware
+controlled you are still able to make it blink or light it up if it isn't,
+but you can't turn it off if the hardware decides to light it up. For this
+reason, hardware controlled LEDs aren't registered as LED class devices.
+
+Required properties:
+  - compatible : should be "brcm,bcm6328-leds".
+  - #address-cells : must be 1.
+  - #size-cells : must be 0.
+  - reg : BCM6328 LED controller address and size.
+
+Optional properties:
+  - brcm,serial-leds : Boolean, enables Serial LEDs.
+    Default : false
+
+Each LED is represented as a sub-node of the brcm,bcm6328-leds device.
+
+LED sub-node required properties:
+  - reg : LED pin number (only LEDs 0 to 23 are valid).
+
+LED sub-node optional properties:
+  a) Optional properties for sub-nodes related to software controlled LEDs:
+    - label : see Documentation/devicetree/bindings/leds/common.txt
+    - active-low : Boolean, makes LED active low.
+      Default : false
+    - default-state : see
+      Documentation/devicetree/bindings/leds/leds-gpio.txt
+    - linux,default-trigger : see
+      Documentation/devicetree/bindings/leds/common.txt
+
+  b) Optional properties for sub-nodes related to hardware controlled LEDs:
+    - brcm,hardware-controlled : Boolean, makes this LED hardware controlled.
+      Default : false
+    - brcm,link-signal-sources : An array of hardware link
+      signal sources. Up to four link hardware signals can get muxed into
+      these LEDs. Only valid for LEDs 0 to 7, where LED signals 0 to 3 may
+      be muxed to LEDs 0 to 3, and signals 4 to 7 may be muxed to LEDs
+      4 to 7. A signal can be muxed to more than one LED, and one LED can
+      have more than one source signal.
+    - brcm,activity-signal-sources : An array of hardware activity
+      signal sources. Up to four activity hardware signals can get muxed into
+      these LEDs. Only valid for LEDs 0 to 7, where LED signals 0 to 3 may
+      be muxed to LEDs 0 to 3, and signals 4 to 7 may be muxed to LEDs
+      4 to 7. A signal can be muxed to more than one LED, and one LED can
+      have more than one source signal.
+
+Examples:
+Scenario 1 : BCM6328 with 4 EPHY LEDs
+	leds0: led-controller@10000800 {
+		compatible = "brcm,bcm6328-leds";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x10000800 0x24>;
+
+		alarm_red@2 {
+			reg = <2>;
+			active-low;
+			label = "red:alarm";
+		};
+		inet_green@3 {
+			reg = <3>;
+			active-low;
+			label = "green:inet";
+		};
+		power_green@4 {
+			reg = <4>;
+			active-low;
+			label = "green:power";
+			default-state = "on";
+		};
+		ephy0_spd@17 {
+			reg = <17>;
+			brcm,hardware-controlled;
+		};
+		ephy1_spd@18 {
+			reg = <18>;
+			brcm,hardware-controlled;
+		};
+		ephy2_spd@19 {
+			reg = <19>;
+			brcm,hardware-controlled;
+		};
+		ephy3_spd@20 {
+			reg = <20>;
+			brcm,hardware-controlled;
+		};
+	};
+
+Scenario 2 : BCM63268 with Serial/GPHY0 LEDs
+	leds0: led-controller@10001900 {
+		compatible = "brcm,bcm6328-leds";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x10001900 0x24>;
+		brcm,serial-leds;
+
+		gphy0_spd0@0 {
+			reg = <0>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <0>;
+		};
+		gphy0_spd1@1 {
+			reg = <1>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <1>;
+		};
+		inet_red@2 {
+			reg = <2>;
+			active-low;
+			label = "red:inet";
+		};
+		dsl_green@3 {
+			reg = <3>;
+			active-low;
+			label = "green:dsl";
+		};
+		usb_green@4 {
+			reg = <4>;
+			active-low;
+			label = "green:usb";
+		};
+		wps_green@7 {
+			reg = <7>;
+			active-low;
+			label = "green:wps";
+		};
+		inet_green@8 {
+			reg = <8>;
+			active-low;
+			label = "green:inet";
+		};
+		ephy0_act@9 {
+			reg = <9>;
+			brcm,hardware-controlled;
+		};
+		ephy1_act@10 {
+			reg = <10>;
+			brcm,hardware-controlled;
+		};
+		ephy2_act@11 {
+			reg = <11>;
+			brcm,hardware-controlled;
+		};
+		gphy0_act@12 {
+			reg = <12>;
+			brcm,hardware-controlled;
+		};
+		ephy0_spd@13 {
+			reg = <13>;
+			brcm,hardware-controlled;
+		};
+		ephy1_spd@14 {
+			reg = <14>;
+			brcm,hardware-controlled;
+		};
+		ephy2_spd@15 {
+			reg = <15>;
+			brcm,hardware-controlled;
+		};
+		power_green@20 {
+			reg = <20>;
+			active-low;
+			label = "green:power";
+			default-state = "on";
+		};
+	};
+
+Scenario 3 : BCM6362 with 1 LED for each EPHY
+	leds0: led-controller@10001900 {
+		compatible = "brcm,bcm6328-leds";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x10001900 0x24>;
+
+		usb@0 {
+			reg = <0>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <0>;
+			brcm,activity-signal-sources = <0>;
+			/* USB link/activity routed to USB LED */
+		};
+		inet@1 {
+			reg = <1>;
+			brcm,hardware-controlled;
+			brcm,activity-signal-sources = <1>;
+			/* INET activity routed to INET LED */
+		};
+		ephy0@4 {
+			reg = <4>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <4>;
+			/* EPHY0 link routed to EPHY0 LED */
+		};
+		ephy1@5 {
+			reg = <5>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <5>;
+			/* EPHY1 link routed to EPHY1 LED */
+		};
+		ephy2@6 {
+			reg = <6>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <6>;
+			/* EPHY2 link routed to EPHY2 LED */
+		};
+		ephy3@7 {
+			reg = <7>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <7>;
+			/* EPHY3 link routed to EPHY3 LED */
+		};
+		power_green@20 {
+			reg = <20>;
+			active-low;
+			label = "green:power";
+			default-state = "on";
+		};
+	};
+
+Scenario 4 : BCM6362 with 1 LED for all EPHYs
+	leds0: led-controller@10001900 {
+		compatible = "brcm,bcm6328-leds";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x10001900 0x24>;
+
+		usb@0 {
+			reg = <0>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <0 1>;
+			brcm,activity-signal-sources = <0 1>;
+			/* USB/INET link/activity routed to USB LED */
+		};
+		ephy@4 {
+			reg = <4>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <4 5 6 7>;
+			/* EPHY0/1/2/3 link routed to EPHY0 LED */
+		};
+		power_green@20 {
+			reg = <20>;
+			active-low;
+			label = "green:power";
+			default-state = "on";
+		};
+	};
+
+Scenario 5 : BCM6362 with EPHY LEDs swapped
+	leds0: led-controller@10001900 {
+		compatible = "brcm,bcm6328-leds";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x10001900 0x24>;
+
+		usb@0 {
+			reg = <0>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <0>;
+			brcm,activity-signal-sources = <0 1>;
+			/* USB link/act and INET act routed to USB LED */
+		};
+		ephy0@4 {
+			reg = <4>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <7>;
+			/* EPHY3 link routed to EPHY0 LED */
+		};
+		ephy1@5 {
+			reg = <5>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <6>;
+			/* EPHY2 link routed to EPHY1 LED */
+		};
+		ephy2@6 {
+			reg = <6>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <5>;
+			/* EPHY1 link routed to EPHY2 LED */
+		};
+		ephy3@7 {
+			reg = <7>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <4>;
+			/* EPHY0 link routed to EPHY3 LED */
+		};
+		power_green@20 {
+			reg = <20>;
+			active-low;
+			label = "green:power";
+			default-state = "on";
+		};
+	};
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v4 2/2] leds: add BCM6328 LED driver
  2015-04-26 12:15     ` [PATCH v4 0/2] " Álvaro Fernández Rojas
  2015-04-26 12:15       ` [PATCH v4 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
@ 2015-04-26 12:15       ` Álvaro Fernández Rojas
  2015-04-28  8:18         ` Jacek Anaszewski
  2015-04-28 16:50       ` [PATCH v5 0/2] " Álvaro Fernández Rojas
  2 siblings, 1 reply; 26+ messages in thread
From: Álvaro Fernández Rojas @ 2015-04-26 12:15 UTC (permalink / raw)
  To: linux-leds, devicetree, cooloney, jogo, f.fainelli, cernekee,
	j.anaszewski
  Cc: Álvaro Fernández Rojas

This adds support for the LED controller on Broadcom's BCM6328.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Signed-off-by: Jonas Gorski <jogo@openwrt.org>
---
 v4: resend, no changes.
 v3: introduce changes suggested by Jacek
 - Revert compatible string to "brcm,bcm6328-leds".
 - Add BCM6328_ prefix to macros.
 - Rename blink_del to blink_delay.
 v2: introduce changes suggested by Florian and Jonas.
 - Improve Kconfig description.
 - Remove unused imports.
 - Fix bug in blink_set so it works for multiple LEDs.
 - "brcm,link-selection" -> "brcm,link-signal-sources".
 - "brcm,activity-selection" -> "brcm,activity-signal-sources".
 - Use of_property_read_bool instead of of_get_property for booleans.
 - Add MODULE_AUTHOR strings.

 drivers/leds/Kconfig        |   8 +
 drivers/leds/Makefile       |   1 +
 drivers/leds/leds-bcm6328.c | 413 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 422 insertions(+)
 create mode 100644 drivers/leds/leds-bcm6328.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 966b960..caa8ee6 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -42,6 +42,14 @@ config LEDS_88PM860X
 	  This option enables support for on-chip LED drivers found on Marvell
 	  Semiconductor 88PM8606 PMIC.
 
+config LEDS_BCM6328
+	tristate "LED Support for Broadcom BCM6328"
+	depends on LEDS_CLASS
+	depends on OF
+	help
+	  This option enables support for LEDs connected to the BCM6328
+	  LED HW controller accessed via MMIO registers.
+
 config LEDS_LM3530
 	tristate "LCD Backlight driver for LM3530"
 	depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index bf46093..309a46b 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_LEDS_TRIGGERS)		+= led-triggers.o
 
 # LED Platform Drivers
 obj-$(CONFIG_LEDS_88PM860X)		+= leds-88pm860x.o
+obj-$(CONFIG_LEDS_BCM6328)		+= leds-bcm6328.o
 obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
 obj-$(CONFIG_LEDS_LOCOMO)		+= leds-locomo.o
 obj-$(CONFIG_LEDS_LM3530)		+= leds-lm3530.o
diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
new file mode 100644
index 0000000..d2bcbc3
--- /dev/null
+++ b/drivers/leds/leds-bcm6328.c
@@ -0,0 +1,413 @@
+/*
+ * Driver for BCM6328 memory-mapped LEDs, based on leds-syscon.c
+ *
+ * Copyright 2015 Álvaro Fernández Rojas <noltari@gmail.com>
+ * Copyright 2015 Jonas Gorski <jogo@openwrt.org>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+#include <linux/io.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+
+#define BCM6328_REG_INIT		0x00
+#define BCM6328_REG_MODE_HI		0x04
+#define BCM6328_REG_MODE_LO		0x08
+#define BCM6328_REG_HWDIS		0x0c
+#define BCM6328_REG_STROBE		0x10
+#define BCM6328_REG_LNKACTSEL_HI	0x14
+#define BCM6328_REG_LNKACTSEL_LO	0x18
+#define BCM6328_REG_RBACK		0x1c
+#define BCM6328_REG_SERMUX		0x20
+
+#define BCM6328_LED_MAX_COUNT		24
+#define BCM6328_LED_DEF_DELAY		500
+#define BCM6328_LED_INTERVAL_MS		20
+
+#define BCM6328_LED_INTV_MASK		0x3f
+#define BCM6328_LED_FAST_INTV_SHIFT	6
+#define BCM6328_LED_FAST_INTV_MASK	(BCM6328_LED_INTV_MASK << \
+					 BCM6328_LED_FAST_INTV_SHIFT)
+#define BCM6328_SERIAL_LED_EN		BIT(12)
+#define BCM6328_SERIAL_LED_MUX		BIT(13)
+#define BCM6328_SERIAL_LED_CLK_NPOL	BIT(14)
+#define BCM6328_SERIAL_LED_DATA_PPOL	BIT(15)
+#define BCM6328_SERIAL_LED_SHIFT_DIR	BIT(16)
+#define BCM6328_LED_SHIFT_TEST		BIT(30)
+#define BCM6328_LED_TEST		BIT(31)
+
+#define BCM6328_LED_MODE_MASK		3
+#define BCM6328_LED_MODE_OFF		0
+#define BCM6328_LED_MODE_FAST		1
+#define BCM6328_LED_MODE_BLINK		2
+#define BCM6328_LED_MODE_ON		3
+#define BCM6328_LED_SHIFT(X)		((X) << 1)
+
+/**
+ * struct bcm6328_led - state container for bcm6328 based LEDs
+ * @cdev: LED class device for this LED
+ * @mem: memory resource
+ * @lock: memory lock
+ * @pin: LED pin number
+ * @blink_leds: blinking LEDs
+ * @blink_delay: blinking delay
+ * @active_low: LED is active low
+ */
+struct bcm6328_led {
+	struct led_classdev cdev;
+	void __iomem *mem;
+	spinlock_t *lock;
+	unsigned long pin;
+	unsigned long *blink_leds;
+	unsigned long *blink_delay;
+	bool active_low;
+};
+
+static void bcm6328_led_write(void __iomem *reg, unsigned long data)
+{
+	iowrite32be(data, reg);
+}
+
+static unsigned long bcm6328_led_read(void __iomem *reg)
+{
+	return ioread32be(reg);
+}
+
+/**
+ * LEDMode 64 bits / 24 LEDs
+ * bits [31:0] -> LEDs 8-23
+ * bits [47:32] -> LEDs 0-7
+ * bits [63:48] -> unused
+ */
+static unsigned long bcm6328_pin2shift(unsigned long pin)
+{
+	if (pin < 8)
+		return pin + 16; /* LEDs 0-7 (bits 47:32) */
+	else
+		return pin - 8; /* LEDs 8-23 (bits 31:0) */
+}
+
+static void bcm6328_led_mode(struct bcm6328_led *led, unsigned long value)
+{
+	void __iomem *mode;
+	unsigned long val, shift;
+
+	shift = bcm6328_pin2shift(led->pin);
+	if (shift / 16)
+		mode = led->mem + BCM6328_REG_MODE_HI;
+	else
+		mode = led->mem + BCM6328_REG_MODE_LO;
+
+	val = bcm6328_led_read(mode);
+	val &= ~(BCM6328_LED_MODE_MASK << BCM6328_LED_SHIFT(shift % 16));
+	val |= (value << BCM6328_LED_SHIFT(shift % 16));
+	bcm6328_led_write(mode, val);
+}
+
+static void bcm6328_led_set(struct led_classdev *led_cdev,
+			    enum led_brightness value)
+{
+	struct bcm6328_led *led =
+		container_of(led_cdev, struct bcm6328_led, cdev);
+	unsigned long flags;
+
+	spin_lock_irqsave(led->lock, flags);
+	*(led->blink_leds) &= ~BIT(led->pin);
+	if ((led->active_low && value == LED_OFF) ||
+	    (!led->active_low && value != LED_OFF))
+		bcm6328_led_mode(led, BCM6328_LED_MODE_OFF);
+	else
+		bcm6328_led_mode(led, BCM6328_LED_MODE_ON);
+	spin_unlock_irqrestore(led->lock, flags);
+}
+
+static int bcm6328_blink_set(struct led_classdev *led_cdev,
+			     unsigned long *delay_on, unsigned long *delay_off)
+{
+	struct bcm6328_led *led =
+		container_of(led_cdev, struct bcm6328_led, cdev);
+	unsigned long delay, flags;
+
+	if (!*delay_on)
+		*delay_on = BCM6328_LED_DEF_DELAY;
+	if (!*delay_off)
+		*delay_off = BCM6328_LED_DEF_DELAY;
+
+	if (*delay_on != *delay_off) {
+		dev_dbg(led_cdev->dev,
+			"fallback to soft blinking (delay_on != delay_off)\n");
+		return -EINVAL;
+	}
+
+	delay = *delay_on / BCM6328_LED_INTERVAL_MS;
+	if (delay == 0)
+		delay = 1;
+	else if (delay > BCM6328_LED_INTV_MASK) {
+		dev_dbg(led_cdev->dev,
+			"fallback to soft blinking (delay > %ums)\n",
+			BCM6328_LED_INTV_MASK * BCM6328_LED_INTERVAL_MS);
+		return -EINVAL;
+	}
+
+	spin_lock_irqsave(led->lock, flags);
+	if (*(led->blink_leds) == 0 ||
+	    *(led->blink_leds) == BIT(led->pin) ||
+	    *(led->blink_delay) == delay) {
+		unsigned long val;
+
+		*(led->blink_leds) |= BIT(led->pin);
+		*(led->blink_delay) = delay;
+
+		val = bcm6328_led_read(led->mem + BCM6328_REG_INIT);
+		val &= ~BCM6328_LED_FAST_INTV_MASK;
+		val |= (delay << BCM6328_LED_FAST_INTV_SHIFT);
+		bcm6328_led_write(led->mem + BCM6328_REG_INIT, val);
+
+		bcm6328_led_mode(led, BCM6328_LED_MODE_BLINK);
+
+		spin_unlock_irqrestore(led->lock, flags);
+	} else {
+		spin_unlock_irqrestore(led->lock, flags);
+		dev_dbg(led_cdev->dev,
+			"fallback to soft blinking (delay already set)\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int bcm6328_hwled(struct device *dev, struct device_node *nc, u32 reg,
+			 void __iomem *mem, spinlock_t *lock)
+{
+	int i, cnt;
+	unsigned long flags, val;
+
+	spin_lock_irqsave(lock, flags);
+	val = bcm6328_led_read(mem + BCM6328_REG_HWDIS);
+	val &= ~BIT(reg);
+	bcm6328_led_write(mem + BCM6328_REG_HWDIS, val);
+	spin_unlock_irqrestore(lock, flags);
+
+	/* Only LEDs 0-7 can be activity/link controlled */
+	if (reg >= 8)
+		return 0;
+
+	cnt = of_property_count_elems_of_size(nc, "brcm,link-signal-sources",
+					      sizeof(u32));
+	for (i = 0; i < cnt; i++) {
+		u32 sel;
+		void __iomem *addr;
+
+		if (reg < 4)
+			addr = mem + BCM6328_REG_LNKACTSEL_LO;
+		else
+			addr = mem + BCM6328_REG_LNKACTSEL_HI;
+
+		of_property_read_u32_index(nc, "brcm,link-signal-sources", i,
+					   &sel);
+
+		if (reg / 4 != sel / 4) {
+			dev_warn(dev, "invalid link signal source\n");
+			continue;
+		}
+
+		spin_lock_irqsave(lock, flags);
+		val = bcm6328_led_read(addr);
+		val |= (BIT(reg) << (((sel % 4) * 4) + 16));
+		bcm6328_led_write(addr, val);
+		spin_unlock_irqrestore(lock, flags);
+	}
+
+	cnt = of_property_count_elems_of_size(nc,
+					      "brcm,activity-signal-sources",
+					      sizeof(u32));
+	for (i = 0; i < cnt; i++) {
+		u32 sel;
+		void __iomem *addr;
+
+		if (reg < 4)
+			addr = mem + BCM6328_REG_LNKACTSEL_LO;
+		else
+			addr = mem + BCM6328_REG_LNKACTSEL_HI;
+
+		of_property_read_u32_index(nc, "brcm,activity-signal-sources",
+					   i, &sel);
+
+		if (reg / 4 != sel / 4) {
+			dev_warn(dev, "invalid activity signal source\n");
+			continue;
+		}
+
+		spin_lock_irqsave(lock, flags);
+		val = bcm6328_led_read(addr);
+		val |= (BIT(reg) << ((sel % 4) * 4));
+		bcm6328_led_write(addr, val);
+		spin_unlock_irqrestore(lock, flags);
+	}
+
+	return 0;
+}
+
+static int bcm6328_led(struct device *dev, struct device_node *nc, u32 reg,
+		       void __iomem *mem, spinlock_t *lock,
+		       unsigned long *blink_leds, unsigned long *blink_delay)
+{
+	struct bcm6328_led *led;
+	unsigned long flags;
+	const char *state;
+	int rc;
+
+	led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+
+	led->pin = reg;
+	led->mem = mem;
+	led->lock = lock;
+	led->blink_leds = blink_leds;
+	led->blink_delay = blink_delay;
+
+	if (of_property_read_bool(nc, "active-low"))
+		led->active_low = 1;
+
+	led->cdev.name = of_get_property(nc, "label", NULL) ? : nc->name;
+	led->cdev.default_trigger = of_get_property(nc,
+						    "linux,default-trigger",
+						    NULL);
+
+	if (!of_property_read_string(nc, "default-state", &state)) {
+		spin_lock_irqsave(lock, flags);
+		if (!strcmp(state, "on")) {
+			led->cdev.brightness = LED_FULL;
+			bcm6328_led_mode(led, BCM6328_LED_MODE_ON);
+		} else if (!strcmp(state, "keep")) {
+			void __iomem *mode;
+			unsigned long val, shift;
+
+			shift = bcm6328_pin2shift(led->pin);
+			if (shift / 16)
+				mode = mem + BCM6328_REG_MODE_HI;
+			else
+				mode = mem + BCM6328_REG_MODE_LO;
+
+			val = bcm6328_led_read(mode) >> (shift % 16);
+			val &= BCM6328_LED_MODE_MASK;
+			if (val == BCM6328_LED_MODE_ON)
+				led->cdev.brightness = LED_FULL;
+			else {
+				led->cdev.brightness = LED_OFF;
+				bcm6328_led_mode(led, BCM6328_LED_MODE_OFF);
+			}
+		} else {
+			led->cdev.brightness = LED_OFF;
+			bcm6328_led_mode(led, BCM6328_LED_MODE_OFF);
+		}
+		spin_unlock_irqrestore(lock, flags);
+	}
+
+	led->cdev.brightness_set = bcm6328_led_set;
+	led->cdev.blink_set = bcm6328_blink_set;
+
+	rc = led_classdev_register(dev, &led->cdev);
+	if (rc < 0)
+		return rc;
+
+	dev_dbg(dev, "registered LED %s\n", led->cdev.name);
+
+	return 0;
+}
+
+static int bcm6328_leds_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *child;
+	struct resource *mem_r;
+	void __iomem *mem;
+	spinlock_t *lock;
+	unsigned long val, *blink_leds, *blink_delay;
+
+	mem_r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mem_r)
+		return -EINVAL;
+
+	mem = devm_ioremap_resource(dev, mem_r);
+	if (IS_ERR(mem))
+		return PTR_ERR(mem);
+
+	lock = devm_kzalloc(dev, sizeof(*lock), GFP_KERNEL);
+	if (!lock)
+		return -ENOMEM;
+
+	blink_leds = devm_kzalloc(dev, sizeof(*blink_leds), GFP_KERNEL);
+	if (!blink_leds)
+		return -ENOMEM;
+
+	blink_delay = devm_kzalloc(dev, sizeof(*blink_delay), GFP_KERNEL);
+	if (!blink_delay)
+		return -ENOMEM;
+
+	spin_lock_init(lock);
+
+	bcm6328_led_write(mem + BCM6328_REG_HWDIS, ~0);
+	bcm6328_led_write(mem + BCM6328_REG_LNKACTSEL_HI, 0);
+	bcm6328_led_write(mem + BCM6328_REG_LNKACTSEL_LO, 0);
+
+	val = bcm6328_led_read(mem + BCM6328_REG_INIT);
+	val &= ~BCM6328_SERIAL_LED_EN;
+	if (of_property_read_bool(np, "brcm,serial-leds"))
+		val |= BCM6328_SERIAL_LED_EN;
+	bcm6328_led_write(mem + BCM6328_REG_INIT, val);
+
+	for_each_available_child_of_node(np, child) {
+		int rc;
+		u32 reg;
+
+		if (of_property_read_u32(child, "reg", &reg))
+			continue;
+
+		if (reg >= BCM6328_LED_MAX_COUNT) {
+			dev_err(dev, "invalid LED (>= %d)\n",
+				BCM6328_LED_MAX_COUNT);
+			continue;
+		}
+
+		if (of_property_read_bool(child, "brcm,hardware-controlled"))
+			rc = bcm6328_hwled(dev, child, reg, mem, lock);
+		else
+			rc = bcm6328_led(dev, child, reg, mem, lock,
+					 blink_leds, blink_delay);
+
+		if (rc < 0)
+			return rc;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id bcm6328_leds_of_match[] = {
+	{ .compatible = "brcm,bcm6328-leds", },
+	{ },
+};
+
+static struct platform_driver bcm6328_leds_driver = {
+	.probe = bcm6328_leds_probe,
+	.driver = {
+		.name = "leds-bcm6328",
+		.of_match_table = bcm6328_leds_of_match,
+	},
+};
+
+module_platform_driver(bcm6328_leds_driver);
+
+MODULE_AUTHOR("Álvaro Fernández Rojas <noltari@gmail.com>");
+MODULE_AUTHOR("Jonas Gorski <jogo@openwrt.org>");
+MODULE_DESCRIPTION("LED driver for BCM6328 controllers");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:leds-bcm6328");
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH v4 2/2] leds: add BCM6328 LED driver
  2015-04-26 12:15       ` [PATCH v4 2/2] leds: add BCM6328 LED driver Álvaro Fernández Rojas
@ 2015-04-28  8:18         ` Jacek Anaszewski
  0 siblings, 0 replies; 26+ messages in thread
From: Jacek Anaszewski @ 2015-04-28  8:18 UTC (permalink / raw)
  To: Álvaro Fernández Rojas
  Cc: linux-leds, devicetree, cooloney, jogo, f.fainelli, cernekee

Hi Alvaro,

Thanks for the update. I see one minor issue below.

On 04/26/2015 02:15 PM, Álvaro Fernández Rojas wrote:
> This adds support for the LED controller on Broadcom's BCM6328.
>
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> Signed-off-by: Jonas Gorski <jogo@openwrt.org>
> ---
>   v4: resend, no changes.
>   v3: introduce changes suggested by Jacek
>   - Revert compatible string to "brcm,bcm6328-leds".
>   - Add BCM6328_ prefix to macros.
>   - Rename blink_del to blink_delay.
>   v2: introduce changes suggested by Florian and Jonas.
>   - Improve Kconfig description.
>   - Remove unused imports.
>   - Fix bug in blink_set so it works for multiple LEDs.
>   - "brcm,link-selection" -> "brcm,link-signal-sources".
>   - "brcm,activity-selection" -> "brcm,activity-signal-sources".
>   - Use of_property_read_bool instead of of_get_property for booleans.
>   - Add MODULE_AUTHOR strings.
>
>   drivers/leds/Kconfig        |   8 +
>   drivers/leds/Makefile       |   1 +
>   drivers/leds/leds-bcm6328.c | 413 ++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 422 insertions(+)
>   create mode 100644 drivers/leds/leds-bcm6328.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 966b960..caa8ee6 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -42,6 +42,14 @@ config LEDS_88PM860X
>   	  This option enables support for on-chip LED drivers found on Marvell
>   	  Semiconductor 88PM8606 PMIC.
>
> +config LEDS_BCM6328
> +	tristate "LED Support for Broadcom BCM6328"
> +	depends on LEDS_CLASS
> +	depends on OF
> +	help
> +	  This option enables support for LEDs connected to the BCM6328
> +	  LED HW controller accessed via MMIO registers.
> +
>   config LEDS_LM3530
>   	tristate "LCD Backlight driver for LM3530"
>   	depends on LEDS_CLASS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index bf46093..309a46b 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_LEDS_TRIGGERS)		+= led-triggers.o
>
>   # LED Platform Drivers
>   obj-$(CONFIG_LEDS_88PM860X)		+= leds-88pm860x.o
> +obj-$(CONFIG_LEDS_BCM6328)		+= leds-bcm6328.o
>   obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
>   obj-$(CONFIG_LEDS_LOCOMO)		+= leds-locomo.o
>   obj-$(CONFIG_LEDS_LM3530)		+= leds-lm3530.o
> diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
> new file mode 100644
> index 0000000..d2bcbc3
> --- /dev/null
> +++ b/drivers/leds/leds-bcm6328.c
> @@ -0,0 +1,413 @@
> +/*
> + * Driver for BCM6328 memory-mapped LEDs, based on leds-syscon.c
> + *
> + * Copyright 2015 Álvaro Fernández Rojas <noltari@gmail.com>
> + * Copyright 2015 Jonas Gorski <jogo@openwrt.org>
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */
> +#include <linux/io.h>
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +
> +#define BCM6328_REG_INIT		0x00
> +#define BCM6328_REG_MODE_HI		0x04
> +#define BCM6328_REG_MODE_LO		0x08
> +#define BCM6328_REG_HWDIS		0x0c
> +#define BCM6328_REG_STROBE		0x10
> +#define BCM6328_REG_LNKACTSEL_HI	0x14
> +#define BCM6328_REG_LNKACTSEL_LO	0x18
> +#define BCM6328_REG_RBACK		0x1c
> +#define BCM6328_REG_SERMUX		0x20
> +
> +#define BCM6328_LED_MAX_COUNT		24
> +#define BCM6328_LED_DEF_DELAY		500
> +#define BCM6328_LED_INTERVAL_MS		20
> +
> +#define BCM6328_LED_INTV_MASK		0x3f
> +#define BCM6328_LED_FAST_INTV_SHIFT	6
> +#define BCM6328_LED_FAST_INTV_MASK	(BCM6328_LED_INTV_MASK << \
> +					 BCM6328_LED_FAST_INTV_SHIFT)
> +#define BCM6328_SERIAL_LED_EN		BIT(12)
> +#define BCM6328_SERIAL_LED_MUX		BIT(13)
> +#define BCM6328_SERIAL_LED_CLK_NPOL	BIT(14)
> +#define BCM6328_SERIAL_LED_DATA_PPOL	BIT(15)
> +#define BCM6328_SERIAL_LED_SHIFT_DIR	BIT(16)
> +#define BCM6328_LED_SHIFT_TEST		BIT(30)
> +#define BCM6328_LED_TEST		BIT(31)
> +
> +#define BCM6328_LED_MODE_MASK		3
> +#define BCM6328_LED_MODE_OFF		0
> +#define BCM6328_LED_MODE_FAST		1
> +#define BCM6328_LED_MODE_BLINK		2
> +#define BCM6328_LED_MODE_ON		3
> +#define BCM6328_LED_SHIFT(X)		((X) << 1)
> +
> +/**
> + * struct bcm6328_led - state container for bcm6328 based LEDs
> + * @cdev: LED class device for this LED
> + * @mem: memory resource
> + * @lock: memory lock
> + * @pin: LED pin number
> + * @blink_leds: blinking LEDs
> + * @blink_delay: blinking delay
> + * @active_low: LED is active low
> + */
> +struct bcm6328_led {
> +	struct led_classdev cdev;
> +	void __iomem *mem;
> +	spinlock_t *lock;
> +	unsigned long pin;
> +	unsigned long *blink_leds;
> +	unsigned long *blink_delay;
> +	bool active_low;
> +};
> +
> +static void bcm6328_led_write(void __iomem *reg, unsigned long data)
> +{
> +	iowrite32be(data, reg);
> +}
> +
> +static unsigned long bcm6328_led_read(void __iomem *reg)
> +{
> +	return ioread32be(reg);
> +}
> +
> +/**
> + * LEDMode 64 bits / 24 LEDs
> + * bits [31:0] -> LEDs 8-23
> + * bits [47:32] -> LEDs 0-7
> + * bits [63:48] -> unused
> + */
> +static unsigned long bcm6328_pin2shift(unsigned long pin)
> +{
> +	if (pin < 8)
> +		return pin + 16; /* LEDs 0-7 (bits 47:32) */
> +	else
> +		return pin - 8; /* LEDs 8-23 (bits 31:0) */
> +}
> +
> +static void bcm6328_led_mode(struct bcm6328_led *led, unsigned long value)
> +{
> +	void __iomem *mode;
> +	unsigned long val, shift;
> +
> +	shift = bcm6328_pin2shift(led->pin);
> +	if (shift / 16)
> +		mode = led->mem + BCM6328_REG_MODE_HI;
> +	else
> +		mode = led->mem + BCM6328_REG_MODE_LO;
> +
> +	val = bcm6328_led_read(mode);
> +	val &= ~(BCM6328_LED_MODE_MASK << BCM6328_LED_SHIFT(shift % 16));
> +	val |= (value << BCM6328_LED_SHIFT(shift % 16));
> +	bcm6328_led_write(mode, val);
> +}
> +
> +static void bcm6328_led_set(struct led_classdev *led_cdev,
> +			    enum led_brightness value)
> +{
> +	struct bcm6328_led *led =
> +		container_of(led_cdev, struct bcm6328_led, cdev);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(led->lock, flags);
> +	*(led->blink_leds) &= ~BIT(led->pin);
> +	if ((led->active_low && value == LED_OFF) ||
> +	    (!led->active_low && value != LED_OFF))
> +		bcm6328_led_mode(led, BCM6328_LED_MODE_OFF);
> +	else
> +		bcm6328_led_mode(led, BCM6328_LED_MODE_ON);
> +	spin_unlock_irqrestore(led->lock, flags);
> +}
> +
> +static int bcm6328_blink_set(struct led_classdev *led_cdev,
> +			     unsigned long *delay_on, unsigned long *delay_off)
> +{
> +	struct bcm6328_led *led =
> +		container_of(led_cdev, struct bcm6328_led, cdev);
> +	unsigned long delay, flags;
> +
> +	if (!*delay_on)
> +		*delay_on = BCM6328_LED_DEF_DELAY;
> +	if (!*delay_off)
> +		*delay_off = BCM6328_LED_DEF_DELAY;
> +
> +	if (*delay_on != *delay_off) {
> +		dev_dbg(led_cdev->dev,
> +			"fallback to soft blinking (delay_on != delay_off)\n");
> +		return -EINVAL;
> +	}
> +
> +	delay = *delay_on / BCM6328_LED_INTERVAL_MS;
> +	if (delay == 0)
> +		delay = 1;
> +	else if (delay > BCM6328_LED_INTV_MASK) {
> +		dev_dbg(led_cdev->dev,
> +			"fallback to soft blinking (delay > %ums)\n",
> +			BCM6328_LED_INTV_MASK * BCM6328_LED_INTERVAL_MS);
> +		return -EINVAL;
> +	}
> +
> +	spin_lock_irqsave(led->lock, flags);
> +	if (*(led->blink_leds) == 0 ||
> +	    *(led->blink_leds) == BIT(led->pin) ||
> +	    *(led->blink_delay) == delay) {
> +		unsigned long val;
> +
> +		*(led->blink_leds) |= BIT(led->pin);
> +		*(led->blink_delay) = delay;
> +
> +		val = bcm6328_led_read(led->mem + BCM6328_REG_INIT);
> +		val &= ~BCM6328_LED_FAST_INTV_MASK;
> +		val |= (delay << BCM6328_LED_FAST_INTV_SHIFT);
> +		bcm6328_led_write(led->mem + BCM6328_REG_INIT, val);
> +
> +		bcm6328_led_mode(led, BCM6328_LED_MODE_BLINK);
> +
> +		spin_unlock_irqrestore(led->lock, flags);
> +	} else {
> +		spin_unlock_irqrestore(led->lock, flags);
> +		dev_dbg(led_cdev->dev,
> +			"fallback to soft blinking (delay already set)\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int bcm6328_hwled(struct device *dev, struct device_node *nc, u32 reg,
> +			 void __iomem *mem, spinlock_t *lock)
> +{
> +	int i, cnt;
> +	unsigned long flags, val;
> +
> +	spin_lock_irqsave(lock, flags);
> +	val = bcm6328_led_read(mem + BCM6328_REG_HWDIS);
> +	val &= ~BIT(reg);
> +	bcm6328_led_write(mem + BCM6328_REG_HWDIS, val);
> +	spin_unlock_irqrestore(lock, flags);
> +
> +	/* Only LEDs 0-7 can be activity/link controlled */
> +	if (reg >= 8)
> +		return 0;
> +
> +	cnt = of_property_count_elems_of_size(nc, "brcm,link-signal-sources",
> +					      sizeof(u32));
> +	for (i = 0; i < cnt; i++) {
> +		u32 sel;
> +		void __iomem *addr;
> +
> +		if (reg < 4)
> +			addr = mem + BCM6328_REG_LNKACTSEL_LO;
> +		else
> +			addr = mem + BCM6328_REG_LNKACTSEL_HI;
> +
> +		of_property_read_u32_index(nc, "brcm,link-signal-sources", i,
> +					   &sel);
> +
> +		if (reg / 4 != sel / 4) {
> +			dev_warn(dev, "invalid link signal source\n");
> +			continue;
> +		}
> +
> +		spin_lock_irqsave(lock, flags);
> +		val = bcm6328_led_read(addr);
> +		val |= (BIT(reg) << (((sel % 4) * 4) + 16));
> +		bcm6328_led_write(addr, val);
> +		spin_unlock_irqrestore(lock, flags);
> +	}
> +
> +	cnt = of_property_count_elems_of_size(nc,
> +					      "brcm,activity-signal-sources",
> +					      sizeof(u32));
> +	for (i = 0; i < cnt; i++) {
> +		u32 sel;
> +		void __iomem *addr;
> +
> +		if (reg < 4)
> +			addr = mem + BCM6328_REG_LNKACTSEL_LO;
> +		else
> +			addr = mem + BCM6328_REG_LNKACTSEL_HI;
> +
> +		of_property_read_u32_index(nc, "brcm,activity-signal-sources",
> +					   i, &sel);
> +
> +		if (reg / 4 != sel / 4) {
> +			dev_warn(dev, "invalid activity signal source\n");
> +			continue;
> +		}
> +
> +		spin_lock_irqsave(lock, flags);
> +		val = bcm6328_led_read(addr);
> +		val |= (BIT(reg) << ((sel % 4) * 4));
> +		bcm6328_led_write(addr, val);
> +		spin_unlock_irqrestore(lock, flags);
> +	}
> +
> +	return 0;
> +}
> +
> +static int bcm6328_led(struct device *dev, struct device_node *nc, u32 reg,
> +		       void __iomem *mem, spinlock_t *lock,
> +		       unsigned long *blink_leds, unsigned long *blink_delay)
> +{
> +	struct bcm6328_led *led;
> +	unsigned long flags;
> +	const char *state;
> +	int rc;
> +
> +	led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
> +	if (!led)
> +		return -ENOMEM;
> +
> +	led->pin = reg;
> +	led->mem = mem;
> +	led->lock = lock;
> +	led->blink_leds = blink_leds;
> +	led->blink_delay = blink_delay;
> +
> +	if (of_property_read_bool(nc, "active-low"))
> +		led->active_low = 1;

active_low is of bool type, so let's assign 'true' here instead of 1.

Having this fixed:

Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com>

> +
> +	led->cdev.name = of_get_property(nc, "label", NULL) ? : nc->name;
> +	led->cdev.default_trigger = of_get_property(nc,
> +						    "linux,default-trigger",
> +						    NULL);
> +
> +	if (!of_property_read_string(nc, "default-state", &state)) {
> +		spin_lock_irqsave(lock, flags);
> +		if (!strcmp(state, "on")) {
> +			led->cdev.brightness = LED_FULL;
> +			bcm6328_led_mode(led, BCM6328_LED_MODE_ON);
> +		} else if (!strcmp(state, "keep")) {
> +			void __iomem *mode;
> +			unsigned long val, shift;
> +
> +			shift = bcm6328_pin2shift(led->pin);
> +			if (shift / 16)
> +				mode = mem + BCM6328_REG_MODE_HI;
> +			else
> +				mode = mem + BCM6328_REG_MODE_LO;
> +
> +			val = bcm6328_led_read(mode) >> (shift % 16);
> +			val &= BCM6328_LED_MODE_MASK;
> +			if (val == BCM6328_LED_MODE_ON)
> +				led->cdev.brightness = LED_FULL;
> +			else {
> +				led->cdev.brightness = LED_OFF;
> +				bcm6328_led_mode(led, BCM6328_LED_MODE_OFF);
> +			}
> +		} else {
> +			led->cdev.brightness = LED_OFF;
> +			bcm6328_led_mode(led, BCM6328_LED_MODE_OFF);
> +		}
> +		spin_unlock_irqrestore(lock, flags);
> +	}
> +
> +	led->cdev.brightness_set = bcm6328_led_set;
> +	led->cdev.blink_set = bcm6328_blink_set;
> +
> +	rc = led_classdev_register(dev, &led->cdev);
> +	if (rc < 0)
> +		return rc;
> +
> +	dev_dbg(dev, "registered LED %s\n", led->cdev.name);
> +
> +	return 0;
> +}
> +
> +static int bcm6328_leds_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *child;
> +	struct resource *mem_r;
> +	void __iomem *mem;
> +	spinlock_t *lock;
> +	unsigned long val, *blink_leds, *blink_delay;
> +
> +	mem_r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!mem_r)
> +		return -EINVAL;
> +
> +	mem = devm_ioremap_resource(dev, mem_r);
> +	if (IS_ERR(mem))
> +		return PTR_ERR(mem);
> +
> +	lock = devm_kzalloc(dev, sizeof(*lock), GFP_KERNEL);
> +	if (!lock)
> +		return -ENOMEM;
> +
> +	blink_leds = devm_kzalloc(dev, sizeof(*blink_leds), GFP_KERNEL);
> +	if (!blink_leds)
> +		return -ENOMEM;
> +
> +	blink_delay = devm_kzalloc(dev, sizeof(*blink_delay), GFP_KERNEL);
> +	if (!blink_delay)
> +		return -ENOMEM;
> +
> +	spin_lock_init(lock);
> +
> +	bcm6328_led_write(mem + BCM6328_REG_HWDIS, ~0);
> +	bcm6328_led_write(mem + BCM6328_REG_LNKACTSEL_HI, 0);
> +	bcm6328_led_write(mem + BCM6328_REG_LNKACTSEL_LO, 0);
> +
> +	val = bcm6328_led_read(mem + BCM6328_REG_INIT);
> +	val &= ~BCM6328_SERIAL_LED_EN;
> +	if (of_property_read_bool(np, "brcm,serial-leds"))
> +		val |= BCM6328_SERIAL_LED_EN;
> +	bcm6328_led_write(mem + BCM6328_REG_INIT, val);
> +
> +	for_each_available_child_of_node(np, child) {
> +		int rc;
> +		u32 reg;
> +
> +		if (of_property_read_u32(child, "reg", &reg))
> +			continue;
> +
> +		if (reg >= BCM6328_LED_MAX_COUNT) {
> +			dev_err(dev, "invalid LED (>= %d)\n",
> +				BCM6328_LED_MAX_COUNT);
> +			continue;
> +		}
> +
> +		if (of_property_read_bool(child, "brcm,hardware-controlled"))
> +			rc = bcm6328_hwled(dev, child, reg, mem, lock);
> +		else
> +			rc = bcm6328_led(dev, child, reg, mem, lock,
> +					 blink_leds, blink_delay);
> +
> +		if (rc < 0)
> +			return rc;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id bcm6328_leds_of_match[] = {
> +	{ .compatible = "brcm,bcm6328-leds", },
> +	{ },
> +};
> +
> +static struct platform_driver bcm6328_leds_driver = {
> +	.probe = bcm6328_leds_probe,
> +	.driver = {
> +		.name = "leds-bcm6328",
> +		.of_match_table = bcm6328_leds_of_match,
> +	},
> +};
> +
> +module_platform_driver(bcm6328_leds_driver);
> +
> +MODULE_AUTHOR("Álvaro Fernández Rojas <noltari@gmail.com>");
> +MODULE_AUTHOR("Jonas Gorski <jogo@openwrt.org>");
> +MODULE_DESCRIPTION("LED driver for BCM6328 controllers");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:leds-bcm6328");
>


-- 
Best Regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v4 1/2] leds: add DT binding for BCM6328 LED controller
  2015-04-26 12:15       ` [PATCH v4 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
@ 2015-04-28  8:18         ` Jacek Anaszewski
  0 siblings, 0 replies; 26+ messages in thread
From: Jacek Anaszewski @ 2015-04-28  8:18 UTC (permalink / raw)
  To: Álvaro Fernández Rojas
  Cc: linux-leds, devicetree, cooloney, jogo, f.fainelli, cernekee

On 04/26/2015 02:15 PM, Álvaro Fernández Rojas wrote:
> This adds device tree binding documentation for the Broadcom BCM6328 LED controller.
>
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> Signed-off-by: Jonas Gorski <jogo@openwrt.org>
> ---
>   v4: introduce changes suggested by Jacek
>   - Follow Documentation convention.
>   v3: introduce changes suggested by Jacek
>   - Revert compatible string to "brcm,bcm6328-leds".
>   - Improved description.
>   - Renamed normal LEDs to hardware controlled.
>   - Explain that LEDs are active high by default on active-low optional property.
>   - Properties are now grouped depending on the type of LED and optional requirement.
>   - Added more examples to explain hardware controlled LEDs and activity/link selection.
>   v2: Introduce changes suggested by Florian and Jonas.
>   - Change compatible string to "brcm,bcm6328-leds-ctrl".
>   - Make valid LEDs statement more strict.
>   - Remove compatible strings from LED subnodes.
>   - Clarify that LEDs are active high by default.
>   - Add a better description for brcm,link-signal-sources and brcm,activity-signal-sources.
>
>   .../devicetree/bindings/leds/leds-bcm6328.txt      | 309 +++++++++++++++++++++
>   1 file changed, 309 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/leds/leds-bcm6328.txt

Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com>

-- 
Best Regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v5 0/2] BCM6328 LED driver
  2015-04-26 12:15     ` [PATCH v4 0/2] " Álvaro Fernández Rojas
  2015-04-26 12:15       ` [PATCH v4 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
  2015-04-26 12:15       ` [PATCH v4 2/2] leds: add BCM6328 LED driver Álvaro Fernández Rojas
@ 2015-04-28 16:50       ` Álvaro Fernández Rojas
  2015-04-28 16:50         ` [PATCH v5 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
                           ` (2 more replies)
  2 siblings, 3 replies; 26+ messages in thread
From: Álvaro Fernández Rojas @ 2015-04-28 16:50 UTC (permalink / raw)
  To: linux-leds, devicetree, cooloney, jogo, f.fainelli, cernekee,
	j.anaszewski
  Cc: Álvaro Fernández Rojas

These patches add support and documentation for the BCM6328 LED driver, which is present on BCM6318, BCM6328, BCM6362 and BCM63268.
In these SoCs it's possible to control LEDs both as GPIOs or by hardware.
However, on some devices there are Serial LEDs (LEDs connected to a 74hc controller), which can either be controlled by software (exporting the 74hc as spi-gpio) or hardware using this driver.
The problem is some of these serial LEDs are hardware controlled (ethernet LEDs) and exporting the 74hc as spi-gpio prevents those LEDs to be hardware controlled, so the only chance to keep them working is by using this driver.

v4->v5: Introduce changes suggested by Jacek.
v3->v4: Introduce changes suggested by Jacek.
v2->v3: Introduce changes suggested by Jacek.
v1->v2: Introduce changes suggested by Florian and Jonas.

Álvaro Fernández Rojas (2):
  leds: add DT binding for BCM6328 LED controller
  leds: add BCM6328 LED driver

 .../devicetree/bindings/leds/leds-bcm6328.txt      | 309 +++++++++++++++
 drivers/leds/Kconfig                               |   8 +
 drivers/leds/Makefile                              |   1 +
 drivers/leds/leds-bcm6328.c                        | 413 +++++++++++++++++++++
 4 files changed, 731 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-bcm6328.txt
 create mode 100644 drivers/leds/leds-bcm6328.c

-- 
1.9.1

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v5 1/2] leds: add DT binding for BCM6328 LED controller
  2015-04-28 16:50       ` [PATCH v5 0/2] " Álvaro Fernández Rojas
@ 2015-04-28 16:50         ` Álvaro Fernández Rojas
  2015-04-28 16:50         ` [PATCH v5 2/2] leds: add BCM6328 LED driver Álvaro Fernández Rojas
       [not found]         ` <1430239850-26120-1-git-send-email-noltari-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2 siblings, 0 replies; 26+ messages in thread
From: Álvaro Fernández Rojas @ 2015-04-28 16:50 UTC (permalink / raw)
  To: linux-leds, devicetree, cooloney, jogo, f.fainelli, cernekee,
	j.anaszewski
  Cc: Álvaro Fernández Rojas

This adds device tree binding documentation for the Broadcom BCM6328 LED controller.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Signed-off-by: Jonas Gorski <jogo@openwrt.org>
---
 v5: resend, no changes.
 v4: introduce changes suggested by Jacek
 - Follow Documentation convention.
 v3: introduce changes suggested by Jacek
 - Revert compatible string to "brcm,bcm6328-leds".
 - Improved description.
 - Renamed normal LEDs to hardware controlled.
 - Explain that LEDs are active high by default on active-low optional property.
 - Properties are now grouped depending on the type of LED and optional requirement.
 - Added more examples to explain hardware controlled LEDs and activity/link selection.
 v2: Introduce changes suggested by Florian and Jonas.
 - Change compatible string to "brcm,bcm6328-leds-ctrl".
 - Make valid LEDs statement more strict.
 - Remove compatible strings from LED subnodes.
 - Clarify that LEDs are active high by default.
 - Add a better description for brcm,link-signal-sources and brcm,activity-signal-sources.

 .../devicetree/bindings/leds/leds-bcm6328.txt      | 309 +++++++++++++++++++++
 1 file changed, 309 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-bcm6328.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-bcm6328.txt b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
new file mode 100644
index 0000000..f9e36ad
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-bcm6328.txt
@@ -0,0 +1,309 @@
+LEDs connected to Broadcom BCM6328 controller
+
+This controller is present on BCM6318, BCM6328, BCM6362 and BCM63268.
+In these SoCs it's possible to control LEDs both as GPIOs or by hardware.
+However, on some devices there are Serial LEDs (LEDs connected to a 74x164
+controller), which can either be controlled by software (exporting the 74x164
+as spi-gpio. See Documentation/devicetree/bindings/gpio/gpio-74x164.txt), or
+by hardware using this driver.
+Some of these Serial LEDs are hardware controlled (e.g. ethernet LEDs) and
+exporting the 74x164 as spi-gpio prevents those LEDs to be hardware
+controlled, so the only chance to keep them working is by using this driver.
+
+BCM6328 LED controller has a HWDIS register, which controls whether a LED
+should be controlled by a hardware signal instead of the MODE register value,
+with 0 meaning hardware control enabled and 1 hardware control disabled. This
+is usually 1:1 for hardware to LED signals, but through the activity/link
+registers you have some limited control over rerouting the LEDs (as
+explained later in brcm,link-signal-sources). Even if a LED is hardware
+controlled you are still able to make it blink or light it up if it isn't,
+but you can't turn it off if the hardware decides to light it up. For this
+reason, hardware controlled LEDs aren't registered as LED class devices.
+
+Required properties:
+  - compatible : should be "brcm,bcm6328-leds".
+  - #address-cells : must be 1.
+  - #size-cells : must be 0.
+  - reg : BCM6328 LED controller address and size.
+
+Optional properties:
+  - brcm,serial-leds : Boolean, enables Serial LEDs.
+    Default : false
+
+Each LED is represented as a sub-node of the brcm,bcm6328-leds device.
+
+LED sub-node required properties:
+  - reg : LED pin number (only LEDs 0 to 23 are valid).
+
+LED sub-node optional properties:
+  a) Optional properties for sub-nodes related to software controlled LEDs:
+    - label : see Documentation/devicetree/bindings/leds/common.txt
+    - active-low : Boolean, makes LED active low.
+      Default : false
+    - default-state : see
+      Documentation/devicetree/bindings/leds/leds-gpio.txt
+    - linux,default-trigger : see
+      Documentation/devicetree/bindings/leds/common.txt
+
+  b) Optional properties for sub-nodes related to hardware controlled LEDs:
+    - brcm,hardware-controlled : Boolean, makes this LED hardware controlled.
+      Default : false
+    - brcm,link-signal-sources : An array of hardware link
+      signal sources. Up to four link hardware signals can get muxed into
+      these LEDs. Only valid for LEDs 0 to 7, where LED signals 0 to 3 may
+      be muxed to LEDs 0 to 3, and signals 4 to 7 may be muxed to LEDs
+      4 to 7. A signal can be muxed to more than one LED, and one LED can
+      have more than one source signal.
+    - brcm,activity-signal-sources : An array of hardware activity
+      signal sources. Up to four activity hardware signals can get muxed into
+      these LEDs. Only valid for LEDs 0 to 7, where LED signals 0 to 3 may
+      be muxed to LEDs 0 to 3, and signals 4 to 7 may be muxed to LEDs
+      4 to 7. A signal can be muxed to more than one LED, and one LED can
+      have more than one source signal.
+
+Examples:
+Scenario 1 : BCM6328 with 4 EPHY LEDs
+	leds0: led-controller@10000800 {
+		compatible = "brcm,bcm6328-leds";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x10000800 0x24>;
+
+		alarm_red@2 {
+			reg = <2>;
+			active-low;
+			label = "red:alarm";
+		};
+		inet_green@3 {
+			reg = <3>;
+			active-low;
+			label = "green:inet";
+		};
+		power_green@4 {
+			reg = <4>;
+			active-low;
+			label = "green:power";
+			default-state = "on";
+		};
+		ephy0_spd@17 {
+			reg = <17>;
+			brcm,hardware-controlled;
+		};
+		ephy1_spd@18 {
+			reg = <18>;
+			brcm,hardware-controlled;
+		};
+		ephy2_spd@19 {
+			reg = <19>;
+			brcm,hardware-controlled;
+		};
+		ephy3_spd@20 {
+			reg = <20>;
+			brcm,hardware-controlled;
+		};
+	};
+
+Scenario 2 : BCM63268 with Serial/GPHY0 LEDs
+	leds0: led-controller@10001900 {
+		compatible = "brcm,bcm6328-leds";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x10001900 0x24>;
+		brcm,serial-leds;
+
+		gphy0_spd0@0 {
+			reg = <0>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <0>;
+		};
+		gphy0_spd1@1 {
+			reg = <1>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <1>;
+		};
+		inet_red@2 {
+			reg = <2>;
+			active-low;
+			label = "red:inet";
+		};
+		dsl_green@3 {
+			reg = <3>;
+			active-low;
+			label = "green:dsl";
+		};
+		usb_green@4 {
+			reg = <4>;
+			active-low;
+			label = "green:usb";
+		};
+		wps_green@7 {
+			reg = <7>;
+			active-low;
+			label = "green:wps";
+		};
+		inet_green@8 {
+			reg = <8>;
+			active-low;
+			label = "green:inet";
+		};
+		ephy0_act@9 {
+			reg = <9>;
+			brcm,hardware-controlled;
+		};
+		ephy1_act@10 {
+			reg = <10>;
+			brcm,hardware-controlled;
+		};
+		ephy2_act@11 {
+			reg = <11>;
+			brcm,hardware-controlled;
+		};
+		gphy0_act@12 {
+			reg = <12>;
+			brcm,hardware-controlled;
+		};
+		ephy0_spd@13 {
+			reg = <13>;
+			brcm,hardware-controlled;
+		};
+		ephy1_spd@14 {
+			reg = <14>;
+			brcm,hardware-controlled;
+		};
+		ephy2_spd@15 {
+			reg = <15>;
+			brcm,hardware-controlled;
+		};
+		power_green@20 {
+			reg = <20>;
+			active-low;
+			label = "green:power";
+			default-state = "on";
+		};
+	};
+
+Scenario 3 : BCM6362 with 1 LED for each EPHY
+	leds0: led-controller@10001900 {
+		compatible = "brcm,bcm6328-leds";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x10001900 0x24>;
+
+		usb@0 {
+			reg = <0>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <0>;
+			brcm,activity-signal-sources = <0>;
+			/* USB link/activity routed to USB LED */
+		};
+		inet@1 {
+			reg = <1>;
+			brcm,hardware-controlled;
+			brcm,activity-signal-sources = <1>;
+			/* INET activity routed to INET LED */
+		};
+		ephy0@4 {
+			reg = <4>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <4>;
+			/* EPHY0 link routed to EPHY0 LED */
+		};
+		ephy1@5 {
+			reg = <5>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <5>;
+			/* EPHY1 link routed to EPHY1 LED */
+		};
+		ephy2@6 {
+			reg = <6>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <6>;
+			/* EPHY2 link routed to EPHY2 LED */
+		};
+		ephy3@7 {
+			reg = <7>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <7>;
+			/* EPHY3 link routed to EPHY3 LED */
+		};
+		power_green@20 {
+			reg = <20>;
+			active-low;
+			label = "green:power";
+			default-state = "on";
+		};
+	};
+
+Scenario 4 : BCM6362 with 1 LED for all EPHYs
+	leds0: led-controller@10001900 {
+		compatible = "brcm,bcm6328-leds";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x10001900 0x24>;
+
+		usb@0 {
+			reg = <0>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <0 1>;
+			brcm,activity-signal-sources = <0 1>;
+			/* USB/INET link/activity routed to USB LED */
+		};
+		ephy@4 {
+			reg = <4>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <4 5 6 7>;
+			/* EPHY0/1/2/3 link routed to EPHY0 LED */
+		};
+		power_green@20 {
+			reg = <20>;
+			active-low;
+			label = "green:power";
+			default-state = "on";
+		};
+	};
+
+Scenario 5 : BCM6362 with EPHY LEDs swapped
+	leds0: led-controller@10001900 {
+		compatible = "brcm,bcm6328-leds";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x10001900 0x24>;
+
+		usb@0 {
+			reg = <0>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <0>;
+			brcm,activity-signal-sources = <0 1>;
+			/* USB link/act and INET act routed to USB LED */
+		};
+		ephy0@4 {
+			reg = <4>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <7>;
+			/* EPHY3 link routed to EPHY0 LED */
+		};
+		ephy1@5 {
+			reg = <5>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <6>;
+			/* EPHY2 link routed to EPHY1 LED */
+		};
+		ephy2@6 {
+			reg = <6>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <5>;
+			/* EPHY1 link routed to EPHY2 LED */
+		};
+		ephy3@7 {
+			reg = <7>;
+			brcm,hardware-controlled;
+			brcm,link-signal-sources = <4>;
+			/* EPHY0 link routed to EPHY3 LED */
+		};
+		power_green@20 {
+			reg = <20>;
+			active-low;
+			label = "green:power";
+			default-state = "on";
+		};
+	};
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 2/2] leds: add BCM6328 LED driver
  2015-04-28 16:50       ` [PATCH v5 0/2] " Álvaro Fernández Rojas
  2015-04-28 16:50         ` [PATCH v5 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
@ 2015-04-28 16:50         ` Álvaro Fernández Rojas
       [not found]         ` <1430239850-26120-1-git-send-email-noltari-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2 siblings, 0 replies; 26+ messages in thread
From: Álvaro Fernández Rojas @ 2015-04-28 16:50 UTC (permalink / raw)
  To: linux-leds, devicetree, cooloney, jogo, f.fainelli, cernekee,
	j.anaszewski
  Cc: Álvaro Fernández Rojas

This adds support for the LED controller on Broadcom's BCM6328.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Signed-off-by: Jonas Gorski <jogo@openwrt.org>
---
 v5: introduce changes suggested by Jacek
 - Set active_low true instead of 1.
 v4: resend, no changes.
 v3: introduce changes suggested by Jacek
 - Revert compatible string to "brcm,bcm6328-leds".
 - Add BCM6328_ prefix to macros.
 - Rename blink_del to blink_delay.
 v2: introduce changes suggested by Florian and Jonas.
 - Improve Kconfig description.
 - Remove unused imports.
 - Fix bug in blink_set so it works for multiple LEDs.
 - "brcm,link-selection" -> "brcm,link-signal-sources".
 - "brcm,activity-selection" -> "brcm,activity-signal-sources".
 - Use of_property_read_bool instead of of_get_property for booleans.
 - Add MODULE_AUTHOR strings.

 drivers/leds/Kconfig        |   8 +
 drivers/leds/Makefile       |   1 +
 drivers/leds/leds-bcm6328.c | 413 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 422 insertions(+)
 create mode 100644 drivers/leds/leds-bcm6328.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 966b960..caa8ee6 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -42,6 +42,14 @@ config LEDS_88PM860X
 	  This option enables support for on-chip LED drivers found on Marvell
 	  Semiconductor 88PM8606 PMIC.
 
+config LEDS_BCM6328
+	tristate "LED Support for Broadcom BCM6328"
+	depends on LEDS_CLASS
+	depends on OF
+	help
+	  This option enables support for LEDs connected to the BCM6328
+	  LED HW controller accessed via MMIO registers.
+
 config LEDS_LM3530
 	tristate "LCD Backlight driver for LM3530"
 	depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index bf46093..309a46b 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_LEDS_TRIGGERS)		+= led-triggers.o
 
 # LED Platform Drivers
 obj-$(CONFIG_LEDS_88PM860X)		+= leds-88pm860x.o
+obj-$(CONFIG_LEDS_BCM6328)		+= leds-bcm6328.o
 obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
 obj-$(CONFIG_LEDS_LOCOMO)		+= leds-locomo.o
 obj-$(CONFIG_LEDS_LM3530)		+= leds-lm3530.o
diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
new file mode 100644
index 0000000..986fe1e
--- /dev/null
+++ b/drivers/leds/leds-bcm6328.c
@@ -0,0 +1,413 @@
+/*
+ * Driver for BCM6328 memory-mapped LEDs, based on leds-syscon.c
+ *
+ * Copyright 2015 Álvaro Fernández Rojas <noltari@gmail.com>
+ * Copyright 2015 Jonas Gorski <jogo@openwrt.org>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+#include <linux/io.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+
+#define BCM6328_REG_INIT		0x00
+#define BCM6328_REG_MODE_HI		0x04
+#define BCM6328_REG_MODE_LO		0x08
+#define BCM6328_REG_HWDIS		0x0c
+#define BCM6328_REG_STROBE		0x10
+#define BCM6328_REG_LNKACTSEL_HI	0x14
+#define BCM6328_REG_LNKACTSEL_LO	0x18
+#define BCM6328_REG_RBACK		0x1c
+#define BCM6328_REG_SERMUX		0x20
+
+#define BCM6328_LED_MAX_COUNT		24
+#define BCM6328_LED_DEF_DELAY		500
+#define BCM6328_LED_INTERVAL_MS		20
+
+#define BCM6328_LED_INTV_MASK		0x3f
+#define BCM6328_LED_FAST_INTV_SHIFT	6
+#define BCM6328_LED_FAST_INTV_MASK	(BCM6328_LED_INTV_MASK << \
+					 BCM6328_LED_FAST_INTV_SHIFT)
+#define BCM6328_SERIAL_LED_EN		BIT(12)
+#define BCM6328_SERIAL_LED_MUX		BIT(13)
+#define BCM6328_SERIAL_LED_CLK_NPOL	BIT(14)
+#define BCM6328_SERIAL_LED_DATA_PPOL	BIT(15)
+#define BCM6328_SERIAL_LED_SHIFT_DIR	BIT(16)
+#define BCM6328_LED_SHIFT_TEST		BIT(30)
+#define BCM6328_LED_TEST		BIT(31)
+
+#define BCM6328_LED_MODE_MASK		3
+#define BCM6328_LED_MODE_OFF		0
+#define BCM6328_LED_MODE_FAST		1
+#define BCM6328_LED_MODE_BLINK		2
+#define BCM6328_LED_MODE_ON		3
+#define BCM6328_LED_SHIFT(X)		((X) << 1)
+
+/**
+ * struct bcm6328_led - state container for bcm6328 based LEDs
+ * @cdev: LED class device for this LED
+ * @mem: memory resource
+ * @lock: memory lock
+ * @pin: LED pin number
+ * @blink_leds: blinking LEDs
+ * @blink_delay: blinking delay
+ * @active_low: LED is active low
+ */
+struct bcm6328_led {
+	struct led_classdev cdev;
+	void __iomem *mem;
+	spinlock_t *lock;
+	unsigned long pin;
+	unsigned long *blink_leds;
+	unsigned long *blink_delay;
+	bool active_low;
+};
+
+static void bcm6328_led_write(void __iomem *reg, unsigned long data)
+{
+	iowrite32be(data, reg);
+}
+
+static unsigned long bcm6328_led_read(void __iomem *reg)
+{
+	return ioread32be(reg);
+}
+
+/**
+ * LEDMode 64 bits / 24 LEDs
+ * bits [31:0] -> LEDs 8-23
+ * bits [47:32] -> LEDs 0-7
+ * bits [63:48] -> unused
+ */
+static unsigned long bcm6328_pin2shift(unsigned long pin)
+{
+	if (pin < 8)
+		return pin + 16; /* LEDs 0-7 (bits 47:32) */
+	else
+		return pin - 8; /* LEDs 8-23 (bits 31:0) */
+}
+
+static void bcm6328_led_mode(struct bcm6328_led *led, unsigned long value)
+{
+	void __iomem *mode;
+	unsigned long val, shift;
+
+	shift = bcm6328_pin2shift(led->pin);
+	if (shift / 16)
+		mode = led->mem + BCM6328_REG_MODE_HI;
+	else
+		mode = led->mem + BCM6328_REG_MODE_LO;
+
+	val = bcm6328_led_read(mode);
+	val &= ~(BCM6328_LED_MODE_MASK << BCM6328_LED_SHIFT(shift % 16));
+	val |= (value << BCM6328_LED_SHIFT(shift % 16));
+	bcm6328_led_write(mode, val);
+}
+
+static void bcm6328_led_set(struct led_classdev *led_cdev,
+			    enum led_brightness value)
+{
+	struct bcm6328_led *led =
+		container_of(led_cdev, struct bcm6328_led, cdev);
+	unsigned long flags;
+
+	spin_lock_irqsave(led->lock, flags);
+	*(led->blink_leds) &= ~BIT(led->pin);
+	if ((led->active_low && value == LED_OFF) ||
+	    (!led->active_low && value != LED_OFF))
+		bcm6328_led_mode(led, BCM6328_LED_MODE_OFF);
+	else
+		bcm6328_led_mode(led, BCM6328_LED_MODE_ON);
+	spin_unlock_irqrestore(led->lock, flags);
+}
+
+static int bcm6328_blink_set(struct led_classdev *led_cdev,
+			     unsigned long *delay_on, unsigned long *delay_off)
+{
+	struct bcm6328_led *led =
+		container_of(led_cdev, struct bcm6328_led, cdev);
+	unsigned long delay, flags;
+
+	if (!*delay_on)
+		*delay_on = BCM6328_LED_DEF_DELAY;
+	if (!*delay_off)
+		*delay_off = BCM6328_LED_DEF_DELAY;
+
+	if (*delay_on != *delay_off) {
+		dev_dbg(led_cdev->dev,
+			"fallback to soft blinking (delay_on != delay_off)\n");
+		return -EINVAL;
+	}
+
+	delay = *delay_on / BCM6328_LED_INTERVAL_MS;
+	if (delay == 0)
+		delay = 1;
+	else if (delay > BCM6328_LED_INTV_MASK) {
+		dev_dbg(led_cdev->dev,
+			"fallback to soft blinking (delay > %ums)\n",
+			BCM6328_LED_INTV_MASK * BCM6328_LED_INTERVAL_MS);
+		return -EINVAL;
+	}
+
+	spin_lock_irqsave(led->lock, flags);
+	if (*(led->blink_leds) == 0 ||
+	    *(led->blink_leds) == BIT(led->pin) ||
+	    *(led->blink_delay) == delay) {
+		unsigned long val;
+
+		*(led->blink_leds) |= BIT(led->pin);
+		*(led->blink_delay) = delay;
+
+		val = bcm6328_led_read(led->mem + BCM6328_REG_INIT);
+		val &= ~BCM6328_LED_FAST_INTV_MASK;
+		val |= (delay << BCM6328_LED_FAST_INTV_SHIFT);
+		bcm6328_led_write(led->mem + BCM6328_REG_INIT, val);
+
+		bcm6328_led_mode(led, BCM6328_LED_MODE_BLINK);
+
+		spin_unlock_irqrestore(led->lock, flags);
+	} else {
+		spin_unlock_irqrestore(led->lock, flags);
+		dev_dbg(led_cdev->dev,
+			"fallback to soft blinking (delay already set)\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int bcm6328_hwled(struct device *dev, struct device_node *nc, u32 reg,
+			 void __iomem *mem, spinlock_t *lock)
+{
+	int i, cnt;
+	unsigned long flags, val;
+
+	spin_lock_irqsave(lock, flags);
+	val = bcm6328_led_read(mem + BCM6328_REG_HWDIS);
+	val &= ~BIT(reg);
+	bcm6328_led_write(mem + BCM6328_REG_HWDIS, val);
+	spin_unlock_irqrestore(lock, flags);
+
+	/* Only LEDs 0-7 can be activity/link controlled */
+	if (reg >= 8)
+		return 0;
+
+	cnt = of_property_count_elems_of_size(nc, "brcm,link-signal-sources",
+					      sizeof(u32));
+	for (i = 0; i < cnt; i++) {
+		u32 sel;
+		void __iomem *addr;
+
+		if (reg < 4)
+			addr = mem + BCM6328_REG_LNKACTSEL_LO;
+		else
+			addr = mem + BCM6328_REG_LNKACTSEL_HI;
+
+		of_property_read_u32_index(nc, "brcm,link-signal-sources", i,
+					   &sel);
+
+		if (reg / 4 != sel / 4) {
+			dev_warn(dev, "invalid link signal source\n");
+			continue;
+		}
+
+		spin_lock_irqsave(lock, flags);
+		val = bcm6328_led_read(addr);
+		val |= (BIT(reg) << (((sel % 4) * 4) + 16));
+		bcm6328_led_write(addr, val);
+		spin_unlock_irqrestore(lock, flags);
+	}
+
+	cnt = of_property_count_elems_of_size(nc,
+					      "brcm,activity-signal-sources",
+					      sizeof(u32));
+	for (i = 0; i < cnt; i++) {
+		u32 sel;
+		void __iomem *addr;
+
+		if (reg < 4)
+			addr = mem + BCM6328_REG_LNKACTSEL_LO;
+		else
+			addr = mem + BCM6328_REG_LNKACTSEL_HI;
+
+		of_property_read_u32_index(nc, "brcm,activity-signal-sources",
+					   i, &sel);
+
+		if (reg / 4 != sel / 4) {
+			dev_warn(dev, "invalid activity signal source\n");
+			continue;
+		}
+
+		spin_lock_irqsave(lock, flags);
+		val = bcm6328_led_read(addr);
+		val |= (BIT(reg) << ((sel % 4) * 4));
+		bcm6328_led_write(addr, val);
+		spin_unlock_irqrestore(lock, flags);
+	}
+
+	return 0;
+}
+
+static int bcm6328_led(struct device *dev, struct device_node *nc, u32 reg,
+		       void __iomem *mem, spinlock_t *lock,
+		       unsigned long *blink_leds, unsigned long *blink_delay)
+{
+	struct bcm6328_led *led;
+	unsigned long flags;
+	const char *state;
+	int rc;
+
+	led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+
+	led->pin = reg;
+	led->mem = mem;
+	led->lock = lock;
+	led->blink_leds = blink_leds;
+	led->blink_delay = blink_delay;
+
+	if (of_property_read_bool(nc, "active-low"))
+		led->active_low = true;
+
+	led->cdev.name = of_get_property(nc, "label", NULL) ? : nc->name;
+	led->cdev.default_trigger = of_get_property(nc,
+						    "linux,default-trigger",
+						    NULL);
+
+	if (!of_property_read_string(nc, "default-state", &state)) {
+		spin_lock_irqsave(lock, flags);
+		if (!strcmp(state, "on")) {
+			led->cdev.brightness = LED_FULL;
+			bcm6328_led_mode(led, BCM6328_LED_MODE_ON);
+		} else if (!strcmp(state, "keep")) {
+			void __iomem *mode;
+			unsigned long val, shift;
+
+			shift = bcm6328_pin2shift(led->pin);
+			if (shift / 16)
+				mode = mem + BCM6328_REG_MODE_HI;
+			else
+				mode = mem + BCM6328_REG_MODE_LO;
+
+			val = bcm6328_led_read(mode) >> (shift % 16);
+			val &= BCM6328_LED_MODE_MASK;
+			if (val == BCM6328_LED_MODE_ON)
+				led->cdev.brightness = LED_FULL;
+			else {
+				led->cdev.brightness = LED_OFF;
+				bcm6328_led_mode(led, BCM6328_LED_MODE_OFF);
+			}
+		} else {
+			led->cdev.brightness = LED_OFF;
+			bcm6328_led_mode(led, BCM6328_LED_MODE_OFF);
+		}
+		spin_unlock_irqrestore(lock, flags);
+	}
+
+	led->cdev.brightness_set = bcm6328_led_set;
+	led->cdev.blink_set = bcm6328_blink_set;
+
+	rc = led_classdev_register(dev, &led->cdev);
+	if (rc < 0)
+		return rc;
+
+	dev_dbg(dev, "registered LED %s\n", led->cdev.name);
+
+	return 0;
+}
+
+static int bcm6328_leds_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *child;
+	struct resource *mem_r;
+	void __iomem *mem;
+	spinlock_t *lock;
+	unsigned long val, *blink_leds, *blink_delay;
+
+	mem_r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mem_r)
+		return -EINVAL;
+
+	mem = devm_ioremap_resource(dev, mem_r);
+	if (IS_ERR(mem))
+		return PTR_ERR(mem);
+
+	lock = devm_kzalloc(dev, sizeof(*lock), GFP_KERNEL);
+	if (!lock)
+		return -ENOMEM;
+
+	blink_leds = devm_kzalloc(dev, sizeof(*blink_leds), GFP_KERNEL);
+	if (!blink_leds)
+		return -ENOMEM;
+
+	blink_delay = devm_kzalloc(dev, sizeof(*blink_delay), GFP_KERNEL);
+	if (!blink_delay)
+		return -ENOMEM;
+
+	spin_lock_init(lock);
+
+	bcm6328_led_write(mem + BCM6328_REG_HWDIS, ~0);
+	bcm6328_led_write(mem + BCM6328_REG_LNKACTSEL_HI, 0);
+	bcm6328_led_write(mem + BCM6328_REG_LNKACTSEL_LO, 0);
+
+	val = bcm6328_led_read(mem + BCM6328_REG_INIT);
+	val &= ~BCM6328_SERIAL_LED_EN;
+	if (of_property_read_bool(np, "brcm,serial-leds"))
+		val |= BCM6328_SERIAL_LED_EN;
+	bcm6328_led_write(mem + BCM6328_REG_INIT, val);
+
+	for_each_available_child_of_node(np, child) {
+		int rc;
+		u32 reg;
+
+		if (of_property_read_u32(child, "reg", &reg))
+			continue;
+
+		if (reg >= BCM6328_LED_MAX_COUNT) {
+			dev_err(dev, "invalid LED (>= %d)\n",
+				BCM6328_LED_MAX_COUNT);
+			continue;
+		}
+
+		if (of_property_read_bool(child, "brcm,hardware-controlled"))
+			rc = bcm6328_hwled(dev, child, reg, mem, lock);
+		else
+			rc = bcm6328_led(dev, child, reg, mem, lock,
+					 blink_leds, blink_delay);
+
+		if (rc < 0)
+			return rc;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id bcm6328_leds_of_match[] = {
+	{ .compatible = "brcm,bcm6328-leds", },
+	{ },
+};
+
+static struct platform_driver bcm6328_leds_driver = {
+	.probe = bcm6328_leds_probe,
+	.driver = {
+		.name = "leds-bcm6328",
+		.of_match_table = bcm6328_leds_of_match,
+	},
+};
+
+module_platform_driver(bcm6328_leds_driver);
+
+MODULE_AUTHOR("Álvaro Fernández Rojas <noltari@gmail.com>");
+MODULE_AUTHOR("Jonas Gorski <jogo@openwrt.org>");
+MODULE_DESCRIPTION("LED driver for BCM6328 controllers");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:leds-bcm6328");
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH v5 0/2] BCM6328 LED driver
       [not found]         ` <1430239850-26120-1-git-send-email-noltari-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2015-04-29  0:46           ` Bryan Wu
  0 siblings, 0 replies; 26+ messages in thread
From: Bryan Wu @ 2015-04-29  0:46 UTC (permalink / raw)
  To: Álvaro Fernández Rojas
  Cc: Linux LED Subsystem, devicetree-u79uwXL29TY76Z2rM5mHXA,
	jogo-p3rKhJxN3npAfugRpC6u6w, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
	cernekee-Re5JQEeQqe8AvxtiuMwx3w, Jacek Anaszewski

On Tue, Apr 28, 2015 at 9:50 AM, Álvaro Fernández Rojas
<noltari-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> These patches add support and documentation for the BCM6328 LED driver, which is present on BCM6318, BCM6328, BCM6362 and BCM63268.
> In these SoCs it's possible to control LEDs both as GPIOs or by hardware.
> However, on some devices there are Serial LEDs (LEDs connected to a 74hc controller), which can either be controlled by software (exporting the 74hc as spi-gpio) or hardware using this driver.
> The problem is some of these serial LEDs are hardware controlled (ethernet LEDs) and exporting the 74hc as spi-gpio prevents those LEDs to be hardware controlled, so the only chance to keep them working is by using this driver.
>
> v4->v5: Introduce changes suggested by Jacek.
> v3->v4: Introduce changes suggested by Jacek.
> v2->v3: Introduce changes suggested by Jacek.
> v1->v2: Introduce changes suggested by Florian and Jonas.
>

Applied to my tree, thanks.
-Bryan

> Álvaro Fernández Rojas (2):
>   leds: add DT binding for BCM6328 LED controller
>   leds: add BCM6328 LED driver
>
>  .../devicetree/bindings/leds/leds-bcm6328.txt      | 309 +++++++++++++++
>  drivers/leds/Kconfig                               |   8 +
>  drivers/leds/Makefile                              |   1 +
>  drivers/leds/leds-bcm6328.c                        | 413 +++++++++++++++++++++
>  4 files changed, 731 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-bcm6328.txt
>  create mode 100644 drivers/leds/leds-bcm6328.c
>
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2015-04-29  0:46 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-02 11:54 [PATCH 0/2] BCM6328 LED driver Álvaro Fernández Rojas
2015-04-02 11:54 ` [PATCH 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
2015-04-03  2:06   ` Florian Fainelli
2015-04-03 12:19     ` Álvaro Fernández Rojas
2015-04-03 14:46       ` Jonas Gorski
2015-04-02 11:54 ` [PATCH 2/2] leds: add BCM6328 LED driver Álvaro Fernández Rojas
2015-04-05 15:08 ` [PATCH v2 0/2] " Álvaro Fernández Rojas
2015-04-05 15:08   ` [PATCH v2 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
2015-04-16 14:39     ` Jacek Anaszewski
2015-04-05 15:08   ` [PATCH 2/2] leds: add BCM6328 LED driver Álvaro Fernández Rojas
2015-04-16 14:37     ` Jacek Anaszewski
2015-04-18  9:14       ` Jonas Gorski
2015-04-20  7:12         ` Jacek Anaszewski
2015-04-24 17:06   ` [PATCH v3 0/2] " Álvaro Fernández Rojas
2015-04-24 17:06     ` [PATCH v3 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
2015-04-26 11:09       ` Jacek Anaszewski
2015-04-24 17:06     ` [PATCH v3 2/2] leds: add BCM6328 LED driver Álvaro Fernández Rojas
2015-04-26 12:15     ` [PATCH v4 0/2] " Álvaro Fernández Rojas
2015-04-26 12:15       ` [PATCH v4 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
2015-04-28  8:18         ` Jacek Anaszewski
2015-04-26 12:15       ` [PATCH v4 2/2] leds: add BCM6328 LED driver Álvaro Fernández Rojas
2015-04-28  8:18         ` Jacek Anaszewski
2015-04-28 16:50       ` [PATCH v5 0/2] " Álvaro Fernández Rojas
2015-04-28 16:50         ` [PATCH v5 1/2] leds: add DT binding for BCM6328 LED controller Álvaro Fernández Rojas
2015-04-28 16:50         ` [PATCH v5 2/2] leds: add BCM6328 LED driver Álvaro Fernández Rojas
     [not found]         ` <1430239850-26120-1-git-send-email-noltari-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-04-29  0:46           ` [PATCH v5 0/2] " Bryan Wu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.