On Fri, 2014-11-14 at 16:32 -0800, Doug Anderson wrote: > In certain suspend modes on certain boards the 8250 UART may lose > state when the device goes to suspend. If we're using > no_console_suspend this can cause lots of problems during resume. > > Let's cache the basic UART config registers at suspend time and if we > notice that the UART loses state (by looking at a canary stored in the > scratch register) we'll restore it. >
Few comments below. > Signed-off-by: Doug Anderson <[email protected]> > --- > Note that I don't have a board that properly suspends and resumes from > the deepest sleep state that runs atop mainline, so this is tested on > a local tree that is based on 3.14 with backports (specifically on > rk3288-pinky). It is compile tested atop mainline. > > drivers/tty/serial/8250/8250_dw.c | 67 > +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 67 insertions(+) > > diff --git a/drivers/tty/serial/8250/8250_dw.c > b/drivers/tty/serial/8250/8250_dw.c > index 0bfdccc..444ff540 100644 > --- a/drivers/tty/serial/8250/8250_dw.c > +++ b/drivers/tty/serial/8250/8250_dw.c > @@ -38,6 +38,9 @@ > #define DW_UART_CPR 0xf4 /* Component Parameter Register */ > #define DW_UART_UCV 0xf8 /* UART Component Version */ > > +/* We'll place this canary in SCR while suspending; if gone we've lost state > */ > +#define DW_UART_SCR_STATE 0x22 > + > /* Component Parameter Register bits */ > #define DW_UART_CPR_ABP_DATA_WIDTH (3 << 0) > #define DW_UART_CPR_AFCE_MODE (1 << 4) > @@ -63,6 +66,13 @@ struct dw8250_data { > struct clk *pclk; > struct reset_control *rst; > struct uart_8250_dma dma; > + > + int suspending; > + int saved_lcr; Why not u32? > + int saved_dll; > + int saved_dlm; > + int saved_ier; > + int saved_fcr; Could it be separate structure (struct reg_context, for example)? > }; > > #define BYT_PRV_CLK 0x800 > @@ -92,10 +102,35 @@ static void dw8250_force_idle(struct uart_port *p) > (void)p->serial_in(p, UART_RX); > } > > +static void dw8250_serial_restore(struct uart_port *p) > +{ > + struct dw8250_data *data = p->private_data; > + struct uart_8250_port *port8250 = serial8250_get_port(data->line); > + > + data->suspending = 0; > + > + serial_out(port8250, UART_LCR, data->saved_lcr | UART_LCR_DLAB); > + serial_out(port8250, UART_DLL, data->saved_dll); > + serial_out(port8250, UART_DLM, data->saved_dlm); > + serial_out(port8250, UART_LCR, data->saved_lcr); > + > + serial_out(port8250, UART_IER, data->saved_ier); > + serial_out(port8250, UART_FCR, data->saved_fcr); > + serial_out(port8250, UART_MCR, data->last_mcr); > +} > + > static void dw8250_serial_out(struct uart_port *p, int offset, int value) > { > struct dw8250_data *d = p->private_data; > > + /* > + * If we started suspending and we see SCR went back to 0, assume we've > + * suspended and resumed and lost state. Restore it now. > + */ > + if (d->suspending && > + readb(p->membase + (UART_SCR << p->regshift)) != DW_UART_SCR_STATE) > + dw8250_serial_restore(p); > + > if (offset == UART_MCR) > d->last_mcr = value; > > @@ -133,6 +168,14 @@ static void dw8250_serial_out32(struct uart_port *p, int > offset, int value) > { > struct dw8250_data *d = p->private_data; > > + /* > + * If we started suspending and we see SCR went back to 0, assume we've > + * suspended and resumed and lost state. Restore it now. > + */ > + if (d->suspending && > + readb(p->membase + (UART_SCR << p->regshift)) != DW_UART_SCR_STATE) > + dw8250_serial_restore(p); > + > if (offset == UART_MCR) > d->last_mcr = value; > > @@ -487,9 +530,29 @@ static int dw8250_remove(struct platform_device *pdev) > static int dw8250_suspend(struct device *dev) > { > struct dw8250_data *data = dev_get_drvdata(dev); > + struct uart_8250_port *port8250 = serial8250_get_port(data->line); > + struct uart_port *port = &port8250->port; > > serial8250_suspend_port(data->line); > > + /* We only deal with ports that were left on (no_console_suspend) */ > + if (port->suspended) > + return 0; > + > + /* We'll save our registers in case we lose state in suspend */ > + data->saved_fcr = serial_in(port8250, UART_FCR); > + data->saved_ier = serial_in(port8250, UART_IER); > + data->saved_lcr = serial_in(port8250, UART_LCR); > + > + serial_out(port8250, UART_LCR, data->saved_lcr | UART_LCR_DLAB); > + data->saved_dlm = serial_in(port8250, UART_DLM); > + data->saved_dll = serial_in(port8250, UART_DLL); > + serial_out(port8250, UART_LCR, data->saved_lcr); > + > + /* Put a special canary in the scratch so we tell when state is lost */ > + serial_out(port8250, UART_SCR, DW_UART_SCR_STATE); > + data->suspending = 1; > + Could it be dw8250_serial_save() in analogue with _restore()? > return 0; > } > > @@ -497,6 +560,10 @@ static int dw8250_resume(struct device *dev) > { > struct dw8250_data *data = dev_get_drvdata(dev); > > + /* We never lost state; stop checking for canary */ > + if (data->suspending) > + data->suspending = 0; > + > serial8250_resume_port(data->line); > > return 0; -- Andy Shevchenko <[email protected]> Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
