Linux Power Management development
 help / color / mirror / Atom feed
* Re: [PATCH 4/6] ACPI: LPSS: Fix ->suspend_late callbacks handling
From: Rafael J. Wysocki @ 2019-06-29 22:02 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Rafael J. Wysocki, Linux PM, Linux PCI, Linux ACPI, LKML,
	Bjorn Helgaas, Andy Shevchenko, Mika Westerberg, Robert R. Howell
In-Reply-To: <c41b5efe-049c-fc72-8697-b2ec9125a55e@redhat.com>

On Sat, Jun 29, 2019 at 1:34 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Rafael,
>
> On 29-06-19 11:50, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > If the resume_from_noirq flag is set in dev_desc, the ->suspend_late
> > callback provided by the device driver will be invoked at the "noirq"
> > stage of system suspend, via acpi_lpss_do_suspend_late(), which is
> > incorrect.
> >
> > To fix that, drop acpi_lpss_do_suspend_late() and rearrange
> > acpi_lpss_suspend_late() to call pm_generic_suspend_late()
> > directly, before calling acpi_lpss_suspend(), in analogy with
> > acpi_subsys_suspend_late().
>
> Ah now I see the logic in your previous test-patch.
>
> I'm afraid that this is going to break things though, the calling
> of the device-driver's suspend-late method at noirq time is
> *intentional* !

But it is a bug too.

> The resume_from_noirq flag is only set for i2c controllers which
> use: drivers/i2c/busses/i2c-designware-platdrv.c as driver.
>
> This driver's suspend late method looks like this:
>
> static int dw_i2c_plat_suspend(struct device *dev)
> {
>          struct dw_i2c_dev *i_dev = dev_get_drvdata(dev);
>
>          i_dev->suspended = true;
>
>          if (i_dev->shared_with_punit)
>                  return 0;
>
>          i_dev->disable(i_dev);
>          i2c_dw_prepare_clk(i_dev, false);
>
>          return 0;
> }
>
> The i_dev->disable(i_dev) and i2c_dw_prepare_clk(i_dev, false) calls here
> will make the i2c controller non functional. But (some of) these i2c
> controllers are used by code in the _PS0  / _PS3 methods of some PCI
> devices and the PCI core calls _PS0 / _PS3 at *noirq* time, so as explained
> in the commit message which introduced acpi_lpss_do_suspend_late():
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=48402cee6889fb3fce58e95fea1471626286dc63
>
> We must not only make sure that the suspending of the i2c controller is
> ordered so that it happens after these PCI devices are suspended, we must
> also make sure that the i2c controller stays functional until the
> i2c-controller is put in the suspend-noirq state.
>
> If you really want to go this route, we must duplicate the resume_from_noirq
> flag inside drivers/i2c/busses/i2c-designware-platdrv.c, setting it
> only for acpi_lpss enumerated devices (the driver handles a whole lot more
> devices) ans then make the driver's suspend_late method a no-op and instead
> to the suspend from its suspend_noirq callback.
>
> Since pm_generic_suspend_late() is just a wrapper to call dev->driver->pm->suspend_late
> duplicating the resume_from_noirq flag inside i2c-designware-platdrv.c seems
> unproductive.
>
> Note that we have the same thing going on in acpi_lpss.c with resume_early vs
> resume_noirq, we call the resume_early callback from acpi_lpss_resume_noirq
> if the resume_from_noirq flag is set.
>
> TL;DR: the behavior you are trying to fix here is intentional and
> IMHO this patch should be dropped.

I can drop the patch, but the current code is simply incorrect.

If the driver provided a ->suspend_late callback, it wanted that
callback to be invoked during the "late" stage of suspend.  Calling it
later simply papers over a driver bug.  If invoking that callback
during the "late" stage doesn't work, the driver should have provided
a "noirq" callback instead.

> I guess we could / should do a patch adding a comment that the calling
> the drivers' suspend_late / resume_early callback at noirq time is intentional
> to avoid this confusing people in the future.

No.  We need to fix drivers doing wrong things.

Thanks!

^ permalink raw reply

* Re: [GIT PULL] Power management fix for v5.2-rc7
From: pr-tracker-bot @ 2019-06-29 11:45 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linus Torvalds, Linux PM, Linux PCI, Linux Kernel Mailing List
In-Reply-To: <CAJZ5v0ieD3bohHtsk4dZtZL-oJF8NUz5MJ3p+zHvQ2McgaSqaw@mail.gmail.com>

The pull request you sent on Sat, 29 Jun 2019 11:07:09 +0200:

> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git pm-5.2-rc7

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/2407e486066b8ce00dabd7e2b3a2cbcc59ea6186

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker

^ permalink raw reply

* Re: [PATCH 4/6] ACPI: LPSS: Fix ->suspend_late callbacks handling
From: Hans de Goede @ 2019-06-29 11:34 UTC (permalink / raw)
  To: Rafael J. Wysocki, Linux PM
  Cc: Linux PCI, Linux ACPI, LKML, Bjorn Helgaas, Andy Shevchenko,
	Mika Westerberg, Robert R. Howell
In-Reply-To: <2981101.tKVHzisTAg@kreacher>

Hi Rafael,

On 29-06-19 11:50, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> If the resume_from_noirq flag is set in dev_desc, the ->suspend_late
> callback provided by the device driver will be invoked at the "noirq"
> stage of system suspend, via acpi_lpss_do_suspend_late(), which is
> incorrect.
> 
> To fix that, drop acpi_lpss_do_suspend_late() and rearrange
> acpi_lpss_suspend_late() to call pm_generic_suspend_late()
> directly, before calling acpi_lpss_suspend(), in analogy with
> acpi_subsys_suspend_late().

Ah now I see the logic in your previous test-patch.

I'm afraid that this is going to break things though, the calling
of the device-driver's suspend-late method at noirq time is
*intentional* !

The resume_from_noirq flag is only set for i2c controllers which
use: drivers/i2c/busses/i2c-designware-platdrv.c as driver.

This driver's suspend late method looks like this:

static int dw_i2c_plat_suspend(struct device *dev)
{
         struct dw_i2c_dev *i_dev = dev_get_drvdata(dev);

         i_dev->suspended = true;

         if (i_dev->shared_with_punit)
                 return 0;

         i_dev->disable(i_dev);
         i2c_dw_prepare_clk(i_dev, false);

         return 0;
}

The i_dev->disable(i_dev) and i2c_dw_prepare_clk(i_dev, false) calls here
will make the i2c controller non functional. But (some of) these i2c
controllers are used by code in the _PS0  / _PS3 methods of some PCI
devices and the PCI core calls _PS0 / _PS3 at *noirq* time, so as explained
in the commit message which introduced acpi_lpss_do_suspend_late():
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=48402cee6889fb3fce58e95fea1471626286dc63

We must not only make sure that the suspending of the i2c controller is
ordered so that it happens after these PCI devices are suspended, we must
also make sure that the i2c controller stays functional until the
i2c-controller is put in the suspend-noirq state.

If you really want to go this route, we must duplicate the resume_from_noirq
flag inside drivers/i2c/busses/i2c-designware-platdrv.c, setting it
only for acpi_lpss enumerated devices (the driver handles a whole lot more
devices) ans then make the driver's suspend_late method a no-op and instead
to the suspend from its suspend_noirq callback.

Since pm_generic_suspend_late() is just a wrapper to call dev->driver->pm->suspend_late
duplicating the resume_from_noirq flag inside i2c-designware-platdrv.c seems
unproductive.

Note that we have the same thing going on in acpi_lpss.c with resume_early vs
resume_noirq, we call the resume_early callback from acpi_lpss_resume_noirq
if the resume_from_noirq flag is set.

TL;DR: the behavior you are trying to fix here is intentional and
IMHO this patch should be dropped.

I guess we could / should do a patch adding a comment that the calling
the drivers' suspend_late / resume_early callback at noirq time is intentional
to avoid this confusing people in the future.

###

Patches 1-3 and 6 look good to me and you may add my:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
to those.

The comments from this patch also impact patch 5 "ACPI: PM: Introduce "poweroff" callbacks for ACPI PM domain and LPSS"
that should be rewritten to call the device's poweroff_late method at noirq
time if the resume_from_noirq flag is set.

I wonder if patch 5 is necessary though, the acpi_lpss code only deals with a small set
of devices, at least i2c-designware-platdrv.c uses SET_LATE_SYSTEM_SLEEP_PM_OPS so the
poweroff and suspend methods are 1 and the same. If this is true for all involved drivers
(I did not check) then patch 5 is not really necessary.

I guess if fixed to deal with resume_from_noirq the same way the suspend methods
do this patch cannot hurt and if we every add separate power_off methods to the
device drivers later we need this.

Regards,

Hans




> Also notice that acpi_subsys_suspend_late() is not used outside
> of the file where it is defined, so make it static and drop its
> header for the header file containing it.
> 
> Fixes: 48402cee6889 (ACPI / LPSS: Resume BYT/CHT I2C controllers from resume_noirq)
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/acpi/acpi_lpss.c |   23 ++++++++++-------------
>   drivers/acpi/device_pm.c |    3 +--
>   include/linux/acpi.h     |    2 --
>   3 files changed, 11 insertions(+), 17 deletions(-)
> 
> Index: linux-pm/drivers/acpi/acpi_lpss.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/acpi_lpss.c
> +++ linux-pm/drivers/acpi/acpi_lpss.c
> @@ -1034,34 +1034,31 @@ static int acpi_lpss_resume(struct devic
>   }
>   
>   #ifdef CONFIG_PM_SLEEP
> -static int acpi_lpss_do_suspend_late(struct device *dev)
> +static int acpi_lpss_suspend_late(struct device *dev)
>   {
> +	struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
>   	int ret;
>   
>   	if (dev_pm_smart_suspend_and_suspended(dev))
>   		return 0;
>   
>   	ret = pm_generic_suspend_late(dev);
> -	return ret ? ret : acpi_lpss_suspend(dev, device_may_wakeup(dev));
> -}
> +	if (ret)
> +		return ret;
>   
> -static int acpi_lpss_suspend_late(struct device *dev)
> -{
> -	struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
> -
> -	if (pdata->dev_desc->resume_from_noirq)
> -		return 0;
> +	if (!pdata->dev_desc->resume_from_noirq)
> +		return acpi_lpss_suspend(dev, device_may_wakeup(dev));
>   
> -	return acpi_lpss_do_suspend_late(dev);
> +	return 0;
>   }
>   
>   static int acpi_lpss_suspend_noirq(struct device *dev)
>   {
>   	struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
> -	int ret;
>   
> -	if (pdata->dev_desc->resume_from_noirq) {
> -		ret = acpi_lpss_do_suspend_late(dev);
> +	if (!dev_pm_smart_suspend_and_suspended(dev) &&
> +	    pdata->dev_desc->resume_from_noirq) {
> +		int ret = acpi_lpss_suspend(dev, device_may_wakeup(dev));
>   		if (ret)
>   			return ret;
>   	}
> Index: linux-pm/drivers/acpi/device_pm.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/device_pm.c
> +++ linux-pm/drivers/acpi/device_pm.c
> @@ -1069,7 +1069,7 @@ EXPORT_SYMBOL_GPL(acpi_subsys_suspend);
>    * Carry out the generic late suspend procedure for @dev and use ACPI to put
>    * it into a low-power state during system transition into a sleep state.
>    */
> -int acpi_subsys_suspend_late(struct device *dev)
> +static int acpi_subsys_suspend_late(struct device *dev)
>   {
>   	int ret;
>   
> @@ -1079,7 +1079,6 @@ int acpi_subsys_suspend_late(struct devi
>   	ret = pm_generic_suspend_late(dev);
>   	return ret ? ret : acpi_dev_suspend(dev, device_may_wakeup(dev));
>   }
> -EXPORT_SYMBOL_GPL(acpi_subsys_suspend_late);
>   
>   /**
>    * acpi_subsys_suspend_noirq - Run the device driver's "noirq" suspend callback.
> Index: linux-pm/include/linux/acpi.h
> ===================================================================
> --- linux-pm.orig/include/linux/acpi.h
> +++ linux-pm/include/linux/acpi.h
> @@ -916,7 +916,6 @@ static inline int acpi_dev_pm_attach(str
>   int acpi_dev_suspend_late(struct device *dev);
>   int acpi_subsys_prepare(struct device *dev);
>   void acpi_subsys_complete(struct device *dev);
> -int acpi_subsys_suspend_late(struct device *dev);
>   int acpi_subsys_suspend_noirq(struct device *dev);
>   int acpi_subsys_suspend(struct device *dev);
>   int acpi_subsys_freeze(struct device *dev);
> @@ -924,7 +923,6 @@ int acpi_subsys_freeze(struct device *de
>   static inline int acpi_dev_resume_early(struct device *dev) { return 0; }
>   static inline int acpi_subsys_prepare(struct device *dev) { return 0; }
>   static inline void acpi_subsys_complete(struct device *dev) {}
> -static inline int acpi_subsys_suspend_late(struct device *dev) { return 0; }
>   static inline int acpi_subsys_suspend_noirq(struct device *dev) { return 0; }
>   static inline int acpi_subsys_suspend(struct device *dev) { return 0; }
>   static inline int acpi_subsys_freeze(struct device *dev) { return 0; }
> 
> 
> 
> 

^ permalink raw reply

* [PATCH 6/6] ACPI: PM: Drop unused function and function header
From: Rafael J. Wysocki @ 2019-06-29  9:52 UTC (permalink / raw)
  To: Linux PM
  Cc: Linux PCI, Linux ACPI, LKML, Bjorn Helgaas, Andy Shevchenko,
	Mika Westerberg, Hans De Goede, Robert R. Howell
In-Reply-To: <2318839.0szTqvJMZa@kreacher>

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Remove a leftover function header and a static inline stub with no
users from the ACPI header file.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 include/linux/acpi.h |    2 --
 1 file changed, 2 deletions(-)

Index: linux-pm/include/linux/acpi.h
===================================================================
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h
@@ -913,7 +913,6 @@ static inline int acpi_dev_pm_attach(str
 #endif
 
 #if defined(CONFIG_ACPI) && defined(CONFIG_PM_SLEEP)
-int acpi_dev_suspend_late(struct device *dev);
 int acpi_subsys_prepare(struct device *dev);
 void acpi_subsys_complete(struct device *dev);
 int acpi_subsys_suspend_noirq(struct device *dev);
@@ -921,7 +920,6 @@ int acpi_subsys_suspend(struct device *d
 int acpi_subsys_freeze(struct device *dev);
 int acpi_subsys_poweroff(struct device *dev);
 #else
-static inline int acpi_dev_resume_early(struct device *dev) { return 0; }
 static inline int acpi_subsys_prepare(struct device *dev) { return 0; }
 static inline void acpi_subsys_complete(struct device *dev) {}
 static inline int acpi_subsys_suspend_noirq(struct device *dev) { return 0; }





^ permalink raw reply

* [PATCH 5/6] ACPI: PM: Introduce "poweroff" callbacks for ACPI PM domain and LPSS
From: Rafael J. Wysocki @ 2019-06-29  9:51 UTC (permalink / raw)
  To: Linux PM
  Cc: Linux PCI, Linux ACPI, LKML, Bjorn Helgaas, Andy Shevchenko,
	Mika Westerberg, Hans De Goede, Robert R. Howell
In-Reply-To: <2318839.0szTqvJMZa@kreacher>

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

In general, it is not correct to call pm_generic_suspend(),
pm_generic_suspend_late() and pm_generic_suspend_noirq() during the
hibernation's "poweroff" transition, because device drivers may
provide special callbacks to be invoked then and the wrappers in
question cause system suspend callbacks to be run.  Unfortunately,
that happens in the ACPI PM domain and ACPI LPSS.

To address this potential issue, introduce "poweroff" callbacks
for the ACPI PM and LPSS that will use pm_generic_poweroff(),
pm_generic_poweroff_late() and pm_generic_poweroff_noirq() as
appropriate.

Fixes: 05087360fd7a (ACPI / PM: Take SMART_SUSPEND driver flag into account)
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_lpss.c |   40 +++++++++++++++++++++++++++++---
 drivers/acpi/device_pm.c |   58 ++++++++++++++++++++++++++++++++++++++++++++---
 include/linux/acpi.h     |    2 +
 3 files changed, 94 insertions(+), 6 deletions(-)

Index: linux-pm/drivers/acpi/acpi_lpss.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_lpss.c
+++ linux-pm/drivers/acpi/acpi_lpss.c
@@ -1128,6 +1128,40 @@ static int acpi_lpss_restore_early(struc
 
 	return pm_generic_restore_early(dev);
 }
+
+static int acpi_lpss_poweroff_late(struct device *dev)
+{
+	struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
+	int ret;
+
+	if (dev_pm_smart_suspend_and_suspended(dev))
+		return 0;
+
+	ret = pm_generic_poweroff_late(dev);
+	if (ret)
+		return ret;
+
+	if (!pdata->dev_desc->resume_from_noirq)
+		return acpi_lpss_suspend(dev, device_may_wakeup(dev));
+
+	return 0;
+}
+
+static int acpi_lpss_poweroff_noirq(struct device *dev)
+{
+	struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
+
+	if (dev_pm_smart_suspend_and_suspended(dev))
+		return 0;
+
+	if (pdata->dev_desc->resume_from_noirq) {
+		int ret = acpi_lpss_suspend(dev, device_may_wakeup(dev));
+		if (ret)
+			return ret;
+	}
+
+	return pm_generic_poweroff_noirq(dev);
+}
 #endif /* CONFIG_PM_SLEEP */
 
 static int acpi_lpss_runtime_suspend(struct device *dev)
@@ -1161,9 +1195,9 @@ static struct dev_pm_domain acpi_lpss_pm
 		.resume_noirq = acpi_lpss_resume_noirq,
 		.resume_early = acpi_lpss_resume_early,
 		.freeze = acpi_subsys_freeze,
-		.poweroff = acpi_subsys_suspend,
-		.poweroff_late = acpi_lpss_suspend_late,
-		.poweroff_noirq = acpi_lpss_suspend_noirq,
+		.poweroff = acpi_subsys_poweroff,
+		.poweroff_late = acpi_lpss_poweroff_late,
+		.poweroff_noirq = acpi_lpss_poweroff_noirq,
 		.restore_noirq = acpi_lpss_restore_noirq,
 		.restore_early = acpi_lpss_restore_early,
 #endif
Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -1175,6 +1175,58 @@ int acpi_subsys_restore_early(struct dev
 	return ret ? ret : pm_generic_restore_early(dev);
 }
 EXPORT_SYMBOL_GPL(acpi_subsys_restore_early);
+
+/**
+ * acpi_subsys_poweroff - Run the device driver's poweroff callback.
+ * @dev: Device to handle.
+ *
+ * Follow PCI and resume devices from runtime suspend before running their
+ * system poweroff callbacks, unless the driver can cope with runtime-suspended
+ * devices during system suspend and there are no ACPI-specific reasons for
+ * resuming them.
+ */
+int acpi_subsys_poweroff(struct device *dev)
+{
+	if (!dev_pm_test_driver_flags(dev, DPM_FLAG_SMART_SUSPEND) ||
+	    acpi_dev_needs_resume(dev, ACPI_COMPANION(dev)))
+		pm_runtime_resume(dev);
+
+	return pm_generic_poweroff(dev);
+}
+EXPORT_SYMBOL_GPL(acpi_subsys_poweroff);
+
+/**
+ * acpi_subsys_poweroff_late - Run the device driver's poweroff callback.
+ * @dev: Device to handle.
+ *
+ * Carry out the generic late poweroff procedure for @dev and use ACPI to put
+ * it into a low-power state during system transition into a sleep state.
+ */
+static int acpi_subsys_poweroff_late(struct device *dev)
+{
+	int ret;
+
+	if (dev_pm_smart_suspend_and_suspended(dev))
+		return 0;
+
+	ret = pm_generic_poweroff_late(dev);
+	if (ret)
+		return ret;
+
+	return acpi_dev_suspend(dev, device_may_wakeup(dev));
+}
+
+/**
+ * acpi_subsys_poweroff_noirq - Run the driver's "noirq" poweroff callback.
+ * @dev: Device to suspend.
+ */
+static int acpi_subsys_poweroff_noirq(struct device *dev)
+{
+	if (dev_pm_smart_suspend_and_suspended(dev))
+		return 0;
+
+	return pm_generic_poweroff_noirq(dev);
+}
 #endif /* CONFIG_PM_SLEEP */
 
 static struct dev_pm_domain acpi_general_pm_domain = {
@@ -1190,9 +1242,9 @@ static struct dev_pm_domain acpi_general
 		.resume_noirq = acpi_subsys_resume_noirq,
 		.resume_early = acpi_subsys_resume_early,
 		.freeze = acpi_subsys_freeze,
-		.poweroff = acpi_subsys_suspend,
-		.poweroff_late = acpi_subsys_suspend_late,
-		.poweroff_noirq = acpi_subsys_suspend_noirq,
+		.poweroff = acpi_subsys_poweroff,
+		.poweroff_late = acpi_subsys_poweroff_late,
+		.poweroff_noirq = acpi_subsys_poweroff_noirq,
 		.restore_early = acpi_subsys_restore_early,
 #endif
 	},
Index: linux-pm/include/linux/acpi.h
===================================================================
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h
@@ -919,6 +919,7 @@ void acpi_subsys_complete(struct device
 int acpi_subsys_suspend_noirq(struct device *dev);
 int acpi_subsys_suspend(struct device *dev);
 int acpi_subsys_freeze(struct device *dev);
+int acpi_subsys_poweroff(struct device *dev);
 #else
 static inline int acpi_dev_resume_early(struct device *dev) { return 0; }
 static inline int acpi_subsys_prepare(struct device *dev) { return 0; }
@@ -926,6 +927,7 @@ static inline void acpi_subsys_complete(
 static inline int acpi_subsys_suspend_noirq(struct device *dev) { return 0; }
 static inline int acpi_subsys_suspend(struct device *dev) { return 0; }
 static inline int acpi_subsys_freeze(struct device *dev) { return 0; }
+static inline int acpi_subsys_poweroff(struct device *dev) { return 0; }
 #endif
 
 #ifdef CONFIG_ACPI





^ permalink raw reply

* [PATCH 4/6] ACPI: LPSS: Fix ->suspend_late callbacks handling
From: Rafael J. Wysocki @ 2019-06-29  9:50 UTC (permalink / raw)
  To: Linux PM
  Cc: Linux PCI, Linux ACPI, LKML, Bjorn Helgaas, Andy Shevchenko,
	Mika Westerberg, Hans De Goede, Robert R. Howell
In-Reply-To: <2318839.0szTqvJMZa@kreacher>

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

If the resume_from_noirq flag is set in dev_desc, the ->suspend_late
callback provided by the device driver will be invoked at the "noirq"
stage of system suspend, via acpi_lpss_do_suspend_late(), which is
incorrect.

To fix that, drop acpi_lpss_do_suspend_late() and rearrange
acpi_lpss_suspend_late() to call pm_generic_suspend_late()
directly, before calling acpi_lpss_suspend(), in analogy with
acpi_subsys_suspend_late().

Also notice that acpi_subsys_suspend_late() is not used outside
of the file where it is defined, so make it static and drop its
header for the header file containing it.

Fixes: 48402cee6889 (ACPI / LPSS: Resume BYT/CHT I2C controllers from resume_noirq)
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_lpss.c |   23 ++++++++++-------------
 drivers/acpi/device_pm.c |    3 +--
 include/linux/acpi.h     |    2 --
 3 files changed, 11 insertions(+), 17 deletions(-)

Index: linux-pm/drivers/acpi/acpi_lpss.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_lpss.c
+++ linux-pm/drivers/acpi/acpi_lpss.c
@@ -1034,34 +1034,31 @@ static int acpi_lpss_resume(struct devic
 }
 
 #ifdef CONFIG_PM_SLEEP
-static int acpi_lpss_do_suspend_late(struct device *dev)
+static int acpi_lpss_suspend_late(struct device *dev)
 {
+	struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
 	int ret;
 
 	if (dev_pm_smart_suspend_and_suspended(dev))
 		return 0;
 
 	ret = pm_generic_suspend_late(dev);
-	return ret ? ret : acpi_lpss_suspend(dev, device_may_wakeup(dev));
-}
+	if (ret)
+		return ret;
 
-static int acpi_lpss_suspend_late(struct device *dev)
-{
-	struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
-
-	if (pdata->dev_desc->resume_from_noirq)
-		return 0;
+	if (!pdata->dev_desc->resume_from_noirq)
+		return acpi_lpss_suspend(dev, device_may_wakeup(dev));
 
-	return acpi_lpss_do_suspend_late(dev);
+	return 0;
 }
 
 static int acpi_lpss_suspend_noirq(struct device *dev)
 {
 	struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
-	int ret;
 
-	if (pdata->dev_desc->resume_from_noirq) {
-		ret = acpi_lpss_do_suspend_late(dev);
+	if (!dev_pm_smart_suspend_and_suspended(dev) &&
+	    pdata->dev_desc->resume_from_noirq) {
+		int ret = acpi_lpss_suspend(dev, device_may_wakeup(dev));
 		if (ret)
 			return ret;
 	}
Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -1069,7 +1069,7 @@ EXPORT_SYMBOL_GPL(acpi_subsys_suspend);
  * Carry out the generic late suspend procedure for @dev and use ACPI to put
  * it into a low-power state during system transition into a sleep state.
  */
-int acpi_subsys_suspend_late(struct device *dev)
+static int acpi_subsys_suspend_late(struct device *dev)
 {
 	int ret;
 
@@ -1079,7 +1079,6 @@ int acpi_subsys_suspend_late(struct devi
 	ret = pm_generic_suspend_late(dev);
 	return ret ? ret : acpi_dev_suspend(dev, device_may_wakeup(dev));
 }
-EXPORT_SYMBOL_GPL(acpi_subsys_suspend_late);
 
 /**
  * acpi_subsys_suspend_noirq - Run the device driver's "noirq" suspend callback.
Index: linux-pm/include/linux/acpi.h
===================================================================
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h
@@ -916,7 +916,6 @@ static inline int acpi_dev_pm_attach(str
 int acpi_dev_suspend_late(struct device *dev);
 int acpi_subsys_prepare(struct device *dev);
 void acpi_subsys_complete(struct device *dev);
-int acpi_subsys_suspend_late(struct device *dev);
 int acpi_subsys_suspend_noirq(struct device *dev);
 int acpi_subsys_suspend(struct device *dev);
 int acpi_subsys_freeze(struct device *dev);
@@ -924,7 +923,6 @@ int acpi_subsys_freeze(struct device *de
 static inline int acpi_dev_resume_early(struct device *dev) { return 0; }
 static inline int acpi_subsys_prepare(struct device *dev) { return 0; }
 static inline void acpi_subsys_complete(struct device *dev) {}
-static inline int acpi_subsys_suspend_late(struct device *dev) { return 0; }
 static inline int acpi_subsys_suspend_noirq(struct device *dev) { return 0; }
 static inline int acpi_subsys_suspend(struct device *dev) { return 0; }
 static inline int acpi_subsys_freeze(struct device *dev) { return 0; }





^ permalink raw reply

* [PATCH 2/6] PCI: PM: Simplify bus-level hibernation callbacks
From: Rafael J. Wysocki @ 2019-06-29  9:49 UTC (permalink / raw)
  To: Linux PM
  Cc: Linux PCI, Linux ACPI, LKML, Bjorn Helgaas, Andy Shevchenko,
	Mika Westerberg, Hans De Goede, Robert R. Howell
In-Reply-To: <2318839.0szTqvJMZa@kreacher>

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

After a previous change causing all runtime-suspended PCI devices
to be resumed before creating a snapshot image of memory during
hibernation, it is not necessary to worry about the case in which
them might be left in runtime-suspend any more, so get rid of the
code related to that from bus-level PCI hibernation callbacks.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/pci-driver.c |   27 ---------------------------
 1 file changed, 27 deletions(-)

Index: linux-pm/drivers/pci/pci-driver.c
===================================================================
--- linux-pm.orig/drivers/pci/pci-driver.c
+++ linux-pm/drivers/pci/pci-driver.c
@@ -1034,22 +1034,11 @@ static int pci_pm_freeze(struct device *
 	return 0;
 }
 
-static int pci_pm_freeze_late(struct device *dev)
-{
-	if (dev_pm_smart_suspend_and_suspended(dev))
-		return 0;
-
-	return pm_generic_freeze_late(dev);
-}
-
 static int pci_pm_freeze_noirq(struct device *dev)
 {
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 	struct device_driver *drv = dev->driver;
 
-	if (dev_pm_smart_suspend_and_suspended(dev))
-		return 0;
-
 	if (pci_has_legacy_pm_support(pci_dev))
 		return pci_legacy_suspend_late(dev, PMSG_FREEZE);
 
@@ -1079,16 +1068,6 @@ static int pci_pm_thaw_noirq(struct devi
 	struct device_driver *drv = dev->driver;
 	int error = 0;
 
-	/*
-	 * If the device is in runtime suspend, the code below may not work
-	 * correctly with it, so skip that code and make the PM core skip all of
-	 * the subsequent "thaw" callbacks for the device.
-	 */
-	if (dev_pm_smart_suspend_and_suspended(dev)) {
-		dev_pm_skip_next_resume_phases(dev);
-		return 0;
-	}
-
 	if (pcibios_pm_ops.thaw_noirq) {
 		error = pcibios_pm_ops.thaw_noirq(dev);
 		if (error)
@@ -1226,10 +1205,6 @@ static int pci_pm_restore_noirq(struct d
 	struct device_driver *drv = dev->driver;
 	int error = 0;
 
-	/* This is analogous to the pci_pm_resume_noirq() case. */
-	if (dev_pm_smart_suspend_and_suspended(dev))
-		pm_runtime_set_active(dev);
-
 	if (pcibios_pm_ops.restore_noirq) {
 		error = pcibios_pm_ops.restore_noirq(dev);
 		if (error)
@@ -1279,7 +1254,6 @@ static int pci_pm_restore(struct device
 #else /* !CONFIG_HIBERNATE_CALLBACKS */
 
 #define pci_pm_freeze		NULL
-#define pci_pm_freeze_late	NULL
 #define pci_pm_freeze_noirq	NULL
 #define pci_pm_thaw		NULL
 #define pci_pm_thaw_noirq	NULL
@@ -1405,7 +1379,6 @@ static const struct dev_pm_ops pci_dev_p
 	.suspend_late = pci_pm_suspend_late,
 	.resume = pci_pm_resume,
 	.freeze = pci_pm_freeze,
-	.freeze_late = pci_pm_freeze_late,
 	.thaw = pci_pm_thaw,
 	.poweroff = pci_pm_poweroff,
 	.poweroff_late = pci_pm_poweroff_late,





^ permalink raw reply

* [PATCH 3/6] ACPI: PM: Simplify and fix PM domain hibernation callbacks
From: Rafael J. Wysocki @ 2019-06-29  9:50 UTC (permalink / raw)
  To: Linux PM
  Cc: Linux PCI, Linux ACPI, LKML, Bjorn Helgaas, Andy Shevchenko,
	Mika Westerberg, Hans De Goede, Robert R. Howell
In-Reply-To: <2318839.0szTqvJMZa@kreacher>

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

First, after a previous change causing all runtime-suspended devices
in the ACPI PM domain (and ACPI LPSS devices) to be resumed before
creating a snapshot image of memory during hibernation, it is not
necessary to worry about the case in which them might be left in
runtime-suspend any more, so get rid of the code related to that from
ACPI PM domain and ACPI LPSS hibernation callbacks.

Second, it is not correct to use pm_generic_resume_early() and
acpi_subsys_resume_noirq() in hibernation "restore" callbacks (which
currently happens in the ACPI PM domain and ACPI LPSS), so introduce
proper _restore_late and _restore_noirq callbacks for the ACPI PM
domain and ACPI LPSS.

Fixes: 05087360fd7a (ACPI / PM: Take SMART_SUSPEND driver flag into account)
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_lpss.c |   59 +++++++++++++++++++++++++++++++++------------
 drivers/acpi/device_pm.c |   61 ++++++-----------------------------------------
 include/linux/acpi.h     |   10 -------
 3 files changed, 52 insertions(+), 78 deletions(-)

Index: linux-pm/drivers/acpi/acpi_lpss.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_lpss.c
+++ linux-pm/drivers/acpi/acpi_lpss.c
@@ -1069,38 +1069,68 @@ static int acpi_lpss_suspend_noirq(struc
 	return acpi_subsys_suspend_noirq(dev);
 }
 
-static int acpi_lpss_do_resume_early(struct device *dev)
+static int acpi_lpss_resume_noirq(struct device *dev)
 {
-	int ret = acpi_lpss_resume(dev);
+	struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
+	int ret;
+
+	/* Follow acpi_subsys_resune_noirq(). */
+	if (dev_pm_may_skip_resume(dev))
+		return 0;
 
-	return ret ? ret : pm_generic_resume_early(dev);
+	if (dev_pm_smart_suspend_and_suspended(dev))
+		pm_runtime_set_active(dev);
+
+	ret = pm_generic_resume_noirq(dev);
+	if (ret)
+		return ret;
+
+	if (pdata->dev_desc->resume_from_noirq)
+		return acpi_lpss_resume(dev);
+
+	return 0;
 }
 
 static int acpi_lpss_resume_early(struct device *dev)
 {
 	struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
 
-	if (pdata->dev_desc->resume_from_noirq)
-		return 0;
+	if (!pdata->dev_desc->resume_from_noirq) {
+		int ret = acpi_lpss_resume(dev);
+		if (ret)
+			return ret;
+	}
 
-	return acpi_lpss_do_resume_early(dev);
+	return pm_generic_resume_early(dev);
 }
 
-static int acpi_lpss_resume_noirq(struct device *dev)
+static int acpi_lpss_restore_noirq(struct device *dev)
 {
 	struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
 	int ret;
 
-	ret = acpi_subsys_resume_noirq(dev);
+	ret = pm_generic_restore_noirq(dev);
 	if (ret)
 		return ret;
 
-	if (!dev_pm_may_skip_resume(dev) && pdata->dev_desc->resume_from_noirq)
-		ret = acpi_lpss_do_resume_early(dev);
+	if (pdata->dev_desc->resume_from_noirq)
+		return acpi_lpss_resume(dev);
 
-	return ret;
+	return 0;
 }
 
+static int acpi_lpss_restore_early(struct device *dev)
+{
+	struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
+
+	if (!pdata->dev_desc->resume_from_noirq) {
+		int ret = acpi_lpss_resume(dev);
+		if (ret)
+			return ret;
+	}
+
+	return pm_generic_restore_early(dev);
+}
 #endif /* CONFIG_PM_SLEEP */
 
 static int acpi_lpss_runtime_suspend(struct device *dev)
@@ -1134,14 +1164,11 @@ static struct dev_pm_domain acpi_lpss_pm
 		.resume_noirq = acpi_lpss_resume_noirq,
 		.resume_early = acpi_lpss_resume_early,
 		.freeze = acpi_subsys_freeze,
-		.freeze_late = acpi_subsys_freeze_late,
-		.freeze_noirq = acpi_subsys_freeze_noirq,
-		.thaw_noirq = acpi_subsys_thaw_noirq,
 		.poweroff = acpi_subsys_suspend,
 		.poweroff_late = acpi_lpss_suspend_late,
 		.poweroff_noirq = acpi_lpss_suspend_noirq,
-		.restore_noirq = acpi_lpss_resume_noirq,
-		.restore_early = acpi_lpss_resume_early,
+		.restore_noirq = acpi_lpss_restore_noirq,
+		.restore_early = acpi_lpss_restore_early,
 #endif
 		.runtime_suspend = acpi_lpss_runtime_suspend,
 		.runtime_resume = acpi_lpss_runtime_resume,
Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -1116,7 +1116,7 @@ EXPORT_SYMBOL_GPL(acpi_subsys_suspend_no
  * acpi_subsys_resume_noirq - Run the device driver's "noirq" resume callback.
  * @dev: Device to handle.
  */
-int acpi_subsys_resume_noirq(struct device *dev)
+static int acpi_subsys_resume_noirq(struct device *dev)
 {
 	if (dev_pm_may_skip_resume(dev))
 		return 0;
@@ -1131,7 +1131,6 @@ int acpi_subsys_resume_noirq(struct devi
 
 	return pm_generic_resume_noirq(dev);
 }
-EXPORT_SYMBOL_GPL(acpi_subsys_resume_noirq);
 
 /**
  * acpi_subsys_resume_early - Resume device using ACPI.
@@ -1141,12 +1140,11 @@ EXPORT_SYMBOL_GPL(acpi_subsys_resume_noi
  * generic early resume procedure for it during system transition into the
  * working state.
  */
-int acpi_subsys_resume_early(struct device *dev)
+static int acpi_subsys_resume_early(struct device *dev)
 {
 	int ret = acpi_dev_resume(dev);
 	return ret ? ret : pm_generic_resume_early(dev);
 }
-EXPORT_SYMBOL_GPL(acpi_subsys_resume_early);
 
 /**
  * acpi_subsys_freeze - Run the device driver's freeze callback.
@@ -1169,52 +1167,15 @@ int acpi_subsys_freeze(struct device *de
 EXPORT_SYMBOL_GPL(acpi_subsys_freeze);
 
 /**
- * acpi_subsys_freeze_late - Run the device driver's "late" freeze callback.
- * @dev: Device to handle.
- */
-int acpi_subsys_freeze_late(struct device *dev)
-{
-
-	if (dev_pm_smart_suspend_and_suspended(dev))
-		return 0;
-
-	return pm_generic_freeze_late(dev);
-}
-EXPORT_SYMBOL_GPL(acpi_subsys_freeze_late);
-
-/**
- * acpi_subsys_freeze_noirq - Run the device driver's "noirq" freeze callback.
- * @dev: Device to handle.
- */
-int acpi_subsys_freeze_noirq(struct device *dev)
-{
-
-	if (dev_pm_smart_suspend_and_suspended(dev))
-		return 0;
-
-	return pm_generic_freeze_noirq(dev);
-}
-EXPORT_SYMBOL_GPL(acpi_subsys_freeze_noirq);
-
-/**
- * acpi_subsys_thaw_noirq - Run the device driver's "noirq" thaw callback.
- * @dev: Device to handle.
+ * acpi_subsys_restore_early - Restore device using ACPI.
+ * @dev: Device to restore.
  */
-int acpi_subsys_thaw_noirq(struct device *dev)
+int acpi_subsys_restore_early(struct device *dev)
 {
-	/*
-	 * If the device is in runtime suspend, the "thaw" code may not work
-	 * correctly with it, so skip the driver callback and make the PM core
-	 * skip all of the subsequent "thaw" callbacks for the device.
-	 */
-	if (dev_pm_smart_suspend_and_suspended(dev)) {
-		dev_pm_skip_next_resume_phases(dev);
-		return 0;
-	}
-
-	return pm_generic_thaw_noirq(dev);
+	int ret = acpi_dev_resume(dev);
+	return ret ? ret : pm_generic_restore_early(dev);
 }
-EXPORT_SYMBOL_GPL(acpi_subsys_thaw_noirq);
+EXPORT_SYMBOL_GPL(acpi_subsys_restore_early);
 #endif /* CONFIG_PM_SLEEP */
 
 static struct dev_pm_domain acpi_general_pm_domain = {
@@ -1230,14 +1191,10 @@ static struct dev_pm_domain acpi_general
 		.resume_noirq = acpi_subsys_resume_noirq,
 		.resume_early = acpi_subsys_resume_early,
 		.freeze = acpi_subsys_freeze,
-		.freeze_late = acpi_subsys_freeze_late,
-		.freeze_noirq = acpi_subsys_freeze_noirq,
-		.thaw_noirq = acpi_subsys_thaw_noirq,
 		.poweroff = acpi_subsys_suspend,
 		.poweroff_late = acpi_subsys_suspend_late,
 		.poweroff_noirq = acpi_subsys_suspend_noirq,
-		.restore_noirq = acpi_subsys_resume_noirq,
-		.restore_early = acpi_subsys_resume_early,
+		.restore_early = acpi_subsys_restore_early,
 #endif
 	},
 };
Index: linux-pm/include/linux/acpi.h
===================================================================
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h
@@ -918,26 +918,16 @@ int acpi_subsys_prepare(struct device *d
 void acpi_subsys_complete(struct device *dev);
 int acpi_subsys_suspend_late(struct device *dev);
 int acpi_subsys_suspend_noirq(struct device *dev);
-int acpi_subsys_resume_noirq(struct device *dev);
-int acpi_subsys_resume_early(struct device *dev);
 int acpi_subsys_suspend(struct device *dev);
 int acpi_subsys_freeze(struct device *dev);
-int acpi_subsys_freeze_late(struct device *dev);
-int acpi_subsys_freeze_noirq(struct device *dev);
-int acpi_subsys_thaw_noirq(struct device *dev);
 #else
 static inline int acpi_dev_resume_early(struct device *dev) { return 0; }
 static inline int acpi_subsys_prepare(struct device *dev) { return 0; }
 static inline void acpi_subsys_complete(struct device *dev) {}
 static inline int acpi_subsys_suspend_late(struct device *dev) { return 0; }
 static inline int acpi_subsys_suspend_noirq(struct device *dev) { return 0; }
-static inline int acpi_subsys_resume_noirq(struct device *dev) { return 0; }
-static inline int acpi_subsys_resume_early(struct device *dev) { return 0; }
 static inline int acpi_subsys_suspend(struct device *dev) { return 0; }
 static inline int acpi_subsys_freeze(struct device *dev) { return 0; }
-static inline int acpi_subsys_freeze_late(struct device *dev) { return 0; }
-static inline int acpi_subsys_freeze_noirq(struct device *dev) { return 0; }
-static inline int acpi_subsys_thaw_noirq(struct device *dev) { return 0; }
 #endif
 
 #ifdef CONFIG_ACPI





^ permalink raw reply

* [PATCH 1/6] PM: ACPI/PCI: Resume all devices during hibernation
From: Rafael J. Wysocki @ 2019-06-29  9:48 UTC (permalink / raw)
  To: Linux PM
  Cc: Linux PCI, Linux ACPI, LKML, Bjorn Helgaas, Andy Shevchenko,
	Mika Westerberg, Hans De Goede, Robert R. Howell
In-Reply-To: <2318839.0szTqvJMZa@kreacher>

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Both the PCI bus type and the ACPI PM domain avoid resuming
runtime-suspended devices with DPM_FLAG_SMART_SUSPEND set during
hibernation (before creating the snapshot image of system memory),
but that turns out to be a mistake.  It leads to functional issues
and adds complexity that's hard to justify.

For this reason, resume all runtime-suspended PCI devices and all
devices in the ACPI PM domains before creating a snapshot image of
system memory during hibernation.

Fixes: 05087360fd7a (ACPI / PM: Take SMART_SUSPEND driver flag into account)
Fixes: c4b65157aeef (PCI / PM: Take SMART_SUSPEND driver flag into account)
Link: https://lore.kernel.org/linux-acpi/917d4399-2e22-67b1-9d54-808561f9083f@uwyo.edu/T/#maf065fe6e4974f2a9d79f332ab99dfaba635f64c
Reported-by: Robert R. Howell <RHowell@uwyo.edu>
Tested-by: Robert R. Howell <RHowell@uwyo.edu>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/device_pm.c |   13 +++++++------
 drivers/pci/pci-driver.c |   16 ++++++++--------
 2 files changed, 15 insertions(+), 14 deletions(-)

Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -1155,13 +1155,14 @@ EXPORT_SYMBOL_GPL(acpi_subsys_resume_ear
 int acpi_subsys_freeze(struct device *dev)
 {
 	/*
-	 * This used to be done in acpi_subsys_prepare() for all devices and
-	 * some drivers may depend on it, so do it here.  Ideally, however,
-	 * runtime-suspended devices should not be touched during freeze/thaw
-	 * transitions.
+	 * Resume all runtime-suspended devices before creating a snapshot
+	 * image of system memory, because the restore kernel generally cannot
+	 * be expected to always handle them consistently and they need to be
+	 * put into the runtime-active metastate during system resume anyway,
+	 * so it is better to ensure that the state saved in the image will be
+	 * alwyas consistent with that.
 	 */
-	if (!dev_pm_test_driver_flags(dev, DPM_FLAG_SMART_SUSPEND))
-		pm_runtime_resume(dev);
+	pm_runtime_resume(dev);
 
 	return pm_generic_freeze(dev);
 }
Index: linux-pm/drivers/pci/pci-driver.c
===================================================================
--- linux-pm.orig/drivers/pci/pci-driver.c
+++ linux-pm/drivers/pci/pci-driver.c
@@ -1012,15 +1012,15 @@ static int pci_pm_freeze(struct device *
 	}
 
 	/*
-	 * This used to be done in pci_pm_prepare() for all devices and some
-	 * drivers may depend on it, so do it here.  Ideally, runtime-suspended
-	 * devices should not be touched during freeze/thaw transitions,
-	 * however.
+	 * Resume all runtime-suspended devices before creating a snapshot
+	 * image of system memory, because the restore kernel generally cannot
+	 * be expected to always handle them consistently and they need to be
+	 * put into the runtime-active metastate during system resume anyway,
+	 * so it is better to ensure that the state saved in the image will be
+	 * alwyas consistent with that.
 	 */
-	if (!dev_pm_smart_suspend_and_suspended(dev)) {
-		pm_runtime_resume(dev);
-		pci_dev->state_saved = false;
-	}
+	pm_runtime_resume(dev);
+	pci_dev->state_saved = false;
 
 	if (pm->freeze) {
 		int error;





^ permalink raw reply

* [PATCH 0/6] PM: PCI/ACPI: Hibernation handling fixes
From: Rafael J. Wysocki @ 2019-06-29  9:33 UTC (permalink / raw)
  To: Linux PM
  Cc: Linux PCI, Linux ACPI, LKML, Bjorn Helgaas, Andy Shevchenko,
	Mika Westerberg, Hans De Goede, Robert R. Howell

Hi All,

This series of patches addresses a few issues related to the handling of
hibernation in the PCI bus type and the ACPI PM domain and ACPI LPSS driver.

First of all, all of the runtime-suspended PCI devices and devices in the ACPI PM and LPSS
PM domains will be resumed during hibernation (first patch).  This appears to be the
only way to avoid weird corner cases and the benefit from avoiding to resume those
devices during hibernation is questionable.

That change allows the the hibernation callbacks in all of the involved subsystems to be
simplified (patches 2 and 3).

While at it, there is a subtle issue in the LPSS suspend callbacks which is addressed
by patch 4.

Moreover, reusing bus-level suspend callbacks for the "poweroff" transition during
hibernation (which is the case for the ACPI PM domain and LPSS) is incorrect, so patch 5
fixes that.

Finally, there are some leftover items in linux/acpi.h that can be dropped (patch 6).

Thanks,
Rafael




^ permalink raw reply

* [GIT PULL] Power management fix for v5.2-rc7
From: Rafael J. Wysocki @ 2019-06-29  9:07 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Linux PM, Linux PCI, Linux Kernel Mailing List

Hi Linus,

Please pull from the tag

 git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git \
 pm-5.2-rc7

with top-most commit 471a739a47aa7d582f0cdf9d392957d04632bae2

 PCI: PM: Avoid skipping bus-level PM on platforms without ACPI

on top of commit 4b972a01a7da614b4796475f933094751a295a2f

 Linux 5.2-rc6

to receive a PCI power management fix for 5.2-rc7.

This avoids skipping bus-level PCI power management during system
resume for PCIe ports left in D0 during the preceding suspend
transition on platforms where the power states of those ports
can change out of the PCI layer's control.

Thanks!


---------------

Rafael J. Wysocki (1):
      PCI: PM: Avoid skipping bus-level PM on platforms without ACPI

---------------

 drivers/pci/pci-driver.c |  8 ++++----
 include/linux/suspend.h  | 26 ++++++++++++++++++++++++--
 kernel/power/suspend.c   |  3 +++
 3 files changed, 31 insertions(+), 6 deletions(-)

^ permalink raw reply

* Re: linux-next: Tree for Jun 28 (power/reset/reboot-mode)
From: Randy Dunlap @ 2019-06-28 23:46 UTC (permalink / raw)
  To: Stephen Rothwell, Linux Next Mailing List
  Cc: Linux Kernel Mailing List, Nandor Han, Andy Yan,
	Sebastian Reichel, Linux PM list
In-Reply-To: <20190628203840.1f74e739@canb.auug.org.au>

On 6/28/19 3:38 AM, Stephen Rothwell wrote:
> Hi all,
> 
> Changes since 20190627:
> 

on x86_64:

when CONFIG_OF is not set/enabled, but
CONFIG_NVMEM_REBOOT_MODE=m selects REBOOT_MODE:

  CC [M]  drivers/power/reset/reboot-mode.o
../drivers/power/reset/reboot-mode.c: In function ‘reboot_mode_register’:
../drivers/power/reset/reboot-mode.c:72:2: error: implicit declaration of function ‘for_each_property_of_node’ [-Werror=implicit-function-declaration]
  for_each_property_of_node(np, prop) {
  ^
../drivers/power/reset/reboot-mode.c:72:38: error: expected ‘;’ before ‘{’ token
  for_each_property_of_node(np, prop) {
                                      ^
../drivers/power/reset/reboot-mode.c:109:1: warning: label ‘error’ defined but not used [-Wunused-label]
 error:
 ^
../drivers/power/reset/reboot-mode.c:67:9: warning: unused variable ‘len’ [-Wunused-variable]
  size_t len = strlen(PREFIX);
         ^
../drivers/power/reset/reboot-mode.c: At top level:
../drivers/power/reset/reboot-mode.c:42:12: warning: ‘reboot_mode_notify’ defined but not used [-Wunused-function]
 static int reboot_mode_notify(struct notifier_block *this,
            ^


-- 
~Randy

^ permalink raw reply

* Re: [PATCH v1 0/3] Add required-opps support to devfreq passive gov
From: Saravana Kannan @ 2019-06-28 20:26 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Viresh Kumar,
	Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
	Android Kernel Team, Linux PM, LKML
In-Reply-To: <20190628064914.4nu6ql7f7h7o4iul@vireshk-i7>

On Thu, Jun 27, 2019 at 11:49 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 26-06-19, 11:10, Saravana Kannan wrote:
> > On Tue, Jun 25, 2019 at 11:32 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> > > So, when a CPU changes frequency, we must change the performance state
> > > of PM domain and change frequency/bw of the cache synchronously.
> >
> > I mean, it's going to be changed when we get the CPUfreq transition
> > notifiers. From a correctness point of view, setting it inside the OPP
> > framework is not any better than doing it when we get the notifiers.
>
> That's what the problem is. All maintainers now a days ask people to
> stay away from notifiers and we are making that base of another new
> thing we are starting.

In that case we can just add direct calls in cpufreq.c to let devfreq
know about the frequency changes. But then again, CPU is just one
example for this use case. I'm just using that because people are more
familiar with that.

> Over that, with many cpufreq drivers we have fast switching enabled
> and notifiers disabled. How will they make these things work ? We
> still want to scale L3 in those cases as well.

Nothing is preventing them from using the xlate OPP API I added to
figure out all the CPU to L3 frequency mapping and then set the L3
frequency directly from the CPUfreq driver.

Also, whether we use OPP framework or devfreq to set the L3 frequency,
it's going to block fast switching because both these frameworks have
APIs that can sleep.

But really, most mobile use cases don't want to permanently tie L3
freq to CPU freq. Having it go through devfreq and being able to
switch governors is a very important need for mobile products.

Keep in mind that nothing in this series does any of the cpufreq stuff
yet. That'll need a few more changes. I was just using CPUfreq as an
example.

> > I see this as "required for good performance". So I don't see it as
> > redefining required-opps. If someone wants good performance/power
> > balance they follow the "required-opps". Technically even the PM
> > pstates are required for good power. Otherwise, the system could leave
> > the voltage at max and stuff would still work.
> >
> > Also, the slave device might need to get input from multiple master
> > devices and aggregate the request before setting the slave device
> > frequency. So I don't think OPP  framework would be the right place to
> > deal with those things. For example, L3 might (will) have different
> > mappings for big vs little cores. So that needs to be aggregated and
> > set properly by the slave device driver. Also, GPU might have a
> > mapping for L3 too. In which case the L3 slave driver needs to take
> > input from even more masters before it decides its frequency. But most
> > importantly, we still need the ability to change governors for L3.
> > Again these are just examples with L3 and it can get more complicated
> > based on the situation.
> >
> > Most importantly, instead of always going by mapping, one might decide
> > to scale the L3 based on some other governor (that looks at some HW
> > counter). Or just set it to performance governor for a use case for
> > which performance is more important. All of this comes for free with
> > devfreq and if we always set it from OPP framework we don't give this
> > required control to userspace.
> >
> > I think going through devfreq is the right approach for this. And we
> > can always rewrite the software if we find problems in the future. But
> > as it stands today, this will help cases like exynos without the need
> > for a lot of changes. Hope I've convinced you.
>
> I understand the aggregation thing and fully support that the
> aggregation can't happen in OPP core and must be done somewhere else.
> But the input can go from OPP core while the frequency is changing,
> isn't it ?

I'm not opposed to OPP sending input to devfreq to let it know that a
master device frequency change is happening. But I think this is kinda
orthogonal to this patch series.

Today the passive governor looks at the master device's devfreq
frequency changes to trigger the frequency change of the slave
devfreq. It neither supports tracking OPP frequency change nor CPUfreq
frequency change. If that's something we want to add, we can look into
that separately as passive governor (or a new governor) changes.

But then not all devices (CPUfreq or otherwise) use OPP to set the
frequencies. So it's beneficial to have all of these frameworks as
inputs for devfreq passive (like) governor. CPUfreq is actually a bit
more tricky because we'll also have to track hotplug, etc. So direct
calls from CPUfreq to devfreq (similar to cpufreq stats tracking)
would be good.

-Saravana

^ permalink raw reply

* Re: [RFCv3 0/8] TurboSched: A scheduler for sustaining Turbo Frequencies for longer durations
From: Parth Shah @ 2019-06-28 16:42 UTC (permalink / raw)
  To: Patrick Bellasi; +Cc: linux-kernel, linux-pm, mingo, peterz, dietmar.eggemann
In-Reply-To: <20190628131430.qmbfocgujeyi3dbt@e110439-lin>

Hi Patrick,

Thank you for taking interest at the patch set.


On 6/28/19 6:44 PM, Patrick Bellasi wrote:
> On 25-Jun 10:07, Parth Shah wrote:
> 
> [...]
> 
>> Implementation
>> ==============
>>
>> These patches uses UCLAMP mechanism[2] used to clamp utilization from the
>> userspace, which can be used to classify the jitter tasks. The task wakeup
>> logic uses this information to pack such tasks onto cores which are already
>> running busy with CPU intensive tasks. The task packing is done at
>> `select_task_rq_fair` only so that in case of wrong decision load balancer
>> may pull the classified jitter tasks for maximizing performance.
>>
>> Any tasks clamped with cpu.util.max=1 (with sched_setattr syscall) are
>> classified as jitter tasks.
> 
> I don't like this approach, it's overloading the meaning of clamps and
> it also brings in un-wanted side effects, like running jitter tasks at
> the minimum OPP.
> 
> Do you have any expected minimum frequency for those jitter tasks ?
> I expect those to be relatively small tasks but still perhaps it makes
> sense to run them on higher then minimal OPP.
>

I absolutely agree with you as it may overload the meaning of clamps.
AFAIK, the only way to detect jitters is by looking at its utilization,
where low util tasks are possibly jitters unless they are important tasks. If
userspace tells if the task is clamped to least OPP, then it is an indication of
low utilization or unimportant tasks, which we say a jitter.

Also, as we discussed in OSPM as well, if all the jitters are given a dedicated
core by the scheduler, then UCLAMP ensures least OPP for such tasks which can help
saving power a further bit, which can be channeled to busier core thus allowing
them to sustain or boost turbo frequencies.

I agree that it may have side-effects but I'm just putting idea out here.
Also, I understand that task packing and frequency are not co-related but for
this specific purpose of Turbo sustaining problem, jitters should be given least
power so that others can have extra one, hence jitters should be given less
frequency.

> Why not just adding a new dedicated per-task scheduling attribute,
> e.g. SCHED_FLAG_LATENCY_TOLERANT, and manage it via
> sched_{set,get}attr() ?
> 
> I guess such a concept could work well on defining a generic
> spread-vs-pack wakeup policy which is something Android also could
> benefit from.
> 

I have made attempts to use per-task attributes for task classification in first
series of TurboSched and it works fine.
https://lwn.net/ml/linux-pm/20190322060621.27021-3-parth015@linux.vnet.ibm.com/

Then from inputs from Dietmar, I thought of giving a try to UCLAMP for this purpose.
But, now I guess having one more task attribute is useful as it can serve multiple
purpose including android and task packing. I will add it v4 then.

> However, what we will still be missing is a proper cgroups support.
> Not always is possible and/or convenient to explicitly set per-task
> attributes. But at the same time, AFAIK using cgroups to define
> task properties which do not represent a "resource repartition" is
> something very difficult to get accepted mainline.
> 

Yeah, I faced that problem in v2.
https://lkml.org/lkml/2019/5/15/1395

> In the past, back in 2011, there was an attempt to introduce a timer
> slack controller, but apparently it was not very well received:
> 
>    Message-ID: <1300111524-5666-1-git-send-email-kirill@shutemov.name>
>    https://lore.kernel.org/lkml/20110314164652.5b44fb9e.akpm@linux-foundation.org/
> 
> But perhaps now the times are more mature and we can try to come up
> with compelling cases from both the server and the mobile world.
> 

The pointed patch series seems appealing and I will have a look at it. 

>> We define a core to be non-idle if it is over 12.5% utilized of its
>> capacity;
> 
> This looks like a random number, can you elaborate on that?

It is an experimental value to define whether a "core" should be considered to be
idle or not. This is because, even-though core is running few bunch of tasks summing
upto around 10% of utilization in a core, it maybe going to shallower idle-states
periodically which is kind of power-saving; placing new tasks on such core should
be avoided as far as possible.

I have just tested this on SMT-4/8 systems and it works as expected but at the end it
is still an experimental value.

> 
>> the jitters are packed over these cores using First-fit
>> approach.
>>
>> To demonstrate/benchmark, one can use a synthetic workload generator
>> `turbo_bench.c`[1] available at
>> https://github.com/parthsl/tools/blob/master/benchmarks/turbo_bench.c
>>
>> Following snippet demonstrates the use of TurboSched feature:
>> ```
>> i=8; ./turbo_bench -t 30 -h $i -n $((i*2)) -j
>> ```
>>
>> Current implementation uses only jitter classified tasks to be packed on
>> the first busy cores, but can be further optimized by getting userspace
>> input of important tasks and keeping track of such tasks.
>> This leads to optimized searching of non idle cores and also more
>> accurate as userspace hints are safer than auto classified busy
>> cores/tasks.
> 
> Hints from user-space looks like an interesting concept, could you
> better elaborate what you are thinking about in this sense?
> 

Currently, we are just tagging tasks as jitters and packing it on already busier
cores (>12.5% core utilization). Packing strategy is a simple first-fit algorithm
looking for first core in a DIE where the waking-up jitter task can be accommodated.
This is a lot of work in fast-path but can be optimized out. If user can also tag
CPU intensive and/or important tasks then we can keep track of the cores occupying
such tasks which can be used for task packing reducing the effort of finding non-idle.
Again, this can be set with UCLAMP by cpu.util-min=SCHED_CAPACITY_SCALE.

Infact, v1 does this but then I thought of breaking down problem into steps and this
optimization can be introduced later.
https://lwn.net/ml/linux-pm/20190322060621.27021-6-parth015@linux.vnet.ibm.com/

So we can have some task attributes like task_type or similar which hints scheduler on
several features like packing, spreading, or giving dedicated core where siblings will
not be scheduled or even core scheduling, which in certain ways affect scheduling
decisions.


Thanks
Parth


^ permalink raw reply

* Re: [PATCH v2] PM / wakeup: show wakeup sources stats in sysfs
From: Greg KH @ 2019-06-28 15:10 UTC (permalink / raw)
  To: Tri Vo
  Cc: rjw, viresh.kumar, rafael, hridya, sspatil, kaleshsingh,
	linux-kernel, linux-pm, kernel-team
In-Reply-To: <20190627225335.72107-1-trong@android.com>

On Thu, Jun 27, 2019 at 03:53:35PM -0700, Tri Vo wrote:
> Userspace can use wakeup_sources debugfs node to plot history of suspend
> blocking wakeup sources over device's boot cycle. This information can
> then be used (1) for power-specific bug reporting and (2) towards
> attributing battery consumption to specific processes over a period of
> time.
> 
> However, debugfs doesn't have stable ABI. For this reason, expose wakeup
> sources statistics in sysfs under /sys/power/wakeup_sources/<name>/
> 
> Embedding a struct kobject into struct wakeup_source changes lifetime
> requirements on the latter. To that end, change deallocation of struct
> wakeup_source using kfree to kobject_put().
> 
> Change struct wakelock's wakeup_source member to a pointer to decouple
> lifetimes of struct wakelock and struct wakeup_source for above reason.
> 
> Introduce CONFIG_PM_SLEEP_STATS that enables/disables showing wakeup
> source statistics in sysfs.
> 
> Signed-off-by: Tri Vo <trong@android.com>

Ok, this looks much better, but I don't like the use of a "raw" kobject
here.  It is much simpler, and less code, to use 'struct device'
instead.

As proof, I reworked the patch to do just that, and it saves over 50
lines of .c code, which is always nice :)

Attached below is the reworked code, along with the updated
documentation file.  It creates devices in a virtual class, and you can
easily iterate over them all by looking in /sys/class/wakeup/.

Note, I'm note quite sure you need all of the changes you made in
kernel/power/wakelock.c when you make the structure contain a pointer to
the wakeup source and not the structure itself, but I just went with it
and got it all to build properly.

Also note, I've not actually tested this at all, only built it, so I
_strongly_ suggest that you test this to make sure it really works :)

What do you think?

thanks,

greg k-h

------------------



---
 Documentation/ABI/testing/sysfs-power |  73 ++++++++++++-
 drivers/base/power/Makefile           |   1 +
 drivers/base/power/wakeup.c           |  11 +-
 drivers/base/power/wakeup_stats.c     | 144 ++++++++++++++++++++++++++
 include/linux/pm_wakeup.h             |  19 ++++
 kernel/power/Kconfig                  |  10 ++
 kernel/power/wakelock.c               |  40 ++++---
 7 files changed, 280 insertions(+), 18 deletions(-)
 create mode 100644 drivers/base/power/wakeup_stats.c

diff --git a/Documentation/ABI/testing/sysfs-power b/Documentation/ABI/testing/sysfs-power
index 18b7dc929234..ef92628e6fc3 100644
--- a/Documentation/ABI/testing/sysfs-power
+++ b/Documentation/ABI/testing/sysfs-power
@@ -300,4 +300,75 @@ Description:
 		attempt.
 
 		Using this sysfs file will override any values that were
-		set using the kernel command line for disk offset.
\ No newline at end of file
+		set using the kernel command line for disk offset.
+
+What:		/sys/class/wakeup/
+Date:		June 2019
+Contact:	Tri Vo <trong@android.com>
+Description:
+		The /sys/class/wakeup/ directory contains pointers to all
+		wakeup sources in the kernel at that moment in time.
+
+What:		/sys/class/wakeup/.../active_count
+Date:		June 2019
+Contact:	Tri Vo <trong@android.com>
+Description:
+		This file contains the number of times the wakeup source was
+		activated.
+
+What:		/sys/class/wakeup/.../event_count
+Date:		June 2019
+Contact:	Tri Vo <trong@android.com>
+Description:
+		This file contains the number of signaled wakeup events
+		associated with the wakeup source.
+
+What:		/sys/class/wakeup/.../wakeup_count
+Date:		June 2019
+Contact:	Tri Vo <trong@android.com>
+Description:
+		This file contains the number of times the wakeup source might
+		abort suspend.
+
+What:		/sys/class/wakeup/.../expire_count
+Date:		June 2019
+Contact:	Tri Vo <trong@android.com>
+Description:
+		This file contains the number of times the wakeup source's
+		timeout has expired.
+
+What:		/sys/class/wakeup/.../active_time_ms
+Date:		June 2019
+Contact:	Tri Vo <trong@android.com>
+Description:
+		This file contains the amount of time the wakeup source has
+		been continuously active, in milliseconds.  If the wakeup
+		source is not active, this file contains '0'.
+
+What:		/sys/class/wakeup/.../total_time_ms
+Date:		June 2019
+Contact:	Tri Vo <trong@android.com>
+Description:
+		This file contains the total amount of time this wakeup source
+		has been active, in milliseconds.
+
+What:		/sys/class/wakeup/.../max_time_ms
+Date:		June 2019
+Contact:	Tri Vo <trong@android.com>
+Description:
+		This file contains the maximum amount of time this wakeup
+		source has been continuously active, in milliseconds.
+
+What:		/sys/class/wakeup/.../last_change_ms
+Date:		June 2019
+Contact:	Tri Vo <trong@android.com>
+Description:
+		This file contains the monotonic clock time when the wakeup
+		source was touched last time, in milliseconds.
+
+What:		/sys/class/wakeup/.../prevent_suspend_time_ms
+Date:		June 2019
+Contact:	Tri Vo <trong@android.com>
+Description:
+		The file contains the total amount of time this wakeup source
+		has been preventing autosleep, in milliseconds.
diff --git a/drivers/base/power/Makefile b/drivers/base/power/Makefile
index e1bb691cf8f1..1963f53d9982 100644
--- a/drivers/base/power/Makefile
+++ b/drivers/base/power/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_PM)	+= sysfs.o generic_ops.o common.o qos.o runtime.o wakeirq.o
 obj-$(CONFIG_PM_SLEEP)	+= main.o wakeup.o
+obj-$(CONFIG_PM_SLEEP_STATS)	+= wakeup_stats.o
 obj-$(CONFIG_PM_TRACE_RTC)	+= trace.o
 obj-$(CONFIG_PM_GENERIC_DOMAINS)	+=  domain.o domain_governor.o
 obj-$(CONFIG_HAVE_CLK)	+= clock_ops.o
diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index 5b2b6a05a4f3..a5e5ffaa4532 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -147,8 +147,7 @@ void wakeup_source_destroy(struct wakeup_source *ws)
 
 	__pm_relax(ws);
 	wakeup_source_record(ws);
-	kfree_const(ws->name);
-	kfree(ws);
+	wakeup_source_sysfs_remove(ws);
 }
 EXPORT_SYMBOL_GPL(wakeup_source_destroy);
 
@@ -205,11 +204,15 @@ EXPORT_SYMBOL_GPL(wakeup_source_remove);
 struct wakeup_source *wakeup_source_register(const char *name)
 {
 	struct wakeup_source *ws;
+	int ret;
 
 	ws = wakeup_source_create(name);
-	if (ws)
+	if (ws) {
+		ret = wakeup_source_sysfs_add(ws);
+		if (ret)
+			return NULL;
 		wakeup_source_add(ws);
-
+	}
 	return ws;
 }
 EXPORT_SYMBOL_GPL(wakeup_source_register);
diff --git a/drivers/base/power/wakeup_stats.c b/drivers/base/power/wakeup_stats.c
new file mode 100644
index 000000000000..e0c4767304be
--- /dev/null
+++ b/drivers/base/power/wakeup_stats.c
@@ -0,0 +1,144 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Wakeup statistics in sysfs
+ *
+ * Copyright (c) 2019 Linux Foundation <gregkh@linuxfoundation.org>
+ * Copyright (c) 2019 Google Inc.
+ */
+
+#include <linux/slab.h>
+#include <linux/kdev_t.h>
+#include <linux/device.h>
+
+#include "power.h"
+
+static struct class *wakeup_class;
+
+#define wakeup_attr(_name)						\
+static ssize_t _name##_show(struct device *dev,				\
+			    struct device_attribute *attr, char *buf)	\
+{									\
+	struct wakeup_source *ws = dev_get_drvdata(dev);		\
+									\
+	return sprintf(buf, "%lu\n", ws->_name);				\
+}									\
+static DEVICE_ATTR_RO(_name)
+
+wakeup_attr(active_count);
+wakeup_attr(event_count);
+wakeup_attr(wakeup_count);
+wakeup_attr(expire_count);
+
+static ssize_t active_time_ms_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct wakeup_source *ws = dev_get_drvdata(dev);
+	ktime_t active_time =
+		ws->active ? ktime_sub(ktime_get(), ws->last_time) : 0;
+
+	return sprintf(buf, "%lld\n", ktime_to_ms(active_time));
+}
+static DEVICE_ATTR_RO(active_time_ms);
+
+static ssize_t total_time_ms_show(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	struct wakeup_source *ws = dev_get_drvdata(dev);
+	ktime_t active_time;
+	ktime_t total_time = ws->total_time;
+
+	if (ws->active) {
+		active_time = ktime_sub(ktime_get(), ws->last_time);
+		total_time = ktime_add(total_time, active_time);
+	}
+	return sprintf(buf, "%lld\n", ktime_to_ms(total_time));
+}
+static DEVICE_ATTR_RO(total_time_ms);
+
+static ssize_t max_time_ms_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct wakeup_source *ws = dev_get_drvdata(dev);
+	ktime_t active_time;
+	ktime_t max_time = ws->max_time;
+
+	if (ws->active) {
+		active_time = ktime_sub(ktime_get(), ws->last_time);
+		if (active_time > max_time)
+			max_time = active_time;
+	}
+	return sprintf(buf, "%lld\n", ktime_to_ms(max_time));
+}
+static DEVICE_ATTR_RO(max_time_ms);
+
+static ssize_t last_change_ms_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct wakeup_source *ws = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%lld\n", ktime_to_ms(ws->last_time));
+}
+static DEVICE_ATTR_RO(last_change_ms);
+
+static ssize_t prevent_suspend_time_ms_show(struct device *dev,
+					    struct device_attribute *attr,
+					    char *buf)
+{
+	struct wakeup_source *ws = dev_get_drvdata(dev);
+	ktime_t prevent_sleep_time = ws->prevent_sleep_time;
+
+	if (ws->active && ws->autosleep_enabled) {
+		prevent_sleep_time = ktime_add(prevent_sleep_time,
+			ktime_sub(ktime_get(), ws->start_prevent_time));
+	}
+	return sprintf(buf, "%lld\n", ktime_to_ms(prevent_sleep_time));
+}
+static DEVICE_ATTR_RO(prevent_suspend_time_ms);
+
+static struct attribute *wakeup_source_attrs[] = {
+	&dev_attr_active_count.attr,
+	&dev_attr_event_count.attr,
+	&dev_attr_wakeup_count.attr,
+	&dev_attr_expire_count.attr,
+	&dev_attr_active_time_ms.attr,
+	&dev_attr_total_time_ms.attr,
+	&dev_attr_max_time_ms.attr,
+	&dev_attr_last_change_ms.attr,
+	&dev_attr_prevent_suspend_time_ms.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(wakeup_source);
+
+/**
+ * wakeup_source_sysfs_add - Add wakeup_source attributes to sysfs.
+ * @ws: Wakeup source to be exposed in sysfs.
+ */
+int wakeup_source_sysfs_add(struct wakeup_source *ws)
+{
+	struct device *dev;
+
+	dev = device_create_with_groups(wakeup_class, NULL, MKDEV(0, 0), ws,
+					wakeup_source_groups, "%s", ws->name);
+	if (IS_ERR(dev))
+		return PTR_ERR(dev);
+	ws->dev = dev;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(wakeup_source_sysfs_add);
+
+void wakeup_source_sysfs_remove(struct wakeup_source *ws)
+{
+	put_device(ws->dev);
+	device_unregister(ws->dev);
+}
+
+static int __init wakeup_sources_sysfs_init(void)
+{
+	wakeup_class = class_create(THIS_MODULE, "wakeup");
+	if (IS_ERR(wakeup_class))
+		return PTR_ERR(wakeup_class);
+
+	return 0;
+}
+
+postcore_initcall(wakeup_sources_sysfs_init);
diff --git a/include/linux/pm_wakeup.h b/include/linux/pm_wakeup.h
index ce57771fab9b..18ebc4ce817c 100644
--- a/include/linux/pm_wakeup.h
+++ b/include/linux/pm_wakeup.h
@@ -35,6 +35,7 @@ struct wake_irq;
  * @relax_count: Number of times the wakeup source was deactivated.
  * @expire_count: Number of times the wakeup source's timeout has expired.
  * @wakeup_count: Number of times the wakeup source might abort suspend.
+ * @dev: struct device for sysfs statistics about the wakeup source
  * @active: Status of the wakeup source.
  * @has_timeout: The wakeup source has been activated with a timeout.
  */
@@ -55,6 +56,7 @@ struct wakeup_source {
 	unsigned long		relax_count;
 	unsigned long		expire_count;
 	unsigned long		wakeup_count;
+	struct device		*dev;
 	bool			active:1;
 	bool			autosleep_enabled:1;
 };
@@ -181,6 +183,23 @@ static inline void pm_wakeup_dev_event(struct device *dev, unsigned int msec,
 
 #endif /* !CONFIG_PM_SLEEP */
 
+#ifdef CONFIG_PM_SLEEP_STATS
+
+/* drivers/base/power/wakeup_stats.c */
+int wakeup_source_sysfs_add(struct wakeup_source *ws);
+void wakeup_source_sysfs_remove(struct wakeup_source *ws);
+
+#else /* !CONFIG_PM_SLEEP_STATS */
+
+static inline int wakeup_source_sysfs_add(struct wakeup_source *ws)
+{
+	return 0;
+}
+static inline void wakeup_source_sysfs_remove(struct wakeup_source *ws)
+{ }
+
+#endif /* !CONFIG_PM_SLEEP_STATS */
+
 static inline void wakeup_source_init(struct wakeup_source *ws,
 				      const char *name)
 {
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index ff8592ddedee..4d258f34020b 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -195,6 +195,16 @@ config PM_SLEEP_DEBUG
 	def_bool y
 	depends on PM_DEBUG && PM_SLEEP
 
+config PM_SLEEP_STATS
+	bool "Wakeup sources statistics"
+	depends on PM_SLEEP
+	help
+	  Expose wakeup sources statistics to user space via sysfs. Collected
+	  statistics are located under /sys/power/wakeup_sources. For more
+	  information, read <file:Documentation/ABI/testing/sysfs-power>.
+
+	  If in doubt, say N.
+
 config DPM_WATCHDOG
 	bool "Device suspend/resume watchdog"
 	depends on PM_DEBUG && PSTORE && EXPERT
diff --git a/kernel/power/wakelock.c b/kernel/power/wakelock.c
index 4210152e56f0..dce02c2f4496 100644
--- a/kernel/power/wakelock.c
+++ b/kernel/power/wakelock.c
@@ -27,7 +27,7 @@ static DEFINE_MUTEX(wakelocks_lock);
 struct wakelock {
 	char			*name;
 	struct rb_node		node;
-	struct wakeup_source	ws;
+	struct wakeup_source	*ws;
 #ifdef CONFIG_PM_WAKELOCKS_GC
 	struct list_head	lru;
 #endif
@@ -46,7 +46,7 @@ ssize_t pm_show_wakelocks(char *buf, bool show_active)
 
 	for (node = rb_first(&wakelocks_tree); node; node = rb_next(node)) {
 		wl = rb_entry(node, struct wakelock, node);
-		if (wl->ws.active == show_active)
+		if (wl->ws->active == show_active)
 			str += scnprintf(str, end - str, "%s ", wl->name);
 	}
 	if (str > buf)
@@ -112,16 +112,16 @@ static void __wakelocks_gc(struct work_struct *work)
 		u64 idle_time_ns;
 		bool active;
 
-		spin_lock_irq(&wl->ws.lock);
-		idle_time_ns = ktime_to_ns(ktime_sub(now, wl->ws.last_time));
-		active = wl->ws.active;
-		spin_unlock_irq(&wl->ws.lock);
+		spin_lock_irq(&wl->ws->lock);
+		idle_time_ns = ktime_to_ns(ktime_sub(now, wl->ws->last_time));
+		active = wl->ws->active;
+		spin_unlock_irq(&wl->ws->lock);
 
 		if (idle_time_ns < ((u64)WL_GC_TIME_SEC * NSEC_PER_SEC))
 			break;
 
 		if (!active) {
-			wakeup_source_remove(&wl->ws);
+			wakeup_source_remove(wl->ws);
 			rb_erase(&wl->node, &wakelocks_tree);
 			list_del(&wl->lru);
 			kfree(wl->name);
@@ -153,6 +153,7 @@ static struct wakelock *wakelock_lookup_add(const char *name, size_t len,
 	struct rb_node **node = &wakelocks_tree.rb_node;
 	struct rb_node *parent = *node;
 	struct wakelock *wl;
+	int ret;
 
 	while (*node) {
 		int diff;
@@ -187,9 +188,22 @@ static struct wakelock *wakelock_lookup_add(const char *name, size_t len,
 		kfree(wl);
 		return ERR_PTR(-ENOMEM);
 	}
-	wl->ws.name = wl->name;
-	wl->ws.last_time = ktime_get();
-	wakeup_source_add(&wl->ws);
+
+	wl->ws = wakeup_source_create(wl->name);
+	if (!wl->ws) {
+		kfree(wl);
+		return ERR_PTR(-ENOMEM);
+	}
+	wl->ws->last_time = ktime_get();
+
+	ret = wakeup_source_sysfs_add(wl->ws);
+	if (ret) {
+		kfree(wl->name);
+		kfree(wl);
+		return ERR_PTR(ret);
+	}
+
+	wakeup_source_add(wl->ws);
 	rb_link_node(&wl->node, parent, node);
 	rb_insert_color(&wl->node, &wakelocks_tree);
 	wakelocks_lru_add(wl);
@@ -233,9 +247,9 @@ int pm_wake_lock(const char *buf)
 		u64 timeout_ms = timeout_ns + NSEC_PER_MSEC - 1;
 
 		do_div(timeout_ms, NSEC_PER_MSEC);
-		__pm_wakeup_event(&wl->ws, timeout_ms);
+		__pm_wakeup_event(wl->ws, timeout_ms);
 	} else {
-		__pm_stay_awake(&wl->ws);
+		__pm_stay_awake(wl->ws);
 	}
 
 	wakelocks_lru_most_recent(wl);
@@ -271,7 +285,7 @@ int pm_wake_unlock(const char *buf)
 		ret = PTR_ERR(wl);
 		goto out;
 	}
-	__pm_relax(&wl->ws);
+	__pm_relax(wl->ws);
 
 	wakelocks_lru_most_recent(wl);
 	wakelocks_gc();
-- 
2.22.0

^ permalink raw reply related

* Re: [RFC PATCH v2 4/5] sched/cpufreq: Introduce sugov_cpu_ramp_boost
From: Patrick Bellasi @ 2019-06-28 15:08 UTC (permalink / raw)
  To: Douglas RAILLARD
  Cc: linux-kernel, linux-pm, mingo, peterz, rjw, viresh.kumar,
	quentin.perret, dietmar.eggemann
In-Reply-To: <20190627171603.14767-5-douglas.raillard@arm.com>

Hi Douglas,

On 27-Jun 18:16, Douglas RAILLARD wrote:
> Use the utilization signals dynamic to detect when the utilization of a
> set of tasks starts increasing because of a change in tasks' behavior.
> This allows detecting when spending extra power for faster frequency
> ramp up response would be beneficial to the reactivity of the system.
> 
> This ramp boost is computed as the difference
> util_avg-util_est_enqueued. This number somehow represents a lower bound
> of how much extra utilization this tasks is actually using, compared to
> our best current stable knowledge of it (which is util_est_enqueued).

Maybe it's worth to call out here that at rq-level we don't have an
EWMA. However, the enqueued estimated utilization is derived by
considering the _task_util_est() which factors in the moving average
of tasks and thus makes the signal more stable even in case of tasks
switching between big and small activations.

> When the set of runnable tasks changes, the boost is disabled as the
> impact of blocked utilization on util_avg will make the delta with
> util_est_enqueued not very informative.
> 
> Signed-off-by: Douglas RAILLARD <douglas.raillard@arm.com>
> ---
>  kernel/sched/cpufreq_schedutil.c | 42 ++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
> index 7ffc6fe3b670..3eabfd815195 100644
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -60,6 +60,9 @@ struct sugov_cpu {
>  	unsigned long		bw_dl;
>  	unsigned long		max;
>  
> +	unsigned long		ramp_boost;
> +	unsigned long		util_est_enqueued;
> +
>  	/* The field below is for single-CPU policies only: */
>  #ifdef CONFIG_NO_HZ_COMMON
>  	unsigned long		saved_idle_calls;
> @@ -174,6 +177,41 @@ static void sugov_deferred_update(struct sugov_policy *sg_policy, u64 time,
>  	}
>  }
>  
> +static unsigned long sugov_cpu_ramp_boost(struct sugov_cpu *sg_cpu)
> +{
> +	return READ_ONCE(sg_cpu->ramp_boost);
> +}
> +
> +static unsigned long sugov_cpu_ramp_boost_update(struct sugov_cpu *sg_cpu,
> +						 unsigned long util)
> +{
> +	struct rq *rq = cpu_rq(sg_cpu->cpu);

Since you don't really need the rq below, maybe better:

        struct sched_avg *sa = &cpu_rq(sg_cpu->cpu)->cfs.avg;

?

> +	unsigned long util_est_enqueued;
> +	unsigned long util_avg;
> +	unsigned long boost = 0;
> +
> +	util_est_enqueued = READ_ONCE(rq->cfs.avg.util_est.enqueued);
> +	util_avg = READ_ONCE(rq->cfs.avg.util_avg);
> +
> +	/*
> +	 * Boost when util_avg becomes higher than the previous stable
> +	 * knowledge of the enqueued tasks' set util, which is CPU's
> +	 * util_est_enqueued.
> +	 *
> +	 * We try to spot changes in the workload itself, so we want to
> +	 * avoid the noise of tasks being enqueued/dequeued. To do that,
> +	 * we only trigger boosting when the "amount of work' enqueued
> +	 * is stable.
> +	 */
> +	if (util_est_enqueued == sg_cpu->util_est_enqueued
> +	    && util_avg > util_est_enqueued)
> +		 boost = util_avg - util_est_enqueued;

The above should be:


 	if (util_est_enqueued == sg_cpu->util_est_enqueue &&
            util_avg > util_est_enqueued) {
 		 boost = util_avg - util_est_enqueued;
        }

but perhaps you can also go for a fast bailout with something like:

        if (util_avg <= util_est_enqueued)
                return 0;
        if (util_est_enqueued == sg_cpu->util_est_enqueue)
                boost = util_avg - util_est_enqueued;


Moreover: could it make sense to add a threshold on a minimal boost
value to return non zero?

> +
> +	sg_cpu->util_est_enqueued = util_est_enqueued;
> +	WRITE_ONCE(sg_cpu->ramp_boost, boost);
> +	return boost;

You don't seem to use this returned value: should be void?

> +}
> +
>  /**
>   * get_next_freq - Compute a new frequency for a given cpufreq policy.
>   * @sg_policy: schedutil policy object to compute the new frequency for.
> @@ -504,6 +542,7 @@ static void sugov_update_single(struct update_util_data *hook, u64 time,
>  	busy = sugov_cpu_is_busy(sg_cpu);
>  
>  	util = sugov_get_util(sg_cpu);
> +	sugov_cpu_ramp_boost_update(sg_cpu, util);
>  	max = sg_cpu->max;
>  	util = sugov_iowait_apply(sg_cpu, time, util, max);
>  	next_f = get_next_freq(sg_policy, util, max);
> @@ -544,6 +583,8 @@ static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu, u64 time)
>  		unsigned long j_util, j_max;
>  
>  		j_util = sugov_get_util(j_sg_cpu);
> +		if (j_sg_cpu == sg_cpu)
> +			sugov_cpu_ramp_boost_update(sg_cpu, j_util);
>  		j_max = j_sg_cpu->max;
>  		j_util = sugov_iowait_apply(j_sg_cpu, time, j_util, j_max);
>  
> @@ -553,6 +594,7 @@ static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu, u64 time)
>  		}
>  	}
>  
> +
>  	return get_next_freq(sg_policy, util, max);
>  }


Best,
Patrick

-- 
#include <best/regards.h>

Patrick Bellasi

^ permalink raw reply

* Re: [PATCH 12/39] docs: interconnect.rst: add it to the driver-api guide
From: Georgi Djakov @ 2019-06-28 13:55 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Linux Doc Mailing List
  Cc: Mauro Carvalho Chehab, linux-kernel, Jonathan Corbet, linux-pm
In-Reply-To: <feb5e217face2ff3f2ccd5c3726192bf1250dd0d.1561724493.git.mchehab+samsung@kernel.org>

On 6/28/19 15:30, Mauro Carvalho Chehab wrote:
> This is intented for Kernel hackers audience.

s/intented/intended/

> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>

Reviewed-by: Georgi Djakov <georgi.djakov@linaro.org>

Thanks!
Georgi

> ---
>  Documentation/driver-api/index.rst                          | 1 +
>  Documentation/{interconnect => driver-api}/interconnect.rst | 2 --
>  MAINTAINERS                                                 | 2 +-
>  3 files changed, 2 insertions(+), 3 deletions(-)
>  rename Documentation/{interconnect => driver-api}/interconnect.rst (99%)
> 
> diff --git a/Documentation/driver-api/index.rst b/Documentation/driver-api/index.rst
> index 2a0b57f12d1a..97bab578ea72 100644
> --- a/Documentation/driver-api/index.rst
> +++ b/Documentation/driver-api/index.rst
> @@ -35,6 +35,7 @@ available subsections can be seen below.
>     spi
>     i2c
>     i3c/index
> +   interconnect
>     hsi
>     edac
>     scsi
> diff --git a/Documentation/interconnect/interconnect.rst b/Documentation/driver-api/interconnect.rst
> similarity index 99%
> rename from Documentation/interconnect/interconnect.rst
> rename to Documentation/driver-api/interconnect.rst
> index 56e331dab70e..c3e004893796 100644
> --- a/Documentation/interconnect/interconnect.rst
> +++ b/Documentation/driver-api/interconnect.rst
> @@ -1,7 +1,5 @@
>  .. SPDX-License-Identifier: GPL-2.0
>  
> -:orphan:
> -
>  =====================================
>  GENERIC SYSTEM INTERCONNECT SUBSYSTEM
>  =====================================
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 55996c9f2e0a..524e2c4300dc 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8273,7 +8273,7 @@ INTERCONNECT API
>  M:	Georgi Djakov <georgi.djakov@linaro.org>
>  L:	linux-pm@vger.kernel.org
>  S:	Maintained
> -F:	Documentation/interconnect/
> +F:	Documentation/driver-api/interconnect.rst
>  F:	Documentation/devicetree/bindings/interconnect/
>  F:	drivers/interconnect/
>  F:	include/dt-bindings/interconnect/
> 

^ permalink raw reply

* Re: [RFCv3 0/8] TurboSched: A scheduler for sustaining Turbo Frequencies for longer durations
From: Patrick Bellasi @ 2019-06-28 13:14 UTC (permalink / raw)
  To: Parth Shah; +Cc: linux-kernel, linux-pm, mingo, peterz, dietmar.eggemann
In-Reply-To: <20190625043726.21490-1-parth@linux.ibm.com>

On 25-Jun 10:07, Parth Shah wrote:

[...]

> Implementation
> ==============
> 
> These patches uses UCLAMP mechanism[2] used to clamp utilization from the
> userspace, which can be used to classify the jitter tasks. The task wakeup
> logic uses this information to pack such tasks onto cores which are already
> running busy with CPU intensive tasks. The task packing is done at
> `select_task_rq_fair` only so that in case of wrong decision load balancer
> may pull the classified jitter tasks for maximizing performance.
> 
> Any tasks clamped with cpu.util.max=1 (with sched_setattr syscall) are
> classified as jitter tasks.

I don't like this approach, it's overloading the meaning of clamps and
it also brings in un-wanted side effects, like running jitter tasks at
the minimum OPP.

Do you have any expected minimum frequency for those jitter tasks ?
I expect those to be relatively small tasks but still perhaps it makes
sense to run them on higher then minimal OPP.

Why not just adding a new dedicated per-task scheduling attribute,
e.g. SCHED_FLAG_LATENCY_TOLERANT, and manage it via
sched_{set,get}attr() ?

I guess such a concept could work well on defining a generic
spread-vs-pack wakeup policy which is something Android also could
benefit from.

However, what we will still be missing is a proper cgroups support.
Not always is possible and/or convenient to explicitly set per-task
attributes. But at the same time, AFAIK using cgroups to define
task properties which do not represent a "resource repartition" is
something very difficult to get accepted mainline.

In the past, back in 2011, there was an attempt to introduce a timer
slack controller, but apparently it was not very well received:

   Message-ID: <1300111524-5666-1-git-send-email-kirill@shutemov.name>
   https://lore.kernel.org/lkml/20110314164652.5b44fb9e.akpm@linux-foundation.org/

But perhaps now the times are more mature and we can try to come up
with compelling cases from both the server and the mobile world.

> We define a core to be non-idle if it is over 12.5% utilized of its
> capacity;

This looks like a random number, can you elaborate on that?

> the jitters are packed over these cores using First-fit
> approach.
> 
> To demonstrate/benchmark, one can use a synthetic workload generator
> `turbo_bench.c`[1] available at
> https://github.com/parthsl/tools/blob/master/benchmarks/turbo_bench.c
> 
> Following snippet demonstrates the use of TurboSched feature:
> ```
> i=8; ./turbo_bench -t 30 -h $i -n $((i*2)) -j
> ```
> 
> Current implementation uses only jitter classified tasks to be packed on
> the first busy cores, but can be further optimized by getting userspace
> input of important tasks and keeping track of such tasks.
> This leads to optimized searching of non idle cores and also more
> accurate as userspace hints are safer than auto classified busy
> cores/tasks.

Hints from user-space looks like an interesting concept, could you
better elaborate what you are thinking about in this sense?

-- 
#include <best/regards.h>

Patrick Bellasi

^ permalink raw reply

* Re: [PATCH 37/39] docs: adds some directories to the main documentation index
From: Bartlomiej Zolnierkiewicz @ 2019-06-28 12:55 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
	Jonathan Corbet, Jens Axboe, Akinobu Mita, Moritz Fischer,
	David S. Miller, Masahiro Yamada, Michal Marek, Josh Poimboeuf,
	Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence,
	Paul Moore, Dominik Brodowski, Rafael J. Wysocki, Len Brown,
	Pavel Machek, Martin K. Petersen, Thomas Gleixner,
	Wim Van Sebroeck, Guenter Roeck, dri-devel, linux-fbdev,
	linux-fpga, linux-ide, linux-kbuild, live-patching, netdev,
	linux-security-module, linux-pm, linux-scsi, target-devel,
	linux-watchdog
In-Reply-To: <b26fc645cb2c81fe88ab13616c65664d2c3cead5.1561724493.git.mchehab+samsung@kernel.org>


On 6/28/19 2:30 PM, Mauro Carvalho Chehab wrote:
> The contents of those directories were orphaned at the documentation
> body.
> 
> While those directories could likely be moved to be inside some guide,
> I'm opting to just adding their indexes to the main one, removing the
> :orphan: and adding the SPDX header.
> 
> For the drivers, the rationale is that the documentation contains
> a mix of Kernelspace, uAPI and admin-guide. So, better to keep them on
> separate directories, as we've be doing with similar subsystem-specific
> docs that were not split yet.
> 
> For the others, well... I'm too lazy to do the move. Also, it
> seems to make sense to keep at least some of those at the main
> dir (like kbuild, for example). In any case, a latter patch
> could do the move.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>

Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

> ---
>  Documentation/cdrom/index.rst           |  2 +-
>  Documentation/fault-injection/index.rst |  2 +-
>  Documentation/fb/index.rst              |  2 +-
>  Documentation/fpga/index.rst            |  2 +-
>  Documentation/ide/index.rst             |  2 +-
>  Documentation/index.rst                 | 14 ++++++++++++++
>  Documentation/kbuild/index.rst          |  2 +-
>  Documentation/livepatch/index.rst       |  2 +-
>  Documentation/netlabel/index.rst        |  2 +-
>  Documentation/pcmcia/index.rst          |  2 +-
>  Documentation/power/index.rst           |  2 +-
>  Documentation/target/index.rst          |  2 +-
>  Documentation/timers/index.rst          |  2 +-
>  Documentation/watchdog/index.rst        |  2 +-
>  14 files changed, 27 insertions(+), 13 deletions(-)
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

^ permalink raw reply

* Re: [PATCH] devfreq: tegra20: add COMMON_CLK dependency
From: Dmitry Osipenko @ 2019-06-28 12:54 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Linux PM list,
	Linux Kernel Mailing List
In-Reply-To: <CAK8P3a1HOOk48G+-wZGWrM_fg5cn3ytH66qaEU6c7e0BjW+SNw@mail.gmail.com>

28.06.2019 15:27, Arnd Bergmann пишет:
> On Fri, Jun 28, 2019 at 1:05 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>> 28.06.2019 14:00, Dmitry Osipenko пишет:
> 
>>>
>>> How is it possible to happen? There is a stub for clk_set_min_rate() when COMMON_CLK
>>> is disabled .. Could you please show the kernel's config that causes the problem?
>>>
>>> Moreover that was me who added the missing stub for clk_set_min_rate() in [1] to fix [2].
>>>
>>> [1]
>>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/include/linux/clk.h?id=b88c9f4129dcec941e5a26508e991c08051ed1ac
>>>
>>> [2] https://lists.01.org/pipermail/kbuild-all/2019-April/060292.html
>>>
>>> So this patch looks wrong to me because apparently it fixes a non-existent problem.
>>>
>>
>> Oh, wait. That was "implicit declaration" and this one is "undefined reference"!
>> Still looks like something else need to be fixed..
> 
> The stub takes care of the case where there is no CLK support at all, i.e.
> CONFIG_HAVE_CLK is disabled. However, some older ARM platforms
> still have a custom implementation of the API and lack clk_set_min_rate().
> 
> $ git grep -l 'EXPORT_SYMBOL.*\<clk_set_rate\>'  | xargs grep -L
> clk_set_min_rate
> arch/arm/mach-ep93xx/clock.c
> arch/arm/mach-mmp/clock.c
> arch/arm/mach-omap1/clock.c
> arch/arm/mach-sa1100/clock.c
> arch/arm/mach-w90x900/clock.c
> arch/c6x/platforms/pll.c
> arch/m68k/coldfire/clk.c
> arch/mips/ar7/clock.c
> arch/mips/bcm63xx/clk.c
> arch/mips/lantiq/clk.c
> arch/mips/loongson64/lemote-2f/clock.c
> arch/mips/ralink/clk.c
> arch/unicore32/kernel/clock.c
> drivers/sh/clk/core.c
> 
> We could in theory convert all of those to COMMON_CLK, or simply add
> a dummy clk_set_min_rate() for each one, but since there is only one user,
> my fix was simpler.

Okay, thank you for the clarification. Looks like "Tegra30 DEVFREQ" should cause the
same trouble for you as well then.

Reviewed-by: Dmitry Osipenko <digetx@gmail.com>

^ permalink raw reply

* [PATCH 12/39] docs: interconnect.rst: add it to the driver-api guide
From: Mauro Carvalho Chehab @ 2019-06-28 12:30 UTC (permalink / raw)
  To: Linux Doc Mailing List
  Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-kernel,
	Jonathan Corbet, Georgi Djakov, linux-pm
In-Reply-To: <cover.1561724493.git.mchehab+samsung@kernel.org>

This is intented for Kernel hackers audience.

Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
 Documentation/driver-api/index.rst                          | 1 +
 Documentation/{interconnect => driver-api}/interconnect.rst | 2 --
 MAINTAINERS                                                 | 2 +-
 3 files changed, 2 insertions(+), 3 deletions(-)
 rename Documentation/{interconnect => driver-api}/interconnect.rst (99%)

diff --git a/Documentation/driver-api/index.rst b/Documentation/driver-api/index.rst
index 2a0b57f12d1a..97bab578ea72 100644
--- a/Documentation/driver-api/index.rst
+++ b/Documentation/driver-api/index.rst
@@ -35,6 +35,7 @@ available subsections can be seen below.
    spi
    i2c
    i3c/index
+   interconnect
    hsi
    edac
    scsi
diff --git a/Documentation/interconnect/interconnect.rst b/Documentation/driver-api/interconnect.rst
similarity index 99%
rename from Documentation/interconnect/interconnect.rst
rename to Documentation/driver-api/interconnect.rst
index 56e331dab70e..c3e004893796 100644
--- a/Documentation/interconnect/interconnect.rst
+++ b/Documentation/driver-api/interconnect.rst
@@ -1,7 +1,5 @@
 .. SPDX-License-Identifier: GPL-2.0
 
-:orphan:
-
 =====================================
 GENERIC SYSTEM INTERCONNECT SUBSYSTEM
 =====================================
diff --git a/MAINTAINERS b/MAINTAINERS
index 55996c9f2e0a..524e2c4300dc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8273,7 +8273,7 @@ INTERCONNECT API
 M:	Georgi Djakov <georgi.djakov@linaro.org>
 L:	linux-pm@vger.kernel.org
 S:	Maintained
-F:	Documentation/interconnect/
+F:	Documentation/driver-api/interconnect.rst
 F:	Documentation/devicetree/bindings/interconnect/
 F:	drivers/interconnect/
 F:	include/dt-bindings/interconnect/
-- 
2.21.0


^ permalink raw reply related

* [PATCH 01/39] docs: thermal: add it to the driver API
From: Mauro Carvalho Chehab @ 2019-06-28 12:29 UTC (permalink / raw)
  To: Linux Doc Mailing List
  Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-kernel,
	Jonathan Corbet, Amit Daniel Kachhap, Viresh Kumar, Javi Merino,
	Kukjin Kim, Krzysztof Kozlowski, Zhang Rui, Eduardo Valentin,
	Daniel Lezcano, linux-pm, linux-arm-kernel, linux-samsung-soc
In-Reply-To: <cover.1561724493.git.mchehab+samsung@kernel.org>

The file contents mostly describes driver internals.

Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
 Documentation/driver-api/index.rst                   |  1 +
 .../{ => driver-api}/thermal/cpu-cooling-api.rst     |  0
 .../{ => driver-api}/thermal/exynos_thermal.rst      |  0
 .../thermal/exynos_thermal_emulation.rst             |  0
 Documentation/{ => driver-api}/thermal/index.rst     |  2 --
 .../{ => driver-api}/thermal/intel_powerclamp.rst    |  0
 .../{ => driver-api}/thermal/nouveau_thermal.rst     |  0
 .../{ => driver-api}/thermal/power_allocator.rst     |  0
 Documentation/{ => driver-api}/thermal/sysfs-api.rst | 12 ++++++------
 .../thermal/x86_pkg_temperature_thermal.rst          |  2 +-
 MAINTAINERS                                          |  2 +-
 include/linux/thermal.h                              |  4 ++--
 12 files changed, 11 insertions(+), 12 deletions(-)
 rename Documentation/{ => driver-api}/thermal/cpu-cooling-api.rst (100%)
 rename Documentation/{ => driver-api}/thermal/exynos_thermal.rst (100%)
 rename Documentation/{ => driver-api}/thermal/exynos_thermal_emulation.rst (100%)
 rename Documentation/{ => driver-api}/thermal/index.rst (95%)
 rename Documentation/{ => driver-api}/thermal/intel_powerclamp.rst (100%)
 rename Documentation/{ => driver-api}/thermal/nouveau_thermal.rst (100%)
 rename Documentation/{ => driver-api}/thermal/power_allocator.rst (100%)
 rename Documentation/{ => driver-api}/thermal/sysfs-api.rst (98%)
 rename Documentation/{ => driver-api}/thermal/x86_pkg_temperature_thermal.rst (94%)

diff --git a/Documentation/driver-api/index.rst b/Documentation/driver-api/index.rst
index 0dbaa987aa11..4e503e360860 100644
--- a/Documentation/driver-api/index.rst
+++ b/Documentation/driver-api/index.rst
@@ -56,6 +56,7 @@ available subsections can be seen below.
    dmaengine/index
    slimbus
    soundwire/index
+   thermal/index
    fpga/index
    acpi/index
    generic-counter
diff --git a/Documentation/thermal/cpu-cooling-api.rst b/Documentation/driver-api/thermal/cpu-cooling-api.rst
similarity index 100%
rename from Documentation/thermal/cpu-cooling-api.rst
rename to Documentation/driver-api/thermal/cpu-cooling-api.rst
diff --git a/Documentation/thermal/exynos_thermal.rst b/Documentation/driver-api/thermal/exynos_thermal.rst
similarity index 100%
rename from Documentation/thermal/exynos_thermal.rst
rename to Documentation/driver-api/thermal/exynos_thermal.rst
diff --git a/Documentation/thermal/exynos_thermal_emulation.rst b/Documentation/driver-api/thermal/exynos_thermal_emulation.rst
similarity index 100%
rename from Documentation/thermal/exynos_thermal_emulation.rst
rename to Documentation/driver-api/thermal/exynos_thermal_emulation.rst
diff --git a/Documentation/thermal/index.rst b/Documentation/driver-api/thermal/index.rst
similarity index 95%
rename from Documentation/thermal/index.rst
rename to Documentation/driver-api/thermal/index.rst
index 8c1c00146cad..68ceb6886561 100644
--- a/Documentation/thermal/index.rst
+++ b/Documentation/driver-api/thermal/index.rst
@@ -1,5 +1,3 @@
-:orphan:
-
 =======
 Thermal
 =======
diff --git a/Documentation/thermal/intel_powerclamp.rst b/Documentation/driver-api/thermal/intel_powerclamp.rst
similarity index 100%
rename from Documentation/thermal/intel_powerclamp.rst
rename to Documentation/driver-api/thermal/intel_powerclamp.rst
diff --git a/Documentation/thermal/nouveau_thermal.rst b/Documentation/driver-api/thermal/nouveau_thermal.rst
similarity index 100%
rename from Documentation/thermal/nouveau_thermal.rst
rename to Documentation/driver-api/thermal/nouveau_thermal.rst
diff --git a/Documentation/thermal/power_allocator.rst b/Documentation/driver-api/thermal/power_allocator.rst
similarity index 100%
rename from Documentation/thermal/power_allocator.rst
rename to Documentation/driver-api/thermal/power_allocator.rst
diff --git a/Documentation/thermal/sysfs-api.rst b/Documentation/driver-api/thermal/sysfs-api.rst
similarity index 98%
rename from Documentation/thermal/sysfs-api.rst
rename to Documentation/driver-api/thermal/sysfs-api.rst
index e4930761d3e5..fab2c9b36d08 100644
--- a/Documentation/thermal/sysfs-api.rst
+++ b/Documentation/driver-api/thermal/sysfs-api.rst
@@ -552,7 +552,7 @@ emul_temp
 sustainable_power
 	An estimate of the sustained power that can be dissipated by
 	the thermal zone. Used by the power allocator governor. For
-	more information see Documentation/thermal/power_allocator.rst
+	more information see Documentation/driver-api/thermal/power_allocator.rst
 
 	Unit: milliwatts
 
@@ -563,7 +563,7 @@ k_po
 	controller during temperature overshoot. Temperature overshoot
 	is when the current temperature is above the "desired
 	temperature" trip point. For more information see
-	Documentation/thermal/power_allocator.rst
+	Documentation/driver-api/thermal/power_allocator.rst
 
 	RW, Optional
 
@@ -572,7 +572,7 @@ k_pu
 	controller during temperature undershoot. Temperature undershoot
 	is when the current temperature is below the "desired
 	temperature" trip point. For more information see
-	Documentation/thermal/power_allocator.rst
+	Documentation/driver-api/thermal/power_allocator.rst
 
 	RW, Optional
 
@@ -580,14 +580,14 @@ k_i
 	The integral term of the power allocator governor's PID
 	controller. This term allows the PID controller to compensate
 	for long term drift. For more information see
-	Documentation/thermal/power_allocator.rst
+	Documentation/driver-api/thermal/power_allocator.rst
 
 	RW, Optional
 
 k_d
 	The derivative term of the power allocator governor's PID
 	controller. For more information see
-	Documentation/thermal/power_allocator.rst
+	Documentation/driver-api/thermal/power_allocator.rst
 
 	RW, Optional
 
@@ -598,7 +598,7 @@ integral_cutoff
 	example, if integral_cutoff is 0, then the integral term only
 	accumulates error when temperature is above the desired
 	temperature trip point. For more information see
-	Documentation/thermal/power_allocator.rst
+	Documentation/driver-api/thermal/power_allocator.rst
 
 	Unit: millidegree Celsius
 
diff --git a/Documentation/thermal/x86_pkg_temperature_thermal.rst b/Documentation/driver-api/thermal/x86_pkg_temperature_thermal.rst
similarity index 94%
rename from Documentation/thermal/x86_pkg_temperature_thermal.rst
rename to Documentation/driver-api/thermal/x86_pkg_temperature_thermal.rst
index f134dbd3f5a9..2ac42ccd236f 100644
--- a/Documentation/thermal/x86_pkg_temperature_thermal.rst
+++ b/Documentation/driver-api/thermal/x86_pkg_temperature_thermal.rst
@@ -40,7 +40,7 @@ This contains two trip points:
 - trip_point_1_temp
 
 User can set any temperature between 0 to TJ-Max temperature. Temperature units
-are in milli-degree Celsius. Refer to "Documentation/thermal/sysfs-api.rst" for
+are in milli-degree Celsius. Refer to "Documentation/driver-api/thermal/sysfs-api.rst" for
 thermal sys-fs details.
 
 Any value other than 0 in these trip points, can trigger thermal notifications.
diff --git a/MAINTAINERS b/MAINTAINERS
index 7ba6d174f49f..9d3a408f5ce1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15762,7 +15762,7 @@ M:	Viresh Kumar <viresh.kumar@linaro.org>
 M:	Javi Merino <javi.merino@kernel.org>
 L:	linux-pm@vger.kernel.org
 S:	Supported
-F:	Documentation/thermal/cpu-cooling-api.rst
+F:	Documentation/driver-api/thermal/cpu-cooling-api.rst
 F:	drivers/thermal/cpu_cooling.c
 F:	include/linux/cpu_cooling.h
 
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 681047f8cc05..e45659c75920 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -251,7 +251,7 @@ struct thermal_bind_params {
 	 * platform characterization. This value is relative to the
 	 * rest of the weights so a cooling device whose weight is
 	 * double that of another cooling device is twice as
-	 * effective. See Documentation/thermal/sysfs-api.rst for more
+	 * effective. See Documentation/driver-api/thermal/sysfs-api.rst for more
 	 * information.
 	 */
 	int weight;
@@ -259,7 +259,7 @@ struct thermal_bind_params {
 	/*
 	 * This is a bit mask that gives the binding relation between this
 	 * thermal zone and cdev, for a particular trip point.
-	 * See Documentation/thermal/sysfs-api.rst for more information.
+	 * See Documentation/driver-api/thermal/sysfs-api.rst for more information.
 	 */
 	int trip_mask;
 
-- 
2.21.0


^ permalink raw reply related

* [PATCH 37/39] docs: adds some directories to the main documentation index
From: Mauro Carvalho Chehab @ 2019-06-28 12:30 UTC (permalink / raw)
  To: Linux Doc Mailing List
  Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-kernel,
	Jonathan Corbet, Jens Axboe, Akinobu Mita,
	Bartlomiej Zolnierkiewicz, Moritz Fischer, David S. Miller,
	Masahiro Yamada, Michal Marek, Josh Poimboeuf, Jiri Kosina,
	Miroslav Benes, Petr Mladek, Joe Lawrence, Paul Moore,
	Dominik Brodowski, Rafael J. Wysocki, Len Brown, Pavel Machek,
	Martin K. Petersen, Thomas Gleixner, Wim Van Sebroeck,
	Guenter Roeck, dri-devel, linux-fbdev, linux-fpga, linux-ide,
	linux-kbuild, live-patching, netdev, linux-security-module,
	linux-pm, linux-scsi, target-devel, linux-watchdog
In-Reply-To: <cover.1561724493.git.mchehab+samsung@kernel.org>

The contents of those directories were orphaned at the documentation
body.

While those directories could likely be moved to be inside some guide,
I'm opting to just adding their indexes to the main one, removing the
:orphan: and adding the SPDX header.

For the drivers, the rationale is that the documentation contains
a mix of Kernelspace, uAPI and admin-guide. So, better to keep them on
separate directories, as we've be doing with similar subsystem-specific
docs that were not split yet.

For the others, well... I'm too lazy to do the move. Also, it
seems to make sense to keep at least some of those at the main
dir (like kbuild, for example). In any case, a latter patch
could do the move.

Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
 Documentation/cdrom/index.rst           |  2 +-
 Documentation/fault-injection/index.rst |  2 +-
 Documentation/fb/index.rst              |  2 +-
 Documentation/fpga/index.rst            |  2 +-
 Documentation/ide/index.rst             |  2 +-
 Documentation/index.rst                 | 14 ++++++++++++++
 Documentation/kbuild/index.rst          |  2 +-
 Documentation/livepatch/index.rst       |  2 +-
 Documentation/netlabel/index.rst        |  2 +-
 Documentation/pcmcia/index.rst          |  2 +-
 Documentation/power/index.rst           |  2 +-
 Documentation/target/index.rst          |  2 +-
 Documentation/timers/index.rst          |  2 +-
 Documentation/watchdog/index.rst        |  2 +-
 14 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/Documentation/cdrom/index.rst b/Documentation/cdrom/index.rst
index efbd5d111825..338ad5f94e7c 100644
--- a/Documentation/cdrom/index.rst
+++ b/Documentation/cdrom/index.rst
@@ -1,4 +1,4 @@
-:orphan:
+.. SPDX-License-Identifier: GPL-2.0
 
 =====
 cdrom
diff --git a/Documentation/fault-injection/index.rst b/Documentation/fault-injection/index.rst
index 92b5639ed07a..8408a8a91b34 100644
--- a/Documentation/fault-injection/index.rst
+++ b/Documentation/fault-injection/index.rst
@@ -1,4 +1,4 @@
-:orphan:
+.. SPDX-License-Identifier: GPL-2.0
 
 ===============
 fault-injection
diff --git a/Documentation/fb/index.rst b/Documentation/fb/index.rst
index d47313714635..baf02393d8ee 100644
--- a/Documentation/fb/index.rst
+++ b/Documentation/fb/index.rst
@@ -1,4 +1,4 @@
-:orphan:
+.. SPDX-License-Identifier: GPL-2.0
 
 ============
 Frame Buffer
diff --git a/Documentation/fpga/index.rst b/Documentation/fpga/index.rst
index 2c87d1ea084f..f80f95667ca2 100644
--- a/Documentation/fpga/index.rst
+++ b/Documentation/fpga/index.rst
@@ -1,4 +1,4 @@
-:orphan:
+.. SPDX-License-Identifier: GPL-2.0
 
 ====
 fpga
diff --git a/Documentation/ide/index.rst b/Documentation/ide/index.rst
index 45bc12d3957f..813dfe611a31 100644
--- a/Documentation/ide/index.rst
+++ b/Documentation/ide/index.rst
@@ -1,4 +1,4 @@
-:orphan:
+.. SPDX-License-Identifier: GPL-2.0
 
 ==================================
 Integrated Drive Electronics (IDE)
diff --git a/Documentation/index.rst b/Documentation/index.rst
index e69d2fde7735..075c732501a2 100644
--- a/Documentation/index.rst
+++ b/Documentation/index.rst
@@ -35,6 +35,7 @@ trying to get it to work optimally on a given system.
    :maxdepth: 2
 
    admin-guide/index
+   kbuild/index
 
 Firmware-related documentation
 ------------------------------
@@ -77,6 +78,9 @@ merged much easier.
    kernel-hacking/index
    trace/index
    maintainer/index
+   fault-injection/index
+   livepatch/index
+
 
 Kernel API documentation
 ------------------------
@@ -94,12 +98,22 @@ needed).
    core-api/index
    accounting/index
    block/index
+   cdrom/index
+   ide/index
+   fb/index
+   fpga/index
    hid/index
    iio/index
    infiniband/index
    leds/index
    media/index
+   netlabel/index
    networking/index
+   pcmcia/index
+   power/index
+   target/index
+   timers/index
+   watchdog/index
    input/index
    hwmon/index
    gpu/index
diff --git a/Documentation/kbuild/index.rst b/Documentation/kbuild/index.rst
index 42d4cbe4460c..e323a3f2cc81 100644
--- a/Documentation/kbuild/index.rst
+++ b/Documentation/kbuild/index.rst
@@ -1,4 +1,4 @@
-:orphan:
+.. SPDX-License-Identifier: GPL-2.0
 
 ===================
 Kernel Build System
diff --git a/Documentation/livepatch/index.rst b/Documentation/livepatch/index.rst
index edd291d51847..17674a9e21b2 100644
--- a/Documentation/livepatch/index.rst
+++ b/Documentation/livepatch/index.rst
@@ -1,4 +1,4 @@
-:orphan:
+.. SPDX-License-Identifier: GPL-2.0
 
 ===================
 Kernel Livepatching
diff --git a/Documentation/netlabel/index.rst b/Documentation/netlabel/index.rst
index 47f1e0e5acd1..984e1b191b12 100644
--- a/Documentation/netlabel/index.rst
+++ b/Documentation/netlabel/index.rst
@@ -1,4 +1,4 @@
-:orphan:
+.. SPDX-License-Identifier: GPL-2.0
 
 ========
 NetLabel
diff --git a/Documentation/pcmcia/index.rst b/Documentation/pcmcia/index.rst
index 779c8527109e..7ae1f62fca14 100644
--- a/Documentation/pcmcia/index.rst
+++ b/Documentation/pcmcia/index.rst
@@ -1,4 +1,4 @@
-:orphan:
+.. SPDX-License-Identifier: GPL-2.0
 
 ======
 pcmcia
diff --git a/Documentation/power/index.rst b/Documentation/power/index.rst
index 20415f21e48a..002e42745263 100644
--- a/Documentation/power/index.rst
+++ b/Documentation/power/index.rst
@@ -1,4 +1,4 @@
-:orphan:
+.. SPDX-License-Identifier: GPL-2.0
 
 ================
 Power Management
diff --git a/Documentation/target/index.rst b/Documentation/target/index.rst
index b68f48982392..4b24f81f747e 100644
--- a/Documentation/target/index.rst
+++ b/Documentation/target/index.rst
@@ -1,4 +1,4 @@
-:orphan:
+.. SPDX-License-Identifier: GPL-2.0
 
 ==================
 TCM Virtual Device
diff --git a/Documentation/timers/index.rst b/Documentation/timers/index.rst
index 91f6f8263c48..df510ad0c989 100644
--- a/Documentation/timers/index.rst
+++ b/Documentation/timers/index.rst
@@ -1,4 +1,4 @@
-:orphan:
+.. SPDX-License-Identifier: GPL-2.0
 
 ======
 timers
diff --git a/Documentation/watchdog/index.rst b/Documentation/watchdog/index.rst
index 33a0de631e84..c177645081d8 100644
--- a/Documentation/watchdog/index.rst
+++ b/Documentation/watchdog/index.rst
@@ -1,4 +1,4 @@
-:orphan:
+.. SPDX-License-Identifier: GPL-2.0
 
 ======================
 Linux Watchdog Support
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH] devfreq: tegra20: add COMMON_CLK dependency
From: Arnd Bergmann @ 2019-06-28 12:27 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Linux PM list,
	Linux Kernel Mailing List
In-Reply-To: <cbf6cff3-f5f5-66ea-9cca-86eb93f23c50@gmail.com>

On Fri, Jun 28, 2019 at 1:05 PM Dmitry Osipenko <digetx@gmail.com> wrote:
> 28.06.2019 14:00, Dmitry Osipenko пишет:

> >
> > How is it possible to happen? There is a stub for clk_set_min_rate() when COMMON_CLK
> > is disabled .. Could you please show the kernel's config that causes the problem?
> >
> > Moreover that was me who added the missing stub for clk_set_min_rate() in [1] to fix [2].
> >
> > [1]
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/include/linux/clk.h?id=b88c9f4129dcec941e5a26508e991c08051ed1ac
> >
> > [2] https://lists.01.org/pipermail/kbuild-all/2019-April/060292.html
> >
> > So this patch looks wrong to me because apparently it fixes a non-existent problem.
> >
>
> Oh, wait. That was "implicit declaration" and this one is "undefined reference"!
> Still looks like something else need to be fixed..

The stub takes care of the case where there is no CLK support at all, i.e.
CONFIG_HAVE_CLK is disabled. However, some older ARM platforms
still have a custom implementation of the API and lack clk_set_min_rate().

$ git grep -l 'EXPORT_SYMBOL.*\<clk_set_rate\>'  | xargs grep -L
clk_set_min_rate
arch/arm/mach-ep93xx/clock.c
arch/arm/mach-mmp/clock.c
arch/arm/mach-omap1/clock.c
arch/arm/mach-sa1100/clock.c
arch/arm/mach-w90x900/clock.c
arch/c6x/platforms/pll.c
arch/m68k/coldfire/clk.c
arch/mips/ar7/clock.c
arch/mips/bcm63xx/clk.c
arch/mips/lantiq/clk.c
arch/mips/loongson64/lemote-2f/clock.c
arch/mips/ralink/clk.c
arch/unicore32/kernel/clock.c
drivers/sh/clk/core.c

We could in theory convert all of those to COMMON_CLK, or simply add
a dummy clk_set_min_rate() for each one, but since there is only one user,
my fix was simpler.

       Arnd

^ permalink raw reply

* Re: [patch 3/5] cpuidle: add haltpoll governor
From: Marcelo Tosatti @ 2019-06-27 18:08 UTC (permalink / raw)
  To: Doug Smythies
  Cc: 'Paolo Bonzini', 'Radim Krcmar',
	'Andrea Arcangeli', 'Rafael J. Wysocki',
	'Peter Zijlstra', 'Wanpeng Li',
	'Konrad Rzeszutek Wilk', 'Raslan KarimAllah',
	'Boris Ostrovsky', 'Ankur Arora',
	'Christian Borntraeger', linux-pm, 'kvm-devel'
In-Reply-To: <002e01d527c9$23be6e10$6b3b4a30$@net>

On Thu, Jun 20, 2019 at 05:34:46PM -0700, Doug Smythies wrote:
> Hi,
> 
> I tried your patch set, but only to check
> that they didn't cause any regression for situations
> where idle state 0 (Poll) is used a lot (teo governor).
> 
> They didn't (my testing was not thorough).
> 
> I do not know if the below matters or not.
> 
> On 2019.06.13 15:46 Marcelo Tosatti wrote:
> 
> ... [snip] ...
> 
> > Index: linux-2.6.git/Documentation/virtual/guest-halt-polling.txt
> > ===================================================================
> > --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> > +++ linux-2.6.git/Documentation/virtual/guest-halt-polling.txt	2019-06-13 18:16:22.414262777 -0400
> > @@ -0,0 +1,79 @@
> > +Guest halt polling
> > +==================
> > +
> > +The cpuidle_haltpoll driver, with the haltpoll governor, allows
> > +the guest vcpus to poll for a specified amount of time before
> > +halting.
> > +This provides the following benefits to host side polling:
> > +
> > +	1) The POLL flag is set while polling is performed, which allows
> > +	   a remote vCPU to avoid sending an IPI (and the associated
> > + 	   cost of handling the IPI) when performing a wakeup.
>    ^
>    |_ While applying the patches, git complains about this space character before the TAB.
> 
> It also complains about a few patches with a blank line before EOF.
> 
> ... Doug

Hi Doug,

Will fix those, thanks.


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox