Hello,
Le 03/05/2022 à 12:05, Tony Lindgren a écrit :
> Hi,
>
> * Romain Naour <romain.naour@xxxxxxxx> [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.
How do you know it should use 2000ms by default?
>
>> 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.
I don't see where pm_runtime_set_autosuspend_delay() would be called to set the
autosuspend delay to 0.
2000ms seems to be related to USB_AUTOSUSPEND_DELAY and only relevant for the
usb stack.
Here nothing seems calling pm_runtime_set_autosuspend_delay(), so the
autosuspend delay is still using 0 as default value. I'm not sure that the
serdev driver really handle the autosuspend delay.
Other driver like the omap-sham set the autosuspend delay default value just
after pm_runtime_use_autosuspend(&pdev->dev):
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/crypto/omap-sham.c?h=linux-5.10.y#n2126
>
> 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.
For now, I'm not sure I need to adjust the timeout.
Best regards,
Romain
>
> Regards,
>
> Tony
>
>
>> [1]
>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=627a545c6bb0c7de09208e6546f5111290477261
