Re: AM5749: tty serial 8250 omap driver crash

[Tony Lindgren <tony@atomide.com>]

Hi,

* Romain Naour <romain.naour@smile.fr> [220402 10:13]:
> Le 17/02/2022 à 13:58, Tony Lindgren a écrit :
> > Yes but note that 8250_omap autosuspend does not do anything unless the
> > timeouts are manually enabled by the userspace. They are initialized to -1
> > during probe.
> 
> Actually it's not initialized to -1 on my board, it's initialized to 0. See
> commit [1].

Oh you're right. The default should be initialized to 2000ms though, not 0.

> I'm starting to think that is an issue when the 8250_omap driver is used with
> another driver like the gnss serial driver (using a serdev driver).

Oh yes you are right. We do this conditionally now.

> Since the commit [1] there is no autosuspend delay at all, I would suggest to
> add a default autosuspend delay value. I tested with 200ms based on another example.
> 
> diff --git a/drivers/tty/serial/8250/8250_omap.c
> b/drivers/tty/serial/8250/8250_omap.c
> index ec7304d57f5f..8ba830bd493a 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -108,6 +108,9 @@
>  /* RX FIFO occupancy indicator */
>  #define UART_OMAP_RX_LVL               0x19
> 
> +/* Runtime PM autosuspend timeout: 0ms may trigger wakeup issues */
> +#define UART_AUTOSUSPEND_TIMEOUT               200
> +
>  struct omap8250_priv {
>         int line;
>         u8 habit;
> @@ -1409,6 +1412,8 @@ static int omap8250_probe(struct platform_device *pdev)
>          */
>         if (!of_get_available_child_count(pdev->dev.of_node))
>                 pm_runtime_set_autosuspend_delay(&pdev->dev, -1);
> +       else
> +               pm_runtime_set_autosuspend_delay(&pdev->dev,
> UART_AUTOSUSPEND_TIMEOUT);
> 
>         pm_runtime_irq_safe(&pdev->dev);

Hmm the value should be set to the default 2000ms already. If it's not, then we
need to find out what is setting it to 0.

For adjusting the timeout, you may want to check the child serdev driver
runtime PM autosuspend timeout, adjusting that seems a better place to
prevent the 8250 idle. Not sure how we should handle the 8250 specific
timeout though based on the serdev driver requirements.

Regards,

Tony


> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=627a545c6bb0c7de09208e6546f5111290477261