[PATCH 0/7] Add support for the iEi Puzzle-M801 board

[PATCH 0/7] Add support for the iEi Puzzle-M801 board

[Luka Kovacic]

This patchset adds support for the iEi Puzzle-M801 1U Rackmount Network
Appliance and for the iEi WT61P803 PUZZLE microcontroller, which enables
some board specific features like fan and LED control, system power
management and temperature sensor reading.

The platform is based on the popular Marvell Armada 8040 SoC and supports
up to 16 GB of DDR4 2400 MHz ECC RAM.
It has a PCIe x16 slot (x2 lanes only) and an M.2 type B slot.

External chassis ports:
2x 10 GbE SFP+
4x 1 GbE (Marvell 88E1512P)
2x USB 3.0
1x RJ45 serial port

All notable board components are supported in this patchset.

Luka Kovacic (7):
  dt-bindings: Add iEi vendor prefix and iEi WT61P803 PUZZLE driver
    bindings
  drivers: mfd: Add a driver for iEi WT61P803 PUZZLE MCU
  drivers: hwmon: Add the iEi WT61P803 PUZZLE HWMON driver
  drivers: leds: Add the iEi WT61P803 PUZZLE LED driver
  Documentation/ABI: Add iei-wt61p803-puzzle driver sysfs interface
    documentation
  MAINTAINERS: Add an entry for the iEi WT61P803 PUZZLE driver
  arm64: dts: marvell: Add a device tree for the iEi Puzzle-M801 board

 .../stable/sysfs-driver-iei-wt61p803-puzzle   |   65 +
 .../hwmon/iei,wt61p803-puzzle-hwmon.yaml      |   41 +
 .../leds/iei,wt61p803-puzzle-leds.yaml        |   48 +
 .../bindings/mfd/iei,wt61p803-puzzle.yaml     |   82 ++
 .../devicetree/bindings/vendor-prefixes.yaml  |    2 +
 MAINTAINERS                                   |   13 +
 arch/arm64/boot/dts/marvell/Makefile          |    1 +
 .../dts/marvell/armada-8040-puzzle-m801.dts   |  519 +++++++
 drivers/hwmon/Kconfig                         |    8 +
 drivers/hwmon/Makefile                        |    1 +
 drivers/hwmon/iei-wt61p803-puzzle-hwmon.c     |  613 ++++++++
 drivers/leds/Kconfig                          |    8 +
 drivers/leds/Makefile                         |    1 +
 drivers/leds/leds-iei-wt61p803-puzzle.c       |  184 +++
 drivers/mfd/Kconfig                           |    8 +
 drivers/mfd/Makefile                          |    1 +
 drivers/mfd/iei-wt61p803-puzzle.c             | 1242 +++++++++++++++++
 include/linux/mfd/iei-wt61p803-puzzle.h       |   27 +
 18 files changed, 2864 insertions(+)
 create mode 100644 Documentation/ABI/stable/sysfs-driver-iei-wt61p803-puzzle
 create mode 100644 Documentation/devicetree/bindings/hwmon/iei,wt61p803-puzzle-hwmon.yaml
 create mode 100644 Documentation/devicetree/bindings/leds/iei,wt61p803-puzzle-leds.yaml
 create mode 100644 Documentation/devicetree/bindings/mfd/iei,wt61p803-puzzle.yaml
 create mode 100644 arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dts
 create mode 100644 drivers/hwmon/iei-wt61p803-puzzle-hwmon.c
 create mode 100644 drivers/leds/leds-iei-wt61p803-puzzle.c
 create mode 100644 drivers/mfd/iei-wt61p803-puzzle.c
 create mode 100644 include/linux/mfd/iei-wt61p803-puzzle.h

-- 
2.20.1

[PATCH 1/7] dt-bindings: Add iEi vendor prefix and iEi WT61P803 PUZZLE driver bindings

[Luka Kovacic]

Add the iEi WT61P803 PUZZLE Device Tree bindings for MFD, HWMON and LED
drivers. A new vendor prefix is also added accordingly for
IEI Integration Corp.

Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
Cc: Luka Perkov <luka.perkov@sartura.hr>
---
 .../hwmon/iei,wt61p803-puzzle-hwmon.yaml      | 41 ++++++++++
 .../leds/iei,wt61p803-puzzle-leds.yaml        | 48 +++++++++++
 .../bindings/mfd/iei,wt61p803-puzzle.yaml     | 82 +++++++++++++++++++
 .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
 4 files changed, 173 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/iei,wt61p803-puzzle-hwmon.yaml
 create mode 100644 Documentation/devicetree/bindings/leds/iei,wt61p803-puzzle-leds.yaml
 create mode 100644 Documentation/devicetree/bindings/mfd/iei,wt61p803-puzzle.yaml

diff --git a/Documentation/devicetree/bindings/hwmon/iei,wt61p803-puzzle-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/iei,wt61p803-puzzle-hwmon.yaml
new file mode 100644
index 000000000000..37f0030df237
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/iei,wt61p803-puzzle-hwmon.yaml
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/iei,wt61p803-puzzle-hwmon.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: iEi WT61P803 PUZZLE MCU HWMON module from IEI Integration Corp.
+
+maintainers:
+  - Luka Kovacic <luka.kovacic@sartura.hr>
+
+description: |
+  This module is a part of the iEi WT61P803 PUZZLE MFD device. For more details
+  see Documentation/devicetree/bindings/mfd/iei,wt61p803-puzzle.yaml.
+
+  The HWMON module is a sub-node of the MCU node in the Device Tree.
+
+properties:
+  compatible:
+    const: iei,wt61p803-puzzle-hwmon
+
+patternProperties:
+  "^fan-group@[0-1]$":
+    type: object
+    properties:
+      reg:
+        minimum: 0
+        maximum: 1
+        description:
+          Fan group ID
+      cooling-levels:
+        maxItems: 255
+        description:
+          Cooling levels for the fans (PWM value mapping)
+    description: |
+      Properties for each fan group.
+    required:
+      - reg
+
+required:
+  - compatible
diff --git a/Documentation/devicetree/bindings/leds/iei,wt61p803-puzzle-leds.yaml b/Documentation/devicetree/bindings/leds/iei,wt61p803-puzzle-leds.yaml
new file mode 100644
index 000000000000..502d97630ecc
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/iei,wt61p803-puzzle-leds.yaml
@@ -0,0 +1,48 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/iei,wt61p803-puzzle-leds.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: iEi WT61P803 PUZZLE MCU LED module from IEI Integration Corp.
+
+maintainers:
+  - Luka Kovacic <luka.kovacic@sartura.hr>
+
+description: |
+  This module is a part of the iEi WT61P803 PUZZLE MFD device. For more details
+  see Documentation/devicetree/bindings/mfd/iei,wt61p803-puzzle.yaml.
+
+  The LED module is a sub-node of the MCU node in the Device Tree.
+
+properties:
+  compatible:
+    const: iei,wt61p803-puzzle-leds
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+patternProperties:
+  "^led@0$":
+    type: object
+    description: |
+      Properties for a single LED.
+
+    properties:
+      reg:
+        description:
+          Index of the LED. Only one LED is supported at the moment.
+        minimum: 0
+        maximum: 0
+
+      label: true
+
+      linux,default-trigger: true
+
+required:
+  - compatible
+  - "#address-cells"
+  - "#size-cells"
diff --git a/Documentation/devicetree/bindings/mfd/iei,wt61p803-puzzle.yaml b/Documentation/devicetree/bindings/mfd/iei,wt61p803-puzzle.yaml
new file mode 100644
index 000000000000..a1a155f19557
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/iei,wt61p803-puzzle.yaml
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/iei,wt61p803-puzzle.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: iEi WT61P803 PUZZLE MCU from IEI Integration Corp.
+
+maintainers:
+  - Luka Kovacic <luka.kovacic@sartura.hr>
+
+description: |
+  iEi WT61P803 PUZZLE MCU is embedded in some iEi Puzzle series boards.
+  It's used for controlling system power states, fans, leds and temperature
+  sensors.
+
+  For Device Tree bindings of other sub-modules (HWMON, LEDs) refer to the
+  binding documents under the respective subsystem directories.
+
+properties:
+  compatible:
+    const: iei,wt61p803-puzzle
+
+  current-speed:
+    description:
+      Serial bus speed in bps
+    maxItems: 1
+
+  enable-probe-beep: true
+
+  iei-wt61p803-hwmon:
+    $ref: ../hwmon/iei,wt61p803-puzzle-hwmon.yaml
+
+  leds:
+    $ref: ../leds/iei,wt61p803-puzzle-leds.yaml
+
+required:
+  - compatible
+  - current-speed
+
+examples:
+  - |
+    #include <dt-bindings/leds/common.h>
+    serial {
+        status = "okay";
+        mcu {
+            compatible = "iei,wt61p803-puzzle";
+            current-speed = <115200>;
+            enable-probe-beep;
+
+            leds {
+                compatible = "iei,wt61p803-puzzle-leds";
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                led@0 {
+                    reg = <0>;
+                    color = <LED_COLOR_ID_BLUE>;
+                    label = "front-power-led";
+                };
+            };
+
+            iei-wt61p803-puzzle-hwmon {
+                compatible = "iei,wt61p803-puzzle-hwmon";
+
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                fan-group@0 {
+                    #cooling-cells = <2>;
+                    reg = <0x00>;
+                    cooling-levels = <64 102 170 230 250>;
+                };
+
+                fan-group@1 {
+                    #cooling-cells = <2>;
+                    reg = <0x01>;
+                    cooling-levels = <64 102 170 230 250>;
+                };
+            };
+        };
+    };
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 63996ab03521..5f2595f0b2ad 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -467,6 +467,8 @@ patternProperties:
     description: IC Plus Corp.
   "^idt,.*":
     description: Integrated Device Technologies, Inc.
+  "^iei,.*":
+    description: IEI Integration Corp.
   "^ifi,.*":
     description: Ingenieurburo Fur Ic-Technologie (I/F/I)
   "^ilitek,.*":
-- 
2.20.1

[PATCH 2/7] drivers: mfd: Add a driver for iEi WT61P803 PUZZLE MCU

[Luka Kovacic]

Add a driver for the iEi WT61P803 PUZZLE microcontroller, used in some
iEi Puzzle series devices. The microcontroller controls system power,
temperature sensors, fans and LEDs.

This driver implements the core functionality for device communication
over the system serial (serdev bus). It handles MCU messages and the
internal MCU properties. Some properties can be managed over sysfs.

Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
Cc: Luka Perkov <luka.perkov@sartura.hr>
---
 drivers/mfd/Kconfig                     |    8 +
 drivers/mfd/Makefile                    |    1 +
 drivers/mfd/iei-wt61p803-puzzle.c       | 1242 +++++++++++++++++++++++
 include/linux/mfd/iei-wt61p803-puzzle.h |   27 +
 4 files changed, 1278 insertions(+)
 create mode 100644 drivers/mfd/iei-wt61p803-puzzle.c
 create mode 100644 include/linux/mfd/iei-wt61p803-puzzle.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 33df0837ab41..3fcda95f07a3 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2118,5 +2118,13 @@ config SGI_MFD_IOC3
 	  If you have an SGI Origin, Octane, or a PCI IOC3 card,
 	  then say Y. Otherwise say N.
 
+config MFD_IEI_WT61P803_PUZZLE
+	tristate "iEi WT61P803 PUZZLE MCU driver"
+	depends on SERIAL_DEV_BUS
+	help
+	  iEi WT61P803 PUZZLE is a system power management microcontroller
+	  used for fan control, temperature sensor reading, led control
+	  and system identification.
+
 endmenu
 endif
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index a60e5f835283..33b88023a68d 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -236,6 +236,7 @@ obj-$(CONFIG_MFD_HI655X_PMIC)   += hi655x-pmic.o
 obj-$(CONFIG_MFD_DLN2)		+= dln2.o
 obj-$(CONFIG_MFD_RT5033)	+= rt5033.o
 obj-$(CONFIG_MFD_SKY81452)	+= sky81452.o
+obj-$(CONFIG_MFD_IEI_WT61P803_PUZZLE)	+= iei-wt61p803-puzzle.o
 
 intel-soc-pmic-objs		:= intel_soc_pmic_core.o intel_soc_pmic_crc.o
 obj-$(CONFIG_INTEL_SOC_PMIC)	+= intel-soc-pmic.o
