[PATCH v1 0/6] misc: add reboot mode driver

[PATCH v1 0/6] misc: add reboot mode driver

[Andy Yan]

This driver parse the reboot commands like "reboot loader"
and "reboot recovery" to get a boot mode described in the
device tree , then call the vendor specific write interfae
to store the boot mode in some place like special register
or sram , which can be read by the bootloader after system
reboot.

This is commonly done on Android based devices, in order to
reboot the device into fastboot or recovery mode.

Before this patch , I have try some hack on[0], and then found
John Stultz also doing the same work[1].

As John is busy these days, I go on with this work.

[0]https://patchwork.kernel.org/patch/7647751/
[1]https://patchwork.kernel.org/patch/7802391/

Changes in v1:
- fix the embarrassed compile warning
- correct the maskrom magic number
- check for the normal reboot
- correct the maskrom magic number
- use macro defined in rockchip_boot-mode.h for reboot-mode DT node

Andy Yan (6):
  dt-bindings: misc: add document for reboot-mode driver
  dt-bindings: soc: add document for rockchip reboot-mode driver
  misc: add reboot mode driver
  soc: rockchip: add reboot mode driver
  ARM: dts: rockchip: add reboot-mode node
  ARM64: dts: rockchip: add reboot-mode node

 .../devicetree/bindings/misc/reboot-mode.txt       | 41 +++++++++
 .../bindings/soc/rockchip/rockchip,reboot-mode.txt | 39 +++++++++
 arch/arm/boot/dts/rk3288.dtsi                      | 26 ++++++
 arch/arm/boot/dts/rk3xxx.dtsi                      | 26 ++++++
 arch/arm64/boot/dts/rockchip/rk3368.dtsi           | 26 ++++++
 drivers/misc/Kconfig                               |  7 ++
 drivers/misc/Makefile                              |  1 +
 drivers/misc/reboot_mode.c                         | 96 ++++++++++++++++++++++
 drivers/soc/rockchip/Kconfig                       |  9 ++
 drivers/soc/rockchip/Makefile                      |  1 +
 drivers/soc/rockchip/reboot.c                      | 68 +++++++++++++++
 include/dt-bindings/soc/rockchip_boot-mode.h       | 30 +++++++
 include/linux/reboot.h                             |  4 +-
 13 files changed, 373 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/misc/reboot-mode.txt
 create mode 100644 Documentation/devicetree/bindings/soc/rockchip/rockchip,reboot-mode.txt
 create mode 100644 drivers/misc/reboot_mode.c
 create mode 100644 drivers/soc/rockchip/reboot.c
 create mode 100644 include/dt-bindings/soc/rockchip_boot-mode.h

-- 
1.9.1

[PATCH v1 1/6] dt-bindings: misc: add document for reboot-mode driver

[Andy Yan]

add device tree bindings document for reboot-mode driver

Signed-off-by: Andy Yan <andy.yan@rock-chips.com>

---

Changes in v1: None

 .../devicetree/bindings/misc/reboot-mode.txt       | 41 ++++++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/reboot-mode.txt

diff --git a/Documentation/devicetree/bindings/misc/reboot-mode.txt b/Documentation/devicetree/bindings/misc/reboot-mode.txt
new file mode 100644
index 0000000..082bc0c
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/reboot-mode.txt
@@ -0,0 +1,41 @@
+Generic reboot mode communication driver
+
+This driver get reboot mode arguments from userspace
+and stores it in special register or ram . Then the
+bootloader will read it and take different action
+according the argument stored.
+
+Required properties:
+        - compatible = "reboot-mode" or other vendor compatible string;
+
+Each mode is represented as a sub-node of reboot_mode:
+
+Subnode required properties:
+        - linux,mode: reboot mode command,such as "loader","recovery", "fastboot".
+        - linux,magic: magic number for the mode, this is vendor specific.
+
+example:
+	reboot_mode {
+		compatible = "rockchip,reboot-mode";
+		rockchip,regmap = <&pmu>;
+		offset = <0x40>;
+		loader {
+			linux,mode = "loader";
+			linux,magic = <0x5242C301>;
+		};
+
+		maskrom {
+			linux,mode = "maskrom";
+			linux,magic = <0x5242C302>;
+		};
+
+		recovery {
+			linux,mode = "recovery";
+			linux,magic = <0x5242C303>;
+		};
+
+		fastboot {
+			linux,mode = "fastboot";
+			linux,magic = <0x5242C309>;
+		};
+        };
-- 
1.9.1

Re: [PATCH v1 1/6] dt-bindings: misc: add document for reboot-mode driver

[Rob Herring]

On Tue, Dec 22, 2015 at 05:05:24PM +0800, Andy Yan wrote:
> add device tree bindings document for reboot-mode driver
> 
> Signed-off-by: Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> 
> ---
> 
> Changes in v1: None
> 
>  .../devicetree/bindings/misc/reboot-mode.txt       | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/reboot-mode.txt
> 
> diff --git a/Documentation/devicetree/bindings/misc/reboot-mode.txt b/Documentation/devicetree/bindings/misc/reboot-mode.txt
> new file mode 100644
> index 0000000..082bc0c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/reboot-mode.txt
> @@ -0,0 +1,41 @@
> +Generic reboot mode communication driver

You're not describing a driver. It is a mapping of boot modes to values.
> +
> +This driver get reboot mode arguments from userspace

Coming from userspace is a Linuxism.

