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

[Andy Shevchenko]

On Tue, Sep 09, 2025 at 04:45:31PM +0300, Andy Shevchenko wrote:
> 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.

TBH, I think those 6 all made the same mistake, i.e. thinking of the compound
literal as a cast. Which is not!

> 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