Re: [PATCH v3 3/3] iio: adc: qcom-pm8xxx-xoadc: add support for reading channel labels

Andy Shevchenko
2026-04-20T18:22:36Z

On Mon, Apr 20, 2026 at 06:58:07PM +0100, Jonathan Cameron wrote:
> On Mon, 6 Apr 2026 22:46:29 +0300
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > On Sun, Apr 05, 2026 at 04:52:21PM +0000, Antony Kurniawan Soemardi wrote:

...

> > > +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);
> >
> > When you have a validation the better style is to split definition and
> > assignment. This makes code robust against (theoretically) possible changes
> > that might reuse the same variable for something else.
>
> I think you just mean

Yes.

> struct pm8xx_chan_info *ch;
>
> ch = pm8xx_get_...
> if (!ch)
> return -EINVAL?

> I suppose slightly better but it's a very small function and unlikely to have
> much complexity added to it.

I am also on the educational side. If we learn how to do things right from the
beginning...

> > > + if (!ch)
> > > + return -EINVAL;
> > > + return sysfs_emit(label, "%s\n", ch->label);
> > > +}
> >
> > Again, no need to resend now, just make it in the next version if that version
> > is asked for.

--
With Best Regards,
Andy Shevchenko