diff mbox

[2/7] usb: hub: Conditionally power on usb's root-hub ports

Message ID 1364376543-7526-3-git-send-email-gautam.vivek@samsung.com
State New
Headers show

Commit Message

Vivek Gautam March 27, 2013, 9:28 a.m. UTC 
Power on root hubs' ports only when they are not yet powered on.
Its seen with USB 3.0 ports that they are powered on after
a H/W reset, as also reflected in XHCI spec (sec 4.3):
"After a Chip Hardware Reset, HCRST, or commanded to the
PLS = RxDetect state, all Root Hub ports shall be in Disconnected
state, i.e. the port is powered on (PP = 1)"

Signed-off-by: Amar <amarendra.xt@samsung.com>
Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
 common/usb_hub.c |   22 +++++++++++++++++++---
 1 files changed, 19 insertions(+), 3 deletions(-)

Comments

Marek Vasut March 28, 2013, 2:28 p.m. UTC | #1 
Dear Vivek Gautam,

> Power on root hubs' ports only when they are not yet powered on.
> Its seen with USB 3.0 ports that they are powered on after
> a H/W reset, as also reflected in XHCI spec (sec 4.3):
> "After a Chip Hardware Reset, HCRST, or commanded to the
> PLS = RxDetect state, all Root Hub ports shall be in Disconnected
> state, i.e. the port is powered on (PP = 1)"
> 
> Signed-off-by: Amar <amarendra.xt@samsung.com>
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>

Just wondering if we shouldn't power-cycle all the ports instead -- aka. turn 
them off and then turn all of them on again.

Best regards,
Marek Vasut
Vivek Gautam April 2, 2013, 9:36 a.m. UTC | #2 
Hi Marek,

Adding CC: Amarendra

On Thu, Mar 28, 2013 at 7:58 PM, Marek Vasut <marex@denx.de> wrote:
> Dear Vivek Gautam,
>
>> Power on root hubs' ports only when they are not yet powered on.
>> Its seen with USB 3.0 ports that they are powered on after
>> a H/W reset, as also reflected in XHCI spec (sec 4.3):
>> "After a Chip Hardware Reset, HCRST, or commanded to the
>> PLS = RxDetect state, all Root Hub ports shall be in Disconnected
>> state, i.e. the port is powered on (PP = 1)"
>>
>> Signed-off-by: Amar <amarendra.xt@samsung.com>
>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>
> Just wondering if we shouldn't power-cycle all the ports instead -- aka. turn
> them off and then turn all of them on again.

Hmm, seems like a fine idea.
Will it be fine to power-cycle even EHCI ports too ?
Or we just check if ports are already powered-up (as in XHCI) we
power-cycle them.
Marek Vasut April 2, 2013, 10:09 a.m. UTC | #3 
Dear Vivek Gautam,

> Hi Marek,
> 
> Adding CC: Amarendra
> 
> On Thu, Mar 28, 2013 at 7:58 PM, Marek Vasut <marex@denx.de> wrote:
> > Dear Vivek Gautam,
> > 
> >> Power on root hubs' ports only when they are not yet powered on.
> >> Its seen with USB 3.0 ports that they are powered on after
> >> a H/W reset, as also reflected in XHCI spec (sec 4.3):
> >> "After a Chip Hardware Reset, HCRST, or commanded to the
> >> PLS = RxDetect state, all Root Hub ports shall be in Disconnected
> >> state, i.e. the port is powered on (PP = 1)"
> >> 
> >> Signed-off-by: Amar <amarendra.xt@samsung.com>
> >> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> > 
> > Just wondering if we shouldn't power-cycle all the ports instead -- aka.
> > turn them off and then turn all of them on again.
> 
> Hmm, seems like a fine idea.
> Will it be fine to power-cycle even EHCI ports too ?
> Or we just check if ports are already powered-up (as in XHCI) we
> power-cycle them.

EHCI as well, all of them without differentiating the type. Why not , do you see 
a problem ?

Best regards,
Marek Vasut
Vivek Gautam April 2, 2013, 10:29 a.m. UTC | #4 
Hi,


On Tue, Apr 2, 2013 at 3:39 PM, Marek Vasut <marex@denx.de> wrote:
> Dear Vivek Gautam,
>
>> Hi Marek,
>>
>> Adding CC: Amarendra
>>
>> On Thu, Mar 28, 2013 at 7:58 PM, Marek Vasut <marex@denx.de> wrote:
>> > Dear Vivek Gautam,
>> >
>> >> Power on root hubs' ports only when they are not yet powered on.
>> >> Its seen with USB 3.0 ports that they are powered on after
>> >> a H/W reset, as also reflected in XHCI spec (sec 4.3):
>> >> "After a Chip Hardware Reset, HCRST, or commanded to the
>> >> PLS = RxDetect state, all Root Hub ports shall be in Disconnected
>> >> state, i.e. the port is powered on (PP = 1)"
>> >>
>> >> Signed-off-by: Amar <amarendra.xt@samsung.com>
>> >> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>> >
>> > Just wondering if we shouldn't power-cycle all the ports instead -- aka.
>> > turn them off and then turn all of them on again.
>>
>> Hmm, seems like a fine idea.
>> Will it be fine to power-cycle even EHCI ports too ?
>> Or we just check if ports are already powered-up (as in XHCI) we
>> power-cycle them.
>
> EHCI as well, all of them without differentiating the type. Why not , do you see
> a problem ?

Not of course, will amend this accordingly.
Marek Vasut April 2, 2013, 10:33 a.m. UTC | #5 
Dear Vivek Gautam,

> Hi,
> 
> On Tue, Apr 2, 2013 at 3:39 PM, Marek Vasut <marex@denx.de> wrote:
> > Dear Vivek Gautam,
> > 
> >> Hi Marek,
> >> 
> >> Adding CC: Amarendra
> >> 
> >> On Thu, Mar 28, 2013 at 7:58 PM, Marek Vasut <marex@denx.de> wrote:
> >> > Dear Vivek Gautam,
> >> > 
> >> >> Power on root hubs' ports only when they are not yet powered on.
> >> >> Its seen with USB 3.0 ports that they are powered on after
> >> >> a H/W reset, as also reflected in XHCI spec (sec 4.3):
> >> >> "After a Chip Hardware Reset, HCRST, or commanded to the
> >> >> PLS = RxDetect state, all Root Hub ports shall be in Disconnected
> >> >> state, i.e. the port is powered on (PP = 1)"
> >> >> 
> >> >> Signed-off-by: Amar <amarendra.xt@samsung.com>
> >> >> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> >> > 
> >> > Just wondering if we shouldn't power-cycle all the ports instead --
> >> > aka. turn them off and then turn all of them on again.
> >> 
> >> Hmm, seems like a fine idea.
> >> Will it be fine to power-cycle even EHCI ports too ?
> >> Or we just check if ports are already powered-up (as in XHCI) we
> >> power-cycle them.
> > 
> > EHCI as well, all of them without differentiating the type. Why not , do
> > you see a problem ?
> 
> Not of course, will amend this accordingly.

Sounds great! Thank you!

Best regards,
Marek Vasut
diff mbox

Patch

diff --git a/common/usb_hub.c b/common/usb_hub.c
index b5eeb62..0677004 100644
--- a/common/usb_hub.c
+++ b/common/usb_hub.c
@@ -111,13 +111,29 @@  static void usb_hub_power_on(struct usb_hub_device *hub)
 	int i;
 	struct usb_device *dev;
 	unsigned pgood_delay = hub->desc.bPwrOn2PwrGood * 2;
+	ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1);
+	unsigned short portstatus;
+	int ret;
 
 	dev = hub->pusb_dev;
-	/* Enable power to the ports */
+
+	/* Enable power to ports whose power is not yet on */
 	USB_HUB_PRINTF("enabling power on all ports\n");
 	for (i = 0; i < dev->maxchild; i++) {
-		usb_set_port_feature(dev, i + 1, USB_PORT_FEAT_POWER);
-		USB_HUB_PRINTF("port %d returns %lX\n", i + 1, dev->status);
+		ret = usb_get_port_status(dev, i + 1, portsts);
+		if (ret < 0) {
+			USB_HUB_PRINTF("port %d: get_port_status failed\n",
+					i + 1);
+			return;
+		}
+
+		portstatus = le16_to_cpu(portsts->wPortStatus);
+
+		if (!(portstatus & (USB_PORT_STAT_POWER << 1))) {
+			usb_set_port_feature(dev, i + 1, USB_PORT_FEAT_POWER);
+			USB_HUB_PRINTF("port %d returns %lX\n", i + 1,
+					dev->status);
+		}
 	}
 
 	/* Wait at least 100 msec for power to become stable */