[PATCHv3 0/4] cpm2: Reset the CPM at startup and fix the cpm_uart driver accordingly.

[PATCHv3 0/4] cpm2: Reset the CPM at startup and fix the cpm_uart driver accordingly.

[Laurent Pinchart]

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

Hi everybody,

these 4 patches reset the CPM in cpm2_reset() and fix the cpm_uart driver to 
initialise SMC ports correctly without relying on any initialisation 
performed by the boot loader/wrapper. They update the boot wrapper code and
the EP8248E device tree to match the new SMC registers description.

-- 
Laurent Pinchart
CSE Semaphore Belgium

Chaussée de Bruxelles, 732A
B-1410 Waterloo
Belgium

T +32 (2) 387 42 59
F +32 (2) 387 42 75

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[PATCHv3 1/4] cpm_uart: Allocate DPRAM memory for SMC ports on CPM2-based platforms.

[Laurent Pinchart]

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

This patch allocates parameter RAM for SMC serial ports without relying on
previous initialisation by a boot loader or a wrapper layer.

SMC parameter RAM on CPM2-based platforms can be allocated anywhere in the
general-purpose areas of the dual-port RAM. The current code relies on the
boot loader to allocate a section of general-purpose CPM RAM and gets the
section address from the device tree.

This patch modifies the device tree address usage to reference the SMC
parameter RAM base pointer instead of a pre-allocated RAM section and
allocates memory from the CPM dual-port RAM when initialising the SMC port.
CPM1-based platforms are not affected.

Signed-off-by: Laurent Pinchart <laurentp@cse-semaphore.com>
---
 drivers/serial/cpm_uart/cpm_uart.h      |    3 ++
 drivers/serial/cpm_uart/cpm_uart_core.c |   19 +++++------
 drivers/serial/cpm_uart/cpm_uart_cpm1.c |   12 +++++++
 drivers/serial/cpm_uart/cpm_uart_cpm2.c |   52 +++++++++++++++++++++++++++++++
 4 files changed, 76 insertions(+), 10 deletions(-)

diff --git a/drivers/serial/cpm_uart/cpm_uart.h b/drivers/serial/cpm_uart/cpm_uart.h
index 80a7d60..5334653 100644
--- a/drivers/serial/cpm_uart/cpm_uart.h
+++ b/drivers/serial/cpm_uart/cpm_uart.h
@@ -93,6 +93,9 @@ extern struct uart_cpm_port cpm_uart_ports[UART_NR];
 
 /* these are located in their respective files */
 void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd);
+void __iomem *cpm_uart_map_pram(struct uart_cpm_port *port,
+				struct device_node *np);
+void cpm_uart_unmap_pram(struct uart_cpm_port *port, void __iomem *pram);
 int cpm_uart_init_portdesc(void);
 int cpm_uart_allocbuf(struct uart_cpm_port *pinfo, unsigned int is_con);
 void cpm_uart_freebuf(struct uart_cpm_port *pinfo);
