| Message ID | 1261176637-23912-1-git-send-email-ptyser@xes-inc.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Commit | d1d47ec6e62ab08d2ebb925fd9203abfad3adfbf |
| Delegated to: | Benjamin Herrenschmidt |
| Headers | show |
Hello list. I've run into a rather odd problem. I seem to have a problem with full-speed isochronous transfers across a USB2.0 Hub. I believe this was observed before with the general EHCI drivers in Linux. In the latest branch of the kernel, the EHCI driver has some "Enhanced Transaction Translation" scheduling. This would be great for me to use as it seems to directly address the issues I've seen. However, I'm not really sure on how to proceed because the DWC_OTG driver is not GPL code while the EHCI code I'd like to use is. Therefore, I don't believe that I can port the Enhanced Transaction Translation routine into the DWC_OTG driver without violating both the GPL and Synopsis's own driver. Does anyone have a suggestion on how to proceed?
Dear Hunter Cobbs, In message <1261190115.14590.5.camel@mobiLinux> you wrote: > Hello list. I've run into a rather odd problem. I seem to have a > problem with full-speed isochronous transfers across a USB2.0 Hub. I > believe this was observed before with the general EHCI drivers in Linux. Which exazct kernel / driver cversions are you talking about? > In the latest branch of the kernel, the EHCI driver has some "Enhanced > Transaction Translation" scheduling. This would be great for me to use > as it seems to directly address the issues I've seen. However, I'm not > really sure on how to proceed because the DWC_OTG driver is not GPL code Oops? if that wa the case you could not use (and distribute) this code in a Linux context at all. > while the EHCI code I'd like to use is. Therefore, I don't believe that > I can port the Enhanced Transaction Translation routine into the DWC_OTG > driver without violating both the GPL and Synopsis's own driver. What makes you think so? The Synopsis dwc_otg driver license does not seem to be in conflict to the GPL, if you ask me (but then, I am not a layer). When we worked on this, we were assured that the use of this driver source in the context of GPLed software like the Linux kernel (or U-Boot) was not considered an issue by Synopsis. > Does anyone have a suggestion on how to proceed? We have been doing some work on the DWC drivers in the past, fixing a number of issues. [We never attempted to psuh any of this into mainline as the Synopsis drivers are - in our opinion - not avcceptable for mainline but require more or less a rewrite from scratch. But you can find all this stuff in our linux-2.6-denx repo.] I recommend you give a try to our latest stable branch. Best regards, Wolfgang Denk
Well, thank you for the assurance on the Synopsis drivers not being in conflict. That makes me feel a little better. We're using Linux 2.6.30(.3)-denx. We've worked on this issue a bit before, although at the time you were doing some work for AMCC on just accessing devices across a hub in general. We've been using the driver for a few months now and it is very stable on first-use of the driver, but once we close a full-speed port and try to reopen, that's when we get error concerning not being able to allocate resources and getting -28 errors. I don't have anything precise in front of me at the moment, my test equipment is at the office. But this seems to be the issues that lead to the Enhanced Transaction Translation Scheduling changes in the main-line kernel. I'm mainly being, I believe, overly cautious about not including GPL code in the Synopsis driver simply because I'd rather be overly careful than be exposed to some liability later down the road if Synopsis decides they want to make their driver completely proprietary. While the code is available, I'm not sure if we can include code from GPL sources in the Synopsis drivers without requiring the Synopsis driver to become completely GPL. I am also not a lawyer, but I'd rather be cautious. I did do a comparison on the previous branch we are (2.6.30.3-denx) and the current stable. The only differences in the code were minor enough that I simply merged them into our existing codebase. It didn't really help our issue, but on block transfers and standard connect/disconnect the driver seemed more responsive. I believe this was mainly from Setphan's actions to put some sleeps inside of tight loops that would chew up major CPU time on a non-preemptable kernel(which we had earlier). On Sat, 2009-12-19 at 19:56 +0100, Wolfgang Denk wrote: > Dear Hunter Cobbs, > > In message <1261190115.14590.5.camel@mobiLinux> you wrote: > > Hello list. I've run into a rather odd problem. I seem to have a > > problem with full-speed isochronous transfers across a USB2.0 Hub. I > > believe this was observed before with the general EHCI drivers in Linux. > > Which exazct kernel / driver cversions are you talking about? > > > In the latest branch of the kernel, the EHCI driver has some "Enhanced > > Transaction Translation" scheduling. This would be great for me to use > > as it seems to directly address the issues I've seen. However, I'm not > > really sure on how to proceed because the DWC_OTG driver is not GPL code > > Oops? if that wa the case you could not use (and distribute) this code > in a Linux context at all. > > > while the EHCI code I'd like to use is. Therefore, I don't believe that > > I can port the Enhanced Transaction Translation routine into the DWC_OTG > > driver without violating both the GPL and Synopsis's own driver. > > What makes you think so? > > The Synopsis dwc_otg driver license does not seem to be in conflict > to the GPL, if you ask me (but then, I am not a layer). When we > worked on this, we were assured that the use of this driver source in > the context of GPLed software like the Linux kernel (or U-Boot) was > not considered an issue by Synopsis. > > > Does anyone have a suggestion on how to proceed? > > We have been doing some work on the DWC drivers in the past, fixing a > number of issues. [We never attempted to psuh any of this into > mainline as the Synopsis drivers are - in our opinion - not > avcceptable for mainline but require more or less a rewrite from > scratch. But you can find all this stuff in our linux-2.6-denx repo.] > > I recommend you give a try to our latest stable branch. > > Best regards, > > Wolfgang Denk >
Dear Hunter Cobbs, In message <1261340025.3473.15.camel@mobiLinux> you wrote: > > We're using Linux 2.6.30(.3)-denx. We've worked on this issue a bit > before, although at the time you were doing some work for AMCC on just > accessing devices across a hub in general. We've been using the driver > for a few months now and it is very stable on first-use of the driver, > but once we close a full-speed port and try to reopen, that's when we > get error concerning not being able to allocate resources and getting > -28 errors. I don't have anything precise in front of me at the moment, > my test equipment is at the office. But this seems to be the issues > that lead to the Enhanced Transaction Translation Scheduling changes in > the main-line kernel. If you have a detailed bug report, ideally including an easy to reproduce test case to trigger the problem then please feel free to send it to our support address. > I'm mainly being, I believe, overly cautious about not including GPL > code in the Synopsis driver simply because I'd rather be overly careful > than be exposed to some liability later down the road if Synopsis > decides they want to make their driver completely proprietary. While > the code is available, I'm not sure if we can include code from GPL > sources in the Synopsis drivers without requiring the Synopsis driver to > become completely GPL. I am also not a lawyer, but I'd rather be > cautious. Well, their license is relatively simple: the driver is unsupported, it "IS NOT an item of Licensed Software or Licensed Product under any End User Software License Agreement or Agreement for Licensed Product", and you "are permitted to use andredistribute this Software in source and binary forms, with or without modification, provided that redistributions of source code must retain this notice." And there is no warranty, of course. IANAL, but I see no risks here. Best regards, Wolfgang Denk
On Fri, 2009-12-18 at 16:50 -0600, Peter Tyser wrote: > Recent U-Boot commit 5ccd29c3679b3669b0bde5c501c1aa0f325a7acb caused > the "cpu-release-addr" device tree property to contain the physical RAM > location that secondary cores were spinning at. Previously, the > "cpu-release-addr" property contained a value referencing the boot page > translation address range of 0xfffffxxx, which then indirectly accessed > RAM. > > The "cpu-release-addr" is currently ioremapped and the secondary cores > kicked. However, due to the recent change in "cpu-release-addr", it > sometimes points to a memory location in low memory that cannot be > ioremapped. For example on a P2020-based board with 512MB of RAM the > following error occurs on bootup: > > <...> > mpic: requesting IPIs ... > __ioremap(): phys addr 0x1ffff000 is RAM lr c05df9a0 > Unable to handle kernel paging request for data at address 0x00000014 > Faulting instruction address: 0xc05df9b0 > Oops: Kernel access of bad area, sig: 11 [#1] > SMP NR_CPUS=2 P2020 RDB > Modules linked in: > <... eventual kernel panic> > > Adding logic to conditionally ioremap or access memory directly resolves > the issue. > > Signed-off-by: Peter Tyser <ptyser@xes-inc.com> > Signed-off-by: Nate Case <ncase@xes-inc.com> > Reported-by: Dipen Dudhat <B09055@freescale.com> > Tested-by: Dipen Dudhat <B09055@freescale.com> Any chance this going to be picked up for 2.6.33? The issue is currently going to bite anyone using an MP-capable 85xx system that doesn't use highmem. Thanks, Peter
On Jan 21, 2010, at 4:28 PM, Peter Tyser wrote: > On Fri, 2009-12-18 at 16:50 -0600, Peter Tyser wrote: >> Recent U-Boot commit 5ccd29c3679b3669b0bde5c501c1aa0f325a7acb caused >> the "cpu-release-addr" device tree property to contain the physical RAM >> location that secondary cores were spinning at. Previously, the >> "cpu-release-addr" property contained a value referencing the boot page >> translation address range of 0xfffffxxx, which then indirectly accessed >> RAM. >> >> The "cpu-release-addr" is currently ioremapped and the secondary cores >> kicked. However, due to the recent change in "cpu-release-addr", it >> sometimes points to a memory location in low memory that cannot be >> ioremapped. For example on a P2020-based board with 512MB of RAM the >> following error occurs on bootup: >> >> <...> >> mpic: requesting IPIs ... >> __ioremap(): phys addr 0x1ffff000 is RAM lr c05df9a0 >> Unable to handle kernel paging request for data at address 0x00000014 >> Faulting instruction address: 0xc05df9b0 >> Oops: Kernel access of bad area, sig: 11 [#1] >> SMP NR_CPUS=2 P2020 RDB >> Modules linked in: >> <... eventual kernel panic> >> >> Adding logic to conditionally ioremap or access memory directly resolves >> the issue. >> >> Signed-off-by: Peter Tyser <ptyser@xes-inc.com> >> Signed-off-by: Nate Case <ncase@xes-inc.com> >> Reported-by: Dipen Dudhat <B09055@freescale.com> >> Tested-by: Dipen Dudhat <B09055@freescale.com> > > Any chance this going to be picked up for 2.6.33? The issue is > currently going to bite anyone using an MP-capable 85xx system that > doesn't use highmem. This just got lost in my queue. Will apply and send up for .33 - k
On Dec 18, 2009, at 4:50 PM, Peter Tyser wrote: > Recent U-Boot commit 5ccd29c3679b3669b0bde5c501c1aa0f325a7acb caused > the "cpu-release-addr" device tree property to contain the physical RAM > location that secondary cores were spinning at. Previously, the > "cpu-release-addr" property contained a value referencing the boot page > translation address range of 0xfffffxxx, which then indirectly accessed > RAM. > > The "cpu-release-addr" is currently ioremapped and the secondary cores > kicked. However, due to the recent change in "cpu-release-addr", it > sometimes points to a memory location in low memory that cannot be > ioremapped. For example on a P2020-based board with 512MB of RAM the > following error occurs on bootup: > > <...> > mpic: requesting IPIs ... > __ioremap(): phys addr 0x1ffff000 is RAM lr c05df9a0 > Unable to handle kernel paging request for data at address 0x00000014 > Faulting instruction address: 0xc05df9b0 > Oops: Kernel access of bad area, sig: 11 [#1] > SMP NR_CPUS=2 P2020 RDB > Modules linked in: > <... eventual kernel panic> > > Adding logic to conditionally ioremap or access memory directly resolves > the issue. > > Signed-off-by: Peter Tyser <ptyser@xes-inc.com> > Signed-off-by: Nate Case <ncase@xes-inc.com> > Reported-by: Dipen Dudhat <B09055@freescale.com> > Tested-by: Dipen Dudhat <B09055@freescale.com> > --- > arch/powerpc/platforms/85xx/smp.c | 21 +++++++++++++++++++-- > 1 files changed, 19 insertions(+), 2 deletions(-) applied to merge for 2.6.33 - k
diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c index 04160a4..a15f582 100644 --- a/arch/powerpc/platforms/85xx/smp.c +++ b/arch/powerpc/platforms/85xx/smp.c @@ -46,6 +46,7 @@ smp_85xx_kick_cpu(int nr) __iomem u32 *bptr_vaddr; struct device_node *np; int n = 0; + int ioremappable; WARN_ON (nr < 0 || nr >= NR_CPUS); @@ -59,21 +60,37 @@ smp_85xx_kick_cpu(int nr) return; } + /* + * A secondary core could be in a spinloop in the bootpage + * (0xfffff000), somewhere in highmem, or somewhere in lowmem. + * The bootpage and highmem can be accessed via ioremap(), but + * we need to directly access the spinloop if its in lowmem. + */ + ioremappable = *cpu_rel_addr > virt_to_phys(high_memory); + /* Map the spin table */ - bptr_vaddr = ioremap(*cpu_rel_addr, SIZE_BOOT_ENTRY); + if (ioremappable) + bptr_vaddr = ioremap(*cpu_rel_addr, SIZE_BOOT_ENTRY); + else + bptr_vaddr = phys_to_virt(*cpu_rel_addr); local_irq_save(flags); out_be32(bptr_vaddr + BOOT_ENTRY_PIR, nr); out_be32(bptr_vaddr + BOOT_ENTRY_ADDR_LOWER, __pa(__early_start)); + if (!ioremappable) + flush_dcache_range((ulong)bptr_vaddr, + (ulong)(bptr_vaddr + SIZE_BOOT_ENTRY)); + /* Wait a bit for the CPU to ack. */ while ((__secondary_hold_acknowledge != nr) && (++n < 1000)) mdelay(1); local_irq_restore(flags); - iounmap(bptr_vaddr); + if (ioremappable) + iounmap(bptr_vaddr); pr_debug("waited %d msecs for CPU #%d.\n", n, nr); }