diff --git a/drivers/mfd/iei-wt61p803-puzzle.c b/drivers/mfd/iei-wt61p803-puzzle.c
new file mode 100644
index 000000000000..110ea1b84c53
--- /dev/null
+++ b/drivers/mfd/iei-wt61p803-puzzle.c
@@ -0,0 +1,1242 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/* iEi WT61P803 PUZZLE MCU Driver
+ *
+ * Copyright (C) 2020 Sartura Ltd.
+ * Author: Luka Kovacic <luka.kovacic@sartura.hr>
+ */
+
+#include <linux/atomic.h>
+#include <linux/delay.h>
+#include <linux/export.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/kernel.h>
+#include <linux/mfd/iei-wt61p803-puzzle.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/sched.h>
+#include <linux/serdev.h>
+#include <asm/unaligned.h>
+#include <linux/delay.h>
+#include <linux/sysfs.h>
+
+#define IEI_WT61P803_PUZZLE_MAX_COMMAND_LENGTH (20 + 2)
+#define IEI_WT61P803_PUZZLE_RESP_BUF_SIZE 512
+#define IEI_WT61P803_PUZZLE_GENERAL_TIMEOUT HZ
+
+/**
+ * struct iei_wt61p803_puzzle_mcu_status - MCU flags state
+ *
+ * @ac_recovery_status_flag: AC Recovery Status Flag
+ * @power_loss_recovery: System recovery after power loss
+ * @power_status: System Power-on Method
+ */
+struct iei_wt61p803_puzzle_mcu_status {
+	u8 ac_recovery_status_flag;
+	u8 power_loss_recovery;
+	u8 power_status;
+};
+
+/**
+ * enum iei_wt61p803_puzzle_reply_state - State of the reply
+ * @FRAME_OK: The frame was completely processed/received
+ * @FRAME_PROCESSING: First bytes were received, but the frame isn't complete
+ * @FRAME_STRUCT_EMPTY: The frame struct is empty, no data was received
+ * @FRAME_TIMEOUT: The frame processing timed out, communication failed
+ *
+ * The enum iei_wt61p803_puzzle_reply_state is used to describe the general state
+ * of the frame that is currently being received.
+ */
+enum iei_wt61p803_puzzle_reply_state {
+	FRAME_OK = 0x00,
+	FRAME_PROCESSING = 0x01,
+	FRAME_STRUCT_EMPTY = 0xFF,
+	FRAME_TIMEOUT = 0xFE
+};
+
+/**
+ * struct iei_wt61p803_puzzle_reply - MCU reply
+ *
+ * @size: Size of the MCU reply
+ * @data: Full MCU reply buffer
+ * @state: Current state of the packet
+ * @received: Was the response fullfilled
+ */
+struct iei_wt61p803_puzzle_reply {
+	size_t size;
+	unsigned char *data;
+	u8 state;
+	struct completion received;
+};
+
+/**
+ * struct iei_wt61p803_puzzle_mcu_version - MCU version status
+ *
+ * @version: Primary firmware version
+ * @build_info: Build date and time
+ * @bootloader_mode: Status of the MCU operation
+ * @protocol_version: MCU communication protocol version
+ * @serial_number: Device factory serial number
+ * @mac_address: Device factory MAC addresses
+ */
+struct iei_wt61p803_puzzle_mcu_version {
+	const char *version;
+	const char *build_info;
+	bool bootloader_mode;
+	const char *protocol_version;
+	const char *serial_number;
+	const char *mac_address[8];
+};
+
+/**
+ * struct iei_wt61p803_puzzle - iEi WT61P803 PUZZLE MCU Driver
+ *
+ * @serdev: Pointer to underlying serdev device
+ * @kobj: Pointer to kobject (sysfs)
+ * @reply_lock: Reply mutex lock
+ * @bus_lock: Bus mutex lock
+ * @reply: Pointer to the iei_wt61p803_puzzle_reply struct
+ * @version_lock: Version R/W mutex lock
+ * @version: Pointer to the iei_wt61p803_puzzle_mcu_version struct
+ * @status_lock: Status R/W mutex lock
+ * @status: Pointer to the iei_wt61p803_puzzle_mcu_status struct
+ */
+struct iei_wt61p803_puzzle {
+	struct serdev_device *serdev;
+	struct kobject *kobj;
+
+	struct mutex reply_lock;
+	struct mutex bus_lock;
+
+	struct iei_wt61p803_puzzle_reply *reply;
+
+	struct mutex version_lock;
+	struct iei_wt61p803_puzzle_mcu_version *version;
+
+	struct mutex status_lock;
+	struct iei_wt61p803_puzzle_mcu_status *status;
+};
+
+static const struct of_device_id iei_wt61p803_puzzle_dt_ids[] = {
+	{ .compatible = "iei,wt61p803-puzzle" },
+	{ /* sentinel */ }
+};
+
+unsigned char iei_wt61p803_puzzle_add_xor_checksum(unsigned char *buf,
+		unsigned char xor_len)
+{
+	unsigned char xor_sum = 0;
+	unsigned int i;
+
+	for (i = 0; i < xor_len; i++)
+		xor_sum ^= buf[i];
+
+	return xor_sum;
+}
+
+static int iei_wt61p803_puzzle_process_resp(struct iei_wt61p803_puzzle *mcu,
+		unsigned char *raw_resp_data, size_t size)
+{
+	struct device *dev = &mcu->serdev->dev;
+
+	unsigned char xor_checksum;
+
+	/*
+	 * Start receiving the frame/Continue receiving the frame
+	 *
+	 * The code below determines whether:
+	 * * we are receiving a new frame
+	 * * appending data to an existing frame
+	 *
+	 */
+
+	/* Lock the reply struct mutex */
+	mutex_lock(&mcu->reply_lock);
+
+	/* Check the incoming frame header */
+	if (!(raw_resp_data[0] == '@' || raw_resp_data[0] == '%' ||
+		(raw_resp_data[0] == 0xF7 && raw_resp_data[1] == 0xA1))) {
+
+		/* Frame header is not correct, check whether to append */
+		if (mcu->reply->state != FRAME_PROCESSING) {
+			/* The incoming frame should be discarded */
+			dev_err(dev, "Invalid frame header and state (0x%x)",
+					mcu->reply->state);
+
+			mutex_unlock(&mcu->reply_lock);
+			return -1;
+		}
+
+		/* The frame should be appended to the existing data */
+		memcpy(mcu->reply->data+mcu->reply->size, raw_resp_data, size);
+		mcu->reply->size += size;
+	} else {
+		/* Start processing a new frame */
+		memcpy(mcu->reply->data, raw_resp_data, size);
+		mcu->reply->size = size;
+		mcu->reply->state = FRAME_PROCESSING;
+	}
+
+	/* Create an xor checksum of the data */
+	xor_checksum = iei_wt61p803_puzzle_add_xor_checksum(mcu->reply->data,
+			((int)mcu->reply->size)-1);
+
+	/* Check whether the caluclated checksum matches the received one */
+	if (xor_checksum != mcu->reply->data[((int)mcu->reply->size)-1]) {
+		/* The checksum doesn't match, waiting for data */
+		mutex_unlock(&mcu->reply_lock);
+		return (int)size;
+	}
+
+	/*
+	 * Update internal driver state with the latest response code
+	 */
+
+	/* Received all the data */
+	mcu->reply->state = FRAME_OK;
+	complete(&mcu->reply->received);
+
+	mutex_unlock(&mcu->reply_lock);
+
+	return (int)size;
+}
+
+static int iei_wt61p803_puzzle_recv_buf(struct serdev_device *serdev,
+		const unsigned char *data, size_t size)
+{
+	struct iei_wt61p803_puzzle *mcu = serdev_device_get_drvdata(serdev);
+	int ret;
+
+	ret = iei_wt61p803_puzzle_process_resp(mcu, (unsigned char *)data, size);
+
+	/* Return the number of processed bytes if function returns error */
+	if (ret < 0)
+		return (int)size;
+
+	return ret;
+}
+
+static const struct serdev_device_ops iei_wt61p803_puzzle_serdev_device_ops = {
+	.receive_buf  = iei_wt61p803_puzzle_recv_buf,
+	.write_wakeup = serdev_device_write_wakeup,
+};
+
+/**
+ * iei_wt61p803_puzzle_write_command_watchdog() - Watchdog of the normal cmd
+ * @mcu: Pointer to the iei_wt61p803_puzzle core MFD struct
+ * @cmd: Pointer to the char array to send (size should be content + 1 (xor))
+ * @size: Size of the cmd char array
+ * @reply_data: Pointer to the reply/response data array (should be allocated)
+ * @reply_size: Pointer to size_t (size of reply_data)
+ * @retry_count: Number of times to retry sending the command to the MCU
+ */
+int iei_wt61p803_puzzle_write_command_watchdog(struct iei_wt61p803_puzzle *mcu,
+		unsigned char *cmd, size_t size, unsigned char *reply_data,
+		size_t *reply_size, int retry_count)
+{
+	struct device *dev = &mcu->serdev->dev;
+	int ret, i;
+
+	for (i = 0; i < retry_count; i++) {
+		ret = iei_wt61p803_puzzle_write_command(mcu, cmd, size,
+				reply_data, reply_size);
+
+		if (ret != -ETIMEDOUT)
+			return ret;
+	}
+
+	dev_err(dev, "%s: Command response timed out. Retries: %d", __func__,
+			retry_count);
+
+	return -ETIMEDOUT;
+}
+EXPORT_SYMBOL_GPL(iei_wt61p803_puzzle_write_command_watchdog);
+
+/**
+ * iei_wt61p803_puzzle_write_command() - Send a structured command to the MCU
+ * @mcu: Pointer to the iei_wt61p803_puzzle core MFD struct
+ * @cmd: Pointer to the char array to send (size should be content + 1 (xor))
+ * @size: Size of the cmd char array
+ * @reply_data: Pointer to the reply/response data array (should be allocated)
+ *
+ * Function iei_wt61p803_puzzle_write_command() sends a structured command to
+ * the MCU.
+ */
+int iei_wt61p803_puzzle_write_command(struct iei_wt61p803_puzzle *mcu,
+		unsigned char *cmd, size_t size, unsigned char *reply_data,
+		size_t *reply_size)
+{
+	struct device *dev = &mcu->serdev->dev;
+
+	int ret;
+	int len = (int)size;
+
+	if (size > IEI_WT61P803_PUZZLE_MAX_COMMAND_LENGTH)
+		return -EINVAL;
+
+	/* Create an XOR checksum for the provided command */
+	cmd[len - 1] = iei_wt61p803_puzzle_add_xor_checksum(cmd, len);
+
+	mutex_lock(&mcu->bus_lock);
+
+	/* Initialize reply struct */
+	mutex_lock(&mcu->reply_lock);
+	if (mcu->reply) {
+		reinit_completion(&mcu->reply->received);
+
+		mcu->reply->state = FRAME_STRUCT_EMPTY;
+
+		mcu->reply->size = 0;
+	} else {
+		dev_err(dev, "Reply struct is NULL.\n");
+
+		mutex_unlock(&mcu->reply_lock);
+		mutex_unlock(&mcu->bus_lock);
+
+		return -1;
+	}
+	mutex_unlock(&mcu->reply_lock);
+
+	/* Write to MCU UART */
+	ret = serdev_device_write(mcu->serdev, cmd, len,
+			IEI_WT61P803_PUZZLE_GENERAL_TIMEOUT);
+
+	if (ret < 0)
+		return ret;
+
+	/* Wait for the MCU response */
+	if (!wait_for_completion_timeout(&mcu->reply->received,
+				IEI_WT61P803_PUZZLE_GENERAL_TIMEOUT)) {
+		dev_err(dev, "Command reply receive timeout\n");
+
+		ret = -ETIMEDOUT;
+
+		mutex_lock(&mcu->reply_lock);
+		reinit_completion(&mcu->reply->received);
+		mcu->reply->state = FRAME_TIMEOUT;
+		mutex_unlock(&mcu->reply_lock);
+	}
+
+	if (ret < 0) {
+		mutex_unlock(&mcu->bus_lock);
+		return ret;
+	}
+
+	/* Verify MCU response status */
+	mutex_lock(&mcu->reply_lock);
+
+	if (mcu->reply) {
+		/* Copy response */
+		*reply_size = mcu->reply->size;
+		memcpy(reply_data, mcu->reply->data, mcu->reply->size);
+	} else {
+		dev_err(dev, "Reply struct is NULL.\n");
+
+		mutex_unlock(&mcu->reply_lock);
+		mutex_unlock(&mcu->bus_lock);
+
+		return -1;
+	}
+
+	mutex_unlock(&mcu->reply_lock);
+	mutex_unlock(&mcu->bus_lock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(iei_wt61p803_puzzle_write_command);
+
+int iei_wt61p803_puzzle_buzzer(struct iei_wt61p803_puzzle *mcu, bool long_beep)
+{
+	/* Buzzer 0.5 sec */
+	unsigned char buzzer_short_cmd[4] = { '@', 'C', '2' };
+	/* Buzzer 1.5 sec */
+	unsigned char buzzer_long_cmd[4] = { '@', 'C', '3' };
+
+	int ret;
+
+	size_t reply_size = 0;
+	unsigned char *resp_buf;
+
+	resp_buf = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
+	if (!resp_buf)
+		return -ENOMEM;
+
+	ret = iei_wt61p803_puzzle_write_command(mcu,
+			long_beep ? buzzer_long_cmd : buzzer_short_cmd, 4,
+			resp_buf, &reply_size);
+
+	if (ret) {
+		kfree(resp_buf);
+		return ret;
+	}
+
+	if (reply_size != 3) {
+		kfree(resp_buf);
+		return -EIO;
+	}
+
+	if (!(resp_buf[0] == '@' && resp_buf[1] == '0' &&
+				resp_buf[2] == 0x70)) {
+		kfree(resp_buf);
+		return -EPROTO;
+	}
+
+	kfree(resp_buf);
+	return 0;
+}
+
+int iei_wt61p803_puzzle_get_version(struct iei_wt61p803_puzzle *mcu)
+{
+	struct device *dev = &mcu->serdev->dev;
+
+	/* Commands */
+	unsigned char version_cmd[3] = { '%', 'V' };
+	unsigned char build_info_cmd[3] = { '%', 'B' };
+	unsigned char bootloader_mode_cmd[3] = { '%', 'M' };
+	unsigned char protocol_version_cmd[3] = { '%', 'P' };
+
+	unsigned char *rd;
+	size_t reply_size = 0;
+
+	int ret;
+
+	/* Allocate memory for the buffer */
+	rd = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
+	if (!rd)
+		return -ENOMEM;
+
+	ret = iei_wt61p803_puzzle_write_command(mcu, version_cmd,
+			sizeof(version_cmd), rd, &reply_size);
+	if (ret)
+		goto err;
+
+	if (reply_size < 7) {
+		ret = -EIO;
+		goto err;
+	}
+
+	mcu->version->version = devm_kasprintf(dev, GFP_KERNEL, "v%c.%c%c%c",
+				rd[2], rd[3], rd[4], rd[5]);
+
+	ret = iei_wt61p803_puzzle_write_command(mcu, build_info_cmd,
+			sizeof(build_info_cmd), rd, &reply_size);
+	if (ret)
+		goto err;
+
+	if (reply_size < 15) {
+		ret = -EIO;
+		goto err;
+	}
+
+	mcu->version->build_info = devm_kasprintf(dev, GFP_KERNEL,
+			"%c%c/%c%c/%c%c%c%c %c%c:%c%c",
+			rd[8], rd[9], rd[6], rd[7], rd[2],
+			rd[3], rd[4], rd[5], rd[10], rd[11],
+			rd[12], rd[13]);
+
+	ret = iei_wt61p803_puzzle_write_command(mcu, bootloader_mode_cmd,
+			sizeof(bootloader_mode_cmd), rd, &reply_size);
+	if (ret)
+		goto err;
+
+	if (reply_size < 4) {
+		ret = -EIO;
+		goto err;
+	}
+
+	if (rd[2] == 0x31) {
+		/* The MCU is in normal mode */
+		mcu->version->bootloader_mode = false;
+	} else if (rd[2] == 0x30) {
+		/* The MCU is in bootloader mode */
+		mcu->version->bootloader_mode = true;
+	}
+
+	ret = iei_wt61p803_puzzle_write_command(mcu, protocol_version_cmd,
+			sizeof(protocol_version_cmd), rd, &reply_size);
+	if (ret)
+		goto err;
+
+	if (reply_size < 9) {
+		ret = -EIO;
+		goto err;
+	}
+
+	mcu->version->protocol_version = devm_kasprintf(dev, GFP_KERNEL,
+			"v%c.%c%c%c%c%c",
+			rd[7], rd[6], rd[5], rd[4], rd[3], rd[2]);
+
+	/* Free the allocated memory resources */
+	kfree(rd);
+
+	return 0;
+err:
+	kfree(rd);
+	return ret;
+}
+
+int iei_wt61p803_puzzle_get_mcu_status(struct iei_wt61p803_puzzle *mcu)
+{
+	/* MCU Status Command */
+	unsigned char mcu_status_cmd[5] = { '@', 'O', 'S', 'S' };
+
+	int ret;
+
+	unsigned char *rd;
+	size_t reply_size = 0;
+
+	/* Allocate memory for the buffer */
+	rd = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
+	if (!rd)
+		return -ENOMEM;
+
+	ret = iei_wt61p803_puzzle_write_command(mcu, mcu_status_cmd,
+			sizeof(mcu_status_cmd), rd, &reply_size);
+	if (ret) {
+		kfree(rd);
+		return ret;
+	}
+
+	if (reply_size < 20) {
+		kfree(rd);
+		return -EIO;
+	}
+
+	/* Response format:
+	 * (IDX	RESPONSE)
+	 * 0	@
+	 * 1	O
+	 * 2	S
+	 * 3	S
+	 * ...
+	 * 5	AC Recovery Status Flag
+	 * ...
+	 * 10	Power Loss Recovery
+	 * ...
+	 * 19	Power Status (system power on method)
+	 * 20	XOR checksum
+	 */
+
+	if (rd[0] == '@' && rd[1] == 'O' && rd[2] == 'S' && rd[3] == 'S') {
+		mutex_lock(&mcu->status_lock);
+		mcu->status->ac_recovery_status_flag = rd[5];
+		mcu->status->power_loss_recovery = rd[10];
+		mcu->status->power_status = rd[19];
+		mutex_unlock(&mcu->status_lock);
+	}
+
+	/* Free the allocated memory resources */
+	kfree(rd);
+
+	return 0;
+}
+
+int iei_wt61p803_puzzle_get_serial_number(struct iei_wt61p803_puzzle *mcu)
+{
+	struct device *dev = &mcu->serdev->dev;
+	unsigned char serial_number_cmd[5] = { 0xF7, 0xA1, 0x00, 0x24 };
+
+	int ret;
+
+	unsigned char *rd;
+	size_t reply_size = 0;
+
+	/* Allocate memory for the buffer */
+	rd = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
+	if (!rd)
+		return -ENOMEM;
+
+	ret = iei_wt61p803_puzzle_write_command(mcu, serial_number_cmd,
+			sizeof(serial_number_cmd), rd, &reply_size);
+	if (ret)
+		goto err;
+
+	mutex_lock(&mcu->version_lock);
+	mcu->version->serial_number = devm_kasprintf(dev, GFP_KERNEL, "%.*s",
+			(int)reply_size - 5, rd + 4);
+	mutex_unlock(&mcu->version_lock);
+
+	/* Free the allocated memory resources */
+	kfree(rd);
+
+	return 0;
+
+err:
+	kfree(rd);
+	return ret;
+
+}
+
+int iei_wt61p803_puzzle_write_serial_number(struct iei_wt61p803_puzzle *mcu,
+		unsigned char serial_number[36])
+{
+	struct device *dev = &mcu->serdev->dev;
+	unsigned char serial_number_header[4] = { 0xF7, 0xA0, 0x00, 0xC };
+
+	/* 4 byte header, 12 byte serial number chunk, 1 byte XOR checksum */
+	unsigned char serial_number_cmd[4+12+1];
+
+	int ret, sn_counter;
+
+	unsigned char *rd;
+	size_t reply_size = 0;
+
+	/* Allocate memory for the buffer */
+	rd = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
+	if (!rd)
+		return -ENOMEM;
+
+	/* The MCU can only handle 22 byte messages, send the S/N in chunks */
+	for (sn_counter = 0; sn_counter < 3; sn_counter++) {
+		serial_number_header[2] = 0x0 + (0xC) * sn_counter;
+
+		memcpy(serial_number_cmd, serial_number_header, 4);
+		memcpy(serial_number_cmd + 4, serial_number + (0xC) * sn_counter, 0xC);
+
+		/* Initialize the last byte */
+		serial_number_cmd[sizeof(serial_number_cmd) - 1] = 0;
+
+		ret = iei_wt61p803_puzzle_write_command(mcu, serial_number_cmd,
+				sizeof(serial_number_cmd), rd, &reply_size);
+		if (ret)
+			goto err;
+
+		if (reply_size != 3) {
+			ret = -EIO;
+			goto err;
+		}
+
+		if (!(rd[0] == '@' && rd[1] == '0' && rd[2] == 0x70)) {
+			ret = -EPROTO;
+			goto err;
+		}
+	}
+
+	mutex_lock(&mcu->version_lock);
+	mcu->version->serial_number = devm_kasprintf(dev, GFP_KERNEL, "%.*s",
+			36, serial_number);
+	mutex_unlock(&mcu->version_lock);
+
+	/* Free the allocated memory resources */
+	kfree(rd);
+
+	return 0;
+
+err:
+	kfree(rd);
+	return ret;
+}
+
+int iei_wt61p803_puzzle_get_mac_addresses(struct iei_wt61p803_puzzle *mcu)
+{
+	struct device *dev = &mcu->serdev->dev;
+	unsigned char mac_address_cmd[5] = { 0xF7, 0xA1, 0x00, 0x11 };
+
+	int ret, mac_counter;
+
+	unsigned char *rd;
+	size_t reply_size = 0;
+
+	/* Allocate memory for the buffer */
+	rd = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
+	if (!rd)
+		return -ENOMEM;
+
+	for (mac_counter = 0; mac_counter < 8; mac_counter++) {
+		mac_address_cmd[2] = 0x24 + (0x11) * mac_counter;
+		mac_address_cmd[4] = 0x00;
+
+		ret = iei_wt61p803_puzzle_write_command(mcu, mac_address_cmd,
+				sizeof(mac_address_cmd), rd, &reply_size);
+		if (ret)
+			continue;
+
+		if (reply_size < 22) {
+			ret = -EIO;
+			goto err;
+		}
+
+		mutex_lock(&mcu->version_lock);
+		mcu->version->mac_address[mac_counter] = devm_kasprintf(dev,
+				GFP_KERNEL, "%.*s", (int)reply_size - 5,
+				rd + 4);
+		mutex_unlock(&mcu->version_lock);
+	}
+
+	/* Free the allocated memory resources */
+	kfree(rd);
+
+	return 0;
+
+err:
+	kfree(rd);
+	return ret;
+}
+
+int iei_wt61p803_puzzle_write_mac_address(struct iei_wt61p803_puzzle *mcu,
+		unsigned char mac_address[17], int mac_address_idx)
+{
+	struct device *dev = &mcu->serdev->dev;
+	unsigned char mac_address_header[4] = { 0xF7, 0xA0, 0x00, 0x11 };
+
+	/* 4 byte header, 17 byte MAC address, 1 byte XOR checksum */
+	unsigned char mac_address_cmd[4+17+1];
+
+	int ret;
+
+	unsigned char *rd;
+	size_t reply_size = 0;
+
+	if (!(mac_address_idx < 8))
+		return -EINVAL;
+
+	/* Allocate memory for the buffer */
+	rd = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
+	if (!rd)
+		return -ENOMEM;
+
+	mac_address_header[2] = 0x24 + (0x11) * mac_address_idx;
+
+	/* Concat mac_address_header, mac_address to mac_address_cmd */
+	memcpy(mac_address_cmd, mac_address_header, 4);
+	memcpy(mac_address_cmd + 4, mac_address, 17);
+
+	/* Initialize the last byte */
+	mac_address_cmd[sizeof(mac_address_cmd) - 1] = 0;
+
+	ret = iei_wt61p803_puzzle_write_command(mcu, mac_address_cmd,
+			sizeof(mac_address_cmd), rd, &reply_size);
+	if (ret)
+		goto err;
+
+	if (reply_size != 3) {
+		ret = -EIO;
+		goto err;
+	}
+
+	if (!(rd[0] == '@' && rd[1] == '0' && rd[2] == 0x70)) {
+		ret = -EPROTO;
+		goto err;
+	}
+
+	mutex_lock(&mcu->version_lock);
+	mcu->version->mac_address[mac_address_idx] = devm_kasprintf(dev,
+			GFP_KERNEL, "%.*s", 17, mac_address);
+	mutex_unlock(&mcu->version_lock);
+
+	/* Free the allocated memory resources */
+	kfree(rd);
+
+	return 0;
+
+err:
+	kfree(rd);
+	return ret;
+}
+
+int iei_wt61p803_puzzle_write_power_loss_recovery(struct iei_wt61p803_puzzle *mcu,
+		int power_loss_recovery_action)
+{
+	unsigned char power_loss_recovery_cmd[5] = { '@', 'O', 'A', '0' };
+
+	unsigned char *resp_buf;
+	size_t reply_size = 0;
+
+	/* Buffer for the power_loss_recovery_action to character */
+	unsigned char cmd_buf[2];
+
+	int ret;
+
+	/* Allocate memory for the buffer */
+	resp_buf = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
+	if (!resp_buf)
+		return -ENOMEM;
+
+	/* Check the acceptable range */
+	if (power_loss_recovery_action < 0 || power_loss_recovery_action > 4) {
+		kfree(resp_buf);
+		return -EINVAL;
+	}
+
+	/* Convert int to char */
+	ret = snprintf(cmd_buf, sizeof(cmd_buf), "%d",
+			power_loss_recovery_action);
+	if (ret < 0) {
+		kfree(resp_buf);
+		return ret;
+	}
+
+	/* Modify the command with the action index */
+	power_loss_recovery_cmd[3] = cmd_buf[0];
+
+	ret = iei_wt61p803_puzzle_write_command(mcu, power_loss_recovery_cmd,
+			sizeof(power_loss_recovery_cmd), resp_buf, &reply_size);
+	if (ret) {
+		kfree(resp_buf);
+		return ret;
+	}
+
+	/* Update the internal status (struct) */
+	mutex_lock(&mcu->status_lock);
+	mcu->status->power_loss_recovery = power_loss_recovery_action;
+	mutex_unlock(&mcu->status_lock);
+
+	/* Free the allocated memory resources */
+	kfree(resp_buf);
+
+	return 0;
+}
+
+
+#define sysfs_container(dev) \
+	(container_of((dev)->kobj.parent, struct device, kobj))
+
+static ssize_t version_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct device *dev_container = sysfs_container(dev);
+	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
+
+	return sprintf(buf, "%s\n", mcu->version->version);
+}
+
+static DEVICE_ATTR_RO(version);
+
+static ssize_t build_info_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct device *dev_container = sysfs_container(dev);
+	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
+
+	return sprintf(buf, "%s\n", mcu->version->build_info);
+}
+
+static DEVICE_ATTR_RO(build_info);
+
+static ssize_t bootloader_mode_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct device *dev_container = sysfs_container(dev);
+	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
+
+	return sprintf(buf, "%d\n", mcu->version->bootloader_mode);
+}
+
+static DEVICE_ATTR_RO(bootloader_mode);
+
+static ssize_t protocol_version_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct device *dev_container = sysfs_container(dev);
+	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
+
+	return sprintf(buf, "%s\n", mcu->version->protocol_version);
+}
+
+static DEVICE_ATTR_RO(protocol_version);
+
+static ssize_t serial_number_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct device *dev_container = sysfs_container(dev);
+	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
+
+	int ret;
+
+	mutex_lock(&mcu->version_lock);
+	ret = sprintf(buf, "%s\n", mcu->version->serial_number);
+	mutex_unlock(&mcu->version_lock);
+
+	return ret;
+}
+
+static ssize_t serial_number_store(struct device *dev,
+		struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	struct device *dev_container = sysfs_container(dev);
+	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
+
+	unsigned char serial_number[36];
+	int ret;
+
+	/* Check whether the serial number is 36 characters long + null */
+	if ((int)count != 36 + 1)
+		return -EINVAL;
+
+	/* Copy 36 characters from the buffer to serial_number */
+	memcpy(serial_number, (unsigned char *)buf, 36);
+
+	ret = iei_wt61p803_puzzle_write_serial_number(mcu, serial_number);
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(serial_number);
+
+static ssize_t mac_address_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct device *dev_container = sysfs_container(dev);
+	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
+
+	int ret;
+
+	mutex_lock(&mcu->version_lock);
+
+	if (!strcmp(attr->attr.name, "mac_address_0"))
+		ret = sprintf(buf, "%s\n", mcu->version->mac_address[0]);
+	else if (!strcmp(attr->attr.name, "mac_address_1"))
+		ret = sprintf(buf, "%s\n", mcu->version->mac_address[1]);
+	else if (!strcmp(attr->attr.name, "mac_address_2"))
+		ret = sprintf(buf, "%s\n", mcu->version->mac_address[2]);
+	else if (!strcmp(attr->attr.name, "mac_address_3"))
+		ret = sprintf(buf, "%s\n", mcu->version->mac_address[3]);
+	else if (!strcmp(attr->attr.name, "mac_address_4"))
+		ret = sprintf(buf, "%s\n", mcu->version->mac_address[4]);
+	else if (!strcmp(attr->attr.name, "mac_address_5"))
+		ret = sprintf(buf, "%s\n", mcu->version->mac_address[5]);
+	else if (!strcmp(attr->attr.name, "mac_address_6"))
+		ret = sprintf(buf, "%s\n", mcu->version->mac_address[6]);
+	else if (!strcmp(attr->attr.name, "mac_address_7"))
+		ret = sprintf(buf, "%s\n", mcu->version->mac_address[7]);
+	else
+		ret = sprintf(buf, "\n");
+
+	mutex_unlock(&mcu->version_lock);
+
+	return ret;
+}
+
+static ssize_t mac_address_store(struct device *dev,
+		struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	struct device *dev_container = sysfs_container(dev);
+	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
+
+	unsigned char mac_address[17];
+	int ret;
+
+	/* Check whether the MAC address is 17 characters long + null */
+	if ((int)count != 17 + 1)
+		return -EINVAL;
+
+	/* Copy 17 characters from the buffer to mac_address */
+	memcpy(mac_address, (unsigned char *)buf, 17);
+
+	if (!strcmp(attr->attr.name, "mac_address_0"))
+		ret = iei_wt61p803_puzzle_write_mac_address(mcu, mac_address, 0);
+	else if (!strcmp(attr->attr.name, "mac_address_1"))
+		ret = iei_wt61p803_puzzle_write_mac_address(mcu, mac_address, 1);
+	else if (!strcmp(attr->attr.name, "mac_address_2"))
+		ret = iei_wt61p803_puzzle_write_mac_address(mcu, mac_address, 2);
+	else if (!strcmp(attr->attr.name, "mac_address_3"))
+		ret = iei_wt61p803_puzzle_write_mac_address(mcu, mac_address, 3);
+	else if (!strcmp(attr->attr.name, "mac_address_4"))
+		ret = iei_wt61p803_puzzle_write_mac_address(mcu, mac_address, 4);
+	else if (!strcmp(attr->attr.name, "mac_address_5"))
+		ret = iei_wt61p803_puzzle_write_mac_address(mcu, mac_address, 5);
+	else if (!strcmp(attr->attr.name, "mac_address_6"))
+		ret = iei_wt61p803_puzzle_write_mac_address(mcu, mac_address, 6);
+	else if (!strcmp(attr->attr.name, "mac_address_7"))
+		ret = iei_wt61p803_puzzle_write_mac_address(mcu, mac_address, 7);
+
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+static DEVICE_ATTR(mac_address_0, 0644, mac_address_show, mac_address_store);
+static DEVICE_ATTR(mac_address_1, 0644, mac_address_show, mac_address_store);
+static DEVICE_ATTR(mac_address_2, 0644, mac_address_show, mac_address_store);
+static DEVICE_ATTR(mac_address_3, 0644, mac_address_show, mac_address_store);
+static DEVICE_ATTR(mac_address_4, 0644, mac_address_show, mac_address_store);
+static DEVICE_ATTR(mac_address_5, 0644, mac_address_show, mac_address_store);
+static DEVICE_ATTR(mac_address_6, 0644, mac_address_show, mac_address_store);
+static DEVICE_ATTR(mac_address_7, 0644, mac_address_show, mac_address_store);
+
+static ssize_t ac_recovery_status_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct device *dev_container = sysfs_container(dev);
+	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
+
+	int ret;
+
+	ret = iei_wt61p803_puzzle_get_mcu_status(mcu);
+	if (ret)
+		return ret;
+
+	mutex_lock(&mcu->status_lock);
+	ret = sprintf(buf, "%x\n", mcu->status->ac_recovery_status_flag);
+	mutex_unlock(&mcu->status_lock);
+
+	return ret;
+}
+
+static DEVICE_ATTR_RO(ac_recovery_status);
+
+static ssize_t power_loss_recovery_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct device *dev_container = sysfs_container(dev);
+	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
+
+	int ret;
+
+	ret = iei_wt61p803_puzzle_get_mcu_status(mcu);
+	if (ret)
+		return ret;
+
+	mutex_lock(&mcu->status_lock);
+	ret = sprintf(buf, "%x\n", mcu->status->power_loss_recovery);
+	mutex_unlock(&mcu->status_lock);
+
+	return ret;
+}
+
+static ssize_t power_loss_recovery_store(struct device *dev,
+		struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	struct device *dev_container = sysfs_container(dev);
+	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
+
+	int ret;
+	long power_loss_recovery_action = 0;
+
+	ret = kstrtol(buf, 10, &power_loss_recovery_action);
+	if (ret)
+		return ret;
+
+	ret = iei_wt61p803_puzzle_write_power_loss_recovery(mcu,
+			(int)power_loss_recovery_action);
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(power_loss_recovery);
+
+static ssize_t power_status_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct device *dev_container = sysfs_container(dev);
+	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
+
+	int ret;
+
+	ret = iei_wt61p803_puzzle_get_mcu_status(mcu);
+	if (ret)
+		return ret;
+
+	mutex_lock(&mcu->status_lock);
+	ret = sprintf(buf, "%x\n", mcu->status->power_status);
+	mutex_unlock(&mcu->status_lock);
+
+	return ret;
+}
+
+static DEVICE_ATTR_RO(power_status);
+
+static struct attribute *iei_wt61p803_puzzle_attrs[] = {
+	&dev_attr_version.attr,
+	&dev_attr_build_info.attr,
+	&dev_attr_bootloader_mode.attr,
+	&dev_attr_protocol_version.attr,
+	&dev_attr_serial_number.attr,
+	&dev_attr_mac_address_0.attr,
+	&dev_attr_mac_address_1.attr,
+	&dev_attr_mac_address_2.attr,
+	&dev_attr_mac_address_3.attr,
+	&dev_attr_mac_address_4.attr,
+	&dev_attr_mac_address_5.attr,
+	&dev_attr_mac_address_6.attr,
+	&dev_attr_mac_address_7.attr,
+	&dev_attr_ac_recovery_status.attr,
+	&dev_attr_power_loss_recovery.attr,
+	&dev_attr_power_status.attr,
+	NULL
+};
+
+ATTRIBUTE_GROUPS(iei_wt61p803_puzzle);
+
+static int iei_wt61p803_puzzle_sysfs_create(struct device *dev,
+		struct iei_wt61p803_puzzle *mcu)
+{
+	int ret;
+
+	mcu->kobj =
+		kobject_create_and_add("iei_wt61p803_puzzle_core", &dev->kobj);
+	if (!mcu->kobj)
+		return -ENOMEM;
+
+	ret = sysfs_create_groups(mcu->kobj, iei_wt61p803_puzzle_groups);
+	if (ret) {
+		dev_err(dev, "sysfs creation failed");
+
+		/* Clean up */
+		kobject_del(mcu->kobj);
+		kobject_put(mcu->kobj);
+		mcu->kobj = NULL;
+
+		return ret;
+	}
+
+	return 0;
+}
+
+static int iei_wt61p803_puzzle_sysfs_remove(struct device *dev,
+		struct iei_wt61p803_puzzle *mcu)
+{
+	/* Remove sysfs groups */
+	sysfs_remove_groups(mcu->kobj, iei_wt61p803_puzzle_groups);
+
+	/* Remove the kobject */
+	kobject_del(mcu->kobj);
+	kobject_put(mcu->kobj);
+	mcu->kobj = NULL;
+
+	return 0;
+}
+
+static int iei_wt61p803_puzzle_probe(struct serdev_device *serdev)
+{
+	struct device *dev = &serdev->dev;
+	struct iei_wt61p803_puzzle *mcu;
+	u32 baud;
+	int ret;
+
+	/* Read the current-speed property from the device tree */
+	if (of_property_read_u32(dev->of_node, "current-speed", &baud)) {
+		dev_err(dev,
+			"'current-speed' is not specified in device node\n");
+		return -EINVAL;
+	}
+
+	/* Log the specified BAUD RATE */
+	dev_info(dev, "Driver baud rate: %d", baud);
+
+	/* Allocate the memory */
+	mcu = devm_kzalloc(dev, sizeof(*mcu), GFP_KERNEL);
+	if (!mcu)
+		return -ENOMEM;
+
+	mcu->version = devm_kzalloc(dev, sizeof(*mcu->version), GFP_KERNEL);
+	if (!mcu->version)
+		return -ENOMEM;
+
+	mcu->status = devm_kzalloc(dev, sizeof(*mcu->version), GFP_KERNEL);
+	if (!mcu->status)
+		return -ENOMEM;
+
+	mcu->reply = devm_kzalloc(dev, sizeof(*mcu->reply), GFP_KERNEL);
+	if (!mcu->reply)
+		return -ENOMEM;
+
+	mcu->reply->data = devm_kzalloc(dev, IEI_WT61P803_PUZZLE_RESP_BUF_SIZE,
+			GFP_KERNEL);
+	if (!mcu->reply->data)
+		return -ENOMEM;
+
+	/* Initialize device struct data */
+	mcu->serdev = serdev;
+
+	mcu->status->ac_recovery_status_flag = 0x00;
+	mcu->status->power_loss_recovery = 0x00;
+	mcu->status->power_status = 0x00;
+
+	init_completion(&mcu->reply->received);
+
+	/* Mutexes */
+	mutex_init(&mcu->reply_lock);
+	mutex_init(&mcu->bus_lock);
+	mutex_init(&mcu->status_lock);
+	mutex_init(&mcu->version_lock);
+
+	/* Setup UART interface */
+	serdev_device_set_drvdata(serdev, mcu);
+	serdev_device_set_client_ops(serdev,
+			&iei_wt61p803_puzzle_serdev_device_ops);
+
+	ret = devm_serdev_device_open(dev, serdev);
+	if (ret)
+		return ret;
+
+	serdev_device_set_baudrate(serdev, baud);
+
+	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 = iei_wt61p803_puzzle_get_version(mcu);
+	if (ret)
+		return ret;
+
+	ret = iei_wt61p803_puzzle_get_mac_addresses(mcu);
+	if (ret)
+		return ret;
+
+	ret = iei_wt61p803_puzzle_get_serial_number(mcu);
+	if (ret)
+		return ret;
+
+	dev_info(dev, "MCU version: %s", mcu->version->version);
+
+	dev_info(dev, "MCU firmware build info: %s", mcu->version->build_info);
+	dev_info(dev, "MCU in bootloader mode: %s",
+			mcu->version->bootloader_mode ? "true" : "false");
+	dev_info(dev, "MCU protocol version: %s", mcu->version->protocol_version);
+
+	if (of_property_read_bool(dev->of_node, "enable-probe-beep")) {
+		ret = iei_wt61p803_puzzle_buzzer(mcu, false);
+		if (ret)
+			return ret;
+	}
+
+	ret = iei_wt61p803_puzzle_sysfs_create(dev, mcu);
+	if (ret)
+		return ret;
+
+	return devm_of_platform_populate(dev);
+}
+
+static void iei_wt61p803_puzzle_remove(struct serdev_device *serdev)
+{
+	struct device *dev = &serdev->dev;
+	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev);
+
+	/* Remove sysfs kobjects and attributes */
+	iei_wt61p803_puzzle_sysfs_remove(dev, mcu);
+
+	dev_info(dev, "Device unregistered");
+}
+
+
+MODULE_DEVICE_TABLE(of, iei_wt61p803_puzzle_dt_ids);
+
+static struct serdev_device_driver iei_wt61p803_puzzle_drv = {
+	.probe			= iei_wt61p803_puzzle_probe,
+	.remove			= iei_wt61p803_puzzle_remove,
+	.driver = {
+		.name		= "iei-wt61p803-puzzle",
+		.of_match_table	= iei_wt61p803_puzzle_dt_ids,
+	},
+};
+
+module_serdev_device_driver(iei_wt61p803_puzzle_drv);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Luka Kovacic <luka.kovacic@sartura.hr>");
+MODULE_DESCRIPTION("iEi WT61P803 PUZZLE MCU Driver");
diff --git a/include/linux/mfd/iei-wt61p803-puzzle.h b/include/linux/mfd/iei-wt61p803-puzzle.h
new file mode 100644
index 000000000000..2bb6256574bc
--- /dev/null
+++ b/include/linux/mfd/iei-wt61p803-puzzle.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+/* iEi WT61P803 PUZZLE MCU MFD Driver
+ *
+ * Copyright (C) 2020 Sartura Ltd.
+ * Author: Luka Kovacic <luka.kovacic@sartura.hr>
+ */
+
+#ifndef _LINUX_IEI_WT61P803_PUZZLE_H_
+#define _LINUX_IEI_WT61P803_PUZZLE_H_
+
+#define IEI_WT61P803_PUZZLE_BUF_SIZE 512
+
+struct iei_wt61p803_puzzle_mcu_version;
+struct iei_wt61p803_puzzle_reply;
+struct iei_wt61p803_puzzle;
+
+int iei_wt61p803_puzzle_write_command_watchdog(struct iei_wt61p803_puzzle *mcu,
+		unsigned char *cmd, size_t size,
+		unsigned char *reply_data, size_t *reply_size,
+		int retry_count);
+
+int iei_wt61p803_puzzle_write_command(struct iei_wt61p803_puzzle *mcu,
+		unsigned char *cmd, size_t size,
+		unsigned char *reply_data, size_t *reply_size);
+
+#endif /* _LINUX_IEI_WT61P803_PUZZLE_H_ */
-- 
2.20.1

[PATCH 3/7] drivers: hwmon: Add the iEi WT61P803 PUZZLE HWMON driver

[Luka Kovacic]

Add the iEi WT61P803 PUZZLE HWMON driver, that handles the fan speed
control via PWM, reading fan speed and reading on-board temperature
sensors.

The driver registers a HWMON device and a simple thermal cooling device to
enable in-kernel fan management.

This driver depends on the iEi WT61P803 PUZZLE MFD driver.

Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
Cc: Luka Perkov <luka.perkov@sartura.hr>
---
 drivers/hwmon/Kconfig                     |   8 +
 drivers/hwmon/Makefile                    |   1 +
 drivers/hwmon/iei-wt61p803-puzzle-hwmon.c | 613 ++++++++++++++++++++++
 3 files changed, 622 insertions(+)
 create mode 100644 drivers/hwmon/iei-wt61p803-puzzle-hwmon.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 8dc28b26916e..ff279df9bf40 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -722,6 +722,14 @@ config SENSORS_IBMPOWERNV
 	  This driver can also be built as a module. If so, the module
 	  will be called ibmpowernv.
 
+config SENSORS_IEI_WT61P803_PUZZLE_HWMON
+	tristate "iEi WT61P803 PUZZLE MFD HWMON Driver"
+	depends on MFD_IEI_WT61P803_PUZZLE
+	help
+	  The iEi WT61P803 PUZZLE MFD HWMON Driver handles reading fan speed
+	  and writing fan PWM values. It also supports reading on-board
+	  temperature sensors.
+
 config SENSORS_IIO_HWMON
 	tristate "Hwmon driver that uses channels specified via iio maps"
 	depends on IIO
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index a8f4b35b136b..b0afb2d6896f 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -83,6 +83,7 @@ obj-$(CONFIG_SENSORS_HIH6130)	+= hih6130.o
 obj-$(CONFIG_SENSORS_ULTRA45)	+= ultra45_env.o
 obj-$(CONFIG_SENSORS_I5500)	+= i5500_temp.o
 obj-$(CONFIG_SENSORS_I5K_AMB)	+= i5k_amb.o
+obj-$(CONFIG_SENSORS_IEI_WT61P803_PUZZLE_HWMON) += iei-wt61p803-puzzle-hwmon.o
 obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
 obj-$(CONFIG_SENSORS_IBMPEX)	+= ibmpex.o
 obj-$(CONFIG_SENSORS_IBMPOWERNV)+= ibmpowernv.o
