LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 3/6] 44x: Removing dead CONFIG_PPC47x
From: Josh Boyer @ 2011-11-30 11:43 UTC (permalink / raw)
  To: Tony Breeds; +Cc: Christoph Egger, LinuxPPC-dev
In-Reply-To: <1322630640-13708-4-git-send-email-tony@bakeyournoodle.com>

On Wed, Nov 30, 2011 at 12:23 AM, Tony Breeds <tony@bakeyournoodle.com> wro=
te:
> From: Christoph Egger <siccegge@cs.fau.de>
>
> CONFIG_PPC47x doesn't exist in Kconfig, therefore removing all
> references for it from the source code.
>
> Signed-off-by: Christoph Egger <siccegge@cs.fau.de>
> ---
> =A0arch/powerpc/mm/44x_mmu.c | =A0 =A04 ----
> =A01 files changed, 0 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/mm/44x_mmu.c b/arch/powerpc/mm/44x_mmu.c
> index f60e006..5d4e3ff 100644
> --- a/arch/powerpc/mm/44x_mmu.c
> +++ b/arch/powerpc/mm/44x_mmu.c
> @@ -78,11 +78,7 @@ static void __init ppc44x_pin_tlb(unsigned int virt, u=
nsigned int phys)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"tlbwe =A0%1,%3,%5\n"
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"tlbwe =A0%0,%3,%6\n"
> =A0 =A0 =A0 =A0:
> -#ifdef CONFIG_PPC47x
> - =A0 =A0 =A0 : "r" (PPC47x_TLB2_S_RWX),
> -#else
> =A0 =A0 =A0 =A0: "r" (PPC44x_TLB_SW | PPC44x_TLB_SR | PPC44x_TLB_SX | PPC=
44x_TLB_G),
> -#endif

That doesn't look right.  The code is there doing something, why is it
just being removed?  I would think the change would be to use
CONFIG_PPC_47x?

Or if the code there isn't needed any longer, the changelog should say why.

josh

^ permalink raw reply

* Re: [PATCH 4/6] powerpc/boot: Add extended precision shifts to the boot wrapper.
From: Josh Boyer @ 2011-11-30 11:45 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: LinuxPPC-dev
In-Reply-To: <1322632107.21641.43.camel@pasglop>

On Wed, Nov 30, 2011 at 12:48 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Wed, 2011-11-30 at 16:23 +1100, Tony Breeds wrote:
>> Code copied from arch/powerpc/kernel/misc_32.S
>>
>> Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
>> ---
>> =A0arch/powerpc/boot/div64.S | =A0 52 ++++++++++++++++++++++++++++++++++=
+++++++++++
>> =A01 files changed, 52 insertions(+), 0 deletions(-)
>
> Should we just link with libgcc ? :-)

Please tell me you're joking.

However, adding this code and wonderful and all but why do we need to
add it?  Changelog should say why.

josh

^ permalink raw reply

* [PATCH net-next v5 4/4] powerpc: tqm8548/tqm8xx: add and update CAN device nodes
From: Wolfgang Grandegger @ 2011-11-30 12:10 UTC (permalink / raw)
  To: netdev; +Cc: devicetree-discuss, linux-can, linuxppc-dev, socketcan-users
In-Reply-To: <1322655035-18809-1-git-send-email-wg@grandegger.com>

This patch enables or updates support for the CC770 and AN82527
CAN controller on the TQM8548 and TQM8xx boards.

CC: devicetree-discuss@lists.ozlabs.org
CC: linuxppc-dev@ozlabs.org
CC: Kumar Gala <galak@kernel.crashing.org>
Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 arch/powerpc/boot/dts/tqm8548-bigflash.dts |   19 ++++++++++++++-----
 arch/powerpc/boot/dts/tqm8548.dts          |   19 ++++++++++++++-----
 arch/powerpc/boot/dts/tqm8xx.dts           |   25 +++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/boot/dts/tqm8548-bigflash.dts b/arch/powerpc/boot/dts/tqm8548-bigflash.dts
index 9452c3c..d918752 100644
--- a/arch/powerpc/boot/dts/tqm8548-bigflash.dts
+++ b/arch/powerpc/boot/dts/tqm8548-bigflash.dts
@@ -352,7 +352,7 @@
 		ranges = <
 			0 0x0 0xfc000000 0x04000000	// NOR FLASH bank 1
 			1 0x0 0xf8000000 0x08000000	// NOR FLASH bank 0
-			2 0x0 0xa3000000 0x00008000	// CAN (2 x i82527)
+			2 0x0 0xa3000000 0x00008000	// CAN (2 x CC770)
 			3 0x0 0xa3010000 0x00008000	// NAND FLASH
 
 		>;
@@ -393,18 +393,27 @@
 		};
 
 		/* Note: CAN support needs be enabled in U-Boot */
-		can0@2,0 {
-			compatible = "intel,82527"; // Bosch CC770
+		can@2,0 {
+			compatible = "bosch,cc770"; // Bosch CC770
 			reg = <2 0x0 0x100>;
 			interrupts = <4 1>;
 			interrupt-parent = <&mpic>;
+			bosch,external-clock-frequency = <16000000>;
+			bosch,disconnect-rx1-input;
+			bosch,disconnect-tx1-output;
+			bosch,iso-low-speed-mux;
+			bosch,clock-out-frequency = <16000000>;
 		};
 
-		can1@2,100 {
-			compatible = "intel,82527"; // Bosch CC770
+		can@2,100 {
+			compatible = "bosch,cc770"; // Bosch CC770
 			reg = <2 0x100 0x100>;
 			interrupts = <4 1>;
 			interrupt-parent = <&mpic>;
+			bosch,external-clock-frequency = <16000000>;
+			bosch,disconnect-rx1-input;
+			bosch,disconnect-tx1-output;
+			bosch,iso-low-speed-mux;
 		};
 
 		/* Note: NAND support needs to be enabled in U-Boot */
diff --git a/arch/powerpc/boot/dts/tqm8548.dts b/arch/powerpc/boot/dts/tqm8548.dts
index 619776f..988d887 100644
--- a/arch/powerpc/boot/dts/tqm8548.dts
+++ b/arch/powerpc/boot/dts/tqm8548.dts
@@ -352,7 +352,7 @@
 		ranges = <
 			0 0x0 0xfc000000 0x04000000	// NOR FLASH bank 1
 			1 0x0 0xf8000000 0x08000000	// NOR FLASH bank 0
-			2 0x0 0xe3000000 0x00008000	// CAN (2 x i82527)
+			2 0x0 0xe3000000 0x00008000	// CAN (2 x CC770)
 			3 0x0 0xe3010000 0x00008000	// NAND FLASH
 
 		>;
@@ -393,18 +393,27 @@
 		};
 
 		/* Note: CAN support needs be enabled in U-Boot */
-		can0@2,0 {
-			compatible = "intel,82527"; // Bosch CC770
+		can@2,0 {
+			compatible = "bosch,cc770"; // Bosch CC770
 			reg = <2 0x0 0x100>;
 			interrupts = <4 1>;
 			interrupt-parent = <&mpic>;
+			bosch,external-clock-frequency = <16000000>;
+			bosch,disconnect-rx1-input;
+			bosch,disconnect-tx1-output;
+			bosch,iso-low-speed-mux;
+			bosch,clock-out-frequency = <16000000>;
 		};
 
-		can1@2,100 {
-			compatible = "intel,82527"; // Bosch CC770
+		can@2,100 {
+			compatible = "bosch,cc770"; // Bosch CC770
 			reg = <2 0x100 0x100>;
 			interrupts = <4 1>;
 			interrupt-parent = <&mpic>;
+			bosch,external-clock-frequency = <16000000>;
+			bosch,disconnect-rx1-input;
+			bosch,disconnect-tx1-output;
+			bosch,iso-low-speed-mux;
 		};
 
 		/* Note: NAND support needs to be enabled in U-Boot */
diff --git a/arch/powerpc/boot/dts/tqm8xx.dts b/arch/powerpc/boot/dts/tqm8xx.dts
index f6da7ec..c3dba25 100644
--- a/arch/powerpc/boot/dts/tqm8xx.dts
+++ b/arch/powerpc/boot/dts/tqm8xx.dts
@@ -57,6 +57,7 @@
 
 		ranges = <
 			0x0 0x0 0x40000000 0x800000
+			0x3 0x0 0xc0000000 0x200
 		>;
 
 		flash@0,0 {
@@ -67,6 +68,30 @@
 			bank-width = <4>;
 			device-width = <2>;
 		};
+
+		/* Note: CAN support needs be enabled in U-Boot */
+		can@3,0 {
+			compatible = "intc,82527";
+			reg = <3 0x0 0x80>;
+			interrupts = <8 1>;
+			interrupt-parent = <&PIC>;
+			bosch,external-clock-frequency = <16000000>;
+			bosch,disconnect-rx1-input;
+			bosch,disconnect-tx1-output;
+			bosch,iso-low-speed-mux;
+			bosch,clock-out-frequency = <16000000>;
+		};
+
+		can@3,100 {
+			compatible = "intc,82527";
+			reg = <3 0x100 0x80>;
+			interrupts = <8 1>;
+			interrupt-parent = <&PIC>;
+			bosch,external-clock-frequency = <16000000>;
+			bosch,disconnect-rx1-input;
+			bosch,disconnect-tx1-output;
+			bosch,iso-low-speed-mux;
+		};
 	};
 
 	soc@fff00000 {
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH net-next v5 3/4] can: cc770: add platform bus driver for the CC770 and AN82527
From: Wolfgang Grandegger @ 2011-11-30 12:10 UTC (permalink / raw)
  To: netdev; +Cc: Devicetree-discuss, linux-can, linuxppc-dev, socketcan-users
In-Reply-To: <1322655035-18809-1-git-send-email-wg@grandegger.com>

This driver works with both, static platform data and device tree
bindings. It has been tested on a TQM855L board with two AN82527
CAN controllers on the local bus.

CC: Devicetree-discuss@lists.ozlabs.org
CC: linuxppc-dev@ozlabs.org
CC: Kumar Gala <galak@kernel.crashing.org>
Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 .../devicetree/bindings/net/can/cc770.txt          |   56 ++++
 drivers/net/can/cc770/Kconfig                      |    7 +
 drivers/net/can/cc770/Makefile                     |    1 +
 drivers/net/can/cc770/cc770_platform.c             |  273 ++++++++++++++++++++
 4 files changed, 337 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/can/cc770.txt
 create mode 100644 drivers/net/can/cc770/cc770_platform.c

diff --git a/Documentation/devicetree/bindings/net/can/cc770.txt b/Documentation/devicetree/bindings/net/can/cc770.txt
new file mode 100644
index 0000000..01e282d
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/cc770.txt
@@ -0,0 +1,56 @@
+Memory mapped Bosch CC770 and Intel AN82527 CAN controller
+
+Note: The CC770 is a CAN controller from Bosch, which is 100%
+compatible with the old AN82527 from Intel, but with "bugs" being fixed.
+
+Required properties:
+
+- compatible : should be "bosch,cc770" for the CC770 and "intc,82527"
+	for the AN82527.
+
+- reg : should specify the chip select, address offset and size required
+	to map the registers of the controller. The size is usually 0x80.
+
+- interrupts : property with a value describing the interrupt source
+	(number and sensitivity) required for the controller.
+
+Optional properties:
+
+- bosch,external-clock-frequency : frequency of the external oscillator
+	clock in Hz. Note that the internal clock frequency used by the
+	controller is half of that value. If not specified, a default
+	value of 16000000 (16 MHz) is used.
+
+- bosch,clock-out-frequency : slock frequency in Hz on the CLKOUT pin.
+	If not specified or if the specified value is 0, the CLKOUT pin
+	will be disabled.
+
+- bosch,slew-rate : slew rate of the CLKOUT signal. If not specified,
+	a resonable value will be calculated.
+
+- bosch,disconnect-rx0-input : see data sheet.
+
+- bosch,disconnect-rx1-input : see data sheet.
+
+- bosch,disconnect-tx1-output : see data sheet.
+
+- bosch,polarity-dominant : see data sheet.
+
+- bosch,divide-memory-clock : see data sheet.
+
+- bosch,iso-low-speed-mux : see data sheet.
+
+For further information, please have a look to the CC770 or AN82527.
+
+Examples:
+
+can@3,100 {
+	compatible = "bosch,cc770";
+	reg = <3 0x100 0x80>;
+	interrupts = <2 0>;
+	interrupt-parent = <&mpic>;
+	bosch,external-clock-frequency = <16000000>;
+};
+
+
+
diff --git a/drivers/net/can/cc770/Kconfig b/drivers/net/can/cc770/Kconfig
index 28e4d48..22c07a8 100644
--- a/drivers/net/can/cc770/Kconfig
+++ b/drivers/net/can/cc770/Kconfig
@@ -11,4 +11,11 @@ config CAN_CC770_ISA
 	  connected to the ISA bus using I/O port, memory mapped or
 	  indirect access.
 
+config CAN_CC770_PLATFORM
+	tristate "Generic Platform Bus based CC770 driver"
+	---help---
+	  This driver adds support for the CC770 and AN82527 chips
+	  connected to the "platform bus" (Linux abstraction for directly
+	  to the processor attached devices).
+
 endif
diff --git a/drivers/net/can/cc770/Makefile b/drivers/net/can/cc770/Makefile
index 872ecff..9fb8321 100644
--- a/drivers/net/can/cc770/Makefile
+++ b/drivers/net/can/cc770/Makefile
@@ -4,5 +4,6 @@
 
 obj-$(CONFIG_CAN_CC770) += cc770.o
 obj-$(CONFIG_CAN_CC770_ISA) += cc770_isa.o
+obj-$(CONFIG_CAN_CC770_PLATFORM) += cc770_platform.o
 
 ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
diff --git a/drivers/net/can/cc770/cc770_platform.c b/drivers/net/can/cc770/cc770_platform.c
new file mode 100644
index 0000000..fb87b22
--- /dev/null
+++ b/drivers/net/can/cc770/cc770_platform.c
@@ -0,0 +1,273 @@
+/*
+ * Driver for CC770 and AN82527 CAN controllers on the platform bus
+ *
+ * Copyright (C) 2009, 2011 Wolfgang Grandegger <wg@grandegger.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the version 2 of the GNU General Public License
+ * as published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+/*
+ * If platform data are used you should have similar definitions
+ * in your board-specific code:
+ *
+ *   static struct cc770_platform_data myboard_cc770_pdata = {
+ *           .osc_freq = 16000000,
+ *           .cir = 0x41,
+ *           .cor = 0x20,
+ *           .bcr = 0x40,
+ *   };
+ *
+ * Please see include/linux/can/platform/cc770.h for description of
+ * above fields.
+ *
+ * If the device tree is used, you need a CAN node definition in your
+ * DTS file similar to:
+ *
+ *   can@3,100 {
+ *           compatible = "bosch,cc770";
+ *           reg = <3 0x100 0x80>;
+ *           interrupts = <2 0>;
+ *           interrupt-parent = <&mpic>;
+ *           bosch,external-clock-frequency = <16000000>;
+ *   };
+ *
+ * See "Documentation/devicetree/bindings/net/can/cc770.txt" for further
+ * information.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/netdevice.h>
+#include <linux/delay.h>
+#include <linux/can.h>
+#include <linux/can/dev.h>
+#include <linux/can/platform/cc770.h>
+
+#include <linux/of_platform.h>
+
+#include "cc770.h"
+
+#define DRV_NAME "cc770_platform"
+
+MODULE_AUTHOR("Wolfgang Grandegger <wg@grandegger.com>");
+MODULE_DESCRIPTION("Socket-CAN driver for CC770 on the platform bus");
+MODULE_LICENSE("GPL v2");
+
+#define CC770_PLATFORM_CAN_CLOCK  16000000
+
+static u8 cc770_platform_read_reg(const struct cc770_priv *priv, int reg)
+{
+	return in_8(priv->reg_base + reg);
+}
+
+static void cc770_platform_write_reg(const struct cc770_priv *priv, int reg,
+				     u8 val)
+{
+	out_8(priv->reg_base + reg, val);
+}
+
+static int __devinit cc770_get_of_node_data(struct platform_device *pdev,
+					    struct cc770_priv *priv)
+{
+	struct device_node *np = pdev->dev.of_node;
+	const u32 *prop;
+	int prop_size;
+	u32 clkext;
+
+	prop = of_get_property(np, "bosch,external-clock-frequency",
+			       &prop_size);
+	if (prop && (prop_size ==  sizeof(u32)))
+		clkext = *prop;
+	else
+		clkext = CC770_PLATFORM_CAN_CLOCK; /* default */
+	priv->can.clock.freq = clkext;
+
+	/* The system clock may not exceed 10 MHz */
+	if (priv->can.clock.freq > 10000000) {
+		priv->cpu_interface |= CPUIF_DSC;
+		priv->can.clock.freq /= 2;
+	}
+
+	/* The memory clock may not exceed 8 MHz */
+	if (priv->can.clock.freq > 8000000)
+		priv->cpu_interface |= CPUIF_DMC;
+
+	if (of_get_property(np, "bosch,divide-memory-clock", NULL))
+		priv->cpu_interface |= CPUIF_DMC;
+	if (of_get_property(np, "bosch,iso-low-speed-mux", NULL))
+		priv->cpu_interface |= CPUIF_MUX;
+
+	if (!of_get_property(np, "bosch,no-comperator-bypass", NULL))
+		priv->bus_config |= BUSCFG_CBY;
+	if (of_get_property(np, "bosch,disconnect-rx0-input", NULL))
+		priv->bus_config |= BUSCFG_DR0;
+	if (of_get_property(np, "bosch,disconnect-rx1-input", NULL))
+		priv->bus_config |= BUSCFG_DR1;
+	if (of_get_property(np, "bosch,disconnect-tx1-output", NULL))
+		priv->bus_config |= BUSCFG_DT1;
+	if (of_get_property(np, "bosch,polarity-dominant", NULL))
+		priv->bus_config |= BUSCFG_POL;
+
+	prop = of_get_property(np, "bosch,clock-out-frequency", &prop_size);
+	if (prop && (prop_size == sizeof(u32)) && *prop > 0) {
+		u32 cdv = clkext / *prop;
+		int slew;
+
+		if (cdv > 0 && cdv < 16) {
+			priv->cpu_interface |= CPUIF_CEN;
+			priv->clkout |= (cdv - 1) & CLKOUT_CD_MASK;
+
+			prop = of_get_property(np, "bosch,slew-rate",
+					       &prop_size);
+			if (prop && (prop_size == sizeof(u32))) {
+				slew = *prop;
+			} else {
+				/* Determine default slew rate */
+				slew = (CLKOUT_SL_MASK >>
+					CLKOUT_SL_SHIFT) -
+					((cdv * clkext - 1) / 8000000);
+				if (slew < 0)
+					slew = 0;
+			}
+			priv->clkout |= (slew << CLKOUT_SL_SHIFT) &
+				CLKOUT_SL_MASK;
+		} else {
+			dev_dbg(&pdev->dev, "invalid clock-out-frequency\n");
+		}
+	}
+
+	return 0;
+}
+
+static int __devinit cc770_get_platform_data(struct platform_device *pdev,
+					     struct cc770_priv *priv)
+{
+
+	struct cc770_platform_data *pdata = pdev->dev.platform_data;
+
+	priv->can.clock.freq = pdata->osc_freq;
+	if (priv->cpu_interface | CPUIF_DSC)
+		priv->can.clock.freq /= 2;
+	priv->clkout = pdata->cor;
+	priv->bus_config = pdata->bcr;
+	priv->cpu_interface = pdata->cir;
+
+	return 0;
+}
+
+static int __devinit cc770_platform_probe(struct platform_device *pdev)
+{
+	struct net_device *dev;
+	struct cc770_priv *priv;
+	struct resource *mem;
+	resource_size_t mem_size;
+	void __iomem *base;
+	int err, irq;
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	irq = platform_get_irq(pdev, 0);
+	if (!mem || irq <= 0)
+		return -ENODEV;
+
+	mem_size = resource_size(mem);
+	if (!request_mem_region(mem->start, mem_size, pdev->name))
+		return -EBUSY;
+
+	base = ioremap(mem->start, mem_size);
+	if (!base) {
+		err = -ENOMEM;
+		goto exit_release_mem;
+	}
+
+	dev = alloc_cc770dev(0);
+	if (!dev) {
+		err = -ENOMEM;
+		goto exit_unmap_mem;
+	}
+
+	dev->irq = irq;
+	priv = netdev_priv(dev);
+	priv->read_reg = cc770_platform_read_reg;
+	priv->write_reg = cc770_platform_write_reg;
+	priv->irq_flags = IRQF_SHARED;
+	priv->reg_base = base;
+
+	if (pdev->dev.of_node)
+		err = cc770_get_of_node_data(pdev, priv);
+	else if (pdev->dev.platform_data)
+		err = cc770_get_platform_data(pdev, priv);
+	else
+		err = -ENODEV;
+	if (err)
+		goto exit_free_cc770;
+
+	dev_dbg(&pdev->dev,
+		 "reg_base=0x%p irq=%d clock=%d cpu_interface=0x%02x "
+		 "bus_config=0x%02x clkout=0x%02x\n",
+		 priv->reg_base, dev->irq, priv->can.clock.freq,
+		 priv->cpu_interface, priv->bus_config, priv->clkout);
+
+	dev_set_drvdata(&pdev->dev, dev);
+	SET_NETDEV_DEV(dev, &pdev->dev);
+
+	err = register_cc770dev(dev);
+	if (err) {
+		dev_err(&pdev->dev,
+			"couldn't register CC700 device (err=%d)\n", err);
+		goto exit_free_cc770;
+	}
+
+	return 0;
+
+exit_free_cc770:
+	free_cc770dev(dev);
+exit_unmap_mem:
+	iounmap(base);
+exit_release_mem:
+	release_mem_region(mem->start, mem_size);
+
+	return err;
+}
+
+static int __devexit cc770_platform_remove(struct platform_device *pdev)
+{
+	struct net_device *dev = dev_get_drvdata(&pdev->dev);
+	struct cc770_priv *priv = netdev_priv(dev);
+	struct resource *mem;
+
+	unregister_cc770dev(dev);
+	iounmap(priv->reg_base);
+	free_cc770dev(dev);
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	release_mem_region(mem->start, resource_size(mem));
+
+	return 0;
+}
+
+static struct of_device_id __devinitdata cc770_platform_table[] = {
+	{.compatible = "bosch,cc770"}, /* CC770 from Bosch */
+	{.compatible = "intc,82527"},  /* AN82527 from Intel CP */
+	{},
+};
+
+static struct platform_driver cc770_platform_driver = {
+	.driver = {
+		.name = DRV_NAME,
+		.owner = THIS_MODULE,
+		.of_match_table = cc770_platform_table,
+	},
+	.probe = cc770_platform_probe,
+	.remove = __devexit_p(cc770_platform_remove),
+};
+
+module_platform_driver(cc770_platform_driver);
+
-- 
1.7.4.1

^ permalink raw reply related

* [RFC PATCH v3 0/4] cpuidle: (POWER) cpuidle driver for pSeries
From: Deepthi Dharwar @ 2011-11-30 12:46 UTC (permalink / raw)
  To: benh, len.brown; +Cc: linuxppc-dev, linux-pm, linux-kernel, linux-pm

This patch series ports the cpuidle framework for ppc64 platform and
implements a cpuidle back-end driver for ppc64 (pSeries) platform.
Currently idle states are managed by pseries_{dedicated,shared}_idle_sleep()
routines in arch/powerpc/platforms/pseries/setup.c.  There are
two idle states (snooze and cede) that are exploited by
these routines based on simple heuristics.

Moving the idle states over to cpuidle framework can take advantage of
the advanced heuristics, tunables, and features provided by cpuidle
framework. Additional idle states like extended cede with hints would be
included and exploited using the cpuidle framework. The statistics and 
tracing infrastructure provided by the cpuidle framework also helps in 
enabling power management related tools and help tune the system and 
applications.

This series aims to maintain compatibility and functionality to
existing pSeries idle cpu management code.  There are no new functions
or idle states added as part of this series.

The previous version of this patch can be found at
https://lkml.org/lkml/2011/11/17/127

Changes from the previous version (v2):

	1] Rebased to latest 3.2-rc3

	2] Incorporated the changes from the feedback provided by Ben
	   in the previous version of this series.

