linux-kernel - Re: [PATCH] usb: musb: fix context save over suspend.

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87zjz9i6s7.fsf@linaro.org>
Date:	Tue, 12 Feb 2013 13:03:36 -0800
From:	Kevin Hilman <khilman@...aro.org>
To:	NeilBrown <neilb@...e.de>
Cc:	Igor Grinberg <grinberg@...pulab.co.il>,
	Felipe Balbi <balbi@...com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-omap@...r.kernel.org
Subject: Re: [PATCH] usb: musb: fix context save over suspend.

NeilBrown <neilb@...e.de> writes:

> On Mon, 21 Jan 2013 13:38:59 +0200 Igor Grinberg <grinberg@...pulab.co.il>
> wrote:
>
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>> 
>> Hi Neil,
>> 
>> On 01/21/13 11:28, NeilBrown wrote:
>> > 
>> > 
>> > The standard suspend sequence involves runtime_resuming
>> > devices before suspending the system.
>> > So just saving context in runtime_suspend and restoring it
>> > in runtime resume isn't enough.  We  must also save in "suspend"
>> > and restore in "resume".
>> > 
>> > Without this patch, and OMAP3 system with off_mode enabled will find
>> > the musb port non-functional after suspend/resume.  With the patch it
>> > works perfectly.
>> 
>> Hmmm... Some time ago, this has been removed in
>> 5d193ce8 (usb: musb: PM: fix context save/restore in suspend/resume path)
>> 
>> Am I missing something? Or things changed and now this patch is correct?
>
> Hi Igor,
>  thanks for alerting me to that patch .... does anyone else get the feeling
>  that power management to too complex to be understood by a mere human?

Yes.  ;)

>  That commit (5d193ce8) suggests that the musb-hdrc device is an
>  'omap_device', or maybe has a PM domain set to something else.
>  However it isn't/doesn't.  dev->pm_domain is NULL.  So no PM domain layer
>  will ever call the musb_core musb_runtime_suspend/resume.
>
>  The parent device - musb-omap2430 - is an omap device, does have pm_domain
>  set, and does have its omap2430_runtime_suspend/resume called for system
>  suspend and so the context for that device is saved and restored.
>  However that doesn't help the context for musb-hdrc.
>
>  Whether musb ever was an omap_device is beyond my archaeological skills to
>  determine.
>
>  Kevin:  Was musb-hdrc ever a device with a pm_domain? or was it only ever
>      the various possible parents that had domains?
>      Are you able to defend your earlier patch in today's kernel?  It
>      certainly causes my device not to work properly.

Sorry for the delay here, I'm back to a place where I can test this on
real hardware.

My patch was fixing a real hang when musb was built-in (or loaded), in
host-mode (mini-A cable attached) but no devices attached.  I just tried
to reproduce this, and with your patch, the system hangs during suspend.

That being said, your description makes sense why this context
save/restore is needed.  Perhaps your patch needs to add a check whether
the device is runtime suspended (I gather this is what Ruslan's patch is
doing.)

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/