| Message ID | 1395396913-24354-1-git-send-email-ben.dooks@codethink.co.uk |
|---|---|
| State | Deferred, archived |
| Delegated to: | David Miller |
| Headers | show |
Hi Ben, On Fri, Mar 21, 2014 at 11:15 AM, Ben Dooks <ben.dooks@codethink.co.uk> wrote: > The driver has a no-op for the pm_runtime callbacks but > the pm_runtime core should correctly ignore drivers without > any pm_rumtime callback ops. The pm_runtime core doesn't ignore non-existing runtime_{suspend,resume} callbacks, it turns them into a failure withv -ENOSYS. Only non-existing runtime_idle callbacks are ignored. rpm_suspend(): callback = rpm_get_suspend_cb(dev); retval = rpm_callback(callback, dev); if (retval) goto fail; (rpm_callback() returns -ENOSYS if !callback). pm_runtime_force_suspend(): callback = rpm_get_suspend_cb(dev); if (!callback) { ret = -ENOSYS; goto err; } Same for resume. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 21/03/14 11:30, Geert Uytterhoeven wrote: > Hi Ben, > > On Fri, Mar 21, 2014 at 11:15 AM, Ben Dooks <ben.dooks@codethink.co.uk> wrote: >> The driver has a no-op for the pm_runtime callbacks but >> the pm_runtime core should correctly ignore drivers without >> any pm_rumtime callback ops. > > The pm_runtime core doesn't ignore non-existing runtime_{suspend,resume} > callbacks, it turns them into a failure withv -ENOSYS. > Only non-existing runtime_idle callbacks are ignored. I've added Rafael Wysocki as he may be able to add better feedback to this issue. [snip rpm_susend code block] I got very confused here. The clock_ops sets dev->pm_domain which over-rides the use of the dev->driver->pm entry as the primary pm for the device. The code above the bit you snipped does a ladder looking for the pm_runtime entry it calls and would stop at finding dev->pm_domain as so: from drivers/base/power/runtime.c: 495 if (dev->pm_domain) 496 callback = dev->pm_domain->ops.runtime_suspend; ... 502 callback = dev->bus->pm->runtime_suspend; 503 else 504 callback = NULL; So for drivers on shmobile with drivers/sh/pm_runtime.c enabled we would never call the drivers' entry as the ops that this code introduces just calls the pm_clk calls and does not send the events on. If we send the events on, then we would use pm_generic_runtime_suspend() to send it. This call treats the lack of runtime_pm driver entry as a do nothing and return 0 which means in this case the code in sh_eth is not necessary to have any pm_runtime functions. This means depending on if we have a pm_domain in the path we get different treatment of NULL runtime pm ops pointer. I am not sure how to handle this, as IIRC a number of other drivers for Renesas currently assume that the NULL case is going to be fine for them. Changing pm_generic_runtime_suspend() to return ENOSYS would end up breaking davinci and probably a number of other platforms. So questions: - Should rpm_suspend() ignore the lack of pm_runtime operations? - Do we need to add a generic `ignore pm runtime callback` - Are any other shmobile drivers similarly affected?
From: Ben Dooks <ben.dooks@codethink.co.uk> Date: Fri, 21 Mar 2014 11:57:23 +0100 > On 21/03/14 11:30, Geert Uytterhoeven wrote: >> Hi Ben, >> >> On Fri, Mar 21, 2014 at 11:15 AM, Ben Dooks >> <ben.dooks@codethink.co.uk> wrote: >>> The driver has a no-op for the pm_runtime callbacks but >>> the pm_runtime core should correctly ignore drivers without >>> any pm_rumtime callback ops. >> >> The pm_runtime core doesn't ignore non-existing >> runtime_{suspend,resume} >> callbacks, it turns them into a failure withv -ENOSYS. >> Only non-existing runtime_idle callbacks are ignored. > > I've added Rafael Wysocki as he may be able to add better > feedback to this issue. This discussion has stalled so I'm marking this patch as "deferred" in patchwork. Please resubmit once things are resolved, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c index b908507..bb93333e 100644 --- a/drivers/net/ethernet/renesas/sh_eth.c +++ b/drivers/net/ethernet/renesas/sh_eth.c @@ -2998,28 +2998,6 @@ static int sh_eth_drv_remove(struct platform_device *pdev) return 0; } -#ifdef CONFIG_PM -static int sh_eth_runtime_nop(struct device *dev) -{ - /* Runtime PM callback shared between ->runtime_suspend() - * and ->runtime_resume(). Simply returns success. - * - * This driver re-initializes all registers after - * pm_runtime_get_sync() anyway so there is no need - * to save and restore registers here. - */ - return 0; -} - -static const struct dev_pm_ops sh_eth_dev_pm_ops = { - .runtime_suspend = sh_eth_runtime_nop, - .runtime_resume = sh_eth_runtime_nop, -}; -#define SH_ETH_PM_OPS (&sh_eth_dev_pm_ops) -#else -#define SH_ETH_PM_OPS NULL -#endif - static struct platform_device_id sh_eth_id_table[] = { { "sh7619-ether", (kernel_ulong_t)&sh7619_data }, { "sh771x-ether", (kernel_ulong_t)&sh771x_data }, @@ -3043,7 +3021,6 @@ static struct platform_driver sh_eth_driver = { .id_table = sh_eth_id_table, .driver = { .name = CARDNAME, - .pm = SH_ETH_PM_OPS, .of_match_table = of_match_ptr(sh_eth_match_table), }, };
The driver has a no-op for the pm_runtime callbacks but the pm_runtime core should correctly ignore drivers without any pm_rumtime callback ops. Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> --- drivers/net/ethernet/renesas/sh_eth.c | 23 ----------------------- 1 file changed, 23 deletions(-)