Re: [PATCH 2/2] iio: adc: qcom-pm8xxx-xoadc: fix incorrect calibration values

From: Antony Kurniawan Soemardi

Date: Mon Oct 27 2025 - 15:54:07 EST

On 10/28/2025 1:35 AM, David Lechner wrote:
On 10/27/25 12:29 PM, Antony Kurniawan Soemardi wrote:
On msm8960 phones, the XOADC driver was using incorrect calibration
values:
absolute calibration dx = 625000 uV, dy = 4 units
ratiometric calibration dx = 1800, dy = -29041 units

As a result, reading from the IIO bus returned unexpected results:
in_voltage_7 (USB_VBUS): 0
in_voltage_10 (125V): 0

The issue was caused by not setting the ratiometric scale (amux_ip_rsv)
from the predefined channels. Additionally, the downstream code always
Mentioning downstream kernels is usually a red flag. :-)

We can justify it here though by saying that there is no documentation
available other than downstream source code, so we are just using it
as a reference.
ah ok, rewording needed then
set the ADC_ARB_USRP_DIG_PARAM register to PM8XXX_ADC_ARB_ANA_DIG [1].
That value does not include the SEL_SHIFT0 and SEL_SHIFT1 bits. Enabling
those bits caused calibration errors too, so they were removed.

With these fixes, calibration now uses the correct values:
absolute calibration dx = 625000 uV, dy = 6307 units
ratiometric calibration dx = 1800, dy = 18249 units

Reading from the IIO bus now returns expected results:
in_voltage_7 (USB_VBUS): 4973836
in_voltage_10 (125V): 1249405
Would be useful to mention which hardware you tested with in case
it turns out that there is some other hardware that does require the
SHIFT0/1 bits to be set.
I did mention Sony Xperia SP from cover letter, but I haven't
referenced it in this commit yet. Also I tried to search on Github for
SHIFT0/1 bits, but couldn't find any usage of them...
[1] https://github.com/LineageOS/android_kernel_sony_msm8960t/blob/93319b1e5aa343ec1c1aabcb028c5e88c7df7c01/drivers/hwmon/pm8xxx-adc.c#L407-L408

Since this is a fix, it should have a Fixes: tag. And it sounds like
possibly two separate fixes. In that case, it should be two separate
patches.
Fixes into 63c3ecd946d4ae2879ec0d8c6dcb90132a74d831?
Signed-off-by: Antony Kurniawan Soemardi <linux@xxxxxxxxxxxxxx>
---
drivers/iio/adc/qcom-pm8xxx-xoadc.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/qcom-pm8xxx-xoadc.c b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
index 8555f34036fb13c41ac720dc02c1dc39876e9198..a53d361456ec36b66d258041877bd96ab37838c4 100644
--- a/drivers/iio/adc/qcom-pm8xxx-xoadc.c
+++ b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
@@ -503,10 +503,11 @@ static int pm8xxx_read_channel_rsv(struct pm8xxx_xoadc *adc,
goto unlock;
/* Decimation factor */
- ret = regmap_write(adc->map, ADC_ARB_USRP_DIG_PARAM,
- ADC_ARB_USRP_DIG_PARAM_SEL_SHIFT0 |
- ADC_ARB_USRP_DIG_PARAM_SEL_SHIFT1 |
- ch->decimation << ADC_DIG_PARAM_DEC_SHIFT);
+ ret = regmap_update_bits(adc->map,
+ ADC_ARB_USRP_DIG_PARAM,
+ ADC_ARB_USRP_DIG_PARAM_DEC_RATE0 |
+ ADC_ARB_USRP_DIG_PARAM_DEC_RATE1,
+ ch->decimation << ADC_DIG_PARAM_DEC_SHIFT);
As a follow-up patch, it would be nice to update the driver to use FIELD_PREP().

I.e. remove ADC_ARB_USRP_DIG_PARAM_DEC_RATE0, ADC_ARB_USRP_DIG_PARAM_DEC_RATE1
and ADC_DIG_PARAM_DEC_SHIFT macros and replace them with one macro:

#define ADC_ARB_USRP_DIG_PARAM_DEC_RATE GENMASK(6, 5)

Then use it like:

ret = regmap_update_bits(adc->map,
ADC_ARB_USRP_DIG_PARAM,
ADC_ARB_USRP_DIG_PARAM_DEC_RATE,
FIELD_PREP(ADC_ARB_USRP_DIG_PARAM_DEC_RATE,
ch->decimation));

This should be done for all of the similar multi-bit fields.
as a follow up patch, you mean next version of this patch series, or
separate patch series?
if (ret)
goto unlock;
@@ -783,6 +784,7 @@ static int pm8xxx_xoadc_parse_channel(struct device *dev,
ch->calibration = VADC_CALIB_ABSOLUTE;
/* Everyone seems to use default ("type 2") decimation */
ch->decimation = VADC_DEF_DECIMATION;
+ ch->amux_ip_rsv = hwchan->amux_ip_rsv;
if (!fwnode_property_read_u32(fwnode, "qcom,ratiometric", &rsv)) {
ch->calibration = VADC_CALIB_RATIOMETRIC;