| 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: <a218ab75-4c0c-62bb-f5b9-a9d692d38880@loongson.cn>
Date: Wed, 25 Nov 2020 15:07:35 +0800
From: Zhi Li <lizhi01@...ngson.cn>
To: Jiaxun Yang <jiaxun.yang@...goat.com>, davem@...emloft.net,
kuba@...nel.org, mcoquelin.stm32@...il.com
Cc: lixuefeng@...ngson.com, gaojuxin@...ngson.com,
linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH] stmmac: pci: Add support for LS7A bridge chip
Hi Jiaxun,
It's my fault, I didn't know to send mail to the mailing list.
We will discuss your suggestions and correct the errors in the code.
I will send a new patch to the mailing list.
Thanks.
-Lizhi
On 11/23/2020 06:31 PM, Jiaxun Yang wrote:
> Hi Lizhi,
>
> You didn't send the patch to any mail list, is this intentional?
>
> 在 2020/11/23 18:03, lizhi01 写道:
>> Add gmac driver to support LS7A bridge chip.
>>
>> Signed-off-by: lizhi01 <lizhi01@...ngson.cn>
>> ---
>> arch/mips/configs/loongson3_defconfig | 4 +-
>> drivers/net/ethernet/stmicro/stmmac/Kconfig | 8 +
>> drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
>> .../net/ethernet/stmicro/stmmac/dwmac-loongson.c | 194
>> +++++++++++++++++++++
>> 4 files changed, 206 insertions(+), 1 deletion(-)
>> create mode 100644
>> drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
>>
>> diff --git a/arch/mips/configs/loongson3_defconfig
>> b/arch/mips/configs/loongson3_defconfig
>> index 38a817e..2e8d2be 100644
>> --- a/arch/mips/configs/loongson3_defconfig
>> +++ b/arch/mips/configs/loongson3_defconfig
>> @@ -225,7 +225,9 @@ CONFIG_R8169=y
>> # CONFIG_NET_VENDOR_SILAN is not set
>> # CONFIG_NET_VENDOR_SIS is not set
>> # CONFIG_NET_VENDOR_SMSC is not set
>> -# CONFIG_NET_VENDOR_STMICRO is not set
>> +CONFIG_NET_VENDOR_STMICR=y
>> +CONFIG_STMMAC_ETH=y
>> +CONFIG_DWMAC_LOONGSON=y
>> # CONFIG_NET_VENDOR_SUN is not set
>> # CONFIG_NET_VENDOR_TEHUTI is not set
>> # CONFIG_NET_VENDOR_TI is not set
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> index 53f14c5..30117cb 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> @@ -230,6 +230,14 @@ config DWMAC_INTEL
>> This selects the Intel platform specific bus support for the
>> stmmac driver. This driver is used for Intel Quark/EHL/TGL.
>> +config DWMAC_LOONGSON
>> + tristate "Intel GMAC support"
>> + depends on STMMAC_ETH && PCI
>> + depends on COMMON_CLK
>> + help
>> + This selects the Intel platform specific bus support for the
>> + stmmac driver.
>
> Intel ???
>
>> +
>> config STMMAC_PCI
>> tristate "STMMAC PCI bus support"
>> depends on STMMAC_ETH && PCI
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile
>> b/drivers/net/ethernet/stmicro/stmmac/Makefile
>> index 24e6145..11ea4569 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
>> @@ -34,4 +34,5 @@ dwmac-altr-socfpga-objs := altr_tse_pcs.o
>> dwmac-socfpga.o
>> obj-$(CONFIG_STMMAC_PCI) += stmmac-pci.o
>> obj-$(CONFIG_DWMAC_INTEL) += dwmac-intel.o
>> +obj-$(CONFIG_DWMAC_LOONGSON) += dwmac-loongson.o
>> stmmac-pci-objs:= stmmac_pci.o
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
>> new file mode 100644
>> index 0000000..765412e
>> --- /dev/null
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
>> @@ -0,0 +1,194 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2020, Loongson Corporation
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/pci.h>
>> +#include <linux/dmi.h>
>> +#include <linux/device.h>
>> +#include <linux/of_irq.h>
>> +#include "stmmac.h"
>> +
>> +struct stmmac_pci_info {
>> + int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data
>> *plat);
>> +};
>> +
>> +static void common_default_data(struct plat_stmmacenet_data *plat)
>> +{
>> + plat->clk_csr = 2;
>> + plat->has_gmac = 1;
>> + plat->force_sf_dma_mode = 1;
>> +
>> + plat->mdio_bus_data->needs_reset = true;
>> +
>> + plat->multicast_filter_bins = HASH_TABLE_SIZE;
>> +
>> + plat->unicast_filter_entries = 1;
>> +
>> + plat->maxmtu = JUMBO_LEN;
>> +
>> + plat->tx_queues_to_use = 1;
>> + plat->rx_queues_to_use = 1;
>> +
>> + plat->tx_queues_cfg[0].use_prio = false;
>> + plat->rx_queues_cfg[0].use_prio = false;
>> +
>> + plat->rx_queues_cfg[0].pkt_route = 0x0;
>> +}
>> +
>> +static int loongson_default_data(struct pci_dev *pdev, struct
>> plat_stmmacenet_data *plat)
>> +{
>> + common_default_data(plat);
>> +
>> + plat->bus_id = pci_dev_id(pdev);
>> + plat->phy_addr = -1;
>> + plat->interface = PHY_INTERFACE_MODE_GMII;
>> +
>> + plat->dma_cfg->pbl = 32;
>> + plat->dma_cfg->pblx8 = true;
>> +
>> + plat->multicast_filter_bins = 256;
>> +
>> + return 0;
>> +}
>
>
> You can merge common and Loongson config as the driver is solely used
> by Loongson.
>
> The callback is not necessary as well...
>
>
>> +
>> +static const struct stmmac_pci_info loongson_pci_info = {
>> + .setup = loongson_default_data,
>> +};
>> +
>> +static int loongson_gmac_probe(struct pci_dev *pdev, const struct
>> pci_device_id *id)
>> +{
>> + struct stmmac_pci_info *info = (struct stmmac_pci_info
>> *)id->driver_data;
>> + struct plat_stmmacenet_data *plat;
>> + struct stmmac_resources res;
>> + int ret, i, lpi_irq;
>> + struct device_node *np;
>> +
>> + plat = devm_kzalloc(&pdev->dev, sizeof(struct
>> plat_stmmacenet_data), GFP_KERNEL);
>> + if (!plat)
>> + return -ENOMEM;
>> +
>> + plat->mdio_bus_data = devm_kzalloc(&pdev->dev, sizeof(struct
>> stmmac_mdio_bus_data), GFP_KERNEL);
>> + if (!plat->mdio_bus_data) {
>> + kfree(plat);
>> + return -ENOMEM;
>> + }
>> +
>> + plat->dma_cfg = devm_kzalloc(&pdev->dev, sizeof(struct
>> stmmac_dma_cfg), GFP_KERNEL);
>> + if (!plat->dma_cfg) {
>> + kfree(plat);
>> + return -ENOMEM;
>> + }
>> +
>> + ret = pci_enable_device(pdev);
>> + if (ret) {
>> + dev_err(&pdev->dev, "%s: ERROR: failed to enable device\n",
>> __func__);
>> + kfree(plat);
>> + return ret;
>> + }
>> +
>> + for (i = 0; i < PCI_STD_NUM_BARS; i++) {
>> + if (pci_resource_len(pdev, i) == 0)
>> + continue;
>> + ret = pcim_iomap_regions(pdev, BIT(0), pci_name(pdev));
>> + if (ret)
>> + return ret;
>> + break;
>> + }
>
>
> The BAR order is fixed on Loongson so there is no need to check it one
> by one.
>
> Simply use BAR0 instead.
>
>
>> +
>> + pci_set_master(pdev);
>> +
>> + ret = info->setup(pdev, plat);
>> + if (ret)
>> + return ret;
>> +
>> + pci_enable_msi(pdev);
>> +
>> + memset(&res, 0, sizeof(res));
>> + res.addr = pcim_iomap_table(pdev)[i];
>> + res.irq = pdev->irq;
>> + res.wol_irq = pdev->irq;
>> +
>> + np = dev_of_node(&pdev->dev);
>
>
> Please check the node earlier and bailing out in case if there is no
> node.
>
> Also you should get both IRQs via DT to avoid misordering.
>
>
>> + lpi_irq = of_irq_get_byname(np, "eth_lpi");
>> + res.lpi_irq = lpi_irq;
>> +
>> + return stmmac_dvr_probe(&pdev->dev, plat, &res);
>> +}
>> +
>> +static void loongson_gmac_remove(struct pci_dev *pdev)
>> +{
>> + int i;
>> +
>> + stmmac_dvr_remove(&pdev->dev);
>> +
>> + for (i = 0; i < PCI_STD_NUM_BARS; i++) {
>> + if (pci_resource_len(pdev, i) == 0)
>> + continue;
>> + pcim_iounmap_regions(pdev, BIT(i));
>> + break;
>> + }
>> +
>> + pci_disable_device(pdev);
>> +}
>> +
>> +static int __maybe_unused loongson_eth_pci_suspend(struct device *dev)
>> +{
>> + struct pci_dev *pdev = to_pci_dev(dev);
>> + int ret;
>> +
>> + ret = stmmac_suspend(dev);
>> + if (ret)
>> + return ret;
>> +
>> + ret = pci_save_state(pdev);
>> + if (ret)
>> + return ret;
>> +
>> + pci_disable_device(pdev);
>> + pci_wake_from_d3(pdev, true);
>> + return 0;
>> +}
>> +
>> +static int __maybe_unused loongson_eth_pci_resume(struct device *dev)
>> +{
>> + struct pci_dev *pdev = to_pci_dev(dev);
>> + int ret;
>> +
>> + pci_restore_state(pdev);
>> + pci_set_power_state(pdev, PCI_D0);
>> +
>> + ret = pci_enable_device(pdev);
>> + if (ret)
>> + return ret;
>> +
>> + pci_set_master(pdev);
>> +
>> + return stmmac_resume(dev);
>> +}
>> +
>> +static SIMPLE_DEV_PM_OPS(loongson_eth_pm_ops,
>> loongson_eth_pci_suspend, loongson_eth_pci_resume);
>> +
>> +#define PCI_DEVICE_ID_LOONGSON_GMAC 0x7a03
>> +
>> +static const struct pci_device_id loongson_gmac_table[] = {
>> + { PCI_DEVICE_DATA(LOONGSON, GMAC, &loongson_pci_info) },
>> + {}
>> +};
>> +MODULE_DEVICE_TABLE(pci, loongson_gmac_table);
>> +
>> +struct pci_driver loongson_gmac_driver = {
>> + .name = "loongson gmac",
>> + .id_table = loongson_gmac_table,
>> + .probe = loongson_gmac_probe,
>> + .remove = loongson_gmac_remove,
>> + .driver = {
>> + .pm = &loongson_eth_pm_ops,
>> + },
>> +};
>> +
>> +module_pci_driver(loongson_gmac_driver);
>> +
>> +MODULE_DESCRIPTION("Loongson DWMAC PCI driver");
>> +MODULE_AUTHOR("Zhi Li <lizhi01@...ngson.com>");
>> +MODULE_LICENSE("GPL v2");
>
>
> Thanks
>
> - Jiaxun
Powered by blists - more mailing lists