2024-04-19 15:35:45

by Josua Mayer

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

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 <[email protected]>
---
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 <[email protected]>



2024-04-19 15:36:02

by Josua Mayer

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

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 <[email protected]>
---
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


2024-04-19 15:37:45

by Josua Mayer

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

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 <[email protected]>
---
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


2024-04-19 15:48:05

by Andrew Lunn

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

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 <[email protected]>

Hi Josua

Please take a look at:

commit 447b80a9330ef2d9a94fc5a9bf35b6eac061f38b
Author: Alexander Stein <[email protected]>
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 <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Signed-off-by: David S. Miller <[email protected]>


Andrew

---
pw-bot: cr

2024-04-20 09:14:02

by Josua Mayer

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

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 <[email protected]>
> Hi Josua
>
> Please take a look at:
>
> commit 447b80a9330ef2d9a94fc5a9bf35b6eac061f38b
> Author: Alexander Stein <[email protected]>
> 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 <[email protected]>
> Reviewed-by: Andrew Lunn <[email protected]>
> Signed-off-by: David S. Miller <[email protected]>
>
>
> 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

2024-04-20 16:12:44

by Andrew Lunn

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

> 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