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

2009-12-09 Thread Kevin Hilman
Govindraj govindraj...@gmail.com writes:

 On Wed, Nov 25, 2009 at 12:18 AM, Kevin Hilman
 khil...@deeprootsystems.com wrote:
 Govindraj.R govindraj.r...@ti.com writes:

 From 4756e3743c7acd2de1030b2bd432c1b19f0b9ff5 Mon Sep 17 00:00:00 2001
 From: Govindraj R govindraj.r...@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.r...@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?

Yes.

 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.

OK, I didn't look closely at that but I'd like to be sure that the
extra checking can be optimized out on the SoCs that don't need it.

Kevin

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2009-12-02 Thread Govindraj
On Wed, Nov 25, 2009 at 12:18 AM, Kevin Hilman
khil...@deeprootsystems.com wrote:
 Govindraj.R govindraj.r...@ti.com writes:

 From 4756e3743c7acd2de1030b2bd432c1b19f0b9ff5 Mon Sep 17 00:00:00 2001
 From: Govindraj R govindraj.r...@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.r...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2009-11-25 Thread Govindraj
On Mon, Nov 23, 2009 at 10:41 PM, Artem Bityutskiy dedeki...@gmail.com wrote:
 On Fri, 2009-11-13 at 12:23 +0530, Govindraj.R wrote:
 From 4756e3743c7acd2de1030b2bd432c1b19f0b9ff5 Mon Sep 17 00:00:00 2001
 From: Govindraj R govindraj.r...@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.r...@ti.com
 ---
  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

 Did you address the irq storm problems pointed by Mika Westerberg?

that issue has been resolved with this current patch series.

---
Regards,
Govindraj.R



 --
 Best Regards,
 Artem Bityutskiy (Артём Битюцкий)

 --
 To unsubscribe from this list: send the line unsubscribe linux-omap in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2009-11-24 Thread Kevin Hilman
Govindraj.R govindraj.r...@ti.com writes:

 From 4756e3743c7acd2de1030b2bd432c1b19f0b9ff5 Mon Sep 17 00:00:00 2001
 From: Govindraj R govindraj.r...@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.r...@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)

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

 diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h
 b/arch/arm/plat-omap/include/plat/omap-serial.h
 new file mode 100644
 index 000..4341fae
 --- /dev/null
 +++ b/arch/arm/plat-omap/include/plat/omap-serial.h
 @@ -0,0 +1,115 @@
 +/*
 + * Driver for OMAP3430 UART controller.

This HW block is not unique to OMAP3.

 + * Copyright (C) 2009 Texas Instruments.

 + * Authors:
 + *   Govindraj R govindraj.r...@ti.com
 + *   Thara Gopinath  th...@ti.com
 + *

You should acknowledge that this drier was heavily borrowed from the
current 8250 driver.

 + * This file is licensed under the terms of the GNU General Public License
 + * version 2. This program is licensed as is without any warranty of any
 + * kind, whether express or implied.
 + */
 +
 +#ifndef __OMAP_SERIAL_H__
 +#define __OMAP_SERIAL_H__
 +
 +#include linux/serial_core.h
 +#include linux/platform_device.h
 +
 +#include plat/control.h
 +#include plat/mux.h
 +
 +#define DRIVER_NAME  omap-hsuart

but you use omap-uart in platform_driver below.  

 +/* *
 + * tty device name used by omap-serial driver,
 + * in bootargs we specify as console=ttyO0 if uart1
 + * is used as console uart.
 + */
 +#define DEVICE_NAME  ttyO

I've said this in each review now.  

Why can't you stick to 'ttyS'?  There is lots of
infrastructure/scripts/userspace/default commandlines/etc. that will
need to change if you change this.y

 +/* *
 + * We default to IRQ0 for the no irq hack.   Some
 + * machine types want others as well - they're free
 + * to redefine this in their header file.
 + */
 +#define is_real_interrupt(irq)  ((irq) != 0)
 +
 +#if defined(CONFIG_SERIAL_OMAP_CONSOLE)  defined(CONFIG_MAGIC_SYSRQ)
 +#define SUPPORT_SYSRQ
 +#endif
 +
 +#define OMAP_MDR1_DISABLE0x07
 +#define OMAP_MDR1_MODE13X0x03
 +#define OMAP_MDR1_MODE16X0x00
 +#define OMAP_MODE13X_SPEED   230400
 +
 +#define CONSOLE_NAME console=
 +
 +#define RX_TIMEOUT   (3 * HZ)
 +
 +/* To be removed while adding omap_hwmod support */
 +#define OMAP_MAX_HSUART_PORTS 3
 +
 +struct uart_port_info {
 + booldma_enabled;
 + unsigned intuartclk;
 + };

