On Thu, 16 Apr 2009, yao yong wrote:
> Hi, Alan,
>
> Please see my comments:
>
> 2009/4/15 Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>:
> > On Wed, 15 Apr 2009, Eric Miao wrote:
> >
> >> From a0e1b72f073f4b3dd811b6c4fe4cae4044e56d45 Mon Sep 17 00:00:00 2001
> >> From: Yong Yao <yaoyong@xxxxxxxxxxx>
> >> Date: Wed, 15 Apr 2009 13:08:56 +0800
> >> Subject: [PATCH] usb core: HNP support in hub.c
> >>
> >> When starting HNP, we need reset the port, so USB_PORT_STAT_ENABLE is
> >> set. In such case, we should not disable the port, otherwise the port
> >> will be disconnected. It will cause that HNP to fail, since the state
> >> machine will change from OTG_STATE_B_HOST to OTG_STATE_B_PERIPHERAL
> >> after the port is disconnected.
> >>
> >> Signed-off-by: Yong Yao <yaoyong@xxxxxxxxxxx>
> >> Signed-off-by: Eric Miao <eric.miao@xxxxxxxxxxx>
> >> ---
> >>
> >> Alan, this is a follow-up of the previous thread, with the fix
> >> to be OTG specific, please help review.
> >>
> >> drivers/usb/core/hub.c | 7 +++++--
> >> 1 files changed, 5 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> >> index b19cbfc..2e28f48 100644
> >> --- a/drivers/usb/core/hub.c
> >> +++ b/drivers/usb/core/hub.c
> >> @@ -688,8 +688,11 @@ static void hub_activate(struct usb_hub *hub,
> >> enum hub_activation_type type)
> >> * Unconnected ports should likewise be disabled (paranoia),
> >> * and so should ports for which we have no usb_device.
> >> */
> >> - if ((portstatus & USB_PORT_STAT_ENABLE) && (
> >> - type != HUB_RESUME ||
> >> + if ((portstatus & USB_PORT_STAT_ENABLE) &&
> >> +#ifdef CONFIG_USB_OTG
> >> + !(hdev->bus->is_b_host) &&
> >> +#endif
> >> + (type != HUB_RESUME ||
> >> !(portstatus & USB_PORT_STAT_CONNECTION) ||
> >> !udev ||
> >> udev->state == USB_STATE_NOTATTACHED)) {
> >
> > This is a lot better than the previous patch. I wonder if there is a
> > simple way to eliminate the ugly preprocessor test. For instance, if
> > CONFIG_USB_OTG isn't defined, won't hdev->bus->is_b_host always be 0?
> >
> [Yong] As I know, only OTG device will change the is_b_host. But I
> think we may reserve this preprecessor in order to align with the
> previous code.
> > I also wonder if this is truly correct. Evidently you want the new
> > behavior to apply during the HNP exchange, but what about at other
> > times? Suppose the device is already a B-host when its root hub gets
> > suspended; what would happen then?
> [Yong] when root hub gets suspended, port will disconnect. In such
> case, otg state machine will change from OTG_STATE_B_HOST to
> OTG_STATE_B_PERIPHERAL. At that time, the B device will be back as the
> peripheral. we don't use host port any more.
Okay, then this patch is basically all right.
Let's see what David thinks about the #ifdef. Adding the extra test
when CONFIG_USB_OTG is off won't hurt anything.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
