Re: [PATCH] cpm_uart: Fix dpram allocation and non-console uarts

Hi,

On Wed, Aug 17, 2005 at 10:42:36PM -0700, Nish Aravamudan wrote:

quoted

On 8/8/05, Kumar Gala [off-list ref] wrote:

quoted

(A believe Marcelo would like to see this in 2.6.13, but I'll let him
fight over that ;)

* Makes dpram allocations work
* Makes non-console UART work on both 8xx and 82xx
* Fixed whitespace in files that were touched

Signed-off-by: Vitaly Bordug <redacted>
Signed-off-by: Pantelis Antoniou <redacted>
Signed-off-by: Kumar Gala <redacted>

---
commit 1de80554bcae877dce3b6d878053eb092ef65c72
tree aba124824607fea1070e86501ddccc9decce362d
parent ad81111fd554c9d3c14c0a50885e076af2f9ac9b
author Kumar K. Gala [off-list ref] Mon, 08 Aug 2005 22:35:39 -0500
committer Kumar K. Gala [off-list ref] Mon, 08 Aug 2005 22:35:39 -0500

<snip>

quoted

diff --git a/drivers/serial/cpm_uart/cpm_uart_core.c b/drivers/serial/cpm_uart/cpm_uart_core.c
--- a/drivers/serial/cpm_uart/cpm_uart_core.c
+++ b/drivers/serial/cpm_uart/cpm_uart_core.c

<snip>

quoted

@@ -376,9 +396,19 @@ static int cpm_uart_startup(struct uart_
                pinfo->sccp->scc_sccm |= UART_SCCM_RX;
        }

+       if (!(pinfo->flags & FLAG_CONSOLE))
+               cpm_line_cr_cmd(line,CPM_CR_INIT_TRX);
        return 0;
 }

+inline void cpm_uart_wait_until_send(struct uart_cpm_port *pinfo)
+{
+       unsigned long target_jiffies = jiffies + pinfo->wait_closing;
+
+       while (!time_after(jiffies, target_jiffies))
+               schedule();
+}

Not sure about that call here. Does the state need to be set so that
you won't be run again immediately? In any case, I think direct
schedule() callers are discouraged? Do you want to call a yield() or
schedule_timeout({0,1}) instead maybe?

Yep, schedule_timeout(pinfo->wait_closing) looks much better.

quoted

quoted

 /*
  * Shutdown the uart
  */

@@ -394,6 +424,12 @@ static void cpm_uart_shutdown(struct uar

        /* If the port is not the console, disable Rx and Tx. */
        if (!(pinfo->flags & FLAG_CONSOLE)) {
+               /* Wait for all the BDs marked sent */
+               while(!cpm_uart_tx_empty(port))
+                       schedule_timeout(2);

<snip>

I think you are using 2 jiffies to guarantee that at least one jiffy
elapses, which is fine. But, if you do not set the state beforehand,
schedule_timeout() returns immediately, so you have a busy-wait here.

Right, what about the following untested patch.

diff --git a/drivers/serial/cpm_uart/cpm_uart_core.c b/drivers/serial/cpm_uart/cpm_uart_core.c
--- a/drivers/serial/cpm_uart/cpm_uart_core.c
+++ b/drivers/serial/cpm_uart/cpm_uart_core.c

@@ -403,10 +403,9 @@ static int cpm_uart_startup(struct uart_
 
 inline void cpm_uart_wait_until_send(struct uart_cpm_port *pinfo)
 {
-	unsigned long target_jiffies = jiffies + pinfo->wait_closing;
-
-	while (!time_after(jiffies, target_jiffies))
-   		schedule();
+	set_current_state(TASK_UNINTERRUPTIBLE);
+	schedule_timeout(pinfo->wait_closing);
+	set_current_state(TASK_RUNNING);
 }