Re: [PATCHv3 1/3] OMAP UART: Add omap-serial driver support.

From: Govindraj <govindraj.ti@gmail.com>
To: Kevin Hilman <khilman@deeprootsystems.com>
Cc: "Govindraj.R" <govindraj.raja@ti.com>, linux-omap@vger.kernel.org
Subject: Re: [PATCHv3 1/3] OMAP UART: Add omap-serial driver support.
Date: Wed, 2 Dec 2009 19:13:25 +0530	[thread overview]
Message-ID: <d6a0f7aa0912020543s7f8310fdueb0a3a6c52df1326@mail.gmail.com> (raw)
In-Reply-To: <87ws1fvjwu.fsf@deeprootsystems.com>

On Wed, Nov 25, 2009 at 12:18 AM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> "Govindraj.R" <govindraj.raja@ti.com> writes:
>
>> From 4756e3743c7acd2de1030b2bd432c1b19f0b9ff5 Mon Sep 17 00:00:00 2001
>> From: Govindraj R <govindraj.raja@ti.com>
>> Date: Fri, 13 Nov 2009 12:01:54 +0530
>> Subject: [PATCH] OMAP UART: Add omap-serial driver support.
>>
>> This patch adds support for OMAP3430-HIGH SPEED UART Controller.
>>
>> It adds support for the following features:
>> 1. It supports Interrupt mode and DMA mode of operation.
>> 2. Supports Hardware flow control and sofware flow control.
>> 3. Debug Console support on all UARTs.
>>
>> Signed-off-by: Govindraj R <govindraj.raja@ti.com>
>
> Some general comments.
>
> This should summarize how this is different from the 8250 driver on
> which it was based, as it's clear that it was based on 8250 but not
> clear at all what the changes are.
>
> At first glance, you've dropped several features from the 8250 driver
> which we currently use.  Namely, the ability for platform code to
> override some of the defaults:
>
> - change irq_flags
> - serial_in function
> - optional ioremapping (omap_hwmod layer will have done ioremap already)

Agree. uart_port_info [should be renamed to omap_uart_port_info]
should grow with fields like irqflags, membase and mapbase feilds.

adding these would need rework on the patch:
http://patchwork.kernel.org/patch/62555/

Should I work on top of above patch?

Serial in function might not be necessary for omap-serial driver,
this function was added to handle RX reading by checking if DR bit set
in LSR reg.
This is taken care in omap-serial driver.

---
Regards,
Govindraj.R

>
> These are now hard-coded in the new driver which makes it
> significantly less flexible.
>
> In a few minutes I will be posting a set of patches to convert the
> mach-omap2/serial.c to use omap_hwmod/omap_device.  This replaces your
> PATCH 2/3, so please use with any subsequent updates.
>
>> ---
>>  arch/arm/plat-omap/include/plat/omap-serial.h |  115 +++
>>  drivers/serial/Kconfig                        |   23 +
>>  drivers/serial/Makefile                       |    1 +
>>  drivers/serial/omap-serial.c                  | 1330 +++++++++++++++++++++++++
>>  include/linux/serial_core.h                   |    3 +
>>  5 files changed, 1472 insertions(+), 0 deletions(-)
>>  create mode 100644 arch/arm/plat-omap/include/plat/omap-serial.h
>>  create mode 100644 drivers/serial/omap-serial.c
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html