[PATCH v9 0/9] Drivers to support the MCU on QNAP NAS devices

[PATCH v9 0/9] Drivers to support the MCU on QNAP NAS devices

[Heiko Stuebner]

This implements a set of drivers for the MCU used on QNAP NAS devices.

Of course no documentation for the serial protocol is available, so
thankfully QNAP has a tool on their rescue-inird to talk to the MCU and
I found interceptty [0] to listen to what goes over the serial connection.

In general it looks like there are two different generations in general,
an "EC" device and now this "MCU" - referenced in the strings of the
userspace handlers for those devices.

For the MCU "SPEC3" and "SPEC4" are listed which is configured in
the model.conf of the device. When setting the value from SPEC4 to
SPEC3 on my TS433, the supported commands change, but the command
interface stays the same and especially the version command is the
same.

The binding also does not expose any interals of the device that
might change, so hopefully there shouldn't be big roadblocks to
support different devices, apart from possibly adapting the commands.


changes in v9:
- add Acks + cc-stable to HID patch
- make timeout-value a constant in core mfd-driver
- some flush serdev before sending a new command

changes in v8:
- patch for hid-sensor hub to not do wonky stuff with an old
  platform-data copy
  I hope my reading of the situation is correct here, but that
  initial platform_data really seemed wrong

mfd:
- flush serial before writing a new command
- wait for send to complete before starting the receive wait-timeout
- set expected length to 0 directly when the reply is complete
  not after leaving the receive callback


changes in v7:
- use ASCII representation in commands where possible instead of hex vals
- drop get_variant function and use mfd platform-data instead

mfd:
- a lot of style improvements

leds:
- name variables better (value -> brightness, num -> num_err_led)
- handle preservation of blink mode more effectively
- snprintf -> scnprintf
- drop duplicate "failed to register ... LED" messages


changes in v6:
- format mcu commands arrays in single lines (Lee)

mfd:
- drop obsolete remain kdoc for the removed
  reply_lock (kernel test robot)


changes in v5:
binding:
- add Conor's Reviewed-by

mfd:
Address comments from Lee
- improve commit message
- improve Kconfig help text
- sort headers alphabetical
- style and spelling improvements
- constants for magic numbers
- drop reply assignment, the mcu only replies to commands sent to it,
  so there should only ever be one command in fligth.

hwmon:
Add Acked-by from Guenter and address some remarks
  - don't allow empty fan subnode
  - use num var directly when getting cooling levels, without using ret
    intermediate
  - use dev_err_probe in thermal init function


changes in v4:
binding:
- move cooling properties into a fan subnode and reference
  fan-common.yaml (Rob)
- dropped Krzysztof's Ack because of this

mfd:
- use correct format-string for size_t (kernel test robot)

input:
- added Dmitry's Ack

hwmon:
- adapted to fan-subnode when reading cooling properties
- dropped Guenter's Ack because of this


changes in v3:
mfd
- use correct power-off priority: default
- constify the cmd-data array in command functions (Dmitry)

leds:
- don't point to temporary buffers for cdev->name (Florian Eckert)

hwmon:
- use clamp_val(), don't try to reimplement (Guenter)
- add Guenter's Ack

input:
address Dmitry's comments
- constify some cmd arrays
- add input-close callback to cancel beep worker
- drop initial input event report


changes in v2:
binding:
- rename to qnap,ts433-mcu.yaml (Krzysztof)
- drop "preserve formatting" indicator (Krzysztof)
- add Krzysztof's Review tag

mfd:
- fix checkpatch --strict CHECKs
- add a MAINTAINERS entry for all qnap-mcu-parts

Heiko Stuebner (9):
  HID: hid-sensor-hub: don't use stale platform-data on remove
  mfd: core: make platform_data pointer const in struct mfd_cell
  dt-bindings: mfd: add binding for qnap,ts433-mcu devices
  mfd: add base driver for qnap-mcu devices
  leds: add driver for LEDs from qnap-mcu devices
  Input: add driver for the input part of qnap-mcu devices
  hwmon: add driver for the hwmon parts of qnap-mcu devices
  arm64: dts: rockchip: hook up the MCU on the QNAP TS433
  arm64: dts: rockchip: set hdd led labels on qnap-ts433

 .../bindings/mfd/qnap,ts433-mcu.yaml          |  42 ++
 Documentation/hwmon/index.rst                 |   1 +
 Documentation/hwmon/qnap-mcu-hwmon.rst        |  27 ++
 MAINTAINERS                                   |   9 +
 .../boot/dts/rockchip/rk3568-qnap-ts433.dts   |  61 +++
 drivers/hid/hid-sensor-hub.c                  |  21 +-
 drivers/hwmon/Kconfig                         |  12 +
 drivers/hwmon/Makefile                        |   1 +
 drivers/hwmon/qnap-mcu-hwmon.c                | 364 ++++++++++++++++++
 drivers/input/misc/Kconfig                    |  12 +
 drivers/input/misc/Makefile                   |   1 +
 drivers/input/misc/qnap-mcu-input.c           | 153 ++++++++
 drivers/leds/Kconfig                          |  11 +
 drivers/leds/Makefile                         |   1 +
 drivers/leds/leds-qnap-mcu.c                  | 227 +++++++++++
 drivers/mfd/Kconfig                           |  13 +
 drivers/mfd/Makefile                          |   2 +
 drivers/mfd/qnap-mcu.c                        | 338 ++++++++++++++++
 include/linux/mfd/core.h                      |   2 +-
 include/linux/mfd/qnap-mcu.h                  |  26 ++
 20 files changed, 1316 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/qnap,ts433-mcu.yaml
 create mode 100644 Documentation/hwmon/qnap-mcu-hwmon.rst
 create mode 100644 drivers/hwmon/qnap-mcu-hwmon.c
 create mode 100644 drivers/input/misc/qnap-mcu-input.c
 create mode 100644 drivers/leds/leds-qnap-mcu.c
 create mode 100644 drivers/mfd/qnap-mcu.c
 create mode 100644 include/linux/mfd/qnap-mcu.h

-- 
2.45.2

[PATCH v9 1/9] HID: hid-sensor-hub: don't use stale platform-data on remove

[Heiko Stuebner]

The hid-sensor-hub creates the individual device structs and transfers them
to the created mfd platform-devices via the platform_data in the mfd_cell.

Before e651a1da442a ("HID: hid-sensor-hub: Allow parallel synchronous reads")
the sensor-hub was managing access centrally, with one "completion" in the
hub's data structure, which needed to be finished on removal at the latest.

The mentioned commit then moved this central management to each hid sensor
device, resulting on a completion in each struct hid_sensor_hub_device.
The remove procedure was adapted to go through all sensor devices and
finish any pending "completion".

What this didn't take into account was, platform_device_add_data() that is
used by mfd_add{_hotplug}_devices() does a kmemdup on the submitted
platform-data. So the data the platform-device gets is a copy of the
original data, meaning that the device worked on a different completion
than what sensor_hub_remove() currently wants to access.

To fix that, use device_for_each_child() to go through each child-device
similar to how mfd_remove_devices() unregisters the devices later and
with that get the live platform_data to finalize the correct completion.

Fixes: e651a1da442a ("HID: hid-sensor-hub: Allow parallel synchronous reads")
Cc: stable@vger.kernel.org
Acked-by: Benjamin Tissoires <bentiss@kernel.org>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/hid/hid-sensor-hub.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 7bd86eef6ec7..4c94c03cb573 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -730,23 +730,30 @@ static int sensor_hub_probe(struct hid_device *hdev,
 	return ret;
 }
 
+static int sensor_hub_finalize_pending_fn(struct device *dev, void *data)
+{
+	struct hid_sensor_hub_device *hsdev = dev->platform_data;
+
+	if (hsdev->pending.status)
+		complete(&hsdev->pending.ready);
+
+	return 0;
+}
+
 static void sensor_hub_remove(struct hid_device *hdev)
 {
 	struct sensor_hub_data *data = hid_get_drvdata(hdev);
 	unsigned long flags;
-	int i;
 
 	hid_dbg(hdev, " hardware removed\n");
 	hid_hw_close(hdev);
 	hid_hw_stop(hdev);
+
 	spin_lock_irqsave(&data->lock, flags);
-	for (i = 0; i < data->hid_sensor_client_cnt; ++i) {
-		struct hid_sensor_hub_device *hsdev =
-			data->hid_sensor_hub_client_devs[i].platform_data;
-		if (hsdev->pending.status)
-			complete(&hsdev->pending.ready);
-	}
+	device_for_each_child(&hdev->dev, NULL,
+			      sensor_hub_finalize_pending_fn);
 	spin_unlock_irqrestore(&data->lock, flags);
+
 	mfd_remove_devices(&hdev->dev);
 	mutex_destroy(&data->mutex);
 }
-- 
2.45.2

[PATCH v9 2/9] mfd: core: make platform_data pointer const in struct mfd_cell

[Heiko Stuebner]

The content of the platform_data of a struct mfd_cell is simply passed on
to the platform_device_add_data() call in mfd_add_device() .

platform_device_add_data() already handles the data behind that pointer
as const and also uses kmemdup to create a copy of the data before
handing that copy over to the newly created platform-device,
so there is no reason to not extend this to struct mfd_cell, as the old
copy in the mfd_cell will be stale anyway.

