Re: [PATCH] leds: turris-omnia: Fix unused variable

[Lee Jones <lee@kernel.org>]

On Fri, 22 Sep 2023, Marek Behún wrote:

> On Fri, 22 Sep 2023 07:59:19 +0100
> Lee Jones <lee@kernel.org> wrote:
> 
> > On Thu, 21 Sep 2023, Marek Behún wrote:
> > 
> > > The variable ret is not used in this function.
> > > 
> > > Fixes: 28350bc0ac77 ("leds: turris-omnia: Do not use SMBUS calls")
> > > Closes: https://lore.kernel.org/linux-leds/202309212215.Yl5VQaSm-lkp@intel.com/T/#u
> > > Signed-off-by: Marek Behún <kabel@kernel.org>
> > > ---
> > >  drivers/leds/leds-turris-omnia.c | 1 -
> > >  1 file changed, 1 deletion(-)  
> > 
> > I already fixed and squashed this.
> > 
> > How was this missed when you tested the set?
> 
> I am not sure, but it is possible that I've refactored that function
> from my original (return 0 on success) to your proposed (return number
> of written bytes on success) and did not notice the warning. I am sure
> I tested that the result works. Maybe I switched to another terminal
> where I was testing it too fast and did not notice the warning.
> 
> Sorry about this.
> 
> Anyway, I have a question. Several days ago I also sent for review
> a new driver for other feautres the Turris Omnia MCU provides (GPIO,
> watchdog, wakeup+poweroff).
> There, I also refactored the _write and _read functions as you
> suggested (to return the number of bytes written/read).
> On review, Andy Shevchenko requested [1] to refactor it to my original
> (return 0 on success). I mentioned to him [2] your request, to which he
> replied [3]:
>   This is strange. For example, regmap APIs never returns amount of
>   data written or read. I think it's solely depends on the API. It might
>   be useful for i²c APIs, in case you can do something about it. but if
>   you have wrappers on top of that already (meaning not using directly
>   the i2c_*() calls, I dunno the positive return is anyhow useful.
> Since I agree with him, taking this into account, would you accept a
> patch that returns those function to how I originally wrote them
> (return 0 on success)?

As I said before, I'm not going to force you into anything.

Fire away.

-- 
Lee Jones [李琼斯]