* [PATCH] serial/core: Initialize the console pm state [not found] <1411367422-2095-1-git-send-email-ssreedharan@mvista.com> @ 2014-10-01 17:27 ` Kevin Hilman 2014-10-03 4:35 ` Greg KH ` (2 more replies) 0 siblings, 3 replies; 17+ messages in thread From: Kevin Hilman @ 2014-10-01 17:27 UTC (permalink / raw) To: linux-arm-kernel On Sun, Sep 21, 2014 at 11:30 PM, Sudhir Sreedharan <ssreedharan@mvista.com> wrote: > For console devices having UART_CAP_SLEEP capability, the uart_pm_state has > to be initialized to UART_PM_STATE_ON. Otherwise the LCR regiser values > are reinitialized when uart_change_pm is called from uart_configure_port. > > Signed-off-by: Sudhir Sreedharan <ssreedharan@mvista.com> Multiple boot failures on ARM[1] were bisected down to this patch. How was this patch tested, and on which platforms? Also, the changelog states that this should be done only for UART_CAP_SLEEP, but the patch does it for every UART. Greg, I suggest this patch be dropped from tty-next until it has been better described and tested. Kevin [1] http://lists.linaro.org/pipermail/kernel-build-reports/2014-October/005550.html ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] serial/core: Initialize the console pm state 2014-10-01 17:27 ` [PATCH] serial/core: Initialize the console pm state Kevin Hilman @ 2014-10-03 4:35 ` Greg KH 2014-10-03 12:22 ` Geert Uytterhoeven 2014-10-06 9:27 ` Sudhir Sreedharan 2 siblings, 0 replies; 17+ messages in thread From: Greg KH @ 2014-10-03 4:35 UTC (permalink / raw) To: linux-arm-kernel On Wed, Oct 01, 2014 at 10:27:20AM -0700, Kevin Hilman wrote: > On Sun, Sep 21, 2014 at 11:30 PM, Sudhir Sreedharan > <ssreedharan@mvista.com> wrote: > > For console devices having UART_CAP_SLEEP capability, the uart_pm_state has > > to be initialized to UART_PM_STATE_ON. Otherwise the LCR regiser values > > are reinitialized when uart_change_pm is called from uart_configure_port. > > > > Signed-off-by: Sudhir Sreedharan <ssreedharan@mvista.com> > > Multiple boot failures on ARM[1] were bisected down to this patch. > > How was this patch tested, and on which platforms? > > Also, the changelog states that this should be done only for > UART_CAP_SLEEP, but the patch does it for every UART. > > Greg, I suggest this patch be dropped from tty-next until it has been > better described and tested. Now reverted, thanks for letting me know. greg k-h ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] serial/core: Initialize the console pm state 2014-10-01 17:27 ` [PATCH] serial/core: Initialize the console pm state Kevin Hilman 2014-10-03 4:35 ` Greg KH @ 2014-10-03 12:22 ` Geert Uytterhoeven 2014-10-06 9:48 ` Sudhir Sreedharan 2014-10-06 9:27 ` Sudhir Sreedharan 2 siblings, 1 reply; 17+ messages in thread From: Geert Uytterhoeven @ 2014-10-03 12:22 UTC (permalink / raw) To: linux-arm-kernel On Wed, Oct 1, 2014 at 7:27 PM, Kevin Hilman <khilman@kernel.org> wrote: > On Sun, Sep 21, 2014 at 11:30 PM, Sudhir Sreedharan > <ssreedharan@mvista.com> wrote: >> For console devices having UART_CAP_SLEEP capability, the uart_pm_state has >> to be initialized to UART_PM_STATE_ON. Otherwise the LCR regiser values >> are reinitialized when uart_change_pm is called from uart_configure_port. >> >> Signed-off-by: Sudhir Sreedharan <ssreedharan@mvista.com> > > Multiple boot failures on ARM[1] were bisected down to this patch. > > How was this patch tested, and on which platforms? > > Also, the changelog states that this should be done only for > UART_CAP_SLEEP, but the patch does it for every UART. > > Greg, I suggest this patch be dropped from tty-next until it has been > better described and tested. > > Kevin > > [1] http://lists.linaro.org/pipermail/kernel-build-reports/2014-October/005550.html Perhaps it should call "uart_change_pm(state, UART_PM_STATE_ON)" instead, so the driver's .pm() method is called? UART_CAP_SLEEP seems to be an 8250-only flag. BTW, this was the first commit I reverted when I had an issue with a serial console yesterday, but it didn't seem to have any influence (for me). Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] serial/core: Initialize the console pm state 2014-10-03 12:22 ` Geert Uytterhoeven @ 2014-10-06 9:48 ` Sudhir Sreedharan 0 siblings, 0 replies; 17+ messages in thread From: Sudhir Sreedharan @ 2014-10-06 9:48 UTC (permalink / raw) To: linux-arm-kernel Hi Geert, On Fri, Oct 3, 2014 at 5:52 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Wed, Oct 1, 2014 at 7:27 PM, Kevin Hilman <khilman@kernel.org> wrote: >> On Sun, Sep 21, 2014 at 11:30 PM, Sudhir Sreedharan >> <ssreedharan@mvista.com> wrote: >>> For console devices having UART_CAP_SLEEP capability, the uart_pm_state has >>> to be initialized to UART_PM_STATE_ON. Otherwise the LCR regiser values >>> are reinitialized when uart_change_pm is called from uart_configure_port. >>> >>> Signed-off-by: Sudhir Sreedharan <ssreedharan@mvista.com> >> >> Multiple boot failures on ARM[1] were bisected down to this patch. >> >> How was this patch tested, and on which platforms? >> >> Also, the changelog states that this should be done only for >> UART_CAP_SLEEP, but the patch does it for every UART. >> >> Greg, I suggest this patch be dropped from tty-next until it has been >> better described and tested. >> >> Kevin >> >> [1] http://lists.linaro.org/pipermail/kernel-build-reports/2014-October/005550.html > > Perhaps it should call "uart_change_pm(state, UART_PM_STATE_ON)" > instead, so the driver's .pm() method is called? > If "uart_change_pm(state, UART_PM_STATE_ON);" is called, it will reinitialize the LCR register, thus changing the configuration of console port. This will throw garbage characters from the point where the serial driver initializes till startup is called from userland. Thanks, Sudhir ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] serial/core: Initialize the console pm state 2014-10-01 17:27 ` [PATCH] serial/core: Initialize the console pm state Kevin Hilman 2014-10-03 4:35 ` Greg KH 2014-10-03 12:22 ` Geert Uytterhoeven @ 2014-10-06 9:27 ` Sudhir Sreedharan 2014-10-09 13:42 ` Sudhir Sreedharan 2 siblings, 1 reply; 17+ messages in thread From: Sudhir Sreedharan @ 2014-10-06 9:27 UTC (permalink / raw) To: linux-arm-kernel Hi Kevin, On Wed, Oct 1, 2014 at 10:57 PM, Kevin Hilman <khilman@kernel.org> wrote: > On Sun, Sep 21, 2014 at 11:30 PM, Sudhir Sreedharan > <ssreedharan@mvista.com> wrote: >> For console devices having UART_CAP_SLEEP capability, the uart_pm_state has >> to be initialized to UART_PM_STATE_ON. Otherwise the LCR regiser values >> are reinitialized when uart_change_pm is called from uart_configure_port. >> >> Signed-off-by: Sudhir Sreedharan <ssreedharan@mvista.com> > > Multiple boot failures on ARM[1] were bisected down to this patch. > > How was this patch tested, and on which platforms? This patch was tested on x86-64(haswell) board, which uses ST16650V2 uart(which has UART_CAP_SLEEP). While serial driver gets initialized, console port LCR register is getting reinitalized to 0. Then boot logs will be seen as garbage characters. I will re-check why this failed on the boards/archs you mentioned. Thanks, Sudhir ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] serial/core: Initialize the console pm state 2014-10-06 9:27 ` Sudhir Sreedharan @ 2014-10-09 13:42 ` Sudhir Sreedharan 2014-10-09 13:45 ` [PATCH] tty: serial: 8250_core: Do not call set_sleep for console port Sudhir Sreedharan 0 siblings, 1 reply; 17+ messages in thread From: Sudhir Sreedharan @ 2014-10-09 13:42 UTC (permalink / raw) To: linux-arm-kernel Hi, On Mon, Oct 6, 2014 at 2:57 PM, Sudhir Sreedharan <ssreedharan@mvista.com> wrote: >> >> Multiple boot failures on ARM[1] were bisected down to this patch. >> >> How was this patch tested, and on which platforms? > > > > This patch was tested on x86-64(haswell) board, which uses ST16650V2 > uart(which has UART_CAP_SLEEP). > While serial driver gets initialized, console port LCR register is > getting reinitalized to 0. > Then boot logs will be seen as garbage characters. > > I will re-check why this failed on the boards/archs you mentioned. The issue is, in the boot logs, once the serial driver gets initialized, it throws garbage in the console. The serial device being used is ST16550V2 which is having SLEEP functionality. So when uart_configure_port is called, it calls the serial8250_set_sleep and set the LCR register to 0. The previous patch got failed because those are not based on 8250 and the do_pm functionality is different. Eg. for arndale board, in s3c24xx_serial_pm it uses the clock enable and disable functionality. I have created a new patch which will be confined only to 8250 based serial devices. I have tested it on x86-64 based haswell board, ARM64 based, P5040 powerpc which all uses 8250 based serial device. > > Thanks, > Sudhir ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] tty: serial: 8250_core: Do not call set_sleep for console port 2014-10-09 13:42 ` Sudhir Sreedharan @ 2014-10-09 13:45 ` Sudhir Sreedharan 2014-10-09 14:45 ` Peter Hurley 0 siblings, 1 reply; 17+ messages in thread From: Sudhir Sreedharan @ 2014-10-09 13:45 UTC (permalink / raw) To: linux-arm-kernel In ST16650V2 based serial uarts, while initalizing the PM state, LCR registers are being initialized to 0 in serial8250_set_sleep(). If console port is already initialized and being used, this will throws garbage in the console. Signed-off-by: Sudhir Sreedharan <ssreedharan@mvista.com> --- drivers/tty/serial/8250/8250_core.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index ca5cfdc..994aa25 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -2618,6 +2618,9 @@ void serial8250_do_pm(struct uart_port *port, unsigned int state, { struct uart_8250_port *p = up_to_u8250p(port); + if (port->cons && (port->cons->flags & CON_ENABLED) && (state == 0)) + return; + serial8250_set_sleep(p, state != 0); } EXPORT_SYMBOL(serial8250_do_pm); -- 1.7.0.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH] tty: serial: 8250_core: Do not call set_sleep for console port 2014-10-09 13:45 ` [PATCH] tty: serial: 8250_core: Do not call set_sleep for console port Sudhir Sreedharan @ 2014-10-09 14:45 ` Peter Hurley 2014-10-15 6:43 ` [PATCH] tty: serial: 8250_core: restore the LCR register in set_sleep Sudhir Sreedharan 0 siblings, 1 reply; 17+ messages in thread From: Peter Hurley @ 2014-10-09 14:45 UTC (permalink / raw) To: linux-arm-kernel Hi Sudhir, On 10/09/2014 09:45 AM, Sudhir Sreedharan wrote: > In ST16650V2 based serial uarts, while initalizing the PM state, > LCR registers are being initialized to 0 in serial8250_set_sleep(). > If console port is already initialized and being used, this will > throws garbage in the console. > > Signed-off-by: Sudhir Sreedharan <ssreedharan@mvista.com> > --- > drivers/tty/serial/8250/8250_core.c | 3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c > index ca5cfdc..994aa25 100644 > --- a/drivers/tty/serial/8250/8250_core.c > +++ b/drivers/tty/serial/8250/8250_core.c > @@ -2618,6 +2618,9 @@ void serial8250_do_pm(struct uart_port *port, unsigned int state, > { > struct uart_8250_port *p = up_to_u8250p(port); > > + if (port->cons && (port->cons->flags & CON_ENABLED) && (state == 0)) > + return; > + > serial8250_set_sleep(p, state != 0); > } > EXPORT_SYMBOL(serial8250_do_pm); Please just fix serial8250_set_sleep() register programming to save and restore the LCR. You could also fix: 1. preserving the other EFR bits 2. acquiring the port lock around the register programming. Not entirely sure it's absolutely necessary because the serial core serializes most heavy duty register programming with port mutex, but it's probably wise anyway. Regards, Peter Hurley ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] tty: serial: 8250_core: restore the LCR register in set_sleep 2014-10-09 14:45 ` Peter Hurley @ 2014-10-15 6:43 ` Sudhir Sreedharan 2014-10-16 7:21 ` Kevin Hilman 2014-10-16 12:35 ` Peter Hurley 0 siblings, 2 replies; 17+ messages in thread From: Sudhir Sreedharan @ 2014-10-15 6:43 UTC (permalink / raw) To: linux-arm-kernel In ST16650V2 based serial uarts, while initalizing the PM state, LCR registers are being initialized to 0 in serial8250_set_sleep(). If console port is already initialized and being used, this will throws garbage in the console. Signed-off-by: Sudhir Sreedharan <ssreedharan@mvista.com> --- drivers/tty/serial/8250/8250_core.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index ca5cfdc..e054482 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -595,6 +595,7 @@ static void serial8250_rpm_put_tx(struct uart_8250_port *p) */ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) { + unsigned char lcr, efr; /* * Exar UARTs have a SLEEP register that enables or disables * each UART to enter sleep mode separately. On the XR17V35x the @@ -611,6 +612,8 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) if (p->capabilities & UART_CAP_SLEEP) { if (p->capabilities & UART_CAP_EFR) { + lcr = serial_in(p, UART_LCR); + efr = serial_in(p, UART_EFR); serial_out(p, UART_LCR, UART_LCR_CONF_MODE_B); serial_out(p, UART_EFR, UART_EFR_ECB); serial_out(p, UART_LCR, 0); @@ -618,8 +621,8 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) serial_out(p, UART_IER, sleep ? UART_IERX_SLEEP : 0); if (p->capabilities & UART_CAP_EFR) { serial_out(p, UART_LCR, UART_LCR_CONF_MODE_B); - serial_out(p, UART_EFR, 0); - serial_out(p, UART_LCR, 0); + serial_out(p, UART_EFR, sleep ? 0 : efr); + serial_out(p, UART_LCR, sleep ? 0 : lcr); } } out: -- 1.7.0.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH] tty: serial: 8250_core: restore the LCR register in set_sleep 2014-10-15 6:43 ` [PATCH] tty: serial: 8250_core: restore the LCR register in set_sleep Sudhir Sreedharan @ 2014-10-16 7:21 ` Kevin Hilman 2014-10-16 10:15 ` Sudhir Sreedharan 2014-10-16 12:35 ` Peter Hurley 1 sibling, 1 reply; 17+ messages in thread From: Kevin Hilman @ 2014-10-16 7:21 UTC (permalink / raw) To: linux-arm-kernel Sudhir Sreedharan <ssreedharan@mvista.com> writes: > In ST16650V2 based serial uarts, while initalizing the PM state, > LCR registers are being initialized to 0 in serial8250_set_sleep(). > If console port is already initialized and being used, this will > throws garbage in the console. > > Signed-off-by: Sudhir Sreedharan <ssreedharan@mvista.com> Since this caused regressions in -next last time, could you describe how this was tested, and on what platforms? Thanks, Kevin ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] tty: serial: 8250_core: restore the LCR register in set_sleep 2014-10-16 7:21 ` Kevin Hilman @ 2014-10-16 10:15 ` Sudhir Sreedharan 0 siblings, 0 replies; 17+ messages in thread From: Sudhir Sreedharan @ 2014-10-16 10:15 UTC (permalink / raw) To: linux-arm-kernel Hi Kevin, On Thu, Oct 16, 2014 at 12:51 PM, Kevin Hilman <khilman@kernel.org> wrote: > Sudhir Sreedharan <ssreedharan@mvista.com> writes: > >> In ST16650V2 based serial uarts, while initalizing the PM state, >> LCR registers are being initialized to 0 in serial8250_set_sleep(). >> If console port is already initialized and being used, this will >> throws garbage in the console. >> >> Signed-off-by: Sudhir Sreedharan <ssreedharan@mvista.com> > > Since this caused regressions in -next last time, could you describe how > this was tested, and on what platforms? I have tested this on arm64 board((U6_16550A)),PowerPC P5040 board(16550A)), x86-64 (haswell(ST16650V2), Rangeley(16550A)) board. This patch will modify only for the 8250 based serial devices. Test Case : Booting Multiple times, suspend-resume. Thanks, Sudhir > > Thanks, > > Kevin ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] tty: serial: 8250_core: restore the LCR register in set_sleep 2014-10-15 6:43 ` [PATCH] tty: serial: 8250_core: restore the LCR register in set_sleep Sudhir Sreedharan 2014-10-16 7:21 ` Kevin Hilman @ 2014-10-16 12:35 ` Peter Hurley 2014-10-17 10:25 ` Sudhir Sreedharan 1 sibling, 1 reply; 17+ messages in thread From: Peter Hurley @ 2014-10-16 12:35 UTC (permalink / raw) To: linux-arm-kernel On 10/15/2014 02:43 AM, Sudhir Sreedharan wrote: > In ST16650V2 based serial uarts, while initalizing the PM state, > LCR registers are being initialized to 0 in serial8250_set_sleep(). > If console port is already initialized and being used, this will > throws garbage in the console. > > Signed-off-by: Sudhir Sreedharan <ssreedharan@mvista.com> > --- > drivers/tty/serial/8250/8250_core.c | 7 +++++-- > 1 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c > index ca5cfdc..e054482 100644 > --- a/drivers/tty/serial/8250/8250_core.c > +++ b/drivers/tty/serial/8250/8250_core.c > @@ -595,6 +595,7 @@ static void serial8250_rpm_put_tx(struct uart_8250_port *p) > */ > static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) > { > + unsigned char lcr, efr; > /* > * Exar UARTs have a SLEEP register that enables or disables > * each UART to enter sleep mode separately. On the XR17V35x the > @@ -611,6 +612,8 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) > > if (p->capabilities & UART_CAP_SLEEP) { > if (p->capabilities & UART_CAP_EFR) { > + lcr = serial_in(p, UART_LCR); > + efr = serial_in(p, UART_EFR); > serial_out(p, UART_LCR, UART_LCR_CONF_MODE_B); > serial_out(p, UART_EFR, UART_EFR_ECB); > serial_out(p, UART_LCR, 0); > @@ -618,8 +621,8 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) > serial_out(p, UART_IER, sleep ? UART_IERX_SLEEP : 0); > if (p->capabilities & UART_CAP_EFR) { > serial_out(p, UART_LCR, UART_LCR_CONF_MODE_B); > - serial_out(p, UART_EFR, 0); > - serial_out(p, UART_LCR, 0); > + serial_out(p, UART_EFR, sleep ? 0 : efr); > + serial_out(p, UART_LCR, sleep ? 0 : lcr); Why is it necessary to clear EFR and LCR here? Does the UART not power down? UARTs with CAP_SLEEP but not CAP_EFR don't clear LCR before sleep. However, if there is some kind of intentional side-effect here, then a comment should note that. Regards, Peter Hurley > } > } > out: > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] tty: serial: 8250_core: restore the LCR register in set_sleep 2014-10-16 12:35 ` Peter Hurley @ 2014-10-17 10:25 ` Sudhir Sreedharan 2014-10-17 12:39 ` [PATCH v1] " Sudhir Sreedharan 0 siblings, 1 reply; 17+ messages in thread From: Sudhir Sreedharan @ 2014-10-17 10:25 UTC (permalink / raw) To: linux-arm-kernel Hi Peter, On Thu, Oct 16, 2014 at 6:05 PM, Peter Hurley <peter@hurleysoftware.com> wrote: > On 10/15/2014 02:43 AM, Sudhir Sreedharan wrote: >> In ST16650V2 based serial uarts, while initalizing the PM state, >> LCR registers are being initialized to 0 in serial8250_set_sleep(). >> If console port is already initialized and being used, this will >> throws garbage in the console. >> >> Signed-off-by: Sudhir Sreedharan <ssreedharan@mvista.com> >> --- >> drivers/tty/serial/8250/8250_core.c | 7 +++++-- >> 1 files changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c >> index ca5cfdc..e054482 100644 >> --- a/drivers/tty/serial/8250/8250_core.c >> +++ b/drivers/tty/serial/8250/8250_core.c >> @@ -595,6 +595,7 @@ static void serial8250_rpm_put_tx(struct uart_8250_port *p) >> */ >> static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) >> { >> + unsigned char lcr, efr; >> /* >> * Exar UARTs have a SLEEP register that enables or disables >> * each UART to enter sleep mode separately. On the XR17V35x the >> @@ -611,6 +612,8 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) >> >> if (p->capabilities & UART_CAP_SLEEP) { >> if (p->capabilities & UART_CAP_EFR) { >> + lcr = serial_in(p, UART_LCR); >> + efr = serial_in(p, UART_EFR); >> serial_out(p, UART_LCR, UART_LCR_CONF_MODE_B); >> serial_out(p, UART_EFR, UART_EFR_ECB); >> serial_out(p, UART_LCR, 0); >> @@ -618,8 +621,8 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) >> serial_out(p, UART_IER, sleep ? UART_IERX_SLEEP : 0); >> if (p->capabilities & UART_CAP_EFR) { >> serial_out(p, UART_LCR, UART_LCR_CONF_MODE_B); >> - serial_out(p, UART_EFR, 0); >> - serial_out(p, UART_LCR, 0); >> + serial_out(p, UART_EFR, sleep ? 0 : efr); >> + serial_out(p, UART_LCR, sleep ? 0 : lcr); > > Why is it necessary to clear EFR and LCR here? Does the UART not > power down? > > UARTs with CAP_SLEEP but not CAP_EFR don't clear LCR before sleep. > > However, if there is some kind of intentional side-effect here, > then a comment should note that. Yes, we can restore the LCR and EFR with the saved values. serial_out(p, UART_EFR, efr); serial_out(p, UART_LCR, lcr); I will send a new patch with the changes. Thanks, Sudhir > > Regards, > Peter Hurley > >> } >> } >> out: >> > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v1] tty: serial: 8250_core: restore the LCR register in set_sleep 2014-10-17 10:25 ` Sudhir Sreedharan @ 2014-10-17 12:39 ` Sudhir Sreedharan 2014-10-17 12:46 ` Peter Hurley 2014-10-20 15:01 ` Kevin Hilman 0 siblings, 2 replies; 17+ messages in thread From: Sudhir Sreedharan @ 2014-10-17 12:39 UTC (permalink / raw) To: linux-arm-kernel In ST16650V2 based serial uarts, while initalizing the PM state, LCR registers are being initialized to 0 in serial8250_set_sleep(). If console port is already initialized and being used, this will throws garbage in the console. Signed-off-by: Sudhir Sreedharan <ssreedharan@mvista.com> --- Changes in v1: Removed the condition of sleep flag for restoring the LCR and EFR. Initialized the lcr and efr variables. drivers/tty/serial/8250/8250_core.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index ca5cfdc..b170487 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -595,6 +595,7 @@ static void serial8250_rpm_put_tx(struct uart_8250_port *p) */ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) { + unsigned char lcr = 0, efr = 0; /* * Exar UARTs have a SLEEP register that enables or disables * each UART to enter sleep mode separately. On the XR17V35x the @@ -611,6 +612,8 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) if (p->capabilities & UART_CAP_SLEEP) { if (p->capabilities & UART_CAP_EFR) { + lcr = serial_in(p, UART_LCR); + efr = serial_in(p, UART_EFR); serial_out(p, UART_LCR, UART_LCR_CONF_MODE_B); serial_out(p, UART_EFR, UART_EFR_ECB); serial_out(p, UART_LCR, 0); @@ -618,8 +621,8 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) serial_out(p, UART_IER, sleep ? UART_IERX_SLEEP : 0); if (p->capabilities & UART_CAP_EFR) { serial_out(p, UART_LCR, UART_LCR_CONF_MODE_B); - serial_out(p, UART_EFR, 0); - serial_out(p, UART_LCR, 0); + serial_out(p, UART_EFR, efr); + serial_out(p, UART_LCR, lcr); } } out: -- 1.7.0.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v1] tty: serial: 8250_core: restore the LCR register in set_sleep 2014-10-17 12:39 ` [PATCH v1] " Sudhir Sreedharan @ 2014-10-17 12:46 ` Peter Hurley 2014-10-20 15:01 ` Kevin Hilman 1 sibling, 0 replies; 17+ messages in thread From: Peter Hurley @ 2014-10-17 12:46 UTC (permalink / raw) To: linux-arm-kernel On 10/17/2014 08:39 AM, Sudhir Sreedharan wrote: > In ST16650V2 based serial uarts, while initalizing the PM state, > LCR registers are being initialized to 0 in serial8250_set_sleep(). > If console port is already initialized and being used, this will > throws garbage in the console. Thanks. Reviewed-by: Peter Hurley <peter@hurleysoftware.com> ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v1] tty: serial: 8250_core: restore the LCR register in set_sleep 2014-10-17 12:39 ` [PATCH v1] " Sudhir Sreedharan 2014-10-17 12:46 ` Peter Hurley @ 2014-10-20 15:01 ` Kevin Hilman 2014-10-21 6:45 ` Sudhir Sreedharan 1 sibling, 1 reply; 17+ messages in thread From: Kevin Hilman @ 2014-10-20 15:01 UTC (permalink / raw) To: linux-arm-kernel Sudhir Sreedharan <ssreedharan@mvista.com> writes: > In ST16650V2 based serial uarts, while initalizing the PM state, > LCR registers are being initialized to 0 in serial8250_set_sleep(). > If console port is already initialized and being used, this will > throws garbage in the console. > > Signed-off-by: Sudhir Sreedharan <ssreedharan@mvista.com> Tested-by: Kevin Hilman <khilman@linaro.org> I boot tested this on a bunch of ARM boards and don't see any more failures/regressions. Kevin ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v1] tty: serial: 8250_core: restore the LCR register in set_sleep 2014-10-20 15:01 ` Kevin Hilman @ 2014-10-21 6:45 ` Sudhir Sreedharan 0 siblings, 0 replies; 17+ messages in thread From: Sudhir Sreedharan @ 2014-10-21 6:45 UTC (permalink / raw) To: linux-arm-kernel On Mon, Oct 20, 2014 at 8:31 PM, Kevin Hilman <khilman@kernel.org> wrote: > Sudhir Sreedharan <ssreedharan@mvista.com> writes: > >> In ST16650V2 based serial uarts, while initalizing the PM state, >> LCR registers are being initialized to 0 in serial8250_set_sleep(). >> If console port is already initialized and being used, this will >> throws garbage in the console. >> >> Signed-off-by: Sudhir Sreedharan <ssreedharan@mvista.com> > > Tested-by: Kevin Hilman <khilman@linaro.org> > > I boot tested this on a bunch of ARM boards and don't see any > more failures/regressions. Thanks Kevin ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2014-10-21 6:45 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1411367422-2095-1-git-send-email-ssreedharan@mvista.com>
2014-10-01 17:27 ` [PATCH] serial/core: Initialize the console pm state Kevin Hilman
2014-10-03 4:35 ` Greg KH
2014-10-03 12:22 ` Geert Uytterhoeven
2014-10-06 9:48 ` Sudhir Sreedharan
2014-10-06 9:27 ` Sudhir Sreedharan
2014-10-09 13:42 ` Sudhir Sreedharan
2014-10-09 13:45 ` [PATCH] tty: serial: 8250_core: Do not call set_sleep for console port Sudhir Sreedharan
2014-10-09 14:45 ` Peter Hurley
2014-10-15 6:43 ` [PATCH] tty: serial: 8250_core: restore the LCR register in set_sleep Sudhir Sreedharan
2014-10-16 7:21 ` Kevin Hilman
2014-10-16 10:15 ` Sudhir Sreedharan
2014-10-16 12:35 ` Peter Hurley
2014-10-17 10:25 ` Sudhir Sreedharan
2014-10-17 12:39 ` [PATCH v1] " Sudhir Sreedharan
2014-10-17 12:46 ` Peter Hurley
2014-10-20 15:01 ` Kevin Hilman
2014-10-21 6:45 ` Sudhir Sreedharan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).