mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 00/10] power: reset: add support for syscon reboot modes
@ 2020-09-16 13:50 Ahmad Fatoum
  2020-09-16 13:50 ` [PATCH 01/10] usbgadget: autostart: support delayed usbgadget.autostart=1 Ahmad Fatoum
                   ` (9 more replies)
  0 siblings, 10 replies; 15+ messages in thread
From: Ahmad Fatoum @ 2020-09-16 13:50 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

With this patch set, we have a framework to generically support passing
reboot modes to previous bootloader stages as well as process them.
Stuff you can do with this:

  linux$ reboot recovery
  * fall into barebox menu prompt, unless overridden *

  linux$ reboot emmc
  * sources /env/bmode/emmc if available *

  barebox$ gpr.reboot_mode.next=serial reset
  * fall into imx-usb-loader mode *

To enable this, besides having kernel and bootloader driver support,
you'll need to describe in the device tree where the reboot mode is
to be stored and what reboot modes there are.
This patch set implements it for two SoC families:

stm32mp15x:
  - All now support `reboot loader` and `reboot recovery` from Linux,
    if the syscon exists (The reboot mode itself is fixed up).

i.MX6Q/DL:
  - No OS communication by default, but you can now easily tell the
    BootROM where to boot from after warm reset

One thing missing in this series is support for nvmem-reboot-mode.
I ported the kernel driver[], but didn't come around to test it.

Details inside. Feedback appreciated. :-)

[1]: https://github.com/a3f/barebox/tree/reboot_mode

Ahmad Fatoum (10):
  usbgadget: autostart: support delayed usbgadget.autostart=1
  drivers: add reboot-mode infrastructure
  power: reset: reboot-mode: port syscon-reboot-mode support
  power: reset: reboot-mode: fix up node into boot device tree
  defaultenv: provide defaults for generic reboot modes
  ARM: dts: stm32mp: setup syscon-reboot-mode on TAMP general purpose
    register
  ARM: stm32mp: remove custom reboot mode logic from arch code
  power: reset: reboot-mode: support multi-word magic
  power: reset: syscon-reboot-mode: support multi-word reboot modes
  ARM: dts: i.MX6qdl: define BootROM reboot-mode on top of SRC_GPR{9,10}

 Documentation/user/defaultenv-2.rst           |  18 +-
 Documentation/user/reboot-mode.rst            |  95 ++++++++
 arch/arm/dts/imx6qdl.dtsi                     |  21 ++
 arch/arm/dts/stm32mp151.dtsi                  |  15 ++
 .../mach-stm32mp/include/mach/bootsource.h    |  12 -
 arch/arm/mach-stm32mp/init.c                  |  16 +-
 common/Kconfig                                |   5 +
 common/startup.c                              |  16 ++
 common/usbgadget.c                            |   6 +-
 defaultenv/Makefile                           |   1 +
 .../defaultenv-2-reboot-mode/bmode/bootloader |   3 +
 .../defaultenv-2-reboot-mode/bmode/loader     |   2 +
 .../defaultenv-2-reboot-mode/bmode/recovery   |   2 +
 defaultenv/defaultenv.c                       |   2 +
 drivers/Kconfig                               |   1 +
 drivers/Makefile                              |   1 +
 drivers/power/Kconfig                         |   2 +
 drivers/power/Makefile                        |   2 +
 drivers/power/reset/Kconfig                   |  16 ++
 drivers/power/reset/Makefile                  |   3 +
 drivers/power/reset/reboot-mode.c             | 229 ++++++++++++++++++
 drivers/power/reset/syscon-reboot-mode.c      | 129 ++++++++++
 include/linux/reboot-mode.h                   |  38 +++
 include/of.h                                  |   2 +
 24 files changed, 604 insertions(+), 33 deletions(-)
 create mode 100644 Documentation/user/reboot-mode.rst
 create mode 100644 defaultenv/defaultenv-2-reboot-mode/bmode/bootloader
 create mode 100755 defaultenv/defaultenv-2-reboot-mode/bmode/loader
 create mode 100644 defaultenv/defaultenv-2-reboot-mode/bmode/recovery
 create mode 100644 drivers/power/Kconfig
 create mode 100644 drivers/power/Makefile
 create mode 100644 drivers/power/reset/Kconfig
 create mode 100644 drivers/power/reset/Makefile
 create mode 100644 drivers/power/reset/reboot-mode.c
 create mode 100644 drivers/power/reset/syscon-reboot-mode.c
 create mode 100644 include/linux/reboot-mode.h

-- 
2.28.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 01/10] usbgadget: autostart: support delayed usbgadget.autostart=1
  2020-09-16 13:50 [PATCH 00/10] power: reset: add support for syscon reboot modes Ahmad Fatoum
@ 2020-09-16 13:50 ` Ahmad Fatoum
  2020-09-16 13:55   ` Ahmad Fatoum
  2020-09-16 13:50 ` [PATCH 02/10] drivers: add reboot-mode infrastructure Ahmad Fatoum
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 15+ messages in thread
From: Ahmad Fatoum @ 2020-09-16 13:50 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

So far, global.usbgadget.autostart=1 from the shell was without effect,
because the variable is only evaluated once at postenvironment_initcall.

Use the new globalvar_add_bool() to allow acting on the variable being
true at any time. This is necessary for scripts that want to enable
the usbgadget autostart functionality selectively without themselves
hardcoding the particularities of what is exported.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/usbgadget.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/common/usbgadget.c b/common/usbgadget.c
index b4f4ba04ca8c..be2bcc467d72 100644
--- a/common/usbgadget.c
+++ b/common/usbgadget.c
@@ -101,7 +101,7 @@ int usbgadget_register(bool dfu, const char *dfu_opts,
 	return ret;
 }
 
-static int usbgadget_autostart(void)
+static int usbgadget_autostart_set(struct param_d *param, void *ctx)
 {
 	bool fastboot_bbu = get_fastboot_bbu();
 
@@ -110,12 +110,12 @@ static int usbgadget_autostart(void)
 
 	return usbgadget_register(true, NULL, true, NULL, acm, fastboot_bbu);
 }
-postenvironment_initcall(usbgadget_autostart);
 
 static int usbgadget_globalvars_init(void)
 {
 	if (IS_ENABLED(CONFIG_USB_GADGET_AUTOSTART)) {
-		globalvar_add_simple_bool("usbgadget.autostart", &autostart);
+		globalvar_add_bool("usbgadget.autostart", usbgadget_autostart_set,
+				   &autostart, NULL);
 		globalvar_add_simple_bool("usbgadget.acm", &acm);
 	}
 	globalvar_add_simple_string("usbgadget.dfu_function", &dfu_function);
-- 
2.28.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 02/10] drivers: add reboot-mode infrastructure
  2020-09-16 13:50 [PATCH 00/10] power: reset: add support for syscon reboot modes Ahmad Fatoum
  2020-09-16 13:50 ` [PATCH 01/10] usbgadget: autostart: support delayed usbgadget.autostart=1 Ahmad Fatoum
@ 2020-09-16 13:50 ` Ahmad Fatoum
  2020-09-16 13:50 ` [PATCH 03/10] power: reset: reboot-mode: port syscon-reboot-mode support Ahmad Fatoum
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Ahmad Fatoum @ 2020-09-16 13:50 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Reboot modes provide a well-defined way to exchange information between
different stage of the boot process. When configured, users can type
`reboot bootloader` in the OS and barebox can read it out a device
parameter. Likewise barebox can write a reboot mode for the BootROM to
evaluate and then reset to fall into a serial recovery mode for example.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 Documentation/user/reboot-mode.rst |  84 +++++++++++++
 drivers/Kconfig                    |   1 +
 drivers/Makefile                   |   1 +
 drivers/power/Kconfig              |   2 +
 drivers/power/Makefile             |   2 +
 drivers/power/reset/Kconfig        |   5 +
 drivers/power/reset/Makefile       |   2 +
 drivers/power/reset/reboot-mode.c  | 185 +++++++++++++++++++++++++++++
 include/linux/reboot-mode.h        |  36 ++++++
 include/of.h                       |   2 +
 10 files changed, 320 insertions(+)
 create mode 100644 Documentation/user/reboot-mode.rst
 create mode 100644 drivers/power/Kconfig
 create mode 100644 drivers/power/Makefile
 create mode 100644 drivers/power/reset/Kconfig
 create mode 100644 drivers/power/reset/Makefile
 create mode 100644 drivers/power/reset/reboot-mode.c
 create mode 100644 include/linux/reboot-mode.h

diff --git a/Documentation/user/reboot-mode.rst b/Documentation/user/reboot-mode.rst
new file mode 100644
index 000000000000..1908da3ed2d7
--- /dev/null
+++ b/Documentation/user/reboot-mode.rst
@@ -0,0 +1,84 @@
+.. _reboot_mode:
+
+Reboot Mode
+-----------
+
+To simplify debugging, many BootROMs sample registers that survive
+a warm reset to customize the boot. These registers can e.g. indicate
+that boot should happen from a different boot medium.
+
+Likewise, many bootloaders reuse such registers, or if unavailable,
+non-volatile memory to determine whether the OS requested a special
+reboot mode, e.g. rebooting into an USB recovery mode. This is
+common on Android systems.
+
+barebox implements the upstream device tree bindings for
+`reboot-modes <https://www.kernel.org/doc/Documentation/devicetree/bindings/power/reset/reboot-mode.txt>`_
+to act upon reboot mode protocols specified in the device tree.
+
+The device tree nodes list a number of reboot modes along with a
+magic value for each. On reboot, an OS implementing the binding
+would take the reboot command's argument and match it against the
+modes in the device tree. If a match is found the associated magic
+is written to the location referenced in the device tree node.
+
+User API
+~~~~~~~~
+
+Devices registered with the reboot mode API gain two parameters:
+
+ - ``$dev_of_reboot_mode.prev`` (read-only): The reboot mode that was
+   set previous to barebox startup
+ - ``$dev_of_reboot_mode.next``: The next reboot mode, for when the
+   system is reset
+
+The reboot mode driver core use the alias name if available to name
+the device. By convention, this should end with ``.reboot_mode``, e.g.::
+
+	/ {
+		aliases {
+			gpr.reboot_name = &reboot_name_gpr;
+		};
+	};
+
+Reboot mode providers have priorities. The provider with the highest
+priority has its parameters aliased as ``$global.system.reboot_mode.prev``
+and ``$global.system.reboot_mode.next``.
+
+Disambiguation
+~~~~~~~~~~~~~~
+
+Some uses of reboot modes partially overlap with other barebox
+functionality. They all ultimately serve different purposes, however.
+
+Comparison to reset reason
+---------------------------
+
+The reset reason ``$global.system.reset`` is populated by different drivers
+to reflect the hardware cause of a reset, e.g. a watchdog. A reboot mode
+describes the OS intention behind a reset, e.g. to fall into a recovery
+mode. Reboot modes besides the default ``normal`` mode usually accompany
+a reset reason of ``RST`` (because the OS intentionally triggered a reset
+to activate the next reboot mode).
+
+Comparison to bootsource
+------------------------
+
+``$bootsource`` reflects the current boot's medium as indicated by the
+SoC. In cases where the reboot mode is used to communicate with the BootROM,
+``$bootsource`` and ``$bootsource_instance`` may describe the same device
+as the reboot mode.
+
+For cases, where the communication instead happens between barebox and an OS,
+they can be completely different, e.g. ``$bootsource`` may say barebox was
+booted from ``spi-nor``, while the reboot mode describes that barebox should
+boot the Kernel off an USB flash drive.
+
+Comparison to barebox state
+---------------------------
+
+barebox state also allows sharing information between barebox and the OS,
+but it does so while providing atomic updates, redundant storage and
+optionally wear leveling. In contrast to state, reboot mode is just that:
+a mode for a single reboot. barebox clears the reboot mode after reading it,
+so this can be reliably used across one reset only.
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 09595433a0e5..dda240578067 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -42,5 +42,6 @@ source "drivers/memory/Kconfig"
 source "drivers/soc/imx/Kconfig"
 source "drivers/nvme/Kconfig"
 source "drivers/ddr/Kconfig"
+source "drivers/power/Kconfig"
 
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 08a17ff459d3..5a03bdceab81 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -42,3 +42,4 @@ obj-y	+= memory/
 obj-y	+= soc/imx/
 obj-y	+= nvme/
 obj-y	+= ddr/
+obj-y	+= power/
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
new file mode 100644
index 000000000000..b56414c49750
--- /dev/null
+++ b/drivers/power/Kconfig
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+source "drivers/power/reset/Kconfig"
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
new file mode 100644
index 000000000000..3009da59bf46
--- /dev/null
+++ b/drivers/power/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+obj-y	+= reset/
diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
new file mode 100644
index 000000000000..5554fc122d92
--- /dev/null
+++ b/drivers/power/reset/Kconfig
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+
+config REBOOT_MODE
+	bool
diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
new file mode 100644
index 000000000000..68231f044a52
--- /dev/null
+++ b/drivers/power/reset/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+obj-$(CONFIG_REBOOT_MODE) += reboot-mode.o
diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
new file mode 100644
index 000000000000..5992a2acd99a
--- /dev/null
+++ b/drivers/power/reset/reboot-mode.c
@@ -0,0 +1,185 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd
+ * Copyright (c) 2019, Ahmad Fatoum, Pengutronix
+ */
+
+#include <common.h>
+#include <driver.h>
+#include <init.h>
+#include <of.h>
+#include <linux/reboot-mode.h>
+#include <globalvar.h>
+#include <magicvar.h>
+
+#define PREFIX "mode-"
+
+static int __priority;
+static struct reboot_mode_driver *__boot_mode;
+
+static int reboot_mode_param_set(struct param_d *p, void *priv)
+{
+	struct reboot_mode_driver *reboot = priv;
+	u32 magic;
+
+	magic = reboot->magics[reboot->reboot_mode_next];
+
+	return reboot->write(reboot, magic);
+}
+
+static int reboot_mode_add_param(struct device_d *dev,
+				 const char *prefix,
+				 struct reboot_mode_driver *reboot)
+{
+	char name[sizeof "system.reboot_mode.when"];
+	struct param_d *param;
+
+	scnprintf(name, sizeof(name), "%sprev", prefix);
+
+	param = dev_add_param_enum_ro(dev, name,
+				      &reboot->reboot_mode_prev, reboot->modes,
+				      reboot->nmodes);
+	if (IS_ERR(param))
+		return PTR_ERR(param);
+
+	scnprintf(name, sizeof(name), "%snext", prefix);
+
+	param = dev_add_param_enum(dev, name,
+				   reboot_mode_param_set, NULL,
+				   &reboot->reboot_mode_next, reboot->modes,
+				   reboot->nmodes, reboot);
+
+	return PTR_ERR_OR_ZERO(param);
+}
+
+static int reboot_mode_add_globalvar(void)
+{
+	struct reboot_mode_driver *reboot = __boot_mode;
+
+	if (!reboot)
+		return 0;
+
+	return reboot_mode_add_param(&global_device, "system.reboot_mode.", reboot);
+}
+late_initcall(reboot_mode_add_globalvar);
+
+
+static void reboot_mode_print(struct reboot_mode_driver *reboot,
+			      const char *prefix, u32 magic)
+{
+	dev_dbg(reboot->dev, "%s: %08x\n", prefix, magic);
+}
+
+/**
+ * reboot_mode_register - register a reboot mode driver
+ * @reboot: reboot mode driver
+ * @reboot_mode: reboot mode read from hardware
+ *
+ * Returns: 0 on success or a negative error code on failure.
+ */
+int reboot_mode_register(struct reboot_mode_driver *reboot, u32 reboot_mode)
+{
+	struct property *prop;
+	struct device_node *np = reboot->dev->device_node;
+	size_t len = strlen(PREFIX);
+	const char *alias;
+	size_t nmodes = 0;
+	int i = 0;
+	int ret;
+
+	for_each_property_of_node(np, prop) {
+		u32 magic;
+
+		if (strncmp(prop->name, PREFIX, len))
+			continue;
+		if (of_property_read_u32(np, prop->name, &magic))
+			continue;
+
+		nmodes++;
+	}
+
+	reboot->nmodes = nmodes;
+	reboot->magics = xzalloc(nmodes * sizeof(u32));
+	reboot->modes = xzalloc(nmodes * sizeof(const char *));
+
+	reboot_mode_print(reboot, "registering magic", reboot_mode);
+
+	for_each_property_of_node(np, prop) {
+		const char **mode;
+		u32 *magic;
+
+		magic = &reboot->magics[i];
+		mode = &reboot->modes[i];
+
+		if (strncmp(prop->name, PREFIX, len))
+			continue;
+
+		if (of_property_read_u32(np, prop->name, magic)) {
+			dev_err(reboot->dev, "reboot mode %s without magic number\n",
+				*mode);
+			continue;
+		}
+
+		*mode = prop->name + len;
+		if (*mode[0] == '\0') {
+			ret = -EINVAL;
+			dev_err(reboot->dev, "invalid mode name(%s): too short!\n",
+				prop->name);
+			goto error;
+		}
+
+		reboot_mode_print(reboot, *mode, *magic);
+
+		i++;
+	}
+
+	for (i = 0; i < reboot->nmodes; i++) {
+		if (reboot->magics[i] == reboot_mode) {
+			reboot->reboot_mode_prev = i;
+			break;
+		}
+	}
+
+	reboot_mode_add_param(reboot->dev, "", reboot);
+
+	/* clear mode for next reboot */
+	reboot->write(reboot, 0);
+
+	if (!reboot->priority)
+		reboot->priority = REBOOT_MODE_DEFAULT_PRIORITY;
+
+	if (reboot->priority >= __priority) {
+		__priority = reboot->priority;
+		__boot_mode = reboot;
+	}
+
+
+	alias = of_alias_get(np);
+	if (alias)
+		dev_set_name(reboot->dev, alias);
+
+	return 0;
+
+error:
+	free(reboot->magics);
+	free(reboot->modes);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(reboot_mode_register);
+
+const char *reboot_mode_get(void)
+{
+	if (!__boot_mode)
+		return NULL;
+
+	return __boot_mode->modes[__boot_mode->reboot_mode_prev];
+}
+EXPORT_SYMBOL_GPL(reboot_mode_get);
+
+BAREBOX_MAGICVAR_NAMED(global_system_reboot_mode_prev,
+		       global.system.reboot_mode.prev,
+		       "reboot-mode: Mode set previously, before barebox start");
+BAREBOX_MAGICVAR_NAMED(global_system_reboot_mode_next,
+		       global.system.reboot_mode.next,
+		       "reboot-mode: Mode to set next, to be evaluated after reset");
diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h
new file mode 100644
index 000000000000..92a1da7b5562
--- /dev/null
+++ b/include/linux/reboot-mode.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __REBOOT_MODE_H__
+#define __REBOOT_MODE_H__
+
+#include <linux/types.h>
+
+struct device_d;
+
+#ifdef CONFIG_REBOOT_MODE
+struct reboot_mode_driver {
+	struct device_d *dev;
+	int (*write)(struct reboot_mode_driver *reboot, u32 magic);
+	int priority;
+
+	/* filled by reboot_mode_register */
+	int reboot_mode_prev, reboot_mode_next;
+	unsigned nmodes;
+	const char **modes;
+	u32 *magics;
+};
+
+int reboot_mode_register(struct reboot_mode_driver *reboot, u32 reboot_mode);
+const char *reboot_mode_get(void);
+
+#define REBOOT_MODE_DEFAULT_PRIORITY 100
+
+#else
+
+static inline const char *reboot_mode_get(void)
+{
+	return NULL;
+}
+
+#endif
+
+#endif
diff --git a/include/of.h b/include/of.h
index d548e517896b..b9b3a102284c 100644
--- a/include/of.h
+++ b/include/of.h
@@ -732,6 +732,8 @@ static inline int of_autoenable_i2c_by_component(char *path)
 
 #endif
 
+#define for_each_property_of_node(dn, pp) \
+	list_for_each_entry(pp, &dn->properties, list)
 #define for_each_node_by_name(dn, name) \
 	for (dn = of_find_node_by_name(NULL, name); dn; \
 	     dn = of_find_node_by_name(dn, name))
-- 
2.28.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 03/10] power: reset: reboot-mode: port syscon-reboot-mode support
  2020-09-16 13:50 [PATCH 00/10] power: reset: add support for syscon reboot modes Ahmad Fatoum
  2020-09-16 13:50 ` [PATCH 01/10] usbgadget: autostart: support delayed usbgadget.autostart=1 Ahmad Fatoum
  2020-09-16 13:50 ` [PATCH 02/10] drivers: add reboot-mode infrastructure Ahmad Fatoum
