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