Skip to contentSkip to conversation

make kurobox-pro be able to shutdown after device-tree migration

PATCH In review Roger Shimizu 9 participants 44 messages Dec 5, 2016 – Jan 27, 2017 2 messages not in the archive

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover make kurobox-pro be able to shutdown after device-tree migration

Dear Sebastian,

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.

I already tested this change on Kurobox-Pro and Linkstation LS-GL devices, with a modified device-tree file. (Previous device-tree of kurobox-pro invokes restart-poweroff, so it simply restarts.)

Thank you and look forward to your feedback!

Cheers,
Roger

Roger Shimizu (3):
power: reset: make qnap-poweroff flexible on hello magic command to
uart1
power: reset: make qnap-poweroff flexible on length of power-off
command
power: reset: make qnap-poweroff support kurobox-pro

 .../bindings/power/reset/qnap-poweroff.txt         |   8 +-
 drivers/power/reset/qnap-poweroff.c                | 186 ++++++++++++++++++++-
 2 files changed, 184 insertions(+), 10 deletions(-)
Signature · 1 line
2.10.2
#1 lore raw

PATCH 1/3 power: reset: make qnap-poweroff flexible on hello magic command to uart1

Current hello magic command sending to UART1 is fixed in qnap_power_off().

In order to support more devices, it's necessary to make hello magic command as an argument in device configuration.

---
 drivers/power/reset/qnap-poweroff.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)
drivers/power/reset/qnap-poweroff.c +16 -4
index 2789a61..47ed120 100644
@@ -2,6 +2,7 @@
  * QNAP Turbo NAS Board power off. Can also be used on Synology devices.
  *
  * Copyright (C) 2012 Andrew Lunn <[email protected]>
+ * Copyright (C) 2016 Roger Shimizu <[email protected]>
  *
  * Based on the code from:
  *
@@ -25,18 +26,29 @@
 
 #define UART1_REG(x)	(base + ((UART_##x) << 2))
 
+/* 4-byte magic hello command to UART1-attached microcontroller */
+static const unsigned char qnap_micon_magic[] = {
+	0x03,
+	0x00,
+	0x00,
+	0x00
+};
+
 struct power_off_cfg {
 	u32 baud;
+	const unsigned char *magic;
 	char cmd;
 };
 
 static const struct power_off_cfg qnap_power_off_cfg = {
 	.baud = 19200,
+	.magic = qnap_micon_magic,
 	.cmd = 'A',
 };
 
 static const struct power_off_cfg synology_power_off_cfg = {
 	.baud = 9600,
+	.magic = qnap_micon_magic,
 	.cmd = '1',
 };
 
@@ -65,10 +77,10 @@ static void qnap_power_off(void)
 	writel(0x83, UART1_REG(LCR));
 	writel(divisor & 0xff, UART1_REG(DLL));
 	writel((divisor >> 8) & 0xff, UART1_REG(DLM));
-	writel(0x03, UART1_REG(LCR));
-	writel(0x00, UART1_REG(IER));
-	writel(0x00, UART1_REG(FCR));
-	writel(0x00, UART1_REG(MCR));
+	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 */
 	writel(cfg->cmd, UART1_REG(TX));
Signature · 1 line
2.10.2
#2 lore raw

PATCH 2/3 power: reset: make qnap-poweroff flexible on length of power-off command

In reply to Roger Shimizu

Current power-off command sending to UART1 is fixed to one-byte. In order to support more devices, it's necessary to make power-off command as 2-D array argument of device configuration.

By using 2-D array, power-off command may consist a few commands, each of which may consist a few bytes.

---
 drivers/power/reset/qnap-poweroff.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)
drivers/power/reset/qnap-poweroff.c +19 -4
index 47ed120..644478c 100644
@@ -25,6 +25,7 @@
 #include <linux/clk.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 qnap_micon_magic[] = {
@@ -34,22 +35,33 @@ static const unsigned char qnap_micon_magic[] = {
 	0x00
 };
 
+// for each row, first byte is the size of command
+static const unsigned char qnap_power_off_cmd[][MICON_CMD_SIZE] = {
+	{ 1, 'A'},
+	{}
+};
+
+static const unsigned char synology_power_off_cmd[][MICON_CMD_SIZE] = {
+	{ 1, '1'},
+	{}
+};
+
 struct power_off_cfg {
 	u32 baud;
 	const unsigned char *magic;
-	char cmd;
+	const unsigned char (*cmd)[MICON_CMD_SIZE];
 };
 
 static const struct power_off_cfg qnap_power_off_cfg = {
 	.baud = 19200,
 	.magic = qnap_micon_magic,
-	.cmd = 'A',
+	.cmd = qnap_power_off_cmd,
 };
 
 static const struct power_off_cfg synology_power_off_cfg = {
 	.baud = 9600,
 	.magic = qnap_micon_magic,
-	.cmd = '1',
+	.cmd = synology_power_off_cmd,
 };
 
 static const struct of_device_id qnap_power_off_of_match_table[] = {
@@ -83,7 +95,10 @@ static void qnap_power_off(void)
 	writel(cfg->magic[3], UART1_REG(MCR));
 
 	/* send the power-off command to PIC */
-	writel(cfg->cmd, UART1_REG(TX));
+	if(cfg->cmd[0][0] == 1 && cfg->cmd[1][0] == 0) {
+		/* for qnap and synology, it's simply one-byte command */
+		writel(cfg->cmd[0][1], UART1_REG(TX));
+	}
 }
 
 static int qnap_power_off_probe(struct platform_device *pdev)
Signature · 1 line
2.10.2
#3 lore raw

PATCH 3/3 power: reset: make qnap-poweroff support kurobox-pro

In reply to Roger Shimizu

Add KuroBox-Pro and Terastation Pro II/Live support on qnap-poweroff

The magic command and function of sending multi-byte command come from files listed below:
- arch/arm/mach-orion5x/kurobox_pro-setup.c
- arch/arm/mach-orion5x/terastation_pro2-setup.c

---
 .../bindings/power/reset/qnap-poweroff.txt         |   8 +-
 drivers/power/reset/qnap-poweroff.c                | 143 ++++++++++++++++++++-
 2 files changed, 149 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/power/reset/qnap-poweroff.txt +7 -1
index af25e77..410993d 100644
@@ -9,8 +9,14 @@ pm_power_off which is called to turn the power off.
 Synology NAS devices use a similar scheme, but a different baud rate,
 9600, and a different character, '1'.
 
+KuroBox Pro, Buffalo Linkstation LS-GL (also known as
+Buffalo Linkstation Pro/Live) and Buffalo Terastation Pro II/Live
+are similar, however it has even higher baud rate and need more
+commands to feed the UART1 before power off.
+
 Required Properties:
-- compatible: Should be "qnap,power-off" or "synology,power-off"
+- compatible: Should be "qnap,power-off", "synology,power-off"
+  or "kuroboxpro,power-off"
 
 - reg: Address and length of the register set for UART1
 - clocks: tclk clock
drivers/power/reset/qnap-poweroff.c +142 -1

195 lines not on this page · raw message

Signature · 1 line
2.10.2
#4 lore raw
In reply to Roger Shimizu
Quoted 11 lines · Roger Shimizu

Dear Sebastian,

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.

I already tested this change on Kurobox-Pro and Linkstation LS-GL devices, with a modified device-tree file. (Previous device-tree of kurobox-pro invokes restart-poweroff, so it simply restarts.)

Thank you and look forward to your feedback!

Hi Roger

You are adding a lot of new code here, and not actually reusing much of that is there, but adding a lot of complexity to what is a simple driver.

I would prefer if you actually write a new driver which just supports Kurobox.

Thanks
Andrew

#5 lore raw

cover make kurobox-pro be able to shutdown after device-tree migration

Dear Sebastian,

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.

I already tested this change on Kurobox-Pro and Linkstation LS-GL devices, with a modified device-tree file. (Previous device-tree of kurobox-pro invokes restart-poweroff, so it simply restarts.)

Thank you and look forward to your feedback!

Dear Andrew,

Thanks for your review!

I split off most of kurobox-pro releated code into separated files. Hope you can accept this series.

Changes:
v0 => v1:
- Update 0003 to split kuroboxpro related code into kuroboxpro-common.c

Cheers,
Roger

Roger Shimizu (3):
power: reset: make qnap-poweroff flexible on hello magic command to
uart1
power: reset: make qnap-poweroff flexible on length of power-off
command
power: reset: make qnap-poweroff support kurobox-pro

 .../bindings/power/reset/qnap-poweroff.txt         |   8 +-
 drivers/power/reset/Makefile                       |   2 +-
 drivers/power/reset/kuroboxpro-common.c            | 128 +++++++++++++++++++++
 drivers/power/reset/kuroboxpro-common.h            |   8 ++
 drivers/power/reset/qnap-poweroff.c                |  82 +++++++++++--
 5 files changed, 216 insertions(+), 12 deletions(-)
 create mode 100644 drivers/power/reset/kuroboxpro-common.c
 create mode 100644 drivers/power/reset/kuroboxpro-common.h
Signature · 1 line
2.10.2
#6 lore raw

PATCH 1/3 power: reset: make qnap-poweroff flexible on hello magic command to uart1

Current hello magic command sending to UART1 is fixed in qnap_power_off().

In order to support more devices, it's necessary to make hello magic command as an argument in device configuration.

---
 drivers/power/reset/qnap-poweroff.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)
drivers/power/reset/qnap-poweroff.c +16 -4
index 2789a61..47ed120 100644
@@ -2,6 +2,7 @@
  * QNAP Turbo NAS Board power off. Can also be used on Synology devices.
  *
  * Copyright (C) 2012 Andrew Lunn <[email protected]>
+ * Copyright (C) 2016 Roger Shimizu <[email protected]>
  *
  * Based on the code from:
  *
@@ -25,18 +26,29 @@
 
 #define UART1_REG(x)	(base + ((UART_##x) << 2))
 
+/* 4-byte magic hello command to UART1-attached microcontroller */
+static const unsigned char qnap_micon_magic[] = {
+	0x03,
+	0x00,
+	0x00,
+	0x00
+};
+
 struct power_off_cfg {
 	u32 baud;
+	const unsigned char *magic;
 	char cmd;
 };
 
 static const struct power_off_cfg qnap_power_off_cfg = {
 	.baud = 19200,
+	.magic = qnap_micon_magic,
 	.cmd = 'A',
 };
 
 static const struct power_off_cfg synology_power_off_cfg = {
 	.baud = 9600,
+	.magic = qnap_micon_magic,
 	.cmd = '1',
 };
 
@@ -65,10 +77,10 @@ static void qnap_power_off(void)
 	writel(0x83, UART1_REG(LCR));
 	writel(divisor & 0xff, UART1_REG(DLL));
 	writel((divisor >> 8) & 0xff, UART1_REG(DLM));
-	writel(0x03, UART1_REG(LCR));
-	writel(0x00, UART1_REG(IER));
-	writel(0x00, UART1_REG(FCR));
-	writel(0x00, UART1_REG(MCR));
+	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 */
 	writel(cfg->cmd, UART1_REG(TX));
Signature · 1 line
2.10.2
#7 lore raw

PATCH 2/3 power: reset: make qnap-poweroff flexible on length of power-off command

In reply to Roger Shimizu

Current power-off command sending to UART1 is fixed to one-byte. In order to support more devices, it's necessary to make power-off command as 2-D array argument of device configuration.

By using 2-D array, power-off command may consist a few commands, each of which may consist a few bytes.

---
 drivers/power/reset/qnap-poweroff.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)
drivers/power/reset/qnap-poweroff.c +19 -4
index 47ed120..644478c 100644
@@ -25,6 +25,7 @@
 #include <linux/clk.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 qnap_micon_magic[] = {
@@ -34,22 +35,33 @@ static const unsigned char qnap_micon_magic[] = {
 	0x00
 };
 
+// for each row, first byte is the size of command
+static const unsigned char qnap_power_off_cmd[][MICON_CMD_SIZE] = {
+	{ 1, 'A'},
+	{}
+};
+
+static const unsigned char synology_power_off_cmd[][MICON_CMD_SIZE] = {
+	{ 1, '1'},
+	{}
+};
+
 struct power_off_cfg {
 	u32 baud;
 	const unsigned char *magic;
-	char cmd;
+	const unsigned char (*cmd)[MICON_CMD_SIZE];
 };
 
 static const struct power_off_cfg qnap_power_off_cfg = {
 	.baud = 19200,
 	.magic = qnap_micon_magic,
-	.cmd = 'A',
+	.cmd = qnap_power_off_cmd,
 };
 
 static const struct power_off_cfg synology_power_off_cfg = {
 	.baud = 9600,
 	.magic = qnap_micon_magic,
-	.cmd = '1',
+	.cmd = synology_power_off_cmd,
 };
 
 static const struct of_device_id qnap_power_off_of_match_table[] = {
@@ -83,7 +95,10 @@ static void qnap_power_off(void)
 	writel(cfg->magic[3], UART1_REG(MCR));
 
 	/* send the power-off command to PIC */
-	writel(cfg->cmd, UART1_REG(TX));
+	if(cfg->cmd[0][0] == 1 && cfg->cmd[1][0] == 0) {
+		/* for qnap and synology, it's simply one-byte command */
+		writel(cfg->cmd[0][1], UART1_REG(TX));
+	}
 }
 
 static int qnap_power_off_probe(struct platform_device *pdev)
Signature · 1 line
2.10.2
#8 lore raw

PATCH 3/3 power: reset: make qnap-poweroff support kurobox-pro

In reply to Roger Shimizu

Add KuroBox-Pro and Terastation Pro II/Live support on qnap-poweroff

The magic command and function of sending multi-byte command come from files listed below:
- arch/arm/mach-orion5x/kurobox_pro-setup.c
- arch/arm/mach-orion5x/terastation_pro2-setup.c

---
 .../bindings/power/reset/qnap-poweroff.txt         |   8 +-
 drivers/power/reset/Makefile                       |   2 +-
 drivers/power/reset/kuroboxpro-common.c            | 128 +++++++++++++++++++++
 drivers/power/reset/kuroboxpro-common.h            |   8 ++
 drivers/power/reset/qnap-poweroff.c                |  39 ++++++-
 5 files changed, 181 insertions(+), 4 deletions(-)
 create mode 100644 drivers/power/reset/kuroboxpro-common.c
 create mode 100644 drivers/power/reset/kuroboxpro-common.h
Documentation/devicetree/bindings/power/reset/qnap-poweroff.txt +7 -1
index af25e77..410993d 100644
@@ -9,8 +9,14 @@ pm_power_off which is called to turn the power off.
 Synology NAS devices use a similar scheme, but a different baud rate,
 9600, and a different character, '1'.
 
+KuroBox Pro, Buffalo Linkstation LS-GL (also known as
+Buffalo Linkstation Pro/Live) and Buffalo Terastation Pro II/Live
+are similar, however it has even higher baud rate and need more
+commands to feed the UART1 before power off.
+
 Required Properties:
-- compatible: Should be "qnap,power-off" or "synology,power-off"
+- compatible: Should be "qnap,power-off", "synology,power-off"
+  or "kuroboxpro,power-off"
 
 - reg: Address and length of the register set for UART1
 - clocks: tclk clock
drivers/power/reset/Makefile +1 -1
index 1be307c..df48285 100644
@@ -11,7 +11,7 @@ obj-$(CONFIG_POWER_RESET_HISI) += hisi-reboot.o
 obj-$(CONFIG_POWER_RESET_IMX) += imx-snvs-poweroff.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
+obj-$(CONFIG_POWER_RESET_QNAP) += qnap-poweroff.o kuroboxpro-common.o
 obj-$(CONFIG_POWER_RESET_RESTART) += restart-poweroff.o
 obj-$(CONFIG_POWER_RESET_ST) += st-poweroff.o
 obj-$(CONFIG_POWER_RESET_VERSATILE) += arm-versatile-reboot.o
drivers/power/reset/kuroboxpro-common.c new file +128 -0

128 lines not on this page · raw message

drivers/power/reset/kuroboxpro-common.h new file +8 -0
new file mode 100644
index 0000000..58d5ee6
@@ -0,0 +1,8 @@
+#ifndef __KUROBOXPRO_COMMON_H__
+#define __KUROBOXPRO_COMMON_H__
+
+#define UART1_REG(x)	(base + ((UART_##x) << 2))
+
+int uart1_micon_send(void *base, const unsigned char *data, int count);
+
+#endif
drivers/power/reset/qnap-poweroff.c +37 -2
index 644478c..5ae7f1b 100644
@@ -1,5 +1,10 @@
 /*
- * QNAP Turbo NAS Board power off. Can also be used on Synology devices.
+ * QNAP Turbo NAS Board power off.
+ * It can also be used on following devices:
+ * - Synology devices
+ * - KuroBox Pro
+ * - Buffalo Linkstation Pro (LS-GL)
+ * - Buffalo Terastation Pro II/Live
  *
  * Copyright (C) 2012 Andrew Lunn <[email protected]>
  * Copyright (C) 2016 Roger Shimizu <[email protected]>
@@ -23,8 +28,8 @@
 #include <linux/of.h>
 #include <linux/io.h>
 #include <linux/clk.h>
+#include "kuroboxpro-common.h"
 
-#define UART1_REG(x)	(base + ((UART_##x) << 2))
 #define MICON_CMD_SIZE	4
 
 /* 4-byte magic hello command to UART1-attached microcontroller */
@@ -35,6 +40,13 @@ static const unsigned char qnap_micon_magic[] = {
 	0x00
 };
 
+static const unsigned char kuroboxpro_micon_magic[] = {
+	0x1b,
+	0x00,
+	0x07,
+	0x00
+};
+
 // for each row, first byte is the size of command
 static const unsigned char qnap_power_off_cmd[][MICON_CMD_SIZE] = {
 	{ 1, 'A'},
@@ -46,6 +58,13 @@ static const unsigned char synology_power_off_cmd[][MICON_CMD_SIZE] = {
 	{}
 };
 
+static const unsigned char kuroboxpro_power_off_cmd[][MICON_CMD_SIZE] = {
+	{ 3, 0x01, 0x35, 0x00},
+	{ 2, 0x00, 0x0c},
+	{ 2, 0x00, 0x06},
+	{}
+};
+
 struct power_off_cfg {
 	u32 baud;
 	const unsigned char *magic;
@@ -64,6 +83,12 @@ static const struct power_off_cfg synology_power_off_cfg = {
 	.cmd = synology_power_off_cmd,
 };
 
+static const struct power_off_cfg kuroboxpro_power_off_cfg = {
+	.baud = 38400,
+	.magic = kuroboxpro_micon_magic,
+	.cmd = kuroboxpro_power_off_cmd,
+};
+
 static const struct of_device_id qnap_power_off_of_match_table[] = {
 	{ .compatible = "qnap,power-off",
 	  .data = &qnap_power_off_cfg,
@@ -71,6 +96,9 @@ static const struct of_device_id qnap_power_off_of_match_table[] = {
 	{ .compatible = "synology,power-off",
 	  .data = &synology_power_off_cfg,
 	},
+	{ .compatible = "kuroboxpro,power-off",
+	  .data = &kuroboxpro_power_off_cfg,
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, qnap_power_off_of_match_table);
@@ -99,6 +127,13 @@ static void qnap_power_off(void)
 		/* for qnap and synology, it's simply one-byte command */
 		writel(cfg->cmd[0][1], UART1_REG(TX));
 	}
+	else {
+		int i;
+		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 qnap_power_off_probe(struct platform_device *pdev)
Signature · 1 line
2.10.2
#9 lore raw
In reply to Roger Shimizu
Quoted 6 lines

Dear Andrew,

Thanks for your review!

I split off most of kurobox-pro releated code into separated files. Hope you can accept this series.

Hi Roger

I still think it should be a separate driver. The 128 lines of kuroboxpro-common.c is only slightly smaller than the unmodified driver.

What code are you actually sharing? The probe and remove functions, which is mostly boiler plate, the same in all driver, and the baud rate generation. So there is very little actually common and re-used.

Andrew

#10 lore raw

PATCH v2 1/1 power: reset: add linkstation-reset driver

In reply to Roger Shimizu

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

Cc Andrew Lunn <[email protected]>
Cc Martin Michlmayr <[email protected]>
Cc Sylver Bruneau <[email protected]>
Cc Herbert Valerio Riedel <[email protected]>

---
Dear Sebastian,

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.

I already tested this change on Kurobox-Pro and Linkstation LS-GL devices, with a modified device-tree file. (Previous device-tree of kurobox-pro invokes restart-poweroff, so it simply restarts.)

Thank you and look forward to your feedback!

Dear Andrew,

Thanks for your 2nd review!

So I accept your suggestion and make the new driver for linkstation series.

Changes:
v0 => v1:
- Update 0003 to split kuroboxpro related code into kuroboxpro-common.c v1 => v2:
- Slipt 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.

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

 .../bindings/power/reset/linkstation-reset.txt     |  26 ++++
 drivers/power/reset/Kconfig                        |  10 ++
 drivers/power/reset/Makefile                       |   1 +
 drivers/power/reset/linkstation-common.c           | 124 +++++++++++++++
 drivers/power/reset/linkstation-common.h           |   8 +
 drivers/power/reset/linkstation-reset.c            | 172 +++++++++++++++++++++
 6 files changed, 341 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/reset/linkstation-reset.txt
 create mode 100644 drivers/power/reset/linkstation-common.c
 create mode 100644 drivers/power/reset/linkstation-common.h
 create mode 100644 drivers/power/reset/linkstation-reset.c
Documentation/devicetree/bindings/power/reset/linkstation-reset.txt new file +26 -0
new file mode 100644
index 0000000..815e340
@@ -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>;
+	};
drivers/power/reset/Kconfig +10 -0
index c74c3f6..77c44ca 100644
@@ -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
drivers/power/reset/Makefile +1 -0
index 1be307c..520afbe 100644
@@ -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 linkstation-common.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
drivers/power/reset/linkstation-common.c new file +124 -0

124 lines not on this page · raw message

drivers/power/reset/linkstation-common.h new file +8 -0
new file mode 100644
index 0000000..89c64a9
@@ -0,0 +1,8 @@
+#ifndef __LINKSTATION_COMMON_H__
+#define __LINKSTATION_COMMON_H__
+
+#define UART1_REG(x)	(base + ((UART_##x) << 2))
+
+int uart1_micon_send(void *base, const unsigned char *data, int count);
+
+#endif
drivers/power/reset/linkstation-reset.c new file +172 -0

172 lines not on this page · raw message

Signature · 1 line
2.10.2
author Tested-by Ryan Tandy on v2 1/1
#11 lore raw

Re: [PATCH v2] power: reset: add linkstation-reset driver

Quoted 8 lines · On Fri, 16 Dec 2016 19:05:01 +0900 Roger Shimizu <[email protected]> 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

Quoted 7 lines
Cc Andrew Lunn <[email protected]>
Cc Martin Michlmayr <[email protected]>
Cc Sylver Bruneau <[email protected]>
Cc Herbert Valerio Riedel <[email protected]>

---

Original reporter Ryan Tandy told me in a separate email that he has confirmed my patch works on his Linkstation Pro device. So please add:

when merging this patch. Thank you!

Cheers,

Signature · 2 lines
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 4096R/6C6ACD6417B3ACB1
#12 lore raw

Re: [PATCH v2] power: reset: add linkstation-reset driver

In reply to Roger Shimizu

Hi Roger,

Quoted 7 lines · Roger Shimizu

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

Ok.

Quoted 38 lines
Cc Andrew Lunn <[email protected]>
Cc Martin Michlmayr <[email protected]>
Cc Sylver Bruneau <[email protected]>
Cc Herbert Valerio Riedel <[email protected]>

---
Dear Sebastian,

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.

I already tested this change on Kurobox-Pro and Linkstation LS-GL devices, with a modified device-tree file. (Previous device-tree of kurobox-pro invokes restart-poweroff, so it simply restarts.)

Thank you and look forward to your feedback!

Dear Andrew,

Thanks for your 2nd review!

So I accept your suggestion and make the new driver for linkstation series.

Changes:
v0 => v1:
- Update 0003 to split kuroboxpro related code into kuroboxpro-common.c v1 => v2:
- Slipt 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.

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

Quoted 7 lines
 .../bindings/power/reset/linkstation-reset.txt     |  26 ++++
 drivers/power/reset/Kconfig                        |  10 ++
 drivers/power/reset/Makefile                       |   1 +
 drivers/power/reset/linkstation-common.c           | 124 +++++++++++++++
 drivers/power/reset/linkstation-common.h           |   8 +
 drivers/power/reset/linkstation-reset.c            | 172 +++++++++++++++++++++
 6 files changed, 341 insertions(+)

With this being its own driver please merge linkstation-common and linkstation-reset. The common part is only used by linkstation-reset anyways.

Quoted 1 line
 create mode 100644 Documentation/devicetree/bindings/power/reset/linkstation-reset.txt

This patch is missing Cc for DT binding people (check "OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" in MAINTAINERS file).

Quoted 26 lines
 create mode 100644 drivers/power/reset/linkstation-common.c
 create mode 100644 drivers/power/reset/linkstation-common.h
 create mode 100644 drivers/power/reset/linkstation-reset.c
Documentation/devicetree/bindings/power/reset/linkstation-reset.txt new file +16 -0
new file mode 100644
index 0000000..815e340
@@ -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"

Documentation/devicetree/bindings/power/reset/linkstation-reset.txt line 26

Quoted 10 lines
Documentation/devicetree/bindings/power/reset/linkstation-reset.txt new file +10 -0
+- 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>;
+	};

This might be another user for UART slave device [0].
[0] https://lkml.org/lkml/2016/8/24/769

Is the UART port used for anything else besides the reset controller?

Quoted 218 lines
drivers/power/reset/Kconfig +10 -0
index c74c3f6..77c44ca 100644
@@ -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
drivers/power/reset/Makefile +1 -0
index 1be307c..520afbe 100644
@@ -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 linkstation-common.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
drivers/power/reset/linkstation-common.c new file +124 -0
new file mode 100644
index 0000000..a6d0930
@@ -0,0 +1,124 @@
+/*
+ * Common I/O routine for micro-controller of Buffalo Linkstation
+ * and its variants.
+ *
+ * Copyright (C) 2016 Roger Shimizu <[email protected]>
+ *
+ * Based on the code from:
+ *
+ * Copyright (C) 2008  Sylver Bruneau <[email protected]>
+ * Copyright (C) 2007  Herbert Valerio Riedel <[email protected]>
+ *
+ * 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/serial_reg.h>
+#include <linux/io.h>
+#include <linux/delay.h>
+#include "linkstation-common.h"
+
+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;
+}
drivers/power/reset/linkstation-common.h new file +8 -0
new file mode 100644
index 0000000..89c64a9
@@ -0,0 +1,8 @@
+#ifndef __LINKSTATION_COMMON_H__
+#define __LINKSTATION_COMMON_H__
+
+#define UART1_REG(x)	(base + ((UART_##x) << 2))
+
+int uart1_micon_send(void *base, const unsigned char *data, int count);
+
+#endif
drivers/power/reset/linkstation-reset.c new file +35 -0
new file mode 100644
index 0000000..78a0137
@@ -0,0 +1,172 @@
+/*
+ * Buffalo Linkstation power reset driver.
+ * It may also be used on following devices:
+ *  - Buffalo Linkstation HG
+ *  - KuroBox HG
+ *  - Buffalo KURO-NAS/T4
+ *  - 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 <[email protected]>
+ *
+ * Based on the code from:
+ *
+ * Copyright (C) 2012  Andrew Lunn <[email protected]>
+ * Copyright (C) 2009  Martin Michlmayr <[email protected]>
+ * Copyright (C) 2008  Byron Bradley <[email protected]>
+ *
+ * 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 "linkstation-common.h"

drivers/power/reset/linkstation-reset.c line 45

Quoted 10 lines
drivers/power/reset/linkstation-reset.c new file +10 -0
+
+#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
+};

4-byte magic hello command? Those are used as uart configuration as far as I can see. Just move this directly into reset_cfg:

struct reset_cfg {
    u32 baud;
    u8 lcr;
    u8 ier;
    u8 fcr;
    u8 mcr;
    const unsigned char (*cmd)[MICON_CMD_SIZE];
};
Quoted 42 lines
drivers/power/reset/linkstation-reset.c new file +42 -0
+// 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 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));
+
+	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));

drivers/power/reset/linkstation-reset.c line 98

Quoted 10 lines
drivers/power/reset/linkstation-reset.c new file +10 -0
+	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 */
+	if(cfg->cmd[0][0] == 1 && cfg->cmd[1][0] == 0) {
+		/* if it's simply one-byte command, send it directly */
+		writel(cfg->cmd[0][1], UART1_REG(TX));
+	}

I guess this optimization can be dropped and you can directly call the for loop with uart1_micon_send().

Quoted 64 lines
drivers/power/reset/linkstation-reset.c new file +64 -0
+	else {
+		int i;
+		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	= {

drivers/power/reset/linkstation-reset.c line 172

Quoted 10 lines
drivers/power/reset/linkstation-reset.c new file +10 -0
+		.name	= "linkstation_reset",
+		.of_match_table = of_match_ptr(linkstation_reset_of_match_table),
+	},
+};
+
+module_platform_driver(linkstation_reset_driver);
+
+MODULE_AUTHOR("Roger Shimizu <[email protected]>");
+MODULE_DESCRIPTION("KuroBox Pro Reset driver");
+MODULE_LICENSE("GPL v2");

-- Sebastian

#13 lore raw

Re: [PATCH v2] power: reset: add linkstation-reset driver

Quoted 11 lines

Documentation/devicetree/bindings/power/reset/linkstation-reset.txt line 26

Quoted 5 lines
Documentation/devicetree/bindings/power/reset/linkstation-reset.txt new file +5 -0
+	reset {
+		compatible = "linkstation,power-off";
+		reg = <0x12100 0x100>;
+		clocks = <&core_clk 0>;
+	};

This might be another user for UART slave device [0].
[0] https://lkml.org/lkml/2016/8/24/769

Is the UART port used for anything else besides the reset controller?

I don't know much about these specific devices, but the qnap equivalent, there is a user space daemon which also talks to the microcontroller, for things like a temperature sensor, buzzer, etc.

https://www.hellion.org.uk/qcontrol/

So the UART can be in a messed up state, which is why the QNAP driver, which this code is modelled on, reset it back to a good state.

Andrew

#14 lore raw

Re: [PATCH v2] power: reset: add linkstation-reset driver

Thanks for your review!

Quoted 12 lines · Andrew Lunn
Quoted 11 lines

Documentation/devicetree/bindings/power/reset/linkstation-reset.txt line 26

Quoted 5 lines
Documentation/devicetree/bindings/power/reset/linkstation-reset.txt new file +5 -0
+   reset {
+           compatible = "linkstation,power-off";
+           reg = <0x12100 0x100>;
+           clocks = <&core_clk 0>;
+   };

This might be another user for UART slave device [0].
[0] https://lkml.org/lkml/2016/8/24/769

Is the UART port used for anything else besides the reset controller?

Quoted 8 lines

I don't know much about these specific devices, but the qnap equivalent, there is a user space daemon which also talks to the microcontroller, for things like a temperature sensor, buzzer, etc.

https://www.hellion.org.uk/qcontrol/

So the UART can be in a messed up state, which is why the QNAP driver, which this code is modelled on, reset it back to a good state.

For Linkstation/KuroBox-Pro, it's quite similar, and the user-land program is called micro-evtd [0], which is co-maintained by Ryan Tandy and me in Debian.

[0] https://tracker.debian.org/pkg/micro-evtd

Quoted 1 line
Quoted 1 line

[0] https://lkml.org/lkml/2016/8/24/769

Do I need to modify anything related to the above UART slave device?

Cheers,

Signature · 2 lines
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 4096R/6C6ACD6417B3ACB1
#15 lore raw

Re: [PATCH v2] power: reset: add linkstation-reset driver

In reply to Sebastian Reichel

Dear Sebastian,

Thanks for your review!

Quoted 9 lines · Sebastian Reichel
Quoted 8 lines
 .../bindings/power/reset/linkstation-reset.txt     |  26 ++++
 drivers/power/reset/Kconfig                        |  10 ++
 drivers/power/reset/Makefile                       |   1 +
 drivers/power/reset/linkstation-common.c           | 124 +++++++++++++++
 drivers/power/reset/linkstation-common.h           |   8 +
 drivers/power/reset/linkstation-reset.c            | 172 +++++++++++++++++++++
 6 files changed, 341 insertions(+)
Quoted 3 lines

With this being its own driver please merge linkstation-common and linkstation-reset. The common part is only used by linkstation-reset anyways.

I'll add them into To/Cc list.

Quoted 36 lines

drivers/power/reset/linkstation-reset.c line 45

Quoted 7 lines
drivers/power/reset/linkstation-reset.c new file +7 -0
+/* 4-byte magic hello command to UART1-attached microcontroller */
+static const unsigned char linkstation_micon_magic[] = {
+     0x1b,
+     0x00,
+     0x07,
+     0x00
+};

4-byte magic hello command? Those are used as uart configuration as far as I can see. Just move this directly into reset_cfg:

struct reset_cfg {
    u32 baud;
    u8 lcr;
    u8 ier;
    u8 fcr;
    u8 mcr;
    const unsigned char (*cmd)[MICON_CMD_SIZE];
};
Quoted 15 lines
drivers/power/reset/linkstation-reset.c new file +15 -0
+static void linkstation_reset(void)
+{
+     const unsigned divisor = ((tclk + (8 * cfg->baud)) / (16 * cfg->baud));
+
+     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));
+
Quoted 8 lines

drivers/power/reset/linkstation-reset.c line 98

Quoted 5 lines
drivers/power/reset/linkstation-reset.c new file +5 -0
+     /* send the power-off command to PIC */
+     if(cfg->cmd[0][0] == 1 && cfg->cmd[1][0] == 0) {
+             /* if it's simply one-byte command, send it directly */
+             writel(cfg->cmd[0][1], UART1_REG(TX));
+     }

I guess this optimization can be dropped and you can directly call the for loop with uart1_micon_send().

Same response regarding above two comments.
The code is extensible because I want to extend in the future.

Current implementation is just for Linkstation Pro / KuroBox Pro to be able to power-off.
But for some other model of Linkstation, restart also need similar command via UART1.

Just one example, Linkstation Pro is ARM based, but it was PowerPC based before. And the device support still exists in kernel tree:
arch/powerpc/platforms/embedded6xx/linkstation.c arch/powerpc/platforms/embedded6xx/ls_uart.c
It shows sending "C" to restart and sending "E" to power-off for PowerPC based Linkstation.

I'm not actually interested in PowerPC based Linkstation, it's just an example to show the reason to be flexible.

If other part is fine, may I send the v3 patch after merging linkstation-common.c into linkstation-reset.c? Thank you!

Cheers,

Signature · 2 lines
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 4096R/6C6ACD6417B3ACB1

A message that is not in the archive (sent off-list or not yet received): CAEQ9gEnQEHdcA4ox3teOXKcrdf2AAqUMp=A6W6c7nXhk4VrKiw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#17 lore raw

Re: [PATCH v2] power: reset: add linkstation-reset driver

Hi,

Quoted 41 lines · Roger Shimizu

[...]

Quoted 15 lines
Quoted 15 lines
drivers/power/reset/linkstation-reset.c new file +15 -0
+static void linkstation_reset(void)
+{
+     const unsigned divisor = ((tclk + (8 * cfg->baud)) / (16 * cfg->baud));
+
+     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));
+
Quoted 8 lines

drivers/power/reset/linkstation-reset.c line 98

Quoted 5 lines
drivers/power/reset/linkstation-reset.c new file +5 -0
+     /* send the power-off command to PIC */
+     if(cfg->cmd[0][0] == 1 && cfg->cmd[1][0] == 0) {
+             /* if it's simply one-byte command, send it directly */
+             writel(cfg->cmd[0][1], UART1_REG(TX));
+     }

I guess this optimization can be dropped and you can directly call the for loop with uart1_micon_send().

Same response regarding above two comments.
The code is extensible because I want to extend in the future.

Current implementation is just for Linkstation Pro / KuroBox Pro to be able to power-off.
But for some other model of Linkstation, restart also need similar command via UART1.

Just one example, Linkstation Pro is ARM based, but it was PowerPC based before. And the device support still exists in kernel tree:
arch/powerpc/platforms/embedded6xx/linkstation.c arch/powerpc/platforms/embedded6xx/ls_uart.c
It shows sending "C" to restart and sending "E" to power-off for PowerPC based Linkstation.

Quoted 6 lines

I'm not actually interested in PowerPC based Linkstation, it's just an example to show the reason to be flexible.

If other part is fine, may I send the v3 patch after merging linkstation-common.c into linkstation-reset.c? Thank you!

These models can just be added to qnap-poweroff, which handles exactly this special case as far as I can see.

-- Sebastian

#18 lore raw

Re: [PATCH v2] power: reset: add linkstation-reset driver

Quoted 2 lines

These models can just be added to qnap-poweroff, which handles exactly this special case as far as I can see.

Nope, qnap is much, much simpler. The configuration of the serial port is simpler, and it only needs to send a single byte. Here we have all sorts of checksums to calculate, stuff coming back from the microcontroller, etc. The complexity is much higher.

V1 of this patchset did extend the qnap driver. But in fact, very little of the original code was left afterwards, and lots of new code was added. So i requested a new driver be written, rather than extend my qnap driver.

I would not like to see the nice and simple qnap driver get all this code added to it, making it much harder to maintain, for very little gain.

Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

#20 lore raw

Re: [PATCH v2] power: reset: add linkstation-reset driver

Hi Andrew,

Quoted 8 lines · Andrew Lunn
Quoted 2 lines

These models can just be added to qnap-poweroff, which handles exactly this special case as far as I can see.

Nope, qnap is much, much simpler. The configuration of the serial port is simpler, and it only needs to send a single byte. Here we have all sorts of checksums to calculate, stuff coming back from the microcontroller, etc. The complexity is much higher.

Quoted 8 lines

V1 of this patchset did extend the qnap driver. But in fact, very little of the original code was left afterwards, and lots of new code was added. So i requested a new driver be written, rather than extend my qnap driver.

I would not like to see the nice and simple qnap driver get all this code added to it, making it much harder to maintain, for very little gain.

I'm talking about the special case, where it also sends only a single byte:

Review

Quoted 5 lines
(quoted diff) +5 -0
+     /* send the power-off command to PIC */
+     if(cfg->cmd[0][0] == 1 && cfg->cmd[1][0] == 0) {
+             /* if it's simply one-byte command, send it directly */
+             writel(cfg->cmd[0][1], UART1_REG(TX));
+     }

The configuration is different, but

a) probably it can just use the config from the qnap driver, since it just sends a single byte.
b) making the 4 config registers configurable in the qnap driver does not add much complexity.

So this case should be removed from the linkstation-reset driver. If any board needs it, it should use the qnap-poweroff driver instead.

-- Sebastian

#21 lore raw

Re: [PATCH v2] power: reset: add linkstation-reset driver

Dear Sebastian,

Thanks for your feedback!

Quoted 12 lines · Sebastian Reichel

I'm talking about the special case, where it also sends only a single byte:

Review

Quoted 5 lines
(quoted diff) +5 -0
+     /* send the power-off command to PIC */
+     if(cfg->cmd[0][0] == 1 && cfg->cmd[1][0] == 0) {
+             /* if it's simply one-byte command, send it directly */
+             writel(cfg->cmd[0][1], UART1_REG(TX));
+     }

The configuration is different, but

Quoted 7 lines

a) probably it can just use the config from the qnap driver, since it just sends a single byte.
b) making the 4 config registers configurable in the qnap driver does not add much complexity.

So this case should be removed from the linkstation-reset driver. If any board needs it, it should use the qnap-poweroff driver instead.

Yes, if other board need sending one-byte command to UART1, it should make use of qnap-poweroff driver.
However, one model of Linkstation need such kind of command, I would like to add it in linkstation-reset driver.

Currently, there's no such usage, so if you don't like the redundant code which sending one-byte command, I can remove in my next patch series.

Merry Xmas and happy new year!

Cheers,

Signature · 2 lines
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 4096R/6C6ACD6417B3ACB1
#22 lore raw

v3 cover make kurobox-pro be able to shutdown after device-tree migration

In reply to Roger Shimizu

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
Signature · 1 line
2.11.0
#23 lore raw

PATCH v3 1/3 power: reset: add linkstation-reset driver

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 <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Martin Michlmayr <[email protected]>
Cc: Sylver Bruneau <[email protected]>
Cc: Herbert Valerio Riedel <[email protected]>
Cc: Ryan Tandy <[email protected]>
Cc: linux-pm at vger.kernel.org
Cc: devicetree at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org

---
 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
drivers/power/reset/Kconfig +10 -0
index c74c3f67b8da..77c44cad7ece 100644
@@ -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
drivers/power/reset/Makefile +1 -0
index 1be307c7fc25..692ba6417cfb 100644
@@ -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
drivers/power/reset/linkstation-reset.c new file +269 -0

269 lines not on this page · raw message

Signature · 1 line
2.11.0
#24 lore raw

PATCH v3 1/3 power: reset: add linkstation-reset driver

Hi Roger,

Quoted 7 lines · Roger Shimizu

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.

drivers/power/reset/linkstation-reset.c line 180

Quoted 4 lines
drivers/power/reset/linkstation-reset.c new file +4 -0
+
+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?

Quoted 45 lines
drivers/power/reset/linkstation-reset.c new file +45 -0
+
+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;

drivers/power/reset/linkstation-reset.c line 235

Quoted 10 lines
drivers/power/reset/linkstation-reset.c new file +10 -0
+	}
+
+	/* 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?

drivers/power/reset/linkstation-reset.c line 245

Quoted 10 lines
drivers/power/reset/linkstation-reset.c new file +10 -0
+
+	/* 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!

Signature · 1 line
Florian
#25 lore raw

PATCH v3 1/3 power: reset: add linkstation-reset driver

Quoted 5 lines
Quoted 5 lines
drivers/power/reset/linkstation-reset.c new file +5 -0
+
+	/* 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,
Quoted 8 lines

drivers/power/reset/linkstation-reset.c line 245

Quoted 5 lines
drivers/power/reset/linkstation-reset.c new file +5 -0
+			"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.

Quoted 2 lines

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

#26 lore raw

PATCH v3 1/3 power: reset: add linkstation-reset driver

Dear Florian, Andrew,

Thanks for your email!

Quoted 6 lines · Florian Fainelli

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!

Quoted 10 lines

Some comments below.

drivers/power/reset/linkstation-reset.c line 180

Quoted 4 lines
drivers/power/reset/linkstation-reset.c new file +4 -0
+
+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.

Quoted 26 lines
Quoted 26 lines
drivers/power/reset/linkstation-reset.c new file +26 -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)) {
Quoted 8 lines

drivers/power/reset/linkstation-reset.c line 235

Quoted 5 lines
drivers/power/reset/linkstation-reset.c new file +5 -0
+             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)

Quoted 18 lines · Andrew Lunn
Quoted 11 lines

drivers/power/reset/linkstation-reset.c line 245

Quoted 10 lines
drivers/power/reset/linkstation-reset.c new file +10 -0
+
+   /* 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;
Quoted 2 lines

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.

Quoted 5 lines
Quoted 2 lines

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?

Quoted 8 lines

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,

Signature · 2 lines
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 4096R/6C6ACD6417B3ACB1
#27 lore raw

PATCH v3 1/3 power: reset: add linkstation-reset driver

Quoted 122 lines · Roger Shimizu

Dear Florian, Andrew,

Thanks for your email!

Quoted 6 lines · Florian Fainelli

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!

Quoted 10 lines

Some comments below.

drivers/power/reset/linkstation-reset.c line 180

Quoted 4 lines
drivers/power/reset/linkstation-reset.c new file +4 -0
+
+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.

Quoted 26 lines
Quoted 26 lines
drivers/power/reset/linkstation-reset.c new file +26 -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)) {
Quoted 8 lines

drivers/power/reset/linkstation-reset.c line 235

Quoted 5 lines
drivers/power/reset/linkstation-reset.c new file +5 -0
+             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)

Quoted 18 lines · Andrew Lunn
Quoted 11 lines

drivers/power/reset/linkstation-reset.c line 245

Quoted 10 lines
drivers/power/reset/linkstation-reset.c new file +10 -0
+
+   /* 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;
Quoted 2 lines

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.

Quoted 5 lines
Quoted 2 lines

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?

Quoted 8 lines

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

Quoted 7 lines

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!

Signature · 1 line
Florian
#28 lore raw

PATCH v3 2/3 DT: bingdings: power: reset: add linkstation-reset doc

In reply to Roger Shimizu

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 <[email protected]>
To: Rob Herring <[email protected]>
To: Mark Rutland <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Ryan Tandy <[email protected]>
Cc: linux-pm at vger.kernel.org
Cc: devicetree at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org

---
 .../bindings/power/reset/linkstation-reset.txt     | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/reset/linkstation-reset.txt
Documentation/devicetree/bindings/power/reset/linkstation-reset.txt new file +26 -0
new file mode 100644
index 000000000000..815e340318f3
@@ -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>;
+	};
Signature · 1 line
2.11.0
#29 lore raw

PATCH v3 2/3 DT: bingdings: power: reset: add linkstation-reset doc

Quoted 29 lines · Roger Shimizu

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 <[email protected]>
To: Rob Herring <[email protected]>
To: Mark Rutland <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Ryan Tandy <[email protected]>
Cc: linux-pm at vger.kernel.org
Cc: devicetree at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org

---
 .../bindings/power/reset/linkstation-reset.txt     | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/reset/linkstation-reset.txt
Documentation/devicetree/bindings/power/reset/linkstation-reset.txt new file +3 -0
new file mode 100644
index 000000000000..815e340318f3
@@ -0,0 +1,26 @@
+* Buffalo Linkstation Reset Driver
+
+Power of some Buffalo Linkstation or KuroBox Pro is managed by

Documentation/devicetree/bindings/power/reset/linkstation-reset.txt line 13

Quoted 10 lines
Documentation/devicetree/bindings/power/reset/linkstation-reset.txt new file +10 -0
+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.

Documentation/devicetree/bindings/power/reset/linkstation-reset.txt line 17

Quoted 4 lines
Documentation/devicetree/bindings/power/reset/linkstation-reset.txt new file +4 -0
+
+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?

Quoted 10 lines
Documentation/devicetree/bindings/power/reset/linkstation-reset.txt new file +9 -0
+- clocks: tclk clock
+
+Example:
+
+	reset {
+		compatible = "linkstation,power-off";
+		reg = <0x12100 0x100>;
+		clocks = <&core_clk 0>;
+	};
Signature · 1 line
Florian
#30 lore raw

PATCH v3 2/3 DT: bingdings: power: reset: add linkstation-reset doc

Quoted 6 lines

Documentation/devicetree/bindings/power/reset/linkstation-reset.txt line 17

Quoted 4 lines
Documentation/devicetree/bindings/power/reset/linkstation-reset.txt new file +4 -0
+
+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

#31 lore raw

PATCH v3 2/3 DT: bingdings: power: reset: add linkstation-reset doc

Dear Florian, Andrew,

Thanks for your comments!

Quoted 10 lines · Andrew Lunn
Quoted 6 lines

Documentation/devicetree/bindings/power/reset/linkstation-reset.txt line 17

Quoted 4 lines
Documentation/devicetree/bindings/power/reset/linkstation-reset.txt new file +4 -0
+
+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,

Signature · 2 lines
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 4096R/6C6ACD6417B3ACB1
maintainer
#32 lore raw

PATCH v3 2/3 DT: bingdings: power: reset: add linkstation-reset doc

In reply to Roger Shimizu
Quoted 26 lines · Roger Shimizu

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 <[email protected]>
To: Rob Herring <[email protected]>
To: Mark Rutland <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Ryan Tandy <[email protected]>
Cc: linux-pm at vger.kernel.org
Cc: devicetree at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org

---
 .../bindings/power/reset/linkstation-reset.txt     | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/reset/linkstation-reset.txt
Documentation/devicetree/bindings/power/reset/linkstation-reset.txt new file +0 -0
new file mode 100644
index 000000000000..815e340318f3

Documentation/devicetree/bindings/power/reset/linkstation-reset.txt line 6

Quoted 6 lines
Documentation/devicetree/bindings/power/reset/linkstation-reset.txt new file +6 -0
@@ -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

#33 lore raw

PATCH v3 2/3 DT: bingdings: power: reset: add linkstation-reset doc

Dear Rob,

Thanks for your comments!

Quoted 9 lines · Rob Herring

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,

Signature · 2 lines
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 4096R/6C6ACD6417B3ACB1
#34 lore raw

PATCH v3 3/3 ARM: DT: add power-off support to linkstation lsgl and kuroboxpro

In reply to Roger Shimizu

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 <[email protected]>
To: Andrew Lunn <[email protected]>
To: Gregory Clement <[email protected]>
To: Sebastian Hesselbarth <[email protected]>
Cc: Ryan Tandy <[email protected]>
Cc: linux-pm at vger.kernel.org
Cc: devicetree at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org

---
 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(-)
arch/arm/boot/dts/orion5x-kuroboxpro.dts +8 -0
index 1a672b098d0b..aba38d802bda 100644
@@ -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 */
arch/arm/boot/dts/orion5x-linkstation-lsgl.dts +10 -0
index 51dc734cd5b9..370fc17a6dd9 100644
@@ -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>;
arch/arm/boot/dts/orion5x-linkstation-lswtgl.dts +4 -0
index 0eead400f427..571a71f5b7ad 100644
@@ -59,6 +59,10 @@
 		reg = <0x00000000 0x4000000>;
 	};
 
+	restart_poweroff {
+		compatible = "restart-poweroff";
+	};
+
 	gpio_keys {
 		power-on-switch {
 			gpios = <&gpio0 8 GPIO_ACTIVE_LOW>;
arch/arm/boot/dts/orion5x-linkstation.dtsi +0 -4
index ed456ab35fd8..40770a8d4b36 100644
@@ -57,10 +57,6 @@
 				 <MBUS_ID(0x01, 0x0f) 0 0xf4000000 0x40000>;
 	};
 
-	restart_poweroff {
-		compatible = "restart-poweroff";
-	};
-
 	regulators {
 		compatible = "simple-bus";
 		#address-cells = <1>;
Signature · 1 line
2.11.0
#35 lore raw

v4 cover make kurobox-pro be able to shutdown after device-tree migration

In reply to Roger Shimizu

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
Signature · 1 line
2.11.0
#36 lore raw

PATCH v4 1/2 power: reset: add linkstation-reset driver

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 <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Martin Michlmayr <[email protected]>
Cc: Sylver Bruneau <[email protected]>
Cc: Herbert Valerio Riedel <[email protected]>
Cc: Ryan Tandy <[email protected]>
Cc: Florian Fainelli <[email protected]>
Cc: linux-pm at vger.kernel.org
Cc: devicetree at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org

---
 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
drivers/power/reset/Kconfig +10 -0
index c74c3f67b8da..77c44cad7ece 100644
@@ -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
drivers/power/reset/Makefile +1 -0
index 1be307c7fc25..692ba6417cfb 100644
@@ -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
drivers/power/reset/linkstation-reset.c new file +270 -0

270 lines not on this page · raw message

Signature · 1 line
2.11.0
#37 lore raw

PATCH v4 1/2 power: reset: add linkstation-reset driver

Hi Roger,

drivers/power/reset/Kconfig config POWER_RESET_IMX · line 110

Quoted 10 lines · Roger Shimizu
drivers/power/reset/Kconfig +10 -0
+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.

#38 lore raw

PATCH v4 1/2 power: reset: add linkstation-reset driver

Dear Ryan,

Thanks for your comments!

Quoted 11 lines · Ryan Tandy
Quoted 9 lines · Roger Shimizu
drivers/power/reset/Kconfig +6 -0
+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.

Quoted 7 lines

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,

Signature · 2 lines
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 4096R/6C6ACD6417B3ACB1
#39 lore raw

PATCH v4 1/2 power: reset: add linkstation-reset driver

Quoted 3 lines · Roger Shimizu

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:

Quoted 2 lines · Sebastian Reichel

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.

#40 lore raw

PATCH v4 1/2 power: reset: add linkstation-reset driver

In reply to Roger Shimizu

Dear Sebastian,

Quoted 7 lines · Roger Shimizu

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,

Signature · 2 lines
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 4096R/6C6ACD6417B3ACB1
#41 lore raw

PATCH v4 1/2 power: reset: add linkstation-reset driver

Hi Roger,

Quoted 11 lines · Roger Shimizu
Quoted 7 lines · Roger Shimizu

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>

#42 lore raw

PATCH v4 1/2 power: reset: add linkstation-reset driver

Hi Roger,

Quoted 21 lines · Sebastian Reichel

Hi Roger,

Quoted 11 lines · Roger Shimizu
Quoted 7 lines · Roger Shimizu

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.

Quoted 7 lines

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

Quoted 6 lines

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

Signature · 4 lines
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#43 lore raw

PATCH v4 1/2 power: reset: add linkstation-reset driver

Dear Gregory,

Thanks for contacting!

Quoted 4 lines · Gregory CLEMENT

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,

Signature · 2 lines
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 4096R/6C6ACD6417B3ACB1
#44 lore raw

PATCH v4 1/2 power: reset: add linkstation-reset driver

Hi Roger,

Quoted 4 lines · Roger Shimizu

Dear Gregory,

Thanks for contacting!

Quoted 8 lines
Quoted 4 lines · Gregory CLEMENT

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

Quoted 5 lines

Cheers,

Signature · 2 lines
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 4096R/6C6ACD6417B3ACB1
Signature · 4 lines
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#45 lore raw

PATCH v4 2/2 ARM: DT: add power-off support to linkstation lsgl and kuroboxpro

In reply to Roger Shimizu

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 <[email protected]>
To: Andrew Lunn <[email protected]>
To: Gregory Clement <[email protected]>
To: Sebastian Hesselbarth <[email protected]>
Cc: Ryan Tandy <[email protected]>
Cc: linux-pm at vger.kernel.org
Cc: devicetree at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org

---
 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(-)
arch/arm/boot/dts/orion5x-kuroboxpro.dts +8 -0
index 1a672b098d0b..aba38d802bda 100644
@@ -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 */
arch/arm/boot/dts/orion5x-linkstation-lsgl.dts +10 -0
index 51dc734cd5b9..370fc17a6dd9 100644
@@ -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>;
arch/arm/boot/dts/orion5x-linkstation-lswtgl.dts +4 -0
index 0eead400f427..571a71f5b7ad 100644
@@ -59,6 +59,10 @@
 		reg = <0x00000000 0x4000000>;
 	};
 
+	restart_poweroff {
+		compatible = "restart-poweroff";
+	};
+
 	gpio_keys {
 		power-on-switch {
 			gpios = <&gpio0 8 GPIO_ACTIVE_LOW>;
arch/arm/boot/dts/orion5x-linkstation.dtsi +0 -4
index ed456ab35fd8..40770a8d4b36 100644
@@ -57,10 +57,6 @@
 				 <MBUS_ID(0x01, 0x0f) 0 0xf4000000 0x40000>;
 	};
 
-	restart_poweroff {
-		compatible = "restart-poweroff";
-	};
-
 	regulators {
 		compatible = "simple-bus";
 		#address-cells = <1>;
Signature · 1 line
2.11.0