Hello Tony,
Sorry for the delay.
Le 17/02/2022 à 13:58, Tony Lindgren a écrit :
> * Romain Naour <romain.naour@xxxxxxxx> [220217 09:09]:
>> On u-boot devicetree the uart4 node is missing, but we don't plan to use the gps
>> from uboot :)
>> Should I add the uart4 node?
>
> There should be no need for that, kernel should be able to initialize it
> properly no matter what the state is from the bootloader.
ok
>
>> Since removing the uart quirk had some other side effect, I did a shameless hack
>> in 8250_omap.c to disable autosuspend.
>>
>> - pm_runtime_put_autosuspend(port->dev);
>> + pm_runtime_dont_use_autosuspend(port->dev);
>>
>> With that the uart is always up and running.
>
> 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].
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).
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);
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=627a545c6bb0c7de09208e6546f5111290477261
>
>>> The test script I posted does that for all the uarts, probably best not
>>> to do that until the other issues are sorted out :) If so, maybe the issue
>>> on close is that the uart has already autoidled.
>>
>> Indeed. But I'm not sure why the autosuspend is enabled by default?
>
> See above, it's always been initialized to -1 by default so it won't
> do anything. But if you ran the test script I posted, autosuspend timeout
> got enabled for all the uarts. But maybe the issue you posted is yet
> another issue that I don't quite understand yet.
>
> To me it seems we should always have runtime PM functions enable the
> serial port to usable state and get rid of the conditional enable for
> probe and dma.
I'm not able to reproduce the issue by preventing the device from being power
managed. I tried with both method:
echo "-1" > /sys/bus/platform/devices/4806e000.serial/power/autosuspend_delay_ms
or
echo on > /sys/bus/platform/devices/4806e000.serial/power/control
I also played with your script modified to keep autosuspend_delay_ms to 0
(default on my board), but It doesn't trigger the issue.
Note: /sys/bus/platform/devices/4806e000.serial/power/wakeup doesn't exist here.
If setting autosuspend_delay to 200ms by default is ok, I will send a patch.
Best regards,
Romain
>
> Regards,
>
> Tony
