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

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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)?

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel