| 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
| ||
|
Message-ID: <iwe34sdzsgeqpgqrc7ndbx6n4ef6kiwd4bczbiwbhlfgv3zesx@rnr5l7diy5z7> Date: Fri, 18 Jul 2025 06:40:50 +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 4/6] iio: imu: inv_icm42600: Simplify pm_runtime setup On Wed, Jul 16, 2025 at 09:00:10AM +0100, Jonathan Cameron wrote: > On Mon, 14 Jul 2025 07:42:43 +0000 > Sean Nyekjaer <sean@...nix.com> wrote: > > > 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 > > As below What is the call path for this final suspend? > Is it coming from update_autosuspend()? Yeah it looks like pm_runtime_dont_use_autosuspend() is calling runtime_suspend(). root@v4:/data/root# rmmod inv-icm42600-i2c inv-icm42600 [ 291.511085] inv_icm42600_runtime_resume() -> inv_icm42600_enable_regulator_vddio() [ 291.511165] inv_icm42600_enable_regulator_vddio() enable vddio [ 291.532398] inv_icm42600_runtime_suspend() disable vddio [ 291.538517] CPU: 0 UID: 0 PID: 331 Comm: rmmod Tainted: G W 6.16.0-rc1-00202-g7fe6e564b5c9-dirty #146 VOLUNTARY [ 291.538559] Tainted: [W]=WARN [ 291.538566] Hardware name: Freescale i.MX6 Ultralite (Device Tree) [ 291.538575] Call trace: [ 291.538590] unwind_backtrace from show_stack+0x10/0x14 [ 291.538643] show_stack from dump_stack_lvl+0x54/0x68 [ 291.538679] dump_stack_lvl from inv_icm42600_runtime_suspend+0x68/0x6c [inv_icm42600] [ 291.538728] inv_icm42600_runtime_suspend [inv_icm42600] from __rpm_callback+0x48/0x18c [ 291.538775] __rpm_callback from rpm_callback+0x5c/0x68 [ 291.538815] rpm_callback from rpm_suspend+0xdc/0x584 [ 291.538853] rpm_suspend from pm_runtime_disable_action+0x30/0x5c [ 291.538885] pm_runtime_disable_action from devres_release_group+0x180/0x1a0 [ 291.538917] devres_release_group from i2c_device_remove+0x34/0x84 [ 291.538949] i2c_device_remove from device_release_driver_internal+0x180/0x1f4 [ 291.538976] device_release_driver_internal from driver_detach+0x54/0xa0 [ 291.538998] driver_detach from bus_remove_driver+0x58/0xa4 [ 291.539030] bus_remove_driver from sys_delete_module+0x16c/0x250 [ 291.539065] sys_delete_module from ret_fast_syscall+0x0/0x54 [ 291.539089] Exception stack(0xd0ab1fa8 to 0xd0ab1ff0) [ 291.539108] 1fa0: be94fe46 be94fd1c 017ccfa4 00000800 0000000a 017ccf68 [ 291.539126] 1fc0: be94fe46 be94fd1c 017ccf68 00000081 00000000 00000001 00000003 017cc190 [ 291.539139] 1fe0: b6c8be41 be94fadc 000179cb b6c8be48 [ 291.685102] inv_icm42600_disable_vddio_reg() disable vddio [ 291.694566] ------------[ cut here ]------------ [ 291.694621] WARNING: CPU: 0 PID: 331 at drivers/regulator/core.c:3016 _regulator_disable+0x140/0x1a0 [ 291.708496] unbalanced disables for regulator-dummy [ 291.713391] Modules linked in: inv_icm42600_i2c(-) inv_icm42600 inv_sensors_timestamp [last unloaded: inv_icm42600] [ 291.723939] CPU: 0 UID: 0 PID: 331 Comm: rmmod Tainted: G W 6.16.0-rc1-00202-g7fe6e564b5c9-dirty #146 VOLUNTARY [ 291.735620] Tainted: [W]=WARN [ 291.738598] Hardware name: Freescale i.MX6 Ultralite (Device Tree) [ 291.744789] Call trace: [ 291.744807] unwind_backtrace from show_stack+0x10/0x14 [ 291.752614] show_stack from dump_stack_lvl+0x54/0x68 [ 291.757704] dump_stack_lvl from __warn+0x7c/0xe0 [ 291.762437] __warn from warn_slowpath_fmt+0x124/0x18c [ 291.767602] warn_slowpath_fmt from _regulator_disable+0x140/0x1a0 [ 291.773812] _regulator_disable from regulator_disable+0x48/0x80 [ 291.779843] regulator_disable from devres_release_group+0x180/0x1a0 [ 291.786231] devres_release_group from i2c_device_remove+0x34/0x84 [ 291.792446] i2c_device_remove from device_release_driver_internal+0x180/0x1f4 [ 291.799699] device_release_driver_internal from driver_detach+0x54/0xa0 [ 291.806427] driver_detach from bus_remove_driver+0x58/0xa4 [ 291.812033] bus_remove_driver from sys_delete_module+0x16c/0x250 [ 291.818160] sys_delete_module from ret_fast_syscall+0x0/0x54 [ 291.823934] Exception stack(0xd0ab1fa8 to 0xd0ab1ff0) [ 291.829007] 1fa0: be94fe46 be94fd1c 017ccfa4 00000800 0000000a 017ccf68 [ 291.837205] 1fc0: be94fe46 be94fd1c 017ccf68 00000081 00000000 00000001 00000003 017cc190 [ 291.845397] 1fe0: b6c8be41 be94fadc 000179cb b6c8be48 [ 291.850632] ---[ end trace 0000000000000000 ]---0 > > > [ 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... ? > > That will make a mess if runtime PM is not built into > the kernel which is why an PM state in remove should return to the state > before it was enabled in the first place (i.e. on!). > That final runtime suspend surprises me. Got it :) /Sean
Powered by blists - more mailing lists