linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V4 0/5] Add X-Gene platform reboot mechanism
@ 2014-01-23 19:19 Feng Kan
  2014-01-23 19:19 ` [PATCH V4 1/5] power: reset: Add generic SYSCON register mapped reset Feng Kan
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Feng Kan @ 2014-01-23 19:19 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, devicetree; +Cc: Feng Kan

Enable reboot driver for the X-Gene platform. Add generic syscon reboot
driver.

V4 Change:
	- Remove old X-Gene reboot driver
	- Add generic syscon reboot driver
	- Add DTS and Kconfig for X-Gene reboot using syscon method

V3 Change:
	- Remove the reboot driver's use of acpi resource patch.
	- Change the reboot driver to use syscon to parse out 
	  system clock register. Remove the old method of getting
	  register from the reboot driver directly.
	- Remove documentation since its now simple.
V2 Change:
	- Add support for using ACPI resource.


Feng Kan (5):
  power: reset: Add generic SYSCON register mapped reset
  power: reset: Remove X-Gene reboot driver
  arm64: dts: Add X-Gene reboot driver dts node
  arm64: Select reboot driver for X-Gene platform
  Documentation: power: reset: Add documentation for generic SYSCON
    reboot driver

 .../bindings/power/reset/syscon-reboot.txt         |   16 +++
 arch/arm64/Kconfig                                 |    2 +
 arch/arm64/boot/dts/apm-storm.dtsi                 |   13 +++
 drivers/power/reset/Kconfig                        |    8 +-
 drivers/power/reset/Makefile                       |    2 +-
 drivers/power/reset/syscon-reboot.c                |  100 +++++++++++++++++++
 drivers/power/reset/xgene-reboot.c                 |  103 --------------------
 7 files changed, 136 insertions(+), 108 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
 create mode 100644 drivers/power/reset/syscon-reboot.c
 delete mode 100644 drivers/power/reset/xgene-reboot.c

-- 
1.7.6.1


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

* [PATCH V4 1/5] power: reset: Add generic SYSCON register mapped reset
  2014-01-23 19:19 [PATCH V4 0/5] Add X-Gene platform reboot mechanism Feng Kan
@ 2014-01-23 19:19 ` Feng Kan
  2014-01-23 19:19 ` [PATCH V4 2/5] power: reset: Remove X-Gene reboot driver Feng Kan
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: Feng Kan @ 2014-01-23 19:19 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, devicetree; +Cc: Feng Kan

Add a generic SYSCON register mapped reset mechanism.

Signed-off-by: Feng Kan <fkan@apm.com>
---
 drivers/power/reset/Kconfig         |    7 +++
 drivers/power/reset/Makefile        |    1 +
 drivers/power/reset/syscon-reboot.c |  100 +++++++++++++++++++++++++++++++++++
 3 files changed, 108 insertions(+), 0 deletions(-)
 create mode 100644 drivers/power/reset/syscon-reboot.c

diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index 9b3ea53..4501c02 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -51,3 +51,10 @@ config POWER_RESET_XGENE
 	depends on POWER_RESET
 	help
 	  Reboot support for the APM SoC X-Gene Eval boards.
+
+config POWER_RESET_SYSCON
+	bool "Generic SYSCON regmap reset driver"
+	depends on MFD_SYSCON
+	depends on POWER_RESET
+	help
+	  Reboot support for generic SYSCON mapped register reset.
diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
index 3e6ed88..f2c0327 100644
--- a/drivers/power/reset/Makefile
+++ b/drivers/power/reset/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_POWER_RESET_QNAP) += qnap-poweroff.o
 obj-$(CONFIG_POWER_RESET_RESTART) += restart-poweroff.o
 obj-$(CONFIG_POWER_RESET_VEXPRESS) += vexpress-poweroff.o
 obj-$(CONFIG_POWER_RESET_XGENE) += xgene-reboot.o
+obj-$(CONFIG_POWER_RESET_SYSCON) += syscon-reboot.o
diff --git a/drivers/power/reset/syscon-reboot.c b/drivers/power/reset/syscon-reboot.c
new file mode 100644
index 0000000..29ed908
--- /dev/null
+++ b/drivers/power/reset/syscon-reboot.c
@@ -0,0 +1,100 @@
+/*
+ * Generic Syscon Reboot Driver
+ *
+ * Copyright (c) 2013, Applied Micro Circuits Corporation
+ * Author: Feng Kan <fkan@apm.com>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ *
+ * This driver provides system reboot functionality for APM X-Gene SoC.
+ * For system shutdown, this is board specify. If a board designer
+ * implements GPIO shutdown, use the gpio-poweroff.c driver.
+ */
+#include <linux/io.h>
+#include <linux/of_device.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/stat.h>
+#include <linux/slab.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+#include <linux/reboot.h>
+#include <asm/system_misc.h>
+
+struct syscon_reboot_context {
+	struct regmap *map;
+	u32 offset;
+	u32 mask;
+};
+
+static struct syscon_reboot_context *syscon_reboot_ctx;
+
+static void syscon_restart(enum reboot_mode reboot_mode, const char *cmd)
+{
+	struct syscon_reboot_context *ctx = syscon_reboot_ctx;
+	unsigned long timeout;
+
+	/* Issue the reboot */
+	if (ctx->map)
+		regmap_write(ctx->map, ctx->offset, ctx->mask);
+
+	timeout = jiffies + HZ;
+	while (time_before(jiffies, timeout))
+		cpu_relax();
+
+	pr_emerg("Unable to restart system\n");
+}
+
+static int syscon_reboot_probe(struct platform_device *pdev)
+{
+	struct syscon_reboot_context *ctx;
+	struct device *dev = &pdev->dev;
+
+	ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx) {
+		dev_err(&pdev->dev, "out of memory for context\n");
+		return -ENOMEM;
+	}
+
+	ctx->map = syscon_regmap_lookup_by_phandle(dev->of_node, "regmap");
+	if (IS_ERR(ctx->map))
+		return PTR_ERR(ctx->map);
+
+	if (of_property_read_u32(pdev->dev.of_node, "offset", &ctx->offset))
+		return -EINVAL;
+
+	if (of_property_read_u32(pdev->dev.of_node, "mask", &ctx->mask))
+		return -EINVAL;
+
+	arm_pm_restart = syscon_restart;
+	syscon_reboot_ctx = ctx;
+
+	return 0;
+}
+
+static struct of_device_id syscon_reboot_of_match[] = {
+	{ .compatible = "syscon-reboot" },
+	{}
+};
+
+static struct platform_driver syscon_reboot_driver = {
+	.probe = syscon_reboot_probe,
+	.driver = {
+		.name = "syscon-reboot",
+		.of_match_table = syscon_reboot_of_match,
+	},
+};
+module_platform_driver(syscon_reboot_driver);
-- 
1.7.6.1


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

* [PATCH V4 2/5] power: reset: Remove X-Gene reboot driver
  2014-01-23 19:19 [PATCH V4 0/5] Add X-Gene platform reboot mechanism Feng Kan
  2014-01-23 19:19 ` [PATCH V4 1/5] power: reset: Add generic SYSCON register mapped reset Feng Kan
@ 2014-01-23 19:19 ` Feng Kan
  2014-01-23 19:19 ` [PATCH V4 3/5] arm64: dts: Add X-Gene reboot driver dts node Feng Kan
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: Feng Kan @ 2014-01-23 19:19 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, devicetree; +Cc: Feng Kan

Remove X-Gene reboot driver.

Signed-off-by: Feng Kan <fkan@apm.com>
---
 drivers/power/reset/Kconfig        |    7 ---
 drivers/power/reset/Makefile       |    1 -
 drivers/power/reset/xgene-reboot.c |  103 ------------------------------------
 3 files changed, 0 insertions(+), 111 deletions(-)
 delete mode 100644 drivers/power/reset/xgene-reboot.c

diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index 4501c02..13a5191 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -45,13 +45,6 @@ config POWER_RESET_VEXPRESS
 	  Power off and reset support for the ARM Ltd. Versatile
 	  Express boards.
 
-config POWER_RESET_XGENE
-	bool "APM SoC X-Gene reset driver"
-	depends on ARM64
-	depends on POWER_RESET
-	help
-	  Reboot support for the APM SoC X-Gene Eval boards.
-
 config POWER_RESET_SYSCON
 	bool "Generic SYSCON regmap reset driver"
 	depends on MFD_SYSCON
diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
index f2c0327..a3137ff 100644
--- a/drivers/power/reset/Makefile
+++ b/drivers/power/reset/Makefile
@@ -3,5 +3,4 @@ obj-$(CONFIG_POWER_RESET_MSM) += msm-poweroff.o
 obj-$(CONFIG_POWER_RESET_QNAP) += qnap-poweroff.o
 obj-$(CONFIG_POWER_RESET_RESTART) += restart-poweroff.o
 obj-$(CONFIG_POWER_RESET_VEXPRESS) += vexpress-poweroff.o
-obj-$(CONFIG_POWER_RESET_XGENE) += xgene-reboot.o
 obj-$(CONFIG_POWER_RESET_SYSCON) += syscon-reboot.o
diff --git a/drivers/power/reset/xgene-reboot.c b/drivers/power/reset/xgene-reboot.c
deleted file mode 100644
index ecd55f8..0000000
--- a/drivers/power/reset/xgene-reboot.c
+++ /dev/null
@@ -1,103 +0,0 @@
-/*
- * AppliedMicro X-Gene SoC Reboot Driver
- *
- * Copyright (c) 2013, Applied Micro Circuits Corporation
- * Author: Feng Kan <fkan@apm.com>
- * Author: Loc Ho <lho@apm.com>
- *
- * 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.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- * MA 02111-1307 USA
- *
- * This driver provides system reboot functionality for APM X-Gene SoC.
- * For system shutdown, this is board specify. If a board designer
- * implements GPIO shutdown, use the gpio-poweroff.c driver.
- */
-#include <linux/io.h>
-#include <linux/of_device.h>
-#include <linux/of_address.h>
-#include <linux/platform_device.h>
-#include <linux/stat.h>
-#include <linux/slab.h>
-#include <asm/system_misc.h>
-
-struct xgene_reboot_context {
-	struct platform_device *pdev;
-	void *csr;
-	u32 mask;
-};
-
-static struct xgene_reboot_context *xgene_restart_ctx;
-
-static void xgene_restart(char str, const char *cmd)
-{
-	struct xgene_reboot_context *ctx = xgene_restart_ctx;
-	unsigned long timeout;
-
-	/* Issue the reboot */
-	if (ctx)
-		writel(ctx->mask, ctx->csr);
-
-	timeout = jiffies + HZ;
-	while (time_before(jiffies, timeout))
-		cpu_relax();
-
-	dev_emerg(&ctx->pdev->dev, "Unable to restart system\n");
-}
-
-static int xgene_reboot_probe(struct platform_device *pdev)
-{
-	struct xgene_reboot_context *ctx;
-
-	ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL);
-	if (!ctx) {
-		dev_err(&pdev->dev, "out of memory for context\n");
-		return -ENODEV;
-	}
-
-	ctx->csr = of_iomap(pdev->dev.of_node, 0);
-	if (!ctx->csr) {
-		devm_kfree(&pdev->dev, ctx);
-		dev_err(&pdev->dev, "can not map resource\n");
-		return -ENODEV;
-	}
-
-	if (of_property_read_u32(pdev->dev.of_node, "mask", &ctx->mask))
-		ctx->mask = 0xFFFFFFFF;
-
-	ctx->pdev = pdev;
-	arm_pm_restart = xgene_restart;
-	xgene_restart_ctx = ctx;
-
-	return 0;
-}
-
-static struct of_device_id xgene_reboot_of_match[] = {
-	{ .compatible = "apm,xgene-reboot" },
-	{}
-};
-
-static struct platform_driver xgene_reboot_driver = {
-	.probe = xgene_reboot_probe,
-	.driver = {
-		.name = "xgene-reboot",
-		.of_match_table = xgene_reboot_of_match,
-	},
-};
-
-static int __init xgene_reboot_init(void)
-{
-	return platform_driver_register(&xgene_reboot_driver);
-}
-device_initcall(xgene_reboot_init);
-- 
1.7.6.1


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

* [PATCH V4 3/5] arm64: dts: Add X-Gene reboot driver dts node
  2014-01-23 19:19 [PATCH V4 0/5] Add X-Gene platform reboot mechanism Feng Kan
  2014-01-23 19:19 ` [PATCH V4 1/5] power: reset: Add generic SYSCON register mapped reset Feng Kan
  2014-01-23 19:19 ` [PATCH V4 2/5] power: reset: Remove X-Gene reboot driver Feng Kan
@ 2014-01-23 19:19 ` Feng Kan
  2014-01-23 19:20 ` [PATCH V4 4/5] arm64: Select reboot driver for X-Gene platform Feng Kan
  2014-01-23 19:20 ` [PATCH V4 5/5] Documentation: power: reset: Add documentation for generic SYSCON reboot driver Feng Kan
  4 siblings, 0 replies; 14+ messages in thread
From: Feng Kan @ 2014-01-23 19:19 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, devicetree; +Cc: Feng Kan

Add X-Gene platform reboot driver dts node.

Signed-off-by: Feng Kan <fkan@apm.com>
---
 arch/arm64/boot/dts/apm-storm.dtsi |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/arch/arm64/boot/dts/apm-storm.dtsi b/arch/arm64/boot/dts/apm-storm.dtsi
index d37d736..4ef9d26 100644
--- a/arch/arm64/boot/dts/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm-storm.dtsi
@@ -103,6 +103,11 @@
 		#size-cells = <2>;
 		ranges;
 
+		scu: system-clk-controller@17000000 {
+			compatible = "apm,xgene-scu","syscon";
+			reg = <0x0 0x17000000 0x0 0x400>;
+		};
+
 		clocks {
 			#address-cells = <2>;
 			#size-cells = <2>;
@@ -187,5 +192,13 @@
 			interrupt-parent = <&gic>;
 			interrupts = <0x0 0x4c 0x4>;
 		};
+
+		reboot@17000014 {
+			compatible = "syscon-reboot";
+			regmap = <&scu>;
+			offset = <0x14>;
+			mask = <0x1>;
+		};
+
 	};
 };
-- 
1.7.6.1


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

* [PATCH V4 4/5] arm64: Select reboot driver for X-Gene platform
  2014-01-23 19:19 [PATCH V4 0/5] Add X-Gene platform reboot mechanism Feng Kan
                   ` (2 preceding siblings ...)
  2014-01-23 19:19 ` [PATCH V4 3/5] arm64: dts: Add X-Gene reboot driver dts node Feng Kan
@ 2014-01-23 19:20 ` Feng Kan
  2014-01-23 19:20 ` [PATCH V4 5/5] Documentation: power: reset: Add documentation for generic SYSCON reboot driver Feng Kan
  4 siblings, 0 replies; 14+ messages in thread
From: Feng Kan @ 2014-01-23 19:20 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, devicetree; +Cc: Feng Kan

Select reboot driver for X-Gene platform.

Signed-off-by: Feng Kan <fkan@apm.com>
---
 arch/arm64/Kconfig |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index dd4327f..f43820f 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -123,6 +123,8 @@ config ARCH_VEXPRESS
 
 config ARCH_XGENE
 	bool "AppliedMicro X-Gene SOC Family"
+	select MFD_SYSCON
+	select POWER_RESET_SYSCON
 	help
 	  This enables support for AppliedMicro X-Gene SOC Family
 
-- 
1.7.6.1


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

* [PATCH V4 5/5] Documentation: power: reset: Add documentation for generic SYSCON reboot driver
  2014-01-23 19:19 [PATCH V4 0/5] Add X-Gene platform reboot mechanism Feng Kan
                   ` (3 preceding siblings ...)
  2014-01-23 19:20 ` [PATCH V4 4/5] arm64: Select reboot driver for X-Gene platform Feng Kan
@ 2014-01-23 19:20 ` Feng Kan
  2014-01-24 11:39   ` Mark Rutland
  4 siblings, 1 reply; 14+ messages in thread
From: Feng Kan @ 2014-01-23 19:20 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, devicetree; +Cc: Feng Kan

Add documentation for generic SYSCON reboot driver.

Signed-off-by: Feng Kan <fkan@apm.com>
---
 .../bindings/power/reset/syscon-reboot.txt         |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt

diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
new file mode 100644
index 0000000..e9eb1fe
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
@@ -0,0 +1,16 @@
+Generic SYSCON mapped register reset driver
+
+Required properties:
+- compatible: should contain "syscon-reboot"
+- regmap: this is phandle to the register map node 
+- offset: offset in the register map for the reboot register
+- mask: the reset value written to the reboot register
+
+Examples:
+
+reboot {
+   compatible = "syscon-reboot";
+   regmap = <&regmapnode>;
+   offset = <0x0>;
+   mask = <0x1>;
+};
-- 
1.7.6.1


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

* Re: [PATCH V4 5/5] Documentation: power: reset: Add documentation for generic SYSCON reboot driver
  2014-01-23 19:20 ` [PATCH V4 5/5] Documentation: power: reset: Add documentation for generic SYSCON reboot driver Feng Kan
@ 2014-01-24 11:39   ` Mark Rutland
  2014-01-24 17:55     ` Christopher Covington
  2014-01-24 18:03     ` Feng Kan
  0 siblings, 2 replies; 14+ messages in thread
From: Mark Rutland @ 2014-01-24 11:39 UTC (permalink / raw)
  To: Feng Kan; +Cc: linux-arm-kernel, linux-kernel, devicetree

On Thu, Jan 23, 2014 at 07:20:01PM +0000, Feng Kan wrote:
> Add documentation for generic SYSCON reboot driver.
> 
> Signed-off-by: Feng Kan <fkan@apm.com>
> ---
>  .../bindings/power/reset/syscon-reboot.txt         |   16 ++++++++++++++++
>  1 files changed, 16 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
> new file mode 100644
> index 0000000..e9eb1fe
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
> @@ -0,0 +1,16 @@
> +Generic SYSCON mapped register reset driver

Bindings should describe hardware, not drivers.

What precisely does this binding describe?

> +
> +Required properties:
> +- compatible: should contain "syscon-reboot"
> +- regmap: this is phandle to the register map node 
> +- offset: offset in the register map for the reboot register
> +- mask: the reset value written to the reboot register
> +
> +Examples:
> +
> +reboot {
> +   compatible = "syscon-reboot";
> +   regmap = <&regmapnode>;
> +   offset = <0x0>;
> +   mask = <0x1>;
> +};

Access size? Endianness?

Why can we not have a binding for the register bank this exists in, and
have that pass on the appropriate details to a syscon-reboot driver?

That way we can change the way we poke things without requiring changes
to bindings or dts.

Thanks,
Mark.

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

* Re: [PATCH V4 5/5] Documentation: power: reset: Add documentation for generic SYSCON reboot driver
  2014-01-24 11:39   ` Mark Rutland
@ 2014-01-24 17:55     ` Christopher Covington
  2014-01-24 18:19       ` Mark Rutland
  2014-01-24 18:03     ` Feng Kan
  1 sibling, 1 reply; 14+ messages in thread
From: Christopher Covington @ 2014-01-24 17:55 UTC (permalink / raw)
  To: Mark Rutland; +Cc: Feng Kan, devicetree, linux-kernel, linux-arm-kernel

On 01/24/2014 06:39 AM, Mark Rutland wrote:
> On Thu, Jan 23, 2014 at 07:20:01PM +0000, Feng Kan wrote:
>> Add documentation for generic SYSCON reboot driver.
>>
>> Signed-off-by: Feng Kan <fkan@apm.com>
>> ---
>>  .../bindings/power/reset/syscon-reboot.txt         |   16 ++++++++++++++++
>>  1 files changed, 16 insertions(+), 0 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>>
>> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>> new file mode 100644
>> index 0000000..e9eb1fe
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>> @@ -0,0 +1,16 @@
>> +Generic SYSCON mapped register reset driver
> 
> Bindings should describe hardware, not drivers.

How is this different than what's done for PSCI?

Thanks,
Christopher

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by the Linux Foundation.

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

* Re: [PATCH V4 5/5] Documentation: power: reset: Add documentation for generic SYSCON reboot driver
  2014-01-24 11:39   ` Mark Rutland
  2014-01-24 17:55     ` Christopher Covington
@ 2014-01-24 18:03     ` Feng Kan
  2014-01-24 18:23       ` Mark Rutland
  1 sibling, 1 reply; 14+ messages in thread
From: Feng Kan @ 2014-01-24 18:03 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-arm-kernel, linux-kernel, devicetree

On Fri, Jan 24, 2014 at 3:39 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Thu, Jan 23, 2014 at 07:20:01PM +0000, Feng Kan wrote:
>> Add documentation for generic SYSCON reboot driver.
>>
>> Signed-off-by: Feng Kan <fkan@apm.com>
>> ---
>>  .../bindings/power/reset/syscon-reboot.txt         |   16 ++++++++++++++++
>>  1 files changed, 16 insertions(+), 0 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>>
>> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>> new file mode 100644
>> index 0000000..e9eb1fe
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>> @@ -0,0 +1,16 @@
>> +Generic SYSCON mapped register reset driver
>
> Bindings should describe hardware, not drivers.
>
> What precisely does this binding describe?
>
>> +
>> +Required properties:
>> +- compatible: should contain "syscon-reboot"
>> +- regmap: this is phandle to the register map node
>> +- offset: offset in the register map for the reboot register
>> +- mask: the reset value written to the reboot register
>> +
>> +Examples:
>> +
>> +reboot {
>> +   compatible = "syscon-reboot";
>> +   regmap = <&regmapnode>;
>> +   offset = <0x0>;
>> +   mask = <0x1>;
>> +};
>
> Access size? Endianness?
FKAN: are you asking for documentation? I don't see alot of example of
support for these.

>
> Why can we not have a binding for the register bank this exists in, and
> have that pass on the appropriate details to a syscon-reboot driver?

FKAN: Thats a good idea. But the hardware in this case (SCU) system
clock unit has a bunch of registers used for different functions. If syscon is
used alot in this case and we pile more attribute into it. It would get kinda
messy after a while.

FKAN: I still haven't figured out how to generically tie to
the reset handler? Maybe the next person can use #define to bridge in the
reset handler they want to use.

>
> That way we can change the way we poke things without requiring changes
> to bindings or dts.
>
> Thanks,
> Mark.

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

* Re: [PATCH V4 5/5] Documentation: power: reset: Add documentation for generic SYSCON reboot driver
  2014-01-24 17:55     ` Christopher Covington
@ 2014-01-24 18:19       ` Mark Rutland
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Rutland @ 2014-01-24 18:19 UTC (permalink / raw)
  To: Christopher Covington
  Cc: Feng Kan, devicetree, linux-kernel, linux-arm-kernel

On Fri, Jan 24, 2014 at 05:55:13PM +0000, Christopher Covington wrote:
> On 01/24/2014 06:39 AM, Mark Rutland wrote:
> > On Thu, Jan 23, 2014 at 07:20:01PM +0000, Feng Kan wrote:
> >> Add documentation for generic SYSCON reboot driver.
> >>
> >> Signed-off-by: Feng Kan <fkan@apm.com>
> >> ---
> >>  .../bindings/power/reset/syscon-reboot.txt         |   16 ++++++++++++++++
> >>  1 files changed, 16 insertions(+), 0 deletions(-)
> >>  create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
> >> new file mode 100644
> >> index 0000000..e9eb1fe
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
> >> @@ -0,0 +1,16 @@
> >> +Generic SYSCON mapped register reset driver
> > 
> > Bindings should describe hardware, not drivers.
> 
> How is this different than what's done for PSCI?

A PSCI node in the DT defines a standard interface to a firmware which
is external to Linux. The PSCI binding does not contain the word
"driver", but describes the interface that the binding describes, with
reference to approriate documentation.

All I'm arguing for here is a description of the class of hardware this
is applicable to, rather than "this is what this particular driver
uses".

Thanks,
Mark.

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

* Re: [PATCH V4 5/5] Documentation: power: reset: Add documentation for generic SYSCON reboot driver
  2014-01-24 18:03     ` Feng Kan
@ 2014-01-24 18:23       ` Mark Rutland
  2014-01-24 18:44         ` Feng Kan
  2014-01-24 23:16         ` Marc C
  0 siblings, 2 replies; 14+ messages in thread
From: Mark Rutland @ 2014-01-24 18:23 UTC (permalink / raw)
  To: Feng Kan; +Cc: linux-arm-kernel, linux-kernel, devicetree

On Fri, Jan 24, 2014 at 06:03:10PM +0000, Feng Kan wrote:
> On Fri, Jan 24, 2014 at 3:39 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Thu, Jan 23, 2014 at 07:20:01PM +0000, Feng Kan wrote:
> >> Add documentation for generic SYSCON reboot driver.
> >>
> >> Signed-off-by: Feng Kan <fkan@apm.com>
> >> ---
> >>  .../bindings/power/reset/syscon-reboot.txt         |   16 ++++++++++++++++
> >>  1 files changed, 16 insertions(+), 0 deletions(-)
> >>  create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
> >> new file mode 100644
> >> index 0000000..e9eb1fe
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
> >> @@ -0,0 +1,16 @@
> >> +Generic SYSCON mapped register reset driver
> >
> > Bindings should describe hardware, not drivers.
> >
> > What precisely does this binding describe?
> >
> >> +
> >> +Required properties:
> >> +- compatible: should contain "syscon-reboot"
> >> +- regmap: this is phandle to the register map node
> >> +- offset: offset in the register map for the reboot register
> >> +- mask: the reset value written to the reboot register
> >> +
> >> +Examples:
> >> +
> >> +reboot {
> >> +   compatible = "syscon-reboot";
> >> +   regmap = <&regmapnode>;
> >> +   offset = <0x0>;
> >> +   mask = <0x1>;
> >> +};
> >
> > Access size? Endianness?
> FKAN: are you asking for documentation? I don't see alot of example of
> support for these.

If I used the enippet in the example, what endianness and access size
should I expect an OS to perform? That should be documented.

If this doesn't match the general case, we can add properties later to
adjust the access size and/or endianness. We just need to document what
the binding actually describes currently, or it's not possible to
implement anything based off of the binding documentation.

I should be able to read a binding document and write a dts. I shouldn't
have to read the code to figure out what the binding describes.

> 
> >
> > Why can we not have a binding for the register bank this exists in, and
> > have that pass on the appropriate details to a syscon-reboot driver?
> 
> FKAN: Thats a good idea. But the hardware in this case (SCU) system
> clock unit has a bunch of registers used for different functions. If syscon is
> used alot in this case and we pile more attribute into it. It would get kinda
> messy after a while.

Huh?

What's wrong with having a system clock unit binding, that the kernel
can decompose as appropriate?

I don't get your syscon argument.

Thanks,
Mark.

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

* Re: [PATCH V4 5/5] Documentation: power: reset: Add documentation for generic SYSCON reboot driver
  2014-01-24 18:23       ` Mark Rutland
@ 2014-01-24 18:44         ` Feng Kan
  2014-01-24 23:16         ` Marc C
  1 sibling, 0 replies; 14+ messages in thread
From: Feng Kan @ 2014-01-24 18:44 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-arm-kernel, linux-kernel, devicetree

On Fri, Jan 24, 2014 at 10:23 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Fri, Jan 24, 2014 at 06:03:10PM +0000, Feng Kan wrote:
>> On Fri, Jan 24, 2014 at 3:39 AM, Mark Rutland <mark.rutland@arm.com> wrote:
>> > On Thu, Jan 23, 2014 at 07:20:01PM +0000, Feng Kan wrote:
>> >> Add documentation for generic SYSCON reboot driver.
>> >>
>> >> Signed-off-by: Feng Kan <fkan@apm.com>
>> >> ---
>> >>  .../bindings/power/reset/syscon-reboot.txt         |   16 ++++++++++++++++
>> >>  1 files changed, 16 insertions(+), 0 deletions(-)
>> >>  create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>> >> new file mode 100644
>> >> index 0000000..e9eb1fe
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>> >> @@ -0,0 +1,16 @@
>> >> +Generic SYSCON mapped register reset driver
>> >
>> > Bindings should describe hardware, not drivers.
>> >
>> > What precisely does this binding describe?
>> >
>> >> +
>> >> +Required properties:
>> >> +- compatible: should contain "syscon-reboot"
>> >> +- regmap: this is phandle to the register map node
>> >> +- offset: offset in the register map for the reboot register
>> >> +- mask: the reset value written to the reboot register
>> >> +
>> >> +Examples:
>> >> +
>> >> +reboot {
>> >> +   compatible = "syscon-reboot";
>> >> +   regmap = <&regmapnode>;
>> >> +   offset = <0x0>;
>> >> +   mask = <0x1>;
>> >> +};
>> >
>> > Access size? Endianness?
>> FKAN: are you asking for documentation? I don't see alot of example of
>> support for these.
>
> If I used the enippet in the example, what endianness and access size
> should I expect an OS to perform? That should be documented.
>
> If this doesn't match the general case, we can add properties later to
> adjust the access size and/or endianness. We just need to document what
> the binding actually describes currently, or it's not possible to
> implement anything based off of the binding documentation.
>
> I should be able to read a binding document and write a dts. I shouldn't
> have to read the code to figure out what the binding describes.
>
>>
>> >
>> > Why can we not have a binding for the register bank this exists in, and
>> > have that pass on the appropriate details to a syscon-reboot driver?
>>
>> FKAN: Thats a good idea. But the hardware in this case (SCU) system
>> clock unit has a bunch of registers used for different functions. If syscon is
>> used alot in this case and we pile more attribute into it. It would get kinda
>> messy after a while.
>
> Huh?
>
> What's wrong with having a system clock unit binding, that the kernel
> can decompose as appropriate?
>
> I don't get your syscon argument.

FKAN: I do have a SCU binding, I thought you wanted to move the offset and
mask to the SCU binding. The only issue I see in that case is when we use
more such methods, the SCU binding would look rather crowded. If this is not
the case, I am a bit confused at what I should do next.

>
> Thanks,
> Mark.

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

* Re: [PATCH V4 5/5] Documentation: power: reset: Add documentation for generic SYSCON reboot driver
  2014-01-24 18:23       ` Mark Rutland
  2014-01-24 18:44         ` Feng Kan
@ 2014-01-24 23:16         ` Marc C
  2014-01-29 15:08           ` Arnd Bergmann
  1 sibling, 1 reply; 14+ messages in thread
From: Marc C @ 2014-01-24 23:16 UTC (permalink / raw)
  To: Mark Rutland, Feng Kan; +Cc: devicetree, linux-kernel, linux-arm-kernel

Hi Mark,

>> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>> new file mode 100644
>> index 0000000..e9eb1fe
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>> @@ -0,0 +1,16 @@
>> +Generic SYSCON mapped register reset driver
>
> Bindings should describe hardware, not drivers

In a perfect world, the hardware designers would place _all_ of the registers needed to
support rebooting in a contiguous section of the memory map. However, this isn't the case
on some platforms, especially on ARM-based SoCs.

While I completely agree with you that the bindings describe hardware, I don't see how
Feng's work is contrary to that. Feng is working on logically-grouping an otherwise
"random" set of registers into a logical grouping. In this case, Feng is uniting a group
of registers and calling them the "reboot" register block.

> What's wrong with having a system clock unit binding, that the kernel
> can decompose as appropriate?

>From what I understand, the arm-soc maintainers want to reduce (and perhaps even
eliminate) these board-specific constructs, and try to utilize common driver-code that
resides in the "driver" folder. I can vouch for the syscon/regmap framework as something
which would enable the effort.

Thanks,
Marc C

On 01/24/2014 10:23 AM, Mark Rutland wrote:
> On Fri, Jan 24, 2014 at 06:03:10PM +0000, Feng Kan wrote:
>> On Fri, Jan 24, 2014 at 3:39 AM, Mark Rutland <mark.rutland@arm.com> wrote:
>>> On Thu, Jan 23, 2014 at 07:20:01PM +0000, Feng Kan wrote:
>>>> Add documentation for generic SYSCON reboot driver.
>>>>
>>>> Signed-off-by: Feng Kan <fkan@apm.com>
>>>> ---
>>>>  .../bindings/power/reset/syscon-reboot.txt         |   16 ++++++++++++++++
>>>>  1 files changed, 16 insertions(+), 0 deletions(-)
>>>>  create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>>>> new file mode 100644
>>>> index 0000000..e9eb1fe
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>>>> @@ -0,0 +1,16 @@
>>>> +Generic SYSCON mapped register reset driver
>>>
>>> Bindings should describe hardware, not drivers.
>>>
>>> What precisely does this binding describe?
>>>
>>>> +
>>>> +Required properties:
>>>> +- compatible: should contain "syscon-reboot"
>>>> +- regmap: this is phandle to the register map node
>>>> +- offset: offset in the register map for the reboot register
>>>> +- mask: the reset value written to the reboot register
>>>> +
>>>> +Examples:
>>>> +
>>>> +reboot {
>>>> +   compatible = "syscon-reboot";
>>>> +   regmap = <&regmapnode>;
>>>> +   offset = <0x0>;
>>>> +   mask = <0x1>;
>>>> +};
>>>
>>> Access size? Endianness?
>> FKAN: are you asking for documentation? I don't see alot of example of
>> support for these.
> 
> If I used the enippet in the example, what endianness and access size
> should I expect an OS to perform? That should be documented.
> 
> If this doesn't match the general case, we can add properties later to
> adjust the access size and/or endianness. We just need to document what
> the binding actually describes currently, or it's not possible to
> implement anything based off of the binding documentation.
> 
> I should be able to read a binding document and write a dts. I shouldn't
> have to read the code to figure out what the binding describes.
> 
>>
>>>
>>> Why can we not have a binding for the register bank this exists in, and
>>> have that pass on the appropriate details to a syscon-reboot driver?
>>
>> FKAN: Thats a good idea. But the hardware in this case (SCU) system
>> clock unit has a bunch of registers used for different functions. If syscon is
>> used alot in this case and we pile more attribute into it. It would get kinda
>> messy after a while.
> 
> Huh?
> 
> What's wrong with having a system clock unit binding, that the kernel
> can decompose as appropriate?
> 
> I don't get your syscon argument.
> 
> Thanks,
> Mark.
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 


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

* Re: [PATCH V4 5/5] Documentation: power: reset: Add documentation for generic SYSCON reboot driver
  2014-01-24 23:16         ` Marc C
@ 2014-01-29 15:08           ` Arnd Bergmann
  0 siblings, 0 replies; 14+ messages in thread
From: Arnd Bergmann @ 2014-01-29 15:08 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Marc C, Mark Rutland, Feng Kan, devicetree, linux-kernel

On Friday 24 January 2014 15:16:32 Marc C wrote:
> 
> > What's wrong with having a system clock unit binding, that the kernel
> > can decompose as appropriate?
> 
> From what I understand, the arm-soc maintainers want to reduce (and perhaps even
> eliminate) these board-specific constructs, and try to utilize common driver-code that
> resides in the "driver" folder. I can vouch for the syscon/regmap framework as something
> which would enable the effort.

While your statement is true in general, it doesn't seem to counter
what Mark R said above.

	Arnd

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

end of thread, other threads:[~2014-01-29 15:08 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-23 19:19 [PATCH V4 0/5] Add X-Gene platform reboot mechanism Feng Kan
2014-01-23 19:19 ` [PATCH V4 1/5] power: reset: Add generic SYSCON register mapped reset Feng Kan
2014-01-23 19:19 ` [PATCH V4 2/5] power: reset: Remove X-Gene reboot driver Feng Kan
2014-01-23 19:19 ` [PATCH V4 3/5] arm64: dts: Add X-Gene reboot driver dts node Feng Kan
2014-01-23 19:20 ` [PATCH V4 4/5] arm64: Select reboot driver for X-Gene platform Feng Kan
2014-01-23 19:20 ` [PATCH V4 5/5] Documentation: power: reset: Add documentation for generic SYSCON reboot driver Feng Kan
2014-01-24 11:39   ` Mark Rutland
2014-01-24 17:55     ` Christopher Covington
2014-01-24 18:19       ` Mark Rutland
2014-01-24 18:03     ` Feng Kan
2014-01-24 18:23       ` Mark Rutland
2014-01-24 18:44         ` Feng Kan
2014-01-24 23:16         ` Marc C
2014-01-29 15:08           ` Arnd Bergmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).