devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] soc: loongson2_pm: add power management support
@ 2023-06-15  9:17 Yinbo Zhu
  2023-06-15  9:17 ` [PATCH v3 1/3] loongarch: export some arch-specific pm interfaces Yinbo Zhu
                   ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Yinbo Zhu @ 2023-06-15  9:17 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Huacai Chen,
	WANG Xuerui, Rafael J . Wysocki, Pavel Machek, Marc Zyngier,
	Arnd Bergmann, linux-pm, devicetree, linux-kernel, loongarch
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, Yinbo Zhu

Loongson-2 platform support Power Management Controller (ACPI) and this
series patch was to add PM driver that base on dts and PM binding support.

Change in v3:
		1. Reword the [1/3] patch commit log and title.
		2. Use the old naming for suspend interface for the [1/3] and
		   [3/3] patch.
		3. Combine some small function in the driver patch.
		4. Rename 'pwrbt' to 'button' in the driver patch.
		5. Use the specific compatible in yaml file.
Change in v2:
		1. Fixup the "suspend-address" description.
		2. Remove the "return -EINVAL" in PM driver probe when firmware
		   no configure "suspend-address" property in dts in oder to
		   other PM state to work.

Yinbo Zhu (3):
  loongarch: export some arch-specific pm interfaces
  soc: dt-bindings: add loongson-2 pm
  soc: loongson2_pm: add power management support

 .../soc/loongson/loongson,ls2k-pmc.yaml       |  53 +++++
 MAINTAINERS                                   |   7 +
 arch/loongarch/include/asm/acpi.h             |   3 +-
 arch/loongarch/include/asm/suspend.h          |  10 +
 arch/loongarch/power/suspend.c                |   8 +-
 drivers/soc/loongson/Kconfig                  |  10 +
 drivers/soc/loongson/Makefile                 |   1 +
 drivers/soc/loongson/loongson2_pm.c           | 218 ++++++++++++++++++
 8 files changed, 304 insertions(+), 6 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 1/3] loongarch: export some arch-specific pm interfaces
  2023-06-15  9:17 [PATCH v3 0/3] soc: loongson2_pm: add power management support Yinbo Zhu
@ 2023-06-15  9:17 ` Yinbo Zhu
  2023-06-16  4:04   ` Huacai Chen
  2023-06-15  9:17 ` [PATCH v3 2/3] soc: dt-bindings: add loongson-2 pm Yinbo Zhu
  2023-06-15  9:37 ` [PATCH v3 3/3] soc: loongson2_pm: add power management support zhuyinbo
  2 siblings, 1 reply; 21+ messages in thread
From: Yinbo Zhu @ 2023-06-15  9:17 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Huacai Chen,
	WANG Xuerui, Rafael J . Wysocki, Pavel Machek, Marc Zyngier,
	Arnd Bergmann, linux-pm, devicetree, linux-kernel, loongarch
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, Yinbo Zhu

Some power management controllers need to support DTS and will use
the suspend interface thus this patch was to export such interface
for their use.

Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
---
 arch/loongarch/include/asm/acpi.h    |  3 +--
 arch/loongarch/include/asm/suspend.h | 10 ++++++++++
 arch/loongarch/power/suspend.c       |  8 ++++----
 3 files changed, 15 insertions(+), 6 deletions(-)
 create mode 100644 arch/loongarch/include/asm/suspend.h

diff --git a/arch/loongarch/include/asm/acpi.h b/arch/loongarch/include/asm/acpi.h
index 976a810352c6..1d7810798c08 100644
--- a/arch/loongarch/include/asm/acpi.h
+++ b/arch/loongarch/include/asm/acpi.h
@@ -8,6 +8,7 @@
 #ifndef _ASM_LOONGARCH_ACPI_H
 #define _ASM_LOONGARCH_ACPI_H
 
+#include <asm/suspend.h>
 #ifdef CONFIG_ACPI
 extern int acpi_strict;
 extern int acpi_disabled;
@@ -37,12 +38,10 @@ extern struct list_head acpi_wakeup_device_list;
 
 extern int loongarch_acpi_suspend(void);
 extern int (*acpi_suspend_lowlevel)(void);
-extern void loongarch_suspend_enter(void);
 
 static inline unsigned long acpi_get_wakeup_address(void)
 {
 #ifdef CONFIG_SUSPEND
-	extern void loongarch_wakeup_start(void);
 	return (unsigned long)loongarch_wakeup_start;
 #endif
 	return 0UL;
diff --git a/arch/loongarch/include/asm/suspend.h b/arch/loongarch/include/asm/suspend.h
new file mode 100644
index 000000000000..fc64089fefaa
--- /dev/null
+++ b/arch/loongarch/include/asm/suspend.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_SUSPEND_H
+#define __ASM_SUSPEND_H
+
+void loongarch_common_resume(void);
+void loongarch_common_suspend(void);
+void loongarch_suspend_enter(void);
+void loongarch_wakeup_start(void);
+
+#endif
diff --git a/arch/loongarch/power/suspend.c b/arch/loongarch/power/suspend.c
index 5e19733e5e05..166d9e06a64b 100644
--- a/arch/loongarch/power/suspend.c
+++ b/arch/loongarch/power/suspend.c
@@ -27,7 +27,7 @@ struct saved_registers {
 };
 static struct saved_registers saved_regs;
 
-static void arch_common_suspend(void)
+void loongarch_common_suspend(void)
 {
 	save_counter();
 	saved_regs.pgd = csr_read64(LOONGARCH_CSR_PGDL);
@@ -40,7 +40,7 @@ static void arch_common_suspend(void)
 	loongarch_suspend_addr = loongson_sysconf.suspend_addr;
 }
 
-static void arch_common_resume(void)
+void loongarch_common_resume(void)
 {
 	sync_counter();
 	local_flush_tlb_all();
@@ -62,12 +62,12 @@ int loongarch_acpi_suspend(void)
 	enable_gpe_wakeup();
 	enable_pci_wakeup();
 
-	arch_common_suspend();
+	loongarch_common_suspend();
 
 	/* processor specific suspend */
 	loongarch_suspend_enter();
 
-	arch_common_resume();
+	loongarch_common_resume();
 
 	return 0;
 }
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 2/3] soc: dt-bindings: add loongson-2 pm
  2023-06-15  9:17 [PATCH v3 0/3] soc: loongson2_pm: add power management support Yinbo Zhu
  2023-06-15  9:17 ` [PATCH v3 1/3] loongarch: export some arch-specific pm interfaces Yinbo Zhu
@ 2023-06-15  9:17 ` Yinbo Zhu
  2023-06-16  6:58   ` Conor Dooley
                     ` (2 more replies)
  2023-06-15  9:37 ` [PATCH v3 3/3] soc: loongson2_pm: add power management support zhuyinbo
  2 siblings, 3 replies; 21+ messages in thread
From: Yinbo Zhu @ 2023-06-15  9:17 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Huacai Chen,
	WANG Xuerui, Rafael J . Wysocki, Pavel Machek, Marc Zyngier,
	Arnd Bergmann, linux-pm, devicetree, linux-kernel, loongarch
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, Yinbo Zhu

Add the Loongson-2 SoC Power Management Controller binding with DT
schema format using json-schema.

Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
---
 .../soc/loongson/loongson,ls2k-pmc.yaml       | 53 +++++++++++++++++++
 MAINTAINERS                                   |  6 +++
 2 files changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml

diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
new file mode 100644
index 000000000000..32499bd10f8c
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/loongson/loongson,ls2k-pmc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Loongson-2 Power Manager controller
+
+maintainers:
+  - Yinbo Zhu <zhuyinbo@loongson.cn>
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - loongson,ls2k1000-pmc
+              - loongson,ls2k0500-pmc
+          - const: syscon
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  suspend-address:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      The "suspend-address" is a deep sleep state (Suspend To RAM)
+      firmware entry address which was jumped from kernel and it's
+      value was dependent on specific platform firmware code. In
+      addition, the PM need according to it to indicate that current
+      SoC whether support Suspend To RAM.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    pmc: pm@1fe27000 {
+        compatible = "loongson,ls2k1000-pmc", "syscon";
+        reg = <0x1fe27000 0x58>;
+        interrupt-parent = <&liointc1>;
+        interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
+        suspend-address = <0x1c000500>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 7a91f14cad2e..bcd05f1fa5c1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12190,6 +12190,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/hwinfo/loongson,ls2k-chipid.yaml
 F:	drivers/soc/loongson/loongson2_guts.c
 
+LOONGSON-2 SOC SERIES PM DRIVER
+M:	Yinbo Zhu <zhuyinbo@loongson.cn>
+L:	linux-pm@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
+
 LOONGSON-2 SOC SERIES PINCTRL DRIVER
 M:	zhanghongchen <zhanghongchen@loongson.cn>
 M:	Yinbo Zhu <zhuyinbo@loongson.cn>
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 3/3] soc: loongson2_pm: add power management support
  2023-06-15  9:17 [PATCH v3 0/3] soc: loongson2_pm: add power management support Yinbo Zhu
  2023-06-15  9:17 ` [PATCH v3 1/3] loongarch: export some arch-specific pm interfaces Yinbo Zhu
  2023-06-15  9:17 ` [PATCH v3 2/3] soc: dt-bindings: add loongson-2 pm Yinbo Zhu
@ 2023-06-15  9:37 ` zhuyinbo
  2023-06-15 10:00   ` Huacai Chen
  2023-06-16  6:52   ` Conor Dooley
  2 siblings, 2 replies; 21+ messages in thread
From: zhuyinbo @ 2023-06-15  9:37 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Huacai Chen,
	WANG Xuerui, Rafael J . Wysocki, Pavel Machek, Marc Zyngier,
	Arnd Bergmann, linux-pm, devicetree, linux-kernel, loongarch
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo

 From 6edcb9d6a1b18ccbecaf283b4f543afc9e7126d6 Mon Sep 17 00:00:00 2001
From: Yinbo Zhu <zhuyinbo@loongson.cn>
Date: Tue, 18 Apr 2023 14:18:00 +0800
Subject: [PATCH v3 3/3] soc: loongson2_pm: add power management support

The Loongson-2's power management controller was ACPI, supports ACPI
S2Idle (Suspend To Idle), ACPI S3 (Suspend To RAM), ACPI S4 (Suspend To
Disk), ACPI S5 (Soft Shutdown) and supports multiple wake-up methods
(USB, GMAC, PWRBTN, etc.). This driver was to add power management
controller support that base on dts for Loongson-2 series SoCs.

Signed-off-by: Liu Yun <liuyun@loongson.cn>
Signed-off-by: Liu Peibao <liupeibao@loongson.cn>
Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
---
  MAINTAINERS                         |   1 +
  drivers/soc/loongson/Kconfig        |  10 ++
  drivers/soc/loongson/Makefile       |   1 +
  drivers/soc/loongson/loongson2_pm.c | 218 ++++++++++++++++++++++++++++
  4 files changed, 230 insertions(+)
  create mode 100644 drivers/soc/loongson/loongson2_pm.c

diff --git a/MAINTAINERS b/MAINTAINERS
index bcd05f1fa5c1..7c4ad0cbaeff 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12195,6 +12195,7 @@ M:	Yinbo Zhu <zhuyinbo@loongson.cn>
  L:	linux-pm@vger.kernel.org
  S:	Maintained
  F:	Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
+F:	drivers/soc/loongson/loongson2_pm.c

  LOONGSON-2 SOC SERIES PINCTRL DRIVER
  M:	zhanghongchen <zhanghongchen@loongson.cn>
diff --git a/drivers/soc/loongson/Kconfig b/drivers/soc/loongson/Kconfig
index 707f56358dc4..4f3ce9eb7520 100644
--- a/drivers/soc/loongson/Kconfig
+++ b/drivers/soc/loongson/Kconfig
@@ -16,3 +16,13 @@ config LOONGSON2_GUTS
  	 SoCs. Initially only reading SVR and registering soc device are
  	 supported. Other guts accesses, such as reading firmware configuration
  	 by default, should eventually be added into this driver as well.
+
+config LOONGSON2_PM
+	bool "Loongson-2 SoC Power Management Controller Driver"
+	depends on LOONGARCH && OF
+	help
+	 The Loongson-2's power management controller was ACPI, supports ACPI
+	 S2Idle (Suspend To Idle), ACPI S3 (Suspend To RAM), ACPI S4 (Suspend To
+	 Disk), ACPI S5 (Soft Shutdown) and supports multiple wake-up methods
+	 (USB, GMAC, PWRBTN, etc.). This driver was to add power management
+	 controller support that base on dts for Loongson-2 series SoCs.
diff --git a/drivers/soc/loongson/Makefile b/drivers/soc/loongson/Makefile
index 263c486df638..4118f50f55e2 100644
--- a/drivers/soc/loongson/Makefile
+++ b/drivers/soc/loongson/Makefile
@@ -4,3 +4,4 @@
  #

  obj-$(CONFIG_LOONGSON2_GUTS)		+= loongson2_guts.o
+obj-$(CONFIG_LOONGSON2_PM)		+= loongson2_pm.o
diff --git a/drivers/soc/loongson/loongson2_pm.c 
b/drivers/soc/loongson/loongson2_pm.c
new file mode 100644
index 000000000000..287828413d72
--- /dev/null
+++ b/drivers/soc/loongson/loongson2_pm.c
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Loongson-2 PM Support
+ *
+ * Copyright (C) 2023 Loongson Technology Corporation Limited
+ */
+
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/suspend.h>
+#include <linux/interrupt.h>
+#include <linux/pm_wakeirq.h>
+#include <linux/platform_device.h>
+#include <asm/bootinfo.h>
+#include <asm/suspend.h>
+
+#define LOONGSON2_PM1_CNT_REG		0x14
+#define LOONGSON2_PM1_STS_REG		0x0c
+#define LOONGSON2_PM1_ENA_REG		0x10
+#define LOONGSON2_GPE0_STS_REG		0x28
+#define LOONGSON2_GPE0_ENA_REG		0x2c
+
+#define LOONGSON2_PM1_PWRBTN_STS	BIT(8)
+#define LOONGSON2_PM1_PCIEXP_WAKE_STS	BIT(14)
+#define LOONGSON2_PM1_WAKE_STS		BIT(15)
+#define LOONGSON2_PM1_CNT_INT_EN	BIT(0)
+#define LOONGSON2_PM1_PWRBTN_EN		LOONGSON2_PM1_PWRBTN_STS
+
+static struct loongson2_pm {
+	void __iomem			*base;
+	struct input_dev		*dev;
+	bool				suspended;
+} loongson2_pm;
+
+#define loongson2_pm_readw(reg)		readw(loongson2_pm.base + reg)
+#define loongson2_pm_readl(reg)		readl(loongson2_pm.base + reg)
+#define loongson2_pm_writew(val, reg)	writew(val, loongson2_pm.base + reg)
+#define loongson2_pm_writel(val, reg)	writel(val, loongson2_pm.base + reg)
+
+static void loongson2_pm_status_clear(void)
+{
+	u16 value;
+
+	value = loongson2_pm_readw(LOONGSON2_PM1_STS_REG);
+	value |= (LOONGSON2_PM1_PWRBTN_STS | LOONGSON2_PM1_PCIEXP_WAKE_STS |
+		  LOONGSON2_PM1_WAKE_STS);
+	loongson2_pm_writew(value, LOONGSON2_PM1_STS_REG);
+	loongson2_pm_writel(loongson2_pm_readl(LOONGSON2_GPE0_STS_REG),
+			    LOONGSON2_GPE0_STS_REG);
+}
+
+static void loongson2_power_button_irq_enable(void)
+{
+	u16 value;
+
+	value = loongson2_pm_readw(LOONGSON2_PM1_CNT_REG);
+	value |= LOONGSON2_PM1_CNT_INT_EN;
+	loongson2_pm_writew(value, LOONGSON2_PM1_CNT_REG);
+
+	value = loongson2_pm_readw(LOONGSON2_PM1_ENA_REG);
+	value |= LOONGSON2_PM1_PWRBTN_EN;
+	loongson2_pm_writew(value, LOONGSON2_PM1_ENA_REG);
+}
+
+static int loongson2_suspend_enter(suspend_state_t state)
+{
+	loongson2_pm_status_clear();
+	loongarch_common_suspend();
+	loongarch_suspend_enter();
+	loongarch_common_resume();
+	loongson2_power_button_irq_enable();
+	pm_set_resume_via_firmware();
+
+	return 0;
+}
+
+static int loongson2_suspend_begin(suspend_state_t state)
+{
+	pm_set_suspend_via_firmware();
+
+	return 0;
+}
+
+static int loongson2_suspend_valid_state(suspend_state_t state)
+{
+	if (state == PM_SUSPEND_MEM)
+		return 1;
+
+	return 0;
+}
+
+static const struct platform_suspend_ops loongson2_suspend_ops = {
+	.valid	= loongson2_suspend_valid_state,
+	.begin	= loongson2_suspend_begin,
+	.enter	= loongson2_suspend_enter,
+};
+
+static int loongson2_power_button_init(struct device *dev, int irq)
+{
+	int ret;
+	struct input_dev *button;
+
+	button = input_allocate_device();
+	if (!dev)
+		return -ENOMEM;
+
+	button->name = "Power Button";
+	button->phys = "pm/button/input0";
+	button->id.bustype = BUS_HOST;
+	button->dev.parent = NULL;
+	input_set_capability(button, EV_KEY, KEY_POWER);
+
+	ret = input_register_device(button);
+	if (ret)
+		goto free_dev;
+
+	dev_pm_set_wake_irq(&button->dev, irq);
+	device_set_wakeup_capable(&button->dev, true);
+	device_set_wakeup_enable(&button->dev, true);
+
+	loongson2_pm.dev = button;
+	dev_info(dev, "Power Button: Init successful!\n");
+
+	return 0;
+
+free_dev:
+	input_free_device(button);
+
+	return ret;
+}
+
+static irqreturn_t loongson2_pm_irq_handler(int irq, void *dev_id)
+{
+	u16 status = loongson2_pm_readw(LOONGSON2_PM1_STS_REG);
+
+	if (!loongson2_pm.suspended && (status & LOONGSON2_PM1_PWRBTN_STS)) {
+		pr_info("Power Button pressed...\n");
+		input_report_key(loongson2_pm.dev, KEY_POWER, 1);
+		input_sync(loongson2_pm.dev);
+		input_report_key(loongson2_pm.dev, KEY_POWER, 0);
+		input_sync(loongson2_pm.dev);
+	}
+
+	loongson2_pm_status_clear();
+
+	return IRQ_HANDLED;
+}
+
+static int __maybe_unused loongson2_pm_suspend(struct device *dev)
+{
+	loongson2_pm.suspended = true;
+
+	return 0;
+}
+
+static int __maybe_unused loongson2_pm_resume(struct device *dev)
+{
+	loongson2_pm.suspended = false;
+
+	return 0;
+}
+static SIMPLE_DEV_PM_OPS(loongson2_pm_ops, loongson2_pm_suspend, 
loongson2_pm_resume);
+
+static int loongson2_pm_probe(struct platform_device *pdev)
+{
+	int irq, retval;
+	u32 suspend_addr;
+	struct device *dev = &pdev->dev;
+
+	loongson2_pm.base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(loongson2_pm.base))
+		return PTR_ERR(loongson2_pm.base);
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
+
+	if (!device_property_read_u32(dev, "suspend-address", &suspend_addr))
+		loongson_sysconf.suspend_addr = (u64)phys_to_virt(suspend_addr);
+	else
+		dev_err(dev, "No suspend-address, could not support S3!\n");
+
+	if (loongson2_power_button_init(dev, irq))
+		return -EINVAL;
+
+	retval = devm_request_irq(&pdev->dev, irq, loongson2_pm_irq_handler,
+				  IRQF_SHARED, "pm_irq", &loongson2_pm);
+	if (retval)
+		return retval;
+
+	loongson2_power_button_irq_enable();
+	loongson2_pm_status_clear();
+
+	if (loongson_sysconf.suspend_addr)
+		suspend_set_ops(&loongson2_suspend_ops);
+
+	return 0;
+}
+
+static const struct of_device_id loongson2_pm_match[] = {
+	{ .compatible = "loongson,ls2k1000-pmc", },
+	{},
+};
+
+static struct platform_driver loongson2_pm_driver = {
+	.driver = {
+		.name = "ls2k-pm",
+		.pm = &loongson2_pm_ops,
+		.of_match_table = loongson2_pm_match,
+	},
+	.probe = loongson2_pm_probe,
+};
+module_platform_driver(loongson2_pm_driver);
+
+MODULE_DESCRIPTION("Loongson-2 PM driver");
+MODULE_LICENSE("GPL");
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 3/3] soc: loongson2_pm: add power management support
  2023-06-15  9:37 ` [PATCH v3 3/3] soc: loongson2_pm: add power management support zhuyinbo
@ 2023-06-15 10:00   ` Huacai Chen
  2023-06-15 11:15     ` zhuyinbo
  2023-06-16  6:52   ` Conor Dooley
  1 sibling, 1 reply; 21+ messages in thread
From: Huacai Chen @ 2023-06-15 10:00 UTC (permalink / raw)
  To: zhuyinbo
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, WANG Xuerui,
	Rafael J . Wysocki, Pavel Machek, Marc Zyngier, Arnd Bergmann,
	linux-pm, devicetree, linux-kernel, loongarch, Jianmin Lv,
	wanghongliang, Liu Peibao, loongson-kernel

Hi, Yinbo,

I'm sorry I still have some comments.

On Thu, Jun 15, 2023 at 5:37 PM zhuyinbo <zhuyinbo@loongson.cn> wrote:
>
>  From 6edcb9d6a1b18ccbecaf283b4f543afc9e7126d6 Mon Sep 17 00:00:00 2001
> From: Yinbo Zhu <zhuyinbo@loongson.cn>
> Date: Tue, 18 Apr 2023 14:18:00 +0800
> Subject: [PATCH v3 3/3] soc: loongson2_pm: add power management support
>
> The Loongson-2's power management controller was ACPI, supports ACPI
> S2Idle (Suspend To Idle), ACPI S3 (Suspend To RAM), ACPI S4 (Suspend To
> Disk), ACPI S5 (Soft Shutdown) and supports multiple wake-up methods
> (USB, GMAC, PWRBTN, etc.). This driver was to add power management
> controller support that base on dts for Loongson-2 series SoCs.
>
> Signed-off-by: Liu Yun <liuyun@loongson.cn>
> Signed-off-by: Liu Peibao <liupeibao@loongson.cn>
> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
> ---
>   MAINTAINERS                         |   1 +
>   drivers/soc/loongson/Kconfig        |  10 ++
>   drivers/soc/loongson/Makefile       |   1 +
>   drivers/soc/loongson/loongson2_pm.c | 218 ++++++++++++++++++++++++++++
>   4 files changed, 230 insertions(+)
>   create mode 100644 drivers/soc/loongson/loongson2_pm.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bcd05f1fa5c1..7c4ad0cbaeff 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12195,6 +12195,7 @@ M:      Yinbo Zhu <zhuyinbo@loongson.cn>
>   L:    linux-pm@vger.kernel.org
>   S:    Maintained
>   F:    Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> +F:     drivers/soc/loongson/loongson2_pm.c
>
>   LOONGSON-2 SOC SERIES PINCTRL DRIVER
>   M:    zhanghongchen <zhanghongchen@loongson.cn>
> diff --git a/drivers/soc/loongson/Kconfig b/drivers/soc/loongson/Kconfig
> index 707f56358dc4..4f3ce9eb7520 100644
> --- a/drivers/soc/loongson/Kconfig
> +++ b/drivers/soc/loongson/Kconfig
> @@ -16,3 +16,13 @@ config LOONGSON2_GUTS
>          SoCs. Initially only reading SVR and registering soc device are
>          supported. Other guts accesses, such as reading firmware configuration
>          by default, should eventually be added into this driver as well.
> +
> +config LOONGSON2_PM
> +       bool "Loongson-2 SoC Power Management Controller Driver"
> +       depends on LOONGARCH && OF
> +       help
> +        The Loongson-2's power management controller was ACPI, supports ACPI
> +        S2Idle (Suspend To Idle), ACPI S3 (Suspend To RAM), ACPI S4 (Suspend To
> +        Disk), ACPI S5 (Soft Shutdown) and supports multiple wake-up methods
> +        (USB, GMAC, PWRBTN, etc.). This driver was to add power management
> +        controller support that base on dts for Loongson-2 series SoCs.
> diff --git a/drivers/soc/loongson/Makefile b/drivers/soc/loongson/Makefile
> index 263c486df638..4118f50f55e2 100644
> --- a/drivers/soc/loongson/Makefile
> +++ b/drivers/soc/loongson/Makefile
> @@ -4,3 +4,4 @@
>   #
>
>   obj-$(CONFIG_LOONGSON2_GUTS)          += loongson2_guts.o
> +obj-$(CONFIG_LOONGSON2_PM)             += loongson2_pm.o
> diff --git a/drivers/soc/loongson/loongson2_pm.c
> b/drivers/soc/loongson/loongson2_pm.c
> new file mode 100644
> index 000000000000..287828413d72
> --- /dev/null
> +++ b/drivers/soc/loongson/loongson2_pm.c
> @@ -0,0 +1,218 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Loongson-2 PM Support
> + *
> + * Copyright (C) 2023 Loongson Technology Corporation Limited
> + */
> +
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/init.h>
> +#include <linux/input.h>
> +#include <linux/suspend.h>
> +#include <linux/interrupt.h>
> +#include <linux/pm_wakeirq.h>
> +#include <linux/platform_device.h>
> +#include <asm/bootinfo.h>
> +#include <asm/suspend.h>
> +
> +#define LOONGSON2_PM1_CNT_REG          0x14
> +#define LOONGSON2_PM1_STS_REG          0x0c
> +#define LOONGSON2_PM1_ENA_REG          0x10
> +#define LOONGSON2_GPE0_STS_REG         0x28
> +#define LOONGSON2_GPE0_ENA_REG         0x2c
> +
> +#define LOONGSON2_PM1_PWRBTN_STS       BIT(8)
> +#define LOONGSON2_PM1_PCIEXP_WAKE_STS  BIT(14)
> +#define LOONGSON2_PM1_WAKE_STS         BIT(15)
> +#define LOONGSON2_PM1_CNT_INT_EN       BIT(0)
> +#define LOONGSON2_PM1_PWRBTN_EN                LOONGSON2_PM1_PWRBTN_STS
> +
> +static struct loongson2_pm {
> +       void __iomem                    *base;
> +       struct input_dev                *dev;
> +       bool                            suspended;
> +} loongson2_pm;
> +
> +#define loongson2_pm_readw(reg)                readw(loongson2_pm.base + reg)
> +#define loongson2_pm_readl(reg)                readl(loongson2_pm.base + reg)
> +#define loongson2_pm_writew(val, reg)  writew(val, loongson2_pm.base + reg)
> +#define loongson2_pm_writel(val, reg)  writel(val, loongson2_pm.base + reg)
> +
> +static void loongson2_pm_status_clear(void)
> +{
> +       u16 value;
> +
> +       value = loongson2_pm_readw(LOONGSON2_PM1_STS_REG);
> +       value |= (LOONGSON2_PM1_PWRBTN_STS | LOONGSON2_PM1_PCIEXP_WAKE_STS |
> +                 LOONGSON2_PM1_WAKE_STS);
> +       loongson2_pm_writew(value, LOONGSON2_PM1_STS_REG);
> +       loongson2_pm_writel(loongson2_pm_readl(LOONGSON2_GPE0_STS_REG),
> +                           LOONGSON2_GPE0_STS_REG);
Long-line warnings is removed in latest kernel, so you don't need to split here.

> +}
> +
> +static void loongson2_power_button_irq_enable(void)

Using loongson2_pm_irq_enable is a little better.

> +{
> +       u16 value;
> +
> +       value = loongson2_pm_readw(LOONGSON2_PM1_CNT_REG);
> +       value |= LOONGSON2_PM1_CNT_INT_EN;
> +       loongson2_pm_writew(value, LOONGSON2_PM1_CNT_REG);
> +
> +       value = loongson2_pm_readw(LOONGSON2_PM1_ENA_REG);
> +       value |= LOONGSON2_PM1_PWRBTN_EN;
> +       loongson2_pm_writew(value, LOONGSON2_PM1_ENA_REG);
> +}
> +
> +static int loongson2_suspend_enter(suspend_state_t state)
> +{
> +       loongson2_pm_status_clear();
> +       loongarch_common_suspend();
> +       loongarch_suspend_enter();
> +       loongarch_common_resume();
> +       loongson2_power_button_irq_enable();
> +       pm_set_resume_via_firmware();
> +
> +       return 0;
> +}
> +
> +static int loongson2_suspend_begin(suspend_state_t state)
> +{
> +       pm_set_suspend_via_firmware();
> +
> +       return 0;
> +}
> +
> +static int loongson2_suspend_valid_state(suspend_state_t state)
> +{
> +       if (state == PM_SUSPEND_MEM)
> +               return 1;
> +
> +       return 0;
"return (state == PM_SUSPEND_MEM)" is enough.

Huacai
> +}
> +
> +static const struct platform_suspend_ops loongson2_suspend_ops = {
> +       .valid  = loongson2_suspend_valid_state,
> +       .begin  = loongson2_suspend_begin,
> +       .enter  = loongson2_suspend_enter,
> +};
> +
> +static int loongson2_power_button_init(struct device *dev, int irq)
> +{
> +       int ret;
> +       struct input_dev *button;
> +
> +       button = input_allocate_device();
> +       if (!dev)
> +               return -ENOMEM;
> +
> +       button->name = "Power Button";
> +       button->phys = "pm/button/input0";
> +       button->id.bustype = BUS_HOST;
> +       button->dev.parent = NULL;
> +       input_set_capability(button, EV_KEY, KEY_POWER);
> +
> +       ret = input_register_device(button);
> +       if (ret)
> +               goto free_dev;
> +
> +       dev_pm_set_wake_irq(&button->dev, irq);
> +       device_set_wakeup_capable(&button->dev, true);
> +       device_set_wakeup_enable(&button->dev, true);
> +
> +       loongson2_pm.dev = button;
> +       dev_info(dev, "Power Button: Init successful!\n");
> +
> +       return 0;
> +
> +free_dev:
> +       input_free_device(button);
> +
> +       return ret;
> +}
> +
> +static irqreturn_t loongson2_pm_irq_handler(int irq, void *dev_id)
> +{
> +       u16 status = loongson2_pm_readw(LOONGSON2_PM1_STS_REG);
> +
> +       if (!loongson2_pm.suspended && (status & LOONGSON2_PM1_PWRBTN_STS)) {
> +               pr_info("Power Button pressed...\n");
> +               input_report_key(loongson2_pm.dev, KEY_POWER, 1);
> +               input_sync(loongson2_pm.dev);
> +               input_report_key(loongson2_pm.dev, KEY_POWER, 0);
> +               input_sync(loongson2_pm.dev);
> +       }
> +
> +       loongson2_pm_status_clear();
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static int __maybe_unused loongson2_pm_suspend(struct device *dev)
> +{
> +       loongson2_pm.suspended = true;
> +
> +       return 0;
> +}
> +
> +static int __maybe_unused loongson2_pm_resume(struct device *dev)
> +{
> +       loongson2_pm.suspended = false;
> +
> +       return 0;
> +}
> +static SIMPLE_DEV_PM_OPS(loongson2_pm_ops, loongson2_pm_suspend,
> loongson2_pm_resume);
> +
> +static int loongson2_pm_probe(struct platform_device *pdev)
> +{
> +       int irq, retval;
> +       u32 suspend_addr;
> +       struct device *dev = &pdev->dev;
> +
> +       loongson2_pm.base = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(loongson2_pm.base))
> +               return PTR_ERR(loongson2_pm.base);
> +
> +       irq = platform_get_irq(pdev, 0);
> +       if (irq < 0)
> +               return irq;
> +
> +       if (!device_property_read_u32(dev, "suspend-address", &suspend_addr))
> +               loongson_sysconf.suspend_addr = (u64)phys_to_virt(suspend_addr);
> +       else
> +               dev_err(dev, "No suspend-address, could not support S3!\n");
> +
> +       if (loongson2_power_button_init(dev, irq))
> +               return -EINVAL;
> +
> +       retval = devm_request_irq(&pdev->dev, irq, loongson2_pm_irq_handler,
> +                                 IRQF_SHARED, "pm_irq", &loongson2_pm);
> +       if (retval)
> +               return retval;
> +
> +       loongson2_power_button_irq_enable();
> +       loongson2_pm_status_clear();
> +
> +       if (loongson_sysconf.suspend_addr)
> +               suspend_set_ops(&loongson2_suspend_ops);
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id loongson2_pm_match[] = {
> +       { .compatible = "loongson,ls2k1000-pmc", },
> +       {},
> +};
> +
> +static struct platform_driver loongson2_pm_driver = {
> +       .driver = {
> +               .name = "ls2k-pm",
> +               .pm = &loongson2_pm_ops,
> +               .of_match_table = loongson2_pm_match,
> +       },
> +       .probe = loongson2_pm_probe,
> +};
> +module_platform_driver(loongson2_pm_driver);
> +
> +MODULE_DESCRIPTION("Loongson-2 PM driver");
> +MODULE_LICENSE("GPL");
> --
> 2.20.1
>
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 3/3] soc: loongson2_pm: add power management support
  2023-06-15 10:00   ` Huacai Chen
@ 2023-06-15 11:15     ` zhuyinbo
  2023-06-16  1:45       ` zhuyinbo
  0 siblings, 1 reply; 21+ messages in thread
From: zhuyinbo @ 2023-06-15 11:15 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, WANG Xuerui,
	Rafael J . Wysocki, Pavel Machek, Marc Zyngier, Arnd Bergmann,
	linux-pm, devicetree, linux-kernel, loongarch, Jianmin Lv,
	wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo



在 2023/6/15 下午6:00, Huacai Chen 写道:

>> +static void loongson2_pm_status_clear(void)
>> +{
>> +       u16 value;
>> +
>> +       value = loongson2_pm_readw(LOONGSON2_PM1_STS_REG);
>> +       value |= (LOONGSON2_PM1_PWRBTN_STS | LOONGSON2_PM1_PCIEXP_WAKE_STS |
>> +                 LOONGSON2_PM1_WAKE_STS);
>> +       loongson2_pm_writew(value, LOONGSON2_PM1_STS_REG);
>> +       loongson2_pm_writel(loongson2_pm_readl(LOONGSON2_GPE0_STS_REG),
>> +                           LOONGSON2_GPE0_STS_REG);
> Long-line warnings is removed in latest kernel, so you don't need to split here.


okay, I got it.

> 
>> +}
>> +
>> +static void loongson2_power_button_irq_enable(void)
> 
> Using loongson2_pm_irq_enable is a little better.


Indeed, this looks better!  I will use it.

...

>> +static int loongson2_suspend_valid_state(suspend_state_t state)
>> +{
>> +       if (state == PM_SUSPEND_MEM)
>> +               return 1;
>> +
>> +       return 0;
> "return (state == PM_SUSPEND_MEM)" is enough.


okay, I got it.


Thanks,
Yinbo


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 3/3] soc: loongson2_pm: add power management support
  2023-06-15 11:15     ` zhuyinbo
@ 2023-06-16  1:45       ` zhuyinbo
  2023-06-16  1:54         ` Huacai Chen
  0 siblings, 1 reply; 21+ messages in thread
From: zhuyinbo @ 2023-06-16  1:45 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, WANG Xuerui,
	Rafael J . Wysocki, Pavel Machek, Marc Zyngier, Arnd Bergmann,
	linux-pm, devicetree, linux-kernel, loongarch, Jianmin Lv,
	wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo


Hi huacai,


在 2023/6/15 下午7:15, zhuyinbo 写道:
> 
> 
> 在 2023/6/15 下午6:00, Huacai Chen 写道:
> 
>>> +static void loongson2_pm_status_clear(void)
>>> +{
>>> +       u16 value;
>>> +
>>> +       value = loongson2_pm_readw(LOONGSON2_PM1_STS_REG);
>>> +       value |= (LOONGSON2_PM1_PWRBTN_STS | 
>>> LOONGSON2_PM1_PCIEXP_WAKE_STS |
>>> +                 LOONGSON2_PM1_WAKE_STS);
>>> +       loongson2_pm_writew(value, LOONGSON2_PM1_STS_REG);
>>> +       loongson2_pm_writel(loongson2_pm_readl(LOONGSON2_GPE0_STS_REG),
>>> +                           LOONGSON2_GPE0_STS_REG);
>> Long-line warnings is removed in latest kernel, so you don't need to 
>> split here.
> 
> 
> okay, I got it.
> 
>>
>>> +}
>>> +
>>> +static void loongson2_power_button_irq_enable(void)
>>
>> Using loongson2_pm_irq_enable is a little better.
> 
> 

Previously, you suggested that I combine loongson2_pm_irq_enable() and
power button irq enable code as loongson2_power_button_irq_enable, then
I remove the function loongson2_pm_irq_enable, in this case that I won't
be able to call loongson2_pm_irq_enable, so have I misunderstood your
meaning ? or only rename loongson2_power_button_irq_enable as
loongson2_pm_irq_enable ?

Thanks,
Yinbo

> 
> ...
> 
>>> +static int loongson2_suspend_valid_state(suspend_state_t state)
>>> +{
>>> +       if (state == PM_SUSPEND_MEM)
>>> +               return 1;
>>> +
>>> +       return 0;
>> "return (state == PM_SUSPEND_MEM)" is enough.
> 
> 
> okay, I got it.
> 
> 
> Thanks,
> Yinbo
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 3/3] soc: loongson2_pm: add power management support
  2023-06-16  1:45       ` zhuyinbo
@ 2023-06-16  1:54         ` Huacai Chen
  2023-06-16  4:01           ` zhuyinbo
  0 siblings, 1 reply; 21+ messages in thread
From: Huacai Chen @ 2023-06-16  1:54 UTC (permalink / raw)
  To: zhuyinbo
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, WANG Xuerui,
	Rafael J . Wysocki, Pavel Machek, Marc Zyngier, Arnd Bergmann,
	linux-pm, devicetree, linux-kernel, loongarch, Jianmin Lv,
	wanghongliang, Liu Peibao, loongson-kernel

Hi, Yinbo,

On Fri, Jun 16, 2023 at 9:45 AM zhuyinbo <zhuyinbo@loongson.cn> wrote:
>
>
> Hi huacai,
>
>
> 在 2023/6/15 下午7:15, zhuyinbo 写道:
> >
> >
> > 在 2023/6/15 下午6:00, Huacai Chen 写道:
> >
> >>> +static void loongson2_pm_status_clear(void)
> >>> +{
> >>> +       u16 value;
> >>> +
> >>> +       value = loongson2_pm_readw(LOONGSON2_PM1_STS_REG);
> >>> +       value |= (LOONGSON2_PM1_PWRBTN_STS |
> >>> LOONGSON2_PM1_PCIEXP_WAKE_STS |
> >>> +                 LOONGSON2_PM1_WAKE_STS);
> >>> +       loongson2_pm_writew(value, LOONGSON2_PM1_STS_REG);
> >>> +       loongson2_pm_writel(loongson2_pm_readl(LOONGSON2_GPE0_STS_REG),
> >>> +                           LOONGSON2_GPE0_STS_REG);
> >> Long-line warnings is removed in latest kernel, so you don't need to
> >> split here.
> >
> >
> > okay, I got it.
> >
> >>
> >>> +}
> >>> +
> >>> +static void loongson2_power_button_irq_enable(void)
> >>
> >> Using loongson2_pm_irq_enable is a little better.
> >
> >
>
> Previously, you suggested that I combine loongson2_pm_irq_enable() and
> power button irq enable code as loongson2_power_button_irq_enable, then
> I remove the function loongson2_pm_irq_enable, in this case that I won't
> be able to call loongson2_pm_irq_enable, so have I misunderstood your
> meaning ? or only rename loongson2_power_button_irq_enable as
> loongson2_pm_irq_enable ?
I'm very sorry for that. At first I only wanted to combine two
functions, but then I found the name  loongson2_pm_irq_enable is
better. So just rename is OK. Thanks.

Huacai
>
> Thanks,
> Yinbo
>
> >
> > ...
> >
> >>> +static int loongson2_suspend_valid_state(suspend_state_t state)
> >>> +{
> >>> +       if (state == PM_SUSPEND_MEM)
> >>> +               return 1;
> >>> +
> >>> +       return 0;
> >> "return (state == PM_SUSPEND_MEM)" is enough.
> >
> >
> > okay, I got it.
> >
> >
> > Thanks,
> > Yinbo
> >
>
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 3/3] soc: loongson2_pm: add power management support
  2023-06-16  1:54         ` Huacai Chen
@ 2023-06-16  4:01           ` zhuyinbo
  0 siblings, 0 replies; 21+ messages in thread
From: zhuyinbo @ 2023-06-16  4:01 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Rob Herring, Krzysztof Kozlowski, zhuyinbo, Conor Dooley,
	WANG Xuerui, Rafael J . Wysocki, Pavel Machek, Marc Zyngier,
	Arnd Bergmann, linux-pm, devicetree, linux-kernel, loongarch,
	Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel



在 2023/6/16 上午9:54, Huacai Chen 写道:
> Hi, Yinbo,
> 
> On Fri, Jun 16, 2023 at 9:45 AM zhuyinbo <zhuyinbo@loongson.cn> wrote:

...

>>>
>>>
>>> okay, I got it.
>>>
>>>>
>>>>> +}
>>>>> +
>>>>> +static void loongson2_power_button_irq_enable(void)
>>>>
>>>> Using loongson2_pm_irq_enable is a little better.
>>>
>>>
>>
>> Previously, you suggested that I combine loongson2_pm_irq_enable() and
>> power button irq enable code as loongson2_power_button_irq_enable, then
>> I remove the function loongson2_pm_irq_enable, in this case that I won't
>> be able to call loongson2_pm_irq_enable, so have I misunderstood your
>> meaning ? or only rename loongson2_power_button_irq_enable as
>> loongson2_pm_irq_enable ?
> I'm very sorry for that. At first I only wanted to combine two
> functions, but then I found the name  loongson2_pm_irq_enable is
> better. So just rename is OK. Thanks.
> 


okay, I got it.

Thanks,
Yinbo


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 1/3] loongarch: export some arch-specific pm interfaces
  2023-06-15  9:17 ` [PATCH v3 1/3] loongarch: export some arch-specific pm interfaces Yinbo Zhu
@ 2023-06-16  4:04   ` Huacai Chen
  2023-06-16  6:06     ` zhuyinbo
  0 siblings, 1 reply; 21+ messages in thread
From: Huacai Chen @ 2023-06-16  4:04 UTC (permalink / raw)
  To: Yinbo Zhu
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, WANG Xuerui,
	Rafael J . Wysocki, Pavel Machek, Marc Zyngier, Arnd Bergmann,
	linux-pm, devicetree, linux-kernel, loongarch, Jianmin Lv,
	wanghongliang, Liu Peibao, loongson-kernel

Hi, Yinbo,

I think this patch should go through the loongarch tree and the others
to go through the soc tree, so I just applied this one. The next
version you can only send the other two, thanks.

Huacai

On Thu, Jun 15, 2023 at 5:18 PM Yinbo Zhu <zhuyinbo@loongson.cn> wrote:
>
> Some power management controllers need to support DTS and will use
> the suspend interface thus this patch was to export such interface
> for their use.
>
> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
> ---
>  arch/loongarch/include/asm/acpi.h    |  3 +--
>  arch/loongarch/include/asm/suspend.h | 10 ++++++++++
>  arch/loongarch/power/suspend.c       |  8 ++++----
>  3 files changed, 15 insertions(+), 6 deletions(-)
>  create mode 100644 arch/loongarch/include/asm/suspend.h
>
> diff --git a/arch/loongarch/include/asm/acpi.h b/arch/loongarch/include/asm/acpi.h
> index 976a810352c6..1d7810798c08 100644
> --- a/arch/loongarch/include/asm/acpi.h
> +++ b/arch/loongarch/include/asm/acpi.h
> @@ -8,6 +8,7 @@
>  #ifndef _ASM_LOONGARCH_ACPI_H
>  #define _ASM_LOONGARCH_ACPI_H
>
> +#include <asm/suspend.h>
>  #ifdef CONFIG_ACPI
>  extern int acpi_strict;
>  extern int acpi_disabled;
> @@ -37,12 +38,10 @@ extern struct list_head acpi_wakeup_device_list;
>
>  extern int loongarch_acpi_suspend(void);
>  extern int (*acpi_suspend_lowlevel)(void);
> -extern void loongarch_suspend_enter(void);
>
>  static inline unsigned long acpi_get_wakeup_address(void)
>  {
>  #ifdef CONFIG_SUSPEND
> -       extern void loongarch_wakeup_start(void);
>         return (unsigned long)loongarch_wakeup_start;
>  #endif
>         return 0UL;
> diff --git a/arch/loongarch/include/asm/suspend.h b/arch/loongarch/include/asm/suspend.h
> new file mode 100644
> index 000000000000..fc64089fefaa
> --- /dev/null
> +++ b/arch/loongarch/include/asm/suspend.h
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __ASM_SUSPEND_H
> +#define __ASM_SUSPEND_H
> +
> +void loongarch_common_resume(void);
> +void loongarch_common_suspend(void);
> +void loongarch_suspend_enter(void);
> +void loongarch_wakeup_start(void);
> +
> +#endif
> diff --git a/arch/loongarch/power/suspend.c b/arch/loongarch/power/suspend.c
> index 5e19733e5e05..166d9e06a64b 100644
> --- a/arch/loongarch/power/suspend.c
> +++ b/arch/loongarch/power/suspend.c
> @@ -27,7 +27,7 @@ struct saved_registers {
>  };
>  static struct saved_registers saved_regs;
>
> -static void arch_common_suspend(void)
> +void loongarch_common_suspend(void)
>  {
>         save_counter();
>         saved_regs.pgd = csr_read64(LOONGARCH_CSR_PGDL);
> @@ -40,7 +40,7 @@ static void arch_common_suspend(void)
>         loongarch_suspend_addr = loongson_sysconf.suspend_addr;
>  }
>
> -static void arch_common_resume(void)
> +void loongarch_common_resume(void)
>  {
>         sync_counter();
>         local_flush_tlb_all();
> @@ -62,12 +62,12 @@ int loongarch_acpi_suspend(void)
>         enable_gpe_wakeup();
>         enable_pci_wakeup();
>
> -       arch_common_suspend();
> +       loongarch_common_suspend();
>
>         /* processor specific suspend */
>         loongarch_suspend_enter();
>
> -       arch_common_resume();
> +       loongarch_common_resume();
>
>         return 0;
>  }
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 1/3] loongarch: export some arch-specific pm interfaces
  2023-06-16  4:04   ` Huacai Chen
