[PATCH 3/6] devfreq: add support for suspend/resume of a devfreq device

[Lukasz Luba]

The patch adds support for handling suspend/resume process.
It uses atomic variables to make sure no race condition
affects the process.

The patch draws on Tobias Jakobi's work posted ~2 years ago, who tried to
solve issue with devfreq device's frequency during suspend/resume.
During the discussion on LKML some corner cases and comments appeared
related to the design. This patch address them keeping in mind suggestions
from Chanwoo Choi.

Suggested-by: Tobias Jakobi 
Suggested-by: Chanwoo Choi 
Signed-off-by: Lukasz Luba 
---
 drivers/devfreq/devfreq.c | 45 +++--
 1 file changed, 39 insertions(+), 6 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index cf9c643..7e09de8 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -872,14 +872,33 @@ EXPORT_SYMBOL(devm_devfreq_remove_device);
  */
 int devfreq_suspend_device(struct devfreq *devfreq)
 {
-   if (!devfreq)
-   return -EINVAL;
+int ret;
+unsigned long prev_freq;
+u32 flags = 0;
+
+if (!devfreq)
+return -EINVAL;
+
+if (devfreq->governor) {
+   ret = devfreq->governor->event_handler(devfreq,
+   DEVFREQ_GOV_SUSPEND, NULL);
+   if (ret)
+   return ret;
+   }
 
-   if (!devfreq->governor)
-   return 0;
+   if (devfreq->suspend_freq) {
+   if (atomic_inc_return(>suspend_count) > 1)
+   return 0;
 
-   return devfreq->governor->event_handler(devfreq,
-   DEVFREQ_GOV_SUSPEND, NULL);
+   ret = devfreq_set_target(devfreq, devfreq->suspend_freq,
+_freq, flags);
+   if (ret)
+   return ret;
+
+   devfreq->resume_freq = prev_freq;
+   }
+
+return 0;
 }
 EXPORT_SYMBOL(devfreq_suspend_device);
 
@@ -893,9 +912,23 @@ EXPORT_SYMBOL(devfreq_suspend_device);
  */
 int devfreq_resume_device(struct devfreq *devfreq)
 {
+int ret;
+unsigned long prev_freq;
+u32 flags = 0;
+
if (!devfreq)
return -EINVAL;
 
+   if (devfreq->suspend_freq) {
+   if (atomic_dec_return(>suspend_count) >= 1)
+   return 0;
+
+   ret = devfreq_set_target(devfreq, devfreq->resume_freq,
+_freq, flags);
+   if (ret)
+   return ret;
+   }
+
if (!devfreq->governor)
return 0;
 
-- 
2.7.4

[PATCH 3/6] devfreq: add support for suspend/resume of a devfreq device

[Lukasz Luba]

The patch adds support for handling suspend/resume process.
It uses atomic variables to make sure no race condition
affects the process.

The patch draws on Tobias Jakobi's work posted ~2 years ago, who tried to
solve issue with devfreq device's frequency during suspend/resume.
During the discussion on LKML some corner cases and comments appeared
related to the design. This patch address them keeping in mind suggestions
from Chanwoo Choi.

Suggested-by: Tobias Jakobi 
Suggested-by: Chanwoo Choi 
Signed-off-by: Lukasz Luba 
---
 drivers/devfreq/devfreq.c | 45 +++--
 1 file changed, 39 insertions(+), 6 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index cf9c643..7e09de8 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -872,14 +872,33 @@ EXPORT_SYMBOL(devm_devfreq_remove_device);
  */
 int devfreq_suspend_device(struct devfreq *devfreq)
 {
-   if (!devfreq)
-   return -EINVAL;
+int ret;
+unsigned long prev_freq;
+u32 flags = 0;
+
+if (!devfreq)
+return -EINVAL;
+
+if (devfreq->governor) {
+   ret = devfreq->governor->event_handler(devfreq,
+   DEVFREQ_GOV_SUSPEND, NULL);
+   if (ret)
+   return ret;
+   }
 
-   if (!devfreq->governor)
-   return 0;
+   if (devfreq->suspend_freq) {
+   if (atomic_inc_return(>suspend_count) > 1)
+   return 0;
 
-   return devfreq->governor->event_handler(devfreq,
-   DEVFREQ_GOV_SUSPEND, NULL);
+   ret = devfreq_set_target(devfreq, devfreq->suspend_freq,
+_freq, flags);
+   if (ret)
+   return ret;
+
+   devfreq->resume_freq = prev_freq;
+   }
+
+return 0;
 }
 EXPORT_SYMBOL(devfreq_suspend_device);
 
@@ -893,9 +912,23 @@ EXPORT_SYMBOL(devfreq_suspend_device);
  */
 int devfreq_resume_device(struct devfreq *devfreq)
 {
+int ret;
+unsigned long prev_freq;
+u32 flags = 0;
+
if (!devfreq)
return -EINVAL;
 
+   if (devfreq->suspend_freq) {
+   if (atomic_dec_return(>suspend_count) >= 1)
+   return 0;
+
+   ret = devfreq_set_target(devfreq, devfreq->resume_freq,
+_freq, flags);
+   if (ret)
+   return ret;
+   }
+
if (!devfreq->governor)
return 0;
 
-- 
2.7.4

[PATCH 5/6] drivers: power: suspend: call devfreq suspend/resume

[Lukasz Luba]

Devfreq framework supports suspend of its devices.
Call the the devfreq interface and allow devfreq devices preserve/restore
their states during suspend/resume.

The patch draws on Tobias Jakobi's work posted ~2 years ago, who tried to
solve issue with devfreq device's frequency during suspend/resume.
During the discussion on LKML some corner cases and comments appeared
related to the design. This patch address them keeping in mind suggestions
from Chanwoo Choi.

Suggested-by: Tobias Jakobi 
Signed-off-by: Lukasz Luba 
---
 drivers/base/power/main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index a690fd4..0992e67 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -32,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "../base.h"
@@ -1078,6 +1079,7 @@ void dpm_resume(pm_message_t state)
dpm_show_time(starttime, state, 0, NULL);
 
cpufreq_resume();
+   devfreq_resume();
trace_suspend_resume(TPS("dpm_resume"), state.event, false);
 }
 
@@ -1852,6 +1854,7 @@ int dpm_suspend(pm_message_t state)
trace_suspend_resume(TPS("dpm_suspend"), state.event, true);
might_sleep();
 
+   devfreq_suspend();
cpufreq_suspend();
 
mutex_lock(_list_mtx);
-- 
2.7.4

[PATCH 5/6] drivers: power: suspend: call devfreq suspend/resume

[Lukasz Luba]

Devfreq framework supports suspend of its devices.
Call the the devfreq interface and allow devfreq devices preserve/restore
their states during suspend/resume.

The patch draws on Tobias Jakobi's work posted ~2 years ago, who tried to
solve issue with devfreq device's frequency during suspend/resume.
During the discussion on LKML some corner cases and comments appeared
related to the design. This patch address them keeping in mind suggestions
from Chanwoo Choi.

Suggested-by: Tobias Jakobi 
Signed-off-by: Lukasz Luba 
---
 drivers/base/power/main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index a690fd4..0992e67 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -32,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "../base.h"
@@ -1078,6 +1079,7 @@ void dpm_resume(pm_message_t state)
dpm_show_time(starttime, state, 0, NULL);
 
cpufreq_resume();
+   devfreq_resume();
trace_suspend_resume(TPS("dpm_resume"), state.event, false);
 }
 
@@ -1852,6 +1854,7 @@ int dpm_suspend(pm_message_t state)
trace_suspend_resume(TPS("dpm_suspend"), state.event, true);
might_sleep();
 
+   devfreq_suspend();
cpufreq_suspend();
 
mutex_lock(_list_mtx);
-- 
2.7.4

[PATCH 0/6] devfreq: handle suspend/resume

[Lukasz Luba]

Hi all,

This patch set aims to address the issue with devfreq devices' frequency
during suspend/resume. It extends suspend/resume by calls to Devfreq
framework. In the devfreq framework there is a small refactoring to avoid
code duplication in changging frequency (patch 2) and there are extensions
for suspending devices.

It has been tested on Odroid u3 with Exynos 4412.

The patch set draws on Tobias Jakobi's work posted ~2 years ago, who tried
to solve issue with devfreq device's frequency during suspend/resume.
During the discussion on LKML some corner cases and comments appeared
related to the design. This patch set address them keeping in mind
suggestions from Chanwoo Choi.
Tobias's paches:
https://www.spinics.net/lists/linux-samsung-soc/msg56602.html 

Regards,
Lukasz Luba

Lukasz Luba (6):
  devfreq: add basic fileds supporting suspend functionality
  devfreq: refactor set_target frequency function
  devfreq: add support for suspend/resume of a devfreq device
  devfreq: add devfreq_suspend/resume() functions
  drivers: power: suspend: call devfreq suspend/resume
  arm: dts: exynos4: set opp-suspend for DMC and leftbus

 arch/arm/boot/dts/exynos4210.dtsi |   2 +
 arch/arm/boot/dts/exynos4412.dtsi |   2 +
 drivers/base/power/main.c |   3 +
 drivers/devfreq/devfreq.c | 159 ++
 include/linux/devfreq.h   |  11 +++
 5 files changed, 146 insertions(+), 31 deletions(-)

-- 
2.7.4

[PATCH 0/6] devfreq: handle suspend/resume

[Lukasz Luba]

Hi all,

This patch set aims to address the issue with devfreq devices' frequency
during suspend/resume. It extends suspend/resume by calls to Devfreq
framework. In the devfreq framework there is a small refactoring to avoid
code duplication in changging frequency (patch 2) and there are extensions
for suspending devices.

It has been tested on Odroid u3 with Exynos 4412.

The patch set draws on Tobias Jakobi's work posted ~2 years ago, who tried
to solve issue with devfreq device's frequency during suspend/resume.
During the discussion on LKML some corner cases and comments appeared
related to the design. This patch set address them keeping in mind
suggestions from Chanwoo Choi.
Tobias's paches:
https://www.spinics.net/lists/linux-samsung-soc/msg56602.html 

Regards,
Lukasz Luba

Lukasz Luba (6):
  devfreq: add basic fileds supporting suspend functionality
  devfreq: refactor set_target frequency function
  devfreq: add support for suspend/resume of a devfreq device
  devfreq: add devfreq_suspend/resume() functions
  drivers: power: suspend: call devfreq suspend/resume
  arm: dts: exynos4: set opp-suspend for DMC and leftbus

 arch/arm/boot/dts/exynos4210.dtsi |   2 +
 arch/arm/boot/dts/exynos4412.dtsi |   2 +
 drivers/base/power/main.c |   3 +
 drivers/devfreq/devfreq.c | 159 ++
 include/linux/devfreq.h   |  11 +++
 5 files changed, 146 insertions(+), 31 deletions(-)

-- 
2.7.4

Re: [PATCH] gpio: mxc: move gpio noirq suspend/resume to syscore phase

[Linus Walleij]

On Fri, Nov 9, 2018 at 5:56 AM Anson Huang  wrote:

> During noirq suspend/resume phase, GPIO irq could arrive
> and its registers like IMR will be changed by irq handle
> process, to make the GPIO registers exactly when it is
> powered ON after resume, move the GPIO noirq suspend/resume
> callback to syscore suspend/resume phase, local irq is
> disabled at this phase so GPIO registers are atomic.
>
> Signed-off-by: Anson Huang 

Patch applied. Cc Fabio so he knows it happens.

Yours,
Linus Walleij

Re: [PATCH] gpio: mxc: move gpio noirq suspend/resume to syscore phase

[Linus Walleij]

On Fri, Nov 9, 2018 at 5:56 AM Anson Huang  wrote:

> During noirq suspend/resume phase, GPIO irq could arrive
> and its registers like IMR will be changed by irq handle
> process, to make the GPIO registers exactly when it is
> powered ON after resume, move the GPIO noirq suspend/resume
> callback to syscore suspend/resume phase, local irq is
> disabled at this phase so GPIO registers are atomic.
>
> Signed-off-by: Anson Huang 

Patch applied. Cc Fabio so he knows it happens.

Yours,
Linus Walleij

Re: [PATCH v2] PCI: imx: Add imx6sx suspend/resume support

[Andrey Smirnov]

On Wed, Nov 7, 2018 at 5:57 AM Leonard Crestez  wrote:
>
> Enable PCI suspend/resume support on imx6sx socs. This is similar to
> imx7d with a few differences:
>
> * The PM_Turn_Off bit is exposed through an IOMUX GPR, like all other
> pcie control bits on 6sx.
> * The pcie_inbound_axi clk needs to be turned off in suspend. On resume
> it is restored via resume -> deassert_core_reset -> enable_ref_clk.
>
> Most of the resume logic is shared with the initial reset after probe.
>
> Signed-off-by: Leonard Crestez 
>
> ---
> Changes since v1:
> * Use a switch statement in imx6_pcie_pm_turnoff. The DT-based turnoff
> path is still an if statement.
> * Did not split imx6_pcie_clk_disable or call it from other paths, this
> would bring complications and is somewhat unrelated.
> * See v1 comments: https://lore.kernel.org/patchwork/patch/996806/
>
>  drivers/pci/controller/dwc/pci-imx6.c   | 44 ++---
>  include/linux/mfd/syscon/imx6q-iomuxc-gpr.h |  1 +
>  2 files changed, 40 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c 
> b/drivers/pci/controller/dwc/pci-imx6.c
> index 2cbef2d7c207..54625569d0bc 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -771,41 +771,75 @@ static void imx6_pcie_ltssm_disable(struct device *dev)
> }
>  }
>
>  static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
>  {
> -   reset_control_assert(imx6_pcie->turnoff_reset);
> -   reset_control_deassert(imx6_pcie->turnoff_reset);
> +   struct device *dev = imx6_pcie->pci->dev;
> +
> +   /* Some variants have a turnoff reset in DT */
> +   if (imx6_pcie->turnoff_reset) {
> +   reset_control_assert(imx6_pcie->turnoff_reset);
> +   reset_control_deassert(imx6_pcie->turnoff_reset);
> +   goto pm_turnoff_sleep;
> +   }
> +
> +   /* Others poke directly at IOMUXC registers */
> +   switch (imx6_pcie->variant) {
> +   case IMX6SX:
> +   regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +   IMX6SX_GPR12_PCIE_PM_TURN_OFF,
> +   IMX6SX_GPR12_PCIE_PM_TURN_OFF);
> +   regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +   IMX6SX_GPR12_PCIE_PM_TURN_OFF, 0);
> +   break;
> +   default:
> +   dev_err(dev, "PME_Turn_Off not implemented\n");
> +   return;

Purely optionally, if you feel like avoiding goto you can change this to:

default:
if (!imx6_pcie->turnoff_reset) {
dev_err(dev, "PME_Turn_Off not implemented\n");
return;
}

reset_control_assert(imx6_pcie->turnoff_reset);
reset_control_deassert(imx6_pcie->turnoff_reset);
break;

but that's up to you. FWIW, patch looks reasonable:

Reviewed-by: Andrey Smirnov 

> +   }
>
> /*
>  * Components with an upstream port must respond to
>  * PME_Turn_Off with PME_TO_Ack but we can't check.
>  *
>  * The standard recommends a 1-10ms timeout after which to
>  * proceed anyway as if acks were received.
>  */
> +pm_turnoff_sleep:
> usleep_range(1000, 1);
>  }
>
>  static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
>  {
> clk_disable_unprepare(imx6_pcie->pcie);
> clk_disable_unprepare(imx6_pcie->pcie_phy);
> clk_disable_unprepare(imx6_pcie->pcie_bus);
>
> -   if (imx6_pcie->variant == IMX7D) {
> +   switch (imx6_pcie->variant) {
> +   case IMX6SX:
> +   clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
> +   break;
> +   case IMX7D:
> regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
>IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
> +   break;
> +   default:
> +   break;
> }
>  }
>
> +static inline bool imx6_pcie_supports_suspend(struct imx6_pcie *imx6_pcie)
> +{
> +   return (imx6_pcie->variant == IMX7D ||
> +   imx6_pcie->variant == IMX6SX);
> +}
> +
>  static int imx6_pcie_suspend_noirq(struct device *dev)
>  {
> struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
>
> -   if (imx6_pcie->variant != IMX7D)
> +   if (!imx6_pcie_supports_suspend(imx6_pcie))
> return 0;
>
> imx6_pcie_pm_turnoff(imx6_pcie);
> imx6_pcie_clk_disable(imx6_pcie);
>   

Re: [PATCH v2] PCI: imx: Add imx6sx suspend/resume support

[Andrey Smirnov]

On Wed, Nov 7, 2018 at 5:57 AM Leonard Crestez  wrote:
>
> Enable PCI suspend/resume support on imx6sx socs. This is similar to
> imx7d with a few differences:
>
> * The PM_Turn_Off bit is exposed through an IOMUX GPR, like all other
> pcie control bits on 6sx.
> * The pcie_inbound_axi clk needs to be turned off in suspend. On resume
> it is restored via resume -> deassert_core_reset -> enable_ref_clk.
>
> Most of the resume logic is shared with the initial reset after probe.
>
> Signed-off-by: Leonard Crestez 
>
> ---
> Changes since v1:
> * Use a switch statement in imx6_pcie_pm_turnoff. The DT-based turnoff
> path is still an if statement.
> * Did not split imx6_pcie_clk_disable or call it from other paths, this
> would bring complications and is somewhat unrelated.
> * See v1 comments: https://lore.kernel.org/patchwork/patch/996806/
>
>  drivers/pci/controller/dwc/pci-imx6.c   | 44 ++---
>  include/linux/mfd/syscon/imx6q-iomuxc-gpr.h |  1 +
>  2 files changed, 40 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c 
> b/drivers/pci/controller/dwc/pci-imx6.c
> index 2cbef2d7c207..54625569d0bc 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -771,41 +771,75 @@ static void imx6_pcie_ltssm_disable(struct device *dev)
> }
>  }
>
>  static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
>  {
> -   reset_control_assert(imx6_pcie->turnoff_reset);
> -   reset_control_deassert(imx6_pcie->turnoff_reset);
> +   struct device *dev = imx6_pcie->pci->dev;
> +
> +   /* Some variants have a turnoff reset in DT */
> +   if (imx6_pcie->turnoff_reset) {
> +   reset_control_assert(imx6_pcie->turnoff_reset);
> +   reset_control_deassert(imx6_pcie->turnoff_reset);
> +   goto pm_turnoff_sleep;
> +   }
> +
> +   /* Others poke directly at IOMUXC registers */
> +   switch (imx6_pcie->variant) {
> +   case IMX6SX:
> +   regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +   IMX6SX_GPR12_PCIE_PM_TURN_OFF,
> +   IMX6SX_GPR12_PCIE_PM_TURN_OFF);
> +   regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +   IMX6SX_GPR12_PCIE_PM_TURN_OFF, 0);
> +   break;
> +   default:
> +   dev_err(dev, "PME_Turn_Off not implemented\n");
> +   return;

Purely optionally, if you feel like avoiding goto you can change this to:

default:
if (!imx6_pcie->turnoff_reset) {
dev_err(dev, "PME_Turn_Off not implemented\n");
return;
}

reset_control_assert(imx6_pcie->turnoff_reset);
reset_control_deassert(imx6_pcie->turnoff_reset);
break;

but that's up to you. FWIW, patch looks reasonable:

Reviewed-by: Andrey Smirnov 

> +   }
>
> /*
>  * Components with an upstream port must respond to
>  * PME_Turn_Off with PME_TO_Ack but we can't check.
>  *
>  * The standard recommends a 1-10ms timeout after which to
>  * proceed anyway as if acks were received.
>  */
> +pm_turnoff_sleep:
> usleep_range(1000, 1);
>  }
>
>  static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
>  {
> clk_disable_unprepare(imx6_pcie->pcie);
> clk_disable_unprepare(imx6_pcie->pcie_phy);
> clk_disable_unprepare(imx6_pcie->pcie_bus);
>
> -   if (imx6_pcie->variant == IMX7D) {
> +   switch (imx6_pcie->variant) {
> +   case IMX6SX:
> +   clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
> +   break;
> +   case IMX7D:
> regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
>IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
> +   break;
> +   default:
> +   break;
> }
>  }
>
> +static inline bool imx6_pcie_supports_suspend(struct imx6_pcie *imx6_pcie)
> +{
> +   return (imx6_pcie->variant == IMX7D ||
> +   imx6_pcie->variant == IMX6SX);
> +}
> +
>  static int imx6_pcie_suspend_noirq(struct device *dev)
>  {
> struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
>
> -   if (imx6_pcie->variant != IMX7D)
> +   if (!imx6_pcie_supports_suspend(imx6_pcie))
> return 0;
>
> imx6_pcie_pm_turnoff(imx6_pcie);
> imx6_pcie_clk_disable(imx6_pcie);
>   

[PATCH 4/5] power: supply: sc27xx: Add suspend/resume interfaces

[Baolin Wang]

From: Yuanjiang Yu 

Add fuel gauge platform suspend and resume interfaces. In suspend state,
we should enable the low voltage and coulomb counter threshold interrupts
to wake up system to calibrate the battery capacity in lower voltage stage.

Signed-off-by: Yuanjiang Yu 
Signed-off-by: Baolin Wang 
---
 drivers/power/supply/sc27xx_fuel_gauge.c |   77 ++
 1 file changed, 77 insertions(+)

diff --git a/drivers/power/supply/sc27xx_fuel_gauge.c 
b/drivers/power/supply/sc27xx_fuel_gauge.c
index 962d0f8..8c52e29 100644
--- a/drivers/power/supply/sc27xx_fuel_gauge.c
+++ b/drivers/power/supply/sc27xx_fuel_gauge.c
@@ -789,6 +789,7 @@ static int sc27xx_fgu_probe(struct platform_device *pdev)
data->bat_present = !!ret;
mutex_init(>lock);
data->dev = >dev;
+   platform_set_drvdata(pdev, data);
 
fgu_cfg.drv_data = data;
fgu_cfg.of_node = np;
@@ -846,6 +847,81 @@ static int sc27xx_fgu_probe(struct platform_device *pdev)
return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int sc27xx_fgu_resume(struct device *dev)
+{
+   struct sc27xx_fgu_data *data = dev_get_drvdata(dev);
+   int ret;
+
+   ret = regmap_update_bits(data->regmap, data->base + SC27XX_FGU_INT_EN,
+SC27XX_FGU_LOW_OVERLOAD_INT |
+SC27XX_FGU_CLBCNT_DELTA_INT, 0);
+   if (ret) {
+   dev_err(data->dev, "failed to disable fgu interrupts\n");
+   return ret;
+   }
+
+   return 0;
+}
+
+static int sc27xx_fgu_suspend(struct device *dev)
+{
+   struct sc27xx_fgu_data *data = dev_get_drvdata(dev);
+   int ret, status, ocv;
+
+   ret = sc27xx_fgu_get_status(data, );
+   if (ret)
+   return ret;
+
+   /*
+* If we are charging, then no need to enable the FGU interrupts to
+* adjust the battery capacity.
+*/
+   if (status != POWER_SUPPLY_STATUS_NOT_CHARGING)
+   return 0;
+
+   ret = regmap_update_bits(data->regmap, data->base + SC27XX_FGU_INT_EN,
+SC27XX_FGU_LOW_OVERLOAD_INT,
+SC27XX_FGU_LOW_OVERLOAD_INT);
+   if (ret) {
+   dev_err(data->dev, "failed to enable low voltage interrupt\n");
+   return ret;
+   }
+
+   ret = sc27xx_fgu_get_vbat_ocv(data, );
+   if (ret)
+   goto disable_int;
+
+   /*
+* If current OCV is less than the minimum voltage, we should enable the
+* coulomb counter threshold interrupt to notify events to adjust the
+* battery capacity.
+*/
+   if (ocv < data->min_volt) {
+   ret = regmap_update_bits(data->regmap,
+data->base + SC27XX_FGU_INT_EN,
+SC27XX_FGU_CLBCNT_DELTA_INT,
+SC27XX_FGU_CLBCNT_DELTA_INT);
+   if (ret) {
+   dev_err(data->dev,
+   "failed to enable coulomb threshold int\n");
+   goto disable_int;
+   }
+   }
+
+   return 0;
+
+disable_int:
+   regmap_update_bits(data->regmap, data->base + SC27XX_FGU_INT_EN,
+  SC27XX_FGU_LOW_OVERLOAD_INT, 0);
+   return ret;
+}
+#endif
+
+static const struct dev_pm_ops sc27xx_fgu_pm_ops = {
+   SET_SYSTEM_SLEEP_PM_OPS(sc27xx_fgu_suspend, sc27xx_fgu_resume)
+};
+
 static const struct of_device_id sc27xx_fgu_of_match[] = {
{ .compatible = "sprd,sc2731-fgu", },
{ }
@@ -856,6 +932,7 @@ static int sc27xx_fgu_probe(struct platform_device *pdev)
.driver = {
.name = "sc27xx-fgu",
.of_match_table = sc27xx_fgu_of_match,
+   .pm = _fgu_pm_ops,
}
 };
 
-- 
1.7.9.5

[PATCH 4/5] power: supply: sc27xx: Add suspend/resume interfaces

[Baolin Wang]

From: Yuanjiang Yu 

Add fuel gauge platform suspend and resume interfaces. In suspend state,
we should enable the low voltage and coulomb counter threshold interrupts
to wake up system to calibrate the battery capacity in lower voltage stage.

Signed-off-by: Yuanjiang Yu 
Signed-off-by: Baolin Wang 
---
 drivers/power/supply/sc27xx_fuel_gauge.c |   77 ++
 1 file changed, 77 insertions(+)

diff --git a/drivers/power/supply/sc27xx_fuel_gauge.c 
b/drivers/power/supply/sc27xx_fuel_gauge.c
index 962d0f8..8c52e29 100644
--- a/drivers/power/supply/sc27xx_fuel_gauge.c
+++ b/drivers/power/supply/sc27xx_fuel_gauge.c
@@ -789,6 +789,7 @@ static int sc27xx_fgu_probe(struct platform_device *pdev)
data->bat_present = !!ret;
mutex_init(>lock);
data->dev = >dev;
+   platform_set_drvdata(pdev, data);
 
fgu_cfg.drv_data = data;
fgu_cfg.of_node = np;
@@ -846,6 +847,81 @@ static int sc27xx_fgu_probe(struct platform_device *pdev)
return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int sc27xx_fgu_resume(struct device *dev)
+{
+   struct sc27xx_fgu_data *data = dev_get_drvdata(dev);
+   int ret;
+
+   ret = regmap_update_bits(data->regmap, data->base + SC27XX_FGU_INT_EN,
+SC27XX_FGU_LOW_OVERLOAD_INT |
+SC27XX_FGU_CLBCNT_DELTA_INT, 0);
+   if (ret) {
+   dev_err(data->dev, "failed to disable fgu interrupts\n");
+   return ret;
+   }
+
+   return 0;
+}
+
+static int sc27xx_fgu_suspend(struct device *dev)
+{
+   struct sc27xx_fgu_data *data = dev_get_drvdata(dev);
+   int ret, status, ocv;
+
+   ret = sc27xx_fgu_get_status(data, );
+   if (ret)
+   return ret;
+
+   /*
+* If we are charging, then no need to enable the FGU interrupts to
+* adjust the battery capacity.
+*/
+   if (status != POWER_SUPPLY_STATUS_NOT_CHARGING)
+   return 0;
+
+   ret = regmap_update_bits(data->regmap, data->base + SC27XX_FGU_INT_EN,
+SC27XX_FGU_LOW_OVERLOAD_INT,
+SC27XX_FGU_LOW_OVERLOAD_INT);
+   if (ret) {
+   dev_err(data->dev, "failed to enable low voltage interrupt\n");
+   return ret;
+   }
+
+   ret = sc27xx_fgu_get_vbat_ocv(data, );
+   if (ret)
+   goto disable_int;
+
+   /*
+* If current OCV is less than the minimum voltage, we should enable the
+* coulomb counter threshold interrupt to notify events to adjust the
+* battery capacity.
+*/
+   if (ocv < data->min_volt) {
+   ret = regmap_update_bits(data->regmap,
+data->base + SC27XX_FGU_INT_EN,
+SC27XX_FGU_CLBCNT_DELTA_INT,
+SC27XX_FGU_CLBCNT_DELTA_INT);
+   if (ret) {
+   dev_err(data->dev,
+   "failed to enable coulomb threshold int\n");
+   goto disable_int;
+   }
+   }
+
+   return 0;
+
+disable_int:
+   regmap_update_bits(data->regmap, data->base + SC27XX_FGU_INT_EN,
+  SC27XX_FGU_LOW_OVERLOAD_INT, 0);
+   return ret;
+}
+#endif
+
+static const struct dev_pm_ops sc27xx_fgu_pm_ops = {
+   SET_SYSTEM_SLEEP_PM_OPS(sc27xx_fgu_suspend, sc27xx_fgu_resume)
+};
+
 static const struct of_device_id sc27xx_fgu_of_match[] = {
{ .compatible = "sprd,sc2731-fgu", },
{ }
@@ -856,6 +932,7 @@ static int sc27xx_fgu_probe(struct platform_device *pdev)
.driver = {
.name = "sc27xx-fgu",
.of_match_table = sc27xx_fgu_of_match,
+   .pm = _fgu_pm_ops,
}
 };
 
-- 
1.7.9.5

[PATCH 3.16 121/366] pwm: lpss: platform: Save/restore the ctrl register over a suspend/resume

[Ben Hutchings]

3.16.61-rc1 review patch.  If anyone has any objections, please let me know.

--

From: Hans de Goede 

commit 1d375b58c12f08d8570b30b865def4734517f04f upstream.

On some devices the contents of the ctrl register get lost over a
suspend/resume and the PWM comes back up disabled after the resume.

This is seen on some Bay Trail devices with the PWM in ACPI enumerated
mode, so it shows up as a platform device instead of a PCI device.

If we still think it is enabled and then try to change the duty-cycle
after this, we end up with a "PWM_SW_UPDATE was not cleared" error and
the PWM is stuck in that state from then on.

This commit adds suspend and resume pm callbacks to the pwm-lpss-platform
code, which save/restore the ctrl register over a suspend/resume, fixing
this.

Note that:

1) There is no need to do this over a runtime suspend, since we
only runtime suspend when disabled and then we properly set the enable
bit and reprogram the timings when we re-enable the PWM.

2) This may be happening on more systems then we realize, but has been
covered up sofar by a bug in the acpi-lpss.c code which was save/restoring
the regular device registers instead of the lpss private registers due to
lpss_device_desc.prv_offset not being set. This is fixed by a later patch
in this series.

Signed-off-by: Hans de Goede 
Reviewed-by: Andy Shevchenko 
Signed-off-by: Thierry Reding 
[bwh: Backported to 3.16:
 - pwm-lpss is a single module, so make the new functions static
 - Only one PWM per chip is supported; remove the npwm assertion and loops
 - Adjust filenames, context]
Signed-off-by: Ben Hutchings 
---
--- a/drivers/pwm/pwm-lpss.c
+++ b/drivers/pwm/pwm-lpss.c
@@ -39,6 +39,7 @@ struct pwm_lpss_chip {
void __iomem *regs;
struct clk *clk;
unsigned long clk_rate;
+   u32 saved_ctrl;
 };
 
 struct pwm_lpss_boardinfo {
@@ -177,6 +178,24 @@ static int pwm_lpss_remove(struct pwm_lp
return pwmchip_remove(>chip);
 }
 
+static int pwm_lpss_suspend(struct device *dev)
+{
+   struct pwm_lpss_chip *lpwm = dev_get_drvdata(dev);
+
+   lpwm->saved_ctrl = readl(lpwm->regs + PWM);
+
+   return 0;
+}
+
+static int pwm_lpss_resume(struct device *dev)
+{
+   struct pwm_lpss_chip *lpwm = dev_get_drvdata(dev);
+
+   writel(lpwm->saved_ctrl, lpwm->regs + PWM);
+
+   return 0;
+}
+
 static int pwm_lpss_probe_pci(struct pci_dev *pdev,
  const struct pci_device_id *id)
 {
@@ -241,6 +260,10 @@ static int pwm_lpss_remove_platform(stru
return pwm_lpss_remove(lpwm);
 }
 
+static SIMPLE_DEV_PM_OPS(pwm_lpss_platform_pm_ops,
+pwm_lpss_suspend,
+pwm_lpss_resume);
+
 static const struct acpi_device_id pwm_lpss_acpi_match[] = {
{ "80860F09", 0 },
{ },
@@ -251,6 +274,7 @@ static struct platform_driver pwm_lpss_d
.driver = {
.name = "pwm-lpss",
.acpi_match_table = pwm_lpss_acpi_match,
+   .pm = _lpss_platform_pm_ops,
},
.probe = pwm_lpss_probe_platform,
.remove = pwm_lpss_remove_platform,

[PATCH 3.16 121/366] pwm: lpss: platform: Save/restore the ctrl register over a suspend/resume

[Ben Hutchings]

3.16.61-rc1 review patch.  If anyone has any objections, please let me know.

--

From: Hans de Goede 

commit 1d375b58c12f08d8570b30b865def4734517f04f upstream.

On some devices the contents of the ctrl register get lost over a
suspend/resume and the PWM comes back up disabled after the resume.

This is seen on some Bay Trail devices with the PWM in ACPI enumerated
mode, so it shows up as a platform device instead of a PCI device.

If we still think it is enabled and then try to change the duty-cycle
after this, we end up with a "PWM_SW_UPDATE was not cleared" error and
the PWM is stuck in that state from then on.

This commit adds suspend and resume pm callbacks to the pwm-lpss-platform
code, which save/restore the ctrl register over a suspend/resume, fixing
this.

Note that:

1) There is no need to do this over a runtime suspend, since we
only runtime suspend when disabled and then we properly set the enable
bit and reprogram the timings when we re-enable the PWM.

2) This may be happening on more systems then we realize, but has been
covered up sofar by a bug in the acpi-lpss.c code which was save/restoring
the regular device registers instead of the lpss private registers due to
lpss_device_desc.prv_offset not being set. This is fixed by a later patch
in this series.

Signed-off-by: Hans de Goede 
Reviewed-by: Andy Shevchenko 
Signed-off-by: Thierry Reding 
[bwh: Backported to 3.16:
 - pwm-lpss is a single module, so make the new functions static
 - Only one PWM per chip is supported; remove the npwm assertion and loops
 - Adjust filenames, context]
Signed-off-by: Ben Hutchings 
---
--- a/drivers/pwm/pwm-lpss.c
+++ b/drivers/pwm/pwm-lpss.c
@@ -39,6 +39,7 @@ struct pwm_lpss_chip {
void __iomem *regs;
struct clk *clk;
unsigned long clk_rate;
+   u32 saved_ctrl;
 };
 
 struct pwm_lpss_boardinfo {
@@ -177,6 +178,24 @@ static int pwm_lpss_remove(struct pwm_lp
return pwmchip_remove(>chip);
 }
 
+static int pwm_lpss_suspend(struct device *dev)
+{
+   struct pwm_lpss_chip *lpwm = dev_get_drvdata(dev);
+
+   lpwm->saved_ctrl = readl(lpwm->regs + PWM);
+
+   return 0;
+}
+
+static int pwm_lpss_resume(struct device *dev)
+{
+   struct pwm_lpss_chip *lpwm = dev_get_drvdata(dev);
+
+   writel(lpwm->saved_ctrl, lpwm->regs + PWM);
+
+   return 0;
+}
+
 static int pwm_lpss_probe_pci(struct pci_dev *pdev,
  const struct pci_device_id *id)
 {
@@ -241,6 +260,10 @@ static int pwm_lpss_remove_platform(stru
return pwm_lpss_remove(lpwm);
 }
 
+static SIMPLE_DEV_PM_OPS(pwm_lpss_platform_pm_ops,
+pwm_lpss_suspend,
+pwm_lpss_resume);
+
 static const struct acpi_device_id pwm_lpss_acpi_match[] = {
{ "80860F09", 0 },
{ },
@@ -251,6 +274,7 @@ static struct platform_driver pwm_lpss_d
.driver = {
.name = "pwm-lpss",
.acpi_match_table = pwm_lpss_acpi_match,
+   .pm = _lpss_platform_pm_ops,
},
.probe = pwm_lpss_probe_platform,
.remove = pwm_lpss_remove_platform,

[PATCH] gpio: mxc: move gpio noirq suspend/resume to syscore phase

[Anson Huang]

During noirq suspend/resume phase, GPIO irq could arrive
and its registers like IMR will be changed by irq handle
process, to make the GPIO registers exactly when it is
powered ON after resume, move the GPIO noirq suspend/resume
callback to syscore suspend/resume phase, local irq is
disabled at this phase so GPIO registers are atomic.

Signed-off-by: Anson Huang 
---
 drivers/gpio/gpio-mxc.c | 39 ---
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 5c69516..2d1dfa1 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -550,31 +551,38 @@ static void mxc_gpio_restore_regs(struct mxc_gpio_port 
*port)
writel(port->gpio_saved_reg.dr, port->base + GPIO_DR);
 }
 
-static int __maybe_unused mxc_gpio_noirq_suspend(struct device *dev)
+static int mxc_gpio_syscore_suspend(void)
 {
-   struct mxc_gpio_port *port = dev_get_drvdata(dev);
+   struct mxc_gpio_port *port;
 
-   mxc_gpio_save_regs(port);
-   clk_disable_unprepare(port->clk);
+   /* walk through all ports */
+   list_for_each_entry(port, _gpio_ports, node) {
+   mxc_gpio_save_regs(port);
+   clk_disable_unprepare(port->clk);
+   }
 
return 0;
 }
 
-static int __maybe_unused mxc_gpio_noirq_resume(struct device *dev)
+static void mxc_gpio_syscore_resume(void)
 {
-   struct mxc_gpio_port *port = dev_get_drvdata(dev);
+   struct mxc_gpio_port *port;
int ret;
 
-   ret = clk_prepare_enable(port->clk);
-   if (ret)
-   return ret;
-   mxc_gpio_restore_regs(port);
-
-   return 0;
+   /* walk through all ports */
+   list_for_each_entry(port, _gpio_ports, node) {
+   ret = clk_prepare_enable(port->clk);
+   if (ret) {
+   pr_err("mxc: failed to enable gpio clock %d\n", ret);
+   return;
+   }
+   mxc_gpio_restore_regs(port);
+   }
 }
 
-static const struct dev_pm_ops mxc_gpio_dev_pm_ops = {
-   SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(mxc_gpio_noirq_suspend, 
mxc_gpio_noirq_resume)
+static struct syscore_ops mxc_gpio_syscore_ops = {
+   .suspend = mxc_gpio_syscore_suspend,
+   .resume = mxc_gpio_syscore_resume,
 };
 
 static struct platform_driver mxc_gpio_driver = {
@@ -582,7 +590,6 @@ static struct platform_driver mxc_gpio_driver = {
.name   = "gpio-mxc",
.of_match_table = mxc_gpio_dt_ids,
.suppress_bind_attrs = true,
-   .pm = _gpio_dev_pm_ops,
},
.probe  = mxc_gpio_probe,
.id_table   = mxc_gpio_devtype,
@@ -590,6 +597,8 @@ static struct platform_driver mxc_gpio_driver = {
 
 static int __init gpio_mxc_init(void)
 {
+   register_syscore_ops(_gpio_syscore_ops);
+
return platform_driver_register(_gpio_driver);
 }
 subsys_initcall(gpio_mxc_init);
-- 
2.7.4

[PATCH] gpio: mxc: move gpio noirq suspend/resume to syscore phase

[Anson Huang]

During noirq suspend/resume phase, GPIO irq could arrive
and its registers like IMR will be changed by irq handle
process, to make the GPIO registers exactly when it is
powered ON after resume, move the GPIO noirq suspend/resume
callback to syscore suspend/resume phase, local irq is
disabled at this phase so GPIO registers are atomic.

Signed-off-by: Anson Huang 
---
 drivers/gpio/gpio-mxc.c | 39 ---
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 5c69516..2d1dfa1 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -550,31 +551,38 @@ static void mxc_gpio_restore_regs(struct mxc_gpio_port 
*port)
writel(port->gpio_saved_reg.dr, port->base + GPIO_DR);
 }
 
-static int __maybe_unused mxc_gpio_noirq_suspend(struct device *dev)
+static int mxc_gpio_syscore_suspend(void)
 {
-   struct mxc_gpio_port *port = dev_get_drvdata(dev);
+   struct mxc_gpio_port *port;
 
-   mxc_gpio_save_regs(port);
-   clk_disable_unprepare(port->clk);
+   /* walk through all ports */
+   list_for_each_entry(port, _gpio_ports, node) {
+   mxc_gpio_save_regs(port);
+   clk_disable_unprepare(port->clk);
+   }
 
return 0;
 }
 
-static int __maybe_unused mxc_gpio_noirq_resume(struct device *dev)
+static void mxc_gpio_syscore_resume(void)
 {
-   struct mxc_gpio_port *port = dev_get_drvdata(dev);
+   struct mxc_gpio_port *port;
int ret;
 
-   ret = clk_prepare_enable(port->clk);
-   if (ret)
-   return ret;
-   mxc_gpio_restore_regs(port);
-
-   return 0;
+   /* walk through all ports */
+   list_for_each_entry(port, _gpio_ports, node) {
+   ret = clk_prepare_enable(port->clk);
+   if (ret) {
+   pr_err("mxc: failed to enable gpio clock %d\n", ret);
+   return;
+   }
+   mxc_gpio_restore_regs(port);
+   }
 }
 
-static const struct dev_pm_ops mxc_gpio_dev_pm_ops = {
-   SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(mxc_gpio_noirq_suspend, 
mxc_gpio_noirq_resume)
+static struct syscore_ops mxc_gpio_syscore_ops = {
+   .suspend = mxc_gpio_syscore_suspend,
+   .resume = mxc_gpio_syscore_resume,
 };
 
 static struct platform_driver mxc_gpio_driver = {
@@ -582,7 +590,6 @@ static struct platform_driver mxc_gpio_driver = {
.name   = "gpio-mxc",
.of_match_table = mxc_gpio_dt_ids,
.suppress_bind_attrs = true,
-   .pm = _gpio_dev_pm_ops,
},
.probe  = mxc_gpio_probe,
.id_table   = mxc_gpio_devtype,
@@ -590,6 +597,8 @@ static struct platform_driver mxc_gpio_driver = {
 
 static int __init gpio_mxc_init(void)
 {
+   register_syscore_ops(_gpio_syscore_ops);
+
return platform_driver_register(_gpio_driver);
 }
 subsys_initcall(gpio_mxc_init);
-- 
2.7.4

[PATCH v2] PCI: imx: Add imx6sx suspend/resume support

[Leonard Crestez]

Enable PCI suspend/resume support on imx6sx socs. This is similar to
imx7d with a few differences:

* The PM_Turn_Off bit is exposed through an IOMUX GPR, like all other
pcie control bits on 6sx.
* The pcie_inbound_axi clk needs to be turned off in suspend. On resume
it is restored via resume -> deassert_core_reset -> enable_ref_clk.

Most of the resume logic is shared with the initial reset after probe.

Signed-off-by: Leonard Crestez 

---
Changes since v1:
* Use a switch statement in imx6_pcie_pm_turnoff. The DT-based turnoff
path is still an if statement.
* Did not split imx6_pcie_clk_disable or call it from other paths, this
would bring complications and is somewhat unrelated.
* See v1 comments: https://lore.kernel.org/patchwork/patch/996806/

 drivers/pci/controller/dwc/pci-imx6.c   | 44 ++---
 include/linux/mfd/syscon/imx6q-iomuxc-gpr.h |  1 +
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c 
b/drivers/pci/controller/dwc/pci-imx6.c
index 2cbef2d7c207..54625569d0bc 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -771,41 +771,75 @@ static void imx6_pcie_ltssm_disable(struct device *dev)
}
 }
 
 static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
 {
-   reset_control_assert(imx6_pcie->turnoff_reset);
-   reset_control_deassert(imx6_pcie->turnoff_reset);
+   struct device *dev = imx6_pcie->pci->dev;
+
+   /* Some variants have a turnoff reset in DT */
+   if (imx6_pcie->turnoff_reset) {
+   reset_control_assert(imx6_pcie->turnoff_reset);
+   reset_control_deassert(imx6_pcie->turnoff_reset);
+   goto pm_turnoff_sleep;
+   }
+
+   /* Others poke directly at IOMUXC registers */
+   switch (imx6_pcie->variant) {
+   case IMX6SX:
+   regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+   IMX6SX_GPR12_PCIE_PM_TURN_OFF,
+   IMX6SX_GPR12_PCIE_PM_TURN_OFF);
+   regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+   IMX6SX_GPR12_PCIE_PM_TURN_OFF, 0);
+   break;
+   default:
+   dev_err(dev, "PME_Turn_Off not implemented\n");
+   return;
+   }
 
/*
 * Components with an upstream port must respond to
 * PME_Turn_Off with PME_TO_Ack but we can't check.
 *
 * The standard recommends a 1-10ms timeout after which to
 * proceed anyway as if acks were received.
 */
+pm_turnoff_sleep:
usleep_range(1000, 1);
 }
 
 static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
 {
clk_disable_unprepare(imx6_pcie->pcie);
clk_disable_unprepare(imx6_pcie->pcie_phy);
clk_disable_unprepare(imx6_pcie->pcie_bus);
 
-   if (imx6_pcie->variant == IMX7D) {
+   switch (imx6_pcie->variant) {
+   case IMX6SX:
+   clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
+   break;
+   case IMX7D:
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
+   break;
+   default:
+   break;
}
 }
 
+static inline bool imx6_pcie_supports_suspend(struct imx6_pcie *imx6_pcie)
+{
+   return (imx6_pcie->variant == IMX7D ||
+   imx6_pcie->variant == IMX6SX);
+}
+
 static int imx6_pcie_suspend_noirq(struct device *dev)
 {
struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
 
-   if (imx6_pcie->variant != IMX7D)
+   if (!imx6_pcie_supports_suspend(imx6_pcie))
return 0;
 
imx6_pcie_pm_turnoff(imx6_pcie);
imx6_pcie_clk_disable(imx6_pcie);
imx6_pcie_ltssm_disable(dev);
@@ -817,11 +851,11 @@ static int imx6_pcie_resume_noirq(struct device *dev)
 {
int ret;
struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
struct pcie_port *pp = _pcie->pci->pp;
 
-   if (imx6_pcie->variant != IMX7D)
+   if (!imx6_pcie_supports_suspend(imx6_pcie))
return 0;
 
imx6_pcie_assert_core_reset(imx6_pcie);
imx6_pcie_init_phy(imx6_pcie);
imx6_pcie_deassert_core_reset(imx6_pcie);
diff --git a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h 
b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
index 6c1ad160ed87..c1b25f5e386d 100644
--- a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
+++ b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
@@ -438,10 +438,11 @@
 #define IMX6SX_GPR5_DISP_MUX_DCIC1_LCDIF1  (0x0 << 1)
 #define IMX6SX_GPR5_DISP_MUX_DCIC1_LVDS(0x1 << 1)
 #define IMX6SX_GPR5_DISP_MUX_DCIC1_MASK(0x1 << 1)
 

[PATCH v2] PCI: imx: Add imx6sx suspend/resume support

[Leonard Crestez]

Enable PCI suspend/resume support on imx6sx socs. This is similar to
imx7d with a few differences:

* The PM_Turn_Off bit is exposed through an IOMUX GPR, like all other
pcie control bits on 6sx.
* The pcie_inbound_axi clk needs to be turned off in suspend. On resume
it is restored via resume -> deassert_core_reset -> enable_ref_clk.

Most of the resume logic is shared with the initial reset after probe.

Signed-off-by: Leonard Crestez 

---
Changes since v1:
* Use a switch statement in imx6_pcie_pm_turnoff. The DT-based turnoff
path is still an if statement.
* Did not split imx6_pcie_clk_disable or call it from other paths, this
would bring complications and is somewhat unrelated.
* See v1 comments: https://lore.kernel.org/patchwork/patch/996806/

 drivers/pci/controller/dwc/pci-imx6.c   | 44 ++---
 include/linux/mfd/syscon/imx6q-iomuxc-gpr.h |  1 +
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c 
b/drivers/pci/controller/dwc/pci-imx6.c
index 2cbef2d7c207..54625569d0bc 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -771,41 +771,75 @@ static void imx6_pcie_ltssm_disable(struct device *dev)
}
 }
 
 static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
 {
-   reset_control_assert(imx6_pcie->turnoff_reset);
-   reset_control_deassert(imx6_pcie->turnoff_reset);
+   struct device *dev = imx6_pcie->pci->dev;
+
+   /* Some variants have a turnoff reset in DT */
+   if (imx6_pcie->turnoff_reset) {
+   reset_control_assert(imx6_pcie->turnoff_reset);
+   reset_control_deassert(imx6_pcie->turnoff_reset);
+   goto pm_turnoff_sleep;
+   }
+
+   /* Others poke directly at IOMUXC registers */
+   switch (imx6_pcie->variant) {
+   case IMX6SX:
+   regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+   IMX6SX_GPR12_PCIE_PM_TURN_OFF,
+   IMX6SX_GPR12_PCIE_PM_TURN_OFF);
+   regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+   IMX6SX_GPR12_PCIE_PM_TURN_OFF, 0);
+   break;
+   default:
+   dev_err(dev, "PME_Turn_Off not implemented\n");
+   return;
+   }
 
/*
 * Components with an upstream port must respond to
 * PME_Turn_Off with PME_TO_Ack but we can't check.
 *
 * The standard recommends a 1-10ms timeout after which to
 * proceed anyway as if acks were received.
 */
+pm_turnoff_sleep:
usleep_range(1000, 1);
 }
 
 static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
 {
clk_disable_unprepare(imx6_pcie->pcie);
clk_disable_unprepare(imx6_pcie->pcie_phy);
clk_disable_unprepare(imx6_pcie->pcie_bus);
 
-   if (imx6_pcie->variant == IMX7D) {
+   switch (imx6_pcie->variant) {
+   case IMX6SX:
+   clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
+   break;
+   case IMX7D:
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
+   break;
+   default:
+   break;
}
 }
 
+static inline bool imx6_pcie_supports_suspend(struct imx6_pcie *imx6_pcie)
+{
+   return (imx6_pcie->variant == IMX7D ||
+   imx6_pcie->variant == IMX6SX);
+}
+
 static int imx6_pcie_suspend_noirq(struct device *dev)
 {
struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
 
-   if (imx6_pcie->variant != IMX7D)
+   if (!imx6_pcie_supports_suspend(imx6_pcie))
return 0;
 
imx6_pcie_pm_turnoff(imx6_pcie);
imx6_pcie_clk_disable(imx6_pcie);
imx6_pcie_ltssm_disable(dev);
@@ -817,11 +851,11 @@ static int imx6_pcie_resume_noirq(struct device *dev)
 {
int ret;
struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
struct pcie_port *pp = _pcie->pci->pp;
 
-   if (imx6_pcie->variant != IMX7D)
+   if (!imx6_pcie_supports_suspend(imx6_pcie))
return 0;
 
imx6_pcie_assert_core_reset(imx6_pcie);
imx6_pcie_init_phy(imx6_pcie);
imx6_pcie_deassert_core_reset(imx6_pcie);
diff --git a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h 
b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
index 6c1ad160ed87..c1b25f5e386d 100644
--- a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
+++ b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
@@ -438,10 +438,11 @@
 #define IMX6SX_GPR5_DISP_MUX_DCIC1_LCDIF1  (0x0 << 1)
 #define IMX6SX_GPR5_DISP_MUX_DCIC1_LVDS(0x1 << 1)
 #define IMX6SX_GPR5_DISP_MUX_DCIC1_MASK(0x1 << 1)
 

Re: [PATCH] PCI: imx: Add imx6sx suspend/resume support

[Leonard Crestez]

On Thu, 2018-11-01 at 11:02 +0100, Philipp Zabel wrote:
> Hi Leonard,
> 
> On Wed, 2018-10-31 at 11:02 +, Leonard Crestez wrote:
> > On 10/8/2018 8:38 PM, Leonard Crestez wrote:
> > > Enable PCI suspend/resume support on imx6sx socs. This is similar to
> > > imx7d with a few differences:
> > > 
> > > * The PM_Turn_Off bit is exposed through an IOMUX GPR, like all other
> > > pcie control bits on 6sx.
> > > * The pcie_inbound_axi clk needs to be turned off in suspend. On resume
> > > it is restored via resume -> deassert_core_reset -> enable_ref_clk.
> > > 
> > > Most of the resume logic is shared with the initial reset after probe.

> > > + /*
> > > +  * Some variants have a turnoff reset in DT while
> > > +  * others poke at iomuxc registers.
> > > +  */
> > > + if (imx6_pcie->turnoff_reset) {
> > > + reset_control_assert(imx6_pcie->turnoff_reset);
> > > + reset_control_deassert(imx6_pcie->turnoff_reset);
> > > + } else if (imx6_pcie->variant == IMX6SX) {
> > > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > > + IMX6SX_GPR12_PCIE_PM_TURN_OFF,
> > > + IMX6SX_GPR12_PCIE_PM_TURN_OFF);
> > > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > > + IMX6SX_GPR12_PCIE_PM_TURN_OFF, 0);
> > > + } else {
> > > + dev_err(dev, "PME_Turn_Off not implemented\n");
> > > + return;
> > > + }
> 
> I'd use switch(imx6_pcie->variant) for consistency with the other places
> where different variants need to be handled separately.

Yes, this makes sense. But the only thing handle as a variant here
would be 6sx itself, for 7d if (imx6_pcie->turnoff_reset) still makes
sense.

This driver has quite a lot of long functions with switch statements,
maybe some day it should be converted to use a per-soc ops structure?

> > > @@ -790,22 +807,35 @@ static void imx6_pcie_clk_disable(struct imx6_pcie 
> > > *imx6_pcie)
> > >   {
> > >   clk_disable_unprepare(imx6_pcie->pcie);
> > >   clk_disable_unprepare(imx6_pcie->pcie_phy);
> > >   clk_disable_unprepare(imx6_pcie->pcie_bus);
> > >   
> > > - if (imx6_pcie->variant == IMX7D) {
> > > + switch (imx6_pcie->variant) {
> > > + case IMX6SX:
> > > + clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
> > > + break;
> > > + case IMX7D:
> > >   regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > >  IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
> > >  IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
> > > + break;
> > > + default:
> > > + break;
> > >   }
> 
> This disables the ref clock. Should this be split into a separate
> function imx6_pcie_disable_ref_clk() for symmetry with the already
> existing imx6_pcie_enable_ref_clk() ?

Not sure. The symmetry would be limited because enabling requirements
are more stringent than shutdown.

For example the mirror operation on imx7d is done in init_phy instead
of enable_ref_clk. I didn't test but I expect that moving refclk
selection around might violate HW init sequencing requirements so I'd
rather not poke at this.

> That could then also be used to disable pcie_inbound_axi in the error
> path of imx6_pcie_deassert_core_reset().

Not sure, clock disabling on error is also complicated.

In imx6_pcie_deassert_core_reset there is no error path after
enable_ref_clk so there would be nowhere to call disable_ref_clk
outside suspend. In theory imx7d phy pll could fail to lock but we just
print something instead of propagating the error.

If imx6_pcie_establish_link fails clocks could be turned off. It makes
sense to save power if no pcie card is inserted, right? This is what
the imx vendor tree does but my understanding is that this does not
pass compliance testing so I'd rather not upstream this behavior.

See 
https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/pci/host/pci-imx6.c?h=imx_4.9.88_2.0.0_ga#n1378

Cutting power if the pci slot is empty seems worthwhile but browsing
through other dwc-pci implementations and they don't seem to do this
either.

Instead clocks should stay on if link fails, I guess this is a
requirement for hotplug?

--
Regards,
Leonard

Re: [PATCH] PCI: imx: Add imx6sx suspend/resume support

[Leonard Crestez]

On Thu, 2018-11-01 at 11:02 +0100, Philipp Zabel wrote:
> Hi Leonard,
> 
> On Wed, 2018-10-31 at 11:02 +, Leonard Crestez wrote:
> > On 10/8/2018 8:38 PM, Leonard Crestez wrote:
> > > Enable PCI suspend/resume support on imx6sx socs. This is similar to
> > > imx7d with a few differences:
> > > 
> > > * The PM_Turn_Off bit is exposed through an IOMUX GPR, like all other
> > > pcie control bits on 6sx.
> > > * The pcie_inbound_axi clk needs to be turned off in suspend. On resume
> > > it is restored via resume -> deassert_core_reset -> enable_ref_clk.
> > > 
> > > Most of the resume logic is shared with the initial reset after probe.

> > > + /*
> > > +  * Some variants have a turnoff reset in DT while
> > > +  * others poke at iomuxc registers.
> > > +  */
> > > + if (imx6_pcie->turnoff_reset) {
> > > + reset_control_assert(imx6_pcie->turnoff_reset);
> > > + reset_control_deassert(imx6_pcie->turnoff_reset);
> > > + } else if (imx6_pcie->variant == IMX6SX) {
> > > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > > + IMX6SX_GPR12_PCIE_PM_TURN_OFF,
> > > + IMX6SX_GPR12_PCIE_PM_TURN_OFF);
> > > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > > + IMX6SX_GPR12_PCIE_PM_TURN_OFF, 0);
> > > + } else {
> > > + dev_err(dev, "PME_Turn_Off not implemented\n");
> > > + return;
> > > + }
> 
> I'd use switch(imx6_pcie->variant) for consistency with the other places
> where different variants need to be handled separately.

Yes, this makes sense. But the only thing handle as a variant here
would be 6sx itself, for 7d if (imx6_pcie->turnoff_reset) still makes
sense.

This driver has quite a lot of long functions with switch statements,
maybe some day it should be converted to use a per-soc ops structure?

> > > @@ -790,22 +807,35 @@ static void imx6_pcie_clk_disable(struct imx6_pcie 
> > > *imx6_pcie)
> > >   {
> > >   clk_disable_unprepare(imx6_pcie->pcie);
> > >   clk_disable_unprepare(imx6_pcie->pcie_phy);
> > >   clk_disable_unprepare(imx6_pcie->pcie_bus);
> > >   
> > > - if (imx6_pcie->variant == IMX7D) {
> > > + switch (imx6_pcie->variant) {
> > > + case IMX6SX:
> > > + clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
> > > + break;
> > > + case IMX7D:
> > >   regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > >  IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
> > >  IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
> > > + break;
> > > + default:
> > > + break;
> > >   }
> 
> This disables the ref clock. Should this be split into a separate
> function imx6_pcie_disable_ref_clk() for symmetry with the already
> existing imx6_pcie_enable_ref_clk() ?

Not sure. The symmetry would be limited because enabling requirements
are more stringent than shutdown.

For example the mirror operation on imx7d is done in init_phy instead
of enable_ref_clk. I didn't test but I expect that moving refclk
selection around might violate HW init sequencing requirements so I'd
rather not poke at this.

> That could then also be used to disable pcie_inbound_axi in the error
> path of imx6_pcie_deassert_core_reset().

Not sure, clock disabling on error is also complicated.

In imx6_pcie_deassert_core_reset there is no error path after
enable_ref_clk so there would be nowhere to call disable_ref_clk
outside suspend. In theory imx7d phy pll could fail to lock but we just
print something instead of propagating the error.

If imx6_pcie_establish_link fails clocks could be turned off. It makes
sense to save power if no pcie card is inserted, right? This is what
the imx vendor tree does but my understanding is that this does not
pass compliance testing so I'd rather not upstream this behavior.

See 
https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/pci/host/pci-imx6.c?h=imx_4.9.88_2.0.0_ga#n1378

Cutting power if the pci slot is empty seems worthwhile but browsing
through other dwc-pci implementations and they don't seem to do this
either.

Instead clocks should stay on if link fails, I guess this is a
requirement for hotplug?

--
Regards,
Leonard

Re: [PATCH] PCI: imx: Add imx6sx suspend/resume support

[Philipp Zabel]

Hi Leonard,

On Wed, 2018-10-31 at 11:02 +, Leonard Crestez wrote:
> On 10/8/2018 8:38 PM, Leonard Crestez wrote:
> > Enable PCI suspend/resume support on imx6sx socs. This is similar to
> > imx7d with a few differences:
> > 
> > * The PM_Turn_Off bit is exposed through an IOMUX GPR, like all other
> > pcie control bits on 6sx.
> > * The pcie_inbound_axi clk needs to be turned off in suspend. On resume
> > it is restored via resume -> deassert_core_reset -> enable_ref_clk.
> > 
> > Most of the resume logic is shared with the initial reset after probe.
> > 
> > Signed-off-by: Leonard Crestez 
> 
> This is a gentle reminder that this patch was posted ~3 weeks ago and 
> there is yet no reply. It's a mostly straight-forward extension of imx7d 
> pci suspend/resume support to a different SOC.
> 
> Lucas/Philipp: can you please take a brief look?

This is a bit out of my wheelhouse, but I'll try my best.

> > ---
> >   drivers/pci/controller/dwc/pci-imx6.c   | 40 ++---
> >   include/linux/mfd/syscon/imx6q-iomuxc-gpr.h |  1 +
> >   2 files changed, 36 insertions(+), 5 deletions(-)
> > 
> > Patch is again linux-next, meant to apply here:
> >   
> > https://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git/log/?h=pci/dwc
> > 
> > This is quite an old patch, mostly did it to prove that imx7d suspend
> > support is indeed generic.
> > 
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c 
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 2cbef2d7c207..6171171db1fc 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -771,12 +771,29 @@ static void imx6_pcie_ltssm_disable(struct device 
> > *dev)
> > }
> >   }
> >   
> >   static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
> >   {
> > -   reset_control_assert(imx6_pcie->turnoff_reset);
> > -   reset_control_deassert(imx6_pcie->turnoff_reset);
> > +   struct device *dev = imx6_pcie->pci->dev;
> > +
> > +   /*
> > +* Some variants have a turnoff reset in DT while
> > +* others poke at iomuxc registers.
> > +*/
> > +   if (imx6_pcie->turnoff_reset) {
> > +   reset_control_assert(imx6_pcie->turnoff_reset);
> > +   reset_control_deassert(imx6_pcie->turnoff_reset);
> > +   } else if (imx6_pcie->variant == IMX6SX) {
> > +   regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > +   IMX6SX_GPR12_PCIE_PM_TURN_OFF,
> > +   IMX6SX_GPR12_PCIE_PM_TURN_OFF);
> > +   regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > +   IMX6SX_GPR12_PCIE_PM_TURN_OFF, 0);
> > +   } else {
> > +   dev_err(dev, "PME_Turn_Off not implemented\n");
> > +   return;
> > +   }

I'd use switch(imx6_pcie->variant) for consistency with the other places
where different variants need to be handled separately.

> >   
> > /*
> >  * Components with an upstream port must respond to
> >  * PME_Turn_Off with PME_TO_Ack but we can't check.
> >  *
> > @@ -790,22 +807,35 @@ static void imx6_pcie_clk_disable(struct imx6_pcie 
> > *imx6_pcie)
> >   {
> > clk_disable_unprepare(imx6_pcie->pcie);
> > clk_disable_unprepare(imx6_pcie->pcie_phy);
> > clk_disable_unprepare(imx6_pcie->pcie_bus);
> >   
> > -   if (imx6_pcie->variant == IMX7D) {
> > +   switch (imx6_pcie->variant) {
> > +   case IMX6SX:
> > +   clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
> > +   break;
> > +   case IMX7D:
> > regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> >IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
> >IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
> > +   break;
> > +   default:
> > +   break;
> > }

This disables the ref clock. Should this be split into a separate
function imx6_pcie_disable_ref_clk() for symmetry with the already
existing imx6_pcie_enable_ref_clk() ?

That could then also be used to disable pcie_inbound_axi in the error
path of imx6_pcie_deassert_core_reset().

> >   }
> >   
> > +static inline bool imx6_pcie_supports_suspend(struct imx6_pcie *imx6_pcie)
> > +{
> > +   return (imx6_pcie->variant == IMX7D ||
> > +   imx6_pcie->variant == IMX6SX);
> > +}
> > +
> >   static int imx6_pcie_suspend_noirq(stru

Re: [PATCH] PCI: imx: Add imx6sx suspend/resume support

[Philipp Zabel]

Hi Leonard,

On Wed, 2018-10-31 at 11:02 +, Leonard Crestez wrote:
> On 10/8/2018 8:38 PM, Leonard Crestez wrote:
> > Enable PCI suspend/resume support on imx6sx socs. This is similar to
> > imx7d with a few differences:
> > 
> > * The PM_Turn_Off bit is exposed through an IOMUX GPR, like all other
> > pcie control bits on 6sx.
> > * The pcie_inbound_axi clk needs to be turned off in suspend. On resume
> > it is restored via resume -> deassert_core_reset -> enable_ref_clk.
> > 
> > Most of the resume logic is shared with the initial reset after probe.
> > 
> > Signed-off-by: Leonard Crestez 
> 
> This is a gentle reminder that this patch was posted ~3 weeks ago and 
> there is yet no reply. It's a mostly straight-forward extension of imx7d 
> pci suspend/resume support to a different SOC.
> 
> Lucas/Philipp: can you please take a brief look?

This is a bit out of my wheelhouse, but I'll try my best.

> > ---
> >   drivers/pci/controller/dwc/pci-imx6.c   | 40 ++---
> >   include/linux/mfd/syscon/imx6q-iomuxc-gpr.h |  1 +
> >   2 files changed, 36 insertions(+), 5 deletions(-)
> > 
> > Patch is again linux-next, meant to apply here:
> >   
> > https://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git/log/?h=pci/dwc
> > 
> > This is quite an old patch, mostly did it to prove that imx7d suspend
> > support is indeed generic.
> > 
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c 
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 2cbef2d7c207..6171171db1fc 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -771,12 +771,29 @@ static void imx6_pcie_ltssm_disable(struct device 
> > *dev)
> > }
> >   }
> >   
> >   static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
> >   {
> > -   reset_control_assert(imx6_pcie->turnoff_reset);
> > -   reset_control_deassert(imx6_pcie->turnoff_reset);
> > +   struct device *dev = imx6_pcie->pci->dev;
> > +
> > +   /*
> > +* Some variants have a turnoff reset in DT while
> > +* others poke at iomuxc registers.
> > +*/
> > +   if (imx6_pcie->turnoff_reset) {
> > +   reset_control_assert(imx6_pcie->turnoff_reset);
> > +   reset_control_deassert(imx6_pcie->turnoff_reset);
> > +   } else if (imx6_pcie->variant == IMX6SX) {
> > +   regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > +   IMX6SX_GPR12_PCIE_PM_TURN_OFF,
> > +   IMX6SX_GPR12_PCIE_PM_TURN_OFF);
> > +   regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > +   IMX6SX_GPR12_PCIE_PM_TURN_OFF, 0);
> > +   } else {
> > +   dev_err(dev, "PME_Turn_Off not implemented\n");
> > +   return;
> > +   }

I'd use switch(imx6_pcie->variant) for consistency with the other places
where different variants need to be handled separately.

> >   
> > /*
> >  * Components with an upstream port must respond to
> >  * PME_Turn_Off with PME_TO_Ack but we can't check.
> >  *
> > @@ -790,22 +807,35 @@ static void imx6_pcie_clk_disable(struct imx6_pcie 
> > *imx6_pcie)
> >   {
> > clk_disable_unprepare(imx6_pcie->pcie);
> > clk_disable_unprepare(imx6_pcie->pcie_phy);
> > clk_disable_unprepare(imx6_pcie->pcie_bus);
> >   
> > -   if (imx6_pcie->variant == IMX7D) {
> > +   switch (imx6_pcie->variant) {
> > +   case IMX6SX:
> > +   clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
> > +   break;
> > +   case IMX7D:
> > regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> >IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
> >IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
> > +   break;
> > +   default:
> > +   break;
> > }

This disables the ref clock. Should this be split into a separate
function imx6_pcie_disable_ref_clk() for symmetry with the already
existing imx6_pcie_enable_ref_clk() ?

That could then also be used to disable pcie_inbound_axi in the error
path of imx6_pcie_deassert_core_reset().

> >   }
> >   
> > +static inline bool imx6_pcie_supports_suspend(struct imx6_pcie *imx6_pcie)
> > +{
> > +   return (imx6_pcie->variant == IMX7D ||
> > +   imx6_pcie->variant == IMX6SX);
> > +}
> > +
> >   static int imx6_pcie_suspend_noirq(stru

Re: [PATCH] PCI: imx: Add imx6sx suspend/resume support

[Leonard Crestez]

On 10/8/2018 8:38 PM, Leonard Crestez wrote:
> Enable PCI suspend/resume support on imx6sx socs. This is similar to
> imx7d with a few differences:
> 
> * The PM_Turn_Off bit is exposed through an IOMUX GPR, like all other
> pcie control bits on 6sx.
> * The pcie_inbound_axi clk needs to be turned off in suspend. On resume
> it is restored via resume -> deassert_core_reset -> enable_ref_clk.
> 
> Most of the resume logic is shared with the initial reset after probe.
> 
> Signed-off-by: Leonard Crestez 

This is a gentle reminder that this patch was posted ~3 weeks ago and 
there is yet no reply. It's a mostly straight-forward extension of imx7d 
pci suspend/resume support to a different SOC.

Lucas/Philipp: can you please take a brief look?

> ---
>   drivers/pci/controller/dwc/pci-imx6.c   | 40 ++---
>   include/linux/mfd/syscon/imx6q-iomuxc-gpr.h |  1 +
>   2 files changed, 36 insertions(+), 5 deletions(-)
> 
> Patch is again linux-next, meant to apply here:
>   
> https://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git/log/?h=pci/dwc
> 
> This is quite an old patch, mostly did it to prove that imx7d suspend
> support is indeed generic.
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c 
> b/drivers/pci/controller/dwc/pci-imx6.c
> index 2cbef2d7c207..6171171db1fc 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -771,12 +771,29 @@ static void imx6_pcie_ltssm_disable(struct device *dev)
>   }
>   }
>   
>   static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
>   {
> - reset_control_assert(imx6_pcie->turnoff_reset);
> - reset_control_deassert(imx6_pcie->turnoff_reset);
> + struct device *dev = imx6_pcie->pci->dev;
> +
> + /*
> +  * Some variants have a turnoff reset in DT while
> +  * others poke at iomuxc registers.
> +  */
> + if (imx6_pcie->turnoff_reset) {
> + reset_control_assert(imx6_pcie->turnoff_reset);
> + reset_control_deassert(imx6_pcie->turnoff_reset);
> + } else if (imx6_pcie->variant == IMX6SX) {
> + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> + IMX6SX_GPR12_PCIE_PM_TURN_OFF,
> + IMX6SX_GPR12_PCIE_PM_TURN_OFF);
> + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> + IMX6SX_GPR12_PCIE_PM_TURN_OFF, 0);
> + } else {
> + dev_err(dev, "PME_Turn_Off not implemented\n");
> + return;
> + }
>   
>   /*
>* Components with an upstream port must respond to
>* PME_Turn_Off with PME_TO_Ack but we can't check.
>*
> @@ -790,22 +807,35 @@ static void imx6_pcie_clk_disable(struct imx6_pcie 
> *imx6_pcie)
>   {
>   clk_disable_unprepare(imx6_pcie->pcie);
>   clk_disable_unprepare(imx6_pcie->pcie_phy);
>   clk_disable_unprepare(imx6_pcie->pcie_bus);
>   
> - if (imx6_pcie->variant == IMX7D) {
> + switch (imx6_pcie->variant) {
> + case IMX6SX:
> + clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
> + break;
> + case IMX7D:
>   regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
>  IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
> + break;
> + default:
> + break;
>   }
>   }
>   
> +static inline bool imx6_pcie_supports_suspend(struct imx6_pcie *imx6_pcie)
> +{
> + return (imx6_pcie->variant == IMX7D ||
> + imx6_pcie->variant == IMX6SX);
> +}
> +
>   static int imx6_pcie_suspend_noirq(struct device *dev)
>   {
>   struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
>   
> - if (imx6_pcie->variant != IMX7D)
> + if (!imx6_pcie_supports_suspend(imx6_pcie))
>   return 0;
>   
>   imx6_pcie_pm_turnoff(imx6_pcie);
>   imx6_pcie_clk_disable(imx6_pcie);
>   imx6_pcie_ltssm_disable(dev);
> @@ -817,11 +847,11 @@ static int imx6_pcie_resume_noirq(struct device *dev)
>   {
>   int ret;
>   struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
>   struct pcie_port *pp = _pcie->pci->pp;
>   
> - if (imx6_pcie->variant != IMX7D)
> + if (!imx6_pcie_supports_suspend(imx6_pcie))
>   return 0;
>   
>   imx6_pcie_assert_core_reset(imx6_pcie);
>   imx6_pcie_init_phy(imx6_pcie);
>   imx6_pcie_deassert_core_reset(imx6_pcie);
> diff --git a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h 
> b/in

Re: [PATCH] PCI: imx: Add imx6sx suspend/resume support

[Leonard Crestez]

On 10/8/2018 8:38 PM, Leonard Crestez wrote:
> Enable PCI suspend/resume support on imx6sx socs. This is similar to
> imx7d with a few differences:
> 
> * The PM_Turn_Off bit is exposed through an IOMUX GPR, like all other
> pcie control bits on 6sx.
> * The pcie_inbound_axi clk needs to be turned off in suspend. On resume
> it is restored via resume -> deassert_core_reset -> enable_ref_clk.
> 
> Most of the resume logic is shared with the initial reset after probe.
> 
> Signed-off-by: Leonard Crestez 

This is a gentle reminder that this patch was posted ~3 weeks ago and 
there is yet no reply. It's a mostly straight-forward extension of imx7d 
pci suspend/resume support to a different SOC.

Lucas/Philipp: can you please take a brief look?

> ---
>   drivers/pci/controller/dwc/pci-imx6.c   | 40 ++---
>   include/linux/mfd/syscon/imx6q-iomuxc-gpr.h |  1 +
>   2 files changed, 36 insertions(+), 5 deletions(-)
> 
> Patch is again linux-next, meant to apply here:
>   
> https://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git/log/?h=pci/dwc
> 
> This is quite an old patch, mostly did it to prove that imx7d suspend
> support is indeed generic.
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c 
> b/drivers/pci/controller/dwc/pci-imx6.c
> index 2cbef2d7c207..6171171db1fc 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -771,12 +771,29 @@ static void imx6_pcie_ltssm_disable(struct device *dev)
>   }
>   }
>   
>   static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
>   {
> - reset_control_assert(imx6_pcie->turnoff_reset);
> - reset_control_deassert(imx6_pcie->turnoff_reset);
> + struct device *dev = imx6_pcie->pci->dev;
> +
> + /*
> +  * Some variants have a turnoff reset in DT while
> +  * others poke at iomuxc registers.
> +  */
> + if (imx6_pcie->turnoff_reset) {
> + reset_control_assert(imx6_pcie->turnoff_reset);
> + reset_control_deassert(imx6_pcie->turnoff_reset);
> + } else if (imx6_pcie->variant == IMX6SX) {
> + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> + IMX6SX_GPR12_PCIE_PM_TURN_OFF,
> + IMX6SX_GPR12_PCIE_PM_TURN_OFF);
> + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> + IMX6SX_GPR12_PCIE_PM_TURN_OFF, 0);
> + } else {
> + dev_err(dev, "PME_Turn_Off not implemented\n");
> + return;
> + }
>   
>   /*
>* Components with an upstream port must respond to
>* PME_Turn_Off with PME_TO_Ack but we can't check.
>*
> @@ -790,22 +807,35 @@ static void imx6_pcie_clk_disable(struct imx6_pcie 
> *imx6_pcie)
>   {
>   clk_disable_unprepare(imx6_pcie->pcie);
>   clk_disable_unprepare(imx6_pcie->pcie_phy);
>   clk_disable_unprepare(imx6_pcie->pcie_bus);
>   
> - if (imx6_pcie->variant == IMX7D) {
> + switch (imx6_pcie->variant) {
> + case IMX6SX:
> + clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
> + break;
> + case IMX7D:
>   regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
>  IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
> + break;
> + default:
> + break;
>   }
>   }
>   
> +static inline bool imx6_pcie_supports_suspend(struct imx6_pcie *imx6_pcie)
> +{
> + return (imx6_pcie->variant == IMX7D ||
> + imx6_pcie->variant == IMX6SX);
> +}
> +
>   static int imx6_pcie_suspend_noirq(struct device *dev)
>   {
>   struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
>   
> - if (imx6_pcie->variant != IMX7D)
> + if (!imx6_pcie_supports_suspend(imx6_pcie))
>   return 0;
>   
>   imx6_pcie_pm_turnoff(imx6_pcie);
>   imx6_pcie_clk_disable(imx6_pcie);
>   imx6_pcie_ltssm_disable(dev);
> @@ -817,11 +847,11 @@ static int imx6_pcie_resume_noirq(struct device *dev)
>   {
>   int ret;
>   struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
>   struct pcie_port *pp = _pcie->pci->pp;
>   
> - if (imx6_pcie->variant != IMX7D)
> + if (!imx6_pcie_supports_suspend(imx6_pcie))
>   return 0;
>   
>   imx6_pcie_assert_core_reset(imx6_pcie);
>   imx6_pcie_init_phy(imx6_pcie);
>   imx6_pcie_deassert_core_reset(imx6_pcie);
> diff --git a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h 
> b/in

[PATCH 5/9] mailbox: tegra-hsp: Add suspend/resume support

[Thierry Reding]

From: Thierry Reding 

Upon resuming from a system sleep state, the interrupts for all active
shared mailboxes need to be reenabled, otherwise they will not work.

Signed-off-by: Thierry Reding 
---
 drivers/mailbox/tegra-hsp.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c
index d070c8e38375..043f7173af8f 100644
--- a/drivers/mailbox/tegra-hsp.c
+++ b/drivers/mailbox/tegra-hsp.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -802,6 +803,23 @@ static int tegra_hsp_remove(struct platform_device *pdev)
return 0;
 }
 
+static int tegra_hsp_resume(struct device *dev)
+{
+   struct tegra_hsp *hsp = dev_get_drvdata(dev);
+   unsigned int i;
+
+   for (i = 0; i < hsp->num_sm; i++) {
+   struct tegra_hsp_mailbox *mb = >mailboxes[i];
+
+   if (mb->channel.chan->cl)
+   tegra_hsp_mailbox_startup(mb->channel.chan);
+   }
+
+   return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(tegra_hsp_pm_ops, NULL, tegra_hsp_resume);
+
 static const struct tegra_hsp_db_map tegra186_hsp_db_map[] = {
{ "ccplex", TEGRA_HSP_DB_MASTER_CCPLEX, HSP_DB_CCPLEX, },
{ "bpmp",   TEGRA_HSP_DB_MASTER_BPMP,   HSP_DB_BPMP,   },
@@ -821,6 +839,7 @@ static struct platform_driver tegra_hsp_driver = {
.driver = {
.name = "tegra-hsp",
.of_match_table = tegra_hsp_match,
+   .pm = _hsp_pm_ops,
},
.probe = tegra_hsp_probe,
.remove = tegra_hsp_remove,
-- 
2.19.1

[PATCH 5/9] mailbox: tegra-hsp: Add suspend/resume support

[Thierry Reding]

From: Thierry Reding 

Upon resuming from a system sleep state, the interrupts for all active
shared mailboxes need to be reenabled, otherwise they will not work.

Signed-off-by: Thierry Reding 
---
 drivers/mailbox/tegra-hsp.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c
index d070c8e38375..043f7173af8f 100644
--- a/drivers/mailbox/tegra-hsp.c
+++ b/drivers/mailbox/tegra-hsp.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -802,6 +803,23 @@ static int tegra_hsp_remove(struct platform_device *pdev)
return 0;
 }
 
+static int tegra_hsp_resume(struct device *dev)
+{
+   struct tegra_hsp *hsp = dev_get_drvdata(dev);
+   unsigned int i;
+
+   for (i = 0; i < hsp->num_sm; i++) {
+   struct tegra_hsp_mailbox *mb = >mailboxes[i];
+
+   if (mb->channel.chan->cl)
+   tegra_hsp_mailbox_startup(mb->channel.chan);
+   }
+
+   return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(tegra_hsp_pm_ops, NULL, tegra_hsp_resume);
+
 static const struct tegra_hsp_db_map tegra186_hsp_db_map[] = {
{ "ccplex", TEGRA_HSP_DB_MASTER_CCPLEX, HSP_DB_CCPLEX, },
{ "bpmp",   TEGRA_HSP_DB_MASTER_BPMP,   HSP_DB_BPMP,   },
@@ -821,6 +839,7 @@ static struct platform_driver tegra_hsp_driver = {
.driver = {
.name = "tegra-hsp",
.of_match_table = tegra_hsp_match,
+   .pm = _hsp_pm_ops,
},
.probe = tegra_hsp_probe,
.remove = tegra_hsp_remove,
-- 
2.19.1

Re: [PATCH v1] i2c: tegra: Remove suspend-resume

[Dmitry Osipenko]

On 10/17/18 10:41 PM, Jon Hunter wrote:
> 
> On 17/10/2018 15:30, Dmitry Osipenko wrote:
>> On 10/17/18 4:59 PM, Jon Hunter wrote:
>>>
>>> On 13/05/2018 22:13, Dmitry Osipenko wrote:
>>>> Nothing prevents I2C clients to access I2C while Tegra's driver is being
>>>> suspended, this results in -EBUSY error returned to the clients and that
>>>> may have unfortunate consequences. In particular this causes problems
>>>> for the TPS6586x MFD driver which emits hundreds of "failed to read
>>>> interrupt status" error messages on resume from suspend. This happens if
>>>> TPS6586X is used to wake system from suspend by the expired RTC alarm
>>>> timer because TPS6586X is an I2C device driver and its IRQ handler reads
>>>> the status register while Tegra's I2C driver is suspended, i.e. just after
>>>> kernel enabled IRQ's during of resume-from-suspend process.
>>>
>>> I have been looking at the above issue with the tps6586x because I am
>>> seeing delays on resume caused by this driver on the stable branches. I
>>> understand that this patch was dropped for stable, but looking at the
>>> specific issue with the tps6586x I am curious why the tps6586x driver
>>> was not fixed because it appears to me that the issue largely resides
>>> with that driver and any other device that uses the tps6586x is
>>> susceptible to it. I was able to fix the tps6586x driver by doing the
>>> following and I am interested in your thoughts ...
>>>
>>> Subject: [PATCH] mfd: tps6586x: Handle interrupts on suspend
>>>
>>> The tps6586x device is registered as an irqchip and the tps6586x-rtc
>>> interrupt is one of it's interrupt sources. When using the tps6586x-rtc
>>> as a wake-up device from suspend, the following is seen:
>>>
>>>  PM: Syncing filesystems ... done.
>>>  Freezing user space processes ... (elapsed 0.001 seconds) done.
>>>  OOM killer disabled.
>>>  Freezing remaining freezable tasks ... (elapsed 0.000 seconds) done.
>>>  Disabling non-boot CPUs ...
>>>  Entering suspend state LP1
>>>  Enabling non-boot CPUs ...
>>>  CPU1 is up
>>>  tps6586x 3-0034: failed to read interrupt status
>>>  tps6586x 3-0034: failed to read interrupt status
>>>
>>> The reason why the tps6586x interrupt status cannot be read is because
>>> the tps6586x interrupt is not masked during suspend and when the
>>> tps6586x-rtc interrupt occurs, to wake-up the device, the interrupt is
>>> seen before the i2c controller has been resumed in order to read the
>>> tps6586x interrupt status.
>>>
>>> The tps6586x-rtc driver sets it's interrupt as a wake-up source during
>>> suspend, which gets propagated to the parent tps6586x interrupt.
>>> However, the tps6586x-rtc driver cannot disable it's interrupt during
>>> suspend otherwise we would never be woken up and so the tps6586x must
>>> disable it's interrupt instead.
>>>
>>> Fix this by disabling the tps6586x interrupt on entering suspend and
>>> re-enabling it on resuming from suspend.
>>
>> Looks like it should work, but I vaguely recalling that something didn't 
>> work after disabling of IRQ on suspend. Maybe wakeup was getting disabled, 
>> but seems it is working fine now. Patch is good to me if you're going to 
>> propose it for backporting, but you should test that it works properly with 
>> all of stable kernels.
> 
> Indeed, I have been setting up some automated testing of various stable
> branches (mainly 4.x LTS releases) and I am seeing this problem there.
> Furthermore, I am using this to validate the change as well.
> 
>> I just found this [0], seems your patch need to address the same review 
>> comment.
>>
>> [0] https://lkml.org/lkml/2011/3/29/18
> 
> Thanks will do.
> 
> I know we don't support low power modes (ie. LP0), however, I do wonder
> if we should have some sort of i2c suspend/resume handler for Tegra?
> Eventually we will need this.

I'd suggest to support LP0 in the core first and only then to start making 
necessary suspend/resume changes in the drivers, otherwise it may end up being 
wasted time and effort for you and other maintainers.

Re: [PATCH v1] i2c: tegra: Remove suspend-resume

[Dmitry Osipenko]

On 10/17/18 10:41 PM, Jon Hunter wrote:
> 
> On 17/10/2018 15:30, Dmitry Osipenko wrote:
>> On 10/17/18 4:59 PM, Jon Hunter wrote:
>>>
>>> On 13/05/2018 22:13, Dmitry Osipenko wrote:
>>>> Nothing prevents I2C clients to access I2C while Tegra's driver is being
>>>> suspended, this results in -EBUSY error returned to the clients and that
>>>> may have unfortunate consequences. In particular this causes problems
>>>> for the TPS6586x MFD driver which emits hundreds of "failed to read
>>>> interrupt status" error messages on resume from suspend. This happens if
>>>> TPS6586X is used to wake system from suspend by the expired RTC alarm
>>>> timer because TPS6586X is an I2C device driver and its IRQ handler reads
>>>> the status register while Tegra's I2C driver is suspended, i.e. just after
>>>> kernel enabled IRQ's during of resume-from-suspend process.
>>>
>>> I have been looking at the above issue with the tps6586x because I am
>>> seeing delays on resume caused by this driver on the stable branches. I
>>> understand that this patch was dropped for stable, but looking at the
>>> specific issue with the tps6586x I am curious why the tps6586x driver
>>> was not fixed because it appears to me that the issue largely resides
>>> with that driver and any other device that uses the tps6586x is
>>> susceptible to it. I was able to fix the tps6586x driver by doing the
>>> following and I am interested in your thoughts ...
>>>
>>> Subject: [PATCH] mfd: tps6586x: Handle interrupts on suspend
>>>
>>> The tps6586x device is registered as an irqchip and the tps6586x-rtc
>>> interrupt is one of it's interrupt sources. When using the tps6586x-rtc
>>> as a wake-up device from suspend, the following is seen:
>>>
>>>  PM: Syncing filesystems ... done.
>>>  Freezing user space processes ... (elapsed 0.001 seconds) done.
>>>  OOM killer disabled.
>>>  Freezing remaining freezable tasks ... (elapsed 0.000 seconds) done.
>>>  Disabling non-boot CPUs ...
>>>  Entering suspend state LP1
>>>  Enabling non-boot CPUs ...
>>>  CPU1 is up
>>>  tps6586x 3-0034: failed to read interrupt status
>>>  tps6586x 3-0034: failed to read interrupt status
>>>
>>> The reason why the tps6586x interrupt status cannot be read is because
>>> the tps6586x interrupt is not masked during suspend and when the
>>> tps6586x-rtc interrupt occurs, to wake-up the device, the interrupt is
>>> seen before the i2c controller has been resumed in order to read the
>>> tps6586x interrupt status.
>>>
>>> The tps6586x-rtc driver sets it's interrupt as a wake-up source during
>>> suspend, which gets propagated to the parent tps6586x interrupt.
>>> However, the tps6586x-rtc driver cannot disable it's interrupt during
>>> suspend otherwise we would never be woken up and so the tps6586x must
>>> disable it's interrupt instead.
>>>
>>> Fix this by disabling the tps6586x interrupt on entering suspend and
>>> re-enabling it on resuming from suspend.
>>
>> Looks like it should work, but I vaguely recalling that something didn't 
>> work after disabling of IRQ on suspend. Maybe wakeup was getting disabled, 
>> but seems it is working fine now. Patch is good to me if you're going to 
>> propose it for backporting, but you should test that it works properly with 
>> all of stable kernels.
> 
> Indeed, I have been setting up some automated testing of various stable
> branches (mainly 4.x LTS releases) and I am seeing this problem there.
> Furthermore, I am using this to validate the change as well.
> 
>> I just found this [0], seems your patch need to address the same review 
>> comment.
>>
>> [0] https://lkml.org/lkml/2011/3/29/18
> 
> Thanks will do.
> 
> I know we don't support low power modes (ie. LP0), however, I do wonder
> if we should have some sort of i2c suspend/resume handler for Tegra?
> Eventually we will need this.

I'd suggest to support LP0 in the core first and only then to start making 
necessary suspend/resume changes in the drivers, otherwise it may end up being 
wasted time and effort for you and other maintainers.

Re: [PATCH v1] i2c: tegra: Remove suspend-resume

[Jon Hunter]

On 17/10/2018 15:30, Dmitry Osipenko wrote:
> On 10/17/18 4:59 PM, Jon Hunter wrote:
>>
>> On 13/05/2018 22:13, Dmitry Osipenko wrote:
>>> Nothing prevents I2C clients to access I2C while Tegra's driver is being
>>> suspended, this results in -EBUSY error returned to the clients and that
>>> may have unfortunate consequences. In particular this causes problems
>>> for the TPS6586x MFD driver which emits hundreds of "failed to read
>>> interrupt status" error messages on resume from suspend. This happens if
>>> TPS6586X is used to wake system from suspend by the expired RTC alarm
>>> timer because TPS6586X is an I2C device driver and its IRQ handler reads
>>> the status register while Tegra's I2C driver is suspended, i.e. just after
>>> kernel enabled IRQ's during of resume-from-suspend process.
>>
>> I have been looking at the above issue with the tps6586x because I am
>> seeing delays on resume caused by this driver on the stable branches. I
>> understand that this patch was dropped for stable, but looking at the
>> specific issue with the tps6586x I am curious why the tps6586x driver
>> was not fixed because it appears to me that the issue largely resides
>> with that driver and any other device that uses the tps6586x is
>> susceptible to it. I was able to fix the tps6586x driver by doing the
>> following and I am interested in your thoughts ...
>>
>> Subject: [PATCH] mfd: tps6586x: Handle interrupts on suspend
>>
>> The tps6586x device is registered as an irqchip and the tps6586x-rtc
>> interrupt is one of it's interrupt sources. When using the tps6586x-rtc
>> as a wake-up device from suspend, the following is seen:
>>
>>  PM: Syncing filesystems ... done.
>>  Freezing user space processes ... (elapsed 0.001 seconds) done.
>>  OOM killer disabled.
>>  Freezing remaining freezable tasks ... (elapsed 0.000 seconds) done.
>>  Disabling non-boot CPUs ...
>>  Entering suspend state LP1
>>  Enabling non-boot CPUs ...
>>  CPU1 is up
>>  tps6586x 3-0034: failed to read interrupt status
>>  tps6586x 3-0034: failed to read interrupt status
>>
>> The reason why the tps6586x interrupt status cannot be read is because
>> the tps6586x interrupt is not masked during suspend and when the
>> tps6586x-rtc interrupt occurs, to wake-up the device, the interrupt is
>> seen before the i2c controller has been resumed in order to read the
>> tps6586x interrupt status.
>>
>> The tps6586x-rtc driver sets it's interrupt as a wake-up source during
>> suspend, which gets propagated to the parent tps6586x interrupt.
>> However, the tps6586x-rtc driver cannot disable it's interrupt during
>> suspend otherwise we would never be woken up and so the tps6586x must
>> disable it's interrupt instead.
>>
>> Fix this by disabling the tps6586x interrupt on entering suspend and
>> re-enabling it on resuming from suspend.
> 
> Looks like it should work, but I vaguely recalling that something didn't work 
> after disabling of IRQ on suspend. Maybe wakeup was getting disabled, but 
> seems it is working fine now. Patch is good to me if you're going to propose 
> it for backporting, but you should test that it works properly with all of 
> stable kernels.

Indeed, I have been setting up some automated testing of various stable
branches (mainly 4.x LTS releases) and I am seeing this problem there.
Furthermore, I am using this to validate the change as well.

> I just found this [0], seems your patch need to address the same review 
> comment.
> 
> [0] https://lkml.org/lkml/2011/3/29/18

Thanks will do.

I know we don't support low power modes (ie. LP0), however, I do wonder
if we should have some sort of i2c suspend/resume handler for Tegra?
Eventually we will need this.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v1] i2c: tegra: Remove suspend-resume

[Jon Hunter]

On 17/10/2018 15:30, Dmitry Osipenko wrote:
> On 10/17/18 4:59 PM, Jon Hunter wrote:
>>
>> On 13/05/2018 22:13, Dmitry Osipenko wrote:
>>> Nothing prevents I2C clients to access I2C while Tegra's driver is being
>>> suspended, this results in -EBUSY error returned to the clients and that
>>> may have unfortunate consequences. In particular this causes problems
>>> for the TPS6586x MFD driver which emits hundreds of "failed to read
>>> interrupt status" error messages on resume from suspend. This happens if
>>> TPS6586X is used to wake system from suspend by the expired RTC alarm
>>> timer because TPS6586X is an I2C device driver and its IRQ handler reads
>>> the status register while Tegra's I2C driver is suspended, i.e. just after
>>> kernel enabled IRQ's during of resume-from-suspend process.
>>
>> I have been looking at the above issue with the tps6586x because I am
>> seeing delays on resume caused by this driver on the stable branches. I
>> understand that this patch was dropped for stable, but looking at the
>> specific issue with the tps6586x I am curious why the tps6586x driver
>> was not fixed because it appears to me that the issue largely resides
>> with that driver and any other device that uses the tps6586x is
>> susceptible to it. I was able to fix the tps6586x driver by doing the
>> following and I am interested in your thoughts ...
>>
>> Subject: [PATCH] mfd: tps6586x: Handle interrupts on suspend
>>
>> The tps6586x device is registered as an irqchip and the tps6586x-rtc
>> interrupt is one of it's interrupt sources. When using the tps6586x-rtc
>> as a wake-up device from suspend, the following is seen:
>>
>>  PM: Syncing filesystems ... done.
>>  Freezing user space processes ... (elapsed 0.001 seconds) done.
>>  OOM killer disabled.
>>  Freezing remaining freezable tasks ... (elapsed 0.000 seconds) done.
>>  Disabling non-boot CPUs ...
>>  Entering suspend state LP1
>>  Enabling non-boot CPUs ...
>>  CPU1 is up
>>  tps6586x 3-0034: failed to read interrupt status
>>  tps6586x 3-0034: failed to read interrupt status
>>
>> The reason why the tps6586x interrupt status cannot be read is because
>> the tps6586x interrupt is not masked during suspend and when the
>> tps6586x-rtc interrupt occurs, to wake-up the device, the interrupt is
>> seen before the i2c controller has been resumed in order to read the
>> tps6586x interrupt status.
>>
>> The tps6586x-rtc driver sets it's interrupt as a wake-up source during
>> suspend, which gets propagated to the parent tps6586x interrupt.
>> However, the tps6586x-rtc driver cannot disable it's interrupt during
>> suspend otherwise we would never be woken up and so the tps6586x must
>> disable it's interrupt instead.
>>
>> Fix this by disabling the tps6586x interrupt on entering suspend and
>> re-enabling it on resuming from suspend.
> 
> Looks like it should work, but I vaguely recalling that something didn't work 
> after disabling of IRQ on suspend. Maybe wakeup was getting disabled, but 
> seems it is working fine now. Patch is good to me if you're going to propose 
> it for backporting, but you should test that it works properly with all of 
> stable kernels.

Indeed, I have been setting up some automated testing of various stable
branches (mainly 4.x LTS releases) and I am seeing this problem there.
Furthermore, I am using this to validate the change as well.

> I just found this [0], seems your patch need to address the same review 
> comment.
> 
> [0] https://lkml.org/lkml/2011/3/29/18

Thanks will do.

I know we don't support low power modes (ie. LP0), however, I do wonder
if we should have some sort of i2c suspend/resume handler for Tegra?
Eventually we will need this.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v1] i2c: tegra: Remove suspend-resume

[Dmitry Osipenko]

On 10/17/18 4:59 PM, Jon Hunter wrote:
> 
> On 13/05/2018 22:13, Dmitry Osipenko wrote:
>> Nothing prevents I2C clients to access I2C while Tegra's driver is being
>> suspended, this results in -EBUSY error returned to the clients and that
>> may have unfortunate consequences. In particular this causes problems
>> for the TPS6586x MFD driver which emits hundreds of "failed to read
>> interrupt status" error messages on resume from suspend. This happens if
>> TPS6586X is used to wake system from suspend by the expired RTC alarm
>> timer because TPS6586X is an I2C device driver and its IRQ handler reads
>> the status register while Tegra's I2C driver is suspended, i.e. just after
>> kernel enabled IRQ's during of resume-from-suspend process.
> 
> I have been looking at the above issue with the tps6586x because I am
> seeing delays on resume caused by this driver on the stable branches. I
> understand that this patch was dropped for stable, but looking at the
> specific issue with the tps6586x I am curious why the tps6586x driver
> was not fixed because it appears to me that the issue largely resides
> with that driver and any other device that uses the tps6586x is
> susceptible to it. I was able to fix the tps6586x driver by doing the
> following and I am interested in your thoughts ...
> 
> Subject: [PATCH] mfd: tps6586x: Handle interrupts on suspend
> 
> The tps6586x device is registered as an irqchip and the tps6586x-rtc
> interrupt is one of it's interrupt sources. When using the tps6586x-rtc
> as a wake-up device from suspend, the following is seen:
> 
>  PM: Syncing filesystems ... done.
>  Freezing user space processes ... (elapsed 0.001 seconds) done.
>  OOM killer disabled.
>  Freezing remaining freezable tasks ... (elapsed 0.000 seconds) done.
>  Disabling non-boot CPUs ...
>  Entering suspend state LP1
>  Enabling non-boot CPUs ...
>  CPU1 is up
>  tps6586x 3-0034: failed to read interrupt status
>  tps6586x 3-0034: failed to read interrupt status
> 
> The reason why the tps6586x interrupt status cannot be read is because
> the tps6586x interrupt is not masked during suspend and when the
> tps6586x-rtc interrupt occurs, to wake-up the device, the interrupt is
> seen before the i2c controller has been resumed in order to read the
> tps6586x interrupt status.
> 
> The tps6586x-rtc driver sets it's interrupt as a wake-up source during
> suspend, which gets propagated to the parent tps6586x interrupt.
> However, the tps6586x-rtc driver cannot disable it's interrupt during
> suspend otherwise we would never be woken up and so the tps6586x must
> disable it's interrupt instead.
> 
> Fix this by disabling the tps6586x interrupt on entering suspend and
> re-enabling it on resuming from suspend.

Looks like it should work, but I vaguely recalling that something didn't work 
after disabling of IRQ on suspend. Maybe wakeup was getting disabled, but seems 
it is working fine now. Patch is good to me if you're going to propose it for 
backporting, but you should test that it works properly with all of stable 
kernels.

I just found this [0], seems your patch need to address the same review comment.

[0] https://lkml.org/lkml/2011/3/29/18

Re: [PATCH v1] i2c: tegra: Remove suspend-resume

[Dmitry Osipenko]

On 10/17/18 4:59 PM, Jon Hunter wrote:
> 
> On 13/05/2018 22:13, Dmitry Osipenko wrote:
>> Nothing prevents I2C clients to access I2C while Tegra's driver is being
>> suspended, this results in -EBUSY error returned to the clients and that
>> may have unfortunate consequences. In particular this causes problems
>> for the TPS6586x MFD driver which emits hundreds of "failed to read
>> interrupt status" error messages on resume from suspend. This happens if
>> TPS6586X is used to wake system from suspend by the expired RTC alarm
>> timer because TPS6586X is an I2C device driver and its IRQ handler reads
>> the status register while Tegra's I2C driver is suspended, i.e. just after
>> kernel enabled IRQ's during of resume-from-suspend process.
> 
> I have been looking at the above issue with the tps6586x because I am
> seeing delays on resume caused by this driver on the stable branches. I
> understand that this patch was dropped for stable, but looking at the
> specific issue with the tps6586x I am curious why the tps6586x driver
> was not fixed because it appears to me that the issue largely resides
> with that driver and any other device that uses the tps6586x is
> susceptible to it. I was able to fix the tps6586x driver by doing the
> following and I am interested in your thoughts ...
> 
> Subject: [PATCH] mfd: tps6586x: Handle interrupts on suspend
> 
> The tps6586x device is registered as an irqchip and the tps6586x-rtc
> interrupt is one of it's interrupt sources. When using the tps6586x-rtc
> as a wake-up device from suspend, the following is seen:
> 
>  PM: Syncing filesystems ... done.
>  Freezing user space processes ... (elapsed 0.001 seconds) done.
>  OOM killer disabled.
>  Freezing remaining freezable tasks ... (elapsed 0.000 seconds) done.
>  Disabling non-boot CPUs ...
>  Entering suspend state LP1
>  Enabling non-boot CPUs ...
>  CPU1 is up
>  tps6586x 3-0034: failed to read interrupt status
>  tps6586x 3-0034: failed to read interrupt status
> 
> The reason why the tps6586x interrupt status cannot be read is because
> the tps6586x interrupt is not masked during suspend and when the
> tps6586x-rtc interrupt occurs, to wake-up the device, the interrupt is
> seen before the i2c controller has been resumed in order to read the
> tps6586x interrupt status.
> 
> The tps6586x-rtc driver sets it's interrupt as a wake-up source during
> suspend, which gets propagated to the parent tps6586x interrupt.
> However, the tps6586x-rtc driver cannot disable it's interrupt during
> suspend otherwise we would never be woken up and so the tps6586x must
> disable it's interrupt instead.
> 
> Fix this by disabling the tps6586x interrupt on entering suspend and
> re-enabling it on resuming from suspend.

Looks like it should work, but I vaguely recalling that something didn't work 
after disabling of IRQ on suspend. Maybe wakeup was getting disabled, but seems 
it is working fine now. Patch is good to me if you're going to propose it for 
backporting, but you should test that it works properly with all of stable 
kernels.

I just found this [0], seems your patch need to address the same review comment.

[0] https://lkml.org/lkml/2011/3/29/18

Re: [PATCH v1] i2c: tegra: Remove suspend-resume

[Jon Hunter]

On 13/05/2018 22:13, Dmitry Osipenko wrote:
> Nothing prevents I2C clients to access I2C while Tegra's driver is being
> suspended, this results in -EBUSY error returned to the clients and that
> may have unfortunate consequences. In particular this causes problems
> for the TPS6586x MFD driver which emits hundreds of "failed to read
> interrupt status" error messages on resume from suspend. This happens if
> TPS6586X is used to wake system from suspend by the expired RTC alarm
> timer because TPS6586X is an I2C device driver and its IRQ handler reads
> the status register while Tegra's I2C driver is suspended, i.e. just after
> kernel enabled IRQ's during of resume-from-suspend process.

I have been looking at the above issue with the tps6586x because I am
seeing delays on resume caused by this driver on the stable branches. I
understand that this patch was dropped for stable, but looking at the
specific issue with the tps6586x I am curious why the tps6586x driver
was not fixed because it appears to me that the issue largely resides
with that driver and any other device that uses the tps6586x is
susceptible to it. I was able to fix the tps6586x driver by doing the
following and I am interested in your thoughts ...

Subject: [PATCH] mfd: tps6586x: Handle interrupts on suspend

The tps6586x device is registered as an irqchip and the tps6586x-rtc
interrupt is one of it's interrupt sources. When using the tps6586x-rtc
as a wake-up device from suspend, the following is seen:

 PM: Syncing filesystems ... done.
 Freezing user space processes ... (elapsed 0.001 seconds) done.
 OOM killer disabled.
 Freezing remaining freezable tasks ... (elapsed 0.000 seconds) done.
 Disabling non-boot CPUs ...
 Entering suspend state LP1
 Enabling non-boot CPUs ...
 CPU1 is up
 tps6586x 3-0034: failed to read interrupt status
 tps6586x 3-0034: failed to read interrupt status

The reason why the tps6586x interrupt status cannot be read is because
the tps6586x interrupt is not masked during suspend and when the
tps6586x-rtc interrupt occurs, to wake-up the device, the interrupt is
seen before the i2c controller has been resumed in order to read the
tps6586x interrupt status.

The tps6586x-rtc driver sets it's interrupt as a wake-up source during
suspend, which gets propagated to the parent tps6586x interrupt.
However, the tps6586x-rtc driver cannot disable it's interrupt during
suspend otherwise we would never be woken up and so the tps6586x must
disable it's interrupt instead.

Fix this by disabling the tps6586x interrupt on entering suspend and
re-enabling it on resuming from suspend.
---
 drivers/mfd/tps6586x.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c
index 5628a6b5b19b..165ac8a3d22c 100644
--- a/drivers/mfd/tps6586x.c
+++ b/drivers/mfd/tps6586x.c
@@ -594,6 +594,27 @@ static int tps6586x_i2c_remove(struct i2c_client
*client)
return 0;
 }

+static int __maybe_unused tps6586x_i2c_suspend(struct device *dev)
+{
+   struct tps6586x *tps6586x = dev_get_drvdata(dev);
+
+   disable_irq(tps6586x->client->irq);
+
+   return 0;
+}
+
+static int __maybe_unused tps6586x_i2c_resume(struct device *dev)
+{
+   struct tps6586x *tps6586x = dev_get_drvdata(dev);
+
+   enable_irq(tps6586x->client->irq);
+
+   return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(tps6586x_pm_ops, tps6586x_i2c_suspend,
+tps6586x_i2c_resume);
+
 static const struct i2c_device_id tps6586x_id_table[] = {
{ "tps6586x", 0 },
{ },
@@ -604,6 +625,7 @@ static int tps6586x_i2c_remove(struct i2c_client
*client)
.driver = {
.name   = "tps6586x",
.of_match_table = of_match_ptr(tps6586x_of_match),
+   .pm = _pm_ops,
},
.probe  = tps6586x_i2c_probe,
.remove = tps6586x_i2c_remove,
-- 
1.9.1

-- 
nvpublic

Re: [PATCH v1] i2c: tegra: Remove suspend-resume

[Jon Hunter]

On 13/05/2018 22:13, Dmitry Osipenko wrote:
> Nothing prevents I2C clients to access I2C while Tegra's driver is being
> suspended, this results in -EBUSY error returned to the clients and that
> may have unfortunate consequences. In particular this causes problems
> for the TPS6586x MFD driver which emits hundreds of "failed to read
> interrupt status" error messages on resume from suspend. This happens if
> TPS6586X is used to wake system from suspend by the expired RTC alarm
> timer because TPS6586X is an I2C device driver and its IRQ handler reads
> the status register while Tegra's I2C driver is suspended, i.e. just after
> kernel enabled IRQ's during of resume-from-suspend process.

I have been looking at the above issue with the tps6586x because I am
seeing delays on resume caused by this driver on the stable branches. I
understand that this patch was dropped for stable, but looking at the
specific issue with the tps6586x I am curious why the tps6586x driver
was not fixed because it appears to me that the issue largely resides
with that driver and any other device that uses the tps6586x is
susceptible to it. I was able to fix the tps6586x driver by doing the
following and I am interested in your thoughts ...

Subject: [PATCH] mfd: tps6586x: Handle interrupts on suspend

The tps6586x device is registered as an irqchip and the tps6586x-rtc
interrupt is one of it's interrupt sources. When using the tps6586x-rtc
as a wake-up device from suspend, the following is seen:

 PM: Syncing filesystems ... done.
 Freezing user space processes ... (elapsed 0.001 seconds) done.
 OOM killer disabled.
 Freezing remaining freezable tasks ... (elapsed 0.000 seconds) done.
 Disabling non-boot CPUs ...
 Entering suspend state LP1
 Enabling non-boot CPUs ...
 CPU1 is up
 tps6586x 3-0034: failed to read interrupt status
 tps6586x 3-0034: failed to read interrupt status

The reason why the tps6586x interrupt status cannot be read is because
the tps6586x interrupt is not masked during suspend and when the
tps6586x-rtc interrupt occurs, to wake-up the device, the interrupt is
seen before the i2c controller has been resumed in order to read the
tps6586x interrupt status.

The tps6586x-rtc driver sets it's interrupt as a wake-up source during
suspend, which gets propagated to the parent tps6586x interrupt.
However, the tps6586x-rtc driver cannot disable it's interrupt during
suspend otherwise we would never be woken up and so the tps6586x must
disable it's interrupt instead.

Fix this by disabling the tps6586x interrupt on entering suspend and
re-enabling it on resuming from suspend.
---
 drivers/mfd/tps6586x.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c
index 5628a6b5b19b..165ac8a3d22c 100644
--- a/drivers/mfd/tps6586x.c
+++ b/drivers/mfd/tps6586x.c
@@ -594,6 +594,27 @@ static int tps6586x_i2c_remove(struct i2c_client
*client)
return 0;
 }

+static int __maybe_unused tps6586x_i2c_suspend(struct device *dev)
+{
+   struct tps6586x *tps6586x = dev_get_drvdata(dev);
+
+   disable_irq(tps6586x->client->irq);
+
+   return 0;
+}
+
+static int __maybe_unused tps6586x_i2c_resume(struct device *dev)
+{
+   struct tps6586x *tps6586x = dev_get_drvdata(dev);
+
+   enable_irq(tps6586x->client->irq);
+
+   return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(tps6586x_pm_ops, tps6586x_i2c_suspend,
+tps6586x_i2c_resume);
+
 static const struct i2c_device_id tps6586x_id_table[] = {
{ "tps6586x", 0 },
{ },
@@ -604,6 +625,7 @@ static int tps6586x_i2c_remove(struct i2c_client
*client)
.driver = {
.name   = "tps6586x",
.of_match_table = of_match_ptr(tps6586x_of_match),
+   .pm = _pm_ops,
},
.probe  = tps6586x_i2c_probe,
.remove = tps6586x_i2c_remove,
-- 
1.9.1

-- 
nvpublic

Re: [PATCH v1 0/2] LP1/LP2 suspend-resume CPU clock fixes for Tegra30

[Dmitry Osipenko]

On 8/30/18 10:04 PM, Dmitry Osipenko wrote:
> Hello,
> 
> This patch-series fixes CPU hanging after suspend-resume / LP2 cpuidle
> on Tegra30. The bug really appears during stress-testing, like frequent
> suspending under variable load + the upcoming Tegra30 CPUFREQ driver.
> 
> Dmitry Osipenko (2):
>   ARM: tegra: Switch CPU to PLLP before powergating on Tegra30
>   ARM: tegra: Switch CPU to PLLP on resume from LP1 on Tegra30
> 
>  arch/arm/mach-tegra/sleep-tegra30.S | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 

ping

Re: [PATCH v1 0/2] LP1/LP2 suspend-resume CPU clock fixes for Tegra30

[Dmitry Osipenko]

On 8/30/18 10:04 PM, Dmitry Osipenko wrote:
> Hello,
> 
> This patch-series fixes CPU hanging after suspend-resume / LP2 cpuidle
> on Tegra30. The bug really appears during stress-testing, like frequent
> suspending under variable load + the upcoming Tegra30 CPUFREQ driver.
> 
> Dmitry Osipenko (2):
>   ARM: tegra: Switch CPU to PLLP before powergating on Tegra30
>   ARM: tegra: Switch CPU to PLLP on resume from LP1 on Tegra30
> 
>  arch/arm/mach-tegra/sleep-tegra30.S | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 

ping

[PATCH] PCI: imx: Add imx6sx suspend/resume support

[Leonard Crestez]

Enable PCI suspend/resume support on imx6sx socs. This is similar to
imx7d with a few differences:

* The PM_Turn_Off bit is exposed through an IOMUX GPR, like all other
pcie control bits on 6sx.
* The pcie_inbound_axi clk needs to be turned off in suspend. On resume
it is restored via resume -> deassert_core_reset -> enable_ref_clk.

Most of the resume logic is shared with the initial reset after probe.

Signed-off-by: Leonard Crestez 
---
 drivers/pci/controller/dwc/pci-imx6.c   | 40 ++---
 include/linux/mfd/syscon/imx6q-iomuxc-gpr.h |  1 +
 2 files changed, 36 insertions(+), 5 deletions(-)

Patch is again linux-next, meant to apply here:
 
https://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git/log/?h=pci/dwc

This is quite an old patch, mostly did it to prove that imx7d suspend
support is indeed generic.

diff --git a/drivers/pci/controller/dwc/pci-imx6.c 
b/drivers/pci/controller/dwc/pci-imx6.c
index 2cbef2d7c207..6171171db1fc 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -771,12 +771,29 @@ static void imx6_pcie_ltssm_disable(struct device *dev)
}
 }
 
 static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
 {
-   reset_control_assert(imx6_pcie->turnoff_reset);
-   reset_control_deassert(imx6_pcie->turnoff_reset);
+   struct device *dev = imx6_pcie->pci->dev;
+
+   /*
+* Some variants have a turnoff reset in DT while
+* others poke at iomuxc registers.
+*/
+   if (imx6_pcie->turnoff_reset) {
+   reset_control_assert(imx6_pcie->turnoff_reset);
+   reset_control_deassert(imx6_pcie->turnoff_reset);
+   } else if (imx6_pcie->variant == IMX6SX) {
+   regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+   IMX6SX_GPR12_PCIE_PM_TURN_OFF,
+   IMX6SX_GPR12_PCIE_PM_TURN_OFF);
+   regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+   IMX6SX_GPR12_PCIE_PM_TURN_OFF, 0);
+   } else {
+   dev_err(dev, "PME_Turn_Off not implemented\n");
+   return;
+   }
 
/*
 * Components with an upstream port must respond to
 * PME_Turn_Off with PME_TO_Ack but we can't check.
 *
@@ -790,22 +807,35 @@ static void imx6_pcie_clk_disable(struct imx6_pcie 
*imx6_pcie)
 {
clk_disable_unprepare(imx6_pcie->pcie);
clk_disable_unprepare(imx6_pcie->pcie_phy);
clk_disable_unprepare(imx6_pcie->pcie_bus);
 
-   if (imx6_pcie->variant == IMX7D) {
+   switch (imx6_pcie->variant) {
+   case IMX6SX:
+   clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
+   break;
+   case IMX7D:
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
+   break;
+   default:
+   break;
}
 }
 
+static inline bool imx6_pcie_supports_suspend(struct imx6_pcie *imx6_pcie)
+{
+   return (imx6_pcie->variant == IMX7D ||
+   imx6_pcie->variant == IMX6SX);
+}
+
 static int imx6_pcie_suspend_noirq(struct device *dev)
 {
struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
 
-   if (imx6_pcie->variant != IMX7D)
+   if (!imx6_pcie_supports_suspend(imx6_pcie))
return 0;
 
imx6_pcie_pm_turnoff(imx6_pcie);
imx6_pcie_clk_disable(imx6_pcie);
imx6_pcie_ltssm_disable(dev);
@@ -817,11 +847,11 @@ static int imx6_pcie_resume_noirq(struct device *dev)
 {
int ret;
struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
struct pcie_port *pp = _pcie->pci->pp;
 
-   if (imx6_pcie->variant != IMX7D)
+   if (!imx6_pcie_supports_suspend(imx6_pcie))
return 0;
 
imx6_pcie_assert_core_reset(imx6_pcie);
imx6_pcie_init_phy(imx6_pcie);
imx6_pcie_deassert_core_reset(imx6_pcie);
diff --git a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h 
b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
index 6c1ad160ed87..c1b25f5e386d 100644
--- a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
+++ b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
@@ -438,10 +438,11 @@
 #define IMX6SX_GPR5_DISP_MUX_DCIC1_LCDIF1  (0x0 << 1)
 #define IMX6SX_GPR5_DISP_MUX_DCIC1_LVDS(0x1 << 1)
 #define IMX6SX_GPR5_DISP_MUX_DCIC1_MASK(0x1 << 1)
 
 #define IMX6SX_GPR12_PCIE_TEST_POWERDOWN   BIT(30)
+#define IMX6SX_GPR12_PCIE_PM_TURN_OFF  BIT(16)
 #define IMX6SX_GPR12_PCIE_RX_EQ_MASK   (0x7 << 0)
 #define IMX6SX_GPR12_PCIE_RX_EQ_2  (0x2 << 0)
 
 /* For imx6ul iomux gpr register field define */
 #define IMX6UL_GPR1_ENET1_CLK_DIR  (0x1 << 17)
