Re: [PATCH v3 07/11] platform/x86: int3472/discrete: Refactor GPIO to sensor mapping

[Andy Shevchenko <andy@kernel.org>]

On Fri, Dec 16, 2022 at 05:15:14PM +0100, Hans de Goede wrote:
> On 12/16/22 14:57, Andy Shevchenko wrote:
> > On Fri, Dec 16, 2022 at 12:30:09PM +0100, Hans de Goede wrote:

...

> >> +static void int3472_get_func_and_polarity(u8 type, const char **func, u32 *polarity)
> > 
> > I find a bit strange not making this a function that returns func:
> > 
> > static const char *int3472_get_func_and_polarity(u8 type, u32 *polarity)
> 
> Why make it return func and not polarity ?

Because of name "func" and "polarity". And as you read a prototype from left to
right it exactly follows that.

> Since there are 2 values to return it makes sense to be
> consistent and return both by reference.

But this is unneeded complication, no?

> Also this sort of comments really get into the territory
> of bikeshedding which is not helpful IMHO.

I find helpful to have a prototype shorter and the switch-case shorter due to
return vs break. Of course you can think it's unhelpful, but I have another
opinion. All by all you are the maintainer there, your call.

-- 
With Best Regards,
Andy Shevchenko