@ 2020-09-16 13:50 ` Ahmad Fatoum
  2020-09-16 13:50 ` [PATCH 04/10] power: reset: reboot-mode: fix up node into boot device tree Ahmad Fatoum
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Ahmad Fatoum @ 2020-09-16 13:50 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Reboot modes are one-shot information that should be valid for only a
single reset. Syscons that survive a warm reset are one way to implement
this. This is used by many BootROMs to allow falling into a recovery
mode on the next boot. This ports the Linux syscon-reboot-mode support.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 Documentation/user/reboot-mode.rst       | 11 +++
 drivers/power/reset/Kconfig              | 11 +++
 drivers/power/reset/Makefile             |  1 +
 drivers/power/reset/syscon-reboot-mode.c | 85 ++++++++++++++++++++++++
 4 files changed, 108 insertions(+)
 create mode 100644 drivers/power/reset/syscon-reboot-mode.c

diff --git a/Documentation/user/reboot-mode.rst b/Documentation/user/reboot-mode.rst
index 1908da3ed2d7..9321d928f41a 100644
--- a/Documentation/user/reboot-mode.rst
+++ b/Documentation/user/reboot-mode.rst
@@ -45,6 +45,17 @@ Reboot mode providers have priorities. The provider with the highest
 priority has its parameters aliased as ``$global.system.reboot_mode.prev``
 and ``$global.system.reboot_mode.next``.
 
+Reset
+~~~~~
+
+Reboot modes can be stored on a syscon wrapping general purpose registers
+that survives warm resets. If the system instead did reset via an external
+power management IC, the registers may lose their value.
+
+If such reboot mode storage is used, users must take care to use the correct
+reset provider. In barebox, multiple reset providers may co-exist. They
+``reset`` command allows listing and choosing a specific reboot mode.
+
 Disambiguation
 ~~~~~~~~~~~~~~
 
diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index 5554fc122d92..f65e1f67fd59 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -3,3 +3,14 @@
 
 config REBOOT_MODE
 	bool
+
+config SYSCON_REBOOT_MODE
+	bool "Generic SYSCON regmap reboot mode driver"
+	depends on OFDEVICE
+	depends on MFD_SYSCON
+	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.
diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
index 68231f044a52..56feec78cf26 100644
--- a/drivers/power/reset/Makefile
+++ b/drivers/power/reset/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_REBOOT_MODE) += reboot-mode.o
+obj-$(CONFIG_SYSCON_REBOOT_MODE) += syscon-reboot-mode.o
diff --git a/drivers/power/reset/syscon-reboot-mode.c b/drivers/power/reset/syscon-reboot-mode.c
new file mode 100644
index 000000000000..5e4aed943d5c
--- /dev/null
+++ b/drivers/power/reset/syscon-reboot-mode.c
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd
+ */
+
+#include <common.h>
+#include <init.h>
+#include <driver.h>
+#include <of.h>
+#include <regmap.h>
+#include <mfd/syscon.h>
+#include <linux/reboot-mode.h>
+
+struct syscon_reboot_mode {
+	struct regmap *map;
+	struct reboot_mode_driver reboot;
+	u32 offset;
+	u32 mask;
+};
+
+static int syscon_reboot_mode_write(struct reboot_mode_driver *reboot,
+				    unsigned int magic)
+{
+	struct syscon_reboot_mode *syscon_rbm;
+	int ret;
+
+	syscon_rbm = container_of(reboot, struct syscon_reboot_mode, reboot);
+
+	ret = regmap_update_bits(syscon_rbm->map, syscon_rbm->offset,
+				 syscon_rbm->mask, magic);
+	if (ret < 0)
+		dev_err(reboot->dev, "update reboot mode bits failed\n");
+
+	return ret;
+}
+
+static int syscon_reboot_mode_probe(struct device_d *dev)
+{
+	int ret;
+	struct syscon_reboot_mode *syscon_rbm;
+	struct device_node *np = dev->device_node;
+	u32 magic;
+
+	syscon_rbm = xzalloc(sizeof(*syscon_rbm));
+
+	syscon_rbm->reboot.dev = dev;
+	syscon_rbm->reboot.write = syscon_reboot_mode_write;
+	syscon_rbm->mask = 0xffffffff;
+
+	syscon_rbm->map = syscon_node_to_regmap(dev->parent->device_node);
+	if (IS_ERR(syscon_rbm->map))
+		return PTR_ERR(syscon_rbm->map);
+
+	if (of_property_read_u32(np, "offset", &syscon_rbm->offset))
+		return -EINVAL;
+
+	of_property_read_u32(np, "mask", &syscon_rbm->mask);
+
+	ret = regmap_read(syscon_rbm->map, syscon_rbm->offset, &magic);
+	if (ret) {
+		dev_err(dev, "error reading reboot mode: %s\n",
+			strerror(-ret));
+		return ret;
+	}
+
+	magic &= syscon_rbm->mask;
+
+	ret = reboot_mode_register(&syscon_rbm->reboot, magic);
+	if (ret)
+		dev_err(dev, "can't register reboot mode\n");
+
+	return ret;
+}
+
+static const struct of_device_id syscon_reboot_mode_of_match[] = {
+	{ .compatible = "syscon-reboot-mode" },
+	{ /* sentinel */ }
+};
+
+static struct driver_d syscon_reboot_mode_driver = {
+	.probe = syscon_reboot_mode_probe,
+	.name = "syscon-reboot-mode",
+	.of_compatible = syscon_reboot_mode_of_match,
+};
+coredevice_platform_driver(syscon_reboot_mode_driver);
-- 
2.28.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 04/10] power: reset: reboot-mode: fix up node into boot device tree
  2020-09-16 13:50 [PATCH 00/10] power: reset: add support for syscon reboot modes Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2020-09-16 13:50 ` [PATCH 03/10] power: reset: reboot-mode: port syscon-reboot-mode support Ahmad Fatoum
@ 2020-09-16 13:50 ` Ahmad Fatoum
  2020-09-21  8:40   ` Sascha Hauer
  2020-09-16 13:50 ` [PATCH 05/10] defaultenv: provide defaults for generic reboot modes Ahmad Fatoum
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 15+ messages in thread
From: Ahmad Fatoum @ 2020-09-16 13:50 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Instead of relying that the kernel and barebox device trees are in sync,
just enforce it by having barebox fix up the device tree node it probed
into the kernel device tree. We usually want that, but some reboot mode
drivers might want to inhibit the fixup, e.g. because they implement
a non-upstream binding or because they communicate with the BootROM,
while the kernel shouldn't. For those the fixup is made optional via
a struct reboot_mode_driver::no_fixup member.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/power/reset/reboot-mode.c | 39 +++++++++++++++++++++++++++++++
 include/linux/reboot-mode.h       |  1 +
 2 files changed, 40 insertions(+)

diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
index 5992a2acd99a..5fc29ebf8091 100644
--- a/drivers/power/reset/reboot-mode.c
+++ b/drivers/power/reset/reboot-mode.c
@@ -52,6 +52,42 @@ static int reboot_mode_add_param(struct device_d *dev,
 	return PTR_ERR_OR_ZERO(param);
 }
 
+static struct device_node *of_get_node_by_reproducible_name(struct device_node *dstroot,
+							    struct device_node *srcnp)
+{
+	struct device_node *dstnp;
+	char *name;
+
+	name = of_get_reproducible_name(srcnp);
+	dstnp = of_find_node_by_reproducible_name(dstroot, name);
+	free(name);
+
+	return dstnp;
+}
+
+static int of_reboot_mode_fixup(struct device_node *root, void *ctx)
+{
+	struct reboot_mode_driver *reboot = ctx;
+	struct device_node *dstnp, *srcnp, *dstparent;
+
+	srcnp = reboot->dev->device_node;
+
+	dstnp = of_get_node_by_reproducible_name(root, srcnp);
+	if (dstnp) {
+		dstparent = dstnp->parent;
+		of_delete_node(dstnp);
+	} else {
+		dstparent = of_get_node_by_reproducible_name(root, srcnp->parent);
+	}
+
+	if (!dstparent)
+		return -EINVAL;
+
+	of_copy_node(dstparent, srcnp);
+
+	return 0;
+}
+
 static int reboot_mode_add_globalvar(void)
 {
 	struct reboot_mode_driver *reboot = __boot_mode;
@@ -59,6 +95,9 @@ static int reboot_mode_add_globalvar(void)
 	if (!reboot)
 		return 0;
 
+	if (!reboot->no_fixup)
+		of_register_fixup(of_reboot_mode_fixup, reboot);
+
 	return reboot_mode_add_param(&global_device, "system.reboot_mode.", reboot);
 }
 late_initcall(reboot_mode_add_globalvar);
diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h
index 92a1da7b5562..bc57f1d72df7 100644
--- a/include/linux/reboot-mode.h
+++ b/include/linux/reboot-mode.h
@@ -11,6 +11,7 @@ struct reboot_mode_driver {
 	struct device_d *dev;
 	int (*write)(struct reboot_mode_driver *reboot, u32 magic);
 	int priority;
+	bool no_fixup;
 
 	/* filled by reboot_mode_register */
 	int reboot_mode_prev, reboot_mode_next;
-- 
2.28.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 05/10] defaultenv: provide defaults for generic reboot modes
  2020-09-16 13:50 [PATCH 00/10] power: reset: add support for syscon reboot modes Ahmad Fatoum
                   ` (3 preceding siblings ...)
  2020-09-16 13:50 ` [PATCH 04/10] power: reset: reboot-mode: fix up node into boot device tree Ahmad Fatoum
@ 2020-09-16 13:50 ` Ahmad Fatoum
  2020-09-16 13:50 ` [PATCH 06/10] ARM: dts: stm32mp: setup syscon-reboot-mode on TAMP general purpose register Ahmad Fatoum
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Ahmad Fatoum @ 2020-09-16 13:50 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

