All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH 0/3] SRAM reboot mode driver
@ 2016-01-27  0:37 John Stultz
  2016-01-27  0:37 ` [RFC][PATCH 1/3] dt-bindings: power: reset: Add document for sram-reboot-mode driver John Stultz
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: John Stultz @ 2016-01-27  0:37 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Andy Yan, Rob Herring, Arnd Bergmann,
	Thierry Reding, Heiko Stübner, Caesar Wang, Kees Cook,
	Guodong Xu, Haojian Zhuang, Vishal Bhoj, Bjorn Andersson,
	devicetree, Android Kernel Team

This patchset extends on Andy Yan's reboot mode driver
work from here: https://lkml.org/lkml/2016/1/12/315

It adds reboot mode/reason support for devices that use
an SRAM location to communicate with the bootloader.

Doing this via an SRAM subnode was a suggestion from
Arnd, but I worry this implementation isn't yet ideal,
since I spent quite a bit of time futzing with getting
the sram dts entry to work properly. So I suspect there
will be a number of suggestions for improvements.

Again, this series dependson Andy's patch set above,
but also was developed & tested against the 4.1 based
hikey tree, so at least the hikey dts patch won't apply.
I'm mostly sending this out for just a rough initial
review of the dts and conceptual usage of sram subnodes.

Also, it was pointed out that the hikey dts entry for
this really should be added by the UEFI firmware, since
alternative bootloaders may be used which do not support
this feature. So the hikey dts patch isn't likely to ever
go upstream, but its a useful illustration for how other
devices might use this driver.

Cc: Andy Yan <andy.yan@rock-chips.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Heiko Stübner <heiko@sntech.de>
Cc: Caesar Wang <wxt@rock-chips.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
Cc: Vishal Bhoj <vishal.bhoj@linaro.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: devicetree@vger.kernel.org
Cc: Android Kernel Team <kernel-team@android.com>

John Stultz (3):
  dt-bindings: power: reset: Add document for sram-reboot-mode driver
  power: reset: Add sram-reboot-mode driver
  dts: hikey: Add hikey support for sram-reboot-mode

 .../bindings/power/reset/sram-reboot-mode.txt      | 47 +++++++++++++++
 arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts     | 36 ++++++++++++
 arch/arm64/configs/hikey_defconfig                 |  3 +
 drivers/power/reset/Kconfig                        |  9 +++
 drivers/power/reset/Makefile                       |  1 +
 drivers/power/reset/sram-reboot-mode.c             | 66 ++++++++++++++++++++++
 6 files changed, 162 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/reset/sram-reboot-mode.txt
 create mode 100644 drivers/power/reset/sram-reboot-mode.c

-- 
1.9.1

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

* [RFC][PATCH 1/3] dt-bindings: power: reset: Add document for sram-reboot-mode driver
  2016-01-27  0:37 [RFC][PATCH 0/3] SRAM reboot mode driver John Stultz
@ 2016-01-27  0:37 ` John Stultz
  2016-01-27  1:11   ` Andy Yan
  2016-01-27  0:37 ` [RFC][PATCH 2/3] power: reset: Add " John Stultz
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: John Stultz @ 2016-01-27  0:37 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Andy Yan, Rob Herring, Arnd Bergmann,
	Thierry Reding, Heiko Stübner, Caesar Wang, Kees Cook,
	Guodong Xu, Haojian Zhuang, Vishal Bhoj, Bjorn Andersson,
	devicetree, Android Kernel Team

Add device tree binding document for reboot-mode driver

Cc: Andy Yan <andy.yan@rock-chips.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Heiko Stübner <heiko@sntech.de>
Cc: Caesar Wang <wxt@rock-chips.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
Cc: Vishal Bhoj <vishal.bhoj@linaro.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: devicetree@vger.kernel.org
Cc: Android Kernel Team <kernel-team@android.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 .../bindings/power/reset/sram-reboot-mode.txt      | 47 ++++++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/reset/sram-reboot-mode.txt

diff --git a/Documentation/devicetree/bindings/power/reset/sram-reboot-mode.txt b/Documentation/devicetree/bindings/power/reset/sram-reboot-mode.txt
new file mode 100644
index 0000000..33bb132
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/sram-reboot-mode.txt
@@ -0,0 +1,47 @@
+SRAM reboot mode driver
+
+This driver get reboot mode magic value form reboot-mode driver
+and stores it in a SRAM address. Then the bootloader
+can read it and take different action according to the magic
+value stored.
+
+This DT node should be represented as a sub-node of a "mmio-sram", "simple-bus"
+node.
+
+Required properties:
+- compatible: should be "sram-reboot-mode"
+- reg: offset from the sram range where to store the magic value(in bytes)
+
+The rest of the properties should follow the generic reboot-mode discription
+found in reboot-mode.txt
+
+Example:
+
+	sram@5f01000 {
+		compatible = "mmio-sram", "simple-bus";
+		reg = <0x0 0x05f01000 0x0 0x00001000>;
+		ranges = <0x0 0x0 0x05f01000 0x00001000>;
+
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		reboot-mode@0 {
+			compatible = "sram-reboot-mode";
+			reg = <0x0 0x4>;
+
+			none {
+				linux,mode = "none";
+				loader,magic = <0x77665501>;
+			};
+
+			bootloader {
+				linux,mode = "bootloader";
+				loader,magic = <0x77665500>;
+			};
+
+			recovery {
+				linux,mode = "recovery";
+				loader,magic = <0x77665502>;
+			};
+		};
+	};
-- 
1.9.1

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

* [RFC][PATCH 2/3] power: reset: Add sram-reboot-mode driver
  2016-01-27  0:37 [RFC][PATCH 0/3] SRAM reboot mode driver John Stultz
  2016-01-27  0:37 ` [RFC][PATCH 1/3] dt-bindings: power: reset: Add document for sram-reboot-mode driver John Stultz
@ 2016-01-27  0:37 ` John Stultz
  2016-01-27  0:38 ` [RFC][PATCH 3/3] dts: hikey: Add hikey support for sram-reboot-mode John Stultz
  2016-01-27  8:50 ` [RFC][PATCH 0/3] SRAM reboot mode driver Vladimir Zapolskiy
  3 siblings, 0 replies; 7+ messages in thread
From: John Stultz @ 2016-01-27  0:37 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Andy Yan, Rob Herring, Arnd Bergmann,
	Thierry Reding, Heiko Stübner, Caesar Wang, Kees Cook,
	Guodong Xu, Haojian Zhuang, Vishal Bhoj, Bjorn Andersson,
	devicetree, Android Kernel Team

Add sram-reboot-mode driver, which enables
reboot modes to be specified from sram subnodes.

Cc: Andy Yan <andy.yan@rock-chips.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Heiko Stübner <heiko@sntech.de>
Cc: Caesar Wang <wxt@rock-chips.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
Cc: Vishal Bhoj <vishal.bhoj@linaro.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: devicetree@vger.kernel.org
Cc: Android Kernel Team <kernel-team@android.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/power/reset/Kconfig            |  9 +++++
 drivers/power/reset/Makefile           |  1 +
 drivers/power/reset/sram-reboot-mode.c | 66 ++++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+)
 create mode 100644 drivers/power/reset/sram-reboot-mode.c

diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index 76a6251..b033922 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -182,5 +182,14 @@ config SYSCON_REBOOT_MODE
 	 register, then the bootloader can read it to take different
 	 action according to the mode.
 
+config SRAM_REBOOT_MODE
+	bool "Generic SRAM reboot mode driver"
+	select REBOOT_MODE
+	help
+	 Say y here will enable reboot mode driver. This will
+	 get reboot mode arguments and store it in an SRAM
+	 address, then the bootloader can read it to take different
+	 action according to the mode.
+
 endif
 
diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
index afd05e9..4780f20 100644
--- a/drivers/power/reset/Makefile
+++ b/drivers/power/reset/Makefile
@@ -21,4 +21,5 @@ obj-$(CONFIG_POWER_RESET_SYSCON_POWEROFF) += syscon-poweroff.o
 obj-$(CONFIG_POWER_RESET_RMOBILE) += rmobile-reset.o
 obj-$(CONFIG_REBOOT_MODE) += reboot-mode.o
 obj-$(CONFIG_SYSCON_REBOOT_MODE) += syscon-reboot-mode.o