> +and stores it in special register or ram . Then the
> +bootloader will read it and take different action
> +according the argument stored.
> +
> +Required properties:
> +        - compatible = "reboot-mode" or other vendor compatible string;
> +
> +Each mode is represented as a sub-node of reboot_mode:
> +
> +Subnode required properties:
> +        - linux,mode: reboot mode command,such as "loader","recovery", "fastboot".
> +        - linux,magic: magic number for the mode, this is vendor specific.
> +
> +example:
> +	reboot_mode {
> +		compatible = "rockchip,reboot-mode";
> +		rockchip,regmap = <&pmu>;
> +		offset = <0x40>;
> +		loader {
> +			linux,mode = "loader";
> +			linux,magic = <0x5242C301>;
> +		};

These can be much more simply expressed as:

loader = <0x5242c301>;

I would like to see the property names here standardized as much as
possible. I'm not sure if we can define the properties as a u32 or need
some flexibility here.

Rob

Re: [PATCH v1 1/6] dt-bindings: misc: add document for reboot-mode driver

[Moritz Fischer]

Rob,

On Tue, Dec 22, 2015 at 4:32 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:

> I would like to see the property names here standardized as much as
> possible. I'm not sure if we can define the properties as a u32 or need
> some flexibility here.

Quick question, semi on topic. I have a similar case (and a driver in
the pipeline),
however all I got is a (persistent over reboot) hardware register to
which I can write whatever.
On the other side I'd have u-boot pull out the value, but as it's up
to u-boot to
implement whatever it does with this info I'm kinda struggling with the idea
of a standardized binding. In my case I'd like it to go into 'safe
mode' or 'firmware update'.

With loader = <0xace0ba5e>, <0xdeadbeef> ... and loader-names <> I
could have u-boot read the values
from fdt at boot. Then again I'm not describing hardware, but specific
settings ...

Would that be acceptable?

Cheers,

Moritz
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 1/6] dt-bindings: misc: add document for reboot-mode driver

[Andy Yan]

Hi Rob:

On 2015年12月23日 08:32, Rob Herring wrote:
> On Tue, Dec 22, 2015 at 05:05:24PM +0800, Andy Yan wrote:
>> add device tree bindings document for reboot-mode driver
>>
>> Signed-off-by: Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>>
>> ---
>>
>> Changes in v1: None
>>
>>   .../devicetree/bindings/misc/reboot-mode.txt       | 41 ++++++++++++++++++++++
>>   1 file changed, 41 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/misc/reboot-mode.txt
>>
>> diff --git a/Documentation/devicetree/bindings/misc/reboot-mode.txt b/Documentation/devicetree/bindings/misc/reboot-mode.txt
>> new file mode 100644
>> index 0000000..082bc0c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/reboot-mode.txt
>> @@ -0,0 +1,41 @@
>> +Generic reboot mode communication driver
> You're not describing a driver. It is a mapping of boot modes to values.
>> +
>> +This driver get reboot mode arguments from userspace
> Coming from userspace is a Linuxism.
>
>> +and stores it in special register or ram . Then the
>> +bootloader will read it and take different action
>> +according the argument stored.
>> +
>> +Required properties:
>> +        - compatible = "reboot-mode" or other vendor compatible string;
>> +
>> +Each mode is represented as a sub-node of reboot_mode:
>> +
>> +Subnode required properties:
>> +        - linux,mode: reboot mode command,such as "loader","recovery", "fastboot".
>> +        - linux,magic: magic number for the mode, this is vendor specific.
>> +
>> +example:
>> +	reboot_mode {
>> +		compatible = "rockchip,reboot-mode";
>> +		rockchip,regmap = <&pmu>;
>> +		offset = <0x40>;
>> +		loader {
>> +			linux,mode = "loader";
>> +			linux,magic = <0x5242C301>;
>> +		};
> These can be much more simply expressed as:
>
> loader = <0x5242c301>;
    how about :
    loader,magic= <0x5242c301>; ?
>
> I would like to see the property names here standardized as much as
> possible. I'm not sure if we can define the properties as a u32 or need
> some flexibility here.
>
> Rob
>
>
>


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 2/6] dt-bindings: soc: add document for rockchip reboot-mode driver

[Andy Yan]

Add device tree binding document for rockchip reboot-mode driver

Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
---

Changes in v1: None

 .../bindings/soc/rockchip/rockchip,reboot-mode.txt | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/rockchip/rockchip,reboot-mode.txt

diff --git a/Documentation/devicetree/bindings/soc/rockchip/rockchip,reboot-mode.txt b/Documentation/devicetree/bindings/soc/rockchip/rockchip,reboot-mode.txt
new file mode 100644
index 0000000..cdb5093
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/rockchip/rockchip,reboot-mode.txt
@@ -0,0 +1,39 @@
+Rockchip reboot mode driver
+
+This driver get reboot mode arguments from userspace
+and stores it in special register. Then the bootloader
+will read it and take different action according the
+argument stored.
+
+Required properties:
+- compatible: should be "rockchip,reboot-mode"
+- regmap: this is phandle to the register map node
+- offset: offset in the register map for the storage register (in bytes)
+The rest of the properties should follow the generic reboot-mode discription
+found in ../../misc/reboot-mode.txt
+
+Examples:
+	reboot_mode {
+		compatible = "rockchip,reboot-mode";
+		rockchip,regmap = <&pmu>;
+		offset = <0x40>;
+		loader {
+			linux,mode = "loader";
+			linux,magic = <BOOT_LOADER>;
+		};
+
+		maskrom {
+			linux,mode = "maskrom";
+			linux,magic = <BOOT_MASKROM>;
+		};
+
+		recovery {
+			linux,mode = "recovery";
+			linux,magic = <BOOT_RECOVERY>;
+		};
+
+		fastboot {
+			linux,mode = "fastboot";
+			linux,magic = <BOOT_FASTBOOT>;
+		};
+	};
-- 
1.9.1

[PATCH v1 3/6] misc: add reboot mode driver

[Andy Yan]

This driver parse the reboot commands like "reboot loader"
and "reboot recovery" to get a boot mode described in the
device tree , then call the vendor specific write interfae
to store the boot mode in some place like special register
or sram , which can be read by the bootloader after system
reboot, then the bootloader can take different action according
to the mode stored.

This is commonly used on Android based devices, in order to
reboot the device into fastboot or recovery mode.

Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
---

Changes in v1: None

 drivers/misc/Kconfig       |  7 ++++
 drivers/misc/Makefile      |  1 +
 drivers/misc/reboot_mode.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/reboot.h     |  4 +-
 4 files changed, 107 insertions(+), 1 deletion(-)
 create mode 100644 drivers/misc/reboot_mode.c

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 22892c7..0cceb74 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -525,6 +525,13 @@ config VEXPRESS_SYSCFG
 	  bus. System Configuration interface is one of the possible means
 	  of generating transactions on this bus.
 
+config REBOOT_MODE
+	tristate
+	depends on OF
+	help
+	 This driver will help to pass the system reboot mode
+	 to bootloader
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 537d7f3..6ada5de 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -56,3 +56,4 @@ obj-$(CONFIG_GENWQE)		+= genwqe/
 obj-$(CONFIG_ECHO)		+= echo/
 obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
 obj-$(CONFIG_CXL_BASE)		+= cxl/
+obj-$(CONFIG_REBOOT_MODE)       += reboot_mode.o
diff --git a/drivers/misc/reboot_mode.c b/drivers/misc/reboot_mode.c
new file mode 100644
index 0000000..16bbaab
--- /dev/null
+++ b/drivers/misc/reboot_mode.c
@@ -0,0 +1,96 @@
+/*
+ * Copyright (c) 2015, 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/device.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/reboot.h>
+
+struct mode_info {
+	const char *mode;
+	int magic;
+	struct list_head list;
+};
+
+struct reboot_mode_driver {
+	struct list_head head;
+	int (*write)(int magic);
+	struct notifier_block reboot_notifier;
+};
+
+static int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
+				 const char *cmd)
+{
+	int magic = 0;
+	struct mode_info *info;
+
+	if (cmd) {
+		list_for_each_entry(info, &reboot->head, list) {
+			if (!strcmp(info->mode, cmd)) {
+				magic = info->magic;
+				break;
+			}
+		}
+	}
+
+	return magic;
+}
+
+static int reboot_mode_notify(struct notifier_block *this,
+			      unsigned long mode, void *cmd)
+{
+	struct reboot_mode_driver *reboot;
+	int magic;
+
+	reboot = container_of(this, struct reboot_mode_driver, reboot_notifier);
+	magic = get_reboot_mode_magic(reboot, cmd);
+	reboot->write(magic);
+
+	return NOTIFY_DONE;
+}
+
+int reboot_mode_register(struct device *dev, int (*write)(int))
+{
+	struct reboot_mode_driver *reboot;
+	struct mode_info *info;
+	struct device_node *child;
+	int ret;
+
+	reboot = devm_kzalloc(dev, sizeof(*reboot), GFP_KERNEL);
+	if (!reboot)
+		return -ENOMEM;
+
+	reboot->write = write;
+	INIT_LIST_HEAD(&reboot->head);
+	for_each_child_of_node(dev->of_node, child) {
+		info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
+		if (!info)
+			return -ENOMEM;
+		info->mode = of_get_property(child, "linux,mode", NULL);
+		if (of_property_read_u32(child, "linux,magic", &info->magic)) {
+			dev_err(dev, "reboot mode %s without magic number\n",
+				info->mode);
+			devm_kfree(dev, info);
+			continue;
+		}
+		list_add_tail(&info->list, &reboot->head);
+	}
+	reboot->reboot_notifier.notifier_call = reboot_mode_notify;
+	ret = register_reboot_notifier(&reboot->reboot_notifier);
+	if (ret)
+		dev_err(dev, "can't register reboot notifier\n");
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(reboot_mode_register);
+
+MODULE_AUTHOR("Andy Yan <andy.yan@rock-chips.com");
+MODULE_DESCRIPTION("System reboot mode driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/reboot.h b/include/linux/reboot.h
index a7ff409..f523c32 100644
--- a/include/linux/reboot.h
+++ b/include/linux/reboot.h
@@ -1,7 +1,7 @@
 #ifndef _LINUX_REBOOT_H
 #define _LINUX_REBOOT_H
 
-
+#include <linux/device.h>
 #include <linux/notifier.h>
 #include <uapi/linux/reboot.h>
 
@@ -42,6 +42,8 @@ extern int register_restart_handler(struct notifier_block *);
 extern int unregister_restart_handler(struct notifier_block *);
 extern void do_kernel_restart(char *cmd);
 
+extern int reboot_mode_register(struct device *dev, int (*write)(int));
+
 /*
  * Architecture-specific implementations of sys_reboot commands.
  */
-- 
1.9.1

[PATCH v1 4/6] soc: rockchip: add reboot mode driver

[Andy Yan]

rockchip platform have a protocol to pass the kernel reboot
mode to bootloader by some special registers when system reboot.
By this way the bootloader can take different action according
to the different kernel reboot mode, for example, command
"reboot loader" will reboot the board to rockusb mode, this is
a very convenient way to get the board enter download mode.

Signed-off-by: Andy Yan <andy.yan@rock-chips.com>

---

Changes in v1:
- fix the embarrassed compile warning
- correct the maskrom magic number
- check for the normal reboot

 drivers/soc/rockchip/Kconfig                 |  9 ++++
 drivers/soc/rockchip/Makefile                |  1 +
 drivers/soc/rockchip/reboot.c                | 68 ++++++++++++++++++++++++++++
 include/dt-bindings/soc/rockchip_boot-mode.h | 30 ++++++++++++
 4 files changed, 108 insertions(+)
 create mode 100644 drivers/soc/rockchip/reboot.c
 create mode 100644 include/dt-bindings/soc/rockchip_boot-mode.h

diff --git a/drivers/soc/rockchip/Kconfig b/drivers/soc/rockchip/Kconfig
index 7140ff8..42525f8 100644
--- a/drivers/soc/rockchip/Kconfig
+++ b/drivers/soc/rockchip/Kconfig
@@ -15,4 +15,13 @@ config ROCKCHIP_PM_DOMAINS
 
           If unsure, say N.
 
+config ROCKCHIP_REBOOT
+	bool "Rockchip reboot mode driver"
+	select REBOOT_MODE
+	help
+	  Say y here will enable reboot mode driver. This will
+	  get reboot mode arguments from userspace and store it
+	  in special register, then the bootloader will read it
+	  to take different action according to the mode.
+
 endif
diff --git a/drivers/soc/rockchip/Makefile b/drivers/soc/rockchip/Makefile
index 3d73d06..9817496 100644
--- a/drivers/soc/rockchip/Makefile
+++ b/drivers/soc/rockchip/Makefile
@@ -2,3 +2,4 @@
 # Rockchip Soc drivers
 #
 obj-$(CONFIG_ROCKCHIP_PM_DOMAINS) += pm_domains.o
+obj-$(CONFIG_ROCKCHIP_REBOOT) += reboot.o
diff --git a/drivers/soc/rockchip/reboot.c b/drivers/soc/rockchip/reboot.c
new file mode 100644
index 0000000..7024532
--- /dev/null
+++ b/drivers/soc/rockchip/reboot.c
@@ -0,0 +1,68 @@
+/*
+ * Copyright (c) 2014, 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/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/reboot.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+#include <dt-bindings/soc/rockchip_boot-mode.h>
+
+static struct regmap *map;
+static u32 offset;
+
+static int rockchip_reboot_mode_write(int magic)
+{
+	if (!magic)
+		magic = BOOT_NORMAL;
+
+	regmap_write(map, offset, magic);
+
+	return 0;
+}
+
+static int rockchip_reboot_probe(struct platform_device *pdev)
+{
+	int ret;
+
+	map = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+					      "rockchip,regmap");
+	if (IS_ERR(map))
+		return PTR_ERR(map);
+
+	if (of_property_read_u32(pdev->dev.of_node, "offset", &offset))
+		return -EINVAL;
+
+	ret = reboot_mode_register(&pdev->dev, rockchip_reboot_mode_write);
+	if (ret)
+		dev_err(&pdev->dev, "can't register reboot mode\n");
+
+	return ret;
+}
+
+static const struct of_device_id rockchip_reboot_of_match[] = {
+	{ .compatible = "rockchip,reboot-mode" },
+	{}
+};
+
+static struct platform_driver rockchip_reboot_driver = {
+	.probe = rockchip_reboot_probe,
+	.driver = {
+		.name = "rockchip-reboot",
+		.of_match_table = rockchip_reboot_of_match,
+	},
+};
+module_platform_driver(rockchip_reboot_driver);
+
+MODULE_AUTHOR("Andy Yan <andy.yan@rock-chips.com");
+MODULE_DESCRIPTION("Rockchip platform reboot notifier driver");
+MODULE_LICENSE("GPL");
diff --git a/include/dt-bindings/soc/rockchip_boot-mode.h b/include/dt-bindings/soc/rockchip_boot-mode.h
new file mode 100644
index 0000000..eedf113
--- /dev/null
+++ b/include/dt-bindings/soc/rockchip_boot-mode.h
@@ -0,0 +1,30 @@
+#ifndef __ROCKCHIP_BOOT_MODE_H
+#define __ROCKCHIP_BOOT_MODE_H
+
+/*high 24 bits is tag, low 8 bits is type*/
+#define	REBOOT_FLAG		0x5242C300
+/* normal boot */
+#define	BOOT_NORMAL		(REBOOT_FLAG + 0)
+/* enter loader rockusb mode */
+#define	BOOT_LOADER		(REBOOT_FLAG + 1)
+/* enter maskrom rockusb mode */
+#define	BOOT_MASKROM		(0xEF08A53C)
+/* enter recovery */
+#define	BOOT_RECOVERY		(REBOOT_FLAG + 3)
+/* do not enter recover */
+#define	BOOT_NORECOVER		(REBOOT_FLAG + 4)
+/* boot second OS*/
+#define	BOOT_SECONDOS		(REBOOT_FLAG + 5)
+/* enter recover and wipe data. */
+#define	BOOT_WIPEDATA		(REBOOT_FLAG + 6)
+/* enter recover and wipe all data. */
+#define	BOOT_WIPEALL		(REBOOT_FLAG + 7)
+/* check firmware img with backup part*/
+#define	BOOT_CHECKIMG		(REBOOT_FLAG + 8)
+ /* enter fast boot mode */
+#define	BOOT_FASTBOOT		(REBOOT_FLAG + 9)
+#define	BOOT_SECUREBOOT_DISABLE	(REBOOT_FLAG + 10)
+/* enter charge mode */
+#define	BOOT_CHARGING		(REBOOT_FLAG + 11)
+
+#endif
-- 
1.9.1

[PATCH v1 5/6] ARM: dts: rockchip: add reboot-mode node

[Andy Yan]

Add reboot mode driver DT node for rk3xxx,rk3288 platform

Signed-off-by: Andy Yan <andy.yan@rock-chips.com>

---

Changes in v1:
- correct the maskrom magic number
- use macro defined in rockchip_boot-mode.h for reboot-mode DT node

 arch/arm/boot/dts/rk3288.dtsi | 26 ++++++++++++++++++++++++++
 arch/arm/boot/dts/rk3xxx.dtsi | 26 ++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 04ea209..c6ea207 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -45,6 +45,7 @@
 #include <dt-bindings/clock/rk3288-cru.h>
 #include <dt-bindings/thermal/thermal.h>
 #include <dt-bindings/power/rk3288-power.h>
+#include <dt-bindings/soc/rockchip_boot-mode.h>
 #include "skeleton.dtsi"
 
 / {
@@ -170,6 +171,31 @@
 		};
 	};
 
+	reboot_mode {
+		compatible = "rockchip,reboot-mode";
+		rockchip,regmap = <&pmu>;
+		offset = <0x94>;
+		loader {
+			linux,mode = "loader";
+			linux,magic = <BOOT_LOADER>;
+		};
+
+		maskrom {
+			linux,mode = "maskrom";
+			linux,magic = <BOOT_MASKROM>;
+		};
+
+		recovery {
+			linux,mode = "recovery";
+			linux,magic = <BOOT_RECOVERY>;
+		};
+
+		fastboot {
+			linux,mode = "fastboot";
+			linux,magic = <BOOT_FASTBOOT>;
+		};
+	};
+
 	reserved-memory {
 		#address-cells = <1>;
 		#size-cells = <1>;
diff --git a/arch/arm/boot/dts/rk3xxx.dtsi b/arch/arm/boot/dts/rk3xxx.dtsi
index 4497d28..735eef4 100644
--- a/arch/arm/boot/dts/rk3xxx.dtsi
+++ b/arch/arm/boot/dts/rk3xxx.dtsi
@@ -43,6 +43,7 @@
 
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/soc/rockchip_boot-mode.h>
 #include "skeleton.dtsi"
 
 / {
@@ -103,6 +104,31 @@
 		};
 	};
 
+	reboot_mode {
+		compatible = "rockchip,reboot-mode";
+		rockchip,regmap = <&pmu>;
+		offset = <0x40>;
+		loader {
+			linux,mode = "loader";
+			linux,magic = <BOOT_LOADER>;
+		};
+
+		maskrom {
+			linux,mode = "maskrom";
+			linux,magic = <BOOT_MASKROM>;
+		};
+
+		recovery {
+			linux,mode = "recovery";
+			linux,magic = <BOOT_RECOVERY>;
+		};
+
+		fastboot {
+			linux,mode = "fastboot";
+			linux,magic = <BOOT_FASTBOOT>;
+		};
+	};
+
 	xin24m: oscillator {
 		compatible = "fixed-clock";
 		clock-frequency = <24000000>;
-- 
1.9.1

Re: [PATCH v1 5/6] ARM: dts: rockchip: add reboot-mode node

[Heiko Stübner]

Hi Andy,

Am Dienstag, 22. Dezember 2015, 17:16:52 schrieb Andy Yan:
> Add reboot mode driver DT node for rk3xxx,rk3288 platform

in general, I like that concept a lot :-) . Below some small issues.


> Signed-off-by: Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> 
> ---
> 
> Changes in v1:
> - correct the maskrom magic number
> - use macro defined in rockchip_boot-mode.h for reboot-mode DT node
> 
>  arch/arm/boot/dts/rk3288.dtsi | 26 ++++++++++++++++++++++++++
>  arch/arm/boot/dts/rk3xxx.dtsi | 26 ++++++++++++++++++++++++++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
> index 04ea209..c6ea207 100644
> --- a/arch/arm/boot/dts/rk3288.dtsi
> +++ b/arch/arm/boot/dts/rk3288.dtsi
> @@ -45,6 +45,7 @@
>  #include <dt-bindings/clock/rk3288-cru.h>
>  #include <dt-bindings/thermal/thermal.h>
>  #include <dt-bindings/power/rk3288-power.h>
> +#include <dt-bindings/soc/rockchip_boot-mode.h>
>  #include "skeleton.dtsi"
> 
>  / {
> @@ -170,6 +171,31 @@
>  		};
>  	};
> 
> +	reboot_mode {

nodes are commonly written with a dash "-" instead of an underscore "_".


> +		compatible = "rockchip,reboot-mode";
> +		rockchip,regmap = <&pmu>;

I do believe this should be a sub-node of the pmu, similar to the power-
domains. Due to the "simple-mfd" compatible of the pmu, it will get probed 
automatically and the driver can than get the regmap via

	parent = dev->parent;
	regmap = syscon_node_to_regmap(parent->of_node);

as the power-domains do and therefore also doesn't need the rockchip,regmap 
property.


> +		offset = <0x94>;
> +		loader {
> +			linux,mode = "loader";
> +			linux,magic = <BOOT_LOADER>;

linux,mode seems correct, but the magic value is actually not linux-specific 
but instead firmware-specific, so I'm not sure about that linux-prefix there

But I guess that would be for the dt-people :-)


> +		};
> +
> +		maskrom {
> +			linux,mode = "maskrom";
> +			linux,magic = <BOOT_MASKROM>;
> +		};

Just for my understanding, while I expect the bootloader to interpret the 
other values, does the maskrom read and handle this one?


> +
> +		recovery {
> +			linux,mode = "recovery";
> +			linux,magic = <BOOT_RECOVERY>;
> +		};
> +
> +		fastboot {
> +			linux,mode = "fastboot";
> +			linux,magic = <BOOT_FASTBOOT>;
> +		};
> +	};
> +
>  	reserved-memory {
>  		#address-cells = <1>;
>  		#size-cells = <1>;

I do believe rk3288-veyron.dtsi would need some sort of special handling.
I don't think coreboot interprets most of those handles, but I guess only the 
maskrom one would be handled by the maskrom?


Heiko

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 5/6] ARM: dts: rockchip: add reboot-mode node

[Andy Yan]

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

Hi Heiko:

2015-12-22 19:23 GMT+08:00 Heiko Stübner <heiko@sntech.de>:

> Hi Andy,
>
> Am Dienstag, 22. Dezember 2015, 17:16:52 schrieb Andy Yan:
> > Add reboot mode driver DT node for rk3xxx,rk3288 platform
>
> in general, I like that concept a lot :-) . Below some small issues.
>
>
> > Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
> >
> > ---
> >
> > Changes in v1:
> > - correct the maskrom magic number
> > - use macro defined in rockchip_boot-mode.h for reboot-mode DT node
> >
> >  arch/arm/boot/dts/rk3288.dtsi | 26 ++++++++++++++++++++++++++
> >  arch/arm/boot/dts/rk3xxx.dtsi | 26 ++++++++++++++++++++++++++
> >  2 files changed, 52 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/rk3288.dtsi
> b/arch/arm/boot/dts/rk3288.dtsi
> > index 04ea209..c6ea207 100644
> > --- a/arch/arm/boot/dts/rk3288.dtsi
> > +++ b/arch/arm/boot/dts/rk3288.dtsi
> > @@ -45,6 +45,7 @@
> >  #include <dt-bindings/clock/rk3288-cru.h>
> >  #include <dt-bindings/thermal/thermal.h>
> >  #include <dt-bindings/power/rk3288-power.h>
> > +#include <dt-bindings/soc/rockchip_boot-mode.h>
> >  #include "skeleton.dtsi"
> >
> >  / {
> > @@ -170,6 +171,31 @@
> >               };
> >       };
> >
> > +     reboot_mode {
>
> nodes are commonly written with a dash "-" instead of an underscore "_".
>

    okay, I will use dash "-" in next version.

>
>
> > +             compatible = "rockchip,reboot-mode";
> > +             rockchip,regmap = <&pmu>;
>
> I do believe this should be a sub-node of the pmu, similar to the power-
> domains. Due to the "simple-mfd" compatible of the pmu, it will get probed
> automatically and the driver can than get the regmap via
>
>         parent = dev->parent;
>         regmap = syscon_node_to_regmap(parent->of_node);
>
> as the power-domains do and therefore also doesn't need the rockchip,regmap
> property.
>
>
>     okay, got it.

> > +             offset = <0x94>;
> > +             loader {
> > +                     linux,mode = "loader";
> > +                     linux,magic = <BOOT_LOADER>;
>
> linux,mode seems correct, but the magic value is actually not
> linux-specific
> but instead firmware-specific, so I'm not sure about that linux-prefix
> there
>
> But I guess that would be for the dt-people :-)
>
>
>     Hope the dt-people will give some advices

> +             };
> > +
> > +             maskrom {
> > +                     linux,mode = "maskrom";
> > +                     linux,magic = <BOOT_MASKROM>;
> > +             };
>
> Just for my understanding, while I expect the bootloader to interpret the
> other values, does the maskrom read and handle this one?
>

 actually, this is also handled by the bootloader, this will be handled
before
 the DDR  initialization.

>
>
> > +
> > +             recovery {
> > +                     linux,mode = "recovery";
> > +                     linux,magic = <BOOT_RECOVERY>;
> > +             };
> > +
> > +             fastboot {
> > +                     linux,mode = "fastboot";
> > +                     linux,magic = <BOOT_FASTBOOT>;
> > +             };
> > +     };
> > +
> >       reserved-memory {
> >               #address-cells = <1>;
> >               #size-cells = <1>;
>
> I do believe rk3288-veyron.dtsi would need some sort of special handling.
> I don't think coreboot interprets most of those handles, but I guess only
> the
> maskrom one would be handled by the maskrom?
>

   yes, these depends on the bootloader.

>
>
> Heiko
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

[-- Attachment #2: Type: text/html, Size: 6441 bytes --]

[PATCH v1 6/6] ARM64: dts: rockchip: add reboot-mode node

[Andy Yan]

Add reboot mode driver DT node for rk3368 platform

Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
---

Changes in v1: None

 arch/arm64/boot/dts/rockchip/rk3368.dtsi | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3368.dtsi b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
index cc093a4..776f1be 100644
--- a/arch/arm64/boot/dts/rockchip/rk3368.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
@@ -45,6 +45,7 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/pinctrl/rockchip.h>
+#include <dt-bindings/soc/rockchip_boot-mode.h>
 
 / {
 	compatible = "rockchip,rk3368";
@@ -202,6 +203,31 @@
 		method = "smc";
 	};
 
+	reboot_mode {
+		compatible = "rockchip,reboot-mode";
+		rockchip,regmap = <&pmugrf>;
+		offset = <0x200>;
+		loader {
+			linux,mode = "loader";
+			linux,magic = <BOOT_LOADER>;
+		};
+
+		maskrom {
+			linux,mode = "maskrom";
+			linux,magic = <BOOT_MASKROM>;
+		};
+
+		recovery {
+			linux,mode = "recovery";
+			linux,magic = <BOOT_RECOVERY>;
+		};
+
+		fastboot {
+			linux,mode = "fastboot";
+			linux,magic = <BOOT_FASTBOOT>;
+		};
+	};
+
 	timer {
 		compatible = "arm,armv8-timer";
 		interrupts = <GIC_PPI 13
-- 
1.9.1

Re: [PATCH v1 0/6] misc: add reboot mode driver

[Alexandre Belloni]

Hi,

On 22/12/2015 at 17:02:29 +0800, Andy Yan wrote :
> 
> This driver parse the reboot commands like "reboot loader"
> and "reboot recovery" to get a boot mode described in the
> device tree , then call the vendor specific write interfae
> to store the boot mode in some place like special register
> or sram , which can be read by the bootloader after system
> reboot.
> 
> This is commonly done on Android based devices, in order to
> reboot the device into fastboot or recovery mode.
> 
> Before this patch , I have try some hack on[0], and then found
> John Stultz also doing the same work[1].
> 
> As John is busy these days, I go on with this work.
> 
> [0]https://patchwork.kernel.org/patch/7647751/
> [1]https://patchwork.kernel.org/patch/7802391/
> 
> Changes in v1:
> - fix the embarrassed compile warning
> - correct the maskrom magic number
> - check for the normal reboot
> - correct the maskrom magic number
> - use macro defined in rockchip_boot-mode.h for reboot-mode DT node
> 
> Andy Yan (6):
>   dt-bindings: misc: add document for reboot-mode driver
>   dt-bindings: soc: add document for rockchip reboot-mode driver
>   misc: add reboot mode driver
>   soc: rockchip: add reboot mode driver
>   ARM: dts: rockchip: add reboot-mode node
>   ARM64: dts: rockchip: add reboot-mode node
> 

This seems nice and useful, some further ideas:

>  .../devicetree/bindings/misc/reboot-mode.txt       | 41 +++++++++
>  .../bindings/soc/rockchip/rockchip,reboot-mode.txt | 39 +++++++++
>  arch/arm/boot/dts/rk3288.dtsi                      | 26 ++++++
>  arch/arm/boot/dts/rk3xxx.dtsi                      | 26 ++++++
>  arch/arm64/boot/dts/rockchip/rk3368.dtsi           | 26 ++++++
>  drivers/misc/Kconfig                               |  7 ++
>  drivers/misc/Makefile                              |  1 +
>  drivers/misc/reboot_mode.c                         | 96 ++++++++++++++++++++++

I think this actually belongs to drivers/power/reset/ instead of misc

>  drivers/soc/rockchip/Kconfig                       |  9 ++
>  drivers/soc/rockchip/Makefile                      |  1 +
>  drivers/soc/rockchip/reboot.c                      | 68 +++++++++++++++

And maybe that part could be made generic instead of rockchip specific.
It simply uses a regmap to do the accesses, I guess a lot of other
platforms will do that. We have syscon-reboot and syscon-poweroff for example.

I think then we can extend the "framework" by having generic drivers to
store the value in eeprom or nvram for example.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 0/6] misc: add reboot mode driver

[Andy Yan]

Hi Alexandre:

On 2015年12月23日 00:47, Alexandre Belloni wrote:
> Hi,
>
> On 22/12/2015 at 17:02:29 +0800, Andy Yan wrote :
>> This driver parse the reboot commands like "reboot loader"
>> and "reboot recovery" to get a boot mode described in the
>> device tree , then call the vendor specific write interfae
>> to store the boot mode in some place like special register
>> or sram , which can be read by the bootloader after system
>> reboot.
>>
>> This is commonly done on Android based devices, in order to
>> reboot the device into fastboot or recovery mode.
>>
>> Before this patch , I have try some hack on[0], and then found
>> John Stultz also doing the same work[1].
>>
>> As John is busy these days, I go on with this work.
>>
>> [0]https://patchwork.kernel.org/patch/7647751/
>> [1]https://patchwork.kernel.org/patch/7802391/
>>
>> Changes in v1:
>> - fix the embarrassed compile warning
>> - correct the maskrom magic number
>> - check for the normal reboot
>> - correct the maskrom magic number
>> - use macro defined in rockchip_boot-mode.h for reboot-mode DT node
>>
>> Andy Yan (6):
>>    dt-bindings: misc: add document for reboot-mode driver
>>    dt-bindings: soc: add document for rockchip reboot-mode driver
>>    misc: add reboot mode driver
>>    soc: rockchip: add reboot mode driver
>>    ARM: dts: rockchip: add reboot-mode node
>>    ARM64: dts: rockchip: add reboot-mode node
>>
> This seems nice and useful, some further ideas:
>
>>   .../devicetree/bindings/misc/reboot-mode.txt       | 41 +++++++++
>>   .../bindings/soc/rockchip/rockchip,reboot-mode.txt | 39 +++++++++
>>   arch/arm/boot/dts/rk3288.dtsi                      | 26 ++++++
>>   arch/arm/boot/dts/rk3xxx.dtsi                      | 26 ++++++
>>   arch/arm64/boot/dts/rockchip/rk3368.dtsi           | 26 ++++++
>>   drivers/misc/Kconfig                               |  7 ++
>>   drivers/misc/Makefile                              |  1 +
>>   drivers/misc/reboot_mode.c                         | 96 ++++++++++++++++++++++
> I think this actually belongs to drivers/power/reset/ instead of misc

    I also have the idea to put is in drivers/power/reset,  But considering
    this driver have not bind anything about power, so I put it in 
driver/misc
    at last. So I hope if some people can give more suggestions here.
>
>>   drivers/soc/rockchip/Kconfig                       |  9 ++
>>   drivers/soc/rockchip/Makefile                      |  1 +
>>   drivers/soc/rockchip/reboot.c                      | 68 +++++++++++++++
> And maybe that part could be made generic instead of rockchip specific.
> It simply uses a regmap to do the accesses, I guess a lot of other
> platforms will do that. We have syscon-reboot and syscon-poweroff for example.
>
> I think then we can extend the "framework" by having generic drivers to
> store the value in eeprom or nvram for example.
>

    I also hope the write interface can be generic. But I found some 
platform
    use different hardware to store the value. For example, John's patch 
use
    SRAM on qcom apq8064 to store value for nexus7. It seems there also have
    some platform use dram or nvram to store it. And these different 
hardware use
    different write method. I don't have a generic way to handle this.

    I have a idea to handle it like this:

+static const struct of_device_id reboot_mode_dt_match[] = {
+        { .compatible = "linux,reboot-mode-sfr",    /*for magic value 
stored in special function register, which  can be accessed by regmap*/
+                .data = (void *)&reboot-mode-sfr },
+        { .compatible = "linux,reboot-mode-sram",  /*for magic value 
stored in
+                .data = (void *)&reboot-mode-sram },
+        { .compatible = "linux,reboot-mode-sdram",
+                .data = (void *)&reboot-mode-sdram }, /*for magic value 
stored
+        { .compatible = "rockchip,reboot-mode-nvram",
+                .data = (void *)&reboot-mode-nvram },
+        {},
+};

    the data point to different hardware access method.

   Hope to see more suggestions from you.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 0/6] misc: add reboot mode driver

[Arnd Bergmann]

On Wednesday 23 December 2015 17:31:45 Andy Yan wrote:
> 
>     I also have the idea to put is in drivers/power/reset,  But considering
>     this driver have not bind anything about power, so I put it in 
> driver/misc
>     at last. So I hope if some people can give more suggestions here.

I vote for drivers/power/reset as well. On some platforms, the two things
are related, on others they are not, but it's good to have it all in one
place I think.

> >>   drivers/soc/rockchip/Kconfig                       |  9 ++
> >>   drivers/soc/rockchip/Makefile                      |  1 +
> >>   drivers/soc/rockchip/reboot.c                     | 68 +++++++++++++++
> > And maybe that part could be made generic instead of rockchip specific.
> > It simply uses a regmap to do the accesses, I guess a lot of other
> > platforms will do that. We have syscon-reboot and syscon-poweroff for example.
> >
> > I think then we can extend the "framework" by having generic drivers to
> > store the value in eeprom or nvram for example.
> >
> 
>     I also hope the write interface can be generic. But I found some 
> platform
>     use different hardware to store the value. For example, John's patch 
> use
>     SRAM on qcom apq8064 to store value for nexus7. It seems there also have
>     some platform use dram or nvram to store it. And these different 
> hardware use
>     different write method. I don't have a generic way to handle this.
> 
>     I have a idea to handle it like this:
> 
> +static const struct of_device_id reboot_mode_dt_match[] = {
> +        { .compatible = "linux,reboot-mode-sfr",    /*for magic value 
> stored in special function register, which  can be accessed by regmap*/
> +                .data = (void *)&reboot-mode-sfr },

I'd make this one syscon-reboot-mode, to go along with syscon-poweroff
as implemented by drivers/power/reset/syscon-poweroff.c. The syscon concept
is already generic enough that we don't need a linux prefix for that.

> +        { .compatible = "linux,reboot-mode-sram",  /*for magic value 
> stored in
> +                .data = (void *)&reboot-mode-sram },

the sram mode should probably follow the generic SRAM binding and make
the location that stores the reboot mode a separate partition, unless
we want to store other data in the same partition, in which case we might
want to use the same implementation as for the nvram.

> +        { .compatible = "linux,reboot-mode-sdram",
> +                .data = (void *)&reboot-mode-sdram }, /*for magic value 
> stored

I think "sdram" is not an appropriate name here, as the main system memory
might use some other technology, e.g. DRAM or DDR2-SDRAM.

> +        { .compatible = "rockchip,reboot-mode-nvram",
> +                .data = (void *)&reboot-mode-nvram },
> +        {},
> +};

nvram is a complex topic by itself, because there are so many ways to do it.
I think what you are referring to here is a battery-backed memory that uses
one or more bytes at a fixed offset to store a particular piece of information,
as the drivers/char/nvram.c driver does. Maybe we should put the reboot mode
into that driver then?

There are other nvram drivers at various places in the kernel, and each may
be slightly different, or completely different, like the EFIVARs driver on
UEFI firmware or the key/value store on Open Firmware, these probably need
their own methods and not share the generic driver.

>     the data point to different hardware access method.
> 
>    Hope to see more suggestions from you.

It's probably best to leave these four examples as separate drivers and we can
add further ones when needed.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 0/6] misc: add reboot mode driver

[Heiko Stübner]

Am Montag, 28. Dezember 2015, 16:28:55 schrieb Arnd Bergmann:
> On Wednesday 23 December 2015 17:31:45 Andy Yan wrote:
> > +        { .compatible = "rockchip,reboot-mode-nvram",
> > +                .data = (void *)&reboot-mode-nvram },
> > +        {},
> > +};
> 
> nvram is a complex topic by itself, because there are so many ways to do it.
> I think what you are referring to here is a battery-backed memory that uses
> one or more bytes at a fixed offset to store a particular piece of
> information, as the drivers/char/nvram.c driver does. Maybe we should put
> the reboot mode into that driver then?
> 
> There are other nvram drivers at various places in the kernel, and each may
> be slightly different, or completely different, like the EFIVARs driver on
> UEFI firmware or the key/value store on Open Firmware, these probably need
> their own methods and not share the generic driver.

actually we now have drivers/nvmem/* that does that byte-wise addressing for 
eeproms, efuses, etc in a generic way.

Re: [PATCH v1 0/6] misc: add reboot mode driver

[Arnd Bergmann]

On Monday 28 December 2015 16:56:12 Heiko Stübner wrote:
> Am Montag, 28. Dezember 2015, 16:28:55 schrieb Arnd Bergmann:
> > On Wednesday 23 December 2015 17:31:45 Andy Yan wrote:
> > > +        { .compatible = "rockchip,reboot-mode-nvram",
> > > +                .data = (void *)&reboot-mode-nvram },
> > > +        {},
> > > +};
> > 
> > nvram is a complex topic by itself, because there are so many ways to do it.
> > I think what you are referring to here is a battery-backed memory that uses
> > one or more bytes at a fixed offset to store a particular piece of
> > information, as the drivers/char/nvram.c driver does. Maybe we should put
> > the reboot mode into that driver then?
> > 
> > There are other nvram drivers at various places in the kernel, and each may
> > be slightly different, or completely different, like the EFIVARs driver on
> > UEFI firmware or the key/value store on Open Firmware, these probably need
> > their own methods and not share the generic driver.
> 
> actually we now have drivers/nvmem/* that does that byte-wise addressing for 
> eeproms, efuses, etc in a generic way.

Good point, so some of the reboot-reason users will be able to use that
framework, but some will not:

- drivers/nvmem only works for fixed-length data in a fixed location, but
  not for key/value pairs, TLVs etc.

- Coming back to an earlier problem I pointed out, a lot of the things
  stored in an nvram need a consistent user interface. This includes stuff
  like kernel command line, boot image name, boot device, console
  configuration, network mac address. We probably want the reboot reason
  to fit into the wider framework for these, and drivers/nvmem doesn't
  (currently) look like a good place for that.

	Arnd

Re: [PATCH v1 0/6] misc: add reboot mode driver

[Andy Yan]

Hi Arnd:

On 2015年12月28日 23:28, Arnd Bergmann wrote:
> On Wednesday 23 December 2015 17:31:45 Andy Yan wrote:
>>      I also have the idea to put is in drivers/power/reset,  But considering
>>      this driver have not bind anything about power, so I put it in
>> driver/misc
>>      at last. So I hope if some people can give more suggestions here.
> I vote for drivers/power/reset as well. On some platforms, the two things
> are related, on others they are not, but it's good to have it all in one
> place I think.
     Okay, I will move to drivers/power/reset next version.
>>>>    drivers/soc/rockchip/Kconfig                       |  9 ++
>>>>    drivers/soc/rockchip/Makefile                      |  1 +
>>>>    drivers/soc/rockchip/reboot.c                     | 68 +++++++++++++++
>>> And maybe that part could be made generic instead of rockchip specific.
>>> It simply uses a regmap to do the accesses, I guess a lot of other
>>> platforms will do that. We have syscon-reboot and syscon-poweroff for example.
>>>
>>> I think then we can extend the "framework" by having generic drivers to
>>> store the value in eeprom or nvram for example.
>>>
>>      I also hope the write interface can be generic. But I found some
>> platform
>>      use different hardware to store the value. For example, John's patch
>> use
>>      SRAM on qcom apq8064 to store value for nexus7. It seems there also have
>>      some platform use dram or nvram to store it. And these different
>> hardware use
>>      different write method. I don't have a generic way to handle this.
>>
>>      I have a idea to handle it like this:
>>
>> +static const struct of_device_id reboot_mode_dt_match[] = {
>> +        { .compatible = "linux,reboot-mode-sfr",    /*for magic value
>> stored in special function register, which  can be accessed by regmap*/
>> +                .data = (void *)&reboot-mode-sfr },
> I'd make this one syscon-reboot-mode, to go along with syscon-poweroff
> as implemented by drivers/power/reset/syscon-poweroff.c. The syscon concept
> is already generic enough that we don't need a linux prefix for that.
     Okay, syscon is better.
>> +        { .compatible = "linux,reboot-mode-sram",  /*for magic value
>> stored in
>> +                .data = (void *)&reboot-mode-sram },
> the sram mode should probably follow the generic SRAM binding and make
> the location that stores the reboot mode a separate partition, unless
> we want to store other data in the same partition, in which case we might
> want to use the same implementation as for the nvram.
>
>> +        { .compatible = "linux,reboot-mode-sdram",
>> +                .data = (void *)&reboot-mode-sdram }, /*for magic value
>> stored
> I think "sdram" is not an appropriate name here, as the main system memory
> might use some other technology, e.g. DRAM or DDR2-SDRAM.
>
>> +        { .compatible = "rockchip,reboot-mode-nvram",
>> +                .data = (void *)&reboot-mode-nvram },
>> +        {},
>> +};
> nvram is a complex topic by itself, because there are so many ways to do it.
> I think what you are referring to here is a battery-backed memory that uses
> one or more bytes at a fixed offset to store a particular piece of information,
> as the drivers/char/nvram.c driver does. Maybe we should put the reboot mode
> into that driver then?
>
> There are other nvram drivers at various places in the kernel, and each may
> be slightly different, or completely different, like the EFIVARs driver on
> UEFI firmware or the key/value store on Open Firmware, these probably need
> their own methods and not share the generic driver.
>
>>      the data point to different hardware access method.
>>
>>     Hope to see more suggestions from you.
> It's probably best to leave these four examples as separate drivers and we can
> add further ones when needed.
>
> 	Arnd
>
>
     Okay, thanks for your suggestion.  I will add the reboot-mode.c as 
the core library, syscon-reboot-mode.c as one example first. As for 
sram/dram/nvram case, I am not familiar with them, so hope there are 
some hero will extend them when needed.


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html