Re: [PATCH v3 2/2] usb: dwc2: host: Clear interrupts before handling them

From: Antti SeppÃlÃ
Date: Thu Nov 19 2015 - 16:09:30 EST


On 19 November 2015 at 21:45, Douglas Anderson <dianders@xxxxxxxxxxxx> wrote:
> In general it is wise to clear interrupts before processing them. If
> you don't do that, you can get:
> 1. Interrupt happens
> 2. You look at system state and process interrupt
> 3. A new interrupt happens
> 4. You clear interrupt without processing it.
>
> This patch was actually a first attempt to fix missing device insertions
> as described in (usb: dwc2: host: Fix missing device insertions) and it
> did solve some of the signal bouncing problems but not all of
> them (which is why I submitted the other patch). Specifically, this
> patch itself would sometimes change:
> 1. hardware sees connect
> 2. hardware sees disconnect
> 3. hardware sees connect
> 4. dwc2_port_intr() - clears connect interrupt
> 5. dwc2_handle_common_intr() - calls dwc2_hcd_disconnect()
>
> ...to:
> 1. hardware sees connect
> 2. hardware sees disconnect
> 3. dwc2_port_intr() - clears connect interrupt
> 4. hardware sees connect
> 5. dwc2_handle_common_intr() - calls dwc2_hcd_disconnect()
>
> ...but with different timing then sometimes we'd still miss cable
> insertions.
>
> In any case, though this patch doesn't fix any (known) problems, it
> still seems wise as a general policy to clear interrupt before handling
> them.
>
> Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> Acked-by: John Youn <johnyoun@xxxxxxxxxxxx>
> Tested-by: John Youn <johnyoun@xxxxxxxxxxxx>
> ---
> Changes in v3:
> - Don't (uselessly) clear the PRTINT anymore (Felipe Balbi).
>
> Changes in v2: None
>

Hi.

It seems that towards the end of hcd_intr.c you seem to be switching a
few calls of dwc2_writel() to plain writel(). It looks like something
that is easily overlooked but please always use dwc2_writel to
preserve correct register endianness on big-endian platforms.

Br,
--
Antti
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/