[RFC PATCH 0/3] ARM64: meson-gxbb: Add support for system suspend

[RFC PATCH 0/3] ARM64: meson-gxbb: Add support for system suspend

[Neil Armstrong]

Thie patchset is a very experiment patchset to support the System Suspend
feature of the Amlogic Meson GX SoCs.

These SoCs implements system suspend using a non-standard PSCI CPU_SUSPEND
parameter to enter system suspend.
A small driver is added to properly fill the platform_suspend_ops and make
to correct SMC call.

In order to wake up from an alarm, these SoCs have a special memory mapped
register where an alarm time delay in seconds is stored.
In order to reuse the RTC wakealarm feature, implement a fake RTC device
that uses the system time to calculate a delay to write to the register.

Note that this RFC is here to seek a better way to handle these platform
specific features.

Neil Armstrong (3):
  ARM64: meson: Add Amlogic Meson GX PM Suspend
  rtc: Add Amlogic Virtual Wake RTC
  ARM64: dts: meson-gxbb: Add support for PM and Virtual RTC

 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi |   9 ++
 drivers/firmware/meson/Kconfig              |   6 +
 drivers/firmware/meson/Makefile             |   1 +
 drivers/firmware/meson/meson_gx_pm.c        |  86 +++++++++++++++
 drivers/rtc/Kconfig                         |  10 ++
 drivers/rtc/Makefile                        |   1 +
 drivers/rtc/rtc-meson-vrtc.c                | 164 ++++++++++++++++++++++++++++
 7 files changed, 277 insertions(+)
 create mode 100644 drivers/firmware/meson/meson_gx_pm.c
 create mode 100644 drivers/rtc/rtc-meson-vrtc.c

-- 
1.9.1

[RFC PATCH 1/3] ARM64: meson: Add Amlogic Meson GX PM Suspend

[Neil Armstrong]

The Amlogic Meson GX SoCs uses a non-standard argument to the
PSCI CPU_SUSPEND call to enter system suspend.

Implement such call within platform_suspend_ops.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/firmware/meson/Kconfig       |  6 +++
 drivers/firmware/meson/Makefile      |  1 +
 drivers/firmware/meson/meson_gx_pm.c | 86 ++++++++++++++++++++++++++++++++++++
 3 files changed, 93 insertions(+)
 create mode 100644 drivers/firmware/meson/meson_gx_pm.c

diff --git a/drivers/firmware/meson/Kconfig b/drivers/firmware/meson/Kconfig
index 170d7e8..5b3fea3 100644
--- a/drivers/firmware/meson/Kconfig
+++ b/drivers/firmware/meson/Kconfig
@@ -7,3 +7,9 @@ config MESON_SM
 	depends on ARM64_4K_PAGES
 	help
 	  Say y here to enable the Amlogic secure monitor driver
+
+config MESON_GX_PM
+	bool
+	default ARCH_MESON if ARM64
+	help
+	  Say y here to enable the Amlogic GX SoC Power Management
diff --git a/drivers/firmware/meson/Makefile b/drivers/firmware/meson/Makefile
index 9ab3884..b6e285d 100644
--- a/drivers/firmware/meson/Makefile
+++ b/drivers/firmware/meson/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_MESON_SM) +=	meson_sm.o
+obj-$(CONFIG_MESON_GX_PM) +=	meson_gx_pm.o
diff --git a/drivers/firmware/meson/meson_gx_pm.c b/drivers/firmware/meson/meson_gx_pm.c
new file mode 100644
index 0000000..c104c2e
--- /dev/null
+++ b/drivers/firmware/meson/meson_gx_pm.c
@@ -0,0 +1,86 @@
+/*
+ * Amlogic Meson GX Power Management
+ *
+ * Copyright (c) 2016 Baylibre, SAS.
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/suspend.h>
+#include <linux/arm-smccc.h>
+
+#include <uapi/linux/psci.h>
+
+#include <asm/suspend.h>
+
+/*
+ * The Amlogic GX SoCs uses a special argument value to the
+ * PSCI CPU_SUSPEND method to enter SUSPEND_MEM.
+ */
+
+#define MESON_SUSPEND_PARAM	0x0010000
+#define PSCI_FN_NATIVE(version, name)	PSCI_##version##_FN64_##name
+
+static int meson_gx_suspend_finish(unsigned long arg)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_smc(PSCI_FN_NATIVE(0_2, CPU_SUSPEND), arg,
+		      virt_to_phys(cpu_resume), 0, 0, 0, 0, 0, &res);
+
+	return res.a0;
+}
+
+static int meson_gx_suspend_enter(suspend_state_t state)
+{
+	switch (state) {
+	case PM_SUSPEND_MEM:
+		return cpu_suspend(MESON_SUSPEND_PARAM,
+				   meson_gx_suspend_finish);
+	}
+
+	return -EINVAL;
+}
+
+static const struct platform_suspend_ops meson_gx_pm_ops = {
+		.enter = meson_gx_suspend_enter,
+		.valid = suspend_valid_only_mem,
+};
+
+static const struct of_device_id meson_gx_pm_match[] = {
+	{ .compatible = "amlogic,meson-gx-pm", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, meson_gx_pm_match);
+
+static int meson_gx_pm_probe(struct platform_device *pdev)
+{
+	suspend_set_ops(&meson_gx_pm_ops);
+
+	return 0;
+}
+
+static struct platform_driver meson_gx_pm_driver = {
+	.probe = meson_gx_pm_probe,
+	.driver = {
+		.name = "meson-gx-pm",
+		.of_match_table = meson_gx_pm_match,
+	},
+};
+
+module_platform_driver(meson_gx_pm_driver);
+
+MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
+MODULE_DESCRIPTION("Amlogic Meson GX PM driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[RFC PATCH 2/3] rtc: Add Amlogic Virtual Wake RTC

[Neil Armstrong]

The Amlogic Meson GX SoCs uses a special register to store the
time in seconds to wakeup after a system suspend.

In order to be able to reuse the RTC wakealarm feature, this
driver implements a fake RTC device which uses the system time
to deduce a suspend delay.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/rtc/Kconfig          |  10 +++
 drivers/rtc/Makefile         |   1 +
 drivers/rtc/rtc-meson-vrtc.c | 164 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 175 insertions(+)
 create mode 100644 drivers/rtc/rtc-meson-vrtc.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index e859d14..d0f65fb 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -351,6 +351,16 @@ config RTC_DRV_MAX77686
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-max77686.
 
+config RTC_DRV_MESON_VRTC
+	tristate "Amlogic Meson Virtual RTC"
+	depends on ARCH_MESON
+	help
+	  If you say yes here you will get support for the
+	  Virtual RTC of Amlogic SoCs.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called rtc-meson-vrtc.
+
 config RTC_DRV_RK808
 	tristate "Rockchip RK808/RK818 RTC"
 	depends on MFD_RK808
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 1ac694a..feb83ad 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -92,6 +92,7 @@ obj-$(CONFIG_RTC_DRV_MAX8907)	+= rtc-max8907.o
 obj-$(CONFIG_RTC_DRV_MAX8925)	+= rtc-max8925.o
 obj-$(CONFIG_RTC_DRV_MAX8997)	+= rtc-max8997.o
 obj-$(CONFIG_RTC_DRV_MAX8998)	+= rtc-max8998.o
+obj-$(CONFIG_RTC_DRV_MESON_VRTC)+= rtc-meson-vrtc.o
 obj-$(CONFIG_RTC_DRV_MC13XXX)	+= rtc-mc13xxx.o
 obj-$(CONFIG_RTC_DRV_MCP795)	+= rtc-mcp795.o
 obj-$(CONFIG_RTC_DRV_MOXART)	+= rtc-moxart.o
diff --git a/drivers/rtc/rtc-meson-vrtc.c b/drivers/rtc/rtc-meson-vrtc.c
new file mode 100644
index 0000000..34c45bd
--- /dev/null
+++ b/drivers/rtc/rtc-meson-vrtc.c
@@ -0,0 +1,164 @@
+/*
+ * drivers/rtc/rtc-meson-vrtc.c
+ *
+ * Copyright (C) 2016 BayLibre, SAS
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ * Copyright (C) 2015 Amlogic, Inc. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ */
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/pm_wakeup.h>
+#include <linux/time64.h>
+
+struct meson_vrtc_data {
+	struct platform_device *pdev;
+	void __iomem *io_alarm;
+	struct rtc_device *rtc;
+	unsigned long alarm_time;
+};
+
+static int meson_vrtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	unsigned long local_time;
+	struct timeval time;
+
+	do_gettimeofday(&time);
+	local_time = time.tv_sec - (sys_tz.tz_minuteswest * 60);
+	rtc_time_to_tm(local_time, tm);
+
+	return 0;
+}
+
+static void meson_vrtc_set_wakeup_time(struct meson_vrtc_data *vrtc,
+				       unsigned long time)
+{
+	writel_relaxed(time, vrtc->io_alarm);
+
+	dev_dbg(&vrtc->pdev->dev, "set_wakeup_time: %lu\n", time);
+}
+
+static int meson_vrtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
+{
+	struct meson_vrtc_data *vrtc = dev_get_drvdata(dev);
+	struct timeval time;
+	unsigned long local_time;
+	unsigned long alarm_secs;
+	int ret;
+
+	if (alarm->enabled) {
+		ret = rtc_tm_to_time(&alarm->time, &alarm_secs);
+		if (ret)
+			return ret;
+
+		do_gettimeofday(&time);
+		local_time = time.tv_sec - (sys_tz.tz_minuteswest * 60);
+
+		vrtc->alarm_time = alarm_secs;
+
+		if (alarm_secs >= local_time) {
+			alarm_secs = alarm_secs - local_time;
+
+			meson_vrtc_set_wakeup_time(vrtc, alarm_secs);
+
+			pr_debug("system will wakeup %lus later\n", alarm_secs);
+		}
+	} else {
+		vrtc->alarm_time = 0;
+		meson_vrtc_set_wakeup_time(vrtc, 0);
+	}
+
+	return 0;
+}
+
+static int meson_vrtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
+{
+	struct meson_vrtc_data *vrtc = dev_get_drvdata(dev);
+
+	if (!vrtc->alarm_time) {
+		alm->enabled = true;
+
+		rtc_time_to_tm(vrtc->alarm_time, &alm->time);
+	}
+
+	return 0;
+}
+
+static const struct rtc_class_ops meson_vrtc_ops = {
+	.read_time = meson_vrtc_read_time,
+	.set_alarm = meson_vrtc_set_alarm,
+	.read_alarm = meson_vrtc_read_alarm,
+};
+
+static int meson_vrtc_probe(struct platform_device *pdev)
+{
+	struct meson_vrtc_data *vrtc;
+	struct resource *res;
+
+	vrtc = devm_kzalloc(&pdev->dev, sizeof(*vrtc), GFP_KERNEL);
+	if (!vrtc)
+		return -ENOMEM;
+
+	vrtc->pdev = pdev;
+
+	/* Alarm registers */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	vrtc->io_alarm = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(vrtc->io_alarm))
+		return PTR_ERR(vrtc->io_alarm);
+
+	device_init_wakeup(&pdev->dev, 1);
+
+	platform_set_drvdata(pdev, vrtc);
+
+	vrtc->rtc = devm_rtc_device_register(&pdev->dev, "meson-vrtc",
+			&meson_vrtc_ops, THIS_MODULE);
+	if (IS_ERR(vrtc->rtc))
+		return PTR_ERR(vrtc->rtc);
+
+	return 0;
+}
+
+int meson_vrtc_resume(struct platform_device *pdev)
+{
+	struct meson_vrtc_data *vrtc = platform_get_drvdata(pdev);
+
+	meson_vrtc_set_wakeup_time(vrtc, 0);
+
+	return 0;
+}
+
+static const struct of_device_id meson_vrtc_dt_match[] = {
+	{ .compatible = "amlogic,meson-vrtc"},
+	{},
+};
+MODULE_DEVICE_TABLE(of, meson_vrtc_dt_match);
+
+struct platform_driver meson_vrtc_driver = {
+	.driver = {
+		.name = "meson-vrtc",
+		.of_match_table = meson_vrtc_dt_match,
+	},
+	.probe = meson_vrtc_probe,
+	.resume = meson_vrtc_resume,
+};
+
+module_platform_driver(meson_vrtc_driver);
+
+MODULE_DESCRIPTION("Amlogic Virtual Wakeup RTC Timer driver");
+MODULE_LICENSE("GPL");
-- 
1.9.1

[RFC PATCH 3/3] ARM64: dts: meson-gxbb: Add support for PM and Virtual RTC

[Neil Armstrong]

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 2d69a3b..6c08d21 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -56,6 +56,10 @@
 		};
 	};
 
+	system-suspend {
+		compatible = "amlogic,meson-gx-pm";
+	};
+
 	efuse: efuse {
 		compatible = "amlogic,meson-gxbb-efuse";
 		#address-cells = <1>;
@@ -355,6 +359,11 @@
 		#reset-cells = <1>;
 	};
 
+	vrtc: rtc@0a8 {
+		compatible = "amlogic,meson-vrtc";
+		reg = <0x0 0x000a8 0x0 0x4>;
+	};
+
 	ir: ir@580 {
 		compatible = "amlogic,meson-gxbb-ir";
 		reg = <0x0 0x00580 0x0 0x40>;
-- 
1.9.1

Re: [RFC PATCH 0/3] ARM64: meson-gxbb: Add support for system suspend

[Mark Rutland]

On Thu, Nov 03, 2016 at 03:29:22PM +0100, Neil Armstrong wrote:
> Thie patchset is a very experiment patchset to support the System Suspend
> feature of the Amlogic Meson GX SoCs.
> 
> These SoCs implements system suspend using a non-standard PSCI CPU_SUSPEND
> parameter to enter system suspend.

This sounds like a violation of the CPU_SUSPEND semantics.

> A small driver is added to properly fill the platform_suspend_ops and make
> to correct SMC call.

Ignoring the fact that this is a blatant violation of the PSCI CPU_SUSPEND
semantics, this certainly should not be a separate driver.

> In order to wake up from an alarm, these SoCs have a special memory mapped
> register where an alarm time delay in seconds is stored.
> In order to reuse the RTC wakealarm feature, implement a fake RTC device
> that uses the system time to calculate a delay to write to the register.
> 
> Note that this RFC is here to seek a better way to handle these platform
> specific features.
> 
> Neil Armstrong (3):
>   ARM64: meson: Add Amlogic Meson GX PM Suspend
>   rtc: Add Amlogic Virtual Wake RTC
>   ARM64: dts: meson-gxbb: Add support for PM and Virtual RTC
> 
>  arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi |   9 ++
>  drivers/firmware/meson/Kconfig              |   6 +
>  drivers/firmware/meson/Makefile             |   1 +
>  drivers/firmware/meson/meson_gx_pm.c        |  86 +++++++++++++++
>  drivers/rtc/Kconfig                         |  10 ++
>  drivers/rtc/Makefile                        |   1 +
>  drivers/rtc/rtc-meson-vrtc.c                | 164 ++++++++++++++++++++++++++++
>  7 files changed, 277 insertions(+)
>  create mode 100644 drivers/firmware/meson/meson_gx_pm.c
>  create mode 100644 drivers/rtc/rtc-meson-vrtc.c
> 
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC PATCH 1/3] ARM64: meson: Add Amlogic Meson GX PM Suspend

[Mark Rutland]

On Thu, Nov 03, 2016 at 03:29:23PM +0100, Neil Armstrong wrote:
> The Amlogic Meson GX SoCs uses a non-standard argument to the
> PSCI CPU_SUSPEND call to enter system suspend.
> 
> Implement such call within platform_suspend_ops.

While I am very much not happy with platform-specific extensions to PSCI, if
this needs to go anywhere, it should live in the existing PSCI driver.

That is to say, NAK to other driver invoking PSCI functions.

Thanks,
Mark.

> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/firmware/meson/Kconfig       |  6 +++
>  drivers/firmware/meson/Makefile      |  1 +
>  drivers/firmware/meson/meson_gx_pm.c | 86 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 93 insertions(+)
>  create mode 100644 drivers/firmware/meson/meson_gx_pm.c
> 
> diff --git a/drivers/firmware/meson/Kconfig b/drivers/firmware/meson/Kconfig
> index 170d7e8..5b3fea3 100644
> --- a/drivers/firmware/meson/Kconfig
> +++ b/drivers/firmware/meson/Kconfig
> @@ -7,3 +7,9 @@ config MESON_SM
>  	depends on ARM64_4K_PAGES
>  	help
>  	  Say y here to enable the Amlogic secure monitor driver
> +
> +config MESON_GX_PM
> +	bool
> +	default ARCH_MESON if ARM64
> +	help
> +	  Say y here to enable the Amlogic GX SoC Power Management
> diff --git a/drivers/firmware/meson/Makefile b/drivers/firmware/meson/Makefile
> index 9ab3884..b6e285d 100644
> --- a/drivers/firmware/meson/Makefile
> +++ b/drivers/firmware/meson/Makefile
> @@ -1 +1,2 @@
>  obj-$(CONFIG_MESON_SM) +=	meson_sm.o
> +obj-$(CONFIG_MESON_GX_PM) +=	meson_gx_pm.o
> diff --git a/drivers/firmware/meson/meson_gx_pm.c b/drivers/firmware/meson/meson_gx_pm.c
> new file mode 100644
> index 0000000..c104c2e
> --- /dev/null
> +++ b/drivers/firmware/meson/meson_gx_pm.c
> @@ -0,0 +1,86 @@
> +/*
> + * Amlogic Meson GX Power Management
> + *
> + * Copyright (c) 2016 Baylibre, SAS.
> + * Author: Neil Armstrong <narmstrong@baylibre.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/suspend.h>
> +#include <linux/arm-smccc.h>
> +
> +#include <uapi/linux/psci.h>
> +
> +#include <asm/suspend.h>
> +
> +/*
> + * The Amlogic GX SoCs uses a special argument value to the
> + * PSCI CPU_SUSPEND method to enter SUSPEND_MEM.
> + */
> +
> +#define MESON_SUSPEND_PARAM	0x0010000
> +#define PSCI_FN_NATIVE(version, name)	PSCI_##version##_FN64_##name
> +
> +static int meson_gx_suspend_finish(unsigned long arg)
> +{
> +	struct arm_smccc_res res;
> +
> +	arm_smccc_smc(PSCI_FN_NATIVE(0_2, CPU_SUSPEND), arg,
> +		      virt_to_phys(cpu_resume), 0, 0, 0, 0, 0, &res);
> +
> +	return res.a0;
> +}
> +
> +static int meson_gx_suspend_enter(suspend_state_t state)
> +{
> +	switch (state) {
> +	case PM_SUSPEND_MEM:
> +		return cpu_suspend(MESON_SUSPEND_PARAM,
> +				   meson_gx_suspend_finish);
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct platform_suspend_ops meson_gx_pm_ops = {
> +		.enter = meson_gx_suspend_enter,
> +		.valid = suspend_valid_only_mem,
> +};
> +
> +static const struct of_device_id meson_gx_pm_match[] = {
> +	{ .compatible = "amlogic,meson-gx-pm", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, meson_gx_pm_match);
> +
> +static int meson_gx_pm_probe(struct platform_device *pdev)
> +{
> +	suspend_set_ops(&meson_gx_pm_ops);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver meson_gx_pm_driver = {
> +	.probe = meson_gx_pm_probe,
> +	.driver = {
> +		.name = "meson-gx-pm",
> +		.of_match_table = meson_gx_pm_match,
> +	},
> +};
> +
> +module_platform_driver(meson_gx_pm_driver);
> +
> +MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
> +MODULE_DESCRIPTION("Amlogic Meson GX PM driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC PATCH 2/3] rtc: Add Amlogic Virtual Wake RTC

[Mark Rutland]

On Thu, Nov 03, 2016 at 03:29:24PM +0100, Neil Armstrong wrote:
> The Amlogic Meson GX SoCs uses a special register to store the
> time in seconds to wakeup after a system suspend.

Where does this register live, exactly? What IP block is it part of?

> In order to be able to reuse the RTC wakealarm feature, this
> driver implements a fake RTC device which uses the system time
> to deduce a suspend delay.

This sounds like an always-on oneshot timer device, not an RTC.

> +static int meson_vrtc_read_time(struct device *dev, struct rtc_time *tm)
> +{
> +	unsigned long local_time;
> +	struct timeval time;
> +
> +	do_gettimeofday(&time);
> +	local_time = time.tv_sec - (sys_tz.tz_minuteswest * 60);
> +	rtc_time_to_tm(local_time, tm);
> +
> +	return 0;
> +}

... if this were a timer, you wouldn't need this hack.

> +static int meson_vrtc_probe(struct platform_device *pdev)
> +{
> +	struct meson_vrtc_data *vrtc;
> +	struct resource *res;
> +
> +	vrtc = devm_kzalloc(&pdev->dev, sizeof(*vrtc), GFP_KERNEL);
> +	if (!vrtc)
> +		return -ENOMEM;
> +
> +	vrtc->pdev = pdev;
> +
> +	/* Alarm registers */
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	vrtc->io_alarm = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(vrtc->io_alarm))
> +		return PTR_ERR(vrtc->io_alarm);
> +
> +	device_init_wakeup(&pdev->dev, 1);
> +
> +	platform_set_drvdata(pdev, vrtc);
> +
> +	vrtc->rtc = devm_rtc_device_register(&pdev->dev, "meson-vrtc",
> +			&meson_vrtc_ops, THIS_MODULE);
> +	if (IS_ERR(vrtc->rtc))
> +		return PTR_ERR(vrtc->rtc);
> +
> +	return 0;
> +}

I see that no interrupt is described. How exactly does this wake the system
from suspend? Is there some interrupt managed by FW for this, for example?

> +static const struct of_device_id meson_vrtc_dt_match[] = {
> +	{ .compatible = "amlogic,meson-vrtc"},

There was no binding documentation in this patch series.

Thanks,
Mark.

Re: [RFC PATCH 0/3] ARM64: meson-gxbb: Add support for system suspend

[Mark Rutland]

Urrgh, I accidentally sent this when trying to save this to my drafts folder;
sorry for the half-finished reply.

On Thu, Nov 03, 2016 at 03:25:46PM +0000, Mark Rutland wrote:
> On Thu, Nov 03, 2016 at 03:29:22PM +0100, Neil Armstrong wrote:
> > Thie patchset is a very experiment patchset to support the System Suspend
> > feature of the Amlogic Meson GX SoCs.
> > 
> > These SoCs implements system suspend using a non-standard PSCI CPU_SUSPEND
> > parameter to enter system suspend.

> > Note that this RFC is here to seek a better way to handle these platform
> > specific features.

One of the reasons that we want PSCI is that it is a standard interface across
platforms. Generally, we do not want platform-specific interfaces.

In this case, PSCI 1.0+ has SYSTEM_SUSPEND for this purpose.

If PSCI is lacking functionality that vendors want, they should speak to ARM
w.r.t. the PSCI specification -- it can be extended with new functionality as
has already happened with PSCI 0.2 and PSCI 1.0.

Thanks,
Mark.

Re: [RFC PATCH 2/3] rtc: Add Amlogic Virtual Wake RTC

[Neil Armstrong]

On 11/03/2016 04:36 PM, Mark Rutland wrote:
> On Thu, Nov 03, 2016 at 03:29:24PM +0100, Neil Armstrong wrote:
>> The Amlogic Meson GX SoCs uses a special register to store the
>> time in seconds to wakeup after a system suspend.
> 
> Where does this register live, exactly? What IP block is it part of?

It's part of the Always-ON APB registers, there is no clear IP block here,
but it seems to be like a scratch register shared with the BL3 FW.

> 
>> In order to be able to reuse the RTC wakealarm feature, this
>> driver implements a fake RTC device which uses the system time
>> to deduce a suspend delay.
> 
> This sounds like an always-on oneshot timer device, not an RTC.

The register seems to never change, and it seems to be an indication for the
FW when it enters the suspend mode.

> 
>> +static int meson_vrtc_read_time(struct device *dev, struct rtc_time *tm)
>> +{
>> +	unsigned long local_time;
>> +	struct timeval time;
>> +
>> +	do_gettimeofday(&time);
>> +	local_time = time.tv_sec - (sys_tz.tz_minuteswest * 60);
>> +	rtc_time_to_tm(local_time, tm);
>> +
>> +	return 0;
>> +}
> 
> ... if this were a timer, you wouldn't need this hack.

Actually the vendor tree uses a 64bit running counter to provide this
but with a fixed start date.

> 
>> +static int meson_vrtc_probe(struct platform_device *pdev)
>> +{
>> +	struct meson_vrtc_data *vrtc;
>> +	struct resource *res;
>> +
>> +	vrtc = devm_kzalloc(&pdev->dev, sizeof(*vrtc), GFP_KERNEL);
>> +	if (!vrtc)
>> +		return -ENOMEM;
>> +
>> +	vrtc->pdev = pdev;
>> +
>> +	/* Alarm registers */
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	vrtc->io_alarm = devm_ioremap_resource(&pdev->dev, res);
>> +	if (IS_ERR(vrtc->io_alarm))
>> +		return PTR_ERR(vrtc->io_alarm);
>> +
>> +	device_init_wakeup(&pdev->dev, 1);
>> +
>> +	platform_set_drvdata(pdev, vrtc);
>> +
>> +	vrtc->rtc = devm_rtc_device_register(&pdev->dev, "meson-vrtc",
>> +			&meson_vrtc_ops, THIS_MODULE);
>> +	if (IS_ERR(vrtc->rtc))
>> +		return PTR_ERR(vrtc->rtc);
>> +
>> +	return 0;
>> +}
> 
> I see that no interrupt is described. How exactly does this wake the system
> from suspend? Is there some interrupt managed by FW for this, for example?

It seems to be managed by the BL3 FW. We have no details on the underlying implementation.

> 
>> +static const struct of_device_id meson_vrtc_dt_match[] = {
>> +	{ .compatible = "amlogic,meson-vrtc"},
> 
> There was no binding documentation in this patch series.
> 
> Thanks,
> Mark.
> 

Re: [RFC PATCH 1/3] ARM64: meson: Add Amlogic Meson GX PM Suspend

[Sudeep Holla]

On Thu, Nov 03, 2016 at 03:29:23PM +0100, Neil Armstrong wrote:
> The Amlogic Meson GX SoCs uses a non-standard argument to the
> PSCI CPU_SUSPEND call to enter system suspend.
> 
> Implement such call within platform_suspend_ops.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/firmware/meson/Kconfig       |  6 +++
>  drivers/firmware/meson/Makefile      |  1 +
>  drivers/firmware/meson/meson_gx_pm.c | 86 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 93 insertions(+)
>  create mode 100644 drivers/firmware/meson/meson_gx_pm.c
> 
> diff --git a/drivers/firmware/meson/Kconfig b/drivers/firmware/meson/Kconfig
> index 170d7e8..5b3fea3 100644
> --- a/drivers/firmware/meson/Kconfig
> +++ b/drivers/firmware/meson/Kconfig
> @@ -7,3 +7,9 @@ config MESON_SM
>  	depends on ARM64_4K_PAGES
>  	help
>  	  Say y here to enable the Amlogic secure monitor driver
> +
> +config MESON_GX_PM
> +	bool
> +	default ARCH_MESON if ARM64
> +	help
> +	  Say y here to enable the Amlogic GX SoC Power Management
> diff --git a/drivers/firmware/meson/Makefile b/drivers/firmware/meson/Makefile
> index 9ab3884..b6e285d 100644
> --- a/drivers/firmware/meson/Makefile
> +++ b/drivers/firmware/meson/Makefile
> @@ -1 +1,2 @@
>  obj-$(CONFIG_MESON_SM) +=	meson_sm.o
> +obj-$(CONFIG_MESON_GX_PM) +=	meson_gx_pm.o
> diff --git a/drivers/firmware/meson/meson_gx_pm.c b/drivers/firmware/meson/meson_gx_pm.c
> new file mode 100644
> index 0000000..c104c2e
> --- /dev/null
> +++ b/drivers/firmware/meson/meson_gx_pm.c
> @@ -0,0 +1,86 @@
> +/*
> + * Amlogic Meson GX Power Management
> + *
> + * Copyright (c) 2016 Baylibre, SAS.
> + * Author: Neil Armstrong <narmstrong@baylibre.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/suspend.h>
> +#include <linux/arm-smccc.h>
> +
> +#include <uapi/linux/psci.h>
> +
> +#include <asm/suspend.h>
> +
> +/*
> + * The Amlogic GX SoCs uses a special argument value to the
> + * PSCI CPU_SUSPEND method to enter SUSPEND_MEM.
> + */
> +
> +#define MESON_SUSPEND_PARAM	0x0010000
> +#define PSCI_FN_NATIVE(version, name)	PSCI_##version##_FN64_##name
> +
> +static int meson_gx_suspend_finish(unsigned long arg)
> +{
> +	struct arm_smccc_res res;
> +
> +	arm_smccc_smc(PSCI_FN_NATIVE(0_2, CPU_SUSPEND), arg,
> +		      virt_to_phys(cpu_resume), 0, 0, 0, 0, 0, &res);
> +
> +	return res.a0;
> +}
> +
> +static int meson_gx_suspend_enter(suspend_state_t state)
> +{
> +	switch (state) {
> +	case PM_SUSPEND_MEM:
> +		return cpu_suspend(MESON_SUSPEND_PARAM,
> +				   meson_gx_suspend_finish);

Short response to this patch: NAK

To be constructive, since this system lacks PSCI system suspend, it just
can't support. Alternatively, this can be normal cpuidle state and you
can use suspend to idle to achieve the same and you need not hack/work
around using platform specific driver.

--
Regards,
Sudeep

Re: [RFC PATCH 1/3] ARM64: meson: Add Amlogic Meson GX PM Suspend

[Neil Armstrong]

On 11/03/2016 10:53 PM, Sudeep Holla wrote:
> 
> Short response to this patch: NAK
> 
> To be constructive, since this system lacks PSCI system suspend, it just
> can't support. Alternatively, this can be normal cpuidle state and you
> can use suspend to idle to achieve the same and you need not hack/work
> around using platform specific driver.
> 
> --
> Regards,
> Sudeep
> 

Hi Mark, Sudeep,

Thanks for your replies.
First of all, I never intended to have this patchset Acked, I certainly know this is a bad hack !

But, in the current Linux PSCI implementation, there is no way to handle System Suspend when the
SoC FW implements PSCI 0.2, except idle-to-suspend which is quite different since the FW does not handle the wakeup.

Don't worry, I'll prefer Amlogic to conform to PSCI 1.0, but like the SCPI, the GXBB was implemented
using an early version of these FW protocols... and the SoC is here and should be supported somehow.

I have a simple question : Could it be possible to declare an idle-state to be used exclusively by suspend-to-mem ?
For example while parsing the idle-states, if we encounter a particular property, the we save the state, don't
add it to the cpuidle states and register a platform_suspend_ops using this state.

Would it be accepted to be able to select a declared DT idle-state and reserve it to suspend-to-mem state ?

In the Amlogic case, their CPU_SUSPEND is partially conform, but since the power_state parameter was left as
implementation defined, they added a deeper cluster sleep state.
But potentially, we could need to handle system suspend on PSCI0.2 systems using a particular idle-state ?

Yes, Sudeep mentioned suspend-to-idle, but in our tests the kernel enters each idle-state at boot time and when
we declare this deep sleep idle state, it makes the whole system enter this system suspend state.

Neil

Re: [rtc-linux] Re: [RFC PATCH 2/3] rtc: Add Amlogic Virtual Wake RTC

[Alexandre Belloni]

On 03/11/2016 at 15:36:48 +0000, Mark Rutland wrote :
> > In order to be able to reuse the RTC wakealarm feature, this
> > driver implements a fake RTC device which uses the system time
> > to deduce a suspend delay.
> 
> This sounds like an always-on oneshot timer device, not an RTC.
> 
> > +static int meson_vrtc_read_time(struct device *dev, struct rtc_time *tm)
> > +{
> > +	unsigned long local_time;
> > +	struct timeval time;
> > +
> > +	do_gettimeofday(&time);
> > +	local_time = time.tv_sec - (sys_tz.tz_minuteswest * 60);
> > +	rtc_time_to_tm(local_time, tm);
> > +
> > +	return 0;
> > +}
> 
> ... if this were a timer, you wouldn't need this hack.
> 

The main issue I think is that the clockevents are not able to wakeup a
platform so it doesn't really fit as a timer inside the kernel.

I think it may be fine to handle that in the RTC subsystem for now.

The same issue can be seen with the flextimer on LS1021A:

http://lists.infradead.org/pipermail/linux-arm-kernel/2015-August/365597.html

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com