From: Dan Carpenter <dan.carpenter@oracle.com>
To: Jonathan Cameron <jic23@kernel.org>
Cc: Alexandru Ardelean <alexandru.ardelean@analog.com>,
linux-iio@vger.kernel.org, devel@driverdev.osuosl.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: iio: ad5933: rework probe to use devm_ function variants
Date: Thu, 7 May 2020 12:50:16 +0300 [thread overview]
Message-ID: <20200507095016.GC9365@kadam> (raw)
In-Reply-To: <20200502192542.63cc25a2@archlinux>
On Sat, May 02, 2020 at 07:25:42PM +0100, Jonathan Cameron wrote:
> On Tue, 28 Apr 2020 12:31:28 +0300
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> > +static void ad5933_cleanup(void *data)
> > +{
> > + struct ad5933_state *st = data;
> > +
> > + clk_disable_unprepare(st->mclk);
> > + regulator_disable(st->reg);
>
> Please do two separate callbacks so that these can be handled
> in the correct places. I.e. you do something then immediately
> register the handler to undo it.
>
> Currently you can end up disabling a clock you haven't enabled
> (which I am fairly sure will give you an error message).
Yeah. It does.
It feels like we should just make a devm_ version of regulator_enable().
Or potentially this is more complicated than it seems, but in that case
probably adding devm_add_action_or_reset() is more complicated than it
seems as well.
regards,
dan carpenter
next prev parent reply other threads:[~2020-05-07 9:50 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-28 9:31 [PATCH] staging: iio: ad5933: rework probe to use devm_ function variants Alexandru Ardelean
2020-05-02 18:25 ` Jonathan Cameron
2020-05-04 5:52 ` Ardelean, Alexandru
2020-05-07 9:50 ` Dan Carpenter [this message]
2020-05-08 12:43 ` Jonathan Cameron
2020-05-08 12:57 ` Mark Brown
2020-05-08 15:30 ` Jonathan Cameron
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=20200507095016.GC9365@kadam \
--to=dan.carpenter@oracle.com \
--cc=alexandru.ardelean@analog.com \
--cc=devel@driverdev.osuosl.org \
--cc=jic23@kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/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.