Re: [PATCH] iio/adc/pac1934: fix channel disable configuration

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 18 Aug 2025 11:08:33 +0000
Aleksandar Gerasimovski <Aleksandar.Gerasimovski@belden.com> wrote:

> Hi Jonathan, and sorry for the previous non-plain text message:
> 
> Fixes: 0fb528c8255b (iio: adc: adding support for PAC193x)
Thanks.  I noticed one more thing when I was about to queue this up..

> 
> Regards,
> Aleksandar
> 
> From: Jonathan Cameron <jic23@kernel.org> 
> Sent: Saturday, August 16, 2025 3:44 PM
> To: Aleksandar Gerasimovski <Aleksandar.Gerasimovski@belden.com>
> Cc: linux-iio@vger.kernel.org; marius.cristea@microchip.com; Rene Straub <Rene.Straub@belden.com>
> Subject: Re: [PATCH] iio/adc/pac1934: fix channel disable configuration
> 
> On Mon, 11 Aug 2025 13: 09: 04 +0000 Aleksandar Gerasimovski <aleksandar. gerasimovski@ belden. com> wrote: > There are two problems with the chip configuration in this driver: > - First, is that writing 12 bytes (ARRAY_SIZE(regs)) would
> 
> On Mon, 11 Aug 2025 13:09:04 +0000
> Aleksandar Gerasimovski <mailto:aleksandar.gerasimovski@belden.com> wrote:
> 
> > There are two problems with the chip configuration in this driver:
> > - First, is that writing 12 bytes (ARRAY_SIZE(regs)) would anyhow
> >   lead to a config overflow due to HW auto increment implementation
> >   in the chip.
> > - Second, the i2c_smbus_write_block_data write ends up in writing
> >   unexpected value to the channel_dis register, this is because
> >   the smbus size that is 0x03 in this case gets written to the
> >   register. The PAC1931/2/3/4 data sheet does not really specify
> >   that block write is indeed supported.
> > 
> > This problem is probably not visible on PAC1934 version where all
> > channels are used as the chip is properly configured by luck,
> > but in our case whenusing PAC1931 this leads to nonfunctional device.
> > 
> > Signed-off-by: Rene Straub <mailto:rene.straub@belden.com>
This SoB chain doesn't comply with the normal requirements.
Given the author is Aleksandar (no From in the commit, so it's the email sender)
my guess is either

a) Rene wrote the patch and the author of the commit accidentally got changed)
b) Rene and Aleksandar both wrote the patch in which case it is missing
Co-developed-by: Rene Straub <mailto:rene.straub@belden.com>
here

I can fix it up if you confirm if it was a or b or something else

Thanks,

Jonathan

> > Signed-off-by: Aleksandar Gerasimovski <mailto:aleksandar.gerasimovski@belden.com>  
> 
> Hi, Thanks for the patch.
> 
> Needs a fixes tag so we know how far to back port it.
> You can just reply with one to this thread rather than sending a new
> version.
> 
> Thanks,
> 
> Jonathan
> 
> > ---
> >  drivers/iio/adc/pac1934.c | 20 ++++++++++++++++++--
> >  1 file changed, 18 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/pac1934.c b/drivers/iio/adc/pac1934.c
> > index 09fe88eb3fb0..2e442e46f679 100644
> > --- a/drivers/iio/adc/pac1934.c
> > +++ b/drivers/iio/adc/pac1934.c
> > @@ -88,6 +88,7 @@
> >  #define PAC1934_VPOWER_3_ADDR			0x19
> >  #define PAC1934_VPOWER_4_ADDR			0x1A
> >  #define PAC1934_REFRESH_V_REG_ADDR		0x1F
> > +#define PAC1934_SLOW_REG_ADDR			0x20
> >  #define PAC1934_CTRL_STAT_REGS_ADDR		0x1C
> >  #define PAC1934_PID_REG_ADDR			0xFD
> >  #define PAC1934_MID_REG_ADDR			0xFE
> > @@ -1265,8 +1266,23 @@ static int pac1934_chip_configure(struct pac1934_chip_info *info)
> >  	/* no SLOW triggered REFRESH, clear POR */
> >  	regs[PAC1934_SLOW_REG_OFF] = 0;
> >  
> > -	ret =  i2c_smbus_write_block_data(client, PAC1934_CTRL_STAT_REGS_ADDR,
> > -					  ARRAY_SIZE(regs), (u8 *)regs);
> > +	/*
> > +	 * Write the three bytes sequentially, as the device does not support
> > +	 * block write.
> > +	 */
> > +	ret = i2c_smbus_write_byte_data(client, PAC1934_CTRL_STAT_REGS_ADDR,
> > +					regs[PAC1934_CHANNEL_DIS_REG_OFF]);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = i2c_smbus_write_byte_data(client,
> > +					PAC1934_CTRL_STAT_REGS_ADDR + PAC1934_NEG_PWR_REG_OFF,
> > +					regs[PAC1934_NEG_PWR_REG_OFF]);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = i2c_smbus_write_byte_data(client, PAC1934_SLOW_REG_ADDR,
> > +					regs[PAC1934_SLOW_REG_OFF]);
> >  	if (ret)
> >  		return ret;
> >    
> 
> **********************************************************************
> DISCLAIMER:
> Privileged and/or Confidential information may be contained in this message. If you are not the addressee of this message, you may not copy, use or deliver this message to anyone. In such event, you should destroy the message and kindly notify the sender by reply e-mail. It is understood that opinions or conclusions that do not relate to the official business of the company are neither given nor endorsed by the company. Thank You.