While reboot mode magic identifiers can be very board specific, we can
settle on common names to allow some generic reboot mode handling:

  - loader     -> drop to bootloader shell on next boot
  - bootloader -> enable fastboot on next boot
  - recovery   -> display barebox boot menu

Boot modes loader and bootloader are admittedly a bit ambiguous, but
this nomenclature was chosen, because it's already in use on Android and
Rockchip systems.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 Documentation/user/defaultenv-2.rst            | 18 ++++++++++++++----
 common/Kconfig                                 |  5 +++++
 common/startup.c                               | 16 ++++++++++++++++
 defaultenv/Makefile                            |  1 +
 .../defaultenv-2-reboot-mode/bmode/bootloader  |  3 +++
 .../defaultenv-2-reboot-mode/bmode/loader      |  2 ++
 .../defaultenv-2-reboot-mode/bmode/recovery    |  2 ++
 defaultenv/defaultenv.c                        |  2 ++
 8 files changed, 45 insertions(+), 4 deletions(-)
 create mode 100644 defaultenv/defaultenv-2-reboot-mode/bmode/bootloader
 create mode 100755 defaultenv/defaultenv-2-reboot-mode/bmode/loader
 create mode 100644 defaultenv/defaultenv-2-reboot-mode/bmode/recovery

diff --git a/Documentation/user/defaultenv-2.rst b/Documentation/user/defaultenv-2.rst
index a79ae83d56c3..da766e4edcbc 100644
--- a/Documentation/user/defaultenv-2.rst
+++ b/Documentation/user/defaultenv-2.rst
@@ -19,10 +19,11 @@ All new boards should use defaultenv-2 exclusively.
 
 The default environment is composed from different directories during compilation::
 
-  defaultenv/defaultenv-2-base   -> base files
-  defaultenv/defaultenv-2-dfu    -> overlay for DFU
-  defaultenv/defaultenv-2-menu   -> overlay for menus
-  arch/$ARCH/boards/<board>/env  -> board specific overlay
+  defaultenv/defaultenv-2-base        -> base files
+  defaultenv/defaultenv-2-dfu         -> overlay for DFU
+  defaultenv/defaultenv-2-reboot-mode -> overlay for reboot modes
+  defaultenv/defaultenv-2-menu        -> overlay for menus
+  arch/$ARCH/boards/<board>/env       -> board specific overlay
 
 The content of the above directories is applied one after another. If the
 same file exists in a later overlay, it will overwrite the preceding one.
@@ -37,6 +38,7 @@ and their respective included directories in ``defaultenv/Makefile``:
   bbenv-$(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW) += defaultenv-2-base
   bbenv-$(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_MENU) += defaultenv-2-menu
   bbenv-$(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_DFU) += defaultenv-2-dfu
+  bbenv-$(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_REBOOT_MODE) += defaultenv-2-reboot-mode
   bbenv-$(CONFIG_DEFAULT_ENVIRONMENT_GENERIC) += defaultenv-1
 
 /env/bin/init
@@ -138,3 +140,11 @@ there will be a file ``eth0`` with a content like this:
   # put code to discover eth0 (i.e. 'usb') to /env/network/eth0-discover
 
   exit 0
+
+/env/bmode/
+-----------
+
+This contains the files to be sourced when barebox detects that the OS
+had requested a specific reboot mode (via e.g. ``reboot bootloader``
+under Linux). After the ``/env/init`` scripts were executed, barebox will
+``source /env/bmode/${global.system.reboot_mode.prev}`` if available.
diff --git a/common/Kconfig b/common/Kconfig
index 4ba58e55d436..a7815d2ab7c8 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -906,6 +906,11 @@ config DEFAULT_ENVIRONMENT_GENERIC_NEW_DFU
 	depends on USB_GADGET_DFU
 	default y
 
+config DEFAULT_ENVIRONMENT_GENERIC_NEW_REBOOT_MODE
+	bool "Generic reboot-mode handlers in the environment"
+	depends on DEFAULT_ENVIRONMENT_GENERIC_NEW
+	depends on REBOOT_MODE
+
 config DEFAULT_ENVIRONMENT_PATH
 	string
 	depends on DEFAULT_ENVIRONMENT
diff --git a/common/startup.c b/common/startup.c
index 075863d22e88..3a4ac81ffb4c 100644
--- a/common/startup.c
+++ b/common/startup.c
@@ -38,6 +38,7 @@
 #include <linux/stat.h>
 #include <envfs.h>
 #include <magicvar.h>
+#include <linux/reboot-mode.h>
 #include <asm/sections.h>
 #include <uncompress.h>
 #include <globalvar.h>
@@ -310,6 +311,7 @@ static int run_init(void)
 	DIR *dir;
 	struct dirent *d;
 	const char *initdir = "/env/init";
+	const char *bmode;
 	bool env_bin_init_exists;
 	enum autoboot_state autoboot;
 	struct stat s;
@@ -350,6 +352,20 @@ static int run_init(void)
 		closedir(dir);
 	}
 
+	/* source matching script in /env/bmode/ */
+	bmode = reboot_mode_get();
+	if (bmode) {
+		char *scr, *path;
+
+		scr = xasprintf("source /env/bmode/%s", bmode);
+		path = &scr[strlen("source ")];
+		if (stat(path, &s) == 0) {
+			pr_info("Invoking '%s'...\n", path);
+			run_command(scr);
+		}
+		free(scr);
+	}
+
 	autoboot = do_autoboot_countdown();
 
 	console_ctrlc_allow();
diff --git a/defaultenv/Makefile b/defaultenv/Makefile
index e030355a4052..91293567c0d3 100644
--- a/defaultenv/Makefile
+++ b/defaultenv/Makefile
@@ -1,6 +1,7 @@
 bbenv-$(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW) += defaultenv-2-base
 bbenv-$(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_MENU) += defaultenv-2-menu
 bbenv-$(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_DFU) += defaultenv-2-dfu
+bbenv-$(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_REBOOT_MODE) += defaultenv-2-reboot-mode
 bbenv-$(CONFIG_DEFAULT_ENVIRONMENT_GENERIC) += defaultenv-1
 obj-$(CONFIG_DEFAULT_ENVIRONMENT) += defaultenv.o
 extra-y += barebox_default_env barebox_default_env.h barebox_default_env$(DEFAULT_COMPRESSION_SUFFIX) barebox_zero_env
diff --git a/defaultenv/defaultenv-2-reboot-mode/bmode/bootloader b/defaultenv/defaultenv-2-reboot-mode/bmode/bootloader
new file mode 100644
index 000000000000..50a7a0f633ae
--- /dev/null
+++ b/defaultenv/defaultenv-2-reboot-mode/bmode/bootloader
@@ -0,0 +1,3 @@
+# Mode to re-flash partitions
+global.autoboot_timeout=30
+global.usbgadget.autostart=1
diff --git a/defaultenv/defaultenv-2-reboot-mode/bmode/loader b/defaultenv/defaultenv-2-reboot-mode/bmode/loader
new file mode 100755
index 000000000000..45647dec29a6
--- /dev/null
+++ b/defaultenv/defaultenv-2-reboot-mode/bmode/loader
@@ -0,0 +1,2 @@
+# Development mode
+global.autoboot=abort
diff --git a/defaultenv/defaultenv-2-reboot-mode/bmode/recovery b/defaultenv/defaultenv-2-reboot-mode/bmode/recovery
new file mode 100644
index 000000000000..0496ba3b0dad
--- /dev/null
+++ b/defaultenv/defaultenv-2-reboot-mode/bmode/recovery
@@ -0,0 +1,2 @@
+# Interactive mode for recovery
+global.autoboot=menu
diff --git a/defaultenv/defaultenv.c b/defaultenv/defaultenv.c
index b773030fe8ce..d69446c8937a 100644
--- a/defaultenv/defaultenv.c
+++ b/defaultenv/defaultenv.c
@@ -45,6 +45,8 @@ static void defaultenv_add_base(void)
 		defaultenv_append_directory(defaultenv_2_menu);
 	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_DFU))
 		defaultenv_append_directory(defaultenv_2_dfu);
+	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_REBOOT_MODE))
+		defaultenv_append_directory(defaultenv_2_reboot_mode);
 	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC))
 		defaultenv_append_directory(defaultenv_1);
 }
-- 
2.28.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 06/10] ARM: dts: stm32mp: setup syscon-reboot-mode on TAMP general purpose register
  2020-09-16 13:50 [PATCH 00/10] power: reset: add support for syscon reboot modes Ahmad Fatoum
                   ` (4 preceding siblings ...)
  2020-09-16 13:50 ` [PATCH 05/10] defaultenv: provide defaults for generic reboot modes Ahmad Fatoum
@ 2020-09-16 13:50 ` Ahmad Fatoum
  2020-09-16 13:50 ` [PATCH 07/10] ARM: stm32mp: remove custom reboot mode logic from arch code Ahmad Fatoum
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Ahmad Fatoum @ 2020-09-16 13:50 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

With the reboot mode infrastructure in place, lets add the first in-tree
user. The STM32MP1 SoCs have general purpose registers in the TAMP
peripheral. Register 20 there is used by the vendor's U-Boot for storing
a forced boot mode. We will use the same location for our reboot mode.
Consistency between barebox and OS is maintained by having barebox fixup
the device tree with the same reboot mode information it used itself, so
we are free to choose our own mode identifiers.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/dts/stm32mp151.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm/dts/stm32mp151.dtsi b/arch/arm/dts/stm32mp151.dtsi
index 5ff3b96fae4a..9638e972264a 100644
--- a/arch/arm/dts/stm32mp151.dtsi
+++ b/arch/arm/dts/stm32mp151.dtsi
@@ -28,6 +28,7 @@
 		pwm15 = &{/soc/timer@44006000/pwm};
 		pwm16 = &{/soc/timer@44007000/pwm};
 		pwm17 = &{/soc/timer@44008000/pwm};
+		tamp.reboot_mode = &reboot_mode_tamp;
 	};
 
 };
@@ -46,6 +47,20 @@
 		compatible = "st,stm32mp1-ddr";
 		reg = <0x5a003000 0x1000>;
 	};
+
+	tamp@5c00a000 {
+		compatible = "simple-bus", "syscon", "simple-mfd";
+		reg = <0x5c00a000 0x400>;
+
+		reboot_mode_tamp: reboot-mode {
+			compatible = "syscon-reboot-mode";
+			offset = <0x150>; /* reg20 */
+			mask = <0xff>;
+			mode-normal = <0>;
+			mode-loader = <0xBB>;
+			mode-recovery = <0xBC>;
+		};
+	};
 };
 
 &bsec {
-- 
2.28.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 07/10] ARM: stm32mp: remove custom reboot mode logic from arch code
  2020-09-16 13:50 [PATCH 00/10] power: reset: add support for syscon reboot modes Ahmad Fatoum
                   ` (5 preceding siblings ...)
  2020-09-16 13:50 ` [PATCH 06/10] ARM: dts: stm32mp: setup syscon-reboot-mode on TAMP general purpose register Ahmad Fatoum
@ 2020-09-16 13:50 ` Ahmad Fatoum
  2020-09-16 13:50 ` [PATCH 08/10] power: reset: reboot-mode: support multi-word magic Ahmad Fatoum
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Ahmad Fatoum @ 2020-09-16 13:50 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

This was ported from U-Boot to support st32mp_get_forced_boot_mode(),
which allows board code to customize boot according to values the kernel
places in the reboot mode region on the TAMP syscon.

We no have a syscon-reboot-mode driver, a device node in the
stm32mp151.dtsi, a ${global.system.reboot_mode} variable and a common
reboot_mode_get(), which together achieve the same, but in a generic
manner. Drop the now duplicate code. There has been no in-tree users so
far, so we don't need to touch anything else.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/mach-stm32mp/include/mach/bootsource.h | 12 ------------
 arch/arm/mach-stm32mp/init.c                    | 16 ++--------------
 2 files changed, 2 insertions(+), 26 deletions(-)

diff --git a/arch/arm/mach-stm32mp/include/mach/bootsource.h b/arch/arm/mach-stm32mp/include/mach/bootsource.h
index 1b6f562ac301..5750dc14482f 100644
--- a/arch/arm/mach-stm32mp/include/mach/bootsource.h
+++ b/arch/arm/mach-stm32mp/include/mach/bootsource.h
@@ -18,16 +18,4 @@ enum stm32mp_boot_device {
 	STM32MP_BOOT_SERIAL_USB_OTG	= 0x62,
 };
 
-enum stm32mp_forced_boot_mode {
-	STM32MP_BOOT_NORMAL	= 0x00,
-	STM32MP_BOOT_FASTBOOT	= 0x01,
-	STM32MP_BOOT_RECOVERY	= 0x02,
-	STM32MP_BOOT_STM32PROG	= 0x03,
-	STM32MP_BOOT_UMS_MMC0	= 0x10,
-	STM32MP_BOOT_UMS_MMC1	= 0x11,
-	STM32MP_BOOT_UMS_MMC2	= 0x12,
-};
-
-enum stm32mp_forced_boot_mode st32mp_get_forced_boot_mode(void);
-
 #endif
diff --git a/arch/arm/mach-stm32mp/init.c b/arch/arm/mach-stm32mp/init.c
index 7f687fa4f29a..7094e0e7dd08 100644
--- a/arch/arm/mach-stm32mp/init.c
+++ b/arch/arm/mach-stm32mp/init.c
@@ -80,12 +80,6 @@
 #define FIXUP_CPU_NUM(mask) ((mask) >> 16)
 #define FIXUP_CPU_HZ(mask) (((mask) & GENMASK(15, 0)) * 1000UL * 1000UL)
 
-static enum stm32mp_forced_boot_mode __stm32mp_forced_boot_mode;
-enum stm32mp_forced_boot_mode st32mp_get_forced_boot_mode(void)
-{
-	return __stm32mp_forced_boot_mode;
-}
-
 static void setup_boot_mode(void)
 {
 	u32 boot_ctx = readl(TAMP_BOOT_CONTEXT);
@@ -121,17 +115,11 @@ static void setup_boot_mode(void)
 		break;
 	}
 
-	__stm32mp_forced_boot_mode = boot_ctx & TAMP_BOOT_FORCED_MASK;
-
-	pr_debug("[boot_ctx=0x%x] => mode=0x%x, instance=%d forced=0x%x\n",
-		 boot_ctx, boot_mode, instance, __stm32mp_forced_boot_mode);
+	pr_debug("[boot_ctx=0x%x] => mode=0x%x, instance=%d\n",
+		 boot_ctx, boot_mode, instance);
 
 	bootsource_set(src);
 	bootsource_set_instance(instance);
-
-	/* clear TAMP for next reboot */
-	clrsetbits_le32(TAMP_BOOT_CONTEXT, TAMP_BOOT_FORCED_MASK,
-			STM32MP_BOOT_NORMAL);
 }
 
 static int __stm32mp_cputype;
-- 
2.28.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 08/10] power: reset: reboot-mode: support multi-word magic
  2020-09-16 13:50 [PATCH 00/10] power: reset: add support for syscon reboot modes Ahmad Fatoum
                   ` (6 preceding siblings ...)
  2020-09-16 13:50 ` [PATCH 07/10] ARM: stm32mp: remove custom reboot mode logic from arch code Ahmad Fatoum
@ 2020-09-16 13:50 ` Ahmad Fatoum
  2020-09-16 13:50 ` [PATCH 09/10] power: reset: syscon-reboot-mode: support multi-word reboot modes Ahmad Fatoum
  2020-09-16 13:50 ` [PATCH 10/10] ARM: dts: i.MX6qdl: define BootROM reboot-mode on top of SRC_GPR{9, 10} Ahmad Fatoum
  9 siblings, 0 replies; 15+ messages in thread
From: Ahmad Fatoum @ 2020-09-16 13:50 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

The upstream binding and driver implementation only supports
reboot modes of 32-bit length. This is insufficient for cases where
multiple registers need to be written for the reboot mode to become
active. The i.MX6 is an example for this, the BootROM expects a second
32-bit register to indicate whether the reboot mode in the first is
valid. In preparation for adding support for this to the
syscon-reboot-mode driver. Migrate the reboot-mode core to support this.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/power/reset/reboot-mode.c        | 37 ++++++++++++++----------
 drivers/power/reset/syscon-reboot-mode.c | 11 +++++--
 include/linux/reboot-mode.h              |  7 +++--
 3 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
index 5fc29ebf8091..5a72f6dca22d 100644
--- a/drivers/power/reset/reboot-mode.c
+++ b/drivers/power/reset/reboot-mode.c
@@ -20,11 +20,9 @@ static struct reboot_mode_driver *__boot_mode;
 static int reboot_mode_param_set(struct param_d *p, void *priv)
 {
 	struct reboot_mode_driver *reboot = priv;
-	u32 magic;
+	size_t i = reboot->reboot_mode_next * reboot->nelems;
 
-	magic = reboot->magics[reboot->reboot_mode_next];
-
-	return reboot->write(reboot, magic);
+	return reboot->write(reboot, &reboot->magics[i]);
 }
 
 static int reboot_mode_add_param(struct device_d *dev,
@@ -104,9 +102,13 @@ late_initcall(reboot_mode_add_globalvar);
 
 
 static void reboot_mode_print(struct reboot_mode_driver *reboot,
-			      const char *prefix, u32 magic)
+			      const char *prefix, const u32 *arr)
 {
-	dev_dbg(reboot->dev, "%s: %08x\n", prefix, magic);
+	size_t i;
+	dev_dbg(reboot->dev, "%s: ", prefix);
+	for (i = 0; i < reboot->nelems; i++)
+		__pr_printk(7, "%08x ", arr[i]);
+	__pr_printk(7, "\n");
 }
 
 /**
@@ -116,7 +118,8 @@ static void reboot_mode_print(struct reboot_mode_driver *reboot,
  *
  * Returns: 0 on success or a negative error code on failure.
  */
-int reboot_mode_register(struct reboot_mode_driver *reboot, u32 reboot_mode)
+int reboot_mode_register(struct reboot_mode_driver *reboot,
+			 const u32 *reboot_mode, size_t nelems)
 {
 	struct property *prop;
 	struct device_node *np = reboot->dev->device_node;
@@ -138,7 +141,8 @@ int reboot_mode_register(struct reboot_mode_driver *reboot, u32 reboot_mode)
 	}
 
 	reboot->nmodes = nmodes;
-	reboot->magics = xzalloc(nmodes * sizeof(u32));
+	reboot->nelems = nelems;
+	reboot->magics = xzalloc(nmodes * nelems * sizeof(u32));
 	reboot->modes = xzalloc(nmodes * sizeof(const char *));
 
 	reboot_mode_print(reboot, "registering magic", reboot_mode);
@@ -147,13 +151,13 @@ int reboot_mode_register(struct reboot_mode_driver *reboot, u32 reboot_mode)
 		const char **mode;
 		u32 *magic;
 
-		magic = &reboot->magics[i];
+		magic = &reboot->magics[i * nelems];
 		mode = &reboot->modes[i];
 
 		if (strncmp(prop->name, PREFIX, len))
 			continue;
 
-		if (of_property_read_u32(np, prop->name, magic)) {
+		if (of_property_read_u32_array(np, prop->name, magic, nelems)) {
 			dev_err(reboot->dev, "reboot mode %s without magic number\n",
 				*mode);
 			continue;
@@ -167,22 +171,23 @@ int reboot_mode_register(struct reboot_mode_driver *reboot, u32 reboot_mode)
 			goto error;
 		}
 
-		reboot_mode_print(reboot, *mode, *magic);
+		reboot_mode_print(reboot, *mode, magic);
 
 		i++;
 	}
 
 	for (i = 0; i < reboot->nmodes; i++) {
-		if (reboot->magics[i] == reboot_mode) {
-			reboot->reboot_mode_prev = i;
-			break;
-		}
+		if (memcmp(&reboot->magics[i * nelems], reboot_mode, nelems * sizeof(u32)))
+			continue;
+
+		reboot->reboot_mode_prev = i;
+		break;
 	}
 
 	reboot_mode_add_param(reboot->dev, "", reboot);
 
 	/* clear mode for next reboot */
-	reboot->write(reboot, 0);
+	reboot->write(reboot, &(u32) { 0 });
 
 	if (!reboot->priority)
 		reboot->priority = REBOOT_MODE_DEFAULT_PRIORITY;
diff --git a/drivers/power/reset/syscon-reboot-mode.c b/drivers/power/reset/syscon-reboot-mode.c
index 5e4aed943d5c..bd4f1a3d04b0 100644
--- a/drivers/power/reset/syscon-reboot-mode.c
+++ b/drivers/power/reset/syscon-reboot-mode.c
@@ -19,7 +19,7 @@ struct syscon_reboot_mode {
 };
 
 static int syscon_reboot_mode_write(struct reboot_mode_driver *reboot,
-				    unsigned int magic)
+				    const u32 *magic)
 {
 	struct syscon_reboot_mode *syscon_rbm;
 	int ret;
@@ -27,7 +27,7 @@ static int syscon_reboot_mode_write(struct reboot_mode_driver *reboot,
 	syscon_rbm = container_of(reboot, struct syscon_reboot_mode, reboot);
 
 	ret = regmap_update_bits(syscon_rbm->map, syscon_rbm->offset,
-				 syscon_rbm->mask, magic);
+				 syscon_rbm->mask, *magic);
 	if (ret < 0)
 		dev_err(reboot->dev, "update reboot mode bits failed\n");
 
@@ -39,8 +39,13 @@ static int syscon_reboot_mode_probe(struct device_d *dev)
 	int ret;
 	struct syscon_reboot_mode *syscon_rbm;
 	struct device_node *np = dev->device_node;
+	size_t nelems;
 	u32 magic;
 
+	nelems = of_property_count_elems_of_size(np, "offset", sizeof(__be32));
+	if (nelems != 1)
+		return -EINVAL;
+
 	syscon_rbm = xzalloc(sizeof(*syscon_rbm));
 
 	syscon_rbm->reboot.dev = dev;
@@ -65,7 +70,7 @@ static int syscon_reboot_mode_probe(struct device_d *dev)
 
 	magic &= syscon_rbm->mask;
 
-	ret = reboot_mode_register(&syscon_rbm->reboot, magic);
+	ret = reboot_mode_register(&syscon_rbm->reboot, &magic, 1);
 	if (ret)
 		dev_err(dev, "can't register reboot mode\n");
 
diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h
index bc57f1d72df7..9d9ce19c0e5f 100644
--- a/include/linux/reboot-mode.h
+++ b/include/linux/reboot-mode.h
@@ -9,18 +9,19 @@ struct device_d;
 #ifdef CONFIG_REBOOT_MODE
 struct reboot_mode_driver {
 	struct device_d *dev;
-	int (*write)(struct reboot_mode_driver *reboot, u32 magic);
+	int (*write)(struct reboot_mode_driver *reboot, const u32 *magic);
 	int priority;
 	bool no_fixup;
 
 	/* filled by reboot_mode_register */
 	int reboot_mode_prev, reboot_mode_next;
-	unsigned nmodes;
+	unsigned nmodes, nelems;
 	const char **modes;
 	u32 *magics;
 };
 
-int reboot_mode_register(struct reboot_mode_driver *reboot, u32 reboot_mode);
+int reboot_mode_register(struct reboot_mode_driver *reboot,
+			 const u32 *magic, size_t num);
 const char *reboot_mode_get(void);
 
 #define REBOOT_MODE_DEFAULT_PRIORITY 100
-- 
2.28.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 09/10] power: reset: syscon-reboot-mode: support multi-word reboot modes
  2020-09-16 13:50 [PATCH 00/10] power: reset: add support for syscon reboot modes Ahmad Fatoum
                   ` (7 preceding siblings ...)
  2020-09-16 13:50 ` [PATCH 08/10] power: reset: reboot-mode: support multi-word magic Ahmad Fatoum
@ 2020-09-16 13:50 ` Ahmad Fatoum
  2020-10-07  8:18   ` Sascha Hauer
  2020-09-16 13:50 ` [PATCH 10/10] ARM: dts: i.MX6qdl: define BootROM reboot-mode on top of SRC_GPR{9, 10} Ahmad Fatoum
  9 siblings, 1 reply; 15+ messages in thread
From: Ahmad Fatoum @ 2020-09-16 13:50 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

SoCs like the i.MX6 have their BootROM consult two distinct 32-bit
registers to determine the reboot mode. Extend the driver to support
this. While backwards compatible, this is not so far supported by the
upstream binding, which is why a new barebox,syscon-reboot-mode is
introduced. This new compatible be inhibit used to inhibit fixup
into a kernel device tree.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/power/reset/syscon-reboot-mode.c | 91 +++++++++++++++++-------
 1 file changed, 65 insertions(+), 26 deletions(-)

diff --git a/drivers/power/reset/syscon-reboot-mode.c b/drivers/power/reset/syscon-reboot-mode.c
index bd4f1a3d04b0..0cbc9d0803ff 100644
--- a/drivers/power/reset/syscon-reboot-mode.c
+++ b/drivers/power/reset/syscon-reboot-mode.c
@@ -10,75 +10,114 @@
 #include <regmap.h>
 #include <mfd/syscon.h>
 #include <linux/reboot-mode.h>
+#include <linux/overflow.h>
+
+struct mode_reg {
+	u32 offset;
+	u32 mask;
+};
 
 struct syscon_reboot_mode {
 	struct regmap *map;
 	struct reboot_mode_driver reboot;
-	u32 offset;
-	u32 mask;
+	struct mode_reg reg[];
 };
 
 static int syscon_reboot_mode_write(struct reboot_mode_driver *reboot,
 				    const u32 *magic)
 {
 	struct syscon_reboot_mode *syscon_rbm;
-	int ret;
+	size_t i;
+	int ret = 0;
 
 	syscon_rbm = container_of(reboot, struct syscon_reboot_mode, reboot);
 
-	ret = regmap_update_bits(syscon_rbm->map, syscon_rbm->offset,
-				 syscon_rbm->mask, *magic);
-	if (ret < 0)
-		dev_err(reboot->dev, "update reboot mode bits failed\n");
+	for (i = 0; i < reboot->nelems; i++) {
+		struct mode_reg *reg = &syscon_rbm->reg[i];
+
+		ret = regmap_update_bits(syscon_rbm->map, reg->offset,
+					 reg->mask, *magic++);
+		if (ret < 0) {
+			dev_err(reboot->dev, "update reboot mode bits failed\n");
+			break;
+		}
+	}
 
 	return ret;
 }
 
 static int syscon_reboot_mode_probe(struct device_d *dev)
 {
-	int ret;
+	int ret, i, nelems;
 	struct syscon_reboot_mode *syscon_rbm;
+	struct reboot_mode_driver *reboot_template;
 	struct device_node *np = dev->device_node;
-	size_t nelems;
-	u32 magic;
+	u32 *magic;
 
 	nelems = of_property_count_elems_of_size(np, "offset", sizeof(__be32));
-	if (nelems != 1)
+	if (nelems <= 0)
 		return -EINVAL;
 
-	syscon_rbm = xzalloc(sizeof(*syscon_rbm));
+	syscon_rbm = xzalloc(struct_size(syscon_rbm, reg, nelems));
+
+	ret = dev_get_drvdata(dev, (const void **)&reboot_template);
+	if (ret)
+		return ret;
 
+	syscon_rbm->reboot = *reboot_template;
 	syscon_rbm->reboot.dev = dev;
-	syscon_rbm->reboot.write = syscon_reboot_mode_write;
-	syscon_rbm->mask = 0xffffffff;
 
 	syscon_rbm->map = syscon_node_to_regmap(dev->parent->device_node);
 	if (IS_ERR(syscon_rbm->map))
 		return PTR_ERR(syscon_rbm->map);
 
-	if (of_property_read_u32(np, "offset", &syscon_rbm->offset))
-		return -EINVAL;
+	magic = xzalloc(nelems * sizeof(*magic));
 
-	of_property_read_u32(np, "mask", &syscon_rbm->mask);
+	for (i = 0; i < nelems; i++) {
+		struct mode_reg *reg = &syscon_rbm->reg[i];
 
-	ret = regmap_read(syscon_rbm->map, syscon_rbm->offset, &magic);
-	if (ret) {
-		dev_err(dev, "error reading reboot mode: %s\n",
-			strerror(-ret));
-		return ret;
-	}
+		ret = of_property_read_u32_index(np, "offset", i, &reg->offset);
+		if (ret)
+			goto free_magic;
 
-	magic &= syscon_rbm->mask;
+		reg->mask = 0xffffffff;
+		of_property_read_u32_index(np, "mask", i, &reg->mask);
+
+		ret = regmap_read(syscon_rbm->map, reg->offset, &magic[i]);
+		if (ret) {
+			dev_err(dev, "error reading reboot mode: %s\n",
+				strerror(-ret));
+			goto free_magic;
+		}
+
+		magic[i] &= reg->mask;
+	}
 
-	ret = reboot_mode_register(&syscon_rbm->reboot, &magic, 1);
+	ret = reboot_mode_register(&syscon_rbm->reboot, magic, nelems);
 	if (ret)
 		dev_err(dev, "can't register reboot mode\n");
 
+free_magic:
+	free(magic);
 	return ret;
+
 }
 
+static struct reboot_mode_driver reboot_fixup  = {
+	.write = syscon_reboot_mode_write,
+	.priority = 100,
+	.no_fixup = false,
+};
+
+static struct reboot_mode_driver reboot_nofixup  = {
+	.write = syscon_reboot_mode_write,
+	.priority = 50,
+	.no_fixup = true,
+};
+
 static const struct of_device_id syscon_reboot_mode_of_match[] = {
-	{ .compatible = "syscon-reboot-mode" },
+	{ .compatible = "syscon-reboot-mode", .data = &reboot_fixup },
+	{ .compatible = "barebox,syscon-reboot-mode", .data = &reboot_nofixup },
 	{ /* sentinel */ }
 };
 
-- 
2.28.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 10/10] ARM: dts: i.MX6qdl: define BootROM reboot-mode on top of SRC_GPR{9, 10}
  2020-09-16 13:50 [PATCH 00/10] power: reset: add support for syscon reboot modes Ahmad Fatoum
                   ` (8 preceding siblings ...)
  2020-09-16 13:50 ` [PATCH 09/10] power: reset: syscon-reboot-mode: support multi-word reboot modes Ahmad Fatoum
@ 2020-09-16 13:50 ` Ahmad Fatoum
  9 siblings, 0 replies; 15+ messages in thread
From: Ahmad Fatoum @ 2020-09-16 13:50 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

The SRC general purpose registers of the i.MX6 keep their values after a
warm reset and are used for communication between the BootROM and upper
level software.

SRC_GPR9 allows software override of SRC_SBMR1, e.g. to boot via
serial download protocol. Define a suitable syscon-reboot-mode
node to use it.

To have SRC_GPR9 take effect, bit 28 in SRC_GPR10 has to be set as well.
To support this, we use the backward-compatible barebox-specific binding
for having multiple 32-bit values for a single mode.

This node will _not_ be fixed up into the kernel device tree due to
the barebox-specific compatible, but as with all reboot mode storage,
the referenced locations will be cleared to the normal (here all-zero)
mode. User software that expects exclusive access to GPR9 while GPR10
bit 28 is zero will be broken.

Rebooting into serial download is now possible via:

  barebox@board:/ gpr.reboot_mode.next=serial reset

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/dts/imx6qdl.dtsi | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/dts/imx6qdl.dtsi b/arch/arm/dts/imx6qdl.dtsi
index 828be9ce0dbb..c3e02d2117b3 100644
--- a/arch/arm/dts/imx6qdl.dtsi
+++ b/arch/arm/dts/imx6qdl.dtsi
@@ -6,5 +6,26 @@
 		pwm2 = &pwm3;
 		pwm3 = &pwm4;
 		ipu0 = &ipu1;
+		gpr.reboot_mode = &reboot_mode_gpr;
+	};
+};
+
+&src {
+	compatible = "fsl,imx6q-src", "fsl,imx51-src", "syscon", "simple-mfd";
+
+	reboot_mode_gpr: reboot-mode {
+		compatible = "barebox,syscon-reboot-mode";
+		offset = <0x40>, <0x44>; /* SRC_GPR{9,10} */
+		mask = <0xffffffff>, <0x10000000>;
+		mode-normal = <0>, <0>;
+		mode-serial = <0x00000010>, <0x10000000>;
+		mode-spi0-0 = <0x08000030>, <0x10000000>;
+		mode-spi0-1 = <0x18000030>, <0x10000000>;
+		mode-spi0-2 = <0x28000030>, <0x10000000>;
+		mode-spi0-3 = <0x38000030>, <0x10000000>;
+		mode-mmc0 = <0x00002040>, <0x10000000>;
+		mode-mmc1 = <0x00002840>, <0x10000000>;
+		mode-mmc2 = <0x00003040>, <0x10000000>;
+		mode-mmc3 = <0x00003840>, <0x10000000>;
 	};
 };
-- 
2.28.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 01/10] usbgadget: autostart: support delayed usbgadget.autostart=1
  2020-09-16 13:50 ` [PATCH 01/10] usbgadget: autostart: support delayed usbgadget.autostart=1 Ahmad Fatoum
@ 2020-09-16 13:55   ` Ahmad Fatoum
  0 siblings, 0 replies; 15+ messages in thread
From: Ahmad Fatoum @ 2020-09-16 13:55 UTC (permalink / raw)
  To: Ahmad Fatoum, barebox

On 9/16/20 3:50 PM, Ahmad Fatoum wrote:
> So far, global.usbgadget.autostart=1 from the shell was without effect,
> because the variable is only evaluated once at postenvironment_initcall.
> 
> Use the new globalvar_add_bool() to allow acting on the variable being

Sent out one patch to few. I just sent out the prerequisite
"globalvar: allow running actions on set with globalvar_add_bool()" patch:
<20200916135409.24896-1-a.fatoum@pengutronix.de>

> true at any time. This is necessary for scripts that want to enable
> the usbgadget autostart functionality selectively without themselves
> hardcoding the particularities of what is exported.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  common/usbgadget.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/common/usbgadget.c b/common/usbgadget.c
> index b4f4ba04ca8c..be2bcc467d72 100644
> --- a/common/usbgadget.c
> +++ b/common/usbgadget.c
> @@ -101,7 +101,7 @@ int usbgadget_register(bool dfu, const char *dfu_opts,
>  	return ret;
>  }
>  
> -static int usbgadget_autostart(void)
> +static int usbgadget_autostart_set(struct param_d *param, void *ctx)
>  {
>  	bool fastboot_bbu = get_fastboot_bbu();
>  
> @@ -110,12 +110,12 @@ static int usbgadget_autostart(void)
>  
>  	return usbgadget_register(true, NULL, true, NULL, acm, fastboot_bbu);
>  }
> -postenvironment_initcall(usbgadget_autostart);
>  
>  static int usbgadget_globalvars_init(void)
>  {
>  	if (IS_ENABLED(CONFIG_USB_GADGET_AUTOSTART)) {
> -		globalvar_add_simple_bool("usbgadget.autostart", &autostart);
> +		globalvar_add_bool("usbgadget.autostart", usbgadget_autostart_set,
> +				   &autostart, NULL);
>  		globalvar_add_simple_bool("usbgadget.acm", &acm);
>  	}
>  	globalvar_add_simple_string("usbgadget.dfu_function", &dfu_function);
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 04/10] power: reset: reboot-mode: fix up node into boot device tree
  2020-09-16 13:50 ` [PATCH 04/10] power: reset: reboot-mode: fix up node into boot device tree Ahmad Fatoum
@ 2020-09-21  8:40   ` Sascha Hauer
  2020-09-21  9:11     ` Ahmad Fatoum
  0 siblings, 1 reply; 15+ messages in thread
From: Sascha Hauer @ 2020-09-21  8:40 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Wed, Sep 16, 2020 at 03:50:29PM +0200, Ahmad Fatoum wrote:
> Instead of relying that the kernel and barebox device trees are in sync,
> just enforce it by having barebox fix up the device tree node it probed
> into the kernel device tree. We usually want that, but some reboot mode
> drivers might want to inhibit the fixup, e.g. because they implement
> a non-upstream binding or because they communicate with the BootROM,
> while the kernel shouldn't. For those the fixup is made optional via
> a struct reboot_mode_driver::no_fixup member.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/power/reset/reboot-mode.c | 39 +++++++++++++++++++++++++++++++
>  include/linux/reboot-mode.h       |  1 +
>  2 files changed, 40 insertions(+)
> 
> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
> index 5992a2acd99a..5fc29ebf8091 100644
> --- a/drivers/power/reset/reboot-mode.c
> +++ b/drivers/power/reset/reboot-mode.c
> @@ -52,6 +52,42 @@ static int reboot_mode_add_param(struct device_d *dev,
>  	return PTR_ERR_OR_ZERO(param);
>  }
>  
> +static struct device_node *of_get_node_by_reproducible_name(struct device_node *dstroot,
> +							    struct device_node *srcnp)
> +{
> +	struct device_node *dstnp;
> +	char *name;
> +
> +	name = of_get_reproducible_name(srcnp);
> +	dstnp = of_find_node_by_reproducible_name(dstroot, name);
> +	free(name);
> +
> +	return dstnp;
> +}
> +
> +static int of_reboot_mode_fixup(struct device_node *root, void *ctx)
> +{
> +	struct reboot_mode_driver *reboot = ctx;
> +	struct device_node *dstnp, *srcnp, *dstparent;
> +
> +	srcnp = reboot->dev->device_node;
> +
> +	dstnp = of_get_node_by_reproducible_name(root, srcnp);
> +	if (dstnp) {
> +		dstparent = dstnp->parent;
> +		of_delete_node(dstnp);
> +	} else {
> +		dstparent = of_get_node_by_reproducible_name(root, srcnp->parent);
> +	}

\o/ barebox KASan found it's first bug!

This ends up in:

ERROR: ==================================================================
ERROR: BUG: KASAN: out-of-bounds in of_copy_node+0x11/0x8e
ERROR: Read of size 4 at addr 2fe0a2a0
ERROR:
WARNING: [<4fcd3101>] (unwind_backtrace+0x1/0x100) from [<4fc93e0d>] (kasan_report+0xa9/0x214)
WARNING: [<4fc93e0d>] (kasan_report+0xa9/0x214) from [<4fc7cd83>] (of_copy_node+0x11/0x8e)
WARNING: [<4fc7cd83>] (of_copy_node+0x11/0x8e) from [<4fc811b7>] (of_reboot_mode_fixup+0x9f/0xd4)
WARNING: [<4fc811b7>] (of_reboot_mode_fixup+0x9f/0xd4) from [<4fc0bda9>] (of_fix_tree+0x3d/0x70)
WARNING: [<4fc0bda9>] (of_fix_tree+0x3d/0x70) from [<4fc0bdf5>] (of_get_fixed_tree+0x19/0x2c)
WARNING: [<4fc0bdf5>] (of_get_fixed_tree+0x19/0x2c) from [<4fc05ae9>] (bootm_get_devicetree+0x279/0x2b4)
WARNING: [<4fc05ae9>] (bootm_get_devicetree+0x279/0x2b4) from [<4fcd2111>] (__do_bootm_linux+0xdd/0x1e0)
WARNING: [<4fcd2111>] (__do_bootm_linux+0xdd/0x1e0) from [<4fcd2299>] (do_bootm_linux+0x85/0xa0)
WARNING: [<4fcd2299>] (do_bootm_linux+0x85/0xa0) from [<4fc062bd>] (bootm_boot+0x60d/0x684)
WARNING: [<4fc062bd>] (bootm_boot+0x60d/0x684) from [<4fc820d5>] (do_bootm+0x1b5/0x1f8)
WARNING: [<4fc820d5>] (do_bootm+0x1b5/0x1f8) from [<4fc06dd3>] (execute_command+0x6b/0xb4)
WARNING: [<4fc06dd3>] (execute_command+0x6b/0xb4) from [<4fc0f835>] (run_list_real+0x985/0x9f0)
WARNING: [<4fc0f835>] (run_list_real+0x985/0x9f0) from [<4fc0ed05>] (parse_stream_outer+0x221/0x2a8)
WARNING: [<4fc0ed05>] (parse_stream_outer+0x221/0x2a8) from [<4fc0fc0b>] (run_shell+0x8b/0xdc)
WARNING: [<4fc0fc0b>] (run_shell+0x8b/0xdc) from [<4fc01895>] (run_init+0x205/0x264)
WARNING: [<4fc01895>] (run_init+0x205/0x264) from [<4fc01933>] (start_barebox+0x3f/0xa4)
WARNING: [<4fc01933>] (start_barebox+0x3f/0xa4) from [<4fcd09f9>] (barebox_non_pbl_start+0x131/0x174)
WARNING: [<4fcd09f9>] (barebox_non_pbl_start+0x131/0x174) from [<4fc00005>] (__bare_init_start+0x1/0xc)

This happens in the case when the to-be-fixed up device tree is the internal
device tree itself. Here of_delete_node(dstnp) is called, but this is
also srcnp which is then read from.

The solution might also be to make sure we make a copy of the internal
tree before doing a of_fixup on it.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 04/10] power: reset: reboot-mode: fix up node into boot device tree
  2020-09-21  8:40   ` Sascha Hauer
@ 2020-09-21  9:11     ` Ahmad Fatoum
  0 siblings, 0 replies; 15+ messages in thread
