[alsa-devel] [PATCH v2] ASoC: Intel: byt-max98090: Add GPIO ACPI mapping table

[Takashi Iwai]

On Sun, 11 Jun 2017 16:01:13 +0200,
Andy Shevchenko wrote:
> 
> On Sat, 2017-06-10 at 21:41 +0200, Takashi Iwai wrote:
> > On Sat, 10 Jun 2017 18:37:41 +0200,
> > Andy Shevchenko wrote:
> > > 
> > > +static const struct acpi_gpio_params hp_gpios = { 0, 0, false };
> > > +static const struct acpi_gpio_params mic_gpios = { 1, 0, false };
> > > +
> > > +static const struct acpi_gpio_mapping acpi_byt_max98090_gpios[] = {
> > > +	{ "hp-gpios", &hp_gpios, 1 },
> > > +	{ "mic-gpios", &mic_gpios, 1 },
> > 
> > Better to use C99 style init?
> 
> Here it makes a little advantage over more uglified code.
> 
> >   I'm not always a fan of it, but this
> > seems deserving.  From the above, it's difficult to know which zero is
> > for what.
> 
> I'm not sure it makes sense. Those fields are filed according to ACPI
> specification and applies to the platform in question. Usually you don't
> need to look into them on regular basis, perhaps one time per each
> platform which uses such combination of SoC + ASoC + Codec which is
> quite unlikely to be more than once twice per year(?).
> 
> So, for over few dozens of drivers which are using GPIO ACPI mapping
> tables this is the first request like this I see.

It's a proof showing that people are just too lazy and copy the first
implementation pattern :)

> Thus, if you insist, I can do it, but my vote is "it doesn't worth of
> doing it at all".

Well, I'm not insisting to change that, but I still think that your
argument is rather weak from the code quality POV.

The C99 style initialization is definitely an improvement for
understanding the code.  The most important question is not about how
often changing the code, but about how better a reader can understand
and how it give less errors.

It results in more lines?  Yes.  It's uglier?  Depends.
So what?  It makes clearer and less error-prone.
Hrm, must I do that?  No, it's still a kind of matter of taste.


thanks,

Takashi