[parent not found: <1470265523-27557-1-git-send-email-john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]
* [RFC][PATCH 1/4] drivers: sram: Have sram driver probe children nodes
[not found] ` <1470265523-27557-1-git-send-email-john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2016-08-03 23:05 ` John Stultz
2016-08-03 23:05 ` [RFC][PATCH 2/4] dt-bindings: power: reset: Add document for sram-reboot-mode driver John Stultz
1 sibling, 0 replies; 14+ messages in thread
From: John Stultz @ 2016-08-03 23:05 UTC (permalink / raw)
To: lkml
Cc: John Stultz, Andy Yan, Rob Herring, Arnd Bergmann, Thierry Reding,
Heiko Stübner, Caesar Wang, Kees Cook, Guodong Xu,
Haojian Zhuang, Vishal Bhoj, Bjorn Andersson,
devicetree-u79uwXL29TY76Z2rM5mHXA, Android Kernel Team
In order to support sub-nodes with the sram driver,
have the sram driver call of_probe_default_children().
This will allow for supportting sram based reboot reasons.
Cc: Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Cc: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Cc: Heiko Stübner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
Cc: Caesar Wang <wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Cc: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Guodong Xu <guodong.xu-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Haojian Zhuang <haojian.zhuang-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Vishal Bhoj <vishal.bhoj-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Android Kernel Team <kernel-team-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: John Stultz <john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
drivers/misc/sram.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index f84b53d..6830a79 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -23,6 +23,7 @@
#include <linux/io.h>
#include <linux/list_sort.h>
#include <linux/of_address.h>
+#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
@@ -384,6 +385,8 @@ static int sram_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, sram);
+ of_platform_default_populate(pdev->dev.of_node,
+ NULL, &pdev->dev);
dev_dbg(sram->dev, "SRAM pool: %zu KiB @ 0x%p\n",
gen_pool_size(sram->pool) / 1024, sram->virt_base);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC][PATCH 2/4] dt-bindings: power: reset: Add document for sram-reboot-mode driver
[not found] ` <1470265523-27557-1-git-send-email-john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2016-08-03 23:05 ` [RFC][PATCH 1/4] drivers: sram: Have sram driver probe children nodes John Stultz
@ 2016-08-03 23:05 ` John Stultz
[not found] ` <1470265523-27557-3-git-send-email-john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
1 sibling, 1 reply; 14+ messages in thread
From: John Stultz @ 2016-08-03 23:05 UTC (permalink / raw)
To: lkml
Cc: John Stultz, Andy Yan, Rob Herring, Arnd Bergmann, Thierry Reding,
Heiko Stübner, Caesar Wang, Kees Cook, Guodong Xu,
Haojian Zhuang, Vishal Bhoj, Bjorn Andersson,
devicetree-u79uwXL29TY76Z2rM5mHXA, Android Kernel Team
Add device tree binding document for reboot-mode driver
Cc: Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Cc: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Cc: Heiko Stübner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
Cc: Caesar Wang <wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Cc: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Guodong Xu <guodong.xu-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Haojian Zhuang <haojian.zhuang-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Vishal Bhoj <vishal.bhoj-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Android Kernel Team <kernel-team-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: John Stultz <john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
.../bindings/power/reset/sram-reboot-mode.txt | 35 ++++++++++++++++++++++
1 file changed, 35 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/reset/sram-reboot-mode.txt
diff --git a/Documentation/devicetree/bindings/power/reset/sram-reboot-mode.txt b/Documentation/devicetree/bindings/power/reset/sram-reboot-mode.txt
new file mode 100644
index 0000000..0a0ed05
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/sram-reboot-mode.txt
@@ -0,0 +1,35 @@
+SRAM reboot mode driver
+
+This driver get reboot mode magic value form reboot-mode driver
+and stores it in a SRAM address. Then the bootloader
+can read it and take different action according to the magic
+value stored.
+
+This DT node should be represented as a sub-node of a "mmio-sram" node.
+
+Required properties:
+- compatible: should be "sram-reboot-mode"
+- reg: offset from the sram range where to store the magic value(in bytes)
+
+The rest of the properties should follow the generic reboot-mode discription
+found in reboot-mode.txt
+
+Example:
+
+ sram@5f01000 {
+ compatible = "mmio-sram";
+ reg = <0x0 0x05f01000 0x0 0x00001000>;
+ ranges = <0x0 0x0 0x05f01000 0x00001000>;
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ reboot-mode@0 {
+ compatible = "sram-reboot-mode";
+ reg = <0x0 0x4>;
+
+ mode-normal = <0x77665501>;
+ mode-bootloader = <0x77665500>;
+ mode-recovery = <0x77665502>;
+ };
+ };
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RFC][PATCH 3/4] power: reset: Add sram-reboot-mode driver
2016-08-03 23:05 [RFC][PATCH 0/4] SRAM based reboot reason driver for HiKey John Stultz
[not found] ` <1470265523-27557-1-git-send-email-john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2016-08-03 23:05 ` John Stultz
[not found] ` <1470265523-27557-4-git-send-email-john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2016-08-05 23:23 ` Paul Gortmaker
2016-08-03 23:05 ` [RFC][PATCH 4/4] dts: hikey: Add hikey support for sram-reboot-mode John Stultz
2016-08-05 12:46 ` [RFC][PATCH 0/4] SRAM based reboot reason driver for HiKey Vladimir Zapolskiy
3 siblings, 2 replies; 14+ messages in thread
From: John Stultz @ 2016-08-03 23:05 UTC (permalink / raw)
To: lkml
Cc: John Stultz, Andy Yan, Rob Herring, Arnd Bergmann, Thierry Reding,
Heiko Stübner, Caesar Wang, Kees Cook, Guodong Xu,
Haojian Zhuang, Vishal Bhoj, Bjorn Andersson, devicetree,
Android Kernel Team
Add sram-reboot-mode driver, which enables
reboot modes to be specified from sram subnodes.
Cc: Andy Yan <andy.yan@rock-chips.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Heiko Stübner <heiko@sntech.de>
Cc: Caesar Wang <wxt@rock-chips.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
Cc: Vishal Bhoj <vishal.bhoj@linaro.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: devicetree@vger.kernel.org
Cc: Android Kernel Team <kernel-team@android.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
drivers/power/reset/Kconfig | 10 ++++
drivers/power/reset/Makefile | 1 +
drivers/power/reset/sram-reboot-mode.c | 95 ++++++++++++++++++++++++++++++++++
3 files changed, 106 insertions(+)
create mode 100644 drivers/power/reset/sram-reboot-mode.c
diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index 3bfac53..af553ed 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -208,5 +208,15 @@ config SYSCON_REBOOT_MODE
register, then the bootloader can read it to take different
action according to the mode.
+config SRAM_REBOOT_MODE
+ bool "Generic SRAM reboot mode driver"
+ select REBOOT_MODE
+ select SRAM
+ help
+ Say y here will enable reboot mode driver. This will
+ get reboot mode arguments and store it in an SRAM
+ address, then the bootloader can read it to take different
+ action according to the mode.
+
endif
diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
index 1be307c..14f23ad 100644
--- a/drivers/power/reset/Makefile
+++ b/drivers/power/reset/Makefile
@@ -24,3 +24,4 @@ obj-$(CONFIG_POWER_RESET_RMOBILE) += rmobile-reset.o
obj-$(CONFIG_POWER_RESET_ZX) += zx-reboot.o
obj-$(CONFIG_REBOOT_MODE) += reboot-mode.o
obj-$(CONFIG_SYSCON_REBOOT_MODE) += syscon-reboot-mode.o
+obj-$(CONFIG_SRAM_REBOOT_MODE) += sram-reboot-mode.o
diff --git a/drivers/power/reset/sram-reboot-mode.c b/drivers/power/reset/sram-reboot-mode.c
new file mode 100644
index 0000000..8945dac
--- /dev/null
+++ b/drivers/power/reset/sram-reboot-mode.c
@@ -0,0 +1,95 @@
+/*
+ * Copyright (c) 2016, Linaro Limited
+ * Based on syscon-reboot-mode.c
+ * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/reboot.h>
+#include <linux/regmap.h>
+#include "reboot-mode.h"
+
+
+
+struct sram_reboot_mode {
+ struct reboot_mode_driver reboot;
+ void __iomem *reboot_reason_val_addr;
+};
+
+static int sram_reboot_mode_write(struct reboot_mode_driver *reboot,
+ unsigned int magic)
+{
+ struct sram_reboot_mode *sram_rbm;
+
+ sram_rbm = container_of(reboot, struct sram_reboot_mode, reboot);
+
+ writel(magic, sram_rbm->reboot_reason_val_addr);
+ return 0;
+}
+
+static int sram_reboot_mode_probe(struct platform_device *pdev)
+{
+ struct sram_reboot_mode *sram_rbm;
+ struct resource *res;
+ int ret;
+
+ sram_rbm = devm_kzalloc(&pdev->dev, sizeof(*sram_rbm), GFP_KERNEL);
+ if (!sram_rbm)
+ return -ENOMEM;
+
+ sram_rbm->reboot.dev = &pdev->dev;
+ sram_rbm->reboot.write = sram_reboot_mode_write;
+
+ dev_set_drvdata(&pdev->dev, sram_rbm);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return PTR_ERR(res);
+
+ sram_rbm->reboot_reason_val_addr = devm_ioremap(&pdev->dev, res->start,
+ resource_size(res));
+ if (IS_ERR(sram_rbm->reboot_reason_val_addr))
+ return PTR_ERR(sram_rbm->reboot_reason_val_addr);
+
+ ret = reboot_mode_register(&sram_rbm->reboot);
+ if (ret)
+ dev_err(&pdev->dev, "can't register reboot mode\n");
+
+ return ret;
+}
+
+static int sram_reboot_mode_remove(struct platform_device *pdev)
+{
+ struct sram_reboot_mode *sram_rbm = dev_get_drvdata(&pdev->dev);
+
+ return reboot_mode_unregister(&sram_rbm->reboot);
+}
+
+static const struct of_device_id sram_reboot_mode_of_match[] = {
+ { .compatible = "sram-reboot-mode" },
+ {}
+};
+
+static struct platform_driver sram_reboot_mode_driver = {
+ .probe = sram_reboot_mode_probe,
+ .remove = sram_reboot_mode_remove,
+ .driver = {
+ .name = "sram-reboot-mode",
+ .of_match_table = sram_reboot_mode_of_match,
+ },
+};
+module_platform_driver(sram_reboot_mode_driver);
+
+MODULE_AUTHOR("John Stultz <john.stultz@linaro.org>");
+MODULE_DESCRIPTION("SRAM reboot mode driver");
+MODULE_LICENSE("GPL v2");
--
1.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread[parent not found: <1470265523-27557-4-git-send-email-john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]
* Re: [RFC][PATCH 3/4] power: reset: Add sram-reboot-mode driver
[not found] ` <1470265523-27557-4-git-send-email-john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2016-08-04 1:03 ` Bjorn Andersson
2016-08-04 3:08 ` John Stultz
0 siblings, 1 reply; 14+ messages in thread
From: Bjorn Andersson @ 2016-08-04 1:03 UTC (permalink / raw)
To: John Stultz
Cc: lkml, Andy Yan, Rob Herring, Arnd Bergmann, Thierry Reding,
Heiko St?bner, Caesar Wang, Kees Cook, Guodong Xu, Haojian Zhuang,
Vishal Bhoj, devicetree-u79uwXL29TY76Z2rM5mHXA,
Android Kernel Team
On Wed 03 Aug 16:05 PDT 2016, John Stultz wrote:
[..]
> diff --git a/drivers/power/reset/sram-reboot-mode.c b/drivers/power/reset/sram-reboot-mode.c
[..]
> +
> +struct sram_reboot_mode {
> + struct reboot_mode_driver reboot;
> + void __iomem *reboot_reason_val_addr;
23 characters is...a lot of characters...
> +};
> +
> +static int sram_reboot_mode_write(struct reboot_mode_driver *reboot,
> + unsigned int magic)
> +{
> + struct sram_reboot_mode *sram_rbm;
> +
> + sram_rbm = container_of(reboot, struct sram_reboot_mode, reboot);
> +
> + writel(magic, sram_rbm->reboot_reason_val_addr);
> + return 0;
> +}
> +
> +static int sram_reboot_mode_probe(struct platform_device *pdev)
> +{
> + struct sram_reboot_mode *sram_rbm;
> + struct resource *res;
> + int ret;
> +
> + sram_rbm = devm_kzalloc(&pdev->dev, sizeof(*sram_rbm), GFP_KERNEL);
> + if (!sram_rbm)
> + return -ENOMEM;
> +
> + sram_rbm->reboot.dev = &pdev->dev;
> + sram_rbm->reboot.write = sram_reboot_mode_write;
> +
> + dev_set_drvdata(&pdev->dev, sram_rbm);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return PTR_ERR(res);
> +
> + sram_rbm->reboot_reason_val_addr = devm_ioremap(&pdev->dev, res->start,
> + resource_size(res));
Use devm_ioremap_resource() instead, it saves you the return value check
on platform_get_resource() and is cleaner.
> + if (IS_ERR(sram_rbm->reboot_reason_val_addr))
> + return PTR_ERR(sram_rbm->reboot_reason_val_addr);
> +
> + ret = reboot_mode_register(&sram_rbm->reboot);
I think you should take the time to throw in a
devm_reboot_mode_register(), it would save you from the
dev_set_drvdata() and you can drop the remove function.
> + if (ret)
> + dev_err(&pdev->dev, "can't register reboot mode\n");
> +
> + return ret;
> +}
> +
Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC][PATCH 3/4] power: reset: Add sram-reboot-mode driver
2016-08-04 1:03 ` Bjorn Andersson
@ 2016-08-04 3:08 ` John Stultz
2016-08-04 5:29 ` Bjorn Andersson
0 siblings, 1 reply; 14+ messages in thread
From: John Stultz @ 2016-08-04 3:08 UTC (permalink / raw)
To: Bjorn Andersson
Cc: lkml, Andy Yan, Rob Herring, Arnd Bergmann, Thierry Reding,
Heiko St?bner, Caesar Wang, Kees Cook, Guodong Xu, Haojian Zhuang,
Vishal Bhoj, devicetree, Android Kernel Team
On Wed, Aug 3, 2016 at 6:03 PM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Wed 03 Aug 16:05 PDT 2016, John Stultz wrote:
>
> [..]
>> diff --git a/drivers/power/reset/sram-reboot-mode.c b/drivers/power/reset/sram-reboot-mode.c
> [..]
>> +
>> +struct sram_reboot_mode {
>> + struct reboot_mode_driver reboot;
>> + void __iomem *reboot_reason_val_addr;
>
> 23 characters is...a lot of characters...
Fair enough. Renamed to reason_addr.
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + if (!res)
>> + return PTR_ERR(res);
>> +
>> + sram_rbm->reboot_reason_val_addr = devm_ioremap(&pdev->dev, res->start,
>> + resource_size(res));
>
> Use devm_ioremap_resource() instead, it saves you the return value check
> on platform_get_resource() and is cleaner.
Ok. Done.
>> + if (IS_ERR(sram_rbm->reboot_reason_val_addr))
>> + return PTR_ERR(sram_rbm->reboot_reason_val_addr);
>> +
>> + ret = reboot_mode_register(&sram_rbm->reboot);
>
> I think you should take the time to throw in a
> devm_reboot_mode_register(), it would save you from the
> dev_set_drvdata() and you can drop the remove function.
So I've only got a vague sense of what your suggesting here. Do you
have a pointer to a good example?
thanks
-john
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC][PATCH 3/4] power: reset: Add sram-reboot-mode driver
2016-08-04 3:08 ` John Stultz
@ 2016-08-04 5:29 ` Bjorn Andersson
0 siblings, 0 replies; 14+ messages in thread
From: Bjorn Andersson @ 2016-08-04 5:29 UTC (permalink / raw)
To: John Stultz
Cc: lkml, Andy Yan, Rob Herring, Arnd Bergmann, Thierry Reding,
Heiko St?bner, Caesar Wang, Kees Cook, Guodong Xu, Haojian Zhuang,
Vishal Bhoj, devicetree, Android Kernel Team
On Wed 03 Aug 20:08 PDT 2016, John Stultz wrote:
> On Wed, Aug 3, 2016 at 6:03 PM, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> > On Wed 03 Aug 16:05 PDT 2016, John Stultz wrote:
> >
> > [..]
> >> diff --git a/drivers/power/reset/sram-reboot-mode.c b/drivers/power/reset/sram-reboot-mode.c
[..]
> >> + ret = reboot_mode_register(&sram_rbm->reboot);
> >
> > I think you should take the time to throw in a
> > devm_reboot_mode_register(), it would save you from the
> > dev_set_drvdata() and you can drop the remove function.
>
> So I've only got a vague sense of what your suggesting here. Do you
> have a pointer to a good example?
>
https://patchwork.kernel.org/patch/9262691/
https://patchwork.kernel.org/patch/9262693/
Regards,
Bjorn
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC][PATCH 3/4] power: reset: Add sram-reboot-mode driver
2016-08-03 23:05 ` [RFC][PATCH 3/4] power: reset: Add " John Stultz
[not found] ` <1470265523-27557-4-git-send-email-john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2016-08-05 23:23 ` Paul Gortmaker
1 sibling, 0 replies; 14+ messages in thread
From: Paul Gortmaker @ 2016-08-05 23:23 UTC (permalink / raw)
To: John Stultz
Cc: lkml, Andy Yan, Rob Herring, Arnd Bergmann, Thierry Reding,
Heiko Stübner, Caesar Wang, Kees Cook, Guodong Xu,
Haojian Zhuang, Vishal Bhoj, Bjorn Andersson, devicetree,
Android Kernel Team
On Wed, Aug 3, 2016 at 7:05 PM, John Stultz <john.stultz@linaro.org> wrote:
> Add sram-reboot-mode driver, which enables
> reboot modes to be specified from sram subnodes.
>
> Cc: Andy Yan <andy.yan@rock-chips.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Heiko Stübner <heiko@sntech.de>
> Cc: Caesar Wang <wxt@rock-chips.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
> Cc: Vishal Bhoj <vishal.bhoj@linaro.org>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: devicetree@vger.kernel.org
> Cc: Android Kernel Team <kernel-team@android.com>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> drivers/power/reset/Kconfig | 10 ++++
> drivers/power/reset/Makefile | 1 +
> drivers/power/reset/sram-reboot-mode.c | 95 ++++++++++++++++++++++++++++++++++
> 3 files changed, 106 insertions(+)
> create mode 100644 drivers/power/reset/sram-reboot-mode.c
>
> diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
> index 3bfac53..af553ed 100644
> --- a/drivers/power/reset/Kconfig
> +++ b/drivers/power/reset/Kconfig
> @@ -208,5 +208,15 @@ config SYSCON_REBOOT_MODE
> register, then the bootloader can read it to take different
> action according to the mode.
>
> +config SRAM_REBOOT_MODE
> + bool "Generic SRAM reboot mode driver"
Since this is bool, can we dump the module.h and any MODULE_*
tags from the driver, and register using a builtin variant?
Thanks,
Paul.
--
> + select REBOOT_MODE
> + select SRAM
> + help
> + Say y here will enable reboot mode driver. This will
> + get reboot mode arguments and store it in an SRAM
> + address, then the bootloader can read it to take different
> + action according to the mode.
> +
> endif
>
> diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
> index 1be307c..14f23ad 100644
> --- a/drivers/power/reset/Makefile
> +++ b/drivers/power/reset/Makefile
> @@ -24,3 +24,4 @@ obj-$(CONFIG_POWER_RESET_RMOBILE) += rmobile-reset.o
> obj-$(CONFIG_POWER_RESET_ZX) += zx-reboot.o
> obj-$(CONFIG_REBOOT_MODE) += reboot-mode.o
> obj-$(CONFIG_SYSCON_REBOOT_MODE) += syscon-reboot-mode.o
> +obj-$(CONFIG_SRAM_REBOOT_MODE) += sram-reboot-mode.o
> diff --git a/drivers/power/reset/sram-reboot-mode.c b/drivers/power/reset/sram-reboot-mode.c
> new file mode 100644
> index 0000000..8945dac
> --- /dev/null
> +++ b/drivers/power/reset/sram-reboot-mode.c
> @@ -0,0 +1,95 @@
> +/*
> + * Copyright (c) 2016, Linaro Limited
> + * Based on syscon-reboot-mode.c
> + * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/reboot.h>
> +#include <linux/regmap.h>
> +#include "reboot-mode.h"
> +
^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFC][PATCH 4/4] dts: hikey: Add hikey support for sram-reboot-mode
2016-08-03 23:05 [RFC][PATCH 0/4] SRAM based reboot reason driver for HiKey John Stultz
[not found] ` <1470265523-27557-1-git-send-email-john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2016-08-03 23:05 ` [RFC][PATCH 3/4] power: reset: Add " John Stultz
@ 2016-08-03 23:05 ` John Stultz
2016-08-05 12:46 ` [RFC][PATCH 0/4] SRAM based reboot reason driver for HiKey Vladimir Zapolskiy
3 siblings, 0 replies; 14+ messages in thread
From: John Stultz @ 2016-08-03 23:05 UTC (permalink / raw)
To: lkml
Cc: John Stultz, Andy Yan, Rob Herring, Arnd Bergmann, Thierry Reding,
Heiko Stübner, Caesar Wang, Kees Cook, Guodong Xu,
Haojian Zhuang, Vishal Bhoj, Bjorn Andersson, devicetree,
Android Kernel Team
Add support to hikey dts and defconfig for the
sram-reboot-mode driver.
The dts entries added here should really be generated
by the UEFI firmware, and not be static in the dts,
since one may be using different firmware on HiKey.
But this patch provides an example of how the
sram-reboot-mode entry would otherwise look.
Cc: Andy Yan <andy.yan@rock-chips.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Heiko Stübner <heiko@sntech.de>
Cc: Caesar Wang <wxt@rock-chips.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
Cc: Vishal Bhoj <vishal.bhoj@linaro.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: devicetree@vger.kernel.org
Cc: Android Kernel Team <kernel-team@android.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
index 593c7e4..62326ab 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
@@ -29,6 +29,7 @@
* Reserve below regions from memory node:
*
* 0x05e0,0000 - 0x05ef,ffff: MCU firmware runtime using
+ * 0x05f0,1000 - 0x05f0,1fff: Reboot reason
* 0x06df,f000 - 0x06df,ffff: Mailbox message data
* 0x0740,f000 - 0x0740,ffff: MCU firmware section
* 0x3e00,0000 - 0x3fff,ffff: OP-TEE
@@ -36,11 +37,30 @@
memory@0 {
device_type = "memory";
reg = <0x00000000 0x00000000 0x00000000 0x05e00000>,
- <0x00000000 0x05f00000 0x00000000 0x00eff000>,
+ <0x00000000 0x05f00000 0x00000000 0x00001000>,
+ <0x00000000 0x05f02000 0x00000000 0x00efd000>,
<0x00000000 0x06e00000 0x00000000 0x0060f000>,
<0x00000000 0x07410000 0x00000000 0x36bf0000>;
};
+ sram@5f01000 {
+ compatible = "mmio-sram";
+ reg = <0x0 0x05f01000 0x0 0x00001000>;
+ ranges = <0x0 0x0 0x05f01000 0x00001000>;
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ reboot-mode@0 {
+ compatible = "sram-reboot-mode";
+ reg = <0x0 0x4>;
+
+ mode-normal = <0x77665501>;
+ mode-bootloader = <0x77665500>;
+ mode-recovery = <0x77665502>;
+ };
+ };
+
soc {
spi0: spi@f7106000 {
status = "ok";
--
1.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [RFC][PATCH 0/4] SRAM based reboot reason driver for HiKey
2016-08-03 23:05 [RFC][PATCH 0/4] SRAM based reboot reason driver for HiKey John Stultz
` (2 preceding siblings ...)
2016-08-03 23:05 ` [RFC][PATCH 4/4] dts: hikey: Add hikey support for sram-reboot-mode John Stultz
@ 2016-08-05 12:46 ` Vladimir Zapolskiy
2016-08-05 22:37 ` Rob Herring
3 siblings, 1 reply; 14+ messages in thread
From: Vladimir Zapolskiy @ 2016-08-05 12:46 UTC (permalink / raw)
To: John Stultz, lkml
Cc: Andy Yan, Rob Herring, Arnd Bergmann, Thierry Reding,
Heiko Stübner, Caesar Wang, Kees Cook, Guodong Xu,
Haojian Zhuang, Vishal Bhoj, Bjorn Andersson, devicetree,
Android Kernel Team
Hi John,
On 08/04/2016 02:05 AM, John Stultz wrote:
> Now that Andy's reboot reason core driver has landed, I wanted
> to resubmit a reworked version of my SRAM based reboot reason
> driver.
>
> This allows the kernel to communicate to the bootloader what mode
> it should reboot to using some reserved memory.
>
> Feedback would be very much appreciated!
in my opinion the taken approach is wrong, and I've already explained
why and how to rework your driver to shrink the change, please see
https://lkml.org/lkml/2016/1/27/133
In this case I think that a SRAM device node should just contain
a plain description of partitions, compatible = "sram-reboot-mode" is
clearly not a device on "SRAM bus", it is not a device at all, so
please let's separate policy from mechanism
Because my proposed alternative approach separates policy from
mechanism, it for instanse allows to avoid overlappings on SRAM areas,
and still other drivers may serve as consumers of partitions on SRAM.
Please add me to Cc list when you send the next version of the driver.
With best wishes,
Vladimir
> thanks
> -john
>
> Cc: Andy Yan <andy.yan@rock-chips.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Heiko Stübner <heiko@sntech.de>
> Cc: Caesar Wang <wxt@rock-chips.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
> Cc: Vishal Bhoj <vishal.bhoj@linaro.org>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: devicetree@vger.kernel.org
> Cc: Android Kernel Team <kernel-team@android.com>
>
> John Stultz (4):
> drivers: sram: Have sram driver probe children nodes
> dt-bindings: power: reset: Add document for sram-reboot-mode driver
> power: reset: Add sram-reboot-mode driver
> dts: hikey: Add hikey support for sram-reboot-mode
>
> .../bindings/power/reset/sram-reboot-mode.txt | 35 ++++++++
> arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 22 ++++-
> drivers/misc/sram.c | 3 +
> drivers/power/reset/Kconfig | 10 +++
> drivers/power/reset/Makefile | 1 +
> drivers/power/reset/sram-reboot-mode.c | 95 ++++++++++++++++++++++
> 6 files changed, 165 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/power/reset/sram-reboot-mode.txt
> create mode 100644 drivers/power/reset/sram-reboot-mode.c
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC][PATCH 0/4] SRAM based reboot reason driver for HiKey
2016-08-05 12:46 ` [RFC][PATCH 0/4] SRAM based reboot reason driver for HiKey Vladimir Zapolskiy
@ 2016-08-05 22:37 ` Rob Herring
2016-08-05 22:51 ` John Stultz
[not found] ` <CAL_JsqLL01G_=b6tV_qsKmPCEJa2Fhp_zk5u9s1-eYdzfHjbxQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 2 replies; 14+ messages in thread
From: Rob Herring @ 2016-08-05 22:37 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: John Stultz, lkml, Andy Yan, Arnd Bergmann, Thierry Reding,
Heiko Stübner, Caesar Wang, Kees Cook, Guodong Xu,
Haojian Zhuang, Vishal Bhoj, Bjorn Andersson,
devicetree@vger.kernel.org, Android Kernel Team
On Fri, Aug 5, 2016 at 7:46 AM, Vladimir Zapolskiy
<vladimir_zapolskiy@mentor.com> wrote:
> Hi John,
>
> On 08/04/2016 02:05 AM, John Stultz wrote:
>>
>> Now that Andy's reboot reason core driver has landed, I wanted
>> to resubmit a reworked version of my SRAM based reboot reason
>> driver.
>>
>> This allows the kernel to communicate to the bootloader what mode
>> it should reboot to using some reserved memory.
>>
>> Feedback would be very much appreciated!
>
>
> in my opinion the taken approach is wrong, and I've already explained
> why and how to rework your driver to shrink the change, please see
> https://lkml.org/lkml/2016/1/27/133
>
> In this case I think that a SRAM device node should just contain
> a plain description of partitions, compatible = "sram-reboot-mode" is
> clearly not a device on "SRAM bus", it is not a device at all, so
> please let's separate policy from mechanism
Having a 2nd node for the driver is still not a device on a bus. It
adds unneeded complexity to the binding IMO.
The current approach also follows the model ramoops is using. Right
now it's using reserved-memory, but that could easily be extended to
SRAM region as well.
> Because my proposed alternative approach separates policy from
> mechanism, it for instanse allows to avoid overlappings on SRAM areas,
> and still other drivers may serve as consumers of partitions on SRAM.
You could still have multiple consumers and having a compatible string
doesn't necessarily imply a driver. Though multiple consumers without
something arbitrating access sounds like broken design to me.
Rob
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC][PATCH 0/4] SRAM based reboot reason driver for HiKey
2016-08-05 22:37 ` Rob Herring
@ 2016-08-05 22:51 ` John Stultz
[not found] ` <CAL_JsqLL01G_=b6tV_qsKmPCEJa2Fhp_zk5u9s1-eYdzfHjbxQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
1 sibling, 0 replies; 14+ messages in thread
From: John Stultz @ 2016-08-05 22:51 UTC (permalink / raw)
To: Rob Herring
Cc: Vladimir Zapolskiy, lkml, Andy Yan, Arnd Bergmann, Thierry Reding,
Heiko Stübner, Caesar Wang, Kees Cook, Guodong Xu,
Haojian Zhuang, Vishal Bhoj, Bjorn Andersson,
devicetree@vger.kernel.org, Android Kernel Team
On Fri, Aug 5, 2016 at 3:37 PM, Rob Herring <robh@kernel.org> wrote:
> On Fri, Aug 5, 2016 at 7:46 AM, Vladimir Zapolskiy
> <vladimir_zapolskiy@mentor.com> wrote:
>> Hi John,
>>
>> On 08/04/2016 02:05 AM, John Stultz wrote:
>>>
>>> Now that Andy's reboot reason core driver has landed, I wanted
>>> to resubmit a reworked version of my SRAM based reboot reason
>>> driver.
>>>
>>> This allows the kernel to communicate to the bootloader what mode
>>> it should reboot to using some reserved memory.
>>>
>>> Feedback would be very much appreciated!
>>
>>
>> in my opinion the taken approach is wrong, and I've already explained
>> why and how to rework your driver to shrink the change, please see
>> https://lkml.org/lkml/2016/1/27/133
>>
>> In this case I think that a SRAM device node should just contain
>> a plain description of partitions, compatible = "sram-reboot-mode" is
>> clearly not a device on "SRAM bus", it is not a device at all, so
>> please let's separate policy from mechanism
>
> Having a 2nd node for the driver is still not a device on a bus. It
> adds unneeded complexity to the binding IMO.
>
> The current approach also follows the model ramoops is using. Right
> now it's using reserved-memory, but that could easily be extended to
> SRAM region as well.
>
>> Because my proposed alternative approach separates policy from
>> mechanism, it for instanse allows to avoid overlappings on SRAM areas,
>> and still other drivers may serve as consumers of partitions on SRAM.
>
> You could still have multiple consumers and having a compatible string
> doesn't necessarily imply a driver. Though multiple consumers without
> something arbitrating access sounds like broken design to me.
So after running into some issues implementing the feedback that Bjorn
suggested, I realized we were going to need to not only extend the
sram driver to probe children, but we'd also have to make it a mfd so
it wouldn't reserve the entire range and the reboot reason driver
could map the memory.
That on top of the fact that we're already duplicating much of the
syscon-reboot-mode driver to work on sram, I decided to just start
over and use the syscon driver, which works fine here. All that is
needed is just adding it to the dts.
I know that its not exactly correct usage of the syscon driver, but it
starts to feel crazy almost completely duplicating the syscon driver
just to have it named sram.
thanks
-john
^ permalink raw reply [flat|nested] 14+ messages in thread[parent not found: <CAL_JsqLL01G_=b6tV_qsKmPCEJa2Fhp_zk5u9s1-eYdzfHjbxQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [RFC][PATCH 0/4] SRAM based reboot reason driver for HiKey
[not found] ` <CAL_JsqLL01G_=b6tV_qsKmPCEJa2Fhp_zk5u9s1-eYdzfHjbxQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2016-08-08 13:48 ` Vladimir Zapolskiy
0 siblings, 0 replies; 14+ messages in thread
From: Vladimir Zapolskiy @ 2016-08-08 13:48 UTC (permalink / raw)
To: Rob Herring
Cc: John Stultz, lkml, Andy Yan, Arnd Bergmann, Thierry Reding,
Heiko Stübner, Caesar Wang, Kees Cook, Guodong Xu,
Haojian Zhuang, Vishal Bhoj, Bjorn Andersson,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Android Kernel Team
Hi Rob,
On 08/06/2016 01:37 AM, Rob Herring wrote:
> On Fri, Aug 5, 2016 at 7:46 AM, Vladimir Zapolskiy
> <vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org> wrote:
>> Hi John,
>>
>> On 08/04/2016 02:05 AM, John Stultz wrote:
>>>
>>> Now that Andy's reboot reason core driver has landed, I wanted
>>> to resubmit a reworked version of my SRAM based reboot reason
>>> driver.
>>>
>>> This allows the kernel to communicate to the bootloader what mode
>>> it should reboot to using some reserved memory.
>>>
>>> Feedback would be very much appreciated!
>>
>>
>> in my opinion the taken approach is wrong, and I've already explained
>> why and how to rework your driver to shrink the change, please see
>> https://lkml.org/lkml/2016/1/27/133
>>
>> In this case I think that a SRAM device node should just contain
>> a plain description of partitions, compatible = "sram-reboot-mode" is
>> clearly not a device on "SRAM bus", it is not a device at all, so
>> please let's separate policy from mechanism
>
> Having a 2nd node for the driver is still not a device on a bus. It
> adds unneeded complexity to the binding IMO.
What second node for the driver do you mean here? If you reference
a reset/syscon driver then there should be only a property pointing
to a partition on SRAM, similar case is found in CODA driver, see
Documentation/devicetree/bindings/media/coda.txt and in my short term
plans to do the same for lpc-eth driver.
And a node which describes an area on SRAM is generally needed in both
cases, however note that with my approach techincally it is possible to
specify the entire SRAM device as a target partition, sometimes it is
sufficient but here it is not wanted, because there will be no control
on offset/size of the particular data stored on SRAM. The essential
part is the meaning of this added second node, either it is a reserved
partition (= unified definition independently on consumers) or
a description with a compatible property for some arbitrary device.
Why zoo of compatibles under SRAM node should be accepted? Why SRAM
should be converted to a bus type device? Should be the same done
with e.g. MTD or NVMEM devices? IMHO clear separation between data
proiders and data consumers should be preserved if possible, and here
it appears to be a simpler solution for the given technical problem.
> The current approach also follows the model ramoops is using. Right
> now it's using reserved-memory, but that could easily be extended to
> SRAM region as well.
>
>> Because my proposed alternative approach separates policy from
>> mechanism, it for instanse allows to avoid overlappings on SRAM areas,
>> and still other drivers may serve as consumers of partitions on SRAM.
>
> You could still have multiple consumers and having a compatible string
> doesn't necessarily imply a driver. Though multiple consumers without
> something arbitrating access sounds like broken design to me.
>
Not in this case, the interface to SRAM partitions and/or SRAM as
a whole deliberately assumes that a memory area is shared among all
consumers in sense of a memory pool, data resides within the
given area but it is not inter-shared among consumers.
--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 14+ messages in thread