| Message ID | 20210215165800.14580-3-bjarni.jonasson@microchip.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v2,1/3] net: phy: mscc: adding LCPLL reset to VSC8514 | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | warning | Series does not have a cover letter |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | fail | 2 blamed authors not CCed: quentin.schulz@bootlin.com kavyasree.kotagiri@microchip.com; 3 maintainers not CCed: quentin.schulz@bootlin.com kavyasree.kotagiri@microchip.com dmurphy@ti.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 37 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On 15.02.2021 17:58, Bjarni Jonasson wrote: > The 'coma mode' (configurable through sw or hw) provides an > optional feature that may be used to control when the PHYs become active. > The typical usage is to synchronize the link-up time across > all PHY instances. This patch releases coma mode if not done by hardware, > otherwise the phys will not link-up > > Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com> > Signed-off-by: Bjarni Jonasson <bjarni.jonasson@microchip.com> > Fixes: e4f9ba642f0b ("net: phy: mscc: add support for VSC8514 PHY.") > --- > v1 -> v2: > Modified coma mode config > Changed net to net-next > > drivers/net/phy/mscc/mscc.h | 3 +++ > drivers/net/phy/mscc/mscc_main.c | 16 ++++++++++++++++ > 2 files changed, 19 insertions(+) > > diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h > index 9d8ee387739e..2b70ccd1b256 100644 > --- a/drivers/net/phy/mscc/mscc.h > +++ b/drivers/net/phy/mscc/mscc.h > @@ -160,6 +160,9 @@ enum rgmii_clock_delay { > #define MSCC_PHY_PAGE_TR 0x52b5 /* Token ring registers */ > #define MSCC_PHY_GPIO_CONTROL_2 14 > > +#define MSCC_PHY_COMA_MODE 0x2000 /* input(1) / output(0) */ > +#define MSCC_PHY_COMA_OUTPUT 0x1000 /* value to output */ > + > /* Extended Page 1 Registers */ > #define MSCC_PHY_CU_MEDIA_CRC_VALID_CNT 18 > #define VALID_CRC_CNT_CRC_MASK GENMASK(13, 0) > diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c > index 03181542bcb7..29302ccf7e7b 100644 > --- a/drivers/net/phy/mscc/mscc_main.c > +++ b/drivers/net/phy/mscc/mscc_main.c > @@ -1520,6 +1520,21 @@ static void vsc8584_get_base_addr(struct phy_device *phydev) > vsc8531->addr = addr; > } > > +static void vsc85xx_coma_mode_release(struct phy_device *phydev) > +{ > + /* The coma mode (pin or reg) provides an optional feature that > + * may be used to control when the PHYs become active. > + * Alternatively the COMA_MODE pin may be connected low > + * so that the PHYs are fully active once out of reset. > + */ > + phy_unlock_mdio_bus(phydev); > + /* Enable output (mode=0) and write zero to it */ > + phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_GPIO, > + MSCC_PHY_GPIO_CONTROL_2, > + MSCC_PHY_COMA_MODE | MSCC_PHY_COMA_OUTPUT, 0); > + phy_lock_mdio_bus(phydev); The temporary unlock is a little bit hacky. Better do: vsc85xx_phy_write_page(MSCC_PHY_PAGE_EXTENDED_GPIO) __phy_modify() vsc85xx_phy_write_page(default page) Alternatively we could add __phy_modify_paged(). But this may not be worth the effort for now. > +} > + > static int vsc8584_config_init(struct phy_device *phydev) > { > struct vsc8531_private *vsc8531 = phydev->priv; > @@ -2604,6 +2619,7 @@ static int vsc8514_config_init(struct phy_device *phydev) > ret = vsc8514_config_host_serdes(phydev); > if (ret) > goto err; > + vsc85xx_coma_mode_release(phydev); > } > > phy_unlock_mdio_bus(phydev); >
On Mon, 2021-02-15 at 21:08 +0100, Heiner Kallweit wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > On 15.02.2021 17:58, Bjarni Jonasson wrote: > > The 'coma mode' (configurable through sw or hw) provides an > > optional feature that may be used to control when the PHYs become > > active. > > The typical usage is to synchronize the link-up time across > > all PHY instances. This patch releases coma mode if not done by > > hardware, > > otherwise the phys will not link-up > > > > Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com> > > Signed-off-by: Bjarni Jonasson <bjarni.jonasson@microchip.com> > > Fixes: e4f9ba642f0b ("net: phy: mscc: add support for VSC8514 > > PHY.") > > --- > > v1 -> v2: > > Modified coma mode config > > Changed net to net-next > > > > drivers/net/phy/mscc/mscc.h | 3 +++ > > drivers/net/phy/mscc/mscc_main.c | 16 ++++++++++++++++ > > 2 files changed, 19 insertions(+) > > > > diff --git a/drivers/net/phy/mscc/mscc.h > > b/drivers/net/phy/mscc/mscc.h > > index 9d8ee387739e..2b70ccd1b256 100644 > > --- a/drivers/net/phy/mscc/mscc.h > > +++ b/drivers/net/phy/mscc/mscc.h > > @@ -160,6 +160,9 @@ enum rgmii_clock_delay { > > #define MSCC_PHY_PAGE_TR 0x52b5 /* Token ring > > registers */ > > #define MSCC_PHY_GPIO_CONTROL_2 14 > > > > +#define MSCC_PHY_COMA_MODE 0x2000 /* input(1) / > > output(0) */ > > +#define MSCC_PHY_COMA_OUTPUT 0x1000 /* value to output > > */ > > + > > /* Extended Page 1 Registers */ > > #define MSCC_PHY_CU_MEDIA_CRC_VALID_CNT 18 > > #define VALID_CRC_CNT_CRC_MASK GENMASK(13, 0) > > diff --git a/drivers/net/phy/mscc/mscc_main.c > > b/drivers/net/phy/mscc/mscc_main.c > > index 03181542bcb7..29302ccf7e7b 100644 > > --- a/drivers/net/phy/mscc/mscc_main.c > > +++ b/drivers/net/phy/mscc/mscc_main.c > > @@ -1520,6 +1520,21 @@ static void vsc8584_get_base_addr(struct > > phy_device *phydev) > > vsc8531->addr = addr; > > } > > > > +static void vsc85xx_coma_mode_release(struct phy_device *phydev) > > +{ > > + /* The coma mode (pin or reg) provides an optional feature > > that > > + * may be used to control when the PHYs become active. > > + * Alternatively the COMA_MODE pin may be connected low > > + * so that the PHYs are fully active once out of reset. > > + */ > > + phy_unlock_mdio_bus(phydev); > > + /* Enable output (mode=0) and write zero to it */ > > + phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_GPIO, > > + MSCC_PHY_GPIO_CONTROL_2, > > + MSCC_PHY_COMA_MODE | MSCC_PHY_COMA_OUTPUT, > > 0); > > + phy_lock_mdio_bus(phydev); > > The temporary unlock is a little bit hacky. Better do: > vsc85xx_phy_write_page(MSCC_PHY_PAGE_EXTENDED_GPIO) > __phy_modify() > vsc85xx_phy_write_page(default page) > > Alternatively we could add __phy_modify_paged(). But this may not > be worth the effort for now. I will follow your suggestion. Thx -- Bjarni Jonasson Microchip > > > +} > > + > > static int vsc8584_config_init(struct phy_device *phydev) > > { > > struct vsc8531_private *vsc8531 = phydev->priv; > > @@ -2604,6 +2619,7 @@ static int vsc8514_config_init(struct > > phy_device *phydev) > > ret = vsc8514_config_host_serdes(phydev); > > if (ret) > > goto err; > > + vsc85xx_coma_mode_release(phydev); > > } > > > > phy_unlock_mdio_bus(phydev); > > > >
diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h index 9d8ee387739e..2b70ccd1b256 100644 --- a/drivers/net/phy/mscc/mscc.h +++ b/drivers/net/phy/mscc/mscc.h @@ -160,6 +160,9 @@ enum rgmii_clock_delay { #define MSCC_PHY_PAGE_TR 0x52b5 /* Token ring registers */ #define MSCC_PHY_GPIO_CONTROL_2 14 +#define MSCC_PHY_COMA_MODE 0x2000 /* input(1) / output(0) */ +#define MSCC_PHY_COMA_OUTPUT 0x1000 /* value to output */ + /* Extended Page 1 Registers */ #define MSCC_PHY_CU_MEDIA_CRC_VALID_CNT 18 #define VALID_CRC_CNT_CRC_MASK GENMASK(13, 0) diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index 03181542bcb7..29302ccf7e7b 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -1520,6 +1520,21 @@ static void vsc8584_get_base_addr(struct phy_device *phydev) vsc8531->addr = addr; } +static void vsc85xx_coma_mode_release(struct phy_device *phydev) +{ + /* The coma mode (pin or reg) provides an optional feature that + * may be used to control when the PHYs become active. + * Alternatively the COMA_MODE pin may be connected low + * so that the PHYs are fully active once out of reset. + */ + phy_unlock_mdio_bus(phydev); + /* Enable output (mode=0) and write zero to it */ + phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_GPIO, + MSCC_PHY_GPIO_CONTROL_2, + MSCC_PHY_COMA_MODE | MSCC_PHY_COMA_OUTPUT, 0); + phy_lock_mdio_bus(phydev); +} + static int vsc8584_config_init(struct phy_device *phydev) { struct vsc8531_private *vsc8531 = phydev->priv; @@ -2604,6 +2619,7 @@ static int vsc8514_config_init(struct phy_device *phydev) ret = vsc8514_config_host_serdes(phydev); if (ret) goto err; + vsc85xx_coma_mode_release(phydev); } phy_unlock_mdio_bus(phydev);