net: phy: dp83867: restore LED configuration after a soft reset

The LED configuration lives in LEDCR1 (the per-LED function nibble) and
LEDCR2 (per-LED driver enable/value and polarity). The driver programs it
through the LED class callbacks -- dp83867_led_brightness_set(),
dp83867_led_hw_control_set() and dp83867_led_polarity_set() -- either once
from device tree at probe, or at runtime from sysfs and the netdev trigger.

dp83867_phy_reset(), the .soft_reset callback, issues a global software
reset (CTRL SW_RESET), which the datasheet defines as resetting all
registers, including the extended registers, to their defaults.
phy_init_hw() runs .soft_reset before .config_init on every attach and
resume -- phy_attach_direct(), mdio_bus_phy_resume() and MAC drivers -- so
the LED configuration is wiped from the first attach onward and never
restored. A device-tree polarity is lost, a manually driven LED goes dark,
and an LED offloaded to the netdev trigger whose link stays down after a
resume keeps the reset-default function until the next link event.

Shadow what the LED callbacks program, as a value and a written-bits mask
per register, and replay it from config_init(), which runs right after the
soft reset. Only bits the driver actually set are restored.

The callbacks run under phydev->lock, but config_init() must not take it:
the cable-test abort path in phy_state_machine() already holds phydev->lock
when it reaches phy_init_hw(), so config_init() taking it would deadlock.
Serialize the shadow and its replay with a dedicated lock instead.

Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
Link: https://patch.msgid.link/20260908235740.120112-1-donggeunyoo.kernel@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Donggeun Yoo 2026-09-09 08:57:40 +09:00 committed by Jakub Kicinski
parent 1f20151408
commit 043777e948

View File

