| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <72hx6yfvbxiuvkunzu2tvn6glum5rjrzqaxsswml2fe6j3537w@ahtfn7q64ffe>
Date: Thu, 25 Jan 2024 21:38:30 +0300
From: Serge Semin <fancer.lancer@...il.com>
To: Yanteng Si <siyanteng@...ngson.cn>
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 v7 7/9] net: stmmac: dwmac-loongson: Add GNET
support
On Thu, Jan 25, 2024 at 04:36:39PM +0800, Yanteng Si wrote:
>
> 在 2024/1/24 21:51, Serge Semin 写道:
> > On Wed, Jan 24, 2024 at 05:21:03PM +0800, Yanteng Si wrote:
> > > 在 2024/1/1 15:27, Yanteng Si 写道:
> > > > 在 2023/12/21 10:34, Serge Semin 写道:
> > > > > On Tue, Dec 19, 2023 at 10:26:47PM +0800, Yanteng Si wrote:
> > > > > > Add Loongson GNET (GMAC with PHY) support. Current GNET does not
> > > > > > support
> > > > > > half duplex mode, and GNET on LS7A only supports ANE when speed
> > > > > > is set to
> > > > > > 1000M.
> > > > > >
> > > > > > 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 | 79
> > > > > > +++++++++++++++++++
> > > > > > .../ethernet/stmicro/stmmac/stmmac_ethtool.c | 6 ++
> > > > > > include/linux/stmmac.h | 2 +
> > > > > > 3 files changed, 87 insertions(+)
> > > > > >
> > > > > > diff --git
> > > > > > a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > > > > > b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > > > > > index 2c08d5495214..9e4953c7e4e0 100644
> > > > > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > > > > > @@ -168,6 +168,83 @@ 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 net_device *ndev = dev_get_drvdata(priv);
> > > > > > + struct stmmac_priv *ptr = netdev_priv(ndev);
> > > > > > +
> > > > > > + /* The controller and PHY don't work well together.
> > > > > > + * 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);
> > > > > {} around the outer if please.
> > > > OK.
> > > > > > +}
> > > > > > +
> > > > > > +static int loongson_gnet_data(struct pci_dev *pdev,
> > > > > > + struct plat_stmmacenet_data *plat)
> > > > > > +{
> > > > > > + loongson_default_data(pdev, plat);
> > > > > > +
> > > > > > + plat->multicast_filter_bins = 256;
> > > > > > +
> > > > > > + plat->mdio_bus_data->phy_mask = 0xfffffffb;
> > > > > ~BIT(2)?
> > > > I still need to confirm, please allow me to get back to you later.
> > > Yes, that's fine.
>
> Oops! A warning will be output:
>
> drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c: In function
> 'loongson_gnet_data':
> drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c:463:41: warning:
> conversion from
>
> 'long unsigned int' to 'unsigned int' changes value from
> '18446744073709551611' to '4294967291' [-Woverflow]
> 463 | plat->mdio_bus_data->phy_mask = ~BIT(2);
> | ^
>
> Unfortunately, we don't have an unsigned int macro for BIT(nr).
Then the alternative ~(1 << 2) would be still more readable then the
open-coded literal like 0xfffffffb. What would be even better than
that:
#define LOONGSON_GNET_PHY_ADDR 0x2
...
plat->mdio_bus_data->phy_mask = ~(1 << LOONGSON_GNET_PHY_ADDR);
...
-Serge(y)
>
>
> Thanks,
>
> Yanteng
>
> >
> > -Serge(y)
> >
> > >
> > > Thanks,
> > >
> > > Yanteng
> > >
> > >
> > > > > > +
> > > > > > + plat->phy_addr = 2;
> > > > > > + 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;
> > > > > > +
> > > > > > + plat->clk_ref_rate = 125000000;
> > > > > > + plat->clk_ptp_rate = 125000000;
> > > > > > +
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > +static int loongson_gnet_config(struct pci_dev *pdev,
> > > > > > + struct plat_stmmacenet_data *plat,
> > > > > > + struct stmmac_resources *res,
> > > > > > + struct device_node *np)
> > > > > > +{
> > > > > > + int ret;
> > > > > > + u32 version = readl(res->addr + GMAC_VERSION);
> > > > > > +
> > > > > > + switch (version & 0xff) {
> > > > > > + case DWLGMAC_CORE_1_00:
> > > > > > + ret = loongson_dwmac_config_multi_msi(pdev, plat, res, np, 8);
> > > > > > + break;
> > > > > > + default:
> > > > > > + ret = loongson_dwmac_config_legacy(pdev, plat, res, np);
> > > > > Hm, do you have two versions of Loongson GNET? What does the second
> > > > Yes.
> > > > > one contain in the GMAC_VERSION register then? Can't you distinguish
> > > > > them by the PCI IDs (device, subsystem, revision)?
> > > > I'm afraid that's not possible.
> > > >
> > > > Because they have the same pci id and revision.
> > > >
> > > >
> > > > Thanks,
> > > >
> > > > Yanteng
> > > >
> > > > > -Serge(y)
> > > > >
> > > > > > + break;
> > > > > > + }
> > > > > > +
> > > > > > + switch (pdev->revision) {
> > > > > > + case 0x00:
> > > > > > + plat->flags |=
> > > > > > + FIELD_PREP(STMMAC_FLAG_DISABLE_HALF_DUPLEX, 1) |
> > > > > > + FIELD_PREP(STMMAC_FLAG_DISABLE_FORCE_1000, 1);
> > > > > > + break;
> > > > > > + case 0x01:
> > > > > > + plat->flags |=
> > > > > > + FIELD_PREP(STMMAC_FLAG_DISABLE_HALF_DUPLEX, 1);
> > > > > > + break;
> > > > > > + default:
> > > > > > + break;
> > > > > > + }
> > > > > > +
> > > > > > + return ret;
> > > > > > +}
> > > > > > +
> > > > > > +static struct stmmac_pci_info loongson_gnet_pci_info = {
> > > > > > + .setup = loongson_gnet_data,
> > > > > > + .config = loongson_gnet_config,
> > > > > > +};
> > > > > > +
> > > > > > static int loongson_dwmac_probe(struct pci_dev *pdev,
> > > > > > const struct pci_device_id *id)
> > > > > > {
> > > > > > @@ -318,9 +395,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);
> > > > > > diff --git
> > > > > > a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> > > > > > b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> > > > > > index 8105ce47c6ad..d6939eb9a0d8 100644
> > > > > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> > > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> > > > > > @@ -420,6 +420,12 @@ stmmac_ethtool_set_link_ksettings(struct
> > > > > > net_device *dev,
> > > > > > return 0;
> > > > > > }
> > > > > > + if (FIELD_GET(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 f07f79d50b06..067030cdb60f 100644
> > > > > > --- a/include/linux/stmmac.h
> > > > > > +++ b/include/linux/stmmac.h
> > > > > > @@ -222,6 +222,8 @@ struct dwmac4_addrs {
> > > > > > #define STMMAC_FLAG_EN_TX_LPI_CLOCKGATING BIT(11)
> > > > > > #define STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY BIT(12)
> > > > > > #define STMMAC_FLAG_HAS_LGMAC BIT(13)
> > > > > > +#define STMMAC_FLAG_DISABLE_HALF_DUPLEX BIT(14)
> > > > > > +#define STMMAC_FLAG_DISABLE_FORCE_1000 BIT(15)
> > > > > > struct plat_stmmacenet_data {
> > > > > > int bus_id;
> > > > > > --
> > > > > > 2.31.4
> > > > > >
>
Powered by blists - more mailing lists