Re: [PATCH v2 1/2] leds: add DT binding for BCM6328 LED controller

Re: [PATCH v2 1/2] leds: add DT binding for BCM6328 LED controller

[Jacek Anaszewski]

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

Re: [PATCH v3 1/2] leds: add DT binding for BCM6328 LED controller

[Jacek Anaszewski]

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

[PATCH v4 0/2] BCM6328 LED driver

[Á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

[PATCH v4 1/2] leds: add DT binding for BCM6328 LED controller

[Á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

[PATCH v4 2/2] leds: add BCM6328 LED driver

[Á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

Re: [PATCH v4 2/2] leds: add BCM6328 LED driver

[Jacek Anaszewski]

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

Re: [PATCH v4 1/2] leds: add DT binding for BCM6328 LED controller

[Jacek Anaszewski]

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

[PATCH v5 0/2] BCM6328 LED driver

[Á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

[PATCH v5 1/2] leds: add DT binding for BCM6328 LED controller

[Á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

[PATCH v5 2/2] leds: add BCM6328 LED driver

[Á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

Re: [PATCH v5 0/2] BCM6328 LED driver

[Bryan Wu]

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