@ -8,6 +8,7 @@
#include <linux/kernel.h>
#include <linux/mii.h>
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/of.h>
#include <linux/phy.h>
#include <linux/delay.h>
@ -196,6 +197,16 @@ struct dp83867_private {
bool set_clk_output;
u32 clk_output_sel;
bool sgmii_ref_clk_en;
/* Shadow of the LED registers, replayed after a soft reset. led_lock
* serializes the shadow and its replay against the LED callbacks,
* because dp83867_config_init() does the replay off phydev->lock.
*/
struct mutex led_lock;
u16 ledcr1;
u16 ledcr1_mask;
u16 ledcr2;
u16 ledcr2_mask;
};
static int dp83867_ack_interrupt(struct phy_device *phydev)
@ -722,6 +733,7 @@ static int dp83867_resume(struct phy_device *phydev)
static int dp83867_probe(struct phy_device *phydev)
{
struct dp83867_private *dp83867;
int ret;
dp83867 = devm_kzalloc(&phydev->mdio.dev, sizeof(*dp83867),
GFP_KERNEL);
@ -730,9 +742,60 @@ static int dp83867_probe(struct phy_device *phydev)
phydev->priv = dp83867;
ret = devm_mutex_init(&phydev->mdio.dev, &dp83867->led_lock);
if (ret)
return ret;
return dp83867_of_init(phydev);
}
/* Update an LED register and mirror the change into the shadow, so that
* dp83867_config_init() can replay it after a soft reset. Caller must hold
* dp83867->led_lock.
*/
static int __dp83867_led_modify(struct phy_device *phydev, u32 reg,
u16 mask, u16 val)
{
struct dp83867_private *dp83867 = phydev->priv;
int ret;
ret = phy_modify(phydev, reg, mask, val);
if (ret)
return ret;
if (reg == DP83867_LEDCR1) {
dp83867->ledcr1 = (dp83867->ledcr1 & ~mask) | (val & mask);
dp83867->ledcr1_mask |= mask;
} else if (reg == DP83867_LEDCR2) {
dp83867->ledcr2 = (dp83867->ledcr2 & ~mask) | (val & mask);
dp83867->ledcr2_mask |= mask;
} else {
WARN_ON_ONCE(1);
}
return 0;
}
/* Restore the LED registers the driver has programmed, cleared by the soft
* reset in dp83867_phy_reset().
*/
static int dp83867_led_restore(struct phy_device *phydev)
{
struct dp83867_private *dp83867 = phydev->priv;
int ret = 0;
mutex_lock(&dp83867->led_lock);
if (dp83867->ledcr1_mask)
ret = phy_modify(phydev, DP83867_LEDCR1,
dp83867->ledcr1_mask, dp83867->ledcr1);
if (!ret && dp83867->ledcr2_mask)
ret = phy_modify(phydev, DP83867_LEDCR2,
dp83867->ledcr2_mask, dp83867->ledcr2);
mutex_unlock(&dp83867->led_lock);
return ret;
}
static int dp83867_config_init(struct phy_device *phydev)
{
struct dp83867_private *dp83867 = phydev->priv;
@ -896,6 +959,10 @@ static int dp83867_config_init(struct phy_device *phydev)
mask, val);
}
ret = dp83867_led_restore(phydev);
if (ret)
phydev_warn(phydev, "failed to restore LED config: %d\n", ret);
return 0;
}
@ -1004,7 +1071,9 @@ static int
dp83867_led_brightness_set(struct phy_device *phydev,
u8 index, enum led_brightness brightness)
{
u32 val;
struct dp83867_private *dp83867 = phydev->priv;
u16 val;
int ret;
if (index >= DP83867_LED_COUNT)
return -EINVAL;
@ -1015,10 +1084,13 @@ dp83867_led_brightness_set(struct phy_device *phydev,
if (brightness)
val |= DP83867_LED_DRV_VAL(index);
return phy_modify(phydev, DP83867_LEDCR2,
DP83867_LED_DRV_VAL(index) |
DP83867_LED_DRV_EN(index),
val);
mutex_lock(&dp83867->led_lock);
ret = __dp83867_led_modify(phydev, DP83867_LEDCR2,
DP83867_LED_DRV_VAL(index) |
DP83867_LED_DRV_EN(index), val);
mutex_unlock(&dp83867->led_lock);
return ret;
}
static int dp83867_led_mode(u8 index, unsigned long rules)
@ -1069,18 +1141,24 @@ static int dp83867_led_hw_is_supported(struct phy_device *phydev, u8 index,
static int dp83867_led_hw_control_set(struct phy_device *phydev, u8 index,
unsigned long rules)
{
struct dp83867_private *dp83867 = phydev->priv;
int mode, ret;
mode = dp83867_led_mode(index, rules);
if (mode < 0)
return mode;
ret = phy_modify(phydev, DP83867_LEDCR1, DP83867_LED_FN_MASK(index),
DP83867_LED_FN(index, mode));
if (ret)
return ret;
mutex_lock(&dp83867->led_lock);
ret = __dp83867_led_modify(phydev, DP83867_LEDCR1,
DP83867_LED_FN_MASK(index),
DP83867_LED_FN(index, mode));
if (!ret)
ret = __dp83867_led_modify(phydev, DP83867_LEDCR2,
DP83867_LED_DRV_EN(index) |
DP83867_LED_DRV_VAL(index), 0);
mutex_unlock(&dp83867->led_lock);
return phy_modify(phydev, DP83867_LEDCR2, DP83867_LED_DRV_EN(index), 0);
return ret;
}
static int dp83867_led_hw_control_get(struct phy_device *phydev, u8 index,
@ -1141,9 +1219,11 @@ static int dp83867_led_hw_control_get(struct phy_device *phydev, u8 index,
static int dp83867_led_polarity_set(struct phy_device *phydev, int index,
unsigned long modes)
{
struct dp83867_private *dp83867 = phydev->priv;
/* Default active high */
u16 polarity = DP83867_LED_POLARITY(index);
u32 mode;
int ret;
for_each_set_bit(mode, &modes, __PHY_LED_MODES_NUM) {
switch (mode) {
@ -1157,8 +1237,13 @@ static int dp83867_led_polarity_set(struct phy_device *phydev, int index,
return -EINVAL;
}
}
return phy_modify(phydev, DP83867_LEDCR2,
DP83867_LED_POLARITY(index), polarity);
mutex_lock(&dp83867->led_lock);
ret = __dp83867_led_modify(phydev, DP83867_LEDCR2,
DP83867_LED_POLARITY(index), polarity);
mutex_unlock(&dp83867->led_lock);
return ret;
}
static unsigned int dp83867_inband_caps(struct phy_device *phydev,