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? -- With Best Regards, Andy Shevchenko
