linux-kernel - Re: [PATCH v2 21/22] platform: Modify platform_get_irq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 22 Dec 2023 14:51:32 -0700
From: Mark Hasemeyer <markhas@...omium.org>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: LKML <linux-kernel@...r.kernel.org>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, 
	Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>, Tzung-Bi Shih <tzungbi@...nel.org>, 
	Raul Rangel <rrangel@...omium.org>, Konrad Dybcio <konrad.dybcio@...aro.org>, 
	Rob Herring <robh@...nel.org>, Sudeep Holla <sudeep.holla@....com>, David Gow <davidgow@...gle.com>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Mark Brown <broonie@...nel.org>, 
	"Rafael J. Wysocki" <rafael@...nel.org>, Takashi Iwai <tiwai@...e.de>, 
	Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
Subject: Re: [PATCH v2 21/22] platform: Modify platform_get_irq_optional() to
 use resource

> >   * For example::
> >   *
> > - *           int irq = platform_get_irq_optional(pdev, 0);
> > + *           int irq = platform_get_irq_resource_optional(pdev, 0, &res);
> >   *           if (irq < 0)
> >   *                   return irq;
> >   *
> >   * Return: non-zero IRQ number on success, negative error number on failure.
>
> Why do we need the irq to be returned via error code?

We don't really. It just matches the convention of
'platform_get_irq()' and 'of_irq_to_resource()'.

> >       int ret;
>
> Missing blank line, have you run checkpatch.pl?

Yes, I normally run checkpatch.pl. I may have missed the warning or it
didn't catch it. I'll add it.

>
> > +     if (IS_ERR_OR_NULL(r))
> > +             return -EINVAL;
>
> If we ever have an error pointer in r, I prefer to see
>
>         if (!r)
>                 return -EINVAL;
>         if (IS_ERR(r))
>                 return PTR_ERR(r);
>
> But Q is the same as earlier: when would we have the error pointer in @r?

I don't see when we would. I'll drop it.

> Can we save this and be consistent with above fwnode API return code check?
>
> > +                     ret = ret ?: r->start;
> >                       goto out;
> > +             }
> >       }

Yep!