LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] Don't panic when EEH_MAX_FAILS is exceeded
From: Stephen Rothwell @ 2008-07-21  4:09 UTC (permalink / raw)
  To: Sean MacLennan; +Cc: linuxppc-dev, Nathan Lynch, Sean MacLennan
In-Reply-To: <20080720234756.03b602ca@lappy.seanm.ca>

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

Hi Sean,

On Sun, 20 Jul 2008 23:47:56 -0400 Sean MacLennan <smaclennan@pikatech.com> wrote:
>
> I guess I am too x86 centric. On the x86 an msleep does not give up the
> CPU. It does a busy wait.

I think you must be thinking of mdelay().

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

^ permalink raw reply

* Re: [PATCH] Don't panic when EEH_MAX_FAILS is exceeded
From: Sean MacLennan @ 2008-07-21  3:47 UTC (permalink / raw)
  To: Nathan Lynch; +Cc: linuxppc-dev, Sean MacLennan
In-Reply-To: <20080720201708.GP9594@localdomain>

On Sun, 20 Jul 2008 15:17:08 -0500
"Nathan Lynch" <ntl@pobox.com> wrote:

> Sean MacLennan wrote:
>
> > Why can you not msleep within a spinlock? And when was this change
> > brought in?
> 
> Giving up the cpu while holding a spinlock risks locking up the system
> in the worst case -- if another task tries to acquire the held lock it
> can spin indefinitely.

I guess I am too x86 centric. On the x86 an msleep does not give up the
CPU. It does a busy wait.

So what sleep do you use on the PowerPC when you need a delay for
hardware reasons?

Cheers,
   Sean

^ permalink raw reply

* Re: [PATCH v3] elf loader support for auxvec base platform string
From: Andrew Morton @ 2008-07-21  3:40 UTC (permalink / raw)
  To: benh
  Cc: John Reiser, linux-kernel, linuxppc-dev, Nathan Lynch, torvalds,
	roland
In-Reply-To: <1216610655.11027.80.camel@pasglop>

On Mon, 21 Jul 2008 13:24:15 +1000 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Fri, 2008-07-18 at 13:28 -0500, Nathan Lynch wrote:
> > John Reiser wrote:
> > > Andrew Morton wrote:
> > > > On Thu, 17 Jul 2008 17:19:32 -0500
> > > > Nathan Lynch <ntl@pobox.com> wrote:
> > > > 
> > > > 
> > > >> [snip]
> > > >> A new aux vector entry, AT_BASE_PLATFORM, will denote the actual hardware.
> > > [snip]
> > > 
> > > > OK.
> > > > 
> > > > But it conflicts directly with the already-queued
> > > > execve-filename-document-and-export-via-auxiliary-vector.patch
> > 
> > Okay, I can rebase on -mm.
> 
> Andrew, we have one other patch (the powerpc bits) on top of that one.
> Do you want to carry both in -mm on top of John's patch ? We would like
> that in .27 though, I don't know what your merge plans are for John's
> patch.
> 

How about I send John's patch Linuswards right now?

^ permalink raw reply

* Re: [PATCH v3] elf loader support for auxvec base platform string
From: Benjamin Herrenschmidt @ 2008-07-21  3:24 UTC (permalink / raw)
  To: Andrew Morton
  Cc: John Reiser, linux-kernel, linuxppc-dev, Nathan Lynch, torvalds,
	roland
In-Reply-To: <20080718182850.GO9594@localdomain>

On Fri, 2008-07-18 at 13:28 -0500, Nathan Lynch wrote:
> John Reiser wrote:
> > Andrew Morton wrote:
> > > On Thu, 17 Jul 2008 17:19:32 -0500
> > > Nathan Lynch <ntl@pobox.com> wrote:
> > > 
> > > 
> > >> [snip]
> > >> A new aux vector entry, AT_BASE_PLATFORM, will denote the actual hardware.
> > [snip]
> > 
> > > OK.
> > > 
> > > But it conflicts directly with the already-queued
> > > execve-filename-document-and-export-via-auxiliary-vector.patch
> 
> Okay, I can rebase on -mm.

Andrew, we have one other patch (the powerpc bits) on top of that one.
Do you want to carry both in -mm on top of John's patch ? We would like
that in .27 though, I don't know what your merge plans are for John's
patch.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH] elf loader support for auxvec base platform string
From: Benjamin Herrenschmidt @ 2008-07-21  3:19 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linuxppc-dev, Andrew Morton, Nathan Lynch, linux-kernel, roland
In-Reply-To: <alpine.LFD.1.10.0807170908060.2959@woody.linux-foundation.org>

On Thu, 2008-07-17 at 09:10 -0700, Linus Torvalds wrote:
> 
> On Thu, 17 Jul 2008, Benjamin Herrenschmidt wrote:
> > 
> > Should I seek somebody's ack before merging a patch like the one below ?
> > 
> > I'm a bit reluctant to merge via the powerpc.git tree some changes to
> > generic files without at least an ack from somebody else :-)
> 
> Gaah. Generally we don't, but you're right to ask. The ELF loading keeps 
> on accumulating these things, and I'm not sure we have the right process 
> for things like this. They're all individually small and insignificant, 
> and they are all individually never going away and making the ELF loader 
> messier and messier.

Yeah, annoying. Oh well, this one at least is now getting Andrew's
scrutinity...

Cheers,
Ben.

^ permalink raw reply

* Re: [patch] powerpc: implement pte_special for 64K pages
From: Benjamin Herrenschmidt @ 2008-07-21  1:27 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Nick Piggin, linuxppc-dev, Dave Kleikamp
In-Reply-To: <20080720181658.02651e2f.akpm@linux-foundation.org>

On Sun, 2008-07-20 at 18:16 -0700, Andrew Morton wrote:
> On Mon, 21 Jul 2008 10:57:55 +1000 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> 
> > On Thu, 2008-07-17 at 12:44 +0200, Nick Piggin wrote:
> > > This can be folded into powerpc-implement-pte_special.patch
> > > 
> > > --
> > > Ben has now freed up a pte bit on 64k pages. Use it for special pte bit.
> > > 
> > > Signed-off-by: Nick Piggin <npiggin@suse.de>
> > > ---
> > 
> > Ack.
> > 
> > Andrew, will you merge this or do you want me to ?
> 
> I can do so.

Thanks.

Cheers,
Ben.

^ permalink raw reply

* Re: [patch] powerpc: implement pte_special for 64K pages
From: Andrew Morton @ 2008-07-21  1:16 UTC (permalink / raw)
  To: benh; +Cc: Nick Piggin, linuxppc-dev, Dave Kleikamp
In-Reply-To: <1216601875.11027.59.camel@pasglop>

On Mon, 21 Jul 2008 10:57:55 +1000 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Thu, 2008-07-17 at 12:44 +0200, Nick Piggin wrote:
> > This can be folded into powerpc-implement-pte_special.patch
> > 
> > --
> > Ben has now freed up a pte bit on 64k pages. Use it for special pte bit.
> > 
> > Signed-off-by: Nick Piggin <npiggin@suse.de>
> > ---
> 
> Ack.
> 
> Andrew, will you merge this or do you want me to ?

I can do so.

^ permalink raw reply

* Re: [patch] powerpc: implement pte_special for 64K pages
From: Benjamin Herrenschmidt @ 2008-07-21  0:57 UTC (permalink / raw)
  To: Nick Piggin; +Cc: linuxppc-dev, Andrew Morton, Dave Kleikamp
In-Reply-To: <20080717104426.GA25083@wotan.suse.de>

On Thu, 2008-07-17 at 12:44 +0200, Nick Piggin wrote:
> This can be folded into powerpc-implement-pte_special.patch
> 
> --
> Ben has now freed up a pte bit on 64k pages. Use it for special pte bit.
> 
> Signed-off-by: Nick Piggin <npiggin@suse.de>
> ---

Ack.

Andrew, will you merge this or do you want me to ?

Cheers,
Ben.

> Index: linux-2.6/include/asm-powerpc/pgtable-64k.h
> ===================================================================
> --- linux-2.6.orig/include/asm-powerpc/pgtable-64k.h	2008-07-17 18:53:03.000000000 +1000
> +++ linux-2.6/include/asm-powerpc/pgtable-64k.h	2008-07-17 20:30:06.000000000 +1000
> @@ -70,11 +70,12 @@
>  #define PGDIR_MASK	(~(PGDIR_SIZE-1))
>  
>  /* Additional PTE bits (don't change without checking asm in hash_low.S) */
> +#define __HAVE_ARCH_PTE_SPECIAL
> +#define _PAGE_SPECIAL	0x00000400 /* software: special page */
>  #define _PAGE_HPTE_SUB	0x0ffff000 /* combo only: sub pages HPTE bits */
>  #define _PAGE_HPTE_SUB0	0x08000000 /* combo only: first sub page */
>  #define _PAGE_COMBO	0x10000000 /* this is a combo 4k page */
>  #define _PAGE_4K_PFN	0x20000000 /* PFN is for a single 4k page */
> -#define _PAGE_SPECIAL	0x0	   /* don't have enough room for this yet */
>  
>  /* For 64K page, we don't have a separate _PAGE_HASHPTE bit. Instead,
>   * we set that to be the whole sub-bits mask. The C code will only

^ permalink raw reply

* Re: [2.6 patch] powerpc/boot/Makefile: change spaces to tabs
From: Benjamin Herrenschmidt @ 2008-07-21  0:59 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: linuxppc-dev, Paul Mackerras, David Gibson
In-Reply-To: <20080717181854.GB18326@cs181140183.pp.htv.fi>

On Thu, 2008-07-17 at 21:18 +0300, Adrian Bunk wrote:
> For C code spaces versus tabs is just a religious issue,
> but for Makefiles it actually matters.
> 
> This patch fixes he following errors:
> /home/bunk/linux/kernel-2.6/git/linux-2.6/arch/powerpc/boot/Makefile:166: *** missing separator.  Stop.
> /home/bunk/linux/kernel-2.6/git/linux-2.6/arch/powerpc/boot/Makefile:171: *** missing separator.  Stop.
> 
> Since this was inside an ifdef DTC_GENPARSER it was not a problem unless 
> someone wanted to regenerate the shipped generated files.
> 
> Signed-off-by: Adrian Bunk <bunk@kernel.org>

And that too :-)

Cheers,
Ben.

> ---
> 
>  arch/powerpc/boot/Makefile |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- a/arch/powerpc/boot/Makefile
> +++ b/arch/powerpc/boot/Makefile
> @@ -163,12 +163,12 @@ quiet_cmd_flex = FLEX    $@
>        cmd_flex = $(FLEX) -o$@ $<; cp $@ $@_shipped
>  
>  $(obj)/dtc-src/dtc-parser.tab.c: $(src)/dtc-src/dtc-parser.y FORCE
> -     $(call if_changed,bison)
> +	$(call if_changed,bison)
>  
>  $(obj)/dtc-src/dtc-parser.tab.h: $(obj)/dtc-src/dtc-parser.tab.c
>  
>  $(obj)/dtc-src/dtc-lexer.lex.c: $(src)/dtc-src/dtc-lexer.l FORCE
> -     $(call if_changed,flex)
> +	$(call if_changed,flex)
>  endif
>  
>  #############

^ permalink raw reply

* Re: [2.6 patch] powerpc: remove duplicate 6xx option
From: Benjamin Herrenschmidt @ 2008-07-21  0:58 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: linuxppc-dev, paulus
In-Reply-To: <20080717181828.GA18326@cs181140183.pp.htv.fi>

On Thu, 2008-07-17 at 21:18 +0300, Adrian Bunk wrote:
> The real option is above in the same Kconfig file.
> 
> Signed-off-by: Adrian Bunk <bunk@kernel.org>
> 
Thanks, I'll pick that up.

Cheers,
Ben.
 
> ---
> 2251e74345df51309579a0a5fd8339e2f14762b9 
> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
> index 5bc4b61..6489d57 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -84,9 +84,6 @@ config TUNE_CELL
>  	  machines. When building a kernel that is supposed to run only
>  	  on Cell, you should also select the POWER4_ONLY option.
>  
> -config 6xx
> -	bool
> -
>  # this is temp to handle compat with arch=ppc
>  config 8xx
>  	bool

^ permalink raw reply

* Re: jumping to code in RAM in a MPC55xx
From: Tehn Yit Chin @ 2008-07-20 23:22 UTC (permalink / raw)
  To: linuxppc-embedded
In-Reply-To: <bf04f99c0807172329m7ec94580n194e02b708bf7597@mail.gmail.com>

I am just going to answer my own questions for future reference.

I did some searching on the internet and came across this note on the
the usenet comp.sys.powerpc.tech

http://newsgroups.derkeiler.com/Archive/Comp/comp.sys.powerpc.tech/2005-11/msg00015.html

In summary, it performs a indirect jump to code via the CTR register.
The following code snippet show how it is done.

	lis	9,0xFC00 /* Function is at 0xFC000000 */
	mtctr	9
	bctrl