diff --git a/drivers/serial/cpm_uart/cpm_uart_core.c b/drivers/serial/cpm_uart/cpm_uart_core.c
index 1ea123c..3a44a3f 100644
--- a/drivers/serial/cpm_uart/cpm_uart_core.c
+++ b/drivers/serial/cpm_uart/cpm_uart_core.c
@@ -997,24 +997,23 @@ static int cpm_uart_init_port(struct device_node *np,
 	if (!mem)
 		return -ENOMEM;
 
-	pram = of_iomap(np, 1);
-	if (!pram) {
-		ret = -ENOMEM;
-		goto out_mem;
-	}
-
 	if (of_device_is_compatible(np, "fsl,cpm1-scc-uart") ||
 	    of_device_is_compatible(np, "fsl,cpm2-scc-uart")) {
 		pinfo->sccp = mem;
-		pinfo->sccup = pram;
+		pinfo->sccup = pram = cpm_uart_map_pram(pinfo, np);
 	} else if (of_device_is_compatible(np, "fsl,cpm1-smc-uart") ||
 	           of_device_is_compatible(np, "fsl,cpm2-smc-uart")) {
 		pinfo->flags |= FLAG_SMC;
 		pinfo->smcp = mem;
-		pinfo->smcup = pram;
+		pinfo->smcup = pram = cpm_uart_map_pram(pinfo, np);
 	} else {
 		ret = -ENODEV;
-		goto out_pram;
+		goto out_mem;
+	}
+
+	if (!pram) {
+		ret = -ENOMEM;
+		goto out_mem;
 	}
 
 	pinfo->tx_nrfifos = TX_NUM_FIFO;
@@ -1038,7 +1037,7 @@ static int cpm_uart_init_port(struct device_node *np,
 	return cpm_uart_request_port(&pinfo->port);
 
 out_pram:
-	iounmap(pram);
+	cpm_uart_unmap_pram(pinfo, pram);
 out_mem:
 	iounmap(mem);
 	return ret;
diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm1.c b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
index 6ea0366..e692593 100644
--- a/drivers/serial/cpm_uart/cpm_uart_cpm1.c
+++ b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
@@ -54,6 +54,18 @@ void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd)
 {
 	cpm_command(port->command, cmd);
 }
+
+void __iomem *cpm_uart_map_pram(struct uart_cpm_port *port,
+				struct device_node *np)
+{
+	return of_iomap(np, 1);
+}
+
+void cpm_uart_unmap_pram(struct uart_cpm_port *port, void __iomem *pram)
+{
+	iounmap(pram);
+}
+
 #else
 void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd)
 {
diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm2.c b/drivers/serial/cpm_uart/cpm_uart_cpm2.c
index 6291094..a4cfb0b 100644
--- a/drivers/serial/cpm_uart/cpm_uart_cpm2.c
+++ b/drivers/serial/cpm_uart/cpm_uart_cpm2.c
@@ -41,6 +41,9 @@
 #include <asm/io.h>
 #include <asm/irq.h>
 #include <asm/fs_pd.h>
+#ifdef CONFIG_PPC_CPM_NEW_BINDING
+#include <asm/prom.h>
+#endif
 
 #include <linux/serial_core.h>
 #include <linux/kernel.h>
@@ -54,6 +57,55 @@ void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd)
 {
 	cpm_command(port->command, cmd);
 }
+
+void __iomem *cpm_uart_map_pram(struct uart_cpm_port *port,
+				struct device_node *np)
+{
+	void __iomem *pram;
+	unsigned long offset;
+	struct resource res;
+	unsigned long len;
+
+	/* Don't remap parameter RAM if it has already been initialized
+	 * during console setup.
+	 */
+	if (IS_SMC(port) && port->smcup)
+		return port->smcup;
+	else if (!IS_SMC(port) && port->sccup)
+		return port->sccup;
+
+	if (of_address_to_resource(np, 1, &res))
+		return NULL;
+
+	len = 1 + res.end - res.start;
+	pram = ioremap(res.start, len);
+	if (!pram)
+		return NULL;
+
+	if (!IS_SMC(port))
+		return pram;
+
+	if (len != 2) {
+		printk(KERN_WARNING "cpm_uart[%d]: device tree references "
+			"SMC pram, using boot loader/wrapper pram mapping. "
+			"Please fix your device tree to reference the pram "
+			"base register instead.\n",
+			port->port.line);
+		return pram;
+	}
+
+	offset = cpm_dpalloc(PROFF_SMC_SIZE, 64);
+	out_be16(pram, offset);
+	iounmap(pram);
+	return cpm_muram_addr(offset);
+}
+
+void cpm_uart_unmap_pram(struct uart_cpm_port *port, void __iomem *pram)
+{
+	if (!IS_SMC(port))
+		iounmap(pram);
+}
+
 #else
 void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd)
 {
-- 
1.5.0


-- 
Laurent Pinchart
CSE Semaphore Belgium

Chaussée de Bruxelles, 732A
B-1410 Waterloo
Belgium

T +32 (2) 387 42 59
F +32 (2) 387 42 75

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[PATCHv3 2/4] cpm-serial: Relocate CPM buffer descriptors and SMC parameter ram.

[Laurent Pinchart]

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

This patch relocates the buffer descriptors and the SMC parameter RAM at the
end of the first CPM muram chunk, as described in the device tree. This allows
device trees to stop excluding SMC parameter ram allocated by the boot loader
from the CPM muram node.

Signed-off-by: Laurent Pinchart <laurentp@cse-semaphore.com>
---
 arch/powerpc/Kconfig.debug     |    2 +-
 arch/powerpc/boot/cpm-serial.c |  132 ++++++++++++++++++++++++++--------------
 2 files changed, 87 insertions(+), 47 deletions(-)

diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index db7cc34..a86d8d8 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -269,7 +269,7 @@ config PPC_EARLY_DEBUG_CPM_ADDR
 	hex "CPM UART early debug transmit descriptor address"
 	depends on PPC_EARLY_DEBUG_CPM
 	default "0xfa202008" if PPC_EP88XC
-	default "0xf0000008" if CPM2
+	default "0xf0001ff8" if CPM2
 	default "0xff002008" if CPM1
 	help
 	  This specifies the address of the transmit descriptor
diff --git a/arch/powerpc/boot/cpm-serial.c b/arch/powerpc/boot/cpm-serial.c
index 28296fa..8d02b2d 100644
--- a/arch/powerpc/boot/cpm-serial.c
+++ b/arch/powerpc/boot/cpm-serial.c
@@ -11,6 +11,7 @@
 #include "types.h"
 #include "io.h"
 #include "ops.h"
+#include "page.h"
 
 struct cpm_scc {
 	u32 gsmrl;
@@ -42,6 +43,22 @@ struct cpm_param {
 	u16 tbase;
 	u8 rfcr;
 	u8 tfcr;
+	u16 mrblr;
+	u32 rstate;
+	u8 res1[4];
+	u16 rbptr;
+	u8 res2[6];
+	u32 tstate;
+	u8 res3[4];
+	u16 tbptr;
+	u8 res4[6];
+	u16 maxidl;
+	u16 idlc;
+	u16 brkln;
+	u16 brkec;
+	u16 brkcr;
+	u16 rmask;
+	u8 res5[4];
 };
 
 struct cpm_bd {
@@ -54,10 +71,10 @@ static void *cpcr;
 static struct cpm_param *param;
 static struct cpm_smc *smc;
 static struct cpm_scc *scc;
-struct cpm_bd *tbdf, *rbdf;
+static struct cpm_bd *tbdf, *rbdf;
 static u32 cpm_cmd;
-static u8 *muram_start;
-static u32 muram_offset;
+static void *cbd_addr;
+static u32 cbd_offset;
 
 static void (*do_cmd)(int op);
 static void (*enable_port)(void);
@@ -119,20 +136,25 @@ static int cpm_serial_open(void)
 
 	out_8(&param->rfcr, 0x10);
 	out_8(&param->tfcr, 0x10);
-
-	rbdf = (struct cpm_bd *)muram_start;
-	rbdf->addr = (u8 *)(rbdf + 2);
+	out_be16(&param->mrblr, 1);
+	out_be16(&param->maxidl, 0);
+	out_be16(&param->brkec, 0);
+	out_be16(&param->brkln, 0);
+	out_be16(&param->brkcr, 0);
+
+	rbdf = cbd_addr;
+	rbdf->addr = (u8 *)rbdf - 1;
 	rbdf->sc = 0xa000;
 	rbdf->len = 1;
 
 	tbdf = rbdf + 1;
-	tbdf->addr = (u8 *)(rbdf + 2) + 1;
+	tbdf->addr = (u8 *)rbdf - 2;
 	tbdf->sc = 0x2000;
 	tbdf->len = 1;
 
 	sync();
-	out_be16(&param->rbase, muram_offset);
-	out_be16(&param->tbase, muram_offset + sizeof(struct cpm_bd));
+	out_be16(&param->rbase, cbd_offset);
+	out_be16(&param->tbase, cbd_offset + sizeof(struct cpm_bd));
 
 	do_cmd(CPM_CMD_INIT_RX_TX);
 
@@ -173,12 +195,31 @@ static unsigned char cpm_serial_getc(void)
 	return c;
 }
 
+static int cpm_get_virtual_address(void *devp, void **addr, int ncells)
+{
+	unsigned long xaddr;
+	int n;
+
+	n = getprop(devp, "virtual-reg", addr, ncells * sizeof *addr);
+	if (n < ncells * sizeof *addr) {
+		for (n = 0; n < ncells; n++) {
+			if (!dt_xlate_reg(devp, n, &xaddr, NULL))
+				return -1;
+
+			addr[n] = (void*)xaddr;
+		}
+	}
+
+	return ncells;
+}
+
 int cpm_console_init(void *devp, struct serial_console_data *scdp)
 {
-	void *reg_virt[2];
-	int is_smc = 0, is_cpm2 = 0, n;
-	unsigned long reg_phys;
+	void *reg[2];
 	void *parent, *muram;
+	void *muram_addr;
+	int is_smc = 0, is_cpm2 = 0;
+	unsigned long muram_offset, muram_size;
 
 	if (dt_is_compatible(devp, "fsl,cpm1-smc-uart")) {
 		is_smc = 1;
@@ -202,63 +243,62 @@ int cpm_console_init(void *devp, struct serial_console_data *scdp)
 	else
 		do_cmd = cpm1_cmd;
 
-	n = getprop(devp, "fsl,cpm-command", &cpm_cmd, 4);
-	if (n < 4)
+	if (getprop(devp, "fsl,cpm-command", &cpm_cmd, 4) < sizeof cpm_cmd)
 		return -1;
 
-	n = getprop(devp, "virtual-reg", reg_virt, sizeof(reg_virt));
-	if (n < (int)sizeof(reg_virt)) {
-		for (n = 0; n < 2; n++) {
-			if (!dt_xlate_reg(devp, n, &reg_phys, NULL))
-				return -1;
-
-			reg_virt[n] = (void *)reg_phys;
-		}
-	}
+	if (cpm_get_virtual_address(devp, reg, 2) < 0)
+		return -1;
 
 	if (is_smc)
-		smc = reg_virt[0];
+		smc = reg[0];
 	else
-		scc = reg_virt[0];
+		scc = reg[0];
 
-	param = reg_virt[1];
+	param = reg[1];
 
 	parent = get_parent(devp);
 	if (!parent)
 		return -1;
 
-	n = getprop(parent, "virtual-reg", reg_virt, sizeof(reg_virt));
-	if (n < (int)sizeof(reg_virt)) {
-		if (!dt_xlate_reg(parent, 0, &reg_phys, NULL))
-			return -1;
-
-		reg_virt[0] = (void *)reg_phys;
-	}
-
-	cpcr = reg_virt[0];
+	if (cpm_get_virtual_address(devp, &cpcr, 1) < 0)
+		return -1;
 
 	muram = finddevice("/soc/cpm/muram/data");
 	if (!muram)
 		return -1;
 
 	/* For bootwrapper-compatible device trees, we assume that the first
-	 * entry has at least 18 bytes, and that #address-cells/#data-cells
+	 * entry has at least 128 bytes, and that #address-cells/#data-cells
 	 * is one for both parent and child.
 	 */
 
-	n = getprop(muram, "virtual-reg", reg_virt, sizeof(reg_virt));
-	if (n < (int)sizeof(reg_virt)) {
-		if (!dt_xlate_reg(muram, 0, &reg_phys, NULL))
-			return -1;
+	if (cpm_get_virtual_address(devp, &muram_addr, 1) < 0)
+		return -1;
+	
+	if (getprop(muram, "reg", reg, sizeof reg) < sizeof reg)
+		return -1;
+
+	muram_offset = (unsigned long)reg[0];
+	muram_size = (unsigned long)reg[1];
+
+	/* Store the buffer descriptors at the end of the first muram chunk.
+	 * For SMC ports on CPM2-based platforms, relocate the parameter RAM
+	 * just before the buffer descriptors.
+	 */
+
+	cbd_offset = muram_offset + muram_size - 2 * sizeof(struct cpm_bd);
 
-		reg_virt[0] = (void *)reg_phys;
-	}
+	if (is_cpm2 && is_smc) {
+		u16 *smc_base = (u16*)param;
+		u16 pram_offset;
 
-	muram_start = reg_virt[0];
+		pram_offset = cbd_offset - 64;
+		pram_offset = _ALIGN_DOWN(pram_offset, 64);
+		*smc_base = pram_offset;
+		param = muram_addr - muram_offset + pram_offset;
+	}
 
-	n = getprop(muram, "reg", &muram_offset, 4);
-	if (n < 4)
-		return -1;
+	cbd_addr = muram_addr - muram_offset + cbd_offset;
 
 	scdp->open = cpm_serial_open;
 	scdp->putc = cpm_serial_putc;
-- 
1.5.0


-- 
Laurent Pinchart
CSE Semaphore Belgium

Chaussée de Bruxelles, 732A
B-1410 Waterloo
Belgium

T +32 (2) 387 42 59
F +32 (2) 387 42 75

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[PATCHv3 3/4] ep8248e: Reference SMC parameter RAM base in the device tree.

[Laurent Pinchart]

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

This patch modifies the Embedded Planet EP8248E device tree to reference the
SMC paramater RAM base register instead of the parameter RAM allocated by the
boot loader.

The cpm_uart driver will allocate parameter RAM itself, making the serial port
initialisation independent of the boot loader.

The patch adds the parameter RAM allocated by the boot loader in the CPM muram
node, making it available to the kernel.

Signed-off-by: Laurent Pinchart <laurentp@cse-semaphore.com>
---
 arch/powerpc/boot/dts/ep8248e.dts |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/boot/dts/ep8248e.dts b/arch/powerpc/boot/dts/ep8248e.dts
index 5d2fb76..756758f 100644
--- a/arch/powerpc/boot/dts/ep8248e.dts
+++ b/arch/powerpc/boot/dts/ep8248e.dts
@@ -121,8 +121,7 @@
 
 				data@0 {
 					compatible = "fsl,cpm-muram-data";
-					reg = <0 0x1100 0x1140
-					       0xec0 0x9800 0x800>;
+					reg = <0 0x2000 0x9800 0x800>;
 				};
 			};
 
@@ -138,7 +137,7 @@
 				device_type = "serial";
 				compatible = "fsl,mpc8248-smc-uart",
 				             "fsl,cpm2-smc-uart";
-				reg = <0x11a80 0x20 0x1100 0x40>;
+				reg = <0x11a80 0x20 0x87fc 2>;
 				interrupts = <4 8>;
 				interrupt-parent = <&PIC>;
 				fsl,cpm-brg = <7>;
-- 
1.5.0


-- 
Laurent Pinchart
CSE Semaphore Belgium

Chaussée de Bruxelles, 732A
B-1410 Waterloo
Belgium

T +32 (2) 387 42 59
F +32 (2) 387 42 75

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[PATCHv3 4/4] cpm2: Reset the CPM when early debugging is not enabled.

[Laurent Pinchart]

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

Similarly to what is done for PQ1-based platforms, this patch resets the
PQ2 Communication Processor Module in cpm2_reset() when early debugging is
not enabled. This helps avoiding conflicts when the boot loader configured
the CPM in an unexpected way.

Signed-off-by: Laurent Pinchart <laurentp@cse-semaphore.com>
---
 arch/powerpc/sysdev/cpm2.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/sysdev/cpm2.c b/arch/powerpc/sysdev/cpm2.c
index 7be7112..57ed1a4 100644
--- a/arch/powerpc/sysdev/cpm2.c
+++ b/arch/powerpc/sysdev/cpm2.c
@@ -80,6 +80,12 @@ void __init cpm2_reset(void)
 	/* Tell everyone where the comm processor resides.
 	 */
 	cpmp = &cpm2_immr->im_cpm;
+
+#ifndef CONFIG_PPC_EARLY_DEBUG_CPM
+	/* Reset the CPM.
+	 */
+	cpm_command(CPM_CR_RST, 0);
+#endif
 }
 
 static DEFINE_SPINLOCK(cmd_lock);
-- 
1.5.0


-- 
Laurent Pinchart
CSE Semaphore Belgium

Chaussée de Bruxelles, 732A
B-1410 Waterloo
Belgium

T +32 (2) 387 42 59
F +32 (2) 387 42 75

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCHv3 2/4] cpm-serial: Relocate CPM buffer descriptors and SMC parameter ram.

[Scott Wood]

Laurent Pinchart wrote:
> This patch relocates the buffer descriptors and the SMC parameter RAM at the
> end of the first CPM muram chunk, as described in the device tree. This allows
> device trees to stop excluding SMC parameter ram allocated by the boot loader
> from the CPM muram node.

It's usually a good idea to state that something is untested if that's 
the case. :-)

This patch cannot work as is.

> +static int cpm_get_virtual_address(void *devp, void **addr, int ncells)
> +{
> +	unsigned long xaddr;
> +	int n;
> +
> +	n = getprop(devp, "virtual-reg", addr, ncells * sizeof *addr);
> +	if (n < ncells * sizeof *addr) {

You must cast the sizeof to a signed int; otherwise, a negative return 
from getprop will be "bigger" than the unsigned size, and you'll return 
garbage as the address.

> +		for (n = 0; n < ncells; n++) {
> +			if (!dt_xlate_reg(devp, n, &xaddr, NULL))
> +				return -1;
> +
> +			addr[n] = (void*)xaddr;

(void *)

> +		}
> +	}
> +
> +	return ncells;
> +}

This could be a generic bootwrapper function.  It should return the 
number of resources (ncells is a misnomer) actually found, though, 
rather than failing if there are fewer than asked for.  Let the caller 
decide if it's fatal.

> @@ -202,63 +243,62 @@ int cpm_console_init(void *devp, struct serial_console_data *scdp)
>  	else
>  		do_cmd = cpm1_cmd;
>  
> -	n = getprop(devp, "fsl,cpm-command", &cpm_cmd, 4);
> -	if (n < 4)
> +	if (getprop(devp, "fsl,cpm-command", &cpm_cmd, 4) < sizeof cpm_cmd)
>  		return -1;

Standard kernel style is sizeof(foo), not sizeof foo.

Plus, if you're going to replace 4 with sizeof(cpm_cmd), do it both 
places.  I don't really see the need, though; a cell is always 4 bytes.

> -	n = getprop(parent, "virtual-reg", reg_virt, sizeof(reg_virt));
> -	if (n < (int)sizeof(reg_virt)) {
> -		if (!dt_xlate_reg(parent, 0, &reg_phys, NULL))
> -			return -1;
> -
> -		reg_virt[0] = (void *)reg_phys;
> -	}
> -
> -	cpcr = reg_virt[0];
> +	if (cpm_get_virtual_address(devp, &cpcr, 1) < 0)
> +		return -1;

s/devp/parent/

>  	muram = finddevice("/soc/cpm/muram/data");
>  	if (!muram)
>  		return -1;
>  
>  	/* For bootwrapper-compatible device trees, we assume that the first
> -	 * entry has at least 18 bytes, and that #address-cells/#data-cells
> +	 * entry has at least 128 bytes, and that #address-cells/#data-cells
>  	 * is one for both parent and child.
>  	 */
>  
> -	n = getprop(muram, "virtual-reg", reg_virt, sizeof(reg_virt));
> -	if (n < (int)sizeof(reg_virt)) {
> -		if (!dt_xlate_reg(muram, 0, &reg_phys, NULL))
> -			return -1;
> +	if (cpm_get_virtual_address(devp, &muram_addr, 1) < 0)
> +		return -1;

s/devp/muram/

> +	
> +	if (getprop(muram, "reg", reg, sizeof reg) < sizeof reg)
> +		return -1;

Should read into array of u32, not void *.

> +	if (is_cpm2 && is_smc) {
> +		u16 *smc_base = (u16*)param;

(u16 *)

> +		u16 pram_offset;
>  
> -	muram_start = reg_virt[0];
> +		pram_offset = cbd_offset - 64;
> +		pram_offset = _ALIGN_DOWN(pram_offset, 64);
> +		*smc_base = pram_offset;

Use out_be16().

The SMC should be stopped before you do this.

-Scott

Re: [PATCHv3 2/4] cpm-serial: Relocate CPM buffer descriptors and SMC parameter ram.

[Laurent Pinchart]

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

On Monday 31 March 2008 21:10, Scott Wood wrote:
> Laurent Pinchart wrote:
> > This patch relocates the buffer descriptors and the SMC parameter RAM at the
> > end of the first CPM muram chunk, as described in the device tree. This allows
> > device trees to stop excluding SMC parameter ram allocated by the boot loader
> > from the CPM muram node.
> 
> It's usually a good idea to state that something is untested if that's 
> the case. :-)

Sorry. I'll state it clearly next time.

> This patch cannot work as is.
> 
> > +static int cpm_get_virtual_address(void *devp, void **addr, int ncells)
> > +{
> > +	unsigned long xaddr;
> > +	int n;
> > +
> > +	n = getprop(devp, "virtual-reg", addr, ncells * sizeof *addr);
> > +	if (n < ncells * sizeof *addr) {
> 
> You must cast the sizeof to a signed int; otherwise, a negative return 
> from getprop will be "bigger" than the unsigned size, and you'll return 
> garbage as the address.
> 
> > +		for (n = 0; n < ncells; n++) {
> > +			if (!dt_xlate_reg(devp, n, &xaddr, NULL))
> > +				return -1;
> > +
> > +			addr[n] = (void*)xaddr;
> 
> (void *)
> 
> > +		}
> > +	}
> > +
> > +	return ncells;
> > +}
> 
> This could be a generic bootwrapper function.  It should return the 
> number of resources (ncells is a misnomer) actually found, though, 
> rather than failing if there are fewer than asked for.  Let the caller 
> decide if it's fatal.

Ok. I'll move it to devtree.c.

> > @@ -202,63 +243,62 @@ int cpm_console_init(void *devp, struct serial_console_data *scdp)
> >  	else
> >  		do_cmd = cpm1_cmd;
> >  
> > -	n = getprop(devp, "fsl,cpm-command", &cpm_cmd, 4);
> > -	if (n < 4)
> > +	if (getprop(devp, "fsl,cpm-command", &cpm_cmd, 4) < sizeof cpm_cmd)
> >  		return -1;
> 
> Standard kernel style is sizeof(foo), not sizeof foo.

Ok.

> Plus, if you're going to replace 4 with sizeof(cpm_cmd), do it both 
> places.  I don't really see the need, though; a cell is always 4 bytes.

Ok, I'll keep 4.

> > -	n = getprop(parent, "virtual-reg", reg_virt, sizeof(reg_virt));
> > -	if (n < (int)sizeof(reg_virt)) {
> > -		if (!dt_xlate_reg(parent, 0, &reg_phys, NULL))
> > -			return -1;
> > -
> > -		reg_virt[0] = (void *)reg_phys;
> > -	}
> > -
> > -	cpcr = reg_virt[0];
> > +	if (cpm_get_virtual_address(devp, &cpcr, 1) < 0)
> > +		return -1;
> 
> s/devp/parent/
> 
> >  	muram = finddevice("/soc/cpm/muram/data");
> >  	if (!muram)
> >  		return -1;
> >  
> >  	/* For bootwrapper-compatible device trees, we assume that the first
> > -	 * entry has at least 18 bytes, and that #address-cells/#data-cells
> > +	 * entry has at least 128 bytes, and that #address-cells/#data-cells
> >  	 * is one for both parent and child.
> >  	 */
> >  
> > -	n = getprop(muram, "virtual-reg", reg_virt, sizeof(reg_virt));
> > -	if (n < (int)sizeof(reg_virt)) {
> > -		if (!dt_xlate_reg(muram, 0, &reg_phys, NULL))
> > -			return -1;
> > +	if (cpm_get_virtual_address(devp, &muram_addr, 1) < 0)
> > +		return -1;
> 
> s/devp/muram/
> 
> > +	
> > +	if (getprop(muram, "reg", reg, sizeof reg) < sizeof reg)
> > +		return -1;
> 
> Should read into array of u32, not void *.

Ok.

> > +	if (is_cpm2 && is_smc) {
> > +		u16 *smc_base = (u16*)param;
> 
> (u16 *)
> 
> > +		u16 pram_offset;
> >  
> > -	muram_start = reg_virt[0];
> > +		pram_offset = cbd_offset - 64;
> > +		pram_offset = _ALIGN_DOWN(pram_offset, 64);
> > +		*smc_base = pram_offset;
> 
> Use out_be16().
> 
> The SMC should be stopped before you do this.

Right.

New patch comming.

-- 
Laurent Pinchart
CSE Semaphore Belgium

Chaussée de Bruxelles, 732A
B-1410 Waterloo
Belgium

T +32 (2) 387 42 59
F +32 (2) 387 42 75

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]