Bug 7482
| Summary: | usb ehci driver crash on system with more tahn 256MByte memory | ||
|---|---|---|---|
| Product: | Drivers | Reporter: | Stefan Meyer (reyems) |
| Component: | USB | Assignee: | Greg Kroah-Hartman (greg) |
| Status: | RESOLVED CODE_FIX | ||
| Severity: | high | ||
| Priority: | P2 | ||
| Hardware: | i386 | ||
| OS: | Linux | ||
| Kernel Version: | 2.6.18.2 | Subsystem: | |
| Regression: | --- | Bisected commit-id: | |
|
Description
Stefan Meyer
2006-11-09 22:00:36 UTC
On Thu, 9 Nov 2006 22:01:21 -0800 bugme-daemon@bugzilla.kernel.org wrote: > http://bugzilla.kernel.org/show_bug.cgi?id=7482 > > Summary: usb ehci driver crash on system with more tahn 256MByte > memory > Kernel Version: 2.6.18.2 > Status: NEW > Severity: high > Owner: greg@kroah.com > Submitter: reyems@telkomsa.net > > > Most recent kernel where this bug did not occur:2.6.18.2 No, we're asking in which kernel did this *not* occur. > Hardware Environment: Embedded MPC8347E-PowerPC with 512MByte DDRAM > Software Environment: Linux > Problem Description: USB EHCI-FSL-HOST driver would give inconsistent errors > when system has more tahn 256MByte RAM installed > > Steps to reproduce: Install more than 256MByte memory > Applying the patch below fix the problem: > --- linux-2.6.18.2/drivers/usb/host/ehci-fsl.c 2006-11-04 03:33:58.000000000 > +0200 > +++ linux-2.6.18.2-dash/drivers/usb/host/ehci-fsl.c 2006-11-07 > 13:53:55.000000000 +0200 > @@ -191,7 +191,7 @@ > platform_data; > /* Enable PHY interface in the control reg. */ > out_be32(non_ehci + FSL_SOC_USB_CTRL, 0x00000004); > - out_be32(non_ehci + FSL_SOC_USB_SNOOP1, 0x0000001b); > + out_be32(non_ehci + FSL_SOC_USB_SNOOP1, 0x0000001C); > > if (pdata->operating_mode == FSL_USB2_DR_HOST) > mpc83xx_setup_phy(ehci, pdata->phy_mode, 0); > Well it looks simple. What does that do? Reply-To: david-b@pacbell.net On Thursday 09 November 2006 10:07 pm, Andrew Morton wrote: > > @@ -191,7 +191,7 @@ > > platform_data; > > /* Enable PHY interface in the control reg. */ > > out_be32(non_ehci + FSL_SOC_USB_CTRL, 0x00000004); > > - out_be32(non_ehci + FSL_SOC_USB_SNOOP1, 0x0000001b); > > + out_be32(non_ehci + FSL_SOC_USB_SNOOP1, 0x0000001C); > > > > if (pdata->operating_mode == FSL_USB2_DR_HOST) > > mpc83xx_setup_phy(ehci, pdata->phy_mode, 0); > > > > Well it looks simple. What does that do? According to the comment it affects the PHY interface. Now, how a PHY could care about DMA ... If that 0x1c is related to physical memory size, it probably shouldn't be hard-wired like that. On Thu, 9 Nov 2006 22:37:17 -0800 David Brownell <david-b@pacbell.net> wrote: > On Thursday 09 November 2006 10:07 pm, Andrew Morton wrote: > > > > @@ -191,7 +191,7 @@ > > > platform_data; > > > /* Enable PHY interface in the control reg. */ > > > out_be32(non_ehci + FSL_SOC_USB_CTRL, 0x00000004); > > > - out_be32(non_ehci + FSL_SOC_USB_SNOOP1, 0x0000001b); > > > + out_be32(non_ehci + FSL_SOC_USB_SNOOP1, 0x0000001C); > > > > > > if (pdata->operating_mode == FSL_USB2_DR_HOST) > > > mpc83xx_setup_phy(ehci, pdata->phy_mode, 0); > > > > > > > Well it looks simple. What does that do? > > According to the comment it affects the PHY interface. > > Now, how a PHY could care about DMA ... > > If that 0x1c is related to physical memory size, it > probably shouldn't be hard-wired like that. Stefan replied but he chopped off all the Cc's :( On Fri, 10 Nov 2006 08:39:54 +0200 "Stefan Meyer" <reyems@telkomsa.net> wrote: > Hi Andrew > I am not much of an expert, but according to the datasheets the Snoop1 & > Snoop2 registers create a coherent DMA window (I presume for the use of USB > related DMA). > The current setting of 0x0000001b creates a window starting at 0x00000000 > with a length of 256MByte. > Changing it to 0x0000001C creates a 512MByte windows (this was of course a > non-generic quick fix). > Running with anything up to 256MByte is fine. > My system has 512MByte and it took quite a bit of chasing to find this > issue. Hi Andrew I am not much of an expert, but according to the datasheets the Snoop1 & Snoop2 registers create a coherent DMA window (I presume for the use of USB related DMA). The current setting of 0x0000001b creates a window starting at 0x00000000 with a length of 256MByte. Changing it to 0x0000001C creates a 512MByte windows (this was of course a non-generic quick fix). Running with anything up to 256MByte is fine. My system has 512MByte and it took quite a bit of chasing to find this issue. Hope that helps Regards Stefan Meyer ----- Original Message ----- From: "Andrew Morton" <akpm@osdl.org> To: <linux-usb-devel@lists.sourceforge.net>; "Alan Stern" <stern@rowland.harvard.edu>; "David Brownell" <david-b@pacbell.net>; "Greg KH" <greg@kroah.com> Cc: "bugme-daemon@kernel-bugs.osdl.org" <bugme-daemon@bugzilla.kernel.org>; <reyems@telkomsa.net> Sent: Friday, November 10, 2006 8:07 AM Subject: Re: [Bugme-new] [Bug 7482] New: usb ehci driver crash on system with more tahn 256MByte memory > On Thu, 9 Nov 2006 22:01:21 -0800 > bugme-daemon@bugzilla.kernel.org wrote: > >> http://bugzilla.kernel.org/show_bug.cgi?id=7482 >> >> Summary: usb ehci driver crash on system with more tahn >> 256MByte >> memory >> Kernel Version: 2.6.18.2 >> Status: NEW >> Severity: high >> Owner: greg@kroah.com >> Submitter: reyems@telkomsa.net >> >> >> Most recent kernel where this bug did not occur:2.6.18.2 > > No, we're asking in which kernel did this *not* occur. > >> Hardware Environment: Embedded MPC8347E-PowerPC with 512MByte DDRAM >> Software Environment: Linux >> Problem Description: USB EHCI-FSL-HOST driver would give inconsistent >> errors >> when system has more tahn 256MByte RAM installed >> >> Steps to reproduce: Install more than 256MByte memory >> Applying the patch below fix the problem: >> --- linux-2.6.18.2/drivers/usb/host/ehci-fsl.c 2006-11-04 >> 03:33:58.000000000 >> +0200 >> +++ linux-2.6.18.2-dash/drivers/usb/host/ehci-fsl.c 2006-11-07 >> 13:53:55.000000000 +0200 >> @@ -191,7 +191,7 @@ >> platform_data; >> /* Enable PHY interface in the control reg. */ >> out_be32(non_ehci + FSL_SOC_USB_CTRL, 0x00000004); >> - out_be32(non_ehci + FSL_SOC_USB_SNOOP1, 0x0000001b); >> + out_be32(non_ehci + FSL_SOC_USB_SNOOP1, 0x0000001C); >> >> if (pdata->operating_mode == FSL_USB2_DR_HOST) >> mpc83xx_setup_phy(ehci, pdata->phy_mode, 0); >> > > Well it looks simple. What does that do? > > > Reply-To: david-b@pacbell.net On Thursday 09 November 2006 11:56 pm, Stefan Meyer wrote: > Changing it to 0x0000001C creates a 512MByte windows (this was of course a > non-generic quick fix). > Running with anything up to 256MByte is fine. > My system has 512MByte and it took quite a bit of chasing to find this > issue. A better patch would (a) fix the comment which says both lines of code affects the PHY, (b) not hard-wire the amount of memory on the system. Assuming it's possible to put non-cacheable memory into that chunk of address space (memory mapped peripherals etc) then it's dangerous to hard-wire _any_ value. - Dave Reply-To: david-b@pacbell.net On Friday 18 May 2007, Andrew Morton wrote: > On Fri, 10 Nov 2006 00:47:59 -0800 David Brownell <david-b@pacbell.net> wrote: > > > On Thursday 09 November 2006 11:56 pm, Stefan Meyer wrote: > > > > > Changing it to 0x0000001C creates a 512MByte windows (this was of course a > > > non-generic quick fix). > > > Running with anything up to 256MByte is fine. > > > My system has 512MByte and it took quite a bit of chasing to find this > > > issue. > > > > A better patch would (a) fix the comment which says both lines of > > code affects the PHY, (b) not hard-wire the amount of memory on > > the system. > > > > Assuming it's possible to put non-cacheable memory into that chunk > > of address space (memory mapped peripherals etc) then it's dangerous > > to hard-wire _any_ value. > > > > So, did this ever get fixed? (Full history at > http://bugzilla.kernel.org/show_bug.cgi?id=7482) Not from what I can tell. I cc'd the Freescale person who seems to be maintaining EHCI on that platform ... > > Thanks. > Reply-To: LeoLi@freescale.com > -----Original Message----- > From: David Brownell [mailto:david-b@pacbell.net] > Sent: Friday, May 18, 2007 6:49 PM > To: Andrew Morton > Cc: Stefan Meyer; linux-usb-devel@lists.sourceforge.net; Alan Stern; Greg KH; > bugme-daemon@kernel-bugs.osdl.org; Natalie Protasevich; Li Yang-r58472 > Subject: Re: [Bulk] Re: [Bugme-new] [Bug 7482] New: usb ehci driver crash on system > with more tahn 256MByte memory > > On Friday 18 May 2007, Andrew Morton wrote: > > On Fri, 10 Nov 2006 00:47:59 -0800 David Brownell <david-b@pacbell.net> wrote: > > > > > On Thursday 09 November 2006 11:56 pm, Stefan Meyer wrote: > > > > > > > Changing it to 0x0000001C creates a 512MByte windows (this was of course a > > > > non-generic quick fix). > > > > Running with anything up to 256MByte is fine. > > > > My system has 512MByte and it took quite a bit of chasing to find this > > > > issue. > > > > > > A better patch would (a) fix the comment which says both lines of > > > code affects the PHY, (b) not hard-wire the amount of memory on > > > the system. > > > > > > Assuming it's possible to put non-cacheable memory into that chunk > > > of address space (memory mapped peripherals etc) then it's dangerous > > > to hard-wire _any_ value. > > > > > > > So, did this ever get fixed? (Full history at > > http://bugzilla.kernel.org/show_bug.cgi?id=7482) > > Not from what I can tell. I cc'd the Freescale person who seems > to be maintaining EHCI on that platform ... I see. The complete fix that I expect is to set snooping on the whole 4G memory space as fsl_usb2_udc did. Also, we need to take care of the case that the Soc used on architectures other than powerpc which don't have snooping mechanism. Shall I provide a patch to fix this? - Leo Reply-To: david-b@pacbell.net On Friday 18 May 2007, Li Yang-r58472 wrote: > > > So, did this ever get fixed? (Full history at > > > http://bugzilla.kernel.org/show_bug.cgi?id=7482) > > > > Not from what I can tell. I cc'd the Freescale person who seems > > to be maintaining EHCI on that platform ... > > I see. The complete fix that I expect is to set snooping on the whole > 4G memory space as fsl_usb2_udc did. Also, we need to take care of the > case that the Soc Well, that "IP core"... > used on architectures other than powerpc which don't > have snooping mechanism. > > Shall I provide a patch to fix this? Yes, please. - Dave Reply-To: leoli@freescale.com Here is the patch to fix it. From: Li Yang <LeoLi@freescale.com> Date: Mon, 21 May 2007 13:08:57 +0800 Subject: [PATCH] ehci-fsl: fix cache coherency problem on system with large memory The patch fix bug http://bugzilla.kernel.org/show_bug.cgi?id=7482. It sets USB snooping on 4G space for PowerPC platforms without CONFIG_NOT_COHERENT_CACHE defined. Reported-by: Stefan Meyer <reyems@telkomsa.net> Signed-off-by: Li Yang <leoli@freescale.com> --- drivers/usb/host/ehci-fsl.c | 11 ++++++++++- drivers/usb/host/ehci-fsl.h | 1 + 2 files changed, 11 insertions(+), 1 deletions(-) diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c index e802a13..369ea94 100644 --- a/drivers/usb/host/ehci-fsl.c +++ b/drivers/usb/host/ehci-fsl.c @@ -194,7 +194,16 @@ static void mpc83xx_usb_setup(struct usb_hcd *hcd) /* Enable PHY interface in the control reg. */ temp = in_be32(non_ehci + FSL_SOC_USB_CTRL); out_be32(non_ehci + FSL_SOC_USB_CTRL, temp | 0x00000004); - out_be32(non_ehci + FSL_SOC_USB_SNOOP1, 0x0000001b); +#if defined(CONFIG_PPC32) && !defined(CONFIG_NOT_COHERENT_CACHE) + /* Turn on cache snooping hardware, since some PowerPC platforms + * wholly rely on hardware to deal with cache coherent. */ + + /* Setup Snooping for all the 4GB space */ + /* SNOOP1 starts from 0x0, size 2G */ + out_be32(non_ehci + FSL_SOC_USB_SNOOP1, 0x0 | SNOOP_SIZE_2GB); + /* SNOOP2 starts from 0x80000000, size 2G */ + out_be32(non_ehci + FSL_SOC_USB_SNOOP2, 0x80000000 | SNOOP_SIZE_2GB); +#endif if ((pdata->operating_mode == FSL_USB2_DR_HOST) || (pdata->operating_mode == FSL_USB2_DR_OTG)) diff --git a/drivers/usb/host/ehci-fsl.h b/drivers/usb/host/ehci-fsl.h index f28736a..b5e59db 100644 --- a/drivers/usb/host/ehci-fsl.h +++ b/drivers/usb/host/ehci-fsl.h @@ -34,4 +34,5 @@ #define FSL_SOC_USB_PRICTRL 0x40c /* NOTE: big-endian */ #define FSL_SOC_USB_SICTRL 0x410 /* NOTE: big-endian */ #define FSL_SOC_USB_CTRL 0x500 /* NOTE: big-endian */ +#define SNOOP_SIZE_2GB 0x1e #endif /* _EHCI_FSL_H */ Li Yang <leoli@freescale.com> prepared a patch for this, which is in -mm. |