Re: [PATCH 07/10] ehci-dbgp,ehci: Allow early or late use of the dbgp device

[Jason Wessel <jason.wessel@windriver.com>]

Alan Stern wrote:
> On Fri, 31 Jul 2009, Jason Wessel wrote:
>
>   
>> Alan Stern wrote:
>>     
>>> What happens across a system suspend?
>>>
>>>   
>>>       
>> I assume you mean as a test case:
>>
>> echo -n mem > /sys/power/state
>>
>> The dbgp device goes away entirely, when used with
>> earlyprintk=dbgp,keep.  But there is a way to fix it reasonably
>> easily.  Perhaps you might ack this resume patch, if you agree?
>>
>> Jason.
>>
>> ---
>> From: Jason Wessel <jason.wessel@windriver.com>
>> Subject: [PATCH] ehci-dbgp,ehci: Allow dbpg to work with suspend/resume
>>
>> In order for the dbgp driver to survive suspend/resume, on every ehci
>> resume operation the debug controller must get re-initialized.
>>
>> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
>> Cc: Greg KH <gregkh@suse.de>
>> Cc: Alan Stern <stern@rowland.harvard.edu>
>> Cc: dbrownell@users.sourceforge.net
>> Cc: Ingo Molnar <mingo@elte.hu>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Yinghai Lu <yinghai@kernel.org>
>> Cc: "Eric W. Biederman" <ebiederm@xmission.com>
>>
>> ---
>>  drivers/usb/host/ehci-hub.c |    6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> --- a/drivers/usb/host/ehci-hub.c
>> +++ b/drivers/usb/host/ehci-hub.c
>> @@ -204,6 +204,13 @@ static int ehci_bus_resume (struct usb_h
>>  		return -ESHUTDOWN;
>>  	}
>>  
>> +	if (unlikely(ehci->debug)) {
>> +		if (ehci->debug && !dbgp_reset_prep())
>> +			ehci->debug = NULL;
>> +		else
>> +			dbgp_external_startup();
>> +	}
>> +
>>  	/* Ideally and we've got a real resume here, and no port's power
>>  	 * was lost.  (For PCI, that means Vaux was maintained.)  But we
>>  	 * could instead be restoring a swsusp snapshot -- so that BIOS was
>>     
>
> It looks fine to me.  Would you perhaps want to make dbgp_reset_prep()
> automatically call dbgp_external_startup() on success so that ehci-hcd
> doesn't have to?
>   

Unfortunately the dbgp_reset_prep() and dbgp_external_startup() have to
be separate for the hcd case because the ehci controller reset comes in
between.  If you don't call dbgp_reset_prep() first, the ehci controller
can hang the system for the "ehci debug controller timeout death
period", or you can end up with undefined behavior if the ehci debug
controller is accessed while the reset code is being processed.

The dbgp_reset_prep() puts the ehci debug controller/driver into a safe
state until after the reset is done, and then the
dbgp_external_startup() re-initializes the ehci debug controller.

For the case of the power suspend, the ehci debug controller appeared to
be usable all the way to the time the power got yanked.  For the resume
operation, the debug controller driver sees that from the resume state
that the enable bit is off and doesn't try to use the device until
dbgp_reset_prep() and dbgp_external_reset() are called in sequence.

The dbgp_reset_prep() has to get called in resume to handle the case
where you remove the dbgp device while suspended and before you resumed
such that it won't hang the kernel.

Jason.