[PATCH 13/15] gpio: sodaville: use new generic GPIO chip API

Andy Shevchenko andriy.shevchenko at intel.com
Tue Sep 9 06:45:31 PDT 2025


On Tue, Sep 09, 2025 at 08:24:23AM -0500, Bartosz Golaszewski wrote:
> On Tue, 9 Sep 2025 15:13:04 +0200, Andy Shevchenko
> <andriy.shevchenko at intel.com> said:
> > On Tue, Sep 09, 2025 at 01:35:04PM +0200, Bartosz Golaszewski wrote:
> >> On Tue, Sep 9, 2025 at 1:31 PM Andy Shevchenko
> >> <andriy.shevchenko at intel.com> wrote:
> >> > On Tue, Sep 09, 2025 at 11:15:40AM +0200, Bartosz Golaszewski wrote:

...

> >> > > +     config = (typeof(config)){
> >> >
> >> > This looks unusual. Why can't properly formed compound literal be used as in
> >> > many other places in the kernel?
> >>
> >> It is correct C
> >
> > If it compiles, it doesn't mean it's correct C, it might be non-standard.
> > Have you checked with the standard (note, I read that part in the past,
> > but I may forgot the details, so I don't know the answer to this)?
> 
> It's a GNU extension alright

clang, I suppose, also okay with this?

> but it's supported in the kernel as it evaluates
> to a simple cast.

There is no cast. And that's make a big difference to what the code tries to do.

> >> and checkpatch doesn't raise any warnings.
> >
> > checkpatch is far from being useful in the questions like this.
> > It false positively complains for for_each*() macros all over
> > the kernel, for example.
> >
> >> It's the
> >> same kind of argument as between kmalloc(sizeof(struct foo)) vs
> >> kmalloc(sizeof(f)).
> >
> > Maybe, but it introduces a new style while all other cases use the other,
> > _established_ style. So we have a precedent and the form the code is written
> > in is against the de facto usage of the compound literals.
> 
> It may not be *very* common but it's hardly new style:

I think your statement is incorrect see below why.

> $ git grep -P "\(typeof\(.*\)\) ?\{" | wc
>     108     529    7315

Not correct. The correct output will be closer to

$ git grep -l -P "\(typeof\(.*\)\) ?\{" | wc -l
15

And if you looked at the output carefully, you see the bug in the RE you used.

So, even closer will be this one:

$ git grep -l -P "=[[:space:]]+\(typeof\(.*\)\) ?\{" | wc -l
7

2 out of which are related to libeth, effectively makes this 6.

No, this is completely non-standard and unusual thing in the kernel.

> >> I guess it's personal taste but I like this version better.
> >
> > In kernel we also try to be consistent. This add inconsistency. Am I wrong?
> >
> >> > > +             .dev = &pdev->dev,
> >> > > +             .sz = 4,
> >> > > +             .dat = sd->gpio_pub_base + GPINR,
> >> > > +             .set = sd->gpio_pub_base + GPOUTR,
> >> > > +             .dirout = sd->gpio_pub_base + GPOER,
> >> > > +     };

-- 
With Best Regards,
Andy Shevchenko





More information about the linux-riscv mailing list