Re: [PATCH V3] leds: pca955x: Add ACPI support for pca955x

[Jacek Anaszewski]

Hi Peter,

On 11/30/2016 10:36 AM, Peter Rosin wrote:
> On 2016-11-30 10:10, Phong Vo wrote:
> > +-----Original Message-----
> > +From: Jacek Anaszewski [mailto:j.anaszewski@xxxxxxxxxxx]
> > +
> > +Hi Phong,
> > +
> > +On 11/30/2016 09:23 AM, Phong Vo wrote:
> > +> +-----Original Message-----
> > +> +From: Jacek Anaszewski [mailto:j.anaszewski@xxxxxxxxxxx]
> > +> +
> > +> +On 11/30/2016 09:06 AM, Tin Huynh wrote:
> > +> +> On Wed, Nov 30, 2016 at 3:01 PM, Jacek Anaszewski <j.anaszewski@xxxxxxxxxxx> wrote:
> > +> +>> On 11/30/2016 08:51 AM, Jacek Anaszewski wrote:
> > +> +>>> On 11/30/2016 04:08 AM, Tin Huynh wrote:
> > +> +>>>> +static const struct acpi_device_id pca955x_acpi_ids[] = {
> > +> +>>>> +    { .id = "PCA9550", .driver_data = pca9550 },
> > +> +>>>> +    { .id = "PCA9551", .driver_data = pca9551 },
> > +> +>>>> +    { .id = "PCA9552", .driver_data = pca9552 },
> > +> +>>>> +    { .id = "PCA9553", .driver_data = pca9553 },
> > +> +>>>> +    { }
> > +> +>>
> > +> +>>
> > +> +>> OK, I see that you brought back explicit properties in the
> > +> +>> structure initializer. Is there some vital reason for that?
> > +>
> > +> It's not vital, but to make it consistent with what was done for
> > +> pca963x,
> > +
> > +For pca963x I applied the version without explicit properties.
> > +Note that this is consistent with pca963x_id array above the added
> > +pca963x_acpi_ids. For pca955x the situation is the same.
> > +
> > +> and also per suggestion by Mika on reviewing  a different driver
> > +> mux:954x in another thread.
> > +
> > +Could you give a reference to that thread? In the review of V1 of this
> > +patch Mika mentioned "{ "PCA9553", pca9553 }" scheme.
> > +
> >
> > Actually it was Peter Rosin (not Mika) on linux-i2c and the reference to
> > that is follows
> >
> > https://lkml.org/lkml/2016/11/18/732
> >
> > I am including Robin here.
>
> I tried to say that I *personally* would have added the explicit
> field specifiers but that it didn't seem like the norm and that both
> of the approaches would therefore be perfectly ok by me (as there are
> other examples of acpi id table initializers with field specifiers).
>
> To sum up, I don't really care. But my question lingers, is there some
> compelling reason to not have the explicit field specifiers?

It certainly downgrades code readability, but in case there is
similar surrounding code there are two options to keep the things
consistent - either stick to the current style or change it.
IMHO the latter would generate only unnecessary noise in this
particular case.

--
Best regards,
Jacek Anaszewski