Hi Andy,
Thank you for the review.
On Sat, Dec 25, 2021 at 11:41 AM Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
>
>
>
> On Friday, December 24, 2021, Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> 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().
>>
>> This patch break's the for loop if there are no more interrupts instead
>> of continuing as done in previous code.
>>
>> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
>> ---
>> v1->v2
>> * Simplified checking error code
>> * Break loop earlier if no interrupts are seen
>> ---
>> drivers/thermal/rcar_thermal.c | 17 ++++++++++++-----
>> 1 file changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
>> index b49f04daaf47..ff9c55999ddf 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)
>> - continue;
>> + int irq;
>> +
>> + irq = platform_get_irq_optional(pdev, i);
>> + if (irq == -ENXIO)
>> + break;
>> + if (irq < 0) {
>> + ret = irq;
>> + goto error_unregister;
>> + }
>
>
> In all your patches which introduce optional IRQ please change the logic to the opposite:
>
> if (irq > 0)
> ...we got one...
> if (irq == -EPROBE_DEFER)
> ...return it, if it is ever possible...
>
> With this you adding me a work.
>
The maintainer has requested me to do so [0].
[0] https://patchwork.kernel.org/project/linux-pm/patch/20211218144136.6663-1-prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx/#24669399
Cheers,
Prabhakar
>
>>
>> +
>> if (!common->base) {
>> /*
>> * platform has IRQ support.
>> @@ -487,7 +494,7 @@ static int rcar_thermal_probe(struct platform_device *pdev)
>> idle = 0; /* polling delay is not needed */
>> }
>>
>> - ret = devm_request_irq(dev, irq->start, rcar_thermal_irq,
>> + ret = devm_request_irq(dev, irq, rcar_thermal_irq,
>> IRQF_SHARED, dev_name(dev), common);
>> if (ret) {
>> dev_err(dev, "irq request failed\n ");
>> --
>> 2.17.1
>>
>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
