linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] make kurobox-pro be able to shutdown after device-tree migration
       [not found] <20161216100501.18173-1-rogershimizu@gmail.com>
@ 2016-12-27  7:06 ` Roger Shimizu
  2016-12-27  7:06   ` [PATCH v3 1/3] power: reset: add linkstation-reset driver Roger Shimizu
                     ` (3 more replies)
  0 siblings, 4 replies; 24+ messages in thread
From: Roger Shimizu @ 2016-12-27  7:06 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Kernel Maintainers,

Kurobox-Pro (and variants) need more commands sending to UART1 to shutdown.
So here I make this patch series to let current qnap-poweroff implementation
be able to handle such case.

Here I give 3 patches, for 3 different kernel subsystems respectivelay:
  - Patch 1/3: System reset/shutdown driver
  - Patch 2/3: Open firmware and flattened device tree bindings
  - Patch 3/3: ARM/Marvell Dove/MV78xx0/Orion SOC support

Ryan Tandy, the issue reporter, and I have already tested those changes on
Linkstation/KuroBoxPro devices, and confirmed it's working well.

Merry Xmax and look forward to your feedback!

Changes:
  v0 => v1:
  - Update 0003 to split kuroboxpro related code into kuroboxpro-common.c
  v1 => v2:
  - Split off linkstation/kuroboxpro related code to linkstation-reset.c
    Because linkstation before kuroboxpro also need this driver to power
    off properly. It's more proper to call it linkstation driver.
  v2 => v3:
  - Split off devicetree bindings doc into a separate patch.
  - Add device-tree patch to make Linkstation LS-GL and KuroBox Pro to
    use the newly created linkstation-reset driver.
  - Remove the unused one-byte command sending case in linkstation-reset
    driver.

Cheers,
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 4096R/6C6ACD6417B3ACB1

Roger Shimizu (3):
  power: reset: add linkstation-reset driver
  DT: bingdings: power: reset: add linkstation-reset doc
  ARM: DT: add power-off support to linkstation lsgl and kuroboxpro

 .../bindings/power/reset/linkstation-reset.txt     |  26 ++
 arch/arm/boot/dts/orion5x-kuroboxpro.dts           |   8 +
 arch/arm/boot/dts/orion5x-linkstation-lsgl.dts     |  10 +
 arch/arm/boot/dts/orion5x-linkstation-lswtgl.dts   |   4 +
 arch/arm/boot/dts/orion5x-linkstation.dtsi         |   4 -
 drivers/power/reset/Kconfig                        |  10 +
 drivers/power/reset/Makefile                       |   1 +
 drivers/power/reset/linkstation-reset.c            | 269 +++++++++++++++++++++
 8 files changed, 328 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/reset/linkstation-reset.txt
 create mode 100644 drivers/power/reset/linkstation-reset.c

-- 
2.11.0

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

* [PATCH v3 1/3] power: reset: add linkstation-reset driver
  2016-12-27  7:06 ` [PATCH v3 0/3] make kurobox-pro be able to shutdown after device-tree migration Roger Shimizu
@ 2016-12-27  7:06   ` Roger Shimizu
  2017-01-03  5:19     ` Florian Fainelli
  2016-12-27  7:06   ` [PATCH v3 2/3] DT: bingdings: power: reset: add linkstation-reset doc Roger Shimizu
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 24+ messages in thread
From: Roger Shimizu @ 2016-12-27  7:06 UTC (permalink / raw)
  To: linux-arm-kernel

Buffalo Linkstation / KuroBox and their variants need magic command
sending to UART1 to power-off.

Power driver linkstation-reset implements the magic command and I/O
routine, which come from files listed below:
  - arch/arm/mach-orion5x/kurobox_pro-setup.c
  - arch/arm/mach-orion5x/terastation_pro2-setup.c

To: Sebastian Reichel <sre@kernel.org>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Martin Michlmayr <tbm@cyrius.com>
Cc: Sylver Bruneau <sylver.bruneau@googlemail.com>
Cc: Herbert Valerio Riedel <hvr@gnu.org>
Cc: Ryan Tandy <ryan@nardis.ca>
Cc: linux-pm at vger.kernel.org
Cc: devicetree at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org

Reported-by: Ryan Tandy <ryan@nardis.ca>
Tested-by: Ryan Tandy <ryan@nardis.ca>
Signed-off-by: Roger Shimizu <rogershimizu@gmail.com>
---
 drivers/power/reset/Kconfig             |  10 ++
 drivers/power/reset/Makefile            |   1 +
 drivers/power/reset/linkstation-reset.c | 269 ++++++++++++++++++++++++++++++++
 3 files changed, 280 insertions(+)
 create mode 100644 drivers/power/reset/linkstation-reset.c

diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index c74c3f67b8da..77c44cad7ece 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -98,6 +98,16 @@ config POWER_RESET_IMX
 	  say N here or disable in dts to make sure pm_power_off never be
 	  overwrote wrongly by this driver.
 
+config POWER_RESET_LINKSTATION
+	bool "Buffalo Linkstation and its variants reset driver"
+	depends on OF_GPIO && PLAT_ORION
+	help
+	  This driver supports power off Buffalo Linkstation / KuroBox Pro
+	  NAS and their variants by sending commands to the micro-controller
+	  which controls the main power.
+
+	  Say Y if you have a Buffalo Linkstation / KuroBox Pro NAS.
+
 config POWER_RESET_MSM
 	bool "Qualcomm MSM power-off driver"
 	depends on ARCH_QCOM
diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
index 1be307c7fc25..692ba6417cfb 100644
--- a/drivers/power/reset/Makefile
+++ b/drivers/power/reset/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_POWER_RESET_GPIO) += gpio-poweroff.o
 obj-$(CONFIG_POWER_RESET_GPIO_RESTART) += gpio-restart.o
 obj-$(CONFIG_POWER_RESET_HISI) += hisi-reboot.o
 obj-$(CONFIG_POWER_RESET_IMX) += imx-snvs-poweroff.o
+obj-$(CONFIG_POWER_RESET_LINKSTATION) += linkstation-reset.o
 obj-$(CONFIG_POWER_RESET_MSM) += msm-poweroff.o
 obj-$(CONFIG_POWER_RESET_LTC2952) += ltc2952-poweroff.o
 obj-$(CONFIG_POWER_RESET_QNAP) += qnap-poweroff.o
diff --git a/drivers/power/reset/linkstation-reset.c b/drivers/power/reset/linkstation-reset.c
new file mode 100644
index 000000000000..4390495dfe0e
--- /dev/null
+++ b/drivers/power/reset/linkstation-reset.c
@@ -0,0 +1,269 @@
+/*
+ * Buffalo Linkstation power reset driver.
+ * It may also be used on following devices:
+ *  - KuroBox Pro
+ *  - Buffalo Linkstation Pro (LS-GL)
+ *  - Buffalo Terastation Pro II/Live
+ *  - Buffalo Linkstation Duo (LS-WTGL)
+ *  - Buffalo Linkstation Mini (LS-WSGL)
+ *
+ * Copyright (C) 2016  Roger Shimizu <rogershimizu@gmail.com>
+ *
+ * Based on the code from:
+ *
+ * Copyright (C) 2012  Andrew Lunn <andrew@lunn.ch>
+ * Copyright (C) 2009  Martin Michlmayr <tbm@cyrius.com>
+ * Copyright (C) 2008  Byron Bradley <byron.bbradley@gmail.com>
+ * Copyright (C) 2008  Sylver Bruneau <sylver.bruneau@googlemail.com>
+ * Copyright (C) 2007  Herbert Valerio Riedel <hvr@gnu.org>
+ *
+ * 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/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/serial_reg.h>
+#include <linux/kallsyms.h>
+#include <linux/of.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+
+#define UART1_REG(x)	(base + ((UART_##x) << 2))
+#define MICON_CMD_SIZE	4
+
+/* 4-byte magic hello command to UART1-attached microcontroller */
+static const unsigned char linkstation_micon_magic[] = {
+	0x1b,
+	0x00,
+	0x07,
+	0x00
+};
+
+/* for each row, first byte is the size of command */
+static const unsigned char linkstation_power_off_cmd[][MICON_CMD_SIZE] = {
+	{ 3,	0x01, 0x35, 0x00},
+	{ 2,	0x00, 0x0c},
+	{ 2,	0x00, 0x06},
+	{}
+};
+
+struct reset_cfg {
+	u32 baud;
+	const unsigned char *magic;
+	const unsigned char (*cmd)[MICON_CMD_SIZE];
+};
+
+static const struct reset_cfg linkstation_power_off_cfg = {
+	.baud = 38400,
+	.magic = linkstation_micon_magic,
+	.cmd = linkstation_power_off_cmd,
+};
+
+static const struct of_device_id linkstation_reset_of_match_table[] = {
+	{ .compatible = "linkstation,power-off",
+	  .data = &linkstation_power_off_cfg,
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, linkstation_reset_of_match_table);
+
+static int uart1_micon_read(void *base, unsigned char *buf, int count)
+{
+	int i;
+	int timeout;
+
+	for (i = 0; i < count; i++) {
+		timeout = 10;
+
+		while (!(readl(UART1_REG(LSR)) & UART_LSR_DR)) {
+			if (--timeout == 0)
+				break;
+			udelay(1000);
+		}
+
+		if (timeout == 0)
+			break;
+		buf[i] = readl(UART1_REG(RX));
+	}
+
+	/* return read bytes */
+	return i;
+}
+
+static int uart1_micon_write(void *base, const unsigned char *buf, int count)
+{
+	int i = 0;
+
+	while (count--) {
+		while (!(readl(UART1_REG(LSR)) & UART_LSR_THRE))
+			barrier();
+		writel(buf[i++], UART1_REG(TX));
+	}
+
+	return 0;
+}
+
+int uart1_micon_send(void *base, const unsigned char *data, int count)
+{
+	int i;
+	unsigned char checksum = 0;
+	unsigned char recv_buf[40];
+	unsigned char send_buf[40];
+	unsigned char correct_ack[3];
+	int retry = 2;
+
+	/* Generate checksum */
+	for (i = 0; i < count; i++)
+		checksum -=  data[i];
+
+	do {
+		/* Send data */
+		uart1_micon_write(base, data, count);
+
+		/* send checksum */
+		uart1_micon_write(base, &checksum, 1);
+
+		if (uart1_micon_read(base, recv_buf, sizeof(recv_buf)) <= 3) {
+			printk(KERN_ERR ">%s: receive failed.\n", __func__);
+
+			/* send preamble to clear the receive buffer */
+			memset(&send_buf, 0xff, sizeof(send_buf));
+			uart1_micon_write(base, send_buf, sizeof(send_buf));
+
+			/* make dummy reads */
+			mdelay(100);
+			uart1_micon_read(base, recv_buf, sizeof(recv_buf));
+		} else {
+			/* Generate expected ack */
+			correct_ack[0] = 0x01;
+			correct_ack[1] = data[1];
+			correct_ack[2] = 0x00;
+
+			/* checksum Check */
+			if ((recv_buf[0] + recv_buf[1] + recv_buf[2] +
+			     recv_buf[3]) & 0xFF) {
+				printk(KERN_ERR ">%s: Checksum Error : "
+					"Received data[%02x, %02x, %02x, %02x]"
+					"\n", __func__, recv_buf[0],
+					recv_buf[1], recv_buf[2], recv_buf[3]);
+			} else {
+				/* Check Received Data */
+				if (correct_ack[0] == recv_buf[0] &&
+				    correct_ack[1] == recv_buf[1] &&
+				    correct_ack[2] == recv_buf[2]) {
+					/* Interval for next command */
+					mdelay(10);
+
+					/* Receive ACK */
+					return 0;
+				}
+			}
+			/* Received NAK or illegal Data */
+			printk(KERN_ERR ">%s: Error : NAK or Illegal Data "
+					"Received\n", __func__);
+		}
+	} while (retry--);
+
+	/* Interval for next command */
+	mdelay(10);
+
+	return -1;
+}
+
+static void __iomem *base;
+static unsigned long tclk;
+static const struct reset_cfg *cfg;
+
+static void linkstation_reset(void)
+{
+	const unsigned divisor = ((tclk + (8 * cfg->baud)) / (16 * cfg->baud));
+	int i;
+
+	pr_err("%s: triggering power-off...\n", __func__);
+
+	/* hijack UART1 and reset into sane state */
+	writel(0x83, UART1_REG(LCR));
+	writel(divisor & 0xff, UART1_REG(DLL));
+	writel((divisor >> 8) & 0xff, UART1_REG(DLM));
+	writel(cfg->magic[0], UART1_REG(LCR));
+	writel(cfg->magic[1], UART1_REG(IER));
+	writel(cfg->magic[2], UART1_REG(FCR));
+	writel(cfg->magic[3], UART1_REG(MCR));
+
+	/* send the power-off command to PIC */
+	for(i = 0; cfg->cmd[i][0] > 0; i ++) {
+		/* [0] is size of the command; command starts from [1] */
+		uart1_micon_send(base, &(cfg->cmd[i][1]), cfg->cmd[i][0]);
+	}
+}
+
+static int linkstation_reset_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct resource *res;
+	struct clk *clk;
+	char symname[KSYM_NAME_LEN];
+
+	const struct of_device_id *match =
+		of_match_node(linkstation_reset_of_match_table, np);
+	cfg = match->data;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "Missing resource");
+		return -EINVAL;
+	}
+
+	base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+	if (!base) {
+		dev_err(&pdev->dev, "Unable to map resource");
+		return -EINVAL;
+	}
+
+	/* We need to know tclk in order to calculate the UART divisor */
+	clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(clk)) {
+		dev_err(&pdev->dev, "Clk missing");
+		return PTR_ERR(clk);
+	}
+
+	tclk = clk_get_rate(clk);
+
+	/* Check that nothing else has already setup a handler */
+	if (pm_power_off) {
+		lookup_symbol_name((ulong)pm_power_off, symname);
+		dev_err(&pdev->dev,
+			"pm_power_off already claimed %p %s",
+			pm_power_off, symname);
+		return -EBUSY;
+	}
+	pm_power_off = linkstation_reset;
+
+	return 0;
+}
+
+static int linkstation_reset_remove(struct platform_device *pdev)
+{
+	pm_power_off = NULL;
+	return 0;
+}
+
+static struct platform_driver linkstation_reset_driver = {
+	.probe	= linkstation_reset_probe,
+	.remove	= linkstation_reset_remove,
+	.driver	= {
+		.name	= "linkstation_reset",
+		.of_match_table = of_match_ptr(linkstation_reset_of_match_table),
+	},
+};
+
+module_platform_driver(linkstation_reset_driver);
+
+MODULE_AUTHOR("Roger Shimizu <rogershimizu@gmail.com>");
+MODULE_DESCRIPTION("Linkstation Reset driver");
+MODULE_LICENSE("GPL v2");
-- 
2.11.0

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

* [PATCH v3 2/3] DT: bingdings: power: reset: add linkstation-reset doc
  2016-12-27  7:06 ` [PATCH v3 0/3] make kurobox-pro be able to shutdown after device-tree migration Roger Shimizu
  2016-12-27  7:06   ` [PATCH v3 1/3] power: reset: add linkstation-reset driver Roger Shimizu
@ 2016-12-27  7:06   ` Roger Shimizu
  2017-01-03  5:21     ` Florian Fainelli
  2017-01-03 17:09     ` Rob Herring
  2016-12-27  7:06   ` [PATCH v3 3/3] ARM: DT: add power-off support to linkstation lsgl and kuroboxpro Roger Shimizu
  2017-01-07 15:04   ` [PATCH v4 0/2] make kurobox-pro be able to shutdown after device-tree migration Roger Shimizu
  3 siblings, 2 replies; 24+ messages in thread
