Re-enable the console device after suspending, causes its cflags, ispeed and ospeed to be set anew, basing on the values stored in uport->cons. The issue is that these values are set only once, when parsing console parameters after boot (see uart_set_options()), next after configuring a port in uart_port_startup() these parameteres (cflags, ispeed and ospeed) are copied to termios structure and the orginal one (stored in uport->cons) are cleared, but there is no place in code where those fields are checked against 0. When kernel calls uart_resume_port() and setups console, it copies cflags, ispeed and ospeed values from uart->cons,but those are alread cleared. The efect is that console is broken. This patch address this by preserving the cflags, ispeed and ospeed fields in uart->cons during uart_port_startup().
Signed-off-by: Lukasz Majczak lma@semihalf.com Cc: stable@vger.kernel.org --- drivers/tty/serial/serial_core.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 2bd32c8ece39..394a05c09d87 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -225,9 +225,6 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state, tty->termios.c_cflag = uport->cons->cflag; tty->termios.c_ispeed = uport->cons->ispeed; tty->termios.c_ospeed = uport->cons->ospeed; - uport->cons->cflag = 0; - uport->cons->ispeed = 0; - uport->cons->ospeed = 0; } /* * Initialise the hardware port settings.
On Wed, Mar 01, 2023 at 08:57:51AM +0100, Lukasz Majczak wrote:
Re-enable the console device after suspending, causes its cflags, ispeed and ospeed to be set anew, basing on the values stored in uport->cons. The issue is that these values are set only once, when parsing console parameters after boot (see uart_set_options()), next after configuring a port in uart_port_startup() these parameteres (cflags, ispeed and ospeed) are copied to termios structure and the orginal one (stored in uport->cons) are cleared, but there is no place in code where those fields are checked against 0. When kernel calls uart_resume_port() and setups console, it copies cflags, ispeed and ospeed values from uart->cons,but those are alread cleared. The efect is that console is broken. This patch address this by preserving the cflags, ispeed and ospeed fields in uart->cons during uart_port_startup().
Signed-off-by: Lukasz Majczak lma@semihalf.com Cc: stable@vger.kernel.org
drivers/tty/serial/serial_core.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 2bd32c8ece39..394a05c09d87 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -225,9 +225,6 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state, tty->termios.c_cflag = uport->cons->cflag; tty->termios.c_ispeed = uport->cons->ispeed; tty->termios.c_ospeed = uport->cons->ospeed;
uport->cons->cflag = 0;uport->cons->ispeed = 0;uport->cons->ospeed = 0;
- Initialise the hardware port settings.
-- 2.39.2.722.g9855ee24e9-goog
What commit id does this fix?
thanks,
greg k-h
śr., 1 mar 2023 o 09:39 Greg Kroah-Hartman gregkh@linuxfoundation.org napisał(a):
On Wed, Mar 01, 2023 at 08:57:51AM +0100, Lukasz Majczak wrote:
Re-enable the console device after suspending, causes its cflags, ispeed and ospeed to be set anew, basing on the values stored in uport->cons. The issue is that these values are set only once, when parsing console parameters after boot (see uart_set_options()), next after configuring a port in uart_port_startup() these parameteres (cflags, ispeed and ospeed) are copied to termios structure and the orginal one (stored in uport->cons) are cleared, but there is no place in code where those fields are checked against 0. When kernel calls uart_resume_port() and setups console, it copies cflags, ispeed and ospeed values from uart->cons,but those are alread cleared. The efect is that console is broken. This patch address this by preserving the cflags, ispeed and ospeed fields in uart->cons during uart_port_startup().
Signed-off-by: Lukasz Majczak lma@semihalf.com Cc: stable@vger.kernel.org
drivers/tty/serial/serial_core.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 2bd32c8ece39..394a05c09d87 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -225,9 +225,6 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state, tty->termios.c_cflag = uport->cons->cflag; tty->termios.c_ispeed = uport->cons->ispeed; tty->termios.c_ospeed = uport->cons->ospeed;
uport->cons->cflag = 0;uport->cons->ispeed = 0;uport->cons->ospeed = 0; } /* * Initialise the hardware port settings.-- 2.39.2.722.g9855ee24e9-goog
What commit id does this fix?
thanks,
greg k-h
Hi Greg,
There are actually two commits that introduce problematic uport flags clearing in uart_startup (for the sake of simplicity I'd ignore the older history): https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h... https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h... It's 10 years between those 2 and to me it was hard to decide about picking a proper one for the `Fixes:` tag. How would you recommend to proceed wrt applying this patch on the stable releases?
Best regards, Lukasz
On Wed, Mar 01, 2023 at 10:51:31AM +0100, Lukasz Majczak wrote:
śr., 1 mar 2023 o 09:39 Greg Kroah-Hartman gregkh@linuxfoundation.org napisał(a):
On Wed, Mar 01, 2023 at 08:57:51AM +0100, Lukasz Majczak wrote:
Re-enable the console device after suspending, causes its cflags, ispeed and ospeed to be set anew, basing on the values stored in uport->cons. The issue is that these values are set only once, when parsing console parameters after boot (see uart_set_options()), next after configuring a port in uart_port_startup() these parameteres (cflags, ispeed and ospeed) are copied to termios structure and the orginal one (stored in uport->cons) are cleared, but there is no place in code where those fields are checked against 0. When kernel calls uart_resume_port() and setups console, it copies cflags, ispeed and ospeed values from uart->cons,but those are alread cleared. The efect is that console is broken. This patch address this by preserving the cflags, ispeed and ospeed fields in uart->cons during uart_port_startup().
Signed-off-by: Lukasz Majczak lma@semihalf.com Cc: stable@vger.kernel.org
drivers/tty/serial/serial_core.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 2bd32c8ece39..394a05c09d87 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -225,9 +225,6 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state, tty->termios.c_cflag = uport->cons->cflag; tty->termios.c_ispeed = uport->cons->ispeed; tty->termios.c_ospeed = uport->cons->ospeed;
uport->cons->cflag = 0;uport->cons->ispeed = 0;uport->cons->ospeed = 0; } /* * Initialise the hardware port settings.-- 2.39.2.722.g9855ee24e9-goog
What commit id does this fix?
thanks,
greg k-h
Hi Greg,
There are actually two commits that introduce problematic uport flags clearing in uart_startup (for the sake of simplicity I'd ignore the older history): https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h... https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h... It's 10 years between those 2 and to me it was hard to decide about picking a proper one for the `Fixes:` tag. How would you recommend to proceed wrt applying this patch on the stable releases?
Where do you think this needs to go to? Pick something?
And as you have obviously found this on a device running an older kernel version, what kernel tree(s) did you test it on?
thanks,
greg k-h
śr., 1 mar 2023 o 13:22 Greg Kroah-Hartman gregkh@linuxfoundation.org napisał(a):
On Wed, Mar 01, 2023 at 10:51:31AM +0100, Lukasz Majczak wrote:
śr., 1 mar 2023 o 09:39 Greg Kroah-Hartman gregkh@linuxfoundation.org napisał(a):
On Wed, Mar 01, 2023 at 08:57:51AM +0100, Lukasz Majczak wrote:
Re-enable the console device after suspending, causes its cflags, ispeed and ospeed to be set anew, basing on the values stored in uport->cons. The issue is that these values are set only once, when parsing console parameters after boot (see uart_set_options()), next after configuring a port in uart_port_startup() these parameteres (cflags, ispeed and ospeed) are copied to termios structure and the orginal one (stored in uport->cons) are cleared, but there is no place in code where those fields are checked against 0. When kernel calls uart_resume_port() and setups console, it copies cflags, ispeed and ospeed values from uart->cons,but those are alread cleared. The efect is that console is broken. This patch address this by preserving the cflags, ispeed and ospeed fields in uart->cons during uart_port_startup().
Signed-off-by: Lukasz Majczak lma@semihalf.com Cc: stable@vger.kernel.org
drivers/tty/serial/serial_core.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 2bd32c8ece39..394a05c09d87 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -225,9 +225,6 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state, tty->termios.c_cflag = uport->cons->cflag; tty->termios.c_ispeed = uport->cons->ispeed; tty->termios.c_ospeed = uport->cons->ospeed;
uport->cons->cflag = 0;uport->cons->ispeed = 0;uport->cons->ospeed = 0; } /* * Initialise the hardware port settings.-- 2.39.2.722.g9855ee24e9-goog
What commit id does this fix?
thanks,
greg k-h
Hi Greg,
There are actually two commits that introduce problematic uport flags clearing in uart_startup (for the sake of simplicity I'd ignore the older history): https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h... https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h... It's 10 years between those 2 and to me it was hard to decide about picking a proper one for the `Fixes:` tag. How would you recommend to proceed wrt applying this patch on the stable releases?
Where do you think this needs to go to? Pick something?
And as you have obviously found this on a device running an older kernel version, what kernel tree(s) did you test it on?
thanks,
greg k-h
As this patch applies without conflict on 4.14, I would suggest 4.14+. I have tested the patch on chromes-5.15 (cannonlake device).
Best regards, Lukasz
śr., 1 mar 2023 o 15:09 Lukasz Majczak lma@semihalf.com napisał(a):
śr., 1 mar 2023 o 13:22 Greg Kroah-Hartman gregkh@linuxfoundation.org napisał(a):
On Wed, Mar 01, 2023 at 10:51:31AM +0100, Lukasz Majczak wrote:
śr., 1 mar 2023 o 09:39 Greg Kroah-Hartman gregkh@linuxfoundation.org napisał(a):
On Wed, Mar 01, 2023 at 08:57:51AM +0100, Lukasz Majczak wrote:
Re-enable the console device after suspending, causes its cflags, ispeed and ospeed to be set anew, basing on the values stored in uport->cons. The issue is that these values are set only once, when parsing console parameters after boot (see uart_set_options()), next after configuring a port in uart_port_startup() these parameteres (cflags, ispeed and ospeed) are copied to termios structure and the orginal one (stored in uport->cons) are cleared, but there is no place in code where those fields are checked against 0. When kernel calls uart_resume_port() and setups console, it copies cflags, ispeed and ospeed values from uart->cons,but those are alread cleared. The efect is that console is broken. This patch address this by preserving the cflags, ispeed and ospeed fields in uart->cons during uart_port_startup().
Signed-off-by: Lukasz Majczak lma@semihalf.com Cc: stable@vger.kernel.org
drivers/tty/serial/serial_core.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 2bd32c8ece39..394a05c09d87 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -225,9 +225,6 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state, tty->termios.c_cflag = uport->cons->cflag; tty->termios.c_ispeed = uport->cons->ispeed; tty->termios.c_ospeed = uport->cons->ospeed;
uport->cons->cflag = 0;uport->cons->ispeed = 0;uport->cons->ospeed = 0; } /* * Initialise the hardware port settings.-- 2.39.2.722.g9855ee24e9-goog
What commit id does this fix?
thanks,
greg k-h
Hi Greg,
There are actually two commits that introduce problematic uport flags clearing in uart_startup (for the sake of simplicity I'd ignore the older history): https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h... https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h... It's 10 years between those 2 and to me it was hard to decide about picking a proper one for the `Fixes:` tag. How would you recommend to proceed wrt applying this patch on the stable releases?
Where do you think this needs to go to? Pick something?
And as you have obviously found this on a device running an older kernel version, what kernel tree(s) did you test it on?
thanks,
greg k-h
As this patch applies without conflict on 4.14, I would suggest 4.14+. I have tested the patch on chromes-5.15 (cannonlake device).
Best regards, Lukasz
I have tested my patch also with 4.14 and 6.1 (again chromeos tree). On 4.14 it fixed the issue, but on 6.1 although the console survived the suspend/resume, it is printing different characters than requested - I will try to debug it further.
Best regards Lukasz
On Wed, 1 Mar 2023, Lukasz Majczak wrote:
Re-enable the console device after suspending, causes its cflags,
Re-enabling
ispeed and ospeed to be set anew, basing on the values stored in uport->cons. The issue is that these values are set only once, when parsing console parameters after boot (see uart_set_options()),
Remove "The issue is that" from here and just state:
"These values are set only once when parsing console parameters after boot (see uart_set_options())."
next after configuring a port in uart_port_startup() these parameteres
parameters
(cflags, ispeed and ospeed) are copied to termios structure and the orginal one (stored in uport->cons) are cleared, but there is no place
original
in code where those fields are checked against 0. When kernel calls uart_resume_port() and setups console, it copies cflags, ispeed and ospeed values from uart->cons,but those are alread cleared.
missing space after comma.
alread -> already
The efect is that console is broken.
effect
This patch address this by preserving the cflags, ispeed and
Too many "this", don't start with "This patch" but go directly to the point.
linux-stable-mirror@lists.linaro.org
-
Greg Kroah-Hartman -
Ilpo Järvinen -
Lukasz Majczak