From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ionut Nicu <ionut.nicu@mindbit.ro>
Subject: Re: [PATCH 1/1] serial: omap-serial: Add support for kernel
 debugger
Date: Sun, 21 Nov 2010 12:56:15 +0200
Message-ID: <1290336975.26899.16.camel@atlantis.mindbit.ro>
References: <1290286296-3589-1-git-send-email-cosmin.cojocar@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Return-path: <linux-omap-owner@vger.kernel.org>
Received: from ns.mindbit.ro ([80.86.127.26]:49286 "EHLO ns.mindbit.ro"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751114Ab0KUK4w (ORCPT <rfc822;linux-omap@vger.kernel.org>);
	Sun, 21 Nov 2010 05:56:52 -0500
In-Reply-To: <1290286296-3589-1-git-send-email-cosmin.cojocar@gmail.com>
Sender: linux-omap-owner@vger.kernel.org
List-Id: linux-omap@vger.kernel.org
To: Cosmin Cojocar <cosmin.cojocar@gmail.com>
Cc: govindraj.raja@ti.com, manjugk@ti.com, khilman@deeprootsystems.com, ben-linux@fluff.org, tklauser@distanz.ch, feng.tang@intel.com, gregkh@suse.de, alan@linux.intel.com, linux-omap@vger.kernel.org

Hi Cosmin,

On Sat, 2010-11-20 at 21:51 +0100, Cosmin Cojocar wrote:
> The kgdb invokes the poll_put_char and poll_get_char when communicating
> with the host. This patch also changes the initialization order because the
> kgdb will check at the very beginning, if there is a valid serial
> driver.
> 
> Signed-off-by: Cosmin Cojocar <cosmin.cojocar@gmail.com>
> ---
>  drivers/serial/Makefile      |    2 +-
>  drivers/serial/omap-serial.c |   38 ++++++++++++++++++++++++++++++++++++--
>  2 files changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
> index c570576..ad86113 100644
> --- a/drivers/serial/Makefile
> +++ b/drivers/serial/Makefile
> @@ -80,6 +80,7 @@ obj-$(CONFIG_SERIAL_NETX) += netx-serial.o
>  obj-$(CONFIG_SERIAL_OF_PLATFORM) += of_serial.o
>  obj-$(CONFIG_SERIAL_OF_PLATFORM_NWPSERIAL) += nwpserial.o
>  obj-$(CONFIG_SERIAL_KS8695) += serial_ks8695.o
> +obj-$(CONFIG_SERIAL_OMAP) += omap-serial.o
>  obj-$(CONFIG_KGDB_SERIAL_CONSOLE) += kgdboc.o
>  obj-$(CONFIG_SERIAL_QE) += ucc_uart.o
>  obj-$(CONFIG_SERIAL_TIMBERDALE)	+= timbuart.o
> @@ -88,4 +89,3 @@ obj-$(CONFIG_SERIAL_ALTERA_JTAGUART) += altera_jtaguart.o
>  obj-$(CONFIG_SERIAL_ALTERA_UART) += altera_uart.o
>  obj-$(CONFIG_SERIAL_MRST_MAX3110)	+= mrst_max3110.o
>  obj-$(CONFIG_SERIAL_MFD_HSU)	+= mfd.o
> -obj-$(CONFIG_SERIAL_OMAP) += omap-serial.o
> diff --git a/drivers/serial/omap-serial.c b/drivers/serial/omap-serial.c
> index 03a96db..df6ba03 100644
> --- a/drivers/serial/omap-serial.c
> +++ b/drivers/serial/omap-serial.c
> @@ -866,14 +866,44 @@ serial_omap_type(struct uart_port *port)
>  	return up->name;
>  }
>  
> +#define BOTH_EMPTY (UART_LSR_TEMT | UART_LSR_THRE)
> +
> +#ifdef CONFIG_CONSOLE_POLL
> +static void serial_omap_poll_put_char(struct uart_port *port, unsigned char ch)
> +{
> +	struct uart_omap_port *up = (struct uart_omap_port *)port;
> +	unsigned int status;
> +
> +	do {
> +		status = serial_in(up, UART_LSR);
> +		udelay(1);
> +	} while ((status & BOTH_EMPTY) != BOTH_EMPTY);
> +

Shouldn't you use wait_for_xmitr() instead? If you do that, then you
don't need to move the BOTH_EMPTY macro anymore.

> +	serial_out(up, UART_TX, ch);
> +}
> +
> +static int serial_omap_poll_get_char(struct uart_port *port)
> +{
> +	struct uart_omap_port *up = (struct uart_omap_port *)port;
> +	unsigned int status;
> +	int ch;
> +
> +	do {
> +		status = serial_in(up, UART_LSR);
> +		udelay(1);
> +	} while ((status & UART_LSR_DR) != UART_LSR_DR);
> +

I think you should be returning NO_POLL_CHAR if the condition is false,
instead of busy looping.

> +	ch = (int)serial_in(up, UART_RX);
> +	return ch;
> +}

I don't think you need the extra ch variable. Just return serial_in(up,
UART_RX).

Cheers,
Ionut.