This patch series includes:

[1/4] - Provides arch specific cpu_idle_wait() function required by cpuidle
  subsystem.
[2/4] - pseries_idle cpuidle driver
[3/4] - Enables cpuidle for pSeries and directly calls cpuidle_idle_call()
[4/4] - Handles powersave=off kernel boot parameter and disables registration
  of pseries_idle cpuidle driver.

This series has been tested on ppc64 pSeries POWER7 system with the snooze
and cede states

--

 arch/powerpc/Kconfig                            |    4 
 arch/powerpc/include/asm/processor.h            |    3 
 arch/powerpc/include/asm/system.h               |    9 +
 arch/powerpc/kernel/idle.c                      |   27 ++
 arch/powerpc/kernel/sysfs.c                     |    2 
 arch/powerpc/platforms/Kconfig                  |    6 
 arch/powerpc/platforms/pseries/Kconfig          |    9 +
 arch/powerpc/platforms/pseries/Makefile         |    1 
 arch/powerpc/platforms/pseries/processor_idle.c |  329 +++++++++++++++++++++++
 arch/powerpc/platforms/pseries/pseries.h        |    3 
 arch/powerpc/platforms/pseries/setup.c          |  104 +------
 arch/powerpc/platforms/pseries/smp.c            |    1 
 include/linux/cpuidle.h                         |    2 
 13 files changed, 411 insertions(+), 89 deletions(-)
 create mode 100644 arch/powerpc/platforms/pseries/processor_idle.c



-Deepthi 

^ permalink raw reply

* [RFC PATCH v3 1/4] cpuidle: (powerpc) Add cpu_idle_wait() to allow switching of idle routines
From: Deepthi Dharwar @ 2011-11-30 12:46 UTC (permalink / raw)
  To: benh, len.brown; +Cc: linuxppc-dev, linux-pm, linux-kernel, linux-pm
In-Reply-To: <20111130124608.972.87712.stgit@deepthi-ThinkPad-T420>

This patch provides cpu_idle_wait() routine for the powerpc
platform which is required by the cpuidle subsystem. This
routine is required to change the idle handler on SMP systems.
The equivalent routine for x86 is in arch/x86/kernel/process.c
but the powerpc implementation is different.

cpuidle_disable variable is to enable/disable cpuidle
framework if power_save option is set during the boot
time.

Signed-off-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
Signed-off-by: Trinabh Gupta <g.trinabh@gmail.com>
Signed-off-by: Arun R Bharadwaj <arun.r.bharadwaj@gmail.com>
---
 arch/powerpc/Kconfig                 |    4 ++++
 arch/powerpc/include/asm/processor.h |    2 ++
 arch/powerpc/include/asm/system.h    |    1 +
 arch/powerpc/kernel/idle.c           |   27 +++++++++++++++++++++++++++
 4 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 951e18f..beeeec7 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -87,6 +87,10 @@ config ARCH_HAS_ILOG2_U64
 	bool
 	default y if 64BIT
 
+config ARCH_HAS_CPU_IDLE_WAIT
+	bool
+	default y
+
 config GENERIC_HWEIGHT
 	bool
 	default y
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index eb11a44..811b7e7 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -382,6 +382,8 @@ static inline unsigned long get_clean_sp(struct pt_regs *regs, int is_32)
 }
 #endif
 
+enum idle_boot_override {IDLE_NO_OVERRIDE = 0, IDLE_POWERSAVE_OFF};
+
 #endif /* __KERNEL__ */
 #endif /* __ASSEMBLY__ */
 #endif /* _ASM_POWERPC_PROCESSOR_H */
diff --git a/arch/powerpc/include/asm/system.h b/arch/powerpc/include/asm/system.h
index e30a13d..ff66680 100644
--- a/arch/powerpc/include/asm/system.h
+++ b/arch/powerpc/include/asm/system.h
@@ -221,6 +221,7 @@ extern unsigned long klimit;
 extern void *zalloc_maybe_bootmem(size_t size, gfp_t mask);
 
 extern int powersave_nap;	/* set if nap mode can be used in idle loop */
+void cpu_idle_wait(void);
 
 /*
  * Atomic exchange
diff --git a/arch/powerpc/kernel/idle.c b/arch/powerpc/kernel/idle.c
index 39a2baa..8574b0e 100644
--- a/arch/powerpc/kernel/idle.c
+++ b/arch/powerpc/kernel/idle.c
@@ -39,9 +39,13 @@
 #define cpu_should_die()	0
 #endif
 
+unsigned long cpuidle_disable = IDLE_NO_OVERRIDE;
+EXPORT_SYMBOL(cpuidle_disable);
+
 static int __init powersave_off(char *arg)
 {
 	ppc_md.power_save = NULL;
+	cpuidle_disable = IDLE_POWERSAVE_OFF;
 	return 0;
 }
 __setup("powersave=off", powersave_off);
@@ -102,6 +106,29 @@ void cpu_idle(void)
 	}
 }
 
+
+/*
+ * cpu_idle_wait - Used to ensure that all the CPUs come out of the old
+ * idle loop and start using the new idle loop.
+ * Required while changing idle handler on SMP systems.
+ * Caller must have changed idle handler to the new value before the call.
+ * This window may be larger on shared systems.
+ */
+void cpu_idle_wait(void)
+{
+	int cpu;
+	smp_mb();
+
+	/* kick all the CPUs so that they exit out of old idle routine */
+	get_online_cpus();
+	for_each_online_cpu(cpu) {
+		if (cpu != smp_processor_id())
+			smp_send_reschedule(cpu);
+	}
+	put_online_cpus();
+}
+EXPORT_SYMBOL_GPL(cpu_idle_wait);
+
 int powersave_nap;
 
 #ifdef CONFIG_SYSCTL

^ permalink raw reply related

* [RFC PATCH v3 2/4] cpuidle: (POWER) cpuidle driver for pSeries
From: Deepthi Dharwar @ 2011-11-30 12:46 UTC (permalink / raw)
  To: benh, len.brown; +Cc: linuxppc-dev, linux-pm, linux-kernel, linux-pm
In-Reply-To: <20111130124608.972.87712.stgit@deepthi-ThinkPad-T420>

This patch implements a back-end cpuidle driver for pSeries
based on pseries_dedicated_idle_loop and pseries_shared_idle_loop
routines.  The driver is built only if CONFIG_CPU_IDLE is set. This
cpuidle driver uses global registration of idle states and
not per-cpu.

Signed-off-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
Signed-off-by: Trinabh Gupta <g.trinabh@gmail.com>
Signed-off-by: Arun R Bharadwaj <arun.r.bharadwaj@gmail.com>
---
 arch/powerpc/include/asm/system.h               |    8 +
 arch/powerpc/kernel/sysfs.c                     |    2 
 arch/powerpc/platforms/pseries/Kconfig          |    9 +
 arch/powerpc/platforms/pseries/Makefile         |    1 
 arch/powerpc/platforms/pseries/processor_idle.c |  326 +++++++++++++++++++++++
 arch/powerpc/platforms/pseries/pseries.h        |    3 
 arch/powerpc/platforms/pseries/setup.c          |    3 
 arch/powerpc/platforms/pseries/smp.c            |    1 
 8 files changed, 350 insertions(+), 3 deletions(-)
 create mode 100644 arch/powerpc/platforms/pseries/processor_idle.c

diff --git a/arch/powerpc/include/asm/system.h b/arch/powerpc/include/asm/system.h
index ff66680..f56a0a7 100644
--- a/arch/powerpc/include/asm/system.h
+++ b/arch/powerpc/include/asm/system.h
@@ -223,6 +223,14 @@ extern void *zalloc_maybe_bootmem(size_t size, gfp_t mask);
 extern int powersave_nap;	/* set if nap mode can be used in idle loop */
 void cpu_idle_wait(void);
 
+#ifdef CONFIG_PSERIES_IDLE
+extern void update_smt_snooze_delay(int snooze);
+extern int pseries_notify_cpuidle_add_cpu(int cpu);
+#else
+static inline void update_smt_snooze_delay(int snooze) {}
+static inline int pseries_notify_cpuidle_add_cpu(int cpu) { return 0; }
+#endif
+
 /*
  * Atomic exchange
  *
diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index ce035c1..ebe5d78 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -18,6 +18,7 @@
 #include <asm/machdep.h>
 #include <asm/smp.h>
 #include <asm/pmc.h>
+#include <asm/system.h>
 
 #include "cacheinfo.h"
 
@@ -51,6 +52,7 @@ static ssize_t store_smt_snooze_delay(struct sys_device *dev,
 		return -EINVAL;
 
 	per_cpu(smt_snooze_delay, cpu->sysdev.id) = snooze;
+	update_smt_snooze_delay(snooze);
 
 	return count;
 }
diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
index c81f6bb..ae7b6d4 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -120,3 +120,12 @@ config DTL
 	  which are accessible through a debugfs file.
 
 	  Say N if you are unsure.
+
+config PSERIES_IDLE
+	tristate "Cpuidle driver for pSeries platforms"
+	depends on CPU_IDLE
+	depends on PPC_PSERIES
+	default y
+	help
+	  Select this option to enable processor idle state management
+	  through cpuidle subsystem.
diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
index 3556e40..236db46 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_PHYP_DUMP)		+= phyp_dump.o
 obj-$(CONFIG_CMM)		+= cmm.o
 obj-$(CONFIG_DTL)		+= dtl.o
 obj-$(CONFIG_IO_EVENT_IRQ)	+= io_event_irq.o
+obj-$(CONFIG_PSERIES_IDLE)	+= processor_idle.o
 
 ifeq ($(CONFIG_PPC_PSERIES),y)
 obj-$(CONFIG_SUSPEND)		+= suspend.o
diff --git a/arch/powerpc/platforms/pseries/processor_idle.c b/arch/powerpc/platforms/pseries/processor_idle.c
new file mode 100644
index 0000000..352b78a
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/processor_idle.c
@@ -0,0 +1,326 @@
+/*
+ *  processor_idle - idle state cpuidle driver.
+ *  Adapted from drivers/idle/intel_idle.c and
+ *  drivers/acpi/processor_idle.c
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/moduleparam.h>
+#include <linux/cpuidle.h>
+#include <linux/cpu.h>
+
+#include <asm/paca.h>
+#include <asm/reg.h>
+#include <asm/system.h>
+#include <asm/machdep.h>
+#include <asm/firmware.h>
+
+#include "plpar_wrappers.h"
+#include "pseries.h"
+
+struct cpuidle_driver pseries_idle_driver = {
+	.name =		"pseries_idle",
+	.owner =	THIS_MODULE,
+};
+
+#define MAX_IDLE_STATE_COUNT	2
+
+static int max_idle_state = MAX_IDLE_STATE_COUNT - 1;
+static struct cpuidle_device __percpu *pseries_cpuidle_devices;
+static struct cpuidle_state *cpuidle_state_table;
+
+void update_smt_snooze_delay(int snooze)
+{
+	struct cpuidle_driver *drv = cpuidle_get_driver();
+	if (drv)
+		drv->states[0].target_residency = snooze;
+}
+
+static inline void idle_loop_prolog(unsigned long *in_purr, ktime_t *kt_before)
+{
+
+	*kt_before = ktime_get_real();
+	*in_purr = mfspr(SPRN_PURR);
+	/*
+	 * Indicate to the HV that we are idle. Now would be
+	 * a good time to find other work to dispatch.
+	 */
+	get_lppaca()->idle = 1;
+}
+
+static inline  s64 idle_loop_epilog(unsigned long in_purr, ktime_t kt_before)
+{
+	get_lppaca()->wait_state_cycles += mfspr(SPRN_PURR) - in_purr;
+	get_lppaca()->idle = 0;
+
+	return ktime_to_us(ktime_sub(ktime_get_real(), kt_before));
+}
+
+static int snooze_loop(struct cpuidle_device *dev,
+			struct cpuidle_driver *drv,
+			int index)
+{
+	unsigned long in_purr;
+	ktime_t kt_before;
+	unsigned long start_snooze;
+	long snooze = drv->states[0].target_residency;
+
+	idle_loop_prolog(&in_purr, &kt_before);
+
+	if (snooze) {
+		start_snooze = get_tb() + snooze * tb_ticks_per_usec;
+		local_irq_enable();
+		set_thread_flag(TIF_POLLING_NRFLAG);
+
+		while ((snooze < 0) || (get_tb() < start_snooze)) {
+			if (need_resched() || cpu_is_offline(dev->cpu))
+				goto out;
+			ppc64_runlatch_off();
+			HMT_low();
+			HMT_very_low();
+		}	
+
+		HMT_medium();
+		clear_thread_flag(TIF_POLLING_NRFLAG);
+		smp_mb();
+		local_irq_disable();
+	}
+
+out:
+	HMT_medium();
+	dev->last_residency =
+		(int)idle_loop_epilog(in_purr, kt_before);
+	return index;
+}
+
+static int dedicated_cede_loop(struct cpuidle_device *dev,
+				struct cpuidle_driver *drv,
+				int index)
+{
+	unsigned long in_purr;
+	ktime_t kt_before;
+
+	idle_loop_prolog(&in_purr, &kt_before);
+	get_lppaca()->donate_dedicated_cpu = 1;
+
+	ppc64_runlatch_off();
+	HMT_medium();
+	cede_processor();
+
+	get_lppaca()->donate_dedicated_cpu = 0;
+	dev->last_residency =
+		(int)idle_loop_epilog(in_purr, kt_before);
+	return index;
+}
+
+static int shared_cede_loop(struct cpuidle_device *dev,
+			struct cpuidle_driver *drv,
+			int index)
+{
+	unsigned long in_purr;
+	ktime_t kt_before;
+
+	idle_loop_prolog(&in_purr, &kt_before);
+
+	/*
+	 * Yield the processor to the hypervisor.  We return if
+	 * an external interrupt occurs (which are driven prior
+	 * to returning here) or if a prod occurs from another
+	 * processor. When returning here, external interrupts
+	 * are enabled.
+	 */
+	cede_processor();
+
+	dev->last_residency =
+		(int)idle_loop_epilog(in_purr, kt_before);
+	return index;
+}
+
+/*
+ * States for dedicated partition case.
+ */
+static struct cpuidle_state dedicated_states[MAX_IDLE_STATE_COUNT] = {
+	{ /* Snooze */
+		.name = "snooze",
+		.desc = "snooze",
+		.flags = CPUIDLE_FLAG_TIME_VALID,
+		.exit_latency = 0,
+		.target_residency = 0,
+		.enter = &snooze_loop },
+	{ /* CEDE */
+		.name = "CEDE",
+		.desc = "CEDE",
+		.flags = CPUIDLE_FLAG_TIME_VALID,
+		.exit_latency = 1,
+		.target_residency = 10,
+		.enter = &dedicated_cede_loop },
+};
+
+/*
+ * States for shared partition case.
+ */
+static struct cpuidle_state shared_states[MAX_IDLE_STATE_COUNT] = {
+	{ /* Shared Cede */
+		.name = "Shared Cede",
+		.desc = "Shared Cede",
+		.flags = CPUIDLE_FLAG_TIME_VALID,
+		.exit_latency = 0,
+		.target_residency = 0,
+		.enter = &shared_cede_loop },
+};
+
+int pseries_notify_cpuidle_add_cpu(int cpu)
+{
+	struct cpuidle_device *dev =
+			per_cpu_ptr(pseries_cpuidle_devices, cpu);
+	if (dev && cpuidle_get_driver()) {
+		cpuidle_disable_device(dev);
+		cpuidle_enable_device(dev);
+	}
+	return 0;
+}
+
+/*
+ * pseries_cpuidle_driver_init()
+ */
+static int pseries_cpuidle_driver_init(void)
+{
+	int idle_state;
+	struct cpuidle_driver *drv = &pseries_idle_driver;
+
+	drv->state_count = 0;
+
+	for (idle_state = 0; idle_state < MAX_IDLE_STATE_COUNT; ++idle_state) {
+
+		if (idle_state > max_idle_state)
+			break;
+
+		/* is the state not enabled? */
+		if (cpuidle_state_table[idle_state].enter == NULL)
+			continue;
+
+		drv->states[drv->state_count] =	/* structure copy */
+			cpuidle_state_table[idle_state];
+
+		if (cpuidle_state_table == dedicated_states)
+			drv->states[drv->state_count].target_residency =
+				__get_cpu_var(smt_snooze_delay);
+
+		drv->state_count += 1;
+	}
+
+	return 0;
+}
+
+/* pseries_idle_devices_uninit(void)
+ * unregister cpuidle devices and de-allocate memory
+ */
+static void pseries_idle_devices_uninit(void)
+{
+	int i;
+	struct cpuidle_device *dev;
+
+	for_each_possible_cpu(i) {
+		dev = per_cpu_ptr(pseries_cpuidle_devices, i);
+		cpuidle_unregister_device(dev);
+	}
+
+	free_percpu(pseries_cpuidle_devices);
+	return;
+}
+
+/* pseries_idle_devices_init()
+ * allocate, initialize and register cpuidle device
+ */
+static int pseries_idle_devices_init(void)
+{
+	int i;
+	struct cpuidle_driver *drv = &pseries_idle_driver;
+	struct cpuidle_device *dev;
+
+	pseries_cpuidle_devices = alloc_percpu(struct cpuidle_device);
+	if (pseries_cpuidle_devices == NULL)
+		return -ENOMEM;
+
+	for_each_possible_cpu(i) {
+		dev = per_cpu_ptr(pseries_cpuidle_devices, i);
+		dev->state_count = drv->state_count;
+		dev->cpu = i;
+		if (cpuidle_register_device(dev)) {
+			printk(KERN_DEBUG \
+				"cpuidle_register_device %d failed!\n", i);
+			return -EIO;
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * pseries_idle_probe()
+ * Choose state table for shared versus dedicated partition
+ */
+static int pseries_idle_probe(void)
+{
+
+	if (!firmware_has_feature(FW_FEATURE_SPLPAR))
+		return -ENODEV;
+
+	if (max_idle_state == 0) {
+		printk(KERN_DEBUG "pseries processor idle disabled.\n");
+		return -EPERM;
+	}
+
+	if (get_lppaca()->shared_proc)
+		cpuidle_state_table = shared_states;
+	else
+		cpuidle_state_table = dedicated_states;
+
+	return 0;
+}
+
+static int __init pseries_processor_idle_init(void)
+{
+	int retval;
+
+	retval = pseries_idle_probe();
+	if (retval)
+		return retval;
+
+	pseries_cpuidle_driver_init();
+	retval = cpuidle_register_driver(&pseries_idle_driver);
+	if (retval) {
+		printk(KERN_DEBUG "Registration of pseries driver failed.\n");
+		return retval;
+	}
+
+	retval = pseries_idle_devices_init();
+	if (retval) {
+		pseries_idle_devices_uninit();
+		cpuidle_unregister_driver(&pseries_idle_driver);
+		return retval;
+	}
+
+	printk(KERN_DEBUG "pseries_idle_driver registered\n");
+
+	return 0;
+}
+
+static void __exit pseries_processor_idle_exit(void)
+{
+
+	pseries_idle_devices_uninit();
+	cpuidle_unregister_driver(&pseries_idle_driver);
+
+	return;
+}
+
+module_init(pseries_processor_idle_init);
+module_exit(pseries_processor_idle_exit);
+
+MODULE_AUTHOR("Deepthi Dharwar <deepthi@linux.vnet.ibm.com>");
+MODULE_DESCRIPTION("Cpuidle driver for POWER");
+MODULE_LICENSE("GPL");
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 24c7162..9a3dda0 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -57,4 +57,7 @@ extern struct device_node *dlpar_configure_connector(u32);
 extern int dlpar_attach_node(struct device_node *);
 extern int dlpar_detach_node(struct device_node *);
 
+/* Snooze Delay, pseries_idle */
+DECLARE_PER_CPU(long, smt_snooze_delay);
+
 #endif /* _PSERIES_PSERIES_H */
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index c3408ca..9c6716a 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -586,9 +586,6 @@ static int __init pSeries_probe(void)
 	return 1;
 }
 
-
-DECLARE_PER_CPU(long, smt_snooze_delay);
-
 static void pseries_dedicated_idle_sleep(void)
 { 
 	unsigned int cpu = smp_processor_id();
diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
index 26e93fd..bbc3c42 100644
--- a/arch/powerpc/platforms/pseries/smp.c
+++ b/arch/powerpc/platforms/pseries/smp.c
@@ -148,6 +148,7 @@ static void __devinit smp_xics_setup_cpu(int cpu)
 	set_cpu_current_state(cpu, CPU_STATE_ONLINE);
 	set_default_offline_state(cpu);
 #endif
+	pseries_notify_cpuidle_add_cpu(cpu);
 }
 
 static int __devinit smp_pSeries_kick_cpu(int nr)

^ permalink raw reply related

* [RFC PATCH v3 3/4] cpuidle: (POWER) Enable cpuidle and directly call cpuidle_idle_call() for pSeries
From: Deepthi Dharwar @ 2011-11-30 12:46 UTC (permalink / raw)
  To: benh, len.brown; +Cc: linuxppc-dev, linux-pm, linux-kernel, linux-pm
In-Reply-To: <20111130124608.972.87712.stgit@deepthi-ThinkPad-T420>

This patch enables cpuidle for pSeries and pSeries_idle is
directly called from the idle loop. As a result of pSeries_idle, cpuidle
driver registered with cpuidle subsystem comes into action. On
failure of loading of the driver or cpuidle framework default idle
is executed as part of the function. This patch
also removes the routines pseries_shared_idle_sleep and
pseries_dedicated_idle_sleep as they are now implemented as part of
pseries_idle cpuidle driver.

Signed-off-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
Signed-off-by: Trinabh Gupta <g.trinabh@gmail.com>
Signed-off-by: Arun R Bharadwaj <arun.r.bharadwaj@gmail.com>
---
 arch/powerpc/platforms/Kconfig         |    6 ++
 arch/powerpc/platforms/pseries/setup.c |  101 +++++---------------------------
 include/linux/cpuidle.h                |    2 -
 3 files changed, 23 insertions(+), 86 deletions(-)

diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
index 3fe6d92..31e1ade 100644
--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig
@@ -211,6 +211,12 @@ config PPC_PASEMI_CPUFREQ
 
 endmenu
 
+menu "CPUIdle driver"
+
+source "drivers/cpuidle/Kconfig"
+
+endmenu
+
 config PPC601_SYNC_FIX
 	bool "Workarounds for PPC601 bugs"
 	depends on 6xx && (PPC_PREP || PPC_PMAC)
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 9c6716a..f19fc52 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -39,6 +39,7 @@
 #include <linux/irq.h>
 #include <linux/seq_file.h>
 #include <linux/root_dev.h>
+#include <linux/cpuidle.h>
 
 #include <asm/mmu.h>
 #include <asm/processor.h>
@@ -74,9 +75,6 @@ EXPORT_SYMBOL(CMO_PageSize);
 
 int fwnmi_active;  /* TRUE if an FWNMI handler is present */
 
-static void pseries_shared_idle_sleep(void);
-static void pseries_dedicated_idle_sleep(void);
-
 static struct device_node *pSeries_mpic_node;
 
 static void pSeries_show_cpuinfo(struct seq_file *m)
@@ -352,6 +350,21 @@ static int alloc_dispatch_log_kmem_cache(void)
 }
 early_initcall(alloc_dispatch_log_kmem_cache);
 
+static void pSeries_idle(void)
+{
+	/* This would call on the cpuidle framework, and the back-end pseries
+	 * driver to  go to idle states
+	 */
+	if (cpuidle_idle_call()) {
+		/* On error, execute default handler
+		 * to go into low thread priority and possibly
+		 * low power mode.
+		 */
+		HMT_low();
+		HMT_very_low();
+	}
+}
+
 static void __init pSeries_setup_arch(void)
 {
 	/* Discover PIC type and setup ppc_md accordingly */
@@ -374,18 +387,9 @@ static void __init pSeries_setup_arch(void)
 
 	pSeries_nvram_init();
 
-	/* Choose an idle loop */
 	if (firmware_has_feature(FW_FEATURE_SPLPAR)) {
 		vpa_init(boot_cpuid);
-		if (get_lppaca()->shared_proc) {
-			printk(KERN_DEBUG "Using shared processor idle loop\n");
-			ppc_md.power_save = pseries_shared_idle_sleep;
-		} else {
-			printk(KERN_DEBUG "Using dedicated idle loop\n");
-			ppc_md.power_save = pseries_dedicated_idle_sleep;
-		}
-	} else {
-		printk(KERN_DEBUG "Using default idle loop\n");
+		ppc_md.power_save = pSeries_idle;
 	}
 
 	if (firmware_has_feature(FW_FEATURE_LPAR))
@@ -586,77 +590,6 @@ static int __init pSeries_probe(void)
 	return 1;
 }
 
-static void pseries_dedicated_idle_sleep(void)
-{ 
-	unsigned int cpu = smp_processor_id();
-	unsigned long start_snooze;
-	unsigned long in_purr, out_purr;
-	long snooze = __get_cpu_var(smt_snooze_delay);
-
-	/*
-	 * Indicate to the HV that we are idle. Now would be
-	 * a good time to find other work to dispatch.
-	 */
-	get_lppaca()->idle = 1;
-	get_lppaca()->donate_dedicated_cpu = 1;
-	in_purr = mfspr(SPRN_PURR);
-
-	/*
-	 * We come in with interrupts disabled, and need_resched()
-	 * has been checked recently.  If we should poll for a little
-	 * while, do so.
-	 */
-	if (snooze) {
-		start_snooze = get_tb() + snooze * tb_ticks_per_usec;
-		local_irq_enable();
-		set_thread_flag(TIF_POLLING_NRFLAG);
-
-		while ((snooze < 0) || (get_tb() < start_snooze)) {
-			if (need_resched() || cpu_is_offline(cpu))
-				goto out;
-			ppc64_runlatch_off();
-			HMT_low();
-			HMT_very_low();
-		}
-
-		HMT_medium();
-		clear_thread_flag(TIF_POLLING_NRFLAG);
-		smp_mb();
-		local_irq_disable();
-		if (need_resched() || cpu_is_offline(cpu))
-			goto out;
-	}
-
-	cede_processor();
-
-out:
-	HMT_medium();
-	out_purr = mfspr(SPRN_PURR);
-	get_lppaca()->wait_state_cycles += out_purr - in_purr;
-	get_lppaca()->donate_dedicated_cpu = 0;
-	get_lppaca()->idle = 0;
-}
-
-static void pseries_shared_idle_sleep(void)
-{
-	/*
-	 * Indicate to the HV that we are idle. Now would be
-	 * a good time to find other work to dispatch.
-	 */
-	get_lppaca()->idle = 1;
-
-	/*
-	 * Yield the processor to the hypervisor.  We return if
-	 * an external interrupt occurs (which are driven prior
-	 * to returning here) or if a prod occurs from another
-	 * processor. When returning here, external interrupts
-	 * are enabled.
-	 */
-	cede_processor();
-
-	get_lppaca()->idle = 0;
-}
-
 static int pSeries_pci_probe_mode(struct pci_bus *bus)
 {
 	if (firmware_has_feature(FW_FEATURE_LPAR))
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 7408af8..23f81de 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -130,7 +130,6 @@ struct cpuidle_driver {
 #ifdef CONFIG_CPU_IDLE
 extern void disable_cpuidle(void);
 extern int cpuidle_idle_call(void);
-
 extern int cpuidle_register_driver(struct cpuidle_driver *drv);
 struct cpuidle_driver *cpuidle_get_driver(void);
 extern void cpuidle_unregister_driver(struct cpuidle_driver *drv);
@@ -145,7 +144,6 @@ extern void cpuidle_disable_device(struct cpuidle_device *dev);
 #else
 static inline void disable_cpuidle(void) { }
 static inline int cpuidle_idle_call(void) { return -ENODEV; }
-
 static inline int cpuidle_register_driver(struct cpuidle_driver *drv)
 {return -ENODEV; }
 static inline struct cpuidle_driver *cpuidle_get_driver(void) {return NULL; }

^ permalink raw reply related

* Re: [PATCH 5/6] powerpc/boot: Add mfdcrx
From: Segher Boessenkool @ 2011-11-30 13:09 UTC (permalink / raw)
  To: Tony Breeds; +Cc: LinuxPPC-dev
In-Reply-To: <1322630640-13708-6-git-send-email-tony@bakeyournoodle.com>

> +#define mfdcrx(rn) \
> +	({	\
> +		unsigned long rval; \
> +		asm volatile("mfdcrx %0,%1" : "=r"(rval) : "g"(rn)); \
> +		rval; \
> +	})

"g" is never correct on PowerPC, you want "r" here.  You can write
this as a static inline btw, you only need the #define stuff when
there is an "i" constraint involved.


Segher

^ permalink raw reply

* Re: [PATCH 6/6] 44x/currituck: Add support for the new IBM currituck platform
From: Kumar Gala @ 2011-11-30 13:23 UTC (permalink / raw)
  To: Tony Breeds; +Cc: LinuxPPC-dev
In-Reply-To: <1322630640-13708-7-git-send-email-tony@bakeyournoodle.com>


On Nov 29, 2011, at 11:24 PM, Tony Breeds wrote:

> Based on original work by David 'Shaggy' Kliekamp.
>=20
> Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
> ---
> arch/powerpc/boot/Makefile                   |    5 +-
> arch/powerpc/boot/dts/currituck.dts          |  240 =
++++++++++++++++++++++++++
> arch/powerpc/boot/treeboot-currituck.c       |  129 ++++++++++++++
> arch/powerpc/boot/wrapper                    |    3 +
> arch/powerpc/configs/44x/currituck_defconfig |  110 ++++++++++++
> arch/powerpc/include/asm/reg.h               |    1 +
> arch/powerpc/kernel/cputable.c               |   14 ++
> arch/powerpc/kernel/head_44x.S               |    2 +
> arch/powerpc/platforms/44x/Kconfig           |   10 +
> arch/powerpc/platforms/44x/Makefile          |    1 +
> arch/powerpc/platforms/44x/ppc47x.c          |  198 =
+++++++++++++++++++++
> arch/powerpc/sysdev/ppc4xx_pci.c             |   57 ++++++-
> arch/powerpc/sysdev/ppc4xx_pci.h             |    7 +
> 13 files changed, 775 insertions(+), 2 deletions(-)
> create mode 100644 arch/powerpc/boot/dts/currituck.dts
> create mode 100644 arch/powerpc/boot/treeboot-currituck.c
> create mode 100644 arch/powerpc/configs/44x/currituck_defconfig
> create mode 100644 arch/powerpc/platforms/44x/ppc47x.c

Split the board support patches from the SoC support.



> diff --git a/arch/powerpc/include/asm/reg.h =
b/arch/powerpc/include/asm/reg.h
> index 559da19..aa38de6 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -951,6 +951,7 @@
> #define PVR_403GCX	0x00201400
> #define PVR_405GP	0x40110000
> #define PVR_476		0x11a52000
> +#define PVR_476CURRITUCK	0x7ff50000

This seems like it should be PVR_476FPE

> #define PVR_STB03XXX	0x40310000
> #define PVR_NP405H	0x41410000
> #define PVR_NP405L	0x41610000
> diff --git a/arch/powerpc/kernel/cputable.c =
b/arch/powerpc/kernel/cputable.c
> index edae5bb..02e0749 100644
> --- a/arch/powerpc/kernel/cputable.c
> +++ b/arch/powerpc/kernel/cputable.c
> @@ -1830,6 +1830,20 @@ static struct cpu_spec __initdata cpu_specs[] =3D=
 {
> 		.machine_check		=3D machine_check_47x,
> 		.platform		=3D "ppc470",
> 	},
> +	{ /* 476 core Currituck */

comment should probably be:
	 /* 476FPE */

> +		.pvr_mask		=3D 0xffff0000,
> +		.pvr_value		=3D 0x7ff50000,
> +		.cpu_name		=3D "476",

should probably be "476FPE"

> +		.cpu_features		=3D CPU_FTRS_47X | =
CPU_FTR_476_DD2,
> +		.cpu_user_features	=3D COMMON_USER_BOOKE |
> +			PPC_FEATURE_HAS_FPU,
> +		.mmu_features		=3D MMU_FTR_TYPE_47x |
> +			MMU_FTR_USE_TLBIVAX_BCAST | =
MMU_FTR_LOCK_BCAST_INVAL,
> +		.icache_bsize		=3D 32,
> +		.dcache_bsize		=3D 128,
> +		.machine_check		=3D machine_check_47x,
> +		.platform		=3D "ppc470",
> +	},
> 	{ /* 476 iss */
> 		.pvr_mask		=3D 0xffff0000,
> 		.pvr_value		=3D 0x00050000,
> diff --git a/arch/powerpc/kernel/head_44x.S =
b/arch/powerpc/kernel/head_44x.S
> index b725dab..3aca1e2 100644
> --- a/arch/powerpc/kernel/head_44x.S
> +++ b/arch/powerpc/kernel/head_44x.S
> @@ -732,6 +732,8 @@ _GLOBAL(init_cpu_state)
> 	/* We use the PVR to differenciate 44x cores from 476 */
> 	mfspr	r3,SPRN_PVR
> 	srwi	r3,r3,16
> +	cmplwi	cr0,r3,PVR_476CURRITUCK@h
> +	beq	head_start_47x
> 	cmplwi	cr0,r3,PVR_476@h
> 	beq	head_start_47x
> 	cmplwi	cr0,r3,PVR_476_ISS@h

- k=

^ permalink raw reply

* Re: Kernel support for the Freescale P2020-MSC8156 AdvancedMC Reference Design
From: Kumar Gala @ 2011-11-30 13:24 UTC (permalink / raw)
  To: Daniel Ng2; +Cc: linuxppc-dev
In-Reply-To: <32883132.post@talk.nabble.com>


On Nov 29, 2011, at 11:34 PM, Daniel Ng2 wrote:

>=20
> Hi,
>=20
> Does anyone know of any kernel support for the Freescale P2020-MSC8156 =
AMC
> board?-
>=20
> =
http://freescale.com.hk/webapp/sps/site/prod_summary.jsp?code=3DP2020-MSC8=
156AMCRD
>=20
> I am looking for platform-specific files ie. the ones that go in
> arch/powerpc/platforms/85xx and DTS files if possible...
>=20
> Cheers,
> Daniel

There isn't any support for this board in the open source kernel.

- k=

^ permalink raw reply

* [RFC PATCH v3 4/4] cpuidle: (POWER) Handle power_save=off
From: Deepthi Dharwar @ 2011-11-30 12:47 UTC (permalink / raw)
  To: benh, len.brown; +Cc: linuxppc-dev, linux-pm, linux-kernel, linux-pm
In-Reply-To: <20111130124608.972.87712.stgit@deepthi-ThinkPad-T420>

This patch makes pseries_idle_driver not to be registered when
power_save=off kernel boot option is specified. The
cpuidle_disable variable used here is similar to
its usage on x86. If cpuidle_disable is set then
sysfs entries for cpuidle framework are not created
and the required drivers are not loaded.

Signed-off-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
Signed-off-by: Trinabh Gupta <g.trinabh@gmail.com>
Signed-off-by: Arun R Bharadwaj <arun.r.bharadwaj@gmail.com>
---
 arch/powerpc/include/asm/processor.h            |    1 +
 arch/powerpc/platforms/pseries/processor_idle.c |    3 +++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 811b7e7..b585bff 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -382,6 +382,7 @@ static inline unsigned long get_clean_sp(struct pt_regs *regs, int is_32)
 }
 #endif
 
+extern unsigned long cpuidle_disable;
 enum idle_boot_override {IDLE_NO_OVERRIDE = 0, IDLE_POWERSAVE_OFF};
 
 #endif /* __KERNEL__ */
diff --git a/arch/powerpc/platforms/pseries/processor_idle.c b/arch/powerpc/platforms/pseries/processor_idle.c
index 352b78a..4f59af0 100644
--- a/arch/powerpc/platforms/pseries/processor_idle.c
+++ b/arch/powerpc/platforms/pseries/processor_idle.c
@@ -269,6 +269,9 @@ static int pseries_idle_probe(void)
 	if (!firmware_has_feature(FW_FEATURE_SPLPAR))
 		return -ENODEV;
 
+	if (cpuidle_disable != IDLE_NO_OVERRIDE)
+		return -ENODEV;
+
 	if (max_idle_state == 0) {
 		printk(KERN_DEBUG "pseries processor idle disabled.\n");
 		return -EPERM;

^ permalink raw reply related

* Re: [PATCH v3 2/8] [booke] Rename mapping based RELOCATABLE to DYNAMIC_MEMSTART for BookE
From: Josh Boyer @ 2011-11-30 14:41 UTC (permalink / raw)
  To: Scott Wood
  Cc: Josh Poimboeuf, David Laight, Alan Modra, Suzuki K. Poulose,
	linuxppc-dev
In-Reply-To: <4ED4126D.7000201@freescale.com>

On Mon, Nov 28, 2011 at 5:59 PM, Scott Wood <scottwood@freescale.com> wrote=
:
> On 11/23/2011 10:47 AM, Josh Boyer wrote:
>> On Mon, Nov 14, 2011 at 12:41 AM, Suzuki K. Poulose <suzuki@in.ibm.com> =
wrote:
>>> The current implementation of CONFIG_RELOCATABLE in BookE is based
>>> on mapping the page aligned kernel load address to KERNELBASE. This
>>> approach however is not enough for platforms, where the TLB page size
>>> is large (e.g, 256M on 44x). So we are renaming the RELOCATABLE used
>>> currently in BookE to DYNAMIC_MEMSTART to reflect the actual method.
>
> Should reword the config help to make it clear what the alignment
> restriction is, or where to find the information for a particular
> platform. =A0Someone reading "page aligned" without any context that we'r=
e
> talking about special large pages is going to think 4K -- and on e500,
> many large page sizes are supported, so the required alignment is found
> in Linux init code rather than a CPU manual.
>
>>>
>>> The CONFIG_RELOCATABLE for PPC32(BookE) based on processing of the
>>> dynamic relocations will be introduced in the later in the patch series=
.
>>>
>>> This change would allow the use of the old method of RELOCATABLE for
>>> platforms which can afford to enforce the page alignment (platforms wit=
h
>>> smaller TLB size).
>>
>> I'm OK with the general direction, but this touches a lot of non-4xx
>> code. =A0I'd prefer it if Ben took this directly on whatever final
>> solution is done.
>>
>>> I haven tested this change only on 440x. I don't have an FSL BookE to v=
erify
>>> the changes there.
>>>
>>> Scott,
>>> Could you please test this patch on FSL and let me know the results ?
>>
>> Scott, did you ever get around to testing this? =A0In my opinion, this
>> shouldn't go in without a Tested-by: from someone that tried it on an
>> FSL platform.
>
> Booted OK for me on e500v2 with RAM starting at 256M.
>
> Tested-by: Scott Wood <scottwood@freescale.com>
>
>> We add DYNAMIC_MEMSTART for 32-bit, and we have RELOCATABLE for
>> 64-bit. =A0Then throughout almost the rest of the patch, all we're doing
>> is duplicating what RELOCATABLE already did (e.g. if ! either thing).
>> It works, but it is kind of ugly.
>>
>> Instead, could we define a helper config variable that can be used in
>> place of that construct? =A0Something like:
>>
>> config NONSTATIC_KERNEL (or whatever)
>> =A0 =A0 bool
>> =A0 =A0 default n
>>
>> ...
>>
>> config DYNAMIC_MEMSTART
>> =A0 =A0 <blah>
>> =A0 =A0 select NONSTATIC_KERNEL
>>
>> ...
>>
>> config RELOCATABLE
>> =A0 =A0 <blah>
>> =A0 =A0 select NONSTATIC_KERNEL
>
> I agree.

Suzie do you think you could respin this patch with the suggested
changes and include Scott's Tested-by?  The rest of the series looks
fine and I'd like to get it included in my next branch.

josh

^ permalink raw reply

* Please pull 'next' branch of new 4xx tree
From: Josh Boyer @ 2011-11-30 15:14 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Stephen Rothwell; +Cc: linuxppc-dev

Hi Ben,

I have a new 4xx tree setup now.  Two small commits for the next
branch are queued up.  I'd like to get the patch series from Suzie and
Tony included soon as well, but thought I'd start with these to get
things rolling.

Stephen, can you switch the linux-next tree to use this instead?

josh

The following changes since commit fa8cbaaf5a68f62db3f9a8444ecbb940b47984cb:

  powerpc+sparc64/mm: Remove hack in mmap randomize layout (2011-11-28
11:42:09 +1100)

are available in the git repository at:
  git://git.infradead.org/users/jwboyer/powerpc-4xx.git next

Josh Boyer (1):
      MAINTAINERS: Update PowerPC 4xx git tree

Tanmay Inamdar (1):
      powerpc/40x: Add APM8018X SOC support

 MAINTAINERS                                 |    2 +-
 arch/powerpc/boot/dts/klondike.dts          |  227 +++++++++++++++++++++++++++
 arch/powerpc/configs/40x/klondike_defconfig |   55 +++++++
 arch/powerpc/kernel/cputable.c              |   13 ++
 arch/powerpc/platforms/40x/Kconfig          |   11 ++
 arch/powerpc/platforms/40x/ppc40x_simple.c  |    1 +
 6 files changed, 308 insertions(+), 1 deletions(-)
 create mode 100644 arch/powerpc/boot/dts/klondike.dts
 create mode 100644 arch/powerpc/configs/40x/klondike_defconfig

^ permalink raw reply

* [PATCH] powerpc/85xx: don't call of_platform_bus_probe() twice
From: Timur Tabi @ 2011-11-30 16:19 UTC (permalink / raw)
  To: kumar.gala, benh, scottwood, dbaryshkov, linuxppc-dev

Commit 46d026ac ("powerpc/85xx: consolidate of_platform_bus_probe calls")
replaced platform-specific of_device_id tables with a single function
that probes the most of the busses in 85xx device trees.  If a specific
platform needed additional busses probed, then it could call
of_platform_bus_probe() again.  Typically, the additional platform-specific
busses are children of existing busses that have already been probed.
of_platform_bus_probe() does not handle those child busses automatically.

Unfortunately, this doesn't actually work.  The second (platform-specific)
call to of_platform_bus_probe() never finds any of the busses it's asked
to find.

To remedy this, the platform-specific of_device_id tables are eliminated,
and their entries are merged into mpc85xx_common_ids[], so that all busses
are probed at once.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 arch/powerpc/platforms/85xx/common.c      |    6 ++++++
 arch/powerpc/platforms/85xx/mpc85xx_mds.c |   11 +----------
 arch/powerpc/platforms/85xx/p1022_ds.c    |   13 +------------
 3 files changed, 8 insertions(+), 22 deletions(-)

diff --git a/arch/powerpc/platforms/85xx/common.c b/arch/powerpc/platforms/85xx/common.c
index 9fef530..67dac22 100644
--- a/arch/powerpc/platforms/85xx/common.c
+++ b/arch/powerpc/platforms/85xx/common.c
@@ -21,6 +21,12 @@ static struct of_device_id __initdata mpc85xx_common_ids[] = {
 	{ .compatible = "fsl,qe", },
 	{ .compatible = "fsl,cpm2", },
 	{ .compatible = "fsl,srio", },
+	/* So that the DMA channel nodes can be probed individually: */
+	{ .compatible = "fsl,eloplus-dma", },
+	/* For the PMC driver */
+	{ .compatible = "fsl,mpc8548-guts", },
+	/* Probably unnecessary? */
+	{ .compatible = "gpio-leds", },
 	{},
 };
 
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
index 495cfd9..4a555ee 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
@@ -410,12 +410,6 @@ static int __init board_fixups(void)
 machine_arch_initcall(mpc8568_mds, board_fixups);
 machine_arch_initcall(mpc8569_mds, board_fixups);
 
-static struct of_device_id mpc85xx_ids[] = {
-	{ .compatible = "fsl,mpc8548-guts", },
-	{ .compatible = "gpio-leds", },
-	{},
-};
-
 static int __init mpc85xx_publish_devices(void)
 {
 	if (machine_is(mpc8568_mds))
@@ -423,10 +417,7 @@ static int __init mpc85xx_publish_devices(void)
 	if (machine_is(mpc8569_mds))
 		simple_gpiochip_init("fsl,mpc8569mds-bcsr-gpio");
 
-	mpc85xx_common_publish_devices();
-	of_platform_bus_probe(NULL, mpc85xx_ids, NULL);
-
-	return 0;
+	return mpc85xx_common_publish_devices();
 }
 
 machine_device_initcall(mpc8568_mds, mpc85xx_publish_devices);
diff --git a/arch/powerpc/platforms/85xx/p1022_ds.c b/arch/powerpc/platforms/85xx/p1022_ds.c
index 2bf4342..ea6190b 100644
--- a/arch/powerpc/platforms/85xx/p1022_ds.c
+++ b/arch/powerpc/platforms/85xx/p1022_ds.c
@@ -326,18 +326,7 @@ static void __init p1022_ds_setup_arch(void)
 	pr_info("Freescale P1022 DS reference board\n");
 }
 
-static struct of_device_id __initdata p1022_ds_ids[] = {
-	/* So that the DMA channel nodes can be probed individually: */
-	{ .compatible = "fsl,eloplus-dma", },
-	{},
-};
-
-static int __init p1022_ds_publish_devices(void)
-{
-	mpc85xx_common_publish_devices();
-	return of_platform_bus_probe(NULL, p1022_ds_ids, NULL);
-}
-machine_device_initcall(p1022_ds, p1022_ds_publish_devices);
+machine_device_initcall(p1022_ds, mpc85xx_common_publish_devices);
 
 machine_arch_initcall(p1022_ds, swiotlb_setup_bus_notifier);
 
-- 
1.7.3.4

^ permalink raw reply related

* Re: [PATCH][RFC] fsldma: fix performance degradation by optimizing spinlock use.
From: Ira W. Snyder @ 2011-11-30 17:07 UTC (permalink / raw)
  To: Shi Xuelin-B29237
  Cc: vinod.koul@intel.com, dan.j.williams@intel.com,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	Li Yang-R58472
In-Reply-To: <DBB740589CE8814680DECFE34BE197AB14A1E0@039-SN1MPN1-006.039d.mgd.msft.net>

On Wed, Nov 30, 2011 at 09:57:47AM +0000, Shi Xuelin-B29237 wrote:
> Hello Ira,
> 
> In drivers/dma/dmaengine.c, we have below tight loop to check DMA completion in mainline Linux:
>        do {
>                 status = dma_async_is_tx_complete(chan, cookie, NULL, NULL);
>                 if (time_after_eq(jiffies, dma_sync_wait_timeout)) {
>                         printk(KERN_ERR "dma_sync_wait_timeout!\n");
>                         return DMA_ERROR;
>                 }
>         } while (status == DMA_IN_PROGRESS);
> 

That is the body of dma_sync_wait(). It is mostly used in the raid code.
I understand that you don't want to change the raid code to use
callbacks.

In any case, I think we've strayed from the topic under consideration,
which is: can we remove this spinlock without introducing a bug.

I'm convinced that "smp_rmb()" is needed when removing the spinlock. As
noted, Documentation/memory-barriers.txt says that stores on one CPU can
be observed by another CPU in a different order.

Previously, there was an UNLOCK (in fsl_dma_tx_submit) followed by a
LOCK (in fsl_tx_status). This provided a "full barrier", forcing the
operations to complete correctly when viewed by the second CPU. From the
text:

> Therefore, from (1), (2) and (4) an UNLOCK followed by an unconditional LOCK is
> equivalent to a full barrier, but a LOCK followed by an UNLOCK is not.

Also, please read "EXAMPLES OF MEMORY BARRIER SEQUENCES" and "INTER-CPU
LOCKING BARRIER EFFECTS". Particularly, in "EXAMPLES OF MEMORY BARRIER
SEQUENCES", the text notes:

> Without intervention, CPU 2 may perceive the events on CPU 1 in some
> effectively random order, despite the write barrier issued by CPU 1:
>
> [snip diagram]
>
> And thirdly, a read barrier acts as a partial order on loads. Consider the
> following sequence of events:
>
> [snip diagram]
>
> Without intervention, CPU 2 may then choose to perceive the events on CPU 1 in
> some effectively random order, despite the write barrier issued by CPU 1:
>
> [snip diagram]
>

And so on. Please read this entire section in the document.

I can't give you an ACK on the proposed patch. To the best of my
understanding, I believe it introduces a bug. I've tried to provide as
much evidence for this belief as I can, in the form of documentation in
the kernel source tree. If you can cite some documentation that shows I
am wrong, I will happily change my mind!

Ira

> -----Original Message-----
> From: Ira W. Snyder [mailto:iws@ovro.caltech.edu] 
> Sent: 2011年11月30日 1:26
> To: Li Yang-R58472
> Cc: Shi Xuelin-B29237; vinod.koul@intel.com; dan.j.williams@intel.com; linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH][RFC] fsldma: fix performance degradation by optimizing spinlock use.
> 
> On Tue, Nov 29, 2011 at 03:19:05AM +0000, Li Yang-R58472 wrote:
> > > Subject: Re: [PATCH][RFC] fsldma: fix performance degradation by 
> > > optimizing spinlock use.
> > > 
> > > On Thu, Nov 24, 2011 at 08:12:25AM +0000, Shi Xuelin-B29237 wrote:
> > > > Hi Ira,
> > > >
> > > > Thanks for your review.
> > > >
> > > > After second thought, I think your scenario may not occur.
> > > > Because the cookie 20 we query must be returned by 
> > > > fsl_dma_tx_submit(...) in
> > > practice.
> > > > We never query a cookie not returned by fsl_dma_tx_submit(...).
> > > >
> > > 
> > > I agree about this part.
> > > 
> > > > When we call fsl_tx_status(20), the chan->common.cookie is 
> > > > definitely wrote as
> > > 20 and cpu2 could not read as 19.
> > > >
> > > 
> > > This is what I don't agree about. However, I'm not an expert on CPU cache vs.
> > > memory accesses in an multi-processor system. The section titled 
> > > "CACHE COHERENCY" in Documentation/memory-barriers.txt leads me to 
> > > believe that the scenario I described is possible.
> > 
> > For Freescale PowerPC, the chip automatically takes care of cache coherency.  Even if this is a concern, spinlock can't address it.
> > 
> > > 
> > > What happens if CPU1's write of chan->common.cookie only goes into 
> > > CPU1's cache. It never makes it to main memory before CPU2 fetches the old value of 19.
> > > 
> > > I don't think you should see any performance impact from the 
> > > smp_mb() operation.
> > 
> > Smp_mb() do have impact on performance if it's in the hot path.  While it might be safer having it, I doubt it is really necessary.  If the CPU1 doesn't have the updated last_used, it's shouldn't have known there is a cookie 20 existed either.
> > 
> 
> I believe that you are correct, for powerpc. However, anything outside of arch/powerpc shouldn't assume it only runs on powerpc. I wouldn't be surprised to see fsldma running on an iMX someday (ARM processor).
> 
> My interpretation says that the change introduces the possibility that
> fsl_tx_status() returns the wrong answer for an extremely small time window, on SMP only, based on Documentation/memory-barriers.txt. But I can't seem convince you.
> 
> My real question is what code path is hitting this spinlock? Is it in mainline Linux? Why is it polling rather than using callbacks to determine DMA completion?
> 
> Thanks,
> Ira
> 
> > > > -----Original Message-----
> > > > From: Ira W. Snyder [mailto:iws@ovro.caltech.edu]
> > > > Sent: 2011年11月23日 2:59
> > > > To: Shi Xuelin-B29237
> > > > Cc: dan.j.williams@intel.com; Li Yang-R58472; zw@zh-kernel.org; 
> > > > vinod.koul@intel.com; linuxppc-dev@lists.ozlabs.org; 
> > > > linux-kernel@vger.kernel.org
> > > > Subject: Re: [PATCH][RFC] fsldma: fix performance degradation by 
> > > > optimizing
> > > spinlock use.
> > > >
> > > > On Tue, Nov 22, 2011 at 12:55:05PM +0800, b29237@freescale.com wrote:
> > > > > From: Forrest Shi <b29237@freescale.com>
> > > > >
> > > > >     dma status check function fsl_tx_status is heavily called in
> > > > >     a tight loop and the desc lock in fsl_tx_status contended by
> > > > >     the dma status update function. this caused the dma performance
> > > > >     degrades much.
> > > > >
> > > > >     this patch releases the lock in the fsl_tx_status function.
> > > > >     I believe it has no neglect impact on the following call of
> > > > >     dma_async_is_complete(...).
> > > > >
> > > > >     we can see below three conditions will be identified as success
> > > > >     a)  x < complete < use
> > > > >     b)  x < complete+N < use+N
> > > > >     c)  x < complete < use+N
> > > > >     here complete is the completed_cookie, use is the last_used
> > > > >     cookie, x is the querying cookie, N is MAX cookie
> > > > >
> > > > >     when chan->completed_cookie is being read, the last_used may
> > > > >     be incresed. Anyway it has no neglect impact on the dma status
> > > > >     decision.
> > > > >
> > > > >     Signed-off-by: Forrest Shi <xuelin.shi@freescale.com>
> > > > > ---
> > > > >  drivers/dma/fsldma.c |    5 -----
> > > > >  1 files changed, 0 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c index 
> > > > > 8a78154..1dca56f 100644
> > > > > --- a/drivers/dma/fsldma.c
> > > > > +++ b/drivers/dma/fsldma.c
> > > > > @@ -986,15 +986,10 @@ static enum dma_status 
> > > > > fsl_tx_status(struct
> > > dma_chan *dchan,
> > > > >  	struct fsldma_chan *chan = to_fsl_chan(dchan);
> > > > >  	dma_cookie_t last_complete;
> > > > >  	dma_cookie_t last_used;
> > > > > -	unsigned long flags;
> > > > > -
> > > > > -	spin_lock_irqsave(&chan->desc_lock, flags);
> > > > >
> > > >
> > > > This will cause a bug. See below for a detailed explanation. You need this instead:
> > > >
> > > > 	/*
> > > > 	 * On an SMP system, we must ensure that this CPU has seen the
> > > > 	 * memory accesses performed by another CPU under the
> > > > 	 * chan->desc_lock spinlock.
> > > > 	 */
> > > > 	smp_mb();
> > > > >  	last_complete = chan->completed_cookie;
> > > > >  	last_used = dchan->cookie;
> > > > >
> > > > > -	spin_unlock_irqrestore(&chan->desc_lock, flags);
> > > > > -
> > > > >  	dma_set_tx_state(txstate, last_complete, last_used, 0);
> > > > >  	return dma_async_is_complete(cookie, last_complete, 
> > > > > last_used);  }
> > > >
> > > > Facts:
> > > > - dchan->cookie is the same member as chan->common.cookie (same 
> > > > memory
> > > > location)
> > > > - chan->common.cookie is the "last allocated cookie for a pending transaction"
> > > > - chan->completed_cookie is the "last completed transaction"
> > > >
> > > > I have replaced "dchan->cookie" with "chan->common.cookie" in the 
> > > > below
> > > explanation, to keep everything referenced from the same structure.
> > > >
> > > > Variable usage before your change. Everything is used locked.
> > > > - RW chan->common.cookie		(fsl_dma_tx_submit)
> > > > - R  chan->common.cookie		(fsl_tx_status)
> > > > - R  chan->completed_cookie		(fsl_tx_status)
> > > > - W  chan->completed_cookie		(dma_do_tasklet)
> > > >
> > > > Variable usage after your change:
> > > > - RW chan->common.cookie		LOCKED
> > > > - R  chan->common.cookie		NO LOCK
> > > > - R  chan->completed_cookie		NO LOCK
> > > > - W  chan->completed_cookie             LOCKED
> > > >
> > > > What if we assume that you have a 2 CPU system (such as a P2020). 
> > > > After your
> > > changes, one possible sequence is:
> > > >
> > > > === CPU1 - allocate + submit descriptor: fsl_dma_tx_submit() === 
> > > > spin_lock_irqsave
> > > > descriptor->cookie = 20		(x in your example)
> > > > chan->common.cookie = 20	(used in your example)
> > > > spin_unlock_irqrestore
> > > >
> > > > === CPU2 - immediately calls fsl_tx_status() ===
> > > > chan->common.cookie == 19
> > > > chan->completed_cookie == 19
> > > > descriptor->cookie == 20
> > > >
> > > > Since we don't have locks anymore, CPU2 may not have seen the 
> > > > write to
> > > > chan->common.cookie yet.
> > > >
> > > > Also assume that the DMA hardware has not started processing the 
> > > > transaction yet. Therefore dma_do_tasklet() has not been called, 
> > > > and
> > > > chan->completed_cookie has not been updated.
> > > >
> > > > In this case, dma_async_is_complete() (on CPU2) returns 
> > > > DMA_SUCCESS, even
> > > though the DMA operation has not succeeded. The DMA operation has 
> > > not even started yet!
> > > >
> > > > The smp_mb() fixes this, since it forces CPU2 to have seen all 
> > > > memory operations
> > > that happened before CPU1 released the spinlock. Spinlocks are 
> > > implicit SMP memory barriers.
> > > >
> > > > Therefore, the above example becomes:
> > > > smp_mb();
> > > > chan->common.cookie == 20
> > > > chan->completed_cookie == 19
> > > > descriptor->cookie == 20
> > > >
> > > > Then dma_async_is_complete() returns DMA_IN_PROGRESS, which is correct.
> > > >
> > > > Thanks,
> > > > Ira
> > > >
> > > >
> > > > _______________________________________________
> > > > Linuxppc-dev mailing list
> > > > Linuxppc-dev@lists.ozlabs.org
> > > > https://lists.ozlabs.org/listinfo/linuxppc-dev
> > 
> 

^ permalink raw reply

* RE: [PATCH 5/6] powerpc/boot: Add mfdcrx
From: David Laight @ 2011-11-30 18:10 UTC (permalink / raw)
  To: Segher Boessenkool, Tony Breeds; +Cc: LinuxPPC-dev
In-Reply-To: <F91E9007-5DA5-4E3A-993E-EA680DE08D79@kernel.crashing.org>

=20
> > +#define mfdcrx(rn) \
> > +	({	\
> > +		unsigned long rval; \
> > +		asm volatile("mfdcrx %0,%1" : "=3Dr"(rval) : "g"(rn)); \
> > +		rval; \
> > +	})
>=20
> "g" is never correct on PowerPC, you want "r" here.  You can write
> this as a static inline btw, you only need the #define stuff when
> there is an "i" constraint involved.

I think you can still use static inlines even when a
constraint is one that requires a compile time constant.
eg (not ppc, but the "n" become part of the instruction
word):

static __inline__ uint32_t
custom_inic(const uint32_t op, uint32_t a, const uint32_t b)
{
    uint32_t result;

    __asm__ ( "custom\t%1, %0, %2, c%3"
        : "=3Dr" (result) : "n" (op), "r" (a), "n" (b));
    return result;
}

	David

^ permalink raw reply

