| Message ID | 20221226071425.3895915-4-yoshihiro.shimoda.uh@renesas.com (mailing list archive) |
|---|---|
| State | Deferred |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ethernet: renesas: rswitch: Modify initialization for SERDES and PHY | expand |
On Mon, Dec 26, 2022 at 04:14:25PM +0900, Yoshihiro Shimoda wrote: > Some Ethernet PHYs (like marvell10g) will decide the host interface > mode by the media-side speed. So, the rswitch driver needs to > initialize one of the Ethernet SERDES (r8a779f0-eth-serdes) ports > after linked the Ethernet PHY up. The r8a779f0-eth-serdes driver has > .init() for initializing all ports and .power_on() for initializing > each port. So, add phy_power_{on,off} calling for it. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- > drivers/net/ethernet/renesas/rswitch.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/net/ethernet/renesas/rswitch.c b/drivers/net/ethernet/renesas/rswitch.c > index ca79ee168206..2f335c95f5a8 100644 > --- a/drivers/net/ethernet/renesas/rswitch.c > +++ b/drivers/net/ethernet/renesas/rswitch.c > @@ -1180,6 +1180,10 @@ static void rswitch_mac_link_down(struct phylink_config *config, > unsigned int mode, > phy_interface_t interface) > { > + struct net_device *ndev = to_net_dev(config->dev); > + struct rswitch_device *rdev = netdev_priv(ndev); > + > + phy_power_off(rdev->serdes); > } > > static void rswitch_mac_link_up(struct phylink_config *config, > @@ -1187,7 +1191,11 @@ static void rswitch_mac_link_up(struct phylink_config *config, > phy_interface_t interface, int speed, > int duplex, bool tx_pause, bool rx_pause) > { > + struct net_device *ndev = to_net_dev(config->dev); > + struct rswitch_device *rdev = netdev_priv(ndev); > + > /* Current hardware cannot change speed at runtime */ > + phy_power_on(rdev->serdes); > } > > static const struct phylink_mac_ops rswitch_phylink_ops = { This looks to me like it will break anyone using an in-band link, where the link status comes from the PCS behind the series that you're now powering down and up.
Hello Russell, > From: Russell King, Sent: Tuesday, January 3, 2023 6:56 PM > > On Mon, Dec 26, 2022 at 04:14:25PM +0900, Yoshihiro Shimoda wrote: > > Some Ethernet PHYs (like marvell10g) will decide the host interface > > mode by the media-side speed. So, the rswitch driver needs to > > initialize one of the Ethernet SERDES (r8a779f0-eth-serdes) ports > > after linked the Ethernet PHY up. The r8a779f0-eth-serdes driver has > > .init() for initializing all ports and .power_on() for initializing > > each port. So, add phy_power_{on,off} calling for it. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > --- > > drivers/net/ethernet/renesas/rswitch.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/net/ethernet/renesas/rswitch.c b/drivers/net/ethernet/renesas/rswitch.c > > index ca79ee168206..2f335c95f5a8 100644 > > --- a/drivers/net/ethernet/renesas/rswitch.c > > +++ b/drivers/net/ethernet/renesas/rswitch.c > > @@ -1180,6 +1180,10 @@ static void rswitch_mac_link_down(struct phylink_config *config, > > unsigned int mode, > > phy_interface_t interface) > > { > > + struct net_device *ndev = to_net_dev(config->dev); > > + struct rswitch_device *rdev = netdev_priv(ndev); > > + > > + phy_power_off(rdev->serdes); > > } > > > > static void rswitch_mac_link_up(struct phylink_config *config, > > @@ -1187,7 +1191,11 @@ static void rswitch_mac_link_up(struct phylink_config *config, > > phy_interface_t interface, int speed, > > int duplex, bool tx_pause, bool rx_pause) > > { > > + struct net_device *ndev = to_net_dev(config->dev); > > + struct rswitch_device *rdev = netdev_priv(ndev); > > + > > /* Current hardware cannot change speed at runtime */ > > + phy_power_on(rdev->serdes); > > } > > > > static const struct phylink_mac_ops rswitch_phylink_ops = { > > This looks to me like it will break anyone using an in-band link, > where the link status comes from the PCS behind the series that > you're now powering down and up. Thank you for your review! To be honest, I didn't know about "an in-band link". So, I studied a little, and then I might understand about your comments. However, I still don't understand whether this hardware (rswitch) can support the "in-band link" or not. This seems to need a special hardware feature for "in-band link" and only software cannot support it, IIUC. # Unfortunately, this hardware's datasheet cannot be in public for now though... Anyway, I'll add a condition which this driver cannot support MLO_AN_INBAND to avoid a problem by someone for now. Best regards, Yoshihiro Shimoda
diff --git a/drivers/net/ethernet/renesas/rswitch.c b/drivers/net/ethernet/renesas/rswitch.c index ca79ee168206..2f335c95f5a8 100644 --- a/drivers/net/ethernet/renesas/rswitch.c +++ b/drivers/net/ethernet/renesas/rswitch.c @@ -1180,6 +1180,10 @@ static void rswitch_mac_link_down(struct phylink_config *config, unsigned int mode, phy_interface_t interface) { + struct net_device *ndev = to_net_dev(config->dev); + struct rswitch_device *rdev = netdev_priv(ndev); + + phy_power_off(rdev->serdes); } static void rswitch_mac_link_up(struct phylink_config *config, @@ -1187,7 +1191,11 @@ static void rswitch_mac_link_up(struct phylink_config *config, phy_interface_t interface, int speed, int duplex, bool tx_pause, bool rx_pause) { + struct net_device *ndev = to_net_dev(config->dev); + struct rswitch_device *rdev = netdev_priv(ndev); + /* Current hardware cannot change speed at runtime */ + phy_power_on(rdev->serdes); } static const struct phylink_mac_ops rswitch_phylink_ops = {
Some Ethernet PHYs (like marvell10g) will decide the host interface mode by the media-side speed. So, the rswitch driver needs to initialize one of the Ethernet SERDES (r8a779f0-eth-serdes) ports after linked the Ethernet PHY up. The r8a779f0-eth-serdes driver has .init() for initializing all ports and .power_on() for initializing each port. So, add phy_power_{on,off} calling for it. Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- drivers/net/ethernet/renesas/rswitch.c | 8 ++++++++ 1 file changed, 8 insertions(+)