Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM

From: Alan Stern
Date: Mon Mar 21 2016 - 10:57:53 EST


On Mon, 21 Mar 2016, Oliver Neukum wrote:

> On Sun, 2016-03-20 at 11:43 +0100, Geert Uytterhoeven wrote:
> > If CONFIG_PM=n:
> >
> > drivers/net/usb/lan78xx.c: In function âlan78xx_get_stats64â:
> > drivers/net/usb/lan78xx.c:3274: error: âstruct dev_pm_infoâ has no
> > member named âruntime_autoâ
> >
> > If PM is disabled, the runtime_auto flag is not available, but auto
> > suspend is not enabled anyway. Hence protect the check for
> > runtime_auto
> > by #ifdef CONFIG_PM to fix this.
> >
> > Fixes: a59f8c5b048dc938 ("lan78xx: add ndo_get_stats64")
> > Reported-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> > Signed-off-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> > ---
> > Alternatively, we can add a dev_pm_runtime_auto_is_enabled() wrapper
> > to
> > include/linux/pm.h, which always return false if CONFIG_PM is
> > disabled.
>
> That is what we do for almost everything else in include/pm_runtime.h
> So it is the best solution to add the stub.

Guenter's question about whether drivers should try to access
runtime_auto in the first place was a good one. A similar problem
arises in the block layer: When a block device has removable media, the
block layer probes at 1-second intervals looking for media changes.
This defeats autosuspend in the same way as we're talking about here.

One possible solution is to export a sysfs parameter to prevent
statistics collection (or more generally, to change the interval at
which it occurs).

But checking the runtime_auto flag is probably not a great idea. Even
if it isn't set, collecting statistics is likely to wait up a device
that otherwise would have remained suspended.

Perhaps the best solution is to collect the statistics only when the
device is not suspended or is about to suspend.

Alan Stern