LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [ewg] Re: [PATCH 2.6.31] ehca: Tolerate dynamic memory operations and huge pages
From: Roland Dreier @ 2009-06-16 16:10 UTC (permalink / raw)
  To: Alexander Schmidt
  Cc: linuxppc-dev, Hoang-Nam Nguyen, raisch, linux-kernel, ewg
In-Reply-To: <20090616090809.34d162fc@BL3D1974.boeblingen.de.ibm.com>


 > Yeah, the notifier code remains untouched as we still do not allow dynamic
 > memory operations _while_ our module is loaded. The patch allows the driver to
 > cope with DMEM operations that happened before the module was loaded, which
 > might result in a non-contiguous memory layout. When the driver registers
 > its global memory region in the system, the memory layout must be considered.
 > 
 > We chose the term "toleration" instead of "support" to illustrate this.

I see.  So things just silently broke in some cases when the driver was
loaded after operations you didn't tolerate?

Anyway, thanks for the explanation.

^ permalink raw reply

* MPC83xx watchdog reset board dead lock
From: Leon Woestenberg @ 2009-06-16 15:52 UTC (permalink / raw)
  To: Linux PPC

Hello,

this is a hardware, even board issue, but I hope to find the right
target audience here.


In our MPC83xx design I would like to prevent dead lock in case where
a field upgrade is performed, i.e. NOR Flash is erased or written, and
the MPC83xx built-in hardware watchdog triggers.

In u-boot the scenario can be easily reproduced by running this
command (WARNING, erases some sectors!) on an MPC8313E-RDB:

erase_wdg=mw.l 0xe0000204 0x10000007 1;mw.w 0xe000020e 0x556c 1;mw.w
0xe000020e 0xaa39 1;erase 1:10-30

This sets up the watchdog to reset soonish, then starts erasing NOR
sectors. Watchdog triggers and resets -> Dead lock.


Most MPC8xxx board designs I have seen suffer from this possible dead lock:
- NOR Flash is put in erase mode or write mode
- Hardware watchdog triggers
- HRESET# is asserted by the processor, during which the configuration
words are read from NOR Flash.

Either
HRESET# is not attached to NOR, NOR stays in erase/write mode and
invalid words will be read -> dead lock

or either:
HRESET# is attached to NOR reset, NOR is reset, but stays in reset as
HRESET# stays asserted.



We have been looking at several solutions hardware wise that reset the
NOR flash on HRESET# going low, but the processors are stubborn,
read the config words only once, than dead lock.

I wonder if there are known-working designs for this.

Regards,
-- 
Leon

^ permalink raw reply

* Re: MPC83xx watchdog reset board dead lock
From: David Hawkins @ 2009-06-16 16:30 UTC (permalink / raw)
  To: Leon Woestenberg; +Cc: Linux PPC
In-Reply-To: <c384c5ea0906160852l4845760cp8594463a866683dc@mail.gmail.com>

Hi Leon,

> Most MPC8xxx board designs I have seen suffer from this possible dead lock:
> - NOR Flash is put in erase mode or write mode
> - Hardware watchdog triggers
> - HRESET# is asserted by the processor, during which the configuration
> words are read from NOR Flash.
> 
> Either
> HRESET# is not attached to NOR, NOR stays in erase/write mode and
> invalid words will be read -> dead lock
> 
> or either:
> HRESET# is attached to NOR reset, NOR is reset, but stays in reset as
> HRESET# stays asserted.
> 
> We have been looking at several solutions hardware wise that reset the
> NOR flash on HRESET# going low, but the processors are stubborn,
> read the config words only once, than dead lock.
> 
> I wonder if there are known-working designs for this.

What do you do in the case of blank flash on a board?

I'm not sure that this will work for you, but it might ...
on my boards, the PowerPC connects to an FPGA on the local
bus, and the flash is connected to that FPGA (it made
routing easier to go through the FPGA). When the board
powers up, a small FSM in the FPGA reads from the first
word in Flash, and checks it is not blank. If it is
blank, the configuration pins are asserted such that
the processor uses a hard-coded reset configuration word,
and the processor reset is deasserted. If the flash is
not blank, the processor is allowed to boot from Flash.

An alternative method for firmware updates would be
to use the boot from high-mem versus low-mem flag
in the RCWs. On a number of the Freescale EVMs, you
can have two images in the Flash; one at low mem,
and one at high mem. Toggling a dip switch selects
the value of the BMS bit in the RCWs, and lets you boot
from one image or the other. This is a good trick that
avoids having to have a debugger to recover back
to a sane image - note that a CPLD delivers the RCWs,
not the flash.

So in either of these examples, the RCWs source is
sometimes not the Flash image ... which requires
that feature designed into the original hardware.

Those two ideas should get you thinking :)

Cheers,
Dave

^ permalink raw reply

* Re: [PATCH v3] powerpc: add ioremap_early() for mapping IO regions before MMU_init()
From: Grant Likely @ 2009-06-16 16:39 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: scottwood, linuxppc-dev
In-Reply-To: <1245048902.19217.47.camel@pasglop>

On Mon, Jun 15, 2009 at 12:55 AM, Benjamin
Herrenschmidt<benh@kernel.crashing.org> wrote:
> On Wed, 2009-05-27 at 12:55 -0600, Grant Likely wrote:
>> From: Grant Likely <grant.likely@secretlab.ca>
>>
>> ioremap_early() is useful for things like mapping SoC internally registe=
rs
>> and early debug output because it allows mappings to devices to be setup
>> early in the boot process where they are needed. =A0It also give a
>> performance boost since BAT mapped registers don't get flushed out of
>> the TLB.
>>
>> Without ioremap_early(), early mappings are set up in an ad-hoc manner
>> and they get lost when the MMU is set up. =A0Drivers then have to perfor=
m
>> hacky fixups to transition over to new mappings.
>>
>> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>> ---
>
> Approach looks sane at first glance.
>
> However, I'm reluctant to but that in until we have all MMU types
> covered or we'll have "interesting" surprises.

I considered this and was originally concerned about the same thing.
However, ioremap_early() is special in that caller cannot take for
granted what it does and must understand the side effects.  For
example; on 6xx ioremap_early is always going to carve out a minimum
of 128k from the virtual address space and it is likely that the range
will extend both both before and after the desired address.  Plus,
because of the limited number of BATs there is real likelyhood that
ioremap_early() will fail.  Code calling it must handle the failure
mode gracefully.

IMHO, I think this is only really applicable for platform code that
understands the memory layout.  ie. map the entire IMMR at once, or
mapping local bus device ranges.  On the 5200 I call it early in the
platform setup on the IMMR range, but I don't actually do anything
with the returned value (unless I'm doing udbg).  Then, future
ioremap() calls to that range get to use the BAT mapping
transparently.

On the 440 and the 405 with the small TLB users also need to be
careful so that too many TLB entries don't get pinned to static
allocation and negate the performance improvement of doing the pinning
in the first place.  Again I think it is best restricted to platform
code, and should never cause the system to fail to boot if the mapping
doesn't work.

I do want to implement it for all MMUs (when I have the bandwidth to
do so), but I don't think merging it needs to wait.  If it is merged
as is and someone uses it in the wrong place (ie. non-platform code),
then 405 and 440 will fail to build, and it will get caught quickly.
Alternately I could implement stub ioremap_early() for non-6xx to just
return NULL until it can be implemented.  Callers who don't handle
NULL gracefully are broken, but it won't be known until boot time.

> Also, the CPM patch
> doesn't actually fix the massive bogon in there :-)

Yeah, that was just to get it to build.  I'll look at fix that too.

>> + =A0 =A0 /* Be loud and annoying if someone calls this too late.
>> + =A0 =A0 =A0* No need to crash the kernel though */
>> + =A0 =A0 WARN_ON(mem_init_done);
>> + =A0 =A0 if (mem_init_done)
>> + =A0 =A0 =A0 =A0 =A0 =A0 return NULL;
>
> Can't we write
>
> =A0 =A0 =A0 =A0if (WARN_ON(mem_init_done))
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return NULL;
>
> nowadays ?

I'll check.

>> + =A0 =A0 /* Make sure request is sane */
>> + =A0 =A0 if (size =3D=3D 0)
>> + =A0 =A0 =A0 =A0 =A0 =A0 return NULL;
>> +
>> + =A0 =A0 /* If the region is already block mapped, then there is nothin=
g
>> + =A0 =A0 =A0* to do; just return the mapped address */
>> + =A0 =A0 v =3D p_mapped_by_bats(addr);
>> + =A0 =A0 if (v)
>> + =A0 =A0 =A0 =A0 =A0 =A0 return (void __iomem *)v;
>
> Should we check the size ?

Ugh.  Yes.  good catch.

>> + =A0 =A0 /* Align region size */
>> + =A0 =A0 for (bl =3D 128<<10; bl < (256<<20); bl <<=3D 1) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 p =3D _ALIGN_DOWN(addr, bl); /* BATs align on =
128k boundaries */
>> + =A0 =A0 =A0 =A0 =A0 =A0 size =3D ALIGN(addr - p + size, bl);
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (bl >=3D size)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
>> + =A0 =A0 }
>> +
>> + =A0 =A0 /* Complain loudly if too much is requested */
>> + =A0 =A0 if (bl >=3D (256<<20)) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 WARN_ON(1);
>> + =A0 =A0 =A0 =A0 =A0 =A0 return NULL;
>> + =A0 =A0 }
>
> Do we avoid that running into the linear mapping ?

No.  I'll fix.

>> + =A0 =A0 /* Allocate the aligned virtual base address. =A0ALIGN_DOWN is=
 used
>> + =A0 =A0 =A0* to ensure no overlaps occur with normal 4k ioremaps. */
>> + =A0 =A0 ioremap_bot =3D _ALIGN_DOWN(ioremap_bot, bl) - size;
>> +
>> + =A0 =A0 /* Set up a BAT for this IO region */
>> + =A0 =A0 i =3D loadbat(ioremap_bot, p, size, PAGE_KERNEL_NCG);
>> + =A0 =A0 if (i < 0)
>> + =A0 =A0 =A0 =A0 =A0 =A0 return NULL;
>> +
>> + =A0 =A0 return (void __iomem *) (ioremap_bot + (addr - p));
>> =A0}
>>
>> =A0/*
>> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_common.c b/arch/powerpc=
/platforms/52xx/mpc52xx_common.c
>> index 8e3dd5a..2c49148 100644
>> --- a/arch/powerpc/platforms/52xx/mpc52xx_common.c
>> +++ b/arch/powerpc/platforms/52xx/mpc52xx_common.c
>> @@ -146,7 +146,20 @@ static struct of_device_id mpc52xx_cdm_ids[] __init=
data =3D {
>> =A0void __init
>> =A0mpc52xx_map_common_devices(void)
>> =A0{
>> + =A0 =A0 const struct of_device_id immr_ids[] =3D {
>> + =A0 =A0 =A0 =A0 =A0 =A0 { .compatible =3D "fsl,mpc5200-immr", },
>> + =A0 =A0 =A0 =A0 =A0 =A0 { .compatible =3D "fsl,mpc5200b-immr", },
>> + =A0 =A0 =A0 =A0 =A0 =A0 { .type =3D "soc", .compatible =3D "mpc5200", =
}, /* lite5200 */
>> + =A0 =A0 =A0 =A0 =A0 =A0 { .type =3D "builtin", .compatible =3D "mpc520=
0", }, /* efika */
>> + =A0 =A0 =A0 =A0 =A0 =A0 {}
>> + =A0 =A0 };
>> =A0 =A0 =A0 struct device_node *np;
>> + =A0 =A0 struct resource res;
>> +
>> + =A0 =A0 /* Pre-map the whole register space using a BAT entry */
>> + =A0 =A0 np =3D of_find_matching_node(NULL, immr_ids);
>> + =A0 =A0 if (np && (of_address_to_resource(np, 0, &res) =3D=3D 0))
>> + =A0 =A0 =A0 =A0 =A0 =A0 ioremap_early(res.start, res.end - res.start +=
 1);
>>
>> =A0 =A0 =A0 /* mpc52xx_wdt is mapped here and used in mpc52xx_restart,
>> =A0 =A0 =A0 =A0* possibly from a interrupt context. wdt is only implemen=
t
>> diff --git a/arch/powerpc/sysdev/cpm_common.c b/arch/powerpc/sysdev/cpm_=
common.c
>> index e4b6d66..370723e 100644
>> --- a/arch/powerpc/sysdev/cpm_common.c
>> +++ b/arch/powerpc/sysdev/cpm_common.c
>> @@ -56,7 +56,7 @@ void __init udbg_init_cpm(void)
>> =A0{
>> =A0 =A0 =A0 if (cpm_udbg_txdesc) {
>> =A0#ifdef CONFIG_CPM2
>> - =A0 =A0 =A0 =A0 =A0 =A0 setbat(1, 0xf0000000, 0xf0000000, 1024*1024, P=
AGE_KERNEL_NCG);
>> + =A0 =A0 =A0 =A0 =A0 =A0 setbat(0xf0000000, 0xf0000000, 1024*1024, PAGE=
_KERNEL_NCG);
>> =A0#endif
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 udbg_putc =3D udbg_putc_cpm;
>> =A0 =A0 =A0 }
>
> That needs to be properly fixed ... maybe using ioremap_early() ? :-)

:-p

> Also, make the initial call ioremap_early_init() just to make things
> clear that one can't just call ioremap(), we are limited to a very
> specific thing here.

ok.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH v3] powerpc: add ioremap_early() for mapping IO regions before MMU_init()
From: Grant Likely @ 2009-06-16 16:40 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: scottwood, linuxppc-dev
In-Reply-To: <1245049031.19217.49.camel@pasglop>

On Mon, Jun 15, 2009 at 12:57 AM, Benjamin
Herrenschmidt<benh@kernel.crashing.org> wrote:
> On Wed, 2009-05-27 at 12:55 -0600, Grant Likely wrote:
>> From: Grant Likely <grant.likely@secretlab.ca>
>>
>> ioremap_early() is useful for things like mapping SoC internally registe=
rs
>> and early debug output because it allows mappings to devices to be setup
>> early in the boot process where they are needed. =A0It also give a
>> performance boost since BAT mapped registers don't get flushed out of
>> the TLB.
>>
>> Without ioremap_early(), early mappings are set up in an ad-hoc manner
>> and they get lost when the MMU is set up. =A0Drivers then have to perfor=
m
>> hacky fixups to transition over to new mappings.
>>
>> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>> ---
>
> My 40x config gives me:
>
> /home/benh/linux-powerpc-test/drivers/video/xilinxfb.c:409: warning:
> =91dcr_host.base=92 may be used uninitialized in this function
>
> (warning, I think, was already there, so the patch is going into -next
> but we may want another one, provided we find a way to shut the idiot up
> without horrible hacks since that's just gcc being stupid I believe).

I'll have the final fix out to you today.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: PowerPC PCI DMA issues (prefetch/coherency?)
From: Scott Wood @ 2009-06-16 16:46 UTC (permalink / raw)
  To: Chris Pringle; +Cc: linuxppc-dev@ozlabs.org list, linux-kernel
In-Reply-To: <4A37C97A.5050508@oxtel.com>

Chris Pringle wrote:
> The kernel version is 2.6.26. Firmware is custom on a custom board.
>> Cache coherency on PCI DMA requires that the memory be mapped with the M
>> attribute on this chip, but that should be happening based on 
>> detection of
>> the core.
>>   
> I'm not sure where to look to verify this?

Check asm/cputable.h for CPU_FTR_NEED_COHERENT.  Make sure that 
CONFIG_8260 is one of the #ifdefs that turns that on.  It looks like 
that was in place by 2.6.26 in arch/powerpc.  I'm not sure what to look 
for in arch/ppc.

>> Also make sure that you park the bus on PCI and raise its arbitration
>> priority, as done at the end of fixup_pci in 
>> arch/powerpc/boot/cuboot-pq2.c.
>>   
> Since this is a reasonably recent kernel,

Not really, there was a fair amount of 82xx work in the mid-2.6.20s. 
The addition of CPU_FTR_NEED_COHERENT to 82xx was somewhere in that time.

Can you try 2.6.30?

> I'd guess that both of these 
> things are correct. I've had a quick look in that file and there is code 
> in there raising arbitartion priority and parking the bus.

Just because the code is there doesn't mean you're using it -- are you 
using cuImage?  Are you using arch/ppc or arch/powerpc?

Typically this would be done by firmware; it's only in cuboot because 
u-boot wasn't doing it.

>> BTW, you may want to post to linuxppc-dev@lists.ozlabs.org for
>> powerpc-specific issues, especially this kind of hardware issue.
>>
>>   
> I've just posted there :-)

Doh, sorry. :-)

> Interestingly, I've just turned off cache snooping and the problem has 
> got much worse. This has surprised me as I thought that part of the job 
> done by pci_map_sg was to flush the CPU cache

It only flushes the cache on hardware that doesn't do coherent DMA.

-Scott

^ permalink raw reply

* Re: PowerPC PCI DMA issues (prefetch/coherency?)
From: Chris Pringle @ 2009-06-16 16:57 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev@ozlabs.org list, linux-kernel
In-Reply-To: <4A37CC72.3060709@freescale.com>

Scott Wood wrote:
>
> Check asm/cputable.h for CPU_FTR_NEED_COHERENT.  Make sure that 
> CONFIG_8260 is one of the #ifdefs that turns that on.  It looks like 
> that was in place by 2.6.26 in arch/powerpc.  I'm not sure what to 
> look for in arch/ppc.
I've just checked that and it's definitely switched on in CPU_FTR_COMMON 
(CONFIG_8260 is also being used).
>
>>> Also make sure that you park the bus on PCI and raise its arbitration
>>> priority, as done at the end of fixup_pci in 
>>> arch/powerpc/boot/cuboot-pq2.c.
>>>   
>> Since this is a reasonably recent kernel,
>
> Not really, there was a fair amount of 82xx work in the mid-2.6.20s. 
> The addition of CPU_FTR_NEED_COHERENT to 82xx was somewhere in that time.
>
> Can you try 2.6.30?
I'll give it a try, but that won't be a quick thing to do - will 
hopefully manage to get that done tomorrow if it patches without too 
many issues. I should point out that we've got the low latency patches 
on this kernel too; I guess it'd be worth trying it without them before 
I move kernels.
>
>> I'd guess that both of these things are correct. I've had a quick 
>> look in that file and there is code in there raising arbitartion 
>> priority and parking the bus.
>
> Just because the code is there doesn't mean you're using it -- are you 
> using cuImage?  Are you using arch/ppc or arch/powerpc?
>
> Typically this would be done by firmware; it's only in cuboot because 
> u-boot wasn't doing it.
Just checked this is being called and it is. We're using arch/powerpc.
>
>
>> Interestingly, I've just turned off cache snooping and the problem 
>> has got much worse. This has surprised me as I thought that part of 
>> the job done by pci_map_sg was to flush the CPU cache
>
> It only flushes the cache on hardware that doesn't do coherent DMA.
>
Ah right - that would explain what we're seeing then... Doh. Thought I 
might have been onto something then. Is there any way to force a cache 
flush? That'd at least prove it was a caching issue if it resolved the 
problem.

Thanks,
Chris

-- 

______________________________
Chris Pringle
Software Engineer

Miranda Technologies Ltd.
Hithercroft Road
Wallingford
Oxfordshire OX10 9DG
UK

Tel. +44 1491 820206
Fax. +44 1491 820001
www.miranda.com

____________________________

Miranda Technologies Limited
Registered in England and Wales CN 02017053
Registered Office: James House, Mere Park, Dedmere Road, Marlow, Bucks, SL7 1FJ

^ permalink raw reply

* Re: MPC83xx watchdog reset board dead lock
From: Leon Woestenberg @ 2009-06-16 16:59 UTC (permalink / raw)
  To: David Hawkins; +Cc: Linux PPC
In-Reply-To: <4A37C88F.8040701@ovro.caltech.edu>

Hello,

On Tue, Jun 16, 2009 at 6:30 PM, David Hawkins<dwh@ovro.caltech.edu> wrote:
>> Most MPC8xxx board designs I have seen suffer from this possible dead
>> lock:
>> - NOR Flash is put in erase mode or write mode
>> - Hardware watchdog triggers
>> - HRESET# is asserted by the processor, during which the configuration
>> words are read from NOR Flash.
>>
>> Either
>> HRESET# is not attached to NOR, NOR stays in erase/write mode and
>> invalid words will be read -> dead lock
>>
>> or either:
>> HRESET# is attached to NOR reset, NOR is reset, but stays in reset as
>> HRESET# stays asserted.
>>
>> We have been looking at several solutions hardware wise that reset the
>> NOR flash on HRESET# going low, but the processors are stubborn,
>> read the config words only once, than dead lock.
>>
>> I wonder if there are known-working designs for this.
>
> What do you do in the case of blank flash on a board?
>
The problem is not with blank flash or firmware upgrades, we know how
to handle that.

Your solution is (a solution) to a different problem.

The problem lies in the fact that board dead lock can occur if the
watchdog triggers, for all reference designs I have seen.

Thanks for thinking along.  I would like to solve the original problem though.
BTW, we use CPLD/FPGAs on most of our boards, this one we do not for
cost reasons.


Regards,
-- 
Leon

^ permalink raw reply

* Re: PowerPC PCI DMA issues (prefetch/coherency?)
From: Scott Wood @ 2009-06-16 17:03 UTC (permalink / raw)
  To: Chris Pringle; +Cc: linuxppc-dev@ozlabs.org list, linux-kernel
In-Reply-To: <4A37CF02.5080906@oxtel.com>

Chris Pringle wrote:
> Ah right - that would explain what we're seeing then... Doh. Thought I 
> might have been onto something then. Is there any way to force a cache 
> flush? That'd at least prove it was a caching issue if it resolved the 
> problem.

You could enable CONFIG_NOT_COHERENT_CACHE.

-Scott

^ permalink raw reply

* Re: [PATCH V2 2/2] mtd/maps/mtd-ram: add an of-platform driver
From: Albrecht Dreß @ 2009-06-16 17:19 UTC (permalink / raw)
  To: Grant Likely
  Cc: devicetree-discuss, linuxppc-dev, linux-mtd, Ben Dooks,
	David Woodhouse
In-Reply-To: <fa686aa40906160553m2f0e7cc4n644c3893573845f@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 874 bytes --]

Am 16.06.09 14:53 schrieb(en) Grant Likely:
> I'm not happy about the use case though.  It probably shouldn't  
> appear in this binding, or if it does it should be tagged as an  
> optional property.

I agree with you that the naming is really misleading - other devices  
which are not mtd's may suffer from the same problem of the lpb (in my  
case, I have an extra memory-mapped Ethernet chip which I didn't try to  
access yet...).

As it is actually a chip select property, what about defining an  
optional property like "cs-width = (8|16|32)" which defaults to 8 and  
may be added to each 5200 lpb child?

> It is only in the 5200 localplus case that bank-width is needed to  
> figure out how to apply the workaround.

Just out of curiosity: what about the "localbus" of other Freescale  
chips (82xx? 83xx? Maybe others?)?

Thanks, Albrecht.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: PowerPC PCI DMA issues (prefetch/coherency?)
From: Arnd Bergmann @ 2009-06-16 17:43 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Scott Wood, Chris Pringle, linux-kernel
In-Reply-To: <4A37D073.6020802@freescale.com>

On Tuesday 16 June 2009, Scott Wood wrote:
> Chris Pringle wrote:
> > Ah right - that would explain what we're seeing then... Doh. Thought I 
> > might have been onto something then. Is there any way to force a cache 
> > flush? That'd at least prove it was a caching issue if it resolved the 
> > problem.
> 
> You could enable CONFIG_NOT_COHERENT_CACHE.

If the whole system is noncoherent, that is the right solution. If the
device is the only one, you can also use dma_alloc_noncoherent() and
flush explicitly with dma_cache_sync().

	Arnd <><

^ permalink raw reply

* Re: PowerPC PCI DMA issues (prefetch/coherency?)
From: Scott Wood @ 2009-06-16 17:49 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Chris Pringle, linuxppc-dev, linux-kernel
In-Reply-To: <200906161943.38764.arnd@arndb.de>

Arnd Bergmann wrote:
> On Tuesday 16 June 2009, Scott Wood wrote:
>> Chris Pringle wrote:
>>> Ah right - that would explain what we're seeing then... Doh. Thought I 
>>> might have been onto something then. Is there any way to force a cache 
>>> flush? That'd at least prove it was a caching issue if it resolved the 
>>> problem.
>> You could enable CONFIG_NOT_COHERENT_CACHE.
> 
> If the whole system is noncoherent, that is the right solution.

I meant it more as a test than a permanent solution...

> If the
> device is the only one, you can also use dma_alloc_noncoherent() and
> flush explicitly with dma_cache_sync().

I don't see how that would help -- aren't those also controlled by 
CONFIG_NOT_COHERENT_CACHE?

-Scott

^ permalink raw reply

* Re: PowerPC PCI DMA issues (prefetch/coherency?)
From: Arnd Bergmann @ 2009-06-16 18:02 UTC (permalink / raw)
  To: Scott Wood; +Cc: Chris Pringle, linuxppc-dev, linux-kernel
In-Reply-To: <4A37DB0F.7060801@freescale.com>

On Tuesday 16 June 2009, Scott Wood wrote:
> > If the
> > device is the only one, you can also use dma_alloc_noncoherent() and
> > flush explicitly with dma_cache_sync().
> 
> I don't see how that would help -- aren't those also controlled by 
> CONFIG_NOT_COHERENT_CACHE?

Ah, yes you are right. PowerPC implements dma_alloc_noncoherent as
dma_alloc_coherent, so dma_cache_sync() is actually a NOP (or should be).

Actually there seems to be a bug in here: Since dma_alloc_noncoherent
gives you a coherent mapping (or NULL) on noncoherent machines,
dma_cache_sync() is redundant and should not actually flush the
cache, or we should change dma_alloc_noncoherent to do a simple
alloc_pages on CONFIG_NON_COHERENT_CACHE and leave dma_cache_sync()
as it is.

	Arnd <><

^ permalink raw reply

* 2.6.30 kernel panic with any network driver
From: John Linn @ 2009-06-16 18:13 UTC (permalink / raw)
  To: linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 2177 bytes --]

I just merged in 2.6.30 into our tree and I'm seeing this only when I
have a network driver built into the kernel, and it doesn't seem to
matter which network driver.

 

Maybe I missed something I need to update in our network drivers to
match the kernel.

 

Thanks for any help,

John

 

console [ttyS0] enabled

brd: module loaded

loop: module loaded

Unable to handle kernel paging request for data at address 0x75686369

Faulting instruction address: 0xc01c3e34

Oops: Kernel access of bad area, sig: 11 [#1]

PREEMPT Xilinx Virtex440

Modules linked in:

NIP: c01c3e34 LR: c01c2ab4 CTR: 00000000

REGS: cf831dc0 TRAP: 0300   Not tainted  (2.6.30)

MSR: 00029000 <EE,ME,CE>  CR: 84284324  XER: 40000020

DEAR: 75686369, ESR: 00000000

TASK = cf82f8a0[1] 'swapper' THREAD: cf830000

GPR00: c01c2ab4 cf831e70 cf82f8a0 cf87b3a8 c034e5a4 00000000 c01c2a80
00000001

GPR08: cfbe69d8 c034dd98 00000f2b cf821778 84284342 fd9781f4 f6f6dff7
6adffffe

GPR16: d7f40173 bbd35ff7 3ef7fbff fff5d3f7 ffffffff 0059dac0 ffff006f
00000000

GPR24: 00000000 00000001 c034dd98 cf0416e0 cf87b3a0 cf87b3dc c034e5a4
75686369

NIP [c01c3e34] platform_match+0x20/0xa8

LR [c01c2ab4] __driver_attach+0x34/0xa8

Call Trace:

[cf831e70] [c027a394] klist_next+0x10c/0x120 (unreliable)

[cf831e90] [c01c2ab4] __driver_attach+0x34/0xa8

[cf831eb0] [c01c123c] bus_for_each_dev+0x5c/0x98

[cf831ee0] [c01c243c] driver_attach+0x24/0x34

[cf831ef0] [c01c2050] bus_add_driver+0x18c/0x264

[cf831f10] [c01c2fa8] driver_register+0x6c/0x170

[cf831f30] [c031f8cc] xemaclite_init+0x1c/0x4c

[cf831f50] [c00011ac] do_one_initcall+0x34/0x1a0

[cf831fd0] [c0307848] kernel_init+0xa0/0x104

[cf831ff0] [c000e188] kernel_thread+0x4c/0x68

Instruction dump:



This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.


[-- Attachment #2: Type: text/html, Size: 7908 bytes --]

^ permalink raw reply

* Re: [RFC PATCH] fsldma: Add DMA_SLAVE support
From: Dan Williams @ 2009-06-16 19:01 UTC (permalink / raw)
  To: Ira Snyder; +Cc: linuxppc-dev@ozlabs.org, Li Yang
In-Reply-To: <20090515225659.GD858@ovro.caltech.edu>

Hi Ira,

Ira Snyder wrote:
> Use the DMA_SLAVE capability of the DMAEngine API to copy/from a
> scatterlist into an arbitrary list of hardware address/length pairs.
> 
> This allows a single DMA transaction to copy data from several different
> devices into a scatterlist at the same time.
> 
> This also adds support to enable some controller-specific features such as
> external start and external pause of a DMA transaction.
> 
> Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
> ---
> 
> This is a request for comments on this patch. I hunch it is not quite
> ready for inclusion, though it is certainly ready for review. Correct
> functioning of this patch depends on the patches submitted earlier.
> 
> As suggested by Dan Williams, I implemented DMA_SLAVE support for the
> fsldma controller to allow me to use the hardware to transfer to/from a
> scatterlist to a list of hardware address/length pairs.
> 
> I implemented support for the extra features available in the DMA
> controller, such as external pause and external start. I have not tested
> the features yet. I am willing to drop the support if everything else
> looks good.
> 
> I have implemented helper functions for creating the list of hardware
> address/length pairs as static inline functions in the linux/fsldma.h
> header. Should I incorporate these into the driver itself and use
> EXPORT_SYMBOL()? I've never done this before :)

Using EXPORT_SYMBOL would defeat the purpose of conforming to the 
dmaengine api which should allow other subsystems to generically 
discover an fsldma resource.

> diff --git a/include/linux/fsldma.h b/include/linux/fsldma.h
> new file mode 100644
> index 0000000..a42dcdd
> --- /dev/null
> +++ b/include/linux/fsldma.h
> @@ -0,0 +1,105 @@
> +/*
> + * Freescale MPC83XX / MPC85XX DMA Controller
> + *
> + * Copyright (c) 2009 Ira W. Snyder <iws@ovro.caltech.edu>
> + *
> + * This file is licensed under the terms of the GNU General Public License
> + * version 2. This program is licensed "as is" without any warranty of any
> + * kind, whether express or implied.
> + */
> +
> +#ifndef __LINUX_FSLDMA_H__
> +#define __LINUX_FSLDMA_H__
> +
> +#include <linux/dmaengine.h>
> +
> +/*
> + * physical hardware address / length pair for use with the
> + * DMAEngine DMA_SLAVE API
> + */
> +struct fsl_dma_hw_addr {
> +       struct list_head entry;
> +
> +       dma_addr_t address;
> +       size_t length;
> +};

Can you explain a bit more why you need the new dma address list, would 
a struct scatterlist suffice?

In general it is difficult to merge new functionality without an in-tree 
user.  Can you share the client of this new api?

I suspect you can get away without needing these new helper routines. 
Have a look at how Haavard implemented his DMA_SLAVE client in 
drivers/mmc/host/atmel-mci.c.

Haavard ended up needing to add some public structure definitions to 
include/linux, but my preference is to keep this in an 
architecture/platform specific header file location if possible.

Regards,
Dan

^ permalink raw reply

* Re: MPC83xx watchdog reset board dead lock
From: David Hawkins @ 2009-06-16 19:02 UTC (permalink / raw)
  To: Leon Woestenberg; +Cc: Linux PPC
In-Reply-To: <c384c5ea0906160959v1fde484av8cff4916cacb84cc@mail.gmail.com>

Hi Leon,

>>> Most MPC8xxx board designs I have seen suffer from this possible dead
>>> lock:
>>> - NOR Flash is put in erase mode or write mode
>>> - Hardware watchdog triggers
>>> - HRESET# is asserted by the processor, during which the configuration
>>> words are read from NOR Flash.
>>>
>>> Either
>>> HRESET# is not attached to NOR, NOR stays in erase/write mode and
>>> invalid words will be read -> dead lock
>>>
>>> or either:
>>> HRESET# is attached to NOR reset, NOR is reset, but stays in reset as
>>> HRESET# stays asserted.
>>>
>>> We have been looking at several solutions hardware wise that reset the
>>> NOR flash on HRESET# going low, but the processors are stubborn,
>>> read the config words only once, than dead lock.
>>>
>>> I wonder if there are known-working designs for this.
>> What do you do in the case of blank flash on a board?
>>
> The problem is not with blank flash or firmware upgrades, we know how
> to handle that.
> 
> Your solution is (a solution) to a different problem.
> 
> The problem lies in the fact that board dead lock can occur if the
> watchdog triggers, for all reference designs I have seen.
> 
> Thanks for thinking along.  I would like to solve the original problem though.
> BTW, we use CPLD/FPGAs on most of our boards, this one we do not for
> cost reasons.

So we're talking about the sequence where HRESET# asserts as
in say the logic analyzer trace on p34:

http://www.ovro.caltech.edu/~dwh/carma_board/powerpc_mpc8349e.pdf

the LALE pulse where the processor reads the RCWs occurs very
soon after the falling edge of HRESET#.

So the Flash needs to be reset to ensure that it is in
read-array mode, so that the processor doesn't choke.
Since HRESET# is still low, thats no good. A pulse
generator that is based on HRESET# might work, but
the pulse would have to be long enough to meet any
reset requirement of the flash, yet short enough so
that the read of the first RCW would be valid. Since
the local bus is running really slow at this point,
I think that could be done ok.

How about a set-reset flip flop that is set on the
falling edge of HRESET# and cleared on the rising
edge of LALE. That'll produce a decent reset
pulse to the flash, and then there is plenty of
time for the first access to produce valid data
on the bus.

Cheers,
Dave

^ permalink raw reply

* Re: [RFC PATCH] fsldma: Add DMA_SLAVE support
From: Ira Snyder @ 2009-06-16 20:12 UTC (permalink / raw)
  To: Dan Williams; +Cc: linuxppc-dev@ozlabs.org, Li Yang
In-Reply-To: <4A37EC14.5010005@intel.com>

On Tue, Jun 16, 2009 at 12:01:40PM -0700, Dan Williams wrote:
> Hi Ira,
>
> Ira Snyder wrote:
>> Use the DMA_SLAVE capability of the DMAEngine API to copy/from a
>> scatterlist into an arbitrary list of hardware address/length pairs.
>>
>> This allows a single DMA transaction to copy data from several different
>> devices into a scatterlist at the same time.
>>
>> This also adds support to enable some controller-specific features such as
>> external start and external pause of a DMA transaction.
>>
>> Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
>> ---
>>
>> This is a request for comments on this patch. I hunch it is not quite
>> ready for inclusion, though it is certainly ready for review. Correct
>> functioning of this patch depends on the patches submitted earlier.
>>
>> As suggested by Dan Williams, I implemented DMA_SLAVE support for the
>> fsldma controller to allow me to use the hardware to transfer to/from a
>> scatterlist to a list of hardware address/length pairs.
>>
>> I implemented support for the extra features available in the DMA
>> controller, such as external pause and external start. I have not tested
>> the features yet. I am willing to drop the support if everything else
>> looks good.
>>
>> I have implemented helper functions for creating the list of hardware
>> address/length pairs as static inline functions in the linux/fsldma.h
>> header. Should I incorporate these into the driver itself and use
>> EXPORT_SYMBOL()? I've never done this before :)
>
> Using EXPORT_SYMBOL would defeat the purpose of conforming to the  
> dmaengine api which should allow other subsystems to generically  
> discover an fsldma resource.
>

Any driver would still use dma_request_channel(), etc. to get access to
a DMA channel. AFAICT, DMA_SLAVE is intended for doing something
completely hardware-specific with the DMA controller.

>> diff --git a/include/linux/fsldma.h b/include/linux/fsldma.h
>> new file mode 100644
>> index 0000000..a42dcdd
>> --- /dev/null
>> +++ b/include/linux/fsldma.h
>> @@ -0,0 +1,105 @@
>> +/*
>> + * Freescale MPC83XX / MPC85XX DMA Controller
>> + *
>> + * Copyright (c) 2009 Ira W. Snyder <iws@ovro.caltech.edu>
>> + *
>> + * This file is licensed under the terms of the GNU General Public License
>> + * version 2. This program is licensed "as is" without any warranty of any
>> + * kind, whether express or implied.
>> + */
>> +
>> +#ifndef __LINUX_FSLDMA_H__
>> +#define __LINUX_FSLDMA_H__
>> +
>> +#include <linux/dmaengine.h>
>> +
>> +/*
>> + * physical hardware address / length pair for use with the
>> + * DMAEngine DMA_SLAVE API
>> + */
>> +struct fsl_dma_hw_addr {
>> +       struct list_head entry;
>> +
>> +       dma_addr_t address;
>> +       size_t length;
>> +};
>
> Can you explain a bit more why you need the new dma address list, would  
> a struct scatterlist suffice?
>

I don't believe so. A scatterlist only holds page/length pairs. How
would you pass an arbitrary dma_addr_t/length pair in a scatterlist. I
/could/ abuse sg_dma_address() and do something like the following, but
I think you'd be even less inclined to take the patch:

struct scatterlist sg[10];

sg_dma_address(sg) = addr1;
sg_dma_len(sg) = len1;
sg++;
sg_dma_address(sg) = addr2;
sg_dma_len(sg) = len2;

/* and so on */

This would mean that there is a scatterlist with the struct page
pointers set to NULL, which has not had dma_map_sg() run on it. Seems
like abuse to me.

What I've implemented is this: (sorry about the poor drawing)

scatterlist           fsl_dma_hw_addr
+--------+            +-------+
|  DATA  | -------->  | DEST1 |
|  DATA  | ----+      +-------+
|  DATA  |     |
|  DATA  |     |      +-------+
|  DATA  |     +--->  | DEST2 |
|  DATA  |            +-------+
+--------+
                          .
                          .
                          .

Of course, the reverse works as well. You can copy from a list of
hardware address/length pairs to a scatterlist.

So, using my implementation of the DMA_SLAVE feature, you can take a big
chunk of data (which is organized into a scatterlist) and DMA it
directly to a set of hardware addresses, all in a single, unbroken
transaction.

I've got an FPGA programmer which needs a ~12MB image dumped to a FIFO
at 0xf0003000 in 4K chunks (all writes must be in the 0xf0003000 to
0xf0004000 range). The programmer is actually in control of the DMA
controller at that time. Internally, the FPGA programmer does some
toggling of pins, etc. which is needed to actually push the image into
the FPGA's themselves.

> In general it is difficult to merge new functionality without an in-tree  
> user.  Can you share the client of this new api?
>

I've inlined the driver for the FPGA programmer below. I don't think it
is appropriate to push into mainline, since it will only work for our
board, and nothing else.

It is pretty simple, but I'm totally open to suggestions for changes. I
used a char device to fill in a scatterlist, then set up the DMA to
0xf0003000 in 4K chunks.

I've got another driver that uses the interface, but this one is a bit
simpler. I can post the other one if you'd like, as well.

> I suspect you can get away without needing these new helper routines.  
> Have a look at how Haavard implemented his DMA_SLAVE client in  
> drivers/mmc/host/atmel-mci.c.
>
> Haavard ended up needing to add some public structure definitions to  
> include/linux, but my preference is to keep this in an  
> architecture/platform specific header file location if possible.
>

I was studying his code when I implemented this routine. He defined
struct dw_dma_slave in include/linux/dw_dmac.h. This is how he passes
information to device_prep_slave_sg() in the mmc driver.

It appears that his controller just uses a single register for
device-to-peripheral transfers. I implemented something different:
scatter/gather IO to/from a list of address/length pairs.

He also used platform data to get the register addresses. I'm unaware of
a way to put arbitrary platform data into the OF tree used on PowerPC.

I didn't want to force other users to implement the allocation routines
for the struct fsl_dma_hw_addr themselves, so I provided routines to do
so.

Thanks for the questions/comments. I feel like I've left something
unanswered, but I can't figure out what it is. Any more questions?
Ira


Some quick notes on the driver below:
carma_device_create()/carma_device_destroy() are just wrappers around
device_create()/device_destroy() which share a struct class, so that all
of the related drivers for FPGA's can go into /sys/class/carma/.

/*
 * CARMA Board Data FPGA Programmer
 *
 * Copyright (c) 2009 Ira W. Snyder <iws@ovro.caltech.edu>
 *
 * This file is licensed under the terms of the GNU General Public License
 * version 2. This program is licensed "as is" without any warranty of any
 * kind, whether express or implied.
 */

#include <linux/cdev.h>
#include <linux/completion.h>
#include <linux/delay.h>
#include <linux/dma-mapping.h>
#include <linux/highmem.h>
#include <linux/init.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/kernel.h>
#include <linux/leds.h>
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/of_platform.h>
#include <linux/dmaengine.h>

/* Freescale DMA Controller DMA_SLAVE interface */
#include <linux/fsldma.h>

/* MPC8349EMDS specific get_immrbase() */
#include <sysdev/fsl_soc.h>

/* CARMA device class */
#include "carma.h"

static const char driver_name[] = "fpga-program";

/*
 * Maximum firmware size
 *
 * 12849552 bytes for a CARMA Digitizer Board
 * XXXXXXXX bytes for a CARMA Correlator Board
 */
#define FW_SIZE_MAX (16 << 20)

struct fpga_fw_elem {
	struct page *page;
	unsigned int len;
	struct list_head entry;
};

struct fpga_dev {

	/* Character Device */
	struct cdev cdev;
	dev_t devno;

	/* Device Registers */
	struct device *dev;
	void __iomem *regs;
	void __iomem *immr;

	/* Freescale DMA Device */
	struct device *dmadev;
	struct dma_chan *chan;

	/* Interrupts */
	int irq, status;
	struct completion completion;

	/* FPGA Bitfile */
	struct mutex lock;
	struct list_head list;
	size_t bytes;
};

#define to_fpga_dev(X) container_of(X, struct fpga_dev, cdev)

/*----------------------------------------------------------------------------*/
/* FPGA Bitfile Element Allocation Helpers                                    */
/*----------------------------------------------------------------------------*/

static struct fpga_fw_elem *fpga_fw_elem_alloc(gfp_t gfp)
{
	struct fpga_fw_elem *elem;

	elem = kzalloc(sizeof(*elem), gfp);
	if (!elem)
		return NULL;

	elem->page = alloc_page(gfp);
	if (!elem->page) {
		kfree(elem);
		return NULL;
	}

	INIT_LIST_HEAD(&elem->entry);
	return elem;
}

static void fpga_fw_elem_free(struct fpga_fw_elem *elem)
{
	if (elem) {
		__free_page(elem->page);
		kfree(elem);
	}
}

/*----------------------------------------------------------------------------*/
/* FPGA Bitfile Helpers                                                       */
/*----------------------------------------------------------------------------*/

/*
 * Drop the firmware bitfile image from memory
 *
 * LOCKING: you must hold the priv->lock mutex
 *
 * @param priv the driver's private data structure
 */
static void fpga_drop_firmware_data(struct fpga_dev *priv)
{
	struct fpga_fw_elem *elem, *tmp;

	priv->bytes = 0;
	list_for_each_entry_safe(elem, tmp, &priv->list, entry) {
		list_del(&elem->entry);
		fpga_fw_elem_free(elem);
	}
}

static unsigned int list_num_entries(struct list_head *list)
{
	struct list_head *tmp;
	unsigned int num = 0;

	list_for_each(tmp, list)
		num++;

	return num;
}

/*----------------------------------------------------------------------------*/
/* LED Trigger (could be a seperate module)                                   */
/*----------------------------------------------------------------------------*/

/*
 * NOTE: this whole thing does have the problem that whenever the led's are
 * NOTE: first set to use the fpga trigger, they could be in the wrong state
 */

DEFINE_LED_TRIGGER(ledtrig_fpga);

static void ledtrig_fpga_programmed(bool enabled)
{
	if (enabled)
		led_trigger_event(ledtrig_fpga, LED_FULL);
	else
		led_trigger_event(ledtrig_fpga, LED_OFF);
}

