| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <35e9ba13-cbd6-4ecd-a8e4-5f4acb40d9b8@linux.dev>
Date: Wed, 7 Aug 2024 15:15:29 +0800
From: YanTeng Si <si.yanteng@...ux.dev>
To: Serge Semin <fancer.lancer@...il.com>, Yanteng Si <siyanteng@...ngson.cn>
Cc: andrew@...n.ch, hkallweit1@...il.com, peppe.cavallaro@...com,
alexandre.torgue@...s.st.com, joabreu@...opsys.com, diasyzhang@...cent.com,
Jose.Abreu@...opsys.com, chenhuacai@...nel.org, linux@...linux.org.uk,
guyinggang@...ngson.cn, netdev@...r.kernel.org, chris.chenfeiyang@...il.com,
Huacai Chen <chenhuacai@...ngson.cn>
Subject: Re: [PATCH net-next v16 12/14] net: stmmac: dwmac-loongson: Add
Loongson Multi-channels GMAC support
在 2024/8/7 3:17, Serge Semin 写道:
> On Tue, Aug 06, 2024 at 07:00:22PM +0800, Yanteng Si wrote:
>> 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 <chenfeiyang@...ngson.cn>
>> Signed-off-by: Yinggang Gu <guyinggang@...ngson.cn>
>> Acked-by: Huacai Chen <chenhuacai@...ngson.cn>
>> Signed-off-by: Yanteng Si <siyanteng@...ngson.cn>
>> ---
>>
>> ...
>>
>> +
>> +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, vecs, vecs, PCI_IRQ_MSI);
>> + if (ret < 0) {
>> + dev_warn(&pdev->dev, "Failed to allocate MSI IRQs\n");
>> + return ret;
>> + }
>> +
>> + res->irq = pci_irq_vector(pdev, 0);
>> +
>> + 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;
>> +
>> + return 0;
>> +}
>> +
>> +static void loongson_dwmac_msi_clear(struct pci_dev *pdev)
>> +{
>> + pci_free_irq_vectors(pdev);
>> +}
>> +
>> static int loongson_dwmac_dt_config(struct pci_dev *pdev,
>> struct plat_stmmacenet_data *plat,
>> struct stmmac_resources *res)
>> @@ -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,7 @@ 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);
>> + loongson_dwmac_msi_clear(pdev);
> Em, why have you dropped the if-statement here? That has caused the
> Simon note to be posted. Please get it back:
> + if (ld->loongson_id == DWMAC_CORE_LS_MULTICHAN)
> + loongson_dwmac_msi_clear(pdev);
OK.
>
>> err_disable_device:
>> pci_disable_device(pdev);
>> return ret;
>> @@ -214,12 +532,15 @@ 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;
>> stmmac_dvr_remove(&pdev->dev);
>>
>> if (dev_of_node(&pdev->dev))
>> loongson_dwmac_dt_clear(pdev, priv->plat);
>> + loongson_dwmac_msi_clear(pdev);
> Ditto. Please get back the conditional MSI-clear method execution:
> +
> + if (ld->loongson_id == DWMAC_CORE_LS_MULTICHAN)
> + loongson_dwmac_msi_clear(pdev);
>
> * Note the empty line above the if-statement.
OK, Thanks!
Thanks,
Yanteng
>
> -Serge(y)
>
>>
>> for (i = 0; i < PCI_STD_NUM_BARS; i++) {
>> if (pci_resource_len(pdev, i) == 0)
>> --
>> 2.31.4
>>
Powered by blists - more mailing lists