[PATCH V3 0/2] usb: introduce "trigger-sources" DT property for usbport trigger

[PATCH V3 0/2] usb: introduce "trigger-sources" DT property for usbport trigger

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

Hi,

This is another try of adding relation between LEDs and devices to the DT (and
usbport). I followed discussions in all old threads and came with this V3.

I'd epxect both patches to go through Greg's usb.git if accepted.

For a reference (and before someome comes with already rejected solution) see
below history of this work:

1) [PATCH V2] leds: trigger: Introduce an USB port trigger
My initial try of adding "usb-ports" property. Rob said it should be more
generic.

2) [PATCH 1/2] dt-bindings: leds: document new usb-ports property
Rob still didn't like it due to being USB specific.

3) [PATCH V2 1/2] dt-bindings: leds: document new led-triggers property
Jacek didn't like it's more generic than what Linux can already support. Later
we agreed (?) that Linux limitations shouldn't influence DT structure.
Jacek still wanted a specific trigger but Rob said he won't accept something
specific to the USB.
Jacek suggested "trigger-sources" and Rob agreed on the name.
Jacek suggested trying triggers as separated nodes.

4) [PATCH 1/4] dt-bindings: leds: document property for LED triggers
This patch was trying to use separated nodes for triggers, e.g.:
foo-trigger { trigger-type = "foo"; };
Rob didn't like this extra level of indirection. He said Linux drivers shouldn't
define/influence the binding.

So finally I came with this patchset. It doesn't try to use separated nodes for
triggers anymore and it uses the name that was mostly accepted I think. It
should be generic and not influenced by Linux design.

Rafał Miłecki (3):
  dt-bindings: leds: document new trigger-sources property
  usb: core: read USB ports from DT in the usbport LED trigger driver
  ARM: BCM53573: Specify ports for USB LED for Tenda AC9

 Documentation/devicetree/bindings/leds/common.txt | 18 ++++++++
 arch/arm/boot/dts/bcm47189-tenda-ac9.dts          |  1 +
 arch/arm/boot/dts/bcm53573.dtsi                   |  4 ++
 drivers/usb/core/ledtrig-usbport.c                | 55 +++++++++++++++++++++++
 4 files changed, 78 insertions(+)

-- 
2.11.0

[PATCH V3 1/2] dt-bindings: leds: document new trigger-sources property

[Rafał Miłecki]

From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

Some LEDs can be related to a specific device(s) described in the DT.
This property allows specifying such relations. E.g. USB LED should
usually be used to indicate some USB port(s) state.

Please note this binding is designed to be generic and not influenced by
any operating system design. Linux developers may find "trigger" part a
bit confusing since in Linux triggers are separated drivers. It
shouldn't define the binding though (we shouldn't add an extra level of
indirection).

Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
---
V2: Replace "usb-ports" with "led-triggers" property which is more generic and
    allows specifying other devices as well.
V3: Use "trigger-sources" which is even more accurate as devices aren't
    precisely triggers.
---
 Documentation/devicetree/bindings/leds/common.txt | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
index 24b656014089..e6e300975a4c 100644
--- a/Documentation/devicetree/bindings/leds/common.txt
+++ b/Documentation/devicetree/bindings/leds/common.txt
@@ -49,6 +49,19 @@ Optional properties for child nodes:
 - panic-indicator : This property specifies that the LED should be used,
 		    if at all possible, as a panic indicator.
 
+- trigger-sources : List of devices which should be used as a source triggering
+		    this LED activity. Some LEDs can be related to a specific
+		    device and should somehow indicate its state. E.g. USB 2.0
+		    LED may react to device(s) in a USB 2.0 port(s).
+		    Another common example is switch or router with multiple
+		    Ethernet ports each of them having its own LED assigned
+		    (assuming they are not hardwired). In such cases this
+		    property should contain phandle(s) of related source
+		    device(s).
+		    In many cases LED can be related to more than one device
+		    (e.g. one USB LED vs. multiple USB ports) so a list of
+		    sources can be specified.
+
 Required properties for flash LED child nodes:
 - flash-max-microamp : Maximum flash LED supply current in microamperes.
 - flash-max-timeout-us : Maximum timeout in microseconds after which the flash
@@ -69,6 +82,11 @@ gpio-leds {
 		linux,default-trigger = "heartbeat";
 		gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>;
 	};
+
+	usb {
+		gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
+		led-triggers = <&ohci_port1>, <&ehci_port1>;
+	};
 };
 
 max77693-led {
-- 
2.11.0

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

Re: [PATCH V3 1/2] dt-bindings: leds: document new trigger-sources property

[Jacek Anaszewski]

Hi Rafał,

On 05/29/2017 04:01 PM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Some LEDs can be related to a specific device(s) described in the DT.
> This property allows specifying such relations. E.g. USB LED should
> usually be used to indicate some USB port(s) state.
> 
> Please note this binding is designed to be generic and not influenced by
> any operating system design. Linux developers may find "trigger" part a
> bit confusing since in Linux triggers are separated drivers. It
> shouldn't define the binding though (we shouldn't add an extra level of
> indirection).
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V2: Replace "usb-ports" with "led-triggers" property which is more generic and
>     allows specifying other devices as well.
> V3: Use "trigger-sources" which is even more accurate as devices aren't
>     precisely triggers.
> ---
>  Documentation/devicetree/bindings/leds/common.txt | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
> index 24b656014089..e6e300975a4c 100644
> --- a/Documentation/devicetree/bindings/leds/common.txt
> +++ b/Documentation/devicetree/bindings/leds/common.txt
> @@ -49,6 +49,19 @@ Optional properties for child nodes:
>  - panic-indicator : This property specifies that the LED should be used,
>  		    if at all possible, as a panic indicator.
>  
> +- trigger-sources : List of devices which should be used as a source triggering
> +		    this LED activity. Some LEDs can be related to a specific
> +		    device and should somehow indicate its state. E.g. USB 2.0
> +		    LED may react to device(s) in a USB 2.0 port(s).
> +		    Another common example is switch or router with multiple
> +		    Ethernet ports each of them having its own LED assigned
> +		    (assuming they are not hardwired). In such cases this
> +		    property should contain phandle(s) of related source
> +		    device(s).
> +		    In many cases LED can be related to more than one device
> +		    (e.g. one USB LED vs. multiple USB ports) so a list of
> +		    sources can be specified.
> +
>  Required properties for flash LED child nodes:
>  - flash-max-microamp : Maximum flash LED supply current in microamperes.
>  - flash-max-timeout-us : Maximum timeout in microseconds after which the flash
> @@ -69,6 +82,11 @@ gpio-leds {
>  		linux,default-trigger = "heartbeat";
>  		gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>;
>  	};
> +
> +	usb {
> +		gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
> +		led-triggers = <&ohci_port1>, <&ehci_port1>;

Didn't you mean "trigger-sources" here instead?

> +	};
>  };
>  
>  max77693-led {
> 

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH V3 1/2] dt-bindings: leds: document new trigger-sources property

[Rafał Miłecki]

On 29 May 2017 at 21:52, Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:
> On 05/29/2017 04:01 PM, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> Some LEDs can be related to a specific device(s) described in the DT.
>> This property allows specifying such relations. E.g. USB LED should
>> usually be used to indicate some USB port(s) state.
>>
>> Please note this binding is designed to be generic and not influenced by
>> any operating system design. Linux developers may find "trigger" part a
>> bit confusing since in Linux triggers are separated drivers. It
>> shouldn't define the binding though (we shouldn't add an extra level of
>> indirection).
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>> V2: Replace "usb-ports" with "led-triggers" property which is more generic and
>>     allows specifying other devices as well.
>> V3: Use "trigger-sources" which is even more accurate as devices aren't
>>     precisely triggers.
>> ---
>>  Documentation/devicetree/bindings/leds/common.txt | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
>> index 24b656014089..e6e300975a4c 100644
>> --- a/Documentation/devicetree/bindings/leds/common.txt
>> +++ b/Documentation/devicetree/bindings/leds/common.txt
>> @@ -49,6 +49,19 @@ Optional properties for child nodes:
>>  - panic-indicator : This property specifies that the LED should be used,
>>                   if at all possible, as a panic indicator.
>>
>> +- trigger-sources : List of devices which should be used as a source triggering
>> +                 this LED activity. Some LEDs can be related to a specific
>> +                 device and should somehow indicate its state. E.g. USB 2.0
>> +                 LED may react to device(s) in a USB 2.0 port(s).
>> +                 Another common example is switch or router with multiple
>> +                 Ethernet ports each of them having its own LED assigned
>> +                 (assuming they are not hardwired). In such cases this
>> +                 property should contain phandle(s) of related source
>> +                 device(s).
>> +                 In many cases LED can be related to more than one device
>> +                 (e.g. one USB LED vs. multiple USB ports) so a list of
>> +                 sources can be specified.
>> +
>>  Required properties for flash LED child nodes:
>>  - flash-max-microamp : Maximum flash LED supply current in microamperes.
>>  - flash-max-timeout-us : Maximum timeout in microseconds after which the flash
>> @@ -69,6 +82,11 @@ gpio-leds {
>>               linux,default-trigger = "heartbeat";
>>               gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>;
>>       };
>> +
>> +     usb {
>> +             gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
>> +             led-triggers = <&ohci_port1>, <&ehci_port1>;
>
> Didn't you mean "trigger-sources" here instead?

Oh, thanks for catching that.

-- 
Rafał

[PATCH V4 0/2] usb: introduce "trigger-sources" DT property for usbport trigger

[Rafał Miłecki]

From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

This patchset (V4) differs by only a tiny fix in 1/2 changing property used
in the DT example.

I'd epxect both patches to go through Greg's usb.git if accepted.

For a reference (and before someome comes with already rejected solution) see
below history of this work:

1) [PATCH V2] leds: trigger: Introduce an USB port trigger
My initial try of adding "usb-ports" property. Rob said it should be more
generic.

