On 20/12/2021 14:48, Geert Uytterhoeven wrote:

On Mon, Dec 20, 2021 at 1:29 PM Daniel Lezcano
[off-list ref] wrote:

quoted

On 18/12/2021 15:41, Lad Prabhakar wrote:

quoted

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@bp.renesas.com>
---
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@bp.renesas.com/

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.

quoted

                if (irq <= 0) {
                        ret = irq ? irq : -ENXIO;

irq == 0 cannot happen.

quoted

                        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@linux-m68k.org

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