* Re: scsi/ipr failed to initialize >=linux-3.0.9, >=linux-3.1.1
From: acrux @ 2011-11-30 18:27 UTC (permalink / raw)
  To: acrux; +Cc: linuxppc-dev
In-Reply-To: <20111127143752.269cc898.acrux_it@libero.it>

On Sun, 27 Nov 2011 14:37:52 +0100
acrux <acrux_it@libero.it> wrote:

> 
> scsi subsystem with ipr driver fails to initialize with every kernel >=3.0.9 and >=3.1.1
> Checked on YDL_Powerstation, IBM 9114-275, IBM 9123-710,
> 

well, it seems this was already fixed by one of these two commits (only for linux-3.2):
[SCSI] ipr: Stop reading adapter dump prematurely
commit	41e9a69641fb3fa86fa9277a179f3ad261d072f7
[SCSI] ipr: Fix BUG on adapter dump timeout
commit	4c647e909fceb9df8ec8f06016dd56244045a929

indeed YDL_Powerstation scsi ctrl has no issue with linux-3.2-rc3 .
Thus i guess also other platforms can work fine.

cheers,
--nico
-- 
acrux <acrux_it@libero.it>

^ permalink raw reply

* Re: [PATCH 3/6] 44x: Removing dead CONFIG_PPC47x
From: Benjamin Herrenschmidt @ 2011-11-30 20:20 UTC (permalink / raw)
  To: Josh Boyer; +Cc: Christoph Egger, LinuxPPC-dev
In-Reply-To: <CA+5PVA637ckfo60Foc_G_yhuJg6=fbuyQcjFDwYV_XFLkd6whA@mail.gmail.com>

On Wed, 2011-11-30 at 06:43 -0500, Josh Boyer wrote:
> 
> That doesn't look right.  The code is there doing something, why is it
> just being removed?  I would think the change would be to use
> CONFIG_PPC_47x?
> 
> Or if the code there isn't needed any longer, the changelog should say
> why.

Ah right, I tripped on this one too when reviewing then figured it out
but I agree, the changelog should be clearer.

If you notice, the original ifdef was in a function that is only ever
used on 44x. There's a separate function that handles 47x. I suppose
this is a leftover of the initial port which somebody forgot to remove.

So the patch is fine, but yes, the changelog could be made clearer.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH 4/6] powerpc/boot: Add extended precision shifts to the boot wrapper.
From: Benjamin Herrenschmidt @ 2011-11-30 20:21 UTC (permalink / raw)
  To: Josh Boyer; +Cc: LinuxPPC-dev
In-Reply-To: <CA+5PVA6kBquh2=sPy25PKGSMBWvoZFFvZjrL4bgdmddFgjyaQA@mail.gmail.com>

On Wed, 2011-11-30 at 06:45 -0500, Josh Boyer wrote:
> On Wed, Nov 30, 2011 at 12:48 AM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> > On Wed, 2011-11-30 at 16:23 +1100, Tony Breeds wrote:
> >> Code copied from arch/powerpc/kernel/misc_32.S
> >>
> >> Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
> >> ---
> >>  arch/powerpc/boot/div64.S |   52 +++++++++++++++++++++++++++++++++++++++++++++
> >>  1 files changed, 52 insertions(+), 0 deletions(-)
> >
> > Should we just link with libgcc ? :-)
> 
> Please tell me you're joking.

Only half... I wonder what it would look like. Wouldn't ld only pickup
what we use anyway ?

> However, adding this code and wonderful and all but why do we need to
> add it?  Changelog should say why.

Agreed.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH 4/6] powerpc/boot: Add extended precision shifts to the boot wrapper.
From: Scott Wood @ 2011-11-30 20:28 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: LinuxPPC-dev
In-Reply-To: <1322684475.29041.2.camel@pasglop>

On 11/30/2011 02:21 PM, Benjamin Herrenschmidt wrote:
> On Wed, 2011-11-30 at 06:45 -0500, Josh Boyer wrote:
>> On Wed, Nov 30, 2011 at 12:48 AM, Benjamin Herrenschmidt
>> <benh@kernel.crashing.org> wrote:
>>> On Wed, 2011-11-30 at 16:23 +1100, Tony Breeds wrote:
>>>> Code copied from arch/powerpc/kernel/misc_32.S
>>>>
>>>> Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
>>>> ---
>>>>  arch/powerpc/boot/div64.S |   52 +++++++++++++++++++++++++++++++++++++++++++++
>>>>  1 files changed, 52 insertions(+), 0 deletions(-)
>>>
>>> Should we just link with libgcc ? :-)
>>
>> Please tell me you're joking.
> 
> Only half... I wonder what it would look like. Wouldn't ld only pickup
> what we use anyway ?

We do it in U-Boot...

Only problem I see is that it would need to be built as soft-float, and
apparently that isn't supported by gcc on 64-bit ppc.  Unfortunately
there's isn't a "no float" mode.

-Scott

^ permalink raw reply

* Re: [PATCH 6/8] powerpc/ps3: Fix pr_debug build warnings
From: Geert Uytterhoeven @ 2011-11-30 20:57 UTC (permalink / raw)
  To: Geoff Levand; +Cc: cbe-oss-dev, linuxppc-dev
In-Reply-To: <0b0c0efa6cca6e5e169b4b3e81373a3bab75716a.1322615824.git.geoff@infradead.org>

Hi Geoff,

On Wed, Nov 30, 2011 at 02:38, Geoff Levand <geoff@infradead.org> wrote:
> Fix some PS3 build warnings when DEBUG is defined.
>
> Fixes warnings like these:
>
> =C2=A0format '%lx' expects type 'long unsigned int', but argument 7 has t=
ype 'u64'
>
> Signed-off-by: Geoff Levand <geoff@infradead.org>
> ---
> =C2=A0arch/powerpc/platforms/ps3/repository.c | =C2=A0 14 ++++++++------
> =C2=A01 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/platforms/ps3/repository.c b/arch/powerpc/platf=
orms/ps3/repository.c
> index cb68729..2ce2782 100644
> --- a/arch/powerpc/platforms/ps3/repository.c
> +++ b/arch/powerpc/platforms/ps3/repository.c
> @@ -1050,7 +1050,7 @@ int ps3_repository_dump_resource_info(const struct =
ps3_repository_device *repo)
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0pr_debug("%s:%d (%=
u:%u) reg_type %u, bus_addr %lxh, len %lxh\n",
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0__func__, __LINE__, repo->bus_index, repo->dev_index,
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 reg_type, bus_addr, len);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 reg_type, (unsigned long)bus_addr, (unsigned long)len);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0pr_debug(" <- %s:%d\n", __func__, __LINE__);

The correct way to format u64 is using the "ll" length modifier. That way y=
ou
don't need casts.

The code above was originally written before the u64 uniformization, when p=
pc64
was still using "unsigned long" for u64.

Gr{oetje,eeting}s,

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k=
.org

In personal conversations with technical people, I call myself a hacker. Bu=
t
when I'm talking to journalists I just say "programmer" or something like t=
hat.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0 =C2=A0=C2=A0 -- Linus Torvalds

^ permalink raw reply

* Re: [BUG?]3.0-rc4+ftrace+kprobe: set kprobe at instruction 'stwu' lead to system crash/freeze
From: Benjamin Herrenschmidt @ 2011-11-30 21:00 UTC (permalink / raw)
  To: tiejun.chen
  Cc: Jim Keniston, Anton Blanchard, linux-kernel, Steven Rostedt,
	Yong Zhang, paulus, yrl.pp-manager.tt, Masami Hiramatsu,
	linuxppc-dev
In-Reply-To: <4ED60E2B.8030603@windriver.com>

On Wed, 2011-11-30 at 19:06 +0800, tiejun.chen wrote:

> >  - Copy the exception frame we're about to unwind to just -below- the
> > new r1 value we want to write to. Then perform the write, and change
> > r1 to point to that copy of the frame.
> > 
> >  - Branch to restore: which will unwind everything from that copy of
> > the frame, and eventually set r1 to GPR(1) in the copy which contains
> > the new value of r1.
> 
> We still can't restore this there.

Yes, we can since we have copied the pt_regs down to -below- where we
are going to write to. That's the whole trick. We copy the pt_regs
somewhere "safe" and restore from there.

> I mean we have to do that real restore here. So I'm really not sure if its a
> good way to add such a codes including check TIF/copy-get new r1/restore
> operation here since this is so deep for the exception return code.

No. Re-read my explanation.

In the do_work case (so when things are still easy), we copy the pt_regs
of the return frame to a safe place (right below where we want to write
to typically), and change r1 to point to this "new" frame which we use
to restore from. Then we do the store which is now safe and go to an
unmodified "restore" exit path.

> > This is the less intrusive approach and should work just fine, it's also
> > more robust than anything I've been able to think of and the approach
> > would work for 32 and 64-bit similarily.
> > 
> > (***) Above comment about a bug: If you look at entry_64.S version of
> > ret_from_except_lite you'll notice that in the !preempt case, after
> > we've checked MSR_PR we test for any TIF flag in _TIF_USER_WORK_MASK to
> > decide whether to go to do_work or not. However, in the preempt case, we
> > do a convoluted trick to test SIGPENDING only if PR was set and always
> > test NEED_RESCHED ... but we forget to test any other bit of
> > _TIF_USER_WORK_MASK !!! So that means that with preempt, we completely
> > fail to test for things like single step, syscall tracing, etc...
> > 
> 
> This is another problem we should address.
> 
> > I think this should be fixed at the same time, by simplifying the code
> > by doing:
> > 
> >  - Test PR. If set, go to test_work_user, else continue (or the other
> > way around and call it test_work_kernel)
> > 
> >  - In test_work_user, always test for _TIF_USER_WORK_MASK to decide to
> > go to do_work, maybe call it do_user_work
> > 
> >  - In test_work_kernel, test for _TIF_KERNEL_WORK_MASK which is set to
> > our new flag along with NEED_RESCHED if preempt is enabled and branch to
> > do_kernel_work.
> > 
> > do_user_work is basically the same as today's user_work
> > 
> > do_kernel_work is basically the same as today preempt block with added
> > code to handle the new flag as described above.
> > 
> > Is anybody volunteering for fixing that ? I don't have the bandwidth
> 
> I always use one specific kprobe stack to fix this for BOOKE and work well in my
> local tree :) Do you remember my v3 patch? I think its possible to extend this
> for all PPC variants.
>
> Anyway, I'd like to be this volunteer with our last solution.

So the second problem I exposed, for which you just volunteered
(hint :-) is an orthogonal issue not related to kprobe or stacks which
happen to be something I discovered yesterday while looking at the code.

As for the solution to the emulation problem, re-read my explanation.
The whole trick is that we can avoid a separate stack (which I really
want to avoid) and we can avoid touching the low level restore code
path.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH v2] powerpc/40x: Add APM8018X SOC support
From: Benjamin Herrenschmidt @ 2011-11-30 21:03 UTC (permalink / raw)
  To: Tanmay Inamdar; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1322550101-28520-1-git-send-email-tinamdar@apm.com>

On Tue, 2011-11-29 at 12:31 +0530, Tanmay Inamdar wrote:
> The AppliedMicro APM8018X embedded processor targets embedded applications that
> require low power and a small footprint. It features a PowerPC 405 processor
> core built in a 65nm low-power CMOS process with a five-stage pipeline executing
> up to one instruction per cycle. The family has 128-kbytes of on-chip memory,
> a 128-bit local bus and on-chip DDR2 SDRAM controller with 16-bit interface.

Thanks, looks much better.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH v2] powerpc/40x: Add APM8018X SOC support
From: Josh Boyer @ 2011-11-30 21:07 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Tanmay Inamdar, linux-kernel
In-Reply-To: <1322686997.29041.11.camel@pasglop>

On Wed, Nov 30, 2011 at 4:03 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Tue, 2011-11-29 at 12:31 +0530, Tanmay Inamdar wrote:
>> The AppliedMicro APM8018X embedded processor targets embedded applications that
>> require low power and a small footprint. It features a PowerPC 405 processor
>> core built in a 65nm low-power CMOS process with a five-stage pipeline executing
>> up to one instruction per cycle. The family has 128-kbytes of on-chip memory,
>> a 128-bit local bus and on-chip DDR2 SDRAM controller with 16-bit interface.
>
> Thanks, looks much better.

Yes, agreed.  So much better I already sent it to Ben in a pull request.

josh

^ 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