2012-08-21 09:19:59

by Felipe Balbi

[permalink] [raw]
Subject: [RFC/PATCH 00/13] OMAP UART patches

Hi guys,

here's a series of cleanup patches to the OMAP serial
driver. A later series could be made re-implementing
DMA using the DMA Engine API. Note that for RX DMA
we could be using RX Timeout IRQ as a hint that we better
use PIO instead ;-)

All patches were tested on my pandaboard, but I'd really
like to receive Tested-by on other platforms.

After this goes in, I'll probably try to get UART wakeup
working again and only after that look at DMA.

cheers

Felipe Balbi (13):
serial: omap: define and use to_uart_omap_port()
serial: omap: always return IRQ_HANDLED
serial: omap: define helpers for pdata function pointers
serial: omap: don't access the platform_device
serial: omap: drop DMA support
serial: add OMAP-specific defines
serial: omap: simplify IRQ handling
serial: omap: refactor receive_chars() into rdi/rlsi handlers
serial: omap: move THRE check to transmit_chars()
serial: omap: stick to put_autosuspend
serial: omap: set dev->drvdata before enabling pm_runtime
serial: omap: drop unnecessary check from remove
serial: omap: make sure to suspend device before remove

arch/arm/mach-omap2/serial.c | 15 +-
arch/arm/plat-omap/include/plat/omap-serial.h | 12 +-
drivers/tty/serial/omap-serial.c | 707 +++++++++-----------------
include/linux/serial_reg.h | 4 +
4 files changed, 250 insertions(+), 488 deletions(-)

--
1.7.12.rc3


2012-08-21 09:20:08

by Felipe Balbi

[permalink] [raw]
Subject: [RFC/PATCH 01/13] serial: omap: define and use to_uart_omap_port()

current code only works because struct uart_port
is the first member on the uart_omap_port structure.

If, for whatever reason, someone puts another
member as the first of the structure, that cast
won't work anymore. In order to be safe, let's use
a container_of() which, for now, gets optimized into
a cast anyway.

Signed-off-by: Felipe Balbi <[email protected]>
---
arch/arm/plat-omap/include/plat/omap-serial.h | 2 ++
drivers/tty/serial/omap-serial.c | 36 +++++++++++++--------------
2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
index 1a52725..f3b35d9 100644
--- a/arch/arm/plat-omap/include/plat/omap-serial.h
+++ b/arch/arm/plat-omap/include/plat/omap-serial.h
@@ -137,4 +137,6 @@ struct uart_omap_port {
struct work_struct qos_work;
};

+#define to_uart_omap_port(p) ((container_of((p), struct uart_omap_port, port)))
+
#endif /* __OMAP_SERIAL_H__ */
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index d3cda0c..5c0d0bc 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -141,7 +141,7 @@ static void serial_omap_stop_rxdma(struct uart_omap_port *up)

static void serial_omap_enable_ms(struct uart_port *port)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);

dev_dbg(up->port.dev, "serial_omap_enable_ms+%d\n", up->port.line);

@@ -153,7 +153,7 @@ static void serial_omap_enable_ms(struct uart_port *port)

static void serial_omap_stop_tx(struct uart_port *port)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;

if (up->use_dma &&
@@ -186,7 +186,7 @@ static void serial_omap_stop_tx(struct uart_port *port)

static void serial_omap_stop_rx(struct uart_port *port)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);

pm_runtime_get_sync(&up->pdev->dev);
if (up->use_dma)
@@ -307,7 +307,7 @@ static inline void serial_omap_enable_ier_thri(struct uart_omap_port *up)

static void serial_omap_start_tx(struct uart_port *port)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
struct circ_buf *xmit;
unsigned int start;
@@ -449,7 +449,7 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)

static unsigned int serial_omap_tx_empty(struct uart_port *port)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
unsigned long flags = 0;
unsigned int ret = 0;

@@ -464,7 +464,7 @@ static unsigned int serial_omap_tx_empty(struct uart_port *port)

static unsigned int serial_omap_get_mctrl(struct uart_port *port)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
unsigned int status;
unsigned int ret = 0;

@@ -487,7 +487,7 @@ static unsigned int serial_omap_get_mctrl(struct uart_port *port)

static void serial_omap_set_mctrl(struct uart_port *port, unsigned int mctrl)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
unsigned char mcr = 0;

dev_dbg(up->port.dev, "serial_omap_set_mctrl+%d\n", up->port.line);
@@ -511,7 +511,7 @@ static void serial_omap_set_mctrl(struct uart_port *port, unsigned int mctrl)

static void serial_omap_break_ctl(struct uart_port *port, int break_state)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
unsigned long flags = 0;

dev_dbg(up->port.dev, "serial_omap_break_ctl+%d\n", up->port.line);
@@ -528,7 +528,7 @@ static void serial_omap_break_ctl(struct uart_port *port, int break_state)

static int serial_omap_startup(struct uart_port *port)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
unsigned long flags = 0;
int retval;

@@ -606,7 +606,7 @@ static int serial_omap_startup(struct uart_port *port)

static void serial_omap_shutdown(struct uart_port *port)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
unsigned long flags = 0;

dev_dbg(up->port.dev, "serial_omap_shutdown+%d\n", up->port.line);
@@ -721,7 +721,7 @@ static void
serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
struct ktermios *old)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
unsigned char cval = 0;
unsigned char efr = 0;
unsigned long flags = 0;
@@ -932,7 +932,7 @@ static void
serial_omap_pm(struct uart_port *port, unsigned int state,
unsigned int oldstate)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
unsigned char efr;

dev_dbg(up->port.dev, "serial_omap_pm+%d\n", up->port.line);
@@ -971,7 +971,7 @@ static int serial_omap_request_port(struct uart_port *port)

static void serial_omap_config_port(struct uart_port *port, int flags)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);

dev_dbg(up->port.dev, "serial_omap_config_port+%d\n",
up->port.line);
@@ -989,7 +989,7 @@ serial_omap_verify_port(struct uart_port *port, struct serial_struct *ser)
static const char *
serial_omap_type(struct uart_port *port)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);

dev_dbg(up->port.dev, "serial_omap_type+%d\n", up->port.line);
return up->name;
@@ -1032,7 +1032,7 @@ static inline void wait_for_xmitr(struct uart_omap_port *up)

static void serial_omap_poll_put_char(struct uart_port *port, unsigned char ch)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);

pm_runtime_get_sync(&up->pdev->dev);
wait_for_xmitr(up);
@@ -1042,7 +1042,7 @@ static void serial_omap_poll_put_char(struct uart_port *port, unsigned char ch)

static int serial_omap_poll_get_char(struct uart_port *port)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
unsigned int status;

pm_runtime_get_sync(&up->pdev->dev);
@@ -1065,7 +1065,7 @@ static struct uart_driver serial_omap_reg;

static void serial_omap_console_putchar(struct uart_port *port, int ch)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);

wait_for_xmitr(up);
serial_out(up, UART_TX, ch);
@@ -1341,7 +1341,7 @@ static void serial_omap_continue_tx(struct uart_omap_port *up)

static void uart_tx_dma_callback(int lch, u16 ch_status, void *data)
{
- struct uart_omap_port *up = (struct uart_omap_port *)data;
+ struct uart_omap_port *up = data;
struct circ_buf *xmit = &up->port.state->xmit;

xmit->tail = (xmit->tail + up->uart_dma.tx_buf_size) & \
--
1.7.12.rc3

2012-08-21 09:20:14

by Felipe Balbi

[permalink] [raw]
Subject: [RFC/PATCH 03/13] serial: omap: define helpers for pdata function pointers

this patch is in preparation to a few other changes
which will align on the prototype for function
pointers passed through pdata.

It also helps cleaning up the driver a little by
agregating checks for pdata in a single location.

Signed-off-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/omap-serial.c | 66 ++++++++++++++++++++++++++++------------
1 file changed, 47 insertions(+), 19 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index b4b95fc..c3e6fa0 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -101,6 +101,40 @@ static inline void serial_omap_clear_fifos(struct uart_omap_port *up)
serial_out(up, UART_FCR, 0);
}

+static int serial_omap_get_context_loss_count(struct uart_omap_port *up)
+{
+ struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
+
+ if (!pdata->get_context_loss_count)
+ return 0;
+
+ return pdata->get_context_loss_count(&up->pdev->dev);
+}
+
+static void serial_omap_set_forceidle(struct uart_omap_port *up)
+{
+ struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
+
+ if (pdata->set_forceidle)
+ pdata->set_forceidle(up->pdev);
+}
+
+static void serial_omap_set_noidle(struct uart_omap_port *up)
+{
+ struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
+
+ if (pdata->set_noidle)
+ pdata->set_noidle(up->pdev);
+}
+
+static void serial_omap_enable_wakeup(struct uart_omap_port *up, bool enable)
+{
+ struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
+
+ if (pdata->enable_wakeup)
+ pdata->enable_wakeup(up->pdev, enable);
+}
+
/*
* serial_omap_get_divisor - calculate divisor value
* @port: uart port info
@@ -177,8 +211,8 @@ static void serial_omap_stop_tx(struct uart_port *port)
serial_out(up, UART_IER, up->ier);
}

- if (!up->use_dma && pdata && pdata->set_forceidle)
- pdata->set_forceidle(up->pdev);
+ if (!up->use_dma && pdata)
+ serial_omap_set_forceidle(up);

pm_runtime_mark_last_busy(&up->pdev->dev);
pm_runtime_put_autosuspend(&up->pdev->dev);
@@ -308,7 +342,6 @@ static inline void serial_omap_enable_ier_thri(struct uart_omap_port *up)
static void serial_omap_start_tx(struct uart_port *port)
{
struct uart_omap_port *up = to_uart_omap_port(port);
- struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
struct circ_buf *xmit;
unsigned int start;
int ret = 0;
@@ -316,8 +349,7 @@ static void serial_omap_start_tx(struct uart_port *port)
if (!up->use_dma) {
pm_runtime_get_sync(&up->pdev->dev);
serial_omap_enable_ier_thri(up);
- if (pdata && pdata->set_noidle)
- pdata->set_noidle(up->pdev);
+ serial_omap_set_noidle(up);
pm_runtime_mark_last_busy(&up->pdev->dev);
pm_runtime_put_autosuspend(&up->pdev->dev);
return;
@@ -1648,28 +1680,26 @@ static int serial_omap_runtime_suspend(struct device *dev)
if (!up)
return -EINVAL;

- if (!pdata || !pdata->enable_wakeup)
+ if (!pdata)
return 0;

- if (pdata->get_context_loss_count)
- up->context_loss_cnt = pdata->get_context_loss_count(dev);
+ up->context_loss_cnt = serial_omap_get_context_loss_count(up);

if (device_may_wakeup(dev)) {
if (!up->wakeups_enabled) {
- pdata->enable_wakeup(up->pdev, true);
+ serial_omap_enable_wakeup(up, true);
up->wakeups_enabled = true;
}
} else {
if (up->wakeups_enabled) {
- pdata->enable_wakeup(up->pdev, false);
+ serial_omap_enable_wakeup(up, false);
up->wakeups_enabled = false;
}
}

/* Errata i291 */
- if (up->use_dma && pdata->set_forceidle &&
- (up->errata & UART_ERRATA_i291_DMA_FORCEIDLE))
- pdata->set_forceidle(up->pdev);
+ if (up->use_dma && (up->errata & UART_ERRATA_i291_DMA_FORCEIDLE))
+ serial_omap_set_forceidle(up);

up->latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
schedule_work(&up->qos_work);
@@ -1683,17 +1713,15 @@ static int serial_omap_runtime_resume(struct device *dev)
struct omap_uart_port_info *pdata = dev->platform_data;

if (up && pdata) {
- if (pdata->get_context_loss_count) {
- u32 loss_cnt = pdata->get_context_loss_count(dev);
+ u32 loss_cnt = serial_omap_get_context_loss_count(up);

if (up->context_loss_cnt != loss_cnt)
serial_omap_restore_context(up);
- }

/* Errata i291 */
- if (up->use_dma && pdata->set_noidle &&
- (up->errata & UART_ERRATA_i291_DMA_FORCEIDLE))
- pdata->set_noidle(up->pdev);
+ if ((up->errata & UART_ERRATA_i291_DMA_FORCEIDLE) &&
+ up->use_dma)
+ serial_omap_set_noidle(up);

up->latency = up->calc_latency;
schedule_work(&up->qos_work);
--
1.7.12.rc3

2012-08-21 09:20:28

by Felipe Balbi

[permalink] [raw]
Subject: [RFC/PATCH 07/13] serial: omap: simplify IRQ handling

quite a few changes here, though they are
pretty obvious. In summary we're making sure
to detect which interrupt type we need to
handle before calling the underlying interrupt
handling procedure.

Signed-off-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/omap-serial.c | 47 ++++++++++++++++++++++++++++++----------
1 file changed, 35 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 4d74a9b..1ca08b8 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -350,32 +350,55 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
{
struct uart_omap_port *up = dev_id;
unsigned int iir, lsr;
+ unsigned int type;
unsigned long flags;

+ spin_lock_irqsave(&up->port.lock, flags);
pm_runtime_get_sync(up->dev);
iir = serial_in(up, UART_IIR);
- if (iir & UART_IIR_NO_INT) {
- pm_runtime_mark_last_busy(up->dev);
- pm_runtime_put_autosuspend(up->dev);
- return IRQ_HANDLED;
- }
+again:
+ if (iir & UART_IIR_NO_INT)
+ goto out;

- spin_lock_irqsave(&up->port.lock, flags);
lsr = serial_in(up, UART_LSR);
- if (iir & UART_IIR_RLSI) {
+
+ /* extract IRQ type from IIR register */
+ type = iir & 0x3e;
+
+ switch (type) {
+ case UART_IIR_MSI:
+ check_modem_status(up);
+ break;
+ case UART_IIR_THRI:
+ if (lsr & UART_LSR_THRE)
+ transmit_chars(up);
+ break;
+ case UART_IIR_RDI:
if (lsr & UART_LSR_DR)
receive_chars(up, &lsr);
+ break;
+ case UART_IIR_RLSI:
+ if (lsr & UART_LSR_BRK_ERROR_BITS)
+ receive_chars(up, &lsr);
+ break;
+ case UART_IIR_RX_TIMEOUT:
+ receive_chars(up, &lsr);
+ break;
+ case UART_IIR_CTS_RTS_DSR:
+ iir = serial_in(up, UART_IIR);
+ goto again;
+ case UART_IIR_XOFF:
+ /* FALLTHROUGH */
+ default:
+ break;
}

- check_modem_status(up);
- if ((lsr & UART_LSR_THRE) && (iir & UART_IIR_THRI))
- transmit_chars(up);
-
+out:
spin_unlock_irqrestore(&up->port.lock, flags);
pm_runtime_mark_last_busy(up->dev);
pm_runtime_put_autosuspend(up->dev);
-
up->port_activity = jiffies;
+
return IRQ_HANDLED;
}

--
1.7.12.rc3

2012-08-21 09:20:41

by Felipe Balbi

[permalink] [raw]
Subject: [RFC/PATCH 10/13] serial: omap: stick to put_autosuspend

Everytime we're done using our TTY, we want
the pm timer to be reinitilized. By sticking
to pm_runtime_pm_autosuspend() we make sure
that this will always be the case.

Signed-off-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/omap-serial.c | 33 ++++++++++++++++++++++-----------
1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 6ea24c5..458d77c 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -164,7 +164,8 @@ static void serial_omap_enable_ms(struct uart_port *port)
pm_runtime_get_sync(up->dev);
up->ier |= UART_IER_MSI;
serial_out(up, UART_IER, up->ier);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

static void serial_omap_stop_tx(struct uart_port *port)
@@ -412,7 +413,8 @@ static unsigned int serial_omap_tx_empty(struct uart_port *port)
spin_lock_irqsave(&up->port.lock, flags);
ret = serial_in(up, UART_LSR) & UART_LSR_TEMT ? TIOCSER_TEMT : 0;
spin_unlock_irqrestore(&up->port.lock, flags);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
return ret;
}

@@ -424,7 +426,8 @@ static unsigned int serial_omap_get_mctrl(struct uart_port *port)

pm_runtime_get_sync(up->dev);
status = check_modem_status(up);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);

dev_dbg(up->port.dev, "serial_omap_get_mctrl+%d\n", up->port.line);

@@ -460,7 +463,8 @@ static void serial_omap_set_mctrl(struct uart_port *port, unsigned int mctrl)
up->mcr = serial_in(up, UART_MCR);
up->mcr |= mcr;
serial_out(up, UART_MCR, up->mcr);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

static void serial_omap_break_ctl(struct uart_port *port, int break_state)
@@ -477,7 +481,8 @@ static void serial_omap_break_ctl(struct uart_port *port, int break_state)
up->lcr &= ~UART_LCR_SBC;
serial_out(up, UART_LCR, up->lcr);
spin_unlock_irqrestore(&up->port.lock, flags);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

static int serial_omap_startup(struct uart_port *port)
@@ -575,7 +580,8 @@ static void serial_omap_shutdown(struct uart_port *port)
if (serial_in(up, UART_LSR) & UART_LSR_DR)
(void) serial_in(up, UART_RX);

- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
free_irq(up->port.irq, up);
}

@@ -846,7 +852,8 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
serial_omap_configure_xonxoff(up, termios);

spin_unlock_irqrestore(&up->port.lock, flags);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
dev_dbg(up->port.dev, "serial_omap_set_termios+%d\n", up->port.line);
}

@@ -877,7 +884,8 @@ serial_omap_pm(struct uart_port *port, unsigned int state,
pm_runtime_allow(up->dev);
}

- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

static void serial_omap_release_port(struct uart_port *port)
@@ -959,7 +967,8 @@ static void serial_omap_poll_put_char(struct uart_port *port, unsigned char ch)
pm_runtime_get_sync(up->dev);
wait_for_xmitr(up);
serial_out(up, UART_TX, ch);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

static int serial_omap_poll_get_char(struct uart_port *port)
@@ -973,7 +982,8 @@ static int serial_omap_poll_get_char(struct uart_port *port)
return NO_POLL_CHAR;

status = serial_in(up, UART_RX);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
return status;
}

@@ -1305,7 +1315,8 @@ static int serial_omap_probe(struct platform_device *pdev)
if (ret != 0)
goto err_add_port;

- pm_runtime_put(&pdev->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
platform_set_drvdata(pdev, up);
return 0;

--
1.7.12.rc3

2012-08-21 09:20:38

by Felipe Balbi

[permalink] [raw]
Subject: [RFC/PATCH 11/13] serial: omap: set dev->drvdata before enabling pm_runtime

by the time we call our first pm_runtme_get_sync()
after enable pm_runtime, our resume method might
be called. To avoid problems, we must make sure
that our dev->drvdata is set correctly before
our resume method gets called.

Signed-off-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/omap-serial.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 458d77c..ba9cdf1 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1298,6 +1298,7 @@ static int serial_omap_probe(struct platform_device *pdev)
serial_omap_uart_wq = create_singlethread_workqueue(up->name);
INIT_WORK(&up->qos_work, serial_omap_uart_qos_work);

+ platform_set_drvdata(pdev, up);
pm_runtime_use_autosuspend(&pdev->dev);
pm_runtime_set_autosuspend_delay(&pdev->dev,
omap_up_info->autosuspend_timeout);
@@ -1317,7 +1318,6 @@ static int serial_omap_probe(struct platform_device *pdev)

pm_runtime_mark_last_busy(up->dev);
pm_runtime_put_autosuspend(up->dev);
- platform_set_drvdata(pdev, up);
return 0;

err_add_port:
--
1.7.12.rc3

2012-08-21 09:21:20

by Felipe Balbi

[permalink] [raw]
Subject: [RFC/PATCH 13/13] serial: omap: make sure to suspend device before remove

before removing the driver, let's make sure
to force device into a suspended state in order
to conserve power.

Signed-off-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/omap-serial.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 25d2055..f849ab9 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1334,6 +1334,7 @@ static int serial_omap_remove(struct platform_device *dev)
{
struct uart_omap_port *up = platform_get_drvdata(dev);

+ pm_runtime_put_sync(up->dev);
pm_runtime_disable(up->dev);
uart_remove_one_port(&serial_omap_reg, &up->port);
pm_qos_remove_request(&up->pm_qos_request);
--
1.7.12.rc3

2012-08-21 09:21:45

by Felipe Balbi

[permalink] [raw]
Subject: [RFC/PATCH 12/13] serial: omap: drop unnecessary check from remove

if platform_get_drvdata() returns NULL, that's
quite a nasty bug on the driver which we want to
catch ASAP. Otherwise, that check is hugely
unneeded.

Signed-off-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/omap-serial.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index ba9cdf1..25d2055 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1334,13 +1334,10 @@ static int serial_omap_remove(struct platform_device *dev)
{
struct uart_omap_port *up = platform_get_drvdata(dev);

- if (up) {
- pm_runtime_disable(up->dev);
- uart_remove_one_port(&serial_omap_reg, &up->port);
- pm_qos_remove_request(&up->pm_qos_request);
- }
+ pm_runtime_disable(up->dev);
+ uart_remove_one_port(&serial_omap_reg, &up->port);
+ pm_qos_remove_request(&up->pm_qos_request);

- platform_set_drvdata(dev, NULL);
return 0;
}

--
1.7.12.rc3

2012-08-21 09:20:23

by Felipe Balbi

[permalink] [raw]
Subject: [RFC/PATCH 06/13] serial: add OMAP-specific defines

OMAP has some extra Interrupt types which can
be really useful for SW. Let's define them
so we can later use those in OMAP's serial driver.

Signed-off-by: Felipe Balbi <[email protected]>
---
include/linux/serial_reg.h | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/include/linux/serial_reg.h b/include/linux/serial_reg.h
index 8ce70d7..5ed325e 100644
--- a/include/linux/serial_reg.h
+++ b/include/linux/serial_reg.h
@@ -40,6 +40,10 @@

#define UART_IIR_BUSY 0x07 /* DesignWare APB Busy Detect */

+#define UART_IIR_RX_TIMEOUT 0x0c /* OMAP RX Timeout interrupt */
+#define UART_IIR_XOFF 0x10 /* OMAP XOFF/Special Character */
+#define UART_IIR_CTS_RTS_DSR 0x20 /* OMAP CTS/RTS/DSR Change */
+
#define UART_FCR 2 /* Out: FIFO Control Register */
#define UART_FCR_ENABLE_FIFO 0x01 /* Enable the FIFO */
#define UART_FCR_CLEAR_RCVR 0x02 /* Clear the RCVR FIFO */
--
1.7.12.rc3

2012-08-21 09:22:08

by Felipe Balbi

[permalink] [raw]
Subject: [RFC/PATCH 09/13] serial: omap: move THRE check to transmit_chars()

since all other IRQ types now do all necessary
checks inside their handlers, transmit_chars()
was the only one left expecting serial_omap_irq()
to check THRE for it. We can move THRE check to
transmit_chars() in order to make serial_omap_irq()
more uniform.

Signed-off-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/omap-serial.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 74a4f0a..6ea24c5 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -195,11 +195,14 @@ static void serial_omap_stop_rx(struct uart_port *port)
pm_runtime_put_autosuspend(up->dev);
}

-static void transmit_chars(struct uart_omap_port *up)
+static void transmit_chars(struct uart_omap_port *up, unsigned int lsr)
{
struct circ_buf *xmit = &up->port.state->xmit;
int count;

+ if (!(lsr & UART_LSR_THRE))
+ return;
+
if (up->port.x_char) {
serial_out(up, UART_TX, up->port.x_char);
up->port.icount.tx++;
@@ -367,8 +370,7 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
check_modem_status(up);
break;
case UART_IIR_THRI:
- if (lsr & UART_LSR_THRE)
- transmit_chars(up);
+ transmit_chars(up, lsr);
break;
case UART_IIR_RX_TIMEOUT:
/* FALLTHROUGH */
--
1.7.12.rc3

2012-08-21 09:22:15

by Felipe Balbi

[permalink] [raw]
Subject: [RFC/PATCH 05/13] serial: omap: drop DMA support

The current support is known to be broken and
a later patch will come re-adding it using
dma engine API.

Signed-off-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/omap-serial.c | 330 ++-------------------------------------
1 file changed, 12 insertions(+), 318 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index fb81366..4d74a9b 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -33,14 +33,12 @@
#include <linux/tty.h>
#include <linux/tty_flip.h>
#include <linux/io.h>
-#include <linux/dma-mapping.h>
#include <linux/clk.h>
#include <linux/serial_core.h>
#include <linux/irq.h>
#include <linux/pm_runtime.h>
#include <linux/of.h>

-#include <plat/dma.h>
#include <plat/dmtimer.h>
#include <plat/omap-serial.h>

@@ -74,9 +72,6 @@
static struct uart_omap_port *ui[OMAP_MAX_HSUART_PORTS];

/* Forward declaration of functions */
-static void uart_tx_dma_callback(int lch, u16 ch_status, void *data);
-static void serial_omap_rxdma_poll(unsigned long uart_no);
-static int serial_omap_start_rxdma(struct uart_omap_port *up);
static void serial_omap_mdr1_errataset(struct uart_omap_port *up, u8 mdr1);

static struct workqueue_struct *serial_omap_uart_wq;
@@ -160,19 +155,6 @@ serial_omap_get_divisor(struct uart_port *port, unsigned int baud)
return port->uartclk/(baud * divisor);
}

-static void serial_omap_stop_rxdma(struct uart_omap_port *up)
-{
- if (up->uart_dma.rx_dma_used) {
- del_timer(&up->uart_dma.rx_timer);
- omap_stop_dma(up->uart_dma.rx_dma_channel);
- omap_free_dma(up->uart_dma.rx_dma_channel);
- up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
- up->uart_dma.rx_dma_used = false;
- pm_runtime_mark_last_busy(up->dev);
- pm_runtime_put_autosuspend(up->dev);
- }
-}
-
static void serial_omap_enable_ms(struct uart_port *port)
{
struct uart_omap_port *up = to_uart_omap_port(port);
@@ -188,22 +170,6 @@ static void serial_omap_enable_ms(struct uart_port *port)
static void serial_omap_stop_tx(struct uart_port *port)
{
struct uart_omap_port *up = to_uart_omap_port(port);
- struct omap_uart_port_info *pdata = up->dev->platform_data;
-
- if (up->use_dma &&
- up->uart_dma.tx_dma_channel != OMAP_UART_DMA_CH_FREE) {
- /*
- * Check if dma is still active. If yes do nothing,
- * return. Else stop dma
- */
- if (omap_get_dma_active_status(up->uart_dma.tx_dma_channel))
- return;
- omap_stop_dma(up->uart_dma.tx_dma_channel);
- omap_free_dma(up->uart_dma.tx_dma_channel);
- up->uart_dma.tx_dma_channel = OMAP_UART_DMA_CH_FREE;
- pm_runtime_mark_last_busy(up->dev);
- pm_runtime_put_autosuspend(up->dev);
- }

pm_runtime_get_sync(up->dev);
if (up->ier & UART_IER_THRI) {
@@ -211,8 +177,7 @@ static void serial_omap_stop_tx(struct uart_port *port)
serial_out(up, UART_IER, up->ier);
}

- if (!up->use_dma && pdata)
- serial_omap_set_forceidle(up);
+ serial_omap_set_forceidle(up);

pm_runtime_mark_last_busy(up->dev);
pm_runtime_put_autosuspend(up->dev);
@@ -223,8 +188,6 @@ static void serial_omap_stop_rx(struct uart_port *port)
struct uart_omap_port *up = to_uart_omap_port(port);

pm_runtime_get_sync(up->dev);
- if (up->use_dma)
- serial_omap_stop_rxdma(up);
up->ier &= ~UART_IER_RLSI;
up->port.read_status_mask &= ~UART_LSR_DR;
serial_out(up, UART_IER, up->ier);
@@ -342,67 +305,12 @@ static inline void serial_omap_enable_ier_thri(struct uart_omap_port *up)
static void serial_omap_start_tx(struct uart_port *port)
{
struct uart_omap_port *up = to_uart_omap_port(port);
- struct circ_buf *xmit;
- unsigned int start;
- int ret = 0;
-
- if (!up->use_dma) {
- pm_runtime_get_sync(up->dev);
- serial_omap_enable_ier_thri(up);
- serial_omap_set_noidle(up);
- pm_runtime_mark_last_busy(up->dev);
- pm_runtime_put_autosuspend(up->dev);
- return;
- }
-
- if (up->uart_dma.tx_dma_used)
- return;
-
- xmit = &up->port.state->xmit;
-
- if (up->uart_dma.tx_dma_channel == OMAP_UART_DMA_CH_FREE) {
- pm_runtime_get_sync(up->dev);
- ret = omap_request_dma(up->uart_dma.uart_dma_tx,
- "UART Tx DMA",
- (void *)uart_tx_dma_callback, up,
- &(up->uart_dma.tx_dma_channel));

- if (ret < 0) {
- serial_omap_enable_ier_thri(up);
- return;
- }
- }
- spin_lock(&(up->uart_dma.tx_lock));
- up->uart_dma.tx_dma_used = true;
- spin_unlock(&(up->uart_dma.tx_lock));
-
- start = up->uart_dma.tx_buf_dma_phys +
- (xmit->tail & (UART_XMIT_SIZE - 1));
-
- up->uart_dma.tx_buf_size = uart_circ_chars_pending(xmit);
- /*
- * It is a circular buffer. See if the buffer has wounded back.
- * If yes it will have to be transferred in two separate dma
- * transfers
- */
- if (start + up->uart_dma.tx_buf_size >=
- up->uart_dma.tx_buf_dma_phys + UART_XMIT_SIZE)
- up->uart_dma.tx_buf_size =
- (up->uart_dma.tx_buf_dma_phys +
- UART_XMIT_SIZE) - start;
-
- omap_set_dma_dest_params(up->uart_dma.tx_dma_channel, 0,
- OMAP_DMA_AMODE_CONSTANT,
- up->uart_dma.uart_base, 0, 0);
- omap_set_dma_src_params(up->uart_dma.tx_dma_channel, 0,
- OMAP_DMA_AMODE_POST_INC, start, 0, 0);
- omap_set_dma_transfer_params(up->uart_dma.tx_dma_channel,
- OMAP_DMA_DATA_TYPE_S8,
- up->uart_dma.tx_buf_size, 1,
- OMAP_DMA_SYNC_ELEMENT,
- up->uart_dma.uart_dma_tx, 0);
- /* FIXME: Cache maintenance needed here? */
- omap_start_dma(up->uart_dma.tx_dma_channel);
+ pm_runtime_get_sync(up->dev);
+ serial_omap_enable_ier_thri(up);
+ serial_omap_set_noidle(up);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

static unsigned int check_modem_status(struct uart_omap_port *up)
@@ -455,16 +363,8 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
spin_lock_irqsave(&up->port.lock, flags);
lsr = serial_in(up, UART_LSR);
if (iir & UART_IIR_RLSI) {
- if (!up->use_dma) {
- if (lsr & UART_LSR_DR)
- receive_chars(up, &lsr);
- } else {
- up->ier &= ~(UART_IER_RDI | UART_IER_RLSI);
- serial_out(up, UART_IER, up->ier);
- if ((serial_omap_start_rxdma(up) != 0) &&
- (lsr & UART_LSR_DR))
- receive_chars(up, &lsr);
- }
+ if (lsr & UART_LSR_DR)
+ receive_chars(up, &lsr);
}

check_modem_status(up);
@@ -605,20 +505,6 @@ static int serial_omap_startup(struct uart_port *port)
spin_unlock_irqrestore(&up->port.lock, flags);

up->msr_saved_flags = 0;
- if (up->use_dma) {
- free_page((unsigned long)up->port.state->xmit.buf);
- up->port.state->xmit.buf = dma_alloc_coherent(NULL,
- UART_XMIT_SIZE,
- (dma_addr_t *)&(up->uart_dma.tx_buf_dma_phys),
- 0);
- init_timer(&(up->uart_dma.rx_timer));
- up->uart_dma.rx_timer.function = serial_omap_rxdma_poll;
- up->uart_dma.rx_timer.data = up->port.line;
- /* Currently the buffer size is 4KB. Can increase it */
- up->uart_dma.rx_buf = dma_alloc_coherent(NULL,
- up->uart_dma.rx_buf_size,
- (dma_addr_t *)&(up->uart_dma.rx_buf_dma_phys), 0);
- }
/*
* Finally, enable interrupts. Note: Modem status interrupts
* are set via set_termios(), which will be occurring imminently
@@ -666,17 +552,6 @@ static void serial_omap_shutdown(struct uart_port *port)
*/
if (serial_in(up, UART_LSR) & UART_LSR_DR)
(void) serial_in(up, UART_RX);
- if (up->use_dma) {
- dma_free_coherent(up->port.dev,
- UART_XMIT_SIZE, up->port.state->xmit.buf,
- up->uart_dma.tx_buf_dma_phys);
- up->port.state->xmit.buf = NULL;
- serial_omap_stop_rx(port);
- dma_free_coherent(up->port.dev,
- up->uart_dma.rx_buf_size, up->uart_dma.rx_buf,
- up->uart_dma.rx_buf_dma_phys);
- up->uart_dma.rx_buf = NULL;
- }

pm_runtime_put(up->dev);
free_irq(up->port.irq, up);
@@ -800,8 +675,6 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,

up->fcr = UART_FCR_R_TRIG_01 | UART_FCR_T_TRIG_01 |
UART_FCR_ENABLE_FIFO;
- if (up->use_dma)
- up->fcr |= UART_FCR_DMA_SELECT;

/*
* Ok, we're now changing the port state. Do it with
@@ -877,14 +750,9 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,

up->scr |= OMAP_UART_SCR_RX_TRIG_GRANU1_MASK;

- if (up->use_dma) {
- serial_out(up, UART_TI752_TLR, 0);
- up->scr |= UART_FCR_TRIGGER_4;
- } else {
- /* Set receive FIFO threshold to 1 byte */
- up->fcr &= ~OMAP_UART_FCR_RX_FIFO_TRIG_MASK;
- up->fcr |= (0x1 << OMAP_UART_FCR_RX_FIFO_TRIG_SHIFT);
- }
+ /* Set receive FIFO threshold to 1 byte */
+ up->fcr &= ~OMAP_UART_FCR_RX_FIFO_TRIG_MASK;
+ up->fcr |= (0x1 << OMAP_UART_FCR_RX_FIFO_TRIG_SHIFT);

serial_out(up, UART_FCR, up->fcr);
serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
@@ -1253,149 +1121,6 @@ static int serial_omap_resume(struct device *dev)
}
#endif

-static void serial_omap_rxdma_poll(unsigned long uart_no)
-{
- struct uart_omap_port *up = ui[uart_no];
- unsigned int curr_dma_pos, curr_transmitted_size;
- int ret = 0;
-
- curr_dma_pos = omap_get_dma_dst_pos(up->uart_dma.rx_dma_channel);
- if ((curr_dma_pos == up->uart_dma.prev_rx_dma_pos) ||
- (curr_dma_pos == 0)) {
- if (jiffies_to_msecs(jiffies - up->port_activity) <
- up->uart_dma.rx_timeout) {
- mod_timer(&up->uart_dma.rx_timer, jiffies +
- usecs_to_jiffies(up->uart_dma.rx_poll_rate));
- } else {
- serial_omap_stop_rxdma(up);
- up->ier |= (UART_IER_RDI | UART_IER_RLSI);
- serial_out(up, UART_IER, up->ier);
- }
- return;
- }
-
- curr_transmitted_size = curr_dma_pos -
- up->uart_dma.prev_rx_dma_pos;
- up->port.icount.rx += curr_transmitted_size;
- tty_insert_flip_string(up->port.state->port.tty,
- up->uart_dma.rx_buf +
- (up->uart_dma.prev_rx_dma_pos -
- up->uart_dma.rx_buf_dma_phys),
- curr_transmitted_size);
- tty_flip_buffer_push(up->port.state->port.tty);
- up->uart_dma.prev_rx_dma_pos = curr_dma_pos;
- if (up->uart_dma.rx_buf_size +
- up->uart_dma.rx_buf_dma_phys == curr_dma_pos) {
- ret = serial_omap_start_rxdma(up);
- if (ret < 0) {
- serial_omap_stop_rxdma(up);
- up->ier |= (UART_IER_RDI | UART_IER_RLSI);
- serial_out(up, UART_IER, up->ier);
- }
- } else {
- mod_timer(&up->uart_dma.rx_timer, jiffies +
- usecs_to_jiffies(up->uart_dma.rx_poll_rate));
- }
- up->port_activity = jiffies;
-}
-
-static void uart_rx_dma_callback(int lch, u16 ch_status, void *data)
-{
- return;
-}
-
-static int serial_omap_start_rxdma(struct uart_omap_port *up)
-{
- int ret = 0;
-
- if (up->uart_dma.rx_dma_channel == -1) {
- pm_runtime_get_sync(up->dev);
- ret = omap_request_dma(up->uart_dma.uart_dma_rx,
- "UART Rx DMA",
- (void *)uart_rx_dma_callback, up,
- &(up->uart_dma.rx_dma_channel));
- if (ret < 0)
- return ret;
-
- omap_set_dma_src_params(up->uart_dma.rx_dma_channel, 0,
- OMAP_DMA_AMODE_CONSTANT,
- up->uart_dma.uart_base, 0, 0);
- omap_set_dma_dest_params(up->uart_dma.rx_dma_channel, 0,
- OMAP_DMA_AMODE_POST_INC,
- up->uart_dma.rx_buf_dma_phys, 0, 0);
- omap_set_dma_transfer_params(up->uart_dma.rx_dma_channel,
- OMAP_DMA_DATA_TYPE_S8,
- up->uart_dma.rx_buf_size, 1,
- OMAP_DMA_SYNC_ELEMENT,
- up->uart_dma.uart_dma_rx, 0);
- }
- up->uart_dma.prev_rx_dma_pos = up->uart_dma.rx_buf_dma_phys;
- /* FIXME: Cache maintenance needed here? */
- omap_start_dma(up->uart_dma.rx_dma_channel);
- mod_timer(&up->uart_dma.rx_timer, jiffies +
- usecs_to_jiffies(up->uart_dma.rx_poll_rate));
- up->uart_dma.rx_dma_used = true;
- return ret;
-}
-
-static void serial_omap_continue_tx(struct uart_omap_port *up)
-{
- struct circ_buf *xmit = &up->port.state->xmit;
- unsigned int start = up->uart_dma.tx_buf_dma_phys
- + (xmit->tail & (UART_XMIT_SIZE - 1));
-
- if (uart_circ_empty(xmit))
- return;
-
- up->uart_dma.tx_buf_size = uart_circ_chars_pending(xmit);
- /*
- * It is a circular buffer. See if the buffer has wounded back.
- * If yes it will have to be transferred in two separate dma
- * transfers
- */
- if (start + up->uart_dma.tx_buf_size >=
- up->uart_dma.tx_buf_dma_phys + UART_XMIT_SIZE)
- up->uart_dma.tx_buf_size =
- (up->uart_dma.tx_buf_dma_phys + UART_XMIT_SIZE) - start;
- omap_set_dma_dest_params(up->uart_dma.tx_dma_channel, 0,
- OMAP_DMA_AMODE_CONSTANT,
- up->uart_dma.uart_base, 0, 0);
- omap_set_dma_src_params(up->uart_dma.tx_dma_channel, 0,
- OMAP_DMA_AMODE_POST_INC, start, 0, 0);
- omap_set_dma_transfer_params(up->uart_dma.tx_dma_channel,
- OMAP_DMA_DATA_TYPE_S8,
- up->uart_dma.tx_buf_size, 1,
- OMAP_DMA_SYNC_ELEMENT,
- up->uart_dma.uart_dma_tx, 0);
- /* FIXME: Cache maintenance needed here? */
- omap_start_dma(up->uart_dma.tx_dma_channel);
-}
-
-static void uart_tx_dma_callback(int lch, u16 ch_status, void *data)
-{
- struct uart_omap_port *up = data;
- struct circ_buf *xmit = &up->port.state->xmit;
-
- xmit->tail = (xmit->tail + up->uart_dma.tx_buf_size) & \
- (UART_XMIT_SIZE - 1);
- up->port.icount.tx += up->uart_dma.tx_buf_size;
-
- if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
- uart_write_wakeup(&up->port);
-
- if (uart_circ_empty(xmit)) {
- spin_lock(&(up->uart_dma.tx_lock));
- serial_omap_stop_tx(&up->port);
- up->uart_dma.tx_dma_used = false;
- spin_unlock(&(up->uart_dma.tx_lock));
- } else {
- omap_stop_dma(up->uart_dma.tx_dma_channel);
- serial_omap_continue_tx(up);
- }
- up->port_activity = jiffies;
- return;
-}
-
static void omap_serial_fill_features_erratas(struct uart_omap_port *up)
{
u32 mvr, scheme;
@@ -1465,7 +1190,7 @@ static struct omap_uart_port_info *of_get_uart_port_info(struct device *dev)
static int serial_omap_probe(struct platform_device *pdev)
{
struct uart_omap_port *up;
- struct resource *mem, *irq, *dma_tx, *dma_rx;
+ struct resource *mem, *irq;
struct omap_uart_port_info *omap_up_info = pdev->dev.platform_data;
int ret = -ENOSPC;

@@ -1490,14 +1215,6 @@ static int serial_omap_probe(struct platform_device *pdev)
return -EBUSY;
}

- dma_rx = platform_get_resource_byname(pdev, IORESOURCE_DMA, "rx");
- if (!dma_rx)
- return -ENXIO;
-
- dma_tx = platform_get_resource_byname(pdev, IORESOURCE_DMA, "tx");
- if (!dma_tx)
- return -ENXIO;
-
up = devm_kzalloc(&pdev->dev, sizeof(*up), GFP_KERNEL);
if (!up)
return -ENOMEM;
@@ -1541,20 +1258,6 @@ static int serial_omap_probe(struct platform_device *pdev)
dev_warn(&pdev->dev, "No clock speed specified: using default:"
"%d\n", DEFAULT_CLK_SPEED);
}
- up->uart_dma.uart_base = mem->start;
-
- if (omap_up_info->dma_enabled) {
- up->uart_dma.uart_dma_tx = dma_tx->start;
- up->uart_dma.uart_dma_rx = dma_rx->start;
- up->use_dma = 1;
- up->uart_dma.rx_buf_size = omap_up_info->dma_rx_buf_size;
- up->uart_dma.rx_timeout = omap_up_info->dma_rx_timeout;
- up->uart_dma.rx_poll_rate = omap_up_info->dma_rx_poll_rate;
- spin_lock_init(&(up->uart_dma.tx_lock));
- spin_lock_init(&(up->uart_dma.rx_lock));
- up->uart_dma.tx_dma_channel = OMAP_UART_DMA_CH_FREE;
- up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
- }

up->latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
up->calc_latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
@@ -1697,10 +1400,6 @@ static int serial_omap_runtime_suspend(struct device *dev)
}
}

- /* Errata i291 */
- if (up->use_dma && (up->errata & UART_ERRATA_i291_DMA_FORCEIDLE))
- serial_omap_set_forceidle(up);
-
up->latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
schedule_work(&up->qos_work);

@@ -1718,11 +1417,6 @@ static int serial_omap_runtime_resume(struct device *dev)
if (up->context_loss_cnt != loss_cnt)
serial_omap_restore_context(up);

- /* Errata i291 */
- if ((up->errata & UART_ERRATA_i291_DMA_FORCEIDLE) &&
- up->use_dma)
- serial_omap_set_noidle(up);
-
up->latency = up->calc_latency;
schedule_work(&up->qos_work);
}
--
1.7.12.rc3

2012-08-21 09:22:11

by Felipe Balbi

[permalink] [raw]
Subject: [RFC/PATCH 08/13] serial: omap: refactor receive_chars() into rdi/rlsi handlers

receive_chars() was getting too big and too difficult
to follow. By splitting it into separate RDI and RSLI
handlers, we have smaller functions which are easy
to understand and only touch the pieces which they need
to touch.

Signed-off-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/omap-serial.c | 203 +++++++++++++++++++--------------------
1 file changed, 100 insertions(+), 103 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 1ca08b8..74a4f0a 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -195,74 +195,6 @@ static void serial_omap_stop_rx(struct uart_port *port)
pm_runtime_put_autosuspend(up->dev);
}

-static inline void receive_chars(struct uart_omap_port *up,
- unsigned int *status)
-{
- struct tty_struct *tty = up->port.state->port.tty;
- unsigned int flag, lsr = *status;
- unsigned char ch = 0;
- int max_count = 256;
-
- do {
- if (likely(lsr & UART_LSR_DR))
- ch = serial_in(up, UART_RX);
- flag = TTY_NORMAL;
- up->port.icount.rx++;
-
- if (unlikely(lsr & UART_LSR_BRK_ERROR_BITS)) {
- /*
- * For statistics only
- */
- if (lsr & UART_LSR_BI) {
- lsr &= ~(UART_LSR_FE | UART_LSR_PE);
- up->port.icount.brk++;
- /*
- * We do the SysRQ and SAK checking
- * here because otherwise the break
- * may get masked by ignore_status_mask
- * or read_status_mask.
- */
- if (uart_handle_break(&up->port))
- goto ignore_char;
- } else if (lsr & UART_LSR_PE) {
- up->port.icount.parity++;
- } else if (lsr & UART_LSR_FE) {
- up->port.icount.frame++;
- }
-
- if (lsr & UART_LSR_OE)
- up->port.icount.overrun++;
-
- /*
- * Mask off conditions which should be ignored.
- */
- lsr &= up->port.read_status_mask;
-
-#ifdef CONFIG_SERIAL_OMAP_CONSOLE
- if (up->port.line == up->port.cons->index) {
- /* Recover the break flag from console xmit */
- lsr |= up->lsr_break_flag;
- }
-#endif
- if (lsr & UART_LSR_BI)
- flag = TTY_BREAK;
- else if (lsr & UART_LSR_PE)
- flag = TTY_PARITY;
- else if (lsr & UART_LSR_FE)
- flag = TTY_FRAME;
- }
-
- if (uart_handle_sysrq_char(&up->port, ch))
- goto ignore_char;
- uart_insert_char(&up->port, lsr, UART_LSR_OE, ch, flag);
-ignore_char:
- lsr = serial_in(up, UART_LSR);
- } while ((lsr & (UART_LSR_DR | UART_LSR_BI)) && (max_count-- > 0));
- spin_unlock(&up->port.lock);
- tty_flip_buffer_push(tty);
- spin_lock(&up->port.lock);
-}
-
static void transmit_chars(struct uart_omap_port *up)
{
struct circ_buf *xmit = &up->port.state->xmit;
@@ -341,6 +273,68 @@ static unsigned int check_modem_status(struct uart_omap_port *up)
return status;
}

+static void serial_omap_rlsi(struct uart_omap_port *up, unsigned int lsr)
+{
+ unsigned int flag;
+
+ up->port.icount.rx++;
+ flag = TTY_NORMAL;
+
+ if (lsr & UART_LSR_BI) {
+ flag = TTY_BREAK;
+ lsr &= ~(UART_LSR_FE | UART_LSR_PE);
+ up->port.icount.brk++;
+ /*
+ * We do the SysRQ and SAK checking
+ * here because otherwise the break
+ * may get masked by ignore_status_mask
+ * or read_status_mask.
+ */
+ if (uart_handle_break(&up->port))
+ return;
+
+ }
+
+ if (lsr & UART_LSR_PE) {
+ flag = TTY_PARITY;
+ up->port.icount.parity++;
+ }
+
+ if (lsr & UART_LSR_FE) {
+ flag = TTY_FRAME;
+ up->port.icount.frame++;
+ }
+
+ if (lsr & UART_LSR_OE)
+ up->port.icount.overrun++;
+
+#ifdef CONFIG_SERIAL_OMAP_CONSOLE
+ if (up->port.line == up->port.cons->index) {
+ /* Recover the break flag from console xmit */
+ lsr |= up->lsr_break_flag;
+ }
+#endif
+ uart_insert_char(&up->port, lsr, UART_LSR_OE, 0, flag);
+}
+
+static void serial_omap_rdi(struct uart_omap_port *up, unsigned int lsr)
+{
+ unsigned char ch = 0;
+ unsigned int flag;
+
+ if (!(lsr & UART_LSR_DR))
+ return;
+
+ ch = serial_in(up, UART_RX);
+ flag = TTY_NORMAL;
+ up->port.icount.rx++;
+
+ if (uart_handle_sysrq_char(&up->port, ch))
+ return;
+
+ uart_insert_char(&up->port, lsr, UART_LSR_OE, ch, flag);
+}
+
/**
* serial_omap_irq() - This handles the interrupt from one port
* @irq: uart port irq number
@@ -349,52 +343,55 @@ static unsigned int check_modem_status(struct uart_omap_port *up)
static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
{
struct uart_omap_port *up = dev_id;
+ struct tty_struct *tty = up->port.state->port.tty;
unsigned int iir, lsr;
unsigned int type;
unsigned long flags;
+ int max_count = 256;

spin_lock_irqsave(&up->port.lock, flags);
pm_runtime_get_sync(up->dev);
- iir = serial_in(up, UART_IIR);
-again:
- if (iir & UART_IIR_NO_INT)
- goto out;

- lsr = serial_in(up, UART_LSR);
+ do {
+ iir = serial_in(up, UART_IIR);
+ if (iir & UART_IIR_NO_INT)
+ break;

- /* extract IRQ type from IIR register */
- type = iir & 0x3e;
+ lsr = serial_in(up, UART_LSR);

- switch (type) {
- case UART_IIR_MSI:
- check_modem_status(up);
- break;
- case UART_IIR_THRI:
- if (lsr & UART_LSR_THRE)
- transmit_chars(up);
- break;
- case UART_IIR_RDI:
- if (lsr & UART_LSR_DR)
- receive_chars(up, &lsr);
- break;
- case UART_IIR_RLSI:
- if (lsr & UART_LSR_BRK_ERROR_BITS)
- receive_chars(up, &lsr);
- break;
- case UART_IIR_RX_TIMEOUT:
- receive_chars(up, &lsr);
- break;
- case UART_IIR_CTS_RTS_DSR:
- iir = serial_in(up, UART_IIR);
- goto again;
- case UART_IIR_XOFF:
- /* FALLTHROUGH */
- default:
- break;
- }
+ /* extract IRQ type from IIR register */
+ type = iir & 0x3e;
+
+ switch (type) {
+ case UART_IIR_MSI:
+ check_modem_status(up);
+ break;
+ case UART_IIR_THRI:
+ if (lsr & UART_LSR_THRE)
+ transmit_chars(up);
+ break;
+ case UART_IIR_RX_TIMEOUT:
+ /* FALLTHROUGH */
+ case UART_IIR_RDI:
+ serial_omap_rdi(up, lsr);
+ break;
+ case UART_IIR_RLSI:
+ serial_omap_rlsi(up, lsr);
+ break;
+ case UART_IIR_CTS_RTS_DSR:
+ /* simply try again */
+ break;
+ case UART_IIR_XOFF:
+ /* FALLTHROUGH */
+ default:
+ break;
+ }
+ } while (!(iir & UART_IIR_NO_INT) && max_count--);

-out:
spin_unlock_irqrestore(&up->port.lock, flags);
+
+ tty_flip_buffer_push(tty);
+
pm_runtime_mark_last_busy(up->dev);
pm_runtime_put_autosuspend(up->dev);
up->port_activity = jiffies;
--
1.7.12.rc3

2012-08-21 09:23:14

by Felipe Balbi

[permalink] [raw]
Subject: [RFC/PATCH 04/13] serial: omap: don't access the platform_device

The driver doesn't need to know about its platform_device.

Everything the driver needs can be done through the
struct device pointer. In case we need to use the
OMAP-specific PM function pointers, those can make
sure to find the device's platform_device pointer
so they can find the struct omap_device through
pdev->archdata field.

Signed-off-by: Felipe Balbi <[email protected]>
---
arch/arm/mach-omap2/serial.c | 15 ++--
arch/arm/plat-omap/include/plat/omap-serial.h | 10 +--
drivers/tty/serial/omap-serial.c | 124 +++++++++++++-------------
3 files changed, 76 insertions(+), 73 deletions(-)

diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index c1b93c7..8f07841 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -81,8 +81,9 @@ static struct omap_uart_port_info omap_serial_default_info[] __initdata = {
};

#ifdef CONFIG_PM
-static void omap_uart_enable_wakeup(struct platform_device *pdev, bool enable)
+static void omap_uart_enable_wakeup(struct device *dev, bool enable)
{
+ struct platform_device *pdev = to_platform_device(dev);
struct omap_device *od = to_omap_device(pdev);

if (!od)
@@ -99,15 +100,17 @@ static void omap_uart_enable_wakeup(struct platform_device *pdev, bool enable)
* in Smartidle Mode When Configured for DMA Operations.
* WA: configure uart in force idle mode.
*/
-static void omap_uart_set_noidle(struct platform_device *pdev)
+static void omap_uart_set_noidle(struct device *dev)
{
+ struct platform_device *pdev = to_platform_device(dev);
struct omap_device *od = to_omap_device(pdev);

omap_hwmod_set_slave_idlemode(od->hwmods[0], HWMOD_IDLEMODE_NO);
}

-static void omap_uart_set_smartidle(struct platform_device *pdev)
+static void omap_uart_set_smartidle(struct device *dev)
{
+ struct platform_device *pdev = to_platform_device(dev);
struct omap_device *od = to_omap_device(pdev);
u8 idlemode;

@@ -120,10 +123,10 @@ static void omap_uart_set_smartidle(struct platform_device *pdev)
}

#else
-static void omap_uart_enable_wakeup(struct platform_device *pdev, bool enable)
+static void omap_uart_enable_wakeup(struct device *dev, bool enable)
{}
-static void omap_uart_set_noidle(struct platform_device *pdev) {}
-static void omap_uart_set_smartidle(struct platform_device *pdev) {}
+static void omap_uart_set_noidle(struct device *dev) {}
+static void omap_uart_set_smartidle(struct device *dev) {}
#endif /* CONFIG_PM */

#ifdef CONFIG_OMAP_MUX
diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
index f3b35d9..743ac80 100644
--- a/arch/arm/plat-omap/include/plat/omap-serial.h
+++ b/arch/arm/plat-omap/include/plat/omap-serial.h
@@ -18,7 +18,7 @@
#define __OMAP_SERIAL_H__

#include <linux/serial_core.h>
-#include <linux/platform_device.h>
+#include <linux/device.h>
#include <linux/pm_qos.h>

#include <plat/mux.h>
@@ -71,9 +71,9 @@ struct omap_uart_port_info {
unsigned int dma_rx_poll_rate;

int (*get_context_loss_count)(struct device *);
- void (*set_forceidle)(struct platform_device *);
- void (*set_noidle)(struct platform_device *);
- void (*enable_wakeup)(struct platform_device *, bool);
+ void (*set_forceidle)(struct device *);
+ void (*set_noidle)(struct device *);
+ void (*enable_wakeup)(struct device *, bool);
};

struct uart_omap_dma {
@@ -105,7 +105,7 @@ struct uart_omap_dma {
struct uart_omap_port {
struct uart_port port;
struct uart_omap_dma uart_dma;
- struct platform_device *pdev;
+ struct device *dev;

unsigned char ier;
unsigned char lcr;
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index c3e6fa0..fb81366 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -103,36 +103,36 @@ static inline void serial_omap_clear_fifos(struct uart_omap_port *up)

static int serial_omap_get_context_loss_count(struct uart_omap_port *up)
{
- struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
+ struct omap_uart_port_info *pdata = up->dev->platform_data;

if (!pdata->get_context_loss_count)
return 0;

- return pdata->get_context_loss_count(&up->pdev->dev);
+ return pdata->get_context_loss_count(up->dev);
}

static void serial_omap_set_forceidle(struct uart_omap_port *up)
{
- struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
+ struct omap_uart_port_info *pdata = up->dev->platform_data;

if (pdata->set_forceidle)
- pdata->set_forceidle(up->pdev);
+ pdata->set_forceidle(up->dev);
}

static void serial_omap_set_noidle(struct uart_omap_port *up)
{
- struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
+ struct omap_uart_port_info *pdata = up->dev->platform_data;

if (pdata->set_noidle)
- pdata->set_noidle(up->pdev);
+ pdata->set_noidle(up->dev);
}

static void serial_omap_enable_wakeup(struct uart_omap_port *up, bool enable)
{
- struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
+ struct omap_uart_port_info *pdata = up->dev->platform_data;

if (pdata->enable_wakeup)
- pdata->enable_wakeup(up->pdev, enable);
+ pdata->enable_wakeup(up->dev, enable);
}

/*
@@ -168,8 +168,8 @@ static void serial_omap_stop_rxdma(struct uart_omap_port *up)
omap_free_dma(up->uart_dma.rx_dma_channel);
up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
up->uart_dma.rx_dma_used = false;
- pm_runtime_mark_last_busy(&up->pdev->dev);
- pm_runtime_put_autosuspend(&up->pdev->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}
}

@@ -179,16 +179,16 @@ static void serial_omap_enable_ms(struct uart_port *port)

dev_dbg(up->port.dev, "serial_omap_enable_ms+%d\n", up->port.line);

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
up->ier |= UART_IER_MSI;
serial_out(up, UART_IER, up->ier);
- pm_runtime_put(&up->pdev->dev);
+ pm_runtime_put(up->dev);
}

static void serial_omap_stop_tx(struct uart_port *port)
{
struct uart_omap_port *up = to_uart_omap_port(port);
- struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
+ struct omap_uart_port_info *pdata = up->dev->platform_data;

if (up->use_dma &&
up->uart_dma.tx_dma_channel != OMAP_UART_DMA_CH_FREE) {
@@ -201,11 +201,11 @@ static void serial_omap_stop_tx(struct uart_port *port)
omap_stop_dma(up->uart_dma.tx_dma_channel);
omap_free_dma(up->uart_dma.tx_dma_channel);
up->uart_dma.tx_dma_channel = OMAP_UART_DMA_CH_FREE;
- pm_runtime_mark_last_busy(&up->pdev->dev);
- pm_runtime_put_autosuspend(&up->pdev->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
if (up->ier & UART_IER_THRI) {
up->ier &= ~UART_IER_THRI;
serial_out(up, UART_IER, up->ier);
@@ -214,22 +214,22 @@ static void serial_omap_stop_tx(struct uart_port *port)
if (!up->use_dma && pdata)
serial_omap_set_forceidle(up);

- pm_runtime_mark_last_busy(&up->pdev->dev);
- pm_runtime_put_autosuspend(&up->pdev->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

static void serial_omap_stop_rx(struct uart_port *port)
{
struct uart_omap_port *up = to_uart_omap_port(port);

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
if (up->use_dma)
serial_omap_stop_rxdma(up);
up->ier &= ~UART_IER_RLSI;
up->port.read_status_mask &= ~UART_LSR_DR;
serial_out(up, UART_IER, up->ier);
- pm_runtime_mark_last_busy(&up->pdev->dev);
- pm_runtime_put_autosuspend(&up->pdev->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

static inline void receive_chars(struct uart_omap_port *up,
@@ -347,11 +347,11 @@ static void serial_omap_start_tx(struct uart_port *port)
int ret = 0;

if (!up->use_dma) {
- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
serial_omap_enable_ier_thri(up);
serial_omap_set_noidle(up);
- pm_runtime_mark_last_busy(&up->pdev->dev);
- pm_runtime_put_autosuspend(&up->pdev->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
return;
}

@@ -361,7 +361,7 @@ static void serial_omap_start_tx(struct uart_port *port)
xmit = &up->port.state->xmit;

if (up->uart_dma.tx_dma_channel == OMAP_UART_DMA_CH_FREE) {
- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
ret = omap_request_dma(up->uart_dma.uart_dma_tx,
"UART Tx DMA",
(void *)uart_tx_dma_callback, up,
@@ -444,11 +444,11 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
unsigned int iir, lsr;
unsigned long flags;

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
iir = serial_in(up, UART_IIR);
if (iir & UART_IIR_NO_INT) {
- pm_runtime_mark_last_busy(&up->pdev->dev);
- pm_runtime_put_autosuspend(&up->pdev->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
return IRQ_HANDLED;
}

@@ -472,8 +472,8 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
transmit_chars(up);

spin_unlock_irqrestore(&up->port.lock, flags);
- pm_runtime_mark_last_busy(&up->pdev->dev);
- pm_runtime_put_autosuspend(&up->pdev->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);

up->port_activity = jiffies;
return IRQ_HANDLED;
@@ -485,12 +485,12 @@ static unsigned int serial_omap_tx_empty(struct uart_port *port)
unsigned long flags = 0;
unsigned int ret = 0;

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
dev_dbg(up->port.dev, "serial_omap_tx_empty+%d\n", up->port.line);
spin_lock_irqsave(&up->port.lock, flags);
ret = serial_in(up, UART_LSR) & UART_LSR_TEMT ? TIOCSER_TEMT : 0;
spin_unlock_irqrestore(&up->port.lock, flags);
- pm_runtime_put(&up->pdev->dev);
+ pm_runtime_put(up->dev);
return ret;
}

@@ -500,9 +500,9 @@ static unsigned int serial_omap_get_mctrl(struct uart_port *port)
unsigned int status;
unsigned int ret = 0;

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
status = check_modem_status(up);
- pm_runtime_put(&up->pdev->dev);
+ pm_runtime_put(up->dev);

dev_dbg(up->port.dev, "serial_omap_get_mctrl+%d\n", up->port.line);

@@ -534,11 +534,11 @@ static void serial_omap_set_mctrl(struct uart_port *port, unsigned int mctrl)
if (mctrl & TIOCM_LOOP)
mcr |= UART_MCR_LOOP;

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
up->mcr = serial_in(up, UART_MCR);
up->mcr |= mcr;
serial_out(up, UART_MCR, up->mcr);
- pm_runtime_put(&up->pdev->dev);
+ pm_runtime_put(up->dev);
}

static void serial_omap_break_ctl(struct uart_port *port, int break_state)
@@ -547,7 +547,7 @@ static void serial_omap_break_ctl(struct uart_port *port, int break_state)
unsigned long flags = 0;

dev_dbg(up->port.dev, "serial_omap_break_ctl+%d\n", up->port.line);
- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
spin_lock_irqsave(&up->port.lock, flags);
if (break_state == -1)
up->lcr |= UART_LCR_SBC;
@@ -555,7 +555,7 @@ static void serial_omap_break_ctl(struct uart_port *port, int break_state)
up->lcr &= ~UART_LCR_SBC;
serial_out(up, UART_LCR, up->lcr);
spin_unlock_irqrestore(&up->port.lock, flags);
- pm_runtime_put(&up->pdev->dev);
+ pm_runtime_put(up->dev);
}

static int serial_omap_startup(struct uart_port *port)
@@ -574,7 +574,7 @@ static int serial_omap_startup(struct uart_port *port)

dev_dbg(up->port.dev, "serial_omap_startup+%d\n", up->port.line);

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
/*
* Clear the FIFO buffers and disable them.
* (they will be reenabled in set_termios())
@@ -630,8 +630,8 @@ static int serial_omap_startup(struct uart_port *port)
/* Enable module level wake up */
serial_out(up, UART_OMAP_WER, OMAP_UART_WER_MOD_WKUP);

- pm_runtime_mark_last_busy(&up->pdev->dev);
- pm_runtime_put_autosuspend(&up->pdev->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
up->port_activity = jiffies;
return 0;
}
@@ -643,7 +643,7 @@ static void serial_omap_shutdown(struct uart_port *port)

dev_dbg(up->port.dev, "serial_omap_shutdown+%d\n", up->port.line);

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
/*
* Disable interrupts from this port
*/
@@ -678,7 +678,7 @@ static void serial_omap_shutdown(struct uart_port *port)
up->uart_dma.rx_buf = NULL;
}

- pm_runtime_put(&up->pdev->dev);
+ pm_runtime_put(up->dev);
free_irq(up->port.irq, up);
}

@@ -807,7 +807,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
* Ok, we're now changing the port state. Do it with
* interrupts disabled.
*/
- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
spin_lock_irqsave(&up->port.lock, flags);

/*
@@ -956,7 +956,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
serial_omap_configure_xonxoff(up, termios);

spin_unlock_irqrestore(&up->port.lock, flags);
- pm_runtime_put(&up->pdev->dev);
+ pm_runtime_put(up->dev);
dev_dbg(up->port.dev, "serial_omap_set_termios+%d\n", up->port.line);
}

@@ -969,7 +969,7 @@ serial_omap_pm(struct uart_port *port, unsigned int state,

dev_dbg(up->port.dev, "serial_omap_pm+%d\n", up->port.line);

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
efr = serial_in(up, UART_EFR);
serial_out(up, UART_EFR, efr | UART_EFR_ECB);
@@ -980,14 +980,14 @@ serial_omap_pm(struct uart_port *port, unsigned int state,
serial_out(up, UART_EFR, efr);
serial_out(up, UART_LCR, 0);

- if (!device_may_wakeup(&up->pdev->dev)) {
+ if (!device_may_wakeup(up->dev)) {
if (!state)
- pm_runtime_forbid(&up->pdev->dev);
+ pm_runtime_forbid(up->dev);
else
- pm_runtime_allow(&up->pdev->dev);
+ pm_runtime_allow(up->dev);
}

- pm_runtime_put(&up->pdev->dev);
+ pm_runtime_put(up->dev);
}

static void serial_omap_release_port(struct uart_port *port)
@@ -1066,10 +1066,10 @@ static void serial_omap_poll_put_char(struct uart_port *port, unsigned char ch)
{
struct uart_omap_port *up = to_uart_omap_port(port);

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
wait_for_xmitr(up);
serial_out(up, UART_TX, ch);
- pm_runtime_put(&up->pdev->dev);
+ pm_runtime_put(up->dev);
}

static int serial_omap_poll_get_char(struct uart_port *port)
@@ -1077,13 +1077,13 @@ static int serial_omap_poll_get_char(struct uart_port *port)
struct uart_omap_port *up = to_uart_omap_port(port);
unsigned int status;

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
status = serial_in(up, UART_LSR);
if (!(status & UART_LSR_DR))
return NO_POLL_CHAR;

status = serial_in(up, UART_RX);
- pm_runtime_put(&up->pdev->dev);
+ pm_runtime_put(up->dev);
return status;
}

@@ -1112,7 +1112,7 @@ serial_omap_console_write(struct console *co, const char *s,
unsigned int ier;
int locked = 1;

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);

local_irq_save(flags);
if (up->port.sysrq)
@@ -1146,8 +1146,8 @@ serial_omap_console_write(struct console *co, const char *s,
if (up->msr_saved_flags)
check_modem_status(up);

- pm_runtime_mark_last_busy(&up->pdev->dev);
- pm_runtime_put_autosuspend(&up->pdev->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
if (locked)
spin_unlock(&up->port.lock);
local_irq_restore(flags);
@@ -1309,7 +1309,7 @@ static int serial_omap_start_rxdma(struct uart_omap_port *up)
int ret = 0;

if (up->uart_dma.rx_dma_channel == -1) {
- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
ret = omap_request_dma(up->uart_dma.uart_dma_rx,
"UART Rx DMA",
(void *)uart_rx_dma_callback, up,
@@ -1421,7 +1421,7 @@ static void omap_serial_fill_features_erratas(struct uart_omap_port *up)
minor = (mvr & OMAP_UART_MVR_MIN_MASK);
break;
default:
- dev_warn(&up->pdev->dev,
+ dev_warn(up->dev,
"Unknown %s revision, defaulting to highest\n",
up->name);
/* highest possible revision */
@@ -1502,7 +1502,7 @@ static int serial_omap_probe(struct platform_device *pdev)
if (!up)
return -ENOMEM;

- up->pdev = pdev;
+ up->dev = &pdev->dev;
up->port.dev = &pdev->dev;
up->port.type = PORT_OMAP;
up->port.iotype = UPIO_MEM;
@@ -1599,7 +1599,7 @@ static int serial_omap_remove(struct platform_device *dev)
struct uart_omap_port *up = platform_get_drvdata(dev);

if (up) {
- pm_runtime_disable(&up->pdev->dev);
+ pm_runtime_disable(up->dev);
uart_remove_one_port(&serial_omap_reg, &up->port);
pm_qos_remove_request(&up->pm_qos_request);
}
@@ -1634,7 +1634,7 @@ static void serial_omap_mdr1_errataset(struct uart_omap_port *up, u8 mdr1)
timeout--;
if (!timeout) {
/* Should *never* happen. we warn and carry on */
- dev_crit(&up->pdev->dev, "Errata i202: timedout %x\n",
+ dev_crit(up->dev, "Errata i202: timedout %x\n",
serial_in(up, UART_LSR));
break;
}
--
1.7.12.rc3

2012-08-21 09:23:57

by Felipe Balbi

[permalink] [raw]
Subject: [RFC/PATCH 02/13] serial: omap: always return IRQ_HANDLED

Even if we enter our IRQ handler just to notice
that the our device didn't generate the IRQ,
that still means "handling" and IRQ, so let's
return IRQ_HANDLED.

Signed-off-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/omap-serial.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 5c0d0bc..b4b95fc 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -417,7 +417,7 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
if (iir & UART_IIR_NO_INT) {
pm_runtime_mark_last_busy(&up->pdev->dev);
pm_runtime_put_autosuspend(&up->pdev->dev);
- return IRQ_NONE;
+ return IRQ_HANDLED;
}

spin_lock_irqsave(&up->port.lock, flags);
--
1.7.12.rc3

2012-08-21 09:44:44

by Santosh Shilimkar

[permalink] [raw]
Subject: Re: [RFC/PATCH 05/13] serial: omap: drop DMA support

On Tue, Aug 21, 2012 at 2:45 PM, Felipe Balbi <[email protected]> wrote:
> The current support is known to be broken and
> a later patch will come re-adding it using
> dma engine API.
>
> Signed-off-by: Felipe Balbi <[email protected]>
> ---
Thanks Felipe !!
One less driver now towards OMAP DMA
engine conversion.

FWIW,
Acked-by: Santosh Shilimkar <[email protected]>

2012-08-21 10:24:47

by Felipe Balbi

[permalink] [raw]
Subject: Re: [RFC/PATCH 05/13] serial: omap: drop DMA support

On Tue, Aug 21, 2012 at 03:14:19PM +0530, Shilimkar, Santosh wrote:
> On Tue, Aug 21, 2012 at 2:45 PM, Felipe Balbi <[email protected]> wrote:
> > The current support is known to be broken and
> > a later patch will come re-adding it using
> > dma engine API.
> >
> > Signed-off-by: Felipe Balbi <[email protected]>
> > ---
> Thanks Felipe !!

no problem.

> One less driver now towards OMAP DMA
> engine conversion.

indeed :-) I'll take a closer look into rx timeout IRQ, but it looks
like we can use it to kick dma only for "big" transfers... need to play
with it for a while first, though.

> FWIW,
> Acked-by: Santosh Shilimkar <[email protected]>

is this Ack for this patch only or the entire series ??

--
balbi


Attachments:
(No filename) (718.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-08-21 10:35:46

by Santosh Shilimkar

[permalink] [raw]
Subject: Re: [RFC/PATCH 05/13] serial: omap: drop DMA support

On Tue, Aug 21, 2012 at 3:50 PM, Felipe Balbi <[email protected]> wrote:
> On Tue, Aug 21, 2012 at 03:14:19PM +0530, Shilimkar, Santosh wrote:
>> On Tue, Aug 21, 2012 at 2:45 PM, Felipe Balbi <[email protected]> wrote:
>> > The current support is known to be broken and
>> > a later patch will come re-adding it using
>> > dma engine API.
>> >
>> > Signed-off-by: Felipe Balbi <[email protected]>
>> > ---
>> Thanks Felipe !!
>
> no problem.
>
>> One less driver now towards OMAP DMA
>> engine conversion.
>
> indeed :-) I'll take a closer look into rx timeout IRQ, but it looks
> like we can use it to kick dma only for "big" transfers... need to play
> with it for a while first, though.
>
Yep. The RX path with DMA is bit of difficult part to manage for
UART.

>> FWIW,
>> Acked-by: Santosh Shilimkar <[email protected]>
>
> is this Ack for this patch only or the entire series ??
>
Two more patches to review and then I will do it for
full series on top of the cover-letter :-)

Regards
Santosh

2012-08-21 10:38:41

by Felipe Balbi

[permalink] [raw]
Subject: Re: [RFC/PATCH 05/13] serial: omap: drop DMA support

On Tue, Aug 21, 2012 at 04:05:22PM +0530, Shilimkar, Santosh wrote:
> On Tue, Aug 21, 2012 at 3:50 PM, Felipe Balbi <[email protected]> wrote:
> > On Tue, Aug 21, 2012 at 03:14:19PM +0530, Shilimkar, Santosh wrote:
> >> On Tue, Aug 21, 2012 at 2:45 PM, Felipe Balbi <[email protected]> wrote:
> >> > The current support is known to be broken and
> >> > a later patch will come re-adding it using
> >> > dma engine API.
> >> >
> >> > Signed-off-by: Felipe Balbi <[email protected]>
> >> > ---
> >> Thanks Felipe !!
> >
> > no problem.
> >
> >> One less driver now towards OMAP DMA
> >> engine conversion.
> >
> > indeed :-) I'll take a closer look into rx timeout IRQ, but it looks
> > like we can use it to kick dma only for "big" transfers... need to play
> > with it for a while first, though.
> >
> Yep. The RX path with DMA is bit of difficult part to manage for
> UART.
>
> >> FWIW,
> >> Acked-by: Santosh Shilimkar <[email protected]>
> >
> > is this Ack for this patch only or the entire series ??
> >
> Two more patches to review and then I will do it for
> full series on top of the cover-letter :-)

cool, thanks

--
balbi


Attachments:
(No filename) (1.10 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-08-21 10:42:35

by Santosh Shilimkar

[permalink] [raw]
Subject: Re: [RFC/PATCH 10/13] serial: omap: stick to put_autosuspend

On Tue, Aug 21, 2012 at 2:45 PM, Felipe Balbi <[email protected]> wrote:
> Everytime we're done using our TTY, we want
> the pm timer to be reinitilized. By sticking
> to pm_runtime_pm_autosuspend() we make sure
> that this will always be the case.
>
> Signed-off-by: Felipe Balbi <[email protected]>
> ---
> drivers/tty/serial/omap-serial.c | 33 ++++++++++++++++++++++-----------
> 1 file changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index 6ea24c5..458d77c 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -164,7 +164,8 @@ static void serial_omap_enable_ms(struct uart_port *port)
> pm_runtime_get_sync(up->dev);
> up->ier |= UART_IER_MSI;
> serial_out(up, UART_IER, up->ier);
> - pm_runtime_put(up->dev);
> + pm_runtime_mark_last_busy(up->dev);
> + pm_runtime_put_autosuspend(up->dev);
> }
>
Can you please expand the change-log a bit ?
Didn't follow the time re-init part completely.

Regards
Santosh

2012-08-21 10:44:25

by Santosh Shilimkar

[permalink] [raw]
Subject: Re: [RFC/PATCH 00/13] OMAP UART patches

On Tue, Aug 21, 2012 at 2:45 PM, Felipe Balbi <[email protected]> wrote:
> Hi guys,
>
> here's a series of cleanup patches to the OMAP serial
> driver. A later series could be made re-implementing
> DMA using the DMA Engine API. Note that for RX DMA
> we could be using RX Timeout IRQ as a hint that we better
> use PIO instead ;-)
>
> All patches were tested on my pandaboard, but I'd really
> like to receive Tested-by on other platforms.
>
> After this goes in, I'll probably try to get UART wakeup
> working again and only after that look at DMA.
>
> cheers
>
> Felipe Balbi (13):
> serial: omap: define and use to_uart_omap_port()
> serial: omap: always return IRQ_HANDLED
> serial: omap: define helpers for pdata function pointers
> serial: omap: don't access the platform_device
> serial: omap: drop DMA support
> serial: add OMAP-specific defines
> serial: omap: simplify IRQ handling
> serial: omap: refactor receive_chars() into rdi/rlsi handlers
> serial: omap: move THRE check to transmit_chars()
> serial: omap: stick to put_autosuspend
> serial: omap: set dev->drvdata before enabling pm_runtime
> serial: omap: drop unnecessary check from remove
> serial: omap: make sure to suspend device before remove
>
Apart from that one question on last patch, rest of the clean-up
is really good. Nice work.

FWIW,
Acked-by: Santosh Shilimkar <[email protected]>

2012-08-21 11:01:05

by Felipe Balbi

[permalink] [raw]
Subject: Re: [RFC/PATCH 10/13] serial: omap: stick to put_autosuspend

On Tue, Aug 21, 2012 at 04:12:11PM +0530, Shilimkar, Santosh wrote:
> On Tue, Aug 21, 2012 at 2:45 PM, Felipe Balbi <[email protected]> wrote:
> > Everytime we're done using our TTY, we want
> > the pm timer to be reinitilized. By sticking
> > to pm_runtime_pm_autosuspend() we make sure
> > that this will always be the case.
> >
> > Signed-off-by: Felipe Balbi <[email protected]>
> > ---
> > drivers/tty/serial/omap-serial.c | 33 ++++++++++++++++++++++-----------
> > 1 file changed, 22 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> > index 6ea24c5..458d77c 100644
> > --- a/drivers/tty/serial/omap-serial.c
> > +++ b/drivers/tty/serial/omap-serial.c
> > @@ -164,7 +164,8 @@ static void serial_omap_enable_ms(struct uart_port *port)
> > pm_runtime_get_sync(up->dev);
> > up->ier |= UART_IER_MSI;
> > serial_out(up, UART_IER, up->ier);
> > - pm_runtime_put(up->dev);
> > + pm_runtime_mark_last_busy(up->dev);
> > + pm_runtime_put_autosuspend(up->dev);
> > }
> >
> Can you please expand the change-log a bit ?
> Didn't follow the time re-init part completely.

It's really just a micro-optimization. The thing is:

if I call pm_runtime_put(), I will not reinitialize the pm timer to
whatever timeout value I used. This means that pm_runtime_put() could
actually execute right away (if timer was about to expire when I called
pm_runtime_put()). While this wouldn't cause any issues, it's better to
reinitialize the timer and make sure if there's another
read/write/set_termios/whatever coming right after this, UART is still
powered up.

I mean, it's really just trying to avoid context save & restore when
UART is still under heavy usage.

Does it make sense ?

--
balbi


Attachments:
(No filename) (1.74 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-08-21 11:05:51

by Santosh Shilimkar

[permalink] [raw]
Subject: Re: [RFC/PATCH 10/13] serial: omap: stick to put_autosuspend

On Tue, Aug 21, 2012 at 4:27 PM, Felipe Balbi <[email protected]> wrote:
> On Tue, Aug 21, 2012 at 04:12:11PM +0530, Shilimkar, Santosh wrote:
>> On Tue, Aug 21, 2012 at 2:45 PM, Felipe Balbi <[email protected]> wrote:
>> > Everytime we're done using our TTY, we want
>> > the pm timer to be reinitilized. By sticking
>> > to pm_runtime_pm_autosuspend() we make sure
>> > that this will always be the case.
>> >
>> > Signed-off-by: Felipe Balbi <[email protected]>
>> > ---
>> > drivers/tty/serial/omap-serial.c | 33 ++++++++++++++++++++++-----------
>> > 1 file changed, 22 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
>> > index 6ea24c5..458d77c 100644
>> > --- a/drivers/tty/serial/omap-serial.c
>> > +++ b/drivers/tty/serial/omap-serial.c
>> > @@ -164,7 +164,8 @@ static void serial_omap_enable_ms(struct uart_port *port)
>> > pm_runtime_get_sync(up->dev);
>> > up->ier |= UART_IER_MSI;
>> > serial_out(up, UART_IER, up->ier);
>> > - pm_runtime_put(up->dev);
>> > + pm_runtime_mark_last_busy(up->dev);
>> > + pm_runtime_put_autosuspend(up->dev);
>> > }
>> >
>> Can you please expand the change-log a bit ?
>> Didn't follow the time re-init part completely.
>
> It's really just a micro-optimization. The thing is:
>
> if I call pm_runtime_put(), I will not reinitialize the pm timer to
> whatever timeout value I used. This means that pm_runtime_put() could
> actually execute right away (if timer was about to expire when I called
> pm_runtime_put()). While this wouldn't cause any issues, it's better to
> reinitialize the timer and make sure if there's another
> read/write/set_termios/whatever coming right after this, UART is still
> powered up.
>
> I mean, it's really just trying to avoid context save & restore when
> UART is still under heavy usage.
>
> Does it make sense ?

It does. Would be good to add the above description in the change-log.
Thanks for clarification.

Regars
Santosh

2012-08-21 11:06:50

by Felipe Balbi

[permalink] [raw]
Subject: Re: [RFC/PATCH 10/13] serial: omap: stick to put_autosuspend

On Tue, Aug 21, 2012 at 04:35:26PM +0530, Shilimkar, Santosh wrote:
> On Tue, Aug 21, 2012 at 4:27 PM, Felipe Balbi <[email protected]> wrote:
> > On Tue, Aug 21, 2012 at 04:12:11PM +0530, Shilimkar, Santosh wrote:
> >> On Tue, Aug 21, 2012 at 2:45 PM, Felipe Balbi <[email protected]> wrote:
> >> > Everytime we're done using our TTY, we want
> >> > the pm timer to be reinitilized. By sticking
> >> > to pm_runtime_pm_autosuspend() we make sure
> >> > that this will always be the case.
> >> >
> >> > Signed-off-by: Felipe Balbi <[email protected]>
> >> > ---
> >> > drivers/tty/serial/omap-serial.c | 33 ++++++++++++++++++++++-----------
> >> > 1 file changed, 22 insertions(+), 11 deletions(-)
> >> >
> >> > diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> >> > index 6ea24c5..458d77c 100644
> >> > --- a/drivers/tty/serial/omap-serial.c
> >> > +++ b/drivers/tty/serial/omap-serial.c
> >> > @@ -164,7 +164,8 @@ static void serial_omap_enable_ms(struct uart_port *port)
> >> > pm_runtime_get_sync(up->dev);
> >> > up->ier |= UART_IER_MSI;
> >> > serial_out(up, UART_IER, up->ier);
> >> > - pm_runtime_put(up->dev);
> >> > + pm_runtime_mark_last_busy(up->dev);
> >> > + pm_runtime_put_autosuspend(up->dev);
> >> > }
> >> >
> >> Can you please expand the change-log a bit ?
> >> Didn't follow the time re-init part completely.
> >
> > It's really just a micro-optimization. The thing is:
> >
> > if I call pm_runtime_put(), I will not reinitialize the pm timer to
> > whatever timeout value I used. This means that pm_runtime_put() could
> > actually execute right away (if timer was about to expire when I called
> > pm_runtime_put()). While this wouldn't cause any issues, it's better to
> > reinitialize the timer and make sure if there's another
> > read/write/set_termios/whatever coming right after this, UART is still
> > powered up.
> >
> > I mean, it's really just trying to avoid context save & restore when
> > UART is still under heavy usage.
> >
> > Does it make sense ?
>
> It does. Would be good to add the above description in the change-log.
> Thanks for clarification.

will do, cheers

--
balbi


Attachments:
(No filename) (2.13 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-08-21 11:13:53

by Felipe Balbi

[permalink] [raw]
Subject: Re: [RFC/PATCH 10/13] serial: omap: stick to put_autosuspend

Hi,

On Tue, Aug 21, 2012 at 02:02:46PM +0300, Felipe Balbi wrote:
> On Tue, Aug 21, 2012 at 04:35:26PM +0530, Shilimkar, Santosh wrote:
> > On Tue, Aug 21, 2012 at 4:27 PM, Felipe Balbi <[email protected]> wrote:
> > > On Tue, Aug 21, 2012 at 04:12:11PM +0530, Shilimkar, Santosh wrote:
> > >> On Tue, Aug 21, 2012 at 2:45 PM, Felipe Balbi <[email protected]> wrote:
> > >> > Everytime we're done using our TTY, we want
> > >> > the pm timer to be reinitilized. By sticking
> > >> > to pm_runtime_pm_autosuspend() we make sure
> > >> > that this will always be the case.
> > >> >
> > >> > Signed-off-by: Felipe Balbi <[email protected]>
> > >> > ---
> > >> > drivers/tty/serial/omap-serial.c | 33 ++++++++++++++++++++++-----------
> > >> > 1 file changed, 22 insertions(+), 11 deletions(-)
> > >> >
> > >> > diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> > >> > index 6ea24c5..458d77c 100644
> > >> > --- a/drivers/tty/serial/omap-serial.c
> > >> > +++ b/drivers/tty/serial/omap-serial.c
> > >> > @@ -164,7 +164,8 @@ static void serial_omap_enable_ms(struct uart_port *port)
> > >> > pm_runtime_get_sync(up->dev);
> > >> > up->ier |= UART_IER_MSI;
> > >> > serial_out(up, UART_IER, up->ier);
> > >> > - pm_runtime_put(up->dev);
> > >> > + pm_runtime_mark_last_busy(up->dev);
> > >> > + pm_runtime_put_autosuspend(up->dev);
> > >> > }
> > >> >
> > >> Can you please expand the change-log a bit ?
> > >> Didn't follow the time re-init part completely.
> > >
> > > It's really just a micro-optimization. The thing is:
> > >
> > > if I call pm_runtime_put(), I will not reinitialize the pm timer to
> > > whatever timeout value I used. This means that pm_runtime_put() could
> > > actually execute right away (if timer was about to expire when I called
> > > pm_runtime_put()). While this wouldn't cause any issues, it's better to
> > > reinitialize the timer and make sure if there's another
> > > read/write/set_termios/whatever coming right after this, UART is still
> > > powered up.
> > >
> > > I mean, it's really just trying to avoid context save & restore when
> > > UART is still under heavy usage.
> > >
> > > Does it make sense ?
> >
> > It does. Would be good to add the above description in the change-log.
> > Thanks for clarification.
>
> will do, cheers

I have updated my branch like below. Will wait for any other comments
before sending another version.

commit 8ff7ab777d2bf8619328ddd43ddf2f8660dd011f
Author: Felipe Balbi <[email protected]>
Date: Tue Aug 21 11:45:47 2012 +0300

serial: omap: stick to put_autosuspend

Everytime we're done using our TTY, we want
the pm timer to be reinitilized. By sticking
to pm_runtime_pm_autosuspend() we make sure
that this will always be the case.

The idea behind this patch is to make sure we
will always reinitialize the pm timer so that
we don't fall into a situation where pm_runtime_put()
expires right away (if timer was already about to
expire when we made the call to pm_runtime_put()).

While suspending right away wouldn't cause any
issues, reinitializing the pm timer can help us
avoiding unnecessary context save & restore
operations (which are somewhat expensive) if there's
another read/write/set_termios request coming right
after. IOW, we are trying to make sure UART is still
powered up while it's still under heavy usage.

Acked-by: Santosh Shilimkar <[email protected]>
Signed-off-by: Felipe Balbi <[email protected]>

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 6ea24c5..458d77c 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -164,7 +164,8 @@ static void serial_omap_enable_ms(struct uart_port *port)
pm_runtime_get_sync(up->dev);
up->ier |= UART_IER_MSI;
serial_out(up, UART_IER, up->ier);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

static void serial_omap_stop_tx(struct uart_port *port)
@@ -412,7 +413,8 @@ static unsigned int serial_omap_tx_empty(struct uart_port *port)
spin_lock_irqsave(&up->port.lock, flags);
ret = serial_in(up, UART_LSR) & UART_LSR_TEMT ? TIOCSER_TEMT : 0;
spin_unlock_irqrestore(&up->port.lock, flags);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
return ret;
}

@@ -424,7 +426,8 @@ static unsigned int serial_omap_get_mctrl(struct uart_port *port)

pm_runtime_get_sync(up->dev);
status = check_modem_status(up);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);

dev_dbg(up->port.dev, "serial_omap_get_mctrl+%d\n", up->port.line);

@@ -460,7 +463,8 @@ static void serial_omap_set_mctrl(struct uart_port *port, unsigned int mctrl)
up->mcr = serial_in(up, UART_MCR);
up->mcr |= mcr;
serial_out(up, UART_MCR, up->mcr);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

static void serial_omap_break_ctl(struct uart_port *port, int break_state)
@@ -477,7 +481,8 @@ static void serial_omap_break_ctl(struct uart_port *port, int break_state)
up->lcr &= ~UART_LCR_SBC;
serial_out(up, UART_LCR, up->lcr);
spin_unlock_irqrestore(&up->port.lock, flags);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

static int serial_omap_startup(struct uart_port *port)
@@ -575,7 +580,8 @@ static void serial_omap_shutdown(struct uart_port *port)
if (serial_in(up, UART_LSR) & UART_LSR_DR)
(void) serial_in(up, UART_RX);

- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
free_irq(up->port.irq, up);
}

@@ -846,7 +852,8 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
serial_omap_configure_xonxoff(up, termios);

spin_unlock_irqrestore(&up->port.lock, flags);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
dev_dbg(up->port.dev, "serial_omap_set_termios+%d\n", up->port.line);
}

@@ -877,7 +884,8 @@ serial_omap_pm(struct uart_port *port, unsigned int state,
pm_runtime_allow(up->dev);
}

- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

static void serial_omap_release_port(struct uart_port *port)
@@ -959,7 +967,8 @@ static void serial_omap_poll_put_char(struct uart_port *port, unsigned char ch)
pm_runtime_get_sync(up->dev);
wait_for_xmitr(up);
serial_out(up, UART_TX, ch);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

static int serial_omap_poll_get_char(struct uart_port *port)
@@ -973,7 +982,8 @@ static int serial_omap_poll_get_char(struct uart_port *port)
return NO_POLL_CHAR;

status = serial_in(up, UART_RX);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
return status;
}

@@ -1305,7 +1315,8 @@ static int serial_omap_probe(struct platform_device *pdev)
if (ret != 0)
goto err_add_port;

