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

[<Marius.Cristea@microchip.com>]

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.