@ 2023-06-16  6:06     ` zhuyinbo
  0 siblings, 0 replies; 21+ messages in thread
From: zhuyinbo @ 2023-06-16  6:06 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, WANG Xuerui,
	Rafael J . Wysocki, Pavel Machek, Marc Zyngier, Arnd Bergmann,
	linux-pm, devicetree, linux-kernel, loongarch, Jianmin Lv,
	wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo



在 2023/6/16 下午12:04, Huacai Chen 写道:
> Hi, Yinbo,
> 
> I think this patch should go through the loongarch tree and the others
> to go through the soc tree, so I just applied this one. The next
> version you can only send the other two, thanks. 


okay, I got it.

Thanks,
Yinbo


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 3/3] soc: loongson2_pm: add power management support
  2023-06-15  9:37 ` [PATCH v3 3/3] soc: loongson2_pm: add power management support zhuyinbo
  2023-06-15 10:00   ` Huacai Chen
@ 2023-06-16  6:52   ` Conor Dooley
  2023-06-16  7:08     ` zhuyinbo
  1 sibling, 1 reply; 21+ messages in thread
From: Conor Dooley @ 2023-06-16  6:52 UTC (permalink / raw)
  To: zhuyinbo
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Huacai Chen,
	WANG Xuerui, Rafael J . Wysocki, Pavel Machek, Marc Zyngier,
	Arnd Bergmann, linux-pm, devicetree, linux-kernel, loongarch,
	Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel

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

On Thu, Jun 15, 2023 at 05:37:18PM +0800, zhuyinbo wrote:
> From 6edcb9d6a1b18ccbecaf283b4f543afc9e7126d6 Mon Sep 17 00:00:00 2001
> From: Yinbo Zhu <zhuyinbo@loongson.cn>
> Date: Tue, 18 Apr 2023 14:18:00 +0800
> Subject: [PATCH v3 3/3] soc: loongson2_pm: add power management support
> 
> The Loongson-2's power management controller was ACPI, supports ACPI
> S2Idle (Suspend To Idle), ACPI S3 (Suspend To RAM), ACPI S4 (Suspend To
> Disk), ACPI S5 (Soft Shutdown) and supports multiple wake-up methods
> (USB, GMAC, PWRBTN, etc.). This driver was to add power management
> controller support that base on dts for Loongson-2 series SoCs.
> 
> Signed-off-by: Liu Yun <liuyun@loongson.cn>
> Signed-off-by: Liu Peibao <liupeibao@loongson.cn>
> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>

3 SoBs, should 2 of these have corresponding co-developed-bys?

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] soc: dt-bindings: add loongson-2 pm
  2023-06-15  9:17 ` [PATCH v3 2/3] soc: dt-bindings: add loongson-2 pm Yinbo Zhu
@ 2023-06-16  6:58   ` Conor Dooley
  2023-06-16  7:53     ` zhuyinbo
  2023-06-16  8:03   ` Krzysztof Kozlowski
  2023-06-16 15:17   ` Rob Herring
  2 siblings, 1 reply; 21+ messages in thread
From: Conor Dooley @ 2023-06-16  6:58 UTC (permalink / raw)
  To: Yinbo Zhu
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Huacai Chen,
	WANG Xuerui, Rafael J . Wysocki, Pavel Machek, Marc Zyngier,
	Arnd Bergmann, linux-pm, devicetree, linux-kernel, loongarch,
	Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel

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

Hey,

Rob, could you take a look at this please? On v2 while you were away I
was kinda struggling w/ suspend-address & whether it made sense.

The v2 & v1 are here:
https://lore.kernel.org/all/20230522093156.7108-3-zhuyinbo@loongson.cn/
https://lore.kernel.org/all/20230517073149.31980-3-zhuyinbo@loongson.cn/

On Thu, Jun 15, 2023 at 05:17:56PM +0800, Yinbo Zhu wrote:
> Add the Loongson-2 SoC Power Management Controller binding with DT
> schema format using json-schema.
> 
> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
> ---
>  .../soc/loongson/loongson,ls2k-pmc.yaml       | 53 +++++++++++++++++++
>  MAINTAINERS                                   |  6 +++
>  2 files changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> new file mode 100644
> index 000000000000..32499bd10f8c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml

The filename should ideally match one of the compatibles.

> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/soc/loongson/loongson,ls2k-pmc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Loongson-2 Power Manager controller
> +
> +maintainers:
> +  - Yinbo Zhu <zhuyinbo@loongson.cn>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - loongson,ls2k1000-pmc
> +              - loongson,ls2k0500-pmc

I notice the driver only supports one of these two. Is there a reason
for that?

> +          - const: syscon
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  suspend-address:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      The "suspend-address" is a deep sleep state (Suspend To RAM)
> +      firmware entry address which was jumped from kernel and it's
> +      value was dependent on specific platform firmware code. In
> +      addition, the PM need according to it to indicate that current
> +      SoC whether support Suspend To RAM.
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    pmc: pm@1fe27000 {
       ^^^

nit: this label isn't used, so you can drop it.

Cheers,
Conor.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 3/3] soc: loongson2_pm: add power management support
  2023-06-16  6:52   ` Conor Dooley
@ 2023-06-16  7:08     ` zhuyinbo
  0 siblings, 0 replies; 21+ messages in thread
From: zhuyinbo @ 2023-06-16  7:08 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Huacai Chen,
	WANG Xuerui, Rafael J . Wysocki, Pavel Machek, Marc Zyngier,
	Arnd Bergmann, linux-pm, devicetree, linux-kernel, loongarch,
	Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo



在 2023/6/16 下午2:52, Conor Dooley 写道:
> On Thu, Jun 15, 2023 at 05:37:18PM +0800, zhuyinbo wrote:
>>  From 6edcb9d6a1b18ccbecaf283b4f543afc9e7126d6 Mon Sep 17 00:00:00 2001
>> From: Yinbo Zhu <zhuyinbo@loongson.cn>
>> Date: Tue, 18 Apr 2023 14:18:00 +0800
>> Subject: [PATCH v3 3/3] soc: loongson2_pm: add power management support
>>
>> The Loongson-2's power management controller was ACPI, supports ACPI
>> S2Idle (Suspend To Idle), ACPI S3 (Suspend To RAM), ACPI S4 (Suspend To
>> Disk), ACPI S5 (Soft Shutdown) and supports multiple wake-up methods
>> (USB, GMAC, PWRBTN, etc.). This driver was to add power management
>> controller support that base on dts for Loongson-2 series SoCs.
>>
>> Signed-off-by: Liu Yun <liuyun@loongson.cn>
>> Signed-off-by: Liu Peibao <liupeibao@loongson.cn>
>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
> 
> 3 SoBs, should 2 of these have corresponding co-developed-bys?


okay, I will add co-developed-bys.

Thanks,
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] soc: dt-bindings: add loongson-2 pm
  2023-06-16  6:58   ` Conor Dooley
@ 2023-06-16  7:53     ` zhuyinbo
  2023-06-16  8:03       ` Conor Dooley
  0 siblings, 1 reply; 21+ messages in thread
From: zhuyinbo @ 2023-06-16  7:53 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Huacai Chen,
	WANG Xuerui, Rafael J . Wysocki, Pavel Machek, Marc Zyngier,
	Arnd Bergmann, linux-pm, devicetree, linux-kernel, loongarch,
	Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo



在 2023/6/16 下午2:58, Conor Dooley 写道:
> Hey,
> 
> Rob, could you take a look at this please? On v2 while you were away I
> was kinda struggling w/ suspend-address & whether it made sense.
> 
> The v2 & v1 are here:
> https://lore.kernel.org/all/20230522093156.7108-3-zhuyinbo@loongson.cn/
> https://lore.kernel.org/all/20230517073149.31980-3-zhuyinbo@loongson.cn/
> 
> On Thu, Jun 15, 2023 at 05:17:56PM +0800, Yinbo Zhu wrote:
>> Add the Loongson-2 SoC Power Management Controller binding with DT
>> schema format using json-schema.
>>
>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
>> ---
>>   .../soc/loongson/loongson,ls2k-pmc.yaml       | 53 +++++++++++++++++++
>>   MAINTAINERS                                   |  6 +++
>>   2 files changed, 59 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
>> new file mode 100644
>> index 000000000000..32499bd10f8c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> 
> The filename should ideally match one of the compatibles.


I learn about that yaml file name need match this compatible, but here
using a specific compatible as the name of the yaml file seems a bit
inappropriate . After all, this yaml file needs to cover lots of ls2k
series SoC rather than a specific SoC, and the yaml file naming in
kernel drivers is basically the same that use cover a series SoC's way.

> 
>> @@ -0,0 +1,53 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/soc/loongson/loongson,ls2k-pmc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Loongson-2 Power Manager controller
>> +
>> +maintainers:
>> +  - Yinbo Zhu <zhuyinbo@loongson.cn>
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - items:
>> +          - enum:
>> +              - loongson,ls2k1000-pmc
>> +              - loongson,ls2k0500-pmc
> 
> I notice the driver only supports one of these two. Is there a reason
> for that?


The driver can support both of the above, and I will add another.

> 
>> +          - const: syscon
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  suspend-address:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      The "suspend-address" is a deep sleep state (Suspend To RAM)
>> +      firmware entry address which was jumped from kernel and it's
>> +      value was dependent on specific platform firmware code. In
>> +      addition, the PM need according to it to indicate that current
>> +      SoC whether support Suspend To RAM.
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +
>> +    pmc: pm@1fe27000 {
>         ^^^
> 
> nit: this label isn't used, so you can drop it.


This lable need to be used by poweroff and reboot node but I don't add
these node that reference pmc here.

Thanks,
Yinbo


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] soc: dt-bindings: add loongson-2 pm
  2023-06-16  7:53     ` zhuyinbo
@ 2023-06-16  8:03       ` Conor Dooley
  2023-06-16  8:47         ` zhuyinbo
  0 siblings, 1 reply; 21+ messages in thread
From: Conor Dooley @ 2023-06-16  8:03 UTC (permalink / raw)
  To: zhuyinbo
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Huacai Chen,
	WANG Xuerui, Rafael J . Wysocki, Pavel Machek, Marc Zyngier,
	Arnd Bergmann, linux-pm, devicetree, linux-kernel, loongarch,
	Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel

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

On Fri, Jun 16, 2023 at 03:53:38PM +0800, zhuyinbo wrote:
> 在 2023/6/16 下午2:58, Conor Dooley 写道:
> > 
> > Rob, could you take a look at this please? On v2 while you were away I
> > was kinda struggling w/ suspend-address & whether it made sense.
> > 
> > The v2 & v1 are here:
> > https://lore.kernel.org/all/20230522093156.7108-3-zhuyinbo@loongson.cn/
> > https://lore.kernel.org/all/20230517073149.31980-3-zhuyinbo@loongson.cn/
> > 
> > On Thu, Jun 15, 2023 at 05:17:56PM +0800, Yinbo Zhu wrote:
> > > Add the Loongson-2 SoC Power Management Controller binding with DT
> > > schema format using json-schema.
> > > 
> > > Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
> > > ---
> > >   .../soc/loongson/loongson,ls2k-pmc.yaml       | 53 +++++++++++++++++++
> > >   MAINTAINERS                                   |  6 +++
> > >   2 files changed, 59 insertions(+)
> > >   create mode 100644 Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> > > new file mode 100644
> > > index 000000000000..32499bd10f8c
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> > 
> > The filename should ideally match one of the compatibles.
> 
> 
> I learn about that yaml file name need match this compatible, but here
> using a specific compatible as the name of the yaml file seems a bit
> inappropriate . After all, this yaml file needs to cover lots of ls2k
> series SoC rather than a specific SoC, and the yaml file naming in
> kernel drivers is basically the same that use cover a series SoC's way.
> 
> > 
> > > @@ -0,0 +1,53 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/soc/loongson/loongson,ls2k-pmc.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Loongson-2 Power Manager controller
> > > +
> > > +maintainers:
> > > +  - Yinbo Zhu <zhuyinbo@loongson.cn>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    oneOf:
> > > +      - items:
> > > +          - enum:
> > > +              - loongson,ls2k1000-pmc
> > > +              - loongson,ls2k0500-pmc
> > 
> > I notice the driver only supports one of these two. Is there a reason
> > for that?
> 
> 
> The driver can support both of the above, and I will add another.

The driver only contains
	static const struct of_device_id loongson2_pm_match[] = {
	       { .compatible = "loongson,ls2k1000-pmc", },
	       {},
	};
so it only supports the 2k1000 right now. Are the 2k1000 and 2k0500
compatible with eachother?

> > > +          - const: syscon
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> > > +
> > > +  suspend-address:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description:
> > > +      The "suspend-address" is a deep sleep state (Suspend To RAM)
> > > +      firmware entry address which was jumped from kernel and it's
> > > +      value was dependent on specific platform firmware code. In
> > > +      addition, the PM need according to it to indicate that current
> > > +      SoC whether support Suspend To RAM.
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - interrupts
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/interrupt-controller/irq.h>
> > > +
> > > +    pmc: pm@1fe27000 {
> >         ^^^
> > 
> > nit: this label isn't used, so you can drop it.
> 
> 
> This lable need to be used by poweroff and reboot node but I don't add
> these node that reference pmc here.

Right, in the dts it might need those, but not in the example in the
binding.

Cheers,
Conor.


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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] soc: dt-bindings: add loongson-2 pm
  2023-06-15  9:17 ` [PATCH v3 2/3] soc: dt-bindings: add loongson-2 pm Yinbo Zhu
  2023-06-16  6:58   ` Conor Dooley
@ 2023-06-16  8:03   ` Krzysztof Kozlowski
  2023-06-16  8:51     ` zhuyinbo
  2023-06-16 15:17   ` Rob Herring
  2 siblings, 1 reply; 21+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-16  8:03 UTC (permalink / raw)
  To: Yinbo Zhu, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Huacai Chen, WANG Xuerui, Rafael J . Wysocki, Pavel Machek,
	Marc Zyngier, Arnd Bergmann, linux-pm, devicetree, linux-kernel,
	loongarch
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel

On 15/06/2023 11:17, Yinbo Zhu wrote:
> Add the Loongson-2 SoC Power Management Controller binding with DT
> schema format using json-schema.
> 
> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
> ---
>  .../soc/loongson/loongson,ls2k-pmc.yaml       | 53 +++++++++++++++++++
>  MAINTAINERS                                   |  6 +++
>  2 files changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> new file mode 100644
> index 000000000000..32499bd10f8c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/soc/loongson/loongson,ls2k-pmc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Loongson-2 Power Manager controller
> +
> +maintainers:
> +  - Yinbo Zhu <zhuyinbo@loongson.cn>
> +
> +properties:
> +  compatible:
> +    oneOf:

Drop oneOf, you don't have here many choices.

> +      - items:
> +          - enum:
> +              - loongson,ls2k1000-pmc
> +              - loongson,ls2k0500-pmc
> +          - const: syscon
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  suspend-address:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      The "suspend-address" is a deep sleep state (Suspend To RAM)
> +      firmware entry address which was jumped from kernel and it's
> +      value was dependent on specific platform firmware code. In
> +      addition, the PM need according to it to indicate that current
> +      SoC whether support Suspend To RAM.
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    pmc: pm@1fe27000 {

Node name: system-controller or power-pamanagement

With these two:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] soc: dt-bindings: add loongson-2 pm
  2023-06-16  8:03       ` Conor Dooley
@ 2023-06-16  8:47         ` zhuyinbo
  0 siblings, 0 replies; 21+ messages in thread
From: zhuyinbo @ 2023-06-16  8:47 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Huacai Chen,
	WANG Xuerui, Rafael J . Wysocki, Pavel Machek, Marc Zyngier,
	Arnd Bergmann, linux-pm, devicetree, linux-kernel, loongarch,
	Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo



在 2023/6/16 下午4:03, Conor Dooley 写道:
> On Fri, Jun 16, 2023 at 03:53:38PM +0800, zhuyinbo wrote:
>> 在 2023/6/16 下午2:58, Conor Dooley 写道:
>>>
>>> Rob, could you take a look at this please? On v2 while you were away I
>>> was kinda struggling w/ suspend-address & whether it made sense.
>>>
>>> The v2 & v1 are here:
>>> https://lore.kernel.org/all/20230522093156.7108-3-zhuyinbo@loongson.cn/
>>> https://lore.kernel.org/all/20230517073149.31980-3-zhuyinbo@loongson.cn/
>>>
>>> On Thu, Jun 15, 2023 at 05:17:56PM +0800, Yinbo Zhu wrote:
>>>> Add the Loongson-2 SoC Power Management Controller binding with DT
>>>> schema format using json-schema.
>>>>
>>>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
>>>> ---
>>>>    .../soc/loongson/loongson,ls2k-pmc.yaml       | 53 +++++++++++++++++++
>>>>    MAINTAINERS                                   |  6 +++
>>>>    2 files changed, 59 insertions(+)
>>>>    create mode 100644 Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
>>>> new file mode 100644
>>>> index 000000000000..32499bd10f8c
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
>>>
>>> The filename should ideally match one of the compatibles.
>>
>>
>> I learn about that yaml file name need match this compatible, but here
>> using a specific compatible as the name of the yaml file seems a bit
>> inappropriate . After all, this yaml file needs to cover lots of ls2k
>> series SoC rather than a specific SoC, and the yaml file naming in
>> kernel drivers is basically the same that use cover a series SoC's way.
>>
>>>
>>>> @@ -0,0 +1,53 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/soc/loongson/loongson,ls2k-pmc.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Loongson-2 Power Manager controller
>>>> +
>>>> +maintainers:
>>>> +  - Yinbo Zhu <zhuyinbo@loongson.cn>
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    oneOf:
>>>> +      - items:
>>>> +          - enum:
>>>> +              - loongson,ls2k1000-pmc
>>>> +              - loongson,ls2k0500-pmc
>>>
>>> I notice the driver only supports one of these two. Is there a reason
>>> for that?
>>
>>
>> The driver can support both of the above, and I will add another.
> 
> The driver only contains
> 	static const struct of_device_id loongson2_pm_match[] = {
> 	       { .compatible = "loongson,ls2k1000-pmc", },
> 	       {},
> 	};
> so it only supports the 2k1000 right now. Are the 2k1000 and 2k0500
> compatible with eachother?


They are not completely compatible, 2k500 may still require some work to
be done, but I can confirm this driver was can compatible 2k0500 and
2k1000. for match yaml file, and I can add 2k0500 compatible in driver.

> 
>>>> +          - const: syscon
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +  interrupts:
>>>> +    maxItems: 1
>>>> +
>>>> +  suspend-address:
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>> +    description:
>>>> +      The "suspend-address" is a deep sleep state (Suspend To RAM)
>>>> +      firmware entry address which was jumped from kernel and it's
>>>> +      value was dependent on specific platform firmware code. In
>>>> +      addition, the PM need according to it to indicate that current
>>>> +      SoC whether support Suspend To RAM.
>>>> +
>>>> +required:
>>>> +  - compatible
>>>> +  - reg
>>>> +  - interrupts
>>>> +
>>>> +additionalProperties: false
>>>> +
>>>> +examples:
>>>> +  - |
>>>> +    #include <dt-bindings/interrupt-controller/irq.h>
>>>> +
>>>> +    pmc: pm@1fe27000 {
>>>          ^^^
>>>
>>> nit: this label isn't used, so you can drop it.
>>
>>
>> This lable need to be used by poweroff and reboot node but I don't add
>> these node that reference pmc here.
> 
> Right, in the dts it might need those, but not in the example in the
> binding.


okay, I got it.

Thanks,
Yinbo


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] soc: dt-bindings: add loongson-2 pm
  2023-06-16  8:03   ` Krzysztof Kozlowski
@ 2023-06-16  8:51     ` zhuyinbo
  0 siblings, 0 replies; 21+ messages in thread
From: zhuyinbo @ 2023-06-16  8:51 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Huacai Chen, WANG Xuerui, Rafael J . Wysocki,
	Pavel Machek, Marc Zyngier, Arnd Bergmann, linux-pm, devicetree,
	linux-kernel, loongarch
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo



在 2023/6/16 下午4:03, Krzysztof Kozlowski 写道:
> On 15/06/2023 11:17, Yinbo Zhu wrote:
>> Add the Loongson-2 SoC Power Management Controller binding with DT
>> schema format using json-schema.
>>
>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
>> ---
>>   .../soc/loongson/loongson,ls2k-pmc.yaml       | 53 +++++++++++++++++++
>>   MAINTAINERS                                   |  6 +++
>>   2 files changed, 59 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
>> new file mode 100644
>> index 000000000000..32499bd10f8c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
>> @@ -0,0 +1,53 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/soc/loongson/loongson,ls2k-pmc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Loongson-2 Power Manager controller
>> +
>> +maintainers:
>> +  - Yinbo Zhu <zhuyinbo@loongson.cn>
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
> 
> Drop oneOf, you don't have here many choices.


okay, I got it.

> 
>> +      - items:
>> +          - enum:
>> +              - loongson,ls2k1000-pmc
>> +              - loongson,ls2k0500-pmc
>> +          - const: syscon
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  suspend-address:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      The "suspend-address" is a deep sleep state (Suspend To RAM)
>> +      firmware entry address which was jumped from kernel and it's
>> +      value was dependent on specific platform firmware code. In
>> +      addition, the PM need according to it to indicate that current
>> +      SoC whether support Suspend To RAM.
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +
>> +    pmc: pm@1fe27000 {
> 
> Node name: system-controller or power-pamanagement
> 
> With these two:
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


okay, I will do it.

Thanks,
Yinbo


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] soc: dt-bindings: add loongson-2 pm
  2023-06-15  9:17 ` [PATCH v3 2/3] soc: dt-bindings: add loongson-2 pm Yinbo Zhu
  2023-06-16  6:58   ` Conor Dooley
  2023-06-16  8:03   ` Krzysztof Kozlowski
@ 2023-06-16 15:17   ` Rob Herring
  2023-06-17  1:57     ` zhuyinbo
  2 siblings, 1 reply; 21+ messages in thread
From: Rob Herring @ 2023-06-16 15:17 UTC (permalink / raw)
  To: Yinbo Zhu
  Cc: Krzysztof Kozlowski, Conor Dooley, Huacai Chen, WANG Xuerui,
	Rafael J . Wysocki, Pavel Machek, Marc Zyngier, Arnd Bergmann,
	linux-pm, devicetree, linux-kernel, loongarch, Jianmin Lv,
	wanghongliang, Liu Peibao, loongson-kernel

On Thu, Jun 15, 2023 at 05:17:56PM +0800, Yinbo Zhu wrote:
> Add the Loongson-2 SoC Power Management Controller binding with DT
> schema format using json-schema.
> 
> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
> ---
>  .../soc/loongson/loongson,ls2k-pmc.yaml       | 53 +++++++++++++++++++
>  MAINTAINERS                                   |  6 +++
>  2 files changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> new file mode 100644
> index 000000000000..32499bd10f8c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/soc/loongson/loongson,ls2k-pmc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Loongson-2 Power Manager controller
> +
> +maintainers:
> +  - Yinbo Zhu <zhuyinbo@loongson.cn>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - loongson,ls2k1000-pmc
> +              - loongson,ls2k0500-pmc
> +          - const: syscon
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  suspend-address:

loongson,suspend-address

> +    $ref: /schemas/types.yaml#/definitions/uint32

Isn't this a 64-bit platform? Probably better if this is a 64-bit value 
in case that's needed in the future.

> +    description:
> +      The "suspend-address" is a deep sleep state (Suspend To RAM)
> +      firmware entry address which was jumped from kernel and it's
> +      value was dependent on specific platform firmware code. In
> +      addition, the PM need according to it to indicate that current
> +      SoC whether support Suspend To RAM.
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    pmc: pm@1fe27000 {
> +        compatible = "loongson,ls2k1000-pmc", "syscon";
> +        reg = <0x1fe27000 0x58>;
> +        interrupt-parent = <&liointc1>;
> +        interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
> +        suspend-address = <0x1c000500>;
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7a91f14cad2e..bcd05f1fa5c1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12190,6 +12190,12 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/hwinfo/loongson,ls2k-chipid.yaml
>  F:	drivers/soc/loongson/loongson2_guts.c
>  
> +LOONGSON-2 SOC SERIES PM DRIVER
> +M:	Yinbo Zhu <zhuyinbo@loongson.cn>
> +L:	linux-pm@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
> +
>  LOONGSON-2 SOC SERIES PINCTRL DRIVER
>  M:	zhanghongchen <zhanghongchen@loongson.cn>
>  M:	Yinbo Zhu <zhuyinbo@loongson.cn>
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] soc: dt-bindings: add loongson-2 pm
  2023-06-16 15:17   ` Rob Herring
@ 2023-06-17  1:57     ` zhuyinbo
  0 siblings, 0 replies; 21+ messages in thread
From: zhuyinbo @ 2023-06-17  1:57 UTC (permalink / raw)
  To: Rob Herring
  Cc: Krzysztof Kozlowski, Conor Dooley, Huacai Chen, WANG Xuerui,
	Rafael J . Wysocki, Pavel Machek, Marc Zyngier, Arnd Bergmann,
	linux-pm, devicetree, linux-kernel, loongarch, Jianmin Lv,
	wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo



在 2023/6/16 下午11:17, Rob Herring 写道:
> On Thu, Jun 15, 2023 at 05:17:56PM +0800, Yinbo Zhu wrote:
>> Add the Loongson-2 SoC Power Management Controller binding with DT
>> schema format using json-schema.
>>
>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
>> ---
>>   .../soc/loongson/loongson,ls2k-pmc.yaml       | 53 +++++++++++++++++++
>>   MAINTAINERS                                   |  6 +++
>>   2 files changed, 59 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
>> new file mode 100644
>> index 000000000000..32499bd10f8c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
>> @@ -0,0 +1,53 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/soc/loongson/loongson,ls2k-pmc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Loongson-2 Power Manager controller
>> +
>> +maintainers:
>> +  - Yinbo Zhu <zhuyinbo@loongson.cn>
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - items:
>> +          - enum:
>> +              - loongson,ls2k1000-pmc
>> +              - loongson,ls2k0500-pmc
>> +          - const: syscon
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  suspend-address:
> 
> loongson,suspend-address


okay, I will use it.

> 
>> +    $ref: /schemas/types.yaml#/definitions/uint32
> 
> Isn't this a 64-bit platform? Probably better if this is a 64-bit value
> in case that's needed in the future.


okay, I got it.

Thanks,
Yinbo


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2023-06-17  1:57 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-15  9:17 [PATCH v3 0/3] soc: loongson2_pm: add power management support Yinbo Zhu
2023-06-15  9:17 ` [PATCH v3 1/3] loongarch: export some arch-specific pm interfaces Yinbo Zhu
2023-06-16  4:04   ` Huacai Chen
2023-06-16  6:06     ` zhuyinbo
2023-06-15  9:17 ` [PATCH v3 2/3] soc: dt-bindings: add loongson-2 pm Yinbo Zhu
2023-06-16  6:58   ` Conor Dooley
2023-06-16  7:53     ` zhuyinbo
2023-06-16  8:03       ` Conor Dooley
2023-06-16  8:47         ` zhuyinbo
2023-06-16  8:03   ` Krzysztof Kozlowski
2023-06-16  8:51     ` zhuyinbo
2023-06-16 15:17   ` Rob Herring
2023-06-17  1:57     ` zhuyinbo
2023-06-15  9:37 ` [PATCH v3 3/3] soc: loongson2_pm: add power management support zhuyinbo
2023-06-15 10:00   ` Huacai Chen
2023-06-15 11:15     ` zhuyinbo
2023-06-16  1:45       ` zhuyinbo
2023-06-16  1:54         ` Huacai Chen
2023-06-16  4:01           ` zhuyinbo
2023-06-16  6:52   ` Conor Dooley
2023-06-16  7:08     ` zhuyinbo

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 NNTP newsgroup(s).