[PATCH net-next v2] net: phy: marvell: add support for PHY LEDs polarity modes

[PATCH net-next v2] net: phy: marvell: add support for PHY LEDs polarity modes

[Aleksander Jan Bajkowski]

Add the led_polarity_set callback for setting LED polarity. Implement
this callback for the 88E1318 and 88E1510 PHYs. This should also work on
other Marvell PHYs, but I don't have access to the TRM or the hardware.

Tested on Adapteva Paralella board with Marvell 88E1318 PHY.

Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
---
Changes in v2:
 - add suppoer for PHY_LED_INACTIVE_HIGH_IMPEDANCE mode
---
 drivers/net/phy/marvell.c | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index f71cffa88406..d57fcabfec2b 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -149,6 +149,9 @@
 #define MII_88E1318S_PHY_LED_FUNC_ON		(0x9)
 #define MII_88E1318S_PHY_LED_FUNC_HI_Z		(0xa)
 #define MII_88E1318S_PHY_LED_FUNC_BLINK		(0xb)
+#define MII_88E1318S_PHY_LED_POL		0x11
+#define MII_88E1318S_PHY_LED_POL_ACT_HIGH(idx)	BIT(2 * (idx))
+#define MII_88E1318S_PHY_LED_POL_HIGH_IMP(idx)	BIT(2 * (idx) + 1)
 #define MII_88E1318S_PHY_LED_TCR		0x12
 #define MII_88E1318S_PHY_LED_TCR_FORCE_INT	BIT(15)
 #define MII_88E1318S_PHY_LED_TCR_INTn_ENABLE	BIT(7)
@@ -304,6 +307,8 @@
 #define NB_FIBER_STATS	1
 #define NB_STAT_MAX	3
 
+#define M88E1318S_MAX_LEDS	3
+
 MODULE_DESCRIPTION("Marvell PHY driver");
 MODULE_AUTHOR("Andy Fleming");
 MODULE_LICENSE("GPL");
@@ -3572,7 +3577,7 @@ static int m88e1318_led_hw_control_get(struct phy_device *phydev, u8 index,
 {
 	int mode, reg;
 
-	if (index > 2)
+	if (index >= M88E1318S_MAX_LEDS)
 		return -EINVAL;
 
 	reg = phy_read_paged(phydev, MII_MARVELL_LED_PAGE,
@@ -3585,6 +3590,36 @@ static int m88e1318_led_hw_control_get(struct phy_device *phydev, u8 index,
 	return marvell_get_led_rules(index, rules, mode);
 }
 
+static int m88e1318_led_polarity_set(struct phy_device *phydev, int index,
+				     unsigned long modes)
+{
+	u16 val = 0;
+	u32 mode;
+
+	if (index >= M88E1318S_MAX_LEDS)
+		return -EINVAL;
+
+	for_each_set_bit(mode, &modes, __PHY_LED_MODES_NUM) {
+		switch (mode) {
+		case PHY_LED_ACTIVE_LOW:
+			break;
+		case PHY_LED_ACTIVE_HIGH:
+			val |= MII_88E1318S_PHY_LED_POL_ACT_HIGH(index);
+			break;
+		case PHY_LED_INACTIVE_HIGH_IMPEDANCE:
+			val |= MII_88E1318S_PHY_LED_POL_HIGH_IMP(index);
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	return phy_modify_paged(phydev, MII_MARVELL_LED_PAGE,
+				MII_88E1318S_PHY_LED_POL,
+				MII_88E1318S_PHY_LED_POL_ACT_HIGH(index) |
+				MII_88E1318S_PHY_LED_POL_HIGH_IMP(index), val);
+}
+
 static int marvell_probe(struct phy_device *phydev)
 {
 	struct marvell_priv *priv;
@@ -3833,6 +3868,7 @@ static struct phy_driver marvell_drivers[] = {
 		.led_hw_is_supported = m88e1318_led_hw_is_supported,
 		.led_hw_control_set = m88e1318_led_hw_control_set,
 		.led_hw_control_get = m88e1318_led_hw_control_get,
+		.led_polarity_set = m88e1318_led_polarity_set,
 	},
 	{
 		.phy_id = MARVELL_PHY_ID_88E1145,
@@ -3947,6 +3983,7 @@ static struct phy_driver marvell_drivers[] = {
 		.led_hw_is_supported = m88e1318_led_hw_is_supported,
 		.led_hw_control_set = m88e1318_led_hw_control_set,
 		.led_hw_control_get = m88e1318_led_hw_control_get,
+		.led_polarity_set = m88e1318_led_polarity_set,
 		.attach_mii_port = m88e1510_attach_mii_port,
 	},
 	{
-- 
2.53.0

Re: [PATCH net-next v2] net: phy: marvell: add support for PHY LEDs polarity modes

[Andrew Lunn]

On Wed, Sep 23, 2026 at 10:45:24PM +0200, Aleksander Jan Bajkowski wrote:
> Add the led_polarity_set callback for setting LED polarity. Implement
> this callback for the 88E1318 and 88E1510 PHYs. This should also work on
> other Marvell PHYs, but I don't have access to the TRM or the hardware.
> 
> Tested on Adapteva Paralella board with Marvell 88E1318 PHY.
> 
> Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
> ---
> Changes in v2:
>  - add suppoer for PHY_LED_INACTIVE_HIGH_IMPEDANCE mode
> ---
>  drivers/net/phy/marvell.c | 39 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> index f71cffa88406..d57fcabfec2b 100644
> --- a/drivers/net/phy/marvell.c
> +++ b/drivers/net/phy/marvell.c
> @@ -149,6 +149,9 @@
>  #define MII_88E1318S_PHY_LED_FUNC_ON		(0x9)
>  #define MII_88E1318S_PHY_LED_FUNC_HI_Z		(0xa)
>  #define MII_88E1318S_PHY_LED_FUNC_BLINK		(0xb)
> +#define MII_88E1318S_PHY_LED_POL		0x11
> +#define MII_88E1318S_PHY_LED_POL_ACT_HIGH(idx)	BIT(2 * (idx))
> +#define MII_88E1318S_PHY_LED_POL_HIGH_IMP(idx)	BIT(2 * (idx) + 1)
>  #define MII_88E1318S_PHY_LED_TCR		0x12
>  #define MII_88E1318S_PHY_LED_TCR_FORCE_INT	BIT(15)
>  #define MII_88E1318S_PHY_LED_TCR_INTn_ENABLE	BIT(7)
> @@ -304,6 +307,8 @@
>  #define NB_FIBER_STATS	1
>  #define NB_STAT_MAX	3
>  
> +#define M88E1318S_MAX_LEDS	3

Why the M88E1318S prefix?

The naming is a bit odd in this file, but maybe NB_MAX_LEDS?

    Andrew

Re: [PATCH net-next v2] net: phy: marvell: add support for PHY LEDs polarity modes

[Aleksander Jan Bajkowski]

Hi Andrew,

On 23/09/2026 23:07, Andrew Lunn wrote:
> On Wed, Sep 23, 2026 at 10:45:24PM +0200, Aleksander Jan Bajkowski wrote:
>> Add the led_polarity_set callback for setting LED polarity. Implement
>> this callback for the 88E1318 and 88E1510 PHYs. This should also work on
>> other Marvell PHYs, but I don't have access to the TRM or the hardware.
>>
>> Tested on Adapteva Paralella board with Marvell 88E1318 PHY.
>>
>> Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
>> ---
>> Changes in v2:
>>   - add suppoer for PHY_LED_INACTIVE_HIGH_IMPEDANCE mode
>> ---
>>   drivers/net/phy/marvell.c | 39 ++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 38 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
>> index f71cffa88406..d57fcabfec2b 100644
>> --- a/drivers/net/phy/marvell.c
>> +++ b/drivers/net/phy/marvell.c
>> @@ -149,6 +149,9 @@
>>   #define MII_88E1318S_PHY_LED_FUNC_ON		(0x9)
>>   #define MII_88E1318S_PHY_LED_FUNC_HI_Z		(0xa)
>>   #define MII_88E1318S_PHY_LED_FUNC_BLINK		(0xb)
>> +#define MII_88E1318S_PHY_LED_POL		0x11
>> +#define MII_88E1318S_PHY_LED_POL_ACT_HIGH(idx)	BIT(2 * (idx))
>> +#define MII_88E1318S_PHY_LED_POL_HIGH_IMP(idx)	BIT(2 * (idx) + 1)
>>   #define MII_88E1318S_PHY_LED_TCR		0x12
>>   #define MII_88E1318S_PHY_LED_TCR_FORCE_INT	BIT(15)
>>   #define MII_88E1318S_PHY_LED_TCR_INTn_ENABLE	BIT(7)
>> @@ -304,6 +307,8 @@
>>   #define NB_FIBER_STATS	1
>>   #define NB_STAT_MAX	3
>>   
>> +#define M88E1318S_MAX_LEDS	3
> Why the M88E1318S prefix?

The number of LEDs depends on the PHY variant:
88E1112 -> 4
88E1116R -> 3
88E1240 -> 4
88E1318S -> 3
88E1510 -> 3
88E1545 -> 4
88E1548P -> 4
All of these PHYs likely have the same polarity register layout.

>
> The naming is a bit odd in this file, but maybe NB_MAX_LEDS?
NB_MAX_LEDS might be confusing since the number of LEDs can
vary, but I don't have a preference and can change M88E1318S to 
NB_MAX_LEDS.
>
>      Andrew

Re: [PATCH net-next v2] net: phy: marvell: add support for PHY LEDs polarity modes

[Andrew Lunn]

> The number of LEDs depends on the PHY variant:
> 88E1112 -> 4
> 88E1116R -> 3
> 88E1240 -> 4
> 88E1318S -> 3
> 88E1510 -> 3
> 88E1545 -> 4
> 88E1548P -> 4

Ah, i had a quick look at the current code, and all the functions have
a switch statement with case 0, 1, 2. So i assumed that all PHYs have
the same number of LEDs.

So any PHY which does have 4 is currently not supported?

So i agree NB_MAX_LED is not a great idea.

   Andrew

Re: [PATCH net-next v2] net: phy: marvell: add support for PHY LEDs polarity modes

[Aleksander Jan Bajkowski]

Hi Andrew,

On 24/09/2026 00:54, Andrew Lunn wrote:
>> The number of LEDs depends on the PHY variant:
>> 88E1112 -> 4
>> 88E1116R -> 3
>> 88E1240 -> 4
>> 88E1318S -> 3
>> 88E1510 -> 3
>> 88E1545 -> 4
>> 88E1548P -> 4
> Ah, i had a quick look at the current code, and all the functions have
> a switch statement with case 0, 1, 2. So i assumed that all PHYs have
> the same number of LEDs.
>
> So any PHY which does have 4 is currently not supported?
Correct. Although some PHY variants support 4 LEDs, only three can be
configured via the LED API. The fourth LED can only be configured via
the 'marvell,reg-init' property.

Best regards,
Aleksander

Re: [PATCH net-next v2] net: phy: marvell: add support for PHY LEDs polarity modes

[Andrew Lunn]

On Thu, Sep 24, 2026 at 08:43:36PM +0200, Aleksander Jan Bajkowski wrote:
> Hi Andrew,
> 
> On 24/09/2026 00:54, Andrew Lunn wrote:
> > > The number of LEDs depends on the PHY variant:
> > > 88E1112 -> 4
> > > 88E1116R -> 3
> > > 88E1240 -> 4
> > > 88E1318S -> 3
> > > 88E1510 -> 3
> > > 88E1545 -> 4
> > > 88E1548P -> 4
> > Ah, i had a quick look at the current code, and all the functions have
> > a switch statement with case 0, 1, 2. So i assumed that all PHYs have
> > the same number of LEDs.
> > 
> > So any PHY which does have 4 is currently not supported?

> Correct. Although some PHY variants support 4 LEDs, only three can be
> configured via the LED API. The fourth LED can only be configured via
> the 'marvell,reg-init' property.

'marvell,reg-init' is deprecated and should not be used in anything
modern. The code should be changed to support 4 LEDs when 4 exist.

	Andrew

Re: [PATCH net-next v2] net: phy: marvell: add support for PHY LEDs polarity modes

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The polarity and tristate settings written to page 3 reg 17
  (MII_88E1318S_PHY_LED_POL) are programmed only once.

--

net: phy: marvell: add support for PHY LEDs polarity modes

This commit adds a led_polarity_set callback for the 88E1318 and 88E1510
PHYs. The callback programs the per-LED active-high and high-impedance
bits in page 3, register 0x11.

> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> index f71cffa884062..d57fcabfec2b9 100644
> --- a/drivers/net/phy/marvell.c
> +++ b/drivers/net/phy/marvell.c

[ ... ]

> @@ -3585,6 +3590,36 @@ static int m88e1318_led_hw_control_get(struct phy_device *phydev, u8 index,
>  	return marvell_get_led_rules(index, rules, mode);
>  }
>  
> +static int m88e1318_led_polarity_set(struct phy_device *phydev, int index,
> +				     unsigned long modes)
> +{

[ ... ]

> +	return phy_modify_paged(phydev, MII_MARVELL_LED_PAGE,
> +				MII_88E1318S_PHY_LED_POL,
> +				MII_88E1318S_PHY_LED_POL_ACT_HIGH(index) |
> +				MII_88E1318S_PHY_LED_POL_HIGH_IMP(index), val);
> +}

[Severity: Medium]
Can the polarity and high-impedance settings be lost after the PHY goes
through a hardware reset?

The register is only written once, from probe:

phy_probe()
  of_phy_leds()
    of_phy_led()
      m88e1318_led_polarity_set()

The requested modes are not saved in struct marvell_priv. Neither
m88e1318_config_init() nor m88e1510_config_init() (through
marvell_config_init()) rewrites MII_88E1318S_PHY_LED_POL.

phy_detach() puts the PHY into reset:

drivers/net/phy/phy_device.c:phy_detach() {
    ...
	/* Assert the reset signal */
	phy_device_reset(phydev, 1);
    ...
}

On the next attach, phy_attach_direct()->phy_init_hw() releases reset and
runs config_init, but of_phy_leds() is not called again.

Take a board with a PHY reset GPIO and a MAC driver that connects the PHY
in ndo_open and disconnects it in ndo_stop. Wouldn't each "ip link set
down" followed by "ip link set up" put the LED pins back to their reset
default polarity and drive state?

The same thing looks possible after resume if the PHY lost power, via
mdio_bus_phy_resume()->phy_init_hw().

qca808x avoids this. It keeps led_polarity_mode in its priv structure and
reapplies it from qca808x_config_init(). Would a similar approach work
here, with the per-LED modes saved in marvell_priv and restored from
config_init?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923204614.471409-1-olek2%40wp.pl