| Message ID | 359b2c226e7b18d4af8bb827ca26a2e7869d5f85.1722253726.git.siyanteng@loongson.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | stmmac: Add Loongson platform support | expand |
On 7/29/24 14:23, 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. > 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. > > In order to have the device probe() and remove() methods less > complicated let's move the DT- and ACPI-specific code to the > respective sub-functions. > > Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn> > Signed-off-by: Yinggang Gu <guyinggang@loongson.cn> > Acked-by: Huacai Chen <chenhuacai@loongson.cn> > Signed-off-by: Yanteng Si <siyanteng@loongson.cn> @Serge: I think this addresses your comment on the previous iteration, but it would be great if you could have a look! Thanks, Paolo
Hi Paolo On Thu, Aug 01, 2024 at 01:32:56PM +0200, Paolo Abeni wrote: > On 7/29/24 14:23, 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. > > 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. > > > > In order to have the device probe() and remove() methods less > > complicated let's move the DT- and ACPI-specific code to the > > respective sub-functions. > > > > Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn> > > Signed-off-by: Yinggang Gu <guyinggang@loongson.cn> > > Acked-by: Huacai Chen <chenhuacai@loongson.cn> > > Signed-off-by: Yanteng Si <siyanteng@loongson.cn> > > @Serge: I think this addresses your comment on the previous iteration, but > it would be great if you could have a look! Thanks for reaching me out. I'll have a look at the series later today. -Serge(y) > > Thanks, > > Paolo >
Hi Yanteng, On Mon, Jul 29, 2024 at 08:23:55PM +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. > 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. > > In order to have the device probe() and remove() methods less > complicated let's move the DT- and ACPI-specific code to the > respective sub-functions. > > Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn> > Signed-off-by: Yinggang Gu <guyinggang@loongson.cn> > Acked-by: Huacai Chen <chenhuacai@loongson.cn> > Signed-off-by: Yanteng Si <siyanteng@loongson.cn> > --- > .../ethernet/stmicro/stmmac/dwmac-loongson.c | 159 +++++++++++------- > 1 file changed, 100 insertions(+), 59 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > index 10b49bea8e3c..010821fb6474 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->tx_queues_to_use = 1; > plat->rx_queues_to_use = 1; > @@ -65,20 +70,83 @@ static struct stmmac_pci_info loongson_gmac_pci_info = { > .setup = loongson_gmac_data, > }; > > +static int loongson_dwmac_dt_config(struct pci_dev *pdev, > + struct plat_stmmacenet_data *plat, > + struct stmmac_resources *res) > +{ > + struct device_node *np = dev_of_node(&pdev->dev); > + int ret; > + > + 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; > + } > + > + ret = of_alias_get_id(np, "ethernet"); > + if (ret >= 0) > + plat->bus_id = 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_put_node; > + } > + > + 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_put_node; > + } > + > + ret = device_get_phy_mode(&pdev->dev); > + if (ret < 0) { > + dev_err(&pdev->dev, "phy_mode not found\n"); > + ret = -ENODEV; > + goto err_put_node; > + } > + > + plat->phy_interface = ret; > + > +err_put_node: > + of_node_put(plat->mdio_node); > + > + return ret; > +} > + > +static void loongson_dwmac_dt_clear(struct pci_dev *pdev, > + struct plat_stmmacenet_data *plat) > +{ > + of_node_put(plat->mdio_node); > +} > + > +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; > +} > + > static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id *id) > { > struct plat_stmmacenet_data *plat; > struct stmmac_pci_info *info; > struct stmmac_resources res; > - struct device_node *np; > - int ret, i, phy_mode; > - > - np = dev_of_node(&pdev->dev); > - > - if (!np) { > - pr_info("dwmac_loongson_pci: No OF node\n"); > - return -ENODEV; > - } > + int ret, i; > > plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL); > if (!plat) > @@ -90,17 +158,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); > @@ -109,6 +169,8 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > goto err_put_node; return ret; * Once again BTW. > } > > + pci_set_master(pdev); > + > /* Get the base address of device */ > for (i = 0; i < PCI_STD_NUM_BARS; i++) { > if (pci_resource_len(pdev, i) == 0) > @@ -119,57 +181,33 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > break; > } > > - plat->bus_id = of_alias_get_id(np, "ethernet"); > - if (plat->bus_id < 0) > - plat->bus_id = pci_dev_id(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; > - } > - > - plat->phy_interface = phy_mode; > - > - pci_set_master(pdev); > - > memset(&res, 0, sizeof(res)); > res.addr = pcim_iomap_table(pdev)[0]; > > info = (struct stmmac_pci_info *)id->driver_data; > - ret = info->setup(plat); > + ret = info->setup(pdev, plat); > if (ret) > goto err_disable_device; > > - 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_device; > - } > - > - 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; > + 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; > - } > > ret = stmmac_dvr_probe(&pdev->dev, plat, &res); > if (ret) > - goto err_disable_device; > + goto err_plat_clear; > > - return ret; > + return 0; > > +err_plat_clear: > + if (dev_of_node(&pdev->dev)) > + loongson_dwmac_dt_clear(pdev, plat); > err_disable_device: > pci_disable_device(pdev); > + return ret; > err_put_node: > of_node_put(plat->mdio_node); > return ret; The main point of my v14 comment was to completely move the of_node_put(plat->mdio_node) call to the DT-config/clear methods (since it's OF-related config) and setting the loongson_dwmac_probe() method free from the direct MDIO-node putting. Don't you see that calling of_node_put() in the loongson_dwmac_probe() and loongson_dwmac_remove() is now redundant? Moreover don't you find the next chunk being weird: err_disable_device: pci_disable_device(pdev); + return ret; err_put_node: of_node_put(plat->mdio_node); return ret; ? Two return statements in a single cleanup-on-error path. After dropping the "err_put_node" label and the respective statements, the cleanup-on-error path will turn to being saner. > @@ -184,6 +222,9 @@ static void loongson_dwmac_remove(struct pci_dev *pdev) > of_node_put(priv->plat->mdio_node); This is done in the loongson_dwmac_dt_clear() method. So the direct of_node_put() call should be dropped. -Serge(y) > stmmac_dvr_remove(&pdev->dev); > > + if (dev_of_node(&pdev->dev)) > + loongson_dwmac_dt_clear(pdev, priv->plat); > + > for (i = 0; i < PCI_STD_NUM_BARS; i++) { > if (pci_resource_len(pdev, i) == 0) > continue; > -- > 2.31.4 >
2024年8月2日 00:50, "Serge Semin" <fancer.lancer@gmail.com> 写到: > > Hi Yanteng, > > On Mon, Jul 29, 2024 at 08:23:55PM +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. > > > > 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. > > > > > > > > In order to have the device probe() and remove() methods less > > > > complicated let's move the DT- and ACPI-specific code to the > > > > respective sub-functions. > > > > > > > > Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn> > > > > Signed-off-by: Yinggang Gu <guyinggang@loongson.cn> > > > > Acked-by: Huacai Chen <chenhuacai@loongson.cn> > > > > Signed-off-by: Yanteng Si <siyanteng@loongson.cn> > > > > --- > > > > .../ethernet/stmicro/stmmac/dwmac-loongson.c | 159 +++++++++++------- > > > > 1 file changed, 100 insertions(+), 59 deletions(-) > > > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > > > > index 10b49bea8e3c..010821fb6474 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->tx_queues_to_use = 1; > > > > plat->rx_queues_to_use = 1; > > > > @@ -65,20 +70,83 @@ static struct stmmac_pci_info loongson_gmac_pci_info = { > > > > .setup = loongson_gmac_data, > > > > }; > > > > > > > > +static int loongson_dwmac_dt_config(struct pci_dev *pdev, > > > > + struct plat_stmmacenet_data *plat, > > > > + struct stmmac_resources *res) > > > > +{ > > > > + struct device_node *np = dev_of_node(&pdev->dev); > > > > + int ret; > > > > + > > > > + 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; > > > > + } > > > > + > > > > + ret = of_alias_get_id(np, "ethernet"); > > > > + if (ret >= 0) > > > > + plat->bus_id = 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_put_node; > > > > + } > > > > + > > > > + 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_put_node; > > > > + } > > > > + > > > > + ret = device_get_phy_mode(&pdev->dev); > > > > + if (ret < 0) { > > > > + dev_err(&pdev->dev, "phy_mode not found\n"); > > > > + ret = -ENODEV; > > > > + goto err_put_node; > > > > + } > > > > + > > > > + plat->phy_interface = ret; > > > > + > > > > +err_put_node: > > > > + of_node_put(plat->mdio_node); > > > > + > > > > + return ret; > > > > +} > > > > + > > > > +static void loongson_dwmac_dt_clear(struct pci_dev *pdev, > > > > + struct plat_stmmacenet_data *plat) > > > > +{ > > > > + of_node_put(plat->mdio_node); > > > > +} > > > > + > > > > +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; > > > > +} > > > > + > > > > static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id *id) > > > > { > > > > struct plat_stmmacenet_data *plat; > > > > struct stmmac_pci_info *info; > > > > struct stmmac_resources res; > > > > - struct device_node *np; > > > > - int ret, i, phy_mode; > > > > - > > > > - np = dev_of_node(&pdev->dev); > > > > - > > > > - if (!np) { > > > > - pr_info("dwmac_loongson_pci: No OF node\n"); > > > > - return -ENODEV; > > > > - } > > > > + int ret, i; > > > > > > > > plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL); > > > > if (!plat) > > > > @@ -90,17 +158,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); > > > > @@ -109,6 +169,8 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > > > > goto err_put_node; > > > > return ret; > > * Once again BTW. > > > > > } > > > > > > > > + pci_set_master(pdev); > > > > + > > > > /* Get the base address of device */ > > > > for (i = 0; i < PCI_STD_NUM_BARS; i++) { > > > > if (pci_resource_len(pdev, i) == 0) > > > > @@ -119,57 +181,33 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id > > > > break; > > > > } > > > > > > > > - plat->bus_id = of_alias_get_id(np, "ethernet"); > > > > - if (plat->bus_id < 0) > > > > - plat->bus_id = pci_dev_id(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; > > > > - } > > > > - > > > > - plat->phy_interface = phy_mode; > > > > - > > > > - pci_set_master(pdev); > > > > - > > > > memset(&res, 0, sizeof(res)); > > > > res.addr = pcim_iomap_table(pdev)[0]; > > > > > > > > info = (struct stmmac_pci_info *)id->driver_data; > > > > - ret = info->setup(plat); > > > > + ret = info->setup(pdev, plat); > > > > if (ret) > > > > goto err_disable_device; > > > > > > > > - 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_device; > > > > - } > > > > - > > > > - 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; > > > > + 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; > > > > - } > > > > > > > > ret = stmmac_dvr_probe(&pdev->dev, plat, &res); > > > > if (ret) > > > > - goto err_disable_device; > > > > + goto err_plat_clear; > > > > > > > > - return ret; > > > > + return 0; > > > > > > > > +err_plat_clear: > > > > + if (dev_of_node(&pdev->dev)) > > > > + loongson_dwmac_dt_clear(pdev, plat); > > > > err_disable_device: > > > > pci_disable_device(pdev); > > > > + return ret; > > > > err_put_node: > > > > of_node_put(plat->mdio_node); > > > > return ret; > > > > The main point of my v14 comment was to completely move the > > of_node_put(plat->mdio_node) call to the DT-config/clear methods > > (since it's OF-related config) and setting the loongson_dwmac_probe() > > method free from the direct MDIO-node putting. Don't you see that > > calling of_node_put() in the loongson_dwmac_probe() and > > loongson_dwmac_remove() is now redundant? Oh, I see! Thanks! Thanks, Yanteng
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c index 10b49bea8e3c..010821fb6474 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->tx_queues_to_use = 1; plat->rx_queues_to_use = 1; @@ -65,20 +70,83 @@ static struct stmmac_pci_info loongson_gmac_pci_info = { .setup = loongson_gmac_data, }; +static int loongson_dwmac_dt_config(struct pci_dev *pdev, + struct plat_stmmacenet_data *plat, + struct stmmac_resources *res) +{ + struct device_node *np = dev_of_node(&pdev->dev); + int ret; + + 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; + } + + ret = of_alias_get_id(np, "ethernet"); + if (ret >= 0) + plat->bus_id = 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_put_node; + } + + 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_put_node; + } + + ret = device_get_phy_mode(&pdev->dev); + if (ret < 0) { + dev_err(&pdev->dev, "phy_mode not found\n"); + ret = -ENODEV; + goto err_put_node; + } + + plat->phy_interface = ret; + +err_put_node: + of_node_put(plat->mdio_node); + + return ret; +} + +static void loongson_dwmac_dt_clear(struct pci_dev *pdev, + struct plat_stmmacenet_data *plat) +{ + of_node_put(plat->mdio_node); +} + +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; +} + static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id *id) { struct plat_stmmacenet_data *plat; struct stmmac_pci_info *info; struct stmmac_resources res; - struct device_node *np; - int ret, i, phy_mode; - - np = dev_of_node(&pdev->dev); - - if (!np) { - pr_info("dwmac_loongson_pci: No OF node\n"); - return -ENODEV; - } + int ret, i; plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL); if (!plat) @@ -90,17 +158,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); @@ -109,6 +169,8 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id goto err_put_node; } + pci_set_master(pdev); + /* Get the base address of device */ for (i = 0; i < PCI_STD_NUM_BARS; i++) { if (pci_resource_len(pdev, i) == 0) @@ -119,57 +181,33 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id break; } - plat->bus_id = of_alias_get_id(np, "ethernet"); - if (plat->bus_id < 0) - plat->bus_id = pci_dev_id(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; - } - - plat->phy_interface = phy_mode; - - pci_set_master(pdev); - memset(&res, 0, sizeof(res)); res.addr = pcim_iomap_table(pdev)[0]; info = (struct stmmac_pci_info *)id->driver_data; - ret = info->setup(plat); + ret = info->setup(pdev, plat); if (ret) goto err_disable_device; - 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_device; - } - - 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; + 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; - } ret = stmmac_dvr_probe(&pdev->dev, plat, &res); if (ret) - goto err_disable_device; + goto err_plat_clear; - return ret; + return 0; +err_plat_clear: + if (dev_of_node(&pdev->dev)) + loongson_dwmac_dt_clear(pdev, plat); err_disable_device: pci_disable_device(pdev); + return ret; err_put_node: of_node_put(plat->mdio_node); return ret; @@ -184,6 +222,9 @@ static void loongson_dwmac_remove(struct pci_dev *pdev) of_node_put(priv->plat->mdio_node); stmmac_dvr_remove(&pdev->dev); + if (dev_of_node(&pdev->dev)) + loongson_dwmac_dt_clear(pdev, priv->plat); + for (i = 0; i < PCI_STD_NUM_BARS; i++) { if (pci_resource_len(pdev, i) == 0) continue;