* [PATCH] iio/adc/pac1934: fix channel disable configuration @ 2025-08-11 13:09 Aleksandar Gerasimovski 2025-08-12 8:36 ` Marius.Cristea 2025-08-16 13:43 ` Jonathan Cameron 0 siblings, 2 replies; 7+ messages in thread From: Aleksandar Gerasimovski @ 2025-08-11 13:09 UTC (permalink / raw) To: linux-iio, marius.cristea, rene.straub; +Cc: Aleksandar Gerasimovski 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 <rene.straub@belden.com> Signed-off-by: Aleksandar Gerasimovski <aleksandar.gerasimovski@belden.com> --- 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; -- 2.34.1 ********************************************************************** 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. ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] iio/adc/pac1934: fix channel disable configuration 2025-08-11 13:09 [PATCH] iio/adc/pac1934: fix channel disable configuration Aleksandar Gerasimovski @ 2025-08-12 8:36 ` Marius.Cristea 2025-08-16 13:43 ` Jonathan Cameron 1 sibling, 0 replies; 7+ messages in thread From: Marius.Cristea @ 2025-08-12 8:36 UTC (permalink / raw) To: aleksandar.gerasimovski, rene.straub, linux-iio Hi, Thank you for finding and fixing the bug. Reviewed-by: Marius Cristea <marius.cristea@microchip.com> On Mon, 2025-08-11 at 13:09 +0000, Aleksandar Gerasimovski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > 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 <rene.straub@belden.com> > Signed-off-by: Aleksandar Gerasimovski > <aleksandar.gerasimovski@belden.com> > --- > 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; > > -- > 2.34.1 > > > ********************************************************************* > * > 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio/adc/pac1934: fix channel disable configuration 2025-08-11 13:09 [PATCH] iio/adc/pac1934: fix channel disable configuration Aleksandar Gerasimovski 2025-08-12 8:36 ` Marius.Cristea @ 2025-08-16 13:43 ` Jonathan Cameron 2025-08-18 11:08 ` Aleksandar Gerasimovski 1 sibling, 1 reply; 7+ messages in thread From: Jonathan Cameron @ 2025-08-16 13:43 UTC (permalink / raw) To: Aleksandar Gerasimovski; +Cc: linux-iio, marius.cristea, rene.straub 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 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 <rene.straub@belden.com> > Signed-off-by: Aleksandar Gerasimovski <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; > ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] iio/adc/pac1934: fix channel disable configuration 2025-08-16 13:43 ` Jonathan Cameron @ 2025-08-18 11:08 ` Aleksandar Gerasimovski 2025-08-18 17:56 ` Jonathan Cameron 0 siblings, 1 reply; 7+ messages in thread From: Aleksandar Gerasimovski @ 2025-08-18 11:08 UTC (permalink / raw) To: Jonathan Cameron Cc: linux-iio@vger.kernel.org, marius.cristea@microchip.com, Rene Straub Hi Jonathan, and sorry for the previous non-plain text message: Fixes: 0fb528c8255b (iio: adc: adding support for PAC193x) 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> > 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio/adc/pac1934: fix channel disable configuration 2025-08-18 11:08 ` Aleksandar Gerasimovski @ 2025-08-18 17:56 ` Jonathan Cameron 2025-08-19 8:57 ` Aleksandar Gerasimovski 0 siblings, 1 reply; 7+ messages in thread From: Jonathan Cameron @ 2025-08-18 17:56 UTC (permalink / raw) To: Aleksandar Gerasimovski Cc: linux-iio@vger.kernel.org, marius.cristea@microchip.com, Rene Straub 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] iio/adc/pac1934: fix channel disable configuration 2025-08-18 17:56 ` Jonathan Cameron @ 2025-08-19 8:57 ` Aleksandar Gerasimovski 2025-08-25 9:45 ` Jonathan Cameron 0 siblings, 1 reply; 7+ messages in thread From: Aleksandar Gerasimovski @ 2025-08-19 8:57 UTC (permalink / raw) To: Jonathan Cameron Cc: linux-iio@vger.kernel.org, marius.cristea@microchip.com, Rene Straub Hi Jonathan, Both Rene and I was investigating the problem in first place, I come with the patch afterwards. So, the sign would be then: Suggested-by: Rene Straub <mailto:rene.straub@belden.com> Signed-off-by: Aleksandar Gerasimovski <aleksandar.gerasimovski@belden.com> Thanks for following up! From: Jonathan Cameron <jic23@kernel.org> Sent: Monday, August 18, 2025 7:56 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, 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) On Mon, 18 Aug 2025 11:08:33 +0000 Aleksandar Gerasimovski <mailto: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 <mailto:jic23@kernel.org> > Sent: Saturday, August 16, 2025 3:44 PM > To: Aleksandar Gerasimovski <mailto:Aleksandar.Gerasimovski@belden.com> > Cc: mailto:linux-iio@vger.kernel.org; mailto:marius.cristea@microchip.com; Rene Straub <mailto: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. ********************************************************************** 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio/adc/pac1934: fix channel disable configuration 2025-08-19 8:57 ` Aleksandar Gerasimovski @ 2025-08-25 9:45 ` Jonathan Cameron 0 siblings, 0 replies; 7+ messages in thread From: Jonathan Cameron @ 2025-08-25 9:45 UTC (permalink / raw) To: Aleksandar Gerasimovski Cc: linux-iio@vger.kernel.org, marius.cristea@microchip.com, Rene Straub On Tue, 19 Aug 2025 08:57:40 +0000 Aleksandar Gerasimovski <Aleksandar.Gerasimovski@belden.com> wrote: > Hi Jonathan, > > Both Rene and I was investigating the problem in first place, I come with the patch afterwards. > So, the sign would be then: > > Suggested-by: Rene Straub <mailto:rene.straub@belden.com> > Signed-off-by: Aleksandar Gerasimovski <aleksandar.gerasimovski@belden.com> > > Thanks for following up! > Applied to the fixes-togreg branch of iio.git and marked for stable. Thanks, Jonathan > From: Jonathan Cameron <jic23@kernel.org> > Sent: Monday, August 18, 2025 7:56 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, 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) > > On Mon, 18 Aug 2025 11:08:33 +0000 > Aleksandar Gerasimovski <mailto: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 <mailto:jic23@kernel.org> > > Sent: Saturday, August 16, 2025 3:44 PM > > To: Aleksandar Gerasimovski <mailto:Aleksandar.Gerasimovski@belden.com> > > Cc: mailto:linux-iio@vger.kernel.org; mailto:marius.cristea@microchip.com; Rene Straub <mailto: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. > > ********************************************************************** > 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. > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-08-25 9:45 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-08-11 13:09 [PATCH] iio/adc/pac1934: fix channel disable configuration Aleksandar Gerasimovski 2025-08-12 8:36 ` Marius.Cristea 2025-08-16 13:43 ` Jonathan Cameron 2025-08-18 11:08 ` Aleksandar Gerasimovski 2025-08-18 17:56 ` Jonathan Cameron 2025-08-19 8:57 ` Aleksandar Gerasimovski 2025-08-25 9:45 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox