linux-kernel - Re: [PATCH 1/2] usb/ehci-hub: Add workaround for EG20T/ML7213/ML7223/ML7831

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] [day] [month] [year] [list]

Date:	Wed, 18 Apr 2012 19:22:43 +0900
From:	Tomoya MORINAGA <tomoya.rohm@...il.com>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	qi.wang@...el.com, yong.y.wang@...el.com, joel.clark@...el.com,
	kok.howg.ewe@...el.com
Subject: Re: [PATCH 1/2] usb/ehci-hub: Add workaround for EG20T/ML7213/ML7223/ML7831

On Tue, Apr 10, 2012 at 11:57 PM, Alan Stern <stern@...land.harvard.edu> wrote:
> On Tue, 10 Apr 2012, Tomoya MORINAGA wrote:
>
>> ISSUE: When a bit of EHCI status register (USBSTS) is set
>> as 1, if the corresponded bit of EHCI interrupt enable
>> register(USBINTR) is set as 1, an interrupt occurs.
>> After that, even if the bit of USBINTR is set as 0, the
>> interrupt continues occurring.
>> Workaround: Clear the bit 3 of USBSTS before enabling the
>> interrupt, at resume process.
>
> These two patches do a lot more than is necessary.  At the same time,
> they also seem to do a lot less.
>
> You don't have to check for the faulty product IDs; clearing the
> STS_FLR bit (bit 3 of USBSTS) is always okay because the driver doesn't
> use it.
>
> On the other hand, the STS_FLR bit will get set once every second (at
> least) whenever the controller is running.  According to your
> description above, this means the controller will cause an interrupt
> storm as soon as its first interrupt occurs.  What you really need to
> do is clear STS_FLR in ehci_irq().
>
> Does the patch below fix your problem?
>
> Alan Stern
>
>
>
>  drivers/usb/host/ehci-hcd.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> Index: usb-3.4/drivers/usb/host/ehci-hcd.c
> ===================================================================
> --- usb-3.4.orig/drivers/usb/host/ehci-hcd.c
> +++ usb-3.4/drivers/usb/host/ehci-hcd.c
> @@ -860,8 +860,13 @@ static irqreturn_t ehci_irq (struct usb_
>                goto dead;
>        }
>
> +       /*
> +        * We don't use STS_FLR, but some controllers don't like it to
> +        * remain on, so mask it out along with the other status bits.
> +        */
> +       masked_status = status & (INTR_MASK | STS_FLR);
> +
>        /* Shared IRQ? */
> -       masked_status = status & INTR_MASK;
>        if (!masked_status || unlikely(ehci->rh_state == EHCI_RH_HALTED)) {
>                spin_unlock(&ehci->lock);
>                return IRQ_NONE;
>

Your suggestion is very good for us.
Your patch fixes our issue. I confirmed your patch worked well on our platform.
I hope for that the patch is applied.
Thanks a lot.

-- 
ROHM Co., Ltd.
tomoya
--
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/