- pm_runtime_put(&pdev->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
platform_set_drvdata(pdev, up);
return 0;


let me know if your Ack is still valid.

--
balbi


Attachments:
(No filename) (7.27 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-08-21 11:32:59

by Alan

[permalink] [raw]
Subject: Re: [RFC/PATCH 02/13] serial: omap: always return IRQ_HANDLED

On Tue, 21 Aug 2012 12:15:44 +0300
Felipe Balbi <[email protected]> wrote:

> Even if we enter our IRQ handler just to notice
> that the our device didn't generate the IRQ,
> that still means "handling" and IRQ, so let's
> return IRQ_HANDLED.

That looks wrong - you'll defeat the stuck IRQ protection. If we didn't
cause the IRQ then we are IRQ_NONE ?

2012-08-21 11:58:28

by Felipe Balbi

[permalink] [raw]
Subject: Re: [RFC/PATCH 02/13] serial: omap: always return IRQ_HANDLED

On Tue, Aug 21, 2012 at 12:50:05PM +0100, Alan Cox wrote:
> On Tue, 21 Aug 2012 12:15:44 +0300
> Felipe Balbi <[email protected]> wrote:
>
> > Even if we enter our IRQ handler just to notice
> > that the our device didn't generate the IRQ,
> > that still means "handling" and IRQ, so let's
> > return IRQ_HANDLED.
>
> That looks wrong - you'll defeat the stuck IRQ protection. If we didn't
> cause the IRQ then we are IRQ_NONE ?

that's true. I'll drop this patch from the list. My bad.

--
balbi


Attachments:
(No filename) (495.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-08-21 12:20:15

by Felipe Balbi

[permalink] [raw]
Subject: [PATCH v2 00/13] OMAP Serial patches

Hi guys,

here's a series of cleanup patches to the OMAP serial
driver. A later series could be made re-implementing
DMA using the DMA Engine API. Note that for RX DMA
we could be using RX Timeout IRQ as a hint that we better
use PIO instead ;-)

All patches were tested on my pandaboard, but I'd really
like to receive Tested-by on other platforms.

After this goes in, I'll probably try to get UART wakeup
working again and only after that look at DMA.

Changes since v1:

. improved commit log on patch 9/13 (formerly 10/13)
. removed patch 2/13
. added a new patch switching from spin_lock_irqsave() to spin_lock and
spin_unlock_irqrestore to spin_unlock

Retested with my pandaboard, UART continues to work:

# grep -i uart /proc/interrupts
106: 124 0 GIC OMAP UART2
# grep -i uart /proc/interrupts
106: 189 0 GIC OMAP UART2
# grep -i uart /proc/interrupts
106: 255 0 GIC OMAP UART2
# grep -i uart /proc/interrupts
106: 321 0 GIC OMAP UART2
# grep -i uart /proc/interrupts
106: 387 0 GIC OMAP UART2
# grep -i uart /proc/interrupts
106: 453 0 GIC OMAP UART2
# grep -i uart /proc/interrupts
106: 519 0 GIC OMAP UART2

cheers

ps: if anyone knows a better test for UART, let me know.

for convenience of anyone testing, patches are available on my git tree [1] on
branch uart

[1] git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git uart

Felipe Balbi (13):
serial: omap: define and use to_uart_omap_port()
serial: omap: define helpers for pdata function pointers
serial: omap: don't access the platform_device
serial: omap: drop DMA support
serial: add OMAP-specific defines
serial: omap: simplify IRQ handling
serial: omap: refactor receive_chars() into rdi/rlsi handlers
serial: omap: move THRE check to transmit_chars()
serial: omap: stick to put_autosuspend
serial: omap: set dev->drvdata before enabling pm_runtime
serial: omap: drop unnecessary check from remove
serial: omap: make sure to suspend device before remove
serial: omap: don't save IRQ flags on hardirq

arch/arm/mach-omap2/serial.c | 15 +-
arch/arm/plat-omap/include/plat/omap-serial.h | 12 +-
drivers/tty/serial/omap-serial.c | 713 +++++++++-----------------
include/linux/serial_reg.h | 4 +
4 files changed, 254 insertions(+), 490 deletions(-)

--
1.7.12.rc3

2012-08-21 12:20:25

by Felipe Balbi

[permalink] [raw]
Subject: [PATCH v2 02/13] serial: omap: define helpers for pdata function pointers

this patch is in preparation to a few other changes
which will align on the prototype for function
pointers passed through pdata.

It also helps cleaning up the driver a little by
agregating checks for pdata in a single location.

Acked-by: Santosh Shilimkar <[email protected]>
Signed-off-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/omap-serial.c | 66 ++++++++++++++++++++++++++++------------
1 file changed, 47 insertions(+), 19 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 5c0d0bc..6814a26 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -101,6 +101,40 @@ static inline void serial_omap_clear_fifos(struct uart_omap_port *up)
serial_out(up, UART_FCR, 0);
}

+static int serial_omap_get_context_loss_count(struct uart_omap_port *up)
+{
+ struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
+
+ if (!pdata->get_context_loss_count)
+ return 0;
+
+ return pdata->get_context_loss_count(&up->pdev->dev);
+}
+
+static void serial_omap_set_forceidle(struct uart_omap_port *up)
+{
+ struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
+
+ if (pdata->set_forceidle)
+ pdata->set_forceidle(up->pdev);
+}
+
+static void serial_omap_set_noidle(struct uart_omap_port *up)
+{
+ struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
+
+ if (pdata->set_noidle)
+ pdata->set_noidle(up->pdev);
+}
+
+static void serial_omap_enable_wakeup(struct uart_omap_port *up, bool enable)
+{
+ struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
+
+ if (pdata->enable_wakeup)
+ pdata->enable_wakeup(up->pdev, enable);
+}
+
/*
* serial_omap_get_divisor - calculate divisor value
* @port: uart port info
@@ -177,8 +211,8 @@ static void serial_omap_stop_tx(struct uart_port *port)
serial_out(up, UART_IER, up->ier);
}

- if (!up->use_dma && pdata && pdata->set_forceidle)
- pdata->set_forceidle(up->pdev);
+ if (!up->use_dma && pdata)
+ serial_omap_set_forceidle(up);

pm_runtime_mark_last_busy(&up->pdev->dev);
pm_runtime_put_autosuspend(&up->pdev->dev);
@@ -308,7 +342,6 @@ static inline void serial_omap_enable_ier_thri(struct uart_omap_port *up)
static void serial_omap_start_tx(struct uart_port *port)
{
struct uart_omap_port *up = to_uart_omap_port(port);
- struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
struct circ_buf *xmit;
unsigned int start;
int ret = 0;
@@ -316,8 +349,7 @@ static void serial_omap_start_tx(struct uart_port *port)
if (!up->use_dma) {
pm_runtime_get_sync(&up->pdev->dev);
serial_omap_enable_ier_thri(up);
- if (pdata && pdata->set_noidle)
- pdata->set_noidle(up->pdev);
+ serial_omap_set_noidle(up);
pm_runtime_mark_last_busy(&up->pdev->dev);
pm_runtime_put_autosuspend(&up->pdev->dev);
return;
@@ -1648,28 +1680,26 @@ static int serial_omap_runtime_suspend(struct device *dev)
if (!up)
return -EINVAL;

- if (!pdata || !pdata->enable_wakeup)
+ if (!pdata)
return 0;

- if (pdata->get_context_loss_count)
- up->context_loss_cnt = pdata->get_context_loss_count(dev);
+ up->context_loss_cnt = serial_omap_get_context_loss_count(up);

if (device_may_wakeup(dev)) {
if (!up->wakeups_enabled) {
- pdata->enable_wakeup(up->pdev, true);
+ serial_omap_enable_wakeup(up, true);
up->wakeups_enabled = true;
}
} else {
if (up->wakeups_enabled) {
- pdata->enable_wakeup(up->pdev, false);
+ serial_omap_enable_wakeup(up, false);
up->wakeups_enabled = false;
}
}

/* Errata i291 */
- if (up->use_dma && pdata->set_forceidle &&
- (up->errata & UART_ERRATA_i291_DMA_FORCEIDLE))
- pdata->set_forceidle(up->pdev);
+ if (up->use_dma && (up->errata & UART_ERRATA_i291_DMA_FORCEIDLE))
+ serial_omap_set_forceidle(up);

up->latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
schedule_work(&up->qos_work);
@@ -1683,17 +1713,15 @@ static int serial_omap_runtime_resume(struct device *dev)
struct omap_uart_port_info *pdata = dev->platform_data;

if (up && pdata) {
- if (pdata->get_context_loss_count) {
- u32 loss_cnt = pdata->get_context_loss_count(dev);
+ u32 loss_cnt = serial_omap_get_context_loss_count(up);

if (up->context_loss_cnt != loss_cnt)
serial_omap_restore_context(up);
- }

/* Errata i291 */
- if (up->use_dma && pdata->set_noidle &&
- (up->errata & UART_ERRATA_i291_DMA_FORCEIDLE))
- pdata->set_noidle(up->pdev);
+ if ((up->errata & UART_ERRATA_i291_DMA_FORCEIDLE) &&
+ up->use_dma)
+ serial_omap_set_noidle(up);

up->latency = up->calc_latency;
schedule_work(&up->qos_work);
--
1.7.12.rc3

2012-08-21 12:20:35

by Felipe Balbi

[permalink] [raw]
Subject: [PATCH v2 05/13] serial: add OMAP-specific defines

OMAP has some extra Interrupt types which can
be really useful for SW. Let's define them
so we can later use those in OMAP's serial driver.

Acked-by: Santosh Shilimkar <[email protected]>
Signed-off-by: Felipe Balbi <[email protected]>
---
include/linux/serial_reg.h | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/include/linux/serial_reg.h b/include/linux/serial_reg.h
index 8ce70d7..5ed325e 100644
--- a/include/linux/serial_reg.h
+++ b/include/linux/serial_reg.h
@@ -40,6 +40,10 @@

#define UART_IIR_BUSY 0x07 /* DesignWare APB Busy Detect */

+#define UART_IIR_RX_TIMEOUT 0x0c /* OMAP RX Timeout interrupt */
+#define UART_IIR_XOFF 0x10 /* OMAP XOFF/Special Character */
+#define UART_IIR_CTS_RTS_DSR 0x20 /* OMAP CTS/RTS/DSR Change */
+
#define UART_FCR 2 /* Out: FIFO Control Register */
#define UART_FCR_ENABLE_FIFO 0x01 /* Enable the FIFO */
#define UART_FCR_CLEAR_RCVR 0x02 /* Clear the RCVR FIFO */
--
1.7.12.rc3

2012-08-21 12:20:48

by Felipe Balbi

[permalink] [raw]
Subject: [PATCH v2 06/13] serial: omap: simplify IRQ handling

quite a few changes here, though they are
pretty obvious. In summary we're making sure
to detect which interrupt type we need to
handle before calling the underlying interrupt
handling procedure.

Acked-by: Santosh Shilimkar <[email protected]>
Signed-off-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/omap-serial.c | 48 +++++++++++++++++++++++++++++++---------
1 file changed, 37 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 16808b6..2d3e64e 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -350,33 +350,59 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
{
struct uart_omap_port *up = dev_id;
unsigned int iir, lsr;
+ unsigned int type;
unsigned long flags;
+ irqreturn_t ret = IRQ_HANDLED;

+ spin_lock_irqsave(&up->port.lock, flags);
pm_runtime_get_sync(up->dev);
iir = serial_in(up, UART_IIR);
+again:
if (iir & UART_IIR_NO_INT) {
- pm_runtime_mark_last_busy(up->dev);
- pm_runtime_put_autosuspend(up->dev);
- return IRQ_NONE;
+ ret = IRQ_NONE;
+ goto out;
}

- spin_lock_irqsave(&up->port.lock, flags);
lsr = serial_in(up, UART_LSR);
- if (iir & UART_IIR_RLSI) {
+
+ /* extract IRQ type from IIR register */
+ type = iir & 0x3e;
+
+ switch (type) {
+ case UART_IIR_MSI:
+ check_modem_status(up);
+ break;
+ case UART_IIR_THRI:
+ if (lsr & UART_LSR_THRE)
+ transmit_chars(up);
+ break;
+ case UART_IIR_RDI:
if (lsr & UART_LSR_DR)
receive_chars(up, &lsr);
+ break;
+ case UART_IIR_RLSI:
+ if (lsr & UART_LSR_BRK_ERROR_BITS)
+ receive_chars(up, &lsr);
+ break;
+ case UART_IIR_RX_TIMEOUT:
+ receive_chars(up, &lsr);
+ break;
+ case UART_IIR_CTS_RTS_DSR:
+ iir = serial_in(up, UART_IIR);
+ goto again;
+ case UART_IIR_XOFF:
+ /* FALLTHROUGH */
+ default:
+ break;
}

- check_modem_status(up);
- if ((lsr & UART_LSR_THRE) && (iir & UART_IIR_THRI))
- transmit_chars(up);
-
+out:
spin_unlock_irqrestore(&up->port.lock, flags);
pm_runtime_mark_last_busy(up->dev);
pm_runtime_put_autosuspend(up->dev);
-
up->port_activity = jiffies;
- return IRQ_HANDLED;
+
+ return ret;
}

static unsigned int serial_omap_tx_empty(struct uart_port *port)
--
1.7.12.rc3

2012-08-21 12:20:57

by Felipe Balbi

[permalink] [raw]
Subject: [PATCH v2 13/13] serial: omap: don't save IRQ flags on hardirq

When we're running our hardirq handler, there's
not need to disable IRQs with spin_lock_irqsave()
because IRQs are already disabled. It also makes
no difference if we save or not IRQ flags.

Switch over to simple spin_lock/spin_unlock and
drop the "flags" variable.

Signed-off-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/omap-serial.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index b79c4fa..a79658d 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -350,11 +350,10 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
struct tty_struct *tty = up->port.state->port.tty;
unsigned int iir, lsr;
unsigned int type;
- unsigned long flags;
irqreturn_t ret = IRQ_HANDLED;
int max_count = 256;

- spin_lock_irqsave(&up->port.lock, flags);
+ spin_lock(&up->port.lock);
pm_runtime_get_sync(up->dev);

do {
@@ -394,7 +393,7 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
}
} while (!(iir & UART_IIR_NO_INT) && max_count--);

- spin_unlock_irqrestore(&up->port.lock, flags);
+ spin_unlock(&up->port.lock);

tty_flip_buffer_push(tty);

--
1.7.12.rc3

2012-08-21 12:21:18

by Felipe Balbi

[permalink] [raw]
Subject: [PATCH v2 12/13] serial: omap: make sure to suspend device before remove

before removing the driver, let's make sure
to force device into a suspended state in order
to conserve power.

Acked-by: Santosh Shilimkar <[email protected]>
Signed-off-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/omap-serial.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 2e1a33a..b79c4fa 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1337,6 +1337,7 @@ static int serial_omap_remove(struct platform_device *dev)
{
struct uart_omap_port *up = platform_get_drvdata(dev);

+ pm_runtime_put_sync(up->dev);
pm_runtime_disable(up->dev);
uart_remove_one_port(&serial_omap_reg, &up->port);
pm_qos_remove_request(&up->pm_qos_request);
--
1.7.12.rc3

2012-08-21 12:21:38

by Felipe Balbi

[permalink] [raw]
Subject: [PATCH v2 10/13] serial: omap: set dev->drvdata before enabling pm_runtime

by the time we call our first pm_runtme_get_sync()
after enable pm_runtime, our resume method might
be called. To avoid problems, we must make sure
that our dev->drvdata is set correctly before
our resume method gets called.

Acked-by: Santosh Shilimkar <[email protected]>
Signed-off-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/omap-serial.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index c272ecc..1aaba77 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1301,6 +1301,7 @@ static int serial_omap_probe(struct platform_device *pdev)
serial_omap_uart_wq = create_singlethread_workqueue(up->name);
INIT_WORK(&up->qos_work, serial_omap_uart_qos_work);

+ platform_set_drvdata(pdev, up);
pm_runtime_use_autosuspend(&pdev->dev);
pm_runtime_set_autosuspend_delay(&pdev->dev,
omap_up_info->autosuspend_timeout);
@@ -1320,7 +1321,6 @@ static int serial_omap_probe(struct platform_device *pdev)

pm_runtime_mark_last_busy(up->dev);
pm_runtime_put_autosuspend(up->dev);
- platform_set_drvdata(pdev, up);
return 0;

err_add_port:
--
1.7.12.rc3

2012-08-21 12:21:41

by Felipe Balbi

[permalink] [raw]
Subject: [PATCH v2 09/13] serial: omap: stick to put_autosuspend

Everytime we're done using our TTY, we want
the pm timer to be reinitilized. By sticking
to pm_runtime_pm_autosuspend() we make sure
that this will always be the case.

The idea behind this patch is to make sure we
will always reinitialize the pm timer so that
we don't fall into a situation where pm_runtime_put()
expires right away (if timer was already about to
expire when we made the call to pm_runtime_put()).

While suspending right away wouldn't cause any
issues, reinitializing the pm timer can help us
avoiding unnecessary context save & restore
operations (which are somewhat expensive) if there's
another read/write/set_termios request coming right
after. IOW, we are trying to make sure UART is still
powered up while it's still under heavy usage.

Acked-by: Santosh Shilimkar <[email protected]>
Signed-off-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/omap-serial.c | 33 ++++++++++++++++++++++-----------
1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index bb63dc9..c272ecc 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -164,7 +164,8 @@ static void serial_omap_enable_ms(struct uart_port *port)
pm_runtime_get_sync(up->dev);
up->ier |= UART_IER_MSI;
serial_out(up, UART_IER, up->ier);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

static void serial_omap_stop_tx(struct uart_port *port)
@@ -415,7 +416,8 @@ static unsigned int serial_omap_tx_empty(struct uart_port *port)
spin_lock_irqsave(&up->port.lock, flags);
ret = serial_in(up, UART_LSR) & UART_LSR_TEMT ? TIOCSER_TEMT : 0;
spin_unlock_irqrestore(&up->port.lock, flags);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
return ret;
}

@@ -427,7 +429,8 @@ static unsigned int serial_omap_get_mctrl(struct uart_port *port)

pm_runtime_get_sync(up->dev);
status = check_modem_status(up);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);

dev_dbg(up->port.dev, "serial_omap_get_mctrl+%d\n", up->port.line);

@@ -463,7 +466,8 @@ static void serial_omap_set_mctrl(struct uart_port *port, unsigned int mctrl)
up->mcr = serial_in(up, UART_MCR);
up->mcr |= mcr;
serial_out(up, UART_MCR, up->mcr);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

static void serial_omap_break_ctl(struct uart_port *port, int break_state)
@@ -480,7 +484,8 @@ static void serial_omap_break_ctl(struct uart_port *port, int break_state)
up->lcr &= ~UART_LCR_SBC;
serial_out(up, UART_LCR, up->lcr);
spin_unlock_irqrestore(&up->port.lock, flags);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

static int serial_omap_startup(struct uart_port *port)
@@ -578,7 +583,8 @@ static void serial_omap_shutdown(struct uart_port *port)
if (serial_in(up, UART_LSR) & UART_LSR_DR)
(void) serial_in(up, UART_RX);

- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
free_irq(up->port.irq, up);
}

@@ -849,7 +855,8 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
serial_omap_configure_xonxoff(up, termios);

spin_unlock_irqrestore(&up->port.lock, flags);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
dev_dbg(up->port.dev, "serial_omap_set_termios+%d\n", up->port.line);
}

@@ -880,7 +887,8 @@ serial_omap_pm(struct uart_port *port, unsigned int state,
pm_runtime_allow(up->dev);
}

- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

static void serial_omap_release_port(struct uart_port *port)
@@ -962,7 +970,8 @@ static void serial_omap_poll_put_char(struct uart_port *port, unsigned char ch)
pm_runtime_get_sync(up->dev);
wait_for_xmitr(up);
serial_out(up, UART_TX, ch);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

static int serial_omap_poll_get_char(struct uart_port *port)
@@ -976,7 +985,8 @@ static int serial_omap_poll_get_char(struct uart_port *port)
return NO_POLL_CHAR;

status = serial_in(up, UART_RX);
- pm_runtime_put(up->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
return status;
}

@@ -1308,7 +1318,8 @@ static int serial_omap_probe(struct platform_device *pdev)
if (ret != 0)
goto err_add_port;

- pm_runtime_put(&pdev->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
platform_set_drvdata(pdev, up);
return 0;

--
1.7.12.rc3

2012-08-21 12:21:33

by Felipe Balbi

[permalink] [raw]
Subject: [PATCH v2 11/13] serial: omap: drop unnecessary check from remove

if platform_get_drvdata() returns NULL, that's
quite a nasty bug on the driver which we want to
catch ASAP. Otherwise, that check is hugely
unneeded.

Acked-by: Santosh Shilimkar <[email protected]>
Signed-off-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/omap-serial.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 1aaba77..2e1a33a 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1337,13 +1337,10 @@ static int serial_omap_remove(struct platform_device *dev)
{
struct uart_omap_port *up = platform_get_drvdata(dev);

- if (up) {
- pm_runtime_disable(up->dev);
- uart_remove_one_port(&serial_omap_reg, &up->port);
- pm_qos_remove_request(&up->pm_qos_request);
- }
+ pm_runtime_disable(up->dev);
+ uart_remove_one_port(&serial_omap_reg, &up->port);
+ pm_qos_remove_request(&up->pm_qos_request);

- platform_set_drvdata(dev, NULL);
return 0;
}

--
1.7.12.rc3

2012-08-21 12:22:24

by Felipe Balbi

[permalink] [raw]
Subject: [PATCH v2 07/13] serial: omap: refactor receive_chars() into rdi/rlsi handlers

receive_chars() was getting too big and too difficult
to follow. By splitting it into separate RDI and RSLI
handlers, we have smaller functions which are easy
to understand and only touch the pieces which they need
to touch.

Acked-by: Santosh Shilimkar <[email protected]>
Signed-off-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/omap-serial.c | 207 +++++++++++++++++++--------------------
1 file changed, 102 insertions(+), 105 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 2d3e64e..fe9852b 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -195,74 +195,6 @@ static void serial_omap_stop_rx(struct uart_port *port)
pm_runtime_put_autosuspend(up->dev);
}

-static inline void receive_chars(struct uart_omap_port *up,
- unsigned int *status)
-{
- struct tty_struct *tty = up->port.state->port.tty;
- unsigned int flag, lsr = *status;
- unsigned char ch = 0;
- int max_count = 256;
-
- do {
- if (likely(lsr & UART_LSR_DR))
- ch = serial_in(up, UART_RX);
- flag = TTY_NORMAL;
- up->port.icount.rx++;
-
- if (unlikely(lsr & UART_LSR_BRK_ERROR_BITS)) {
- /*
- * For statistics only
- */
- if (lsr & UART_LSR_BI) {
- lsr &= ~(UART_LSR_FE | UART_LSR_PE);
- up->port.icount.brk++;
- /*
- * We do the SysRQ and SAK checking
- * here because otherwise the break
- * may get masked by ignore_status_mask
- * or read_status_mask.
- */
- if (uart_handle_break(&up->port))
- goto ignore_char;
- } else if (lsr & UART_LSR_PE) {
- up->port.icount.parity++;
- } else if (lsr & UART_LSR_FE) {
- up->port.icount.frame++;
- }
-
- if (lsr & UART_LSR_OE)
- up->port.icount.overrun++;
-
- /*
- * Mask off conditions which should be ignored.
- */
- lsr &= up->port.read_status_mask;
-
-#ifdef CONFIG_SERIAL_OMAP_CONSOLE
- if (up->port.line == up->port.cons->index) {
- /* Recover the break flag from console xmit */
- lsr |= up->lsr_break_flag;
- }
-#endif
- if (lsr & UART_LSR_BI)
- flag = TTY_BREAK;
- else if (lsr & UART_LSR_PE)
- flag = TTY_PARITY;
- else if (lsr & UART_LSR_FE)
- flag = TTY_FRAME;
- }
-
- if (uart_handle_sysrq_char(&up->port, ch))
- goto ignore_char;
- uart_insert_char(&up->port, lsr, UART_LSR_OE, ch, flag);
-ignore_char:
- lsr = serial_in(up, UART_LSR);
- } while ((lsr & (UART_LSR_DR | UART_LSR_BI)) && (max_count-- > 0));
- spin_unlock(&up->port.lock);
- tty_flip_buffer_push(tty);
- spin_lock(&up->port.lock);
-}
-
static void transmit_chars(struct uart_omap_port *up)
{
struct circ_buf *xmit = &up->port.state->xmit;
@@ -341,6 +273,68 @@ static unsigned int check_modem_status(struct uart_omap_port *up)
return status;
}

