[PATCHv2 0/3] wifi: ath9k: ahb: add OF LED support

[PATCHv2 0/3] wifi: ath9k: ahb: add OF LED support

[Rosen Penev]

A port of the prior platform code to use OF.

v2: use reg instead of led-sources

Rosen Penev (3):
  dt-bindings: net: wireless: ath9k: add led bindings
  wifi: ath9k: ahb: add led pin OF support
  mips: qca: use led-sources for WMAC LED

 .../bindings/net/wireless/qca,ath9k.yaml           | 14 ++++++++++++++
 arch/mips/boot/dts/qca/ar9132_tl_wr1043nd_v1.dts   | 10 +++++-----
 arch/mips/boot/dts/qca/ar9331_dragino_ms14.dts     | 10 ++++------
 arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts        | 10 ++++------
 drivers/net/wireless/ath/ath9k/init.c              | 11 +++++++++++
 5 files changed, 38 insertions(+), 17 deletions(-)

-- 
2.50.1

[PATCHv2 1/3] dt-bindings: net: wireless: ath9k: add led bindings

[Rosen Penev]

The ath9k driver has various pin GPIO numbers for different chipsets
which are not always correct for every device.

Add bindings to specify the correct number and if it should be
active-low.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 .../bindings/net/wireless/qca,ath9k.yaml           | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
index d16ca8e0a25d..e701046146f2 100644
--- a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
+++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
@@ -50,6 +50,17 @@ properties:
 
   ieee80211-freq-limit: true
 
+  led:
+    type: object
+    properties:
+      reg:
+        maxItems: 1
+
+      led-active-low:
+        description:
+          LED is enabled with ground signal.
+        type: boolean
+
   qca,no-eeprom:
     $ref: /schemas/types.yaml#/definitions/flag
     description:
@@ -102,5 +113,8 @@ examples:
         compatible = "qca,ar9130-wifi";
         reg = <0x180c0000 0x230000>;
         interrupts = <2>;
+        led {
+          led-sources = <0>;
+        };
       };
     };
-- 
2.50.1

Re: [PATCHv2 1/3] dt-bindings: net: wireless: ath9k: add led bindings

[Krzysztof Kozlowski]

On 12/08/2025 21:23, Rosen Penev wrote:
> The ath9k driver has various pin GPIO numbers for different chipsets
> which are not always correct for every device.
> 
> Add bindings to specify the correct number and if it should be
> active-low.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  .../bindings/net/wireless/qca,ath9k.yaml           | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
> index d16ca8e0a25d..e701046146f2 100644
> --- a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
> +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
> @@ -50,6 +50,17 @@ properties:
>  
>    ieee80211-freq-limit: true
>  
> +  led:
> +    type: object

Each node must end with additional/unevaluatedProperties: false. See
example schema and writing schema.

That will probably lead you to missing LED common binding.

> +    properties:
> +      reg:
> +        maxItems: 1
> +
> +      led-active-low:
> +        description:
> +          LED is enabled with ground signal.

Aren't you redefining existing properties?

