| Message ID | 1550118667-119947-2-git-send-email-shenjian15@huawei.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: phy: add new led configuration for marvell m88e1510 | expand |
On 2/13/2019 8:31 PM, Jian Shen wrote: > The default m88e1510 LED configuration is 0x1177, used LED[0] > for 1000M link, LED[1] for 100M link, and LED[2] for active. > But for our boards, we want to use 0x1040, which use LED[0] for > link, and LED[1] for active. > > This patch adds a new m88e1510 LED configuration for it. There appears to be a precedent with the DNS323 flag that was defined for the same purpose, but this unfortunately does not scale we cannot have every new platform come up with its own LED configuration without having a more structured approach to representing the LED configuration. Maybe we can encode the desired LED behavior in a more generic way and utilize the 32 flag bits available to denote a selection, e.g.: MARVELL_PHY_FLAG_LED0_100M BIT(3) MARVELL_PHY_FLAG_LED0_1000M BIT(4) etc. or maybe even better would be to expose the LEDs using the standard LEDs class subsystem and allow configuring different triggers. We have some amount of support for PHY LEDs already in tree, but AFAIR what we do not have support for is a "hardware blinking" trigger which those LEDs are. > > Signed-off-by: Jian Shen <shenjian15@huawei.com> > --- > drivers/net/phy/marvell.c | 22 +++++++++++++++++++++- > include/linux/marvell_phy.h | 1 + > 2 files changed, 22 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c > index 3ccba37..c195286 100644 > --- a/drivers/net/phy/marvell.c > +++ b/drivers/net/phy/marvell.c > @@ -128,6 +128,10 @@ > #define MII_PHY_LED_CTRL 16 > #define MII_88E1121_PHY_LED_DEF 0x0030 > #define MII_88E1510_PHY_LED_DEF 0x1177 > +#define MII_88E1510_PHY_HNS3_LED_DEF 0x1040 > + > +#define MII_88E1510_PHY_LED_POLARITY_CTRL 0x11 > +#define MII_88E1510_PHY_HNS3_LED_POLARITY 0x4415 > > #define MII_M1011_PHY_STATUS 0x11 > #define MII_M1011_PHY_STATUS_1000 0x8000 > @@ -619,12 +623,19 @@ static void marvell_config_led(struct phy_device *phydev) > def_config = MII_88E1121_PHY_LED_DEF; > break; > /* Default PHY LED config: > + * For hns3: > + * LED[0] .. Link > + * LED[1] .. Activity > + * For others: > * LED[0] .. 1000Mbps Link > * LED[1] .. 100Mbps Link > * LED[2] .. Blink, Activity > */ > case MARVELL_PHY_FAMILY_ID(MARVELL_PHY_ID_88E1510): > - def_config = MII_88E1510_PHY_LED_DEF; > + if (phydev->dev_flags & MARVELL_PHY_M1510_HNS3_LEDS) > + def_config = MII_88E1510_PHY_HNS3_LED_DEF; > + else > + def_config = MII_88E1510_PHY_LED_DEF; > break; > default: > return; > @@ -634,6 +645,15 @@ static void marvell_config_led(struct phy_device *phydev) > def_config); > if (err < 0) > phydev_warn(phydev, "Fail to config marvell phy LED.\n"); > + > + if (phydev->dev_flags & MARVELL_PHY_M1510_HNS3_LEDS) { > + err = phy_write_paged(phydev, MII_MARVELL_LED_PAGE, > + MII_88E1510_PHY_LED_POLARITY_CTRL, > + MII_88E1510_PHY_HNS3_LED_POLARITY); > + if (err < 0) > + phydev_warn(phydev, > + "Fail to config marvell phy LED polarity.\n"); > + } > } > > static int marvell_config_init(struct phy_device *phydev) > diff --git a/include/linux/marvell_phy.h b/include/linux/marvell_phy.h > index 1eb6f24..99e0bbb 100644 > --- a/include/linux/marvell_phy.h > +++ b/include/linux/marvell_phy.h > @@ -32,5 +32,6 @@ > /* struct phy_device dev_flags definitions */ > #define MARVELL_PHY_M1145_FLAGS_RESISTANCE 0x00000001 > #define MARVELL_PHY_M1118_DNS323_LEDS 0x00000002 > +#define MARVELL_PHY_M1510_HNS3_LEDS 0x00000004 > > #endif /* _MARVELL_PHY_H */ >
在 2019/2/14 12:06, Florian Fainelli 写道: > > > On 2/13/2019 8:31 PM, Jian Shen wrote: >> The default m88e1510 LED configuration is 0x1177, used LED[0] >> for 1000M link, LED[1] for 100M link, and LED[2] for active. >> But for our boards, we want to use 0x1040, which use LED[0] for >> link, and LED[1] for active. >> >> This patch adds a new m88e1510 LED configuration for it. > > There appears to be a precedent with the DNS323 flag that was defined > for the same purpose, but this unfortunately does not scale we cannot > have every new platform come up with its own LED configuration without > having a more structured approach to representing the LED configuration. > > Maybe we can encode the desired LED behavior in a more generic way and > utilize the 32 flag bits available to denote a selection, e.g.: > > MARVELL_PHY_FLAG_LED0_100M BIT(3) > MARVELL_PHY_FLAG_LED0_1000M BIT(4) > > etc. > > or maybe even better would be to expose the LEDs using the standard LEDs > class subsystem and allow configuring different triggers. We have some > amount of support for PHY LEDs already in tree, but AFAIR what we do not > have support for is a "hardware blinking" trigger which those LEDs are. > Thanks Florian. I agree that we should use a generic way. Maybe I understood incorrectly, for "utilize the 32 flag bits available to denote a selection", do you mean to define a serial flags for each usage of each led ? For each led can be configured to several usages, include link/10M link/100M link/1000M link/active/..., it needs a lot bits for these combination, e.g: #define MARVELL_PHY_FLAG_LED0_LINK BIT(2) #define MARVELL_PHY_FLAG_LED0_LINK_100M BIT(3) #define MARVELL_PHY_FLAG_LED0_LINK_1000M BIT(4) #define MARVELL_PHY_FLAG_LED0_ACTIVE BIT(5) #define MARVELL_PHY_FLAG_LED1_LINK BIT(6) #define MARVELL_PHY_FLAG_LED1_LINK_100M BIT(7) #define MARVELL_PHY_FLAG_LED1_LINK_1000M BIT(8) #define MARVELL_PHY_FLAG_LED1_ACTIVE BIT(9) ... Jian Shen >> >> Signed-off-by: Jian Shen <shenjian15@huawei.com> >> --- >> drivers/net/phy/marvell.c | 22 +++++++++++++++++++++- >> include/linux/marvell_phy.h | 1 + >> 2 files changed, 22 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c >> index 3ccba37..c195286 100644 >> --- a/drivers/net/phy/marvell.c >> +++ b/drivers/net/phy/marvell.c >> @@ -128,6 +128,10 @@ >> #define MII_PHY_LED_CTRL 16 >> #define MII_88E1121_PHY_LED_DEF 0x0030 >> #define MII_88E1510_PHY_LED_DEF 0x1177 >> +#define MII_88E1510_PHY_HNS3_LED_DEF 0x1040 >> + >> +#define MII_88E1510_PHY_LED_POLARITY_CTRL 0x11 >> +#define MII_88E1510_PHY_HNS3_LED_POLARITY 0x4415 >> >> #define MII_M1011_PHY_STATUS 0x11 >> #define MII_M1011_PHY_STATUS_1000 0x8000 >> @@ -619,12 +623,19 @@ static void marvell_config_led(struct phy_device *phydev) >> def_config = MII_88E1121_PHY_LED_DEF; >> break; >> /* Default PHY LED config: >> + * For hns3: >> + * LED[0] .. Link >> + * LED[1] .. Activity >> + * For others: >> * LED[0] .. 1000Mbps Link >> * LED[1] .. 100Mbps Link >> * LED[2] .. Blink, Activity >> */ >> case MARVELL_PHY_FAMILY_ID(MARVELL_PHY_ID_88E1510): >> - def_config = MII_88E1510_PHY_LED_DEF; >> + if (phydev->dev_flags & MARVELL_PHY_M1510_HNS3_LEDS) >> + def_config = MII_88E1510_PHY_HNS3_LED_DEF; >> + else >> + def_config = MII_88E1510_PHY_LED_DEF; >> break; >> default: >> return; >> @@ -634,6 +645,15 @@ static void marvell_config_led(struct phy_device *phydev) >> def_config); >> if (err < 0) >> phydev_warn(phydev, "Fail to config marvell phy LED.\n"); >> + >> + if (phydev->dev_flags & MARVELL_PHY_M1510_HNS3_LEDS) { >> + err = phy_write_paged(phydev, MII_MARVELL_LED_PAGE, >> + MII_88E1510_PHY_LED_POLARITY_CTRL, >> + MII_88E1510_PHY_HNS3_LED_POLARITY); >> + if (err < 0) >> + phydev_warn(phydev, >> + "Fail to config marvell phy LED polarity.\n"); >> + } >> } >> >> static int marvell_config_init(struct phy_device *phydev) >> diff --git a/include/linux/marvell_phy.h b/include/linux/marvell_phy.h >> index 1eb6f24..99e0bbb 100644 >> --- a/include/linux/marvell_phy.h >> +++ b/include/linux/marvell_phy.h >> @@ -32,5 +32,6 @@ >> /* struct phy_device dev_flags definitions */ >> #define MARVELL_PHY_M1145_FLAGS_RESISTANCE 0x00000001 >> #define MARVELL_PHY_M1118_DNS323_LEDS 0x00000002 >> +#define MARVELL_PHY_M1510_HNS3_LEDS 0x00000004 >> >> #endif /* _MARVELL_PHY_H */ >> >
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index 3ccba37..c195286 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -128,6 +128,10 @@ #define MII_PHY_LED_CTRL 16 #define MII_88E1121_PHY_LED_DEF 0x0030 #define MII_88E1510_PHY_LED_DEF 0x1177 +#define MII_88E1510_PHY_HNS3_LED_DEF 0x1040 + +#define MII_88E1510_PHY_LED_POLARITY_CTRL 0x11 +#define MII_88E1510_PHY_HNS3_LED_POLARITY 0x4415 #define MII_M1011_PHY_STATUS 0x11 #define MII_M1011_PHY_STATUS_1000 0x8000 @@ -619,12 +623,19 @@ static void marvell_config_led(struct phy_device *phydev) def_config = MII_88E1121_PHY_LED_DEF; break; /* Default PHY LED config: + * For hns3: + * LED[0] .. Link + * LED[1] .. Activity + * For others: * LED[0] .. 1000Mbps Link * LED[1] .. 100Mbps Link * LED[2] .. Blink, Activity */ case MARVELL_PHY_FAMILY_ID(MARVELL_PHY_ID_88E1510): - def_config = MII_88E1510_PHY_LED_DEF; + if (phydev->dev_flags & MARVELL_PHY_M1510_HNS3_LEDS) + def_config = MII_88E1510_PHY_HNS3_LED_DEF; + else + def_config = MII_88E1510_PHY_LED_DEF; break; default: return; @@ -634,6 +645,15 @@ static void marvell_config_led(struct phy_device *phydev) def_config); if (err < 0) phydev_warn(phydev, "Fail to config marvell phy LED.\n"); + + if (phydev->dev_flags & MARVELL_PHY_M1510_HNS3_LEDS) { + err = phy_write_paged(phydev, MII_MARVELL_LED_PAGE, + MII_88E1510_PHY_LED_POLARITY_CTRL, + MII_88E1510_PHY_HNS3_LED_POLARITY); + if (err < 0) + phydev_warn(phydev, + "Fail to config marvell phy LED polarity.\n"); + } } static int marvell_config_init(struct phy_device *phydev) diff --git a/include/linux/marvell_phy.h b/include/linux/marvell_phy.h index 1eb6f24..99e0bbb 100644 --- a/include/linux/marvell_phy.h +++ b/include/linux/marvell_phy.h @@ -32,5 +32,6 @@ /* struct phy_device dev_flags definitions */ #define MARVELL_PHY_M1145_FLAGS_RESISTANCE 0x00000001 #define MARVELL_PHY_M1118_DNS323_LEDS 0x00000002 +#define MARVELL_PHY_M1510_HNS3_LEDS 0x00000004 #endif /* _MARVELL_PHY_H */
The default m88e1510 LED configuration is 0x1177, used LED[0] for 1000M link, LED[1] for 100M link, and LED[2] for active. But for our boards, we want to use 0x1040, which use LED[0] for link, and LED[1] for active. This patch adds a new m88e1510 LED configuration for it. Signed-off-by: Jian Shen <shenjian15@huawei.com> --- drivers/net/phy/marvell.c | 22 +++++++++++++++++++++- include/linux/marvell_phy.h | 1 + 2 files changed, 22 insertions(+), 1 deletion(-)