[PATCH] dt-bindings: leds: add "usbport" trigger

[PATCH] dt-bindings: leds: add "usbport" trigger

[Rafał Miłecki]

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

It's a trigger used on many home routers that have LEDs to indicate
specific USB port state.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 Documentation/devicetree/bindings/leds/common.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
index 15e3f6645682..95b316ee3146 100644
--- a/Documentation/devicetree/bindings/leds/common.yaml
+++ b/Documentation/devicetree/bindings/leds/common.yaml
@@ -99,6 +99,7 @@ properties:
           - pattern
           - usb-gadget
           - usb-host
+          - usbport
       - pattern: "^cpu[0-9]*$"
       - pattern: "^hci[0-9]+-power$"
         # LED is triggered by Bluetooth activity
-- 
2.34.1

Re: [PATCH] dt-bindings: leds: add "usbport" trigger

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 391 bytes --]

On Tue 2023-02-28 15:49:33, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> It's a trigger used on many home routers that have LEDs to indicate
> specific USB port state.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

Acked-by: Pavel Machek <pavel@ucw.cz>

BR,
								Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH] dt-bindings: leds: add "usbport" trigger

[Rob Herring]

On Tue, Feb 28, 2023 at 03:49:33PM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> It's a trigger used on many home routers that have LEDs to indicate
> specific USB port state.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  Documentation/devicetree/bindings/leds/common.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
> index 15e3f6645682..95b316ee3146 100644
> --- a/Documentation/devicetree/bindings/leds/common.yaml
> +++ b/Documentation/devicetree/bindings/leds/common.yaml
> @@ -99,6 +99,7 @@ properties:
>            - pattern
>            - usb-gadget
>            - usb-host
> +          - usbport

Can we stop adding entries which are clearly likely to have multiple 
instances. We have a better binding to map the trigger source...

>        - pattern: "^cpu[0-9]*$"
>        - pattern: "^hci[0-9]+-power$"
>          # LED is triggered by Bluetooth activity
> -- 
> 2.34.1
> 

Re: [PATCH] dt-bindings: leds: add "usbport" trigger

[Rafał Miłecki]

On 1.03.2023 01:02, Rob Herring wrote:
> On Tue, Feb 28, 2023 at 03:49:33PM +0100, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> It's a trigger used on many home routers that have LEDs to indicate
>> specific USB port state.
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>>   Documentation/devicetree/bindings/leds/common.yaml | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
>> index 15e3f6645682..95b316ee3146 100644
>> --- a/Documentation/devicetree/bindings/leds/common.yaml
>> +++ b/Documentation/devicetree/bindings/leds/common.yaml
>> @@ -99,6 +99,7 @@ properties:
>>             - pattern
>>             - usb-gadget
>>             - usb-host
>> +          - usbport
> 
> Can we stop adding entries which are clearly likely to have multiple
> instances. We have a better binding to map the trigger source...

I'm sorry, I really don't understand this.
I'm not sure what do you mean by multuple "usbport" instances.
Could you point me to that better place, please?

This is probably something obvious but I really can't figure it out
since yesterday.


>>         - pattern: "^cpu[0-9]*$"
>>         - pattern: "^hci[0-9]+-power$"
>>           # LED is triggered by Bluetooth activity
>> -- 
>> 2.34.1
>>

Re: [PATCH] dt-bindings: leds: add "usbport" trigger

[Rob Herring]

On Wed, Mar 1, 2023 at 1:27 AM Rafał Miłecki <zajec5@gmail.com> wrote:
>
> On 1.03.2023 01:02, Rob Herring wrote:
> > On Tue, Feb 28, 2023 at 03:49:33PM +0100, Rafał Miłecki wrote:
> >> From: Rafał Miłecki <rafal@milecki.pl>
> >>
> >> It's a trigger used on many home routers that have LEDs to indicate
> >> specific USB port state.
> >>
> >> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> >> ---
> >>   Documentation/devicetree/bindings/leds/common.yaml | 1 +
> >>   1 file changed, 1 insertion(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
> >> index 15e3f6645682..95b316ee3146 100644
> >> --- a/Documentation/devicetree/bindings/leds/common.yaml
> >> +++ b/Documentation/devicetree/bindings/leds/common.yaml
> >> @@ -99,6 +99,7 @@ properties:
> >>             - pattern
> >>             - usb-gadget
> >>             - usb-host
> >> +          - usbport
> >
> > Can we stop adding entries which are clearly likely to have multiple
> > instances. We have a better binding to map the trigger source...
>
> I'm sorry, I really don't understand this.
> I'm not sure what do you mean by multuple "usbport" instances.
> Could you point me to that better place, please?

Suppose I have a device with 4 USB ports and 4 LEDs for each one. How
would one define the connection of LEDs to USB ports? Extend this to
usbport[0-9]? No.

> This is probably something obvious but I really can't figure it out
> since yesterday.

"trigger-sources"

Rob

Re: [PATCH] dt-bindings: leds: add "usbport" trigger

[Rafał Miłecki]

On 1.03.2023 14:43, Rob Herring wrote:
> On Wed, Mar 1, 2023 at 1:27 AM Rafał Miłecki <zajec5@gmail.com> wrote:
>>
>> On 1.03.2023 01:02, Rob Herring wrote:
>>> On Tue, Feb 28, 2023 at 03:49:33PM +0100, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>>
>>>> It's a trigger used on many home routers that have LEDs to indicate
>>>> specific USB port state.
>>>>
>>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>>> ---
>>>>    Documentation/devicetree/bindings/leds/common.yaml | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
>>>> index 15e3f6645682..95b316ee3146 100644
>>>> --- a/Documentation/devicetree/bindings/leds/common.yaml
>>>> +++ b/Documentation/devicetree/bindings/leds/common.yaml
>>>> @@ -99,6 +99,7 @@ properties:
>>>>              - pattern
>>>>              - usb-gadget
>>>>              - usb-host
>>>> +          - usbport
>>>
>>> Can we stop adding entries which are clearly likely to have multiple
>>> instances. We have a better binding to map the trigger source...
>>
>> I'm sorry, I really don't understand this.
>> I'm not sure what do you mean by multuple "usbport" instances.
>> Could you point me to that better place, please?
> 
> Suppose I have a device with 4 USB ports and 4 LEDs for each one. How
> would one define the connection of LEDs to USB ports? Extend this to
> usbport[0-9]? No.
> 
>> This is probably something obvious but I really can't figure it out
>> since yesterday.
> 
> "trigger-sources"

Ah, I suppose that "usbport" LED trigger in Linux can be confusing.

So: no matter how many USB ports you have, Linux *doesn't* create one
trigger per USB port. There is only one trigger. It's called exactly
"usbport".

Once you choose "usbport" trigger in Linux, you can choose which ports
should it "monitor". That can be done using procfs (ABI). The default
set of ports to monitor can be specified using "trigger-sources".

For decision details behind this see 0f247626cbbf ("usb: core: Introduce
a USB port LED trigger").

So Linux on home routers needs both:
1. linux,default-trigger (for selecting default trigger)
2. trigger-sources (for providing default set of ports to monitor)

Does it make more sense? Should I improve commit description and resend
it? Or should we still rework it somehow?

Re: [PATCH] dt-bindings: leds: add "usbport" trigger

[Rob Herring]

On Wed, Mar 1, 2023 at 7:52 AM Rafał Miłecki <zajec5@gmail.com> wrote:
>
> On 1.03.2023 14:43, Rob Herring wrote:
> > On Wed, Mar 1, 2023 at 1:27 AM Rafał Miłecki <zajec5@gmail.com> wrote:
> >>
> >> On 1.03.2023 01:02, Rob Herring wrote:
> >>> On Tue, Feb 28, 2023 at 03:49:33PM +0100, Rafał Miłecki wrote:
> >>>> From: Rafał Miłecki <rafal@milecki.pl>
> >>>>
> >>>> It's a trigger used on many home routers that have LEDs to indicate
> >>>> specific USB port state.
> >>>>
> >>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> >>>> ---
> >>>>    Documentation/devicetree/bindings/leds/common.yaml | 1 +
> >>>>    1 file changed, 1 insertion(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
> >>>> index 15e3f6645682..95b316ee3146 100644
> >>>> --- a/Documentation/devicetree/bindings/leds/common.yaml
> >>>> +++ b/Documentation/devicetree/bindings/leds/common.yaml
> >>>> @@ -99,6 +99,7 @@ properties:
> >>>>              - pattern
> >>>>              - usb-gadget
> >>>>              - usb-host
> >>>> +          - usbport
> >>>
> >>> Can we stop adding entries which are clearly likely to have multiple
> >>> instances. We have a better binding to map the trigger source...
> >>
> >> I'm sorry, I really don't understand this.
> >> I'm not sure what do you mean by multuple "usbport" instances.
> >> Could you point me to that better place, please?
> >
> > Suppose I have a device with 4 USB ports and 4 LEDs for each one. How
> > would one define the connection of LEDs to USB ports? Extend this to
> > usbport[0-9]? No.
> >
> >> This is probably something obvious but I really can't figure it out
> >> since yesterday.
> >
> > "trigger-sources"
>
> Ah, I suppose that "usbport" LED trigger in Linux can be confusing.
>
> So: no matter how many USB ports you have, Linux *doesn't* create one
> trigger per USB port. There is only one trigger. It's called exactly
> "usbport".
>
> Once you choose "usbport" trigger in Linux, you can choose which ports
> should it "monitor". That can be done using procfs (ABI). The default
> set of ports to monitor can be specified using "trigger-sources".
>
> For decision details behind this see 0f247626cbbf ("usb: core: Introduce
> a USB port LED trigger").
>
> So Linux on home routers needs both:
> 1. linux,default-trigger (for selecting default trigger)
> 2. trigger-sources (for providing default set of ports to monitor)

I still don't understand why defining a trigger source doesn't also
set the default. It is after all just the default. The DT should have
how the LEDs are intended to be used by the design, not what a user
wants. A user should change that from userspace.

In any case, just make the commit message clear this is not a new
trigger, but one that has been in use for some time.

Rob