uart.c: avoid changing parameter RAM on-the-fly for the console

uart.c: avoid changing parameter RAM on-the-fly for the console

[Steffen Rumler]

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

Hi,

We are using the 2.4.20 PPC kernel running on the TQM859T module.

A lot of this kind of modules always hangs during the boot,
something like this:

      Linux version 2.4.20-rthal5 (ru@styx) (gcc version 2.95.3 20010111 (prerelease/franzo/20010111))
      #31 Thu Feb 3 15:07:0
      1 CET 2005
      On node 0 totalpages: 8128
      zone(0): 8128 pages.
      zone(1): 0 pages.
      zone(2): 0 pages.
      ...
      CPM UART driver version 0.03+
      ttyS00 at 0x0280 is a SMC
                           ^^^^^
                             THE SYSTEM HANGS HERE :-(

Deeper inspecting the uart.c, we recognized that the SMC (or SCC)
hardware related to the console will be (re-)configured multiple times in:

    (1) serial_console_setup():  early setup with small BD tables

    (2) rs_8xx_init(): bigger BD tables; without interrupts

    (3) startup(): with interrupts

The re-configuring (1->2, 2->3) will be performed on-the-fly,
without disabling the hardware.

According to Motorola this is _NOT_ allowed and dangerous.

For the SMC, it is suggested to run a 'STOP TX' CPCR command followed
by disabling RX/TX in SMCMR (TEN/REN), in order to stop the controller
correctly.

I suggest to do this inside rs_8xx_init() and startup() as shown
in the patch below.
The SCC is not addressed by the patch.

With this fix, the modules hanging before can boot now.


Steffen

--







[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 2472 bytes --]

diff -Naur old/arch/ppc/8xx_io/uart.c new/arch/ppc/8xx_io/uart.c
--- old/arch/ppc/8xx_io/uart.c	Thu Feb  3 15:54:21 2005
+++ new/arch/ppc/8xx_io/uart.c	Thu Feb  3 15:52:44 2005
@@ -119,6 +119,10 @@
 static int  uart_buf_read_proc    (char *, char **, off_t, int, int *, void *);
 #endif
 
+#ifdef CONFIG_SERIAL_CONSOLE
+static void full_seq_smc_stop (int port);
+#endif
+
 /*
  * Serial driver configuration section.  Here are the various options:
  */
@@ -831,10 +835,17 @@
 	else {
 		smcp = &cpmp->cp_smc[idx];
 
+#ifdef CONFIG_SERIAL_CONSOLE
+                /*  stop SMC in the correct way, before re-configuring it
+                 */
+		if (((state - rs_table) == CONFIG_SERIAL_CONSOLE_PORT)){
+                    full_seq_smc_stop(PORT_NUM(info->state->smc_scc_num));
+                }
+#endif
+
 		/* Enable interrupts and I/O.
 		*/
 		smcp->smc_smcm |= (SMCM_RX | SMCM_TX);
-		smcp->smc_smcmr |= (SMCMR_REN | SMCMR_TEN);
 
 		/* We can tune the buffer length and idle characters
 		 * to take advantage of the entire incoming buffer size.
@@ -848,6 +859,8 @@
 		up->smc_mrblr = RX_BUF_SIZE;
 		up->smc_maxidl = RX_BUF_SIZE;
 		up->smc_brkcr = 1;	/* number of break chars */
+
+		smcp->smc_smcmr |= (SMCMR_REN | SMCMR_TEN);
 	}
 
 	info->flags |= ASYNC_INITIALIZED;
@@ -2694,6 +2707,30 @@
   
 } /* end uart_removeProcEntries() */
 
+
+#ifdef CONFIG_SERIAL_CONSOLE
+static void full_seq_smc_stop (int port)
+{
+    volatile cpm8xx_t *cp=cpmp;
+    volatile smc_t *sp;
+    ushort chan;
+    unsigned long flags;
+
+    sp   = &cp->cp_smc[port];
+    chan = smc_chan_map[port];
+
+    local_irq_save(flags);
+
+    while (cp->cp_cpcr & CPM_CR_FLG);
+    cp->cp_cpcr = mk_cr_cmd(chan, CPM_CR_STOP_TX) | CPM_CR_FLG;
+    while (cp->cp_cpcr & CPM_CR_FLG);
+    sp->smc_smcmr &= ~(SMCMR_REN | SMCMR_TEN);
+
+    local_irq_restore(flags);
+
+}  /* full_seq_smc_stop */ 
+#endif  /* CONFIG_SERIAL_CONSOLE */
+
 /*
  * The serial driver boot-time initialization code!
  */