diff --git a/drivers/hwmon/iei-wt61p803-puzzle-hwmon.c b/drivers/hwmon/iei-wt61p803-puzzle-hwmon.c
new file mode 100644
index 000000000000..ca26d0cc6884
--- /dev/null
+++ b/drivers/hwmon/iei-wt61p803-puzzle-hwmon.c
@@ -0,0 +1,613 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/* iEi WT61P803 PUZZLE MCU HWMON Driver
+ *
+ * Copyright (C) 2020 Sartura Ltd.
+ * Author: Luka Kovacic <luka.kovacic@sartura.hr>
+ */
+
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/iei-wt61p803-puzzle.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/math64.h>
+#include <linux/err.h>
+
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/thermal.h>
+
+#define IEI_WT61P803_PUZZLE_HWMON_MAX_TEMP_NUM 2
+#define IEI_WT61P803_PUZZLE_HWMON_MAX_FAN_NUM 5
+#define IEI_WT61P803_PUZZLE_HWMON_MAX_PWM_NUM 2
+#define IEI_WT61P803_PUZZLE_HWMON_MAX_PWM_VAL 255
+
+/**
+ * struct iei_wt61p803_puzzle_thermal_cooling_device - Thermal cooling device instance
+ *
+ * @mcu_hwmon: MCU HWMON struct pointer
+ * @tcdev: Thermal cooling device pointer
+ * @name: Thermal cooling device name
+ * @pwm_channel: PWM channel (0 or 1)
+ * @cooling_levels: Thermal cooling device cooling levels
+ */
+struct iei_wt61p803_puzzle_thermal_cooling_device {
+	struct iei_wt61p803_puzzle_hwmon *mcu_hwmon;
+	struct thermal_cooling_device *tcdev;
+	char name[THERMAL_NAME_LENGTH];
+	int pwm_channel;
+	u8 *cooling_levels;
+};
+
+/**
+ * struct iei_wt61p803_puzzle_hwmon - MCU HWMON Driver
+ *
+ * @mcu: MCU struct pointer
+ * @temp_lock: Mutex for temp_sensor_val
+ * @temp_sensor_val: Temperature sensor values
+ * @fan_lock: Mutex for fan_speed_val
+ * @fan_speed_val: FAN speed (RPM) values
+ * @pwm_lock: Mutex for pwm_val
+ * @pwm_val: PWM values (0-255)
+ * @thermal_cooling_dev_lock: Mutex for Thermal Framework related members
+ * @thermal_cooling_dev_present: Per-channel thermal cooling device control
+ * @cdev: Per-channel thermal cooling device private structure
+ */
+struct iei_wt61p803_puzzle_hwmon {
+	struct iei_wt61p803_puzzle *mcu;
+
+	struct mutex temp_lock;
+	int temp_sensor_val[IEI_WT61P803_PUZZLE_HWMON_MAX_TEMP_NUM];
+
+	struct mutex fan_lock;
+	int fan_speed_val[IEI_WT61P803_PUZZLE_HWMON_MAX_FAN_NUM];
+
+	struct mutex pwm_lock;
+	int pwm_val[IEI_WT61P803_PUZZLE_HWMON_MAX_PWM_NUM];
+
+	struct mutex thermal_cooling_dev_lock;
+	bool thermal_cooling_dev_present[IEI_WT61P803_PUZZLE_HWMON_MAX_PWM_NUM];
+	struct iei_wt61p803_puzzle_thermal_cooling_device
+		*cdev[IEI_WT61P803_PUZZLE_HWMON_MAX_PWM_NUM];
+};
+
+/*
+ * Generic MCU access functions
+ *
+ * Description: The functions below are used as generic translation functions
+ * between the kernel and the MCU hardware. These can be used from HWMON or
+ * from the Thermal Framework.
+ */
+
+#define raw_temp_to_milidegree_celsius(x) ((int)((x - 0x80)*1000))
+static int iei_wt61p803_puzzle_read_temp_sensor
+(struct iei_wt61p803_puzzle_hwmon *mcu_hwmon, int channel, int *value)
+{
+	int ret;
+	size_t reply_size = 0;
+	unsigned char *resp_buf;
+
+	/* MCU Command: Retrieve all available NTC values */
+	unsigned char temp_sensor_ntc_cmd[4] = { '@', 'T', 'A' };
+
+	if (channel > 1 && channel < 0)
+		return -EINVAL;
+
+	resp_buf = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
+	if (!resp_buf)
+		return -ENOMEM;
+
+	/* Write the command to the MCU */
+	ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu,
+			temp_sensor_ntc_cmd, sizeof(temp_sensor_ntc_cmd),
+			resp_buf, &reply_size);
+	if (!ret) {
+		/* Check the number of NTC values (should be 0x32/'2') */
+		if (resp_buf[3] == 0x32) {
+			mutex_lock(&mcu_hwmon->temp_lock);
+
+			/* Write values to the struct */
+			mcu_hwmon->temp_sensor_val[0] =
+				raw_temp_to_milidegree_celsius(resp_buf[4]);
+			mcu_hwmon->temp_sensor_val[1] =
+				raw_temp_to_milidegree_celsius(resp_buf[5]);
+
+			mutex_unlock(&mcu_hwmon->temp_lock);
+		}
+
+	}
+
+	kfree(resp_buf);
+
+	mutex_lock(&mcu_hwmon->temp_lock);
+	*value = mcu_hwmon->temp_sensor_val[channel];
+	mutex_unlock(&mcu_hwmon->temp_lock);
+
+	return ret;
+}
+
+#define raw_fan_val_to_rpm(x, y) ((int)(((x)<<8|(y))/2)*60)
+static int iei_wt61p803_puzzle_read_fan_speed
+(struct iei_wt61p803_puzzle_hwmon *mcu_hwmon, int channel, int *value)
+{
+	int ret;
+	size_t reply_size = 0;
+	unsigned char *resp_buf;
+
+	/* MCU Command: Retrieve fan speed value */
+	unsigned char fan_speed_cmd[4] = { '@', 'F', 'A' };
+
+	resp_buf = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
+	if (!resp_buf)
+		return -ENOMEM;
+
+	switch (channel) {
+	case 0:
+		fan_speed_cmd[2] = 'A';
+		break;
+	case 1:
+		fan_speed_cmd[2] = 'B';
+		break;
+	case 2:
+		fan_speed_cmd[2] = 'C';
+		break;
+	case 3:
+		fan_speed_cmd[2] = 'D';
+		break;
+	case 4:
+		fan_speed_cmd[2] = 'E';
+		break;
+	default:
+		kfree(resp_buf);
+		return -EINVAL;
+	}
+
+	/* Write the command to the MCU */
+	ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, fan_speed_cmd,
+			sizeof(fan_speed_cmd), resp_buf, &reply_size);
+	if (!ret) {
+		mutex_lock(&mcu_hwmon->fan_lock);
+
+		/* Calculate fan RPM */
+		mcu_hwmon->fan_speed_val[channel] = raw_fan_val_to_rpm(resp_buf[3],
+				resp_buf[4]);
+
+		mutex_unlock(&mcu_hwmon->fan_lock);
+	}
+
+	kfree(resp_buf);
+
+	mutex_lock(&mcu_hwmon->fan_lock);
+	*value = mcu_hwmon->fan_speed_val[channel];
+	mutex_unlock(&mcu_hwmon->fan_lock);
+
+	return 0;
+}
+
+static int iei_wt61p803_puzzle_write_pwm_channel
+(struct iei_wt61p803_puzzle_hwmon *mcu_hwmon, int channel, long pwm_set_val)
+{
+	int ret;
+	size_t reply_size = 0;
+	unsigned char *resp_buf;
+
+	/* MCU Command: Set PWM value (Default channel is 0/0x30 at index 3) */
+	unsigned char pwm_set_cmd[6] = { '@', 'F', 'W', 0x30, 0x00 };
+
+	resp_buf = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
+	if (!resp_buf)
+		return -ENOMEM;
+
+	/* Determine the PWM channel */
+	switch (channel) {
+	case 0:
+		pwm_set_cmd[3] = 0x30;
+		break;
+	case 1:
+		pwm_set_cmd[3] = 0x31;
+		break;
+	default:
+		kfree(resp_buf);
+		return -EINVAL;
+	}
+
+	if (pwm_set_val < 0 || pwm_set_val > IEI_WT61P803_PUZZLE_HWMON_MAX_PWM_VAL) {
+		kfree(resp_buf);
+		return -EINVAL;
+	}
+
+	/* Add the value to the command */
+	pwm_set_cmd[4] = (char)pwm_set_val;
+
+	/* Write the command to the MCU */
+	ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, pwm_set_cmd,
+			sizeof(pwm_set_cmd), resp_buf, &reply_size);
+	if (!ret) {
+		/* Store the PWM value */
+		if (resp_buf[0] == '@' && resp_buf[1] == 0x30) {
+			mutex_lock(&mcu_hwmon->pwm_lock);
+			mcu_hwmon->pwm_val[channel] = (int)pwm_set_val;
+			mutex_unlock(&mcu_hwmon->pwm_lock);
+		}
+	}
+
+	kfree(resp_buf);
+	return 0;
+}
+
+static int iei_wt61p803_puzzle_read_pwm_channel
+(struct iei_wt61p803_puzzle_hwmon *mcu_hwmon, int channel, int *value)
+{
+	int ret;
+	size_t reply_size = 0;
+	unsigned char *resp_buf;
+
+	/* MCU Command: Retrieve PWM value (Default channel: 0x30 at idx 3) */
+	unsigned char pwm_get_cmd[5] = { '@', 'F', 'Z', 0x30 };
+
+	resp_buf = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
+	if (!resp_buf)
+		return -ENOMEM;
+
+	/* Determine the PWM channel */
+	switch (channel) {
+	case 0:
+		pwm_get_cmd[3] = 0x30;
+		break;
+	case 1:
+		pwm_get_cmd[3] = 0x31;
+		break;
+	default:
+		kfree(resp_buf);
+		return -EINVAL;
+	}
+
+	/* Write the command to the MCU */
+	ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, pwm_get_cmd,
+			sizeof(pwm_get_cmd), resp_buf, &reply_size);
+	if (!ret) {
+		/* Store the PWM value */
+		if (resp_buf[2] == 'Z') {
+			mutex_lock(&mcu_hwmon->pwm_lock);
+			mcu_hwmon->pwm_val[channel] = (int)resp_buf[3];
+			mutex_unlock(&mcu_hwmon->pwm_lock);
+		}
+	}
+
+	kfree(resp_buf);
+
+	mutex_lock(&mcu_hwmon->pwm_lock);
+	*value = mcu_hwmon->pwm_val[channel];
+	mutex_unlock(&mcu_hwmon->pwm_lock);
+
+	return 0;
+}
+
+/*
+ * HWMON attributes
+ *
+ * Description: The attributes below are registered with the HWMON subsystem.
+ * Fans can only be controlled, if they are not controlled by the Thermal
+ * Framework.
+ */
+
+static ssize_t temp_input_show(struct device *dev,
+			struct device_attribute *attr, char *buf)
+{
+	struct iei_wt61p803_puzzle_hwmon *mcu_hwmon =
+		dev_get_drvdata(dev->parent);
+	struct sensor_device_attribute *sensor_dev_attr =
+		to_sensor_dev_attr(attr);
+	int nr = sensor_dev_attr->index;
+
+	int ret, value;
+
+	ret = iei_wt61p803_puzzle_read_temp_sensor(mcu_hwmon, nr, &value);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%d\n", value);
+}
+
+static SENSOR_DEVICE_ATTR_RO(temp1_input, temp_input, 0);
+static SENSOR_DEVICE_ATTR_RO(temp2_input, temp_input, 1);
+
+static ssize_t fan_input_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct iei_wt61p803_puzzle_hwmon *mcu_hwmon =
+		dev_get_drvdata(dev->parent);
+	struct sensor_device_attribute *sensor_dev_attr =
+		to_sensor_dev_attr(attr);
+	int nr = sensor_dev_attr->index;
+
+	int ret, value;
+
+	ret = iei_wt61p803_puzzle_read_fan_speed(mcu_hwmon, nr, &value);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%d\n", value);
+}
+
+static SENSOR_DEVICE_ATTR_RO(fan1_input, fan_input, 0);
+static SENSOR_DEVICE_ATTR_RO(fan2_input, fan_input, 1);
+static SENSOR_DEVICE_ATTR_RO(fan3_input, fan_input, 2);
+static SENSOR_DEVICE_ATTR_RO(fan4_input, fan_input, 3);
+static SENSOR_DEVICE_ATTR_RO(fan5_input, fan_input, 4);
+
+static ssize_t pwm_store(struct device *dev, struct device_attribute *attr,
+			const char *buf, size_t count)
+{
+	struct iei_wt61p803_puzzle_hwmon *mcu_hwmon =
+		dev_get_drvdata(dev->parent);
+	struct sensor_device_attribute *sensor_dev_attr =
+		to_sensor_dev_attr(attr);
+	int nr = sensor_dev_attr->index;
+
+	int ret;
+	long pwm_value;
+
+	if (mcu_hwmon->thermal_cooling_dev_present[nr]) {
+		/*
+		 * The Thermal Framework has already claimed this specific PWM
+		 * channel. Return 0, to indicate 0 bytes written.
+		 */
+
+		return 0;
+	}
+
+	/* Convert the string to a long */
+	ret = kstrtol(buf, 10, &pwm_value);
+	if (ret)
+		return ret;
+
+	ret = iei_wt61p803_puzzle_write_pwm_channel(mcu_hwmon, nr, pwm_value);
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+static ssize_t pwm_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct iei_wt61p803_puzzle_hwmon *mcu_hwmon =
+		dev_get_drvdata(dev->parent);
+	struct sensor_device_attribute *sensor_dev_attr =
+		to_sensor_dev_attr(attr);
+	int nr = sensor_dev_attr->index;
+
+	int ret, value;
+
+	ret = iei_wt61p803_puzzle_read_pwm_channel(mcu_hwmon, nr, &value);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%d\n", value);
+}
+
+static SENSOR_DEVICE_ATTR_RW(pwm1, pwm, 0);
+static SENSOR_DEVICE_ATTR_RW(pwm2, pwm, 1);
+
+static struct attribute *iei_wt61p803_puzzle_attributes_temp[] = {
+	&sensor_dev_attr_temp1_input.dev_attr.attr,
+	&sensor_dev_attr_temp2_input.dev_attr.attr,
+	NULL
+};
+
+static struct attribute *iei_wt61p803_puzzle_attributes_fan[] = {
+	&sensor_dev_attr_fan1_input.dev_attr.attr,
+	&sensor_dev_attr_fan2_input.dev_attr.attr,
+	&sensor_dev_attr_fan3_input.dev_attr.attr,
+	&sensor_dev_attr_fan4_input.dev_attr.attr,
+	&sensor_dev_attr_fan5_input.dev_attr.attr,
+	NULL
+};
+
+static struct attribute *iei_wt61p803_puzzle_attributes_pwm[] = {
+	&sensor_dev_attr_pwm1.dev_attr.attr,
+	&sensor_dev_attr_pwm2.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group iei_wt61p803_puzzle_group_temp = {
+	.attrs = iei_wt61p803_puzzle_attributes_temp
+};
+
+static const struct attribute_group iei_wt61p803_puzzle_group_fan = {
+	.attrs = iei_wt61p803_puzzle_attributes_fan
+};
+
+static const struct attribute_group iei_wt61p803_puzzle_group_pwm = {
+	.attrs = iei_wt61p803_puzzle_attributes_pwm
+};
+
+static const struct attribute_group *iei_wt61p803_puzzle_attr_groups[] = {
+	&iei_wt61p803_puzzle_group_temp,
+	&iei_wt61p803_puzzle_group_fan,
+	&iei_wt61p803_puzzle_group_pwm,
+	NULL
+};
+
+/*
+ * Thermal cooling device
+ *
+ * Description: The functions below are thermal cooling device ops, registered
+ * with the Thermal Framework.
+ */
+
+static int iei_wt61p803_puzzle_get_max_state
+(struct thermal_cooling_device *tcdev, unsigned long *state)
+{
+	*state = IEI_WT61P803_PUZZLE_HWMON_MAX_PWM_VAL;
+
+	return 0;
+}
+
+static int iei_wt61p803_puzzle_get_cur_state
+(struct thermal_cooling_device *tcdev, unsigned long *state)
+{
+	struct iei_wt61p803_puzzle_thermal_cooling_device *cdev = tcdev->devdata;
+	struct iei_wt61p803_puzzle_hwmon *mcu_hwmon = cdev->mcu_hwmon;
+
+	int ret, value;
+
+	if (!mcu_hwmon)
+		return -EINVAL;
+
+	ret = iei_wt61p803_puzzle_read_pwm_channel(mcu_hwmon,
+			cdev->pwm_channel, &value);
+	if (ret)
+		return ret;
+
+	*state = (unsigned long)value;
+
+	return 0;
+}
+
+static int iei_wt61p803_puzzle_set_cur_state
+(struct thermal_cooling_device *tcdev, unsigned long state)
+{
+	struct iei_wt61p803_puzzle_thermal_cooling_device *cdev = tcdev->devdata;
+	struct iei_wt61p803_puzzle_hwmon *mcu_hwmon = cdev->mcu_hwmon;
+
+	if (!mcu_hwmon)
+		return -EINVAL;
+
+	return iei_wt61p803_puzzle_write_pwm_channel(mcu_hwmon,
+			cdev->pwm_channel, state);
+}
+
+static const struct thermal_cooling_device_ops iei_wt61p803_puzzle_cooling_ops = {
+	.get_max_state = iei_wt61p803_puzzle_get_max_state,
+	.get_cur_state = iei_wt61p803_puzzle_get_cur_state,
+	.set_cur_state = iei_wt61p803_puzzle_set_cur_state,
+};
+
+/*
+ * Driver setup
+ */
+
+static int iei_wt61p803_puzzle_enable_thermal_cooling_dev
+(struct device *dev, struct device_node *child, struct iei_wt61p803_puzzle_hwmon *mcu_hwmon)
+{
+	u32 pwm_channel;
+	int ret, num_levels;
+
+	struct iei_wt61p803_puzzle_thermal_cooling_device *cdev;
+
+	ret = of_property_read_u32(child, "reg", &pwm_channel);
+	if (ret)
+		return ret;
+
+	mutex_lock(&mcu_hwmon->thermal_cooling_dev_lock);
+	mcu_hwmon->thermal_cooling_dev_present[pwm_channel] = true;
+	mutex_unlock(&mcu_hwmon->thermal_cooling_dev_lock);
+
+	num_levels = of_property_count_u8_elems(child, "cooling-levels");
+	if (num_levels > 0) {
+		cdev = devm_kzalloc(dev, sizeof(*cdev), GFP_KERNEL);
+		if (!cdev)
+			return -ENOMEM;
+
+		cdev->cooling_levels = devm_kzalloc(dev, num_levels, GFP_KERNEL);
+		if (!cdev->cooling_levels)
+			return -ENOMEM;
+
+		ret = of_property_read_u8_array(child, "cooling-levels",
+				cdev->cooling_levels, num_levels);
+		if (ret) {
+			dev_err(dev, "Couldn't read property 'cooling-levels'");
+			return ret;
+		}
+
+		snprintf(cdev->name, THERMAL_NAME_LENGTH, "iei_wt61p803_puzzle_%d", pwm_channel);
+
+		cdev->tcdev = devm_thermal_of_cooling_device_register(dev, child,
+				cdev->name, cdev, &iei_wt61p803_puzzle_cooling_ops);
+		if (IS_ERR(cdev->tcdev))
+			return PTR_ERR(cdev->tcdev);
+
+		cdev->mcu_hwmon = mcu_hwmon;
+		cdev->pwm_channel = pwm_channel;
+
+		mutex_lock(&mcu_hwmon->thermal_cooling_dev_lock);
+		mcu_hwmon->cdev[pwm_channel] = cdev;
+		mutex_unlock(&mcu_hwmon->thermal_cooling_dev_lock);
+	}
+	return 0;
+}
+
+static int iei_wt61p803_puzzle_hwmon_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *child;
+
+	/* iEi WT61P803 PUZZLE HWMON priv struct */
+	struct iei_wt61p803_puzzle_hwmon *mcu_hwmon;
+
+	/* iEi WT61P803 PUZZLE MCU Core driver */
+	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev->parent);
+
+	struct device *hwmon_dev;
+
+	int ret;
+
+	mcu_hwmon = devm_kzalloc(dev, sizeof(*mcu_hwmon), GFP_KERNEL);
+	if (!mcu_hwmon)
+		return -ENOMEM;
+
+	/* Set reference to the parent 'core' MFD driver */
+	mcu_hwmon->mcu = mcu;
+
+	/* Initialize the mutex members */
+	mutex_init(&mcu_hwmon->temp_lock);
+	mutex_init(&mcu_hwmon->fan_lock);
+	mutex_init(&mcu_hwmon->pwm_lock);
+	mutex_init(&mcu_hwmon->thermal_cooling_dev_lock);
+
+	platform_set_drvdata(pdev, mcu_hwmon);
+
+	hwmon_dev = devm_hwmon_device_register_with_groups(dev,
+					"iei_wt61p803_puzzle",
+					mcu_hwmon,
+					iei_wt61p803_puzzle_attr_groups);
+
+	/* Control fans via PWM lines via Linux Kernel */
+	if (IS_ENABLED(CONFIG_THERMAL)) {
+		for_each_child_of_node(dev->of_node, child) {
+			ret = iei_wt61p803_puzzle_enable_thermal_cooling_dev(dev, child, mcu_hwmon);
+			if (ret) {
+				dev_err(dev, "Enabling the PWM fan failed\n");
+				of_node_put(child);
+				return ret;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static const struct of_device_id iei_wt61p803_puzzle_hwmon_id_table[] = {
+	{ .compatible = "iei,wt61p803-puzzle-hwmon" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, iei_wt61p803_puzzle_hwmon_id_table);
+
+static struct platform_driver iei_wt61p803_puzzle_hwmon_driver = {
+	.driver = {
+		.name = "iei-wt61p803-puzzle-hwmon",
+		.of_match_table = iei_wt61p803_puzzle_hwmon_id_table,
+	},
+	.probe = iei_wt61p803_puzzle_hwmon_probe,
+};
+
+module_platform_driver(iei_wt61p803_puzzle_hwmon_driver);
+
+MODULE_DESCRIPTION("iEi WT61P803 PUZZLE MCU HWMON Driver");
+MODULE_AUTHOR("Luka Kovacic <luka.kovacic@sartura.hr>");
+MODULE_LICENSE("GPL");
-- 
2.20.1

[PATCH 4/7] drivers: leds: Add the iEi WT61P803 PUZZLE LED driver

[Luka Kovacic]

Add support for the iEi WT61P803 PUZZLE LED driver.
Currently only the front panel power LED is supported.

This driver depends on the iEi WT61P803 PUZZLE MFD driver.

Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
Cc: Luka Perkov <luka.perkov@sartura.hr>
---
 drivers/leds/Kconfig                    |   8 ++
 drivers/leds/Makefile                   |   1 +
 drivers/leds/leds-iei-wt61p803-puzzle.c | 184 ++++++++++++++++++++++++
 3 files changed, 193 insertions(+)
 create mode 100644 drivers/leds/leds-iei-wt61p803-puzzle.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 1c181df24eae..8a25fb753dec 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -332,6 +332,14 @@ config LEDS_IPAQ_MICRO
 	  Choose this option if you want to use the notification LED on
 	  Compaq/HP iPAQ h3100 and h3600.
 
+config LEDS_IEI_WT61P803_PUZZLE
+	tristate "LED Support for the iEi WT61P803 PUZZLE MCU"
+	depends on LEDS_CLASS
+	depends on MFD_IEI_WT61P803_PUZZLE
+	help
+	  This option enables support for LEDs controlled by the iEi WT61P803
+	  M801 MCU.
+
 config LEDS_HP6XX
 	tristate "LED Support for the HP Jornada 6xx"
 	depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index c2c7d7ade0d0..cd362437fefd 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_LEDS_HP6XX)		+= leds-hp6xx.o
 obj-$(CONFIG_LEDS_INTEL_SS4200)		+= leds-ss4200.o
 obj-$(CONFIG_LEDS_IP30)			+= leds-ip30.o
 obj-$(CONFIG_LEDS_IPAQ_MICRO)		+= leds-ipaq-micro.o
+obj-$(CONFIG_LEDS_IEI_WT61P803_PUZZLE)	+= leds-iei-wt61p803-puzzle.o
 obj-$(CONFIG_LEDS_IS31FL319X)		+= leds-is31fl319x.o
 obj-$(CONFIG_LEDS_IS31FL32XX)		+= leds-is31fl32xx.o
 obj-$(CONFIG_LEDS_KTD2692)		+= leds-ktd2692.o
diff --git a/drivers/leds/leds-iei-wt61p803-puzzle.c b/drivers/leds/leds-iei-wt61p803-puzzle.c
new file mode 100644
index 000000000000..50d1e4e81571
--- /dev/null
+++ b/drivers/leds/leds-iei-wt61p803-puzzle.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/* iEi WT61P803 PUZZLE MCU LED Driver
+ *
+ * Copyright (C) 2020 Sartura Ltd.
+ * Author: Luka Kovacic <luka.kovacic@sartura.hr>
+ */
+
+#include <linux/module.h>
+#include <linux/mfd/iei-wt61p803-puzzle.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include <linux/leds.h>
+
+#define CMD_CHAR(x) (char)(x)
+
+/**
+ * enum iei_wt61p803_puzzle_led_state - LED state values
+ *
+ * @IEI_LED_OFF: The LED is turned off
+ * @IEI_LED_ON: The LED is turned on
+ * @IEI_LED_BLINK_5HZ: The LED will blink with a freq of 5 Hz
+ * @IEI_LED_BLINK_1HZ: The LED will blink with a freq of 1 Hz
+ */
+enum iei_wt61p803_puzzle_led_state {
+	IEI_LED_OFF = 0x30,
+	IEI_LED_ON = 0x31,
+	IEI_LED_BLINK_5HZ = 0x32,
+	IEI_LED_BLINK_1HZ = 0x33
+};
+
+/**
+ * struct iei_wt61p803_puzzle_led - MCU LED Driver
+ *
+ * @mcu: MCU struct pointer
+ * @lock: General mutex lock for LED operations
+ * @led_power_state: State of the front panel power LED
+ */
+struct iei_wt61p803_puzzle_led {
+	struct iei_wt61p803_puzzle *mcu;
+	struct mutex lock;
+
+	int led_power_state;
+};
+
+static inline struct iei_wt61p803_puzzle_led *
+		cdev_to_iei_wt61p803_puzzle_led(struct led_classdev *led_cdev)
+{
+	return dev_get_drvdata(led_cdev->dev->parent);
+}
+
+static int iei_wt61p803_puzzle_led_brightness_set_blocking
+	(struct led_classdev *cdev, enum led_brightness brightness)
+{
+	struct iei_wt61p803_puzzle_led *mcu_led =
+		cdev_to_iei_wt61p803_puzzle_led(cdev);
+	unsigned char led_power_cmd[5] = { '@', 'R', '1',
+		CMD_CHAR(IEI_LED_OFF) };
+
+	int ret;
+
+	size_t reply_size = 0;
+	unsigned char *resp_buf = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
+
+	mutex_lock(&mcu_led->lock);
+
+	if (brightness == LED_OFF) {
+		led_power_cmd[3] = CMD_CHAR(IEI_LED_OFF);
+		mcu_led->led_power_state = LED_OFF;
+	} else {
+		led_power_cmd[3] = CMD_CHAR(IEI_LED_ON);
+		mcu_led->led_power_state = LED_ON;
+	}
+
+	mutex_unlock(&mcu_led->lock);
+
+	ret = iei_wt61p803_puzzle_write_command(mcu_led->mcu, led_power_cmd,
+			sizeof(led_power_cmd), resp_buf, &reply_size);
+
+	kfree(resp_buf);
+
+	return ret;
+}
+
+static enum led_brightness
+iei_wt61p803_puzzle_led_brightness_get(struct led_classdev *cdev)
+{
+	struct iei_wt61p803_puzzle_led *mcu_led =
+		cdev_to_iei_wt61p803_puzzle_led(cdev);
+
+	int led_state;
+
+	mutex_lock(&mcu_led->lock);
+	led_state = mcu_led->led_power_state;
+	mutex_unlock(&mcu_led->lock);
+
+	return led_state;
+}
+
+static int iei_wt61p803_puzzle_led_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev->parent);
+	struct device_node *np = dev->of_node;
+
+	struct iei_wt61p803_puzzle_led *mcu_led;
+	struct device_node *child;
+
+	int ret;
+
+	mcu_led = devm_kzalloc(dev, sizeof(*mcu_led), GFP_KERNEL);
+	if (!mcu_led)
+		return -ENOMEM;
+
+	mcu_led->mcu = mcu;
+
+	/* The default LED power state is 1 */
+	mcu_led->led_power_state = 1;
+
+	/* Init the mutex lock */
+	mutex_init(&mcu_led->lock);
+
+	dev_set_drvdata(dev, mcu_led);
+
+	for_each_child_of_node(np, child) {
+		struct led_classdev *led;
+		u32 reg;
+
+		led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
+		if (!led)
+			return -ENOMEM;
+
+		ret = of_property_read_u32(child, "reg", &reg);
+		if (ret || reg > 1) {
+			dev_err(dev, "Could not register 'reg' of %s\n",
+				child->name);
+			continue;
+		}
+
+		if (of_property_read_string(child, "label", &led->name))
+			led->name = child->name;
+
+		of_property_read_string(child, "linux,default-trigger",
+				&led->default_trigger);
+
+		led->brightness_set_blocking =
+			iei_wt61p803_puzzle_led_brightness_set_blocking;
+		led->brightness_get = iei_wt61p803_puzzle_led_brightness_get;
+
+		led->max_brightness = 1;
+
+		ret = devm_led_classdev_register(dev, led);
+		if (ret) {
+			dev_err(dev, "Could not register %s\n", led->name);
+			return ret;
+		}
+	}
+
+	return 0;
+
+}
+
+static const struct of_device_id iei_wt61p803_puzzle_led_of_match[] = {
+	{ .compatible = "iei,wt61p803-puzzle-leds" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, iei_wt61p803_puzzle_led_of_match);
+
+static struct platform_driver iei_wt61p803_puzzle_led_driver = {
+	.driver = {
+		.name = "iei-wt61p803-puzzle-led",
+		.of_match_table = iei_wt61p803_puzzle_led_of_match,
+	},
+	.probe = iei_wt61p803_puzzle_led_probe,
+};
+module_platform_driver(iei_wt61p803_puzzle_led_driver);
+
+MODULE_DESCRIPTION("iEi WT61P803 PUZZLE front panel LED driver");
+MODULE_AUTHOR("Luka Kovacic <luka.kovacic@sartura.hr>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:leds-iei-wt61p803-puzzle");
-- 
2.20.1

[PATCH 5/7] Documentation/ABI: Add iei-wt61p803-puzzle driver sysfs interface documentation

[Luka Kovacic]

Add the iei-wt61p803-puzzle driver sysfs interface documentation to allow
monitoring and control of the microcontroller from user space.

Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
Cc: Luka Perkov <luka.perkov@sartura.hr>
---
 .../stable/sysfs-driver-iei-wt61p803-puzzle   | 65 +++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 Documentation/ABI/stable/sysfs-driver-iei-wt61p803-puzzle

diff --git a/Documentation/ABI/stable/sysfs-driver-iei-wt61p803-puzzle b/Documentation/ABI/stable/sysfs-driver-iei-wt61p803-puzzle
new file mode 100644
index 000000000000..36fca70d66ef
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-iei-wt61p803-puzzle
@@ -0,0 +1,65 @@
+What:		/sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/mac_address_*
+Date:		September 2020
+Contact:	Luka Kovacic <luka.kovacic@sartura.hr>
+Description:	Read the internal iEi WT61P803 PUZZLE MCU MAC address values.
+		These are factory assigned and can be changed.
+
+What:		/sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/serial_number
+Date:		September 2020
+Contact:	Luka Kovacic <luka.kovacic@sartura.hr>
+Description:	Read the internal iEi WT61P803 PUZZLE MCU serial number.
+		This value is factory assigned and can be changed.
+
+What:		/sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/version
+Date:		September 2020
+Contact:	Luka Kovacic <luka.kovacic@sartura.hr>
+Description:	Read the internal iEi WT61P803 PUZZLE MCU version.
+		This value is read only.
+
+What:		/sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/protocol_version
+Date:		September 2020
+Contact:	Luka Kovacic <luka.kovacic@sartura.hr>
+Description:	Read the internal iEi WT61P803 PUZZLE MCU protocol version.
+		This value is read only.
+
+What:		/sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/power_loss_recovery
+Date:		September 2020
+Contact:	Luka Kovacic <luka.kovacic@sartura.hr>
+Description:	Read the iEi WT61P803 PUZZLE MCU power loss recovery value.
+		This value is read write.
+		Value mapping: 0 - Always-On, 1 - Always-Off, 2 - Always-AC, 3 - Always-WA
+
+What:		/sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/bootloader_mode
+Date:		September 2020
+Contact:	Luka Kovacic <luka.kovacic@sartura.hr>
+Description:	Read whether the MCU is in bootloader mode.
+		This value is read only.
+
+What:		/sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/power_status
+Date:		September 2020
+Contact:	Luka Kovacic <luka.kovacic@sartura.hr>
+Description:	Read the iEi WT61P803 PUZZLE MCU power status. Power status indicates
+		the power on method.
+		This value is read only.
+		Value mapping (bitwise list):
+		0x80 - Null
+		0x40 - Firmware flag
+		0x20 - Power loss detection flag (powered off)
+		0x10 - Power loss detection flag (AC mode)
+		0x08 - Button power on
+		0x04 - WOL power on
+		0x02 - RTC alarm power on
+		0x01 - AC recover power on
+
+What:		/sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/build_info
+Date:		September 2020
+Contact:	Luka Kovacic <luka.kovacic@sartura.hr>
+Description:	Read the iEi WT61P803 PUZZLE MCU firmware build date.
+		This value is read only.
+		Format: yyyy/mm/dd hh:mm
+
+What:		/sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/ac_recovery_status
+Date:		September 2020
+Contact:	Luka Kovacic <luka.kovacic@sartura.hr>
+Description:	Read the iEi WT61P803 PUZZLE MCU AC recovery status.
+		This value is read only.
-- 
2.20.1

[PATCH 6/7] MAINTAINERS: Add an entry for the iEi WT61P803 PUZZLE driver

[Luka Kovacic]

Add an entry for the iEi WT61P803 PUZZLE driver (MFD, HWMON, LED drivers).

Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
Cc: Luka Perkov <luka.perkov@sartura.hr>
---
 MAINTAINERS | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index e4647c84c987..01a85d753d81 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8436,6 +8436,19 @@ F:	include/net/nl802154.h
 F:	net/ieee802154/
 F:	net/mac802154/
 
+IEI WT61P803 M801 MFD DRIVER
+M:	Luka Kovacic <luka.kovacic@sartura.hr>
+L:	linux-kernel@vger.kernel.org
+S:	Maintained
+F:	Documentation/ABI/stable/sysfs-driver-iei-wt61p803-puzzle
+F:	Documentation/devicetree/bindings/hwmon/iei,wt61p803-puzzle-hwmon.yaml
+F:	Documentation/devicetree/bindings/leds/iei,wt61p803-puzzle-leds.yaml
+F:	Documentation/devicetree/bindings/mfd/iei,wt61p803-puzzle.yaml
+F:	drivers/hwmon/iei-wt61p803-puzzle-hwmon.c
+F:	drivers/leds/leds-iei-wt61p803-puzzle.c
+F:	drivers/mfd/iei-wt61p803-puzzle.c
+F:	include/linux/mfd/iei-wt61p803-puzzle.h
+
 IFE PROTOCOL
 M:	Yotam Gigi <yotam.gi@gmail.com>
 M:	Jamal Hadi Salim <jhs@mojatatu.com>
-- 
2.20.1

[PATCH 7/7] arm64: dts: marvell: Add a device tree for the iEi Puzzle-M801 board

[Luka Kovacic]

Add initial support for the iEi Puzzle-M801 1U Rackmount Network
Appliance board.

The board is based on the quad-core Marvell Armada 8040 SoC and supports
up to 16 GB of DDR4 2400 MHz ECC RAM. It has a PCIe x16 slot (x2 lanes
only) and an M.2 type B slot.

Main system hardware:
2x USB 3.0
4x Gigabit Ethernet
2x SFP+
1x SATA 3.0
1x M.2 type B
1x RJ45 UART
1x SPI flash
1x iEi WT61P803 PUZZLE Microcontroller
1x EPSON RX8010 RTC (used instead of the integrated Marvell RTC controller)
6x SFP+ LED
1x HDD LED

All of the hardware listed above is supported and tested in this port.

Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
Cc: Luka Perkov <luka.perkov@sartura.hr>
---
 arch/arm64/boot/dts/marvell/Makefile          |   1 +
 .../dts/marvell/armada-8040-puzzle-m801.dts   | 519 ++++++++++++++++++
 2 files changed, 520 insertions(+)
 create mode 100644 arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dts

diff --git a/arch/arm64/boot/dts/marvell/Makefile b/arch/arm64/boot/dts/marvell/Makefile
index 3e5f2e7a040c..e413c3261792 100644
--- a/arch/arm64/boot/dts/marvell/Makefile
+++ b/arch/arm64/boot/dts/marvell/Makefile
@@ -12,6 +12,7 @@ dtb-$(CONFIG_ARCH_MVEBU) += armada-8040-clearfog-gt-8k.dtb
 dtb-$(CONFIG_ARCH_MVEBU) += armada-8040-db.dtb
 dtb-$(CONFIG_ARCH_MVEBU) += armada-8040-mcbin.dtb
 dtb-$(CONFIG_ARCH_MVEBU) += armada-8040-mcbin-singleshot.dtb
+dtb-$(CONFIG_ARCH_MVEBU) += armada-8040-puzzle-m801.dtb
 dtb-$(CONFIG_ARCH_MVEBU) += armada-8080-db.dtb
 dtb-$(CONFIG_ARCH_MVEBU) += cn9130-db.dtb
 dtb-$(CONFIG_ARCH_MVEBU) += cn9131-db.dtb
diff --git a/arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dts b/arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dts
new file mode 100644
index 000000000000..2921202b4f7b
--- /dev/null
+++ b/arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dts
@@ -0,0 +1,519 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (C) 2016 Marvell Technology Group Ltd.
+ * Copyright (C) 2020 Sartura Ltd.
+ *
+ * Device Tree file for iEi Puzzle-M801
+ */
+
+#include "armada-8040.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/leds/common.h>
+
+/ {
+	model = "iEi-Puzzle-M801";
+	compatible = "marvell,armada8040", "marvell,armada-ap806-quad", "marvell,armada-ap806";
+
+	aliases {
+		ethernet0 = &cp0_eth0;
+		ethernet1 = &cp1_eth0;
+		ethernet2 = &cp0_eth1;
+		ethernet3 = &cp0_eth2;
+		ethernet4 = &cp1_eth1;
+		ethernet5 = &cp1_eth2;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	memory@0 {
+		device_type = "memory";
+		reg = <0x0 0x0 0x0 0x80000000>;
+	};
+
+	/* Regulator labels correspond with schematics */
+	v_3_3: regulator-3-3v {
+		compatible = "regulator-fixed";
+		regulator-name = "v_3_3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-always-on;
+		status = "okay";
+	};
+
+	v_5v0_usb3_hst_vbus: regulator-usb3-vbus0 {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&cp0_gpio2 15 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&cp0_xhci_vbus_pins>;
+		regulator-name = "v_5v0_usb3_hst_vbus";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		status = "okay";
+	};
+
+	v_vddo_h: regulator-1-8v {
+		compatible = "regulator-fixed";
+		regulator-name = "v_vddo_h";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		regulator-always-on;
+		status = "okay";
+	};
+
+	sfp_cp0_eth0: sfp-cp0-eth0 {
+		compatible = "sff,sfp";
+		i2c-bus = <&sfpplus0_i2c>;
+		los-gpio = <&sfpplus_gpio 11 GPIO_ACTIVE_HIGH>;
+		mod-def0-gpio = <&sfpplus_gpio 10 GPIO_ACTIVE_LOW>;
+		tx-disable-gpio = <&sfpplus_gpio 9 GPIO_ACTIVE_HIGH>;
+		tx-fault-gpio  = <&sfpplus_gpio 8 GPIO_ACTIVE_HIGH>;
+		maximum-power-milliwatt = <3000>;
+	};
+
+	sfp_cp1_eth0: sfp-cp1-eth0 {
+		compatible = "sff,sfp";
+		i2c-bus = <&sfpplus1_i2c>;
+		los-gpio = <&sfpplus_gpio 3 GPIO_ACTIVE_HIGH>;
+		mod-def0-gpio = <&sfpplus_gpio 2 GPIO_ACTIVE_LOW>;
+		tx-disable-gpio = <&sfpplus_gpio 1 GPIO_ACTIVE_HIGH>;
+		tx-fault-gpio  = <&sfpplus_gpio 0 GPIO_ACTIVE_HIGH>;
+		maximum-power-milliwatt = <3000>;
+	};
+
+	leds {
+		compatible = "gpio-leds";
+		status = "okay";
+		pinctrl-0 = <&cp0_sfpplus_led_pins &cp1_sfpplus_led_pins>;
+		pinctrl-names = "default";
+
+		led0 {
+			function = LED_FUNCTION_STATUS;
+			label = "p2_act";
+			gpios = <&cp1_gpio1 6 GPIO_ACTIVE_LOW>;
+		};
+
+		led1 {
+			function = LED_FUNCTION_STATUS;
+			label = "p1_act";
+			gpios = <&cp1_gpio1 14 GPIO_ACTIVE_LOW>;
+		};
+
+		led2 {
+			function = LED_FUNCTION_STATUS;
+			label = "p2_10g";
+			gpios = <&cp1_gpio1 7 GPIO_ACTIVE_LOW>;
+		};
+
+		led3 {
+			function = LED_FUNCTION_STATUS;
+			label = "p2_1g";
+			gpios = <&cp1_gpio1 8 GPIO_ACTIVE_LOW>;
+		};
+
+		led4 {
+			function = LED_FUNCTION_STATUS;
+			label = "p1_10g";
+			gpios = <&cp1_gpio1 10 GPIO_ACTIVE_LOW>;
+		};
+
+		led5 {
+			function = LED_FUNCTION_STATUS;
+			label = "p1_1g";
+			gpios = <&cp1_gpio1 31 GPIO_ACTIVE_LOW>;
+		};
+
+		led6 {
+			function = LED_FUNCTION_STATUS;
+			linux,default-trigger = "disk-activity";
+			label = "front-hdd-led";
+			gpios = <&cp0_gpio2 22 GPIO_ACTIVE_HIGH>;
+		};
+
+	};
+};
+
+&ap_sdhci0 {
+	bus-width = <8>;
+	/*
+	 * Not stable in HS modes - phy needs "more calibration", so add
+	 * the "slow-mode" and disable SDR104, SDR50 and DDR50 modes.
+	 */
+	marvell,xenon-phy-slow-mode;
+	no-1-8-v;
+	no-sd;
+	no-sdio;
+	non-removable;
+	status = "okay";
+	vqmmc-supply = <&v_vddo_h>;
+};
+
+&ap_thermal_cpu1 {
+	trips {
+		cpu_active: cpu-active {
+			temperature = <44000>;
+			hysteresis = <2000>;
+			type = "active";
+		};
+	};
+	cooling-maps {
+		fan-map {
+			trip = <&cpu_active>;
+			cooling-device = <&chassis_fan_group0 64 THERMAL_NO_LIMIT>,
+					<&chassis_fan_group1 64 THERMAL_NO_LIMIT>;
+		};
+	};
+};
+
+&i2c0 {
+	clock-frequency = <100000>;
+	status = "okay";
+
+	rtc@32 {
+		compatible = "epson,rx8010";
+		reg = <0x32>;
+	};
+};
+
+&spi0 {
+	status = "okay";
+	spi-flash@0 {
+		#address-cells = <0x1>;
+		#size-cells = <0x1>;
+		compatible = "jedec,spi-nor";
+		reg = <0x0>;
+		spi-max-frequency = <20000000>;
+		partition@u-boot {
+			label = "u-boot";
+			reg = <0x00000000 0x001f0000>;
+		};
+		partition@u-boot-env {
+			label = "u-boot-env";
+			reg = <0x001f0000 0x00010000>;
+		};
+		partition@ubi1 {
+			label = "ubi1";
+			reg = <0x00200000 0x03f00000>;
+		};
+		partition@ubi2 {
+			label = "ubi2";
+			reg = <0x04100000 0x03f00000>;
+		};
+	};
+};
+
+&uart0 {
+	status = "okay";
+	pinctrl-0 = <&uart0_pins>;
+	pinctrl-names = "default";
+};
+
+&uart1 {
+	status = "okay";
+	/* iEi WT61P803 PUZZLE MCU Controller */
+	mcu {
+		compatible = "iei,wt61p803-puzzle";
+		current-speed = <115200>;
+		enable-probe-beep;
+
+		leds {
+			compatible = "iei,wt61p803-puzzle-leds";
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			led@0 {
+				reg = <0>;
+				color = <LED_COLOR_ID_BLUE>;
+				label = "front-power-led";
+			};
+		};
+
+		iei-wt61p803-puzzle-hwmon {
+			compatible = "iei,wt61p803-puzzle-hwmon";
+
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			chassis_fan_group0:fan-group@0 {
+				#cooling-cells = <2>;
+				reg = <0x00>;
+				cooling-levels = <64 102 170 230 250>;
+			};
+
+			chassis_fan_group1:fan-group@1 {
+				#cooling-cells = <2>;
+				reg = <0x01>;
+				cooling-levels = <64 102 170 230 250>;
+			};
+		};
+	};
+};
+
+&cp0_rtc {
+	status = "disabled";
+};
+
+&cp0_i2c0 {
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&cp0_i2c0_pins>;
+	status = "okay";
+
+	sfpplus_gpio: gpio@21 {
+		compatible = "nxp,pca9555";
+		reg = <0x21>;
+		gpio-controller;
+		#gpio-cells = <2>;
+	};
+
+	eeprom@54 {
+		compatible = "atmel,24c04";
+		reg = <0x54>;
+	};
+};
+
+&cp0_i2c1 {
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&cp0_i2c1_pins>;
+	status = "okay";
+
+	i2c-switch@70 {
+		compatible = "nxp,pca9544";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x70>;
+
+		sfpplus0_i2c: i2c@0 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0>;
+		};
+
+		sfpplus1_i2c: i2c@1 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <1>;
+		};
+	};
+};
+
+&cp0_uart1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&cp0_uart1_pins>;
+	status = "okay";
+};
+
+&cp0_mdio {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	status = "okay";
+
+	ge_phy2: ethernet-phy@2 {
+		reg = <0>;
+	};
+
+	ge_phy3: ethernet-phy@3 {
+		reg = <1>;
+	};
+};
+
+&cp0_pcie0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&cp0_pcie_pins>;
+	num-lanes = <1>;
+	num-viewport = <8>;
+	reset-gpios = <&cp0_gpio2 20 GPIO_ACTIVE_LOW>;
+	ranges = <0x82000000 0x0 0xc0000000 0x0 0xc0000000 0x0 0x20000000>;
+	phys = <&cp0_comphy0 0>;
+	phy-names = "cp0-pcie0-x1-phy";
+	status = "okay";
+};
+
+&cp0_pinctrl {
+	cp0_ge_mdio_pins: ge-mdio-pins {
+		marvell,pins = "mpp32", "mpp34";
+		marvell,function = "ge";
+	};
+	cp0_i2c1_pins: i2c1-pins {
+		marvell,pins = "mpp35", "mpp36";
+		marvell,function = "i2c1";
+	};
+	cp0_i2c0_pins: i2c0-pins {
+		marvell,pins = "mpp37", "mpp38";
+		marvell,function = "i2c0";
+	};
+	cp0_uart1_pins: uart1-pins {
+		marvell,pins = "mpp40", "mpp41";
+		marvell,function = "uart1";
+	};
+	cp0_xhci_vbus_pins: xhci0-vbus-pins {
+		marvell,pins = "mpp47";
+		marvell,function = "gpio";
+	};
+	cp0_pcie_pins: pcie-pins {
+		marvell,pins = "mpp52";
+		marvell,function = "gpio";
+	};
+	cp0_sdhci_pins: sdhci-pins {
+		marvell,pins = "mpp55", "mpp56", "mpp57", "mpp58", "mpp59",
+			       "mpp60", "mpp61";
+		marvell,function = "sdio";
+	};
+	cp0_sfpplus_led_pins: sfpplus-led-pins {
+		marvell,pins = "mpp54";
+		marvell,function = "gpio";
+	};
+};
+
+&cp0_ethernet {
+	status = "okay";
+};
+
+&cp0_eth0 {
+	status = "okay";
+	phy-mode = "10gbase-r";
+	phys = <&cp0_comphy4 0>;
+	local-mac-address = [00 50 43 de ff 00];
+	sfp = <&sfp_cp0_eth0>;
+	managed = "in-band-status";
+};
+
+&cp0_eth1 {
+	status = "okay";
+	phy = <&ge_phy2>;
+	phy-mode = "sgmii";
+	local-mac-address = [00 50 43 de ff 01];
+	phys = <&cp0_comphy3 1>;
+};
+
+&cp0_eth2 {
+	status = "okay";
+	phy-mode = "sgmii";
+	phys = <&cp0_comphy1 2>;
+	local-mac-address = [00 50 43 de ff 02];
+	phy = <&ge_phy3>;
+};
+
+&cp0_sata0 {
+	status = "okay";
+
+	sata-port@0 {
+		phys = <&cp0_comphy2 0>;
+		phy-names = "cp0-sata0-0-phy";
+	};
+
+	sata-port@1 {
+		phys = <&cp0_comphy5 1>;
+		phy-names = "cp0-sata0-1-phy";
+	};
+};
+
+&cp0_sdhci0 {
+	broken-cd;
+	bus-width = <4>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&cp0_sdhci_pins>;
+	status = "okay";
+	vqmmc-supply = <&v_3_3>;
+};
+
+&cp0_usb3_0 {
+	status = "okay";
+};
+
+&cp0_usb3_1 {
+	status = "okay";
+};
+
+&cp1_i2c0 {
+	clock-frequency = <100000>;
+	status = "disabled";
+};
+
+&cp1_i2c1 {
+	clock-frequency = <100000>;
+	status = "disabled";
+};
+
+&cp1_rtc {
+	status = "disabled";
+};
+
+&cp1_ethernet {
+	status = "okay";
+};
+
+&cp1_eth0 {
+	status = "okay";
+	phy-mode = "10gbase-r";
+	phys = <&cp1_comphy4 0>;
+	local-mac-address = [00 50 43 de ff 03];
+	sfp = <&sfp_cp1_eth0>;
+	managed = "in-band-status";
+};
+
+&cp1_eth1 {
+	status = "okay";
+	phy = <&ge_phy4>;
+	phy-mode = "sgmii";
+	local-mac-address = [00 50 43 de ff 04];
+	phys = <&cp1_comphy3 1>;
+};
+
+&cp1_eth2 {
+	status = "okay";
+	phy-mode = "sgmii";
+	local-mac-address = [00 50 43 de ff 05];
+	phys = <&cp1_comphy5 2>;
+	phy = <&ge_phy5>;
+};
+
+&cp1_pinctrl {
+	cp1_sfpplus_led_pins: sfpplus-led-pins {
+		marvell,pins = "mpp6", "mpp7", "mpp8", "mpp10", "mpp14", "mpp31";
+		marvell,function = "gpio";
+	};
+};
+
+&cp1_uart0 {
+	status = "disabled";
+};
+
+&cp1_comphy2 {
+	cp1_usbh0_con: connector {
+		compatible = "usb-a-connector";
+		phy-supply = <&v_5v0_usb3_hst_vbus>;
+	};
+};
+
+&cp1_usb3_0 {
+	phys = <&cp1_comphy2 0>;
+	phy-names = "cp1-usb3h0-comphy";
+	status = "okay";
+};
+
+&cp1_mdio {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	status = "okay";
+
+	ge_phy4: ethernet-phy@4 {
+		reg = <1>;
+	};
+	ge_phy5: ethernet-phy@5 {
+		reg = <0>;
+	};
+};
+
+&cp1_pcie0 {
+	num-lanes = <2>;
+	phys = <&cp1_comphy0 0>, <&cp1_comphy1 0>;
+	phy-names = "cp1-pcie0-x2-lane0-phy", "cp1-pcie0-x2-lane1-phy";
+	status = "okay";
+};
-- 
2.20.1

Re: [PATCH 7/7] arm64: dts: marvell: Add a device tree for the iEi Puzzle-M801 board

[Andrew Lunn]

On Sat, Sep 05, 2020 at 03:03:36PM +0200, Luka Kovacic wrote:

> +&cp0_mdio {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	status = "okay";
> +
> +	ge_phy2: ethernet-phy@2 {
> +		reg = <0>;

Hi Luka

The value after the @ should be the same as the reg value.
So this should be ge_phy2: ethernet-phy@0

> +	};
> +
> +	ge_phy3: ethernet-phy@3 {
> +		reg = <1>;

And this is:

ge_phy2: ethernet-phy@1

This is a general DT rule, so please check the whole file.

> +&cp0_eth0 {
> +	status = "okay";
> +	phy-mode = "10gbase-r";
> +	phys = <&cp0_comphy4 0>;
> +	local-mac-address = [00 50 43 de ff 00];

Does the bootloader overwrite these with per device MAC addresses?

Looking up this OUI i find:

00:50:43 Marvell Semiconductor, Inc.

I'm not sure you should be using that!

    Andrew

Re: [PATCH 3/7] drivers: hwmon: Add the iEi WT61P803 PUZZLE HWMON driver

[Guenter Roeck]

On 9/5/20 6:03 AM, Luka Kovacic wrote:
> Add the iEi WT61P803 PUZZLE HWMON driver, that handles the fan speed
> control via PWM, reading fan speed and reading on-board temperature
> sensors.
> 
> The driver registers a HWMON device and a simple thermal cooling device to
> enable in-kernel fan management.
> 
> This driver depends on the iEi WT61P803 PUZZLE MFD driver.
> 
> Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
> Cc: Luka Perkov <luka.perkov@sartura.hr>

New hwmon drivers should use the [devm_]hwmon_device_register_with_info() API.

Guenter

> ---
>  drivers/hwmon/Kconfig                     |   8 +
>  drivers/hwmon/Makefile                    |   1 +
>  drivers/hwmon/iei-wt61p803-puzzle-hwmon.c | 613 ++++++++++++++++++++++
>  3 files changed, 622 insertions(+)
>  create mode 100644 drivers/hwmon/iei-wt61p803-puzzle-hwmon.c
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 8dc28b26916e..ff279df9bf40 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -722,6 +722,14 @@ config SENSORS_IBMPOWERNV
>  	  This driver can also be built as a module. If so, the module
>  	  will be called ibmpowernv.
>  
> +config SENSORS_IEI_WT61P803_PUZZLE_HWMON
> +	tristate "iEi WT61P803 PUZZLE MFD HWMON Driver"
> +	depends on MFD_IEI_WT61P803_PUZZLE
> +	help
> +	  The iEi WT61P803 PUZZLE MFD HWMON Driver handles reading fan speed
> +	  and writing fan PWM values. It also supports reading on-board
> +	  temperature sensors.
> +
>  config SENSORS_IIO_HWMON
>  	tristate "Hwmon driver that uses channels specified via iio maps"
>  	depends on IIO
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index a8f4b35b136b..b0afb2d6896f 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -83,6 +83,7 @@ obj-$(CONFIG_SENSORS_HIH6130)	+= hih6130.o
>  obj-$(CONFIG_SENSORS_ULTRA45)	+= ultra45_env.o
>  obj-$(CONFIG_SENSORS_I5500)	+= i5500_temp.o
>  obj-$(CONFIG_SENSORS_I5K_AMB)	+= i5k_amb.o
> +obj-$(CONFIG_SENSORS_IEI_WT61P803_PUZZLE_HWMON) += iei-wt61p803-puzzle-hwmon.o
>  obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
>  obj-$(CONFIG_SENSORS_IBMPEX)	+= ibmpex.o
>  obj-$(CONFIG_SENSORS_IBMPOWERNV)+= ibmpowernv.o
> diff --git a/drivers/hwmon/iei-wt61p803-puzzle-hwmon.c b/drivers/hwmon/iei-wt61p803-puzzle-hwmon.c
> new file mode 100644
> index 000000000000..ca26d0cc6884
> --- /dev/null
> +++ b/drivers/hwmon/iei-wt61p803-puzzle-hwmon.c
> @@ -0,0 +1,613 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/* iEi WT61P803 PUZZLE MCU HWMON Driver
> + *
> + * Copyright (C) 2020 Sartura Ltd.
> + * Author: Luka Kovacic <luka.kovacic@sartura.hr>
> + */
> +
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/iei-wt61p803-puzzle.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/math64.h>
> +#include <linux/err.h>
> +
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/thermal.h>
> +
> +#define IEI_WT61P803_PUZZLE_HWMON_MAX_TEMP_NUM 2
> +#define IEI_WT61P803_PUZZLE_HWMON_MAX_FAN_NUM 5
> +#define IEI_WT61P803_PUZZLE_HWMON_MAX_PWM_NUM 2
> +#define IEI_WT61P803_PUZZLE_HWMON_MAX_PWM_VAL 255
> +
> +/**
> + * struct iei_wt61p803_puzzle_thermal_cooling_device - Thermal cooling device instance
> + *
> + * @mcu_hwmon: MCU HWMON struct pointer
> + * @tcdev: Thermal cooling device pointer
> + * @name: Thermal cooling device name
> + * @pwm_channel: PWM channel (0 or 1)
> + * @cooling_levels: Thermal cooling device cooling levels
> + */
> +struct iei_wt61p803_puzzle_thermal_cooling_device {
> +	struct iei_wt61p803_puzzle_hwmon *mcu_hwmon;
> +	struct thermal_cooling_device *tcdev;
> +	char name[THERMAL_NAME_LENGTH];
> +	int pwm_channel;
> +	u8 *cooling_levels;
> +};
> +
> +/**
> + * struct iei_wt61p803_puzzle_hwmon - MCU HWMON Driver
> + *
> + * @mcu: MCU struct pointer
> + * @temp_lock: Mutex for temp_sensor_val
> + * @temp_sensor_val: Temperature sensor values
> + * @fan_lock: Mutex for fan_speed_val
> + * @fan_speed_val: FAN speed (RPM) values
> + * @pwm_lock: Mutex for pwm_val
> + * @pwm_val: PWM values (0-255)
> + * @thermal_cooling_dev_lock: Mutex for Thermal Framework related members
> + * @thermal_cooling_dev_present: Per-channel thermal cooling device control
> + * @cdev: Per-channel thermal cooling device private structure
> + */
> +struct iei_wt61p803_puzzle_hwmon {
> +	struct iei_wt61p803_puzzle *mcu;
> +
> +	struct mutex temp_lock;
> +	int temp_sensor_val[IEI_WT61P803_PUZZLE_HWMON_MAX_TEMP_NUM];
> +
> +	struct mutex fan_lock;
> +	int fan_speed_val[IEI_WT61P803_PUZZLE_HWMON_MAX_FAN_NUM];
> +
> +	struct mutex pwm_lock;
> +	int pwm_val[IEI_WT61P803_PUZZLE_HWMON_MAX_PWM_NUM];
> +
> +	struct mutex thermal_cooling_dev_lock;
> +	bool thermal_cooling_dev_present[IEI_WT61P803_PUZZLE_HWMON_MAX_PWM_NUM];
> +	struct iei_wt61p803_puzzle_thermal_cooling_device
> +		*cdev[IEI_WT61P803_PUZZLE_HWMON_MAX_PWM_NUM];
> +};
> +
> +/*
> + * Generic MCU access functions
> + *
> + * Description: The functions below are used as generic translation functions
> + * between the kernel and the MCU hardware. These can be used from HWMON or
> + * from the Thermal Framework.
> + */
> +
> +#define raw_temp_to_milidegree_celsius(x) ((int)((x - 0x80)*1000))
> +static int iei_wt61p803_puzzle_read_temp_sensor
> +(struct iei_wt61p803_puzzle_hwmon *mcu_hwmon, int channel, int *value)
> +{
> +	int ret;
> +	size_t reply_size = 0;
> +	unsigned char *resp_buf;
> +
> +	/* MCU Command: Retrieve all available NTC values */
> +	unsigned char temp_sensor_ntc_cmd[4] = { '@', 'T', 'A' };
> +
> +	if (channel > 1 && channel < 0)
> +		return -EINVAL;
> +
> +	resp_buf = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
> +	if (!resp_buf)
> +		return -ENOMEM;
> +
> +	/* Write the command to the MCU */
> +	ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu,
> +			temp_sensor_ntc_cmd, sizeof(temp_sensor_ntc_cmd),
> +			resp_buf, &reply_size);
> +	if (!ret) {
> +		/* Check the number of NTC values (should be 0x32/'2') */
> +		if (resp_buf[3] == 0x32) {
> +			mutex_lock(&mcu_hwmon->temp_lock);
> +
> +			/* Write values to the struct */
> +			mcu_hwmon->temp_sensor_val[0] =
> +				raw_temp_to_milidegree_celsius(resp_buf[4]);
> +			mcu_hwmon->temp_sensor_val[1] =
> +				raw_temp_to_milidegree_celsius(resp_buf[5]);
> +
> +			mutex_unlock(&mcu_hwmon->temp_lock);
> +		}
> +
> +	}
> +
> +	kfree(resp_buf);
> +
> +	mutex_lock(&mcu_hwmon->temp_lock);
> +	*value = mcu_hwmon->temp_sensor_val[channel];
> +	mutex_unlock(&mcu_hwmon->temp_lock);
> +
> +	return ret;
> +}
> +
> +#define raw_fan_val_to_rpm(x, y) ((int)(((x)<<8|(y))/2)*60)
> +static int iei_wt61p803_puzzle_read_fan_speed
> +(struct iei_wt61p803_puzzle_hwmon *mcu_hwmon, int channel, int *value)
> +{
> +	int ret;
> +	size_t reply_size = 0;
> +	unsigned char *resp_buf;
> +
> +	/* MCU Command: Retrieve fan speed value */
> +	unsigned char fan_speed_cmd[4] = { '@', 'F', 'A' };
> +
> +	resp_buf = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
> +	if (!resp_buf)
> +		return -ENOMEM;
> +
> +	switch (channel) {
> +	case 0:
> +		fan_speed_cmd[2] = 'A';
> +		break;
> +	case 1:
> +		fan_speed_cmd[2] = 'B';
> +		break;
> +	case 2:
> +		fan_speed_cmd[2] = 'C';
> +		break;
> +	case 3:
> +		fan_speed_cmd[2] = 'D';
> +		break;
> +	case 4:
> +		fan_speed_cmd[2] = 'E';
> +		break;
> +	default:
> +		kfree(resp_buf);
> +		return -EINVAL;
> +	}
> +
> +	/* Write the command to the MCU */
> +	ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, fan_speed_cmd,
> +			sizeof(fan_speed_cmd), resp_buf, &reply_size);
> +	if (!ret) {
> +		mutex_lock(&mcu_hwmon->fan_lock);
> +
> +		/* Calculate fan RPM */
> +		mcu_hwmon->fan_speed_val[channel] = raw_fan_val_to_rpm(resp_buf[3],
> +				resp_buf[4]);
> +
> +		mutex_unlock(&mcu_hwmon->fan_lock);
> +	}
> +
> +	kfree(resp_buf);
> +
> +	mutex_lock(&mcu_hwmon->fan_lock);
> +	*value = mcu_hwmon->fan_speed_val[channel];
> +	mutex_unlock(&mcu_hwmon->fan_lock);
> +
> +	return 0;
> +}
> +
> +static int iei_wt61p803_puzzle_write_pwm_channel
> +(struct iei_wt61p803_puzzle_hwmon *mcu_hwmon, int channel, long pwm_set_val)
> +{
> +	int ret;
> +	size_t reply_size = 0;
> +	unsigned char *resp_buf;
> +
> +	/* MCU Command: Set PWM value (Default channel is 0/0x30 at index 3) */
> +	unsigned char pwm_set_cmd[6] = { '@', 'F', 'W', 0x30, 0x00 };
> +
> +	resp_buf = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
> +	if (!resp_buf)
> +		return -ENOMEM;
> +
> +	/* Determine the PWM channel */
> +	switch (channel) {
> +	case 0:
> +		pwm_set_cmd[3] = 0x30;
> +		break;
> +	case 1:
> +		pwm_set_cmd[3] = 0x31;
> +		break;
> +	default:
> +		kfree(resp_buf);
> +		return -EINVAL;
> +	}
> +
> +	if (pwm_set_val < 0 || pwm_set_val > IEI_WT61P803_PUZZLE_HWMON_MAX_PWM_VAL) {
> +		kfree(resp_buf);
> +		return -EINVAL;
> +	}
> +
> +	/* Add the value to the command */
> +	pwm_set_cmd[4] = (char)pwm_set_val;
> +
> +	/* Write the command to the MCU */
> +	ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, pwm_set_cmd,
> +			sizeof(pwm_set_cmd), resp_buf, &reply_size);
> +	if (!ret) {
> +		/* Store the PWM value */
> +		if (resp_buf[0] == '@' && resp_buf[1] == 0x30) {
> +			mutex_lock(&mcu_hwmon->pwm_lock);
> +			mcu_hwmon->pwm_val[channel] = (int)pwm_set_val;
> +			mutex_unlock(&mcu_hwmon->pwm_lock);
> +		}
> +	}
> +
> +	kfree(resp_buf);
> +	return 0;
> +}
> +
> +static int iei_wt61p803_puzzle_read_pwm_channel
> +(struct iei_wt61p803_puzzle_hwmon *mcu_hwmon, int channel, int *value)
> +{
> +	int ret;
> +	size_t reply_size = 0;
> +	unsigned char *resp_buf;
> +
> +	/* MCU Command: Retrieve PWM value (Default channel: 0x30 at idx 3) */
> +	unsigned char pwm_get_cmd[5] = { '@', 'F', 'Z', 0x30 };
> +
> +	resp_buf = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
> +	if (!resp_buf)
> +		return -ENOMEM;
> +
> +	/* Determine the PWM channel */
> +	switch (channel) {
> +	case 0:
> +		pwm_get_cmd[3] = 0x30;
> +		break;
> +	case 1:
> +		pwm_get_cmd[3] = 0x31;
> +		break;
> +	default:
> +		kfree(resp_buf);
> +		return -EINVAL;
> +	}
> +
> +	/* Write the command to the MCU */
> +	ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, pwm_get_cmd,
> +			sizeof(pwm_get_cmd), resp_buf, &reply_size);
> +	if (!ret) {
> +		/* Store the PWM value */
> +		if (resp_buf[2] == 'Z') {
> +			mutex_lock(&mcu_hwmon->pwm_lock);
> +			mcu_hwmon->pwm_val[channel] = (int)resp_buf[3];
> +			mutex_unlock(&mcu_hwmon->pwm_lock);
> +		}
> +	}
> +
> +	kfree(resp_buf);
> +
> +	mutex_lock(&mcu_hwmon->pwm_lock);
> +	*value = mcu_hwmon->pwm_val[channel];
> +	mutex_unlock(&mcu_hwmon->pwm_lock);
> +
> +	return 0;
> +}
> +
> +/*
> + * HWMON attributes
> + *
> + * Description: The attributes below are registered with the HWMON subsystem.
> + * Fans can only be controlled, if they are not controlled by the Thermal
> + * Framework.
> + */
> +
> +static ssize_t temp_input_show(struct device *dev,
> +			struct device_attribute *attr, char *buf)
> +{
> +	struct iei_wt61p803_puzzle_hwmon *mcu_hwmon =
> +		dev_get_drvdata(dev->parent);
> +	struct sensor_device_attribute *sensor_dev_attr =
> +		to_sensor_dev_attr(attr);
> +	int nr = sensor_dev_attr->index;
> +
> +	int ret, value;
> +
> +	ret = iei_wt61p803_puzzle_read_temp_sensor(mcu_hwmon, nr, &value);
> +	if (ret)
> +		return ret;
> +
> +	return sprintf(buf, "%d\n", value);
> +}
> +
> +static SENSOR_DEVICE_ATTR_RO(temp1_input, temp_input, 0);
> +static SENSOR_DEVICE_ATTR_RO(temp2_input, temp_input, 1);
> +
> +static ssize_t fan_input_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct iei_wt61p803_puzzle_hwmon *mcu_hwmon =
> +		dev_get_drvdata(dev->parent);
> +	struct sensor_device_attribute *sensor_dev_attr =
> +		to_sensor_dev_attr(attr);
> +	int nr = sensor_dev_attr->index;
> +
> +	int ret, value;
> +
> +	ret = iei_wt61p803_puzzle_read_fan_speed(mcu_hwmon, nr, &value);
> +	if (ret)
> +		return ret;
> +
> +	return sprintf(buf, "%d\n", value);
> +}
> +
> +static SENSOR_DEVICE_ATTR_RO(fan1_input, fan_input, 0);
> +static SENSOR_DEVICE_ATTR_RO(fan2_input, fan_input, 1);
> +static SENSOR_DEVICE_ATTR_RO(fan3_input, fan_input, 2);
> +static SENSOR_DEVICE_ATTR_RO(fan4_input, fan_input, 3);
> +static SENSOR_DEVICE_ATTR_RO(fan5_input, fan_input, 4);
> +
> +static ssize_t pwm_store(struct device *dev, struct device_attribute *attr,
> +			const char *buf, size_t count)
> +{
> +	struct iei_wt61p803_puzzle_hwmon *mcu_hwmon =
> +		dev_get_drvdata(dev->parent);
> +	struct sensor_device_attribute *sensor_dev_attr =
> +		to_sensor_dev_attr(attr);
> +	int nr = sensor_dev_attr->index;
> +
> +	int ret;
> +	long pwm_value;
> +
> +	if (mcu_hwmon->thermal_cooling_dev_present[nr]) {
> +		/*
> +		 * The Thermal Framework has already claimed this specific PWM
> +		 * channel. Return 0, to indicate 0 bytes written.
> +		 */
> +
> +		return 0;
> +	}
> +
> +	/* Convert the string to a long */
> +	ret = kstrtol(buf, 10, &pwm_value);
> +	if (ret)
> +		return ret;
> +
> +	ret = iei_wt61p803_puzzle_write_pwm_channel(mcu_hwmon, nr, pwm_value);
> +	if (ret)
> +		return ret;
> +
> +	return count;
> +}
> +
> +static ssize_t pwm_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct iei_wt61p803_puzzle_hwmon *mcu_hwmon =
> +		dev_get_drvdata(dev->parent);
> +	struct sensor_device_attribute *sensor_dev_attr =
> +		to_sensor_dev_attr(attr);
> +	int nr = sensor_dev_attr->index;
> +
> +	int ret, value;
> +
> +	ret = iei_wt61p803_puzzle_read_pwm_channel(mcu_hwmon, nr, &value);
> +	if (ret)
> +		return ret;
> +
> +	return sprintf(buf, "%d\n", value);
> +}
> +
> +static SENSOR_DEVICE_ATTR_RW(pwm1, pwm, 0);
> +static SENSOR_DEVICE_ATTR_RW(pwm2, pwm, 1);
> +
> +static struct attribute *iei_wt61p803_puzzle_attributes_temp[] = {
> +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> +	&sensor_dev_attr_temp2_input.dev_attr.attr,
> +	NULL
> +};
> +
> +static struct attribute *iei_wt61p803_puzzle_attributes_fan[] = {
> +	&sensor_dev_attr_fan1_input.dev_attr.attr,
> +	&sensor_dev_attr_fan2_input.dev_attr.attr,
> +	&sensor_dev_attr_fan3_input.dev_attr.attr,
> +	&sensor_dev_attr_fan4_input.dev_attr.attr,
> +	&sensor_dev_attr_fan5_input.dev_attr.attr,
> +	NULL
> +};
> +
> +static struct attribute *iei_wt61p803_puzzle_attributes_pwm[] = {
> +	&sensor_dev_attr_pwm1.dev_attr.attr,
> +	&sensor_dev_attr_pwm2.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group iei_wt61p803_puzzle_group_temp = {
> +	.attrs = iei_wt61p803_puzzle_attributes_temp
> +};
> +
> +static const struct attribute_group iei_wt61p803_puzzle_group_fan = {
> +	.attrs = iei_wt61p803_puzzle_attributes_fan
> +};
> +
> +static const struct attribute_group iei_wt61p803_puzzle_group_pwm = {
> +	.attrs = iei_wt61p803_puzzle_attributes_pwm
> +};
> +
> +static const struct attribute_group *iei_wt61p803_puzzle_attr_groups[] = {
> +	&iei_wt61p803_puzzle_group_temp,
> +	&iei_wt61p803_puzzle_group_fan,
> +	&iei_wt61p803_puzzle_group_pwm,
> +	NULL
> +};
> +
> +/*
> + * Thermal cooling device
> + *
> + * Description: The functions below are thermal cooling device ops, registered
> + * with the Thermal Framework.
> + */
> +
> +static int iei_wt61p803_puzzle_get_max_state
> +(struct thermal_cooling_device *tcdev, unsigned long *state)
> +{
> +	*state = IEI_WT61P803_PUZZLE_HWMON_MAX_PWM_VAL;
> +
> +	return 0;
> +}
> +
> +static int iei_wt61p803_puzzle_get_cur_state
> +(struct thermal_cooling_device *tcdev, unsigned long *state)
> +{
> +	struct iei_wt61p803_puzzle_thermal_cooling_device *cdev = tcdev->devdata;
> +	struct iei_wt61p803_puzzle_hwmon *mcu_hwmon = cdev->mcu_hwmon;
> +
> +	int ret, value;
> +
> +	if (!mcu_hwmon)
> +		return -EINVAL;
> +
> +	ret = iei_wt61p803_puzzle_read_pwm_channel(mcu_hwmon,
> +			cdev->pwm_channel, &value);
> +	if (ret)
> +		return ret;
> +
> +	*state = (unsigned long)value;
> +
> +	return 0;
> +}
> +
> +static int iei_wt61p803_puzzle_set_cur_state
> +(struct thermal_cooling_device *tcdev, unsigned long state)
> +{
> +	struct iei_wt61p803_puzzle_thermal_cooling_device *cdev = tcdev->devdata;
> +	struct iei_wt61p803_puzzle_hwmon *mcu_hwmon = cdev->mcu_hwmon;
> +
> +	if (!mcu_hwmon)
> +		return -EINVAL;
> +
> +	return iei_wt61p803_puzzle_write_pwm_channel(mcu_hwmon,
> +			cdev->pwm_channel, state);
> +}
> +
> +static const struct thermal_cooling_device_ops iei_wt61p803_puzzle_cooling_ops = {
> +	.get_max_state = iei_wt61p803_puzzle_get_max_state,
> +	.get_cur_state = iei_wt61p803_puzzle_get_cur_state,
> +	.set_cur_state = iei_wt61p803_puzzle_set_cur_state,
> +};
> +
> +/*
> + * Driver setup
> + */
> +
> +static int iei_wt61p803_puzzle_enable_thermal_cooling_dev
> +(struct device *dev, struct device_node *child, struct iei_wt61p803_puzzle_hwmon *mcu_hwmon)
> +{
> +	u32 pwm_channel;
> +	int ret, num_levels;
> +
> +	struct iei_wt61p803_puzzle_thermal_cooling_device *cdev;
> +
> +	ret = of_property_read_u32(child, "reg", &pwm_channel);
> +	if (ret)
> +		return ret;
> +
> +	mutex_lock(&mcu_hwmon->thermal_cooling_dev_lock);
> +	mcu_hwmon->thermal_cooling_dev_present[pwm_channel] = true;
> +	mutex_unlock(&mcu_hwmon->thermal_cooling_dev_lock);
> +
> +	num_levels = of_property_count_u8_elems(child, "cooling-levels");
> +	if (num_levels > 0) {
> +		cdev = devm_kzalloc(dev, sizeof(*cdev), GFP_KERNEL);
> +		if (!cdev)
> +			return -ENOMEM;
> +
> +		cdev->cooling_levels = devm_kzalloc(dev, num_levels, GFP_KERNEL);
> +		if (!cdev->cooling_levels)
> +			return -ENOMEM;
> +
> +		ret = of_property_read_u8_array(child, "cooling-levels",
> +				cdev->cooling_levels, num_levels);
> +		if (ret) {
> +			dev_err(dev, "Couldn't read property 'cooling-levels'");
> +			return ret;
> +		}
> +
> +		snprintf(cdev->name, THERMAL_NAME_LENGTH, "iei_wt61p803_puzzle_%d", pwm_channel);
> +
> +		cdev->tcdev = devm_thermal_of_cooling_device_register(dev, child,
> +				cdev->name, cdev, &iei_wt61p803_puzzle_cooling_ops);
> +		if (IS_ERR(cdev->tcdev))
> +			return PTR_ERR(cdev->tcdev);
> +
> +		cdev->mcu_hwmon = mcu_hwmon;
> +		cdev->pwm_channel = pwm_channel;
> +
> +		mutex_lock(&mcu_hwmon->thermal_cooling_dev_lock);
> +		mcu_hwmon->cdev[pwm_channel] = cdev;
> +		mutex_unlock(&mcu_hwmon->thermal_cooling_dev_lock);
> +	}
> +	return 0;
> +}
> +
> +static int iei_wt61p803_puzzle_hwmon_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *child;
> +
> +	/* iEi WT61P803 PUZZLE HWMON priv struct */
> +	struct iei_wt61p803_puzzle_hwmon *mcu_hwmon;
> +
> +	/* iEi WT61P803 PUZZLE MCU Core driver */
> +	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev->parent);
> +
> +	struct device *hwmon_dev;
> +
> +	int ret;
> +
> +	mcu_hwmon = devm_kzalloc(dev, sizeof(*mcu_hwmon), GFP_KERNEL);
> +	if (!mcu_hwmon)
> +		return -ENOMEM;
> +
> +	/* Set reference to the parent 'core' MFD driver */
> +	mcu_hwmon->mcu = mcu;
> +
> +	/* Initialize the mutex members */
> +	mutex_init(&mcu_hwmon->temp_lock);
> +	mutex_init(&mcu_hwmon->fan_lock);
> +	mutex_init(&mcu_hwmon->pwm_lock);
> +	mutex_init(&mcu_hwmon->thermal_cooling_dev_lock);
> +
> +	platform_set_drvdata(pdev, mcu_hwmon);
> +
> +	hwmon_dev = devm_hwmon_device_register_with_groups(dev,
> +					"iei_wt61p803_puzzle",
> +					mcu_hwmon,
> +					iei_wt61p803_puzzle_attr_groups);
> +
> +	/* Control fans via PWM lines via Linux Kernel */
> +	if (IS_ENABLED(CONFIG_THERMAL)) {
> +		for_each_child_of_node(dev->of_node, child) {
> +			ret = iei_wt61p803_puzzle_enable_thermal_cooling_dev(dev, child, mcu_hwmon);
> +			if (ret) {
> +				dev_err(dev, "Enabling the PWM fan failed\n");
> +				of_node_put(child);
> +				return ret;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id iei_wt61p803_puzzle_hwmon_id_table[] = {
> +	{ .compatible = "iei,wt61p803-puzzle-hwmon" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, iei_wt61p803_puzzle_hwmon_id_table);
> +
> +static struct platform_driver iei_wt61p803_puzzle_hwmon_driver = {
> +	.driver = {
> +		.name = "iei-wt61p803-puzzle-hwmon",
> +		.of_match_table = iei_wt61p803_puzzle_hwmon_id_table,
> +	},
> +	.probe = iei_wt61p803_puzzle_hwmon_probe,
> +};
> +
> +module_platform_driver(iei_wt61p803_puzzle_hwmon_driver);
> +
> +MODULE_DESCRIPTION("iEi WT61P803 PUZZLE MCU HWMON Driver");
> +MODULE_AUTHOR("Luka Kovacic <luka.kovacic@sartura.hr>");
> +MODULE_LICENSE("GPL");
> 

Re: [PATCH 2/7] drivers: mfd: Add a driver for iEi WT61P803 PUZZLE MCU

[Randy Dunlap]

On 9/5/20 6:03 AM, Luka Kovacic wrote:
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 33df0837ab41..3fcda95f07a3 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2118,5 +2118,13 @@ config SGI_MFD_IOC3
>  	  If you have an SGI Origin, Octane, or a PCI IOC3 card,
>  	  then say Y. Otherwise say N.
>  
> +config MFD_IEI_WT61P803_PUZZLE
> +	tristate "iEi WT61P803 PUZZLE MCU driver"
> +	depends on SERIAL_DEV_BUS
> +	help
> +	  iEi WT61P803 PUZZLE is a system power management microcontroller
> +	  used for fan control, temperature sensor reading, led control

	                                                    LED
please.

> +	  and system identification.
> +
>  endmenu
>  endif


thanks.
-- 
~Randy

Re: [PATCH 4/7] drivers: leds: Add the iEi WT61P803 PUZZLE LED driver

[Pavel Machek]

Hi!

> Add support for the iEi WT61P803 PUZZLE LED driver.
> Currently only the front panel power LED is supported.
> 
> This driver depends on the iEi WT61P803 PUZZLE MFD driver.
> 
> Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
> Cc: Luka Perkov <luka.perkov@sartura.hr>

> +#define CMD_CHAR(x) (char)(x)

Come on... no need to hide this in macro.

> *resp_buf = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL); + + 

AFAICT you'll happily dereference NULL when kmalloc fails.

If it is small, should you just put buffer on stack?
									Pavel

Re: [PATCH 4/7] drivers: leds: Add the iEi WT61P803 PUZZLE LED driver

[Andy Shevchenko]

On Sat, Sep 5, 2020 at 4:10 PM Luka Kovacic <luka.kovacic@sartura.hr> wrote:
>
> Add support for the iEi WT61P803 PUZZLE LED driver.
> Currently only the front panel power LED is supported.
>
> This driver depends on the iEi WT61P803 PUZZLE MFD driver.

Can we make it OF independent?
See below how to achieve this.

> Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
> Cc: Luka Perkov <luka.perkov@sartura.hr>
> ---
>  drivers/leds/Kconfig                    |   8 ++
>  drivers/leds/Makefile                   |   1 +
>  drivers/leds/leds-iei-wt61p803-puzzle.c | 184 ++++++++++++++++++++++++
>  3 files changed, 193 insertions(+)
>  create mode 100644 drivers/leds/leds-iei-wt61p803-puzzle.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 1c181df24eae..8a25fb753dec 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -332,6 +332,14 @@ config LEDS_IPAQ_MICRO
>           Choose this option if you want to use the notification LED on
>           Compaq/HP iPAQ h3100 and h3600.
>
> +config LEDS_IEI_WT61P803_PUZZLE
> +       tristate "LED Support for the iEi WT61P803 PUZZLE MCU"
> +       depends on LEDS_CLASS
> +       depends on MFD_IEI_WT61P803_PUZZLE
> +       help
> +         This option enables support for LEDs controlled by the iEi WT61P803
> +         M801 MCU.
> +
>  config LEDS_HP6XX
>         tristate "LED Support for the HP Jornada 6xx"
>         depends on LEDS_CLASS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index c2c7d7ade0d0..cd362437fefd 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -34,6 +34,7 @@ obj-$(CONFIG_LEDS_HP6XX)              += leds-hp6xx.o
>  obj-$(CONFIG_LEDS_INTEL_SS4200)                += leds-ss4200.o
>  obj-$(CONFIG_LEDS_IP30)                        += leds-ip30.o
>  obj-$(CONFIG_LEDS_IPAQ_MICRO)          += leds-ipaq-micro.o
> +obj-$(CONFIG_LEDS_IEI_WT61P803_PUZZLE) += leds-iei-wt61p803-puzzle.o
>  obj-$(CONFIG_LEDS_IS31FL319X)          += leds-is31fl319x.o
>  obj-$(CONFIG_LEDS_IS31FL32XX)          += leds-is31fl32xx.o
>  obj-$(CONFIG_LEDS_KTD2692)             += leds-ktd2692.o
> diff --git a/drivers/leds/leds-iei-wt61p803-puzzle.c b/drivers/leds/leds-iei-wt61p803-puzzle.c
> new file mode 100644
> index 000000000000..50d1e4e81571
> --- /dev/null
> +++ b/drivers/leds/leds-iei-wt61p803-puzzle.c
> @@ -0,0 +1,184 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/* iEi WT61P803 PUZZLE MCU LED Driver
> + *
> + * Copyright (C) 2020 Sartura Ltd.
> + * Author: Luka Kovacic <luka.kovacic@sartura.hr>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/mfd/iei-wt61p803-puzzle.h>

> +#include <linux/of.h>
> +#include <linux/of_device.h>

of*.h -> mod_devicetable.h

> +#include <linux/platform_device.h>

+ property.h

> +#include <linux/slab.h>
> +
> +#include <linux/leds.h>
> +
> +#define CMD_CHAR(x) (char)(x)
> +
> +/**
> + * enum iei_wt61p803_puzzle_led_state - LED state values

> + *

This is not needed.

> + * @IEI_LED_OFF: The LED is turned off
> + * @IEI_LED_ON: The LED is turned on
> + * @IEI_LED_BLINK_5HZ: The LED will blink with a freq of 5 Hz
> + * @IEI_LED_BLINK_1HZ: The LED will blink with a freq of 1 Hz
> + */
> +enum iei_wt61p803_puzzle_led_state {
> +       IEI_LED_OFF = 0x30,
> +       IEI_LED_ON = 0x31,
> +       IEI_LED_BLINK_5HZ = 0x32,
> +       IEI_LED_BLINK_1HZ = 0x33

Can we leave a comma here?

> +};
> +
> +/**
> + * struct iei_wt61p803_puzzle_led - MCU LED Driver

> + *

This is not needed.

> + * @mcu: MCU struct pointer
> + * @lock: General mutex lock for LED operations
> + * @led_power_state: State of the front panel power LED
> + */
> +struct iei_wt61p803_puzzle_led {
> +       struct iei_wt61p803_puzzle *mcu;
> +       struct mutex lock;
> +
> +       int led_power_state;
> +};
> +
> +static inline struct iei_wt61p803_puzzle_led *
> +               cdev_to_iei_wt61p803_puzzle_led(struct led_classdev *led_cdev)

It's a rather strange indentation.

> +{
> +       return dev_get_drvdata(led_cdev->dev->parent);
> +}
> +
> +static int iei_wt61p803_puzzle_led_brightness_set_blocking
> +       (struct led_classdev *cdev, enum led_brightness brightness)

Ditto.

> +{
> +       struct iei_wt61p803_puzzle_led *mcu_led =
> +               cdev_to_iei_wt61p803_puzzle_led(cdev);

> +       unsigned char led_power_cmd[5] = { '@', 'R', '1',
> +               CMD_CHAR(IEI_LED_OFF) };

Ditto.

> +

This is not needed.

> +       int ret;

> +

Ditto.

> +       size_t reply_size = 0;

> +       unsigned char *resp_buf = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);

Please move the assignment closer to its use and add an error check.
Btw, how big is the buffer?

> +       mutex_lock(&mcu_led->lock);
> +
> +       if (brightness == LED_OFF) {
> +               led_power_cmd[3] = CMD_CHAR(IEI_LED_OFF);
> +               mcu_led->led_power_state = LED_OFF;
> +       } else {
> +               led_power_cmd[3] = CMD_CHAR(IEI_LED_ON);
> +               mcu_led->led_power_state = LED_ON;
> +       }
> +
> +       mutex_unlock(&mcu_led->lock);
> +
> +       ret = iei_wt61p803_puzzle_write_command(mcu_led->mcu, led_power_cmd,
> +                       sizeof(led_power_cmd), resp_buf, &reply_size);
> +
> +       kfree(resp_buf);
> +
> +       return ret;
> +}
> +
> +static enum led_brightness
> +iei_wt61p803_puzzle_led_brightness_get(struct led_classdev *cdev)
> +{
> +       struct iei_wt61p803_puzzle_led *mcu_led =
> +               cdev_to_iei_wt61p803_puzzle_led(cdev);

> +

This is not needed.

> +       int led_state;
> +
> +       mutex_lock(&mcu_led->lock);
> +       led_state = mcu_led->led_power_state;
> +       mutex_unlock(&mcu_led->lock);
> +
> +       return led_state;
> +}
> +
> +static int iei_wt61p803_puzzle_led_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev->parent);

> +       struct device_node *np = dev->of_node;

Drop this.

> +

This is not needed.

> +       struct iei_wt61p803_puzzle_led *mcu_led;

> +       struct device_node *child;

struct fwnode_handle *child;

> +

This is not needed

> +       int ret;
> +
> +       mcu_led = devm_kzalloc(dev, sizeof(*mcu_led), GFP_KERNEL);
> +       if (!mcu_led)
> +               return -ENOMEM;
> +
> +       mcu_led->mcu = mcu;
> +
> +       /* The default LED power state is 1 */
> +       mcu_led->led_power_state = 1;
> +
> +       /* Init the mutex lock */
> +       mutex_init(&mcu_led->lock);
> +
> +       dev_set_drvdata(dev, mcu_led);
> +

> +       for_each_child_of_node(np, child) {

device_for_each_child_node()

> +               struct led_classdev *led;
> +               u32 reg;
> +
> +               led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
> +               if (!led)
> +                       return -ENOMEM;
> +

> +               ret = of_property_read_u32(child, "reg", &reg);

fwnode_property_read_u32()

> +               if (ret || reg > 1) {

> +                       dev_err(dev, "Could not register 'reg' of %s\n",
> +                               child->name);

%pfw

> +                       continue;
> +               }
> +

> +               if (of_property_read_string(child, "label", &led->name))

of_ -> fwnode_

> +                       led->name = child->name;

devm_kasprintf()

See, for example, drivers/leds/leds-max77650.c for such approach.

> +
> +               of_property_read_string(child, "linux,default-trigger",
> +                               &led->default_trigger);

of_ -> fwnode_

> +
> +               led->brightness_set_blocking =
> +                       iei_wt61p803_puzzle_led_brightness_set_blocking;
> +               led->brightness_get = iei_wt61p803_puzzle_led_brightness_get;
> +
> +               led->max_brightness = 1;
> +
> +               ret = devm_led_classdev_register(dev, led);
> +               if (ret) {
> +                       dev_err(dev, "Could not register %s\n", led->name);
> +                       return ret;
> +               }
> +       }
> +
> +       return 0;

> +

This is not needed.

> +}
> +
> +static const struct of_device_id iei_wt61p803_puzzle_led_of_match[] = {
> +       { .compatible = "iei,wt61p803-puzzle-leds" },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, iei_wt61p803_puzzle_led_of_match);
> +
> +static struct platform_driver iei_wt61p803_puzzle_led_driver = {
> +       .driver = {
> +               .name = "iei-wt61p803-puzzle-led",
> +               .of_match_table = iei_wt61p803_puzzle_led_of_match,
> +       },
> +       .probe = iei_wt61p803_puzzle_led_probe,
> +};
> +module_platform_driver(iei_wt61p803_puzzle_led_driver);
> +
> +MODULE_DESCRIPTION("iEi WT61P803 PUZZLE front panel LED driver");
> +MODULE_AUTHOR("Luka Kovacic <luka.kovacic@sartura.hr>");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:leds-iei-wt61p803-puzzle");
> --
> 2.20.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/7] drivers: mfd: Add a driver for iEi WT61P803 PUZZLE MCU

[Lee Jones]

On Sat, 05 Sep 2020, Luka Kovacic wrote:

> Add a driver for the iEi WT61P803 PUZZLE microcontroller, used in some
> iEi Puzzle series devices. The microcontroller controls system power,
> temperature sensors, fans and LEDs.
> 
> This driver implements the core functionality for device communication
> over the system serial (serdev bus). It handles MCU messages and the
> internal MCU properties. Some properties can be managed over sysfs.

Do you provide documentation for the new sysfs entries?

> Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
> Cc: Luka Perkov <luka.perkov@sartura.hr>
> ---
>  drivers/mfd/Kconfig                     |    8 +
>  drivers/mfd/Makefile                    |    1 +
>  drivers/mfd/iei-wt61p803-puzzle.c       | 1242 +++++++++++++++++++++++
>  include/linux/mfd/iei-wt61p803-puzzle.h |   27 +
>  4 files changed, 1278 insertions(+)
>  create mode 100644 drivers/mfd/iei-wt61p803-puzzle.c
>  create mode 100644 include/linux/mfd/iei-wt61p803-puzzle.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 33df0837ab41..3fcda95f07a3 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2118,5 +2118,13 @@ config SGI_MFD_IOC3
>  	  If you have an SGI Origin, Octane, or a PCI IOC3 card,
>  	  then say Y. Otherwise say N.
>  
> +config MFD_IEI_WT61P803_PUZZLE
> +	tristate "iEi WT61P803 PUZZLE MCU driver"
> +	depends on SERIAL_DEV_BUS
> +	help
> +	  iEi WT61P803 PUZZLE is a system power management microcontroller
> +	  used for fan control, temperature sensor reading, led control
> +	  and system identification.
> +
>  endmenu
>  endif
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index a60e5f835283..33b88023a68d 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -236,6 +236,7 @@ obj-$(CONFIG_MFD_HI655X_PMIC)   += hi655x-pmic.o
>  obj-$(CONFIG_MFD_DLN2)		+= dln2.o
>  obj-$(CONFIG_MFD_RT5033)	+= rt5033.o
>  obj-$(CONFIG_MFD_SKY81452)	+= sky81452.o
> +obj-$(CONFIG_MFD_IEI_WT61P803_PUZZLE)	+= iei-wt61p803-puzzle.o
>  
>  intel-soc-pmic-objs		:= intel_soc_pmic_core.o intel_soc_pmic_crc.o
>  obj-$(CONFIG_INTEL_SOC_PMIC)	+= intel-soc-pmic.o
> diff --git a/drivers/mfd/iei-wt61p803-puzzle.c b/drivers/mfd/iei-wt61p803-puzzle.c
> new file mode 100644
> index 000000000000..110ea1b84c53
> --- /dev/null
> +++ b/drivers/mfd/iei-wt61p803-puzzle.c
> @@ -0,0 +1,1242 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +

Please remove this line.

> +/* iEi WT61P803 PUZZLE MCU Driver
> + *
> + * Copyright (C) 2020 Sartura Ltd.
> + * Author: Luka Kovacic <luka.kovacic@sartura.hr>
> + */
> +
> +#include <linux/atomic.h>
> +#include <linux/delay.h>
> +#include <linux/export.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/iei-wt61p803-puzzle.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/sched.h>
> +#include <linux/serdev.h>
> +#include <asm/unaligned.h>
> +#include <linux/delay.h>
> +#include <linux/sysfs.h>

Alphabetical.

> +#define IEI_WT61P803_PUZZLE_MAX_COMMAND_LENGTH (20 + 2)
> +#define IEI_WT61P803_PUZZLE_RESP_BUF_SIZE 512
> +#define IEI_WT61P803_PUZZLE_GENERAL_TIMEOUT HZ

Can you tab align the values please.

Comment required for the use of 'HZ'.

> +/**
> + * struct iei_wt61p803_puzzle_mcu_status - MCU flags state
> + *
> + * @ac_recovery_status_flag: AC Recovery Status Flag
> + * @power_loss_recovery: System recovery after power loss
> + * @power_status: System Power-on Method
> + */

Please tab out the descriptions.

> +struct iei_wt61p803_puzzle_mcu_status {
> +	u8 ac_recovery_status_flag;
> +	u8 power_loss_recovery;
> +	u8 power_status;
> +};
> +
> +/**
> + * enum iei_wt61p803_puzzle_reply_state - State of the reply
> + * @FRAME_OK: The frame was completely processed/received
> + * @FRAME_PROCESSING: First bytes were received, but the frame isn't complete
> + * @FRAME_STRUCT_EMPTY: The frame struct is empty, no data was received
> + * @FRAME_TIMEOUT: The frame processing timed out, communication failed

As above.  Just makes it easier on the eye IMHO.

> + * The enum iei_wt61p803_puzzle_reply_state is used to describe the general state

s/The enum iei_wt61p803_puzzle_reply_state is used to describe/Describes/

> + * of the frame that is currently being received.
> + */
> +enum iei_wt61p803_puzzle_reply_state {
> +	FRAME_OK = 0x00,
> +	FRAME_PROCESSING = 0x01,
> +	FRAME_STRUCT_EMPTY = 0xFF,
> +	FRAME_TIMEOUT = 0xFE
> +};
> +
> +/**
> + * struct iei_wt61p803_puzzle_reply - MCU reply
> + *
> + * @size: Size of the MCU reply
> + * @data: Full MCU reply buffer
> + * @state: Current state of the packet
> + * @received: Was the response fullfilled
> + */
> +struct iei_wt61p803_puzzle_reply {
> +	size_t size;
> +	unsigned char *data;
> +	u8 state;
> +	struct completion received;
> +};
> +
> +/**
> + * struct iei_wt61p803_puzzle_mcu_version - MCU version status
> + *
> + * @version: Primary firmware version
> + * @build_info: Build date and time
> + * @bootloader_mode: Status of the MCU operation
> + * @protocol_version: MCU communication protocol version
> + * @serial_number: Device factory serial number
> + * @mac_address: Device factory MAC addresses
> + */
> +struct iei_wt61p803_puzzle_mcu_version {
> +	const char *version;
> +	const char *build_info;
> +	bool bootloader_mode;
> +	const char *protocol_version;
> +	const char *serial_number;
> +	const char *mac_address[8];
> +};
> +
> +/**
> + * struct iei_wt61p803_puzzle - iEi WT61P803 PUZZLE MCU Driver
> + *
> + * @serdev: Pointer to underlying serdev device
> + * @kobj: Pointer to kobject (sysfs)
> + * @reply_lock: Reply mutex lock
> + * @bus_lock: Bus mutex lock
> + * @reply: Pointer to the iei_wt61p803_puzzle_reply struct
> + * @version_lock: Version R/W mutex lock
> + * @version: Pointer to the iei_wt61p803_puzzle_mcu_version struct
> + * @status_lock: Status R/W mutex lock
> + * @status: Pointer to the iei_wt61p803_puzzle_mcu_status struct
> + */
> +struct iei_wt61p803_puzzle {
> +	struct serdev_device *serdev;
> +	struct kobject *kobj;
> +
> +	struct mutex reply_lock;
> +	struct mutex bus_lock;
> +
> +	struct iei_wt61p803_puzzle_reply *reply;
> +
> +	struct mutex version_lock;
> +	struct iei_wt61p803_puzzle_mcu_version *version;
> +
> +	struct mutex status_lock;
> +	struct iei_wt61p803_puzzle_mcu_status *status;
> +};

Why do you need so many locks?

Are they intertwined?

Any reason why you can't just use one?

> +static const struct of_device_id iei_wt61p803_puzzle_dt_ids[] = {
> +	{ .compatible = "iei,wt61p803-puzzle" },
> +	{ /* sentinel */ }

Please remove this comment.

> +};

Move this to just above 'struct serdev_device_driver'.

> +unsigned char iei_wt61p803_puzzle_add_xor_checksum(unsigned char *buf,

'iei_wt61p803_puzzle_checksum' will be fine.

> +		unsigned char xor_len)

'len'

> +{
> +	unsigned char xor_sum = 0;

'checksum'

> +	unsigned int i;
> +
> +	for (i = 0; i < xor_len; i++)
> +		xor_sum ^= buf[i];
> +
> +	return xor_sum;
> +}
> +
> +static int iei_wt61p803_puzzle_process_resp(struct iei_wt61p803_puzzle *mcu,

Could you change all instances of 'mcu' to 'ddata' please.

> +		unsigned char *raw_resp_data, size_t size)
> +{
> +	struct device *dev = &mcu->serdev->dev;
> +
> +	unsigned char xor_checksum;
> +
> +	/*
> +	 * Start receiving the frame/Continue receiving the frame

	 * Start/continue receiving the frame

> +	 *
> +	 * The code below determines whether:

"the code below" is assumed.

> +	 * * we are receiving a new frame
> +	 * * appending data to an existing frame
> +	 *
> +	 */

Is this comment really required?

> +	/* Lock the reply struct mutex */

This comment is definitely not required.

> +	mutex_lock(&mcu->reply_lock);
> +
> +	/* Check the incoming frame header */
> +	if (!(raw_resp_data[0] == '@' || raw_resp_data[0] == '%' ||
> +		(raw_resp_data[0] == 0xF7 && raw_resp_data[1] == 0xA1))) {

Please define *all* magic numbers.

> +		/* Frame header is not correct, check whether to append */
> +		if (mcu->reply->state != FRAME_PROCESSING) {
> +			/* The incoming frame should be discarded */

Again, this really isn't required.  We can see the error return.

> +			dev_err(dev, "Invalid frame header and state (0x%x)",
> +					mcu->reply->state);
> +
> +			mutex_unlock(&mcu->reply_lock);
> +			return -1;

Are you sure you want to return -EPERM?  I suggest not.

Please use properly defined Linux error codes throughout:

 https://www-numi.fnal.gov/offline_software/srt_public_context/WebDocs/Errors/unix_system_errors.html

> +		}
> +
> +		/* The frame should be appended to the existing data */

Tell the code what to do:

 "Append the frame to existing data"

> +		memcpy(mcu->reply->data+mcu->reply->size, raw_resp_data, size);
> +		mcu->reply->size += size;
> +	} else {
> +		/* Start processing a new frame */
> +		memcpy(mcu->reply->data, raw_resp_data, size);
> +		mcu->reply->size = size;
> +		mcu->reply->state = FRAME_PROCESSING;
> +	}
> +
> +	/* Create an xor checksum of the data */

Please remove.  Code is clear enough.

> +	xor_checksum = iei_wt61p803_puzzle_add_xor_checksum(mcu->reply->data,

Just 'checksum' will do.

> +			((int)mcu->reply->size)-1);

Why are you casting to a function you authored?

> +
> +	/* Check whether the caluclated checksum matches the received one */

There's wayyyyy too much commenting happening here.  You only need to
provide comments for points which would otherwise be unclear.

> +	if (xor_checksum != mcu->reply->data[((int)mcu->reply->size)-1]) {

Remove all of these casts please - no need.

> +		/* The checksum doesn't match, waiting for data */

No error messages to that effect?

> +		mutex_unlock(&mcu->reply_lock);
> +		return (int)size;
> +	}
> +
> +	/*
> +	 * Update internal driver state with the latest response code
> +	 */
> +
> +	/* Received all the data */
> +	mcu->reply->state = FRAME_OK;
> +	complete(&mcu->reply->received);
> +
> +	mutex_unlock(&mcu->reply_lock);
> +
> +	return (int)size;
> +}
> +
> +static int iei_wt61p803_puzzle_recv_buf(struct serdev_device *serdev,
> +		const unsigned char *data, size_t size)
> +{
> +	struct iei_wt61p803_puzzle *mcu = serdev_device_get_drvdata(serdev);
> +	int ret;
> +
> +	ret = iei_wt61p803_puzzle_process_resp(mcu, (unsigned char *)data, size);
> +
> +	/* Return the number of processed bytes if function returns error */
> +	if (ret < 0)
> +		return (int)size;
> +
> +	return ret;
> +}
> +
> +static const struct serdev_device_ops iei_wt61p803_puzzle_serdev_device_ops = {
> +	.receive_buf  = iei_wt61p803_puzzle_recv_buf,
> +	.write_wakeup = serdev_device_write_wakeup,
> +};
> +
> +/**
> + * iei_wt61p803_puzzle_write_command_watchdog() - Watchdog of the normal cmd
> + * @mcu: Pointer to the iei_wt61p803_puzzle core MFD struct
> + * @cmd: Pointer to the char array to send (size should be content + 1 (xor))
> + * @size: Size of the cmd char array
> + * @reply_data: Pointer to the reply/response data array (should be allocated)
> + * @reply_size: Pointer to size_t (size of reply_data)
> + * @retry_count: Number of times to retry sending the command to the MCU
> + */
> +int iei_wt61p803_puzzle_write_command_watchdog(struct iei_wt61p803_puzzle *mcu,
> +		unsigned char *cmd, size_t size, unsigned char *reply_data,
> +		size_t *reply_size, int retry_count)
> +{
> +	struct device *dev = &mcu->serdev->dev;
> +	int ret, i;
> +
> +	for (i = 0; i < retry_count; i++) {
> +		ret = iei_wt61p803_puzzle_write_command(mcu, cmd, size,
> +				reply_data, reply_size);
> +
> +		if (ret != -ETIMEDOUT)
> +			return ret;
> +	}
> +
> +	dev_err(dev, "%s: Command response timed out. Retries: %d", __func__,
> +			retry_count);
> +
> +	return -ETIMEDOUT;
> +}
> +EXPORT_SYMBOL_GPL(iei_wt61p803_puzzle_write_command_watchdog);
> +
> +/**
> + * iei_wt61p803_puzzle_write_command() - Send a structured command to the MCU
> + * @mcu: Pointer to the iei_wt61p803_puzzle core MFD struct
> + * @cmd: Pointer to the char array to send (size should be content + 1 (xor))
> + * @size: Size of the cmd char array
> + * @reply_data: Pointer to the reply/response data array (should be allocated)
> + *
> + * Function iei_wt61p803_puzzle_write_command() sends a structured command to
> + * the MCU.
> + */
> +int iei_wt61p803_puzzle_write_command(struct iei_wt61p803_puzzle *mcu,
> +		unsigned char *cmd, size_t size, unsigned char *reply_data,
> +		size_t *reply_size)
> +{
> +	struct device *dev = &mcu->serdev->dev;
> +
> +	int ret;
> +	int len = (int)size;
> +
> +	if (size > IEI_WT61P803_PUZZLE_MAX_COMMAND_LENGTH)
> +		return -EINVAL;
> +
> +	/* Create an XOR checksum for the provided command */
> +	cmd[len - 1] = iei_wt61p803_puzzle_add_xor_checksum(cmd, len);
> +
> +	mutex_lock(&mcu->bus_lock);
> +
> +	/* Initialize reply struct */
> +	mutex_lock(&mcu->reply_lock);
> +	if (mcu->reply) {
> +		reinit_completion(&mcu->reply->received);
> +
> +		mcu->reply->state = FRAME_STRUCT_EMPTY;
> +
> +		mcu->reply->size = 0;
> +	} else {
> +		dev_err(dev, "Reply struct is NULL.\n");

This doesn't mean anything.

Please state what happened.  'Why' is the struct NULL?

> +		mutex_unlock(&mcu->reply_lock);
> +		mutex_unlock(&mcu->bus_lock);

Improve the error handling with 'goto's.

Move the unlocks to the bottom and 'goto' them instead of placing
unlocks in each error handler.

> +		return -1;
> +	}

Just check for '!mcu->reply'.

Break the 'reinit' code out of the 'if ()'

> +	mutex_unlock(&mcu->reply_lock);
> +
> +	/* Write to MCU UART */
> +	ret = serdev_device_write(mcu->serdev, cmd, len,
> +			IEI_WT61P803_PUZZLE_GENERAL_TIMEOUT);
> +

Remove this line.

> +	if (ret < 0)
> +		return ret;

Isn't the bus_lock still taken?

> +	/* Wait for the MCU response */
> +	if (!wait_for_completion_timeout(&mcu->reply->received,
> +				IEI_WT61P803_PUZZLE_GENERAL_TIMEOUT)) {
> +		dev_err(dev, "Command reply receive timeout\n");
> +
> +		ret = -ETIMEDOUT;
> +
> +		mutex_lock(&mcu->reply_lock);
> +		reinit_completion(&mcu->reply->received);
> +		mcu->reply->state = FRAME_TIMEOUT;
> +		mutex_unlock(&mcu->reply_lock);
> +	}
> +
> +	if (ret < 0) {
> +		mutex_unlock(&mcu->bus_lock);
> +		return ret;
> +	}

Why is this out here and not handled inside the 'if ()' above?

> +	/* Verify MCU response status */
> +	mutex_lock(&mcu->reply_lock);
> +
> +	if (mcu->reply) {
> +		/* Copy response */
> +		*reply_size = mcu->reply->size;
> +		memcpy(reply_data, mcu->reply->data, mcu->reply->size);
> +	} else {
> +		dev_err(dev, "Reply struct is NULL.\n");
> +
> +		mutex_unlock(&mcu->reply_lock);
> +		mutex_unlock(&mcu->bus_lock);
> +
> +		return -1;
> +	}

Same comments as above.

> +	mutex_unlock(&mcu->reply_lock);
> +	mutex_unlock(&mcu->bus_lock);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(iei_wt61p803_puzzle_write_command);
> +
> +int iei_wt61p803_puzzle_buzzer(struct iei_wt61p803_puzzle *mcu, bool long_beep)
> +{
> +	/* Buzzer 0.5 sec */

Place this at the end of the line.

> +	unsigned char buzzer_short_cmd[4] = { '@', 'C', '2' };
> +	/* Buzzer 1.5 sec */

As above.

> +	unsigned char buzzer_long_cmd[4] = { '@', 'C', '3' };
> +

Remove this line.

> +	int ret;
> +

Remove this line.

> +	size_t reply_size = 0;
> +	unsigned char *resp_buf;
> +
> +	resp_buf = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);

What's the difference between this and 'mcu->reply->data'?

> +	if (!resp_buf)
> +		return -ENOMEM;
> +
> +	ret = iei_wt61p803_puzzle_write_command(mcu,
> +			long_beep ? buzzer_long_cmd : buzzer_short_cmd, 4,

Define all magic numbers.

Do this throughout the code.

> +			resp_buf, &reply_size);
> +
> +	if (ret) {
> +		kfree(resp_buf);

Use 'goto's so you don't have to free in each handler.

Do this throughout the code.

> +		return ret;
> +	}
> +
> +	if (reply_size != 3) {
> +		kfree(resp_buf);
> +		return -EIO;
> +	}
> +
> +	if (!(resp_buf[0] == '@' && resp_buf[1] == '0' &&
> +				resp_buf[2] == 0x70)) {
> +		kfree(resp_buf);
> +		return -EPROTO;
> +	}
> +
> +	kfree(resp_buf);
> +	return 0;
> +}
> +
> +int iei_wt61p803_puzzle_get_version(struct iei_wt61p803_puzzle *mcu)
> +{
> +	struct device *dev = &mcu->serdev->dev;
> +
> +	/* Commands */

Not needed.

> +	unsigned char version_cmd[3] = { '%', 'V' };
> +	unsigned char build_info_cmd[3] = { '%', 'B' };
> +	unsigned char bootloader_mode_cmd[3] = { '%', 'M' };
> +	unsigned char protocol_version_cmd[3] = { '%', 'P' };
> +

Remove empty lines between declarations.

Do this throughout the code.

> +	unsigned char *rd;
> +	size_t reply_size = 0;
> +
> +	int ret;
> +
> +	/* Allocate memory for the buffer */
> +	rd = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
> +	if (!rd)
> +		return -ENOMEM;
> +
> +	ret = iei_wt61p803_puzzle_write_command(mcu, version_cmd,
> +			sizeof(version_cmd), rd, &reply_size);
> +	if (ret)
> +		goto err;
> +
> +	if (reply_size < 7) {
> +		ret = -EIO;
> +		goto err;
> +	}
> +
> +	mcu->version->version = devm_kasprintf(dev, GFP_KERNEL, "v%c.%c%c%c",
> +				rd[2], rd[3], rd[4], rd[5]);

Please remove empty lines for each group.  Like:

	ret = iei_wt61p803_puzzle_write_command(mcu, version_cmd,
			sizeof(version_cmd), rd, &reply_size);
	if (ret)
		goto err;
	if (reply_size < 7) {
		ret = -EIO;
		goto err;
	}
	mcu->version->version = devm_kasprintf(dev, GFP_KERNEL, "v%c.%c%c%c",
				rd[2], rd[3], rd[4], rd[5]);

> +	ret = iei_wt61p803_puzzle_write_command(mcu, build_info_cmd,
> +			sizeof(build_info_cmd), rd, &reply_size);
> +	if (ret)
> +		goto err;
> +
> +	if (reply_size < 15) {
> +		ret = -EIO;
> +		goto err;
> +	}
> +
> +	mcu->version->build_info = devm_kasprintf(dev, GFP_KERNEL,
> +			"%c%c/%c%c/%c%c%c%c %c%c:%c%c",
> +			rd[8], rd[9], rd[6], rd[7], rd[2],
> +			rd[3], rd[4], rd[5], rd[10], rd[11],
> +			rd[12], rd[13]);
> +
> +	ret = iei_wt61p803_puzzle_write_command(mcu, bootloader_mode_cmd,
> +			sizeof(bootloader_mode_cmd), rd, &reply_size);
> +	if (ret)
> +		goto err;
> +
> +	if (reply_size < 4) {
> +		ret = -EIO;
> +		goto err;
> +	}
> +
> +	if (rd[2] == 0x31) {
> +		/* The MCU is in normal mode */
> +		mcu->version->bootloader_mode = false;
> +	} else if (rd[2] == 0x30) {
> +		/* The MCU is in bootloader mode */
> +		mcu->version->bootloader_mode = true;
> +	}
> +
> +	ret = iei_wt61p803_puzzle_write_command(mcu, protocol_version_cmd,
> +			sizeof(protocol_version_cmd), rd, &reply_size);
> +	if (ret)
> +		goto err;
> +
> +	if (reply_size < 9) {
> +		ret = -EIO;
> +		goto err;
> +	}
> +
> +	mcu->version->protocol_version = devm_kasprintf(dev, GFP_KERNEL,
> +			"v%c.%c%c%c%c%c",
> +			rd[7], rd[6], rd[5], rd[4], rd[3], rd[2]);
> +
> +	/* Free the allocated memory resources */
> +	kfree(rd);
> +
> +	return 0;

No need to kfree/return twice.

Remove the 5 lines above.

> +err:
> +	kfree(rd);
> +	return ret;
> +}
> +
> +int iei_wt61p803_puzzle_get_mcu_status(struct iei_wt61p803_puzzle *mcu)
> +{
> +	/* MCU Status Command */
> +	unsigned char mcu_status_cmd[5] = { '@', 'O', 'S', 'S' };
> +
> +	int ret;
> +
> +	unsigned char *rd;
> +	size_t reply_size = 0;
> +
> +	/* Allocate memory for the buffer */
> +	rd = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
> +	if (!rd)
> +		return -ENOMEM;
> +
> +	ret = iei_wt61p803_puzzle_write_command(mcu, mcu_status_cmd,
> +			sizeof(mcu_status_cmd), rd, &reply_size);
> +	if (ret) {
> +		kfree(rd);
> +		return ret;
> +	}
> +
> +	if (reply_size < 20) {
> +		kfree(rd);
> +		return -EIO;
> +	}
> +
> +	/* Response format:
> +	 * (IDX	RESPONSE)
> +	 * 0	@
> +	 * 1	O
> +	 * 2	S
> +	 * 3	S
> +	 * ...
> +	 * 5	AC Recovery Status Flag
> +	 * ...
> +	 * 10	Power Loss Recovery
> +	 * ...
> +	 * 19	Power Status (system power on method)
> +	 * 20	XOR checksum
> +	 */
> +
> +	if (rd[0] == '@' && rd[1] == 'O' && rd[2] == 'S' && rd[3] == 'S') {
> +		mutex_lock(&mcu->status_lock);
> +		mcu->status->ac_recovery_status_flag = rd[5];
> +		mcu->status->power_loss_recovery = rd[10];
> +		mcu->status->power_status = rd[19];
> +		mutex_unlock(&mcu->status_lock);
> +	}
> +
> +	/* Free the allocated memory resources */
> +	kfree(rd);
> +
> +	return 0;
> +}
> +
> +int iei_wt61p803_puzzle_get_serial_number(struct iei_wt61p803_puzzle *mcu)
> +{
> +	struct device *dev = &mcu->serdev->dev;
> +	unsigned char serial_number_cmd[5] = { 0xF7, 0xA1, 0x00, 0x24 };
> +
> +	int ret;
> +
> +	unsigned char *rd;
> +	size_t reply_size = 0;
> +
> +	/* Allocate memory for the buffer */
> +	rd = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
> +	if (!rd)
> +		return -ENOMEM;

Allocate this once in .probe().

No need to keep allocating and freeing each time.

> +	ret = iei_wt61p803_puzzle_write_command(mcu, serial_number_cmd,
> +			sizeof(serial_number_cmd), rd, &reply_size);
> +	if (ret)
> +		goto err;
> +
> +	mutex_lock(&mcu->version_lock);
> +	mcu->version->serial_number = devm_kasprintf(dev, GFP_KERNEL, "%.*s",
> +			(int)reply_size - 5, rd + 4);
> +	mutex_unlock(&mcu->version_lock);
> +
> +	/* Free the allocated memory resources */
> +	kfree(rd);
> +
> +	return 0;
> +
> +err:
> +	kfree(rd);
> +	return ret;
> +

Remove all unnecessary blank lines.

> +}
> +
> +int iei_wt61p803_puzzle_write_serial_number(struct iei_wt61p803_puzzle *mcu,
> +		unsigned char serial_number[36])
> +{
> +	struct device *dev = &mcu->serdev->dev;
> +	unsigned char serial_number_header[4] = { 0xF7, 0xA0, 0x00, 0xC };
> +
> +	/* 4 byte header, 12 byte serial number chunk, 1 byte XOR checksum */
> +	unsigned char serial_number_cmd[4+12+1];
> +
> +	int ret, sn_counter;
> +
> +	unsigned char *rd;
> +	size_t reply_size = 0;
> +
> +	/* Allocate memory for the buffer */
> +	rd = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
> +	if (!rd)
> +		return -ENOMEM;
> +
> +	/* The MCU can only handle 22 byte messages, send the S/N in chunks */
> +	for (sn_counter = 0; sn_counter < 3; sn_counter++) {
> +		serial_number_header[2] = 0x0 + (0xC) * sn_counter;
> +
> +		memcpy(serial_number_cmd, serial_number_header, 4);
> +		memcpy(serial_number_cmd + 4, serial_number + (0xC) * sn_counter, 0xC);
> +
> +		/* Initialize the last byte */
> +		serial_number_cmd[sizeof(serial_number_cmd) - 1] = 0;
> +
> +		ret = iei_wt61p803_puzzle_write_command(mcu, serial_number_cmd,
> +				sizeof(serial_number_cmd), rd, &reply_size);
> +		if (ret)
> +			goto err;
> +
> +		if (reply_size != 3) {
> +			ret = -EIO;
> +			goto err;
> +		}
> +
> +		if (!(rd[0] == '@' && rd[1] == '0' && rd[2] == 0x70)) {
> +			ret = -EPROTO;
> +			goto err;
> +		}
> +	}
> +
> +	mutex_lock(&mcu->version_lock);
> +	mcu->version->serial_number = devm_kasprintf(dev, GFP_KERNEL, "%.*s",
> +			36, serial_number);
> +	mutex_unlock(&mcu->version_lock);
> +
> +	/* Free the allocated memory resources */
> +	kfree(rd);
> +
> +	return 0;
> +
> +err:
> +	kfree(rd);
> +	return ret;
> +}

Same comments as above.

> +int iei_wt61p803_puzzle_get_mac_addresses(struct iei_wt61p803_puzzle *mcu)
> +{
> +	struct device *dev = &mcu->serdev->dev;
> +	unsigned char mac_address_cmd[5] = { 0xF7, 0xA1, 0x00, 0x11 };
> +
> +	int ret, mac_counter;
> +
> +	unsigned char *rd;
> +	size_t reply_size = 0;
> +
> +	/* Allocate memory for the buffer */
> +	rd = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
> +	if (!rd)
> +		return -ENOMEM;
> +
> +	for (mac_counter = 0; mac_counter < 8; mac_counter++) {
> +		mac_address_cmd[2] = 0x24 + (0x11) * mac_counter;
> +		mac_address_cmd[4] = 0x00;
> +
> +		ret = iei_wt61p803_puzzle_write_command(mcu, mac_address_cmd,
> +				sizeof(mac_address_cmd), rd, &reply_size);
> +		if (ret)
> +			continue;
> +
> +		if (reply_size < 22) {
> +			ret = -EIO;
> +			goto err;
> +		}
> +
> +		mutex_lock(&mcu->version_lock);
> +		mcu->version->mac_address[mac_counter] = devm_kasprintf(dev,
> +				GFP_KERNEL, "%.*s", (int)reply_size - 5,
> +				rd + 4);
> +		mutex_unlock(&mcu->version_lock);
> +	}
> +
> +	/* Free the allocated memory resources */
> +	kfree(rd);
> +
> +	return 0;
> +
> +err:
> +	kfree(rd);
> +	return ret;
> +}

Same comments as above.

> +int iei_wt61p803_puzzle_write_mac_address(struct iei_wt61p803_puzzle *mcu,
> +		unsigned char mac_address[17], int mac_address_idx)
> +{
> +	struct device *dev = &mcu->serdev->dev;
> +	unsigned char mac_address_header[4] = { 0xF7, 0xA0, 0x00, 0x11 };
> +
> +	/* 4 byte header, 17 byte MAC address, 1 byte XOR checksum */
> +	unsigned char mac_address_cmd[4+17+1];
> +
> +	int ret;
> +
> +	unsigned char *rd;
> +	size_t reply_size = 0;
> +
> +	if (!(mac_address_idx < 8))
> +		return -EINVAL;
> +
> +	/* Allocate memory for the buffer */
> +	rd = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
> +	if (!rd)
> +		return -ENOMEM;
> +
> +	mac_address_header[2] = 0x24 + (0x11) * mac_address_idx;
> +
> +	/* Concat mac_address_header, mac_address to mac_address_cmd */
> +	memcpy(mac_address_cmd, mac_address_header, 4);
> +	memcpy(mac_address_cmd + 4, mac_address, 17);
> +
> +	/* Initialize the last byte */
> +	mac_address_cmd[sizeof(mac_address_cmd) - 1] = 0;
> +
> +	ret = iei_wt61p803_puzzle_write_command(mcu, mac_address_cmd,
> +			sizeof(mac_address_cmd), rd, &reply_size);
> +	if (ret)
> +		goto err;
> +
> +	if (reply_size != 3) {
> +		ret = -EIO;
> +		goto err;
> +	}
> +
> +	if (!(rd[0] == '@' && rd[1] == '0' && rd[2] == 0x70)) {
> +		ret = -EPROTO;
> +		goto err;
> +	}
> +
> +	mutex_lock(&mcu->version_lock);
> +	mcu->version->mac_address[mac_address_idx] = devm_kasprintf(dev,
> +			GFP_KERNEL, "%.*s", 17, mac_address);
> +	mutex_unlock(&mcu->version_lock);
> +
> +	/* Free the allocated memory resources */
> +	kfree(rd);
> +
> +	return 0;
> +
> +err:
> +	kfree(rd);
> +	return ret;
> +}

Same comments as above.

> +int iei_wt61p803_puzzle_write_power_loss_recovery(struct iei_wt61p803_puzzle *mcu,
> +		int power_loss_recovery_action)
> +{
> +	unsigned char power_loss_recovery_cmd[5] = { '@', 'O', 'A', '0' };
> +
> +	unsigned char *resp_buf;
> +	size_t reply_size = 0;
> +
> +	/* Buffer for the power_loss_recovery_action to character */
> +	unsigned char cmd_buf[2];
> +
> +	int ret;
> +
> +	/* Allocate memory for the buffer */
> +	resp_buf = kmalloc(IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
> +	if (!resp_buf)
> +		return -ENOMEM;
> +
> +	/* Check the acceptable range */
> +	if (power_loss_recovery_action < 0 || power_loss_recovery_action > 4) {
> +		kfree(resp_buf);
> +		return -EINVAL;
> +	}
> +
> +	/* Convert int to char */
> +	ret = snprintf(cmd_buf, sizeof(cmd_buf), "%d",
> +			power_loss_recovery_action);
> +	if (ret < 0) {
> +		kfree(resp_buf);
> +		return ret;
> +	}
> +
> +	/* Modify the command with the action index */
> +	power_loss_recovery_cmd[3] = cmd_buf[0];
> +
> +	ret = iei_wt61p803_puzzle_write_command(mcu, power_loss_recovery_cmd,
> +			sizeof(power_loss_recovery_cmd), resp_buf, &reply_size);
> +	if (ret) {
> +		kfree(resp_buf);
> +		return ret;
> +	}
> +
> +	/* Update the internal status (struct) */
> +	mutex_lock(&mcu->status_lock);
> +	mcu->status->power_loss_recovery = power_loss_recovery_action;
> +	mutex_unlock(&mcu->status_lock);
> +
> +	/* Free the allocated memory resources */
> +	kfree(resp_buf);
> +
> +	return 0;
> +}

Same comments as above.

> +
> +

Remove this line.

> +#define sysfs_container(dev) \
> +	(container_of((dev)->kobj.parent, struct device, kobj))

What device is this fetching?

> +static ssize_t version_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct device *dev_container = sysfs_container(dev);
> +	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
> +
> +	return sprintf(buf, "%s\n", mcu->version->version);
> +}
> +

Remove this line.

Same throughout.

> +static DEVICE_ATTR_RO(version);
> +
> +static ssize_t build_info_show(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	struct device *dev_container = sysfs_container(dev);
> +	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
> +
> +	return sprintf(buf, "%s\n", mcu->version->build_info);
> +}
> +
> +static DEVICE_ATTR_RO(build_info);
> +
> +static ssize_t bootloader_mode_show(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	struct device *dev_container = sysfs_container(dev);
> +	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
> +
> +	return sprintf(buf, "%d\n", mcu->version->bootloader_mode);
> +}
> +
> +static DEVICE_ATTR_RO(bootloader_mode);
> +
> +static ssize_t protocol_version_show(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	struct device *dev_container = sysfs_container(dev);
> +	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
> +
> +	return sprintf(buf, "%s\n", mcu->version->protocol_version);
> +}
> +
> +static DEVICE_ATTR_RO(protocol_version);
> +
> +static ssize_t serial_number_show(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	struct device *dev_container = sysfs_container(dev);
> +	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
> +
> +	int ret;
> +
> +	mutex_lock(&mcu->version_lock);
> +	ret = sprintf(buf, "%s\n", mcu->version->serial_number);
> +	mutex_unlock(&mcu->version_lock);
> +
> +	return ret;
> +}
> +
> +static ssize_t serial_number_store(struct device *dev,
> +		struct device_attribute *attr,
> +		const char *buf, size_t count)
> +{
> +	struct device *dev_container = sysfs_container(dev);
> +	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
> +
> +	unsigned char serial_number[36];
> +	int ret;
> +
> +	/* Check whether the serial number is 36 characters long + null */
> +	if ((int)count != 36 + 1)
> +		return -EINVAL;
> +
> +	/* Copy 36 characters from the buffer to serial_number */
> +	memcpy(serial_number, (unsigned char *)buf, 36);
> +
> +	ret = iei_wt61p803_puzzle_write_serial_number(mcu, serial_number);
> +	if (ret)
> +		return ret;
> +
> +	return count;
> +}
> +
> +static DEVICE_ATTR_RW(serial_number);
> +
> +static ssize_t mac_address_show(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	struct device *dev_container = sysfs_container(dev);
> +	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
> +
> +	int ret;
> +
> +	mutex_lock(&mcu->version_lock);
> +
> +	if (!strcmp(attr->attr.name, "mac_address_0"))
> +		ret = sprintf(buf, "%s\n", mcu->version->mac_address[0]);
> +	else if (!strcmp(attr->attr.name, "mac_address_1"))
> +		ret = sprintf(buf, "%s\n", mcu->version->mac_address[1]);
> +	else if (!strcmp(attr->attr.name, "mac_address_2"))
> +		ret = sprintf(buf, "%s\n", mcu->version->mac_address[2]);
> +	else if (!strcmp(attr->attr.name, "mac_address_3"))
> +		ret = sprintf(buf, "%s\n", mcu->version->mac_address[3]);
> +	else if (!strcmp(attr->attr.name, "mac_address_4"))
> +		ret = sprintf(buf, "%s\n", mcu->version->mac_address[4]);
> +	else if (!strcmp(attr->attr.name, "mac_address_5"))
> +		ret = sprintf(buf, "%s\n", mcu->version->mac_address[5]);
> +	else if (!strcmp(attr->attr.name, "mac_address_6"))
> +		ret = sprintf(buf, "%s\n", mcu->version->mac_address[6]);
> +	else if (!strcmp(attr->attr.name, "mac_address_7"))
> +		ret = sprintf(buf, "%s\n", mcu->version->mac_address[7]);
> +	else
> +		ret = sprintf(buf, "\n");
> +
> +	mutex_unlock(&mcu->version_lock);
> +
> +	return ret;
> +}
> +
> +static ssize_t mac_address_store(struct device *dev,
> +		struct device_attribute *attr,
> +		const char *buf, size_t count)
> +{
> +	struct device *dev_container = sysfs_container(dev);
> +	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
> +
> +	unsigned char mac_address[17];
> +	int ret;
> +
> +	/* Check whether the MAC address is 17 characters long + null */
> +	if ((int)count != 17 + 1)
> +		return -EINVAL;
> +
> +	/* Copy 17 characters from the buffer to mac_address */
> +	memcpy(mac_address, (unsigned char *)buf, 17);
> +
> +	if (!strcmp(attr->attr.name, "mac_address_0"))
> +		ret = iei_wt61p803_puzzle_write_mac_address(mcu, mac_address, 0);
> +	else if (!strcmp(attr->attr.name, "mac_address_1"))
> +		ret = iei_wt61p803_puzzle_write_mac_address(mcu, mac_address, 1);
> +	else if (!strcmp(attr->attr.name, "mac_address_2"))
> +		ret = iei_wt61p803_puzzle_write_mac_address(mcu, mac_address, 2);
> +	else if (!strcmp(attr->attr.name, "mac_address_3"))
> +		ret = iei_wt61p803_puzzle_write_mac_address(mcu, mac_address, 3);
> +	else if (!strcmp(attr->attr.name, "mac_address_4"))
> +		ret = iei_wt61p803_puzzle_write_mac_address(mcu, mac_address, 4);
> +	else if (!strcmp(attr->attr.name, "mac_address_5"))
> +		ret = iei_wt61p803_puzzle_write_mac_address(mcu, mac_address, 5);
> +	else if (!strcmp(attr->attr.name, "mac_address_6"))
> +		ret = iei_wt61p803_puzzle_write_mac_address(mcu, mac_address, 6);
> +	else if (!strcmp(attr->attr.name, "mac_address_7"))
> +		ret = iei_wt61p803_puzzle_write_mac_address(mcu, mac_address, 7);
> +
> +	if (ret)
> +		return ret;
> +
> +	return count;
> +}
> +
> +static DEVICE_ATTR(mac_address_0, 0644, mac_address_show, mac_address_store);
> +static DEVICE_ATTR(mac_address_1, 0644, mac_address_show, mac_address_store);
> +static DEVICE_ATTR(mac_address_2, 0644, mac_address_show, mac_address_store);
> +static DEVICE_ATTR(mac_address_3, 0644, mac_address_show, mac_address_store);
> +static DEVICE_ATTR(mac_address_4, 0644, mac_address_show, mac_address_store);
> +static DEVICE_ATTR(mac_address_5, 0644, mac_address_show, mac_address_store);
> +static DEVICE_ATTR(mac_address_6, 0644, mac_address_show, mac_address_store);
> +static DEVICE_ATTR(mac_address_7, 0644, mac_address_show, mac_address_store);
> +
> +static ssize_t ac_recovery_status_show(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	struct device *dev_container = sysfs_container(dev);
> +	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
> +
> +	int ret;
> +
> +	ret = iei_wt61p803_puzzle_get_mcu_status(mcu);
> +	if (ret)
> +		return ret;
> +
> +	mutex_lock(&mcu->status_lock);
> +	ret = sprintf(buf, "%x\n", mcu->status->ac_recovery_status_flag);
> +	mutex_unlock(&mcu->status_lock);
> +
> +	return ret;
> +}
> +
> +static DEVICE_ATTR_RO(ac_recovery_status);
> +
> +static ssize_t power_loss_recovery_show(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	struct device *dev_container = sysfs_container(dev);
> +	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
> +
> +	int ret;
> +
> +	ret = iei_wt61p803_puzzle_get_mcu_status(mcu);
> +	if (ret)
> +		return ret;
> +
> +	mutex_lock(&mcu->status_lock);
> +	ret = sprintf(buf, "%x\n", mcu->status->power_loss_recovery);
> +	mutex_unlock(&mcu->status_lock);
> +
> +	return ret;
> +}
> +
> +static ssize_t power_loss_recovery_store(struct device *dev,
> +		struct device_attribute *attr,
> +		const char *buf, size_t count)
> +{
> +	struct device *dev_container = sysfs_container(dev);
> +	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
> +
> +	int ret;
> +	long power_loss_recovery_action = 0;
> +
> +	ret = kstrtol(buf, 10, &power_loss_recovery_action);
> +	if (ret)
> +		return ret;
> +
> +	ret = iei_wt61p803_puzzle_write_power_loss_recovery(mcu,
> +			(int)power_loss_recovery_action);
> +	if (ret)
> +		return ret;
> +
> +	return count;
> +}
> +
> +static DEVICE_ATTR_RW(power_loss_recovery);
> +
> +static ssize_t power_status_show(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	struct device *dev_container = sysfs_container(dev);
> +	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
> +
> +	int ret;
> +
> +	ret = iei_wt61p803_puzzle_get_mcu_status(mcu);
> +	if (ret)
> +		return ret;
> +
> +	mutex_lock(&mcu->status_lock);
> +	ret = sprintf(buf, "%x\n", mcu->status->power_status);
> +	mutex_unlock(&mcu->status_lock);
> +
> +	return ret;
> +}
> +
> +static DEVICE_ATTR_RO(power_status);
> +
> +static struct attribute *iei_wt61p803_puzzle_attrs[] = {
> +	&dev_attr_version.attr,
> +	&dev_attr_build_info.attr,
> +	&dev_attr_bootloader_mode.attr,
> +	&dev_attr_protocol_version.attr,
> +	&dev_attr_serial_number.attr,
> +	&dev_attr_mac_address_0.attr,
> +	&dev_attr_mac_address_1.attr,
> +	&dev_attr_mac_address_2.attr,
> +	&dev_attr_mac_address_3.attr,
> +	&dev_attr_mac_address_4.attr,
> +	&dev_attr_mac_address_5.attr,
> +	&dev_attr_mac_address_6.attr,
> +	&dev_attr_mac_address_7.attr,
> +	&dev_attr_ac_recovery_status.attr,
> +	&dev_attr_power_loss_recovery.attr,
> +	&dev_attr_power_status.attr,
> +	NULL
> +};
> +
> +ATTRIBUTE_GROUPS(iei_wt61p803_puzzle);
> +
> +static int iei_wt61p803_puzzle_sysfs_create(struct device *dev,
> +		struct iei_wt61p803_puzzle *mcu)
> +{
> +	int ret;
> +
> +	mcu->kobj =

This is an odd place for a line break.

> +		kobject_create_and_add("iei_wt61p803_puzzle_core", &dev->kobj);
> +	if (!mcu->kobj)
> +		return -ENOMEM;
> +
> +	ret = sysfs_create_groups(mcu->kobj, iei_wt61p803_puzzle_groups);
> +	if (ret) {
> +		dev_err(dev, "sysfs creation failed");
> +
> +		/* Clean up */
> +		kobject_del(mcu->kobj);
> +		kobject_put(mcu->kobj);
> +		mcu->kobj = NULL;
> +
> +		return ret;

Drop this line.

> +	}
> +
> +	return 0;

Return ret.

> +}
> +
> +static int iei_wt61p803_puzzle_sysfs_remove(struct device *dev,
> +		struct iei_wt61p803_puzzle *mcu)
> +{
> +	/* Remove sysfs groups */
> +	sysfs_remove_groups(mcu->kobj, iei_wt61p803_puzzle_groups);
> +
> +	/* Remove the kobject */
> +	kobject_del(mcu->kobj);
> +	kobject_put(mcu->kobj);
> +	mcu->kobj = NULL;
> +
> +	return 0;
> +}
> +
> +static int iei_wt61p803_puzzle_probe(struct serdev_device *serdev)
> +{
> +	struct device *dev = &serdev->dev;
> +	struct iei_wt61p803_puzzle *mcu;
> +	u32 baud;
> +	int ret;
> +
> +	/* Read the current-speed property from the device tree */
> +	if (of_property_read_u32(dev->of_node, "current-speed", &baud)) {

Surely this is the baud you want to set, not what is already set?

> +		dev_err(dev,
> +			"'current-speed' is not specified in device node\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Log the specified BAUD RATE */
> +	dev_info(dev, "Driver baud rate: %d", baud);
> +
> +	/* Allocate the memory */
> +	mcu = devm_kzalloc(dev, sizeof(*mcu), GFP_KERNEL);
> +	if (!mcu)
> +		return -ENOMEM;
> +
> +	mcu->version = devm_kzalloc(dev, sizeof(*mcu->version), GFP_KERNEL);
> +	if (!mcu->version)
> +		return -ENOMEM;
> +
> +	mcu->status = devm_kzalloc(dev, sizeof(*mcu->version), GFP_KERNEL);
> +	if (!mcu->status)
> +		return -ENOMEM;
> +
> +	mcu->reply = devm_kzalloc(dev, sizeof(*mcu->reply), GFP_KERNEL);
> +	if (!mcu->reply)
> +		return -ENOMEM;
> +
> +	mcu->reply->data = devm_kzalloc(dev, IEI_WT61P803_PUZZLE_RESP_BUF_SIZE,
> +			GFP_KERNEL);
> +	if (!mcu->reply->data)
> +		return -ENOMEM;

Why not allocate all memory at once?

> +	/* Initialize device struct data */
> +	mcu->serdev = serdev;
> +
> +	mcu->status->ac_recovery_status_flag = 0x00;
> +	mcu->status->power_loss_recovery = 0x00;
> +	mcu->status->power_status = 0x00;

These should already be 0.

> +	init_completion(&mcu->reply->received);
> +
> +	/* Mutexes */
> +	mutex_init(&mcu->reply_lock);
> +	mutex_init(&mcu->bus_lock);
> +	mutex_init(&mcu->status_lock);
> +	mutex_init(&mcu->version_lock);

Why so many?

> +	/* Setup UART interface */
> +	serdev_device_set_drvdata(serdev, mcu);
> +	serdev_device_set_client_ops(serdev,
> +			&iei_wt61p803_puzzle_serdev_device_ops);
> +
> +	ret = devm_serdev_device_open(dev, serdev);
> +	if (ret)
> +		return ret;
> +
> +	serdev_device_set_baudrate(serdev, baud);
> +
> +	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 = iei_wt61p803_puzzle_get_version(mcu);
> +	if (ret)
> +		return ret;
> +
> +	ret = iei_wt61p803_puzzle_get_mac_addresses(mcu);
> +	if (ret)
> +		return ret;
> +
> +	ret = iei_wt61p803_puzzle_get_serial_number(mcu);
> +	if (ret)
> +		return ret;
> +
> +	dev_info(dev, "MCU version: %s", mcu->version->version);
> +
> +	dev_info(dev, "MCU firmware build info: %s", mcu->version->build_info);
> +	dev_info(dev, "MCU in bootloader mode: %s",
> +			mcu->version->bootloader_mode ? "true" : "false");
> +	dev_info(dev, "MCU protocol version: %s", mcu->version->protocol_version);
> +
> +	if (of_property_read_bool(dev->of_node, "enable-probe-beep")) {

Probing is a Linuxisum.  It has no place in DT.

> +		ret = iei_wt61p803_puzzle_buzzer(mcu, false);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = iei_wt61p803_puzzle_sysfs_create(dev, mcu);
> +	if (ret)
> +		return ret;
> +
> +	return devm_of_platform_populate(dev);
> +}
> +
> +static void iei_wt61p803_puzzle_remove(struct serdev_device *serdev)
> +{
> +	struct device *dev = &serdev->dev;
> +	struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev);
> +
> +	/* Remove sysfs kobjects and attributes */
> +	iei_wt61p803_puzzle_sysfs_remove(dev, mcu);
> +
> +	dev_info(dev, "Device unregistered");

No contentless/functionless prints please.

> +}
> +
> +

Place the table here.

> +MODULE_DEVICE_TABLE(of, iei_wt61p803_puzzle_dt_ids);
> +
> +static struct serdev_device_driver iei_wt61p803_puzzle_drv = {
> +	.probe			= iei_wt61p803_puzzle_probe,
> +	.remove			= iei_wt61p803_puzzle_remove,
> +	.driver = {
> +		.name		= "iei-wt61p803-puzzle",
> +		.of_match_table	= iei_wt61p803_puzzle_dt_ids,
> +	},
> +};
> +
> +module_serdev_device_driver(iei_wt61p803_puzzle_drv);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Luka Kovacic <luka.kovacic@sartura.hr>");
> +MODULE_DESCRIPTION("iEi WT61P803 PUZZLE MCU Driver");
> diff --git a/include/linux/mfd/iei-wt61p803-puzzle.h b/include/linux/mfd/iei-wt61p803-puzzle.h
> new file mode 100644
> index 000000000000..2bb6256574bc
> --- /dev/null
> +++ b/include/linux/mfd/iei-wt61p803-puzzle.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +/* iEi WT61P803 PUZZLE MCU MFD Driver

No such thing as an MFD.

Please describe the device.

> + *
> + * Copyright (C) 2020 Sartura Ltd.
> + * Author: Luka Kovacic <luka.kovacic@sartura.hr>
> + */
> +
> +#ifndef _LINUX_IEI_WT61P803_PUZZLE_H_
> +#define _LINUX_IEI_WT61P803_PUZZLE_H_

s/LINUX/MFD/

> +#define IEI_WT61P803_PUZZLE_BUF_SIZE 512
> +
> +struct iei_wt61p803_puzzle_mcu_version;
> +struct iei_wt61p803_puzzle_reply;
> +struct iei_wt61p803_puzzle;
> +
> +int iei_wt61p803_puzzle_write_command_watchdog(struct iei_wt61p803_puzzle *mcu,
> +		unsigned char *cmd, size_t size,
> +		unsigned char *reply_data, size_t *reply_size,
> +		int retry_count);
> +
> +int iei_wt61p803_puzzle_write_command(struct iei_wt61p803_puzzle *mcu,
> +		unsigned char *cmd, size_t size,
> +		unsigned char *reply_data, size_t *reply_size);
> +
> +#endif /* _LINUX_IEI_WT61P803_PUZZLE_H_ */

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 4/7] drivers: leds: Add the iEi WT61P803 PUZZLE LED driver

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 509 bytes --]

Hi!

> > Add support for the iEi WT61P803 PUZZLE LED driver.
> > Currently only the front panel power LED is supported.
> >
> > This driver depends on the iEi WT61P803 PUZZLE MFD driver.
> 
> Can we make it OF independent?
> See below how to achieve this.

Is there reason to believe this will be found in non-OF systems?

Best regards,

   								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 4/7] drivers: leds: Add the iEi WT61P803 PUZZLE LED driver

[Andrew Lunn]

On Wed, Sep 09, 2020 at 12:36:38PM +0200, Pavel Machek wrote:
> Hi!
> 
> > > Add support for the iEi WT61P803 PUZZLE LED driver.
> > > Currently only the front panel power LED is supported.
> > >
> > > This driver depends on the iEi WT61P803 PUZZLE MFD driver.
> > 
> > Can we make it OF independent?
> > See below how to achieve this.
> 
> Is there reason to believe this will be found in non-OF systems?

For this specific board with 2x 10G SFP+, i doubt it will be a pure
ACPI system. It could be a mixed system, ACPI for the simple bits, and
DT for the advanced parts.

Could this MFD appear on boards which could be pure ACPI? Maybe, if
their networking is simple enough, or Linux is not in control of the
low level details. There are some Intel Puzzle devices which could
potentially use the MFD.

      Andrew

Re: [PATCH 4/7] drivers: leds: Add the iEi WT61P803 PUZZLE LED driver

[Andy Shevchenko]

On Wed, Sep 9, 2020 at 1:36 PM Pavel Machek <pavel@ucw.cz> wrote:
> > > Add support for the iEi WT61P803 PUZZLE LED driver.
> > > Currently only the front panel power LED is supported.
> > >
> > > This driver depends on the iEi WT61P803 PUZZLE MFD driver.
> >
> > Can we make it OF independent?
> > See below how to achieve this.
>
> Is there reason to believe this will be found in non-OF systems?

It's one aspect. Another one is to give a better example to anybody
who might use this to copy'n'paste from. I believe that most of the
LED drivers can appear on non-DT systems.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/7] drivers: leds: Add the iEi WT61P803 PUZZLE LED driver

[Luka Kovačič]

Hello everyone,

I'm sending the email again, as I didn't send it as plain text
earlier, sorry.

Thanks for the comments, I'll review them and fix the issues.

This is currently the only iEi Puzzle series device that is using this
microcontroller. Their Intel-based platforms most likely will not use
this MCU, as they resorted to using more standard components there.

Some upcoming iEi Puzzle ARM-based boards might also implement this
microcontroller, so I do agree that using the new API would be beneficial
to future proof the driver.

Kind regards,
Luka

On Wed, Sep 9, 2020 at 3:52 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Wed, Sep 9, 2020 at 1:36 PM Pavel Machek <pavel@ucw.cz> wrote:
> > > > Add support for the iEi WT61P803 PUZZLE LED driver.
> > > > Currently only the front panel power LED is supported.
> > > >
> > > > This driver depends on the iEi WT61P803 PUZZLE MFD driver.
> > >
> > > Can we make it OF independent?
> > > See below how to achieve this.
> >
> > Is there reason to believe this will be found in non-OF systems?
>
> It's one aspect. Another one is to give a better example to anybody
> who might use this to copy'n'paste from. I believe that most of the
> LED drivers can appear on non-DT systems.
>
> --
> With Best Regards,
> Andy Shevchenko