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 v6:
1. The patch "[PATCH v3 1/3] loongarch: export some arch-specific
pm interfaces" had been merged into mainline tree in v6.5-rc1
thus this v6 series patch need drop it and need depend on it
and it's patch link was:
https://lore.kernel.org/all/[email protected]/
2. Adding Ulf Hansson to Cc.
3. Adding [email protected] to Cc.
4. Keep indented with one tab +2 spaces in Kconfig help text.
Change in v5:
1. The patch "[PATCH v3 1/3] loongarch: export some arch-specific
pm interfaces" had been merged into linux-next tree thus this
v4 series patch need drop it and need depend on it and it's
patch link was:
https://lore.kernel.org/all/[email protected]/
2. Swap the positions of compatible for 2k1000 and 2k0500.
Change in v4:
1. The patch "[PATCH v3 1/3] loongarch: export some arch-specific
pm interfaces" had been merged into linux-next tree thus this
v4 series patch need drop it and need depend on it and it's
patch link was:
https://lore.kernel.org/all/[email protected]/
2. Remove the pmc label in dt-binding patch.
3. Add the Co-developed-by for driver patch.
4. Simplify the loongson2_suspend_valid_state that "return
(state == PM_SUSPEND_MEM)".
5. Use Using loongson2_pm_irq_enable() to replace.
loongson2_power_button_irq_enable().
6. Remove the "oneOf" in dt-bindings patch.
7. Replace "suspend-address" that use "loongson,suspend-address".
8. Use u64 type that for "loongson,suspend-address".
9. Rename "pm" to "power-mangement" in dt-bindings patch.
10. Add the reivewed-by for dt-bindings patch.
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 (2):
soc: dt-bindings: add loongson-2 pm
soc: loongson2_pm: add power management support
.../soc/loongson/loongson,ls2k-pmc.yaml | 52 +++++
MAINTAINERS | 7 +
drivers/soc/loongson/Kconfig | 10 +
drivers/soc/loongson/Makefile | 1 +
drivers/soc/loongson/loongson2_pm.c | 215 ++++++++++++++++++
5 files changed, 285 insertions(+)
--
2.20.1
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.
Co-developed-by: Liu Yun <[email protected]>
Signed-off-by: Liu Yun <[email protected]>
Co-developed-by: Liu Peibao <[email protected]>
Signed-off-by: Liu Peibao <[email protected]>
Cc: [email protected]
Cc: Ulf Hansson <[email protected]>
Signed-off-by: Yinbo Zhu <[email protected]>
---
MAINTAINERS | 1 +
drivers/soc/loongson/Kconfig | 10 ++
drivers/soc/loongson/Makefile | 1 +
drivers/soc/loongson/loongson2_pm.c | 215 ++++++++++++++++++++++++++++
4 files changed, 227 insertions(+)
create mode 100644 drivers/soc/loongson/loongson2_pm.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 608a00473498..35e0757785f1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12370,6 +12370,7 @@ M: Yinbo Zhu <[email protected]>
L: [email protected]
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 <[email protected]>
diff --git a/drivers/soc/loongson/Kconfig b/drivers/soc/loongson/Kconfig
index 707f56358dc4..314e13bb3e01 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..796add6e8b63
--- /dev/null
+++ b/drivers/soc/loongson/loongson2_pm.c
@@ -0,0 +1,215 @@
+// 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_pm_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_pm_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)
+{
+ return (state == PM_SUSPEND_MEM);
+}
+
+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;
+ u64 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_u64(dev, "loongson,suspend-address", &suspend_addr))
+ loongson_sysconf.suspend_addr = (u64)phys_to_virt(suspend_addr);
+ else
+ dev_err(dev, "No loongson,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_pm_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,ls2k0500-pmc", },
+ { .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
Add the Loongson-2 SoC Power Management Controller binding with DT
schema format using json-schema.
Signed-off-by: Yinbo Zhu <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
.../soc/loongson/loongson,ls2k-pmc.yaml | 52 +++++++++++++++++++
MAINTAINERS | 6 +++
2 files changed, 58 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..da2dcfeebf12
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
@@ -0,0 +1,52 @@
+# 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 <[email protected]>
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - loongson,ls2k0500-pmc
+ - loongson,ls2k1000-pmc
+ - const: syscon
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ loongson,suspend-address:
+ $ref: /schemas/types.yaml#/definitions/uint64
+ description:
+ The "loongson,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>
+
+ power-management@1fe27000 {
+ compatible = "loongson,ls2k1000-pmc", "syscon";
+ reg = <0x1fe27000 0x58>;
+ interrupt-parent = <&liointc1>;
+ interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
+ loongson,suspend-address = <0x0 0x1c000500>;
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 1089ef3319f2..608a00473498 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12365,6 +12365,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 <[email protected]>
+L: [email protected]
+S: Maintained
+F: Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml
+
LOONGSON-2 SOC SERIES PINCTRL DRIVER
M: zhanghongchen <[email protected]>
M: Yinbo Zhu <[email protected]>
--
2.20.1
On Thu, Aug 3, 2023, at 08:37, Yinbo Zhu wrote:
> 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.
>
> Co-developed-by: Liu Yun <[email protected]>
> Signed-off-by: Liu Yun <[email protected]>
> Co-developed-by: Liu Peibao <[email protected]>
> Signed-off-by: Liu Peibao <[email protected]>
> Cc: [email protected]
> Cc: Ulf Hansson <[email protected]>
> Signed-off-by: Yinbo Zhu <[email protected]>
I'm still waiting for Ulf to take a look here to see whether
this should be in drivers/genpd instead, but he might still
be on vacation.
A few minor comments from me in the meantime:
> +#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)
I would prefer these to be 'static inline' functions rather than
macros, or you can just open-code them, as each macro is only
used once at the moment.
> +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");
The message is probably more appropriate as a pr_debug() than
pr_info().
> +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);
Please change this to DEFINE_SIMPLE_DEV_PM_OPS() and remove the
__maybe_unused, this is what all drivers should have these days.
> +
> +static int loongson2_pm_probe(struct platform_device *pdev)
> +{
> + int irq, retval;
> + u64 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_u64(dev, "loongson,suspend-address",
> &suspend_addr))
> + loongson_sysconf.suspend_addr = (u64)phys_to_virt(suspend_addr);
> + else
Having a custom "loongson,suspend-address" property here feels wrong
to me. Can't this be moved into the "regs" property that holds
the other mmio registers?
Arnd
On Thu, Aug 3, 2023, at 08:37, Yinbo Zhu wrote:
> + loongson,suspend-address:
> + $ref: /schemas/types.yaml#/definitions/uint64
> + description:
> + The "loongson,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.
> +
I just commented on this in the driver patch, assuming this
was an MMIO address, but I'm even more confused now, since
we try hard to not rely on being able to just interface with
firmware like this.
If this is executable code, where does this actually reside?
Is this some SRAM that needs to execute the suspend logic
in order to shut down memory and cache controllers? Or is
this a runtime firmware interface similar to how UEFI handles
its runtime services to keep the implementation out of
the kernel?
Does the code work with both traditional suspend-to-ram and
modern suspend-to-idle logic?
Arnd
?? 2023/8/3 ????3:44, Arnd Bergmann д??:
> On Thu, Aug 3, 2023, at 08:37, Yinbo Zhu wrote:
>
>> + loongson,suspend-address:
>> + $ref: /schemas/types.yaml#/definitions/uint64
>> + description:
>> + The "loongson,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.
>> +
>
> I just commented on this in the driver patch, assuming this
> was an MMIO address, but I'm even more confused now, since
> we try hard to not rely on being able to just interface with
> firmware like this.
>
> If this is executable code, where does this actually reside?
Pmon firmware code.
> Is this some SRAM that needs to execute the suspend logic
> in order to shut down memory and cache controllers?
Yes, The suspend-to-ram after into pmon firmware code and set
self-refresh mode in memory controller and ensure that memory data is
not lost then shut down memory controller.
> Or is
> this a runtime firmware interface similar to how UEFI handles
> its runtime services to keep the implementation out of
> the kernel?
No, The main cpu and other cpu will offline that after into firmware and
finished Corresponding operations, the pmon firmware will not run.
>
> Does the code work with both traditional suspend-to-ram and
> modern suspend-to-idle logic?
Yes, It can support suspend-to-ram and suspend-to-idle.
Thanks,
Yinbo
On Thu, 3 Aug 2023 at 09:03, Arnd Bergmann <[email protected]> wrote:
>
> On Thu, Aug 3, 2023, at 08:37, Yinbo Zhu wrote:
> > 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.
> >
> > Co-developed-by: Liu Yun <[email protected]>
> > Signed-off-by: Liu Yun <[email protected]>
> > Co-developed-by: Liu Peibao <[email protected]>
> > Signed-off-by: Liu Peibao <[email protected]>
> > Cc: [email protected]
> > Cc: Ulf Hansson <[email protected]>
> > Signed-off-by: Yinbo Zhu <[email protected]>
>
> I'm still waiting for Ulf to take a look here to see whether
> this should be in drivers/genpd instead, but he might still
> be on vacation.
I don't think this belongs in drivers/genpd/ as it's not a genpd
provider. Besides that, no further comments from me at this point.
Kind regards
Uffe
>
> A few minor comments from me in the meantime:
>
> > +#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)
>
> I would prefer these to be 'static inline' functions rather than
> macros, or you can just open-code them, as each macro is only
> used once at the moment.
>
> > +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");
>
> The message is probably more appropriate as a pr_debug() than
> pr_info().
>
> > +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);
>
> Please change this to DEFINE_SIMPLE_DEV_PM_OPS() and remove the
> __maybe_unused, this is what all drivers should have these days.
>
> > +
> > +static int loongson2_pm_probe(struct platform_device *pdev)
> > +{
> > + int irq, retval;
> > + u64 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_u64(dev, "loongson,suspend-address",
> > &suspend_addr))
> > + loongson_sysconf.suspend_addr = (u64)phys_to_virt(suspend_addr);
> > + else
>
> Having a custom "loongson,suspend-address" property here feels wrong
> to me. Can't this be moved into the "regs" property that holds
> the other mmio registers?
>
> Arnd
On Tue, Aug 8, 2023, at 16:42, Ulf Hansson wrote:
> On Thu, 3 Aug 2023 at 09:03, Arnd Bergmann <[email protected]> wrote:
>>
>> On Thu, Aug 3, 2023, at 08:37, Yinbo Zhu wrote:
>> > 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.
>> >
>> > Co-developed-by: Liu Yun <[email protected]>
>> > Signed-off-by: Liu Yun <[email protected]>
>> > Co-developed-by: Liu Peibao <[email protected]>
>> > Signed-off-by: Liu Peibao <[email protected]>
>> > Cc: [email protected]
>> > Cc: Ulf Hansson <[email protected]>
>> > Signed-off-by: Yinbo Zhu <[email protected]>
>>
>> I'm still waiting for Ulf to take a look here to see whether
>> this should be in drivers/genpd instead, but he might still
>> be on vacation.
>
> I don't think this belongs in drivers/genpd/ as it's not a genpd
> provider. Besides that, no further comments from me at this point.
Ok, thanks!
Let's try to finish discussing the suspend-address question
and merge it through the soc tree once that is resolved then.
Arnd
On Fri, Aug 4, 2023, at 04:54, Yinbo Zhu wrote:
> 在 2023/8/3 下午3:44, Arnd Bergmann 写道:
>> On Thu, Aug 3, 2023, at 08:37, Yinbo Zhu wrote:
>>
>>> + loongson,suspend-address:
>>> + $ref: /schemas/types.yaml#/definitions/uint64
>>> + description:
>>> + The "loongson,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.
>>> +
>>
>> I just commented on this in the driver patch, assuming this
>> was an MMIO address, but I'm even more confused now, since
>> we try hard to not rely on being able to just interface with
>> firmware like this.
>>
>> If this is executable code, where does this actually reside?
>
>
> Pmon firmware code.
>
>> Is this some SRAM that needs to execute the suspend logic
>> in order to shut down memory and cache controllers?
>
>
> Yes, The suspend-to-ram after into pmon firmware code and set
> self-refresh mode in memory controller and ensure that memory data is
> not lost then shut down memory controller.
I'm sorry I missed your reply earlier, getting back to the
thread now. So it's clear that this code needs to run in a
special memory from your description, but I'm still trying
to understand the details better.
I found https://github.com/loongson-community/pmon source
code, and a reference to its origin at LSI Logic at
https://www.linux-mips.org/wiki/PMON but otherwise have
no idea about what this actually is, or how it relates
to your UEFI firmware. Did you add UEFI support to PMON,
or do you use it as a first stage loader that loads
the actual UEFI implementation (EDK2 or u-boot, I guess)?
>> Or is
>> this a runtime firmware interface similar to how UEFI handles
>> its runtime services to keep the implementation out of
>> the kernel?
>
>
> No, The main cpu and other cpu will offline that after into firmware and
> finished Corresponding operations, the pmon firmware will not run.
I'm still trying to understand your explanations here.
You say that pmon no longer runs, but that seems to contradict
what you said earlier about branching into pmon firmware code
for suspend.
Is this executing directly from ROM then?
Arnd
在 2023/8/12 下午8:25, Arnd Bergmann 写道:
> On Fri, Aug 4, 2023, at 04:54, Yinbo Zhu wrote:
>> 在 2023/8/3 下午3:44, Arnd Bergmann 写道:
>>> On Thu, Aug 3, 2023, at 08:37, Yinbo Zhu wrote:
>>>
>>>> + loongson,suspend-address:
>>>> + $ref: /schemas/types.yaml#/definitions/uint64
>>>> + description:
>>>> + The "loongson,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.
>>>> +
>>>
>>> I just commented on this in the driver patch, assuming this
>>> was an MMIO address, but I'm even more confused now, since
>>> we try hard to not rely on being able to just interface with
>>> firmware like this.
>>>
>>> If this is executable code, where does this actually reside?
>>
>>
>> Pmon firmware code.
>>
>>> Is this some SRAM that needs to execute the suspend logic
>>> in order to shut down memory and cache controllers?
>>
>>
>> Yes, The suspend-to-ram after into pmon firmware code and set
>> self-refresh mode in memory controller and ensure that memory data is
>> not lost then shut down memory controller.
>
> I'm sorry I missed your reply earlier, getting back to the
> thread now. So it's clear that this code needs to run in a
> special memory from your description, but I'm still trying
> to understand the details better.
>
> I found https://github.com/loongson-community/pmon source
> code, and a reference to its origin at LSI Logic at
> https://www.linux-mips.org/wiki/PMON but otherwise have
> no idea about what this actually is, or how it relates
> to your UEFI firmware. Did you add UEFI support to PMON,
> or do you use it as a first stage loader that loads
> the actual UEFI implementation (EDK2 or u-boot, I guess)?
Pmon and uefi are two different firmware, and there is no connection
between them.
>
>>> Or is
>>> this a runtime firmware interface similar to how UEFI handles
>>> its runtime services to keep the implementation out of
>>> the kernel?
>>
>>
>> No, The main cpu and other cpu will offline that after into firmware and
>> finished Corresponding operations, the pmon firmware will not run.
>
> I'm still trying to understand your explanations here.
> You say that pmon no longer runs, but that seems to contradict
> what you said earlier about branching into pmon firmware code
> for suspend.
It's not contradictory. The suspend-to-ram is that from kernel goto to
pmon firmware code, then pmon finished corresponding operations, which
was to set self-refresh mode in memory controller, then memory HW will
maintain its own data and no longer requires software processing, pmon
firmware will not run.
>
> Is this executing directly from ROM then?
Yes.
Thanks,
Yinbo
On Mon, Aug 14, 2023, at 09:57, Yinbo Zhu wrote:
> 在 2023/8/12 下午8:25, Arnd Bergmann 写道:
>> On Fri, Aug 4, 2023, at 04:54, Yinbo Zhu wrote:
>>> 在 2023/8/3 下午3:44, Arnd Bergmann 写道:
>>>> On Thu, Aug 3, 2023, at 08:37, Yinbo Zhu wrote:
>>>
>>>> Is this some SRAM that needs to execute the suspend logic
>>>> in order to shut down memory and cache controllers?
>>>
>>>
>>> Yes, The suspend-to-ram after into pmon firmware code and set
>>> self-refresh mode in memory controller and ensure that memory data is
>>> not lost then shut down memory controller.
>>
>> I'm sorry I missed your reply earlier, getting back to the
>> thread now. So it's clear that this code needs to run in a
>> special memory from your description, but I'm still trying
>> to understand the details better.
>>
>> I found https://github.com/loongson-community/pmon source
>> code, and a reference to its origin at LSI Logic at
>> https://www.linux-mips.org/wiki/PMON but otherwise have
>> no idea about what this actually is, or how it relates
>> to your UEFI firmware. Did you add UEFI support to PMON,
>> or do you use it as a first stage loader that loads
>> the actual UEFI implementation (EDK2 or u-boot, I guess)?
>
>
> Pmon and uefi are two different firmware, and there is no connection
> between them.
It sounds like we still have problems with terminology.
I don't think categorizing UEFI as a firmware is correct,
it's the interface used by various firmware implementations
to load the operating system. As far as I understand,
loongarch currently mandates the use of UEFI in whichever
firmware is used, so if you have Pmon installed in ROM,
and Pmon does not itself implement UEFI, it would have
to load some other firmware such as u-boot in order to
load a kernel through the UEFI protocol, right?
Has the assumption that loongarch requires UEFI changed?
>>>> Or is
>>>> this a runtime firmware interface similar to how UEFI handles
>>>> its runtime services to keep the implementation out of
>>>> the kernel?
>>>
>>>
>>> No, The main cpu and other cpu will offline that after into firmware and
>>> finished Corresponding operations, the pmon firmware will not run.
>>
>> I'm still trying to understand your explanations here.
>> You say that pmon no longer runs, but that seems to contradict
>> what you said earlier about branching into pmon firmware code
>> for suspend.
>
>
> It's not contradictory. The suspend-to-ram is that from kernel goto to
> pmon firmware code, then pmon finished corresponding operations, which
> was to set self-refresh mode in memory controller, then memory HW will
> maintain its own data and no longer requires software processing, pmon
> firmware will not run.
That is what I mean with a "runtime firmware interface", i.e. you
jump into firmware in order to request services from it. Clearly the
firmware itself does not run while the OS is executing code, but it is
still there and waiting to be called here, which is similar to
things like UEFI runtime services, PowerPC RTAS, Arm EL3/trustzone
based firmware or x86 SMM firmware, except that this is much less
formalized and only consists of an entry point with undocument
calling conventions.
>> Is this executing directly from ROM then?
>
> Yes.
Is this the only runtime call into the firmware, or are there
others that are either already called from mainline kernels
or in your downsteam implementation?
How do you ensure that the DTB matches the actual ROM code
after rebuilding Pmon? Does Pmon itself fill that field with
the correct address, or do you rely on it being a hardcoded
constant?
Arnd
在 2023/8/14 下午4:19, Arnd Bergmann 写道:
> On Mon, Aug 14, 2023, at 09:57, Yinbo Zhu wrote:
>> 在 2023/8/12 下午8:25, Arnd Bergmann 写道:
>>> On Fri, Aug 4, 2023, at 04:54, Yinbo Zhu wrote:
>>>> 在 2023/8/3 下午3:44, Arnd Bergmann 写道:
>>>>> On Thu, Aug 3, 2023, at 08:37, Yinbo Zhu wrote:
>>>>
>>>>> Is this some SRAM that needs to execute the suspend logic
>>>>> in order to shut down memory and cache controllers?
>>>>
>>>>
>>>> Yes, The suspend-to-ram after into pmon firmware code and set
>>>> self-refresh mode in memory controller and ensure that memory data is
>>>> not lost then shut down memory controller.
>>>
>>> I'm sorry I missed your reply earlier, getting back to the
>>> thread now. So it's clear that this code needs to run in a
>>> special memory from your description, but I'm still trying
>>> to understand the details better.
>>>
>>> I found https://github.com/loongson-community/pmon source
>>> code, and a reference to its origin at LSI Logic at
>>> https://www.linux-mips.org/wiki/PMON but otherwise have
>>> no idea about what this actually is, or how it relates
>>> to your UEFI firmware. Did you add UEFI support to PMON,
>>> or do you use it as a first stage loader that loads
>>> the actual UEFI implementation (EDK2 or u-boot, I guess)?
>>
>>
>> Pmon and uefi are two different firmware, and there is no connection
>> between them.
>
> It sounds like we still have problems with terminology. >
> I don't think categorizing UEFI as a firmware is correct,
Sorry to have confused you, uefi firmware is our internal name, which is
actually what you referred to as EDK2, the EDK2 need use UEFI.
> it's the interface used by various firmware implementations
> to load the operating system. As far as I understand,
> loongarch currently mandates the use of UEFI in whichever
> firmware is used, so if you have Pmon installed in ROM > and Pmon does not itself implement UEFI, it would have
> to load some other firmware such as u-boot in order to
> load a kernel through the UEFI protocol, right?
PMON is an independent firmware and loader that can directly load the
operating system and it does not require the use of UEFI.
>
> Has the assumption that loongarch requires UEFI changed?
LoongArch embedded board was use Pmon firmware, The other one uses UEFI
firmware (EDK2) on LoongArch platform.
>
>>>>> Or is
>>>>> this a runtime firmware interface similar to how UEFI handles
>>>>> its runtime services to keep the implementation out of
>>>>> the kernel?
>>>>
>>>>
>>>> No, The main cpu and other cpu will offline that after into firmware and
>>>> finished Corresponding operations, the pmon firmware will not run.
>>>
>>> I'm still trying to understand your explanations here.
>>> You say that pmon no longer runs, but that seems to contradict
>>> what you said earlier about branching into pmon firmware code
>>> for suspend.
>>
>>
>> It's not contradictory. The suspend-to-ram is that from kernel goto to
>> pmon firmware code, then pmon finished corresponding operations, which
>> was to set self-refresh mode in memory controller, then memory HW will
>> maintain its own data and no longer requires software processing, pmon
>> firmware will not run.
>
> That is what I mean with a "runtime firmware interface", i.e. you
> jump into firmware in order to request services from it. Clearly the
> firmware itself does not run while the OS is executing code, but it is
> still there and waiting to be called here, which is similar to
> things like UEFI runtime services, PowerPC RTAS, Arm EL3/trustzone
> based firmware or x86 SMM firmware, except that this is much less
> formalized and only consists of an entry point with undocument
> calling conventions.
>
>>> Is this executing directly from ROM then?
>>
>> Yes.
>
> Is this the only runtime call into the firmware,
Only when suspend-to-ram occurs, the kernel will call into the firmware.
No other case.
> or are there
> others that are either already called from mainline kernels
> or in your downsteam implementation?
>
> How do you ensure that the DTB matches the actual ROM code
> after rebuilding Pmon? Does Pmon itself fill that field with
> the correct address, or do you rely on it being a hardcoded
> constant?
Use Pmon, firmware team will always ensure that DTB matches the actual
ROM code. The "suspend-address" of dtb and pmon entry address will
synchronized modification by firmware team.
Thanks,
Yinbo
On Mon, Aug 14, 2023, at 13:57, Yinbo Zhu wrote:
> 在 2023/8/14 下午4:19, Arnd Bergmann 写道:
>> On Mon, Aug 14, 2023, at 09:57, Yinbo Zhu wrote:
>>> 在 2023/8/12 下午8:25, Arnd Bergmann 写道:
>>>> I found https://github.com/loongson-community/pmon source
>>>> code, and a reference to its origin at LSI Logic at
>>>> https://www.linux-mips.org/wiki/PMON but otherwise have
>>>> no idea about what this actually is, or how it relates
>>>> to your UEFI firmware. Did you add UEFI support to PMON,
>>>> or do you use it as a first stage loader that loads
>>>> the actual UEFI implementation (EDK2 or u-boot, I guess)?
>>>
>>>
>>> Pmon and uefi are two different firmware, and there is no connection
>>> between them.
>>
>> It sounds like we still have problems with terminology. >
>> I don't think categorizing UEFI as a firmware is correct,
>
>
> Sorry to have confused you, uefi firmware is our internal name, which is
> actually what you referred to as EDK2, the EDK2 need use UEFI.
Ok
>> it's the interface used by various firmware implementations
>> to load the operating system. As far as I understand,
>> loongarch currently mandates the use of UEFI in whichever
>> firmware is used, so if you have Pmon installed in ROM > and Pmon does not itself implement UEFI, it would have
>> to load some other firmware such as u-boot in order to
>> load a kernel through the UEFI protocol, right?
>
>
> PMON is an independent firmware and loader that can directly load the
> operating system and it does not require the use of UEFI.
>>
>> Has the assumption that loongarch requires UEFI changed?
>
>
> LoongArch embedded board was use Pmon firmware, The other one uses UEFI
> firmware (EDK2) on LoongArch platform.
I'm pretty sure we discussed this when the loongarch port
was originally merged, with the decisionto just use UEFI for
booting any kernel, as the legacy entry point for the ACPI
based environment was not really well-defined and the UEFI
stub was an easy alternative to have more commonality
with other architectures.
I see this was already extended for embedded CPUs to use
the uefi stub with DT in commit 88d4d957edc70 ("LoongArch: Add
FDT booting support from efi system table"), which seems like
the right direction.
Can you explain why this board would want yet another method
for entering the kernel? Is there any documentation for the
boot protocol?
>>>> Is this executing directly from ROM then?
>>>
>>> Yes.
>>
>> Is this the only runtime call into the firmware,
>
>
> Only when suspend-to-ram occurs, the kernel will call into the firmware.
> No other case.
Ok
>> or are there
>> others that are either already called from mainline kernels
>> or in your downsteam implementation?
>>
>> How do you ensure that the DTB matches the actual ROM code
>> after rebuilding Pmon? Does Pmon itself fill that field with
>> the correct address, or do you rely on it being a hardcoded
>> constant?
>
>
> Use Pmon, firmware team will always ensure that DTB matches the actual
> ROM code. The "suspend-address" of dtb and pmon entry address will
> synchronized modification by firmware team.
Ok. So it's linked against libfdt to fill the dtb information,
or do you have to provide a dtb blob that matches the firmware?
Arnd
在 2023/8/14 下午9:41, Arnd Bergmann 写道:
> On Mon, Aug 14, 2023, at 13:57, Yinbo Zhu wrote:
>> 在 2023/8/14 下午4:19, Arnd Bergmann 写道:
>>> On Mon, Aug 14, 2023, at 09:57, Yinbo Zhu wrote:
>>>> 在 2023/8/12 下午8:25, Arnd Bergmann 写道:
>>>>> I found https://github.com/loongson-community/pmon source
>>>>> code, and a reference to its origin at LSI Logic at
>>>>> https://www.linux-mips.org/wiki/PMON but otherwise have
>>>>> no idea about what this actually is, or how it relates
>>>>> to your UEFI firmware. Did you add UEFI support to PMON,
>>>>> or do you use it as a first stage loader that loads
>>>>> the actual UEFI implementation (EDK2 or u-boot, I guess)?
>>>>
>>>>
>>>> Pmon and uefi are two different firmware, and there is no connection
>>>> between them.
>>>
>>> It sounds like we still have problems with terminology. >
>>> I don't think categorizing UEFI as a firmware is correct,
>>
>>
>> Sorry to have confused you, uefi firmware is our internal name, which is
>> actually what you referred to as EDK2, the EDK2 need use UEFI.
>
> Ok
>
>>> it's the interface used by various firmware implementations
>>> to load the operating system. As far as I understand,
>>> loongarch currently mandates the use of UEFI in whichever
>>> firmware is used, so if you have Pmon installed in ROM > and Pmon does not itself implement UEFI, it would have
>>> to load some other firmware such as u-boot in order to
>>> load a kernel through the UEFI protocol, right?
>>
>>
>> PMON is an independent firmware and loader that can directly load the
>> operating system and it does not require the use of UEFI.
>>>
>>> Has the assumption that loongarch requires UEFI changed?
>>
>>
>> LoongArch embedded board was use Pmon firmware, The other one uses UEFI
>> firmware (EDK2) on LoongArch platform.
>
> I'm pretty sure we discussed this when the loongarch port
> was originally merged, with the decisionto just use UEFI for
> booting any kernel, as the legacy entry point for the ACPI
> based environment was not really well-defined and the UEFI
> stub was an easy alternative to have more commonality
> with other architectures.
>
> I see this was already extended for embedded CPUs to use
> the uefi stub with DT in commit 88d4d957edc70 ("LoongArch: Add
> FDT booting support from efi system table"), which seems like
> the right direction. >
> Can you explain why this board would want yet another method
> for entering the kernel? Is there any documentation for the
> boot protocol?
Yes, you're right, the latest PMON does indeed support UEFI, the PMON
used in the product code is still outdated and does not support UEFI.
Actually, whether using EDK2, Pmon firmware, or other firmware, the
LoongArch platform's s3 (suspend-to-ram) requires a suspend-address to
be defined, if dts is supported, it is defined in dts, and if acpi table
is supported, it is defined in acpi table.
>
>>>>> Is this executing directly from ROM then?
>>>>
>>>> Yes.
>>>
>>> Is this the only runtime call into the firmware,
>>
>>
>> Only when suspend-to-ram occurs, the kernel will call into the firmware.
>> No other case.
>
> Ok
>
>>> or are there
>>> others that are either already called from mainline kernels
>>> or in your downsteam implementation?
>>>
>>> How do you ensure that the DTB matches the actual ROM code
>>> after rebuilding Pmon? Does Pmon itself fill that field with
>>> the correct address, or do you rely on it being a hardcoded
>>> constant?
>>
>>
>> Use Pmon, firmware team will always ensure that DTB matches the actual
>> ROM code. The "suspend-address" of dtb and pmon entry address will
>> synchronized modification by firmware team.
>
> Ok. So it's linked against libfdt to fill the dtb information,
> or do you have to provide a dtb blob that matches the firmware?
For pmon firmware, the dtb was as part of firmware, the firmware has
defined the address of the DTB's suspend and the address of the firmware
entry, and is consistent.
Thanks,
Yinbo
On Thu, Aug 3, 2023, at 02:37, Yinbo Zhu wrote:
> 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 v6:
> 1. The patch "[PATCH v3 1/3] loongarch: export some arch-specific
> pm interfaces" had been merged into mainline tree in v6.5-rc1
> thus this v6 series patch need drop it and need depend on it
> and it's patch link was:
> https://lore.kernel.org/all/[email protected]/
> 2. Adding Ulf Hansson to Cc.
> 3. Adding [email protected] to Cc.
> 4. Keep indented with one tab +2 spaces in Kconfig help text.
I talked to WANG Xuerui on IRC, and he was able to clarify some of the
missing bits of information for me, after which I merged both patches,
even though my concerns are not fully addressed:
- I still think that branching into ROM code from the kernel is a mistake
and we should have never allowed that as an ad-hoc interface in the ACPI
variant to start with. It's hard to change that now though, and having
a DT interface to access the same entry point does not really make it
worse. This might need a redesign for future firmware though, to have
a proper runtime interface
- The bigger problem I still see is the DT-enabled boot with PMon without
the UEFI firmware. This does not impact the DT binding, but I would
consider all non-UEFI booting firmware images broken and not supported
by the kernel, as we originally discussed when merging the kernel.
These should still be fixable by upgrading PMon to a UEFI-enabled
version.
Arnd