Re: [PATCH] iio: mtk-auxadc: update case IIO_CHAN_INFO_PROCESSED

From: Jonathan Cameron <jic23@kernel.org>
To: Hui Liu <hui.liu@mediatek.com>
Cc: <robh+dt@kernel.org>, <lars@metafoo.de>, <pmeerw@pmeerw.net>,
	<srv_heupstream@mediatek.com>, <zhiyong.tao@mediatek.com>,
	<chun-hung.wu@mediatek.com>, <yingjoe.chen@mediatek.com>,
	<seiya.wang@mediatek.com>, <ben.tseng@mediatek.com>,
	<matthias.bgg@gmail.com>, <s.hauer@pengutronix.de>,
	<devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-iio@vger.kernel.org>, <linux-mediatek@lists.infradead.org>
Subject: Re: [PATCH] iio: mtk-auxadc: update case IIO_CHAN_INFO_PROCESSED
Date: Sat, 18 Sep 2021 19:20:59 +0100	[thread overview]
Message-ID: <20210918192059.4c13d157@jic23-huawei> (raw)
In-Reply-To: <20210914130901.1716-2-hui.liu@mediatek.com>

On Tue, 14 Sep 2021 21:09:01 +0800
Hui Liu <hui.liu@mediatek.com> wrote:

> Convert raw data to processed data.
> 
> Fixes: ace4cdfe67be ("iio: adc: mt2701: Add Mediatek auxadc driver for
> mt2701.")
> Signed-off-by: Hui Liu <hui.liu@mediatek.com>

Hi Hui Liu

This fix is obviously correct but I think we can improve it a little.

1) Add a bit more detail to the patch description.  Perhaps change it to something like
Previously the driver did not apply the scaling necessary to take the
voltage range of this ADC into account. 

2) If you change to

	*val = *val * VOLTAGE_FULL_RANGE;
	*val2 = 12;
	return IIO_VAL_FRACTIONAL_LOG2;

then you should get a more precise answer.  (Please check that though!)
This might be an issue if you have consumers drivers though that can not
cope with this particular type.  If so please state that in the patch description
and add a comment to the code to say that so we don't end up 'improving' this
in future without taking those consumers into account.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/mt6577_auxadc.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
> index 79c1dd68b909..d4fccd52ef08 100644
> --- a/drivers/iio/adc/mt6577_auxadc.c
> +++ b/drivers/iio/adc/mt6577_auxadc.c
> @@ -82,6 +82,10 @@ static const struct iio_chan_spec mt6577_auxadc_iio_channels[] = {
>  	MT6577_AUXADC_CHANNEL(15),
>  };
>  
> +/* For Voltage calculation */
> +#define VOLTAGE_FULL_RANGE  1500	/* VA voltage */
> +#define AUXADC_PRECISE      4096	/* 12 bits */
> +
>  static int mt_auxadc_get_cali_data(int rawdata, bool enable_cali)
>  {
>  	return rawdata;
> @@ -191,6 +195,10 @@ static int mt6577_auxadc_read_raw(struct iio_dev *indio_dev,
>  		}
>  		if (adc_dev->dev_comp->sample_data_cali)
>  			*val = mt_auxadc_get_cali_data(*val, true);
> +
> +		/* Convert adc raw data to voltage: 0 - 1500 mV */
> +		*val = *val * VOLTAGE_FULL_RANGE / AUXADC_PRECISE;
> +
>  		return IIO_VAL_INT;
>  
>  	default:

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel