From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Dooks Date: Mon, 17 Mar 2014 13:44:02 +0000 Subject: Re: [PATCH] sh_eth: ensure pm_runtime cannot suspend the device during init Message-Id: <5326FC22.4060405@codethink.co.uk> List-Id: References: <1394823605-31883-1-git-send-email-ben.dooks@codethink.co.uk> In-Reply-To: <1394823605-31883-1-git-send-email-ben.dooks@codethink.co.uk> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: linux-sh@vger.kernel.org On 17/03/14 13:38, Geert Uytterhoeven wrote: > On Mon, Mar 17, 2014 at 2:36 PM, Ben Dooks wrote: >> I have yet to ascertain how this ends up happening with device probe, >> it seems to be very dependant on the code. > > Adding a WARN() in cpg_mstp_clock_endisable(): > > [] (cpg_mstp_clock_endisable) from [] > (cpg_mstp_clock_disable+0x14/0 > x18) > r10:eec7a000 r9:00000003 r8:eed41508 r7:00000001 r6:a0000113 r5:eec23a00 > r4:eec23a00 > [] (cpg_mstp_clock_disable) from [] > (__clk_disable+0x68/0x78) > [] (__clk_disable) from [] (clk_disable+0x20/0x2c) > r4:60000193 r3:00000002 > [] (clk_disable) from [] (pm_clk_suspend+0x54/0x78) > r5:eed41500 r4:eed414c0 > [] (pm_clk_suspend) from [] (__rpm_callback+0x4c/0x7c) > r8:0000000c r7:00000004 r6:00000000 r5:eed42810 r4:eed42810 r3:c01fcfac > [] (__rpm_callback) from [] (rpm_callback+0x50/0x84) > r5:eed42810 r4:eec7a000 > [] (rpm_callback) from [] (rpm_suspend+0x214/0x3e4) > r6:00000000 r5:00000000 r4:eed42810 r3:eed42810 > [] (rpm_suspend) from [] (rpm_idle+0x184/0x19c) > r10:00000010 r9:00000000 r8:ee4710bc r7:00000004 r6:00000004 r5:00000000 > r4:eed42810 > [] (rpm_idle) from [] (__pm_runtime_idle+0x64/0x7c) > r6:60000113 r5:eed42870 r4:eed42810 r3:eed42870 > [] (__pm_runtime_idle) from [] > (sh_eth_get_stats+0x228/0x23c) > r7:ee471000 r6:c0356bd4 r5:ee428000 r4:ee428000 > [] (sh_eth_get_stats) from [] (dev_get_stats+0x5c/0x84) > r4:eec7bc30 r3:c023ec68 > [] (dev_get_stats) from [] (rtnl_fill_ifinfo+0x410/0x8dc) > r6:00000000 r5:ee428000 r4:eed76ec0 r3:00000684 > [] (rtnl_fill_ifinfo) from [] (rtmsg_ifinfo+0x6c/0xd4) > r10:00000010 r9:ee428510 r8:ffffffff r7:eed76ec0 r6:ee428000 r5:000000d0 > r4:00000000 > [] (rtmsg_ifinfo) from [] (register_netdevice+0x3dc/0x428) > r10:00000000 r8:eed42810 r7:ee428010 r6:ee428000 r5:c047f600 r4:00000000 > [] (register_netdevice) from [] (register_netdev+0x1c/0x2c) > r7:eed42800 r6:ef203978 r5:ee46e790 r4:ee428000 > [] (register_netdev) from [] (sh_eth_drv_probe+0x994/0xb7c) > r4:ee428000 r3:00000040 > [] (sh_eth_drv_probe) from [] (platform_drv_probe+0x20/0x50) > r10:c042b548 r9:c045c544 r8:00000065 r7:c047d830 r6:00000000 r5:c047d830 > r4:eed42810 > [] (platform_drv_probe) from [] > (driver_probe_device+0xc8/0x1f8) > r5:eed42844 r4:eed42810 > [] (driver_probe_device) from [] (__driver_attach+0x68/0x8c) > r7:00000000 r6:c047d830 r5:eed42844 r4:eed42810 > [] (__driver_attach) from [] (bus_for_each_dev+0x5c/0x94) > r6:c01f2558 r5:eec7be48 r4:c047d830 r3:c01f2558 > [] (bus_for_each_dev) from [] (driver_attach+0x20/0x28) > r7:00000000 r6:c047a148 r5:ee461480 r4:c047d830 > [] (driver_attach) from [] (bus_add_driver+0xb4/0x1c8) > [] (bus_add_driver) from [] (driver_register+0xa4/0xe8) > r7:c0484780 r6:00000000 r5:c0454c18 r4:c047d830 > [] (driver_register) from [] > (__platform_driver_register+0x50/0x64) > r5:c0454c18 r4:c0442acc > [] (__platform_driver_register) from [] > (sh_eth_driver_init+0x18/0x20) > [] (sh_eth_driver_init) from [] (do_one_initcall+0x98/0x13c) > [] (do_one_initcall) from [] > (kernel_init_freeable+0x100/0x1cc) > r9:c045c544 r8:00000065 r7:c0484780 r6:c0454bf8 r5:c0454c18 r4:00000006 > [] (kernel_init_freeable) from [] (kernel_init+0x10/0xec) > r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c032af98 > r4:00000000 > [] (kernel_init) from [] (ret_from_fork+0x14/0x3c) > r4:00000000 r3:00000000 this explains it, the call to stats causes a get_sync/put_sync which puts the device into a state where it /could/ be suspended and thus does get suspended in this case from the pm code. I'm not /sure/ why the pm_runtime code is not protecting against running this when a device probe is in progress, but it seems the best thing is to ensure that we always do a get/put sync in the driver to ensure we have a reference during probe. -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius