[PATCH/RFC v4 0/4] treewide: add regulator condition on _mmc_suspend()

[PATCH/RFC v4 0/4] treewide: add regulator condition on _mmc_suspend()

[Yoshihiro Shimoda]

The regulator-fixed driver is possible to be off by firmware
like PSCI while the system is suspended. If a consumer could get
such a condition from regulator_is_enabled(), it's useful by
consumers.

The patch series alllows MMC subsystem to get the disabled condition
and then eMMC device condition of my environment (r8a77951-salvator-xs)
will be better than before.
 before:
  - enter sleep mode and then turn the vmmc and vqmmc off without
    any Power Off Notification.
 after:
  - call mmc_poweroff_nofity() and then turn the vmmc and vqmmc off.

We can apply each patch to each subsystem without any dependency.

Note that v5.8-rc2 with r8a77951-salvator-xs seems to cause panic from
PCI driver when the system is suspended. So, I disabled the PCI
devices when I tested this patch series.

Changes from v3:
 - Modify regulator subsytem and regulator/fixed driver.
 - Use regulator_is_enabled() instead of firmware API.
 - Update R-Car Gen3 related dts files for the reference.
   But, I have only tested on r8a779{5,61}-salvaltor-xs.dts.
 https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=306281

Changes from v2:
 - Fix typo of function name in patch2.
 - Remove RFC.
 https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=305523

Changes from v1:
 - Use pm_suspend_via_firmware() API instead of pm_suspend_target_state.
 - Modify the psci driver to call pm_set_suspend_via_firmware.
 https://patchwork.kernel.org/patch/11557505/

Yoshihiro Shimoda (4):
  regulator: core: add prepare and resume_early
  regulator: fixed: add regulator_ops members for suspend/resume
  mmc: core: Call mmc_poweroff_nofity() if regulators are disabled
  arm64: dts: renesas: add regulator-off-in-suspend property for eMMC

 arch/arm64/boot/dts/renesas/r8a77970-v3msk.dts   | 10 ++++--
 arch/arm64/boot/dts/renesas/r8a77980-condor.dts  | 10 ++++--
 arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts   | 10 ++++--
 arch/arm64/boot/dts/renesas/r8a77995-draak.dts   |  9 ++++-
 arch/arm64/boot/dts/renesas/salvator-common.dtsi | 10 ++++--
 arch/arm64/boot/dts/renesas/ulcb.dtsi            | 10 ++++--
 drivers/mmc/core/mmc.c                           | 16 ++++++++-
 drivers/regulator/core.c                         | 42 ++++++++++++++++++++++++
 drivers/regulator/fixed.c                        | 29 ++++++++++++++++
 include/linux/regulator/driver.h                 |  6 ++++
 10 files changed, 140 insertions(+), 12 deletions(-)

-- 
2.7.4

[PATCH/RFC v4 1/4] regulator: core: add prepare and resume_early

[Yoshihiro Shimoda]

The regulator-fixed driver is possible to be off by firmware
like PSCI while the system is suspended. If a consumer could get
such a condition from regulator_is_enabled(), it's useful by
consumers.

The regulator subsystem already has regulator-state-(standby|mem|disk)
sub-nodes and regulator-off-in-suspend property. However,
suitable regulator_ops APIs didn't exist.

So, add new regulator_ops APIs and prepare()/resume_early() in
the regulator_pm_ops to set/clear the condition by new APIs before
suspend() functions of consumers are called.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/regulator/core.c         | 42 ++++++++++++++++++++++++++++++++++++++++
 include/linux/regulator/driver.h |  6 ++++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 03154f5..93eb2a3 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -5286,6 +5286,46 @@ void regulator_unregister(struct regulator_dev *rdev)
 EXPORT_SYMBOL_GPL(regulator_unregister);
 
 #ifdef CONFIG_SUSPEND
+static int regulator_prepare(struct device *dev)
+{
+	struct regulator_dev *rdev = dev_to_rdev(dev);
+	suspend_state_t state = pm_suspend_target_state;
+	struct regulator_state *rstate;
+	int ret = 0;
+
+	rstate = regulator_get_suspend_state(rdev, state);
+	if (rstate == NULL)
+		return 0;
+
+	regulator_lock(rdev);
+	if (rstate->enabled == DISABLE_IN_SUSPEND &&
+	    rdev->desc->ops->set_prepare_disable)
+		ret = rdev->desc->ops->set_prepare_disable(rdev);
+	regulator_unlock(rdev);
+
+	return ret;
+}
+
+static int regulator_resume_early(struct device *dev)
+{
+	struct regulator_dev *rdev = dev_to_rdev(dev);
+	suspend_state_t state = pm_suspend_target_state;
+	struct regulator_state *rstate;
+	int ret = 0;
+
+	rstate = regulator_get_suspend_state(rdev, state);
+	if (rstate == NULL)
+		return 0;
+
+	regulator_lock(rdev);
+	if (rstate->enabled == DISABLE_IN_SUSPEND &&
+	    rdev->desc->ops->clear_resume_early_disable)
+		ret = rdev->desc->ops->clear_resume_early_disable(rdev);
+	regulator_unlock(rdev);
+
+	return ret;
+}
+
 /**
  * regulator_suspend - prepare regulators for system wide suspend
  * @dev: ``&struct device`` pointer that is passed to _regulator_suspend()
@@ -5336,6 +5376,8 @@ static int regulator_resume(struct device *dev)
 
 #ifdef CONFIG_PM
 static const struct dev_pm_ops __maybe_unused regulator_pm_ops = {
+	.prepare	= regulator_prepare,
+	.resume_early	= regulator_resume_early,
 	.suspend	= regulator_suspend,
 	.resume		= regulator_resume,
 };
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 7eb9fea..299a504 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -115,6 +115,10 @@ enum regulator_status {
  *                      suspended.
  * @set_suspend_disable: Mark the regulator as disabled when the system is
  *                       suspended.
+ * @set_prepare_disable: Mark the regulator as disabled when the system is
+ *                       suspending.
+ * @clear_resume_early_disable: Unmark the regulator as disabled when
+ *                              the system is resuming.
  * @set_suspend_mode: Set the operating mode for the regulator when the
  *                    system is suspended.
  *
@@ -195,6 +199,8 @@ struct regulator_ops {
 	/* enable/disable regulator in suspend state */
 	int (*set_suspend_enable) (struct regulator_dev *);
 	int (*set_suspend_disable) (struct regulator_dev *);
+	int (*set_prepare_disable) (struct regulator_dev *);
+	int (*clear_resume_early_disable) (struct regulator_dev *);
 
 	/* set regulator suspend operating mode (defined in consumer.h) */
 	int (*set_suspend_mode) (struct regulator_dev *, unsigned int mode);
-- 
2.7.4

[PATCH/RFC v4 2/4] regulator: fixed: add regulator_ops members for suspend/resume

[Yoshihiro Shimoda]

The regulator-fixed driver is possible to be off by firmware
like PSCI while the system is suspended. If a consumer could get
such a condition from regulator_is_enabled(), it's useful by
consumers.

So, add some regulator_ops members for it. And then, if
regulator-fixed nodes have suitable sub-nodes and properties [1],
regulator_is_enabled() will return false while suspend() of
a consumer.

