LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: powerpc/cell/cpufreq: add spu aware cpufreq governor
From: Dominik Brodowski @ 2008-07-10 21:16 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Stephen Rothwell, Dave Jones, cpufreq, linuxppc-dev, Jeremy Kerr,
	cbe-oss-dev
In-Reply-To: <200807102305.08752.arnd@arndb.de>

On Thu, Jul 10, 2008 at 11:05:08PM +0200, Arnd Bergmann wrote:
> On Thursday 10 July 2008, Dominik Brodowski wrote:
> > Actually, "cpufreq-set -g spu_governor" should be enough. (BTW, do we really
> > want this name?)
> 
> Do you have a better suggestion for the name? I don't think there was a
> particularly strong reason to choose it over something else.

The issue I was pondering is whether to include "governor" in the name, for
we do not do this for the other governors. So here are my suggestions :)

	spu
	spuload
	spudemand

Best,
	Dominik

^ permalink raw reply

* [1/3] powerpc: add _HEAD_GLOBAL to place functions in .text.head
From: Milton Miller @ 2008-07-10 21:14 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <for-27-patch1-0@bga.com>

This is the first step to moving the interrupt vectors to .text.head.

Rather than creating more repetition, factor out creating a function
descriptor, from declaring it to be global in a section.

The order of .globl and the symbol definitions is changed but is
a don't care.

Signed-off-by: Milton Miller <miltonm@bga.com>
---
I left the section in the macros, and added a new macro.   I didn't
want to hunt down other uses of _GLOBAL to audit the section, and _KPROBE
needs to set the section.


diff --git a/include/asm-powerpc/ppc_asm.h b/include/asm-powerpc/ppc_asm.h
index 0966899..cb5a4b0 100644
--- a/include/asm-powerpc/ppc_asm.h
+++ b/include/asm-powerpc/ppc_asm.h
@@ -178,11 +178,8 @@ END_FTR_SECTION_IFCLR(CPU_FTR_PURR);					\
 #define XGLUE(a,b) a##b
 #define GLUE(a,b) XGLUE(a,b)
 
-#define _GLOBAL(name) \
-	.section ".text"; \
+#define _FUNCTION(name) \
 	.align 2 ; \
-	.globl name; \
-	.globl GLUE(.,name); \
 	.section ".opd","aw"; \
 name: \
 	.quad GLUE(.,name); \
@@ -192,57 +189,41 @@ name: \
 	.type GLUE(.,name),@function; \
 GLUE(.,name):
 
+#define _GLOBAL(name) \
+	.section ".text"; \
+	.globl name; \
+	.globl GLUE(.,name); \
+	_FUNCTION(name)
+
 #define _INIT_GLOBAL(name) \
 	.section ".text.init.refok"; \
-	.align 2 ; \
 	.globl name; \
 	.globl GLUE(.,name); \
-	.section ".opd","aw"; \
-name: \
-	.quad GLUE(.,name); \
-	.quad .TOC.@tocbase; \
-	.quad 0; \
-	.previous; \
-	.type GLUE(.,name),@function; \
-GLUE(.,name):
+	_FUNCTION(name)
+
+#define _HEAD_GLOBAL(name) \
+	.section ".text.head"; \
+	.globl name; \
+	.globl GLUE(.,name); \
+	_FUNCTION(name)
 
 #define _KPROBE(name) \
 	.section ".kprobes.text","a"; \
-	.align 2 ; \
 	.globl name; \
 	.globl GLUE(.,name); \
-	.section ".opd","aw"; \
-name: \
-	.quad GLUE(.,name); \
-	.quad .TOC.@tocbase; \
-	.quad 0; \
-	.previous; \
-	.type GLUE(.,name),@function; \
-GLUE(.,name):
+	_FUNCTION(name)
 
 #define _STATIC(name) \
 	.section ".text"; \
-	.align 2 ; \
-	.section ".opd","aw"; \
-name: \
-	.quad GLUE(.,name); \
-	.quad .TOC.@tocbase; \
-	.quad 0; \
-	.previous; \
-	.type GLUE(.,name),@function; \
-GLUE(.,name):
+	_FUNCTION(name)
 
 #define _INIT_STATIC(name) \
 	.section ".text.init.refok"; \
-	.align 2 ; \
-	.section ".opd","aw"; \
-name: \
-	.quad GLUE(.,name); \
-	.quad .TOC.@tocbase; \
-	.quad 0; \
-	.previous; \
-	.type GLUE(.,name),@function; \
-GLUE(.,name):
+	_FUNCTION(name)
+
+#define _HEAD_STATIC(name) \
+	.section ".text.head"; \
+	_FUNCTION(name)
 
 #else /* 32-bit */
 

^ permalink raw reply related

* mtd: remove __PPC__ hardcoded address from nand/diskonchip and devices/docprobe
From: Milton Miller @ 2008-07-10 21:14 UTC (permalink / raw)
  To: David Woodhouse, linux-mtd; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <for-27-patch1-0@bga.com>

Such a hardcoded address can cause a checkstop or machine check if
the driver is in the kernel but the address is not acknowledged.

Both drivers allow an address to be specified as either a module
parameter or config option.   Any future powerpc board should either
use one of these methods or find the address in the device tree.

Signed-off-by: Milton Miller <miltonm@bga.com>
---
There is no powerpc defconfig that sets either CONFIG_DOC200{0,1*} nor
CONFIG_MTD_NAND_DISKONCHIP in next-20080710.

diff --git a/drivers/mtd/devices/docprobe.c b/drivers/mtd/devices/docprobe.c
index 6e5d811..6e62922 100644
--- a/drivers/mtd/devices/docprobe.c
+++ b/drivers/mtd/devices/docprobe.c
@@ -76,8 +76,6 @@ static unsigned long __initdata doc_locations[] = {
 	0xe0000, 0xe2000, 0xe4000, 0xe6000,
 	0xe8000, 0xea000, 0xec000, 0xee000,
 #endif /*  CONFIG_MTD_DOCPROBE_HIGH */
-#elif defined(__PPC__)
-	0xe4000000,
 #else
 #warning Unknown architecture for DiskOnChip. No default probe locations defined
 #endif
diff --git a/drivers/mtd/nand/diskonchip.c b/drivers/mtd/nand/diskonchip.c
index cd4393c..765d4f0 100644
--- a/drivers/mtd/nand/diskonchip.c
+++ b/drivers/mtd/nand/diskonchip.c
@@ -52,8 +52,6 @@ static unsigned long __initdata doc_locations[] = {
 	0xe0000, 0xe2000, 0xe4000, 0xe6000,
 	0xe8000, 0xea000, 0xec000, 0xee000,
 #endif /*  CONFIG_MTD_DOCPROBE_HIGH */
-#elif defined(__PPC__)
-	0xe4000000,
 #else
 #warning Unknown architecture for DiskOnChip. No default probe locations defined
 #endif

^ permalink raw reply related

* [RFC] (almost) booting allyesconfig -- please don't poke super-io without request_region
From: Milton Miller @ 2008-07-10 21:12 UTC (permalink / raw)
  To: linux-kernel, lm-sensors; +Cc: linuxppc-dev, Samuel Ortiz

After the following patch to mark the isa region busy and applying a few
patches[1], I was able to kexec boot into an all-yes-config kernel from 
linux-next, with the following KCONFIG_ALLCONFIG file:

# CONFIG_KALLSYMS_EXTRA_PASS is not set
# CONFIG_CRASH_DUMP is not set
CONFIG_PPC_EARLY_DEBUG_RTAS_CONSOLE=y
# CONFIG_NSC_FIR is not set
# CONFIG_SMC_IRCC_FIR is not set
# CONFIG_ALI_FIR is not set
# CONFIG_TCG_NSC is not set
# CONFIG_TCG_ATMEL is not set
# CONFIG_SENSORS_F71805F is not set
# CONFIG_SENSORS_F71882FG is not set
# CONFIG_SENSORS_F75375S is not set
# CONFIG_SENSORS_IT87 is not set
# CONFIG_SENSORS_PC87360 is not set
# CONFIG_SENSORS_PC87427 is not set
# CONFIG_SENSORS_DME1737 is not set
# CONFIG_SENSORS_SMSC47M1 is not set
# CONFIG_SENSORS_SMSC47B397 is not set
# CONFIG_SENSORS_VT1211 is not set
# CONFIG_SENSORS_W83627HF is not set
# CONFIG_SENSORS_W83627EHF is not set

While the first two might not be required, and the third is just
selecting the right platform, it would be nice to get the drivers
to play as well as the rest of the kernel.

The drivers all are for super-io chips, either irda/FIR drivers or hwmon,
and poke at isa ports without checking request_region.

It would be great if these 17 super-io drivers would check for resource
availability before poking at io ports.

[not that I expect this to merge as it, but maybe insert-resource or something]



[1] 3 section layout patches and the MTD DISKONCHIP patch,
see linuxppc-dev for full list


diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
index 2c6ded2..db54620 100644
--- a/arch/powerpc/platforms/pseries/pci.c
+++ b/arch/powerpc/platforms/pseries/pci.c
@@ -58,8 +58,10 @@ DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pcibios_name_device);
 
 static void __init pSeries_request_regions(void)
 {
-	if (!isa_io_base)
+	if (!isa_io_base) {
+		request_region(0x0,0x400,"isa");
 		return;
+		}
 
 	request_region(0x20,0x20,"pic1");
 	request_region(0xa0,0x20,"pic2");

^ permalink raw reply related

* Re: [PATCH] mpc7448: add alias list to DTS, clean out old chosen node
From: Scott Wood @ 2008-07-10 21:10 UTC (permalink / raw)
  To: Jon Loeliger; +Cc: Paul Gortmaker, linuxppc-dev
In-Reply-To: <487679B1.3040900@freescale.com>

Jon Loeliger wrote:
> Scott Wood wrote:
>> Paul Gortmaker wrote:
>>> OK, so does that mean that the cuboot wrapper is explicitly
>>> not supported for all the 83xx, 85xx, and 86xx boards?
>>
>> No (except 86xx, which doesn't have cuboot, because it never existed 
>> in arch/ppc and thus there's no compatibility to maintain), it just 
>> means that chosen was never added to those dts files, and thus cuboot 
>> has no console output.
> 
> Also note that nothing stops the causal kernel booter

As opposed to the spontaneous kernel booter? :-)

> from _starting_ with an arch/powerpc/boot/dts/ file,
> adding a node to it, and using _that_.

The "casual" user shouldn't need to make any changes, especially for the 
kernel's own dts files to work with the kernel's own wrapper.

The decision on whether to put the chosen node in the device tree should 
be based on which version of u-boot it's more important to maintain 
compatibility with (for some platforms, no chosen-duplicating version of 
u-boot supported device trees on the platform), and what the 
consequences of each form of incompatibility are (e.g. no console output 
during the bootwrapper, versus no boot arguments or initrd in the kernel).

-Scott

^ permalink raw reply

* Re: [PATCH] mpc7448: add alias list to DTS, clean out old chosen node
From: Jon Loeliger @ 2008-07-10 21:05 UTC (permalink / raw)
  To: Scott Wood; +Cc: Paul Gortmaker, linuxppc-dev
In-Reply-To: <48767636.3090906@freescale.com>

Scott Wood wrote:
> Paul Gortmaker wrote:
>> OK, so does that mean that the cuboot wrapper is explicitly
>> not supported for all the 83xx, 85xx, and 86xx boards?
> 
> No (except 86xx, which doesn't have cuboot, because it never existed in 
> arch/ppc and thus there's no compatibility to maintain), it just means 
> that chosen was never added to those dts files, and thus cuboot has no 
> console output.

Also note that nothing stops the causal kernel booter
from _starting_ with an arch/powerpc/boot/dts/ file,
adding a node to it, and using _that_.

jdl

^ permalink raw reply

* Re: powerpc/cell/cpufreq: add spu aware cpufreq governor
From: Arnd Bergmann @ 2008-07-10 21:05 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Dave Jones, Stephen Rothwell, cpufreq, Dominik Brodowski,
	Jeremy Kerr, cbe-oss-dev
In-Reply-To: <20080710180553.GB13890@isilmar.linta.de>

On Thursday 10 July 2008, Dominik Brodowski wrote:
> Actually, "cpufreq-set -g spu_governor" should be enough. (BTW, do we really
> want this name?)

Do you have a better suggestion for the name? I don't think there was a
particularly strong reason to choose it over something else.

	Arnd <><

^ permalink raw reply

* Re: [PATCH] mpc7448: add alias list to DTS, clean out old chosen node
From: Scott Wood @ 2008-07-10 20:51 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: linuxppc-dev
In-Reply-To: <48767696.6090603@windriver.com>

Paul Gortmaker wrote:
> OK, so does that mean that the cuboot wrapper is explicitly
> not supported for all the 83xx, 85xx, and 86xx boards?

No (except 86xx, which doesn't have cuboot, because it never existed in 
arch/ppc and thus there's no compatibility to maintain), it just means 
that chosen was never added to those dts files, and thus cuboot has no 
console output.

-Scott

^ permalink raw reply

* Re: [PATCH] mpc7448: add alias list to DTS, clean out old chosen node
From: Paul Gortmaker @ 2008-07-10 20:52 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <48766E4D.5060607@freescale.com>

Scott Wood wrote:
> Paul Gortmaker wrote:
>> The mpc7448hpc2 board doesn't have an alias block like
>> most of the other modern eval boards have.  We need this
>> block in order to have u-boot be able to make use of the
>> CONFIG_OF_STDOUT_VIA_ALIAS (vs. having a hard coded node)
>> in the future.
>>
>> Also remove the old, redundant chosen node.  Of all the modern
>> Freescale eval boards (incl. 83xx, 85xx, 86xx) this is the only
>> one which still has it.  Its presence also breaks with some older
>> versions of u-boot, like 1.3.1 -- which try and insert a
>> second chosen node.
>
> The chosen node is still required for output from the cuboot wrapper.

OK, so does that mean that the cuboot wrapper is explicitly
not supported for all the 83xx, 85xx, and 86xx boards?

Thanks,
Paul.

>
> -Scott

^ permalink raw reply

* Re: [PATCH] mpc7448: add alias list to DTS, clean out old chosen node
From: Scott Wood @ 2008-07-10 20:17 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: linuxppc-dev
In-Reply-To: <1215721295-17555-1-git-send-email-paul.gortmaker@windriver.com>

Paul Gortmaker wrote:
> The mpc7448hpc2 board doesn't have an alias block like
> most of the other modern eval boards have.  We need this
> block in order to have u-boot be able to make use of the
> CONFIG_OF_STDOUT_VIA_ALIAS (vs. having a hard coded node)
> in the future.
> 
> Also remove the old, redundant chosen node.  Of all the modern
> Freescale eval boards (incl. 83xx, 85xx, 86xx) this is the only
> one which still has it.  Its presence also breaks with some older
> versions of u-boot, like 1.3.1 -- which try and insert a
> second chosen node.

The chosen node is still required for output from the cuboot wrapper.

-Scott

^ permalink raw reply

* [PATCH] mpc7448: add alias list to DTS, clean out old chosen node
From: Paul Gortmaker @ 2008-07-10 20:21 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Paul Gortmaker

The mpc7448hpc2 board doesn't have an alias block like
most of the other modern eval boards have.  We need this
block in order to have u-boot be able to make use of the
CONFIG_OF_STDOUT_VIA_ALIAS (vs. having a hard coded node)
in the future.

Also remove the old, redundant chosen node.  Of all the modern
Freescale eval boards (incl. 83xx, 85xx, 86xx) this is the only
one which still has it.  Its presence also breaks with some older
versions of u-boot, like 1.3.1 -- which try and insert a
second chosen node.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 arch/powerpc/boot/dts/mpc7448hpc2.dts |   24 +++++++++++++++---------
 1 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc7448hpc2.dts b/arch/powerpc/boot/dts/mpc7448hpc2.dts
index 705c23c..2544f3e 100644
--- a/arch/powerpc/boot/dts/mpc7448hpc2.dts
+++ b/arch/powerpc/boot/dts/mpc7448hpc2.dts
@@ -18,6 +18,16 @@
 	#address-cells = <1>;
 	#size-cells = <1>;
 
+	aliases {
+		ethernet0 = &enet0;
+		ethernet1 = &enet1;
+
+		serial0 = &serial0;
+		serial1 = &serial1;
+
+		pci0 = &pci0;
+	};
+
 	cpus {
 		#address-cells = <1>;
 		#size-cells =<0>;
@@ -78,7 +88,7 @@
 
 		};
 
-		ethernet@6200 {
+		enet0: ethernet@6200 {
 			linux,network-index = <0>;
 			#size-cells = <0>;
 			device_type = "network";
@@ -91,7 +101,7 @@
 			phy-handle = <&phy8>;
 		};
 
-		ethernet@6600 {
+		enet1: ethernet@6600 {
 			linux,network-index = <1>;
 			#address-cells = <1>;
 			#size-cells = <0>;
@@ -105,7 +115,7 @@
 			phy-handle = <&phy9>;
 		};
 
-		serial@7808 {
+		serial0: serial@7808 {
 			device_type = "serial";
 			compatible = "ns16550";
 			reg = <0x7808 0x200>;
@@ -114,7 +124,7 @@
 			interrupt-parent = <&mpic>;
 		};
 
-		serial@7c08 {
+		serial1: serial@7c08 {
 			device_type = "serial";
 			compatible = "ns16550";
 			reg = <0x7c08 0x200>;
@@ -131,7 +141,7 @@
 			compatible = "chrp,open-pic";
 			device_type = "open-pic";
 		};
-		pci@1000 {
+		pci0: pci@1000 {
 			compatible = "tsi108-pci";
 			device_type = "pci";
 			#interrupt-cells = <1>;
@@ -184,8 +194,4 @@
 			};
 		};
 	};
-	chosen {
-		linux,stdout-path = "/tsi108@c0000000/serial@7808";
-	};
-
 };
-- 
1.5.6.2

^ permalink raw reply related

* [PATCH] [V2] powerpc: Xilinx: PS2: driver updates based on review
From: John Linn @ 2008-07-10 19:34 UTC (permalink / raw)
  To: linuxppc-dev, linux-input; +Cc: dmitry.torokhov, Sadanand, John Linn

Review comments were incorporated to improve the driver.

1. Some data was eliminated that was not needed.
2. Renaming of variables for clarity.
3. Removed unneeded type casting.
4. Changed to use dev_err rather than other I/O.
5. Merged together some functions.
6. Added kerneldoc format to functions.

Signed-off-by: Sadanand <sadanan@xilinx.com>
Signed-off-by: John Linn <john.linn@xilinx.com>
---

This patch is an incremental patch to be applied to 
[V3] powerpc: Xilinx: PS2: Added new XPS PS2 driver.

V2
	Updated based on Peter's comments.

 drivers/input/serio/xilinx_ps2.c |  207 ++++++++++++++++++++------------------
 1 files changed, 111 insertions(+), 96 deletions(-)

diff --git a/drivers/input/serio/xilinx_ps2.c b/drivers/input/serio/xilinx_ps2.c
index e86f11b..86ec91c 100644
--- a/drivers/input/serio/xilinx_ps2.c
+++ b/drivers/input/serio/xilinx_ps2.c
@@ -58,23 +58,20 @@
 
 /* Mask for all the Receive Interrupts */
 #define XPS2_IPIXR_RX_ALL	(XPS2_IPIXR_RX_OVF | XPS2_IPIXR_RX_ERR |  \
-					XPS2_IPIXR_RX_FULL)
+				 XPS2_IPIXR_RX_FULL)
 
 /* Mask for all the Interrupts */
 #define XPS2_IPIXR_ALL		(XPS2_IPIXR_TX_ALL | XPS2_IPIXR_RX_ALL |  \
-					XPS2_IPIXR_WDT_TOUT)
+				 XPS2_IPIXR_WDT_TOUT)
 
 /* Global Interrupt Enable mask */
 #define XPS2_GIER_GIE_MASK	0x80000000
 
 struct xps2data {
 	int irq;
-	u32 phys_addr;
-	u32 remap_size;
 	spinlock_t lock;
-	u8 rxb;				/* Rx buffer */
 	void __iomem *base_address;	/* virt. address of control registers */
-	unsigned int dfl;
+	unsigned int flags;
 	struct serio serio;		/* serio */
 };
 
@@ -82,8 +79,13 @@ struct xps2data {
 /* XPS PS/2 data transmission calls */
 /************************************/
 
-/*
- * xps2_recv() will attempt to receive a byte of data from the PS/2 port.
+/**
+ * xps2_recv() - attempts to receive a byte from the PS/2 port.
+ * @drvdata:	pointer to ps2 device private data structure
+ * @byte:	address where the read data will be copied
+ *
+ * If there is any data available in the PS/2 receiver, this functions reads
+ * the data, otherwise it returns error.
  */
 static int xps2_recv(struct xps2data *drvdata, u8 *byte)
 {
@@ -105,7 +107,7 @@ static int xps2_recv(struct xps2data *drvdata, u8 *byte)
 /*********************/
 static irqreturn_t xps2_interrupt(int irq, void *dev_id)
 {
-	struct xps2data *drvdata = (struct xps2data *)dev_id;
+	struct xps2data *drvdata = dev_id;
 	u32 intr_sr;
 	u8 c;
 	int status;
@@ -115,35 +117,28 @@ static irqreturn_t xps2_interrupt(int irq, void *dev_id)
 	out_be32(drvdata->base_address + XPS2_IPISR_OFFSET, intr_sr);
 
 	/* Check which interrupt is active */
-	if (intr_sr & XPS2_IPIXR_RX_OVF) {
-		printk(KERN_ERR "%s: receive overrun error\n",
-			drvdata->serio.name);
-	}
+	if (intr_sr & XPS2_IPIXR_RX_OVF)
+		dev_err(drvdata->serio.dev.parent, "receive overrun error\n");
 
 	if (intr_sr & XPS2_IPIXR_RX_ERR)
-		drvdata->dfl |= SERIO_PARITY;
+		drvdata->flags |= SERIO_PARITY;
 
 	if (intr_sr & (XPS2_IPIXR_TX_NOACK | XPS2_IPIXR_WDT_TOUT))
-		drvdata->dfl |= SERIO_TIMEOUT;
+		drvdata->flags |= SERIO_TIMEOUT;
 
 	if (intr_sr & XPS2_IPIXR_RX_FULL) {
-		status = xps2_recv(drvdata, &drvdata->rxb);
+		status = xps2_recv(drvdata, &c);
 
 		/* Error, if a byte is not received */
 		if (status) {
-			printk(KERN_ERR
-				"%s: wrong rcvd byte count (%d)\n",
-				drvdata->serio.name, status);
+			dev_err(drvdata->serio.dev.parent,
+				"wrong rcvd byte count (%d)\n", status);
 		} else {
-			c = drvdata->rxb;
-			serio_interrupt(&drvdata->serio, c, drvdata->dfl);
-			drvdata->dfl = 0;
+			serio_interrupt(&drvdata->serio, c, drvdata->flags);
+			drvdata->flags = 0;
 		}
 	}
 
-	if (intr_sr & XPS2_IPIXR_TX_ACK)
-		drvdata->dfl = 0;
-
 	return IRQ_HANDLED;
 }
 
@@ -151,8 +146,15 @@ static irqreturn_t xps2_interrupt(int irq, void *dev_id)
 /* serio callbacks */
 /*******************/
 
-/*
- * sxps2_write() sends a byte out through the PS/2 interface.
+/**
+ * sxps2_write() - sends a byte out through the PS/2 port.
+ * @pserio:	pointer to the serio structure of the PS/2 port
+ * @c:		data that needs to be written to the PS/2 port
+ *
+ * This function checks if the PS/2 transmitter is empty and sends a byte.
+ * Otherwise it returns error. Transmission fails only when nothing is connected
+ * to the PS/2 port. Thats why, we do not try to resend the data in case of a
+ * failure.
  */
 static int sxps2_write(struct serio *pserio, unsigned char c)
 {
@@ -173,33 +175,39 @@ static int sxps2_write(struct serio *pserio, unsigned char c)
 	return status;
 }
 
-/*
- * sxps2_open() is called when a port is open by the higher layer.
+/**
+ * sxps2_open() - called when a port is opened by the higher layer.
+ * @pserio:	pointer to the serio structure of the PS/2 device
+ *
+ * This function requests irq and enables interrupts for the PS/2 device.
  */
 static int sxps2_open(struct serio *pserio)
 {
 	struct xps2data *drvdata = pserio->port_data;
 	int retval;
+	u8 c;
 
 	retval = request_irq(drvdata->irq, &xps2_interrupt, 0,
 				DRIVER_NAME, drvdata);
 	if (retval) {
-		printk(KERN_ERR
-			"%s: Couldn't allocate interrupt %d\n",
-			drvdata->serio.name, drvdata->irq);
+		dev_err(drvdata->serio.dev.parent,
+			"Couldn't allocate interrupt %d\n", drvdata->irq);
 		return retval;
 	}
 
 	/* start reception by enabling the interrupts */
 	out_be32(drvdata->base_address + XPS2_GIER_OFFSET, XPS2_GIER_GIE_MASK);
 	out_be32(drvdata->base_address + XPS2_IPIER_OFFSET, XPS2_IPIXR_RX_ALL);
-	(void)xps2_recv(drvdata, &drvdata->rxb);
+	(void)xps2_recv(drvdata, &c);
 
 	return 0;		/* success */
 }
 
-/*
- * sxps2_close() frees the interrupt.
+/**
+ * sxps2_close() - frees the interrupt.
+ * @pserio:	pointer to the serio structure of the PS/2 device
+ *
+ * This function frees the irq and disables interrupts for the PS/2 device.
  */
 static void sxps2_close(struct serio *pserio)
 {
@@ -211,21 +219,48 @@ static void sxps2_close(struct serio *pserio)
 	free_irq(drvdata->irq, drvdata);
 }
 
-/*********************/
-/* Device setup code */
-/*********************/
-
-static int xps2_setup(struct device *dev, struct resource *regs_res,
-		      struct resource *irq_res)
+/**
+ * xps2_of_probe - probe method for the PS/2 device.
+ * @of_dev:	pointer to OF device structure
+ * @match:	pointer to the stucture used for matching a device
+ *
+ * This function probes the PS/2 device in the device tree.
+ * It initializes the driver data structure and the hardware.
+ * It returns 0, if the driver is bound to the PS/2 device, or a negative
+ * value if there is an error.
+ */
+static int __devinit xps2_of_probe(struct of_device *ofdev, const struct
+				   of_device_id * match)
 {
+	struct resource r_irq; /* Interrupt resources */
+	struct resource r_mem; /* IO mem resources */
 	struct xps2data *drvdata;
 	struct serio *serio;
-	unsigned long remap_size;
-	int retval;
+	struct device *dev;
+	resource_size_t remap_size, phys_addr;
+	int rc = 0;
 
+	dev = &ofdev->dev;
 	if (!dev)
 		return -EINVAL;
 
+	dev_info(dev, "Device Tree Probing \'%s\'\n",
+			ofdev->node->name);
+
+	/* Get iospace for the device */
+	rc = of_address_to_resource(ofdev->node, 0, &r_mem);
+	if (rc) {
+		dev_err(dev, "invalid address\n");
+		return rc;
+	}
+
+	/* Get IRQ for the device */
+	rc = of_irq_to_resource(ofdev->node, 0, &r_irq);
+	if (rc == NO_IRQ) {
+		dev_err(dev, "no IRQ found\n");
+		return rc;
+	}
+
 	drvdata = kzalloc(sizeof(struct xps2data), GFP_KERNEL);
 	if (!drvdata) {
 		dev_err(dev, "Couldn't allocate device private record\n");
@@ -234,31 +269,24 @@ static int xps2_setup(struct device *dev, struct resource *regs_res,
 	spin_lock_init(&drvdata->lock);
 	dev_set_drvdata(dev, drvdata);
 
-	if (!regs_res || !irq_res) {
-		dev_err(dev, "IO resource(s) not found\n");
-		retval = -EFAULT;
-		goto failed1;
-	}
-
-	drvdata->irq = irq_res->start;
-	remap_size = regs_res->end - regs_res->start + 1;
-	if (!request_mem_region(regs_res->start, remap_size, DRIVER_NAME)) {
+	drvdata->irq = r_irq.start;
+	phys_addr = r_mem.start;
+	remap_size = r_mem.end - r_mem.start + 1;
+	if (!request_mem_region(phys_addr, remap_size, DRIVER_NAME)) {
 
 		dev_err(dev, "Couldn't lock memory region at 0x%08X\n",
-			(unsigned int)regs_res->start);
-		retval = -EBUSY;
+			(unsigned int)phys_addr);
+		rc = -EBUSY;
 		goto failed1;
 	}
 
 	/* Fill in configuration data and add them to the list */
-	drvdata->phys_addr = regs_res->start;
-	drvdata->remap_size = remap_size;
-	drvdata->base_address = ioremap(regs_res->start, remap_size);
+	drvdata->base_address = ioremap(phys_addr, remap_size);
 	if (drvdata->base_address == NULL) {
 
 		dev_err(dev, "Couldn't ioremap memory at 0x%08X\n",
-			(unsigned int)regs_res->start);
-		retval = -EFAULT;
+			(unsigned int)phys_addr);
+		rc = -EFAULT;
 		goto failed2;
 	}
 
@@ -270,7 +298,8 @@ static int xps2_setup(struct device *dev, struct resource *regs_res,
 	out_be32(drvdata->base_address + XPS2_SRST_OFFSET, XPS2_SRST_RESET);
 
 	dev_info(dev, "Xilinx PS2 at 0x%08X mapped to 0x%08X, irq=%d\n",
-		drvdata->phys_addr, (u32)drvdata->base_address, drvdata->irq);
+		 (unsigned int)phys_addr, (u32)drvdata->base_address,
+		 drvdata->irq);
 
 	serio = &drvdata->serio;
 	serio->id.type = SERIO_8042;
@@ -280,58 +309,37 @@ static int xps2_setup(struct device *dev, struct resource *regs_res,
 	serio->port_data = drvdata;
 	serio->dev.parent = dev;
 	snprintf(drvdata->serio.name, sizeof(serio->name),
-		 "Xilinx XPS PS/2 at %08X", drvdata->phys_addr);
+		 "Xilinx XPS PS/2 at %08X", (unsigned int)phys_addr);
 	snprintf(drvdata->serio.phys, sizeof(serio->phys),
-		 "xilinxps2/serio at %08X", drvdata->phys_addr);
+		 "xilinxps2/serio at %08X", (unsigned int)phys_addr);
 	serio_register_port(serio);
 
 	return 0;		/* success */
 
 failed2:
-	release_mem_region(regs_res->start, remap_size);
+	release_mem_region(phys_addr, remap_size);
 
 failed1:
 	kfree(drvdata);
 	dev_set_drvdata(dev, NULL);
 
-	return retval;
-}
-
-/***************************/
-/* OF Platform Bus Support */
-/***************************/
-
-static int __devinit xps2_of_probe(struct of_device *ofdev, const struct
-				   of_device_id * match)
-{
-	struct resource r_irq; /* Interrupt resources */
-	struct resource r_mem; /* IO mem resources */
-	int rc = 0;
-
-	printk(KERN_INFO "Device Tree Probing \'%s\'\n",
-			ofdev->node->name);
-
-	/* Get iospace for the device */
-	rc = of_address_to_resource(ofdev->node, 0, &r_mem);
-	if (rc) {
-		dev_err(&ofdev->dev, "invalid address\n");
-		return rc;
-	}
-
-	/* Get IRQ for the device */
-	rc = of_irq_to_resource(ofdev->node, 0, &r_irq);
-	if (rc == NO_IRQ) {
-		dev_err(&ofdev->dev, "no IRQ found\n");
-		return rc;
-	}
-
-	return xps2_setup(&ofdev->dev, &r_mem, &r_irq);
+	return rc;
 }
 
+/**
+ * xps2_of_remove - unbinds the driver from the PS/2 device.
+ * @of_dev:	pointer to OF device structure
+ *
+ * This function is called if a device is physically removed from the system or
+ * if the driver module is being unloaded. It frees any resources allocated to
+ * the device.
+ */
 static int __devexit xps2_of_remove(struct of_device *of_dev)
 {
 	struct xps2data *drvdata;
 	struct device *dev;
+	struct resource r_mem; /* IO mem resources */
+	int rc;
 
 	dev = &of_dev->dev;
 	if (!dev)
@@ -343,7 +351,14 @@ static int __devexit xps2_of_remove(struct of_device *of_dev)
 
 	iounmap(drvdata->base_address);
 
-	release_mem_region(drvdata->phys_addr, drvdata->remap_size);
+	/* Get iospace of the device */
+	rc = of_address_to_resource(of_dev->node, 0, &r_mem);
+	if (rc) {
+		dev_err(dev, "invalid address\n");
+		return rc;
+	}
+
+	release_mem_region(r_mem.start, r_mem.end - r_mem.start + 1);
 
 	kfree(drvdata);
 	dev_set_drvdata(dev, NULL);
-- 
1.5.2.1



This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

^ permalink raw reply related

* Re: linux-next: manual merge of the powerpc tree
From: Bartlomiej Zolnierkiewicz @ 2008-07-11 19:01 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Daniel Walker, linuxppc-dev, linux-next, Paul Mackerras,
	Andrew Morton
In-Reply-To: <20080707230717.10429faf.sfr@canb.auug.org.au>


Hi,

On Monday 07 July 2008, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the powerpc tree got a conflict in
> drivers/macintosh/mediabay.c between commit
> 7ad963b103d3863b1161c59f3e65a435979804ed ("ide-pmac: media-bay support
> fixes (take 4)") from the ide tree and commit
> 9a24729d8aeef967eac7af71c6a69edc83d06558 ("macintosh/media bay: Convert
> semaphore to mutex") from the powerpc tree.

Since I haven't heard back from Ben [1] on ide-pmac/media-bay IRQ issue
I took another look at ide-pmac patches and I think that it should be
possible to rework them in such way that consecutive ide patches (> 100)
won't depend on "ide-pmac: media-bay support fixes (take 4)" patch.

This would allow us to re-schedule it to 2.6.28 (which is probably what
we want because 2.6.26 is probably just around the corner and we will be
pretty busy with 2.6.27 merge window soon).  Ben, what's your opinion?

[1] which doesn't surprise me given his new responsibilities ;)

Thanks,
Bart

^ permalink raw reply

* Re: [PATCH] powerpc: Xilinx: PS2: driver updates based on review
From: Peter Korsgaard @ 2008-07-10 18:27 UTC (permalink / raw)
  To: John Linn; +Cc: Sadanand, dmitry.torokhov, linuxppc-dev, linux-input
In-Reply-To: <20080710154010.7358F1BA004D@mail124-sin.bigfish.com>

>>>>> "John" == John Linn <john.linn@xilinx.com> writes:

 > Review comments were incorporated to improve the driver.
 > 1. Some data was eliminated that was not needed.
 > 2. Renaming of variables for clarity.
 > 3. Removed unneeded type casting.
 > 4. Changed to use dev_err rather than other I/O.
 > 5. Merged together some functions.
 > 6. Added kerneldoc format to functions.

 > Signed-off-by: Sadanand <sadanan@xilinx.com>
 > Signed-off-by: John Linn <john.linn@xilinx.com>

If the minor issues below gets fixed:

Acked-by: Peter Korsgaard <jacmet@sunsite.dk>


 > This patch is an incremental patch to be applied to 
 > [V3] powerpc: Xilinx: PS2: Added new XPS PS2 driver.

 >  drivers/input/serio/xilinx_ps2.c |  220 +++++++++++++++++++++----------------
 >  1 files changed, 125 insertions(+), 95 deletions(-)

 > diff --git a/drivers/input/serio/xilinx_ps2.c b/drivers/input/serio/xilinx_ps2.c
 > index e86f11b..eba46c7 100644
 > --- a/drivers/input/serio/xilinx_ps2.c
 > +++ b/drivers/input/serio/xilinx_ps2.c
 > @@ -58,23 +58,20 @@
 
 >  /* Mask for all the Receive Interrupts */
 >  #define XPS2_IPIXR_RX_ALL	(XPS2_IPIXR_RX_OVF | XPS2_IPIXR_RX_ERR |  \
 > -					XPS2_IPIXR_RX_FULL)
 > +				 XPS2_IPIXR_RX_FULL)
 
 >  /* Mask for all the Interrupts */
 >  #define XPS2_IPIXR_ALL		(XPS2_IPIXR_TX_ALL | XPS2_IPIXR_RX_ALL |  \
 > -					XPS2_IPIXR_WDT_TOUT)
 > +				 XPS2_IPIXR_WDT_TOUT)
 
 >  /* Global Interrupt Enable mask */
 >  #define XPS2_GIER_GIE_MASK	0x80000000
 
 >  struct xps2data {
 >  	int irq;
 > -	u32 phys_addr;
 > -	u32 remap_size;
 >  	spinlock_t lock;
 > -	u8 rxb;				/* Rx buffer */
 >  	void __iomem *base_address;	/* virt. address of control registers */
 > -	unsigned int dfl;
 > +	unsigned int flags;
 >  	struct serio serio;		/* serio */
 >  };
 
 > @@ -82,8 +79,13 @@ struct xps2data {
 >  /* XPS PS/2 data transmission calls */
 >  /************************************/
 
 > -/*
 > - * xps2_recv() will attempt to receive a byte of data from the PS/2 port.
 > +/**
 > + * xps2_recv() - attempts to receive a byte from the PS/2 port.
 > + * @drvdata:	pointer to ps2 device private data structure
 > + * @byte:	address where the read data will be copied
 > + *
 > + * If there is any data available in the PS/2 receiver, this functions reads
 > + * the data, otherwise it returns error.
 >   */
 >  static int xps2_recv(struct xps2data *drvdata, u8 *byte)
 >  {
 > @@ -105,7 +107,7 @@ static int xps2_recv(struct xps2data *drvdata, u8 *byte)
 >  /*********************/
 >  static irqreturn_t xps2_interrupt(int irq, void *dev_id)
 >  {
 > -	struct xps2data *drvdata = (struct xps2data *)dev_id;
 > +	struct xps2data *drvdata = dev_id;
 >  	u32 intr_sr;
 >  	u8 c;
 >  	int status;
 > @@ -115,35 +117,28 @@ static irqreturn_t xps2_interrupt(int irq, void *dev_id)
 >  	out_be32(drvdata->base_address + XPS2_IPISR_OFFSET, intr_sr);
 
 >  	/* Check which interrupt is active */
 > -	if (intr_sr & XPS2_IPIXR_RX_OVF) {
 > -		printk(KERN_ERR "%s: receive overrun error\n",
 > -			drvdata->serio.name);
 > -	}
 > +	if (intr_sr & XPS2_IPIXR_RX_OVF)
 > +		dev_err(drvdata->serio.dev.parent, "receive overrun error\n");
 
 >  	if (intr_sr & XPS2_IPIXR_RX_ERR)
 > -		drvdata->dfl |= SERIO_PARITY;
 > +		drvdata->flags |= SERIO_PARITY;
 
 >  	if (intr_sr & (XPS2_IPIXR_TX_NOACK | XPS2_IPIXR_WDT_TOUT))
 > -		drvdata->dfl |= SERIO_TIMEOUT;
 > +		drvdata->flags |= SERIO_TIMEOUT;
 
 >  	if (intr_sr & XPS2_IPIXR_RX_FULL) {
 > -		status = xps2_recv(drvdata, &drvdata->rxb);
 > +		status = xps2_recv(drvdata, &c);
 
 >  		/* Error, if a byte is not received */
 >  		if (status) {
 > -			printk(KERN_ERR
 > -				"%s: wrong rcvd byte count (%d)\n",
 > -				drvdata->serio.name, status);
 > +			dev_err(drvdata->serio.dev.parent,
 > +				"wrong rcvd byte count\n");

You used to print the byte count - Isn't that interesting debugging
data?


 >  		} else {
 > -			c = drvdata->rxb;
 > -			serio_interrupt(&drvdata->serio, c, drvdata->dfl);
 > -			drvdata->dfl = 0;
 > +			serio_interrupt(&drvdata->serio, c, drvdata->flags);
 > +			drvdata->flags = 0;
 >  		}
 >  	}
 
 > -	if (intr_sr & XPS2_IPIXR_TX_ACK)
 > -		drvdata->dfl = 0;
 > -
 >  	return IRQ_HANDLED;
 >  }
 
 > @@ -151,8 +146,15 @@ static irqreturn_t xps2_interrupt(int irq, void *dev_id)
 >  /* serio callbacks */
 >  /*******************/
 
 > -/*
 > - * sxps2_write() sends a byte out through the PS/2 interface.
 > +/**
 > + * sxps2_write() - sends a byte out through the PS/2 port.
 > + * @pserio:	pointer to the serio structure of the PS/2 port
 > + * @c:		data that needs to be written to the PS/2 port
 > + *
 > + * This fucntion checks if the PS/2 transmitter is empty and sends a byte.


s/fucntion/function/


 > + * Otherwise it returns error. Transmission fails only when nothing is connected
 > + * to the PS/2 port. Thats why, we do not try to resend the data in case of a
 > + * failure.
 >   */
 >  static int sxps2_write(struct serio *pserio, unsigned char c)
 >  {
 > @@ -173,33 +175,39 @@ static int sxps2_write(struct serio *pserio, unsigned char c)
 >  	return status;
 >  }
 
 > -/*
 > - * sxps2_open() is called when a port is open by the higher layer.
 > +/**
 > + * sxps2_open() - called when a port is opened by the higher layer.
 > + * @pserio:	pointer to the serio structure of the PS/2 device
 > + *
 > + * This function requests irq and enables interrupts for the PS/2 device.
 >   */
 >  static int sxps2_open(struct serio *pserio)
 >  {
 >  	struct xps2data *drvdata = pserio->port_data;
 >  	int retval;
 > +	u8 c;
 
 >  	retval = request_irq(drvdata->irq, &xps2_interrupt, 0,
 >  				DRIVER_NAME, drvdata);
 >  	if (retval) {
 > -		printk(KERN_ERR
 > -			"%s: Couldn't allocate interrupt %d\n",
 > -			drvdata->serio.name, drvdata->irq);
 > +		dev_err(drvdata->serio.dev.parent,
 > +			"Couldn't allocate interrupt %d\n", drvdata->irq);
 >  		return retval;
 >  	}
 
 >  	/* start reception by enabling the interrupts */
 >  	out_be32(drvdata->base_address + XPS2_GIER_OFFSET, XPS2_GIER_GIE_MASK);
 >  	out_be32(drvdata->base_address + XPS2_IPIER_OFFSET, XPS2_IPIXR_RX_ALL);
 > -	(void)xps2_recv(drvdata, &drvdata->rxb);
 > +	(void)xps2_recv(drvdata, &c);
 
 >  	return 0;		/* success */
 >  }
 
 > -/*
 > - * sxps2_close() frees the interrupt.
 > +/**
 > + * sxps2_close() - frees the interrupt.
 > + * @pserio:	pointer to the serio structure of the PS/2 device
 > + *
 > + * This function frees the irq and disables interrupts for the PS/2 device.
 >   */
 >  static void sxps2_close(struct serio *pserio)
 >  {
 > @@ -211,21 +219,60 @@ static void sxps2_close(struct serio *pserio)
 >  	free_irq(drvdata->irq, drvdata);
 >  }
 
 > -/*********************/
 > -/* Device setup code */
 > -/*********************/
 > +/***************************/
 > +/* OF Platform Bus Support */
 > +/***************************/
 
 > -static int xps2_setup(struct device *dev, struct resource *regs_res,
 > -		      struct resource *irq_res)
 > +/**
 > + * xps2_of_probe - probe method for the PS/2 device.
 > + * @of_dev:	pointer to OF device structure
 > + * @match:	pointer to the stucture used for matching a device
 > + *
 > + * This function probes for a matching PS/2 device using OF properties and
 > + * attaches it to the platform bus. It initializes the driver data structure
 > + * and also initializes the hardware.
 > + * It returns 0, if the driver is bound to the PS/2 device, or a negative value
 > + * if there is an error. It releases all the allocated resources in case of an
 > + * error.

The function doesn't have anything to do with the platform bus, and
the comment is imho more verbose than needed.


 > + *
 > + */
 > +static int __devinit xps2_of_probe(struct of_device *ofdev, const struct
 > +				   of_device_id * match)
 >  {
 > +	struct resource r_irq; /* Interrupt resources */
 > +	struct resource r_mem; /* IO mem resources */
 >  	struct xps2data *drvdata;
 >  	struct serio *serio;
 > -	unsigned long remap_size;
 > -	int retval;
 > +	struct device *dev;
 > +	resource_size_t remap_size, phys_addr;
 > +	int rc = 0;
 
 > +	dev = &ofdev->dev;
 >  	if (!dev)
 >  		return -EINVAL;
 
 > +	dev_info(dev, "Device Tree Probing \'%s\'\n",
 > +			ofdev->node->name);
 > +
 > +	/* Get iospace for the device */
 > +	rc = of_address_to_resource(ofdev->node, 0, &r_mem);
 > +	if (rc) {
 > +		dev_err(dev, "invalid address\n");
 > +		return rc;
 > +	}
 > +
 > +	/* Get IRQ for the device */
 > +	rc = of_irq_to_resource(ofdev->node, 0, &r_irq);
 > +	if (rc == NO_IRQ) {
 > +		dev_err(dev, "no IRQ found\n");
 > +		return rc;
 > +	}
 > +
 > +	if (!(&r_mem) || !(&r_irq)) {
 > +		dev_err(dev, "IO resource(s) not found\n");
 > +		return -EFAULT;
 > +	}

How can this ever fail? They are local structures, not pointers

 > +
 >  	drvdata = kzalloc(sizeof(struct xps2data), GFP_KERNEL);
 >  	if (!drvdata) {
 >  		dev_err(dev, "Couldn't allocate device private record\n");
 > @@ -234,31 +281,24 @@ static int xps2_setup(struct device *dev, struct resource *regs_res,
 >  	spin_lock_init(&drvdata->lock);
 >  	dev_set_drvdata(dev, drvdata);
 
 > -	if (!regs_res || !irq_res) {
 > -		dev_err(dev, "IO resource(s) not found\n");
 > -		retval = -EFAULT;
 > -		goto failed1;
 > -	}
 > -
 > -	drvdata->irq = irq_res->start;
 > -	remap_size = regs_res->end - regs_res->start + 1;
 > -	if (!request_mem_region(regs_res->start, remap_size, DRIVER_NAME)) {
 > +	drvdata->irq = r_irq.start;
 > +	phys_addr = r_mem.start;
 > +	remap_size = r_mem.end - r_mem.start + 1;
 > +	if (!request_mem_region(phys_addr, remap_size, DRIVER_NAME)) {
 
 >  		dev_err(dev, "Couldn't lock memory region at 0x%08X\n",
 > -			(unsigned int)regs_res->start);
 > -		retval = -EBUSY;
 > +			(unsigned int)phys_addr);
 > +		rc = -EBUSY;
 >  		goto failed1;
 >  	}
 
 >  	/* Fill in configuration data and add them to the list */
 > -	drvdata->phys_addr = regs_res->start;
 > -	drvdata->remap_size = remap_size;
 > -	drvdata->base_address = ioremap(regs_res->start, remap_size);
 > +	drvdata->base_address = ioremap(phys_addr, remap_size);
 >  	if (drvdata->base_address == NULL) {
 
 >  		dev_err(dev, "Couldn't ioremap memory at 0x%08X\n",
 > -			(unsigned int)regs_res->start);
 > -		retval = -EFAULT;
 > +			(unsigned int)phys_addr);
 > +		rc = -EFAULT;
 >  		goto failed2;
 >  	}
 
 > @@ -270,7 +310,8 @@ static int xps2_setup(struct device *dev, struct resource *regs_res,
 >  	out_be32(drvdata->base_address + XPS2_SRST_OFFSET, XPS2_SRST_RESET);
 
 >  	dev_info(dev, "Xilinx PS2 at 0x%08X mapped to 0x%08X, irq=%d\n",
 > -		drvdata->phys_addr, (u32)drvdata->base_address, drvdata->irq);
 > +		 (unsigned int)phys_addr, (u32)drvdata->base_address,
 > +		 drvdata->irq);
 
 >  	serio = &drvdata->serio;
 serio-> id.type = SERIO_8042;
 > @@ -280,58 +321,38 @@ static int xps2_setup(struct device *dev, struct resource *regs_res,
 serio-> port_data = drvdata;
 serio-> dev.parent = dev;
 >  	snprintf(drvdata->serio.name, sizeof(serio->name),
 > -		 "Xilinx XPS PS/2 at %08X", drvdata->phys_addr);
 > +		 "Xilinx XPS PS/2 at %08X", (unsigned int)phys_addr);
 >  	snprintf(drvdata->serio.phys, sizeof(serio->phys),
 > -		 "xilinxps2/serio at %08X", drvdata->phys_addr);
 > +		 "xilinxps2/serio at %08X", (unsigned int)phys_addr);
 >  	serio_register_port(serio);
 
 >  	return 0;		/* success */
 
 >  failed2:
 > -	release_mem_region(regs_res->start, remap_size);
 > +	release_mem_region(phys_addr, remap_size);
 
 >  failed1:
 >  	kfree(drvdata);
 >  	dev_set_drvdata(dev, NULL);
 
 > -	return retval;
 > -}
 > -
 > -/***************************/
 > -/* OF Platform Bus Support */
 > -/***************************/
 > -
 > -static int __devinit xps2_of_probe(struct of_device *ofdev, const struct
 > -				   of_device_id * match)
 > -{
 > -	struct resource r_irq; /* Interrupt resources */
 > -	struct resource r_mem; /* IO mem resources */
 > -	int rc = 0;
 > -
 > -	printk(KERN_INFO "Device Tree Probing \'%s\'\n",
 > -			ofdev->node->name);
 > -
 > -	/* Get iospace for the device */
 > -	rc = of_address_to_resource(ofdev->node, 0, &r_mem);
 > -	if (rc) {
 > -		dev_err(&ofdev->dev, "invalid address\n");
 > -		return rc;
 > -	}
 > -
 > -	/* Get IRQ for the device */
 > -	rc = of_irq_to_resource(ofdev->node, 0, &r_irq);
 > -	if (rc == NO_IRQ) {
 > -		dev_err(&ofdev->dev, "no IRQ found\n");
 > -		return rc;
 > -	}
 > -
 > -	return xps2_setup(&ofdev->dev, &r_mem, &r_irq);
 > +	return rc;
 >  }
 
 > +/**
 > + * xps2_of_remove - unbinds the driver from the PS/2 device.
 > + * @of_dev:	pointer to OF device structure
 > + *
 > + * This function is called if a device is physically removed from the system or
 > + * if the driver module is being unloaded. It checks if the device is present

Where's that check?


 > + * and frees any resources allocated to the device.
 > + */
 >  static int __devexit xps2_of_remove(struct of_device *of_dev)
 >  {
 >  	struct xps2data *drvdata;
 >  	struct device *dev;
 > +	struct resource r_mem; /* IO mem resources */
 > +	resource_size_t phys_addr, remap_size;
 > +	int rc;
 
 >  	dev = &of_dev->dev;
 >  	if (!dev)
 > @@ -343,7 +364,16 @@ static int __devexit xps2_of_remove(struct of_device *of_dev)
 
 >  	iounmap(drvdata->base_address);
 
 > -	release_mem_region(drvdata->phys_addr, drvdata->remap_size);
 > +	/* Get iospace of the device */
 > +	rc = of_address_to_resource(of_dev->node, 0, &r_mem);
 > +	if (rc) {
 > +		dev_err(dev, "invalid address\n");
 > +		return rc;
 > +	}
 > +
 > +	phys_addr = r_mem.start;
 > +	remap_size = r_mem.end - r_mem.start + 1;
 > +	release_mem_region(phys_addr, remap_size);


You only use phys_addr / remap_size once - just use r_mem directly instead.


 >  	kfree(drvdata);
 >  	dev_set_drvdata(dev, NULL);
 > -- 
 > 1.5.2.1

-- 
Bye, Peter Korsgaard

^ permalink raw reply

* Re: powerpc/cell/cpufreq: add spu aware cpufreq governor
From: Dominik Brodowski @ 2008-07-10 18:05 UTC (permalink / raw)
  To: Dave Jones
  Cc: Stephen Rothwell, Arnd Bergmann, linuxppc-dev, Jeremy Kerr,
	cpufreq, cbe-oss-dev
In-Reply-To: <20080708152728.GF4997@codemonkey.org.uk>

On Tue, Jul 08, 2008 at 11:27:28AM -0400, Dave Jones wrote:
> On Tue, Jul 08, 2008 at 08:43:43AM +0200, Arnd Bergmann wrote:
>  > On Monday 07 July 2008, Dave Jones wrote:
>  > > One question I do have though, is how userspace scripts are supposed
>  > > to know they're to echo cbe_spu_governor into the relevant parts of
>  > > sysfs.  I've not used anything with a cell. Do they expose the SPUs
>  > > as regular CPUs, or do they show up in a different part of the tree?
>  > 
>  > An SPU is very different from a CPU from the user perspective.
>  > SPUs show up in /sys/devices/system/spus, and if a user wants to access
>  > them, the "spufs" file system needs to be mounted in the system, by
>  > convention on /spu. 
> 
> Ok, that should be fairly simple to write scripts for.

Actually, "cpufreq-set -g spu_governor" should be enough. (BTW, do we really
want this name?)

Best,
	Dominik

^ permalink raw reply

* Making __copy_tofrom_user more readable for powerpc (arch/powerpc/lib/copy_32.S)
From: prodyut hazarika @ 2008-07-10 18:06 UTC (permalink / raw)
  To: linuxppc-dev

Hi all,
I am trying to optimize the __copy_tofrom_user function for PPC4xx,
and I would want to know why the exception handling code has been made
so complicated. All the fixup code should do is to store the number of
bytes that failed in copy in r3 and return back.

The current code tries to copy one byte at a time after read fault. I
don't understand why that is necessary. It then clears out the
destination. All these logic has made the code very unfriendly to
read.

I have a version which just keeps a count of bytes copied till any
fault happened. Then for any exception, I just substract this value
from the total number of bytes to be copied, and store in r3 and
return back. This is the common fixup code for all paths. It makes the
code much more readable like other architectures (eg. x86).

My questions are:
1) Why do we need to distinguish between load and store failure? If
either load or store fails, the copy did not go thru. But the code
sets r9 to 0 for load failure, and r9 to 1 for write failure. Why do
we need to care for that?

2) For read failure, why do we clear out the destination? Other
architecture don't do that.

I would appreciate any comments. I will submit my patch later.

Thanks,
Prodyut

^ permalink raw reply

* Re: linux/powerpc memory management
From: Arnd Bergmann @ 2008-07-10 17:25 UTC (permalink / raw)
  To: linuxppc-embedded; +Cc: Scott Wood, sumedh tirodkar
In-Reply-To: <d2b9ccd90807101006v79658fcateb5af4c83fc728da@mail.gmail.com>

On Thursday 10 July 2008, sumedh tirodkar wrote:
> by pointers i meant any documentation of design as to how exactly the memory
> management is done...
> how does linux use hardware features provided by powerpc for protection
> between tasks and kernel,etc...
> sorry for not being more specific previously...
> 

It depends on what platform you want to look at, powerpc64, powerpc32 classic
and powerpc32 book E are very different, but you'll see that in the code.

arch/powerpc/mm/hash*
arch/powerpc/mm/pgtable*
arch/powerpc/mm/mem.c
arch/powerpc/mm/ppc_mmu_32.c
arch/powerpc/mm/slb*
arch/powerpc/mm/stab.c
arch/powerpc/mm/tlb*
include/asm-powerpc/mmu*
include/asm-powerpc/pgalloc*
include/asm-powerpc/pgtable*

That should cover all you need to know.

	Arnd <><

^ permalink raw reply

* Re: linux/powerpc memory management
From: sumedh tirodkar @ 2008-07-10 17:06 UTC (permalink / raw)
  To: Scott Wood, linuxppc-embedded
In-Reply-To: <48762608.3060605@freescale.com>

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

by pointers i meant any documentation of design as to how exactly the memory
management is done...
how does linux use hardware features provided by powerpc for protection
between tasks and kernel,etc...
sorry for not being more specific previously...

Regards,
Sumedh.

On Thu, Jul 10, 2008 at 8:38 PM, Scott Wood <scottwood@freescale.com> wrote:

> sumedh tirodkar wrote:
>
>> Can anyone give pointers as to how memory management(detailed) is done in
>> linux on powerpc.
>>
>
> arch/powerpc/mm
>
> (Hey, you wanted a detailed answer to a very broad question...)
>
> -Scott
>

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

^ permalink raw reply

* Re: Booting ML405 (Kernel panic - not syncing: No init found)
From: Ron Sass @ 2008-07-10 15:54 UTC (permalink / raw)
  To: neeraj garg; +Cc: linuxppc-embedded, John.Linn, marco.stornelli
In-Reply-To: <4875B95C.8020809@cdac.in>


In addition to what John wrote, I would also investigate your ramdisk.
I would be sure to check that you have the permissions/owner set correctly
on bin/busybox.  Also, I would double check that, if you compiled busybox
with shared libraries, the shared libraries are in the right place
on your ramdisk.

Ron

> 
> Hi,
> 
> Yes I am using ARCH=ppc (actual line is $make ARCH=ppc 
> CROSS_COMPILE=powerpc-405-linux-gnu- zImage.initrd ) for this I have 
> placed ramdisk.image.gz in arch/ppc/boot/images. In case of ARCH=powerpc 
> I cannot find processor type 405 , in make menuconfig. Thats why i am 
> using ARCH=ppc.
> 
> And when I give kernel command string as init=/bin/sh , it says :
>  >Failed to execute /bin/sh.  Attempting defaults...
> [    3.744035] Kernel panic - not syncing: No init found.  Try passing 
> init= option to kernel.
> [    3.768073] Rebooting in 30 seconds..
> 
> --init is present in sbin/init which is a soft link to ../bin/busybox
> 
> --I used powepc-405-gnu-readelf -h bin/sh to verify that it is compiled 
> for powerpc itself.
> 
> Any other suggestions ?
> 

^ permalink raw reply

* Re: [PATCH 1/2] Convert i2c-mpc from a platform driver into a of_platform driver, V4
From: Timur Tabi @ 2008-07-10 16:36 UTC (permalink / raw)
  To: Grant Likely; +Cc: Linuxppc-dev, i2c
In-Reply-To: <fa686aa40807091022q22f6be99j89f311780bd3d369@mail.gmail.com>


On Jul 9, 2008, at 1:22 PM, Grant Likely wrote:

> On Mon, Jun 30, 2008 at 5:01 PM, Jon Smirl <jonsmirl@gmail.com> wrote:
>> Convert i2c-mpc to an of_platform driver. Utilize the code in  
>> drivers/of-i2c.c to make i2c modules dynamically loadable by the  
>> device tree.
>
> Timur, can you please test this one on an 83xx platform?  It works on
> 5200, but I want to make sure 83xx doesn't break before I pick it up.

I'm on vacation this week, so I'll have to deal with it next week,  
after I've handled higher-priority issues.

^ permalink raw reply

* [PATCH] powerpc: Xilinx: PS2: driver updates based on review
From: John Linn @ 2008-07-10 15:40 UTC (permalink / raw)
  To: linuxppc-dev, linux-input; +Cc: dmitry.torokhov, Sadanand, John Linn

Review comments were incorporated to improve the driver.

1. Some data was eliminated that was not needed.
2. Renaming of variables for clarity.
3. Removed unneeded type casting.
4. Changed to use dev_err rather than other I/O.
5. Merged together some functions.
6. Added kerneldoc format to functions.

Signed-off-by: Sadanand <sadanan@xilinx.com>
Signed-off-by: John Linn <john.linn@xilinx.com>
---

This patch is an incremental patch to be applied to 
[V3] powerpc: Xilinx: PS2: Added new XPS PS2 driver.

 drivers/input/serio/xilinx_ps2.c |  220 +++++++++++++++++++++----------------
 1 files changed, 125 insertions(+), 95 deletions(-)

diff --git a/drivers/input/serio/xilinx_ps2.c b/drivers/input/serio/xilinx_ps2.c
index e86f11b..eba46c7 100644
--- a/drivers/input/serio/xilinx_ps2.c
+++ b/drivers/input/serio/xilinx_ps2.c
@@ -58,23 +58,20 @@
 
 /* Mask for all the Receive Interrupts */
 #define XPS2_IPIXR_RX_ALL	(XPS2_IPIXR_RX_OVF | XPS2_IPIXR_RX_ERR |  \
-					XPS2_IPIXR_RX_FULL)
+				 XPS2_IPIXR_RX_FULL)
 
 /* Mask for all the Interrupts */
 #define XPS2_IPIXR_ALL		(XPS2_IPIXR_TX_ALL | XPS2_IPIXR_RX_ALL |  \
-					XPS2_IPIXR_WDT_TOUT)
+				 XPS2_IPIXR_WDT_TOUT)
 
 /* Global Interrupt Enable mask */
 #define XPS2_GIER_GIE_MASK	0x80000000
 
 struct xps2data {
 	int irq;
-	u32 phys_addr;
-	u32 remap_size;
 	spinlock_t lock;
-	u8 rxb;				/* Rx buffer */
 	void __iomem *base_address;	/* virt. address of control registers */
-	unsigned int dfl;
+	unsigned int flags;
 	struct serio serio;		/* serio */
 };
 
@@ -82,8 +79,13 @@ struct xps2data {
 /* XPS PS/2 data transmission calls */
 /************************************/
 
-/*
- * xps2_recv() will attempt to receive a byte of data from the PS/2 port.
+/**
+ * xps2_recv() - attempts to receive a byte from the PS/2 port.
+ * @drvdata:	pointer to ps2 device private data structure
+ * @byte:	address where the read data will be copied
+ *
+ * If there is any data available in the PS/2 receiver, this functions reads
+ * the data, otherwise it returns error.
  */
 static int xps2_recv(struct xps2data *drvdata, u8 *byte)
 {
@@ -105,7 +107,7 @@ static int xps2_recv(struct xps2data *drvdata, u8 *byte)
 /*********************/
 static irqreturn_t xps2_interrupt(int irq, void *dev_id)
 {
-	struct xps2data *drvdata = (struct xps2data *)dev_id;
+	struct xps2data *drvdata = dev_id;
 	u32 intr_sr;
 	u8 c;
 	int status;
@@ -115,35 +117,28 @@ static irqreturn_t xps2_interrupt(int irq, void *dev_id)
 	out_be32(drvdata->base_address + XPS2_IPISR_OFFSET, intr_sr);
 
 	/* Check which interrupt is active */
-	if (intr_sr & XPS2_IPIXR_RX_OVF) {
-		printk(KERN_ERR "%s: receive overrun error\n",
-			drvdata->serio.name);
-	}
+	if (intr_sr & XPS2_IPIXR_RX_OVF)
+		dev_err(drvdata->serio.dev.parent, "receive overrun error\n");
 
 	if (intr_sr & XPS2_IPIXR_RX_ERR)
-		drvdata->dfl |= SERIO_PARITY;
+		drvdata->flags |= SERIO_PARITY;
 
 	if (intr_sr & (XPS2_IPIXR_TX_NOACK | XPS2_IPIXR_WDT_TOUT))
-		drvdata->dfl |= SERIO_TIMEOUT;
+		drvdata->flags |= SERIO_TIMEOUT;
 
 	if (intr_sr & XPS2_IPIXR_RX_FULL) {
-		status = xps2_recv(drvdata, &drvdata->rxb);
+		status = xps2_recv(drvdata, &c);
 
 		/* Error, if a byte is not received */
 		if (status) {
-			printk(KERN_ERR
-				"%s: wrong rcvd byte count (%d)\n",
-				drvdata->serio.name, status);
+			dev_err(drvdata->serio.dev.parent,
+				"wrong rcvd byte count\n");
 		} else {
-			c = drvdata->rxb;
-			serio_interrupt(&drvdata->serio, c, drvdata->dfl);
-			drvdata->dfl = 0;
+			serio_interrupt(&drvdata->serio, c, drvdata->flags);
+			drvdata->flags = 0;
 		}
 	}
 
-	if (intr_sr & XPS2_IPIXR_TX_ACK)
-		drvdata->dfl = 0;
-
 	return IRQ_HANDLED;
 }
 
@@ -151,8 +146,15 @@ static irqreturn_t xps2_interrupt(int irq, void *dev_id)
 /* serio callbacks */
 /*******************/
 
-/*
- * sxps2_write() sends a byte out through the PS/2 interface.
+/**
+ * sxps2_write() - sends a byte out through the PS/2 port.
+ * @pserio:	pointer to the serio structure of the PS/2 port
+ * @c:		data that needs to be written to the PS/2 port
+ *
+ * This fucntion checks if the PS/2 transmitter is empty and sends a byte.
+ * Otherwise it returns error. Transmission fails only when nothing is connected
+ * to the PS/2 port. Thats why, we do not try to resend the data in case of a
+ * failure.
  */
 static int sxps2_write(struct serio *pserio, unsigned char c)
 {
@@ -173,33 +175,39 @@ static int sxps2_write(struct serio *pserio, unsigned char c)
 	return status;
 }
 
-/*
- * sxps2_open() is called when a port is open by the higher layer.
+/**
+ * sxps2_open() - called when a port is opened by the higher layer.
+ * @pserio:	pointer to the serio structure of the PS/2 device
+ *
+ * This function requests irq and enables interrupts for the PS/2 device.
  */
 static int sxps2_open(struct serio *pserio)
 {
 	struct xps2data *drvdata = pserio->port_data;
 	int retval;
+	u8 c;
 
 	retval = request_irq(drvdata->irq, &xps2_interrupt, 0,
 				DRIVER_NAME, drvdata);
 	if (retval) {
-		printk(KERN_ERR
-			"%s: Couldn't allocate interrupt %d\n",
-			drvdata->serio.name, drvdata->irq);
+		dev_err(drvdata->serio.dev.parent,
+			"Couldn't allocate interrupt %d\n", drvdata->irq);
 		return retval;
 	}
 
 	/* start reception by enabling the interrupts */
 	out_be32(drvdata->base_address + XPS2_GIER_OFFSET, XPS2_GIER_GIE_MASK);
 	out_be32(drvdata->base_address + XPS2_IPIER_OFFSET, XPS2_IPIXR_RX_ALL);
-	(void)xps2_recv(drvdata, &drvdata->rxb);
+	(void)xps2_recv(drvdata, &c);
 
 	return 0;		/* success */
 }
 
-/*
- * sxps2_close() frees the interrupt.
+/**
+ * sxps2_close() - frees the interrupt.
+ * @pserio:	pointer to the serio structure of the PS/2 device
+ *
+ * This function frees the irq and disables interrupts for the PS/2 device.
  */
 static void sxps2_close(struct serio *pserio)
 {
@@ -211,21 +219,60 @@ static void sxps2_close(struct serio *pserio)
 	free_irq(drvdata->irq, drvdata);
 }
 
-/*********************/
-/* Device setup code */
-/*********************/
+/***************************/
+/* OF Platform Bus Support */
+/***************************/
 
-static int xps2_setup(struct device *dev, struct resource *regs_res,
-		      struct resource *irq_res)
+/**
+ * xps2_of_probe - probe method for the PS/2 device.
+ * @of_dev:	pointer to OF device structure
+ * @match:	pointer to the stucture used for matching a device
+ *
+ * This function probes for a matching PS/2 device using OF properties and
+ * attaches it to the platform bus. It initializes the driver data structure
+ * and also initializes the hardware.
+ * It returns 0, if the driver is bound to the PS/2 device, or a negative value
+ * if there is an error. It releases all the allocated resources in case of an
+ * error.
+ *
+ */
+static int __devinit xps2_of_probe(struct of_device *ofdev, const struct
+				   of_device_id * match)
 {
+	struct resource r_irq; /* Interrupt resources */
+	struct resource r_mem; /* IO mem resources */
 	struct xps2data *drvdata;
 	struct serio *serio;
-	unsigned long remap_size;
-	int retval;
+	struct device *dev;
+	resource_size_t remap_size, phys_addr;
+	int rc = 0;
 
+	dev = &ofdev->dev;
 	if (!dev)
 		return -EINVAL;
 
+	dev_info(dev, "Device Tree Probing \'%s\'\n",
+			ofdev->node->name);
+
+	/* Get iospace for the device */
+	rc = of_address_to_resource(ofdev->node, 0, &r_mem);
+	if (rc) {
+		dev_err(dev, "invalid address\n");
+		return rc;
+	}
+
+	/* Get IRQ for the device */
+	rc = of_irq_to_resource(ofdev->node, 0, &r_irq);
+	if (rc == NO_IRQ) {
+		dev_err(dev, "no IRQ found\n");
+		return rc;
+	}
+
+	if (!(&r_mem) || !(&r_irq)) {
+		dev_err(dev, "IO resource(s) not found\n");
+		return -EFAULT;
+	}
+
 	drvdata = kzalloc(sizeof(struct xps2data), GFP_KERNEL);
 	if (!drvdata) {
 		dev_err(dev, "Couldn't allocate device private record\n");
@@ -234,31 +281,24 @@ static int xps2_setup(struct device *dev, struct resource *regs_res,
 	spin_lock_init(&drvdata->lock);
 	dev_set_drvdata(dev, drvdata);
 
-	if (!regs_res || !irq_res) {
-		dev_err(dev, "IO resource(s) not found\n");
-		retval = -EFAULT;
-		goto failed1;
-	}
-
-	drvdata->irq = irq_res->start;
-	remap_size = regs_res->end - regs_res->start + 1;
-	if (!request_mem_region(regs_res->start, remap_size, DRIVER_NAME)) {
+	drvdata->irq = r_irq.start;
+	phys_addr = r_mem.start;
+	remap_size = r_mem.end - r_mem.start + 1;
+	if (!request_mem_region(phys_addr, remap_size, DRIVER_NAME)) {
 
 		dev_err(dev, "Couldn't lock memory region at 0x%08X\n",
-			(unsigned int)regs_res->start);
-		retval = -EBUSY;
+			(unsigned int)phys_addr);
+		rc = -EBUSY;
 		goto failed1;
 	}
 
 	/* Fill in configuration data and add them to the list */
-	drvdata->phys_addr = regs_res->start;
-	drvdata->remap_size = remap_size;
-	drvdata->base_address = ioremap(regs_res->start, remap_size);
+	drvdata->base_address = ioremap(phys_addr, remap_size);
 	if (drvdata->base_address == NULL) {
 
 		dev_err(dev, "Couldn't ioremap memory at 0x%08X\n",
-			(unsigned int)regs_res->start);
-		retval = -EFAULT;
+			(unsigned int)phys_addr);
+		rc = -EFAULT;
 		goto failed2;
 	}
 
@@ -270,7 +310,8 @@ static int xps2_setup(struct device *dev, struct resource *regs_res,
 	out_be32(drvdata->base_address + XPS2_SRST_OFFSET, XPS2_SRST_RESET);
 
 	dev_info(dev, "Xilinx PS2 at 0x%08X mapped to 0x%08X, irq=%d\n",
-		drvdata->phys_addr, (u32)drvdata->base_address, drvdata->irq);
+		 (unsigned int)phys_addr, (u32)drvdata->base_address,
+		 drvdata->irq);
 
 	serio = &drvdata->serio;
 	serio->id.type = SERIO_8042;
@@ -280,58 +321,38 @@ static int xps2_setup(struct device *dev, struct resource *regs_res,
 	serio->port_data = drvdata;
 	serio->dev.parent = dev;
 	snprintf(drvdata->serio.name, sizeof(serio->name),
-		 "Xilinx XPS PS/2 at %08X", drvdata->phys_addr);
+		 "Xilinx XPS PS/2 at %08X", (unsigned int)phys_addr);
 	snprintf(drvdata->serio.phys, sizeof(serio->phys),
-		 "xilinxps2/serio at %08X", drvdata->phys_addr);
+		 "xilinxps2/serio at %08X", (unsigned int)phys_addr);
 	serio_register_port(serio);
 
 	return 0;		/* success */
 
 failed2:
-	release_mem_region(regs_res->start, remap_size);
+	release_mem_region(phys_addr, remap_size);
 
 failed1:
 	kfree(drvdata);
 	dev_set_drvdata(dev, NULL);
 
-	return retval;
-}
-
-/***************************/
-/* OF Platform Bus Support */
-/***************************/
-
-static int __devinit xps2_of_probe(struct of_device *ofdev, const struct
-				   of_device_id * match)
-{
-	struct resource r_irq; /* Interrupt resources */
-	struct resource r_mem; /* IO mem resources */
-	int rc = 0;
-
-	printk(KERN_INFO "Device Tree Probing \'%s\'\n",
-			ofdev->node->name);
-
-	/* Get iospace for the device */
-	rc = of_address_to_resource(ofdev->node, 0, &r_mem);
-	if (rc) {
-		dev_err(&ofdev->dev, "invalid address\n");
-		return rc;
-	}
-
-	/* Get IRQ for the device */
-	rc = of_irq_to_resource(ofdev->node, 0, &r_irq);
-	if (rc == NO_IRQ) {
-		dev_err(&ofdev->dev, "no IRQ found\n");
-		return rc;
-	}
-
-	return xps2_setup(&ofdev->dev, &r_mem, &r_irq);
+	return rc;
 }
 
+/**
+ * xps2_of_remove - unbinds the driver from the PS/2 device.
+ * @of_dev:	pointer to OF device structure
+ *
+ * This function is called if a device is physically removed from the system or
+ * if the driver module is being unloaded. It checks if the device is present
+ * and frees any resources allocated to the device.
+ */
 static int __devexit xps2_of_remove(struct of_device *of_dev)
 {
 	struct xps2data *drvdata;
 	struct device *dev;
+	struct resource r_mem; /* IO mem resources */
+	resource_size_t phys_addr, remap_size;
+	int rc;
 
 	dev = &of_dev->dev;
 	if (!dev)
@@ -343,7 +364,16 @@ static int __devexit xps2_of_remove(struct of_device *of_dev)
 
 	iounmap(drvdata->base_address);
 
-	release_mem_region(drvdata->phys_addr, drvdata->remap_size);
+	/* Get iospace of the device */
+	rc = of_address_to_resource(of_dev->node, 0, &r_mem);
+	if (rc) {
+		dev_err(dev, "invalid address\n");
+		return rc;
+	}
+
+	phys_addr = r_mem.start;
+	remap_size = r_mem.end - r_mem.start + 1;
+	release_mem_region(phys_addr, remap_size);
 
 	kfree(drvdata);
 	dev_set_drvdata(dev, NULL);
-- 
1.5.2.1



This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

^ permalink raw reply related

* Re: [PATCH] powerpc: Fix problems with 32bit PPC's running with more than 2GB of RAM
From: Becky Bruce @ 2008-07-10 15:31 UTC (permalink / raw)
  To: Josh Boyer; +Cc: Stefan Roese, linuxppc-dev
In-Reply-To: <20080709204435.4214f277@zod.rchland.ibm.com>


On Jul 9, 2008, at 7:44 PM, Josh Boyer wrote:

> On Thu, 10 Jul 2008 06:39:39 +1000
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
>> On Wed, 2008-07-09 at 16:22 -0400, Josh Boyer wrote:
>>> On Thu, 10 Jul 2008 06:02:38 +1000
>>> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>>>
>>>> On Wed, 2008-07-09 at 15:44 +0200, Stefan Roese wrote:
>>>>> This patch enables 32bit PPC's (with 36bit physical address  
>>>>> space, e.g.
>>>>> IBM/AMCC PPC44x) to run with more than 2GB of RAM. Mostly its just
>>>>> replacing types (unsigned long -> phys_addr_t).
>>>>>
>>>>> Tested on an AMCC Katmai with 4GB of DDR2.
>>>>>
>>>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>>>
>>>> But DMAs will break no ?
>>>
>>> How?
>>
>> Hrm... forget it. It's fine up to 4G of RAM... (ie. as long as DMA is
>> below 32 bits).
>
> Right.  We haven't really dealt with anything larger than 4 GiB, and  
> we
> certainly aren't dealing with discontiguous DRAM due to I/O ranges.

Are you *sure* you can see all 4GB?  I thought we lost some of the 32- 
bit PCI address space for PCI IO......  Disclaimer: I'm no expert on  
PCI :)

FYI, I'm *very* close to having swiotlb fully functional on powerpc to  
support larger RAM - I am able to boot with 6GB on 8641 right now.   
There are some bugs that I'm still shaking out - I hope to push this  
out in the next couple of weeks.

Cheers,
B

^ permalink raw reply

* Re: [BUG] fec_mpc52xx: Don't call mpc52xx_fec_reset() in ISR
From: Grant Likely @ 2008-07-10 15:31 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linuxppc-dev, domen.puncer
In-Reply-To: <20080710123909.GA4275@pengutronix.de>

On Thu, Jul 10, 2008 at 02:39:09PM +0200, Wolfram Sang wrote:
> Hello,
> 
> today, I was debugging a kernel crash on a board with a MPC5200B using
> 2.6.26-rc9. I found the following code in drivers/net/fec_mpc52xx.c:
<snip>
> I assume the proper thing to do is to set a flag in the ISR and handle
> the soft reset later in some other context. Having never dealt with the
> network core and its drivers so far, I am not sure which place would be
> the right one to perform the soft reset. To not make things worse, I
> hope people with more insight to network stuff can deliver a suitable
> solution to this problem.

Thanks for the bug report.  I'll take a look.

g.

^ permalink raw reply

* Re: linux/powerpc memory management
From: Scott Wood @ 2008-07-10 15:08 UTC (permalink / raw)
  To: sumedh tirodkar; +Cc: linuxppc-embedded
In-Reply-To: <d2b9ccd90807092114kc1883d2ud33c09c245d94d86@mail.gmail.com>

sumedh tirodkar wrote:
> Can anyone give pointers as to how memory management(detailed) is done 
> in linux on powerpc.

arch/powerpc/mm

(Hey, you wanted a detailed answer to a very broad question...)

-Scott

^ permalink raw reply

* Re: linux-next: kbuild tree build failure
From: Roman Zippel @ 2008-07-10 14:59 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Stephen Rothwell, linux-next, Paul Mackerras, Sam Ravnborg,
	linuxppc-dev
In-Reply-To: <1215651082.13950.7.camel@localhost>

Hi,

On Thu, 10 Jul 2008, Michael Ellerman wrote:

> Well yes :)  But I think that's because you're thinking of 
> "end-users" and I'm thinking of "users" like myself - ie. _I_ use
> Kconfig and I do expect myself to be able to type a 64-bit address.

That doesn't really answer my question, why you need this.

> > > --- .config.orig	2008-07-08 09:30:00.000000000 +1000
> > > +++ .config	2008-07-08 09:30:43.000000000 +1000
> > > @@ -370,9 +370,8 @@
> > >  CONFIG_HOTPLUG_PCI_RPA=m
> > >  CONFIG_HOTPLUG_PCI_RPA_DLPAR=m
> > >  # CONFIG_HAS_RAPIDIO is not set
> > > -CONFIG_PAGE_OFFSET=0xc000000000000000
> > > -CONFIG_KERNEL_START=0xc000000002000000
> > > -CONFIG_PHYSICAL_START=0x02000000
> > > +CONFIG_PAGE_OFFSET=0xc0000000
> > > +CONFIG_PHYSICAL_START=0x2000000
> > 
> > Why is this worse? These are constants, you're not supposed to change them 
> > anyway.
> > The remaining values are generated in page.h and should be the same as 
> > before. If that isn't the case and this patch produces a nonworking 
> > kernel, I'd like to hear about it.
> 
> You're right the built kernel is fine. So it's not a bug,

Good, could someone please ack whether the powerpc changes are acceptable?

> but I think it is nicer to have the real values in the .config.

Why?

bye, Roman

^ 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