* [PATCH v3 0/5] x86: atom-punit/-pmc s2idle device state checks
@ 2024-01-08 13:02 Hans de Goede
2024-01-08 13:02 ` [PATCH v3 1/5] clk: x86: Move clk-pmc-atom register defines to include/linux/platform_data/x86/pmc_atom.h Hans de Goede
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Hans de Goede @ 2024-01-08 13:02 UTC (permalink / raw
To: Johannes Stezenbach, Takashi Iwai, Ilpo Järvinen,
Andy Shevchenko, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, H . Peter Anvin, Michael Turquette, Stephen Boyd
Cc: Hans de Goede, platform-driver-x86, x86, linux-clk
Hi All,
These patches are an upstream submission of a patch titled:
"Intel Atom suspend: add debug check for S0ix blockers"
Which I have been carrying in my personal kernel tree for years now.
This code originally comes from the latte-l-oss branch of:
https://github.com/MiCode/Xiaomi_Kernel_OpenSource
And has been posted on upstream mailinglists before by
Johannes Stezenbach, whose authorship I have kept for
the 2 base patches and has been reposted by Takashi Iwai
and at one point in time I picked this up from Takashi's
reposting as can be seen from the S-o-b lines. Unfortunately
I cannot find the original postings, so I have no link to
those.
The original version of this added some ugly hooks into
the intel_idle driver which I presume is why these patches
never got anywhere upstream.
With the new acpi_s2idle_dev_ops and acpi_register_lps0_dev()
functionality this functionality can now be implemented cleanly
and that is what this patch-series does.
clk and x86/tip maintainers, it is probably the cleanest if this
entire series is merged through the pdx86 tree (*). Can we have
your ack for merging patch 1/5 resp. 5/5 through the pdx86 tree ?
Changes in v3:
- Reword commit message of patch 3/5 and 5/5
- Drop confusing /* Low Part */ and /* High Part */ comments in pmc_atom.c
- Add punit_s2idle_check_[un]register() helper functions
Changes in v2:
- Move CLK reg defines to include/linux/platform_data/x86/pmc_atom.h
- Drop duplicated "pmc_atom: " prefix from logged messages
Regards,
Hans
*) Andy recently mentioned that it might be a good idea to move
some of the arch/x86/platform code to drivers/platform/x86,
arch/x86/platform/atom/punit_atom_debug.c which is a completely
standalone driver definitly is a good candidate for this
Hans de Goede (3):
clk: x86: Move clk-pmc-atom register defines to
include/linux/platform_data/x86/pmc_atom.h
platform/x86: pmc_atom: Annotate d3_sts register bit defines
platform/x86: pmc_atom: Check state of PMC clocks on s2idle
Johannes Stezenbach (2):
platform/x86: pmc_atom: Check state of PMC managed devices on s2idle
x86/platform/atom: Check state of Punit managed devices on s2idle
arch/x86/platform/atom/punit_atom_debug.c | 54 +++++++++++++++-
drivers/clk/x86/clk-pmc-atom.c | 13 +---
drivers/platform/x86/pmc_atom.c | 75 ++++++++++++++++++++++
include/linux/platform_data/x86/pmc_atom.h | 25 ++++++--
4 files changed, 148 insertions(+), 19 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 1/5] clk: x86: Move clk-pmc-atom register defines to include/linux/platform_data/x86/pmc_atom.h
2024-01-08 13:02 [PATCH v3 0/5] x86: atom-punit/-pmc s2idle device state checks Hans de Goede
@ 2024-01-08 13:02 ` Hans de Goede
2024-01-08 23:33 ` Stephen Boyd
2024-01-08 13:02 ` [PATCH v3 2/5] platform/x86: pmc_atom: Annotate d3_sts register bit defines Hans de Goede
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Hans de Goede @ 2024-01-08 13:02 UTC (permalink / raw
To: Johannes Stezenbach, Takashi Iwai, Ilpo Järvinen,
Andy Shevchenko, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, H . Peter Anvin, Michael Turquette, Stephen Boyd
Cc: Hans de Goede, platform-driver-x86, x86, linux-clk
Move the register defines for the Atom (Bay Trail, Cherry Trail) PMC
clocks to include/linux/platform_data/x86/pmc_atom.h.
This is a preparation patch to extend the S0i3 readiness checks
in drivers/platform/x86/pmc_atom.c with checking that the PMC
clocks are off on suspend entry.
Note these are added to include/linux/platform_data/x86/pmc_atom.h rather
then to include/linux/platform_data/x86/clk-pmc-atom.h because the former
already has all the other Atom PMC register defines.
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- This is a new patch in v2 of this series
---
drivers/clk/x86/clk-pmc-atom.c | 13 +------------
include/linux/platform_data/x86/pmc_atom.h | 13 +++++++++++++
2 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/clk/x86/clk-pmc-atom.c b/drivers/clk/x86/clk-pmc-atom.c
index 2974dd0ec6f4..5ec9255e33fa 100644
--- a/drivers/clk/x86/clk-pmc-atom.c
+++ b/drivers/clk/x86/clk-pmc-atom.c
@@ -11,23 +11,12 @@
#include <linux/err.h>
#include <linux/io.h>
#include <linux/platform_data/x86/clk-pmc-atom.h>
+#include <linux/platform_data/x86/pmc_atom.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
#define PLT_CLK_NAME_BASE "pmc_plt_clk"
-#define PMC_CLK_CTL_OFFSET 0x60
-#define PMC_CLK_CTL_SIZE 4
-#define PMC_CLK_NUM 6
-#define PMC_CLK_CTL_GATED_ON_D3 0x0
-#define PMC_CLK_CTL_FORCE_ON 0x1
-#define PMC_CLK_CTL_FORCE_OFF 0x2
-#define PMC_CLK_CTL_RESERVED 0x3
-#define PMC_MASK_CLK_CTL GENMASK(1, 0)
-#define PMC_MASK_CLK_FREQ BIT(2)
-#define PMC_CLK_FREQ_XTAL (0 << 2) /* 25 MHz */
-#define PMC_CLK_FREQ_PLL (1 << 2) /* 19.2 MHz */
-
struct clk_plt_fixed {
struct clk_hw *clk;
struct clk_lookup *lookup;
diff --git a/include/linux/platform_data/x86/pmc_atom.h b/include/linux/platform_data/x86/pmc_atom.h
index b8a701c77fd0..557622ef0390 100644
--- a/include/linux/platform_data/x86/pmc_atom.h
+++ b/include/linux/platform_data/x86/pmc_atom.h
@@ -43,6 +43,19 @@
BIT_ORED_DEDICATED_IRQ_GPSC | \
BIT_SHARED_IRQ_GPSS)
+/* External clk generator settings */
+#define PMC_CLK_CTL_OFFSET 0x60
+#define PMC_CLK_CTL_SIZE 4
+#define PMC_CLK_NUM 6
+#define PMC_CLK_CTL_GATED_ON_D3 0x0
+#define PMC_CLK_CTL_FORCE_ON 0x1
+#define PMC_CLK_CTL_FORCE_OFF 0x2
+#define PMC_CLK_CTL_RESERVED 0x3
+#define PMC_MASK_CLK_CTL GENMASK(1, 0)
+#define PMC_MASK_CLK_FREQ BIT(2)
+#define PMC_CLK_FREQ_XTAL (0 << 2) /* 25 MHz */
+#define PMC_CLK_FREQ_PLL (1 << 2) /* 19.2 MHz */
+
/* The timers accumulate time spent in sleep state */
#define PMC_S0IR_TMR 0x80
#define PMC_S0I1_TMR 0x84
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 2/5] platform/x86: pmc_atom: Annotate d3_sts register bit defines
2024-01-08 13:02 [PATCH v3 0/5] x86: atom-punit/-pmc s2idle device state checks Hans de Goede
2024-01-08 13:02 ` [PATCH v3 1/5] clk: x86: Move clk-pmc-atom register defines to include/linux/platform_data/x86/pmc_atom.h Hans de Goede
@ 2024-01-08 13:02 ` Hans de Goede
2024-01-08 13:02 ` [PATCH v3 3/5] platform/x86: pmc_atom: Check state of PMC managed devices on s2idle Hans de Goede
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Hans de Goede @ 2024-01-08 13:02 UTC (permalink / raw
To: Johannes Stezenbach, Takashi Iwai, Ilpo Järvinen,
Andy Shevchenko, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, H . Peter Anvin, Michael Turquette, Stephen Boyd
Cc: Hans de Goede, platform-driver-x86, x86, linux-clk
The include/linux/platform_data/x86/pmc_atom.h d3_sts register bit defines
are named after how these bits are used on Bay Trail devices.
On Cherry Trail (CHT) devices some of these bits have a different meaning
according to the datasheet.
At a comment to the defines for bits which have a different meaning
on Cherry Trail devices.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
include/linux/platform_data/x86/pmc_atom.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/linux/platform_data/x86/pmc_atom.h b/include/linux/platform_data/x86/pmc_atom.h
index 557622ef0390..161e4bc1c9ee 100644
--- a/include/linux/platform_data/x86/pmc_atom.h
+++ b/include/linux/platform_data/x86/pmc_atom.h
@@ -117,14 +117,14 @@
#define BIT_SCC_SDIO BIT(9)
#define BIT_SCC_SDCARD BIT(10)
#define BIT_SCC_MIPI BIT(11)
-#define BIT_HDA BIT(12)
+#define BIT_HDA BIT(12) /* CHT datasheet: reserved */
#define BIT_LPE BIT(13)
#define BIT_OTG BIT(14)
-#define BIT_USH BIT(15)
-#define BIT_GBE BIT(16)
-#define BIT_SATA BIT(17)
-#define BIT_USB_EHCI BIT(18)
-#define BIT_SEC BIT(19)
+#define BIT_USH BIT(15) /* CHT datasheet: reserved */
+#define BIT_GBE BIT(16) /* CHT datasheet: reserved */
+#define BIT_SATA BIT(17) /* CHT datasheet: reserved */
+#define BIT_USB_EHCI BIT(18) /* CHT datasheet: XHCI! */
+#define BIT_SEC BIT(19) /* BYT datasheet: reserved */
#define BIT_PCIE_PORT0 BIT(20)
#define BIT_PCIE_PORT1 BIT(21)
#define BIT_PCIE_PORT2 BIT(22)
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 3/5] platform/x86: pmc_atom: Check state of PMC managed devices on s2idle
2024-01-08 13:02 [PATCH v3 0/5] x86: atom-punit/-pmc s2idle device state checks Hans de Goede
2024-01-08 13:02 ` [PATCH v3 1/5] clk: x86: Move clk-pmc-atom register defines to include/linux/platform_data/x86/pmc_atom.h Hans de Goede
2024-01-08 13:02 ` [PATCH v3 2/5] platform/x86: pmc_atom: Annotate d3_sts register bit defines Hans de Goede
@ 2024-01-08 13:02 ` Hans de Goede
2024-01-08 13:12 ` Ilpo Järvinen
2024-01-08 13:02 ` [PATCH v3 4/5] platform/x86: pmc_atom: Check state of PMC clocks " Hans de Goede
2024-01-08 13:02 ` [PATCH v3 5/5] x86/platform/atom: Check state of Punit managed devices " Hans de Goede
4 siblings, 1 reply; 9+ messages in thread
From: Hans de Goede @ 2024-01-08 13:02 UTC (permalink / raw
To: Johannes Stezenbach, Takashi Iwai, Ilpo Järvinen,
Andy Shevchenko, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, H . Peter Anvin, Michael Turquette, Stephen Boyd
Cc: Hans de Goede, platform-driver-x86, x86, linux-clk
From: Johannes Stezenbach <js@sig21.net>
For the Bay Trail or Cherry Trail SoC to enter the S0i3 power-level
at s2idle suspend requires most of the hw-blocks / devices in the SoC
to be in D3 when entering s2idle suspend.
If some devices are not in D3 then the SoC will stay in a higher
power state, consuming much more power from the battery then in S0i3.
Use the new acpi_s2idle_dev_ops and acpi_register_lps0_dev()
functionality to register a new s2idle check function which checks that
all hardware blocks in the South complex (controlled by the PMC)
are in a state that allows the SoC to enter S0i3 and prints an error
message for any device in D0.
Some blocks are not used on lower-featured versions of the SoC and
these blocks will always report being in D0 on SoCs were they are
not used. A false-positive mask is used to identify these blocks
and for blocks in this mask the error is turned into a debug message
to avoid false-positive error messages.
Note the pmc_atom code is enabled by CONFIG_X86_INTEL_LPSS which
already depends on ACPI.
Signed-off-by: Johannes Stezenbach <js@sig21.net>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
[hdegoede: Use acpi_s2idle_dev_ops, ignore fused off blocks, PMIC I2C]
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
- Reword commit message
- Drop confusing /* Low Part */ and /* High Part */ comments
Changes in v2:
- Drop duplicated "pmc_atom: " prefix from pr_err() / pr_dbg() messages
---
drivers/platform/x86/pmc_atom.c | 64 +++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)
diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
index 93a6414c6611..ec60b734b9cb 100644
--- a/drivers/platform/x86/pmc_atom.c
+++ b/drivers/platform/x86/pmc_atom.c
@@ -6,6 +6,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/acpi.h>
#include <linux/debugfs.h>
#include <linux/device.h>
#include <linux/dmi.h>
@@ -17,6 +18,7 @@
#include <linux/platform_device.h>
#include <linux/pci.h>
#include <linux/seq_file.h>
+#include <linux/suspend.h>
struct pmc_bit_map {
const char *name;
@@ -448,6 +450,64 @@ static int pmc_setup_clks(struct pci_dev *pdev, void __iomem *pmc_regmap,
return 0;
}
+#ifdef CONFIG_SUSPEND
+static void pmc_dev_state_check(u32 sts, const struct pmc_bit_map *sts_map,
+ u32 fd, const struct pmc_bit_map *fd_map,
+ u32 sts_possible_false_pos)
+{
+ int index;
+
+ for (index = 0; sts_map[index].name; index++) {
+ if (!(fd_map[index].bit_mask & fd) &&
+ !(sts_map[index].bit_mask & sts)) {
+ if (sts_map[index].bit_mask & sts_possible_false_pos)
+ pm_pr_dbg("%s is in D0 prior to s2idle\n",
+ sts_map[index].name);
+ else
+ pr_err("%s is in D0 prior to s2idle\n",
+ sts_map[index].name);
+ }
+ }
+}
+
+static void pmc_s2idle_check(void)
+{
+ struct pmc_dev *pmc = &pmc_device;
+ const struct pmc_reg_map *m = pmc->map;
+ u32 func_dis, func_dis_2;
+ u32 d3_sts_0, d3_sts_1;
+ u32 false_pos_sts_0, false_pos_sts_1;
+
+ func_dis = pmc_reg_read(pmc, PMC_FUNC_DIS);
+ func_dis_2 = pmc_reg_read(pmc, PMC_FUNC_DIS_2);
+ d3_sts_0 = pmc_reg_read(pmc, PMC_D3_STS_0);
+ d3_sts_1 = pmc_reg_read(pmc, PMC_D3_STS_1);
+
+ /*
+ * Some blocks are not used on lower-featured versions of the SoC and
+ * always report D0, add these to false_pos mask to log at debug level.
+ */
+ if (m->d3_sts_1 == byt_d3_sts_1_map) {
+ /* Bay Trail */
+ false_pos_sts_0 = BIT_GBE | BIT_SATA | BIT_PCIE_PORT0 |
+ BIT_PCIE_PORT1 | BIT_PCIE_PORT2 | BIT_PCIE_PORT3 |
+ BIT_LPSS2_F5_I2C5;
+ false_pos_sts_1 = BIT_SMB | BIT_USH_SS_PHY | BIT_DFX;
+ } else {
+ /* Cherry Trail */
+ false_pos_sts_0 = BIT_GBE | BIT_SATA | BIT_LPSS2_F7_I2C7;
+ false_pos_sts_1 = BIT_SMB | BIT_STS_ISH;
+ }
+
+ pmc_dev_state_check(d3_sts_0, m->d3_sts_0, func_dis, m->func_dis, false_pos_sts_0);
+ pmc_dev_state_check(d3_sts_1, m->d3_sts_1, func_dis_2, m->func_dis_2, false_pos_sts_1);
+}
+
+static struct acpi_s2idle_dev_ops pmc_s2idle_ops = {
+ .check = pmc_s2idle_check,
+};
+#endif
+
static int pmc_setup_dev(struct pci_dev *pdev, const struct pci_device_id *ent)
{
struct pmc_dev *pmc = &pmc_device;
@@ -485,6 +545,10 @@ static int pmc_setup_dev(struct pci_dev *pdev, const struct pci_device_id *ent)
dev_warn(&pdev->dev, "platform clocks register failed: %d\n",
ret);
+#ifdef CONFIG_SUSPEND
+ acpi_register_lps0_dev(&pmc_s2idle_ops);
+#endif
+
pmc->init = true;
return ret;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 4/5] platform/x86: pmc_atom: Check state of PMC clocks on s2idle
2024-01-08 13:02 [PATCH v3 0/5] x86: atom-punit/-pmc s2idle device state checks Hans de Goede
` (2 preceding siblings ...)
2024-01-08 13:02 ` [PATCH v3 3/5] platform/x86: pmc_atom: Check state of PMC managed devices on s2idle Hans de Goede
@ 2024-01-08 13:02 ` Hans de Goede
2024-01-08 13:02 ` [PATCH v3 5/5] x86/platform/atom: Check state of Punit managed devices " Hans de Goede
4 siblings, 0 replies; 9+ messages in thread
From: Hans de Goede @ 2024-01-08 13:02 UTC (permalink / raw
To: Johannes Stezenbach, Takashi Iwai, Ilpo Järvinen,
Andy Shevchenko, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, H . Peter Anvin, Michael Turquette, Stephen Boyd
Cc: Hans de Goede, platform-driver-x86, x86, linux-clk
Extend the s2idle check with checking that none of the PMC clocks
is in the forced-on state. If one of the clocks is in forced on
state then S0i3 cannot be reached.
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
- Improve comment for clocks check
Changes in v2:
- Drop the PMC_CLK_* defines these are defined in
include/linux/platform_data/x86/pmc_atom.h now
- Drop duplicated "pmc_atom: " prefix from pr_err() message
---
drivers/platform/x86/pmc_atom.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
index ec60b734b9cb..9f2009a354f9 100644
--- a/drivers/platform/x86/pmc_atom.c
+++ b/drivers/platform/x86/pmc_atom.c
@@ -477,6 +477,7 @@ static void pmc_s2idle_check(void)
u32 func_dis, func_dis_2;
u32 d3_sts_0, d3_sts_1;
u32 false_pos_sts_0, false_pos_sts_1;
+ int i;
func_dis = pmc_reg_read(pmc, PMC_FUNC_DIS);
func_dis_2 = pmc_reg_read(pmc, PMC_FUNC_DIS_2);
@@ -501,6 +502,16 @@ static void pmc_s2idle_check(void)
pmc_dev_state_check(d3_sts_0, m->d3_sts_0, func_dis, m->func_dis, false_pos_sts_0);
pmc_dev_state_check(d3_sts_1, m->d3_sts_1, func_dis_2, m->func_dis_2, false_pos_sts_1);
+
+ /* Forced-on PMC clocks prevent S0i3 */
+ for (i = 0; i < PMC_CLK_NUM; i++) {
+ u32 ctl = pmc_reg_read(pmc, PMC_CLK_CTL_OFFSET + 4 * i);
+
+ if ((ctl & PMC_MASK_CLK_CTL) != PMC_CLK_CTL_FORCE_ON)
+ continue;
+
+ pr_err("clock %d is ON prior to freeze (ctl 0x%08x)\n", i, ctl);
+ }
}
static struct acpi_s2idle_dev_ops pmc_s2idle_ops = {
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 5/5] x86/platform/atom: Check state of Punit managed devices on s2idle
2024-01-08 13:02 [PATCH v3 0/5] x86: atom-punit/-pmc s2idle device state checks Hans de Goede
` (3 preceding siblings ...)
2024-01-08 13:02 ` [PATCH v3 4/5] platform/x86: pmc_atom: Check state of PMC clocks " Hans de Goede
@ 2024-01-08 13:02 ` Hans de Goede
2024-01-08 13:08 ` Ilpo Järvinen
4 siblings, 1 reply; 9+ messages in thread
From: Hans de Goede @ 2024-01-08 13:02 UTC (permalink / raw
To: Johannes Stezenbach, Takashi Iwai, Ilpo Järvinen,
Andy Shevchenko, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, H . Peter Anvin, Michael Turquette, Stephen Boyd
Cc: Hans de Goede, platform-driver-x86, x86, linux-clk
From: Johannes Stezenbach <js@sig21.net>
For the Bay Trail or Cherry Trail SoC to enter the S0i3 power-level
at s2idle suspend requires most of the hw-blocks / devices in the SoC
to be in D3 when entering s2idle suspend.
If some devices are not in D3 then the SoC will stay in a higher
power state, consuming much more power from the battery then in S0i3.
Use the new acpi_s2idle_dev_ops and acpi_register_lps0_dev()
functionality to register a new s2idle check function which checks that
all hardware blocks in theNorth complex (controlled by Punit) are in
a state that allows the SoC to enter S0i3 and prints an error message
for any device in D0.
Signed-off-by: Johannes Stezenbach <js@sig21.net>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Acked-by: Borislav Petkov (AMD) <bp@alien8.de>
[hdegoede: Use acpi_s2idle_dev_ops]
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
- Reword commit message
- Add punit_s2idle_check_[un]register() helper functions
---
arch/x86/platform/atom/punit_atom_debug.c | 54 ++++++++++++++++++++++-
1 file changed, 53 insertions(+), 1 deletion(-)
diff --git a/arch/x86/platform/atom/punit_atom_debug.c b/arch/x86/platform/atom/punit_atom_debug.c
index f8ed5f66cd20..6b9c6deca8ba 100644
--- a/arch/x86/platform/atom/punit_atom_debug.c
+++ b/arch/x86/platform/atom/punit_atom_debug.c
@@ -7,6 +7,9 @@
* Copyright (c) 2015, Intel Corporation.
*/
+#define pr_fmt(fmt) "punit_atom: " fmt
+
+#include <linux/acpi.h>
#include <linux/module.h>
#include <linux/init.h>
#include <linux/device.h>
@@ -117,6 +120,51 @@ static void punit_dbgfs_unregister(void)
debugfs_remove_recursive(punit_dbg_file);
}
+#if defined(CONFIG_ACPI) && defined(CONFIG_SUSPEND)
+static const struct punit_device *punit_dev;
+
+static void punit_s2idle_check(void)
+{
+ const struct punit_device *punit_devp;
+ u32 punit_pwr_status, dstate;
+ int status;
+
+ for (punit_devp = punit_dev; punit_devp->name; punit_devp++) {
+ /* Skip MIO, it is on till the very last moment */
+ if (punit_devp->reg == MIO_SS_PM)
+ continue;
+
+ status = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ,
+ punit_devp->reg, &punit_pwr_status);
+ if (status) {
+ pr_err("%s read failed\n", punit_devp->name);
+ } else {
+ dstate = (punit_pwr_status >> punit_devp->sss_pos) & 3;
+ if (!dstate)
+ pr_err("%s is in D0 prior to s2idle\n", punit_devp->name);
+ }
+ }
+}
+
+static struct acpi_s2idle_dev_ops punit_s2idle_ops = {
+ .check = punit_s2idle_check,
+};
+
+static void punit_s2idle_check_register(struct punit_device *punit_device)
+{
+ punit_dev = punit_device;
+ acpi_register_lps0_dev(&punit_s2idle_ops);
+}
+
+static void punit_s2idle_check_unregister(void)
+{
+ acpi_unregister_lps0_dev(&punit_s2idle_ops);
+}
+#else
+static void punit_s2idle_check_register(struct punit_device *punit_device) {}
+static void punit_s2idle_check_unregister(void) {}
+#endif
+
#define X86_MATCH(model, data) \
X86_MATCH_VENDOR_FAM_MODEL_FEATURE(INTEL, 6, INTEL_FAM6_##model, \
X86_FEATURE_MWAIT, data)
@@ -131,19 +179,23 @@ MODULE_DEVICE_TABLE(x86cpu, intel_punit_cpu_ids);
static int __init punit_atom_debug_init(void)
{
+ struct punit_device *punit_device;
const struct x86_cpu_id *id;
id = x86_match_cpu(intel_punit_cpu_ids);
if (!id)
return -ENODEV;
- punit_dbgfs_register((struct punit_device *)id->driver_data);
+ punit_device = (struct punit_device *)id->driver_data;
+ punit_dbgfs_register(punit_device);
+ punit_s2idle_check_register(punit_device);
return 0;
}
static void __exit punit_atom_debug_exit(void)
{
+ punit_s2idle_check_unregister();
punit_dbgfs_unregister();
}
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 5/5] x86/platform/atom: Check state of Punit managed devices on s2idle
2024-01-08 13:02 ` [PATCH v3 5/5] x86/platform/atom: Check state of Punit managed devices " Hans de Goede
@ 2024-01-08 13:08 ` Ilpo Järvinen
0 siblings, 0 replies; 9+ messages in thread
From: Ilpo Järvinen @ 2024-01-08 13:08 UTC (permalink / raw
To: Hans de Goede
Cc: Johannes Stezenbach, Takashi Iwai, Andy Shevchenko,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H . Peter Anvin, Michael Turquette, Stephen Boyd,
platform-driver-x86, x86, linux-clk
[-- Attachment #1: Type: text/plain, Size: 4037 bytes --]
On Mon, 8 Jan 2024, Hans de Goede wrote:
> From: Johannes Stezenbach <js@sig21.net>
>
> For the Bay Trail or Cherry Trail SoC to enter the S0i3 power-level
> at s2idle suspend requires most of the hw-blocks / devices in the SoC
> to be in D3 when entering s2idle suspend.
>
> If some devices are not in D3 then the SoC will stay in a higher
> power state, consuming much more power from the battery then in S0i3.
>
> Use the new acpi_s2idle_dev_ops and acpi_register_lps0_dev()
> functionality to register a new s2idle check function which checks that
> all hardware blocks in theNorth complex (controlled by Punit) are in
nit: missing space.
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
> a state that allows the SoC to enter S0i3 and prints an error message
> for any device in D0.
>
> Signed-off-by: Johannes Stezenbach <js@sig21.net>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> Acked-by: Borislav Petkov (AMD) <bp@alien8.de>
> [hdegoede: Use acpi_s2idle_dev_ops]
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v3:
> - Reword commit message
> - Add punit_s2idle_check_[un]register() helper functions
> ---
> arch/x86/platform/atom/punit_atom_debug.c | 54 ++++++++++++++++++++++-
> 1 file changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/platform/atom/punit_atom_debug.c b/arch/x86/platform/atom/punit_atom_debug.c
> index f8ed5f66cd20..6b9c6deca8ba 100644
> --- a/arch/x86/platform/atom/punit_atom_debug.c
> +++ b/arch/x86/platform/atom/punit_atom_debug.c
> @@ -7,6 +7,9 @@
> * Copyright (c) 2015, Intel Corporation.
> */
>
> +#define pr_fmt(fmt) "punit_atom: " fmt
> +
> +#include <linux/acpi.h>
> #include <linux/module.h>
> #include <linux/init.h>
> #include <linux/device.h>
> @@ -117,6 +120,51 @@ static void punit_dbgfs_unregister(void)
> debugfs_remove_recursive(punit_dbg_file);
> }
>
> +#if defined(CONFIG_ACPI) && defined(CONFIG_SUSPEND)
> +static const struct punit_device *punit_dev;
> +
> +static void punit_s2idle_check(void)
> +{
> + const struct punit_device *punit_devp;
> + u32 punit_pwr_status, dstate;
> + int status;
> +
> + for (punit_devp = punit_dev; punit_devp->name; punit_devp++) {
> + /* Skip MIO, it is on till the very last moment */
> + if (punit_devp->reg == MIO_SS_PM)
> + continue;
> +
> + status = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ,
> + punit_devp->reg, &punit_pwr_status);
> + if (status) {
> + pr_err("%s read failed\n", punit_devp->name);
> + } else {
> + dstate = (punit_pwr_status >> punit_devp->sss_pos) & 3;
> + if (!dstate)
> + pr_err("%s is in D0 prior to s2idle\n", punit_devp->name);
> + }
> + }
> +}
> +
> +static struct acpi_s2idle_dev_ops punit_s2idle_ops = {
> + .check = punit_s2idle_check,
> +};
> +
> +static void punit_s2idle_check_register(struct punit_device *punit_device)
> +{
> + punit_dev = punit_device;
> + acpi_register_lps0_dev(&punit_s2idle_ops);
> +}
> +
> +static void punit_s2idle_check_unregister(void)
> +{
> + acpi_unregister_lps0_dev(&punit_s2idle_ops);
> +}
> +#else
> +static void punit_s2idle_check_register(struct punit_device *punit_device) {}
> +static void punit_s2idle_check_unregister(void) {}
> +#endif
> +
> #define X86_MATCH(model, data) \
> X86_MATCH_VENDOR_FAM_MODEL_FEATURE(INTEL, 6, INTEL_FAM6_##model, \
> X86_FEATURE_MWAIT, data)
> @@ -131,19 +179,23 @@ MODULE_DEVICE_TABLE(x86cpu, intel_punit_cpu_ids);
>
> static int __init punit_atom_debug_init(void)
> {
> + struct punit_device *punit_device;
> const struct x86_cpu_id *id;
>
> id = x86_match_cpu(intel_punit_cpu_ids);
> if (!id)
> return -ENODEV;
>
> - punit_dbgfs_register((struct punit_device *)id->driver_data);
> + punit_device = (struct punit_device *)id->driver_data;
> + punit_dbgfs_register(punit_device);
> + punit_s2idle_check_register(punit_device);
>
> return 0;
> }
>
> static void __exit punit_atom_debug_exit(void)
> {
> + punit_s2idle_check_unregister();
> punit_dbgfs_unregister();
> }
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/5] platform/x86: pmc_atom: Check state of PMC managed devices on s2idle
2024-01-08 13:02 ` [PATCH v3 3/5] platform/x86: pmc_atom: Check state of PMC managed devices on s2idle Hans de Goede
@ 2024-01-08 13:12 ` Ilpo Järvinen
0 siblings, 0 replies; 9+ messages in thread
From: Ilpo Järvinen @ 2024-01-08 13:12 UTC (permalink / raw
To: Hans de Goede
Cc: Johannes Stezenbach, Takashi Iwai, Andy Shevchenko,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H . Peter Anvin, Michael Turquette, Stephen Boyd,
platform-driver-x86, x86, linux-clk
[-- Attachment #1: Type: text/plain, Size: 4868 bytes --]
On Mon, 8 Jan 2024, Hans de Goede wrote:
> From: Johannes Stezenbach <js@sig21.net>
>
> For the Bay Trail or Cherry Trail SoC to enter the S0i3 power-level
> at s2idle suspend requires most of the hw-blocks / devices in the SoC
> to be in D3 when entering s2idle suspend.
>
> If some devices are not in D3 then the SoC will stay in a higher
> power state, consuming much more power from the battery then in S0i3.
>
> Use the new acpi_s2idle_dev_ops and acpi_register_lps0_dev()
> functionality to register a new s2idle check function which checks that
> all hardware blocks in the South complex (controlled by the PMC)
> are in a state that allows the SoC to enter S0i3 and prints an error
> message for any device in D0.
>
> Some blocks are not used on lower-featured versions of the SoC and
> these blocks will always report being in D0 on SoCs were they are
> not used. A false-positive mask is used to identify these blocks
> and for blocks in this mask the error is turned into a debug message
> to avoid false-positive error messages.
>
> Note the pmc_atom code is enabled by CONFIG_X86_INTEL_LPSS which
> already depends on ACPI.
>
> Signed-off-by: Johannes Stezenbach <js@sig21.net>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> [hdegoede: Use acpi_s2idle_dev_ops, ignore fused off blocks, PMIC I2C]
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v3:
> - Reword commit message
> - Drop confusing /* Low Part */ and /* High Part */ comments
>
> Changes in v2:
> - Drop duplicated "pmc_atom: " prefix from pr_err() / pr_dbg() messages
> ---
> drivers/platform/x86/pmc_atom.c | 64 +++++++++++++++++++++++++++++++++
> 1 file changed, 64 insertions(+)
>
> diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
> index 93a6414c6611..ec60b734b9cb 100644
> --- a/drivers/platform/x86/pmc_atom.c
> +++ b/drivers/platform/x86/pmc_atom.c
> @@ -6,6 +6,7 @@
>
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> +#include <linux/acpi.h>
> #include <linux/debugfs.h>
> #include <linux/device.h>
> #include <linux/dmi.h>
> @@ -17,6 +18,7 @@
> #include <linux/platform_device.h>
> #include <linux/pci.h>
> #include <linux/seq_file.h>
> +#include <linux/suspend.h>
>
> struct pmc_bit_map {
> const char *name;
> @@ -448,6 +450,64 @@ static int pmc_setup_clks(struct pci_dev *pdev, void __iomem *pmc_regmap,
> return 0;
> }
>
> +#ifdef CONFIG_SUSPEND
> +static void pmc_dev_state_check(u32 sts, const struct pmc_bit_map *sts_map,
> + u32 fd, const struct pmc_bit_map *fd_map,
> + u32 sts_possible_false_pos)
> +{
> + int index;
> +
> + for (index = 0; sts_map[index].name; index++) {
> + if (!(fd_map[index].bit_mask & fd) &&
> + !(sts_map[index].bit_mask & sts)) {
> + if (sts_map[index].bit_mask & sts_possible_false_pos)
> + pm_pr_dbg("%s is in D0 prior to s2idle\n",
> + sts_map[index].name);
> + else
> + pr_err("%s is in D0 prior to s2idle\n",
> + sts_map[index].name);
> + }
> + }
> +}
> +
> +static void pmc_s2idle_check(void)
> +{
> + struct pmc_dev *pmc = &pmc_device;
> + const struct pmc_reg_map *m = pmc->map;
> + u32 func_dis, func_dis_2;
> + u32 d3_sts_0, d3_sts_1;
> + u32 false_pos_sts_0, false_pos_sts_1;
> +
> + func_dis = pmc_reg_read(pmc, PMC_FUNC_DIS);
> + func_dis_2 = pmc_reg_read(pmc, PMC_FUNC_DIS_2);
> + d3_sts_0 = pmc_reg_read(pmc, PMC_D3_STS_0);
> + d3_sts_1 = pmc_reg_read(pmc, PMC_D3_STS_1);
> +
> + /*
> + * Some blocks are not used on lower-featured versions of the SoC and
> + * always report D0, add these to false_pos mask to log at debug level.
> + */
> + if (m->d3_sts_1 == byt_d3_sts_1_map) {
> + /* Bay Trail */
> + false_pos_sts_0 = BIT_GBE | BIT_SATA | BIT_PCIE_PORT0 |
> + BIT_PCIE_PORT1 | BIT_PCIE_PORT2 | BIT_PCIE_PORT3 |
> + BIT_LPSS2_F5_I2C5;
> + false_pos_sts_1 = BIT_SMB | BIT_USH_SS_PHY | BIT_DFX;
> + } else {
> + /* Cherry Trail */
> + false_pos_sts_0 = BIT_GBE | BIT_SATA | BIT_LPSS2_F7_I2C7;
> + false_pos_sts_1 = BIT_SMB | BIT_STS_ISH;
> + }
> +
> + pmc_dev_state_check(d3_sts_0, m->d3_sts_0, func_dis, m->func_dis, false_pos_sts_0);
> + pmc_dev_state_check(d3_sts_1, m->d3_sts_1, func_dis_2, m->func_dis_2, false_pos_sts_1);
> +}
> +
> +static struct acpi_s2idle_dev_ops pmc_s2idle_ops = {
> + .check = pmc_s2idle_check,
> +};
> +#endif
> +
> static int pmc_setup_dev(struct pci_dev *pdev, const struct pci_device_id *ent)
> {
> struct pmc_dev *pmc = &pmc_device;
> @@ -485,6 +545,10 @@ static int pmc_setup_dev(struct pci_dev *pdev, const struct pci_device_id *ent)
> dev_warn(&pdev->dev, "platform clocks register failed: %d\n",
> ret);
>
> +#ifdef CONFIG_SUSPEND
> + acpi_register_lps0_dev(&pmc_s2idle_ops);
> +#endif
I'm sorry I missed this in v2, this could have also used #else +
dummy function.
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/5] clk: x86: Move clk-pmc-atom register defines to include/linux/platform_data/x86/pmc_atom.h
2024-01-08 13:02 ` [PATCH v3 1/5] clk: x86: Move clk-pmc-atom register defines to include/linux/platform_data/x86/pmc_atom.h Hans de Goede
@ 2024-01-08 23:33 ` Stephen Boyd
0 siblings, 0 replies; 9+ messages in thread
From: Stephen Boyd @ 2024-01-08 23:33 UTC (permalink / raw
To: Andy Shevchenko, Borislav Petkov, Dave Hansen, H . Peter Anvin,
Hans de Goede, Ilpo Järvinen, Ingo Molnar,
Johannes Stezenbach, Michael Turquette, Takashi Iwai,
Thomas Gleixner
Cc: Hans de Goede, platform-driver-x86, x86, linux-clk
Quoting Hans de Goede (2024-01-08 05:02:34)
> Move the register defines for the Atom (Bay Trail, Cherry Trail) PMC
> clocks to include/linux/platform_data/x86/pmc_atom.h.
>
> This is a preparation patch to extend the S0i3 readiness checks
> in drivers/platform/x86/pmc_atom.c with checking that the PMC
> clocks are off on suspend entry.
>
> Note these are added to include/linux/platform_data/x86/pmc_atom.h rather
> then to include/linux/platform_data/x86/clk-pmc-atom.h because the former
> already has all the other Atom PMC register defines.
>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
Acked-by: Stephen Boyd <sboyd@kernel.org>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-01-08 23:33 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-08 13:02 [PATCH v3 0/5] x86: atom-punit/-pmc s2idle device state checks Hans de Goede
2024-01-08 13:02 ` [PATCH v3 1/5] clk: x86: Move clk-pmc-atom register defines to include/linux/platform_data/x86/pmc_atom.h Hans de Goede
2024-01-08 23:33 ` Stephen Boyd
2024-01-08 13:02 ` [PATCH v3 2/5] platform/x86: pmc_atom: Annotate d3_sts register bit defines Hans de Goede
2024-01-08 13:02 ` [PATCH v3 3/5] platform/x86: pmc_atom: Check state of PMC managed devices on s2idle Hans de Goede
2024-01-08 13:12 ` Ilpo Järvinen
2024-01-08 13:02 ` [PATCH v3 4/5] platform/x86: pmc_atom: Check state of PMC clocks " Hans de Goede
2024-01-08 13:02 ` [PATCH v3 5/5] x86/platform/atom: Check state of Punit managed devices " Hans de Goede
2024-01-08 13:08 ` Ilpo Järvinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).