Re: [PATCH v3 6/6] leds: turris-omnia: add support for enabling/disabling HW gamma correction

["Marek Behún" <kabel@kernel.org>]

On Fri, 18 Aug 2023 11:30:03 +0100
Lee Jones <lee@kernel.org> wrote:

> > -static int omnia_cmd_read(const struct i2c_client *client, u8 cmd)
> > +static int omnia_cmd_read_raw(struct i2c_adapter *adapter, u8 addr, u8 cmd,
> > +			      void *reply, size_t len)
> >  {
> >  	struct i2c_msg msgs[2];
> > -	u8 reply;
> >  	int ret;
> >  
> > -	msgs[0].addr = client->addr;
> > +	msgs[0].addr = addr;
> >  	msgs[0].flags = 0;
> >  	msgs[0].len = 1;
> >  	msgs[0].buf = &cmd;
> > -	msgs[1].addr = client->addr;
> > +	msgs[1].addr = addr;
> >  	msgs[1].flags = I2C_M_RD;
> > -	msgs[1].len = 1;
> > -	msgs[1].buf = &reply;
> > +	msgs[1].len = len;
> > +	msgs[1].buf = reply;
> >  
> > -	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> > +	ret = i2c_transfer(adapter, msgs, ARRAY_SIZE(msgs));
> >  	if (likely(ret == ARRAY_SIZE(msgs)))
> > -		return reply;
> > +		return 0;  
> 
> Why not return ret and use that throughout?
> 
> >  	else if (ret < 0)
> >  		return ret;
> >  	else
> >  		return -EIO;
> >  }  
> 

As I explained to your similar question in your reply to patch 2/6

  https://lore.kernel.org/linux-leds/20230821120136.130cc916@dellmb/T/#me5782c9896bc3d994cb36e8b5485d6368cd92da0

the reason I normalize to zero is because of the intended semantics of
these functions: return 0 on success, -errno on error.

If instead in omnia_cmd_read() and omnia_cmd_write() I simply return
the return value of the underlying functions, which are
  i2c_transfer()    for omnia_cmd_read()
  i2c_master_send() for omnia_cmd_write()
the semantics would be inconsistent, because:
  i2c_transfer() returns the number of successful I2C transfers,
  i2c_manster_send() returns the number of written bytes over I2C.

Nonetheless, if you insist on this change, I will do it. Reluctantly
and with a silent protest, but I will not argue further, since I want
this functionality in upstream more than to argue over nitpicks :-)

Marek