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

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

[Daniel Golle]

Other than described in commit c94d1783136e ("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>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
v2: fix commit sha truncation in commit message

 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.47.0

[PATCH net-next v2 2/5] 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.
As only either 'active-high' or 'active-low' can be set at the
same time, WARN and return an error in case both are set.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: emmit warning and return error if both, 'active-high' and
    'active-low' are set

 drivers/net/phy/phy_device.c | 6 ++++++
 include/linux/phy.h          | 5 +++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 4ccf504a8b2c..e92fac9aad92 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -3357,11 +3357,17 @@ 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"))
 		set_bit(PHY_LED_INACTIVE_HIGH_IMPEDANCE, &modes);
 
+	if (WARN_ON(modes & BIT(PHY_LED_ACTIVE_LOW) &&
+		    modes & BIT(PHY_LED_ACTIVE_HIGH)))
+		return -EINVAL;
+
 	if (modes) {
 		/* Return error if asked to set polarity modes but not supported */
 		if (!phydev->drv->led_polarity_set)
diff --git a/include/linux/phy.h b/include/linux/phy.h
index ff762a3d8270..bf0eb4e5d35c 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.47.0

[PATCH net-next v2 3/5] 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>
---
v2: use dedicated bools force_active_high and force_active_low to make
    aqr_phy_led_polarity_set() more robust

 drivers/net/phy/aquantia/aquantia.h      |  1 +
 drivers/net/phy/aquantia/aquantia_leds.c | 19 ++++++++++++++-----
 drivers/net/phy/aquantia/aquantia_main.c | 12 +++++++++---
 3 files changed, 24 insertions(+), 8 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..2bafd6c78b00 100644
--- a/drivers/net/phy/aquantia/aquantia_leds.c
+++ b/drivers/net/phy/aquantia/aquantia_leds.c
@@ -121,13 +121,13 @@ 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)
 {
+	bool force_active_low = false, force_active_high = false;
 	struct aqr107_priv *priv = phydev->priv;
-	bool active_low = false;
 	u32 mode;
 
 	if (index >= AQR_MAX_LEDS)
@@ -136,7 +136,10 @@ int aqr_phy_led_polarity_set(struct phy_device *phydev, int index, unsigned long
 	for_each_set_bit(mode, &modes, __PHY_LED_MODES_NUM) {
 		switch (mode) {
 		case PHY_LED_ACTIVE_LOW:
-			active_low = true;
+			force_active_low = true;
+			break;
+		case PHY_LED_ACTIVE_HIGH:
+			force_active_high = true;
 			break;
 		default:
 			return -EINVAL;
@@ -144,8 +147,14 @@ 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)
+	if (force_active_low)
 		priv->leds_active_low |= BIT(index);
 
-	return aqr_phy_led_active_low_set(phydev, index, active_low);
+	if (force_active_high)
+		priv->leds_active_high |= BIT(index);
+
+	if (force_active_high || force_active_low)
+		return aqr_phy_led_active_low_set(phydev, index, force_active_low);
+
+	unreachable();
 }
diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
index dcad3fa1ddc3..c62fc65ddbdb 100644
--- a/drivers/net/phy/aquantia/aquantia_main.c
+++ b/drivers/net/phy/aquantia/aquantia_main.c
@@ -517,7 +517,7 @@ static int aqr107_config_mdi(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 */
@@ -548,8 +548,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.47.0

[PATCH net-next v2 4/5] 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>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
v2: use dedicated bools force_active_high and force_active_low to make
    gpy_led_polarity_set() more robust

 drivers/net/phy/mxl-gpy.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
index bc4abb957e15..00676e272913 100644
--- a/drivers/net/phy/mxl-gpy.c
+++ b/drivers/net/phy/mxl-gpy.c
@@ -989,7 +989,7 @@ static int gpy_led_hw_control_set(struct phy_device *phydev, u8 index,
 static int gpy_led_polarity_set(struct phy_device *phydev, int index,
 				unsigned long modes)
 {
-	bool active_low = false;
+	bool force_active_low = false, force_active_high = false;
 	u32 mode;
 
 	if (index >= GPY_MAX_LEDS)
@@ -998,15 +998,23 @@ static int gpy_led_polarity_set(struct phy_device *phydev, int index,
 	for_each_set_bit(mode, &modes, __PHY_LED_MODES_NUM) {
 		switch (mode) {
 		case PHY_LED_ACTIVE_LOW:
-			active_low = true;
+			force_active_low = true;
+			break;
+		case PHY_LED_ACTIVE_HIGH:
+			force_active_high = true;
 			break;
 		default:
 			return -EINVAL;
 		}
 	}
 
-	return phy_modify(phydev, PHY_LED, PHY_LED_POLARITY(index),
-			  active_low ? 0 : PHY_LED_POLARITY(index));
+	if (force_active_low)
+		return phy_set_bits(phydev, PHY_LED, PHY_LED_POLARITY(index));
+
+	if (force_active_high)
+		return phy_clear_bits(phydev, PHY_LED, PHY_LED_POLARITY(index));
+
+	unreachable();
 }
 
 static struct phy_driver gpy_drivers[] = {
-- 
2.47.0

[PATCH net-next v2 5/5] net: phy: intel-xway: add support for PHY LEDs

[Daniel Golle]

The intel-xway PHY driver predates the PHY LED framework and currently
initializes all LED pins to equal default values.

Add PHY LED functions to the drivers and don't set default values if
LEDs are defined in device tree.

According the datasheets 3 LEDs are supported on all Intel XWAY PHYs.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: use dedicated bools force_active_high and force_active_low to make
    xway_gphy_led_polarity_set() more robust

 drivers/net/phy/intel-xway.c | 253 +++++++++++++++++++++++++++++++++--
 1 file changed, 244 insertions(+), 9 deletions(-)

diff --git a/drivers/net/phy/intel-xway.c b/drivers/net/phy/intel-xway.c
index 3c032868ef04..0e5454b203c4 100644
--- a/drivers/net/phy/intel-xway.c
+++ b/drivers/net/phy/intel-xway.c
@@ -151,6 +151,13 @@
 #define XWAY_MMD_LED3H			0x01E8
 #define XWAY_MMD_LED3L			0x01E9
 
+#define XWAY_GPHY_MAX_LEDS		3
+#define XWAY_GPHY_LED_INV(idx)		BIT(12 + (idx))
+#define XWAY_GPHY_LED_EN(idx)		BIT(8 + (idx))
+#define XWAY_GPHY_LED_DA(idx)		BIT(idx)
+#define XWAY_MMD_LEDxH(idx)		(XWAY_MMD_LED0H + 2 * (idx))
+#define XWAY_MMD_LEDxL(idx)		(XWAY_MMD_LED0L + 2 * (idx))
+
 #define PHY_ID_PHY11G_1_3		0x030260D1
 #define PHY_ID_PHY22F_1_3		0x030260E1
 #define PHY_ID_PHY11G_1_4		0xD565A400
@@ -229,20 +236,12 @@ static int xway_gphy_rgmii_init(struct phy_device *phydev)
 			  XWAY_MDIO_MIICTRL_TXSKEW_MASK, val);
 }
 
-static int xway_gphy_config_init(struct phy_device *phydev)
+static int xway_gphy_init_leds(struct phy_device *phydev)
 {
 	int err;
 	u32 ledxh;
 	u32 ledxl;
 
-	/* Mask all interrupts */
-	err = phy_write(phydev, XWAY_MDIO_IMASK, 0);
-	if (err)
-		return err;
-
-	/* Clear all pending interrupts */
-	phy_read(phydev, XWAY_MDIO_ISTAT);
-
 	/* Ensure that integrated led function is enabled for all leds */
 	err = phy_write(phydev, XWAY_MDIO_LED,
 			XWAY_MDIO_LED_LED0_EN |
@@ -276,6 +275,26 @@ static int xway_gphy_config_init(struct phy_device *phydev)
 	phy_write_mmd(phydev, MDIO_MMD_VEND2, XWAY_MMD_LED2H, ledxh);
 	phy_write_mmd(phydev, MDIO_MMD_VEND2, XWAY_MMD_LED2L, ledxl);
 
+	return 0;
+}
+
+static int xway_gphy_config_init(struct phy_device *phydev)
+{
+	struct device_node *np = phydev->mdio.dev.of_node;
+	int err;
+
+	/* Mask all interrupts */
+	err = phy_write(phydev, XWAY_MDIO_IMASK, 0);
+	if (err)
+		return err;
+
+	/* Use default LED configuration if 'leds' node isn't defined */
+	if (!of_get_child_by_name(np, "leds"))
+		xway_gphy_init_leds(phydev);
+
+	/* Clear all pending interrupts */
+	phy_read(phydev, XWAY_MDIO_ISTAT);
+
 	err = xway_gphy_rgmii_init(phydev);
 	if (err)
 		return err;
@@ -347,6 +366,172 @@ static irqreturn_t xway_gphy_handle_interrupt(struct phy_device *phydev)
 	return IRQ_HANDLED;
 }
 
+static int xway_gphy_led_brightness_set(struct phy_device *phydev,
+					u8 index, enum led_brightness value)
+{
+	int ret;
+
+	if (index >= XWAY_GPHY_MAX_LEDS)
+		return -EINVAL;
+
+	/* clear EN and set manual LED state */
+	ret = phy_modify(phydev, XWAY_MDIO_LED,
+			 ((value == LED_OFF) ? XWAY_GPHY_LED_EN(index) : 0) |
+			 XWAY_GPHY_LED_DA(index),
+			 (value == LED_OFF) ? 0 : XWAY_GPHY_LED_DA(index));
+	if (ret)
+		return ret;
+
+	/* clear HW LED setup */
+	if (value == LED_OFF) {
+		ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, XWAY_MMD_LEDxH(index), 0);
+		if (ret)
+			return ret;
+
+		return phy_write_mmd(phydev, MDIO_MMD_VEND2, XWAY_MMD_LEDxL(index), 0);
+	} else {
+		return 0;
+	}
+}
+
+static const unsigned long supported_triggers = (BIT(TRIGGER_NETDEV_LINK) |
+						 BIT(TRIGGER_NETDEV_LINK_10) |
+						 BIT(TRIGGER_NETDEV_LINK_100) |
+						 BIT(TRIGGER_NETDEV_LINK_1000) |
+						 BIT(TRIGGER_NETDEV_RX) |
+						 BIT(TRIGGER_NETDEV_TX));
+
+static int xway_gphy_led_hw_is_supported(struct phy_device *phydev, u8 index,
+					 unsigned long rules)
+{
+	if (index >= XWAY_GPHY_MAX_LEDS)
+		return -EINVAL;
+
+	/* activity triggers are not possible without combination with a link
+	 * trigger.
+	 */
+	if (rules & (BIT(TRIGGER_NETDEV_RX) | BIT(TRIGGER_NETDEV_TX)) &&
+	    !(rules & (BIT(TRIGGER_NETDEV_LINK) |
+		       BIT(TRIGGER_NETDEV_LINK_10) |
+		       BIT(TRIGGER_NETDEV_LINK_100) |
+		       BIT(TRIGGER_NETDEV_LINK_1000))))
+		return -EOPNOTSUPP;
+
+	/* All other combinations of the supported triggers are allowed */
+	if (rules & ~supported_triggers)
+		return -EOPNOTSUPP;
+
+	return 0;
+}
+
+static int xway_gphy_led_hw_control_get(struct phy_device *phydev, u8 index,
+					unsigned long *rules)
+{
+	int lval, hval;
+
+	if (index >= XWAY_GPHY_MAX_LEDS)
+		return -EINVAL;
+
+	hval = phy_read_mmd(phydev, MDIO_MMD_VEND2, XWAY_MMD_LEDxH(index));
+	if (hval < 0)
+		return hval;
+
+	lval = phy_read_mmd(phydev, MDIO_MMD_VEND2, XWAY_MMD_LEDxL(index));
+	if (lval < 0)
+		return lval;
+
+	if (hval & XWAY_MMD_LEDxH_CON_LINK10)
+		*rules |= BIT(TRIGGER_NETDEV_LINK_10);
+
+	if (hval & XWAY_MMD_LEDxH_CON_LINK100)
+		*rules |= BIT(TRIGGER_NETDEV_LINK_100);
+
+	if (hval & XWAY_MMD_LEDxH_CON_LINK1000)
+		*rules |= BIT(TRIGGER_NETDEV_LINK_1000);
+
+	if ((hval & XWAY_MMD_LEDxH_CON_LINK10) &&
+	    (hval & XWAY_MMD_LEDxH_CON_LINK100) &&
+	    (hval & XWAY_MMD_LEDxH_CON_LINK1000))
+		*rules |= BIT(TRIGGER_NETDEV_LINK);
+
+	if (lval & XWAY_MMD_LEDxL_PULSE_TXACT)
+		*rules |= BIT(TRIGGER_NETDEV_TX);
+
+	if (lval & XWAY_MMD_LEDxL_PULSE_RXACT)
+		*rules |= BIT(TRIGGER_NETDEV_RX);
+
+	return 0;
+}
+
+static int xway_gphy_led_hw_control_set(struct phy_device *phydev, u8 index,
+					unsigned long rules)
+{
+	u16 hval = 0, lval = 0;
+	int ret;
+
+	if (index >= XWAY_GPHY_MAX_LEDS)
+		return -EINVAL;
+
+	if (rules & BIT(TRIGGER_NETDEV_LINK) ||
+	    rules & BIT(TRIGGER_NETDEV_LINK_10))
+		hval |= XWAY_MMD_LEDxH_CON_LINK10;
+
+	if (rules & BIT(TRIGGER_NETDEV_LINK) ||
+	    rules & BIT(TRIGGER_NETDEV_LINK_100))
+		hval |= XWAY_MMD_LEDxH_CON_LINK100;
+
+	if (rules & BIT(TRIGGER_NETDEV_LINK) ||
+	    rules & BIT(TRIGGER_NETDEV_LINK_1000))
+		hval |= XWAY_MMD_LEDxH_CON_LINK1000;
+
+	if (rules & BIT(TRIGGER_NETDEV_TX))
+		lval |= XWAY_MMD_LEDxL_PULSE_TXACT;
+
+	if (rules & BIT(TRIGGER_NETDEV_RX))
+		lval |= XWAY_MMD_LEDxL_PULSE_RXACT;
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, XWAY_MMD_LEDxH(index), hval);
+	if (ret)
+		return ret;
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, XWAY_MMD_LEDxL(index), lval);
+	if (ret)
+		return ret;
+
+	return phy_set_bits(phydev, XWAY_MDIO_LED, XWAY_GPHY_LED_EN(index));
+}
+
+static int xway_gphy_led_polarity_set(struct phy_device *phydev, int index,
+				      unsigned long modes)
+{
+	bool force_active_low = false, force_active_high = false;
+	u32 mode;
+
+	if (index >= XWAY_GPHY_MAX_LEDS)
+		return -EINVAL;
+
+	for_each_set_bit(mode, &modes, __PHY_LED_MODES_NUM) {
+		switch (mode) {
+		case PHY_LED_ACTIVE_LOW:
+			force_active_low = true;
+			break;
+		case PHY_LED_ACTIVE_HIGH:
+			force_active_high = true;
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	if (force_active_low)
+		return phy_set_bits(phydev, XWAY_MDIO_LED, XWAY_GPHY_LED_INV(index));
+
+	if (force_active_high)
+		return phy_clear_bits(phydev, XWAY_MDIO_LED, XWAY_GPHY_LED_INV(index));
+
+	unreachable();
+}
+
 static struct phy_driver xway_gphy[] = {
 	{
 		.phy_id		= PHY_ID_PHY11G_1_3,
@@ -359,6 +544,11 @@ static struct phy_driver xway_gphy[] = {
 		.config_intr	= xway_gphy_config_intr,
 		.suspend	= genphy_suspend,
 		.resume		= genphy_resume,
+		.led_brightness_set = xway_gphy_led_brightness_set,
+		.led_hw_is_supported = xway_gphy_led_hw_is_supported,
+		.led_hw_control_get = xway_gphy_led_hw_control_get,
+		.led_hw_control_set = xway_gphy_led_hw_control_set,
+		.led_polarity_set = xway_gphy_led_polarity_set,
 	}, {
 		.phy_id		= PHY_ID_PHY22F_1_3,
 		.phy_id_mask	= 0xffffffff,
@@ -370,6 +560,11 @@ static struct phy_driver xway_gphy[] = {
 		.config_intr	= xway_gphy_config_intr,
 		.suspend	= genphy_suspend,
 		.resume		= genphy_resume,
+		.led_brightness_set = xway_gphy_led_brightness_set,
+		.led_hw_is_supported = xway_gphy_led_hw_is_supported,
+		.led_hw_control_get = xway_gphy_led_hw_control_get,
+		.led_hw_control_set = xway_gphy_led_hw_control_set,
+		.led_polarity_set = xway_gphy_led_polarity_set,
 	}, {
 		.phy_id		= PHY_ID_PHY11G_1_4,
 		.phy_id_mask	= 0xffffffff,
@@ -381,6 +576,11 @@ static struct phy_driver xway_gphy[] = {
 		.config_intr	= xway_gphy_config_intr,
 		.suspend	= genphy_suspend,
 		.resume		= genphy_resume,
+		.led_brightness_set = xway_gphy_led_brightness_set,
+		.led_hw_is_supported = xway_gphy_led_hw_is_supported,
+		.led_hw_control_get = xway_gphy_led_hw_control_get,
+		.led_hw_control_set = xway_gphy_led_hw_control_set,
+		.led_polarity_set = xway_gphy_led_polarity_set,
 	}, {
 		.phy_id		= PHY_ID_PHY22F_1_4,
 		.phy_id_mask	= 0xffffffff,
@@ -392,6 +592,11 @@ static struct phy_driver xway_gphy[] = {
 		.config_intr	= xway_gphy_config_intr,
 		.suspend	= genphy_suspend,
 		.resume		= genphy_resume,
+		.led_brightness_set = xway_gphy_led_brightness_set,
+		.led_hw_is_supported = xway_gphy_led_hw_is_supported,
+		.led_hw_control_get = xway_gphy_led_hw_control_get,
+		.led_hw_control_set = xway_gphy_led_hw_control_set,
+		.led_polarity_set = xway_gphy_led_polarity_set,
 	}, {
 		.phy_id		= PHY_ID_PHY11G_1_5,
 		.phy_id_mask	= 0xffffffff,
@@ -402,6 +607,11 @@ static struct phy_driver xway_gphy[] = {
 		.config_intr	= xway_gphy_config_intr,
 		.suspend	= genphy_suspend,
 		.resume		= genphy_resume,
+		.led_brightness_set = xway_gphy_led_brightness_set,
+		.led_hw_is_supported = xway_gphy_led_hw_is_supported,
+		.led_hw_control_get = xway_gphy_led_hw_control_get,
+		.led_hw_control_set = xway_gphy_led_hw_control_set,
+		.led_polarity_set = xway_gphy_led_polarity_set,
 	}, {
 		.phy_id		= PHY_ID_PHY22F_1_5,
 		.phy_id_mask	= 0xffffffff,
@@ -412,6 +622,11 @@ static struct phy_driver xway_gphy[] = {
 		.config_intr	= xway_gphy_config_intr,
 		.suspend	= genphy_suspend,
 		.resume		= genphy_resume,
+		.led_brightness_set = xway_gphy_led_brightness_set,
+		.led_hw_is_supported = xway_gphy_led_hw_is_supported,
+		.led_hw_control_get = xway_gphy_led_hw_control_get,
+		.led_hw_control_set = xway_gphy_led_hw_control_set,
+		.led_polarity_set = xway_gphy_led_polarity_set,
 	}, {
 		.phy_id		= PHY_ID_PHY11G_VR9_1_1,
 		.phy_id_mask	= 0xffffffff,
@@ -422,6 +637,11 @@ static struct phy_driver xway_gphy[] = {
 		.config_intr	= xway_gphy_config_intr,
 		.suspend	= genphy_suspend,
 		.resume		= genphy_resume,
+		.led_brightness_set = xway_gphy_led_brightness_set,
+		.led_hw_is_supported = xway_gphy_led_hw_is_supported,
+		.led_hw_control_get = xway_gphy_led_hw_control_get,
+		.led_hw_control_set = xway_gphy_led_hw_control_set,
+		.led_polarity_set = xway_gphy_led_polarity_set,
 	}, {
 		.phy_id		= PHY_ID_PHY22F_VR9_1_1,
 		.phy_id_mask	= 0xffffffff,
@@ -432,6 +652,11 @@ static struct phy_driver xway_gphy[] = {
 		.config_intr	= xway_gphy_config_intr,
 		.suspend	= genphy_suspend,
 		.resume		= genphy_resume,
+		.led_brightness_set = xway_gphy_led_brightness_set,
+		.led_hw_is_supported = xway_gphy_led_hw_is_supported,
+		.led_hw_control_get = xway_gphy_led_hw_control_get,
+		.led_hw_control_set = xway_gphy_led_hw_control_set,
+		.led_polarity_set = xway_gphy_led_polarity_set,
 	}, {
 		.phy_id		= PHY_ID_PHY11G_VR9_1_2,
 		.phy_id_mask	= 0xffffffff,
@@ -442,6 +667,11 @@ static struct phy_driver xway_gphy[] = {
 		.config_intr	= xway_gphy_config_intr,
 		.suspend	= genphy_suspend,
 		.resume		= genphy_resume,
+		.led_brightness_set = xway_gphy_led_brightness_set,
+		.led_hw_is_supported = xway_gphy_led_hw_is_supported,
+		.led_hw_control_get = xway_gphy_led_hw_control_get,
+		.led_hw_control_set = xway_gphy_led_hw_control_set,
+		.led_polarity_set = xway_gphy_led_polarity_set,
 	}, {
 		.phy_id		= PHY_ID_PHY22F_VR9_1_2,
 		.phy_id_mask	= 0xffffffff,
@@ -452,6 +682,11 @@ static struct phy_driver xway_gphy[] = {
 		.config_intr	= xway_gphy_config_intr,
 		.suspend	= genphy_suspend,
 		.resume		= genphy_resume,
+		.led_brightness_set = xway_gphy_led_brightness_set,
+		.led_hw_is_supported = xway_gphy_led_hw_is_supported,
+		.led_hw_control_get = xway_gphy_led_hw_control_get,
+		.led_hw_control_set = xway_gphy_led_hw_control_set,
+		.led_polarity_set = xway_gphy_led_polarity_set,
 	},
 };
 module_phy_driver(xway_gphy);
-- 
2.47.0

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

[Andrew Lunn]

On Thu, Oct 10, 2024 at 01:54:19PM +0100, Daniel Golle wrote:
> In addition to 'active-low' and 'inactive-high-impedance' also
> support 'active-high' property for PHY LED pin configuration.
> As only either 'active-high' or 'active-low' can be set at the
> same time, WARN and return an error in case both are set.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

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

[Andrew Lunn]

On Thu, Oct 10, 2024 at 01:55:00PM +0100, Daniel Golle wrote:
> 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>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next v2 5/5] net: phy: intel-xway: add support for PHY LEDs

[Andrew Lunn]

On Thu, Oct 10, 2024 at 01:55:29PM +0100, Daniel Golle wrote:
> The intel-xway PHY driver predates the PHY LED framework and currently
> initializes all LED pins to equal default values.
> 
> Add PHY LED functions to the drivers and don't set default values if
> LEDs are defined in device tree.
> 
> According the datasheets 3 LEDs are supported on all Intel XWAY PHYs.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

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

[Jakub Kicinski]

On Thu, 10 Oct 2024 13:53:36 +0100 Daniel Golle wrote:
> Other than described in commit c94d1783136e ("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.

Daniel, please make sure you provide a cover letter for any submissions
longer than 2 patches. If nothing else it gives the maintainers a quick
overview of which files you're touching.

This submission is okay but please correct going forward.

Let's wait a bit longer for Lee to ack / take the first patch and then
I presume we take the rest.

Re: (subset) [PATCH net-next v2 1/5] dt-bindings: leds: add 'active-high' property

[Lee Jones]

On Thu, 10 Oct 2024 13:53:36 +0100, Daniel Golle wrote:
> Other than described in commit c94d1783136e ("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.
> 
> [...]

Applied, thanks!

[1/5] dt-bindings: leds: add 'active-high' property
      commit: fcaade450ea25e0162ee4a28ac0c7b911fa25674

--
Lee Jones [李琼斯]

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

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Thu, 10 Oct 2024 13:53:36 +0100 you wrote:
> Other than described in commit c94d1783136e ("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.
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/5] dt-bindings: leds: add 'active-high' property
    (no matching commit)
  - [net-next,v2,2/5] net: phy: support 'active-high' property for PHY LEDs
    https://git.kernel.org/netdev/net-next/c/a274465cc3be
  - [net-next,v2,3/5] net: phy: aquantia: correctly describe LED polarity override
    https://git.kernel.org/netdev/net-next/c/9d55e68b19f2
  - [net-next,v2,4/5] net: phy: mxl-gpy: correctly describe LED polarity
    https://git.kernel.org/netdev/net-next/c/eb89c79c1b8f
  - [net-next,v2,5/5] net: phy: intel-xway: add support for PHY LEDs
    https://git.kernel.org/netdev/net-next/c/1758af47b98c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html