CPM_UART should allocate DPRAM for SMCx parameter RAM on MPC82xx

CPM_UART should allocate DPRAM for SMCx parameter RAM on MPC82xx

[Laurent Pinchart]

Hi everybody,

the move to platform devices introduced a bug in the CPM_UART SMC driver.

Unlike SCC and FCC, the SMC parameter ram is not at a fixed location. A 
pointer to the parameter ram is instead stored at PROFF_SMCx_BASE.

The SMC platform device resources (in arch/ppc/syslib/pq2_devices.c) reserves 
the memory resource PROFF_SMCx_BASE - PROFF_SMCx_BASE+1. The CPM UART driver 
considers that value as the SMC parameter ram offset. It should instead 
allocate 64 bytes (on a 64 bytes boundary) of DPRAM for its parameter ram, 
and store the offset at PROFF_SMCx_BASE. 

I'm not sure how to fix the problem as it seems that the platform device 
support is not complete yet (cpm_uart_init_portdesc is still used to 
initialize the console). A possible workaround is to set the pram resource to 
0x0000-0x003f and 0x0040-0x007f instead of 0x87fc-0x87fd and 0x88fc-0x88fd 
for SMC1 and SMC2. This will not work if cpm_uart_init_portdesc is not 
called.

Laurent Pinchart

Re: CPM_UART should allocate DPRAM for SMCx parameter RAM on MPC82xx

[Vitaly Bordug]

On Wed, 7 Jun 2006 14:34:08 +0200
Laurent Pinchart <laurent.pinchart@tbox.biz> wrote:

> Hi everybody,
> 
> the move to platform devices introduced a bug in the CPM_UART SMC driver.
> 
> Unlike SCC and FCC, the SMC parameter ram is not at a fixed location. A 
> pointer to the parameter ram is instead stored at PROFF_SMCx_BASE.
> 
> The SMC platform device resources (in arch/ppc/syslib/pq2_devices.c) reserves 
> the memory resource PROFF_SMCx_BASE - PROFF_SMCx_BASE+1. The CPM UART driver 
> considers that value as the SMC parameter ram offset. It should instead 
> allocate 64 bytes (on a 64 bytes boundary) of DPRAM for its parameter ram, 
> and store the offset at PROFF_SMCx_BASE. 
> 
> I'm not sure how to fix the problem as it seems that the platform device 
> support is not complete yet (cpm_uart_init_portdesc is still used to 
> initialize the console). A possible workaround is to set the pram resource to 
> 0x0000-0x003f and 0x0040-0x007f instead of 0x87fc-0x87fd and 0x88fc-0x88fd 
> for SMC1 and SMC2. This will not work if cpm_uart_init_portdesc is not 
> called.
> 

No, pdev support for cpm uart is complete and works for every board I have handy, both 8xx and 82xx.
8xx use SMC stuff and are ok, so I suppose the issue got inside because this is first case we head with combination of pq2/smc. 

init_portedsc is called in compatibility mode only, when driver was
unable to locate platform device (early_uart_get_pdev call). The
offsets are definitely odd - we should count the proper values (as it
was done for 8xx - offset from immr where pram will be located that
is). So, pq2_devices should be fixed, as well as bsp code should
provide early_uart_get_pdev call (see 8272 as reference).

-- 
Sincerely, 
Vitaly

Re: CPM_UART should allocate DPRAM for SMCx parameter RAM on MPC82xx

[Laurent Pinchart]

> > the move to platform devices introduced a bug in the CPM_UART SMC drive=
r.
> >
> > Unlike SCC and FCC, the SMC parameter ram is not at a fixed location. A
> > pointer to the parameter ram is instead stored at PROFF_SMCx_BASE.
> >
> > The SMC platform device resources (in arch/ppc/syslib/pq2_devices.c)
> > reserves the memory resource PROFF_SMCx_BASE - PROFF_SMCx_BASE+1. The C=
PM
> > UART driver considers that value as the SMC parameter ram offset. It
> > should instead allocate 64 bytes (on a 64 bytes boundary) of DPRAM for
> > its parameter ram, and store the offset at PROFF_SMCx_BASE.
> >
> > I'm not sure how to fix the problem as it seems that the platform device
> > support is not complete yet (cpm_uart_init_portdesc is still used to
> > initialize the console). A possible workaround is to set the pram
> > resource to 0x0000-0x003f and 0x0040-0x007f instead of 0x87fc-0x87fd and
> > 0x88fc-0x88fd for SMC1 and SMC2. This will not work if
> > cpm_uart_init_portdesc is not called.
>
> No, pdev support for cpm uart is complete and works for every board I have
> handy, both 8xx and 82xx. 8xx use SMC stuff and are ok, so I suppose the
> issue got inside because this is first case we head with combination of
> pq2/smc.
>
> init_portedsc is called in compatibility mode only, when driver was
> unable to locate platform device (early_uart_get_pdev call). The
> offsets are definitely odd - we should count the proper values (as it
> was done for 8xx - offset from immr where pram will be located that
> is). So, pq2_devices should be fixed, as well as bsp code should
> provide early_uart_get_pdev call (see 8272 as reference).

=46rom my understanding, SMC pram should be allocated, and its offset shoul=
d be=20
stored at PROFF_SMCx_BASE. pq2_devices returns the PRFF_SMCx_BASE resource,=
=20
and the CPM UART driver should use cpm_dpalloc to allocate the parameter RA=
M.=20
Is that right ? If so, where should I allocate memory (and where should I=20
free it) in the CPM UART driver ?

Laurent Pinchart

Re: CPM_UART should allocate DPRAM for SMCx parameter RAM on MPC82xx

[Vitaly Bordug]

On Wed, 7 Jun 2006 15:39:57 +0200
Laurent Pinchart <laurent.pinchart@tbox.biz> wrote:

> > > the move to platform devices introduced a bug in the CPM_UART SMC driver.
> > >
> > > Unlike SCC and FCC, the SMC parameter ram is not at a fixed location. A
> > > pointer to the parameter ram is instead stored at PROFF_SMCx_BASE.
> > >
> > > The SMC platform device resources (in arch/ppc/syslib/pq2_devices.c)
> > > reserves the memory resource PROFF_SMCx_BASE - PROFF_SMCx_BASE+1. The CPM
> > > UART driver considers that value as the SMC parameter ram offset. It
> > > should instead allocate 64 bytes (on a 64 bytes boundary) of DPRAM for
> > > its parameter ram, and store the offset at PROFF_SMCx_BASE.
> > >
> > > I'm not sure how to fix the problem as it seems that the platform device
> > > support is not complete yet (cpm_uart_init_portdesc is still used to
> > > initialize the console). A possible workaround is to set the pram
> > > resource to 0x0000-0x003f and 0x0040-0x007f instead of 0x87fc-0x87fd and
> > > 0x88fc-0x88fd for SMC1 and SMC2. This will not work if
> > > cpm_uart_init_portdesc is not called.
> >
> > No, pdev support for cpm uart is complete and works for every board I have
> > handy, both 8xx and 82xx. 8xx use SMC stuff and are ok, so I suppose the
> > issue got inside because this is first case we head with combination of
> > pq2/smc.
> >
> > init_portedsc is called in compatibility mode only, when driver was
> > unable to locate platform device (early_uart_get_pdev call). The
> > offsets are definitely odd - we should count the proper values (as it
> > was done for 8xx - offset from immr where pram will be located that
> > is). So, pq2_devices should be fixed, as well as bsp code should
> > provide early_uart_get_pdev call (see 8272 as reference).
> 
> From my understanding, SMC pram should be allocated, and its offset should be 
> stored at PROFF_SMCx_BASE. pq2_devices returns the PRFF_SMCx_BASE resource, 
> and the CPM UART driver should use cpm_dpalloc to allocate the parameter RAM. 
> Is that right ? If so, where should I allocate memory (and where should I 
> free it) in the CPM UART driver ?
> 
As I said, SMC offsets relative to pram things are screwed for PQ2.
That needs to be fixed. It is assumed, that for each SMC ready-for-use offset should be defined in pram resource (as for 8xx). There is no need to dpalloc/dpfree because the idea is that pram thing should be fixed to prevent confusion. So the odd offset for SMC devices in pq2_devices.c should be replaced with correct values (which should be dpram_start+PROFF_SMCx - have a look at 8xx as reference)

-- 
Sincerely, 
Vitaly

Re: *** PROBABLY SPAM *** Re: CPM_UART should allocate DPRAM for SMCx parameter RAM on MPC82xx

[Laurent Pinchart]

> As I said, SMC offsets relative to pram things are screwed for PQ2.
> That needs to be fixed. It is assumed, that for each SMC ready-for-use
> offset should be defined in pram resource (as for 8xx). There is no need to
> dpalloc/dpfree because the idea is that pram thing should be fixed to
> prevent confusion. So the odd offset for SMC devices in pq2_devices.c
> should be replaced with correct values (which should be
> dpram_start+PROFF_SMCx - have a look at 8xx as reference)

Ok. dpram[PROFF_SMCx_BASE] must still be set to the values returned by 
platform_get_resource_byname(). Where should that be done ?

Laurent Pinchart

Re: CPM_UART should allocate DPRAM for SMCx parameter RAM on MPC82xx

[Dan Malek]

On Jun 7, 2006, at 8:34 AM, Laurent Pinchart wrote:

> I'm not sure how to fix the problem

They way this _used_ to work is the DPRAM memory allocator
knew the SMCs had allocated the lower 128 bytes of DPRAM and
it never would allocate this space to anyone else (and it should not
free it either).  The constant #defined values for the SMC base
address would work just fine and could be assigned as part of
the SMC uart initialization.  Due to the alignment restrictions and
no reason to relocate this space anywhere else, this still seems
to be the most efficient method.


	-- Dan

Re: CPM_UART should allocate DPRAM for SMCx parameter RAM on MPC82xx

[Laurent Pinchart]

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

Hi Vitaly,

> If and when you'll have some code that address your problem, submit it
> to me cc linuxppc-embedded, and I'll push it upstream.

here's a simple patch (against 2.6.17-rc6) that fixes the issue by modifying 
the pram addresses for SMC resources. I'm not completely satisfied, as the 
SMCx_BASE registers configuration is board-specific (must be handled in 
init_ioports as you mentionned in your previous e-mail), which will lead to 
duplicated code.

Laurent Pinchart

[-- Attachment #2: pq2_devices.diff --]
[-- Type: text/x-diff, Size: 1047 bytes --]

diff --git a/arch/ppc/syslib/pq2_devices.c b/arch/ppc/syslib/pq2_devices.c
index fefbc21..0a7082e 100644
--- a/arch/ppc/syslib/pq2_devices.c
+++ b/arch/ppc/syslib/pq2_devices.c
@@ -289,15 +289,15 @@ struct platform_device ppc_sys_platform_
 		.num_resources	 = 3,
 		.resource = (struct resource[]) {
 			{
-				.name	= "smc_mem",
+				.name	= "regs",
 				.start	= 0x11A80,
 				.end	= 0x11A8F,
 				.flags	= IORESOURCE_MEM,
 			},
 			{
-				.name	= "smc_pram",
-				.start	= 0x87fc,
-				.end	= 0x87fd,
+				.name	= "pram",
+				.start	= 0x0000,
+				.end	= 0x003f,
 				.flags	= IORESOURCE_MEM,
 			},
 			{
@@ -313,15 +313,15 @@ struct platform_device ppc_sys_platform_
 		.num_resources	 = 3,
 		.resource = (struct resource[]) {
 			{
-				.name	= "smc_mem",
+				.name	= "regs",
 				.start	= 0x11A90,
 				.end	= 0x11A9F,
 				.flags	= IORESOURCE_MEM,
 			},
 			{
-				.name	= "smc_pram",
-				.start	= 0x88fc,
-				.end	= 0x88fd,
+				.name	= "pram",
+				.start	= 0x0040,
+				.end	= 0x007f,
 				.flags	= IORESOURCE_MEM,
 			},
 			{