This allows to pass structs gathered from of_device_get_match_data()
as platform-data to sub-devices - which is retrieved as const already.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 include/linux/mfd/core.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
index e8bcad641d8c..faeea7abd688 100644
--- a/include/linux/mfd/core.h
+++ b/include/linux/mfd/core.h
@@ -72,7 +72,7 @@ struct mfd_cell {
 	int			(*resume)(struct platform_device *dev);
 
 	/* platform data passed to the sub devices drivers */
-	void			*platform_data;
+	const void		*platform_data;
 	size_t			pdata_size;
 
 	/* Matches ACPI */
-- 
2.45.2

[PATCH v9 3/9] dt-bindings: mfd: add binding for qnap,ts433-mcu devices

[Heiko Stuebner]

These MCUs can be found in network attached storage devices made by QNAP.
They are connected to a serial port of the host device and provide
functionality like LEDs, power-control and temperature monitoring.

LEDs, buttons, etc are all elements of the MCU firmware itself, so don't
need devicetree input, though the fan gets its cooling settings from
a fan-0 subnode.

A binding for the LEDs for setting the linux-default-trigger may come
later, once all the LEDs are understood and ATA controllers actually
can address individual port-LEDs, but are really optional.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 .../bindings/mfd/qnap,ts433-mcu.yaml          | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/qnap,ts433-mcu.yaml

diff --git a/Documentation/devicetree/bindings/mfd/qnap,ts433-mcu.yaml b/Documentation/devicetree/bindings/mfd/qnap,ts433-mcu.yaml
new file mode 100644
index 000000000000..877078ac172f
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/qnap,ts433-mcu.yaml
@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/qnap,ts433-mcu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: QNAP NAS on-board Microcontroller
+
+maintainers:
+  - Heiko Stuebner <heiko@sntech.de>
+
+description:
+  QNAP embeds a microcontroller on their NAS devices adding system feature
+  as PWM Fan control, additional LEDs, power button status and more.
+
+properties:
+  compatible:
+    enum:
+      - qnap,ts433-mcu
+
+patternProperties:
+  "^fan-[0-9]+$":
+    $ref: /schemas/hwmon/fan-common.yaml#
+    unevaluatedProperties: false
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    uart {
+      mcu {
+        compatible = "qnap,ts433-mcu";
+
+        fan-0 {
+          #cooling-cells = <2>;
+          cooling-levels = <0 64 89 128 166 204 221 238>;
+        };
+      };
+    };
-- 
2.45.2

[PATCH v9 4/9] mfd: add base driver for qnap-mcu devices

[Heiko Stuebner]

These microcontroller units are used in network-attached-storage devices
made by QNAP and provide additional functionality to the system.

This adds the base driver that implements the serial protocol via
serdev and additionally hooks into the poweroff handlers to turn
off the parts of the system not supplied by the general PMIC.

Turning off (at least the TSx33 devices using Rockchip SoCs) consists of
two separate actions. Turning off the MCU alone does not turn off the main
SoC and turning off only the SoC/PMIC does not turn off the hard-drives.
Also if the MCU is not turned off, the system also won't start again until
it is unplugged from power.

So on shutdown the MCU needs to be turned off separately before the
main PMIC.

The protocol spoken by the MCU is sadly not documented, but was
obtained by listening to the chatter on the serial port, as thankfully
the "hal_app" program from QNAPs firmware allows triggering all/most
MCU actions from the command line.

The implementation of how to talk to the serial device got some
inspiration from the rave-sp servdev driver.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 MAINTAINERS                  |   6 +
 drivers/mfd/Kconfig          |  13 ++
 drivers/mfd/Makefile         |   2 +
 drivers/mfd/qnap-mcu.c       | 338 +++++++++++++++++++++++++++++++++++
 include/linux/mfd/qnap-mcu.h |  26 +++
 5 files changed, 385 insertions(+)
 create mode 100644 drivers/mfd/qnap-mcu.c
 create mode 100644 include/linux/mfd/qnap-mcu.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 65b7bbfeea96..cd8d18f76d28 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18878,6 +18878,12 @@ L:	linux-media@vger.kernel.org
 S:	Odd Fixes
 F:	drivers/media/tuners/qm1d1c0042*
 
+QNAP MCU DRIVER
+M:	Heiko Stuebner <heiko@sntech.de>
+S:	Maintained
+F:	drivers/mfd/qnap-mcu.c
+F:	include/linux/qnap-mcu.h
+
 QNX4 FILESYSTEM
 M:	Anders Larsen <al@alarsen.net>
 S:	Maintained
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index ae23b317a64e..74f4de8cd6f1 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2386,6 +2386,19 @@ config MFD_INTEL_M10_BMC_PMCI
 	  additional drivers must be enabled in order to use the functionality
 	  of the device.
 
+config MFD_QNAP_MCU
+	tristate "QNAP microcontroller unit core driver"
+	depends on SERIAL_DEV_BUS
+	select MFD_CORE
+	help
+	  Select this to get support for the QNAP MCU device found in
+	  several devices of QNAP network attached storage products that
+	  implements additional functionality for the device, like fan
+	  and LED control.
+
+	  This driver implements the base serial protocol to talk to the
+	  device and provides functions for the other parts to hook into.
+
 config MFD_RSMU_I2C
 	tristate "Renesas Synchronization Management Unit with I2C"
 	depends on I2C && OF
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index e057d6d6faef..b2d540934179 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -288,5 +288,7 @@ obj-$(CONFIG_MFD_INTEL_M10_BMC_PMCI)   += intel-m10-bmc-pmci.o
 obj-$(CONFIG_MFD_ATC260X)	+= atc260x-core.o
 obj-$(CONFIG_MFD_ATC260X_I2C)	+= atc260x-i2c.o
 
+obj-$(CONFIG_MFD_QNAP_MCU)	+= qnap-mcu.o
+
 obj-$(CONFIG_MFD_RSMU_I2C)	+= rsmu_i2c.o rsmu_core.o
 obj-$(CONFIG_MFD_RSMU_SPI)	+= rsmu_spi.o rsmu_core.o
diff --git a/drivers/mfd/qnap-mcu.c b/drivers/mfd/qnap-mcu.c
new file mode 100644
index 000000000000..4be39d8b2905
--- /dev/null
+++ b/drivers/mfd/qnap-mcu.c
@@ -0,0 +1,338 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Core driver for the microcontroller unit in QNAP NAS devices that is
+ * connected via a dedicated UART port.
+ *
+ * Copyright (C) 2024 Heiko Stuebner <heiko@sntech.de>
+ */
+
+#include <linux/cleanup.h>
+#include <linux/export.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/qnap-mcu.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/reboot.h>
+#include <linux/serdev.h>
+#include <linux/slab.h>
+
+/* The longest command found so far is 5 bytes long */
+#define QNAP_MCU_MAX_CMD_SIZE		5
+#define QNAP_MCU_MAX_DATA_SIZE		36
+#define QNAP_MCU_CHECKSUM_SIZE		1
+
+#define QNAP_MCU_RX_BUFFER_SIZE		\
+		(QNAP_MCU_MAX_DATA_SIZE + QNAP_MCU_CHECKSUM_SIZE)
+
+#define QNAP_MCU_TX_BUFFER_SIZE		\
+		(QNAP_MCU_MAX_CMD_SIZE + QNAP_MCU_CHECKSUM_SIZE)
+
+#define QNAP_MCU_ACK_LEN		2
+#define QNAP_MCU_VERSION_LEN		4
+
+#define QNAP_MCU_TIMEOUT_MS		500
+
+/**
+ * struct qnap_mcu_reply - Reply to a command
+ *
+ * @data:	Buffer to store reply payload in
+ * @length:	Expected reply length, including the checksum
+ * @received:	Received number of bytes, so far
+ * @done:	Triggered when the entire reply has been received
+ */
+struct qnap_mcu_reply {
+	u8 *data;
+	size_t length;
+	size_t received;
+	struct completion done;
+};
+
+/**
+ * struct qnap_mcu - QNAP NAS embedded controller
+ *
+ * @serdev:	Pointer to underlying serdev
+ * @bus_lock:	Lock to serialize access to the device
+ * @reply:	Reply data structure
+ * @variant:	Device variant specific information
+ * @version:	MCU firmware version
+ */
+struct qnap_mcu {
+	struct serdev_device *serdev;
+	struct mutex bus_lock;
+	struct qnap_mcu_reply reply;
+	const struct qnap_mcu_variant *variant;
+	u8 version[QNAP_MCU_VERSION_LEN];
+};
+
+/*
+ * The QNAP-MCU uses a basic XOR checksum.
+ * It is always the last byte and XORs the whole previous message.
+ */
+static u8 qnap_mcu_csum(const u8 *buf, size_t size)
+{
+	u8 csum = 0;
+
+	while (size--)
+		csum ^= *buf++;
+
+	return csum;
+}
+
+static int qnap_mcu_write(struct qnap_mcu *mcu, const u8 *data, u8 data_size)
+{
+	unsigned char tx[QNAP_MCU_TX_BUFFER_SIZE];
+	size_t length = data_size + QNAP_MCU_CHECKSUM_SIZE;
+
+	if (length > sizeof(tx)) {
+		dev_err(&mcu->serdev->dev, "data too big for transmit buffer");
+		return -EINVAL;
+	}
+
+	memcpy(tx, data, data_size);
+	tx[data_size] = qnap_mcu_csum(data, data_size);
+
+	serdev_device_write_flush(mcu->serdev);
+
+	return serdev_device_write(mcu->serdev, tx, length, HZ);
+}
+
+static size_t qnap_mcu_receive_buf(struct serdev_device *serdev, const u8 *buf, size_t size)
+{
+	struct device *dev = &serdev->dev;
+	struct qnap_mcu *mcu = dev_get_drvdata(dev);
+	struct qnap_mcu_reply *reply = &mcu->reply;
+	const u8 *src = buf;
+	const u8 *end = buf + size;
+
+	if (!reply->length) {
+		dev_warn(dev, "Received %zu bytes, we were not waiting for\n", size);
+		return size;
+	}
+
+	while (src < end) {
+		reply->data[reply->received] = *src++;
+		reply->received++;
+
+		if (reply->received == reply->length) {
+			/* We don't expect any characters from the device now */
+			reply->length = 0;
+
+			complete(&reply->done);
+
+			/*
+			 * We report the consumed number of bytes. If there
+			 * are still bytes remaining (though there shouldn't)
+			 * the serdev layer will re-execute this handler with
+			 * the remainder of the Rx bytes.
+			 */
+			return src - buf;
+		}
+	}
+
+	/*
+	 * The only way to get out of the above loop and end up here
+	 * is through consuming all of the supplied data, so here we
+	 * report that we processed it all.
+	 */
+	return size;
+}
+
+static const struct serdev_device_ops qnap_mcu_serdev_device_ops = {
+	.receive_buf  = qnap_mcu_receive_buf,
+	.write_wakeup = serdev_device_write_wakeup,
+};
+
+int qnap_mcu_exec(struct qnap_mcu *mcu,
+		  const u8 *cmd_data, size_t cmd_data_size,
+		  u8 *reply_data, size_t reply_data_size)
+{
+	unsigned char rx[QNAP_MCU_RX_BUFFER_SIZE];
+	size_t length = reply_data_size + QNAP_MCU_CHECKSUM_SIZE;
+	struct qnap_mcu_reply *reply = &mcu->reply;
+	int ret = 0;
+
+	if (length > sizeof(rx)) {
+		dev_err(&mcu->serdev->dev, "expected data too big for receive buffer");
+		return -EINVAL;
+	}
+
+	mutex_lock(&mcu->bus_lock);
+
+	reply->data = rx,
+	reply->length = length,
+	reply->received = 0,
+	reinit_completion(&reply->done);
+
+	qnap_mcu_write(mcu, cmd_data, cmd_data_size);
+
+	serdev_device_wait_until_sent(mcu->serdev, msecs_to_jiffies(QNAP_MCU_TIMEOUT_MS));
+
+	if (!wait_for_completion_timeout(&reply->done, msecs_to_jiffies(QNAP_MCU_TIMEOUT_MS))) {
+		dev_err(&mcu->serdev->dev, "Command timeout\n");
+		ret = -ETIMEDOUT;
+	} else {
+		u8 crc = qnap_mcu_csum(rx, reply_data_size);
+
+		if (crc != rx[reply_data_size]) {
+			dev_err(&mcu->serdev->dev,
+				"Invalid Checksum received\n");
+			ret = -EIO;
+		} else {
+			memcpy(reply_data, rx, reply_data_size);
+		}
+	}
+
+	mutex_unlock(&mcu->bus_lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(qnap_mcu_exec);
+
+int qnap_mcu_exec_with_ack(struct qnap_mcu *mcu,
+			   const u8 *cmd_data, size_t cmd_data_size)
+{
+	u8 ack[QNAP_MCU_ACK_LEN];
+	int ret;
+
+	ret = qnap_mcu_exec(mcu, cmd_data, cmd_data_size, ack, sizeof(ack));
+	if (ret)
+		return ret;
+
+	/* Should return @0 */
+	if (ack[0] != '@' || ack[1] != '0') {
+		dev_err(&mcu->serdev->dev, "Did not receive ack\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(qnap_mcu_exec_with_ack);
+
+static int qnap_mcu_get_version(struct qnap_mcu *mcu)
+{
+	const u8 cmd[] = { '%', 'V' };
+	u8 rx[14];
+	int ret;
+
+	/* Reply is the 2 command-bytes + 4 bytes describing the version */
+	ret = qnap_mcu_exec(mcu, cmd, sizeof(cmd), rx, QNAP_MCU_VERSION_LEN + 2);
+	if (ret)
+		return ret;
+
+	memcpy(mcu->version, &rx[2], QNAP_MCU_VERSION_LEN);
+
+	return 0;
+}
+
+/*
+ * The MCU controls power to the peripherals but not the CPU.
+ *
+ * So using the PMIC to power off the system keeps the MCU and hard-drives
+ * running. This also then prevents the system from turning back on until
+ * the MCU is turned off by unplugging the power cable.
+ * Turning off the MCU alone on the other hand turns off the hard drives,
+ * LEDs, etc while the main SoC stays running - including its network ports.
+ */
+static int qnap_mcu_power_off(struct sys_off_data *data)
+{
+	const u8 cmd[] = { '@', 'C', '0' };
+	struct qnap_mcu *mcu = data->cb_data;
+	int ret;
+
+	ret = qnap_mcu_exec_with_ack(mcu, cmd, sizeof(cmd));
+	if (ret) {
+		dev_err(&mcu->serdev->dev, "MCU poweroff failed %d\n", ret);
+		return NOTIFY_STOP;
+	}
+
+	return NOTIFY_DONE;
+}
+
+static const struct qnap_mcu_variant qnap_ts433_mcu = {
+	.baud_rate = 115200,
+	.num_drives = 4,
+	.fan_pwm_min = 51,  /* Specified in original model.conf */
+	.fan_pwm_max = 255,
+	.usb_led = true,
+};
+
+static struct mfd_cell qnap_mcu_cells[] = {
+	{ .name = "qnap-mcu-input", },
+	{ .name = "qnap-mcu-leds", },
+	{ .name = "qnap-mcu-hwmon", }
+};
+
+static int qnap_mcu_probe(struct serdev_device *serdev)
+{
+	struct device *dev = &serdev->dev;
+	struct qnap_mcu *mcu;
+	int ret;
+
+	mcu = devm_kzalloc(dev, sizeof(*mcu), GFP_KERNEL);
+	if (!mcu)
+		return -ENOMEM;
+
+	mcu->serdev = serdev;
+	dev_set_drvdata(dev, mcu);
+
+	mcu->variant = of_device_get_match_data(dev);
+	if (!mcu->variant)
+		return -ENODEV;
+
+	mutex_init(&mcu->bus_lock);
+	init_completion(&mcu->reply.done);
+
+	serdev_device_set_client_ops(serdev, &qnap_mcu_serdev_device_ops);
+	ret = devm_serdev_device_open(dev, serdev);
+	if (ret)
+		return ret;
+
+	serdev_device_set_baudrate(serdev, mcu->variant->baud_rate);
+	serdev_device_set_flow_control(serdev, false);
+
+	ret = serdev_device_set_parity(serdev, SERDEV_PARITY_NONE);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to set parity\n");
+
+	ret = qnap_mcu_get_version(mcu);
+	if (ret)
+		return ret;
+
+	ret = devm_register_sys_off_handler(dev,
+					    SYS_OFF_MODE_POWER_OFF_PREPARE,
+					    SYS_OFF_PRIO_DEFAULT,
+					    &qnap_mcu_power_off, mcu);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to register poweroff handler\n");
+
+	for (int i = 0; i < ARRAY_SIZE(qnap_mcu_cells); i++) {
+		qnap_mcu_cells[i].platform_data = mcu->variant;
+		qnap_mcu_cells[i].pdata_size = sizeof(*mcu->variant);
+	}
+
+	ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, qnap_mcu_cells,
+				   ARRAY_SIZE(qnap_mcu_cells), NULL, 0, NULL);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add child devices\n");
+
+	return 0;
+}
+
+static const struct of_device_id qnap_mcu_dt_ids[] = {
+	{ .compatible = "qnap,ts433-mcu", .data = &qnap_ts433_mcu },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, qnap_mcu_dt_ids);
+
+static struct serdev_device_driver qnap_mcu_drv = {
+	.probe = qnap_mcu_probe,
+	.driver = {
+		.name = "qnap-mcu",
+		.of_match_table = qnap_mcu_dt_ids,
+	},
+};
+module_serdev_device_driver(qnap_mcu_drv);
+
+MODULE_AUTHOR("Heiko Stuebner <heiko@sntech.de>");
+MODULE_DESCRIPTION("QNAP MCU core driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/qnap-mcu.h b/include/linux/mfd/qnap-mcu.h
new file mode 100644
index 000000000000..8d48c212fd44
--- /dev/null
+++ b/include/linux/mfd/qnap-mcu.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Core definitions for QNAP MCU MFD driver.
+ * Copyright (C) 2024 Heiko Stuebner <heiko@sntech.de>
+ */
+
+#ifndef _LINUX_QNAP_MCU_H_
+#define _LINUX_QNAP_MCU_H_
+
+struct qnap_mcu;
+
+struct qnap_mcu_variant {
+	u32 baud_rate;
+	int num_drives;
+	int fan_pwm_min;
+	int fan_pwm_max;
+	bool usb_led;
+};
+
+int qnap_mcu_exec(struct qnap_mcu *mcu,
+		  const u8 *cmd_data, size_t cmd_data_size,
+		  u8 *reply_data, size_t reply_data_size);
+int qnap_mcu_exec_with_ack(struct qnap_mcu *mcu,
+			   const u8 *cmd_data, size_t cmd_data_size);
+
+#endif /* _LINUX_QNAP_MCU_H_ */
-- 
2.45.2

[PATCH v9 5/9] leds: add driver for LEDs from qnap-mcu devices

[Heiko Stuebner]

This adds a driver that connects to the qnap-mcu mfd driver and provides
access to the LEDs on it.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 MAINTAINERS                  |   1 +
 drivers/leds/Kconfig         |  11 ++
 drivers/leds/Makefile        |   1 +
 drivers/leds/leds-qnap-mcu.c | 227 +++++++++++++++++++++++++++++++++++
 4 files changed, 240 insertions(+)
 create mode 100644 drivers/leds/leds-qnap-mcu.c

diff --git a/MAINTAINERS b/MAINTAINERS
index cd8d18f76d28..9abea0772d2b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18881,6 +18881,7 @@ F:	drivers/media/tuners/qm1d1c0042*
 QNAP MCU DRIVER
 M:	Heiko Stuebner <heiko@sntech.de>
 S:	Maintained
+F:	drivers/leds/leds-qnap-mcu.c
 F:	drivers/mfd/qnap-mcu.c
 F:	include/linux/qnap-mcu.h
 
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index b784bb74a837..28a208fa893e 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -580,6 +580,17 @@ config LEDS_PCA995X
 	  LED driver chips accessed via the I2C bus. Supported
 	  devices include PCA9955BTW, PCA9952TW and PCA9955TW.
 
+config LEDS_QNAP_MCU
+	tristate "LED Support for QNAP MCU controllers"
+	depends on LEDS_CLASS
+	depends on MFD_QNAP_MCU
+	help
+	  This option enables support for LEDs available on embedded
+	  controllers used in QNAP NAS devices.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called qnap-mcu-leds.
+
 config LEDS_WM831X_STATUS
 	tristate "LED support for status LEDs on WM831x PMICs"
 	depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 18afbb5a23ee..c6f74865d729 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -79,6 +79,7 @@ obj-$(CONFIG_LEDS_PCA995X)		+= leds-pca995x.o
 obj-$(CONFIG_LEDS_PM8058)		+= leds-pm8058.o
 obj-$(CONFIG_LEDS_POWERNV)		+= leds-powernv.o
 obj-$(CONFIG_LEDS_PWM)			+= leds-pwm.o
+obj-$(CONFIG_LEDS_QNAP_MCU)		+= leds-qnap-mcu.o
 obj-$(CONFIG_LEDS_REGULATOR)		+= leds-regulator.o
 obj-$(CONFIG_LEDS_SC27XX_BLTC)		+= leds-sc27xx-bltc.o
 obj-$(CONFIG_LEDS_SUN50I_A100)		+= leds-sun50i-a100.o
diff --git a/drivers/leds/leds-qnap-mcu.c b/drivers/leds/leds-qnap-mcu.c
new file mode 100644
index 000000000000..4e4709456261
--- /dev/null
+++ b/drivers/leds/leds-qnap-mcu.c
@@ -0,0 +1,227 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Driver for LEDs found on QNAP MCU devices
+ *
+ * Copyright (C) 2024 Heiko Stuebner <heiko@sntech.de>
+ */
+
+#include <linux/leds.h>
+#include <linux/mfd/qnap-mcu.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <uapi/linux/uleds.h>
+
+enum qnap_mcu_err_led_mode {
+	QNAP_MCU_ERR_LED_ON = 0,
+	QNAP_MCU_ERR_LED_OFF = 1,
+	QNAP_MCU_ERR_LED_BLINK_FAST = 2,
+	QNAP_MCU_ERR_LED_BLINK_SLOW = 3,
+};
+
+struct qnap_mcu_err_led {
+	struct qnap_mcu *mcu;
+	struct led_classdev cdev;
+	char name[LED_MAX_NAME_SIZE];
+	u8 num;
+	u8 mode;
+};
+
+static inline struct qnap_mcu_err_led *
+		cdev_to_qnap_mcu_err_led(struct led_classdev *led_cdev)
+{
+	return container_of(led_cdev, struct qnap_mcu_err_led, cdev);
+}
+
+static int qnap_mcu_err_led_set(struct led_classdev *led_cdev,
+				enum led_brightness brightness)
+{
+	struct qnap_mcu_err_led *err_led = cdev_to_qnap_mcu_err_led(led_cdev);
+	u8 cmd[] = { '@', 'R', '0' + err_led->num, '0' };
+
+	/* Don't disturb a possible set blink-mode if LED stays on */
+	if (brightness != 0 && err_led->mode >= QNAP_MCU_ERR_LED_BLINK_FAST)
+		return 0;
+
+	err_led->mode = brightness ? QNAP_MCU_ERR_LED_ON : QNAP_MCU_ERR_LED_OFF;
+	cmd[3] = '0' + err_led->mode;
+
+	return qnap_mcu_exec_with_ack(err_led->mcu, cmd, sizeof(cmd));
+}
+
+static int qnap_mcu_err_led_blink_set(struct led_classdev *led_cdev,
+				      unsigned long *delay_on,
+				      unsigned long *delay_off)
+{
+	struct qnap_mcu_err_led *err_led = cdev_to_qnap_mcu_err_led(led_cdev);
+	u8 cmd[] = { '@', 'R', '0' + err_led->num, '0' };
+
+	/* LED is off, nothing to do */
+	if (err_led->mode == QNAP_MCU_ERR_LED_OFF)
+		return 0;
+
+	if (*delay_on < 500) {
+		*delay_on = 100;
+		*delay_off = 100;
+		err_led->mode = QNAP_MCU_ERR_LED_BLINK_FAST;
+	} else {
+		*delay_on = 500;
+		*delay_off = 500;
+		err_led->mode = QNAP_MCU_ERR_LED_BLINK_SLOW;
+	}
+
+	cmd[3] = '0' + err_led->mode;
+
+	return qnap_mcu_exec_with_ack(err_led->mcu, cmd, sizeof(cmd));
+}
+
+static int qnap_mcu_register_err_led(struct device *dev, struct qnap_mcu *mcu, int num_err_led)
+{
+	struct qnap_mcu_err_led *err_led;
+	int ret;
+
+	err_led = devm_kzalloc(dev, sizeof(*err_led), GFP_KERNEL);
+	if (!err_led)
+		return -ENOMEM;
+
+	err_led->mcu = mcu;
+	err_led->num = num_err_led;
+	err_led->mode = QNAP_MCU_ERR_LED_OFF;
+
+	scnprintf(err_led->name, LED_MAX_NAME_SIZE, "hdd%d:red:status", num_err_led + 1);
+	err_led->cdev.name = err_led->name;
+
+	err_led->cdev.brightness_set_blocking = qnap_mcu_err_led_set;
+	err_led->cdev.blink_set = qnap_mcu_err_led_blink_set;
+	err_led->cdev.brightness = 0;
+	err_led->cdev.max_brightness = 1;
+
+	ret = devm_led_classdev_register(dev, &err_led->cdev);
+	if (ret)
+		return ret;
+
+	return qnap_mcu_err_led_set(&err_led->cdev, 0);
+}
+
+enum qnap_mcu_usb_led_mode {
+	QNAP_MCU_USB_LED_ON = 1,
+	QNAP_MCU_USB_LED_OFF = 3,
+	QNAP_MCU_USB_LED_BLINK = 2,
+};
+
+struct qnap_mcu_usb_led {
+	struct qnap_mcu *mcu;
+	struct led_classdev cdev;
+	u8 mode;
+};
+
+static inline struct qnap_mcu_usb_led *
+		cdev_to_qnap_mcu_usb_led(struct led_classdev *led_cdev)
+{
+	return container_of(led_cdev, struct qnap_mcu_usb_led, cdev);
+}
+
+static int qnap_mcu_usb_led_set(struct led_classdev *led_cdev,
+				enum led_brightness brightness)
+{
+	struct qnap_mcu_usb_led *usb_led = cdev_to_qnap_mcu_usb_led(led_cdev);
+	u8 cmd[] = { '@', 'C', 0 };
+
+	/* Don't disturb a possible set blink-mode if LED stays on */
+	if (brightness != 0 && usb_led->mode == QNAP_MCU_USB_LED_BLINK)
+		return 0;
+
+	usb_led->mode = brightness ? QNAP_MCU_USB_LED_ON : QNAP_MCU_USB_LED_OFF;
+
+	/*
+	 * Byte 3 is shared between the usb led target on/off/blink
+	 * and also the buzzer control (in the input driver)
+	 */
+	cmd[2] = 'D' + usb_led->mode;
+
+	return qnap_mcu_exec_with_ack(usb_led->mcu, cmd, sizeof(cmd));
+}
+
+static int qnap_mcu_usb_led_blink_set(struct led_classdev *led_cdev,
+				      unsigned long *delay_on,
+				      unsigned long *delay_off)
+{
+	struct qnap_mcu_usb_led *usb_led = cdev_to_qnap_mcu_usb_led(led_cdev);
+	u8 cmd[] = { '@', 'C', 0 };
+
+	/* LED is off, nothing to do */
+	if (usb_led->mode == QNAP_MCU_USB_LED_OFF)
+		return 0;
+
+	*delay_on = 250;
+	*delay_off = 250;
+	usb_led->mode = QNAP_MCU_USB_LED_BLINK;
+
+	/*
+	 * Byte 3 is shared between the USB LED target on/off/blink
+	 * and also the buzzer control (in the input driver)
+	 */
+	cmd[2] = 'D' + usb_led->mode;
+
+	return qnap_mcu_exec_with_ack(usb_led->mcu, cmd, sizeof(cmd));
+}
+
+static int qnap_mcu_register_usb_led(struct device *dev, struct qnap_mcu *mcu)
+{
+	struct qnap_mcu_usb_led *usb_led;
+	int ret;
+
+	usb_led = devm_kzalloc(dev, sizeof(*usb_led), GFP_KERNEL);
+	if (!usb_led)
+		return -ENOMEM;
+
+	usb_led->mcu = mcu;
+	usb_led->mode = QNAP_MCU_USB_LED_OFF;
+	usb_led->cdev.name = "usb:blue:disk";
+	usb_led->cdev.brightness_set_blocking = qnap_mcu_usb_led_set;
+	usb_led->cdev.blink_set = qnap_mcu_usb_led_blink_set;
+	usb_led->cdev.brightness = 0;
+	usb_led->cdev.max_brightness = 1;
+
+	ret = devm_led_classdev_register(dev, &usb_led->cdev);
+	if (ret)
+		return ret;
+
+	return qnap_mcu_usb_led_set(&usb_led->cdev, 0);
+}
+
+static int qnap_mcu_leds_probe(struct platform_device *pdev)
+{
+	struct qnap_mcu *mcu = dev_get_drvdata(pdev->dev.parent);
+	const struct qnap_mcu_variant *variant = pdev->dev.platform_data;
+	int ret;
+
+	for (int i = 0; i < variant->num_drives; i++) {
+		ret = qnap_mcu_register_err_led(&pdev->dev, mcu, i);
+		if (ret)
+			return dev_err_probe(&pdev->dev, ret,
+					"failed to register error LED %d\n", i);
+	}
+
+	if (variant->usb_led) {
+		ret = qnap_mcu_register_usb_led(&pdev->dev, mcu);
+		if (ret)
+			return dev_err_probe(&pdev->dev, ret,
+					"failed to register USB LED\n");
+	}
+
+	return 0;
+}
+
+static struct platform_driver qnap_mcu_leds_driver = {
+	.probe = qnap_mcu_leds_probe,
+	.driver = {
+		.name = "qnap-mcu-leds",
+	},
+};
+module_platform_driver(qnap_mcu_leds_driver);
+
+MODULE_ALIAS("platform:qnap-mcu-leds");
+MODULE_AUTHOR("Heiko Stuebner <heiko@sntech.de>");
+MODULE_DESCRIPTION("QNAP MCU LEDs driver");
+MODULE_LICENSE("GPL");
-- 
2.45.2

[PATCH v9 6/9] Input: add driver for the input part of qnap-mcu devices

[Heiko Stuebner]

The MCU controls the power-button and beeper, so expose them as input
device. There is of course no interrupt line, so the status of the
power-button needs to be polled. To generate an event the power-button
also needs to be held for 1-2 seconds, so the polling interval does
not need to be overly fast.

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 MAINTAINERS                         |   1 +
 drivers/input/misc/Kconfig          |  12 +++
 drivers/input/misc/Makefile         |   1 +
 drivers/input/misc/qnap-mcu-input.c | 153 ++++++++++++++++++++++++++++
 4 files changed, 167 insertions(+)
 create mode 100644 drivers/input/misc/qnap-mcu-input.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 9abea0772d2b..9e66441c50da 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18881,6 +18881,7 @@ F:	drivers/media/tuners/qm1d1c0042*
 QNAP MCU DRIVER
 M:	Heiko Stuebner <heiko@sntech.de>
 S:	Maintained
+F:	drivers/input/misc/qnap-mcu-input.c
 F:	drivers/leds/leds-qnap-mcu.c
 F:	drivers/mfd/qnap-mcu.c
 F:	include/linux/qnap-mcu.h
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 6a852c76331b..13d135257e06 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -917,6 +917,18 @@ config INPUT_HISI_POWERKEY
 	  To compile this driver as a module, choose M here: the
 	  module will be called hisi_powerkey.
 
+config INPUT_QNAP_MCU
+	tristate "Input Support for QNAP MCU controllers"
+	depends on MFD_QNAP_MCU
+	help
+	  This option enables support for input elements available on
+	  embedded controllers used in QNAP NAS devices.
+
+	  This includes a polled power-button as well as a beeper.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called qnap-mcu-input.
+
 config INPUT_RAVE_SP_PWRBUTTON
 	tristate "RAVE SP Power button Driver"
 	depends on RAVE_SP_CORE
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 4f7f736831ba..6d91804d0a6f 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -68,6 +68,7 @@ obj-$(CONFIG_INPUT_PMIC8XXX_PWRKEY)	+= pmic8xxx-pwrkey.o
 obj-$(CONFIG_INPUT_POWERMATE)		+= powermate.o
 obj-$(CONFIG_INPUT_PWM_BEEPER)		+= pwm-beeper.o
 obj-$(CONFIG_INPUT_PWM_VIBRA)		+= pwm-vibra.o
+obj-$(CONFIG_INPUT_QNAP_MCU)		+= qnap-mcu-input.o
 obj-$(CONFIG_INPUT_RAVE_SP_PWRBUTTON)	+= rave-sp-pwrbutton.o
 obj-$(CONFIG_INPUT_RB532_BUTTON)	+= rb532_button.o
 obj-$(CONFIG_INPUT_REGULATOR_HAPTIC)	+= regulator-haptic.o
diff --git a/drivers/input/misc/qnap-mcu-input.c b/drivers/input/misc/qnap-mcu-input.c
new file mode 100644
index 000000000000..76e62f0816c1
--- /dev/null
+++ b/drivers/input/misc/qnap-mcu-input.c
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/*
+ * Driver for input events on QNAP-MCUs
+ *
+ * Copyright (C) 2024 Heiko Stuebner <heiko@sntech.de>
+ */
+
+#include <linux/input.h>
+#include <linux/mfd/qnap-mcu.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <uapi/linux/input-event-codes.h>
+
+/*
+ * The power-key needs to be pressed for a while to create an event,
+ * so there is no use for overly frequent polling.
+ */
+#define POLL_INTERVAL		500
+
+struct qnap_mcu_input_dev {
+	struct input_dev *input;
+	struct qnap_mcu *mcu;
+	struct device *dev;
+
+	struct work_struct beep_work;
+	int beep_type;
+};
+
+static void qnap_mcu_input_poll(struct input_dev *input)
+{
+	struct qnap_mcu_input_dev *idev = input_get_drvdata(input);
+	static const u8 cmd[] = { '@', 'C', 'V' };
+	u8 reply[4];
+	int state, ret;
+
+	/* poll the power button */
+	ret = qnap_mcu_exec(idev->mcu, cmd, sizeof(cmd), reply, sizeof(reply));
+	if (ret)
+		return;
+
+	/* First bytes must mirror the sent command */
+	if (memcmp(cmd, reply, sizeof(cmd))) {
+		dev_err(idev->dev, "malformed data received\n");
+		return;
+	}
+
+	state = reply[3] - 0x30;
+	input_event(input, EV_KEY, KEY_POWER, state);
+	input_sync(input);
+}
+
+static void qnap_mcu_input_beeper_work(struct work_struct *work)
+{
+	struct qnap_mcu_input_dev *idev =
+		container_of(work, struct qnap_mcu_input_dev, beep_work);
+	const u8 cmd[] = { '@', 'C', (idev->beep_type == SND_TONE) ? '3' : '2' };
+
+	qnap_mcu_exec_with_ack(idev->mcu, cmd, sizeof(cmd));
+}
+
+static int qnap_mcu_input_event(struct input_dev *input, unsigned int type,
+				unsigned int code, int value)
+{
+	struct qnap_mcu_input_dev *idev = input_get_drvdata(input);
+
+	if (type != EV_SND || (code != SND_BELL && code != SND_TONE))
+		return -EOPNOTSUPP;
+
+	if (value < 0)
+		return -EINVAL;
+
+	/* beep runtime is determined by the MCU */
+	if (value == 0)
+		return 0;
+
+	/* Schedule work to actually turn the beeper on */
+	idev->beep_type = code;
+	schedule_work(&idev->beep_work);
+
+	return 0;
+}
+
+static void qnap_mcu_input_close(struct input_dev *input)
+{
+	struct qnap_mcu_input_dev *idev = input_get_drvdata(input);
+
+	cancel_work_sync(&idev->beep_work);
+}
+
+static int qnap_mcu_input_probe(struct platform_device *pdev)
+{
+	struct qnap_mcu *mcu = dev_get_drvdata(pdev->dev.parent);
+	struct qnap_mcu_input_dev *idev;
+	struct device *dev = &pdev->dev;
+	struct input_dev *input;
+	int ret;
+
+	idev = devm_kzalloc(dev, sizeof(*idev), GFP_KERNEL);
+	if (!idev)
+		return -ENOMEM;
+
+	input = devm_input_allocate_device(dev);
+	if (!input)
+		return dev_err_probe(dev, -ENOMEM, "no memory for input device\n");
+
+	idev->input = input;
+	idev->dev = dev;
+	idev->mcu = mcu;
+
+	input_set_drvdata(input, idev);
+
+	input->name		= "qnap-mcu";
+	input->phys		= "qnap-mcu-input/input0";
+	input->id.bustype	= BUS_HOST;
+	input->id.vendor	= 0x0001;
+	input->id.product	= 0x0001;
+	input->id.version	= 0x0100;
+	input->event		= qnap_mcu_input_event;
+	input->close		= qnap_mcu_input_close;
+
+	input_set_capability(input, EV_KEY, KEY_POWER);
+	input_set_capability(input, EV_SND, SND_BELL);
+	input_set_capability(input, EV_SND, SND_TONE);
+
+	INIT_WORK(&idev->beep_work, qnap_mcu_input_beeper_work);
+
+	ret = input_setup_polling(input, qnap_mcu_input_poll);
+	if (ret)
+		return dev_err_probe(dev, ret, "unable to set up polling\n");
+
+	input_set_poll_interval(input, POLL_INTERVAL);
+
+	ret = input_register_device(input);
+	if (ret)
+		return dev_err_probe(dev, ret, "unable to register input device\n");
+
+	return 0;
+}
+
+static struct platform_driver qnap_mcu_input_driver = {
+	.probe = qnap_mcu_input_probe,
+	.driver = {
+		.name = "qnap-mcu-input",
+	},
+};
+module_platform_driver(qnap_mcu_input_driver);
+
+MODULE_ALIAS("platform:qnap-mcu-input");
+MODULE_AUTHOR("Heiko Stuebner <heiko@sntech.de>");
+MODULE_DESCRIPTION("QNAP MCU input driver");
+MODULE_LICENSE("GPL");
-- 
2.45.2

[PATCH v9 7/9] hwmon: add driver for the hwmon parts of qnap-mcu devices

[Heiko Stuebner]

The MCU can be found on network-attached-storage devices made by QNAP
and provides access to fan control including reading back its RPM as
well as reading the temperature of the NAS case.

Acked-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 Documentation/hwmon/index.rst          |   1 +
 Documentation/hwmon/qnap-mcu-hwmon.rst |  27 ++
 MAINTAINERS                            |   1 +
 drivers/hwmon/Kconfig                  |  12 +
 drivers/hwmon/Makefile                 |   1 +
 drivers/hwmon/qnap-mcu-hwmon.c         | 364 +++++++++++++++++++++++++
 6 files changed, 406 insertions(+)
 create mode 100644 Documentation/hwmon/qnap-mcu-hwmon.rst
 create mode 100644 drivers/hwmon/qnap-mcu-hwmon.c

diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index ea3b5be8fe4f..e134a4f8558f 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -199,6 +199,7 @@ Hardware Monitoring Kernel Drivers
    pxe1610
    pwm-fan
    q54sj108a2
+   qnap-mcu-hwmon
    raspberrypi-hwmon
    sbrmi
    sbtsi_temp
diff --git a/Documentation/hwmon/qnap-mcu-hwmon.rst b/Documentation/hwmon/qnap-mcu-hwmon.rst
new file mode 100644
index 000000000000..83407e3408f2
--- /dev/null
+++ b/Documentation/hwmon/qnap-mcu-hwmon.rst
@@ -0,0 +1,27 @@
+.. SPDX-License-Identifier: GPL-2.0-or-later
+
+Kernel driver qnap-mcu-hwmon
+============================
+
+This driver enables the use of the hardware monitoring and fan control
+of the MCU used on some QNAP network attached storage devices.
+
+Author: Heiko Stuebner <heiko@sntech.de>
+
+Description
+-----------
+
+The driver implements a simple interface for driving the fan controlled by
+setting its PWM output value and exposes the fan rpm and case-temperature
+to user space through hwmon's sysfs interface.
+
+The fan rotation speed returned via the optional 'fan1_input' is calculated
+inside the MCU device.
+
+The driver provides the following sensor accesses in sysfs:
+
+=============== ======= =======================================================
+fan1_input	ro	fan tachometer speed in RPM
+pwm1		rw	relative speed (0-255), 255=max. speed.
+temp1_input	ro	Measured temperature in millicelsius
+=============== ======= =======================================================
diff --git a/MAINTAINERS b/MAINTAINERS
index 9e66441c50da..4e6719d15d75 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18881,6 +18881,7 @@ F:	drivers/media/tuners/qm1d1c0042*
 QNAP MCU DRIVER
 M:	Heiko Stuebner <heiko@sntech.de>
 S:	Maintained
+F:	drivers/hwmon/qnap-mcu-hwmon.c
 F:	drivers/input/misc/qnap-mcu-input.c
 F:	drivers/leds/leds-qnap-mcu.c
 F:	drivers/mfd/qnap-mcu.c
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 08a3c863f80a..74d33a96f7cc 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1800,6 +1800,18 @@ config SENSORS_PWM_FAN
 	  This driver can also be built as a module. If so, the module
 	  will be called pwm-fan.
 
+config SENSORS_QNAP_MCU_HWMON
+	tristate "QNAP MCU hardware monitoring"
+	depends on MFD_QNAP_MCU
+	depends on THERMAL || THERMAL=n
+	help
+	  Say yes here to enable support for fan and temperature sensor
+	  connected to a QNAP MCU, as found in a number of QNAP network
+	  attached storage devices.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called qnap-mcu-hwmon.
+
 config SENSORS_RASPBERRYPI_HWMON
 	tristate "Raspberry Pi voltage monitor"
 	depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST && !RASPBERRYPI_FIRMWARE)
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 9554d2fdcf7b..872f70f9902d 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -187,6 +187,7 @@ obj-$(CONFIG_SENSORS_POWERZ)	+= powerz.o
 obj-$(CONFIG_SENSORS_POWR1220)  += powr1220.o
 obj-$(CONFIG_SENSORS_PT5161L)	+= pt5161l.o
 obj-$(CONFIG_SENSORS_PWM_FAN)	+= pwm-fan.o
+obj-$(CONFIG_SENSORS_QNAP_MCU_HWMON)	+= qnap-mcu-hwmon.o
 obj-$(CONFIG_SENSORS_RASPBERRYPI_HWMON)	+= raspberrypi-hwmon.o
 obj-$(CONFIG_SENSORS_SBTSI)	+= sbtsi_temp.o
 obj-$(CONFIG_SENSORS_SBRMI)	+= sbrmi.o
diff --git a/drivers/hwmon/qnap-mcu-hwmon.c b/drivers/hwmon/qnap-mcu-hwmon.c
new file mode 100644
index 000000000000..29057514739c
--- /dev/null
+++ b/drivers/hwmon/qnap-mcu-hwmon.c
@@ -0,0 +1,364 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/*
+ * Driver for hwmon elements found on QNAP-MCU devices
+ *
+ * Copyright (C) 2024 Heiko Stuebner <heiko@sntech.de>
+ */
+
+#include <linux/fwnode.h>
+#include <linux/hwmon.h>
+#include <linux/mfd/qnap-mcu.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/thermal.h>
+
+struct qnap_mcu_hwmon {
+	struct qnap_mcu *mcu;
+	struct device *dev;
+
+	unsigned int pwm_min;
+	unsigned int pwm_max;
+
+	struct fwnode_handle *fan_node;
+	unsigned int fan_state;
+	unsigned int fan_max_state;
+	unsigned int *fan_cooling_levels;
+
+	struct thermal_cooling_device *cdev;
+	struct hwmon_chip_info info;
+};
+
+static int qnap_mcu_hwmon_get_rpm(struct qnap_mcu_hwmon *hwm)
+{
+	static const u8 cmd[] = { '@', 'F', 'A' };
+	u8 reply[6];
+	int ret;
+
+	/* poll the fan rpm */
+	ret = qnap_mcu_exec(hwm->mcu, cmd, sizeof(cmd), reply, sizeof(reply));
+	if (ret)
+		return ret;
+
+	/* First 2 bytes must mirror the sent command */
+	if (memcmp(cmd, reply, 2))
+		return -EIO;
+
+	return reply[4] * 30;
+}
+
+static int qnap_mcu_hwmon_get_pwm(struct qnap_mcu_hwmon *hwm)
+{
+	static const u8 cmd[] = { '@', 'F', 'Z', '0' }; /* 0 = fan-id? */
+	u8 reply[4];
+	int ret;
+
+	/* poll the fan pwm */
+	ret = qnap_mcu_exec(hwm->mcu, cmd, sizeof(cmd), reply, sizeof(reply));
+	if (ret)
+		return ret;
+
+	/* First 3 bytes must mirror the sent command */
+	if (memcmp(cmd, reply, 3))
+		return -EIO;
+
+	return reply[3];
+}
+
+static int qnap_mcu_hwmon_set_pwm(struct qnap_mcu_hwmon *hwm, u8 pwm)
+{
+	const u8 cmd[] = { '@', 'F', 'W', '0', pwm }; /* 0 = fan-id?, pwm 0-255 */
+
+	/* set the fan pwm */
+	return qnap_mcu_exec_with_ack(hwm->mcu, cmd, sizeof(cmd));
+}
+
+static int qnap_mcu_hwmon_get_temp(struct qnap_mcu_hwmon *hwm)
+{
+	static const u8 cmd[] = { '@', 'T', '3' };
+	u8 reply[4];
+	int ret;
+
+	/* poll the fan rpm */
+	ret = qnap_mcu_exec(hwm->mcu, cmd, sizeof(cmd), reply, sizeof(reply));
+	if (ret)
+		return ret;
+
+	/* First bytes must mirror the sent command */
+	if (memcmp(cmd, reply, sizeof(cmd)))
+		return -EIO;
+
+	/*
+	 * There is an unknown bit set in bit7.
+	 * Bits [6:0] report the actual temperature as returned by the
+	 * original qnap firmware-tools, so just drop bit7 for now.
+	 */
+	return (reply[3] & 0x7f) * 1000;
+}
+
+static int qnap_mcu_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
+				u32 attr, int channel, long val)
+{
+	struct qnap_mcu_hwmon *hwm = dev_get_drvdata(dev);
+
+	switch (attr) {
+	case hwmon_pwm_input:
+		if (val < 0 || val > 255)
+			return -EINVAL;
+
+		if (val != 0)
+			val = clamp_val(val, hwm->pwm_min, hwm->pwm_max);
+
+		return qnap_mcu_hwmon_set_pwm(hwm, val);
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
+static int qnap_mcu_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+			       u32 attr, int channel, long *val)
+{
+	struct qnap_mcu_hwmon *hwm = dev_get_drvdata(dev);
+	int ret;
+
+	switch (type) {
+	case hwmon_pwm:
+		switch (attr) {
+		case hwmon_pwm_input:
+			ret = qnap_mcu_hwmon_get_pwm(hwm);
+			if (ret < 0)
+				return ret;
+
+			*val = ret;
+			return 0;
+		default:
+			return -EOPNOTSUPP;
+		}
+	case hwmon_fan:
+		ret = qnap_mcu_hwmon_get_rpm(hwm);
+		if (ret < 0)
+			return ret;
+
+		*val = ret;
+		return 0;
+	case hwmon_temp:
+		ret = qnap_mcu_hwmon_get_temp(hwm);
+		if (ret < 0)
+			return ret;
+
+		*val = ret;
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static umode_t qnap_mcu_hwmon_is_visible(const void *data,
+					 enum hwmon_sensor_types type,
+					 u32 attr, int channel)
+{
+	switch (type) {
+	case hwmon_temp:
+		return 0444;
+
+	case hwmon_pwm:
+		return 0644;
+
+	case hwmon_fan:
+		return 0444;
+
+	default:
+		return 0;
+	}
+}
+
+static const struct hwmon_ops qnap_mcu_hwmon_hwmon_ops = {
+	.is_visible = qnap_mcu_hwmon_is_visible,
+	.read = qnap_mcu_hwmon_read,
+	.write = qnap_mcu_hwmon_write,
+};
+
+/* thermal cooling device callbacks */
+static int qnap_mcu_hwmon_get_max_state(struct thermal_cooling_device *cdev,
+					unsigned long *state)
+{
+	struct qnap_mcu_hwmon *hwm = cdev->devdata;
+
+	if (!hwm)
+		return -EINVAL;
+
+	*state = hwm->fan_max_state;
+
+	return 0;
+}
+
+static int qnap_mcu_hwmon_get_cur_state(struct thermal_cooling_device *cdev,
+					unsigned long *state)
+{
+	struct qnap_mcu_hwmon *hwm = cdev->devdata;
+
+	if (!hwm)
+		return -EINVAL;
+
+	*state = hwm->fan_state;
+
+	return 0;
+}
+
+static int qnap_mcu_hwmon_set_cur_state(struct thermal_cooling_device *cdev,
+					unsigned long state)
+{
+	struct qnap_mcu_hwmon *hwm = cdev->devdata;
+	int ret;
+
+	if (!hwm || state > hwm->fan_max_state)
+		return -EINVAL;
+
+	if (state == hwm->fan_state)
+		return 0;
+
+	ret = qnap_mcu_hwmon_set_pwm(hwm, hwm->fan_cooling_levels[state]);
+	if (ret)
+		return ret;
+
+	hwm->fan_state = state;
+
+	return ret;
+}
+
+static const struct thermal_cooling_device_ops qnap_mcu_hwmon_cooling_ops = {
+	.get_max_state = qnap_mcu_hwmon_get_max_state,
+	.get_cur_state = qnap_mcu_hwmon_get_cur_state,
+	.set_cur_state = qnap_mcu_hwmon_set_cur_state,
+};
+
+static void devm_fan_node_release(void *data)
+{
+	struct qnap_mcu_hwmon *hwm = data;
+
+	fwnode_handle_put(hwm->fan_node);
+}
+
+static int qnap_mcu_hwmon_get_cooling_data(struct device *dev, struct qnap_mcu_hwmon *hwm)
+{
+	struct fwnode_handle *fwnode;
+	int num, i, ret;
+
+	fwnode = device_get_named_child_node(dev->parent, "fan-0");
+	if (!fwnode)
+		return 0;
+
+	/* if we found the fan-node, we're keeping it until device-unbind */
+	hwm->fan_node = fwnode;
+	ret = devm_add_action_or_reset(dev, devm_fan_node_release, hwm);
+	if (ret)
+		return ret;
+
+	num = fwnode_property_count_u32(fwnode, "cooling-levels");
+	if (num <= 0)
+		return dev_err_probe(dev, num ? : -EINVAL,
+				     "Failed to count elements in 'cooling-levels'\n");
+
+	hwm->fan_cooling_levels = devm_kcalloc(dev, num, sizeof(u32),
+					       GFP_KERNEL);
+	if (!hwm->fan_cooling_levels)
+		return -ENOMEM;
+
+	ret = fwnode_property_read_u32_array(fwnode, "cooling-levels",
+					     hwm->fan_cooling_levels, num);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to read 'cooling-levels'\n");
+
+	for (i = 0; i < num; i++) {
+		if (hwm->fan_cooling_levels[i] > hwm->pwm_max)
+			return dev_err_probe(dev, -EINVAL, "fan state[%d]:%d > %d\n", i,
+					     hwm->fan_cooling_levels[i], hwm->pwm_max);
+	}
+
+	hwm->fan_max_state = num - 1;
+
+	return 0;
+}
+
+static const struct hwmon_channel_info * const qnap_mcu_hwmon_channels[] = {
+	HWMON_CHANNEL_INFO(pwm, HWMON_PWM_INPUT),
+	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT),
+	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
+	NULL
+};
+
+static int qnap_mcu_hwmon_probe(struct platform_device *pdev)
+{
+	struct qnap_mcu *mcu = dev_get_drvdata(pdev->dev.parent);
+	const struct qnap_mcu_variant *variant = pdev->dev.platform_data;
+	struct qnap_mcu_hwmon *hwm;
+	struct thermal_cooling_device *cdev;
+	struct device *dev = &pdev->dev;
+	struct device *hwmon;
+	int ret;
+
+	hwm = devm_kzalloc(dev, sizeof(*hwm), GFP_KERNEL);
+	if (!hwm)
+		return -ENOMEM;
+
+	hwm->mcu = mcu;
+	hwm->dev = &pdev->dev;
+	hwm->pwm_min = variant->fan_pwm_min;
+	hwm->pwm_max = variant->fan_pwm_max;
+
+	platform_set_drvdata(pdev, hwm);
+
+	/*
+	 * Set duty cycle to maximum allowed.
+	 */
+	ret = qnap_mcu_hwmon_set_pwm(hwm, hwm->pwm_max);
+	if (ret)
+		return ret;
+
+	hwm->info.ops = &qnap_mcu_hwmon_hwmon_ops;
+	hwm->info.info = qnap_mcu_hwmon_channels;
+
+	ret = qnap_mcu_hwmon_get_cooling_data(dev, hwm);
+	if (ret)
+		return ret;
+
+	hwm->fan_state = hwm->fan_max_state;
+
+	hwmon = devm_hwmon_device_register_with_info(dev, "qnapmcu",
+						     hwm, &hwm->info, NULL);
+	if (IS_ERR(hwmon))
+		return dev_err_probe(dev, PTR_ERR(hwmon), "Failed to register hwmon device\n");
+
+	/*
+	 * Only register cooling device when we found cooling-levels.
+	 * qnap_mcu_hwmon_get_cooling_data() will fail when reading malformed
+	 * levels and only succeed with either no or correct cooling levels.
+	 */
+	if (IS_ENABLED(CONFIG_THERMAL) && hwm->fan_cooling_levels) {
+		cdev = devm_thermal_of_cooling_device_register(dev,
+					to_of_node(hwm->fan_node), "qnap-mcu-hwmon",
+					hwm, &qnap_mcu_hwmon_cooling_ops);
+		if (IS_ERR(cdev))
+			return dev_err_probe(dev, PTR_ERR(cdev),
+				"Failed to register qnap-mcu-hwmon as cooling device\n");
+		hwm->cdev = cdev;
+	}
+
+	return 0;
+}
+
+static struct platform_driver qnap_mcu_hwmon_driver = {
+	.probe = qnap_mcu_hwmon_probe,
+	.driver = {
+		.name = "qnap-mcu-hwmon",
+	},
+};
+module_platform_driver(qnap_mcu_hwmon_driver);
+
+MODULE_ALIAS("platform:qnap-mcu-hwmon");
+MODULE_AUTHOR("Heiko Stuebner <heiko@sntech.de>");
+MODULE_DESCRIPTION("QNAP MCU hwmon driver");
+MODULE_LICENSE("GPL");
-- 
2.45.2

[PATCH v9 8/9] arm64: dts: rockchip: hook up the MCU on the QNAP TS433

[Heiko Stuebner]

The MCU is an important part of the device functionality. It provides
functionality like fan-control, more leds, etc and even more important
without it, the NAS-device cannot even fully turned off.

Hook up the serial device to its uart and hook into the thermal
management to control the fan according to the cpu temperature.

While the MCU also provides a temperature sensor for the case, this one
is just polled and does not provide functionality for handling trip
points in hardware, so a lot of polling would be involved.
As the cpu is only cooled passively in these devices, it's temperature
rising will indicate the temperature level of the system just earlier.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 .../boot/dts/rockchip/rk3568-qnap-ts433.dts   | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts b/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
index e601d9271ba8..4bc5f5691d45 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
@@ -483,6 +483,54 @@ rgmii_phy0: ethernet-phy@0 {
 	};
 };
 
+/*
+ * The MCU can provide system temperature too, but only by polling and of
+ * course also cannot set trip points. So attach to the cpu thermal-zone
+ * instead to control the fan.
+ */
+&cpu_thermal {
+	trips {
+		case_fan0: case-fan0 {
+			hysteresis = <2000>;
+			temperature = <35000>;
+			type = "active";
+		};
+
+		case_fan1: case-fan1 {
+			hysteresis = <2000>;
+			temperature = <45000>;
+			type = "active";
+		};
+
+		case_fan2: case-fan2 {
+			hysteresis = <2000>;
+			temperature = <65000>;
+			type = "active";
+		};
+	};
+
+	cooling-maps {
+		/*
+		 * Always provide some air movement, due to small case
+		 * full of harddrives.
+		 */
+		map1 {
+			cooling-device = <&fan THERMAL_NO_LIMIT 1>;
+			trip = <&case_fan0>;
+		};
+
+		map2 {
+			cooling-device = <&fan 2 3>;
+			trip = <&case_fan1>;
+		};
+
+		map3 {
+			cooling-device = <&fan 4 THERMAL_NO_LIMIT>;
+			trip = <&case_fan2>;
+		};
+	};
+};
+
 &pcie30phy {
 	data-lanes = <1 2>;
 	status = "okay";
@@ -582,6 +630,15 @@ &tsadc {
  */
 &uart0 {
 	status = "okay";
+
+	mcu {
+		compatible = "qnap,ts433-mcu";
+
+		fan: fan-0 {
+			#cooling-cells = <2>;
+			cooling-levels = <0 64 89 128 166 204 221 238>;
+		};
+	};
 };
 
 /*
-- 
2.45.2

[PATCH v9 9/9] arm64: dts: rockchip: set hdd led labels on qnap-ts433

[Heiko Stuebner]

The automatically generated names for the LEDs from color and function
do not match nicely for the 4 hdds, so set them manually per the label
property to also match the LEDs generated from the MCU.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts b/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
index 4bc5f5691d45..7bd32d230ad2 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
@@ -50,6 +50,7 @@ led-0 {
 			color = <LED_COLOR_ID_GREEN>;
 			function = LED_FUNCTION_DISK;
 			gpios = <&gpio1 RK_PD5 GPIO_ACTIVE_LOW>;
+			label = "hdd1:green:disk";
 			linux,default-trigger = "disk-activity";
 			pinctrl-names = "default";
 			pinctrl-0 = <&hdd1_led_pin>;
@@ -59,6 +60,7 @@ led-1 {
 			color = <LED_COLOR_ID_GREEN>;
 			function = LED_FUNCTION_DISK;
 			gpios = <&gpio1 RK_PD6 GPIO_ACTIVE_LOW>;
+			label = "hdd2:green:disk";
 			linux,default-trigger = "disk-activity";
 			pinctrl-names = "default";
 			pinctrl-0 = <&hdd2_led_pin>;
@@ -68,6 +70,7 @@ led-2 {
 			color = <LED_COLOR_ID_GREEN>;
 			function = LED_FUNCTION_DISK;
 			gpios = <&gpio1 RK_PD7 GPIO_ACTIVE_LOW>;
+			label = "hdd3:green:disk";
 			linux,default-trigger = "disk-activity";
 			pinctrl-names = "default";
 			pinctrl-0 = <&hdd3_led_pin>;
@@ -77,6 +80,7 @@ led-3 {
 			color = <LED_COLOR_ID_GREEN>;
 			function = LED_FUNCTION_DISK;
 			gpios = <&gpio2 RK_PA0 GPIO_ACTIVE_LOW>;
+			label = "hdd4:green:disk";
 			linux,default-trigger = "disk-activity";
 			pinctrl-names = "default";
 			pinctrl-0 = <&hdd4_led_pin>;
-- 
2.45.2

Re: [PATCH v9 1/9] HID: hid-sensor-hub: don't use stale platform-data on remove

[Jiri Kosina]

On Thu, 7 Nov 2024, Heiko Stuebner wrote:

> The hid-sensor-hub creates the individual device structs and transfers them
> to the created mfd platform-devices via the platform_data in the mfd_cell.
> 
> Before e651a1da442a ("HID: hid-sensor-hub: Allow parallel synchronous reads")
> the sensor-hub was managing access centrally, with one "completion" in the
> hub's data structure, which needed to be finished on removal at the latest.
> 
> The mentioned commit then moved this central management to each hid sensor
> device, resulting on a completion in each struct hid_sensor_hub_device.
> The remove procedure was adapted to go through all sensor devices and
> finish any pending "completion".
> 
> What this didn't take into account was, platform_device_add_data() that is
> used by mfd_add{_hotplug}_devices() does a kmemdup on the submitted
> platform-data. So the data the platform-device gets is a copy of the
> original data, meaning that the device worked on a different completion
> than what sensor_hub_remove() currently wants to access.
> 
> To fix that, use device_for_each_child() to go through each child-device
> similar to how mfd_remove_devices() unregisters the devices later and
> with that get the live platform_data to finalize the correct completion.
> 
> Fixes: e651a1da442a ("HID: hid-sensor-hub: Allow parallel synchronous reads")
> Cc: stable@vger.kernel.org
> Acked-by: Benjamin Tissoires <bentiss@kernel.org>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

Acked-by: Jiri Kosina <jkosina@suse.com>

Are you planning to merge this together with the rest of the set, or do 
you want me to expedite it? I'll be happy to apply it separately as a 
proper fix.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v9 1/9] HID: hid-sensor-hub: don't use stale platform-data on remove

[Heiko Stübner]

Hi Jiri,

Am Donnerstag, 7. November 2024, 13:59:04 CET schrieb Jiri Kosina:
> On Thu, 7 Nov 2024, Heiko Stuebner wrote:
> 
> > The hid-sensor-hub creates the individual device structs and transfers them
> > to the created mfd platform-devices via the platform_data in the mfd_cell.
> > 
> > Before e651a1da442a ("HID: hid-sensor-hub: Allow parallel synchronous reads")
> > the sensor-hub was managing access centrally, with one "completion" in the
> > hub's data structure, which needed to be finished on removal at the latest.
> > 
> > The mentioned commit then moved this central management to each hid sensor
> > device, resulting on a completion in each struct hid_sensor_hub_device.
> > The remove procedure was adapted to go through all sensor devices and
> > finish any pending "completion".
> > 
> > What this didn't take into account was, platform_device_add_data() that is
> > used by mfd_add{_hotplug}_devices() does a kmemdup on the submitted
> > platform-data. So the data the platform-device gets is a copy of the
> > original data, meaning that the device worked on a different completion
> > than what sensor_hub_remove() currently wants to access.
> > 
> > To fix that, use device_for_each_child() to go through each child-device
> > similar to how mfd_remove_devices() unregisters the devices later and
> > with that get the live platform_data to finalize the correct completion.
> > 
> > Fixes: e651a1da442a ("HID: hid-sensor-hub: Allow parallel synchronous reads")
> > Cc: stable@vger.kernel.org
> > Acked-by: Benjamin Tissoires <bentiss@kernel.org>
> > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> 
> Acked-by: Jiri Kosina <jkosina@suse.com>
> 
> Are you planning to merge this together with the rest of the set, or do 
> you want me to expedite it? I'll be happy to apply it separately as a 
> proper fix.

This change was more or less a surprise find, because I wanted to make
the platform_data pointer in the mfd_cell struct const and this the hid
sensor hub stood out as doing something strange ;-) .

So patch 2 of this series actually depends on this change to not cause
build errors.

But seeing that we're after -rc6 alredy, I would assume the brunt of the
mcu series might need to wait after 6.13-rc1 anyway - but I guess that
depends on how Lee sees things ;-) .


Heiko

Re: [PATCH v9 1/9] HID: hid-sensor-hub: don't use stale platform-data on remove

[Jiri Kosina]

On Thu, 7 Nov 2024, Heiko Stübner wrote:

> This change was more or less a surprise find, because I wanted to make
> the platform_data pointer in the mfd_cell struct const and this the hid
> sensor hub stood out as doing something strange ;-) .
> 
> So patch 2 of this series actually depends on this change to not cause
> build errors.

Ah, right.

> But seeing that we're after -rc6 alredy, I would assume the brunt of the 
> mcu series might need to wait after 6.13-rc1 anyway - but I guess that 
> depends on how Lee sees things ;-) .

OK, I am keeping my hands off it for the time being.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v9 1/9] HID: hid-sensor-hub: don't use stale platform-data on remove

[Lee Jones]

On Thu, 07 Nov 2024, Jiri Kosina wrote:

> On Thu, 7 Nov 2024, Heiko Stübner wrote:
> 
> > This change was more or less a surprise find, because I wanted to make
> > the platform_data pointer in the mfd_cell struct const and this the hid
> > sensor hub stood out as doing something strange ;-) .
> > 
> > So patch 2 of this series actually depends on this change to not cause
> > build errors.
> 
> Ah, right.
> 
> > But seeing that we're after -rc6 alredy, I would assume the brunt of the 
> > mcu series might need to wait after 6.13-rc1 anyway - but I guess that 
> > depends on how Lee sees things ;-) .
> 
> OK, I am keeping my hands off it for the time being.

I can take it now with an Ack.

-- 
Lee Jones [李琼斯]

Re: [PATCH v9 1/9] HID: hid-sensor-hub: don't use stale platform-data on remove

[Lee Jones]

On Tue, 12 Nov 2024, Lee Jones wrote:

> On Thu, 07 Nov 2024, Jiri Kosina wrote:
> 
> > On Thu, 7 Nov 2024, Heiko Stübner wrote:
> > 
> > > This change was more or less a surprise find, because I wanted to make
> > > the platform_data pointer in the mfd_cell struct const and this the hid
> > > sensor hub stood out as doing something strange ;-) .
> > > 
> > > So patch 2 of this series actually depends on this change to not cause
> > > build errors.
> > 
> > Ah, right.
> > 
> > > But seeing that we're after -rc6 alredy, I would assume the brunt of the 
> > > mcu series might need to wait after 6.13-rc1 anyway - but I guess that 
> > > depends on how Lee sees things ;-) .
> > 
> > OK, I am keeping my hands off it for the time being.
> 
> I can take it now with an Ack.

Looking to apply this set now.

Ack please.

-- 
Lee Jones [李琼斯]

Re: [PATCH v9 1/9] HID: hid-sensor-hub: don't use stale platform-data on remove

[Jiri Kosina]

On Wed, 11 Dec 2024, Lee Jones wrote:

> > > > This change was more or less a surprise find, because I wanted to make
> > > > the platform_data pointer in the mfd_cell struct const and this the hid
> > > > sensor hub stood out as doing something strange ;-) .
> > > > 
> > > > So patch 2 of this series actually depends on this change to not cause
> > > > build errors.
> > > 
> > > Ah, right.
> > > 
> > > > But seeing that we're after -rc6 alredy, I would assume the brunt of the 
> > > > mcu series might need to wait after 6.13-rc1 anyway - but I guess that 
> > > > depends on how Lee sees things ;-) .
> > > 
> > > OK, I am keeping my hands off it for the time being.
> > 
> > I can take it now with an Ack.
> 
> Looking to apply this set now.
> 
> Ack please.

I'd preferer if Srinivas could ack this as the more specific maintainer. 
Srinivas, please? 

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v9 1/9] HID: hid-sensor-hub: don't use stale platform-data on remove

[Heiko Stübner]

Am Mittwoch, 11. Dezember 2024, 13:24:42 CET schrieb Jiri Kosina:
> On Wed, 11 Dec 2024, Lee Jones wrote:
> 
> > > > > This change was more or less a surprise find, because I wanted to make
> > > > > the platform_data pointer in the mfd_cell struct const and this the hid
> > > > > sensor hub stood out as doing something strange ;-) .
> > > > > 
> > > > > So patch 2 of this series actually depends on this change to not cause
> > > > > build errors.
> > > > 
> > > > Ah, right.
> > > > 
> > > > > But seeing that we're after -rc6 alredy, I would assume the brunt of the 
> > > > > mcu series might need to wait after 6.13-rc1 anyway - but I guess that 
> > > > > depends on how Lee sees things ;-) .
> > > > 
> > > > OK, I am keeping my hands off it for the time being.
> > > 
> > > I can take it now with an Ack.
> > 
> > Looking to apply this set now.
> > 
> > Ack please.
> 
> I'd preferer if Srinivas could ack this as the more specific maintainer. 
> Srinivas, please? 

The patch already includes the
   Ack from Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
from a previous version, so I guess it should be ok already?

Re: [PATCH v9 1/9] HID: hid-sensor-hub: don't use stale platform-data on remove

[Jiri Kosina]

On Wed, 11 Dec 2024, Heiko Stübner wrote:

> > > > > > This change was more or less a surprise find, because I wanted to make
> > > > > > the platform_data pointer in the mfd_cell struct const and this the hid
> > > > > > sensor hub stood out as doing something strange ;-) .
> > > > > > 
> > > > > > So patch 2 of this series actually depends on this change to not cause
> > > > > > build errors.
> > > > > 
> > > > > Ah, right.
> > > > > 
> > > > > > But seeing that we're after -rc6 alredy, I would assume the brunt of the 
> > > > > > mcu series might need to wait after 6.13-rc1 anyway - but I guess that 
> > > > > > depends on how Lee sees things ;-) .
> > > > > 
> > > > > OK, I am keeping my hands off it for the time being.
> > > > 
> > > > I can take it now with an Ack.
> > > 
> > > Looking to apply this set now.
> > > 
> > > Ack please.
> > 
> > I'd preferer if Srinivas could ack this as the more specific maintainer. 
> > Srinivas, please? 
> 
> The patch already includes the
>    Ack from Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> from a previous version, so I guess it should be ok already?

Ah, I missed that, indeed, sorry for the noise.

With that

	Acked-by: Jiri Kosina <jkosina@suse.com>

and Lee, please feel free to take it.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v9 1/9] HID: hid-sensor-hub: don't use stale platform-data on remove

[srinivas pandruvada]

On Wed, 2024-12-11 at 13:24 +0100, Jiri Kosina wrote:
> On Wed, 11 Dec 2024, Lee Jones wrote:
> 
> > > > > This change was more or less a surprise find, because I
> > > > > wanted to make
> > > > > the platform_data pointer in the mfd_cell struct const and
> > > > > this the hid
> > > > > sensor hub stood out as doing something strange ;-) .
> > > > > 
> > > > > So patch 2 of this series actually depends on this change to
> > > > > not cause
> > > > > build errors.
> > > > 
> > > > Ah, right.
> > > > 
> > > > > But seeing that we're after -rc6 alredy, I would assume the
> > > > > brunt of the 
> > > > > mcu series might need to wait after 6.13-rc1 anyway - but I
> > > > > guess that 
> > > > > depends on how Lee sees things ;-) .
> > > > 
> > > > OK, I am keeping my hands off it for the time being.
> > > 
> > > I can take it now with an Ack.
> > 
> > Looking to apply this set now.
> > 
> > Ack please.
> 
> I'd preferer if Srinivas could ack this as the more specific
> maintainer. 
> Srinivas, please? 
My ACK is already in the patch:

Fixes: e651a1da442a ("HID: hid-sensor-hub: Allow parallel synchronous
reads")
Cc: stable@vger.kernel.org
Acked-by: Benjamin Tissoires <bentiss@kernel.org>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>


Thanks,
Srinivas

> 
> Thanks,
> 

Re: [PATCH v9 1/9] HID: hid-sensor-hub: don't use stale platform-data on remove

[Lee Jones]

On Wed, 11 Dec 2024, Jiri Kosina wrote:

> On Wed, 11 Dec 2024, Heiko Stübner wrote:
> 
> > > > > > > This change was more or less a surprise find, because I wanted to make
> > > > > > > the platform_data pointer in the mfd_cell struct const and this the hid
> > > > > > > sensor hub stood out as doing something strange ;-) .
> > > > > > > 
> > > > > > > So patch 2 of this series actually depends on this change to not cause
> > > > > > > build errors.
> > > > > > 
> > > > > > Ah, right.
> > > > > > 
> > > > > > > But seeing that we're after -rc6 alredy, I would assume the brunt of the 
> > > > > > > mcu series might need to wait after 6.13-rc1 anyway - but I guess that 
> > > > > > > depends on how Lee sees things ;-) .
> > > > > > 
> > > > > > OK, I am keeping my hands off it for the time being.
> > > > > 
> > > > > I can take it now with an Ack.
> > > > 
> > > > Looking to apply this set now.
> > > > 
> > > > Ack please.
> > > 
> > > I'd preferer if Srinivas could ack this as the more specific maintainer. 
> > > Srinivas, please? 
> > 
> > The patch already includes the
> >    Ack from Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > from a previous version, so I guess it should be ok already?
> 
> Ah, I missed that, indeed, sorry for the noise.
> 
> With that
> 
> 	Acked-by: Jiri Kosina <jkosina@suse.com>
> 
> and Lee, please feel free to take it.

Thanks, will do.

-- 
Lee Jones [李琼斯]

Re: [PATCH v9 0/9] Drivers to support the MCU on QNAP NAS devices

[Lee Jones]

On Thu, 07 Nov 2024 12:47:03 +0100, Heiko Stuebner wrote:
> This implements a set of drivers for the MCU used on QNAP NAS devices.
> 
> Of course no documentation for the serial protocol is available, so
> thankfully QNAP has a tool on their rescue-inird to talk to the MCU and
> I found interceptty [0] to listen to what goes over the serial connection.
> 
> In general it looks like there are two different generations in general,
> an "EC" device and now this "MCU" - referenced in the strings of the
> userspace handlers for those devices.
> 
> [...]

Applied, thanks!

[1/9] HID: hid-sensor-hub: don't use stale platform-data on remove
      commit: e079a120f31e3f9c00180aa13c1df18cc138f7fe
[2/9] mfd: core: make platform_data pointer const in struct mfd_cell
      commit: 8f4009ad901c44f0428dbde654c4dd1fb29c863b
[3/9] dt-bindings: mfd: add binding for qnap,ts433-mcu devices
      commit: 8877bcff3e3b4f08a1fc0232dbfdaeda085cfdf3
[4/9] mfd: add base driver for qnap-mcu devices
      commit: 944ca826f69e4723853b3876875b03aeafe67b60
[5/9] leds: add driver for LEDs from qnap-mcu devices
      commit: fe6a21ee38f12e3e5f9adbd2f9a840be105b943f
[6/9] Input: add driver for the input part of qnap-mcu devices
      commit: 4b27e0da257371d3d141fae38fdbdc3c3a67bce6
[7/9] hwmon: add driver for the hwmon parts of qnap-mcu devices
      commit: 41755872a8a8ab8d1644459d9634c53b743fe2be
[8/9] arm64: dts: rockchip: hook up the MCU on the QNAP TS433
      (no commit info)
[9/9] arm64: dts: rockchip: set hdd led labels on qnap-ts433
      (no commit info)

--
Lee Jones [李琼斯]

Re: [PATCH v9 0/9] Drivers to support the MCU on QNAP NAS devices

[Lee Jones]

On Thu, 12 Dec 2024, Lee Jones wrote:

> On Thu, 07 Nov 2024 12:47:03 +0100, Heiko Stuebner wrote:
> > This implements a set of drivers for the MCU used on QNAP NAS devices.
> > 
> > Of course no documentation for the serial protocol is available, so
> > thankfully QNAP has a tool on their rescue-inird to talk to the MCU and
> > I found interceptty [0] to listen to what goes over the serial connection.
> > 
> > In general it looks like there are two different generations in general,
> > an "EC" device and now this "MCU" - referenced in the strings of the
> > userspace handlers for those devices.
> > 
> > [...]
> 
> Applied, thanks!
> 
> [1/9] HID: hid-sensor-hub: don't use stale platform-data on remove
>       commit: e079a120f31e3f9c00180aa13c1df18cc138f7fe
> [2/9] mfd: core: make platform_data pointer const in struct mfd_cell
>       commit: 8f4009ad901c44f0428dbde654c4dd1fb29c863b
> [3/9] dt-bindings: mfd: add binding for qnap,ts433-mcu devices
>       commit: 8877bcff3e3b4f08a1fc0232dbfdaeda085cfdf3
> [4/9] mfd: add base driver for qnap-mcu devices
>       commit: 944ca826f69e4723853b3876875b03aeafe67b60
> [5/9] leds: add driver for LEDs from qnap-mcu devices
>       commit: fe6a21ee38f12e3e5f9adbd2f9a840be105b943f
> [6/9] Input: add driver for the input part of qnap-mcu devicesj
>       commit: 4b27e0da257371d3d141fae38fdbdc3c3a67bce6
> [7/9] hwmon: add driver for the hwmon parts of qnap-mcu devices
>       commit: 41755872a8a8ab8d1644459d9634c53b743fe2be

> [8/9] arm64: dts: rockchip: hook up the MCU on the QNAP TS433
>       (no commit info)
> [9/9] arm64: dts: rockchip: set hdd led labels on qnap-ts433
>       (no commit info)

As you can see, I have _not_ applied these two DTS patches.

-- 
Lee Jones [李琼斯]

Re: [PATCH v9 0/9] Drivers to support the MCU on QNAP NAS devices

[Lee Jones]

On Thu, 12 Dec 2024, Lee Jones wrote:

> On Thu, 07 Nov 2024 12:47:03 +0100, Heiko Stuebner wrote:
> > This implements a set of drivers for the MCU used on QNAP NAS devices.
> > 
> > Of course no documentation for the serial protocol is available, so
> > thankfully QNAP has a tool on their rescue-inird to talk to the MCU and
> > I found interceptty [0] to listen to what goes over the serial connection.
> > 
> > In general it looks like there are two different generations in general,
> > an "EC" device and now this "MCU" - referenced in the strings of the
> > userspace handlers for those devices.
> > 
> > [...]
> 
> Applied, thanks!
> 
> [1/9] HID: hid-sensor-hub: don't use stale platform-data on remove
>       commit: e079a120f31e3f9c00180aa13c1df18cc138f7fe
> [2/9] mfd: core: make platform_data pointer const in struct mfd_cell
>       commit: 8f4009ad901c44f0428dbde654c4dd1fb29c863b
> [3/9] dt-bindings: mfd: add binding for qnap,ts433-mcu devices
>       commit: 8877bcff3e3b4f08a1fc0232dbfdaeda085cfdf3
> [4/9] mfd: add base driver for qnap-mcu devices
>       commit: 944ca826f69e4723853b3876875b03aeafe67b60
> [5/9] leds: add driver for LEDs from qnap-mcu devices
>       commit: fe6a21ee38f12e3e5f9adbd2f9a840be105b943f
> [6/9] Input: add driver for the input part of qnap-mcu devices
>       commit: 4b27e0da257371d3d141fae38fdbdc3c3a67bce6
> [7/9] hwmon: add driver for the hwmon parts of qnap-mcu devices
>       commit: 41755872a8a8ab8d1644459d9634c53b743fe2be

Once build testing is complete, I'll send out a PR.

Note to self: ib-mfd-hid-hwmon-input-leds-6.14

-- 
Lee Jones [李琼斯]

[GIT PULL] Immutable branch between MFD, HID, HWMON, Input and LEDs due for the v6.14 merge window

[Lee Jones]

Enjoy!

The following changes since commit 40384c840ea1944d7c5a392e8975ed088ecf0b37:

  Linux 6.13-rc1 (2024-12-01 14:28:56 -0800)

are available in the Git repository at:

  ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git tags/ib-mfd-hid-hwmon-input-leds-v6.14

for you to fetch changes up to 9855caf5d4eb1d8b8bba60be256186ea8e0f907c:

  hwmon: add driver for the hwmon parts of qnap-mcu devices (2024-12-17 13:14:48 +0000)

----------------------------------------------------------------
Immutable branch between MFD, HID, HWMON, Input and LEDs due for the v6.14 merge window

----------------------------------------------------------------
Heiko Stuebner (7):
      HID: hid-sensor-hub: don't use stale platform-data on remove
      mfd: core: Make platform_data pointer const in struct mfd_cell
      dt-bindings: mfd: Add binding for qnap,ts433-mcu devices
      mfd: Add base driver for qnap-mcu devices
      leds: Add driver for LEDs from qnap-mcu devices
      Input: add driver for the input part of qnap-mcu devices
      hwmon: add driver for the hwmon parts of qnap-mcu devices

 .../devicetree/bindings/mfd/qnap,ts433-mcu.yaml    |  42 +++
 Documentation/hwmon/index.rst                      |   1 +
 Documentation/hwmon/qnap-mcu-hwmon.rst             |  27 ++
 MAINTAINERS                                        |   9 +
 drivers/hid/hid-sensor-hub.c                       |  21 +-
 drivers/hwmon/Kconfig                              |  12 +
 drivers/hwmon/Makefile                             |   1 +
 drivers/hwmon/qnap-mcu-hwmon.c                     | 364 +++++++++++++++++++++
 drivers/input/misc/Kconfig                         |  12 +
 drivers/input/misc/Makefile                        |   1 +
 drivers/input/misc/qnap-mcu-input.c                | 153 +++++++++
 drivers/leds/Kconfig                               |  11 +
 drivers/leds/Makefile                              |   1 +
 drivers/leds/leds-qnap-mcu.c                       | 227 +++++++++++++
 drivers/mfd/Kconfig                                |  13 +
 drivers/mfd/Makefile                               |   2 +
 drivers/mfd/qnap-mcu.c                             | 338 +++++++++++++++++++
 include/linux/mfd/core.h                           |   2 +-
 include/linux/mfd/qnap-mcu.h                       |  26 ++
 19 files changed, 1255 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/qnap,ts433-mcu.yaml
 create mode 100644 Documentation/hwmon/qnap-mcu-hwmon.rst
 create mode 100644 drivers/hwmon/qnap-mcu-hwmon.c
 create mode 100644 drivers/input/misc/qnap-mcu-input.c
 create mode 100644 drivers/leds/leds-qnap-mcu.c
 create mode 100644 drivers/mfd/qnap-mcu.c
 create mode 100644 include/linux/mfd/qnap-mcu.h

-- 
Lee Jones [李琼斯]

Re: (subset) [PATCH v9 0/9] Drivers to support the MCU on QNAP NAS devices

[Heiko Stuebner]

On Thu, 07 Nov 2024 12:47:03 +0100, Heiko Stuebner wrote:
> This implements a set of drivers for the MCU used on QNAP NAS devices.
> 
> Of course no documentation for the serial protocol is available, so
> thankfully QNAP has a tool on their rescue-inird to talk to the MCU and
> I found interceptty [0] to listen to what goes over the serial connection.
> 
> In general it looks like there are two different generations in general,
> an "EC" device and now this "MCU" - referenced in the strings of the
> userspace handlers for those devices.
> 
> [...]

Applied, thanks!

[8/9] arm64: dts: rockchip: hook up the MCU on the QNAP TS433
      commit: 989e3dd871349e76919911ac628e97ff6cb1ad51
[9/9] arm64: dts: rockchip: set hdd led labels on qnap-ts433
      commit: 47f34eab58294d7bb2583f9519b4022e74d56437

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>

Re: [PATCH v9 0/9] Drivers to support the MCU on QNAP NAS devices

[Heiko Stübner]

Am Donnerstag, 12. Dezember 2024, 18:19:54 CET schrieb Lee Jones:
> On Thu, 12 Dec 2024, Lee Jones wrote:
> 
> > On Thu, 07 Nov 2024 12:47:03 +0100, Heiko Stuebner wrote:
> > > This implements a set of drivers for the MCU used on QNAP NAS devices.
> > > 
> > > Of course no documentation for the serial protocol is available, so
> > > thankfully QNAP has a tool on their rescue-inird to talk to the MCU and
> > > I found interceptty [0] to listen to what goes over the serial connection.
> > > 
> > > In general it looks like there are two different generations in general,
> > > an "EC" device and now this "MCU" - referenced in the strings of the
> > > userspace handlers for those devices.
> > > 
> > > [...]
> > 
> > Applied, thanks!
> > 
> > [1/9] HID: hid-sensor-hub: don't use stale platform-data on remove
> >       commit: e079a120f31e3f9c00180aa13c1df18cc138f7fe
> > [2/9] mfd: core: make platform_data pointer const in struct mfd_cell
> >       commit: 8f4009ad901c44f0428dbde654c4dd1fb29c863b
> > [3/9] dt-bindings: mfd: add binding for qnap,ts433-mcu devices
> >       commit: 8877bcff3e3b4f08a1fc0232dbfdaeda085cfdf3
> > [4/9] mfd: add base driver for qnap-mcu devices
> >       commit: 944ca826f69e4723853b3876875b03aeafe67b60
> > [5/9] leds: add driver for LEDs from qnap-mcu devices
> >       commit: fe6a21ee38f12e3e5f9adbd2f9a840be105b943f
> > [6/9] Input: add driver for the input part of qnap-mcu devices
> >       commit: 4b27e0da257371d3d141fae38fdbdc3c3a67bce6
> > [7/9] hwmon: add driver for the hwmon parts of qnap-mcu devices
> >       commit: 41755872a8a8ab8d1644459d9634c53b743fe2be
> 
> Once build testing is complete, I'll send out a PR.

thanks a lot for picking up the driver patches.

Heiko

Re: [PATCH v9 4/9] mfd: add base driver for qnap-mcu devices

[Kees Bakker]

Op 07-11-2024 om 12:47 schreef Heiko Stuebner:
> These microcontroller units are used in network-attached-storage devices
> made by QNAP and provide additional functionality to the system.
>
> This adds the base driver that implements the serial protocol via
> serdev and additionally hooks into the poweroff handlers to turn
> off the parts of the system not supplied by the general PMIC.
>
> Turning off (at least the TSx33 devices using Rockchip SoCs) consists of
> two separate actions. Turning off the MCU alone does not turn off the main
> SoC and turning off only the SoC/PMIC does not turn off the hard-drives.
> Also if the MCU is not turned off, the system also won't start again until
> it is unplugged from power.
>
> So on shutdown the MCU needs to be turned off separately before the
> main PMIC.
>
> The protocol spoken by the MCU is sadly not documented, but was
> obtained by listening to the chatter on the serial port, as thankfully
> the "hal_app" program from QNAPs firmware allows triggering all/most
> MCU actions from the command line.
>
> The implementation of how to talk to the serial device got some
> inspiration from the rave-sp servdev driver.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>   MAINTAINERS                  |   6 +
>   drivers/mfd/Kconfig          |  13 ++
>   drivers/mfd/Makefile         |   2 +
>   drivers/mfd/qnap-mcu.c       | 338 +++++++++++++++++++++++++++++++++++
>   include/linux/mfd/qnap-mcu.h |  26 +++
>   5 files changed, 385 insertions(+)
>   create mode 100644 drivers/mfd/qnap-mcu.c
>   create mode 100644 include/linux/mfd/qnap-mcu.h
>
> [...]
> diff --git a/drivers/mfd/qnap-mcu.c b/drivers/mfd/qnap-mcu.c
> new file mode 100644
> index 000000000000..4be39d8b2905
> --- /dev/null
> +++ b/drivers/mfd/qnap-mcu.c
> [...]
> +int qnap_mcu_exec(struct qnap_mcu *mcu,
> +		  const u8 *cmd_data, size_t cmd_data_size,
> +		  u8 *reply_data, size_t reply_data_size)
> +{
> +	unsigned char rx[QNAP_MCU_RX_BUFFER_SIZE];
> +	size_t length = reply_data_size + QNAP_MCU_CHECKSUM_SIZE;
> +	struct qnap_mcu_reply *reply = &mcu->reply;
> +	int ret = 0;
> +
> +	if (length > sizeof(rx)) {
> +		dev_err(&mcu->serdev->dev, "expected data too big for receive buffer");
> +		return -EINVAL;
> +	}
> +
> +	mutex_lock(&mcu->bus_lock);
> +
> +	reply->data = rx,
> +	reply->length = length,
> +	reply->received = 0,
> +	reinit_completion(&reply->done);
> +
> +	qnap_mcu_write(mcu, cmd_data, cmd_data_size);
> +
> +	serdev_device_wait_until_sent(mcu->serdev, msecs_to_jiffies(QNAP_MCU_TIMEOUT_MS));
> +
> +	if (!wait_for_completion_timeout(&reply->done, msecs_to_jiffies(QNAP_MCU_TIMEOUT_MS))) {
> +		dev_err(&mcu->serdev->dev, "Command timeout\n");
> +		ret = -ETIMEDOUT;
> +	} else {
> +		u8 crc = qnap_mcu_csum(rx, reply_data_size);
Here `rx` is still not initialized.
> +
> +		if (crc != rx[reply_data_size]) {
> +			dev_err(&mcu->serdev->dev,
> +				"Invalid Checksum received\n");
> +			ret = -EIO;
> +		} else {
> +			memcpy(reply_data, rx, reply_data_size);
> +		}
> +	}
> +
> +	mutex_unlock(&mcu->bus_lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(qnap_mcu_exec);
>

Re: [PATCH v9 4/9] mfd: add base driver for qnap-mcu devices

[Heiko Stübner]

Hi Kees,

Am Donnerstag, 19. Dezember 2024, 20:18:38 CET schrieb Kees Bakker:
> Op 07-11-2024 om 12:47 schreef Heiko Stuebner:
> > These microcontroller units are used in network-attached-storage devices
> > made by QNAP and provide additional functionality to the system.
> >
> > This adds the base driver that implements the serial protocol via
> > serdev and additionally hooks into the poweroff handlers to turn
> > off the parts of the system not supplied by the general PMIC.
> >
> > Turning off (at least the TSx33 devices using Rockchip SoCs) consists of
> > two separate actions. Turning off the MCU alone does not turn off the main
> > SoC and turning off only the SoC/PMIC does not turn off the hard-drives.
> > Also if the MCU is not turned off, the system also won't start again until
> > it is unplugged from power.
> >
> > So on shutdown the MCU needs to be turned off separately before the
> > main PMIC.
> >
> > The protocol spoken by the MCU is sadly not documented, but was
> > obtained by listening to the chatter on the serial port, as thankfully
> > the "hal_app" program from QNAPs firmware allows triggering all/most
> > MCU actions from the command line.
> >
> > The implementation of how to talk to the serial device got some
> > inspiration from the rave-sp servdev driver.
> >
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> >   MAINTAINERS                  |   6 +
> >   drivers/mfd/Kconfig          |  13 ++
> >   drivers/mfd/Makefile         |   2 +
> >   drivers/mfd/qnap-mcu.c       | 338 +++++++++++++++++++++++++++++++++++
> >   include/linux/mfd/qnap-mcu.h |  26 +++
> >   5 files changed, 385 insertions(+)
> >   create mode 100644 drivers/mfd/qnap-mcu.c
> >   create mode 100644 include/linux/mfd/qnap-mcu.h
> >
> > [...]
> > diff --git a/drivers/mfd/qnap-mcu.c b/drivers/mfd/qnap-mcu.c
> > new file mode 100644
> > index 000000000000..4be39d8b2905
> > --- /dev/null
> > +++ b/drivers/mfd/qnap-mcu.c
> > [...]
> > +int qnap_mcu_exec(struct qnap_mcu *mcu,
> > +		  const u8 *cmd_data, size_t cmd_data_size,
> > +		  u8 *reply_data, size_t reply_data_size)
> > +{
> > +	unsigned char rx[QNAP_MCU_RX_BUFFER_SIZE];
> > +	size_t length = reply_data_size + QNAP_MCU_CHECKSUM_SIZE;
> > +	struct qnap_mcu_reply *reply = &mcu->reply;
> > +	int ret = 0;
> > +
> > +	if (length > sizeof(rx)) {
> > +		dev_err(&mcu->serdev->dev, "expected data too big for receive buffer");
> > +		return -EINVAL;
> > +	}
> > +
> > +	mutex_lock(&mcu->bus_lock);
> > +
> > +	reply->data = rx,
> > +	reply->length = length,
> > +	reply->received = 0,
> > +	reinit_completion(&reply->done);
> > +
> > +	qnap_mcu_write(mcu, cmd_data, cmd_data_size);
> > +
> > +	serdev_device_wait_until_sent(mcu->serdev, msecs_to_jiffies(QNAP_MCU_TIMEOUT_MS));
> > +
> > +	if (!wait_for_completion_timeout(&reply->done, msecs_to_jiffies(QNAP_MCU_TIMEOUT_MS))) {
> > +		dev_err(&mcu->serdev->dev, "Command timeout\n");
> > +		ret = -ETIMEDOUT;
> > +	} else {
> > +		u8 crc = qnap_mcu_csum(rx, reply_data_size);
> Here `rx` is still not initialized.

The MCU works in a way that a sent command always causes "reply_data_size"
bytes to be returned.

So for each qnap_mcu_write() above we know that this amount of bytes has
been returned (and thus written into rx) if the completion above finishes
sucessfully.

"rx" is assigned to reply->data above (same as the expected received size).
When characters are received on the serial line, this will trigger
qnap_mcu_receive_buf() from the serdev and thus fill those elements in rx.

So if we land at the qnap_mcu_csum() call, we do have received the expected
amount of bytes from the serdev and thus rx is filled accordingly.

If we don't receive the needed amount of bytes, we end up in the timeout
above that.

What did I miss?


Heiko

> > +
> > +		if (crc != rx[reply_data_size]) {
> > +			dev_err(&mcu->serdev->dev,
> > +				"Invalid Checksum received\n");
> > +			ret = -EIO;
> > +		} else {
> > +			memcpy(reply_data, rx, reply_data_size);
> > +		}
> > +	}
> > +
> > +	mutex_unlock(&mcu->bus_lock);
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(qnap_mcu_exec);
> >
> 

Re: [PATCH v9 4/9] mfd: add base driver for qnap-mcu devices

[Kees Bakker]

Op 19-12-2024 om 20:43 schreef Heiko Stübner:
> Hi Kees,
>
> Am Donnerstag, 19. Dezember 2024, 20:18:38 CET schrieb Kees Bakker:
>> Op 07-11-2024 om 12:47 schreef Heiko Stuebner:
>>> These microcontroller units are used in network-attached-storage devices
>>> made by QNAP and provide additional functionality to the system.
>>>
>>> This adds the base driver that implements the serial protocol via
>>> serdev and additionally hooks into the poweroff handlers to turn
>>> off the parts of the system not supplied by the general PMIC.
>>>
>>> Turning off (at least the TSx33 devices using Rockchip SoCs) consists of
>>> two separate actions. Turning off the MCU alone does not turn off the main
>>> SoC and turning off only the SoC/PMIC does not turn off the hard-drives.
>>> Also if the MCU is not turned off, the system also won't start again until
>>> it is unplugged from power.
>>>
>>> So on shutdown the MCU needs to be turned off separately before the
>>> main PMIC.
>>>
>>> The protocol spoken by the MCU is sadly not documented, but was
>>> obtained by listening to the chatter on the serial port, as thankfully
>>> the "hal_app" program from QNAPs firmware allows triggering all/most
>>> MCU actions from the command line.
>>>
>>> The implementation of how to talk to the serial device got some
>>> inspiration from the rave-sp servdev driver.
>>>
>>> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>>> ---
>>>    MAINTAINERS                  |   6 +
>>>    drivers/mfd/Kconfig          |  13 ++
>>>    drivers/mfd/Makefile         |   2 +
>>>    drivers/mfd/qnap-mcu.c       | 338 +++++++++++++++++++++++++++++++++++
>>>    include/linux/mfd/qnap-mcu.h |  26 +++
>>>    5 files changed, 385 insertions(+)
>>>    create mode 100644 drivers/mfd/qnap-mcu.c
>>>    create mode 100644 include/linux/mfd/qnap-mcu.h
>>>
>>> [...]
>>> diff --git a/drivers/mfd/qnap-mcu.c b/drivers/mfd/qnap-mcu.c
>>> new file mode 100644
>>> index 000000000000..4be39d8b2905
>>> --- /dev/null
>>> +++ b/drivers/mfd/qnap-mcu.c
>>> [...]
>>> +int qnap_mcu_exec(struct qnap_mcu *mcu,
>>> +		  const u8 *cmd_data, size_t cmd_data_size,
>>> +		  u8 *reply_data, size_t reply_data_size)
>>> +{
>>> +	unsigned char rx[QNAP_MCU_RX_BUFFER_SIZE];
>>> +	size_t length = reply_data_size + QNAP_MCU_CHECKSUM_SIZE;
>>> +	struct qnap_mcu_reply *reply = &mcu->reply;
>>> +	int ret = 0;
>>> +
>>> +	if (length > sizeof(rx)) {
>>> +		dev_err(&mcu->serdev->dev, "expected data too big for receive buffer");
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	mutex_lock(&mcu->bus_lock);
>>> +
>>> +	reply->data = rx,
>>> +	reply->length = length,
>>> +	reply->received = 0,
>>> +	reinit_completion(&reply->done);
>>> +
>>> +	qnap_mcu_write(mcu, cmd_data, cmd_data_size);
>>> +
>>> +	serdev_device_wait_until_sent(mcu->serdev, msecs_to_jiffies(QNAP_MCU_TIMEOUT_MS));
>>> +
>>> +	if (!wait_for_completion_timeout(&reply->done, msecs_to_jiffies(QNAP_MCU_TIMEOUT_MS))) {
>>> +		dev_err(&mcu->serdev->dev, "Command timeout\n");
>>> +		ret = -ETIMEDOUT;
>>> +	} else {
>>> +		u8 crc = qnap_mcu_csum(rx, reply_data_size);
>> Here `rx` is still not initialized.
> The MCU works in a way that a sent command always causes "reply_data_size"
> bytes to be returned.
>
> So for each qnap_mcu_write() above we know that this amount of bytes has
> been returned (and thus written into rx) if the completion above finishes
> sucessfully.
>
> "rx" is assigned to reply->data above (same as the expected received size).
> When characters are received on the serial line, this will trigger
> qnap_mcu_receive_buf() from the serdev and thus fill those elements in rx.
>
> So if we land at the qnap_mcu_csum() call, we do have received the expected
> amount of bytes from the serdev and thus rx is filled accordingly.
>
> If we don't receive the needed amount of bytes, we end up in the timeout
> above that.
>
> What did I miss?
Sorry, my fault. I missed the essential part of the external event (external
for this function that is).
Thanks for explaining.
>
>
> Heiko
>
>>> +
>>> +		if (crc != rx[reply_data_size]) {
>>> +			dev_err(&mcu->serdev->dev,
>>> +				"Invalid Checksum received\n");
>>> +			ret = -EIO;
>>> +		} else {
>>> +			memcpy(reply_data, rx, reply_data_size);
>>> +		}
>>> +	}
>>> +
>>> +	mutex_unlock(&mcu->bus_lock);
>>> +	return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(qnap_mcu_exec);
>>>
>
>
>

Re: [PATCH v9 4/9] mfd: add base driver for qnap-mcu devices

[Heiko Stübner]

Am Donnerstag, 19. Dezember 2024, 20:51:41 CET schrieb Kees Bakker:
> Op 19-12-2024 om 20:43 schreef Heiko Stübner:
> > Hi Kees,
> >
> > Am Donnerstag, 19. Dezember 2024, 20:18:38 CET schrieb Kees Bakker:
> >> Op 07-11-2024 om 12:47 schreef Heiko Stuebner:
> >>> These microcontroller units are used in network-attached-storage devices
> >>> made by QNAP and provide additional functionality to the system.
> >>>
> >>> This adds the base driver that implements the serial protocol via
> >>> serdev and additionally hooks into the poweroff handlers to turn
> >>> off the parts of the system not supplied by the general PMIC.
> >>>
> >>> Turning off (at least the TSx33 devices using Rockchip SoCs) consists of
> >>> two separate actions. Turning off the MCU alone does not turn off the main
> >>> SoC and turning off only the SoC/PMIC does not turn off the hard-drives.
> >>> Also if the MCU is not turned off, the system also won't start again until
> >>> it is unplugged from power.
> >>>
> >>> So on shutdown the MCU needs to be turned off separately before the
> >>> main PMIC.
> >>>
> >>> The protocol spoken by the MCU is sadly not documented, but was
> >>> obtained by listening to the chatter on the serial port, as thankfully
> >>> the "hal_app" program from QNAPs firmware allows triggering all/most
> >>> MCU actions from the command line.
> >>>
> >>> The implementation of how to talk to the serial device got some
> >>> inspiration from the rave-sp servdev driver.
> >>>
> >>> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> >>> ---
> >>>    MAINTAINERS                  |   6 +
> >>>    drivers/mfd/Kconfig          |  13 ++
> >>>    drivers/mfd/Makefile         |   2 +
> >>>    drivers/mfd/qnap-mcu.c       | 338 +++++++++++++++++++++++++++++++++++
> >>>    include/linux/mfd/qnap-mcu.h |  26 +++
> >>>    5 files changed, 385 insertions(+)
> >>>    create mode 100644 drivers/mfd/qnap-mcu.c
> >>>    create mode 100644 include/linux/mfd/qnap-mcu.h
> >>>
> >>> [...]
> >>> diff --git a/drivers/mfd/qnap-mcu.c b/drivers/mfd/qnap-mcu.c
> >>> new file mode 100644
> >>> index 000000000000..4be39d8b2905
> >>> --- /dev/null
> >>> +++ b/drivers/mfd/qnap-mcu.c
> >>> [...]
> >>> +int qnap_mcu_exec(struct qnap_mcu *mcu,
> >>> +		  const u8 *cmd_data, size_t cmd_data_size,
> >>> +		  u8 *reply_data, size_t reply_data_size)
> >>> +{
> >>> +	unsigned char rx[QNAP_MCU_RX_BUFFER_SIZE];
> >>> +	size_t length = reply_data_size + QNAP_MCU_CHECKSUM_SIZE;
> >>> +	struct qnap_mcu_reply *reply = &mcu->reply;
> >>> +	int ret = 0;
> >>> +
> >>> +	if (length > sizeof(rx)) {
> >>> +		dev_err(&mcu->serdev->dev, "expected data too big for receive buffer");
> >>> +		return -EINVAL;
> >>> +	}
> >>> +
> >>> +	mutex_lock(&mcu->bus_lock);
> >>> +
> >>> +	reply->data = rx,
> >>> +	reply->length = length,
> >>> +	reply->received = 0,
> >>> +	reinit_completion(&reply->done);
> >>> +
> >>> +	qnap_mcu_write(mcu, cmd_data, cmd_data_size);
> >>> +
> >>> +	serdev_device_wait_until_sent(mcu->serdev, msecs_to_jiffies(QNAP_MCU_TIMEOUT_MS));
> >>> +
> >>> +	if (!wait_for_completion_timeout(&reply->done, msecs_to_jiffies(QNAP_MCU_TIMEOUT_MS))) {
> >>> +		dev_err(&mcu->serdev->dev, "Command timeout\n");
> >>> +		ret = -ETIMEDOUT;
> >>> +	} else {
> >>> +		u8 crc = qnap_mcu_csum(rx, reply_data_size);
> >> Here `rx` is still not initialized.
> > The MCU works in a way that a sent command always causes "reply_data_size"
> > bytes to be returned.
> >
> > So for each qnap_mcu_write() above we know that this amount of bytes has
> > been returned (and thus written into rx) if the completion above finishes
> > sucessfully.
> >
> > "rx" is assigned to reply->data above (same as the expected received size).
> > When characters are received on the serial line, this will trigger
> > qnap_mcu_receive_buf() from the serdev and thus fill those elements in rx.
> >
> > So if we land at the qnap_mcu_csum() call, we do have received the expected
> > amount of bytes from the serdev and thus rx is filled accordingly.
> >
> > If we don't receive the needed amount of bytes, we end up in the timeout
> > above that.
> >
> > What did I miss?
> Sorry, my fault. I missed the essential part of the external event (external
> for this function that is).
> Thanks for explaining.

no worries :-) .

The more eyes, the better.

Heiko