LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 1/8] pseries: phyp dump: Docmentation
From: Michael Ellerman @ 2008-01-10  4:52 UTC (permalink / raw)
  To: linasvepstas
  Cc: linuxppc-dev, lkessler, mahuja, Nathan Lynch, Olof Johansson,
	strosake
In-Reply-To: <3ae3aa420801092012m5e47cbd7lc7a5f91842074af7@mail.gmail.com>

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

On Wed, 2008-01-09 at 22:12 -0600, Linas Vepstas wrote:
> On 09/01/2008, Olof Johansson <olof@lixom.net> wrote:
> > On Wed, Jan 09, 2008 at 08:33:53PM -0600, Linas Vepstas wrote:
> >
> > > Heh. That's the elbow-grease of this thing.  The easy part is to get
> > > the core function working. The hard part is to test these various configs,
> > > and when they don't work, figure out what went wrong. That will take
> > > perseverence and brains.
> >
> > This just sounds like a whole lot of extra work to get a feature that
> > already exists.
> 
> Well, no. kexec is horribly ill-behaved with respect to PCI. The
> kexec kernel starts running with PCI devices in some random
> state; maybe they're DMA'ing or who knows what. kexec tries
> real hard to whack a few needed pci devices into submission
> but it has been hit-n-miss, and the source of 90% of the kexec
> headaches and debugging effort. Its not pretty.

Isn't that what EEH and the IOMMU are for? :)

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [PATCH 1/8] pseries: phyp dump: Docmentation
From: Linas Vepstas @ 2008-01-10  4:12 UTC (permalink / raw)
  To: Olof Johansson; +Cc: mahuja, linuxppc-dev, lkessler, Nathan Lynch, strosake
In-Reply-To: <20080110031723.GA22168@lixom.net>

On 09/01/2008, Olof Johansson <olof@lixom.net> wrote:
> On Wed, Jan 09, 2008 at 08:33:53PM -0600, Linas Vepstas wrote:
>
> > Heh. That's the elbow-grease of this thing.  The easy part is to get
> > the core function working. The hard part is to test these various configs,
> > and when they don't work, figure out what went wrong. That will take
> > perseverence and brains.
>
> This just sounds like a whole lot of extra work to get a feature that
> already exists.

Well, no. kexec is horribly ill-behaved with respect to PCI. The
kexec kernel starts running with PCI devices in some random
state; maybe they're DMA'ing or who knows what. kexec tries
real hard to whack a few needed pci devices into submission
but it has been hit-n-miss, and the source of 90% of the kexec
headaches and debugging effort. Its not pretty.

If all pci-host bridges could shut-down or settle the bus, and
raise the #RST line high, and then if all BIOS'es supported
this, you'd be right. But they can't ....

--linas

^ permalink raw reply

* Re: [DTC PATCH 2/2] Preserve scanner state when /include/ing.
From: David Gibson @ 2008-01-10  4:05 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, jdl
In-Reply-To: <20080107202751.GB17782@ld0162-tx32.am.freescale.net>

On Mon, Jan 07, 2008 at 02:27:51PM -0600, Scott Wood wrote:
> This allows /include/s to work when in non-default states,
> such as PROPNODECHAR.
> 
> We may want to use state stacks to get rid of BEGIN_DEFAULT() altogether...

And we should, if we're going to go to stacked states at all.  I was
anticipating we might need to use stacked states for handling
propnamestate as we need expression support.  I just hadn't realised
we already needed it for includes.


-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH 3/3] Return a non-zero exit code if an error occurs during dts parsing.
From: David Gibson @ 2008-01-10  3:56 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, jdl
In-Reply-To: <20080106225509.GC8239@ld0162-tx32.am.freescale.net>

On Sun, Jan 06, 2008 at 04:55:09PM -0600, Scott Wood wrote:
> On Fri, Jan 04, 2008 at 03:30:33PM +1100, David Gibson wrote:
> > This is unequivocally wrong.  boot_info should have information about
> > the contents of the blob, not state information like the error.
> 
> "This blob is invalid" *is* information about the contents of the blob.
> 
> > If you're going to use an ugly global, then use it everywhere.
> 
> Why go out of our way to make the code even less library-able/thread-safe?

It doesn't make it any less thread-safe.  A global variable used some
places is just as bad as a global variable used everywhere from that
point of view, and is more complicated.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH 1/8] pseries: phyp dump: Docmentation
From: Michael Ellerman @ 2008-01-10  3:55 UTC (permalink / raw)
  To: linasvepstas; +Cc: mahuja, linuxppc-dev, lkessler, Nathan Lynch, strosake
In-Reply-To: <3ae3aa420801091847l189df8dwb2348624f0267af@mail.gmail.com>

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

On Wed, 2008-01-09 at 20:47 -0600, Linas Vepstas wrote:
> On 09/01/2008, Michael Ellerman <michael@ellerman.id.au> wrote:
> >
> > > > Only if you can get at rtas, but you can't get at rtas at that point.
> >
> > AFAICT you don't need to get at RTAS, you just need to look at the
> > device tree to see if the property is present, and that is trivial.
> >
> > You probably just need to add a check in early_init_dt_scan_rtas() which
> > sets a flag for the PHYP dump stuff, or add your own scan routine if you
> > need.
> 
> I no longer remember the details. I do remember spending a lot of time
> trying to figure out how to do this. I know I didn't want to write my own scan
> routine; maybe that's what stopped me.  As it happens, we also did most
> of the development on a broken phyp which simply did not even have
> this property, no matter what, and so that may have brain-damaged me.

Sure, the API docs for the kernel are a little lacking ;)

> I went for the "most elegant" solution, where "most elegant" is defined
> as "fewest lines of code", "least effort", etc.
> 
> Manish may need some hands-on help to extract this token during
> early boot.  Hopefully, he'll let us know.

It would just be something like:

--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -901,6 +901,11 @@ int __init early_init_dt_scan_rtas(unsigned long node,
                rtas.size = *sizep;
        }
 
+#ifdef CONFIG_PHYP_DUMP
+       if (of_get_flat_dt_prop(node, "ibm,dump-kernel", NULL))
+               phyp_dump_is_active++;
+#endif
+
 #ifdef CONFIG_UDBG_RTAS_CONSOLE
        basep = of_get_flat_dt_prop(node, "put-term-char", NULL);
        if (basep)


Or to do your own scan routine:


diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index acc0d24..442134e 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -1022,6 +1022,7 @@ void __init early_init_devtree(void *params)
        /* Some machines might need RTAS info for debugging, grab it now. */
        of_scan_flat_dt(early_init_dt_scan_rtas, NULL);
 #endif
+       of_scan_flat_dt(early_init_dt_scan_phyp_dump, NULL);
 
        /* Retrieve various informations from the /chosen node of the
         * device-tree, including the platform type, initrd location and
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 52e95c2..af2b6e8 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -883,6 +883,19 @@ void __init rtas_initialize(void)
 #endif
 }
 
+int __init early_init_dt_scan_phyp_dump(unsigned long node,
+               const char *uname, int depth, void *data)
+{
+#ifdef CONFIG_PHYP_DUMP
+       if (depth != 1 || strcmp(uname, "rtas") != 0)
+               return 0;
+
+       if (of_get_flat_dt_prop(node, "ibm,dump-kernel", NULL))
+               phyp_dump_is_active++;
+#endif
+       return 1;
+}
+
 int __init early_init_dt_scan_rtas(unsigned long node,
                const char *uname, int depth, void *data)
 {


cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply related

* Re: [PATCH 2/3] Look for include files in the directory of the including file.
From: David Gibson @ 2008-01-10  3:52 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, jdl
In-Reply-To: <20080106225252.GB8239@ld0162-tx32.am.freescale.net>

On Sun, Jan 06, 2008 at 04:52:52PM -0600, Scott Wood wrote:
> On Fri, Jan 04, 2008 at 03:27:39PM +1100, David Gibson wrote:
> > > +	newfile = dtc_open_file(filename, searchptr);
> > > +	if (!newfile) {
> > > +		yyerrorf("Couldn't open \"%s\": %s",
> > > +		         filename, strerror(errno));
> > > +		exit(1);
> > 
> > Use die() here, that's what it's for.
> 
> die() doesn't print file and line information.
> 
> > > +	while (search) {
> > > +		if (dtc_open_one(file, search->dir, fname))
> > > +			return file;
> > 
> > Don't we need a different case here somewhere for if someone specifies
> > an include file as an absolute path?  Have I missed something?
> 
> Yeah, I forgot about that, and sent another patch to fix it when I
> noticed (jdl had already pulled, so I didn't send an amended patch).
> 
> > [snip]
> > > +struct search_path {
> > > +	const char *dir; /* NULL for current directory */
> > > +	struct search_path *prev, *next;
> > > +};
> > 
> > I wouldn't suggest a doubly linked list here.  Or at least not without
> > converting our many existing singly linked lists at the same time.
> 
> The doubly-linked list is intended to make it easier to construct search
> path lists one-at-a-time from arguments in the proper order, without
> needing to reverse the list at the end.

We've already got that problem with a bunch of the lists we create
during parsing (we have several ugly add-to-end-of-singly-linked-list
functions).  Going to doubly-linked lists might not be a bad idea, but
we should do it across the board, probably using the kernel's list.h
or something like it.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
From: David Gibson @ 2008-01-10  3:49 UTC (permalink / raw)
  To: Timur Tabi, Jon Smirl, Liam Girdwood, alsa-devel, linuxppc-dev
In-Reply-To: <20080107182853.GA17312@sirena.org.uk>

On Mon, Jan 07, 2008 at 06:28:54PM +0000, Mark Brown wrote:
> On Mon, Jan 07, 2008 at 09:52:03AM -0600, Timur Tabi wrote:
> > David Gibson wrote:
> 
> > > Ok, but couldn't you strucutre your I2S or fabric driver so that it
> > > only becomes fully operational once the codec driver has registered
> > > with it?
> 
> > Not in ASoC V1.  You have to understand, ASoC V1 was designed without any 
> > consideration for runtime-bindings and other OF goodies.  All connections 
> > between the drivers are static, literally.  In fact, I wouldn't be surprised if 
> > some ASoC drivers cannot be compiled as modules.
> 
> I'd just like to emphasise this point - ASoC v1 really doesn't
> understand the idea that the components of the sound subsystem might be
> probed separately.  It's set up to handle bare hardware with everything
> being probed from code in the machine/fabric driver.  This makes life
> very messy for platforms with something like the device tree.
> 
> As has been said, handling this properly is one of the major motivations
> behind ASoC v2.

Ick.  Ok.

Nonetheless, messing up the device tree to workaround ASoC V1's silly
limitations is not a good idea.  The device tree must represent the
hardware as much as possible.  If that means we have to have a bunch
of platform-specific hacks to instatiate the drivers in the correct
order / combination, that's unfortunate, but there you go.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH 2/5] Warp Base Platform - dts
From: David Gibson @ 2008-01-10  3:35 UTC (permalink / raw)
  To: Sean MacLennan; +Cc: linuxppc-dev
In-Reply-To: <47859224.70109@pikatech.com>

On Wed, Jan 09, 2008 at 10:33:56PM -0500, Sean MacLennan wrote:
> Ok, the FPGA is off the EBC, I found it in the documentation.
> 
> Under the ebc, I notice the walnut has @n,m. What are n,m? Are they tied 
> to chip selects?

n is the chipselect, m is the address offset within that chipselect.

> The FPGA is CS2 according to the documentation. Do I make it
> fpga@2,0?

Probably, yes.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH 2/5] Warp Base Platform - dts
From: Josh Boyer @ 2008-01-10  3:36 UTC (permalink / raw)
  To: Sean MacLennan; +Cc: linuxppc-dev
In-Reply-To: <47859224.70109@pikatech.com>

On Wed, 09 Jan 2008 22:33:56 -0500
Sean MacLennan <smaclennan@pikatech.com> wrote:

> Ok, the FPGA is off the EBC, I found it in the documentation.
> 
> Under the ebc, I notice the walnut has @n,m. What are n,m? Are they tied 
> to chip selects?

chip select,offset.

> The FPGA is CS2 according to the documentation. Do I make it fpga@2,0?

If the fpga is on chip select 2, offset 0 from that, yes.  Otherwise,
substitute the proper offset in place of 0.

The ranges property of the EBC node will do the actual address
translation.

josh

^ permalink raw reply

* Re: [PATCH 2/5] Warp Base Platform - dts
From: Sean MacLennan @ 2008-01-10  3:33 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev
In-Reply-To: <20080109211758.26c8e235@zod.rchland.ibm.com>

Ok, the FPGA is off the EBC, I found it in the documentation.

Under the ebc, I notice the walnut has @n,m. What are n,m? Are they tied 
to chip selects?

The FPGA is CS2 according to the documentation. Do I make it fpga@2,0?

Cheers,
    Sean

^ permalink raw reply

* Re: [PATCH 3/5] Warp Base Platform
From: David Gibson @ 2008-01-10  3:29 UTC (permalink / raw)
  To: Sean MacLennan; +Cc: linuxppc-dev
In-Reply-To: <47858E46.9010505@pikatech.com>

On Wed, Jan 09, 2008 at 10:17:26PM -0500, Sean MacLennan wrote:
> David Gibson wrote:
> > On Wed, Jan 09, 2008 at 03:22:46PM -0500, Sean MacLennan wrote:
> >   
> >> Basically the powerpc/boot directory files.
> >>     
> >
> > [snip]
> >   
> >> --- /dev/null	2005-11-20 22:22:37.000000000 -0500
> >> +++ arch/powerpc/boot/cuboot-warp.c	2008-01-08 12:09:39.000000000 -0500
> >> @@ -0,0 +1,25 @@
> >> +/*
> >> + * Copyright (c) 2008 PIKA Technologies
> >> + *   Sean MacLennan <smaclennan@pikatech.com>
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify it
> >> + * under the terms of the GNU General Public License version 2 as published
> >> + * by the Free Software Foundation.
> >> + */
> >> +
> >> +#include "ops.h"
> >> +#include "44x.h"
> >> +#include "cuboot.h"
> >> +
> >> +#define TARGET_44x
> >> +#include "ppcboot.h"
> >> +
> >> +static bd_t bd;
> >> +
> >> +void platform_init(unsigned long r3, unsigned long r4, unsigned long r5,
> >> +				   unsigned long r6, unsigned long r7)
> >> +{
> >> +	CUBOOT_INIT();
> >> +
> >> +	warp_init(&bd.bi_enetaddr, &bd.bi_enet1addr);
> >> +}
> >> --- /dev/null	2005-11-20 22:22:37.000000000 -0500
> >> +++ arch/powerpc/boot/warp.c	2008-01-08 12:09:54.000000000 -0500
> >>     
> >
> > Fold all this into cuboot-warp.c, unless you actually anticipate
> > adding another wrapper for another firmware which will also use the
> > functions in warp.c.
> >
> >   
> Yes, there is still a plan to use the u-boot device tree. Although not 
> in the near feature. I could roll them togeather for now and split them 
> out later.

Yes, but device-tree aware u-boot doesn't need anything platform
specific in the bootwrapper, so won't be a second user of warp.c.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH 2/5] Warp Base Platform - dts
From: Josh Boyer @ 2008-01-10  3:17 UTC (permalink / raw)
  To: Sean MacLennan; +Cc: linuxppc-dev
In-Reply-To: <47858D89.4070706@pikatech.com>

On Wed, 09 Jan 2008 22:14:17 -0500
Sean MacLennan <smaclennan@pikatech.com> wrote:

> David Gibson wrote:
> > On Wed, Jan 09, 2008 at 03:21:07PM -0500, Sean MacLennan wrote:
> >   
> >> Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
> >> ---
> >> --- /dev/null	2005-11-20 22:22:37.000000000 -0500
> >> +++ arch/powerpc/boot/dts/warp.dts	2008-01-08 12:04:10.000000000 -0500
> >>     
> >
> > [snip]
> >   
> >> +	plb {
> >> +		compatible = "ibm,plb-440ep", "ibm,plb-440gp", "ibm,plb4";
> >> +		#address-cells = <2>;
> >> +		#size-cells = <1>;
> >> +		ranges;
> >> +		clock-frequency = <0>; /* Filled in by zImage */
> >> +
> >> +		SDRAM0: sdram {
> >> +			compatible = "ibm,sdram-440ep", "ibm,sdram-405gp";
> >> +			dcr-reg = <010 2>;
> >> +		};
> >> +
> >> +		DMA0: dma {
> >> +			compatible = "ibm,dma-440ep", "ibm,dma-440gp";
> >> +			dcr-reg = <100 027>;
> >> +		};
> >> +
> >> +		FPGA0: fpga {
> >>     
> >
> > Surely this must be off the EBC, not directly of the PLB...?
> >   
> Could be, I don't really know :( I will move it if it makes more sense.

Well, "making more sense" would be finding out where it is on the board
and putting it in the proper place :).

josh

^ permalink raw reply

* Re: [PATCH 3/5] Warp Base Platform
From: Sean MacLennan @ 2008-01-10  3:17 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20080110024926.GD17816@localhost.localdomain>

David Gibson wrote:
> On Wed, Jan 09, 2008 at 03:22:46PM -0500, Sean MacLennan wrote:
>   
>> Basically the powerpc/boot directory files.
>>     
>
> [snip]
>   
>> --- /dev/null	2005-11-20 22:22:37.000000000 -0500
>> +++ arch/powerpc/boot/cuboot-warp.c	2008-01-08 12:09:39.000000000 -0500
>> @@ -0,0 +1,25 @@
>> +/*
>> + * Copyright (c) 2008 PIKA Technologies
>> + *   Sean MacLennan <smaclennan@pikatech.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2 as published
>> + * by the Free Software Foundation.
>> + */
>> +
>> +#include "ops.h"
>> +#include "44x.h"
>> +#include "cuboot.h"
>> +
>> +#define TARGET_44x
>> +#include "ppcboot.h"
>> +
>> +static bd_t bd;
>> +
>> +void platform_init(unsigned long r3, unsigned long r4, unsigned long r5,
>> +				   unsigned long r6, unsigned long r7)
>> +{
>> +	CUBOOT_INIT();
>> +
>> +	warp_init(&bd.bi_enetaddr, &bd.bi_enet1addr);
>> +}
>> --- /dev/null	2005-11-20 22:22:37.000000000 -0500
>> +++ arch/powerpc/boot/warp.c	2008-01-08 12:09:54.000000000 -0500
>>     
>
> Fold all this into cuboot-warp.c, unless you actually anticipate
> adding another wrapper for another firmware which will also use the
> functions in warp.c.
>
>   
Yes, there is still a plan to use the u-boot device tree. Although not 
in the near feature. I could roll them togeather for now and split them 
out later.

Cheers,
    Sean

^ permalink raw reply

* Re: [PATCH 2/5] Warp Base Platform - dts
From: Sean MacLennan @ 2008-01-10  3:14 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20080110024753.GC17816@localhost.localdomain>

David Gibson wrote:
> On Wed, Jan 09, 2008 at 03:21:07PM -0500, Sean MacLennan wrote:
>   
>> Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
>> ---
>> --- /dev/null	2005-11-20 22:22:37.000000000 -0500
>> +++ arch/powerpc/boot/dts/warp.dts	2008-01-08 12:04:10.000000000 -0500
>>     
>
> [snip]
>   
>> +	plb {
>> +		compatible = "ibm,plb-440ep", "ibm,plb-440gp", "ibm,plb4";
>> +		#address-cells = <2>;
>> +		#size-cells = <1>;
>> +		ranges;
>> +		clock-frequency = <0>; /* Filled in by zImage */
>> +
>> +		SDRAM0: sdram {
>> +			compatible = "ibm,sdram-440ep", "ibm,sdram-405gp";
>> +			dcr-reg = <010 2>;
>> +		};
>> +
>> +		DMA0: dma {
>> +			compatible = "ibm,dma-440ep", "ibm,dma-440gp";
>> +			dcr-reg = <100 027>;
>> +		};
>> +
>> +		FPGA0: fpga {
>>     
>
> Surely this must be off the EBC, not directly of the PLB...?
>   
Could be, I don't really know :( I will move it if it makes more sense.

Cheers,
    Sean

^ permalink raw reply

* Re: How complete should the DTS be?
From: David Gibson @ 2008-01-10  3:13 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev, Sean MacLennan
In-Reply-To: <86AA8535-E2CF-4891-900B-340049A5CA19@kernel.crashing.org>

On Tue, Jan 08, 2008 at 12:04:36AM -0600, Kumar Gala wrote:
> 
> On Jan 7, 2008, at 8:07 PM, Sean MacLennan wrote:
> 
> > Just a general question about DTS "completeness". Like all 440EP
> > processors, the taco has two i2c buses. However, only one bus has
> > anything connected to it.
> >
> > Should I show both bus entries in the DTS, or only the one that is  
> > used?
> > I have generally only been showing the devices that are present. i.e.
> > Only one emac, only one serial port.
> >
> > Is there a convention for this?
> 
> The .dts should reflect the HW as its used.  On some reference boards  
> we might put out more info because of the various configs these types  
> of boards can be setup in.  However if something has a static config  
> just describe that.  So in your example of two i2c buses with only one  
> connected, just describe the one that is used.

Hrm... I'd say this is not something which has a firm convention yet.

It's going to become more of an issue once we get a macros system for
dtc, so the "440EP" macro would have all the devices, even if some are
not connected on a given board.

I'm contemplating suggesting that we adopt the "status" property from
IEEE1275 to cover this.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH 1/8] pseries: phyp dump: Docmentation
From: Olof Johansson @ 2008-01-10  3:17 UTC (permalink / raw)
  To: Linas Vepstas; +Cc: mahuja, linuxppc-dev, lkessler, Nathan Lynch, strosake
In-Reply-To: <3ae3aa420801091833i6cf32616o2a060579be1f3191@mail.gmail.com>

On Wed, Jan 09, 2008 at 08:33:53PM -0600, Linas Vepstas wrote:

> Heh. That's the elbow-grease of this thing.  The easy part is to get
> the core function working. The hard part is to test these various configs,
> and when they don't work, figure out what went wrong. That will take
> perseverence and brains.

This just sounds like a whole lot of extra work to get a feature that
already exists. Also, features like these seem to just get tested when the
next enterprise distro is released, so they're broken for long stretches
of time in mainline.

There's a bunch of problems like the NUMA ones, which would by far be
easiest to solve by just doing another reboot or kexec, wouldn't they?


-Olof

^ permalink raw reply

* Re: [PATCH 4/7] sbc8560: Add device tree source for Wind River SBC8560 board
From: David Gibson @ 2008-01-10  2:56 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: linuxppc-dev
In-Reply-To: <d10d083bafb978936975111f09669120201072ec.1199715362.git.paul.gortmaker@windriver.com>

On Mon, Jan 07, 2008 at 09:25:29AM -0500, Paul Gortmaker wrote:
> This adds the device tree source for the Wind River SBC8560 board.  The
> biggest difference between this and the MPC8560ADS reference platform
> dts is the use of an external 16550 compatible UART instead of the CPM2.
> 
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>  arch/powerpc/boot/dts/sbc8560.dts |  285 +++++++++++++++++++++++++++++++++++++

[snip]
> +	epld@fc000000 {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "localbus";

This compatible doesn't look specific enough.  It should at least have
a vendor prefix.

> +		ranges = <0 fc000000 00c00000>;

Typically, we've been doing these external bust controller type
gadgets with address-cells = <2>, the first cell explicitly encoding
the chipselect.  This gets us closer to the ideal of the device tree
encoding only hardware information, not how the bridge controller is
configured (although "ranges" will still have to contain configuration
dependent information).


> +
> +		serial0: serial@700000 {
> +			device_type = "serial";
> +			compatible = "ns16550";
> +			reg = <700000 100>;
> +			clock-frequency = <1C2000>;
> +			interrupts = <9 2>;
> +			interrupt-parent = <&mpic>;
> +		};
> +
> +		serial1: serial@800000 {
> +			device_type = "serial";
> +			compatible = "ns16550";
> +			reg = <800000 100>;
> +			clock-frequency = <1C2000>;
> +			interrupts = <a 2>;
> +			interrupt-parent = <&mpic>;
> +		};
> +
> +		rtc@900000 {
> +			compatible = "m48t59";
> +			reg = <900000 2000>;
> +		};
> +	};
> +};

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH 3/5] Warp Base Platform
From: David Gibson @ 2008-01-10  2:49 UTC (permalink / raw)
  To: Sean MacLennan; +Cc: linuxppc-dev
In-Reply-To: <47852D16.2070906@pikatech.com>

On Wed, Jan 09, 2008 at 03:22:46PM -0500, Sean MacLennan wrote:
> Basically the powerpc/boot directory files.

[snip]
> --- /dev/null	2005-11-20 22:22:37.000000000 -0500
> +++ arch/powerpc/boot/cuboot-warp.c	2008-01-08 12:09:39.000000000 -0500
> @@ -0,0 +1,25 @@
> +/*
> + * Copyright (c) 2008 PIKA Technologies
> + *   Sean MacLennan <smaclennan@pikatech.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> + * by the Free Software Foundation.
> + */
> +
> +#include "ops.h"
> +#include "44x.h"
> +#include "cuboot.h"
> +
> +#define TARGET_44x
> +#include "ppcboot.h"
> +
> +static bd_t bd;
> +
> +void platform_init(unsigned long r3, unsigned long r4, unsigned long r5,
> +				   unsigned long r6, unsigned long r7)
> +{
> +	CUBOOT_INIT();
> +
> +	warp_init(&bd.bi_enetaddr, &bd.bi_enet1addr);
> +}
> --- /dev/null	2005-11-20 22:22:37.000000000 -0500
> +++ arch/powerpc/boot/warp.c	2008-01-08 12:09:54.000000000 -0500

Fold all this into cuboot-warp.c, unless you actually anticipate
adding another wrapper for another firmware which will also use the
functions in warp.c.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH 1/8] pseries: phyp dump: Docmentation
From: Linas Vepstas @ 2008-01-10  2:47 UTC (permalink / raw)
  To: michael; +Cc: mahuja, linuxppc-dev, lkessler, Nathan Lynch, strosake
In-Reply-To: <1199919545.7880.11.camel@concordia>

On 09/01/2008, Michael Ellerman <michael@ellerman.id.au> wrote:
>
> > > Only if you can get at rtas, but you can't get at rtas at that point.
>
> AFAICT you don't need to get at RTAS, you just need to look at the
> device tree to see if the property is present, and that is trivial.
>
> You probably just need to add a check in early_init_dt_scan_rtas() which
> sets a flag for the PHYP dump stuff, or add your own scan routine if you
> need.

I no longer remember the details. I do remember spending a lot of time
trying to figure out how to do this. I know I didn't want to write my own scan
routine; maybe that's what stopped me.  As it happens, we also did most
of the development on a broken phyp which simply did not even have
this property, no matter what, and so that may have brain-damaged me.

I went for the "most elegant" solution, where "most elegant" is defined
as "fewest lines of code", "least effort", etc.

Manish may need some hands-on help to extract this token during
early boot.  Hopefully, he'll let us know.

--linas

^ permalink raw reply

* Re: [PATCH 2/5] Warp Base Platform - dts
From: David Gibson @ 2008-01-10  2:47 UTC (permalink / raw)
  To: Sean MacLennan; +Cc: linuxppc-dev
In-Reply-To: <47852CB3.3000208@pikatech.com>

On Wed, Jan 09, 2008 at 03:21:07PM -0500, Sean MacLennan wrote:
> Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
> ---
> --- /dev/null	2005-11-20 22:22:37.000000000 -0500
> +++ arch/powerpc/boot/dts/warp.dts	2008-01-08 12:04:10.000000000 -0500

[snip]
> +	plb {
> +		compatible = "ibm,plb-440ep", "ibm,plb-440gp", "ibm,plb4";
> +		#address-cells = <2>;
> +		#size-cells = <1>;
> +		ranges;
> +		clock-frequency = <0>; /* Filled in by zImage */
> +
> +		SDRAM0: sdram {
> +			compatible = "ibm,sdram-440ep", "ibm,sdram-405gp";
> +			dcr-reg = <010 2>;
> +		};
> +
> +		DMA0: dma {
> +			compatible = "ibm,dma-440ep", "ibm,dma-440gp";
> +			dcr-reg = <100 027>;
> +		};
> +
> +		FPGA0: fpga {

Surely this must be off the EBC, not directly of the PLB...?

> +			compatible = "pika,fpga";
> +	   		reg = <0 80000000 2200>;
> +			interrupts = <18 8>;
> +			interrupt-parent = <&UIC0>;
> +		};

[snip]
> +			IIC0: i2c@ef600700 {
> +				device_type = "i2c";

Drop this device_type.

> +				compatible = "ibm,iic-440ep", "ibm,iic-440gp", "ibm,iic";
> +				reg = <ef600700 14>;
> +				interrupt-parent = <&UIC0>;
> +				interrupts = <2 4>;
> +			};
> +
> +			ZMII0: emac-zmii@ef600d00 {
> +				device_type = "zmii-interface";

And this one.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH 1/8] pseries: phyp dump: Docmentation
From: Linas Vepstas @ 2008-01-10  2:33 UTC (permalink / raw)
  To: Nathan Lynch; +Cc: mahuja, linuxppc-dev, lkessler, strosake
In-Reply-To: <20080109184437.GU14201@localdomain>

On 09/01/2008, Nathan Lynch <ntl@pobox.com> wrote:
> Hi Linas,
>
> Linas Vepstas wrote:
> >
> > As a side effect, the system is in
> > production *while* the dump is being taken;
>
> A dubious feature IMO.

Hmm.  Take it up with Ken Rozendal, this is supposed to be
one of the two main selling points of this thing.

> Seems that the design potentially trades
> reliability of first failure data capture for availability.
> E.g. system crashes, reboots, resumes processing while copying dump,
> crashes again before dump procedure is complete.  How is that handled,
> if at all?

Its handled by the hypervisor.  phyp maintains the copy of the
RMO of  first crash, until such time that the OS declares the
dump of the RMO to be complete. So you'll always have
the RMO of the first crash.

For the rest of RAM, it will come in two parts: some portion
will have been dumped already. The rest has not yet been dumped,
and it will still be there, preserved across the second crash.

So you get both RMO and all of RAM from the first crash.

> > with kdump,
> > you can't go into production until after the dump is finished,
> > and the system has been rebooted a second time.  On
> > systems with terabytes of RAM, the time difference can be
> > hours.
>
> The difference in time it takes to resume the normal workload may be
> significant, yes.  But the time it takes to get a usable dump image
> would seem to be the basically the same.

Yes.

> Since you bring up large systems... a system with terabytes of RAM is
> practically guaranteed to be a NUMA configuration with dozens of cpus.
> When processing a dump on such a system, I wonder how well we fare:
> can we successfully boot with (say) 128 cpus and 256MB of usable
> memory?  Do we have to hot-online nodes as system memory is freed up
> (and does that even work)?  We need to be able to restore the system
> to its optimal topology when the dump is finished; if the best we can
> do is a degraded configuration, the workload will suffer and the
> system admin is likely to just reboot the machine again so the kernel
> will have the right NUMA topology.

Heh. That's the elbow-grease of this thing.  The easy part is to get
the core function working. The hard part is to test these various configs,
and when they don't work, figure out what went wrong. That will take
perseverence and brains.

--linas

^ permalink raw reply

* Re: add phy-handle property for fec_mpc52xx
From: Paul Mackerras @ 2008-01-10  2:21 UTC (permalink / raw)
  To: David Woodhouse; +Cc: Olaf Hering, linuxppc-dev
In-Reply-To: <1199893757.2978.73.camel@pmac.infradead.org>

David Woodhouse writes:

> It would be much better if the kernel would 'just work'. The ideal way
> of achieving that is for the firmware to be fixed -- but that's been
> promised for a long time now, and we just don't believe you any more. So
> working round it in the kernel seems to be the next best option.

Does the efika use a boot wrapper?  If so then putting the fixup in
the boot wrapper might be better.

Paul.

^ permalink raw reply

* Re: add phy-handle property for fec_mpc52xx
From: Paul Mackerras @ 2008-01-10  2:20 UTC (permalink / raw)
  To: Olaf Hering; +Cc: linuxppc-dev
In-Reply-To: <20080109140608.GA15673@aepfle.de>

Olaf Hering writes:

> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -1487,6 +1487,34 @@ static void __init prom_find_mmu(void)
>  	else if (strncmp(version, "FirmWorks,3.", 12) == 0) {
>  		of_workarounds = OF_WA_CLAIM | OF_WA_LONGTRAIL;
>  		call_prom("interpret", 1, 1, "dev /memory 0 to allow-reclaim");
> +#ifdef CONFIG_PPC_MPC52xx
> +	} else if (strcmp(version, "EFIKA5K2") == 0) {

Why have you added this stuff in prom_find_mmu rather than putting it
in fixup_device_tree_efika()?

Paul.

^ permalink raw reply

* Re: [PATCH 4/7] Device tree for MPC5121 ADS
From: David Gibson @ 2008-01-10  2:18 UTC (permalink / raw)
  To: John Rigby; +Cc: linuxppc-dev
In-Reply-To: <1199808093-15929-5-git-send-email-jrigby@freescale.com>

On Tue, Jan 08, 2008 at 09:01:30AM -0700, John Rigby wrote:
> Bare minimum tree containing only
> what is currently supported.

[snip]
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		PowerPC,5121@0 {
> +			device_type = "cpu";
> +			reg = <0>;
> +			d-cache-line-size = <20>;	// 32 bytes
> +			i-cache-line-size = <20>;	// 32 bytes
> +			d-cache-size = <8000>;		// L1, 32K
> +			i-cache-size = <8000>;		// L1, 32K
> +			ref-frequency = <3ef1480>;	// 66MHz ref clock
> +			timebase-frequency = <2f34f60>; // 49.5MHz (396MHz/8) makes time tick correctly
> +			bus-frequency = <bcd3d80>;	// 198MHz csb bus
> +			clock-frequency = <179a7b00>;	// 396MHz ppc core ??
> +			32-bit;

The "32-bit" property was only ever added by mistake.  Drop it.

[snip]
> +	cpld@82000000 {
> +		device_type = "board-control";

No device_type here.  But you should have a "compatible" property.

[snip]
> +	soc5121@80000000 {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		#interrupt-cells = <2>;
> +		device_type = "soc";
> +		ranges = <0 80000000 400000>;
> +		reg = <80000000 400000>;
> +		ref-frequency = <3ef1480>;	// 66MHz ref

What the hell is ref-frequency?  Unfortunately, you have to work with
existing broken practice for SoC nodes here, but the principle clock
frequency for any device should always be encoded in a property called
"clock-frequency".

[snip]
> +		ipic: pic@c00 {

Should be "interrupt-controller@c00"

> +			interrupt-controller;
> +			#address-cells = <0>;
> +			#interrupt-cells = <2>;
> +			reg = <c00 100>;
> +			built-in;
> +			device_type = "ipic";

Drop this device_type.  Should have a compatible value instead.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH 2/3] [libata] pata_of_platform: OF-Platform PATA device driver
From: Stephen Rothwell @ 2008-01-10  2:17 UTC (permalink / raw)
  To: Olof Johansson; +Cc: Paul Mundt, linuxppc-dev, Jeff Garzik, linux-ide
In-Reply-To: <20080110003634.GA19966@lixom.net>

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

On Wed, 9 Jan 2008 18:36:34 -0600 Olof Johansson <olof@lixom.net> wrote:
>
> On Thu, Jan 10, 2008 at 10:40:48AM +1100, Stephen Rothwell wrote:
> > Hi Anton,
> > 
> > Juts one small trivial comment (could be fixed later).
> > 
> > On Wed, 9 Jan 2008 22:10:41 +0300 Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
> > >
> > > +static struct of_device_id pata_of_platform_match[] = {
> > 
> > This could be declared const.
> 
> Good point, but let's not hold up merge based on this. Need something
> for janitors to do too, and it's good enough to merge as-is. :)

Absolutely. To me that is what "(could be fixed later)" means.  :-)

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

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

^ 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