Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1318495 > unrolled thread

[RFC][PATCH 0/3] SRAM reboot mode driver

Started byJohn Stultz <john.stultz@linaro.org>
First post2016-01-27 01:40 +0100
Last post2016-01-27 10:00 +0100
Articles 7 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [RFC][PATCH 0/3] SRAM reboot mode driver John Stultz <john.stultz@linaro.org> - 2016-01-27 01:40 +0100
    [RFC][PATCH 3/3] dts: hikey: Add hikey support for sram-reboot-mode John Stultz <john.stultz@linaro.org> - 2016-01-27 01:40 +0100
      Re: [RFC][PATCH 3/3] dts: hikey: Add hikey support for sram-reboot-mode Rob Herring <robh@kernel.org> - 2016-01-27 18:40 +0100
    [RFC][PATCH 2/3] power: reset: Add sram-reboot-mode driver John Stultz <john.stultz@linaro.org> - 2016-01-27 01:40 +0100
    [RFC][PATCH 1/3] dt-bindings: power: reset: Add document for sram-reboot-mode driver John Stultz <john.stultz@linaro.org> - 2016-01-27 01:40 +0100
      Re: [RFC][PATCH 1/3] dt-bindings: power: reset: Add document for  sram-reboot-mode driver Andy Yan <andy.yan@rock-chips.com> - 2016-01-27 02:20 +0100
    Re: [RFC][PATCH 0/3] SRAM reboot mode driver Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> - 2016-01-27 10:00 +0100

#1318495 — [RFC][PATCH 0/3] SRAM reboot mode driver

FromJohn Stultz <john.stultz@linaro.org>
Date2016-01-27 01:40 +0100
Subject[RFC][PATCH 0/3] SRAM reboot mode driver
Message-ID<qVmjn-3ND-1@gated-at.bofh.it>
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

[toc] | [next] | [standalone]

#1318496 — [RFC][PATCH 3/3] dts: hikey: Add hikey support for sram-reboot-mode

FromJohn Stultz <john.stultz@linaro.org>
Date2016-01-27 01:40 +0100
Subject[RFC][PATCH 3/3] dts: hikey: Add hikey support for sram-reboot-mode
Message-ID<qVmjn-3ND-3@gated-at.bofh.it>
In reply to#1318495
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

[toc] | [prev] | [next] | [standalone]

#1319183 — Re: [RFC][PATCH 3/3] dts: hikey: Add hikey support for sram-reboot-mode

FromRob Herring <robh@kernel.org>
Date2016-01-27 18:40 +0100
SubjectRe: [RFC][PATCH 3/3] dts: hikey: Add hikey support for sram-reboot-mode
Message-ID<qVCeu-72J-29@gated-at.bofh.it>
In reply to#1318496
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
>

[toc] | [prev] | [next] | [standalone]

#1318498 — [RFC][PATCH 2/3] power: reset: Add sram-reboot-mode driver

FromJohn Stultz <john.stultz@linaro.org>
Date2016-01-27 01:40 +0100
Subject[RFC][PATCH 2/3] power: reset: Add sram-reboot-mode driver
Message-ID<qVmjn-3ND-7@gated-at.bofh.it>
In reply to#1318495
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

[toc] | [prev] | [next] | [standalone]

#1318500 — [RFC][PATCH 1/3] dt-bindings: power: reset: Add document for sram-reboot-mode driver

FromJohn Stultz <john.stultz@linaro.org>
Date2016-01-27 01:40 +0100
Subject[RFC][PATCH 1/3] dt-bindings: power: reset: Add document for sram-reboot-mode driver
Message-ID<qVmjn-3ND-13@gated-at.bofh.it>
In reply to#1318495
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

[toc] | [prev] | [next] | [standalone]

#1318522 — Re: [RFC][PATCH 1/3] dt-bindings: power: reset: Add document for sram-reboot-mode driver

FromAndy Yan <andy.yan@rock-chips.com>
Date2016-01-27 02:20 +0100
SubjectRe: [RFC][PATCH 1/3] dt-bindings: power: reset: Add document for sram-reboot-mode driver
Message-ID<qVmW6-4h1-17@gated-at.bofh.it>
In reply to#1318500
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>;
> +			};
> +		};
> +	};

[toc] | [prev] | [next] | [standalone]

#1318759

FromVladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Date2016-01-27 10:00 +0100
Message-ID<qVu7h-TW-21@gated-at.bofh.it>
In reply to#1318495
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
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web