linux-kernel - Re: [PATCH v2 4/5] iio: adc: ad7380: fix supplies for ad7380-4

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEHHSvZxrt3cPmmLwNj9nts9KhBWg4CwnzWnoTXYJL30AbJBsA@mail.gmail.com>
Date: Mon, 21 Oct 2024 14:50:55 +0200
From: Julien Stephan <jstephan@...libre.com>
To: Nuno Sá <noname.nuno@...il.com>
Cc: Lars-Peter Clausen <lars@...afoo.de>, Michael Hennerich <Michael.Hennerich@...log.com>, 
	Nuno Sá <nuno.sa@...log.com>, 
	David Lechner <dlechner@...libre.com>, Jonathan Cameron <jic23@...nel.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>, 
	Jonathan Corbet <corbet@....net>, linux-iio@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Conor Dooley <conor.dooley@...rochip.com>, 
	Jonathan Cameron <Jonathan.Cameron@...wei.com>, linux-doc@...r.kernel.org
Subject: Re: [PATCH v2 4/5] iio: adc: ad7380: fix supplies for ad7380-4

Le lun. 21 oct. 2024 à 13:18, Nuno Sá <noname.nuno@...il.com> a écrit :
>
> On Mon, 2024-10-21 at 12:00 +0200, Julien Stephan wrote:
> > ad7380-4 is the only device in the family that does not have an internal
> > reference. It uses "refin" as a required external reference.
> > All other devices in the family use "refio"" as an optional external
> > reference.
> >
> > Fixes: 737413da8704 ("iio: adc: ad7380: add support for ad738x-4 4 channels
> > variants")
> > Signed-off-by: Julien Stephan <jstephan@...libre.com>
> > ---
>
> Hi Julien,
>
> Patch looks good. Sorry if this already came out in the previous version or in
> the other patchset you mention but shouldn't this fix come first in the series?
>

Hi Nuno,
That was my plan at first, but doing the
devm_regulator_get_enable_read_voltage() first, simplifies the next
changes ... and also eases the review :)

If needed I can do the rebase

Cheers
Julien

> Anyways, for the patch itself:
>
> Reviewed-by: Nuno Sa <nuno.sa@...log.com>
>
> >  drivers/iio/adc/ad7380.c | 36 ++++++++++++++++++++++++++----------
> >  1 file changed, 26 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c
> > index
> > e257f78d63edd7910fcb936ec5344922f8e70b99..65096717f0dd3ea6a4ff7020bc544d62b84c
> > b8fd 100644
> > --- a/drivers/iio/adc/ad7380.c
> > +++ b/drivers/iio/adc/ad7380.c
> > @@ -89,6 +89,7 @@ struct ad7380_chip_info {
> >       bool has_mux;
> >       const char * const *supplies;
> >       unsigned int num_supplies;
> > +     bool external_ref_only;
> >       const char * const *vcm_supplies;
> >       unsigned int num_vcm_supplies;
> >       const unsigned long *available_scan_masks;
> > @@ -431,6 +432,7 @@ static const struct ad7380_chip_info ad7380_4_chip_info =
> > {
> >       .num_simult_channels = 4,
> >       .supplies = ad7380_supplies,
> >       .num_supplies = ARRAY_SIZE(ad7380_supplies),
> > +     .external_ref_only = true,
> >       .available_scan_masks = ad7380_4_channel_scan_masks,
> >       .timing_specs = &ad7380_4_timing,
> >  };
> > @@ -1047,17 +1049,31 @@ static int ad7380_probe(struct spi_device *spi)
> >                                    "Failed to enable power supplies\n");
> >       msleep(T_POWERUP_MS);
> >
> > -     /*
> > -      * If there is no REFIO supply, then it means that we are using
> > -      * the internal 2.5V reference, otherwise REFIO is reference voltage.
> > -      */
> > -     ret = devm_regulator_get_enable_read_voltage(&spi->dev, "refio");
> > -     if (ret < 0 && ret != -ENODEV)
> > -             return dev_err_probe(&spi->dev, ret,
> > -                                  "Failed to get refio regulator\n");
> > +     if (st->chip_info->external_ref_only) {
> > +             ret = devm_regulator_get_enable_read_voltage(&spi->dev,
> > +                                                          "refin");
> > +             if (ret < 0)
> > +                     return dev_err_probe(&spi->dev, ret,
> > +                                          "Failed to get refin
> > regulator\n");
> > +
> > +             st->vref_mv = ret / 1000;
> >
> > -     external_ref_en = ret != -ENODEV;
> > -     st->vref_mv = external_ref_en ? ret / 1000 : AD7380_INTERNAL_REF_MV;
> > +             /* these chips don't have a register bit for this */
> > +             external_ref_en = false;
> > +     } else {
> > +             /*
> > +              * If there is no REFIO supply, then it means that we are
> > using
> > +              * the internal reference, otherwise REFIO is reference
> > voltage.
> > +              */
> > +             ret = devm_regulator_get_enable_read_voltage(&spi->dev,
> > +                                                          "refio");
> > +             if (ret < 0 && ret != -ENODEV)
> > +                     return dev_err_probe(&spi->dev, ret,
> > +                                          "Failed to get refio
> > regulator\n");
> > +
> > +             external_ref_en = ret != -ENODEV;
> > +             st->vref_mv = external_ref_en ? ret / 1000 :
> > AD7380_INTERNAL_REF_MV;
> > +     }
> >
> >       if (st->chip_info->num_vcm_supplies > ARRAY_SIZE(st->vcm_mv))
> >               return dev_err_probe(&spi->dev, -EINVAL,
> >
>