* [PATCH net-next 0/2] net: phy: adin: add support for setting led-, link-status-pin polarity
@ 2024-04-19 15:35 Josua Mayer
2024-04-19 15:35 ` [PATCH net-next 1/2] dt-bindings: net: adin: add pin polarity properties for LED_0, LINK_ST Josua Mayer
2024-04-19 15:35 ` [PATCH net-next 2/2] net: phy: adin: add support for setting led-, link-status-pin polarity Josua Mayer
0 siblings, 2 replies; 6+ messages in thread
From: Josua Mayer @ 2024-04-19 15:35 UTC (permalink / raw
To: Michael Hennerich, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandru Tachici, Andrew Lunn, Heiner Kallweit, Russell King
Cc: Jon Nettleton, Yazan Shhady, netdev, devicetree, linux-kernel,
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next 1/2] dt-bindings: net: adin: add pin polarity properties for LED_0, LINK_ST
2024-04-19 15:35 [PATCH net-next 0/2] net: phy: adin: add support for setting led-, link-status-pin polarity Josua Mayer
@ 2024-04-19 15:35 ` Josua Mayer
2024-04-19 15:35 ` [PATCH net-next 2/2] net: phy: adin: add support for setting led-, link-status-pin polarity Josua Mayer
1 sibling, 0 replies; 6+ messages in thread
From: Josua Mayer @ 2024-04-19 15:35 UTC (permalink / raw
To: Michael Hennerich, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandru Tachici, Andrew Lunn, Heiner Kallweit, Russell King
Cc: Jon Nettleton, Yazan Shhady, netdev, devicetree, linux-kernel,
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net-next 2/2] net: phy: adin: add support for setting led-, link-status-pin polarity
2024-04-19 15:35 [PATCH net-next 0/2] net: phy: adin: add support for setting led-, link-status-pin polarity Josua Mayer
2024-04-19 15:35 ` [PATCH net-next 1/2] dt-bindings: net: adin: add pin polarity properties for LED_0, LINK_ST Josua Mayer
@ 2024-04-19 15:35 ` Josua Mayer
2024-04-19 15:47 ` Andrew Lunn
1 sibling, 1 reply; 6+ messages in thread
From: Josua Mayer @ 2024-04-19 15:35 UTC (permalink / raw
To: Michael Hennerich, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandru Tachici, Andrew Lunn, Heiner Kallweit, Russell King
Cc: Jon Nettleton, Yazan Shhady, netdev, devicetree, linux-kernel,
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 2/2] net: phy: adin: add support for setting led-, link-status-pin polarity
2024-04-19 15:35 ` [PATCH net-next 2/2] net: phy: adin: add support for setting led-, link-status-pin polarity Josua Mayer
@ 2024-04-19 15:47 ` Andrew Lunn
2024-04-20 9:10 ` Josua Mayer
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Lunn @ 2024-04-19 15:47 UTC (permalink / raw
To: Josua Mayer
Cc: Michael Hennerich, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandru Tachici, Heiner Kallweit, Russell King, Jon Nettleton,
Yazan Shhady, netdev, devicetree, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 2/2] net: phy: adin: add support for setting led-, link-status-pin polarity
2024-04-19 15:47 ` Andrew Lunn
@ 2024-04-20 9:10 ` Josua Mayer
2024-04-20 16:04 ` Andrew Lunn
0 siblings, 1 reply; 6+ messages in thread
From: Josua Mayer @ 2024-04-20 9:10 UTC (permalink / raw
To: Andrew Lunn
Cc: Michael Hennerich, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandru Tachici, Heiner Kallweit, Russell King, Jon Nettleton,
Yazan Shhady, netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 2/2] net: phy: adin: add support for setting led-, link-status-pin polarity
2024-04-20 9:10 ` Josua Mayer
@ 2024-04-20 16:04 ` Andrew Lunn
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Lunn @ 2024-04-20 16:04 UTC (permalink / raw
To: Josua Mayer
Cc: Michael Hennerich, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandru Tachici, Heiner Kallweit, Russell King, Jon Nettleton,
Yazan Shhady, netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-04-20 16:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-19 15:35 [PATCH net-next 0/2] net: phy: adin: add support for setting led-, link-status-pin polarity Josua Mayer
2024-04-19 15:35 ` [PATCH net-next 1/2] dt-bindings: net: adin: add pin polarity properties for LED_0, LINK_ST Josua Mayer
2024-04-19 15:35 ` [PATCH net-next 2/2] net: phy: adin: add support for setting led-, link-status-pin polarity Josua Mayer
2024-04-19 15:47 ` Andrew Lunn
2024-04-20 9:10 ` Josua Mayer
2024-04-20 16:04 ` Andrew Lunn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).