[PATCH v2] iio: adc: axp20x_adc: fix charging current reporting on AXP22x

[PATCH v2] iio: adc: axp20x_adc: fix charging current reporting on AXP22x

[Evgeny Boger]

Both the charging and discharging currents on AXP22x are stored as
12-bit integers, in accordance with the datasheet.
It's also confirmed by vendor BSP (axp20x_adc.c:axp22_icharge_to_mA).

The scale factor of 0.5 is never mentioned in datasheet, nor in the
vendor source code. I think it was here to compensate for
erroneous addition bit in register width.

Tested on custom A40i+AXP221s board with external ammeter as
a reference.

Signed-off-by: Evgeny Boger <boger@wirenboard.com>
---

Notes:
    Changes from v1:
      - return scale factor of 1 as IIO_VAL_INT, not IIO_VAL_INT_PLUS_MICRO
      - get rid of unused variable

 drivers/iio/adc/axp20x_adc.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
index 3e0c0233b431..df99f1365c39 100644
--- a/drivers/iio/adc/axp20x_adc.c
+++ b/drivers/iio/adc/axp20x_adc.c
@@ -251,19 +251,8 @@ static int axp22x_adc_raw(struct iio_dev *indio_dev,
 			  struct iio_chan_spec const *chan, int *val)
 {
 	struct axp20x_adc_iio *info = iio_priv(indio_dev);
-	int size;
 
-	/*
-	 * N.B.: Unlike the Chinese datasheets tell, the charging current is
-	 * stored on 12 bits, not 13 bits. Only discharging current is on 13
-	 * bits.
-	 */
-	if (chan->type == IIO_CURRENT && chan->channel == AXP22X_BATT_DISCHRG_I)
-		size = 13;
-	else
-		size = 12;
-
-	*val = axp20x_read_variable_width(info->regmap, chan->address, size);
+	*val = axp20x_read_variable_width(info->regmap, chan->address, 12);
 	if (*val < 0)
 		return *val;
 
@@ -386,9 +375,8 @@ static int axp22x_adc_scale(struct iio_chan_spec const *chan, int *val,
 		return IIO_VAL_INT_PLUS_MICRO;
 
 	case IIO_CURRENT:
-		*val = 0;
-		*val2 = 500000;
-		return IIO_VAL_INT_PLUS_MICRO;
+		*val = 1;
+		return IIO_VAL_INT;
 
 	case IIO_TEMP:
 		*val = 100;
-- 
2.25.1

Re: [PATCH v2] iio: adc: axp20x_adc: fix charging current reporting on AXP22x

[Jonathan Cameron]

On Wed, 17 Nov 2021 00:37:46 +0300
Evgeny Boger <boger@wirenboard.com> wrote:

> Both the charging and discharging currents on AXP22x are stored as
> 12-bit integers, in accordance with the datasheet.
> It's also confirmed by vendor BSP (axp20x_adc.c:axp22_icharge_to_mA).
> 
> The scale factor of 0.5 is never mentioned in datasheet, nor in the
> vendor source code. I think it was here to compensate for
> erroneous addition bit in register width.
> 
> Tested on custom A40i+AXP221s board with external ammeter as
> a reference.
> 
> Signed-off-by: Evgeny Boger <boger@wirenboard.com>

I know Quentin has moved on from Bootlin, so looking for input from Chen-Yu Tsai
for these as I have no idea :)

I have pinged Quentin as well on off chance he still wants to take a look.

Jonathan

> ---
> 
> Notes:
>     Changes from v1:
>       - return scale factor of 1 as IIO_VAL_INT, not IIO_VAL_INT_PLUS_MICRO
>       - get rid of unused variable
> 
>  drivers/iio/adc/axp20x_adc.c | 18 +++---------------
>  1 file changed, 3 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
> index 3e0c0233b431..df99f1365c39 100644
> --- a/drivers/iio/adc/axp20x_adc.c
> +++ b/drivers/iio/adc/axp20x_adc.c
> @@ -251,19 +251,8 @@ static int axp22x_adc_raw(struct iio_dev *indio_dev,
>  			  struct iio_chan_spec const *chan, int *val)
>  {
>  	struct axp20x_adc_iio *info = iio_priv(indio_dev);
> -	int size;
>  
> -	/*
> -	 * N.B.: Unlike the Chinese datasheets tell, the charging current is
> -	 * stored on 12 bits, not 13 bits. Only discharging current is on 13
> -	 * bits.
> -	 */
> -	if (chan->type == IIO_CURRENT && chan->channel == AXP22X_BATT_DISCHRG_I)
> -		size = 13;
> -	else
> -		size = 12;
> -
> -	*val = axp20x_read_variable_width(info->regmap, chan->address, size);
> +	*val = axp20x_read_variable_width(info->regmap, chan->address, 12);
>  	if (*val < 0)
>  		return *val;
>  
> @@ -386,9 +375,8 @@ static int axp22x_adc_scale(struct iio_chan_spec const *chan, int *val,
>  		return IIO_VAL_INT_PLUS_MICRO;
>  
>  	case IIO_CURRENT:
> -		*val = 0;
> -		*val2 = 500000;
> -		return IIO_VAL_INT_PLUS_MICRO;
> +		*val = 1;
> +		return IIO_VAL_INT;
>  
>  	case IIO_TEMP:
>  		*val = 100;

Re: [PATCH v2] iio: adc: axp20x_adc: fix charging current reporting on AXP22x

[Chen-Yu Tsai]

Hi,

On Sun, Nov 21, 2021 at 1:44 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Wed, 17 Nov 2021 00:37:46 +0300
> Evgeny Boger <boger@wirenboard.com> wrote:
>
> > Both the charging and discharging currents on AXP22x are stored as
> > 12-bit integers, in accordance with the datasheet.
> > It's also confirmed by vendor BSP (axp20x_adc.c:axp22_icharge_to_mA).
> >
> > The scale factor of 0.5 is never mentioned in datasheet, nor in the
> > vendor source code. I think it was here to compensate for
> > erroneous addition bit in register width.
> >
> > Tested on custom A40i+AXP221s board with external ammeter as
> > a reference.
> >
> > Signed-off-by: Evgeny Boger <boger@wirenboard.com>
>
> I know Quentin has moved on from Bootlin, so looking for input from Chen-Yu Tsai
> for these as I have no idea :)

The datasheet only lists the registers for reading the value, but nothing
is said about how to interpret the data read. And the datasheet lists 13
bits split between two registers.

Evgeny mentioned that the original code is wrong, and the BSP code is
likely right, and has test results that match. That's good enough for
me. Unfortunately I don't have any way to double check it right now. So

Acked-by: Chen-Yu Tsai <wens@csie.org>

Re: [PATCH v2] iio: adc: axp20x_adc: fix charging current reporting on AXP22x

[Jonathan Cameron]

On Sun, 21 Nov 2021 01:58:08 +0800
Chen-Yu Tsai <wens@csie.org> wrote:

> Hi,
> 
> On Sun, Nov 21, 2021 at 1:44 AM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Wed, 17 Nov 2021 00:37:46 +0300
> > Evgeny Boger <boger@wirenboard.com> wrote:
> >  
> > > Both the charging and discharging currents on AXP22x are stored as
> > > 12-bit integers, in accordance with the datasheet.
> > > It's also confirmed by vendor BSP (axp20x_adc.c:axp22_icharge_to_mA).
> > >
> > > The scale factor of 0.5 is never mentioned in datasheet, nor in the
> > > vendor source code. I think it was here to compensate for
> > > erroneous addition bit in register width.
> > >
> > > Tested on custom A40i+AXP221s board with external ammeter as
> > > a reference.
> > >
> > > Signed-off-by: Evgeny Boger <boger@wirenboard.com>  
> >
> > I know Quentin has moved on from Bootlin, so looking for input from Chen-Yu Tsai
> > for these as I have no idea :)  
> 
> The datasheet only lists the registers for reading the value, but nothing
> is said about how to interpret the data read. And the datasheet lists 13
> bits split between two registers.
> 
> Evgeny mentioned that the original code is wrong, and the BSP code is
> likely right, and has test results that match. That's good enough for
> me. Unfortunately I don't have any way to double check it right now. So
> 
> Acked-by: Chen-Yu Tsai <wens@csie.org>

Applied to the fixes-togreg branch of iio.git with a fixes tag for the original
driver introduction (as it seems this dates back that far) and marked for stable.

Thanks,

Jonathan