-- 
2.17.1

[PATCH] PCI: imx: Add imx6sx suspend/resume support

[Leonard Crestez]

Enable PCI suspend/resume support on imx6sx socs. This is similar to
imx7d with a few differences:

* The PM_Turn_Off bit is exposed through an IOMUX GPR, like all other
pcie control bits on 6sx.
* The pcie_inbound_axi clk needs to be turned off in suspend. On resume
it is restored via resume -> deassert_core_reset -> enable_ref_clk.

Most of the resume logic is shared with the initial reset after probe.

Signed-off-by: Leonard Crestez 
---
 drivers/pci/controller/dwc/pci-imx6.c   | 40 ++---
 include/linux/mfd/syscon/imx6q-iomuxc-gpr.h |  1 +
 2 files changed, 36 insertions(+), 5 deletions(-)

Patch is again linux-next, meant to apply here:
 
https://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git/log/?h=pci/dwc

This is quite an old patch, mostly did it to prove that imx7d suspend
support is indeed generic.

diff --git a/drivers/pci/controller/dwc/pci-imx6.c 
b/drivers/pci/controller/dwc/pci-imx6.c
index 2cbef2d7c207..6171171db1fc 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -771,12 +771,29 @@ static void imx6_pcie_ltssm_disable(struct device *dev)
}
 }
 
 static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
 {
-   reset_control_assert(imx6_pcie->turnoff_reset);
-   reset_control_deassert(imx6_pcie->turnoff_reset);
+   struct device *dev = imx6_pcie->pci->dev;
+
+   /*
+* Some variants have a turnoff reset in DT while
+* others poke at iomuxc registers.
+*/
+   if (imx6_pcie->turnoff_reset) {
+   reset_control_assert(imx6_pcie->turnoff_reset);
+   reset_control_deassert(imx6_pcie->turnoff_reset);
+   } else if (imx6_pcie->variant == IMX6SX) {
+   regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+   IMX6SX_GPR12_PCIE_PM_TURN_OFF,
+   IMX6SX_GPR12_PCIE_PM_TURN_OFF);
+   regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+   IMX6SX_GPR12_PCIE_PM_TURN_OFF, 0);
+   } else {
+   dev_err(dev, "PME_Turn_Off not implemented\n");
+   return;
+   }
 
/*
 * Components with an upstream port must respond to
 * PME_Turn_Off with PME_TO_Ack but we can't check.
 *
@@ -790,22 +807,35 @@ static void imx6_pcie_clk_disable(struct imx6_pcie 
*imx6_pcie)
 {
clk_disable_unprepare(imx6_pcie->pcie);
clk_disable_unprepare(imx6_pcie->pcie_phy);
clk_disable_unprepare(imx6_pcie->pcie_bus);
 
-   if (imx6_pcie->variant == IMX7D) {
+   switch (imx6_pcie->variant) {
+   case IMX6SX:
+   clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
+   break;
+   case IMX7D:
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
+   break;
+   default:
+   break;
}
 }
 
+static inline bool imx6_pcie_supports_suspend(struct imx6_pcie *imx6_pcie)
+{
+   return (imx6_pcie->variant == IMX7D ||
+   imx6_pcie->variant == IMX6SX);
+}
+
 static int imx6_pcie_suspend_noirq(struct device *dev)
 {
struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
 
-   if (imx6_pcie->variant != IMX7D)
+   if (!imx6_pcie_supports_suspend(imx6_pcie))
return 0;
 
imx6_pcie_pm_turnoff(imx6_pcie);
imx6_pcie_clk_disable(imx6_pcie);
imx6_pcie_ltssm_disable(dev);
@@ -817,11 +847,11 @@ static int imx6_pcie_resume_noirq(struct device *dev)
 {
int ret;
struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
struct pcie_port *pp = _pcie->pci->pp;
 
-   if (imx6_pcie->variant != IMX7D)
+   if (!imx6_pcie_supports_suspend(imx6_pcie))
return 0;
 
imx6_pcie_assert_core_reset(imx6_pcie);
imx6_pcie_init_phy(imx6_pcie);
imx6_pcie_deassert_core_reset(imx6_pcie);
diff --git a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h 
b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
index 6c1ad160ed87..c1b25f5e386d 100644
--- a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
+++ b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
@@ -438,10 +438,11 @@
 #define IMX6SX_GPR5_DISP_MUX_DCIC1_LCDIF1  (0x0 << 1)
 #define IMX6SX_GPR5_DISP_MUX_DCIC1_LVDS(0x1 << 1)
 #define IMX6SX_GPR5_DISP_MUX_DCIC1_MASK(0x1 << 1)
 
 #define IMX6SX_GPR12_PCIE_TEST_POWERDOWN   BIT(30)
+#define IMX6SX_GPR12_PCIE_PM_TURN_OFF  BIT(16)
 #define IMX6SX_GPR12_PCIE_RX_EQ_MASK   (0x7 << 0)
 #define IMX6SX_GPR12_PCIE_RX_EQ_2  (0x2 << 0)
 
 /* For imx6ul iomux gpr register field define */
 #define IMX6UL_GPR1_ENET1_CLK_DIR  (0x1 << 17)
-- 
2.17.1

[PATCH V1 4/7] mmc: core: add support for devfreq suspend/resume

[Sayali Lokhande]

This change adds support for devfreq suspend/resume
to be called on each system suspend/resume, runtime
suspend/resume, power restore.

Signed-off-by: Talel Shenhar 
Signed-off-by: Subhash Jadavani 
Signed-off-by: Sayali Lokhande 
---
 drivers/mmc/core/core.c | 112 
 drivers/mmc/core/core.h |   2 +
 drivers/mmc/core/host.c |   8 +++-
 drivers/mmc/core/mmc.c  |  27 
 drivers/mmc/core/sd.c   |  13 ++
 5 files changed, 160 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index a3d4a92..78be523 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -614,6 +614,111 @@ int mmc_init_clk_scaling(struct mmc_host *host)
 EXPORT_SYMBOL(mmc_init_clk_scaling);
 
 /**
+ * mmc_suspend_clk_scaling() - suspend clock scaling
+ * @host: pointer to mmc host structure
+ *
+ * This API will suspend devfreq feature for the specific host.
+ * The statistics collected by mmc will be cleared.
+ * This function is intended to be called by the pm callbacks
+ * (e.g. runtime_suspend, suspend) of the mmc device
+ */
+int mmc_suspend_clk_scaling(struct mmc_host *host)
+{
+   int err;
+
+   if (!host) {
+   WARN(1, "bad host parameter\n");
+   return -EINVAL;
+   }
+
+   if (!mmc_can_scale_clk(host) || !host->clk_scaling.enable)
+   return 0;
+
+   if (!host->clk_scaling.devfreq) {
+   pr_err("%s: %s: no devfreq is assosiated with this device\n",
+   mmc_hostname(host), __func__);
+   return -EPERM;
+   }
+
+   atomic_inc(>clk_scaling.devfreq_abort);
+   wake_up(>wq);
+   err = devfreq_suspend_device(host->clk_scaling.devfreq);
+   if (err) {
+   pr_err("%s: %s: failed to suspend devfreq\n",
+   mmc_hostname(host), __func__);
+   return err;
+   }
+   host->clk_scaling.enable = false;
+
+   host->clk_scaling.total_busy_time_us = 0;
+
+   pr_debug("%s: devfreq was removed\n", mmc_hostname(host));
+
+   return 0;
+}
+EXPORT_SYMBOL(mmc_suspend_clk_scaling);
+
+/**
+ * mmc_resume_clk_scaling() - resume clock scaling
+ * @host: pointer to mmc host structure
+ *
+ * This API will resume devfreq feature for the specific host.
+ * This API is intended to be called by the pm callbacks
+ * (e.g. runtime_suspend, suspend) of the mmc device
+ */
+int mmc_resume_clk_scaling(struct mmc_host *host)
+{
+   int err = 0;
+   u32 max_clk_idx = 0;
+   u32 devfreq_max_clk = 0;
+   u32 devfreq_min_clk = 0;
+
+   if (!host) {
+   WARN(1, "bad host parameter\n");
+   return -EINVAL;
+   }
+
+   if (!mmc_can_scale_clk(host))
+   return 0;
+
+   /*
+* If clock scaling is already exited when resume is called, like
+* during mmc shutdown, it is not an error and should not fail the
+* API calling this.
+*/
+   if (!host->clk_scaling.devfreq) {
+   pr_warn("%s: %s: no devfreq is assosiated with this device\n",
+   mmc_hostname(host), __func__);
+   return 0;
+   }
+
+   atomic_set(>clk_scaling.devfreq_abort, 0);
+
+   max_clk_idx = host->clk_scaling.freq_table_sz - 1;
+   devfreq_max_clk = host->clk_scaling.freq_table[max_clk_idx];
+   devfreq_min_clk = host->clk_scaling.freq_table[0];
+
+   host->clk_scaling.curr_freq = devfreq_max_clk;
+   if (host->ios.clock < host->clk_scaling.freq_table[max_clk_idx])
+   host->clk_scaling.curr_freq = devfreq_min_clk;
+
+   host->clk_scaling.clk_scaling_in_progress = false;
+   host->clk_scaling.need_freq_change = false;
+
+   err = devfreq_resume_device(host->clk_scaling.devfreq);
+   if (err) {
+   pr_err("%s: %s: failed to resume devfreq (%d)\n",
+   mmc_hostname(host), __func__, err);
+   } else {
+   host->clk_scaling.enable = true;
+   pr_debug("%s: devfreq resumed\n", mmc_hostname(host));
+   }
+
+   return err;
+}
+EXPORT_SYMBOL(mmc_resume_clk_scaling);
+
+/**
  * mmc_exit_devfreq_clk_scaling() - Disable clock scaling
  * @host: pointer to mmc host structure
  *
@@ -638,6 +743,13 @@ int mmc_exit_clk_scaling(struct mmc_host *host)
return -EPERM;
}
 
+   err = mmc_suspend_clk_scaling(host);
+   if (err) {
+   pr_err("%s: %s: fail to suspend clock scaling (%d)\n",
+   mmc_hostname(host), __func__,  err);
+   return err;
+   }
+
err = devfreq_remove_device(host->clk_scaling.devfreq);
if (err) {
pr_err("%s: remove devfreq failed (%d)\n",
diff --git a/drivers/mmc/core/core.h b/drivers

[PATCH V1 4/7] mmc: core: add support for devfreq suspend/resume

[Sayali Lokhande]

This change adds support for devfreq suspend/resume
to be called on each system suspend/resume, runtime
suspend/resume, power restore.

Signed-off-by: Talel Shenhar 
Signed-off-by: Subhash Jadavani 
Signed-off-by: Sayali Lokhande 
---
 drivers/mmc/core/core.c | 112 
 drivers/mmc/core/core.h |   2 +
 drivers/mmc/core/host.c |   8 +++-
 drivers/mmc/core/mmc.c  |  27 
 drivers/mmc/core/sd.c   |  13 ++
 5 files changed, 160 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index a3d4a92..78be523 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -614,6 +614,111 @@ int mmc_init_clk_scaling(struct mmc_host *host)
 EXPORT_SYMBOL(mmc_init_clk_scaling);
 
 /**
+ * mmc_suspend_clk_scaling() - suspend clock scaling
+ * @host: pointer to mmc host structure
+ *
+ * This API will suspend devfreq feature for the specific host.
+ * The statistics collected by mmc will be cleared.
+ * This function is intended to be called by the pm callbacks
+ * (e.g. runtime_suspend, suspend) of the mmc device
+ */
+int mmc_suspend_clk_scaling(struct mmc_host *host)
+{
+   int err;
+
+   if (!host) {
+   WARN(1, "bad host parameter\n");
+   return -EINVAL;
+   }
+
+   if (!mmc_can_scale_clk(host) || !host->clk_scaling.enable)
+   return 0;
+
+   if (!host->clk_scaling.devfreq) {
+   pr_err("%s: %s: no devfreq is assosiated with this device\n",
+   mmc_hostname(host), __func__);
+   return -EPERM;
+   }
+
+   atomic_inc(>clk_scaling.devfreq_abort);
+   wake_up(>wq);
+   err = devfreq_suspend_device(host->clk_scaling.devfreq);
+   if (err) {
+   pr_err("%s: %s: failed to suspend devfreq\n",
+   mmc_hostname(host), __func__);
+   return err;
+   }
+   host->clk_scaling.enable = false;
+
+   host->clk_scaling.total_busy_time_us = 0;
+
+   pr_debug("%s: devfreq was removed\n", mmc_hostname(host));
+
+   return 0;
+}
+EXPORT_SYMBOL(mmc_suspend_clk_scaling);
+
+/**
+ * mmc_resume_clk_scaling() - resume clock scaling
+ * @host: pointer to mmc host structure
+ *
+ * This API will resume devfreq feature for the specific host.
+ * This API is intended to be called by the pm callbacks
+ * (e.g. runtime_suspend, suspend) of the mmc device
+ */
+int mmc_resume_clk_scaling(struct mmc_host *host)
+{
+   int err = 0;
+   u32 max_clk_idx = 0;
+   u32 devfreq_max_clk = 0;
+   u32 devfreq_min_clk = 0;
+
+   if (!host) {
+   WARN(1, "bad host parameter\n");
+   return -EINVAL;
+   }
+
+   if (!mmc_can_scale_clk(host))
+   return 0;
+
+   /*
+* If clock scaling is already exited when resume is called, like
+* during mmc shutdown, it is not an error and should not fail the
+* API calling this.
+*/
+   if (!host->clk_scaling.devfreq) {
+   pr_warn("%s: %s: no devfreq is assosiated with this device\n",
+   mmc_hostname(host), __func__);
+   return 0;
+   }
+
+   atomic_set(>clk_scaling.devfreq_abort, 0);
+
+   max_clk_idx = host->clk_scaling.freq_table_sz - 1;
+   devfreq_max_clk = host->clk_scaling.freq_table[max_clk_idx];
+   devfreq_min_clk = host->clk_scaling.freq_table[0];
+
+   host->clk_scaling.curr_freq = devfreq_max_clk;
+   if (host->ios.clock < host->clk_scaling.freq_table[max_clk_idx])
+   host->clk_scaling.curr_freq = devfreq_min_clk;
+
+   host->clk_scaling.clk_scaling_in_progress = false;
+   host->clk_scaling.need_freq_change = false;
+
+   err = devfreq_resume_device(host->clk_scaling.devfreq);
+   if (err) {
+   pr_err("%s: %s: failed to resume devfreq (%d)\n",
+   mmc_hostname(host), __func__, err);
+   } else {
+   host->clk_scaling.enable = true;
+   pr_debug("%s: devfreq resumed\n", mmc_hostname(host));
+   }
+
+   return err;
+}
+EXPORT_SYMBOL(mmc_resume_clk_scaling);
+
+/**
  * mmc_exit_devfreq_clk_scaling() - Disable clock scaling
  * @host: pointer to mmc host structure
  *
@@ -638,6 +743,13 @@ int mmc_exit_clk_scaling(struct mmc_host *host)
return -EPERM;
}
 
+   err = mmc_suspend_clk_scaling(host);
+   if (err) {
+   pr_err("%s: %s: fail to suspend clock scaling (%d)\n",
+   mmc_hostname(host), __func__,  err);
+   return err;
+   }
+
err = devfreq_remove_device(host->clk_scaling.devfreq);
if (err) {
pr_err("%s: remove devfreq failed (%d)\n",
diff --git a/drivers/mmc/core/core.h b/drivers

Re: [PATCH v4 0/6] PCI: imx: Initial imx7d suspend/resume support

[Leonard Crestez]

On Mon, 2018-09-17 at 17:52 +0100, Lorenzo Pieralisi wrote:
> On Mon, Sep 17, 2018 at 04:01:21PM +, Leonard Crestez wrote:
> > On Mon, 2018-09-17 at 16:09 +0100, Lorenzo Pieralisi wrote:
> > > On Tue, Aug 14, 2018 at 07:50:14PM +0300, Leonard Crestez wrote:
> > > > V4 adds 4 more patches with PME_Turn_Off support on top, using a new
> > > > reset bit. I generally try to keep series short but in this case some
> > > > planning might be needed to get patches into 4.20.
> > > > 
> > > > Since the new reset is treated as optional with old DTB there should be
> > > > again no problem if reset and pci are merged out of order.
> > > > 
> > > > Shawn/Philipp/Lorenzo: Would it make sense to merge this series through 
> > > > a
> > > > single specific tree, such as the one for imx?
> > > 
> > > This series is a bit of a mixture of multiple things hard to discern
> > > (actually I already merged patch 2 and patch 1 seems completely
> > > unrelated).
> > > 
> > > I would take the series through the PCI tree but I need an ACK for
> > > patches 5 and 6, please let me know how you want to handle it.
> > 
> > Patches 1 and 2 are already in, the rest need review. I can now just
> > resend patches 3-6 as a separate series, unless somebody complains
> > about spam.
> 
> What do you mean by "are already in" ? Patch 2 I queued via the PCI
> tree, patch 1 ? Can I drop it from the PCI patch queue ?

Sorry, what I meant here is "accepted by a maintainer". So keep patch 2
please; patch 1 was accepted by Shawn few weeks ago.

> I do not think there is any need to resend, I can merge patches 3-4
> since they have been reviewed but patches 5 and 6 need review.

Patches 3-4 were acked by Rob Herring mostly from the devicetree
perspective. Since patches 3-6 are not useful independently somebody
like Lucas should review the whole series and then it can be merged.

--
Regards,
Leonard

Re: [PATCH v4 0/6] PCI: imx: Initial imx7d suspend/resume support

[Leonard Crestez]

On Mon, 2018-09-17 at 17:52 +0100, Lorenzo Pieralisi wrote:
> On Mon, Sep 17, 2018 at 04:01:21PM +, Leonard Crestez wrote:
> > On Mon, 2018-09-17 at 16:09 +0100, Lorenzo Pieralisi wrote:
> > > On Tue, Aug 14, 2018 at 07:50:14PM +0300, Leonard Crestez wrote:
> > > > V4 adds 4 more patches with PME_Turn_Off support on top, using a new
> > > > reset bit. I generally try to keep series short but in this case some
> > > > planning might be needed to get patches into 4.20.
> > > > 
> > > > Since the new reset is treated as optional with old DTB there should be
> > > > again no problem if reset and pci are merged out of order.
> > > > 
> > > > Shawn/Philipp/Lorenzo: Would it make sense to merge this series through 
> > > > a
> > > > single specific tree, such as the one for imx?
> > > 
> > > This series is a bit of a mixture of multiple things hard to discern
> > > (actually I already merged patch 2 and patch 1 seems completely
> > > unrelated).
> > > 
> > > I would take the series through the PCI tree but I need an ACK for
> > > patches 5 and 6, please let me know how you want to handle it.
> > 
> > Patches 1 and 2 are already in, the rest need review. I can now just
> > resend patches 3-6 as a separate series, unless somebody complains
> > about spam.
> 
> What do you mean by "are already in" ? Patch 2 I queued via the PCI
> tree, patch 1 ? Can I drop it from the PCI patch queue ?

Sorry, what I meant here is "accepted by a maintainer". So keep patch 2
please; patch 1 was accepted by Shawn few weeks ago.

> I do not think there is any need to resend, I can merge patches 3-4
> since they have been reviewed but patches 5 and 6 need review.

Patches 3-4 were acked by Rob Herring mostly from the devicetree
perspective. Since patches 3-6 are not useful independently somebody
like Lucas should review the whole series and then it can be merged.

--
Regards,
Leonard

Re: [PATCH v4 0/6] PCI: imx: Initial imx7d suspend/resume support

[Lorenzo Pieralisi]

On Mon, Sep 17, 2018 at 04:01:21PM +, Leonard Crestez wrote:
> On Mon, 2018-09-17 at 16:09 +0100, Lorenzo Pieralisi wrote:
> > On Tue, Aug 14, 2018 at 07:50:14PM +0300, Leonard Crestez wrote:
> 
> > > V4 adds 4 more patches with PME_Turn_Off support on top, using a new
> > > reset bit. I generally try to keep series short but in this case some
> > > planning might be needed to get patches into 4.20.
> > > 
> > > Since the new reset is treated as optional with old DTB there should be
> > > again no problem if reset and pci are merged out of order.
> > > 
> > > Shawn/Philipp/Lorenzo: Would it make sense to merge this series through a
> > > single specific tree, such as the one for imx?
> > 
> > This series is a bit of a mixture of multiple things hard to discern
> > (actually I already merged patch 2 and patch 1 seems completely
> > unrelated).
> > 
> > I would take the series through the PCI tree but I need an ACK for
> > patches 5 and 6, please let me know how you want to handle it.
> 
> Patches 1 and 2 are already in, the rest need review. I can now just
> resend patches 3-6 as a separate series, unless somebody complains
> about spam.

What do you mean by "are already in" ? Patch 2 I queued via the PCI
tree, patch 1 ? Can I drop it from the PCI patch queue ?

I do not think there is any need to resend, I can merge patches 3-4
since they have been reviewed but patches 5 and 6 need review.

Lorenzo

> Multiple separate patches are required because it touches reset + dt +
> pci. I guess adding the include constant should be moved from the dts
> patch to dt-bindings though.
> 
> Merging reset and pci out of order should be fine here and is required
> by DT compatibility rules anyway.
> 
> --
> Regards,
> Leonard

Re: [PATCH v4 0/6] PCI: imx: Initial imx7d suspend/resume support

[Lorenzo Pieralisi]

On Mon, Sep 17, 2018 at 04:01:21PM +, Leonard Crestez wrote:
> On Mon, 2018-09-17 at 16:09 +0100, Lorenzo Pieralisi wrote:
> > On Tue, Aug 14, 2018 at 07:50:14PM +0300, Leonard Crestez wrote:
> 
> > > V4 adds 4 more patches with PME_Turn_Off support on top, using a new
> > > reset bit. I generally try to keep series short but in this case some
> > > planning might be needed to get patches into 4.20.
> > > 
> > > Since the new reset is treated as optional with old DTB there should be
> > > again no problem if reset and pci are merged out of order.
> > > 
> > > Shawn/Philipp/Lorenzo: Would it make sense to merge this series through a
> > > single specific tree, such as the one for imx?
> > 
> > This series is a bit of a mixture of multiple things hard to discern
> > (actually I already merged patch 2 and patch 1 seems completely
> > unrelated).
> > 
> > I would take the series through the PCI tree but I need an ACK for
> > patches 5 and 6, please let me know how you want to handle it.
> 
> Patches 1 and 2 are already in, the rest need review. I can now just
> resend patches 3-6 as a separate series, unless somebody complains
> about spam.

What do you mean by "are already in" ? Patch 2 I queued via the PCI
tree, patch 1 ? Can I drop it from the PCI patch queue ?

I do not think there is any need to resend, I can merge patches 3-4
since they have been reviewed but patches 5 and 6 need review.

Lorenzo

> Multiple separate patches are required because it touches reset + dt +
> pci. I guess adding the include constant should be moved from the dts
> patch to dt-bindings though.
> 
> Merging reset and pci out of order should be fine here and is required
> by DT compatibility rules anyway.
> 
> --
> Regards,
> Leonard

Re: [PATCH v4 0/6] PCI: imx: Initial imx7d suspend/resume support

[Leonard Crestez]

On Mon, 2018-09-17 at 16:09 +0100, Lorenzo Pieralisi wrote:
> On Tue, Aug 14, 2018 at 07:50:14PM +0300, Leonard Crestez wrote:

> > V4 adds 4 more patches with PME_Turn_Off support on top, using a new
> > reset bit. I generally try to keep series short but in this case some
> > planning might be needed to get patches into 4.20.
> > 
> > Since the new reset is treated as optional with old DTB there should be
> > again no problem if reset and pci are merged out of order.
> > 
> > Shawn/Philipp/Lorenzo: Would it make sense to merge this series through a
> > single specific tree, such as the one for imx?
> 
> This series is a bit of a mixture of multiple things hard to discern
> (actually I already merged patch 2 and patch 1 seems completely
> unrelated).
> 
> I would take the series through the PCI tree but I need an ACK for
> patches 5 and 6, please let me know how you want to handle it.

Patches 1 and 2 are already in, the rest need review. I can now just
resend patches 3-6 as a separate series, unless somebody complains
about spam.

Multiple separate patches are required because it touches reset + dt +
pci. I guess adding the include constant should be moved from the dts
patch to dt-bindings though.

Merging reset and pci out of order should be fine here and is required
by DT compatibility rules anyway.

--
Regards,
Leonard

Re: [PATCH v4 0/6] PCI: imx: Initial imx7d suspend/resume support

[Leonard Crestez]

On Mon, 2018-09-17 at 16:09 +0100, Lorenzo Pieralisi wrote:
> On Tue, Aug 14, 2018 at 07:50:14PM +0300, Leonard Crestez wrote:

> > V4 adds 4 more patches with PME_Turn_Off support on top, using a new
> > reset bit. I generally try to keep series short but in this case some
> > planning might be needed to get patches into 4.20.
> > 
> > Since the new reset is treated as optional with old DTB there should be
> > again no problem if reset and pci are merged out of order.
> > 
> > Shawn/Philipp/Lorenzo: Would it make sense to merge this series through a
> > single specific tree, such as the one for imx?
> 
> This series is a bit of a mixture of multiple things hard to discern
> (actually I already merged patch 2 and patch 1 seems completely
> unrelated).
> 
> I would take the series through the PCI tree but I need an ACK for
> patches 5 and 6, please let me know how you want to handle it.

Patches 1 and 2 are already in, the rest need review. I can now just
resend patches 3-6 as a separate series, unless somebody complains
about spam.

Multiple separate patches are required because it touches reset + dt +
pci. I guess adding the include constant should be moved from the dts
patch to dt-bindings though.

Merging reset and pci out of order should be fine here and is required
by DT compatibility rules anyway.

--
Regards,
Leonard

Re: [PATCH v4 0/6] PCI: imx: Initial imx7d suspend/resume support

[Lorenzo Pieralisi]

On Tue, Aug 14, 2018 at 07:50:14PM +0300, Leonard Crestez wrote:
> On imx7d the pcie-phy power domain is turned off in suspend and this can
> make the system hang on resume when attempting any read from PCI.
> 
> Fix this by adding PM_SLEEP support to the imx6 pci driver. This is
> currently only enabled for imx7d but the suspend/resume sequence also
> applies to other socs.
> 
> V3 of this series was reviewed by Lucas but stalled because the merge
> window opened.
> 
> There was also some confusion about how to deal with the dependence on
> commit 26fce0557fa6 ("reset: imx7: Fix always writing bits as 0"). To
> clarify: both patch 2 and 26fce0557fa6 are required to fix imx7d suspend
> but merging one without the other shouldn't cause other issues.
> 
> 
> V4 adds 4 more patches with PME_Turn_Off support on top, using a new
> reset bit. I generally try to keep series short but in this case some
> planning might be needed to get patches into 4.20.
> 
> Since the new reset is treated as optional with old DTB there should be
> again no problem if reset and pci are merged out of order.
> 
> 
> Shawn/Philipp/Lorenzo: Would it make sense to merge this series through a
> single specific tree, such as the one for imx?

This series is a bit of a mixture of multiple things hard to discern
(actually I already merged patch 2 and patch 1 seems completely
unrelated).

I would take the series through the PCI tree but I need an ACK for
patches 5 and 6, please let me know how you want to handle it.

Lorenzo

> Link to v3: https://lkml.org/lkml/2018/7/24/713
> 
> Leonard Crestez (6):
>   Revert "ARM: dts: imx7d: Invert legacy PCI irq mapping"
>   PCI: imx: Initial imx7d pm support
>   reset: imx7: Add PCIE_CTRL_APPS_TURNOFF
>   dt-bindings: imx6q-pcie: Add turnoff reset for imx7d
>   ARM: dts: imx7d: Add turnoff reset
>   PCI: imx: Add PME_Turn_Off support
> 
>  .../bindings/pci/fsl,imx6q-pcie.txt   |   1 +
>  arch/arm/boot/dts/imx7d.dtsi  |  17 ++-
>  drivers/pci/controller/dwc/pci-imx6.c | 112 +-
>  drivers/reset/reset-imx7.c|   1 +
>  include/dt-bindings/reset/imx7-reset.h|   4 +-
>  5 files changed, 123 insertions(+), 12 deletions(-)
> 
> -- 
> 2.17.1
> 

Re: [PATCH v4 0/6] PCI: imx: Initial imx7d suspend/resume support

[Lorenzo Pieralisi]

On Tue, Aug 14, 2018 at 07:50:14PM +0300, Leonard Crestez wrote:
> On imx7d the pcie-phy power domain is turned off in suspend and this can
> make the system hang on resume when attempting any read from PCI.
> 
> Fix this by adding PM_SLEEP support to the imx6 pci driver. This is
> currently only enabled for imx7d but the suspend/resume sequence also
> applies to other socs.
> 
> V3 of this series was reviewed by Lucas but stalled because the merge
> window opened.
> 
> There was also some confusion about how to deal with the dependence on
> commit 26fce0557fa6 ("reset: imx7: Fix always writing bits as 0"). To
> clarify: both patch 2 and 26fce0557fa6 are required to fix imx7d suspend
> but merging one without the other shouldn't cause other issues.
> 
> 
> V4 adds 4 more patches with PME_Turn_Off support on top, using a new
> reset bit. I generally try to keep series short but in this case some
> planning might be needed to get patches into 4.20.
> 
> Since the new reset is treated as optional with old DTB there should be
> again no problem if reset and pci are merged out of order.
> 
> 
> Shawn/Philipp/Lorenzo: Would it make sense to merge this series through a
> single specific tree, such as the one for imx?

This series is a bit of a mixture of multiple things hard to discern
(actually I already merged patch 2 and patch 1 seems completely
unrelated).

I would take the series through the PCI tree but I need an ACK for
patches 5 and 6, please let me know how you want to handle it.

Lorenzo

> Link to v3: https://lkml.org/lkml/2018/7/24/713
> 
> Leonard Crestez (6):
>   Revert "ARM: dts: imx7d: Invert legacy PCI irq mapping"
>   PCI: imx: Initial imx7d pm support
>   reset: imx7: Add PCIE_CTRL_APPS_TURNOFF
>   dt-bindings: imx6q-pcie: Add turnoff reset for imx7d
>   ARM: dts: imx7d: Add turnoff reset
>   PCI: imx: Add PME_Turn_Off support
> 
>  .../bindings/pci/fsl,imx6q-pcie.txt   |   1 +
>  arch/arm/boot/dts/imx7d.dtsi  |  17 ++-
>  drivers/pci/controller/dwc/pci-imx6.c | 112 +-
>  drivers/reset/reset-imx7.c|   1 +
>  include/dt-bindings/reset/imx7-reset.h|   4 +-
>  5 files changed, 123 insertions(+), 12 deletions(-)
> 
> -- 
> 2.17.1
> 

[PATCH 4.14 101/115] usb: dwc3: core: Fix ULPI PHYs and prevent phy_get/ulpi_init during suspend/resume

[Greg Kroah-Hartman]

4.14-stable review patch.  If anyone has any objections, please let me know.

--

From: Roger Quadros 

commit 98112041bcca164676367e261c8c1073ef70cb51 upstream.

In order for ULPI PHYs to work, dwc3_phy_setup() and dwc3_ulpi_init()
must be doene before dwc3_core_get_phy().

commit 541768b08a40 ("usb: dwc3: core: Call dwc3_core_get_phy() before 
initializing phys")
broke this.

The other issue is that dwc3_core_get_phy() and dwc3_ulpi_init() should
be called only once during the life cycle of the driver. However,
as dwc3_core_init() is called during system suspend/resume it will
result in multiple calls to dwc3_core_get_phy() and dwc3_ulpi_init()
which is wrong.

Fix this by moving dwc3_ulpi_init() out of dwc3_phy_setup()
into dwc3_core_ulpi_init(). Use a flag 'ulpi_ready' to ensure that
dwc3_core_ulpi_init() is called only once from dwc3_core_init().

Use another flag 'phys_ready' to call dwc3_core_get_phy() only once from
dwc3_core_init().

Fixes: 541768b08a40 ("usb: dwc3: core: Call dwc3_core_get_phy() before 
initializing phys")
Fixes: f54edb539c11 ("usb: dwc3: core: initialize ULPI before trying to get the 
PHY")
Cc: linux-stable  # >= v4.13
Signed-off-by: Roger Quadros 
Signed-off-by: Felipe Balbi 
Signed-off-by: Sudip Mukherjee 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/usb/dwc3/core.c |   47 ---
 drivers/usb/dwc3/core.h |5 +
 2 files changed, 41 insertions(+), 11 deletions(-)

--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -511,6 +511,22 @@ static void dwc3_cache_hwparams(struct d
parms->hwparams8 = dwc3_readl(dwc->regs, DWC3_GHWPARAMS8);
 }
 
+static int dwc3_core_ulpi_init(struct dwc3 *dwc)
+{
+   int intf;
+   int ret = 0;
+
+   intf = DWC3_GHWPARAMS3_HSPHY_IFC(dwc->hwparams.hwparams3);
+
+   if (intf == DWC3_GHWPARAMS3_HSPHY_IFC_ULPI ||
+   (intf == DWC3_GHWPARAMS3_HSPHY_IFC_UTMI_ULPI &&
+dwc->hsphy_interface &&
+!strncmp(dwc->hsphy_interface, "ulpi", 4)))
+   ret = dwc3_ulpi_init(dwc);
+
+   return ret;
+}
+
 /**
  * dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core
  * @dwc: Pointer to our controller context structure
@@ -522,7 +538,6 @@ static void dwc3_cache_hwparams(struct d
 static int dwc3_phy_setup(struct dwc3 *dwc)
 {
u32 reg;
-   int ret;
 
reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
 
@@ -593,9 +608,6 @@ static int dwc3_phy_setup(struct dwc3 *d
}
/* FALLTHROUGH */
case DWC3_GHWPARAMS3_HSPHY_IFC_ULPI:
-   ret = dwc3_ulpi_init(dwc);
-   if (ret)
-   return ret;
/* FALLTHROUGH */
default:
break;
@@ -752,6 +764,7 @@ static void dwc3_core_setup_global_contr
 }
 
 static int dwc3_core_get_phy(struct dwc3 *dwc);
+static int dwc3_core_ulpi_init(struct dwc3 *dwc);
 
 /**
  * dwc3_core_init - Low-level initialization of DWC3 Core
@@ -783,17 +796,27 @@ static int dwc3_core_init(struct dwc3 *d
dwc->maximum_speed = USB_SPEED_HIGH;
}
 
-   ret = dwc3_core_get_phy(dwc);
+   ret = dwc3_phy_setup(dwc);
if (ret)
goto err0;
 
-   ret = dwc3_core_soft_reset(dwc);
-   if (ret)
-   goto err0;
+   if (!dwc->ulpi_ready) {
+   ret = dwc3_core_ulpi_init(dwc);
+   if (ret)
+   goto err0;
+   dwc->ulpi_ready = true;
+   }
 
-   ret = dwc3_phy_setup(dwc);
+   if (!dwc->phys_ready) {
+   ret = dwc3_core_get_phy(dwc);
+   if (ret)
+   goto err0a;
+   dwc->phys_ready = true;
+   }
+
+   ret = dwc3_core_soft_reset(dwc);
if (ret)
-   goto err0;
+   goto err0a;
 
dwc3_core_setup_global_control(dwc);
dwc3_core_num_eps(dwc);
@@ -866,6 +889,9 @@ err1:
phy_exit(dwc->usb2_generic_phy);
phy_exit(dwc->usb3_generic_phy);
 
+err0a:
+   dwc3_ulpi_exit(dwc);
+
 err0:
return ret;
 }
@@ -1256,7 +1282,6 @@ err4:
 
 err3:
dwc3_free_event_buffers(dwc);
-   dwc3_ulpi_exit(dwc);
 
 err2:
pm_runtime_allow(>dev);
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -805,7 +805,9 @@ struct dwc3_scratchpad_array {
  * @usb3_phy: pointer to USB3 PHY
  * @usb2_generic_phy: pointer to USB2 PHY
  * @usb3_generic_phy: pointer to USB3 PHY
+ * @phys_ready: flag to indicate that PHYs are ready
  * @ulpi: pointer to ulpi interface
+ * @ulpi_ready: flag to indicate that ULPI is initialized
  * @isoch_delay: wValue from Set Isochronous Delay request;
  * @u2sel: parameter from Set SEL request.
  * @u2pel: parameter from Set SEL request.
@@ -903,7 +905,10 @@ struct dwc3 {
struct phy  *usb2_generic

[PATCH 4.14 101/115] usb: dwc3: core: Fix ULPI PHYs and prevent phy_get/ulpi_init during suspend/resume

[Greg Kroah-Hartman]

4.14-stable review patch.  If anyone has any objections, please let me know.

--

From: Roger Quadros 

commit 98112041bcca164676367e261c8c1073ef70cb51 upstream.

In order for ULPI PHYs to work, dwc3_phy_setup() and dwc3_ulpi_init()
must be doene before dwc3_core_get_phy().

commit 541768b08a40 ("usb: dwc3: core: Call dwc3_core_get_phy() before 
initializing phys")
broke this.

The other issue is that dwc3_core_get_phy() and dwc3_ulpi_init() should
be called only once during the life cycle of the driver. However,
as dwc3_core_init() is called during system suspend/resume it will
result in multiple calls to dwc3_core_get_phy() and dwc3_ulpi_init()
which is wrong.

Fix this by moving dwc3_ulpi_init() out of dwc3_phy_setup()
into dwc3_core_ulpi_init(). Use a flag 'ulpi_ready' to ensure that
dwc3_core_ulpi_init() is called only once from dwc3_core_init().

Use another flag 'phys_ready' to call dwc3_core_get_phy() only once from
dwc3_core_init().

Fixes: 541768b08a40 ("usb: dwc3: core: Call dwc3_core_get_phy() before 
initializing phys")
Fixes: f54edb539c11 ("usb: dwc3: core: initialize ULPI before trying to get the 
PHY")
Cc: linux-stable  # >= v4.13
Signed-off-by: Roger Quadros 
Signed-off-by: Felipe Balbi 
Signed-off-by: Sudip Mukherjee 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/usb/dwc3/core.c |   47 ---
 drivers/usb/dwc3/core.h |5 +
 2 files changed, 41 insertions(+), 11 deletions(-)

--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -511,6 +511,22 @@ static void dwc3_cache_hwparams(struct d
parms->hwparams8 = dwc3_readl(dwc->regs, DWC3_GHWPARAMS8);
 }
 
+static int dwc3_core_ulpi_init(struct dwc3 *dwc)
+{
+   int intf;
+   int ret = 0;
+
+   intf = DWC3_GHWPARAMS3_HSPHY_IFC(dwc->hwparams.hwparams3);
+
+   if (intf == DWC3_GHWPARAMS3_HSPHY_IFC_ULPI ||
+   (intf == DWC3_GHWPARAMS3_HSPHY_IFC_UTMI_ULPI &&
+dwc->hsphy_interface &&
+!strncmp(dwc->hsphy_interface, "ulpi", 4)))
+   ret = dwc3_ulpi_init(dwc);
+
+   return ret;
+}
+
 /**
  * dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core
  * @dwc: Pointer to our controller context structure
@@ -522,7 +538,6 @@ static void dwc3_cache_hwparams(struct d
 static int dwc3_phy_setup(struct dwc3 *dwc)
 {
u32 reg;
-   int ret;
 
reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
 
@@ -593,9 +608,6 @@ static int dwc3_phy_setup(struct dwc3 *d
}
/* FALLTHROUGH */
case DWC3_GHWPARAMS3_HSPHY_IFC_ULPI:
-   ret = dwc3_ulpi_init(dwc);
-   if (ret)
-   return ret;
/* FALLTHROUGH */
default:
break;
@@ -752,6 +764,7 @@ static void dwc3_core_setup_global_contr
 }
 
 static int dwc3_core_get_phy(struct dwc3 *dwc);
+static int dwc3_core_ulpi_init(struct dwc3 *dwc);
 
 /**
  * dwc3_core_init - Low-level initialization of DWC3 Core
@@ -783,17 +796,27 @@ static int dwc3_core_init(struct dwc3 *d
dwc->maximum_speed = USB_SPEED_HIGH;
}
 
-   ret = dwc3_core_get_phy(dwc);
+   ret = dwc3_phy_setup(dwc);
if (ret)
goto err0;
 
-   ret = dwc3_core_soft_reset(dwc);
-   if (ret)
-   goto err0;
+   if (!dwc->ulpi_ready) {
+   ret = dwc3_core_ulpi_init(dwc);
+   if (ret)
+   goto err0;
+   dwc->ulpi_ready = true;
+   }
 
-   ret = dwc3_phy_setup(dwc);
+   if (!dwc->phys_ready) {
+   ret = dwc3_core_get_phy(dwc);
+   if (ret)
+   goto err0a;
+   dwc->phys_ready = true;
+   }
+
+   ret = dwc3_core_soft_reset(dwc);
if (ret)
-   goto err0;
+   goto err0a;
 
dwc3_core_setup_global_control(dwc);
dwc3_core_num_eps(dwc);
@@ -866,6 +889,9 @@ err1:
phy_exit(dwc->usb2_generic_phy);
phy_exit(dwc->usb3_generic_phy);
 
+err0a:
+   dwc3_ulpi_exit(dwc);
+
 err0:
return ret;
 }
@@ -1256,7 +1282,6 @@ err4:
 
 err3:
dwc3_free_event_buffers(dwc);
-   dwc3_ulpi_exit(dwc);
 
 err2:
pm_runtime_allow(>dev);
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -805,7 +805,9 @@ struct dwc3_scratchpad_array {
  * @usb3_phy: pointer to USB3 PHY
  * @usb2_generic_phy: pointer to USB2 PHY
  * @usb3_generic_phy: pointer to USB3 PHY
+ * @phys_ready: flag to indicate that PHYs are ready
  * @ulpi: pointer to ulpi interface
+ * @ulpi_ready: flag to indicate that ULPI is initialized
  * @isoch_delay: wValue from Set Isochronous Delay request;
  * @u2sel: parameter from Set SEL request.
  * @u2pel: parameter from Set SEL request.
@@ -903,7 +905,10 @@ struct dwc3 {
struct phy  *usb2_generic

Re: [PATCH v2] mfd: max8997: Disable interrupt handling for suspend/resume cycle

[Lee Jones]

On Wed, 05 Sep 2018, Marek Szyprowski wrote:

> Disable IRQs during suspend/resume cycle to ensure handling of wakeup
> interrupts (i.e. RTC wake alarm) after max8997_resume(). This way it can
> be properly handled when I2C bus is finally available. This pattern is
> also used in other MAX PMIC MFD drivers.
> 
> Signed-off-by: Marek Szyprowski 
> ---
> changelog:
> v2:
> - reordered the sequence of operation as suggested by Krzysztof Kozlowski
> ---
>  drivers/mfd/max8997.c | 2 ++
>  1 file changed, 2 insertions(+)

Applied, thanks.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2] mfd: max8997: Disable interrupt handling for suspend/resume cycle

[Lee Jones]

On Wed, 05 Sep 2018, Marek Szyprowski wrote:

> Disable IRQs during suspend/resume cycle to ensure handling of wakeup
> interrupts (i.e. RTC wake alarm) after max8997_resume(). This way it can
> be properly handled when I2C bus is finally available. This pattern is
> also used in other MAX PMIC MFD drivers.
> 
> Signed-off-by: Marek Szyprowski 
> ---
> changelog:
> v2:
> - reordered the sequence of operation as suggested by Krzysztof Kozlowski
> ---
>  drivers/mfd/max8997.c | 2 ++
>  1 file changed, 2 insertions(+)

Applied, thanks.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2] mfd: max8997: Disable interrupt handling for suspend/resume cycle

[Krzysztof Kozlowski]

On Wed, Sep 05, 2018 at 04:36:06PM +0200, Marek Szyprowski wrote:
> Disable IRQs during suspend/resume cycle to ensure handling of wakeup
> interrupts (i.e. RTC wake alarm) after max8997_resume(). This way it can
> be properly handled when I2C bus is finally available. This pattern is
> also used in other MAX PMIC MFD drivers.
> 
> Signed-off-by: Marek Szyprowski 
> ---
> changelog:
> v2:
> - reordered the sequence of operation as suggested by Krzysztof Kozlowski
> ---
>  drivers/mfd/max8997.c | 2 ++
>  1 file changed, 2 insertions(+)
> 

Reviewed-by: Krzysztof Kozlowski 

Best regards,
Krzysztof

Re: [PATCH v2] mfd: max8997: Disable interrupt handling for suspend/resume cycle

[Krzysztof Kozlowski]

On Wed, Sep 05, 2018 at 04:36:06PM +0200, Marek Szyprowski wrote:
> Disable IRQs during suspend/resume cycle to ensure handling of wakeup
> interrupts (i.e. RTC wake alarm) after max8997_resume(). This way it can
> be properly handled when I2C bus is finally available. This pattern is
> also used in other MAX PMIC MFD drivers.
> 
> Signed-off-by: Marek Szyprowski 
> ---
> changelog:
> v2:
> - reordered the sequence of operation as suggested by Krzysztof Kozlowski
> ---
>  drivers/mfd/max8997.c | 2 ++
>  1 file changed, 2 insertions(+)
> 

Reviewed-by: Krzysztof Kozlowski 

Best regards,
Krzysztof

[PATCH v2] mfd: max8997: Disable interrupt handling for suspend/resume cycle

[Marek Szyprowski]

Disable IRQs during suspend/resume cycle to ensure handling of wakeup
interrupts (i.e. RTC wake alarm) after max8997_resume(). This way it can
be properly handled when I2C bus is finally available. This pattern is
also used in other MAX PMIC MFD drivers.

Signed-off-by: Marek Szyprowski 
---
changelog:
v2:
- reordered the sequence of operation as suggested by Krzysztof Kozlowski
---
 drivers/mfd/max8997.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
index 3f554c447521..b17eff50ad52 100644
--- a/drivers/mfd/max8997.c
+++ b/drivers/mfd/max8997.c
@@ -468,6 +468,7 @@ static int max8997_suspend(struct device *dev)
struct i2c_client *i2c = to_i2c_client(dev);
struct max8997_dev *max8997 = i2c_get_clientdata(i2c);
 
+   disable_irq(max8997->irq);
if (device_may_wakeup(dev))
irq_set_irq_wake(max8997->irq, 1);
return 0;
@@ -480,6 +481,7 @@ static int max8997_resume(struct device *dev)
 
if (device_may_wakeup(dev))
irq_set_irq_wake(max8997->irq, 0);
+   enable_irq(max8997->irq);
return max8997_irq_resume(max8997);
 }
 
-- 
2.17.1

[PATCH v2] mfd: max8997: Disable interrupt handling for suspend/resume cycle

[Marek Szyprowski]

Disable IRQs during suspend/resume cycle to ensure handling of wakeup
interrupts (i.e. RTC wake alarm) after max8997_resume(). This way it can
be properly handled when I2C bus is finally available. This pattern is
also used in other MAX PMIC MFD drivers.

Signed-off-by: Marek Szyprowski 
---
changelog:
v2:
- reordered the sequence of operation as suggested by Krzysztof Kozlowski
---
 drivers/mfd/max8997.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
index 3f554c447521..b17eff50ad52 100644
--- a/drivers/mfd/max8997.c
+++ b/drivers/mfd/max8997.c
@@ -468,6 +468,7 @@ static int max8997_suspend(struct device *dev)
struct i2c_client *i2c = to_i2c_client(dev);
struct max8997_dev *max8997 = i2c_get_clientdata(i2c);
 
+   disable_irq(max8997->irq);
if (device_may_wakeup(dev))
irq_set_irq_wake(max8997->irq, 1);
return 0;
@@ -480,6 +481,7 @@ static int max8997_resume(struct device *dev)
 
if (device_may_wakeup(dev))
irq_set_irq_wake(max8997->irq, 0);
+   enable_irq(max8997->irq);
return max8997_irq_resume(max8997);
 }
 
-- 
2.17.1

Re: [PATCH] mfd: max8997: Disable interrupt handling for suspend/resume cycle

[Krzysztof Kozlowski]

On Wed, 5 Sep 2018 at 14:32, Marek Szyprowski  wrote:
>
> Disable IRQs during suspend/resume cycle to ensure handling of wakeup
> interrupts (i.e. RTC wake alarm) after max8997_resume(). This way it can
> be properly handled when I2C bus is finally available. This pattern is
> also used in other MAX PMIC MFD drivers.
>
> Signed-off-by: Marek Szyprowski 
> ---
>  drivers/mfd/max8997.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
> index d1495d76bf2c..5a50ea976c70 100644
> --- a/drivers/mfd/max8997.c
> +++ b/drivers/mfd/max8997.c
> @@ -464,6 +464,7 @@ static int max8997_suspend(struct device *dev)
>
> if (device_may_wakeup(dev))
> irq_set_irq_wake(max8997->irq, 1);
> +   disable_irq(max8997->irq);
> return 0;
>  }
>
> @@ -474,6 +475,7 @@ static int max8997_resume(struct device *dev)
>
> if (device_may_wakeup(dev))
> irq_set_irq_wake(max8997->irq, 0);
> +   enable_irq(max8997->irq);
> return max8997_irq_resume(max8997);

Looks good except that here and in some existing drivers we do not
resume in reverse order of suspend. How about making it like in
drivers/mfd/max77843.c? It should not differ from functional point of
view, just logically it makes sense.

Best regards,
Krzysztof

Re: [PATCH] mfd: max8997: Disable interrupt handling for suspend/resume cycle

[Krzysztof Kozlowski]

On Wed, 5 Sep 2018 at 14:32, Marek Szyprowski  wrote:
>
> Disable IRQs during suspend/resume cycle to ensure handling of wakeup
> interrupts (i.e. RTC wake alarm) after max8997_resume(). This way it can
> be properly handled when I2C bus is finally available. This pattern is
> also used in other MAX PMIC MFD drivers.
>
> Signed-off-by: Marek Szyprowski 
> ---
>  drivers/mfd/max8997.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
> index d1495d76bf2c..5a50ea976c70 100644
> --- a/drivers/mfd/max8997.c
> +++ b/drivers/mfd/max8997.c
> @@ -464,6 +464,7 @@ static int max8997_suspend(struct device *dev)
>
> if (device_may_wakeup(dev))
> irq_set_irq_wake(max8997->irq, 1);
> +   disable_irq(max8997->irq);
> return 0;
>  }
>
> @@ -474,6 +475,7 @@ static int max8997_resume(struct device *dev)
>
> if (device_may_wakeup(dev))
> irq_set_irq_wake(max8997->irq, 0);
> +   enable_irq(max8997->irq);
> return max8997_irq_resume(max8997);

Looks good except that here and in some existing drivers we do not
resume in reverse order of suspend. How about making it like in
drivers/mfd/max77843.c? It should not differ from functional point of
view, just logically it makes sense.

Best regards,
Krzysztof

[PATCH] mfd: max8997: Disable interrupt handling for suspend/resume cycle

[Marek Szyprowski]

Disable IRQs during suspend/resume cycle to ensure handling of wakeup
interrupts (i.e. RTC wake alarm) after max8997_resume(). This way it can
be properly handled when I2C bus is finally available. This pattern is
also used in other MAX PMIC MFD drivers.

Signed-off-by: Marek Szyprowski 
---
 drivers/mfd/max8997.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
index d1495d76bf2c..5a50ea976c70 100644
--- a/drivers/mfd/max8997.c
+++ b/drivers/mfd/max8997.c
@@ -464,6 +464,7 @@ static int max8997_suspend(struct device *dev)
 
if (device_may_wakeup(dev))
irq_set_irq_wake(max8997->irq, 1);
+   disable_irq(max8997->irq);
return 0;
 }
 
@@ -474,6 +475,7 @@ static int max8997_resume(struct device *dev)
 
if (device_may_wakeup(dev))
irq_set_irq_wake(max8997->irq, 0);
+   enable_irq(max8997->irq);
return max8997_irq_resume(max8997);
 }
 
-- 
2.17.1

[PATCH] mfd: max8997: Disable interrupt handling for suspend/resume cycle

[Marek Szyprowski]

Disable IRQs during suspend/resume cycle to ensure handling of wakeup
interrupts (i.e. RTC wake alarm) after max8997_resume(). This way it can
be properly handled when I2C bus is finally available. This pattern is
also used in other MAX PMIC MFD drivers.

Signed-off-by: Marek Szyprowski 
---
 drivers/mfd/max8997.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
index d1495d76bf2c..5a50ea976c70 100644
--- a/drivers/mfd/max8997.c
+++ b/drivers/mfd/max8997.c
@@ -464,6 +464,7 @@ static int max8997_suspend(struct device *dev)
 
if (device_may_wakeup(dev))
irq_set_irq_wake(max8997->irq, 1);
+   disable_irq(max8997->irq);
return 0;
 }
 
@@ -474,6 +475,7 @@ static int max8997_resume(struct device *dev)
 
if (device_may_wakeup(dev))
irq_set_irq_wake(max8997->irq, 0);
+   enable_irq(max8997->irq);
return max8997_irq_resume(max8997);
 }
 
-- 
2.17.1

Re: [PATCH 1/2] regulator: Fix useless O^2 complexity in suspend/resume

[Marek Szyprowski]

Hi Mark,

On 2018-09-03 17:09, Mark Brown wrote:
> On Mon, Sep 03, 2018 at 04:49:36PM +0200, Marek Szyprowski wrote:
>> regulator_pm_ops with regulator_suspend and regulator_resume functions are
>> assigned to every regulator device registered in the system, so there is no
>> need to iterate over all again in them. Replace class_for_each_device()
>> construction with direct operation on the rdev embedded in the given
>> regulator device. This saves a lots of useless operations in suspend and
>> resume paths.
> This would've been better as the second patch since it's an optimization
> and not so urgent for stable.

Frankly, the slow down caused by this O^2 in case of 40 regulators in the
system (not that unusual in nowadays mobiles) is noticeable and quite
annoying. IMHO this is still a good candidate for stable.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R Institute Poland

Re: [PATCH 1/2] regulator: Fix useless O^2 complexity in suspend/resume

[Marek Szyprowski]

Hi Mark,

On 2018-09-03 17:09, Mark Brown wrote:
> On Mon, Sep 03, 2018 at 04:49:36PM +0200, Marek Szyprowski wrote:
>> regulator_pm_ops with regulator_suspend and regulator_resume functions are
>> assigned to every regulator device registered in the system, so there is no
>> need to iterate over all again in them. Replace class_for_each_device()
>> construction with direct operation on the rdev embedded in the given
>> regulator device. This saves a lots of useless operations in suspend and
>> resume paths.
> This would've been better as the second patch since it's an optimization
> and not so urgent for stable.

Frankly, the slow down caused by this O^2 in case of 40 regulators in the
system (not that unusual in nowadays mobiles) is noticeable and quite
annoying. IMHO this is still a good candidate for stable.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R Institute Poland

[PATCH 4.18 087/123] s390/mm: fix addressing exception after suspend/resume

[Greg Kroah-Hartman]

4.18-stable review patch.  If anyone has any objections, please let me know.

--

From: Gerald Schaefer 

commit 37a366face294facb9c9d9fdd9f5b64a27456cbd upstream.

Commit c9b5ad546e7d "s390/mm: tag normal pages vs pages used in page tables"
accidentally changed the logic in arch_set_page_states(), which is used by
the suspend/resume code. set_page_stable(page, order) was changed to
set_page_stable_dat(page, 0). After this, only the first page of higher order
pages will be set to stable, and a write to one of the unstable pages will
result in an addressing exception.

Fix this by using "order" again, instead of "0".

Fixes: c9b5ad546e7d ("s390/mm: tag normal pages vs pages used in page tables")
Cc: sta...@vger.kernel.org # 4.14+
Reviewed-by: Heiko Carstens 
Signed-off-by: Gerald Schaefer 
Signed-off-by: Martin Schwidefsky 
Signed-off-by: Greg Kroah-Hartman 

---
 arch/s390/mm/page-states.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/s390/mm/page-states.c
+++ b/arch/s390/mm/page-states.c
@@ -271,7 +271,7 @@ void arch_set_page_states(int make_stabl
list_for_each(l, >free_area[order].free_list[t]) {
page = list_entry(l, struct page, lru);
if (make_stable)
-   set_page_stable_dat(page, 0);
+   set_page_stable_dat(page, order);
else
set_page_unused(page, order);
}

[PATCH 4.18 087/123] s390/mm: fix addressing exception after suspend/resume

[Greg Kroah-Hartman]

4.18-stable review patch.  If anyone has any objections, please let me know.

--

From: Gerald Schaefer 

commit 37a366face294facb9c9d9fdd9f5b64a27456cbd upstream.

Commit c9b5ad546e7d "s390/mm: tag normal pages vs pages used in page tables"
accidentally changed the logic in arch_set_page_states(), which is used by
the suspend/resume code. set_page_stable(page, order) was changed to
set_page_stable_dat(page, 0). After this, only the first page of higher order
pages will be set to stable, and a write to one of the unstable pages will
result in an addressing exception.

Fix this by using "order" again, instead of "0".

Fixes: c9b5ad546e7d ("s390/mm: tag normal pages vs pages used in page tables")
Cc: sta...@vger.kernel.org # 4.14+
Reviewed-by: Heiko Carstens 
Signed-off-by: Gerald Schaefer 
Signed-off-by: Martin Schwidefsky 
Signed-off-by: Greg Kroah-Hartman 

---
 arch/s390/mm/page-states.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/s390/mm/page-states.c
+++ b/arch/s390/mm/page-states.c
@@ -271,7 +271,7 @@ void arch_set_page_states(int make_stabl
list_for_each(l, >free_area[order].free_list[t]) {
page = list_entry(l, struct page, lru);
if (make_stable)
-   set_page_stable_dat(page, 0);
+   set_page_stable_dat(page, order);
else
set_page_unused(page, order);
}

[PATCH 4.14 142/165] s390/mm: fix addressing exception after suspend/resume

[Greg Kroah-Hartman]

4.14-stable review patch.  If anyone has any objections, please let me know.

--

From: Gerald Schaefer 

commit 37a366face294facb9c9d9fdd9f5b64a27456cbd upstream.

Commit c9b5ad546e7d "s390/mm: tag normal pages vs pages used in page tables"
accidentally changed the logic in arch_set_page_states(), which is used by
the suspend/resume code. set_page_stable(page, order) was changed to
set_page_stable_dat(page, 0). After this, only the first page of higher order
pages will be set to stable, and a write to one of the unstable pages will
result in an addressing exception.

Fix this by using "order" again, instead of "0".

Fixes: c9b5ad546e7d ("s390/mm: tag normal pages vs pages used in page tables")
Cc: sta...@vger.kernel.org # 4.14+
Reviewed-by: Heiko Carstens 
Signed-off-by: Gerald Schaefer 
Signed-off-by: Martin Schwidefsky 
Signed-off-by: Greg Kroah-Hartman 

---
 arch/s390/mm/page-states.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/s390/mm/page-states.c
+++ b/arch/s390/mm/page-states.c
@@ -271,7 +271,7 @@ void arch_set_page_states(int make_stabl
list_for_each(l, >free_area[order].free_list[t]) {
page = list_entry(l, struct page, lru);
if (make_stable)
-   set_page_stable_dat(page, 0);
+   set_page_stable_dat(page, order);
else
set_page_unused(page, order);
}

[PATCH 4.14 142/165] s390/mm: fix addressing exception after suspend/resume

[Greg Kroah-Hartman]

4.14-stable review patch.  If anyone has any objections, please let me know.

--

From: Gerald Schaefer 

commit 37a366face294facb9c9d9fdd9f5b64a27456cbd upstream.

Commit c9b5ad546e7d "s390/mm: tag normal pages vs pages used in page tables"
accidentally changed the logic in arch_set_page_states(), which is used by
the suspend/resume code. set_page_stable(page, order) was changed to
set_page_stable_dat(page, 0). After this, only the first page of higher order
pages will be set to stable, and a write to one of the unstable pages will
result in an addressing exception.

Fix this by using "order" again, instead of "0".

Fixes: c9b5ad546e7d ("s390/mm: tag normal pages vs pages used in page tables")
Cc: sta...@vger.kernel.org # 4.14+
Reviewed-by: Heiko Carstens 
Signed-off-by: Gerald Schaefer 
Signed-off-by: Martin Schwidefsky 
Signed-off-by: Greg Kroah-Hartman 

---
 arch/s390/mm/page-states.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/s390/mm/page-states.c
+++ b/arch/s390/mm/page-states.c
@@ -271,7 +271,7 @@ void arch_set_page_states(int make_stabl
list_for_each(l, >free_area[order].free_list[t]) {
page = list_entry(l, struct page, lru);
if (make_stable)
-   set_page_stable_dat(page, 0);
+   set_page_stable_dat(page, order);
else
set_page_unused(page, order);
}

Applied "regulator: Fix useless O^2 complexity in suspend/resume" to the regulator tree

[Mark Brown]

The patch

   regulator: Fix useless O^2 complexity in suspend/resume

has been applied to the regulator tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From cd7e36ab7222af85597517bafd66013cbc8f9877 Mon Sep 17 00:00:00 2001
From: Marek Szyprowski 
Date: Mon, 3 Sep 2018 16:49:36 +0200
Subject: [PATCH] regulator: Fix useless O^2 complexity in suspend/resume

regulator_pm_ops with regulator_suspend and regulator_resume functions are
assigned to every regulator device registered in the system, so there is no
need to iterate over all again in them. Replace class_for_each_device()
construction with direct operation on the rdev embedded in the given
regulator device. This saves a lots of useless operations in suspend and
resume paths.

Fixes: f7efad10b5c4: regulator: add PM suspend and resume hooks
Signed-off-by: Marek Szyprowski 
Signed-off-by: Mark Brown 
---
 drivers/regulator/core.c | 39 +++
 1 file changed, 11 insertions(+), 28 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index f686f2311317..a147871af09b 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -4464,19 +4464,6 @@ void regulator_unregister(struct regulator_dev *rdev)
 EXPORT_SYMBOL_GPL(regulator_unregister);
 
 #ifdef CONFIG_SUSPEND
-static int _regulator_suspend(struct device *dev, void *data)
-{
-   struct regulator_dev *rdev = dev_to_rdev(dev);
-   suspend_state_t *state = data;
-   int ret;
-
-   regulator_lock(rdev);
-   ret = suspend_set_state(rdev, *state);
-   regulator_unlock(rdev);
-
-   return ret;
-}
-
 /**
  * regulator_suspend - prepare regulators for system wide suspend
  * @state: system suspend state
@@ -4485,20 +4472,25 @@ static int _regulator_suspend(struct device *dev, void 
*data)
  */
 static int regulator_suspend(struct device *dev)
 {
+   struct regulator_dev *rdev = dev_to_rdev(dev);
suspend_state_t state = pm_suspend_target_state;
+   int ret;
+
+   regulator_lock(rdev);
+   ret = suspend_set_state(rdev, state);
+   regulator_unlock(rdev);
 
-   return class_for_each_device(_class, NULL, ,
-_regulator_suspend);
+   return ret;
 }
 
-static int _regulator_resume(struct device *dev, void *data)
+static int regulator_resume(struct device *dev)
 {
-   int ret = 0;
+   suspend_state_t state = pm_suspend_target_state;
struct regulator_dev *rdev = dev_to_rdev(dev);
-   suspend_state_t *state = data;
struct regulator_state *rstate;
+   int ret = 0;
 
-   rstate = regulator_get_suspend_state(rdev, *state);
+   rstate = regulator_get_suspend_state(rdev, state);
if (rstate == NULL)
return 0;
 
@@ -4513,15 +4505,6 @@ static int _regulator_resume(struct device *dev, void 
*data)
 
return ret;
 }
-
-static int regulator_resume(struct device *dev)
-{
-   suspend_state_t state = pm_suspend_target_state;
-
-   return class_for_each_device(_class, NULL, ,
-_regulator_resume);
-}
-
 #else /* !CONFIG_SUSPEND */
 
 #define regulator_suspend  NULL
-- 
2.19.0.rc1

Applied "regulator: Fix useless O^2 complexity in suspend/resume" to the regulator tree

[Mark Brown]

The patch

   regulator: Fix useless O^2 complexity in suspend/resume

has been applied to the regulator tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From cd7e36ab7222af85597517bafd66013cbc8f9877 Mon Sep 17 00:00:00 2001
From: Marek Szyprowski 
Date: Mon, 3 Sep 2018 16:49:36 +0200
Subject: [PATCH] regulator: Fix useless O^2 complexity in suspend/resume

regulator_pm_ops with regulator_suspend and regulator_resume functions are
assigned to every regulator device registered in the system, so there is no
need to iterate over all again in them. Replace class_for_each_device()
construction with direct operation on the rdev embedded in the given
regulator device. This saves a lots of useless operations in suspend and
resume paths.

Fixes: f7efad10b5c4: regulator: add PM suspend and resume hooks
Signed-off-by: Marek Szyprowski 
Signed-off-by: Mark Brown 
---
 drivers/regulator/core.c | 39 +++
 1 file changed, 11 insertions(+), 28 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index f686f2311317..a147871af09b 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -4464,19 +4464,6 @@ void regulator_unregister(struct regulator_dev *rdev)
 EXPORT_SYMBOL_GPL(regulator_unregister);
 
 #ifdef CONFIG_SUSPEND
-static int _regulator_suspend(struct device *dev, void *data)
-{
-   struct regulator_dev *rdev = dev_to_rdev(dev);
-   suspend_state_t *state = data;
-   int ret;
-
-   regulator_lock(rdev);
-   ret = suspend_set_state(rdev, *state);
-   regulator_unlock(rdev);
-
-   return ret;
-}
-
 /**
  * regulator_suspend - prepare regulators for system wide suspend
  * @state: system suspend state
@@ -4485,20 +4472,25 @@ static int _regulator_suspend(struct device *dev, void 
*data)
  */
 static int regulator_suspend(struct device *dev)
 {
+   struct regulator_dev *rdev = dev_to_rdev(dev);
suspend_state_t state = pm_suspend_target_state;
+   int ret;
+
+   regulator_lock(rdev);
+   ret = suspend_set_state(rdev, state);
+   regulator_unlock(rdev);
 
-   return class_for_each_device(_class, NULL, ,
-_regulator_suspend);
+   return ret;
 }
 
-static int _regulator_resume(struct device *dev, void *data)
+static int regulator_resume(struct device *dev)
 {
-   int ret = 0;
+   suspend_state_t state = pm_suspend_target_state;
struct regulator_dev *rdev = dev_to_rdev(dev);
-   suspend_state_t *state = data;
struct regulator_state *rstate;
+   int ret = 0;
 
-   rstate = regulator_get_suspend_state(rdev, *state);
+   rstate = regulator_get_suspend_state(rdev, state);
if (rstate == NULL)
return 0;
 
@@ -4513,15 +4505,6 @@ static int _regulator_resume(struct device *dev, void 
*data)
 
return ret;
 }
-
-static int regulator_resume(struct device *dev)
-{
-   suspend_state_t state = pm_suspend_target_state;
-
-   return class_for_each_device(_class, NULL, ,
-_regulator_resume);
-}
-
 #else /* !CONFIG_SUSPEND */
 
 #define regulator_suspend  NULL
-- 
2.19.0.rc1

Re: [PATCH 0/2] regualtors: Fix suspend/resume issues since v4.16

[Mark Brown]

On Mon, Sep 03, 2018 at 04:49:35PM +0200, Marek Szyprowski wrote:

> I've been really surprised that no-one noticed those issues for almost
> 4 kernel releases. Please review my fixes and apply to v4.19-rcX if
> possible.

I suspect this is down to relatively few systems having devices with
suspend operations, and fewer actually ever suspending in use.


signature.asc
Description: PGP signature

Re: [PATCH 0/2] regualtors: Fix suspend/resume issues since v4.16

[Mark Brown]

On Mon, Sep 03, 2018 at 04:49:35PM +0200, Marek Szyprowski wrote:

> I've been really surprised that no-one noticed those issues for almost
> 4 kernel releases. Please review my fixes and apply to v4.19-rcX if
> possible.

I suspect this is down to relatively few systems having devices with
suspend operations, and fewer actually ever suspending in use.


signature.asc
Description: PGP signature

Re: [PATCH 1/2] regulator: Fix useless O^2 complexity in suspend/resume

[Mark Brown]

On Mon, Sep 03, 2018 at 04:49:36PM +0200, Marek Szyprowski wrote:
> regulator_pm_ops with regulator_suspend and regulator_resume functions are
> assigned to every regulator device registered in the system, so there is no
> need to iterate over all again in them. Replace class_for_each_device()
> construction with direct operation on the rdev embedded in the given
> regulator device. This saves a lots of useless operations in suspend and
> resume paths.

This would've been better as the second patch since it's an optimization
and not so urgent for stable.


signature.asc
Description: PGP signature

Re: [PATCH 1/2] regulator: Fix useless O^2 complexity in suspend/resume

[Mark Brown]

On Mon, Sep 03, 2018 at 04:49:36PM +0200, Marek Szyprowski wrote:
> regulator_pm_ops with regulator_suspend and regulator_resume functions are
> assigned to every regulator device registered in the system, so there is no
> need to iterate over all again in them. Replace class_for_each_device()
> construction with direct operation on the rdev embedded in the given
> regulator device. This saves a lots of useless operations in suspend and
> resume paths.

This would've been better as the second patch since it's an optimization
and not so urgent for stable.


signature.asc
Description: PGP signature

[PATCH 0/2] regualtors: Fix suspend/resume issues since v4.16

[Marek Szyprowski]

Hi All,

While working on suspend/resume support for Samsung Exynos5433-based TM2
board (arch/arm64/boot/dts/exynos/exynos5433-tm2.dts), I've noticed that
v4.16 kernel release introduced some serious problems with regulator
configuration in system suspend state. Further investigation revealed
that the following 2 commits are responsible for my issues:

1. f7efad10b5c4: regulator: add PM suspend and resume hooks
2. 72069f9957a1: regulator: leave one item to record whether regulator is 
enabled

I've been really surprised that no-one noticed those issues for almost
4 kernel releases. Please review my fixes and apply to v4.19-rcX if
possible.

Best regards
Marek Szyprowski, PhD
Samsung R Institute Poland


Patch summary:

Marek Szyprowski (2):
  regulator: Fix useless O^2 complexity in suspend/resume
  regulator: Fix 'do-nothing' value for regulators without suspend state

 drivers/regulator/core.c  | 41 +--
 drivers/regulator/of_regulator.c  |  2 --
 include/linux/regulator/machine.h |  6 ++---
 3 files changed, 15 insertions(+), 34 deletions(-)

-- 
2.17.1

[PATCH 0/2] regualtors: Fix suspend/resume issues since v4.16

[Marek Szyprowski]

Hi All,

While working on suspend/resume support for Samsung Exynos5433-based TM2
board (arch/arm64/boot/dts/exynos/exynos5433-tm2.dts), I've noticed that
v4.16 kernel release introduced some serious problems with regulator
configuration in system suspend state. Further investigation revealed
that the following 2 commits are responsible for my issues:

1. f7efad10b5c4: regulator: add PM suspend and resume hooks
2. 72069f9957a1: regulator: leave one item to record whether regulator is 
enabled

I've been really surprised that no-one noticed those issues for almost
4 kernel releases. Please review my fixes and apply to v4.19-rcX if
possible.

Best regards
Marek Szyprowski, PhD
Samsung R Institute Poland


Patch summary:

Marek Szyprowski (2):
  regulator: Fix useless O^2 complexity in suspend/resume
  regulator: Fix 'do-nothing' value for regulators without suspend state

 drivers/regulator/core.c  | 41 +--
 drivers/regulator/of_regulator.c  |  2 --
 include/linux/regulator/machine.h |  6 ++---
 3 files changed, 15 insertions(+), 34 deletions(-)

-- 
2.17.1

[PATCH 1/2] regulator: Fix useless O^2 complexity in suspend/resume

[Marek Szyprowski]

regulator_pm_ops with regulator_suspend and regulator_resume functions are
assigned to every regulator device registered in the system, so there is no
need to iterate over all again in them. Replace class_for_each_device()
construction with direct operation on the rdev embedded in the given
regulator device. This saves a lots of useless operations in suspend and
resume paths.

Fixes: f7efad10b5c4: regulator: add PM suspend and resume hooks
Signed-off-by: Marek Szyprowski 
---
 drivers/regulator/core.c | 39 +++
 1 file changed, 11 insertions(+), 28 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index bb1324f93143..71ba040c7c5b 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -4455,19 +4455,6 @@ void regulator_unregister(struct regulator_dev *rdev)
 EXPORT_SYMBOL_GPL(regulator_unregister);
 
 #ifdef CONFIG_SUSPEND
-static int _regulator_suspend(struct device *dev, void *data)
-{
-   struct regulator_dev *rdev = dev_to_rdev(dev);
-   suspend_state_t *state = data;
-   int ret;
-
-   regulator_lock(rdev);
-   ret = suspend_set_state(rdev, *state);
-   regulator_unlock(rdev);
-
-   return ret;
-}
-
 /**
  * regulator_suspend - prepare regulators for system wide suspend
  * @state: system suspend state
@@ -4476,20 +4463,25 @@ static int _regulator_suspend(struct device *dev, void 
*data)
  */
 static int regulator_suspend(struct device *dev)
 {
+   struct regulator_dev *rdev = dev_to_rdev(dev);
suspend_state_t state = pm_suspend_target_state;
+   int ret;
+
+   regulator_lock(rdev);
+   ret = suspend_set_state(rdev, state);
+   regulator_unlock(rdev);
 
-   return class_for_each_device(_class, NULL, ,
-_regulator_suspend);
+   return ret;
 }
 
-static int _regulator_resume(struct device *dev, void *data)
+static int regulator_resume(struct device *dev)
 {
-   int ret = 0;
+   suspend_state_t state = pm_suspend_target_state;
struct regulator_dev *rdev = dev_to_rdev(dev);
-   suspend_state_t *state = data;
struct regulator_state *rstate;
+   int ret = 0;
 
-   rstate = regulator_get_suspend_state(rdev, *state);
+   rstate = regulator_get_suspend_state(rdev, state);
if (rstate == NULL)
return 0;
 
@@ -4504,15 +4496,6 @@ static int _regulator_resume(struct device *dev, void 
*data)
 
return ret;
 }
-
-static int regulator_resume(struct device *dev)
-{
-   suspend_state_t state = pm_suspend_target_state;
-
-   return class_for_each_device(_class, NULL, ,
-_regulator_resume);
-}
-
 #else /* !CONFIG_SUSPEND */
 
 #define regulator_suspend  NULL
-- 
2.17.1

[PATCH 1/2] regulator: Fix useless O^2 complexity in suspend/resume

[Marek Szyprowski]

regulator_pm_ops with regulator_suspend and regulator_resume functions are
assigned to every regulator device registered in the system, so there is no
need to iterate over all again in them. Replace class_for_each_device()
construction with direct operation on the rdev embedded in the given
regulator device. This saves a lots of useless operations in suspend and
resume paths.

Fixes: f7efad10b5c4: regulator: add PM suspend and resume hooks
Signed-off-by: Marek Szyprowski 
---
 drivers/regulator/core.c | 39 +++
 1 file changed, 11 insertions(+), 28 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index bb1324f93143..71ba040c7c5b 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -4455,19 +4455,6 @@ void regulator_unregister(struct regulator_dev *rdev)
 EXPORT_SYMBOL_GPL(regulator_unregister);
 
 #ifdef CONFIG_SUSPEND
-static int _regulator_suspend(struct device *dev, void *data)
-{
-   struct regulator_dev *rdev = dev_to_rdev(dev);
-   suspend_state_t *state = data;
-   int ret;
-
-   regulator_lock(rdev);
-   ret = suspend_set_state(rdev, *state);
-   regulator_unlock(rdev);
-
-   return ret;
-}
-
 /**
  * regulator_suspend - prepare regulators for system wide suspend
  * @state: system suspend state
@@ -4476,20 +4463,25 @@ static int _regulator_suspend(struct device *dev, void 
*data)
  */
 static int regulator_suspend(struct device *dev)
 {
+   struct regulator_dev *rdev = dev_to_rdev(dev);
suspend_state_t state = pm_suspend_target_state;
+   int ret;
+
+   regulator_lock(rdev);
+   ret = suspend_set_state(rdev, state);
+   regulator_unlock(rdev);
 
-   return class_for_each_device(_class, NULL, ,
-_regulator_suspend);
+   return ret;
 }
 
-static int _regulator_resume(struct device *dev, void *data)
+static int regulator_resume(struct device *dev)
 {
-   int ret = 0;
+   suspend_state_t state = pm_suspend_target_state;
struct regulator_dev *rdev = dev_to_rdev(dev);
-   suspend_state_t *state = data;
struct regulator_state *rstate;
+   int ret = 0;
 
-   rstate = regulator_get_suspend_state(rdev, *state);
+   rstate = regulator_get_suspend_state(rdev, state);
if (rstate == NULL)
return 0;
 
@@ -4504,15 +4496,6 @@ static int _regulator_resume(struct device *dev, void 
*data)
 
return ret;
 }
-
-static int regulator_resume(struct device *dev)
-{
-   suspend_state_t state = pm_suspend_target_state;
-
-   return class_for_each_device(_class, NULL, ,
-_regulator_resume);
-}
-
 #else /* !CONFIG_SUSPEND */
 
 #define regulator_suspend  NULL
-- 
2.17.1

[PATCH v1 0/2] LP1/LP2 suspend-resume CPU clock fixes for Tegra30

[Dmitry Osipenko]

Hello,

This patch-series fixes CPU hanging after suspend-resume / LP2 cpuidle
on Tegra30. The bug really appears during stress-testing, like frequent
suspending under variable load + the upcoming Tegra30 CPUFREQ driver.

Dmitry Osipenko (2):
  ARM: tegra: Switch CPU to PLLP before powergating on Tegra30
  ARM: tegra: Switch CPU to PLLP on resume from LP1 on Tegra30

 arch/arm/mach-tegra/sleep-tegra30.S | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.18.0

[PATCH v1 0/2] LP1/LP2 suspend-resume CPU clock fixes for Tegra30

[Dmitry Osipenko]

Hello,

This patch-series fixes CPU hanging after suspend-resume / LP2 cpuidle
on Tegra30. The bug really appears during stress-testing, like frequent
suspending under variable load + the upcoming Tegra30 CPUFREQ driver.

Dmitry Osipenko (2):
  ARM: tegra: Switch CPU to PLLP before powergating on Tegra30
  ARM: tegra: Switch CPU to PLLP on resume from LP1 on Tegra30

 arch/arm/mach-tegra/sleep-tegra30.S | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.18.0

[PATCH v3 8/8] ARM: tegra: Add firmware calls required for suspend-resume

[Dmitry Osipenko]

In order to resume CPU from suspend via trusted Foundations firmware,
the LP1/LP2 boot vectors shall be specified using the firmware calls.

Signed-off-by: Dmitry Osipenko 
---
 arch/arm/mach-tegra/pm.c|  7 ++
 arch/arm/mach-tegra/reset-handler.S | 33 +++--
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
index 66c8cd63dd86..12341ffabb99 100644
--- a/arch/arm/mach-tegra/pm.c
+++ b/arch/arm/mach-tegra/pm.c
@@ -33,6 +33,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -150,6 +151,10 @@ bool tegra_set_cpu_in_lp2(void)
tegra20_cpu_set_resettable_soon();
 
spin_unlock(_lp2_lock);
+
+   if (last_cpu)
+   call_firmware_op(prepare_idle, TF_PM_MODE_LP2);
+
return last_cpu;
 }
 
@@ -316,6 +321,8 @@ static void tegra_suspend_enter_lp1(void)
tegra_lp1_iram.start_addr, iram_save_size);
 
*((u32 *)tegra_cpu_lp1_mask) = 1;
+
+   call_firmware_op(prepare_idle, TF_PM_MODE_LP1);
 }
 
 static void tegra_suspend_exit_lp1(void)
diff --git a/arch/arm/mach-tegra/reset-handler.S 
b/arch/arm/mach-tegra/reset-handler.S
index 4973ea053bd7..0e208b2e246e 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -69,7 +69,7 @@ ENTRY(tegra_resume)
 
mov32   r9, 0xc09
cmp r8, r9
-   bne end_ca9_scu_resume
+   bne end_ca9_scu_l2_resume
 #ifdef CONFIG_HAVE_ARM_SCU
/* enable SCU */
mov32   r0, TEGRA_ARM_PERIF_BASE
@@ -77,14 +77,43 @@ ENTRY(tegra_resume)
orr r1, r1, #1
str r1, [r0]
 #endif
+   bl  tegra_resume_trusted_foundations
 
-end_ca9_scu_resume:
+#ifdef CONFIG_CACHE_L2X0
+   /* L2 cache resume & re-enable */
+   bleql2c310_early_resume @ No, resume cache early
+#endif
+end_ca9_scu_l2_resume:
mov32   r9, 0xc0f
cmp r8, r9
bleqtegra_init_l2_for_a15
 
b   cpu_resume
 ENDPROC(tegra_resume)
+
+/*
+ * tegra_resume_trusted_foundations
+ *
+ *   Trusted Foundations firmware initialization.
+ *
+ * Doesn't return if firmware presents.
+ * Corrupted registers: r1, r2
+ */
+ENTRY(tegra_resume_trusted_foundations)
+   /* Check whether Trusted Foundations firmware presents. */
+   mov32   r2, TEGRA_IRAM_BASE + TEGRA_IRAM_RESET_HANDLER_OFFSET
+   ldr r1, =__tegra_cpu_reset_handler_data_offset + \
+   RESET_DATA(TF_PRESENT)
+   ldr r1, [r2, r1]
+   cmp r1, #0
+   reteq   lr
+
+ .arch_extension sec
+   /* First call after suspend wakes firmware. No arguments required. */
+   smc #0
+
+   b   cpu_resume
+ENDPROC(tegra_resume_trusted_foundations)
 #endif
 
.align L1_CACHE_SHIFT
-- 
2.18.0

[PATCH v3 8/8] ARM: tegra: Add firmware calls required for suspend-resume

[Dmitry Osipenko]

In order to resume CPU from suspend via trusted Foundations firmware,
the LP1/LP2 boot vectors shall be specified using the firmware calls.

Signed-off-by: Dmitry Osipenko 
---
 arch/arm/mach-tegra/pm.c|  7 ++
 arch/arm/mach-tegra/reset-handler.S | 33 +++--
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
index 66c8cd63dd86..12341ffabb99 100644
--- a/arch/arm/mach-tegra/pm.c
+++ b/arch/arm/mach-tegra/pm.c
@@ -33,6 +33,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -150,6 +151,10 @@ bool tegra_set_cpu_in_lp2(void)
tegra20_cpu_set_resettable_soon();
 
spin_unlock(_lp2_lock);
+
+   if (last_cpu)
+   call_firmware_op(prepare_idle, TF_PM_MODE_LP2);
+
return last_cpu;
 }
 
@@ -316,6 +321,8 @@ static void tegra_suspend_enter_lp1(void)
tegra_lp1_iram.start_addr, iram_save_size);
 
*((u32 *)tegra_cpu_lp1_mask) = 1;
+
+   call_firmware_op(prepare_idle, TF_PM_MODE_LP1);
 }
 
 static void tegra_suspend_exit_lp1(void)
diff --git a/arch/arm/mach-tegra/reset-handler.S 
b/arch/arm/mach-tegra/reset-handler.S
index 4973ea053bd7..0e208b2e246e 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -69,7 +69,7 @@ ENTRY(tegra_resume)
 
mov32   r9, 0xc09
cmp r8, r9
-   bne end_ca9_scu_resume
+   bne end_ca9_scu_l2_resume
 #ifdef CONFIG_HAVE_ARM_SCU
/* enable SCU */
mov32   r0, TEGRA_ARM_PERIF_BASE
@@ -77,14 +77,43 @@ ENTRY(tegra_resume)
orr r1, r1, #1
str r1, [r0]
 #endif
+   bl  tegra_resume_trusted_foundations
 
-end_ca9_scu_resume:
+#ifdef CONFIG_CACHE_L2X0
+   /* L2 cache resume & re-enable */
+   bleql2c310_early_resume @ No, resume cache early
+#endif
+end_ca9_scu_l2_resume:
mov32   r9, 0xc0f
cmp r8, r9
bleqtegra_init_l2_for_a15
 
b   cpu_resume
 ENDPROC(tegra_resume)
+
+/*
+ * tegra_resume_trusted_foundations
+ *
+ *   Trusted Foundations firmware initialization.
+ *
+ * Doesn't return if firmware presents.
+ * Corrupted registers: r1, r2
+ */
+ENTRY(tegra_resume_trusted_foundations)
+   /* Check whether Trusted Foundations firmware presents. */
+   mov32   r2, TEGRA_IRAM_BASE + TEGRA_IRAM_RESET_HANDLER_OFFSET
+   ldr r1, =__tegra_cpu_reset_handler_data_offset + \
+   RESET_DATA(TF_PRESENT)
+   ldr r1, [r2, r1]
+   cmp r1, #0
+   reteq   lr
+
+ .arch_extension sec
+   /* First call after suspend wakes firmware. No arguments required. */
+   smc #0
+
+   b   cpu_resume
+ENDPROC(tegra_resume_trusted_foundations)
 #endif
 
.align L1_CACHE_SHIFT
-- 
2.18.0

[PATCH 3.18 42/56] ARM: pxa: irq: fix handling of ICMR registers in suspend/resume

[Greg Kroah-Hartman]

3.18-stable review patch.  If anyone has any objections, please let me know.

--

From: Daniel Mack 

[ Upstream commit 0c1049dcb4ceec640d8bd797335bcbebdcab44d2 ]

PXA3xx platforms have 56 interrupts that are stored in two ICMR
registers. The code in pxa_irq_suspend() and pxa_irq_resume() however
does a simple division by 32 which only leads to one register being
saved at suspend and restored at resume time. The NAND interrupt
setting, for instance, is lost.

Fix this by using DIV_ROUND_UP() instead.

Signed-off-by: Daniel Mack 
Signed-off-by: Robert Jarzmik 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman 
---
 arch/arm/mach-pxa/irq.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/arch/arm/mach-pxa/irq.c
+++ b/arch/arm/mach-pxa/irq.c
@@ -160,7 +160,7 @@ static int pxa_irq_suspend(void)
 {
int i;
 
-   for (i = 0; i < pxa_internal_irq_nr / 32; i++) {
+   for (i = 0; i < DIV_ROUND_UP(pxa_internal_irq_nr, 32); i++) {
void __iomem *base = irq_base(i);
 
saved_icmr[i] = __raw_readl(base + ICMR);
@@ -179,7 +179,7 @@ static void pxa_irq_resume(void)
 {
int i;
 
-   for (i = 0; i < pxa_internal_irq_nr / 32; i++) {
+   for (i = 0; i < DIV_ROUND_UP(pxa_internal_irq_nr, 32); i++) {
void __iomem *base = irq_base(i);
 
__raw_writel(saved_icmr[i], base + ICMR);

[PATCH 3.18 42/56] ARM: pxa: irq: fix handling of ICMR registers in suspend/resume

[Greg Kroah-Hartman]

3.18-stable review patch.  If anyone has any objections, please let me know.

--

From: Daniel Mack 

[ Upstream commit 0c1049dcb4ceec640d8bd797335bcbebdcab44d2 ]

PXA3xx platforms have 56 interrupts that are stored in two ICMR
registers. The code in pxa_irq_suspend() and pxa_irq_resume() however
does a simple division by 32 which only leads to one register being
saved at suspend and restored at resume time. The NAND interrupt
setting, for instance, is lost.

Fix this by using DIV_ROUND_UP() instead.

Signed-off-by: Daniel Mack 
Signed-off-by: Robert Jarzmik 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman 
---
 arch/arm/mach-pxa/irq.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/arch/arm/mach-pxa/irq.c
+++ b/arch/arm/mach-pxa/irq.c
@@ -160,7 +160,7 @@ static int pxa_irq_suspend(void)
 {
int i;
 
-   for (i = 0; i < pxa_internal_irq_nr / 32; i++) {
+   for (i = 0; i < DIV_ROUND_UP(pxa_internal_irq_nr, 32); i++) {
void __iomem *base = irq_base(i);
 
saved_icmr[i] = __raw_readl(base + ICMR);
@@ -179,7 +179,7 @@ static void pxa_irq_resume(void)
 {
int i;
 
-   for (i = 0; i < pxa_internal_irq_nr / 32; i++) {
+   for (i = 0; i < DIV_ROUND_UP(pxa_internal_irq_nr, 32); i++) {
void __iomem *base = irq_base(i);
 
__raw_writel(saved_icmr[i], base + ICMR);

[PATCH 4.17 206/324] ARM: pxa: irq: fix handling of ICMR registers in suspend/resume

[Greg Kroah-Hartman]

4.17-stable review patch.  If anyone has any objections, please let me know.

--

From: Daniel Mack 

[ Upstream commit 0c1049dcb4ceec640d8bd797335bcbebdcab44d2 ]

PXA3xx platforms have 56 interrupts that are stored in two ICMR
registers. The code in pxa_irq_suspend() and pxa_irq_resume() however
does a simple division by 32 which only leads to one register being
saved at suspend and restored at resume time. The NAND interrupt
setting, for instance, is lost.

Fix this by using DIV_ROUND_UP() instead.

Signed-off-by: Daniel Mack 
Signed-off-by: Robert Jarzmik 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman 
---
 arch/arm/mach-pxa/irq.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/arch/arm/mach-pxa/irq.c
+++ b/arch/arm/mach-pxa/irq.c
@@ -185,7 +185,7 @@ static int pxa_irq_suspend(void)
 {
int i;
 
-   for (i = 0; i < pxa_internal_irq_nr / 32; i++) {
+   for (i = 0; i < DIV_ROUND_UP(pxa_internal_irq_nr, 32); i++) {
void __iomem *base = irq_base(i);
 
saved_icmr[i] = __raw_readl(base + ICMR);
@@ -204,7 +204,7 @@ static void pxa_irq_resume(void)
 {
int i;
 
-   for (i = 0; i < pxa_internal_irq_nr / 32; i++) {
+   for (i = 0; i < DIV_ROUND_UP(pxa_internal_irq_nr, 32); i++) {
void __iomem *base = irq_base(i);
 
__raw_writel(saved_icmr[i], base + ICMR);

[PATCH 4.17 206/324] ARM: pxa: irq: fix handling of ICMR registers in suspend/resume

[Greg Kroah-Hartman]

4.17-stable review patch.  If anyone has any objections, please let me know.

--

From: Daniel Mack 

[ Upstream commit 0c1049dcb4ceec640d8bd797335bcbebdcab44d2 ]

PXA3xx platforms have 56 interrupts that are stored in two ICMR
registers. The code in pxa_irq_suspend() and pxa_irq_resume() however
does a simple division by 32 which only leads to one register being
saved at suspend and restored at resume time. The NAND interrupt
setting, for instance, is lost.

Fix this by using DIV_ROUND_UP() instead.

Signed-off-by: Daniel Mack 
Signed-off-by: Robert Jarzmik 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman 
---
 arch/arm/mach-pxa/irq.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/arch/arm/mach-pxa/irq.c
+++ b/arch/arm/mach-pxa/irq.c
@@ -185,7 +185,7 @@ static int pxa_irq_suspend(void)
 {
int i;
 
-   for (i = 0; i < pxa_internal_irq_nr / 32; i++) {
+   for (i = 0; i < DIV_ROUND_UP(pxa_internal_irq_nr, 32); i++) {
void __iomem *base = irq_base(i);
 
saved_icmr[i] = __raw_readl(base + ICMR);
@@ -204,7 +204,7 @@ static void pxa_irq_resume(void)
 {
int i;
 
-   for (i = 0; i < pxa_internal_irq_nr / 32; i++) {
+   for (i = 0; i < DIV_ROUND_UP(pxa_internal_irq_nr, 32); i++) {
void __iomem *base = irq_base(i);
 
__raw_writel(saved_icmr[i], base + ICMR);

[PATCH 4.14 149/217] ARM: pxa: irq: fix handling of ICMR registers in suspend/resume

[Greg Kroah-Hartman]

4.14-stable review patch.  If anyone has any objections, please let me know.

--

From: Daniel Mack 

[ Upstream commit 0c1049dcb4ceec640d8bd797335bcbebdcab44d2 ]

PXA3xx platforms have 56 interrupts that are stored in two ICMR
registers. The code in pxa_irq_suspend() and pxa_irq_resume() however
does a simple division by 32 which only leads to one register being
saved at suspend and restored at resume time. The NAND interrupt
setting, for instance, is lost.

Fix this by using DIV_ROUND_UP() instead.

Signed-off-by: Daniel Mack 
Signed-off-by: Robert Jarzmik 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman 
---
 arch/arm/mach-pxa/irq.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/arch/arm/mach-pxa/irq.c
+++ b/arch/arm/mach-pxa/irq.c
@@ -185,7 +185,7 @@ static int pxa_irq_suspend(void)
 {
int i;
 
-   for (i = 0; i < pxa_internal_irq_nr / 32; i++) {
+   for (i = 0; i < DIV_ROUND_UP(pxa_internal_irq_nr, 32); i++) {
void __iomem *base = irq_base(i);
 
saved_icmr[i] = __raw_readl(base + ICMR);
@@ -204,7 +204,7 @@ static void pxa_irq_resume(void)
 {
int i;
 
-   for (i = 0; i < pxa_internal_irq_nr / 32; i++) {
+   for (i = 0; i < DIV_ROUND_UP(pxa_internal_irq_nr, 32); i++) {
void __iomem *base = irq_base(i);
 
__raw_writel(saved_icmr[i], base + ICMR);

[PATCH 4.14 149/217] ARM: pxa: irq: fix handling of ICMR registers in suspend/resume

[Greg Kroah-Hartman]

4.14-stable review patch.  If anyone has any objections, please let me know.

--

From: Daniel Mack 

[ Upstream commit 0c1049dcb4ceec640d8bd797335bcbebdcab44d2 ]

PXA3xx platforms have 56 interrupts that are stored in two ICMR
registers. The code in pxa_irq_suspend() and pxa_irq_resume() however
does a simple division by 32 which only leads to one register being
saved at suspend and restored at resume time. The NAND interrupt
setting, for instance, is lost.

Fix this by using DIV_ROUND_UP() instead.

Signed-off-by: Daniel Mack 
Signed-off-by: Robert Jarzmik 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman 
---
 arch/arm/mach-pxa/irq.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/arch/arm/mach-pxa/irq.c
+++ b/arch/arm/mach-pxa/irq.c
@@ -185,7 +185,7 @@ static int pxa_irq_suspend(void)
 {
int i;
 
-   for (i = 0; i < pxa_internal_irq_nr / 32; i++) {
+   for (i = 0; i < DIV_ROUND_UP(pxa_internal_irq_nr, 32); i++) {
void __iomem *base = irq_base(i);
 
saved_icmr[i] = __raw_readl(base + ICMR);
@@ -204,7 +204,7 @@ static void pxa_irq_resume(void)
 {
int i;
 
-   for (i = 0; i < pxa_internal_irq_nr / 32; i++) {
+   for (i = 0; i < DIV_ROUND_UP(pxa_internal_irq_nr, 32); i++) {
void __iomem *base = irq_base(i);
 
__raw_writel(saved_icmr[i], base + ICMR);

[PATCH 4.9 086/130] ARM: pxa: irq: fix handling of ICMR registers in suspend/resume

[Greg Kroah-Hartman]

4.9-stable review patch.  If anyone has any objections, please let me know.

--

From: Daniel Mack 

[ Upstream commit 0c1049dcb4ceec640d8bd797335bcbebdcab44d2 ]

PXA3xx platforms have 56 interrupts that are stored in two ICMR
registers. The code in pxa_irq_suspend() and pxa_irq_resume() however
does a simple division by 32 which only leads to one register being
saved at suspend and restored at resume time. The NAND interrupt
setting, for instance, is lost.

Fix this by using DIV_ROUND_UP() instead.

Signed-off-by: Daniel Mack 
Signed-off-by: Robert Jarzmik 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman 
---
 arch/arm/mach-pxa/irq.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/arch/arm/mach-pxa/irq.c
+++ b/arch/arm/mach-pxa/irq.c
@@ -185,7 +185,7 @@ static int pxa_irq_suspend(void)
 {
int i;
 
-   for (i = 0; i < pxa_internal_irq_nr / 32; i++) {
+   for (i = 0; i < DIV_ROUND_UP(pxa_internal_irq_nr, 32); i++) {
void __iomem *base = irq_base(i);
 
saved_icmr[i] = __raw_readl(base + ICMR);
@@ -204,7 +204,7 @@ static void pxa_irq_resume(void)
 {
int i;
 
-   for (i = 0; i < pxa_internal_irq_nr / 32; i++) {
+   for (i = 0; i < DIV_ROUND_UP(pxa_internal_irq_nr, 32); i++) {
void __iomem *base = irq_base(i);
 
__raw_writel(saved_icmr[i], base + ICMR);

[PATCH 4.9 086/130] ARM: pxa: irq: fix handling of ICMR registers in suspend/resume

[Greg Kroah-Hartman]

4.9-stable review patch.  If anyone has any objections, please let me know.

--

From: Daniel Mack 

[ Upstream commit 0c1049dcb4ceec640d8bd797335bcbebdcab44d2 ]

PXA3xx platforms have 56 interrupts that are stored in two ICMR
registers. The code in pxa_irq_suspend() and pxa_irq_resume() however
does a simple division by 32 which only leads to one register being
saved at suspend and restored at resume time. The NAND interrupt
setting, for instance, is lost.

Fix this by using DIV_ROUND_UP() instead.

Signed-off-by: Daniel Mack 
Signed-off-by: Robert Jarzmik 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman 
---
 arch/arm/mach-pxa/irq.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/arch/arm/mach-pxa/irq.c
+++ b/arch/arm/mach-pxa/irq.c
@@ -185,7 +185,7 @@ static int pxa_irq_suspend(void)
 {
int i;
 
-   for (i = 0; i < pxa_internal_irq_nr / 32; i++) {
+   for (i = 0; i < DIV_ROUND_UP(pxa_internal_irq_nr, 32); i++) {
void __iomem *base = irq_base(i);
 
saved_icmr[i] = __raw_readl(base + ICMR);
@@ -204,7 +204,7 @@ static void pxa_irq_resume(void)
 {
int i;
 
-   for (i = 0; i < pxa_internal_irq_nr / 32; i++) {
+   for (i = 0; i < DIV_ROUND_UP(pxa_internal_irq_nr, 32); i++) {
void __iomem *base = irq_base(i);
 
__raw_writel(saved_icmr[i], base + ICMR);

[PATCH 4.4 46/79] ARM: pxa: irq: fix handling of ICMR registers in suspend/resume

[Greg Kroah-Hartman]

4.4-stable review patch.  If anyone has any objections, please let me know.

--

From: Daniel Mack 

[ Upstream commit 0c1049dcb4ceec640d8bd797335bcbebdcab44d2 ]

PXA3xx platforms have 56 interrupts that are stored in two ICMR
registers. The code in pxa_irq_suspend() and pxa_irq_resume() however
does a simple division by 32 which only leads to one register being
saved at suspend and restored at resume time. The NAND interrupt
setting, for instance, is lost.

Fix this by using DIV_ROUND_UP() instead.

Signed-off-by: Daniel Mack 
Signed-off-by: Robert Jarzmik 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman 
---
 arch/arm/mach-pxa/irq.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/arch/arm/mach-pxa/irq.c
+++ b/arch/arm/mach-pxa/irq.c
@@ -185,7 +185,7 @@ static int pxa_irq_suspend(void)
 {
int i;
 
-   for (i = 0; i < pxa_internal_irq_nr / 32; i++) {
+   for (i = 0; i < DIV_ROUND_UP(pxa_internal_irq_nr, 32); i++) {
void __iomem *base = irq_base(i);
 
saved_icmr[i] = __raw_readl(base + ICMR);
@@ -204,7 +204,7 @@ static void pxa_irq_resume(void)
 {
int i;
 
-   for (i = 0; i < pxa_internal_irq_nr / 32; i++) {
+   for (i = 0; i < DIV_ROUND_UP(pxa_internal_irq_nr, 32); i++) {
void __iomem *base = irq_base(i);
 
__raw_writel(saved_icmr[i], base + ICMR);

[PATCH 4.4 46/79] ARM: pxa: irq: fix handling of ICMR registers in suspend/resume

[Greg Kroah-Hartman]

4.4-stable review patch.  If anyone has any objections, please let me know.

--

From: Daniel Mack 

[ Upstream commit 0c1049dcb4ceec640d8bd797335bcbebdcab44d2 ]

PXA3xx platforms have 56 interrupts that are stored in two ICMR
registers. The code in pxa_irq_suspend() and pxa_irq_resume() however
does a simple division by 32 which only leads to one register being
saved at suspend and restored at resume time. The NAND interrupt
setting, for instance, is lost.

Fix this by using DIV_ROUND_UP() instead.

Signed-off-by: Daniel Mack 
Signed-off-by: Robert Jarzmik 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman 
---
 arch/arm/mach-pxa/irq.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/arch/arm/mach-pxa/irq.c
+++ b/arch/arm/mach-pxa/irq.c
@@ -185,7 +185,7 @@ static int pxa_irq_suspend(void)
 {
int i;
 
-   for (i = 0; i < pxa_internal_irq_nr / 32; i++) {
+   for (i = 0; i < DIV_ROUND_UP(pxa_internal_irq_nr, 32); i++) {
void __iomem *base = irq_base(i);
 
saved_icmr[i] = __raw_readl(base + ICMR);
@@ -204,7 +204,7 @@ static void pxa_irq_resume(void)
 {
int i;
 
-   for (i = 0; i < pxa_internal_irq_nr / 32; i++) {
+   for (i = 0; i < DIV_ROUND_UP(pxa_internal_irq_nr, 32); i++) {
void __iomem *base = irq_base(i);
 
__raw_writel(saved_icmr[i], base + ICMR);

Re: Regression: 18996f2db918 ("ACPICA: Events: Stop unconditionally clearing ACPI IRQs during suspend/resume" breaks resume from suspend

[Michal Suchánek]

On Tue, 21 Aug 2018 08:50:36 +0200
"Rafael J. Wysocki"  wrote:

> On Mon, Aug 20, 2018 at 6:29 PM Michal Suchánek 
> wrote:
> >
> > Hello,
> >
> > after commit 18996f2db918 ("ACPICA: Events: Stop unconditionally
> > clearing ACPI IRQs during suspend/resume") I am no longer able to
> > resume from suspend.
> >
> > Reported in bugzilla
> > https://bugzilla.kernel.org/show_bug.cgi?id=200841
> >
> > Reverting this on top of 4.18 fixes the issue. acpidump output is
> > attaches in the bugzilla.
> >
> > Is there anything else that can be done to diagnose the issue?  
> 
> Can you please test the patch at
> https://patchwork.kernel.org/patch/10563631/ to start with?
> 

Yes, that works for me.

Thanks

Michal

Re: Regression: 18996f2db918 ("ACPICA: Events: Stop unconditionally clearing ACPI IRQs during suspend/resume" breaks resume from suspend

[Michal Suchánek]

On Tue, 21 Aug 2018 08:50:36 +0200
"Rafael J. Wysocki"  wrote:

> On Mon, Aug 20, 2018 at 6:29 PM Michal Suchánek 
> wrote:
> >
> > Hello,
> >
> > after commit 18996f2db918 ("ACPICA: Events: Stop unconditionally
> > clearing ACPI IRQs during suspend/resume") I am no longer able to
> > resume from suspend.
> >
> > Reported in bugzilla
> > https://bugzilla.kernel.org/show_bug.cgi?id=200841
> >
> > Reverting this on top of 4.18 fixes the issue. acpidump output is
> > attaches in the bugzilla.
> >
> > Is there anything else that can be done to diagnose the issue?  
> 
> Can you please test the patch at
> https://patchwork.kernel.org/patch/10563631/ to start with?
> 

Yes, that works for me.

Thanks

Michal

Re: Regression: 18996f2db918 ("ACPICA: Events: Stop unconditionally clearing ACPI IRQs during suspend/resume" breaks resume from suspend

[Rafael J. Wysocki]

On Mon, Aug 20, 2018 at 6:29 PM Michal Suchánek  wrote:
>
> Hello,
>
> after commit 18996f2db918 ("ACPICA: Events: Stop unconditionally
> clearing ACPI IRQs during suspend/resume") I am no longer able to
> resume from suspend.
>
> Reported in bugzilla https://bugzilla.kernel.org/show_bug.cgi?id=200841
>
> Reverting this on top of 4.18 fixes the issue. acpidump output is
> attaches in the bugzilla.
>
> Is there anything else that can be done to diagnose the issue?

Can you please test the patch at
https://patchwork.kernel.org/patch/10563631/ to start with?

It's already queued up for inclusion.

Thanks,
Rafael

Re: Regression: 18996f2db918 ("ACPICA: Events: Stop unconditionally clearing ACPI IRQs during suspend/resume" breaks resume from suspend

[Rafael J. Wysocki]

On Mon, Aug 20, 2018 at 6:29 PM Michal Suchánek  wrote:
>
> Hello,
>
> after commit 18996f2db918 ("ACPICA: Events: Stop unconditionally
> clearing ACPI IRQs during suspend/resume") I am no longer able to
> resume from suspend.
>
> Reported in bugzilla https://bugzilla.kernel.org/show_bug.cgi?id=200841
>
> Reverting this on top of 4.18 fixes the issue. acpidump output is
> attaches in the bugzilla.
>
> Is there anything else that can be done to diagnose the issue?

Can you please test the patch at
https://patchwork.kernel.org/patch/10563631/ to start with?

It's already queued up for inclusion.

Thanks,
Rafael

Regression: 18996f2db918 ("ACPICA: Events: Stop unconditionally clearing ACPI IRQs during suspend/resume" breaks resume from suspend

[Michal Suchánek]

Hello,

after commit 18996f2db918 ("ACPICA: Events: Stop unconditionally
clearing ACPI IRQs during suspend/resume") I am no longer able to
resume from suspend.

Reported in bugzilla https://bugzilla.kernel.org/show_bug.cgi?id=200841

Reverting this on top of 4.18 fixes the issue. acpidump output is
attaches in the bugzilla.

Is there anything else that can be done to diagnose the issue?

Thanks

Michal

Regression: 18996f2db918 ("ACPICA: Events: Stop unconditionally clearing ACPI IRQs during suspend/resume" breaks resume from suspend

[Michal Suchánek]

Hello,

after commit 18996f2db918 ("ACPICA: Events: Stop unconditionally
clearing ACPI IRQs during suspend/resume") I am no longer able to
resume from suspend.

Reported in bugzilla https://bugzilla.kernel.org/show_bug.cgi?id=200841

Reverting this on top of 4.18 fixes the issue. acpidump output is
attaches in the bugzilla.

Is there anything else that can be done to diagnose the issue?

Thanks

Michal

Re: [RFC PATCH] ASoC: core: Optimise suspend/resume of DAPM widgets

[Jon Hunter]

On 16/08/18 11:51, Mark Brown wrote:
> On Wed, Aug 15, 2018 at 07:48:47PM +0100, Jon Hunter wrote:
> 
>> So then I made the following change to avoid scheduling the function
>> calls unnecessarily (which I think should be fine) ...
> 
>> Please note that I am just using ktime_get() to log the time on entry
>> and exit from dapm_power_widgets() and so yes the time may not be
>> purely the time take to execute this function if we are preempted,
>> but I am measuring in the same way in both cases and so it does seem
>> to show some improvement.
> 
> That seems like a simple and useful optimization, will you send a patch?

Yes I will send a patch for this.

Cheers
Jon

-- 
nvpublic

Re: [RFC PATCH] ASoC: core: Optimise suspend/resume of DAPM widgets

[Jon Hunter]

On 16/08/18 11:51, Mark Brown wrote:
> On Wed, Aug 15, 2018 at 07:48:47PM +0100, Jon Hunter wrote:
> 
>> So then I made the following change to avoid scheduling the function
>> calls unnecessarily (which I think should be fine) ...
> 
>> Please note that I am just using ktime_get() to log the time on entry
>> and exit from dapm_power_widgets() and so yes the time may not be
>> purely the time take to execute this function if we are preempted,
>> but I am measuring in the same way in both cases and so it does seem
>> to show some improvement.
> 
> That seems like a simple and useful optimization, will you send a patch?

Yes I will send a patch for this.

Cheers
Jon

-- 
nvpublic

Re: [RFC PATCH] ASoC: core: Optimise suspend/resume of DAPM widgets

[Mark Brown]

On Wed, Aug 15, 2018 at 07:48:47PM +0100, Jon Hunter wrote:

> So then I made the following change to avoid scheduling the function
> calls unnecessarily (which I think should be fine) ...

> Please note that I am just using ktime_get() to log the time on entry
> and exit from dapm_power_widgets() and so yes the time may not be
> purely the time take to execute this function if we are preempted,
> but I am measuring in the same way in both cases and so it does seem
> to show some improvement.

That seems like a simple and useful optimization, will you send a patch?


signature.asc
Description: PGP signature

Re: [RFC PATCH] ASoC: core: Optimise suspend/resume of DAPM widgets

[Mark Brown]

On Wed, Aug 15, 2018 at 07:48:47PM +0100, Jon Hunter wrote:

> So then I made the following change to avoid scheduling the function
> calls unnecessarily (which I think should be fine) ...

> Please note that I am just using ktime_get() to log the time on entry
> and exit from dapm_power_widgets() and so yes the time may not be
> purely the time take to execute this function if we are preempted,
> but I am measuring in the same way in both cases and so it does seem
> to show some improvement.

That seems like a simple and useful optimization, will you send a patch?


signature.asc
Description: PGP signature