linux-kernel - Re: [PATCH 6/6] iio: imu: inv_icm42600: Avoid configuring if already pm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <kcrov3lqigiqzea2eds73ibhix46ovqrqkhodfatqwfmjanxya@l2cla3fkl6ow>
Date: Mon, 14 Jul 2025 10:15:59 +0000
From: Sean Nyekjaer <sean@...nix.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@....com>, David Lechner <dlechner@...libre.com>, Nuno Sá <nuno.sa@...log.com>, Andy Shevchenko <andy@...nel.org>, linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 6/6] iio: imu: inv_icm42600: Avoid configuring if already pm_runtime suspended

On Sun, Jul 13, 2025 at 03:32:27PM +0100, Jonathan Cameron wrote:
> On Wed, 09 Jul 2025 14:35:14 +0200
> Sean Nyekjaer <sean@...nix.com> wrote:
> 
> > Do as in suspend, skip resume configuration steps if the device is already
> > pm_runtime suspended. This avoids reconfiguring a device that is already
> > in the correct low-power state and ensures that pm_runtimeM handles the
> > power state transitions properly.
> >
> > Fixes: 31c24c1e93c3 ("iio: imu: inv_icm42600: add core of new inv_icm42600 driver")
> > Signed-off-by: Sean Nyekjaer <sean@...nix.com>
> > ---
> 
> Not really related to what you have here, but this code would really
> benefit from using guard(mutex)()
> 
> Jonathan

I have converted most of this driver to use guard(mutex).

Does it make sense to use guard(mutex) in functions that still relies on
goto error out? Like...

static int inv_icm42600_temp_read(struct inv_icm42600_state *st, s16 *temp)
{
	struct device *dev = regmap_get_device(st->map);
	__be16 *raw;
	int ret;

	pm_runtime_get_sync(dev);
	mutex_lock(&st->lock);

	ret = inv_icm42600_set_temp_conf(st, true, NULL);
	if (ret)
		goto exit;

	raw = (__be16 *)&st->buffer[0];
	ret = regmap_bulk_read(st->map, INV_ICM42600_REG_TEMP_DATA, raw, sizeof(*raw));
	if (ret)
		goto exit;

	*temp = (s16)be16_to_cpup(raw);
	if (*temp == INV_ICM42600_DATA_INVALID)
		ret = -EINVAL;

exit:
	mutex_unlock(&st->lock);
	pm_runtime_mark_last_busy(dev);
	pm_runtime_put_autosuspend(dev);

	return ret;
}


If I use guard_scoped(..) it creates a lot of diff lines...

/Sean