On Fri, 22 Sep 2023, Marek Behún wrote:
> On Fri, 22 Sep 2023 07:59:19 +0100
> Lee Jones <lee@xxxxxxxxxx> 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@xxxxxxxxx/T/#u
> > > Signed-off-by: Marek Behún <kabel@xxxxxxxxxx>
> > > ---
> > > 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 [李琼斯]