/*----------------------------------------------------------------------------*/
/* FPGA Register Helpers                                                      */
/*----------------------------------------------------------------------------*/

/* Register Definitions */
#define FPGA_CONFIG_CONTROL		0x40
#define FPGA_CONFIG_STATUS		0x44
#define FPGA_CONFIG_FIFO_SIZE		0x48
#define FPGA_CONFIG_FIFO_USED		0x4C
#define FPGA_CONFIG_TOTAL_BYTE_COUNT	0x50
#define FPGA_CONFIG_CUR_BYTE_COUNT	0x54

#define FPGA_FIFO_ADDRESS		0x3000

static int fpga_fifo_size(void __iomem *regs)
{
	return ioread32be(regs + FPGA_CONFIG_FIFO_SIZE);
}

static int fpga_config_error(void __iomem *regs)
{
	return ioread32be(regs + FPGA_CONFIG_STATUS) & 0xFFFE;
}

static int fpga_fifo_empty(void __iomem *regs)
{
	return ioread32be(regs + FPGA_CONFIG_FIFO_USED) == 0;
}

static void fpga_fifo_write(void __iomem *regs, u32 val)
{
	iowrite32be(val, regs + FPGA_FIFO_ADDRESS);
}

static void fpga_set_byte_count(void __iomem *regs, u32 count)
{
	iowrite32be(count, regs + FPGA_CONFIG_TOTAL_BYTE_COUNT);
}

static void fpga_programmer_enable(struct fpga_dev *priv, bool dma)
{
	if (dma)
		iowrite32be(0x5, priv->regs + FPGA_CONFIG_CONTROL);
	else
		iowrite32be(0x1, priv->regs + FPGA_CONFIG_CONTROL);
}

static void fpga_programmer_disable(struct fpga_dev *priv)
{
	iowrite32be(0x0, priv->regs + FPGA_CONFIG_CONTROL);
}

#ifdef DEBUG
static void fpga_dump_registers(struct fpga_dev *priv)
{
	/* Dump all status registers */
	dev_info(priv->dev, "Configuration failed, dumping status registers\n");
	dev_info(priv->dev, "Control:    0x%.8x\n", ioread32be(priv->regs + FPGA_CONFIG_CONTROL));
	dev_info(priv->dev, "Status:     0x%.8x\n", ioread32be(priv->regs + FPGA_CONFIG_STATUS));
	dev_info(priv->dev, "FIFO Size:  0x%.8x\n", ioread32be(priv->regs + FPGA_CONFIG_FIFO_SIZE));
	dev_info(priv->dev, "FIFO Used:  0x%.8x\n", ioread32be(priv->regs + FPGA_CONFIG_FIFO_USED));
	dev_info(priv->dev, "FIFO Total: 0x%.8x\n", ioread32be(priv->regs + FPGA_CONFIG_TOTAL_BYTE_COUNT));
	dev_info(priv->dev, "FIFO Curr:  0x%.8x\n", ioread32be(priv->regs + FPGA_CONFIG_CUR_BYTE_COUNT));
}
#else
static void fpga_dump_registers(struct fpga_dev *priv)
{
}
#endif

/*----------------------------------------------------------------------------*/
/* FPGA Power Supply Code                                                     */
/*----------------------------------------------------------------------------*/

/*
 * Determine if the FPGA power is good for all supplies
 */
static bool fpga_power_good(struct fpga_dev *priv)
{
	const int addr[] = { 0x2006, 0x2008, 0x200A, 0x200D, };
	int i;

	for (i = 0; i < ARRAY_SIZE(addr); i++) {
		u8 val = ioread8(priv->regs + addr[i]);
		dev_dbg(priv->dev, "FPGA pgood 0x%.4x -> 0x%.2x\n", addr[i], val);

		if (!(val & 0x1))
			return false;
	}

	return true;
}

/*
 * Disable the FPGA power supplies
 */
static void fpga_disable_power_supplies(struct fpga_dev *priv)
{
	iowrite8(0x00, priv->regs + 0x2007); /* raw supply */
	iowrite8(0x00, priv->regs + 0x2009); /* 1.2v supply */
	iowrite8(0x00, priv->regs + 0x200B); /* 2.5v supply */
	iowrite8(0x00, priv->regs + 0x200E); /* 3.3v supply */
}

/*
 * Enable the FPGA power supplies
 *
 * @return 0 if the power went good, -ERRNO otherwise
 */
static int fpga_enable_power_supplies(struct fpga_dev *priv)
{
	unsigned long timeout;

	iowrite8(0x01, priv->regs + 0x2007); /* raw supply */
	iowrite8(0x7A, priv->regs + 0x2009); /* 1.2v supply */
	iowrite8(0x03, priv->regs + 0x200B); /* 2.5v supply */
	iowrite8(0x01, priv->regs + 0x200E); /* 3.3v supply */

	/* We'll give 1 second for the power supplies to enable */
	timeout = jiffies + HZ;

	while (!time_after(jiffies, timeout)) {
		if (fpga_power_good(priv))
			return 0;

		msleep(10);
	}

	/* Timed out, so disable the power supplies */
	fpga_disable_power_supplies(priv);

	return -ETIMEDOUT;
}

/*
 * Determine if the FPGA power supplies are all enabled
 */
static bool fpga_power_enabled(struct fpga_dev *priv)
{
	const int addr[] = { 0x2007, 0x2009, 0x200B, 0x200E, };
	const int vals[] = { 0x01, 0x7A, 0x03, 0x01, };
	int i;

	BUILD_BUG_ON(ARRAY_SIZE(addr) != ARRAY_SIZE(vals));

	/* Check each enable against the expected values */
	for (i = 0; i < ARRAY_SIZE(addr); i++) {
		if (ioread8(priv->regs + addr[i]) != vals[i])
			return false;
	}

	return true;
}

/*
 * Determine if the FPGA's are programmed and running correctly
 */
static bool fpga_running(struct fpga_dev *priv)
{
	if (!fpga_power_good(priv))
		return false;

	/* Check the config done bit */
	return ioread32be(priv->regs + FPGA_CONFIG_STATUS) & (1 << 18);
}

/*----------------------------------------------------------------------------*/
/* FPGA Programming Code                                                      */
/*----------------------------------------------------------------------------*/

/*
 * Program some data to the FPGA fifo
 *
 * @priv the private data
 * @buf the data to program
 * @count the length of data to program (must be a multiple of 4 bytes)
 *
 * @return 0 on success, -ERRNO otherwise
 */
static int fpga_program_block(struct fpga_dev *priv, void *buf, size_t count)
{
	u32 *data = buf;
	int size = fpga_fifo_size(priv->regs);
	int i, len;
	unsigned long timeout;

	/* FIXME: BUG_ON instead */
	WARN_ON_ONCE(count % 4 != 0);

	while (count > 0) {

		/* Get the size of the block to write (maximum is FIFO_SIZE) */
		len = min_t(size_t, count, size);
		timeout = jiffies + HZ / 4;

		/* Write the block */
		for (i = 0; i < len / 4; i++)
			fpga_fifo_write(priv->regs, data[i]);

		/* Update the amounts left */
		count -= len;
		data += len / 4;

		/* Wait for the fifo to empty */
		while (true) {

			if (fpga_fifo_empty(priv->regs)) {
				break;
			} else {
				dev_dbg(priv->dev, "Fifo not empty\n");
				cpu_relax();
			}

			if (fpga_config_error(priv->regs)) {
				dev_err(priv->dev, "Error detected\n");
				return -EIO;
			}

			if (time_after(jiffies, timeout)) {
				dev_err(priv->dev, "Fifo timed out\n");
				return -ETIMEDOUT;
			}

			msleep(10);
		}
	}

	return 0;
}

/*
 * Program the FPGA's using the CPU
 *
 * @param priv the driver's private data structure
 * @return 0 on success, -ERRNO otherwise
 */
static noinline int fpga_program_cpu(struct fpga_dev *priv)
{
	struct fpga_fw_elem *elem;
	void *data;
	int ret;

	/* Disable the programmer */
	fpga_programmer_disable(priv);

	/* Set the total byte count */
	fpga_set_byte_count(priv->regs, priv->bytes);
	dev_dbg(priv->dev, "total byte count %u bytes\n", priv->bytes);

	/* Enable the controller for programming */
	fpga_programmer_enable(priv, false);
	dev_dbg(priv->dev, "enabled the controller\n");

	/* Write each chunk of the FPGA bitfile to FPGA programmer */
	list_for_each_entry(elem, &priv->list, entry) {
		data = kmap(elem->page);
		ret = fpga_program_block(priv, data, elem->len);
		kunmap(elem->page);

		if (ret)
			goto out_disable_controller;
	}

	/* Wait for the interrupt handler to notify us that programming finished */
	ret = wait_for_completion_timeout(&priv->completion, 2 * HZ);
	if (!ret) {
		dev_err(priv->dev, "Timed out waiting for completion\n");
		ret = -ETIMEDOUT;
		goto out_disable_controller;
	}

	/* Retrieve the status from the interrupt handler */
	ret = priv->status;

out_disable_controller:
	fpga_programmer_disable(priv);
	return ret;
}

/*
 * Program the FPGA's using the DMA controller
 *
 * @param priv the driver's private data structure
 * @return 0 on success, -ERRNO otherwise
 */
static noinline int fpga_program_dma(struct fpga_dev *priv)
{
	struct dma_chan *chan = priv->chan;
	struct dma_async_tx_descriptor *tx;
	struct fsl_dma_slave *slave;
	struct sg_table table;
	dma_cookie_t cookie;
	unsigned int nents, npages;
	size_t len, avail = 0;
	int ret;
	struct scatterlist *sg;
	struct fpga_fw_elem *elem;

	/* Disable the programmer */
	fpga_programmer_disable(priv);
	chan->device->device_terminate_all(chan);

	/* Allocate the DMA_SLAVE structure */
	slave = fsl_dma_slave_alloc(GFP_KERNEL);
	if (!slave) {
		dev_err(priv->dev, "Unable to allocate DMA_SLAVE structure\n");
		ret = -ENOMEM;
		goto out_return;
	}

	/* Set the DMA controller in external start mode */
	slave->external_start = true;

	/* Allocate the SG table */
	npages = list_num_entries(&priv->list);
	ret = sg_alloc_table(&table, npages, GFP_KERNEL);
	if (ret) {
		dev_err(priv->dev, "Unable to allocate SG table\n");
		goto out_free_slave;
	}

	/* Fill the SG table */
	sg = table.sgl;
	list_for_each_entry(elem, &priv->list, entry) {
		sg_set_page(sg, elem->page, elem->len, 0);
		sg = sg_next(sg);
	}

	/* Map the SG table for DMA */
	nents = dma_map_sg(priv->dmadev, table.sgl, npages, DMA_TO_DEVICE);
	if (nents <= 0) {
		dev_err(priv->dev, "Unable to DMA map SG table\n");
		ret = -ENOMEM;
		goto out_free_table;
	}

	/* Append the addresses to the DMA_SLAVE structure */
	avail = priv->bytes;
	while (avail > 0) {
		len = min_t(size_t, avail, PAGE_SIZE);
		ret = fsl_dma_slave_append(slave, 0xf0003000, len);
		if (ret) {
			dev_err(priv->dev, "Unable to append FIFO address\n");
			goto out_unmap_table;
		}

		avail -= len;
	}

	/* Submit the DMA slave */
	chan->private = slave;
	tx = chan->device->device_prep_slave_sg(chan, table.sgl, nents,
						DMA_TO_DEVICE, 0);
	if (!tx) {
		dev_err(priv->dev, "Unable to prep DMA_SLAVE transaction\n");
		ret = -ENOMEM;
		goto out_unmap_table;
	}

	/*
	 * Submit the transaction to the DMA controller
	 *
	 * We would leak memory if the submission failed, but that doesn't
	 * happen in the fsldma driver, so it isn't an issue
	 */
	cookie = tx->tx_submit(tx);
	if (dma_submit_error(cookie)) {
		dev_err(priv->dev, "Unable to submit DMA_SLAVE transaction\n");
		ret = -ENOMEM;
		goto out_unmap_table;
	}

	dma_async_memcpy_issue_pending(chan);

	/* Set the total byte count */
	fpga_set_byte_count(priv->regs, priv->bytes);
	dev_dbg(priv->dev, "total byte count %u bytes\n", priv->bytes);

	/* Enable the controller for DMA programming */
	fpga_programmer_enable(priv, true);
	dev_dbg(priv->dev, "enabled the controller\n");

	/* Wait for the interrupt handler to notify us that programming finished */
	ret = wait_for_completion_timeout(&priv->completion, 2 * HZ);
	if (!ret) {
		dev_err(priv->dev, "Timed out waiting for completion\n");
		ret = -ETIMEDOUT;
		goto out_disable_controller;
	}

	/* Retrieve the status from the interrupt handler */
	ret = priv->status;

out_disable_controller:
	fpga_programmer_disable(priv);
	chan->device->device_terminate_all(chan);
out_unmap_table:
	dma_unmap_sg(priv->dmadev, table.sgl, npages, DMA_TO_DEVICE);
out_free_table:
	sg_free_table(&table);
out_free_slave:
	fsl_dma_slave_free(slave);
out_return:
	return ret;
}

/*----------------------------------------------------------------------------*/
/* Interrupt Handling                                                         */
/*----------------------------------------------------------------------------*/

static irqreturn_t fpga_interrupt(int irq, void *dev_id)
{
	struct fpga_dev *priv = dev_id;

	/* Save the status */
	priv->status = fpga_config_error(priv->regs) ? -EIO : 0;
	dev_dbg(priv->dev, "INTERRUPT status %d\n", priv->status);
	fpga_dump_registers(priv);

	/* Disabling the programmer clears the interrupt */
	fpga_programmer_disable(priv);

	/* Notify any waiters */
	complete(&priv->completion);

	return IRQ_HANDLED;
}

/*----------------------------------------------------------------------------*/
/* SYSFS Helpers                                                              */
/*----------------------------------------------------------------------------*/

static int fpga_do_stop(struct fpga_dev *priv)
{
	/* TODO: anything else here ? */

	/* Set the led to unprogrammed */
	ledtrig_fpga_programmed(false);

	return 0;
}

static noinline int fpga_do_program(struct fpga_dev *priv)
{
	int ret;

	if (list_empty(&priv->list)) {
		dev_err(priv->dev, "No data to program\n");
		return -EINVAL;
	}

	if (!fpga_power_enabled(priv)) {
		dev_err(priv->dev, "Power not enabled\n");
		return -EINVAL;
	}

	if (!fpga_power_good(priv)) {
		dev_err(priv->dev, "Power not good\n");
		return -EINVAL;
	}

	/* Try to program the FPGA's using DMA */
	ret = fpga_program_dma(priv);

	/* If DMA failed or doesn't exist, try with CPU */
	if (ret) {
		dev_warn(priv->dev, "Falling back to CPU programming\n");
		ret = fpga_program_cpu(priv);
	}

	if (ret) {
		dev_err(priv->dev, "Unable to program FPGA's\n");
		return ret;
	}

	/* Drop the firmware bitfile from memory */
	fpga_drop_firmware_data(priv);

	dev_dbg(priv->dev, "FPGA programming successful\n");
	ledtrig_fpga_programmed(true);

	return 0;
}

/*----------------------------------------------------------------------------*/
/* File Operations                                                            */
/*----------------------------------------------------------------------------*/

static int fpga_open(struct inode *inode, struct file *filp)
{
	struct fpga_dev *priv = to_fpga_dev(inode->i_cdev);

	/* We only allow one process at a time */
	if (mutex_lock_interruptible(&priv->lock))
		return -ERESTARTSYS;

	filp->private_data = priv;

	/* Free any data left over from a previous open (if any) */
	fpga_drop_firmware_data(priv);

	return nonseekable_open(inode, filp);
}

static int fpga_release(struct inode *inode, struct file *filp)
{
	struct fpga_dev *priv = filp->private_data;

	mutex_unlock(&priv->lock);
	return 0;
}

static ssize_t fpga_write(struct file *filp, const char __user *buf,
			  size_t count, loff_t *f_pos)
{
	struct fpga_dev *priv = filp->private_data;
	struct fpga_fw_elem *elem, *tmp;
	size_t used, len, copy;
	struct list_head list;
	void *mem;
	int ret;

	/* Disallow firmware images larger than 16MB */
	if (priv->bytes >= FW_SIZE_MAX)
		return -ENOSPC;

	count = min_t(size_t, FW_SIZE_MAX - priv->bytes, count);

	INIT_LIST_HEAD(&list);
	len = count;
	used = 0;

	while (len > 0) {

		/* Allocate a list element */
		elem = fpga_fw_elem_alloc(GFP_KERNEL);
		if (!elem) {
			count = used;
			goto out_success;
		}

		/* Copy the data from userspace */
		copy = min_t(size_t, PAGE_SIZE, len);
		mem = kmap(elem->page);
		ret = copy_from_user(mem, buf + used, copy);
		kunmap(elem->page);

		if (ret) {
			count = -EFAULT;
			goto out_cleanup;
		}

		elem->len = copy;
		list_add_tail(&elem->entry, &list);

		len -= copy;
		used += copy;
	}

out_success:
	if (list_empty(&list))
		return -ENOMEM;

	list_splice_tail_init(&list, &priv->list);
	priv->bytes += count;

	*f_pos += count;
	return count;

out_cleanup:
	/* Free the last allocated element */
	fpga_fw_elem_free(elem);

	/* Free all of the elements on the temporary list */
	list_for_each_entry_safe(elem, tmp, &list, entry) {
		list_del(&elem->entry);
		fpga_fw_elem_free(elem);
	}

	return count;
}

static const struct file_operations fpga_fops = {
	.owner		= THIS_MODULE,
	.open		= fpga_open,
	.release	= fpga_release,
	.write		= fpga_write,
	.llseek		= no_llseek,
};

/*----------------------------------------------------------------------------*/
/* Device Attributes                                                          */
/*----------------------------------------------------------------------------*/

static ssize_t pgood_show(struct device *dev, struct device_attribute *attr,
			  char *buf)
{
	struct fpga_dev *priv = dev_get_drvdata(dev);
	return snprintf(buf, PAGE_SIZE, "%d\n", fpga_power_good(priv));
}

static ssize_t penable_show(struct device *dev, struct device_attribute *attr,
			    char *buf)
{
	struct fpga_dev *priv = dev_get_drvdata(dev);
	return snprintf(buf, PAGE_SIZE, "%d\n", fpga_power_enabled(priv));
}

static ssize_t penable_store(struct device *dev, struct device_attribute *attr,
			     const char *buf, size_t count)
{
	struct fpga_dev *priv = dev_get_drvdata(dev);
	unsigned long val;

	if (strict_strtoul(buf, 0, &val))
		return -EINVAL;

	if (val) {
		fpga_enable_power_supplies(priv);
	} else {
		fpga_do_stop(priv);
		fpga_disable_power_supplies(priv);
	}

	return count;
}

static ssize_t program_show(struct device *dev, struct device_attribute *attr,
			    char *buf)
{
	struct fpga_dev *priv = dev_get_drvdata(dev);
	return snprintf(buf, PAGE_SIZE, "%d\n", fpga_running(priv));
}

static ssize_t program_store(struct device *dev, struct device_attribute *attr,
			     const char *buf, size_t count)
{
	struct fpga_dev *priv = dev_get_drvdata(dev);
	unsigned long val;
	int ret;

	if (strict_strtoul(buf, 0, &val))
		return -EINVAL;

	/* We can't have an image writer and be programming simultaneously */
	if (mutex_lock_interruptible(&priv->lock))
		return -ERESTARTSYS;

	if (val) {
		ret = fpga_do_program(priv);
		if (ret)
			goto out_unlock;
	}

	ret = count;

out_unlock:
	mutex_unlock(&priv->lock);
	return ret;
}

static DEVICE_ATTR(power_good, S_IRUGO, pgood_show, NULL);
static DEVICE_ATTR(power_enable, S_IRUGO | S_IWUSR, penable_show, penable_store);
static DEVICE_ATTR(program, S_IRUGO | S_IWUSR, program_show, program_store);

static struct attribute *fpga_attributes[] = {
	&dev_attr_power_good.attr,
	&dev_attr_power_enable.attr,
	&dev_attr_program.attr,
	NULL,
};

static const struct attribute_group fpga_attr_group = {
	.attrs = fpga_attributes,
};

/*----------------------------------------------------------------------------*/
/* OpenFirmware Device Subsystem                                              */
/*----------------------------------------------------------------------------*/

static bool dma_filter(struct dma_chan *chan, void *data)
{
	/*
	 * DMA Channel #0 is the only acceptable device
	 *
	 * This probably won't survive an unload/load cycle of the Freescale
	 * DMAEngine driver, but that won't be a problem
	 */
	return chan->chan_id == 0 && chan->device->dev_id == 0;
}

static int fpga_of_remove(struct of_device *op)
{
	struct fpga_dev *priv = dev_get_drvdata(&op->dev);

	sysfs_remove_group(&priv->dev->kobj, &fpga_attr_group);

	cdev_del(&priv->cdev);
	free_irq(priv->irq, priv);

	iounmap(priv->immr);

	fpga_disable_power_supplies(priv);
	iounmap(priv->regs);

	dma_release_channel(priv->chan);
	carma_device_destroy(priv->devno);
	unregister_chrdev_region(priv->devno, 1);

	/* Free any firmware image that has not been programmed */
	fpga_drop_firmware_data(priv);

	mutex_destroy(&priv->lock);
	kfree(priv);

	return 0;
}

static int fpga_of_probe(struct of_device *op, const struct of_device_id *match)
{
	struct fpga_dev *priv;
	dma_cap_mask_t mask;
	u32 val;
	int ret;

	/* Allocate private data */
	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
	if (!priv) {
		dev_err(&op->dev, "Unable to allocate private data\n");
		ret = -ENOMEM;
		goto out_return;
	}

	dev_set_drvdata(&op->dev, priv);
	mutex_init(&priv->lock);
	init_completion(&priv->completion);
	cdev_init(&priv->cdev, &fpga_fops);
	priv->dmadev = &op->dev;
	INIT_LIST_HEAD(&priv->list);

	/* Allocate the character device */
	ret = alloc_chrdev_region(&priv->devno, 0, 1, driver_name);
	if (ret) {
		dev_err(&op->dev, "Unable to allocate chardev region\n");
		goto out_free_priv;
	}

	/* Allocate the CARMA device */
	priv->dev = carma_device_create(&op->dev, priv->devno, driver_name);
	if (IS_ERR(priv->dev)) {
		dev_err(&op->dev, "Unable to create CARMA device\n");
		ret = PTR_ERR(priv->dev);
		goto out_unregister_chrdev_region;
	}

	dev_set_drvdata(priv->dev, priv);
	dma_cap_zero(mask);
	dma_cap_set(DMA_MEMCPY, mask);
	dma_cap_set(DMA_INTERRUPT, mask);
	dma_cap_set(DMA_SLAVE, mask);

	/* Get control of DMA channel #0 */
	priv->chan = dma_request_channel(mask, dma_filter, NULL);
	if (!priv->chan) {
		dev_err(&op->dev, "Unable to acquire DMA channel #0\n");
		goto out_carma_device_destroy;
	}

	/* Remap the registers for use */
	priv->regs = of_iomap(op->node, 0);
	if (!priv->regs) {
		dev_err(&op->dev, "Unable to ioremap registers\n");
		ret = -ENOMEM;
		goto out_dma_release_channel;
	}

	/* Remap the IMMR for use */
	priv->immr = ioremap(get_immrbase(), 0x100000);
	if (!priv->immr) {
		dev_err(&op->dev, "Unable to ioremap IMMR\n");
		ret = -ENOMEM;
		goto out_unmap_regs;
	}

	/*
	 * Enable external DMA control pins
	 *
	 * WARNING: this must be done before attempting to set up the DMA
	 * WARNING: controller for externally initiated transfers. The default
	 * WARNING: state of the DMA control pins is incorrect for proper
	 * WARNING: operation of the FPGA programmer!!!
	 *
	 * NOTE: 2009-06-12: the U-Boot bootloader handles this now
	 *
	 * Failing to do so will cause the DMA controller to transfer a single
	 * cacheline worth of data, and then wedge itself.
	 */
	iowrite32be(0x80000E00, priv->immr + 0x114);

	/*
	 * TODO: make a better interface for this
	 *
	 * The DMA controller is not being used by the driver at this point,
	 * so we will just add the bits we need, and the driver will not alter
	 * them. This sets the following values:
	 *
	 * DMA Request Count: 8 cache lines
	 * Bandwidth Control: 8 cache lines
	 *
	 * These request count setting is necessary for the programmer to work
	 * in external start mode. The bandwidth control setting is optional.
	 */
	val = ioread32(priv->immr + 0x8100);
	iowrite32(val | 0x08600000, priv->immr + 0x8100);

	/* Enable the FPGA Programmer Interrupt */
	iowrite32be(0x1, priv->regs + 0x24); /* IRQ0 internal control */
	iowrite32be(0x1, priv->regs + 0x28); /* IRQ0 enabled */

	/* Find the correct IRQ number */
	priv->irq = irq_of_parse_and_map(op->node, 0);
	ret = request_irq(priv->irq, fpga_interrupt, IRQF_SHARED, driver_name, priv);
	if (ret) {
		dev_err(&op->dev, "Unable to request IRQ %d\n", priv->irq);
		ret = -ENODEV;
		goto out_unmap_immr;
	}

	/* Reset and stop the FPGA's, just in case */
	fpga_do_stop(priv);

	/* Enable the FPGA power supplies */
	ret = fpga_enable_power_supplies(priv);
	if (ret) {
		dev_err(&op->dev, "Unable to enable FPGA power supplies\n");
		ret = -ENODEV;
		goto out_free_irq;
	}

	/* Register the character device */
	ret = cdev_add(&priv->cdev, priv->devno, 1);
	if (ret) {
		dev_err(&op->dev, "Unable to add character device\n");
		goto out_disable_power_supplies;
	}

	/* Create the sysfs files */
	ret = sysfs_create_group(&priv->dev->kobj, &fpga_attr_group);
	if (ret) {
		dev_err(&op->dev, "Unable to create sysfs files\n");
		goto out_cdev_del;
	}

	dev_info(priv->dev, "CARMA FPGA Programmer Driver loaded\n");
	return 0;

out_cdev_del:
	cdev_del(&priv->cdev);
out_disable_power_supplies:
	fpga_disable_power_supplies(priv);
out_free_irq:
	free_irq(priv->irq, priv);
out_unmap_immr:
	iounmap(priv->immr);
out_unmap_regs:
	iounmap(priv->regs);
out_dma_release_channel:
	dma_release_channel(priv->chan);
out_carma_device_destroy:
	carma_device_destroy(priv->devno);
out_unregister_chrdev_region:
	unregister_chrdev_region(priv->devno, 1);
out_free_priv:
	mutex_destroy(&priv->lock);
	kfree(priv);
out_return:
	return ret;
}

static struct of_device_id fpga_of_match[] = {
	{ .compatible = "carma,fpga-programmer", },
	{},
};

static struct of_platform_driver fpga_of_driver = {
	.owner		= THIS_MODULE,
	.name		= driver_name,
	.match_table	= fpga_of_match,
	.probe		= fpga_of_probe,
	.remove		= fpga_of_remove,
};

/*----------------------------------------------------------------------------*/
/* Module Init / Exit                                                         */
/*----------------------------------------------------------------------------*/

static int __init fpga_init(void)
{
	led_trigger_register_simple("fpga", &ledtrig_fpga);
	return of_register_platform_driver(&fpga_of_driver);
}

static void __exit fpga_exit(void)
{
	of_unregister_platform_driver(&fpga_of_driver);
	led_trigger_unregister_simple(ledtrig_fpga);
}

MODULE_AUTHOR("Ira W. Snyder <iws@ovro.caltech.edu>");
MODULE_DESCRIPTION("CARMA FPGA Programmer Driver");
MODULE_LICENSE("GPL");

module_init(fpga_init);
module_exit(fpga_exit);

^ permalink raw reply

* [PATCH RFC 1/2] Makefile: Never use -fno-omit-frame-pointer
From: Anton Vorontsov @ 2009-06-16 20:16 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, linuxppc-dev, Steven Rostedt, Sam Ravnborg

According to Segher Boessenkool and GCC manual, -fomit-frame-pointer
is only the default when optimising on archs/ABIs where it doesn't
hinder debugging and -pg. So, we do not get it by default on x86,
not at any optimisation level.

On the other hand, *using* -fno-omit-frame-pointer causes gcc to
produce buggy code on PowerPC targets.

If Segher and GCC manual are right, this patch should be a no-op
for all arches except PowerPC, where the patch fixes gcc issues.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---

See this thread for more discussion:
http://osdir.com/ml/linux-kernel/2009-05/msg01754.html

p.s.
Obviously, I didn't test this patch on anything else but PPC32. ;-)

Segher, do you know if all GCC versions that we support for
building Linux are behaving the way that GCC manual describe?

Thanks,

 Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index ea63667..70ad1ff 100644
--- a/Makefile
+++ b/Makefile
@@ -535,7 +535,7 @@ KBUILD_CFLAGS += $(call cc-option, -fno-stack-protector)
 endif
 
 ifdef CONFIG_FRAME_POINTER
-KBUILD_CFLAGS	+= -fno-omit-frame-pointer -fno-optimize-sibling-calls
+KBUILD_CFLAGS	+= -fno-optimize-sibling-calls
 else
 KBUILD_CFLAGS	+= -fomit-frame-pointer
 endif
-- 
1.6.3.1

^ permalink raw reply related

* [PATCH RFC 2/2] powerpc: Remove -fno-omit-frame-pointer workarounds
From: Anton Vorontsov @ 2009-06-16 20:16 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, linuxppc-dev, Steven Rostedt, Sam Ravnborg

The workarounds aren't needed any longer since the top level Makefile
doesn't pass -fno-omit-frame-pointer cflag for PowerPC builds.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/Makefile                    |    5 -----
 arch/powerpc/kernel/Makefile             |   12 ++++++------
 arch/powerpc/platforms/powermac/Makefile |    2 +-
 lib/Kconfig.debug                        |    6 +++---
 4 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index bc35f4e..dd3c63b 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -120,11 +120,6 @@ ifeq ($(CONFIG_6xx),y)
 KBUILD_CFLAGS		+= -mcpu=powerpc
 endif
 
-# Work around a gcc code-gen bug with -fno-omit-frame-pointer.
-ifeq ($(CONFIG_FUNCTION_TRACER),y)
-KBUILD_CFLAGS		+= -mno-sched-epilog
-endif
-
 cpu-as-$(CONFIG_4xx)		+= -Wa,-m405
 cpu-as-$(CONFIG_6xx)		+= -Wa,-maltivec
 cpu-as-$(CONFIG_POWER4)		+= -Wa,-maltivec
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 612b0c4..19ebe1b 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -14,14 +14,14 @@ endif
 
 ifdef CONFIG_FUNCTION_TRACER
 # Do not trace early boot code
-CFLAGS_REMOVE_cputable.o = -pg -mno-sched-epilog
-CFLAGS_REMOVE_prom_init.o = -pg -mno-sched-epilog
-CFLAGS_REMOVE_btext.o = -pg -mno-sched-epilog
-CFLAGS_REMOVE_prom.o = -pg -mno-sched-epilog
+CFLAGS_REMOVE_cputable.o = -pg
+CFLAGS_REMOVE_prom_init.o = -pg
+CFLAGS_REMOVE_btext.o = -pg
+CFLAGS_REMOVE_prom.o = -pg
 # do not trace tracer code
-CFLAGS_REMOVE_ftrace.o = -pg -mno-sched-epilog
+CFLAGS_REMOVE_ftrace.o = -pg
 # timers used by tracing
-CFLAGS_REMOVE_time.o = -pg -mno-sched-epilog
+CFLAGS_REMOVE_time.o = -pg
 endif
 
 obj-y				:= cputable.o ptrace.o syscalls.o \
diff --git a/arch/powerpc/platforms/powermac/Makefile b/arch/powerpc/platforms/powermac/Makefile
index 50f1693..0eb8781 100644
--- a/arch/powerpc/platforms/powermac/Makefile
+++ b/arch/powerpc/platforms/powermac/Makefile
@@ -2,7 +2,7 @@ CFLAGS_bootx_init.o  		+= -fPIC
 
 ifdef CONFIG_FUNCTION_TRACER
 # Do not trace early boot code
-CFLAGS_REMOVE_bootx_init.o = -pg -mno-sched-epilog
+CFLAGS_REMOVE_bootx_init.o = -pg
 endif
 
 obj-y				+= pic.o setup.o time.o feature.o pci.o \
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 39f113a..1479e54 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -492,7 +492,7 @@ config LOCKDEP
 	bool
 	depends on DEBUG_KERNEL && TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
 	select STACKTRACE
-	select FRAME_POINTER if !MIPS && !PPC && !ARM_UNWIND && !S390
+	select FRAME_POINTER if !MIPS && !ARM_UNWIND && !S390
 	select KALLSYMS
 	select KALLSYMS_ALL
 
@@ -861,13 +861,13 @@ config FAULT_INJECTION_STACKTRACE_FILTER
 	depends on FAULT_INJECTION_DEBUG_FS && STACKTRACE_SUPPORT
 	depends on !X86_64
 	select STACKTRACE
-	select FRAME_POINTER if !PPC && !S390
+	select FRAME_POINTER if !S390
 	help
 	  Provide stacktrace filter for fault-injection capabilities
 
 config LATENCYTOP
 	bool "Latency measuring infrastructure"
-	select FRAME_POINTER if !MIPS && !PPC && !S390
+	select FRAME_POINTER if !MIPS && !S390
 	select KALLSYMS
 	select KALLSYMS_ALL
 	select STACKTRACE
-- 
1.6.3.1

^ permalink raw reply related

* Re: 2.6.30 kernel panic with any network driver
From: Scott Wood @ 2009-06-16 20:29 UTC (permalink / raw)
  To: John Linn; +Cc: linuxppc-dev
In-Reply-To: <20090616181334.1EBABA8056@mail2-sin.bigfish.com>

John Linn wrote:
> console [ttyS0] enabled
> 
> brd: module loaded
> 
> loop: module loaded
> 
> Unable to handle kernel paging request for data at address 0x75686369

That address spells "uhci", FWIW.  Maybe a bad pointer cast somewhere in 
the USB platform data?

-Scott

^ permalink raw reply

* Re: [PATCH] RFC: powerpc: expose the multi-bit ops that underlie single-bit ops.
From: Benjamin Herrenschmidt @ 2009-06-16 21:33 UTC (permalink / raw)
  To: Geoff Thorpe; +Cc: linuxppc-dev
In-Reply-To: <4A37AC09.1020200@freescale.com>

On Tue, 2009-06-16 at 10:28 -0400, Geoff Thorpe wrote:

> > Hi ! Sorry for the delay, that was on my "have a look one of these days,
> > low priority" list for a bit too long :-)
> 
> NP, optimal throughput often requires a compromise in latency :-)

Hehehe, so true :-)
 
> > I'm not sure it's useful to provide a multi-bit variant of the
> > "lock" and "unlock" primitives. Do other archs do ?
> 
> For clear_bit_unlock(), no they don't appear to. There is a fallback in
> include/asm-generic though, and I notice that it's used in a few places,
> eg. drivers/rtc/rtc-dev. OTOH some other archs appear to provide their
> own test_and_set_bit_lock(), and there's a fallback in
> includes/asm-generic for that too.
> 
> Do you see a reason to isolate either of these and not factor out the
> inner word-based logic?

Don't bother, especially if we are using a macro to generate them, we
may as well make them all look the same.

 .../...

> Yup, believe it or not, I saw this coming but didn't have the guts to
> try proposing something like it up-front (in particular, I was wary of
> botching some subtleties in the assembly).

Fair enough :-)

> > Maybe we can shrink that file significantly (and avoid the risk for
> > typos etc...) by generating them all from a macro.
> > 
> > Something like (typed directly into the mailer :-)
> > 
> > #define DEFINE_BITOP(op, prefix, postfix) \
> > 	asm volatile (			  \
> > 	prefix				  \
> > "1:"    PPC_LLARX "%0,0,%3\n"		  \
> > 	__stringify(op) "%1,%0,%2\n"	  \
> > 	PPC405_ERR77(0,%3)		  \
> > 	PPC_STLCX "%1,0,%3\n"		  \
> > 	"bne- 1b\n"			  \
> > 	postfix				  \
> > 	 : "=&r" (old), "=&r" (t)
> > 	 : "r" (mask), "r" (p)
> > 	 : "cc", "memory")
> > 
> > and so:
> > 
> > static inline void set_bits(unsigned long mask, volatile unsigned long *addr)
> > {
> > 	unsigned long old, t;
> > 
> > 	DEFINE_BITOP(or, "", "");
> > }
> > 
> > static inline void test_and_set_bits(unsigned long mask, volatile unsigned long *addr)
> > {
> > 	unsigned long old, t;
> > 
> > 	DEFINE_BITOP(or, LWSYNC_ON_SMP, ISYNC_ON_SMP);
> > 
> > 	return (old & mask) != 0;
> > }
> > 
> > etc...
> 
> 
> Sounds good, I'll try working this up and I'll send a new patch shortly.

You can also go totally mad and generate the whole function (both -s and
non -s variants) from one macro but I wouldn't go that far :-)

> So can I assume implicitly that changing the set_bits() function to add
> the 'volatile' qualifier to the prototype (and the missing
> PPC405_ERR77() workaround) was OK?

The PPC405_ERR77 workaround is definitely needed. The volatile, well, I
suspect it's useless, but it will remove warnings when callers call
these on something that is declared as volatile in the first place.

Do x86 use volatile there ? If not, then don't do it on powerpc neither,
it could well be an historical remain. It's not functionally useful, the
"memory" clobber in the asm takes care of telling the compiler not to
mess around I believe.

> Also - any opinion on whether the same re-factoring of the asm-generic
> versions should be undertaken? I'm not looking to bite off more than I
> can chew, but I don't know if it's frowned upon to make powerpc-only
> extensions to the API. And if you think an asm-generic patch makes
> sense, could that be taken via linuxppc-dev too or does it need to go
> elsewhere?

I'm not people care -that- much :-) You can always try and post it to
lkml (maybe linux-arch too) and see what comes back... but let's finish
the powerpc variant first :-)

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH 1/2] lib: Provide generic atomic64_t implementation
From: Gabriel Paubert @ 2009-06-16 22:27 UTC (permalink / raw)
  To: Avi Kivity
  Cc: Linus Torvalds, akpm, Paul Mackerras, linux-kernel, linuxppc-dev
In-Reply-To: <4A34F564.2010500@redhat.com>

On Sun, Jun 14, 2009 at 04:04:36PM +0300, Avi Kivity wrote:
> Paul Mackerras wrote:
>> Avi Kivity writes:
>>
>>   
>>> An alternative implementation using 64-bit cmpxchg will recover most 
>>> of the costs of hashed spinlocks.  I assume most serious 32-bit  
>>> architectures have them?
>>>     
>>
>> Have a 64-bit cmpxchg, you mean?  x86 is the only one I know of, and
>> it already has an atomic64_t implementation using cmpxchg8b (or
>> whatever it's called).
>>   
>
> Yes (and it is cmpxchg8b).  I'm surprised powerpc doesn't have DCAS support.

Well, s390 and m68k have the equivalent (although I don't think Linux
suppiorts SMP m68k, although some dual 68040/68060 boards have existed).

But 32 bit PPC will never have it. It just does not fit in the architecture
since integer loads and stores are limited to 32 bit (or split into 32 bit
chunks). Besides that there is no instruction that performs a read-modify-write
of memory. This would make the LSU much more complex for a corner case.

Hey, Intel also botched the first implementation of cmpxchg8b on the Pentium:
the (in)famous f00f bug is actually "lock cmpxchg8b" with a register operand.

Now for these counters, other solutions could be considered, like using
the most significant bit as a lock and having "only" 63 usable bits (when 
counting ns, this overflows at 292 years). 

>
>> My thinking is that the 32-bit non-x86 architectures will be mostly
>> UP, so the overhead is just an interrupt enable/restore.  Those that
>> are SMP I would expect to be small SMP -- mostly just 2 cpus and maybe
>> a few 4-way systems.
>>   
>
> The new Nehalems provide 8 logical threads in a single socket.  All  
> those threads share a cache, and they have cmpxchg8b anyway, so this  
> won't matter.
>

The problem is not Nehalem (who wants to run 32 bit kernels on a Nehalem
anyway) or x86.

The problem is that the assumption that the largest PPC32 SMP are 4 way
may be outdated:

http://www.freescale.com/webapp/sps/site/prod_summary.jsp?fastpreview=1&code=P4080

and some products including that processor have been announced (I don't know
whether they are shipping or not) and (apparently) run Linux.

	Gabriel

^ permalink raw reply

* Re: [PATCH 2/2] [PATCH 2/2 v2] qe: add polling timeout to qe_issue_cmd()
From: Kumar Gala @ 2009-06-16 23:58 UTC (permalink / raw)
  To: Timur Tabi; +Cc: geert.uytterhoeven, linuxppc-dev, arnd
In-Reply-To: <1243351302-32062-3-git-send-email-timur@freescale.com>


On May 26, 2009, at 10:21 AM, Timur Tabi wrote:

> The qe_issue_cmd() function (Freescale PowerPC QUICC Engine library)  
> polls on
> a register until a status bit changes, but does not include a  
> timeout to
> handle the situation if the bit never changes.  Change the code to  
> use the new
> spin_event_timeout() macro, which simplifies polling on a register  
> without
> a timeout.
>
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
>
> This patch depends on my previous patch, "powerpc: introduce macro
> spin_event_timeout()".
>
> arch/powerpc/sysdev/qe_lib/qe.c |    9 ++++++---
> 1 files changed, 6 insertions(+), 3 deletions(-)

applied to next

- k

^ permalink raw reply

* Re: PowerPC PCI DMA issues (prefetch/coherency?)
From: Benjamin Herrenschmidt @ 2009-06-17  0:18 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Scott Wood, Chris Pringle, linuxppc-dev, linux-kernel
In-Reply-To: <200906162002.12376.arnd@arndb.de>

On Tue, 2009-06-16 at 20:02 +0200, Arnd Bergmann wrote:
> On Tuesday 16 June 2009, Scott Wood wrote:
> > > If the
> > > device is the only one, you can also use dma_alloc_noncoherent() and
> > > flush explicitly with dma_cache_sync().
> > 
> > I don't see how that would help -- aren't those also controlled by 
> > CONFIG_NOT_COHERENT_CACHE?
> 
> Ah, yes you are right. PowerPC implements dma_alloc_noncoherent as
> dma_alloc_coherent, so dma_cache_sync() is actually a NOP (or should be).

But we still need to sync the result of dma_map_* when used multiple
times for a single mapping.

Cheers,
Ben.

> Actually there seems to be a bug in here: Since dma_alloc_noncoherent
> gives you a coherent mapping (or NULL) on noncoherent machines,
> dma_cache_sync() is redundant and should not actually flush the
> cache, or we should change dma_alloc_noncoherent to do a simple
> alloc_pages on CONFIG_NON_COHERENT_CACHE and leave dma_cache_sync()
> as it is.
> 
> 	Arnd <><
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: PowerPC PCI DMA issues (prefetch/coherency?)
From: FUJITA Tomonori @ 2009-06-17  0:37 UTC (permalink / raw)
  To: benh; +Cc: scottwood, chris.pringle, linuxppc-dev, linux-kernel, arnd
In-Reply-To: <1245197925.21602.0.camel@pasglop>

On Wed, 17 Jun 2009 10:18:45 +1000
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Tue, 2009-06-16 at 20:02 +0200, Arnd Bergmann wrote:
> > On Tuesday 16 June 2009, Scott Wood wrote:
> > > > If the
> > > > device is the only one, you can also use dma_alloc_noncoherent() and
> > > > flush explicitly with dma_cache_sync().
> > > 
> > > I don't see how that would help -- aren't those also controlled by 
> > > CONFIG_NOT_COHERENT_CACHE?
> > 
> > Ah, yes you are right. PowerPC implements dma_alloc_noncoherent as
> > dma_alloc_coherent, so dma_cache_sync() is actually a NOP (or should be).
> 
> But we still need to sync the result of dma_map_* when used multiple
> times for a single mapping.

We have dma_sync_{single|sg}_for_{cpu|device} API for the above
purpose.

dma_cache_sync is supposed to be used only with the buffers that
dma_alloc_noncoherent() returns. On architecutures that maps
dma_alloc_noncoherent to dma_alloc_coherent, dma_cache_sync() is
supposed to be NOP.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox