Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1220745 > unrolled thread
| Started by | Andy Yan <andy.yan@rock-chips.com> |
|---|---|
| First post | 2015-09-08 14:50 +0200 |
| Last post | 2015-09-17 13:10 +0200 |
| Articles | 7 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH] ARM: rockchip: add reboot notifier Andy Yan <andy.yan@rock-chips.com> - 2015-09-08 14:50 +0200
Re: [PATCH] ARM: rockchip: add reboot notifier Alexey Klimov <klimov.linux@gmail.com> - 2015-09-09 00:00 +0200
Re: [PATCH] ARM: rockchip: add reboot notifier Andy Yan <andy.yan@rock-chips.com> - 2015-09-10 12:50 +0200
set rockchip-specific uboot bootmode flags on reboot (was: [PATCH] ARM: rockchip: add reboot notifier) Heiko Stübner <heiko@sntech.de> - 2015-09-09 00:50 +0200
Re: set rockchip-specific uboot bootmode flags on reboot Andy Yan <andy.yan@rock-chips.com> - 2015-09-09 03:10 +0200
Re: set rockchip-specific uboot bootmode flags on reboot (was: [PATCH] ARM: rockchip: add reboot notifier) Simon Glass <sjg@chromium.org> - 2015-09-09 20:10 +0200
Re: set rockchip-specific uboot bootmode flags on reboot Andy Yan <andy.yan@rock-chips.com> - 2015-09-17 13:10 +0200
| From | Andy Yan <andy.yan@rock-chips.com> |
|---|---|
| Date | 2015-09-08 14:50 +0200 |
| Subject | [PATCH] ARM: rockchip: add reboot notifier |
| Message-ID | <q6qvw-4v7-21@gated-at.bofh.it> |
rockchip platform have a protocol to pass the the kernel
reboot mode to bootloader by some special registers when
system reboot.By this way the bootloader can take different
action according to the different kernel reboot mode, for
example, command "reboot loader" will reboot the board to
rockusb mode, this is a very convenient way to get the board
to download mode.
Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
---
arch/arm/mach-rockchip/Makefile | 2 +-
arch/arm/mach-rockchip/loader.h | 22 +++++++++
arch/arm/mach-rockchip/reboot.c | 103 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 126 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/mach-rockchip/loader.h
create mode 100644 arch/arm/mach-rockchip/reboot.c
diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
index 5c3a9b2..cd291e3 100644
--- a/arch/arm/mach-rockchip/Makefile
+++ b/arch/arm/mach-rockchip/Makefile
@@ -1,5 +1,5 @@
CFLAGS_platsmp.o := -march=armv7-a
-obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip.o
+obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip.o reboot.o
obj-$(CONFIG_PM_SLEEP) += pm.o sleep.o
obj-$(CONFIG_SMP) += headsmp.o platsmp.o
diff --git a/arch/arm/mach-rockchip/loader.h b/arch/arm/mach-rockchip/loader.h
new file mode 100644
index 0000000..bf51baa
--- /dev/null
+++ b/arch/arm/mach-rockchip/loader.h
@@ -0,0 +1,22 @@
+#ifndef __MACH_ROCKCHIP_LOADER_H
+#define __MACH_ROCKCHIP_LOADER_H
+
+/*high 24 bits is tag, low 8 bits is type*/
+#define SYS_LOADER_REBOOT_FLAG 0x5242C300
+
+enum {
+ BOOT_NORMAL = 0, /* normal boot */
+ BOOT_LOADER, /* enter loader rockusb mode */
+ BOOT_MASKROM, /* enter maskrom rockusb mode (not support now) */
+ BOOT_RECOVER, /* enter recover */
+ BOOT_NORECOVER, /* do not enter recover */
+ BOOT_SECONDOS, /* boot second OS (not support now)*/
+ BOOT_WIPEDATA, /* enter recover and wipe data. */
+ BOOT_WIPEALL, /* enter recover and wipe all data. */
+ BOOT_CHECKIMG, /* check firmware img with backup part*/
+ BOOT_FASTBOOT, /* enter fast boot mode */
+ BOOT_SECUREBOOT_DISABLE,
+ BOOT_CHARGING, /* enter charge mode */
+ BOOT_MAX /* MAX VALID BOOT TYPE.*/
+};
+#endif
diff --git a/arch/arm/mach-rockchip/reboot.c b/arch/arm/mach-rockchip/reboot.c
new file mode 100644
index 0000000..704bc16
--- /dev/null
+++ b/arch/arm/mach-rockchip/reboot.c
@@ -0,0 +1,103 @@
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/reboot.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+#include "loader.h"
+
+#define RK3188_PMU_SYS_REG0 0x40
+#define RK3288_PMU_SYS_REG0 0x94
+
+struct regmap *regmap;
+int flag_reg;
+
+static int rockchip_get_pmu_regmap(void)
+{
+ struct device_node *node;
+
+ node = of_find_node_by_path("/cpus");
+
+ regmap = syscon_regmap_lookup_by_phandle(node, "rockchip,pmu");
+ of_node_put(node);
+ if (!IS_ERR(regmap))
+ return 0;
+
+ regmap = syscon_regmap_lookup_by_compatible("rockchip,rk3066-pmu");
+ of_node_put(node);
+ if (!IS_ERR(regmap))
+ return 0;
+
+ return -ENODEV;
+}
+
+static int rockchip_get_reboot_flag_regmap(void)
+{
+ int ret = rockchip_get_pmu_regmap();
+
+ if (ret < 0)
+ return ret;
+
+ if (of_machine_is_compatible("rockchip,rk3288")) {
+ flag_reg = RK3288_PMU_SYS_REG0;
+ return 0;
+ } else if (of_machine_is_compatible("rockchip,rk3066a") ||
+ of_machine_is_compatible("rockchip,rk3066b") ||
+ of_machine_is_compatible("rockchip,rk3188")) {
+ flag_reg = RK3188_PMU_SYS_REG0;
+ return 0;
+ }
+
+ return -ENODEV;
+}
+
+static void rockchip_get_reboot_flag(const char *cmd, u32 *flag)
+{
+ *flag = SYS_LOADER_REBOOT_FLAG + BOOT_NORMAL;
+
+ if (cmd) {
+ if (!strcmp(cmd, "loader") || !strcmp(cmd, "bootloader"))
+ *flag = SYS_LOADER_REBOOT_FLAG + BOOT_LOADER;
+ else if (!strcmp(cmd, "recovery"))
+ *flag = SYS_LOADER_REBOOT_FLAG + BOOT_RECOVER;
+ else if (!strcmp(cmd, "charge"))
+ *flag = SYS_LOADER_REBOOT_FLAG + BOOT_CHARGING;
+ }
+}
+
+static int rockchip_reboot_notify(struct notifier_block *this,
+ unsigned long mode, void *cmd)
+{
+ u32 flag;
+
+ rockchip_get_reboot_flag(cmd, &flag);
+ regmap_write(regmap, flag_reg, flag);
+
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block rockchip_reboot_handler = {
+ .notifier_call = rockchip_reboot_notify,
+ .priority = 150,
+};
+
+static int __init rockchip_reboot_init(void)
+{
+ int ret = 0;
+
+ if (!rockchip_get_reboot_flag_regmap()) {
+ ret = register_restart_handler(&rockchip_reboot_handler);
+ if (ret)
+ pr_err("%s: cannot register reboot handler, %d\n",
+ __func__, ret);
+ }
+
+return ret;
+}
+
+module_init(rockchip_reboot_init);
+MODULE_AUTHOR("Andy Yan <andy.yan@rock-chips.com");
+MODULE_DESCRIPTION("Rockchip platform reboot notifier driver");
+MODULE_LICENSE("GPL");
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Alexey Klimov <klimov.linux@gmail.com> |
|---|---|
| Date | 2015-09-09 00:00 +0200 |
| Message-ID | <q6z5M-8sk-19@gated-at.bofh.it> |
| In reply to | #1220745 |
Hi Andy,
On Tue, Sep 8, 2015 at 3:43 PM, Andy Yan <andy.yan@rock-chips.com> wrote:
> rockchip platform have a protocol to pass the the kernel
Double 'the'?
> reboot mode to bootloader by some special registers when
> system reboot. By this way the bootloader can take different
> action according to the different kernel reboot mode, for
> example, command "reboot loader" will reboot the board to
> rockusb mode, this is a very convenient way to get the board
> to download mode.
>
> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
> ---
>
> arch/arm/mach-rockchip/Makefile | 2 +-
> arch/arm/mach-rockchip/loader.h | 22 +++++++++
> arch/arm/mach-rockchip/reboot.c | 103 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 126 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm/mach-rockchip/loader.h
> create mode 100644 arch/arm/mach-rockchip/reboot.c
>
> diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
> index 5c3a9b2..cd291e3 100644
> --- a/arch/arm/mach-rockchip/Makefile
> +++ b/arch/arm/mach-rockchip/Makefile
> @@ -1,5 +1,5 @@
> CFLAGS_platsmp.o := -march=armv7-a
>
> -obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip.o
> +obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip.o reboot.o
> obj-$(CONFIG_PM_SLEEP) += pm.o sleep.o
> obj-$(CONFIG_SMP) += headsmp.o platsmp.o
> diff --git a/arch/arm/mach-rockchip/loader.h b/arch/arm/mach-rockchip/loader.h
> new file mode 100644
> index 0000000..bf51baa
> --- /dev/null
> +++ b/arch/arm/mach-rockchip/loader.h
> @@ -0,0 +1,22 @@
> +#ifndef __MACH_ROCKCHIP_LOADER_H
> +#define __MACH_ROCKCHIP_LOADER_H
> +
> +/*high 24 bits is tag, low 8 bits is type*/
> +#define SYS_LOADER_REBOOT_FLAG 0x5242C300
> +
> +enum {
> + BOOT_NORMAL = 0, /* normal boot */
> + BOOT_LOADER, /* enter loader rockusb mode */
> + BOOT_MASKROM, /* enter maskrom rockusb mode (not support now) */
> + BOOT_RECOVER, /* enter recover */
> + BOOT_NORECOVER, /* do not enter recover */
> + BOOT_SECONDOS, /* boot second OS (not support now)*/
> + BOOT_WIPEDATA, /* enter recover and wipe data. */
> + BOOT_WIPEALL, /* enter recover and wipe all data. */
> + BOOT_CHECKIMG, /* check firmware img with backup part*/
> + BOOT_FASTBOOT, /* enter fast boot mode */
> + BOOT_SECUREBOOT_DISABLE,
> + BOOT_CHARGING, /* enter charge mode */
> + BOOT_MAX /* MAX VALID BOOT TYPE.*/
Looks like you only implemented NORMAL, RECOVER, LOADER and CHARGING.
Are you keeping other entries for keeping right order and keep
consistency?
Or have plans for future?
> +};
> +#endif
> diff --git a/arch/arm/mach-rockchip/reboot.c b/arch/arm/mach-rockchip/reboot.c
> new file mode 100644
> index 0000000..704bc16
> --- /dev/null
> +++ b/arch/arm/mach-rockchip/reboot.c
> @@ -0,0 +1,103 @@
> +#include <linux/init.h>
Usually people place in the beginning copyright and GPL license header info.
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/reboot.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> +#include "loader.h"
> +
> +#define RK3188_PMU_SYS_REG0 0x40
> +#define RK3288_PMU_SYS_REG0 0x94
> +
> +struct regmap *regmap;
> +int flag_reg;
> +
> +static int rockchip_get_pmu_regmap(void)
> +{
> + struct device_node *node;
> +
> + node = of_find_node_by_path("/cpus");
Is it critical not to check node for NULL here?
> + regmap = syscon_regmap_lookup_by_phandle(node, "rockchip,pmu");
> + of_node_put(node);
> + if (!IS_ERR(regmap))
> + return 0;
> +
> + regmap = syscon_regmap_lookup_by_compatible("rockchip,rk3066-pmu");
> + of_node_put(node);
> + if (!IS_ERR(regmap))
> + return 0;
> +
> + return -ENODEV;
> +}
This double of_node_put(node) confuses me. Could you please guide me over it?
After I tried to re-create it by myself looking to code I think that
second of_node_put() is not needed.
> +static int rockchip_get_reboot_flag_regmap(void)
> +{
> + int ret = rockchip_get_pmu_regmap();
> +
> + if (ret < 0)
> + return ret;
> +
> + if (of_machine_is_compatible("rockchip,rk3288")) {
> + flag_reg = RK3288_PMU_SYS_REG0;
> + return 0;
> + } else if (of_machine_is_compatible("rockchip,rk3066a") ||
> + of_machine_is_compatible("rockchip,rk3066b") ||
> + of_machine_is_compatible("rockchip,rk3188")) {
> + flag_reg = RK3188_PMU_SYS_REG0;
> + return 0;
> + }
> +
> + return -ENODEV;
[..]
> +
> +static int __init rockchip_reboot_init(void)
> +{
> + int ret = 0;
> +
> + if (!rockchip_get_reboot_flag_regmap()) {
> + ret = register_restart_handler(&rockchip_reboot_handler);
> + if (ret)
> + pr_err("%s: cannot register reboot handler, %d\n",
> + __func__, ret);
> + }
> +
> +return ret;
Please align this correctly.
Thanks,
Alexey Klimov
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Andy Yan <andy.yan@rock-chips.com> |
|---|---|
| Date | 2015-09-10 12:50 +0200 |
| Message-ID | <q77Au-7r7-7@gated-at.bofh.it> |
| In reply to | #1221113 |
Hi Alexey:
On 2015年09月09日 05:50, Alexey Klimov wrote:
> Hi Andy,
>
> On Tue, Sep 8, 2015 at 3:43 PM, Andy Yan <andy.yan@rock-chips.com> wrote:
>> rockchip platform have a protocol to pass the the kernel
> Double 'the'?
this is will be removed.
>
>> reboot mode to bootloader by some special registers when
>> system reboot. By this way the bootloader can take different
>> action according to the different kernel reboot mode, for
>> example, command "reboot loader" will reboot the board to
>> rockusb mode, this is a very convenient way to get the board
>> to download mode.
>>
>> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
>> ---
>>
>> arch/arm/mach-rockchip/Makefile | 2 +-
>> arch/arm/mach-rockchip/loader.h | 22 +++++++++
>> arch/arm/mach-rockchip/reboot.c | 103 ++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 126 insertions(+), 1 deletion(-)
>> create mode 100644 arch/arm/mach-rockchip/loader.h
>> create mode 100644 arch/arm/mach-rockchip/reboot.c
>>
>> diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
>> index 5c3a9b2..cd291e3 100644
>> --- a/arch/arm/mach-rockchip/Makefile
>> +++ b/arch/arm/mach-rockchip/Makefile
>> @@ -1,5 +1,5 @@
>> CFLAGS_platsmp.o := -march=armv7-a
>>
>> -obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip.o
>> +obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip.o reboot.o
>> obj-$(CONFIG_PM_SLEEP) += pm.o sleep.o
>> obj-$(CONFIG_SMP) += headsmp.o platsmp.o
>> diff --git a/arch/arm/mach-rockchip/loader.h b/arch/arm/mach-rockchip/loader.h
>> new file mode 100644
>> index 0000000..bf51baa
>> --- /dev/null
>> +++ b/arch/arm/mach-rockchip/loader.h
>> @@ -0,0 +1,22 @@
>> +#ifndef __MACH_ROCKCHIP_LOADER_H
>> +#define __MACH_ROCKCHIP_LOADER_H
>> +
>> +/*high 24 bits is tag, low 8 bits is type*/
>> +#define SYS_LOADER_REBOOT_FLAG 0x5242C300
>> +
>> +enum {
>> + BOOT_NORMAL = 0, /* normal boot */
>> + BOOT_LOADER, /* enter loader rockusb mode */
>> + BOOT_MASKROM, /* enter maskrom rockusb mode (not support now) */
>> + BOOT_RECOVER, /* enter recover */
>> + BOOT_NORECOVER, /* do not enter recover */
>> + BOOT_SECONDOS, /* boot second OS (not support now)*/
>> + BOOT_WIPEDATA, /* enter recover and wipe data. */
>> + BOOT_WIPEALL, /* enter recover and wipe all data. */
>> + BOOT_CHECKIMG, /* check firmware img with backup part*/
>> + BOOT_FASTBOOT, /* enter fast boot mode */
>> + BOOT_SECUREBOOT_DISABLE,
>> + BOOT_CHARGING, /* enter charge mode */
>> + BOOT_MAX /* MAX VALID BOOT TYPE.*/
> Looks like you only implemented NORMAL, RECOVER, LOADER and CHARGING.
> Are you keeping other entries for keeping right order and keep
> consistency?
> Or have plans for future?
to keep the right order,some of them maybe implemented in
the future.
>
>> +};
>> +#endif
>> diff --git a/arch/arm/mach-rockchip/reboot.c b/arch/arm/mach-rockchip/reboot.c
>> new file mode 100644
>> index 0000000..704bc16
>> --- /dev/null
>> +++ b/arch/arm/mach-rockchip/reboot.c
>> @@ -0,0 +1,103 @@
>> +#include <linux/init.h>
> Usually people place in the beginning copyright and GPL license header info.
>
>> +#include <linux/module.h>
>> +#include <linux/kernel.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/reboot.h>
>> +#include <linux/regmap.h>
>> +#include <linux/mfd/syscon.h>
>> +#include "loader.h"
>> +
>> +#define RK3188_PMU_SYS_REG0 0x40
>> +#define RK3288_PMU_SYS_REG0 0x94
>> +
>> +struct regmap *regmap;
>> +int flag_reg;
>> +
>> +static int rockchip_get_pmu_regmap(void)
>> +{
>> + struct device_node *node;
>> +
>> + node = of_find_node_by_path("/cpus");
> Is it critical not to check node for NULL here?
ok, I will add a check here
>
>> + regmap = syscon_regmap_lookup_by_phandle(node, "rockchip,pmu");
>> + of_node_put(node);
>> + if (!IS_ERR(regmap))
>> + return 0;
>> +
>> + regmap = syscon_regmap_lookup_by_compatible("rockchip,rk3066-pmu");
>> + of_node_put(node);
>> + if (!IS_ERR(regmap))
>> + return 0;
>> +
>> + return -ENODEV;
>> +}
> This double of_node_put(node) confuses me. Could you please guide me over it?
>
> After I tried to re-create it by myself looking to code I think that
> second of_node_put() is not needed.
the second of_node_put is not needed, it will be removed.
>> +static int rockchip_get_reboot_flag_regmap(void)
>> +{
>> + int ret = rockchip_get_pmu_regmap();
>> +
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (of_machine_is_compatible("rockchip,rk3288")) {
>> + flag_reg = RK3288_PMU_SYS_REG0;
>> + return 0;
>> + } else if (of_machine_is_compatible("rockchip,rk3066a") ||
>> + of_machine_is_compatible("rockchip,rk3066b") ||
>> + of_machine_is_compatible("rockchip,rk3188")) {
>> + flag_reg = RK3188_PMU_SYS_REG0;
>> + return 0;
>> + }
>> +
>> + return -ENODEV;
> [..]
>
>> +
>> +static int __init rockchip_reboot_init(void)
>> +{
>> + int ret = 0;
>> +
>> + if (!rockchip_get_reboot_flag_regmap()) {
>> + ret = register_restart_handler(&rockchip_reboot_handler);
>> + if (ret)
>> + pr_err("%s: cannot register reboot handler, %d\n",
>> + __func__, ret);
>> + }
>> +
>> +return ret;
> Please align this correctly.
OK, this will be aligned next version
>
> Thanks,
> Alexey Klimov
>
>
>
Thanks for your review.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Heiko Stübner <heiko@sntech.de> |
|---|---|
| Date | 2015-09-09 00:50 +0200 |
| Subject | set rockchip-specific uboot bootmode flags on reboot (was: [PATCH] ARM: rockchip: add reboot notifier) |
| Message-ID | <q6zSa-1bn-23@gated-at.bofh.it> |
| In reply to | #1220745 |
Hi Andy,
Am Dienstag, 8. September 2015, 20:43:07 schrieb Andy Yan:
> rockchip platform have a protocol to pass the the kernel
> reboot mode to bootloader by some special registers when
> system reboot.By this way the bootloader can take different
> action according to the different kernel reboot mode, for
> example, command "reboot loader" will reboot the board to
> rockusb mode, this is a very convenient way to get the board
> to download mode.
>
> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
[...]
> @@ -0,0 +1,22 @@
> +#ifndef __MACH_ROCKCHIP_LOADER_H
> +#define __MACH_ROCKCHIP_LOADER_H
> +
> +/*high 24 bits is tag, low 8 bits is type*/
> +#define SYS_LOADER_REBOOT_FLAG 0x5242C300
> +
> +enum {
> + BOOT_NORMAL = 0, /* normal boot */
> + BOOT_LOADER, /* enter loader rockusb mode */
> + BOOT_MASKROM, /* enter maskrom rockusb mode (not support now) */
> + BOOT_RECOVER, /* enter recover */
> + BOOT_NORECOVER, /* do not enter recover */
> + BOOT_SECONDOS, /* boot second OS (not support now)*/
> + BOOT_WIPEDATA, /* enter recover and wipe data. */
> + BOOT_WIPEALL, /* enter recover and wipe all data. */
> + BOOT_CHECKIMG, /* check firmware img with backup part*/
> + BOOT_FASTBOOT, /* enter fast boot mode */
> + BOOT_SECUREBOOT_DISABLE,
> + BOOT_CHARGING, /* enter charge mode */
> + BOOT_MAX /* MAX VALID BOOT TYPE.*/
> +};
> +#endif
These flags rely on code in the bootloader to actually handle the target
action. Nowadays this is uboot, but still a rockchip-specific fork. And we're
actively moving away from that, with the recent rk3288 addition to mainline
uboot.
So unless you convince uboot people that the _underlying special
functionality_ behind these flags should be part of uboot, I don't think this
is going to fly.
In a way this is similar to gpu kernel code talking to proprietary userspace
libs - these are also not eligible for the kernel. (meaning stuff like the
mali kernel driver not being allowed).
[...]
> +static int rockchip_reboot_notify(struct notifier_block *this,
> + unsigned long mode, void *cmd)
> +{
> + u32 flag;
> +
> + rockchip_get_reboot_flag(cmd, &flag);
> + regmap_write(regmap, flag_reg, flag);
> +
> + return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block rockchip_reboot_handler = {
> + .notifier_call = rockchip_reboot_notify,
> + .priority = 150,
> +};
the restart handlers are meant to really only restart the system, not to
execute some actions before the restart happens.
See https://lkml.org/lkml/2015/6/3/707 for a similar case.
Heiko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Andy Yan <andy.yan@rock-chips.com> |
|---|---|
| Date | 2015-09-09 03:10 +0200 |
| Subject | Re: set rockchip-specific uboot bootmode flags on reboot |
| Message-ID | <q6C3E-4tG-1@gated-at.bofh.it> |
| In reply to | #1221127 |
Hi Heiko:
On 2015å¹´09æ09æ¥ 06:46, Heiko Stübner wrote:
> Hi Andy,
>
> Am Dienstag, 8. September 2015, 20:43:07 schrieb Andy Yan:
>> rockchip platform have a protocol to pass the the kernel
>> reboot mode to bootloader by some special registers when
>> system reboot.By this way the bootloader can take different
>> action according to the different kernel reboot mode, for
>> example, command "reboot loader" will reboot the board to
>> rockusb mode, this is a very convenient way to get the board
>> to download mode.
>>
>> Signed-off-by: Andy Yan<andy.yan@rock-chips.com>
> [...]
>
>> @@ -0,0 +1,22 @@
>> +#ifndef __MACH_ROCKCHIP_LOADER_H
>> +#define __MACH_ROCKCHIP_LOADER_H
>> +
>> +/*high 24 bits is tag, low 8 bits is type*/
>> +#define SYS_LOADER_REBOOT_FLAG 0x5242C300
>> +
>> +enum {
>> + BOOT_NORMAL = 0, /* normal boot */
>> + BOOT_LOADER, /* enter loader rockusb mode */
>> + BOOT_MASKROM, /* enter maskrom rockusb mode (not support now) */
>> + BOOT_RECOVER, /* enter recover */
>> + BOOT_NORECOVER, /* do not enter recover */
>> + BOOT_SECONDOS, /* boot second OS (not support now)*/
>> + BOOT_WIPEDATA, /* enter recover and wipe data. */
>> + BOOT_WIPEALL, /* enter recover and wipe all data. */
>> + BOOT_CHECKIMG, /* check firmware img with backup part*/
>> + BOOT_FASTBOOT, /* enter fast boot mode */
>> + BOOT_SECUREBOOT_DISABLE,
>> + BOOT_CHARGING, /* enter charge mode */
>> + BOOT_MAX /* MAX VALID BOOT TYPE.*/
>> +};
>> +#endif
> These flags rely on code in the bootloader to actually handle the target
> action. Nowadays this is uboot, but still a rockchip-specific fork. And we're
> actively moving away from that, with the recent rk3288 addition to mainline
> uboot.
  Sorry, I don't know about this action before, but this is really a
very convenient way
  to get machine enter download mode, it seems that many Android devices
  have this function to support commands like "reboot recovery",
"reboot fastboot".
  Why should we moving away from that?
> So unless you convince uboot people that the _underlying special
> functionality_ behind these flags should be part of uboot, I don't think this
> is going to fly.
>
>
> In a way this is similar to gpu kernel code talking to proprietary userspace
> libs - these are also not eligible for the kernel. (meaning stuff like the
> mali kernel driver not being allowed).
>
> [...]
>
>> +static int rockchip_reboot_notify(struct notifier_block *this,
>> + unsigned long mode, void *cmd)
>> +{
>> + u32 flag;
>> +
>> + rockchip_get_reboot_flag(cmd, &flag);
>> + regmap_write(regmap, flag_reg, flag);
>> +
>> + return NOTIFY_DONE;
>> +}
>> +
>> +static struct notifier_block rockchip_reboot_handler = {
>> + .notifier_call = rockchip_reboot_notify,
>> + .priority = 150,
>> +};
> the restart handlers are meant to really only restart the system, not to
> execute some actions before the restart happens.
>
> Seehttps://lkml.org/lkml/2015/6/3/707 for a similar case.
>
So maybe I can use reboot notifier here?
Thank you.
> Heiko
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Simon Glass <sjg@chromium.org> |
|---|---|
| Date | 2015-09-09 20:10 +0200 |
| Subject | Re: set rockchip-specific uboot bootmode flags on reboot (was: [PATCH] ARM: rockchip: add reboot notifier) |
| Message-ID | <q6RYK-2ez-3@gated-at.bofh.it> |
| In reply to | #1221127 |
Hi,
On 8 September 2015 at 16:46, Heiko Stübner <heiko@sntech.de> wrote:
>
> Hi Andy,
>
> Am Dienstag, 8. September 2015, 20:43:07 schrieb Andy Yan:
> > rockchip platform have a protocol to pass the the kernel
> > reboot mode to bootloader by some special registers when
> > system reboot.By this way the bootloader can take different
> > action according to the different kernel reboot mode, for
> > example, command "reboot loader" will reboot the board to
> > rockusb mode, this is a very convenient way to get the board
> > to download mode.
> >
> > Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
>
> [...]
>
> > @@ -0,0 +1,22 @@
> > +#ifndef __MACH_ROCKCHIP_LOADER_H
> > +#define __MACH_ROCKCHIP_LOADER_H
> > +
> > +/*high 24 bits is tag, low 8 bits is type*/
> > +#define SYS_LOADER_REBOOT_FLAG 0x5242C300
> > +
> > +enum {
> > + BOOT_NORMAL = 0, /* normal boot */
> > + BOOT_LOADER, /* enter loader rockusb mode */
> > + BOOT_MASKROM, /* enter maskrom rockusb mode (not support now) */
> > + BOOT_RECOVER, /* enter recover */
> > + BOOT_NORECOVER, /* do not enter recover */
> > + BOOT_SECONDOS, /* boot second OS (not support now)*/
> > + BOOT_WIPEDATA, /* enter recover and wipe data. */
> > + BOOT_WIPEALL, /* enter recover and wipe all data. */
> > + BOOT_CHECKIMG, /* check firmware img with backup part*/
> > + BOOT_FASTBOOT, /* enter fast boot mode */
> > + BOOT_SECUREBOOT_DISABLE,
> > + BOOT_CHARGING, /* enter charge mode */
> > + BOOT_MAX /* MAX VALID BOOT TYPE.*/
> > +};
> > +#endif
>
> These flags rely on code in the bootloader to actually handle the target
> action. Nowadays this is uboot, but still a rockchip-specific fork. And we're
> actively moving away from that, with the recent rk3288 addition to mainline
> uboot.
>
> So unless you convince uboot people that the _underlying special
> functionality_ behind these flags should be part of uboot, I don't think this
> is going to fly.
>
>
> In a way this is similar to gpu kernel code talking to proprietary userspace
> libs - these are also not eligible for the kernel. (meaning stuff like the
> mali kernel driver not being allowed).
I don't want to comment on what Linux does or does not want. But I can
see this sort of feature being useful for devs at least. So long as it
is defined in a way that is not Rockchip-specific (and the above enum
looks pretty reasonable on that front, I think it makes sense.
Of course it's a bit odd to target a downstream U-Boot with a Linux
feature. But hopefully Rockchip's U-Boot support and development will
move to mainline with time.
>
> [...]
>
> > +static int rockchip_reboot_notify(struct notifier_block *this,
> > + unsigned long mode, void *cmd)
> > +{
> > + u32 flag;
> > +
> > + rockchip_get_reboot_flag(cmd, &flag);
> > + regmap_write(regmap, flag_reg, flag);
> > +
> > + return NOTIFY_DONE;
> > +}
> > +
> > +static struct notifier_block rockchip_reboot_handler = {
> > + .notifier_call = rockchip_reboot_notify,
> > + .priority = 150,
> > +};
>
> the restart handlers are meant to really only restart the system, not to
> execute some actions before the restart happens.
>
> See https://lkml.org/lkml/2015/6/3/707 for a similar case.
>
>
> Heiko
Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Andy Yan <andy.yan@rock-chips.com> |
|---|---|
| Date | 2015-09-17 13:10 +0200 |
| Subject | Re: set rockchip-specific uboot bootmode flags on reboot |
| Message-ID | <q9FeH-1Hx-33@gated-at.bofh.it> |
| In reply to | #1221600 |
Hi Heiko:
On 2015年09月10日 02:05, Simon Glass wrote:
> Hi,
>
> On 8 September 2015 at 16:46, Heiko Stübner <heiko@sntech.de> wrote:
>> Hi Andy,
>>
>> Am Dienstag, 8. September 2015, 20:43:07 schrieb Andy Yan:
>>> rockchip platform have a protocol to pass the the kernel
>>> reboot mode to bootloader by some special registers when
>>> system reboot.By this way the bootloader can take different
>>> action according to the different kernel reboot mode, for
>>> example, command "reboot loader" will reboot the board to
>>> rockusb mode, this is a very convenient way to get the board
>>> to download mode.
>>>
>>> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
>> [...]
>>
>>> @@ -0,0 +1,22 @@
>>> +#ifndef __MACH_ROCKCHIP_LOADER_H
>>> +#define __MACH_ROCKCHIP_LOADER_H
>>> +
>>> +/*high 24 bits is tag, low 8 bits is type*/
>>> +#define SYS_LOADER_REBOOT_FLAG 0x5242C300
>>> +
>>> +enum {
>>> + BOOT_NORMAL = 0, /* normal boot */
>>> + BOOT_LOADER, /* enter loader rockusb mode */
>>> + BOOT_MASKROM, /* enter maskrom rockusb mode (not support now) */
>>> + BOOT_RECOVER, /* enter recover */
>>> + BOOT_NORECOVER, /* do not enter recover */
>>> + BOOT_SECONDOS, /* boot second OS (not support now)*/
>>> + BOOT_WIPEDATA, /* enter recover and wipe data. */
>>> + BOOT_WIPEALL, /* enter recover and wipe all data. */
>>> + BOOT_CHECKIMG, /* check firmware img with backup part*/
>>> + BOOT_FASTBOOT, /* enter fast boot mode */
>>> + BOOT_SECUREBOOT_DISABLE,
>>> + BOOT_CHARGING, /* enter charge mode */
>>> + BOOT_MAX /* MAX VALID BOOT TYPE.*/
>>> +};
>>> +#endif
>> These flags rely on code in the bootloader to actually handle the target
>> action. Nowadays this is uboot, but still a rockchip-specific fork. And we're
>> actively moving away from that, with the recent rk3288 addition to mainline
>> uboot.
>>
>> So unless you convince uboot people that the _underlying special
>> functionality_ behind these flags should be part of uboot, I don't think this
>> is going to fly.
>>
>>
>> In a way this is similar to gpu kernel code talking to proprietary userspace
>> libs - these are also not eligible for the kernel. (meaning stuff like the
>> mali kernel driver not being allowed).
> I don't want to comment on what Linux does or does not want. But I can
> see this sort of feature being useful for devs at least. So long as it
> is defined in a way that is not Rockchip-specific (and the above enum
> looks pretty reasonable on that front, I think it makes sense.
>
> Of course it's a bit odd to target a downstream U-Boot with a Linux
> feature. But hopefully Rockchip's U-Boot support and development will
> move to mainline with time.
Is there any chance for this patch to be landed?
As Simon says, it is useful for development. And
he is upstreaming Rockchip U-boot.
>> [...]
>>
>>> +static int rockchip_reboot_notify(struct notifier_block *this,
>>> + unsigned long mode, void *cmd)
>>> +{
>>> + u32 flag;
>>> +
>>> + rockchip_get_reboot_flag(cmd, &flag);
>>> + regmap_write(regmap, flag_reg, flag);
>>> +
>>> + return NOTIFY_DONE;
>>> +}
>>> +
>>> +static struct notifier_block rockchip_reboot_handler = {
>>> + .notifier_call = rockchip_reboot_notify,
>>> + .priority = 150,
>>> +};
>> the restart handlers are meant to really only restart the system, not to
>> execute some actions before the restart happens.
>>
>> See https://lkml.org/lkml/2015/6/3/707 for a similar case.
>>
>>
>> Heiko
> Regards,
> Simon
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web