| Message ID | 20210212140643.23436-3-bjarni.jonasson@microchip.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v1,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 |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 2 maintainers not CCed: kavyasree.kotagiri@microchip.com quentin.schulz@bootlin.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 | fail | Errors and warnings before: 84 this patch: 84 |
| 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, 25 lines checked |
| netdev/build_allmodconfig_warn | fail | Errors and warnings before: 84 this patch: 84 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Fri, Feb 12, 2021 at 03:06:43PM +0100, 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.") > --- > drivers/net/phy/mscc/mscc_main.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c > index 7546d9cc3abd..0600b592618b 100644 > --- a/drivers/net/phy/mscc/mscc_main.c > +++ b/drivers/net/phy/mscc/mscc_main.c > @@ -1418,6 +1418,18 @@ 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_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_EXTENDED_GPIO); > + __phy_write(phydev, MSCC_PHY_GPIO_CONTROL_2, 0x0600); > + __phy_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD); Can you please do: phy_write_paged(phydev, MSCC_PHY_PAGE_EXTENDED_GPIO, MSCC_PHY_GPIO_CONTROL_2, 0x0600); And can you please provide some definitions for what 0x0600 is? My reference manual says that: Bit 13: COMA_MODE output enable (active low) Bit 12: COMA_MODE output data Bit 11: COMA_MODE input data Bit 10: Reserved Bit 9: Tri-state enable for LEDs 0x600 is BIT(10) | BIT(9). But BIT(10) is reserved. Sure this is correct? > +} > + > static int vsc8584_config_init(struct phy_device *phydev) > { > struct vsc8531_private *vsc8531 = phydev->priv; > @@ -2610,6 +2622,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); > -- > 2.17.1 >
On Fri, 2021-02-12 at 16:32 +0000, Vladimir Oltean wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > On Fri, Feb 12, 2021 at 03:06:43PM +0100, 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.") > > --- > > drivers/net/phy/mscc/mscc_main.c | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/drivers/net/phy/mscc/mscc_main.c > > b/drivers/net/phy/mscc/mscc_main.c > > index 7546d9cc3abd..0600b592618b 100644 > > --- a/drivers/net/phy/mscc/mscc_main.c > > +++ b/drivers/net/phy/mscc/mscc_main.c > > @@ -1418,6 +1418,18 @@ 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_write(phydev, MSCC_EXT_PAGE_ACCESS, > > MSCC_PHY_PAGE_EXTENDED_GPIO); > > + __phy_write(phydev, MSCC_PHY_GPIO_CONTROL_2, 0x0600); > > + __phy_write(phydev, MSCC_EXT_PAGE_ACCESS, > > MSCC_PHY_PAGE_STANDARD); > > Can you please do: > phy_write_paged(phydev, MSCC_PHY_PAGE_EXTENDED_GPIO, > MSCC_PHY_GPIO_CONTROL_2, 0x0600); > > And can you please provide some definitions for what 0x0600 is? > My reference manual says that: > > Bit 13: > COMA_MODE output enable (active low) > Bit 12: > COMA_MODE output data > Bit 11: > COMA_MODE input data > Bit 10: > Reserved > Bit 9: > Tri-state enable for LEDs > > 0x600 is BIT(10) | BIT(9). But BIT(10) is reserved. Sure this is > correct? I can see this is unclear. The code is actualy writing zero to bit 12 and 13. Bit 9 and 10 are not interesting in this context. I will change it to use phy_modify_paged() bit 12 and 13. > > > +} > > + > > static int vsc8584_config_init(struct phy_device *phydev) > > { > > struct vsc8531_private *vsc8531 = phydev->priv; > > @@ -2610,6 +2622,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); > > -- > > 2.17.1 > >
diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index 7546d9cc3abd..0600b592618b 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -1418,6 +1418,18 @@ 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_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_EXTENDED_GPIO); + __phy_write(phydev, MSCC_PHY_GPIO_CONTROL_2, 0x0600); + __phy_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD); +} + static int vsc8584_config_init(struct phy_device *phydev) { struct vsc8531_private *vsc8531 = phydev->priv; @@ -2610,6 +2622,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);