MIME-Version: 1.0
References: <20220105180323.8563-1-prabhakar.mahadev-lad.rj@bp.renesas.com>
In-Reply-To: <20220105180323.8563-1-prabhakar.mahadev-lad.rj@bp.renesas.com>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Thu, 6 Jan 2022 16:23:28 +0200
Message-ID: <CAHp75Veud0JbA0Gury3pK942xnyRzwums3_Kt=0ENbrDw3yrUQ@mail.gmail.com>
Subject: Re: [PATCH v6] soc: ti: smartreflex: Use platform_get_irq_optional()
 to get the interrupt
To:     Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Cc:     Nishanth Menon <nm@ti.com>,
        Santosh Shilimkar <ssantosh@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Prabhakar <prabhakar.csengg@gmail.com>,
        Linux PM <linux-pm@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        linux-arm Mailing List <linux-arm-kernel@lists.infradead.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Jan 5, 2022 at 8:03 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> 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().
>
> While at it return 0 instead of returning ret in the probe success path.

LGTM, thanks, FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v5->v6:
> * Dropped printing function name in error message.
>
> v4->v5:
> * Fixed missing return while using dev_err_probe().
>
> v3->v4:
> * Used dev_err_probe() to print error message
> * Returning 0 in probe success path.
>
> v2->v3
> * Switch back to platform_get_irq_optional()
> * Only print error in case of error, and not when interrupt is missing.
>
> v1->v2
> * Updated commit message
> * Drop check for IRQ0
> * Switched to using platform_get_irq() so that the probe won't
>   fail silently as requested by Nishanth.
>
> v1:
> * https://www.spinics.net/lists/arm-kernel/msg942549.html
> ---
>  drivers/soc/ti/smartreflex.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/soc/ti/smartreflex.c b/drivers/soc/ti/smartreflex.c
> index b5b2fa538d5c..ad2bb72e640c 100644
> --- a/drivers/soc/ti/smartreflex.c
> +++ b/drivers/soc/ti/smartreflex.c
> @@ -819,7 +819,7 @@ static int omap_sr_probe(struct platform_device *pdev)
>  {
>         struct omap_sr *sr_info;
>         struct omap_sr_data *pdata = pdev->dev.platform_data;
> -       struct resource *mem, *irq;
> +       struct resource *mem;
>         struct dentry *nvalue_dir;
>         int i, ret = 0;
>
> @@ -844,7 +844,11 @@ static int omap_sr_probe(struct platform_device *pdev)
>         if (IS_ERR(sr_info->base))
>                 return PTR_ERR(sr_info->base);
>
> -       irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +       ret = platform_get_irq_optional(pdev, 0);
> +       if (ret < 0 && ret != -ENXIO)
> +               return dev_err_probe(&pdev->dev, ret, "failed to get IRQ resource\n");
> +       if (ret > 0)
> +               sr_info->irq = ret;
>
>         sr_info->fck = devm_clk_get(pdev->dev.parent, "fck");
>         if (IS_ERR(sr_info->fck))
> @@ -870,9 +874,6 @@ static int omap_sr_probe(struct platform_device *pdev)
>         sr_info->autocomp_active = false;
>         sr_info->ip_type = pdata->ip_type;
>
> -       if (irq)
> -               sr_info->irq = irq->start;
> -
>         sr_set_clk_length(sr_info);
>
>         list_add(&sr_info->node, &sr_list);
> @@ -926,7 +927,7 @@ static int omap_sr_probe(struct platform_device *pdev)
>
>         }
>
> -       return ret;
> +       return 0;
>
>  err_debugfs:
>         debugfs_remove_recursive(sr_info->dbg_dir);
> --
> 2.17.1
>


-- 
With Best Regards,
Andy Shevchenko