@@ -2905,6 +2942,15 @@
 			}
 			else {
 				sp = &cp->cp_smc[idx];
+
+#ifdef CONFIG_SERIAL_CONSOLE
+                                /*  stop SMC in the correct way, before re-configuring it
+                                 */
+                                if (i == CONFIG_SERIAL_CONSOLE_PORT){
+                                    full_seq_smc_stop(idx);
+                                }
+#endif
+
 				up = (smc_uart_t *)&cp->cp_dparam[state->port];
 				up->smc_rbase = dp_addr;
 			}

Re: uart.c: avoid changing parameter RAM on-the-fly for the console

[Dan Malek]

On Feb 3, 2005, at 10:17 AM, Steffen Rumler wrote:

> According to Motorola this is _NOT_ allowed and dangerous.

:-)  yeah, ok ...

> I suggest to do this inside rs_8xx_init() and startup() as shown
> in the patch below.

would you please read Documentation/CodingStyle and
try sending the patch again?

Although it doesn't make too much difference in this case,
2.4.20 is pretty old and it helps to get patches against
the latest kernels.

> With this fix, the modules hanging before can boot now.

I suspect there is something else amiss, but I'll take a
look at it.  What is the speed of the processor and the
baud rate?

Thanks.

	-- Dan

Re: uart.c: avoid changing parameter RAM on-the-fly for the console

[Steffen Rumler]

> would you please read Documentation/CodingStyle and
> try sending the patch again?

What in detail is wrong concerning the coding style ?
Note, this is a serious problem for us.

> Although it doesn't make too much difference in this case,
> 2.4.20 is pretty old and it helps to get patches against
> the latest kernels.

2.4.20 is the kernel we are actually using.

> I suspect there is something else amiss, but I'll take a
> look at it.  What is the speed of the processor and the
> baud rate?

CPU:       MPC859T
CPU clock: 133Mhz
Baud Rate: 19200


Steffen

-- 


--------------------------------------------------------------

Steffen Rumler
ICN CP D NT SW 3
Siemens AG
Hofmannstr. 51                 Email: Steffen.Rumler@siemens.com
D-81359 Munich                 Phone: +49 89 722-44061
Germany                        Fax  : +49 89 722-36703

--------------------------------------------------------------

Re: uart.c: avoid changing parameter RAM on-the-fly for the console

[Steffen Rumler]

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

Dan Malek wrote:
> 
> On Feb 3, 2005, at 10:17 AM, Steffen Rumler wrote:
> 
>> According to Motorola this is _NOT_ allowed and dangerous.
> 
> 
> :-)  yeah, ok ...
> 
>> I suggest to do this inside rs_8xx_init() and startup() as shown
>> in the patch below.
> 
> 
> would you please read Documentation/CodingStyle and
> try sending the patch again?
> 
> Although it doesn't make too much difference in this case,
> 2.4.20 is pretty old and it helps to get patches against
> the latest kernels.
> 
>> With this fix, the modules hanging before can boot now.
> 
> 
> I suspect there is something else amiss, but I'll take a
> look at it.  What is the speed of the processor and the
> baud rate?

Hi Dan,

have you checked this problem in detail now ?

Unfortunately, I have forgotten the 'INIT RX TX' inside
my patched startup() (see new patch attached).
Sorry.

Thank you for help. The serial driver is rather complex.
Please, can you look into this (ignoring coding style at this time).


Steffen

-- 






[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 3216 bytes --]

diff -Naur old/arch/ppc/8xx_io/uart.c new/arch/ppc/8xx_io/uart.c
--- old/arch/ppc/8xx_io/uart.c	Wed Feb 16 10:43:24 2005
+++ new/arch/ppc/8xx_io/uart.c	Wed Feb 16 10:42:01 2005
@@ -119,6 +119,11 @@
 static int  uart_buf_read_proc    (char *, char **, off_t, int, int *, void *);
 #endif
 
+#ifdef CONFIG_SERIAL_CONSOLE
+static void full_seq_smc_stop (int port);
+static void smc_init_rx_tx (int port);
+#endif
+
 /*
  * Serial driver configuration section.  Here are the various options:
  */
@@ -831,10 +836,17 @@
 	else {
 		smcp = &cpmp->cp_smc[idx];
 
+#ifdef CONFIG_SERIAL_CONSOLE
+                /*  stop SMC in the correct way, before re-configuring it
+                 */
+		if (((state - rs_table) == CONFIG_SERIAL_CONSOLE_PORT)){
+                    full_seq_smc_stop(PORT_NUM(info->state->smc_scc_num));
+                }
+#endif
+
 		/* Enable interrupts and I/O.
 		*/
 		smcp->smc_smcm |= (SMCM_RX | SMCM_TX);
-		smcp->smc_smcmr |= (SMCMR_REN | SMCMR_TEN);
 
 		/* We can tune the buffer length and idle characters
 		 * to take advantage of the entire incoming buffer size.
@@ -848,6 +860,17 @@
 		up->smc_mrblr = RX_BUF_SIZE;
 		up->smc_maxidl = RX_BUF_SIZE;
 		up->smc_brkcr = 1;	/* number of break chars */
+
+#ifdef CONFIG_SERIAL_CONSOLE
+                if (((state - rs_table) == CONFIG_SERIAL_CONSOLE_PORT)){
+
+                    smc_init_rx_tx(PORT_NUM(info->state->smc_scc_num));
+                    info->rx_cur = info->rx_bd_base;
+                    info->tx_cur = info->tx_bd_base;
+                }
+#endif
+
+		smcp->smc_smcmr |= (SMCMR_REN | SMCMR_TEN);
 	}
 
 	info->flags |= ASYNC_INITIALIZED;
@@ -2694,6 +2717,46 @@
   
 } /* end uart_removeProcEntries() */
 
+
+#ifdef CONFIG_SERIAL_CONSOLE
+static void full_seq_smc_stop (int port)
+{
+    volatile cpm8xx_t *cp=cpmp;
+    volatile smc_t *sp;
+    ushort chan;
+    unsigned long flags;
+
+    sp   = &cp->cp_smc[port];
+    chan = smc_chan_map[port];
+
+    local_irq_save(flags);
+
+    while (cp->cp_cpcr & CPM_CR_FLG);
+    cp->cp_cpcr = mk_cr_cmd(chan, CPM_CR_STOP_TX) | CPM_CR_FLG;
+    while (cp->cp_cpcr & CPM_CR_FLG);
+    sp->smc_smcmr &= ~(SMCMR_REN | SMCMR_TEN);
+
+    local_irq_restore(flags);
+
+}  /* full_seq_smc_stop */ 
+
+static void smc_init_rx_tx (int port)
+{
+    unsigned long flags;
+    volatile cpm8xx_t *cp=cpmp;
+    ushort chan;
+
+    chan = smc_chan_map[port];
+
+    local_irq_save(flags);
+    while (cp->cp_cpcr & CPM_CR_FLG);
+    cp->cp_cpcr = mk_cr_cmd(chan, CPM_CR_INIT_TRX) | CPM_CR_FLG;
+    while (cp->cp_cpcr & CPM_CR_FLG);
+    local_irq_restore(flags);
+
+}  /* smc_init_rx_tx */
+#endif  /* CONFIG_SERIAL_CONSOLE */
+
 /*
  * The serial driver boot-time initialization code!
  */
@@ -2905,6 +2968,15 @@
 			}
 			else {
 				sp = &cp->cp_smc[idx];
+
+#ifdef CONFIG_SERIAL_CONSOLE
+                                /*  stop SMC in the correct way, before re-configuring it
+                                 */
+                                if (i == CONFIG_SERIAL_CONSOLE_PORT){
+                                    full_seq_smc_stop(idx);
+                                }
+#endif
+
 				up = (smc_uart_t *)&cp->cp_dparam[state->port];
 				up->smc_rbase = dp_addr;
 			}