LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 1/9] powerpc/44x: Add PowerPC 44x simple platform support
From: Arnd Bergmann @ 2008-08-20 13:33 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <496103659f7b122a8301703b055ef4c6bd3092af.1219160188.git.jwboyer@linux.vnet.ibm.com>

On Tuesday 19 August 2008, Josh Boyer wrote:
> This adds a common board file for almost all of the "simple" PowerPC 44x
> boards that exist today.  This is intended to be a single place to add
> support for boards that do not differ in platform support from most of the
> evaluation boards that are used as reference platforms.  Boards that have
> specific requirements or custom hardware setup should still have their own
> board.c file.

The code looks correct, but since this is going to be example code
that may get copied into other platforms, I would take extra care
for coding style:

> +#include <linux/init.h>
> +#include <linux/of_platform.h>
> +
> +#include <asm/machdep.h>
> +#include <asm/prom.h>
> +#include <asm/udbg.h>
> +#include <asm/time.h>
> +#include <asm/uic.h>
> +#include <asm/pci-bridge.h>
> +#include <asm/ppc4xx.h>

#include lines should be ordered alphabetically in an ideal world.

> +static char *board[] __initdata = {
> +	"amcc,bamboo",
> +	"amcc,cayonlands",
> +	"ibm,ebony",
> +	"amcc,katmai",
> +	"amcc,rainier",
> +	"amcc,sequoia",
> +	"amcc,taishan",
> +	NULL
> +};

You don't need the NULL termination here, since the array is only
used statically and you can use ARRAY_SIZE().

> +static int __init ppc44x_probe(void)
> +{
> +	unsigned long root = of_get_flat_dt_root();
> +	int i = 0;
> +
> +	while (board[i]) {
> +		if (of_flat_dt_is_compatible(root, board[i]))
> +			break;
> +		i++;
> +	}

This looks like a for() loop in disguise, so you can better write
it as

	int i;
	for (i = 0; i < ARRAY_SIZE(board); i++) {
		if (of_flat_dt_is_compatible(root, board[i])) {
			ppc_pci_flags = PPC_PCI_REASSIGN_ALL_RSRC;
			return 1;
		}
	}
	return 0;


	Arnd <><

^ permalink raw reply

* Re: ftrace introduces instability into kernel 2.6.27(-rc2,-rc3)
From: Eran Liberty @ 2008-08-20 13:36 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Mathieu Desnoyers, linux-kernel, linuxppc-dev, Steven Rostedt,
	Alan Modra, Scott Wood, Paul E. McKenney
In-Reply-To: <alpine.DEB.1.10.0808200916090.13132@gandalf.stny.rr.com>

Steven Rostedt wrote:
> On Wed, 20 Aug 2008, Steven Rostedt wrote:
>
>   
>> On Wed, 20 Aug 2008, Benjamin Herrenschmidt wrote:
>>
>>     
>>> Found the problem (or at least -a- problem), it's a gcc bug.
>>>
>>> Well, first I must say the code generated by -pg is just plain
>>> horrible :-)
>>>
>>> Appart from that, look at the exit of, for example, __d_lookup, as
>>> generated by gcc when ftrace is enabled:
>>>
>>> c00c0498:       38 60 00 00     li      r3,0
>>> c00c049c:       81 61 00 00     lwz     r11,0(r1)
>>> c00c04a0:       80 0b 00 04     lwz     r0,4(r11)
>>> c00c04a4:       7d 61 5b 78     mr      r1,r11
>>> c00c04a8:       bb 0b ff e0     lmw     r24,-32(r11)
>>> c00c04ac:       7c 08 03 a6     mtlr    r0
>>> c00c04b0:       4e 80 00 20     blr
>>>
>>> As you can see, it restores r1 -before- it pops r24..r31 off
>>> the stack ! I let you imagine what happens if an interrupt happens
>>> just in between those two instructions (mr and lmw). We don't do
>>> redzones on our ABI, so basically, the registers end up corrupted
>>> by the interrupt.
>>>       
>> Ouch!  You've disassembled this without -pg too, and it does not have this 
>> bug? What version of gcc do you have?
>>
>>     
>
> I have:
>  gcc (Debian 4.3.1-2) 4.3.1
>
> c00c64c8:       81 61 00 00     lwz     r11,0(r1)
> c00c64cc:       7f 83 e3 78     mr      r3,r28
> c00c64d0:       80 0b 00 04     lwz     r0,4(r11)
> c00c64d4:       ba eb ff dc     lmw     r23,-36(r11)
> c00c64d8:       7d 61 5b 78     mr      r1,r11
> c00c64dc:       7c 08 03 a6     mtlr    r0
> c00c64e0:       4e 80 00 20     blr
>
>
> My version looks fine.  I'm thinking that this is a separate issue than 
> what Eran is seeing.
>
> Eran, can you do an "objdump -dr vmlinux" and search for __d_lookup, and 
> print out the end of the function dump.
>
> Thanks,
>
> -- Steve
>
>
>
>   
powerpc-linux-gnu-objdump -dr --start-address=0xc00bb584 vmlinux | head 
-n 100

vmlinux:     file format elf32-powerpc

Disassembly of section .text:

c00bb584 <__d_lookup>:
c00bb584:       7c 08 02 a6     mflr    r0
c00bb588:       90 01 00 04     stw     r0,4(r1)
c00bb58c:       4b f5 5c 51     bl      c00111dc <_mcount>
c00bb590:       94 21 ff d0     stwu    r1,-48(r1)
c00bb594:       7c 08 02 a6     mflr    r0
c00bb598:       3d 20 9e 37     lis     r9,-25033
c00bb59c:       bf 01 00 10     stmw    r24,16(r1)
c00bb5a0:       61 29 00 01     ori     r9,r9,1
c00bb5a4:       3d 60 c0 38     lis     r11,-16328
c00bb5a8:       90 01 00 34     stw     r0,52(r1)
c00bb5ac:       7c 60 4a 78     xor     r0,r3,r9
c00bb5b0:       54 00 d9 7e     rlwinm  r0,r0,27,5,31
c00bb5b4:       83 84 00 00     lwz     r28,0(r4)
c00bb5b8:       7c 3f 0b 78     mr      r31,r1
c00bb5bc:       81 0b 1a 2c     lwz     r8,6700(r11)
c00bb5c0:       39 6b 1a 2c     addi    r11,r11,6700
c00bb5c4:       7c 00 e2 14     add     r0,r0,r28
c00bb5c8:       81 4b 00 04     lwz     r10,4(r11)
c00bb5cc:       7c 09 4a 78     xor     r9,r0,r9
c00bb5d0:       83 24 00 04     lwz     r25,4(r4)
c00bb5d4:       7d 29 44 30     srw     r9,r9,r8
c00bb5d8:       81 0b 00 08     lwz     r8,8(r11)
c00bb5dc:       7d 29 02 78     xor     r9,r9,r0
c00bb5e0:       83 04 00 08     lwz     r24,8(r4)
c00bb5e4:       7d 29 50 38     and     r9,r9,r10
c00bb5e8:       55 29 10 3a     rlwinm  r9,r9,2,0,29
c00bb5ec:       7c 09 40 2e     lwzx    r0,r9,r8
c00bb5f0:       7c 9a 23 78     mr      r26,r4
c00bb5f4:       7c 7b 1b 78     mr      r27,r3
c00bb5f8:       2f 80 00 00     cmpwi   cr7,r0,0
c00bb5fc:       7c 1e 03 78     mr      r30,r0
c00bb600:       40 be 00 14     bne+    cr7,c00bb614 <__d_lookup+0x90>
c00bb604:       48 00 00 7c     b       c00bb680 <__d_lookup+0xfc>
c00bb608:       83 de 00 00     lwz     r30,0(r30)
c00bb60c:       2f 9e 00 00     cmpwi   cr7,r30,0
c00bb610:       41 9e 00 70     beq-    cr7,c00bb680 <__d_lookup+0xfc>
c00bb614:       80 1e 00 00     lwz     r0,0(r30)
c00bb618:       2f 80 00 00     cmpwi   cr7,r0,0
c00bb61c:       41 9e 00 08     beq-    cr7,c00bb624 <__d_lookup+0xa0>
c00bb620:       7c 00 02 2c     dcbt    r0,r0
c00bb624:       3b be ff f4     addi    r29,r30,-12
c00bb628:       80 1d 00 18     lwz     r0,24(r29)
c00bb62c:       7f 80 e0 00     cmpw    cr7,r0,r28
c00bb630:       40 9e ff d8     bne+    cr7,c00bb608 <__d_lookup+0x84>
c00bb634:       80 1d 00 14     lwz     r0,20(r29)
c00bb638:       7f 80 d8 00     cmpw    cr7,r0,r27
c00bb63c:       40 9e ff cc     bne+    cr7,c00bb608 <__d_lookup+0x84>
c00bb640:       81 3b 00 48     lwz     r9,72(r27)
c00bb644:       38 9d 00 18     addi    r4,r29,24
c00bb648:       2f 89 00 00     cmpwi   cr7,r9,0
c00bb64c:       41 9e 00 50     beq-    cr7,c00bb69c <__d_lookup+0x118>
c00bb650:       80 09 00 08     lwz     r0,8(r9)
c00bb654:       2f 80 00 00     cmpwi   cr7,r0,0
c00bb658:       41 9e 00 44     beq-    cr7,c00bb69c <__d_lookup+0x118>
c00bb65c:       7f 63 db 78     mr      r3,r27
c00bb660:       7c 09 03 a6     mtctr   r0
c00bb664:       7f 45 d3 78     mr      r5,r26
c00bb668:       4e 80 04 21     bctrl
c00bb66c:       2f 83 00 00     cmpwi   cr7,r3,0
c00bb670:       41 9e 00 50     beq-    cr7,c00bb6c0 <__d_lookup+0x13c>
c00bb674:       83 de 00 00     lwz     r30,0(r30)
c00bb678:       2f 9e 00 00     cmpwi   cr7,r30,0
c00bb67c:       40 9e ff 98     bne+    cr7,c00bb614 <__d_lookup+0x90>
c00bb680:       38 60 00 00     li      r3,0
c00bb684:       81 61 00 00     lwz     r11,0(r1)
c00bb688:       80 0b 00 04     lwz     r0,4(r11)
c00bb68c:       7d 61 5b 78     mr      r1,r11
c00bb690:       bb 0b ff e0     lmw     r24,-32(r11)
c00bb694:       7c 08 03 a6     mtlr    r0
c00bb698:       4e 80 00 20     blr
c00bb69c:       80 04 00 04     lwz     r0,4(r4)
c00bb6a0:       7f 80 c8 00     cmpw    cr7,r0,r25
c00bb6a4:       40 9e ff 64     bne+    cr7,c00bb608 <__d_lookup+0x84>
c00bb6a8:       80 64 00 08     lwz     r3,8(r4)
c00bb6ac:       7f 25 cb 78     mr      r5,r25
c00bb6b0:       7f 04 c3 78     mr      r4,r24
c00bb6b4:       4b f5 ab 65     bl      c0016218 <memcmp>
c00bb6b8:       2f 83 00 00     cmpwi   cr7,r3,0
c00bb6bc:       40 9e ff 4c     bne+    cr7,c00bb608 <__d_lookup+0x84>
c00bb6c0:       80 1d 00 04     lwz     r0,4(r29)
c00bb6c4:       70 09 00 10     andi.   r9,r0,16
c00bb6c8:       40 a2 ff b8     bne-    c00bb680 <__d_lookup+0xfc>
c00bb6cc:       7c 00 e8 28     lwarx   r0,0,r29
c00bb6d0:       30 00 00 01     addic   r0,r0,1
c00bb6d4:       7c 00 e9 2d     stwcx.  r0,0,r29
c00bb6d8:       40 a2 ff f4     bne-    c00bb6cc <__d_lookup+0x148>
c00bb6dc:       7f a3 eb 78     mr      r3,r29
c00bb6e0:       4b ff ff a4     b       c00bb684 <__d_lookup+0x100>

c00bb6e4 <d_lookup>:
c00bb6e4:       7c 08 02 a6     mflr    r0
c00bb6e8:       90 01 00 04     stw     r0,4(r1)

^ permalink raw reply

* Re: ftrace introduces instability into kernel 2.6.27(-rc2,-rc3)
From: Steven Rostedt @ 2008-08-20 13:43 UTC (permalink / raw)
  To: Eran Liberty
  Cc: Mathieu Desnoyers, linux-kernel, linuxppc-dev, Steven Rostedt,
	Alan Modra, Scott Wood, Paul E. McKenney
In-Reply-To: <48AC1DD8.9080702@extricom.com>


On Wed, 20 Aug 2008, Eran Liberty wrote:

