On 20/12/2021 14:48, Geert Uytterhoeven wrote: > On Mon, Dec 20, 2021 at 1:29 PM Daniel Lezcano > <daniel.lezcano@xxxxxxxxxx> wrote: >> On 18/12/2021 15:41, Lad Prabhakar wrote: >>> platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static >>> allocation of IRQ resources in DT core code, this causes an issue >>> when using hierarchical interrupt domains using "interrupts" property >>> in the node as this bypasses the hierarchical setup and messes up the >>> irq chaining. >>> >>> In preparation for removal of static setup of IRQ resource from DT core >>> code use platform_get_irq_optional(). >>> >>> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> >>> --- >>> Hi, >>> >>> Dropping usage of platform_get_resource() was agreed based on >>> the discussion [0]. >>> >>> [0] https://patchwork.kernel.org/project/linux-renesas-soc/ >>> patch/20211209001056.29774-1-prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx/ >>> >>> Cheers, >>> Prabhakar >>> --- >>> drivers/thermal/rcar_thermal.c | 15 +++++++++++---- >>> 1 file changed, 11 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c >>> index b49f04daaf47..e4c7bc1bf7ef 100644 >>> --- a/drivers/thermal/rcar_thermal.c >>> +++ b/drivers/thermal/rcar_thermal.c >>> @@ -445,7 +445,7 @@ static int rcar_thermal_probe(struct platform_device *pdev) >>> struct rcar_thermal_common *common; >>> struct rcar_thermal_priv *priv; >>> struct device *dev = &pdev->dev; >>> - struct resource *res, *irq; >>> + struct resource *res; >>> const struct rcar_thermal_chip *chip = of_device_get_match_data(dev); >>> int mres = 0; >>> int i; >>> @@ -467,9 +467,16 @@ static int rcar_thermal_probe(struct platform_device *pdev) >>> pm_runtime_get_sync(dev); >>> >>> for (i = 0; i < chip->nirqs; i++) { >>> - irq = platform_get_resource(pdev, IORESOURCE_IRQ, i); >>> - if (!irq) >>> + int irq; >>> + >>> + irq = platform_get_irq_optional(pdev, i); >>> + if (irq <= 0 && irq != -ENXIO) { >>> + ret = irq ? irq : -ENXIO; >>> + goto error_unregister; >>> + } >>> + if (irq == -ENXIO) >>> continue; >> >> Why not invert the conditions? >> >> if (irq == -ENXIO) >> continue; > > And this can be break. > >> >> if (irq <= 0) { >> ret = irq ? irq : -ENXIO; > > irq == 0 cannot happen. > >> goto out_unregister; >> } Sorry, I don't get the two comments. May be I missed something but it seems for me the results are the same with the inverted conditions or not. if (irq <= 0 && irq != -ENXIO) goto out; if (irq == -ENXIO) continue; Can be changed to: if (irq != -ENXIO) if (irq <= 0) goto out; if (irq == -ENXIO) continue; Can be changed to: if (irq == -ENXIO) continue; if (irq != -ENXIO) if (irq <= 0) goto out; The second condition is always true because the first condition is the opposite of the second condition, if the second condition block is reached, that means irq != -ENXIO, so we can remove the second condition and that results into: if (irq == -ENXIO) continue; if (irq <= 0) goto out; Did I miss your point ? > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds > -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
