* [PATCH v4 0/6] Add support for the iEi WT61P803 PUZZLE MCU
@ 2020-10-07 0:48 Luka Kovacic
2020-10-07 0:48 ` [PATCH v4 1/6] dt-bindings: Add iEi vendor prefix and iEi WT61P803 PUZZLE driver bindings Luka Kovacic
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Luka Kovacic @ 2020-10-07 0:48 UTC (permalink / raw)
To: linux-kernel, linux-hwmon, linux-leds
Cc: lee.jones, pavel, dmurphy, robh+dt, jdelvare, linux, marek.behun,
luka.perkov, robert.marko, Luka Kovacic
This patchset adds support for the iEi WT61P803 PUZZLE microcontroller,
which enables some board specific features like fan and LED control,
system power management and temperature sensor reading on some iEi
Puzzle series boards.
The first board to use this functionality is iEi Puzzle-M801 1U
Rackmount Network Appliance and is since v4 sent separately, as a
standalone patch.
Changes for v2:
- Use LAAs for local-mac-address and match reg values
- Code styling changes
- Error handling moved to the end of the function
- Define all magic numbers in the main header file
- Convert the driver to make it OF independent
- Refactor hwmon to use devm_hwmon_device_register_with_info()
- Reduce the number of mutex locks
- Allocate memory once for the response buffer
- Reduce managed memory allocations
Changes for v3:
- Move iei-wt61p803-puzzle driver sysfs interface documentation to testing
- Change some internal functions to static
- Sync dt-bindings examples with the iEi Puzzle-M801 board dts
- Remove obsolete device tree properties and correct LED functions
- Reverse christmas tree variable declaration order, where possible
- MAC address sysfs function rewrite
- Fixed struct members size, where reasonable (MFD driver)
- Add an error check for hwmon_dev
- Use devm_led_classdev_register_ext() in the LED driver
Changes for v4:
- Clean up sensible checks reported by checkpatch --strict
- Document the mutex lock usage in the LED driver
- Fix error handling and code styling issues in the HWMON driver
- Break up the patchset and send the iEi Puzzle-M801 board support
patch separately
Luka Kovacic (6):
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
.../testing/sysfs-driver-iei-wt61p803-puzzle | 55 +
.../hwmon/iei,wt61p803-puzzle-hwmon.yaml | 41 +
.../leds/iei,wt61p803-puzzle-leds.yaml | 45 +
.../bindings/mfd/iei,wt61p803-puzzle.yaml | 82 ++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
MAINTAINERS | 13 +
drivers/hwmon/Kconfig | 8 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/iei-wt61p803-puzzle-hwmon.c | 457 +++++++
drivers/leds/Kconfig | 8 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-iei-wt61p803-puzzle.c | 156 +++
drivers/mfd/Kconfig | 8 +
drivers/mfd/Makefile | 1 +
drivers/mfd/iei-wt61p803-puzzle.c | 1053 +++++++++++++++++
include/linux/mfd/iei-wt61p803-puzzle.h | 69 ++
16 files changed, 2000 insertions(+)
create mode 100644 Documentation/ABI/testing/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 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.26.2
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 1/6] dt-bindings: Add iEi vendor prefix and iEi WT61P803 PUZZLE driver bindings
2020-10-07 0:48 [PATCH v4 0/6] Add support for the iEi WT61P803 PUZZLE MCU Luka Kovacic
@ 2020-10-07 0:48 ` Luka Kovacic
2020-10-07 0:48 ` [PATCH v4 2/6] drivers: mfd: Add a driver for iEi WT61P803 PUZZLE MCU Luka Kovacic
` (4 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Luka Kovacic @ 2020-10-07 0:48 UTC (permalink / raw)
To: linux-kernel, linux-hwmon, linux-leds
Cc: lee.jones, pavel, dmurphy, robh+dt, jdelvare, linux, marek.behun,
luka.perkov, robert.marko, 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>
Cc: Robert Marko <robert.marko@sartura.hr>
---
.../hwmon/iei,wt61p803-puzzle-hwmon.yaml | 41 ++++++++++
.../leds/iei,wt61p803-puzzle-leds.yaml | 45 ++++++++++
.../bindings/mfd/iei,wt61p803-puzzle.yaml | 82 +++++++++++++++++++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
4 files changed, 170 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..0d353e5803bf
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/iei,wt61p803-puzzle-leds.yaml
@@ -0,0 +1,45 @@
+# 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
+ $ref: common.yaml
+ 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
+
+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..79a232d75093
--- /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-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-beep;
+
+ leds {
+ compatible = "iei,wt61p803-puzzle-leds";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ led@0 {
+ reg = <0>;
+ function = LED_FUNCTION_POWER;
+ color = <LED_COLOR_ID_BLUE>;
+ };
+ };
+
+ 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.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 2/6] drivers: mfd: Add a driver for iEi WT61P803 PUZZLE MCU
2020-10-07 0:48 [PATCH v4 0/6] Add support for the iEi WT61P803 PUZZLE MCU Luka Kovacic
2020-10-07 0:48 ` [PATCH v4 1/6] dt-bindings: Add iEi vendor prefix and iEi WT61P803 PUZZLE driver bindings Luka Kovacic
@ 2020-10-07 0:48 ` Luka Kovacic
2020-10-07 0:48 ` [PATCH v4 3/6] drivers: hwmon: Add the iEi WT61P803 PUZZLE HWMON driver Luka Kovacic
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Luka Kovacic @ 2020-10-07 0:48 UTC (permalink / raw)
To: linux-kernel, linux-hwmon, linux-leds
Cc: lee.jones, pavel, dmurphy, robh+dt, jdelvare, linux, marek.behun,
luka.perkov, robert.marko, 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>
Cc: Robert Marko <robert.marko@sartura.hr>
---
drivers/mfd/Kconfig | 8 +
drivers/mfd/Makefile | 1 +
drivers/mfd/iei-wt61p803-puzzle.c | 1053 +++++++++++++++++++++++
include/linux/mfd/iei-wt61p803-puzzle.h | 69 ++
4 files changed, 1131 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..b1588845894e 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..8aea993a4063
--- /dev/null
+++ b/drivers/mfd/iei-wt61p803-puzzle.c
@@ -0,0 +1,1053 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* iEi WT61P803 PUZZLE MCU Driver
+ * System management microcontroller for fan control, temperature sensor reading,
+ * LED control and system identification on iEi Puzzle series ARM-based appliances.
+ *
+ * Copyright (C) 2020 Sartura Ltd.
+ * Author: Luka Kovacic <luka.kovacic@sartura.hr>
+ */
+
+#include <asm/unaligned.h>
+#include <linux/atomic.h>
+#include <linux/delay.h>
+#include <linux/delay.h>
+#include <linux/export.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/mfd/iei-wt61p803-puzzle.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/property.h>
+#include <linux/sched.h>
+#include <linux/serdev.h>
+#include <linux/slab.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_VERSION_MAC_LENGTH 17
+#define IEI_WT61P803_PUZZLE_VERSION_SN_LENGTH 36
+#define IEI_WT61P803_PUZZLE_VERSION_VERSION_LENGTH 6
+#define IEI_WT61P803_PUZZLE_VERSION_BUILD_INFO_LENGTH 16
+#define IEI_WT61P803_PUZZLE_VERSION_PROTOCOL_VERSION_LENGTH 8
+
+/* Use HZ as a timeout value throughout the driver */
+#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
+ *
+ * Describes 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 {
+ char version[IEI_WT61P803_PUZZLE_VERSION_VERSION_LENGTH + 1];
+ char build_info[IEI_WT61P803_PUZZLE_VERSION_BUILD_INFO_LENGTH + 1];
+ bool bootloader_mode;
+ char protocol_version[IEI_WT61P803_PUZZLE_VERSION_PROTOCOL_VERSION_LENGTH + 1];
+ char serial_number[IEI_WT61P803_PUZZLE_VERSION_SN_LENGTH + 1];
+ char mac_address[8][IEI_WT61P803_PUZZLE_VERSION_MAC_LENGTH + 1];
+};
+
+/**
+ * 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: MCU version related data
+ * @status: MCU status related data
+ * @response_buffer Command response buffer allocation
+ * @lock General member mutex lock
+ */
+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 iei_wt61p803_puzzle_mcu_version version;
+ struct iei_wt61p803_puzzle_mcu_status status;
+ unsigned char *response_buffer;
+ struct mutex lock;
+};
+
+static unsigned char iei_wt61p803_puzzle_checksum(unsigned char *buf, size_t len)
+{
+ unsigned char checksum = 0;
+ unsigned int i;
+
+ for (i = 0; i < len; i++)
+ checksum ^= buf[i];
+
+ return checksum;
+}
+
+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 checksum;
+
+ mutex_lock(&mcu->reply_lock);
+
+ /* Check the incoming frame header */
+ if (!(raw_resp_data[0] == IEI_WT61P803_PUZZLE_CMD_HEADER_START ||
+ raw_resp_data[0] == IEI_WT61P803_PUZZLE_CMD_HEADER_START_OTHER ||
+ (raw_resp_data[0] == IEI_WT61P803_PUZZLE_CMD_HEADER_EEPROM &&
+ raw_resp_data[1] == IEI_WT61P803_PUZZLE_CMD_EEPROM_READ))) {
+
+ /* Frame header is not correct, check whether to append */
+ if (mcu->reply->state != FRAME_PROCESSING) {
+ dev_err(dev, "Invalid frame header and state (0x%x)", mcu->reply->state);
+ mutex_unlock(&mcu->reply_lock);
+ return -EIO;
+ }
+
+ /* 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;
+ }
+
+ checksum = iei_wt61p803_puzzle_checksum(mcu->reply->data, mcu->reply->size - 1);
+
+ if (checksum != mcu->reply->data[mcu->reply->size - 1]) {
+ /* The checksum isn't matched yet, wait for new frames */
+ mutex_unlock(&mcu->reply_lock);
+ return (int)size;
+ }
+
+ /* 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)
+ *
+ * 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;
+
+ cmd[len - 1] = iei_wt61p803_puzzle_checksum(cmd, size);
+
+ mutex_lock(&mcu->bus_lock);
+ mutex_lock(&mcu->reply_lock);
+
+ if (!mcu->reply) {
+ ret = -EFAULT;
+ goto exit;
+ }
+
+ /* Initialize reply struct */
+ reinit_completion(&mcu->reply->received);
+ mcu->reply->state = FRAME_STRUCT_EMPTY;
+ mcu->reply->size = 0;
+ mutex_unlock(&mcu->reply_lock);
+
+ ret = serdev_device_write(mcu->serdev, cmd, len, IEI_WT61P803_PUZZLE_GENERAL_TIMEOUT);
+
+ if (ret < 0) {
+ mutex_unlock(&mcu->bus_lock);
+ return ret;
+ }
+
+ if (!wait_for_completion_timeout(&mcu->reply->received,
+ IEI_WT61P803_PUZZLE_GENERAL_TIMEOUT)) {
+ dev_err(dev, "Command reply receive timeout\n");
+ mutex_lock(&mcu->reply_lock);
+ reinit_completion(&mcu->reply->received);
+ mcu->reply->state = FRAME_TIMEOUT;
+
+ ret = -ETIMEDOUT;
+ goto exit;
+ }
+
+ mutex_lock(&mcu->reply_lock);
+
+ if (!mcu->reply) {
+ ret = -EFAULT;
+ goto exit;
+ }
+
+ *reply_size = mcu->reply->size;
+ /* Copy the received data, as it will not be available after a new frame is received */
+ memcpy(reply_data, mcu->reply->data, mcu->reply->size);
+
+ ret = 0;
+exit:
+ mutex_unlock(&mcu->reply_lock);
+ mutex_unlock(&mcu->bus_lock);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iei_wt61p803_puzzle_write_command);
+
+int iei_wt61p803_puzzle_buzzer(struct iei_wt61p803_puzzle *mcu, bool long_beep)
+{
+ unsigned char buzzer_short_cmd[4] = {
+ IEI_WT61P803_PUZZLE_CMD_HEADER_START,
+ IEI_WT61P803_PUZZLE_CMD_FUNCTION_SINGLE,
+ '2'
+ }; /* Buzzer 0.5 sec */
+ unsigned char buzzer_long_cmd[4] = {
+ IEI_WT61P803_PUZZLE_CMD_HEADER_START,
+ IEI_WT61P803_PUZZLE_CMD_FUNCTION_SINGLE,
+ '3'
+ }; /* Buzzer 1.5 sec */
+ unsigned char *resp_buf = mcu->response_buffer;
+ size_t reply_size = 0;
+ int ret;
+
+ mutex_lock(&mcu->lock);
+ ret = iei_wt61p803_puzzle_write_command(mcu,
+ long_beep ? buzzer_long_cmd : buzzer_short_cmd,
+ 4, resp_buf, &reply_size);
+ if (ret)
+ goto exit;
+
+ if (reply_size != 3) {
+ ret = -EIO;
+ goto exit;
+ }
+
+ if (!(resp_buf[0] == IEI_WT61P803_PUZZLE_CMD_HEADER_START &&
+ resp_buf[1] == IEI_WT61P803_PUZZLE_CMD_RESPONSE_OK &&
+ resp_buf[2] == IEI_WT61P803_PUZZLE_CHECKSUM_RESPONSE_OK)) {
+ ret = -EPROTO;
+ goto exit;
+ }
+exit:
+ mutex_unlock(&mcu->lock);
+ return ret;
+}
+
+static int iei_wt61p803_puzzle_get_version(struct iei_wt61p803_puzzle *mcu)
+{
+ unsigned char version_cmd[3] = {
+ IEI_WT61P803_PUZZLE_CMD_HEADER_START_OTHER,
+ IEI_WT61P803_PUZZLE_CMD_OTHER_VERSION
+ };
+ unsigned char build_info_cmd[3] = {
+ IEI_WT61P803_PUZZLE_CMD_HEADER_START_OTHER,
+ IEI_WT61P803_PUZZLE_CMD_OTHER_BUILD
+ };
+ unsigned char bootloader_mode_cmd[3] = {
+ IEI_WT61P803_PUZZLE_CMD_HEADER_START_OTHER,
+ IEI_WT61P803_PUZZLE_CMD_OTHER_BOOTLOADER_MODE
+ };
+ unsigned char protocol_version_cmd[3] = {
+ IEI_WT61P803_PUZZLE_CMD_HEADER_START_OTHER,
+ IEI_WT61P803_PUZZLE_CMD_OTHER_PROTOCOL_VERSION
+ };
+ unsigned char *rb = mcu->response_buffer;
+ size_t reply_size = 0;
+ int ret;
+
+ mutex_lock(&mcu->lock);
+
+ ret = iei_wt61p803_puzzle_write_command(mcu, version_cmd, sizeof(version_cmd),
+ rb, &reply_size);
+ if (ret)
+ goto err;
+ if (reply_size < 7) {
+ ret = -EIO;
+ goto err;
+ }
+ sprintf(mcu->version.version, "v%c.%c%c%c", rb[2], rb[3], rb[4], rb[5]);
+
+ ret = iei_wt61p803_puzzle_write_command(mcu, build_info_cmd,
+ sizeof(build_info_cmd), rb,
+ &reply_size);
+ if (ret)
+ goto err;
+ if (reply_size < 15) {
+ ret = -EIO;
+ goto err;
+ }
+ sprintf(mcu->version.build_info, "%c%c/%c%c/%c%c%c%c %c%c:%c%c",
+ rb[8], rb[9], rb[6], rb[7], rb[2],
+ rb[3], rb[4], rb[5], rb[10], rb[11],
+ rb[12], rb[13]);
+
+ ret = iei_wt61p803_puzzle_write_command(mcu, bootloader_mode_cmd,
+ sizeof(bootloader_mode_cmd), rb,
+ &reply_size);
+ if (ret)
+ goto err;
+ if (reply_size < 4) {
+ ret = -EIO;
+ goto err;
+ }
+ if (rb[2] == IEI_WT61P803_PUZZLE_CMD_OTHER_MODE_APPS)
+ mcu->version.bootloader_mode = false;
+ else if (rb[2] == IEI_WT61P803_PUZZLE_CMD_OTHER_MODE_BOOTLOADER)
+ mcu->version.bootloader_mode = true;
+
+ ret = iei_wt61p803_puzzle_write_command(mcu, protocol_version_cmd,
+ sizeof(protocol_version_cmd), rb,
+ &reply_size);
+ if (ret)
+ goto err;
+ if (reply_size < 9) {
+ ret = -EIO;
+ goto err;
+ }
+ sprintf(mcu->version.protocol_version, "v%c.%c%c%c%c%c",
+ rb[7], rb[6], rb[5], rb[4], rb[3], rb[2]);
+err:
+ mutex_unlock(&mcu->lock);
+ return ret;
+}
+
+static int iei_wt61p803_puzzle_get_mcu_status(struct iei_wt61p803_puzzle *mcu)
+{
+ unsigned char mcu_status_cmd[5] = {
+ IEI_WT61P803_PUZZLE_CMD_HEADER_START,
+ IEI_WT61P803_PUZZLE_CMD_FUNCTION_OTHER,
+ IEI_WT61P803_PUZZLE_CMD_FUNCTION_OTHER_STATUS,
+ IEI_WT61P803_PUZZLE_CMD_FUNCTION_OTHER_STATUS
+ };
+ unsigned char *resp_buf = mcu->response_buffer;
+ size_t reply_size = 0;
+ int ret;
+
+ mutex_lock(&mcu->lock);
+ ret = iei_wt61p803_puzzle_write_command(mcu, mcu_status_cmd, sizeof(mcu_status_cmd),
+ resp_buf, &reply_size);
+ if (ret)
+ goto exit;
+ if (reply_size < 20) {
+ ret = -EIO;
+ goto exit;
+ }
+
+ /* 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 (resp_buf[0] == IEI_WT61P803_PUZZLE_CMD_HEADER_START &&
+ resp_buf[1] == IEI_WT61P803_PUZZLE_CMD_FUNCTION_OTHER &&
+ resp_buf[2] == IEI_WT61P803_PUZZLE_CMD_FUNCTION_OTHER_STATUS &&
+ resp_buf[3] == IEI_WT61P803_PUZZLE_CMD_FUNCTION_OTHER_STATUS) {
+ mcu->status.ac_recovery_status_flag = resp_buf[5];
+ mcu->status.power_loss_recovery = resp_buf[10];
+ mcu->status.power_status = resp_buf[19];
+ }
+exit:
+ mutex_unlock(&mcu->lock);
+ return ret;
+}
+
+static int iei_wt61p803_puzzle_get_serial_number(struct iei_wt61p803_puzzle *mcu)
+{
+ unsigned char *resp_buf = mcu->response_buffer;
+ unsigned char serial_number_cmd[5] = {
+ IEI_WT61P803_PUZZLE_CMD_HEADER_EEPROM,
+ IEI_WT61P803_PUZZLE_CMD_EEPROM_READ,
+ 0x00,
+ 0x24
+ };
+ size_t reply_size = 0;
+ int ret;
+
+ mutex_lock(&mcu->lock);
+ ret = iei_wt61p803_puzzle_write_command(mcu, serial_number_cmd,
+ sizeof(serial_number_cmd),
+ resp_buf, &reply_size);
+ if (ret)
+ goto err;
+
+ sprintf(mcu->version.serial_number, "%.*s",
+ IEI_WT61P803_PUZZLE_VERSION_SN_LENGTH, resp_buf + 4);
+err:
+ mutex_unlock(&mcu->lock);
+ return ret;
+}
+
+static int iei_wt61p803_puzzle_write_serial_number(struct iei_wt61p803_puzzle *mcu,
+ unsigned char serial_number[36])
+{
+ unsigned char *resp_buf = mcu->response_buffer;
+ unsigned char serial_number_header[4] = {
+ IEI_WT61P803_PUZZLE_CMD_HEADER_EEPROM,
+ IEI_WT61P803_PUZZLE_CMD_EEPROM_WRITE,
+ 0x00,
+ 0xC
+ };
+ unsigned char serial_number_cmd[4+12+1]; /* header, serial number chunk, XOR checksum */
+ size_t reply_size = 0;
+ int ret, sn_counter;
+
+ /* The MCU can only handle 22 byte messages, send the S/N in chunks */
+ mutex_lock(&mcu->lock);
+ 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);
+
+ serial_number_cmd[sizeof(serial_number_cmd) - 1] = 0;
+
+ ret = iei_wt61p803_puzzle_write_command(mcu, serial_number_cmd,
+ sizeof(serial_number_cmd),
+ resp_buf, &reply_size);
+ if (ret)
+ goto err;
+ if (reply_size != 3) {
+ ret = -EIO;
+ goto err;
+ }
+ if (!(resp_buf[0] == IEI_WT61P803_PUZZLE_CMD_HEADER_START &&
+ resp_buf[1] == IEI_WT61P803_PUZZLE_CMD_RESPONSE_OK &&
+ resp_buf[2] == IEI_WT61P803_PUZZLE_CHECKSUM_RESPONSE_OK)) {
+ ret = -EPROTO;
+ goto err;
+ }
+ }
+
+ sprintf(mcu->version.serial_number, "%.*s",
+ IEI_WT61P803_PUZZLE_VERSION_SN_LENGTH, serial_number);
+err:
+ mutex_unlock(&mcu->lock);
+ return ret;
+}
+
+static int iei_wt61p803_puzzle_get_mac_addresses(struct iei_wt61p803_puzzle *mcu)
+{
+ unsigned char *resp_buf = mcu->response_buffer;
+ unsigned char mac_address_cmd[5] = {
+ IEI_WT61P803_PUZZLE_CMD_HEADER_EEPROM,
+ IEI_WT61P803_PUZZLE_CMD_EEPROM_READ,
+ 0x00,
+ 0x11
+ };
+ size_t reply_size = 0;
+ int ret, mac_counter;
+
+ mutex_lock(&mcu->lock);
+ 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),
+ resp_buf, &reply_size);
+ if (ret)
+ continue;
+
+ if (reply_size < 22) {
+ ret = -EIO;
+ goto err;
+ }
+
+ sprintf(mcu->version.mac_address[mac_counter], "%.*s",
+ IEI_WT61P803_PUZZLE_VERSION_MAC_LENGTH, resp_buf + 4);
+ }
+err:
+ mutex_unlock(&mcu->lock);
+ return ret;
+}
+
+static int iei_wt61p803_puzzle_write_mac_address(struct iei_wt61p803_puzzle *mcu,
+ unsigned char mac_address[17],
+ int mac_address_idx)
+{
+ unsigned char *resp_buf = mcu->response_buffer;
+ unsigned char mac_address_header[4] = {
+ IEI_WT61P803_PUZZLE_CMD_HEADER_EEPROM,
+ IEI_WT61P803_PUZZLE_CMD_EEPROM_WRITE,
+ 0x00,
+ 0x11
+ };
+ unsigned char mac_address_cmd[4+17+1]; /* header, MAC address, XOR checksum*/
+ size_t reply_size = 0;
+ int ret;
+
+ if (!(mac_address_idx < 8))
+ return -EINVAL;
+
+ 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);
+
+ mac_address_cmd[sizeof(mac_address_cmd) - 1] = 0;
+
+ mutex_lock(&mcu->lock);
+ ret = iei_wt61p803_puzzle_write_command(mcu, mac_address_cmd,
+ sizeof(mac_address_cmd),
+ resp_buf, &reply_size);
+ if (ret)
+ goto err;
+ if (reply_size != 3) {
+ ret = -EIO;
+ goto err;
+ }
+ if (!(resp_buf[0] == IEI_WT61P803_PUZZLE_CMD_HEADER_START &&
+ resp_buf[1] == IEI_WT61P803_PUZZLE_CMD_RESPONSE_OK &&
+ resp_buf[2] == IEI_WT61P803_PUZZLE_CHECKSUM_RESPONSE_OK)) {
+ ret = -EPROTO;
+ goto err;
+ }
+
+ sprintf(mcu->version.mac_address[mac_address_idx], "%.*s",
+ IEI_WT61P803_PUZZLE_VERSION_MAC_LENGTH, mac_address);
+err:
+ mutex_unlock(&mcu->lock);
+ return ret;
+}
+
+static 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] = {
+ IEI_WT61P803_PUZZLE_CMD_HEADER_START,
+ IEI_WT61P803_PUZZLE_CMD_FUNCTION_OTHER,
+ IEI_WT61P803_PUZZLE_CMD_FUNCTION_OTHER_POWER_LOSS,
+ '0'
+ };
+ unsigned char *resp_buf = mcu->response_buffer;
+ unsigned char cmd_buf[2];
+ size_t reply_size = 0;
+ int ret;
+
+ if (power_loss_recovery_action < 0 || power_loss_recovery_action > 4)
+ return -EINVAL;
+
+ ret = snprintf(cmd_buf, sizeof(cmd_buf), "%d", power_loss_recovery_action);
+ if (ret < 0)
+ return ret;
+
+ /* Modify the command with the action index */
+ power_loss_recovery_cmd[3] = cmd_buf[0];
+
+ mutex_lock(&mcu->lock);
+ ret = iei_wt61p803_puzzle_write_command(mcu, power_loss_recovery_cmd,
+ sizeof(power_loss_recovery_cmd),
+ resp_buf, &reply_size);
+ if (ret)
+ goto exit;
+ mcu->status.power_loss_recovery = power_loss_recovery_action;
+exit:
+ mutex_unlock(&mcu->lock);
+ return ret;
+}
+
+#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->lock);
+ ret = sprintf(buf, "%s\n", mcu->version.serial_number);
+ mutex_unlock(&mcu->lock);
+
+ return ret;
+}
+
+static ssize_t serial_number_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ unsigned char serial_number[IEI_WT61P803_PUZZLE_VERSION_SN_LENGTH];
+ struct device *dev_container = sysfs_container(dev);
+ struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
+ int ret;
+
+ if ((int)count != IEI_WT61P803_PUZZLE_VERSION_SN_LENGTH + 1)
+ return -EINVAL;
+
+ memcpy(serial_number, (unsigned char *)buf, IEI_WT61P803_PUZZLE_VERSION_SN_LENGTH);
+
+ 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 idx, ret;
+
+ mutex_lock(&mcu->lock);
+
+ if (strlen(attr->attr.name) != 13)
+ return -EIO;
+
+ idx = attr->attr.name[12] - '0';
+ if (idx < 0 || idx > 7)
+ return -EIO;
+
+ ret = sprintf(buf, "%s\n", mcu->version.mac_address[idx]);
+
+ mutex_unlock(&mcu->lock);
+ return ret;
+}
+
+static ssize_t mac_address_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ unsigned char mac_address[IEI_WT61P803_PUZZLE_VERSION_MAC_LENGTH];
+ struct device *dev_container = sysfs_container(dev);
+ struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev_container);
+ int idx, ret;
+
+ if ((int)count != 17 + 1)
+ return -EINVAL;
+
+ memcpy(mac_address, (unsigned char *)buf, IEI_WT61P803_PUZZLE_VERSION_MAC_LENGTH);
+
+ if (strlen(attr->attr.name) != 13)
+ return -EIO;
+
+ idx = attr->attr.name[12] - '0';
+ if (idx < 0 || idx > 7)
+ return -EIO;
+
+ ret = iei_wt61p803_puzzle_write_mac_address(mcu, mac_address, idx);
+ 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->lock);
+ ret = sprintf(buf, "%x\n", mcu->status.ac_recovery_status_flag);
+ mutex_unlock(&mcu->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->lock);
+ ret = sprintf(buf, "%x\n", mcu->status.power_loss_recovery);
+ mutex_unlock(&mcu->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->lock);
+ ret = sprintf(buf, "%x\n", mcu->status.power_status);
+ mutex_unlock(&mcu->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) {
+ kobject_del(mcu->kobj);
+ kobject_put(mcu->kobj);
+ mcu->kobj = NULL;
+ }
+
+ 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 baud rate from 'current-speed', because the MCU supports different rates */
+ if (device_property_read_u32(dev, "current-speed", &baud)) {
+ dev_err(dev,
+ "'current-speed' is not specified in device node\n");
+ return -EINVAL;
+ }
+ dev_info(dev, "Driver baud rate: %d", baud);
+
+ /* Allocate the memory */
+ mcu = devm_kzalloc(dev, sizeof(*mcu), GFP_KERNEL);
+ if (!mcu)
+ 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;
+
+ mcu->response_buffer = devm_kzalloc(dev, IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
+ if (!mcu->response_buffer)
+ return -ENOMEM;
+
+ /* Initialize device struct data */
+ mcu->serdev = serdev;
+ init_completion(&mcu->reply->received);
+ mutex_init(&mcu->reply_lock);
+ mutex_init(&mcu->bus_lock);
+ mutex_init(&mcu->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");
+ 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 (device_property_read_bool(dev, "enable-beep")) {
+ ret = iei_wt61p803_puzzle_buzzer(mcu, false);
+ if (ret)
+ return ret;
+ }
+
+ ret = iei_wt61p803_puzzle_sysfs_create(dev, mcu);
+
+ 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);
+
+ iei_wt61p803_puzzle_sysfs_remove(dev, mcu);
+}
+
+static const struct of_device_id iei_wt61p803_puzzle_dt_ids[] = {
+ { .compatible = "iei,wt61p803-puzzle" },
+ { }
+};
+
+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..27c3a4a9014f
--- /dev/null
+++ b/include/linux/mfd/iei-wt61p803-puzzle.h
@@ -0,0 +1,69 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* iEi WT61P803 PUZZLE MCU Driver
+ * System management microcontroller for fan control, temperature sensor reading,
+ * LED control and system identification on iEi Puzzle series ARM-based appliances.
+ *
+ * Copyright (C) 2020 Sartura Ltd.
+ * Author: Luka Kovacic <luka.kovacic@sartura.hr>
+ */
+
+#ifndef _MFD_IEI_WT61P803_PUZZLE_H_
+#define _MFD_IEI_WT61P803_PUZZLE_H_
+
+#define IEI_WT61P803_PUZZLE_BUF_SIZE 512
+
+/* Command magic numbers */
+#define IEI_WT61P803_PUZZLE_CMD_HEADER_START 0x40 /* @ */
+#define IEI_WT61P803_PUZZLE_CMD_HEADER_START_OTHER 0x25 /* % */
+#define IEI_WT61P803_PUZZLE_CMD_HEADER_EEPROM 0xF7
+
+#define IEI_WT61P803_PUZZLE_CMD_RESPONSE_OK 0x30 /* 0 */
+#define IEI_WT61P803_PUZZLE_CHECKSUM_RESPONSE_OK 0x70
+
+#define IEI_WT61P803_PUZZLE_CMD_EEPROM_READ 0xA1
+#define IEI_WT61P803_PUZZLE_CMD_EEPROM_WRITE 0xA0
+
+#define IEI_WT61P803_PUZZLE_CMD_OTHER_VERSION 0x56 /* V */
+#define IEI_WT61P803_PUZZLE_CMD_OTHER_BUILD 0x42 /* B */
+#define IEI_WT61P803_PUZZLE_CMD_OTHER_BOOTLOADER_MODE 0x4D /* M */
+#define IEI_WT61P803_PUZZLE_CMD_OTHER_MODE_BOOTLOADER 0x30
+#define IEI_WT61P803_PUZZLE_CMD_OTHER_MODE_APPS 0x31
+#define IEI_WT61P803_PUZZLE_CMD_OTHER_PROTOCOL_VERSION 0x50 /* P */
+
+#define IEI_WT61P803_PUZZLE_CMD_FUNCTION_SINGLE 0x43 /* C */
+#define IEI_WT61P803_PUZZLE_CMD_FUNCTION_OTHER 0x4F /* O */
+#define IEI_WT61P803_PUZZLE_CMD_FUNCTION_OTHER_STATUS 0x53 /* S */
+#define IEI_WT61P803_PUZZLE_CMD_FUNCTION_OTHER_POWER_LOSS 0x41 /* A */
+
+#define IEI_WT61P803_PUZZLE_CMD_LED 0x52 /* R */
+#define IEI_WT61P803_PUZZLE_CMD_LED_POWER 0x31 /* 1 */
+
+#define IEI_WT61P803_PUZZLE_CMD_TEMP 0x54 /* T */
+#define IEI_WT61P803_PUZZLE_CMD_TEMP_ALL 0x41 /* A */
+
+#define IEI_WT61P803_PUZZLE_CMD_FAN 0x46 /* F */
+#define IEI_WT61P803_PUZZLE_CMD_FAN_PWM_0 0x30
+#define IEI_WT61P803_PUZZLE_CMD_FAN_PWM_1 0x31
+#define IEI_WT61P803_PUZZLE_CMD_FAN_PWM_READ 0x5A /* Z */
+#define IEI_WT61P803_PUZZLE_CMD_FAN_PWM_WRITE 0x57 /* W */
+#define IEI_WT61P803_PUZZLE_CMD_FAN_RPM_0 0x41 /* A */
+#define IEI_WT61P803_PUZZLE_CMD_FAN_RPM_1 0x42 /* B */
+#define IEI_WT61P803_PUZZLE_CMD_FAN_RPM_2 0x43 /* C */
+#define IEI_WT61P803_PUZZLE_CMD_FAN_RPM_3 0x44 /* D */
+#define IEI_WT61P803_PUZZLE_CMD_FAN_RPM_4 0x45 /* E */
+#define IEI_WT61P803_PUZZLE_CMD_FAN_RPM_5 0x46 /* F */
+
+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 /* _MFD_IEI_WT61P803_PUZZLE_H_ */
--
2.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 3/6] drivers: hwmon: Add the iEi WT61P803 PUZZLE HWMON driver
2020-10-07 0:48 [PATCH v4 0/6] Add support for the iEi WT61P803 PUZZLE MCU Luka Kovacic
2020-10-07 0:48 ` [PATCH v4 1/6] dt-bindings: Add iEi vendor prefix and iEi WT61P803 PUZZLE driver bindings Luka Kovacic
2020-10-07 0:48 ` [PATCH v4 2/6] drivers: mfd: Add a driver for iEi WT61P803 PUZZLE MCU Luka Kovacic
@ 2020-10-07 0:48 ` Luka Kovacic
2020-10-11 21:26 ` Guenter Roeck
2020-10-07 0:48 ` [PATCH v4 4/6] drivers: leds: Add the iEi WT61P803 PUZZLE LED driver Luka Kovacic
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Luka Kovacic @ 2020-10-07 0:48 UTC (permalink / raw)
To: linux-kernel, linux-hwmon, linux-leds
Cc: lee.jones, pavel, dmurphy, robh+dt, jdelvare, linux, marek.behun,
luka.perkov, robert.marko, 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>
Cc: Robert Marko <robert.marko@sartura.hr>
---
drivers/hwmon/Kconfig | 8 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/iei-wt61p803-puzzle-hwmon.c | 457 ++++++++++++++++++++++
3 files changed, 466 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..be7b019d126c
--- /dev/null
+++ b/drivers/hwmon/iei-wt61p803-puzzle-hwmon.c
@@ -0,0 +1,457 @@
+// 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/err.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/hwmon.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/math64.h>
+#include <linux/mfd/iei-wt61p803-puzzle.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/slab.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
+ * @response_buffer Global MCU response buffer allocation
+ * @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;
+ unsigned char *response_buffer;
+ 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];
+};
+
+#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)
+{
+ unsigned char *resp_buf = mcu_hwmon->response_buffer;
+ unsigned char temp_sensor_ntc_cmd[4] = {
+ IEI_WT61P803_PUZZLE_CMD_HEADER_START,
+ IEI_WT61P803_PUZZLE_CMD_TEMP,
+ IEI_WT61P803_PUZZLE_CMD_TEMP_ALL
+ };
+ size_t reply_size = 0;
+ int ret;
+
+ ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, temp_sensor_ntc_cmd,
+ sizeof(temp_sensor_ntc_cmd), resp_buf,
+ &reply_size);
+
+ if (ret)
+ return ret;
+
+ /* Check the number of NTC values (should be 0x32/'2') */
+ if (resp_buf[3] != 0x32)
+ return -EIO;
+
+ *value = raw_temp_to_milidegree_celsius(resp_buf[4 + channel]);
+
+ return 0;
+}
+
+#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)
+{
+ unsigned char *resp_buf = mcu_hwmon->response_buffer;
+ static const u8 fan_speed_cmds[] = {
+ IEI_WT61P803_PUZZLE_CMD_FAN_RPM_0,
+ IEI_WT61P803_PUZZLE_CMD_FAN_RPM_1,
+ IEI_WT61P803_PUZZLE_CMD_FAN_RPM_2,
+ IEI_WT61P803_PUZZLE_CMD_FAN_RPM_3,
+ IEI_WT61P803_PUZZLE_CMD_FAN_RPM_4
+ };
+ unsigned char fan_speed_cmd[4] = {
+ IEI_WT61P803_PUZZLE_CMD_HEADER_START,
+ IEI_WT61P803_PUZZLE_CMD_FAN,
+ IEI_WT61P803_PUZZLE_CMD_FAN_RPM_0
+ };
+ size_t reply_size = 0;
+ int ret;
+
+ fan_speed_cmd[2] = fan_speed_cmds[channel];
+
+ ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, fan_speed_cmd,
+ sizeof(fan_speed_cmd), resp_buf,
+ &reply_size);
+ if (ret)
+ return ret;
+
+ *value = raw_fan_val_to_rpm(resp_buf[3], resp_buf[4]);
+
+ return 0;
+}
+
+static int iei_wt61p803_puzzle_write_pwm_channel
+(struct iei_wt61p803_puzzle_hwmon *mcu_hwmon, int channel, long pwm_set_val)
+{
+ unsigned char *resp_buf = mcu_hwmon->response_buffer;
+ static const u8 pwm_set_cmds[] = {
+ IEI_WT61P803_PUZZLE_CMD_FAN_PWM_0,
+ IEI_WT61P803_PUZZLE_CMD_FAN_PWM_1
+ };
+ unsigned char pwm_set_cmd[6] = {
+ IEI_WT61P803_PUZZLE_CMD_HEADER_START,
+ IEI_WT61P803_PUZZLE_CMD_FAN,
+ IEI_WT61P803_PUZZLE_CMD_FAN_PWM_WRITE,
+ IEI_WT61P803_PUZZLE_CMD_FAN_PWM_0,
+ 0x00
+ };
+ size_t reply_size = 0;
+ int ret;
+
+ pwm_set_cmd[3] = pwm_set_cmds[channel];
+
+ /* Add the PWM value to the command */
+ pwm_set_cmd[4] = (unsigned char)pwm_set_val;
+
+ ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, pwm_set_cmd,
+ sizeof(pwm_set_cmd), resp_buf,
+ &reply_size);
+ if (ret)
+ return ret;
+
+ /* Store the PWM value */
+ if (!(resp_buf[0] == IEI_WT61P803_PUZZLE_CMD_HEADER_START &&
+ resp_buf[1] == IEI_WT61P803_PUZZLE_CMD_RESPONSE_OK &&
+ resp_buf[2] == IEI_WT61P803_PUZZLE_CHECKSUM_RESPONSE_OK))
+ return -EIO;
+
+ return 0;
+}
+
+static int iei_wt61p803_puzzle_read_pwm_channel
+(struct iei_wt61p803_puzzle_hwmon *mcu_hwmon, int channel, int *value)
+{
+ unsigned char *resp_buf = mcu_hwmon->response_buffer;
+ static const u8 pwm_get_cmds[] = {
+ IEI_WT61P803_PUZZLE_CMD_FAN_PWM_0,
+ IEI_WT61P803_PUZZLE_CMD_FAN_PWM_1
+ };
+ unsigned char pwm_get_cmd[5] = {
+ IEI_WT61P803_PUZZLE_CMD_HEADER_START,
+ IEI_WT61P803_PUZZLE_CMD_FAN,
+ IEI_WT61P803_PUZZLE_CMD_FAN_PWM_READ,
+ IEI_WT61P803_PUZZLE_CMD_FAN_PWM_0
+ };
+ size_t reply_size = 0;
+ int ret;
+
+ pwm_get_cmd[3] = pwm_get_cmds[channel];
+
+ ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, pwm_get_cmd,
+ sizeof(pwm_get_cmd), resp_buf,
+ &reply_size);
+ if (ret)
+ return ret;
+
+ if (resp_buf[2] != IEI_WT61P803_PUZZLE_CMD_FAN_PWM_READ)
+ return -EIO;
+
+ *value = resp_buf[3];
+
+ return 0;
+}
+
+static int iei_wt61p803_puzzle_read(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, long *val)
+{
+ struct iei_wt61p803_puzzle_hwmon *mcu_hwmon =
+ dev_get_drvdata(dev->parent);
+ int ret, value;
+
+ switch (type) {
+ case hwmon_pwm:
+ if (attr != hwmon_pwm_input)
+ return -ENODEV;
+ ret = iei_wt61p803_puzzle_read_pwm_channel(mcu_hwmon, channel, &value);
+ if (ret)
+ return ret;
+ *val = (long)value;
+ return ret;
+ case hwmon_fan:
+ if (attr != hwmon_fan_input)
+ return -ENODEV;
+ ret = iei_wt61p803_puzzle_read_fan_speed(mcu_hwmon, channel, &value);
+ if (ret)
+ return ret;
+ *val = (long)value;
+ return ret;
+ case hwmon_temp:
+ if (attr != hwmon_temp_input)
+ return -ENODEV;
+ ret = iei_wt61p803_puzzle_read_temp_sensor(mcu_hwmon, channel, &value);
+ if (ret)
+ return ret;
+ *val = (long)value;
+ return ret;
+ default:
+ return -ENODEV;
+ }
+}
+
+static int iei_wt61p803_puzzle_write(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, long val)
+{
+ struct iei_wt61p803_puzzle_hwmon *mcu_hwmon =
+ dev_get_drvdata(dev->parent);
+
+ if (attr != hwmon_pwm_input)
+ return -ENODEV;
+ if (mcu_hwmon->thermal_cooling_dev_present[channel]) {
+ /*
+ * The Thermal Framework has already claimed this specific PWM
+ * channel.
+ */
+ return -EBUSY;
+ }
+ return iei_wt61p803_puzzle_write_pwm_channel(mcu_hwmon, channel, val);
+}
+
+static umode_t iei_wt61p803_puzzle_is_visible(const void *data, enum hwmon_sensor_types type,
+ u32 attr, int channel)
+{
+ switch (type) {
+ case hwmon_pwm:
+ switch (attr) {
+ case hwmon_pwm_input:
+ return 0644;
+ default:
+ return 0;
+ }
+ case hwmon_fan:
+ switch (attr) {
+ case hwmon_fan_input:
+ return 0444;
+ default:
+ return 0;
+ }
+ case hwmon_temp:
+ switch (attr) {
+ case hwmon_temp_input:
+ return 0444;
+ default:
+ return 0;
+ }
+ default:
+ return 0;
+ }
+}
+
+static const struct hwmon_ops iei_wt61p803_puzzle_hwmon_ops = {
+ .is_visible = iei_wt61p803_puzzle_is_visible,
+ .read = iei_wt61p803_puzzle_read,
+ .write = iei_wt61p803_puzzle_write,
+};
+
+static const struct hwmon_channel_info *iei_wt61p803_puzzle_info[] = {
+ HWMON_CHANNEL_INFO(pwm,
+ HWMON_PWM_INPUT,
+ HWMON_PWM_INPUT),
+ HWMON_CHANNEL_INFO(fan,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT),
+ HWMON_CHANNEL_INFO(temp,
+ HWMON_T_INPUT,
+ HWMON_T_INPUT),
+ NULL
+};
+
+static const struct hwmon_chip_info iei_wt61p803_puzzle_chip_info = {
+ .ops = &iei_wt61p803_puzzle_hwmon_ops,
+ .info = iei_wt61p803_puzzle_info,
+};
+
+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,
+};
+
+static int iei_wt61p803_puzzle_enable_thermal_cooling_dev
+(struct device *dev, struct fwnode_handle *child, struct iei_wt61p803_puzzle_hwmon *mcu_hwmon)
+{
+ struct iei_wt61p803_puzzle_thermal_cooling_device *cdev;
+ int ret, num_levels;
+ u32 pwm_channel;
+
+ ret = fwnode_property_read_u32(child, "reg", &pwm_channel);
+ if (ret)
+ return ret;
+
+ mcu_hwmon->thermal_cooling_dev_present[pwm_channel] = true;
+
+ num_levels = fwnode_property_read_u8_array(child, "cooling-levels", NULL, 0);
+ 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 = fwnode_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, NULL,
+ 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;
+
+ mcu_hwmon->cdev[pwm_channel] = cdev;
+ }
+ return 0;
+}
+
+static int iei_wt61p803_puzzle_hwmon_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev->parent);
+ struct iei_wt61p803_puzzle_hwmon *mcu_hwmon;
+ struct fwnode_handle *child;
+ struct device *hwmon_dev;
+ int ret;
+
+ mcu_hwmon = devm_kzalloc(dev, sizeof(*mcu_hwmon), GFP_KERNEL);
+ if (!mcu_hwmon)
+ return -ENOMEM;
+
+ mcu_hwmon->response_buffer = devm_kzalloc(dev, IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
+ if (!mcu_hwmon->response_buffer)
+ return -ENOMEM;
+
+ mcu_hwmon->mcu = mcu;
+ platform_set_drvdata(pdev, mcu_hwmon);
+
+ hwmon_dev = devm_hwmon_device_register_with_info(dev, "iei_wt61p803_puzzle",
+ mcu_hwmon,
+ &iei_wt61p803_puzzle_chip_info,
+ NULL);
+
+ if (IS_ERR(hwmon_dev))
+ return PTR_ERR(hwmon_dev);
+
+ /* Control fans via PWM lines via Linux Kernel */
+ if (IS_ENABLED(CONFIG_THERMAL)) {
+ device_for_each_child_node(dev, child) {
+ ret = iei_wt61p803_puzzle_enable_thermal_cooling_dev(dev, child, mcu_hwmon);
+ if (ret) {
+ dev_err(dev, "Enabling the PWM fan failed\n");
+ fwnode_handle_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.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 4/6] drivers: leds: Add the iEi WT61P803 PUZZLE LED driver
2020-10-07 0:48 [PATCH v4 0/6] Add support for the iEi WT61P803 PUZZLE MCU Luka Kovacic
` (2 preceding siblings ...)
2020-10-07 0:48 ` [PATCH v4 3/6] drivers: hwmon: Add the iEi WT61P803 PUZZLE HWMON driver Luka Kovacic
@ 2020-10-07 0:48 ` Luka Kovacic
2020-10-07 11:27 ` Pavel Machek
2020-10-07 0:49 ` [PATCH v4 5/6] Documentation/ABI: Add iei-wt61p803-puzzle driver sysfs interface documentation Luka Kovacic
2020-10-07 0:49 ` [PATCH v4 6/6] MAINTAINERS: Add an entry for the iEi WT61P803 PUZZLE driver Luka Kovacic
5 siblings, 1 reply; 14+ messages in thread
From: Luka Kovacic @ 2020-10-07 0:48 UTC (permalink / raw)
To: linux-kernel, linux-hwmon, linux-leds
Cc: lee.jones, pavel, dmurphy, robh+dt, jdelvare, linux, marek.behun,
luka.perkov, robert.marko, 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>
Cc: Robert Marko <robert.marko@sartura.hr>
---
drivers/leds/Kconfig | 8 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-iei-wt61p803-puzzle.c | 156 ++++++++++++++++++++++++
3 files changed, 165 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..9c5d5c6b4502
--- /dev/null
+++ b/drivers/leds/leds-iei-wt61p803-puzzle.c
@@ -0,0 +1,156 @@
+// 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/leds.h>
+#include <linux/mfd/iei-wt61p803-puzzle.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/slab.h>
+
+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
+ * @response_buffer Global MCU response buffer allocation
+ * @lock: General mutex lock to protect simultaneous R/W access to led_power_state
+ * @led_power_state: State of the front panel power LED
+ * @cdev: LED classdev
+ */
+struct iei_wt61p803_puzzle_led {
+ struct iei_wt61p803_puzzle *mcu;
+ unsigned char *response_buffer;
+ struct mutex lock;
+ int led_power_state;
+ struct led_classdev cdev;
+};
+
+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 *priv = cdev_to_iei_wt61p803_puzzle_led(cdev);
+ unsigned char *resp_buf = priv->response_buffer;
+ unsigned char led_power_cmd[5] = {
+ IEI_WT61P803_PUZZLE_CMD_HEADER_START,
+ IEI_WT61P803_PUZZLE_CMD_LED,
+ IEI_WT61P803_PUZZLE_CMD_LED_POWER,
+ (char)IEI_LED_OFF
+ };
+ size_t reply_size;
+
+ mutex_lock(&priv->lock);
+ if (brightness == LED_OFF) {
+ led_power_cmd[3] = (char)IEI_LED_OFF;
+ priv->led_power_state = LED_OFF;
+ } else {
+ led_power_cmd[3] = (char)IEI_LED_ON;
+ priv->led_power_state = LED_ON;
+ }
+ mutex_unlock(&priv->lock);
+
+ return iei_wt61p803_puzzle_write_command(priv->mcu, led_power_cmd,
+ sizeof(led_power_cmd), resp_buf, &reply_size);
+}
+
+static enum led_brightness
+iei_wt61p803_puzzle_led_brightness_get(struct led_classdev *cdev)
+{
+ struct iei_wt61p803_puzzle_led *priv =
+ cdev_to_iei_wt61p803_puzzle_led(cdev);
+ int led_state;
+
+ mutex_lock(&priv->lock);
+ led_state = priv->led_power_state;
+ mutex_unlock(&priv->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 iei_wt61p803_puzzle_led *priv;
+ struct led_init_data init_data = {};
+ struct fwnode_handle *child;
+ int ret;
+ u32 reg;
+
+ if (device_get_child_node_count(dev) != 1)
+ return -EINVAL;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->response_buffer = devm_kzalloc(dev, IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
+ if (!priv->response_buffer)
+ return -ENOMEM;
+
+ priv->mcu = mcu;
+ priv->led_power_state = 1;
+ mutex_init(&priv->lock);
+ dev_set_drvdata(dev, priv);
+
+ child = device_get_next_child_node(dev, NULL);
+
+ ret = fwnode_property_read_u32(child, "reg", ®);
+ if (ret || reg > 1) {
+ dev_err(dev, "Could not register 'reg' (%lu)\n", (unsigned long)reg);
+ ret = -EINVAL;
+ goto err_child_node;
+ }
+
+ priv->cdev.brightness_set_blocking = iei_wt61p803_puzzle_led_brightness_set_blocking;
+ priv->cdev.brightness_get = iei_wt61p803_puzzle_led_brightness_get;
+ priv->cdev.max_brightness = 1;
+ init_data.fwnode = child;
+
+ ret = devm_led_classdev_register_ext(dev, &priv->cdev, &init_data);
+ if (ret) {
+ dev_err(dev, "Could not register LED\n");
+ goto err_child_node;
+ }
+ return 0;
+err_child_node:
+ fwnode_handle_put(child);
+ return ret;
+}
+
+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.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 5/6] Documentation/ABI: Add iei-wt61p803-puzzle driver sysfs interface documentation
2020-10-07 0:48 [PATCH v4 0/6] Add support for the iEi WT61P803 PUZZLE MCU Luka Kovacic
` (3 preceding siblings ...)
2020-10-07 0:48 ` [PATCH v4 4/6] drivers: leds: Add the iEi WT61P803 PUZZLE LED driver Luka Kovacic
@ 2020-10-07 0:49 ` Luka Kovacic
2020-10-07 0:49 ` [PATCH v4 6/6] MAINTAINERS: Add an entry for the iEi WT61P803 PUZZLE driver Luka Kovacic
5 siblings, 0 replies; 14+ messages in thread
From: Luka Kovacic @ 2020-10-07 0:49 UTC (permalink / raw)
To: linux-kernel, linux-hwmon, linux-leds
Cc: lee.jones, pavel, dmurphy, robh+dt, jdelvare, linux, marek.behun,
luka.perkov, robert.marko, 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>
Cc: Robert Marko <robert.marko@sartura.hr>
---
.../testing/sysfs-driver-iei-wt61p803-puzzle | 55 +++++++++++++++++++
1 file changed, 55 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-iei-wt61p803-puzzle
diff --git a/Documentation/ABI/testing/sysfs-driver-iei-wt61p803-puzzle b/Documentation/ABI/testing/sysfs-driver-iei-wt61p803-puzzle
new file mode 100644
index 000000000000..6e71d85f3296
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-iei-wt61p803-puzzle
@@ -0,0 +1,55 @@
+What: /sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/mac_address_*
+Date: September 2020
+Contact: Luka Kovacic <luka.kovacic@sartura.hr>
+Description: (RW) Internal factory assigned MAC address values
+
+What: /sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/serial_number
+Date: September 2020
+Contact: Luka Kovacic <luka.kovacic@sartura.hr>
+Description: (RW) Internal factory assigned serial number
+
+What: /sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/version
+Date: September 2020
+Contact: Luka Kovacic <luka.kovacic@sartura.hr>
+Description: (RO) Internal MCU firmware version
+
+What: /sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/protocol_version
+Date: September 2020
+Contact: Luka Kovacic <luka.kovacic@sartura.hr>
+Description: (RO) Internal MCU communication protocol version
+
+What: /sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/power_loss_recovery
+Date: September 2020
+Contact: Luka Kovacic <luka.kovacic@sartura.hr>
+Description: (RW) Host platform power loss recovery settings
+ 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: (RO) Internal MCU bootloader mode status
+
+What: /sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/power_status
+Date: September 2020
+Contact: Luka Kovacic <luka.kovacic@sartura.hr>
+Description: (RO) Power status indicates the host platform power on method.
+ 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: (RO) Internal MCU firmware build date
+ 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: (RO) Host platform AC recovery status value
--
2.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 6/6] MAINTAINERS: Add an entry for the iEi WT61P803 PUZZLE driver
2020-10-07 0:48 [PATCH v4 0/6] Add support for the iEi WT61P803 PUZZLE MCU Luka Kovacic
` (4 preceding siblings ...)
2020-10-07 0:49 ` [PATCH v4 5/6] Documentation/ABI: Add iei-wt61p803-puzzle driver sysfs interface documentation Luka Kovacic
@ 2020-10-07 0:49 ` Luka Kovacic
5 siblings, 0 replies; 14+ messages in thread
From: Luka Kovacic @ 2020-10-07 0:49 UTC (permalink / raw)
To: linux-kernel, linux-hwmon, linux-leds
Cc: lee.jones, pavel, dmurphy, robh+dt, jdelvare, linux, marek.behun,
luka.perkov, robert.marko, 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>
Cc: Robert Marko <robert.marko@sartura.hr>
---
MAINTAINERS | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 33b27e62ce19..7b17195511ae 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8455,6 +8455,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.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v4 4/6] drivers: leds: Add the iEi WT61P803 PUZZLE LED driver
2020-10-07 0:48 ` [PATCH v4 4/6] drivers: leds: Add the iEi WT61P803 PUZZLE LED driver Luka Kovacic
@ 2020-10-07 11:27 ` Pavel Machek
2020-10-09 23:25 ` Luka Kovacic
0 siblings, 1 reply; 14+ messages in thread
From: Pavel Machek @ 2020-10-07 11:27 UTC (permalink / raw)
To: Luka Kovacic
Cc: linux-kernel, linux-hwmon, linux-leds, lee.jones, dmurphy,
robh+dt, jdelvare, linux, marek.behun, luka.perkov, robert.marko
[-- Attachment #1: Type: text/plain, Size: 1613 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.
> +static int iei_wt61p803_puzzle_led_brightness_set_blocking(struct led_classdev *cdev,
> + enum led_brightness brightness)
> +{
> + struct iei_wt61p803_puzzle_led *priv = cdev_to_iei_wt61p803_puzzle_led(cdev);
> + unsigned char *resp_buf = priv->response_buffer;
> + unsigned char led_power_cmd[5] = {
> + IEI_WT61P803_PUZZLE_CMD_HEADER_START,
> + IEI_WT61P803_PUZZLE_CMD_LED,
> + IEI_WT61P803_PUZZLE_CMD_LED_POWER,
> + (char)IEI_LED_OFF
> + };
> + size_t reply_size;
> +
> + mutex_lock(&priv->lock);
> + if (brightness == LED_OFF) {
> + led_power_cmd[3] = (char)IEI_LED_OFF;
> + priv->led_power_state = LED_OFF;
> + } else {
> + led_power_cmd[3] = (char)IEI_LED_ON;
> + priv->led_power_state = LED_ON;
> + }
> + mutex_unlock(&priv->lock);
Are you sure you need the mutex?
> + ret = devm_led_classdev_register_ext(dev, &priv->cdev, &init_data);
> + if (ret) {
> + dev_err(dev, "Could not register LED\n");
> + goto err_child_node;
> + }
> + return 0;
> +err_child_node:
> + fwnode_handle_put(child);
> + return ret;
> +}
Is the fwnode_handle_put(child); missing in non-error path somewhere?
> +MODULE_LICENSE("GPL");
Make sure this is consistent with file header. GPLv2+, if you can.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 4/6] drivers: leds: Add the iEi WT61P803 PUZZLE LED driver
2020-10-07 11:27 ` Pavel Machek
@ 2020-10-09 23:25 ` Luka Kovacic
2020-10-10 0:06 ` [PATCH v5 4/7] " Luka Kovacic
0 siblings, 1 reply; 14+ messages in thread
From: Luka Kovacic @ 2020-10-09 23:25 UTC (permalink / raw)
To: Pavel Machek
Cc: Linux Kernel Mailing List, linux-hwmon, Linux LED Subsystem,
Lee Jones, Dan Murphy, Rob Herring, Jean Delvare, Guenter Roeck,
Marek Behun, Luka Perkov, Robert Marko
On Wed, Oct 7, 2020 at 1:27 PM Pavel Machek <pavel@ucw.cz> 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.
>
> > +static int iei_wt61p803_puzzle_led_brightness_set_blocking(struct led_classdev *cdev,
> > + enum led_brightness brightness)
> > +{
> > + struct iei_wt61p803_puzzle_led *priv = cdev_to_iei_wt61p803_puzzle_led(cdev);
> > + unsigned char *resp_buf = priv->response_buffer;
> > + unsigned char led_power_cmd[5] = {
> > + IEI_WT61P803_PUZZLE_CMD_HEADER_START,
> > + IEI_WT61P803_PUZZLE_CMD_LED,
> > + IEI_WT61P803_PUZZLE_CMD_LED_POWER,
> > + (char)IEI_LED_OFF
> > + };
> > + size_t reply_size;
> > +
> > + mutex_lock(&priv->lock);
> > + if (brightness == LED_OFF) {
> > + led_power_cmd[3] = (char)IEI_LED_OFF;
> > + priv->led_power_state = LED_OFF;
> > + } else {
> > + led_power_cmd[3] = (char)IEI_LED_ON;
> > + priv->led_power_state = LED_ON;
> > + }
> > + mutex_unlock(&priv->lock);
>
> Are you sure you need the mutex?
>
> > + ret = devm_led_classdev_register_ext(dev, &priv->cdev, &init_data);
> > + if (ret) {
> > + dev_err(dev, "Could not register LED\n");
> > + goto err_child_node;
> > + }
> > + return 0;
> > +err_child_node:
> > + fwnode_handle_put(child);
> > + return ret;
> > +}
>
> Is the fwnode_handle_put(child); missing in non-error path somewhere?
>
> > +MODULE_LICENSE("GPL");
>
> Make sure this is consistent with file header. GPLv2+, if you can.
>
> Best regards,
>
> Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi Pavel,
The mutex is locked in
iei_wt61p803_puzzle_led_brightness_set_blocking(), as concurrent
access to the same private structure member should be possible, when
reading the state
from iei_wt61p803_puzzle_led_brightness_get().
It does look like I missed the final fwnode_handle_put(child) here.
My understanding regarding the license is that when
MODULE_LICENSE("GPL") is used
the SPDX identifier can either be GPL-2.0-only or GPL-2.0-or-later.
Kind regards,
Luka
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 4/7] drivers: leds: Add the iEi WT61P803 PUZZLE LED driver
2020-10-09 23:25 ` Luka Kovacic
@ 2020-10-10 0:06 ` Luka Kovacic
0 siblings, 0 replies; 14+ messages in thread
From: Luka Kovacic @ 2020-10-10 0:06 UTC (permalink / raw)
To: linux-kernel, linux-hwmon, linux-leds, pavel
Cc: lee.jones, dmurphy, robh+dt, jdelvare, linux, marek.behun,
luka.perkov, robert.marko, 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>
Cc: Robert Marko <robert.marko@sartura.hr>
---
Changes for v5:
- Remove the return before goto to also fwnode_handle_put(child)
when ret is 0.
This change will be added to the patchset changelog.
drivers/leds/Kconfig | 8 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-iei-wt61p803-puzzle.c | 155 ++++++++++++++++++++++++
3 files changed, 164 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..4e287572b1f5
--- /dev/null
+++ b/drivers/leds/leds-iei-wt61p803-puzzle.c
@@ -0,0 +1,155 @@
+// 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/leds.h>
+#include <linux/mfd/iei-wt61p803-puzzle.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/slab.h>
+
+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
+ * @response_buffer Global MCU response buffer allocation
+ * @lock: General mutex lock to protect simultaneous R/W access to led_power_state
+ * @led_power_state: State of the front panel power LED
+ * @cdev: LED classdev
+ */
+struct iei_wt61p803_puzzle_led {
+ struct iei_wt61p803_puzzle *mcu;
+ unsigned char *response_buffer;
+ struct mutex lock;
+ int led_power_state;
+ struct led_classdev cdev;
+};
+
+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 *priv = cdev_to_iei_wt61p803_puzzle_led(cdev);
+ unsigned char *resp_buf = priv->response_buffer;
+ unsigned char led_power_cmd[5] = {
+ IEI_WT61P803_PUZZLE_CMD_HEADER_START,
+ IEI_WT61P803_PUZZLE_CMD_LED,
+ IEI_WT61P803_PUZZLE_CMD_LED_POWER,
+ (char)IEI_LED_OFF
+ };
+ size_t reply_size;
+
+ mutex_lock(&priv->lock);
+ if (brightness == LED_OFF) {
+ led_power_cmd[3] = (char)IEI_LED_OFF;
+ priv->led_power_state = LED_OFF;
+ } else {
+ led_power_cmd[3] = (char)IEI_LED_ON;
+ priv->led_power_state = LED_ON;
+ }
+ mutex_unlock(&priv->lock);
+
+ return iei_wt61p803_puzzle_write_command(priv->mcu, led_power_cmd,
+ sizeof(led_power_cmd), resp_buf, &reply_size);
+}
+
+static enum led_brightness
+iei_wt61p803_puzzle_led_brightness_get(struct led_classdev *cdev)
+{
+ struct iei_wt61p803_puzzle_led *priv =
+ cdev_to_iei_wt61p803_puzzle_led(cdev);
+ int led_state;
+
+ mutex_lock(&priv->lock);
+ led_state = priv->led_power_state;
+ mutex_unlock(&priv->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 iei_wt61p803_puzzle_led *priv;
+ struct led_init_data init_data = {};
+ struct fwnode_handle *child;
+ int ret;
+ u32 reg;
+
+ if (device_get_child_node_count(dev) != 1)
+ return -EINVAL;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->response_buffer = devm_kzalloc(dev, IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
+ if (!priv->response_buffer)
+ return -ENOMEM;
+
+ priv->mcu = mcu;
+ priv->led_power_state = 1;
+ mutex_init(&priv->lock);
+ dev_set_drvdata(dev, priv);
+
+ child = device_get_next_child_node(dev, NULL);
+
+ ret = fwnode_property_read_u32(child, "reg", ®);
+ if (ret || reg > 1) {
+ dev_err(dev, "Could not register 'reg' (%lu)\n", (unsigned long)reg);
+ ret = -EINVAL;
+ goto err_child_node;
+ }
+
+ priv->cdev.brightness_set_blocking = iei_wt61p803_puzzle_led_brightness_set_blocking;
+ priv->cdev.brightness_get = iei_wt61p803_puzzle_led_brightness_get;
+ priv->cdev.max_brightness = 1;
+ init_data.fwnode = child;
+
+ ret = devm_led_classdev_register_ext(dev, &priv->cdev, &init_data);
+ if (ret) {
+ dev_err(dev, "Could not register LED\n");
+ goto err_child_node;
+ }
+err_child_node:
+ fwnode_handle_put(child);
+ return ret;
+}
+
+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.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v4 3/6] drivers: hwmon: Add the iEi WT61P803 PUZZLE HWMON driver
2020-10-07 0:48 ` [PATCH v4 3/6] drivers: hwmon: Add the iEi WT61P803 PUZZLE HWMON driver Luka Kovacic
@ 2020-10-11 21:26 ` Guenter Roeck
2020-10-13 18:09 ` Luka Kovacic
0 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2020-10-11 21:26 UTC (permalink / raw)
To: Luka Kovacic
Cc: linux-kernel, linux-hwmon, linux-leds, lee.jones, pavel, dmurphy,
robh+dt, jdelvare, marek.behun, luka.perkov, robert.marko
On Wed, Oct 07, 2020 at 02:48:58AM +0200, 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>
> Cc: Robert Marko <robert.marko@sartura.hr>
> ---
> drivers/hwmon/Kconfig | 8 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/iei-wt61p803-puzzle-hwmon.c | 457 ++++++++++++++++++++++
> 3 files changed, 466 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..be7b019d126c
> --- /dev/null
> +++ b/drivers/hwmon/iei-wt61p803-puzzle-hwmon.c
> @@ -0,0 +1,457 @@
> +// 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/err.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/hwmon.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/math64.h>
> +#include <linux/mfd/iei-wt61p803-puzzle.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/slab.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
> + * @response_buffer Global MCU response buffer allocation
> + * @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;
> + unsigned char *response_buffer;
> + 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];
> +};
> +
> +#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)
> +{
> + unsigned char *resp_buf = mcu_hwmon->response_buffer;
> + unsigned char temp_sensor_ntc_cmd[4] = {
static ?
> + IEI_WT61P803_PUZZLE_CMD_HEADER_START,
> + IEI_WT61P803_PUZZLE_CMD_TEMP,
> + IEI_WT61P803_PUZZLE_CMD_TEMP_ALL
> + };
> + size_t reply_size = 0;
> + int ret;
> +
> + ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, temp_sensor_ntc_cmd,
> + sizeof(temp_sensor_ntc_cmd), resp_buf,
> + &reply_size);
> +
> + if (ret)
> + return ret;
> +
> + /* Check the number of NTC values (should be 0x32/'2') */
> + if (resp_buf[3] != 0x32)
> + return -EIO;
> +
> + *value = raw_temp_to_milidegree_celsius(resp_buf[4 + channel]);
> +
> + return 0;
> +}
> +
> +#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)
> +{
> + unsigned char *resp_buf = mcu_hwmon->response_buffer;
> + static const u8 fan_speed_cmds[] = {
> + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_0,
> + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_1,
> + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_2,
> + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_3,
> + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_4
> + };
> + unsigned char fan_speed_cmd[4] = {
> + IEI_WT61P803_PUZZLE_CMD_HEADER_START,
> + IEI_WT61P803_PUZZLE_CMD_FAN,
> + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_0
This is unconditionally overwritten below.
> + };
> + size_t reply_size = 0;
> + int ret;
> +
> + fan_speed_cmd[2] = fan_speed_cmds[channel];
> +
> + ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, fan_speed_cmd,
> + sizeof(fan_speed_cmd), resp_buf,
> + &reply_size);
> + if (ret)
> + return ret;
> +
> + *value = raw_fan_val_to_rpm(resp_buf[3], resp_buf[4]);
> +
> + return 0;
> +}
> +
> +static int iei_wt61p803_puzzle_write_pwm_channel
> +(struct iei_wt61p803_puzzle_hwmon *mcu_hwmon, int channel, long pwm_set_val)
> +{
> + unsigned char *resp_buf = mcu_hwmon->response_buffer;
> + static const u8 pwm_set_cmds[] = {
> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_0,
> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_1
> + };
> + unsigned char pwm_set_cmd[6] = {
> + IEI_WT61P803_PUZZLE_CMD_HEADER_START,
> + IEI_WT61P803_PUZZLE_CMD_FAN,
> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_WRITE,
> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_0,
Another unconditional overwrite.
> + 0x00
As is this, and more below. Guess it is on purpose, but I don't
see the point.
> + };
> + size_t reply_size = 0;
> + int ret;
> +
> + pwm_set_cmd[3] = pwm_set_cmds[channel];
> +
> + /* Add the PWM value to the command */
> + pwm_set_cmd[4] = (unsigned char)pwm_set_val;
> +
> + ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, pwm_set_cmd,
> + sizeof(pwm_set_cmd), resp_buf,
> + &reply_size);
> + if (ret)
> + return ret;
> +
> + /* Store the PWM value */
> + if (!(resp_buf[0] == IEI_WT61P803_PUZZLE_CMD_HEADER_START &&
> + resp_buf[1] == IEI_WT61P803_PUZZLE_CMD_RESPONSE_OK &&
> + resp_buf[2] == IEI_WT61P803_PUZZLE_CHECKSUM_RESPONSE_OK))
> + return -EIO;
> +
> + return 0;
> +}
> +
> +static int iei_wt61p803_puzzle_read_pwm_channel
> +(struct iei_wt61p803_puzzle_hwmon *mcu_hwmon, int channel, int *value)
I am sure I mentioned before that I don't like those odd follow-up line
indentations.
> +{
> + unsigned char *resp_buf = mcu_hwmon->response_buffer;
> + static const u8 pwm_get_cmds[] = {
> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_0,
> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_1
> + };
> + unsigned char pwm_get_cmd[5] = {
> + IEI_WT61P803_PUZZLE_CMD_HEADER_START,
> + IEI_WT61P803_PUZZLE_CMD_FAN,
> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_READ,
> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_0
> + };
> + size_t reply_size = 0;
> + int ret;
> +
> + pwm_get_cmd[3] = pwm_get_cmds[channel];
> +
> + ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, pwm_get_cmd,
> + sizeof(pwm_get_cmd), resp_buf,
> + &reply_size);
> + if (ret)
> + return ret;
> +
> + if (resp_buf[2] != IEI_WT61P803_PUZZLE_CMD_FAN_PWM_READ)
> + return -EIO;
> +
> + *value = resp_buf[3];
> +
> + return 0;
> +}
> +
> +static int iei_wt61p803_puzzle_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long *val)
> +{
> + struct iei_wt61p803_puzzle_hwmon *mcu_hwmon =
> + dev_get_drvdata(dev->parent);
> + int ret, value;
> +
> + switch (type) {
> + case hwmon_pwm:
> + if (attr != hwmon_pwm_input)
> + return -ENODEV;
Those are all unnecessary tests, and -ENODEV is wrong.
> + ret = iei_wt61p803_puzzle_read_pwm_channel(mcu_hwmon, channel, &value);
> + if (ret)
> + return ret;
> + *val = (long)value;
Unnecessary type cast. The return value of this function is _always_
type casted, which really doesn't make sense.
> + return ret;
> + case hwmon_fan:
> + if (attr != hwmon_fan_input)
> + return -ENODEV;
> + ret = iei_wt61p803_puzzle_read_fan_speed(mcu_hwmon, channel, &value);
> + if (ret)
> + return ret;
> + *val = (long)value;
> + return ret;
> + case hwmon_temp:
> + if (attr != hwmon_temp_input)
> + return -ENODEV;
> + ret = iei_wt61p803_puzzle_read_temp_sensor(mcu_hwmon, channel, &value);
> + if (ret)
> + return ret;
> + *val = (long)value;
Hmm, seems like you like type casts. Ok, I'll want to see an explanation
for each of those: Explain why the function argument is an int *, and
explain why the it is necessary to type cast the result here.
> + return ret;
> + default:
> + return -ENODEV;
> + }
> +}
> +
> +static int iei_wt61p803_puzzle_write(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long val)
> +{
> + struct iei_wt61p803_puzzle_hwmon *mcu_hwmon =
> + dev_get_drvdata(dev->parent);
> +
> + if (attr != hwmon_pwm_input)
> + return -ENODEV;
> + if (mcu_hwmon->thermal_cooling_dev_present[channel]) {
> + /*
> + * The Thermal Framework has already claimed this specific PWM
> + * channel.
... and if it did, this code won't be called. I am sure I mentioned
this before. If you insist keeping it, I'll want to see an explanation
here because I don't want to have to deal with follow-up patches removing
the unnecessary code.
> + */
> + return -EBUSY;
> + }
> + return iei_wt61p803_puzzle_write_pwm_channel(mcu_hwmon, channel, val);
> +}
> +
> +static umode_t iei_wt61p803_puzzle_is_visible(const void *data, enum hwmon_sensor_types type,
> + u32 attr, int channel)
> +{
> + switch (type) {
> + case hwmon_pwm:
> + switch (attr) {
> + case hwmon_pwm_input:
> + return 0644;
> + default:
> + return 0;
> + }
> + case hwmon_fan:
> + switch (attr) {
> + case hwmon_fan_input:
> + return 0444;
> + default:
> + return 0;
> + }
> + case hwmon_temp:
> + switch (attr) {
> + case hwmon_temp_input:
> + return 0444;
> + default:
> + return 0;
> + }
> + default:
> + return 0;
> + }
> +}
> +
> +static const struct hwmon_ops iei_wt61p803_puzzle_hwmon_ops = {
> + .is_visible = iei_wt61p803_puzzle_is_visible,
> + .read = iei_wt61p803_puzzle_read,
> + .write = iei_wt61p803_puzzle_write,
> +};
> +
> +static const struct hwmon_channel_info *iei_wt61p803_puzzle_info[] = {
> + HWMON_CHANNEL_INFO(pwm,
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT),
> + HWMON_CHANNEL_INFO(fan,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT),
> + HWMON_CHANNEL_INFO(temp,
> + HWMON_T_INPUT,
> + HWMON_T_INPUT),
> + NULL
> +};
> +
> +static const struct hwmon_chip_info iei_wt61p803_puzzle_chip_info = {
> + .ops = &iei_wt61p803_puzzle_hwmon_ops,
> + .info = iei_wt61p803_puzzle_info,
> +};
> +
> +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;
This check needs to be explained.
> +
> + 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,
> +};
> +
> +static int iei_wt61p803_puzzle_enable_thermal_cooling_dev
> +(struct device *dev, struct fwnode_handle *child, struct iei_wt61p803_puzzle_hwmon *mcu_hwmon)
> +{
> + struct iei_wt61p803_puzzle_thermal_cooling_device *cdev;
> + int ret, num_levels;
> + u32 pwm_channel;
> +
> + ret = fwnode_property_read_u32(child, "reg", &pwm_channel);
> + if (ret)
> + return ret;
> +
> + mcu_hwmon->thermal_cooling_dev_present[pwm_channel] = true;
> +
> + num_levels = fwnode_property_read_u8_array(child, "cooling-levels", NULL, 0);
> + 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 = fwnode_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, NULL,
> + 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;
> +
> + mcu_hwmon->cdev[pwm_channel] = cdev;
> + }
> + return 0;
> +}
> +
> +static int iei_wt61p803_puzzle_hwmon_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev->parent);
> + struct iei_wt61p803_puzzle_hwmon *mcu_hwmon;
> + struct fwnode_handle *child;
> + struct device *hwmon_dev;
> + int ret;
> +
> + mcu_hwmon = devm_kzalloc(dev, sizeof(*mcu_hwmon), GFP_KERNEL);
> + if (!mcu_hwmon)
> + return -ENOMEM;
> +
> + mcu_hwmon->response_buffer = devm_kzalloc(dev, IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
> + if (!mcu_hwmon->response_buffer)
> + return -ENOMEM;
> +
> + mcu_hwmon->mcu = mcu;
> + platform_set_drvdata(pdev, mcu_hwmon);
> +
> + hwmon_dev = devm_hwmon_device_register_with_info(dev, "iei_wt61p803_puzzle",
> + mcu_hwmon,
> + &iei_wt61p803_puzzle_chip_info,
> + NULL);
> +
> + if (IS_ERR(hwmon_dev))
> + return PTR_ERR(hwmon_dev);
> +
> + /* Control fans via PWM lines via Linux Kernel */
> + if (IS_ENABLED(CONFIG_THERMAL)) {
> + device_for_each_child_node(dev, child) {
> + ret = iei_wt61p803_puzzle_enable_thermal_cooling_dev(dev, child, mcu_hwmon);
> + if (ret) {
> + dev_err(dev, "Enabling the PWM fan failed\n");
> + fwnode_handle_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.26.2
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 3/6] drivers: hwmon: Add the iEi WT61P803 PUZZLE HWMON driver
2020-10-11 21:26 ` Guenter Roeck
@ 2020-10-13 18:09 ` Luka Kovacic
2020-10-13 18:51 ` Guenter Roeck
0 siblings, 1 reply; 14+ messages in thread
From: Luka Kovacic @ 2020-10-13 18:09 UTC (permalink / raw)
To: Guenter Roeck
Cc: Linux Kernel Mailing List, linux-hwmon, Linux LED Subsystem,
Lee Jones, Pavel Machek, Dan Murphy, Rob Herring, Jean Delvare,
Marek Behun, Luka Perkov, Robert Marko
Hello Guenter,
On Sun, Oct 11, 2020 at 11:26 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Wed, Oct 07, 2020 at 02:48:58AM +0200, 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>
> > Cc: Robert Marko <robert.marko@sartura.hr>
> > ---
> > drivers/hwmon/Kconfig | 8 +
> > drivers/hwmon/Makefile | 1 +
> > drivers/hwmon/iei-wt61p803-puzzle-hwmon.c | 457 ++++++++++++++++++++++
> > 3 files changed, 466 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..be7b019d126c
> > --- /dev/null
> > +++ b/drivers/hwmon/iei-wt61p803-puzzle-hwmon.c
> > @@ -0,0 +1,457 @@
> > +// 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/err.h>
> > +#include <linux/hwmon-sysfs.h>
> > +#include <linux/hwmon.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/irq.h>
> > +#include <linux/math64.h>
> > +#include <linux/mfd/iei-wt61p803-puzzle.h>
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/property.h>
> > +#include <linux/slab.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
> > + * @response_buffer Global MCU response buffer allocation
> > + * @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;
> > + unsigned char *response_buffer;
> > + 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];
> > +};
> > +
> > +#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)
> > +{
> > + unsigned char *resp_buf = mcu_hwmon->response_buffer;
> > + unsigned char temp_sensor_ntc_cmd[4] = {
>
> static ?
These can be changed to static to avoid reallocations, every time the
function is called.
Although I will always have to assign these values anyway.
>
> > + IEI_WT61P803_PUZZLE_CMD_HEADER_START,
> > + IEI_WT61P803_PUZZLE_CMD_TEMP,
> > + IEI_WT61P803_PUZZLE_CMD_TEMP_ALL
> > + };
> > + size_t reply_size = 0;
> > + int ret;
> > +
> > + ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, temp_sensor_ntc_cmd,
> > + sizeof(temp_sensor_ntc_cmd), resp_buf,
> > + &reply_size);
> > +
> > + if (ret)
> > + return ret;
> > +
> > + /* Check the number of NTC values (should be 0x32/'2') */
> > + if (resp_buf[3] != 0x32)
> > + return -EIO;
> > +
> > + *value = raw_temp_to_milidegree_celsius(resp_buf[4 + channel]);
> > +
> > + return 0;
> > +}
> > +
> > +#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)
> > +{
> > + unsigned char *resp_buf = mcu_hwmon->response_buffer;
> > + static const u8 fan_speed_cmds[] = {
> > + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_0,
> > + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_1,
> > + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_2,
> > + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_3,
> > + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_4
> > + };
> > + unsigned char fan_speed_cmd[4] = {
> > + IEI_WT61P803_PUZZLE_CMD_HEADER_START,
> > + IEI_WT61P803_PUZZLE_CMD_FAN,
> > + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_0
>
> This is unconditionally overwritten below.
I also add it to the array, to make it easier to read if someone is
debugging the code.
>
> > + };
> > + size_t reply_size = 0;
> > + int ret;
> > +
> > + fan_speed_cmd[2] = fan_speed_cmds[channel];
> > +
> > + ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, fan_speed_cmd,
> > + sizeof(fan_speed_cmd), resp_buf,
> > + &reply_size);
> > + if (ret)
> > + return ret;
> > +
> > + *value = raw_fan_val_to_rpm(resp_buf[3], resp_buf[4]);
> > +
> > + return 0;
> > +}
> > +
> > +static int iei_wt61p803_puzzle_write_pwm_channel
> > +(struct iei_wt61p803_puzzle_hwmon *mcu_hwmon, int channel, long pwm_set_val)
> > +{
> > + unsigned char *resp_buf = mcu_hwmon->response_buffer;
> > + static const u8 pwm_set_cmds[] = {
> > + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_0,
> > + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_1
> > + };
> > + unsigned char pwm_set_cmd[6] = {
> > + IEI_WT61P803_PUZZLE_CMD_HEADER_START,
> > + IEI_WT61P803_PUZZLE_CMD_FAN,
> > + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_WRITE,
> > + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_0,
>
> Another unconditional overwrite.
>
> > + 0x00
>
> As is this, and more below. Guess it is on purpose, but I don't
> see the point.
This one can be removed as it's calculated by the
iei_wt61p803_puzzle_write_command
function.
>
> > + };
> > + size_t reply_size = 0;
> > + int ret;
> > +
> > + pwm_set_cmd[3] = pwm_set_cmds[channel];
> > +
> > + /* Add the PWM value to the command */
> > + pwm_set_cmd[4] = (unsigned char)pwm_set_val;
> > +
> > + ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, pwm_set_cmd,
> > + sizeof(pwm_set_cmd), resp_buf,
> > + &reply_size);
> > + if (ret)
> > + return ret;
> > +
> > + /* Store the PWM value */
> > + if (!(resp_buf[0] == IEI_WT61P803_PUZZLE_CMD_HEADER_START &&
> > + resp_buf[1] == IEI_WT61P803_PUZZLE_CMD_RESPONSE_OK &&
> > + resp_buf[2] == IEI_WT61P803_PUZZLE_CHECKSUM_RESPONSE_OK))
> > + return -EIO;
> > +
> > + return 0;
> > +}
> > +
> > +static int iei_wt61p803_puzzle_read_pwm_channel
> > +(struct iei_wt61p803_puzzle_hwmon *mcu_hwmon, int channel, int *value)
>
> I am sure I mentioned before that I don't like those odd follow-up line
> indentations.
I'll fix these, sorry.
>
> > +{
> > + unsigned char *resp_buf = mcu_hwmon->response_buffer;
> > + static const u8 pwm_get_cmds[] = {
> > + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_0,
> > + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_1
> > + };
> > + unsigned char pwm_get_cmd[5] = {
> > + IEI_WT61P803_PUZZLE_CMD_HEADER_START,
> > + IEI_WT61P803_PUZZLE_CMD_FAN,
> > + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_READ,
> > + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_0
> > + };
> > + size_t reply_size = 0;
> > + int ret;
> > +
> > + pwm_get_cmd[3] = pwm_get_cmds[channel];
> > +
> > + ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, pwm_get_cmd,
> > + sizeof(pwm_get_cmd), resp_buf,
> > + &reply_size);
> > + if (ret)
> > + return ret;
> > +
> > + if (resp_buf[2] != IEI_WT61P803_PUZZLE_CMD_FAN_PWM_READ)
> > + return -EIO;
> > +
> > + *value = resp_buf[3];
> > +
> > + return 0;
> > +}
> > +
> > +static int iei_wt61p803_puzzle_read(struct device *dev, enum hwmon_sensor_types type,
> > + u32 attr, int channel, long *val)
> > +{
> > + struct iei_wt61p803_puzzle_hwmon *mcu_hwmon =
> > + dev_get_drvdata(dev->parent);
> > + int ret, value;
> > +
> > + switch (type) {
> > + case hwmon_pwm:
> > + if (attr != hwmon_pwm_input)
> > + return -ENODEV;
>
> Those are all unnecessary tests, and -ENODEV is wrong.
Okay, I can remove the attr tests.
>
> > + ret = iei_wt61p803_puzzle_read_pwm_channel(mcu_hwmon, channel, &value);
> > + if (ret)
> > + return ret;
> > + *val = (long)value;
>
> Unnecessary type cast. The return value of this function is _always_
> type casted, which really doesn't make sense.
>
> > + return ret;
> > + case hwmon_fan:
> > + if (attr != hwmon_fan_input)
> > + return -ENODEV;
> > + ret = iei_wt61p803_puzzle_read_fan_speed(mcu_hwmon, channel, &value);
> > + if (ret)
> > + return ret;
> > + *val = (long)value;
> > + return ret;
> > + case hwmon_temp:
> > + if (attr != hwmon_temp_input)
> > + return -ENODEV;
> > + ret = iei_wt61p803_puzzle_read_temp_sensor(mcu_hwmon, channel, &value);
> > + if (ret)
> > + return ret;
> > + *val = (long)value;
>
> Hmm, seems like you like type casts. Ok, I'll want to see an explanation
> for each of those: Explain why the function argument is an int *, and
> explain why the it is necessary to type cast the result here.
I'll change the function argument to long * to avoid these.
>
> > + return ret;
> > + default:
> > + return -ENODEV;
> > + }
> > +}
> > +
> > +static int iei_wt61p803_puzzle_write(struct device *dev, enum hwmon_sensor_types type,
> > + u32 attr, int channel, long val)
> > +{
> > + struct iei_wt61p803_puzzle_hwmon *mcu_hwmon =
> > + dev_get_drvdata(dev->parent);
> > +
> > + if (attr != hwmon_pwm_input)
> > + return -ENODEV;
> > + if (mcu_hwmon->thermal_cooling_dev_present[channel]) {
> > + /*
> > + * The Thermal Framework has already claimed this specific PWM
> > + * channel.
>
> ... and if it did, this code won't be called. I am sure I mentioned
> this before. If you insist keeping it, I'll want to see an explanation
> here because I don't want to have to deal with follow-up patches removing
> the unnecessary code.
In the previous patch this was done here and by setting the visibility
(umode_t).
I'd rather return an error with more context to the user instead of
just changing
the permissions on the sysfs attribute.
>
> > + */
> > + return -EBUSY;
> > + }
> > + return iei_wt61p803_puzzle_write_pwm_channel(mcu_hwmon, channel, val);
> > +}
> > +
> > +static umode_t iei_wt61p803_puzzle_is_visible(const void *data, enum hwmon_sensor_types type,
> > + u32 attr, int channel)
> > +{
> > + switch (type) {
> > + case hwmon_pwm:
> > + switch (attr) {
> > + case hwmon_pwm_input:
> > + return 0644;
> > + default:
> > + return 0;
> > + }
> > + case hwmon_fan:
> > + switch (attr) {
> > + case hwmon_fan_input:
> > + return 0444;
> > + default:
> > + return 0;
> > + }
> > + case hwmon_temp:
> > + switch (attr) {
> > + case hwmon_temp_input:
> > + return 0444;
> > + default:
> > + return 0;
> > + }
> > + default:
> > + return 0;
> > + }
> > +}
> > +
> > +static const struct hwmon_ops iei_wt61p803_puzzle_hwmon_ops = {
> > + .is_visible = iei_wt61p803_puzzle_is_visible,
> > + .read = iei_wt61p803_puzzle_read,
> > + .write = iei_wt61p803_puzzle_write,
> > +};
> > +
> > +static const struct hwmon_channel_info *iei_wt61p803_puzzle_info[] = {
> > + HWMON_CHANNEL_INFO(pwm,
> > + HWMON_PWM_INPUT,
> > + HWMON_PWM_INPUT),
> > + HWMON_CHANNEL_INFO(fan,
> > + HWMON_F_INPUT,
> > + HWMON_F_INPUT,
> > + HWMON_F_INPUT,
> > + HWMON_F_INPUT,
> > + HWMON_F_INPUT),
> > + HWMON_CHANNEL_INFO(temp,
> > + HWMON_T_INPUT,
> > + HWMON_T_INPUT),
> > + NULL
> > +};
> > +
> > +static const struct hwmon_chip_info iei_wt61p803_puzzle_chip_info = {
> > + .ops = &iei_wt61p803_puzzle_hwmon_ops,
> > + .info = iei_wt61p803_puzzle_info,
> > +};
> > +
> > +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;
>
> This check needs to be explained.
This is not needed and will be removed.
>
> > +
> > + 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,
> > +};
> > +
> > +static int iei_wt61p803_puzzle_enable_thermal_cooling_dev
> > +(struct device *dev, struct fwnode_handle *child, struct iei_wt61p803_puzzle_hwmon *mcu_hwmon)
> > +{
> > + struct iei_wt61p803_puzzle_thermal_cooling_device *cdev;
> > + int ret, num_levels;
> > + u32 pwm_channel;
> > +
> > + ret = fwnode_property_read_u32(child, "reg", &pwm_channel);
> > + if (ret)
> > + return ret;
> > +
> > + mcu_hwmon->thermal_cooling_dev_present[pwm_channel] = true;
> > +
> > + num_levels = fwnode_property_read_u8_array(child, "cooling-levels", NULL, 0);
> > + 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 = fwnode_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, NULL,
> > + 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;
> > +
> > + mcu_hwmon->cdev[pwm_channel] = cdev;
> > + }
> > + return 0;
> > +}
> > +
> > +static int iei_wt61p803_puzzle_hwmon_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev->parent);
> > + struct iei_wt61p803_puzzle_hwmon *mcu_hwmon;
> > + struct fwnode_handle *child;
> > + struct device *hwmon_dev;
> > + int ret;
> > +
> > + mcu_hwmon = devm_kzalloc(dev, sizeof(*mcu_hwmon), GFP_KERNEL);
> > + if (!mcu_hwmon)
> > + return -ENOMEM;
> > +
> > + mcu_hwmon->response_buffer = devm_kzalloc(dev, IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
> > + if (!mcu_hwmon->response_buffer)
> > + return -ENOMEM;
> > +
> > + mcu_hwmon->mcu = mcu;
> > + platform_set_drvdata(pdev, mcu_hwmon);
> > +
> > + hwmon_dev = devm_hwmon_device_register_with_info(dev, "iei_wt61p803_puzzle",
> > + mcu_hwmon,
> > + &iei_wt61p803_puzzle_chip_info,
> > + NULL);
> > +
> > + if (IS_ERR(hwmon_dev))
> > + return PTR_ERR(hwmon_dev);
> > +
> > + /* Control fans via PWM lines via Linux Kernel */
> > + if (IS_ENABLED(CONFIG_THERMAL)) {
> > + device_for_each_child_node(dev, child) {
> > + ret = iei_wt61p803_puzzle_enable_thermal_cooling_dev(dev, child, mcu_hwmon);
> > + if (ret) {
> > + dev_err(dev, "Enabling the PWM fan failed\n");
> > + fwnode_handle_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.26.2
> >
Kind regards,
Luka
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 3/6] drivers: hwmon: Add the iEi WT61P803 PUZZLE HWMON driver
2020-10-13 18:09 ` Luka Kovacic
@ 2020-10-13 18:51 ` Guenter Roeck
2020-10-14 18:25 ` Luka Kovacic
0 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2020-10-13 18:51 UTC (permalink / raw)
To: Luka Kovacic
Cc: Linux Kernel Mailing List, linux-hwmon, Linux LED Subsystem,
Lee Jones, Pavel Machek, Dan Murphy, Rob Herring, Jean Delvare,
Marek Behun, Luka Perkov, Robert Marko
On 10/13/20 11:09 AM, Luka Kovacic wrote:
> Hello Guenter,
>
> On Sun, Oct 11, 2020 at 11:26 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On Wed, Oct 07, 2020 at 02:48:58AM +0200, 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>
>>> Cc: Robert Marko <robert.marko@sartura.hr>
>>> ---
>>> drivers/hwmon/Kconfig | 8 +
>>> drivers/hwmon/Makefile | 1 +
>>> drivers/hwmon/iei-wt61p803-puzzle-hwmon.c | 457 ++++++++++++++++++++++
>>> 3 files changed, 466 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..be7b019d126c
>>> --- /dev/null
>>> +++ b/drivers/hwmon/iei-wt61p803-puzzle-hwmon.c
>>> @@ -0,0 +1,457 @@
>>> +// 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/err.h>
>>> +#include <linux/hwmon-sysfs.h>
>>> +#include <linux/hwmon.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/irq.h>
>>> +#include <linux/math64.h>
>>> +#include <linux/mfd/iei-wt61p803-puzzle.h>
>>> +#include <linux/mod_devicetable.h>
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/property.h>
>>> +#include <linux/slab.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
>>> + * @response_buffer Global MCU response buffer allocation
>>> + * @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;
>>> + unsigned char *response_buffer;
>>> + 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];
>>> +};
>>> +
>>> +#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)
>>> +{
>>> + unsigned char *resp_buf = mcu_hwmon->response_buffer;
>>> + unsigned char temp_sensor_ntc_cmd[4] = {
>>
>> static ?
> These can be changed to static to avoid reallocations, every time the
> function is called.
> Although I will always have to assign these values anyway.
>>
>>> + IEI_WT61P803_PUZZLE_CMD_HEADER_START,
>>> + IEI_WT61P803_PUZZLE_CMD_TEMP,
>>> + IEI_WT61P803_PUZZLE_CMD_TEMP_ALL
>>> + };
>>> + size_t reply_size = 0;
>>> + int ret;
>>> +
>>> + ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, temp_sensor_ntc_cmd,
>>> + sizeof(temp_sensor_ntc_cmd), resp_buf,
>>> + &reply_size);
>>> +
>>> + if (ret)
>>> + return ret;
>>> +
>>> + /* Check the number of NTC values (should be 0x32/'2') */
>>> + if (resp_buf[3] != 0x32)
>>> + return -EIO;
>>> +
>>> + *value = raw_temp_to_milidegree_celsius(resp_buf[4 + channel]);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +#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)
>>> +{
>>> + unsigned char *resp_buf = mcu_hwmon->response_buffer;
>>> + static const u8 fan_speed_cmds[] = {
>>> + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_0,
>>> + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_1,
>>> + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_2,
>>> + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_3,
>>> + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_4
>>> + };
>>> + unsigned char fan_speed_cmd[4] = {
>>> + IEI_WT61P803_PUZZLE_CMD_HEADER_START,
>>> + IEI_WT61P803_PUZZLE_CMD_FAN,
>>> + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_0
>>
>> This is unconditionally overwritten below.
> I also add it to the array, to make it easier to read if someone is
> debugging the code.
That doesn't make it easier. It causes everyone (including me) to wonder
if the overwrite index is wrong, and having to look into details
to notice that the overwritten value is similar. And that has to be
repeated for each of the functions over and over again, because
after all one can not be sure if the submitter maybe got
some index wrong.
That kind of code in your driver caused me to get stuck on that level
of review, and I didn't even get to the point where I actually reviewed
the _real_ code. That kind of code is extremely difficult to review,
it takes a lot of time to do it, and it can easily result in missing
the real problems.
Really, code such as
unsigned char fan_speed_cmd[4] = {};
...
fan_speed_cmd[0] = IEI_WT61P803_PUZZLE_CMD_HEADER_START;
fan_speed_cmd[1] = IEI_WT61P803_PUZZLE_CMD_FAN;
fan_speed_cmd[2] = fan_speed_cmds[channel];
may not look as fancy, but it would be much easier to understand,
and it would be much easier to see that it does what it is supposed
to do.
FWIW, the same is true for the actual commands. It is obviously
your call, but something like
#define IEI_WT61P803_PUZZLE_CMD_FAN_RPM_BASE 'A'
#define IEI_WT61P803_PUZZLE_CMD_FAN_RPM(x) (IEI_WT61P803_PUZZLE_CMD_FAN_RPM_BASE + (x))
fan_speed_cmd[2] = IEI_WT61P803_PUZZLE_CMD_FAN_RPM(channel)
would be much easier to understand and be more straightforward
than all those arrays.
Guenter
>>
>>> + };
>>> + size_t reply_size = 0;
>>> + int ret;
>>> +
>>> + fan_speed_cmd[2] = fan_speed_cmds[channel];
>>> +
>>> + ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, fan_speed_cmd,
>>> + sizeof(fan_speed_cmd), resp_buf,
>>> + &reply_size);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + *value = raw_fan_val_to_rpm(resp_buf[3], resp_buf[4]);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int iei_wt61p803_puzzle_write_pwm_channel
>>> +(struct iei_wt61p803_puzzle_hwmon *mcu_hwmon, int channel, long pwm_set_val)
>>> +{
>>> + unsigned char *resp_buf = mcu_hwmon->response_buffer;
>>> + static const u8 pwm_set_cmds[] = {
>>> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_0,
>>> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_1
>>> + };
>>> + unsigned char pwm_set_cmd[6] = {
>>> + IEI_WT61P803_PUZZLE_CMD_HEADER_START,
>>> + IEI_WT61P803_PUZZLE_CMD_FAN,
>>> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_WRITE,
>>> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_0,
>>
>> Another unconditional overwrite.
>>
>>> + 0x00
>>
>> As is this, and more below. Guess it is on purpose, but I don't
>> see the point.
> This one can be removed as it's calculated by the
> iei_wt61p803_puzzle_write_command
> function.
>>
>>> + };
>>> + size_t reply_size = 0;
>>> + int ret;
>>> +
>>> + pwm_set_cmd[3] = pwm_set_cmds[channel];
>>> +
>>> + /* Add the PWM value to the command */
>>> + pwm_set_cmd[4] = (unsigned char)pwm_set_val;
>>> +
>>> + ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, pwm_set_cmd,
>>> + sizeof(pwm_set_cmd), resp_buf,
>>> + &reply_size);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + /* Store the PWM value */
>>> + if (!(resp_buf[0] == IEI_WT61P803_PUZZLE_CMD_HEADER_START &&
>>> + resp_buf[1] == IEI_WT61P803_PUZZLE_CMD_RESPONSE_OK &&
>>> + resp_buf[2] == IEI_WT61P803_PUZZLE_CHECKSUM_RESPONSE_OK))
>>> + return -EIO;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int iei_wt61p803_puzzle_read_pwm_channel
>>> +(struct iei_wt61p803_puzzle_hwmon *mcu_hwmon, int channel, int *value)
>>
>> I am sure I mentioned before that I don't like those odd follow-up line
>> indentations.
> I'll fix these, sorry.
>>
>>> +{
>>> + unsigned char *resp_buf = mcu_hwmon->response_buffer;
>>> + static const u8 pwm_get_cmds[] = {
>>> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_0,
>>> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_1
>>> + };
>>> + unsigned char pwm_get_cmd[5] = {
>>> + IEI_WT61P803_PUZZLE_CMD_HEADER_START,
>>> + IEI_WT61P803_PUZZLE_CMD_FAN,
>>> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_READ,
>>> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_0
>>> + };
>>> + size_t reply_size = 0;
>>> + int ret;
>>> +
>>> + pwm_get_cmd[3] = pwm_get_cmds[channel];
>>> +
>>> + ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, pwm_get_cmd,
>>> + sizeof(pwm_get_cmd), resp_buf,
>>> + &reply_size);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (resp_buf[2] != IEI_WT61P803_PUZZLE_CMD_FAN_PWM_READ)
>>> + return -EIO;
>>> +
>>> + *value = resp_buf[3];
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int iei_wt61p803_puzzle_read(struct device *dev, enum hwmon_sensor_types type,
>>> + u32 attr, int channel, long *val)
>>> +{
>>> + struct iei_wt61p803_puzzle_hwmon *mcu_hwmon =
>>> + dev_get_drvdata(dev->parent);
>>> + int ret, value;
>>> +
>>> + switch (type) {
>>> + case hwmon_pwm:
>>> + if (attr != hwmon_pwm_input)
>>> + return -ENODEV;
>>
>> Those are all unnecessary tests, and -ENODEV is wrong.
> Okay, I can remove the attr tests.
>>
>>> + ret = iei_wt61p803_puzzle_read_pwm_channel(mcu_hwmon, channel, &value);
>>> + if (ret)
>>> + return ret;
>>> + *val = (long)value;
>>
>> Unnecessary type cast. The return value of this function is _always_
>> type casted, which really doesn't make sense.
>>
>>> + return ret;
>>> + case hwmon_fan:
>>> + if (attr != hwmon_fan_input)
>>> + return -ENODEV;
>>> + ret = iei_wt61p803_puzzle_read_fan_speed(mcu_hwmon, channel, &value);
>>> + if (ret)
>>> + return ret;
>>> + *val = (long)value;
>>> + return ret;
>>> + case hwmon_temp:
>>> + if (attr != hwmon_temp_input)
>>> + return -ENODEV;
>>> + ret = iei_wt61p803_puzzle_read_temp_sensor(mcu_hwmon, channel, &value);
>>> + if (ret)
>>> + return ret;
>>> + *val = (long)value;
>>
>> Hmm, seems like you like type casts. Ok, I'll want to see an explanation
>> for each of those: Explain why the function argument is an int *, and
>> explain why the it is necessary to type cast the result here.
> I'll change the function argument to long * to avoid these.
>>
>>> + return ret;
>>> + default:
>>> + return -ENODEV;
>>> + }
>>> +}
>>> +
>>> +static int iei_wt61p803_puzzle_write(struct device *dev, enum hwmon_sensor_types type,
>>> + u32 attr, int channel, long val)
>>> +{
>>> + struct iei_wt61p803_puzzle_hwmon *mcu_hwmon =
>>> + dev_get_drvdata(dev->parent);
>>> +
>>> + if (attr != hwmon_pwm_input)
>>> + return -ENODEV;
>>> + if (mcu_hwmon->thermal_cooling_dev_present[channel]) {
>>> + /*
>>> + * The Thermal Framework has already claimed this specific PWM
>>> + * channel.
>>
>> ... and if it did, this code won't be called. I am sure I mentioned
>> this before. If you insist keeping it, I'll want to see an explanation
>> here because I don't want to have to deal with follow-up patches removing
>> the unnecessary code.
> In the previous patch this was done here and by setting the visibility
> (umode_t).
> I'd rather return an error with more context to the user instead of
> just changing
> the permissions on the sysfs attribute.
>>
>>> + */
>>> + return -EBUSY;
>>> + }
>>> + return iei_wt61p803_puzzle_write_pwm_channel(mcu_hwmon, channel, val);
>>> +}
>>> +
>>> +static umode_t iei_wt61p803_puzzle_is_visible(const void *data, enum hwmon_sensor_types type,
>>> + u32 attr, int channel)
>>> +{
>>> + switch (type) {
>>> + case hwmon_pwm:
>>> + switch (attr) {
>>> + case hwmon_pwm_input:
>>> + return 0644;
>>> + default:
>>> + return 0;
>>> + }
>>> + case hwmon_fan:
>>> + switch (attr) {
>>> + case hwmon_fan_input:
>>> + return 0444;
>>> + default:
>>> + return 0;
>>> + }
>>> + case hwmon_temp:
>>> + switch (attr) {
>>> + case hwmon_temp_input:
>>> + return 0444;
>>> + default:
>>> + return 0;
>>> + }
>>> + default:
>>> + return 0;
>>> + }
>>> +}
>>> +
>>> +static const struct hwmon_ops iei_wt61p803_puzzle_hwmon_ops = {
>>> + .is_visible = iei_wt61p803_puzzle_is_visible,
>>> + .read = iei_wt61p803_puzzle_read,
>>> + .write = iei_wt61p803_puzzle_write,
>>> +};
>>> +
>>> +static const struct hwmon_channel_info *iei_wt61p803_puzzle_info[] = {
>>> + HWMON_CHANNEL_INFO(pwm,
>>> + HWMON_PWM_INPUT,
>>> + HWMON_PWM_INPUT),
>>> + HWMON_CHANNEL_INFO(fan,
>>> + HWMON_F_INPUT,
>>> + HWMON_F_INPUT,
>>> + HWMON_F_INPUT,
>>> + HWMON_F_INPUT,
>>> + HWMON_F_INPUT),
>>> + HWMON_CHANNEL_INFO(temp,
>>> + HWMON_T_INPUT,
>>> + HWMON_T_INPUT),
>>> + NULL
>>> +};
>>> +
>>> +static const struct hwmon_chip_info iei_wt61p803_puzzle_chip_info = {
>>> + .ops = &iei_wt61p803_puzzle_hwmon_ops,
>>> + .info = iei_wt61p803_puzzle_info,
>>> +};
>>> +
>>> +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;
>>
>> This check needs to be explained.
> This is not needed and will be removed.
>>
>>> +
>>> + 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,
>>> +};
>>> +
>>> +static int iei_wt61p803_puzzle_enable_thermal_cooling_dev
>>> +(struct device *dev, struct fwnode_handle *child, struct iei_wt61p803_puzzle_hwmon *mcu_hwmon)
>>> +{
>>> + struct iei_wt61p803_puzzle_thermal_cooling_device *cdev;
>>> + int ret, num_levels;
>>> + u32 pwm_channel;
>>> +
>>> + ret = fwnode_property_read_u32(child, "reg", &pwm_channel);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + mcu_hwmon->thermal_cooling_dev_present[pwm_channel] = true;
>>> +
>>> + num_levels = fwnode_property_read_u8_array(child, "cooling-levels", NULL, 0);
>>> + 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 = fwnode_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, NULL,
>>> + 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;
>>> +
>>> + mcu_hwmon->cdev[pwm_channel] = cdev;
>>> + }
>>> + return 0;
>>> +}
>>> +
>>> +static int iei_wt61p803_puzzle_hwmon_probe(struct platform_device *pdev)
>>> +{
>>> + struct device *dev = &pdev->dev;
>>> + struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev->parent);
>>> + struct iei_wt61p803_puzzle_hwmon *mcu_hwmon;
>>> + struct fwnode_handle *child;
>>> + struct device *hwmon_dev;
>>> + int ret;
>>> +
>>> + mcu_hwmon = devm_kzalloc(dev, sizeof(*mcu_hwmon), GFP_KERNEL);
>>> + if (!mcu_hwmon)
>>> + return -ENOMEM;
>>> +
>>> + mcu_hwmon->response_buffer = devm_kzalloc(dev, IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
>>> + if (!mcu_hwmon->response_buffer)
>>> + return -ENOMEM;
>>> +
>>> + mcu_hwmon->mcu = mcu;
>>> + platform_set_drvdata(pdev, mcu_hwmon);
>>> +
>>> + hwmon_dev = devm_hwmon_device_register_with_info(dev, "iei_wt61p803_puzzle",
>>> + mcu_hwmon,
>>> + &iei_wt61p803_puzzle_chip_info,
>>> + NULL);
>>> +
>>> + if (IS_ERR(hwmon_dev))
>>> + return PTR_ERR(hwmon_dev);
>>> +
>>> + /* Control fans via PWM lines via Linux Kernel */
>>> + if (IS_ENABLED(CONFIG_THERMAL)) {
>>> + device_for_each_child_node(dev, child) {
>>> + ret = iei_wt61p803_puzzle_enable_thermal_cooling_dev(dev, child, mcu_hwmon);
>>> + if (ret) {
>>> + dev_err(dev, "Enabling the PWM fan failed\n");
>>> + fwnode_handle_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.26.2
>>>
>
> Kind regards,
> Luka
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 3/6] drivers: hwmon: Add the iEi WT61P803 PUZZLE HWMON driver
2020-10-13 18:51 ` Guenter Roeck
@ 2020-10-14 18:25 ` Luka Kovacic
0 siblings, 0 replies; 14+ messages in thread
From: Luka Kovacic @ 2020-10-14 18:25 UTC (permalink / raw)
To: Guenter Roeck
Cc: Linux Kernel Mailing List, linux-hwmon, Linux LED Subsystem,
Lee Jones, Pavel Machek, Dan Murphy, Rob Herring, Jean Delvare,
Marek Behun, Luka Perkov, Robert Marko
Hello Guenter,
On Tue, Oct 13, 2020 at 8:51 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 10/13/20 11:09 AM, Luka Kovacic wrote:
> > Hello Guenter,
> >
> > On Sun, Oct 11, 2020 at 11:26 PM Guenter Roeck <linux@roeck-us.net> wrote:
> >>
> >> On Wed, Oct 07, 2020 at 02:48:58AM +0200, 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>
> >>> Cc: Robert Marko <robert.marko@sartura.hr>
> >>> ---
> >>> drivers/hwmon/Kconfig | 8 +
> >>> drivers/hwmon/Makefile | 1 +
> >>> drivers/hwmon/iei-wt61p803-puzzle-hwmon.c | 457 ++++++++++++++++++++++
> >>> 3 files changed, 466 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..be7b019d126c
> >>> --- /dev/null
> >>> +++ b/drivers/hwmon/iei-wt61p803-puzzle-hwmon.c
> >>> @@ -0,0 +1,457 @@
> >>> +// 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/err.h>
> >>> +#include <linux/hwmon-sysfs.h>
> >>> +#include <linux/hwmon.h>
> >>> +#include <linux/interrupt.h>
> >>> +#include <linux/irq.h>
> >>> +#include <linux/math64.h>
> >>> +#include <linux/mfd/iei-wt61p803-puzzle.h>
> >>> +#include <linux/mod_devicetable.h>
> >>> +#include <linux/module.h>
> >>> +#include <linux/platform_device.h>
> >>> +#include <linux/property.h>
> >>> +#include <linux/slab.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
> >>> + * @response_buffer Global MCU response buffer allocation
> >>> + * @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;
> >>> + unsigned char *response_buffer;
> >>> + 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];
> >>> +};
> >>> +
> >>> +#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)
> >>> +{
> >>> + unsigned char *resp_buf = mcu_hwmon->response_buffer;
> >>> + unsigned char temp_sensor_ntc_cmd[4] = {
> >>
> >> static ?
> > These can be changed to static to avoid reallocations, every time the
> > function is called.
> > Although I will always have to assign these values anyway.
> >>
> >>> + IEI_WT61P803_PUZZLE_CMD_HEADER_START,
> >>> + IEI_WT61P803_PUZZLE_CMD_TEMP,
> >>> + IEI_WT61P803_PUZZLE_CMD_TEMP_ALL
> >>> + };
> >>> + size_t reply_size = 0;
> >>> + int ret;
> >>> +
> >>> + ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, temp_sensor_ntc_cmd,
> >>> + sizeof(temp_sensor_ntc_cmd), resp_buf,
> >>> + &reply_size);
> >>> +
> >>> + if (ret)
> >>> + return ret;
> >>> +
> >>> + /* Check the number of NTC values (should be 0x32/'2') */
> >>> + if (resp_buf[3] != 0x32)
> >>> + return -EIO;
> >>> +
> >>> + *value = raw_temp_to_milidegree_celsius(resp_buf[4 + channel]);
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +#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)
> >>> +{
> >>> + unsigned char *resp_buf = mcu_hwmon->response_buffer;
> >>> + static const u8 fan_speed_cmds[] = {
> >>> + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_0,
> >>> + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_1,
> >>> + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_2,
> >>> + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_3,
> >>> + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_4
> >>> + };
> >>> + unsigned char fan_speed_cmd[4] = {
> >>> + IEI_WT61P803_PUZZLE_CMD_HEADER_START,
> >>> + IEI_WT61P803_PUZZLE_CMD_FAN,
> >>> + IEI_WT61P803_PUZZLE_CMD_FAN_RPM_0
> >>
> >> This is unconditionally overwritten below.
> > I also add it to the array, to make it easier to read if someone is
> > debugging the code.
>
> That doesn't make it easier. It causes everyone (including me) to wonder
> if the overwrite index is wrong, and having to look into details
> to notice that the overwritten value is similar. And that has to be
> repeated for each of the functions over and over again, because
> after all one can not be sure if the submitter maybe got
> some index wrong.
>
> That kind of code in your driver caused me to get stuck on that level
> of review, and I didn't even get to the point where I actually reviewed
> the _real_ code. That kind of code is extremely difficult to review,
> it takes a lot of time to do it, and it can easily result in missing
> the real problems.
>
> Really, code such as
> unsigned char fan_speed_cmd[4] = {};
> ...
>
> fan_speed_cmd[0] = IEI_WT61P803_PUZZLE_CMD_HEADER_START;
> fan_speed_cmd[1] = IEI_WT61P803_PUZZLE_CMD_FAN;
> fan_speed_cmd[2] = fan_speed_cmds[channel];
>
> may not look as fancy, but it would be much easier to understand,
> and it would be much easier to see that it does what it is supposed
> to do.
>
> FWIW, the same is true for the actual commands. It is obviously
> your call, but something like
>
> #define IEI_WT61P803_PUZZLE_CMD_FAN_RPM_BASE 'A'
> #define IEI_WT61P803_PUZZLE_CMD_FAN_RPM(x) (IEI_WT61P803_PUZZLE_CMD_FAN_RPM_BASE + (x))
>
> fan_speed_cmd[2] = IEI_WT61P803_PUZZLE_CMD_FAN_RPM(channel)
>
> would be much easier to understand and be more straightforward
> than all those arrays.
>
> Guenter
>
Understood. That does look easier to understand.
I'll convert the command arrays to reflect this, where the content of the array
is modified before it's sent.
I will also change the commands, where this applies.
Kind regards,
Luka
> >>
> >>> + };
> >>> + size_t reply_size = 0;
> >>> + int ret;
> >>> +
> >>> + fan_speed_cmd[2] = fan_speed_cmds[channel];
> >>> +
> >>> + ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, fan_speed_cmd,
> >>> + sizeof(fan_speed_cmd), resp_buf,
> >>> + &reply_size);
> >>> + if (ret)
> >>> + return ret;
> >>> +
> >>> + *value = raw_fan_val_to_rpm(resp_buf[3], resp_buf[4]);
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static int iei_wt61p803_puzzle_write_pwm_channel
> >>> +(struct iei_wt61p803_puzzle_hwmon *mcu_hwmon, int channel, long pwm_set_val)
> >>> +{
> >>> + unsigned char *resp_buf = mcu_hwmon->response_buffer;
> >>> + static const u8 pwm_set_cmds[] = {
> >>> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_0,
> >>> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_1
> >>> + };
> >>> + unsigned char pwm_set_cmd[6] = {
> >>> + IEI_WT61P803_PUZZLE_CMD_HEADER_START,
> >>> + IEI_WT61P803_PUZZLE_CMD_FAN,
> >>> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_WRITE,
> >>> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_0,
> >>
> >> Another unconditional overwrite.
> >>
> >>> + 0x00
> >>
> >> As is this, and more below. Guess it is on purpose, but I don't
> >> see the point.
> > This one can be removed as it's calculated by the
> > iei_wt61p803_puzzle_write_command
> > function.
> >>
> >>> + };
> >>> + size_t reply_size = 0;
> >>> + int ret;
> >>> +
> >>> + pwm_set_cmd[3] = pwm_set_cmds[channel];
> >>> +
> >>> + /* Add the PWM value to the command */
> >>> + pwm_set_cmd[4] = (unsigned char)pwm_set_val;
> >>> +
> >>> + ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, pwm_set_cmd,
> >>> + sizeof(pwm_set_cmd), resp_buf,
> >>> + &reply_size);
> >>> + if (ret)
> >>> + return ret;
> >>> +
> >>> + /* Store the PWM value */
> >>> + if (!(resp_buf[0] == IEI_WT61P803_PUZZLE_CMD_HEADER_START &&
> >>> + resp_buf[1] == IEI_WT61P803_PUZZLE_CMD_RESPONSE_OK &&
> >>> + resp_buf[2] == IEI_WT61P803_PUZZLE_CHECKSUM_RESPONSE_OK))
> >>> + return -EIO;
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static int iei_wt61p803_puzzle_read_pwm_channel
> >>> +(struct iei_wt61p803_puzzle_hwmon *mcu_hwmon, int channel, int *value)
> >>
> >> I am sure I mentioned before that I don't like those odd follow-up line
> >> indentations.
> > I'll fix these, sorry.
> >>
> >>> +{
> >>> + unsigned char *resp_buf = mcu_hwmon->response_buffer;
> >>> + static const u8 pwm_get_cmds[] = {
> >>> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_0,
> >>> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_1
> >>> + };
> >>> + unsigned char pwm_get_cmd[5] = {
> >>> + IEI_WT61P803_PUZZLE_CMD_HEADER_START,
> >>> + IEI_WT61P803_PUZZLE_CMD_FAN,
> >>> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_READ,
> >>> + IEI_WT61P803_PUZZLE_CMD_FAN_PWM_0
> >>> + };
> >>> + size_t reply_size = 0;
> >>> + int ret;
> >>> +
> >>> + pwm_get_cmd[3] = pwm_get_cmds[channel];
> >>> +
> >>> + ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, pwm_get_cmd,
> >>> + sizeof(pwm_get_cmd), resp_buf,
> >>> + &reply_size);
> >>> + if (ret)
> >>> + return ret;
> >>> +
> >>> + if (resp_buf[2] != IEI_WT61P803_PUZZLE_CMD_FAN_PWM_READ)
> >>> + return -EIO;
> >>> +
> >>> + *value = resp_buf[3];
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static int iei_wt61p803_puzzle_read(struct device *dev, enum hwmon_sensor_types type,
> >>> + u32 attr, int channel, long *val)
> >>> +{
> >>> + struct iei_wt61p803_puzzle_hwmon *mcu_hwmon =
> >>> + dev_get_drvdata(dev->parent);
> >>> + int ret, value;
> >>> +
> >>> + switch (type) {
> >>> + case hwmon_pwm:
> >>> + if (attr != hwmon_pwm_input)
> >>> + return -ENODEV;
> >>
> >> Those are all unnecessary tests, and -ENODEV is wrong.
> > Okay, I can remove the attr tests.
> >>
> >>> + ret = iei_wt61p803_puzzle_read_pwm_channel(mcu_hwmon, channel, &value);
> >>> + if (ret)
> >>> + return ret;
> >>> + *val = (long)value;
> >>
> >> Unnecessary type cast. The return value of this function is _always_
> >> type casted, which really doesn't make sense.
> >>
> >>> + return ret;
> >>> + case hwmon_fan:
> >>> + if (attr != hwmon_fan_input)
> >>> + return -ENODEV;
> >>> + ret = iei_wt61p803_puzzle_read_fan_speed(mcu_hwmon, channel, &value);
> >>> + if (ret)
> >>> + return ret;
> >>> + *val = (long)value;
> >>> + return ret;
> >>> + case hwmon_temp:
> >>> + if (attr != hwmon_temp_input)
> >>> + return -ENODEV;
> >>> + ret = iei_wt61p803_puzzle_read_temp_sensor(mcu_hwmon, channel, &value);
> >>> + if (ret)
> >>> + return ret;
> >>> + *val = (long)value;
> >>
> >> Hmm, seems like you like type casts. Ok, I'll want to see an explanation
> >> for each of those: Explain why the function argument is an int *, and
> >> explain why the it is necessary to type cast the result here.
> > I'll change the function argument to long * to avoid these.
> >>
> >>> + return ret;
> >>> + default:
> >>> + return -ENODEV;
> >>> + }
> >>> +}
> >>> +
> >>> +static int iei_wt61p803_puzzle_write(struct device *dev, enum hwmon_sensor_types type,
> >>> + u32 attr, int channel, long val)
> >>> +{
> >>> + struct iei_wt61p803_puzzle_hwmon *mcu_hwmon =
> >>> + dev_get_drvdata(dev->parent);
> >>> +
> >>> + if (attr != hwmon_pwm_input)
> >>> + return -ENODEV;
> >>> + if (mcu_hwmon->thermal_cooling_dev_present[channel]) {
> >>> + /*
> >>> + * The Thermal Framework has already claimed this specific PWM
> >>> + * channel.
> >>
> >> ... and if it did, this code won't be called. I am sure I mentioned
> >> this before. If you insist keeping it, I'll want to see an explanation
> >> here because I don't want to have to deal with follow-up patches removing
> >> the unnecessary code.
> > In the previous patch this was done here and by setting the visibility
> > (umode_t).
> > I'd rather return an error with more context to the user instead of
> > just changing
> > the permissions on the sysfs attribute.
> >>
> >>> + */
> >>> + return -EBUSY;
> >>> + }
> >>> + return iei_wt61p803_puzzle_write_pwm_channel(mcu_hwmon, channel, val);
> >>> +}
> >>> +
> >>> +static umode_t iei_wt61p803_puzzle_is_visible(const void *data, enum hwmon_sensor_types type,
> >>> + u32 attr, int channel)
> >>> +{
> >>> + switch (type) {
> >>> + case hwmon_pwm:
> >>> + switch (attr) {
> >>> + case hwmon_pwm_input:
> >>> + return 0644;
> >>> + default:
> >>> + return 0;
> >>> + }
> >>> + case hwmon_fan:
> >>> + switch (attr) {
> >>> + case hwmon_fan_input:
> >>> + return 0444;
> >>> + default:
> >>> + return 0;
> >>> + }
> >>> + case hwmon_temp:
> >>> + switch (attr) {
> >>> + case hwmon_temp_input:
> >>> + return 0444;
> >>> + default:
> >>> + return 0;
> >>> + }
> >>> + default:
> >>> + return 0;
> >>> + }
> >>> +}
> >>> +
> >>> +static const struct hwmon_ops iei_wt61p803_puzzle_hwmon_ops = {
> >>> + .is_visible = iei_wt61p803_puzzle_is_visible,
> >>> + .read = iei_wt61p803_puzzle_read,
> >>> + .write = iei_wt61p803_puzzle_write,
> >>> +};
> >>> +
> >>> +static const struct hwmon_channel_info *iei_wt61p803_puzzle_info[] = {
> >>> + HWMON_CHANNEL_INFO(pwm,
> >>> + HWMON_PWM_INPUT,
> >>> + HWMON_PWM_INPUT),
> >>> + HWMON_CHANNEL_INFO(fan,
> >>> + HWMON_F_INPUT,
> >>> + HWMON_F_INPUT,
> >>> + HWMON_F_INPUT,
> >>> + HWMON_F_INPUT,
> >>> + HWMON_F_INPUT),
> >>> + HWMON_CHANNEL_INFO(temp,
> >>> + HWMON_T_INPUT,
> >>> + HWMON_T_INPUT),
> >>> + NULL
> >>> +};
> >>> +
> >>> +static const struct hwmon_chip_info iei_wt61p803_puzzle_chip_info = {
> >>> + .ops = &iei_wt61p803_puzzle_hwmon_ops,
> >>> + .info = iei_wt61p803_puzzle_info,
> >>> +};
> >>> +
> >>> +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;
> >>
> >> This check needs to be explained.
> > This is not needed and will be removed.
> >>
> >>> +
> >>> + 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,
> >>> +};
> >>> +
> >>> +static int iei_wt61p803_puzzle_enable_thermal_cooling_dev
> >>> +(struct device *dev, struct fwnode_handle *child, struct iei_wt61p803_puzzle_hwmon *mcu_hwmon)
> >>> +{
> >>> + struct iei_wt61p803_puzzle_thermal_cooling_device *cdev;
> >>> + int ret, num_levels;
> >>> + u32 pwm_channel;
> >>> +
> >>> + ret = fwnode_property_read_u32(child, "reg", &pwm_channel);
> >>> + if (ret)
> >>> + return ret;
> >>> +
> >>> + mcu_hwmon->thermal_cooling_dev_present[pwm_channel] = true;
> >>> +
> >>> + num_levels = fwnode_property_read_u8_array(child, "cooling-levels", NULL, 0);
> >>> + 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 = fwnode_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, NULL,
> >>> + 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;
> >>> +
> >>> + mcu_hwmon->cdev[pwm_channel] = cdev;
> >>> + }
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static int iei_wt61p803_puzzle_hwmon_probe(struct platform_device *pdev)
> >>> +{
> >>> + struct device *dev = &pdev->dev;
> >>> + struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev->parent);
> >>> + struct iei_wt61p803_puzzle_hwmon *mcu_hwmon;
> >>> + struct fwnode_handle *child;
> >>> + struct device *hwmon_dev;
> >>> + int ret;
> >>> +
> >>> + mcu_hwmon = devm_kzalloc(dev, sizeof(*mcu_hwmon), GFP_KERNEL);
> >>> + if (!mcu_hwmon)
> >>> + return -ENOMEM;
> >>> +
> >>> + mcu_hwmon->response_buffer = devm_kzalloc(dev, IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
> >>> + if (!mcu_hwmon->response_buffer)
> >>> + return -ENOMEM;
> >>> +
> >>> + mcu_hwmon->mcu = mcu;
> >>> + platform_set_drvdata(pdev, mcu_hwmon);
> >>> +
> >>> + hwmon_dev = devm_hwmon_device_register_with_info(dev, "iei_wt61p803_puzzle",
> >>> + mcu_hwmon,
> >>> + &iei_wt61p803_puzzle_chip_info,
> >>> + NULL);
> >>> +
> >>> + if (IS_ERR(hwmon_dev))
> >>> + return PTR_ERR(hwmon_dev);
> >>> +
> >>> + /* Control fans via PWM lines via Linux Kernel */
> >>> + if (IS_ENABLED(CONFIG_THERMAL)) {
> >>> + device_for_each_child_node(dev, child) {
> >>> + ret = iei_wt61p803_puzzle_enable_thermal_cooling_dev(dev, child, mcu_hwmon);
> >>> + if (ret) {
> >>> + dev_err(dev, "Enabling the PWM fan failed\n");
> >>> + fwnode_handle_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.26.2
> >>>
> >
> > Kind regards,
> > Luka
> >
>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-10-14 18:25 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-07 0:48 [PATCH v4 0/6] Add support for the iEi WT61P803 PUZZLE MCU Luka Kovacic
2020-10-07 0:48 ` [PATCH v4 1/6] dt-bindings: Add iEi vendor prefix and iEi WT61P803 PUZZLE driver bindings Luka Kovacic
2020-10-07 0:48 ` [PATCH v4 2/6] drivers: mfd: Add a driver for iEi WT61P803 PUZZLE MCU Luka Kovacic
2020-10-07 0:48 ` [PATCH v4 3/6] drivers: hwmon: Add the iEi WT61P803 PUZZLE HWMON driver Luka Kovacic
2020-10-11 21:26 ` Guenter Roeck
2020-10-13 18:09 ` Luka Kovacic
2020-10-13 18:51 ` Guenter Roeck
2020-10-14 18:25 ` Luka Kovacic
2020-10-07 0:48 ` [PATCH v4 4/6] drivers: leds: Add the iEi WT61P803 PUZZLE LED driver Luka Kovacic
2020-10-07 11:27 ` Pavel Machek
2020-10-09 23:25 ` Luka Kovacic
2020-10-10 0:06 ` [PATCH v5 4/7] " Luka Kovacic
2020-10-07 0:49 ` [PATCH v4 5/6] Documentation/ABI: Add iei-wt61p803-puzzle driver sysfs interface documentation Luka Kovacic
2020-10-07 0:49 ` [PATCH v4 6/6] MAINTAINERS: Add an entry for the iEi WT61P803 PUZZLE driver Luka Kovacic
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).