[PATCH] usb: musb: dsps: handle the otg_state_a_wait_vrise_timeout case

Gregory CLEMENT gregory.clement at free-electrons.com
Tue Dec 8 01:07:58 PST 2015

Hi Felipe,
 On lun., déc. 07 2015, Felipe Balbi <balbi at ti.com> wrote:

> Hi,
> Gregory CLEMENT <gregory.clement at free-electrons.com> writes:
>> Hi Felipe,
>> I am going back on this subject (again :) )
>>  On mar., oct. 20 2015, Gregory CLEMENT <gregory.clement at free-electrons.com> wrote:
>>> Hi Felipe,
>>>  On lun., oct. 05 2015, Felipe Balbi <balbi at ti.com> wrote:
>>>>> So after many tests on different devices, 200ms is enough for most of
>>>>> them, but for one, 2000ms (2s) was needed!
>>>>> I see several option:
>>>>> - adding a sysfs entry to setup the time
>>>>> - adding a debugs entry entry
>>>>> - adding configuration option in menuconfig
>>>>> - using 2000ms and hopping it was enough for everyone
>>>>> My preference would go to the first option, what is your opinion?
>>>> I'm not sure if either of them is good. man 2s is just too large. If the
>> Well 2s is lon I agree, but currently instead of 2s we have an infinite
>> wait, which is worse!
>>>> device isn't following the specification, I'm afraid that device needs
>>>> to be fixed.
>> I agree but these devices are for most of them USB stick that we find in
>> retail. We have no influence on them, so we have to do with them, even
>> if they do not follow the sepc.
>> So what about using configfs or sysfs to let the user confgiure this
>> value if needed?
> iff we have to; I'm still not 100% convinced.
>> I go back on this because, your suggestion didn't work. At least, I
>> didn't manage to make it improve the situation.
>> Thanks,
>>>> I think the real issue here is the lack of a disconnect IRQ from AM335x
>>>> devices. But here's something I've been meaning to test but never had
>>>> time. If you look into AM335x address space, there's a bit in the
>>>> wrapper which tells it to use the standard MUSB registers for IRQ,
>>>> instead of the TI-specific thing. Can you see if we get a disconnect IRQ
>>>> with that ?
>>>> TRM is at [1], see page 2566. Basically, if you set bit 3 in register
>>>> offset 0x1014, then it should use Mentor IRQ registers. If that works,
>>>> quite a few problems will actually vanish :-p
>>> So I tried it with the following patch:
>>> -------------------------------------
>>> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
>>> index c791ba5..c714500 100644
>>> --- a/drivers/usb/musb/musb_dsps.c
>>> +++ b/drivers/usb/musb/musb_dsps.c
>>> @@ -470,6 +470,7 @@ static int dsps_musb_init(struct musb *musb)
>>>         /* Reset the musb */
>>>         dsps_writel(reg_base, wrp->control, (1 << wrp->reset));
>>> +       dsps_writel(musb->ctrl_base, wrp->control, BIT(3));
> overwritting reset.
>>>         musb->isr = dsps_interrupt;
>>> @@ -625,6 +626,7 @@ static int dsps_musb_reset(struct musb *musb)
>>>         if (session_restart || !glue->sw_babble_enabled) {
>>>                 dev_info(musb->controller, "Restarting MUSB to recover from Babble\n");
>>>                 dsps_writel(musb->ctrl_base, wrp->control, (1 << wrp->reset));
>>> +               dsps_writel(musb->ctrl_base, wrp->control, BIT(3));
> here too. No wonder it won't work, right :-)
>>> I am not very familiar with that driver but my understanding was that
>>> the Mentor IRQ registeres are managed by the musb_interrupt() function
>>> which is called from the dsps_interrupt() interrupt handler.
>>> Am I right?
> yeah, however the way IRQs are reported is a different thing. AM335x
> introduced its own IRQ reporting scheme which, basically, reads MUSB
> generic IRQ status registers and reports on an AM335x specific
> register. No idea why TI did that for AM335x devices.
>>> if it is the case then it didn't fix the issue I had.
>>> I activated the following debug line:
>>> [musb_hdrc]musb_interrupt =_ "** IRQ %s usb%04x tx%04x rx%04x\012"
>>> [musb_dsps]dsps_interrupt =p "usbintr (%x) epintr(%x)\012"
>>> But I didn't get any interrupt while disconnecting the cable without any
>>> device connected on it (whereas I got an interrupt when I connected it).
>>> Note that I applied this patch instead of the "usb: musb: dsps: handle
>>> the otg_state_a_wait_vrise_timeout case", is what you had in mind ?
> yeah, that's what I had in mind. But your patch seems wrong :-)
> I tried writing a more correct version here and found 2 issues:
> a) bit 3 doesn't do anything :-p I cannot read IRQs from mentor's
> registers
> b) when setting RESET_ISOLATION bit, reads of CTRL register hang. Note
> that according to TRM, RESET_ISOLATION _must_ be set prior to a soft
> reset and cleared afterwards. But right after setting RESET_ISOLATION,
> if I try a read of CTRL, it'll hang forever.

The datasheet seems not very coherent about it,

on one side we have:
"This bit should be set high prior to setting bit 0 and cleared after bit 0
is cleared."

and on the other side:
"Both the soft_reset and soft_reset_isolation bits should be asserted

The hang you saw could be explained by the following:
"Setting only the soft_reset_isolation bit will cause all USB0 output
signals to go to a known constant value via multiplexers.
This will
prevent future access to USB0."  page 2567


> Bin, do you know about these problems ? They both seem rather alarming
> to me.
> -- 
> balbi

Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.

More information about the linux-arm-kernel mailing list