Thread (21 messages) 21 messages, 6 authors, 2016-02-02

Re: [PATCH v2 1/4] dt-bindings: power: reset: add document for reboot-mode driver

From: Rob Herring <robh@kernel.org>
Date: 2016-01-20 18:28:49
Also in: linux-arm-kernel, linux-devicetree, linux-pm, lkml

On Tue, Jan 12, 2016 at 07:29:49PM +0800, Andy Yan wrote:
quoted hunk ↗ jump to hunk
add device tree binding document for reboot-mode driver

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

---

Changes in v2: None
Changes in v1: None

 .../bindings/power/reset/reboot-mode.txt           | 41 +++++++++++++++++
 .../bindings/power/reset/syscon-reboot-mode.txt    | 52 ++++++++++++++++++++++
 2 files changed, 93 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..81d9f66
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/reboot-mode.txt
@@ -0,0 +1,41 @@
+Generic reboot mode core map driver
+
+This driver get reboot mode arguments and call the write
+interface to stores the magic value in special register
+or ram . Then the bootloader can read it and take different
+action according to the argument stored.
+
+Required properties:
+- compatible: only support "syscon-reboot-mode" now.
+
+Each mode is represented as a sub-node of reboot_mode:
+
+Subnode required properties:
+- linux,mode: reboot mode command,such as "loader", "recovery", "fastboot".
+- loader,magic: magic number for the mode, this is vendor specific.
+
+Example:
+	reboot_mode {
reboot-mode instead please.
+		compatible = "syscon-reboot-mode";
+		offset = <0x40>;
This doc by itself is a little confusing. For example, is a child of the 
syscon node? I would remove offset (and perhaps compatible) from this 
example.
+
+		loader {
+			linux,mode = "loader";
+			loader,magic = <BOOT_LOADER>;
+		};
Sorry, my previous suggestion was not clear. I'm suggesting get rid of 
the subnodes and just do properties like this:

loader = <BOOT_LOADER>;
maskrom = <BOOT_MASKROM>;

That's the same amount of information unless node names and linux,mode 
values are going to diverge. Do they need to? I can't see a reason.


We need to be clear what loader means. More specifically, it is boot 
into bootloader shell. 
+
+		maskrom {
In theory, the bootrom could have multiple modes. This typically means a 
USB download mode. So perhaps a more precise name would be 
"rom-download". 

In chips I'm familiar with the bootrom mode is selected via a different 
mechanism than the secondary bootloader modes, but I suppose the same 
mechanism could be used.
+			linux,mode = "maskrom";
+			loader,magic = <BOOT_MASKROM>;
+		};
+
+		recovery {
+			linux,mode = "recovery";
+			loader,magic = <BOOT_RECOVERY>;
+		};
+
+		fastboot {
+			linux,mode = "fastboot";
+			loader,magic = <BOOT_FASTBOOT>;
+		};
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help