Message ID | fa22df795219256b093659195c4609445a175a1d.1716973237.git.siyanteng@loongson.cn (mailing list archive) |
---|---|
State | Deferred |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | stmmac: Add Loongson platform support | expand |
On Wed, May 29, 2024 at 06:20:26PM +0800, Yanteng Si wrote: > The Loongson GMAC driver currently supports the network controllers > installed on the LS2K1000 SoC and LS7A1000 chipset, for which the GMAC > devices are required to be defined in the platform device tree source. > Let's extend the driver functionality with the case of having the > Loongson GMAC probed on the PCI bus with no device tree node defined > for it. That requires to make the device DT-node optional, to rely on > the IRQ line detected by the PCI core and to have the MDIO bus ID > calculated using the PCIe Domain+BDF numbers. We need to mention the ACPI-part here. Like this: "The Loongson GMAC driver currently supports the network controllers installed on the LS2K1000 SoC and LS7A1000 chipset, for which the GMAC devices are required to be defined in the platform device tree source. But Loongson machines may have UEFI (implies ACPI) or PMON/UBOOT (implies FDT) as the system bootloaders. In order to have both system configurations support let's extend the driver functionality with the case of having the Loongson GMAC probed on the PCI bus with no device tree node defined for it. That requires to make the device DT-node optional, to rely on the IRQ line detected by the PCI core and to have the MDIO bus ID calculated using the PCIe Domain+BDF numbers." This shall well justify the change introduced in this patch and in the patch "net: stmmac: dwmac-loongson: Add loongson_dwmac_dt_config" which content I suggest to merge in into this one. > > Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn> > Signed-off-by: Yinggang Gu <guyinggang@loongson.cn> > Signed-off-by: Yanteng Si <siyanteng@loongson.cn> > --- > .../ethernet/stmicro/stmmac/dwmac-loongson.c | 105 +++++++++--------- > 1 file changed, 53 insertions(+), 52 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > index fec2aa0607d4..8bcf9d522781 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > @@ -12,11 +12,15 @@ > #define PCI_DEVICE_ID_LOONGSON_GMAC 0x7a03 > > struct stmmac_pci_info { > - int (*setup)(struct plat_stmmacenet_data *plat); > + int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat); > }; > > -static void loongson_default_data(struct plat_stmmacenet_data *plat) > +static void loongson_default_data(struct pci_dev *pdev, > + struct plat_stmmacenet_data *plat) > { > + /* Get bus_id, this can be overwritten later */ > + plat->bus_id = pci_dev_id(pdev); > + > plat->clk_csr = 2; /* clk_csr_i = 20-35MHz & MDC = clk_csr_i/16 */ > plat->has_gmac = 1; > plat->force_sf_dma_mode = 1; > @@ -49,9 +53,10 @@ static void loongson_default_data(struct plat_stmmacenet_data *plat) > plat->dma_cfg->pblx8 = true; > } > > -static int loongson_gmac_data(struct plat_stmmacenet_data *plat) > +static int loongson_gmac_data(struct pci_dev *pdev, > + struct plat_stmmacenet_data *plat) > { > - loongson_default_data(plat); > + loongson_default_data(pdev, plat); > > plat->phy_interface = PHY_INTERFACE_MODE_RGMII_ID; > > @@ -68,15 +73,10 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > struct stmmac_pci_info *info; > struct stmmac_resources res; > struct device_node *np; > - int ret, i, phy_mode; > + int ret, i; > > np = dev_of_node(&pdev->dev); > > - if (!np) { > - pr_info("dwmac_loongson_pci: No OF node\n"); > - return -ENODEV; > - } > - > plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL); > if (!plat) > return -ENOMEM; > @@ -87,17 +87,9 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > if (!plat->mdio_bus_data) > return -ENOMEM; > > - plat->mdio_node = of_get_child_by_name(np, "mdio"); > - if (plat->mdio_node) { > - dev_info(&pdev->dev, "Found MDIO subnode\n"); > - plat->mdio_bus_data->needs_reset = true; > - } > - > plat->dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*plat->dma_cfg), GFP_KERNEL); > - if (!plat->dma_cfg) { > - ret = -ENOMEM; > - goto err_put_node; > - } > + if (!plat->dma_cfg) > + return -ENOMEM; > > /* Enable pci device */ > ret = pci_enable_device(pdev); > @@ -117,49 +109,58 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > } > > info = (struct stmmac_pci_info *)id->driver_data; > - ret = info->setup(plat); > + ret = info->setup(pdev, plat); > if (ret) > goto err_disable_device; > > - plat->bus_id = of_alias_get_id(np, "ethernet"); > - if (plat->bus_id < 0) > - plat->bus_id = pci_dev_id(pdev); > + pci_set_master(pdev); > > - phy_mode = device_get_phy_mode(&pdev->dev); > - if (phy_mode < 0) { > - dev_err(&pdev->dev, "phy_mode not found\n"); > - ret = phy_mode; > - goto err_disable_device; > - } > + if (np) { > + plat->mdio_node = of_get_child_by_name(np, "mdio"); > + if (plat->mdio_node) { > + dev_info(&pdev->dev, "Found MDIO subnode\n"); > + plat->mdio_bus_data->needs_reset = true; > + } > > - plat->phy_interface = phy_mode; > + ret = of_alias_get_id(np, "ethernet"); > + if (ret >= 0) > + plat->bus_id = ret; > > - pci_set_master(pdev); > + ret = device_get_phy_mode(&pdev->dev); > + if (ret < 0) { > + dev_err(&pdev->dev, "phy_mode not found\n"); > + goto err_disable_device; > + } > + > + plat->phy_interface = ret; > + > + res.irq = of_irq_get_byname(np, "macirq"); > + if (res.irq < 0) { > + dev_err(&pdev->dev, "IRQ macirq not found\n"); > + ret = -ENODEV; > + goto err_disable_msi; > + } > + > + 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"); > + ret = -ENODEV; > + goto err_disable_msi; > + } > + } else { > + res.irq = pdev->irq; > + } Please merge in the patch: [PATCH net-next v13 11/15] net: stmmac: dwmac-loongson: Add loongson_dwmac_dt_config content into this one and introduce the method: static int loongson_dwmac_acpi_config(struct pci_dev *pdev, struct plat_stmmacenet_data *plat, struct stmmac_resources *res) { if (!pdev->irq) return -EINVAL; res->irq = pdev->irq; return 0; } just below the loongson_dwmac_dt_config() function. Thus the only code left in the probe() method would be: if (dev_of_node(&pdev->dev)) ret = loongson_dwmac_dt_config(pdev, plat, &res); else ret = loongson_dwmac_acpi_config(pdev, plat, &res); if (ret) goto err_disable_device; As a result this patch will look simpler and more coherent. So will the probe() method with no additional change required for that. -Serge(y) > > pci_enable_msi(pdev); > memset(&res, 0, sizeof(res)); > res.addr = pcim_iomap_table(pdev)[0]; > > - res.irq = of_irq_get_byname(np, "macirq"); > - if (res.irq < 0) { > - dev_err(&pdev->dev, "IRQ macirq not found\n"); > - ret = -ENODEV; > - goto err_disable_msi; > - } > - > - 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"); > - ret = -ENODEV; > - goto err_disable_msi; > - } > - > plat->tx_queues_to_use = 1; > plat->rx_queues_to_use = 1; > > -- > 2.31.4 >
在 2024/7/2 17:35, Serge Semin 写道: > On Wed, May 29, 2024 at 06:20:26PM +0800, Yanteng Si wrote: >> The Loongson GMAC driver currently supports the network controllers >> installed on the LS2K1000 SoC and LS7A1000 chipset, for which the GMAC >> devices are required to be defined in the platform device tree source. >> Let's extend the driver functionality with the case of having the >> Loongson GMAC probed on the PCI bus with no device tree node defined >> for it. That requires to make the device DT-node optional, to rely on >> the IRQ line detected by the PCI core and to have the MDIO bus ID >> calculated using the PCIe Domain+BDF numbers. > We need to mention the ACPI-part here. Like this: > > "The Loongson GMAC driver currently supports the network controllers > installed on the LS2K1000 SoC and LS7A1000 chipset, for which the GMAC > devices are required to be defined in the platform device tree source. > But Loongson machines may have UEFI (implies ACPI) or PMON/UBOOT > (implies FDT) as the system bootloaders. In order to have both system > configurations support let's extend the driver functionality with the > case of having the Loongson GMAC probed on the PCI bus with no device > tree node defined for it. That requires to make the device DT-node > optional, to rely on the IRQ line detected by the PCI core and to > have the MDIO bus ID calculated using the PCIe Domain+BDF numbers." > > This shall well justify the change introduced in this patch and in the > patch "net: stmmac: dwmac-loongson: Add loongson_dwmac_dt_config" > which content I suggest to merge in into this one. OK, Thanks! > >> Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn> >> Signed-off-by: Yinggang Gu <guyinggang@loongson.cn> >> Signed-off-by: Yanteng Si <siyanteng@loongson.cn> >> --- >> .../ethernet/stmicro/stmmac/dwmac-loongson.c | 105 +++++++++--------- >> 1 file changed, 53 insertions(+), 52 deletions(-) >> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c >> index fec2aa0607d4..8bcf9d522781 100644 >> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c >> @@ -12,11 +12,15 @@ >> #define PCI_DEVICE_ID_LOONGSON_GMAC 0x7a03 >> >> struct stmmac_pci_info { >> - int (*setup)(struct plat_stmmacenet_data *plat); >> + int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat); >> }; >> >> -static void loongson_default_data(struct plat_stmmacenet_data *plat) >> +static void loongson_default_data(struct pci_dev *pdev, >> + struct plat_stmmacenet_data *plat) >> { >> + /* Get bus_id, this can be overwritten later */ >> + plat->bus_id = pci_dev_id(pdev); >> + >> plat->clk_csr = 2; /* clk_csr_i = 20-35MHz & MDC = clk_csr_i/16 */ >> plat->has_gmac = 1; >> plat->force_sf_dma_mode = 1; >> @@ -49,9 +53,10 @@ static void loongson_default_data(struct plat_stmmacenet_data *plat) >> plat->dma_cfg->pblx8 = true; >> } >> >> -static int loongson_gmac_data(struct plat_stmmacenet_data *plat) >> +static int loongson_gmac_data(struct pci_dev *pdev, >> + struct plat_stmmacenet_data *plat) >> { >> - loongson_default_data(plat); >> + loongson_default_data(pdev, plat); >> >> plat->phy_interface = PHY_INTERFACE_MODE_RGMII_ID; >> >> @@ -68,15 +73,10 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id >> struct stmmac_pci_info *info; >> struct stmmac_resources res; >> struct device_node *np; >> - int ret, i, phy_mode; >> + int ret, i; >> >> np = dev_of_node(&pdev->dev); >> >> - if (!np) { >> - pr_info("dwmac_loongson_pci: No OF node\n"); >> - return -ENODEV; >> - } >> - >> plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL); >> if (!plat) >> return -ENOMEM; >> @@ -87,17 +87,9 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id >> if (!plat->mdio_bus_data) >> return -ENOMEM; >> >> - plat->mdio_node = of_get_child_by_name(np, "mdio"); >> - if (plat->mdio_node) { >> - dev_info(&pdev->dev, "Found MDIO subnode\n"); >> - plat->mdio_bus_data->needs_reset = true; >> - } >> - >> plat->dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*plat->dma_cfg), GFP_KERNEL); >> - if (!plat->dma_cfg) { >> - ret = -ENOMEM; >> - goto err_put_node; >> - } >> + if (!plat->dma_cfg) >> + return -ENOMEM; >> >> /* Enable pci device */ >> ret = pci_enable_device(pdev); >> @@ -117,49 +109,58 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id >> } >> >> info = (struct stmmac_pci_info *)id->driver_data; >> - ret = info->setup(plat); >> + ret = info->setup(pdev, plat); >> if (ret) >> goto err_disable_device; >> >> - plat->bus_id = of_alias_get_id(np, "ethernet"); >> - if (plat->bus_id < 0) >> - plat->bus_id = pci_dev_id(pdev); >> + pci_set_master(pdev); >> >> - phy_mode = device_get_phy_mode(&pdev->dev); >> - if (phy_mode < 0) { >> - dev_err(&pdev->dev, "phy_mode not found\n"); >> - ret = phy_mode; >> - goto err_disable_device; >> - } >> + if (np) { >> + plat->mdio_node = of_get_child_by_name(np, "mdio"); >> + if (plat->mdio_node) { >> + dev_info(&pdev->dev, "Found MDIO subnode\n"); >> + plat->mdio_bus_data->needs_reset = true; >> + } >> >> - plat->phy_interface = phy_mode; >> + ret = of_alias_get_id(np, "ethernet"); >> + if (ret >= 0) >> + plat->bus_id = ret; >> >> - pci_set_master(pdev); >> + ret = device_get_phy_mode(&pdev->dev); >> + if (ret < 0) { >> + dev_err(&pdev->dev, "phy_mode not found\n"); >> + goto err_disable_device; >> + } >> + >> + plat->phy_interface = ret; >> + >> + res.irq = of_irq_get_byname(np, "macirq"); >> + if (res.irq < 0) { >> + dev_err(&pdev->dev, "IRQ macirq not found\n"); >> + ret = -ENODEV; >> + goto err_disable_msi; >> + } >> + >> + 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"); >> + ret = -ENODEV; >> + goto err_disable_msi; >> + } >> + } else { >> + res.irq = pdev->irq; >> + } > Please merge in the patch: > [PATCH net-next v13 11/15] net: stmmac: dwmac-loongson: Add loongson_dwmac_dt_config > content into this one and introduce the method: > > static int loongson_dwmac_acpi_config(struct pci_dev *pdev, > struct plat_stmmacenet_data *plat, > struct stmmac_resources *res) > { > if (!pdev->irq) > return -EINVAL; > > res->irq = pdev->irq; > > return 0; > } > > just below the loongson_dwmac_dt_config() function. Thus the only code > left in the probe() method would be: > > if (dev_of_node(&pdev->dev)) > ret = loongson_dwmac_dt_config(pdev, plat, &res); > else > ret = loongson_dwmac_acpi_config(pdev, plat, &res); > > if (ret) > goto err_disable_device; > > As a result this patch will look simpler and more coherent. So will > the probe() method with no additional change required for that. OK! Thanks, Yanteng
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c index fec2aa0607d4..8bcf9d522781 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c @@ -12,11 +12,15 @@ #define PCI_DEVICE_ID_LOONGSON_GMAC 0x7a03 struct stmmac_pci_info { - int (*setup)(struct plat_stmmacenet_data *plat); + int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat); }; -static void loongson_default_data(struct plat_stmmacenet_data *plat) +static void loongson_default_data(struct pci_dev *pdev, + struct plat_stmmacenet_data *plat) { + /* Get bus_id, this can be overwritten later */ + plat->bus_id = pci_dev_id(pdev); + plat->clk_csr = 2; /* clk_csr_i = 20-35MHz & MDC = clk_csr_i/16 */ plat->has_gmac = 1; plat->force_sf_dma_mode = 1; @@ -49,9 +53,10 @@ static void loongson_default_data(struct plat_stmmacenet_data *plat) plat->dma_cfg->pblx8 = true; } -static int loongson_gmac_data(struct plat_stmmacenet_data *plat) +static int loongson_gmac_data(struct pci_dev *pdev, + struct plat_stmmacenet_data *plat) { - loongson_default_data(plat); + loongson_default_data(pdev, plat); plat->phy_interface = PHY_INTERFACE_MODE_RGMII_ID; @@ -68,15 +73,10 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id struct stmmac_pci_info *info; struct stmmac_resources res; struct device_node *np; - int ret, i, phy_mode; + int ret, i; np = dev_of_node(&pdev->dev); - if (!np) { - pr_info("dwmac_loongson_pci: No OF node\n"); - return -ENODEV; - } - plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL); if (!plat) return -ENOMEM; @@ -87,17 +87,9 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id if (!plat->mdio_bus_data) return -ENOMEM; - plat->mdio_node = of_get_child_by_name(np, "mdio"); - if (plat->mdio_node) { - dev_info(&pdev->dev, "Found MDIO subnode\n"); - plat->mdio_bus_data->needs_reset = true; - } - plat->dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*plat->dma_cfg), GFP_KERNEL); - if (!plat->dma_cfg) { - ret = -ENOMEM; - goto err_put_node; - } + if (!plat->dma_cfg) + return -ENOMEM; /* Enable pci device */ ret = pci_enable_device(pdev); @@ -117,49 +109,58 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id } info = (struct stmmac_pci_info *)id->driver_data; - ret = info->setup(plat); + ret = info->setup(pdev, plat); if (ret) goto err_disable_device; - plat->bus_id = of_alias_get_id(np, "ethernet"); - if (plat->bus_id < 0) - plat->bus_id = pci_dev_id(pdev); + pci_set_master(pdev); - phy_mode = device_get_phy_mode(&pdev->dev); - if (phy_mode < 0) { - dev_err(&pdev->dev, "phy_mode not found\n"); - ret = phy_mode; - goto err_disable_device; - } + if (np) { + plat->mdio_node = of_get_child_by_name(np, "mdio"); + if (plat->mdio_node) { + dev_info(&pdev->dev, "Found MDIO subnode\n"); + plat->mdio_bus_data->needs_reset = true; + } - plat->phy_interface = phy_mode; + ret = of_alias_get_id(np, "ethernet"); + if (ret >= 0) + plat->bus_id = ret; - pci_set_master(pdev); + ret = device_get_phy_mode(&pdev->dev); + if (ret < 0) { + dev_err(&pdev->dev, "phy_mode not found\n"); + goto err_disable_device; + } + + plat->phy_interface = ret; + + res.irq = of_irq_get_byname(np, "macirq"); + if (res.irq < 0) { + dev_err(&pdev->dev, "IRQ macirq not found\n"); + ret = -ENODEV; + goto err_disable_msi; + } + + 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"); + ret = -ENODEV; + goto err_disable_msi; + } + } else { + res.irq = pdev->irq; + } pci_enable_msi(pdev); memset(&res, 0, sizeof(res)); res.addr = pcim_iomap_table(pdev)[0]; - res.irq = of_irq_get_byname(np, "macirq"); - if (res.irq < 0) { - dev_err(&pdev->dev, "IRQ macirq not found\n"); - ret = -ENODEV; - goto err_disable_msi; - } - - 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"); - ret = -ENODEV; - goto err_disable_msi; - } - plat->tx_queues_to_use = 1; plat->rx_queues_to_use = 1;