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
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
[1] https://github.com/LineageOS/android_kernel_sony_msm8960t/blob/93319b1e5aa343ec1c1aabcb028c5e88c7df7c01/drivers/hwmon/pm8xxx-adc.c#L407-L408
Signed-off-by: Antony Kurniawan Soemardi <linux@smankusors.com>
---
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);
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;
--
2.34.1On 10/27/25 6: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 > 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 > > [1] https://github.com/LineageOS/android_kernel_sony_msm8960t/blob/93319b1e5aa343ec1c1aabcb028c5e88c7df7c01/drivers/hwmon/pm8xxx-adc.c#L407-L408 > > Signed-off-by: Antony Kurniawan Soemardi <linux@smankusors.com> > --- > 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, The PM8921 datasheet suggests a single valid value of BIT(5)=1, BIT(6)=0 for a "1K" (1/1024?) ratio, although a comment in this file suggests BIT(5)|BIT(6) is also valid and corresponds to 1/4096.. I wouldn't be surprised if that is the case The previously set bits are a field called DECI_SEL but are otherwise left undescribed Hope this is useful Konrad
On 10/28/2025 4:44 PM, Konrad Dybcio wrote: > On 10/27/25 6: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 >> 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 >> >> [1] https://github.com/LineageOS/android_kernel_sony_msm8960t/blob/93319b1e5aa343ec1c1aabcb028c5e88c7df7c01/drivers/hwmon/pm8xxx-adc.c#L407-L408 >> >> Signed-off-by: Antony Kurniawan Soemardi <linux@smankusors.com> >> --- >> 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, > The PM8921 datasheet suggests a single valid value of BIT(5)=1, BIT(6)=0 > for a "1K" (1/1024?) ratio, although a comment in this file suggests > BIT(5)|BIT(6) is also valid and corresponds to 1/4096.. I wouldn't be > surprised if that is the case > > The previously set bits are a field called DECI_SEL but are otherwise left > undescribed So, do you think we can leave the BIT(0) and BIT(1) as is? I have a feeling that if they aren't set, these changes might prevent the APQ8060 Dragonboard from reading the cm3605 sensor? or maybe not? I mean this one, since the driver was originally tested on that board https://github.com/torvalds/linux/blob/e53642b87a4f4b03a8d7e5f8507fc3cd0c595ea6/arch/arm/boot/dts/qcom/qcom-apq8060-dragonboard.dts#L67-L79 > Hope this is useful > > Konrad Thanks, Antony K. S.
On 10/30/25 5:57 PM, Antony Kurniawan Soemardi wrote: > On 10/28/2025 4:44 PM, Konrad Dybcio wrote: >> On 10/27/25 6: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 >>> 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 >>> >>> [1] https://github.com/LineageOS/android_kernel_sony_msm8960t/blob/93319b1e5aa343ec1c1aabcb028c5e88c7df7c01/drivers/hwmon/pm8xxx-adc.c#L407-L408 >>> >>> Signed-off-by: Antony Kurniawan Soemardi <linux@smankusors.com> >>> --- >>> 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, >> The PM8921 datasheet suggests a single valid value of BIT(5)=1, BIT(6)=0 >> for a "1K" (1/1024?) ratio, although a comment in this file suggests >> BIT(5)|BIT(6) is also valid and corresponds to 1/4096.. I wouldn't be >> surprised if that is the case >> >> The previously set bits are a field called DECI_SEL but are otherwise left >> undescribed > > So, do you think we can leave the BIT(0) and BIT(1) as is? I have a feeling > that if they aren't set, these changes might prevent the APQ8060 Dragonboard > from reading the cm3605 sensor? or maybe not? > > I mean this one, since the driver was originally tested on that board > https://github.com/torvalds/linux/blob/e53642b87a4f4b03a8d7e5f8507fc3cd0c595ea6/arch/arm/boot/dts/qcom/qcom-apq8060-dragonboard.dts#L67-L79 +Dmitry would you have that (or similar) board in your museum? Konrad
On Fri, Oct 31, 2025 at 10:00:25AM +0100, Konrad Dybcio wrote: > On 10/30/25 5:57 PM, Antony Kurniawan Soemardi wrote: > > On 10/28/2025 4:44 PM, Konrad Dybcio wrote: > >> On 10/27/25 6: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 > >>> 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 > >>> > >>> [1] https://github.com/LineageOS/android_kernel_sony_msm8960t/blob/93319b1e5aa343ec1c1aabcb028c5e88c7df7c01/drivers/hwmon/pm8xxx-adc.c#L407-L408 > >>> > >>> Signed-off-by: Antony Kurniawan Soemardi <linux@smankusors.com> > >>> --- > >>> 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, > >> The PM8921 datasheet suggests a single valid value of BIT(5)=1, BIT(6)=0 > >> for a "1K" (1/1024?) ratio, although a comment in this file suggests > >> BIT(5)|BIT(6) is also valid and corresponds to 1/4096.. I wouldn't be > >> surprised if that is the case > >> > >> The previously set bits are a field called DECI_SEL but are otherwise left > >> undescribed > > > > So, do you think we can leave the BIT(0) and BIT(1) as is? I have a feeling > > that if they aren't set, these changes might prevent the APQ8060 Dragonboard > > from reading the cm3605 sensor? or maybe not? > > > > I mean this one, since the driver was originally tested on that board > > https://github.com/torvalds/linux/blob/e53642b87a4f4b03a8d7e5f8507fc3cd0c595ea6/arch/arm/boot/dts/qcom/qcom-apq8060-dragonboard.dts#L67-L79 > > +Dmitry would you have that (or similar) board in your museum? I do, but I sadly I didn't test it lately (nor do I remember if I have sensors board or not). I can try testing it next week, if I have time. -- With best wishes Dmitry
On 11/1/2025 4:25 PM, Dmitry Baryshkov wrote: > On Fri, Oct 31, 2025 at 10:00:25AM +0100, Konrad Dybcio wrote: >> On 10/30/25 5:57 PM, Antony Kurniawan Soemardi wrote: >>> On 10/28/2025 4:44 PM, Konrad Dybcio wrote: >>>> On 10/27/25 6: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 >>>>> 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 >>>>> >>>>> [1] https://github.com/LineageOS/android_kernel_sony_msm8960t/blob/93319b1e5aa343ec1c1aabcb028c5e88c7df7c01/drivers/hwmon/pm8xxx-adc.c#L407-L408 >>>>> >>>>> Signed-off-by: Antony Kurniawan Soemardi <linux@smankusors.com> >>>>> --- >>>>> 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, >>>> The PM8921 datasheet suggests a single valid value of BIT(5)=1, BIT(6)=0 >>>> for a "1K" (1/1024?) ratio, although a comment in this file suggests >>>> BIT(5)|BIT(6) is also valid and corresponds to 1/4096.. I wouldn't be >>>> surprised if that is the case >>>> >>>> The previously set bits are a field called DECI_SEL but are otherwise left >>>> undescribed >>> So, do you think we can leave the BIT(0) and BIT(1) as is? I have a feeling >>> that if they aren't set, these changes might prevent the APQ8060 Dragonboard >>> from reading the cm3605 sensor? or maybe not? >>> >>> I mean this one, since the driver was originally tested on that board >>> https://github.com/torvalds/linux/blob/e53642b87a4f4b03a8d7e5f8507fc3cd0c595ea6/arch/arm/boot/dts/qcom/qcom-apq8060-dragonboard.dts#L67-L79 >> +Dmitry would you have that (or similar) board in your museum? > I do, but I sadly I didn't test it lately (nor do I remember if I have > sensors board or not). I can try testing it next week, if I have time. > Hi Dmitry, I wanted to follow up and ask whether you’ve had a chance to test the APQ8060 DragonBoard though? (Also happy new year! Eh, it's 12 days late...) Thanks,
On Sun, Jan 11, 2026 at 05:31:45PM +0000, Antony Kurniawan Soemardi wrote: > On 11/1/2025 4:25 PM, Dmitry Baryshkov wrote: > > On Fri, Oct 31, 2025 at 10:00:25AM +0100, Konrad Dybcio wrote: > > > On 10/30/25 5:57 PM, Antony Kurniawan Soemardi wrote: > > > > On 10/28/2025 4:44 PM, Konrad Dybcio wrote: > > > > > On 10/27/25 6: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 > > > > > > 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 > > > > > > > > > > > > [1] https://github.com/LineageOS/android_kernel_sony_msm8960t/blob/93319b1e5aa343ec1c1aabcb028c5e88c7df7c01/drivers/hwmon/pm8xxx-adc.c#L407-L408 > > > > > > > > > > > > Signed-off-by: Antony Kurniawan Soemardi <linux@smankusors.com> > > > > > > --- > > > > > > 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, > > > > > The PM8921 datasheet suggests a single valid value of BIT(5)=1, BIT(6)=0 > > > > > for a "1K" (1/1024?) ratio, although a comment in this file suggests > > > > > BIT(5)|BIT(6) is also valid and corresponds to 1/4096.. I wouldn't be > > > > > surprised if that is the case > > > > > > > > > > The previously set bits are a field called DECI_SEL but are otherwise left > > > > > undescribed > > > > So, do you think we can leave the BIT(0) and BIT(1) as is? I have a feeling > > > > that if they aren't set, these changes might prevent the APQ8060 Dragonboard > > > > from reading the cm3605 sensor? or maybe not? > > > > > > > > I mean this one, since the driver was originally tested on that board > > > > https://github.com/torvalds/linux/blob/e53642b87a4f4b03a8d7e5f8507fc3cd0c595ea6/arch/arm/boot/dts/qcom/qcom-apq8060-dragonboard.dts#L67-L79 > > > +Dmitry would you have that (or similar) board in your museum? > > I do, but I sadly I didn't test it lately (nor do I remember if I have > > sensors board or not). I can try testing it next week, if I have time. > > > Hi Dmitry, I wanted to follow up and ask whether you’ve had a chance to test > the APQ8060 DragonBoard though? No, after reshuffling of the lab it's still in the box, sorry. > > (Also happy new year! Eh, it's 12 days late...) > > Thanks, -- With best wishes Dmitry
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.
> 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.
>
> [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.
> Signed-off-by: Antony Kurniawan Soemardi <linux@smankusors.com>
> ---
> 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.
> 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;
>
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@smankusors.com>
>> ---
>> 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;
>>
On 10/27/25 2:53 PM, Antony Kurniawan Soemardi wrote:
> 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?
The correct format is:
Fixes: 63c3ecd946d4 ("iio: adc: add a driver for Qualcomm PM8xxx HK/XOADC")
See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-changes
>>> Signed-off-by: Antony Kurniawan Soemardi <linux@smankusors.com>
>>> ---
>>> 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?
Either way is fine. It's just a "nice to have" change.
>>> 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;
>>>
>
© 2016 - 2026 Red Hat, Inc.