On Fri, Jul 18, 2008 at 4:29 PM, Tehn Yit Chin <tehn.yit.chin@gmail.com> wrote:
>
> Hi,
>
> This question is not directly related to linux, but a question on how to execute code from.
>
> My test code is as follows..
>
> void ram(unsigned int cat) __attribute__ ((section(".ram_code")));
> void ram1(void) __attribute__ ((section(".ram_code")));
> void flash(void) __attribute__ ((section(".text")));
> void flash1(unsigned int y) __attribute__ ((section(".text")));
>
> void ram(unsigned int cat)
> {
>     unsigned int abc;
>
>     abc += 12;
>     abc += cat;
> }
>
> void ram1(void)
> {
>     ram(123);
> }
>
>
> void flash1(unsigned int y)
> {
>     unsigned int x;
>
>     x = x + y;
>
> }
>
> void flash(void)
> {
>     unsigned int def;
>
>     def += 12;
>
>     asm (" bel ram1\n\t");
> }
>
> I have the section .ram_code mapped into the internal SRAM of the MPC55xx, which starts at 0x40000000. Code in .text are mapped into MPC55xx's internal FLASH, starting at 0x00000000.
>
> When I attempt to link the code together,  I get the error
>
> U:\src\applications\comms/fatdog.c(35,1): relocation truncated to fit: R_PPC_REL24 against symbol `ram1' defined in .ram_code section in U:\src\applications\comms\fatdog.o
>
> I googled the error and I think it means that the ram1() is too far away and the branch address has been truncated to fit into the binary.
>
> Other than assembler bel instruction, I have also tried bl, b and bea and all of them gave the same link error.
>
> I was wondering if anyone can give some insight on how I can get the MPC55xx to branch between FLASH and SRAM?
>
> Many thanks,
> Tehn Yit Chin
>

^ permalink raw reply

* Re: [PATCH] Don't panic when EEH_MAX_FAILS is exceeded
From: Linas Vepstas @ 2008-07-20 23:19 UTC (permalink / raw)
  To: Nathan Lynch; +Cc: linuxppc-dev, paulus
In-Reply-To: <20080720203333.GQ9594@localdomain>

2008/7/20 Nathan Lynch <ntl@pobox.com>:
> Mike Mason wrote:
>>
>> This patch changes the EEH_MAX_FAILS action from panic to printing
>> an error message.  Panicking under under this condition is too
>> harsh.


>>                       /* re-read the slot reset state */
>>                       if (read_slot_reset_state(pdn, rets) != 0)
>>                               rets[0] = -1;   /* reset state unknown */
>
> While I tend to agree that panic() is unnecessary, don't we want a
> stack dump unconditionally (i.e. not bracketed in #ifdef DEBUG)?

Probably. This stack trace would reveal a point inside the
inf loop, which can then be analyzed and fixed.

> I'd prefer just removing the code instead of adding #ifdef's in the
> middle of this function.  eeh.c needs less #ifdef DEBUG, not more :)

I didn't know that there was a lot of ifdef DEBUG in there.
Yes, we don't need an ifdef DEBUG for this.

Pending these changes, I'd happily add:

Acked-by: Linas Vepstas <linasvepstas@gmail.com>

--linas

^ permalink raw reply

* Re: dma_alloc_coherent() on PPC32: physical addresses above 2G possible?
From: Benjamin Herrenschmidt @ 2008-07-20 21:35 UTC (permalink / raw)
  To: Stefan Richter; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <488385A7.4010509@s5r6.in-berlin.de>

On Sun, 2008-07-20 at 20:36 +0200, Stefan Richter wrote:
> Hi all,
> 
> I have to implement a workaround for a PCI device which gets into 
> trouble if descriptors are located at 32bit addresses, while 31bit 
> addresses are fine.  I would like to avoid this workaround on machines 
> on which dma_alloc_coherent() won't ever go at memory above 2 GB.
> 
> Is defined(CONFIG_PPC32) a safe test for this?  I'm under the impression 
> that defined(CONFIG_X86_32) is safe.

CONFIG_PPC32 -may- be safe today... the  current implementation
for DMA coherent machines seem to have been copied from x86 or so :-) It
sets GFP_DMA if the coherent_dma_mask < 32 bits, but we don't have
separate ZONE_NORMAL and ZONE_DMA anyway, so that is irrelevant. In any
case, it won't give you memory in highmem, so you should be safe.

In the  non-DMA coherent case, make sure you don't pass __GFP_HIGHMEM.

There's some work in progress to support swiotlb and more advanced DMA
mapping ops for PPC32 (using function pointers like PPC64), so things
will probably change. However, the need for 31 bits DMA seem to be
common enough that we should probably do something specific about it,
for example, ensure that lowmem is never > 2G (shouldn't be a big deal)
and thus make dma masks < 32 bits always clear __GFP_HIGHMEM from the
allocation mask.

Ben.

^ permalink raw reply

* Re: [PATCH] Don't panic when EEH_MAX_FAILS is exceeded
From: Nathan Lynch @ 2008-07-20 20:33 UTC (permalink / raw)
  To: Mike Mason; +Cc: linasvepstas, paulus, linuxppc-dev
In-Reply-To: <488383D4.9000602@us.ibm.com>

Mike Mason wrote:
>
> This patch changes the EEH_MAX_FAILS action from panic to printing
> an error message.  Panicking under under this condition is too
> harsh.  Although performance will be affected and the device may not
> recover, the system is still running, which at the very least,
> should allow for a more graceful shutdown.  The panic() is now
> wrapped in a DEBUG statement for development purposes.  The patch
> also removes the msleep() within a spinlock, which is not allowed.

> @@ -509,18 +510,24 @@

For ease of review, please try to use diff -p to generate patches.

> 	rc = 1;
> 	if (pdn->eeh_mode & EEH_MODE_ISOLATED) {
> 		pdn->eeh_check_count ++;
> -		if (pdn->eeh_check_count >= EEH_MAX_FAILS) {
> -			printk (KERN_ERR "EEH: Device driver ignored %d bad reads, panicing\n",
> -			        pdn->eeh_check_count);
> +		if (pdn->eeh_check_count % EEH_MAX_FAILS == 0) {
> +			location = (char *) of_get_property(dn, "ibm,loc-code", NULL);

Unneeded cast here, I think.

> +			printk (KERN_ERR "EEH: %d reads ignored for recovering device at "
> +				"location=%s driver=%s pci addr=%s\n",
> +				pdn->eeh_check_count, location,
> +				dev->driver->name, pci_name(dev));
> +			printk (KERN_ERR "EEH: Might be infinite loop in %s driver\n",
> +				dev->driver->name);
> +#ifdef DEBUG
> 			dump_stack();
> -			msleep(5000);
> -			
> +
> 			/* re-read the slot reset state */
> 			if (read_slot_reset_state(pdn, rets) != 0)
> 				rets[0] = -1;	/* reset state unknown */
>
> 			/* If we are here, then we hit an infinite loop. Stop. */
> 			panic("EEH: MMIO halt (%d) on device:%s\n", rets[0], pci_name(dev));
> +#endif

While I tend to agree that panic() is unnecessary, don't we want a
stack dump unconditionally (i.e. not bracketed in #ifdef DEBUG)?

I'd prefer just removing the code instead of adding #ifdef's in the
middle of this function.  eeh.c needs less #ifdef DEBUG, not more :)

^ permalink raw reply

* Re: [PATCH] Don't panic when EEH_MAX_FAILS is exceeded
From: Nathan Lynch @ 2008-07-20 20:17 UTC (permalink / raw)
  To: Sean MacLennan; +Cc: linuxppc-dev
In-Reply-To: <20080720145858.5be92934@lappy.seanm.ca>

Sean MacLennan wrote:
> On Sun, 20 Jul 2008 11:28:36 -0700
> "Mike Mason" <mmlnx@us.ibm.com> wrote:
> 
> > This patch changes the EEH_MAX_FAILS action from panic to printing an
> > error message.  Panicking under under this condition is too harsh.
> > Although performance will be affected and the device may not recover,
> > the system is still running, which at the very least, should allow
> > for a more graceful shutdown.  The panic() is now wrapped in a DEBUG
> > statement for development purposes.  The patch also removes the
> > msleep() within a spinlock, which is not allowed.
> 
> Why can you not msleep within a spinlock? And when was this change
> brought in?

Giving up the cpu while holding a spinlock risks locking up the system
in the worst case -- if another task tries to acquire the held lock it
can spin indefinitely.

^ permalink raw reply

* Re: dma_alloc_coherent() on PPC32: physical addresses above 2G possible?
From: Stefan Richter @ 2008-07-20 20:11 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20080720123900.02677e6e@infradead.org>

Arjan van de Ven wrote:
> On Sun, 20 Jul 2008 21:25:51 +0200
> Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
>> 	if (dev->needs_dma_mask_workaround)
>> 		pci_set_consistent_dma_mask(pdev, DMA_31BIT_MASK);
>> 	allocate_something_special;
>> 	if (dev->needs_dma_mask_workaround)
>> 		pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
...
> something like this.
> But realistically, how many consistent/coherent allocations do you have?
> some ring buffers and other one time stuff surely... but not after that?

It's for DMA programs (i.e. lists of descriptors), not for the data DMA
buffers themselves.  FireWire controllers have several DMA contexts for
isochronous and asynchronous reception and transmission, and some
others.  Only context programs for isochronous reception need the
workaround.

We are dynamically appending new descriptors during runtime.  I.e. the
affected allocations happen during setup of the DMA context and
sometimes while the DMA context is active.  Particularly, in
drivers/firewire/fw-ohci.c:
	ohci_allocate_iso_context
		context_init
			context_add_buffer  <--
and
	ohci_queue_iso
		ohci_queue_iso_receive_dualbuffer
			context_get_descriptors
				context_add_buffer  <--

The other unaffected context types use context_add_buffer too.
-- 
Stefan Richter
-=====-==--- -=== =-=--
http://arcgraph.de/sr/

^ permalink raw reply

* Re: dma_alloc_coherent() on PPC32: physical addresses above 2G possible?
From: Stefan Richter @ 2008-07-20 19:25 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20080720114358.6c88e048@infradead.org>

Arjan van de Ven wrote:
> On Sun, 20 Jul 2008 20:36:23 +0200
> Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> 
>> PS:  I don't want to set the DMA mask of this device to
>> DMA_31BIT_MASK because that would be detrimental to other functions
>> of the device. It's a TI TSB43AB22A FireWire controller.
> 
> Hi,
> 
> just want to mention that you can set the coherent mask separately from
> the generic mask... is that sufficient for your load?
> (you can even set it just for this allocation..)

Hmm.  Would that be done this way?
During probe:

	if (chip_is_tsb43ab22a) {
		if (dma_supported(dev, DMA_31BIT_MASK))
			chip->needs_dma_mask_workaround = 1;
		else
			chip->needs_some_other_workaround = 1;
	}

Later on:

	if (dev->needs_dma_mask_workaround)
		pci_set_consistent_dma_mask(pdev, DMA_31BIT_MASK);
	allocate_something_special;
	if (dev->needs_dma_mask_workaround)
		pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);

Or is there a variant of dma_alloc_coherent() which directly accepts a
mask argument?
-- 
Stefan Richter
-=====-==--- -=== =-=--
http://arcgraph.de/sr/

^ permalink raw reply

* Re: dma_alloc_coherent() on PPC32: physical addresses above 2G possible?
From: Arjan van de Ven @ 2008-07-20 19:48 UTC (permalink / raw)
  To: Stefan Richter; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <4883913F.9040706@s5r6.in-berlin.de>

On Sun, 20 Jul 2008 21:25:51 +0200
Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:

> Arjan van de Ven wrote:
> > On Sun, 20 Jul 2008 20:36:23 +0200
> > Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> > 
> >> PS:  I don't want to set the DMA mask of this device to
> >> DMA_31BIT_MASK because that would be detrimental to other functions
> >> of the device. It's a TI TSB43AB22A FireWire controller.
> > 
> > Hi,
> > 
> > just want to mention that you can set the coherent mask separately
> > from the generic mask... is that sufficient for your load?
> > (you can even set it just for this allocation..)
> 
> Hmm.  Would that be done this way?
> During probe:
> 
> 	if (chip_is_tsb43ab22a) {
> 		if (dma_supported(dev, DMA_31BIT_MASK))
> 			chip->needs_dma_mask_workaround = 1;
> 		else
> 			chip->needs_some_other_workaround = 1;
> 	}

btw it might be nicer to make this

chip->something_special_mask = DMA_31BIT_MASK;

then you can just use the mask from this struct rather than another
check


-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

^ permalink raw reply

* Re: dma_alloc_coherent() on PPC32: physical addresses above 2G possible?
From: Arjan van de Ven @ 2008-07-20 19:39 UTC (permalink / raw)
  To: Stefan Richter; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <4883913F.9040706@s5r6.in-berlin.de>

On Sun, 20 Jul 2008 21:25:51 +0200
Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:

> 
> Later on:
> 
> 	if (dev->needs_dma_mask_workaround)
> 		pci_set_consistent_dma_mask(pdev, DMA_31BIT_MASK);
> 	allocate_something_special;
> 	if (dev->needs_dma_mask_workaround)
> 		pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
> 
> Or is there a variant of dma_alloc_coherent() which directly accepts a
> mask argument?

something like this.
But realistically, how many consistent/coherent allocations do you have?
some ring buffers and other one time stuff surely... but not after that?

-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

^ permalink raw reply

* Re: [PATCH] Don't panic when EEH_MAX_FAILS is exceeded
From: Sean MacLennan @ 2008-07-20 18:58 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <488383D4.9000602@us.ibm.com>

On Sun, 20 Jul 2008 11:28:36 -0700
"Mike Mason" <mmlnx@us.ibm.com> wrote:

> This patch changes the EEH_MAX_FAILS action from panic to printing an
> error message.  Panicking under under this condition is too harsh.
> Although performance will be affected and the device may not recover,
> the system is still running, which at the very least, should allow
> for a more graceful shutdown.  The panic() is now wrapped in a DEBUG
> statement for development purposes.  The patch also removes the
> msleep() within a spinlock, which is not allowed.

Why can you not msleep within a spinlock? And when was this change
brought in?

Cheers,
   Sean

^ permalink raw reply

* dma_alloc_coherent() on PPC32: physical addresses above 2G possible?
From: Stefan Richter @ 2008-07-20 18:36 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: linux-kernel

Hi all,

I have to implement a workaround for a PCI device which gets into 
trouble if descriptors are located at 32bit addresses, while 31bit 
addresses are fine.  I would like to avoid this workaround on machines 
on which dma_alloc_coherent() won't ever go at memory above 2 GB.

Is defined(CONFIG_PPC32) a safe test for this?  I'm under the impression 
that defined(CONFIG_X86_32) is safe.

Are there any other means to detect when the workaround can be omitted, 
at compile time or at runtime?

PS:  I don't want to set the DMA mask of this device to DMA_31BIT_MASK 
because that would be detrimental to other functions of the device. It's 
a TI TSB43AB22A FireWire controller.
-- 
Stefan Richter
-=====-==--- -=== =-=--
http://arcgraph.de/sr/

^ permalink raw reply

* Re: dma_alloc_coherent() on PPC32: physical addresses above 2G possible?
From: Arjan van de Ven @ 2008-07-20 18:43 UTC (permalink / raw)
  To: Stefan Richter; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <488385A7.4010509@s5r6.in-berlin.de>

On Sun, 20 Jul 2008 20:36:23 +0200
Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:

> PS:  I don't want to set the DMA mask of this device to
> DMA_31BIT_MASK because that would be detrimental to other functions
> of the device. It's a TI TSB43AB22A FireWire controller.

Hi,

just want to mention that you can set the coherent mask separately from
the generic mask... is that sufficient for your load?
(you can even set it just for this allocation..)

-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

^ permalink raw reply

* [PATCH] Don't panic when EEH_MAX_FAILS is exceeded
From: Mike Mason @ 2008-07-20 18:28 UTC (permalink / raw)
  To: paulus, benh, linasvepstas, linuxppc-dev

This patch changes the EEH_MAX_FAILS action from panic to printing an error message.  Panicking under under this condition is too harsh.  Although performance will be affected and the device may not recover, the system is still running, which at the very least, should allow for a more graceful shutdown.  The panic() is now wrapped in a DEBUG statement for development purposes.  The patch also removes the msleep() within a spinlock, which is not allowed.

Signed-off-by: Mike Mason <mmlnx@us.ibm.com> 

--- powerpc.git/arch/powerpc/platforms/pseries/eeh.c	2008-07-18 08:51:42.000000000 -0700
+++ powerpc.git-new/arch/powerpc/platforms/pseries/eeh.c	2008-07-18 13:26:37.000000000 -0700
@@ -75,9 +75,9 @@
  */
 
 /* If a device driver keeps reading an MMIO register in an interrupt
- * handler after a slot isolation event has occurred, we assume it
- * is broken and panic.  This sets the threshold for how many read
- * attempts we allow before panicking.
+ * handler after a slot isolation event, it might be broken.
+ * This sets the threshold for how many read attempts we allow
+ * before printing an error message.
  */
 #define EEH_MAX_FAILS	2100000
 
@@ -470,6 +470,7 @@
 	unsigned long flags;
 	struct pci_dn *pdn;
 	int rc = 0;
+	const char *location;
 
 	total_mmio_ffs++;
 
@@ -509,18 +510,24 @@
 	rc = 1;
 	if (pdn->eeh_mode & EEH_MODE_ISOLATED) {
 		pdn->eeh_check_count ++;
-		if (pdn->eeh_check_count >= EEH_MAX_FAILS) {
-			printk (KERN_ERR "EEH: Device driver ignored %d bad reads, panicing\n",
-			        pdn->eeh_check_count);
+		if (pdn->eeh_check_count % EEH_MAX_FAILS == 0) {
+			location = (char *) of_get_property(dn, "ibm,loc-code", NULL);
+			printk (KERN_ERR "EEH: %d reads ignored for recovering device at "
+				"location=%s driver=%s pci addr=%s\n",
+				pdn->eeh_check_count, location,
+				dev->driver->name, pci_name(dev));
+			printk (KERN_ERR "EEH: Might be infinite loop in %s driver\n",
+				dev->driver->name);
+#ifdef DEBUG
 			dump_stack();
-			msleep(5000);
-			
+
 			/* re-read the slot reset state */
 			if (read_slot_reset_state(pdn, rets) != 0)
 				rets[0] = -1;	/* reset state unknown */
 
 			/* If we are here, then we hit an infinite loop. Stop. */
 			panic("EEH: MMIO halt (%d) on device:%s\n", rets[0], pci_name(dev));
+#endif
 		}
 		goto dn_unlock;
 	}

^ permalink raw reply

* Re: going to OLS?
From: Vitaly Bordug @ 2008-07-20 14:00 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev list
In-Reply-To: <4313E14A-C3FB-4996-B188-767704F380C3@kernel.crashing.org>


Can't make it but wish you guys all the best at official and unofficial
PPC BoFs/beers... I'll be heading for Linux Plumbers this autumn though.

--
Sincerely, Vitaly

^ permalink raw reply

* radeonfb, dedicate memory to something else
From: Matt Sealey @ 2008-07-20 10:03 UTC (permalink / raw)
  To: ppc-dev

Hi guys,

I know this isn't a PPC question, but since some of the RadeonFB developers
live here I thought best (and it's about a PPC platform).

Is there any way to hack up the RadeonFB driver - or anything related - to
reserve portions of the memory for a "fake" MTD or so, and still use the
Radeon as a graphics device? What I am talking about basically is turning
a 64MB Radeon card into a 32MB Radeon card, or a 128MB one into a 64MB
card..

I think this can be done by hacking a PCI fixup perhaps, for Efika only
in my case, whereby the memory ranges are fiddled.

Perhaps, a better solution is to use some kind of in-kernel subsystem to
"allocate" and return a 32MB segment of PCI memory (and put it in an
named rheap) but I assume other drivers can and will simply walk all
over it?

-- 
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations

^ 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