All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] CPM_UART: Fixed SMC handling for CPM2 processors
@ 2007-02-12 10:33 Heiko Schocher
  2007-02-12 17:55 ` Vitaly Bordug
  0 siblings, 1 reply; 10+ messages in thread
From: Heiko Schocher @ 2007-02-12 10:33 UTC (permalink / raw)
  To: linuxppc-embedded

Hello Vitaly,

I tried the Patch from Kalle Pokki
http://ozlabs.org/pipermail/linuxppc-embedded/2006-November/025108.html

but my SMC didnt work, without this patch, it works fine. I think that
the pram_base must be set the follwing way:

diff --git a/drivers/serial/cpm_uart/cpm_uart_core.c
b/drivers/serial/cpm_uart/cpm_uart_core.c
index ea85f6a..76ab6e5 100644
--- a/drivers/serial/cpm_uart/cpm_uart_core.c
+++ b/drivers/serial/cpm_uart/cpm_uart_core.c
@@ -1025,6 +1025,7 @@ int cpm_uart_drv_get_platform_data(struct
platform_device *pdev, int is_con)
 	struct uart_cpm_port *pinfo;
 	int line;
 	u32 mem, pram, pram_base;
+	int	base;
 
 	idx = pdata->fs_no = fs_uart_get_id(pdata);
 
@@ -1050,11 +1051,12 @@ int cpm_uart_drv_get_platform_data(struct
platform_device *pdev, int is_con)
 	if (!(r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pram")))
 		return -EINVAL;
 	pram = (u32)ioremap(r->start, r->end - r->start + 1);
-
+	base = r->start;
+	
 	r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pram_base");
 	if (r) {
 		pram_base = r->start;
-		out_be16((u16 *)pram_base, pram & 0xffff);
+		out_be16((u16 *)pram_base, base & 0xffff);
 	}
 
 	if(idx > fsid_smc2_uart) {


with this patch it works fine :-)

Best regards
Heiko

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] CPM_UART: Fixed SMC handling for CPM2 processors
  2007-02-12 10:33 [PATCH] CPM_UART: Fixed SMC handling for CPM2 processors Heiko Schocher
@ 2007-02-12 17:55 ` Vitaly Bordug
  2007-02-13  8:09   ` Heiko Schocher
  0 siblings, 1 reply; 10+ messages in thread
From: Vitaly Bordug @ 2007-02-12 17:55 UTC (permalink / raw)
  To: hs; +Cc: linuxppc-embedded

On Mon, 12 Feb 2007 11:33:42 +0100
Heiko Schocher <hs@denx.de> wrote:

> Hello Vitaly,
> 
> I tried the Patch from Kalle Pokki
> http://ozlabs.org/pipermail/linuxppc-embedded/2006-November/025108.html
> 
> but my SMC didnt work, without this patch, it works fine. I think that
> the pram_base must be set the follwing way:
> 
Thanks for the patch, but I guess it requires a bit of investigation first since 
we cannot just drop ioremapped offset and get back to raw one that has been returned by 
platform_get_....

We require ioremap stuff for powerpc approach, and I think we'd have to figure out why 
ioremapped address did not satisfy smc (and I'll try on my 8xx).

Guessing we are speaking about arch/ppc/ domain upper...

> diff --git a/drivers/serial/cpm_uart/cpm_uart_core.c
> b/drivers/serial/cpm_uart/cpm_uart_core.c
> index ea85f6a..76ab6e5 100644
> --- a/drivers/serial/cpm_uart/cpm_uart_core.c
> +++ b/drivers/serial/cpm_uart/cpm_uart_core.c
> @@ -1025,6 +1025,7 @@ int cpm_uart_drv_get_platform_data(struct
> platform_device *pdev, int is_con)
>  	struct uart_cpm_port *pinfo;
>  	int line;
>  	u32 mem, pram, pram_base;
> +	int	base;
>  
>  	idx = pdata->fs_no = fs_uart_get_id(pdata);
>  
> @@ -1050,11 +1051,12 @@ int cpm_uart_drv_get_platform_data(struct
> platform_device *pdev, int is_con)
>  	if (!(r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pram")))
>  		return -EINVAL;
>  	pram = (u32)ioremap(r->start, r->end - r->start + 1);
> -
> +	base = r->start;
> +	
>  	r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pram_base");
>  	if (r) {
>  		pram_base = r->start;
> -		out_be16((u16 *)pram_base, pram & 0xffff);
> +		out_be16((u16 *)pram_base, base & 0xffff);
>  	}
>  
>  	if(idx > fsid_smc2_uart) {
> 
> 
> with this patch it works fine :-)
> 
> Best regards
> Heiko
> 
> -- 
> DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
> Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
> 


-- 
Sincerely, 
Vitaly

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] CPM_UART: Fixed SMC handling for CPM2 processors
  2007-02-12 17:55 ` Vitaly Bordug
@ 2007-02-13  8:09   ` Heiko Schocher
  2007-02-13 11:42     ` Vitaly Bordug
  0 siblings, 1 reply; 10+ messages in thread
From: Heiko Schocher @ 2007-02-13  8:09 UTC (permalink / raw)
  To: Vitaly Bordug; +Cc: linuxppc-embedded

Hello Vitaly,

Vitaly Bordug wrote:
> > I tried the Patch from Kalle Pokki
> > http://ozlabs.org/pipermail/linuxppc-embedded/2006-November/025108.html
> > 
> > but my SMC didnt work, without this patch, it works fine. I think that
> > the pram_base must be set the follwing way:
> > 
> Thanks for the patch, but I guess it requires a bit of investigation first since 
> we cannot just drop ioremapped offset and get back to raw one that has been returned by 
> platform_get_....

Ok, "Linux" writes/reads with the ioremapped Address in the Dualported
RAM we get with platform_get_...("pram"), thats Okay, but the CPM needs
the "real" Dualported RAM Address from this Area at offset SMCx_BASE,
which we get with platform_get_...("pram_base"), so it can find the SMC
Parameter RAM, he(CPM) didnt know anything about ioremapped addresses.

> We require ioremap stuff for powerpc approach, and I think we'd have to figure out why 
> ioremapped address did not satisfy smc (and I'll try on my 8xx).

Hmm... you mean the following?:

  r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pram_base");
  if (r) {
+           pram_base = (u32)ioremap(r->start, r->end - r->start + 1);
            out_be16((u16 *)pram_base, base & 0xffff);
+           iounmap(pram_base);
         }

But thats not, what I correct with my patch, the error was, that
we wrote the ioremapped address pram in pram_base and not the real
address ... and with this ioremapped address, the CPM cannot do anything
(I think).

> Guessing we are speaking about arch/ppc/ domain upper...

Yes.

thanks
Heiko

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] CPM_UART: Fixed SMC handling for CPM2 processors
  2007-02-13  8:09   ` Heiko Schocher
@ 2007-02-13 11:42     ` Vitaly Bordug
  0 siblings, 0 replies; 10+ messages in thread
From: Vitaly Bordug @ 2007-02-13 11:42 UTC (permalink / raw)
  To: hs; +Cc: linuxppc-embedded

On Tue, 13 Feb 2007 09:09:47 +0100
Heiko Schocher wrote:

> Hello Vitaly,
> 
> Vitaly Bordug wrote:
> > > I tried the Patch from Kalle Pokki
> > > http://ozlabs.org/pipermail/linuxppc-embedded/2006-November/025108.html
> > > 
> > > but my SMC didnt work, without this patch, it works fine. I think
> > > that the pram_base must be set the follwing way:
> > > 
> > Thanks for the patch, but I guess it requires a bit of
> > investigation first since we cannot just drop ioremapped offset and
> > get back to raw one that has been returned by platform_get_....
> 
> Ok, "Linux" writes/reads with the ioremapped Address in the Dualported
> RAM we get with platform_get_...("pram"), thats Okay, but the CPM
> needs the "real" Dualported RAM Address from this Area at offset
> SMCx_BASE, which we get with platform_get_...("pram_base"), so it can
> find the SMC Parameter RAM, he(CPM) didnt know anything about
> ioremapped addresses.
> 
> > We require ioremap stuff for powerpc approach, and I think we'd
> > have to figure out why ioremapped address did not satisfy smc (and
> > I'll try on my 8xx).
> 
> Hmm... you mean the following?:
> 
>   r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pram_base");
>   if (r) {
> +           pram_base = (u32)ioremap(r->start, r->end - r->start + 1);
>             out_be16((u16 *)pram_base, base & 0xffff);
> +           iounmap(pram_base);
>          }
> 
> But thats not, what I correct with my patch, the error was, that
> we wrote the ioremapped address pram in pram_base and not the real
> address ... and with this ioremapped address, the CPM cannot do
> anything (I think).
> 

That is what I am about - I think there is a bit of confusion around the upper. I mean, we'll have to investigate
what's happening in arch/powerpc since the SMC CPM works fine for me in aprch/powerpc with the ioremap stuff. Maybe it has to do something with io_block_map() and the ioremap - I'm not sure. Well it worths looking at it, if it will be unclear we can end up with #ifdef that areas to keep ppc/ happy at least.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] CPM_UART: Fixed SMC handling for CPM2 processors
  2006-11-07 13:21       ` Kalle Pokki
@ 2006-11-07 14:47         ` Vitaly Bordug
  0 siblings, 0 replies; 10+ messages in thread
From: Vitaly Bordug @ 2006-11-07 14:47 UTC (permalink / raw)
  To: Kalle Pokki; +Cc: Paul, linuxppc-embedded, Mackerras

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

On Tue, 7 Nov 2006 15:21:00 +0200 (EET)
Kalle Pokki wrote:

> On Tue, 7 Nov 2006, Vitaly Bordug wrote:
> 
> > Well, yes, but are you _sure_ pram_base will be the same across all
> > the 82xx PQ2, that happen to have smc wired to Ethernet?
> >
> > If not I am considering storing it in the platform_data is better
> > approach.
> 
> Yes, pram_base is always 0x87fc for SMC1 and 0x88fc for SMC2. This is
> for all PowerQUICC II families (8260, 8272, and 8280). I'm not sure
> how PQ2 Pro and PQ3 and handled, but I suspect they don't share these 
> definitions.
> 
ok
> Anyway, I'm only extending the already existing conventions to the 
> platform device approach. These same decisions have already been made
> in the past and are used in the cpm_uart compat mode. It may be that 
> Freescale someday releases a microcode patch that relocates the SMC 
> parameter RAM, but even in this case it would be better to use the
> same approach with compat mode and platform device mode to avoid
> confusion.
> 
> I could have used the numerical address offsets in the resource 
> definition, but I wanted to emphasize the fact that the offsets are 
> already defined by the DPRAM memory allocator (this is a little
> hackish, yes) instead of hardware directly requiring these exact
> values.
> 

Aha, I recall now. There was nearly exactly the same discussion in the past. 
The recap was since ppc_platform_devices[] approach is not flexible enough, revisit issue from the 
arch/powerpc POV. 

> This snippet is from cpm2.h:
> 
>  	/* Dual Port RAM addresses.  The first 16K is available for
> almost
>  	 * any CPM use, so we put the BDs there.  The first 128
> bytes are
>  	 * used for SMC1 and SMC2 parameter RAM, so we start
> allocating
>  	 * BDs above that.  All of this must change when we start
>  	 * downloading RAM microcode.
>  	 */
>  	#define CPM_DATAONLY_BASE       ((uint)128)
> 
> My patch puts pram_base exactly here.
> 
> 

I know, the questionable thing was if there is enough "value" to add yet another platform device for that.

Since it improves current ppc being and sort of puts a note for powerpc port, I'm inclined to ACK.

Thanks,

-Vitaly

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] CPM_UART: Fixed SMC handling for CPM2 processors
  2006-11-07 12:08     ` Vitaly Bordug
@ 2006-11-07 13:21       ` Kalle Pokki
  2006-11-07 14:47         ` Vitaly Bordug
  0 siblings, 1 reply; 10+ messages in thread
From: Kalle Pokki @ 2006-11-07 13:21 UTC (permalink / raw)
  To: Vitaly Bordug; +Cc: linuxppc-embedded, Paul Mackerras

On Tue, 7 Nov 2006, Vitaly Bordug wrote:

> Well, yes, but are you _sure_ pram_base will be the same across all the 
> 82xx PQ2, that happen to have smc wired to Ethernet?
>
> If not I am considering storing it in the platform_data is better approach.

Yes, pram_base is always 0x87fc for SMC1 and 0x88fc for SMC2. This is
for all PowerQUICC II families (8260, 8272, and 8280). I'm not sure how 
PQ2 Pro and PQ3 and handled, but I suspect they don't share these 
definitions.

Anyway, I'm only extending the already existing conventions to the 
platform device approach. These same decisions have already been made in 
the past and are used in the cpm_uart compat mode. It may be that 
Freescale someday releases a microcode patch that relocates the SMC 
parameter RAM, but even in this case it would be better to use the same 
approach with compat mode and platform device mode to avoid confusion.

I could have used the numerical address offsets in the resource 
definition, but I wanted to emphasize the fact that the offsets are 
already defined by the DPRAM memory allocator (this is a little hackish, 
yes) instead of hardware directly requiring these exact values.

This snippet is from cpm2.h:

 	/* Dual Port RAM addresses.  The first 16K is available for almost
 	 * any CPM use, so we put the BDs there.  The first 128 bytes are
 	 * used for SMC1 and SMC2 parameter RAM, so we start allocating
 	 * BDs above that.  All of this must change when we start
 	 * downloading RAM microcode.
 	 */
 	#define CPM_DATAONLY_BASE       ((uint)128)

My patch puts pram_base exactly here.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] CPM_UART: Fixed SMC handling for CPM2 processors
  2006-11-06 20:49   ` Kalle Pokki
@ 2006-11-07 12:08     ` Vitaly Bordug
  2006-11-07 13:21       ` Kalle Pokki
  0 siblings, 1 reply; 10+ messages in thread
From: Vitaly Bordug @ 2006-11-07 12:08 UTC (permalink / raw)
  To: Kalle Pokki; +Cc: Paul Mackerras, linuxppc-embedded

On Mon, 6 Nov 2006 22:49:49 +0200 (EET)
Kalle Pokki <kalle.pokki@iki.fi> wrote:

> On Mon, 6 Nov 2006, Vitaly Bordug wrote:
> 
> >> This patch renames these the two existing resources, and introduces a
> >> new one, "pram_base", which is a pointer to the parameter RAM. The
> >> parameter RAM for SMC1 and SMC2 is put in the first 128 bytes of the
> >> DPRAM. This memory was already reserved from the DPRAM memory
> >> allocator for this purpose.
> >>
> > Well just one objection. pram_base should not be a device unless it applies to all the stuff of
> > SoC family which is not the case.
> >
> > For this aim, I'd put what you need into the platform_data, or follow the same approach 8xx stuff having.
> 
> I'm not sure I follow you now. "pram_base" is not a device by itself, but 
> it's just another resource in the PQ2 version of the SMC devices in the 
> platform_data. So the resource is only present when it is needed, and for 
> other platform devices that don't have this resource, the cpm_uart code 
> does nothing (as it should not).
> 
Well, yes, but are you _sure_ pram_base will be the same across all the 82xx PQ2, that happen to have smc wired to Ethernet? 

If not I am considering storing it in the platform_data is better approach.

> The only difference to the 8xx version is the new "pram_base" resource, 
> and it is required with PQ2, right?
Prolly, and I actually tried the same when introduced 8xx stuff. It was not permitted because such a thing could not be stuck to one value, and as long as we'll have it compiled into the devices table, others will have to use that offset (yet maybe having the specific dpram required for another purpose).

Hope that clarifies my position...
-- 
Sincerely, 
Vitaly

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] CPM_UART: Fixed SMC handling for CPM2 processors
  2006-11-06 17:55 ` Vitaly Bordug
@ 2006-11-06 20:49   ` Kalle Pokki
  2006-11-07 12:08     ` Vitaly Bordug
  0 siblings, 1 reply; 10+ messages in thread
From: Kalle Pokki @ 2006-11-06 20:49 UTC (permalink / raw)
  To: Vitaly Bordug; +Cc: linuxppc-embedded, Paul Mackerras

On Mon, 6 Nov 2006, Vitaly Bordug wrote:

>> This patch renames these the two existing resources, and introduces a
>> new one, "pram_base", which is a pointer to the parameter RAM. The
>> parameter RAM for SMC1 and SMC2 is put in the first 128 bytes of the
>> DPRAM. This memory was already reserved from the DPRAM memory
>> allocator for this purpose.
>>
> Well just one objection. pram_base should not be a device unless it applies to all the stuff of
> SoC family which is not the case.
>
> For this aim, I'd put what you need into the platform_data, or follow the same approach 8xx stuff having.

I'm not sure I follow you now. "pram_base" is not a device by itself, but 
it's just another resource in the PQ2 version of the SMC devices in the 
platform_data. So the resource is only present when it is needed, and for 
other platform devices that don't have this resource, the cpm_uart code 
does nothing (as it should not).

The only difference to the 8xx version is the new "pram_base" resource, 
and it is required with PQ2, right?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] CPM_UART: Fixed SMC handling for CPM2 processors
  2006-11-06 13:29 Kalle Pokki
@ 2006-11-06 17:55 ` Vitaly Bordug
  2006-11-06 20:49   ` Kalle Pokki
  0 siblings, 1 reply; 10+ messages in thread
From: Vitaly Bordug @ 2006-11-06 17:55 UTC (permalink / raw)
  To: Kalle Pokki; +Cc: Paul Mackerras, linuxppc-embedded

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

On Mon, 6 Nov 2006 15:29:47 +0200 (EET)
Kalle Pokki wrote:

> SMC handling is broken in two places when using then platform device
> approach with CPM2 devices.
> 
> 1. The resources in pq2_devices are not named "regs" and "pram", thus
> they are not found in cpm_uart_drv_get_platform_data().
> 
> 2. The code in cpm_uart_drv_get_platform_data() assumes the parameter
> RAM is at "pram". With SMCs of CPM2 devices, "pram" is just a pointer
> to the actual parameter RAM, which the code should reserve somewhere
> in DPRAM.
> 
Right, since I had nothing to probe with pq2/smc, that part of code remain intact since creation.
Nice to see this stuff tuned finally :)

> This patch renames these the two existing resources, and introduces a
> new one, "pram_base", which is a pointer to the parameter RAM. The
> parameter RAM for SMC1 and SMC2 is put in the first 128 bytes of the
> DPRAM. This memory was already reserved from the DPRAM memory
> allocator for this purpose.
> 
Well just one objection. pram_base should not be a device unless it applies to all the stuff of 
SoC family which is not the case.

For this aim, I'd put what you need into the platform_data, or follow the same approach 8xx stuff having.

Thanks, Vitaly

> Signed-off-by: Kalle Pokki <kalle.pokki@iki.fi>
> ---
>   arch/ppc/syslib/pq2_devices.c           |   24
> ++++++++++++++++++------ drivers/serial/cpm_uart/cpm_uart_core.c |
> 8 +++++++- 2 files changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/ppc/syslib/pq2_devices.c
> b/arch/ppc/syslib/pq2_devices.c index fefbc21..e8f7ba2 100644
> --- a/arch/ppc/syslib/pq2_devices.c
> +++ b/arch/ppc/syslib/pq2_devices.c
> @@ -286,16 +286,22 @@ struct platform_device ppc_sys_platform_
>   	[MPC82xx_CPM_SMC1] = {
>   		.name = "fsl-cpm-smc",
>   		.id	= 1,
> -		.num_resources	 = 3,
> +		.num_resources	 = 4,
>   		.resource = (struct resource[]) {
>   			{
> -				.name	= "smc_mem",
> +				.name	= "regs",
>   				.start	= 0x11A80,
>   				.end	= 0x11A8F,
>   				.flags	= IORESOURCE_MEM,
>   			},
>   			{
> -				.name	= "smc_pram",
> +				.name	= "pram",
> +				.start	= PROFF_SMC1,
> +				.end	= PROFF_SMC1 + 63,
> +				.flags	= IORESOURCE_MEM,
> +			},
> +			{
> +				.name	= "pram_base",
>   				.start	= 0x87fc,
>   				.end	= 0x87fd,
>   				.flags	= IORESOURCE_MEM,
> @@ -310,16 +316,22 @@ struct platform_device ppc_sys_platform_
>   	[MPC82xx_CPM_SMC2] = {
>   		.name = "fsl-cpm-smc",
>   		.id	= 2,
> -		.num_resources	 = 3,
> +		.num_resources	 = 4,
>   		.resource = (struct resource[]) {
>   			{
> -				.name	= "smc_mem",
> +				.name	= "regs",
>   				.start	= 0x11A90,
>   				.end	= 0x11A9F,
>   				.flags	= IORESOURCE_MEM,
>   			},
>   			{
> -				.name	= "smc_pram",
> +				.name	= "pram",
> +				.start	= PROFF_SMC2,
> +				.end	= PROFF_SMC2 + 63,
> +				.flags	= IORESOURCE_MEM,
> +			},
> +			{
> +				.name	= "pram_base",
>   				.start	= 0x88fc,
>   				.end	= 0x88fd,
>   				.flags	= IORESOURCE_MEM,
> diff --git a/drivers/serial/cpm_uart/cpm_uart_core.c
> b/drivers/serial/cpm_uart/cpm_uart_core.c index 4047530..55419b1
> 100644 --- a/drivers/serial/cpm_uart/cpm_uart_core.c
> +++ b/drivers/serial/cpm_uart/cpm_uart_core.c
> @@ -1024,7 +1024,7 @@ int cpm_uart_drv_get_platform_data(struc
>   	int idx = pdata->fs_no;	/* It is UART_SMCx or
> UART_SCCx index */ struct uart_cpm_port *pinfo;
>   	int line;
> -	u32 mem, pram;
> +	u32 mem, pram, pram_base;
> 
>   	line = cpm_uart_id2nr(idx);
>   	if(line < 0) {
> @@ -1049,6 +1049,12 @@ int cpm_uart_drv_get_platform_data(struc
>   		return -EINVAL;
>   	pram = r->start;
> 
> +	r = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "pram_base");
> +	if (r) {
> +		pram_base = r->start;
> +		out_be16((u16 *)pram_base, pram & 0xffff);
> +	}
> +
>   	if(idx > fsid_smc2_uart) {
>   		pinfo->sccp = (scc_t *)mem;
>   		pinfo->sccup = (scc_uart_t *)pram;

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] CPM_UART: Fixed SMC handling for CPM2 processors
@ 2006-11-06 13:29 Kalle Pokki
  2006-11-06 17:55 ` Vitaly Bordug
  0 siblings, 1 reply; 10+ messages in thread
From: Kalle Pokki @ 2006-11-06 13:29 UTC (permalink / raw)
  To: linuxppc-embedded, Vitaly Bordug, Paul Mackerras

SMC handling is broken in two places when using then platform device
approach with CPM2 devices.

1. The resources in pq2_devices are not named "regs" and "pram", thus they
    are not found in cpm_uart_drv_get_platform_data().

2. The code in cpm_uart_drv_get_platform_data() assumes the parameter RAM
    is at "pram". With SMCs of CPM2 devices, "pram" is just a pointer to the
    actual parameter RAM, which the code should reserve somewhere in DPRAM.

This patch renames these the two existing resources, and introduces a new one,
"pram_base", which is a pointer to the parameter RAM. The parameter RAM for SMC1
and SMC2 is put in the first 128 bytes of the DPRAM. This memory was already
reserved from the DPRAM memory allocator for this purpose.

Signed-off-by: Kalle Pokki <kalle.pokki@iki.fi>
---
  arch/ppc/syslib/pq2_devices.c           |   24 ++++++++++++++++++------
  drivers/serial/cpm_uart/cpm_uart_core.c |    8 +++++++-
  2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/arch/ppc/syslib/pq2_devices.c b/arch/ppc/syslib/pq2_devices.c
index fefbc21..e8f7ba2 100644
--- a/arch/ppc/syslib/pq2_devices.c
+++ b/arch/ppc/syslib/pq2_devices.c
@@ -286,16 +286,22 @@ struct platform_device ppc_sys_platform_
  	[MPC82xx_CPM_SMC1] = {
  		.name = "fsl-cpm-smc",
  		.id	= 1,
-		.num_resources	 = 3,
+		.num_resources	 = 4,
  		.resource = (struct resource[]) {
  			{
-				.name	= "smc_mem",
+				.name	= "regs",
  				.start	= 0x11A80,
  				.end	= 0x11A8F,
  				.flags	= IORESOURCE_MEM,
  			},
  			{
-				.name	= "smc_pram",
+				.name	= "pram",
+				.start	= PROFF_SMC1,
+				.end	= PROFF_SMC1 + 63,
+				.flags	= IORESOURCE_MEM,
+			},
+			{
+				.name	= "pram_base",
  				.start	= 0x87fc,
  				.end	= 0x87fd,
  				.flags	= IORESOURCE_MEM,
@@ -310,16 +316,22 @@ struct platform_device ppc_sys_platform_
  	[MPC82xx_CPM_SMC2] = {
  		.name = "fsl-cpm-smc",
  		.id	= 2,
-		.num_resources	 = 3,
+		.num_resources	 = 4,
  		.resource = (struct resource[]) {
  			{
-				.name	= "smc_mem",
+				.name	= "regs",
  				.start	= 0x11A90,
  				.end	= 0x11A9F,
  				.flags	= IORESOURCE_MEM,
  			},
  			{
-				.name	= "smc_pram",
+				.name	= "pram",
+				.start	= PROFF_SMC2,
+				.end	= PROFF_SMC2 + 63,
+				.flags	= IORESOURCE_MEM,
+			},
+			{
+				.name	= "pram_base",
  				.start	= 0x88fc,
  				.end	= 0x88fd,
  				.flags	= IORESOURCE_MEM,
diff --git a/drivers/serial/cpm_uart/cpm_uart_core.c b/drivers/serial/cpm_uart/cpm_uart_core.c
index 4047530..55419b1 100644
--- a/drivers/serial/cpm_uart/cpm_uart_core.c
+++ b/drivers/serial/cpm_uart/cpm_uart_core.c
@@ -1024,7 +1024,7 @@ int cpm_uart_drv_get_platform_data(struc
  	int idx = pdata->fs_no;	/* It is UART_SMCx or UART_SCCx index */
  	struct uart_cpm_port *pinfo;
  	int line;
-	u32 mem, pram;
+	u32 mem, pram, pram_base;

  	line = cpm_uart_id2nr(idx);
  	if(line < 0) {
@@ -1049,6 +1049,12 @@ int cpm_uart_drv_get_platform_data(struc
  		return -EINVAL;
  	pram = r->start;

+	r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pram_base");
+	if (r) {
+		pram_base = r->start;
+		out_be16((u16 *)pram_base, pram & 0xffff);
+	}
+
  	if(idx > fsid_smc2_uart) {
  		pinfo->sccp = (scc_t *)mem;
  		pinfo->sccup = (scc_uart_t *)pram;
-- 
1.4.1.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2007-02-13 11:42 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-02-12 10:33 [PATCH] CPM_UART: Fixed SMC handling for CPM2 processors Heiko Schocher
2007-02-12 17:55 ` Vitaly Bordug
2007-02-13  8:09   ` Heiko Schocher
2007-02-13 11:42     ` Vitaly Bordug
  -- strict thread matches above, loose matches on Subject: below --
2006-11-06 13:29 Kalle Pokki
2006-11-06 17:55 ` Vitaly Bordug
2006-11-06 20:49   ` Kalle Pokki
2006-11-07 12:08     ` Vitaly Bordug
2006-11-07 13:21       ` Kalle Pokki
2006-11-07 14:47         ` Vitaly Bordug

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.