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

[Takashi Iwai]

On Sun, 11 Jun 2017 19:02:07 +0200,
Andy Shevchenko wrote:
> 
> On Sun, 2017-06-11 at 18:48 +0200, Takashi Iwai wrote:
> > 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.
> 
> In general I certainly agree with you. In this particular case
> uglification vs. clearness not in the right balance (to me at least).
> 
> > It results in more lines?  Yes.  It's uglier?  Depends.
> > So what?  It makes clearer and less error-prone.
> 
> Again, above case one has to file all members no matter what, since it's
> a mapping and explicit better than implicit there. Taking above into
> consideration the C99 style just adds a noise here for my POV.
> 
> > Hrm, must I do that?  No, it's still a kind of matter of taste.
> 
> So, let's agree not to touch this particular case(s)?

Yeah, that implies it, I don't mind too much details.


thanks,

Takashi