This series adds support for reading channel labels from the device tree
in the Qualcomm PM8xxx XOADC driver, along with the corresponding DT
updates for the PM8921 PMIC. Also removes the redundant error logs when
reading values, as returning -EINVAL and -ETIMEDOUT are sufficient to
report failures.
This series also includes minor refactoring, such as removing redundant
error loggings.
Tested on Sony Xperia SP (PM8921):
iio_hwmon-isa-0000
Adapter: ISA adapter
vbat: 267.86 V
dcin: 4.82 kV
...
chg_temp: +1071.4°C
Note: the sensor readings above are incorrect due to pending calibration
fixes are not included in this series. This patch only addresses label
visibility. The values are expected to be wrong until the calibration
fixes lands separately.
---
Changes in v6:
- Onelining wait_for_completion_timeout on removing redundant error logs
- Fixed dtbs_check failure by adding the missing label property to the
DT bindings
- Link to v5: https://patch.msgid.link/20260427-pm8xxx-xoadc-label-v5-0-9e7b5a53ef99@smankusors.com
Changes in v5:
- Drop constify pointers patch as it causes build failures
- Link to v4: https://patch.msgid.link/20260426-pm8xxx-xoadc-label-v4-0-0ec782362615@smankusors.com
Changes in v4:
- Constify pointers in read operations
- Reworked wait_for_completion_timeout() handling
- Updated variable declaration style in pm8xxx_read_label()
- Link to v3: https://lore.kernel.org/r/20260405-pm8xxx-xoadc-label-v3-0-9fe179c283ec@smankusors.com
Changes in v3:
- Simplify label assignment logic by removing the unnecessary
conditional branch
- Remove redundant error logging in pm8xxx_read_channel_rsv, since
-ETIMEDOUT is sufficient to report failures.
- Link to v2: https://lore.kernel.org/r/20260403-pm8xxx-xoadc-label-v2-0-29b50bf821e6@smankusors.com
Changes in v2:
- Remove redundant error logging in pm8xxx_read_raw, since -EINVAL is
sufficient to report failures.
- Reword the uncommon Tested-by commit tag to freeform text
- Link to v1: https://lore.kernel.org/r/20260326-pm8xxx-xoadc-label-v1-0-027805dad4db@smankusors.com
Changes since original patch:
- The label is now read from the platform description (device tree)
instead of the internal datasheet name.
- Link to original patch:
https://lore.kernel.org/all/20251028-pm8xxx-xoadc-fix-v1-1-b000e1036e41@smankusors.com/
---
Antony Kurniawan Soemardi (4):
dt-bindings: iio: adc: qcom,pm8018-adc: add label property for ADC channels
ARM: dts: qcom: pm8921: add labels for ADC channels
iio: adc: qcom-pm8xxx-xoadc: remove redundant error logs when reading values
iio: adc: qcom-pm8xxx-xoadc: add support for reading channel labels
.../bindings/iio/adc/qcom,pm8018-adc.yaml | 15 +++++++++
arch/arm/boot/dts/qcom/pm8921.dtsi | 12 +++++++
drivers/iio/adc/qcom-pm8xxx-xoadc.c | 39 ++++++++++++----------
3 files changed, 49 insertions(+), 17 deletions(-)
---
base-commit: 7527e0bfead35a65431b3e865333e5277f97e706
change-id: 20260321-pm8xxx-xoadc-label-47afdf7f06a9
Best regards,
--
Antony Kurniawan Soemardi [off-list ref]
Add label properties to all XOADC ADC channel nodes in the PM8921 PMIC
device tree. This allows userspace and drivers to identify channels by
name rather than relying on datasheet name.
Acked-by: Konrad Dybcio <redacted>
Reviewed-by: Dmitry Baryshkov <redacted>
Signed-off-by: Antony Kurniawan Soemardi <redacted>
---
arch/arm/boot/dts/qcom/pm8921.dtsi | 12 ++++++++++++
1 file changed, 12 insertions(+)
Implement the .read_label callback to allow userspace to identify ADC
channels via the "label" property in the device tree. The name field in
pm8xxx_chan_info is renamed to label to better reflect its purpose. If
no label is provided in the device tree, it defaults to the hardware
datasheet name.
The change has been tested on Sony Xperia SP (PM8921).
Reviewed-by: Dmitry Baryshkov <redacted>
Signed-off-by: Antony Kurniawan Soemardi <redacted>
---
drivers/iio/adc/qcom-pm8xxx-xoadc.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
@@ -716,8 +716,21 @@ static int pm8xxx_fwnode_xlate(struct iio_dev *indio_dev,return-EINVAL;}+staticintpm8xxx_read_label(structiio_dev*indio_dev,+structiio_chan_specconst*chan,char*label)+{+structpm8xxx_xoadc*adc=iio_priv(indio_dev);+conststructpm8xxx_chan_info*ch;++ch=pm8xxx_get_channel(adc,chan->address);+if(!ch)+return-EINVAL;+returnsysfs_emit(label,"%s\n",ch->label);+}+staticconststructiio_infopm8xxx_xoadc_info={.fwnode_xlate=pm8xxx_fwnode_xlate,+.read_label=pm8xxx_read_label,.read_raw=pm8xxx_read_raw,};
@@ -761,7 +774,8 @@ static int pm8xxx_xoadc_parse_channel(struct device *dev,pre_scale_mux,amux_channel);return-EINVAL;}-ch->name=name;+ch->label=hwchan->datasheet_name;+fwnode_property_read_string(fwnode,"label",&ch->label);ch->hwchan=hwchan;/* Everyone seems to use absolute calibration except in special cases */ch->calibration=VADC_CALIB_ABSOLUTE;
From: Andy Shevchenko <hidden> Date: 2026-05-10 07:19:44
On Sun, May 10, 2026 at 07:01:45AM +0000, Antony Kurniawan Soemardi wrote:
Implement the .read_label callback to allow userspace to identify ADC
I would write in a usual pattern here, id est .read_label().
But no need to resend just for that.
channels via the "label" property in the device tree. The name field in
pm8xxx_chan_info is renamed to label to better reflect its purpose. If
no label is provided in the device tree, it defaults to the hardware
datasheet name.
The change has been tested on Sony Xperia SP (PM8921).
Add a new optional label property for ADC channels to help users
identify each channel when reading values from the sysfs interface.
Signed-off-by: Antony Kurniawan Soemardi <redacted>
---
.../devicetree/bindings/iio/adc/qcom,pm8018-adc.yaml | 15 +++++++++++++++
1 file changed, 15 insertions(+)
@@ -78,6 +78,10 @@ patternProperties:reg:maxItems:1+label:+description:|+Unique name to identify which channel this is.+qcom,decimation:$ref:/schemas/types.yaml#/definitions/uint32description:|
On Sun, May 10, 2026 at 9:04 AM Antony Kurniawan Soemardi
[off-list ref] wrote:
Add a new optional label property for ADC channels to help users
identify each channel when reading values from the sysfs interface.
Signed-off-by: Antony Kurniawan Soemardi <redacted>
Makes sense to me, nice with some roadsigns in this djungle.
Reviewed-by: Linus Walleij <linusw@kernel.org>
Yours,
Linus Walleij
On Sun, May 10, 2026 at 9:02 AM Antony Kurniawan Soemardi
[off-list ref] wrote:
Implement the .read_label callback to allow userspace to identify ADC
channels via the "label" property in the device tree. The name field in
pm8xxx_chan_info is renamed to label to better reflect its purpose. If
no label is provided in the device tree, it defaults to the hardware
datasheet name.
The change has been tested on Sony Xperia SP (PM8921).
Reviewed-by: Dmitry Baryshkov <redacted>
Signed-off-by: Antony Kurniawan Soemardi <redacted>
From: Jonathan Cameron <jic23@kernel.org> Date: 2026-05-11 16:27:40
On Sun, 10 May 2026 07:01:29 +0000 (UTC)
Antony Kurniawan Soemardi [off-list ref] wrote:
This series adds support for reading channel labels from the device tree
in the Qualcomm PM8xxx XOADC driver, along with the corresponding DT
updates for the PM8921 PMIC. Also removes the redundant error logs when
reading values, as returning -EINVAL and -ETIMEDOUT are sufficient to
report failures.
This series also includes minor refactoring, such as removing redundant
error loggings.
Tested on Sony Xperia SP (PM8921):
iio_hwmon-isa-0000
Adapter: ISA adapter
vbat: 267.86 V
dcin: 4.82 kV
...
chg_temp: +1071.4°C
Note: the sensor readings above are incorrect due to pending calibration
fixes are not included in this series. This patch only addresses label
visibility. The values are expected to be wrong until the calibration
fixes lands separately.
Applied patches 3 and 4 to the testing branch of iio.git.
When I get caught up with the backlog and the tests finish I'll push that
out as togreg for linux-next to pick up.
Thanks,
Jonathan
From: Jonathan Cameron <jic23@kernel.org> Date: 2026-05-11 16:29:26
On Mon, 11 May 2026 11:28:18 +0200
Linus Walleij [off-list ref] wrote:
On Sun, May 10, 2026 at 9:04 AM Antony Kurniawan Soemardi
[off-list ref] wrote:
quoted
Add a new optional label property for ADC channels to help users
identify each channel when reading values from the sysfs interface.
Signed-off-by: Antony Kurniawan Soemardi <redacted>
Makes sense to me, nice with some roadsigns in this djungle.
Reviewed-by: Linus Walleij <linusw@kernel.org>
I've applied this as well on basis it's very standard but
happy to have feedback from DT folk.
Thanks,
Jonathan
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The timeout value passed to wait_for_completion_timeout() is in microseconds but the function expects jiffies, resulting in a massively inflated wait time.
- [High] The timeout error path in pm8xxx_read_channel_rsv() fails to disable the ADC arbiter, which can cause delayed interrupts to corrupt subsequent readings.
- [Medium] Removing the error log for -ETIMEDOUT hides legitimate hardware and driver failures from dmesg.
--
commit 24271541e5551b415dc4bb4616582d63863e0096
Author: Antony Kurniawan Soemardi [off-list ref]
iio: adc: qcom-pm8xxx-xoadc: remove redundant error logs when reading values
This commit drops dev_err() logging for -EINVAL and -ETIMEDOUT cases and relies
on return values to report errors, reducing unnecessary log noise.
@@ -535,10 +535,7 @@ static int pm8xxx_read_channel_rsv(struct pm8xxx_xoadc *adc,gotounlock;/* Next the interrupt occurs */-ret=wait_for_completion_timeout(&adc->complete,-VADC_CONV_TIME_MAX_US);-if(!ret){-dev_err(adc->dev,"conversion timed out\n");+if(!wait_for_completion_timeout(&adc->complete,VADC_CONV_TIME_MAX_US)){
This is a pre-existing issue, but wait_for_completion_timeout() expects a
timeout in jiffies. Does passing VADC_CONV_TIME_MAX_US directly result in a
massively inflated wait time instead of the intended microseconds?
Also, while removing the log for an invalid channel addresses user input
noise, does completely suppressing the -ETIMEDOUT error hide legitimate
hardware or interrupt failures from dmesg? Would dev_err_ratelimited() be a
better alternative here?
ret = -ETIMEDOUT;
goto unlock;
This is also a pre-existing issue, but if the timeout occurs and the code
jumps directly to the unlock label, it bypasses turning off the ADC arbiter
which is handled later in the function:
pm8xxx_read_channel_rsv() {
...
/* Turn off the ADC by setting the arbiter to 0 twice */
ret = regmap_write(adc->map, ADC_ARB_USRP_CNTRL, 0);
if (ret)
goto unlock;
...
}
Does jumping to unlock leave the hardware running with its interrupt armed,
potentially causing a delayed interrupt to fire and corrupt a subsequent
reading?
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The new `.read_label` callback and pre-existing `.read_raw` fail to uniquely identify channels, causing incorrect labels and voltage readings.
- [High] Race condition in probe exposes uncalibrated device, risking divide-by-zero kernel panic.
--
commit 50e7569708562dacb984c644e8571f2c7c6624cb
Author: Antony Kurniawan Soemardi [off-list ref]
iio: adc: qcom-pm8xxx-xoadc: add support for reading channel labels
This commit implements the .read_label callback to allow userspace to
identify ADC channels via the device tree label property. The name field
in pm8xxx_chan_info is renamed to label, defaulting to the datasheet name.
Is amux_channel guaranteed to be a unique identifier?
Looking at the hardware definition array, multiple channels (like vcoin and
atest_8) use amux_channel = 0x00 and differ only in their pre_scale_mux
values.
Since pm8xxx_get_channel() only checks chan->address against
ch->hwchan->amux_channel:
pm8xxx_get_channel() {
...
for (i = 0; i < adc->nchans; i++) {
struct pm8xxx_chan_info *ch = &adc->chans[i];
if (ch->hwchan->amux_channel == chan)
return ch;
}
...
}
Could this return the first matching channel instead of the correct one if
multiple overlapping channels are configured, causing this new callback to
return incorrect labels?
This also appears to be a pre-existing issue in pm8xxx_read_raw() which
might configure the multiplexer incorrectly and read the wrong hardware
voltages.
Since adc->chans and adc->iio_chans are allocated together, could the
correct channel be resolved using pointer arithmetic, such as
&adc->chans[chan - adc->iio_chans]?
While reviewing this driver, I also noticed a pre-existing race condition in
pm8xxx_xoadc_probe().
Does calling iio_device_register() before pm8xxx_calibrate_device() expose
the IIO sysfs attributes to userspace too early?
If userspace reads a processed channel value concurrently, pm8xxx_read_raw()
will invoke qcom_vadc_scale().
Since adc->graph is allocated via devm_iio_device_alloc() and hasn't been
populated by the calibration routine yet, calib_graph->dy will be 0.
Could this result in a divide-by-zero panic in qcom_vadc_scale_calib()
when computing div64_s64(*scale_voltage, calib_graph->dy)?
Should iio_device_register() be moved to the end of the probe function,
after all hardware initialization and calibration are complete?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260510-pm8xxx-xoadc-label-v6-0-49700fd03005@smankusors.com?part=4
On Sun, 10 May 2026 07:01:29 +0000, Antony Kurniawan Soemardi wrote:
This series adds support for reading channel labels from the device tree
in the Qualcomm PM8xxx XOADC driver, along with the corresponding DT
updates for the PM8921 PMIC. Also removes the redundant error logs when
reading values, as returning -EINVAL and -ETIMEDOUT are sufficient to
report failures.
This series also includes minor refactoring, such as removing redundant
error loggings.
[...]