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

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

[Laurent Pinchart]

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

Hi everybody,

these 3 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 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 --]

[PATCHv2 1/3] 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 --]

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

[Laurent Pinchart]

[-- Attachment #1: Type: text/plain, Size: 1332 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.

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

[PATCHv2 0/3] 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: [PATCHv2 0/3] cpm2: Reset the CPM when early debugging is not enabled.

[Laurent Pinchart]

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

This one should of course have been PATCHv2 3/3.

-- 
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: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.

[Scott Wood]

On Wed, Mar 26, 2008 at 12:20:42PM +0100, Laurent Pinchart wrote:
> 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>;
>  				};

Can we leave this as reserved, so that udbg still works?

-Scott

Re: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.

[Scott Wood]

Laurent Pinchart wrote:
> @@ -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>;

This breaks the bootwrapper console.

-Scott

Re: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.

[Laurent Pinchart]

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

On Wednesday 26 March 2008 16:57, Scott Wood wrote:
> On Wed, Mar 26, 2008 at 12:20:42PM +0100, Laurent Pinchart wrote:
> > 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>;
> >  				};
> 
> Can we leave this as reserved, so that udbg still works?

Ok. Should I add a comment to the DTS to explain additional muram can be 
reclaimed if udbg isn't used ?

-- 
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: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.

[Laurent Pinchart]

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

On Wednesday 26 March 2008 17:59, Scott Wood wrote:
> Laurent Pinchart wrote:
> > @@ -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>;
> 
> This breaks the bootwrapper console.

And of course I forgot about that :-)

The boot wrapper code doesn't have any dpram allocator. Any objection against 
using a chunk of dpram at a fixed location ? What about at the beginning of 
the dpram ? The DTS muram node would then exclude a chunk of dpram at offset 
0x0000 instead of 0x1100.

-- 
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: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.

[Scott Wood]

On Thu, Mar 27, 2008 at 10:10:33AM +0100, Laurent Pinchart wrote:
> On Wednesday 26 March 2008 17:59, Scott Wood wrote:
> > This breaks the bootwrapper console.
> 
> And of course I forgot about that :-)
> 
> The boot wrapper code doesn't have any dpram allocator. Any objection against 
> using a chunk of dpram at a fixed location ? What about at the beginning of 
> the dpram ? The DTS muram node would then exclude a chunk of dpram at offset 
> 0x0000 instead of 0x1100.

I'm not entirely comfortable with using a chunk outside of what's in the
muram node, and assuming that it's for the SMC pram -- what if there's
microcode or something there?

Since udbg is only for debugging, and is marked as potentially dangerous,
how about just using the end of muram (as described in the device tree)? 
If the muram is fully allocated, it won't happen until after the real
serial console is initialized.

-Scott

Re: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.

[Laurent Pinchart]

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

On Thursday 27 March 2008 16:39, Scott Wood wrote:
> On Thu, Mar 27, 2008 at 10:10:33AM +0100, Laurent Pinchart wrote:
> > On Wednesday 26 March 2008 17:59, Scott Wood wrote:
> > > This breaks the bootwrapper console.
> > 
> > And of course I forgot about that :-)
> > 
> > The boot wrapper code doesn't have any dpram allocator. Any objection against 
> > using a chunk of dpram at a fixed location ? What about at the beginning of 
> > the dpram ? The DTS muram node would then exclude a chunk of dpram at offset 
> > 0x0000 instead of 0x1100.
> 
> I'm not entirely comfortable with using a chunk outside of what's in the
> muram node, and assuming that it's for the SMC pram -- what if there's
> microcode or something there?
> 
> Since udbg is only for debugging, and is marked as potentially dangerous,
> how about just using the end of muram (as described in the device tree)? 
> If the muram is fully allocated, it won't happen until after the real
> serial console is initialized.

Very good idea. I'll prepare a new patch.

-- 
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: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.

[Laurent Pinchart]

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

On Friday 28 March 2008 14:58, Laurent Pinchart wrote:
> On Thursday 27 March 2008 16:39, Scott Wood wrote:
> > On Thu, Mar 27, 2008 at 10:10:33AM +0100, Laurent Pinchart wrote:
> > > On Wednesday 26 March 2008 17:59, Scott Wood wrote:
> > > > This breaks the bootwrapper console.
> > > 
> > > And of course I forgot about that :-)
> > > 
> > > The boot wrapper code doesn't have any dpram allocator. Any objection 
against 
> > > using a chunk of dpram at a fixed location ? What about at the beginning 
of 
> > > the dpram ? The DTS muram node would then exclude a chunk of dpram at 
offset 
> > > 0x0000 instead of 0x1100.
> > 
> > I'm not entirely comfortable with using a chunk outside of what's in the
> > muram node, and assuming that it's for the SMC pram -- what if there's
> > microcode or something there?
> > 
> > Since udbg is only for debugging, and is marked as potentially dangerous,
> > how about just using the end of muram (as described in the device tree)? 
> > If the muram is fully allocated, it won't happen until after the real
> > serial console is initialized.
> 
> Very good idea. I'll prepare a new patch.

arch/powerpc/boot/cpm-serial.c stores the udbg buffer descriptors at the 
beginning of the muram. Should I move them at the end as well ?

-- 
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: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.

[Scott Wood]

On Fri, Mar 28, 2008 at 03:06:58PM +0100, Laurent Pinchart wrote:
> arch/powerpc/boot/cpm-serial.c stores the udbg buffer descriptors at the 
> beginning of the muram. Should I move them at the end as well ?

Sure; make sure to update the default descriptor addresses in the
Kconfig.

-Scott

Re: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.

[Laurent Pinchart]

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

On Thursday 27 March 2008 16:39, Scott Wood wrote:
> On Thu, Mar 27, 2008 at 10:10:33AM +0100, Laurent Pinchart wrote:
> > On Wednesday 26 March 2008 17:59, Scott Wood wrote:
> > > This breaks the bootwrapper console.
> > 
> > And of course I forgot about that :-)
> > 
> > The boot wrapper code doesn't have any dpram allocator. Any objection
> > against using a chunk of dpram at a fixed location ? What about at the
> > beginning of the dpram ? The DTS muram node would then exclude a chunk of
> > dpram at offset 0x0000 instead of 0x1100.
> 
> I'm not entirely comfortable with using a chunk outside of what's in the
> muram node, and assuming that it's for the SMC pram -- what if there's
> microcode or something there?
> 
> Since udbg is only for debugging, and is marked as potentially dangerous,
> how about just using the end of muram (as described in the device tree)? 
> If the muram is fully allocated, it won't happen until after the real
> serial console is initialized.

Locating the end of the muram isn't as straightforward as it could be. As the 
current code already uses the beginning of the muram to store the BDs and 
data buffers, should I really bother locating the end or can I store the SMC 
parameter ram at the beginning as well ?

If I'm not mistaken, once the SMC parameter ram gets relocated to the 
beginning/end of the muram, the boot loader preallocated space can be 
reclaimed and can be added to the muram in the device tree like I did in my 
previous patch. Is that correct ?

-- 
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: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.

[Scott Wood]

Laurent Pinchart wrote:
> Locating the end of the muram isn't as straightforward as it could be. As the 
> current code already uses the beginning of the muram to store the BDs and 
> data buffers, should I really bother locating the end or can I store the SMC 
> parameter ram at the beginning as well ?

Maybe, but the end would be safer.  What's the problem with finding the 
end?  Even the end of the first reg resource would be OK.

> If I'm not mistaken, once the SMC parameter ram gets relocated to the 
> beginning/end of the muram, the boot loader preallocated space can be 
> reclaimed and can be added to the muram in the device tree like I did in my 
> previous patch. Is that correct ?

Yes.

-Scott

Re: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.

[Laurent Pinchart]

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

On Friday 28 March 2008 18:11, Scott Wood wrote:
> Laurent Pinchart wrote:
> > Locating the end of the muram isn't as straightforward as it could be. As
> > the current code already uses the beginning of the muram to store the BDs
> > and data buffers, should I really bother locating the end or can I store
> > the SMC parameter ram at the beginning as well ?
> 
> Maybe, but the end would be safer.  What's the problem with finding the 
> end?

That requires manual parsing of all the cells in the reg property. The 
device-tree API doesn't provide a way to get the length of a property, so 
I'll have to use a big enough pre-allocated buffer. I'm also not sure if 
resources are guaranteed to be sorted in increasing order.

This doesn't make finding the end of the muram really difficult. I was just 
wondering if the increased code complexity was worth it, especially seeing 
how the cpm_serial code in the boot wrapper seem quite unstable.

I'm not familiar with the boot wrapper code so I'm sometimes not very 
confident in my assumptions, but isn't the handling of the virtual-reg 
property in cpm_console_init broken ?

        void *reg_virt[2];

	...

        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 (is_smc)
                smc = reg_virt[0];
        else
                scc = reg_virt[0];

        param = reg_virt[1];

If I'm not mistaken, getprop will return the address and size of the first 
resource and not the addresses of the first two resources. What is 
virtual-reg used for ? To report the virtual address without requiring a 
device tree walk ? Does it provide any information that dt_xlate_reg can't 
find ?

> Even the end of the first reg resource would be OK. 

If I use the end of the first resource, can I assume it spans 0x0000 - 0x8000 
to set the default tx BD address in Kconfig ?

> > If I'm not mistaken, once the SMC parameter ram gets relocated to the 
> > beginning/end of the muram, the boot loader preallocated space can be 
> > reclaimed and can be added to the muram in the device tree like I did in
> > my previous patch. Is that correct ?
> 
> Yes.

-- 
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: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.

[Scott Wood]

Laurent Pinchart wrote:
> On Friday 28 March 2008 18:11, Scott Wood wrote:
>> Laurent Pinchart wrote:
>>> Locating the end of the muram isn't as straightforward as it
>>> could be. As the current code already uses the beginning of the
>>> muram to store the BDs and data buffers, should I really bother
>>> locating the end or can I store the SMC parameter ram at the
>>> beginning as well ?
>> Maybe, but the end would be safer.  What's the problem with finding
>> the end?
> 
> That requires manual parsing of all the cells in the reg property.
> The device-tree API doesn't provide a way to get the length of a
> property,

Sure it does.  Do a getprop with an insufficiently large buffer, and it
tells you how much you really need. :-)

> so I'll have to use a big enough pre-allocated buffer. I'm also not
> sure if resources are guaranteed to be sorted in increasing order.

Ah, good point.

> This doesn't make finding the end of the muram really difficult. I
> was just wondering if the increased code complexity was worth it,
> especially seeing how the cpm_serial code in the boot wrapper seem
> quite unstable.

Unstable in what way?

> I'm not familiar with the boot wrapper code so I'm sometimes not very
>  confident in my assumptions, but isn't the handling of the
> virtual-reg property in cpm_console_init broken ?

Not as far as I can see.

> If I'm not mistaken, getprop will return the address and size of the
> first resource and not the addresses of the first two resources.

No, it'll get as much of the virtual-reg property as will fit in the 
buffer.  There's no size in virtual-reg.

> What is virtual-reg used for ? To report the virtual address without
> requiring a device tree walk ? Does it provide any information that
> dt_xlate_reg can't find ?

Yes, it tells you the virtual address when it's not an identity mapping. 
  It's not currently used on CPM platforms, but might be used down the 
road with a QE device on 85xx.

>> Even the end of the first reg resource would be OK.
> 
> If I use the end of the first resource, can I assume it spans 0x0000
> - 0x8000 to set the default tx BD address in Kconfig ?

No, especially seeing as it doesn't on any existing boards. :-)

You could set the default to just before 0x2000 with board-specific 
exceptions, though.

-Scott

Re: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.

[Laurent Pinchart]

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

On Friday 28 March 2008 19:07, Scott Wood wrote:
> Laurent Pinchart wrote:
> > On Friday 28 March 2008 18:11, Scott Wood wrote:
> >> Laurent Pinchart wrote:
> >>> Locating the end of the muram isn't as straightforward as it
> >>> could be. As the current code already uses the beginning of the
> >>> muram to store the BDs and data buffers, should I really bother
> >>> locating the end or can I store the SMC parameter ram at the
> >>> beginning as well ?
> >> Maybe, but the end would be safer.  What's the problem with finding
> >> the end?
> > 
> > That requires manual parsing of all the cells in the reg property.
> > The device-tree API doesn't provide a way to get the length of a
> > property,
> 
> Sure it does.  Do a getprop with an insufficiently large buffer, and it
> tells you how much you really need. :-)

Ok thanks.

> > so I'll have to use a big enough pre-allocated buffer. I'm also not
> > sure if resources are guaranteed to be sorted in increasing order.
> 
> Ah, good point.
> 
> > This doesn't make finding the end of the muram really difficult. I
> > was just wondering if the increased code complexity was worth it,
> > especially seeing how the cpm_serial code in the boot wrapper seem
> > quite unstable.
> 
> Unstable in what way?

I was refering to the virtual-reg (non-)issue I mentionned below.

> > I'm not familiar with the boot wrapper code so I'm sometimes not very
> >  confident in my assumptions, but isn't the handling of the
> > virtual-reg property in cpm_console_init broken ?
> 
> Not as far as I can see.
> 
> > If I'm not mistaken, getprop will return the address and size of the
> > first resource and not the addresses of the first two resources.
> 
> No, it'll get as much of the virtual-reg property as will fit in the 
> buffer.  There's no size in virtual-reg.

Ah right. Sorry about the misunderstanding.

> > What is virtual-reg used for ? To report the virtual address without
> > requiring a device tree walk ? Does it provide any information that
> > dt_xlate_reg can't find ?
> 
> Yes, it tells you the virtual address when it's not an identity mapping. 
>   It's not currently used on CPM platforms, but might be used down the 
> road with a QE device on 85xx.

Will the virtual-reg property on the muram node list the addresses of all 
muram chunks or the address of the first chunk only ?

> >> Even the end of the first reg resource would be OK.
> > 
> > If I use the end of the first resource, can I assume it spans 0x0000
> > - 0x8000 to set the default tx BD address in Kconfig ?
> 
> No, especially seeing as it doesn't on any existing boards. :-)

