linux-kernel - Re: [PATCH] USB: initialize or shutdown PHY when add or remove host controller

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]

Date:	Tue, 18 Jun 2013 11:45:05 +0300
From:	Roger Quadros <rogerq@...com>
To:	<balbi@...com>
CC:	Chao Xie <chao.xie@...vell.com>, <gregkh@...uxfoundation.org>,
	<stern@...land.harvard.edu>, <linux-usb@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>, <xiechao.mail@...il.com>
Subject: Re: [PATCH] USB: initialize or shutdown PHY when add or remove host
 controller

On 06/18/2013 11:37 AM, Felipe Balbi wrote:
> Hi,
> 
> On Tue, Jun 18, 2013 at 11:34:08AM +0300, Roger Quadros wrote:
>>>>> On Tue, Jun 18, 2013 at 03:15:01AM -0400, Chao Xie wrote:
>>>>>> Some controller need software to initialize PHY before add
>>>>>> host controller, and shut down PHY after remove host controller.
>>>>>> Add the generic code for these controllers so they do not need
>>>>>> do it in its own host controller driver.
>>>>>>
>>>>>> Signed-off-by: Chao Xie <chao.xie@...vell.com>
>>>>>> ---
>>>>>>  drivers/usb/core/hcd.c |   19 ++++++++++++++++++-
>>>>>>  1 files changed, 18 insertions(+), 1 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>>>>>> index d53547d..b26196b 100644
>>>>>> --- a/drivers/usb/core/hcd.c
>>>>>> +++ b/drivers/usb/core/hcd.c
>>>>>> @@ -43,6 +43,7 @@
>>>>>>  
>>>>>>  #include <linux/usb.h>
>>>>>>  #include <linux/usb/hcd.h>
>>>>>> +#include <linux/usb/phy.h>
>>>>>>  
>>>>>>  #include "usb.h"
>>>>>>  
>>>>>> @@ -2531,12 +2532,22 @@ int usb_add_hcd(struct usb_hcd *hcd,
>>>>>>  	 */
>>>>>>  	set_bit(HCD_FLAG_RH_RUNNING, &hcd->flags);
>>>>>>  
>>>>>> +	/* Initialize the PHY before other hardware operation. */
>>>>>> +	if (hcd->phy) {
>>>>>
>>>>> this looks wrong for two reasons:
>>>>>
>>>>> a) you're not grabbing the PHY here.
>>>>>
>>>>> 	You can't just assume another entity grabbed your PHY for you.
>>>>
>>>> Isn't that done in the controller drivers e.g. ehci-fsl.c, ohci-omap, etc?
>>>
>>> right, and what I'm saying is that it should all be re-factored into
>>> ehci-hcd core :-)
>>>
>>>> If the controllers don't want HCD core to manage the PHY they can just set it
>>>> to some error code.
>>>
>>> they shouldn't have the choice, otherwise it'll be a bit of a PITA to
>>> maintain the code. ehci core tries to grab the PHY, if it's not there,
>>> try to continue anyway. Assume it's not needed.
>>>
>>
>> OK fine, but ehci-omap is a weird case as it needs a slightly different
>> sequence as to when PHY is initialized depending on which mode it is. (Transceiver
>> or transceiver-less). please see this fix.
>> http://www.spinics.net/lists/stable/msg12106.html
>>
>> All I'm saying as that ehci-omap needs a way to tell hcd core that it needs PHY
>> handling for itself.
> 
> why don't you do that always ? Meaning, why don't you *always* take PHY
> out of suspend ? If PHY is suspended, you can't wakeup unless you have
> (in OMAP case) pad wakeup working, right ?
> 

Maybe I wasn't clear before. This is nothing about wakeup and e always take PHY out of suspend. 
The problem is when to take it out of suspend relative to when EHCI controller starts.
Let me clarify.

In Transceiver mode we need this.

- bring phy out of reset
- start EHCI controller

Whereas for Transceiver-less mode we need this.

- start EHCI controller
- bring phy out of reset

If there is some way to signal this behaviour to the HCD core, it should be good enough.

cheers,
-roger
--
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/