linux-kernel - Re: [PATCH 06/13] media: davinci: vpif: Use 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] [day] [month] [year] [list]

Message-ID: <CA+V-a8s22x28R2Y_1rVADgSzv9+em-EhtvfzC4zZd_9sop6bVQ@mail.gmail.com>
Date:   Thu, 6 Jan 2022 16:46:08 +0000
From:   "Lad, Prabhakar" <prabhakar.csengg@...il.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Hans Verkuil <hverkuil-cisco@...all.nl>,
        Rob Herring <robh+dt@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux-Renesas <linux-renesas-soc@...r.kernel.org>
Subject: Re: [PATCH 06/13] media: davinci: vpif: Use platform_get_irq_optional()
 to get the interrupt

Hi Andy,

On Thu, Jan 6, 2022 at 4:28 PM Andy Shevchenko
<andy.shevchenko@...il.com> wrote:
>
> On Thu, Jan 6, 2022 at 6:11 PM Lad, Prabhakar
> <prabhakar.csengg@...il.com> wrote:
> > On Thu, Jan 6, 2022 at 4:02 PM Andy Shevchenko
> > <andy.shevchenko@...il.com> wrote:
> > > On Thu, Jan 6, 2022 at 5:27 PM Lad, Prabhakar
> > > <prabhakar.csengg@...il.com> wrote:
> > > > On Thu, Jan 6, 2022 at 2:15 PM Andy Shevchenko
> > > > <andy.shevchenko@...il.com> wrote:
> > > > > On Thu, Jan 6, 2022 at 3:43 PM Andy Shevchenko
> > > > > <andy.shevchenko@...il.com> wrote:
> > > > > > On Wed, Jan 5, 2022 at 7:41 PM Lad, Prabhakar
> > > > > > <prabhakar.csengg@...il.com> wrote:
> > > > > > > On Wed, Jan 5, 2022 at 9:43 AM Andy Shevchenko
> > >
> > > ...
> > >
> > > > > > >  #define DEFINE_RES_NAMED(_start, _size, _name, _flags)                 \
> > > > > > > -       {                                                               \
> > > > > > > +       (struct resource) {                                     \
> > > > > >
> > > > > > Yep, that's it.
> > > > > >
> > > > > > >                 .start = (_start),                                      \
> > > > > > >                 .end = (_start) + (_size) - 1,                          \
> > > > > > >                 .name = (_name),                                        \
> > > > > > >
> > > > > > > But there are some instances which need to be touched, for example
> > > > > > > vexpress-sysreg.c [1]. Are you OK with files to be changed?
> > > > > >
> > > > > > Nice! That's exactly my point and you can sell it to the community
> > > > > > because there are already users of it like this.
> > > > > >
> > > > > > Yes, I'm fine, but it seems it needs to be done treewide in one patch.
> > > > > > Btw, how many of those already in use?
> > > > >
> > > > > Actually you don't need to change that. It's an array of resources and
> > > > > everything should be kept as is there.
> > > > >
> > > > I do get below build failures, with the above literal change for
> > > > vexpress-sysreg.c.
> > > >
> > > > drivers/mfd/vexpress-sysreg.c: At top level:
> > > > drivers/mfd/vexpress-sysreg.c:64:37: error: initialiser element is not constant
> > > >    64 |   .resources = (struct resource []) {
> > > >       |                                     ^
> > > > drivers/mfd/vexpress-sysreg.c:64:37: note: (near initialisation for
> > > > ‘vexpress_sysreg_cells[0]’)
>
> > > Hmm... Interesting, so I suppose the fix is to drop (struct resource
> > > []) parts from the driver?
> > >
> > A bit more than that like something below:
>
> > - .resources = (struct resource []) {
> > - DEFINE_RES_MEM_NAMED(SYS_LED, 0x4, "dat"),
> > - },
> > + .resources = &DEFINE_RES_MEM_NAMED(SYS_LED, 0x4, "dat"),
>
> This is not an equivalent change.
> The warning is about const qualifier. Can it rather be  const struct
> resource [] ?
>
No, since it's just a single resource, just the below should be OK.

.resources = &DEFINE_RES_MEM_NAMED(SYS_LED, 0x4, "dat"),

[1] https://elixir.bootlin.com/linux/v5.16-rc8/source/include/linux/mfd/core.h#L108

On the other note I could use the below without changing the macro:

if (dev_of_node(...))
  res_irq = (struct resource) DEFINE_RES_IRQ_NAMED(...)
else
  res_irq =  (struct resource) DEFINE_RES_IRQ(...);
res_irq->flags |= irq_get_trigger_type(irq);

Cheers,
Prabhakar