I still need a default value :-) It obviously won't work for all boards.

> You could set the default to just before 0x2000 with board-specific 
> exceptions, though.

We're getting a bit lost. I'll try to summarize the discussion.

- The muram node has a reg property that lists the offsets and sizes of all 
muram chunks, and an optional virtual-reg property that lists the virtual 
address of all chunks/the first chunk only.

- From the above information I can locate a section of muram at the end of the 
first chunk (easy) or at the end of the muram (not really difficult, just a 
bit more complex, especially if chunks are not sorted by their start 
address).

- Kconfig needs a default address for the tx BD. This depends on the 
allocation strategy (end of first chunk vs. end of last chunk). Is there some 
consistent default across QE devices ?

-- 
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: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.

[Scott Wood]

On Mon, Mar 31, 2008 at 11:08:58AM +0200, Laurent Pinchart wrote:
> On Friday 28 March 2008 19:07, Scott Wood wrote:
> > Yes, it tells you the virtual address when it's not an identity mapping. 
> >   It's not currently used on CPM platforms, but might be used down the 
> > road with a QE device on 85xx.
> 
> Will the virtual-reg property on the muram node list the addresses of all 
> muram chunks or the address of the first chunk only ?

It should list all the chunks.  If you want the size of each chunk, just
look at the reg property.

> > >> Even the end of the first reg resource would be OK.
> > > 
> > > If I use the end of the first resource, can I assume it spans 0x0000
> > > - 0x8000 to set the default tx BD address in Kconfig ?
> > 
> > No, especially seeing as it doesn't on any existing boards. :-)
> 
> I still need a default value :-) It obviously won't work for all boards.

Just before 0x8000 won't work for any board, because that area is
reserved on CPM2.

> > You could set the default to just before 0x2000 with board-specific 
> > exceptions, though.
> 
> We're getting a bit lost. I'll try to summarize the discussion.
> 
> - The muram node has a reg property that lists the offsets and sizes of all 
> muram chunks, and an optional virtual-reg property that lists the virtual 
> address of all chunks/the first chunk only.
> 
> - From the above information I can locate a section of muram at the end of the 
> first chunk (easy) or at the end of the muram (not really difficult, just a 
> bit more complex, especially if chunks are not sorted by their start 
> address).
> 
> - Kconfig needs a default address for the tx BD. This depends on the 
> allocation strategy (end of first chunk vs. end of last chunk). Is there some 
> consistent default across QE devices ?

0x2000 minus sizeof(...) would be a good default for CPM1 and CPM2 (8280
has its first chunk go up to 0x4000, but for some reason that didn't get
reflected in the dts for the one 8280 board in-tree).

-Scott

RE: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in thedevice tree.

[Rune Torgersen]

Scott Wood wrote:
> 0x2000 minus sizeof(...) would be a good default for CPM1 and CPM2
> (8280 has its first chunk go up to 0x4000, but for some reason that
> didn't get reflected in the dts for the one 8280 board in-tree).

Except that last time I tested it, it is not from 0 - 0x4000, but the
extra 0x2000 is added from offset 0x9000.....
So 8280 has available muram from 0 - 0x2000 and 0x9000 - 0xb000

Re: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in thedevice tree.

[Scott Wood]

Rune Torgersen wrote:
> Scott Wood wrote:
>> 0x2000 minus sizeof(...) would be a good default for CPM1 and CPM2
>> (8280 has its first chunk go up to 0x4000, but for some reason that
>> didn't get reflected in the dts for the one 8280 board in-tree).
> 
> Except that last time I tested it, it is not from 0 - 0x4000, but the
> extra 0x2000 is added from offset 0x9000.....
> So 8280 has available muram from 0 - 0x2000 and 0x9000 - 0xb000

According to the docs, it has 0 - 0x4000 and 0x9000 - 0xc000.

-Scott

RE: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in thedevice tree.

[Rune Torgersen]

Scott Wood wrote:
> Rune Torgersen wrote:
>> Scott Wood wrote:
>>> 0x2000 minus sizeof(...) would be a good default for CPM1 and CPM2
>>> (8280 has its first chunk go up to 0x4000, but for some reason that
>>> didn't get reflected in the dts for the one 8280 board in-tree).
>>=20
>> Except that last time I tested it, it is not from 0 - 0x4000, but the
>> extra 0x2000 is added from offset 0x9000.....
>> So 8280 has available muram from 0 - 0x2000 and 0x9000 - 0xb000
>=20
> According to the docs, it has 0 - 0x4000 and 0x9000 - 0xc000.

I tried it on out 8280 board inhouse, and any addresses from 0x2000 to
0x3fff does not work with at least the MCC's.
(running ss7 with the extended ss7 microcode)
If I only used 0-0x2000 and 0x9000 to 0xB000 then it is happy.