Hello Alan,
On Wed, Oct 24, 2012 at 9:55 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> [Changed the Subject: line to something that makes more sense]
>
Sincere apologies for the mistake. Thanks for changing the subject line.
> On Wed, 24 Oct 2012, Mohan V wrote:
>
>> Hello All,
>>
>> We are working on OMAP4430 based board which has EHCI and OHCI ports and
>> are testing OHCI functionality. We are using kernel-3.4.
>>
>> When the USB bus is in suspend, a device connected to OHCI port does
>> not get detected, whereas the device connected to EHCI port is getting detected.
>
> Can you provide a dmesg log showing the OHCI problem with
> CONFIG_USB_DEBUG enabled?
>
Please find the attached log (ohci-detect.log), where the device is
disconnected from the
OHCI port and then does not get detected when connected again.
We are using android 3.0 kernel.
>> We are testing using the below commands:
>>
>> echo enabled > /sys/bus/usb/devices/usb2/power/wakeup
>> echo enabled > /sys/bus/usb/devices/2-2/power/wakeup
>
> The wakeup settings are irrelevant here because you are not putting
> your system to sleep.
>
>> echo auto > /sys/bus/usb/devices/usb2/power/control
>> echo auto > /sys/bus/usb/devices/2-2/power/control
>
> What is the 2-2 device? Is it a hub? If it is, do you plug the new
> device into the hub or directly into the OHCI controller?
>
The 2-2 device is the USB mouse connected to OHCI port. The device is connected
directly to OHCI port.
>> We see that after making the below change in "ohci_irq", the device
>> gets detected.
>> ---------------------------------------------------------------------------------------------------
>> --- a/drivers/usb/host/ohci-hcd.c
>> +++ b/drivers/usb/host/ohci-hcd.c
>> @@ -819,7 +819,7 @@ static irqreturn_t ohci_irq (struct usb_hcd *hcd)
>> * to turn on RHSC along with RD. But for remote wakeup events
>> * this might not happen.
>> */
>> - else if (ints & OHCI_INTR_RD) {
>> + if (ints & OHCI_INTR_RD) {
>> ---------------------------------------------------------------------------------------------------
>> Even in the OHCI specification (Sec 5.3 page 80), it is recommended to
>> check individual bits
>> (RHSC and RD).
>
> The driver does check the individual bits. (And are you sure you don't
> mean page 82?)
>
Yes, I meant page 82.
>> Can you please let us know if this change is valid and will not cause
>> any regression?
>
> Yes, it is likely to cause a regression. To understand why, you should
> read the original commit that added the code you want to change (commit
> 583ceada075597a5b6acab1140d61ac81586a2a6, USB: OHCI: fix root-hub
> resume bug). In short, your proposed change would cause the driver to
> try to resume the root hub twice.
>
> More to the point, why doesn't the current code work? It appears that
> the OHCI_INTR_RHCS bit is set, so the handler should call
> usb_hcd_poll_rh_status(), which in turn calls ohci_hub_status_data(),
> which calls ohci_root_hub_state_changes(), which should call
> usb_hcd_resume_root_hub(). If this isn't happening, maybe you can add
> some debugging statements to find out what's going wrong.
>
We sometime see the crash in "ohci_hub_status_data" function. So, when
we add the
below check at the entry of the function, there is no crash. Please
find attached crash log.
(ohci-crash.log)
if (!HC_IS_RUNNING(hcd->state)) {
return 0;
}
But the device does not get detected subsequently with this change. There is
a similar check in ehci-hub.c.
Please let us know your comments.
Thanks,
Mohan
Attachment:
ohci-detect.log
Description: Binary data
Attachment:
ohci-crash.log
Description: Binary data
