Hi Felipe,
 
 On lun., d?c. 07 2015, Felipe Balbi [off-list ref] wrote:

Hi,

Gregory CLEMENT [off-list ref] writes:

quoted

Hi Felipe,

I am going back on this subject (again :) )
 
 On mar., oct. 20 2015, Gregory CLEMENT [off-list ref] wrote:

quoted

Hi Felipe,
 
 On lun., oct. 05 2015, Felipe Balbi [off-list ref] wrote:

quoted

quoted

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!

quoted

quoted

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.

quoted

I go back on this because, your suggestion didn't work. At least, I
didn't manage to make it improve the situation.

Thanks,

quoted

quoted

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.

quoted

quoted

        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 :-)

quoted

quoted

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.

quoted

quoted

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
simultaneously."

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

Gregory

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.
http://free-electrons.com