From: Roger Shimizu @ 2016-12-27  7:06 UTC (permalink / raw)
  To: linux-arm-kernel

Add linkstation-reset doc to describe the newly added
POWER_RESET_LINKSTATION driver, which controls magic command
sending to UART1 to power-off Buffalo Linkstation / KuroBox
and their variants.

To: Sebastian Reichel <sre@kernel.org>
To: Rob Herring <robh+dt@kernel.org>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Ryan Tandy <ryan@nardis.ca>
Cc: linux-pm at vger.kernel.org
Cc: devicetree at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org

Signed-off-by: Roger Shimizu <rogershimizu@gmail.com>
---
 .../bindings/power/reset/linkstation-reset.txt     | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/reset/linkstation-reset.txt

diff --git a/Documentation/devicetree/bindings/power/reset/linkstation-reset.txt b/Documentation/devicetree/bindings/power/reset/linkstation-reset.txt
new file mode 100644
index 000000000000..815e340318f3
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/linkstation-reset.txt
@@ -0,0 +1,26 @@
+* Buffalo Linkstation Reset Driver
+
+Power of some Buffalo Linkstation or KuroBox Pro is managed by
+micro-controller, which connects to UART1. After being fed from UART1
+by a few magic numbers, the so-called power-off command,
+the micro-controller will turn power off the device.
+
+This is very similar to QNAP or Synology NAS devices, which is
+described in qnap-poweroff.txt, however the command is much simpler,
+only 1-byte long and without checksums.
+
+This driver adds a handler to pm_power_off which is called to turn the
+power off.
+
+Required Properties:
+- compatible: Should be "linkstation,power-off"
+- reg: Address and length of the register set for UART1
+- clocks: tclk clock
+
+Example:
+
+	reset {
+		compatible = "linkstation,power-off";
+		reg = <0x12100 0x100>;
+		clocks = <&core_clk 0>;
+	};
-- 
2.11.0

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

* [PATCH v3 3/3] ARM: DT: add power-off support to linkstation lsgl and kuroboxpro
  2016-12-27  7:06 ` [PATCH v3 0/3] make kurobox-pro be able to shutdown after device-tree migration Roger Shimizu
  2016-12-27  7:06   ` [PATCH v3 1/3] power: reset: add linkstation-reset driver Roger Shimizu
  2016-12-27  7:06   ` [PATCH v3 2/3] DT: bingdings: power: reset: add linkstation-reset doc Roger Shimizu
@ 2016-12-27  7:06   ` Roger Shimizu
  2017-01-07 15:04   ` [PATCH v4 0/2] make kurobox-pro be able to shutdown after device-tree migration Roger Shimizu
  3 siblings, 0 replies; 24+ messages in thread
From: Roger Shimizu @ 2016-12-27  7:06 UTC (permalink / raw)
  To: linux-arm-kernel

A few models of Linkstation / KuroBox has micro-controller which
controls the power (as well as FAN, but not related here), while
others not. So remove the restart-poweroff driver in linkstation
common dtsi file, and specify the proper power driver in dts
of each device.

Devices need micro-controler to power-off:
  - Linkstation LS-GL
  - KuroBox Pro
Device continues using original restart-poweroff driver:
  - Linkstation LS-WTGL

To: Jason Cooper <jason@lakedaemon.net>
To: Andrew Lunn <andrew@lunn.ch>
To: Gregory Clement <gregory.clement@free-electrons.com>
To: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Ryan Tandy <ryan@nardis.ca>
Cc: linux-pm at vger.kernel.org
Cc: devicetree at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org

Reported-by: Ryan Tandy <ryan@nardis.ca>
Tested-by: Ryan Tandy <ryan@nardis.ca>
Signed-off-by: Roger Shimizu <rogershimizu@gmail.com>
---
 arch/arm/boot/dts/orion5x-kuroboxpro.dts         |  8 ++++++++
 arch/arm/boot/dts/orion5x-linkstation-lsgl.dts   | 10 ++++++++++
 arch/arm/boot/dts/orion5x-linkstation-lswtgl.dts |  4 ++++
 arch/arm/boot/dts/orion5x-linkstation.dtsi       |  4 ----
 4 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/orion5x-kuroboxpro.dts b/arch/arm/boot/dts/orion5x-kuroboxpro.dts
index 1a672b098d0b..aba38d802bda 100644
--- a/arch/arm/boot/dts/orion5x-kuroboxpro.dts
+++ b/arch/arm/boot/dts/orion5x-kuroboxpro.dts
@@ -60,6 +60,14 @@
 				 <MBUS_ID(0x09, 0x00) 0 0xf2200000 0x800>,
 				 <MBUS_ID(0x01, 0x0f) 0 0xf4000000 0x40000>,
 				 <MBUS_ID(0x01, 0x1e) 0 0xfc000000 0x1000000>;
+
+		internal-regs {
+			power_off {
+				compatible = "linkstation,power-off";
+				reg = <0x12100 0x100>;
+				clocks = <&core_clk 0>;
+			};
+		};
 	};
 
 	memory { /* 128 MB */
diff --git a/arch/arm/boot/dts/orion5x-linkstation-lsgl.dts b/arch/arm/boot/dts/orion5x-linkstation-lsgl.dts
index 51dc734cd5b9..370fc17a6dd9 100644
--- a/arch/arm/boot/dts/orion5x-linkstation-lsgl.dts
+++ b/arch/arm/boot/dts/orion5x-linkstation-lsgl.dts
@@ -56,6 +56,16 @@
 	model = "Buffalo Linkstation Pro/Live";
 	compatible = "buffalo,lsgl", "marvell,orion5x-88f5182", "marvell,orion5x";
 
+	soc {
+		internal-regs {
+			power_off {
+				compatible = "linkstation,power-off";
+				reg = <0x12100 0x100>;
+				clocks = <&core_clk 0>;
+			};
+		};
+	};
+
 	memory { /* 128 MB */
 		device_type = "memory";
 		reg = <0x00000000 0x8000000>;
diff --git a/arch/arm/boot/dts/orion5x-linkstation-lswtgl.dts b/arch/arm/boot/dts/orion5x-linkstation-lswtgl.dts
index 0eead400f427..571a71f5b7ad 100644
--- a/arch/arm/boot/dts/orion5x-linkstation-lswtgl.dts
+++ b/arch/arm/boot/dts/orion5x-linkstation-lswtgl.dts
@@ -59,6 +59,10 @@
 		reg = <0x00000000 0x4000000>;
 	};
 
+	restart_poweroff {
+		compatible = "restart-poweroff";
+	};
+
 	gpio_keys {
 		power-on-switch {
 			gpios = <&gpio0 8 GPIO_ACTIVE_LOW>;
diff --git a/arch/arm/boot/dts/orion5x-linkstation.dtsi b/arch/arm/boot/dts/orion5x-linkstation.dtsi
index ed456ab35fd8..40770a8d4b36 100644
--- a/arch/arm/boot/dts/orion5x-linkstation.dtsi
+++ b/arch/arm/boot/dts/orion5x-linkstation.dtsi
@@ -57,10 +57,6 @@
 				 <MBUS_ID(0x01, 0x0f) 0 0xf4000000 0x40000>;
 	};
 
-	restart_poweroff {
-		compatible = "restart-poweroff";
-	};
-
 	regulators {
 		compatible = "simple-bus";
 		#address-cells = <1>;
-- 
2.11.0

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

* [PATCH v3 1/3] power: reset: add linkstation-reset driver
  2016-12-27  7:06   ` [PATCH v3 1/3] power: reset: add linkstation-reset driver Roger Shimizu
@ 2017-01-03  5:19     ` Florian Fainelli
  2017-01-03 13:09       ` Andrew Lunn
  0 siblings, 1 reply; 24+ messages in thread
From: Florian Fainelli @ 2017-01-03  5:19 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Roger,

On 12/26/2016 11:06 PM, Roger Shimizu wrote:
> Buffalo Linkstation / KuroBox and their variants need magic command
> sending to UART1 to power-off.
> 
> Power driver linkstation-reset implements the magic command and I/O
> routine, which come from files listed below:
>   - arch/arm/mach-orion5x/kurobox_pro-setup.c
>   - arch/arm/mach-orion5x/terastation_pro2-setup.c

Interestingly, I submitted a patch doing nearly the same thing recently
after hacking on a Buffalo Terastation Pro II two days after yours
without seeing yours:

https://lkml.org/lkml/2016/12/28/273

Some comments below.

> +
> +static void __iomem *base;
> +static unsigned long tclk;
> +static const struct reset_cfg *cfg;

How about avoiding the singletons here and pass this down from the
platform driver's private data after we (see below) also make use of a
reboot notifier?

> +
> +static void linkstation_reset(void)
> +{
> +	const unsigned divisor = ((tclk + (8 * cfg->baud)) / (16 * cfg->baud));
> +	int i;
> +
> +	pr_err("%s: triggering power-off...\n", __func__);
> +
> +	/* hijack UART1 and reset into sane state */
> +	writel(0x83, UART1_REG(LCR));
> +	writel(divisor & 0xff, UART1_REG(DLL));
> +	writel((divisor >> 8) & 0xff, UART1_REG(DLM));
> +	writel(cfg->magic[0], UART1_REG(LCR));
> +	writel(cfg->magic[1], UART1_REG(IER));
> +	writel(cfg->magic[2], UART1_REG(FCR));
> +	writel(cfg->magic[3], UART1_REG(MCR));
> +
> +	/* send the power-off command to PIC */
> +	for(i = 0; cfg->cmd[i][0] > 0; i ++) {
> +		/* [0] is size of the command; command starts from [1] */
> +		uart1_micon_send(base, &(cfg->cmd[i][1]), cfg->cmd[i][0]);
> +	}
> +}
> +
> +static int linkstation_reset_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct resource *res;
> +	struct clk *clk;
> +	char symname[KSYM_NAME_LEN];
> +
> +	const struct of_device_id *match =
> +		of_match_node(linkstation_reset_of_match_table, np);
> +	cfg = match->data;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(&pdev->dev, "Missing resource");
> +		return -EINVAL;
> +	}
> +
> +	base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> +	if (!base) {
> +		dev_err(&pdev->dev, "Unable to map resource");
> +		return -EINVAL;
> +	}
> +
> +	/* We need to know tclk in order to calculate the UART divisor */
> +	clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(clk)) {
> +		dev_err(&pdev->dev, "Clk missing");
> +		return PTR_ERR(clk);
> +	}
> +
> +	tclk = clk_get_rate(clk);

Does this work with the Terastation II which has not been converted to
DT yet? Is tclk available through the CLK API there?

> +
> +	/* Check that nothing else has already setup a handler */
> +	if (pm_power_off) {
> +		lookup_symbol_name((ulong)pm_power_off, symname);
> +		dev_err(&pdev->dev,
> +			"pm_power_off already claimed %p %s",
> +			pm_power_off, symname);
> +		return -EBUSY;
> +	}
> +	pm_power_off = linkstation_reset;

That seems a bit complicated, why not just assume that there will be
either this driver used, or not at all?

Also, you are supposed to register a reboot notifier to which you can
pass private context:

https://lkml.org/lkml/2016/12/28/275

As indicated in my patch series, the UART1-attached micro controller
does a lot more things that just providing reboot, which is why I chose
to move this code to a MFD driver, as the starting point before adding
support for LEDs, FAN, PWM, beeper which would be other types of devices.

Is adding support for other peripherals on your TODO as well?

Thanks!
-- 
Florian

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

* [PATCH v3 2/3] DT: bingdings: power: reset: add linkstation-reset doc
  2016-12-27  7:06   ` [PATCH v3 2/3] DT: bingdings: power: reset: add linkstation-reset doc Roger Shimizu
@ 2017-01-03  5:21     ` Florian Fainelli
  2017-01-03 13:12       ` Andrew Lunn
  2017-01-03 17:09     ` Rob Herring
  1 sibling, 1 reply; 24+ messages in thread
From: Florian Fainelli @ 2017-01-03  5:21 UTC (permalink / raw)
  To: linux-arm-kernel



On 12/26/2016 11:06 PM, Roger Shimizu wrote:
> Add linkstation-reset doc to describe the newly added
> POWER_RESET_LINKSTATION driver, which controls magic command
> sending to UART1 to power-off Buffalo Linkstation / KuroBox
> and their variants.
> 
> To: Sebastian Reichel <sre@kernel.org>
> To: Rob Herring <robh+dt@kernel.org>
> To: Mark Rutland <mark.rutland@arm.com>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Ryan Tandy <ryan@nardis.ca>
> Cc: linux-pm at vger.kernel.org
> Cc: devicetree at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> 
> Signed-off-by: Roger Shimizu <rogershimizu@gmail.com>
> ---
>  .../bindings/power/reset/linkstation-reset.txt     | 26 ++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/reset/linkstation-reset.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/reset/linkstation-reset.txt b/Documentation/devicetree/bindings/power/reset/linkstation-reset.txt
> new file mode 100644
> index 000000000000..815e340318f3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/linkstation-reset.txt
> @@ -0,0 +1,26 @@
> +* Buffalo Linkstation Reset Driver
> +
> +Power of some Buffalo Linkstation or KuroBox Pro is managed by
> +micro-controller, which connects to UART1. After being fed from UART1
> +by a few magic numbers, the so-called power-off command,
> +the micro-controller will turn power off the device.
> +
> +This is very similar to QNAP or Synology NAS devices, which is
> +described in qnap-poweroff.txt, however the command is much simpler,
> +only 1-byte long and without checksums.
> +
> +This driver adds a handler to pm_power_off which is called to turn the
> +power off.

This is a driver implementation detail, so does not really belong in the
DT here.

> +
> +Required Properties:
> +- compatible: Should be "linkstation,power-off"
> +- reg: Address and length of the register set for UART1

Humm, should we instead have a phandle to the uart1 node?

> +- clocks: tclk clock
> +
> +Example:
> +
> +	reset {
> +		compatible = "linkstation,power-off";
> +		reg = <0x12100 0x100>;
> +		clocks = <&core_clk 0>;
> +	};
> 

-- 
Florian

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

* [PATCH v3 1/3] power: reset: add linkstation-reset driver
  2017-01-03  5:19     ` Florian Fainelli
@ 2017-01-03 13:09       ` Andrew Lunn
  2017-01-03 14:08         ` Roger Shimizu
  0 siblings, 1 reply; 24+ messages in thread
From: Andrew Lunn @ 2017-01-03 13:09 UTC (permalink / raw)
  To: linux-arm-kernel

> > +
> > +	/* Check that nothing else has already setup a handler */
> > +	if (pm_power_off) {
> > +		lookup_symbol_name((ulong)pm_power_off, symname);
> > +		dev_err(&pdev->dev,
> > +			"pm_power_off already claimed %p %s",
> > +			pm_power_off, symname);
> > +		return -EBUSY;
> > +	}
> > +	pm_power_off = linkstation_reset;
> 
> That seems a bit complicated, why not just assume that there will be
> either this driver used, or not at all?

That is probably my fault. This is a copy from code i wrote many years
ago for the QNAP. I guess at the time i was battling with two
different pm_power_off handlers, so put in this code.

> Also, you are supposed to register a reboot notifier to which you can
> pass private context:

At the time i wrote the QNAP code, this did not exist. So maybe my
code is no longer a good example to copy.

	 Andrew

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

* [PATCH v3 2/3] DT: bingdings: power: reset: add linkstation-reset doc
  2017-01-03  5:21     ` Florian Fainelli
@ 2017-01-03 13:12       ` Andrew Lunn
  2017-01-03 14:11         ` Roger Shimizu
  0 siblings, 1 reply; 24+ messages in thread
From: Andrew Lunn @ 2017-01-03 13:12 UTC (permalink / raw)
  To: linux-arm-kernel

> > +
> > +Required Properties:
> > +- compatible: Should be "linkstation,power-off"
> > +- reg: Address and length of the register set for UART1
> 
> Humm, should we instead have a phandle to the uart1 node?

Probably. Again, this is to do with copying the QNAP driver. I was
young, new to device tree, and i just did a logical conversion of the
existing code, and did not at the time understand phandles, etc.

	 Andrew

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

* [PATCH v3 1/3] power: reset: add linkstation-reset driver
  2017-01-03 13:09       ` Andrew Lunn
@ 2017-01-03 14:08         ` Roger Shimizu
  2017-01-03 18:39           ` Florian Fainelli
  0 siblings, 1 reply; 24+ messages in thread
From: Roger Shimizu @ 2017-01-03 14:08 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Florian, Andrew,

Thanks for your email!

On Tue, Jan 3, 2017 at 2:19 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> Interestingly, I submitted a patch doing nearly the same thing recently
> after hacking on a Buffalo Terastation Pro II two days after yours
> without seeing yours:
>
> https://lkml.org/lkml/2016/12/28/273

Glad to know there's other developer working on linkstation/kurobox platform!

> Some comments below.
>
>> +
>> +static void __iomem *base;
>> +static unsigned long tclk;
>> +static const struct reset_cfg *cfg;
>
> How about avoiding the singletons here and pass this down from the
> platform driver's private data after we (see below) also make use of a
> reboot notifier?

I see your patches. Indeed, it's a good idea to avoid the singletons
and use private data instead.

>> +static int linkstation_reset_probe(struct platform_device *pdev)
>> +{
>> +     struct device_node *np = pdev->dev.of_node;
>> +     struct resource *res;
>> +     struct clk *clk;
>> +     char symname[KSYM_NAME_LEN];
>> +
>> +     const struct of_device_id *match =
>> +             of_match_node(linkstation_reset_of_match_table, np);
>> +     cfg = match->data;
>> +
>> +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +     if (!res) {
>> +             dev_err(&pdev->dev, "Missing resource");
>> +             return -EINVAL;
>> +     }
>> +
>> +     base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
>> +     if (!base) {
>> +             dev_err(&pdev->dev, "Unable to map resource");
>> +             return -EINVAL;
>> +     }
>> +
>> +     /* We need to know tclk in order to calculate the UART divisor */
>> +     clk = devm_clk_get(&pdev->dev, NULL);
>> +     if (IS_ERR(clk)) {
>> +             dev_err(&pdev->dev, "Clk missing");
>> +             return PTR_ERR(clk);
>> +     }
>> +
>> +     tclk = clk_get_rate(clk);
>
> Does this work with the Terastation II which has not been converted to
> DT yet? Is tclk available through the CLK API there?

I have no idea whether device-based legacy code and make use of power
reset driver.
Maybe Sebastian Reichel can offer a comment?

However, I think Terastation II should convert to DT first.
If you're willing to test, I can help to provide a dts/dtb.
(If you use Debian, I can even provide DEB of kernel image and
flash-kernel patch, which is easy for you to test)

On Tue, Jan 3, 2017 at 10:09 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> > +
>> > +   /* Check that nothing else has already setup a handler */
>> > +   if (pm_power_off) {
>> > +           lookup_symbol_name((ulong)pm_power_off, symname);
>> > +           dev_err(&pdev->dev,
>> > +                   "pm_power_off already claimed %p %s",
>> > +                   pm_power_off, symname);
>> > +           return -EBUSY;
>> > +   }
>> > +   pm_power_off = linkstation_reset;
>>
>> That seems a bit complicated, why not just assume that there will be
>> either this driver used, or not at all?
>
> That is probably my fault. This is a copy from code i wrote many years
> ago for the QNAP. I guess at the time i was battling with two
> different pm_power_off handlers, so put in this code.
>
>> Also, you are supposed to register a reboot notifier to which you can
>> pass private context:
>
> At the time i wrote the QNAP code, this did not exist. So maybe my
> code is no longer a good example to copy.

Really appreciated, Andrew!
I'll modify this part in next series.

BTW. the private data passing to reboot notifier can be shared to
power-off function as well?
Do you have example?

> https://lkml.org/lkml/2016/12/28/275
>
> As indicated in my patch series, the UART1-attached micro controller
> does a lot more things that just providing reboot, which is why I chose
> to move this code to a MFD driver, as the starting point before adding
> support for LEDs, FAN, PWM, beeper which would be other types of devices.
>
> Is adding support for other peripherals on your TODO as well?

Except reset feature (power-off and reboot), other feature, such as
LEDs / FAN speed / buttons, is managed by user-land program micro-evtd
[0][1].
Since the upstream [1] is not active anymore, Ryan Tandy (in CC) and I
are maintaining it in Debian [0].

[0] https://tracker.debian.org/pkg/micro-evtd
[1] https://sourceforge.net/projects/ppc-evtd

micro-evtd worked well for device-based legacy code, but after DT
conversion, Ryan found the device cannot shutdown properly (reboot is
OK).
That why I created this patch series.
I think for such old hardware and mature user-land program, it doesn't
deserve the effort to implement those again in kernel side.
What do you think?

Cheers,
-- 
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 4096R/6C6ACD6417B3ACB1

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

* [PATCH v3 2/3] DT: bingdings: power: reset: add linkstation-reset doc
  2017-01-03 13:12       ` Andrew Lunn
@ 2017-01-03 14:11         ` Roger Shimizu
  0 siblings, 0 replies; 24+ messages in thread
From: Roger Shimizu @ 2017-01-03 14:11 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Florian, Andrew,

Thanks for your comments!

On Tue, Jan 3, 2017 at 10:12 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> > +
>> > +Required Properties:
>> > +- compatible: Should be "linkstation,power-off"
>> > +- reg: Address and length of the register set for UART1
>>
>> Humm, should we instead have a phandle to the uart1 node?
>
> Probably. Again, this is to do with copying the QNAP driver. I was
> young, new to device tree, and i just did a logical conversion of the
> existing code, and did not at the time understand phandles, etc.

Can you tell me where should I place this document?
or simply remove it?

Cheers,
-- 
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 4096R/6C6ACD6417B3ACB1

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

* [PATCH v3 2/3] DT: bingdings: power: reset: add linkstation-reset doc
  2016-12-27  7:06   ` [PATCH v3 2/3] DT: bingdings: power: reset: add linkstation-reset doc Roger Shimizu
  2017-01-03  5:21     ` Florian Fainelli
@ 2017-01-03 17:09     ` Rob Herring
  2017-01-06 12:18       ` Roger Shimizu
  1 sibling, 1 reply; 24+ messages in thread
From: Rob Herring @ 2017-01-03 17:09 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Dec 27, 2016 at 04:06:10PM +0900, Roger Shimizu wrote:
> Add linkstation-reset doc to describe the newly added
> POWER_RESET_LINKSTATION driver, which controls magic command
> sending to UART1 to power-off Buffalo Linkstation / KuroBox
> and their variants.
> 
> To: Sebastian Reichel <sre@kernel.org>
> To: Rob Herring <robh+dt@kernel.org>
> To: Mark Rutland <mark.rutland@arm.com>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Ryan Tandy <ryan@nardis.ca>
> Cc: linux-pm at vger.kernel.org
> Cc: devicetree at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> 
> Signed-off-by: Roger Shimizu <rogershimizu@gmail.com>
> ---
>  .../bindings/power/reset/linkstation-reset.txt     | 26 ++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/reset/linkstation-reset.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/reset/linkstation-reset.txt b/Documentation/devicetree/bindings/power/reset/linkstation-reset.txt
> new file mode 100644
> index 000000000000..815e340318f3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/linkstation-reset.txt
> @@ -0,0 +1,26 @@
> +* Buffalo Linkstation Reset Driver
> +
> +Power of some Buffalo Linkstation or KuroBox Pro is managed by
> +micro-controller, which connects to UART1. After being fed from UART1
> +by a few magic numbers, the so-called power-off command,
> +the micro-controller will turn power off the device.

This needs to model the uC connected to the UART rather than some node 
that defines only some portion of the functionality. I'm working on 
bindings and proper bus support for this[1], but it's not done yet. 
Though, the binding side is pretty simple.

Rob

[1] https://git.kernel.org/cgit/linux/kernel/git/robh/linux.git/log/?h=serial-bus-v2

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

* [PATCH v3 1/3] power: reset: add linkstation-reset driver
  2017-01-03 14:08         ` Roger Shimizu