[1]
 - The node has regulator-state-(standby|mem|disk) sub-nodes.
 - The node doesn't have regulator-always-on.
 - The sub-node has regulator-off-in-suspend property.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/regulator/fixed.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index d54830e..0bd4a1a 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -35,6 +35,7 @@ struct fixed_voltage_data {
 
 	struct clk *enable_clock;
 	unsigned int clk_enable_counter;
+	bool disabled_in_suspend;
 };
 
 struct fixed_dev_type {
@@ -49,6 +50,31 @@ static const struct fixed_dev_type fixed_clkenable_data = {
 	.has_enable_clock = true,
 };
 
+static int reg_is_enabled(struct regulator_dev *rdev)
+{
+	struct fixed_voltage_data *priv = rdev_get_drvdata(rdev);
+
+	return !priv->disabled_in_suspend;
+}
+
+static int reg_prepare_disable(struct regulator_dev *rdev)
+{
+	struct fixed_voltage_data *priv = rdev_get_drvdata(rdev);
+
+	priv->disabled_in_suspend = true;
+
+	return 0;
+}
+
+static int reg_resume_early_disable(struct regulator_dev *rdev)
+{
+	struct fixed_voltage_data *priv = rdev_get_drvdata(rdev);
+
+	priv->disabled_in_suspend = false;
+
+	return 0;
+}
+
 static int reg_clock_enable(struct regulator_dev *rdev)
 {
 	struct fixed_voltage_data *priv = rdev_get_drvdata(rdev);
@@ -132,6 +158,9 @@ of_get_fixed_voltage_config(struct device *dev,
 }
 
 static struct regulator_ops fixed_voltage_ops = {
+	.is_enabled = reg_is_enabled,
+	.set_prepare_disable = reg_prepare_disable,
+	.clear_resume_early_disable = reg_resume_early_disable,
 };
 
 static struct regulator_ops fixed_voltage_clkenabled_ops = {
-- 
2.7.4

[PATCH/RFC v4 3/4] mmc: core: Call mmc_poweroff_nofity() if regulators are disabled

[Yoshihiro Shimoda]

If regulator_is_enabled() of both vmmc and vqmmc returns false,
_mmc_suspend() should call mmc_poweroff_nofity() instead of
mmc_sleep().

Note that this is possible to happen when the regulator-fixed driver
turns the vmmc and vqmmc off by firmware like PSCI while the system
is suspended.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/mmc/core/mmc.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 4203303..75df5f8 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -12,6 +12,7 @@
 #include <linux/slab.h>
 #include <linux/stat.h>
 #include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
 
 #include <linux/mmc/host.h>
 #include <linux/mmc/card.h>
@@ -2022,6 +2023,18 @@ static void mmc_detect(struct mmc_host *host)
 	}
 }
 
+static bool mmc_regulators_are_disabled(struct mmc_host *host)
+{
+	if (IS_ERR(host->supply.vmmc) ||
+	    regulator_is_enabled(host->supply.vmmc))
+		return false;
+	if (IS_ERR(host->supply.vqmmc) ||
+	    regulator_is_enabled(host->supply.vqmmc))
+		return false;
+
+	return true;
+}
+
 static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
 {
 	int err = 0;
@@ -2038,7 +2051,8 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
 		goto out;
 
 	if (mmc_can_poweroff_notify(host->card) &&
-		((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
+	    ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend ||
+	     mmc_regulators_are_disabled(host)))
 		err = mmc_poweroff_notify(host->card, notify_type);
 	else if (mmc_can_sleep(host->card))
 		err = mmc_sleep(host);
-- 
2.7.4

[PATCH/RFC v4 4/4] arm64: dts: renesas: add regulator-off-in-suspend property for eMMC

[Yoshihiro Shimoda]

Add regulator-off-in-suspend property into eMMC related regulator-fixed
nodes because PSCI on the boards will turn the regulators off in suspend.

By this property, the regulator's status will be disabled in suspend.
MMC subsystem can get the condition and then eMMC condition will
be better than before.
 before:
  - enter sleep mode and then turn the vmmc and vqmmc off.
 after:
  - call mmc_poweroff_nofity() and then turn the vmmc and vqmmc off.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a77970-v3msk.dts   | 10 ++++++++--
 arch/arm64/boot/dts/renesas/r8a77980-condor.dts  | 10 ++++++++--
 arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts   | 10 ++++++++--
 arch/arm64/boot/dts/renesas/r8a77995-draak.dts   |  9 ++++++++-
 arch/arm64/boot/dts/renesas/salvator-common.dtsi | 10 ++++++++--
 arch/arm64/boot/dts/renesas/ulcb.dtsi            | 10 ++++++++--
 6 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a77970-v3msk.dts b/arch/arm64/boot/dts/renesas/r8a77970-v3msk.dts
index 01c4ba0..9fe634a 100644
--- a/arch/arm64/boot/dts/renesas/r8a77970-v3msk.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77970-v3msk.dts
@@ -74,7 +74,10 @@
 		regulator-min-microvolt = <1800000>;
 		regulator-max-microvolt = <1800000>;
 		regulator-boot-on;
-		regulator-always-on;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
 	};
 
 	vcc_d3_3v: regulator-1 {
@@ -83,7 +86,10 @@
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
 		regulator-boot-on;
-		regulator-always-on;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
 	};
 
 	vcc_vddq_vin0: regulator-2 {
diff --git a/arch/arm64/boot/dts/renesas/r8a77980-condor.dts b/arch/arm64/boot/dts/renesas/r8a77980-condor.dts
index ef8350a..5898c7f 100644
--- a/arch/arm64/boot/dts/renesas/r8a77980-condor.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77980-condor.dts
@@ -37,7 +37,10 @@
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
 		regulator-boot-on;
-		regulator-always-on;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
 	};
 
 	hdmi-out {
@@ -87,7 +90,10 @@
 		regulator-min-microvolt = <1800000>;
 		regulator-max-microvolt = <1800000>;
 		regulator-boot-on;
-		regulator-always-on;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
 	};
 
 	x1_clk: x1-clock {
diff --git a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
index dc24cec4..80736f8 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
@@ -113,7 +113,10 @@
 		regulator-min-microvolt = <1800000>;
 		regulator-max-microvolt = <1800000>;
 		regulator-boot-on;
-		regulator-always-on;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
 	};
 
 	reg_3p3v: regulator1 {
@@ -122,7 +125,10 @@
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
 		regulator-boot-on;
-		regulator-always-on;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
 	};
 
 	reg_12p0v: regulator2 {
diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
index 79c73a9..9ac5361 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
@@ -103,7 +103,10 @@
 		regulator-min-microvolt = <1800000>;
 		regulator-max-microvolt = <1800000>;
 		regulator-boot-on;
-		regulator-always-on;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
 	};
 
 	reg_3p3v: regulator-3p3v {
@@ -113,6 +116,10 @@
 		regulator-max-microvolt = <3300000>;
 		regulator-boot-on;
 		regulator-always-on;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
 	};
 
 	reg_12p0v: regulator-12p0v {
diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
index 98bbcaf..fa8c45f 100644
--- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
+++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
@@ -172,7 +172,10 @@
 		regulator-min-microvolt = <1800000>;
 		regulator-max-microvolt = <1800000>;
 		regulator-boot-on;
-		regulator-always-on;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
 	};
 
 	reg_3p3v: regulator1 {
@@ -181,7 +184,10 @@
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
 		regulator-boot-on;
-		regulator-always-on;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
 	};
 
 	reg_12v: regulator2 {
diff --git a/arch/arm64/boot/dts/renesas/ulcb.dtsi b/arch/arm64/boot/dts/renesas/ulcb.dtsi
index ff88af8..7c5bccc 100644
--- a/arch/arm64/boot/dts/renesas/ulcb.dtsi
+++ b/arch/arm64/boot/dts/renesas/ulcb.dtsi
@@ -79,7 +79,10 @@
 		regulator-min-microvolt = <1800000>;
 		regulator-max-microvolt = <1800000>;
 		regulator-boot-on;
-		regulator-always-on;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
 	};
 
 	reg_3p3v: regulator1 {
@@ -88,7 +91,10 @@
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
 		regulator-boot-on;
-		regulator-always-on;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
 	};
 
 	sound_card: sound {
-- 
2.7.4

Re: [PATCH/RFC v4 0/4] treewide: add regulator condition on _mmc_suspend()

[Geert Uytterhoeven]

Hi Shimoda-san,

[reducing audience]

On Fri, Jun 26, 2020 at 11:32 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> Note that v5.8-rc2 with r8a77951-salvator-xs seems to cause panic from
> PCI driver when the system is suspended. So, I disabled the PCI
> devices when I tested this patch series.

Does this happen with current renesas-devel and renesas_defconfig?
(it doesn't for me)
Do you have any PCIe devices attached? (I haven't)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH/RFC v4 1/4] regulator: core: add prepare and resume_early

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3792 bytes --]

Hi Yoshihiro,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on renesas-devel/next]
[also build test ERROR on regulator/for-next v5.8-rc2 next-20200626]
[cannot apply to ulf.hansson-mmc/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Yoshihiro-Shimoda/treewide-add-regulator-condition-on-_mmc_suspend/20200626-173437
base:   https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-devel.git next
config: arc-allyesconfig (attached as .config)
compiler: arc-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/regulator/core.c:3805:5: warning: no previous prototype for 'regulator_suspend_enable' [-Wmissing-prototypes]
    3805 | int regulator_suspend_enable(struct regulator_dev *rdev,
         |     ^~~~~~~~~~~~~~~~~~~~~~~~
   drivers/regulator/core.c:3812:5: warning: no previous prototype for 'regulator_suspend_disable' [-Wmissing-prototypes]
    3812 | int regulator_suspend_disable(struct regulator_dev *rdev,
         |     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/regulator/core.c:3851:5: warning: no previous prototype for 'regulator_set_suspend_voltage' [-Wmissing-prototypes]
    3851 | int regulator_set_suspend_voltage(struct regulator *regulator, int min_uV,
         |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/regulator/core.c:5379:13: error: 'regulator_prepare' undeclared here (not in a function); did you mean 'regulator_resume'?
    5379 |  .prepare = regulator_prepare,
         |             ^~~~~~~~~~~~~~~~~
         |             regulator_resume
>> drivers/regulator/core.c:5380:18: error: 'regulator_resume_early' undeclared here (not in a function); did you mean 'regulator_resume'?
    5380 |  .resume_early = regulator_resume_early,
         |                  ^~~~~~~~~~~~~~~~~~~~~~
         |                  regulator_resume
   In file included from include/linux/perf_event.h:25,
                    from include/linux/trace_events.h:10,
                    from include/trace/trace_events.h:21,
                    from include/trace/define_trace.h:102,
                    from include/trace/events/regulator.h:174,
                    from drivers/regulator/core.c:31:
   arch/arc/include/asm/perf_event.h:126:23: warning: 'arc_pmu_cache_map' defined but not used [-Wunused-const-variable=]
     126 | static const unsigned arc_pmu_cache_map[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
         |                       ^~~~~~~~~~~~~~~~~
   arch/arc/include/asm/perf_event.h:91:27: warning: 'arc_pmu_ev_hw_map' defined but not used [-Wunused-const-variable=]
      91 | static const char * const arc_pmu_ev_hw_map[] = {
         |                           ^~~~~~~~~~~~~~~~~

vim +5379 drivers/regulator/core.c

  5376	
  5377	#ifdef CONFIG_PM
  5378	static const struct dev_pm_ops __maybe_unused regulator_pm_ops = {
> 5379		.prepare	= regulator_prepare,
> 5380		.resume_early	= regulator_resume_early,
  5381		.suspend	= regulator_suspend,
  5382		.resume		= regulator_resume,
  5383	};
  5384	#endif
  5385	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 64635 bytes --]

Re: [PATCH/RFC v4 1/4] regulator: core: add prepare and resume_early

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1027 bytes --]

On Fri, Jun 26, 2020 at 06:32:19PM +0900, Yoshihiro Shimoda wrote:

> The regulator-fixed driver is possible to be off by firmware
> like PSCI while the system is suspended. If a consumer could get
> such a condition from regulator_is_enabled(), it's useful by
> consumers.

> The regulator subsystem already has regulator-state-(standby|mem|disk)
> sub-nodes and regulator-off-in-suspend property. However,
> suitable regulator_ops APIs didn't exist.

> So, add new regulator_ops APIs and prepare()/resume_early() in
> the regulator_pm_ops to set/clear the condition by new APIs before
> suspend() functions of consumers are called.

I can't follow this explanation at all, I really can't understand what
these functions are supposed to do or how they are supposed to be used.
Nothing in the rest of this series is at all enlightening either.  It
seems there is some need for a consumer to query things about the
suspend state but there is no obvious connection from that to adding
these new operations for regulator drivers.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH/RFC v4 2/4] regulator: fixed: add regulator_ops members for suspend/resume

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 977 bytes --]

On Fri, Jun 26, 2020 at 06:32:20PM +0900, Yoshihiro Shimoda wrote:

> +static int reg_is_enabled(struct regulator_dev *rdev)
> +{
> +	struct fixed_voltage_data *priv = rdev_get_drvdata(rdev);
> +
> +	return !priv->disabled_in_suspend;
> +}

This is broken, the state of the regualtor during system runtime need
have no connection with the state of the regulator during system
suspend.

> +static int reg_prepare_disable(struct regulator_dev *rdev)
> +{
> +	struct fixed_voltage_data *priv = rdev_get_drvdata(rdev);
> +
> +	priv->disabled_in_suspend = true;
> +
> +	return 0;
> +}

According to the changelog this is all about reflecting changes in the
system state done by firmware but there's no interaction with firmware
here which means this will be at best fragile.  If we need to reflect
changes in firmware configuration I'd expect there to be some
interaction with firmware about how it is configured, or at least that
the configuration would come from the same source.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH/RFC v4 3/4] mmc: core: Call mmc_poweroff_nofity() if regulators are disabled

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 971 bytes --]

On Fri, Jun 26, 2020 at 06:32:21PM +0900, Yoshihiro Shimoda wrote:

> If regulator_is_enabled() of both vmmc and vqmmc returns false,
> _mmc_suspend() should call mmc_poweroff_nofity() instead of
> mmc_sleep().

This is possibly something it makes sense to do, if the power did
suddenly vanish on the device then using a separate cleanup path for
that seems sensible (it's probably something worth complaining loudly
about).  Registering for notification on power loss might also be
sensible.

> Note that this is possible to happen when the regulator-fixed driver
> turns the vmmc and vqmmc off by firmware like PSCI while the system
> is suspended.

This is not a good interface, if there's a need to query the state over
suspend then we should query the state over suspend rather than trying
to somehow shoehorn it via the runtime enable state which is going to
break any other users and relies on the regulator driver doing dodgy
stuff representing the enable state.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH/RFC v4 3/4] mmc: core: Call mmc_poweroff_nofity() if regulators are disabled

[Sergei Shtylyov]

Hello!

On 06/26/2020 12:32 PM, Yoshihiro Shimoda wrote:

> If regulator_is_enabled() of both vmmc and vqmmc returns false,
> _mmc_suspend() should call mmc_poweroff_nofity() instead of
                                          ^^^^^^
  That hard word again. :-)        

> mmc_sleep().
> 
> Note that this is possible to happen when the regulator-fixed driver
> turns the vmmc and vqmmc off by firmware like PSCI while the system
> is suspended.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
[...]

MBR, Sergei

RE: [PATCH/RFC v4 1/4] regulator: core: add prepare and resume_early

[Yoshihiro Shimoda]

Hi Mark,

> From: Mark Brown, Sent: Friday, June 26, 2020 11:30 PM
> 
> On Fri, Jun 26, 2020 at 06:32:19PM +0900, Yoshihiro Shimoda wrote:
> 
> > The regulator-fixed driver is possible to be off by firmware
> > like PSCI while the system is suspended. If a consumer could get
> > such a condition from regulator_is_enabled(), it's useful by
> > consumers.
> 
> > The regulator subsystem already has regulator-state-(standby|mem|disk)
> > sub-nodes and regulator-off-in-suspend property. However,
> > suitable regulator_ops APIs didn't exist.
> 
> > So, add new regulator_ops APIs and prepare()/resume_early() in
> > the regulator_pm_ops to set/clear the condition by new APIs before
> > suspend() functions of consumers are called.
> 
> I can't follow this explanation at all, I really can't understand what
> these functions are supposed to do or how they are supposed to be used.
> Nothing in the rest of this series is at all enlightening either.  It
> seems there is some need for a consumer to query things about the
> suspend state but there is no obvious connection from that to adding
> these new operations for regulator drivers.

I'm very sorry for lack description... Perhaps I should have described
one of use cases like below.

regulator_prepare()
--> call regulator_ops.set_prepare_disable() if DISABLE_IN_SUSPEND
 --> A regulator can be disabled by the operation.
 --> We can guarantee an order which is called before a consumer if
     it uses dev_pm_ops.suspend().
..
A consumer driver's suspend().
--> call regulator_is_enabled()
 --> If the regulator was called set_prepare_disable(), this can returns false.

Best regards,
Yoshihiro Shimoda

RE: [PATCH/RFC v4 2/4] regulator: fixed: add regulator_ops members for suspend/resume

[Yoshihiro Shimoda]

Hi Mark,

> From: Mark Brown, Sent: Friday, June 26, 2020 11:39 PM
> 
> On Fri, Jun 26, 2020 at 06:32:20PM +0900, Yoshihiro Shimoda wrote:
> 
> > +static int reg_is_enabled(struct regulator_dev *rdev)
> > +{
> > +	struct fixed_voltage_data *priv = rdev_get_drvdata(rdev);
> > +
> > +	return !priv->disabled_in_suspend;
> > +}
> 
> This is broken, the state of the regualtor during system runtime need
> have no connection with the state of the regulator during system
> suspend.
> 
> > +static int reg_prepare_disable(struct regulator_dev *rdev)
> > +{
> > +	struct fixed_voltage_data *priv = rdev_get_drvdata(rdev);
> > +
> > +	priv->disabled_in_suspend = true;
> > +
> > +	return 0;
> > +}
> 
> According to the changelog this is all about reflecting changes in the
> system state done by firmware but there's no interaction with firmware
> here which means this will be at best fragile.  If we need to reflect
> changes in firmware configuration I'd expect there to be some
> interaction with firmware about how it is configured, or at least that
> the configuration would come from the same source.

I should have described background of previous patch series though,
according to previous discussion [1] the firmware side (like PSCI) is
also fragile unfortunately... So, I thought using regulator-off-in-suspend
in a regulator was better.

On other hand, Ulf is talking about either adding a property (perhaps like
regulator-off-in-suspend) into a regulator or just adding a new property
into MMC [2]. What do you think about Ulf' comment? I'm thinking
adding a new property "full-pwr-cycle-in-suspend" is the best solution.
This is because using a regulator property and reflecting a state of regulator without
firmware is fragile, as you said.

[1]
https://lore.kernel.org/linux-renesas-soc/CAMuHMdXjU7N4oG89YsozGijMpjgKGN6ezw2qm6FeGX=JyRhsvg@mail.gmail.com/

[2]
https://lore.kernel.org/linux-renesas-soc/CAPDyKFpiBU1D+a7zb+Ggm0_HZ+YR4=LXJZ5MPytXtT=uBEdjPA@mail.gmail.com/

Best regards,
Yoshihiro Shimoda

RE: [PATCH/RFC v4 3/4] mmc: core: Call mmc_poweroff_nofity() if regulators are disabled

[Yoshihiro Shimoda]

Hi Mark,

> From: Mark Brown, Sent: Saturday, June 27, 2020 12:14 AM
> 
> On Fri, Jun 26, 2020 at 06:32:21PM +0900, Yoshihiro Shimoda wrote:
> > Note that this is possible to happen when the regulator-fixed driver
> > turns the vmmc and vqmmc off by firmware like PSCI while the system
> > is suspended.
> 
> This is not a good interface, if there's a need to query the state over
> suspend then we should query the state over suspend rather than trying
> to somehow shoehorn it via the runtime enable state which is going to
> break any other users and relies on the regulator driver doing dodgy
> stuff representing the enable state.

I understood it.
So, as I mentioned other email thread, I'm thinking adding a new property
into MMC is better.

Best regards,
Yoshihiro Shimoda

RE: [PATCH/RFC v4 3/4] mmc: core: Call mmc_poweroff_nofity() if regulators are disabled

[Yoshihiro Shimoda]

Hello!

> From: Sergei Shtylyov, Sent: Saturday, June 27, 2020 12:54 AM
> 
> Hello!
> 
> On 06/26/2020 12:32 PM, Yoshihiro Shimoda wrote:
> 
> > If regulator_is_enabled() of both vmmc and vqmmc returns false,
> > _mmc_suspend() should call mmc_poweroff_nofity() instead of
>                                           ^^^^^^
>   That hard word again. :-)

Oops! Thank you for pointed it out!
# Also, the subject was incorrect...

Best regards,
Yoshihiro Shimoda

RE: [PATCH/RFC v4 0/4] treewide: add regulator condition on _mmc_suspend()

[Yoshihiro Shimoda]

Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Friday, June 26, 2020 7:13 PM
> 
> Hi Shimoda-san,
> 
> [reducing audience]
> 
> On Fri, Jun 26, 2020 at 11:32 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > Note that v5.8-rc2 with r8a77951-salvator-xs seems to cause panic from
> > PCI driver when the system is suspended. So, I disabled the PCI
> > devices when I tested this patch series.
> 
> Does this happen with current renesas-devel and renesas_defconfig?
> (it doesn't for me)

Yes. I enabled PM_DEBUG and E1000E though.

> Do you have any PCIe devices attached? (I haven't)

Yes. (Intel Ethernet card is connected to the PCI slot.)

< my environment >
- r8a77961-salvator-xs
- renesas-devel-2020-06-26-v5.8-rc2
 + renesas_defconfig + PM_DEBUG + E1000E
- initramfs

JFYI, I pasted the kernel log like below.

--
[    0.000000] Booting Linux on physical CPU 0x0000000000 [0x411fd073]
[    0.000000] Linux version 5.8.0-rc2-arm64-renesas (shimoda@shimoda-SVE1411AJ) (aarch64-linux-gnu-gcc (Linaro GCC 7.5-2019.12) 7.5.0, GNU ld (Linaro_Binutils-2019.12) 2.28.2.20170706) #57 SMP PREEMPT Mon Jun 29 18:00:16 JST 2020
[    0.000000] Machine model: Renesas Salvator-X 2nd version board based on r8a77961
[    0.000000] printk: debug: ignoring loglevel setting.
[    0.000000] efi: UEFI not found.
[    0.000000] cma: Reserved 128 MiB at 0x00000000b8000000
[    0.000000] Zone ranges:
[    0.000000]   DMA      [mem 0x0000000048000000-0x000000007fffffff]
[    0.000000]   DMA32    [mem 0x0000000080000000-0x00000000ffffffff]
[    0.000000]   Normal   [mem 0x0000000100000000-0x00000006ffffffff]
[    0.000000] Movable zone start for each node
[    0.000000] Early memory node ranges
[    0.000000]   node   0: [mem 0x0000000048000000-0x00000000bfffffff]
[    0.000000]   node   0: [mem 0x0000000480000000-0x00000004ffffffff]
[    0.000000]   node   0: [mem 0x0000000600000000-0x00000006ffffffff]
[    0.000000] Initmem setup node 0 [mem 0x0000000048000000-0x00000006ffffffff]
[    0.000000] On node 0 totalpages: 2064384
[    0.000000]   DMA zone: 3584 pages used for memmap
[    0.000000]   DMA zone: 0 pages reserved
[    0.000000]   DMA zone: 229376 pages, LIFO batch:63
[    0.000000]   DMA32 zone: 4096 pages used for memmap
[    0.000000]   DMA32 zone: 262144 pages, LIFO batch:63
[    0.000000]   Normal zone: 24576 pages used for memmap
[    0.000000]   Normal zone: 1572864 pages, LIFO batch:63
[    0.000000] psci: probing for conduit method from DT.
[    0.000000] psci: PSCIv1.1 detected in firmware.
[    0.000000] psci: Using standard PSCI v0.2 function IDs
[    0.000000] psci: Trusted OS resident on physical CPU 0x0
[    0.000000] psci: SMC Calling Convention v1.1
[    0.000000] percpu: Embedded 30 pages/cpu s91408 r0 d31472 u122880
[    0.000000] pcpu-alloc: s91408 r0 d31472 u122880 alloc=30*4096
[    0.000000] pcpu-alloc: [0] 0 [0] 1 [0] 2 [0] 3 [0] 4 [0] 5 
[    0.000000] Detected PIPT I-cache on CPU0
[    0.000000] CPU features: detected: EL2 vector hardening
[    0.000000] Speculative Store Bypass Disable mitigation not required
[    0.000000] CPU features: detected: ARM errata 1165522, 1319367, or 1530923
[    0.000000] Built 1 zonelists, mobility grouping on.  Total pages: 2032128
[    0.000000] Kernel command line: console=ttySC0,115200 ignore_loglevel rw ip=none rdinit=/sbin/init
[    0.000000] Dentry cache hash table entries: 1048576 (order: 11, 8388608 bytes, linear)
[    0.000000] Inode-cache hash table entries: 524288 (order: 10, 4194304 bytes, linear)
[    0.000000] mem auto-init: stack:off, heap alloc:off, heap free:off
[    0.000000] software IO TLB: mapped [mem 0x7bfff000-0x7ffff000] (64MB)
[    0.000000] Memory: 7855196K/8257536K available (10812K kernel code, 1372K rwdata, 3500K rodata, 18048K init, 10976K bss, 271268K reserved, 131072K cma-reserved)
[    0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=6, Nodes=1
[    0.000000] Running RCU self tests
[    0.000000] rcu: Preemptible hierarchical RCU implementation.
[    0.000000] rcu: ^[[4CRCU event tracing is enabled.
[    0.000000] rcu: ^[[4CRCU lockdep checking is enabled.
[    0.000000] rcu: ^[[4CRCU restricting CPUs from NR_CPUS=8 to nr_cpu_ids=6.
[    0.000000]  Trampoline variant of Tasks RCU enabled.
[    0.000000] rcu: RCU calculated value of scheduler-enlistment delay is 25 jiffies.
[    0.000000] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=6
[    0.000000] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
[    0.000000] random: get_random_bytes called from start_kernel+0x378/0x508 with crng_init=0
[    0.000000] arch_timer: cp15 timer(s) running at 8.32MHz (virt).
[    0.000000] clocksource: arch_sys_counter: mask: 0xffffffffffffff max_cycles: 0x1eb398c07, max_idle_ns: 440795202503 ns
[    0.000004] sched_clock: 56 bits at 8MHz, resolution 120ns, wraps every 2199023255503ns
[    0.000189] Console: colour dummy device 80x25
[    0.000208] Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., Ingo Molnar
[    0.000217] ... MAX_LOCKDEP_SUBCLASSES:  8
[    0.000225] ... MAX_LOCK_DEPTH:          48
[    0.000233] ... MAX_LOCKDEP_KEYS:        8192
[    0.000241] ... CLASSHASH_SIZE:          4096
[    0.000249] ... MAX_LOCKDEP_ENTRIES:     32768
[    0.000257] ... MAX_LOCKDEP_CHAINS:      65536
[    0.000264] ... CHAINHASH_SIZE:          32768
[    0.000272]  memory used by lock dependency info: 6301 kB
[    0.000280]  memory used for stack traces: 4224 kB
[    0.000288]  per task-struct memory footprint: 1920 bytes
[    0.000341] Calibrating delay loop (skipped), value calculated using timer frequency.. 16.64 BogoMIPS (lpj=33280)
[    0.000355] pid_max: default: 32768 minimum: 301
[    0.000602] Mount-cache hash table entries: 16384 (order: 5, 131072 bytes, linear)
[    0.000657] Mountpoint-cache hash table entries: 16384 (order: 5, 131072 bytes, linear)
[    0.004464] rcu: Hierarchical SRCU implementation.
[    0.005395] Detected Renesas R-Car Gen3 r8a77961 ES3.0
[    0.006066] EFI services will not be available.
[    0.006740] smp: Bringing up secondary CPUs ...
[    0.008009] Detected PIPT I-cache on CPU1
[    0.008074] CPU1: Booted secondary processor 0x0000000001 [0x411fd073]
[    0.009527] CPU features: detected: ARM erratum 845719
[    0.009545] Detected VIPT I-cache on CPU2
[    0.009612] CPU2: Booted secondary processor 0x0000000100 [0x410fd034]
[    0.010957] Detected VIPT I-cache on CPU3
[    0.010991] CPU3: Booted secondary processor 0x0000000101 [0x410fd034]
[    0.012380] Detected VIPT I-cache on CPU4
[    0.012415] CPU4: Booted secondary processor 0x0000000102 [0x410fd034]
[    0.013705] Detected VIPT I-cache on CPU5
[    0.013740] CPU5: Booted secondary processor 0x0000000103 [0x410fd034]
[    0.014074] smp: Brought up 1 node, 6 CPUs
[    0.014120] SMP: Total of 6 processors activated.
[    0.014131] CPU features: detected: 32-bit EL0 Support
[    0.014141] CPU features: detected: CRC32 instructions
[    0.014150] CPU features: detected: 32-bit EL1 Support
[    0.015136] CPU: All CPU(s) started at EL1
[    0.015201] alternatives: patching kernel code
[    0.017277] devtmpfs: initialized
[    0.033308] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 7645041785100000 ns
[    0.033354] futex hash table entries: 2048 (order: 6, 262144 bytes, linear)
[    0.035560] pinctrl core: initialized pinctrl subsystem
[    0.037017] thermal_sys: Registered thermal governor 'step_wise'
[    0.038125] DMI not present or invalid.
[    0.038713] NET: Registered protocol family 16
[    0.040921] DMA: preallocated 1024 KiB GFP_KERNEL pool for atomic allocations
[    0.041113] DMA: preallocated 1024 KiB GFP_KERNEL|GFP_DMA pool for atomic allocations
[    0.041670] DMA: preallocated 1024 KiB GFP_KERNEL|GFP_DMA32 pool for atomic allocations
[    0.041822] audit: initializing netlink subsys (disabled)
[    0.042179] audit: type=2000 audit(0.040:1): state=initialized audit_enabled=0 res=1
[    0.043007] cpuidle: using governor menu
[    0.043201] hw-breakpoint: found 6 breakpoint and 4 watchpoint registers.
[    0.043473] ASID allocator initialised with 65536 entries
[    0.076130] sh-pfc e6060000.pin-controller: r8a77961_pfc support registered
[    0.088931] HugeTLB registered 1.00 GiB page size, pre-allocated 0 pages
[    0.088945] HugeTLB registered 32.0 MiB page size, pre-allocated 0 pages
[    0.088955] HugeTLB registered 2.00 MiB page size, pre-allocated 0 pages
[    0.088964] HugeTLB registered 64.0 KiB page size, pre-allocated 0 pages
[    0.092908] cryptd: max_cpu_qlen set to 1000
[    0.101004] iommu: Default domain type: Translated 
[    0.101586] vgaarb: loaded
[    0.102214] SCSI subsystem initialized
[    0.102584] libata version 3.00 loaded.
[    0.102827] usbcore: registered new interface driver usbfs
[    0.102889] usbcore: registered new interface driver hub
[    0.103037] usbcore: registered new device driver usb
[    0.105141] i2c-sh_mobile e60b0000.i2c: I2C adapter 0, bus speed 400000 Hz
[    0.105621] mc: Linux media interface: v0.10
[    0.105675] videodev: Linux video capture interface: v2.00
[    0.105781] pps_core: LinuxPPS API ver. 1 registered
[    0.105790] pps_core: Software ver. 5.3.6 - Copyright 2005-2007 Rodolfo Giometti <giometti@linux.it>
[    0.105815] PTP clock support registered
[    0.106722] Advanced Linux Sound Architecture Driver Initialized.
[    0.108211] clocksource: Switched to clocksource arch_sys_counter
[    0.586632] VFS: Disk quotas dquot_6.6.0
[    0.586729] VFS: Dquot-cache hash table entries: 512 (order 0, 4096 bytes)
[    0.601849] NET: Registered protocol family 2
[    0.602529] tcp_listen_portaddr_hash hash table entries: 4096 (order: 6, 327680 bytes, linear)
[    0.603046] TCP established hash table entries: 65536 (order: 7, 524288 bytes, linear)
[    0.604788] TCP bind hash table entries: 65536 (order: 10, 4718592 bytes, vmalloc)
[    0.612302] TCP: Hash tables configured (established 65536 bind 65536)
[    0.612677] UDP hash table entries: 4096 (order: 7, 655360 bytes, linear)
[    0.613755] UDP-Lite hash table entries: 4096 (order: 7, 655360 bytes, linear)\b
[    0.615045] NET: Registered protocol family 1
[    0.616241] RPC: Registered named UNIX socket transport module.
[    0.616269] RPC: Registered udp transport module.
[    0.616279] RPC: Registered tcp transport module.
[    0.616288] RPC: Registered tcp NFSv4.1 backchannel transport module.
[    0.616305] PCI: CLS 0 bytes, default 64
[    0.697485] hw perfevents: enabled with armv8_cortex_a53 PMU driver, 7 counters available
[    0.698098] hw perfevents: enabled with armv8_cortex_a57 PMU driver, 7 counters available
[    0.698580] kvm [1]: HYP mode not available
[    0.708317] workingset: timestamp_bits=46 max_order=21 bucket_order=0
[    0.722635] squashfs: version 4.0 (2009/01/31) Phillip Lougher
[    0.724131] NFS: Registering the id_resolver key type
[    0.724299] Key type id_resolver registered
[    0.724328] Key type id_legacy registered
[    0.724351] nfs4filelayout_init: NFSv4 File Layout Driver Registering...
[    0.724378] nfs4flexfilelayout_init: NFSv4 Flexfile Layout Driver Registering...
[    0.725163] Block layer SCSI generic (bsg) driver version 0.4 loaded (major 245)
[    0.725197] io scheduler mq-deadline registered
[    0.725207] io scheduler kyber registered
[    0.732851] gpio_rcar e6050000.gpio: driving 16 GPIOs
[    0.733448] gpio_rcar e6051000.gpio: driving 29 GPIOs
[    0.733994] gpio_rcar e6052000.gpio: driving 15 GPIOs
[    0.734530] gpio_rcar e6053000.gpio: driving 16 GPIOs
[    0.735081] gpio_rcar e6054000.gpio: driving 18 GPIOs
[    0.735636] gpio_rcar e6055000.gpio: driving 26 GPIOs
[    0.736180] gpio_rcar e6055400.gpio: driving 32 GPIOs
[    0.736850] gpio_rcar e6055800.gpio: driving 4 GPIOs
[    0.738501] rcar-pcie fe000000.pcie: host bridge /soc/pcie@fe000000 ranges:
[    0.738563] rcar-pcie fe000000.pcie:       IO 0x00fe100000..0x00fe1fffff -> 0x0000000000
[    0.738647] rcar-pcie fe000000.pcie:      MEM 0x00fe200000..0x00fe3fffff -> 0x00fe200000
[    0.738697] rcar-pcie fe000000.pcie:      MEM 0x0030000000..0x0037ffffff -> 0x0030000000
[    0.738728] rcar-pcie fe000000.pcie:      MEM 0x0038000000..0x003fffffff -> 0x0038000000
[    0.738766] rcar-pcie fe000000.pcie:   IB MEM 0x0040000000..0x00bfffffff -> 0x0040000000
[    0.752979] rcar-pcie fe000000.pcie: PCIe x1: link up
[    0.754952] rcar-pcie fe000000.pcie: PCI host bridge to bus 0000:00
[    0.754972] pci_bus 0000:00: root bus resource [bus 00-ff]
[    0.754985] pci_bus 0000:00: root bus resource [io  0x0000-0xfffff]
[    0.754996] pci_bus 0000:00: root bus resource [mem 0xfe200000-0xfe3fffff]
[    0.755007] pci_bus 0000:00: root bus resource [mem 0x30000000-0x37ffffff]
[    0.755018] pci_bus 0000:00: root bus resource [mem 0x38000000-0x3fffffff pref]
[    0.755099] pci 0000:00:00.0: [1912:0028] type 01 class 0x060400
[    0.755167] pci 0000:00:00.0: enabling Extended Tags
[    0.755283] pci 0000:00:00.0: PME# supported from D0 D3hot D3cold
[    0.757386] pci 0000:01:00.0: [8086:10d3] type 00 class 0x020000
[    0.757495] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x0001ffff]
[    0.757534] pci 0000:01:00.0: reg 0x14: [mem 0x00000000-0x0007ffff]
[    0.757573] pci 0000:01:00.0: reg 0x18: [io  0x0000-0x001f]
[    0.757611] pci 0000:01:00.0: reg 0x1c: [mem 0x00000000-0x00003fff]
[    0.757707] pci 0000:01:00.0: reg 0x30: [mem 0x00000000-0x0003ffff pref]
[    0.757953] pci 0000:01:00.0: PME# supported from D0 D3hot D3cold
[    0.769639] pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
[    0.769676] pci 0000:00:00.0: BAR 14: assigned [mem 0xfe200000-0xfe2fffff]
[    0.769689] pci 0000:00:00.0: BAR 15: assigned [mem 0x38000000-0x380fffff pref]
[    0.769700] pci 0000:00:00.0: BAR 13: assigned [io  0x1000-0x1fff]
[    0.769721] pci 0000:01:00.0: BAR 1: assigned [mem 0xfe200000-0xfe27ffff]
[    0.769745] pci 0000:01:00.0: BAR 6: assigned [mem 0x38000000-0x3803ffff pref]\b
[    0.769756] pci 0000:01:00.0: BAR 0: assigned [mem 0xfe280000-0xfe29ffff]
[    0.769778] pci 0000:01:00.0: BAR 3: assigned [mem 0xfe2a0000-0xfe2a3fff]
[    0.769801] pci 0000:01:00.0: BAR 2: assigned [io  0x1000-0x101f]
[    0.769828] pci 0000:00:00.0: PCI bridge to [bus 01]
[    0.769839] pci 0000:00:00.0:   bridge window [io  0x1000-0x1fff]
[    0.769853] pci 0000:00:00.0:   bridge window [mem 0xfe200000-0xfe2fffff]
[    0.769865] pci 0000:00:00.0:   bridge window [mem 0x38000000-0x380fffff pref]\b
[    0.770087] pcieport 0000:00:00.0: enabling device (0000 -> 0003)
[    0.770414] pcieport 0000:00:00.0: PME: Signaling with IRQ 123
[    0.771005] rcar-pcie ee800000.pcie: host bridge /soc/pcie@ee800000 ranges:
[    0.771064] rcar-pcie ee800000.pcie:       IO 0x00ee900000..0x00ee9fffff -> 0x0000000000
[    0.771117] rcar-pcie ee800000.pcie:      MEM 0x00eea00000..0x00eebfffff -> 0x00eea00000
[    0.771170] rcar-pcie ee800000.pcie:      MEM 0x00c0000000..0x00c7ffffff -> 0x00c0000000
[    0.771200] rcar-pcie ee800000.pcie:      MEM 0x00c8000000..0x00cfffffff -> 0x00c8000000
[    0.771239] rcar-pcie ee800000.pcie:   IB MEM 0x0040000000..0x00bfffffff -> 0x0040000000
[    0.836449] rcar-pcie ee800000.pcie: PCIe link down
[    0.949044] SuperH (H)SCI(F) driver initialized
[    0.950166] e6550000.serial: ttySC1 at MMIO 0xe6550000 (irq = 26, base_baud = 0) is a hscif
[    0.951497] e6e88000.serial: ttySC0 at MMIO 0xe6e88000 (irq = 109, base_baud = 0) is a scif
[    0.951552] BUG: spinlock bad magic on CPU#1, swapper/0/1
[    0.951573]  lock: sci_ports+0x0/0x4c80, .magic: 00000000, .owner: <none>/-1, .owner_cpu: 0
[    0.951584] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc2-arm64-renesas #57
[    0.951589] Hardware name: Renesas Salvator-X 2nd version board based on r8a77961 (DT)
[    0.951595] Call trace:
[    0.951609]  dump_backtrace+0x0/0x1d8
[    0.951615]  show_stack+0x14/0x20
[    0.951626]  dump_stack+0xe8/0x130
[    0.951635]  spin_dump+0x6c/0x88
[    0.951641]  do_raw_spin_lock+0xb0/0xf8
[    0.951650]  _raw_spin_lock_irqsave+0x80/0xa0
[    0.951660]  uart_add_one_port+0x3b0/0x4e8
[    0.951666]  sci_probe+0x504/0x7c8
[    0.951675]  platform_drv_probe+0x50/0xa0
[    0.951686]  really_probe+0xdc/0x330
[    0.951692]  driver_probe_device+0x58/0xb8
[    0.951699]  device_driver_attach+0x6c/0x90
[    0.951705]  __driver_attach+0x54/0xe0
[    0.951712]  bus_for_each_dev+0x74/0xc8
[    0.951718]  driver_attach+0x20/0x28
[    0.951725]  bus_add_driver+0x140/0x1e8
[    0.951730]  driver_register+0x60/0x110
[    0.951737]  __platform_driver_register+0x40/0x48
[    0.951746]  sci_init+0x2c/0x34
[    0.951754]  do_one_initcall+0x160/0x32c
[    0.951759]  kernel_init_freeable+0x2c0/0x328
[    0.951765]  kernel_init+0x10/0x108
[    0.951772]  ret_from_fork+0x10/0x18
[    2.359682] printk: console [ttySC0] enabled
[    2.380944] loop: module loaded
[    2.386504] libphy: Fixed MDIO Bus: probed
[    2.391119] tun: Universal TUN/TAP device driver, 1.6
[    2.396428] CAN device driver interface
[    2.400719] e1000e: Intel(R) PRO/1000 Network Driver - 3.2.6-k
[    2.406565] e1000e: Copyright(c) 1999 - 2015 Intel Corporation.
[    2.412679] e1000e 0000:01:00.0: enabling device (0000 -> 0002)
[    2.418852] e1000e 0000:01:00.0: Interrupt Throttling Rate (ints/sec) set to dynamic conservative mode
[    2.428231] e1000e 0000:01:00.0 0000:01:00.0 (uninitialized): Failed to initialize MSI-X interrupts.  Falling back to MSI interrupts.
[    2.488894] e1000e 0000:01:00.0 0000:01:00.0 (uninitialized): registered PHC clock
[    2.550588] e1000e 0000:01:00.0 eth0: (PCI Express:2.5GT/s:Width x1) 68:05:ca:40:68:2b
[    2.558551] e1000e 0000:01:00.0 eth0: Intel(R) PRO/1000 Network Connection
[    2.565453] e1000e 0000:01:00.0 eth0: MAC: 3, PHY: 8, PBA No: E46981-008
[    2.573693] libphy: ravb_mii: probed
[    2.578523] ravb e6800000.ethernet eth1: Base address at 0xe6800000, 2e:09:0a:01:99:3f, IRQ 106.
[    2.587769] VFIO - User Level meta-driver version: 0.3
[    2.593313] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
[    2.599864] ehci-pci: EHCI PCI platform driver
[    2.604370] ehci-platform: EHCI generic platform driver
[    2.610305] ehci-platform ee0a0100.usb: EHCI Host Controller
[    2.616008] ehci-platform ee0a0100.usb: new USB bus registered, assigned bus number 1
[    2.623965] ehci-platform ee0a0100.usb: irq 113, io mem 0xee0a0100
[    2.644217] ehci-platform ee0a0100.usb: USB 2.0 started, EHCI 1.10
[    2.651195] hub 1-0:1.0: USB hub found
[    2.654994] hub 1-0:1.0: 1 port detected
[    2.659421] ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
[    2.665690] ohci-pci: OHCI PCI platform driver
[    2.670190] ohci-platform: OHCI generic platform driver
[    2.676062] ohci-platform ee0a0000.usb: Generic Platform OHCI controller
[    2.682794] ohci-platform ee0a0000.usb: new USB bus registered, assigned bus number 2
[    2.690746] ohci-platform ee0a0000.usb: irq 113, io mem 0xee0a0000
[    2.787535] hub 2-0:1.0: USB hub found
[    2.791330] hub 2-0:1.0: 1 port detected
[    2.796339] xhci-hcd ee000000.usb: xHCI Host Controller
[    2.801630] xhci-hcd ee000000.usb: new USB bus registered, assigned bus number 3
[    2.809181] xhci-hcd ee000000.usb: Direct firmware load for r8a779x_usb3_v3.dlmem failed with error -2
[    2.818543] xhci-hcd ee000000.usb: can't setup: -2
[    2.823356] xhci-hcd ee000000.usb: USB bus 3 deregistered
[    2.828855] xhci-hcd: probe of ee000000.usb failed with error -2
[    2.835064] usbcore: registered new interface driver usb-storage
[    2.841817] renesas_usbhs e6590000.usb: host probed
[    2.846720] renesas_usbhs e6590000.usb: no transceiver found
[    2.852525] renesas_usbhs e6590000.usb: gadget probed
[    2.857816] renesas_usbhs e6590000.usb: probed
[    2.863300] renesas_usb3 ee020000.usb: probed with phy
[    2.869064] i2c /dev entries driver
[    2.880206] cs2000-cp 1-004f: revision - C1
[    2.884510] i2c-rcar e6510000.i2c: probed
[    2.889315] pca953x 2-0020: supply vcc not found, using dummy regulator
[    2.896042] pca953x 2-0020: using no AI
[    2.907460] i2c-rcar e66d8000.i2c: probed
[    2.912100] adv748x 2-0070: Endpoint /soc/i2c@e66d8000/video-receiver@70/port@7/endpoint on port 7
[    2.921131] adv748x 2-0070: Endpoint /soc/i2c@e66d8000/video-receiver@70/port@8/endpoint on port 8
[    2.930138] adv748x 2-0070: Endpoint /soc/i2c@e66d8000/video-receiver@70/port@a/endpoint on port 10
[    2.939227] adv748x 2-0070: Endpoint /soc/i2c@e66d8000/video-receiver@70/port@b/endpoint on port 11
[    2.948456] random: fast init done
[    2.952071] adv748x 2-0070: chip found @ 0xe0 revision 2143
[    3.002137] rcar_gen3_thermal e6198000.thermal: TSC0: Loaded 1 trip points
[    3.013252] rcar_gen3_thermal e6198000.thermal: TSC1: Loaded 1 trip points
[    3.024345] rcar_gen3_thermal e6198000.thermal: TSC2: Loaded 2 trip points
[    3.033379] cpufreq: cpufreq_online: CPU0: Running at unlisted freq: 1497600 KHz
[    3.040873] cpufreq: cpufreq_online: CPU0: Unlisted initial frequency changed to: 1500000 KHz
[    3.050435] cpufreq: cpufreq_online: CPU2: Running at unlisted freq: 1198080 KHz
[    3.058412] cpufreq: cpufreq_online: CPU2: Unlisted initial frequency changed to: 1200000 KHz
[    3.068500] renesas_sdhi_internal_dmac ee100000.sd: Got CD GPIO
[    3.074469] renesas_sdhi_internal_dmac ee100000.sd: Got WP GPIO
[    3.130048] renesas_sdhi_internal_dmac ee140000.sd: mmc0 base at 0x00000000ee140000, max clock rate 200 MHz
[    3.140414] renesas_sdhi_internal_dmac ee160000.sd: Got CD GPIO
[    3.146422] renesas_sdhi_internal_dmac ee160000.sd: Got WP GPIO
[    3.153370] ledtrig-cpu: registered to indicate activity on CPUs
[    3.159672] ccree e6601000.crypto: ARM CryptoCell 630P Driver: HW version 0xAF400001/0xDCC63000, Driver version 5.0
[    3.175898] ccree e6601000.crypto: ARM ccree device initialized
[    3.182622] usbcore: registered new interface driver usbhid
[    3.188298] usbhid: USB HID core driver
[    3.210319] NET: Registered protocol family 17
[    3.214806] can: controller area network core (rev 20170425 abi 9)
[    3.221137] NET: Registered protocol family 29
[    3.225621] can: raw protocol (rev 20170425)
[    3.229928] can: broadcast manager protocol (rev 20170425 t)
[    3.235606] can: netlink gateway (rev 20190810) max_hops=1
[    3.241350] Key type dns_resolver registered
[    3.246029] registered taskstats version 1
[    3.300561] mmc0: new HS400 MMC card at address 0001
[    3.306169] mmcblk0: mmc0:0001 BGSD4R 29.1 GiB 
[    3.308260] ehci-platform ee080100.usb: EHCI Host Controller
[    3.310909] mmcblk0boot0: mmc0:0001 BGSD4R partition 1 31.9 MiB
[    3.316393] ehci-platform ee080100.usb: new USB bus registered, assigned bus number 3
[    3.322464] mmcblk0boot1: mmc0:0001 BGSD4R partition 2 31.9 MiB
[    3.330211] usb 2-1: new full-speed USB device number 2 using ohci-platform
[    3.336227] mmcblk0rpmb: mmc0:0001 BGSD4R partition 3 4.00 MiB, chardev (243:0)
[    3.343167] ehci-platform ee080100.usb: irq 112, io mem 0xee080100
[    3.357009]  mmcblk0: p1
[    3.780248] ehci-platform ee080100.usb: USB 2.0 started, EHCI 1.10
[    3.787213] hub 3-0:1.0: USB hub found
[    3.791013] hub 3-0:1.0: 1 port detected
[    3.795878] ohci-platform ee080000.usb: Generic Platform OHCI controller
[    3.802640] ohci-platform ee080000.usb: new USB bus registered, assigned bus number 4
[    3.810593] ohci-platform ee080000.usb: irq 112, io mem 0xee080000
[    3.907545] hub 4-0:1.0: USB hub found
[    3.911408] hub 4-0:1.0: 1 port detected
[    3.916512] renesas_sdhi_internal_dmac ee100000.sd: Got CD GPIO
[    3.922489] renesas_sdhi_internal_dmac ee100000.sd: Got WP GPIO
[    3.976633] renesas_sdhi_internal_dmac ee100000.sd: mmc1 base at 0x00000000ee100000, max clock rate 200 MHz
[    3.987262] renesas_sdhi_internal_dmac ee160000.sd: Got CD GPIO
[    3.993238] renesas_sdhi_internal_dmac ee160000.sd: Got WP GPIO
[    4.048185] renesas_sdhi_internal_dmac ee160000.sd: mmc2 base at 0x00000000ee160000, max clock rate 200 MHz
[    4.062206] input: keys as /devices/platform/keys/input/input0
[    4.069056] ALSA device list:
[    4.072030]   No soundcards found.
[    4.075505] Warning: unable to open an initial console.
[    4.091456] Freeing unused kernel memory: 18048K
[    4.096287] Run /sbin/init as init process
[    4.100486]   with arguments:
[    4.103524]     /sbin/init
[    4.106284]   with environment:
[    4.109495]     HOME=/
[    4.111868]     TERM=linux
[    4.294444] mmc1: new ultra high speed SDR104 SDHC card at address aaaa
[    4.302053] mmcblk1: mmc1:aaaa SL16G 14.8 GiB 
[    4.311499]  mmcblk1: p1
[    4.399371] mmc2: new ultra high speed SDR104 SDHC card at address aaaa
[    4.406708] mmcblk2: mmc2:aaaa SL32G 29.7 GiB 
[    4.416734]  mmcblk2: p1

(none) login: root
login[222]: root login on 'ttySC0'
~ # echo N > /sys/module/printk/parameters/console_suspend
~ # echo platform > /sys/power/pm_test
~ # dmesg | grep pci
[    0.738501] rcar-pcie fe000000.pcie: host bridge /soc/pcie@fe000000 ranges:
[    0.738563] rcar-pcie fe000000.pcie:       IO 0x00fe100000..0x00fe1fffff -> 0x0000000000
[    0.738647] rcar-pcie fe000000.pcie:      MEM 0x00fe200000..0x00fe3fffff -> 0x00fe200000
[    0.738697] rcar-pcie fe000000.pcie:      MEM 0x0030000000..0x0037ffffff -> 0x0030000000
[    0.738728] rcar-pcie fe000000.pcie:      MEM 0x0038000000..0x003fffffff -> 0x0038000000
[    0.738766] rcar-pcie fe000000.pcie:   IB MEM 0x0040000000..0x00bfffffff -> 0x0040000000
[    0.752979] rcar-pcie fe000000.pcie: PCIe x1: link up
[    0.754952] rcar-pcie fe000000.pcie: PCI host bridge to bus 0000:00
[    0.754972] pci_bus 0000:00: root bus resource [bus 00-ff]
[    0.754985] pci_bus 0000:00: root bus resource [io  0x0000-0xfffff]
[    0.754996] pci_bus 0000:00: root bus resource [mem 0xfe200000-0xfe3fffff]
[    0.755007] pci_bus 0000:00: root bus resource [mem 0x30000000-0x37ffffff]
[    0.755018] pci_bus 0000:00: root bus resource [mem 0x38000000-0x3fffffff pref]
[    0.755099] pci 0000:00:00.0: [1912:0028] type 01 class 0x060400
[    0.755167] pci 0000:00:00.0: enabling Extended Tags
[    0.755283] pci 0000:00:00.0: PME# supported from D0 D3hot D3cold
[    0.757386] pci 0000:01:00.0: [8086:10d3] type 00 class 0x020000
[    0.757495] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x0001ffff]
[    0.757534] pci 0000:01:00.0: reg 0x14: [mem 0x00000000-0x0007ffff]
[    0.757573] pci 0000:01:00.0: reg 0x18: [io  0x0000-0x001f]
[    0.757611] pci 0000:01:00.0: reg 0x1c: [mem 0x00000000-0x00003fff]
[    0.757707] pci 0000:01:00.0: reg 0x30: [mem 0x00000000-0x0003ffff pref]
[    0.757953] pci 0000:01:00.0: PME# supported from D0 D3hot D3cold
[    0.769639] pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
[    0.769676] pci 0000:00:00.0: BAR 14: assigned [mem 0xfe200000-0xfe2fffff]
[    0.769689] pci 0000:00:00.0: BAR 15: assigned [mem 0x38000000-0x380fffff pref]
[    0.769700] pci 0000:00:00.0: BAR 13: assigned [io  0x1000-0x1fff]
[    0.769721] pci 0000:01:00.0: BAR 1: assigned [mem 0xfe200000-0xfe27ffff]
[    0.769745] pci 0000:01:00.0: BAR 6: assigned [mem 0x38000000-0x3803ffff pref]\b
[    0.769756] pci 0000:01:00.0: BAR 0: assigned [mem 0xfe280000-0xfe29ffff]
[    0.769778] pci 0000:01:00.0: BAR 3: assigned [mem 0xfe2a0000-0xfe2a3fff]
[    0.769801] pci 0000:01:00.0: BAR 2: assigned [io  0x1000-0x101f]
[    0.769828] pci 0000:00:00.0: PCI bridge to [bus 01]
[    0.769839] pci 0000:00:00.0:   bridge window [io  0x1000-0x1fff]
[    0.769853] pci 0000:00:00.0:   bridge window [mem 0xfe200000-0xfe2fffff]
[    0.769865] pci 0000:00:00.0:   bridge window [mem 0x38000000-0x380fffff pref]\b
[    0.770087] pcieport 0000:00:00.0: enabling device (0000 -> 0003)
[    0.770414] pcieport 0000:00:00.0: PME: Signaling with IRQ 123
[    0.771005] rcar-pcie ee800000.pcie: host bridge /soc/pcie@ee800000 ranges:
[    0.771064] rcar-pcie ee800000.pcie:       IO 0x00ee900000..0x00ee9fffff -> 0x0000000000
[    0.771117] rcar-pcie ee800000.pcie:      MEM 0x00eea00000..0x00eebfffff -> 0x00eea00000
[    0.771170] rcar-pcie ee800000.pcie:      MEM 0x00c0000000..0x00c7ffffff -> 0x00c0000000
[    0.771200] rcar-pcie ee800000.pcie:      MEM 0x00c8000000..0x00cfffffff -> 0x00c8000000
[    0.771239] rcar-pcie ee800000.pcie:   IB MEM 0x0040000000..0x00bfffffff -> 0x0040000000
[    0.836449] rcar-pcie ee800000.pcie: PCIe link down
[    2.599864] ehci-pci: EHCI PCI platform driver
[    2.665690] ohci-pci: OHCI PCI platform driver
~ # cat /proc/net/dev
Inter-|   Receive                                                |  Transmit
 face |bytes    packets errs drop fifo frame compressed multicast|bytes    packets errs drop fifo colls carrier compressed
    lo:       0       0    0    0    0     0          0         0        0       0    0    0    0     0       0          0
  eth0:       0       0    0    0    0     0          0         0        0       0    0    0    0     0       0          0
  eth1:       0       0    0    0    0     0          0         0        0       0    0    0    0     0       0          0
~ # echo mem > /sys/power/state
[  374.072298] PM: suspend entry (deep)
[  374.075994] Filesystems sync: 0.000 seconds
[  374.083159] Freezing user space processes ... (elapsed 0.001 seconds) done.
[  374.091570] OOM killer disabled.
[  374.094823] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
[  374.227534] e1000e: EEE TX LPI TIMER: 00000000
[  374.354388] SError Interrupt on CPU0, code 0xbf000002 -- SError
[  374.354390] CPU: 0 PID: 633 Comm: kworker/u12:4 Not tainted 5.8.0-rc2-arm64-renesas #57
[  374.354392] Hardware name: Renesas Salvator-X 2nd version board based on r8a77961 (DT)
[  374.354393] Workqueue: events_unbound async_run_entry_fn
[  374.354395] pstate: 40000085 (nZcv daIf -PAN -UAO BTYPE=--)
[  374.354396] pc : rcar_pci_read_reg+0xc/0x20
[  374.354397] lr : rcar_pcie_config_access+0xe4/0x1c0
[  374.354398] sp : ffff8000148aba80
[  374.354399] x29: ffff8000148aba80 x28: 0000000000000000 
[  374.354402] x27: ffff800011fb0000 x26: 0000000000000000 
[  374.354404] x25: 00000000000000cc x24: 0000000000000000 
[  374.354406] x23: ffff8000148abb54 x22: 0000000000000000 
[  374.354408] x21: ffff0006b484f800 x20: 0000000000000000 
[  374.354411] x19: ffff0006b3a14580 x18: 0000000000000000 
[  374.354413] x17: 0000000000000000 x16: 0000000000000000 
[  374.354415] x15: 0000000000000000 x14: 000006e45d8ca55e 
[  374.354417] x13: 0000000000000330 x12: 000000000000016d 
[  374.354419] x11: 0000000000000000 x10: 0000000000000000 
[  374.354421] x9 : ffff800012059120 x8 : ffff800011fb0948 
[  374.354423] x7 : ffff8000105075a8 x6 : 0000000000000000 
[  374.354425] x5 : ffff8000148abb54 x4 : 00000000000000cc 
[  374.354427] x3 : 0000000000000000 x2 : ffff800013700018 
[  374.354429] x1 : 0000000000000020 x0 : 00000000ffffffff 
[  374.354432] Kernel panic - not syncing: Asynchronous SError Interrupt
[  374.354433] CPU: 0 PID: 633 Comm: kworker/u12:4 Not tainted 5.8.0-rc2-arm64-renesas #57
[  374.354434] Hardware name: Renesas Salvator-X 2nd version board based on r8a77961 (DT)
[  374.354435] Workqueue: events_unbound async_run_entry_fn
[  374.354437] Call trace:
[  374.354438]  dump_backtrace+0x0/0x1d8
[  374.354439]  show_stack+0x14/0x20
[  374.354439]  dump_stack+0xe8/0x130
[  374.354440]  panic+0x168/0x380
[  374.354441]  nmi_panic+0x6c/0x70
[  374.354442]  arm64_serror_panic+0x74/0x88
[  374.354443]  do_serror+0x88/0x1a0
[  374.354444]  el1_error+0x84/0x100
[  374.354444]  rcar_pci_read_reg+0xc/0x20
[  374.354445]  rcar_pcie_read_conf+0x38/0xb0
[  374.354446]  pci_bus_read_config_word+0x84/0xe0
[  374.354447]  pci_read_config_word+0x28/0x40
[  374.354448]  pci_raw_set_power_state+0x108/0x2a8
[  374.354449]  pci_set_power_state+0x5c/0x150
[  374.354450]  pci_prepare_to_sleep+0x60/0x98
[  374.354451]  pci_pm_suspend_noirq+0xe4/0x278
[  374.354451]  dpm_run_callback+0x88/0x3c0
[  374.354452]  __device_suspend_noirq+0x68/0x1f0
[  374.354453]  async_suspend_noirq+0x20/0xa8
[  374.354454]  async_run_entry_fn+0x44/0x108
[  374.354455]  process_one_work+0x2a0/0x718
[  374.354456]  worker_thread+0x40/0x458
[  374.354457]  kthread+0x150/0x158
[  374.354457]  ret_from_fork+0x10/0x18
[  374.354484] SMP: stopping secondary CPUs
[  374.354485] Kernel Offset: disabled
[  374.354486] CPU features: 0x200022,21006004
---

Best regards,
Yoshihiro Shimoda

Re: [PATCH/RFC v4 0/4] treewide: add regulator condition on _mmc_suspend()

[Geert Uytterhoeven]

Hi Shimoda-san,

On Mon, Jun 29, 2020 at 12:04 PM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> > From: Geert Uytterhoeven, Sent: Friday, June 26, 2020 7:13 PM
> > On Fri, Jun 26, 2020 at 11:32 AM Yoshihiro Shimoda
> > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > Note that v5.8-rc2 with r8a77951-salvator-xs seems to cause panic from
> > > PCI driver when the system is suspended. So, I disabled the PCI
> > > devices when I tested this patch series.
> >
> > Does this happen with current renesas-devel and renesas_defconfig?
> > (it doesn't for me)
>
> Yes. I enabled PM_DEBUG and E1000E though.
>
> > Do you have any PCIe devices attached? (I haven't)
>
> Yes. (Intel Ethernet card is connected to the PCI slot.)
>
> < my environment >
> - r8a77961-salvator-xs
> - renesas-devel-2020-06-26-v5.8-rc2
>  + renesas_defconfig + PM_DEBUG + E1000E
> - initramfs

Doesn't fail for me on R-Car H3 ES2.0, so it needs the presence of a
PCIe card.  Unfortunately I haven't any (added to shopping wishlist).

Is this the board in Magnus' farm?
I do have access to that one.

> ~ # echo mem > /sys/power/state
> [  374.072298] PM: suspend entry (deep)
> [  374.075994] Filesystems sync: 0.000 seconds
> [  374.083159] Freezing user space processes ... (elapsed 0.001 seconds) done.
> [  374.091570] OOM killer disabled.
> [  374.094823] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
> [  374.227534] e1000e: EEE TX LPI TIMER: 00000000
> [  374.354388] SError Interrupt on CPU0, code 0xbf000002 -- SError
> [  374.354390] CPU: 0 PID: 633 Comm: kworker/u12:4 Not tainted 5.8.0-rc2-arm64-renesas #57
> [  374.354392] Hardware name: Renesas Salvator-X 2nd version board based on r8a77961 (DT)
> [  374.354393] Workqueue: events_unbound async_run_entry_fn
> [  374.354395] pstate: 40000085 (nZcv daIf -PAN -UAO BTYPE=--)
> [  374.354396] pc : rcar_pci_read_reg+0xc/0x20
> [  374.354397] lr : rcar_pcie_config_access+0xe4/0x1c0
> [  374.354398] sp : ffff8000148aba80
> [  374.354399] x29: ffff8000148aba80 x28: 0000000000000000
> [  374.354402] x27: ffff800011fb0000 x26: 0000000000000000
> [  374.354404] x25: 00000000000000cc x24: 0000000000000000
> [  374.354406] x23: ffff8000148abb54 x22: 0000000000000000
> [  374.354408] x21: ffff0006b484f800 x20: 0000000000000000
> [  374.354411] x19: ffff0006b3a14580 x18: 0000000000000000
> [  374.354413] x17: 0000000000000000 x16: 0000000000000000
> [  374.354415] x15: 0000000000000000 x14: 000006e45d8ca55e
> [  374.354417] x13: 0000000000000330 x12: 000000000000016d
> [  374.354419] x11: 0000000000000000 x10: 0000000000000000
> [  374.354421] x9 : ffff800012059120 x8 : ffff800011fb0948
> [  374.354423] x7 : ffff8000105075a8 x6 : 0000000000000000
> [  374.354425] x5 : ffff8000148abb54 x4 : 00000000000000cc
> [  374.354427] x3 : 0000000000000000 x2 : ffff800013700018
> [  374.354429] x1 : 0000000000000020 x0 : 00000000ffffffff
> [  374.354432] Kernel panic - not syncing: Asynchronous SError Interrupt
> [  374.354433] CPU: 0 PID: 633 Comm: kworker/u12:4 Not tainted 5.8.0-rc2-arm64-renesas #57
> [  374.354434] Hardware name: Renesas Salvator-X 2nd version board based on r8a77961 (DT)
> [  374.354435] Workqueue: events_unbound async_run_entry_fn
> [  374.354437] Call trace:
> [  374.354438]  dump_backtrace+0x0/0x1d8
> [  374.354439]  show_stack+0x14/0x20
> [  374.354439]  dump_stack+0xe8/0x130
> [  374.354440]  panic+0x168/0x380
> [  374.354441]  nmi_panic+0x6c/0x70
> [  374.354442]  arm64_serror_panic+0x74/0x88
> [  374.354443]  do_serror+0x88/0x1a0
> [  374.354444]  el1_error+0x84/0x100
> [  374.354444]  rcar_pci_read_reg+0xc/0x20
> [  374.354445]  rcar_pcie_read_conf+0x38/0xb0
> [  374.354446]  pci_bus_read_config_word+0x84/0xe0
> [  374.354447]  pci_read_config_word+0x28/0x40
> [  374.354448]  pci_raw_set_power_state+0x108/0x2a8
> [  374.354449]  pci_set_power_state+0x5c/0x150
> [  374.354450]  pci_prepare_to_sleep+0x60/0x98
> [  374.354451]  pci_pm_suspend_noirq+0xe4/0x278
> [  374.354451]  dpm_run_callback+0x88/0x3c0
> [  374.354452]  __device_suspend_noirq+0x68/0x1f0
> [  374.354453]  async_suspend_noirq+0x20/0xa8
> [  374.354454]  async_run_entry_fn+0x44/0x108
> [  374.354455]  process_one_work+0x2a0/0x718
> [  374.354456]  worker_thread+0x40/0x458
> [  374.354457]  kthread+0x150/0x158
> [  374.354457]  ret_from_fork+0x10/0x18
> [  374.354484] SMP: stopping secondary CPUs
> [  374.354485] Kernel Offset: disabled
> [  374.354486] CPU features: 0x200022,21006004

The failure mode looks like the PCI card is accessed while the PCI host
bridge has been suspended.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH/RFC v4 2/4] regulator: fixed: add regulator_ops members for suspend/resume

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2156 bytes --]

On Mon, Jun 29, 2020 at 02:42:26AM +0000, Yoshihiro Shimoda wrote:
> > From: Mark Brown, Sent: Friday, June 26, 2020 11:39 PM

Copying in Sudeep for the feedback on firmware interfaces.

> > According to the changelog this is all about reflecting changes in the
> > system state done by firmware but there's no interaction with firmware
> > here which means this will be at best fragile.  If we need to reflect
> > changes in firmware configuration I'd expect there to be some
> > interaction with firmware about how it is configured, or at least that
> > the configuration would come from the same source.

> I should have described background of previous patch series though,
> according to previous discussion [1] the firmware side (like PSCI) is
> also fragile unfortunately... So, I thought using regulator-off-in-suspend
> in a regulator was better.

> On other hand, Ulf is talking about either adding a property (perhaps like
> regulator-off-in-suspend) into a regulator or just adding a new property
> into MMC [2]. What do you think about Ulf' comment? I'm thinking
> adding a new property "full-pwr-cycle-in-suspend" is the best solution.
> This is because using a regulator property and reflecting a state of regulator without
> firmware is fragile, as you said.

TBH I worry about a property drifting out of sync with the firmware on
systems where the firmware can be updated.  Personally my default
assumption would always be that we're going to loose power for anything
except the RAM and whatever is needed for wake sources during suspend so
I find the discussion a bit surprising but in any case that seems like a
better option than trying to shoehorn things in the way the series here
did.  Like I said in my earlier replies if this is done through the
regulator API I'd expect it to be via the suspend interface.

> [1]
> https://lore.kernel.org/linux-renesas-soc/CAMuHMdXjU7N4oG89YsozGijMpjgKGN6ezw2qm6FeGX=JyRhsvg@mail.gmail.com/
> 
> [2]
> https://lore.kernel.org/linux-renesas-soc/CAPDyKFpiBU1D+a7zb+Ggm0_HZ+YR4=LXJZ5MPytXtT=uBEdjPA@mail.gmail.com/
> 
> Best regards,
> Yoshihiro Shimoda
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH/RFC v4 2/4] regulator: fixed: add regulator_ops members for suspend/resume

[Sudeep Holla]

On Mon, Jun 29, 2020 at 01:57:56PM +0100, Mark Brown wrote:
> On Mon, Jun 29, 2020 at 02:42:26AM +0000, Yoshihiro Shimoda wrote:
> > > From: Mark Brown, Sent: Friday, June 26, 2020 11:39 PM
>
> Copying in Sudeep for the feedback on firmware interfaces.
>

Thanks Mark.

> > > According to the changelog this is all about reflecting changes in the
> > > system state done by firmware but there's no interaction with firmware
> > > here which means this will be at best fragile.  If we need to reflect
> > > changes in firmware configuration I'd expect there to be some
> > > interaction with firmware about how it is configured, or at least that
> > > the configuration would come from the same source.
>

I agree.

> > I should have described background of previous patch series though,
> > according to previous discussion [1] the firmware side (like PSCI) is
> > also fragile unfortunately... So, I thought using regulator-off-in-suspend
> > in a regulator was better.
>

Please fix the firmware. You might have bigger problem than this if the
PSCI firmware is fragile as you state. Better to disable power management
on the platform if the firmware can't be fixed.

> > On other hand, Ulf is talking about either adding a property (perhaps like
> > regulator-off-in-suspend) into a regulator or just adding a new property
> > into MMC [2]. What do you think about Ulf' comment? I'm thinking
> > adding a new property "full-pwr-cycle-in-suspend" is the best solution.
> > This is because using a regulator property and reflecting a state of regulator without
> > firmware is fragile, as you said.

I haven't followed all the threads, but if it related to the policy you
want in the Linux, then may be use DT property or something. I don't know.
But if this is to indicate something based on firmware runtime/configuration,
then NACK for any approaches unconditionally.

>
> TBH I worry about a property drifting out of sync with the firmware on
> systems where the firmware can be updated.  Personally my default
> assumption would always be that we're going to loose power for anything
> except the RAM and whatever is needed for wake sources during suspend so
> I find the discussion a bit surprising but in any case that seems like a
> better option than trying to shoehorn things in the way the series here
> did.  Like I said in my earlier replies if this is done through the
> regulator API I'd expect it to be via the suspend interface.
>

+1. If this platform needs Linux to keep some state on for users in the
firmware or anything outside Linux, it must resume back in the same state
as we entered the suspend state from the kernel.

--
Regards,
Sudeep

Re: [PATCH/RFC v4 2/4] regulator: fixed: add regulator_ops members for suspend/resume

[Geert Uytterhoeven]

Hi Sudeep,

On Mon, Jun 29, 2020 at 3:40 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> On Mon, Jun 29, 2020 at 01:57:56PM +0100, Mark Brown wrote:
> > On Mon, Jun 29, 2020 at 02:42:26AM +0000, Yoshihiro Shimoda wrote:
> > > > From: Mark Brown, Sent: Friday, June 26, 2020 11:39 PM
> > > > According to the changelog this is all about reflecting changes in the
> > > > system state done by firmware but there's no interaction with firmware
> > > > here which means this will be at best fragile.  If we need to reflect
> > > > changes in firmware configuration I'd expect there to be some
> > > > interaction with firmware about how it is configured, or at least that
> > > > the configuration would come from the same source.
>
> I agree.
>
> > > I should have described background of previous patch series though,
> > > according to previous discussion [1] the firmware side (like PSCI) is
> > > also fragile unfortunately... So, I thought using regulator-off-in-suspend
> > > in a regulator was better.
>
> Please fix the firmware. You might have bigger problem than this if the
> PSCI firmware is fragile as you state. Better to disable power management
> on the platform if the firmware can't be fixed.

Saying the implementation is "fragile" might be bad wording.
The issue is more with the specification being vague (see more below).

> > > On other hand, Ulf is talking about either adding a property (perhaps like
> > > regulator-off-in-suspend) into a regulator or just adding a new property
> > > into MMC [2]. What do you think about Ulf' comment? I'm thinking
> > > adding a new property "full-pwr-cycle-in-suspend" is the best solution.
> > > This is because using a regulator property and reflecting a state of regulator without
> > > firmware is fragile, as you said.
>
> I haven't followed all the threads, but if it related to the policy you
> want in the Linux, then may be use DT property or something. I don't know.
> But if this is to indicate something based on firmware runtime/configuration,
> then NACK for any approaches unconditionally.

Like "arm,psci-system-suspend-is-power-down"[1]?

> > TBH I worry about a property drifting out of sync with the firmware on
> > systems where the firmware can be updated.  Personally my default
> > assumption would always be that we're going to loose power for anything

OK, so that's the "safe" way to handle this: assume power is lost.

> > except the RAM and whatever is needed for wake sources during suspend so

Oh, even wake-up sources may become unpowered[2] ;-)
And thus stop working ;-(

> > I find the discussion a bit surprising but in any case that seems like a
> > better option than trying to shoehorn things in the way the series here
> > did.  Like I said in my earlier replies if this is done through the
> > regulator API I'd expect it to be via the suspend interface.
>
> +1. If this platform needs Linux to keep some state on for users in the
> firmware or anything outside Linux, it must resume back in the same state
> as we entered the suspend state from the kernel.

I think you're misunderstanding the issue: this is not related at all
to Linux keeping state for non-Linux users.

This is all about how to know what exactly PSCI is powering down during
SYSTEM_SUSPEND.  In this specific case, it is about knowing if the eMMC
is powered down or not, as Linux should follow a specific procedure to
prepare the eMMC for that, and Linux should not if that isn't the case.

I had a quick look at the latest revision of the PSCI specification, and
it doesn't look like anything has changed in that area since my old patch
series from 2017.  So it still boils down to: we don't know what a
specific PSCI implementation will do, as basically anything is
compliant, so the only safe thing is to assume the worst.

Or am I missing something?
Thanks!

[1] "[PATCH/RFC 4/6] drivers: firmware: psci: Fix non-PMIC wake-up if
    SYSTEM_SUSPEND cuts power"
    https://lore.kernel.org/linux-arm-kernel/1487622809-25127-5-git-send-email-geert+renesas@glider.be/

[2] [PATCH/RFC 0/6] PSCI: Fix non-PMIC wake-up if SYSTEM_SUSPEND cuts
    power
    https://lore.kernel.org/linux-arm-kernel/1487622809-25127-1-git-send-email-geert+renesas@glider.be/

[3] https://developer.arm.com/architectures/system-architectures/software-standards/psci

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH/RFC v4 2/4] regulator: fixed: add regulator_ops members for suspend/resume

[Sudeep Holla]

On Mon, Jun 29, 2020 at 04:15:39PM +0200, Geert Uytterhoeven wrote:
> Hi Sudeep,
>
> On Mon, Jun 29, 2020 at 3:40 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > On Mon, Jun 29, 2020 at 01:57:56PM +0100, Mark Brown wrote:
> > > On Mon, Jun 29, 2020 at 02:42:26AM +0000, Yoshihiro Shimoda wrote:
> > > > > From: Mark Brown, Sent: Friday, June 26, 2020 11:39 PM
> > > > > According to the changelog this is all about reflecting changes in the
> > > > > system state done by firmware but there's no interaction with firmware
> > > > > here which means this will be at best fragile.  If we need to reflect
> > > > > changes in firmware configuration I'd expect there to be some
> > > > > interaction with firmware about how it is configured, or at least that
> > > > > the configuration would come from the same source.
> >
> > I agree.
> >
> > > > I should have described background of previous patch series though,
> > > > according to previous discussion [1] the firmware side (like PSCI) is
> > > > also fragile unfortunately... So, I thought using regulator-off-in-suspend
> > > > in a regulator was better.
> >
> > Please fix the firmware. You might have bigger problem than this if the
> > PSCI firmware is fragile as you state. Better to disable power management
> > on the platform if the firmware can't be fixed.
>
> Saying the implementation is "fragile" might be bad wording.
> The issue is more with the specification being vague (see more below).
>

Please elaborate on the vague part of the specification, happy to help you
get that fixed if it really needs to be.

> > > > On other hand, Ulf is talking about either adding a property (perhaps like
> > > > regulator-off-in-suspend) into a regulator or just adding a new property
> > > > into MMC [2]. What do you think about Ulf' comment? I'm thinking
> > > > adding a new property "full-pwr-cycle-in-suspend" is the best solution.
> > > > This is because using a regulator property and reflecting a state of regulator without
> > > > firmware is fragile, as you said.
> >
> > I haven't followed all the threads, but if it related to the policy you
> > want in the Linux, then may be use DT property or something. I don't know.
> > But if this is to indicate something based on firmware runtime/configuration,
> > then NACK for any approaches unconditionally.
>
> Like "arm,psci-system-suspend-is-power-down"[1]?
>

Really sounds hack to me.

> > > TBH I worry about a property drifting out of sync with the firmware on
> > > systems where the firmware can be updated.  Personally my default
> > > assumption would always be that we're going to loose power for anything
>
> OK, so that's the "safe" way to handle this: assume power is lost.
>
> > > except the RAM and whatever is needed for wake sources during suspend so
>
> Oh, even wake-up sources may become unpowered[2] ;-)

That is some serious issue. If there is no power, how can you expect it
to be wake up source ? Sounds something wrong fundamentally IMO.

> And thus stop working ;-(
>
> > > I find the discussion a bit surprising but in any case that seems like a
> > > better option than trying to shoehorn things in the way the series here
> > > did.  Like I said in my earlier replies if this is done through the
> > > regulator API I'd expect it to be via the suspend interface.
> >
> > +1. If this platform needs Linux to keep some state on for users in the
> > firmware or anything outside Linux, it must resume back in the same state
> > as we entered the suspend state from the kernel.
>
> I think you're misunderstanding the issue: this is not related at all
> to Linux keeping state for non-Linux users.
>

OK, thanks for confirming.

> This is all about how to know what exactly PSCI is powering down during
> SYSTEM_SUSPEND.  In this specific case, it is about knowing if the eMMC
> is powered down or not, as Linux should follow a specific procedure to
> prepare the eMMC for that, and Linux should not if that isn't the case.
>

OK, unless you are optimising, you shouldn't care then what PSCI does.
If you don't need eMMC, just suspend/power it off before you enter system/
psci suspend.

> I had a quick look at the latest revision of the PSCI specification, and
> it doesn't look like anything has changed in that area since my old patch
> series from 2017.  So it still boils down to: we don't know what a
> specific PSCI implementation will do, as basically anything is
> compliant, so the only safe thing is to assume the worst.
>

The specification states clearly:
"... all devices in the system must be in a state that is compatible
with entry into the system state. These preconditions are beyond the scope
of this specification and are therefore not described here."
"Prior to the call, the OS must disable all sources of wakeup other than
those it needs to support for its implementation of suspend to RAM."

And of course, the firmware must rely on OSPM to do proper device PM if
it is not shared resource. Trying to be aggressive and turning off all
the wakeup sources which out knowledge of it is broken firmware.

I see nothing has been fixed in the firmware too and we are still
discussing the same after 3 years 😄. Clearly we should start trusting
firmware and built capability to fix and replace it if there are bugs
just like kernel and stop hacking around in the kernel to deal with
just broken platform/psci firmware.

--
Regards,
Sudeep

Re: [PATCH/RFC v4 2/4] regulator: fixed: add regulator_ops members for suspend/resume

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2600 bytes --]

On Mon, Jun 29, 2020 at 04:07:28PM +0100, Sudeep Holla wrote:
> On Mon, Jun 29, 2020 at 04:15:39PM +0200, Geert Uytterhoeven wrote:

> > This is all about how to know what exactly PSCI is powering down during
> > SYSTEM_SUSPEND.  In this specific case, it is about knowing if the eMMC
> > is powered down or not, as Linux should follow a specific procedure to
> > prepare the eMMC for that, and Linux should not if that isn't the case.

> OK, unless you are optimising, you shouldn't care then what PSCI does.
> If you don't need eMMC, just suspend/power it off before you enter system/
> psci suspend.

That only works if the power off procedure doesn't require that power be
removed as part of the procedure.  There's a reasonable argument that
specs that have such requirements are unhelpful but that doesn't mean
that nobody will make hardware with such requrements which creates
problems for generic code that needs to control that hardware if it
can't discover the power state over suspend.

> > I had a quick look at the latest revision of the PSCI specification, and
> > it doesn't look like anything has changed in that area since my old patch
> > series from 2017.  So it still boils down to: we don't know what a
> > specific PSCI implementation will do, as basically anything is
> > compliant, so the only safe thing is to assume the worst.

> The specification states clearly:
> "... all devices in the system must be in a state that is compatible
> with entry into the system state. These preconditions are beyond the scope
> of this specification and are therefore not described here."
> "Prior to the call, the OS must disable all sources of wakeup other than
> those it needs to support for its implementation of suspend to RAM."

This gets a bit circular for a generic OS since the OS needs some way to
figure out what it's supposed to do on a given platform - for example
the OS may be happy to use wakeup sources that the firmware is just
going to cut power on.

> I see nothing has been fixed in the firmware too and we are still
> discussing the same after 3 years 😄. Clearly we should start trusting
> firmware and built capability to fix and replace it if there are bugs
> just like kernel and stop hacking around in the kernel to deal with
> just broken platform/psci firmware.

This isn't just an issue of buggy firmware as far as I can see, it's
also a lack of ability for the OS and firmware to communicate
information about their intentions to each other.  As things stand you'd
need to put static information in the DT.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH/RFC v4 2/4] regulator: fixed: add regulator_ops members for suspend/resume

[Sudeep Holla]

On Mon, Jun 29, 2020 at 05:14:50PM +0100, Mark Brown wrote:
> On Mon, Jun 29, 2020 at 04:07:28PM +0100, Sudeep Holla wrote:
> > On Mon, Jun 29, 2020 at 04:15:39PM +0200, Geert Uytterhoeven wrote:
>
> > > This is all about how to know what exactly PSCI is powering down during
> > > SYSTEM_SUSPEND.  In this specific case, it is about knowing if the eMMC
> > > is powered down or not, as Linux should follow a specific procedure to
> > > prepare the eMMC for that, and Linux should not if that isn't the case.
>
> > OK, unless you are optimising, you shouldn't care then what PSCI does.
> > If you don't need eMMC, just suspend/power it off before you enter system/
> > psci suspend.
>
> That only works if the power off procedure doesn't require that power be
> removed as part of the procedure.  There's a reasonable argument that
> specs that have such requirements are unhelpful but that doesn't mean
> that nobody will make hardware with such requrements which creates
> problems for generic code that needs to control that hardware if it
> can't discover the power state over suspend.
>

Fair enough.

> > > I had a quick look at the latest revision of the PSCI specification, and
> > > it doesn't look like anything has changed in that area since my old patch
> > > series from 2017.  So it still boils down to: we don't know what a
> > > specific PSCI implementation will do, as basically anything is
> > > compliant, so the only safe thing is to assume the worst.
>
> > The specification states clearly:
> > "... all devices in the system must be in a state that is compatible
> > with entry into the system state. These preconditions are beyond the scope
> > of this specification and are therefore not described here."
> > "Prior to the call, the OS must disable all sources of wakeup other than
> > those it needs to support for its implementation of suspend to RAM."
>
> This gets a bit circular for a generic OS since the OS needs some way to
> figure out what it's supposed to do on a given platform - for example
> the OS may be happy to use wakeup sources that the firmware is just
> going to cut power on.
>

While I understand the sentiments here, PSCI is targeted to address CPU
power state management mainly and system states like suspend/reset and
poweroff which involves last CPU. This is one of the reason it is out of
the scope of the specification.

Here is my understanding. DT specifies all the wakeup sources. Linux
can configure those and user may choose to enable a subset of them is
wakeup. As stated in the spec and also based on what we already do in
the kernel, we disable all other wakeup sources.

The PSCI firmware can then either read those from the interrupt controller
or based on static platform understanding, must not disable those wakeup
sources.

> > I see nothing has been fixed in the firmware too and we are still
> > discussing the same after 3 years 😄. Clearly we should start trusting
> > firmware and built capability to fix and replace it if there are bugs
> > just like kernel and stop hacking around in the kernel to deal with
> > just broken platform/psci firmware.
>
> This isn't just an issue of buggy firmware as far as I can see, it's
> also a lack of ability for the OS and firmware to communicate
> information about their intentions to each other.  As things stand you'd
> need to put static information in the DT.

It is easy for DT to get out of sync with firmware unless it is generated
by the same firmware. That's the reason why I am against such multiple
sources of information. I understand ACPI has more flexibility and I did
discuss this in LPC few years back and people were happy to have simple
model as I have mentioned above. I was pushing to add more clarity to
the specification but as I mentioned it was rejected as it is more a cpu
and system centric spec and avoids talking about devices which I kind
of agree.

Each device or platform having its specific property in DT is not scalable.
Not sure if it is a generic problem. If it is, I would like to understand
more details on such lack of ability for communtication between OS and
firmware.

--
Regards,
Sudeep

Re: [PATCH/RFC v4 2/4] regulator: fixed: add regulator_ops members for suspend/resume

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 3334 bytes --]

On Mon, Jun 29, 2020 at 05:42:07PM +0100, Sudeep Holla wrote:
> On Mon, Jun 29, 2020 at 05:14:50PM +0100, Mark Brown wrote:
> > On Mon, Jun 29, 2020 at 04:07:28PM +0100, Sudeep Holla wrote:
> >
> > > The specification states clearly:
> > > "... all devices in the system must be in a state that is compatible
> > > with entry into the system state. These preconditions are beyond the scope
> > > of this specification and are therefore not described here."
> > > "Prior to the call, the OS must disable all sources of wakeup other than
> > > those it needs to support for its implementation of suspend to RAM."

> > This gets a bit circular for a generic OS since the OS needs some way to
> > figure out what it's supposed to do on a given platform - for example
> > the OS may be happy to use wakeup sources that the firmware is just
> > going to cut power on.

> While I understand the sentiments here, PSCI is targeted to address CPU
> power state management mainly and system states like suspend/reset and
> poweroff which involves last CPU. This is one of the reason it is out of
> the scope of the specification.

Sure, but as soon as we start talking about the last CPU stuff we're
inevitably talking about the system as a whole.

> Here is my understanding. DT specifies all the wakeup sources. Linux
> can configure those and user may choose to enable a subset of them is
> wakeup. As stated in the spec and also based on what we already do in
> the kernel, we disable all other wakeup sources.

> The PSCI firmware can then either read those from the interrupt controller
> or based on static platform understanding, must not disable those wakeup
> sources.

That bit about static platform understanding isn't super helpful for the
OS, so long as the firmware might do that the OS is pretty much out of
luck.  

> > > I see nothing has been fixed in the firmware too and we are still
> > > discussing the same after 3 years 😄. Clearly we should start trusting
> > > firmware and built capability to fix and replace it if there are bugs
> > > just like kernel and stop hacking around in the kernel to deal with
> > > just broken platform/psci firmware.

> > This isn't just an issue of buggy firmware as far as I can see, it's
> > also a lack of ability for the OS and firmware to communicate
> > information about their intentions to each other.  As things stand you'd
> > need to put static information in the DT.

> It is easy for DT to get out of sync with firmware unless it is generated
> by the same firmware. That's the reason why I am against such multiple

The ability for things to get out of sync also concerns me as I said
further back in the thread but I'm not sure we have much alternative,
realistically we're going to need some facility to work around firmware
that isn't ideal.

> sources of information. I understand ACPI has more flexibility and I did

ACPI has a much stronger idea of what the system looks like which helps
it a lot here.

> Each device or platform having its specific property in DT is not scalable.
> Not sure if it is a generic problem. If it is, I would like to understand
> more details on such lack of ability for communtication between OS and
> firmware.

It seems like a generic issue from where I'm sitting.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH/RFC v4 2/4] regulator: fixed: add regulator_ops members for suspend/resume

[Sudeep Holla]

On Mon, Jun 29, 2020 at 06:26:51PM +0100, Mark Brown wrote:
> On Mon, Jun 29, 2020 at 05:42:07PM +0100, Sudeep Holla wrote:
> > On Mon, Jun 29, 2020 at 05:14:50PM +0100, Mark Brown wrote:
> > > On Mon, Jun 29, 2020 at 04:07:28PM +0100, Sudeep Holla wrote:
> > >
> > > > The specification states clearly:
> > > > "... all devices in the system must be in a state that is compatible
> > > > with entry into the system state. These preconditions are beyond the scope
> > > > of this specification and are therefore not described here."
> > > > "Prior to the call, the OS must disable all sources of wakeup other than
> > > > those it needs to support for its implementation of suspend to RAM."
>
> > > This gets a bit circular for a generic OS since the OS needs some way to
> > > figure out what it's supposed to do on a given platform - for example
> > > the OS may be happy to use wakeup sources that the firmware is just
> > > going to cut power on.
>
> > While I understand the sentiments here, PSCI is targeted to address CPU
> > power state management mainly and system states like suspend/reset and
> > poweroff which involves last CPU. This is one of the reason it is out of
> > the scope of the specification.
>
> Sure, but as soon as we start talking about the last CPU stuff we're
> inevitably talking about the system as a whole.
>
> > Here is my understanding. DT specifies all the wakeup sources. Linux
> > can configure those and user may choose to enable a subset of them is
> > wakeup. As stated in the spec and also based on what we already do in
> > the kernel, we disable all other wakeup sources.
>
> > The PSCI firmware can then either read those from the interrupt controller
> > or based on static platform understanding, must not disable those wakeup
> > sources.
>
> That bit about static platform understanding isn't super helpful for the
> OS, so long as the firmware might do that the OS is pretty much out of
> luck.
>

I don't disagree. That's one of the reason I wanted to gather requirement
few years back when PSCI system suspend was introduced. I couldn't
convince PSCI spec authors myself.

> > > > I see nothing has been fixed in the firmware too and we are still
> > > > discussing the same after 3 years 😄. Clearly we should start trusting
> > > > firmware and built capability to fix and replace it if there are bugs
> > > > just like kernel and stop hacking around in the kernel to deal with
> > > > just broken platform/psci firmware.
>
> > > This isn't just an issue of buggy firmware as far as I can see, it's
> > > also a lack of ability for the OS and firmware to communicate
> > > information about their intentions to each other.  As things stand you'd
> > > need to put static information in the DT.
>
> > It is easy for DT to get out of sync with firmware unless it is generated
> > by the same firmware. That's the reason why I am against such multiple
>
> The ability for things to get out of sync also concerns me as I said
> further back in the thread but I'm not sure we have much alternative,
> realistically we're going to need some facility to work around firmware
> that isn't ideal.
>

I understand and I agree. That's the main reason I want to understand this
better and provide a generic solution. The current pm_suspend_via_firmware
seem to have different intentions(at-least that's what I grasped quickly
reading through the document)

> > sources of information. I understand ACPI has more flexibility and I did
>
> ACPI has a much stronger idea of what the system looks like which helps
> it a lot here.
>

Yes, I wanted something similar initially but didn't get good response
both from community and PSCI spec authors.

> > Each device or platform having its specific property in DT is not scalable.
> > Not sure if it is a generic problem. If it is, I would like to understand
> > more details on such lack of ability for communtication between OS and
> > firmware.
>
> It seems like a generic issue from where I'm sitting.

I can't disagree with that. The description of the issue and the solution
in the thread is. We need make it more generic and provide generic solution,.
I already see 2-3 threads addressing this in isolation as specific issue.
Also we need to check with DT maintainers if there are fine with the idea
of binding to address this issue.

--
Regards,
Sudeep

RE: [PATCH/RFC v4 2/4] regulator: fixed: add regulator_ops members for suspend/resume

[Yoshihiro Shimoda]

Hi Mark,

> From: Mark Brown, Sent: Monday, June 29, 2020 9:58 PM
> 
> On Mon, Jun 29, 2020 at 02:42:26AM +0000, Yoshihiro Shimoda wrote:
> > > From: Mark Brown, Sent: Friday, June 26, 2020 11:39 PM
> 
> Copying in Sudeep for the feedback on firmware interfaces.

Thank you very much for the discussion about the firmware.

> > > According to the changelog this is all about reflecting changes in the
> > > system state done by firmware but there's no interaction with firmware
> > > here which means this will be at best fragile.  If we need to reflect
> > > changes in firmware configuration I'd expect there to be some
> > > interaction with firmware about how it is configured, or at least that
> > > the configuration would come from the same source.
> 
> > I should have described background of previous patch series though,
> > according to previous discussion [1] the firmware side (like PSCI) is
> > also fragile unfortunately... So, I thought using regulator-off-in-suspend
> > in a regulator was better.
> 
> > On other hand, Ulf is talking about either adding a property (perhaps like
> > regulator-off-in-suspend) into a regulator or just adding a new property
> > into MMC [2]. What do you think about Ulf' comment? I'm thinking
> > adding a new property "full-pwr-cycle-in-suspend" is the best solution.
> > This is because using a regulator property and reflecting a state of regulator without
> > firmware is fragile, as you said.
> 
> TBH I worry about a property drifting out of sync with the firmware on
> systems where the firmware can be updated.

I understood it.

>  Personally my default
> assumption would always be that we're going to loose power for anything
> except the RAM and whatever is needed for wake sources during suspend so
> I find the discussion a bit surprising but in any case that seems like a
> better option than trying to shoehorn things in the way the series here
> did.

Thank you for your comment! So, I'll make such a patch series later.

>  Like I said in my earlier replies if this is done through the
> regulator API I'd expect it to be via the suspend interface.

I don't intend to use any regulator API for this issue for now.

Best regards,
Yoshihiro Shimoda

> > [1]
> > https://lore.kernel.org/linux-renesas-soc/CAMuHMdXjU7N4oG89YsozGijMpjgKGN6ezw2qm6FeGX=JyRhsvg@mail.gmail.com/
> >
> > [2]
> > https://lore.kernel.org/linux-renesas-soc/CAPDyKFpiBU1D+a7zb+Ggm0_HZ+YR4=LXJZ5MPytXtT=uBEdjPA@mail.gmail.com/
> >
> > Best regards,
> > Yoshihiro Shimoda
> >

Re: [PATCH/RFC v4 0/4] treewide: add regulator condition on _mmc_suspend()

[Geert Uytterhoeven]

Hi Shimoda-san,

On Mon, Jun 29, 2020 at 1:49 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Mon, Jun 29, 2020 at 12:04 PM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > From: Geert Uytterhoeven, Sent: Friday, June 26, 2020 7:13 PM
> > > On Fri, Jun 26, 2020 at 11:32 AM Yoshihiro Shimoda
> > > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > > Note that v5.8-rc2 with r8a77951-salvator-xs seems to cause panic from
> > > > PCI driver when the system is suspended. So, I disabled the PCI
> > > > devices when I tested this patch series.
> > >
> > > Does this happen with current renesas-devel and renesas_defconfig?
> > > (it doesn't for me)
> >
> > Yes. I enabled PM_DEBUG and E1000E though.
> >
> > > Do you have any PCIe devices attached? (I haven't)
> >
> > Yes. (Intel Ethernet card is connected to the PCI slot.)
> >
> > < my environment >
> > - r8a77961-salvator-xs
> > - renesas-devel-2020-06-26-v5.8-rc2
> >  + renesas_defconfig + PM_DEBUG + E1000E
> > - initramfs
>
> Doesn't fail for me on R-Car H3 ES2.0, so it needs the presence of a
> PCIe card.  Unfortunately I haven't any (added to shopping wishlist).

[...]

> The failure mode looks like the PCI card is accessed while the PCI host
> bridge has been suspended.

Does "[PATCH v1] driver core: Fix suspend/resume order issue with
deferred probe"[1] help?

[1] https://lore.kernel.org/lkml/20200625032430.152447-1-saravanak@google.com/

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH/RFC v4 0/4] treewide: add regulator condition on _mmc_suspend()

[Yoshihiro Shimoda]

Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Tuesday, June 30, 2020 10:19 PM
> 
> Hi Shimoda-san,
> 
> On Mon, Jun 29, 2020 at 1:49 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Mon, Jun 29, 2020 at 12:04 PM Yoshihiro Shimoda
> > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > > From: Geert Uytterhoeven, Sent: Friday, June 26, 2020 7:13 PM
> > > > On Fri, Jun 26, 2020 at 11:32 AM Yoshihiro Shimoda
> > > > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > > > Note that v5.8-rc2 with r8a77951-salvator-xs seems to cause panic from
> > > > > PCI driver when the system is suspended. So, I disabled the PCI
> > > > > devices when I tested this patch series.
> > > >
> > > > Does this happen with current renesas-devel and renesas_defconfig?
> > > > (it doesn't for me)
> > >
> > > Yes. I enabled PM_DEBUG and E1000E though.
> > >
> > > > Do you have any PCIe devices attached? (I haven't)
> > >
> > > Yes. (Intel Ethernet card is connected to the PCI slot.)
> > >
> > > < my environment >
> > > - r8a77961-salvator-xs
> > > - renesas-devel-2020-06-26-v5.8-rc2
> > >  + renesas_defconfig + PM_DEBUG + E1000E
> > > - initramfs
> >
> > Doesn't fail for me on R-Car H3 ES2.0, so it needs the presence of a
> > PCIe card.  Unfortunately I haven't any (added to shopping wishlist).
> 
> [...]
> 
> > The failure mode looks like the PCI card is accessed while the PCI host
> > bridge has been suspended.
> 
> Does "[PATCH v1] driver core: Fix suspend/resume order issue with
> deferred probe"[1] help?
> 
> [1] https://lore.kernel.org/lkml/20200625032430.152447-1-saravanak@google.com/

Even if I applied this patch, the issue still happened unfortunately.


By the way, I'm guessing the issue is related to my environment which uses BSP's ATF.
According to the commit log of upstream ATF [1], PCIe hardware is possible to causes SError.
Unfortunately, I cannot try to update the firmware for some reasons now... I'll prepare
updated firmware somehow...

[1]
https://github.com/ARM-software/arm-trusted-firmware/commit/0969397f295621aa26b3d14b76dd397d22be58bf

Best regards,
Yoshihiro Shimoda

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Re: [PATCH/RFC v4 0/4] treewide: add regulator condition on _mmc_suspend()

[Geert Uytterhoeven]

Hi Shimoda-san,

On Fri, Jul 3, 2020 at 1:10 PM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> > From: Geert Uytterhoeven, Sent: Tuesday, June 30, 2020 10:19 PM
> > On Mon, Jun 29, 2020 at 1:49 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Mon, Jun 29, 2020 at 12:04 PM Yoshihiro Shimoda
> > > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > > > From: Geert Uytterhoeven, Sent: Friday, June 26, 2020 7:13 PM
> > > > > On Fri, Jun 26, 2020 at 11:32 AM Yoshihiro Shimoda
> > > > > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > > > > Note that v5.8-rc2 with r8a77951-salvator-xs seems to cause panic from
> > > > > > PCI driver when the system is suspended. So, I disabled the PCI
> > > > > > devices when I tested this patch series.
> > > > >
> > > > > Does this happen with current renesas-devel and renesas_defconfig?
> > > > > (it doesn't for me)
> > > >
> > > > Yes. I enabled PM_DEBUG and E1000E though.
> > > >
> > > > > Do you have any PCIe devices attached? (I haven't)
> > > >
> > > > Yes. (Intel Ethernet card is connected to the PCI slot.)
> > > >
> > > > < my environment >
> > > > - r8a77961-salvator-xs
> > > > - renesas-devel-2020-06-26-v5.8-rc2
> > > >  + renesas_defconfig + PM_DEBUG + E1000E
> > > > - initramfs
> > >
> > > Doesn't fail for me on R-Car H3 ES2.0, so it needs the presence of a
> > > PCIe card.  Unfortunately I haven't any (added to shopping wishlist).
> >
> > [...]
> >
> > > The failure mode looks like the PCI card is accessed while the PCI host
> > > bridge has been suspended.
> >
> > Does "[PATCH v1] driver core: Fix suspend/resume order issue with
> > deferred probe"[1] help?
> >
> > [1] https://lore.kernel.org/lkml/20200625032430.152447-1-saravanak@google.com/
>
> Even if I applied this patch, the issue still happened unfortunately.

OK.

I managed to reproduce it on the M3-W+ in Magnus' farm.

> By the way, I'm guessing the issue is related to my environment which uses BSP's ATF.
> According to the commit log of upstream ATF [1], PCIe hardware is possible to causes SError.
> Unfortunately, I cannot try to update the firmware for some reasons now... I'll prepare
> updated firmware somehow...

I don't think it's firmware-related.  The issue happens in the PCI
suspend_noirq callback, which is called during late system suspend.
With the following debug code added:

@@ -789,6 +789,8 @@ static int pci_pm_suspend_noirq(struct device *dev)
        struct pci_dev *pci_dev = to_pci_dev(dev);
        const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;

+dev_info(dev, "%s:%u: is_suspended = %u\n", __func__, __LINE__,
dev->power.is_suspended);
+dev_info(&pci_dev->bus->self->dev, "%s:%u: parent: is_suspended =
%u\n", __func__, __LINE__,
pci_dev->bus->self->dev.power.is_suspended);
        if (dev_pm_skip_suspend(dev))
                return 0;

I get:

     e1000e 0000:01:00.0: pci_pm_suspend_noirq:792: is_suspended = 1
    pci 0000:00:00.0: pci_pm_suspend_noirq:793: parent: is_suspended = 1

Oops, PCIe bridge already suspended.

    SError Interrupt on CPU0, code 0xbf000002 -- SError
    CPU: 0 PID: 172 Comm: kworker/u12:1 Not tainted
5.8.0-rc3-rcar3-initrd-00505-g6c843129e6faaf01-dirty #141
    Hardware name: Renesas Salvator-X 2nd version board based on r8a77961 (DT)
    Workqueue: events_unbound async_run_entry_fn
    pstate: 40000085 (nZcv daIf -PAN -UAO BTYPE=--)
    pc : rcar_pci_read_reg+0x10/0x28
    lr : rcar_pcie_config_access+0x148/0x178
    sp : ffffffc0112abb10
    x29: ffffffc0112abb10 x28: 0000000000000000
    x27: 0000000000000000 x26: 0000000000000000
    x25: 00000000000000cc x24: ffffff86b5f7e800
    x23: 0000000000000000 x22: 0000000000000000
    x21: 0000000000000000 x20: ffffffc0112abbe4
    x19: ffffff86b5f29c00 x18: 000000000000000a
    x17: 0000000000000000 x16: 0000000000000000
    x15: 000000000008dba6 x14: 0720072007200720
    x13: 0720072007200720 x12: 0000000000000001
    x11: 0000000000000000 x10: 0000000000001810
    x9 : ffffffc0112aba90 x8 : ffffff86b5f43a30
    x7 : ffffff86bef76bc0 x6 : ffffff86b5f423c0
    x5 : ffffffc0112abbe4 x4 : 00000000000000cc
    x3 : 0000000000000000 x2 : ffffffc011400018
    x1 : 0000000000000020 x0 : 00000000ffffffff
    Kernel panic - not syncing: Asynchronous SError Interrupt
    CPU: 0 PID: 172 Comm: kworker/u12:1 Not tainted
5.8.0-rc3-rcar3-initrd-00505-g6c843129e6faaf01-dirty #141
    Hardware name: Renesas Salvator-X 2nd version board based on r8a77961 (DT)
    Workqueue: events_unbound async_run_entry_fn
    Call trace:
     dump_backtrace+0x0/0x188
     show_stack+0x18/0x24
     dump_stack+0xb8/0xe8
     panic+0x158/0x33c
     nmi_panic+0x60/0x7c
     arm64_serror_panic+0x68/0x74
     do_serror+0xa8/0x144
     el1_error+0x84/0x100
     rcar_pci_read_reg+0x10/0x28
     rcar_pcie_read_conf+0x3c/0x9c
     pci_bus_read_config_word+0x80/0xd8
     pci_read_config_word+0x3c/0x48
     pci_raw_set_power_state+0x1a4/0x284
     pci_set_power_state+0xa8/0xd4
     pci_prepare_to_sleep+0x58/0x90
     pci_pm_suspend_noirq+0x280/0x284
     dpm_run_callback.isra.17+0x50/0xb8
     __device_suspend_noirq+0x84/0x188
     async_suspend_noirq+0x2c/0x7c
     async_run_entry_fn+0x50/0x12c
     process_one_work+0x180/0x1f0
     worker_thread+0x1d0/0x268
     kthread+0xdc/0xec
     ret_from_fork+0x10/0x18
    SMP: stopping secondary CPUs
    Kernel Offset: disabled
    CPU features: 0x000022,20006004
    Memory Limit: none

Anyone who can reproduce this on a different board, also on R-Car Gen2
or even R-Car H1?

    Intel E1000E card with CONFIG_E1000E=y

    echo 0 > /sys/module/printk/parameters/console_suspend
    echo mem > /sys/power/state

Why haven't we seen this before?
I can reproduce the issue on v5.5 (first version that supported M3-W+,
but needs backported DTS for PCIe support) and later.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

E1000 s2idle crash (was: Re: [PATCH/RFC v4 0/4] treewide: add regulator condition on _mmc_suspend())

[Geert Uytterhoeven]

Hi Shimoda-san,

On Mon, Jul 6, 2020 at 1:14 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Fri, Jul 3, 2020 at 1:10 PM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > From: Geert Uytterhoeven, Sent: Tuesday, June 30, 2020 10:19 PM
> > > On Mon, Jun 29, 2020 at 1:49 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > On Mon, Jun 29, 2020 at 12:04 PM Yoshihiro Shimoda
> > > > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > > > > From: Geert Uytterhoeven, Sent: Friday, June 26, 2020 7:13 PM
> > > > > > On Fri, Jun 26, 2020 at 11:32 AM Yoshihiro Shimoda
> > > > > > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > > > > > Note that v5.8-rc2 with r8a77951-salvator-xs seems to cause panic from
> > > > > > > PCI driver when the system is suspended. So, I disabled the PCI
> > > > > > > devices when I tested this patch series.
> > > > > >
> > > > > > Does this happen with current renesas-devel and renesas_defconfig?
> > > > > > (it doesn't for me)
> > > > >
> > > > > Yes. I enabled PM_DEBUG and E1000E though.
> > > > >
> > > > > > Do you have any PCIe devices attached? (I haven't)
> > > > >
> > > > > Yes. (Intel Ethernet card is connected to the PCI slot.)
> > > > >
> > > > > < my environment >
> > > > > - r8a77961-salvator-xs
> > > > > - renesas-devel-2020-06-26-v5.8-rc2
> > > > >  + renesas_defconfig + PM_DEBUG + E1000E
> > > > > - initramfs
> > > >
> > > > Doesn't fail for me on R-Car H3 ES2.0, so it needs the presence of a
> > > > PCIe card.  Unfortunately I haven't any (added to shopping wishlist).

"Intel Corporation 82574L Gigabit Network Connection" arrived and installed
on local Salvator-X with M3-W.

> > >
> > > [...]
> > >
> > > > The failure mode looks like the PCI card is accessed while the PCI host
> > > > bridge has been suspended.
> > >
> > > Does "[PATCH v1] driver core: Fix suspend/resume order issue with
> > > deferred probe"[1] help?
> > >
> > > [1] https://lore.kernel.org/lkml/20200625032430.152447-1-saravanak@google.com/
> >
> > Even if I applied this patch, the issue still happened unfortunately.
>
> OK.
>
> I managed to reproduce it on the M3-W+ in Magnus' farm.

And on my local M3-W.

> > By the way, I'm guessing the issue is related to my environment which uses BSP's ATF.
> > According to the commit log of upstream ATF [1], PCIe hardware is possible to causes SError.
> > Unfortunately, I cannot try to update the firmware for some reasons now... I'll prepare
> > updated firmware somehow...
>
> I don't think it's firmware-related.  The issue happens in the PCI
> suspend_noirq callback, which is called during late system suspend.

You were right. It turns out the ATF on my M3-W board was two weeks too
old to have commit 0969397f295621aa ("rcar_gen3: plat: Prevent PCIe hang
during L1X config access"). Updating all firmware components to today's
versions fixed that, and both s2idle and s2ram now work fine.

I assume the same would be true for M3-W+, so case closed (for R-Car Gen3)?

> Anyone who can reproduce this on a different board, also on R-Car Gen2
> or even R-Car H1?
>
>     Intel E1000E card with CONFIG_E1000E=y
>
>     echo 0 > /sys/module/printk/parameters/console_suspend
>     echo mem > /sys/power/state

I moved the E1000E card to an R-Car Gen2 board (r8a7791/koelsch), and
s2idle crashes in a similar way:

    Unhandled fault: asynchronous external abort (0x1211) at 0x00000000
    pgd = ceadf1f8
    [00000000] *pgd=80000040004003, *pmd=00000000
    Internal error: : 1211 [#1] SMP ARM
    Modules linked in:
    CPU: 0 PID: 124 Comm: kworker/u4:6 Not tainted
5.8.0-koelsch-00539-gce07c9ba6e9f601c #867
    Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
    Workqueue: events_unbound async_run_entry_fn
    PC is at rcar_pcie_config_access+0x10c/0x13c
    LR is at rcar_pcie_config_access+0x10c/0x13c
    pc : [<c04a4ab4>]    lr : [<c04a4ab4>]    psr: 60000093
    sp : e67b3e00  ip : 00000000  fp : 00000000
    r10: 00000000  r9 : 00000000  r8 : e7369800
    r7 : 00000000  r6 : e67b3e40  r5 : e7369640  r4 : 000000cc
    r3 : f0900000  r2 : f0900018  r1 : f0900020  r0 : 00000000
    Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment user
    Control: 30c5387d  Table: 648fe480  DAC: fffffffd
    Process kworker/u4:6 (pid: 124, stack limit = 0x0dcce627)
    Stack: (0xe67b3e00 to 0xe67b4000)
    ...
    [<c04a4ab4>] (rcar_pcie_config_access) from [<c04a4be0>]
(rcar_pcie_read_conf+0x28/0x80)
    [<c04a4be0>] (rcar_pcie_read_conf) from [<c048a4e0>]
(pci_bus_read_config_word+0x68/0xa8)
    [<c048a4e0>] (pci_bus_read_config_word) from [<c0490030>]
(pci_raw_set_power_state+0x18c/0x270)
    [<c0490030>] (pci_raw_set_power_state) from [<c0492e20>]
(pci_set_power_state+0x98/0xcc)
    [<c0492e20>] (pci_set_power_state) from [<c0492ea0>]
(pci_prepare_to_sleep+0x4c/0x6c)
    [<c0492ea0>] (pci_prepare_to_sleep) from [<c0496c84>]
(pci_pm_suspend_noirq+0x228/0x244)
    [<c0496c84>] (pci_pm_suspend_noirq) from [<c0509d88>]
(dpm_run_callback.part.5+0x44/0xac)
    [<c0509d88>] (dpm_run_callback.part.5) from [<c050b38c>]
(__device_suspend_noirq+0x74/0x1a4)

> Why haven't we seen this before?
> I can reproduce the issue on v5.5 (first version that supported M3-W+,
> but needs backported DTS for PCIe support) and later.

On Koelsch, I could easily reproduce this on v4.10 and later[1].

As this time no firmware is involved, I guess Linux itself needs to
become aware of this issue, and handle it in a similar way like ATF
on arm64[2]?

[1] Using shmobile_defconfig + CONFIG_NET_VENDOR_INTEL=y + CONFIG_E1000E=y.
    v4.10 needs CONFIG_PCI_MSI=y + CONFIG_PCIE_RCAR=y, too.
    Older kernels are not compatible with my Debian (systemd!) nfsroot
    userland.

[2] https://github.com/ARM-software/arm-trusted-firmware/commit/0969397f295621aa26b3d14b76dd397d22be58bf

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: E1000 s2idle crash (was: Re: [PATCH/RFC v4 0/4] treewide: add regulator condition on _mmc_suspend())

[Yoshihiro Shimoda]

Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Tuesday, August 11, 2020 10:50 PM
> 
> Hi Shimoda-san,
> 
> On Mon, Jul 6, 2020 at 1:14 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Fri, Jul 3, 2020 at 1:10 PM Yoshihiro Shimoda
> > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > > From: Geert Uytterhoeven, Sent: Tuesday, June 30, 2020 10:19 PM
> > > > On Mon, Jun 29, 2020 at 1:49 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > On Mon, Jun 29, 2020 at 12:04 PM Yoshihiro Shimoda
> > > > > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > > > > > From: Geert Uytterhoeven, Sent: Friday, June 26, 2020 7:13 PM
> > > > > > > On Fri, Jun 26, 2020 at 11:32 AM Yoshihiro Shimoda
> > > > > > > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > > > > > > Note that v5.8-rc2 with r8a77951-salvator-xs seems to cause panic from
> > > > > > > > PCI driver when the system is suspended. So, I disabled the PCI
> > > > > > > > devices when I tested this patch series.
> > > > > > >
> > > > > > > Does this happen with current renesas-devel and renesas_defconfig?
> > > > > > > (it doesn't for me)
> > > > > >
> > > > > > Yes. I enabled PM_DEBUG and E1000E though.
> > > > > >
> > > > > > > Do you have any PCIe devices attached? (I haven't)
> > > > > >
> > > > > > Yes. (Intel Ethernet card is connected to the PCI slot.)
> > > > > >
> > > > > > < my environment >
> > > > > > - r8a77961-salvator-xs
> > > > > > - renesas-devel-2020-06-26-v5.8-rc2
> > > > > >  + renesas_defconfig + PM_DEBUG + E1000E
> > > > > > - initramfs
> > > > >
> > > > > Doesn't fail for me on R-Car H3 ES2.0, so it needs the presence of a
> > > > > PCIe card.  Unfortunately I haven't any (added to shopping wishlist).
> 
> "Intel Corporation 82574L Gigabit Network Connection" arrived and installed
> on local Salvator-X with M3-W.
> 
> > > >
> > > > [...]
> > > >
> > > > > The failure mode looks like the PCI card is accessed while the PCI host
> > > > > bridge has been suspended.
> > > >
> > > > Does "[PATCH v1] driver core: Fix suspend/resume order issue with
> > > > deferred probe"[1] help?
> > > >
> > > > [1] https://lore.kernel.org/lkml/20200625032430.152447-1-saravanak@google.com/
> > >
> > > Even if I applied this patch, the issue still happened unfortunately.
> >
> > OK.
> >
> > I managed to reproduce it on the M3-W+ in Magnus' farm.
> 
> And on my local M3-W.

Thank you for reproducing it on M3-W.

> > > By the way, I'm guessing the issue is related to my environment which uses BSP's ATF.
> > > According to the commit log of upstream ATF [1], PCIe hardware is possible to causes SError.
> > > Unfortunately, I cannot try to update the firmware for some reasons now... I'll prepare
> > > updated firmware somehow...
> >
> > I don't think it's firmware-related.  The issue happens in the PCI
> > suspend_noirq callback, which is called during late system suspend.
> 
> You were right. It turns out the ATF on my M3-W board was two weeks too
> old to have commit 0969397f295621aa ("rcar_gen3: plat: Prevent PCIe hang
> during L1X config access"). Updating all firmware components to today's
> versions fixed that, and both s2idle and s2ram now work fine.

I got it.

> I assume the same would be true for M3-W+, so case closed (for R-Car Gen3)?

I think so.

> > Anyone who can reproduce this on a different board, also on R-Car Gen2
> > or even R-Car H1?
> >
> >     Intel E1000E card with CONFIG_E1000E=y
> >
> >     echo 0 > /sys/module/printk/parameters/console_suspend
> >     echo mem > /sys/power/state
> 
> I moved the E1000E card to an R-Car Gen2 board (r8a7791/koelsch), and
> s2idle crashes in a similar way:
> 
>     Unhandled fault: asynchronous external abort (0x1211) at 0x00000000
>     pgd = ceadf1f8
>     [00000000] *pgd=80000040004003, *pmd=00000000
>     Internal error: : 1211 [#1] SMP ARM
>     Modules linked in:
>     CPU: 0 PID: 124 Comm: kworker/u4:6 Not tainted
> 5.8.0-koelsch-00539-gce07c9ba6e9f601c #867
>     Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
>     Workqueue: events_unbound async_run_entry_fn
>     PC is at rcar_pcie_config_access+0x10c/0x13c
>     LR is at rcar_pcie_config_access+0x10c/0x13c
>     pc : [<c04a4ab4>]    lr : [<c04a4ab4>]    psr: 60000093
>     sp : e67b3e00  ip : 00000000  fp : 00000000
>     r10: 00000000  r9 : 00000000  r8 : e7369800
>     r7 : 00000000  r6 : e67b3e40  r5 : e7369640  r4 : 000000cc
>     r3 : f0900000  r2 : f0900018  r1 : f0900020  r0 : 00000000
>     Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment user
>     Control: 30c5387d  Table: 648fe480  DAC: fffffffd
>     Process kworker/u4:6 (pid: 124, stack limit = 0x0dcce627)
>     Stack: (0xe67b3e00 to 0xe67b4000)
>     ...
>     [<c04a4ab4>] (rcar_pcie_config_access) from [<c04a4be0>]
> (rcar_pcie_read_conf+0x28/0x80)
>     [<c04a4be0>] (rcar_pcie_read_conf) from [<c048a4e0>]
> (pci_bus_read_config_word+0x68/0xa8)
>     [<c048a4e0>] (pci_bus_read_config_word) from [<c0490030>]
> (pci_raw_set_power_state+0x18c/0x270)
>     [<c0490030>] (pci_raw_set_power_state) from [<c0492e20>]
> (pci_set_power_state+0x98/0xcc)
>     [<c0492e20>] (pci_set_power_state) from [<c0492ea0>]
> (pci_prepare_to_sleep+0x4c/0x6c)
>     [<c0492ea0>] (pci_prepare_to_sleep) from [<c0496c84>]
> (pci_pm_suspend_noirq+0x228/0x244)
>     [<c0496c84>] (pci_pm_suspend_noirq) from [<c0509d88>]
> (dpm_run_callback.part.5+0x44/0xac)
>     [<c0509d88>] (dpm_run_callback.part.5) from [<c050b38c>]
> (__device_suspend_noirq+0x74/0x1a4)
> 
> > Why haven't we seen this before?
> > I can reproduce the issue on v5.5 (first version that supported M3-W+,
> > but needs backported DTS for PCIe support) and later.
> 
> On Koelsch, I could easily reproduce this on v4.10 and later[1].
> 
> As this time no firmware is involved, I guess Linux itself needs to
> become aware of this issue, and handle it in a similar way like ATF
> on arm64[2]?

I agree. But, I'm not sure how to implement a similar way on arm32 though...
Maybe, we should unbind a PCI device before R-Car Gen2 is suspended?

Best regards,
Yoshihiro Shimoda

Re: E1000 s2idle crash (was: Re: [PATCH/RFC v4 0/4] treewide: add regulator condition on _mmc_suspend())

[Geert Uytterhoeven]

Hi Shimoda-san,

On Mon, Aug 17, 2020 at 2:00 PM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> > From: Geert Uytterhoeven, Sent: Tuesday, August 11, 2020 10:50 PM
> > On Mon, Jul 6, 2020 at 1:14 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > I moved the E1000E card to an R-Car Gen2 board (r8a7791/koelsch), and
> > s2idle crashes in a similar way:
> >
> >     Unhandled fault: asynchronous external abort (0x1211) at 0x00000000
> >     pgd = ceadf1f8
> >     [00000000] *pgd=80000040004003, *pmd=00000000
> >     Internal error: : 1211 [#1] SMP ARM
> >     Modules linked in:
> >     CPU: 0 PID: 124 Comm: kworker/u4:6 Not tainted
> > 5.8.0-koelsch-00539-gce07c9ba6e9f601c #867
> >     Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> >     Workqueue: events_unbound async_run_entry_fn
> >     PC is at rcar_pcie_config_access+0x10c/0x13c
> >     LR is at rcar_pcie_config_access+0x10c/0x13c
> >     pc : [<c04a4ab4>]    lr : [<c04a4ab4>]    psr: 60000093
> >     sp : e67b3e00  ip : 00000000  fp : 00000000
> >     r10: 00000000  r9 : 00000000  r8 : e7369800
> >     r7 : 00000000  r6 : e67b3e40  r5 : e7369640  r4 : 000000cc
> >     r3 : f0900000  r2 : f0900018  r1 : f0900020  r0 : 00000000
> >     Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment user
> >     Control: 30c5387d  Table: 648fe480  DAC: fffffffd
> >     Process kworker/u4:6 (pid: 124, stack limit = 0x0dcce627)
> >     Stack: (0xe67b3e00 to 0xe67b4000)
> >     ...
> >     [<c04a4ab4>] (rcar_pcie_config_access) from [<c04a4be0>]
> > (rcar_pcie_read_conf+0x28/0x80)
> >     [<c04a4be0>] (rcar_pcie_read_conf) from [<c048a4e0>]
> > (pci_bus_read_config_word+0x68/0xa8)
> >     [<c048a4e0>] (pci_bus_read_config_word) from [<c0490030>]
> > (pci_raw_set_power_state+0x18c/0x270)
> >     [<c0490030>] (pci_raw_set_power_state) from [<c0492e20>]
> > (pci_set_power_state+0x98/0xcc)
> >     [<c0492e20>] (pci_set_power_state) from [<c0492ea0>]
> > (pci_prepare_to_sleep+0x4c/0x6c)
> >     [<c0492ea0>] (pci_prepare_to_sleep) from [<c0496c84>]
> > (pci_pm_suspend_noirq+0x228/0x244)
> >     [<c0496c84>] (pci_pm_suspend_noirq) from [<c0509d88>]
> > (dpm_run_callback.part.5+0x44/0xac)
> >     [<c0509d88>] (dpm_run_callback.part.5) from [<c050b38c>]
> > (__device_suspend_noirq+0x74/0x1a4)
> >
> > > Why haven't we seen this before?
> > > I can reproduce the issue on v5.5 (first version that supported M3-W+,
> > > but needs backported DTS for PCIe support) and later.
> >
> > On Koelsch, I could easily reproduce this on v4.10 and later[1].
> >
> > As this time no firmware is involved, I guess Linux itself needs to
> > become aware of this issue, and handle it in a similar way like ATF
> > on arm64[2]?
>
> I agree. But, I'm not sure how to implement a similar way on arm32 though...
> Maybe, we should unbind a PCI device before R-Car Gen2 is suspended?

Unbinding PCI devices before suspending the system means that e.g.
Wake-on-LAN will no longer work.  Unless WoL-enabled devices are
not unbound.

However, it seems Wake-on-LAN is enabled by default on E1000E.
Hence even if WoL is enabled, the PCIe device will send an L1_Enter_PM DLLP
request, triggering the issue.



Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: E1000 s2idle crash (was: Re: [PATCH/RFC v4 0/4] treewide: add regulator condition on _mmc_suspend())

[Geert Uytterhoeven]

Hi Shimoda-san,

On Thu, Aug 27, 2020 at 11:15 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Mon, Aug 17, 2020 at 2:00 PM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > From: Geert Uytterhoeven, Sent: Tuesday, August 11, 2020 10:50 PM
> > > On Mon, Jul 6, 2020 at 1:14 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > I moved the E1000E card to an R-Car Gen2 board (r8a7791/koelsch), and
> > > s2idle crashes in a similar way:
> > >
> > >     Unhandled fault: asynchronous external abort (0x1211) at 0x00000000
> > >     pgd = ceadf1f8
> > >     [00000000] *pgd=80000040004003, *pmd=00000000
> > >     Internal error: : 1211 [#1] SMP ARM
> > >     Modules linked in:
> > >     CPU: 0 PID: 124 Comm: kworker/u4:6 Not tainted
> > > 5.8.0-koelsch-00539-gce07c9ba6e9f601c #867
> > >     Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> > >     Workqueue: events_unbound async_run_entry_fn
> > >     PC is at rcar_pcie_config_access+0x10c/0x13c
> > >     LR is at rcar_pcie_config_access+0x10c/0x13c
> > >     pc : [<c04a4ab4>]    lr : [<c04a4ab4>]    psr: 60000093
> > >     sp : e67b3e00  ip : 00000000  fp : 00000000
> > >     r10: 00000000  r9 : 00000000  r8 : e7369800
> > >     r7 : 00000000  r6 : e67b3e40  r5 : e7369640  r4 : 000000cc
> > >     r3 : f0900000  r2 : f0900018  r1 : f0900020  r0 : 00000000
> > >     Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment user
> > >     Control: 30c5387d  Table: 648fe480  DAC: fffffffd
> > >     Process kworker/u4:6 (pid: 124, stack limit = 0x0dcce627)
> > >     Stack: (0xe67b3e00 to 0xe67b4000)
> > >     ...
> > >     [<c04a4ab4>] (rcar_pcie_config_access) from [<c04a4be0>]
> > > (rcar_pcie_read_conf+0x28/0x80)
> > >     [<c04a4be0>] (rcar_pcie_read_conf) from [<c048a4e0>]
> > > (pci_bus_read_config_word+0x68/0xa8)
> > >     [<c048a4e0>] (pci_bus_read_config_word) from [<c0490030>]
> > > (pci_raw_set_power_state+0x18c/0x270)
> > >     [<c0490030>] (pci_raw_set_power_state) from [<c0492e20>]
> > > (pci_set_power_state+0x98/0xcc)
> > >     [<c0492e20>] (pci_set_power_state) from [<c0492ea0>]
> > > (pci_prepare_to_sleep+0x4c/0x6c)
> > >     [<c0492ea0>] (pci_prepare_to_sleep) from [<c0496c84>]
> > > (pci_pm_suspend_noirq+0x228/0x244)
> > >     [<c0496c84>] (pci_pm_suspend_noirq) from [<c0509d88>]
> > > (dpm_run_callback.part.5+0x44/0xac)
> > >     [<c0509d88>] (dpm_run_callback.part.5) from [<c050b38c>]
> > > (__device_suspend_noirq+0x74/0x1a4)
> > >
> > > > Why haven't we seen this before?
> > > > I can reproduce the issue on v5.5 (first version that supported M3-W+,
> > > > but needs backported DTS for PCIe support) and later.
> > >
> > > On Koelsch, I could easily reproduce this on v4.10 and later[1].
> > >
> > > As this time no firmware is involved, I guess Linux itself needs to
> > > become aware of this issue, and handle it in a similar way like ATF
> > > on arm64[2]?
> >
> > I agree. But, I'm not sure how to implement a similar way on arm32 though...
> > Maybe, we should unbind a PCI device before R-Car Gen2 is suspended?
>
> Unbinding PCI devices before suspending the system means that e.g.
> Wake-on-LAN will no longer work.  Unless WoL-enabled devices are
> not unbound.
>
> However, it seems Wake-on-LAN is enabled by default on E1000E.
> Hence even if WoL is enabled, the PCIe device will send an L1_Enter_PM DLLP
> request, triggering the issue.

Perhaps we can hook something into PCI error handling (EEH)?
But that seems to be used on PPC only.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds