On Sun, Dec 02, 2012 at 02:48:50PM +0100, Rafael J. Wysocki wrote:
> On Sunday, December 02, 2012 07:46:25 PM NeilBrown wrote:
> > On Mon, 26 Mar 2012 00:29:24 +0200 "Rafael J. Wysocki" <rjw@xxxxxxx> wrote:
> >
> >
> > > Thanks for the confirmation.
> > >
> > > Below it goes again with a changelog and tags.
> > >
> > > I don't really think that SDIO does the right thing here overall, but that's
> > > all I can do to address the problem timely.
> > >
> > > Thanks,
> > > Rafael
> >
> > Hi Rafael,
> > I just discovered that this patch has since been reverted - with an 'ack'
> > from you:
> > ----------
> > commit d8e2ac330f65bcf47e8894fe5331a7e8ee019c06
> > Author: Thierry Reding <thierry.reding@xxxxxxxxxxxxxxxxx>
> > Date: Thu Aug 9 09:32:21 2012 +0000
> >
> > mmc: sdio: Fix PM_SLEEP related build warnings
> >
> > Power management callbacks defined by SIMPLE_DEV_PM_OPS are only used if
> > the PM_SLEEP Kconfig symbol has been defined. If not, the compiler will
> > complain about them being unused. However, since the callback for this
> > driver doesn't do anything it can just as well be dropped.
> >
> > Signed-off-by: Thierry Reding <thierry.reding@xxxxxxxxxxxxxxxxx>
> > Acked-by: Rafael J. Wysocki <rjw@xxxxxxx>
> > Signed-off-by: Chris Ball <cjb@xxxxxxxxxx>
> > -----------
> >
> > Unsurprisingly the problem which your patch fixed has come back.
> >
> > Do you think we could get the patch back in again. This time maybe we should
> > put some comments in there pointing out that having a function which does
> > nothing is very different from not having any function at all?
>
> Well, I agree. I didn't remember that the callback had been added for
> a purpose and hence my "ack" for that patch.
>
> What about applying the appended patch (hopefully, the build warnings should
> be fixed properly this time)?
>
> Rafael
>
>
> ---
> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> Subject: SDIO / PM: Add empty bus-level suspend/resume callbacks
>
> Suspend methods provided by SDIO drivers are not supposed to be
> called by the PM core. Instead, when the SDIO core gets to suspend
> a device's ancestor, it calls the device driver's suspend routine.
> However, the PM core executes suspend callback routines directly for
> device drivers whose bus types don't provide suspend callbacks.
> In consequece, because the SDIO bus type doesn't provide a suspend
> callback, the SDIO drivers' suspend routines will be executed by the
> PM core (which shouldn't happen).
>
> To prevent this from happening, add empty system suspend/resume
> callbacks for the SDIO bus type.
>
> An analogous change had been made already by commit (e841a7c mmc:
> sdio: Use empty system suspend/resume callbacks at the bus level),
> but then it was reverted inadvertently by commit (d8e2ac3 mmc: sdio:
> Fix PM_SLEEP related build warnings) that attempted to fix build
> warnings introduced by commit e841a7c.
>
> Reported-by: NeilBrown <neilb@xxxxxxx>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> ---
> drivers/mmc/core/sdio_bus.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> Index: linux/drivers/mmc/core/sdio_bus.c
> ===================================================================
> --- linux.orig/drivers/mmc/core/sdio_bus.c
> +++ linux/drivers/mmc/core/sdio_bus.c
> @@ -193,7 +193,21 @@ static int sdio_bus_remove(struct device
> }
>
> #ifdef CONFIG_PM
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int pm_no_operation(struct device *dev)
> +{
> + /*
> + * Prevent the PM core from calling SDIO device drivers' suspend
> + * callback routines, which it is not supposed to do, by using this
> + * empty function as the bus type suspend callaback for SDIO.
> + */
> + return 0;
> +}
> +#endif
> +
> static const struct dev_pm_ops sdio_bus_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(pm_no_operation, pm_no_operation)
> SET_RUNTIME_PM_OPS(
> pm_generic_runtime_suspend,
> pm_generic_runtime_resume,
>
Hehe... if my memory serves me well, that's exactly (well, modulo the
comment) what my initial patch did before somebody suggested that the
empty callbacks should just be removed altogether. =)
Thierry
Attachment:
pgpgPdQw9SL74.pgp
Description: PGP signature
