[PATCH] CPM_UART: Fix non-console initialisation (v2)

[PATCH] CPM_UART: Fix non-console initialisation (v2)

[Kalle Pokki]

This is a cleaned-up version of a patch sent on Oct 19th. The patch is 
fairly straightforward and does not affect other than the compat mode. As 
not having this patch renders the cpm_uart driver unusable with a frame 
buffer console, I think this should be included in 2.6.19.

---
The cpm_uart driver is initialised incorrectly, if there is a frame buffer
console, and CONFIG_SERIAL_CPM_CONSOLE is defined. The driver fails to 
call cpm_uart_init_portdesc() and set_lineif() in this case.

Signed-off-by: Kalle Pokki <kalle.pokki@iki.fi>
---
  drivers/serial/cpm_uart/cpm_uart.h      |    2 +-
  drivers/serial/cpm_uart/cpm_uart_core.c |   11 ++++++-----
  drivers/serial/cpm_uart/cpm_uart_cpm1.c |    2 +-
  drivers/serial/cpm_uart/cpm_uart_cpm2.c |    2 +-
  4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/serial/cpm_uart/cpm_uart.h b/drivers/serial/cpm_uart/cpm_uart.h
index 3b35cb7..19a6ffe 100644
--- a/drivers/serial/cpm_uart/cpm_uart.h
+++ b/drivers/serial/cpm_uart/cpm_uart.h
@@ -89,7 +89,7 @@ extern struct uart_cpm_port cpm_uart_por

  /* these are located in their respective files */
  void cpm_line_cr_cmd(int line, int cmd);
-int cpm_uart_init_portdesc(void);
+int __init 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 8f3b3e5..4047530 100644
--- a/drivers/serial/cpm_uart/cpm_uart_core.c
+++ b/drivers/serial/cpm_uart/cpm_uart_core.c
@@ -1349,11 +1349,10 @@ static int cpm_uart_init(void) {
  		pr_info("cpm_uart: WARNING: no UART devices found on platform bus!\n");
  		pr_info(
  		"cpm_uart: the driver will guess configuration, but this mode is no longer supported.\n");
-#ifndef CONFIG_SERIAL_CPM_CONSOLE
-		ret = cpm_uart_init_portdesc();
-		if (ret)
-			return ret;
-#endif
+
+		/* Don't run this again, if the console driver did it already */
+		if (cpm_uart_nr == 0)
+			cpm_uart_init_portdesc();

  		cpm_reg.nr = cpm_uart_nr;
  		ret = uart_register_driver(&cpm_reg);
@@ -1365,6 +1364,8 @@ #endif
  			int con = cpm_uart_port_map[i];
  			cpm_uart_ports[con].port.line = i;
  			cpm_uart_ports[con].port.flags = UPF_BOOT_AUTOCONF;
+			if (cpm_uart_ports[con].set_lineif)
+				cpm_uart_ports[con].set_lineif(&cpm_uart_ports[con]);
  			uart_add_one_port(&cpm_reg, &cpm_uart_ports[con].port);
  		}

diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm1.c b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
index 95afc37..08e55fd 100644
--- a/drivers/serial/cpm_uart/cpm_uart_cpm1.c
+++ b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
@@ -184,7 +184,7 @@ void cpm_uart_freebuf(struct uart_cpm_po
  }

  /* Setup any dynamic params in the uart desc */
