mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Brian Norris <briannorris@chromium.org>
To: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: "Jonathan Cameron" <jic23@kernel.org>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Ulf Hansson" <ulfh@kernel.org>,
	linux-pm@vger.kernel.org, "Andy Shevchenko" <andy@kernel.org>,
	"David Lechner" <dlechner@baylibre.com>,
	linux-iio@vger.kernel.org,
	"Geert Uytterhoeven" <geert+renesas@glider.be>,
	"Magnus Damm" <magnus.damm@gmail.com>,
	"Marek Vasut" <marek.vasut+renesas@mailbox.org>,
	"Rui Miguel Silva" <rmfrfs@gmail.com>,
	linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH v2 00/17] iio: Stop looking at pm_runtime_put_autosuspend() return codes
Date: Fri, 23 Oct 2026 09:43:54 -0700	[thread overview]
Message-ID: <atuOysdhmPRA7Waf@google.com> (raw)
In-Reply-To: <atsEql7X20hGytXv@ashevche-desk.local>

On Fri, Oct 23, 2026 at 09:54:34AM +0300, Andy Shevchenko wrote:
> On Thu, Oct 22, 2026 at 10:47:34AM -0700, Brian Norris wrote:
> > On Thu, Oct 22, 2026 at 10:01:14AM +0300, Andy Shevchenko wrote:
> > > After three patches I stopped because it seems more works needs in each of
> > > the individual drivers. Yes, the series can be applied as is in exchange for
> > > a bigger additional churn in the future. My preference is to see everything
> > > settled down in a single step.
> > 
> > There are limits in how much work I'm willing to do on a bunch of
> > drivers I don't use, can't test, and frankly have no actual interest in.
> > I'm sure if I spent a few more minutes on each driver, I could find
> > another dozen things to try to improve.
> > 
> > I can go back to the v1 approach (fix only what I came to do) if you'd
> > prefer.
> > 
> > Or, I'd prefer to stick with the sentiment Jonathan expressed in
> > optionally requesting additional refactoring:
> > https://lore.kernel.org/all/20261018211807.66bfea0d@jic23-hlaptop/
> > that most of these changes are optional, and that fixing 1 thing is not
> > a subscription to fixing everything.
> 
> Right, but my concerns is that in (some) cases you can do one step further and
> use PM_RUNTIME_ACQUIRE*() instead of putting first the explicit calls in this
> series and makes someone to basically do the same again.

I'm not making anyone do anything except stop looking at the return code
for pm_runtime_put_autosuspend(). Those cleanup-based helpers are
totally optional, and are not always all that helpful. In fact,
sometimes they introduce more problems than they're worth.

> To me it sounds like
> a double effort. Yes, that is_active thing perhaps to much to follow, but even
> there it can be refactored to something like
> 
> 	if (is_active) {
> 		ret = regmap...
> 	} else {
> 		PM_RUNTIME_ACQUIRE*();
> 		...
> 		ret = regmap...
> 	}
> 
> without changing the driver logic.

Well, IMO, this is adding more trickiness about the scoping, precisely
because all these drivers have strange logic. Here, it may or may not be
obvious to someone that the runtime PM acquisition goes out of scope at
the end of the 'else' and so drops the refcount. Then you have to reason
about, "what does the rest of the function do?" -- hopefully nothing
relevant, but that may not always be the case.

Don't get me wrong, I think the cleanup helpers are great for many
things, and especially for well-structured code. But they're optional,
and have their own downsides and risks, especially for unfamiliar
people.

Anyway, I disagree with piling your personal desires on a submitter to
"fix" when they're unrelated, unnecessary, and may cause more work and
problems of their own (or any combination of those 3).

Brian

> Whatever, I am not a maintainer here, so if Jonathan is okay with this churn,
> then it's fine.
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

      reply	other threads:[~2026-10-23 16:43 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-21 23:57 Brian Norris
2026-10-21 23:57 ` [PATCH v2 01/17] iio: accel: fxls8962af: Discard pm_runtime_put_autosuspend() return values Brian Norris
2026-10-22  6:53   ` Andy Shevchenko
2026-10-22 17:51     ` Brian Norris
2026-10-23  3:29       ` Jonathan Cameron
2026-10-21 23:57 ` [PATCH v2 02/17] iio: accel: kxcjk-1013: " Brian Norris
2026-10-22  6:58   ` Andy Shevchenko
2026-10-23  3:36   ` Jonathan Cameron
2026-10-23 16:43     ` Brian Norris
2026-10-21 23:57 ` [PATCH v2 03/17] iio: accel: mma8452: " Brian Norris
2026-10-22  6:59   ` Andy Shevchenko
2026-10-21 23:57 ` [PATCH v2 04/17] iio: accel: mma955x: " Brian Norris
2026-10-21 23:57 ` [PATCH v2 05/17] iio: accel: bmc150: " Brian Norris
2026-10-21 23:57 ` [PATCH v2 06/17] iio: adc: rcar-gyroadc: " Brian Norris
2026-10-22  9:23   ` Geert Uytterhoeven
2026-10-21 23:57 ` [PATCH v2 07/17] iio: adc: ti-ads1015: " Brian Norris
2026-10-21 23:57 ` [PATCH v2 08/17] iio: chemical: atlas-sensor: " Brian Norris
2026-10-21 23:57 ` [PATCH v2 09/17] iio: hid-sensors: " Brian Norris
2026-10-21 23:57 ` [PATCH v2 10/17] iio: gyro: bmg160: " Brian Norris
2026-10-21 23:57 ` [PATCH v2 11/17] iio: gyro: fxas21002c: " Brian Norris
2026-10-21 23:57 ` [PATCH v2 12/17] iio: imu: kmx61: " Brian Norris
2026-10-21 23:57 ` [PATCH v2 13/17] iio: light: apds9960: " Brian Norris
2026-10-21 23:57 ` [PATCH v2 14/17] iio: light: pa12203001: " Brian Norris
2026-10-21 23:57 ` [PATCH v2 15/17] iio: light: rpr0521: " Brian Norris
2026-10-21 23:57 ` [PATCH v2 16/17] iio: light: tsl2583: " Brian Norris
2026-10-21 23:57 ` [PATCH v2 17/17] iio: light: us5182d: " Brian Norris
2026-10-22  7:01 ` [PATCH v2 00/17] iio: Stop looking at pm_runtime_put_autosuspend() return codes Andy Shevchenko
2026-10-22 17:47   ` Brian Norris
2026-10-23  6:54     ` Andy Shevchenko
2026-10-23 16:43       ` Brian Norris [this message]

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=atuOysdhmPRA7Waf@google.com \
    --to=briannorris@chromium.org \
    --cc=andriy.shevchenko@intel.com \
    --cc=andy@kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=geert+renesas@glider.be \
    --cc=jic23@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=magnus.damm@gmail.com \
    --cc=marek.vasut+renesas@mailbox.org \
    --cc=nuno.sa@analog.com \
    --cc=rafael@kernel.org \
    --cc=rmfrfs@gmail.com \
    --cc=ulfh@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®