netdev - Re: [PATCH net-next v12 13/15] net: stmmac: dwmac-loongson: Add Loongson GNET support

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <460a6b52-249e-4d50-8d3e-28cc9da6a01b@loongson.cn>
Date: Fri, 17 May 2024 16:42:51 +0800
From: Yanteng Si <siyanteng@...ngson.cn>
To: Huacai Chen <chenhuacai@...nel.org>, Serge Semin <fancer.lancer@...il.com>
Cc: andrew@...n.ch, hkallweit1@...il.com, peppe.cavallaro@...com,
 alexandre.torgue@...s.st.com, joabreu@...opsys.com, Jose.Abreu@...opsys.com,
 linux@...linux.org.uk, guyinggang@...ngson.cn, netdev@...r.kernel.org,
 chris.chenfeiyang@...il.com, siyanteng01@...il.com
Subject: Re: [PATCH net-next v12 13/15] net: stmmac: dwmac-loongson: Add
 Loongson GNET support

Hi Huacai, Serge,

在 2024/5/15 21:55, Huacai Chen 写道:
>>>> Once again about the naming. From the retrospective point of view the
>>>> so called legacy PCI IRQs (in fact PCI INTx) and the platform IRQs
>>>> look similar because these are just the level-type signals connected
>>>> to the system IRQ controller. But when it comes to the PCI_Express_,
>>>> the implementation is completely different. The PCIe INTx is just the
>>>> PCIe TLPs of special type, like MSI. Upon receiving these special
>>>> messages the PCIe host controller delivers the IRQ up to the
>>>> respective system IRQ controller. So in order to avoid the confusion
>>>> between the actual legacy PCI INTx, PCI Express INTx and the just
>>>> platform IRQs, it's better to emphasize the actual way of the IRQs
>>>> delivery. In this case it's the later method.
>>> You are absolutely right, and I think I found a method to use your
>>> framework to solve our problems:
>>>
>>>     static int loongson_dwmac_config_irqs(struct pci_dev *pdev,
>>>                                            struct plat_stmmacenet_data *plat,
>>>                                            struct stmmac_resources *res)
>>>     {
>>>         int i, ret, vecs;
>>>
>>>         /* INT NAME | MAC | CH7 rx | CH7 tx | ... | CH0 rx | CH0 tx |
>>>          * --------- ----- -------- --------  ...  -------- --------
>>>          * IRQ NUM  |  0  |   1    |   2    | ... |   15   |   16   |
>>>          */
>>>         vecs = plat->rx_queues_to_use + plat->tx_queues_to_use + 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[CHANNELS_NUM - 1 - i] =
>>>                                 pci_irq_vector(pdev, 1 + i * 2);
>>>                 }
>>>                 for (i = 0; i < plat->tx_queues_to_use; i++) {
>>>                         res->tx_irq[CHANNELS_NUM - 1 - i] =
>>>                                 pci_irq_vector(pdev, 2 + i * 2);
>>>                 }
>>>
>>>                 plat->flags |= STMMAC_FLAG_MULTI_MSI_EN;
>>>         }
>>>
>>>         res->irq = pci_irq_vector(pdev, 0);
>>>
>>>       if (np) {
>>>           res->irq = of_irq_get_byname(np, "macirq");
>>>           if (res->irq < 0) {
>>>              dev_err(&pdev->dev, "IRQ macirq not found\n");
>>>              return -ENODEV;
>>>           }
>>>
>>>           res->wol_irq = of_irq_get_byname(np, "eth_wake_irq");
>>>           if (res->wol_irq < 0) {
>>>              dev_info(&pdev->dev,
>>>                   "IRQ eth_wake_irq not found, using macirq\n");
>>>              res->wol_irq = res->irq;
>>>           }
>>>
>>>           res->lpi_irq = of_irq_get_byname(np, "eth_lpi");
>>>           if (res->lpi_irq < 0) {
>>>              dev_err(&pdev->dev, "IRQ eth_lpi not found\n");
>>>              return -ENODEV;
>>>           }
>>>       }
>>>         return 0;
>>>     }
>>>
>>> If your agree, Yanteng can use this method in V13, then avoid furthur changes.
>> Since yesterday I have been too relaxed sitting back to explain in
>> detail the problems with the code above. Shortly speaking, no to the
>> method designed as above.
> This function is copy-paste from your version which you suggest to
> Yanteng, and plus the fallback parts for DT. If you don't want to
> discuss it any more, we can discuss after V13.

All right. I have been preparing v13 and will send it as soon as possible.

Let's continue the discussion in v13. Of course, I will copy the part 
that has

not received a clear reply to v13.

>
> BTW, we cannot remove "res->wol_irq = res->irq", because Loongson
> GMAC/GNET indeed supports WoL.

Okay, I will not drop it in v13.


Thanks,

Yanteng