[1/4] dt-bindings: leds: document property for LED triggers

Message ID 290ed068-2518-50ef-4d02-394bef8b7ee9@gmail.com
State Changes Requested
Headers show

Commit Message

Rafał Miłecki Feb. 28, 2017, 9:51 p.m.
On 02/28/2017 10:38 PM, Jacek Anaszewski wrote:
> I think that it would be simpler if we could initially see
> a complete sample dts implementation containing all required DT
> nodes. The example could contain timer trigger as well as usb-port
> trigger specific bindings.

Please take a look at attached patch. I used it on Tenda AC9 with:

usb_trigger: usb-trigger {
	trigger-type = "usbport";
	ports = <&ohci_port1>, <&ehci_port1>;
};

usb {
	label = "bcm53xx:blue:usb";
	gpios = <&chipcommon 1 GPIO_ACTIVE_HIGH>;
	triggers = <&usb_trigger>;
};


> I suppose that we should see DT nodes containing #list-cells
> properties that define the quantity of phandle arguments.
>
> It seems that this approach allows for defining a list of elements
> with variable number of arguments, i.e. what you were initially
> asking for.

Are you sure we need #list-cells? Can't we simply use
of_count_phandle_with_args(np, "triggers", NULL);
?


From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= <rafal@milecki.pl>
Date: Mon, 3 Oct 2016 16:49:05 +0200
Subject: [PATCH] usb: core: read USB ports from DT in the usbport LED trigger
  driver
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
  .../devicetree/bindings/leds/triggers-usbport.txt  | 19 ++++++++
  drivers/leds/led-triggers.c                        |  3 +-
  drivers/usb/core/ledtrig-usbport.c                 | 56 ++++++++++++++++++++++
  3 files changed, 77 insertions(+), 1 deletion(-)
  create mode 100644 Documentation/devicetree/bindings/leds/triggers-usbport.txt

Comments

Rob Herring Feb. 28, 2017, 10:12 p.m. | #1
On Tue, Feb 28, 2017 at 3:51 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
> On 02/28/2017 10:38 PM, Jacek Anaszewski wrote:
>>
>> I think that it would be simpler if we could initially see
>> a complete sample dts implementation containing all required DT
>> nodes. The example could contain timer trigger as well as usb-port
>> trigger specific bindings.
>
>
> Please take a look at attached patch. I used it on Tenda AC9 with:

I'm not sure about this extra level of indirection. I don't see the need.

> usb_trigger: usb-trigger {
>         trigger-type = "usbport";

Why do we need to know the type? The trigger device knows what type it
is. All we should need to know here is what device(s) controls an LED.
The rest the kernel can figure out.

>         ports = <&ohci_port1>, <&ehci_port1>;
> };
>
> usb {
>         label = "bcm53xx:blue:usb";
>         gpios = <&chipcommon 1 GPIO_ACTIVE_HIGH>;
>         triggers = <&usb_trigger>;
> };
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jacek Anaszewski March 1, 2017, 9:04 p.m. | #2
On 02/28/2017 11:12 PM, Rob Herring wrote:
> On Tue, Feb 28, 2017 at 3:51 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
>> On 02/28/2017 10:38 PM, Jacek Anaszewski wrote:
>>>
>>> I think that it would be simpler if we could initially see
>>> a complete sample dts implementation containing all required DT
>>> nodes. The example could contain timer trigger as well as usb-port
>>> trigger specific bindings.
>>
>>
>> Please take a look at attached patch. I used it on Tenda AC9 with:
> 
> I'm not sure about this extra level of indirection. I don't see the need.
> 
>> usb_trigger: usb-trigger {
>>         trigger-type = "usbport";
> 
> Why do we need to know the type? The trigger device knows what type it
> is. All we should need to know here is what device(s) controls an LED.
> The rest the kernel can figure out.

The thing is that in the proposed approach the trigger is not necessary
a device. The trigger node is here only a container with initialization
data.

We could have e,g, two such nodes with different set of ports
(say usb1-trigger and usb2-trigger). Then if we had two LED nodes usb1
and usb2, the former could have its triggers property initialized
to &usb1-trigger and the latter to &usb2-trigger. Thanks to that both
LEDs after executing "echo usbport > triggers" would listen to events
from different set of usb ports.

Also, e.g. for the timer trigger we could define two separate DT nodes
with different delay intervals.

>>         ports = <&ohci_port1>, <&ehci_port1>;
>> };
>>
>> usb {
>>         label = "bcm53xx:blue:usb";
>>         gpios = <&chipcommon 1 GPIO_ACTIVE_HIGH>;
>>         triggers = <&usb_trigger>;
>> };
>
Jacek Anaszewski March 1, 2017, 9:04 p.m. | #3
On 02/28/2017 10:51 PM, Rafał Miłecki wrote:
> On 02/28/2017 10:38 PM, Jacek Anaszewski wrote:
>> I think that it would be simpler if we could initially see
>> a complete sample dts implementation containing all required DT
>> nodes. The example could contain timer trigger as well as usb-port
>> trigger specific bindings.
> 
> Please take a look at attached patch. I used it on Tenda AC9 with:
> 
> usb_trigger: usb-trigger {
>     trigger-type = "usbport";
>     ports = <&ohci_port1>, <&ehci_port1>;
> };
> 
> usb {
>     label = "bcm53xx:blue:usb";
>     gpios = <&chipcommon 1 GPIO_ACTIVE_HIGH>;
>     triggers = <&usb_trigger>;
> };

