Date: Thu, 14 Mar 2024 21:12:49 +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 06/11] net: stmmac: dwmac-loongson: Add GNET
 support


在 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.
>
> >
> >> 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.
>
> >>   	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，
>
> 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