Should this be uart_omap_port_info?

 +struct uart_omap_dma {
 + u8  uart_dma_tx;
 + u8  uart_dma_rx;
 + int rx_dma_channel;
 + int tx_dma_channel;
 + /* Physical adress of RX DMA buffer */
 + dma_addr_t  rx_buf_dma_phys;
 + /* Physical adress of TX DMA buffer */
 + dma_addr_t  tx_buf_dma_phys;
 + /* *
 +  * Buffer for rx dma.It is not required for tx because the buffer
 +  * comes from port structure
 +  */
 + unsigned intuart_base;
 + unsigned char   *rx_buf;
 + unsigned intprev_rx_dma_pos;
 + int tx_buf_size;
 + int tx_dma_state;
 + int rx_dma_state;
 + spinlock_t  tx_lock;
 + spinlock_t  rx_lock;
 + /* timer to poll activity on rx dma */
 + struct timer_list   rx_timer;
 + int   

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

2009-11-24 Thread Kevin Hilman
Govindraj.R govindraj.r...@ti.com writes:

 From 4756e3743c7acd2de1030b2bd432c1b19f0b9ff5 Mon Sep 17 00:00:00 2001
 From: Govindraj R govindraj.r...@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.r...@ti.com

Govindraj,

FYI... to use this version of the driver with the UART omap_device
conversion I just posted, I used this additional patch on top of your
driver.

These are various issues I pointed out in the review as well, but
wanted to share this with V3 + this patch, I was able to use it
with the omap_device conversion.

Kevin

commit 79285bfc6c8711f580895256a82e7dc127156824
Author: Kevin Hilman khil...@deeprootsystems.com
Date:   Mon Nov 23 15:10:13 2009 -0800

OMAP: HS-UART: misc. fixes

diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h 
b/arch/arm/plat-omap/include/plat/omap-serial.h
index 4341fae..86bec82 100644
--- a/arch/arm/plat-omap/include/plat/omap-serial.h
+++ b/arch/arm/plat-omap/include/plat/omap-serial.h
@@ -28,7 +28,7 @@
  * in bootargs we specify as console=ttyO0 if uart1
  * is used as console uart.
  */
-#define DEVICE_NAMEttyO
+#define DEVICE_NAMEttyS
 
 /* *
  * We default to IRQ0 for the no irq hack.   Some
diff --git a/drivers/serial/omap-serial.c b/drivers/serial/omap-serial.c
index 6dd8bb4..65d05f8 100644
--- a/drivers/serial/omap-serial.c
+++ b/drivers/serial/omap-serial.c
@@ -993,7 +993,7 @@ static struct console serial_omap_console = {
 
 static void serial_omap_add_console_port(struct uart_omap_port *up)
 {
-   serial_omap_console_ports[up-pdev-id - 1] = up;
+   serial_omap_console_ports[up-pdev-id] = up;
 }
 
 #define OMAP_CONSOLE   (serial_omap_console)
@@ -1237,7 +1237,7 @@ static int serial_omap_probe(struct platform_device *pdev)
up-port.regshift = 2;
up-port.fifosize = 64;
up-port.ops = serial_omap_pops;
-   up-port.line = pdev-id - 1;
+   up-port.line = pdev-id;
up-port.membase = ioremap_nocache(up-port.mapbase,
0x16  up-port.regshift);
up-port.flags = UPF_BOOT_AUTOCONF;
@@ -1263,7 +1263,7 @@ static int serial_omap_probe(struct platform_device *pdev)
pr_err(\n %s: UART Driver Init Failed!\n, __func__);
return -EPERM;
}
-   ui[pdev-id - 1] = up;
+   ui[pdev-id] = up;
serial_omap_add_console_port(up);
 
ret = uart_add_one_port(serial_omap_reg, up-port);
@@ -1298,7 +1298,7 @@ static struct platform_driver serial_omap_driver = {
.suspend= serial_omap_suspend,
.resume = serial_omap_resume,
.driver = {
-   .name   = omap-uart,
+   .name   = DRIVER_NAME,
},
 };
 
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2009-11-23 Thread Artem Bityutskiy
On Fri, 2009-11-13 at 12:23 +0530, Govindraj.R wrote:
 From 4756e3743c7acd2de1030b2bd432c1b19f0b9ff5 Mon Sep 17 00:00:00 2001
 From: Govindraj R govindraj.r...@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.r...@ti.com
 ---
  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

Did you address the irq storm problems pointed by Mika Westerberg?

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2009-11-12 Thread Govindraj.R
From 4756e3743c7acd2de1030b2bd432c1b19f0b9ff5 Mon Sep 17 00:00:00 2001
From: Govindraj R govindraj.r...@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.r...@ti.com
---
 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

diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h
b/arch/arm/plat-omap/include/plat/omap-serial.h
new file mode 100644
index 000..4341fae
--- /dev/null
+++ b/arch/arm/plat-omap/include/plat/omap-serial.h
@@ -0,0 +1,115 @@
+/*
+ * Driver for OMAP3430 UART controller.
+ *
+ * Copyright (C) 2009 Texas Instruments.
+ *
+ * Authors:
+ * Govindraj R govindraj.r...@ti.com
+ * Thara Gopinath  th...@ti.com
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2. This program is licensed as is without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#ifndef __OMAP_SERIAL_H__
+#define __OMAP_SERIAL_H__
+
+#include linux/serial_core.h
+#include linux/platform_device.h
+
+#include plat/control.h
+#include plat/mux.h
+
+#define DRIVER_NAMEomap-hsuart
+
+/* *
+ * tty device name used by omap-serial driver,
+ * in bootargs we specify as console=ttyO0 if uart1
+ * is used as console uart.
+ */
+#define DEVICE_NAMEttyO
+
+/* *
+ * We default to IRQ0 for the no irq hack.   Some
+ * machine types want others as well - they're free
+ * to redefine this in their header file.
+ */
+#define is_real_interrupt(irq)  ((irq) != 0)
+
+#if defined(CONFIG_SERIAL_OMAP_CONSOLE)  defined(CONFIG_MAGIC_SYSRQ)
+#define SUPPORT_SYSRQ
+#endif
+
+#define OMAP_MDR1_DISABLE  0x07
+#define OMAP_MDR1_MODE13X  0x03
+#define OMAP_MDR1_MODE16X  0x00
+#define OMAP_MODE13X_SPEED 230400
+
+#define CONSOLE_NAME   console=
+
+#define RX_TIMEOUT (3 * HZ)
+
+/* To be removed while adding omap_hwmod support */
+#define OMAP_MAX_HSUART_PORTS 3
+
+struct uart_port_info {
+   booldma_enabled;
+   unsigned intuartclk;
+   };
+
+struct uart_omap_dma {
+   u8  uart_dma_tx;
+   u8  uart_dma_rx;
+   int rx_dma_channel;
+   int tx_dma_channel;
+   /* Physical adress of RX DMA buffer */
+   dma_addr_t  rx_buf_dma_phys;
+   /* Physical adress of TX DMA buffer */
+   dma_addr_t  tx_buf_dma_phys;
+   /* *
+* Buffer for rx dma.It is not required for tx because the buffer
+* comes from port structure
+*/
+   unsigned intuart_base;
+   unsigned char   *rx_buf;
+   unsigned intprev_rx_dma_pos;
+   int tx_buf_size;
+   int tx_dma_state;
+   int rx_dma_state;
+   spinlock_t  tx_lock;
+   spinlock_t  rx_lock;
+   /* timer to poll activity on rx dma */
+   struct timer_list   rx_timer;
+   int rx_buf_size;
+   int rx_timeout;
+};
+
+struct uart_omap_port {
+   struct uart_portport;
+   struct uart_omap_dmauart_dma;
+   struct platform_device  *pdev;
+
+   unsigned char   ier;
+   unsigned char   lcr;
+   unsigned char   mcr;
+   unsigned char   fcr;
+   unsigned char   efr;
+
+   int use_dma;
+   int is_buf_dma_alloced;
+   /*
+* Some bits in registers are cleared on a read, so they must
+* be saved whenever the register is read but the bits will not
+* be immediately processed.
+*/
+   unsigned intlsr_break_flag;
+#define MSR_SAVE_FLAGS UART_MSR_ANY_DELTA
+   unsigned char   msr_saved_flags;
+   charname[20];
+   spinlock_t  uart_lock;
+   unsigned long   port_activity;
+};
+
+extern char *saved_command_line;
+#endif /* __OMAP_SERIAL_H__ */
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index e522572..64bbf35 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -1366,6 +1366,29 @@ config SERIAL_OF_PLATFORM