LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [RFC:PATCH 02/03] powerpc: Add definitions for Debug Registers on BookE Platforms
From: Dave Kleikamp @ 2009-12-11  1:31 UTC (permalink / raw)
  To: David Gibson
  Cc: linuxppc-dev list, Sergio Durigan Junior, Torez Smith,
	Thiago Jung Bauermann
In-Reply-To: <20091211005344.GB8852@yookeroo>

On Fri, 2009-12-11 at 11:53 +1100, David Gibson wrote:
> On Thu, Dec 10, 2009 at 01:57:21PM -0200, Dave Kleikamp wrote:
> > powerpc: Add definitions for Debug Registers on BookE Platforms
> > 
> > From: Torez Smith <lnxtorez@linux.vnet.ibm.com>
> > 
> > This patch adds additional definitions for BookE Debug Registers
> > to the reg_booke.h header file.
> > 
> > Signed-off-by: Torez Smith  <lnxtorez@linux.vnet.ibm.com>
> > Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> 
> As with patch 1/3, none of the comments below is anything that
> couldn't be fixed up after merging.  So,
> 
> Acked-by: David Gibson <dwg@au1.ibm.com>
> 
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>
> > Cc: Sergio Durigan Junior <sergiodj@br.ibm.com>
> > Cc: David Gibson <dwg@au1.ibm.com>
> > Cc: linuxppc-dev list <Linuxppc-dev@ozlabs.org>
> > ---
> > 
> >  arch/powerpc/include/asm/processor.h |   30 +++++-
> >  arch/powerpc/include/asm/reg_booke.h |  176 +++++++++++++++++++++++++++++-----
> >  2 files changed, 178 insertions(+), 28 deletions(-)
> 
> [snip]
> > +	/*
> > +	 * The following will contain addresses used by debug applications
> > +	 * to help trace and trap on particular address locations.
> > +	 * The bits in the Debug Control Registers above help define which
> > +	 * of the following registers will contain valid data and/or addresses.
> > +	 */
> > +	unsigned long	iac1;
> > +	unsigned long	iac2;
> > +	unsigned long	iac3;
> > +	unsigned long	iac4;
> > +	unsigned long	dac1;
> > +	unsigned long	dac2;
> > +	unsigned long	dvc1;
> > +	unsigned long	dvc2;
> 
> I think you'd make the logic in patch 3 substantially easier, if you
> defined these as
> 	unsigned long iac[4];
> 	unsigned long dac[2];
> 	unsigned long dvc[2];
> instead of as individual structure members.

I'll give that a look.  You're probably right.

> [snip]
> > +#define DBCR0_USER_DEBUG	(DBCR0_IDM | DBCR0_ICMP | DBCR0_IAC1 | \
> > +				 DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4)
> > +#define DBCR0_BASE_REG_VALUE	0
> 
> These constants are left over from when the interface allowed
> more-or-less direct access to the debug regs.  I don't think the
> USER_DEBUG constant is used at all any more, and the BASE_REG_VALUE is
> just used in the load_default function, and might as well be inline
> there.

Right.

> [snip]
> > +
> > +#define dbcr_iac_range(task)	((task)->thread.dbcr0)
> 
> Hrm, I think the way these macros work to do the 40x vs. BookE
> abstration is kind of ugly.  But an unequivocally better way doesn't
> immediately occur to me.

Without this, the ifdef's were horrendous.  I'm open to renaming this or
redefining it to be more intuitive if anyone has a better idea.
-- 
David Kleikamp
IBM Linux Technology Center

^ permalink raw reply

* Re: [RFC:PATCH 02/03] powerpc: Add definitions for Debug Registers on BookE Platforms
From: David Gibson @ 2009-12-11  0:53 UTC (permalink / raw)
  To: Dave Kleikamp
  Cc: linuxppc-dev list, Sergio Durigan Junior, Torez Smith,
	Thiago Jung Bauermann
In-Reply-To: <20091210155721.6697.40863.sendpatchset@norville.austin.ibm.com>

On Thu, Dec 10, 2009 at 01:57:21PM -0200, Dave Kleikamp wrote:
> powerpc: Add definitions for Debug Registers on BookE Platforms
> 
> From: Torez Smith <lnxtorez@linux.vnet.ibm.com>
> 
> This patch adds additional definitions for BookE Debug Registers
> to the reg_booke.h header file.
> 
> Signed-off-by: Torez Smith  <lnxtorez@linux.vnet.ibm.com>
> Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>

As with patch 1/3, none of the comments below is anything that
couldn't be fixed up after merging.  So,

Acked-by: David Gibson <dwg@au1.ibm.com>

> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>
> Cc: Sergio Durigan Junior <sergiodj@br.ibm.com>
> Cc: David Gibson <dwg@au1.ibm.com>
> Cc: linuxppc-dev list <Linuxppc-dev@ozlabs.org>
> ---
> 
>  arch/powerpc/include/asm/processor.h |   30 +++++-
>  arch/powerpc/include/asm/reg_booke.h |  176 +++++++++++++++++++++++++++++-----
>  2 files changed, 178 insertions(+), 28 deletions(-)

[snip]
> +	/*
> +	 * The following will contain addresses used by debug applications
> +	 * to help trace and trap on particular address locations.
> +	 * The bits in the Debug Control Registers above help define which
> +	 * of the following registers will contain valid data and/or addresses.
> +	 */
> +	unsigned long	iac1;
> +	unsigned long	iac2;
> +	unsigned long	iac3;
> +	unsigned long	iac4;
> +	unsigned long	dac1;
> +	unsigned long	dac2;
> +	unsigned long	dvc1;
> +	unsigned long	dvc2;

I think you'd make the logic in patch 3 substantially easier, if you
defined these as
	unsigned long iac[4];
	unsigned long dac[2];
	unsigned long dvc[2];
instead of as individual structure members.

[snip]
> +#define DBCR0_USER_DEBUG	(DBCR0_IDM | DBCR0_ICMP | DBCR0_IAC1 | \
> +				 DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4)
> +#define DBCR0_BASE_REG_VALUE	0

These constants are left over from when the interface allowed
more-or-less direct access to the debug regs.  I don't think the
USER_DEBUG constant is used at all any more, and the BASE_REG_VALUE is
just used in the load_default function, and might as well be inline
there.

[snip]
> +
> +#define dbcr_iac_range(task)	((task)->thread.dbcr0)

Hrm, I think the way these macros work to do the 40x vs. BookE
abstration is kind of ugly.  But an unequivocally better way doesn't
immediately occur to me.

-- 
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: [RFC:PATCH 01/03] powerpc: Extended ptrace interface
From: David Gibson @ 2009-12-11  0:44 UTC (permalink / raw)
  To: Dave Kleikamp
  Cc: linuxppc-dev list, Sergio Durigan Junior, Torez Smith,
	Thiago Jung Bauermann
In-Reply-To: <20091210155715.6697.92627.sendpatchset@norville.austin.ibm.com>

On Thu, Dec 10, 2009 at 01:57:15PM -0200, Dave Kleikamp wrote:
> powerpc: Extended ptrace interface
> 
> From: Torez Smith <lnxtorez@linux.vnet.ibm.com>
> 
> Add a new extended ptrace interface so that user-space has a single
> interface for powerpc, without having to know the specific layout
> of the debug registers.


> Implement:
> PPC_PTRACE_GETHWDEBUGINFO
> PPC_PTRACE_SETHWDEBUG
> PPC_PTRACE_DELHWDEBUG
> 
> Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> Signed-off-by: Torez Smith  <lnxtorez@linux.vnet.ibm.com>

Apart from the data breakpoint alignment for 32-bit systems, all the
comments below are trivial nits, so:

Acked-by: David Gibson <dwg@au1.ibm.com>

[snip]
> +/*
> + * Trigger Type
> + */
> +#define PPC_BREAKPOINT_TRIGGER_EXECUTE	0x1
> +#define PPC_BREAKPOINT_TRIGGER_READ	0x2
> +#define PPC_BREAKPOINT_TRIGGER_WRITE	0x4
> +#define PPC_BREAKPOINT_TRIGGER_RW	0x6

For a little extra safety, I'd tend towards defining the RW constant
in terms of the READ and WRITE constants.

> +
> +/*
> + * Address Mode
> + */
> +#define PPC_BREAKPOINT_MODE_EXACT		0x0
> +#define PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE	0x1
> +#define PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE	0x2
> +#define PPC_BREAKPOINT_MODE_MASK		0x3
> +
> +/*
> + * Condition Mode
> + */
> +#define PPC_BREAKPOINT_CONDITION_NONE	0x0
> +#define PPC_BREAKPOINT_CONDITION_AND	0x1
> +#define PPC_BREAKPOINT_CONDITION_EXACT	0x1

And likewuse define EXACT in terms of AND.

> +#define PPC_BREAKPOINT_CONDITION_OR	0x2
> +#define PPC_BREAKPOINT_CONDITION_AND_OR	0x3
> +#define PPC_BREAKPOINT_CONDITION_BE_ALL	0x00ff0000
> +#define PPC_BREAKPOINT_CONDITION_BE_SHIFT	16
> +#define PPC_BREAKPOINT_CONDITION_BE(n)	\
> +	(1<<((n)+PPC_BREAKPOINT_CONDITION_BE_SHIFT))

[snip]
> +	case PPC_PTRACE_GETHWDBGINFO: {
> +		struct ppc_debug_info dbginfo;
> +
> +		dbginfo.version = 1;
> +		dbginfo.num_instruction_bps = 0;
> +		dbginfo.num_data_bps = 1;
> +		dbginfo.num_condition_regs = 0;
> +#ifdef CONFIG_PPC64
> +		dbginfo.data_bp_alignment = 8;
> +#else
> +		dbginfo.data_bp_alignment = 0;

Uh.. this looks wrong.  Surely it should be 4.

-- 
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: dts file for MPC8343EA
From: Junita Ajith @ 2009-12-11  0:34 UTC (permalink / raw)
  To: Scott Wood; +Cc: Linuxppc-dev
In-Reply-To: <20091207180719.GB8914@loki.buserror.net>

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

Hi Scott:

I am still stuck at Linux kernel booting in MPC8343EA based board.

I have disabled "Ethernet, PCI, USB, dma engines " in the *.dts file and
also in the kernel config.
I am using MPC8349emitxgp.dts  ; enabled MPC8349ITX support in kernel config
also. In fact, I tried building MPC8349emITX; MPC8349MDS also. Same
behaviour!

With this, the kernel boots up (explicitly passing the DTB file and cuImage
NEVER worked) and hangs after

I am using Linux-V-2.6.27.18 and using powerpc-e300c3-gnu  toolchain  -
gcc-4.1.69-eglibc-2.5.69-1 to build u-boot-1.3.2 & linux.

With this, the kernel boots up (explicitly passing the DTB file and cuImage
NEVER worked) and hangs after printing
"Calibrating delay loop... 133.12 BogoMIPS (lpj=266240)"

With a few debug printks looks like the kernel hangs in
"cpu_idle" in --main/init.c

Any clues..?? HELP!!!
Please find the dts file and screen-dump below:

Thanks,
Junita
DTS file:
=======

/dts-v1/;

/ {
        model = "MPC8349EMITXGP";
        compatible = "MPC8349EMITXGP", "MPC834xMITX", "MPC83xxMITX";
        #address-cells = <1>;
        #size-cells = <1>;

        aliases {
                serial0 = &serial0;
        };

        cpus {
                #address-cells = <1>;
                #size-cells = <0>;

                PowerPC,8349@0 {
                        device_type = "cpu";
                        reg = <0x0>;
                        d-cache-line-size = <32>;
                        i-cache-line-size = <32>;
                        d-cache-size = <32768>;
                        i-cache-size = <32768>;
                        timebase-frequency = <0>;       // from bootloader
                        bus-frequency = <0>;            // from bootloader
                        clock-frequency = <0>;          // from bootloader
                };
        };
memory {
                device_type = "memory";
                reg = <0x00000000 0x10000000>;
        };


        soc8349@e0000000 {
                #address-cells = <1>;
                #size-cells = <1>;
                device_type = "soc";
                compatible = "simple-bus";
                ranges = <0x0 0xe0000000 0x00100000>;
                reg = <0xe0000000 0x00000200>;
                bus-frequency = <0x0fe502a8>;                    // from
bootloader

                wdt@200 {
                        device_type = "watchdog";
                        compatible = "mpc83xx_wdt";
                        reg = <0x200 0x100>;
                };

                i2c@3000 {
                        #address-cells = <1>;
                        #size-cells = <0>;
                        cell-index = <0>;
                        compatible = "fsl-i2c";
                        reg = <0x3000 0x100>;
                        interrupts = <14 0x8>;
                        interrupt-parent = <&ipic>;
                        dfsrr;
                };

                mdio@24520 {
                        #address-cells = <1>;
                        #size-cells = <0>;
                        compatible = "fsl,gianfar-mdio";
                        reg = <0x24520 0x20>;

                };
serial0: serial@4500 {
                        cell-index = <0>;
                        device_type = "serial";
                        compatible = "ns16550";
                        reg = <0x4500 0x100>;
                        clock-frequency = <0>;          // from bootloader
                        interrupts = <9 0x8>;
                        interrupt-parent = <&ipic>;
                };

                ipic: pic@700 {
                        interrupt-controller;
                        #address-cells = <0>;
                        #interrupt-cells = <2>;
                        reg = <0x700 0x100>;
                        device_type = "ipic";
                };
        };

};

Screen-shot:
==========
Trying TSEC0
Speed: 100, full duplex
Using TSEC0 device
TFTP from server 192.168.201.14; our IP address is 192.168.201.199
Filename '8349.dtb'.
Load address: 0x900000
Loading: #
done
Bytes transferred = 12288 (3000 hex)
SC3000> bootm 0x00600000 - 0x00900000
## Booting image at 00600000 ...
   Image Name:   Linux-2.6.27.18
   Created:      2009-12-11   0:16:24 UTC
   Image Type:   PowerPC Linux Kernel Image (gzip compressed)
   Data Size:    850785 Bytes = 830.8 kB
   Load Address: 00000000
   Entry Point:  00000000
   Verifying Checksum ... OK
   Uncompressing Kernel Image ... OK
## Current stack ends at 0x0FF9BD48 => set upper limit to 0x00800000
## cmdline at 0x007FFF00 ... 0x007FFF2E
bd address  = 0x0FF9BFC4
memstart    = 0x00000000
memsize     = 0x10000000
flashstart  = 0xFE000000
flashsize   = 0x00080000
flashoffset = 0x00034000
sramstart   = 0x00000000
sramsize    = 0x00000000
bootflags   = 0x00000001
intfreq     = 399.999 MHz
busfreq     = 266.666 MHz
ethaddr     = 00:E0:0C:00:8C:01
IP addr     = 192.168.201.199
baudrate    = 115200 bps
Skipping initrd
   Booting using the fdt at 0x900000
No initrd
## device tree at 0x00900000 ... 0x00902FFF (len=12288=0x3000)
   Loading Device Tree to 007fc000, end 007fefff ... OK
Updating property 'timebase-frequency' =  03 f9 40 aa
Updating property 'bus-frequency' =  0f e5 02 a8
Updating property 'clock-frequency' =  17 d7 83 fc
Updating property 'bus-frequency' =  0f e5 02 a8
Updating property 'clock-frequency' =  0f e5 02 a8
## Transferring control to Linux (at address 00000000) ...
Using MPC834x ITX machine description
Linux version 2.6.27.18 (root@localhost.localdomain) (gcc version 4.1.2) #26
Th9 -> find_legacy_serial_port()
stdout is /soc8349@e0000000/serial@4500
Found legacy serial port 0 for /soc8349@e0000000/serial@4500
  mem=e0004500, taddr=e0004500, irq=0, clk=266666664, speed=0
legacy_serial_console = 0
default console speed = 115740
 <- find_legacy_serial_port()
console [udbg0] enabled
setup_arch: bootmem
mpc834x_itx_setup_arch()
arch: exit
Top of RAM: 0x8000000, Total RAM: 0x8000000
Memory hole size: 0MB
Zone PFN ranges:
  DMA      0x00000000 -> 0x00008000
  Normal   0x00008000 -> 0x00008000
Movable zone start PFN for each node
early_node_map[1] active PFN ranges
    0: 0x00000000 -> 0x00008000
On node 0 totalpages: 32768
free_area_init_node: node 0, pgdat c01b6224, node_mem_map c01d0000
  DMA zone: 32512 pages, LIFO batch:7
Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 32512
Kernel command line: root=/dev/ram rw console=ttyS0,115200 mem=128M
IPIC (128 IRQ sources) at fdffd700
PID hash table entries: 512 (order: 9, 2048 bytes)
time_init: decrementer frequency = 66.666666 MHz
time_init: processor frequency   = 399.999996 MHz
clocksource: timebase mult[3c00001] shift[22] registered
clockevent: decrementer mult[1111] shift[16] cpu[0]
 -> check_legacy_serial_console()
 console was specified !
Dentry cache hash table entries: 16384 (order: 4, 65536 bytes)
Inode-cache hash table entries: 8192 (order: 3, 32768 bytes)
SC3000 - After vfs_caches_init_early -1
SC3000 - After vfs_caches_init_early-2
Memory: 127912k/131072k available (1688k kernel code, 3000k reserved, 68k
data,)SC3000 - After vfs_caches_init_early-3
SLUB: Genslabs=12, HWalign=32, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
Calibrating delay loop... 133.12 BogoMIPS (lpj=266240)



On 12/7/09, Scott Wood <scottwood@freescale.com> wrote:
>
> On Sun, Dec 06, 2009 at 10:41:25PM -0800, ajijuni@gmail.com wrote:
> > Hi
> >
> > We have an MPC8343EA based custom board.
> >
> > I am not able to get Linux up and running in this. No serial output to
> debug further.
> >  U-boot shows correct 'bdinfo' & 'clocks' output.
> > inux hangs at machine_probe.
>
> Check that the platform file you're intending to use matches the compatible
> field in the root node of the device tree -- and that said platform file is
> actually being built.
>
>
> -Scott
>

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

^ permalink raw reply

* Re: Floating point in the kernel
From: Sean MacLennan @ 2009-12-11  0:17 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev
In-Reply-To: <200912102235.47985.arnd@arndb.de>

Found it. We are calling sock_sendmsg, which is definitely a call that
can block! The receive side is done in a thread (which does no floating
point ;), but the send was called directly from the "evil FP thread".

It looks like under light load, you tend to get away with it, so our
trivial testing did not catch it. And most of our warp users do RTP via
asterisk, so this RTP path was not really tested.

I really appreciate the input, the comments convinced me I was going
in the wrong direction and forced me to look harder. I am going to back
out the two patches I sent and fix this properly instead.

Cheers,
   Sean

^ permalink raw reply

* Re: [PATCH 09/11] of: merge of_attach_node() & of_detach_node()
From: Grant Likely @ 2009-12-10 22:21 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, sparclinux,
	linuxppc-dev, davem
In-Reply-To: <1259208468.16367.234.camel@pasglop>

On Wed, Nov 25, 2009 at 9:07 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Tue, 2009-11-24 at 01:19 -0700, Grant Likely wrote:
>> Merge common code between PowerPC and Microblaze
>
> Some of those guys might wnat to be in of_dynamic (see previous email)
>
> Remember: We want to keep the footprint low for embedded archs that
> don't want to do dynamic stuff. Really low.

I agree.  If it is okay by you I'll update this in a subsequent patch.
 Once the code is merge, then I'm going to do a bunch of refactoring
patches.

g.

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

^ permalink raw reply

* Re: Deprecating of_platform, the path from here...
From: Grant Likely @ 2009-12-10 22:03 UTC (permalink / raw)
  To: David Miller; +Cc: devicetree-discuss, linuxppc-dev, paulus, jk
In-Reply-To: <20091210.135609.124000757.davem@davemloft.net>

On Thu, Dec 10, 2009 at 2:56 PM, David Miller <davem@davemloft.net> wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> Date: Thu, 10 Dec 2009 13:47:33 -0700
>
>> Trying to go the other way around (deprecate platform and encouraging
>> of_platform instead) I don't think will gain much traction; whereas I
>> think bringing of_platform features into platform will be an easier
>> sell. =A0I'm trying to be pragmatic here.
>
> When people use words like "traction" and "pragmatic" that means they
> are making decisions for reasons other than technical ones. :-)

100% true.  :-)

g.

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

^ permalink raw reply

* Re: Deprecating of_platform, the path from here...
From: David Miller @ 2009-12-10 21:56 UTC (permalink / raw)
  To: grant.likely; +Cc: devicetree-discuss, linuxppc-dev, paulus, jk
In-Reply-To: <fa686aa40912101247k55d56783ge0163dd1da073b49@mail.gmail.com>

From: Grant Likely <grant.likely@secretlab.ca>
Date: Thu, 10 Dec 2009 13:47:33 -0700

> Trying to go the other way around (deprecate platform and encouraging
> of_platform instead) I don't think will gain much traction; whereas I
> think bringing of_platform features into platform will be an easier
> sell.  I'm trying to be pragmatic here.

When people use words like "traction" and "pragmatic" that means they
are making decisions for reasons other than technical ones. :-)

^ permalink raw reply

* Re: Deprecating of_platform, the path from here...
From: Grant Likely @ 2009-12-10 21:53 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: devicetree-discuss, linuxppc-dev, paulus, jk, David Miller
In-Reply-To: <1260409535.16132.109.camel@pasglop>

Hey guys, some more thoughts below...

On Wed, Dec 9, 2009 at 6:45 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> First the probing because that's the real important issue, I believe the
> other one is mostly academic and can be dealt on a per driver basis
> (I'll discuss it later too).
>
> I'm not totally sold on the idea of the table that Grant proposed. I
> think I proposed it initially in a discussion we had on IRC as one
> possible option and in fact it was suggested by Paulus so don't
> completely blame Grant for it though. It does mean that either platform
> code or core code will have some kind of big table that associates a
> whole pile of of_device_id with a platform device name. It somewhat
> sucks when I think more about it and it does feel like a step backward
> from of_platform_device.

For the record, I don't like it either.  But it is the pragmatic thing
to do because it is simple to implement, and can easily be replaced
later with a better scheme with little impact on drivers.  Basically,
it's a migration plan.

> If we could make platform_driver grow an of_device_id match list it
> would be indeed nicer. Of course you'll reply to that "let's just use
> of_platform" instead :-)
>
> I would then argue that I don't like having two different bus types,
> platform and of_platform for "generic platform" devices that aren't
> typed on a bus type. There's a lot of existing "platform device" and it
> would be quite hard to convert them all. Especially since a lot of them
> are used today on archs that don't have device trees and may never grow
> one.
>
> IE. I believe it's going to be an easier path to grow platform_device
> into something that has the of_device probing functionality rather than
> turn everything into of_platform. (I'm not talking here about the pdata
> and retrieval of informations from the tree, this is the second part of
> the discussion, discussed below).
>
> The main problem with moving existing platform_device to of_platform is
> how do we deal with archs that don't have the device-tree infrastructure
> and use those drivers today ?
>
> We could I suppose create a helper that looks a lot like the current
> platform device creation one, which would create an of_platform device
> instead, along with a struct device_node attached (which isn't part of a
> tree) to it, and create a single "compatible" property whose content is
> the platform data name.
>
> But that means that for every driver we want to be able to use a
> device-tree probing for, we would have to convert all archs & platforms
> that may instanciate it to use the new helpers, in addition to replacing
> whatever pdata they have statically stored into a device node (see the
> second part of the discussion).
>
> It's possible I suppose. I just feel that it's going to be a tougher
> sell to the rest of the world.
>
> There's one nit to be careful of. Some drivers (sadly) have the fact
> that they appear under /sys/bus/platform/ as a userspace ABI thingy. Sad
> but a fact. This is one of the reasons why rather than actually
> converting to of_platform I'd rather find a way to add the of_device_id
> match mechanism to the existing platform_device and deprecate the pdata
> over time.
>
> It will also provide with an easier transition. Basically transform
> platform_device into of_platform_device by first adding the missing bits
> and -then- trimming the crufty remains.

I 100% agree with you here on the matching problem.

> Now let's move to the second part of the discussion which is the
> retrieval of various configuration informations by the driver.
>
> Here too, our model is better, I think there's little argument there. A
> named list of properties is just so much more flexible than a statically
> data structure that has to be in sync between the driver and all
> platforms using it leaves little room for improvement or adding platform
> specific attributes which some drivers might need, etc...
>
> Grant proposal is to have drivers create the pdata from the device-tree.
> This is something I believe we both disagree with, though you more
> vehemently than me I suppose :-)

See my pdata comment lower down.

> There are various things at play here:
>
> First, let me make it clear that imho, the device type
> (of_platform_device vs. platform_device) is irrelevant to that aspect of
> the problem since nowadays we have the ability to carry a device node
> pointer in any descendant of struct device (and we use that heavily for
> devices using specific bus types already).

And even if we kept of_platform; the same problem of parsing device
tree data exists for all other bus types.  Places where of_platform
isn't used today (i2c, spi, pci, mdio, etc).  It makes sense to
establish a pattern for matching and probing with device tree data
that is usable by all bus types.

> If you take an existing platform driver that you want to use on a
> device-tree enabled platform (other than just creating it and pdata from
> arch code which we all agree sucks), the two choices have different
> consequences:
>
> In one case, converting to of_platform_device, you pretty much _have_ to
> get rid of pdata, and convert the driver into using of_get_property()
> instead. This is probably not a bad thing in the long run, except that
> this means you also -have- to convert all platforms in all archs that
> use that specific platform driver to also generate a device node
> (possibly statically in many cases).
>
> This can be a lot of code churn deep into platform code for things like
> ARM which can be pretty nasty, for which none of us have any way to test
> on the relevant hardware. Other arch people will (maybe rightfully so)
> protest especially if they have no intent to use the device-tree stuff
> in their architecture or not yet anyways. And that for each platform
> driver involved.
>
> On the other case, converting to platform_device, adding the device
> node, we have the ability to do an easier transition and easier to sell.
> yes, we do take the risk of getting in that limbo land where drivers end
> up forever in the "transition" state though. That's a con of this
> approach, I do admit.
>
> I don't agree with grant idea however that just converting the content
> of the device node into properties is the way to go.

s/properties/pdata

I'm being pragmatic here.  pdata sucks, but the lowest impact path to
getting device tree data into a lot of existing drivers is to populate
the data source it is already using.  However, at least by putting the
translator into the driver itself, the pdata isn't an anonymous
pointer anymore and the compiler can correctly type check it.

But this is a minor point.  It really is contained within the driver
with no external impact.  pdata can be eliminated one driver at a
time.

> I do prefer your proposed approach (from our IRC discussion) which is
> instead to allocate a struct device-node, convert pdata into properties,
> and modify the drier to use these properties.

I like this approach.  It would actually be quite easy to convert on a
driver-by-driver basis from .pdata to device nodes.  Even platforms
that statically declare their platform devices could be changed by
statically declaring the sub tree for the device.  For (a completely
bogus) example:

  static struct platform_device omap3beagle_nand_device = {
  	.name		= "omap2-nand",
  	.id		= -1,
- 	.dev		= {
- 		.platform_data	= &omap3beagle_nand_data,
- 	},
- 	.num_resources	= 1,
- 	.resource	= &omap3beagle_nand_resource,
+ 	.device_node = (struct device_node *) {
+ 		.name = "nand",
+ 		.properties = (struct property *) {
+ 			.name = "blah",
+ 			.length = sizeof("foo"),
+ 			.value = "foo"
+ 			.next = (struct property *)
+ 		{
+ 			.name = "bar",
+ 			.length = sizeof("wazzit"),
+ 			.value = "wazzit"
+ 		} }
+ 	},
  };

Naturally this example is brutally ugly, and it could be done in a far
cleaner manner; but you get the idea.  As you and I just discussed on
IRC, the device_node structure isn't really designed for static
declarations, but it could work.  Basically it would allow the
migration to a better model that ugly, non-type-checked anonymous
pdata pointers.

> The main difference thus between the two type of conversions (convert to
> of_platform vs convert to platform) is that in the first case, you have
> to convert the driver to use properties -and- convert all platforms in
> all archs including gory ARM cell phone stuff you really don't want to
> go anywhere near. In the second case, you still convert the driver to
> use properties natively, but you keep a "wart" to turn pdata into a
> device-node -inside the driver-, protected by a CONFIG option maybe, so
> that those archs can be left alone until it becomes so obvious to
> everybody what approach is better that they'll end up being converted
> too and the wart can go.
>
> I believe the second approach, while less "clean" in the absolute is a
> more realistic path to take.
>
> Now, orthogonally to that, I do believe it's still nice to provide a way
> to statically lay out a device node in platform code, to allow archs
> that don't otherwise have the device-tree to replace pdata with
> something nicer and get rid of the wart quicker.
>
> We could either find a way with macros to layout an actual struct
> device_node statically along with all the properties etc... but that
> sounds a tad hard.

Heh.  I hacked out my example above before reading this far.

> We could have something that convert an entirely ASCII representation
> into a struct device_node, but that would be akin of having dtc in the
> kernel, might be a bit bloated no ? Though it could be made simpler and
> more restrictive.

Ewh.  Ugly.

> Or we could find an in-between .. .A different struct type that is more
> suitable for being laid out statically (a name, a type, and an enum of
> structs for various property "types", ie, strings, words, bytes, ...)
> with a little helper function that conver that into a device node at
> runtime ?

I think this bears some investigation.  A slightly different layout
(ie, using arrays instead of linked list for properties) would be
easier to prepare.  Worth some investigation.

> What do you think ?

I liked what I read.

Cheers,
g.

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

^ permalink raw reply

* Re: Floating point in the kernel
From: Arnd Bergmann @ 2009-12-10 21:35 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: linuxppc-dev, Sean MacLennan
In-Reply-To: <20091210153359.3a480488@lappy.seanm.ca>

On Thursday 10 December 2009, Sean MacLennan wrote:
> To be honest, I can't find why we are scheduling :( They only way we
> give up the CPU is with locking... and none of the locks where hit
> during the problem. We also never get near our timeslice... the longest
> run I saw when the problem happened was 670us. 
> 
> Is there a way to disable scheduling? We currently do not have preempt
> enabled... but may in the future.

If you do preempt_disable(), it should give you a nice oops with a backtrace
at the point where an actual schedule happens.

The sequence should be something like

preempt_disable();
enable_kernel_fp();
/* use FP here */
preempt_enable();
/* may schedule again */

	Arnd <><

^ permalink raw reply

* Re: Deprecating of_platform, the path from here...
From: Benjamin Herrenschmidt @ 2009-12-10 21:30 UTC (permalink / raw)
  To: David Miller; +Cc: linuxppc-dev, devicetree-discuss, paulus, jk
In-Reply-To: <1260409535.16132.109.camel@pasglop>

On Thu, 2009-12-10 at 12:45 +1100, Benjamin Herrenschmidt wrote:

> I don't agree with grant idea however that just converting the content
> of the device node into properties is the way to go.

And here of course I meant " converting the content of the device node
into into pdata" ...

> I do prefer your proposed approach (from our IRC discussion) which is
> instead to allocate a struct device-node, convert pdata into properties,
> and modify the drier to use these properties.
> 
> The main difference thus between the two type of conversions (convert to
> of_platform vs convert to platform) is that in the first case, you have
> to convert the driver to use properties -and- convert all platforms in
> all archs including gory ARM cell phone stuff you really don't want to
> go anywhere near. In the second case, you still convert the driver to
> use properties natively, but you keep a "wart" to turn pdata into a
> device-node -inside the driver-, protected by a CONFIG option maybe, so
> that those archs can be left alone until it becomes so obvious to
> everybody what approach is better that they'll end up being converted
> too and the wart can go.
> 
> I believe the second approach, while less "clean" in the absolute is a
> more realistic path to take.
> 
> Now, orthogonally to that, I do believe it's still nice to provide a way
> to statically lay out a device node in platform code, to allow archs
> that don't otherwise have the device-tree to replace pdata with
> something nicer and get rid of the wart quicker.
> 
> We could either find a way with macros to layout an actual struct
> device_node statically along with all the properties etc... but that
> sounds a tad hard.
> 
> We could have something that convert an entirely ASCII representation
> into a struct device_node, but that would be akin of having dtc in the
> kernel, might be a bit bloated no ? Though it could be made simpler and
> more restrictive.
> 
> Or we could find an in-between .. .A different struct type that is more
> suitable for being laid out statically (a name, a type, and an enum of
> structs for various property "types", ie, strings, words, bytes, ...)
> with a little helper function that conver that into a device node at
> runtime ?
> 
> What do you think ?
> 
> Cheers,
> Ben.
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: Floating point in the kernel
From: Benjamin Herrenschmidt @ 2009-12-10 20:56 UTC (permalink / raw)
  To: Sean MacLennan; +Cc: linuxppc-dev
In-Reply-To: <20091210153359.3a480488@lappy.seanm.ca>

On Thu, 2009-12-10 at 15:33 -0500, Sean MacLennan wrote:
> To be honest, I can't find *why* we are scheduling :( They only way we
> give up the CPU is with locking... and none of the locks where hit
> during the problem. We also never get near our timeslice... the
> longest
> run I saw when the problem happened was 670us. 
> 
> Is there a way to disable scheduling? We currently do not have preempt
> enabled... but may in the future.

Well, that would be interesting to see where you schedule indeed.

Outside of preempt and an explicit lock or memory allocation I don't
see ... Or get/put_user ?

Cheers,
Ben.

^ permalink raw reply

* Re: Deprecating of_platform, the path from here...
From: Grant Likely @ 2009-12-10 20:47 UTC (permalink / raw)
  To: David Miller; +Cc: devicetree-discuss, linuxppc-dev, paulus, jk
In-Reply-To: <20091209.162121.256573183.davem@davemloft.net>

Hi David,

On Wed, Dec 9, 2009 at 5:21 PM, David Miller <davem@davemloft.net> wrote:
> From: David Miller <davem@davemloft.net>
> Date: Wed, 09 Dec 2009 16:15:50 -0800 (PST)
>
>> From: Grant Likely <grant.likely@secretlab.ca>
>> Date: Wed, 9 Dec 2009 15:06:29 -0700
>>
>>> 1) of_platform will be deprecated in preference of the platform bus.
>>
>> What a shame, it's one of the cleanest driver probing models
>> in the tree.
>
> And BTW, have you folks who "decided" this considered at all the fact
> that it is much easier to describe represent platform devices using
> of OF devices rather than the other way around?

Yup.  I also think of_platform is a cleaner implementation than
platform, but the fact remains that there are far more platform
drivers than there are of_platform.  So, as nice as of_platform is, it
still does pretty much exactly the same job as platform.  I'd rather
see of_platform features migrated to platform than creating drivers
with dual registrations to be used on both OF and non-OF platforms.

Trying to go the other way around (deprecate platform and encouraging
of_platform instead) I don't think will gain much traction; whereas I
think bringing of_platform features into platform will be an easier
sell.  I'm trying to be pragmatic here.

> The platform device pdata mechanism requires data structure changes
> and is not dynamically extensible, whereas OF devices are
> fundamentally so.
>
> I don't like the idea to get rid of of_platform devices at all.

There are no plans to actually remove of_platform.  I certainly don't
plan to try and force SPARC to switch from of_platform to platform
bus.  But on PowerPC (and probably Microblaze) the plan is to move
away from of_platform for all the reasons discussed, and I'm not be
bringing of_platform over as I work on ARM support.

> OF devices are really clean, much like netlink messages, where
> arbitrary named attributes can be added or removed without any data
> structure changes at all.

No argument here.

g.

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

^ permalink raw reply

* Re: Floating point in the kernel
From: Sean MacLennan @ 2009-12-10 20:33 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, linuxppc-dev
In-Reply-To: <1260476379.16132.224.camel@pasglop>

On Fri, 11 Dec 2009 07:19:39 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> I'm not sure that will work in all cases, you are playing a bit with
> fire :-) I suppose I could think it through after breakfast but my
> first thought is "don't do that !". Among other things you may not
> have a pt_regs to save the registers to.

Actually, we usually do have pt_regs, or we are stealing some ;)

> > We also hit another problem under high RTP load... and this is the
> > patch that fixes it:
> > 
> > diff --git a/arch/powerpc/kernel/fpu.S b/arch/powerpc/kernel/fpu.S
> > index fc8f5b1..051a02c 100644
> > --- a/arch/powerpc/kernel/fpu.S
> > +++ b/arch/powerpc/kernel/fpu.S
> > @@ -83,6 +83,11 @@ END_FTR_SECTION_IFSET(CPU_FTR_VSX)
> >         stfd    fr0,THREAD_FPSCR(r4)
> >         PPC_LL  r5,PT_REGS(r4)
> >         toreal(r5)
> > +
> > +       /* Under heavy RTP load the hsp thread can have a NULL
> > pt_regs. */
> > +       PPC_LCMPI       0,r5,0
> > +       beq     1f
> > +  
> 
> Right and that means you just lost the content of your FP registers.

This only happens once in a blue moon, even under heavy RTP load. But I
agree, this could be a real problem.

> >         PPC_LL  r4,_MSR-STACK_FRAME_OVERHEAD(r5)
> >         li      r10,MSR_FP|MSR_FE0|MSR_FE1
> >         andc    r4,r4,r10               /* disable FP for previous
> > task */
> > 
> > So, if you are still reading this far, I am just looking for any
> > suggestions. Are there better ways of handling this? Have I
> > missed something? Anybody know why pt_regs might be NULL?  
> 
> Just don't schedule when you enable_kernel_fp() or move your workload
> to userspace :-)

To be honest, I can't find *why* we are scheduling :( They only way we
give up the CPU is with locking... and none of the locks where hit
during the problem. We also never get near our timeslice... the longest
run I saw when the problem happened was 670us. 

Is there a way to disable scheduling? We currently do not have preempt
enabled... but may in the future.

Cheers,
   Sean

^ permalink raw reply

* Re: Floating point in the kernel
From: arnd @ 2009-12-10 20:32 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Sean MacLennan
In-Reply-To: <20091210131311.78cab78c@lappy.seanm.ca>

On Thursday 10 December 2009 18:13:11 Sean MacLennan wrote:
> One of our drivers has code that was originally running on a DSP. The
> code makes heavy use of floating point. We have isolated all the
> floating point to one kthread in the driver. Using enable_kernel_fp()
> this has worked well.
> 
> But under a specific heavy RTP load, we started getting kernel panics.
> To make a long story short, the scheduler disables FP when you are
> context switched out. When you come back and access a FP instruction,
> you trap and call load_up_fpu() and everything is fine..... unless you
> are in the kernel. If you are in the kernel, like our kthread is, you
> get a "kernel FP unavailable exception".

I think the rule here is that you have to disable preemption and must not
call any potentially blocking functions like kmalloc when enable_kernel_fp
is set. The kernel has good control over whether a thread get context switched
or not, so it should be able to prevent these problems.

	Arnd <><

^ permalink raw reply

* Re: Floating point in the kernel
From: Benjamin Herrenschmidt @ 2009-12-10 20:19 UTC (permalink / raw)
  To: Sean MacLennan; +Cc: linuxppc-dev
In-Reply-To: <20091210131311.78cab78c@lappy.seanm.ca>

On Thu, 2009-12-10 at 13:13 -0500, Sean MacLennan wrote:
> One of our drivers has code that was originally running on a DSP. The
> code makes heavy use of floating point. We have isolated all the
> floating point to one kthread in the driver. Using enable_kernel_fp()
> this has worked well.
> 
> But under a specific heavy RTP load, we started getting kernel panics.
> To make a long story short, the scheduler disables FP when you are
> context switched out. When you come back and access a FP instruction,
> you trap and call load_up_fpu() and everything is fine..... unless you
> are in the kernel. If you are in the kernel, like our kthread is, you
> get a "kernel FP unavailable exception".

Right, you must not use floating point in the kernel -and- expect it to
survive schedule. You should use preempt_disable() and ensure you don't
schedule() around a block using the FP.

Note that you may also lose the FP register content if you schedule.

> Basically we got away with it for two years because the thread is at
> high priority (-20) and tries very hard to finish within 1ms. But the
> RTP high load causes us to context switch out and crash. The following
> patch fixes this:
> 
> diff --git a/arch/powerpc/kernel/head_booke.h b/arch/powerpc/kernel/head_booke.h
> index 50504ae..3476de9 100644
> --- a/arch/powerpc/kernel/head_booke.h
> +++ b/arch/powerpc/kernel/head_booke.h
> @@ -383,7 +383,7 @@ label:
>  #define FP_UNAVAILABLE_EXCEPTION                                             \
>         START_EXCEPTION(FloatingPointUnavailable)                             \
>         NORMAL_EXCEPTION_PROLOG;                                              \
> -       beq     1f;                                                           \
> +       /* SAM beq      1f; */                                          \
>         bl      load_up_fpu;            /* if from user, just load it up */   \
>         b       fast_exception_return;                                        \
>  1:     addi    r3,r1,STACK_FRAME_OVERHEAD;                                   \
> 
> With the patch we run fine, at the expense that we lose the ability to
> catch real FP unavailable exceptions in the kernel. It is because of
> this loss that I have not submitted this patch.

I'm not sure that will work in all cases, you are playing a bit with
fire :-) I suppose I could think it through after breakfast but my first
thought is "don't do that !". Among other things you may not have a
pt_regs to save the registers to.

> We also hit another problem under high RTP load... and this is the
> patch that fixes it:
> 
> diff --git a/arch/powerpc/kernel/fpu.S b/arch/powerpc/kernel/fpu.S
> index fc8f5b1..051a02c 100644
> --- a/arch/powerpc/kernel/fpu.S
> +++ b/arch/powerpc/kernel/fpu.S
> @@ -83,6 +83,11 @@ END_FTR_SECTION_IFSET(CPU_FTR_VSX)
>         stfd    fr0,THREAD_FPSCR(r4)
>         PPC_LL  r5,PT_REGS(r4)
>         toreal(r5)
> +
> +       /* Under heavy RTP load the hsp thread can have a NULL pt_regs. */
> +       PPC_LCMPI       0,r5,0
> +       beq     1f
> +

Right and that means you just lost the content of your FP registers.

>         PPC_LL  r4,_MSR-STACK_FRAME_OVERHEAD(r5)
>         li      r10,MSR_FP|MSR_FE0|MSR_FE1
>         andc    r4,r4,r10               /* disable FP for previous task */
> 
> So, if you are still reading this far, I am just looking for any
> suggestions. Are there better ways of handling this? Have I
> missed something? Anybody know why pt_regs might be NULL?

Just don't schedule when you enable_kernel_fp() or move your workload to
userspace :-)

Cheers,
Ben.

> Cheers,
>    Sean
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH v3] cpm2_pic: Allow correct flow_types for port C interrupts
From: Kumar Gala @ 2009-12-10 19:48 UTC (permalink / raw)
  To: Mark Ware; +Cc: Scott Wood, Linuxppc-dev Development, avorontsov
In-Reply-To: <4B20D81A.7010503@elphinstone.net>


On Dec 10, 2009, at 5:14 AM, Mark Ware wrote:

> Port C interrupts can be either falling edge, or either edge.
> Other external interrupts are either falling edge or active low.
> Tested on a custom 8280 based board.
> 
> Signed-off-by: Mark Ware <mware@elphinstone.net>
> ---
> Changed in v3:
> - Cosmetic improvements as suggested by Anton and Scott
> - Added tested note to changelog
> 
> arch/powerpc/sysdev/cpm2_pic.c |   28 +++++++++++++++++++++-------
> 1 files changed, 21 insertions(+), 7 deletions(-)

Applied to next

- k

^ permalink raw reply

* Floating point in the kernel
From: Sean MacLennan @ 2009-12-10 18:13 UTC (permalink / raw)
  To: linuxppc-dev

One of our drivers has code that was originally running on a DSP. The
code makes heavy use of floating point. We have isolated all the
floating point to one kthread in the driver. Using enable_kernel_fp()
this has worked well.

But under a specific heavy RTP load, we started getting kernel panics.
To make a long story short, the scheduler disables FP when you are
context switched out. When you come back and access a FP instruction,
you trap and call load_up_fpu() and everything is fine..... unless you
are in the kernel. If you are in the kernel, like our kthread is, you
get a "kernel FP unavailable exception".

Basically we got away with it for two years because the thread is at
high priority (-20) and tries very hard to finish within 1ms. But the
RTP high load causes us to context switch out and crash. The following
patch fixes this:

diff --git a/arch/powerpc/kernel/head_booke.h b/arch/powerpc/kernel/head_booke.h
index 50504ae..3476de9 100644
--- a/arch/powerpc/kernel/head_booke.h
+++ b/arch/powerpc/kernel/head_booke.h
@@ -383,7 +383,7 @@ label:
 #define FP_UNAVAILABLE_EXCEPTION                                             \
        START_EXCEPTION(FloatingPointUnavailable)                             \
        NORMAL_EXCEPTION_PROLOG;                                              \
-       beq     1f;                                                           \
+       /* SAM beq      1f; */                                          \
        bl      load_up_fpu;            /* if from user, just load it up */   \
        b       fast_exception_return;                                        \
 1:     addi    r3,r1,STACK_FRAME_OVERHEAD;                                   \

With the patch we run fine, at the expense that we lose the ability to
catch real FP unavailable exceptions in the kernel. It is because of
this loss that I have not submitted this patch.

We also hit another problem under high RTP load... and this is the
patch that fixes it:

diff --git a/arch/powerpc/kernel/fpu.S b/arch/powerpc/kernel/fpu.S
index fc8f5b1..051a02c 100644
--- a/arch/powerpc/kernel/fpu.S
+++ b/arch/powerpc/kernel/fpu.S
@@ -83,6 +83,11 @@ END_FTR_SECTION_IFSET(CPU_FTR_VSX)
        stfd    fr0,THREAD_FPSCR(r4)
        PPC_LL  r5,PT_REGS(r4)
        toreal(r5)
+
+       /* Under heavy RTP load the hsp thread can have a NULL pt_regs. */
+       PPC_LCMPI       0,r5,0
+       beq     1f
+
        PPC_LL  r4,_MSR-STACK_FRAME_OVERHEAD(r5)
        li      r10,MSR_FP|MSR_FE0|MSR_FE1
        andc    r4,r4,r10               /* disable FP for previous task */

So, if you are still reading this far, I am just looking for any
suggestions. Are there better ways of handling this? Have I
missed something? Anybody know why pt_regs might be NULL?

Cheers,
   Sean

^ permalink raw reply related

* Re: [PATCH v2 0/3] powerpc/83xx: Sleep and deep sleep support for MPC8315E-RDB
From: Scott Wood @ 2009-12-10 18:07 UTC (permalink / raw)
  To: avorontsov; +Cc: linuxppc-dev
In-Reply-To: <20091210175957.GA30682@oksana.dev.rtsoft.ru>

Anton Vorontsov wrote:
> Hi all,
> 
> This is quite late resend, sorry.
> 
> Only the third patch has changed, i.e. I got rid of sleep-nexus
> stuff per Scott and Benjamin suggestions.

Acked-by: Scott Wood <scottwood@freescale.com>

-Scott

^ permalink raw reply

* Re: [RFC:PATCH 03/03] powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace
From: Dave Kleikamp @ 2009-12-10 18:05 UTC (permalink / raw)
  To: Josh Boyer
  Cc: linuxppc-dev list, Sergio Durigan Junior, Torez Smith,
	Thiago Jung Bauermann, David Gibson
In-Reply-To: <20091210174135.GW2937@zod.rchland.ibm.com>

On Thu, 2009-12-10 at 12:41 -0500, Josh Boyer wrote:
> On Thu, Dec 10, 2009 at 12:27:11PM -0500, Josh Boyer wrote:
> >On Thu, Dec 10, 2009 at 01:57:27PM -0200, Dave Kleikamp wrote:
> >>+static void prime_debug_regs(struct thread_struct *thread)
> >>+{
> >>+	mtspr(SPRN_IAC1, thread->iac1);
> >>+	mtspr(SPRN_IAC2, thread->iac2);
> >>+	mtspr(SPRN_IAC3, thread->iac3);
> >>+	mtspr(SPRN_IAC4, thread->iac4);
> >>+	mtspr(SPRN_DAC1, thread->dac1);
> >>+	mtspr(SPRN_DAC2, thread->dac2);
> >>+	mtspr(SPRN_DVC1, thread->dvc1);
> >>+	mtspr(SPRN_DVC2, thread->dvc2);
> >>+	mtspr(SPRN_DBCR0, thread->dbcr0);
> >>+	mtspr(SPRN_DBCR1, thread->dbcr1);
> >>+	mtspr(SPRN_DBCR2, thread->dbcr2);
> >
> >405 has no DBCR2, so I doubt setting it via mtspr would be good.  Does this
> >compile for 40x and did you test it at all?
> >
> >Also, looking at the current kernel it seems SPRN_DBCR2 is only defined for
> >CONFIG_PPC_BOOK3E_64 which is also inaccurate.
> 
> I mis-read the code on this part.  SPRN_DBCR2 gets defined unconditionally,
> so Book-E should be fine.  Still seems broken on 405 though.

Agreed.  I'll rushed through the 405 stuff a bit.  I'll scrutinize and
test it before I post again.

Thanks,
Shaggy
-- 
David Kleikamp
IBM Linux Technology Center

^ permalink raw reply

* [PATCH 3/3] powerpc/83xx: Add power management support for MPC8315E-RDB boards
From: Anton Vorontsov @ 2009-12-10 18:01 UTC (permalink / raw)
  To: Kumar Gala; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <20091210175957.GA30682@oksana.dev.rtsoft.ru>

- Add nodes for PMC and GTM controllers. GTM4 can be used as a wakeup
  source;

- Add fsl,magic-packet properties to eTSEC nodes, i.e. wake-on-lan
  support. Unlike MPC8313 processors, MPC8315 can resume from deep
  sleep upon magic packet reception.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/boot/dts/mpc8315erdb.dts |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8315erdb.dts b/arch/powerpc/boot/dts/mpc8315erdb.dts
index 32e10f5..8a3a4f3 100644
--- a/arch/powerpc/boot/dts/mpc8315erdb.dts
+++ b/arch/powerpc/boot/dts/mpc8315erdb.dts
@@ -204,6 +204,7 @@
 			interrupt-parent = <&ipic>;
 			tbi-handle = <&tbi0>;
 			phy-handle = < &phy0 >;
+			fsl,magic-packet;
 
 			mdio@520 {
 				#address-cells = <1>;
@@ -246,6 +247,7 @@
 			interrupt-parent = <&ipic>;
 			tbi-handle = <&tbi1>;
 			phy-handle = < &phy1 >;
+			fsl,magic-packet;
 
 			mdio@520 {
 				#address-cells = <1>;
@@ -309,6 +311,22 @@
 			interrupt-parent = <&ipic>;
 		};
 
+		gtm1: timer@500 {
+			compatible = "fsl,mpc8315-gtm", "fsl,gtm";
+			reg = <0x500 0x100>;
+			interrupts = <90 8 78 8 84 8 72 8>;
+			interrupt-parent = <&ipic>;
+			clock-frequency = <133333333>;
+		};
+
+		timer@600 {
+			compatible = "fsl,mpc8315-gtm", "fsl,gtm";
+			reg = <0x600 0x100>;
+			interrupts = <91 8 79 8 85 8 73 8>;
+			interrupt-parent = <&ipic>;
+			clock-frequency = <133333333>;
+		};
+
 		/* IPIC
 		 * interrupts cell = <intr #, sense>
 		 * sense values match linux IORESOURCE_IRQ_* defines:
@@ -337,6 +355,15 @@
 				      0x59 0x8>;
 			interrupt-parent = < &ipic >;
 		};
+
+		pmc: power@b00 {
+			compatible = "fsl,mpc8315-pmc", "fsl,mpc8313-pmc",
+				     "fsl,mpc8349-pmc";
+			reg = <0xb00 0x100 0xa00 0x100>;
+			interrupts = <80 8>;
+			interrupt-parent = <&ipic>;
+			fsl,mpc8313-wakeup-timer = <&gtm1>;
+		};
 	};
 
 	pci0: pci@e0008500 {
-- 
1.6.3.3

^ permalink raw reply related

* [PATCH 2/3] powerpc/83xx/suspend: Save and restore SICRL, SICRH and SCCR
From: Anton Vorontsov @ 2009-12-10 18:00 UTC (permalink / raw)
  To: Kumar Gala; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <20091210175957.GA30682@oksana.dev.rtsoft.ru>

We need to save SICRL, SICRH and SCCR registers on suspend, and restore
them on resume. Otherwise, we lose IO and clocks setup on MPC8315E-RDB
boards when ULPI USB PHY is used (non-POR setup).

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/platforms/83xx/suspend.c |   48 +++++++++++++++++++++++++++++++++
 1 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/suspend.c b/arch/powerpc/platforms/83xx/suspend.c
index b0c2619..4380534 100644
--- a/arch/powerpc/platforms/83xx/suspend.c
+++ b/arch/powerpc/platforms/83xx/suspend.c
@@ -32,6 +32,7 @@
 #define PMCCR1_NEXT_STATE       0x0C /* Next state for power management */
 #define PMCCR1_NEXT_STATE_SHIFT 2
 #define PMCCR1_CURR_STATE       0x03 /* Current state for power management*/
+#define IMMR_SYSCR_OFFSET       0x100
 #define IMMR_RCW_OFFSET         0x900
 #define RCW_PCI_HOST            0x80000000
 
@@ -78,6 +79,22 @@ struct mpc83xx_clock {
 	u32 sccr;
 };
 
+struct mpc83xx_syscr {
+	__be32 sgprl;
+	__be32 sgprh;
+	__be32 spridr;
+	__be32 :32;
+	__be32 spcr;
+	__be32 sicrl;
+	__be32 sicrh;
+};
+
+struct mpc83xx_saved {
+	u32 sicrl;
+	u32 sicrh;
+	u32 sccr;
+};
+
 struct pmc_type {
 	int has_deep_sleep;
 };
@@ -87,6 +104,8 @@ static int has_deep_sleep, deep_sleeping;
 static int pmc_irq;
 static struct mpc83xx_pmc __iomem *pmc_regs;
 static struct mpc83xx_clock __iomem *clock_regs;
+static struct mpc83xx_syscr __iomem *syscr_regs;
+static struct mpc83xx_saved saved_regs;
 static int is_pci_agent, wake_from_pci;
 static phys_addr_t immrbase;
 static int pci_pm_state;
@@ -137,6 +156,20 @@ static irqreturn_t pmc_irq_handler(int irq, void *dev_id)
 	return ret;
 }
 
+static void mpc83xx_suspend_restore_regs(void)
+{
+	out_be32(&syscr_regs->sicrl, saved_regs.sicrl);
+	out_be32(&syscr_regs->sicrh, saved_regs.sicrh);
+	out_be32(&clock_regs->sccr, saved_regs.sccr);
+}
+
+static void mpc83xx_suspend_save_regs(void)
+{
+	saved_regs.sicrl = in_be32(&syscr_regs->sicrl);
+	saved_regs.sicrh = in_be32(&syscr_regs->sicrh);
+	saved_regs.sccr = in_be32(&clock_regs->sccr);
+}
+
 static int mpc83xx_suspend_enter(suspend_state_t state)
 {
 	int ret = -EAGAIN;
@@ -166,6 +199,8 @@ static int mpc83xx_suspend_enter(suspend_state_t state)
 	 */
 
 	if (deep_sleeping) {
+		mpc83xx_suspend_save_regs();
+
 		out_be32(&pmc_regs->mask, PMCER_ALL);
 
 		out_be32(&pmc_regs->config1,
@@ -179,6 +214,8 @@ static int mpc83xx_suspend_enter(suspend_state_t state)
 		         in_be32(&pmc_regs->config1) & ~PMCCR1_POWER_OFF);
 
 		out_be32(&pmc_regs->mask, PMCER_PMCI);
+
+		mpc83xx_suspend_restore_regs();
 	} else {
 		out_be32(&pmc_regs->mask, PMCER_PMCI);
 
@@ -333,12 +370,23 @@ static int pmc_probe(struct of_device *ofdev,
 		goto out_pmc;
 	}
 
+	if (has_deep_sleep) {
+		syscr_regs = ioremap(immrbase + IMMR_SYSCR_OFFSET,
+				     sizeof(*syscr_regs));
+		if (!syscr_regs) {
+			ret = -ENOMEM;
+			goto out_syscr;
+		}
+	}
+
 	if (is_pci_agent)
 		mpc83xx_set_agent();
 
 	suspend_set_ops(&mpc83xx_suspend_ops);
 	return 0;
 
+out_syscr:
+	iounmap(clock_regs);
 out_pmc:
 	iounmap(pmc_regs);
 out:
-- 
1.6.3.3

^ permalink raw reply related

* [PATCH 1/3] powerpc/83xx/suspend: Clear deep_sleeping after devices resume
From: Anton Vorontsov @ 2009-12-10 18:00 UTC (permalink / raw)
  To: Kumar Gala; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <20091210175957.GA30682@oksana.dev.rtsoft.ru>

Currently 83xx PMC driver clears deep_sleeping variable very early,
before devices are resumed. This makes fsl_deep_sleep() unusable in
drivers' resume() callback.

Sure, drivers can store fsl_deep_sleep() value on suspend and use
the stored value on resume. But a better solution is to postpone
clearing the deep_sleeping variable, i.e. move it into finish()
callback.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Acked-by: Scott Wood <scottwood@freescale.com>
---
 arch/powerpc/platforms/83xx/suspend.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/suspend.c b/arch/powerpc/platforms/83xx/suspend.c
index d306f07..b0c2619 100644
--- a/arch/powerpc/platforms/83xx/suspend.c
+++ b/arch/powerpc/platforms/83xx/suspend.c
@@ -194,7 +194,7 @@ out:
 	return ret;
 }
 
-static void mpc83xx_suspend_finish(void)
+static void mpc83xx_suspend_end(void)
 {
 	deep_sleeping = 0;
 }
@@ -278,7 +278,7 @@ static struct platform_suspend_ops mpc83xx_suspend_ops = {
 	.valid = mpc83xx_suspend_valid,
 	.begin = mpc83xx_suspend_begin,
 	.enter = mpc83xx_suspend_enter,
-	.finish = mpc83xx_suspend_finish,
+	.end = mpc83xx_suspend_end,
 };
 
 static int pmc_probe(struct of_device *ofdev,
-- 
1.6.3.3

^ permalink raw reply related

* [PATCH v2 0/3] powerpc/83xx: Sleep and deep sleep support for MPC8315E-RDB
From: Anton Vorontsov @ 2009-12-10 17:59 UTC (permalink / raw)
  To: Kumar Gala; +Cc: Scott Wood, linuxppc-dev

Hi all,

This is quite late resend, sorry.

Only the third patch has changed, i.e. I got rid of sleep-nexus
stuff per Scott and Benjamin suggestions.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply

* Re: [PATCH v3] cpm2_pic: Allow correct flow_types for port C interrupts
From: Scott Wood @ 2009-12-10 17:41 UTC (permalink / raw)
  To: avorontsov; +Cc: Linuxppc-dev Development, Mark Ware
In-Reply-To: <20091210145808.GA16421@oksana.dev.rtsoft.ru>

Anton Vorontsov wrote:
> On Thu, Dec 10, 2009 at 08:52:01AM -0600, Kumar Gala wrote:
>> On Dec 10, 2009, at 5:14 AM, Mark Ware wrote:
>>
>>> Port C interrupts can be either falling edge, or either edge.
>>> Other external interrupts are either falling edge or active low.
>>> Tested on a custom 8280 based board.
>>>
>>> Signed-off-by: Mark Ware <mware@elphinstone.net>
>>> ---
>>> Changed in v3:
>>> - Cosmetic improvements as suggested by Anton and Scott
>>> - Added tested note to changelog
>>>
>>> arch/powerpc/sysdev/cpm2_pic.c |   28 +++++++++++++++++++++-------
>>> 1 files changed, 21 insertions(+), 7 deletions(-)
>> Scott, Anton do you want to add an Ack to this version?
> 
> Acked-by: Anton Vorontsov <avorontsov@ru.mvista.com>

Acked-by: Scott Wood <scottwood@freescale.com>

-Scott

^ 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