> +        type: boolean
> +
>    qca,no-eeprom:
>      $ref: /schemas/types.yaml#/definitions/flag
>      description:
> @@ -102,5 +113,8 @@ examples:
>          compatible = "qca,ar9130-wifi";
>          reg = <0x180c0000 0x230000>;
>          interrupts = <2>;
> +        led {
> +          led-sources = <0>;

Totally confusing with schema. active-low in one place, different
property in the example and no source for that property at all :/


Best regards,
Krzysztof

Re: [PATCHv2 1/3] dt-bindings: net: wireless: ath9k: add led bindings

[Rosen Penev]

On Wed, Aug 13, 2025 at 1:16 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 12/08/2025 21:23, Rosen Penev wrote:
> > The ath9k driver has various pin GPIO numbers for different chipsets
> > which are not always correct for every device.
> >
> > Add bindings to specify the correct number and if it should be
> > active-low.
> >
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >  .../bindings/net/wireless/qca,ath9k.yaml           | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
> > index d16ca8e0a25d..e701046146f2 100644
> > --- a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
> > +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
> > @@ -50,6 +50,17 @@ properties:
> >
> >    ieee80211-freq-limit: true
> >
> > +  led:
> > +    type: object
>
> Each node must end with additional/unevaluatedProperties: false. See
> example schema and writing schema.
>
> That will probably lead you to missing LED common binding.

>
> > +    properties:
> > +      reg:
> > +        maxItems: 1
> > +
> > +      led-active-low:
> > +        description:
> > +          LED is enabled with ground signal.
>
> Aren't you redefining existing properties?
I don't think led-active-low is specified in any central location:

Documentation/devicetree/bindings/leds/irled/ir-spi-led.yaml:  led-active-low:
Documentation/devicetree/bindings/leds/irled/ir-spi-led.yaml:
  led-active-low;
Documentation/devicetree/bindings/net/wireless/mediatek,mt76.yaml:
 led-active-low:
Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml:
led-active-low:
Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml:
 led-active-low;
>
> > +        type: boolean
> > +
> >    qca,no-eeprom:
> >      $ref: /schemas/types.yaml#/definitions/flag
> >      description:
> > @@ -102,5 +113,8 @@ examples:
> >          compatible = "qca,ar9130-wifi";
> >          reg = <0x180c0000 0x230000>;
> >          interrupts = <2>;
> > +        led {
> > +          led-sources = <0>;
>
> Totally confusing with schema. active-low in one place, different
> property in the example and no source for that property at all :/
Ah right. Will fix.
>
>
> Best regards,
> Krzysztof

Re: [PATCHv2 1/3] dt-bindings: net: wireless: ath9k: add led bindings

[Krzysztof Kozlowski]

On Wed, Aug 13, 2025 at 04:04:27PM -0700, Rosen Penev wrote:
> On Wed, Aug 13, 2025 at 1:16 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > On 12/08/2025 21:23, Rosen Penev wrote:
> > > The ath9k driver has various pin GPIO numbers for different chipsets
> > > which are not always correct for every device.
> > >
> > > Add bindings to specify the correct number and if it should be
> > > active-low.
> > >
> > > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > > ---
> > >  .../bindings/net/wireless/qca,ath9k.yaml           | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
> > > index d16ca8e0a25d..e701046146f2 100644
> > > --- a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
> > > +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
> > > @@ -50,6 +50,17 @@ properties:
> > >
> > >    ieee80211-freq-limit: true
> > >
> > > +  led:
> > > +    type: object
> >
> > Each node must end with additional/unevaluatedProperties: false. See
> > example schema and writing schema.
> >
> > That will probably lead you to missing LED common binding.
> 
> >
> > > +    properties:
> > > +      reg:
> > > +        maxItems: 1
> > > +
> > > +      led-active-low:
> > > +        description:
> > > +          LED is enabled with ground signal.
> >
> > Aren't you redefining existing properties?
> I don't think led-active-low is specified in any central location:

True, because it is active-low which you could easily find if you opened
and read the LED binding you were already referencing here.

Best regards,
Krzysztof

[PATCHv2 2/3] wifi: ath9k: ahb: add led pin OF support

[Rosen Penev]

The old and removed platform code had support for assigning a value for
the LED pin for when the default is not correct. Effectively a fix for a
non working LED.

For setting an LED to active high, a negation of led-active-low is used,
as two drivers currently use that and no drivers use led-active-high or
something similar.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/wireless/ath/ath9k/init.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index c911b178dcc2..7826b113235d 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -662,6 +662,17 @@ static int ath9k_of_init(struct ath_softc *sc)
 	if (ret == -EPROBE_DEFER)
 		return ret;
 
+	np = of_get_child_by_name(np, "led");
+	if (np && of_device_is_available(np)) {
+		u32 led_pin;
+
+		if (!of_property_read_u32(np, "reg", &led_pin))
+			ah->led_pin = led_pin;
+
+		ah->config.led_active_high = !of_property_read_bool(np, "led-active-low");
+		of_node_put(np);
+	}
+
 	return 0;
 }
 
-- 
2.50.1

Re: [PATCHv2 2/3] wifi: ath9k: ahb: add led pin OF support

[Krzysztof Kozlowski]

On 12/08/2025 21:23, Rosen Penev wrote:
> The old and removed platform code had support for assigning a value for
> the LED pin for when the default is not correct. Effectively a fix for a
> non working LED.
> 
> For setting an LED to active high, a negation of led-active-low is used,
> as two drivers currently use that and no drivers use led-active-high or
> something similar.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/net/wireless/ath/ath9k/init.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
> index c911b178dcc2..7826b113235d 100644
> --- a/drivers/net/wireless/ath/ath9k/init.c
> +++ b/drivers/net/wireless/ath/ath9k/init.c
> @@ -662,6 +662,17 @@ static int ath9k_of_init(struct ath_softc *sc)
>  	if (ret == -EPROBE_DEFER)
>  		return ret;
>  
> +	np = of_get_child_by_name(np, "led");
> +	if (np && of_device_is_available(np)) {

You are open-coding of_get_available_child_by_name().

> +		u32 led_pin;
> +
> +		if (!of_property_read_u32(np, "reg", &led_pin))
> +			ah->led_pin = led_pin;
> +
> +		ah->config.led_active_high = !of_property_read_bool(np, "led-active-low");
> +		of_node_put(np);
> +	}

Leaking OF node.

> +
>  	return 0;
>  }
>  


Best regards,
Krzysztof

Re: [PATCHv2 2/3] wifi: ath9k: ahb: add led pin OF support

[Rosen Penev]

On Wed, Aug 13, 2025 at 1:20 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 12/08/2025 21:23, Rosen Penev wrote:
> > The old and removed platform code had support for assigning a value for
> > the LED pin for when the default is not correct. Effectively a fix for a
> > non working LED.
> >
> > For setting an LED to active high, a negation of led-active-low is used,
> > as two drivers currently use that and no drivers use led-active-high or
> > something similar.
> >
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >  drivers/net/wireless/ath/ath9k/init.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
> > index c911b178dcc2..7826b113235d 100644
> > --- a/drivers/net/wireless/ath/ath9k/init.c
> > +++ b/drivers/net/wireless/ath/ath9k/init.c
> > @@ -662,6 +662,17 @@ static int ath9k_of_init(struct ath_softc *sc)
> >       if (ret == -EPROBE_DEFER)
> >               return ret;
> >
> > +     np = of_get_child_by_name(np, "led");
> > +     if (np && of_device_is_available(np)) {
>
> You are open-coding of_get_available_child_by_name().
Will fix.
>
> > +             u32 led_pin;
> > +
> > +             if (!of_property_read_u32(np, "reg", &led_pin))
> > +                     ah->led_pin = led_pin;
> > +
> > +             ah->config.led_active_high = !of_property_read_bool(np, "led-active-low");
> > +             of_node_put(np);
> > +     }
>
> Leaking OF node.

Not following here.
>
> > +
> >       return 0;
> >  }
> >
>
>
> Best regards,
> Krzysztof

Re: [PATCHv2 2/3] wifi: ath9k: ahb: add led pin OF support

[Krzysztof Kozlowski]

On 14/08/2025 00:59, Rosen Penev wrote:
>>
>>> +             u32 led_pin;
>>> +
>>> +             if (!of_property_read_u32(np, "reg", &led_pin))
>>> +                     ah->led_pin = led_pin;
>>> +
>>> +             ah->config.led_active_high = !of_property_read_bool(np, "led-active-low");
>>> +             of_node_put(np);
>>> +     }
>>
>> Leaking OF node.
> 
> Not following here.

You miss proper cleanup, leaking of node.

Best regards,
Krzysztof

[PATCHv2 3/3] mips: qca: use led-sources for WMAC LED

[Rosen Penev]

The ath9k driver creates an LED unconditionally being driven with
sometimes the wrong pin. Not only that, the current dts definitions have
LEDs for the WMAC that do not behave in response to it. Fix both issues.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 arch/mips/boot/dts/qca/ar9132_tl_wr1043nd_v1.dts | 10 +++++-----
 arch/mips/boot/dts/qca/ar9331_dragino_ms14.dts   | 10 ++++------
 arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts      | 10 ++++------
 3 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/arch/mips/boot/dts/qca/ar9132_tl_wr1043nd_v1.dts b/arch/mips/boot/dts/qca/ar9132_tl_wr1043nd_v1.dts
index a7901bb040ce..344e1a2ee6ea 100644
--- a/arch/mips/boot/dts/qca/ar9132_tl_wr1043nd_v1.dts
+++ b/arch/mips/boot/dts/qca/ar9132_tl_wr1043nd_v1.dts
@@ -56,11 +56,6 @@ led-2 {
 			label = "tp-link:green:qss";
 			gpios = <&gpio 5 GPIO_ACTIVE_HIGH>;
 		};
-
-		led-3 {
-			label = "tp-link:green:wlan";
-			gpios = <&gpio 9 GPIO_ACTIVE_LOW>;
-		};
 	};
 };
 
@@ -111,4 +106,9 @@ partition@2 {
 
 &wifi {
 	status = "okay";
+
+	led {
+		reg = <9>;
+		led-active-low;
+	};
 };
diff --git a/arch/mips/boot/dts/qca/ar9331_dragino_ms14.dts b/arch/mips/boot/dts/qca/ar9331_dragino_ms14.dts
index 37a74aabe4b4..573ca7752698 100644
--- a/arch/mips/boot/dts/qca/ar9331_dragino_ms14.dts
+++ b/arch/mips/boot/dts/qca/ar9331_dragino_ms14.dts
@@ -22,12 +22,6 @@ memory@0 {
 	leds {
 		compatible = "gpio-leds";
 
-		led-wlan {
-			label = "dragino2:red:wlan";
-			gpios = <&gpio 0 GPIO_ACTIVE_HIGH>;
-			default-state = "off";
-		};
-
 		led-lan {
 			label = "dragino2:red:lan";
 			gpios = <&gpio 13 GPIO_ACTIVE_LOW>;
@@ -101,4 +95,8 @@ spiflash: w25q128@0 {
 
 &wifi {
 	status = "okay";
+
+	led {
+		reg = <0>;
+	};
 };
diff --git a/arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts b/arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts
index a7108c803eb3..6891d9589b68 100644
--- a/arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts
+++ b/arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts
@@ -22,12 +22,6 @@ memory@0 {
 	leds {
 		compatible = "gpio-leds";
 
-		led-wlan {
-			label = "tp-link:green:wlan";
-			gpios = <&gpio 0 GPIO_ACTIVE_HIGH>;
-			default-state = "off";
-		};
-
 		led-lan {
 			label = "tp-link:green:lan";
 			gpios = <&gpio 17 GPIO_ACTIVE_LOW>;
@@ -117,4 +111,8 @@ spiflash: s25sl032p@0 {
 
 &wifi {
 	status = "okay";
+
+	led {
+		reg = <0>;
+	};
 };
-- 
2.50.1

Re: [PATCHv2 0/3] wifi: ath9k: ahb: add OF LED support

[Krzysztof Kozlowski]

On 12/08/2025 21:23, Rosen Penev wrote:
> A port of the prior platform code to use OF.
> 
> v2: use reg instead of led-sources

Where? Really, where? There is no reg in the binding at all. There is
led-sources, though. Also many other things got changed, the binding is
completely different and nothing in changelog explains that.

Best regards,
Krzysztof

Re: [PATCHv2 0/3] wifi: ath9k: ahb: add OF LED support

[Rosen Penev]

On Wed, Aug 13, 2025 at 1:18 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 12/08/2025 21:23, Rosen Penev wrote:
> > A port of the prior platform code to use OF.
> >
> > v2: use reg instead of led-sources
>
> Where? Really, where? There is no reg in the binding at all. There is
> led-sources, though. Also many other things got changed, the binding is
> completely different and nothing in changelog explains that.
Yeah. The initial was copy/paste from mt76 driver. But then on review
I was told to change led-sources to reg and did not completely change
that in the patchset.
>
> Best regards,
> Krzysztof