[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <2b6459cf-7be3-4e69-aff0-8fc463eace64@loongson.cn>
Date: Thu, 14 Mar 2024 21:18:15 +0800
From: Yanteng Si <siyanteng@...ngson.cn>
To: Serge Semin <fancer.lancer@...il.com>
Cc: andrew@...n.ch, hkallweit1@...il.com, peppe.cavallaro@...com,
alexandre.torgue@...s.st.com, joabreu@...opsys.com, Jose.Abreu@...opsys.com,
chenhuacai@...ngson.cn, linux@...linux.org.uk, guyinggang@...ngson.cn,
netdev@...r.kernel.org, chris.chenfeiyang@...il.com
Subject: Re: [PATCH net-next v8 08/11] net: stmmac: dwmac-loongson: Fix MAC
speed for GNET
在 2024/3/14 17:43, Yanteng Si 写道:
> 在 2024/2/6 05:55, Serge Semin 写道:
> > On Tue, Jan 30, 2024 at 04:48:20PM +0800, Yanteng Si wrote:
> >> Current GNET on LS7A only supports ANE when speed is
> >> set to 1000M.
> > If so you need to merge it into the patch
> > [PATCH net-next v8 06/11] net: stmmac: dwmac-loongson: Add GNET support
>> Current GNET on LS7A only supports ANE when speed is
>> set to 1000M.
>If so you need to merge it into the patch
>[PATCH net-next v8 06/11] net: stmmac: dwmac-loongson: Add GNET support
OK.
> >
> >> Signed-off-by: Yanteng Si<siyanteng@...ngson.cn>
> >> Signed-off-by: Feiyang Chen<chenfeiyang@...ngson.cn>
> >> Signed-off-by: Yinggang Gu<guyinggang@...ngson.cn>
> >> ---
> >> .../ethernet/stmicro/stmmac/dwmac-loongson.c | 19 +++++++++++++++++++
> >> .../ethernet/stmicro/stmmac/stmmac_ethtool.c | 6 ++++++
> >> include/linux/stmmac.h | 1 +
> >> 3 files changed, 26 insertions(+)
> >>
> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> >> index 60d0a122d7c9..264c4c198d5a 100644
> >> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> >> @@ -344,6 +344,21 @@ static struct stmmac_pci_info loongson_gmac_pci_info = {
> >> .config = loongson_gmac_config,
> >> };
> >>
> >> +static void loongson_gnet_fix_speed(void *priv, unsigned int speed, unsigned int mode)
> >> +{
> >> + struct loongson_data *ld = (struct loongson_data *)priv;
> >> + struct net_device *ndev = dev_get_drvdata(ld->dev);
> >> + struct stmmac_priv *ptr = netdev_priv(ndev);
> >> +
> >> + /* The controller and PHY don't work well together.
> > So there _is_ a PHY. What is the interface between MAC and PHY then?
> >
> > GMAC only has a MAC chip inside the chip and needs an external PHY chip; GNET
> > has the PHY chip inside the chip.
> >> + * We need to use the PS bit to check if the controller's status
> >> + * is correct and reset PHY if necessary.
> >> + */
> >> + if (speed == SPEED_1000)
> >> + if (readl(ptr->ioaddr + MAC_CTRL_REG) & (1 << 15) /* PS */)
> >> + phy_restart_aneg(ndev->phydev);
> > 1. Please add curly braces for the outer if-statement.
> OK,
> > 2. MAC_CTRL_REG.15 is defined by the GMAC_CONTROL_PS macro.
>
> OK.
>
> if(speed==SPEED_1000){
> /*MAC_CTRL_REG.15 is defined by the GMAC_CONTROL_PS macro.*/
> if(readl(ptr->ioaddr+MAC_CTRL_REG) &(1<<15))
> phy_restart_aneg(ndev->phydev);
> }
>
> > 3. How is the AN-restart helps? PHY-reset is done in
> > stmmac_init_phy()->phylink_connect_phy()->... a bit earlier than
> > this is called in the framework of the stmmac_mac_link_up() callback.
> > Wouldn't that restart AN too?
>
> Due to a bug in the chip's internal PHY, the network is still not working after
> the first self-negotiation, and it needs to be self-negotiated again.
>
> >
> >> +}
> >> +
> >> static struct mac_device_info *loongson_setup(void *apriv)
> >> {
> >> struct stmmac_priv *priv = apriv;
> >> @@ -401,6 +416,7 @@ static int loongson_gnet_data(struct pci_dev *pdev,
> >> plat->phy_interface = PHY_INTERFACE_MODE_INTERNAL;
> >>
> >> plat->bsp_priv = &pdev->dev;
> >> + plat->fix_mac_speed = loongson_gnet_fix_speed;
> >>
> >> plat->dma_cfg->pbl = 32;
> >> plat->dma_cfg->pblx8 = true;
> >> @@ -416,6 +432,9 @@ static int loongson_gnet_config(struct pci_dev *pdev,
> >> struct stmmac_resources *res,
> >> struct device_node *np)
> >> {
> >> + if (pdev->revision == 0x00 || pdev->revision == 0x01)
> >> + plat->flags |= STMMAC_FLAG_DISABLE_FORCE_1000;
> >> +
> > This should be in the patch
> > [PATCH net-next v8 06/11] net: stmmac: dwmac-loongson: Add GNET support
> OK.
> >
> >> return 0;
> >> }
> >>
> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> >> index 42d27b97dd1d..31068fbc23c9 100644
> >> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> >> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> >> @@ -422,6 +422,12 @@ stmmac_ethtool_set_link_ksettings(struct net_device *dev,
> >> return 0;
> >> }
> >>
> >> + if (FIELD_GET(STMMAC_FLAG_DISABLE_FORCE_1000, priv->plat->flags)) {
> > FIELD_GET()?
>
> OK,
>
> if (STMMAC_FLAG_DISABLE_FORCE_1000 & priv->plat->flags) {
>
> >
> >> + if (cmd->base.speed == SPEED_1000 &&
> >> + cmd->base.autoneg != AUTONEG_ENABLE)
> >> + return -EOPNOTSUPP;
> >> + }
> >> +
> >> return phylink_ethtool_ksettings_set(priv->phylink, cmd);
> >> }
> >>
> >> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> >> index dee5ad6e48c5..2810361e4048 100644
> >> --- a/include/linux/stmmac.h
> >> +++ b/include/linux/stmmac.h
> >> @@ -221,6 +221,7 @@ struct dwmac4_addrs {
> >> #define STMMAC_FLAG_RX_CLK_RUNS_IN_LPI BIT(10)
> >> #define STMMAC_FLAG_EN_TX_LPI_CLOCKGATING BIT(11)
> >> #define STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY BIT(12)
> >> +#define STMMAC_FLAG_DISABLE_FORCE_1000 BIT(13)
> > Detach the change introducing the STMMAC_FLAG_DISABLE_FORCE_1000 flag
> > into a separate patch a place it before
> > [PATCH net-next v8 06/11] net: stmmac: dwmac-loongson: Add GNET support
> > as a pre-requisite/preparation patch.
> > Don't forget a _detailed_ description of why it's necessary, what is
> > wrong with GNET so 1G speed doesn't work without AN.
>
> OK.
>
>
> Thanks,
>
> Yanteng
>
> >
> > -Serge(y)
> >
> >>
> >> struct plat_stmmacenet_data {
> >> int bus_id;
> >> --
> >> 2.31.4
> >>
Powered by blists - more mailing lists