Re: [PATCH v3] iio: imu: inv_mpu6050: refactor aux read/write to use shared xfer logic

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 15 May 2025 17:47:11 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Sat, 10 May 2025 15:39:33 -0300
> Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:
> 
> > Hi Isabella, Rodrigo,
> > 
> > This patch looks okay to me. Though, I think Jean-Baptiste made a valid point in
> > v1 about inv_mpu_i2c_master_xfer() not implying any specific slave slot
> > previously. I'd suggest to add a slv parameter to inv_mpu_i2c_master_xfer(), but
> > that wouldn't make much difference since inv_mpu_aux_read/write() are still
> > hardcoded for SLV 0.
> > Besides that, I have one minor comment that's probably not a reason for a v4 if
> > this is deemed to be good.  
> 
> I'm looking for a tag from Jean-Baptiste on this.

Jean-Baptiste. If you have time to look at this version that would be great.

> 
> Jonathan
> 
> > 
> > Regards,
> > Marcelo
> > 
> > On 05/07, Isabella Caselli wrote:  
> > > Refactors inv_mpu_aux_read() and inv_mpu_aux_write() to extract the common
> > > I2C transfer sequence into inv_mpu_i2c_master_xfer(), which now handles
> > > starting and stopping the I2C master, waiting for completion, disabling
> > > SLV0, and checking for NACK errors.
> > > 
> > > This refactoring removes code duplication and improves maintainability.
> > > No functional changes are intended.
> > > 
> > > Signed-off-by: Isabella Caselli <bellacaselli20@gmail.com>
> > > Co-developed-by: Rodrigo Michelassi <rodrigo.michelassi@usp.br>
> > > Signed-off-by: Rodrigo Michelassi <rodrigo.michelassi@usp.br>
> > > ---    
> > ...  
> > >  static int inv_mpu_i2c_master_xfer(const struct inv_mpu6050_state *st)
> > >  {
> > > @@ -23,6 +25,7 @@ static int inv_mpu_i2c_master_xfer(const struct inv_mpu6050_state *st)
> > >  	uint8_t d;
> > >  	unsigned int user_ctrl;
> > >  	int ret;
> > > +	unsigned int status;    
> > I'd declare status above ret, or together with user_ctrl.
> > Just minor code style thing.  
> 
>