| Message ID | 776bfe84003b203ebe320dc7bf6b98707a667fa9.1706601050.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, Jan 30, 2024 at 04:43:24PM +0800, Yanteng Si wrote: > Add loongson_dwmac_config and moving irq config related > code to loongson_dwmac_config. > > Removing MSI to prepare for adding loongson multi-channel > support later. Please detach this change into a separate patch and thoroughly explain why it was necessary. > > Signed-off-by: Yanteng Si <siyanteng@loongson.cn> > Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn> > Signed-off-by: Yinggang Gu <guyinggang@loongson.cn> > --- > .../ethernet/stmicro/stmmac/dwmac-loongson.c | 85 ++++++++++++------- > 1 file changed, 55 insertions(+), 30 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > index 979c9b6dab3f..e7ce027cc14e 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > @@ -11,8 +11,46 @@ > > struct stmmac_pci_info { > int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat); > + int (*config)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat, > + struct stmmac_resources *res, struct device_node *np); > }; > > +static int loongson_dwmac_config_legacy(struct pci_dev *pdev, > + struct plat_stmmacenet_data *plat, > + struct stmmac_resources *res, > + struct device_node *np) > +{ > + if (np) { > + res->irq = of_irq_get_byname(np, "macirq"); > + if (res->irq < 0) { > + dev_err(&pdev->dev, "IRQ macirq not found\n"); > + return -ENODEV; > + } > + > + 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"); > + return -ENODEV; > + } > + } else { > + res->irq = pdev->irq; > + res->wol_irq = res->irq; > + } > + > + plat->flags &= ~STMMAC_FLAG_MULTI_MSI_EN; > + dev_info(&pdev->dev, "%s: Single IRQ enablement successful\n", > + __func__); Why is this here all of the sudden? I don't see this in the original code. Please move it to the patch which requires the flag setup/cleanup or drop if it isn't necessary. > + > + return 0; > +} > + > static void loongson_default_data(struct pci_dev *pdev, > struct plat_stmmacenet_data *plat) > { > @@ -66,8 +104,21 @@ static int loongson_gmac_data(struct pci_dev *pdev, > return 0; > } > > +static int loongson_gmac_config(struct pci_dev *pdev, > + struct plat_stmmacenet_data *plat, > + struct stmmac_resources *res, > + struct device_node *np) > +{ > + int ret; > + > + ret = loongson_dwmac_config_legacy(pdev, plat, res, np); > + > + return ret; > +} > + You introduce the config callback here and convert to a dummy method in [PATCH 07/11] net: stmmac: dwmac-loongson: Add multi-channel supports for loongson It's just pointless. What about introducing the loongson_dwmac_config_legacy() method and call it directly? > static struct stmmac_pci_info loongson_gmac_pci_info = { > .setup = loongson_gmac_data, > + .config = loongson_gmac_config, > }; > > static int loongson_dwmac_probe(struct pci_dev *pdev, > @@ -139,44 +190,19 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, > plat->phy_interface = phy_mode; > } > > - pci_enable_msi(pdev); See my first note in this message. -Serge(y) > memset(&res, 0, sizeof(res)); > res.addr = pcim_iomap_table(pdev)[0]; > > - 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 = info->config(pdev, plat, &res, np); > + if (ret) > + goto err_disable_device; > > ret = stmmac_dvr_probe(&pdev->dev, plat, &res); > if (ret) > - goto err_disable_msi; > + goto err_disable_device; > > return ret; > > -err_disable_msi: > - pci_disable_msi(pdev); > err_disable_device: > pci_disable_device(pdev); > err_put_node: > @@ -200,7 +226,6 @@ static void loongson_dwmac_remove(struct pci_dev *pdev) > break; > } > > - pci_disable_msi(pdev); > pci_disable_device(pdev); > } > > -- > 2.31.4 >
在 2024/2/6 01:01, Serge Semin 写道: > On Tue, Jan 30, 2024 at 04:43:24PM +0800, Yanteng Si wrote: >> Add loongson_dwmac_config and moving irq config related >> code to loongson_dwmac_config. >> >> Removing MSI to prepare for adding loongson multi-channel >> support later. > Please detach this change into a separate patch and thoroughly explain > why it was necessary. OK. > >> Signed-off-by: Yanteng Si <siyanteng@loongson.cn> >> Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn> >> Signed-off-by: Yinggang Gu <guyinggang@loongson.cn> >> --- >> .../ethernet/stmicro/stmmac/dwmac-loongson.c | 85 ++++++++++++------- >> 1 file changed, 55 insertions(+), 30 deletions(-) >> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c >> index 979c9b6dab3f..e7ce027cc14e 100644 >> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c >> @@ -11,8 +11,46 @@ >> >> struct stmmac_pci_info { >> int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat); >> + int (*config)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat, >> + struct stmmac_resources *res, struct device_node *np); >> }; >> >> +static int loongson_dwmac_config_legacy(struct pci_dev *pdev, >> + struct plat_stmmacenet_data *plat, >> + struct stmmac_resources *res, >> + struct device_node *np) >> +{ >> + if (np) { >> + res->irq = of_irq_get_byname(np, "macirq"); >> + if (res->irq < 0) { >> + dev_err(&pdev->dev, "IRQ macirq not found\n"); >> + return -ENODEV; >> + } >> + >> + 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"); >> + return -ENODEV; >> + } >> + } else { >> + res->irq = pdev->irq; >> + res->wol_irq = res->irq; >> + } >> + >> + plat->flags &= ~STMMAC_FLAG_MULTI_MSI_EN; >> + dev_info(&pdev->dev, "%s: Single IRQ enablement successful\n", >> + __func__); > Why is this here all of the sudden? I don't see this in the original > code. Please move it to the patch which requires the flag > setup/cleanup or drop if it isn't necessary. + plat->flags &= ~STMMAC_FLAG_MULTI_MSI_EN; This cannot be removed because it appeared in a rebase(v4 -> v5). See <https://lore.kernel.org/all/20230710090001.303225-9-brgl@bgdev.pl/> + dev_info(&pdev->dev, "%s: Single IRQ enablement successful\n", + __func__); OK, drop it. > >> + >> + return 0; >> +} >> + >> static void loongson_default_data(struct pci_dev *pdev, >> struct plat_stmmacenet_data *plat) >> { >> @@ -66,8 +104,21 @@ static int loongson_gmac_data(struct pci_dev *pdev, >> return 0; >> } >> >> +static int loongson_gmac_config(struct pci_dev *pdev, >> + struct plat_stmmacenet_data *plat, >> + struct stmmac_resources *res, >> + struct device_node *np) >> +{ >> + int ret; >> + >> + ret = loongson_dwmac_config_legacy(pdev, plat, res, np); >> + >> + return ret; >> +} >> + > You introduce the config callback here and convert to a dummy method > in > [PATCH 07/11] net: stmmac: dwmac-loongson: Add multi-channel supports for loongson > It's just pointless. What about introducing the > loongson_dwmac_config_legacy() method and call it directly? OK, I will try. > >> static struct stmmac_pci_info loongson_gmac_pci_info = { >> .setup = loongson_gmac_data, >> + .config = loongson_gmac_config, >> }; >> >> static int loongson_dwmac_probe(struct pci_dev *pdev, >> @@ -139,44 +190,19 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, >> plat->phy_interface = phy_mode; >> } >> >> - pci_enable_msi(pdev); > See my first note in this message. OK. Thanks, Yanteng > > -Serge(y) > >> memset(&res, 0, sizeof(res)); >> res.addr = pcim_iomap_table(pdev)[0]; >> >> - 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 = info->config(pdev, plat, &res, np); >> + if (ret) >> + goto err_disable_device; >> >> ret = stmmac_dvr_probe(&pdev->dev, plat, &res); >> if (ret) >> - goto err_disable_msi; >> + goto err_disable_device; >> >> return ret; >> >> -err_disable_msi: >> - pci_disable_msi(pdev); >> err_disable_device: >> pci_disable_device(pdev); >> err_put_node: >> @@ -200,7 +226,6 @@ static void loongson_dwmac_remove(struct pci_dev *pdev) >> break; >> } >> >> - pci_disable_msi(pdev); >> pci_disable_device(pdev); >> } >> >> -- >> 2.31.4 >>
On Wed, Mar 13, 2024 at 04:14:28PM +0800, Yanteng Si wrote: > > 在 2024/2/6 01:01, Serge Semin 写道: > > On Tue, Jan 30, 2024 at 04:43:24PM +0800, Yanteng Si wrote: > > > Add loongson_dwmac_config and moving irq config related > > > code to loongson_dwmac_config. > > > > > > Removing MSI to prepare for adding loongson multi-channel > > > support later. > > Please detach this change into a separate patch and thoroughly explain > > why it was necessary. > OK. > > > > > Signed-off-by: Yanteng Si <siyanteng@loongson.cn> > > > Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn> > > > Signed-off-by: Yinggang Gu <guyinggang@loongson.cn> > > > --- > > > .../ethernet/stmicro/stmmac/dwmac-loongson.c | 85 ++++++++++++------- > > > 1 file changed, 55 insertions(+), 30 deletions(-) > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > > > index 979c9b6dab3f..e7ce027cc14e 100644 > > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c > > > @@ -11,8 +11,46 @@ > > > struct stmmac_pci_info { > > > int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat); > > > + int (*config)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat, > > > + struct stmmac_resources *res, struct device_node *np); > > > }; > > > +static int loongson_dwmac_config_legacy(struct pci_dev *pdev, > > > + struct plat_stmmacenet_data *plat, > > > + struct stmmac_resources *res, > > > + struct device_node *np) > > > +{ > > > + if (np) { > > > + res->irq = of_irq_get_byname(np, "macirq"); > > > + if (res->irq < 0) { > > > + dev_err(&pdev->dev, "IRQ macirq not found\n"); > > > + return -ENODEV; > > > + } > > > + > > > + 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"); > > > + return -ENODEV; > > > + } > > > + } else { > > > + res->irq = pdev->irq; > > > + res->wol_irq = res->irq; > > > + } > > > + > > > + plat->flags &= ~STMMAC_FLAG_MULTI_MSI_EN; > > > + dev_info(&pdev->dev, "%s: Single IRQ enablement successful\n", > > > + __func__); > > Why is this here all of the sudden? I don't see this in the original > > code. Please move it to the patch which requires the flag > > setup/cleanup or drop if it isn't necessary. > > + plat->flags &= ~STMMAC_FLAG_MULTI_MSI_EN; > This cannot be removed because it appeared in a rebase(v4 -> v5). See > <https://lore.kernel.org/all/20230710090001.303225-9-brgl@bgdev.pl/> AFAICS it _can_ be removed. The patch you referred to is a formal conversion of - plat->multi_msi_en = 0; to + plat->flags &= ~STMMAC_FLAG_MULTI_MSI_EN; First of all the "multi_msi_en" field clearance had been redundant there since the code setting the flag was executed after the code which may cause the field clearance performed. Second AFAICS the "multi_msi_en" field clearance was originally added to emphasize the functions semantics: intel_eth_config_multi_msi() - config multi IRQ device, intel_eth_config_single_msi() - config single IRQ device. So in your case there is no any reason of clearing the STMMAC_FLAG_MULTI_MSI_EN flag. Please, either drop it or move the change into a separate patch. -Serge(y) > + dev_info(&pdev->dev, "%s: Single IRQ enablement successful\n", > + __func__); > > OK, drop it. > > > > > > + > > > + return 0; > > > +} > > > + > > > static void loongson_default_data(struct pci_dev *pdev, > > > struct plat_stmmacenet_data *plat) > > > { > > > @@ -66,8 +104,21 @@ static int loongson_gmac_data(struct pci_dev *pdev, > > > return 0; > > > } > > > +static int loongson_gmac_config(struct pci_dev *pdev, > > > + struct plat_stmmacenet_data *plat, > > > + struct stmmac_resources *res, > > > + struct device_node *np) > > > +{ > > > + int ret; > > > + > > > + ret = loongson_dwmac_config_legacy(pdev, plat, res, np); > > > + > > > + return ret; > > > +} > > > + > > You introduce the config callback here and convert to a dummy method > > in > > [PATCH 07/11] net: stmmac: dwmac-loongson: Add multi-channel supports for loongson > > It's just pointless. What about introducing the > > loongson_dwmac_config_legacy() method and call it directly? > OK, I will try. > > > > > static struct stmmac_pci_info loongson_gmac_pci_info = { > > > .setup = loongson_gmac_data, > > > + .config = loongson_gmac_config, > > > }; > > > static int loongson_dwmac_probe(struct pci_dev *pdev, > > > @@ -139,44 +190,19 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, > > > plat->phy_interface = phy_mode; > > > } > > > - pci_enable_msi(pdev); > > See my first note in this message. > > OK. > > > Thanks, > > Yanteng > > > > > -Serge(y) > > > > > memset(&res, 0, sizeof(res)); > > > res.addr = pcim_iomap_table(pdev)[0]; > > > - 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 = info->config(pdev, plat, &res, np); > > > + if (ret) > > > + goto err_disable_device; > > > ret = stmmac_dvr_probe(&pdev->dev, plat, &res); > > > if (ret) > > > - goto err_disable_msi; > > > + goto err_disable_device; > > > return ret; > > > -err_disable_msi: > > > - pci_disable_msi(pdev); > > > err_disable_device: > > > pci_disable_device(pdev); > > > err_put_node: > > > @@ -200,7 +226,6 @@ static void loongson_dwmac_remove(struct pci_dev *pdev) > > > break; > > > } > > > - pci_disable_msi(pdev); > > > pci_disable_device(pdev); > > > } > > > -- > > > 2.31.4 > > > >
在 2024/3/19 21:43, Serge Semin 写道: > On Wed, Mar 13, 2024 at 04:14:28PM +0800, Yanteng Si wrote: >> 在 2024/2/6 01:01, Serge Semin 写道: >>> On Tue, Jan 30, 2024 at 04:43:24PM +0800, Yanteng Si wrote: >>>> Add loongson_dwmac_config and moving irq config related >>>> code to loongson_dwmac_config. >>>> >>>> Removing MSI to prepare for adding loongson multi-channel >>>> support later. >>> Please detach this change into a separate patch and thoroughly explain >>> why it was necessary. >> OK. >>>> Signed-off-by: Yanteng Si <siyanteng@loongson.cn> >>>> Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn> >>>> Signed-off-by: Yinggang Gu <guyinggang@loongson.cn> >>>> --- >>>> .../ethernet/stmicro/stmmac/dwmac-loongson.c | 85 ++++++++++++------- >>>> 1 file changed, 55 insertions(+), 30 deletions(-) >>>> >>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c >>>> index 979c9b6dab3f..e7ce027cc14e 100644 >>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c >>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c >>>> @@ -11,8 +11,46 @@ >>>> struct stmmac_pci_info { >>>> int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat); >>>> + int (*config)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat, >>>> + struct stmmac_resources *res, struct device_node *np); >>>> }; >>>> +static int loongson_dwmac_config_legacy(struct pci_dev *pdev, >>>> + struct plat_stmmacenet_data *plat, >>>> + struct stmmac_resources *res, >>>> + struct device_node *np) >>>> +{ >>>> + if (np) { >>>> + res->irq = of_irq_get_byname(np, "macirq"); >>>> + if (res->irq < 0) { >>>> + dev_err(&pdev->dev, "IRQ macirq not found\n"); >>>> + return -ENODEV; >>>> + } >>>> + >>>> + 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"); >>>> + return -ENODEV; >>>> + } >>>> + } else { >>>> + res->irq = pdev->irq; >>>> + res->wol_irq = res->irq; >>>> + } >>>> + >>>> + plat->flags &= ~STMMAC_FLAG_MULTI_MSI_EN; >>>> + dev_info(&pdev->dev, "%s: Single IRQ enablement successful\n", >>>> + __func__); >>> Why is this here all of the sudden? I don't see this in the original >>> code. Please move it to the patch which requires the flag >>> setup/cleanup or drop if it isn't necessary. >> + plat->flags &= ~STMMAC_FLAG_MULTI_MSI_EN; >> This cannot be removed because it appeared in a rebase(v4 -> v5). See >> <https://lore.kernel.org/all/20230710090001.303225-9-brgl@bgdev.pl/> > AFAICS it _can_ be removed. The patch you referred to is a formal > conversion of > - plat->multi_msi_en = 0; > to > + plat->flags &= ~STMMAC_FLAG_MULTI_MSI_EN; > First of all the "multi_msi_en" field clearance had been > redundant there since the code setting the flag was executed after the > code which may cause the field clearance performed. Second AFAICS the > "multi_msi_en" field clearance was originally added to emphasize the > functions semantics: > intel_eth_config_multi_msi() - config multi IRQ device, > intel_eth_config_single_msi() - config single IRQ device. > > So in your case there is no any reason of clearing the > STMMAC_FLAG_MULTI_MSI_EN flag. Please, either drop it or move the > change into a separate patch. OK, you are right. drop it. Thanks, Yanteng > > -Serge(y) > >> + dev_info(&pdev->dev, "%s: Single IRQ enablement successful\n", >> + __func__); >> >> OK, drop it. >> >>>> + >>>> + return 0; >>>> +} >>>> + >>>> static void loongson_default_data(struct pci_dev *pdev, >>>> struct plat_stmmacenet_data *plat) >>>> { >>>> @@ -66,8 +104,21 @@ static int loongson_gmac_data(struct pci_dev *pdev, >>>> return 0; >>>> } >>>> +static int loongson_gmac_config(struct pci_dev *pdev, >>>> + struct plat_stmmacenet_data *plat, >>>> + struct stmmac_resources *res, >>>> + struct device_node *np) >>>> +{ >>>> + int ret; >>>> + >>>> + ret = loongson_dwmac_config_legacy(pdev, plat, res, np); >>>> + >>>> + return ret; >>>> +} >>>> + >>> You introduce the config callback here and convert to a dummy method >>> in >>> [PATCH 07/11] net: stmmac: dwmac-loongson: Add multi-channel supports for loongson >>> It's just pointless. What about introducing the >>> loongson_dwmac_config_legacy() method and call it directly? >> OK, I will try. >>>> static struct stmmac_pci_info loongson_gmac_pci_info = { >>>> .setup = loongson_gmac_data, >>>> + .config = loongson_gmac_config, >>>> }; >>>> static int loongson_dwmac_probe(struct pci_dev *pdev, >>>> @@ -139,44 +190,19 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, >>>> plat->phy_interface = phy_mode; >>>> } >>>> - pci_enable_msi(pdev); >>> See my first note in this message. >> OK. >> >> >> Thanks, >> >> Yanteng >> >>> -Serge(y) >>> >>>> memset(&res, 0, sizeof(res)); >>>> res.addr = pcim_iomap_table(pdev)[0]; >>>> - 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 = info->config(pdev, plat, &res, np); >>>> + if (ret) >>>> + goto err_disable_device; >>>> ret = stmmac_dvr_probe(&pdev->dev, plat, &res); >>>> if (ret) >>>> - goto err_disable_msi; >>>> + goto err_disable_device; >>>> return ret; >>>> -err_disable_msi: >>>> - pci_disable_msi(pdev); >>>> err_disable_device: >>>> pci_disable_device(pdev); >>>> err_put_node: >>>> @@ -200,7 +226,6 @@ static void loongson_dwmac_remove(struct pci_dev *pdev) >>>> break; >>>> } >>>> - pci_disable_msi(pdev); >>>> pci_disable_device(pdev); >>>> } >>>> -- >>>> 2.31.4 >>>>
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c index 979c9b6dab3f..e7ce027cc14e 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c @@ -11,8 +11,46 @@ struct stmmac_pci_info { int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat); + int (*config)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat, + struct stmmac_resources *res, struct device_node *np); }; +static int loongson_dwmac_config_legacy(struct pci_dev *pdev, + struct plat_stmmacenet_data *plat, + struct stmmac_resources *res, + struct device_node *np) +{ + if (np) { + res->irq = of_irq_get_byname(np, "macirq"); + if (res->irq < 0) { + dev_err(&pdev->dev, "IRQ macirq not found\n"); + return -ENODEV; + } + + 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"); + return -ENODEV; + } + } else { + res->irq = pdev->irq; + res->wol_irq = res->irq; + } + + plat->flags &= ~STMMAC_FLAG_MULTI_MSI_EN; + dev_info(&pdev->dev, "%s: Single IRQ enablement successful\n", + __func__); + + return 0; +} + static void loongson_default_data(struct pci_dev *pdev, struct plat_stmmacenet_data *plat) { @@ -66,8 +104,21 @@ static int loongson_gmac_data(struct pci_dev *pdev, return 0; } +static int loongson_gmac_config(struct pci_dev *pdev, + struct plat_stmmacenet_data *plat, + struct stmmac_resources *res, + struct device_node *np) +{ + int ret; + + ret = loongson_dwmac_config_legacy(pdev, plat, res, np); + + return ret; +} + static struct stmmac_pci_info loongson_gmac_pci_info = { .setup = loongson_gmac_data, + .config = loongson_gmac_config, }; static int loongson_dwmac_probe(struct pci_dev *pdev, @@ -139,44 +190,19 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, plat->phy_interface = phy_mode; } - pci_enable_msi(pdev); memset(&res, 0, sizeof(res)); res.addr = pcim_iomap_table(pdev)[0]; - 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 = info->config(pdev, plat, &res, np); + if (ret) + goto err_disable_device; ret = stmmac_dvr_probe(&pdev->dev, plat, &res); if (ret) - goto err_disable_msi; + goto err_disable_device; return ret; -err_disable_msi: - pci_disable_msi(pdev); err_disable_device: pci_disable_device(pdev); err_put_node: @@ -200,7 +226,6 @@ static void loongson_dwmac_remove(struct pci_dev *pdev) break; } - pci_disable_msi(pdev); pci_disable_device(pdev); }