Messages in this thread | | | Subject | Re: [PATCH net-next 1/2] net: phy: marvell: add new m88e1510 LED configuration | From | "shenjian (K)" <> | Date | Thu, 14 Feb 2019 15:24:44 +0800 |
| |
在 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 */ >> >
| |