-int cpm_uart_init_portdesc(void)
+int __init cpm_uart_init_portdesc(void)
  {
  	pr_debug("CPM uart[-]:init portdesc\n");

diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm2.c b/drivers/serial/cpm_uart/cpm_uart_cpm2.c
index ef3bb47..b392aea 100644
--- a/drivers/serial/cpm_uart/cpm_uart_cpm2.c
+++ b/drivers/serial/cpm_uart/cpm_uart_cpm2.c
@@ -252,7 +252,7 @@ void cpm_uart_freebuf(struct uart_cpm_po
  }

  /* Setup any dynamic params in the uart desc */
-int cpm_uart_init_portdesc(void)
+int __init cpm_uart_init_portdesc(void)
  {
  	pr_debug("CPM uart[-]:init portdesc\n");

-- 
1.4.1.1

Re: [PATCH] CPM_UART: Fix non-console initialisation (v2)

[Vitaly Bordug]

On Wed, 1 Nov 2006 15:08:13 +0200 (EET)
Kalle Pokki <kalle.pokki@iki.fi> wrote:

> This is a cleaned-up version of a patch sent on Oct 19th. The patch is 
> fairly straightforward and does not affect other than the compat mode. As 
> not having this patch renders the cpm_uart driver unusable with a frame 
> buffer console, I think this should be included in 2.6.19.
> 
Fine with me.

> ---
> The cpm_uart driver is initialised incorrectly, if there is a frame buffer
> console, and CONFIG_SERIAL_CPM_CONSOLE is defined. The driver fails to 
> call cpm_uart_init_portdesc() and set_lineif() in this case.
> 
> Signed-off-by: Kalle Pokki <kalle.pokki@iki.fi>
Signed-off-by: Vitaly Bordug <vbordug@ru.mvista.com>

> ---
>   drivers/serial/cpm_uart/cpm_uart.h      |    2 +-
>   drivers/serial/cpm_uart/cpm_uart_core.c |   11 ++++++-----
>   drivers/serial/cpm_uart/cpm_uart_cpm1.c |    2 +-
>   drivers/serial/cpm_uart/cpm_uart_cpm2.c |    2 +-
>   4 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/serial/cpm_uart/cpm_uart.h b/drivers/serial/cpm_uart/cpm_uart.h
> index 3b35cb7..19a6ffe 100644
> --- a/drivers/serial/cpm_uart/cpm_uart.h
> +++ b/drivers/serial/cpm_uart/cpm_uart.h
> @@ -89,7 +89,7 @@ extern struct uart_cpm_port cpm_uart_por
> 
>   /* these are located in their respective files */
>   void cpm_line_cr_cmd(int line, int cmd);
> -int cpm_uart_init_portdesc(void);
> +int __init 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 8f3b3e5..4047530 100644
> --- a/drivers/serial/cpm_uart/cpm_uart_core.c
> +++ b/drivers/serial/cpm_uart/cpm_uart_core.c
> @@ -1349,11 +1349,10 @@ static int cpm_uart_init(void) {
>   		pr_info("cpm_uart: WARNING: no UART devices found on platform bus!\n");
>   		pr_info(
>   		"cpm_uart: the driver will guess configuration, but this mode is no longer supported.\n");
> -#ifndef CONFIG_SERIAL_CPM_CONSOLE
> -		ret = cpm_uart_init_portdesc();
> -		if (ret)
> -			return ret;
> -#endif
> +
> +		/* Don't run this again, if the console driver did it already */
> +		if (cpm_uart_nr == 0)
> +			cpm_uart_init_portdesc();
> 
>   		cpm_reg.nr = cpm_uart_nr;
>   		ret = uart_register_driver(&cpm_reg);
> @@ -1365,6 +1364,8 @@ #endif
>   			int con = cpm_uart_port_map[i];
>   			cpm_uart_ports[con].port.line = i;
>   			cpm_uart_ports[con].port.flags = UPF_BOOT_AUTOCONF;
> +			if (cpm_uart_ports[con].set_lineif)
> +				cpm_uart_ports[con].set_lineif(&cpm_uart_ports[con]);
>   			uart_add_one_port(&cpm_reg, &cpm_uart_ports[con].port);
>   		}
> 
> diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm1.c b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
> index 95afc37..08e55fd 100644
> --- a/drivers/serial/cpm_uart/cpm_uart_cpm1.c
> +++ b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
> @@ -184,7 +184,7 @@ void cpm_uart_freebuf(struct uart_cpm_po
>   }
> 
>   /* Setup any dynamic params in the uart desc */
> -int cpm_uart_init_portdesc(void)
> +int __init cpm_uart_init_portdesc(void)
>   {
>   	pr_debug("CPM uart[-]:init portdesc\n");
> 
> diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm2.c b/drivers/serial/cpm_uart/cpm_uart_cpm2.c
> index ef3bb47..b392aea 100644
> --- a/drivers/serial/cpm_uart/cpm_uart_cpm2.c
> +++ b/drivers/serial/cpm_uart/cpm_uart_cpm2.c
> @@ -252,7 +252,7 @@ void cpm_uart_freebuf(struct uart_cpm_po
>   }
> 
>   /* Setup any dynamic params in the uart desc */
> -int cpm_uart_init_portdesc(void)
> +int __init cpm_uart_init_portdesc(void)
>   {
>   	pr_debug("CPM uart[-]:init portdesc\n");
> 
> -- 
> 1.4.1.1
> 
> 


-- 
Sincerely, 
Vitaly

Re: [PATCH] CPM_UART: Fix non-console initialisation (v2)

[Paul Mackerras]

Kalle Pokki writes:

> This is a cleaned-up version of a patch sent on Oct 19th. The patch is 
> fairly straightforward and does not affect other than the compat mode. As 
> not having this patch renders the cpm_uart driver unusable with a frame 
> buffer console, I think this should be included in 2.6.19.

The last hunk, i.e. the change to drivers/serial/cpm_uart/cpm_uart_cpm2.c,
doesn't apply because cpm_uart_init_portdesc now looks different to the
context in your patch, so I dropped that hunk (it looked to be pretty
much independent of the other changes).

Also, the whitespace is damaged throughout this and your other
patches.  I fixed it up this time, but please fix your mailer not to
munge the whitespace.  Specifically, where a line begins with a space
character, your mailer removes it if there is nothing else on the
line, or turns it into two spaces otherwise. :P

Paul.

Re: [PATCH] CPM_UART: Fix non-console initialisation (v2)

[Kalle Pokki]

On 11/10/06, Paul Mackerras <paulus@samba.org> wrote:
>
> The last hunk, i.e. the change to drivers/serial/cpm_uart/cpm_uart_cpm2.c,
> doesn't apply because cpm_uart_init_portdesc now looks different to the
> context in your patch, so I dropped that hunk (it looked to be pretty
> much independent of the other changes).

OK, better drop all the hunks except the one that concerns
drivers/serial/cpm_uart/cpm_uart_core.c then, since I added the same
'__init' lines in cpm_uart.h, cpm_uart_cpm1.c, and cpm_uart_cpm2.c.

> Also, the whitespace is damaged throughout this and your other
> patches.  I fixed it up this time, but please fix your mailer not to
> munge the whitespace.  Specifically, where a line begins with a space
> character, your mailer removes it if there is nothing else on the
> line, or turns it into two spaces otherwise. :P

I'll fix this. It seems I was sending the patches with pine making them flowed.

Re: [PATCH] CPM_UART: Fix non-console initialisation (v2)

[Kalle Pokki]

On Fri, 10 Nov 2006, Kalle Pokki wrote:

> OK, better drop all the hunks except the one that concerns
> drivers/serial/cpm_uart/cpm_uart_core.c then, since I added the same
> '__init' lines in cpm_uart.h, cpm_uart_cpm1.c, and cpm_uart_cpm2.c.

Hmm, it seems you already pushed the changes. This should apply to the 
current 'merge' branch.

 drivers/serial/cpm_uart/cpm_uart_cpm2.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm2.c b/drivers/serial/cpm_uart/cpm_uart_cpm2.c
index b691d3e..787a8f1 100644
--- a/drivers/serial/cpm_uart/cpm_uart_cpm2.c
+++ b/drivers/serial/cpm_uart/cpm_uart_cpm2.c
@@ -282,7 +282,7 @@ void cpm_uart_freebuf(struct uart_cpm_po
 }
 
 /* Setup any dynamic params in the uart desc */
-int cpm_uart_init_portdesc(void)
+int __init cpm_uart_init_portdesc(void)
 {
 #if defined(CONFIG_SERIAL_CPM_SMC1) || defined(CONFIG_SERIAL_CPM_SMC2)
 	u32 addr;
-- 
1.4.1.1

Re: [PATCH] CPM_UART: Fix non-console initialisation (v2)

[Paul Mackerras]

Kalle Pokki writes:

> Hmm, it seems you already pushed the changes. This should apply to the 
> current 'merge' branch.

That on its own is hard to justify as a bug fix, isn't it?

Paul.

Re: [PATCH] CPM_UART: Fix non-console initialisation (v2)

[Kalle Pokki]

On 11/11/06, Paul Mackerras <paulus@samba.org> wrote:
> Kalle Pokki writes:
>
> > Hmm, it seems you already pushed the changes. This should apply to the
> > current 'merge' branch.
>
> That on its own is hard to justify as a bug fix, isn't it?

Sure, but since you removed the last hunk of the patch that became
0091cf5a6ae6e52fc95ceb53200975ef2c81c206, we now have a little
inconsistent definitions:

in cpm_uart.h
int __init cpm_uart_init_portdesc(void);

in cpm_uart_cpm1.c
int __init cpm_uart_init_portdesc(void)
{
}

in cpm_uart_cpm2.c
int cpm_uart_init_portdesc(void)
{
}

Re: [PATCH] CPM_UART: Fix non-console initialisation (v2)

[Vitaly Bordug]

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

Paul,

patch with proper description inlined.

Thanks,
-Vitaly
---
[POWERPC] CPM_UART: Fixes inconsistency of function definition

From: Kalle Pokki <kalle.pokki@iki.fi>

The below hunk was missed from the recent patch, and now, there are somewhat 
inconsistent definitions:

in cpm_uart.h:
int __init cpm_uart_init_portdesc(void);

in cpm_uart_cpm1.c:
int __init cpm_uart_init_portdesc(void)
{
}

in cpm_uart_cpm2.c:
int cpm_uart_init_portdesc(void)
{
}

Signed-off-by: Kalle Pokki <kalle.pokki@iki.fi>
Signed-off-by: Vitaly Bordug <vbordug@ru.mvista.com>
---

 drivers/serial/cpm_uart/cpm_uart_cpm2.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm2.c b/drivers/serial/cpm_uart/cpm_uart_cpm2.c
index b691d3e..787a8f1 100644
--- a/drivers/serial/cpm_uart/cpm_uart_cpm2.c
+++ b/drivers/serial/cpm_uart/cpm_uart_cpm2.c
@@ -282,7 +282,7 @@ void cpm_uart_freebuf(struct uart_cpm_po
 }
 
 /* Setup any dynamic params in the uart desc */
-int cpm_uart_init_portdesc(void)
+int __init cpm_uart_init_portdesc(void)
 {
 #if defined(CONFIG_SERIAL_CPM_SMC1) || defined(CONFIG_SERIAL_CPM_SMC2)
 	u32 addr;

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

Re: [PATCH] CPM_UART: Fix non-console initialisation (v2)

[Paul Mackerras]

Vitaly Bordug writes:

> patch with proper description inlined.
> 
> Thanks,
> -Vitaly
> ---
> [POWERPC] CPM_UART: Fixes inconsistency of function definition
> 
> From: Kalle Pokki <kalle.pokki@iki.fi>
> 
> The below hunk was missed from the recent patch, and now, there are somewhat 
> inconsistent definitions:

Well, what I want to know is this: why was the addition of __init
bundled in with what I was told was a bug-fix that was needed for
2.6.19?  Does the addition of __init fix a bug somehow?

Paul.

Re: [PATCH] CPM_UART: Fix non-console initialisation (v2)

[Kalle Pokki]

On 11/13/06, Paul Mackerras <paulus@samba.org> wrote:
> Well, what I want to know is this: why was the addition of __init
> bundled in with what I was told was a bug-fix that was needed for
> 2.6.19?  Does the addition of __init fix a bug somehow?

That wasn't necessary, but the change was so small I thought we could
just as well do it at the same time when touching this function
anyway. (Although the second version of the patch didn't modify it
anymore, thus making the part adding __init actually larger in the
patch than the part having the bugfix.)

Re: [PATCH] CPM_UART: Fix non-console initialisation (v2)

[Vitaly Bordug]

On Mon, 13 Nov 2006 20:54:21 +1100
Paul Mackerras <paulus@samba.org> wrote:

> Vitaly Bordug writes:
> 
> > patch with proper description inlined.
> > 
> > Thanks,
> > -Vitaly
> > ---
> > [POWERPC] CPM_UART: Fixes inconsistency of function definition
> > 
> > From: Kalle Pokki <kalle.pokki@iki.fi>
> > 
> > The below hunk was missed from the recent patch, and now, there are somewhat 
> > inconsistent definitions:
> 
> Well, what I want to know is this: why was the addition of __init
> bundled in with what I was told was a bug-fix that was needed for
> 2.6.19?  Does the addition of __init fix a bug somehow?
> 
> 
That wasn't for .19 - I assumed your comment as -ENO_PROPER_DESCRIPTION.
The issue is small but shouldn't be put aside.


-- 
Sincerely, 
Vitaly