[PATCH net-next 1/4] dt-bindings: leds: add 'active-high' property

[PATCH net-next 1/4] dt-bindings: leds: add 'active-high' property

[Daniel Golle]

Other than described in commit c94d1783136 ("dt-bindings: net: phy: Make
LED active-low property common") the absence of the 'active-low'
property means not to touch the polarity settings which are inherited
from reset defaults, the bootloader or bootstrap configuration.
Hence, in order to override a LED pin being active-high in case of the
default, bootloader or bootstrap setting being active-low an additional
property 'active-high' is required.
Document that property and make it mutually exclusive to the existing
'active-low' property.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 Documentation/devicetree/bindings/leds/common.yaml | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
index bf9a101e4d42..7c3cd7b7412e 100644
--- a/Documentation/devicetree/bindings/leds/common.yaml
+++ b/Documentation/devicetree/bindings/leds/common.yaml
@@ -202,6 +202,12 @@ properties:
       #trigger-source-cells property in the source node.
     $ref: /schemas/types.yaml#/definitions/phandle-array
 
+  active-high:
+    type: boolean
+    description:
+      Makes LED active high. To turn the LED ON, line needs to be
+      set to high voltage instead of low.
+
   active-low:
     type: boolean
     description:
@@ -225,6 +231,14 @@ properties:
       Maximum timeout in microseconds after which the flash LED is turned off.
       Required for flash LED nodes with configurable timeout.
 
+allOf:
+  - if:
+      required:
+        - active-low
+    then:
+      properties:
+        active-high: false
+
 additionalProperties: true
 
 examples:
-- 
2.46.2

[PATCH net-next 2/4] net: phy: support 'active-high' property for PHY LEDs

[Daniel Golle]

In addition to 'active-low' and 'inactive-high-impedance' also
support 'active-high' property for PHY LED pin configuration.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/net/phy/phy_device.c | 2 ++
 include/linux/phy.h          | 5 +++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 560e338b307a..df67d9297253 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -3357,6 +3357,8 @@ static int of_phy_led(struct phy_device *phydev,
 	if (index > U8_MAX)
 		return -EINVAL;
 
+	if (of_property_read_bool(led, "active-high"))
+		set_bit(PHY_LED_ACTIVE_HIGH, &modes);
 	if (of_property_read_bool(led, "active-low"))
 		set_bit(PHY_LED_ACTIVE_LOW, &modes);
 	if (of_property_read_bool(led, "inactive-high-impedance"))
diff --git a/include/linux/phy.h b/include/linux/phy.h
index a98bc91a0cde..22dde07cbec7 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -877,8 +877,9 @@ struct phy_plca_status {
 
 /* Modes for PHY LED configuration */
 enum phy_led_modes {
-	PHY_LED_ACTIVE_LOW = 0,
-	PHY_LED_INACTIVE_HIGH_IMPEDANCE = 1,
+	PHY_LED_ACTIVE_HIGH = 0,
+	PHY_LED_ACTIVE_LOW = 1,
+	PHY_LED_INACTIVE_HIGH_IMPEDANCE = 2,
 
 	/* keep it last */
 	__PHY_LED_MODES_NUM,
-- 
2.46.2

[PATCH net-next 3/4] net: phy: aquantia: correctly describe LED polarity override

[Daniel Golle]

Use newly defined 'active-high' property to set the
VEND1_GLOBAL_LED_DRIVE_VDD bit and let 'active-low' clear that bit. This
reflects the technical reality which was inverted in the previous
description in which the 'active-low' property was used to actually set
the VEND1_GLOBAL_LED_DRIVE_VDD bit, which means that VDD (ie. supply
voltage) of the LED is driven rather than GND.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/net/phy/aquantia/aquantia.h      |  1 +
 drivers/net/phy/aquantia/aquantia_leds.c |  6 +++++-
 drivers/net/phy/aquantia/aquantia_main.c | 12 +++++++++---
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/aquantia/aquantia.h b/drivers/net/phy/aquantia/aquantia.h
index 2465345081f8..0c78bfabace5 100644
--- a/drivers/net/phy/aquantia/aquantia.h
+++ b/drivers/net/phy/aquantia/aquantia.h
@@ -177,6 +177,7 @@ static const struct aqr107_hw_stat aqr107_hw_stats[] = {
 struct aqr107_priv {
 	u64 sgmii_stats[AQR107_SGMII_STAT_SZ];
 	unsigned long leds_active_low;
+	unsigned long leds_active_high;
 };
 
 #if IS_REACHABLE(CONFIG_HWMON)
diff --git a/drivers/net/phy/aquantia/aquantia_leds.c b/drivers/net/phy/aquantia/aquantia_leds.c
index 201c8df93fad..21b8ede2a105 100644
--- a/drivers/net/phy/aquantia/aquantia_leds.c
+++ b/drivers/net/phy/aquantia/aquantia_leds.c
@@ -121,7 +121,7 @@ int aqr_phy_led_active_low_set(struct phy_device *phydev, int index, bool enable
 {
 	return phy_modify_mmd(phydev, MDIO_MMD_VEND1, AQR_LED_DRIVE(index),
 			      VEND1_GLOBAL_LED_DRIVE_VDD,
-			      enable ? VEND1_GLOBAL_LED_DRIVE_VDD : 0);
+			      enable ? 0 : VEND1_GLOBAL_LED_DRIVE_VDD);
 }
 
 int aqr_phy_led_polarity_set(struct phy_device *phydev, int index, unsigned long modes)
@@ -138,6 +138,8 @@ int aqr_phy_led_polarity_set(struct phy_device *phydev, int index, unsigned long
 		case PHY_LED_ACTIVE_LOW:
 			active_low = true;
 			break;
+		case PHY_LED_ACTIVE_HIGH:
+			break;
 		default:
 			return -EINVAL;
 		}
@@ -146,6 +148,8 @@ int aqr_phy_led_polarity_set(struct phy_device *phydev, int index, unsigned long
 	/* Save LED driver vdd state to restore on SW reset */
 	if (active_low)
 		priv->leds_active_low |= BIT(index);
+	else
+		priv->leds_active_high |= BIT(index);
 
 	return aqr_phy_led_active_low_set(phydev, index, active_low);
 }
diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
index c33a5ef34ba0..c7381d5ede37 100644
--- a/drivers/net/phy/aquantia/aquantia_main.c
+++ b/drivers/net/phy/aquantia/aquantia_main.c
@@ -488,7 +488,7 @@ static void aqr107_chip_info(struct phy_device *phydev)
 static int aqr107_config_init(struct phy_device *phydev)
 {
 	struct aqr107_priv *priv = phydev->priv;
-	u32 led_active_low;
+	u32 led_idx;
 	int ret;
 
 	/* Check that the PHY interface type is compatible */
@@ -515,8 +515,14 @@ static int aqr107_config_init(struct phy_device *phydev)
 		return ret;
 
 	/* Restore LED polarity state after reset */
-	for_each_set_bit(led_active_low, &priv->leds_active_low, AQR_MAX_LEDS) {
-		ret = aqr_phy_led_active_low_set(phydev, led_active_low, true);
+	for_each_set_bit(led_idx, &priv->leds_active_low, AQR_MAX_LEDS) {
+		ret = aqr_phy_led_active_low_set(phydev, led_idx, true);
+		if (ret)
+			return ret;
+	}
+
+	for_each_set_bit(led_idx, &priv->leds_active_high, AQR_MAX_LEDS) {
+		ret = aqr_phy_led_active_low_set(phydev, led_idx, false);
 		if (ret)
 			return ret;
 	}
-- 
2.46.2

[PATCH net-next 4/4] net: phy: mxl-gpy: correctly describe LED polarity

[Daniel Golle]

According the datasheet covering the LED (0x1b) register:
0B Active High LEDx pin driven high when activated
1B Active Low LEDx pin driven low when activated

Make use of the now available 'active-high' property and correctly
reflect the polarity setting which was previously inverted.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/net/phy/mxl-gpy.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
index 1c9cd00c7bee..f50697f35160 100644
--- a/drivers/net/phy/mxl-gpy.c
+++ b/drivers/net/phy/mxl-gpy.c
@@ -1013,13 +1013,15 @@ static int gpy_led_polarity_set(struct phy_device *phydev, int index,
 		case PHY_LED_ACTIVE_LOW:
 			active_low = true;
 			break;
+		case PHY_LED_ACTIVE_HIGH:
+			break;
 		default:
 			return -EINVAL;
 		}
 	}
 
 	return phy_modify(phydev, PHY_LED, PHY_LED_POLARITY(index),
-			  active_low ? 0 : PHY_LED_POLARITY(index));
+			  active_low ? PHY_LED_POLARITY(index) : 0);
 }
 
 static struct phy_driver gpy_drivers[] = {
-- 
2.46.2

Re: [PATCH net-next 1/4] dt-bindings: leds: add 'active-high' property

[Krzysztof Kozlowski]

On Sat, Oct 05, 2024 at 05:24:20PM +0100, Daniel Golle wrote:
> Other than described in commit c94d1783136 ("dt-bindings: net: phy: Make

Please run scripts/checkpatch.pl and fix reported warnings. Then please
run 'scripts/checkpatch.pl --strict' and (probably) fix more warnings.
Some warnings can be ignored, especially from --strict run, but the code
here looks like it needs a fix. Feel free to get in touch if the warning
is not clear.

> LED active-low property common") the absence of the 'active-low'
> property means not to touch the polarity settings which are inherited
> from reset defaults, the bootloader or bootstrap configuration.
> Hence, in order to override a LED pin being active-high in case of the
> default, bootloader or bootstrap setting being active-low an additional
> property 'active-high' is required.
> Document that property and make it mutually exclusive to the existing
> 'active-low' property.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>  Documentation/devicetree/bindings/leds/common.yaml | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
> index bf9a101e4d42..7c3cd7b7412e 100644
> --- a/Documentation/devicetree/bindings/leds/common.yaml
> +++ b/Documentation/devicetree/bindings/leds/common.yaml
> @@ -202,6 +202,12 @@ properties:
>        #trigger-source-cells property in the source node.
>      $ref: /schemas/types.yaml#/definitions/phandle-array
>  
> +  active-high:
> +    type: boolean
> +    description:
> +      Makes LED active high. To turn the LED ON, line needs to be
> +      set to high voltage instead of low.

And then we are going to get 2 more bools for other variants...

I think this should be just string enum, see marvell,marvell10g.yaml

Best regards,
Krzysztof

Re: [PATCH net-next 1/4] dt-bindings: leds: add 'active-high' property

[Daniel Golle]

On Sun, Oct 06, 2024 at 02:44:44PM +0200, Krzysztof Kozlowski wrote:
> On Sat, Oct 05, 2024 at 05:24:20PM +0100, Daniel Golle wrote:
> > Other than described in commit c94d1783136 ("dt-bindings: net: phy: Make
> 
> Please run scripts/checkpatch.pl and fix reported warnings. Then please
> run 'scripts/checkpatch.pl --strict' and (probably) fix more warnings.
> Some warnings can be ignored, especially from --strict run, but the code
> here looks like it needs a fix. Feel free to get in touch if the warning
> is not clear.

Sorry about that, I was expecting '--fix-inplace' to take care of that
but it didn't and I didn't notice. I will address that in a follow-up
patch.

> 
> > LED active-low property common") the absence of the 'active-low'
> > property means not to touch the polarity settings which are inherited
> > from reset defaults, the bootloader or bootstrap configuration.
> > Hence, in order to override a LED pin being active-high in case of the
> > default, bootloader or bootstrap setting being active-low an additional
> > property 'active-high' is required.
> > Document that property and make it mutually exclusive to the existing
> > 'active-low' property.
> > 
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> >  Documentation/devicetree/bindings/leds/common.yaml | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
> > index bf9a101e4d42..7c3cd7b7412e 100644
> > --- a/Documentation/devicetree/bindings/leds/common.yaml
> > +++ b/Documentation/devicetree/bindings/leds/common.yaml
> > @@ -202,6 +202,12 @@ properties:
> >        #trigger-source-cells property in the source node.
> >      $ref: /schemas/types.yaml#/definitions/phandle-array
> >  
> > +  active-high:
> > +    type: boolean
> > +    description:
> > +      Makes LED active high. To turn the LED ON, line needs to be
> > +      set to high voltage instead of low.
> 
> And then we are going to get 2 more bools for other variants...

I don't see a problem combining 'active-high' or 'active-low' with
'inactive-high-impedance' which would be the equivalent of
'active-low-tristate' and 'active-high-tristate'.

> 
> I think this should be just string enum, see marvell,marvell10g.yaml

I found the vendor-specific 'marvell,polarity' property in
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231214201442.660447-5-tobias@waldekranz.com/

However, I can't find that file in any Linux tree.

Looking at the suggested patch on patchwork, I got a few questions on
how to deal with the situation as of today:

So should the existing support for the 'active-low' and
'inactive-high-impedance' properties be replaced by that string enum?
Or should the string property be interpreted in addition to the
bools defined in leds/common.yaml?

Should the string property be defined for each PHY or should we move
it into a common file?

If so, should that common file also be leds/common.yaml or should we
create a new file only for PHY LEDs instead?

Sorry for being confused, I don't mind going down what ever path to have
LED polarity configurable properly in DT.

Re: [PATCH net-next 1/4] dt-bindings: leds: add 'active-high' property

[Krzysztof Kozlowski]

On Sun, Oct 06, 2024 at 02:04:35PM +0100, Daniel Golle wrote:
> On Sun, Oct 06, 2024 at 02:44:44PM +0200, Krzysztof Kozlowski wrote:
> > On Sat, Oct 05, 2024 at 05:24:20PM +0100, Daniel Golle wrote:
> > > Other than described in commit c94d1783136 ("dt-bindings: net: phy: Make
> > 
> > Please run scripts/checkpatch.pl and fix reported warnings. Then please
> > run 'scripts/checkpatch.pl --strict' and (probably) fix more warnings.
> > Some warnings can be ignored, especially from --strict run, but the code
> > here looks like it needs a fix. Feel free to get in touch if the warning
> > is not clear.
> 
> Sorry about that, I was expecting '--fix-inplace' to take care of that
> but it didn't and I didn't notice. I will address that in a follow-up
> patch.
> 
> > 
> > > LED active-low property common") the absence of the 'active-low'
> > > property means not to touch the polarity settings which are inherited
> > > from reset defaults, the bootloader or bootstrap configuration.
> > > Hence, in order to override a LED pin being active-high in case of the
> > > default, bootloader or bootstrap setting being active-low an additional
> > > property 'active-high' is required.
> > > Document that property and make it mutually exclusive to the existing
> > > 'active-low' property.
> > > 
> > > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > > ---
> > >  Documentation/devicetree/bindings/leds/common.yaml | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
> > > index bf9a101e4d42..7c3cd7b7412e 100644
> > > --- a/Documentation/devicetree/bindings/leds/common.yaml
> > > +++ b/Documentation/devicetree/bindings/leds/common.yaml
> > > @@ -202,6 +202,12 @@ properties:
> > >        #trigger-source-cells property in the source node.
> > >      $ref: /schemas/types.yaml#/definitions/phandle-array
> > >  
> > > +  active-high:
> > > +    type: boolean
> > > +    description:
> > > +      Makes LED active high. To turn the LED ON, line needs to be
> > > +      set to high voltage instead of low.
> > 
> > And then we are going to get 2 more bools for other variants...
> 
> I don't see a problem combining 'active-high' or 'active-low' with
> 'inactive-high-impedance' which would be the equivalent of
> 'active-low-tristate' and 'active-high-tristate'.

Oh, I missed that we have already two bool properties. I thought that
there is only active-high.

> 
> > 
> > I think this should be just string enum, see marvell,marvell10g.yaml
> 
> I found the vendor-specific 'marvell,polarity' property in
> https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231214201442.660447-5-tobias@waldekranz.com/
> 
> However, I can't find that file in any Linux tree.
> 
> Looking at the suggested patch on patchwork, I got a few questions on
> how to deal with the situation as of today:
> 
> So should the existing support for the 'active-low' and
> 'inactive-high-impedance' properties be replaced by that string enum?
> Or should the string property be interpreted in addition to the
> bools defined in leds/common.yaml?
> 
> Should the string property be defined for each PHY or should we move
> it into a common file?
> 
> If so, should that common file also be leds/common.yaml or should we
> create a new file only for PHY LEDs instead?
> 
> Sorry for being confused, I don't mind going down what ever path to have
> LED polarity configurable properly in DT.

Let's ignore my idea.

However I still wonder whether your choice for lack of properties is
appropriate. Lack of properties as "bootloader default" means it can
change. Why would anyone prefer to keep bootloader default? The wiring
is fixed - it's never "we design PCB based on bootloader, so with new
bootloader we will change PCB"?

And if you meant bootstrapping through some hardwired configuration,
then again it is known and defined.

Best regards,
Krzysztof

Re: [PATCH net-next 1/4] dt-bindings: leds: add 'active-high' property

[Daniel Golle]

On Mon, Oct 07, 2024 at 08:38:27AM +0200, Krzysztof Kozlowski wrote:
> On Sun, Oct 06, 2024 at 02:04:35PM +0100, Daniel Golle wrote:
> > On Sun, Oct 06, 2024 at 02:44:44PM +0200, Krzysztof Kozlowski wrote:
> > > I think this should be just string enum, see marvell,marvell10g.yaml
> > 
> > I found the vendor-specific 'marvell,polarity' property in
> > https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231214201442.660447-5-tobias@waldekranz.com/
> > 
> > However, I can't find that file in any Linux tree.
> > 
> > Looking at the suggested patch on patchwork, I got a few questions on
> > how to deal with the situation as of today:
> > 
> > So should the existing support for the 'active-low' and
> > 'inactive-high-impedance' properties be replaced by that string enum?
> > Or should the string property be interpreted in addition to the
> > bools defined in leds/common.yaml?
> > 
> > Should the string property be defined for each PHY or should we move
> > it into a common file?
> > 
> > If so, should that common file also be leds/common.yaml or should we
> > create a new file only for PHY LEDs instead?
> > 
> > Sorry for being confused, I don't mind going down what ever path to have
> > LED polarity configurable properly in DT.
> 
> Let's ignore my idea.
> 
> However I still wonder whether your choice for lack of properties is
> appropriate. Lack of properties as "bootloader default" means it can
> change. Why would anyone prefer to keep bootloader default? The wiring
> is fixed - it's never "we design PCB based on bootloader, so with new
> bootloader we will change PCB"?
> 
> And if you meant bootstrapping through some hardwired configuration,
> then again it is known and defined.

I agree, and my original intention was to just always apply polarity
settings and force people to correctly declare them in DT.
However, that would break DT compatibility on devices not making use
of those properties and relying only on strapping or bootloader
defaults. See also RFC discussed here:

https://patchwork.kernel.org/project/netdevbpf/patch/473d62f268f2a317fd81d0f38f15d2f2f98e2451.1728056697.git.daniel@makrotopia.org/

Re: [PATCH net-next 1/4] dt-bindings: leds: add 'active-high' property

[Daniel Golle]

On Mon, Oct 07, 2024 at 12:30:53PM +0100, Daniel Golle wrote:
> On Mon, Oct 07, 2024 at 08:38:27AM +0200, Krzysztof Kozlowski wrote:
> > On Sun, Oct 06, 2024 at 02:04:35PM +0100, Daniel Golle wrote:
> > > On Sun, Oct 06, 2024 at 02:44:44PM +0200, Krzysztof Kozlowski wrote:
> > > > I think this should be just string enum, see marvell,marvell10g.yaml
> > > 
> > > I found the vendor-specific 'marvell,polarity' property in
> > > https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231214201442.660447-5-tobias@waldekranz.com/
> > > 
> > > However, I can't find that file in any Linux tree.
> > > 
> > > Looking at the suggested patch on patchwork, I got a few questions on
> > > how to deal with the situation as of today:
> > > 
> > > So should the existing support for the 'active-low' and
> > > 'inactive-high-impedance' properties be replaced by that string enum?
> > > Or should the string property be interpreted in addition to the
> > > bools defined in leds/common.yaml?
> > > 
> > > Should the string property be defined for each PHY or should we move
> > > it into a common file?
> > > 
> > > If so, should that common file also be leds/common.yaml or should we
> > > create a new file only for PHY LEDs instead?
> > > 
> > > Sorry for being confused, I don't mind going down what ever path to have
> > > LED polarity configurable properly in DT.
> > 
> > Let's ignore my idea.
> > 
> > However I still wonder whether your choice for lack of properties is
> > appropriate. Lack of properties as "bootloader default" means it can
> > change. Why would anyone prefer to keep bootloader default? The wiring
> > is fixed - it's never "we design PCB based on bootloader, so with new
> > bootloader we will change PCB"?
> > 
> > And if you meant bootstrapping through some hardwired configuration,
> > then again it is known and defined.
> 
> I agree, and my original intention was to just always apply polarity
> settings and force people to correctly declare them in DT.
> However, that would break DT compatibility on devices not making use
> of those properties and relying only on strapping or bootloader
> defaults. See also RFC discussed here:
> 
> https://patchwork.kernel.org/project/netdevbpf/patch/473d62f268f2a317fd81d0f38f15d2f2f98e2451.1728056697.git.daniel@makrotopia.org/
> 

I see that the series was marked as "Not Applicable" in patchwork.
What is the reason for that? To me it looks like it can be applied on
today's net-next cleanly:

[daniel@box linux.git]$ git fetch https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next
From https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next
 * branch                      HEAD       -> FETCH_HEAD
[daniel@box linux.git]$ git checkout FETCH_HEAD
HEAD is now at 6607c17c6c5e net: mana: Enable debugfs files for MANA device
[daniel@box linux.git]$ wget -q -O - https://patchwork.kernel.org/series/895863/mbox/ | git am
Applying: dt-bindings: leds: add 'active-high' property
Applying: net: phy: support 'active-high' property for PHY LEDs
Applying: net: phy: aquantia: correctly describe LED polarity override
Applying: net: phy: mxl-gpy: correctly describe LED polarity
[daniel@box linux.git]$ 

Or did I misunderstand the meaning of "Not Applicable"? If so, please
clarify.

Re: [PATCH net-next 1/4] dt-bindings: leds: add 'active-high' property

[Jakub Kicinski]

On Wed, 9 Oct 2024 14:32:45 +0100 Daniel Golle wrote:
> What is the reason for that? 

No idea.

But we do need an ack from device tree maintainers, I don't see one.

Re: [PATCH net-next 1/4] dt-bindings: leds: add 'active-high' property

[Krzysztof Kozlowski]

On 05/10/2024 18:24, Daniel Golle wrote:
> Other than described in commit c94d1783136 ("dt-bindings: net: phy: Make
> LED active-low property common") the absence of the 'active-low'
> property means not to touch the polarity settings which are inherited
> from reset defaults, the bootloader or bootstrap configuration.
> Hence, in order to override a LED pin being active-high in case of the
> default, bootloader or bootstrap setting being active-low an additional
> property 'active-high' is required.
> Document that property and make it mutually exclusive to the existing
> 'active-low' property.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>  Documentation/devicetree/bindings/leds/common.yaml | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
> index bf9a101e4d42..7c3cd7b7412e 100644
> --- a/Documentation/devicetree/bindings/leds/common.yaml
> +++ b/Documentation/devicetree/bindings/leds/common.yaml
> @@ -202,6 +202,12 @@ properties:
>        #trigger-source-cells property in the source node.
>      $ref: /schemas/types.yaml#/definitions/phandle-array
>  
> +  active-high:
> +    type: boolean
> +    description:
> +      Makes LED active high. To turn the LED ON, line needs to be
> +      set to high voltage instead of low.
> +
>    active-low:
>      type: boolean
>      description:
> @@ -225,6 +231,14 @@ properties:
>        Maximum timeout in microseconds after which the flash LED is turned off.
>        Required for flash LED nodes with configurable timeout.
>  
> +allOf:
> +  - if:
> +      required:
> +        - active-low
> +    then:
> +      properties:
> +        active-high: false

I read prior discussion, so indeed that is safest bet.

With the commit SHA fixed:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof