Messages in this thread Patch in this message | | | From | <> | Subject | Re: [PATCH net v1 3/3] net: phy: mscc: coma mode disabled for VSC8514 | Date | Mon, 15 Feb 2021 09:36:05 +0000 |
| |
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 > >
| |