From: Dan Carpenter <dan.carpenter@oracle.com>
To: Liam Beguin <liambeguin@gmail.com>
Cc: lvb@xiphos.com, linux-iio@vger.kernel.org
Subject: Re: [bug report] iio: adc: ad7949: add vref selection support
Date: Wed, 22 Sep 2021 09:00:26 +0300 [thread overview]
Message-ID: <20210922060026.GW2116@kadam> (raw)
In-Reply-To: <YUq63O3ksdr9R3pE@shaak>
On Wed, Sep 22, 2021 at 01:10:52AM -0400, Liam Beguin wrote:
> >
> > 369 if (IS_ERR(ad7949_adc->vref)) {
> > 370 ret = PTR_ERR(ad7949_adc->vref);
> > 371 if (ret != -ENODEV)
> > 372 return ret;
> > 373 /* unbuffered? */
> > 374 ad7949_adc->vref = devm_regulator_get_optional(dev, "vref");
> > 375 if (IS_ERR(ad7949_adc->vref)) {
> > 376 ret = PTR_ERR(ad7949_adc->vref);
> > 377 if (ret != -ENODEV)
> > 378 return ret;
> >
> > Instead of returning NULL when the regulator is disabled it returns
> > -ENODEV. How do you differentiate from an -ENODEV which is caused by an
> > error vs an -ENODEV which is caused because the optional regulator is
> > disabled? You'll just have to hope that the errors are less common and
> > assume it means disabled.
>
> I see.. So far, I've only used fixed-regulators to provide a constant
> voltage reference here, and I guess those are quite unlikely to fail.
>
> > You might be doubting that devm_regulator_get_optional() can return
> > -ENODEV on error? Look at the code and prepare your heart for sadness.
>
> Thanks for the warning, I see what you meant now.
>
> I wasn't able to use smatch to reproduce the error with the following:
>
> make O=builds/smatch CHECK="~/dev/smatch/smatch -p=kernel" C=1 Image.gz
>
> Would you have any pointer for that?
This requires building the cross function Database:
~/dev/smatch/smatch_scripts/build_kernel_data.sh
The command takes 5 hours to run so here is a short cut which just
builds the minimum two files:
~/dev/smatch/smatch_scripts/kchecker --info drivers/regulator/core.c | tee out
~/dev/smatch/smatch_data/db/create_db.sh -p=kernel out
~/dev/smatch/smatch_scripts/kchecker --info drivers/regulator/devres.c | tee out
~/dev/smatch/smatch_data/db/reload_partial.sh out
~/dev/smatch/smatch_scripts/kchecker --spammy drivers/iio/adc/ad7949.c
>
> Anyway, I believe the following would address the error you mentioned.
>
> -- >8 --
> diff --git a/drivers/iio/adc/ad7949.c b/drivers/iio/adc/ad7949.c
> index 44bb5fde83de..3613f4e55e1c 100644
> --- a/drivers/iio/adc/ad7949.c
> +++ b/drivers/iio/adc/ad7949.c
> @@ -368,12 +368,14 @@ static int ad7949_spi_probe(struct spi_device *spi)
> ad7949_adc->vref = devm_regulator_get_optional(dev, "vrefin");
> if (IS_ERR(ad7949_adc->vref)) {
> ret = PTR_ERR(ad7949_adc->vref);
> + ad7949_adc->vref = NULL;
This is not required because it will just be reassigned in a couple
lines.
> if (ret != -ENODEV)
> return ret;
> /* unbuffered? */
> ad7949_adc->vref = devm_regulator_get_optional(dev, "vref");
> if (IS_ERR(ad7949_adc->vref)) {
> ret = PTR_ERR(ad7949_adc->vref);
> + ad7949_adc->vref = NULL;
But this also won't work. Passing a NULL to regulator_enable() will
cause an Oops. All the reference to ->vref need checks. :/
> if (ret != -ENODEV)
> return ret;
> } else {
regards,
dan carpenter
next prev parent reply other threads:[~2021-09-22 6:01 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-21 6:35 [bug report] iio: adc: ad7949: add vref selection support Dan Carpenter
2021-09-22 5:10 ` Liam Beguin
2021-09-22 6:00 ` Dan Carpenter [this message]
2021-09-22 14:48 ` Liam Beguin
2021-09-23 5:47 ` Dan Carpenter
2021-09-24 0:21 ` Liam Beguin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210922060026.GW2116@kadam \
--to=dan.carpenter@oracle.com \
--cc=liambeguin@gmail.com \
--cc=linux-iio@vger.kernel.org \
--cc=lvb@xiphos.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.