Hello Greg, On 2/13/25 10:58, Greg Kroah-Hartman wrote: > On Thu, Feb 13, 2025 at 09:25:04AM +0100, Alexis Lothoré wrote: [...] >> - mctrl_gpio_disable_ms(up->gpios); >> + mctrl_gpio_disable_ms(up->gpios, false); > > This a bad api. > > When you read this line in the driver, do you know what "false" means > here? > > Please make two functions, mctrl_gpio_disable_ms_sync() and > mctrl_gpio_disable_ms_no_sync() which can internally just call > mctrl_gpio_disable_ms() with the boolean, but no driver should have to > call that at all. > > That way, when you read driver code, you KNOW what is happening and you > don't have to hunt around in a totally different C file to try to figure > it out and loose your concentration. Makes sense, I'll spin a v2 with a more explicit API. Thanks, Alexis > > thanks, > > greg k-h -- Alexis Lothoré, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