2) [PATCH 1/2] dt-bindings: leds: document new usb-ports property
Rob still didn't like it due to being USB specific.

3) [PATCH V2 1/2] dt-bindings: leds: document new led-triggers property
Jacek didn't like it's more generic than what Linux can already support. Later
we agreed (?) that Linux limitations shouldn't influence DT structure.
Jacek still wanted a specific trigger but Rob said he won't accept something
specific to the USB.
Jacek suggested "trigger-sources" and Rob agreed on the name.
Jacek suggested trying triggers as separated nodes.

4) [PATCH 1/4] dt-bindings: leds: document property for LED triggers
This patch was trying to use separated nodes for triggers, e.g.:
foo-trigger { trigger-type = "foo"; };
Rob didn't like this extra level of indirection. He said Linux drivers shouldn't
define/influence the binding.

So finally I came with this patchset. It doesn't try to use separated nodes for
triggers anymore and it uses the name that was mostly accepted I think. It
should be generic and not influenced by Linux design.

Rafał Miłecki (3):
  dt-bindings: leds: document new trigger-sources property
  usb: core: read USB ports from DT in the usbport LED trigger driver
  ARM: BCM53573: Specify ports for USB LED for Tenda AC9

 Documentation/devicetree/bindings/leds/common.txt | 18 ++++++++
 arch/arm/boot/dts/bcm47189-tenda-ac9.dts          |  1 +
 arch/arm/boot/dts/bcm53573.dtsi                   |  4 ++
 drivers/usb/core/ledtrig-usbport.c                | 55 +++++++++++++++++++++++
 4 files changed, 78 insertions(+)

-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V4 1/2] dt-bindings: leds: document new trigger-sources property

[Rafał Miłecki]

From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

Some LEDs can be related to a specific device(s) described in the DT.
This property allows specifying such relations. E.g. USB LED should
usually be used to indicate some USB port(s) state.

Please note this binding is designed to be generic and not influenced by
any operating system design. Linux developers may find "trigger" part a
bit confusing since in Linux triggers are separated drivers. It
shouldn't define the binding though (we shouldn't add an extra level of
indirection).

Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
---
V2: Replace "usb-ports" with "led-triggers" property which is more generic and
    allows specifying other devices as well.
V3: Use "trigger-sources" which is even more accurate as devices aren't
    precisely triggers.
V4: Update example to use the correct property
---
 Documentation/devicetree/bindings/leds/common.txt | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
index 24b656014089..7efaa2cff624 100644
--- a/Documentation/devicetree/bindings/leds/common.txt
+++ b/Documentation/devicetree/bindings/leds/common.txt
@@ -49,6 +49,19 @@ Optional properties for child nodes:
 - panic-indicator : This property specifies that the LED should be used,
 		    if at all possible, as a panic indicator.
 
+- trigger-sources : List of devices which should be used as a source triggering
+		    this LED activity. Some LEDs can be related to a specific
+		    device and should somehow indicate its state. E.g. USB 2.0
+		    LED may react to device(s) in a USB 2.0 port(s).
+		    Another common example is switch or router with multiple
+		    Ethernet ports each of them having its own LED assigned
+		    (assuming they are not hardwired). In such cases this
+		    property should contain phandle(s) of related source
+		    device(s).
+		    In many cases LED can be related to more than one device
+		    (e.g. one USB LED vs. multiple USB ports) so a list of
+		    sources can be specified.
+
 Required properties for flash LED child nodes:
 - flash-max-microamp : Maximum flash LED supply current in microamperes.
 - flash-max-timeout-us : Maximum timeout in microseconds after which the flash
@@ -69,6 +82,11 @@ gpio-leds {
 		linux,default-trigger = "heartbeat";
 		gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>;
 	};
+
+	usb {
+		gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
+		trigger-sources = <&ohci_port1>, <&ehci_port1>;
+	};
 };
 
 max77693-led {
-- 
2.11.0

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

Re: [PATCH V4 1/2] dt-bindings: leds: document new trigger-sources property

[Rob Herring]

On Tue, May 30, 2017 at 11:27:04AM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> 
> Some LEDs can be related to a specific device(s) described in the DT.
> This property allows specifying such relations. E.g. USB LED should
> usually be used to indicate some USB port(s) state.
> 
> Please note this binding is designed to be generic and not influenced by
> any operating system design. Linux developers may find "trigger" part a
> bit confusing since in Linux triggers are separated drivers. It
> shouldn't define the binding though (we shouldn't add an extra level of
> indirection).
> 
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> ---
> V2: Replace "usb-ports" with "led-triggers" property which is more generic and
>     allows specifying other devices as well.
> V3: Use "trigger-sources" which is even more accurate as devices aren't
>     precisely triggers.
> V4: Update example to use the correct property
> ---
>  Documentation/devicetree/bindings/leds/common.txt | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
> index 24b656014089..7efaa2cff624 100644
> --- a/Documentation/devicetree/bindings/leds/common.txt
> +++ b/Documentation/devicetree/bindings/leds/common.txt
> @@ -49,6 +49,19 @@ Optional properties for child nodes:
>  - panic-indicator : This property specifies that the LED should be used,
>  		    if at all possible, as a panic indicator.
>  
> +- trigger-sources : List of devices which should be used as a source triggering
> +		    this LED activity. Some LEDs can be related to a specific
> +		    device and should somehow indicate its state. E.g. USB 2.0
> +		    LED may react to device(s) in a USB 2.0 port(s).
> +		    Another common example is switch or router with multiple
> +		    Ethernet ports each of them having its own LED assigned
> +		    (assuming they are not hardwired). In such cases this
> +		    property should contain phandle(s) of related source
> +		    device(s).
> +		    In many cases LED can be related to more than one device
> +		    (e.g. one USB LED vs. multiple USB ports) so a list of
> +		    sources can be specified.
> +

Where's #source-cells documented?

I really prefer if we have consistency in naming with "<prop>s" and 
"#<prop>-cells". #source-cells is also not very clear what it is for. 

Rob
--
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

[PATCH V4 2/2] usb: core: read USB ports from DT in the usbport LED trigger driver

[Rafał Miłecki]

From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

This uses DT info to read relation description of LEDs and USB ports. If
DT has properly described LEDs, trigger will know when to turn them on.

Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
---
V2: Update to use "led-triggers"
V3: Update to use "trigger-sources"
---
 drivers/usb/core/ledtrig-usbport.c | 55 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/usb/core/ledtrig-usbport.c b/drivers/usb/core/ledtrig-usbport.c
index 1713248ab15a..29c9202226bf 100644
--- a/drivers/usb/core/ledtrig-usbport.c
+++ b/drivers/usb/core/ledtrig-usbport.c
@@ -11,8 +11,10 @@
 #include <linux/device.h>
 #include <linux/leds.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/slab.h>
 #include <linux/usb.h>
+#include <linux/usb/of.h>
 
 struct usbport_trig_data {
 	struct led_classdev *led_cdev;
@@ -123,6 +125,56 @@ static const struct attribute_group ports_group = {
  * Adding & removing ports
  ***************************************/
 
+/**
+ * usbport_trig_port_observed - Check if port should be observed
+ */
+static bool usbport_trig_port_observed(struct usbport_trig_data *usbport_data,
+				       struct usb_device *usb_dev, int port1)
+{
+	struct device *dev = usbport_data->led_cdev->dev;
+	struct device_node *led_np = dev->of_node;
+	struct of_phandle_args args;
+	struct device_node *port_np;
+	int count, i;
+
+	if (!led_np)
+		return false;
+
+	/* Get node of port being added */
+	port_np = usb_of_get_child_node(usb_dev->dev.of_node, port1);
+	if (!port_np)
+		return false;
+
+	/* Amount of trigger sources for this LED */
+	count = of_count_phandle_with_args(led_np, "trigger-sources",
+					   "#source-cells");
+	if (count < 0) {
+		dev_warn(dev, "Failed to get trigger sources for %s\n",
+			 led_np->full_name);
+		return false;
+	}
+
+	/* Check list of sources for this specific port */
+	for (i = 0; i < count; i++) {
+		int err;
+
+		err = of_parse_phandle_with_args(led_np, "trigger-sources",
+						 "#source-cells", i, &args);
+		if (err) {
+			dev_err(dev, "Failed to get trigger source phandle at index %d: %d\n",
+				i, err);
+			continue;
+		}
+
+		of_node_put(args.np);
+
+		if (args.np == port_np)
+			return true;
+	}
+
+	return false;
+}
+
 static int usbport_trig_add_port(struct usbport_trig_data *usbport_data,
 				 struct usb_device *usb_dev,
 				 const char *hub_name, int portnum)
@@ -141,6 +193,8 @@ static int usbport_trig_add_port(struct usbport_trig_data *usbport_data,
 	port->data = usbport_data;
 	port->hub = usb_dev;
 	port->portnum = portnum;
+	port->observed = usbport_trig_port_observed(usbport_data, usb_dev,
+						    portnum);
 
 	len = strlen(hub_name) + 8;
 	port->port_name = kzalloc(len, GFP_KERNEL);
@@ -255,6 +309,7 @@ static void usbport_trig_activate(struct led_classdev *led_cdev)
 	if (err)
 		goto err_free;
 	usb_for_each_dev(usbport_data, usbport_trig_add_usb_dev_ports);
+	usbport_trig_update_count(usbport_data);
 
 	/* Notifications */
 	usbport_data->nb.notifier_call = usbport_trig_notify,
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[EXAMPLE V4 3/2] ARM: BCM53573: Specify ports for USB LED for Tenda AC9

[Rafał Miłecki]

From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
---
This patch *should not* be applied. It's only an EXAMPLE and that's why it uses
that weird 3/2 number.

It's a proof of concept, it was tested & will be submitted through ARM tree if
previous patches get accepted.

V3: Switch to the new binding
---
 arch/arm/boot/dts/bcm47189-tenda-ac9.dts | 1 +
 arch/arm/boot/dts/bcm53573.dtsi          | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/bcm47189-tenda-ac9.dts b/arch/arm/boot/dts/bcm47189-tenda-ac9.dts
index 34417dac1cd0..ee44dec7d7ec 100644
--- a/arch/arm/boot/dts/bcm47189-tenda-ac9.dts
+++ b/arch/arm/boot/dts/bcm47189-tenda-ac9.dts
@@ -26,6 +26,7 @@
 		usb {
 			label = "bcm53xx:blue:usb";
 			gpios = <&chipcommon 1 GPIO_ACTIVE_HIGH>;
+			trigger-sources = <&ohci_port1>, <&ehci_port1>;
 		};
 
 		wps {
diff --git a/arch/arm/boot/dts/bcm53573.dtsi b/arch/arm/boot/dts/bcm53573.dtsi
index eae623f76401..5c0b9b4d679a 100644
--- a/arch/arm/boot/dts/bcm53573.dtsi
+++ b/arch/arm/boot/dts/bcm53573.dtsi
@@ -138,10 +138,12 @@
 
 				ehci_port1: port@1 {
 					reg = <1>;
+					#source-cells = <0>;
 				};
 
 				ehci_port2: port@2 {
 					reg = <2>;
+					#source-cells = <0>;
 				};
 			};
 
@@ -158,10 +160,12 @@
 
 				ohci_port1: port@1 {
 					reg = <1>;
+					#source-cells = <0>;
 				};
 
 				ohci_port2: port@2 {
 					reg = <2>;
+					#source-cells = <0>;
 				};
 			};
 		};
-- 
2.11.0

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

[PATCH V5 0/2] usb: introduce "trigger-sources" DT property for usbport trigger

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

This version of my patchset (V5) differs by renaming #source-cells to the
#trigger-source-cells and documenting it in the leds/common.txt.

I'd epxect both patches to go through Greg's usb.git if accepted.

For a reference (and before someome comes with already rejected solution) see
below history of this work:

1) [PATCH V2] leds: trigger: Introduce an USB port trigger
My initial try of adding "usb-ports" property. Rob said it should be more
generic.

2) [PATCH 1/2] dt-bindings: leds: document new usb-ports property
Rob still didn't like it due to being USB specific.

3) [PATCH V2 1/2] dt-bindings: leds: document new led-triggers property
Jacek didn't like it's more generic than what Linux can already support. Later
we agreed (?) that Linux limitations shouldn't influence DT structure.
Jacek still wanted a specific trigger but Rob said he won't accept something
specific to the USB.
Jacek suggested "trigger-sources" and Rob agreed on the name.
Jacek suggested trying triggers as separated nodes.

4) [PATCH 1/4] dt-bindings: leds: document property for LED triggers
This patch was trying to use separated nodes for triggers, e.g.:
foo-trigger { trigger-type = "foo"; };
Rob didn't like this extra level of indirection. He said Linux drivers shouldn't
define/influence the binding.

So finally I came with this patchset. It doesn't try to use separated nodes for
triggers anymore and it uses the name that was mostly accepted I think. It
should be generic and not influenced by Linux design.

Rafał Miłecki (3):
  dt-bindings: leds: document new trigger-sources property
  usb: core: read USB ports from DT in the usbport LED trigger driver
  ARM: BCM53573: Specify ports for USB LED for Tenda AC9

 Documentation/devicetree/bindings/leds/common.txt | 35 +++++++++++++-
 arch/arm/boot/dts/bcm47189-tenda-ac9.dts          |  1 +
 arch/arm/boot/dts/bcm53573.dtsi                   |  4 ++
 drivers/usb/core/ledtrig-usbport.c                | 56 +++++++++++++++++++++++
 4 files changed, 94 insertions(+), 2 deletions(-)

-- 
2.11.0

[PATCH V5 1/2] dt-bindings: leds: document new trigger-sources property

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

Some LEDs can be related to a specific device(s) described in the DT.
This property allows specifying such relations. E.g. USB LED should
usually be used to indicate some USB port(s) state.

Please note this binding is designed to be generic and not influenced by
any operating system design. Linux developers may find "trigger" part a
bit confusing since in Linux triggers are separated drivers. It
shouldn't define the binding though (we shouldn't add an extra level of
indirection).

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Replace "usb-ports" with "led-triggers" property which is more generic and
    allows specifying other devices as well.
V3: Use "trigger-sources" which is even more accurate as devices aren't
    precisely triggers.
V4: Update example to use the correct property
V5: Document #trigger-source-cells
---
 Documentation/devicetree/bindings/leds/common.txt | 35 +++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
index 24b656014089..1d4afe9644b6 100644
--- a/Documentation/devicetree/bindings/leds/common.txt
+++ b/Documentation/devicetree/bindings/leds/common.txt
@@ -1,4 +1,4 @@
-Common leds properties.
+* Common leds properties.
 
 LED and flash LED devices provide the same basic functionality as current
 regulators, but extended with LED and flash LED specific features like
@@ -49,6 +49,22 @@ Optional properties for child nodes:
 - panic-indicator : This property specifies that the LED should be used,
 		    if at all possible, as a panic indicator.
 
+- trigger-sources : List of devices which should be used as a source triggering
+		    this LED activity. Some LEDs can be related to a specific
+		    device and should somehow indicate its state. E.g. USB 2.0
+		    LED may react to device(s) in a USB 2.0 port(s).
+		    Another common example is switch or router with multiple
+		    Ethernet ports each of them having its own LED assigned
+		    (assuming they are not hardwired). In such cases this
+		    property should contain phandle(s) of related source
+		    device(s).
+		    In many cases LED can be related to more than one device
+		    (e.g. one USB LED vs. multiple USB ports). Each source
+		    should be represented by a node in the device tree and be
+		    referenced by a phandle and a set of phandle arguments. A
+		    length of arguments should be specified by the
+		    #trigger-source-cells property in the source node.
+
 Required properties for flash LED child nodes:
 - flash-max-microamp : Maximum flash LED supply current in microamperes.
 - flash-max-timeout-us : Maximum timeout in microseconds after which the flash
@@ -59,7 +75,17 @@ property can be omitted.
 For controllers that have no configurable timeout the flash-max-timeout-us
 property can be omitted.
 
-Examples:
+* Trigger source providers
+
+Each trigger source should be represented by a device tree node. It may be e.g.
+a USB port or an Ethernet device.
+
+Required properties for trigger source:
+- #trigger-source-cells : Number of cells in a source trigger. Typically 0 for
+			  nodes of simple trigger sources (e.g. a specific USB
+			  port).
+
+* Examples
 
 gpio-leds {
 	compatible = "gpio-leds";
@@ -69,6 +95,11 @@ gpio-leds {
 		linux,default-trigger = "heartbeat";
 		gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>;
 	};
+
+	usb {
+		gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
+		trigger-sources = <&ohci_port1>, <&ehci_port1>;
+	};
 };
 
 max77693-led {
-- 
2.11.0

Re: [PATCH V5 1/2] dt-bindings: leds: document new trigger-sources property

[Rob Herring]

On Thu, Jun 08, 2017 at 06:08:31PM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> 
> Some LEDs can be related to a specific device(s) described in the DT.
> This property allows specifying such relations. E.g. USB LED should
> usually be used to indicate some USB port(s) state.
> 
> Please note this binding is designed to be generic and not influenced by
> any operating system design. Linux developers may find "trigger" part a
> bit confusing since in Linux triggers are separated drivers. It
> shouldn't define the binding though (we shouldn't add an extra level of
> indirection).
> 
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> ---
> V2: Replace "usb-ports" with "led-triggers" property which is more generic and
>     allows specifying other devices as well.
> V3: Use "trigger-sources" which is even more accurate as devices aren't
>     precisely triggers.
> V4: Update example to use the correct property
> V5: Document #trigger-source-cells
> ---
>  Documentation/devicetree/bindings/leds/common.txt | 35 +++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)

Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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

[PATCH V5 2/2] usb: core: read USB ports from DT in the usbport LED trigger driver

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

This uses DT info to read relation description of LEDs and USB ports. If
DT has properly described LEDs, trigger will know when to turn them on.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Update to use "led-triggers"
V3: Update to use "trigger-sources"
V5: Read #trigger-source-cells instead of #source-cells
---
 drivers/usb/core/ledtrig-usbport.c | 56 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/drivers/usb/core/ledtrig-usbport.c b/drivers/usb/core/ledtrig-usbport.c
index 1713248ab15a..16c19a31dad1 100644
--- a/drivers/usb/core/ledtrig-usbport.c
+++ b/drivers/usb/core/ledtrig-usbport.c
@@ -11,8 +11,10 @@
 #include <linux/device.h>
 #include <linux/leds.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/slab.h>
 #include <linux/usb.h>
+#include <linux/usb/of.h>
 
 struct usbport_trig_data {
 	struct led_classdev *led_cdev;
@@ -123,6 +125,57 @@ static const struct attribute_group ports_group = {
  * Adding & removing ports
  ***************************************/
 
+/**
+ * usbport_trig_port_observed - Check if port should be observed
+ */
+static bool usbport_trig_port_observed(struct usbport_trig_data *usbport_data,
+				       struct usb_device *usb_dev, int port1)
+{
+	struct device *dev = usbport_data->led_cdev->dev;
+	struct device_node *led_np = dev->of_node;
+	struct of_phandle_args args;
+	struct device_node *port_np;
+	int count, i;
+
+	if (!led_np)
+		return false;
+
+	/* Get node of port being added */
+	port_np = usb_of_get_child_node(usb_dev->dev.of_node, port1);
+	if (!port_np)
+		return false;
+
+	/* Amount of trigger sources for this LED */
+	count = of_count_phandle_with_args(led_np, "trigger-sources",
+					   "#trigger-source-cells");
+	if (count < 0) {
+		dev_warn(dev, "Failed to get trigger sources for %s\n",
+			 led_np->full_name);
+		return false;
+	}
+
+	/* Check list of sources for this specific port */
+	for (i = 0; i < count; i++) {
+		int err;
+
+		err = of_parse_phandle_with_args(led_np, "trigger-sources",
+						 "#trigger-source-cells", i,
+						 &args);
+		if (err) {
+			dev_err(dev, "Failed to get trigger source phandle at index %d: %d\n",
+				i, err);
+			continue;
+		}
+
+		of_node_put(args.np);
+
+		if (args.np == port_np)
+			return true;
+	}
+
+	return false;
+}
+
 static int usbport_trig_add_port(struct usbport_trig_data *usbport_data,
 				 struct usb_device *usb_dev,
 				 const char *hub_name, int portnum)
@@ -141,6 +194,8 @@ static int usbport_trig_add_port(struct usbport_trig_data *usbport_data,
 	port->data = usbport_data;
 	port->hub = usb_dev;
 	port->portnum = portnum;
+	port->observed = usbport_trig_port_observed(usbport_data, usb_dev,
+						    portnum);
 
 	len = strlen(hub_name) + 8;
 	port->port_name = kzalloc(len, GFP_KERNEL);
@@ -255,6 +310,7 @@ static void usbport_trig_activate(struct led_classdev *led_cdev)
 	if (err)
 		goto err_free;
 	usb_for_each_dev(usbport_data, usbport_trig_add_usb_dev_ports);
+	usbport_trig_update_count(usbport_data);
 
 	/* Notifications */
 	usbport_data->nb.notifier_call = usbport_trig_notify,
-- 
2.11.0

Re: [PATCH V5 2/2] usb: core: read USB ports from DT in the usbport LED trigger driver

[Rafał Miłecki]

On 8 June 2017 at 18:08, Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>
> This uses DT info to read relation description of LEDs and USB ports. If
> DT has properly described LEDs, trigger will know when to turn them on.
>
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

Hi Greg,

do you mind if I ask about this patchset?
[PATCH V5 1/2] dt-bindings: leds: document new trigger-sources property
[PATCH V5 2/2] usb: core: read USB ports from DT in the usbport LED
trigger driver

Would that be appropriable to get it through your tree?
--
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

[EXAMPLE V5 3/2] ARM: BCM53573: Specify ports for USB LED for Tenda AC9

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
This patch *should not* be applied. It's only an EXAMPLE and that's why it uses
that weird 3/2 number.

It's a proof of concept, it was tested & will be submitted through ARM tree if
previous patches get accepted.

V3: Switch to the new binding
V5: Use #trigger-source-cells instead of #source-cells
---
 arch/arm/boot/dts/bcm47189-tenda-ac9.dts | 1 +
 arch/arm/boot/dts/bcm53573.dtsi          | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/bcm47189-tenda-ac9.dts b/arch/arm/boot/dts/bcm47189-tenda-ac9.dts
index 34417dac1cd0..ee44dec7d7ec 100644
--- a/arch/arm/boot/dts/bcm47189-tenda-ac9.dts
+++ b/arch/arm/boot/dts/bcm47189-tenda-ac9.dts
@@ -26,6 +26,7 @@
 		usb {
 			label = "bcm53xx:blue:usb";
 			gpios = <&chipcommon 1 GPIO_ACTIVE_HIGH>;
+			trigger-sources = <&ohci_port1>, <&ehci_port1>;
 		};
 
 		wps {
diff --git a/arch/arm/boot/dts/bcm53573.dtsi b/arch/arm/boot/dts/bcm53573.dtsi
index eae623f76401..5c0b9b4d679a 100644
--- a/arch/arm/boot/dts/bcm53573.dtsi
+++ b/arch/arm/boot/dts/bcm53573.dtsi
@@ -138,10 +138,12 @@
 
 				ehci_port1: port@1 {
 					reg = <1>;
+					#trigger-source-cells = <0>;
 				};
 
 				ehci_port2: port@2 {
 					reg = <2>;
+					#trigger-source-cells = <0>;
 				};
 			};
 
@@ -158,10 +160,12 @@
 
 				ohci_port1: port@1 {
 					reg = <1>;
+					#trigger-source-cells = <0>;
 				};
 
 				ohci_port2: port@2 {
 					reg = <2>;
+					#trigger-source-cells = <0>;
 				};
 			};
 		};
-- 
2.11.0

[PATCH V3 2/2] usb: core: read USB ports from DT in the usbport LED trigger driver

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

This uses DT info to read relation description of LEDs and USB ports. If
DT has properly described LEDs, trigger will know when to turn them on.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Update to use "led-triggers"
V3: Update to use "trigger-sources"
---
 drivers/usb/core/ledtrig-usbport.c | 55 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/usb/core/ledtrig-usbport.c b/drivers/usb/core/ledtrig-usbport.c
index 1713248ab15a..29c9202226bf 100644
--- a/drivers/usb/core/ledtrig-usbport.c
+++ b/drivers/usb/core/ledtrig-usbport.c
@@ -11,8 +11,10 @@
 #include <linux/device.h>
 #include <linux/leds.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/slab.h>
 #include <linux/usb.h>
+#include <linux/usb/of.h>
 
 struct usbport_trig_data {
 	struct led_classdev *led_cdev;
@@ -123,6 +125,56 @@ static const struct attribute_group ports_group = {
  * Adding & removing ports
  ***************************************/
 
+/**
+ * usbport_trig_port_observed - Check if port should be observed
+ */
+static bool usbport_trig_port_observed(struct usbport_trig_data *usbport_data,
+				       struct usb_device *usb_dev, int port1)
+{
+	struct device *dev = usbport_data->led_cdev->dev;
+	struct device_node *led_np = dev->of_node;
+	struct of_phandle_args args;
+	struct device_node *port_np;
+	int count, i;
+
+	if (!led_np)
+		return false;
+
+	/* Get node of port being added */
+	port_np = usb_of_get_child_node(usb_dev->dev.of_node, port1);
+	if (!port_np)
+		return false;
+
+	/* Amount of trigger sources for this LED */
+	count = of_count_phandle_with_args(led_np, "trigger-sources",
+					   "#source-cells");
+	if (count < 0) {
+		dev_warn(dev, "Failed to get trigger sources for %s\n",
+			 led_np->full_name);
+		return false;
+	}
+
+	/* Check list of sources for this specific port */
+	for (i = 0; i < count; i++) {
+		int err;
+
+		err = of_parse_phandle_with_args(led_np, "trigger-sources",
+						 "#source-cells", i, &args);
+		if (err) {
+			dev_err(dev, "Failed to get trigger source phandle at index %d: %d\n",
+				i, err);
+			continue;
+		}
+
+		of_node_put(args.np);
+
+		if (args.np == port_np)
+			return true;
+	}
+
+	return false;
+}
+
 static int usbport_trig_add_port(struct usbport_trig_data *usbport_data,
 				 struct usb_device *usb_dev,
 				 const char *hub_name, int portnum)
@@ -141,6 +193,8 @@ static int usbport_trig_add_port(struct usbport_trig_data *usbport_data,
 	port->data = usbport_data;
 	port->hub = usb_dev;
 	port->portnum = portnum;
+	port->observed = usbport_trig_port_observed(usbport_data, usb_dev,
+						    portnum);
 
 	len = strlen(hub_name) + 8;
 	port->port_name = kzalloc(len, GFP_KERNEL);
@@ -255,6 +309,7 @@ static void usbport_trig_activate(struct led_classdev *led_cdev)
 	if (err)
 		goto err_free;
 	usb_for_each_dev(usbport_data, usbport_trig_add_usb_dev_ports);
+	usbport_trig_update_count(usbport_data);
 
 	/* Notifications */
 	usbport_data->nb.notifier_call = usbport_trig_notify,
-- 
2.11.0

[EXAMPLE V3 3/2] ARM: BCM53573: Specify ports for USB LED for Tenda AC9

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
This patch *should not* be applied. It's only an EXAMPLE and that's why it uses
that weird 3/2 number.

It's a proof of concept, it was tested & will be submitted through ARM tree if
previous patches get accepted.

V3: Switch to the new binding
---
 arch/arm/boot/dts/bcm47189-tenda-ac9.dts | 1 +
 arch/arm/boot/dts/bcm53573.dtsi          | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/bcm47189-tenda-ac9.dts b/arch/arm/boot/dts/bcm47189-tenda-ac9.dts
index 34417dac1cd0..ee44dec7d7ec 100644
--- a/arch/arm/boot/dts/bcm47189-tenda-ac9.dts
+++ b/arch/arm/boot/dts/bcm47189-tenda-ac9.dts
@@ -26,6 +26,7 @@
 		usb {
 			label = "bcm53xx:blue:usb";
 			gpios = <&chipcommon 1 GPIO_ACTIVE_HIGH>;
+			trigger-sources = <&ohci_port1>, <&ehci_port1>;
 		};
 
 		wps {
diff --git a/arch/arm/boot/dts/bcm53573.dtsi b/arch/arm/boot/dts/bcm53573.dtsi
index eae623f76401..5c0b9b4d679a 100644
--- a/arch/arm/boot/dts/bcm53573.dtsi
+++ b/arch/arm/boot/dts/bcm53573.dtsi
@@ -138,10 +138,12 @@
 
 				ehci_port1: port@1 {
 					reg = <1>;
+					#source-cells = <0>;
 				};
 
 				ehci_port2: port@2 {
 					reg = <2>;
+					#source-cells = <0>;
 				};
 			};
 
@@ -158,10 +160,12 @@
 
 				ohci_port1: port@1 {
 					reg = <1>;
+					#source-cells = <0>;
 				};
 
 				ohci_port2: port@2 {
 					reg = <2>;
+					#source-cells = <0>;
 				};
 			};
 		};
-- 
2.11.0