From: Ahmad Fatoum @ 2020-09-21  9:11 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

Hi,

On 9/21/20 10:40 AM, Sascha Hauer wrote:
> On Wed, Sep 16, 2020 at 03:50:29PM +0200, Ahmad Fatoum wrote:
>> Instead of relying that the kernel and barebox device trees are in sync,
>> just enforce it by having barebox fix up the device tree node it probed
>> into the kernel device tree. We usually want that, but some reboot mode
>> drivers might want to inhibit the fixup, e.g. because they implement
>> a non-upstream binding or because they communicate with the BootROM,
>> while the kernel shouldn't. For those the fixup is made optional via
>> a struct reboot_mode_driver::no_fixup member.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  drivers/power/reset/reboot-mode.c | 39 +++++++++++++++++++++++++++++++
>>  include/linux/reboot-mode.h       |  1 +
>>  2 files changed, 40 insertions(+)
>>
>> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
>> index 5992a2acd99a..5fc29ebf8091 100644
>> --- a/drivers/power/reset/reboot-mode.c
>> +++ b/drivers/power/reset/reboot-mode.c
>> @@ -52,6 +52,42 @@ static int reboot_mode_add_param(struct device_d *dev,
>>  	return PTR_ERR_OR_ZERO(param);
>>  }
>>  
>> +static struct device_node *of_get_node_by_reproducible_name(struct device_node *dstroot,
>> +							    struct device_node *srcnp)
>> +{
>> +	struct device_node *dstnp;
>> +	char *name;
>> +
>> +	name = of_get_reproducible_name(srcnp);
>> +	dstnp = of_find_node_by_reproducible_name(dstroot, name);
>> +	free(name);
>> +
>> +	return dstnp;
>> +}
>> +
>> +static int of_reboot_mode_fixup(struct device_node *root, void *ctx)
>> +{
>> +	struct reboot_mode_driver *reboot = ctx;
>> +	struct device_node *dstnp, *srcnp, *dstparent;
>> +
>> +	srcnp = reboot->dev->device_node;
>> +
>> +	dstnp = of_get_node_by_reproducible_name(root, srcnp);
>> +	if (dstnp) {
>> +		dstparent = dstnp->parent;
>> +		of_delete_node(dstnp);
>> +	} else {
>> +		dstparent = of_get_node_by_reproducible_name(root, srcnp->parent);
>> +	}
> 
> \o/ barebox KASan found it's first bug!

\o/

> 
> This ends up in:
> 
> ERROR: ==================================================================
> ERROR: BUG: KASAN: out-of-bounds in of_copy_node+0x11/0x8e
> ERROR: Read of size 4 at addr 2fe0a2a0
> ERROR:
> WARNING: [<4fcd3101>] (unwind_backtrace+0x1/0x100) from [<4fc93e0d>] (kasan_report+0xa9/0x214)
> WARNING: [<4fc93e0d>] (kasan_report+0xa9/0x214) from [<4fc7cd83>] (of_copy_node+0x11/0x8e)
> WARNING: [<4fc7cd83>] (of_copy_node+0x11/0x8e) from [<4fc811b7>] (of_reboot_mode_fixup+0x9f/0xd4)
> WARNING: [<4fc811b7>] (of_reboot_mode_fixup+0x9f/0xd4) from [<4fc0bda9>] (of_fix_tree+0x3d/0x70)
> WARNING: [<4fc0bda9>] (of_fix_tree+0x3d/0x70) from [<4fc0bdf5>] (of_get_fixed_tree+0x19/0x2c)
> WARNING: [<4fc0bdf5>] (of_get_fixed_tree+0x19/0x2c) from [<4fc05ae9>] (bootm_get_devicetree+0x279/0x2b4)
> WARNING: [<4fc05ae9>] (bootm_get_devicetree+0x279/0x2b4) from [<4fcd2111>] (__do_bootm_linux+0xdd/0x1e0)
> WARNING: [<4fcd2111>] (__do_bootm_linux+0xdd/0x1e0) from [<4fcd2299>] (do_bootm_linux+0x85/0xa0)
> WARNING: [<4fcd2299>] (do_bootm_linux+0x85/0xa0) from [<4fc062bd>] (bootm_boot+0x60d/0x684)
> WARNING: [<4fc062bd>] (bootm_boot+0x60d/0x684) from [<4fc820d5>] (do_bootm+0x1b5/0x1f8)
> WARNING: [<4fc820d5>] (do_bootm+0x1b5/0x1f8) from [<4fc06dd3>] (execute_command+0x6b/0xb4)
> WARNING: [<4fc06dd3>] (execute_command+0x6b/0xb4) from [<4fc0f835>] (run_list_real+0x985/0x9f0)
> WARNING: [<4fc0f835>] (run_list_real+0x985/0x9f0) from [<4fc0ed05>] (parse_stream_outer+0x221/0x2a8)
> WARNING: [<4fc0ed05>] (parse_stream_outer+0x221/0x2a8) from [<4fc0fc0b>] (run_shell+0x8b/0xdc)
> WARNING: [<4fc0fc0b>] (run_shell+0x8b/0xdc) from [<4fc01895>] (run_init+0x205/0x264)
> WARNING: [<4fc01895>] (run_init+0x205/0x264) from [<4fc01933>] (start_barebox+0x3f/0xa4)
> WARNING: [<4fc01933>] (start_barebox+0x3f/0xa4) from [<4fcd09f9>] (barebox_non_pbl_start+0x131/0x174)
> WARNING: [<4fcd09f9>] (barebox_non_pbl_start+0x131/0x174) from [<4fc00005>] (__bare_init_start+0x1/0xc)
> 
> This happens in the case when the to-be-fixed up device tree is the internal
> device tree itself. Here of_delete_node(dstnp) is called, but this is
> also srcnp which is then read from.
> 
> The solution might also be to make sure we make a copy of the internal
> tree before doing a of_fixup on it.

As the fixup is just a of_copy from the built-in device tree, we should just skip the
fixup altogether. I can resend the patch later. Could the other ones be applied? :-)

> 
> Sascha
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 09/10] power: reset: syscon-reboot-mode: support multi-word reboot modes
  2020-09-16 13:50 ` [PATCH 09/10] power: reset: syscon-reboot-mode: support multi-word reboot modes Ahmad Fatoum
@ 2020-10-07  8:18   ` Sascha Hauer
  0 siblings, 0 replies; 15+ messages in thread
From: Sascha Hauer @ 2020-10-07  8:18 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Wed, Sep 16, 2020 at 03:50:34PM +0200, Ahmad Fatoum wrote:
> SoCs like the i.MX6 have their BootROM consult two distinct 32-bit
> registers to determine the reboot mode. Extend the driver to support
> this. While backwards compatible, this is not so far supported by the
> upstream binding, which is why a new barebox,syscon-reboot-mode is
> introduced. This new compatible be inhibit used to inhibit fixup
> into a kernel device tree.
> 
>  static const struct of_device_id syscon_reboot_mode_of_match[] = {
> -	{ .compatible = "syscon-reboot-mode" },
> +	{ .compatible = "syscon-reboot-mode", .data = &reboot_fixup },
> +	{ .compatible = "barebox,syscon-reboot-mode", .data = &reboot_nofixup },

Please add this to Documentation/devicetree/bindings/

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

end of thread, other threads:[~2020-10-07  8:18 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-16 13:50 [PATCH 00/10] power: reset: add support for syscon reboot modes Ahmad Fatoum
2020-09-16 13:50 ` [PATCH 01/10] usbgadget: autostart: support delayed usbgadget.autostart=1 Ahmad Fatoum
2020-09-16 13:55   ` Ahmad Fatoum
2020-09-16 13:50 ` [PATCH 02/10] drivers: add reboot-mode infrastructure Ahmad Fatoum
2020-09-16 13:50 ` [PATCH 03/10] power: reset: reboot-mode: port syscon-reboot-mode support Ahmad Fatoum
2020-09-16 13:50 ` [PATCH 04/10] power: reset: reboot-mode: fix up node into boot device tree Ahmad Fatoum
2020-09-21  8:40   ` Sascha Hauer
2020-09-21  9:11     ` Ahmad Fatoum
2020-09-16 13:50 ` [PATCH 05/10] defaultenv: provide defaults for generic reboot modes Ahmad Fatoum
2020-09-16 13:50 ` [PATCH 06/10] ARM: dts: stm32mp: setup syscon-reboot-mode on TAMP general purpose register Ahmad Fatoum
2020-09-16 13:50 ` [PATCH 07/10] ARM: stm32mp: remove custom reboot mode logic from arch code Ahmad Fatoum
2020-09-16 13:50 ` [PATCH 08/10] power: reset: reboot-mode: support multi-word magic Ahmad Fatoum
2020-09-16 13:50 ` [PATCH 09/10] power: reset: syscon-reboot-mode: support multi-word reboot modes Ahmad Fatoum
2020-10-07  8:18   ` Sascha Hauer
2020-09-16 13:50 ` [PATCH 10/10] ARM: dts: i.MX6qdl: define BootROM reboot-mode on top of SRC_GPR{9, 10} Ahmad Fatoum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox