netdev - Re: [PATCH net-next v13 14/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: <9ba4aa1d-16b0-4f5c-a87b-bf6f709f80a7@loongson.cn>
Date: Wed, 5 Jun 2024 17:43:18 +0800
From: Yanteng Si <siyanteng@...ngson.cn>
To: Huacai Chen <chenhuacai@...nel.org>
Cc: andrew@...n.ch, hkallweit1@...il.com, peppe.cavallaro@...com,
 alexandre.torgue@...s.st.com, joabreu@...opsys.com, fancer.lancer@...il.com,
 Jose.Abreu@...opsys.com, linux@...linux.org.uk, guyinggang@...ngson.cn,
 netdev@...r.kernel.org, chris.chenfeiyang@...il.com, si.yanteng@...ux.dev
Subject: Re: [PATCH net-next v13 14/15] net: stmmac: dwmac-loongson: Add
 Loongson GNET support


在 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