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.

Tested-on: Sony Xperia SP (PM8921)
Signed-off-by: Antony Kurniawan Soemardi <linux@smankusors.com>
---
 drivers/iio/adc/qcom-pm8xxx-xoadc.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/qcom-pm8xxx-xoadc.c b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
index 31f88cf7f7f18297132d152648b312c0fb60608e..206a379ea74113264d4e5e75a81f838204174700 100644
--- a/drivers/iio/adc/qcom-pm8xxx-xoadc.c
+++ b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
@@ -369,7 +369,7 @@ static const struct xoadc_channel pm8921_xoadc_channels[] = {
 
 /**
  * struct pm8xxx_chan_info - ADC channel information
- * @name: name of this channel
+ * @label: label of this channel from device tree (defaults to datasheet name if not specified)
  * @hwchan: pointer to hardware channel information (muxing & scaling settings)
  * @calibration: whether to use absolute or ratiometric calibration
  * @decimation: 0,1,2,3
@@ -377,7 +377,7 @@ static const struct xoadc_channel pm8921_xoadc_channels[] = {
  * calibration: 0, 1, 2, 4, 5.
  */
 struct pm8xxx_chan_info {
-	const char *name;
+	const char *label;
 	const struct xoadc_channel *hwchan;
 	enum vadc_calibration calibration;
 	u8 decimation:2;
@@ -446,7 +446,7 @@ static int pm8xxx_read_channel_rsv(struct pm8xxx_xoadc *adc,
 	u8 lsb, msb;
 
 	dev_dbg(adc->dev, "read channel \"%s\", amux %d, prescale/mux: %d, rsv %d\n",
-		ch->name, ch->hwchan->amux_channel, ch->hwchan->pre_scale_mux, rsv);
+		ch->label, ch->hwchan->amux_channel, ch->hwchan->pre_scale_mux, rsv);
 
 	mutex_lock(&adc->lock);
 
@@ -725,8 +725,22 @@ static int pm8xxx_fwnode_xlate(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
+static int pm8xxx_read_label(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan, char *label)
+{
+	struct pm8xxx_xoadc *adc = iio_priv(indio_dev);
+	struct pm8xxx_chan_info *ch = pm8xxx_get_channel(adc, chan->address);
+
+	if (!ch) {
+		dev_err(adc->dev, "no such channel %lu\n", chan->address);
+		return -EINVAL;
+	}
+	return sysfs_emit(label, "%s\n", ch->label);
+}
+
 static const struct iio_info pm8xxx_xoadc_info = {
 	.fwnode_xlate = pm8xxx_fwnode_xlate,
+	.read_label = pm8xxx_read_label,
 	.read_raw = pm8xxx_read_raw,
 };
 
@@ -770,7 +784,9 @@ static int pm8xxx_xoadc_parse_channel(struct device *dev,
 			pre_scale_mux, amux_channel);
 		return -EINVAL;
 	}
-	ch->name = name;
+	ret = fwnode_property_read_string(fwnode, "label", &ch->label);
+	if (ret)
+		ch->label = hwchan->datasheet_name;
 	ch->hwchan = hwchan;
 	/* Everyone seems to use absolute calibration except in special cases */
 	ch->calibration = VADC_CALIB_ABSOLUTE;
@@ -812,7 +828,7 @@ static int pm8xxx_xoadc_parse_channel(struct device *dev,
 
 	dev_dbg(dev,
 		"channel [PRESCALE/MUX: %02x AMUX: %02x] \"%s\" ref voltage: %d, decimation %d prescale %d/%d, scale function %d\n",
-		hwchan->pre_scale_mux, hwchan->amux_channel, ch->name,
+		hwchan->pre_scale_mux, hwchan->amux_channel, ch->label,
 		ch->amux_ip_rsv, ch->decimation, hwchan->prescale.numerator,
 		hwchan->prescale.denominator, hwchan->scale_fn_type);
 

-- 
2.34.1

On Wed, Mar 25, 2026 at 09:02:41PM +0000, Antony Kurniawan Soemardi 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.

> Tested-on: Sony Xperia SP (PM8921)

Interesting, never saw this tag before.

> Signed-off-by: Antony Kurniawan Soemardi <linux@smankusors.com>

...

> +static int pm8xxx_read_label(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan, char *label)
> +{
> +	struct pm8xxx_xoadc *adc = iio_priv(indio_dev);
> +	struct pm8xxx_chan_info *ch = pm8xxx_get_channel(adc, chan->address);

> +	if (!ch) {
> +		dev_err(adc->dev, "no such channel %lu\n", chan->address);
> +		return -EINVAL;
> +	}

Isn't it a dead code? Also poisoning dmesg with this recurrent message is not
good idea to begin with (the user space will have a door to flood it, which
might be considered as an assistance to hackers to clear immediate logs after
a successful attack).

> +	return sysfs_emit(label, "%s\n", ch->label);
> +}

-- 
With Best Regards,
Andy Shevchenko

On 3/26/2026 5:18 PM, Andy Shevchenko wrote:
 >> Tested-on: Sony Xperia SP (PM8921)
 >
 > Interesting, never saw this tag before.

Oh, I just realized I misremember Tested-by tag as Tested-on... Let me
know if it's not acceptable.

 >> +	if (!ch) {
 >> +		dev_err(adc->dev, "no such channel %lu\n", chan->address);
 >> +		return -EINVAL;
 >> +	}
 >
 > Isn't it a dead code? Also poisoning dmesg with this recurrent 
message is not
 > good idea to begin with (the user space will have a door to flood it, 
which
 > might be considered as an assistance to hackers to clear immediate 
logs after
 > a successful attack).

Good point about the successful attack hint! I was copying the existing
code from pm8xxx_read_raw. Do you think those checks are unnecessary for
pm8xxx_read_raw as well?

Thanks,
Antony K. S.

On Thu, Mar 26, 2026 at 12:00:52PM +0000, Antony Kurniawan Soemardi wrote:
> On 3/26/2026 5:18 PM, Andy Shevchenko wrote:

...

> >> Tested-on: Sony Xperia SP (PM8921)
> >
> > Interesting, never saw this tag before.
> 
> Oh, I just realized I misremember Tested-by tag as Tested-on... Let me
> know if it's not acceptable.

You can just put it in a free text:

  "The change has been tested on ..."

...


> >> +	if (!ch) {
> >> +		dev_err(adc->dev, "no such channel %lu\n", chan->address);
> >> +		return -EINVAL;
> >> +	}
> >
> > Isn't it a dead code? Also poisoning dmesg with this recurrent message is
> > not good idea to begin with (the user space will have a door to flood it,
> > which might be considered as an assistance to hackers to clear immediate
> > logs after a successful attack).
> 
> Good point about the successful attack hint! I was copying the existing
> code from pm8xxx_read_raw. Do you think those checks are unnecessary for
> pm8xxx_read_raw as well?

Yes, I think they are not as the returned code should be enough to identify
the problem. (For no such channel I would rather see -ENOENT, but we can't
simply replace that in the existing code as it's part of ABI.)

-- 
With Best Regards,
Andy Shevchenko

On 3/27/2026 2:04 AM, Andy Shevchenko wrote:
> On Thu, Mar 26, 2026 at 12:00:52PM +0000, Antony Kurniawan Soemardi wrote:
>> On 3/26/2026 5:18 PM, Andy Shevchenko wrote:
> ...
>>>> +	if (!ch) {
>>>> +		dev_err(adc->dev, "no such channel %lu\n", chan->address);
>>>> +		return -EINVAL;
>>>> +	}
>>>
>>> Isn't it a dead code? Also poisoning dmesg with this recurrent message is
>>> not good idea to begin with (the user space will have a door to flood it,
>>> which might be considered as an assistance to hackers to clear immediate
>>> logs after a successful attack).
>>
>> Good point about the successful attack hint! I was copying the existing
>> code from pm8xxx_read_raw. Do you think those checks are unnecessary for
>> pm8xxx_read_raw as well?
> 
> Yes, I think they are not as the returned code should be enough to identify
> the problem. (For no such channel I would rather see -ENOENT, but we can't
> simply replace that in the existing code as it's part of ABI.)

Just to re-clarify, do you mean for both pm8xxx_read_label &
pm8xxx_read_raw:
1. if the check fails, it should only return -EINVAL without any
    logging; or
2. remove the checks because there's no way it's not found?

-- 
Thanks,
Antony K. S.

On Fri, Mar 27, 2026 at 07:05:04PM +0000, Antony Kurniawan Soemardi wrote:
> On 3/27/2026 2:04 AM, Andy Shevchenko wrote:
> > On Thu, Mar 26, 2026 at 12:00:52PM +0000, Antony Kurniawan Soemardi wrote:
> > > On 3/26/2026 5:18 PM, Andy Shevchenko wrote:

...

> > > > > +	if (!ch) {
> > > > > +		dev_err(adc->dev, "no such channel %lu\n", chan->address);
> > > > > +		return -EINVAL;
> > > > > +	}
> > > > 
> > > > Isn't it a dead code? Also poisoning dmesg with this recurrent message is
> > > > not good idea to begin with (the user space will have a door to flood it,
> > > > which might be considered as an assistance to hackers to clear immediate
> > > > logs after a successful attack).
> > > 
> > > Good point about the successful attack hint! I was copying the existing
> > > code from pm8xxx_read_raw. Do you think those checks are unnecessary for
> > > pm8xxx_read_raw as well?
> > 
> > Yes, I think they are not as the returned code should be enough to identify
> > the problem. (For no such channel I would rather see -ENOENT, but we can't
> > simply replace that in the existing code as it's part of ABI.)
> 
> Just to re-clarify, do you mean for both pm8xxx_read_label &
> pm8xxx_read_raw:
> 1. if the check fails, it should only return -EINVAL without any
>    logging; or
> 2. remove the checks because there's no way it's not found?

The first one. And yeah, -EINVAL in the both cases for the sake of consistency.

-- 
With Best Regards,
Andy Shevchenko