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@xxxxxxxxxx> 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@xxxxxxxxxx>
>
> 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.
>
>
> > 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 :).
- Nuno Sá
