Re: [PATCH] serial: 8250_dw: Fix runtime PM handling

Re: [PATCH] serial: 8250_dw: Fix runtime PM handling

[Guenter Roeck]

On Thu, May 10, 2018 at 03:13:56PM +0100, Phil Edworthy wrote:
> When using kgdb, you get an abort when accessing the UART registers.
> This is because the driver has already entered runtime PM and so turned
> off the bus clock needed to access the registers.
> 
> To fix this, set the capability indicating Runtime PM is active while idle.
> 

With this patch applied, I get the already reported traceback [1].
Obviously neither the promised revert [2] nor the presumed fix [3] made it
into the mainline Kernel.

Greg, can you please revert this patch ? If people don't care enough
to fix the problem, we should at least not have to suffer the consequences.

Thanks,
Guenter

> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/tty/serial/8250/8250_dw.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index 0529b5cc094b..aff04f1de3a5 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -576,6 +576,10 @@ static int dw8250_probe(struct platform_device *pdev)
>  	if (!data->skip_autocfg)
>  		dw8250_setup_port(p);
>  
> +#ifdef CONFIG_PM
> +	uart.capabilities |= UART_CAP_RPM;
> +#endif
> +
>  	/* If we have a valid fifosize, try hooking up DMA */
>  	if (p->fifosize) {
>  		data->dma.rxconf.src_maxburst = p->fifosize / 4;
> -- 
> 2.7.4

---
[1] https://www.spinics.net/lists/linux-serial/msg30379.html
[2] https://www.spinics.net/lists/linux-serial/msg30380.html
[3] https://www.spinics.net/lists/linux-serial/msg30757.html

Re: [PATCH] serial: 8250_dw: Fix runtime PM handling

[Tony Lindgren]

* Guenter Roeck <linux@roeck-us.net> [180928 22:45]:
> On Thu, May 10, 2018 at 03:13:56PM +0100, Phil Edworthy wrote:
> > When using kgdb, you get an abort when accessing the UART registers.
> > This is because the driver has already entered runtime PM and so turned
> > off the bus clock needed to access the registers.
> > 
> > To fix this, set the capability indicating Runtime PM is active while idle.
> > 
> 
> With this patch applied, I get the already reported traceback [1].
> Obviously neither the promised revert [2] nor the presumed fix [3] made it
> into the mainline Kernel.
> 
> Greg, can you please revert this patch ? If people don't care enough
> to fix the problem, we should at least not have to suffer the consequences.

Yes please revert until things are fixed.

FYI, what Andy and I discussed earlier is that we can disable
runtime PM for serial layer by taking a PM runtime use count
on the serial driver if there is an active console but we need
a way to attach/detach kernel serial consoel via /sys to prevent
regressions with PM.

So how do we attach/detach kernel serial console? We don't seem
to have a way to attach/detach a kernel console to a UART via
/sys do we? Or am I again 10 years behind or something?

We have /sys/class/vtconsole to bind and unbind a frame
buffer console though.

Andy, do you have your updated patches available somewhere?

Regards,

Tony


> [1] https://www.spinics.net/lists/linux-serial/msg30379.html
> [2] https://www.spinics.net/lists/linux-serial/msg30380.html
> [3] https://www.spinics.net/lists/linux-serial/msg30757.html

[PATCH] serial: 8250_dw: Fix runtime PM handling

[Phil Edworthy]

When using kgdb, you get an abort when accessing the UART registers.
This is because the driver has already entered runtime PM and so turned
off the bus clock needed to access the registers.

To fix this, set the capability indicating Runtime PM is active while idle.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 drivers/tty/serial/8250/8250_dw.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 0529b5c..aff04f1 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -576,6 +576,10 @@ static int dw8250_probe(struct platform_device *pdev)
 	if (!data->skip_autocfg)
 		dw8250_setup_port(p);
 
+#ifdef CONFIG_PM
+	uart.capabilities |= UART_CAP_RPM;
+#endif
+
 	/* If we have a valid fifosize, try hooking up DMA */
 	if (p->fifosize) {
 		data->dma.rxconf.src_maxburst = p->fifosize / 4;
-- 
2.7.4