[PATCH] CPM_UART: Fix non-console initialisation

[PATCH] CPM_UART: Fix non-console initialisation

[kalle.pokki]

If there is a frame buffer console, the cpm_uart_init_portdesc()
function is never called in the compat mode. Also, the set_lineif()
method is not called in the compat mode.

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 |    9 ++++-----
  drivers/serial/cpm_uart/cpm_uart_cpm1.c |    6 +++++-
  drivers/serial/cpm_uart/cpm_uart_cpm2.c |    6 +++++-
  4 files changed, 15 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..895f2f1 100644
--- a/drivers/serial/cpm_uart/cpm_uart_core.c
+++ b/drivers/serial/cpm_uart/cpm_uart_core.c
@@ -1349,11 +1349,8 @@ 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
+
+		cpm_uart_init_portdesc();

  		cpm_reg.nr = cpm_uart_nr;
  		ret = uart_register_driver(&cpm_reg);
@@ -1365,6 +1362,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..6f04a9f 100644
--- a/drivers/serial/cpm_uart/cpm_uart_cpm1.c
+++ b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
@@ -184,10 +184,14 @@ 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");

+	/* Don't run this again, if the console driver did it already */
+	if (cpm_uart_nr > 0)
+		return 0;
+
  	cpm_uart_nr = 0;
  #ifdef CONFIG_SERIAL_CPM_SMC1
  	cpm_uart_ports[UART_SMC1].smcp = &cpmp->cp_smc[0];
diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm2.c b/drivers/serial/cpm_uart/cpm_uart_cpm2.c
index ef3bb47..0f21543 100644
--- a/drivers/serial/cpm_uart/cpm_uart_cpm2.c
+++ b/drivers/serial/cpm_uart/cpm_uart_cpm2.c
@@ -252,10 +252,14 @@ 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");

+	/* Don't run this again, if the console driver did it already */
+	if (cpm_uart_nr > 0)
+		return 0;
+
  	cpm_uart_nr = 0;
  #ifdef CONFIG_SERIAL_CPM_SMC1
  	cpm_uart_ports[UART_SMC1].smcp = (smc_t *) & cpm2_immr->im_smc[0];
-- 
1.4.1.1

Re: [PATCH] CPM_UART: Fix non-console initialisation

[Vitaly Bordug]

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

On Thu, 19 Oct 2006 15:31:10 +0300 (EEST)
kalle.pokki@iki.fi wrote:

> If there is a frame buffer console, the cpm_uart_init_portdesc()
> function is never called in the compat mode. Also, the set_lineif()
> method is not called in the compat mode.
> 
for the fbconsole case, I agree we do have  to address it. But not the proposed way.
See below.
 

> 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 |    9 ++++-----
>   drivers/serial/cpm_uart/cpm_uart_cpm1.c |    6 +++++-
>   drivers/serial/cpm_uart/cpm_uart_cpm2.c |    6 +++++-
>   4 files changed, 15 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..895f2f1
> 100644 --- a/drivers/serial/cpm_uart/cpm_uart_core.c
> +++ b/drivers/serial/cpm_uart/cpm_uart_core.c
> @@ -1349,11 +1349,8 @@ 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
> +
> +		cpm_uart_init_portdesc();
> 
And we end up with init_portdesc called unconditionally? this would likely break, or at least confuse the
code, that uses platform dev to pass resources offsets.

>   		cpm_reg.nr = cpm_uart_nr;
>   		ret = uart_register_driver(&cpm_reg);
> @@ -1365,6 +1362,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..6f04a9f
> 100644 --- a/drivers/serial/cpm_uart/cpm_uart_cpm1.c
> +++ b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
> @@ -184,10 +184,14 @@ 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");
> 
> +	/* Don't run this again, if the console driver did it
> already */
> +	if (cpm_uart_nr > 0)
> +		return 0;
> +
>   	cpm_uart_nr = 0;
>   #ifdef CONFIG_SERIAL_CPM_SMC1
>   	cpm_uart_ports[UART_SMC1].smcp = &cpmp->cp_smc[0];
> diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm2.c
> b/drivers/serial/cpm_uart/cpm_uart_cpm2.c index ef3bb47..0f21543
> 100644 --- a/drivers/serial/cpm_uart/cpm_uart_cpm2.c
> +++ b/drivers/serial/cpm_uart/cpm_uart_cpm2.c
> @@ -252,10 +252,14 @@ 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");
> 
> +	/* Don't run this again, if the console driver did it
> already */
> +	if (cpm_uart_nr > 0)
> +		return 0;
> +

Personally, I don't care much of compat stuff, unless it is breaking/confusing targets 
using proper resources...

>   	cpm_uart_nr = 0;
>   #ifdef CONFIG_SERIAL_CPM_SMC1
>   	cpm_uart_ports[UART_SMC1].smcp = (smc_t *) &
> cpm2_immr->im_smc[0];

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

Re: [PATCH] CPM_UART: Fix non-console initialisation

[Kalle Pokki]

On 10/19/06, Vitaly Bordug <vbordug@ru.mvista.com> wrote:
> And we end up with init_portdesc called unconditionally? this would likely break, or at least confuse the
> code, that uses platform dev to pass resources offsets.

Well now that I look at it again, we should probably do

if (cpm_uart_no == 0)
     cpm_uart_init_portdesc();

to make it at least a little less confusing. Remember that
init_portdesc() is not called when using platform device, so this
should not affect those targets using it at all.

> Personally, I don't care much of compat stuff, unless it is breaking/confusing targets
> using proper resources...

I'm sure there are still lots of people using the compat mode. As long
as we have it, it should be kept functional.

Re: [PATCH] CPM_UART: Fix non-console initialisation

[Vitaly Bordug]

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

On Thu, 19 Oct 2006 22:40:27 +0300
Kalle Pokki wrote:

> On 10/19/06, Vitaly Bordug <vbordug@ru.mvista.com> wrote:
> > And we end up with init_portdesc called unconditionally? this would
> > likely break, or at least confuse the code, that uses platform dev
> > to pass resources offsets.
> 
> Well now that I look at it again, we should probably do
> 
> if (cpm_uart_no == 0)
>      cpm_uart_init_portdesc();
> 
> to make it at least a little less confusing. Remember that
> init_portdesc() is not called when using platform device, so this
> should not affect those targets using it at all.
> 
ok.
> > Personally, I don't care much of compat stuff, unless it is
> > breaking/confusing targets using proper resources...
> 
> I'm sure there are still lots of people using the compat mode. As long
> as we have it, it should be kept functional.
I'm not for removing it - just any update to the compat stuff should 
not touch platform-managed/ make it more  complex/confusing to deal with.

I believe it's cumbersome enough yet :-)


> 
> 
-Vitaly

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