This driver parses the reboot commands like "reboot bootloader"
and "reboot recovery" to get a boot mode described in the
device tree , then call the corresponding write interfae
to store the boot mode in some place like special register
or ram, 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.
Changes in v7:
- fix some spelling mistakes
- declare that the mode magic should be none-zero value
- address some suggestions from Krzysztof, make syscon-reboot-mode driver data self-contained.
Changes in v6:
- fix a typo with "property"
- describe property "mask" more clear
Changes in v5:
- delete a unnecessary blank line in syscon-reboot-mode.txt
- rename mode-fastoboot to mode-bootloader in syscon-reboot-mode.txt
- rename macro BOOT_LOADER to BOOT_BL_DOWNLOAD, which gives a more clear mean
- use two blank space under help in Kconfig
- use unsigned int instead of int for member magic in struct mode_info
Changes in v4:
- remove mode-maskrom
- rename mode-fastboot to mode-bootloader to keep compatible with the exiting Android device
- make this driver depends on OF to avoid kbuild test error
Changes in v3:
- descirbe all reboot mode as properity instead of subnode
- scan multi properities
- add mask value for some platform which only use some bits of the register
to store boot mode magic value
- descirbe all reboot mode as property instead of subnode
- add rk3036 support
- descirbe all reboot mode as property instead of subnode
Changes in v2:
- move to dir drivers/power/reset/
- make syscon-reboot-mode a generic driver
- make this node as a subnode of PMU
- make this node as a subnode of pmugrf
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 (4):
dt-bindings: power: reset: add document for reboot-mode driver
power: reset: add reboot mode driver
ARM: dts: rockchip: add syscon-reboot-mode DT node
ARM64: dts: rockchip: add syscon-reboot-mode DT node
.../bindings/power/reset/reboot-mode.txt | 25 +++++
.../bindings/power/reset/syscon-reboot-mode.txt | 35 +++++++
arch/arm/boot/dts/rk3036.dtsi | 11 +-
arch/arm/boot/dts/rk3288.dtsi | 10 ++
arch/arm/boot/dts/rk3xxx.dtsi | 12 ++-
arch/arm64/boot/dts/rockchip/rk3368.dtsi | 12 ++-
drivers/power/reset/Kconfig | 13 +++
drivers/power/reset/Makefile | 2 +
drivers/power/reset/reboot-mode.c | 115 +++++++++++++++++++++
drivers/power/reset/reboot-mode.h | 6 ++
drivers/power/reset/syscon-reboot-mode.c | 86 +++++++++++++++
include/dt-bindings/soc/rockchip_boot-mode.h | 15 +++
12 files changed, 339 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power/reset/reboot-mode.txt
create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt
create mode 100644 drivers/power/reset/reboot-mode.c
create mode 100644 drivers/power/reset/reboot-mode.h
create mode 100644 drivers/power/reset/syscon-reboot-mode.c
create mode 100644 include/dt-bindings/soc/rockchip_boot-mode.h
--
1.9.1
Add device tree bindings document for reboot-mode driver.
Signed-off-by: Andy Yan <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
Changes in v7:
- fix some spelling mistakes
- declare that the mode magic should be none-zero value
Changes in v6:
- fix a typo with "property"
- describe property "mask" more clear
Changes in v5:
- delete a unnecessary blank line in syscon-reboot-mode.txt
- rename mode-fastoboot to mode-bootloader in syscon-reboot-mode.txt
- rename macro BOOT_LOADER to BOOT_BL_DOWNLOAD, which gives a more clear mean
Changes in v4:
- remove mode-maskrom
- rename mode-fastboot to mode-bootloader to keep compatible with the exiting Android device
Changes in v3:
- descirbe all reboot mode as properity instead of subnode
Changes in v2: None
Changes in v1: None
.../bindings/power/reset/reboot-mode.txt | 25 ++++++++++++++++
.../bindings/power/reset/syscon-reboot-mode.txt | 35 ++++++++++++++++++++++
2 files changed, 60 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/reset/reboot-mode.txt
create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt
diff --git a/Documentation/devicetree/bindings/power/reset/reboot-mode.txt b/Documentation/devicetree/bindings/power/reset/reboot-mode.txt
new file mode 100644
index 0000000..de34f27
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/reboot-mode.txt
@@ -0,0 +1,25 @@
+Generic reboot mode core map driver
+
+This driver get reboot mode arguments and call the write
+interface to store the magic value in special register
+or ram. Then the bootloader can read it and take different
+action according to the argument stored.
+
+All mode properties are vendor specific, it is a indication to tell
+the bootloader what to do when the system reboots, and should be named
+as mode-xxx = <magic> (xxx is mode name, magic should be a none-zero value).
+
+For example modes common on Android platform:
+- mode-normal: Normal reboot mode, system reboot with command "reboot".
+- mode-recovery: Android Recovery mode, it is a mode to format the device or update a new image.
+- mode-bootloader: Android fastboot mode, it's a mode to re-flash partitions on the Android based device.
+- mode-loader: A bootloader mode, it's a mode used to download image on Rockchip platform,
+ usually used in development.
+
+Example:
+ reboot-mode {
+ mode-normal = <BOOT_NORMAL>;
+ mode-recovery = <BOOT_RECOVERY>;
+ mode-bootloader = <BOOT_FASTBOOT>;
+ mode-loader = <BOOT_BL_DOWNLOAD>;
+ }
diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt
new file mode 100644
index 0000000..f7ce1d8
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt
@@ -0,0 +1,35 @@
+SYSCON reboot mode driver
+
+This driver gets reboot mode magic value form reboot-mode driver
+and stores it in a SYSCON mapped register. 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 "syscon", "simple-mfd"
+node.
+
+Required properties:
+- compatible: should be "syscon-reboot-mode"
+- offset: offset in the register map for the storage register (in bytes)
+
+Optional property:
+- mask: bits mask of the bits in the register to store the reboot mode magic value,
+ default set to 0xffffffff if missing.
+
+The rest of the properties should follow the generic reboot-mode description
+found in reboot-mode.txt
+
+Example:
+ pmu: pmu@20004000 {
+ compatible = "rockchip,rk3066-pmu", "syscon", "simple-mfd";
+ reg = <0x20004000 0x100>;
+
+ reboot-mode {
+ compatible = "syscon-reboot-mode";
+ offset = <0x40>;
+ mode-normal = <BOOT_NORMAL>;
+ mode-recovery = <BOOT_RECOVERY>;
+ mode-bootloader = <BOOT_FASTBOOT>;
+ mode-loader = <BOOT_BL_DOWNLOAD>;
+ };
+ };
--
1.9.1
This driver parses the reboot commands like "reboot bootloader"
and "reboot recovery" to get a boot mode described in the
device tree , then call the 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.
Reviewed-by: Matthias Brugger <[email protected]>
Reviewed-by: Moritz Fischer <[email protected]>
Tested-by: John Stultz <[email protected]>
Acked-by: John Stultz <[email protected]>
Signed-off-by: Andy Yan <[email protected]>
---
Changes in v7:
- address some suggestions from Krzysztof, make syscon-reboot-mode driver data self-contained.
Changes in v6: None
Changes in v5:
- use two blank space under help in Kconfig
- use unsigned int instead of int for member magic in struct mode_info
Changes in v4:
- make this driver depends on OF to avoid kbuild test error
Changes in v3:
- scan multi properities
- add mask value for some platform which only use some bits of the register
to store boot mode magic value
Changes in v2:
- move to dir drivers/power/reset/
- make syscon-reboot-mode a generic driver
Changes in v1:
- fix the embarrassed compile warning
- correct the maskrom magic number
- check for the normal reboot
drivers/power/reset/Kconfig | 13 ++++
drivers/power/reset/Makefile | 2 +
drivers/power/reset/reboot-mode.c | 115 +++++++++++++++++++++++++++++++
drivers/power/reset/reboot-mode.h | 6 ++
drivers/power/reset/syscon-reboot-mode.c | 86 +++++++++++++++++++++++
5 files changed, 222 insertions(+)
create mode 100644 drivers/power/reset/reboot-mode.c
create mode 100644 drivers/power/reset/reboot-mode.h
create mode 100644 drivers/power/reset/syscon-reboot-mode.c
diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index 1131cf7..cf50630 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -173,5 +173,18 @@ config POWER_RESET_ZX
help
Reboot support for ZTE SoCs.
+config REBOOT_MODE
+ tristate
+
+config SYSCON_REBOOT_MODE
+ bool "Generic SYSCON regmap reboot mode driver"
+ depends on OF
+ select REBOOT_MODE
+ help
+ Say y here will enable reboot mode driver. This will
+ get reboot mode arguments and store it in SYSCON mapped
+ register, 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 096fa67..a63865b 100644
--- a/drivers/power/reset/Makefile
+++ b/drivers/power/reset/Makefile
@@ -20,3 +20,5 @@ obj-$(CONFIG_POWER_RESET_SYSCON) += syscon-reboot.o
obj-$(CONFIG_POWER_RESET_SYSCON_POWEROFF) += syscon-poweroff.o
obj-$(CONFIG_POWER_RESET_RMOBILE) += rmobile-reset.o
obj-$(CONFIG_POWER_RESET_ZX) += zx-reboot.o
+obj-$(CONFIG_REBOOT_MODE) += reboot-mode.o
+obj-$(CONFIG_SYSCON_REBOOT_MODE) += syscon-reboot-mode.o
diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
new file mode 100644
index 0000000..ae6f931
--- /dev/null
+++ b/drivers/power/reset/reboot-mode.c
@@ -0,0 +1,115 @@
+/*
+ * 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/device.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/reboot.h>
+#include "reboot-mode.h"
+
+#define PREFIX "mode-"
+
+struct mode_info {
+ const char *mode;
+ unsigned int magic;
+ struct list_head list;
+};
+
+struct reboot_mode_driver {
+ struct device *dev;
+ struct list_head head;
+ int (*write)(struct device *dev, int magic);
+ struct notifier_block reboot_notifier;
+};
+
+static int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
+ const char *cmd)
+{
+ const char *normal = "normal";
+ int magic = 0;
+ struct mode_info *info;
+
+ if (!cmd)
+ cmd = normal;
+
+ 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);
+ if (magic)
+ reboot->write(reboot->dev, magic);
+
+ return NOTIFY_DONE;
+}
+
+int reboot_mode_register(struct device *dev, int (*write)(struct device *, int))
+{
+ struct reboot_mode_driver *reboot;
+ struct mode_info *info;
+ struct property *prop;
+ struct device_node *np;
+ size_t len = strlen(PREFIX);
+ int ret;
+
+ reboot = devm_kzalloc(dev, sizeof(*reboot), GFP_KERNEL);
+ if (!reboot)
+ return -ENOMEM;
+
+ reboot->dev = dev;
+ reboot->write = write;
+ INIT_LIST_HEAD(&reboot->head);
+
+ np = of_node_get(dev->of_node);
+ for_each_property_of_node(np, prop) {
+ if (len > strlen(prop->name) || strncmp(prop->name, PREFIX, len))
+ continue;
+
+ info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+
+ info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
+ if (of_property_read_u32(np, prop->name, &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);
+ }
+ of_node_put(np);
+
+ 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 <[email protected]");
+MODULE_DESCRIPTION("System reboot mode driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/power/reset/reboot-mode.h b/drivers/power/reset/reboot-mode.h
new file mode 100644
index 0000000..c861129
--- /dev/null
+++ b/drivers/power/reset/reboot-mode.h
@@ -0,0 +1,6 @@
+#ifndef __REBOOT_MODE_H__
+#define __REBOOT_MODE_H__
+
+int reboot_mode_register(struct device *dev, int (*write)(struct device *, int));
+
+#endif
diff --git a/drivers/power/reset/syscon-reboot-mode.c b/drivers/power/reset/syscon-reboot-mode.c
new file mode 100644
index 0000000..f1b8072
--- /dev/null
+++ b/drivers/power/reset/syscon-reboot-mode.c
@@ -0,0 +1,86 @@
+/*
+ * 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/of.h>
+#include <linux/platform_device.h>
+#include <linux/reboot.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+#include "reboot-mode.h"
+
+struct syscon_reboot_mode {
+ struct device *dev;
+ struct regmap *map;
+ u32 offset;
+ u32 mask;
+};
+
+static int syscon_reboot_mode_write(struct device *dev, int magic)
+{
+ struct syscon_reboot_mode *syscon_rbm = dev_get_drvdata(dev);
+ int ret;
+
+ ret = regmap_update_bits(syscon_rbm->map, syscon_rbm->offset,
+ syscon_rbm->mask, magic);
+ if (ret < 0)
+ dev_err(dev, "update reboot mode bits failed\n");
+
+ return ret;
+}
+
+static int syscon_reboot_mode_probe(struct platform_device *pdev)
+{
+ int ret;
+ struct syscon_reboot_mode *syscon_rbm;
+
+ syscon_rbm = devm_kzalloc(&pdev->dev, sizeof(*syscon_rbm), GFP_KERNEL);
+ if (!syscon_rbm)
+ return -ENOMEM;
+
+ syscon_rbm->dev = &pdev->dev;
+ syscon_rbm->mask = 0xffffffff;
+
+ syscon_rbm->map = syscon_node_to_regmap(pdev->dev.parent->of_node);
+ if (IS_ERR(syscon_rbm->map))
+ return PTR_ERR(syscon_rbm->map);
+
+ if (of_property_read_u32(pdev->dev.of_node, "offset", &syscon_rbm->offset))
+ return -EINVAL;
+
+ of_property_read_u32(pdev->dev.of_node, "mask", &syscon_rbm->mask);
+
+ dev_set_drvdata(&pdev->dev, syscon_rbm);
+
+ ret = reboot_mode_register(&pdev->dev, syscon_reboot_mode_write);
+ if (ret)
+ dev_err(&pdev->dev, "can't register reboot mode\n");
+
+ return ret;
+}
+
+static const struct of_device_id syscon_reboot_mode_of_match[] = {
+ { .compatible = "syscon-reboot-mode" },
+ {}
+};
+
+static struct platform_driver syscon_reboot_mode_driver = {
+ .probe = syscon_reboot_mode_probe,
+ .driver = {
+ .name = "syscon-reboot-mode",
+ .of_match_table = syscon_reboot_mode_of_match,
+ },
+};
+module_platform_driver(syscon_reboot_mode_driver);
+
+MODULE_AUTHOR("Andy Yan <[email protected]");
+MODULE_DESCRIPTION("SYSCON reboot mode driver");
+MODULE_LICENSE("GPL v2");
--
1.9.1
Rockchip platform use a SYSCON mapped register store
the reboot mode magic value for bootloader to use when
system reboot. So add syscon-reboot-mode driver DT node
for rk3xxx/rk3036/rk3288 based platform
Reviewed-by: Matthias Brugger <[email protected]>
Signed-off-by: Andy Yan <[email protected]>
---
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3:
- descirbe all reboot mode as property instead of subnode
- add rk3036 support
Changes in v2:
- make this node as a subnode of PMU
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/rk3036.dtsi | 11 ++++++++++-
arch/arm/boot/dts/rk3288.dtsi | 10 ++++++++++
arch/arm/boot/dts/rk3xxx.dtsi | 12 +++++++++++-
include/dt-bindings/soc/rockchip_boot-mode.h | 15 +++++++++++++++
4 files changed, 46 insertions(+), 2 deletions(-)
create mode 100644 include/dt-bindings/soc/rockchip_boot-mode.h
diff --git a/arch/arm/boot/dts/rk3036.dtsi b/arch/arm/boot/dts/rk3036.dtsi
index b9567c1..4011c2e 100644
--- a/arch/arm/boot/dts/rk3036.dtsi
+++ b/arch/arm/boot/dts/rk3036.dtsi
@@ -43,6 +43,7 @@
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/pinctrl/rockchip.h>
#include <dt-bindings/clock/rk3036-cru.h>
+#include <dt-bindings/soc/rockchip_boot-mode.h>
#include "skeleton.dtsi"
/ {
@@ -261,8 +262,16 @@
};
grf: syscon@20008000 {
- compatible = "rockchip,rk3036-grf", "syscon";
+ compatible = "rockchip,rk3036-grf", "syscon", "simple-mfd";
reg = <0x20008000 0x1000>;
+ reboot-mode {
+ compatible = "syscon-reboot-mode";
+ offset = <0x1d8>;
+ mode-normal = <BOOT_NORMAL>;
+ mode-recovery = <BOOT_RECOVERY>;
+ mode-bootloader = <BOOT_FASTBOOT>;
+ mode-loader = <BOOT_BL_DOWNLOAD>;
+ };
};
acodec: acodec-ana@20030000 {
diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 8ac49f3..9aa7d73 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"
/ {
@@ -713,6 +714,15 @@
clocks = <&cru ACLK_GPU>;
};
};
+
+ reboot-mode {
+ compatible = "syscon-reboot-mode";
+ offset = <0x94>;
+ mode-normal = <BOOT_NORMAL>;
+ mode-recovery = <BOOT_RECOVERY>;
+ mode-bootloader = <BOOT_FASTBOOT>;
+ mode-loader = <BOOT_BL_DOWNLOAD>;
+ };
};
sgrf: syscon@ff740000 {
diff --git a/arch/arm/boot/dts/rk3xxx.dtsi b/arch/arm/boot/dts/rk3xxx.dtsi
index 99eeea7..f8f661f 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"
/ {
@@ -243,8 +244,17 @@
};
pmu: pmu@20004000 {
- compatible = "rockchip,rk3066-pmu", "syscon";
+ compatible = "rockchip,rk3066-pmu", "syscon", "simple-mfd";
reg = <0x20004000 0x100>;
+
+ reboot-mode {
+ compatible = "syscon-reboot-mode";
+ offset = <0x40>;
+ mode-normal = <BOOT_NORMAL>;
+ mode-recovery = <BOOT_RECOVERY>;
+ mode-bootloader = <BOOT_FASTBOOT>;
+ mode-loader = <BOOT_BL_DOWNLOAD>;
+ };
};
grf: grf@20008000 {
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..ae7c867
--- /dev/null
+++ b/include/dt-bindings/soc/rockchip_boot-mode.h
@@ -0,0 +1,15 @@
+#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 bootloader rockusb mode */
+#define BOOT_BL_DOWNLOAD (REBOOT_FLAG + 1)
+/* enter recovery */
+#define BOOT_RECOVERY (REBOOT_FLAG + 3)
+ /* enter fastboot mode */
+#define BOOT_FASTBOOT (REBOOT_FLAG + 9)
+
+#endif
--
1.9.1
Add syscon-reboot-mode driver DT node for rk3368 platform
Tested-by: Caesar Wang <[email protected]>
Signed-off-by: Andy Yan <[email protected]>
---
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3:
- descirbe all reboot mode as property instead of subnode
Changes in v2:
- make this node as a subnode of pmugrf
Changes in v1: None
arch/arm64/boot/dts/rockchip/rk3368.dtsi | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3368.dtsi b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
index 122777b..df062ea 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>
#include <dt-bindings/thermal/thermal.h>
/ {
@@ -554,8 +555,17 @@
};
pmugrf: syscon@ff738000 {
- compatible = "rockchip,rk3368-pmugrf", "syscon";
+ compatible = "rockchip,rk3368-pmugrf", "syscon", "simple-mfd";
reg = <0x0 0xff738000 0x0 0x1000>;
+
+ reboot-mode {
+ compatible = "syscon-reboot-mode";
+ offset = <0x200>;
+ mode-normal = <BOOT_NORMAL>;
+ mode-recovery = <BOOT_RECOVERY>;
+ mode-bootloader = <BOOT_FASTBOOT>;
+ mode-loader = <BOOT_BL_DOWNLOAD>;
+ };
};
cru: clock-controller@ff760000 {
--
1.9.1
On 30.03.2016 11:20, Andy Yan wrote:
> This driver parses the reboot commands like "reboot bootloader"
> and "reboot recovery" to get a boot mode described in the
> device tree , then call the 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.
>
> Reviewed-by: Matthias Brugger <[email protected]>
> Reviewed-by: Moritz Fischer <[email protected]>
> Tested-by: John Stultz <[email protected]>
> Acked-by: John Stultz <[email protected]>
> Signed-off-by: Andy Yan <[email protected]>
>
> ---
>
> Changes in v7:
> - address some suggestions from Krzysztof, make syscon-reboot-mode driver data self-contained.
>
Thanks for changes, few comments below.
(...)
> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
> new file mode 100644
> index 0000000..ae6f931
> --- /dev/null
> +++ b/drivers/power/reset/reboot-mode.c
> @@ -0,0 +1,115 @@
> +/*
> + * 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/device.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/reboot.h>
> +#include "reboot-mode.h"
> +
> +#define PREFIX "mode-"
> +
> +struct mode_info {
> + const char *mode;
> + unsigned int magic;
> + struct list_head list;
> +};
> +
> +struct reboot_mode_driver {
> + struct device *dev;
> + struct list_head head;
> + int (*write)(struct device *dev, int magic);
> + struct notifier_block reboot_notifier;
> +};
> +
> +static int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
> + const char *cmd)
> +{
> + const char *normal = "normal";
> + int magic = 0;
> + struct mode_info *info;
> +
> + if (!cmd)
> + cmd = normal;
> +
> + 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);
> + if (magic)
> + reboot->write(reboot->dev, magic);
> +
> + return NOTIFY_DONE;
> +}
> +
> +int reboot_mode_register(struct device *dev, int (*write)(struct device *, int))
> +{
> + struct reboot_mode_driver *reboot;
> + struct mode_info *info;
> + struct property *prop;
> + struct device_node *np;
> + size_t len = strlen(PREFIX);
> + int ret;
> +
> + reboot = devm_kzalloc(dev, sizeof(*reboot), GFP_KERNEL);
> + if (!reboot)
> + return -ENOMEM;
> +
> + reboot->dev = dev;
> + reboot->write = write;
> + INIT_LIST_HEAD(&reboot->head);
> +
> + np = of_node_get(dev->of_node);
> + for_each_property_of_node(np, prop) {
> + if (len > strlen(prop->name) || strncmp(prop->name, PREFIX, len))
> + continue;
> +
> + info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
> + if (!info)
Clean up needed: of_node_put() and kfree_const() for items stored on the
list from previous iterations.
> + return -ENOMEM;
> +
> + info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
> + if (of_property_read_u32(np, prop->name, &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);
> + }
> + of_node_put(np);
If you of_node_put() here, there is no sense in getting it before. I
mentioned of_node_get() only because I am not sure about life-cycle of
nodes in case of DT overlays and you are storing the pointer to string
from DT.
The doubts I have are concerning only the case of freeing nodes from
overlay.
I don't know if of_node_get() is needed but of_node_get()+of_node_put()
seems useless.
> +
> + 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 <[email protected]");
> +MODULE_DESCRIPTION("System reboot mode driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/power/reset/reboot-mode.h b/drivers/power/reset/reboot-mode.h
> new file mode 100644
> index 0000000..c861129
> --- /dev/null
> +++ b/drivers/power/reset/reboot-mode.h
> @@ -0,0 +1,6 @@
> +#ifndef __REBOOT_MODE_H__
> +#define __REBOOT_MODE_H__
> +
> +int reboot_mode_register(struct device *dev, int (*write)(struct device *, int));
> +
> +#endif
> diff --git a/drivers/power/reset/syscon-reboot-mode.c b/drivers/power/reset/syscon-reboot-mode.c
> new file mode 100644
> index 0000000..f1b8072
> --- /dev/null
> +++ b/drivers/power/reset/syscon-reboot-mode.c
> @@ -0,0 +1,86 @@
> +/*
> + * 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/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reboot.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> +#include "reboot-mode.h"
> +
> +struct syscon_reboot_mode {
> + struct device *dev;
'dev' looks unused (only set).
> + struct regmap *map;
> + u32 offset;
> + u32 mask;
> +};
> +
> +static int syscon_reboot_mode_write(struct device *dev, int magic)
> +{
> + struct syscon_reboot_mode *syscon_rbm = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = regmap_update_bits(syscon_rbm->map, syscon_rbm->offset,
> + syscon_rbm->mask, magic);
> + if (ret < 0)
> + dev_err(dev, "update reboot mode bits failed\n");
> +
> + return ret;
> +}
> +
> +static int syscon_reboot_mode_probe(struct platform_device *pdev)
> +{
> + int ret;
> + struct syscon_reboot_mode *syscon_rbm;
> +
> + syscon_rbm = devm_kzalloc(&pdev->dev, sizeof(*syscon_rbm), GFP_KERNEL);
> + if (!syscon_rbm)
> + return -ENOMEM;
> +
> + syscon_rbm->dev = &pdev->dev;
> + syscon_rbm->mask = 0xffffffff;
> +
> + syscon_rbm->map = syscon_node_to_regmap(pdev->dev.parent->of_node);
> + if (IS_ERR(syscon_rbm->map))
> + return PTR_ERR(syscon_rbm->map);
> +
> + if (of_property_read_u32(pdev->dev.of_node, "offset", &syscon_rbm->offset))
> + return -EINVAL;
> +
> + of_property_read_u32(pdev->dev.of_node, "mask", &syscon_rbm->mask);
> +
> + dev_set_drvdata(&pdev->dev, syscon_rbm);
> +
> + ret = reboot_mode_register(&pdev->dev, syscon_reboot_mode_write);
> + if (ret)
> + dev_err(&pdev->dev, "can't register reboot mode\n");
> +
> + return ret;
> +}
> +
> +static const struct of_device_id syscon_reboot_mode_of_match[] = {
> + { .compatible = "syscon-reboot-mode" },
> + {}
> +};
> +
> +static struct platform_driver syscon_reboot_mode_driver = {
> + .probe = syscon_reboot_mode_probe,
Cleanup needed. What will happen after device unbind? Memory will be
released (devm-*()) but reboot notifier won't thus leading to OOPS on
reboot.
Best regards,
Krzysztof
On 30.03.2016 11:19, Andy Yan wrote:
> Add device tree bindings document for reboot-mode driver.
>
> Signed-off-by: Andy Yan <[email protected]>
> Acked-by: Rob Herring <[email protected]>
>
> ---
>
> Changes in v7:
> - fix some spelling mistakes
> - declare that the mode magic should be none-zero value
>
> Changes in v6:
> - fix a typo with "property"
> - describe property "mask" more clear
>
> Changes in v5:
> - delete a unnecessary blank line in syscon-reboot-mode.txt
> - rename mode-fastoboot to mode-bootloader in syscon-reboot-mode.txt
> - rename macro BOOT_LOADER to BOOT_BL_DOWNLOAD, which gives a more clear mean
>
> Changes in v4:
> - remove mode-maskrom
> - rename mode-fastboot to mode-bootloader to keep compatible with the exiting Android device
>
> Changes in v3:
> - descirbe all reboot mode as properity instead of subnode
>
> Changes in v2: None
> Changes in v1: None
>
> .../bindings/power/reset/reboot-mode.txt | 25 ++++++++++++++++
> .../bindings/power/reset/syscon-reboot-mode.txt | 35 ++++++++++++++++++++++
> 2 files changed, 60 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/reset/reboot-mode.txt
> create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
Hi Krzysztof:
Thanks for you comments.
On 2016年03月30日 11:36, Krzysztof Kozlowski wrote:
> On 30.03.2016 11:20, Andy Yan wrote:
>> This driver parses the reboot commands like "reboot bootloader"
>> and "reboot recovery" to get a boot mode described in the
>> device tree , then call the 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.
>>
>> Reviewed-by: Matthias Brugger <[email protected]>
>> Reviewed-by: Moritz Fischer <[email protected]>
>> Tested-by: John Stultz <[email protected]>
>> Acked-by: John Stultz <[email protected]>
>> Signed-off-by: Andy Yan <[email protected]>
>>
>> ---
>>
>> Changes in v7:
>> - address some suggestions from Krzysztof, make syscon-reboot-mode driver data self-contained.
>>
> Thanks for changes, few comments below.
>
> (...)
>
>> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
>> new file mode 100644
>> index 0000000..ae6f931
>> --- /dev/null
>> +++ b/drivers/power/reset/reboot-mode.c
>> @@ -0,0 +1,115 @@
>> +/*
>> + * 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/device.h>
>> +#include <linux/init.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/reboot.h>
>> +#include "reboot-mode.h"
>> +
>> +#define PREFIX "mode-"
>> +
>> +struct mode_info {
>> + const char *mode;
>> + unsigned int magic;
>> + struct list_head list;
>> +};
>> +
>> +struct reboot_mode_driver {
>> + struct device *dev;
>> + struct list_head head;
>> + int (*write)(struct device *dev, int magic);
>> + struct notifier_block reboot_notifier;
>> +};
>> +
>> +static int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
>> + const char *cmd)
>> +{
>> + const char *normal = "normal";
>> + int magic = 0;
>> + struct mode_info *info;
>> +
>> + if (!cmd)
>> + cmd = normal;
>> +
>> + 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);
>> + if (magic)
>> + reboot->write(reboot->dev, magic);
>> +
>> + return NOTIFY_DONE;
>> +}
>> +
>> +int reboot_mode_register(struct device *dev, int (*write)(struct device *, int))
>> +{
>> + struct reboot_mode_driver *reboot;
>> + struct mode_info *info;
>> + struct property *prop;
>> + struct device_node *np;
>> + size_t len = strlen(PREFIX);
>> + int ret;
>> +
>> + reboot = devm_kzalloc(dev, sizeof(*reboot), GFP_KERNEL);
>> + if (!reboot)
>> + return -ENOMEM;
>> +
>> + reboot->dev = dev;
>> + reboot->write = write;
>> + INIT_LIST_HEAD(&reboot->head);
>> +
>> + np = of_node_get(dev->of_node);
>> + for_each_property_of_node(np, prop) {
>> + if (len > strlen(prop->name) || strncmp(prop->name, PREFIX, len))
>> + continue;
>> +
>> + info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
>> + if (!info)
> Clean up needed: of_node_put() and kfree_const() for items stored on the
> list from previous iterations.
Okay, I will add it in V8.
>> + return -ENOMEM;
>> +
>> + info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
>> + if (of_property_read_u32(np, prop->name, &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);
>> + }
>> + of_node_put(np);
> If you of_node_put() here, there is no sense in getting it before. I
> mentioned of_node_get() only because I am not sure about life-cycle of
> nodes in case of DT overlays and you are storing the pointer to string
> from DT.
>
> The doubts I have are concerning only the case of freeing nodes from
> overlay.
>
> I don't know if of_node_get() is needed but of_node_get()+of_node_put()
> seems useless.
I am also not sure about it. Maybe just drop of_node_get/put ?
>> +
>> + 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 <[email protected]");
>> +MODULE_DESCRIPTION("System reboot mode driver");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/power/reset/reboot-mode.h b/drivers/power/reset/reboot-mode.h
>> new file mode 100644
>> index 0000000..c861129
>> --- /dev/null
>> +++ b/drivers/power/reset/reboot-mode.h
>> @@ -0,0 +1,6 @@
>> +#ifndef __REBOOT_MODE_H__
>> +#define __REBOOT_MODE_H__
>> +
>> +int reboot_mode_register(struct device *dev, int (*write)(struct device *, int));
>> +
>> +#endif
>> diff --git a/drivers/power/reset/syscon-reboot-mode.c b/drivers/power/reset/syscon-reboot-mode.c
>> new file mode 100644
>> index 0000000..f1b8072
>> --- /dev/null
>> +++ b/drivers/power/reset/syscon-reboot-mode.c
>> @@ -0,0 +1,86 @@
>> +/*
>> + * 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/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/reboot.h>
>> +#include <linux/regmap.h>
>> +#include <linux/mfd/syscon.h>
>> +#include "reboot-mode.h"
>> +
>> +struct syscon_reboot_mode {
>> + struct device *dev;
> 'dev' looks unused (only set).
This will be removed.
>
>> + struct regmap *map;
>> + u32 offset;
>> + u32 mask;
>> +};
>> +
>> +static int syscon_reboot_mode_write(struct device *dev, int magic)
>> +{
>> + struct syscon_reboot_mode *syscon_rbm = dev_get_drvdata(dev);
>> + int ret;
>> +
>> + ret = regmap_update_bits(syscon_rbm->map, syscon_rbm->offset,
>> + syscon_rbm->mask, magic);
>> + if (ret < 0)
>> + dev_err(dev, "update reboot mode bits failed\n");
>> +
>> + return ret;
>> +}
>> +
>> +static int syscon_reboot_mode_probe(struct platform_device *pdev)
>> +{
>> + int ret;
>> + struct syscon_reboot_mode *syscon_rbm;
>> +
>> + syscon_rbm = devm_kzalloc(&pdev->dev, sizeof(*syscon_rbm), GFP_KERNEL);
>> + if (!syscon_rbm)
>> + return -ENOMEM;
>> +
>> + syscon_rbm->dev = &pdev->dev;
>> + syscon_rbm->mask = 0xffffffff;
>> +
>> + syscon_rbm->map = syscon_node_to_regmap(pdev->dev.parent->of_node);
>> + if (IS_ERR(syscon_rbm->map))
>> + return PTR_ERR(syscon_rbm->map);
>> +
>> + if (of_property_read_u32(pdev->dev.of_node, "offset", &syscon_rbm->offset))
>> + return -EINVAL;
>> +
>> + of_property_read_u32(pdev->dev.of_node, "mask", &syscon_rbm->mask);
>> +
>> + dev_set_drvdata(&pdev->dev, syscon_rbm);
>> +
>> + ret = reboot_mode_register(&pdev->dev, syscon_reboot_mode_write);
>> + if (ret)
>> + dev_err(&pdev->dev, "can't register reboot mode\n");
>> +
>> + return ret;
>> +}
>> +
>> +static const struct of_device_id syscon_reboot_mode_of_match[] = {
>> + { .compatible = "syscon-reboot-mode" },
>> + {}
>> +};
>> +
>> +static struct platform_driver syscon_reboot_mode_driver = {
>> + .probe = syscon_reboot_mode_probe,
> Cleanup needed. What will happen after device unbind? Memory will be
> released (devm-*()) but reboot notifier won't thus leading to OOPS on
> reboot.
From the kernel_restart_prepare function, the reboot notifier will
be called before device_shutdown. Is there any other case the device
unbind before reboot notifier
called?
> Best regards,
> Krzysztof
>
> _______________________________________________
> Linux-rockchip mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
>
>
On 06.04.2016 09:50, Andy Yan wrote:
(...)
>>> + return -ENOMEM;
>>> +
>>> + info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
>>> + if (of_property_read_u32(np, prop->name, &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);
>>> + }
>>> + of_node_put(np);
>> If you of_node_put() here, there is no sense in getting it before. I
>> mentioned of_node_get() only because I am not sure about life-cycle of
>> nodes in case of DT overlays and you are storing the pointer to string
>> from DT.
>>
>> The doubts I have are concerning only the case of freeing nodes from
>> overlay.
>>
>> I don't know if of_node_get() is needed but of_node_get()+of_node_put()
>> seems useless.
>
>
> I am also not sure about it. Maybe just drop of_node_get/put ?
OK, let's drop both get() and put().
(...)
>>> +
>>> +static const struct of_device_id syscon_reboot_mode_of_match[] = {
>>> + { .compatible = "syscon-reboot-mode" },
>>> + {}
>>> +};
>>> +
>>> +static struct platform_driver syscon_reboot_mode_driver = {
>>> + .probe = syscon_reboot_mode_probe,
>> Cleanup needed. What will happen after device unbind? Memory will be
>> released (devm-*()) but reboot notifier won't thus leading to OOPS on
>> reboot.
>
> From the kernel_restart_prepare function, the reboot notifier will
> be called before device_shutdown. Is there any other case the device
> unbind before reboot notifier
> called?
This is a regular module platform driver so unbind can happen any time
initiated by user, either by unbind command or by module removal. User
can then re-bind device or not - probably does not matter. Anyway after
such first unbind, the restart will kaboom instead of do a restart.
Beside that, you always should clean up, regardless of restart or not.
If you do not want unbind (thus no need of cleanup) then forbid it by
making it a non-module with suppressed bind.
Best regards,
Krzysztof
Hi Krzysztof:
On 2016年04月06日 09:00, Krzysztof Kozlowski wrote:
> On 06.04.2016 09:50, Andy Yan wrote:
>
> (...)
>
>>>> + return -ENOMEM;
>>>> +
>>>> + info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
>>>> + if (of_property_read_u32(np, prop->name, &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);
>>>> + }
>>>> + of_node_put(np);
>>> If you of_node_put() here, there is no sense in getting it before. I
>>> mentioned of_node_get() only because I am not sure about life-cycle of
>>> nodes in case of DT overlays and you are storing the pointer to string
>>> from DT.
>>>
>>> The doubts I have are concerning only the case of freeing nodes from
>>> overlay.
>>>
>>> I don't know if of_node_get() is needed but of_node_get()+of_node_put()
>>> seems useless.
>>
>> I am also not sure about it. Maybe just drop of_node_get/put ?
> OK, let's drop both get() and put().
>
> (...)
>
>>>> +
>>>> +static const struct of_device_id syscon_reboot_mode_of_match[] = {
>>>> + { .compatible = "syscon-reboot-mode" },
>>>> + {}
>>>> +};
>>>> +
>>>> +static struct platform_driver syscon_reboot_mode_driver = {
>>>> + .probe = syscon_reboot_mode_probe,
>>> Cleanup needed. What will happen after device unbind? Memory will be
>>> released (devm-*()) but reboot notifier won't thus leading to OOPS on
>>> reboot.
>> From the kernel_restart_prepare function, the reboot notifier will
>> be called before device_shutdown. Is there any other case the device
>> unbind before reboot notifier
>> called?
> This is a regular module platform driver so unbind can happen any time
> initiated by user, either by unbind command or by module removal. User
> can then re-bind device or not - probably does not matter. Anyway after
> such first unbind, the restart will kaboom instead of do a restart.
I just need to do clean up in remove?
> Beside that, you always should clean up, regardless of restart or not.
> If you do not want unbind (thus no need of cleanup) then forbid it by
> making it a non-module with suppressed bind.
>
> Best regards,
> Krzysztof
>
>
>
>
On 04/12/2016 11:27 AM, Andy Yan wrote:
>>>>> +
>>>>> +static const struct of_device_id syscon_reboot_mode_of_match[] = {
>>>>> + { .compatible = "syscon-reboot-mode" },
>>>>> + {}
>>>>> +};
>>>>> +
>>>>> +static struct platform_driver syscon_reboot_mode_driver = {
>>>>> + .probe = syscon_reboot_mode_probe,
>>>> Cleanup needed. What will happen after device unbind? Memory will be
>>>> released (devm-*()) but reboot notifier won't thus leading to OOPS on
>>>> reboot.
>>> From the kernel_restart_prepare function, the reboot notifier will
>>> be called before device_shutdown. Is there any other case the device
>>> unbind before reboot notifier
>>> called?
>> This is a regular module platform driver so unbind can happen any time
>> initiated by user, either by unbind command or by module removal. User
>> can then re-bind device or not - probably does not matter. Anyway after
>> such first unbind, the restart will kaboom instead of do a restart.
>
> I just need to do clean up in remove?
Yes, you need to clean up everything in remove() callback (in a reversed
order).
Best regards,
Krzysztof
Hi Krzysztof:
Thanks for you kindly suggestion. I will follow it in next version.
On 2016年04月12日 19:18, Krzysztof Kozlowski wrote:
> On 04/12/2016 11:27 AM, Andy Yan wrote:
>>>>>> +
>>>>>> +static const struct of_device_id syscon_reboot_mode_of_match[] = {
>>>>>> + { .compatible = "syscon-reboot-mode" },
>>>>>> + {}
>>>>>> +};
>>>>>> +
>>>>>> +static struct platform_driver syscon_reboot_mode_driver = {
>>>>>> + .probe = syscon_reboot_mode_probe,
>>>>> Cleanup needed. What will happen after device unbind? Memory will be
>>>>> released (devm-*()) but reboot notifier won't thus leading to OOPS on
>>>>> reboot.
>>>> From the kernel_restart_prepare function, the reboot notifier will
>>>> be called before device_shutdown. Is there any other case the device
>>>> unbind before reboot notifier
>>>> called?
>>> This is a regular module platform driver so unbind can happen any time
>>> initiated by user, either by unbind command or by module removal. User
>>> can then re-bind device or not - probably does not matter. Anyway after
>>> such first unbind, the restart will kaboom instead of do a restart.
>> I just need to do clean up in remove?
> Yes, you need to clean up everything in remove() callback (in a reversed
> order).
>
> Best regards,
> Krzysztof
>
>
>