LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] powerpc: add new required termio functions
From: Geoff Levand @ 2007-09-12  2:19 UTC (permalink / raw)
  To: Michael Neuling
  Cc: linux-kernel, linuxppc-dev, paulus, Alan Cox, akpm,
	Linus Torvalds, David S. Miller
In-Reply-To: <8018.1189562679@neuling.org>

Michael Neuling wrote:
> The "tty: termios locking functions break with new termios type" patch
> (f629307c857c030d5a3dd777fee37c8bb395e171) breaks the powerpc compile.
> This adds the required API to asm-powerpc.
> 
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> --
> This needs to go up for 2.6.23.
> 
> Should we really put these definitions in asm-generic/termios.h as I'm
> guessing other architectures are broken too?

I think it would be better to do so, as that is where we pickup the defs for
the original kernel_termios_to_user_termios and user_termios_to_kernel_termios.

-Geoff

^ permalink raw reply

* Re: [PATCH] powerpc: add new required termio functions
From: Tony Breeds @ 2007-09-12  2:33 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Michael Neuling, linux-kernel, linuxppc-dev, paulus, Alan Cox,
	akpm, David S. Miller
In-Reply-To: <alpine.LFD.0.999.0709111915350.16478@woody.linux-foundation.org>

On Tue, Sep 11, 2007 at 07:17:42PM -0700, Linus Torvalds wrote:
 
> Really?
> 
> It shouldn't. The use of kernel_termios_to_user_termios_1() is conditional 
> on the architecture having a define for TCGETS2, and I think they match 
> up. I see:
> 
> 	[torvalds@woody linux]$ git grep -l kernel_termios_to_user_termios_1 include | wc -l
> 	10
> 	[torvalds@woody linux]$ git grep -l TCGETS2 include | wc -l
> 	10
> 
> and in neither case is ppc in that list of architecures.
> 
> So maybe you just read the patch without actually testing whether it 
> actually broke powerpc?
> 
> Or is something subtler going on?

As far as I can see TIOCSLCKTRMIOS and TIOCGLCKTRMIOS aren't protected
by TCGETS2 guards.  Do they need to be ...  Perhaps


From: Tony Breeds <tony@bakeyournoodle.com>

Add Guards around TIOCSLCKTRMIOS and TIOCGLCKTRMIOS.

Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>

---

 drivers/char/tty_ioctl.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/char/tty_ioctl.c b/drivers/char/tty_ioctl.c
index 4a8969c..3ee73cf 100644
--- a/drivers/char/tty_ioctl.c
+++ b/drivers/char/tty_ioctl.c
@@ -795,6 +795,19 @@ int n_tty_ioctl(struct tty_struct * tty, struct file * file,
 			if (L_ICANON(tty))
 				retval = inq_canon(tty);
 			return put_user(retval, (unsigned int __user *) arg);
+#ifndef TCGETS2
+		case TIOCGLCKTRMIOS:
+			if (kernel_termios_to_user_termios((struct termios __user *)arg, real_tty->termios_locked))
+				return -EFAULT;
+			return 0;
+
+		case TIOCSLCKTRMIOS:
+			if (!capable(CAP_SYS_ADMIN))
+				return -EPERM;
+			if (user_termios_to_kernel_termios(real_tty->termios_locked, (struct termios __user *) arg))
+				return -EFAULT;
+			return 0;
+#else
 		case TIOCGLCKTRMIOS:
 			if (kernel_termios_to_user_termios_1((struct termios __user *)arg, real_tty->termios_locked))
 				return -EFAULT;
@@ -806,6 +819,7 @@ int n_tty_ioctl(struct tty_struct * tty, struct file * file,
 			if (user_termios_to_kernel_termios_1(real_tty->termios_locked, (struct termios __user *) arg))
 				return -EFAULT;
 			return 0;
+#endif
 
 		case TIOCPKT:
 		{

Yours Tony

  linux.conf.au        http://linux.conf.au/ || http://lca2008.linux.org.au/
  Jan 28 - Feb 02 2008 The Australian Linux Technical Conference!

^ permalink raw reply related

* Re: [PATCH] powerpc: add new required termio functions
From: Michael Neuling @ 2007-09-12  2:34 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-kernel, linuxppc-dev, paulus, Alan Cox, akpm,
	David S. Miller
In-Reply-To: <alpine.LFD.0.999.0709111915350.16478@woody.linux-foundation.org>

> On Wed, 12 Sep 2007, Michael Neuling wrote:
> >
> > The "tty: termios locking functions break with new termios type" patch
> > (f629307c857c030d5a3dd777fee37c8bb395e171) breaks the powerpc compile.
> 
> Really?
> 
> It shouldn't. The use of kernel_termios_to_user_termios_1() is conditional 
> on the architecture having a define for TCGETS2, and I think they match 
> up. I see:
> 
> 	[torvalds@woody linux]$ git grep -l kernel_termios_to_user_termios_1 in
clude | wc -l
> 	10
> 	[torvalds@woody linux]$ git grep -l TCGETS2 include | wc -l
> 	10
> 
> and in neither case is ppc in that list of architecures.
> 
> So maybe you just read the patch without actually testing whether it 
> actually broke powerpc?

Not, I actually compiled it.

> Or is something subtler going on?

Looks like those new calls are not protected by the TCGETS2 define.
Adding those ifdefs seems like the correct fix.  

Mikey

^ permalink raw reply

* Re: [PATCH] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port
From: David Gibson @ 2007-09-12  3:00 UTC (permalink / raw)
  To: Kumar Gala; +Cc: Olof Johansson, linuxppc-dev
In-Reply-To: <C1272D92-42A0-43E4-9AE0-12161178AAA2@kernel.crashing.org>

On Tue, Sep 11, 2007 at 12:59:22PM -0500, Kumar Gala wrote:
> 
> On Sep 11, 2007, at 12:48 PM, Scott Wood wrote:
> 
> > Olof Johansson wrote:
> >> On Tue, Sep 11, 2007 at 12:21:30PM -0500, Scott Wood wrote:
> >>> Olof Johansson wrote:
> >>>> On Tue, Sep 11, 2007 at 11:00:28AM -0500, Kumar Gala wrote:
> >>>>> well the ifdefs are orthogonal.  We don't have a way of knowing
> >>>>> primary from the device tree today.
> >>>> How about something like "fsl,primary-phb" in the bus device  
> >>>> node? I don't
> >>>> know, maybe it's already been discussed and turned down for some  
> >>>> reason.
> >>> It's more of a Linux issue than anything to do with the hardware.
> >>
> >> That doesn't stop firmware from telling linux which bus is the  
> >> primary
> >> one on the system to help out.
> >
> > The entire notion of a "primary" PCI bus is due to a Linux flaw.
> >
> > If we did put it in the device tree, it should be something like
> > "linux,primary-phb".  But since Linux can tell from the node's  
> > children,
> > there doesn't seem to be much point.
> 
> Once someone rights code to do this I'm happy to change over.  I took  
> this model of explicitly knowing the primary PHB from the pmac code.

In the meantime, couldn't the code still be merged, using an explicit
test of the root node's 'compatible' or 'model' properties to decide
on the right primary bus.

-- 
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 v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port
From: David Gibson @ 2007-09-12  3:11 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <Pine.LNX.4.64.0709111436290.14121@blarg.am.freescale.net>

On Tue, Sep 11, 2007 at 02:37:56PM -0500, Kumar Gala wrote:
> Added basic board port for MPC8572 DS reference platform that is
> similiar to the MPC8544/33 DS reference platform in uniprocessor mode.
> 
> ---
> 
> Rev 3 -- updates to the device tree based on discussion on how we want
> to handle memory busses going forward.

[snip]
> +		mdio@24520 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			device_type = "mdio";

I don't think we actually have an mdio device_type binding defined.


> +			compatible = "gianfar";

This needs to be more specific.  And it certainly shouldn't be the
same compatible string as the ethernet MACs have.

> +			reg = <24520 20>;
> +			phy0: ethernet-phy@0 {
> +				interrupt-parent = <&mpic>;
> +				interrupts = <a 1>;
> +				reg = <0>;
> +				device_type = "ethernet-phy";

Do we actually have an ethernet-phy device_type binding?  If not, we
should kill this.  'compatible' properties for phys would probably be
a good idea, though (giving the actual phy model).

> +			};
> +			phy1: ethernet-phy@1 {
> +				interrupt-parent = <&mpic>;
> +				interrupts = <a 1>;
> +				reg = <1>;
> +				device_type = "ethernet-phy";
> +			};
> +			phy2: ethernet-phy@2 {
> +				interrupt-parent = <&mpic>;
> +				interrupts = <a 1>;
> +				reg = <2>;
> +				device_type = "ethernet-phy";
> +			};
> +			phy3: ethernet-phy@3 {
> +				interrupt-parent = <&mpic>;
> +				interrupts = <a 1>;
> +				reg = <3>;
> +				device_type = "ethernet-phy";
> +			};
> +		};
> +
> +		ethernet@24000 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			device_type = "network";
> +			model = "eTSEC";
> +			compatible = "gianfar";
> +			reg = <24000 1000>;
> +			local-mac-address = [ 00 00 00 00 00 00 ];
> +			interrupts = <1d 2 1e 2 22 2>;
> +			interrupt-parent = <&mpic>;
> +			phy-handle = <&phy0>;
> +			phy-connection-type = "rgmii-id";
> +		};
> +
> +		ethernet@25000 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			device_type = "network";
> +			model = "eTSEC";
> +			compatible = "gianfar";
> +			reg = <25000 1000>;
> +			local-mac-address = [ 00 00 00 00 00 00 ];
> +			interrupts = <23 2 24 2 28 2>;
> +			interrupt-parent = <&mpic>;
> +			phy-handle = <&phy1>;
> +			phy-connection-type = "rgmii-id";
> +		};
> +
> +		ethernet@26000 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			device_type = "network";
> +			model = "eTSEC";
> +			compatible = "gianfar";
> +			reg = <26000 1000>;
> +			local-mac-address = [ 00 00 00 00 00 00 ];
> +			interrupts = <1f 2 20 2 21 2>;
> +			interrupt-parent = <&mpic>;
> +			phy-handle = <&phy2>;
> +			phy-connection-type = "rgmii-id";
> +		};
> +
> +		ethernet@27000 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			device_type = "network";
> +			model = "eTSEC";
> +			compatible = "gianfar";
> +			reg = <27000 1000>;
> +			local-mac-address = [ 00 00 00 00 00 00 ];
> +			interrupts = <25 2 26 2 27 2>;
> +			interrupt-parent = <&mpic>;
> +			phy-handle = <&phy3>;
> +			phy-connection-type = "rgmii-id";
> +		};


> +
> +		serial@4500 {
> +			device_type = "serial";
> +			compatible = "ns16550";
> +			reg = <4500 100>;
> +			clock-frequency = <0>;
> +			interrupts = <2a 2>;
> +			interrupt-parent = <&mpic>;
> +		};
> +
> +		serial@4600 {
> +			device_type = "serial";
> +			compatible = "ns16550";
> +			reg = <4600 100>;
> +			clock-frequency = <0>;
> +			interrupts = <2a 2>;
> +			interrupt-parent = <&mpic>;
> +		};
> +
> +		global-utilities@e0000 {	//global utilities block
> +			compatible = "fsl,mpc8548-guts";

Hrm... this should have "fsp,mpc8572-guts" in addition to the older
model with which it is compatible.

> +			reg = <e0000 1000>;
> +			fsl,has-rstcr;
> +		};
> +
> +		mpic: pic@40000 {
> +			clock-frequency = <0>;
> +			interrupt-controller;
> +			#address-cells = <0>;
> +			#interrupt-cells = <2>;
> +			reg = <40000 40000>;
> +			compatible = "chrp,open-pic";
> +			device_type = "open-pic";
> +			big-endian;
> +		};
> +	};
> +
> +	pcie@ffe08000 {
> +		compatible = "fsl,mpc8548-pcie";

And again, "fsl,mpc8572-pcie", "fsl,mpc8548-pcie".

> +		device_type = "pci";
> +		#interrupt-cells = <1>;
> +		#size-cells = <2>;
> +		#address-cells = <3>;
> +		reg = <ffe08000 1000>;
> +		bus-range = <0 ff>;
> +		ranges = <02000000 0 80000000 80000000 0 20000000
> +			  01000000 0 00000000 ffc00000 0 00010000>;
> +		clock-frequency = <1fca055>;
> +		interrupt-parent = <&mpic>;
> +		interrupts = <18 2>;
> +		interrupt-map-mask = <fb00 0 0 0>;
> +		interrupt-map = <
> +			/* IDSEL 0x11 - PCI slot 1 */
> +			8800 0 0 1 &mpic 2 1
> +			8800 0 0 2 &mpic 3 1
> +			8800 0 0 3 &mpic 4 1
> +			8800 0 0 4 &mpic 1 1
> +
> +			/* IDSEL 0x12 - PCI slot 2 */
> +			9000 0 0 1 &mpic 3 1
> +			9000 0 0 2 &mpic 4 1
> +			9000 0 0 3 &mpic 1 1
> +			9000 0 0 4 &mpic 2 1
> +
> +			// IDSEL 0x1c  USB
> +			e000 0 0 0 &i8259 c 2
> +			e100 0 0 0 &i8259 9 2
> +			e200 0 0 0 &i8259 a 2
> +			e300 0 0 0 &i8259 b 2
> +
> +			// IDSEL 0x1d  Audio
> +			e800 0 0 0 &i8259 6 2
> +
> +			// IDSEL 0x1e Legacy
> +			f000 0 0 0 &i8259 7 2
> +			f100 0 0 0 &i8259 7 2
> +
> +			// IDSEL 0x1f IDE/SATA
> +			f800 0 0 0 &i8259 e 2
> +			f900 0 0 0 &i8259 5 2
> +
> +			>;
> +		uli1575@0 {
> +			reg = <0 0 0 0 0>;

This looks kind of bogus...

> +			#size-cells = <2>;
> +			#address-cells = <3>;
> +			ranges = <02000000 0 80000000
> +				  02000000 0 80000000
> +				  0 20000000
> +				  01000000 0 00000000
> +				  01000000 0 00000000
> +				  0 00100000>;
> +
> +			pci_bridge@0 {

Ok.. why is pci_bridge nested within uli1575 - with the matching reg
and ranges, it looks like they ought to be one device.  Also if this
is a PCI<->PCI bridge, I believe it shold have device_type = "pci".

> +				reg = <0 0 0 0 0>;
> +				#size-cells = <2>;
> +				#address-cells = <3>;
> +				ranges = <02000000 0 80000000
> +					  02000000 0 80000000
> +					  0 20000000
> +					  01000000 0 00000000
> +					  01000000 0 00000000
> +					  0 00100000>;
> +
> +				isa@1e {
> +					device_type = "isa";
> +					#interrupt-cells = <2>;
> +					#size-cells = <1>;
> +					#address-cells = <2>;
> +					reg = <f000 0 0 0 0>;
> +					ranges = <1 0 01000000 0 0
> +						  00001000>;
> +					interrupt-parent = <&i8259>;
> +
> +					i8259: interrupt-controller@20 {
> +						reg = <1 20 2
> +						       1 a0 2
> +						       1 4d0 2>;
> +						clock-frequency = <0>;

Hrm.. what is clock-frequency for on an i8259?  I see that other 8259
descriptions have this as well, so it's not a problem with this patch
specifically.

-- 
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: SYSFS: need a noncaching read
From: Michael Ellerman @ 2007-09-12  3:18 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev, Heiko Schocher, linux-kernel, Detlev Zundel
In-Reply-To: <20070912020526.GD16001@localhost.localdomain>

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

On Wed, 2007-09-12 at 12:05 +1000, David Gibson wrote:
> On Tue, Sep 11, 2007 at 11:43:17AM +0200, Heiko Schocher wrote:
> > Hello,
> > 
> > I have developed a device driver and use the sysFS to export some
> > registers to userspace. I opened the sysFS File for one register and did
> > some reads from this File, but I alwas becoming the same value from the
> > register, whats not OK, because they are changing. So I found out that
> > the sysFS caches the reads ... :-(
> > 
> > Is there a way to retrigger the reads (in that way, that the sysFS
> > rereads the values from the driver), without closing and opening the
> > sysFS Files? Or must I better use the ioctl () Driver-interface for
> > exporting these registers?
> > 
> > I am asking this, because I must read every 10 ms 2 registers, so
> > doing a open/read/close for reading one registers is a little bit too
> > much overhead.
> > 
> > I made a sysFS seek function, which retriggers the read, and that works
> > fine, but I have again 2 syscalls, whats also is not optimal.
> > 
> > Or can we make a open () with a (new?)Flag, that informs the sysFS to
> > always reread the values from the underlying driver?
> > 
> > Or a new flag in the "struct attribute_group" in include/linux/sysfs.h,
> > which let the sysfs rereading the values?
> 
> This sounds more like sysfs is really not the right interface for
> polling your registers.  You would probably be better off having your
> driver export a character device from which the register values could
> be read.

I thought relay(fs) was the trendy way to do this these days?

Documentation/filesystems/relay.txt

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] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port
From: Kumar Gala @ 2007-09-12  3:35 UTC (permalink / raw)
  To: David Gibson; +Cc: Olof Johansson, linuxppc-dev
In-Reply-To: <20070912030005.GA20218@localhost.localdomain>


On Sep 11, 2007, at 10:00 PM, David Gibson wrote:

> On Tue, Sep 11, 2007 at 12:59:22PM -0500, Kumar Gala wrote:
>>
>> On Sep 11, 2007, at 12:48 PM, Scott Wood wrote:
>>
>>> Olof Johansson wrote:
>>>> On Tue, Sep 11, 2007 at 12:21:30PM -0500, Scott Wood wrote:
>>>>> Olof Johansson wrote:
>>>>>> On Tue, Sep 11, 2007 at 11:00:28AM -0500, Kumar Gala wrote:
>>>>>>> well the ifdefs are orthogonal.  We don't have a way of knowing
>>>>>>> primary from the device tree today.
>>>>>> How about something like "fsl,primary-phb" in the bus device
>>>>>> node? I don't
>>>>>> know, maybe it's already been discussed and turned down for some
>>>>>> reason.
>>>>> It's more of a Linux issue than anything to do with the hardware.
>>>>
>>>> That doesn't stop firmware from telling linux which bus is the
>>>> primary
>>>> one on the system to help out.
>>>
>>> The entire notion of a "primary" PCI bus is due to a Linux flaw.
>>>
>>> If we did put it in the device tree, it should be something like
>>> "linux,primary-phb".  But since Linux can tell from the node's
>>> children,
>>> there doesn't seem to be much point.
>>
>> Once someone rights code to do this I'm happy to change over.  I took
>> this model of explicitly knowing the primary PHB from the pmac code.
>
> In the meantime, couldn't the code still be merged, using an explicit
> test of the root node's 'compatible' or 'model' properties to decide
> on the right primary bus.

I will be, I'm not going to wait on having some device tree spec for  
this.  The board code can handle it until we come to some agreement  
on how to do this.  I'm in agreement with Scott in that code should  
be added to scan or allow explicit determination.  Adding a 'prop' to  
the device tree just for linux seems a bit silly.

- k

^ permalink raw reply

* Re: [PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port
From: Kumar Gala @ 2007-09-12  3:33 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20070912031132.GC20218@localhost.localdomain>


On Sep 11, 2007, at 10:11 PM, David Gibson wrote:

> On Tue, Sep 11, 2007 at 02:37:56PM -0500, Kumar Gala wrote:
>> Added basic board port for MPC8572 DS reference platform that is
>> similiar to the MPC8544/33 DS reference platform in uniprocessor  
>> mode.
>>
>> ---
>>
>> Rev 3 -- updates to the device tree based on discussion on how we  
>> want
>> to handle memory busses going forward.
>
> [snip]
>> +		mdio@24520 {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +			device_type = "mdio";
>
> I don't think we actually have an mdio device_type binding defined.

We've had this on 83xx/85xx/86xx device trees for a far amount of  
time now.  Look at section 2a in booting-without-of.txt

>
>
>> +			compatible = "gianfar";
>
> This needs to be more specific.  And it certainly shouldn't be the
> same compatible string as the ethernet MACs have.

Why not its the same controller?  Again, we've been doing this for  
some period of time already.

>
>> +			reg = <24520 20>;
>> +			phy0: ethernet-phy@0 {
>> +				interrupt-parent = <&mpic>;
>> +				interrupts = <a 1>;
>> +				reg = <0>;
>> +				device_type = "ethernet-phy";
>
> Do we actually have an ethernet-phy device_type binding?  If not, we
> should kill this.  'compatible' properties for phys would probably be
> a good idea, though (giving the actual phy model).

Look section 2c in booting-without-of.txt

>> +			};
>> +			phy1: ethernet-phy@1 {
>> +				interrupt-parent = <&mpic>;
>> +				interrupts = <a 1>;
>> +				reg = <1>;
>> +				device_type = "ethernet-phy";
>> +			};
>> +			phy2: ethernet-phy@2 {
>> +				interrupt-parent = <&mpic>;
>> +				interrupts = <a 1>;
>> +				reg = <2>;
>> +				device_type = "ethernet-phy";
>> +			};
>> +			phy3: ethernet-phy@3 {
>> +				interrupt-parent = <&mpic>;
>> +				interrupts = <a 1>;
>> +				reg = <3>;
>> +				device_type = "ethernet-phy";
>> +			};
>> +		};

[snip]

>>
>> +		global-utilities@e0000 {	//global utilities block
>> +			compatible = "fsl,mpc8548-guts";
>
> Hrm... this should have "fsp,mpc8572-guts" in addition to the older
> model with which it is compatible.

I've changed this to just fsl,mpc8572-guts

>
>> +			reg = <e0000 1000>;
>> +			fsl,has-rstcr;
>> +		};
>> +
>> +		mpic: pic@40000 {
>> +			clock-frequency = <0>;
>> +			interrupt-controller;
>> +			#address-cells = <0>;
>> +			#interrupt-cells = <2>;
>> +			reg = <40000 40000>;
>> +			compatible = "chrp,open-pic";
>> +			device_type = "open-pic";
>> +			big-endian;
>> +		};
>> +	};
>> +
>> +	pcie@ffe08000 {
>> +		compatible = "fsl,mpc8548-pcie";
>
> And again, "fsl,mpc8572-pcie", "fsl,mpc8548-pcie".

But why?  there is no difference between the PCIe controller in  
mpc8548 and mpc8572?

>
>> +		device_type = "pci";
>> +		#interrupt-cells = <1>;
>> +		#size-cells = <2>;
>> +		#address-cells = <3>;
>> +		reg = <ffe08000 1000>;
>> +		bus-range = <0 ff>;
>> +		ranges = <02000000 0 80000000 80000000 0 20000000
>> +			  01000000 0 00000000 ffc00000 0 00010000>;
>> +		clock-frequency = <1fca055>;
>> +		interrupt-parent = <&mpic>;
>> +		interrupts = <18 2>;
>> +		interrupt-map-mask = <fb00 0 0 0>;
>> +		interrupt-map = <
>> +			/* IDSEL 0x11 - PCI slot 1 */
>> +			8800 0 0 1 &mpic 2 1
>> +			8800 0 0 2 &mpic 3 1
>> +			8800 0 0 3 &mpic 4 1
>> +			8800 0 0 4 &mpic 1 1
>> +
>> +			/* IDSEL 0x12 - PCI slot 2 */
>> +			9000 0 0 1 &mpic 3 1
>> +			9000 0 0 2 &mpic 4 1
>> +			9000 0 0 3 &mpic 1 1
>> +			9000 0 0 4 &mpic 2 1
>> +
>> +			// IDSEL 0x1c  USB
>> +			e000 0 0 0 &i8259 c 2
>> +			e100 0 0 0 &i8259 9 2
>> +			e200 0 0 0 &i8259 a 2
>> +			e300 0 0 0 &i8259 b 2
>> +
>> +			// IDSEL 0x1d  Audio
>> +			e800 0 0 0 &i8259 6 2
>> +
>> +			// IDSEL 0x1e Legacy
>> +			f000 0 0 0 &i8259 7 2
>> +			f100 0 0 0 &i8259 7 2
>> +
>> +			// IDSEL 0x1f IDE/SATA
>> +			f800 0 0 0 &i8259 e 2
>> +			f900 0 0 0 &i8259 5 2
>> +
>> +			>;
>> +		uli1575@0 {
>> +			reg = <0 0 0 0 0>;
>
> This looks kind of bogus...

Its a PCIe to PCI bridge that is transparent.

>> +			#size-cells = <2>;
>> +			#address-cells = <3>;
>> +			ranges = <02000000 0 80000000
>> +				  02000000 0 80000000
>> +				  0 20000000
>> +				  01000000 0 00000000
>> +				  01000000 0 00000000
>> +				  0 00100000>;
>> +
>> +			pci_bridge@0 {
>
> Ok.. why is pci_bridge nested within uli1575 - with the matching reg
> and ranges, it looks like they ought to be one device.  Also if this
> is a PCI<->PCI bridge, I believe it shold have device_type = "pci".

We've been using this as it stands for a while.  If there are some  
changes here that make sense I'm willing to make them.

>> +				reg = <0 0 0 0 0>;
>> +				#size-cells = <2>;
>> +				#address-cells = <3>;
>> +				ranges = <02000000 0 80000000
>> +					  02000000 0 80000000
>> +					  0 20000000
>> +					  01000000 0 00000000
>> +					  01000000 0 00000000
>> +					  0 00100000>;
>> +
>> +				isa@1e {
>> +					device_type = "isa";
>> +					#interrupt-cells = <2>;
>> +					#size-cells = <1>;
>> +					#address-cells = <2>;
>> +					reg = <f000 0 0 0 0>;
>> +					ranges = <1 0 01000000 0 0
>> +						  00001000>;
>> +					interrupt-parent = <&i8259>;
>> +
>> +					i8259: interrupt-controller@20 {
>> +						reg = <1 20 2
>> +						       1 a0 2
>> +						       1 4d0 2>;
>> +						clock-frequency = <0>;
>
> Hrm.. what is clock-frequency for on an i8259?  I see that other 8259
> descriptions have this as well, so it's not a problem with this patch
> specifically.

Its a copy-paste thing so I don't know.

- k

^ permalink raw reply

* Re: [PATCH] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port
From: Olof Johansson @ 2007-09-12  3:37 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev, David Gibson
In-Reply-To: <0F762D4F-C84B-4979-92D4-42C5D40AE455@kernel.crashing.org>

On Tue, Sep 11, 2007 at 10:35:01PM -0500, Kumar Gala wrote:
> > In the meantime, couldn't the code still be merged, using an explicit
> > test of the root node's 'compatible' or 'model' properties to decide
> > on the right primary bus.
> 
> I will be, I'm not going to wait on having some device tree spec for  
> this.  The board code can handle it until we come to some agreement  
> on how to do this.  I'm in agreement with Scott in that code should  
> be added to scan or allow explicit determination.  Adding a 'prop' to  
> the device tree just for linux seems a bit silly.

Sounds good to me as well.


-Olof

^ permalink raw reply

* Re: [PATCH] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port
From: David Gibson @ 2007-09-12  3:37 UTC (permalink / raw)
  To: Kumar Gala; +Cc: Olof Johansson, linuxppc-dev
In-Reply-To: <0F762D4F-C84B-4979-92D4-42C5D40AE455@kernel.crashing.org>

On Tue, Sep 11, 2007 at 10:35:01PM -0500, Kumar Gala wrote:
> 
> On Sep 11, 2007, at 10:00 PM, David Gibson wrote:
> 
> > On Tue, Sep 11, 2007 at 12:59:22PM -0500, Kumar Gala wrote:
> >>
> >> On Sep 11, 2007, at 12:48 PM, Scott Wood wrote:
> >>
> >>> Olof Johansson wrote:
> >>>> On Tue, Sep 11, 2007 at 12:21:30PM -0500, Scott Wood wrote:
> >>>>> Olof Johansson wrote:
> >>>>>> On Tue, Sep 11, 2007 at 11:00:28AM -0500, Kumar Gala wrote:
> >>>>>>> well the ifdefs are orthogonal.  We don't have a way of knowing
> >>>>>>> primary from the device tree today.
> >>>>>> How about something like "fsl,primary-phb" in the bus device
> >>>>>> node? I don't
> >>>>>> know, maybe it's already been discussed and turned down for some
> >>>>>> reason.
> >>>>> It's more of a Linux issue than anything to do with the hardware.
> >>>>
> >>>> That doesn't stop firmware from telling linux which bus is the
> >>>> primary
> >>>> one on the system to help out.
> >>>
> >>> The entire notion of a "primary" PCI bus is due to a Linux flaw.
> >>>
> >>> If we did put it in the device tree, it should be something like
> >>> "linux,primary-phb".  But since Linux can tell from the node's
> >>> children,
> >>> there doesn't seem to be much point.
> >>
> >> Once someone rights code to do this I'm happy to change over.  I took
> >> this model of explicitly knowing the primary PHB from the pmac code.
> >
> > In the meantime, couldn't the code still be merged, using an explicit
> > test of the root node's 'compatible' or 'model' properties to decide
> > on the right primary bus.
> 
> I will be, I'm not going to wait on having some device tree spec for  
> this.  The board code can handle it until we come to some agreement  
> on how to do this.  I'm in agreement with Scott in that code should  
> be added to scan or allow explicit determination.  Adding a 'prop' to  
> the device tree just for linux seems a bit silly.

Yes, saw that in your new version after I'd posted that.  Sorry.

-- 
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 v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port
From: David Gibson @ 2007-09-12  3:53 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <AC5C6EF4-EE8E-487F-BF45-3653A5C2617D@kernel.crashing.org>

On Tue, Sep 11, 2007 at 10:33:20PM -0500, Kumar Gala wrote:
> 
> On Sep 11, 2007, at 10:11 PM, David Gibson wrote:
> 
> > On Tue, Sep 11, 2007 at 02:37:56PM -0500, Kumar Gala wrote:
> >> Added basic board port for MPC8572 DS reference platform that is
> >> similiar to the MPC8544/33 DS reference platform in uniprocessor  
> >> mode.
> >>
> >> ---
> >>
> >> Rev 3 -- updates to the device tree based on discussion on how we  
> >> want
> >> to handle memory busses going forward.
> >
> > [snip]
> >> +		mdio@24520 {
> >> +			#address-cells = <1>;
> >> +			#size-cells = <0>;
> >> +			device_type = "mdio";
> >
> > I don't think we actually have an mdio device_type binding defined.
> 
> We've had this on 83xx/85xx/86xx device trees for a far amount of  
> time now.  Look at section 2a in booting-without-of.txt

Ah, so we have; sorry.  Although the binding as it is currently
written is pretty much pointless - it should actually define some
mappings between dt properties / addresses and the standards defining
the MDIO bus.x

> >
> >> +			compatible = "gianfar";
> >
> > This needs to be more specific.  And it certainly shouldn't be the
> > same compatible string as the ethernet MACs have.
> 
> Why not its the same controller?  Again, we've been doing this for  
> some period of time already.

Yes you have, but it's still crap.  'compatible' should be sufficient
to distinguish the driver needed for device nodes, but the MACs and
MDIO should clearly have different drivers (or at least, different
parts of a driver).

> >> +			reg = <24520 20>;
> >> +			phy0: ethernet-phy@0 {
> >> +				interrupt-parent = <&mpic>;
> >> +				interrupts = <a 1>;
> >> +				reg = <0>;
> >> +				device_type = "ethernet-phy";
> >
> > Do we actually have an ethernet-phy device_type binding?  If not, we
> > should kill this.  'compatible' properties for phys would probably be
> > a good idea, though (giving the actual phy model).
> 
> Look section 2c in booting-without-of.txt

Ah, yes.  That one's a rather less redeemable pointless device_type
binding.  We should kill it from booting-without-of.txt.

> >> +			reg = <e0000 1000>;
> >> +			fsl,has-rstcr;
> >> +		};
> >> +
> >> +		mpic: pic@40000 {
> >> +			clock-frequency = <0>;
> >> +			interrupt-controller;
> >> +			#address-cells = <0>;
> >> +			#interrupt-cells = <2>;
> >> +			reg = <40000 40000>;
> >> +			compatible = "chrp,open-pic";
> >> +			device_type = "open-pic";
> >> +			big-endian;
> >> +		};
> >> +	};
> >> +
> >> +	pcie@ffe08000 {
> >> +		compatible = "fsl,mpc8548-pcie";
> >
> > And again, "fsl,mpc8572-pcie", "fsl,mpc8548-pcie".
> 
> But why?  there is no difference between the PCIe controller in  
> mpc8548 and mpc8572?

As far as you've yet discovered...

> >> +		uli1575@0 {
> >> +			reg = <0 0 0 0 0>;
> >
> > This looks kind of bogus...
> 
> Its a PCIe to PCI bridge that is transparent.

Right.... if it has no control registers, I think it should just lack
'reg', not define a zero-length register block.

> >> +			#size-cells = <2>;
> >> +			#address-cells = <3>;
> >> +			ranges = <02000000 0 80000000
> >> +				  02000000 0 80000000
> >> +				  0 20000000
> >> +				  01000000 0 00000000
> >> +				  01000000 0 00000000
> >> +				  0 00100000>;

And if truly transparent, it should perhaps have just ranges;
indicating that child addresses are identity mapped to parent
addresses.

> >> +
> >> +			pci_bridge@0 {
> >
> > Ok.. why is pci_bridge nested within uli1575 - with the matching reg
> > and ranges, it looks like they ought to be one device.  Also if this
> > is a PCI<->PCI bridge, I believe it shold have device_type = "pci".
> 
> We've been using this as it stands for a while.  If there are some  
> changes here that make sense I'm willing to make them.

Right, at present I don't see why you couldn't just ditch the
pci_bridge node, and drop its contents straight into the uli1575 node.

-- 
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

* Whitespace changes to define_machine(mpc885_ads).
From: Tony Breeds @ 2007-09-12  3:58 UTC (permalink / raw)
  To: LinuxPPC-dev, Paul Mackerras

Make the define_machine() block for mpc885_ads more greppable and consistent
with other examples in tree.

Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>

---

 arch/powerpc/platforms/8xx/mpc885ads_setup.c |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

Index: working/arch/powerpc/platforms/8xx/mpc885ads_setup.c
===================================================================
--- working.orig/arch/powerpc/platforms/8xx/mpc885ads_setup.c	2007-09-10 16:56:54.000000000 +1000
+++ working/arch/powerpc/platforms/8xx/mpc885ads_setup.c	2007-09-12 13:53:17.000000000 +1000
@@ -441,9 +441,14 @@ static int __init mpc885ads_probe(void)
 
 define_machine(mpc885_ads)
 {
-.name = "MPC885 ADS",.probe = mpc885ads_probe,.setup_arch =
-	    mpc885ads_setup_arch,.init_IRQ =
-	    m8xx_pic_init,.show_cpuinfo = mpc8xx_show_cpuinfo,.get_irq =
-	    mpc8xx_get_irq,.restart = mpc8xx_restart,.calibrate_decr =
-	    mpc8xx_calibrate_decr,.set_rtc_time =
-	    mpc8xx_set_rtc_time,.get_rtc_time = mpc8xx_get_rtc_time,};
+	.name            = "MPC885 ADS",
+	.probe           = mpc885ads_probe,
+	.setup_arch      = mpc885ads_setup_arch,
+	.init_IRQ        = m8xx_pic_init,
+	.show_cpuinfo    = mpc8xx_show_cpuinfo,
+	.get_irq         = mpc8xx_get_irq,
+	.restart         = mpc8xx_restart,
+	.calibrate_decr  = mpc8xx_calibrate_decr,
+	.set_rtc_time    = mpc8xx_set_rtc_time,
+	.get_rtc_time    = mpc8xx_get_rtc_time,
+};

Yours Tony

  linux.conf.au        http://linux.conf.au/ || http://lca2008.linux.org.au/
  Jan 28 - Feb 02 2008 The Australian Linux Technical Conference!

^ permalink raw reply

* Re: writing to flash from linux
From: Ratheesh @ 2007-09-12  4:08 UTC (permalink / raw)
  To: Yoni Levin; +Cc: linuxppc-embedded
In-Reply-To: <1E2D24B6DA8A4EE68CFFD33998B2BC26.MAI@mail.livedns.co.il>

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

Hi,
     You can write into flash from linux if you have mapped the flash in mtd
partition. you can open /dev/mtdx(partition that covers your flash
area) file and use normal read/write function calls to read/write from the
flash. Remember to erase the flash before attempting to write data. You can
refer sourcecode of mtd-utils.

Regards
Ratheesh


On 9/11/07, Yoni Levin <yoni.l@slyde-tech.com> wrote:
>
>  Hi , I have EN29LV640H flash (http://www.eonsdi.com/pdf/EN29LV640.pdf)
>
> On my mpc83xx board.
>
> How can I write data to flash from linux.?
>
> I guess it done with mtd , there is an example somewhere?
>
> Thanks.
>
>
>
> _______________________________________________
> Linuxppc-embedded mailing list
> Linuxppc-embedded@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-embedded
>

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

^ permalink raw reply

* Re: [RFC/PATCH] Implement {read,update}_persistent_clock. v2
From: Tony Breeds @ 2007-09-12  4:56 UTC (permalink / raw)
  To: Milton Miller; +Cc: ppcdev
In-Reply-To: <4d781cd1a4751e20098cae29aa4d2b60@bga.com>

On Tue, Sep 11, 2007 at 09:34:13AM -0500, Milton Miller wrote:
 
> Previously we called ppc_md.get_boot_time at most once.  How about 
> moving the check for it into the if (first) block?

Yup on investigatiion it looks like moving it inside the "if (fist)" is
safe.
 
> Have you tested with a platform that doesn't implement get_rtc_time?

Well no I haven't.  AFAICT without my patch read_persistent_clock() is a
weak symbol that always returns 0.  So any platform that doesn't have a
ppc_md.get_boot_time() or ppc_md.get_rtc_time() will still get a 0 at
all the call sites.

I'll locate a machine and verify I'm not making things worse.

Yours Tony

  linux.conf.au        http://linux.conf.au/ || http://lca2008.linux.org.au/
  Jan 28 - Feb 02 2008 The Australian Linux Technical Conference!

^ permalink raw reply

* Re: SYSFS: need a noncaching read
From: Robert Schwebel @ 2007-09-12  5:32 UTC (permalink / raw)
  To: Heiko Schocher; +Cc: linuxppc-dev, linux-kernel, Detlev Zundel
In-Reply-To: <1189503798.6674.46.camel@Zeus.EmbLux>

On Tue, Sep 11, 2007 at 11:43:17AM +0200, Heiko Schocher wrote:
> I have developed a device driver and use the sysFS to export some
> registers to userspace.

Uuuh, uggly. Don't do that. Device drivers are there to abstract things,
not to play around with registers from userspace.

> I opened the sysFS File for one register and did some reads from this
> File, but I alwas becoming the same value from the register, whats not
> OK, because they are changing. So I found out that the sysFS caches
> the reads ... :-(

Yes, it does. What you can do is close()ing the file handle between
accesses, which makes it work but is slow.

> Is there a way to retrigger the reads (in that way, that the sysFS
> rereads the values from the driver), without closing and opening the
> sysFS Files? Or must I better use the ioctl () Driver-interface for
> exporting these registers?

What kind of problem do you want to solve? Userspace is for
applications, and applications usually don't have to know about hardware
details like registers. So if you have to do something every 10 ms from
userspace, your design is probably wrong.

If you absolutely need to do such things from userspace, have a look at
uio. But in most cases the answer is: make a proper abstraction for the
problem you wanna solve and write a proper driver.

Robert
-- 
Pengutronix - Linux Solutions for Science and Industry
Entwicklungszentrum Nord     http://www.pengutronix.de

^ permalink raw reply

* Re: Which 2.6 kernel for MPC5200
From: Robert Schwebel @ 2007-09-12  6:28 UTC (permalink / raw)
  To: Jon Smirl; +Cc: Babarovic Ivica, linuxppc-embedded
In-Reply-To: <9e4733910709101622u6978baa6n8618975f75ae1315@mail.gmail.com>

Jon,

On Mon, Sep 10, 2007 at 07:22:39PM -0400, Jon Smirl wrote:
> > Then you need to apply the bestcomm patches. You can get them here:
> >
> > http://www.246tnt.com/mpc52xx/dev_full/
>
> I just finished rebasing the Efika version of those patches to current mainline.
>
> I'm working on merging support for my Phytec pcm030 into them.

We have the OSELAS.BSP patch stack for the PCM030 working ontop of
Linus' git. It still uses an older version of the Bestcom patches, so if
you do some work in that area, please keep me informed about your
progress.

Robert
-- 
Pengutronix - Linux Solutions for Science and Industry
Entwicklungszentrum Nord     http://www.pengutronix.de

^ permalink raw reply

* RE: [PATCH] [POWERPC] Fix interrupt routing and setup of ULI M1575 onFSL boards
From: Swarthout Edward L-SWARTHOU @ 2007-09-12  6:33 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <Pine.LNX.4.64.0708170003210.21006@blarg.am.freescale.net>

From: Kumar Gala:
...
> Fixed RTC support that requires a dummy memory read on the P2P
> bridge to unlock the RTC and setup the default of the RTC alarm
> registers to match with a basic x86 style CMOS RTC.
...
> diff --git a/arch/powerpc/platforms/fsl_uli1575.c
...
> +/* We have to do a dummy read on the P2P for the RTC to work, WTF
> +static void __devinit quirk_final_uli5249(struct pci_dev *dev)
...
> +	for (i =3D 0; i < PCI_BUS_NUM_RESOURCES; i++) {
> +		if ((bus->resource[i]) &&
> +			(bus->resource[i]->flags & IORESOURCE_MEM)) {
> +			dummy =3D ioremap(bus->resource[i]->start, 0x4);
> +			if (dummy) {
> +				in_8(dummy);

This read (to resource->start) can cause a hang on 8572ds when there
is a PCI card plugged into the uli slot and the card is configured
(but not enabled) in the memory space.

The read is going to the slot, 2.11, which has 0x8000_0000 assigned.
=20
To be safe, the read needs to go to a device internal to the uli.

-ELS

^ permalink raw reply

* Re: PCI target implementation on AMCC PPC CPUs
From: Matthias Fuchs @ 2007-09-12  7:17 UTC (permalink / raw)
  To: David Hawkins; +Cc: Leonid, linuxppc-embedded
In-Reply-To: <46E6D131.2030904@ovro.caltech.edu>

Hi David,

I must admit, that the AMCC PowerPC's PCI interrupt capabilities could
be better:-) In both directions the host CPU has to do PCI
configuration cycles either to generate or acknowledge an interrupt.
The later is problematic for some OS coming from Redmond: you 
have to do pci configuration cycles from interrupt level - and
these OS do not 'like' that. 

In later designs and where possible we also switched to alternative
interrupt mechanisms (GPIO for target to host and gated flags in FPGA
registers for the other direction). Multiple interrupt sources 
are identified by messages that are written to the other sides
memory.

I think we should stop this discussion because its a little bit
off-topic on this list.

Matthias


On Tuesday 11 September 2007 19:32, David Hawkins wrote:
> Hi Matthias,
> 
> > we build a couple of PCI target designs using AMCC PowerPCs.
> > You are right that some things could be better. But ..
> > 
> > On Thursday 06 September 2007 22:26, David Hawkins wrote:
> >> There are several fundamental problems with the AMCC 440EP
> >> acting as a PCI target/slave;
> >>
> >> 2. Look in the data sheet and see if you can figure out
> >>     how the host processor can generate an interrupt to
> >>     the PowerPC core ... oops, you can't. That kind of
> >>     makes it difficult to work with doesn't it.
> >
> > You CAN! You can generate an interrupt to the PowerPC from the host
> > CPU bei writing to the PCI command register. You have to read the user manual
> > carefully. Perhaps it not that obvious.
> 
> Really!? Someone should tell AMCC tech support then.
> When I failed to find a method (other than hooking up
> an external GPIO), I contacted them and they came to
> the same conclusion (on the 440EP anyway).
> 
> I'll look in the latest user manual to be sure ...
> 
> PPC440EP_UM2000_v1_23.pdf
> 
> p394 has their 'cheesy' implementation of PCI INTA# control;
> toggle a single bit.
> 
> Then backing up a little, p388 has the PCI command register ...
> Nope, no comment there that a write causes an interrupt to
> the PowerPC core.
> 
> Ok, so going back to the UIC in Chapter 10, p224.
> 
> Ah-ha, PCI CMD write generates an interrupt 5!
> 
> So, I stand corrected; the host can generate an interrupt to
> the PowerPC core, and the method is 'cheesier' than the PCI
> INTA# control.
> 
> And my experience with AMCC's tech support is now a notch
> lower, as even they did not offer this as a solution :)
> 
> I sure am glad I changed to a Freescale processor ;)
> 
> Cheers,
> Dave

^ permalink raw reply

* Re: FDT for Microblaze and PPC405
From: Michal Simek @ 2007-09-12  7:34 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20070912020339.GC16001@localhost.localdomain>

Hi David and Grant

>> > For example emaclite driver needs information about turning on/off ping pong
buffer...
>> >
>> > I would like to make this properly.
>>
>> FDT design is just as much art as it is science.  It takes taste and
>> judgement to desgin a nice set of bindings.  Your best option is to
>> draft something and post it to the linuxppc-embedded mailing list for
>> review.
>
>Yes, this is the preferred procedure, for now.

OK. Science is good for me because I am looking for theme for dissertation thesis
and FPGA based FDT can be good choice.

>> > And second question is on early console logs and timers setting. I read
>about aliases in FDT. I think that aliases can cover this setting.
>> > For example my design contain 4 serial line and I would like to know which
>serial line is set on early console.
>>
>> You use the chosen node for this.  In the chosen node, you add a
>> property called "linux,stdout-path" which holds the path to your
>> console.  You can look at examples under arch/powerpc/boot/dts/*

>Yes.  Currently we don't use /aliases in the flat device tree.  This
>is possibly a mistake, and something I'm thinking about changing.
>But, for now, linux,stdout-path in /chosen is the way to do this.

OK. I will make the first draft and send it to list for review but it will be
only part of patch rely on with ppc405 because we don't have microblaze kernel code.

Michal Simek

^ permalink raw reply

* [PATCH 01/15] Extract the file descriptor search logic in SPU coredump code
From: Michael Ellerman @ 2007-09-12  7:43 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Jeremy Kerr, linux-kernel

Extract the logic for searching through the file descriptors for spu contexts
into a separate routine, coredump_next_context(), so we can use it elsewhere
in future. In the process we flatten the for loop, and move the NOSCHED test
into coredump_next_context().

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/cell/spufs/coredump.c |   58 +++++++++++++++++---------
 1 files changed, 38 insertions(+), 20 deletions(-)

diff --git a/arch/powerpc/platforms/cell/spufs/coredump.c b/arch/powerpc/platforms/cell/spufs/coredump.c
index 5e31799..99f8e0b 100644
--- a/arch/powerpc/platforms/cell/spufs/coredump.c
+++ b/arch/powerpc/platforms/cell/spufs/coredump.c
@@ -109,16 +109,11 @@ static int spufs_ctx_note_size(struct spufs_ctx_info *ctx_info)
 	return total;
 }
 
-static int spufs_add_one_context(struct file *file, int dfd)
+static int spufs_add_one_context(struct spu_context *ctx, int dfd)
 {
-	struct spu_context *ctx;
 	struct spufs_ctx_info *ctx_info;
 	int size;
 
-	ctx = SPUFS_I(file->f_dentry->d_inode)->i_ctx;
-	if (ctx->flags & SPU_CREATE_NOSCHED)
-		return 0;
-
 	ctx_info = kzalloc(sizeof(*ctx_info), GFP_KERNEL);
 	if (unlikely(!ctx_info))
 		return -ENOMEM;
@@ -142,22 +137,45 @@ static int spufs_add_one_context(struct file *file, int dfd)
  * internal functionality to dump them without needing to actually
  * open the files.
  */
-static int spufs_arch_notes_size(void)
+static struct spu_context *coredump_next_context(int *fd)
 {
 	struct fdtable *fdt = files_fdtable(current->files);
-	int size = 0, fd;
-
-	for (fd = 0; fd < fdt->max_fds; fd++) {
-		if (FD_ISSET(fd, fdt->open_fds)) {
-			struct file *file = fcheck(fd);
-
-			if (file && file->f_op == &spufs_context_fops) {
-				int rval = spufs_add_one_context(file, fd);
-				if (rval < 0)
-					break;
-				size += rval;
-			}
-		}
+	struct file *file;
+	struct spu_context *ctx = NULL;
+
+	for (; *fd < fdt->max_fds; (*fd)++) {
+		if (!FD_ISSET(*fd, fdt->open_fds))
+			continue;
+
+		file = fcheck(*fd);
+
+		if (!file || file->f_op != &spufs_context_fops)
+			continue;
+
+		ctx = SPUFS_I(file->f_dentry->d_inode)->i_ctx;
+		if (ctx->flags & SPU_CREATE_NOSCHED)
+			continue;
+
+		/* start searching the next fd next time we're called */
+		(*fd)++;
+		break;
+	}
+
+	return ctx;
+}
+
+static int spufs_arch_notes_size(void)
+{
+	struct spu_context *ctx;
+	int size = 0, rc, fd;
+
+	fd = 0;
+	while ((ctx = coredump_next_context(&fd)) != NULL) {
+		rc = spufs_add_one_context(ctx, fd);
+		if (rc < 0)
+			break;
+
+		size += rc;
 	}
 
 	return size;
-- 
1.5.1.3.g7a33b

^ permalink raw reply related

* [PATCH 02/15] Remove ctx_info and ctx_info_list
From: Michael Ellerman @ 2007-09-12  7:43 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Jeremy Kerr, linux-kernel
In-Reply-To: <2900ea4dbfd6e98e71ff400cbd25d1283a278972.1189583010.git.michael@ellerman.id.au>

Remove the ctx_info struct entirely, and also the ctx_info_list. This fixes
a race where two processes can clobber each other's ctx_info structs.

Instead of using the list, we just repeat the search through the file
descriptor table.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/cell/spufs/coredump.c |   79 ++++++-------------------
 1 files changed, 19 insertions(+), 60 deletions(-)

diff --git a/arch/powerpc/platforms/cell/spufs/coredump.c b/arch/powerpc/platforms/cell/spufs/coredump.c
index 99f8e0b..6663669 100644
--- a/arch/powerpc/platforms/cell/spufs/coredump.c
+++ b/arch/powerpc/platforms/cell/spufs/coredump.c
@@ -31,15 +31,6 @@
 
 #include "spufs.h"
 
-struct spufs_ctx_info {
-	struct list_head list;
-	int dfd;
-	int memsize; /* in bytes */
-	struct spu_context *ctx;
-};
-
-static LIST_HEAD(ctx_info_list);
-
 static ssize_t do_coredump_read(int num, struct spu_context *ctx, void __user *buffer,
 				size_t size, loff_t *off)
 {
@@ -73,25 +64,17 @@ static int spufs_dump_seek(struct file *file, loff_t off)
 	return 1;
 }
 
-static void spufs_fill_memsize(struct spufs_ctx_info *ctx_info)
+static u64 ctx_ls_size(struct spu_context *ctx)
 {
-	struct spu_context *ctx;
-	unsigned long long lslr;
-
-	ctx = ctx_info->ctx;
-	lslr = ctx->csa.priv2.spu_lslr_RW;
-	ctx_info->memsize = lslr + 1;
+	return ctx->csa.priv2.spu_lslr_RW + 1;
 }
 
-static int spufs_ctx_note_size(struct spufs_ctx_info *ctx_info)
+static int spufs_ctx_note_size(struct spu_context *ctx, int dfd)
 {
-	int dfd, memsize, i, sz, total = 0;
+	int i, sz, total = 0;
 	char *name;
 	char fullname[80];
 
-	dfd = ctx_info->dfd;
-	memsize = ctx_info->memsize;
-
 	for (i = 0; spufs_coredump_read[i].name; i++) {
 		name = spufs_coredump_read[i].name;
 		sz = spufs_coredump_read[i].size;
@@ -101,7 +84,7 @@ static int spufs_ctx_note_size(struct spufs_ctx_info *ctx_info)
 		total += sizeof(struct elf_note);
 		total += roundup(strlen(fullname) + 1, 4);
 		if (!strcmp(name, "mem"))
-			total += roundup(memsize, 4);
+			total += roundup(ctx_ls_size(ctx), 4);
 		else
 			total += roundup(sz, 4);
 	}
@@ -109,25 +92,6 @@ static int spufs_ctx_note_size(struct spufs_ctx_info *ctx_info)
 	return total;
 }
 
-static int spufs_add_one_context(struct spu_context *ctx, int dfd)
-{
-	struct spufs_ctx_info *ctx_info;
-	int size;
-
-	ctx_info = kzalloc(sizeof(*ctx_info), GFP_KERNEL);
-	if (unlikely(!ctx_info))
-		return -ENOMEM;
-
-	ctx_info->dfd = dfd;
-	ctx_info->ctx = ctx;
-
-	spufs_fill_memsize(ctx_info);
-
-	size = spufs_ctx_note_size(ctx_info);
-	list_add(&ctx_info->list, &ctx_info_list);
-	return size;
-}
-
 /*
  * The additional architecture-specific notes for Cell are various
  * context files in the spu context.
@@ -171,7 +135,7 @@ static int spufs_arch_notes_size(void)
 
 	fd = 0;
 	while ((ctx = coredump_next_context(&fd)) != NULL) {
-		rc = spufs_add_one_context(ctx, fd);
+		rc = spufs_ctx_note_size(ctx, fd);
 		if (rc < 0)
 			break;
 
@@ -181,12 +145,11 @@ static int spufs_arch_notes_size(void)
 	return size;
 }
 
-static void spufs_arch_write_note(struct spufs_ctx_info *ctx_info, int i,
-				struct file *file)
+static void spufs_arch_write_note(struct spu_context *ctx, int i,
+				struct file *file, int dfd)
 {
-	struct spu_context *ctx;
 	loff_t pos = 0;
-	int sz, dfd, rc, total = 0;
+	int sz, rc, total = 0;
 	const int bufsz = PAGE_SIZE;
 	char *name;
 	char fullname[80], *buf;
@@ -196,18 +159,13 @@ static void spufs_arch_write_note(struct spufs_ctx_info *ctx_info, int i,
 	if (!buf)
 		return;
 
-	dfd = ctx_info->dfd;
 	name = spufs_coredump_read[i].name;
 
 	if (!strcmp(name, "mem"))
-		sz = ctx_info->memsize;
+		sz = ctx_ls_size(ctx);
 	else
 		sz = spufs_coredump_read[i].size;
 
-	ctx = ctx_info->ctx;
-	if (!ctx)
-		goto out;
-
 	sprintf(fullname, "SPU/%d/%s", dfd, name);
 	en.n_namesz = strlen(fullname) + 1;
 	en.n_descsz = sz;
@@ -237,16 +195,17 @@ out:
 
 static void spufs_arch_write_notes(struct file *file)
 {
-	int j;
-	struct spufs_ctx_info *ctx_info, *next;
+	struct spu_context *ctx;
+	int fd, j;
+
+	fd = 0;
+	while ((ctx = coredump_next_context(&fd)) != NULL) {
+		spu_acquire_saved(ctx);
 
-	list_for_each_entry_safe(ctx_info, next, &ctx_info_list, list) {
-		spu_acquire_saved(ctx_info->ctx);
 		for (j = 0; j < spufs_coredump_num_notes; j++)
-			spufs_arch_write_note(ctx_info, j, file);
-		spu_release_saved(ctx_info->ctx);
-		list_del(&ctx_info->list);
-		kfree(ctx_info);
+			spufs_arch_write_note(ctx, j, file, fd);
+
+		spu_release_saved(ctx);
 	}
 }
 
-- 
1.5.1.3.g7a33b

^ permalink raw reply related

* [PATCH 03/15] Call spu_acquire_saved() before calculating the SPU note sizes
From: Michael Ellerman @ 2007-09-12  7:43 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Jeremy Kerr, linux-kernel
In-Reply-To: <2900ea4dbfd6e98e71ff400cbd25d1283a278972.1189583010.git.michael@ellerman.id.au>

It makes sense to stop the SPU processes as soon as possible. Also if we
dont acquire_saved() I think there's a possibility that the value in
csa.priv2.spu_lslr_RW won't be accurate.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/cell/spufs/coredump.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/cell/spufs/coredump.c b/arch/powerpc/platforms/cell/spufs/coredump.c
index 6663669..21283f6 100644
--- a/arch/powerpc/platforms/cell/spufs/coredump.c
+++ b/arch/powerpc/platforms/cell/spufs/coredump.c
@@ -135,7 +135,9 @@ static int spufs_arch_notes_size(void)
 
 	fd = 0;
 	while ((ctx = coredump_next_context(&fd)) != NULL) {
+		spu_acquire_saved(ctx);
 		rc = spufs_ctx_note_size(ctx, fd);
+		spu_release_saved(ctx);
 		if (rc < 0)
 			break;
 
-- 
1.5.1.3.g7a33b

^ permalink raw reply related

* [PATCH 04/15] Use computed sizes/#defines rather than literals in SPU coredump code
From: Michael Ellerman @ 2007-09-12  7:43 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Jeremy Kerr, linux-kernel
In-Reply-To: <2900ea4dbfd6e98e71ff400cbd25d1283a278972.1189583010.git.michael@ellerman.id.au>

The spufs_coredump_reader array contains the size of the data that will be
returned by the read routine. Currently these are specified as literals, and
though some are obvious, sizeof(u32) == 4, others are not, 69 * 8 ==  ???

Instead, use sizeof() whatever type is returned by each routine, or in
the case of spufs_mem_read() the #define LS_SIZE.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/cell/spufs/file.c |   21 +++++++++++----------
 1 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/platforms/cell/spufs/file.c b/arch/powerpc/platforms/cell/spufs/file.c
index a4a8770..18ddde8 100644
--- a/arch/powerpc/platforms/cell/spufs/file.c
+++ b/arch/powerpc/platforms/cell/spufs/file.c
@@ -2231,23 +2231,24 @@ struct tree_descr spufs_dir_nosched_contents[] = {
 };
 
 struct spufs_coredump_reader spufs_coredump_read[] = {
-	{ "regs", __spufs_regs_read, NULL, 128 * 16 },
-	{ "fpcr", __spufs_fpcr_read, NULL, 16 },
+	{ "regs", __spufs_regs_read, NULL, sizeof(struct spu_reg128[128])},
+	{ "fpcr", __spufs_fpcr_read, NULL, sizeof(struct spu_reg128) },
 	{ "lslr", NULL, __spufs_lslr_get, 11 },
 	{ "decr", NULL, __spufs_decr_get, 11 },
 	{ "decr_status", NULL, __spufs_decr_status_get, 11 },
-	{ "mem", __spufs_mem_read, NULL, 256 * 1024, },
-	{ "signal1", __spufs_signal1_read, NULL, 4 },
+	{ "mem", __spufs_mem_read, NULL, LS_SIZE, },
+	{ "signal1", __spufs_signal1_read, NULL, sizeof(u32) },
 	{ "signal1_type", NULL, __spufs_signal1_type_get, 2 },
-	{ "signal2", __spufs_signal2_read, NULL, 4 },
+	{ "signal2", __spufs_signal2_read, NULL, sizeof(u32) },
 	{ "signal2_type", NULL, __spufs_signal2_type_get, 2 },
 	{ "event_mask", NULL, __spufs_event_mask_get, 8 },
 	{ "event_status", NULL, __spufs_event_status_get, 8 },
-	{ "mbox_info", __spufs_mbox_info_read, NULL, 4 },
-	{ "ibox_info", __spufs_ibox_info_read, NULL, 4 },
-	{ "wbox_info", __spufs_wbox_info_read, NULL, 16 },
-	{ "dma_info", __spufs_dma_info_read, NULL, 69 * 8 },
-	{ "proxydma_info", __spufs_proxydma_info_read, NULL, 35 * 8 },
+	{ "mbox_info", __spufs_mbox_info_read, NULL, sizeof(u32) },
+	{ "ibox_info", __spufs_ibox_info_read, NULL, sizeof(u32) },
+	{ "wbox_info", __spufs_wbox_info_read, NULL, 4 * sizeof(u32)},
+	{ "dma_info", __spufs_dma_info_read, NULL, sizeof(struct spu_dma_info)},
+	{ "proxydma_info", __spufs_proxydma_info_read,
+			   NULL, sizeof(struct spu_proxydma_info)},
 	{ "object-id", NULL, __spufs_object_id_get, 19 },
 	{ },
 };
-- 
1.5.1.3.g7a33b

^ permalink raw reply related

* [PATCH 05/15] Write some SPU coredump values as ASCII
From: Michael Ellerman @ 2007-09-12  7:43 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Jeremy Kerr, linux-kernel
In-Reply-To: <2900ea4dbfd6e98e71ff400cbd25d1283a278972.1189583010.git.michael@ellerman.id.au>

Unfortunately GDB expects some of the SPU coredump values to be identical
in format to what is found in spufs. This means we need to dump some of
the values as ASCII strings, not the actual values.

Because we don't know what the values will be, we always print the values
with the format "0x%.16lx", that way we know the result will be 19 bytes.

do_coredump_read() doesn't take a __user buffer, so remove the annotation,
and because we know that it's safe to just snprintf() directly to it.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/cell/spufs/coredump.c |    8 +++++---
 arch/powerpc/platforms/cell/spufs/file.c     |   14 +++++++-------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/platforms/cell/spufs/coredump.c b/arch/powerpc/platforms/cell/spufs/coredump.c
index 21283f6..c65b717 100644
--- a/arch/powerpc/platforms/cell/spufs/coredump.c
+++ b/arch/powerpc/platforms/cell/spufs/coredump.c
@@ -31,7 +31,7 @@
 
 #include "spufs.h"
 
-static ssize_t do_coredump_read(int num, struct spu_context *ctx, void __user *buffer,
+static ssize_t do_coredump_read(int num, struct spu_context *ctx, void *buffer,
 				size_t size, loff_t *off)
 {
 	u64 data;
@@ -41,8 +41,10 @@ static ssize_t do_coredump_read(int num, struct spu_context *ctx, void __user *b
 		return spufs_coredump_read[num].read(ctx, buffer, size, off);
 
 	data = spufs_coredump_read[num].get(ctx);
-	ret = copy_to_user(buffer, &data, 8);
-	return ret ? -EFAULT : 8;
+	ret = snprintf(buffer, size, "0x%.16lx", data);
+	if (ret >= size)
+		return size;
+	return ++ret; /* count trailing NULL */
 }
 
 /*
diff --git a/arch/powerpc/platforms/cell/spufs/file.c b/arch/powerpc/platforms/cell/spufs/file.c
index 18ddde8..85edbec 100644
--- a/arch/powerpc/platforms/cell/spufs/file.c
+++ b/arch/powerpc/platforms/cell/spufs/file.c
@@ -2233,16 +2233,16 @@ struct tree_descr spufs_dir_nosched_contents[] = {
 struct spufs_coredump_reader spufs_coredump_read[] = {
 	{ "regs", __spufs_regs_read, NULL, sizeof(struct spu_reg128[128])},
 	{ "fpcr", __spufs_fpcr_read, NULL, sizeof(struct spu_reg128) },
-	{ "lslr", NULL, __spufs_lslr_get, 11 },
-	{ "decr", NULL, __spufs_decr_get, 11 },
-	{ "decr_status", NULL, __spufs_decr_status_get, 11 },
+	{ "lslr", NULL, __spufs_lslr_get, 19 },
+	{ "decr", NULL, __spufs_decr_get, 19 },
+	{ "decr_status", NULL, __spufs_decr_status_get, 19 },
 	{ "mem", __spufs_mem_read, NULL, LS_SIZE, },
 	{ "signal1", __spufs_signal1_read, NULL, sizeof(u32) },
-	{ "signal1_type", NULL, __spufs_signal1_type_get, 2 },
+	{ "signal1_type", NULL, __spufs_signal1_type_get, 19 },
 	{ "signal2", __spufs_signal2_read, NULL, sizeof(u32) },
-	{ "signal2_type", NULL, __spufs_signal2_type_get, 2 },
-	{ "event_mask", NULL, __spufs_event_mask_get, 8 },
-	{ "event_status", NULL, __spufs_event_status_get, 8 },
+	{ "signal2_type", NULL, __spufs_signal2_type_get, 19 },
+	{ "event_mask", NULL, __spufs_event_mask_get, 19 },
+	{ "event_status", NULL, __spufs_event_status_get, 19 },
 	{ "mbox_info", __spufs_mbox_info_read, NULL, sizeof(u32) },
 	{ "ibox_info", __spufs_ibox_info_read, NULL, sizeof(u32) },
 	{ "wbox_info", __spufs_wbox_info_read, NULL, 4 * sizeof(u32)},
-- 
1.5.1.3.g7a33b

^ permalink raw reply related

* [PATCH 06/15] Correctly calculate the size of the local-store to dump
From: Michael Ellerman @ 2007-09-12  7:43 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Jeremy Kerr, linux-kernel
In-Reply-To: <2900ea4dbfd6e98e71ff400cbd25d1283a278972.1189583010.git.michael@ellerman.id.au>

The routine to dump the local store, __spufs_mem_read(), does not take the
spu_lslr_RW value into account - so we shouldn't check it when we're
calculating the size either.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/cell/spufs/coredump.c |   16 ++--------------
 1 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/platforms/cell/spufs/coredump.c b/arch/powerpc/platforms/cell/spufs/coredump.c
index c65b717..52d6219 100644
--- a/arch/powerpc/platforms/cell/spufs/coredump.c
+++ b/arch/powerpc/platforms/cell/spufs/coredump.c
@@ -66,11 +66,6 @@ static int spufs_dump_seek(struct file *file, loff_t off)
 	return 1;
 }
 
-static u64 ctx_ls_size(struct spu_context *ctx)
-{
-	return ctx->csa.priv2.spu_lslr_RW + 1;
-}
-
 static int spufs_ctx_note_size(struct spu_context *ctx, int dfd)
 {
 	int i, sz, total = 0;
@@ -85,10 +80,7 @@ static int spufs_ctx_note_size(struct spu_context *ctx, int dfd)
 
 		total += sizeof(struct elf_note);
 		total += roundup(strlen(fullname) + 1, 4);
-		if (!strcmp(name, "mem"))
-			total += roundup(ctx_ls_size(ctx), 4);
-		else
-			total += roundup(sz, 4);
+		total += roundup(sz, 4);
 	}
 
 	return total;
@@ -164,11 +156,7 @@ static void spufs_arch_write_note(struct spu_context *ctx, int i,
 		return;
 
 	name = spufs_coredump_read[i].name;
-
-	if (!strcmp(name, "mem"))
-		sz = ctx_ls_size(ctx);
-	else
-		sz = spufs_coredump_read[i].size;
+	sz = spufs_coredump_read[i].size;
 
 	sprintf(fullname, "SPU/%d/%s", dfd, name);
 	en.n_namesz = strlen(fullname) + 1;
-- 
1.5.1.3.g7a33b

^ permalink raw reply related


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