Govindraj <govindraj.ti@xxxxxxxxx> writes:
> On Sat, Jun 25, 2011 at 5:00 AM, Kevin Hilman <khilman@xxxxxx> wrote:
>> "Govindraj.R" <govindraj.raja@xxxxxx> writes:
>>
>>> Adapts omap-serial driver to use pm_runtime api's.
>>>
>>> 1.) Populate reg values to uart port which can be used for context restore.
>>
>> Please make this part a separate patch.
>>
>>> 2.) Moving context_restore func to driver from serial.c
>>> 3.) Adding port_enable/disable func to enable/disable given uart port.
>>> enable port using get_sync and disable using autosuspend.
>>> 4.) using runtime irq safe api to make get_sync be called from irq context.
>>
>>>
>>> Acked-by: Alan Cox <alan@xxxxxxxxxxxxxxx>
>>> Signed-off-by: Govindraj.R <govindraj.raja@xxxxxx>
>>
>> This is great! we're almost there. Some minor comments below...
>>
>>> ---
>>> arch/arm/mach-omap2/serial.c | 22 +++
>>> arch/arm/plat-omap/include/plat/omap-serial.h | 2 +
>>> drivers/tty/serial/omap-serial.c | 212 ++++++++++++++++++++++---
>>> 3 files changed, 210 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
>>> index 8c1a4c7..1651c2c 100644
>>> --- a/arch/arm/mach-omap2/serial.c
>>> +++ b/arch/arm/mach-omap2/serial.c
>>> @@ -189,6 +189,27 @@ static void omap_serial_fill_default_pads(struct omap_board_data *bdata)
>>> }
>>> }
>>>
>>> +static void omap_uart_wakeup_enable(struct platform_device *pdev, bool enable)
>>> +{
>>> + struct omap_uart_port_info *up = pdev->dev.platform_data;
>>> +
>>> + /* Set or clear wake-enable bit */
>>> + if (up->wk_en && up->wk_mask) {
>>> + u32 v = __raw_readl(up->wk_en);
>>> + if (enable)
>>> + v |= up->wk_mask;
>>> + else
>>> + v &= ~up->wk_mask;
>>> + __raw_writel(v, up->wk_en);
>>> + }
>>
>> I think I asked this in previous series, but can't remember the answer
>> now... can't we enable bits in the WKEN registers once at init time,
>> and then just use omap_hwmod_[enable|disable]_wakeup() here?
>>
>
> by default all bits are enabled in WKEN,
where default is the PRCM init code, yes.
> I will use omap_hwmod_[enable|disable]_wakeup() api's
>
> if these API's take care of WKEN regs, then no issue
> using the same.
No, these APIs only affect the module-level wakeup bit in the modules
SYSCONFIG register (ENAWAKEUP bit.)
My question is assuming the PRCM WKEN bits are just left enabled, is
simply toggling the modules SYSCONFIG.ENAWAKEUP bit enough to
enable/disable module wakeups. I assume so, but have not validated it.
>>> + /* Enable or clear io-pad wakeup */
>>> + if (enable)
>>> + omap_device_enable_ioring_wakeup(pdev);
>>> + else
>>> + omap_device_disable_ioring_wakeup(pdev);
>>> +}
>>> +
>>> static void omap_uart_idle_init(struct omap_uart_port_info *uart,
>>> unsigned short num)
>>> {
>>> @@ -332,6 +353,7 @@ void __init omap_serial_init_port(struct omap_board_data *bdata)
>>>
>>> pdata->uartclk = OMAP24XX_BASE_BAUD * 16;
>>> pdata->flags = UPF_BOOT_AUTOCONF;
>>> + pdata->enable_wakeup = omap_uart_wakeup_enable;
>>> if (bdata->id == omap_uart_con_id)
>>> pdata->console_uart = true;
>>>
>>> diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
>>> index 2ca885b..ac30de8 100644
>>> --- a/arch/arm/plat-omap/include/plat/omap-serial.h
>>> +++ b/arch/arm/plat-omap/include/plat/omap-serial.h
>>> @@ -65,6 +65,7 @@ struct omap_uart_port_info {
>>> unsigned int errata;
>>> unsigned int console_uart;
>>>
>>> + void (*enable_wakeup)(struct platform_device *, bool);
>>> void __iomem *wk_st;
>>> void __iomem *wk_en;
>>> u32 wk_mask;
>>> @@ -120,6 +121,7 @@ struct uart_omap_port {
>>> char name[20];
>>> unsigned long port_activity;
>>> unsigned int errata;
>>> + void (*enable_wakeup)(struct platform_device *, bool);
>>> };
>>>
>>> #endif /* __OMAP_SERIAL_H__ */
>>> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
>>> index 47cadf4..897416f 100644
>>> --- a/drivers/tty/serial/omap-serial.c
>>> +++ b/drivers/tty/serial/omap-serial.c
>>> @@ -37,10 +37,14 @@
>>> #include <linux/clk.h>
>>> #include <linux/serial_core.h>
>>> #include <linux/irq.h>
>>> +#include <linux/pm_runtime.h>
>>>
>>> #include <plat/dma.h>
>>> #include <plat/dmtimer.h>
>>> #include <plat/omap-serial.h>
>>> +#include <plat/omap_device.h>
>>> +
>>> +#define OMAP_UART_AUTOSUSPEND_DELAY (30 * HZ) /* Value is msecs */
>>
>> As Jon already pointed out, the HZ here is wrong. Please define this
>> value in msecs.
>>
>
> corrected.
>
>>> static struct uart_omap_port *ui[OMAP_MAX_HSUART_PORTS];
>>>
>>> @@ -94,6 +98,17 @@ serial_omap_get_divisor(struct uart_port *port, unsigned int baud)
>>> return port->uartclk/(baud * divisor);
>>> }
>>>
>>> +static inline void serial_omap_port_disable(struct uart_omap_port *up)
>>> +{
>>> + pm_runtime_mark_last_busy(&up->pdev->dev);
>>> + pm_runtime_put_autosuspend(&up->pdev->dev);
>>> +}
>>> +
>>> +static inline void serial_omap_port_enable(struct uart_omap_port *up)
>>> +{
>>> + pm_runtime_get_sync(&up->pdev->dev);
>>> +}
>>
>> These inlines are not needed. Please use runtime PM calls directly in
>> the code. For _enable(), this is straight forward. For _disable()
>> though, I don't think you want (or need) to _mark_last_busy() for every
>> instance of omap_port_disable(). You only need to do this for ones
>> TX/RX transactions...
>>
>
> Fine. will modify.
>
>
>>> static void serial_omap_stop_rxdma(struct uart_omap_port *up)
>>> {
>>> if (up->uart_dma.rx_dma_used) {
>>> @@ -110,8 +125,11 @@ static void serial_omap_enable_ms(struct uart_port *port)
>>> struct uart_omap_port *up = (struct uart_omap_port *)port;
>>>
>>> dev_dbg(up->port.dev, "serial_omap_enable_ms+%d\n", up->pdev->id);
>>> +
>>> + serial_omap_port_enable(up);
>>> up->ier |= UART_IER_MSI;
>>> serial_out(up, UART_IER, up->ier);
>>> + serial_omap_port_disable(up);
>>
>> ...for example, this one is not really activity based, so should
>> probably just be pm_runtime_get_sync(), write the register, then
>> pm_runtime_put() (async version.)
>>
>> I didn't look at all the others below, but they should be looked at
>> individually.
>>
>
> ok. I will check them.
>
>>> }
>>>
>>> static void serial_omap_stop_tx(struct uart_port *port)
>>> @@ -131,21 +149,26 @@ static void serial_omap_stop_tx(struct uart_port *port)
>>> up->uart_dma.tx_dma_channel = OMAP_UART_DMA_CH_FREE;
>>> }
>>>
>>> + serial_omap_port_enable(up);
>>> if (up->ier & UART_IER_THRI) {
>>> up->ier &= ~UART_IER_THRI;
>>> serial_out(up, UART_IER, up->ier);
>>> }
>>> +
>>> + serial_omap_port_disable(up);
>>> }
>>>
>>> static void serial_omap_stop_rx(struct uart_port *port)
>>> {
>>> struct uart_omap_port *up = (struct uart_omap_port *)port;
>>>
>>> + serial_omap_port_enable(up);
>>> 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);
>>> + serial_omap_port_disable(up);
>>> }
>>>
>>> static inline void receive_chars(struct uart_omap_port *up, int *status)
>>> @@ -261,8 +284,10 @@ static void serial_omap_start_tx(struct uart_port *port)
>>> unsigned int start;
>>> int ret = 0;
>>>
>>> + serial_omap_port_enable(up);
>>> if (!up->use_dma) {
>>> serial_omap_enable_ier_thri(up);
>>> + serial_omap_port_disable(up);
>>> return;
>>> }
>>>
>>> @@ -354,9 +379,12 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
>>> unsigned int iir, lsr;
>>> unsigned long flags;
>>>
>>> + serial_omap_port_enable(up);
>>> iir = serial_in(up, UART_IIR);
>>> - if (iir & UART_IIR_NO_INT)
>>> + if (iir & UART_IIR_NO_INT) {
>>> + serial_omap_port_disable(up);
>>> return IRQ_NONE;
>>> + }
>>>
>>> spin_lock_irqsave(&up->port.lock, flags);
>>> lsr = serial_in(up, UART_LSR);
>>> @@ -378,6 +406,8 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
>>> transmit_chars(up);
>>>
>>> spin_unlock_irqrestore(&up->port.lock, flags);
>>> + serial_omap_port_disable(up);
>>> +
>>> up->port_activity = jiffies;
>>> return IRQ_HANDLED;
>>> }
>>> @@ -388,11 +418,12 @@ static unsigned int serial_omap_tx_empty(struct uart_port *port)
>>> unsigned long flags = 0;
>>> unsigned int ret = 0;
>>>
>>> + serial_omap_port_enable(up);
>>> dev_dbg(up->port.dev, "serial_omap_tx_empty+%d\n", up->pdev->id);
>>> 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);
>>> -
>>> + serial_omap_port_disable(up);
>>> return ret;
>>> }
>>>
>>> @@ -402,7 +433,10 @@ static unsigned int serial_omap_get_mctrl(struct uart_port *port)
>>> unsigned char status;
>>> unsigned int ret = 0;
>>>
>>> + serial_omap_port_enable(up);
>>> status = check_modem_status(up);
>>> + serial_omap_port_disable(up);
>>> +
>>> dev_dbg(up->port.dev, "serial_omap_get_mctrl+%d\n", up->pdev->id);
>>>
>>> if (status & UART_MSR_DCD)
>>> @@ -434,7 +468,9 @@ static void serial_omap_set_mctrl(struct uart_port *port, unsigned int mctrl)
>>> mcr |= UART_MCR_LOOP;
>>>
>>> mcr |= up->mcr;
>>> + serial_omap_port_enable(up);
>>> serial_out(up, UART_MCR, mcr);
>>> + serial_omap_port_disable(up);
>>> }
>>>
>>> static void serial_omap_break_ctl(struct uart_port *port, int break_state)
>>> @@ -443,6 +479,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->pdev->id);
>>> + serial_omap_port_enable(up);
>>> spin_lock_irqsave(&up->port.lock, flags);
>>> if (break_state == -1)
>>> up->lcr |= UART_LCR_SBC;
>>> @@ -450,6 +487,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);
>>> + serial_omap_port_disable(up);
>>> }
>>>
>>> static int serial_omap_startup(struct uart_port *port)
>>> @@ -468,6 +506,7 @@ static int serial_omap_startup(struct uart_port *port)
>>>
>>> dev_dbg(up->port.dev, "serial_omap_startup+%d\n", up->pdev->id);
>>>
>>> + serial_omap_port_enable(up);
>>> /*
>>> * Clear the FIFO buffers and disable them.
>>> * (they will be reenabled in set_termios())
>>> @@ -523,6 +562,7 @@ 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);
>>>
>>> + serial_omap_port_disable(up);
>>> up->port_activity = jiffies;
>>> return 0;
>>> }
>>> @@ -533,6 +573,8 @@ static void serial_omap_shutdown(struct uart_port *port)
>>> unsigned long flags = 0;
>>>
>>> dev_dbg(up->port.dev, "serial_omap_shutdown+%d\n", up->pdev->id);
>>> +
>>> + serial_omap_port_enable(up);
>>> /*
>>> * Disable interrupts from this port
>>> */
>>> @@ -566,6 +608,7 @@ static void serial_omap_shutdown(struct uart_port *port)
>>> up->uart_dma.rx_buf_dma_phys);
>>> up->uart_dma.rx_buf = NULL;
>>> }
>>> + serial_omap_port_disable(up);
>>> free_irq(up->port.irq, up);
>>> }
>>>
>>> @@ -671,6 +714,10 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
>>> baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk/13);
>>> quot = serial_omap_get_divisor(port, baud);
>>>
>>> + up->dll = quot & 0xff;
>>> + up->dlh = quot >> 8;
>>> + up->mdr1 = UART_OMAP_MDR1_DISABLE;
>>> +
>>> up->fcr = UART_FCR_R_TRIG_01 | UART_FCR_T_TRIG_01 |
>>> UART_FCR_ENABLE_FIFO;
>>> if (up->use_dma)
>>> @@ -680,6 +727,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.
>>> */
>>> + serial_omap_port_enable(up);
>>> spin_lock_irqsave(&up->port.lock, flags);
>>>
>>> /*
>>> @@ -723,6 +771,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
>>> up->ier |= UART_IER_MSI;
>>> serial_out(up, UART_IER, up->ier);
>>> serial_out(up, UART_LCR, cval); /* reset DLAB */
>>> + up->lcr = cval;
>>>
>>> /* FIFOs and DMA Settings */
>>>
>>> @@ -758,8 +807,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
>>> serial_out(up, UART_MCR, up->mcr);
>>>
>>> /* Protocol, Baud Rate, and Interrupt Settings */
>>> -
>>> - serial_out(up, UART_OMAP_MDR1, UART_OMAP_MDR1_DISABLE);
>>> + serial_out(up, UART_OMAP_MDR1, up->mdr1);
>>> serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
>>>
>>> up->efr = serial_in(up, UART_EFR);
>>> @@ -769,8 +817,8 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
>>> serial_out(up, UART_IER, 0);
>>> serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
>>>
>>> - serial_out(up, UART_DLL, quot & 0xff); /* LS of divisor */
>>> - serial_out(up, UART_DLM, quot >> 8); /* MS of divisor */
>>> + serial_out(up, UART_DLL, up->dll); /* LS of divisor */
>>> + serial_out(up, UART_DLM, up->dlh); /* MS of divisor */
>>>
>>> serial_out(up, UART_LCR, 0);
>>> serial_out(up, UART_IER, up->ier);
>>> @@ -780,9 +828,11 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
>>> serial_out(up, UART_LCR, cval);
>>>
>>> if (baud > 230400 && baud != 3000000)
>>> - serial_out(up, UART_OMAP_MDR1, UART_OMAP_MDR1_13X_MODE);
>>> + up->mdr1 = UART_OMAP_MDR1_13X_MODE;
>>> else
>>> - serial_out(up, UART_OMAP_MDR1, UART_OMAP_MDR1_16X_MODE);
>>> + up->mdr1 = UART_OMAP_MDR1_16X_MODE;
>>> +
>>> + serial_out(up, UART_OMAP_MDR1, up->mdr1)
>>>
>>> /* Hardware Flow Control Configuration */
>>>
>>> @@ -810,6 +860,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);
>>> + serial_omap_port_disable(up);
>>> dev_dbg(up->port.dev, "serial_omap_set_termios+%d\n", up->pdev->id);
>>> }
>>>
>>> @@ -821,6 +872,8 @@ serial_omap_pm(struct uart_port *port, unsigned int state,
>>> unsigned char efr;
>>>
>>> dev_dbg(up->port.dev, "serial_omap_pm+%d\n", up->pdev->id);
>>> +
>>> + serial_omap_port_enable(up);
>>> 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);
>>> @@ -830,6 +883,10 @@ serial_omap_pm(struct uart_port *port, unsigned int state,
>>> serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
>>> serial_out(up, UART_EFR, efr);
>>> serial_out(up, UART_LCR, 0);
>>> + if (state)
>>> + pm_runtime_put_sync(&up->pdev->dev);
>>> + else
>>> + serial_omap_port_disable(up);
>>
>> OK, so this boils down to either a _put_sync() or a _mark_last_busy +
>> _put_autosuspend(), depending on whether this is suspending or resuming,
>> right?
>>
>
> yes also during bootup.
> disable the ports immediately that are not used during bootup.
>
>> Why the difference?
>
> Need to put the ports down immediately during system wide suspend
> other wise autosuspend delay will prevent system to enter
> suspend state immediately.
OK, as I stated above. All of these serial_omap_port_disable() calls
need to be removed and replaced with runtime PM API calls directly.
Here, I think it can always be a basic _put() for suspend and
_get_sync() for resume.
Incendentally, the 'else' clause of that code (state == 0) should be the
resume case, but you're calling _disable(). That doesn't look right.
>>
>>> }
>>>
>>> static void serial_omap_release_port(struct uart_port *port)
>>> @@ -907,25 +964,31 @@ 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;
>>> +
>>> + serial_omap_port_enable(up);
>>> wait_for_xmitr(up);
>>> serial_out(up, UART_TX, ch);
>>> + serial_omap_port_disable(up);
>>> }
>>>
>>> static int serial_omap_poll_get_char(struct uart_port *port)
>>> {
>>> struct uart_omap_port *up = (struct uart_omap_port *)port;
>>> - unsigned int status = serial_in(up, UART_LSR);
>>> + unsigned int status;
>>>
>>> + serial_omap_port_enable(up);
>>> + status = serial_in(up, UART_LSR);
>>> if (!(status & UART_LSR_DR))
>>> return NO_POLL_CHAR;
>>>
>>> - return serial_in(up, UART_RX);
>>> + status = serial_in(up, UART_RX);
>>> + serial_omap_port_disable(up);
>>> + return status;
>>> }
>>>
>>> #endif /* CONFIG_CONSOLE_POLL */
>>>
>>> #ifdef CONFIG_SERIAL_OMAP_CONSOLE
>>> -
>>> static struct uart_omap_port *serial_omap_console_ports[4];
>>>
>>> static struct uart_driver serial_omap_reg;
>>> @@ -955,6 +1018,8 @@ serial_omap_console_write(struct console *co, const char *s,
>>> else
>>> spin_lock(&up->port.lock);
>>>
>>> + serial_omap_port_enable(up);
>>> +
>>> /*
>>> * First save the IER then disable the interrupts
>>> */
>>> @@ -979,6 +1044,7 @@ serial_omap_console_write(struct console *co, const char *s,
>>> if (up->msr_saved_flags)
>>> check_modem_status(up);
>>>
>>> + serial_omap_port_disable(up);
>>> if (locked)
>>> spin_unlock(&up->port.lock);
>>> local_irq_restore(flags);
>>> @@ -1061,22 +1127,27 @@ static struct uart_driver serial_omap_reg = {
>>> .cons = OMAP_CONSOLE,
>>> };
>>>
>>> -static int
>>> -serial_omap_suspend(struct platform_device *pdev, pm_message_t state)
>>> +static int serial_omap_suspend(struct device *dev)
>>> {
>>> - struct uart_omap_port *up = platform_get_drvdata(pdev);
>>> + struct uart_omap_port *up = dev_get_drvdata(dev);
>>>
>>> - if (up)
>>> + if (up) {
>>> uart_suspend_port(&serial_omap_reg, &up->port);
>>> + console_trylock();
>>
>> This locking needs to be explained. Why it is needed, but very
>> importantly, why you are not checking the return value of the _trylock()
>>
>
> any print after this point can result in failure of immediate system suspending
> due to delayed autosuspend from omap_console_write.
I mean explained in the changelog and preferrably the code.
Also, technically speaking, it's not until the clocks are cut (after the
devices runtime_suspend callback is called) that UART writes will fault.
> log prints after uart suspend and print them during resume.
>
>
>>> + serial_omap_pm(&up->port, 3, 0);
>>
>> Why is this call needed?
>>
>
> Actually this is needed if no_console_suspend is used, for that
> case the console will remain active since serial_omap_pm is not
> getting called from serial_core and port is not suspended
> immediately with put_sync.
>
> prints during system wide suspend and delayed autosuspend
> from console_write keeps system active in no_console_suspend case
> so put in forced suspend state and log all prints to be printed later.
>
> probably needs to a condition check if (no_console_suspend)
No. If the user has requested no_console_suspend, then the user should
expect that the UARTs remain active and thus prevent low-power.
>> uart_suspend_port() calls uart_change_pm() which should call the ->pm
>> method of struct uart_ops (which is serial_omap_pm). What am I missing?
>>
>> Also notice you're not calling serial_omap_pm() in the resume method
>> below to change it back.
>>
>>> + }
>>> return 0;
>>> }
>>
>> Also, device wakeup capability should be checked and enabled/disabled in
>> the suspend path (not only the runtime suspend path.)
>>
>
> ok.
>
>
>>> -static int serial_omap_resume(struct platform_device *dev)
>>> +static int serial_omap_resume(struct device *dev)
>>> {
>>> - struct uart_omap_port *up = platform_get_drvdata(dev);
>>> + struct uart_omap_port *up = dev_get_drvdata(dev);
>>>
>>> - if (up)
>>> + if (up) {
>>> uart_resume_port(&serial_omap_reg, &up->port);
>>> + console_unlock();
>>
>> Again, please describe locking in comments.
>>
>> Also, what happens here if the console_trylock() in your suspend method
>> failed?
>>
>
> need to add a flag to check and unlock
> from return status of trylock.
>
>>> + }
>>> +
>>> return 0;
>>> }
>>>
>>> @@ -1097,6 +1168,7 @@ static void serial_omap_rx_timeout(unsigned long uart_no)
>>> serial_omap_stop_rxdma(up);
>>> up->ier |= (UART_IER_RDI | UART_IER_RLSI);
>>> serial_out(up, UART_IER, up->ier);
>>> + serial_omap_port_disable(up);
>>> }
>>> return;
>>> }
>>> @@ -1128,6 +1200,9 @@ static void serial_omap_rx_timeout(unsigned long uart_no)
>>>
>>> static void uart_rx_dma_callback(int lch, u16 ch_status, void *data)
>>> {
>>> + struct uart_omap_port *up = (struct uart_omap_port *)data;
>>> +
>>> + serial_omap_port_disable(up);
>>> return;
>>> }
>>>
>>> @@ -1135,6 +1210,7 @@ static int serial_omap_start_rxdma(struct uart_omap_port *up)
>>> {
>>> int ret = 0;
>>>
>>> + serial_omap_port_enable(up);
>>> if (up->uart_dma.rx_dma_channel == -1) {
>>> ret = omap_request_dma(up->uart_dma.uart_dma_rx,
>>> "UART Rx DMA",
>>> @@ -1214,6 +1290,7 @@ static void uart_tx_dma_callback(int lch, u16 ch_status, void *data)
>>> serial_omap_stop_tx(&up->port);
>>> up->uart_dma.tx_dma_used = false;
>>> spin_unlock(&(up->uart_dma.tx_lock));
>>> + serial_omap_port_disable(up);
>>> } else {
>>> omap_stop_dma(up->uart_dma.tx_dma_channel);
>>> serial_omap_continue_tx(up);
>>> @@ -1224,9 +1301,10 @@ static void uart_tx_dma_callback(int lch, u16 ch_status, void *data)
>>>
>>> static int serial_omap_probe(struct platform_device *pdev)
>>> {
>>> - struct uart_omap_port *up;
>>> + struct uart_omap_port *up = NULL;
>>> struct resource *mem, *irq, *dma_tx, *dma_rx;
>>> struct omap_uart_port_info *omap_up_info = pdev->dev.platform_data;
>>> + struct omap_device *od;
>>> int ret = -ENOSPC;
>>>
>>> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> @@ -1276,12 +1354,20 @@ static int serial_omap_probe(struct platform_device *pdev)
>>> up->port.ops = &serial_omap_pops;
>>> up->port.line = pdev->id;
>>>
>>> - up->port.membase = omap_up_info->membase;
>>> - up->port.mapbase = omap_up_info->mapbase;
>>> + up->port.mapbase = mem->start;
>>> + up->port.membase = ioremap(mem->start, mem->end - mem->start);
>>> +
>>> + if (!up->port.membase) {
>>> + dev_err(&pdev->dev, "can't ioremap UART\n");
>>> + ret = -ENOMEM;
>>> + goto err1;
>>> + }
>>> +
>>> up->port.flags = omap_up_info->flags;
>>> - up->port.irqflags = omap_up_info->irqflags;
>>> up->port.uartclk = omap_up_info->uartclk;
>>> up->uart_dma.uart_base = mem->start;
>>> + up->errata = omap_up_info->errata;
>>> + up->enable_wakeup = omap_up_info->enable_wakeup;
>>>
>>> if (omap_up_info->dma_enabled) {
>>> up->uart_dma.uart_dma_tx = dma_tx->start;
>>> @@ -1295,18 +1381,36 @@ static int serial_omap_probe(struct platform_device *pdev)
>>> up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
>>> }
>>>
>>> + pm_runtime_use_autosuspend(&pdev->dev);
>>> + pm_runtime_set_autosuspend_delay(&pdev->dev,
>>> + OMAP_UART_AUTOSUSPEND_DELAY);
>>> +
>>> + pm_runtime_enable(&pdev->dev);
>>> + pm_runtime_irq_safe(&pdev->dev);
>>> +
>>> + if (omap_up_info->console_uart) {
>>> + od = to_omap_device(up->pdev);
>>> + omap_hwmod_idle(od->hwmods[0]);
>>> + serial_omap_port_enable(up);
>>> + serial_omap_port_disable(up);
>>> + }
>>> +
>>> ui[pdev->id] = up;
>>> serial_omap_add_console_port(up);
>>>
>>> ret = uart_add_one_port(&serial_omap_reg, &up->port);
>>> if (ret != 0)
>>> - goto do_release_region;
>>> + goto err1;
>>>
>>> + dev_set_drvdata(&pdev->dev, up);
>>> platform_set_drvdata(pdev, up);
>>> +
>>> return 0;
>>> err:
>>> dev_err(&pdev->dev, "[UART%d]: failure [%s]: %d\n",
>>> pdev->id, __func__, ret);
>>> +err1:
>>> + kfree(up);
>>> do_release_region:
>>> release_mem_region(mem->start, (mem->end - mem->start) + 1);
>>> return ret;
>>> @@ -1318,20 +1422,76 @@ static int serial_omap_remove(struct platform_device *dev)
>>>
>>> platform_set_drvdata(dev, NULL);
>>> if (up) {
>>> + pm_runtime_disable(&up->pdev->dev);
>>> uart_remove_one_port(&serial_omap_reg, &up->port);
>>> kfree(up);
>>> }
>>> return 0;
>>> }
>>>
>>> +static void omap_uart_restore_context(struct uart_omap_port *up)
>>> +{
>>> + u16 efr = 0;
>>> +
>>> + serial_out(up, UART_OMAP_MDR1, up->mdr1);
>>> + serial_out(up, UART_LCR, 0xBF); /* Config B mode */
>>> + efr = serial_in(up, UART_EFR);
>>> + serial_out(up, UART_EFR, UART_EFR_ECB);
>>> + serial_out(up, UART_LCR, 0x0); /* Operational mode */
>>> + serial_out(up, UART_IER, 0x0);
>>> + serial_out(up, UART_LCR, 0xBF); /* Config B mode */
>>> + serial_out(up, UART_DLL, up->dll);
>>> + serial_out(up, UART_DLM, up->dlh);
>>> + serial_out(up, UART_LCR, 0x0); /* Operational mode */
>>> + serial_out(up, UART_IER, up->ier);
>>> + serial_out(up, UART_FCR, up->fcr);
>>> + serial_out(up, UART_LCR, 0x80);
>>> + serial_out(up, UART_MCR, up->mcr);
>>> + serial_out(up, UART_LCR, 0xBF); /* Config B mode */
>>> + serial_out(up, UART_EFR, efr);
>>> + serial_out(up, UART_LCR, up->lcr);
>>> + /* UART 16x mode */
>>> + serial_out(up, UART_OMAP_MDR1, up->mdr1);
>>> +}
>>> +
>>> +static int omap_serial_runtime_suspend(struct device *dev)
>>> +{
>>> + struct uart_omap_port *up = dev_get_drvdata(dev);
>>> +
>>> + if (!up)
>>> + goto done;
>>> +
>>> + if (device_may_wakeup(dev))
>>> + up->enable_wakeup(up->pdev, true);
>>> + else
>>> + up->enable_wakeup(up->pdev, false);
>>
>> I know the earlier code was doing it this way too, but an optimization
>> would be to have the driver keep state whether the wakeups are enabled
>> or disabled, so you don't have to keep calling this function (which can
>> be expensive with the PRCM reads/writes.
>>
>
> if dynamically disabled from user space from sys-fs interface.
> it may not reflect disable_wakup immediately if internal state machine of
> wakeup is maintained within uart driver.
Why does that matter? It does not need to take effect immedately, it
only needs to take effect on suspend.
Just update internal state whenever you change the hardware, and whenver
you're about to change the hardware again, check device_may_wakeup() and
check the driver's internal state.
> also need to modify the internals of this func pointer to use
> hmwod API's as commented above.
OK
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
