| Message ID | 16ec5a0665bcce96757be140019d81b0fe5f6303.1716973237.git.siyanteng@loongson.cn (mailing list archive) |
|---|---|
| State | Deferred |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | stmmac: Add Loongson platform support | expand |
Hi, Yanteng, On Wed, May 29, 2024 at 6:21 PM Yanteng Si <siyanteng@loongson.cn> wrote: > > Aside with the Loongson GMAC controllers which can be normally found > on the LS2K1000 SoC and LS7A1000 chipset, Loongson released a new > version of the network controllers called Loongson GNET. It has > been synthesized into the new generation LS2K2000 SoC and LS7A2000 > chipset with the next DW GMAC features enabled: > > DW GMAC IP-core: v3.73a > Speeds: 10/100/1000Mbps > Duplex: Full (both versions), Half (LS2K2000 SoC only) > DMA-descriptors type: enhanced > L3/L4 filters availability: Y > VLAN hash table filter: Y > PHY-interface: GMII (PHY is integrated into the chips) > Remote Wake-up support: Y > Mac Management Counters (MMC): Y > Number of additional MAC addresses: 5 > MAC Hash-based filter: Y > Hash Table Size: 256 > AV feature: Y (LS2K2000 SoC only) > DMA channels: 8 (LS2K2000 SoC), 1 (LS7A2000 chipset) > > The integrated PHY has a weird problem with switching from the low > speeds to 1000Mbps mode. The speedup procedure requires the PHY-link > re-negotiation. Besides the LS2K2000 GNET controller the next > peculiarities: > 1. Split up Tx and Rx DMA IRQ status/mask bits: > Name Tx Rx > DMA_INTR_ENA_NIE = 0x00040000 | 0x00020000; > DMA_INTR_ENA_AIE = 0x00010000 | 0x00008000; > DMA_STATUS_NIS = 0x00040000 | 0x00020000; > DMA_STATUS_AIS = 0x00010000 | 0x00008000; > DMA_STATUS_FBI = 0x00002000 | 0x00001000; > 2. Custom Synopsys ID hardwired into the GMAC_VERSION.SNPSVER field. > It's 0x10 while it should have been 0x37 in accordance with the actual > DW GMAC IP-core version. > > Thus in order to have the Loongson GNET controllers supported let's > modify the Loongson DWMAC driver in accordance with all the > peculiarities described above: > > 1. Create the Loongson GNET-specific > stmmac_dma_ops::dma_interrupt() > stmmac_dma_ops::init_chan() > callbacks due to the non-standard DMA IRQ CSR flags layout. > 2. Create the Loongson GNET-specific platform setup() method which > gets to initialize the DMA-ops with the dwmac1000_dma_ops instance > and overrides the callbacks described in 1, and overrides the custom > Synopsys ID with the real one in order to have the rest of the > HW-specific callbacks correctly detected by the driver core. > 3. Make sure the Loongson GNET-specific platform setup() method > enables the duplex modes supported by the controller. > 4. Provide the plat_stmmacenet_data::fix_mac_speed() callback which > will restart the link Auto-negotiation in case of the speed change. > > Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn> > Signed-off-by: Yinggang Gu <guyinggang@loongson.cn> > Signed-off-by: Yanteng Si <siyanteng@loongson.cn> > --- > drivers/net/ethernet/stmicro/stmmac/common.h | 1 + > .../ethernet/stmicro/stmmac/dwmac-loongson.c | 390 +++++++++++++++++- > 2 files changed, 387 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h > index 9cd62b2110a1..aed6ae80cc7c 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/common.h > +++ b/drivers/net/ethernet/stmicro/stmmac/common.h > @@ -29,6 +29,7 @@ > /* Synopsys Core versions */ > #define DWMAC_CORE_3_40 0x34 > #define DWMAC_CORE_3_50 0x35 > +#define DWMAC_CORE_3_70 0x37 > #define DWMAC_CORE_4_00 0x40 > #define DWMAC_CORE_4_10 0x41 > #define DWMAC_CORE_5_00 0x50 > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > index 45dcc35b7955..559215e3fe41 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > @@ -8,8 +8,71 @@ > #include <linux/device.h> > #include <linux/of_irq.h> > #include "stmmac.h" > +#include "dwmac_dma.h" > +#include "dwmac1000.h" > + > +/* Normal Loongson Tx Summary */ > +#define DMA_INTR_ENA_NIE_TX_LOONGSON 0x00040000 > +/* Normal Loongson Rx Summary */ > +#define DMA_INTR_ENA_NIE_RX_LOONGSON 0x00020000 > + > +#define DMA_INTR_NORMAL_LOONGSON (DMA_INTR_ENA_NIE_TX_LOONGSON | \ > + DMA_INTR_ENA_NIE_RX_LOONGSON | \ > + DMA_INTR_ENA_RIE | DMA_INTR_ENA_TIE) > + > +/* Abnormal Loongson Tx Summary */ > +#define DMA_INTR_ENA_AIE_TX_LOONGSON 0x00010000 > +/* Abnormal Loongson Rx Summary */ > +#define DMA_INTR_ENA_AIE_RX_LOONGSON 0x00008000 > + > +#define DMA_INTR_ABNORMAL_LOONGSON (DMA_INTR_ENA_AIE_TX_LOONGSON | \ > + DMA_INTR_ENA_AIE_RX_LOONGSON | \ > + DMA_INTR_ENA_FBE | DMA_INTR_ENA_UNE) > + > +#define DMA_INTR_DEFAULT_MASK_LOONGSON (DMA_INTR_NORMAL_LOONGSON | \ > + DMA_INTR_ABNORMAL_LOONGSON) > + > +/* Normal Loongson Tx Interrupt Summary */ > +#define DMA_STATUS_NIS_TX_LOONGSON 0x00040000 > +/* Normal Loongson Rx Interrupt Summary */ > +#define DMA_STATUS_NIS_RX_LOONGSON 0x00020000 > + > +/* Abnormal Loongson Tx Interrupt Summary */ > +#define DMA_STATUS_AIS_TX_LOONGSON 0x00010000 > +/* Abnormal Loongson Rx Interrupt Summary */ > +#define DMA_STATUS_AIS_RX_LOONGSON 0x00008000 > + > +/* Fatal Loongson Tx Bus Error Interrupt */ > +#define DMA_STATUS_FBI_TX_LOONGSON 0x00002000 > +/* Fatal Loongson Rx Bus Error Interrupt */ > +#define DMA_STATUS_FBI_RX_LOONGSON 0x00001000 > + > +#define DMA_STATUS_MSK_COMMON_LOONGSON (DMA_STATUS_NIS_TX_LOONGSON | \ > + DMA_STATUS_NIS_RX_LOONGSON | \ > + DMA_STATUS_AIS_TX_LOONGSON | \ > + DMA_STATUS_AIS_RX_LOONGSON | \ > + DMA_STATUS_FBI_TX_LOONGSON | \ > + DMA_STATUS_FBI_RX_LOONGSON) > + > +#define DMA_STATUS_MSK_RX_LOONGSON (DMA_STATUS_ERI | DMA_STATUS_RWT | \ > + DMA_STATUS_RPS | DMA_STATUS_RU | \ > + DMA_STATUS_RI | DMA_STATUS_OVF | \ > + DMA_STATUS_MSK_COMMON_LOONGSON) > + > +#define DMA_STATUS_MSK_TX_LOONGSON (DMA_STATUS_ETI | DMA_STATUS_UNF | \ > + DMA_STATUS_TJT | DMA_STATUS_TU | \ > + DMA_STATUS_TPS | DMA_STATUS_TI | \ > + DMA_STATUS_MSK_COMMON_LOONGSON) > > #define PCI_DEVICE_ID_LOONGSON_GMAC 0x7a03 > +#define PCI_DEVICE_ID_LOONGSON_GNET 0x7a13 > +#define DWMAC_CORE_LS2K2000 0x10 /* Loongson custom IP */ It is not suitable to call 0x10 "LS2K2000", because LS2K2000 is the name of the whole SOC, not the NIC IP. As an example, ThinkPad is the name of a whole computer series, you cannot call its CPU "ThinkPad CPU". Right? From my point of view, the name "LOONGSON_DWMAC_CORE_1_00" in V12 is much better. If any macro name for 0x10 is unacceptable, and open-code 0x10 is also unaccpetable, then there is an alternative way, apply the below patch on top of this one: diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c index b41ffdc6d3d0..81293e2570e8 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c @@ -66,11 +66,10 @@ #define PCI_DEVICE_ID_LOONGSON_GMAC 0x7a03 #define PCI_DEVICE_ID_LOONGSON_GNET 0x7a13 -#define DWMAC_CORE_LS2K2000 0x10 /* Loongson custom IP */ #define CHANNEL_NUM 8 struct loongson_data { - u32 loongson_id; + int has_multichan; struct device *dev; }; @@ -370,7 +369,7 @@ static struct mac_device_info *loongson_dwmac_setup(void *apriv) * AV feature and GMAC_INT_STATUS CSR flags layout. Get back the * original value so the correct HW-interface would be selected. */ - if (ld->loongson_id == DWMAC_CORE_LS2K2000) { + if (ld->has_multichan) { priv->synopsys_id = DWMAC_CORE_3_70; *dma = dwmac1000_dma_ops; dma->init_chan = loongson_gnet_dma_init_channel; @@ -397,7 +396,7 @@ static struct mac_device_info *loongson_dwmac_setup(void *apriv) if (pdev->device == PCI_DEVICE_ID_LOONGSON_GMAC) { mac->link.caps = MAC_10 | MAC_100 | MAC_1000; } else { - if (ld->loongson_id == DWMAC_CORE_LS2K2000) + if (ld->has_multichan) mac->link.caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE | MAC_10 | MAC_100 | MAC_1000; else @@ -474,6 +473,7 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id struct stmmac_pci_info *info; struct stmmac_resources res; struct loongson_data *ld; + u32 gmac_version; int ret, i; plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL); @@ -530,9 +530,19 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id memset(&res, 0, sizeof(res)); res.addr = pcim_iomap_table(pdev)[0]; - ld->loongson_id = readl(res.addr + GMAC_VERSION) & 0xff; + gmac_version = readl(res.addr + GMAC_VERSION) & 0xff; - if (ld->loongson_id == DWMAC_CORE_LS2K2000) { + switch (gmac_version) { + case DWMAC_CORE_3_50: + case DWMAC_CORE_3_70: + ld->has_multichan = 0; + plat->tx_queues_to_use = 1; + plat->rx_queues_to_use = 1; + ret = loongson_dwmac_intx_config(pdev, plat, &res); + break; + + default: + ld->has_multichan = 1; plat->rx_queues_to_use = CHANNEL_NUM; plat->tx_queues_to_use = CHANNEL_NUM; @@ -543,12 +553,8 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id plat->tx_queues_cfg[i].coe_unsupported = 1; ret = loongson_dwmac_msi_config(pdev, plat, &res); - } else { - plat->tx_queues_to_use = 1; - plat->rx_queues_to_use = 1; + } - ret = loongson_dwmac_intx_config(pdev, plat, &res); - } if (ret) goto err_disable_device; Huacai > +#define CHANNEL_NUM 8 > + > +struct loongson_data { > + u32 loongson_id; > + struct device *dev; > +}; > > struct stmmac_pci_info { > int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat); > @@ -67,6 +130,298 @@ static struct stmmac_pci_info loongson_gmac_pci_info = { > .setup = loongson_gmac_data, > }; > > +static void loongson_gnet_dma_init_channel(struct stmmac_priv *priv, > + void __iomem *ioaddr, > + struct stmmac_dma_cfg *dma_cfg, > + u32 chan) > +{ > + int txpbl = dma_cfg->txpbl ?: dma_cfg->pbl; > + int rxpbl = dma_cfg->rxpbl ?: dma_cfg->pbl; > + u32 value; > + > + value = readl(ioaddr + DMA_CHAN_BUS_MODE(chan)); > + > + if (dma_cfg->pblx8) > + value |= DMA_BUS_MODE_MAXPBL; > + > + value |= DMA_BUS_MODE_USP; > + value &= ~(DMA_BUS_MODE_PBL_MASK | DMA_BUS_MODE_RPBL_MASK); > + value |= (txpbl << DMA_BUS_MODE_PBL_SHIFT); > + value |= (rxpbl << DMA_BUS_MODE_RPBL_SHIFT); > + > + /* Set the Fixed burst mode */ > + if (dma_cfg->fixed_burst) > + value |= DMA_BUS_MODE_FB; > + > + /* Mixed Burst has no effect when fb is set */ > + if (dma_cfg->mixed_burst) > + value |= DMA_BUS_MODE_MB; > + > + if (dma_cfg->atds) > + value |= DMA_BUS_MODE_ATDS; > + > + if (dma_cfg->aal) > + value |= DMA_BUS_MODE_AAL; > + > + writel(value, ioaddr + DMA_CHAN_BUS_MODE(chan)); > + > + /* Mask interrupts by writing to CSR7 */ > + writel(DMA_INTR_DEFAULT_MASK_LOONGSON, ioaddr + > + DMA_CHAN_INTR_ENA(chan)); > +} > + > +static int loongson_gnet_dma_interrupt(struct stmmac_priv *priv, > + void __iomem *ioaddr, > + struct stmmac_extra_stats *x, > + u32 chan, u32 dir) > +{ > + struct stmmac_pcpu_stats *stats = this_cpu_ptr(priv->xstats.pcpu_stats); > + u32 abnor_intr_status; > + u32 nor_intr_status; > + u32 fb_intr_status; > + u32 intr_status; > + int ret = 0; > + > + /* read the status register (CSR5) */ > + intr_status = readl(ioaddr + DMA_CHAN_STATUS(chan)); > + > + if (dir == DMA_DIR_RX) > + intr_status &= DMA_STATUS_MSK_RX_LOONGSON; > + else if (dir == DMA_DIR_TX) > + intr_status &= DMA_STATUS_MSK_TX_LOONGSON; > + > + nor_intr_status = intr_status & (DMA_STATUS_NIS_TX_LOONGSON | > + DMA_STATUS_NIS_RX_LOONGSON); > + abnor_intr_status = intr_status & (DMA_STATUS_AIS_TX_LOONGSON | > + DMA_STATUS_AIS_RX_LOONGSON); > + fb_intr_status = intr_status & (DMA_STATUS_FBI_TX_LOONGSON | > + DMA_STATUS_FBI_RX_LOONGSON); > + > + /* ABNORMAL interrupts */ > + if (unlikely(abnor_intr_status)) { > + if (unlikely(intr_status & DMA_STATUS_UNF)) { > + ret = tx_hard_error_bump_tc; > + x->tx_undeflow_irq++; > + } > + if (unlikely(intr_status & DMA_STATUS_TJT)) > + x->tx_jabber_irq++; > + if (unlikely(intr_status & DMA_STATUS_OVF)) > + x->rx_overflow_irq++; > + if (unlikely(intr_status & DMA_STATUS_RU)) > + x->rx_buf_unav_irq++; > + if (unlikely(intr_status & DMA_STATUS_RPS)) > + x->rx_process_stopped_irq++; > + if (unlikely(intr_status & DMA_STATUS_RWT)) > + x->rx_watchdog_irq++; > + if (unlikely(intr_status & DMA_STATUS_ETI)) > + x->tx_early_irq++; > + if (unlikely(intr_status & DMA_STATUS_TPS)) { > + x->tx_process_stopped_irq++; > + ret = tx_hard_error; > + } > + if (unlikely(fb_intr_status)) { > + x->fatal_bus_error_irq++; > + ret = tx_hard_error; > + } > + } > + /* TX/RX NORMAL interrupts */ > + if (likely(nor_intr_status)) { > + if (likely(intr_status & DMA_STATUS_RI)) { > + u32 value = readl(ioaddr + DMA_INTR_ENA); > + /* to schedule NAPI on real RIE event. */ > + if (likely(value & DMA_INTR_ENA_RIE)) { > + u64_stats_update_begin(&stats->syncp); > + u64_stats_inc(&stats->rx_normal_irq_n[chan]); > + u64_stats_update_end(&stats->syncp); > + ret |= handle_rx; > + } > + } > + if (likely(intr_status & DMA_STATUS_TI)) { > + u64_stats_update_begin(&stats->syncp); > + u64_stats_inc(&stats->tx_normal_irq_n[chan]); > + u64_stats_update_end(&stats->syncp); > + ret |= handle_tx; > + } > + if (unlikely(intr_status & DMA_STATUS_ERI)) > + x->rx_early_irq++; > + } > + /* Optional hardware blocks, interrupts should be disabled */ > + if (unlikely(intr_status & > + (DMA_STATUS_GPI | DMA_STATUS_GMI | DMA_STATUS_GLI))) > + pr_warn("%s: unexpected status %08x\n", __func__, intr_status); > + > + /* Clear the interrupt by writing a logic 1 to the CSR5[19-0] */ > + writel((intr_status & 0x7ffff), ioaddr + DMA_CHAN_STATUS(chan)); > + > + return ret; > +} > + > +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 integrated PHY has a weird problem with switching from the low > + * speeds to 1000Mbps mode. The speedup procedure requires the PHY-link > + * re-negotiation. > + */ > + if (speed == SPEED_1000) { > + if (readl(ptr->ioaddr + MAC_CTRL_REG) & > + GMAC_CONTROL_PS) > + /* Word around hardware bug, restart autoneg */ > + phy_restart_aneg(ndev->phydev); > + } > +} > + > +static int loongson_gnet_data(struct pci_dev *pdev, > + struct plat_stmmacenet_data *plat) > +{ > + loongson_default_data(pdev, plat); > + > + plat->phy_interface = PHY_INTERFACE_MODE_GMII; > + plat->mdio_bus_data->phy_mask = ~(u32)BIT(2); > + plat->fix_mac_speed = loongson_gnet_fix_speed; > + > + /* GNET devices with dev revision 0x00 do not support manually > + * setting the speed to 1000. > + */ > + if (pdev->revision == 0x00) > + plat->flags |= STMMAC_FLAG_DISABLE_FORCE_1000; > + > + return 0; > +} > + > +static struct stmmac_pci_info loongson_gnet_pci_info = { > + .setup = loongson_gnet_data, > +}; > + > +static int loongson_dwmac_intx_config(struct pci_dev *pdev, > + struct plat_stmmacenet_data *plat, > + struct stmmac_resources *res) > +{ > + res->irq = pdev->irq; > + > + return 0; > +} > + > +static int loongson_dwmac_msi_config(struct pci_dev *pdev, > + struct plat_stmmacenet_data *plat, > + struct stmmac_resources *res) > +{ > + int i, ret, vecs; > + > + vecs = roundup_pow_of_two(CHANNEL_NUM * 2 + 1); > + ret = pci_alloc_irq_vectors(pdev, 1, vecs, PCI_IRQ_MSI | PCI_IRQ_INTX); > + if (ret < 0) { > + dev_err(&pdev->dev, "Failed to allocate PCI IRQs\n"); > + return ret; > + } > + > + if (ret >= vecs) { > + for (i = 0; i < plat->rx_queues_to_use; i++) { > + res->rx_irq[CHANNEL_NUM - 1 - i] = > + pci_irq_vector(pdev, 1 + i * 2); > + } > + for (i = 0; i < plat->tx_queues_to_use; i++) { > + res->tx_irq[CHANNEL_NUM - 1 - i] = > + pci_irq_vector(pdev, 2 + i * 2); > + } > + > + plat->flags |= STMMAC_FLAG_MULTI_MSI_EN; > + } > + > + res->irq = pci_irq_vector(pdev, 0); > + > + return 0; > +} > + > +static int loongson_dwmac_msi_clear(struct pci_dev *pdev) > +{ > + pci_free_irq_vectors(pdev); > + > + return 0; > +} > + > +static struct mac_device_info *loongson_dwmac_setup(void *apriv) > +{ > + struct stmmac_priv *priv = apriv; > + struct mac_device_info *mac; > + struct stmmac_dma_ops *dma; > + struct loongson_data *ld; > + struct pci_dev *pdev; > + > + ld = priv->plat->bsp_priv; > + pdev = to_pci_dev(priv->device); > + > + mac = devm_kzalloc(priv->device, sizeof(*mac), GFP_KERNEL); > + if (!mac) > + return NULL; > + > + dma = devm_kzalloc(priv->device, sizeof(*dma), GFP_KERNEL); > + if (!dma) > + return NULL; > + > + /* The original IP-core version is v3.73a in all Loongson GNET > + * (LS2K2000 and LS7A2000), but the GNET HW designers have changed the > + * GMAC_VERSION.SNPSVER field to the custom 0x10 value on the Loongson > + * LS2K2000 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. > + */ > + if (ld->loongson_id == DWMAC_CORE_LS2K2000) { > + priv->synopsys_id = DWMAC_CORE_3_70; > + *dma = dwmac1000_dma_ops; > + dma->init_chan = loongson_gnet_dma_init_channel; > + dma->dma_interrupt = loongson_gnet_dma_interrupt; > + mac->dma = dma; > + } > + > + priv->dev->priv_flags |= IFF_UNICAST_FLT; > + > + /* Pre-initialize the respective "mac" fields as it's done in > + * dwmac1000_setup() > + */ > + mac->pcsr = priv->ioaddr; > + mac->multicast_filter_bins = priv->plat->multicast_filter_bins; > + mac->unicast_filter_entries = priv->plat->unicast_filter_entries; > + mac->mcast_bits_log2 = 0; > + > + if (mac->multicast_filter_bins) > + mac->mcast_bits_log2 = ilog2(mac->multicast_filter_bins); > + > + /* Loongson GMAC doesn't support the flow control. LS2K2000 > + * GNET doesn't support the half-duplex link mode. > + */ > + if (pdev->device == PCI_DEVICE_ID_LOONGSON_GMAC) { > + mac->link.caps = MAC_10 | MAC_100 | MAC_1000; > + } else { > + if (ld->loongson_id == DWMAC_CORE_LS2K2000) > + mac->link.caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE | > + MAC_10 | MAC_100 | MAC_1000; > + else > + mac->link.caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE | > + MAC_10FD | MAC_100FD | MAC_1000FD; > + } > + > + mac->link.duplex = GMAC_CONTROL_DM; > + mac->link.speed10 = GMAC_CONTROL_PS; > + mac->link.speed100 = GMAC_CONTROL_PS | GMAC_CONTROL_FES; > + mac->link.speed1000 = 0; > + mac->link.speed_mask = GMAC_CONTROL_PS | GMAC_CONTROL_FES; > + mac->mii.addr = GMAC_MII_ADDR; > + mac->mii.data = GMAC_MII_DATA; > + mac->mii.addr_shift = 11; > + mac->mii.addr_mask = 0x0000F800; > + mac->mii.reg_shift = 6; > + mac->mii.reg_mask = 0x000007C0; > + mac->mii.clk_csr_shift = 2; > + mac->mii.clk_csr_mask = GENMASK(5, 2); > + > + return mac; > +} > + > static int loongson_dwmac_dt_config(struct pci_dev *pdev, > struct plat_stmmacenet_data *plat, > struct stmmac_resources *res) > @@ -119,6 +474,7 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > struct plat_stmmacenet_data *plat; > struct stmmac_pci_info *info; > struct stmmac_resources res; > + struct loongson_data *ld; > int ret, i; > > plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL); > @@ -135,6 +491,10 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > if (!plat->dma_cfg) > return -ENOMEM; > > + ld = devm_kzalloc(&pdev->dev, sizeof(*ld), GFP_KERNEL); > + if (!ld) > + return -ENOMEM; > + > /* Enable pci device */ > ret = pci_enable_device(pdev); > if (ret) { > @@ -159,19 +519,39 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > > pci_set_master(pdev); > > + plat->bsp_priv = ld; > + plat->setup = loongson_dwmac_setup; > + ld->dev = &pdev->dev; > + > if (dev_of_node(&pdev->dev)) { > ret = loongson_dwmac_dt_config(pdev, plat, &res); > if (ret) > goto err_disable_device; > - } else { > - res.irq = pdev->irq; > } > > memset(&res, 0, sizeof(res)); > res.addr = pcim_iomap_table(pdev)[0]; > + ld->loongson_id = readl(res.addr + GMAC_VERSION) & 0xff; > + > + if (ld->loongson_id == DWMAC_CORE_LS2K2000) { > + plat->rx_queues_to_use = CHANNEL_NUM; > + plat->tx_queues_to_use = CHANNEL_NUM; > + > + /* Only channel 0 supports checksum, > + * so turn off checksum to enable multiple channels. > + */ > + for (i = 1; i < CHANNEL_NUM; i++) > + plat->tx_queues_cfg[i].coe_unsupported = 1; > > - plat->tx_queues_to_use = 1; > - plat->rx_queues_to_use = 1; > + ret = loongson_dwmac_msi_config(pdev, plat, &res); > + } else { > + plat->tx_queues_to_use = 1; > + plat->rx_queues_to_use = 1; > + > + ret = loongson_dwmac_intx_config(pdev, plat, &res); > + } > + if (ret) > + goto err_disable_device; > > ret = stmmac_dvr_probe(&pdev->dev, plat, &res); > if (ret) > @@ -202,6 +582,7 @@ static void loongson_dwmac_remove(struct pci_dev *pdev) > break; > } > > + loongson_dwmac_msi_clear(pdev); > pci_disable_device(pdev); > } > > @@ -245,6 +626,7 @@ static SIMPLE_DEV_PM_OPS(loongson_dwmac_pm_ops, loongson_dwmac_suspend, > > 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); > -- > 2.31.4 >
在 2024/5/30 10:46, Huacai Chen 写道: >> #define PCI_DEVICE_ID_LOONGSON_GMAC 0x7a03 >> +#define PCI_DEVICE_ID_LOONGSON_GNET 0x7a13 >> +#define DWMAC_CORE_LS2K2000 0x10 /* Loongson custom IP */ > It is not suitable to call 0x10 "LS2K2000", because LS2K2000 is the > name of the whole SOC, not the NIC IP. As an example, ThinkPad is the > name of a whole computer series, you cannot call its CPU "ThinkPad > CPU". Right? > From my point of view, the name "LOONGSON_DWMAC_CORE_1_00" in V12 is > much better. > > If any macro name for 0x10 is unacceptable, and open-code 0x10 is also > unaccpetable, then there is an alternative way, apply the below patch > on top of this one: > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > index b41ffdc6d3d0..81293e2570e8 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > @@ -66,11 +66,10 @@ > > #define PCI_DEVICE_ID_LOONGSON_GMAC 0x7a03 > #define PCI_DEVICE_ID_LOONGSON_GNET 0x7a13 > -#define DWMAC_CORE_LS2K2000 0x10 /* Loongson custom IP */ > #define CHANNEL_NUM 8 > > struct loongson_data { > - u32 loongson_id; > + int has_multichan; > struct device *dev; > }; > > @@ -370,7 +369,7 @@ static struct mac_device_info > *loongson_dwmac_setup(void *apriv) > * AV feature and GMAC_INT_STATUS CSR flags layout. Get back the > * original value so the correct HW-interface would be selected. > */ > - if (ld->loongson_id == DWMAC_CORE_LS2K2000) { > + if (ld->has_multichan) { > priv->synopsys_id = DWMAC_CORE_3_70; > *dma = dwmac1000_dma_ops; > dma->init_chan = loongson_gnet_dma_init_channel; > @@ -397,7 +396,7 @@ static struct mac_device_info > *loongson_dwmac_setup(void *apriv) > if (pdev->device == PCI_DEVICE_ID_LOONGSON_GMAC) { > mac->link.caps = MAC_10 | MAC_100 | MAC_1000; > } else { > - if (ld->loongson_id == DWMAC_CORE_LS2K2000) > + if (ld->has_multichan) > mac->link.caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE | > MAC_10 | MAC_100 | MAC_1000; > else > @@ -474,6 +473,7 @@ static int loongson_dwmac_probe(struct pci_dev > *pdev, const struct pci_device_id > struct stmmac_pci_info *info; > struct stmmac_resources res; > struct loongson_data *ld; > + u32 gmac_version; > int ret, i; > > plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL); > @@ -530,9 +530,19 @@ static int loongson_dwmac_probe(struct pci_dev > *pdev, const struct pci_device_id > > memset(&res, 0, sizeof(res)); > res.addr = pcim_iomap_table(pdev)[0]; > - ld->loongson_id = readl(res.addr + GMAC_VERSION) & 0xff; > + gmac_version = readl(res.addr + GMAC_VERSION) & 0xff; > > - if (ld->loongson_id == DWMAC_CORE_LS2K2000) { > + switch (gmac_version) { > + case DWMAC_CORE_3_50: > + case DWMAC_CORE_3_70: > + ld->has_multichan = 0; > + plat->tx_queues_to_use = 1; > + plat->rx_queues_to_use = 1; > + ret = loongson_dwmac_intx_config(pdev, plat, &res); > + break; > + > + default: > + ld->has_multichan = 1; > plat->rx_queues_to_use = CHANNEL_NUM; > plat->tx_queues_to_use = CHANNEL_NUM; > @@ -543,12 +553,8 @@ static int loongson_dwmac_probe(struct pci_dev > *pdev, const struct pci_device_id > plat->tx_queues_cfg[i].coe_unsupported = 1; > > ret = loongson_dwmac_msi_config(pdev, plat, &res); > - } else { > - plat->tx_queues_to_use = 1; > - plat->rx_queues_to_use = 1; > + } > > - ret = loongson_dwmac_intx_config(pdev, plat, &res); > - } > if (ret) > goto err_disable_device; I think it's great. What about everyone else? Thanks, Yanteng
Hi all, 在 2024/5/30 10:46, Huacai Chen 写道: >> #define PCI_DEVICE_ID_LOONGSON_GMAC 0x7a03 >> +#define PCI_DEVICE_ID_LOONGSON_GNET 0x7a13 >> +#define DWMAC_CORE_LS2K2000 0x10 /* Loongson custom IP */ > It is not suitable to call 0x10 "LS2K2000", because LS2K2000 is the > name of the whole SOC, not the NIC IP. As an example, ThinkPad is the > name of a whole computer series, you cannot call its CPU "ThinkPad > CPU". Right? > From my point of view, the name "LOONGSON_DWMAC_CORE_1_00" in V12 is > much better. > > If any macro name for 0x10 is unacceptable, and open-code 0x10 is also > unaccpetable, then there is an alternative way, apply the below patch > on top of this one: > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > index b41ffdc6d3d0..81293e2570e8 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > @@ -66,11 +66,10 @@ > > #define PCI_DEVICE_ID_LOONGSON_GMAC 0x7a03 > #define PCI_DEVICE_ID_LOONGSON_GNET 0x7a13 > -#define DWMAC_CORE_LS2K2000 0x10 /* Loongson custom IP */ > #define CHANNEL_NUM 8 > > struct loongson_data { > - u32 loongson_id; > + int has_multichan; > struct device *dev; > }; > > @@ -370,7 +369,7 @@ static struct mac_device_info > *loongson_dwmac_setup(void *apriv) > * AV feature and GMAC_INT_STATUS CSR flags layout. Get back the > * original value so the correct HW-interface would be selected. > */ > - if (ld->loongson_id == DWMAC_CORE_LS2K2000) { > + if (ld->has_multichan) { > priv->synopsys_id = DWMAC_CORE_3_70; > *dma = dwmac1000_dma_ops; > dma->init_chan = loongson_gnet_dma_init_channel; > @@ -397,7 +396,7 @@ static struct mac_device_info > *loongson_dwmac_setup(void *apriv) > if (pdev->device == PCI_DEVICE_ID_LOONGSON_GMAC) { > mac->link.caps = MAC_10 | MAC_100 | MAC_1000; > } else { > - if (ld->loongson_id == DWMAC_CORE_LS2K2000) > + if (ld->has_multichan) > mac->link.caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE | > MAC_10 | MAC_100 | MAC_1000; > else > @@ -474,6 +473,7 @@ static int loongson_dwmac_probe(struct pci_dev > *pdev, const struct pci_device_id > struct stmmac_pci_info *info; > struct stmmac_resources res; > struct loongson_data *ld; > + u32 gmac_version; > int ret, i; > > plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL); > @@ -530,9 +530,19 @@ static int loongson_dwmac_probe(struct pci_dev > *pdev, const struct pci_device_id > > memset(&res, 0, sizeof(res)); > res.addr = pcim_iomap_table(pdev)[0]; > - ld->loongson_id = readl(res.addr + GMAC_VERSION) & 0xff; > + gmac_version = readl(res.addr + GMAC_VERSION) & 0xff; > > - if (ld->loongson_id == DWMAC_CORE_LS2K2000) { > + switch (gmac_version) { > + case DWMAC_CORE_3_50: > + case DWMAC_CORE_3_70: > + ld->has_multichan = 0; > + plat->tx_queues_to_use = 1; > + plat->rx_queues_to_use = 1; > + ret = loongson_dwmac_intx_config(pdev, plat, &res); > + break; > + > + default: > + ld->has_multichan = 1; > plat->rx_queues_to_use = CHANNEL_NUM; > plat->tx_queues_to_use = CHANNEL_NUM; > @@ -543,12 +553,8 @@ static int loongson_dwmac_probe(struct pci_dev > *pdev, const struct pci_device_id > plat->tx_queues_cfg[i].coe_unsupported = 1; > > ret = loongson_dwmac_msi_config(pdev, plat, &res); > - } else { > - plat->tx_queues_to_use = 1; > - plat->rx_queues_to_use = 1; > + } > > - ret = loongson_dwmac_intx_config(pdev, plat, &res); > - } > if (ret) > goto err_disable_device; > Huacai's method works. Thanks, Yanteng > Huacai
> [PATCH net-next v13 14/15] net: stmmac: dwmac-loongson: Add Loongson GNET support Seeing the multi-channels AV-feature can be found on the Loongson GMACs too we have to reconsider this patch logic by converting it to adding the multi-channels support for the Loongson GMAC device only. Everything Loongson GNET-specific should be moved to a new following up patch. So firstly please change this patch subject to: [PATCH net-next v14 13/15] net: stmmac: dwmac-loongson: Add Loongson Multi-channels GMAC support On Wed, May 29, 2024 at 06:21:09PM +0800, Yanteng Si wrote: > Aside with the Loongson GMAC controllers which can be normally found > on the LS2K1000 SoC and LS7A1000 chipset, Loongson released a new > version of the network controllers called Loongson GNET. It has > been synthesized into the new generation LS2K2000 SoC and LS7A2000 > chipset with the next DW GMAC features enabled: > > DW GMAC IP-core: v3.73a > Speeds: 10/100/1000Mbps > Duplex: Full (both versions), Half (LS2K2000 SoC only) > DMA-descriptors type: enhanced > L3/L4 filters availability: Y > VLAN hash table filter: Y > PHY-interface: GMII (PHY is integrated into the chips) > Remote Wake-up support: Y > Mac Management Counters (MMC): Y > Number of additional MAC addresses: 5 > MAC Hash-based filter: Y > Hash Table Size: 256 > AV feature: Y (LS2K2000 SoC only) > DMA channels: 8 (LS2K2000 SoC), 1 (LS7A2000 chipset) > > The integrated PHY has a weird problem with switching from the low > speeds to 1000Mbps mode. The speedup procedure requires the PHY-link > re-negotiation. Besides the LS2K2000 GNET controller the next > peculiarities: > 1. Split up Tx and Rx DMA IRQ status/mask bits: > Name Tx Rx > DMA_INTR_ENA_NIE = 0x00040000 | 0x00020000; > DMA_INTR_ENA_AIE = 0x00010000 | 0x00008000; > DMA_STATUS_NIS = 0x00040000 | 0x00020000; > DMA_STATUS_AIS = 0x00010000 | 0x00008000; > DMA_STATUS_FBI = 0x00002000 | 0x00001000; > 2. Custom Synopsys ID hardwired into the GMAC_VERSION.SNPSVER field. > It's 0x10 while it should have been 0x37 in accordance with the actual > DW GMAC IP-core version. > > Thus in order to have the Loongson GNET controllers supported let's > modify the Loongson DWMAC driver in accordance with all the > peculiarities described above: > > 1. Create the Loongson GNET-specific > stmmac_dma_ops::dma_interrupt() > stmmac_dma_ops::init_chan() > callbacks due to the non-standard DMA IRQ CSR flags layout. > 2. Create the Loongson GNET-specific platform setup() method which > gets to initialize the DMA-ops with the dwmac1000_dma_ops instance > and overrides the callbacks described in 1, and overrides the custom > Synopsys ID with the real one in order to have the rest of the > HW-specific callbacks correctly detected by the driver core. > 3. Make sure the Loongson GNET-specific platform setup() method > enables the duplex modes supported by the controller. > 4. Provide the plat_stmmacenet_data::fix_mac_speed() callback which > will restart the link Auto-negotiation in case of the speed change. Please convert the commit log of this patch to containing the multi-channels feature description only. Like this (correct me if I am wrong in understanding some of the Loongson network controllers aspects): "The Loongson DWMAC driver currently supports the Loongson GMAC devices (based on the DW GMAC v3.50a/v3.73a IP-core) installed to the LS2K1000 SoC and LS7A1000 chipset. But recently a new generation LS2K2000 SoC was released with the new version of the Loongson GMAC synthesized in. The new controller is based on the DW GMAC v3.73a IP-core with the AV-feature enabled, which implies the multi DMA-channels support. The multi DMA-channels feature has the next vendor-specific peculiarities: 1. Split up Tx and Rx DMA IRQ status/mask bits: Name Tx Rx DMA_INTR_ENA_NIE = 0x00040000 | 0x00020000; DMA_INTR_ENA_AIE = 0x00010000 | 0x00008000; DMA_STATUS_NIS = 0x00040000 | 0x00020000; DMA_STATUS_AIS = 0x00010000 | 0x00008000; DMA_STATUS_FBI = 0x00002000 | 0x00001000; 2. Custom Synopsys ID hardwired into the GMAC_VERSION.SNPSVER register field. It's 0x10 while it should have been 0x37 in accordance with the actual DW GMAC IP-core version. 3. There are eight DMA-channels available meanwhile the Synopsys DW GMAC IP-core supports up to three DMA-channels. 4. It's possible to have each DMA-channel IRQ independently delivered. The MSI IRQs must be utilized for that. Thus in order to have the multi-channels Loongson GMAC controllers supported let's modify the Loongson DWMAC driver in accordance with all the peculiarities described above: 1. Create the multi-channels Loongson GMAC-specific stmmac_dma_ops::dma_interrupt() stmmac_dma_ops::init_chan() callbacks due to the non-standard DMA IRQ CSR flags layout. 2. Create the Loongson DWMAC-specific platform setup() method which gets to initialize the DMA-ops with the dwmac1000_dma_ops instance and overrides the callbacks described in 1. The method also overrides the custom Synopsys ID with the real one in order to have the rest of the HW-specific callbacks correctly detected by the driver core. 3. Make sure the platform setup() method enables the flow control and duplex modes supported by the controller. " Once again, please correct the text if I was wrong in understanding the way your devices work. Especially regarding the flow-control and half-duplex mode. --- The Loongson GNET-specific changes should be moved to the new patch applied after this one. The subject of the new patch should be the same as the former subject of this patch: [PATCH net-next v14 14/15] net: stmmac: dwmac-loongson: Add Loongson GNET support but of course the commit log shall be different. Like this: "The new generation Loongson LS2K2000 SoC and LS7A2000 chipset are equipped with the network controllers called Loongson GNET. It's the single and multi DMA-channels Loongson GMAC but with a PHY attached. Here is the summary of the DW GMAC features the controller has: DW GMAC IP-core: v3.73a Speeds: 10/100/1000Mbps Duplex: Full (both versions), Half (LS2K2000 GNET only) DMA-descriptors type: enhanced L3/L4 filters availability: Y VLAN hash table filter: Y PHY-interface: GMII (PHY is integrated into the chips) Remote Wake-up support: Y Mac Management Counters (MMC): Y Number of additional MAC addresses: 5 MAC Hash-based filter: Y Hash Table Size: 256 AV feature: Y (LS2K2000 GNET only) DMA channels: 8 (LS2K2000 GNET), 1 (LS7A2000 GNET) Let's update the Loongson DWMAC driver to supporting the new Loongson GNET controller. The change is mainly trivial: the driver shall be bound to the PCIe device with DID 0x7a13, and the device-specific setup() method shall be called for it. The only peculiarity concerns the integrated PHY speed change procedure. The PHY has a weird problem with switching from the low speeds to 1000Mbps mode. The speedup procedure requires the PHY-link re-negotiation. So the suggested change provide the device-specific fix_mac_speed() method to overcome the problem." > > Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn> > Signed-off-by: Yinggang Gu <guyinggang@loongson.cn> > Signed-off-by: Yanteng Si <siyanteng@loongson.cn> > --- > drivers/net/ethernet/stmicro/stmmac/common.h | 1 + > .../ethernet/stmicro/stmmac/dwmac-loongson.c | 390 +++++++++++++++++- > 2 files changed, 387 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h > index 9cd62b2110a1..aed6ae80cc7c 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/common.h > +++ b/drivers/net/ethernet/stmicro/stmmac/common.h > @@ -29,6 +29,7 @@ > /* Synopsys Core versions */ > #define DWMAC_CORE_3_40 0x34 > #define DWMAC_CORE_3_50 0x35 > +#define DWMAC_CORE_3_70 0x37 > #define DWMAC_CORE_4_00 0x40 > #define DWMAC_CORE_4_10 0x41 > #define DWMAC_CORE_5_00 0x50 > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > index 45dcc35b7955..559215e3fe41 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > @@ -8,8 +8,71 @@ > #include <linux/device.h> > #include <linux/of_irq.h> > #include "stmmac.h" > +#include "dwmac_dma.h" > +#include "dwmac1000.h" > + > +/* Normal Loongson Tx Summary */ > +#define DMA_INTR_ENA_NIE_TX_LOONGSON 0x00040000 > +/* Normal Loongson Rx Summary */ > +#define DMA_INTR_ENA_NIE_RX_LOONGSON 0x00020000 > + > +#define DMA_INTR_NORMAL_LOONGSON (DMA_INTR_ENA_NIE_TX_LOONGSON | \ > + DMA_INTR_ENA_NIE_RX_LOONGSON | \ > + DMA_INTR_ENA_RIE | DMA_INTR_ENA_TIE) > + > +/* Abnormal Loongson Tx Summary */ > +#define DMA_INTR_ENA_AIE_TX_LOONGSON 0x00010000 > +/* Abnormal Loongson Rx Summary */ > +#define DMA_INTR_ENA_AIE_RX_LOONGSON 0x00008000 > + > +#define DMA_INTR_ABNORMAL_LOONGSON (DMA_INTR_ENA_AIE_TX_LOONGSON | \ > + DMA_INTR_ENA_AIE_RX_LOONGSON | \ > + DMA_INTR_ENA_FBE | DMA_INTR_ENA_UNE) > + > +#define DMA_INTR_DEFAULT_MASK_LOONGSON (DMA_INTR_NORMAL_LOONGSON | \ > + DMA_INTR_ABNORMAL_LOONGSON) > + > +/* Normal Loongson Tx Interrupt Summary */ > +#define DMA_STATUS_NIS_TX_LOONGSON 0x00040000 > +/* Normal Loongson Rx Interrupt Summary */ > +#define DMA_STATUS_NIS_RX_LOONGSON 0x00020000 > + > +/* Abnormal Loongson Tx Interrupt Summary */ > +#define DMA_STATUS_AIS_TX_LOONGSON 0x00010000 > +/* Abnormal Loongson Rx Interrupt Summary */ > +#define DMA_STATUS_AIS_RX_LOONGSON 0x00008000 > + > +/* Fatal Loongson Tx Bus Error Interrupt */ > +#define DMA_STATUS_FBI_TX_LOONGSON 0x00002000 > +/* Fatal Loongson Rx Bus Error Interrupt */ > +#define DMA_STATUS_FBI_RX_LOONGSON 0x00001000 > + > +#define DMA_STATUS_MSK_COMMON_LOONGSON (DMA_STATUS_NIS_TX_LOONGSON | \ > + DMA_STATUS_NIS_RX_LOONGSON | \ > + DMA_STATUS_AIS_TX_LOONGSON | \ > + DMA_STATUS_AIS_RX_LOONGSON | \ > + DMA_STATUS_FBI_TX_LOONGSON | \ > + DMA_STATUS_FBI_RX_LOONGSON) > + > +#define DMA_STATUS_MSK_RX_LOONGSON (DMA_STATUS_ERI | DMA_STATUS_RWT | \ > + DMA_STATUS_RPS | DMA_STATUS_RU | \ > + DMA_STATUS_RI | DMA_STATUS_OVF | \ > + DMA_STATUS_MSK_COMMON_LOONGSON) > + > +#define DMA_STATUS_MSK_TX_LOONGSON (DMA_STATUS_ETI | DMA_STATUS_UNF | \ > + DMA_STATUS_TJT | DMA_STATUS_TU | \ > + DMA_STATUS_TPS | DMA_STATUS_TI | \ > + DMA_STATUS_MSK_COMMON_LOONGSON) > > #define PCI_DEVICE_ID_LOONGSON_GMAC 0x7a03 > +#define PCI_DEVICE_ID_LOONGSON_GNET 0x7a13 Please move this to the new patch adding the Loongson GNET support. > +#define DWMAC_CORE_LS2K2000 0x10 /* Loongson custom IP */ Note it's perfectly fine to have a device named after the SoC it's equipped to. For example see the compatible strings defined for the vendor-specific versions of the DW *MAC IP-cores: Documentation/devicetree/bindings/net/snps,dwmac.yaml But if you aren't comfortable with such naming we can change the macro to something like: #define DWMAC_CORE_LOONGSON_MULTI_CH 0x10 > +#define CHANNEL_NUM 8 > + > +struct loongson_data { > + u32 loongson_id; > + struct device *dev; Please add the "dev" field in the new patch adding the Loongson GNET support. > +}; > > struct stmmac_pci_info { > int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat); > @@ -67,6 +130,298 @@ static struct stmmac_pci_info loongson_gmac_pci_info = { > .setup = loongson_gmac_data, > }; > > +static void loongson_gnet_dma_init_channel(struct stmmac_priv *priv, Since the multi-channel feature is no longer GNET-specific, please rename the method prefix to loongson_dwmac_ . > + void __iomem *ioaddr, > + struct stmmac_dma_cfg *dma_cfg, > + u32 chan) > +{ > + int txpbl = dma_cfg->txpbl ?: dma_cfg->pbl; > + int rxpbl = dma_cfg->rxpbl ?: dma_cfg->pbl; > + u32 value; > + > + value = readl(ioaddr + DMA_CHAN_BUS_MODE(chan)); > + > + if (dma_cfg->pblx8) > + value |= DMA_BUS_MODE_MAXPBL; > + > + value |= DMA_BUS_MODE_USP; > + value &= ~(DMA_BUS_MODE_PBL_MASK | DMA_BUS_MODE_RPBL_MASK); > + value |= (txpbl << DMA_BUS_MODE_PBL_SHIFT); > + value |= (rxpbl << DMA_BUS_MODE_RPBL_SHIFT); > + > + /* Set the Fixed burst mode */ > + if (dma_cfg->fixed_burst) > + value |= DMA_BUS_MODE_FB; > + > + /* Mixed Burst has no effect when fb is set */ > + if (dma_cfg->mixed_burst) > + value |= DMA_BUS_MODE_MB; > + > + if (dma_cfg->atds) > + value |= DMA_BUS_MODE_ATDS; > + > + if (dma_cfg->aal) > + value |= DMA_BUS_MODE_AAL; > + > + writel(value, ioaddr + DMA_CHAN_BUS_MODE(chan)); > + > + /* Mask interrupts by writing to CSR7 */ > + writel(DMA_INTR_DEFAULT_MASK_LOONGSON, ioaddr + > + DMA_CHAN_INTR_ENA(chan)); > +} > + > +static int loongson_gnet_dma_interrupt(struct stmmac_priv *priv, Similarly the loongson_dwmac_ prefix seems more appropriate now. > + void __iomem *ioaddr, > + struct stmmac_extra_stats *x, > + u32 chan, u32 dir) > +{ > + struct stmmac_pcpu_stats *stats = this_cpu_ptr(priv->xstats.pcpu_stats); > + u32 abnor_intr_status; > + u32 nor_intr_status; > + u32 fb_intr_status; > + u32 intr_status; > + int ret = 0; > + > + /* read the status register (CSR5) */ > + intr_status = readl(ioaddr + DMA_CHAN_STATUS(chan)); > + > + if (dir == DMA_DIR_RX) > + intr_status &= DMA_STATUS_MSK_RX_LOONGSON; > + else if (dir == DMA_DIR_TX) > + intr_status &= DMA_STATUS_MSK_TX_LOONGSON; > + > + nor_intr_status = intr_status & (DMA_STATUS_NIS_TX_LOONGSON | > + DMA_STATUS_NIS_RX_LOONGSON); > + abnor_intr_status = intr_status & (DMA_STATUS_AIS_TX_LOONGSON | > + DMA_STATUS_AIS_RX_LOONGSON); > + fb_intr_status = intr_status & (DMA_STATUS_FBI_TX_LOONGSON | > + DMA_STATUS_FBI_RX_LOONGSON); > + > + /* ABNORMAL interrupts */ > + if (unlikely(abnor_intr_status)) { > + if (unlikely(intr_status & DMA_STATUS_UNF)) { > + ret = tx_hard_error_bump_tc; > + x->tx_undeflow_irq++; > + } > + if (unlikely(intr_status & DMA_STATUS_TJT)) > + x->tx_jabber_irq++; > + if (unlikely(intr_status & DMA_STATUS_OVF)) > + x->rx_overflow_irq++; > + if (unlikely(intr_status & DMA_STATUS_RU)) > + x->rx_buf_unav_irq++; > + if (unlikely(intr_status & DMA_STATUS_RPS)) > + x->rx_process_stopped_irq++; > + if (unlikely(intr_status & DMA_STATUS_RWT)) > + x->rx_watchdog_irq++; > + if (unlikely(intr_status & DMA_STATUS_ETI)) > + x->tx_early_irq++; > + if (unlikely(intr_status & DMA_STATUS_TPS)) { > + x->tx_process_stopped_irq++; > + ret = tx_hard_error; > + } > + if (unlikely(fb_intr_status)) { > + x->fatal_bus_error_irq++; > + ret = tx_hard_error; > + } > + } > + /* TX/RX NORMAL interrupts */ > + if (likely(nor_intr_status)) { > + if (likely(intr_status & DMA_STATUS_RI)) { > + u32 value = readl(ioaddr + DMA_INTR_ENA); > + /* to schedule NAPI on real RIE event. */ > + if (likely(value & DMA_INTR_ENA_RIE)) { > + u64_stats_update_begin(&stats->syncp); > + u64_stats_inc(&stats->rx_normal_irq_n[chan]); > + u64_stats_update_end(&stats->syncp); > + ret |= handle_rx; > + } > + } > + if (likely(intr_status & DMA_STATUS_TI)) { > + u64_stats_update_begin(&stats->syncp); > + u64_stats_inc(&stats->tx_normal_irq_n[chan]); > + u64_stats_update_end(&stats->syncp); > + ret |= handle_tx; > + } > + if (unlikely(intr_status & DMA_STATUS_ERI)) > + x->rx_early_irq++; > + } > + /* Optional hardware blocks, interrupts should be disabled */ > + if (unlikely(intr_status & > + (DMA_STATUS_GPI | DMA_STATUS_GMI | DMA_STATUS_GLI))) > + pr_warn("%s: unexpected status %08x\n", __func__, intr_status); > + > + /* Clear the interrupt by writing a logic 1 to the CSR5[19-0] */ > + writel((intr_status & 0x7ffff), ioaddr + DMA_CHAN_STATUS(chan)); > + > + return ret; > +} > + > +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 integrated PHY has a weird problem with switching from the low > + * speeds to 1000Mbps mode. The speedup procedure requires the PHY-link > + * re-negotiation. > + */ > + if (speed == SPEED_1000) { > + if (readl(ptr->ioaddr + MAC_CTRL_REG) & > + GMAC_CONTROL_PS) > + /* Word around hardware bug, restart autoneg */ > + phy_restart_aneg(ndev->phydev); > + } > +} > + > +static int loongson_gnet_data(struct pci_dev *pdev, > + struct plat_stmmacenet_data *plat) > +{ > + loongson_default_data(pdev, plat); > + > + plat->phy_interface = PHY_INTERFACE_MODE_GMII; > + plat->mdio_bus_data->phy_mask = ~(u32)BIT(2); > + plat->fix_mac_speed = loongson_gnet_fix_speed; > + > + /* GNET devices with dev revision 0x00 do not support manually > + * setting the speed to 1000. > + */ > + if (pdev->revision == 0x00) > + plat->flags |= STMMAC_FLAG_DISABLE_FORCE_1000; > + > + return 0; > +} > + > +static struct stmmac_pci_info loongson_gnet_pci_info = { > + .setup = loongson_gnet_data, > +}; Please move this to the new patch adding the Loongson GNET support. > + > +static int loongson_dwmac_intx_config(struct pci_dev *pdev, > + struct plat_stmmacenet_data *plat, > + struct stmmac_resources *res) > +{ > + res->irq = pdev->irq; > + > + return 0; > +} If you get to implement what suggested here https://lore.kernel.org/netdev/glm3jfqf36t5vnkmk4gsdqfx53ga7ohs3pxnsizqlogkbim7gg@a3dxav5siczn/ than the loongson_dwmac_intx_config() won't be needed. > + > +static int loongson_dwmac_msi_config(struct pci_dev *pdev, > + struct plat_stmmacenet_data *plat, > + struct stmmac_resources *res) > +{ > + int i, ret, vecs; > + > + vecs = roundup_pow_of_two(CHANNEL_NUM * 2 + 1); > + ret = pci_alloc_irq_vectors(pdev, 1, vecs, PCI_IRQ_MSI | PCI_IRQ_INTX); > + if (ret < 0) { > + dev_err(&pdev->dev, "Failed to allocate PCI IRQs\n"); > + return ret; > + } > + > + if (ret >= vecs) { > + for (i = 0; i < plat->rx_queues_to_use; i++) { > + res->rx_irq[CHANNEL_NUM - 1 - i] = > + pci_irq_vector(pdev, 1 + i * 2); > + } > + for (i = 0; i < plat->tx_queues_to_use; i++) { > + res->tx_irq[CHANNEL_NUM - 1 - i] = > + pci_irq_vector(pdev, 2 + i * 2); > + } > + > + plat->flags |= STMMAC_FLAG_MULTI_MSI_EN; > + } > + > + res->irq = pci_irq_vector(pdev, 0); > + > + return 0; > +} Could you please clarify whether the multi-channel feature will work if the pci_alloc_irq_vectors() method failed to allocated as many MSI vectors as there are channels? > + > +static int loongson_dwmac_msi_clear(struct pci_dev *pdev) > +{ > + pci_free_irq_vectors(pdev); > + > + return 0; > +} > + > +static struct mac_device_info *loongson_dwmac_setup(void *apriv) > +{ > + struct stmmac_priv *priv = apriv; > + struct mac_device_info *mac; > + struct stmmac_dma_ops *dma; > + struct loongson_data *ld; > + struct pci_dev *pdev; > + > + ld = priv->plat->bsp_priv; > + pdev = to_pci_dev(priv->device); > + > + mac = devm_kzalloc(priv->device, sizeof(*mac), GFP_KERNEL); > + if (!mac) > + return NULL; > + > + dma = devm_kzalloc(priv->device, sizeof(*dma), GFP_KERNEL); > + if (!dma) > + return NULL; > + > + /* The original IP-core version is v3.73a in all Loongson GNET > + * (LS2K2000 and LS7A2000), but the GNET HW designers have changed the > + * GMAC_VERSION.SNPSVER field to the custom 0x10 value on the Loongson > + * LS2K2000 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. > + */ The comment needs to be altered since the multi-channel feature is no longer GNET-specific. Like this: /* The Loongson GMAC and GNET devices are based on the DW GMAC * v3.50a and v3.73a IP-cores. But the HW designers have changed the * GMAC_VERSION.SNPSVER field to the custom 0x10 value on the * network controllers with the multi-channels feature * available 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. */ > + if (ld->loongson_id == DWMAC_CORE_LS2K2000) { > + priv->synopsys_id = DWMAC_CORE_3_70; > + *dma = dwmac1000_dma_ops; > + dma->init_chan = loongson_gnet_dma_init_channel; > + dma->dma_interrupt = loongson_gnet_dma_interrupt; > + mac->dma = dma; > + } > + > + priv->dev->priv_flags |= IFF_UNICAST_FLT; > + > + /* Pre-initialize the respective "mac" fields as it's done in > + * dwmac1000_setup() > + */ > + mac->pcsr = priv->ioaddr; > + mac->multicast_filter_bins = priv->plat->multicast_filter_bins; > + mac->unicast_filter_entries = priv->plat->unicast_filter_entries; > + mac->mcast_bits_log2 = 0; > + > + if (mac->multicast_filter_bins) > + mac->mcast_bits_log2 = ilog2(mac->multicast_filter_bins); > + > + /* Loongson GMAC doesn't support the flow control. LS2K2000 > + * GNET doesn't support the half-duplex link mode. > + */ > + if (pdev->device == PCI_DEVICE_ID_LOONGSON_GMAC) { > + mac->link.caps = MAC_10 | MAC_100 | MAC_1000; > + } else { > + if (ld->loongson_id == DWMAC_CORE_LS2K2000) > + mac->link.caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE | > + MAC_10 | MAC_100 | MAC_1000; > + else > + mac->link.caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE | > + MAC_10FD | MAC_100FD | MAC_1000FD; > + } AFAIU The only part that is applicable for the multi-channels case of the Loongson GMAC is: + /* Loongson GMAC doesn't support the flow control */ + mac->link.caps = MAC_10 | MAC_100 | MAC_1000; If so the rest of the changes in this chunk should be moved to the new patch adding the Loongson GNET support. > + > + mac->link.duplex = GMAC_CONTROL_DM; > + mac->link.speed10 = GMAC_CONTROL_PS; > + mac->link.speed100 = GMAC_CONTROL_PS | GMAC_CONTROL_FES; > + mac->link.speed1000 = 0; > + mac->link.speed_mask = GMAC_CONTROL_PS | GMAC_CONTROL_FES; > + mac->mii.addr = GMAC_MII_ADDR; > + mac->mii.data = GMAC_MII_DATA; > + mac->mii.addr_shift = 11; > + mac->mii.addr_mask = 0x0000F800; > + mac->mii.reg_shift = 6; > + mac->mii.reg_mask = 0x000007C0; > + mac->mii.clk_csr_shift = 2; > + mac->mii.clk_csr_mask = GENMASK(5, 2); > + > + return mac; > +} > + > static int loongson_dwmac_dt_config(struct pci_dev *pdev, > struct plat_stmmacenet_data *plat, > struct stmmac_resources *res) > @@ -119,6 +474,7 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > struct plat_stmmacenet_data *plat; > struct stmmac_pci_info *info; > struct stmmac_resources res; > + struct loongson_data *ld; > int ret, i; > > plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL); > @@ -135,6 +491,10 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > if (!plat->dma_cfg) > return -ENOMEM; > > + ld = devm_kzalloc(&pdev->dev, sizeof(*ld), GFP_KERNEL); > + if (!ld) > + return -ENOMEM; > + > /* Enable pci device */ > ret = pci_enable_device(pdev); > if (ret) { > @@ -159,19 +519,39 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > > pci_set_master(pdev); > > + plat->bsp_priv = ld; > + plat->setup = loongson_dwmac_setup; > + ld->dev = &pdev->dev; > + Please make sure the chunk above and + ld->loongson_id = readl(res.addr + GMAC_VERSION) & 0xff; are performed before the stmmac_pci_info::setup() is called so the callback could use the private platform data with the loongson_id field initialized. > if (dev_of_node(&pdev->dev)) { > ret = loongson_dwmac_dt_config(pdev, plat, &res); > if (ret) > goto err_disable_device; > - } else { > - res.irq = pdev->irq; > } > > memset(&res, 0, sizeof(res)); I've just realised the memset() call will clear out everything initialized in "res" by the loongson_dwmac_dt_config() method. > res.addr = pcim_iomap_table(pdev)[0]; > + ld->loongson_id = readl(res.addr + GMAC_VERSION) & 0xff; > + > + if (ld->loongson_id == DWMAC_CORE_LS2K2000) { > + plat->rx_queues_to_use = CHANNEL_NUM; > + plat->tx_queues_to_use = CHANNEL_NUM; > + > + /* Only channel 0 supports checksum, > + * so turn off checksum to enable multiple channels. > + */ > + for (i = 1; i < CHANNEL_NUM; i++) > + plat->tx_queues_cfg[i].coe_unsupported = 1; > > - plat->tx_queues_to_use = 1; > - plat->rx_queues_to_use = 1; Please move this part to the loongson_gmac_data() method (and to the loongson_gnet_data() in the new patch). The only code which would be left here is: if (ld->loongson_id == DWMAC_CORE_LOONGSON_MULTI_CH) { ret = loongson_dwmac_msi_config(pdev, plat, &res); if (ret) goto err_disable_device; } > + ret = loongson_dwmac_msi_config(pdev, plat, &res); > + } else { > + plat->tx_queues_to_use = 1; > + plat->rx_queues_to_use = 1; > + > + ret = loongson_dwmac_intx_config(pdev, plat, &res); > + } > + if (ret) > + goto err_disable_device; This won't be needed if you get to implement what is suggested here https://lore.kernel.org/netdev/glm3jfqf36t5vnkmk4gsdqfx53ga7ohs3pxnsizqlogkbim7gg@a3dxav5siczn/ > > ret = stmmac_dvr_probe(&pdev->dev, plat, &res); > if (ret) I don't see the MSI-configs being undone in case of the stmmac_dvr_probe() failure. What should have been done: ret = stmmac_dvr_probe(&pdev->dev, plat, &res); if (ret) goto err_clear_msi; return 0; err_clear_msi: if (ld->loongson_id == DWMAC_CORE_LOONGSON_MULTI_CH) loongson_dwmac_msi_clear(pdev); err_disable_device: pci_disable_device(pdev); ... > @@ -202,6 +582,7 @@ static void loongson_dwmac_remove(struct pci_dev *pdev) > break; > } > > + loongson_dwmac_msi_clear(pdev); Shouldn't this be done for the multi-channels device only? Like this if (ld->loongson_id == DWMAC_CORE_LOONGSON_MULTI_CH) loongson_dwmac_msi_clear(pdev); > pci_disable_device(pdev); > } > > @@ -245,6 +626,7 @@ static SIMPLE_DEV_PM_OPS(loongson_dwmac_pm_ops, loongson_dwmac_suspend, > > 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) }, Please move this to the new patch adding the Loongson GNET support. Sigh... The comments are brutal but if no more unexpected details get to be revealed the respective alterations shall bring us to the coherent set of changes. And hopefully there won't be need in such comprehensive patchset refactoring anymore.( -Serge(y) > {} > }; > MODULE_DEVICE_TABLE(pci, loongson_dwmac_id_table); > -- > 2.31.4 >
On Tue, Jul 2, 2024 at 9:43 PM Serge Semin <fancer.lancer@gmail.com> wrote: > > > [PATCH net-next v13 14/15] net: stmmac: dwmac-loongson: Add Loongson GNET support > > Seeing the multi-channels AV-feature can be found on the Loongson > GMACs too we have to reconsider this patch logic by converting it to > adding the multi-channels support for the Loongson GMAC device only. > Everything Loongson GNET-specific should be moved to a new following > up patch. > > So firstly please change this patch subject to: > [PATCH net-next v14 13/15] net: stmmac: dwmac-loongson: Add Loongson Multi-channels GMAC support > > On Wed, May 29, 2024 at 06:21:09PM +0800, Yanteng Si wrote: > > Aside with the Loongson GMAC controllers which can be normally found > > on the LS2K1000 SoC and LS7A1000 chipset, Loongson released a new > > version of the network controllers called Loongson GNET. It has > > been synthesized into the new generation LS2K2000 SoC and LS7A2000 > > chipset with the next DW GMAC features enabled: > > > > DW GMAC IP-core: v3.73a > > Speeds: 10/100/1000Mbps > > Duplex: Full (both versions), Half (LS2K2000 SoC only) > > DMA-descriptors type: enhanced > > L3/L4 filters availability: Y > > VLAN hash table filter: Y > > PHY-interface: GMII (PHY is integrated into the chips) > > Remote Wake-up support: Y > > Mac Management Counters (MMC): Y > > Number of additional MAC addresses: 5 > > MAC Hash-based filter: Y > > Hash Table Size: 256 > > AV feature: Y (LS2K2000 SoC only) > > DMA channels: 8 (LS2K2000 SoC), 1 (LS7A2000 chipset) > > > > The integrated PHY has a weird problem with switching from the low > > speeds to 1000Mbps mode. The speedup procedure requires the PHY-link > > re-negotiation. Besides the LS2K2000 GNET controller the next > > peculiarities: > > 1. Split up Tx and Rx DMA IRQ status/mask bits: > > Name Tx Rx > > DMA_INTR_ENA_NIE = 0x00040000 | 0x00020000; > > DMA_INTR_ENA_AIE = 0x00010000 | 0x00008000; > > DMA_STATUS_NIS = 0x00040000 | 0x00020000; > > DMA_STATUS_AIS = 0x00010000 | 0x00008000; > > DMA_STATUS_FBI = 0x00002000 | 0x00001000; > > 2. Custom Synopsys ID hardwired into the GMAC_VERSION.SNPSVER field. > > It's 0x10 while it should have been 0x37 in accordance with the actual > > DW GMAC IP-core version. > > > > Thus in order to have the Loongson GNET controllers supported let's > > modify the Loongson DWMAC driver in accordance with all the > > peculiarities described above: > > > > 1. Create the Loongson GNET-specific > > stmmac_dma_ops::dma_interrupt() > > stmmac_dma_ops::init_chan() > > callbacks due to the non-standard DMA IRQ CSR flags layout. > > 2. Create the Loongson GNET-specific platform setup() method which > > gets to initialize the DMA-ops with the dwmac1000_dma_ops instance > > and overrides the callbacks described in 1, and overrides the custom > > Synopsys ID with the real one in order to have the rest of the > > HW-specific callbacks correctly detected by the driver core. > > 3. Make sure the Loongson GNET-specific platform setup() method > > enables the duplex modes supported by the controller. > > 4. Provide the plat_stmmacenet_data::fix_mac_speed() callback which > > will restart the link Auto-negotiation in case of the speed change. > > Please convert the commit log of this patch to containing the > multi-channels feature description only. Like this (correct me if I am > wrong in understanding some of the Loongson network controllers > aspects): > > "The Loongson DWMAC driver currently supports the Loongson GMAC > devices (based on the DW GMAC v3.50a/v3.73a IP-core) installed to the > LS2K1000 SoC and LS7A1000 chipset. But recently a new generation > LS2K2000 SoC was released with the new version of the Loongson GMAC > synthesized in. The new controller is based on the DW GMAC v3.73a > IP-core with the AV-feature enabled, which implies the multi > DMA-channels support. The multi DMA-channels feature has the next > vendor-specific peculiarities: > > 1. Split up Tx and Rx DMA IRQ status/mask bits: > Name Tx Rx > DMA_INTR_ENA_NIE = 0x00040000 | 0x00020000; > DMA_INTR_ENA_AIE = 0x00010000 | 0x00008000; > DMA_STATUS_NIS = 0x00040000 | 0x00020000; > DMA_STATUS_AIS = 0x00010000 | 0x00008000; > DMA_STATUS_FBI = 0x00002000 | 0x00001000; > 2. Custom Synopsys ID hardwired into the GMAC_VERSION.SNPSVER register > field. It's 0x10 while it should have been 0x37 in accordance with > the actual DW GMAC IP-core version. > 3. There are eight DMA-channels available meanwhile the Synopsys DW > GMAC IP-core supports up to three DMA-channels. > 4. It's possible to have each DMA-channel IRQ independently delivered. > The MSI IRQs must be utilized for that. > > Thus in order to have the multi-channels Loongson GMAC controllers > supported let's modify the Loongson DWMAC driver in accordance with > all the peculiarities described above: > > 1. Create the multi-channels Loongson GMAC-specific > stmmac_dma_ops::dma_interrupt() > stmmac_dma_ops::init_chan() > callbacks due to the non-standard DMA IRQ CSR flags layout. > 2. Create the Loongson DWMAC-specific platform setup() method > which gets to initialize the DMA-ops with the dwmac1000_dma_ops > instance and overrides the callbacks described in 1. The method also > overrides the custom Synopsys ID with the real one in order to have > the rest of the HW-specific callbacks correctly detected by the driver > core. > 3. Make sure the platform setup() method enables the flow control and > duplex modes supported by the controller. > " > > Once again, please correct the text if I was wrong in understanding > the way your devices work. Especially regarding the flow-control and > half-duplex mode. > > --- > > The Loongson GNET-specific changes should be moved to the new patch > applied after this one. The subject of the new patch should be the same > as the former subject of this patch: > [PATCH net-next v14 14/15] net: stmmac: dwmac-loongson: Add Loongson GNET support > but of course the commit log shall be different. Like this: > > "The new generation Loongson LS2K2000 SoC and LS7A2000 chipset are > equipped with the network controllers called Loongson GNET. It's the > single and multi DMA-channels Loongson GMAC but with a PHY attached. > Here is the summary of the DW GMAC features the controller has: > > DW GMAC IP-core: v3.73a > Speeds: 10/100/1000Mbps > Duplex: Full (both versions), Half (LS2K2000 GNET only) > DMA-descriptors type: enhanced > L3/L4 filters availability: Y > VLAN hash table filter: Y > PHY-interface: GMII (PHY is integrated into the chips) > Remote Wake-up support: Y > Mac Management Counters (MMC): Y > Number of additional MAC addresses: 5 > MAC Hash-based filter: Y > Hash Table Size: 256 > AV feature: Y (LS2K2000 GNET only) > DMA channels: 8 (LS2K2000 GNET), 1 (LS7A2000 GNET) > > Let's update the Loongson DWMAC driver to supporting the new Loongson > GNET controller. The change is mainly trivial: the driver shall be > bound to the PCIe device with DID 0x7a13, and the device-specific > setup() method shall be called for it. The only peculiarity concerns > the integrated PHY speed change procedure. The PHY has a weird problem > with switching from the low speeds to 1000Mbps mode. The speedup > procedure requires the PHY-link re-negotiation. So the suggested > change provide the device-specific fix_mac_speed() method to overcome > the problem." > > > > > Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn> > > Signed-off-by: Yinggang Gu <guyinggang@loongson.cn> > > Signed-off-by: Yanteng Si <siyanteng@loongson.cn> > > --- > > drivers/net/ethernet/stmicro/stmmac/common.h | 1 + > > .../ethernet/stmicro/stmmac/dwmac-loongson.c | 390 +++++++++++++++++- > > 2 files changed, 387 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h > > index 9cd62b2110a1..aed6ae80cc7c 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/common.h > > +++ b/drivers/net/ethernet/stmicro/stmmac/common.h > > @@ -29,6 +29,7 @@ > > /* Synopsys Core versions */ > > #define DWMAC_CORE_3_40 0x34 > > #define DWMAC_CORE_3_50 0x35 > > +#define DWMAC_CORE_3_70 0x37 > > #define DWMAC_CORE_4_00 0x40 > > #define DWMAC_CORE_4_10 0x41 > > #define DWMAC_CORE_5_00 0x50 > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > > index 45dcc35b7955..559215e3fe41 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > > @@ -8,8 +8,71 @@ > > #include <linux/device.h> > > #include <linux/of_irq.h> > > #include "stmmac.h" > > +#include "dwmac_dma.h" > > +#include "dwmac1000.h" > > + > > +/* Normal Loongson Tx Summary */ > > +#define DMA_INTR_ENA_NIE_TX_LOONGSON 0x00040000 > > +/* Normal Loongson Rx Summary */ > > +#define DMA_INTR_ENA_NIE_RX_LOONGSON 0x00020000 > > + > > +#define DMA_INTR_NORMAL_LOONGSON (DMA_INTR_ENA_NIE_TX_LOONGSON | \ > > + DMA_INTR_ENA_NIE_RX_LOONGSON | \ > > + DMA_INTR_ENA_RIE | DMA_INTR_ENA_TIE) > > + > > +/* Abnormal Loongson Tx Summary */ > > +#define DMA_INTR_ENA_AIE_TX_LOONGSON 0x00010000 > > +/* Abnormal Loongson Rx Summary */ > > +#define DMA_INTR_ENA_AIE_RX_LOONGSON 0x00008000 > > + > > +#define DMA_INTR_ABNORMAL_LOONGSON (DMA_INTR_ENA_AIE_TX_LOONGSON | \ > > + DMA_INTR_ENA_AIE_RX_LOONGSON | \ > > + DMA_INTR_ENA_FBE | DMA_INTR_ENA_UNE) > > + > > +#define DMA_INTR_DEFAULT_MASK_LOONGSON (DMA_INTR_NORMAL_LOONGSON | \ > > + DMA_INTR_ABNORMAL_LOONGSON) > > + > > +/* Normal Loongson Tx Interrupt Summary */ > > +#define DMA_STATUS_NIS_TX_LOONGSON 0x00040000 > > +/* Normal Loongson Rx Interrupt Summary */ > > +#define DMA_STATUS_NIS_RX_LOONGSON 0x00020000 > > + > > +/* Abnormal Loongson Tx Interrupt Summary */ > > +#define DMA_STATUS_AIS_TX_LOONGSON 0x00010000 > > +/* Abnormal Loongson Rx Interrupt Summary */ > > +#define DMA_STATUS_AIS_RX_LOONGSON 0x00008000 > > + > > +/* Fatal Loongson Tx Bus Error Interrupt */ > > +#define DMA_STATUS_FBI_TX_LOONGSON 0x00002000 > > +/* Fatal Loongson Rx Bus Error Interrupt */ > > +#define DMA_STATUS_FBI_RX_LOONGSON 0x00001000 > > + > > +#define DMA_STATUS_MSK_COMMON_LOONGSON (DMA_STATUS_NIS_TX_LOONGSON | \ > > + DMA_STATUS_NIS_RX_LOONGSON | \ > > + DMA_STATUS_AIS_TX_LOONGSON | \ > > + DMA_STATUS_AIS_RX_LOONGSON | \ > > + DMA_STATUS_FBI_TX_LOONGSON | \ > > + DMA_STATUS_FBI_RX_LOONGSON) > > + > > +#define DMA_STATUS_MSK_RX_LOONGSON (DMA_STATUS_ERI | DMA_STATUS_RWT | \ > > + DMA_STATUS_RPS | DMA_STATUS_RU | \ > > + DMA_STATUS_RI | DMA_STATUS_OVF | \ > > + DMA_STATUS_MSK_COMMON_LOONGSON) > > + > > +#define DMA_STATUS_MSK_TX_LOONGSON (DMA_STATUS_ETI | DMA_STATUS_UNF | \ > > + DMA_STATUS_TJT | DMA_STATUS_TU | \ > > + DMA_STATUS_TPS | DMA_STATUS_TI | \ > > + DMA_STATUS_MSK_COMMON_LOONGSON) > > > > #define PCI_DEVICE_ID_LOONGSON_GMAC 0x7a03 > > > +#define PCI_DEVICE_ID_LOONGSON_GNET 0x7a13 > > Please move this to the new patch adding the Loongson GNET support. > > > +#define DWMAC_CORE_LS2K2000 0x10 /* Loongson custom IP */ > > Note it's perfectly fine to have a device named after the SoC it's > equipped to. For example see the compatible strings defined for the > vendor-specific versions of the DW *MAC IP-cores: > Documentation/devicetree/bindings/net/snps,dwmac.yaml > > But if you aren't comfortable with such naming we can change the > macro to something like: > #define DWMAC_CORE_LOONGSON_MULTI_CH 0x10 Maybe DWMAC_CORE_LOONGSON_MULTICHAN or DWMAC_CORE_LOONGSON_MULTI_CHAN is a little better? Huacai > > > +#define CHANNEL_NUM 8 > > + > > +struct loongson_data { > > + u32 loongson_id; > > > + struct device *dev; > > Please add the "dev" field in the new patch adding the Loongson GNET support. > > > +}; > > > > struct stmmac_pci_info { > > int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat); > > @@ -67,6 +130,298 @@ static struct stmmac_pci_info loongson_gmac_pci_info = { > > .setup = loongson_gmac_data, > > }; > > > > > +static void loongson_gnet_dma_init_channel(struct stmmac_priv *priv, > > Since the multi-channel feature is no longer GNET-specific, please > rename the method prefix to loongson_dwmac_ . > > > + void __iomem *ioaddr, > > + struct stmmac_dma_cfg *dma_cfg, > > + u32 chan) > > +{ > > + int txpbl = dma_cfg->txpbl ?: dma_cfg->pbl; > > + int rxpbl = dma_cfg->rxpbl ?: dma_cfg->pbl; > > + u32 value; > > + > > + value = readl(ioaddr + DMA_CHAN_BUS_MODE(chan)); > > + > > + if (dma_cfg->pblx8) > > + value |= DMA_BUS_MODE_MAXPBL; > > + > > + value |= DMA_BUS_MODE_USP; > > + value &= ~(DMA_BUS_MODE_PBL_MASK | DMA_BUS_MODE_RPBL_MASK); > > + value |= (txpbl << DMA_BUS_MODE_PBL_SHIFT); > > + value |= (rxpbl << DMA_BUS_MODE_RPBL_SHIFT); > > + > > + /* Set the Fixed burst mode */ > > + if (dma_cfg->fixed_burst) > > + value |= DMA_BUS_MODE_FB; > > + > > + /* Mixed Burst has no effect when fb is set */ > > + if (dma_cfg->mixed_burst) > > + value |= DMA_BUS_MODE_MB; > > + > > + if (dma_cfg->atds) > > + value |= DMA_BUS_MODE_ATDS; > > + > > + if (dma_cfg->aal) > > + value |= DMA_BUS_MODE_AAL; > > + > > + writel(value, ioaddr + DMA_CHAN_BUS_MODE(chan)); > > + > > + /* Mask interrupts by writing to CSR7 */ > > + writel(DMA_INTR_DEFAULT_MASK_LOONGSON, ioaddr + > > + DMA_CHAN_INTR_ENA(chan)); > > +} > > + > > > +static int loongson_gnet_dma_interrupt(struct stmmac_priv *priv, > > Similarly the loongson_dwmac_ prefix seems more appropriate now. > > > + void __iomem *ioaddr, > > + struct stmmac_extra_stats *x, > > + u32 chan, u32 dir) > > +{ > > + struct stmmac_pcpu_stats *stats = this_cpu_ptr(priv->xstats.pcpu_stats); > > + u32 abnor_intr_status; > > + u32 nor_intr_status; > > + u32 fb_intr_status; > > + u32 intr_status; > > + int ret = 0; > > + > > + /* read the status register (CSR5) */ > > + intr_status = readl(ioaddr + DMA_CHAN_STATUS(chan)); > > + > > + if (dir == DMA_DIR_RX) > > + intr_status &= DMA_STATUS_MSK_RX_LOONGSON; > > + else if (dir == DMA_DIR_TX) > > + intr_status &= DMA_STATUS_MSK_TX_LOONGSON; > > + > > + nor_intr_status = intr_status & (DMA_STATUS_NIS_TX_LOONGSON | > > + DMA_STATUS_NIS_RX_LOONGSON); > > + abnor_intr_status = intr_status & (DMA_STATUS_AIS_TX_LOONGSON | > > + DMA_STATUS_AIS_RX_LOONGSON); > > + fb_intr_status = intr_status & (DMA_STATUS_FBI_TX_LOONGSON | > > + DMA_STATUS_FBI_RX_LOONGSON); > > + > > + /* ABNORMAL interrupts */ > > + if (unlikely(abnor_intr_status)) { > > + if (unlikely(intr_status & DMA_STATUS_UNF)) { > > + ret = tx_hard_error_bump_tc; > > + x->tx_undeflow_irq++; > > + } > > + if (unlikely(intr_status & DMA_STATUS_TJT)) > > + x->tx_jabber_irq++; > > + if (unlikely(intr_status & DMA_STATUS_OVF)) > > + x->rx_overflow_irq++; > > + if (unlikely(intr_status & DMA_STATUS_RU)) > > + x->rx_buf_unav_irq++; > > + if (unlikely(intr_status & DMA_STATUS_RPS)) > > + x->rx_process_stopped_irq++; > > + if (unlikely(intr_status & DMA_STATUS_RWT)) > > + x->rx_watchdog_irq++; > > + if (unlikely(intr_status & DMA_STATUS_ETI)) > > + x->tx_early_irq++; > > + if (unlikely(intr_status & DMA_STATUS_TPS)) { > > + x->tx_process_stopped_irq++; > > + ret = tx_hard_error; > > + } > > + if (unlikely(fb_intr_status)) { > > + x->fatal_bus_error_irq++; > > + ret = tx_hard_error; > > + } > > + } > > + /* TX/RX NORMAL interrupts */ > > + if (likely(nor_intr_status)) { > > + if (likely(intr_status & DMA_STATUS_RI)) { > > + u32 value = readl(ioaddr + DMA_INTR_ENA); > > + /* to schedule NAPI on real RIE event. */ > > + if (likely(value & DMA_INTR_ENA_RIE)) { > > + u64_stats_update_begin(&stats->syncp); > > + u64_stats_inc(&stats->rx_normal_irq_n[chan]); > > + u64_stats_update_end(&stats->syncp); > > + ret |= handle_rx; > > + } > > + } > > + if (likely(intr_status & DMA_STATUS_TI)) { > > + u64_stats_update_begin(&stats->syncp); > > + u64_stats_inc(&stats->tx_normal_irq_n[chan]); > > + u64_stats_update_end(&stats->syncp); > > + ret |= handle_tx; > > + } > > + if (unlikely(intr_status & DMA_STATUS_ERI)) > > + x->rx_early_irq++; > > + } > > + /* Optional hardware blocks, interrupts should be disabled */ > > + if (unlikely(intr_status & > > + (DMA_STATUS_GPI | DMA_STATUS_GMI | DMA_STATUS_GLI))) > > + pr_warn("%s: unexpected status %08x\n", __func__, intr_status); > > + > > + /* Clear the interrupt by writing a logic 1 to the CSR5[19-0] */ > > + writel((intr_status & 0x7ffff), ioaddr + DMA_CHAN_STATUS(chan)); > > + > > + return ret; > > +} > > + > > > > +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 integrated PHY has a weird problem with switching from the low > > + * speeds to 1000Mbps mode. The speedup procedure requires the PHY-link > > + * re-negotiation. > > + */ > > + if (speed == SPEED_1000) { > > + if (readl(ptr->ioaddr + MAC_CTRL_REG) & > > + GMAC_CONTROL_PS) > > + /* Word around hardware bug, restart autoneg */ > > + phy_restart_aneg(ndev->phydev); > > + } > > +} > > + > > +static int loongson_gnet_data(struct pci_dev *pdev, > > + struct plat_stmmacenet_data *plat) > > +{ > > + loongson_default_data(pdev, plat); > > + > > + plat->phy_interface = PHY_INTERFACE_MODE_GMII; > > + plat->mdio_bus_data->phy_mask = ~(u32)BIT(2); > > + plat->fix_mac_speed = loongson_gnet_fix_speed; > > + > > + /* GNET devices with dev revision 0x00 do not support manually > > + * setting the speed to 1000. > > + */ > > + if (pdev->revision == 0x00) > > + plat->flags |= STMMAC_FLAG_DISABLE_FORCE_1000; > > + > > + return 0; > > +} > > + > > +static struct stmmac_pci_info loongson_gnet_pci_info = { > > + .setup = loongson_gnet_data, > > +}; > > Please move this to the new patch adding the Loongson GNET support. > > > + > > +static int loongson_dwmac_intx_config(struct pci_dev *pdev, > > + struct plat_stmmacenet_data *plat, > > + struct stmmac_resources *res) > > +{ > > + res->irq = pdev->irq; > > + > > + return 0; > > +} > > If you get to implement what suggested here > https://lore.kernel.org/netdev/glm3jfqf36t5vnkmk4gsdqfx53ga7ohs3pxnsizqlogkbim7gg@a3dxav5siczn/ > than the loongson_dwmac_intx_config() won't be needed. > > > + > > +static int loongson_dwmac_msi_config(struct pci_dev *pdev, > > + struct plat_stmmacenet_data *plat, > > + struct stmmac_resources *res) > > +{ > > + int i, ret, vecs; > > + > > + vecs = roundup_pow_of_two(CHANNEL_NUM * 2 + 1); > > + ret = pci_alloc_irq_vectors(pdev, 1, vecs, PCI_IRQ_MSI | PCI_IRQ_INTX); > > + if (ret < 0) { > > + dev_err(&pdev->dev, "Failed to allocate PCI IRQs\n"); > > + return ret; > > + } > > + > > + if (ret >= vecs) { > > + for (i = 0; i < plat->rx_queues_to_use; i++) { > > + res->rx_irq[CHANNEL_NUM - 1 - i] = > > + pci_irq_vector(pdev, 1 + i * 2); > > + } > > + for (i = 0; i < plat->tx_queues_to_use; i++) { > > + res->tx_irq[CHANNEL_NUM - 1 - i] = > > + pci_irq_vector(pdev, 2 + i * 2); > > + } > > + > > + plat->flags |= STMMAC_FLAG_MULTI_MSI_EN; > > + } > > + > > + res->irq = pci_irq_vector(pdev, 0); > > + > > + return 0; > > +} > > Could you please clarify whether the multi-channel feature will work > if the pci_alloc_irq_vectors() method failed to allocated as many MSI > vectors as there are channels? > > > + > > +static int loongson_dwmac_msi_clear(struct pci_dev *pdev) > > +{ > > + pci_free_irq_vectors(pdev); > > + > > + return 0; > > +} > > + > > +static struct mac_device_info *loongson_dwmac_setup(void *apriv) > > +{ > > + struct stmmac_priv *priv = apriv; > > + struct mac_device_info *mac; > > + struct stmmac_dma_ops *dma; > > + struct loongson_data *ld; > > + struct pci_dev *pdev; > > + > > + ld = priv->plat->bsp_priv; > > + pdev = to_pci_dev(priv->device); > > + > > + mac = devm_kzalloc(priv->device, sizeof(*mac), GFP_KERNEL); > > + if (!mac) > > + return NULL; > > + > > + dma = devm_kzalloc(priv->device, sizeof(*dma), GFP_KERNEL); > > + if (!dma) > > + return NULL; > > + > > > + /* The original IP-core version is v3.73a in all Loongson GNET > > + * (LS2K2000 and LS7A2000), but the GNET HW designers have changed the > > + * GMAC_VERSION.SNPSVER field to the custom 0x10 value on the Loongson > > + * LS2K2000 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. > > + */ > > The comment needs to be altered since the multi-channel feature is no > longer GNET-specific. Like this: > > /* The Loongson GMAC and GNET devices are based on the DW GMAC > * v3.50a and v3.73a IP-cores. But the HW designers have changed the > * GMAC_VERSION.SNPSVER field to the custom 0x10 value on the > * network controllers with the multi-channels feature > * available 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. > */ > > > + if (ld->loongson_id == DWMAC_CORE_LS2K2000) { > > + priv->synopsys_id = DWMAC_CORE_3_70; > > + *dma = dwmac1000_dma_ops; > > + dma->init_chan = loongson_gnet_dma_init_channel; > > + dma->dma_interrupt = loongson_gnet_dma_interrupt; > > + mac->dma = dma; > > + } > > + > > + priv->dev->priv_flags |= IFF_UNICAST_FLT; > > + > > + /* Pre-initialize the respective "mac" fields as it's done in > > + * dwmac1000_setup() > > + */ > > + mac->pcsr = priv->ioaddr; > > + mac->multicast_filter_bins = priv->plat->multicast_filter_bins; > > + mac->unicast_filter_entries = priv->plat->unicast_filter_entries; > > + mac->mcast_bits_log2 = 0; > > + > > + if (mac->multicast_filter_bins) > > + mac->mcast_bits_log2 = ilog2(mac->multicast_filter_bins); > > + > > > + /* Loongson GMAC doesn't support the flow control. LS2K2000 > > + * GNET doesn't support the half-duplex link mode. > > + */ > > + if (pdev->device == PCI_DEVICE_ID_LOONGSON_GMAC) { > > + mac->link.caps = MAC_10 | MAC_100 | MAC_1000; > > + } else { > > + if (ld->loongson_id == DWMAC_CORE_LS2K2000) > > + mac->link.caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE | > > + MAC_10 | MAC_100 | MAC_1000; > > + else > > + mac->link.caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE | > > + MAC_10FD | MAC_100FD | MAC_1000FD; > > + } > > AFAIU The only part that is applicable for the multi-channels case of > the Loongson GMAC is: > + /* Loongson GMAC doesn't support the flow control */ > + mac->link.caps = MAC_10 | MAC_100 | MAC_1000; > > If so the rest of the changes in this chunk should be moved to the new > patch adding the Loongson GNET support. > > > + > > + mac->link.duplex = GMAC_CONTROL_DM; > > + mac->link.speed10 = GMAC_CONTROL_PS; > > + mac->link.speed100 = GMAC_CONTROL_PS | GMAC_CONTROL_FES; > > + mac->link.speed1000 = 0; > > + mac->link.speed_mask = GMAC_CONTROL_PS | GMAC_CONTROL_FES; > > + mac->mii.addr = GMAC_MII_ADDR; > > + mac->mii.data = GMAC_MII_DATA; > > + mac->mii.addr_shift = 11; > > + mac->mii.addr_mask = 0x0000F800; > > + mac->mii.reg_shift = 6; > > + mac->mii.reg_mask = 0x000007C0; > > + mac->mii.clk_csr_shift = 2; > > + mac->mii.clk_csr_mask = GENMASK(5, 2); > > + > > + return mac; > > +} > > + > > static int loongson_dwmac_dt_config(struct pci_dev *pdev, > > struct plat_stmmacenet_data *plat, > > struct stmmac_resources *res) > > @@ -119,6 +474,7 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > > struct plat_stmmacenet_data *plat; > > struct stmmac_pci_info *info; > > struct stmmac_resources res; > > + struct loongson_data *ld; > > int ret, i; > > > > plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL); > > @@ -135,6 +491,10 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > > if (!plat->dma_cfg) > > return -ENOMEM; > > > > + ld = devm_kzalloc(&pdev->dev, sizeof(*ld), GFP_KERNEL); > > + if (!ld) > > + return -ENOMEM; > > + > > /* Enable pci device */ > > ret = pci_enable_device(pdev); > > if (ret) { > > @@ -159,19 +519,39 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > > > > pci_set_master(pdev); > > > > > + plat->bsp_priv = ld; > > + plat->setup = loongson_dwmac_setup; > > + ld->dev = &pdev->dev; > > + > > Please make sure the chunk above and > + ld->loongson_id = readl(res.addr + GMAC_VERSION) & 0xff; > are performed before the stmmac_pci_info::setup() is called so the > callback could use the private platform data with the loongson_id > field initialized. > > > if (dev_of_node(&pdev->dev)) { > > ret = loongson_dwmac_dt_config(pdev, plat, &res); > > if (ret) > > goto err_disable_device; > > - } else { > > - res.irq = pdev->irq; > > } > > > > memset(&res, 0, sizeof(res)); > > I've just realised the memset() call will clear out everything > initialized in "res" by the loongson_dwmac_dt_config() method. > > > res.addr = pcim_iomap_table(pdev)[0]; > > + ld->loongson_id = readl(res.addr + GMAC_VERSION) & 0xff; > > + > > > > + if (ld->loongson_id == DWMAC_CORE_LS2K2000) { > > + plat->rx_queues_to_use = CHANNEL_NUM; > > + plat->tx_queues_to_use = CHANNEL_NUM; > > + > > + /* Only channel 0 supports checksum, > > + * so turn off checksum to enable multiple channels. > > + */ > > + for (i = 1; i < CHANNEL_NUM; i++) > > + plat->tx_queues_cfg[i].coe_unsupported = 1; > > > > - plat->tx_queues_to_use = 1; > > - plat->rx_queues_to_use = 1; > > Please move this part to the loongson_gmac_data() method (and to the > loongson_gnet_data() in the new patch). The only > code which would be left here is: > if (ld->loongson_id == DWMAC_CORE_LOONGSON_MULTI_CH) { > ret = loongson_dwmac_msi_config(pdev, plat, &res); > if (ret) > goto err_disable_device; > } > > > + ret = loongson_dwmac_msi_config(pdev, plat, &res); > > > + } else { > > + plat->tx_queues_to_use = 1; > > + plat->rx_queues_to_use = 1; > > + > > + ret = loongson_dwmac_intx_config(pdev, plat, &res); > > + } > > + if (ret) > > + goto err_disable_device; > > This won't be needed if you get to implement what is suggested here > https://lore.kernel.org/netdev/glm3jfqf36t5vnkmk4gsdqfx53ga7ohs3pxnsizqlogkbim7gg@a3dxav5siczn/ > > > > > ret = stmmac_dvr_probe(&pdev->dev, plat, &res); > > if (ret) > > I don't see the MSI-configs being undone in case of the > stmmac_dvr_probe() failure. What should have been done: > > ret = stmmac_dvr_probe(&pdev->dev, plat, &res); > if (ret) > goto err_clear_msi; > > return 0; > > err_clear_msi: > if (ld->loongson_id == DWMAC_CORE_LOONGSON_MULTI_CH) > loongson_dwmac_msi_clear(pdev); > > err_disable_device: > pci_disable_device(pdev); > > ... > > > @@ -202,6 +582,7 @@ static void loongson_dwmac_remove(struct pci_dev *pdev) > > break; > > } > > > > > + loongson_dwmac_msi_clear(pdev); > > Shouldn't this be done for the multi-channels device only? Like this > if (ld->loongson_id == DWMAC_CORE_LOONGSON_MULTI_CH) > loongson_dwmac_msi_clear(pdev); > > > pci_disable_device(pdev); > > } > > > > @@ -245,6 +626,7 @@ static SIMPLE_DEV_PM_OPS(loongson_dwmac_pm_ops, loongson_dwmac_suspend, > > > > 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) }, > > Please move this to the new patch adding the Loongson GNET support. > > > > Sigh... The comments are brutal but if no more unexpected details get > to be revealed the respective alterations shall bring us to the > coherent set of changes. And hopefully there won't be need in such > comprehensive patchset refactoring anymore.( > > -Serge(y) > > > {} > > }; > > MODULE_DEVICE_TABLE(pci, loongson_dwmac_id_table); > > -- > > 2.31.4 > >
在 2024/7/2 21:43, Serge Semin 写道: >> [PATCH net-next v13 14/15] net: stmmac: dwmac-loongson: Add Loongson GNET support > Seeing the multi-channels AV-feature can be found on the Loongson > GMACs too we have to reconsider this patch logic by converting it to > adding the multi-channels support for the Loongson GMAC device only. > Everything Loongson GNET-specific should be moved to a new following > up patch. > > So firstly please change this patch subject to: > [PATCH net-next v14 13/15] net: stmmac: dwmac-loongson: Add Loongson Multi-channels GMAC support > > On Wed, May 29, 2024 at 06:21:09PM +0800, Yanteng Si wrote: >> Aside with the Loongson GMAC controllers which can be normally found >> on the LS2K1000 SoC and LS7A1000 chipset, Loongson released a new >> version of the network controllers called Loongson GNET. It has >> been synthesized into the new generation LS2K2000 SoC and LS7A2000 >> chipset with the next DW GMAC features enabled: >> >> DW GMAC IP-core: v3.73a >> Speeds: 10/100/1000Mbps >> Duplex: Full (both versions), Half (LS2K2000 SoC only) >> DMA-descriptors type: enhanced >> L3/L4 filters availability: Y >> VLAN hash table filter: Y >> PHY-interface: GMII (PHY is integrated into the chips) >> Remote Wake-up support: Y >> Mac Management Counters (MMC): Y >> Number of additional MAC addresses: 5 >> MAC Hash-based filter: Y >> Hash Table Size: 256 >> AV feature: Y (LS2K2000 SoC only) >> DMA channels: 8 (LS2K2000 SoC), 1 (LS7A2000 chipset) >> >> The integrated PHY has a weird problem with switching from the low >> speeds to 1000Mbps mode. The speedup procedure requires the PHY-link >> re-negotiation. Besides the LS2K2000 GNET controller the next >> peculiarities: >> 1. Split up Tx and Rx DMA IRQ status/mask bits: >> Name Tx Rx >> DMA_INTR_ENA_NIE = 0x00040000 | 0x00020000; >> DMA_INTR_ENA_AIE = 0x00010000 | 0x00008000; >> DMA_STATUS_NIS = 0x00040000 | 0x00020000; >> DMA_STATUS_AIS = 0x00010000 | 0x00008000; >> DMA_STATUS_FBI = 0x00002000 | 0x00001000; >> 2. Custom Synopsys ID hardwired into the GMAC_VERSION.SNPSVER field. >> It's 0x10 while it should have been 0x37 in accordance with the actual >> DW GMAC IP-core version. >> >> Thus in order to have the Loongson GNET controllers supported let's >> modify the Loongson DWMAC driver in accordance with all the >> peculiarities described above: >> >> 1. Create the Loongson GNET-specific >> stmmac_dma_ops::dma_interrupt() >> stmmac_dma_ops::init_chan() >> callbacks due to the non-standard DMA IRQ CSR flags layout. >> 2. Create the Loongson GNET-specific platform setup() method which >> gets to initialize the DMA-ops with the dwmac1000_dma_ops instance >> and overrides the callbacks described in 1, and overrides the custom >> Synopsys ID with the real one in order to have the rest of the >> HW-specific callbacks correctly detected by the driver core. >> 3. Make sure the Loongson GNET-specific platform setup() method >> enables the duplex modes supported by the controller. >> 4. Provide the plat_stmmacenet_data::fix_mac_speed() callback which >> will restart the link Auto-negotiation in case of the speed change. > Please convert the commit log of this patch to containing the > multi-channels feature description only. Like this (correct me if I am > wrong in understanding some of the Loongson network controllers > aspects): > > "The Loongson DWMAC driver currently supports the Loongson GMAC > devices (based on the DW GMAC v3.50a/v3.73a IP-core) installed to the > LS2K1000 SoC and LS7A1000 chipset. But recently a new generation > LS2K2000 SoC was released with the new version of the Loongson GMAC > synthesized in. The new controller is based on the DW GMAC v3.73a > IP-core with the AV-feature enabled, which implies the multi > DMA-channels support. The multi DMA-channels feature has the next > vendor-specific peculiarities: > > 1. Split up Tx and Rx DMA IRQ status/mask bits: > Name Tx Rx > DMA_INTR_ENA_NIE = 0x00040000 | 0x00020000; > DMA_INTR_ENA_AIE = 0x00010000 | 0x00008000; > DMA_STATUS_NIS = 0x00040000 | 0x00020000; > DMA_STATUS_AIS = 0x00010000 | 0x00008000; > DMA_STATUS_FBI = 0x00002000 | 0x00001000; > 2. Custom Synopsys ID hardwired into the GMAC_VERSION.SNPSVER register > field. It's 0x10 while it should have been 0x37 in accordance with > the actual DW GMAC IP-core version. > 3. There are eight DMA-channels available meanwhile the Synopsys DW > GMAC IP-core supports up to three DMA-channels. > 4. It's possible to have each DMA-channel IRQ independently delivered. > The MSI IRQs must be utilized for that. > > Thus in order to have the multi-channels Loongson GMAC controllers > supported let's modify the Loongson DWMAC driver in accordance with > all the peculiarities described above: > > 1. Create the multi-channels Loongson GMAC-specific > stmmac_dma_ops::dma_interrupt() > stmmac_dma_ops::init_chan() > callbacks due to the non-standard DMA IRQ CSR flags layout. > 2. Create the Loongson DWMAC-specific platform setup() method > which gets to initialize the DMA-ops with the dwmac1000_dma_ops > instance and overrides the callbacks described in 1. The method also > overrides the custom Synopsys ID with the real one in order to have > the rest of the HW-specific callbacks correctly detected by the driver > core. > 3. Make sure the platform setup() method enables the flow control and > duplex modes supported by the controller. > " > > Once again, please correct the text if I was wrong in understanding > the way your devices work. Especially regarding the flow-control and > half-duplex mode. > > --- > > The Loongson GNET-specific changes should be moved to the new patch > applied after this one. The subject of the new patch should be the same > as the former subject of this patch: > [PATCH net-next v14 14/15] net: stmmac: dwmac-loongson: Add Loongson GNET support > but of course the commit log shall be different. Like this: > > "The new generation Loongson LS2K2000 SoC and LS7A2000 chipset are > equipped with the network controllers called Loongson GNET. It's the > single and multi DMA-channels Loongson GMAC but with a PHY attached. > Here is the summary of the DW GMAC features the controller has: > > DW GMAC IP-core: v3.73a > Speeds: 10/100/1000Mbps > Duplex: Full (both versions), Half (LS2K2000 GNET only) > DMA-descriptors type: enhanced > L3/L4 filters availability: Y > VLAN hash table filter: Y > PHY-interface: GMII (PHY is integrated into the chips) > Remote Wake-up support: Y > Mac Management Counters (MMC): Y > Number of additional MAC addresses: 5 > MAC Hash-based filter: Y > Hash Table Size: 256 > AV feature: Y (LS2K2000 GNET only) > DMA channels: 8 (LS2K2000 GNET), 1 (LS7A2000 GNET) > > Let's update the Loongson DWMAC driver to supporting the new Loongson > GNET controller. The change is mainly trivial: the driver shall be > bound to the PCIe device with DID 0x7a13, and the device-specific > setup() method shall be called for it. The only peculiarity concerns > the integrated PHY speed change procedure. The PHY has a weird problem > with switching from the low speeds to 1000Mbps mode. The speedup > procedure requires the PHY-link re-negotiation. So the suggested > change provide the device-specific fix_mac_speed() method to overcome > the problem." OK. > >> Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn> >> Signed-off-by: Yinggang Gu <guyinggang@loongson.cn> >> Signed-off-by: Yanteng Si <siyanteng@loongson.cn> >> --- >> drivers/net/ethernet/stmicro/stmmac/common.h | 1 + >> .../ethernet/stmicro/stmmac/dwmac-loongson.c | 390 +++++++++++++++++- >> 2 files changed, 387 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h >> index 9cd62b2110a1..aed6ae80cc7c 100644 >> --- a/drivers/net/ethernet/stmicro/stmmac/common.h >> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h >> @@ -29,6 +29,7 @@ >> /* Synopsys Core versions */ >> #define DWMAC_CORE_3_40 0x34 >> #define DWMAC_CORE_3_50 0x35 >> +#define DWMAC_CORE_3_70 0x37 >> #define DWMAC_CORE_4_00 0x40 >> #define DWMAC_CORE_4_10 0x41 >> #define DWMAC_CORE_5_00 0x50 >> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c >> index 45dcc35b7955..559215e3fe41 100644 >> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c >> @@ -8,8 +8,71 @@ >> #include <linux/device.h> >> #include <linux/of_irq.h> >> #include "stmmac.h" >> +#include "dwmac_dma.h" >> +#include "dwmac1000.h" >> + >> +/* Normal Loongson Tx Summary */ >> +#define DMA_INTR_ENA_NIE_TX_LOONGSON 0x00040000 >> +/* Normal Loongson Rx Summary */ >> +#define DMA_INTR_ENA_NIE_RX_LOONGSON 0x00020000 >> + >> +#define DMA_INTR_NORMAL_LOONGSON (DMA_INTR_ENA_NIE_TX_LOONGSON | \ >> + DMA_INTR_ENA_NIE_RX_LOONGSON | \ >> + DMA_INTR_ENA_RIE | DMA_INTR_ENA_TIE) >> + >> +/* Abnormal Loongson Tx Summary */ >> +#define DMA_INTR_ENA_AIE_TX_LOONGSON 0x00010000 >> +/* Abnormal Loongson Rx Summary */ >> +#define DMA_INTR_ENA_AIE_RX_LOONGSON 0x00008000 >> + >> +#define DMA_INTR_ABNORMAL_LOONGSON (DMA_INTR_ENA_AIE_TX_LOONGSON | \ >> + DMA_INTR_ENA_AIE_RX_LOONGSON | \ >> + DMA_INTR_ENA_FBE | DMA_INTR_ENA_UNE) >> + >> +#define DMA_INTR_DEFAULT_MASK_LOONGSON (DMA_INTR_NORMAL_LOONGSON | \ >> + DMA_INTR_ABNORMAL_LOONGSON) >> + >> +/* Normal Loongson Tx Interrupt Summary */ >> +#define DMA_STATUS_NIS_TX_LOONGSON 0x00040000 >> +/* Normal Loongson Rx Interrupt Summary */ >> +#define DMA_STATUS_NIS_RX_LOONGSON 0x00020000 >> + >> +/* Abnormal Loongson Tx Interrupt Summary */ >> +#define DMA_STATUS_AIS_TX_LOONGSON 0x00010000 >> +/* Abnormal Loongson Rx Interrupt Summary */ >> +#define DMA_STATUS_AIS_RX_LOONGSON 0x00008000 >> + >> +/* Fatal Loongson Tx Bus Error Interrupt */ >> +#define DMA_STATUS_FBI_TX_LOONGSON 0x00002000 >> +/* Fatal Loongson Rx Bus Error Interrupt */ >> +#define DMA_STATUS_FBI_RX_LOONGSON 0x00001000 >> + >> +#define DMA_STATUS_MSK_COMMON_LOONGSON (DMA_STATUS_NIS_TX_LOONGSON | \ >> + DMA_STATUS_NIS_RX_LOONGSON | \ >> + DMA_STATUS_AIS_TX_LOONGSON | \ >> + DMA_STATUS_AIS_RX_LOONGSON | \ >> + DMA_STATUS_FBI_TX_LOONGSON | \ >> + DMA_STATUS_FBI_RX_LOONGSON) >> + >> +#define DMA_STATUS_MSK_RX_LOONGSON (DMA_STATUS_ERI | DMA_STATUS_RWT | \ >> + DMA_STATUS_RPS | DMA_STATUS_RU | \ >> + DMA_STATUS_RI | DMA_STATUS_OVF | \ >> + DMA_STATUS_MSK_COMMON_LOONGSON) >> + >> +#define DMA_STATUS_MSK_TX_LOONGSON (DMA_STATUS_ETI | DMA_STATUS_UNF | \ >> + DMA_STATUS_TJT | DMA_STATUS_TU | \ >> + DMA_STATUS_TPS | DMA_STATUS_TI | \ >> + DMA_STATUS_MSK_COMMON_LOONGSON) >> >> #define PCI_DEVICE_ID_LOONGSON_GMAC 0x7a03 >> +#define PCI_DEVICE_ID_LOONGSON_GNET 0x7a13 > Please move this to the new patch adding the Loongson GNET support. OK > >> +#define DWMAC_CORE_LS2K2000 0x10 /* Loongson custom IP */ > Note it's perfectly fine to have a device named after the SoC it's > equipped to. For example see the compatible strings defined for the > vendor-specific versions of the DW *MAC IP-cores: > Documentation/devicetree/bindings/net/snps,dwmac.yaml > > But if you aren't comfortable with such naming we can change the > macro to something like: > #define DWMAC_CORE_LOONGSON_MULTI_CH 0x10 Hmmm, It's all okay. What about huacai's comment? > >> +#define CHANNEL_NUM 8 >> + >> +struct loongson_data { >> + u32 loongson_id; >> + struct device *dev; > Please add the "dev" field in the new patch adding the Loongson GNET support. OK. > >> +}; >> >> struct stmmac_pci_info { >> int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat); >> @@ -67,6 +130,298 @@ static struct stmmac_pci_info loongson_gmac_pci_info = { >> .setup = loongson_gmac_data, >> }; >> >> +static void loongson_gnet_dma_init_channel(struct stmmac_priv *priv, > Since the multi-channel feature is no longer GNET-specific, please > rename the method prefix to loongson_dwmac_ . OK. > >> + void __iomem *ioaddr, >> + struct stmmac_dma_cfg *dma_cfg, >> + u32 chan) >> +{ >> + int txpbl = dma_cfg->txpbl ?: dma_cfg->pbl; >> + int rxpbl = dma_cfg->rxpbl ?: dma_cfg->pbl; >> + u32 value; >> + >> + value = readl(ioaddr + DMA_CHAN_BUS_MODE(chan)); >> + >> + if (dma_cfg->pblx8) >> + value |= DMA_BUS_MODE_MAXPBL; >> + >> + value |= DMA_BUS_MODE_USP; >> + value &= ~(DMA_BUS_MODE_PBL_MASK | DMA_BUS_MODE_RPBL_MASK); >> + value |= (txpbl << DMA_BUS_MODE_PBL_SHIFT); >> + value |= (rxpbl << DMA_BUS_MODE_RPBL_SHIFT); >> + >> + /* Set the Fixed burst mode */ >> + if (dma_cfg->fixed_burst) >> + value |= DMA_BUS_MODE_FB; >> + >> + /* Mixed Burst has no effect when fb is set */ >> + if (dma_cfg->mixed_burst) >> + value |= DMA_BUS_MODE_MB; >> + >> + if (dma_cfg->atds) >> + value |= DMA_BUS_MODE_ATDS; >> + >> + if (dma_cfg->aal) >> + value |= DMA_BUS_MODE_AAL; >> + >> + writel(value, ioaddr + DMA_CHAN_BUS_MODE(chan)); >> + >> + /* Mask interrupts by writing to CSR7 */ >> + writel(DMA_INTR_DEFAULT_MASK_LOONGSON, ioaddr + >> + DMA_CHAN_INTR_ENA(chan)); >> +} >> + >> +static int loongson_gnet_dma_interrupt(struct stmmac_priv *priv, > Similarly the loongson_dwmac_ prefix seems more appropriate now. OK. > >> + void __iomem *ioaddr, >> + struct stmmac_extra_stats *x, >> + u32 chan, u32 dir) >> +{ >> + struct stmmac_pcpu_stats *stats = this_cpu_ptr(priv->xstats.pcpu_stats); >> + u32 abnor_intr_status; >> + u32 nor_intr_status; >> + u32 fb_intr_status; >> + u32 intr_status; >> + int ret = 0; >> + >> + /* read the status register (CSR5) */ >> + intr_status = readl(ioaddr + DMA_CHAN_STATUS(chan)); >> + >> + if (dir == DMA_DIR_RX) >> + intr_status &= DMA_STATUS_MSK_RX_LOONGSON; >> + else if (dir == DMA_DIR_TX) >> + intr_status &= DMA_STATUS_MSK_TX_LOONGSON; >> + >> + nor_intr_status = intr_status & (DMA_STATUS_NIS_TX_LOONGSON | >> + DMA_STATUS_NIS_RX_LOONGSON); >> + abnor_intr_status = intr_status & (DMA_STATUS_AIS_TX_LOONGSON | >> + DMA_STATUS_AIS_RX_LOONGSON); >> + fb_intr_status = intr_status & (DMA_STATUS_FBI_TX_LOONGSON | >> + DMA_STATUS_FBI_RX_LOONGSON); >> + >> + /* ABNORMAL interrupts */ >> + if (unlikely(abnor_intr_status)) { >> + if (unlikely(intr_status & DMA_STATUS_UNF)) { >> + ret = tx_hard_error_bump_tc; >> + x->tx_undeflow_irq++; >> + } >> + if (unlikely(intr_status & DMA_STATUS_TJT)) >> + x->tx_jabber_irq++; >> + if (unlikely(intr_status & DMA_STATUS_OVF)) >> + x->rx_overflow_irq++; >> + if (unlikely(intr_status & DMA_STATUS_RU)) >> + x->rx_buf_unav_irq++; >> + if (unlikely(intr_status & DMA_STATUS_RPS)) >> + x->rx_process_stopped_irq++; >> + if (unlikely(intr_status & DMA_STATUS_RWT)) >> + x->rx_watchdog_irq++; >> + if (unlikely(intr_status & DMA_STATUS_ETI)) >> + x->tx_early_irq++; >> + if (unlikely(intr_status & DMA_STATUS_TPS)) { >> + x->tx_process_stopped_irq++; >> + ret = tx_hard_error; >> + } >> + if (unlikely(fb_intr_status)) { >> + x->fatal_bus_error_irq++; >> + ret = tx_hard_error; >> + } >> + } >> + /* TX/RX NORMAL interrupts */ >> + if (likely(nor_intr_status)) { >> + if (likely(intr_status & DMA_STATUS_RI)) { >> + u32 value = readl(ioaddr + DMA_INTR_ENA); >> + /* to schedule NAPI on real RIE event. */ >> + if (likely(value & DMA_INTR_ENA_RIE)) { >> + u64_stats_update_begin(&stats->syncp); >> + u64_stats_inc(&stats->rx_normal_irq_n[chan]); >> + u64_stats_update_end(&stats->syncp); >> + ret |= handle_rx; >> + } >> + } >> + if (likely(intr_status & DMA_STATUS_TI)) { >> + u64_stats_update_begin(&stats->syncp); >> + u64_stats_inc(&stats->tx_normal_irq_n[chan]); >> + u64_stats_update_end(&stats->syncp); >> + ret |= handle_tx; >> + } >> + if (unlikely(intr_status & DMA_STATUS_ERI)) >> + x->rx_early_irq++; >> + } >> + /* Optional hardware blocks, interrupts should be disabled */ >> + if (unlikely(intr_status & >> + (DMA_STATUS_GPI | DMA_STATUS_GMI | DMA_STATUS_GLI))) >> + pr_warn("%s: unexpected status %08x\n", __func__, intr_status); >> + >> + /* Clear the interrupt by writing a logic 1 to the CSR5[19-0] */ >> + writel((intr_status & 0x7ffff), ioaddr + DMA_CHAN_STATUS(chan)); >> + >> + return ret; >> +} >> + > >> +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 integrated PHY has a weird problem with switching from the low >> + * speeds to 1000Mbps mode. The speedup procedure requires the PHY-link >> + * re-negotiation. >> + */ >> + if (speed == SPEED_1000) { >> + if (readl(ptr->ioaddr + MAC_CTRL_REG) & >> + GMAC_CONTROL_PS) >> + /* Word around hardware bug, restart autoneg */ >> + phy_restart_aneg(ndev->phydev); >> + } >> +} >> + >> +static int loongson_gnet_data(struct pci_dev *pdev, >> + struct plat_stmmacenet_data *plat) >> +{ >> + loongson_default_data(pdev, plat); >> + >> + plat->phy_interface = PHY_INTERFACE_MODE_GMII; >> + plat->mdio_bus_data->phy_mask = ~(u32)BIT(2); >> + plat->fix_mac_speed = loongson_gnet_fix_speed; >> + >> + /* GNET devices with dev revision 0x00 do not support manually >> + * setting the speed to 1000. >> + */ >> + if (pdev->revision == 0x00) >> + plat->flags |= STMMAC_FLAG_DISABLE_FORCE_1000; >> + >> + return 0; >> +} >> + >> +static struct stmmac_pci_info loongson_gnet_pci_info = { >> + .setup = loongson_gnet_data, >> +}; > Please move this to the new patch adding the Loongson GNET support. OK. > >> + >> +static int loongson_dwmac_intx_config(struct pci_dev *pdev, >> + struct plat_stmmacenet_data *plat, >> + struct stmmac_resources *res) >> +{ >> + res->irq = pdev->irq; >> + >> + return 0; >> +} > If you get to implement what suggested here > https://lore.kernel.org/netdev/glm3jfqf36t5vnkmk4gsdqfx53ga7ohs3pxnsizqlogkbim7gg@a3dxav5siczn/ > than the loongson_dwmac_intx_config() won't be needed. Yeah! and loongson_dwmac_acpi_config() also won't be needed. Because loongson_dwmac_msi_config() will do everything. I already reply in patch 6. > >> + >> +static int loongson_dwmac_msi_config(struct pci_dev *pdev, >> + struct plat_stmmacenet_data *plat, >> + struct stmmac_resources *res) >> +{ >> + int i, ret, vecs; >> + >> + vecs = roundup_pow_of_two(CHANNEL_NUM * 2 + 1); >> + ret = pci_alloc_irq_vectors(pdev, 1, vecs, PCI_IRQ_MSI | PCI_IRQ_INTX); >> + if (ret < 0) { >> + dev_err(&pdev->dev, "Failed to allocate PCI IRQs\n"); >> + return ret; >> + } >> + >> + if (ret >= vecs) { >> + for (i = 0; i < plat->rx_queues_to_use; i++) { >> + res->rx_irq[CHANNEL_NUM - 1 - i] = >> + pci_irq_vector(pdev, 1 + i * 2); >> + } >> + for (i = 0; i < plat->tx_queues_to_use; i++) { >> + res->tx_irq[CHANNEL_NUM - 1 - i] = >> + pci_irq_vector(pdev, 2 + i * 2); >> + } >> + >> + plat->flags |= STMMAC_FLAG_MULTI_MSI_EN; >> + } >> + >> + res->irq = pci_irq_vector(pdev, 0); >> + >> + return 0; >> +} > Could you please clarify whether the multi-channel feature will work > if the pci_alloc_irq_vectors() method failed to allocated as many MSI > vectors as there are channels? What I can confirm is that the multi-channel device can work in a single channel, and as for the intermediate state, I still need to do experiments. > >> + >> +static int loongson_dwmac_msi_clear(struct pci_dev *pdev) >> +{ >> + pci_free_irq_vectors(pdev); >> + >> + return 0; >> +} >> + >> +static struct mac_device_info *loongson_dwmac_setup(void *apriv) >> +{ >> + struct stmmac_priv *priv = apriv; >> + struct mac_device_info *mac; >> + struct stmmac_dma_ops *dma; >> + struct loongson_data *ld; >> + struct pci_dev *pdev; >> + >> + ld = priv->plat->bsp_priv; >> + pdev = to_pci_dev(priv->device); >> + >> + mac = devm_kzalloc(priv->device, sizeof(*mac), GFP_KERNEL); >> + if (!mac) >> + return NULL; >> + >> + dma = devm_kzalloc(priv->device, sizeof(*dma), GFP_KERNEL); >> + if (!dma) >> + return NULL; >> + >> + /* The original IP-core version is v3.73a in all Loongson GNET >> + * (LS2K2000 and LS7A2000), but the GNET HW designers have changed the >> + * GMAC_VERSION.SNPSVER field to the custom 0x10 value on the Loongson >> + * LS2K2000 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. >> + */ > The comment needs to be altered since the multi-channel feature is no > longer GNET-specific. Like this: > > /* The Loongson GMAC and GNET devices are based on the DW GMAC > * v3.50a and v3.73a IP-cores. But the HW designers have changed the > * GMAC_VERSION.SNPSVER field to the custom 0x10 value on the > * network controllers with the multi-channels feature > * available 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. >> + if (ld->loongson_id == DWMAC_CORE_LS2K2000) { >> + priv->synopsys_id = DWMAC_CORE_3_70; >> + *dma = dwmac1000_dma_ops; >> + dma->init_chan = loongson_gnet_dma_init_channel; >> + dma->dma_interrupt = loongson_gnet_dma_interrupt; >> + mac->dma = dma; >> + } >> + >> + priv->dev->priv_flags |= IFF_UNICAST_FLT; >> + >> + /* Pre-initialize the respective "mac" fields as it's done in >> + * dwmac1000_setup() >> + */ >> + mac->pcsr = priv->ioaddr; >> + mac->multicast_filter_bins = priv->plat->multicast_filter_bins; >> + mac->unicast_filter_entries = priv->plat->unicast_filter_entries; >> + mac->mcast_bits_log2 = 0; >> + >> + if (mac->multicast_filter_bins) >> + mac->mcast_bits_log2 = ilog2(mac->multicast_filter_bins); >> + >> + /* Loongson GMAC doesn't support the flow control. LS2K2000 >> + * GNET doesn't support the half-duplex link mode. >> + */ >> + if (pdev->device == PCI_DEVICE_ID_LOONGSON_GMAC) { >> + mac->link.caps = MAC_10 | MAC_100 | MAC_1000; >> + } else { >> + if (ld->loongson_id == DWMAC_CORE_LS2K2000) >> + mac->link.caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE | >> + MAC_10 | MAC_100 | MAC_1000; >> + else >> + mac->link.caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE | >> + MAC_10FD | MAC_100FD | MAC_1000FD; >> + } > AFAIU The only part that is applicable for the multi-channels case of > the Loongson GMAC is: > + /* Loongson GMAC doesn't support the flow control */ > + mac->link.caps = MAC_10 | MAC_100 | MAC_1000; > > If so the rest of the changes in this chunk should be moved to the new > patch adding the Loongson GNET support. OK. > >> + >> + mac->link.duplex = GMAC_CONTROL_DM; >> + mac->link.speed10 = GMAC_CONTROL_PS; >> + mac->link.speed100 = GMAC_CONTROL_PS | GMAC_CONTROL_FES; >> + mac->link.speed1000 = 0; >> + mac->link.speed_mask = GMAC_CONTROL_PS | GMAC_CONTROL_FES; >> + mac->mii.addr = GMAC_MII_ADDR; >> + mac->mii.data = GMAC_MII_DATA; >> + mac->mii.addr_shift = 11; >> + mac->mii.addr_mask = 0x0000F800; >> + mac->mii.reg_shift = 6; >> + mac->mii.reg_mask = 0x000007C0; >> + mac->mii.clk_csr_shift = 2; >> + mac->mii.clk_csr_mask = GENMASK(5, 2); >> + >> + return mac; >> +} >> + >> static int loongson_dwmac_dt_config(struct pci_dev *pdev, >> struct plat_stmmacenet_data *plat, >> struct stmmac_resources *res) >> @@ -119,6 +474,7 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id >> struct plat_stmmacenet_data *plat; >> struct stmmac_pci_info *info; >> struct stmmac_resources res; >> + struct loongson_data *ld; >> int ret, i; >> >> plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL); >> @@ -135,6 +491,10 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id >> if (!plat->dma_cfg) >> return -ENOMEM; >> >> + ld = devm_kzalloc(&pdev->dev, sizeof(*ld), GFP_KERNEL); >> + if (!ld) >> + return -ENOMEM; >> + >> /* Enable pci device */ >> ret = pci_enable_device(pdev); >> if (ret) { >> @@ -159,19 +519,39 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id >> >> pci_set_master(pdev); >> >> + plat->bsp_priv = ld; >> + plat->setup = loongson_dwmac_setup; >> + ld->dev = &pdev->dev; >> + > Please make sure the chunk above and > + ld->loongson_id = readl(res.addr + GMAC_VERSION) & 0xff; > are performed before the stmmac_pci_info::setup() is called so the > callback could use the private platform data with the loongson_id > field initialized. OK. I've used your method on patch 6 and it works. > >> if (dev_of_node(&pdev->dev)) { >> ret = loongson_dwmac_dt_config(pdev, plat, &res); >> if (ret) >> goto err_disable_device; >> - } else { >> - res.irq = pdev->irq; >> } >> >> memset(&res, 0, sizeof(res)); > I've just realised the memset() call will clear out everything > initialized in "res" by the loongson_dwmac_dt_config() method. As your comments in patch 6: memset(&res, 0, sizeof(res)); res.addr = pcim_iomap_table(pdev)[0]; plat->bsp_priv = ld; plat->setup = loongson_dwmac_setup; ld->dev = &pdev->dev; ld->loongson_id = readl(res.addr + GMAC_VERSION) & 0xff; info = (struct stmmac_pci_info *)id->driver_data; ret = info->setup(pdev, plat); if (ret) goto err_disable_device; if (dev_of_node(&pdev->dev)) { ret = loongson_dwmac_dt_config(pdev, plat, &res); if (ret) goto err_disable_device; } ... > >> res.addr = pcim_iomap_table(pdev)[0]; >> + ld->loongson_id = readl(res.addr + GMAC_VERSION) & 0xff; >> + > >> + if (ld->loongson_id == DWMAC_CORE_LS2K2000) { >> + plat->rx_queues_to_use = CHANNEL_NUM; >> + plat->tx_queues_to_use = CHANNEL_NUM; >> + >> + /* Only channel 0 supports checksum, >> + * so turn off checksum to enable multiple channels. >> + */ >> + for (i = 1; i < CHANNEL_NUM; i++) >> + plat->tx_queues_cfg[i].coe_unsupported = 1; >> >> - plat->tx_queues_to_use = 1; >> - plat->rx_queues_to_use = 1; > Please move this part to the loongson_gmac_data() method (and to the > loongson_gnet_data() in the new patch). The only > code which would be left here is: > if (ld->loongson_id == DWMAC_CORE_LOONGSON_MULTI_CH) { > ret = loongson_dwmac_msi_config(pdev, plat, &res); > if (ret) > goto err_disable_device; > } OK. We don't need if else. I've explained this in patch 6. > >> + ret = loongson_dwmac_msi_config(pdev, plat, &res); >> + } else { >> + plat->tx_queues_to_use = 1; >> + plat->rx_queues_to_use = 1; >> + >> + ret = loongson_dwmac_intx_config(pdev, plat, &res); >> + } >> + if (ret) >> + goto err_disable_device; > This won't be needed if you get to implement what is suggested here > https://lore.kernel.org/netdev/glm3jfqf36t5vnkmk4gsdqfx53ga7ohs3pxnsizqlogkbim7gg@a3dxav5siczn/ I will drop it > >> >> ret = stmmac_dvr_probe(&pdev->dev, plat, &res); >> if (ret) > I don't see the MSI-configs being undone in case of the > stmmac_dvr_probe() failure. What should have been done: > > ret = stmmac_dvr_probe(&pdev->dev, plat, &res); > if (ret) > goto err_clear_msi; > > return 0; > > err_clear_msi: > if (ld->loongson_id == DWMAC_CORE_LOONGSON_MULTI_CH) > loongson_dwmac_msi_clear(pdev); > > err_disable_device: > pci_disable_device(pdev); > > ... OK! > >> @@ -202,6 +582,7 @@ static void loongson_dwmac_remove(struct pci_dev *pdev) >> break; >> } >> >> + loongson_dwmac_msi_clear(pdev); > Shouldn't this be done for the multi-channels device only? Like this > if (ld->loongson_id == DWMAC_CORE_LOONGSON_MULTI_CH) > loongson_dwmac_msi_clear(pdev); Yeah! > >> pci_disable_device(pdev); >> } >> >> @@ -245,6 +626,7 @@ static SIMPLE_DEV_PM_OPS(loongson_dwmac_pm_ops, loongson_dwmac_suspend, >> >> 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) }, > Please move this to the new patch adding the Loongson GNET support. OK. > > > > Sigh... The comments are brutal but if no more unexpected details get > to be revealed the respective alterations shall bring us to the > coherent set of changes. And hopefully there won't be need in such > comprehensive patchset refactoring anymore.( Sigh... Thanks for you review! Thanks, Yanteng > > -Serge(y) > >> {} >> }; >> MODULE_DEVICE_TABLE(pci, loongson_dwmac_id_table); >> -- >> 2.31.4 >>
Hi Huacai On Wed, Jul 03, 2024 at 09:19:59AM +0800, Huacai Chen wrote: > On Tue, Jul 2, 2024 at 9:43 PM Serge Semin <fancer.lancer@gmail.com> wrote: > > ... > > On Wed, May 29, 2024 at 06:21:09PM +0800, Yanteng Si wrote: > > > ... > > ... > > > +#define DWMAC_CORE_LS2K2000 0x10 /* Loongson custom IP */ > > > > Note it's perfectly fine to have a device named after the SoC it's > > equipped to. For example see the compatible strings defined for the > > vendor-specific versions of the DW *MAC IP-cores: > > Documentation/devicetree/bindings/net/snps,dwmac.yaml > > > > But if you aren't comfortable with such naming we can change the > > macro to something like: > > #define DWMAC_CORE_LOONGSON_MULTI_CH 0x10 > Maybe DWMAC_CORE_LOONGSON_MULTICHAN or DWMAC_CORE_LOONGSON_MULTI_CHAN > is a little better? > Well, I don't have a strong opinion about that in this case. Personally I prefer to have the shortest and still readable version. It decreases the probability of the lines splitting in case of the long-line statements or highly indented code. From that perspective something like DWMAC_CORE_LS_MULTI_CH would be even better. But seeing the driver currently don't have such cases, we can use any of those name. But it's better to be of such length so the code lines the name is utilized in wouldn't exceed +80 chars. -Serge(y)
>>> But if you aren't comfortable with such naming we can change the >>> macro to something like: >>> #define DWMAC_CORE_LOONGSON_MULTI_CH 0x10 >> Maybe DWMAC_CORE_LOONGSON_MULTICHAN or DWMAC_CORE_LOONGSON_MULTI_CHAN >> is a little better? >> > Well, I don't have a strong opinion about that in this case. > Personally I prefer to have the shortest and still readable version. > It decreases the probability of the lines splitting in case of the > long-line statements or highly indented code. From that perspective > something like DWMAC_CORE_LS_MULTI_CH would be even better. But seeing > the driver currently don't have such cases, we can use any of those > name. But it's better to be of such length so the code lines the name > is utilized in wouldn't exceed +80 chars. Okay. I added an indent before 0xXX and left three Spaces before the comment, which uses huacai's MULTICHAN and doesn't exceed 80 chars. Thanks, Yanteng
On Fri, Jul 05, 2024 at 08:06:32PM +0800, Yanteng Si wrote: > > > > But if you aren't comfortable with such naming we can change the > > > > macro to something like: > > > > #define DWMAC_CORE_LOONGSON_MULTI_CH 0x10 > > > Maybe DWMAC_CORE_LOONGSON_MULTICHAN or DWMAC_CORE_LOONGSON_MULTI_CHAN > > > is a little better? > > > > > Well, I don't have a strong opinion about that in this case. > > Personally I prefer to have the shortest and still readable version. > > It decreases the probability of the lines splitting in case of the > > long-line statements or highly indented code. From that perspective > > something like DWMAC_CORE_LS_MULTI_CH would be even better. But seeing > > the driver currently don't have such cases, we can use any of those > > name. But it's better to be of such length so the code lines the name > > is utilized in wouldn't exceed +80 chars. > > Okay. > > I added an indent before 0xXX and left three Spaces before the comment, > > which uses huacai's MULTICHAN and doesn't exceed 80 chars. I meant that it's better to have the length of the macro name so !the code where it's utilized! wouldn't exceed +80 chars. That's the criteria for the upper length boundary I normally follow in such cases. -Serge
在 2024/7/5 20:17, Serge Semin 写道: > On Fri, Jul 05, 2024 at 08:06:32PM +0800, Yanteng Si wrote: >>>>> But if you aren't comfortable with such naming we can change the >>>>> macro to something like: >>>>> #define DWMAC_CORE_LOONGSON_MULTI_CH 0x10 >>>> Maybe DWMAC_CORE_LOONGSON_MULTICHAN or DWMAC_CORE_LOONGSON_MULTI_CHAN >>>> is a little better? >>>> >>> Well, I don't have a strong opinion about that in this case. >>> Personally I prefer to have the shortest and still readable version. >>> It decreases the probability of the lines splitting in case of the >>> long-line statements or highly indented code. From that perspective >>> something like DWMAC_CORE_LS_MULTI_CH would be even better. But seeing >>> the driver currently don't have such cases, we can use any of those >>> name. But it's better to be of such length so the code lines the name >>> is utilized in wouldn't exceed +80 chars. >> Okay. >> >> I added an indent before 0xXX and left three Spaces before the comment, >> >> which uses huacai's MULTICHAN and doesn't exceed 80 chars. > I meant that it's better to have the length of the macro name so > !the code where it's utilized! > wouldn't exceed +80 chars. That's the criteria for the upper length > boundary I normally follow in such cases. > Oh, I see! Hmm, let's compare the two options: DWMAC_CORE_LS_MULTI_CH DWMAC_CORE_LS_MULTICHAN With just one more char, the increased readability seems to be worth it. Thanks, Yanteng
On Sat, Jul 6, 2024 at 6:31 PM Yanteng Si <siyanteng@loongson.cn> wrote: > > > 在 2024/7/5 20:17, Serge Semin 写道: > > On Fri, Jul 05, 2024 at 08:06:32PM +0800, Yanteng Si wrote: > >>>>> But if you aren't comfortable with such naming we can change the > >>>>> macro to something like: > >>>>> #define DWMAC_CORE_LOONGSON_MULTI_CH 0x10 > >>>> Maybe DWMAC_CORE_LOONGSON_MULTICHAN or DWMAC_CORE_LOONGSON_MULTI_CHAN > >>>> is a little better? > >>>> > >>> Well, I don't have a strong opinion about that in this case. > >>> Personally I prefer to have the shortest and still readable version. > >>> It decreases the probability of the lines splitting in case of the > >>> long-line statements or highly indented code. From that perspective > >>> something like DWMAC_CORE_LS_MULTI_CH would be even better. But seeing > >>> the driver currently don't have such cases, we can use any of those > >>> name. But it's better to be of such length so the code lines the name > >>> is utilized in wouldn't exceed +80 chars. > >> Okay. > >> > >> I added an indent before 0xXX and left three Spaces before the comment, > >> > >> which uses huacai's MULTICHAN and doesn't exceed 80 chars. > > I meant that it's better to have the length of the macro name so > > !the code where it's utilized! > > wouldn't exceed +80 chars. That's the criteria for the upper length > > boundary I normally follow in such cases. > > > Oh, I see! > > Hmm, let's compare the two options: > > DWMAC_CORE_LS_MULTI_CH > > DWMAC_CORE_LS_MULTICHAN > > With just one more char, the increased readability seems to be > worth it. If you really like short names, please use DWMAC_CORE_MULTICHAN. LS has no valuable meaning in this loongson-specific file. Huacai > > > Thanks, > > Yanteng >
On Sat, Jul 06, 2024 at 06:36:06PM +0800, Huacai Chen wrote: > On Sat, Jul 6, 2024 at 6:31 PM Yanteng Si <siyanteng@loongson.cn> wrote: > > > > > > 在 2024/7/5 20:17, Serge Semin 写道: > > > On Fri, Jul 05, 2024 at 08:06:32PM +0800, Yanteng Si wrote: > > >>>>> But if you aren't comfortable with such naming we can change the > > >>>>> macro to something like: > > >>>>> #define DWMAC_CORE_LOONGSON_MULTI_CH 0x10 > > >>>> Maybe DWMAC_CORE_LOONGSON_MULTICHAN or DWMAC_CORE_LOONGSON_MULTI_CHAN > > >>>> is a little better? > > >>>> > > >>> Well, I don't have a strong opinion about that in this case. > > >>> Personally I prefer to have the shortest and still readable version. > > >>> It decreases the probability of the lines splitting in case of the > > >>> long-line statements or highly indented code. From that perspective > > >>> something like DWMAC_CORE_LS_MULTI_CH would be even better. But seeing > > >>> the driver currently don't have such cases, we can use any of those > > >>> name. But it's better to be of such length so the code lines the name > > >>> is utilized in wouldn't exceed +80 chars. > > >> Okay. > > >> > > >> I added an indent before 0xXX and left three Spaces before the comment, > > >> > > >> which uses huacai's MULTICHAN and doesn't exceed 80 chars. > > > I meant that it's better to have the length of the macro name so > > > !the code where it's utilized! > > > wouldn't exceed +80 chars. That's the criteria for the upper length > > > boundary I normally follow in such cases. > > > > > Oh, I see! > > > > Hmm, let's compare the two options: > > > > DWMAC_CORE_LS_MULTI_CH > > > > DWMAC_CORE_LS_MULTICHAN > > > > With just one more char, the increased readability seems to be > > worth it. > If you really like short names, please use DWMAC_CORE_MULTICHAN. LS > has no valuable meaning in this loongson-specific file. At least some version of the Loongson vendor name should be in the macro. Omitting it may cause a confusion since the name would turn to a too generic form. Seeing the multi DMA-channels feature is the Synopsys invention, should you meet the macro like DWMAC_CORE_MULTI_CH in the code that may cause an impression that there is a special Synopsys DW MAC ID for that. Meanwhile in fact the custom ID is specific for the Loongson GMAC/GNET controllers only. -Serge(y) > > Huacai > > > > > > > Thanks, > > > > Yanteng > >
On Sun, Jul 7, 2024 at 6:51 PM Serge Semin <fancer.lancer@gmail.com> wrote: > > On Sat, Jul 06, 2024 at 06:36:06PM +0800, Huacai Chen wrote: > > On Sat, Jul 6, 2024 at 6:31 PM Yanteng Si <siyanteng@loongson.cn> wrote: > > > > > > > > > 在 2024/7/5 20:17, Serge Semin 写道: > > > > On Fri, Jul 05, 2024 at 08:06:32PM +0800, Yanteng Si wrote: > > > >>>>> But if you aren't comfortable with such naming we can change the > > > >>>>> macro to something like: > > > >>>>> #define DWMAC_CORE_LOONGSON_MULTI_CH 0x10 > > > >>>> Maybe DWMAC_CORE_LOONGSON_MULTICHAN or DWMAC_CORE_LOONGSON_MULTI_CHAN > > > >>>> is a little better? > > > >>>> > > > >>> Well, I don't have a strong opinion about that in this case. > > > >>> Personally I prefer to have the shortest and still readable version. > > > >>> It decreases the probability of the lines splitting in case of the > > > >>> long-line statements or highly indented code. From that perspective > > > >>> something like DWMAC_CORE_LS_MULTI_CH would be even better. But seeing > > > >>> the driver currently don't have such cases, we can use any of those > > > >>> name. But it's better to be of such length so the code lines the name > > > >>> is utilized in wouldn't exceed +80 chars. > > > >> Okay. > > > >> > > > >> I added an indent before 0xXX and left three Spaces before the comment, > > > >> > > > >> which uses huacai's MULTICHAN and doesn't exceed 80 chars. > > > > I meant that it's better to have the length of the macro name so > > > > !the code where it's utilized! > > > > wouldn't exceed +80 chars. That's the criteria for the upper length > > > > boundary I normally follow in such cases. > > > > > > > Oh, I see! > > > > > > Hmm, let's compare the two options: > > > > > > DWMAC_CORE_LS_MULTI_CH > > > > > > DWMAC_CORE_LS_MULTICHAN > > > > > > With just one more char, the increased readability seems to be > > > worth it. > > If you really like short names, please use DWMAC_CORE_MULTICHAN. LS > > has no valuable meaning in this loongson-specific file. > > At least some version of the Loongson vendor name should be in the > macro. Omitting it may cause a confusion since the name would turn to > a too generic form. Seeing the multi DMA-channels feature is the > Synopsys invention, should you meet the macro like DWMAC_CORE_MULTI_CH > in the code that may cause an impression that there is a special > Synopsys DW MAC ID for that. Meanwhile in fact the custom ID is > specific for the Loongson GMAC/GNET controllers only. Well, I prefer DWMAC_CORE_LOONGSON_MULTI_CHAN / DWMAC_CORE_LOONGSON_MULTICHAN / DWMAC_CORE_LOONGSON_MCH / DWMAC_CORE_MULTICHAN, But I also accept DWMAC_CORE_LS_MULTI_CHAN / DWMAC_CORE_LS_MULTICHAN, But I can't accept DWMAC_CORE_LS2K2000. Huacai > > -Serge(y) > > > > > Huacai > > > > > > > > > > > Thanks, > > > > > > Yanteng > > >
在 2024/7/7 21:57, Huacai Chen 写道: > On Sun, Jul 7, 2024 at 6:51 PM Serge Semin <fancer.lancer@gmail.com> wrote: >> On Sat, Jul 06, 2024 at 06:36:06PM +0800, Huacai Chen wrote: >>> On Sat, Jul 6, 2024 at 6:31 PM Yanteng Si <siyanteng@loongson.cn> wrote: >>>> >>>> 在 2024/7/5 20:17, Serge Semin 写道: >>>>> On Fri, Jul 05, 2024 at 08:06:32PM +0800, Yanteng Si wrote: >>>>>>>>> But if you aren't comfortable with such naming we can change the >>>>>>>>> macro to something like: >>>>>>>>> #define DWMAC_CORE_LOONGSON_MULTI_CH 0x10 >>>>>>>> Maybe DWMAC_CORE_LOONGSON_MULTICHAN or DWMAC_CORE_LOONGSON_MULTI_CHAN >>>>>>>> is a little better? >>>>>>>> >>>>>>> Well, I don't have a strong opinion about that in this case. >>>>>>> Personally I prefer to have the shortest and still readable version. >>>>>>> It decreases the probability of the lines splitting in case of the >>>>>>> long-line statements or highly indented code. From that perspective >>>>>>> something like DWMAC_CORE_LS_MULTI_CH would be even better. But seeing >>>>>>> the driver currently don't have such cases, we can use any of those >>>>>>> name. But it's better to be of such length so the code lines the name >>>>>>> is utilized in wouldn't exceed +80 chars. >>>>>> Okay. >>>>>> >>>>>> I added an indent before 0xXX and left three Spaces before the comment, >>>>>> >>>>>> which uses huacai's MULTICHAN and doesn't exceed 80 chars. >>>>> I meant that it's better to have the length of the macro name so >>>>> !the code where it's utilized! >>>>> wouldn't exceed +80 chars. That's the criteria for the upper length >>>>> boundary I normally follow in such cases. >>>>> >>>> Oh, I see! >>>> >>>> Hmm, let's compare the two options: >>>> >>>> DWMAC_CORE_LS_MULTI_CH >>>> >>>> DWMAC_CORE_LS_MULTICHAN >>>> >>>> With just one more char, the increased readability seems to be >>>> worth it. >>> If you really like short names, please use DWMAC_CORE_MULTICHAN. LS >>> has no valuable meaning in this loongson-specific file. >> At least some version of the Loongson vendor name should be in the >> macro. Omitting it may cause a confusion since the name would turn to >> a too generic form. Seeing the multi DMA-channels feature is the >> Synopsys invention, should you meet the macro like DWMAC_CORE_MULTI_CH >> in the code that may cause an impression that there is a special >> Synopsys DW MAC ID for that. Meanwhile in fact the custom ID is >> specific for the Loongson GMAC/GNET controllers only. > Well, > I prefer > DWMAC_CORE_LOONGSON_MULTI_CHAN / DWMAC_CORE_LOONGSON_MULTICHAN / > DWMAC_CORE_LOONGSON_MCH / DWMAC_CORE_MULTICHAN, > But I also accept DWMAC_CORE_LS_MULTI_CHAN / DWMAC_CORE_LS_MULTICHAN, > But I can't accept DWMAC_CORE_LS2K2000. > I'll use DWMAC_CORE_LS_MULTICHAN for now, Let's continue discussing this macro in v14 (If necessary). Thanks, Yanteng
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h index 9cd62b2110a1..aed6ae80cc7c 100644 --- a/drivers/net/ethernet/stmicro/stmmac/common.h +++ b/drivers/net/ethernet/stmicro/stmmac/common.h @@ -29,6 +29,7 @@ /* Synopsys Core versions */ #define DWMAC_CORE_3_40 0x34 #define DWMAC_CORE_3_50 0x35 +#define DWMAC_CORE_3_70 0x37 #define DWMAC_CORE_4_00 0x40 #define DWMAC_CORE_4_10 0x41 #define DWMAC_CORE_5_00 0x50 diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c index 45dcc35b7955..559215e3fe41 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c @@ -8,8 +8,71 @@ #include <linux/device.h> #include <linux/of_irq.h> #include "stmmac.h" +#include "dwmac_dma.h" +#include "dwmac1000.h" + +/* Normal Loongson Tx Summary */ +#define DMA_INTR_ENA_NIE_TX_LOONGSON 0x00040000 +/* Normal Loongson Rx Summary */ +#define DMA_INTR_ENA_NIE_RX_LOONGSON 0x00020000 + +#define DMA_INTR_NORMAL_LOONGSON (DMA_INTR_ENA_NIE_TX_LOONGSON | \ + DMA_INTR_ENA_NIE_RX_LOONGSON | \ + DMA_INTR_ENA_RIE | DMA_INTR_ENA_TIE) + +/* Abnormal Loongson Tx Summary */ +#define DMA_INTR_ENA_AIE_TX_LOONGSON 0x00010000 +/* Abnormal Loongson Rx Summary */ +#define DMA_INTR_ENA_AIE_RX_LOONGSON 0x00008000 + +#define DMA_INTR_ABNORMAL_LOONGSON (DMA_INTR_ENA_AIE_TX_LOONGSON | \ + DMA_INTR_ENA_AIE_RX_LOONGSON | \ + DMA_INTR_ENA_FBE | DMA_INTR_ENA_UNE) + +#define DMA_INTR_DEFAULT_MASK_LOONGSON (DMA_INTR_NORMAL_LOONGSON | \ + DMA_INTR_ABNORMAL_LOONGSON) + +/* Normal Loongson Tx Interrupt Summary */ +#define DMA_STATUS_NIS_TX_LOONGSON 0x00040000 +/* Normal Loongson Rx Interrupt Summary */ +#define DMA_STATUS_NIS_RX_LOONGSON 0x00020000 + +/* Abnormal Loongson Tx Interrupt Summary */ +#define DMA_STATUS_AIS_TX_LOONGSON 0x00010000 +/* Abnormal Loongson Rx Interrupt Summary */ +#define DMA_STATUS_AIS_RX_LOONGSON 0x00008000 + +/* Fatal Loongson Tx Bus Error Interrupt */ +#define DMA_STATUS_FBI_TX_LOONGSON 0x00002000 +/* Fatal Loongson Rx Bus Error Interrupt */ +#define DMA_STATUS_FBI_RX_LOONGSON 0x00001000 + +#define DMA_STATUS_MSK_COMMON_LOONGSON (DMA_STATUS_NIS_TX_LOONGSON | \ + DMA_STATUS_NIS_RX_LOONGSON | \ + DMA_STATUS_AIS_TX_LOONGSON | \ + DMA_STATUS_AIS_RX_LOONGSON | \ + DMA_STATUS_FBI_TX_LOONGSON | \ + DMA_STATUS_FBI_RX_LOONGSON) + +#define DMA_STATUS_MSK_RX_LOONGSON (DMA_STATUS_ERI | DMA_STATUS_RWT | \ + DMA_STATUS_RPS | DMA_STATUS_RU | \ + DMA_STATUS_RI | DMA_STATUS_OVF | \ + DMA_STATUS_MSK_COMMON_LOONGSON) + +#define DMA_STATUS_MSK_TX_LOONGSON (DMA_STATUS_ETI | DMA_STATUS_UNF | \ + DMA_STATUS_TJT | DMA_STATUS_TU | \ + DMA_STATUS_TPS | DMA_STATUS_TI | \ + DMA_STATUS_MSK_COMMON_LOONGSON) #define PCI_DEVICE_ID_LOONGSON_GMAC 0x7a03 +#define PCI_DEVICE_ID_LOONGSON_GNET 0x7a13 +#define DWMAC_CORE_LS2K2000 0x10 /* Loongson custom IP */ +#define CHANNEL_NUM 8 + +struct loongson_data { + u32 loongson_id; + struct device *dev; +}; struct stmmac_pci_info { int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat); @@ -67,6 +130,298 @@ static struct stmmac_pci_info loongson_gmac_pci_info = { .setup = loongson_gmac_data, }; +static void loongson_gnet_dma_init_channel(struct stmmac_priv *priv, + void __iomem *ioaddr, + struct stmmac_dma_cfg *dma_cfg, + u32 chan) +{ + int txpbl = dma_cfg->txpbl ?: dma_cfg->pbl; + int rxpbl = dma_cfg->rxpbl ?: dma_cfg->pbl; + u32 value; + + value = readl(ioaddr + DMA_CHAN_BUS_MODE(chan)); + + if (dma_cfg->pblx8) + value |= DMA_BUS_MODE_MAXPBL; + + value |= DMA_BUS_MODE_USP; + value &= ~(DMA_BUS_MODE_PBL_MASK | DMA_BUS_MODE_RPBL_MASK); + value |= (txpbl << DMA_BUS_MODE_PBL_SHIFT); + value |= (rxpbl << DMA_BUS_MODE_RPBL_SHIFT); + + /* Set the Fixed burst mode */ + if (dma_cfg->fixed_burst) + value |= DMA_BUS_MODE_FB; + + /* Mixed Burst has no effect when fb is set */ + if (dma_cfg->mixed_burst) + value |= DMA_BUS_MODE_MB; + + if (dma_cfg->atds) + value |= DMA_BUS_MODE_ATDS; + + if (dma_cfg->aal) + value |= DMA_BUS_MODE_AAL; + + writel(value, ioaddr + DMA_CHAN_BUS_MODE(chan)); + + /* Mask interrupts by writing to CSR7 */ + writel(DMA_INTR_DEFAULT_MASK_LOONGSON, ioaddr + + DMA_CHAN_INTR_ENA(chan)); +} + +static int loongson_gnet_dma_interrupt(struct stmmac_priv *priv, + void __iomem *ioaddr, + struct stmmac_extra_stats *x, + u32 chan, u32 dir) +{ + struct stmmac_pcpu_stats *stats = this_cpu_ptr(priv->xstats.pcpu_stats); + u32 abnor_intr_status; + u32 nor_intr_status; + u32 fb_intr_status; + u32 intr_status; + int ret = 0; + + /* read the status register (CSR5) */ + intr_status = readl(ioaddr + DMA_CHAN_STATUS(chan)); + + if (dir == DMA_DIR_RX) + intr_status &= DMA_STATUS_MSK_RX_LOONGSON; + else if (dir == DMA_DIR_TX) + intr_status &= DMA_STATUS_MSK_TX_LOONGSON; + + nor_intr_status = intr_status & (DMA_STATUS_NIS_TX_LOONGSON | + DMA_STATUS_NIS_RX_LOONGSON); + abnor_intr_status = intr_status & (DMA_STATUS_AIS_TX_LOONGSON | + DMA_STATUS_AIS_RX_LOONGSON); + fb_intr_status = intr_status & (DMA_STATUS_FBI_TX_LOONGSON | + DMA_STATUS_FBI_RX_LOONGSON); + + /* ABNORMAL interrupts */ + if (unlikely(abnor_intr_status)) { + if (unlikely(intr_status & DMA_STATUS_UNF)) { + ret = tx_hard_error_bump_tc; + x->tx_undeflow_irq++; + } + if (unlikely(intr_status & DMA_STATUS_TJT)) + x->tx_jabber_irq++; + if (unlikely(intr_status & DMA_STATUS_OVF)) + x->rx_overflow_irq++; + if (unlikely(intr_status & DMA_STATUS_RU)) + x->rx_buf_unav_irq++; + if (unlikely(intr_status & DMA_STATUS_RPS)) + x->rx_process_stopped_irq++; + if (unlikely(intr_status & DMA_STATUS_RWT)) + x->rx_watchdog_irq++; + if (unlikely(intr_status & DMA_STATUS_ETI)) + x->tx_early_irq++; + if (unlikely(intr_status & DMA_STATUS_TPS)) { + x->tx_process_stopped_irq++; + ret = tx_hard_error; + } + if (unlikely(fb_intr_status)) { + x->fatal_bus_error_irq++; + ret = tx_hard_error; + } + } + /* TX/RX NORMAL interrupts */ + if (likely(nor_intr_status)) { + if (likely(intr_status & DMA_STATUS_RI)) { + u32 value = readl(ioaddr + DMA_INTR_ENA); + /* to schedule NAPI on real RIE event. */ + if (likely(value & DMA_INTR_ENA_RIE)) { + u64_stats_update_begin(&stats->syncp); + u64_stats_inc(&stats->rx_normal_irq_n[chan]); + u64_stats_update_end(&stats->syncp); + ret |= handle_rx; + } + } + if (likely(intr_status & DMA_STATUS_TI)) { + u64_stats_update_begin(&stats->syncp); + u64_stats_inc(&stats->tx_normal_irq_n[chan]); + u64_stats_update_end(&stats->syncp); + ret |= handle_tx; + } + if (unlikely(intr_status & DMA_STATUS_ERI)) + x->rx_early_irq++; + } + /* Optional hardware blocks, interrupts should be disabled */ + if (unlikely(intr_status & + (DMA_STATUS_GPI | DMA_STATUS_GMI | DMA_STATUS_GLI))) + pr_warn("%s: unexpected status %08x\n", __func__, intr_status); + + /* Clear the interrupt by writing a logic 1 to the CSR5[19-0] */ + writel((intr_status & 0x7ffff), ioaddr + DMA_CHAN_STATUS(chan)); + + return ret; +} + +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 integrated PHY has a weird problem with switching from the low + * speeds to 1000Mbps mode. The speedup procedure requires the PHY-link + * re-negotiation. + */ + if (speed == SPEED_1000) { + if (readl(ptr->ioaddr + MAC_CTRL_REG) & + GMAC_CONTROL_PS) + /* Word around hardware bug, restart autoneg */ + phy_restart_aneg(ndev->phydev); + } +} + +static int loongson_gnet_data(struct pci_dev *pdev, + struct plat_stmmacenet_data *plat) +{ + loongson_default_data(pdev, plat); + + plat->phy_interface = PHY_INTERFACE_MODE_GMII; + plat->mdio_bus_data->phy_mask = ~(u32)BIT(2); + plat->fix_mac_speed = loongson_gnet_fix_speed; + + /* GNET devices with dev revision 0x00 do not support manually + * setting the speed to 1000. + */ + if (pdev->revision == 0x00) + plat->flags |= STMMAC_FLAG_DISABLE_FORCE_1000; + + return 0; +} + +static struct stmmac_pci_info loongson_gnet_pci_info = { + .setup = loongson_gnet_data, +}; + +static int loongson_dwmac_intx_config(struct pci_dev *pdev, + struct plat_stmmacenet_data *plat, + struct stmmac_resources *res) +{ + res->irq = pdev->irq; + + return 0; +} + +static int loongson_dwmac_msi_config(struct pci_dev *pdev, + struct plat_stmmacenet_data *plat, + struct stmmac_resources *res) +{ + int i, ret, vecs; + + vecs = roundup_pow_of_two(CHANNEL_NUM * 2 + 1); + ret = pci_alloc_irq_vectors(pdev, 1, vecs, PCI_IRQ_MSI | PCI_IRQ_INTX); + if (ret < 0) { + dev_err(&pdev->dev, "Failed to allocate PCI IRQs\n"); + return ret; + } + + if (ret >= vecs) { + for (i = 0; i < plat->rx_queues_to_use; i++) { + res->rx_irq[CHANNEL_NUM - 1 - i] = + pci_irq_vector(pdev, 1 + i * 2); + } + for (i = 0; i < plat->tx_queues_to_use; i++) { + res->tx_irq[CHANNEL_NUM - 1 - i] = + pci_irq_vector(pdev, 2 + i * 2); + } + + plat->flags |= STMMAC_FLAG_MULTI_MSI_EN; + } + + res->irq = pci_irq_vector(pdev, 0); + + return 0; +} + +static int loongson_dwmac_msi_clear(struct pci_dev *pdev) +{ + pci_free_irq_vectors(pdev); + + return 0; +} + +static struct mac_device_info *loongson_dwmac_setup(void *apriv) +{ + struct stmmac_priv *priv = apriv; + struct mac_device_info *mac; + struct stmmac_dma_ops *dma; + struct loongson_data *ld; + struct pci_dev *pdev; + + ld = priv->plat->bsp_priv; + pdev = to_pci_dev(priv->device); + + mac = devm_kzalloc(priv->device, sizeof(*mac), GFP_KERNEL); + if (!mac) + return NULL; + + dma = devm_kzalloc(priv->device, sizeof(*dma), GFP_KERNEL); + if (!dma) + return NULL; + + /* The original IP-core version is v3.73a in all Loongson GNET + * (LS2K2000 and LS7A2000), but the GNET HW designers have changed the + * GMAC_VERSION.SNPSVER field to the custom 0x10 value on the Loongson + * LS2K2000 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. + */ + if (ld->loongson_id == DWMAC_CORE_LS2K2000) { + priv->synopsys_id = DWMAC_CORE_3_70; + *dma = dwmac1000_dma_ops; + dma->init_chan = loongson_gnet_dma_init_channel; + dma->dma_interrupt = loongson_gnet_dma_interrupt; + mac->dma = dma; + } + + priv->dev->priv_flags |= IFF_UNICAST_FLT; + + /* Pre-initialize the respective "mac" fields as it's done in + * dwmac1000_setup() + */ + mac->pcsr = priv->ioaddr; + mac->multicast_filter_bins = priv->plat->multicast_filter_bins; + mac->unicast_filter_entries = priv->plat->unicast_filter_entries; + mac->mcast_bits_log2 = 0; + + if (mac->multicast_filter_bins) + mac->mcast_bits_log2 = ilog2(mac->multicast_filter_bins); + + /* Loongson GMAC doesn't support the flow control. LS2K2000 + * GNET doesn't support the half-duplex link mode. + */ + if (pdev->device == PCI_DEVICE_ID_LOONGSON_GMAC) { + mac->link.caps = MAC_10 | MAC_100 | MAC_1000; + } else { + if (ld->loongson_id == DWMAC_CORE_LS2K2000) + mac->link.caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE | + MAC_10 | MAC_100 | MAC_1000; + else + mac->link.caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE | + MAC_10FD | MAC_100FD | MAC_1000FD; + } + + mac->link.duplex = GMAC_CONTROL_DM; + mac->link.speed10 = GMAC_CONTROL_PS; + mac->link.speed100 = GMAC_CONTROL_PS | GMAC_CONTROL_FES; + mac->link.speed1000 = 0; + mac->link.speed_mask = GMAC_CONTROL_PS | GMAC_CONTROL_FES; + mac->mii.addr = GMAC_MII_ADDR; + mac->mii.data = GMAC_MII_DATA; + mac->mii.addr_shift = 11; + mac->mii.addr_mask = 0x0000F800; + mac->mii.reg_shift = 6; + mac->mii.reg_mask = 0x000007C0; + mac->mii.clk_csr_shift = 2; + mac->mii.clk_csr_mask = GENMASK(5, 2); + + return mac; +} + static int loongson_dwmac_dt_config(struct pci_dev *pdev, struct plat_stmmacenet_data *plat, struct stmmac_resources *res) @@ -119,6 +474,7 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id struct plat_stmmacenet_data *plat; struct stmmac_pci_info *info; struct stmmac_resources res; + struct loongson_data *ld; int ret, i; plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL); @@ -135,6 +491,10 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id if (!plat->dma_cfg) return -ENOMEM; + ld = devm_kzalloc(&pdev->dev, sizeof(*ld), GFP_KERNEL); + if (!ld) + return -ENOMEM; + /* Enable pci device */ ret = pci_enable_device(pdev); if (ret) { @@ -159,19 +519,39 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id pci_set_master(pdev); + plat->bsp_priv = ld; + plat->setup = loongson_dwmac_setup; + ld->dev = &pdev->dev; + if (dev_of_node(&pdev->dev)) { ret = loongson_dwmac_dt_config(pdev, plat, &res); if (ret) goto err_disable_device; - } else { - res.irq = pdev->irq; } memset(&res, 0, sizeof(res)); res.addr = pcim_iomap_table(pdev)[0]; + ld->loongson_id = readl(res.addr + GMAC_VERSION) & 0xff; + + if (ld->loongson_id == DWMAC_CORE_LS2K2000) { + plat->rx_queues_to_use = CHANNEL_NUM; + plat->tx_queues_to_use = CHANNEL_NUM; + + /* Only channel 0 supports checksum, + * so turn off checksum to enable multiple channels. + */ + for (i = 1; i < CHANNEL_NUM; i++) + plat->tx_queues_cfg[i].coe_unsupported = 1; - plat->tx_queues_to_use = 1; - plat->rx_queues_to_use = 1; + ret = loongson_dwmac_msi_config(pdev, plat, &res); + } else { + plat->tx_queues_to_use = 1; + plat->rx_queues_to_use = 1; + + ret = loongson_dwmac_intx_config(pdev, plat, &res); + } + if (ret) + goto err_disable_device; ret = stmmac_dvr_probe(&pdev->dev, plat, &res); if (ret) @@ -202,6 +582,7 @@ static void loongson_dwmac_remove(struct pci_dev *pdev) break; } + loongson_dwmac_msi_clear(pdev); pci_disable_device(pdev); } @@ -245,6 +626,7 @@ static SIMPLE_DEV_PM_OPS(loongson_dwmac_pm_ops, loongson_dwmac_suspend, 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);
Aside with the Loongson GMAC controllers which can be normally found on the LS2K1000 SoC and LS7A1000 chipset, Loongson released a new version of the network controllers called Loongson GNET. It has been synthesized into the new generation LS2K2000 SoC and LS7A2000 chipset with the next DW GMAC features enabled: DW GMAC IP-core: v3.73a Speeds: 10/100/1000Mbps Duplex: Full (both versions), Half (LS2K2000 SoC only) DMA-descriptors type: enhanced L3/L4 filters availability: Y VLAN hash table filter: Y PHY-interface: GMII (PHY is integrated into the chips) Remote Wake-up support: Y Mac Management Counters (MMC): Y Number of additional MAC addresses: 5 MAC Hash-based filter: Y Hash Table Size: 256 AV feature: Y (LS2K2000 SoC only) DMA channels: 8 (LS2K2000 SoC), 1 (LS7A2000 chipset) The integrated PHY has a weird problem with switching from the low speeds to 1000Mbps mode. The speedup procedure requires the PHY-link re-negotiation. Besides the LS2K2000 GNET controller the next peculiarities: 1. Split up Tx and Rx DMA IRQ status/mask bits: Name Tx Rx DMA_INTR_ENA_NIE = 0x00040000 | 0x00020000; DMA_INTR_ENA_AIE = 0x00010000 | 0x00008000; DMA_STATUS_NIS = 0x00040000 | 0x00020000; DMA_STATUS_AIS = 0x00010000 | 0x00008000; DMA_STATUS_FBI = 0x00002000 | 0x00001000; 2. Custom Synopsys ID hardwired into the GMAC_VERSION.SNPSVER field. It's 0x10 while it should have been 0x37 in accordance with the actual DW GMAC IP-core version. Thus in order to have the Loongson GNET controllers supported let's modify the Loongson DWMAC driver in accordance with all the peculiarities described above: 1. Create the Loongson GNET-specific stmmac_dma_ops::dma_interrupt() stmmac_dma_ops::init_chan() callbacks due to the non-standard DMA IRQ CSR flags layout. 2. Create the Loongson GNET-specific platform setup() method which gets to initialize the DMA-ops with the dwmac1000_dma_ops instance and overrides the callbacks described in 1, and overrides the custom Synopsys ID with the real one in order to have the rest of the HW-specific callbacks correctly detected by the driver core. 3. Make sure the Loongson GNET-specific platform setup() method enables the duplex modes supported by the controller. 4. Provide the plat_stmmacenet_data::fix_mac_speed() callback which will restart the link Auto-negotiation in case of the speed change. Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn> Signed-off-by: Yinggang Gu <guyinggang@loongson.cn> Signed-off-by: Yanteng Si <siyanteng@loongson.cn> --- drivers/net/ethernet/stmicro/stmmac/common.h | 1 + .../ethernet/stmicro/stmmac/dwmac-loongson.c | 390 +++++++++++++++++- 2 files changed, 387 insertions(+), 4 deletions(-)