[PATCH net-next 0/2] net: phy: adin: add support for setting led-, link-status-pin polarity

[PATCH net-next 0/2] net: phy: adin: add support for setting led-, link-status-pin polarity

[Josua Mayer]

ADIN1300 supports software control over pin polarity for both LED_0 and
LINK_ST pins.

Add bindings for specifying the polarity in device-tree, and implement
settings in adin1300 phy driver.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
Josua Mayer (2):
      dt-bindings: net: adin: add pin polarity properties for LED_0, LINK_ST
      net: phy: adin: add support for setting led-, link-status-pin polarity

 .../devicetree/bindings/net/adi,adin.yaml          | 18 ++++++++
 drivers/net/phy/adin.c                             | 53 ++++++++++++++++++++++
 2 files changed, 71 insertions(+)
---
base-commit: 4cece764965020c22cff7665b18a012006359095
change-id: 20240419-adin-pin-polarity-30f7f85f6756

Sincerely,
-- 
Josua Mayer <josua@solid-run.com>

[PATCH net-next 1/2] dt-bindings: net: adin: add pin polarity properties for LED_0, LINK_ST

[Josua Mayer]

ADIN1300 supports software control over pin polarity for both LED_0 and
LINK_ST pins.

Add new properties to set pin polarity:

- adi,led-polarity:
  LED_0 is used as hardware configuration signal during reset.
  Depending on external voltage on the line default value is either
  active-high  (0) or active-low (1).
- adi,link-st-polarity:
  LINK_ST is always active-high (0) after reset.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 Documentation/devicetree/bindings/net/adi,adin.yaml | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml b/Documentation/devicetree/bindings/net/adi,adin.yaml
index 929cf8c0b0fd..ff9262dc69f9 100644
--- a/Documentation/devicetree/bindings/net/adi,adin.yaml
+++ b/Documentation/devicetree/bindings/net/adi,adin.yaml
@@ -52,6 +52,24 @@ properties:
     description: Enable 25MHz reference clock output on CLK25_REF pin.
     type: boolean
 
+  adi,led-polarity:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      LED_0 pin polarity. If unspecified keep phy reset-default derived
+      from hardware configuration pins.
+    enum:
+      - 0 # active high
+      - 1 # active low
+
+  adi,link-st-polarity:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      LINK_ST pin polarity.
+    enum:
+      - 0 # active high
+      - 1 # active low
+    default: 0
+
 unevaluatedProperties: false
 
 examples:

-- 
2.35.3

[PATCH net-next 2/2] net: phy: adin: add support for setting led-, link-status-pin polarity

[Josua Mayer]

ADIN1300 supports software control over pin polarity for both LED_0 and
LINK_ST pins.

Configure the polarity during probe based on device-tree properties.

Led polarity is only set if specified in device-tree, otherwise the phy
can choose either active-low or active-high based on external line
voltage. Link-status polarity is set to active-high as default if not
specified, which is always the reset-default.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 drivers/net/phy/adin.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index 2e1a46e121d9..53159dea6381 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -114,6 +114,9 @@
 
 #define ADIN1300_CDIAG_FLT_DIST(x)		(0xba21 + (x))
 
+#define ADIN1300_LED_A_INV_EN_REG		0xbc01
+#define   ADIN1300_LED_A_INV_EN			BIT(0)
+
 #define ADIN1300_GE_SOFT_RESET_REG		0xff0c
 #define   ADIN1300_GE_SOFT_RESET		BIT(0)
 
@@ -158,6 +161,9 @@
 #define ADIN1300_RMII_20_BITS			0x0004
 #define ADIN1300_RMII_24_BITS			0x0005
 
+#define ADIN1300_GE_LNK_STAT_INV_EN_REG		0xff3c
+#define   ADIN1300_GE_LNK_STAT_INV_EN		BIT(0)
+
 /**
  * struct adin_cfg_reg_map - map a config value to aregister value
  * @cfg:	value in device configuration
@@ -522,6 +528,49 @@ static int adin_config_clk_out(struct phy_device *phydev)
 			      ADIN1300_GE_CLK_CFG_MASK, sel);
 }
 
+static int adin_config_pin_polarity(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	int ret;
+	u32 val;
+
+	/* set led polarity, if property present */
+	if (device_property_present(dev, "adi,led-polarity")) {
+		ret = device_property_read_u32(dev, "adi,led-polarity", &val);
+		if (ret)  {
+			return ret;
+		} else if (val > 1) {
+			phydev_err(phydev, "invalid adi,led-polarity\n");
+			return -EINVAL;
+		}
+
+		ret = phy_modify_mmd(phydev, MDIO_MMD_VEND1,
+				     ADIN1300_LED_A_INV_EN_REG,
+				     ADIN1300_LED_A_INV_EN, val);
+		if (ret)
+			return ret;
+	}
+
+	/* set link-status polarity, default to active-high (0) */
+	if (device_property_present(dev, "adi,link-st-polarity")) {
+		ret = device_property_read_u32(dev, "adi,link-st-polarity", &val);
+		if (ret) {
+			return ret;
+		} else if (val > 1) {
+			phydev_err(phydev, "invalid adi,link-st-polarity\n");
+			return -EINVAL;
+		}
+	} else {
+		val = 0;
+	}
+
+	ret = phy_modify_mmd(phydev, MDIO_MMD_VEND1,
+			     ADIN1300_GE_LNK_STAT_INV_EN_REG,
+			     ADIN1300_GE_LNK_STAT_INV_EN, val);
+
+	return ret;
+}
+
 static int adin_config_init(struct phy_device *phydev)
 {
 	int rc;
@@ -548,6 +597,10 @@ static int adin_config_init(struct phy_device *phydev)
 	if (rc < 0)
 		return rc;
 
+	rc = adin_config_pin_polarity(phydev);
+	if (rc < 0)
+		return rc;
+
 	phydev_dbg(phydev, "PHY is using mode '%s'\n",
 		   phy_modes(phydev->interface));
 

-- 
2.35.3

Re: [PATCH net-next 2/2] net: phy: adin: add support for setting led-, link-status-pin polarity

[Andrew Lunn]

On Fri, Apr 19, 2024 at 05:35:18PM +0200, Josua Mayer wrote:
> ADIN1300 supports software control over pin polarity for both LED_0 and
> LINK_ST pins.
> 
> Configure the polarity during probe based on device-tree properties.
> 
> Led polarity is only set if specified in device-tree, otherwise the phy
> can choose either active-low or active-high based on external line
> voltage. Link-status polarity is set to active-high as default if not
> specified, which is always the reset-default.
> 
> Signed-off-by: Josua Mayer <josua@solid-run.com>

Hi Josua

Please take a look at:

commit 447b80a9330ef2d9a94fc5a9bf35b6eac061f38b
Author: Alexander Stein <alexander.stein@ew.tq-group.com>
Date:   Wed Jan 31 08:50:48 2024 +0100

    net: phy: dp83867: Add support for active-low LEDs
    
    Add the led_polarity_set callback for setting LED polarity.
    
    Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
    Reviewed-by: Andrew Lunn <andrew@lunn.ch>
    Signed-off-by: David S. Miller <davem@davemloft.net>


    Andrew

---
pw-bot: cr

Re: [PATCH net-next 2/2] net: phy: adin: add support for setting led-, link-status-pin polarity

[Josua Mayer]

Am 19.04.24 um 17:47 schrieb Andrew Lunn:
> On Fri, Apr 19, 2024 at 05:35:18PM +0200, Josua Mayer wrote:
>> ADIN1300 supports software control over pin polarity for both LED_0 and
>> LINK_ST pins.
>>
>> Configure the polarity during probe based on device-tree properties.
>>
>> Led polarity is only set if specified in device-tree, otherwise the phy
>> can choose either active-low or active-high based on external line
>> voltage. Link-status polarity is set to active-high as default if not
>> specified, which is always the reset-default.
>>
>> Signed-off-by: Josua Mayer <josua@solid-run.com>
> Hi Josua
>
> Please take a look at:
>
> commit 447b80a9330ef2d9a94fc5a9bf35b6eac061f38b
> Author: Alexander Stein <alexander.stein@ew.tq-group.com>
> Date:   Wed Jan 31 08:50:48 2024 +0100
>
>     net: phy: dp83867: Add support for active-low LEDs
>     
>     Add the led_polarity_set callback for setting LED polarity.
>     
>     Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
>     Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>     Signed-off-by: David S. Miller <davem@davemloft.net>
>
>
>     Andrew
>
> ---

Hi Andrew,

That looks very much related!

I was already planning to investigate adding led support ... .

1. for the  LINK_ST pin I believe we still need a non-led-framework
device property for setting polarity, as it is a fixed function signal
that we can't even turn on or off from software.

2. LED_0 control not currently supported by adin driver.
The phy supports what data-sheet calls extended configuration
(disabled by default) for controlling led state (on, off, patterns).

Since it is not default, I see the polarity setting separate from leds.
However I do believe the led_polarity_set callback is an acceptable
solution.

I might prepare a reduced v2 for only the fixed-function link-status pin.

sincerely
Josua Mayer

Re: [PATCH net-next 2/2] net: phy: adin: add support for setting led-, link-status-pin polarity

[Andrew Lunn]

> Hi Andrew,
> 
> That looks very much related!
> 
> I was already planning to investigate adding led support ... .
> 
> 1. for the  LINK_ST pin I believe we still need a non-led-framework
> device property for setting polarity, as it is a fixed function signal
> that we can't even turn on or off from software.

We should separate the device tree binding from the implementation of
the binding. The binding describes the hardware. The hardware is
active low. And the binding can describe that. So i don't see a need
for anything vendor specific.

I think the real question is, can the current generic LED code be made
to handle this LED, or should you have code in the PHY driver to
handle this binding in a specific way for this LED?

Given the restrictions on this LED, i don't think it makes sense to
expose it in /sys/class/leds. But is it possible to leverage the
framework to parse the binding and call the polarity function?

> 2. LED_0 control not currently supported by adin driver.
> The phy supports what data-sheet calls extended configuration
> (disabled by default) for controlling led state (on, off, patterns).
> 
> Since it is not default, I see the polarity setting separate from leds.
> However I do believe the led_polarity_set callback is an acceptable
> solution.

Again, you should use the LED binding, even if you don't use the LED
framework. I just wounder how much code you will duplicate if you do
decide to implement the binding in the driver. And when you fully
implement the control of the LED using the framework, are you going to
throw the code away again?

       Andrew