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

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

[Marek Behún]

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(-)

diff --git a/drivers/leds/leds-turris-omnia.c b/drivers/leds/leds-turris-omnia.c
index e1a4629479c5..f27241896970 100644
--- a/drivers/leds/leds-turris-omnia.c
+++ b/drivers/leds/leds-turris-omnia.c
@@ -60,7 +60,6 @@ struct omnia_leds {
 static int omnia_cmd_write_u8(const struct i2c_client *client, u8 cmd, u8 val)
 {
 	u8 buf[2] = { cmd, val };
-	int ret;
 
 	return i2c_master_send(client, buf, sizeof(buf));
 }
-- 
2.41.0

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

[Lee Jones]

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?

-- 
Lee Jones [李琼斯]

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

[Marek Behún]

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)?

Thanks.

Marek

[1]
https://lore.kernel.org/linux-gpio/ZQmUFPvIx91+ps6k@smile.fi.intel.com/
[2]
https://lore.kernel.org/linux-gpio/ZQnn+Gi0xVlsGCYA@smile.fi.intel.com/
[3]
https://lore.kernel.org/linux-gpio/ZQnn+Gi0xVlsGCYA@smile.fi.intel.com/

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

[Lee Jones]

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 [李琼斯]