Hi, On 11/29/22 12:59, Andy Shevchenko wrote: > On Tue, Nov 29, 2022 at 1:32 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: >> On 11/29/22 11:28, Andy Shevchenko wrote: >>> On Mon, Nov 28, 2022 at 11:44 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: >>>> >>>> The tps68470 has support for 2 indicator LED outputs called >>>> ileda and iledb, at support for these as GPIO pins 10 + 11. >>> >>> add ? >> >> This models ileda and iledb outputs *as* GPIO pins 10 + 11 >> on the linux gpiochip. >> >> But yes it also adds gpio pins 10 + 11 to the gpiochip, so >> either one works I guess :) > > I had to be a bit more precise. 'at support'?! Perhaps it should be > 'add support'? > > ... > >>>> +static void tps68470_gpio_get_reg_and_mask(bool get, unsigned int offset, >>>> + unsigned int *reg, int *mask) >>> >>> Hmm... Usual way is to put the get/set flag at the end of the list of >>> parameters. >> >> For functions returning values by reference I always follow the >> pattern of input parameters first, then output parameters. >> >>> Also why not naming it as 'dir' to avoid confusion with the _get in >>> the function name? >> >> Because dir is meaningless without an enum to to define what a dir >> of 0/false means. Where as get is clear without such an enum. >> get is set to true when this function is called from >> tps68470_gpio_get() and false when it is called from >> tps68470_gpio_set(). It does not get more straight forward then that. > > But it's about the buffer (in hw sense) to read value from. What about > naming it out_or_in? "out or in" still does not make clear what a value of true means does true mean out or does it mean in ? I'm sorry, but just like with your comments on patch 1/3 I really don't see the problem here. Unlike "dir" or "out_or_in" get is a perfectly fine parameter name which is actually clearly defined by the parameter-name itself. And the function + the callers are all in the same file, so anyone reading the code can easily deduce the meaning from the code. Regards, Hans