+static void serial_omap_rlsi(struct uart_omap_port *up, unsigned int lsr)
+{
+ unsigned int flag;
+
+ up->port.icount.rx++;
+ flag = TTY_NORMAL;
+
+ if (lsr & UART_LSR_BI) {
+ flag = TTY_BREAK;
+ lsr &= ~(UART_LSR_FE | UART_LSR_PE);
+ up->port.icount.brk++;
+ /*
+ * We do the SysRQ and SAK checking
+ * here because otherwise the break
+ * may get masked by ignore_status_mask
+ * or read_status_mask.
+ */
+ if (uart_handle_break(&up->port))
+ return;
+
+ }
+
+ if (lsr & UART_LSR_PE) {
+ flag = TTY_PARITY;
+ up->port.icount.parity++;
+ }
+
+ if (lsr & UART_LSR_FE) {
+ flag = TTY_FRAME;
+ up->port.icount.frame++;
+ }
+
+ if (lsr & UART_LSR_OE)
+ up->port.icount.overrun++;
+
+#ifdef CONFIG_SERIAL_OMAP_CONSOLE
+ if (up->port.line == up->port.cons->index) {
+ /* Recover the break flag from console xmit */
+ lsr |= up->lsr_break_flag;
+ }
+#endif
+ uart_insert_char(&up->port, lsr, UART_LSR_OE, 0, flag);
+}
+
+static void serial_omap_rdi(struct uart_omap_port *up, unsigned int lsr)
+{
+ unsigned char ch = 0;
+ unsigned int flag;
+
+ if (!(lsr & UART_LSR_DR))
+ return;
+
+ ch = serial_in(up, UART_RX);
+ flag = TTY_NORMAL;
+ up->port.icount.rx++;
+
+ if (uart_handle_sysrq_char(&up->port, ch))
+ return;
+
+ uart_insert_char(&up->port, lsr, UART_LSR_OE, ch, flag);
+}
+
/**
* serial_omap_irq() - This handles the interrupt from one port
* @irq: uart port irq number
@@ -349,55 +343,58 @@ static unsigned int check_modem_status(struct uart_omap_port *up)
static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
{
struct uart_omap_port *up = dev_id;
+ struct tty_struct *tty = up->port.state->port.tty;
unsigned int iir, lsr;
unsigned int type;
unsigned long flags;
irqreturn_t ret = IRQ_HANDLED;
+ int max_count = 256;

spin_lock_irqsave(&up->port.lock, flags);
pm_runtime_get_sync(up->dev);
- iir = serial_in(up, UART_IIR);
-again:
- if (iir & UART_IIR_NO_INT) {
- ret = IRQ_NONE;
- goto out;
- }

- lsr = serial_in(up, UART_LSR);
+ do {
+ iir = serial_in(up, UART_IIR);
+ if (iir & UART_IIR_NO_INT) {
+ ret = IRQ_NONE;
+ break;
+ }

- /* extract IRQ type from IIR register */
- type = iir & 0x3e;
+ lsr = serial_in(up, UART_LSR);

- switch (type) {
- case UART_IIR_MSI:
- check_modem_status(up);
- break;
- case UART_IIR_THRI:
- if (lsr & UART_LSR_THRE)
- transmit_chars(up);
- break;
- case UART_IIR_RDI:
- if (lsr & UART_LSR_DR)
- receive_chars(up, &lsr);
- break;
- case UART_IIR_RLSI:
- if (lsr & UART_LSR_BRK_ERROR_BITS)
- receive_chars(up, &lsr);
- break;
- case UART_IIR_RX_TIMEOUT:
- receive_chars(up, &lsr);
- break;
- case UART_IIR_CTS_RTS_DSR:
- iir = serial_in(up, UART_IIR);
- goto again;
- case UART_IIR_XOFF:
- /* FALLTHROUGH */
- default:
- break;
- }
+ /* extract IRQ type from IIR register */
+ type = iir & 0x3e;
+
+ switch (type) {
+ case UART_IIR_MSI:
+ check_modem_status(up);
+ break;
+ case UART_IIR_THRI:
+ if (lsr & UART_LSR_THRE)
+ transmit_chars(up);
+ break;
+ case UART_IIR_RX_TIMEOUT:
+ /* FALLTHROUGH */
+ case UART_IIR_RDI:
+ serial_omap_rdi(up, lsr);
+ break;
+ case UART_IIR_RLSI:
+ serial_omap_rlsi(up, lsr);
+ break;
+ case UART_IIR_CTS_RTS_DSR:
+ /* simply try again */
+ break;
+ case UART_IIR_XOFF:
+ /* FALLTHROUGH */
+ default:
+ break;
+ }
+ } while (!(iir & UART_IIR_NO_INT) && max_count--);

-out:
spin_unlock_irqrestore(&up->port.lock, flags);
+
+ tty_flip_buffer_push(tty);
+
pm_runtime_mark_last_busy(up->dev);
pm_runtime_put_autosuspend(up->dev);
up->port_activity = jiffies;
--
1.7.12.rc3

2012-08-21 12:22:22

by Felipe Balbi

[permalink] [raw]
Subject: [PATCH v2 08/13] serial: omap: move THRE check to transmit_chars()

since all other IRQ types now do all necessary
checks inside their handlers, transmit_chars()
was the only one left expecting serial_omap_irq()
to check THRE for it. We can move THRE check to
transmit_chars() in order to make serial_omap_irq()
more uniform.

Acked-by: Santosh Shilimkar <[email protected]>
Signed-off-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/omap-serial.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index fe9852b..bb63dc9 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -195,11 +195,14 @@ static void serial_omap_stop_rx(struct uart_port *port)
pm_runtime_put_autosuspend(up->dev);
}

-static void transmit_chars(struct uart_omap_port *up)
+static void transmit_chars(struct uart_omap_port *up, unsigned int lsr)
{
struct circ_buf *xmit = &up->port.state->xmit;
int count;

+ if (!(lsr & UART_LSR_THRE))
+ return;
+
if (up->port.x_char) {
serial_out(up, UART_TX, up->port.x_char);
up->port.icount.tx++;
@@ -370,8 +373,7 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
check_modem_status(up);
break;
case UART_IIR_THRI:
- if (lsr & UART_LSR_THRE)
- transmit_chars(up);
+ transmit_chars(up, lsr);
break;
case UART_IIR_RX_TIMEOUT:
/* FALLTHROUGH */
--
1.7.12.rc3

2012-08-21 12:23:01

by Felipe Balbi

[permalink] [raw]
Subject: [PATCH v2 04/13] serial: omap: drop DMA support

The current support is known to be broken and
a later patch will come re-adding it using
dma engine API.

Acked-by: Santosh Shilimkar <[email protected]>
Signed-off-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/omap-serial.c | 330 ++-------------------------------------
1 file changed, 12 insertions(+), 318 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 6ab3582..16808b6 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -33,14 +33,12 @@
#include <linux/tty.h>
#include <linux/tty_flip.h>
#include <linux/io.h>
-#include <linux/dma-mapping.h>
#include <linux/clk.h>
#include <linux/serial_core.h>
#include <linux/irq.h>
#include <linux/pm_runtime.h>
#include <linux/of.h>

-#include <plat/dma.h>
#include <plat/dmtimer.h>
#include <plat/omap-serial.h>

@@ -74,9 +72,6 @@
static struct uart_omap_port *ui[OMAP_MAX_HSUART_PORTS];

/* Forward declaration of functions */
-static void uart_tx_dma_callback(int lch, u16 ch_status, void *data);
-static void serial_omap_rxdma_poll(unsigned long uart_no);
-static int serial_omap_start_rxdma(struct uart_omap_port *up);
static void serial_omap_mdr1_errataset(struct uart_omap_port *up, u8 mdr1);

static struct workqueue_struct *serial_omap_uart_wq;
@@ -160,19 +155,6 @@ serial_omap_get_divisor(struct uart_port *port, unsigned int baud)
return port->uartclk/(baud * divisor);
}

-static void serial_omap_stop_rxdma(struct uart_omap_port *up)
-{
- if (up->uart_dma.rx_dma_used) {
- del_timer(&up->uart_dma.rx_timer);
- omap_stop_dma(up->uart_dma.rx_dma_channel);
- omap_free_dma(up->uart_dma.rx_dma_channel);
- up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
- up->uart_dma.rx_dma_used = false;
- pm_runtime_mark_last_busy(up->dev);
- pm_runtime_put_autosuspend(up->dev);
- }
-}
-
static void serial_omap_enable_ms(struct uart_port *port)
{
struct uart_omap_port *up = to_uart_omap_port(port);
@@ -188,22 +170,6 @@ static void serial_omap_enable_ms(struct uart_port *port)
static void serial_omap_stop_tx(struct uart_port *port)
{
struct uart_omap_port *up = to_uart_omap_port(port);
- struct omap_uart_port_info *pdata = up->dev->platform_data;
-
- if (up->use_dma &&
- up->uart_dma.tx_dma_channel != OMAP_UART_DMA_CH_FREE) {
- /*
- * Check if dma is still active. If yes do nothing,
- * return. Else stop dma
- */
- if (omap_get_dma_active_status(up->uart_dma.tx_dma_channel))
- return;
- omap_stop_dma(up->uart_dma.tx_dma_channel);
- omap_free_dma(up->uart_dma.tx_dma_channel);
- up->uart_dma.tx_dma_channel = OMAP_UART_DMA_CH_FREE;
- pm_runtime_mark_last_busy(up->dev);
- pm_runtime_put_autosuspend(up->dev);
- }

pm_runtime_get_sync(up->dev);
if (up->ier & UART_IER_THRI) {
@@ -211,8 +177,7 @@ static void serial_omap_stop_tx(struct uart_port *port)
serial_out(up, UART_IER, up->ier);
}

- if (!up->use_dma && pdata)
- serial_omap_set_forceidle(up);
+ serial_omap_set_forceidle(up);

pm_runtime_mark_last_busy(up->dev);
pm_runtime_put_autosuspend(up->dev);
@@ -223,8 +188,6 @@ static void serial_omap_stop_rx(struct uart_port *port)
struct uart_omap_port *up = to_uart_omap_port(port);

pm_runtime_get_sync(up->dev);
- if (up->use_dma)
- serial_omap_stop_rxdma(up);
up->ier &= ~UART_IER_RLSI;
up->port.read_status_mask &= ~UART_LSR_DR;
serial_out(up, UART_IER, up->ier);
@@ -342,67 +305,12 @@ static inline void serial_omap_enable_ier_thri(struct uart_omap_port *up)
static void serial_omap_start_tx(struct uart_port *port)
{
struct uart_omap_port *up = to_uart_omap_port(port);
- struct circ_buf *xmit;
- unsigned int start;
- int ret = 0;
-
- if (!up->use_dma) {
- pm_runtime_get_sync(up->dev);
- serial_omap_enable_ier_thri(up);
- serial_omap_set_noidle(up);
- pm_runtime_mark_last_busy(up->dev);
- pm_runtime_put_autosuspend(up->dev);
- return;
- }
-
- if (up->uart_dma.tx_dma_used)
- return;
-
- xmit = &up->port.state->xmit;
-
- if (up->uart_dma.tx_dma_channel == OMAP_UART_DMA_CH_FREE) {
- pm_runtime_get_sync(up->dev);
- ret = omap_request_dma(up->uart_dma.uart_dma_tx,
- "UART Tx DMA",
- (void *)uart_tx_dma_callback, up,
- &(up->uart_dma.tx_dma_channel));

- if (ret < 0) {
- serial_omap_enable_ier_thri(up);
- return;
- }
- }
- spin_lock(&(up->uart_dma.tx_lock));
- up->uart_dma.tx_dma_used = true;
- spin_unlock(&(up->uart_dma.tx_lock));
-
- start = up->uart_dma.tx_buf_dma_phys +
- (xmit->tail & (UART_XMIT_SIZE - 1));
-
- up->uart_dma.tx_buf_size = uart_circ_chars_pending(xmit);
- /*
- * It is a circular buffer. See if the buffer has wounded back.
- * If yes it will have to be transferred in two separate dma
- * transfers
- */
- if (start + up->uart_dma.tx_buf_size >=
- up->uart_dma.tx_buf_dma_phys + UART_XMIT_SIZE)
- up->uart_dma.tx_buf_size =
- (up->uart_dma.tx_buf_dma_phys +
- UART_XMIT_SIZE) - start;
-
- omap_set_dma_dest_params(up->uart_dma.tx_dma_channel, 0,
- OMAP_DMA_AMODE_CONSTANT,
- up->uart_dma.uart_base, 0, 0);
- omap_set_dma_src_params(up->uart_dma.tx_dma_channel, 0,
- OMAP_DMA_AMODE_POST_INC, start, 0, 0);
- omap_set_dma_transfer_params(up->uart_dma.tx_dma_channel,
- OMAP_DMA_DATA_TYPE_S8,
- up->uart_dma.tx_buf_size, 1,
- OMAP_DMA_SYNC_ELEMENT,
- up->uart_dma.uart_dma_tx, 0);
- /* FIXME: Cache maintenance needed here? */
- omap_start_dma(up->uart_dma.tx_dma_channel);
+ pm_runtime_get_sync(up->dev);
+ serial_omap_enable_ier_thri(up);
+ serial_omap_set_noidle(up);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

static unsigned int check_modem_status(struct uart_omap_port *up)
@@ -455,16 +363,8 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
spin_lock_irqsave(&up->port.lock, flags);
lsr = serial_in(up, UART_LSR);
if (iir & UART_IIR_RLSI) {
- if (!up->use_dma) {
- if (lsr & UART_LSR_DR)
- receive_chars(up, &lsr);
- } else {
- up->ier &= ~(UART_IER_RDI | UART_IER_RLSI);
- serial_out(up, UART_IER, up->ier);
- if ((serial_omap_start_rxdma(up) != 0) &&
- (lsr & UART_LSR_DR))
- receive_chars(up, &lsr);
- }
+ if (lsr & UART_LSR_DR)
+ receive_chars(up, &lsr);
}

check_modem_status(up);
@@ -605,20 +505,6 @@ static int serial_omap_startup(struct uart_port *port)
spin_unlock_irqrestore(&up->port.lock, flags);

up->msr_saved_flags = 0;
- if (up->use_dma) {
- free_page((unsigned long)up->port.state->xmit.buf);
- up->port.state->xmit.buf = dma_alloc_coherent(NULL,
- UART_XMIT_SIZE,
- (dma_addr_t *)&(up->uart_dma.tx_buf_dma_phys),
- 0);
- init_timer(&(up->uart_dma.rx_timer));
- up->uart_dma.rx_timer.function = serial_omap_rxdma_poll;
- up->uart_dma.rx_timer.data = up->port.line;
- /* Currently the buffer size is 4KB. Can increase it */
- up->uart_dma.rx_buf = dma_alloc_coherent(NULL,
- up->uart_dma.rx_buf_size,
- (dma_addr_t *)&(up->uart_dma.rx_buf_dma_phys), 0);
- }
/*
* Finally, enable interrupts. Note: Modem status interrupts
* are set via set_termios(), which will be occurring imminently
@@ -666,17 +552,6 @@ static void serial_omap_shutdown(struct uart_port *port)
*/
if (serial_in(up, UART_LSR) & UART_LSR_DR)
(void) serial_in(up, UART_RX);
- if (up->use_dma) {
- dma_free_coherent(up->port.dev,
- UART_XMIT_SIZE, up->port.state->xmit.buf,
- up->uart_dma.tx_buf_dma_phys);
- up->port.state->xmit.buf = NULL;
- serial_omap_stop_rx(port);
- dma_free_coherent(up->port.dev,
- up->uart_dma.rx_buf_size, up->uart_dma.rx_buf,
- up->uart_dma.rx_buf_dma_phys);
- up->uart_dma.rx_buf = NULL;
- }

pm_runtime_put(up->dev);
free_irq(up->port.irq, up);
@@ -800,8 +675,6 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,

up->fcr = UART_FCR_R_TRIG_01 | UART_FCR_T_TRIG_01 |
UART_FCR_ENABLE_FIFO;
- if (up->use_dma)
- up->fcr |= UART_FCR_DMA_SELECT;

/*
* Ok, we're now changing the port state. Do it with
@@ -877,14 +750,9 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,

up->scr |= OMAP_UART_SCR_RX_TRIG_GRANU1_MASK;

- if (up->use_dma) {
- serial_out(up, UART_TI752_TLR, 0);
- up->scr |= UART_FCR_TRIGGER_4;
- } else {
- /* Set receive FIFO threshold to 1 byte */
- up->fcr &= ~OMAP_UART_FCR_RX_FIFO_TRIG_MASK;
- up->fcr |= (0x1 << OMAP_UART_FCR_RX_FIFO_TRIG_SHIFT);
- }
+ /* Set receive FIFO threshold to 1 byte */
+ up->fcr &= ~OMAP_UART_FCR_RX_FIFO_TRIG_MASK;
+ up->fcr |= (0x1 << OMAP_UART_FCR_RX_FIFO_TRIG_SHIFT);

serial_out(up, UART_FCR, up->fcr);
serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
@@ -1253,149 +1121,6 @@ static int serial_omap_resume(struct device *dev)
}
#endif

-static void serial_omap_rxdma_poll(unsigned long uart_no)
-{
- struct uart_omap_port *up = ui[uart_no];
- unsigned int curr_dma_pos, curr_transmitted_size;
- int ret = 0;
-
- curr_dma_pos = omap_get_dma_dst_pos(up->uart_dma.rx_dma_channel);
- if ((curr_dma_pos == up->uart_dma.prev_rx_dma_pos) ||
- (curr_dma_pos == 0)) {
- if (jiffies_to_msecs(jiffies - up->port_activity) <
- up->uart_dma.rx_timeout) {
- mod_timer(&up->uart_dma.rx_timer, jiffies +
- usecs_to_jiffies(up->uart_dma.rx_poll_rate));
- } else {
- serial_omap_stop_rxdma(up);
- up->ier |= (UART_IER_RDI | UART_IER_RLSI);
- serial_out(up, UART_IER, up->ier);
- }
- return;
- }
-
- curr_transmitted_size = curr_dma_pos -
- up->uart_dma.prev_rx_dma_pos;
- up->port.icount.rx += curr_transmitted_size;
- tty_insert_flip_string(up->port.state->port.tty,
- up->uart_dma.rx_buf +
- (up->uart_dma.prev_rx_dma_pos -
- up->uart_dma.rx_buf_dma_phys),
- curr_transmitted_size);
- tty_flip_buffer_push(up->port.state->port.tty);
- up->uart_dma.prev_rx_dma_pos = curr_dma_pos;
- if (up->uart_dma.rx_buf_size +
- up->uart_dma.rx_buf_dma_phys == curr_dma_pos) {
- ret = serial_omap_start_rxdma(up);
- if (ret < 0) {
- serial_omap_stop_rxdma(up);
- up->ier |= (UART_IER_RDI | UART_IER_RLSI);
- serial_out(up, UART_IER, up->ier);
- }
- } else {
- mod_timer(&up->uart_dma.rx_timer, jiffies +
- usecs_to_jiffies(up->uart_dma.rx_poll_rate));
- }
- up->port_activity = jiffies;
-}
-
-static void uart_rx_dma_callback(int lch, u16 ch_status, void *data)
-{
- return;
-}
-
-static int serial_omap_start_rxdma(struct uart_omap_port *up)
-{
- int ret = 0;
-
- if (up->uart_dma.rx_dma_channel == -1) {
- pm_runtime_get_sync(up->dev);
- ret = omap_request_dma(up->uart_dma.uart_dma_rx,
- "UART Rx DMA",
- (void *)uart_rx_dma_callback, up,
- &(up->uart_dma.rx_dma_channel));
- if (ret < 0)
- return ret;
-
- omap_set_dma_src_params(up->uart_dma.rx_dma_channel, 0,
- OMAP_DMA_AMODE_CONSTANT,
- up->uart_dma.uart_base, 0, 0);
- omap_set_dma_dest_params(up->uart_dma.rx_dma_channel, 0,
- OMAP_DMA_AMODE_POST_INC,
- up->uart_dma.rx_buf_dma_phys, 0, 0);
- omap_set_dma_transfer_params(up->uart_dma.rx_dma_channel,
- OMAP_DMA_DATA_TYPE_S8,
- up->uart_dma.rx_buf_size, 1,
- OMAP_DMA_SYNC_ELEMENT,
- up->uart_dma.uart_dma_rx, 0);
- }
- up->uart_dma.prev_rx_dma_pos = up->uart_dma.rx_buf_dma_phys;
- /* FIXME: Cache maintenance needed here? */
- omap_start_dma(up->uart_dma.rx_dma_channel);
- mod_timer(&up->uart_dma.rx_timer, jiffies +
- usecs_to_jiffies(up->uart_dma.rx_poll_rate));
- up->uart_dma.rx_dma_used = true;
- return ret;
-}
-
-static void serial_omap_continue_tx(struct uart_omap_port *up)
-{
- struct circ_buf *xmit = &up->port.state->xmit;
- unsigned int start = up->uart_dma.tx_buf_dma_phys
- + (xmit->tail & (UART_XMIT_SIZE - 1));
-
- if (uart_circ_empty(xmit))
- return;
-
- up->uart_dma.tx_buf_size = uart_circ_chars_pending(xmit);
- /*
- * It is a circular buffer. See if the buffer has wounded back.
- * If yes it will have to be transferred in two separate dma
- * transfers
- */
- if (start + up->uart_dma.tx_buf_size >=
- up->uart_dma.tx_buf_dma_phys + UART_XMIT_SIZE)
- up->uart_dma.tx_buf_size =
- (up->uart_dma.tx_buf_dma_phys + UART_XMIT_SIZE) - start;
- omap_set_dma_dest_params(up->uart_dma.tx_dma_channel, 0,
- OMAP_DMA_AMODE_CONSTANT,
- up->uart_dma.uart_base, 0, 0);
- omap_set_dma_src_params(up->uart_dma.tx_dma_channel, 0,
- OMAP_DMA_AMODE_POST_INC, start, 0, 0);
- omap_set_dma_transfer_params(up->uart_dma.tx_dma_channel,
- OMAP_DMA_DATA_TYPE_S8,
- up->uart_dma.tx_buf_size, 1,
- OMAP_DMA_SYNC_ELEMENT,
- up->uart_dma.uart_dma_tx, 0);
- /* FIXME: Cache maintenance needed here? */
- omap_start_dma(up->uart_dma.tx_dma_channel);
-}
-
-static void uart_tx_dma_callback(int lch, u16 ch_status, void *data)
-{
- struct uart_omap_port *up = data;
- struct circ_buf *xmit = &up->port.state->xmit;
-
- xmit->tail = (xmit->tail + up->uart_dma.tx_buf_size) & \
- (UART_XMIT_SIZE - 1);
- up->port.icount.tx += up->uart_dma.tx_buf_size;
-
- if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
- uart_write_wakeup(&up->port);
-
- if (uart_circ_empty(xmit)) {
- spin_lock(&(up->uart_dma.tx_lock));
- serial_omap_stop_tx(&up->port);
- up->uart_dma.tx_dma_used = false;
- spin_unlock(&(up->uart_dma.tx_lock));
- } else {
- omap_stop_dma(up->uart_dma.tx_dma_channel);
- serial_omap_continue_tx(up);
- }
- up->port_activity = jiffies;
- return;
-}
-
static void omap_serial_fill_features_erratas(struct uart_omap_port *up)
{
u32 mvr, scheme;
@@ -1465,7 +1190,7 @@ static struct omap_uart_port_info *of_get_uart_port_info(struct device *dev)
static int serial_omap_probe(struct platform_device *pdev)
{
struct uart_omap_port *up;
- struct resource *mem, *irq, *dma_tx, *dma_rx;
+ struct resource *mem, *irq;
struct omap_uart_port_info *omap_up_info = pdev->dev.platform_data;
int ret = -ENOSPC;

@@ -1490,14 +1215,6 @@ static int serial_omap_probe(struct platform_device *pdev)
return -EBUSY;
}

- dma_rx = platform_get_resource_byname(pdev, IORESOURCE_DMA, "rx");
- if (!dma_rx)
- return -ENXIO;
-
- dma_tx = platform_get_resource_byname(pdev, IORESOURCE_DMA, "tx");
- if (!dma_tx)
- return -ENXIO;
-
up = devm_kzalloc(&pdev->dev, sizeof(*up), GFP_KERNEL);
if (!up)
return -ENOMEM;
@@ -1541,20 +1258,6 @@ static int serial_omap_probe(struct platform_device *pdev)
dev_warn(&pdev->dev, "No clock speed specified: using default:"
"%d\n", DEFAULT_CLK_SPEED);
}
- up->uart_dma.uart_base = mem->start;
-
- if (omap_up_info->dma_enabled) {
- up->uart_dma.uart_dma_tx = dma_tx->start;
- up->uart_dma.uart_dma_rx = dma_rx->start;
- up->use_dma = 1;
- up->uart_dma.rx_buf_size = omap_up_info->dma_rx_buf_size;
- up->uart_dma.rx_timeout = omap_up_info->dma_rx_timeout;
- up->uart_dma.rx_poll_rate = omap_up_info->dma_rx_poll_rate;
- spin_lock_init(&(up->uart_dma.tx_lock));
- spin_lock_init(&(up->uart_dma.rx_lock));
- up->uart_dma.tx_dma_channel = OMAP_UART_DMA_CH_FREE;
- up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
- }

up->latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
up->calc_latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
@@ -1697,10 +1400,6 @@ static int serial_omap_runtime_suspend(struct device *dev)
}
}

- /* Errata i291 */
- if (up->use_dma && (up->errata & UART_ERRATA_i291_DMA_FORCEIDLE))
- serial_omap_set_forceidle(up);
-
up->latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
schedule_work(&up->qos_work);

@@ -1718,11 +1417,6 @@ static int serial_omap_runtime_resume(struct device *dev)
if (up->context_loss_cnt != loss_cnt)
serial_omap_restore_context(up);

- /* Errata i291 */
- if ((up->errata & UART_ERRATA_i291_DMA_FORCEIDLE) &&
- up->use_dma)
- serial_omap_set_noidle(up);
-
up->latency = up->calc_latency;
schedule_work(&up->qos_work);
}
--
1.7.12.rc3

2012-08-21 12:23:56

by Felipe Balbi

[permalink] [raw]
Subject: [PATCH v2 03/13] serial: omap: don't access the platform_device

The driver doesn't need to know about its platform_device.

Everything the driver needs can be done through the
struct device pointer. In case we need to use the
OMAP-specific PM function pointers, those can make
sure to find the device's platform_device pointer
so they can find the struct omap_device through
pdev->archdata field.

Acked-by: Santosh Shilimkar <[email protected]>
Signed-off-by: Felipe Balbi <[email protected]>
---
arch/arm/mach-omap2/serial.c | 15 ++--
arch/arm/plat-omap/include/plat/omap-serial.h | 10 +--
drivers/tty/serial/omap-serial.c | 124 +++++++++++++-------------
3 files changed, 76 insertions(+), 73 deletions(-)

diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index c1b93c7..8f07841 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -81,8 +81,9 @@ static struct omap_uart_port_info omap_serial_default_info[] __initdata = {
};

#ifdef CONFIG_PM
-static void omap_uart_enable_wakeup(struct platform_device *pdev, bool enable)
+static void omap_uart_enable_wakeup(struct device *dev, bool enable)
{
+ struct platform_device *pdev = to_platform_device(dev);
struct omap_device *od = to_omap_device(pdev);

if (!od)
@@ -99,15 +100,17 @@ static void omap_uart_enable_wakeup(struct platform_device *pdev, bool enable)
* in Smartidle Mode When Configured for DMA Operations.
* WA: configure uart in force idle mode.
*/
-static void omap_uart_set_noidle(struct platform_device *pdev)
+static void omap_uart_set_noidle(struct device *dev)
{
+ struct platform_device *pdev = to_platform_device(dev);
struct omap_device *od = to_omap_device(pdev);

omap_hwmod_set_slave_idlemode(od->hwmods[0], HWMOD_IDLEMODE_NO);
}

-static void omap_uart_set_smartidle(struct platform_device *pdev)
+static void omap_uart_set_smartidle(struct device *dev)
{
+ struct platform_device *pdev = to_platform_device(dev);
struct omap_device *od = to_omap_device(pdev);
u8 idlemode;

@@ -120,10 +123,10 @@ static void omap_uart_set_smartidle(struct platform_device *pdev)
}

#else
-static void omap_uart_enable_wakeup(struct platform_device *pdev, bool enable)
+static void omap_uart_enable_wakeup(struct device *dev, bool enable)
{}
-static void omap_uart_set_noidle(struct platform_device *pdev) {}
-static void omap_uart_set_smartidle(struct platform_device *pdev) {}
+static void omap_uart_set_noidle(struct device *dev) {}
+static void omap_uart_set_smartidle(struct device *dev) {}
#endif /* CONFIG_PM */

#ifdef CONFIG_OMAP_MUX
diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
index f3b35d9..743ac80 100644
--- a/arch/arm/plat-omap/include/plat/omap-serial.h
+++ b/arch/arm/plat-omap/include/plat/omap-serial.h
@@ -18,7 +18,7 @@
#define __OMAP_SERIAL_H__

#include <linux/serial_core.h>
-#include <linux/platform_device.h>
+#include <linux/device.h>
#include <linux/pm_qos.h>

#include <plat/mux.h>
@@ -71,9 +71,9 @@ struct omap_uart_port_info {
unsigned int dma_rx_poll_rate;

int (*get_context_loss_count)(struct device *);
- void (*set_forceidle)(struct platform_device *);
- void (*set_noidle)(struct platform_device *);
- void (*enable_wakeup)(struct platform_device *, bool);
+ void (*set_forceidle)(struct device *);
+ void (*set_noidle)(struct device *);
+ void (*enable_wakeup)(struct device *, bool);
};

struct uart_omap_dma {
@@ -105,7 +105,7 @@ struct uart_omap_dma {
struct uart_omap_port {
struct uart_port port;
struct uart_omap_dma uart_dma;
- struct platform_device *pdev;
+ struct device *dev;

unsigned char ier;
unsigned char lcr;
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 6814a26..6ab3582 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -103,36 +103,36 @@ static inline void serial_omap_clear_fifos(struct uart_omap_port *up)

static int serial_omap_get_context_loss_count(struct uart_omap_port *up)
{
- struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
+ struct omap_uart_port_info *pdata = up->dev->platform_data;

if (!pdata->get_context_loss_count)
return 0;

- return pdata->get_context_loss_count(&up->pdev->dev);
+ return pdata->get_context_loss_count(up->dev);
}

static void serial_omap_set_forceidle(struct uart_omap_port *up)
{
- struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
+ struct omap_uart_port_info *pdata = up->dev->platform_data;

if (pdata->set_forceidle)
- pdata->set_forceidle(up->pdev);
+ pdata->set_forceidle(up->dev);
}

static void serial_omap_set_noidle(struct uart_omap_port *up)
{
- struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
+ struct omap_uart_port_info *pdata = up->dev->platform_data;

if (pdata->set_noidle)
- pdata->set_noidle(up->pdev);
+ pdata->set_noidle(up->dev);
}

static void serial_omap_enable_wakeup(struct uart_omap_port *up, bool enable)
{
- struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
+ struct omap_uart_port_info *pdata = up->dev->platform_data;

if (pdata->enable_wakeup)
- pdata->enable_wakeup(up->pdev, enable);
+ pdata->enable_wakeup(up->dev, enable);
}

/*
@@ -168,8 +168,8 @@ static void serial_omap_stop_rxdma(struct uart_omap_port *up)
omap_free_dma(up->uart_dma.rx_dma_channel);
up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
up->uart_dma.rx_dma_used = false;
- pm_runtime_mark_last_busy(&up->pdev->dev);
- pm_runtime_put_autosuspend(&up->pdev->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}
}

@@ -179,16 +179,16 @@ static void serial_omap_enable_ms(struct uart_port *port)

dev_dbg(up->port.dev, "serial_omap_enable_ms+%d\n", up->port.line);

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
up->ier |= UART_IER_MSI;
serial_out(up, UART_IER, up->ier);
- pm_runtime_put(&up->pdev->dev);
+ pm_runtime_put(up->dev);
}

static void serial_omap_stop_tx(struct uart_port *port)
{
struct uart_omap_port *up = to_uart_omap_port(port);
- struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
+ struct omap_uart_port_info *pdata = up->dev->platform_data;

if (up->use_dma &&
up->uart_dma.tx_dma_channel != OMAP_UART_DMA_CH_FREE) {
@@ -201,11 +201,11 @@ static void serial_omap_stop_tx(struct uart_port *port)
omap_stop_dma(up->uart_dma.tx_dma_channel);
omap_free_dma(up->uart_dma.tx_dma_channel);
up->uart_dma.tx_dma_channel = OMAP_UART_DMA_CH_FREE;
- pm_runtime_mark_last_busy(&up->pdev->dev);
- pm_runtime_put_autosuspend(&up->pdev->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
if (up->ier & UART_IER_THRI) {
up->ier &= ~UART_IER_THRI;
serial_out(up, UART_IER, up->ier);
@@ -214,22 +214,22 @@ static void serial_omap_stop_tx(struct uart_port *port)
if (!up->use_dma && pdata)
serial_omap_set_forceidle(up);

- pm_runtime_mark_last_busy(&up->pdev->dev);
- pm_runtime_put_autosuspend(&up->pdev->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

static void serial_omap_stop_rx(struct uart_port *port)
{
struct uart_omap_port *up = to_uart_omap_port(port);

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
if (up->use_dma)
serial_omap_stop_rxdma(up);
up->ier &= ~UART_IER_RLSI;
up->port.read_status_mask &= ~UART_LSR_DR;
serial_out(up, UART_IER, up->ier);
- pm_runtime_mark_last_busy(&up->pdev->dev);
- pm_runtime_put_autosuspend(&up->pdev->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
}

static inline void receive_chars(struct uart_omap_port *up,
@@ -347,11 +347,11 @@ static void serial_omap_start_tx(struct uart_port *port)
int ret = 0;

if (!up->use_dma) {
- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
serial_omap_enable_ier_thri(up);
serial_omap_set_noidle(up);
- pm_runtime_mark_last_busy(&up->pdev->dev);
- pm_runtime_put_autosuspend(&up->pdev->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
return;
}

@@ -361,7 +361,7 @@ static void serial_omap_start_tx(struct uart_port *port)
xmit = &up->port.state->xmit;

if (up->uart_dma.tx_dma_channel == OMAP_UART_DMA_CH_FREE) {
- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
ret = omap_request_dma(up->uart_dma.uart_dma_tx,
"UART Tx DMA",
(void *)uart_tx_dma_callback, up,
@@ -444,11 +444,11 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
unsigned int iir, lsr;
unsigned long flags;

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
iir = serial_in(up, UART_IIR);
if (iir & UART_IIR_NO_INT) {
- pm_runtime_mark_last_busy(&up->pdev->dev);
- pm_runtime_put_autosuspend(&up->pdev->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
return IRQ_NONE;
}

@@ -472,8 +472,8 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
transmit_chars(up);

spin_unlock_irqrestore(&up->port.lock, flags);
- pm_runtime_mark_last_busy(&up->pdev->dev);
- pm_runtime_put_autosuspend(&up->pdev->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);

up->port_activity = jiffies;
return IRQ_HANDLED;
@@ -485,12 +485,12 @@ static unsigned int serial_omap_tx_empty(struct uart_port *port)
unsigned long flags = 0;
unsigned int ret = 0;

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
dev_dbg(up->port.dev, "serial_omap_tx_empty+%d\n", up->port.line);
spin_lock_irqsave(&up->port.lock, flags);
ret = serial_in(up, UART_LSR) & UART_LSR_TEMT ? TIOCSER_TEMT : 0;
spin_unlock_irqrestore(&up->port.lock, flags);
- pm_runtime_put(&up->pdev->dev);
+ pm_runtime_put(up->dev);
return ret;
}

@@ -500,9 +500,9 @@ static unsigned int serial_omap_get_mctrl(struct uart_port *port)
unsigned int status;
unsigned int ret = 0;

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
status = check_modem_status(up);
- pm_runtime_put(&up->pdev->dev);
+ pm_runtime_put(up->dev);

dev_dbg(up->port.dev, "serial_omap_get_mctrl+%d\n", up->port.line);

@@ -534,11 +534,11 @@ static void serial_omap_set_mctrl(struct uart_port *port, unsigned int mctrl)
if (mctrl & TIOCM_LOOP)
mcr |= UART_MCR_LOOP;

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
up->mcr = serial_in(up, UART_MCR);
up->mcr |= mcr;
serial_out(up, UART_MCR, up->mcr);
- pm_runtime_put(&up->pdev->dev);
+ pm_runtime_put(up->dev);
}

static void serial_omap_break_ctl(struct uart_port *port, int break_state)
@@ -547,7 +547,7 @@ static void serial_omap_break_ctl(struct uart_port *port, int break_state)
unsigned long flags = 0;

dev_dbg(up->port.dev, "serial_omap_break_ctl+%d\n", up->port.line);
- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
spin_lock_irqsave(&up->port.lock, flags);
if (break_state == -1)
up->lcr |= UART_LCR_SBC;
@@ -555,7 +555,7 @@ static void serial_omap_break_ctl(struct uart_port *port, int break_state)
up->lcr &= ~UART_LCR_SBC;
serial_out(up, UART_LCR, up->lcr);
spin_unlock_irqrestore(&up->port.lock, flags);
- pm_runtime_put(&up->pdev->dev);
+ pm_runtime_put(up->dev);
}

static int serial_omap_startup(struct uart_port *port)
@@ -574,7 +574,7 @@ static int serial_omap_startup(struct uart_port *port)

dev_dbg(up->port.dev, "serial_omap_startup+%d\n", up->port.line);

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
/*
* Clear the FIFO buffers and disable them.
* (they will be reenabled in set_termios())
@@ -630,8 +630,8 @@ static int serial_omap_startup(struct uart_port *port)
/* Enable module level wake up */
serial_out(up, UART_OMAP_WER, OMAP_UART_WER_MOD_WKUP);

- pm_runtime_mark_last_busy(&up->pdev->dev);
- pm_runtime_put_autosuspend(&up->pdev->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
up->port_activity = jiffies;
return 0;
}
@@ -643,7 +643,7 @@ static void serial_omap_shutdown(struct uart_port *port)

dev_dbg(up->port.dev, "serial_omap_shutdown+%d\n", up->port.line);

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
/*
* Disable interrupts from this port
*/
@@ -678,7 +678,7 @@ static void serial_omap_shutdown(struct uart_port *port)
up->uart_dma.rx_buf = NULL;
}

- pm_runtime_put(&up->pdev->dev);
+ pm_runtime_put(up->dev);
free_irq(up->port.irq, up);
}

@@ -807,7 +807,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
* Ok, we're now changing the port state. Do it with
* interrupts disabled.
*/
- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
spin_lock_irqsave(&up->port.lock, flags);

/*
@@ -956,7 +956,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
serial_omap_configure_xonxoff(up, termios);

spin_unlock_irqrestore(&up->port.lock, flags);
- pm_runtime_put(&up->pdev->dev);
+ pm_runtime_put(up->dev);
dev_dbg(up->port.dev, "serial_omap_set_termios+%d\n", up->port.line);
}

@@ -969,7 +969,7 @@ serial_omap_pm(struct uart_port *port, unsigned int state,

dev_dbg(up->port.dev, "serial_omap_pm+%d\n", up->port.line);

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
efr = serial_in(up, UART_EFR);
serial_out(up, UART_EFR, efr | UART_EFR_ECB);
@@ -980,14 +980,14 @@ serial_omap_pm(struct uart_port *port, unsigned int state,
serial_out(up, UART_EFR, efr);
serial_out(up, UART_LCR, 0);

- if (!device_may_wakeup(&up->pdev->dev)) {
+ if (!device_may_wakeup(up->dev)) {
if (!state)
- pm_runtime_forbid(&up->pdev->dev);
+ pm_runtime_forbid(up->dev);
else
- pm_runtime_allow(&up->pdev->dev);
+ pm_runtime_allow(up->dev);
}

- pm_runtime_put(&up->pdev->dev);
+ pm_runtime_put(up->dev);
}

static void serial_omap_release_port(struct uart_port *port)
@@ -1066,10 +1066,10 @@ static void serial_omap_poll_put_char(struct uart_port *port, unsigned char ch)
{
struct uart_omap_port *up = to_uart_omap_port(port);

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
wait_for_xmitr(up);
serial_out(up, UART_TX, ch);
- pm_runtime_put(&up->pdev->dev);
+ pm_runtime_put(up->dev);
}

static int serial_omap_poll_get_char(struct uart_port *port)
@@ -1077,13 +1077,13 @@ static int serial_omap_poll_get_char(struct uart_port *port)
struct uart_omap_port *up = to_uart_omap_port(port);
unsigned int status;

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
status = serial_in(up, UART_LSR);
if (!(status & UART_LSR_DR))
return NO_POLL_CHAR;

status = serial_in(up, UART_RX);
- pm_runtime_put(&up->pdev->dev);
+ pm_runtime_put(up->dev);
return status;
}

@@ -1112,7 +1112,7 @@ serial_omap_console_write(struct console *co, const char *s,
unsigned int ier;
int locked = 1;

- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);

local_irq_save(flags);
if (up->port.sysrq)
@@ -1146,8 +1146,8 @@ serial_omap_console_write(struct console *co, const char *s,
if (up->msr_saved_flags)
check_modem_status(up);

- pm_runtime_mark_last_busy(&up->pdev->dev);
- pm_runtime_put_autosuspend(&up->pdev->dev);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
if (locked)
spin_unlock(&up->port.lock);
local_irq_restore(flags);
@@ -1309,7 +1309,7 @@ static int serial_omap_start_rxdma(struct uart_omap_port *up)
int ret = 0;

if (up->uart_dma.rx_dma_channel == -1) {
- pm_runtime_get_sync(&up->pdev->dev);
+ pm_runtime_get_sync(up->dev);
ret = omap_request_dma(up->uart_dma.uart_dma_rx,
"UART Rx DMA",
(void *)uart_rx_dma_callback, up,
@@ -1421,7 +1421,7 @@ static void omap_serial_fill_features_erratas(struct uart_omap_port *up)
minor = (mvr & OMAP_UART_MVR_MIN_MASK);
break;
default:
- dev_warn(&up->pdev->dev,
+ dev_warn(up->dev,
"Unknown %s revision, defaulting to highest\n",
up->name);
/* highest possible revision */
@@ -1502,7 +1502,7 @@ static int serial_omap_probe(struct platform_device *pdev)
if (!up)
return -ENOMEM;

- up->pdev = pdev;
+ up->dev = &pdev->dev;
up->port.dev = &pdev->dev;
up->port.type = PORT_OMAP;
up->port.iotype = UPIO_MEM;
@@ -1599,7 +1599,7 @@ static int serial_omap_remove(struct platform_device *dev)
struct uart_omap_port *up = platform_get_drvdata(dev);

if (up) {
- pm_runtime_disable(&up->pdev->dev);
+ pm_runtime_disable(up->dev);
uart_remove_one_port(&serial_omap_reg, &up->port);
pm_qos_remove_request(&up->pm_qos_request);
}
@@ -1634,7 +1634,7 @@ static void serial_omap_mdr1_errataset(struct uart_omap_port *up, u8 mdr1)
timeout--;
if (!timeout) {
/* Should *never* happen. we warn and carry on */
- dev_crit(&up->pdev->dev, "Errata i202: timedout %x\n",
+ dev_crit(up->dev, "Errata i202: timedout %x\n",
serial_in(up, UART_LSR));
break;
}
--
1.7.12.rc3

2012-08-21 12:23:59

by Felipe Balbi

[permalink] [raw]
Subject: [PATCH v2 01/13] serial: omap: define and use to_uart_omap_port()

current code only works because struct uart_port
is the first member on the uart_omap_port structure.

If, for whatever reason, someone puts another
member as the first of the structure, that cast
won't work anymore. In order to be safe, let's use
a container_of() which, for now, gets optimized into
a cast anyway.

Acked-by: Santosh Shilimkar <[email protected]>
Signed-off-by: Felipe Balbi <[email protected]>
---
arch/arm/plat-omap/include/plat/omap-serial.h | 2 ++
drivers/tty/serial/omap-serial.c | 36 +++++++++++++--------------
2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
index 1a52725..f3b35d9 100644
--- a/arch/arm/plat-omap/include/plat/omap-serial.h
+++ b/arch/arm/plat-omap/include/plat/omap-serial.h
@@ -137,4 +137,6 @@ struct uart_omap_port {
struct work_struct qos_work;
};

+#define to_uart_omap_port(p) ((container_of((p), struct uart_omap_port, port)))
+
#endif /* __OMAP_SERIAL_H__ */
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index d3cda0c..5c0d0bc 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -141,7 +141,7 @@ static void serial_omap_stop_rxdma(struct uart_omap_port *up)

static void serial_omap_enable_ms(struct uart_port *port)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);

dev_dbg(up->port.dev, "serial_omap_enable_ms+%d\n", up->port.line);

@@ -153,7 +153,7 @@ static void serial_omap_enable_ms(struct uart_port *port)

static void serial_omap_stop_tx(struct uart_port *port)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;

if (up->use_dma &&
@@ -186,7 +186,7 @@ static void serial_omap_stop_tx(struct uart_port *port)

static void serial_omap_stop_rx(struct uart_port *port)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);

pm_runtime_get_sync(&up->pdev->dev);
if (up->use_dma)
@@ -307,7 +307,7 @@ static inline void serial_omap_enable_ier_thri(struct uart_omap_port *up)

static void serial_omap_start_tx(struct uart_port *port)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
struct circ_buf *xmit;
unsigned int start;
@@ -449,7 +449,7 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)

static unsigned int serial_omap_tx_empty(struct uart_port *port)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
unsigned long flags = 0;
unsigned int ret = 0;

@@ -464,7 +464,7 @@ static unsigned int serial_omap_tx_empty(struct uart_port *port)

static unsigned int serial_omap_get_mctrl(struct uart_port *port)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
unsigned int status;
unsigned int ret = 0;

@@ -487,7 +487,7 @@ static unsigned int serial_omap_get_mctrl(struct uart_port *port)

static void serial_omap_set_mctrl(struct uart_port *port, unsigned int mctrl)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
unsigned char mcr = 0;

dev_dbg(up->port.dev, "serial_omap_set_mctrl+%d\n", up->port.line);
@@ -511,7 +511,7 @@ static void serial_omap_set_mctrl(struct uart_port *port, unsigned int mctrl)

static void serial_omap_break_ctl(struct uart_port *port, int break_state)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
unsigned long flags = 0;

dev_dbg(up->port.dev, "serial_omap_break_ctl+%d\n", up->port.line);
@@ -528,7 +528,7 @@ static void serial_omap_break_ctl(struct uart_port *port, int break_state)

static int serial_omap_startup(struct uart_port *port)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
unsigned long flags = 0;
int retval;

@@ -606,7 +606,7 @@ static int serial_omap_startup(struct uart_port *port)

static void serial_omap_shutdown(struct uart_port *port)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
unsigned long flags = 0;

dev_dbg(up->port.dev, "serial_omap_shutdown+%d\n", up->port.line);
@@ -721,7 +721,7 @@ static void
serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
struct ktermios *old)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
unsigned char cval = 0;
unsigned char efr = 0;
unsigned long flags = 0;
@@ -932,7 +932,7 @@ static void
serial_omap_pm(struct uart_port *port, unsigned int state,
unsigned int oldstate)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
unsigned char efr;

dev_dbg(up->port.dev, "serial_omap_pm+%d\n", up->port.line);
@@ -971,7 +971,7 @@ static int serial_omap_request_port(struct uart_port *port)

static void serial_omap_config_port(struct uart_port *port, int flags)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);

dev_dbg(up->port.dev, "serial_omap_config_port+%d\n",
up->port.line);
@@ -989,7 +989,7 @@ serial_omap_verify_port(struct uart_port *port, struct serial_struct *ser)
static const char *
serial_omap_type(struct uart_port *port)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);

dev_dbg(up->port.dev, "serial_omap_type+%d\n", up->port.line);
return up->name;
@@ -1032,7 +1032,7 @@ static inline void wait_for_xmitr(struct uart_omap_port *up)

static void serial_omap_poll_put_char(struct uart_port *port, unsigned char ch)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);

pm_runtime_get_sync(&up->pdev->dev);
wait_for_xmitr(up);
@@ -1042,7 +1042,7 @@ static void serial_omap_poll_put_char(struct uart_port *port, unsigned char ch)

static int serial_omap_poll_get_char(struct uart_port *port)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);
unsigned int status;

pm_runtime_get_sync(&up->pdev->dev);
@@ -1065,7 +1065,7 @@ static struct uart_driver serial_omap_reg;

static void serial_omap_console_putchar(struct uart_port *port, int ch)
{
- struct uart_omap_port *up = (struct uart_omap_port *)port;
+ struct uart_omap_port *up = to_uart_omap_port(port);

wait_for_xmitr(up);
serial_out(up, UART_TX, ch);
@@ -1341,7 +1341,7 @@ static void serial_omap_continue_tx(struct uart_omap_port *up)

static void uart_tx_dma_callback(int lch, u16 ch_status, void *data)
{
- struct uart_omap_port *up = (struct uart_omap_port *)data;
+ struct uart_omap_port *up = data;
struct circ_buf *xmit = &up->port.state->xmit;

xmit->tail = (xmit->tail + up->uart_dma.tx_buf_size) & \
--
1.7.12.rc3

2012-08-21 13:06:07

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH v2 00/13] OMAP Serial patches

Hi,

On Tue, Aug 21, 2012 at 03:15:58PM +0300, Felipe Balbi wrote:
> Hi guys,
>
> here's a series of cleanup patches to the OMAP serial
> driver. A later series could be made re-implementing
> DMA using the DMA Engine API. Note that for RX DMA
> we could be using RX Timeout IRQ as a hint that we better
> use PIO instead ;-)
>
> All patches were tested on my pandaboard, but I'd really
> like to receive Tested-by on other platforms.
>
> After this goes in, I'll probably try to get UART wakeup
> working again and only after that look at DMA.
>
> Changes since v1:
>
> . improved commit log on patch 9/13 (formerly 10/13)
> . removed patch 2/13
> . added a new patch switching from spin_lock_irqsave() to spin_lock and
> spin_unlock_irqrestore to spin_unlock
>
> Retested with my pandaboard, UART continues to work:
>
> # grep -i uart /proc/interrupts
> 106: 124 0 GIC OMAP UART2
> # grep -i uart /proc/interrupts
> 106: 189 0 GIC OMAP UART2
> # grep -i uart /proc/interrupts
> 106: 255 0 GIC OMAP UART2
> # grep -i uart /proc/interrupts
> 106: 321 0 GIC OMAP UART2
> # grep -i uart /proc/interrupts
> 106: 387 0 GIC OMAP UART2
> # grep -i uart /proc/interrupts
> 106: 453 0 GIC OMAP UART2
> # grep -i uart /proc/interrupts
> 106: 519 0 GIC OMAP UART2
>
> cheers
>
> ps: if anyone knows a better test for UART, let me know.
>
> for convenience of anyone testing, patches are available on my git tree [1] on
> branch uart
>
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git uart

I have added one extra patch to this series:

From 6921efdb13dda7af216b331bb35535d4b53f004a Mon Sep 17 00:00:00 2001
From: Felipe Balbi <[email protected]>
Date: Tue, 21 Aug 2012 15:48:35 +0300
Subject: [PATCH] serial: omap: drop pm_runtime_irq_safe() usage

pm_runtime_irq_safe() will essentially do an
unbalanced pm_runtime_get_sync() on our parent
device, which might cause problems when trying
to suspend.

In order to prevent that we drop pm_runtime_irq_safe
usage in exchange for a little performance hit
when we enter our IRQ handler while still suspended.

If that happens, what we will do is set the irq_pending
flag, do an asynchronous pm_runtime_get() call and
return IRQ_HANDLED. When our runtime_resume() callback
is executed, we check for that flag, and run our
IRQ handler so that we receive/transmit the pending
characters.

Signed-off-by: Felipe Balbi <[email protected]>
---

One extra patch to OMAP UART driver. This seems to be working
pretty well even after echo mem > /sys/power/state. I can
see that I can even wake my pandaboard up by sending a character
through serial:

# echo mem > /sys/power/state
[ 1335.679260] PM: Syncing filesystems ... done.
[ 1335.684387] Freezing user space processes ... (elapsed 0.00 seconds) done.
[ 1335.691741] Freezing remaining freezable tasks ... (elapsed 0.02 seconds) done.
[ 1335.720886] Suspending console(s) (use no_console_suspend to debug)

[ 1335.734405] PM: suspend of devices complete after 5.492 msecs
[ 1335.735534] PM: late suspend of devices complete after 1.128 msecs
[ 1335.737518] PM: noirq suspend of devices complete after 1.952 msecs
[ 1335.737518] Disabling non-boot CPUs ...
[ 1335.738525] CPU1: shutdown
[ 1338.543762] Successfully put all powerdomains to target state
[ 1338.543853] Enabling non-boot CPUs ...
[ 1338.545654] CPU1: Booted secondary processor
[ 1338.546020] CPU1 is up
[ 1338.547027] PM: noirq resume of devices complete after 0.976 msecs
[ 1338.548400] PM: early resume of devices complete after 0.762 msecs
[ 1339.827087] PM: resume of devices complete after 1278.686 msecs
[ 1339.890960] Restarting tasks ... done.
# # grep -i uart /proc/interrupts
106: 3385 0 GIC OMAP UART2
# echo mem > /sys/power/state
[ 1358.015624] PM: Syncing filesystems ... done.
[ 1358.020812] Freezing user space processes ... (elapsed 0.00 seconds) done.
[ 1358.028167] Freezing remaining freezable tasks ... (elapsed 0.01 seconds) done.
[ 1358.055084] Suspending console(s) (use no_console_suspend to debug)

[ 1358.068847] PM: suspend of devices complete after 5.633 msecs
[ 1358.070007] PM: late suspend of devices complete after 1.126 msecs
[ 1358.072051] PM: noirq suspend of devices complete after 2.040 msecs
[ 1358.072051] Disabling non-boot CPUs ...
[ 1358.073211] CPU1: shutdown
[ 1359.104156] Successfully put all powerdomains to target state
[ 1359.104278] Enabling non-boot CPUs ...
[ 1359.106079] CPU1: Booted secondary processor
[ 1359.106475] CPU1 is up
[ 1359.107482] PM: noirq resume of devices complete after 1.004 msecs
[ 1359.108886] PM: early resume of devices complete after 0.761 msecs
[ 1360.414794] PM: resume of devices complete after 1305.836 msecs
[ 1360.478668] Restarting tasks ... done.
# # grep -i uart /proc/interrupts
106: 3511 0 GIC OMAP UART2

arch/arm/plat-omap/include/plat/omap-serial.h | 1 +
drivers/tty/serial/omap-serial.c | 16 +++++++++++++---
2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
index 743ac80..52328ca 100644
--- a/arch/arm/plat-omap/include/plat/omap-serial.h
+++ b/arch/arm/plat-omap/include/plat/omap-serial.h
@@ -130,6 +130,7 @@ struct uart_omap_port {
u32 context_loss_cnt;
u32 errata;
u8 wakeups_enabled;
+ unsigned int irq_pending:1;

struct pm_qos_request pm_qos_request;
u32 latency;
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index a79658d..7e237f3 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -353,9 +353,13 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
irqreturn_t ret = IRQ_HANDLED;
int max_count = 256;

- spin_lock(&up->port.lock);
- pm_runtime_get_sync(up->dev);
+ if (pm_runtime_suspended(up->dev)) {
+ up->irq_pending = true;
+ pm_runtime_get(up->dev);
+ return IRQ_HANDLED;
+ }

+ spin_lock(&up->port.lock);
do {
iir = serial_in(up, UART_IIR);
if (iir & UART_IIR_NO_INT) {
@@ -400,6 +404,7 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
pm_runtime_mark_last_busy(up->dev);
pm_runtime_put_autosuspend(up->dev);
up->port_activity = jiffies;
+ up->irq_pending = false;

return ret;
}
@@ -1305,7 +1310,6 @@ static int serial_omap_probe(struct platform_device *pdev)
pm_runtime_set_autosuspend_delay(&pdev->dev,
omap_up_info->autosuspend_timeout);

- pm_runtime_irq_safe(&pdev->dev);
pm_runtime_enable(&pdev->dev);
pm_runtime_get_sync(&pdev->dev);

@@ -1416,6 +1420,9 @@ static int serial_omap_runtime_suspend(struct device *dev)
if (!up)
return -EINVAL;

+ if (up->irq_pending)
+ return -EBUSY;
+
if (!pdata)
return 0;

@@ -1452,6 +1459,9 @@ static int serial_omap_runtime_resume(struct device *dev)

up->latency = up->calc_latency;
schedule_work(&up->qos_work);
+
+ if (up->irq_pending)
+ serial_omap_irq(up->port.irq, up);
}

return 0;
--
1.7.12.rc3


--
balbi


Attachments:
(No filename) (7.06 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-08-21 15:11:11

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH v2 00/13] OMAP Serial patches

Hi,

On Tue, Aug 21, 2012 at 04:01:36PM +0300, Felipe Balbi wrote:
> Hi,
>
> On Tue, Aug 21, 2012 at 03:15:58PM +0300, Felipe Balbi wrote:
> > Hi guys,
> >
> > here's a series of cleanup patches to the OMAP serial
> > driver. A later series could be made re-implementing
> > DMA using the DMA Engine API. Note that for RX DMA
> > we could be using RX Timeout IRQ as a hint that we better
> > use PIO instead ;-)
> >
> > All patches were tested on my pandaboard, but I'd really
> > like to receive Tested-by on other platforms.
> >
> > After this goes in, I'll probably try to get UART wakeup
> > working again and only after that look at DMA.
> >
> > Changes since v1:
> >
> > . improved commit log on patch 9/13 (formerly 10/13)
> > . removed patch 2/13
> > . added a new patch switching from spin_lock_irqsave() to spin_lock and
> > spin_unlock_irqrestore to spin_unlock
> >
> > Retested with my pandaboard, UART continues to work:
> >
> > # grep -i uart /proc/interrupts
> > 106: 124 0 GIC OMAP UART2
> > # grep -i uart /proc/interrupts
> > 106: 189 0 GIC OMAP UART2
> > # grep -i uart /proc/interrupts
> > 106: 255 0 GIC OMAP UART2
> > # grep -i uart /proc/interrupts
> > 106: 321 0 GIC OMAP UART2
> > # grep -i uart /proc/interrupts
> > 106: 387 0 GIC OMAP UART2
> > # grep -i uart /proc/interrupts
> > 106: 453 0 GIC OMAP UART2
> > # grep -i uart /proc/interrupts
> > 106: 519 0 GIC OMAP UART2
> >
> > cheers
> >
> > ps: if anyone knows a better test for UART, let me know.
> >
> > for convenience of anyone testing, patches are available on my git tree [1] on
> > branch uart
> >
> > [1] git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git uart
>
> I have added one extra patch to this series:
>
> From 6921efdb13dda7af216b331bb35535d4b53f004a Mon Sep 17 00:00:00 2001
> From: Felipe Balbi <[email protected]>
> Date: Tue, 21 Aug 2012 15:48:35 +0300
> Subject: [PATCH] serial: omap: drop pm_runtime_irq_safe() usage
>
> pm_runtime_irq_safe() will essentially do an
> unbalanced pm_runtime_get_sync() on our parent
> device, which might cause problems when trying
> to suspend.
>
> In order to prevent that we drop pm_runtime_irq_safe
> usage in exchange for a little performance hit
> when we enter our IRQ handler while still suspended.
>
> If that happens, what we will do is set the irq_pending
> flag, do an asynchronous pm_runtime_get() call and
> return IRQ_HANDLED. When our runtime_resume() callback
> is executed, we check for that flag, and run our
> IRQ handler so that we receive/transmit the pending
> characters.
>
> Signed-off-by: Felipe Balbi <[email protected]>
> ---
>
> One extra patch to OMAP UART driver. This seems to be working
> pretty well even after echo mem > /sys/power/state. I can
> see that I can even wake my pandaboard up by sending a character
> through serial:
>
> # echo mem > /sys/power/state
> [ 1335.679260] PM: Syncing filesystems ... done.
> [ 1335.684387] Freezing user space processes ... (elapsed 0.00 seconds) done.
> [ 1335.691741] Freezing remaining freezable tasks ... (elapsed 0.02 seconds) done.
> [ 1335.720886] Suspending console(s) (use no_console_suspend to debug)
>
> [ 1335.734405] PM: suspend of devices complete after 5.492 msecs
> [ 1335.735534] PM: late suspend of devices complete after 1.128 msecs
> [ 1335.737518] PM: noirq suspend of devices complete after 1.952 msecs
> [ 1335.737518] Disabling non-boot CPUs ...
> [ 1335.738525] CPU1: shutdown
> [ 1338.543762] Successfully put all powerdomains to target state
> [ 1338.543853] Enabling non-boot CPUs ...
> [ 1338.545654] CPU1: Booted secondary processor
> [ 1338.546020] CPU1 is up
> [ 1338.547027] PM: noirq resume of devices complete after 0.976 msecs
> [ 1338.548400] PM: early resume of devices complete after 0.762 msecs
> [ 1339.827087] PM: resume of devices complete after 1278.686 msecs
> [ 1339.890960] Restarting tasks ... done.
> # # grep -i uart /proc/interrupts
> 106: 3385 0 GIC OMAP UART2
> # echo mem > /sys/power/state
> [ 1358.015624] PM: Syncing filesystems ... done.
> [ 1358.020812] Freezing user space processes ... (elapsed 0.00 seconds) done.
> [ 1358.028167] Freezing remaining freezable tasks ... (elapsed 0.01 seconds) done.
> [ 1358.055084] Suspending console(s) (use no_console_suspend to debug)
>
> [ 1358.068847] PM: suspend of devices complete after 5.633 msecs
> [ 1358.070007] PM: late suspend of devices complete after 1.126 msecs
> [ 1358.072051] PM: noirq suspend of devices complete after 2.040 msecs
> [ 1358.072051] Disabling non-boot CPUs ...
> [ 1358.073211] CPU1: shutdown
> [ 1359.104156] Successfully put all powerdomains to target state
> [ 1359.104278] Enabling non-boot CPUs ...
> [ 1359.106079] CPU1: Booted secondary processor
> [ 1359.106475] CPU1 is up
> [ 1359.107482] PM: noirq resume of devices complete after 1.004 msecs
> [ 1359.108886] PM: early resume of devices complete after 0.761 msecs
> [ 1360.414794] PM: resume of devices complete after 1305.836 msecs
> [ 1360.478668] Restarting tasks ... done.
> # # grep -i uart /proc/interrupts
> 106: 3511 0 GIC OMAP UART2
>
> arch/arm/plat-omap/include/plat/omap-serial.h | 1 +
> drivers/tty/serial/omap-serial.c | 16 +++++++++++++---
> 2 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
> index 743ac80..52328ca 100644
> --- a/arch/arm/plat-omap/include/plat/omap-serial.h
> +++ b/arch/arm/plat-omap/include/plat/omap-serial.h
> @@ -130,6 +130,7 @@ struct uart_omap_port {
> u32 context_loss_cnt;
> u32 errata;
> u8 wakeups_enabled;
> + unsigned int irq_pending:1;
>
> struct pm_qos_request pm_qos_request;
> u32 latency;
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index a79658d..7e237f3 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -353,9 +353,13 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
> irqreturn_t ret = IRQ_HANDLED;
> int max_count = 256;
>
> - spin_lock(&up->port.lock);
> - pm_runtime_get_sync(up->dev);
> + if (pm_runtime_suspended(up->dev)) {
> + up->irq_pending = true;
> + pm_runtime_get(up->dev);
> + return IRQ_HANDLED;
> + }
>
> + spin_lock(&up->port.lock);
> do {
> iir = serial_in(up, UART_IIR);
> if (iir & UART_IIR_NO_INT) {
> @@ -400,6 +404,7 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
> pm_runtime_mark_last_busy(up->dev);
> pm_runtime_put_autosuspend(up->dev);
> up->port_activity = jiffies;
> + up->irq_pending = false;
>
> return ret;
> }
> @@ -1305,7 +1310,6 @@ static int serial_omap_probe(struct platform_device *pdev)
> pm_runtime_set_autosuspend_delay(&pdev->dev,
> omap_up_info->autosuspend_timeout);
>
> - pm_runtime_irq_safe(&pdev->dev);
> pm_runtime_enable(&pdev->dev);
> pm_runtime_get_sync(&pdev->dev);
>
> @@ -1416,6 +1420,9 @@ static int serial_omap_runtime_suspend(struct device *dev)
> if (!up)
> return -EINVAL;
>
> + if (up->irq_pending)
> + return -EBUSY;
> +
> if (!pdata)
> return 0;
>
> @@ -1452,6 +1459,9 @@ static int serial_omap_runtime_resume(struct device *dev)
>
> up->latency = up->calc_latency;
> schedule_work(&up->qos_work);
> +
> + if (up->irq_pending)
> + serial_omap_irq(up->port.irq, up);
> }
>
> return 0;

let's not apply this extra patch. It actually caused a regression which
was pretty difficult to trigger. I have now dropped it from my series.
Sorry for the noise.

--
balbi


Attachments:
(No filename) (7.65 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-08-23 06:30:56

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH v2 00/13] OMAP Serial patches

Hi,

On Tue, Aug 21, 2012 at 03:15:58PM +0300, Felipe Balbi wrote:
> Hi guys,
>
> here's a series of cleanup patches to the OMAP serial
> driver. A later series could be made re-implementing
> DMA using the DMA Engine API. Note that for RX DMA
> we could be using RX Timeout IRQ as a hint that we better
> use PIO instead ;-)
>
> All patches were tested on my pandaboard, but I'd really
> like to receive Tested-by on other platforms.
>
> After this goes in, I'll probably try to get UART wakeup
> working again and only after that look at DMA.
>
> Changes since v1:
>
> . improved commit log on patch 9/13 (formerly 10/13)
> . removed patch 2/13
> . added a new patch switching from spin_lock_irqsave() to spin_lock and
> spin_unlock_irqrestore to spin_unlock
>
> Retested with my pandaboard, UART continues to work:
>
> # grep -i uart /proc/interrupts
> 106: 124 0 GIC OMAP UART2
> # grep -i uart /proc/interrupts
> 106: 189 0 GIC OMAP UART2
> # grep -i uart /proc/interrupts
> 106: 255 0 GIC OMAP UART2
> # grep -i uart /proc/interrupts
> 106: 321 0 GIC OMAP UART2
> # grep -i uart /proc/interrupts
> 106: 387 0 GIC OMAP UART2
> # grep -i uart /proc/interrupts
> 106: 453 0 GIC OMAP UART2
> # grep -i uart /proc/interrupts
> 106: 519 0 GIC OMAP UART2
>
> cheers
>
> ps: if anyone knows a better test for UART, let me know.
>
> for convenience of anyone testing, patches are available on my git tree [1] on
> branch uart
>
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git uart

Well, it turns out we found a small issue with one of these patches. We
have already fixed the isse. Will re-send the series soon with a few
more patches added. cheers

--
balbi


Attachments:
(No filename) (1.83 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments