Carlo,

Please explain how the over current condition is examined.
Please also post the USB portion of the schematics.

Hi Bin ,

Issue is not cosed  , I m waiting customer currents and be back asap

please could you suggest anything form schemtics ? plus same configuration was perfect on DM3730  ( many 10Ks sold)

thank you

regrads

Carlo

Hi Carlo,

I think to have resolved this issue by the following patch which deals with USB_PORT_STAT_C_OVERCURRENT event at the "hub level" of the driver (Kernel 4.9.45-rt23).

Otherwise with the current driver this event (at the "port level) is discharged.

Alessandro

--
drivers/usb/core/hub.c | 11 ++++++++++++++

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 80d4ef3..ef4f1f7 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -5167,6 +5168,19 @@ static void hub_event(struct work_struct *work)
for (i = 1; i <= hdev->maxchild; i++) {
struct usb_port *port_dev = hub->ports[i - 1];

+ u16 portstatus, portchange;
+ if( hub_port_status(hub, i, &portstatus, &portchange) == 0)
+ {
+ if (portchange & USB_PORT_STAT_C_OVERCURRENT)
+ {
+ usb_clear_port_feature(hdev, i,
+ USB_PORT_FEAT_C_OVER_CURRENT);
+ msleep(100); /* Cool down */
+ hub_power_on(hub, true);
+ }
+ }
if (test_bit(i, hub->event_bits)
|| test_bit(i, hub->change_bits)
|| test_bit(i, hub->wakeup_bits)) {

Hi Bin Liu, I did some debug and what happens is that when an Overcurrent occurs (case A) no hub->event_bits or
hub->change_bits is raised.

---

drivers/usb/core/hub.c:

static void hub_event(struct work_struct *work)
{

...
if (test_bit(i, hub->event_bits)
|| test_bit(i, hub->change_bits)
|| test_bit(i, hub->wakeup_bits)) {
/*
* The get_noresume and barrier ensure that if
* the port was in the process of resuming, we
* flush that work and keep the port active for
* the duration of the port_event(). However,
* if the port is runtime pm suspended
* (powered-off), we leave it in that state, run
* an abbreviated port_event(), and move on.
*/
pm_runtime_get_noresume(&port_dev->dev);
pm_runtime_barrier(&port_dev->dev);
usb_lock_port(port_dev);
port_event(hub, i);
usb_unlock_port(port_dev);
pm_runtime_put_sync(&port_dev->dev);
}
---

so port_event() is never called and consequently hub_port_status() is not enquired so that
not reading wPortChange the C_PORT_OVER_CURRENT is not handled.
The reason why event_bits or change_bits don't change their status it is obscure to me,
this should be done at irq level anyway ... May be it depends on DCW3 IP ?

I would like to find an explanation to this behavior in order to be sure of my patch.

Thanks

to

Hi Bin Liu,

Kernel 2.6.32 's hub.c (the one working in the  DM3730 platform) is quite different from Kernel 4.9 rt 's one but there is in both a test on hub->change_bits and only in the former it succeeds when

an Overcurrent occurs. I think that a big difference may be that the latter is a Real Time kernel so interrupts are treated differently (as threads) but I can't prove it though. What do you think about it ?

I wrapped my patch with "pm_runtime_*() and usb_lock_port() calls" as you suggested.

Alessandro Antenucci said:

- (portstatus & USB_PORT_STAT_OVERCURRENT))
+ (portstatus & USB_PORT_STAT_OVERCURRENT) || (portchange && USB_PORT_STAT_C_OVERCURRENT))

Should the new condition be "(portchange & USB_PORT_STAT_C_OVERCURRENT)"?

Alessandro,

I think this is the right fix. But I have posted your information to the kernel community mailing list to confirm.

By the way, I just read the v3.2 kernel code and understood why your system worked in v3.2.

hub_events() in v3.2 kernel checks for either hub->event_bits or hub->change_bits, then queries for port status from the hub, and handles the over-current condition.

3514                         connect_change = test_bit(i, hub->change_bits);         
3515                         if (!test_and_clear_bit(i, hub->event_bits) &&          
3516                                         !connect_change)                        
3517                                 continue;                                       
3518                                                                                 
3519                         ret = hub_port_status(hub, i,                           
3520                                         &portstatus, &portchange);
...
3582                         if (portchange & USB_PORT_STAT_C_OVERCURRENT) {

Alessandro,

I got a confirmation from the kernel mailing list that your patch is the right thing to do (of cause with a single & in the new condition).

Do you want to send the patch directly to linux-usb@vger.kernel.org so that you get the credit? Or I can send the patch and add your name and email address as Reported-By?

Hi Bin Liu,

 I'm very happy about it ! You can send the patch on my behalf (it's for sure the fastest thing to do), my email is antenucci@korg.it.

 Thank you a lot for your support,

 Alessandro

Actually in the 2nd option adding 'Tested-by: Alessandro Antenucci <antenucci@korg.it>' would be better.

Hi Bin Liu,

 it's ok if you create and send the patch (2nd option).

 Thanks,

Alessandro