linux-kernel - Re: [PATCH 4/6] iio: imu: inv_icm42600: Simplify 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: <76fnxeuufv56fmfvq6odi5xz2yjtjxymz24t436zk7rtuyst4s@oihlvsoxhllp>
Date: Mon, 14 Jul 2025 07:42:43 +0000
From: Sean Nyekjaer <sean@...nix.com>
To: Jonathan Cameron <jic23@...nel.org>, 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 4/6] iio: imu: inv_icm42600: Simplify pm_runtime setup

On Mon, Jul 14, 2025 at 07:24:57AM +0100, Sean Nyekjaer wrote:
> On Sun, Jul 13, 2025 at 03:28:10PM +0100, Jonathan Cameron wrote:
> > On Wed, 09 Jul 2025 14:35:12 +0200
> > Sean Nyekjaer <sean@...nix.com> wrote:
> > 
> > > Remove unnecessary pm_runtime_get_noresume() and pm_runtime_put()
> > > calls during probe. These are not required when the device is marked
> > > active via pm_runtime_set_active() before enabling pm_runtime with
> > > pm_runtime_enable().
> > >
> > > Also remove the redundant pm_runtime_put_sync() call from the cleanup
> > > path, since the core is not incrementing the usage count beforehand.
> > >
> > > This simplifies the PM setup and avoids manipulating the usage counter
> > > unnecessarily.
> > 
> > Could we switch directly to using devm_pm_runtime_enable() for this driver?
> > 
> > At first glance looks like this code is missing the disable of autosuspend
> > that should be there (which devm_pm_runtime_enable() will also handle).
> > 
> 
> I have tried to use devm_pm_runtime_enable() but on rmmod it warns
> "unbalanced disables for regulator"
> 
> If I remove this:
> -	ret = devm_add_action_or_reset(dev, inv_icm42600_disable_vddio_reg, st);
> -	if (ret)
> -		return ret;
> 
> Everything seems okay again. I have checked with printk's that
> inv_icm42600_disable_vddio_reg() is called twice with
> devm_pm_runtime_enable() used.
> Does it make sense?
> 
> /Sean

with pm_runtime_enable():
root@v4:/data/root# insmod /tmp/inv-icm42600.ko; insmod /tmp/inv-icm42600-i2c.ko
[ 3793.713077] inv-icm42600-i2c 1-0068: no INT1 interrupt defined, fallback to first interrupt
[ 3793.727728] inv-icm42600-i2c 1-0068: mounting matrix not found: using identity...
[ 3793.737660] inv-icm42600-i2c 1-0068: supply vdd not found, using dummy regulator
[ 3793.856891] inv-icm42600-i2c 1-0068: supply vddio not found, using dummy regulator
[ 3793.866872] inv_icm42600_enable_regulator_vddio() enable vddio
[ 3793.920739] inv_icm42600_runtime_suspend() disable vddio
root@v4:/data/root# rmmod inv_icm42600_i2c inv_icm42600
[ 3796.954850] inv_icm42600_runtime_resume() -> inv_icm42600_enable_regulator_vddio()
[ 3796.954910] inv_icm42600_enable_regulator_vddio() enable vddio
[ 3796.985140] inv_icm42600_disable_vddio_reg() disable vddio

with devm_pm_runtime_enable():
root@v4:/data/root# insmod /tmp/inv-icm42600.ko; insmod /tmp/inv-icm42600-i2c.ko
[ 3852.873887] inv-icm42600-i2c 1-0068: no INT1 interrupt defined, fallback to first interrupt
[ 3852.888715] inv-icm42600-i2c 1-0068: mounting matrix not found: using identity...
[ 3852.898514] inv-icm42600-i2c 1-0068: supply vdd not found, using dummy regulator
[ 3853.016890] inv-icm42600-i2c 1-0068: supply vddio not found, using dummy regulator
[ 3853.026860] inv_icm42600_enable_regulator_vddio() enable vddio
[ 3853.080835] inv_icm42600_runtime_suspend() disable vddio
root@v4:/data/root# rmmod inv_icm42600_i2c inv_icm42600
[ 3854.448461] inv_icm42600_runtime_resume() -> inv_icm42600_enable_regulator_vddio()
[ 3854.448540] inv_icm42600_enable_regulator_vddio() enable vddio
[ 3854.467061] inv_icm42600_runtime_suspend() disable vddio
[ 3854.477170] inv_icm42600_disable_vddio_reg() disable vddio
[ 3854.483835] ------------[ cut here ]------------
[ 3854.483912] WARNING: CPU: 0 PID: 582 at drivers/regulator/core.c:3016 _regulator_disable+0x140/0x1a0
[ 3854.497853] unbalanced disables for regulator

Is the way from here to remove the devm_add_action_or_reset(dev,
inv_icm42600_disable_vddio_reg... ?

/Sean