LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* 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: 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: [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: [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: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 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 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 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: 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 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: [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: 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: 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: 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 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: 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: [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/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: [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 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: [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 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: [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: [RFC] add phy-handle property for fec_mpc52xx
From: Benjamin Herrenschmidt @ 2008-01-10  5:01 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, olaf, sven
In-Reply-To: <20080109163058.21643.30876.stgit@trillian.secretlab.ca>


On Wed, 2008-01-09 at 09:32 -0700, Grant Likely wrote:
> From: Olaf Hering <olaf@aepfle.de>
> 
> The new network driver fec_mpc52xx will not work on efika because the
> firmware does not provide all required properties.
> http://www.powerdeveloper.org/asset/by-id/46 has a Forth script to
> create more properties. But only the phy stuff is required to get a
> working network.
> 
> This should go into the kernel because its appearently
> impossible to boot the script via tftp and then load the real boot
> binary (yaboot or zimage).
> 
> (Olaf's s-o-b line needs to go here)
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

As much as I despise having to work around firmware bugs, it looks
indeed like this one has been broken for way too long to the point where
users are being hurt, distros are being hurt, and telling people to
whack things in nvramrc is just plain gross, so let's merge it.
> ---
> 
> Here's my respin of Olaf's patch to move it to fixup_device_tree_efika()
> and to make it check if the nodes exist before blindly creating them.
> 
> Cheers,
> g.
> 
>  arch/powerpc/kernel/prom_init.c |   39 +++++++++++++++++++++++++++++++++++++++
>  1 files changed, 39 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index 1add6ef..5d89a21 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -2216,6 +2216,45 @@ static void __init fixup_device_tree_efika(void)
>  			prom_printf("fixup_device_tree_efika: ",
>  				"skipped entry %x - setprop error\n", i);
>  	}
> +
> +	/* Make sure ethernet mdio bus node exists */
> +	node = call_prom("finddevice", 1, 1, ADDR("/builtin/mdio"));
> +	if (!PHANDLE_VALID(node)) {
> +		prom_printf("Adding Ethernet MDIO node\n");
> +		call_prom("interpret", 1, 1,
> +			" s\" /builtin\" find-device"
> +			" new-device"
> +				" 1 encode-int s\" #address-cells\" property"
> +				" 0 encode-int s\" #size-cells\" property"
> +				" s\" mdio\" 2dup device-name device-type"
> +				" s\" mpc5200b-fec-phy\" encode-string"
> +				" s\" compatible\" property"
> +				" 0xf0003000 0x400 reg"
> +				" 0x2 encode-int"
> +				" 0x5 encode-int encode+"
> +				" 0x3 encode-int encode+"
> +				" s\" interrupts\" property"
> +			" finish-device");
> +	};
> +
> +	/* Make sure ethernet phy device node exist */
> +	node = call_prom("finddevice", 1, 1, ADDR("/builtin/mdio/ethernet-phy"));
> +	if (!PHANDLE_VALID(node)) {
> +		prom_printf("Adding Ethernet PHY node\n");
> +		call_prom("interpret", 1, 1,
> +			" s\" /builtin/mdio\" find-device"
> +			" new-device"
> +				" s\" ethernet-phy\" device-name"
> +				" 0x10 encode-int s\" reg\" property"
> +				" my-self"
> +				" ihandle>phandle"
> +			" finish-device"
> +			" s\" /builtin/ethernet\" find-device"
> +				" encode-int"
> +				" s\" phy-handle\" property"
> +			" device-end");
> +	}
> +
>  }
>  #else
>  #define fixup_device_tree_efika()
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

^ permalink raw reply

* Re: add phy-handle property for fec_mpc52xx
From: Benjamin Herrenschmidt @ 2008-01-10  5:04 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: linuxppc-dev, Olaf Hering, David Woodhouse
In-Reply-To: <18309.33088.653225.116715@cargo.ozlabs.ibm.com>


On Thu, 2008-01-10 at 13:21 +1100, Paul Mackerras wrote:
> 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.

It's standard OF boot process, the fixup belongs in prom_init. Grant has
a better version of the patch that puts the fixup along with other efika
related ones that are already there.

Cheers,
Ben.

^ 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