Hi Dirk, On Wed, May 11, 2016 at 10:55 AM, Dirk Behme <dirk.behme@xxxxxxxxxxxx> wrote: > On 11.05.2016 10:41, Geert Uytterhoeven wrote: >> On Wed, May 11, 2016 at 10:39 AM, Dirk Behme <dirk.behme@xxxxxxxxxxxx> >> wrote: >>> On 11.05.2016 09:54, Geert Uytterhoeven wrote: >>>> On Wed, May 11, 2016 at 7:29 AM, Dirk Behme <dirk.behme@xxxxxxxxxxxx> >>>> wrote: >>>>> --- /dev/null >>>>> +++ b/drivers/misc/boot-mode-reg/core.c >>>>> +/** >>>>> + * boot_mode_reg_set() - record boot mode register value >>>>> + * @mode: implementation-dependent boot mode register value >>>>> + * >>>>> + * Records the boot mode register value which may subsequently >>>>> + * be retrieved using boot_mode_reg_get(). >>>>> + * >>>>> + * return: 0 on success >>>>> + */ >>>>> +int boot_mode_reg_set(u32 mode) >>>>> +{ >>>>> + int err = -EBUSY; >>>>> + >>>>> + mutex_lock(&boot_mode_mutex); >>>>> + if (!boot_mode_is_set) { >>>> >>>> You've dropped the check for calling this function a subsequent time >>>> with >>>> a different value of mode? >>> >>> Sometimes inverting 'complex' if statements is not that easy ;) >>> >>> You mean >>> >>> if (!boot_mode_is_set || boot_mode != mode) >> >> >> No, De Morgan says >> >> if (!boot_mode_is_set || boot_mode == mode) > > Hmm, sorry if I don't have enough coffee today ;) > > But why should we do > > if (... boot_mode == mode) > boot_mode = mode; > > ? > > Or in other words: > > We want to execute the if code if > > boot_mode_is_set is false > > or > > boot_mode is not equal mode, i.e. mode has changed > > ? We want to fail if the mode has changed. Doing the assignment a second time doesn't hurt. But as this seems to hurt readability, the better solution may be: if (!boot_mode_is_set) { boot_mode = mode; boot_mode_is_set = true; err = 0; } else if (boot_mode == mode) { err = 0; } else { err = -EBUSY; } Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds