| 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: <CAAhV-H5ELoqM8n5A-DD7LOzjb2mkRDR+pM-CAOcfGwZYcVQQ-A@mail.gmail.com>
Date: Mon, 21 Apr 2025 12:20:17 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: Yanteng Si <si.yanteng@...ux.dev>
Cc: Huacai Chen <chenhuacai@...ngson.cn>, Andrew Lunn <andrew+netdev@...n.ch>,
"David S . Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Feiyang Chen <chris.chenfeiyang@...il.com>, loongarch@...ts.linux.dev,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
Andrew Lunn <andrew@...n.ch>, Henry Chen <chenx97@...c.io>, Biao Dong <dongbiao@...ngson.cn>,
Baoqi Zhang <zhangbaoqi@...ngson.cn>
Subject: Re: [PATCH net-next V2 2/3] net: stmmac: dwmac-loongson: Add new
multi-chan IP core support
Hi, Yanteng,
On Mon, Apr 21, 2025 at 10:04 AM Yanteng Si <si.yanteng@...ux.dev> wrote:
>
>
> 在 4/16/25 10:41 PM, Huacai Chen 写道:
> > Add a new multi-chan IP core (0x12) support which is used in Loongson-
> > 2K3000/Loongson-3B6000M. Compared with the 0x10 core, the new 0x12 core
> > reduces channel numbers from 8 to 4, but checksum is supported for all
> > channels.
> >
> > Reviewed-by: Andrew Lunn <andrew@...n.ch>
> > Tested-by: Henry Chen <chenx97@...c.io>
> > Tested-by: Biao Dong <dongbiao@...ngson.cn>
> > Signed-off-by: Baoqi Zhang <zhangbaoqi@...ngson.cn>
> > Signed-off-by: Huacai Chen <chenhuacai@...ngson.cn>
> > ---
> > .../ethernet/stmicro/stmmac/dwmac-loongson.c | 62 +++++++++++--------
> > 1 file changed, 37 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > index 2fb7a137b312..57917f26ab4d 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > @@ -68,10 +68,11 @@
> >
> > #define PCI_DEVICE_ID_LOONGSON_GMAC 0x7a03
> > #define PCI_DEVICE_ID_LOONGSON_GNET 0x7a13
> > -#define DWMAC_CORE_LS_MULTICHAN 0x10 /* Loongson custom ID */
> > -#define CHANNEL_NUM 8
> > +#define DWMAC_CORE_MULTICHAN_V1 0x10 /* Loongson custom ID 0x10 */
> > +#define DWMAC_CORE_MULTICHAN_V2 0x12 /* Loongson custom ID 0x12 */
> >
> > struct loongson_data {
> > + u32 multichan;
>
> In order to make the logic clearer, I suggest splitting this patch.：
>
>
> 2/4 Add multichan for loongson_data
>
> 3/4 Add new multi-chan IP core support
I don't think the patch is unclear now, the multichan flag is really a
combination of DWMAC_CORE_MULTICHAN_V1 and DWMAC_CORE_MULTICHAN_V2.
>
>
> > u32 loongson_id;
> > struct device *dev;
> > };
> > @@ -119,18 +120,29 @@ static void loongson_default_data(struct pci_dev *pdev,
> > plat->dma_cfg->pbl = 32;
> > plat->dma_cfg->pblx8 = true;
> >
> > - if (ld->loongson_id == DWMAC_CORE_LS_MULTICHAN) {
> > - plat->rx_queues_to_use = CHANNEL_NUM;
> > - plat->tx_queues_to_use = CHANNEL_NUM;
> > + switch (ld->loongson_id) {
> > + case DWMAC_CORE_MULTICHAN_V1:
>
> How about adding some comments? For example:
>
> case DWMAC_CORE_MULTICHAN_V1: /* 2K2000 */
> case DWMAC_CORE_MULTICHAN_V2: /* 2K3000 and 3B6000M */
Do you know why we deprecate PRID (a.k.a SOC name) detection and
prefer CPUCFG detection in cpu_probe()? Because SOC-types and function
features are orthogonal, they should not be strictly bound. There will
be other SOCs using MULTICHAN_V1 or MULTICHAN_V2, should we update the
comments every time when a new SOC publishes? There may also be one
SOC with different IP-cores, then how to comment on this case?
> ...
>
> > + ld->multichan = 1;
> > + plat->rx_queues_to_use = 8;
> > + plat->tx_queues_to_use = 8;
> >
> > /* Only channel 0 supports checksum,
> > * so turn off checksum to enable multiple channels.
> > */
> > - for (int i = 1; i < CHANNEL_NUM; i++)
> > + for (int i = 1; i < 8; i++)
> > plat->tx_queues_cfg[i].coe_unsupported = 1;
> > - } else {
> > +
> > + break;
> > + case DWMAC_CORE_MULTICHAN_V2:
> > + ld->multichan = 1;
> > + plat->rx_queues_to_use = 4;
> > + plat->tx_queues_to_use = 4;
> > + break;
> > + default:
> > + ld->multichan = 0;
> > plat->tx_queues_to_use = 1;
> > plat->rx_queues_to_use = 1;
> > + break;
> > }
> > }
> >
> > @@ -328,14 +340,14 @@ static struct mac_device_info *loongson_dwmac_setup(void *apriv)
> > return NULL;
> >
> > /* 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
> > + * v3.50a and v3.73a IP-cores. But the HW designers have changed
> > + * the GMAC_VERSION.SNPSVER field to the custom 0x10/0x12 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_LS_MULTICHAN) {
> > + if (ld->multichan) {
> > priv->synopsys_id = DWMAC_CORE_3_70;
> > *dma = dwmac1000_dma_ops;
> > dma->init_chan = loongson_dwmac_dma_init_channel;
> > @@ -356,13 +368,13 @@ static struct mac_device_info *loongson_dwmac_setup(void *apriv)
> > 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.
> > + /* Loongson GMAC doesn't support the flow control. Loongson GNET
> > + * without multi-channel 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_LS_MULTICHAN)
> > + if (ld->multichan)
> > mac->link.caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
> > MAC_10 | MAC_100 | MAC_1000;
> > else
> > @@ -391,9 +403,11 @@ static int loongson_dwmac_msi_config(struct pci_dev *pdev,
> > struct plat_stmmacenet_data *plat,
> > struct stmmac_resources *res)
> > {
> > - int i, ret, vecs;
> > + int i, ch_num, ret, vecs;
> >
> > - vecs = roundup_pow_of_two(CHANNEL_NUM * 2 + 1);
> > + ch_num = min(plat->tx_queues_to_use, plat->rx_queues_to_use);
>
> I'm curious. Will there still be hardware with RX not equal to TX in the
> future?
Currently rx queue number is equal to tx queue number, but
min(plat->tx_queues_to_use, plat->rx_queues_to_use) is still the best
solution. If not, which one should be used here? tx_queues_to_use or
rx_queues_to_use?
Huacai
>
>
> Thanks,
>
> Yanteng
>
>
Powered by blists - more mailing lists