| Message ID | 98b53d15bb983c309f79acf9619b88ea4fbb8f14.1689215889.git.chenfeiyang@loongson.cn (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: phy/stmmac: Add Loongson platform support | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply, async |
On Thu, Jul 13, 2023 at 10:49:38AM +0800, Feiyang Chen wrote: > Add GNET support. Use the fix_mac_speed() callback to workaround > issues with the Loongson PHY. What are the issues? > +static void loongson_gnet_fix_speed(void *priv, unsigned int speed) > +{ > + struct net_device *ndev = (struct net_device *)(*(unsigned long *)priv); > + struct stmmac_priv *ptr = netdev_priv(ndev); > + > + if (speed == SPEED_1000) > + if (readl(ptr->ioaddr + MAC_CTRL_REG) & (1 << 15) /* PS */) > + phy_restart_aneg(ndev->phydev); This needs a comment at least, but if you explain what is actually FUBAR in this PHY, we might be able to tell you a better way to work around its problems. > +static int loongson_gnet_data(struct pci_dev *pdev, > + struct plat_stmmacenet_data *plat) > +{ > + common_default_data(pdev, plat); > + > + plat->mdio_bus_data->phy_mask = 0xfffffffb; > + > + plat->phy_addr = 2; > + plat->phy_interface = PHY_INTERFACE_MODE_GMII; You would normally get this from DT. Is the PHY integrated? Is it really using GMII, or would PHY_INTERFACE_MODE_INTERNAL be better? Andrew
On Thu, Jul 13, 2023 at 10:49:38AM +0800, Feiyang Chen wrote: > Add GNET support. Use the fix_mac_speed() callback to workaround > issues with the Loongson PHY. It would be good to document what those issue(s) are, and if they are a PHY issue, why they need to be resolved in a MAC driver rather than using the PHY driver's link_change_notify(). Thanks.
On Thu, Jul 13, 2023 at 12:07 PM Andrew Lunn <andrew@lunn.ch> wrote: > > On Thu, Jul 13, 2023 at 10:49:38AM +0800, Feiyang Chen wrote: > > Add GNET support. Use the fix_mac_speed() callback to workaround > > issues with the Loongson PHY. > > What are the issues? > Hi, Andrew, There is an issue with the synchronization between the network card and the PHY. In the case of gigabit operation, if the PHY's speed changes, the network card's speed remains unaffected. Hence, it is necessary to initiate a re-negotiation process with the PHY to align the link speeds properly. > > +static void loongson_gnet_fix_speed(void *priv, unsigned int speed) > > +{ > > + struct net_device *ndev = (struct net_device *)(*(unsigned long *)priv); > > + struct stmmac_priv *ptr = netdev_priv(ndev); > > + > > + if (speed == SPEED_1000) > > + if (readl(ptr->ioaddr + MAC_CTRL_REG) & (1 << 15) /* PS */) > > + phy_restart_aneg(ndev->phydev); > > This needs a comment at least, but if you explain what is actually > FUBAR in this PHY, we might be able to tell you a better way to work > around its problems. > > > +static int loongson_gnet_data(struct pci_dev *pdev, > > + struct plat_stmmacenet_data *plat) > > +{ > > + common_default_data(pdev, plat); > > + > > + plat->mdio_bus_data->phy_mask = 0xfffffffb; > > + > > + plat->phy_addr = 2; > > + plat->phy_interface = PHY_INTERFACE_MODE_GMII; > > You would normally get this from DT. Is the PHY integrated? Is it > really using GMII, or would PHY_INTERFACE_MODE_INTERNAL be better? > Yes, the PHY is integrated. I'll try to use PHY_INTERFACE_MODE_INTERNAL later. Thanks, Feiyang > Andrew
On Thu, Jul 13, 2023 at 4:06 PM Russell King (Oracle) <linux@armlinux.org.uk> wrote: > > On Thu, Jul 13, 2023 at 10:49:38AM +0800, Feiyang Chen wrote: > > Add GNET support. Use the fix_mac_speed() callback to workaround > > issues with the Loongson PHY. > > It would be good to document what those issue(s) are, and if they are a > PHY issue, why they need to be resolved in a MAC driver rather than > using the PHY driver's link_change_notify(). > Hi, Russell, There is an issue with the synchronization between the network card and the PHY. I believe that I am unable to access certain bits of the network card's registers within the PHY driver. Thanks, Feiyang > Thanks. > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> Hi, Russell, > > There is an issue with the synchronization between the network card > and the PHY. Details. We need the details in order to tell you the best workaround for your broken hardware. I believe that I am unable to access certain bits of the > network card's registers within the PHY driver. The PHY driver and the MAC driver should never access each others registers. Andrew
On Fri, Jul 14, 2023 at 10:24:37AM +0800, Feiyang Chen wrote: > On Thu, Jul 13, 2023 at 12:07 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > On Thu, Jul 13, 2023 at 10:49:38AM +0800, Feiyang Chen wrote: > > > Add GNET support. Use the fix_mac_speed() callback to workaround > > > issues with the Loongson PHY. > > > > What are the issues? > > > > Hi, Andrew, > > There is an issue with the synchronization between the network card > and the PHY. In the case of gigabit operation, if the PHY's speed > changes, the network card's speed remains unaffected. Hence, it is > necessary to initiate a re-negotiation process with the PHY to align > the link speeds properly. When the line side speed changes, the PHY will first report link down via the adjust_link callback in the MAC driver. Once the PHY has determined the new speed, the adjust_link callback will be called again with the new speed and pause parameters. So what is actually going wrong? Andrew
On Fri, Jul 14, 2023 at 10:24:37AM +0800, Feiyang Chen wrote: > On Thu, Jul 13, 2023 at 12:07 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > On Thu, Jul 13, 2023 at 10:49:38AM +0800, Feiyang Chen wrote: > > > Add GNET support. Use the fix_mac_speed() callback to workaround > > > issues with the Loongson PHY. > > > > What are the issues? > > > > Hi, Andrew, > > There is an issue with the synchronization between the network card > and the PHY. In the case of gigabit operation, if the PHY's speed > changes, the network card's speed remains unaffected. Hence, it is > necessary to initiate a re-negotiation process with the PHY to align > the link speeds properly. Please could you explain a bit more what happens when "the PHY's speed changes". Are you suggesting that: You have a connection where the media side has negotiated 1G speed. The gigabit partner is disconnected, so the link goes down, and is then replaced by a partner only capable of 100M. The link comes back up at 100M, but the network card continues trying to operate at 1G? Thanks.
On Fri, Jul 14, 2023 at 06:26:14AM +0200, Andrew Lunn wrote: > On Fri, Jul 14, 2023 at 10:24:37AM +0800, Feiyang Chen wrote: > > On Thu, Jul 13, 2023 at 12:07 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > On Thu, Jul 13, 2023 at 10:49:38AM +0800, Feiyang Chen wrote: > > > > Add GNET support. Use the fix_mac_speed() callback to workaround > > > > issues with the Loongson PHY. > > > > > > What are the issues? > > > > > > > Hi, Andrew, > > > > There is an issue with the synchronization between the network card > > and the PHY. In the case of gigabit operation, if the PHY's speed > > changes, the network card's speed remains unaffected. Hence, it is > > necessary to initiate a re-negotiation process with the PHY to align > > the link speeds properly. > > When the line side speed changes, the PHY will first report link down > via the adjust_link callback in the MAC driver. Once the PHY has > determined the new speed, the adjust_link callback will be called > again with the new speed and pause parameters. Or in the case of stmmac, phylink is involved, so .mac_link_down will get called and then .mac_link_up with the new parameters.
On Fri, Jul 14, 2023 at 4:45 PM Russell King (Oracle) <linux@armlinux.org.uk> wrote: > > On Fri, Jul 14, 2023 at 10:24:37AM +0800, Feiyang Chen wrote: > > On Thu, Jul 13, 2023 at 12:07 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > On Thu, Jul 13, 2023 at 10:49:38AM +0800, Feiyang Chen wrote: > > > > Add GNET support. Use the fix_mac_speed() callback to workaround > > > > issues with the Loongson PHY. > > > > > > What are the issues? > > > > > > > Hi, Andrew, > > > > There is an issue with the synchronization between the network card > > and the PHY. In the case of gigabit operation, if the PHY's speed > > changes, the network card's speed remains unaffected. Hence, it is > > necessary to initiate a re-negotiation process with the PHY to align > > the link speeds properly. > > Please could you explain a bit more what happens when "the PHY's speed > changes". Are you suggesting that: > > You have a connection where the media side has negotiated 1G speed. > The gigabit partner is disconnected, so the link goes down, and is then > replaced by a partner only capable of 100M. The link comes back up at > 100M, but the network card continues trying to operate at 1G? > Hi, Russell, Andrew, This bug shows up in the following way: when the speed is set to 1000M, PHY is set up correctly and its status is normal. However, the controller and PHY don't work well together, causing the controller to fail in establishing a gigabit connection, which leads to a network disruption. So, we need to use this bit to check if the controller's status is correct and reset PHY if necessary. if (readl(ptr->ioaddr + MAC_CTRL_REG) & (1 << 15) /* PS */) The troublesome issue is that we have to check this bit in stmmac. As a result, we are forced to place phy_restart_aneg() there. Thanks, Feiyang > Thanks. > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig index 5f5a997f21f3..8c25d5616941 100644 --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig @@ -279,6 +279,7 @@ config DWMAC_LOONGSON default MACH_LOONGSON64 depends on STMMAC_ETH && PCI depends on COMMON_CLK + select LOONGSON_PHY help This selects the LOONGSON PCI bus support for the stmmac driver, Support for ethernet controller on Loongson-2K1000 SoC and LS7A1000 bridge. diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c index 3732f90a79c7..c840d9d1d72a 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c @@ -69,6 +69,38 @@ static struct stmmac_pci_info loongson_gmac_pci_info = { .setup = loongson_gmac_data, }; +static void loongson_gnet_fix_speed(void *priv, unsigned int speed) +{ + struct net_device *ndev = (struct net_device *)(*(unsigned long *)priv); + struct stmmac_priv *ptr = netdev_priv(ndev); + + if (speed == SPEED_1000) + if (readl(ptr->ioaddr + MAC_CTRL_REG) & (1 << 15) /* PS */) + phy_restart_aneg(ndev->phydev); +} + +static int loongson_gnet_data(struct pci_dev *pdev, + struct plat_stmmacenet_data *plat) +{ + common_default_data(pdev, plat); + + plat->mdio_bus_data->phy_mask = 0xfffffffb; + + plat->phy_addr = 2; + plat->phy_interface = PHY_INTERFACE_MODE_GMII; + + plat->fix_mac_speed = loongson_gnet_fix_speed; + + /* Get netdev pointer address */ + plat->bsp_priv = &(pdev->dev.driver_data); + + return 0; +} + +static struct stmmac_pci_info loongson_gnet_pci_info = { + .setup = loongson_gnet_data, +}; + static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id *id) { @@ -296,9 +328,11 @@ static SIMPLE_DEV_PM_OPS(loongson_dwmac_pm_ops, loongson_dwmac_suspend, loongson_dwmac_resume); #define PCI_DEVICE_ID_LOONGSON_GMAC 0x7a03 +#define PCI_DEVICE_ID_LOONGSON_GNET 0x7a13 static const struct pci_device_id loongson_dwmac_id_table[] = { { PCI_DEVICE_DATA(LOONGSON, GMAC, &loongson_gmac_pci_info) }, + { PCI_DEVICE_DATA(LOONGSON, GNET, &loongson_gnet_pci_info) }, {} }; MODULE_DEVICE_TABLE(pci, loongson_dwmac_id_table);
Add GNET support. Use the fix_mac_speed() callback to workaround issues with the Loongson PHY. Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn> --- drivers/net/ethernet/stmicro/stmmac/Kconfig | 1 + .../ethernet/stmicro/stmmac/dwmac-loongson.c | 34 +++++++++++++++++++ 2 files changed, 35 insertions(+)