@ 2017-01-03 18:39           ` Florian Fainelli
  0 siblings, 0 replies; 24+ messages in thread
From: Florian Fainelli @ 2017-01-03 18:39 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/03/2017 06:08 AM, Roger Shimizu wrote:
> Dear Florian, Andrew,
> 
> Thanks for your email!
> 
> On Tue, Jan 3, 2017 at 2:19 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>> Interestingly, I submitted a patch doing nearly the same thing recently
>> after hacking on a Buffalo Terastation Pro II two days after yours
>> without seeing yours:
>>
>> https://lkml.org/lkml/2016/12/28/273
> 
> Glad to know there's other developer working on linkstation/kurobox platform!
> 
>> Some comments below.
>>
>>> +
>>> +static void __iomem *base;
>>> +static unsigned long tclk;
>>> +static const struct reset_cfg *cfg;
>>
>> How about avoiding the singletons here and pass this down from the
>> platform driver's private data after we (see below) also make use of a
>> reboot notifier?
> 
> I see your patches. Indeed, it's a good idea to avoid the singletons
> and use private data instead.
> 
>>> +static int linkstation_reset_probe(struct platform_device *pdev)
>>> +{
>>> +     struct device_node *np = pdev->dev.of_node;
>>> +     struct resource *res;
>>> +     struct clk *clk;
>>> +     char symname[KSYM_NAME_LEN];
>>> +
>>> +     const struct of_device_id *match =
>>> +             of_match_node(linkstation_reset_of_match_table, np);
>>> +     cfg = match->data;
>>> +
>>> +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +     if (!res) {
>>> +             dev_err(&pdev->dev, "Missing resource");
>>> +             return -EINVAL;
>>> +     }
>>> +
>>> +     base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
>>> +     if (!base) {
>>> +             dev_err(&pdev->dev, "Unable to map resource");
>>> +             return -EINVAL;
>>> +     }
>>> +
>>> +     /* We need to know tclk in order to calculate the UART divisor */
>>> +     clk = devm_clk_get(&pdev->dev, NULL);
>>> +     if (IS_ERR(clk)) {
>>> +             dev_err(&pdev->dev, "Clk missing");
>>> +             return PTR_ERR(clk);
>>> +     }
>>> +
>>> +     tclk = clk_get_rate(clk);
>>
>> Does this work with the Terastation II which has not been converted to
>> DT yet? Is tclk available through the CLK API there?
> 
> I have no idea whether device-based legacy code and make use of power
> reset driver.
> Maybe Sebastian Reichel can offer a comment?
> 
> However, I think Terastation II should convert to DT first.
> If you're willing to test, I can help to provide a dts/dtb.
> (If you use Debian, I can even provide DEB of kernel image and
> flash-kernel patch, which is easy for you to test)
> 
> On Tue, Jan 3, 2017 at 10:09 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>>>> +
>>>> +   /* Check that nothing else has already setup a handler */
>>>> +   if (pm_power_off) {
>>>> +           lookup_symbol_name((ulong)pm_power_off, symname);
>>>> +           dev_err(&pdev->dev,
>>>> +                   "pm_power_off already claimed %p %s",
>>>> +                   pm_power_off, symname);
>>>> +           return -EBUSY;
>>>> +   }
>>>> +   pm_power_off = linkstation_reset;
>>>
>>> That seems a bit complicated, why not just assume that there will be
>>> either this driver used, or not at all?
>>
>> That is probably my fault. This is a copy from code i wrote many years
>> ago for the QNAP. I guess at the time i was battling with two
>> different pm_power_off handlers, so put in this code.
>>
>>> Also, you are supposed to register a reboot notifier to which you can
>>> pass private context:
>>
>> At the time i wrote the QNAP code, this did not exist. So maybe my
>> code is no longer a good example to copy.
> 
> Really appreciated, Andrew!
> I'll modify this part in next series.
> 
> BTW. the private data passing to reboot notifier can be shared to
> power-off function as well?
> Do you have example?
> 
>> https://lkml.org/lkml/2016/12/28/275
>>
>> As indicated in my patch series, the UART1-attached micro controller
>> does a lot more things that just providing reboot, which is why I chose
>> to move this code to a MFD driver, as the starting point before adding
>> support for LEDs, FAN, PWM, beeper which would be other types of devices.
>>
>> Is adding support for other peripherals on your TODO as well?
> 
> Except reset feature (power-off and reboot), other feature, such as
> LEDs / FAN speed / buttons, is managed by user-land program micro-evtd
> [0][1].
> Since the upstream [1] is not active anymore, Ryan Tandy (in CC) and I
> are maintaining it in Debian [0].
> 
> [0] https://tracker.debian.org/pkg/micro-evtd
> [1] https://sourceforge.net/projects/ppc-evtd
> 
> micro-evtd worked well for device-based legacy code, but after DT
> conversion, Ryan found the device cannot shutdown properly (reboot is
> OK).
> That why I created this patch series.
> I think for such old hardware and mature user-land program, it doesn't
> deserve the effort to implement those again in kernel side.
> What do you think?

An argument could be made that these are hardware peripherals, and so
the kernel is the best place to abstract support for that, and present
you with the standard device drive model for such kind of peripherals.

We don't have to do everything at the same time, so first things first,
get the reboot working, have me convert the Terastation over to Device
Tree and then we can decide what to do with these additional peripherals.

Thanks!
-- 
Florian

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

* [PATCH v3 2/3] DT: bingdings: power: reset: add linkstation-reset doc
  2017-01-03 17:09     ` Rob Herring
@ 2017-01-06 12:18       ` Roger Shimizu
  0 siblings, 0 replies; 24+ messages in thread
From: Roger Shimizu @ 2017-01-06 12:18 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Rob,

Thanks for your comments!

On Wed, Jan 4, 2017 at 2:09 AM, Rob Herring <robh@kernel.org> wrote:
>
> This needs to model the uC connected to the UART rather than some node
> that defines only some portion of the functionality. I'm working on
> bindings and proper bus support for this[1], but it's not done yet.
> Though, the binding side is pretty simple.
>
> Rob
>
> [1] https://git.kernel.org/cgit/linux/kernel/git/robh/linux.git/log/?h=serial-bus-v2

Actually I have limited knowledge on DT binding.
I think it should be OK to remove the 2/3 patch (this thread) in my
next series, and I'll do my homework when you finish it.
Thank you!

Cheers,
-- 
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 4096R/6C6ACD6417B3ACB1

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

* [PATCH v4 0/2] make kurobox-pro be able to shutdown after device-tree migration
  2016-12-27  7:06 ` [PATCH v3 0/3] make kurobox-pro be able to shutdown after device-tree migration Roger Shimizu
                     ` (2 preceding siblings ...)
  2016-12-27  7:06   ` [PATCH v3 3/3] ARM: DT: add power-off support to linkstation lsgl and kuroboxpro Roger Shimizu
@ 2017-01-07 15:04   ` Roger Shimizu
  2017-01-07 15:04     ` [PATCH v4 1/2] power: reset: add linkstation-reset driver Roger Shimizu
  2017-01-07 15:04     ` [PATCH v4 2/2] ARM: DT: add power-off support to linkstation lsgl and kuroboxpro Roger Shimizu
  3 siblings, 2 replies; 24+ messages in thread
From: Roger Shimizu @ 2017-01-07 15:04 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Kernel Maintainers,

Kurobox-Pro (and variants) need more commands sending to UART1 to shutdown.
So here I make this patch series to let current qnap-poweroff implementation
be able to handle such case.

Here I give 3 patches, for 3 different kernel subsystems respectivelay:
  - Patch 1/3: System reset/shutdown driver
  - Patch 2/3: Open firmware and flattened device tree bindings
  - Patch 3/3: ARM/Marvell Dove/MV78xx0/Orion SOC support

Ryan Tandy, the issue reporter, and I have already tested those changes on
Linkstation/KuroBoxPro devices, and confirmed it's working well.

Merry Xmax and look forward to your feedback!

Changes:
  v0 => v1:
  - Update 0003 to split kuroboxpro related code into kuroboxpro-common.c
  v1 => v2:
  - Split off linkstation/kuroboxpro related code to linkstation-reset.c
    Because linkstation before kuroboxpro also need this driver to power
    off properly. It's more proper to call it linkstation driver.
  v2 => v3:
  - Split off devicetree bindings doc into a separate patch.
  - Add device-tree patch to make Linkstation LS-GL and KuroBox Pro to
    use the newly created linkstation-reset driver.
  - Remove the unused one-byte command sending case in linkstation-reset
    driver.
  v3 => v4:
  - Reduce the global singleton variables by moving to structure, which makes
    the driver easy to support reboot function in the future.  Thanks to
    Florian Fainelli.
  - Simplify the power_off function registration. Thanks to Florian Fainelli.
  - Drop DT binding doc.

Cheers,
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 4096R/6C6ACD6417B3ACB1

Roger Shimizu (2):
  power: reset: add linkstation-reset driver
  ARM: DT: add power-off support to linkstation lsgl and kuroboxpro

 arch/arm/boot/dts/orion5x-kuroboxpro.dts         |   8 +
 arch/arm/boot/dts/orion5x-linkstation-lsgl.dts   |  10 +
 arch/arm/boot/dts/orion5x-linkstation-lswtgl.dts |   4 +
 arch/arm/boot/dts/orion5x-linkstation.dtsi       |   4 -
 drivers/power/reset/Kconfig                      |  10 +
 drivers/power/reset/Makefile                     |   1 +
 drivers/power/reset/linkstation-reset.c          | 270 +++++++++++++++++++++++
 7 files changed, 303 insertions(+), 4 deletions(-)
 create mode 100644 drivers/power/reset/linkstation-reset.c

-- 
2.11.0

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

* [PATCH v4 1/2] power: reset: add linkstation-reset driver
  2017-01-07 15:04   ` [PATCH v4 0/2] make kurobox-pro be able to shutdown after device-tree migration Roger Shimizu