> Steven Rostedt wrote:
> > On Wed, 20 Aug 2008, Steven Rostedt wrote:
> > 
> >   
> > > On Wed, 20 Aug 2008, Benjamin Herrenschmidt wrote:
> > > 
> > >     
> > > > Found the problem (or at least -a- problem), it's a gcc bug.
> > > > 
> > > > Well, first I must say the code generated by -pg is just plain
> > > > horrible :-)
> > > > 
> > > > Appart from that, look at the exit of, for example, __d_lookup, as
> > > > generated by gcc when ftrace is enabled:
> > > > 
> > > > c00c0498:       38 60 00 00     li      r3,0
> > > > c00c049c:       81 61 00 00     lwz     r11,0(r1)
> > > > c00c04a0:       80 0b 00 04     lwz     r0,4(r11)
> > > > c00c04a4:       7d 61 5b 78     mr      r1,r11
> > > > c00c04a8:       bb 0b ff e0     lmw     r24,-32(r11)
> > > > c00c04ac:       7c 08 03 a6     mtlr    r0
> > > > c00c04b0:       4e 80 00 20     blr
> > > > 
> > > > As you can see, it restores r1 -before- it pops r24..r31 off
> > > > the stack ! I let you imagine what happens if an interrupt happens
> > > > just in between those two instructions (mr and lmw). We don't do
> > > > redzones on our ABI, so basically, the registers end up corrupted
> > > > by the interrupt.
> > > >       
> > > Ouch!  You've disassembled this without -pg too, and it does not have this
> > > bug? What version of gcc do you have?
> > > 
> > >     
> > 
> > I have:
> >  gcc (Debian 4.3.1-2) 4.3.1
> > 
> > c00c64c8:       81 61 00 00     lwz     r11,0(r1)
> > c00c64cc:       7f 83 e3 78     mr      r3,r28
> > c00c64d0:       80 0b 00 04     lwz     r0,4(r11)
> > c00c64d4:       ba eb ff dc     lmw     r23,-36(r11)
> > c00c64d8:       7d 61 5b 78     mr      r1,r11
> > c00c64dc:       7c 08 03 a6     mtlr    r0
> > c00c64e0:       4e 80 00 20     blr
> > 
> > 
> > My version looks fine.  I'm thinking that this is a separate issue than what
> > Eran is seeing.
> > 
> > Eran, can you do an "objdump -dr vmlinux" and search for __d_lookup, and
> > print out the end of the function dump.
> > 
> > Thanks,
> > 
> > -- Steve
> > 
> > 
> > 
> >   
> powerpc-linux-gnu-objdump -dr --start-address=0xc00bb584 vmlinux | head -n 100
> 
> vmlinux:     file format elf32-powerpc
> 
> Disassembly of section .text:
> 
> c00bb584 <__d_lookup>:

[...]

> c00bb670:       41 9e 00 50     beq-    cr7,c00bb6c0 <__d_lookup+0x13c>
> c00bb674:       83 de 00 00     lwz     r30,0(r30)
> c00bb678:       2f 9e 00 00     cmpwi   cr7,r30,0
> c00bb67c:       40 9e ff 98     bne+    cr7,c00bb614 <__d_lookup+0x90>
> c00bb680:       38 60 00 00     li      r3,0
> c00bb684:       81 61 00 00     lwz     r11,0(r1)
> c00bb688:       80 0b 00 04     lwz     r0,4(r11)
> c00bb68c:       7d 61 5b 78     mr      r1,r11

[ BUG HERE IF INTERRUPT HAPPENS ]

> c00bb690:       bb 0b ff e0     lmw     r24,-32(r11)
> c00bb694:       7c 08 03 a6     mtlr    r0
> c00bb698:       4e 80 00 20     blr

Yep, you have the same bug in your compiler.

-- Steve

^ permalink raw reply

* Re: [PATCH 1/9] powerpc/44x: Add PowerPC 44x simple platform support
From: Josh Boyer @ 2008-08-20 14:03 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev
In-Reply-To: <200808201533.22258.arnd@arndb.de>

On Wed, 20 Aug 2008 15:33:21 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Tuesday 19 August 2008, Josh Boyer wrote:
> > This adds a common board file for almost all of the "simple" PowerPC 44x
> > boards that exist today.  This is intended to be a single place to add
> > support for boards that do not differ in platform support from most of the
> > evaluation boards that are used as reference platforms.  Boards that have
> > specific requirements or custom hardware setup should still have their own
> > board.c file.
> 
> The code looks correct, but since this is going to be example code
> that may get copied into other platforms, I would take extra care
> for coding style:

Fair enough.  Your comments all make sense to me.  I'll fixup things
and resend this one.

josh

^ permalink raw reply

* Re: ftrace introduces instability into kernel 2.6.27(-rc2,-rc3)
From: Eran Liberty @ 2008-08-20 14:02 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Mathieu Desnoyers, linux-kernel, linuxppc-dev, Steven Rostedt,
	Alan Modra, Scott Wood, Paul E. McKenney
In-Reply-To: <alpine.DEB.1.10.0808200941560.13132@gandalf.stny.rr.com>

Steven Rostedt wrote:
> On Wed, 20 Aug 2008, Eran Liberty wrote:
>
>   
>> Steven Rostedt wrote:
>>     
>>> On Wed, 20 Aug 2008, Steven Rostedt wrote:
>>>
>>>   
>>>       
>>>> On Wed, 20 Aug 2008, Benjamin Herrenschmidt wrote:
>>>>
>>>>     
>>>>         
>>>>> Found the problem (or at least -a- problem), it's a gcc bug.
>>>>>
>>>>> Well, first I must say the code generated by -pg is just plain
>>>>> horrible :-)
>>>>>
>>>>> Appart from that, look at the exit of, for example, __d_lookup, as
>>>>> generated by gcc when ftrace is enabled:
>>>>>
>>>>> c00c0498:       38 60 00 00     li      r3,0
>>>>> c00c049c:       81 61 00 00     lwz     r11,0(r1)
>>>>> c00c04a0:       80 0b 00 04     lwz     r0,4(r11)
>>>>> c00c04a4:       7d 61 5b 78     mr      r1,r11
>>>>> c00c04a8:       bb 0b ff e0     lmw     r24,-32(r11)
>>>>> c00c04ac:       7c 08 03 a6     mtlr    r0
>>>>> c00c04b0:       4e 80 00 20     blr
>>>>>
>>>>> As you can see, it restores r1 -before- it pops r24..r31 off
>>>>> the stack ! I let you imagine what happens if an interrupt happens
>>>>> just in between those two instructions (mr and lmw). We don't do
>>>>> redzones on our ABI, so basically, the registers end up corrupted
>>>>> by the interrupt.
>>>>>       
>>>>>           
>>>> Ouch!  You've disassembled this without -pg too, and it does not have this
>>>> bug? What version of gcc do you have?
>>>>
>>>>     
>>>>         
>>> I have:
>>>  gcc (Debian 4.3.1-2) 4.3.1
>>>
>>> c00c64c8:       81 61 00 00     lwz     r11,0(r1)
>>> c00c64cc:       7f 83 e3 78     mr      r3,r28
>>> c00c64d0:       80 0b 00 04     lwz     r0,4(r11)
>>> c00c64d4:       ba eb ff dc     lmw     r23,-36(r11)
>>> c00c64d8:       7d 61 5b 78     mr      r1,r11
>>> c00c64dc:       7c 08 03 a6     mtlr    r0
>>> c00c64e0:       4e 80 00 20     blr
>>>
>>>
>>> My version looks fine.  I'm thinking that this is a separate issue than what
>>> Eran is seeing.
>>>
>>> Eran, can you do an "objdump -dr vmlinux" and search for __d_lookup, and
>>> print out the end of the function dump.
>>>
>>> Thanks,
>>>
>>> -- Steve
>>>
>>>
>>>
>>>   
>>>       
>> powerpc-linux-gnu-objdump -dr --start-address=0xc00bb584 vmlinux | head -n 100
>>
>> vmlinux:     file format elf32-powerpc
>>
>> Disassembly of section .text:
>>
>> c00bb584 <__d_lookup>:
>>     
>
> [...]
>
>   
>> c00bb670:       41 9e 00 50     beq-    cr7,c00bb6c0 <__d_lookup+0x13c>
>> c00bb674:       83 de 00 00     lwz     r30,0(r30)
>> c00bb678:       2f 9e 00 00     cmpwi   cr7,r30,0
>> c00bb67c:       40 9e ff 98     bne+    cr7,c00bb614 <__d_lookup+0x90>
>> c00bb680:       38 60 00 00     li      r3,0
>> c00bb684:       81 61 00 00     lwz     r11,0(r1)
>> c00bb688:       80 0b 00 04     lwz     r0,4(r11)
>> c00bb68c:       7d 61 5b 78     mr      r1,r11
>>     
>
> [ BUG HERE IF INTERRUPT HAPPENS ]
>
>   
>> c00bb690:       bb 0b ff e0     lmw     r24,-32(r11)
>> c00bb694:       7c 08 03 a6     mtlr    r0
>> c00bb698:       4e 80 00 20     blr
>>     
>
> Yep, you have the same bug in your compiler.
>
> -- Steve
>   
Hmm... so whats now?

Is there a way to prove this scenario is indeed the one that caused the 
opps?

-- Liberty

^ permalink raw reply

* Re: [PATCH 1/9] powerpc/44x: Add PowerPC 44x simple platform support
From: Grant Likely @ 2008-08-20 14:15 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev
In-Reply-To: <200808201533.22258.arnd@arndb.de>

On Wed, Aug 20, 2008 at 7:33 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 19 August 2008, Josh Boyer wrote:
>> This adds a common board file for almost all of the "simple" PowerPC 44x
>> boards that exist today.  This is intended to be a single place to add
>> support for boards that do not differ in platform support from most of the
>> evaluation boards that are used as reference platforms.  Boards that have
>> specific requirements or custom hardware setup should still have their own
>> board.c file.
>
> The code looks correct, but since this is going to be example code
> that may get copied into other platforms, I would take extra care
> for coding style:

I second Arnd's comments.  Otherwise, this looks pretty good to me.

Cheers,
g.

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

^ permalink raw reply

* Re: ftrace introduces instability into kernel 2.6.27(-rc2,-rc3)
From: Josh Boyer @ 2008-08-20 14:16 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Paul E. McKenney, Mathieu Desnoyers, linux-kernel, linuxppc-dev,
	Steven Rostedt, Alan Modra, Scott Wood, Eran Liberty
In-Reply-To: <alpine.DEB.1.10.0808200913070.13132@gandalf.stny.rr.com>

On Wed, 2008-08-20 at 09:14 -0400, Steven Rostedt wrote:
> On Wed, 20 Aug 2008, Benjamin Herrenschmidt wrote:
> 
> > Found the problem (or at least -a- problem), it's a gcc bug.
> > 
> > Well, first I must say the code generated by -pg is just plain
> > horrible :-)
> > 
> > Appart from that, look at the exit of, for example, __d_lookup, as
> > generated by gcc when ftrace is enabled:
> > 
> > c00c0498:       38 60 00 00     li      r3,0
> > c00c049c:       81 61 00 00     lwz     r11,0(r1)
> > c00c04a0:       80 0b 00 04     lwz     r0,4(r11)
> > c00c04a4:       7d 61 5b 78     mr      r1,r11
> > c00c04a8:       bb 0b ff e0     lmw     r24,-32(r11)
> > c00c04ac:       7c 08 03 a6     mtlr    r0
> > c00c04b0:       4e 80 00 20     blr
> > 
> > As you can see, it restores r1 -before- it pops r24..r31 off
> > the stack ! I let you imagine what happens if an interrupt happens
> > just in between those two instructions (mr and lmw). We don't do
> > redzones on our ABI, so basically, the registers end up corrupted
> > by the interrupt.
> 
> Ouch!  You've disassembled this without -pg too, and it does not have this 
> bug? What version of gcc do you have?

Segher was looking at this a bit this morning.  He thinks it's really
-fno-omit-frame-pointer that is causing this.  That really shouldn't
even be set on PowerPC, but FTRACE uses select which overrides the
depends on stuff in Kconfig.

Segher can probably tell you more once his email is back up.

josh

^ permalink raw reply

* Re: ftrace introduces instability into kernel 2.6.27(-rc2,-rc3)
From: Steven Rostedt @ 2008-08-20 14:22 UTC (permalink / raw)
  To: Josh Boyer
  Cc: Paul E. McKenney, Mathieu Desnoyers, linux-kernel, linuxppc-dev,
	Steven Rostedt, Alan Modra, Scott Wood, Eran Liberty
In-Reply-To: <1219241819.26429.24.camel@jdub.homelinux.org>


On Wed, 20 Aug 2008, Josh Boyer wrote:
> 
> Segher was looking at this a bit this morning.  He thinks it's really
> -fno-omit-frame-pointer that is causing this.  That really shouldn't
> even be set on PowerPC, but FTRACE uses select which overrides the
> depends on stuff in Kconfig.

I can easily make a patch that makes that select an x86 only.

-- Steve

^ permalink raw reply

* Re: [PATCH 1/9] powerpc/44x: Add PowerPC 44x simple platform support
From: Josh Boyer @ 2008-08-20 14:45 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, Arnd Bergmann
In-Reply-To: <fa686aa40808200715x6c850593p5655d8b5b8e7b61e@mail.gmail.com>

On Wed, 2008-08-20 at 08:15 -0600, Grant Likely wrote:
> On Wed, Aug 20, 2008 at 7:33 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 19 August 2008, Josh Boyer wrote:
> >> This adds a common board file for almost all of the "simple" PowerPC 44x
> >> boards that exist today.  This is intended to be a single place to add
> >> support for boards that do not differ in platform support from most of the
> >> evaluation boards that are used as reference platforms.  Boards that have
> >> specific requirements or custom hardware setup should still have their own
> >> board.c file.
> >
> > The code looks correct, but since this is going to be example code
> > that may get copied into other platforms, I would take extra care
> > for coding style:
> 
> I second Arnd's comments.  Otherwise, this looks pretty good to me.

You should fix 52xx with the same for loop change then, since I
blatantly most of this file from you ;)

josh

^ permalink raw reply

* Re: ftrace introduces instability into kernel 2.6.27(-rc2,-rc3)
From: Josh Boyer @ 2008-08-20 14:50 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Paul E. McKenney, Mathieu Desnoyers, linux-kernel, linuxppc-dev,
	Steven Rostedt, Alan Modra, Scott Wood, Eran Liberty
In-Reply-To: <alpine.DEB.1.10.0808201022080.23571@gandalf.stny.rr.com>

On Wed, 20 Aug 2008 10:22:49 -0400 (EDT)
Steven Rostedt <rostedt@goodmis.org> wrote:

> 
> On Wed, 20 Aug 2008, Josh Boyer wrote:
> > 
> > Segher was looking at this a bit this morning.  He thinks it's really
> > -fno-omit-frame-pointer that is causing this.  That really shouldn't
> > even be set on PowerPC, but FTRACE uses select which overrides the
> > depends on stuff in Kconfig.
> 
> I can easily make a patch that makes that select an x86 only.

That's probably a first step, but you might want to wait until Segher
can fill in more details.  I'm sort of just relaying what he and I were
talking about on IRC.  IIRC, he was testing builds with and without
both -pg and -fno-omit-frame-pointer and found the bug only when
-fno-omit-frame-pointer was present.

josh

^ permalink raw reply

* Re: ftrace introduces instability into kernel 2.6.27(-rc2,-rc3)
From: Jon Smirl @ 2008-08-20 14:55 UTC (permalink / raw)
  To: Eran Liberty
  Cc: Mathieu Desnoyers, linux-kernel, Steven Rostedt, linuxppc-dev,
	Steven Rostedt, Alan Modra, Scott Wood, Paul E. McKenney
In-Reply-To: <48AC23F4.80900@extricom.com>

On 8/20/08, Eran Liberty <liberty@extricom.com> wrote:
> Steven Rostedt wrote:
>
> > On Wed, 20 Aug 2008, Eran Liberty wrote:
> >
> >
> >
> > > Steven Rostedt wrote:
> > >
> > >
> > > > On Wed, 20 Aug 2008, Steven Rostedt wrote:
> > > >
> > > >
> > > >
> > > > > On Wed, 20 Aug 2008, Benjamin Herrenschmidt wrote:
> > > > >
> > > > >
> > > > >
> > > > > > Found the problem (or at least -a- problem), it's a gcc bug.
> > > > > >
> > > > > > Well, first I must say the code generated by -pg is just plain
> > > > > > horrible :-)
> > > > > >
> > > > > > Appart from that, look at the exit of, for example, __d_lookup, as
> > > > > > generated by gcc when ftrace is enabled:
> > > > > >
> > > > > > c00c0498:       38 60 00 00     li      r3,0
> > > > > > c00c049c:       81 61 00 00     lwz     r11,0(r1)
> > > > > > c00c04a0:       80 0b 00 04     lwz     r0,4(r11)
> > > > > > c00c04a4:       7d 61 5b 78     mr      r1,r11
> > > > > > c00c04a8:       bb 0b ff e0     lmw     r24,-32(r11)
> > > > > > c00c04ac:       7c 08 03 a6     mtlr    r0
> > > > > > c00c04b0:       4e 80 00 20     blr
> > > > > >
> > > > > > As you can see, it restores r1 -before- it pops r24..r31 off
> > > > > > the stack ! I let you imagine what happens if an interrupt happens
> > > > > > just in between those two instructions (mr and lmw). We don't do
> > > > > > redzones on our ABI, so basically, the registers end up corrupted
> > > > > > by the interrupt.
> > > > > >
> > > > > >
> > > > > Ouch!  You've disassembled this without -pg too, and it does not
> have this
> > > > > bug? What version of gcc do you have?
> > > > >
> > > > >
> > > > >
> > > > I have:
> > > >  gcc (Debian 4.3.1-2) 4.3.1
> > > >
> > > > c00c64c8:       81 61 00 00     lwz     r11,0(r1)
> > > > c00c64cc:       7f 83 e3 78     mr      r3,r28
> > > > c00c64d0:       80 0b 00 04     lwz     r0,4(r11)
> > > > c00c64d4:       ba eb ff dc     lmw     r23,-36(r11)
> > > > c00c64d8:       7d 61 5b 78     mr      r1,r11
> > > > c00c64dc:       7c 08 03 a6     mtlr    r0
> > > > c00c64e0:       4e 80 00 20     blr
> > > >
> > > >
> > > > My version looks fine.  I'm thinking that this is a separate issue
> than what
> > > > Eran is seeing.
> > > >
> > > > Eran, can you do an "objdump -dr vmlinux" and search for __d_lookup,
> and
> > > > print out the end of the function dump.
> > > >
> > > > Thanks,
> > > >
> > > > -- Steve
> > > >
> > > >
> > > >
> > > >
> > > >
> > > powerpc-linux-gnu-objdump -dr --start-address=0xc00bb584 vmlinux | head
> -n 100
> > >
> > > vmlinux:     file format elf32-powerpc
> > >
> > > Disassembly of section .text:
> > >
> > > c00bb584 <__d_lookup>:
> > >
> > >
> >
> > [...]
> >
> >
> >
> > > c00bb670:       41 9e 00 50     beq-    cr7,c00bb6c0 <__d_lookup+0x13c>
> > > c00bb674:       83 de 00 00     lwz     r30,0(r30)
> > > c00bb678:       2f 9e 00 00     cmpwi   cr7,r30,0
> > > c00bb67c:       40 9e ff 98     bne+    cr7,c00bb614 <__d_lookup+0x90>
> > > c00bb680:       38 60 00 00     li      r3,0
> > > c00bb684:       81 61 00 00     lwz     r11,0(r1)
> > > c00bb688:       80 0b 00 04     lwz     r0,4(r11)
> > > c00bb68c:       7d 61 5b 78     mr      r1,r11
> > >
> > >
> >
> > [ BUG HERE IF INTERRUPT HAPPENS ]
> >
> >
> >
> > > c00bb690:       bb 0b ff e0     lmw     r24,-32(r11)
> > > c00bb694:       7c 08 03 a6     mtlr    r0
> > > c00bb698:       4e 80 00 20     blr
> > >
> > >
> >
> > Yep, you have the same bug in your compiler.
> >
> > -- Steve
> >
> >
>  Hmm... so whats now?
>
>  Is there a way to prove this scenario is indeed the one that caused the
> opps?

Manually edit the broken binary to change the order of the restore and
see if the problem disappears. That will keep everything else
constant.


>
>  -- Liberty
>
>  _______________________________________________
>  Linuxppc-dev mailing list
>  Linuxppc-dev@ozlabs.org
>  https://ozlabs.org/mailman/listinfo/linuxppc-dev
>


-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: powerpc/cell/oprofile: fix mutex locking for spu-oprofile
From: Carl Love @ 2008-08-20 14:58 UTC (permalink / raw)
  To: Robert Richter
  Cc: Arnd Bergmann, linux-kernel, linuxppc-dev, Paul Mackerras,
	oprofile-list, cel, cbe-oss-dev
In-Reply-To: <20080820123907.GP13011@erda.amd.com>


