| Message ID | 20191008135843.30640-1-enric.balletbo@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iommu/rockchip: Don't loop until failure to count interrupts | expand |
Hi Enric, Am Dienstag, 8. Oktober 2019, 15:58:43 CEST schrieb Enric Balletbo i Serra: > As platform_get_irq() now prints an error when the interrupt does not > exist, counting interrupts by looping until failure causes the printing > of scary messages like: > > rk_iommu ff924000.iommu: IRQ index 1 not found > rk_iommu ff914000.iommu: IRQ index 1 not found > rk_iommu ff903f00.iommu: IRQ index 1 not found > rk_iommu ff8f3f00.iommu: IRQ index 1 not found > rk_iommu ff650800.iommu: IRQ index 1 not found > > Fix this by using the platform_irq_count() helper to avoid touching > non-existent interrupts. It looks like we did the same fix ... see my patch from september 25: https://patchwork.kernel.org/patch/11161221/ > Fixes: 7723f4c5ecdb8d83 ("driver core: platform: Add an error message to platform_get_irq*()") > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> > --- > > drivers/iommu/rockchip-iommu.c | 35 +++++++++++++++++++++++----------- > 1 file changed, 24 insertions(+), 11 deletions(-) > > diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c > index 26290f310f90..8c6318bd1b37 100644 > --- a/drivers/iommu/rockchip-iommu.c > +++ b/drivers/iommu/rockchip-iommu.c > @@ -1136,7 +1136,7 @@ static int rk_iommu_probe(struct platform_device *pdev) > struct rk_iommu *iommu; > struct resource *res; > int num_res = pdev->num_resources; > - int err, i, irq; > + int err, i, irq, num_irqs; > > iommu = devm_kzalloc(dev, sizeof(*iommu), GFP_KERNEL); > if (!iommu) > @@ -1219,20 +1219,28 @@ static int rk_iommu_probe(struct platform_device *pdev) > > pm_runtime_enable(dev); > > - i = 0; > - while ((irq = platform_get_irq(pdev, i++)) != -ENXIO) { > - if (irq < 0) > - return irq; > + num_irqs = platform_irq_count(pdev); > + if (num_irqs < 0) { > + err = num_irqs; > + goto err_disable_pm_runtime; > + } By moving the basic irq count above the pm_runtime_enable you save some lines and the whole goto logic ... see my patch for reference :-D Heiko > + > + for (i = 0; i < num_irqs; i++) { > + irq = platform_get_irq(pdev, i); > + if (irq < 0) { > + err = irq; > + goto err_disable_pm_runtime; > + } > > err = devm_request_irq(iommu->dev, irq, rk_iommu_irq, > IRQF_SHARED, dev_name(dev), iommu); > - if (err) { > - pm_runtime_disable(dev); > - goto err_remove_sysfs; > - } > + if (err) > + goto err_disable_pm_runtime; > } > > return 0; > +err_disable_pm_runtime: > + pm_runtime_disable(dev); > err_remove_sysfs: > iommu_device_sysfs_remove(&iommu->iommu); > err_put_group: > @@ -1245,10 +1253,15 @@ static int rk_iommu_probe(struct platform_device *pdev) > static void rk_iommu_shutdown(struct platform_device *pdev) > { > struct rk_iommu *iommu = platform_get_drvdata(pdev); > - int i = 0, irq; > + int i, irq, num_irqs; > > - while ((irq = platform_get_irq(pdev, i++)) != -ENXIO) > + num_irqs = platform_irq_count(pdev); > + for (i = 0; i < num_irqs; i++) { > + irq = platform_get_irq(pdev, i); > + if (irq < 0) > + continue; > devm_free_irq(iommu->dev, irq, iommu); > + } > > pm_runtime_force_suspend(&pdev->dev); > } >
Hi Heiko, Missatge de Heiko Stübner <heiko@sntech.de> del dia dt., 8 d’oct. 2019 a les 19:58: > > Hi Enric, > > Am Dienstag, 8. Oktober 2019, 15:58:43 CEST schrieb Enric Balletbo i Serra: > > As platform_get_irq() now prints an error when the interrupt does not > > exist, counting interrupts by looping until failure causes the printing > > of scary messages like: > > > > rk_iommu ff924000.iommu: IRQ index 1 not found > > rk_iommu ff914000.iommu: IRQ index 1 not found > > rk_iommu ff903f00.iommu: IRQ index 1 not found > > rk_iommu ff8f3f00.iommu: IRQ index 1 not found > > rk_iommu ff650800.iommu: IRQ index 1 not found > > > > Fix this by using the platform_irq_count() helper to avoid touching > > non-existent interrupts. > > It looks like we did the same fix ... see my patch from september 25: > https://patchwork.kernel.org/patch/11161221/ > How in the hell I didn't catch this patch! Anyway, forget about this patch then, sorry for the noise. Thanks, Enric > > > Fixes: 7723f4c5ecdb8d83 ("driver core: platform: Add an error message to platform_get_irq*()") > > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> > > --- > > > > drivers/iommu/rockchip-iommu.c | 35 +++++++++++++++++++++++----------- > > 1 file changed, 24 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c > > index 26290f310f90..8c6318bd1b37 100644 > > --- a/drivers/iommu/rockchip-iommu.c > > +++ b/drivers/iommu/rockchip-iommu.c > > @@ -1136,7 +1136,7 @@ static int rk_iommu_probe(struct platform_device *pdev) > > struct rk_iommu *iommu; > > struct resource *res; > > int num_res = pdev->num_resources; > > - int err, i, irq; > > + int err, i, irq, num_irqs; > > > > iommu = devm_kzalloc(dev, sizeof(*iommu), GFP_KERNEL); > > if (!iommu) > > @@ -1219,20 +1219,28 @@ static int rk_iommu_probe(struct platform_device *pdev) > > > > pm_runtime_enable(dev); > > > > - i = 0; > > - while ((irq = platform_get_irq(pdev, i++)) != -ENXIO) { > > - if (irq < 0) > > - return irq; > > + num_irqs = platform_irq_count(pdev); > > + if (num_irqs < 0) { > > + err = num_irqs; > > + goto err_disable_pm_runtime; > > + } > > By moving the basic irq count above the pm_runtime_enable > you save some lines and the whole goto logic ... see my patch > for reference :-D > > Heiko > > > + > > + for (i = 0; i < num_irqs; i++) { > > + irq = platform_get_irq(pdev, i); > > + if (irq < 0) { > > + err = irq; > > + goto err_disable_pm_runtime; > > + } > > > > err = devm_request_irq(iommu->dev, irq, rk_iommu_irq, > > IRQF_SHARED, dev_name(dev), iommu); > > - if (err) { > > - pm_runtime_disable(dev); > > - goto err_remove_sysfs; > > - } > > + if (err) > > + goto err_disable_pm_runtime; > > } > > > > return 0; > > +err_disable_pm_runtime: > > + pm_runtime_disable(dev); > > err_remove_sysfs: > > iommu_device_sysfs_remove(&iommu->iommu); > > err_put_group: > > @@ -1245,10 +1253,15 @@ static int rk_iommu_probe(struct platform_device *pdev) > > static void rk_iommu_shutdown(struct platform_device *pdev) > > { > > struct rk_iommu *iommu = platform_get_drvdata(pdev); > > - int i = 0, irq; > > + int i, irq, num_irqs; > > > > - while ((irq = platform_get_irq(pdev, i++)) != -ENXIO) > > + num_irqs = platform_irq_count(pdev); > > + for (i = 0; i < num_irqs; i++) { > > + irq = platform_get_irq(pdev, i); > > + if (irq < 0) > > + continue; > > devm_free_irq(iommu->dev, irq, iommu); > > + } > > > > pm_runtime_force_suspend(&pdev->dev); > > } > > > > > > > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c index 26290f310f90..8c6318bd1b37 100644 --- a/drivers/iommu/rockchip-iommu.c +++ b/drivers/iommu/rockchip-iommu.c @@ -1136,7 +1136,7 @@ static int rk_iommu_probe(struct platform_device *pdev) struct rk_iommu *iommu; struct resource *res; int num_res = pdev->num_resources; - int err, i, irq; + int err, i, irq, num_irqs; iommu = devm_kzalloc(dev, sizeof(*iommu), GFP_KERNEL); if (!iommu) @@ -1219,20 +1219,28 @@ static int rk_iommu_probe(struct platform_device *pdev) pm_runtime_enable(dev); - i = 0; - while ((irq = platform_get_irq(pdev, i++)) != -ENXIO) { - if (irq < 0) - return irq; + num_irqs = platform_irq_count(pdev); + if (num_irqs < 0) { + err = num_irqs; + goto err_disable_pm_runtime; + } + + for (i = 0; i < num_irqs; i++) { + irq = platform_get_irq(pdev, i); + if (irq < 0) { + err = irq; + goto err_disable_pm_runtime; + } err = devm_request_irq(iommu->dev, irq, rk_iommu_irq, IRQF_SHARED, dev_name(dev), iommu); - if (err) { - pm_runtime_disable(dev); - goto err_remove_sysfs; - } + if (err) + goto err_disable_pm_runtime; } return 0; +err_disable_pm_runtime: + pm_runtime_disable(dev); err_remove_sysfs: iommu_device_sysfs_remove(&iommu->iommu); err_put_group: @@ -1245,10 +1253,15 @@ static int rk_iommu_probe(struct platform_device *pdev) static void rk_iommu_shutdown(struct platform_device *pdev) { struct rk_iommu *iommu = platform_get_drvdata(pdev); - int i = 0, irq; + int i, irq, num_irqs; - while ((irq = platform_get_irq(pdev, i++)) != -ENXIO) + num_irqs = platform_irq_count(pdev); + for (i = 0; i < num_irqs; i++) { + irq = platform_get_irq(pdev, i); + if (irq < 0) + continue; devm_free_irq(iommu->dev, irq, iommu); + } pm_runtime_force_suspend(&pdev->dev); }
As platform_get_irq() now prints an error when the interrupt does not exist, counting interrupts by looping until failure causes the printing of scary messages like: rk_iommu ff924000.iommu: IRQ index 1 not found rk_iommu ff914000.iommu: IRQ index 1 not found rk_iommu ff903f00.iommu: IRQ index 1 not found rk_iommu ff8f3f00.iommu: IRQ index 1 not found rk_iommu ff650800.iommu: IRQ index 1 not found Fix this by using the platform_irq_count() helper to avoid touching non-existent interrupts. Fixes: 7723f4c5ecdb8d83 ("driver core: platform: Add an error message to platform_get_irq*()") Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> --- drivers/iommu/rockchip-iommu.c | 35 +++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-)