From: Jonathan Cameron <jic23@kernel.org>
To: "Nuno Sá" <noname.nuno@gmail.com>
Cc: Nuno Sa <nuno.sa@analog.com>,
linux-iio@vger.kernel.org, Lars-Peter Clausen <lars@metafoo.de>
Subject: Re: [PATCH v3 4/4] iio: inkern: move to the cleanup.h magic
Date: Sat, 16 Mar 2024 13:26:18 +0000 [thread overview]
Message-ID: <20240316132618.434ff138@jic23-huawei> (raw)
In-Reply-To: <20240309174145.0834de04@jic23-huawei>
On Sat, 9 Mar 2024 17:41:45 +0000
Jonathan Cameron <jic23@kernel.org> wrote:
> On Mon, 04 Mar 2024 09:04:49 +0100
> Nuno Sá <noname.nuno@gmail.com> wrote:
>
> > On Sun, 2024-03-03 at 14:24 +0000, Jonathan Cameron wrote:
> > > On Thu, 29 Feb 2024 16:10:28 +0100
> > > Nuno Sa <nuno.sa@analog.com> wrote:
> > >
> > > > Use the new cleanup magic for handling mutexes in IIO. This allows us to
> > > > greatly simplify some code paths.
> > > >
> > > > While at it, also use __free(kfree) where allocations are done and drop
> > > > obvious comment in iio_channel_read_min().
> > > >
> > > > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> > >
> > > Hi Nuno
> > >
> > > Series looks very nice. One trivial thing inline - I can tidy that up whilst
> > > applying if nothing else comes up.
> > >
> > > Given this obviously touches a lot of core code, so even though simple it's
> > > high risk for queuing up late. I also have a complex mess already queued up
> > > for the coming merge window. Hence I'm going to hold off on applying this
> > > series until the start of the next cycle.
> > >
> > > Nothing outside IIO is going to depend on it, so it's rather simpler decision
> > > to hold it than for the ones that add new general purpose infrastructure.
> > >
> > >
> >
> > Seems reasonable... It may even give us some time to see how the cond_guard()
> > and scoped_cond_guard() will end up.
>
> Absolutely - thankfully converting to the suggestions Linus made will be straight
> forwards, so hopefully the worst that happens is a complex merge, or some
> fixing up to do afterwards.
>
> >
> > >
> > >
> > > > EXPORT_SYMBOL_GPL(iio_read_channel_attribute);
> > > >
> > > > @@ -757,29 +711,24 @@ int iio_read_channel_processed_scale(struct
> > > > iio_channel *chan, int *val,
> > > > struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(chan-
> > > > >indio_dev);
> > > > int ret;
> > > >
> > > > - mutex_lock(&iio_dev_opaque->info_exist_lock);
> > > > - if (!chan->indio_dev->info) {
> > > > - ret = -ENODEV;
> > > > - goto err_unlock;
> > > > - }
> > > > + guard(mutex)(&iio_dev_opaque->info_exist_lock);
> > > > + if (!chan->indio_dev->info)
> > > > + return -ENODEV;
> > > >
> > > > if (iio_channel_has_info(chan->channel, IIO_CHAN_INFO_PROCESSED)) {
> > > > ret = iio_channel_read(chan, val, NULL,
> > > > IIO_CHAN_INFO_PROCESSED);
> > > > if (ret < 0)
> > > > - goto err_unlock;
> > > > + return ret;
> > > > *val *= scale;
> > >
> > > return 0;
> > >
> > > > } else {
> > > could drop the else.
> > >
> > > > ret = iio_channel_read(chan, val, NULL, IIO_CHAN_INFO_RAW);
> > > > if (ret < 0)
> > > > - goto err_unlock;
> > > > + return ret;
> > > > ret = iio_convert_raw_to_processed_unlocked(chan, *val,
> > > > val,
> > > > scale);
> > > return iio_convert_raw_to_proc...
> > >
> >
> > Hmm, unless I completely misunderstood your comments on v2, this was exactly
> > what I had but you recommended to leave the else branch :).
> >
> That was a younger me :) Either way is fine.
I compromised - move the returns into the two branches, but kept the else.
Given I've started queuing stuff up for next cycle, seemed sensible to pick these
up. Applied to the togreg-normal branch of iio.git.
That will get rebased on rc1 and become togreg as normal in a few weeks time
and hopefully I'll retire the togreg-normal / togreg-cleanup split.
Thanks,
Jonathan
>
> Jonathan
>
>
> > - Nuno Sá
> >
>
>
next prev parent reply other threads:[~2024-03-16 13:26 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-29 15:10 [PATCH v3 0/4] iio: move IIO to the cleanup.h magic Nuno Sa
2024-02-29 15:10 ` [PATCH v3 1/4] iio: core: move to " Nuno Sa
2024-02-29 15:10 ` [PATCH v3 2/4] iio: trigger: move to the " Nuno Sa
2024-03-16 19:32 ` Andy Shevchenko
2024-03-18 12:33 ` Jonathan Cameron
2024-03-18 13:12 ` Andy Shevchenko
2024-03-18 14:15 ` Jonathan Cameron
2024-03-16 19:39 ` Andy Shevchenko
2024-03-18 9:22 ` Nuno Sá
2024-02-29 15:10 ` [PATCH v3 3/4] iio: buffer: iio: core: " Nuno Sa
2024-03-16 19:38 ` Andy Shevchenko
2024-03-18 9:23 ` Nuno Sá
2024-03-18 12:35 ` Jonathan Cameron
2024-03-16 19:49 ` Andy Shevchenko
2024-02-29 15:10 ` [PATCH v3 4/4] iio: inkern: " Nuno Sa
2024-03-03 14:24 ` Jonathan Cameron
2024-03-04 8:04 ` Nuno Sá
2024-03-09 17:41 ` Jonathan Cameron
2024-03-16 13:26 ` Jonathan Cameron [this message]
2024-03-16 19:48 ` Andy Shevchenko
2024-03-18 9:20 ` Nuno Sá
2024-03-23 18:09 ` 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=20240316132618.434ff138@jic23-huawei \
--to=jic23@kernel.org \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=noname.nuno@gmail.com \
--cc=nuno.sa@analog.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).