[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <442kw74awamryq5kwxnznaxv2kszlbwpzt4fzmw67ijqx4v3or@zypwow6rmuwb>
Date: Wed, 20 Mar 2024 15:18:20 +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 v8 06/11] net: stmmac: dwmac-loongson: Add GNET
support
On Wed, Mar 20, 2024 at 06:23:14PM +0800, Yanteng Si wrote:
>
> 在 2024/3/19 23:03, Serge Semin 写道:
> > On Thu, Mar 14, 2024 at 09:12:49PM +0800, Yanteng Si wrote:
> > > 在 2024/3/14 16:27, Yanteng Si 写道:
> > > > 在 2024/2/6 04:58, Serge Semin 写道:
> > > > > On Tue, Jan 30, 2024 at 04:48:18PM +0800, Yanteng Si wrote:
> > > > > > Add Loongson GNET (GMAC with PHY) support, override
> > > > > > stmmac_priv.synopsys_id with 0x37.
> > > > > Please add more details of all the device capabilities: supported
> > > > > speeds, duplexness, IP-core version, DMA-descriptors type
> > > > > (normal/enhanced), MTL Tx/Rx FIFO size, Perfect and Hash-based MAC
> > > > > Filter tables size, L3/L4 filters availability, VLAN hash table
> > > > > filter, PHY-interface (GMII, RGMII, etc), EEE support,
> > > > > AV-feature/Multi-channels support, IEEE 1588 Timestamp support, Magic
> > > > > Frame support, Remote Wake-up support, IP Checksum, Tx/Rx TCP/IP
> > > > > Checksum, Mac Management Counters (MMC), SMA/MDIO interface,
> > > > The gnet (2k2000) of 0x10 supports full-duplex and half-duplex at 1000/100/10M.
> > > > The gnet of 0x37 (i.e. the gnet of 7a2000) supports 1000/100/10M full duplex.
> > > >
> > > > The gnet with 0x10 has 8 DMA channels, except for channel 0, which does not
> > > > support sending hardware checksums.
> > > >
> > > > Supported AV features are Qav, Qat, and Qas, and other features should be
> > > > consistent with the 3.73 IP.
> > Just list all of these features in the commit message referring to the
> > respective controller. Like this:
> > "There are two types of them Loongson GNET controllers:
> > Loongson 2k2000 GNET and the rest of the Loongson GNETs (like
> > presented on the 7a2000 SoC). All of them of the DW GMAC 3.73a
> > IP-core with the next features:
> > Speeds, DMA-descriptors type (normal/enhanced), MTL Tx/Rx FIFO size,
> > Perfect and Hash-based MAC Filter tables size, L3/L4 filters availability,
> > VLAN hash table filter, PHY-interface (GMII, RGMII, etc), EEE support,
> > IEEE 1588 Timestamp support, Magic Frame support, Remote Wake-up support,
> > IP Checksum, Tx/Rx TCP/IP Checksum, Mac Management Counters (MMC),
> > SMA/MDIO interface.
> >
> > The difference is that the Loongson 2k2000 GNET controller supports 8
> > DMA-channels, AV features (Qav, Qat, and Qas) and half-duplex link,
> > meanwhile the rest of the GNETs don't have these capabilities
> > available."
> OK, Thanks!
Please don't forget to replace the next text:
"... Speeds, DMA-descriptors type (normal/enhanced), MTL Tx/Rx FIFO size,
Perfect and Hash-based MAC Filter tables size, L3/L4 filters availability,
VLAN hash table filter, PHY-interface (GMII, RGMII, etc), EEE support,
IEEE 1588 Timestamp support, Magic Frame support, Remote Wake-up support,
IP Checksum, Tx/Rx TCP/IP Checksum, Mac Management Counters (MMC),
SMA/MDIO interface."
with the actual device capabilities, like:
Speeds - 10/100/1000Mbps,
DMA-descriptors type - Normal DMA descriptors or Enhanced DMA descriptors,
MTL Tx/Rx FIFO size - X Kb MTL Tx FIFO, Y Kb MTL Rx FIFO,
Perfect and Hash-based MAC Filter tables size - X MAC addresses,
X-entries hash address filter,
etc
-Serge(y)
> >
> > > > > > 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 | 44 +++++++++++++++++++
> > > > > > 1 file changed, 44 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > > > > > index 3b3578318cc1..584f7322bd3e 100644
> > > > > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > > > > > @@ -318,6 +318,8 @@ static struct mac_device_info *loongson_setup(void *apriv)
> > > > > > if (!mac)
> > > > > > return NULL;
> > > > > > >> + priv->synopsys_id = 0x37; /*Overwrite custom IP*/
> > > > > > +
> > > > > Please add a more descriptive comment _above_ the subjected line. In
> > > > > particular note why the override is needed, what is the real DW GMAC
> > > > > IP-core version and what is the original value the statement above
> > > > > overrides.
> > > > The IP-core version of the gnet device on the loongson 2k2000 is 0x10, which is
> > > > a custom IP.
> > > >
> > > > Compared to 0x37, we have split some of the dma registers into two (tx and rx).
> > > > After overwriting stmmac_dma_ops.dma_interrupt() and stmmac_dma_ops.init_chan(),
> > > > the logic is consistent with 0x37,
> > > >
> > > > so we overwrite synopsys_id to 0x37.
> > Yeah, something like that:
> > /* The original IP-core version is 0x37 in all Loongson GNET
> > * (2k2000 and 7a2000), but the GNET HW designers have changed the
> > * GMAC_VERSION.SNPSVER field to the custom 0x10 value on the Loongson
> > * 2k2000 MAC to emphasize the differences: multiple DMA-channels, AV
> > * feature and GMAC_INT_STATUS CSR flags layout. Get back the
> > * original value so the correct HW-interface would be
> > * selected.
> > */
> OK, Thanks!
> > > > > > ld = priv->plat->bsp_priv;
> > > > > > mac->dma = &ld->dwlgmac_dma_ops;
> > > > > > >> @@ -350,6 +352,46 @@ static struct mac_device_info
> > > > *loongson_setup(void *apriv)
> > > > > > return mac;
> > > > > > }
> > > > > > >> +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 = ~(u32)BIT(2);
> > > > > > +
> > > > > > + plat->phy_addr = 2;
> > > > > > + plat->phy_interface = PHY_INTERFACE_MODE_INTERNAL;
> > > > > Are you sure PHY-interface is supposed to be defined as "internal"?
> > > > Yes, because the gnet hardware has a integrated PHY, so we set it to internal,
> > > >
> > Why do you need the phy_addr set to 2 then? Is PHY still discoverable
> > on the subordinate MDIO-bus?
> >
> > kdoc in "include/linux/phy.h" defines the PHY_INTERFACE_MODE_INTERNAL
> > mode as for a case of the MAC and PHY being combined. IIUC it's
> > reserved for a case when you can't determine actual interface between
> > the MAC and PHY. Is it your case? Are you sure the interface between
> > MAC and PHY isn't something like GMII/RGMII/etc?
>
> Please allow me to consult with our hardware engineers before replying to
> you. :)
>
>
> Thanks,
>
> Yanteng
>
> >
> > -Serge(y)
> >
> > > > Correspondingly, our gmac hardware PHY is external.
> > > >
> > > > > > +
> > > > > > + plat->bsp_priv = &pdev->dev;
> > > > > > +
> > > > > > + 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;
> > > > > > +
> > > > > > + ret = loongson_dwmac_config_legacy(pdev, plat, res, np);
> > > > > Again. This will be moved to the probe() method in one of the next
> > > > > patches leaving loongson_gnet_config() empty. What was the problem
> > > > > with doing that right away with no intermediate change?
> > > > No problem. My original intention is to break the patches down into smaller pieces.
> > > >
> > > > In the next version, I will try to re-break them based on your comments.
> > > >
> > > > > > +
> > > > > > + 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)
> > > > > > {
> > > > > > @@ -516,9 +558,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) },
> > > > > After this the driver is supposed to correctly handle the Loongson
> > > > > GNET devices. Based on the patches introduced further it isn't.
> > > > > Please consider re-arranging the changes (see my comments in the
> > > > > further patches).
> > > > OK.
> > > >
> > > >
> > > > Thanks,
> > > >
> > > > Yanteng
> > > >
> > > >
> > > > > -Serge(y)
> > > > >
> > > > > > {}
> > > > > > };
> > > > > > MODULE_DEVICE_TABLE(pci, loongson_dwmac_id_table);
> > > > > > -- >> 2.31.4
> > > > > >
>
Powered by blists - more mailing lists