2024年8月2日 01:09, "Serge Semin" [off-list ref] 写到:

On Mon, Jul 29, 2024 at 08:23:56PM +0800, Yanteng Si wrote:

quoted

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.

 

 Signed-off-by: Feiyang Chen [off-list ref]

 Signed-off-by: Yinggang Gu [off-list ref]

 Acked-by: Huacai Chen [off-list ref]

 Signed-off-by: Yanteng Si [off-list ref]

 [...]

 @@ -146,6 +450,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);

 @@ -162,6 +467,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) {

 @@ -184,6 +493,10 @@ 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];

 

 + plat->bsp_priv = ld;

 + plat->setup = loongson_dwmac_setup;

 + ld->loongson_id = readl(res.addr + GMAC_VERSION) & 0xff;

 +

 info = (struct stmmac_pci_info *)id->driver_data;

 ret = info->setup(pdev, plat);

 if (ret)

 @@ -196,6 +509,10 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id

 if (ret)

 goto err_disable_device;

 

 + /* Use the common MAC IRQ if per-channel MSIs allocation failed */

 + if (ld->loongson_id == DWMAC_CORE_LS_MULTICHAN)

 + loongson_dwmac_msi_config(pdev, plat, &res);

 +

 ret = stmmac_dvr_probe(&pdev->dev, plat, &res);

 if (ret)

 goto err_plat_clear;

 @@ -205,6 +522,8 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id

 err_plat_clear:

 if (dev_of_node(&pdev->dev))

 loongson_dwmac_dt_clear(pdev, plat);

 + else if (ld->loongson_id == DWMAC_CORE_LS_MULTICHAN)

 + loongson_dwmac_msi_clear(pdev);

Why implementing "else if" here if the loongson_dwmac_msi_config() is

called for both DT and ACPI cases? AFAICS it should be just "if".

Yeah, you are right!

quoted

err_disable_device:

 pci_disable_device(pdev);

 return ret;

 @@ -217,8 +536,10 @@ static void loongson_dwmac_remove(struct pci_dev *pdev)

 {

 struct net_device *ndev = dev_get_drvdata(&pdev->dev);

 struct stmmac_priv *priv = netdev_priv(ndev);

 + struct loongson_data *ld;

 int i;

 

 + ld = priv->plat->bsp_priv;

 of_node_put(priv->plat->mdio_node);

 stmmac_dvr_remove(&pdev->dev);

 

 @@ -232,6 +553,9 @@ static void loongson_dwmac_remove(struct pci_dev *pdev)

 break;

 }

 

 + if (ld->loongson_id == DWMAC_CORE_LS_MULTICHAN)

 + loongson_dwmac_msi_clear(pdev);

 +

Please do this just below the 

 if (dev_of_node(&pdev->dev))

 loongson_dwmac_dt_clear(pdev, priv->plat);

chunk so the remove() method would look similar to the

cleanup-on-error path of the probe() method.

OK.

Thanks,
Yanteng

-Serge(y)

quoted

pci_disable_device(pdev);

 }

 

 -- 

 2.31.4