+obj-$(CONFIG_SRAM_REBOOT_MODE) += sram-reboot-mode.o
 
diff --git a/drivers/power/reset/sram-reboot-mode.c b/drivers/power/reset/sram-reboot-mode.c
new file mode 100644
index 0000000..065c3a8
--- /dev/null
+++ b/drivers/power/reset/sram-reboot-mode.c
@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) 2016, Linaro Limited
+ * Based on syscon-reboot-mode.c
+ * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * 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.
+ */
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/reboot.h>
+#include <linux/regmap.h>
+#include "reboot-mode.h"
+
+static void __iomem *reboot_reason_val_addr;
+
+static int sram_reboot_mode_write(int magic)
+{
+	writel(magic, reboot_reason_val_addr);
+	return 0;
+}
+
+static int sram_reboot_mode_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	int ret;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return PTR_ERR(res);
+
+	reboot_reason_val_addr = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+	if (IS_ERR(reboot_reason_val_addr))
+		return PTR_ERR(reboot_reason_val_addr);
+
+	ret = reboot_mode_register(&pdev->dev, sram_reboot_mode_write);
+	if (ret)
+		dev_err(&pdev->dev, "can't register reboot mode\n");
+
+	return ret;
+}
+
+static const struct of_device_id sram_reboot_mode_of_match[] = {
+	{ .compatible = "sram-reboot-mode" },
+	{}
+};
+
+static struct platform_driver sram_reboot_mode_driver = {
+	.probe = sram_reboot_mode_probe,
+	.driver = {
+		.name = "sram-reboot-mode",
+		.of_match_table = sram_reboot_mode_of_match,
+	},
+};
+module_platform_driver(sram_reboot_mode_driver);
+
+MODULE_AUTHOR("John Stultz <john.stultz@linaro.org>");
+MODULE_DESCRIPTION("SRAM reboot mode driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

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

* [RFC][PATCH 3/3] dts: hikey: Add hikey support for sram-reboot-mode
  2016-01-27  0:37 [RFC][PATCH 0/3] SRAM reboot mode driver John Stultz
  2016-01-27  0:37 ` [RFC][PATCH 1/3] dt-bindings: power: reset: Add document for sram-reboot-mode driver John Stultz
  2016-01-27  0:37 ` [RFC][PATCH 2/3] power: reset: Add " John Stultz
@ 2016-01-27  0:38 ` John Stultz
  2016-01-27 17:38   ` Rob Herring
  2016-01-27  8:50 ` [RFC][PATCH 0/3] SRAM reboot mode driver Vladimir Zapolskiy
  3 siblings, 1 reply; 7+ messages in thread
From: John Stultz @ 2016-01-27  0:38 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Andy Yan, Rob Herring, Arnd Bergmann,
	Thierry Reding, Heiko Stübner, Caesar Wang, Kees Cook,
	Guodong Xu, Haojian Zhuang, Vishal Bhoj, Bjorn Andersson,
	devicetree, Android Kernel Team

Add support to hikey dts and defconfig for the
sram-reboot-mode driver.

The dts entries added here should really be generated
by the UEFI firmware, and not be static in the dts,
since one may be using different firmware on HiKey.
But this patch provides an example of how the
sram-reboot-mode entry would otherwise look.

Cc: Andy Yan <andy.yan@rock-chips.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Heiko Stübner <heiko@sntech.de>
Cc: Caesar Wang <wxt@rock-chips.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
Cc: Vishal Bhoj <vishal.bhoj@linaro.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: devicetree@vger.kernel.org
Cc: Android Kernel Team <kernel-team@android.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 36 ++++++++++++++++++++++++++
 arch/arm64/configs/hikey_defconfig             |  3 +++
 2 files changed, 39 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
index 92f1704..67d12d0 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
@@ -52,6 +52,11 @@
 			      <0x0 0x0740f000 0x0 0x00001000>;	/* MCU firmware section */
 		};
 
+		reboot-reason@05f01000 {
+			no-map;
+			reg = <0x0 0x05f01000 0x0 0x00001000>;
+		};
+
 		mbox-buf@06dff000 {
 			no-map;
 			reg = <0x0 0x06dff000 0x0 0x00001000>;	/* Mailbox message buf */
@@ -71,6 +76,37 @@
 		ftrace-size	= <0x0 0x00020000>;
 	};
 
+
+	/* This entry should really be provided by the UEFI firmware */
+	sram@5f01000 {
+		compatible = "mmio-sram", "simple-bus";
+		reg = <0x0 0x05f01000 0x0 0x00001000>;
+		ranges = <0x0 0x0 0x05f01000 0x00001000>;
+
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		reboot-mode@0 {
+			compatible = "sram-reboot-mode";
+			reg = <0x0 0x4>;
+
+			none {
+				linux,mode = "none";
+				loader,magic = <0x77665501>;
+			};
+
+			bootloader {
+				linux,mode = "bootloader";
+				loader,magic = <0x77665500>;
+			};
+
+			recovery {
+				linux,mode = "recovery";
+				loader,magic = <0x77665502>;
+			};
+		};
+	};
+
 	smb {
 		uart0: uart@f8015000 {	/* console */
 			status = "ok";
diff --git a/arch/arm64/configs/hikey_defconfig b/arch/arm64/configs/hikey_defconfig
index a8375aa..e9db588 100644
--- a/arch/arm64/configs/hikey_defconfig
+++ b/arch/arm64/configs/hikey_defconfig
@@ -201,6 +201,7 @@ CONFIG_BLK_DEV_LOOP=y
 CONFIG_BLK_DEV_RAM=y
 CONFIG_BLK_DEV_RAM_SIZE=16384
 CONFIG_VIRTIO_BLK=y
+CONFIG_SRAM=y
 CONFIG_TI_ST=y
 CONFIG_ST_HCI=y
 # CONFIG_SCSI_PROC_FS is not set
@@ -266,6 +267,8 @@ CONFIG_SPI_PL022=y
 CONFIG_GPIO_SYSFS=y
 CONFIG_GPIO_XGENE=y
 CONFIG_POWER_RESET_SYSCON=y
+CONFIG_SYSCON_REBOOT_MODE=y
+CONFIG_SRAM_REBOOT_MODE=y
 CONFIG_THERMAL=y
 CONFIG_THERMAL_WRITABLE_TRIPS=y
 CONFIG_THERMAL_DEFAULT_GOV_POWER_ALLOCATOR=y
-- 
1.9.1

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

* Re: [RFC][PATCH 1/3] dt-bindings: power: reset: Add document for sram-reboot-mode driver
  2016-01-27  0:37 ` [RFC][PATCH 1/3] dt-bindings: power: reset: Add document for sram-reboot-mode driver John Stultz
@ 2016-01-27  1:11   ` Andy Yan
  0 siblings, 0 replies; 7+ messages in thread
From: Andy Yan @ 2016-01-27  1:11 UTC (permalink / raw)
  To: John Stultz, lkml
  Cc: Rob Herring, Arnd Bergmann, Thierry Reding, Heiko Stübner,
	Caesar Wang, Kees Cook, Guodong Xu, Haojian Zhuang, Vishal Bhoj,
	Bjorn Andersson, devicetree, Android Kernel Team

Hi John:
   Welcome.
   One nit.

On 2016年01月27日 08:37, John Stultz wrote:
> Add device tree binding document for reboot-mode driver

  Maybe you want to use "sram-reboot-mode" instead of "reboot-mode" here.
>
> Cc: Andy Yan <andy.yan@rock-chips.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Heiko Stübner <heiko@sntech.de>
> Cc: Caesar Wang <wxt@rock-chips.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
> Cc: Vishal Bhoj <vishal.bhoj@linaro.org>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: devicetree@vger.kernel.org
> Cc: Android Kernel Team <kernel-team@android.com>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>   .../bindings/power/reset/sram-reboot-mode.txt      | 47 ++++++++++++++++++++++
>   1 file changed, 47 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/power/reset/sram-reboot-mode.txt
>
> diff --git a/Documentation/devicetree/bindings/power/reset/sram-reboot-mode.txt b/Documentation/devicetree/bindings/power/reset/sram-reboot-mode.txt
> new file mode 100644
> index 0000000..33bb132
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/sram-reboot-mode.txt
> @@ -0,0 +1,47 @@
> +SRAM reboot mode driver
> +
> +This driver get reboot mode magic value form reboot-mode driver
> +and stores it in a SRAM address. Then the bootloader
> +can read it and take different action according to the magic
> +value stored.
> +
> +This DT node should be represented as a sub-node of a "mmio-sram", "simple-bus"
> +node.
> +
> +Required properties:
> +- compatible: should be "sram-reboot-mode"
> +- reg: offset from the sram range where to store the magic value(in bytes)
> +
> +The rest of the properties should follow the generic reboot-mode discription
> +found in reboot-mode.txt
> +
> +Example:
> +
> +	sram@5f01000 {
> +		compatible = "mmio-sram", "simple-bus";
> +		reg = <0x0 0x05f01000 0x0 0x00001000>;
> +		ranges = <0x0 0x0 0x05f01000 0x00001000>;
> +
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +
> +		reboot-mode@0 {
> +			compatible = "sram-reboot-mode";
> +			reg = <0x0 0x4>;
> +
> +			none {
> +				linux,mode = "none";
> +				loader,magic = <0x77665501>;
> +			};
> +
> +			bootloader {
> +				linux,mode = "bootloader";
> +				loader,magic = <0x77665500>;
> +			};
> +
> +			recovery {
> +				linux,mode = "recovery";
> +				loader,magic = <0x77665502>;
> +			};
> +		};
> +	};

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

* Re: [RFC][PATCH 0/3] SRAM reboot mode driver
  2016-01-27  0:37 [RFC][PATCH 0/3] SRAM reboot mode driver John Stultz
                   ` (2 preceding siblings ...)
  2016-01-27  0:38 ` [RFC][PATCH 3/3] dts: hikey: Add hikey support for sram-reboot-mode John Stultz
@ 2016-01-27  8:50 ` Vladimir Zapolskiy
  3 siblings, 0 replies; 7+ messages in thread
From: Vladimir Zapolskiy @ 2016-01-27  8:50 UTC (permalink / raw)
  To: John Stultz, lkml
  Cc: Andy Yan, Rob Herring, Arnd Bergmann, Thierry Reding,
	Heiko Stübner, Caesar Wang, Kees Cook, Guodong Xu,
	Haojian Zhuang, Vishal Bhoj, Bjorn Andersson, devicetree,
	Android Kernel Team

Hi John,

On 27.01.2016 02:37, John Stultz wrote:
> This patchset extends on Andy Yan's reboot mode driver
> work from here: https://lkml.org/lkml/2016/1/12/315
> 
> It adds reboot mode/reason support for devices that use
> an SRAM location to communicate with the bootloader.
> 
> Doing this via an SRAM subnode was a suggestion from
> Arnd, but I worry this implementation isn't yet ideal,
> since I spent quite a bit of time futzing with getting
> the sram dts entry to work properly. So I suspect there
> will be a number of suggestions for improvements.

sorry, I missed that discussion, what are the problems you've
encountered?
Probably SRAM driver is ready to serve your purposes?

I believe I'm screwing up some HiSilicon specifics, but here
is a rough draft, which you may find helpful:

1. Add SRAM node with defined reboot-reason area:

---8<---
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
index 8185251..c35f5ed 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
@@ -31,6 +31,20 @@
 		device_type = "memory";
 		reg = <0x0 0x0 0x0 0x40000000>;
 	};
+
+	sram@5f01000 {
+		compatible = "mmio-sram";
+		reg = <0x0 0x05f01000 0x0 0x00001000>;
+		ranges = <0x0 0x0 0x05f01000 0x00001000>;
+
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		reboot-reason: reboot-reason {
+			reg = <0x0 0x4>;
+			pool;
+		};
+	};
 };

 &uart2 {
---8<---

2. Reference reboot-reason node from reboot mode driver's node,
probably you need to add an optional property (here "reason-storage") to its
binding:

---8<---
		reboot-mode {
			compatible = "syscon-reboot-mode";
[snip]
			reason-storage = <&reboot-reason>;
		}
---8<---

3. In reboot-mode driver read or write to that SRAM partition:

---8<---
	storage = of_gen_pool_get(reboot_mode_np, "reason-storage", 0);
	if (!storage)
		goto no_storage;

	size = gen_pool_size(storage);
        base = gen_pool_alloc(storage, pool_size);
	if (!base)
		return -EINVAL;

	writel(0x77665501, base);
	gen_pool_free(storage, base, size);

no_storage:
	return 0;
---8<---

There is one noticeable benefit of this approach - you don't need
to write another driver and you don't need to add another DT binding.

With best wishes,
Vladimir

> Again, this series dependson Andy's patch set above,
> but also was developed & tested against the 4.1 based
> hikey tree, so at least the hikey dts patch won't apply.
> I'm mostly sending this out for just a rough initial
> review of the dts and conceptual usage of sram subnodes.
> 
> Also, it was pointed out that the hikey dts entry for
> this really should be added by the UEFI firmware, since
> alternative bootloaders may be used which do not support
> this feature. So the hikey dts patch isn't likely to ever
> go upstream, but its a useful illustration for how other
> devices might use this driver.
> 
> Cc: Andy Yan <andy.yan@rock-chips.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Heiko Stübner <heiko@sntech.de>
> Cc: Caesar Wang <wxt@rock-chips.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
> Cc: Vishal Bhoj <vishal.bhoj@linaro.org>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: devicetree@vger.kernel.org
> Cc: Android Kernel Team <kernel-team@android.com>
> 
> John Stultz (3):
>   dt-bindings: power: reset: Add document for sram-reboot-mode driver
>   power: reset: Add sram-reboot-mode driver
>   dts: hikey: Add hikey support for sram-reboot-mode
> 
>  .../bindings/power/reset/sram-reboot-mode.txt      | 47 +++++++++++++++
>  arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts     | 36 ++++++++++++
>  arch/arm64/configs/hikey_defconfig                 |  3 +
>  drivers/power/reset/Kconfig                        |  9 +++
>  drivers/power/reset/Makefile                       |  1 +
>  drivers/power/reset/sram-reboot-mode.c             | 66 ++++++++++++++++++++++
>  6 files changed, 162 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/reset/sram-reboot-mode.txt
>  create mode 100644 drivers/power/reset/sram-reboot-mode.c
> 

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

* Re: [RFC][PATCH 3/3] dts: hikey: Add hikey support for sram-reboot-mode
  2016-01-27  0:38 ` [RFC][PATCH 3/3] dts: hikey: Add hikey support for sram-reboot-mode John Stultz
@ 2016-01-27 17:38   ` Rob Herring
  0 siblings, 0 replies; 7+ messages in thread
From: Rob Herring @ 2016-01-27 17:38 UTC (permalink / raw)
  To: John Stultz
  Cc: lkml, Andy Yan, Arnd Bergmann, Thierry Reding,
	Heiko Stübner, Caesar Wang, Kees Cook, Guodong Xu,
	Haojian Zhuang, Vishal Bhoj, Bjorn Andersson, devicetree,
	Android Kernel Team

On Tue, Jan 26, 2016 at 6:38 PM, John Stultz <john.stultz@linaro.org> wrote:
> Add support to hikey dts and defconfig for the
> sram-reboot-mode driver.
>
> The dts entries added here should really be generated
> by the UEFI firmware, and not be static in the dts,
> since one may be using different firmware on HiKey.
> But this patch provides an example of how the
> sram-reboot-mode entry would otherwise look.
>
> Cc: Andy Yan <andy.yan@rock-chips.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Heiko Stübner <heiko@sntech.de>
> Cc: Caesar Wang <wxt@rock-chips.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
> Cc: Vishal Bhoj <vishal.bhoj@linaro.org>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: devicetree@vger.kernel.org
> Cc: Android Kernel Team <kernel-team@android.com>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>  arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 36 ++++++++++++++++++++++++++
>  arch/arm64/configs/hikey_defconfig             |  3 +++
>  2 files changed, 39 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
> index 92f1704..67d12d0 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
> +++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
> @@ -52,6 +52,11 @@
>                               <0x0 0x0740f000 0x0 0x00001000>;  /* MCU firmware section */
>                 };
>
> +               reboot-reason@05f01000 {
> +                       no-map;
> +                       reg = <0x0 0x05f01000 0x0 0x00001000>;
> +               };
> +

This hunk should be dropped. These regions are for main memory.

>                 mbox-buf@06dff000 {
>                         no-map;
>                         reg = <0x0 0x06dff000 0x0 0x00001000>;  /* Mailbox message buf */
> @@ -71,6 +76,37 @@
>                 ftrace-size     = <0x0 0x00020000>;
>         };
>
> +
> +       /* This entry should really be provided by the UEFI firmware */
> +       sram@5f01000 {
> +               compatible = "mmio-sram", "simple-bus";

The suggestion to put "simple-bus" here was really just for debugging.
Really, the mmio-sram driver should call of_platform_default_populate
to probe it's children.

> +               reg = <0x0 0x05f01000 0x0 0x00001000>;
> +               ranges = <0x0 0x0 0x05f01000 0x00001000>;
> +
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +
> +               reboot-mode@0 {
> +                       compatible = "sram-reboot-mode";
> +                       reg = <0x0 0x4>;
> +
> +                       none {
> +                               linux,mode = "none";
> +                               loader,magic = <0x77665501>;
> +                       };
> +
> +                       bootloader {
> +                               linux,mode = "bootloader";
> +                               loader,magic = <0x77665500>;
> +                       };
> +
> +                       recovery {
> +                               linux,mode = "recovery";
> +                               loader,magic = <0x77665502>;
> +                       };
> +               };
> +       };
> +
>         smb {
>                 uart0: uart@f8015000 {  /* console */
>                         status = "ok";
> diff --git a/arch/arm64/configs/hikey_defconfig b/arch/arm64/configs/hikey_defconfig
> index a8375aa..e9db588 100644
> --- a/arch/arm64/configs/hikey_defconfig
> +++ b/arch/arm64/configs/hikey_defconfig
> @@ -201,6 +201,7 @@ CONFIG_BLK_DEV_LOOP=y
>  CONFIG_BLK_DEV_RAM=y
>  CONFIG_BLK_DEV_RAM_SIZE=16384
>  CONFIG_VIRTIO_BLK=y
> +CONFIG_SRAM=y
>  CONFIG_TI_ST=y
>  CONFIG_ST_HCI=y
>  # CONFIG_SCSI_PROC_FS is not set
> @@ -266,6 +267,8 @@ CONFIG_SPI_PL022=y
>  CONFIG_GPIO_SYSFS=y
>  CONFIG_GPIO_XGENE=y
>  CONFIG_POWER_RESET_SYSCON=y
> +CONFIG_SYSCON_REBOOT_MODE=y
> +CONFIG_SRAM_REBOOT_MODE=y
>  CONFIG_THERMAL=y
>  CONFIG_THERMAL_WRITABLE_TRIPS=y
>  CONFIG_THERMAL_DEFAULT_GOV_POWER_ALLOCATOR=y
> --
> 1.9.1
>

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

end of thread, other threads:[~2016-01-27 17:38 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-27  0:37 [RFC][PATCH 0/3] SRAM reboot mode driver John Stultz
2016-01-27  0:37 ` [RFC][PATCH 1/3] dt-bindings: power: reset: Add document for sram-reboot-mode driver John Stultz
2016-01-27  1:11   ` Andy Yan
2016-01-27  0:37 ` [RFC][PATCH 2/3] power: reset: Add " John Stultz
2016-01-27  0:38 ` [RFC][PATCH 3/3] dts: hikey: Add hikey support for sram-reboot-mode John Stultz
2016-01-27 17:38   ` Rob Herring
2016-01-27  8:50 ` [RFC][PATCH 0/3] SRAM reboot mode driver Vladimir Zapolskiy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.