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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKv9HNbtQjnOzctNH9qVy97KZCW-W0M+s+vZAt0oRDv7KUtsUw@mail.gmail.com>
Date:	Thu, 19 Nov 2015 23:09:25 +0200
From:	Antti Seppälä <a.seppala@...il.com>
To:	Douglas Anderson <dianders@...omium.org>
Cc:	John Youn <John.Youn@...opsys.com>, balbi@...com,
	Yunzhi Li <lyz@...k-chips.com>,
	Heiko Stübner <heiko@...ech.de>,
	linux-rockchip@...ts.infradead.org,
	Julius Werner <jwerner@...omium.org>,
	gregory.herrero@...el.com, yousaf.kaukab@...el.com,
	dinguyen@...nsource.altera.com, johnyoun@...opsys.com,
	gregkh@...uxfoundation.org, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/2] usb: dwc2: host: Clear interrupts before handling them

On 19 November 2015 at 21:45, Douglas Anderson <dianders@...omium.org> 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@...omium.org>
> Acked-by: John Youn <johnyoun@...opsys.com>
> Tested-by: John Youn <johnyoun@...opsys.com>
> ---
> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/