2023-02-07 10:41:40

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: mt76: add active-low property to led

On 07/02/2023 11:25, Frank Wunderlich wrote:
> From: Frank Wunderlich <[email protected]>
>
> LEDs can be in low-active mode, so add dt property for it.
>
> Signed-off-by: Frank Wunderlich <[email protected]>
> ---
> .../devicetree/bindings/net/wireless/mediatek,mt76.yaml | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.yaml b/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.yaml
> index f0c78f994491..212508672979 100644
> --- a/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.yaml
> +++ b/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.yaml
> @@ -112,6 +112,11 @@ properties:
> $ref: /schemas/leds/common.yaml#
> additionalProperties: false
> properties:
> + led-active-low:
> + description:
> + LED is enabled with ground signal.

What does it mean? You set voltage of regulator to 0? Or you set GPIO as
0? If the latter, it's not the property of LED...

Best regards,
Krzysztof



2023-02-07 12:13:10

by Frank Wunderlich

[permalink] [raw]
Subject: Aw: Re: [PATCH] dt-bindings: mt76: add active-low property to led

> Gesendet: Dienstag, 07. Februar 2023 um 11:40 Uhr
> Von: "Krzysztof Kozlowski" <[email protected]>
> On 07/02/2023 11:25, Frank Wunderlich wrote:
> > From: Frank Wunderlich <[email protected]>
> >
> > LEDs can be in low-active mode, so add dt property for it.
> >
> > Signed-off-by: Frank Wunderlich <[email protected]>
> > ---
> > .../devicetree/bindings/net/wireless/mediatek,mt76.yaml | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.yaml b/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.yaml
> > index f0c78f994491..212508672979 100644
> > --- a/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.yaml
> > +++ b/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.yaml
> > @@ -112,6 +112,11 @@ properties:
> > $ref: /schemas/leds/common.yaml#
> > additionalProperties: false
> > properties:
> > + led-active-low:
> > + description:
> > + LED is enabled with ground signal.
>
> What does it mean? You set voltage of regulator to 0? Or you set GPIO as
> 0? If the latter, it's not the property of LED...

basicly it is a gpio-led mapped into the mt76 driver, but not passing gpio itself in this property (like gpio-led does).
This gpio is set to 0 signal (gnd) to let the led go on ;) so imho it is a led-property, but below the wifi-node as
the trigger comes from mt76 hardware, not an external (soc) gpio controller.

mt76 driver supports it already like i post change here:

https://patchwork.kernel.org/project/linux-mediatek/patch/[email protected]/

only needed the binding for it.

> Best regards,
> Krzysztof
>
>

2023-02-07 12:26:29

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: Aw: Re: [PATCH] dt-bindings: mt76: add active-low property to led

On 07/02/2023 13:12, Frank Wunderlich wrote:
>> Gesendet: Dienstag, 07. Februar 2023 um 11:40 Uhr
>> Von: "Krzysztof Kozlowski" <[email protected]>
>> On 07/02/2023 11:25, Frank Wunderlich wrote:
>>> From: Frank Wunderlich <[email protected]>
>>>
>>> LEDs can be in low-active mode, so add dt property for it.
>>>
>>> Signed-off-by: Frank Wunderlich <[email protected]>
>>> ---
>>> .../devicetree/bindings/net/wireless/mediatek,mt76.yaml | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.yaml b/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.yaml
>>> index f0c78f994491..212508672979 100644
>>> --- a/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.yaml
>>> +++ b/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.yaml
>>> @@ -112,6 +112,11 @@ properties:
>>> $ref: /schemas/leds/common.yaml#
>>> additionalProperties: false
>>> properties:
>>> + led-active-low:
>>> + description:
>>> + LED is enabled with ground signal.
>>
>> What does it mean? You set voltage of regulator to 0? Or you set GPIO as
>> 0? If the latter, it's not the property of LED...
>
> basicly it is a gpio-led mapped into the mt76 driver, but not passing gpio itself in this property (like gpio-led does).
> This gpio is set to 0 signal (gnd) to let the led go on ;) so imho it is a led-property, but below the wifi-node as
> the trigger comes from mt76 hardware, not an external (soc) gpio controller.
>
> mt76 driver supports it already like i post change here:
>

If the driver supports it already and it was never documented, please
state it. Your commit says you add a new property.

Best regards,
Krzysztof