Re: [PATCH v2 16/21] gpio: Unify style of acpi_device_id arrays

On Fri, Jul 17, 2026 at 10:31:36AM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> On Thu, Jul 16, 2026 at 08:41:15PM +0300, Andy Shevchenko wrote:
> > On Wed, Jul 15, 2026 at 06:55:21PM +0200, Uwe Kleine-König (The Capable Hub) wrote:

...

> > >  static const struct acpi_device_id dwapb_acpi_match[] = {
> > > -	{ .id = "HISI0181", .driver_data = GPIO_REG_OFFSET_V1 },
> > >  	{ .id = "APMC0D07", .driver_data = GPIO_REG_OFFSET_V1 },
> > >  	{ .id = "APMC0D81", .driver_data = GPIO_REG_OFFSET_V2 },
> > >  	{ .id = "FUJI200A", .driver_data = GPIO_REG_OFFSET_V1 },
> > > +	{ .id = "HISI0181", .driver_data = GPIO_REG_OFFSET_V1 },
> > >  	{ .id = "LECA0001", .driver_data = GPIO_REG_OFFSET_V1 },
> > >  	{ }
> > 
> > Why this churn? It simply should be folded to the previous patch.
> 
> I guess with "previous patch" you mean "[PATCH v2 12/21] gpio: Use named
> initializers for acpi_device_id array"?

> I didn't fold the reordering
> into that, because reordering isn't about using named initializers and
> mixing in other changes makes reviewing harder.

I have, obviously, the opposite opinion. This makes an additional unneeded
churn while reordering does naturally fit the other patch.

> And given that there is
> a "churn" patch anyhow (which is "needed" because not all arrays that
> use unusual styling have an earlier change) fixing the ordering in the
> "unify style" patch IMHO fits better.
> 
> But note that for removing trailing , and spacing in the lines that I
> touch in patch #12, I agree to do that there. So we're talking about
> grey and subjective areas. But note that reordering breaks the promise
> of patch #12 "This patch doesn't modify the compiled arrays", which was
> the relevant reason for my choice when considering where to implement
> your sort request.
> 
> Also note that folding cleanups into patches that touch the same code
> area also makes it harder to keep commit logs up-to-date, which you
> already noticed I didn't master even with only doing one thing per
> patch.

Sometimes being too pedantic is not a good thing. You can tell it towards my
review as well, but my point here is to bend a rule very slightly to avoid
unneeded churn. Having stuck with "one patch — one logical change" makes a lot
of noise in such a conversion.

-- 
With Best Regards,
Andy Shevchenko