| Message ID | 1401166194-19085-1-git-send-email-wens@csie.org |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, May 26, 2014 at 11:49 PM, Chen-Yu Tsai <wens@csie.org> wrote: > The following patch moved device tree interrupt resolution into > platform_get_irq_byname: > > ad69674 of/irq: do irq resolution in platform_get_irq_byname() > > As a result, the function no longer only return -ENXIO on error. > This breaks DT based probing of stmmac, as seen in test runs of > linux-next next-20140526 cubie2-sunxi_defconfig: > > http://lists.linaro.org/pipermail/kernel-build-reports/2014-May/003659.html > > This patch makes the stmmac_platform probe function properly handle > error codes, such as returning for deferred probing, and other codes > returned by of_irq_get_by_name. ST u8500 is also broken as reported by Kevin Hilman. > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> > --- > .../net/ethernet/stmicro/stmmac/stmmac_platform.c | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c > index 46aef510..9562b18 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c > @@ -237,10 +237,12 @@ static int stmmac_pltfr_probe(struct platform_device *pdev) > > /* Get the MAC information */ > priv->dev->irq = platform_get_irq_byname(pdev, "macirq"); > - if (priv->dev->irq == -ENXIO) { > - pr_err("%s: ERROR: MAC IRQ configuration " > - "information not found\n", __func__); > - return -ENXIO; > + if (priv->dev->irq < 0) { > + if (priv->wol_irq != -EPROBE_DEFER) { s/wol_irq/dev->irq/ > + pr_err("%s: ERROR: MAC IRQ configuration " netdev_err > + "information not found\n", __func__); Don't split strings. > + } > + return priv->dev->irq; > } > > /* > @@ -252,10 +254,18 @@ static int stmmac_pltfr_probe(struct platform_device *pdev) > * so the driver will continue to use the mac irq (ndev->irq) > */ > priv->wol_irq = platform_get_irq_byname(pdev, "eth_wake_irq"); > - if (priv->wol_irq == -ENXIO) > + if (priv->wol_irq < 0) { > + if (priv->wol_irq == -EPROBE_DEFER) > + return -EPROBE_DEFER; > priv->wol_irq = priv->dev->irq; > + } > > priv->lpi_irq = platform_get_irq_byname(pdev, "eth_lpi"); > + if (priv->lpi_irq < 0) { You don't need this if. > + if (priv->wol_irq == -EPROBE_DEFER) s/wol_irq/lpi_irq/ > + return -EPROBE_DEFER; > + priv->lpi_irq = -ENXIO; > + } > > platform_set_drvdata(pdev, priv->dev); > > -- > 2.0.0.rc2 > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, May 28, 2014 at 8:58 AM, Rob Herring <robh@kernel.org> wrote: > On Mon, May 26, 2014 at 11:49 PM, Chen-Yu Tsai <wens@csie.org> wrote: >> The following patch moved device tree interrupt resolution into >> platform_get_irq_byname: >> >> ad69674 of/irq: do irq resolution in platform_get_irq_byname() >> >> As a result, the function no longer only return -ENXIO on error. >> This breaks DT based probing of stmmac, as seen in test runs of >> linux-next next-20140526 cubie2-sunxi_defconfig: >> >> http://lists.linaro.org/pipermail/kernel-build-reports/2014-May/003659.html >> >> This patch makes the stmmac_platform probe function properly handle >> error codes, such as returning for deferred probing, and other codes >> returned by of_irq_get_by_name. > > ST u8500 is also broken as reported by Kevin Hilman. Saw the mail today. >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >> --- >> .../net/ethernet/stmicro/stmmac/stmmac_platform.c | 20 +++++++++++++++----- >> 1 file changed, 15 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c >> index 46aef510..9562b18 100644 >> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c >> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c >> @@ -237,10 +237,12 @@ static int stmmac_pltfr_probe(struct platform_device *pdev) >> >> /* Get the MAC information */ >> priv->dev->irq = platform_get_irq_byname(pdev, "macirq"); >> - if (priv->dev->irq == -ENXIO) { >> - pr_err("%s: ERROR: MAC IRQ configuration " >> - "information not found\n", __func__); >> - return -ENXIO; >> + if (priv->dev->irq < 0) { >> + if (priv->wol_irq != -EPROBE_DEFER) { > > s/wol_irq/dev->irq/ Copy paste error :( >> + pr_err("%s: ERROR: MAC IRQ configuration " > > netdev_err > >> + "information not found\n", __func__); > > Don't split strings. It was in the original code. Will fix. >> + } >> + return priv->dev->irq; >> } >> >> /* >> @@ -252,10 +254,18 @@ static int stmmac_pltfr_probe(struct platform_device *pdev) >> * so the driver will continue to use the mac irq (ndev->irq) >> */ >> priv->wol_irq = platform_get_irq_byname(pdev, "eth_wake_irq"); >> - if (priv->wol_irq == -ENXIO) >> + if (priv->wol_irq < 0) { >> + if (priv->wol_irq == -EPROBE_DEFER) >> + return -EPROBE_DEFER; >> priv->wol_irq = priv->dev->irq; >> + } >> >> priv->lpi_irq = platform_get_irq_byname(pdev, "eth_lpi"); >> + if (priv->lpi_irq < 0) { > > You don't need this if. stmmac core driver checks lpi_irq != -ENXIO for valid interrupt. Might as well change that check to lpi_irq >= 0. (0 is a valid interrupt, correct?) Interestingly, stmmac_pci never sets lpi_irq, there might be a glitch there. >> + if (priv->wol_irq == -EPROBE_DEFER) > > s/wol_irq/lpi_irq/ ditto >> + return -EPROBE_DEFER; >> + priv->lpi_irq = -ENXIO; >> + } >> >> platform_set_drvdata(pdev, priv->dev); >> >> -- >> 2.0.0.rc2 >> Thanks ChenYu -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, May 28, 2014 at 1:44 AM, Chen-Yu Tsai <wens@csie.org> wrote: > On Wed, May 28, 2014 at 8:58 AM, Rob Herring <robh@kernel.org> wrote: >> On Mon, May 26, 2014 at 11:49 PM, Chen-Yu Tsai <wens@csie.org> wrote: >>> The following patch moved device tree interrupt resolution into >>> platform_get_irq_byname: >>> >>> ad69674 of/irq: do irq resolution in platform_get_irq_byname() >>> >>> As a result, the function no longer only return -ENXIO on error. >>> This breaks DT based probing of stmmac, as seen in test runs of >>> linux-next next-20140526 cubie2-sunxi_defconfig: [...] >>> priv->lpi_irq = platform_get_irq_byname(pdev, "eth_lpi"); >>> + if (priv->lpi_irq < 0) { >> >> You don't need this if. > > stmmac core driver checks lpi_irq != -ENXIO for valid interrupt. > Might as well change that check to lpi_irq >= 0. > (0 is a valid interrupt, correct?) No. NO_IRQ is defined to 0 (preferred) or -1 depending on the arch. You should be testing for "> 0" for valid irq rather than only ENXIO. My comment still stands. If lpi_irq is already <0, then there is no reason to set it to -ENXIO (again). If there are other error codes returned, changing the error code is not good practice. > Interestingly, stmmac_pci never sets lpi_irq, there might be a glitch > there. I believe they would have to be combined to a single interrrupt for PCI. Rob -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c index 46aef510..9562b18 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c @@ -237,10 +237,12 @@ static int stmmac_pltfr_probe(struct platform_device *pdev) /* Get the MAC information */ priv->dev->irq = platform_get_irq_byname(pdev, "macirq"); - if (priv->dev->irq == -ENXIO) { - pr_err("%s: ERROR: MAC IRQ configuration " - "information not found\n", __func__); - return -ENXIO; + if (priv->dev->irq < 0) { + if (priv->wol_irq != -EPROBE_DEFER) { + pr_err("%s: ERROR: MAC IRQ configuration " + "information not found\n", __func__); + } + return priv->dev->irq; } /* @@ -252,10 +254,18 @@ static int stmmac_pltfr_probe(struct platform_device *pdev) * so the driver will continue to use the mac irq (ndev->irq) */ priv->wol_irq = platform_get_irq_byname(pdev, "eth_wake_irq"); - if (priv->wol_irq == -ENXIO) + if (priv->wol_irq < 0) { + if (priv->wol_irq == -EPROBE_DEFER) + return -EPROBE_DEFER; priv->wol_irq = priv->dev->irq; + } priv->lpi_irq = platform_get_irq_byname(pdev, "eth_lpi"); + if (priv->lpi_irq < 0) { + if (priv->wol_irq == -EPROBE_DEFER) + return -EPROBE_DEFER; + priv->lpi_irq = -ENXIO; + } platform_set_drvdata(pdev, priv->dev);
The following patch moved device tree interrupt resolution into platform_get_irq_byname: ad69674 of/irq: do irq resolution in platform_get_irq_byname() As a result, the function no longer only return -ENXIO on error. This breaks DT based probing of stmmac, as seen in test runs of linux-next next-20140526 cubie2-sunxi_defconfig: http://lists.linaro.org/pipermail/kernel-build-reports/2014-May/003659.html This patch makes the stmmac_platform probe function properly handle error codes, such as returning for deferred probing, and other codes returned by of_irq_get_by_name. Signed-off-by: Chen-Yu Tsai <wens@csie.org> --- .../net/ethernet/stmicro/stmmac/stmmac_platform.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-)