On Wed, 2008-08-20 at 14:39 +0200, Robert Richter wrote:
> On 20.08.08 14:05:31, Arnd Bergmann wrote:
> > On Wednesday 20 August 2008, Robert Richter wrote:
> > > I am fine with the changes with the exception of removing
> > > add_event_entry() from include/linux/oprofile.h. Though there is no
> > > usage of the function also in other architectures anymore, this change
> > > in the API should be discussed on the oprofile mailing list. Please
> > > separate the change in a different patch and submit it to the mailing
> > > list. If there are no objections then, this change can go upstream as
> > > well.
> > 
> > As an explanation, the removal of add_event_entry is the whole point
> > of this patch. add_event_entry must only be called with buffer_mutex
> > held, but buffer_mutex itself is not exported.
> 
> Thanks for pointing this out.
> 

We originally added add_event_entry() to include/linux/oprofile.h
specifically because it was needed for the CELL SPU support.  As it
turns out it the approach was not completely thought through.  We were
using the function call without holding the mutex lock.  As we
discovered later, this can result in corrupting the data put into the
event buffer.  So exposing the function without a way to hold the mutex
lock is actually a really bad idea as it would encourage others to fall
into the same mistake that we made.  So, as Arnd said, the whole point
of this patch is to come up with a correct approach to adding the data.

> > I'm pretty sure that no other user of add_event_entry exists, as it
> > was exported specifically for the SPU support and that never worked.
> > Any other (theoretical) code using it would be broken in the same way
> > and need a corresponding fix.
> > 
> > We can easily leave the declaration in place, but I'd recommend removing
> > it eventually. If you prefer to keep it, how about marking it as
> > __deprecated?
> 
> No, since this is broken by design we remove it. The patch can go
> upstream as it is.
> 
> Thanks,

> -Robert
> 

It really is best to remove it.  Thank you for taking the time to review
and comment on the patch.

               Carl Love

^ permalink raw reply

* boot cuImage on an old u-boot
From: Sebastian Siewior @ 2008-08-20 15:09 UTC (permalink / raw)
  To: linuxppc-dev

I have here a mpc8540ads board and a u-boot 1.0.0. I've build the
defconfig for the board and I tried to boot the genarated
cuImage.mpc8540ads image. After the bootm command I see just

|8540> bootm 1000000
|## Booting image at 01000000 ...
|   Image Name:   Linux-2.6.26
|   Image Type:   PowerPC Linux Kernel Image (gzip compressed)
|   Data Size:    1232711 Bytes =  1.2 MB
|   Load Address: 00400000
|   Entry Point:  004005ac
|   Verifying Checksum ... OK
|   Uncompressing Kernel Image ... OK
|

I don't see the device tree fixups printfs, so crash happend quite
early. Anyone an idea what I could have done wrong?

Sebastian

^ permalink raw reply

* Re: [PATCH 1/9] powerpc/44x: Add PowerPC 44x simple platform support
From: Josh Boyer @ 2008-08-20 15:19 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, Arnd Bergmann
In-Reply-To: <1219243548.26429.25.camel@jdub.homelinux.org>

On Wed, 20 Aug 2008 10:45:47 -0400
Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:

> On Wed, 2008-08-20 at 08:15 -0600, Grant Likely wrote:
> > On Wed, Aug 20, 2008 at 7:33 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Tuesday 19 August 2008, Josh Boyer wrote:
> > >> This adds a common board file for almost all of the "simple" PowerPC 44x
> > >> boards that exist today.  This is intended to be a single place to add
> > >> support for boards that do not differ in platform support from most of the
> > >> evaluation boards that are used as reference platforms.  Boards that have
> > >> specific requirements or custom hardware setup should still have their own
> > >> board.c file.
> > >
> > > The code looks correct, but since this is going to be example code
> > > that may get copied into other platforms, I would take extra care
> > > for coding style:
> > 
> > I second Arnd's comments.  Otherwise, this looks pretty good to me.
> 
> You should fix 52xx with the same for loop change then, since I
> blatantly most of this file from you ;)
           ^ stole

/me sighs.

josh

^ permalink raw reply

* Re: ftrace introduces instability into kernel 2.6.27(-rc2,-rc3)
From: Steven Rostedt @ 2008-08-20 15:23 UTC (permalink / raw)
  To: Jon Smirl
  Cc: Paul E. McKenney, Mathieu Desnoyers, linux-kernel, linuxppc-dev,
	Steven Rostedt, Alan Modra, Scott Wood, Eran Liberty
In-Reply-To: <9e4733910808200755y1128ae56p6a1235684bfbb3ec@mail.gmail.com>


On Wed, 20 Aug 2008, Jon Smirl wrote:
> > >
> > > > c00bb690:       bb 0b ff e0     lmw     r24,-32(r11)
> > > > c00bb694:       7c 08 03 a6     mtlr    r0
> > > > c00bb698:       4e 80 00 20     blr
> > > >
> > > >
> > >
> > > Yep, you have the same bug in your compiler.
> > >
> > > -- Steve
> > >
> > >
> >  Hmm... so whats now?
> >
> >  Is there a way to prove this scenario is indeed the one that caused the
> > opps?
> 
> Manually edit the broken binary to change the order of the restore and
> see if the problem disappears. That will keep everything else
> constant.

Does it only happen on this function? Or is it happening on others as 
well. So manually fixing it here won't help much unless we fix it 
everywhere.

-- Steve

^ permalink raw reply

* Re: ftrace introduces instability into kernel 2.6.27(-rc2,-rc3)
From: Steven Rostedt @ 2008-08-20 15:27 UTC (permalink / raw)
  To: Eran Liberty
  Cc: Mathieu Desnoyers, linux-kernel, linuxppc-dev, Steven Rostedt,
	Alan Modra, Scott Wood, Paul E. McKenney
In-Reply-To: <48AC23F4.80900@extricom.com>


On Wed, 20 Aug 2008, Eran Liberty wrote:
> > 
> >   
> > > c00bb670:       41 9e 00 50     beq-    cr7,c00bb6c0 <__d_lookup+0x13c>
> > > c00bb674:       83 de 00 00     lwz     r30,0(r30)
> > > c00bb678:       2f 9e 00 00     cmpwi   cr7,r30,0
> > > c00bb67c:       40 9e ff 98     bne+    cr7,c00bb614 <__d_lookup+0x90>
> > > c00bb680:       38 60 00 00     li      r3,0
> > > c00bb684:       81 61 00 00     lwz     r11,0(r1)
> > > c00bb688:       80 0b 00 04     lwz     r0,4(r11)
> > > c00bb68c:       7d 61 5b 78     mr      r1,r11
> > >     
> > 
> > [ BUG HERE IF INTERRUPT HAPPENS ]
> > 
> >   
> > > c00bb690:       bb 0b ff e0     lmw     r24,-32(r11)
> > > c00bb694:       7c 08 03 a6     mtlr    r0
> > > c00bb698:       4e 80 00 20     blr
> > >     
> > 
> > Yep, you have the same bug in your compiler.
> > 
> > -- Steve
> >   
> Hmm... so whats now?

Try anothe gcc version ;-)

> 
> Is there a way to prove this scenario is indeed the one that caused the opps?

I'm not convinced that this is the bug you were seeing. Do you still get 
the crash if you disable dynamic ftrace but leave ftrace on?

-- Steve

^ permalink raw reply

* [PATCH v2] powerpc: fix memory leaks in QE library
From: Timur Tabi @ 2008-08-20 15:29 UTC (permalink / raw)
  To: galak, linuxppc-dev, avorontsov, benh, tony

Fix two memory leaks in the Freescale QE library: add a missing kfree() in
ucc_fast_init() and ucc_slow_init() if the ioremap() fails, and update
ucc_fast_free() and ucc_slow_free() to call iounmap() if necessary.

Based on a patch from Tony Breeds <tony@bakeyournoodle.com>.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 arch/powerpc/sysdev/qe_lib/ucc_fast.c |    4 ++++
 arch/powerpc/sysdev/qe_lib/ucc_slow.c |    8 +++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/sysdev/qe_lib/ucc_fast.c b/arch/powerpc/sysdev/qe_lib/ucc_fast.c
index 1aecb07..25fbbfa 100644
--- a/arch/powerpc/sysdev/qe_lib/ucc_fast.c
+++ b/arch/powerpc/sysdev/qe_lib/ucc_fast.c
@@ -208,6 +208,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct ucc_fast_private ** ucc
 	uccf->uf_regs = ioremap(uf_info->regs, sizeof(struct ucc_fast));
 	if (uccf->uf_regs == NULL) {
 		printk(KERN_ERR "%s: Cannot map UCC registers\n", __func__);
+		kfree(uccf);
 		return -ENOMEM;
 	}
 
@@ -355,6 +356,9 @@ void ucc_fast_free(struct ucc_fast_private * uccf)
 	if (uccf->ucc_fast_rx_virtual_fifo_base_offset)
 		qe_muram_free(uccf->ucc_fast_rx_virtual_fifo_base_offset);
 
+	if (uccf->uf_regs)
+		iounmap(uccf->uf_regs);
+
 	kfree(uccf);
 }
 EXPORT_SYMBOL(ucc_fast_free);
diff --git a/arch/powerpc/sysdev/qe_lib/ucc_slow.c b/arch/powerpc/sysdev/qe_lib/ucc_slow.c
index a578bc7..e1d6a13 100644
--- a/arch/powerpc/sysdev/qe_lib/ucc_slow.c
+++ b/arch/powerpc/sysdev/qe_lib/ucc_slow.c
@@ -171,6 +171,7 @@ int ucc_slow_init(struct ucc_slow_info * us_info, struct ucc_slow_private ** ucc
 	uccs->us_regs = ioremap(us_info->regs, sizeof(struct ucc_slow));
 	if (uccs->us_regs == NULL) {
 		printk(KERN_ERR "%s: Cannot map UCC registers\n", __func__);
+		kfree(uccs);
 		return -ENOMEM;
 	}
 
@@ -367,10 +368,11 @@ void ucc_slow_free(struct ucc_slow_private * uccs)
 	if (uccs->tx_base_offset)
 		qe_muram_free(uccs->tx_base_offset);
 
-	if (uccs->us_pram) {
+	if (uccs->us_pram)
 		qe_muram_free(uccs->us_pram_offset);
-		uccs->us_pram = NULL;
-	}
+
+	if (uccs->us_regs)
+		iounmap(uccs->us_regs);
 
 	kfree(uccs);
 }
-- 
1.5.5

^ permalink raw reply related

* cache model of ppc
From: Felix Schmidt @ 2008-08-20 15:30 UTC (permalink / raw)
  To: linuxppc-dev

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
 
Dear,

I some questions about the PowerPC cache model.

How many caches are there?
How many caches are in multi-core systems? do you have a L1 cache per
cpu and one shared L2? or is there a victim L3 cache?

can you tell me something about the path through the cache?
for example I look up for data in L1, but this was a miss. Then i look
up in L2 this was also a miss. but the date will be in another L1
cache of another cpu. Is there a 1.5 or 2.5 hit?

can you help me please?
this is for my bachelorthesis

thanks a lot

felix
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
 
iEYEARECAAYFAkisOIwACgkQmH8OAwYoDBk0ugCgu6zzJEQWA9NldC4ekjOcWgIu
cYIAoMquvbvAf/qA1WmjtROpq+R1X6Yb
=JepB
-----END PGP SIGNATURE-----

^ permalink raw reply

* Re: [PATCH 1/9] powerpc/44x: Add PowerPC 44x simple platform support
From: Roland Dreier @ 2008-08-20 16:33 UTC (permalink / raw)
  To: jwboyer; +Cc: linuxppc-dev, Arnd Bergmann
In-Reply-To: <1219243548.26429.25.camel@jdub.homelinux.org>

 > You should fix 52xx with the same for loop change then, since I
 > blatantly most of this file from you ;)

Heh, then factor out mpc5200_simple_probe() into a helper and use it
instead of copying it as ppc44x_probe?  ;)

If you stick to the NULL-terminated array version, then it becomes easy
to convert some other platform probing code to use your new code too, eg
tqm85xx_probe() could use an array too.

 - R.

^ permalink raw reply

* Re: [PATCH 0/9] Rework PowerPC 44x board support
From: Valentine Barshak @ 2008-08-20 16:51 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev
In-Reply-To: <cover.1219160188.git.jwboyer@linux.vnet.ibm.com>

Josh Boyer wrote:
> The following patch series reworks the board support code for PowerPC 44x
> platforms.  It eliminates a number of redundant <board>.c files and add a
> ppc44x_simple.c file that has an explicit list of boards that are supported
> by it.  This is the same mechanism that Grant Likely has used for MPC 5200
> boards.
> 
> It also adds some more explicit support for Glacier and Yosemite boards, as
> those boards were using a board level compatible property in their DTS files
> that was a bit confusing.
> 
> Review would be appreciated.  Tested on Sequoia, and I plan on testing on as
> many boards as I can before committing to my tree.
> 
> josh

Sorry if I miss anything, but
what happens if I build an uImage just for Rainier (not enabling other 
platforms in the kernel config) and then start it on Canyonlands for 
example?
They both use PowerPC_44x_simple stuff and looks like now we claim that 
all these "simple" boards are compatible. So, board check always passes, 
although Canyonlands support is actually disabled (CONFIG_460EX = n; 
CONFIG_PPC4xx_PCI_EXPRESS = n). It's not critical, but I just thought we 
might also remove the platform-specific options from 
arch/powerpc/platforms/44x/Kconfig for these boards at all. Just enable 
all board-specific stuff under "config PPC44x_SIMPLE".
Otherwise we probably should have a configurable char *board[] array.

Thanks,
Valentine.

^ permalink raw reply

* Re: [PATCH 0/9] Rework PowerPC 44x board support
From: prodyut hazarika @ 2008-08-20 16:54 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev, Stefan Roese
In-Reply-To: <20080819144545.4cebe670@zod.rchland.ibm.com>

Hi Josh,

>> >
>> > Review would be appreciated.  Tested on Sequoia, and I plan on testing on
>> > as many boards as I can before committing to my tree.
>>

The changes looks great.
>
> That would be much appreciated.  I have a number of the effected
> boards, but I don't have them all so any testing you can do would be
> awesome.

I tried pulling up the latest code from your git tree. Seems it is not
yet put in there.
Do I have to manually apply all the patches to my git tree. I was
planning to test the changes on Canyonlands, glacier and Kilauea
boards here.

Thanks,
Prodyut Hazarika

===============
Staff S/W Engineer
AMCC
===============

> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>

^ permalink raw reply

* Re: [PATCH 0/9] Rework PowerPC 44x board support
From: Josh Boyer @ 2008-08-20 17:08 UTC (permalink / raw)
  To: Valentine Barshak; +Cc: linuxppc-dev
In-Reply-To: <48AC4B86.8010902@ru.mvista.com>

On Wed, 20 Aug 2008 20:51:18 +0400
Valentine Barshak <vbarshak@ru.mvista.com> wrote:

> Josh Boyer wrote:
> > The following patch series reworks the board support code for PowerPC 44x
> > platforms.  It eliminates a number of redundant <board>.c files and add a
> > ppc44x_simple.c file that has an explicit list of boards that are supported
> > by it.  This is the same mechanism that Grant Likely has used for MPC 5200
> > boards.
> > 
> > It also adds some more explicit support for Glacier and Yosemite boards, as
> > those boards were using a board level compatible property in their DTS files
> > that was a bit confusing.
> > 
> > Review would be appreciated.  Tested on Sequoia, and I plan on testing on as
> > many boards as I can before committing to my tree.
> > 
> > josh
> 
> Sorry if I miss anything, but
> what happens if I build an uImage just for Rainier (not enabling other 
> platforms in the kernel config) and then start it on Canyonlands for 
> example?

Why would you do that?  If you want that, use ppc44x_defconfig as it
has all the boards enabled.  Also, if you're talking about the default
make targets in the kernel, that would build a cuImage for Rainier, and
I'd expect that to blow up spectacularly on a Canoynlands because the
device tree would be in no way correct.

> They both use PowerPC_44x_simple stuff and looks like now we claim that 
> all these "simple" boards are compatible. So, board check always passes, 

No, we don't claim they are compatible at all.  We simply claim that
they can all be supported by the ppc44x_simple.c platform file.  The
compatible stuff is done through the device tree, and if you look at
the patch series you'll see that I actually removed all the cross-board
compatible statements from the boards that had more than one in the DTS
file.

> although Canyonlands support is actually disabled (CONFIG_460EX = n; 
> CONFIG_PPC4xx_PCI_EXPRESS = n). It's not critical, but I just thought we 
> might also remove the platform-specific options from 
> arch/powerpc/platforms/44x/Kconfig for these boards at all. Just enable 

I left them in there on purpose.  People want to be able to say "I have
a Rainier board.  I only want a kernel for that."  They're also used
for the wrapper stuff to build the right images.

> all board-specific stuff under "config PPC44x_SIMPLE".
> Otherwise we probably should have a configurable char *board[] array.

We could do that if it's _really_ necessary, but I'm not sure it is.

josh

^ permalink raw reply

* Re: [PATCH 0/9] Rework PowerPC 44x board support
From: Josh Boyer @ 2008-08-20 17:09 UTC (permalink / raw)
  To: prodyut hazarika; +Cc: linuxppc-dev, Stefan Roese
In-Reply-To: <49c0ff980808200954p38e00e7aqb612e7e7eb1bbc51@mail.gmail.com>

On Wed, 20 Aug 2008 09:54:40 -0700
"prodyut hazarika" <prodyuth@gmail.com> wrote:

> Hi Josh,
> 
> >> >
> >> > Review would be appreciated.  Tested on Sequoia, and I plan on testing on
> >> > as many boards as I can before committing to my tree.
> >>
> 
> The changes looks great.
> >
> > That would be much appreciated.  I have a number of the effected
> > boards, but I don't have them all so any testing you can do would be
> > awesome.
> 
> I tried pulling up the latest code from your git tree. Seems it is not
> yet put in there.

Right.  I tend to not commit patches that even I send out until they've
at least had some review.

> Do I have to manually apply all the patches to my git tree. I was
> planning to test the changes on Canyonlands, glacier and Kilauea
> boards here.

For now, yes.  In a day or so I'll probably fix the small comments I've
gotten and then commit the series to my next branch.

josh

^ permalink raw reply

* Re: [PATCH 1/9] powerpc/44x: Add PowerPC 44x simple platform support
From: Josh Boyer @ 2008-08-20 17:11 UTC (permalink / raw)
  To: Roland Dreier; +Cc: linuxppc-dev, Bergmann, Arnd
In-Reply-To: <adak5ebpry5.fsf@cisco.com>

On Wed, 20 Aug 2008 09:33:06 -0700
Roland Dreier <rdreier@cisco.com> wrote:

>  > You should fix 52xx with the same for loop change then, since I
>  > blatantly most of this file from you ;)
> 
> Heh, then factor out mpc5200_simple_probe() into a helper and use it
> instead of copying it as ppc44x_probe?  ;)
> 
> If you stick to the NULL-terminated array version, then it becomes easy
> to convert some other platform probing code to use your new code too, eg
> tqm85xx_probe() could use an array too.

Except that logically doesn't make much sense.  Why would you have a
list of mpc52xx and 44x boards together?  They require completely
different kernels because the MMU and drive set is entirely different.

Or am I totally missing what you are saying?

josh

^ permalink raw reply

* Re: [PATCH 1/9] powerpc/44x: Add PowerPC 44x simple platform support
From: Roland Dreier @ 2008-08-20 17:19 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev, Arnd, Bergmann
In-Reply-To: <20080820131124.05cfee9f@zod.rchland.ibm.com>

 > Except that logically doesn't make much sense.  Why would you have a
 > list of mpc52xx and 44x boards together?  They require completely
 > different kernels because the MMU and drive set is entirely different.
 > 
 > Or am I totally missing what you are saying?

Yeah, I wasn't clear -- I meant to add a new helper like
of_flat_dt_is_compatible_list() (not sure of the name) that takes a node
and a NULL-terminated array of strings, and then mpc5200_simple_probe()
can become a one-liner, along with mpc5121_generic_probe(),
tqm85xx_probe(), ppc44x_probe(), etc.

 - R.

^ 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