OK, I got it, thanks.

> 
>> I suppose that we should see DT nodes containing #list-cells
>> properties that define the quantity of phandle arguments.
>>
>> It seems that this approach allows for defining a list of elements
>> with variable number of arguments, i.e. what you were initially
>> asking for.
> 
> Are you sure we need #list-cells? Can't we simply use
> of_count_phandle_with_args(np, "triggers", NULL);
> ?

I'm not sure, I just read the function documentation :-)
I haven't verified nor have I used this API.

> 
> From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= <rafal@milecki.pl>
> Date: Mon, 3 Oct 2016 16:49:05 +0200
> Subject: [PATCH] usb: core: read USB ports from DT in the usbport LED
> trigger
>  driver
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  .../devicetree/bindings/leds/triggers-usbport.txt  | 19 ++++++++
>  drivers/leds/led-triggers.c                        |  3 +-
>  drivers/usb/core/ledtrig-usbport.c                 | 56
> ++++++++++++++++++++++
>  3 files changed, 77 insertions(+), 1 deletion(-)
>  create mode 100644
> Documentation/devicetree/bindings/leds/triggers-usbport.txt
> 
> diff --git a/Documentation/devicetree/bindings/leds/triggers-usbport.txt
> b/Documentation/devicetree/bindings/leds/triggers-usbport.txt
> new file mode 100644
> index 000000000000..10e55122cded
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/triggers-usbport.txt
> @@ -0,0 +1,19 @@
> +USB port LED trigger properties.
> +
> +USB port trigger can be used for signalling to the user a presence of USB
> +device(s) in given ports. It's possible to specify USB ports in DT by
> providing
> +their references.
> +
> +Properties:
> +- trigger-type : Must be "usbport".
> +- ports : List of USB ports related to this LED. Some devices may have
> one USB
> +      LED for all ports, other may have few of them (e.g. USB version
> +      specific). It's used by usbport trigger for reading a list of ports
> +      that should cause LED to turn on whenver device get connected.
> +
> +Examples:
> +
> +usbport-trigger {
> +    trigger-type = "usbport";
> +    ports = <&ohci_port1>, <&ehci_port1>;
> +};
> diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
> index c53c20d676cd..1d8bda9755ca 100644
> --- a/drivers/leds/led-triggers.c
> +++ b/drivers/leds/led-triggers.c
> @@ -181,7 +181,8 @@ static void led_trigger_of_read_trigger(struct
> led_classdev *led_cdev)
>      }
> 
>      /* Check if trigger specified in DT is supported */
> -    if (strcmp(trigger_type, "timer"))
> +    if (strcmp(trigger_type, "timer") &&
> +        strcmp(trigger_type, "usbport"))
>          goto err_node_put;
> 
>      led_cdev->default_trigger = trigger_type;
> diff --git a/drivers/usb/core/ledtrig-usbport.c
> b/drivers/usb/core/ledtrig-usbport.c
> index 1713248ab15a..599f7e6b0ba8 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,58 @@ static const struct attribute_group ports_group = {
>   * Adding & removing ports
>   ***************************************/
> 
> +/**
> + * usbport_trig_port_observed - Check if port should be observed
> + *
> + * Each LED may have list of related USB ports specified in a DT. This
> function
> + * reads them using ports property and sets a proper state.
> + */
> +static bool usbport_trig_port_observed(struct usbport_trig_data
> *usbport_data,
> +                       struct usb_device *usb_dev, int port1)
> +{
> +    struct device_node *led_np = usbport_data->led_cdev->trigger_node;
> +    struct device *dev = usbport_data->led_cdev->dev;
> +    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 ports this LED references */
> +    count = of_count_phandle_with_args(led_np, "ports", NULL);
> +    if (count < 0) {
> +        dev_warn(dev, "Failed to get USB ports for %s\n",
> +             led_np->full_name);
> +        return false;
> +    }
> +
> +    /* Check if port is on this LED's list */
> +    for (i = 0; i < count; i++) {
> +        int err;
> +
> +        err = of_parse_phandle_with_args(led_np, "ports", NULL, i,
> +                         &args);
> +        if (err) {
> +            dev_err(dev, "Failed to get USB port 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 +195,7 @@ 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,
Rafal Milecki March 6, 2017, 6:06 a.m. | #4
On 03/01/2017 10:04 PM, Jacek Anaszewski wrote:
> On 02/28/2017 11:12 PM, Rob Herring wrote:
>> On Tue, Feb 28, 2017 at 3:51 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
>>> On 02/28/2017 10:38 PM, Jacek Anaszewski wrote:
>>>>
>>>> I think that it would be simpler if we could initially see
>>>> a complete sample dts implementation containing all required DT
>>>> nodes. The example could contain timer trigger as well as usb-port
>>>> trigger specific bindings.
>>>
>>>
>>> Please take a look at attached patch. I used it on Tenda AC9 with:
>>
>> I'm not sure about this extra level of indirection. I don't see the need.
>>
>>> usb_trigger: usb-trigger {
>>>         trigger-type = "usbport";
>>
>> Why do we need to know the type? The trigger device knows what type it
>> is. All we should need to know here is what device(s) controls an LED.
>> The rest the kernel can figure out.
>
> The thing is that in the proposed approach the trigger is not necessary
> a device. The trigger node is here only a container with initialization
> data.
>
> We could have e,g, two such nodes with different set of ports
> (say usb1-trigger and usb2-trigger). Then if we had two LED nodes usb1
> and usb2, the former could have its triggers property initialized
> to &usb1-trigger and the latter to &usb2-trigger. Thanks to that both
> LEDs after executing "echo usbport > triggers" would listen to events
> from different set of usb ports.
>
> Also, e.g. for the timer trigger we could define two separate DT nodes
> with different delay intervals.

Hi Rob, what do you think about this?

Jacek is correct, we could have e.g.
timer-trigger-1 {
	trigger-type = "timer";
	delay-on = <200>;
	delay-off = <500>;
};
timer-trigger-2 {
	trigger-type = "timer";
	delay-on = <500>;
	delay-off = <1000>;
};

or something like:
usbport-2-0-trigger {
	trigger-type = "usbport";
	ports = <&ohci_port1>, <&ehci_port1>;
};
usbport-3-0-trigger {
	trigger-type = "usbport";
	ports = <&xhci_port1>;
};

Does it make more sense?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafal Milecki March 6, 2017, 6:16 a.m. | #5
On 03/01/2017 10:04 PM, Jacek Anaszewski wrote:
> On 02/28/2017 10:51 PM, Rafał Miłecki wrote:
>> On 02/28/2017 10:38 PM, Jacek Anaszewski wrote:
>>> I think that it would be simpler if we could initially see
>>> a complete sample dts implementation containing all required DT
>>> nodes. The example could contain timer trigger as well as usb-port
>>> trigger specific bindings.
>>
>> Please take a look at attached patch. I used it on Tenda AC9 with:
>>
>> usb_trigger: usb-trigger {
>>     trigger-type = "usbport";
>>     ports = <&ohci_port1>, <&ehci_port1>;
>> };
>>
>> usb {
>>     label = "bcm53xx:blue:usb";
>>     gpios = <&chipcommon 1 GPIO_ACTIVE_HIGH>;
>>     triggers = <&usb_trigger>;
>> };
>
> OK, I got it, thanks.
>
>>
>>> I suppose that we should see DT nodes containing #list-cells
>>> properties that define the quantity of phandle arguments.
>>>
>>> It seems that this approach allows for defining a list of elements
>>> with variable number of arguments, i.e. what you were initially
>>> asking for.
>>
>> Are you sure we need #list-cells? Can't we simply use
>> of_count_phandle_with_args(np, "triggers", NULL);
>> ?
>
> I'm not sure, I just read the function documentation :-)
> I haven't verified nor have I used this API.

So we could use #list-cells to support something like:

usb {
	label = "bcm53xx:blue:usb";
	gpios = <&chipcommon 1 GPIO_ACTIVE_HIGH>;
	triggers = <&usb_trigger 1 2>, <&timer_trigger 3 4 5>;
};

but I don't see any need for this. Just specifying triggers like:
triggers = <&usb_trigger>, <&timer_trigger>, <&foo>;
should be always enough, especially with the new trigger nodes you suggested.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jacek Anaszewski March 6, 2017, 7:59 p.m. | #6
On 03/06/2017 07:16 AM, Rafał Miłecki wrote:
> On 03/01/2017 10:04 PM, Jacek Anaszewski wrote:
>> On 02/28/2017 10:51 PM, Rafał Miłecki wrote:
>>> On 02/28/2017 10:38 PM, Jacek Anaszewski wrote:
>>>> I think that it would be simpler if we could initially see
>>>> a complete sample dts implementation containing all required DT
>>>> nodes. The example could contain timer trigger as well as usb-port
>>>> trigger specific bindings.
>>>
>>> Please take a look at attached patch. I used it on Tenda AC9 with:
>>>
>>> usb_trigger: usb-trigger {
>>>     trigger-type = "usbport";
>>>     ports = <&ohci_port1>, <&ehci_port1>;
>>> };
>>>
>>> usb {
>>>     label = "bcm53xx:blue:usb";
>>>     gpios = <&chipcommon 1 GPIO_ACTIVE_HIGH>;
>>>     triggers = <&usb_trigger>;
>>> };
>>
>> OK, I got it, thanks.
>>
>>>
>>>> I suppose that we should see DT nodes containing #list-cells
>>>> properties that define the quantity of phandle arguments.
>>>>
>>>> It seems that this approach allows for defining a list of elements
>>>> with variable number of arguments, i.e. what you were initially
>>>> asking for.
>>>
>>> Are you sure we need #list-cells? Can't we simply use
>>> of_count_phandle_with_args(np, "triggers", NULL);
>>> ?
>>
>> I'm not sure, I just read the function documentation :-)
>> I haven't verified nor have I used this API.
> 
> So we could use #list-cells to support something like:
> 
> usb {
>     label = "bcm53xx:blue:usb";
>     gpios = <&chipcommon 1 GPIO_ACTIVE_HIGH>;
>     triggers = <&usb_trigger 1 2>, <&timer_trigger 3 4 5>;
> };
> 
> but I don't see any need for this. Just specifying triggers like:
> triggers = <&usb_trigger>, <&timer_trigger>, <&foo>;
> should be always enough, especially with the new trigger nodes you
> suggested.

Indeed, it seems to be not needed for this approach.
I'm OK with this design.
Rob Herring March 12, 2017, 11:44 a.m. | #7
On Mon, Mar 06, 2017 at 07:06:32AM +0100, Rafał Miłecki wrote:
> On 03/01/2017 10:04 PM, Jacek Anaszewski wrote:
> > On 02/28/2017 11:12 PM, Rob Herring wrote:
> > > On Tue, Feb 28, 2017 at 3:51 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
> > > > On 02/28/2017 10:38 PM, Jacek Anaszewski wrote:
> > > > > 
> > > > > I think that it would be simpler if we could initially see
> > > > > a complete sample dts implementation containing all required DT
> > > > > nodes. The example could contain timer trigger as well as usb-port
> > > > > trigger specific bindings.
> > > > 
> > > > 
> > > > Please take a look at attached patch. I used it on Tenda AC9 with:
> > > 
> > > I'm not sure about this extra level of indirection. I don't see the need.
> > > 
> > > > usb_trigger: usb-trigger {
> > > >         trigger-type = "usbport";
> > > 
> > > Why do we need to know the type? The trigger device knows what type it
> > > is. All we should need to know here is what device(s) controls an LED.
> > > The rest the kernel can figure out.
> > 
> > The thing is that in the proposed approach the trigger is not necessary
> > a device. The trigger node is here only a container with initialization
> > data.
> > 
> > We could have e,g, two such nodes with different set of ports
> > (say usb1-trigger and usb2-trigger). Then if we had two LED nodes usb1
> > and usb2, the former could have its triggers property initialized
> > to &usb1-trigger and the latter to &usb2-trigger. Thanks to that both
> > LEDs after executing "echo usbport > triggers" would listen to events
> > from different set of usb ports.
> > 
> > Also, e.g. for the timer trigger we could define two separate DT nodes
> > with different delay intervals.
> 
> Hi Rob, what do you think about this?
> 
> Jacek is correct, we could have e.g.
> timer-trigger-1 {
> 	trigger-type = "timer";
> 	delay-on = <200>;
> 	delay-off = <500>;
> };
> timer-trigger-2 {
> 	trigger-type = "timer";
> 	delay-on = <500>;
> 	delay-off = <1000>;
> };

This is just letting the Linux driver structure define/influence the 
binding. This could all be described with a single LED property like 
"led-blink-pattern". The fact that it gets implemented by the timer 
trigger is irrelevant to the binding.

Plus timer based triggering seems more like user controlled than 
platform or hardware defined. So trying to create a binding for all 
Linux triggers is a mistake. The only thing we need here is a way to 
associate an LED with a h/w device. 

> 
> or something like:
> usbport-2-0-trigger {
> 	trigger-type = "usbport";
> 	ports = <&ohci_port1>, <&ehci_port1>;
> };
> usbport-3-0-trigger {
> 	trigger-type = "usbport";
> 	ports = <&xhci_port1>;
> };
> 
> Does it make more sense?

No, for the reasons above.

Rob

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

Patch

diff --git a/Documentation/devicetree/bindings/leds/triggers-usbport.txt b/Documentation/devicetree/bindings/leds/triggers-usbport.txt
new file mode 100644
index 000000000000..10e55122cded
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/triggers-usbport.txt
@@ -0,0 +1,19 @@ 
+USB port LED trigger properties.
+
+USB port trigger can be used for signalling to the user a presence of USB
+device(s) in given ports. It's possible to specify USB ports in DT by providing
+their references.
+
+Properties:
+- trigger-type : Must be "usbport".
+- ports : List of USB ports related to this LED. Some devices may have one USB
+	  LED for all ports, other may have few of them (e.g. USB version
+	  specific). It's used by usbport trigger for reading a list of ports
+	  that should cause LED to turn on whenver device get connected.
+
+Examples:
+
+usbport-trigger {
+	trigger-type = "usbport";
+	ports = <&ohci_port1>, <&ehci_port1>;
+};
diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index c53c20d676cd..1d8bda9755ca 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -181,7 +181,8 @@  static void led_trigger_of_read_trigger(struct led_classdev *led_cdev)
  	}

  	/* Check if trigger specified in DT is supported */
-	if (strcmp(trigger_type, "timer"))
+	if (strcmp(trigger_type, "timer") &&
+	    strcmp(trigger_type, "usbport"))
  		goto err_node_put;

  	led_cdev->default_trigger = trigger_type;
diff --git a/drivers/usb/core/ledtrig-usbport.c b/drivers/usb/core/ledtrig-usbport.c
index 1713248ab15a..599f7e6b0ba8 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,58 @@  static const struct attribute_group ports_group = {
   * Adding & removing ports
   ***************************************/

+/**
+ * usbport_trig_port_observed - Check if port should be observed
+ *
+ * Each LED may have list of related USB ports specified in a DT. This function
+ * reads them using ports property and sets a proper state.
+ */
+static bool usbport_trig_port_observed(struct usbport_trig_data *usbport_data,
+				       struct usb_device *usb_dev, int port1)
+{
+	struct device_node *led_np = usbport_data->led_cdev->trigger_node;
+	struct device *dev = usbport_data->led_cdev->dev;
+	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 ports this LED references */
+	count = of_count_phandle_with_args(led_np, "ports", NULL);
+	if (count < 0) {
+		dev_warn(dev, "Failed to get USB ports for %s\n",
+			 led_np->full_name);
+		return false;
+	}
+
+	/* Check if port is on this LED's list */
+	for (i = 0; i < count; i++) {
+		int err;
+
+		err = of_parse_phandle_with_args(led_np, "ports", NULL, i,
+						 &args);
+		if (err) {
+			dev_err(dev, "Failed to get USB port 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 +195,7 @@  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,