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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux