* [PATCH v4 1/7] dt-bindings: mfd: add binding for qnap,ts433-mcu devices
2024-08-10 18:47 [PATCH v4 0/7] Drivers to support the MCU on QNAP NAS devices Heiko Stuebner
@ 2024-08-10 18:47 ` Heiko Stuebner
2024-08-12 16:11 ` Conor Dooley
2024-08-10 18:47 ` [PATCH v4 2/7] mfd: add base driver for qnap-mcu devices Heiko Stuebner
` (5 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Heiko Stuebner @ 2024-08-10 18:47 UTC (permalink / raw)
To: lee, jdelvare, linux, dmitry.torokhov, pavel
Cc: robh, krzk+dt, conor+dt, heiko, ukleinek, devicetree,
linux-kernel, linux-arm-kernel, linux-rockchip, linux-hwmon,
linux-input, linux-leds
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.
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 0000000000000..877078ac172f1
--- /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.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4 1/7] dt-bindings: mfd: add binding for qnap,ts433-mcu devices
2024-08-10 18:47 ` [PATCH v4 1/7] dt-bindings: mfd: add binding for qnap,ts433-mcu devices Heiko Stuebner
@ 2024-08-12 16:11 ` Conor Dooley
0 siblings, 0 replies; 14+ messages in thread
From: Conor Dooley @ 2024-08-12 16:11 UTC (permalink / raw)
To: Heiko Stuebner
Cc: lee, jdelvare, linux, dmitry.torokhov, pavel, robh, krzk+dt,
conor+dt, ukleinek, devicetree, linux-kernel, linux-arm-kernel,
linux-rockchip, linux-hwmon, linux-input, linux-leds
[-- Attachment #1: Type: text/plain, Size: 776 bytes --]
On Sat, Aug 10, 2024 at 08:47:37PM +0200, Heiko Stuebner wrote:
> 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.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 2/7] mfd: add base driver for qnap-mcu devices
2024-08-10 18:47 [PATCH v4 0/7] Drivers to support the MCU on QNAP NAS devices Heiko Stuebner
2024-08-10 18:47 ` [PATCH v4 1/7] dt-bindings: mfd: add binding for qnap,ts433-mcu devices Heiko Stuebner
@ 2024-08-10 18:47 ` Heiko Stuebner
2024-08-16 17:13 ` Lee Jones
2024-08-10 18:47 ` [PATCH v4 3/7] leds: add driver for LEDs from " Heiko Stuebner
` (4 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Heiko Stuebner @ 2024-08-10 18:47 UTC (permalink / raw)
To: lee, jdelvare, linux, dmitry.torokhov, pavel
Cc: robh, krzk+dt, conor+dt, heiko, ukleinek, devicetree,
linux-kernel, linux-arm-kernel, linux-rockchip, linux-hwmon,
linux-input, linux-leds
These MCUs 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) is twofold.
Turning off the MCU does not turn off the SoC and turning off the SoC
does not turn off the hard-drives. And 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 before the general 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 mfd.
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
MAINTAINERS | 6 +
drivers/mfd/Kconfig | 10 +
drivers/mfd/Makefile | 2 +
drivers/mfd/qnap-mcu.c | 358 +++++++++++++++++++++++++++++++++++
include/linux/mfd/qnap-mcu.h | 28 +++
5 files changed, 404 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 8766f3e5e87e0..f9f605a3c12a4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18652,6 +18652,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 bc8be2e593b6b..ca7289e906a7b 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2362,6 +2362,16 @@ 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 MCU core driver"
+ depends on SERIAL_DEV_BUS
+ help
+ Select this to get support for the QNAP MCU device found in
+ several devices of QNAP network attached storages.
+
+ It 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 02b651cd75352..fc8b825725ff2 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -286,5 +286,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 0000000000000..be73bd88d7608
--- /dev/null
+++ b/drivers/mfd/qnap-mcu.c
@@ -0,0 +1,358 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/*
+ * MFD core driver for the MCU in Qnap NAS devices that is connected
+ * via a dedicated UART port
+ *
+ * Copyright (C) 2024 Heiko Stuebner <heiko@sntech.de>
+ */
+
+#include <linux/export.h>
+#include <linux/slab.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>
+
+/* 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)
+
+/**
+ * struct qnap_mcu_reply - Reply to a command
+ *
+ * @data: Buffer to store reply payload in
+ * @length: Expected reply length, including the checksum
+ * @received: So far received number of bytes
+ * @done: Reply received completely
+ */
+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_lock: Lock protecting @reply
+ * @reply: Pointer to memory to store reply payload
+ * @variant: Device variant specific information
+ * @version: MCU firmware version
+ */
+struct qnap_mcu {
+ struct serdev_device *serdev;
+ /* Serialize access to the device */
+ struct mutex bus_lock;
+ /* Protect access to the reply pointer */
+ struct mutex reply_lock;
+ struct qnap_mcu_reply *reply;
+ const struct qnap_mcu_variant *variant;
+ u8 version[4];
+};
+
+/*
+ * 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 *sp, const u8 *data, u8 data_size)
+{
+ unsigned char tx[QNAP_MCU_TX_BUFFER_SIZE];
+ size_t length = data_size + QNAP_MCU_CHECKSUM_SIZE;
+
+ if (WARN_ON(length > sizeof(tx)))
+ return -ENOMEM;
+
+ memcpy(tx, data, data_size);
+ tx[data_size] = qnap_mcu_csum(data, data_size);
+
+ print_hex_dump_debug("qnap-mcu tx: ", DUMP_PREFIX_NONE,
+ 16, 1, tx, length, false);
+
+ return serdev_device_write(sp->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;
+
+ mutex_lock(&mcu->reply_lock);
+ if (!reply) {
+ dev_warn(dev, "received %zu bytes, we were not waiting for\n",
+ size);
+ mutex_unlock(&mcu->reply_lock);
+ return size;
+ }
+
+ while (src < end) {
+ reply->data[reply->received] = *src++;
+ reply->received++;
+
+ if (reply->received == reply->length) {
+ complete(&reply->done);
+ mutex_unlock(&mcu->reply_lock);
+
+ /*
+ * 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.
+ */
+ mutex_unlock(&mcu->reply_lock);
+ 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 = {
+ .data = rx,
+ .length = length,
+ .received = 0,
+ .done = COMPLETION_INITIALIZER_ONSTACK(reply.done),
+ };
+ int ret;
+
+ if (WARN_ON(length > sizeof(rx)))
+ return -ENOMEM;
+
+ mutex_lock(&mcu->bus_lock);
+
+ mutex_lock(&mcu->reply_lock);
+ mcu->reply = &reply;
+ mutex_unlock(&mcu->reply_lock);
+
+ qnap_mcu_write(mcu, cmd_data, cmd_data_size);
+
+ if (!wait_for_completion_timeout(&reply.done,
+ msecs_to_jiffies(500))) {
+ dev_err(&mcu->serdev->dev, "Command timeout\n");
+ ret = -ETIMEDOUT;
+ } else {
+ u8 crc = qnap_mcu_csum(rx, reply_data_size);
+
+ print_hex_dump_debug("qnap-mcu rx: ", DUMP_PREFIX_NONE,
+ 16, 1, rx, length, false);
+
+ if (crc != rx[reply_data_size]) {
+ dev_err(&mcu->serdev->dev,
+ "Checksum 0x%02x wrong for data\n", crc);
+ ret = -EIO;
+ } else {
+ memcpy(reply_data, rx, reply_data_size);
+ ret = 0;
+ }
+ }
+
+ mutex_lock(&mcu->reply_lock);
+ mcu->reply = NULL;
+ mutex_unlock(&mcu->reply_lock);
+
+ 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[2];
+ 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] != 0x40 || ack[1] != 0x30) {
+ dev_err(&mcu->serdev->dev, "Did not receive ack\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(qnap_mcu_exec_with_ack);
+
+const struct qnap_mcu_variant *qnap_mcu_get_variant_data(struct qnap_mcu *mcu)
+{
+ return mcu->variant;
+}
+EXPORT_SYMBOL_GPL(qnap_mcu_get_variant_data);
+
+static int qnap_mcu_get_version(struct qnap_mcu *mcu)
+{
+ u8 cmd[] = {
+ [0] = 0x25, /* % */
+ [1] = 0x56 /* V */
+ };
+ u8 rx[14];
+ int ret;
+
+ ret = qnap_mcu_exec(mcu, cmd, sizeof(cmd), rx, 6);
+ if (ret)
+ return ret;
+
+ memcpy(mcu->version, &rx[2], 4);
+
+ 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)
+{
+ struct qnap_mcu *mcu = data->cb_data;
+ int ret;
+ u8 cmd[] = {
+ [0] = 0x40, /* @ */
+ [1] = 0x43, /* C */
+ [2] = 0x30 /* 0 */
+ };
+
+ dev_dbg(&mcu->serdev->dev, "running MCU poweroff\n");
+ 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 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 const struct mfd_cell qnap_mcu_subdevs[] = {
+ { .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);
+ mutex_init(&mcu->reply_lock);
+
+ 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) {
+ dev_err(dev, "Failed to set parity\n");
+ return ret;
+ }
+
+ 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");
+
+ ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, qnap_mcu_subdevs,
+ ARRAY_SIZE(qnap_mcu_subdevs), NULL, 0, NULL);
+ if (ret)
+ return dev_err_probe(dev, ret, "adding qnap mfd devices failed\n");
+
+ return 0;
+}
+
+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_LICENSE("GPL");
+MODULE_AUTHOR("Heiko Stuebner <heiko@sntech.de>");
+MODULE_DESCRIPTION("QNAP MCU core driver");
diff --git a/include/linux/mfd/qnap-mcu.h b/include/linux/mfd/qnap-mcu.h
new file mode 100644
index 0000000000000..f954815d3025b
--- /dev/null
+++ b/include/linux/mfd/qnap-mcu.h
@@ -0,0 +1,28 @@
+/* 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);
+const struct qnap_mcu_variant *qnap_mcu_get_variant_data(struct qnap_mcu *mcu);
+
+#endif /* _LINUX_QNAP_MCU_H_ */
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4 2/7] mfd: add base driver for qnap-mcu devices
2024-08-10 18:47 ` [PATCH v4 2/7] mfd: add base driver for qnap-mcu devices Heiko Stuebner
@ 2024-08-16 17:13 ` Lee Jones
2024-08-18 21:15 ` Heiko Stübner
0 siblings, 1 reply; 14+ messages in thread
From: Lee Jones @ 2024-08-16 17:13 UTC (permalink / raw)
To: Heiko Stuebner
Cc: jdelvare, linux, dmitry.torokhov, pavel, robh, krzk+dt, conor+dt,
ukleinek, devicetree, linux-kernel, linux-arm-kernel,
linux-rockchip, linux-hwmon, linux-input, linux-leds
On Sat, 10 Aug 2024, Heiko Stuebner wrote:
> These MCUs 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) is twofold.
This sentence doesn't make sense.
"twofold" means "times (multiply by) two".
> Turning off the MCU does not turn off the SoC and turning off the SoC
> does not turn off the hard-drives. And 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 before the general 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 mfd.
"MFD" or better yet "driver".
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
> MAINTAINERS | 6 +
> drivers/mfd/Kconfig | 10 +
> drivers/mfd/Makefile | 2 +
> drivers/mfd/qnap-mcu.c | 358 +++++++++++++++++++++++++++++++++++
> include/linux/mfd/qnap-mcu.h | 28 +++
> 5 files changed, 404 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 8766f3e5e87e0..f9f605a3c12a4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -18652,6 +18652,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 bc8be2e593b6b..ca7289e906a7b 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2362,6 +2362,16 @@ 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 MCU core driver"
Now's the time to expand "MCU"
> + depends on SERIAL_DEV_BUS
You need to depend on the MFD Core too.
> + help
> + Select this to get support for the QNAP MCU device found in
> + several devices of QNAP network attached storages.
s/storages/storage devices/
> +
> + It 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 02b651cd75352..fc8b825725ff2 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -286,5 +286,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 0000000000000..be73bd88d7608
> --- /dev/null
> +++ b/drivers/mfd/qnap-mcu.c
> @@ -0,0 +1,358 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
Superfluous newline.
> +/*
> + * MFD core driver for the MCU in Qnap NAS devices that is connected
No such thing as an "MFD". Please describe the device.
Is it QNAP or Qnap? Please be consistent.
> + * via a dedicated UART port
> + *
> + * Copyright (C) 2024 Heiko Stuebner <heiko@sntech.de>
> + */
> +
> +#include <linux/export.h>
What is this used for?
> +#include <linux/slab.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>
Alphabetical.
> +/* 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)
> +
> +/**
> + * struct qnap_mcu_reply - Reply to a command
> + *
> + * @data: Buffer to store reply payload in
> + * @length: Expected reply length, including the checksum
> + * @received: So far received number of bytes
"Received number of Bytes, so far"
> + * @done: Reply received completely
"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_lock: Lock protecting @reply
> + * @reply: Pointer to memory to store reply payload
> + * @variant: Device variant specific information
> + * @version: MCU firmware version
> + */
> +struct qnap_mcu {
> + struct serdev_device *serdev;
> + /* Serialize access to the device */
Comments and K-doc is OOT.
> + struct mutex bus_lock;
> + /* Protect access to the reply pointer */
> + struct mutex reply_lock;
> + struct qnap_mcu_reply *reply;
> + const struct qnap_mcu_variant *variant;
> + u8 version[4];
Please define all magic numbers.
> +};
> +
> +/*
> + * 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 *sp, const u8 *data, u8 data_size)
> +{
> + unsigned char tx[QNAP_MCU_TX_BUFFER_SIZE];
> + size_t length = data_size + QNAP_MCU_CHECKSUM_SIZE;
> +
> + if (WARN_ON(length > sizeof(tx)))
Are you sure you want to warn like this here?
A dev_err() seems more appropriate.
> + return -ENOMEM;
Why does this condition signify OOM?
Maybe consider -EINVAL?
> + memcpy(tx, data, data_size);
> + tx[data_size] = qnap_mcu_csum(data, data_size);
> +
> + print_hex_dump_debug("qnap-mcu tx: ", DUMP_PREFIX_NONE,
> + 16, 1, tx, length, false);
You can remove this now development is complete.
> + return serdev_device_write(sp->serdev, tx, length, HZ);
> +}
> +
> +static size_t qnap_mcu_receive_buf(struct serdev_device *serdev,
> + const u8 *buf, size_t size)
Use up to 100-chars to prevent early line-wrap.
> +{
> + 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;
> +
> + mutex_lock(&mcu->reply_lock);
> + if (!reply) {
> + dev_warn(dev, "received %zu bytes, we were not waiting for\n",
> + size);
> + mutex_unlock(&mcu->reply_lock);
guard(mutex)?
> + return size;
> + }
> +
> + while (src < end) {
> + reply->data[reply->received] = *src++;
> + reply->received++;
> +
> + if (reply->received == reply->length) {
> + complete(&reply->done);
> + mutex_unlock(&mcu->reply_lock);
> +
> + /*
> + * 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.
> + */
> + mutex_unlock(&mcu->reply_lock);
> + 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 = {
> + .data = rx,
> + .length = length,
> + .received = 0,
> + .done = COMPLETION_INITIALIZER_ONSTACK(reply.done),
> + };
> + int ret;
> +
> + if (WARN_ON(length > sizeof(rx)))
> + return -ENOMEM;
> +
> + mutex_lock(&mcu->bus_lock);
> +
> + mutex_lock(&mcu->reply_lock);
> + mcu->reply = &reply;
> + mutex_unlock(&mcu->reply_lock);
> +
> + qnap_mcu_write(mcu, cmd_data, cmd_data_size);
> +
> + if (!wait_for_completion_timeout(&reply.done,
> + msecs_to_jiffies(500))) {
> + dev_err(&mcu->serdev->dev, "Command timeout\n");
> + ret = -ETIMEDOUT;
> + } else {
> + u8 crc = qnap_mcu_csum(rx, reply_data_size);
> +
> + print_hex_dump_debug("qnap-mcu rx: ", DUMP_PREFIX_NONE,
> + 16, 1, rx, length, false);
> +
> + if (crc != rx[reply_data_size]) {
> + dev_err(&mcu->serdev->dev,
> + "Checksum 0x%02x wrong for data\n", crc);
> + ret = -EIO;
> + } else {
> + memcpy(reply_data, rx, reply_data_size);
> + ret = 0;
> + }
> + }
> +
> + mutex_lock(&mcu->reply_lock);
> + mcu->reply = NULL;
> + mutex_unlock(&mcu->reply_lock);
> +
> + 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[2];
> + 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] != 0x40 || ack[1] != 0x30) {
Why not use the char variants?
> + dev_err(&mcu->serdev->dev, "Did not receive ack\n");
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(qnap_mcu_exec_with_ack);
> +
> +const struct qnap_mcu_variant *qnap_mcu_get_variant_data(struct qnap_mcu *mcu)
> +{
> + return mcu->variant;
> +}
> +EXPORT_SYMBOL_GPL(qnap_mcu_get_variant_data);
> +
> +static int qnap_mcu_get_version(struct qnap_mcu *mcu)
> +{
> + u8 cmd[] = {
> + [0] = 0x25, /* % */
> + [1] = 0x56 /* V */
> + };
> + u8 rx[14];
> + int ret;
> +
> + ret = qnap_mcu_exec(mcu, cmd, sizeof(cmd), rx, 6);
> + if (ret)
> + return ret;
> +
> + memcpy(mcu->version, &rx[2], 4);
> +
> + 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)
> +{
> + struct qnap_mcu *mcu = data->cb_data;
> + int ret;
> + u8 cmd[] = {
> + [0] = 0x40, /* @ */
> + [1] = 0x43, /* C */
> + [2] = 0x30 /* 0 */
> + };
u8 cmd [] = { '@', 'C', '0' }; ?
> + dev_dbg(&mcu->serdev->dev, "running MCU poweroff\n");
This is unlikely to be useful post-development.
> + 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 */
Please start sentences with upper-case chars.
> + .fan_pwm_max = 255,
> + .usb_led = true,
> +};
> +
> +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 const struct mfd_cell qnap_mcu_subdevs[] = {
> + { .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);
> + mutex_init(&mcu->reply_lock);
Can you not get away with a single lock?
> + 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) {
> + dev_err(dev, "Failed to set parity\n");
> + return ret;
> + }
> +
> + 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");
> +
> + ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, qnap_mcu_subdevs,
> + ARRAY_SIZE(qnap_mcu_subdevs), NULL, 0, NULL);
> + if (ret)
> + return dev_err_probe(dev, ret, "adding qnap mfd devices failed\n");
"Failed to add child devices"
> +
> + return 0;
> +}
> +
> +static struct serdev_device_driver qnap_mcu_drv = {
> + .probe = qnap_mcu_probe,
> + .driver = {
> + .name = "qnap-mcu",
> + .of_match_table = qnap_mcu_dt_ids,
> + },
> +};
This tabbing is odd.
> +module_serdev_device_driver(qnap_mcu_drv);
> +
> +MODULE_LICENSE("GPL");
Suggest this goes at the bottom.
> +MODULE_AUTHOR("Heiko Stuebner <heiko@sntech.de>");
> +MODULE_DESCRIPTION("QNAP MCU core driver");
> diff --git a/include/linux/mfd/qnap-mcu.h b/include/linux/mfd/qnap-mcu.h
> new file mode 100644
> index 0000000000000..f954815d3025b
> --- /dev/null
> +++ b/include/linux/mfd/qnap-mcu.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
As above.
> +/*
> + * 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);
> +const struct qnap_mcu_variant *qnap_mcu_get_variant_data(struct qnap_mcu *mcu);
> +
> +#endif /* _LINUX_QNAP_MCU_H_ */
> --
> 2.39.2
>
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 2/7] mfd: add base driver for qnap-mcu devices
2024-08-16 17:13 ` Lee Jones
@ 2024-08-18 21:15 ` Heiko Stübner
2024-08-22 10:10 ` Lee Jones
0 siblings, 1 reply; 14+ messages in thread
From: Heiko Stübner @ 2024-08-18 21:15 UTC (permalink / raw)
To: Lee Jones
Cc: jdelvare, linux, dmitry.torokhov, pavel, robh, krzk+dt, conor+dt,
ukleinek, devicetree, linux-kernel, linux-arm-kernel,
linux-rockchip, linux-hwmon, linux-input, linux-leds
Hi Lee,
thanks a lot for your review,
Am Freitag, 16. August 2024, 19:13:36 CEST schrieb Lee Jones:
> On Sat, 10 Aug 2024, Heiko Stuebner wrote:
> > +/*
> > + * MFD core driver for the MCU in Qnap NAS devices that is connected
>
> No such thing as an "MFD". Please describe the device.
>
> Is it QNAP or Qnap? Please be consistent.
Looks like QNAP spells itself in all upper case on their website, so I did
go with that
> > + * via a dedicated UART port
> > + *
> > + * Copyright (C) 2024 Heiko Stuebner <heiko@sntech.de>
> > + */
> > +
> > +#include <linux/export.h>
>
> What is this used for?
the #define EXPORT_SYMBOL_GPL that gets used below lives in that file.
> > + * struct qnap_mcu - QNAP NAS embedded controller
> > + *
> > + * @serdev: Pointer to underlying serdev
> > + * @bus_lock: Lock to serialize access to the device
> > + * @reply_lock: Lock protecting @reply
> > + * @reply: Pointer to memory to store reply payload
> > + * @variant: Device variant specific information
> > + * @version: MCU firmware version
> > + */
> > +struct qnap_mcu {
> > + struct serdev_device *serdev;
> > + /* Serialize access to the device */
>
> Comments and K-doc is OOT.
I don't really know what OOT means, but I guess you mean that only
one or the other is needed?
Though somehow checkpatch was very unhappy without those comments directly
above the mutex declaration. But I can of course remove these additional
comments
> > + mutex_lock(&mcu->reply_lock);
> > + if (!reply) {
> > + dev_warn(dev, "received %zu bytes, we were not waiting for\n",
> > + size);
> > + mutex_unlock(&mcu->reply_lock);
>
> guard(mutex)?
thanks a lot for pointing out this neat feature
> > +int qnap_mcu_exec_with_ack(struct qnap_mcu *mcu,
> > + const u8 *cmd_data, size_t cmd_data_size)
> > +{
> > + u8 ack[2];
> > + 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] != 0x40 || ack[1] != 0x30) {
>
> Why not use the char variants?
Consistency, I guess.
While the basic commands _seem_ to use values equivalent to ascii
characters, I've also seen other commands where the values are not based
on those but use real numbers / hex values for data instead.
So I opted to go with hex values for all and kept the comments, if someone
wants to get started talking to their MCU via a terminal.
> > +
> > +/*
> > + * 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)
> > +{
> > + struct qnap_mcu *mcu = data->cb_data;
> > + int ret;
> > + u8 cmd[] = {
> > + [0] = 0x40, /* @ */
> > + [1] = 0x43, /* C */
> > + [2] = 0x30 /* 0 */
> > + };
>
> u8 cmd [] = { '@', 'C', '0' }; ?
see above.
I guess this is a style choice, we could of course go with
u8 cmd[] = { 0x40, 0x43, 0x30 }
if you prefer that.
> > +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);
> > + mutex_init(&mcu->reply_lock);
>
> Can you not get away with a single lock?
Of course all behaviour information of the device come from observation
alone right now, but it does look like the MCU does not cause
transmissions of its own, but only as a reply to a sent command.
So yes, I can probably do away with the whole reply assignment and just
use one dataset and thus just the main lock.
Heiko
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 2/7] mfd: add base driver for qnap-mcu devices
2024-08-18 21:15 ` Heiko Stübner
@ 2024-08-22 10:10 ` Lee Jones
0 siblings, 0 replies; 14+ messages in thread
From: Lee Jones @ 2024-08-22 10:10 UTC (permalink / raw)
To: Heiko Stübner
Cc: jdelvare, linux, dmitry.torokhov, pavel, robh, krzk+dt, conor+dt,
ukleinek, devicetree, linux-kernel, linux-arm-kernel,
linux-rockchip, linux-hwmon, linux-input, linux-leds
On Sun, 18 Aug 2024, Heiko Stübner wrote:
> Hi Lee,
>
> thanks a lot for your review,
>
> Am Freitag, 16. August 2024, 19:13:36 CEST schrieb Lee Jones:
> > On Sat, 10 Aug 2024, Heiko Stuebner wrote:
>
>
> > > +/*
> > > + * MFD core driver for the MCU in Qnap NAS devices that is connected
> >
> > No such thing as an "MFD". Please describe the device.
> >
> > Is it QNAP or Qnap? Please be consistent.
>
> Looks like QNAP spells itself in all upper case on their website, so I did
> go with that
Super, thanks.
> > > +
> > > +/*
> > > + * 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)
> > > +{
> > > + struct qnap_mcu *mcu = data->cb_data;
> > > + int ret;
> > > + u8 cmd[] = {
> > > + [0] = 0x40, /* @ */
> > > + [1] = 0x43, /* C */
> > > + [2] = 0x30 /* 0 */
> > > + };
> >
> > u8 cmd [] = { '@', 'C', '0' }; ?
>
> see above.
>
> I guess this is a style choice, we could of course go with
> u8 cmd[] = { 0x40, 0x43, 0x30 }
> if you prefer that.
Yes please.
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 3/7] leds: add driver for LEDs from qnap-mcu devices
2024-08-10 18:47 [PATCH v4 0/7] Drivers to support the MCU on QNAP NAS devices Heiko Stuebner
2024-08-10 18:47 ` [PATCH v4 1/7] dt-bindings: mfd: add binding for qnap,ts433-mcu devices Heiko Stuebner
2024-08-10 18:47 ` [PATCH v4 2/7] mfd: add base driver for qnap-mcu devices Heiko Stuebner
@ 2024-08-10 18:47 ` Heiko Stuebner
2024-08-10 18:47 ` [PATCH v4 4/7] Input: add driver for the input part of " Heiko Stuebner
` (3 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Heiko Stuebner @ 2024-08-10 18:47 UTC (permalink / raw)
To: lee, jdelvare, linux, dmitry.torokhov, pavel
Cc: robh, krzk+dt, conor+dt, heiko, ukleinek, devicetree,
linux-kernel, linux-arm-kernel, linux-rockchip, linux-hwmon,
linux-input, linux-leds
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 | 247 +++++++++++++++++++++++++++++++++++
4 files changed, 260 insertions(+)
create mode 100644 drivers/leds/leds-qnap-mcu.c
diff --git a/MAINTAINERS b/MAINTAINERS
index f9f605a3c12a4..1473c3f7fdb04 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18655,6 +18655,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 8d9d8da376e46..9a337478dd80c 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 18afbb5a23ee5..c6f74865d7299 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 0000000000000..aa5d662f2c7fa
--- /dev/null
+++ b/drivers/leds/leds-qnap-mcu.c
@@ -0,0 +1,247 @@
+// 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 value)
+{
+ struct qnap_mcu_err_led *err_led = cdev_to_qnap_mcu_err_led(led_cdev);
+ u8 cmd[] = {
+ [0] = 0x40,
+ [1] = 0x52,
+ [2] = 0x30 + err_led->num,
+ [3] = 0x30
+ };
+
+ /*
+ * If the led is off, turn it on. Otherwise don't disturb
+ * a possible set blink-mode.
+ */
+ if (value == 0)
+ err_led->mode = QNAP_MCU_ERR_LED_OFF;
+ else if (err_led->mode == QNAP_MCU_ERR_LED_OFF)
+ err_led->mode = QNAP_MCU_ERR_LED_ON;
+
+ cmd[3] = 0x30 + 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[] = {
+ [0] = 0x40,
+ [1] = 0x52,
+ [2] = 0x30 + err_led->num,
+ [3] = 0x30
+ };
+
+ /* 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] = 0x30 + 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)
+{
+ 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->mode = QNAP_MCU_ERR_LED_OFF;
+
+ snprintf(err_led->name, LED_MAX_NAME_SIZE, "hdd%d:red:status", num + 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 dev_err_probe(dev, ret, "failed to register hdd led %d", num);
+
+ 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 value)
+{
+ struct qnap_mcu_usb_led *usb_led = cdev_to_qnap_mcu_usb_led(led_cdev);
+ u8 cmd[] = {
+ [0] = 0x40,
+ [1] = 0x43,
+ [2] = 0
+ };
+
+ /*
+ * If the led is off, turn it on. Otherwise don't disturb
+ * a possible set blink-mode.
+ */
+ if (value == 0)
+ usb_led->mode = QNAP_MCU_USB_LED_OFF;
+ else if (usb_led->mode == QNAP_MCU_USB_LED_OFF)
+ usb_led->mode = QNAP_MCU_USB_LED_ON;
+
+ /* byte 3 is shared between the usb led target and setting the mode */
+ cmd[2] = 0x44 | 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[] = {
+ [0] = 0x40,
+ [1] = 0x43,
+ [2] = 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 and setting the mode */
+ cmd[2] = 0x44 | 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 dev_err_probe(dev, ret, "failed to register usb led");
+
+ 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 = qnap_mcu_get_variant_data(mcu);
+ int ret, i;
+
+ for (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 %d\n", i);
+ }
+
+ 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.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v4 4/7] Input: add driver for the input part of qnap-mcu devices
2024-08-10 18:47 [PATCH v4 0/7] Drivers to support the MCU on QNAP NAS devices Heiko Stuebner
` (2 preceding siblings ...)
2024-08-10 18:47 ` [PATCH v4 3/7] leds: add driver for LEDs from " Heiko Stuebner
@ 2024-08-10 18:47 ` Heiko Stuebner
2024-08-10 18:47 ` [PATCH v4 5/7] hwmon: add driver for the hwmon parts " Heiko Stuebner
` (2 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Heiko Stuebner @ 2024-08-10 18:47 UTC (permalink / raw)
To: lee, jdelvare, linux, dmitry.torokhov, pavel
Cc: robh, krzk+dt, conor+dt, heiko, ukleinek, devicetree,
linux-kernel, linux-arm-kernel, linux-rockchip, linux-hwmon,
linux-input, linux-leds
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 | 161 ++++++++++++++++++++++++++++
4 files changed, 175 insertions(+)
create mode 100644 drivers/input/misc/qnap-mcu-input.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 1473c3f7fdb04..baaec814bea1b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18655,6 +18655,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 6a852c76331b6..13d135257e060 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 4f7f736831ba8..6d91804d0a6f7 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 0000000000000..8debc8f988246
--- /dev/null
+++ b/drivers/input/misc/qnap-mcu-input.c
@@ -0,0 +1,161 @@
+// 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[] = {
+ [0] = 0x40, /* @ */
+ [1] = 0x43, /* C */
+ [2] = 0x56 /* 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[] = {
+ [0] = 0x40, /* @ */
+ [1] = 0x43, /* C */
+ [2] = (idev->beep_type == SND_TONE) ? 0x33 : 0x32
+ };
+
+ 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.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v4 5/7] hwmon: add driver for the hwmon parts of qnap-mcu devices
2024-08-10 18:47 [PATCH v4 0/7] Drivers to support the MCU on QNAP NAS devices Heiko Stuebner
` (3 preceding siblings ...)
2024-08-10 18:47 ` [PATCH v4 4/7] Input: add driver for the input part of " Heiko Stuebner
@ 2024-08-10 18:47 ` Heiko Stuebner
2024-08-13 16:03 ` Guenter Roeck
2024-08-10 18:47 ` [PATCH v4 6/7] arm64: dts: rockchip: hook up the MCU on the QNAP TS433 Heiko Stuebner
2024-08-10 18:47 ` [PATCH v4 7/7] arm64: dts: rockchip: set hdd led labels on qnap-ts433 Heiko Stuebner
6 siblings, 1 reply; 14+ messages in thread
From: Heiko Stuebner @ 2024-08-10 18:47 UTC (permalink / raw)
To: lee, jdelvare, linux, dmitry.torokhov, pavel
Cc: robh, krzk+dt, conor+dt, heiko, ukleinek, devicetree,
linux-kernel, linux-arm-kernel, linux-rockchip, linux-hwmon,
linux-input, linux-leds
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.
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 | 392 +++++++++++++++++++++++++
6 files changed, 434 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 913c11390a457..7adbe23f06597 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 0000000000000..83407e3408f2b
--- /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 baaec814bea1b..282042e3d5f80 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18655,6 +18655,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 b60fe2e58ad6c..0118ad91057e0 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1793,6 +1793,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 b1c7056c37db5..d60f46a03cc96 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 0000000000000..f106fe8b012ea
--- /dev/null
+++ b/drivers/hwmon/qnap-mcu-hwmon.c
@@ -0,0 +1,392 @@
+// 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[] = {
+ [0] = 0x40, /* @ */
+ [1] = 0x46, /* F */
+ [2] = 0x41 /* 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[] = {
+ [0] = 0x40, /* @ */
+ [1] = 0x46, /* F */
+ [2] = 0x5a, /* Z */
+ [3] = 0x30 /* 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[] = {
+ [0] = 0x40, /* @ */
+ [1] = 0x46, /* F */
+ [2] = 0x57, /* W */
+ [3] = 0x30, /* 0 ... fan-id? */
+ [4] = pwm
+ };
+
+ /* 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[] = {
+ [0] = 0x40, /* @ */
+ [1] = 0x54, /* T */
+ [2] = 0x33 /* 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;
+
+ if (!fwnode_property_present(fwnode, "cooling-levels"))
+ return 0;
+
+ ret = fwnode_property_count_u32(fwnode, "cooling-levels");
+ if (ret <= 0) {
+ dev_err(dev, "Failed to count elements in 'cooling-levels'\n");
+ return ret ? : -EINVAL;
+ }
+
+ num = ret;
+ 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) {
+ dev_err(dev, "Failed to read 'cooling-levels'\n");
+ return ret;
+ }
+
+ for (i = 0; i < num; i++) {
+ if (hwm->fan_cooling_levels[i] > hwm->pwm_max) {
+ dev_err(dev, "fan state[%d]:%d > %d\n", i,
+ hwm->fan_cooling_levels[i], hwm->pwm_max);
+ return -EINVAL;
+ }
+ }
+
+ 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 = qnap_mcu_get_variant_data(mcu);
+ 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.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4 5/7] hwmon: add driver for the hwmon parts of qnap-mcu devices
2024-08-10 18:47 ` [PATCH v4 5/7] hwmon: add driver for the hwmon parts " Heiko Stuebner
@ 2024-08-13 16:03 ` Guenter Roeck
2024-08-13 20:39 ` Heiko Stübner
0 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2024-08-13 16:03 UTC (permalink / raw)
To: Heiko Stuebner, lee, jdelvare, dmitry.torokhov, pavel
Cc: robh, krzk+dt, conor+dt, ukleinek, devicetree, linux-kernel,
linux-arm-kernel, linux-rockchip, linux-hwmon, linux-input,
linux-leds
On 8/10/24 11:47, Heiko Stuebner wrote:
> 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.
>
> 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 | 392 +++++++++++++++++++++++++
> 6 files changed, 434 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 913c11390a457..7adbe23f06597 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 0000000000000..83407e3408f2b
> --- /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 baaec814bea1b..282042e3d5f80 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -18655,6 +18655,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 b60fe2e58ad6c..0118ad91057e0 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1793,6 +1793,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 b1c7056c37db5..d60f46a03cc96 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 0000000000000..f106fe8b012ea
> --- /dev/null
> +++ b/drivers/hwmon/qnap-mcu-hwmon.c
> @@ -0,0 +1,392 @@
> +// 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[] = {
> + [0] = 0x40, /* @ */
> + [1] = 0x46, /* F */
> + [2] = 0x41 /* 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[] = {
> + [0] = 0x40, /* @ */
> + [1] = 0x46, /* F */
> + [2] = 0x5a, /* Z */
> + [3] = 0x30 /* 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[] = {
> + [0] = 0x40, /* @ */
> + [1] = 0x46, /* F */
> + [2] = 0x57, /* W */
> + [3] = 0x30, /* 0 ... fan-id? */
> + [4] = pwm
> + };
> +
> + /* 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[] = {
> + [0] = 0x40, /* @ */
> + [1] = 0x54, /* T */
> + [2] = 0x33 /* 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");
Is the dummy "-0" index mandated somewhere ?
I don't care either way, it just seems odd. Either case,
Acked-by: Guenter Roeck <linux@roeck-us.net>
> + 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;
> +
> + if (!fwnode_property_present(fwnode, "cooling-levels"))
> + return 0;
> +
Side note: One could argue that a sub-node with no content does not really
make sense and should be invalid.
> + ret = fwnode_property_count_u32(fwnode, "cooling-levels");
> + if (ret <= 0) {
> + dev_err(dev, "Failed to count elements in 'cooling-levels'\n");
> + return ret ? : -EINVAL;
> + }
> +
> + num = ret;
Another side note: Using ret here isn't necessary. I'd just have used
'num' directly.
> + 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) {
> + dev_err(dev, "Failed to read 'cooling-levels'\n");
> + return ret;
> + }
> +
> + for (i = 0; i < num; i++) {
> + if (hwm->fan_cooling_levels[i] > hwm->pwm_max) {
> + dev_err(dev, "fan state[%d]:%d > %d\n", i,
> + hwm->fan_cooling_levels[i], hwm->pwm_max);
> + return -EINVAL;
In case you send another version, you might want to consider using dev_err_probe().
> + }
> + }
> +
> + 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 = qnap_mcu_get_variant_data(mcu);
> + 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");
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 5/7] hwmon: add driver for the hwmon parts of qnap-mcu devices
2024-08-13 16:03 ` Guenter Roeck
@ 2024-08-13 20:39 ` Heiko Stübner
0 siblings, 0 replies; 14+ messages in thread
From: Heiko Stübner @ 2024-08-13 20:39 UTC (permalink / raw)
To: lee, jdelvare, dmitry.torokhov, pavel, Guenter Roeck
Cc: robh, krzk+dt, conor+dt, ukleinek, devicetree, linux-kernel,
linux-arm-kernel, linux-rockchip, linux-hwmon, linux-input,
linux-leds
Hi Guenter,
Am Dienstag, 13. August 2024, 18:03:57 CEST schrieb Guenter Roeck:
> On 8/10/24 11:47, Heiko Stuebner wrote:
> > +static int qnap_mcu_hwmon_set_pwm(struct qnap_mcu_hwmon *hwm, u8 pwm)
> > +{
> > + const u8 cmd[] = {
> > + [0] = 0x40, /* @ */
> > + [1] = 0x46, /* F */
> > + [2] = 0x57, /* W */
> > + [3] = 0x30, /* 0 ... fan-id? */
> > + [4] = pwm
> > + };
> > +
> > + /* set the fan pwm */
> > + return qnap_mcu_exec_with_ack(hwm->mcu, cmd, sizeof(cmd));
> > +}
> > +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");
>
> Is the dummy "-0" index mandated somewhere ?
I don't think it is. The node should just begin with "fan" I think.
I've added the -0 because from everything I've seen of the qnap software-
side, the mcu firmware can address multiple fans.
In the original firmware, everything is done in userspace wrt. the MCU,
and the fan commands in their abstraction layer allow for multiple fans.
Also there is that suspicious "0" in the command sequence when
getting/setting the fan pwm. And in the original device config this is
labeled as the first fan.
From everything I've seen, the MCU is not limited to the Rockchip-line
of devices and I do hope others will find this useful in the future,
so adding the "-0" was a better safe than sorry choice.
Because that way adding that theoretical 2nd fan in the future won't
cause too much trouble in the dt-binding.
> I don't care either way, it just seems odd. Either case,
>
> Acked-by: Guenter Roeck <linux@roeck-us.net>
>
> > + 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;
> > +
> > + if (!fwnode_property_present(fwnode, "cooling-levels"))
> > + return 0;
> > +
>
> Side note: One could argue that a sub-node with no content does not really
> make sense and should be invalid.
I remember thinking about that, but didn't come to a real decision on my
own, hence left it as it was. So will follow your suggestion :-)
> > + ret = fwnode_property_count_u32(fwnode, "cooling-levels");
> > + if (ret <= 0) {
> > + dev_err(dev, "Failed to count elements in 'cooling-levels'\n");
> > + return ret ? : -EINVAL;
> > + }
> > +
> > + num = ret;
>
> Another side note: Using ret here isn't necessary. I'd just have used
> 'num' directly.
will do
>
> > + 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) {
> > + dev_err(dev, "Failed to read 'cooling-levels'\n");
> > + return ret;
> > + }
> > +
> > + for (i = 0; i < num; i++) {
> > + if (hwm->fan_cooling_levels[i] > hwm->pwm_max) {
> > + dev_err(dev, "fan state[%d]:%d > %d\n", i,
> > + hwm->fan_cooling_levels[i], hwm->pwm_max);
> > + return -EINVAL;
>
> In case you send another version, you might want to consider using dev_err_probe().
ok will do.
I was probably way overthinking if I should not use dev_err_probe in a
function that is not a probe function (though of course part of the probe
process).
Thanks a lot for looking over the code once again
Heiko
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 6/7] arm64: dts: rockchip: hook up the MCU on the QNAP TS433
2024-08-10 18:47 [PATCH v4 0/7] Drivers to support the MCU on QNAP NAS devices Heiko Stuebner
` (4 preceding siblings ...)
2024-08-10 18:47 ` [PATCH v4 5/7] hwmon: add driver for the hwmon parts " Heiko Stuebner
@ 2024-08-10 18:47 ` Heiko Stuebner
2024-08-10 18:47 ` [PATCH v4 7/7] arm64: dts: rockchip: set hdd led labels on qnap-ts433 Heiko Stuebner
6 siblings, 0 replies; 14+ messages in thread
From: Heiko Stuebner @ 2024-08-10 18:47 UTC (permalink / raw)
To: lee, jdelvare, linux, dmitry.torokhov, pavel
Cc: robh, krzk+dt, conor+dt, heiko, ukleinek, devicetree,
linux-kernel, linux-arm-kernel, linux-rockchip, linux-hwmon,
linux-input, linux-leds
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 90d8d5266299b..6405ec7d47361 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
@@ -463,6 +463,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";
@@ -562,6 +610,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.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v4 7/7] arm64: dts: rockchip: set hdd led labels on qnap-ts433
2024-08-10 18:47 [PATCH v4 0/7] Drivers to support the MCU on QNAP NAS devices Heiko Stuebner
` (5 preceding siblings ...)
2024-08-10 18:47 ` [PATCH v4 6/7] arm64: dts: rockchip: hook up the MCU on the QNAP TS433 Heiko Stuebner
@ 2024-08-10 18:47 ` Heiko Stuebner
6 siblings, 0 replies; 14+ messages in thread
From: Heiko Stuebner @ 2024-08-10 18:47 UTC (permalink / raw)
To: lee, jdelvare, linux, dmitry.torokhov, pavel
Cc: robh, krzk+dt, conor+dt, heiko, ukleinek, devicetree,
linux-kernel, linux-arm-kernel, linux-rockchip, linux-hwmon,
linux-input, linux-leds
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 6405ec7d47361..110c323786848 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.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread