On Thu, Oct 24, 2013 at 12:09:03PM -0400, Alan Stern wrote: > On Thu, 24 Oct 2013, [iso-8859-2] J�nosi Zoli wrote: > > > https://bugzilla.kernel.org/show_bug.cgi?id=63611 > > � > > Bug ID: 63611 > > Summary: cant connect sony phones in mass storage mode > > Is CONFIG_USB_OTG turned on in your kernel's .config? If it is, try > building a new kernel with it turned off.
Sorry for the long delay here. This got lost in my maildir for whatever
reason.
> Felipe:
>
> Is the usb_enumerate_device_otg() routine really correct? It looks
> like it will try to enable HNP whenever CONFIG_USB_OTG is set, even if
> the root hub isn't OTG-capable.
>
> The routine does this:
>
> /* enable HNP before suspend, it's simpler */
> if (port1 == bus->otg_port)
> bus->b_hnp_enable = 1;
> err = usb_control_msg(udev,
> usb_sndctrlpipe(udev, 0),
> USB_REQ_SET_FEATURE, 0,
> bus->b_hnp_enable
> ? USB_DEVICE_B_HNP_ENABLE
> : USB_DEVICE_A_ALT_HNP_SUPPORT,
> 0, NULL, 0, USB_CTRL_SET_TIMEOUT);
> if (err < 0) {
> /* OTG MESSAGE: report errors here,
> * customize to match your product.
> */
> dev_info(&udev->dev,
> "can't set HNP mode: %d\n",
> err);
> bus->b_hnp_enable = 0;
> }
>
> Maybe the usb_control_msg and the error check should be inside the
> first "if" statement?
I don't think so, see below:
| static int usb_enumerate_device_otg(struct usb_device *udev)
| {
| int err = 0;
|
| #ifdef CONFIG_USB_OTG
first, we know OTG is enabled.
| /*
| * OTG-aware devices on OTG-capable root hubs may be able to use SRP,
| * to wake us after we've powered off VBUS; and HNP, switching roles
| * "host" to "peripheral". The OTG descriptor helps figure this out.
| */
| if (!udev->bus->is_b_host
| && udev->config
| && udev->parent == udev->bus->root_hub) {
| struct usb_otg_descriptor *desc = NULL;
| struct usb_bus *bus = udev->bus;
|
| /* descriptor may appear anywhere in config */
| if (__usb_get_extra_descriptor (udev->rawdescriptors[0],
|
le16_to_cpu(udev->config[0].desc.wTotalLength),
| USB_DT_OTG, (void **) &desc) == 0) {
| if (desc->bmAttributes & USB_OTG_HNP) {
this check ensures the port supports HNP
| unsigned port1 = udev->portnum;
|
| dev_info(&udev->dev,
| "Dual-Role OTG device on %sHNP port\n",
| (port1 == bus->otg_port)
| ? "" : "non-");
|
| /* enable HNP before suspend, it's simpler */
| if (port1 == bus->otg_port)
| bus->b_hnp_enable = 1;
this check tells is just to help you choose between enabling HNP on a B
device, or telling the device that another root hub port supports HNP
| err = usb_control_msg(udev,
| usb_sndctrlpipe(udev, 0),
| USB_REQ_SET_FEATURE, 0,
| bus->b_hnp_enable
| ? USB_DEVICE_B_HNP_ENABLE
| : USB_DEVICE_A_ALT_HNP_SUPPORT,
so, based on this, we're just enabling HNP on the device, but HNP won't
kick in until the root hub port is suspended.
| 0, NULL, 0, USB_CTRL_SET_TIMEOUT);
| if (err < 0) {
| /* OTG MESSAGE: report errors here,
| * customize to match your product.
| */
| dev_info(&udev->dev,
| "can't set HNP mode: %d\n",
| err);
| bus->b_hnp_enable = 0;
| }
| }
| }
| }
|
| if (!is_targeted(udev)) {
and, BTW, this needs to change. We shouldn't have a TPL *in* the kernel
as that hinders the possibility of allowing USB Accessory manufactures
to ship their driver in an App Store of some sort. Basically,
is_targeted() should return false only after we know the device doesn't
bind to any of our USB drivers. But that's for future, as that'd be a
*really* invasive change to usbcore.
hope this helps.
--
balbi
signature.asc
Description: Digital signature
