On 10/10/23 08:51, Tony Lindgren wrote:
> Hi,
>
> * Thomas Richard <thomas.richard@xxxxxxxxxxx> [231009 15:13]:
>>> The runtime PM usage count should keep the related power domain on though,
>>> sounds like this issue somewhere else if the power domains get force
>>> suspended with runtime PM usage count?
>>
>> If I understand correctly, there are 2 solutions to keep the power
>> domain on through.
>> The first one is to set the flag GENPD_FLAG_ALWAYS_ON for the power domain.
>> The second one is to set the wakup_path of the device (using
>> device_set_wakeup_path) and set the flag GENPD_FLAG_ACTIVE_WAKEUP to the
>> power domain.
>>
>> I didn't found any flag or option to say that the device is not
>> suspended, but it is not an error, so we don't want to poweroff the
>> power domain.
>
> If no_console_suspend is set then GENPD_FLAG_ALWAYS_ON makes sense to
> me as we want to see the debug messages. This will also alter the SoCs
> suspend state though, so no_console_suspend is of limited use. Can you
> please send an updated patch against tty-next branch for this?
Please find below a proposal based on tty-next branch.
I had to add your patch 'serial: 8250_omap: Fix errors with
no_console_suspend' as it is not present in this branch.
And I need it to not have an error in omap8250_suspend.
>
> It would be good to understand why the related power domain gets suspended
> with active runtime PM usage count though. To me it seems this might be
> an issue somewhere in the SoC related power domain code that just tries
> to force suspend everything.
I found nothing to say, there is a device in use (and not suspended) in
this power domain, so do not poweroff it.
Maybe I missed something.
Regards,
Thomas
>
> Regards,
>
> Tony
diff --git a/drivers/tty/serial/8250/8250_omap.c
b/drivers/tty/serial/8250/8250_omap.c
index 4b33f4529aed..2d42f485c987 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -28,6 +28,7 @@
#include <linux/pm_wakeirq.h>
#include <linux/dma-mapping.h>
#include <linux/sys_soc.h>
+#include <linux/pm_domain.h>
#include "8250.h"
@@ -115,6 +116,12 @@
/* RX FIFO occupancy indicator */
#define UART_OMAP_RX_LVL 0x19
+/*
+ * Copy of the genpd flags for the console.
+ * Only used if console suspend is disabled
+ */
+static unsigned int genpd_flags_console;
+
struct omap8250_priv {
void __iomem *membase;
int line;
@@ -1623,6 +1630,7 @@ static int omap8250_suspend(struct device *dev)
{
struct omap8250_priv *priv = dev_get_drvdata(dev);
struct uart_8250_port *up = serial8250_get_port(priv->line);
+ struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
int err = 0;
serial8250_suspend_port(priv->line);
@@ -1633,8 +1641,19 @@ static int omap8250_suspend(struct device *dev)
if (!device_may_wakeup(dev))
priv->wer = 0;
serial_out(up, UART_OMAP_WER, priv->wer);
- if (uart_console(&up->port) && console_suspend_enabled)
- err = pm_runtime_force_suspend(dev);
+ if (uart_console(&up->port)) {
+ if (console_suspend_enabled)
+ err = pm_runtime_force_suspend(dev);
+ else {
+ /*
+ * The pd shall not be powered-off (no console suspend).
+ * Make copy of genpd flags before to set it always on.
+ * The original value is restored during the resume.
+ */
+ genpd_flags_console = genpd->flags;
+ genpd->flags |= GENPD_FLAG_ALWAYS_ON;
+ }
+ }
flush_work(&priv->qos_work);
return err;
@@ -1644,12 +1663,16 @@ static int omap8250_resume(struct device *dev)
{
struct omap8250_priv *priv = dev_get_drvdata(dev);
struct uart_8250_port *up = serial8250_get_port(priv->line);
+ struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
int err;
if (uart_console(&up->port) && console_suspend_enabled) {
- err = pm_runtime_force_resume(dev);
- if (err)
- return err;
+ if (console_suspend_enabled) {
+ err = pm_runtime_force_resume(dev);
+ if (err)
+ return err;
+ } else
+ genpd->flags = genpd_flags_console;
}
serial8250_resume_port(priv->line);
--
Thomas Richard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
