On Fri, 20 Oct 2017, Uwe Kleine-König wrote:
> ehci_hub_control does the following related to overcurrent handling
> (simplified):
>
> temp = ehci_readl(ehci, status_reg);
>
> if (temp & PORT_OCC) {
> status |= USB_PORT_STAT_C_OVERCURRENT << 16;
>
> if (temp & PORT_OC) {
> ehci_port_power(ehci, wIndex, false);
> temp = ehci_readl(ehci, status_reg);
> }
> }
>
> if (temp & PORT_OC)
> status |= USB_PORT_STAT_OVERCURRENT;
>
> At least on my i.MX25 based machine rereading the PORTSC register after
> port power was removed doesn't yield PORT_OC any more. The result is
> that repeatedly changes related to over-current
> (status |= USB_PORT_STAT_C_OVERCURRENT << 16) are reported, but
> USB_PORT_STAT_OVERCURRENT doesn't toggle as expected and is always
> unset. As a side effect the message
>
> dev_err(&port_dev->dev, "over-current condition\n");
>
> from drivers/usb/core/hub.c's port_event() never makes it to the user.
>
> Rereading the value was introduced with the rationale:
>
> I also think that the fact that we've cut off the port power
> should be reflected in the result of GetPortStatus request
> immediately, hence I'm adding a PORTSCn register readback after
> write...
>
> As there are no further expected changes, just mask the PORT_POWER bit
> instead of rereading the register.
>
> An alternative would be to check for PORT_OC before checking for
> PORT_OCC.
>
> Fixes: 81463c1d7071 ("EHCI: only power off port if over-current is active")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> ---
> drivers/usb/host/ehci-hub.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
> index df169c8e7225..08f59654654b 100644
> --- a/drivers/usb/host/ehci-hub.c
> +++ b/drivers/usb/host/ehci-hub.c
> @@ -1031,7 +1031,7 @@ int ehci_hub_control(
> spin_unlock_irqrestore(&ehci->lock, flags);
> ehci_port_power(ehci, wIndex, false);
> spin_lock_irqsave(&ehci->lock, flags);
> - temp = ehci_readl(ehci, status_reg);
> + temp |= ~PORT_POWER;
> }
> }
>
Well, I don't know. The USB spec says that if port power is off then a
lot of other port status bits have to be off also. But the code tries
to have it both ways -- some of the status bits are recorded from
before the port power is turned off and some of them from afterward.
Maybe my original suggestion wasn't so good, and it would be better
simply to remove this line entirely. Then the reported port status
would just be the values before power was removed, with no
inconsistencies. Can you try doing that instead and see if it works
equally well?
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