@ 2017-01-07 15:04     ` Roger Shimizu
  2017-01-08 17:02       ` Ryan Tandy
  2017-01-18 12:08       ` Roger Shimizu
  2017-01-07 15:04     ` [PATCH v4 2/2] ARM: DT: add power-off support to linkstation lsgl and kuroboxpro Roger Shimizu
  1 sibling, 2 replies; 24+ messages in thread
From: Roger Shimizu @ 2017-01-07 15:04 UTC (permalink / raw)
  To: linux-arm-kernel

Buffalo Linkstation / KuroBox and their variants need magic command
sending to UART1 to power-off.

Power driver linkstation-reset implements the magic command and I/O
routine, which come from files listed below:
  - arch/arm/mach-orion5x/kurobox_pro-setup.c
  - arch/arm/mach-orion5x/terastation_pro2-setup.c

To: Sebastian Reichel <sre@kernel.org>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Martin Michlmayr <tbm@cyrius.com>
Cc: Sylver Bruneau <sylver.bruneau@googlemail.com>
Cc: Herbert Valerio Riedel <hvr@gnu.org>
Cc: Ryan Tandy <ryan@nardis.ca>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: linux-pm at vger.kernel.org
Cc: devicetree at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org

Reported-by: Ryan Tandy <ryan@nardis.ca>
Tested-by: Ryan Tandy <ryan@nardis.ca>
Signed-off-by: Roger Shimizu <rogershimizu@gmail.com>
---
 drivers/power/reset/Kconfig             |  10 ++
 drivers/power/reset/Makefile            |   1 +
 drivers/power/reset/linkstation-reset.c | 270 ++++++++++++++++++++++++++++++++
 3 files changed, 281 insertions(+)
 create mode 100644 drivers/power/reset/linkstation-reset.c

diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index c74c3f67b8da..77c44cad7ece 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -98,6 +98,16 @@ config POWER_RESET_IMX
 	  say N here or disable in dts to make sure pm_power_off never be
 	  overwrote wrongly by this driver.
 
+config POWER_RESET_LINKSTATION
+	bool "Buffalo Linkstation and its variants reset driver"
+	depends on OF_GPIO && PLAT_ORION
+	help
+	  This driver supports power off Buffalo Linkstation / KuroBox Pro
+	  NAS and their variants by sending commands to the micro-controller
+	  which controls the main power.
+
+	  Say Y if you have a Buffalo Linkstation / KuroBox Pro NAS.
+
 config POWER_RESET_MSM
 	bool "Qualcomm MSM power-off driver"
 	depends on ARCH_QCOM
diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
index 1be307c7fc25..692ba6417cfb 100644
--- a/drivers/power/reset/Makefile
+++ b/drivers/power/reset/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_POWER_RESET_GPIO) += gpio-poweroff.o
 obj-$(CONFIG_POWER_RESET_GPIO_RESTART) += gpio-restart.o
 obj-$(CONFIG_POWER_RESET_HISI) += hisi-reboot.o
 obj-$(CONFIG_POWER_RESET_IMX) += imx-snvs-poweroff.o
+obj-$(CONFIG_POWER_RESET_LINKSTATION) += linkstation-reset.o
 obj-$(CONFIG_POWER_RESET_MSM) += msm-poweroff.o
 obj-$(CONFIG_POWER_RESET_LTC2952) += ltc2952-poweroff.o
 obj-$(CONFIG_POWER_RESET_QNAP) += qnap-poweroff.o
diff --git a/drivers/power/reset/linkstation-reset.c b/drivers/power/reset/linkstation-reset.c
new file mode 100644
index 000000000000..c191b7671076
--- /dev/null
+++ b/drivers/power/reset/linkstation-reset.c
@@ -0,0 +1,270 @@
+/*
+ * Buffalo Linkstation power reset driver.
+ * It may also be used on following devices:
+ *  - KuroBox Pro
+ *  - Buffalo Linkstation Pro (LS-GL)
+ *  - Buffalo Terastation Pro II/Live
+ *  - Buffalo Linkstation Duo (LS-WTGL)
+ *  - Buffalo Linkstation Mini (LS-WSGL)
+ *
+ * Copyright (C) 2016  Roger Shimizu <rogershimizu@gmail.com>
+ *
+ * Based on the code from:
+ *
+ * Copyright (C) 2012  Andrew Lunn <andrew@lunn.ch>
+ * Copyright (C) 2009  Martin Michlmayr <tbm@cyrius.com>
+ * Copyright (C) 2008  Byron Bradley <byron.bbradley@gmail.com>
+ * Copyright (C) 2008  Sylver Bruneau <sylver.bruneau@googlemail.com>
+ * Copyright (C) 2007  Herbert Valerio Riedel <hvr@gnu.org>
+ *
+ * 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/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/serial_reg.h>
+#include <linux/kallsyms.h>
+#include <linux/of.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+
+#define UART1_REG(x)	((UART_##x) << 2)
+#define MICON_CMD_SIZE	4
+
+/* 4-byte magic hello command to UART1-attached microcontroller */
+static const unsigned char linkstation_micon_magic[] = {
+	0x1b,
+	0x00,
+	0x07,
+	0x00
+};
+
+/* for each row, first byte is the size of command */
+static const unsigned char linkstation_power_off_cmd[][MICON_CMD_SIZE] = {
+	{ 3,	0x01, 0x35, 0x00},
+	{ 2,	0x00, 0x0c},
+	{ 2,	0x00, 0x06},
+	{}
+};
+
+struct reset_cfg {
+	u32 baud;
+	const unsigned char *magic;
+	const unsigned char (*cmd)[MICON_CMD_SIZE];
+};
+
+struct device_cfg {
+	const struct device *dev;
+	void __iomem *base;
+	unsigned long tclk;
+	const struct reset_cfg *cfg;
+};
+
+static const struct reset_cfg linkstation_power_off_cfg = {
+	.baud = 38400,
+	.magic = linkstation_micon_magic,
+	.cmd = linkstation_power_off_cmd,
+};
+
+static const struct of_device_id linkstation_reset_of_match_table[] = {
+	{ .compatible = "linkstation,power-off",
+	  .data = &linkstation_power_off_cfg,
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, linkstation_reset_of_match_table);
+
+static int uart1_micon_read(const struct device_cfg *dev, unsigned char *buf, int count)
+{
+	int i;
+	int timeout;
+
+	for (i = 0; i < count; i++) {
+		timeout = 10;
+
+		while (!(readl(dev->base + UART1_REG(LSR)) & UART_LSR_DR)) {
+			if (--timeout == 0)
+				break;
+			udelay(1000);
+		}
+
+		if (timeout == 0)
+			break;
+		buf[i] = readl(dev->base + UART1_REG(RX));
+	}
+
+	/* return read bytes */
+	return i;
+}
+
+static int uart1_micon_write(const struct device_cfg *dev, const unsigned char *buf, int count)
+{
+	int i = 0;
+
+	while (count--) {
+		while (!(readl(dev->base + UART1_REG(LSR)) & UART_LSR_THRE))
+			barrier();
+		writel(buf[i++], dev->base + UART1_REG(TX));
+	}
+
+	return 0;
+}
+
+int uart1_micon_send(const struct device_cfg *dev, const unsigned char *data, int count)
+{
+	int i;
+	unsigned char checksum = 0;
+	unsigned char recv_buf[40];
+	unsigned char send_buf[40];
+	unsigned char correct_ack[3];
+	int retry = 2;
+
+	/* Generate checksum */
+	for (i = 0; i < count; i++)
+		checksum -=  data[i];
+
+	do {
+		/* Send data */
+		uart1_micon_write(dev, data, count);
+
+		/* send checksum */
+		uart1_micon_write(dev, &checksum, 1);
+
+		if (uart1_micon_read(dev, recv_buf, sizeof(recv_buf)) <= 3) {
+			dev_err(dev->dev, ">%s: receive failed.\n", __func__);
+
+			/* send preamble to clear the receive buffer */
+			memset(&send_buf, 0xff, sizeof(send_buf));
+			uart1_micon_write(dev, send_buf, sizeof(send_buf));
+
+			/* make dummy reads */
+			mdelay(100);
+			uart1_micon_read(dev, recv_buf, sizeof(recv_buf));
+		} else {
+			/* Generate expected ack */
+			correct_ack[0] = 0x01;
+			correct_ack[1] = data[1];
+			correct_ack[2] = 0x00;
+
+			/* checksum Check */
+			if ((recv_buf[0] + recv_buf[1] + recv_buf[2] +
+			     recv_buf[3]) & 0xFF) {
+				dev_err(dev->dev, ">%s: Checksum Error : "
+					"Received data[%02x, %02x, %02x, %02x]"
+					"\n", __func__, recv_buf[0],
+					recv_buf[1], recv_buf[2], recv_buf[3]);
+			} else {
+				/* Check Received Data */
+				if (correct_ack[0] == recv_buf[0] &&
+				    correct_ack[1] == recv_buf[1] &&
+				    correct_ack[2] == recv_buf[2]) {
+					/* Interval for next command */
+					mdelay(10);
+
+					/* Receive ACK */
+					return 0;
+				}
+			}
+			/* Received NAK or illegal Data */
+			dev_err(dev->dev, ">%s: Error : NAK or Illegal Data "
+					"Received\n", __func__);
+		}
+	} while (retry--);
+
+	/* Interval for next command */
+	mdelay(10);
+
+	return -1;
+}
+
+static struct device_cfg reset;
+
+static void linkstation_reset(void)
+{
+	const unsigned divisor = ((reset.tclk + (8 * reset.cfg->baud)) / (16 * reset.cfg->baud));
+	int i;
+
+	pr_err("%s: triggering power-off...\n", __func__);
+
+	/* hijack UART1 and reset into sane state */
+	writel(0x83, reset.base + UART1_REG(LCR));
+	writel(divisor & 0xff, reset.base + UART1_REG(DLL));
+	writel((divisor >> 8) & 0xff, reset.base + UART1_REG(DLM));
+	writel(reset.cfg->magic[0], reset.base + UART1_REG(LCR));
+	writel(reset.cfg->magic[1], reset.base + UART1_REG(IER));
+	writel(reset.cfg->magic[2], reset.base + UART1_REG(FCR));
+	writel(reset.cfg->magic[3], reset.base + UART1_REG(MCR));
+
+	/* send the power-off command to PIC */
+	for(i = 0; reset.cfg->cmd[i][0] > 0; i ++) {
+		/* [0] is size of the command; command starts from [1] */
+		uart1_micon_send(&reset, &(reset.cfg->cmd[i][1]), reset.cfg->cmd[i][0]);
+	}
+}
+
+static int linkstation_reset_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct resource *res;
+	struct clk *clk;
+
+	const struct of_device_id *match =
+		of_match_node(linkstation_reset_of_match_table, np);
+	reset.cfg = match->data;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "Missing resource");
+		return -EINVAL;
+	}
+
+	reset.dev = &pdev->dev;
+	reset.base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+	if (!reset.base) {
+		dev_err(reset.dev, "Unable to map resource");
+		return -EINVAL;
+	}
+
+	/* We need to know tclk in order to calculate the UART divisor */
+	clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(clk)) {
+		dev_err(reset.dev, "Clk missing");
+		return PTR_ERR(clk);
+	}
+
+	reset.tclk = clk_get_rate(clk);
+
+	/* Check that nothing else has already setup a handler */
+	if (!pm_power_off) {
+		pm_power_off = linkstation_reset;
+	}
+
+	return 0;
+}
+
+static int linkstation_reset_remove(struct platform_device *pdev)
+{
+	if (pm_power_off == linkstation_reset)
+		pm_power_off = NULL;
+	return 0;
+}
+
+static struct platform_driver linkstation_reset_driver = {
+	.probe	= linkstation_reset_probe,
+	.remove	= linkstation_reset_remove,
+	.driver	= {
+		.name	= "linkstation_reset",
+		.of_match_table = of_match_ptr(linkstation_reset_of_match_table),
+	},
+};
+
+module_platform_driver(linkstation_reset_driver);
+
+MODULE_AUTHOR("Roger Shimizu <rogershimizu@gmail.com>");
+MODULE_DESCRIPTION("Linkstation Reset driver");
+MODULE_LICENSE("GPL v2");
-- 
2.11.0

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

* [PATCH v4 2/2] ARM: DT: add power-off support to linkstation lsgl and kuroboxpro
  2017-01-07 15:04   ` [PATCH v4 0/2] make kurobox-pro be able to shutdown after device-tree migration Roger Shimizu
  2017-01-07 15:04     ` [PATCH v4 1/2] power: reset: add linkstation-reset driver Roger Shimizu
@ 2017-01-07 15:04     ` Roger Shimizu
  1 sibling, 0 replies; 24+ messages in thread
From: Roger Shimizu @ 2017-01-07 15:04 UTC (permalink / raw)
  To: linux-arm-kernel

A few models of Linkstation / KuroBox has micro-controller which
controls the power (as well as FAN, but not related here), while
others not. So remove the restart-poweroff driver in linkstation
common dtsi file, and specify the proper power driver in dts
of each device.

Devices need micro-controler to power-off:
  - Linkstation LS-GL
  - KuroBox Pro
Device continues using original restart-poweroff driver:
  - Linkstation LS-WTGL

To: Jason Cooper <jason@lakedaemon.net>
To: Andrew Lunn <andrew@lunn.ch>
To: Gregory Clement <gregory.clement@free-electrons.com>
To: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Ryan Tandy <ryan@nardis.ca>
Cc: linux-pm at vger.kernel.org
Cc: devicetree at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org

Reported-by: Ryan Tandy <ryan@nardis.ca>
Tested-by: Ryan Tandy <ryan@nardis.ca>
Signed-off-by: Roger Shimizu <rogershimizu@gmail.com>
---
 arch/arm/boot/dts/orion5x-kuroboxpro.dts         |  8 ++++++++
 arch/arm/boot/dts/orion5x-linkstation-lsgl.dts   | 10 ++++++++++
 arch/arm/boot/dts/orion5x-linkstation-lswtgl.dts |  4 ++++
 arch/arm/boot/dts/orion5x-linkstation.dtsi       |  4 ----
 4 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/orion5x-kuroboxpro.dts b/arch/arm/boot/dts/orion5x-kuroboxpro.dts
index 1a672b098d0b..aba38d802bda 100644
--- a/arch/arm/boot/dts/orion5x-kuroboxpro.dts
+++ b/arch/arm/boot/dts/orion5x-kuroboxpro.dts
@@ -60,6 +60,14 @@
 				 <MBUS_ID(0x09, 0x00) 0 0xf2200000 0x800>,
 				 <MBUS_ID(0x01, 0x0f) 0 0xf4000000 0x40000>,
 				 <MBUS_ID(0x01, 0x1e) 0 0xfc000000 0x1000000>;
+
+		internal-regs {
+			power_off {
+				compatible = "linkstation,power-off";
+				reg = <0x12100 0x100>;
+				clocks = <&core_clk 0>;
+			};
+		};
 	};
 
 	memory { /* 128 MB */
diff --git a/arch/arm/boot/dts/orion5x-linkstation-lsgl.dts b/arch/arm/boot/dts/orion5x-linkstation-lsgl.dts
index 51dc734cd5b9..370fc17a6dd9 100644
--- a/arch/arm/boot/dts/orion5x-linkstation-lsgl.dts
+++ b/arch/arm/boot/dts/orion5x-linkstation-lsgl.dts
@@ -56,6 +56,16 @@
 	model = "Buffalo Linkstation Pro/Live";
 	compatible = "buffalo,lsgl", "marvell,orion5x-88f5182", "marvell,orion5x";
 
+	soc {
+		internal-regs {
+			power_off {
+				compatible = "linkstation,power-off";
+				reg = <0x12100 0x100>;
+				clocks = <&core_clk 0>;
+			};
+		};
+	};
+
 	memory { /* 128 MB */
 		device_type = "memory";
 		reg = <0x00000000 0x8000000>;
diff --git a/arch/arm/boot/dts/orion5x-linkstation-lswtgl.dts b/arch/arm/boot/dts/orion5x-linkstation-lswtgl.dts
index 0eead400f427..571a71f5b7ad 100644
--- a/arch/arm/boot/dts/orion5x-linkstation-lswtgl.dts
+++ b/arch/arm/boot/dts/orion5x-linkstation-lswtgl.dts
@@ -59,6 +59,10 @@
 		reg = <0x00000000 0x4000000>;
 	};
 
+	restart_poweroff {
+		compatible = "restart-poweroff";
+	};
+
 	gpio_keys {
 		power-on-switch {
 			gpios = <&gpio0 8 GPIO_ACTIVE_LOW>;
diff --git a/arch/arm/boot/dts/orion5x-linkstation.dtsi b/arch/arm/boot/dts/orion5x-linkstation.dtsi
index ed456ab35fd8..40770a8d4b36 100644
--- a/arch/arm/boot/dts/orion5x-linkstation.dtsi
+++ b/arch/arm/boot/dts/orion5x-linkstation.dtsi
@@ -57,10 +57,6 @@
 				 <MBUS_ID(0x01, 0x0f) 0 0xf4000000 0x40000>;
 	};
 
-	restart_poweroff {
-		compatible = "restart-poweroff";
-	};
-
 	regulators {
 		compatible = "simple-bus";
 		#address-cells = <1>;
-- 
2.11.0

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

* [PATCH v4 1/2] power: reset: add linkstation-reset driver
  2017-01-07 15:04     ` [PATCH v4 1/2] power: reset: add linkstation-reset driver Roger Shimizu
@ 2017-01-08 17:02       ` Ryan Tandy
  2017-01-09  3:31         ` Roger Shimizu
  2017-01-18 12:08       ` Roger Shimizu
  1 sibling, 1 reply; 24+ messages in thread
From: Ryan Tandy @ 2017-01-08 17:02 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Roger,

On Sun, Jan 08, 2017 at 12:04:50AM +0900, Roger Shimizu wrote:
>+config POWER_RESET_LINKSTATION
>+	bool "Buffalo Linkstation and its variants reset driver"
>+	depends on OF_GPIO && PLAT_ORION
>+	help
>+	  This driver supports power off Buffalo Linkstation / KuroBox Pro
>+	  NAS and their variants by sending commands to the micro-controller
>+	  which controls the main power.
>+
>+	  Say Y if you have a Buffalo Linkstation / KuroBox Pro NAS.
>+

Would it make sense to mention something about these being the 
ARM9/orion5x Linkstations? If I understand correctly, the older PPC 
Linkstations have a single-byte poweroff command. (Maybe they could even 
be supported by qnap-poweroff.)

See arch/powerpc/platforms/embedded6xx/linkstation.c around line 123, 
and arch/powerpc/platforms/embedded6xx/ls_uart.c.

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

* [PATCH v4 1/2] power: reset: add linkstation-reset driver
  2017-01-08 17:02       ` Ryan Tandy
@ 2017-01-09  3:31         ` Roger Shimizu
  2017-01-09  5:43           ` Ryan Tandy
  0 siblings, 1 reply; 24+ messages in thread
From: Roger Shimizu @ 2017-01-09  3:31 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Ryan,

Thanks for your comments!

On Mon, Jan 9, 2017 at 2:02 AM, Ryan Tandy <ryan@nardis.ca> wrote:
> On Sun, Jan 08, 2017 at 12:04:50AM +0900, Roger Shimizu wrote:
>>
>> +config POWER_RESET_LINKSTATION
>> +       bool "Buffalo Linkstation and its variants reset driver"
>> +       depends on OF_GPIO && PLAT_ORION
>> +       help
>> +         This driver supports power off Buffalo Linkstation / KuroBox Pro
>> +         NAS and their variants by sending commands to the
>> micro-controller
>> +         which controls the main power.
>
> Would it make sense to mention something about these being the ARM9/orion5x
> Linkstations? If I understand correctly, the older PPC Linkstations have a
> single-byte poweroff command. (Maybe they could even be supported by
> qnap-poweroff.)
>
> See arch/powerpc/platforms/embedded6xx/linkstation.c around line 123, and
> arch/powerpc/platforms/embedded6xx/ls_uart.c.

This driver, linkstation-reset, can also handle PPC Linkstation after
it's converted to DT.
I already considered this and mentioned in previous reply [0].

[0] http://marc.info/?l=linux-pm&m=148216908031283

Cheers,
-- 
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 4096R/6C6ACD6417B3ACB1

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

* [PATCH v4 1/2] power: reset: add linkstation-reset driver
  2017-01-09  3:31         ` Roger Shimizu
@ 2017-01-09  5:43           ` Ryan Tandy
  0 siblings, 0 replies; 24+ messages in thread
From: Ryan Tandy @ 2017-01-09  5:43 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jan 09, 2017 at 12:31:44PM +0900, Roger Shimizu wrote:
>This driver, linkstation-reset, can also handle PPC Linkstation after
>it's converted to DT.
>I already considered this and mentioned in previous reply [0].

OK. I was thinking of Sebastian's earlier comment:

On Wed, Dec 21, 2016 at 04:59:29PM +0100, Sebastian Reichel wrote:
>These models can just be added to qnap-poweroff, which handles
>exactly this special case as far as I can see.

and forgot that you planned to handle it in this driver. Sorry for the 
noise.

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

* [PATCH v4 1/2] power: reset: add linkstation-reset driver
  2017-01-07 15:04     ` [PATCH v4 1/2] power: reset: add linkstation-reset driver Roger Shimizu
  2017-01-08 17:02       ` Ryan Tandy
@ 2017-01-18 12:08       ` Roger Shimizu
  2017-01-19  4:43         ` Sebastian Reichel
  1 sibling, 1 reply; 24+ messages in thread
From: Roger Shimizu @ 2017-01-18 12:08 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Sebastian,

On Sun, Jan 8, 2017 at 12:04 AM, Roger Shimizu <rogershimizu@gmail.com> wrote:
> Buffalo Linkstation / KuroBox and their variants need magic command
> sending to UART1 to power-off.
>
> Power driver linkstation-reset implements the magic command and I/O
> routine, which come from files listed below:
>   - arch/arm/mach-orion5x/kurobox_pro-setup.c
>   - arch/arm/mach-orion5x/terastation_pro2-setup.c

I think there's not much concern regarding to this series.
Could you kindly help to apply this patch?
Thank you!

Cheers,
-- 
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 4096R/6C6ACD6417B3ACB1

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

* [PATCH v4 1/2] power: reset: add linkstation-reset driver
  2017-01-18 12:08       ` Roger Shimizu
@ 2017-01-19  4:43         ` Sebastian Reichel
  2017-01-26 15:28           ` Gregory CLEMENT
  0 siblings, 1 reply; 24+ messages in thread
From: Sebastian Reichel @ 2017-01-19  4:43 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Roger,

On Wed, Jan 18, 2017 at 09:08:13PM +0900, Roger Shimizu wrote:
> On Sun, Jan 8, 2017 at 12:04 AM, Roger Shimizu <rogershimizu@gmail.com> wrote:
> > Buffalo Linkstation / KuroBox and their variants need magic command
> > sending to UART1 to power-off.
> >
> > Power driver linkstation-reset implements the magic command and I/O
> > routine, which come from files listed below:
> >   - arch/arm/mach-orion5x/kurobox_pro-setup.c
> >   - arch/arm/mach-orion5x/terastation_pro2-setup.c
> 
> I think there's not much concern regarding to this series.
> Could you kindly help to apply this patch?

Well you dropped the DT binding, but still introduce new DT
properties. Since they are not documented without the binding
I won't merge this. DT binding documentation is not optional.
In other words: NAK on the binding effectively means NAK on
the driver in its current state.

If you want to see this merged rebase it to Rob's generic
serial bindings [0] and help to support him getting everything
into mainline ASAP. Currently the feedback seems to be quite
positive, so I hope to see it merged for 4.11.

[0] https://lwn.net/Articles/711794/ (current proposal from 2 days ago)

-- Sebastian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170119/ef6a2441/attachment.sig>

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

* [PATCH v4 1/2] power: reset: add linkstation-reset driver
  2017-01-19  4:43         ` Sebastian Reichel
@ 2017-01-26 15:28           ` Gregory CLEMENT
  2017-01-26 15:33             ` Roger Shimizu
  0 siblings, 1 reply; 24+ messages in thread
From: Gregory CLEMENT @ 2017-01-26 15:28 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Roger,
 
 On jeu., janv. 19 2017, Sebastian Reichel <sre@kernel.org> wrote:

> Hi Roger,
>
> On Wed, Jan 18, 2017 at 09:08:13PM +0900, Roger Shimizu wrote:
>> On Sun, Jan 8, 2017 at 12:04 AM, Roger Shimizu <rogershimizu@gmail.com> wrote:
>> > Buffalo Linkstation / KuroBox and their variants need magic command
>> > sending to UART1 to power-off.
>> >
>> > Power driver linkstation-reset implements the magic command and I/O
>> > routine, which come from files listed below:
>> >   - arch/arm/mach-orion5x/kurobox_pro-setup.c
>> >   - arch/arm/mach-orion5x/terastation_pro2-setup.c
>> 
>> I think there's not much concern regarding to this series.
>> Could you kindly help to apply this patch?
>
> Well you dropped the DT binding, but still introduce new DT
> properties. Since they are not documented without the binding
> I won't merge this. DT binding documentation is not optional.
> In other words: NAK on the binding effectively means NAK on
> the driver in its current state.
>
> If you want to see this merged rebase it to Rob's generic
> serial bindings [0] and help to support him getting everything
> into mainline ASAP. Currently the feedback seems to be quite
> positive, so I hope to see it merged for 4.11.
>
> [0] https://lwn.net/Articles/711794/ (current proposal from 2 days
> ago)

What is te status for your series?

Do you plan sending a new version soon?

Gregory


>
> -- Sebastian
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

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

* [PATCH v4 1/2] power: reset: add linkstation-reset driver
  2017-01-26 15:28           ` Gregory CLEMENT
@ 2017-01-26 15:33             ` Roger Shimizu
  2017-01-27  9:15               ` Gregory CLEMENT
  0 siblings, 1 reply; 24+ messages in thread
From: Roger Shimizu @ 2017-01-26 15:33 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Gregory,

Thanks for contacting!

On Fri, Jan 27, 2017 at 12:28 AM, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
>
> What is te status for your series?
>
> Do you plan sending a new version soon?

I'm still trying/working on it, but seems won't finish it very soon.

Cheers,
-- 
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 4096R/6C6ACD6417B3ACB1

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

* [PATCH v4 1/2] power: reset: add linkstation-reset driver
  2017-01-26 15:33             ` Roger Shimizu
@ 2017-01-27  9:15               ` Gregory CLEMENT
  0 siblings, 0 replies; 24+ messages in thread
From: Gregory CLEMENT @ 2017-01-27  9:15 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Roger,
 
 On jeu., janv. 26 2017, Roger Shimizu <rogershimizu@gmail.com> wrote:

> Dear Gregory,
>
> Thanks for contacting!
>
> On Fri, Jan 27, 2017 at 12:28 AM, Gregory CLEMENT
> <gregory.clement@free-electrons.com> wrote:
>>
>> What is te status for your series?
>>
>> Do you plan sending a new version soon?
>
> I'm still trying/working on it, but seems won't finish it very soon.

OK it is not a problem for me, but just keep in mind that the merge
window for arm-soc will be close soon.  That means that the dt part
could be delayed to 4.12.

Gregory


>
> Cheers,
> -- 
> Roger Shimizu, GMT +9 Tokyo
> PGP/GPG: 4096R/6C6ACD6417B3ACB1

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

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

end of thread, other threads:[~2017-01-27  9:15 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20161216100501.18173-1-rogershimizu@gmail.com>
2016-12-27  7:06 ` [PATCH v3 0/3] make kurobox-pro be able to shutdown after device-tree migration Roger Shimizu
2016-12-27  7:06   ` [PATCH v3 1/3] power: reset: add linkstation-reset driver Roger Shimizu
2017-01-03  5:19     ` Florian Fainelli
2017-01-03 13:09       ` Andrew Lunn
2017-01-03 14:08         ` Roger Shimizu
2017-01-03 18:39           ` Florian Fainelli
2016-12-27  7:06   ` [PATCH v3 2/3] DT: bingdings: power: reset: add linkstation-reset doc Roger Shimizu
2017-01-03  5:21     ` Florian Fainelli
2017-01-03 13:12       ` Andrew Lunn
2017-01-03 14:11         ` Roger Shimizu
2017-01-03 17:09     ` Rob Herring
2017-01-06 12:18       ` Roger Shimizu
2016-12-27  7:06   ` [PATCH v3 3/3] ARM: DT: add power-off support to linkstation lsgl and kuroboxpro Roger Shimizu
2017-01-07 15:04   ` [PATCH v4 0/2] make kurobox-pro be able to shutdown after device-tree migration Roger Shimizu
2017-01-07 15:04     ` [PATCH v4 1/2] power: reset: add linkstation-reset driver Roger Shimizu
2017-01-08 17:02       ` Ryan Tandy
2017-01-09  3:31         ` Roger Shimizu
2017-01-09  5:43           ` Ryan Tandy
2017-01-18 12:08       ` Roger Shimizu
2017-01-19  4:43         ` Sebastian Reichel
2017-01-26 15:28           ` Gregory CLEMENT
2017-01-26 15:33             ` Roger Shimizu
2017-01-27  9:15               ` Gregory CLEMENT
2017-01-07 15:04     ` [PATCH v4 2/2] ARM: DT: add power-off support to linkstation lsgl and kuroboxpro Roger Shimizu

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).