Re: serial console vs suspend

On Thu, 2018-05-17 at 10:54 +0300, Andriy Gapon wrote:
> 
> It seems that the serial console, or rather a UART used by it, may require
> re-initialization after waking up (from suspend to RAM).  At least one of my
> systems fails to wake up properly if I configure the serial console.  I've done
> some experimenting with cu (and without the console) and the UART seems to be in
> some weird state, it echoes back the input and does not send anything on the
> wire.  I guess that trying to print to the serial console while the UART is in
> that state results in a hang.
> 
> To test the theory I made this hack and it does help:
> Index: sys/dev/uart/uart_tty.c
> ===================================================================
> --- sys/dev/uart/uart_tty.c	(revision 333667)
> +++ sys/dev/uart/uart_tty.c	(working copy)
> _at__at_ -114,6 +114,13 _at__at_ uart_cninit(struct consdev *cp)
>  	uart_init(di);
>  }
> 
> +void uart_resume(void);
> +void
> +uart_resume(void)
> +{
> +	uart_init(&uart_console);
> +}
> +
>  static void
>  uart_cnterm(struct consdev *cp)
>  {
> Index: sys/x86/acpica/acpi_wakeup.c
> ===================================================================
> --- sys/x86/acpica/acpi_wakeup.c	(revision 333667)
> +++ sys/x86/acpica/acpi_wakeup.c	(working copy)
> _at__at_ -204,6 +205,8 _at__at_ acpi_wakeup_cpus(struct acpi_softc *sc)
>  }
>  #endif
> 
> +extern void uart_resume(void);
> +
>  int
>  acpi_sleep_machdep(struct acpi_softc *sc, int state)
>  {
> _at__at_ -300,6 +303,7 _at__at_ acpi_sleep_machdep(struct acpi_softc *sc, int stat
>  #else
>  		npxresume(susppcbs[0]->sp_fpususpend);
>  #endif
> +		uart_resume();
>  	}
> 
>  	return (1);	/* wakeup successfully */
> 
> 
> ===================================================================
> 
> This is quick and dirty, of course.
> I think that this should go through the console layer.
> And, obviously, not all consoles actually need such a reinit.
> 
> So, maybe:
> cnresume()
> {
> 	for each console {
> 		if cn->cn_ops->cn_resume != NULL
> 			cn->cn_ops->cn_resume(cn)
> 	}
> }
> 
> uart_resume(struct consdev *cp)
> {
> 	uart_init(cp->cn_arg);
> }
> 
> What do you think?
> 
> Hmm, it looks like CONSOLE_DRIVER() does not allow to omit a console method.
> So, will I have to add a dummy resume to all console drivers?

Why should it go through the console layer? If the uart hardware needs
some re-init on resume, won't that be true whether the uart is serving
as a console, a dial-in terminal, or the interface to wifi or bluetooth
chip?

-- Ian