| Message ID | b43293919f4ddb869a795e41266f7c3107f79faf.1702990507.git.siyanteng@loongson.cn (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | stmmac: Add Loongson platform support | expand |
On Tue, Dec 19, 2023 at 10:17:06PM +0800, Yanteng Si wrote: > @@ -125,42 +126,48 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, > if (ret) > goto err_disable_device; > > - bus_id = of_alias_get_id(np, "ethernet"); > - if (bus_id >= 0) > - plat->bus_id = bus_id; > + if (np) { > + bus_id = of_alias_get_id(np, "ethernet"); > + if (bus_id >= 0) > + plat->bus_id = bus_id; > > - 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; > + 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; > } > > - plat->phy_interface = phy_mode; > - So this is why phy_interface changes in patch 2. It would have been good to make a forward reference to this change to explain in patch 2 why the "default" value has been set there. Or maybe move the setting of that default value into this patch?
在 2024/1/2 18:40, Russell King (Oracle) 写道: > On Tue, Dec 19, 2023 at 10:17:06PM +0800, Yanteng Si wrote: >> @@ -125,42 +126,48 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, >> if (ret) >> goto err_disable_device; >> >> - bus_id = of_alias_get_id(np, "ethernet"); >> - if (bus_id >= 0) >> - plat->bus_id = bus_id; >> + if (np) { >> + bus_id = of_alias_get_id(np, "ethernet"); >> + if (bus_id >= 0) >> + plat->bus_id = bus_id; >> >> - 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; >> + 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; >> } >> >> - plat->phy_interface = phy_mode; >> - > So this is why phy_interface changes in patch 2. It would have been good > to make a forward reference to this change to explain in patch 2 why the > "default" value has been set there. Or maybe move the setting of that > default value into this patch? Thank you for your review. Sorry, I have been modifying the code according to serge's comments for the past 10 days. I have not forgotten your comment. I will reply to you next week. Thanks, Yanteng >
在 2024/1/2 18:40, Russell King (Oracle) 写道: > On Tue, Dec 19, 2023 at 10:17:06PM +0800, Yanteng Si wrote: >> @@ -125,42 +126,48 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, >> if (ret) >> goto err_disable_device; >> >> - bus_id = of_alias_get_id(np, "ethernet"); >> - if (bus_id >= 0) >> - plat->bus_id = bus_id; >> + if (np) { >> + bus_id = of_alias_get_id(np, "ethernet"); >> + if (bus_id >= 0) >> + plat->bus_id = bus_id; >> >> - 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; >> + 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; >> } >> >> - plat->phy_interface = phy_mode; >> - > So this is why phy_interface changes in patch 2. It would have been good > to make a forward reference to this change to explain in patch 2 why the > "default" value has been set there. Or maybe move the setting of that > default value into this patch? This sounds great, I will try it. Thanks, Yanteng >
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c index 56d1fd8c61e1..0d79104d7fd3 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c @@ -16,6 +16,10 @@ struct stmmac_pci_info { static void loongson_default_data(struct pci_dev *pdev, struct plat_stmmacenet_data *plat) { + /* Get bus_id, this can be overloaded later */ + plat->bus_id = (pci_domain_nr(pdev->bus) << 16) | + PCI_DEVID(pdev->bus->number, pdev->devfn); + plat->clk_csr = 2; /* clk_csr_i = 20-35MHz & MDC = clk_csr_i/16 */ plat->has_gmac = 1; plat->force_sf_dma_mode = 1; @@ -56,6 +60,9 @@ static int loongson_gmac_data(struct pci_dev *pdev, plat->dma_cfg->pbl = 32; plat->dma_cfg->pblx8 = true; + plat->clk_ref_rate = 125000000; + plat->clk_ptp_rate = 125000000; + return 0; } @@ -72,13 +79,6 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, struct stmmac_resources res; struct device_node *np; - 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; @@ -94,6 +94,7 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, if (!plat->dma_cfg) return -ENOMEM; + np = dev_of_node(&pdev->dev); plat->mdio_node = of_get_child_by_name(np, "mdio"); if (plat->mdio_node) { dev_info(&pdev->dev, "Found MDIO subnode\n"); @@ -125,42 +126,48 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, if (ret) goto err_disable_device; - bus_id = of_alias_get_id(np, "ethernet"); - if (bus_id >= 0) - plat->bus_id = bus_id; + if (np) { + bus_id = of_alias_get_id(np, "ethernet"); + if (bus_id >= 0) + plat->bus_id = bus_id; - 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; + 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; } - plat->phy_interface = phy_mode; - 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; + if (np) { + 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; + res.wol_irq = pdev->irq; } ret = stmmac_dvr_probe(&pdev->dev, plat, &res);