* [PATCH v9 0/7] Add support for the IEI WT61P803 PUZZLE MCU
@ 2021-08-24 12:44 Luka Kovacic
2021-08-24 12:44 ` [PATCH v9 1/7] dt-bindings: Add IEI vendor prefix and IEI WT61P803 PUZZLE driver bindings Luka Kovacic
` (6 more replies)
0 siblings, 7 replies; 16+ messages in thread
From: Luka Kovacic @ 2021-08-24 12:44 UTC (permalink / raw)
To: linux-doc, linux-leds, devicetree, linux-hwmon, linux-kernel,
geert+renesas, Max.Merchel, linux, daniel, shawnguo, sam, arnd,
krzysztof.kozlowski, pavo.banicevic, corbet, lee.jones, pavel,
robh+dt, linux, jdelvare, goran.medic, luka.perkov, robert.marko
Cc: 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
Changes for v5:
- Remove the return before goto to also fwnode_handle_put(child)
when ret is 0 (LED driver)
- Change unsigned char arrays to static where applicable
- Fix unconventional line indentations
- Remove unnecessary checks in the HWMON driver
- Remove unnecessary type casts
- Clear up command array assignments, where the command array is
modified before it is sent
- Resolve a checksum calculation issue
- Add Luka Perkov to MAINTAINERS
Changes for v6:
- Use the container_of() macro to get the led_cdev parent struct
- Use %u instead of %lu in a printf() (LED driver)
Changes for v7:
- Use the correct vendor title (IEI instead of iEi)
- Add missing properties to dt-bindings and fix styling issues
- Styling changes in the IEI WT61P803 PUZZLE HWMON driver
- Add missing commas in array definitions
- Check reply_size, where possible
- Clean up kernel-doc comments
Changes for v8:
- Fix multiple style issues
- Reduce number of special allocations
- Remove unnecessary special state tracking with write/read commands
- Remove most of commands calls from probing of driver
- Add hwmon documentation file
- Replace some sprintf() calls with safer scnprintf()
- Remove unused defines
- Remove all explicit castings
- Shorter name of device for hwmon driver so snprintf() can
store device number
- Remove special kobj usage
- Use dev groups in place of deprecated device attributes
- Extend documentation for some descriptions of ABI documentation
- Add missing locking for response buffer in hwmon driver
- Fix inconsistency with license versions
- Check return value of iei_wt61p803_puzzle_sysfs_create()
- Remove unneeded initializations of reply_size variable in drivers
- Use define for fixed number of MAC addresses
- Reorder some variable declarations to follow preferred style
- Spell whole WoL acronym
- Drop __func__ from device error message
- Use sizeof() where it is possible
- Use hex_asc[] array for converting number to ascii value
- Remove unnecessary hwmon-sysfs.h include
- Stop being overly verbose when probing mfd driver
Changes for v9:
- Add hwmon documentation to index.rst
- Describe the temperature sensors in hwmon documentation
- Only describe 1 LED in the dt-bindings
- Use absoulute references in dt-bindings, instead of relative ones
Luka Kovacic (7):
dt-bindings: Add IEI vendor prefix and IEI WT61P803 PUZZLE driver
bindings
drivers: mfd: Add a driver for IEI WT61P803 PUZZLE MCU
drivers: hwmon: Add the IEI WT61P803 PUZZLE HWMON driver
drivers: leds: Add the IEI WT61P803 PUZZLE LED driver
Documentation/ABI: Add iei-wt61p803-puzzle driver sysfs interface
documentation
Documentation/hwmon: Add iei-wt61p803-puzzle hwmon driver
documentation
MAINTAINERS: Add an entry for the IEI WT61P803 PUZZLE driver
.../testing/sysfs-driver-iei-wt61p803-puzzle | 61 ++
.../hwmon/iei,wt61p803-puzzle-hwmon.yaml | 53 +
.../leds/iei,wt61p803-puzzle-leds.yaml | 39 +
.../bindings/mfd/iei,wt61p803-puzzle.yaml | 82 ++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
.../hwmon/iei-wt61p803-puzzle-hwmon.rst | 43 +
Documentation/hwmon/index.rst | 1 +
MAINTAINERS | 16 +
drivers/hwmon/Kconfig | 8 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/iei-wt61p803-puzzle-hwmon.c | 413 ++++++++
drivers/leds/Kconfig | 8 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-iei-wt61p803-puzzle.c | 147 +++
drivers/mfd/Kconfig | 8 +
drivers/mfd/Makefile | 1 +
drivers/mfd/iei-wt61p803-puzzle.c | 908 ++++++++++++++++++
include/linux/mfd/iei-wt61p803-puzzle.h | 66 ++
18 files changed, 1858 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 Documentation/hwmon/iei-wt61p803-puzzle-hwmon.rst
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.31.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v9 1/7] dt-bindings: Add IEI vendor prefix and IEI WT61P803 PUZZLE driver bindings
2021-08-24 12:44 [PATCH v9 0/7] Add support for the IEI WT61P803 PUZZLE MCU Luka Kovacic
@ 2021-08-24 12:44 ` Luka Kovacic
2021-08-24 18:09 ` Rob Herring
2021-08-24 12:44 ` [PATCH v9 2/7] drivers: mfd: Add a driver for IEI WT61P803 PUZZLE MCU Luka Kovacic
` (5 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Luka Kovacic @ 2021-08-24 12:44 UTC (permalink / raw)
To: linux-doc, linux-leds, devicetree, linux-hwmon, linux-kernel,
geert+renesas, Max.Merchel, linux, daniel, shawnguo, sam, arnd,
krzysztof.kozlowski, pavo.banicevic, corbet, lee.jones, pavel,
robh+dt, linux, jdelvare, goran.medic, luka.perkov, robert.marko
Cc: 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>
Signed-off-by: Pavo Banicevic <pavo.banicevic@sartura.hr>
Cc: Luka Perkov <luka.perkov@sartura.hr>
Cc: Robert Marko <robert.marko@sartura.hr>
---
.../hwmon/iei,wt61p803-puzzle-hwmon.yaml | 53 ++++++++++++
.../leds/iei,wt61p803-puzzle-leds.yaml | 39 +++++++++
.../bindings/mfd/iei,wt61p803-puzzle.yaml | 82 +++++++++++++++++++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
4 files changed, 176 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..c24a24e90495
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/iei,wt61p803-puzzle-hwmon.yaml
@@ -0,0 +1,53 @@
+# 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
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+patternProperties:
+ "^fan-group@[0-1]$":
+ type: object
+ properties:
+ reg:
+ minimum: 0
+ maximum: 1
+ description:
+ Fan group ID
+
+ cooling-levels:
+ minItems: 1
+ maxItems: 255
+ description:
+ Cooling levels for the fans (PWM value mapping)
+ description: |
+ Properties for each fan group.
+ required:
+ - reg
+
+required:
+ - compatible
+ - "#address-cells"
+ - "#size-cells"
+
+additionalProperties: false
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..af219ddc190f
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/iei,wt61p803-puzzle-leds.yaml
@@ -0,0 +1,39 @@
+# 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
+
+ led@0:
+ type: object
+ $ref: common.yaml
+ description: |
+ Properties for a single LED.
+
+required:
+ - compatible
+ - "#address-cells"
+ - "#size-cells"
+
+additionalProperties: false
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..8ce18c29177b
--- /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
+
+ hwmon:
+ $ref: /schemas/hwmon/iei,wt61p803-puzzle-hwmon.yaml
+
+ leds:
+ $ref: /schemas/leds/iei,wt61p803-puzzle-leds.yaml
+
+required:
+ - compatible
+ - current-speed
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/leds/common.h>
+ serial {
+ 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>;
+ };
+ };
+
+ 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 07fb0d25fc15..2d800454542e 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -515,6 +515,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.31.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v9 2/7] drivers: mfd: Add a driver for IEI WT61P803 PUZZLE MCU
2021-08-24 12:44 [PATCH v9 0/7] Add support for the IEI WT61P803 PUZZLE MCU Luka Kovacic
2021-08-24 12:44 ` [PATCH v9 1/7] dt-bindings: Add IEI vendor prefix and IEI WT61P803 PUZZLE driver bindings Luka Kovacic
@ 2021-08-24 12:44 ` Luka Kovacic
2021-09-22 10:54 ` Lee Jones
2021-09-22 11:41 ` Greg KH
2021-08-24 12:44 ` [PATCH v9 3/7] drivers: hwmon: Add the IEI WT61P803 PUZZLE HWMON driver Luka Kovacic
` (4 subsequent siblings)
6 siblings, 2 replies; 16+ messages in thread
From: Luka Kovacic @ 2021-08-24 12:44 UTC (permalink / raw)
To: linux-doc, linux-leds, devicetree, linux-hwmon, linux-kernel,
geert+renesas, Max.Merchel, linux, daniel, shawnguo, sam, arnd,
krzysztof.kozlowski, pavo.banicevic, corbet, lee.jones, pavel,
robh+dt, linux, jdelvare, goran.medic, luka.perkov, robert.marko
Cc: 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>
Signed-off-by: Pavo Banicevic <pavo.banicevic@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 | 908 ++++++++++++++++++++++++
include/linux/mfd/iei-wt61p803-puzzle.h | 66 ++
4 files changed, 983 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 6a3fd2d75f96..c2183eb0775f 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2170,6 +2170,14 @@ 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.
+
config MFD_INTEL_M10_BMC
tristate "Intel MAX 10 Board Management Controller"
depends on SPI_MASTER
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 8116c19d5fd4..42b9767ec37a 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -236,6 +236,7 @@ obj-$(CONFIG_MFD_DLN2) += dln2.o
obj-$(CONFIG_MFD_RT4831) += rt4831.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..596ecbc65627
--- /dev/null
+++ b/drivers/mfd/iei-wt61p803-puzzle.c
@@ -0,0 +1,908 @@
+// 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 <linux/atomic.h>
+#include <linux/delay.h>
+#include <linux/export.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/iei-wt61p803-puzzle.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/property.h>
+#include <linux/sched.h>
+#include <linux/serdev.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+#include <asm/unaligned.h>
+
+/* start, payload and XOR checksum at end */
+#define IEI_WT61P803_PUZZLE_MAX_COMMAND_LENGTH (1 + 20 + 1)
+#define IEI_WT61P803_PUZZLE_RESP_BUF_SIZE 512
+
+#define IEI_WT61P803_PUZZLE_MAC_LENGTH 17
+#define IEI_WT61P803_PUZZLE_SN_LENGTH 36
+#define IEI_WT61P803_PUZZLE_VERSION_LENGTH 6
+#define IEI_WT61P803_PUZZLE_BUILD_INFO_LENGTH 16
+#define IEI_WT61P803_PUZZLE_PROTOCOL_VERSION_LENGTH 8
+#define IEI_WT61P803_PUZZLE_NB_MAC 8
+
+/* Use HZ as a timeout value throughout the driver */
+#define IEI_WT61P803_PUZZLE_GENERAL_TIMEOUT HZ
+
+enum iei_wt61p803_puzzle_attribute_type {
+ IEI_WT61P803_PUZZLE_VERSION,
+ IEI_WT61P803_PUZZLE_BUILD_INFO,
+ IEI_WT61P803_PUZZLE_BOOTLOADER_MODE,
+ IEI_WT61P803_PUZZLE_PROTOCOL_VERSION,
+ IEI_WT61P803_PUZZLE_SERIAL_NUMBER,
+ IEI_WT61P803_PUZZLE_MAC_ADDRESS,
+ IEI_WT61P803_PUZZLE_AC_RECOVERY_STATUS,
+ IEI_WT61P803_PUZZLE_POWER_LOSS_RECOVERY,
+ IEI_WT61P803_PUZZLE_POWER_STATUS,
+};
+
+struct iei_wt61p803_puzzle_device_attribute {
+ struct device_attribute dev_attr;
+ enum iei_wt61p803_puzzle_attribute_type type;
+ u8 index;
+};
+
+/**
+ * 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;
+};
+
+/**
+ * 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[IEI_WT61P803_PUZZLE_RESP_BUF_SIZE];
+ 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
+ *
+ * Last element of arrays is reserved for '\0'.
+ */
+struct iei_wt61p803_puzzle_mcu_version {
+ char version[IEI_WT61P803_PUZZLE_VERSION_LENGTH + 1];
+ char build_info[IEI_WT61P803_PUZZLE_BUILD_INFO_LENGTH + 1];
+ bool bootloader_mode;
+ char protocol_version[IEI_WT61P803_PUZZLE_PROTOCOL_VERSION_LENGTH + 1];
+ char serial_number[IEI_WT61P803_PUZZLE_SN_LENGTH + 1];
+ char mac_address[IEI_WT61P803_PUZZLE_NB_MAC][IEI_WT61P803_PUZZLE_MAC_LENGTH + 1];
+};
+
+/**
+ * struct iei_wt61p803_puzzle - IEI WT61P803 PUZZLE MCU Driver
+ * @serdev: Pointer to underlying serdev device
+ * @dev: Pointer to underlying dev device
+ * @reply_lock: Reply 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 device *dev;
+ struct mutex reply_lock; /* lock to prevent multiple firmware calls */
+ struct iei_wt61p803_puzzle_reply *reply;
+ struct iei_wt61p803_puzzle_mcu_version version;
+ struct iei_wt61p803_puzzle_mcu_status status;
+ unsigned char response_buffer[IEI_WT61P803_PUZZLE_BUF_SIZE];
+ struct mutex lock; /* lock to protect response buffer */
+};
+
+static unsigned char iei_wt61p803_puzzle_checksum(unsigned char *buf, size_t len)
+{
+ unsigned char checksum = 0;
+ size_t i;
+
+ for (i = 0; i < len; i++)
+ checksum ^= buf[i];
+ return checksum;
+}
+
+static int iei_wt61p803_puzzle_process_resp(struct iei_wt61p803_puzzle *mcu,
+ const unsigned char *raw_resp_data, size_t size)
+{
+ unsigned char checksum;
+
+ /* 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))) {
+ if (mcu->reply->size + size >= sizeof(mcu->reply->data))
+ return -EIO;
+
+ /* Append the frame to existing data */
+ memcpy(mcu->reply->data + mcu->reply->size, raw_resp_data, size);
+ mcu->reply->size += size;
+ } else {
+ if (size >= sizeof(mcu->reply->data))
+ return -EIO;
+
+ /* Start processing a new frame */
+ memcpy(mcu->reply->data, raw_resp_data, size);
+ mcu->reply->size = size;
+ }
+
+ 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 */
+ return size;
+ }
+
+ /* Received all the data */
+ complete(&mcu->reply->received);
+
+ return 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, data, size);
+ /* Return the number of processed bytes if function returns error,
+ * discard the remaining incoming data, since the frame this data
+ * belongs to is broken anyway
+ */
+ if (ret < 0)
+ return 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, "Command response timed out. Retries: %d\n", 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;
+
+ if (size <= 1 || size > IEI_WT61P803_PUZZLE_MAX_COMMAND_LENGTH)
+ return -EINVAL;
+
+ mutex_lock(&mcu->reply_lock);
+
+ cmd[size - 1] = iei_wt61p803_puzzle_checksum(cmd, size - 1);
+
+ /* Initialize reply struct */
+ reinit_completion(&mcu->reply->received);
+ mcu->reply->size = 0;
+ usleep_range(2000, 10000);
+ serdev_device_write_flush(mcu->serdev);
+ ret = serdev_device_write_buf(mcu->serdev, cmd, size);
+ if (ret < 0)
+ goto exit;
+
+ serdev_device_wait_until_sent(mcu->serdev, IEI_WT61P803_PUZZLE_GENERAL_TIMEOUT);
+ ret = wait_for_completion_timeout(&mcu->reply->received,
+ IEI_WT61P803_PUZZLE_GENERAL_TIMEOUT);
+ if (ret == 0) {
+ dev_err(dev, "Command reply receive timeout\n");
+ ret = -ETIMEDOUT;
+ 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);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iei_wt61p803_puzzle_write_command);
+
+static int iei_wt61p803_puzzle_buzzer(struct iei_wt61p803_puzzle *mcu, bool long_beep)
+{
+ unsigned char *resp_buf = mcu->response_buffer;
+ unsigned char buzzer_cmd[4] = {};
+ size_t reply_size;
+ int ret;
+
+ buzzer_cmd[0] = IEI_WT61P803_PUZZLE_CMD_HEADER_START;
+ buzzer_cmd[1] = IEI_WT61P803_PUZZLE_CMD_FUNCTION_SINGLE;
+ buzzer_cmd[2] = long_beep ? '3' : '2'; /* Buzzer 1.5 / 0.5 second beep */
+
+ mutex_lock(&mcu->lock);
+ ret = iei_wt61p803_puzzle_write_command(mcu, buzzer_cmd, sizeof(buzzer_cmd),
+ 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;
+ 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.%.3s", rb[2], &rb[3]);
+
+ 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/%.4s %c%c:%c%c",
+ rb[8], rb[9], rb[6], rb[7], &rb[2], 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;
+ 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, /* EEPROM read address */
+ 0x24, /* Data length */
+ };
+ size_t reply_size;
+ 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;
+
+ if (reply_size < IEI_WT61P803_PUZZLE_SN_LENGTH + 4) {
+ ret = -EIO;
+ goto err;
+ }
+
+ sprintf(mcu->version.serial_number, "%.*s",
+ IEI_WT61P803_PUZZLE_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, /* EEPROM write address */
+ 0xC, /* Data length */
+ };
+ unsigned char serial_number_cmd[4 + 12 + 1]; /* header, serial number, XOR checksum */
+ int ret, sn_counter;
+ size_t reply_size;
+
+ /* The MCU can only handle 22 byte messages, send the S/N in 12 byte 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, sizeof(serial_number_header));
+ memcpy(serial_number_cmd + sizeof(serial_number_header),
+ serial_number + 0xC * sn_counter, 0xC);
+
+ 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_SN_LENGTH, serial_number);
+err:
+ mutex_unlock(&mcu->lock);
+ return ret;
+}
+
+static int iei_wt61p803_puzzle_get_mac_address(struct iei_wt61p803_puzzle *mcu, int index)
+{
+ 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, /* EEPROM read address */
+ 0x11, /* Data length */
+ };
+ size_t reply_size;
+ int ret;
+
+ mutex_lock(&mcu->lock);
+ mac_address_cmd[2] = 0x24 + 0x11 * index;
+
+ 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 < 22) {
+ ret = -EIO;
+ goto err;
+ }
+
+ sprintf(mcu->version.mac_address[index], "%.*s",
+ IEI_WT61P803_PUZZLE_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[IEI_WT61P803_PUZZLE_MAC_LENGTH],
+ int mac_address_idx)
+{
+ unsigned char mac_address_cmd[4 + IEI_WT61P803_PUZZLE_MAC_LENGTH + 1];
+ 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, /* EEPROM write address */
+ 0x11, /* Data length */
+ };
+ size_t reply_size;
+ int ret;
+
+ if (mac_address_idx < 0 || mac_address_idx >= IEI_WT61P803_PUZZLE_NB_MAC)
+ 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, sizeof(mac_address_header));
+ memcpy(mac_address_cmd + sizeof(mac_address_header), mac_address,
+ IEI_WT61P803_PUZZLE_MAC_LENGTH);
+
+ 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_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 *resp_buf = mcu->response_buffer;
+ unsigned char power_loss_recovery_cmd[5] = {};
+ size_t reply_size;
+ int ret;
+
+ if (power_loss_recovery_action < 0 || power_loss_recovery_action > 4)
+ return -EINVAL;
+
+ power_loss_recovery_cmd[0] = IEI_WT61P803_PUZZLE_CMD_HEADER_START;
+ power_loss_recovery_cmd[1] = IEI_WT61P803_PUZZLE_CMD_FUNCTION_OTHER;
+ power_loss_recovery_cmd[2] = IEI_WT61P803_PUZZLE_CMD_FUNCTION_OTHER_POWER_LOSS;
+ power_loss_recovery_cmd[3] = hex_asc[power_loss_recovery_action];
+
+ 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 to_puzzle_dev_attr(_attr) \
+ container_of(_attr, struct iei_wt61p803_puzzle_device_attribute, dev_attr)
+
+static ssize_t show_output(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev);
+ struct iei_wt61p803_puzzle_device_attribute *pattr = to_puzzle_dev_attr(attr);
+ int ret;
+
+ switch (pattr->type) {
+ case IEI_WT61P803_PUZZLE_VERSION:
+ return scnprintf(buf, PAGE_SIZE, "%s\n", mcu->version.version);
+ case IEI_WT61P803_PUZZLE_BUILD_INFO:
+ return scnprintf(buf, PAGE_SIZE, "%s\n", mcu->version.build_info);
+ case IEI_WT61P803_PUZZLE_BOOTLOADER_MODE:
+ return scnprintf(buf, PAGE_SIZE, "%d\n", mcu->version.bootloader_mode);
+ case IEI_WT61P803_PUZZLE_PROTOCOL_VERSION:
+ return scnprintf(buf, PAGE_SIZE, "%s\n", mcu->version.protocol_version);
+ case IEI_WT61P803_PUZZLE_SERIAL_NUMBER:
+ ret = iei_wt61p803_puzzle_get_serial_number(mcu);
+ if (!ret)
+ ret = scnprintf(buf, PAGE_SIZE, "%s\n", mcu->version.serial_number);
+ else
+ ret = 0;
+ return ret;
+ case IEI_WT61P803_PUZZLE_MAC_ADDRESS:
+ ret = iei_wt61p803_puzzle_get_mac_address(mcu, pattr->index);
+ if (!ret)
+ ret = scnprintf(buf, PAGE_SIZE, "%s\n",
+ mcu->version.mac_address[pattr->index]);
+ else
+ ret = 0;
+ return ret;
+ case IEI_WT61P803_PUZZLE_AC_RECOVERY_STATUS:
+ case IEI_WT61P803_PUZZLE_POWER_LOSS_RECOVERY:
+ case IEI_WT61P803_PUZZLE_POWER_STATUS:
+ ret = iei_wt61p803_puzzle_get_mcu_status(mcu);
+ if (ret)
+ return ret;
+
+ mutex_lock(&mcu->lock);
+ switch (pattr->type) {
+ case IEI_WT61P803_PUZZLE_AC_RECOVERY_STATUS:
+ ret = scnprintf(buf, PAGE_SIZE, "%x\n",
+ mcu->status.ac_recovery_status_flag);
+ break;
+ case IEI_WT61P803_PUZZLE_POWER_LOSS_RECOVERY:
+ ret = scnprintf(buf, PAGE_SIZE, "%x\n", mcu->status.power_loss_recovery);
+ break;
+ case IEI_WT61P803_PUZZLE_POWER_STATUS:
+ ret = scnprintf(buf, PAGE_SIZE, "%x\n", mcu->status.power_status);
+ break;
+ default:
+ ret = 0;
+ break;
+ }
+ mutex_unlock(&mcu->lock);
+ return ret;
+ default:
+ return 0;
+ }
+
+ return 0;
+}
+
+static ssize_t store_output(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ unsigned char serial_number[IEI_WT61P803_PUZZLE_SN_LENGTH];
+ unsigned char mac_address[IEI_WT61P803_PUZZLE_MAC_LENGTH];
+ struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev);
+ struct iei_wt61p803_puzzle_device_attribute *pattr = to_puzzle_dev_attr(attr);
+ int power_loss_recovery_action = 0;
+ int ret;
+
+ switch (pattr->type) {
+ case IEI_WT61P803_PUZZLE_SERIAL_NUMBER:
+ if (len != (size_t)(IEI_WT61P803_PUZZLE_SN_LENGTH + 1))
+ return -EINVAL;
+ memcpy(serial_number, buf, sizeof(serial_number));
+ ret = iei_wt61p803_puzzle_write_serial_number(mcu, serial_number);
+ if (ret)
+ return ret;
+ return len;
+ case IEI_WT61P803_PUZZLE_MAC_ADDRESS:
+ if (len != (size_t)(IEI_WT61P803_PUZZLE_MAC_LENGTH + 1))
+ return -EINVAL;
+
+ memcpy(mac_address, buf, sizeof(mac_address));
+
+ if (strlen(attr->attr.name) != 13)
+ return -EIO;
+
+ ret = iei_wt61p803_puzzle_write_mac_address(mcu, mac_address, pattr->index);
+ if (ret)
+ return ret;
+ return len;
+ case IEI_WT61P803_PUZZLE_POWER_LOSS_RECOVERY:
+ ret = kstrtoint(buf, 10, &power_loss_recovery_action);
+ if (ret)
+ return ret;
+ ret = iei_wt61p803_puzzle_write_power_loss_recovery(mcu,
+ power_loss_recovery_action);
+ if (ret)
+ return ret;
+ return len;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+#define IEI_WT61P803_PUZZLE_ATTR(_name, _mode, _show, _store, _type, _index) \
+ struct iei_wt61p803_puzzle_device_attribute dev_attr_##_name = \
+ { .dev_attr = __ATTR(_name, _mode, _show, _store), \
+ .type = _type, \
+ .index = _index }
+
+#define IEI_WT61P803_PUZZLE_ATTR_RO(_name, _type, _id) \
+ IEI_WT61P803_PUZZLE_ATTR(_name, 0444, show_output, NULL, _type, _id)
+
+#define IEI_WT61P803_PUZZLE_ATTR_RW(_name, _type, _id) \
+ IEI_WT61P803_PUZZLE_ATTR(_name, 0644, show_output, store_output, _type, _id)
+
+static IEI_WT61P803_PUZZLE_ATTR_RO(version, IEI_WT61P803_PUZZLE_VERSION, 0);
+static IEI_WT61P803_PUZZLE_ATTR_RO(build_info, IEI_WT61P803_PUZZLE_BUILD_INFO, 0);
+static IEI_WT61P803_PUZZLE_ATTR_RO(bootloader_mode, IEI_WT61P803_PUZZLE_BOOTLOADER_MODE, 0);
+static IEI_WT61P803_PUZZLE_ATTR_RO(protocol_version, IEI_WT61P803_PUZZLE_PROTOCOL_VERSION, 0);
+static IEI_WT61P803_PUZZLE_ATTR_RW(serial_number, IEI_WT61P803_PUZZLE_SERIAL_NUMBER, 0);
+static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_0, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 0);
+static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_1, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 1);
+static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_2, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 2);
+static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_3, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 3);
+static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_4, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 4);
+static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_5, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 5);
+static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_6, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 6);
+static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_7, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 7);
+static IEI_WT61P803_PUZZLE_ATTR_RO(ac_recovery_status, IEI_WT61P803_PUZZLE_AC_RECOVERY_STATUS, 0);
+static IEI_WT61P803_PUZZLE_ATTR_RW(power_loss_recovery, IEI_WT61P803_PUZZLE_POWER_LOSS_RECOVERY, 0);
+static IEI_WT61P803_PUZZLE_ATTR_RO(power_status, IEI_WT61P803_PUZZLE_POWER_STATUS, 0);
+
+static struct attribute *iei_wt61p803_puzzle_attrs[] = {
+ &dev_attr_version.dev_attr.attr,
+ &dev_attr_build_info.dev_attr.attr,
+ &dev_attr_bootloader_mode.dev_attr.attr,
+ &dev_attr_protocol_version.dev_attr.attr,
+ &dev_attr_serial_number.dev_attr.attr,
+ &dev_attr_mac_address_0.dev_attr.attr,
+ &dev_attr_mac_address_1.dev_attr.attr,
+ &dev_attr_mac_address_2.dev_attr.attr,
+ &dev_attr_mac_address_3.dev_attr.attr,
+ &dev_attr_mac_address_4.dev_attr.attr,
+ &dev_attr_mac_address_5.dev_attr.attr,
+ &dev_attr_mac_address_6.dev_attr.attr,
+ &dev_attr_mac_address_7.dev_attr.attr,
+ &dev_attr_ac_recovery_status.dev_attr.attr,
+ &dev_attr_power_loss_recovery.dev_attr.attr,
+ &dev_attr_power_status.dev_attr.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(iei_wt61p803_puzzle);
+
+static int iei_wt61p803_puzzle_sysfs_create(struct device *dev,
+ struct iei_wt61p803_puzzle *mcu)
+{
+ int ret;
+
+ ret = sysfs_create_groups(&mcu->dev->kobj, iei_wt61p803_puzzle_groups);
+ if (ret)
+ mfd_remove_devices(mcu->dev);
+
+ return ret;
+}
+
+static int iei_wt61p803_puzzle_sysfs_remove(struct device *dev,
+ struct iei_wt61p803_puzzle *mcu)
+{
+ /* Remove sysfs groups */
+ sysfs_remove_groups(&mcu->dev->kobj, iei_wt61p803_puzzle_groups);
+ mfd_remove_devices(mcu->dev);
+
+ 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_dbg(dev, "Driver baud rate: %d\n", 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;
+
+ /* Initialize device struct data */
+ mcu->serdev = serdev;
+ mcu->dev = dev;
+ init_completion(&mcu->reply->received);
+ mutex_init(&mcu->reply_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\n");
+ return ret;
+ }
+
+ ret = iei_wt61p803_puzzle_get_version(mcu);
+ if (ret)
+ return ret;
+
+ dev_dbg(dev, "MCU version: %s\n", mcu->version.version);
+ dev_dbg(dev, "MCU firmware build info: %s\n", mcu->version.build_info);
+ dev_dbg(dev, "MCU in bootloader mode: %s\n",
+ mcu->version.bootloader_mode ? "true" : "false");
+ dev_dbg(dev, "MCU protocol version: %s\n", 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);
+ if (ret)
+ return ret;
+
+ return devm_of_platform_populate(dev);
+}
+
+static void iei_wt61p803_puzzle_remove(struct serdev_device *serdev)
+{
+ struct device *dev = &serdev->dev;
+ struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev);
+
+ 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 v2");
+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..7f2da5887dae
--- /dev/null
+++ b/include/linux/mfd/iei-wt61p803-puzzle.h
@@ -0,0 +1,66 @@
+/* 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_READ 0x5A /* Z */
+#define IEI_WT61P803_PUZZLE_CMD_FAN_PWM_WRITE 0x57 /* W */
+#define IEI_WT61P803_PUZZLE_CMD_FAN_PWM_BASE 0x30
+#define IEI_WT61P803_PUZZLE_CMD_FAN_RPM_BASE 0x41 /* A */
+
+#define IEI_WT61P803_PUZZLE_CMD_FAN_PWM(x) (IEI_WT61P803_PUZZLE_CMD_FAN_PWM_BASE + (x)) /* 0 - 1 */
+#define IEI_WT61P803_PUZZLE_CMD_FAN_RPM(x) (IEI_WT61P803_PUZZLE_CMD_FAN_RPM_BASE + (x)) /* 0 - 5 */
+
+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.31.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v9 3/7] drivers: hwmon: Add the IEI WT61P803 PUZZLE HWMON driver
2021-08-24 12:44 [PATCH v9 0/7] Add support for the IEI WT61P803 PUZZLE MCU Luka Kovacic
2021-08-24 12:44 ` [PATCH v9 1/7] dt-bindings: Add IEI vendor prefix and IEI WT61P803 PUZZLE driver bindings Luka Kovacic
2021-08-24 12:44 ` [PATCH v9 2/7] drivers: mfd: Add a driver for IEI WT61P803 PUZZLE MCU Luka Kovacic
@ 2021-08-24 12:44 ` Luka Kovacic
2021-08-24 12:44 ` [PATCH v9 4/7] drivers: leds: Add the IEI WT61P803 PUZZLE LED driver Luka Kovacic
` (3 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Luka Kovacic @ 2021-08-24 12:44 UTC (permalink / raw)
To: linux-doc, linux-leds, devicetree, linux-hwmon, linux-kernel,
geert+renesas, Max.Merchel, linux, daniel, shawnguo, sam, arnd,
krzysztof.kozlowski, pavo.banicevic, corbet, lee.jones, pavel,
robh+dt, linux, jdelvare, goran.medic, luka.perkov, robert.marko
Cc: 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>
Signed-off-by: Pavo Banicevic <pavo.banicevic@sartura.hr>
Acked-by: Guenter Roeck <linux@roeck-us.net>
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 | 413 ++++++++++++++++++++++
3 files changed, 422 insertions(+)
create mode 100644 drivers/hwmon/iei-wt61p803-puzzle-hwmon.c
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index e3675377bc5d..53fdc8c2ae37 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 d712c61c1f5e..b31f8d7e7c96 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -84,6 +84,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..3a895ca08474
--- /dev/null
+++ b/drivers/hwmon/iei-wt61p803-puzzle-hwmon.c
@@ -0,0 +1,413 @@
+// 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.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_PWM 2
+#define IEI_WT61P803_PUZZLE_HWMON_MAX_PWM_VAL 255
+
+/**
+ * struct iei_wt61p803_puzzle_thermal_cooling_device - Thermal cooling device instance
+ * @mcu_hwmon: Parent driver struct pointer
+ * @tcdev: Thermal cooling device pointer
+ * @name: Thermal cooling device name
+ * @pwm_channel: Controlled PWM channel (0 or 1)
+ * @cooling_levels: Thermal cooling device cooling levels (DT)
+ */
+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
+ * @thermal_cooling_dev_present: Per-channel thermal cooling device control indicator
+ * @cdev: Per-channel thermal cooling device private structure
+ */
+struct iei_wt61p803_puzzle_hwmon {
+ struct iei_wt61p803_puzzle *mcu;
+ unsigned char response_buffer[IEI_WT61P803_PUZZLE_BUF_SIZE];
+ bool thermal_cooling_dev_present[IEI_WT61P803_PUZZLE_HWMON_MAX_PWM];
+ struct iei_wt61p803_puzzle_thermal_cooling_device
+ *cdev[IEI_WT61P803_PUZZLE_HWMON_MAX_PWM];
+ struct mutex lock; /* mutex to protect response_buffer array */
+};
+
+#define raw_temp_to_milidegree_celsius(x) (((x) - 0x80) * 1000)
+static int iei_wt61p803_puzzle_read_temp_sensor(struct iei_wt61p803_puzzle_hwmon *mcu_hwmon,
+ int channel, long *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;
+ int ret;
+
+ mutex_lock(&mcu_hwmon->lock);
+ ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, temp_sensor_ntc_cmd,
+ sizeof(temp_sensor_ntc_cmd), resp_buf,
+ &reply_size);
+ if (ret)
+ goto exit;
+
+ if (reply_size != 7) {
+ ret = -EIO;
+ goto exit;
+ }
+
+ /* Check the number of NTC values */
+ if (resp_buf[3] != '2') {
+ ret = -EIO;
+ goto exit;
+ }
+
+ *value = raw_temp_to_milidegree_celsius(resp_buf[4 + channel]);
+exit:
+ mutex_unlock(&mcu_hwmon->lock);
+ return ret;
+}
+
+#define raw_fan_val_to_rpm(x, y) ((((x) << 8 | (y)) / 2) * 60)
+static int iei_wt61p803_puzzle_read_fan_speed(struct iei_wt61p803_puzzle_hwmon *mcu_hwmon,
+ int channel, long *value)
+{
+ unsigned char *resp_buf = mcu_hwmon->response_buffer;
+ unsigned char fan_speed_cmd[4] = {};
+ size_t reply_size;
+ int ret;
+
+ fan_speed_cmd[0] = IEI_WT61P803_PUZZLE_CMD_HEADER_START;
+ fan_speed_cmd[1] = IEI_WT61P803_PUZZLE_CMD_FAN;
+ fan_speed_cmd[2] = IEI_WT61P803_PUZZLE_CMD_FAN_RPM(channel);
+
+ mutex_lock(&mcu_hwmon->lock);
+ ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, fan_speed_cmd,
+ sizeof(fan_speed_cmd), resp_buf,
+ &reply_size);
+ if (ret)
+ goto exit;
+
+ if (reply_size != 7) {
+ ret = -EIO;
+ goto exit;
+ }
+
+ *value = raw_fan_val_to_rpm(resp_buf[3], resp_buf[4]);
+exit:
+ mutex_unlock(&mcu_hwmon->lock);
+ return ret;
+}
+
+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;
+ unsigned char pwm_set_cmd[6] = {};
+ size_t reply_size;
+ int ret;
+
+ pwm_set_cmd[0] = IEI_WT61P803_PUZZLE_CMD_HEADER_START;
+ pwm_set_cmd[1] = IEI_WT61P803_PUZZLE_CMD_FAN;
+ pwm_set_cmd[2] = IEI_WT61P803_PUZZLE_CMD_FAN_PWM_WRITE;
+ pwm_set_cmd[3] = IEI_WT61P803_PUZZLE_CMD_FAN_PWM(channel);
+ pwm_set_cmd[4] = pwm_set_val;
+
+ mutex_lock(&mcu_hwmon->lock);
+ ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu, pwm_set_cmd,
+ sizeof(pwm_set_cmd), 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 = -EIO;
+ goto exit;
+ }
+exit:
+ mutex_unlock(&mcu_hwmon->lock);
+ return ret;
+}
+
+static int iei_wt61p803_puzzle_read_pwm_channel(struct iei_wt61p803_puzzle_hwmon *mcu_hwmon,
+ int channel, long *value)
+{
+ unsigned char *resp_buf = mcu_hwmon->response_buffer;
+ unsigned char pwm_get_cmd[5] = {};
+ size_t reply_size;
+ int ret;
+
+ pwm_get_cmd[0] = IEI_WT61P803_PUZZLE_CMD_HEADER_START;
+ pwm_get_cmd[1] = IEI_WT61P803_PUZZLE_CMD_FAN;
+ pwm_get_cmd[2] = IEI_WT61P803_PUZZLE_CMD_FAN_PWM_READ;
+ pwm_get_cmd[3] = IEI_WT61P803_PUZZLE_CMD_FAN_PWM(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 (reply_size != 5)
+ return -EIO;
+
+ 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);
+
+ switch (type) {
+ case hwmon_pwm:
+ return iei_wt61p803_puzzle_read_pwm_channel(mcu_hwmon, channel, val);
+ case hwmon_fan:
+ return iei_wt61p803_puzzle_read_fan_speed(mcu_hwmon, channel, val);
+ case hwmon_temp:
+ return iei_wt61p803_puzzle_read_temp_sensor(mcu_hwmon, channel, val);
+ default:
+ return -EINVAL;
+ }
+}
+
+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);
+
+ 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)
+{
+ const struct iei_wt61p803_puzzle_hwmon *mcu_hwmon = data;
+
+ switch (type) {
+ case hwmon_pwm:
+ if (mcu_hwmon->thermal_cooling_dev_present[channel])
+ return 0444;
+ if (attr == hwmon_pwm_input)
+ return 0644;
+ break;
+ case hwmon_fan:
+ if (attr == hwmon_fan_input)
+ return 0444;
+ break;
+ case hwmon_temp:
+ if (attr == hwmon_temp_input)
+ return 0444;
+ break;
+ default:
+ return 0;
+ }
+
+ 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;
+ long value;
+ int ret;
+
+ ret = iei_wt61p803_puzzle_read_pwm_channel(mcu_hwmon, cdev->pwm_channel, &value);
+ if (ret)
+ return ret;
+ *state = 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;
+
+ 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;
+ u32 pwm_channel;
+ u8 num_levels;
+ int ret;
+
+ 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_count_u8(child, "cooling-levels");
+ if (!num_levels)
+ return -EINVAL;
+
+ cdev = devm_kzalloc(dev, sizeof(*cdev), GFP_KERNEL);
+ if (!cdev)
+ return -ENOMEM;
+
+ cdev->cooling_levels = devm_kmalloc_array(dev, num_levels, sizeof(u8), 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'\n");
+ return ret;
+ }
+
+ snprintf(cdev->name, THERMAL_NAME_LENGTH, "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->mcu = mcu;
+ platform_set_drvdata(pdev, mcu_hwmon);
+ mutex_init(&mcu_hwmon->lock);
+
+ 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 v2");
--
2.31.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v9 4/7] drivers: leds: Add the IEI WT61P803 PUZZLE LED driver
2021-08-24 12:44 [PATCH v9 0/7] Add support for the IEI WT61P803 PUZZLE MCU Luka Kovacic
` (2 preceding siblings ...)
2021-08-24 12:44 ` [PATCH v9 3/7] drivers: hwmon: Add the IEI WT61P803 PUZZLE HWMON driver Luka Kovacic
@ 2021-08-24 12:44 ` Luka Kovacic
2021-08-24 12:44 ` [PATCH v9 5/7] Documentation/ABI: Add iei-wt61p803-puzzle driver sysfs interface documentation Luka Kovacic
` (2 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Luka Kovacic @ 2021-08-24 12:44 UTC (permalink / raw)
To: linux-doc, linux-leds, devicetree, linux-hwmon, linux-kernel,
geert+renesas, Max.Merchel, linux, daniel, shawnguo, sam, arnd,
krzysztof.kozlowski, pavo.banicevic, corbet, lee.jones, pavel,
robh+dt, linux, jdelvare, goran.medic, luka.perkov, robert.marko
Cc: Luka Kovacic
Add support for the IEI WT61P803 PUZZLE LED driver.
Currently only the front panel power LED is supported,
since it is the only LED on this board wired through the
MCU.
The LED is wired directly to the on-board MCU controller
and is toggled using an MCU command.
Support for more LEDs is going to be added in case more
boards implement this microcontroller, as LEDs use many
different GPIOs.
This driver depends on the IEI WT61P803 PUZZLE MFD driver.
Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
Signed-off-by: Pavo Banicevic <pavo.banicevic@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 | 147 ++++++++++++++++++++++++
3 files changed, 156 insertions(+)
create mode 100644 drivers/leds/leds-iei-wt61p803-puzzle.c
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index bdf16180f5ff..702ffd98486b 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -333,6 +333,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 7e604d3028c8..2cae3d7e5b1b 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -35,6 +35,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..67d542d6ac72
--- /dev/null
+++ b/drivers/leds/leds-iei-wt61p803-puzzle.c
@@ -0,0 +1,147 @@
+// 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
+ * @cdev: LED classdev
+ * @mcu: MCU struct pointer
+ * @response_buffer Global MCU response buffer
+ * @lock: General mutex lock to protect simultaneous R/W access to led_power_state
+ * @led_power_state: State of the front panel power LED
+ */
+struct iei_wt61p803_puzzle_led {
+ struct led_classdev cdev;
+ struct iei_wt61p803_puzzle *mcu;
+ unsigned char response_buffer[IEI_WT61P803_PUZZLE_BUF_SIZE];
+ struct mutex lock; /* mutex to protect led_power_state */
+ int led_power_state;
+};
+
+static inline struct iei_wt61p803_puzzle_led *cdev_to_iei_wt61p803_puzzle_led
+ (struct led_classdev *led_cdev)
+{
+ return container_of(led_cdev, struct iei_wt61p803_puzzle_led, cdev);
+}
+
+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] = {};
+ size_t reply_size;
+ int ret;
+
+ led_power_cmd[0] = IEI_WT61P803_PUZZLE_CMD_HEADER_START;
+ led_power_cmd[1] = IEI_WT61P803_PUZZLE_CMD_LED;
+ led_power_cmd[2] = IEI_WT61P803_PUZZLE_CMD_LED_POWER;
+ led_power_cmd[3] = brightness == LED_OFF ? IEI_LED_OFF : IEI_LED_ON;
+
+ ret = iei_wt61p803_puzzle_write_command(priv->mcu, led_power_cmd,
+ sizeof(led_power_cmd),
+ resp_buf,
+ &reply_size);
+ if (ret)
+ return ret;
+
+ if (reply_size != 3)
+ return -EIO;
+
+ 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;
+
+ mutex_lock(&priv->lock);
+ priv->led_power_state = brightness;
+ mutex_unlock(&priv->lock);
+
+ return 0;
+}
+
+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;
+
+ if (device_get_child_node_count(dev) != 1)
+ return -EINVAL;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ 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);
+ init_data.fwnode = child;
+
+ 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;
+
+ ret = devm_led_classdev_register_ext(dev, &priv->cdev, &init_data);
+ if (ret)
+ dev_err(dev, "Could not register LED\n");
+
+ 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 v2");
+MODULE_ALIAS("platform:leds-iei-wt61p803-puzzle");
--
2.31.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v9 5/7] Documentation/ABI: Add iei-wt61p803-puzzle driver sysfs interface documentation
2021-08-24 12:44 [PATCH v9 0/7] Add support for the IEI WT61P803 PUZZLE MCU Luka Kovacic
` (3 preceding siblings ...)
2021-08-24 12:44 ` [PATCH v9 4/7] drivers: leds: Add the IEI WT61P803 PUZZLE LED driver Luka Kovacic
@ 2021-08-24 12:44 ` Luka Kovacic
2021-08-24 12:44 ` [PATCH v9 6/7] Documentation/hwmon: Add iei-wt61p803-puzzle hwmon driver documentation Luka Kovacic
2021-08-24 12:44 ` [PATCH v9 7/7] MAINTAINERS: Add an entry for the IEI WT61P803 PUZZLE driver Luka Kovacic
6 siblings, 0 replies; 16+ messages in thread
From: Luka Kovacic @ 2021-08-24 12:44 UTC (permalink / raw)
To: linux-doc, linux-leds, devicetree, linux-hwmon, linux-kernel,
geert+renesas, Max.Merchel, linux, daniel, shawnguo, sam, arnd,
krzysztof.kozlowski, pavo.banicevic, corbet, lee.jones, pavel,
robh+dt, linux, jdelvare, goran.medic, luka.perkov, robert.marko
Cc: 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>
Signed-off-by: Pavo Banicevic <pavo.banicevic@sartura.hr>
Cc: Luka Perkov <luka.perkov@sartura.hr>
Cc: Robert Marko <robert.marko@sartura.hr>
---
.../testing/sysfs-driver-iei-wt61p803-puzzle | 61 +++++++++++++++++++
1 file changed, 61 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..ab4415587f67
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-iei-wt61p803-puzzle
@@ -0,0 +1,61 @@
+What: /sys/bus/serial/devices/.../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/.../serial_number
+Date: September 2020
+Contact: Luka Kovacic <luka.kovacic@sartura.hr>
+Description: (RW) Internal factory assigned serial number
+
+What: /sys/bus/serial/devices/.../version
+Date: September 2020
+Contact: Luka Kovacic <luka.kovacic@sartura.hr>
+Description: (RO) Internal MCU firmware version
+
+What: /sys/bus/serial/devices/.../protocol_version
+Date: September 2020
+Contact: Luka Kovacic <luka.kovacic@sartura.hr>
+Description: (RO) Internal MCU communication protocol version
+
+What: /sys/bus/serial/devices/.../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/.../bootloader_mode
+Date: September 2020
+Contact: Luka Kovacic <luka.kovacic@sartura.hr>
+Description: (RO) Internal MCU bootloader mode status
+ Value mapping:
+ 0 - normal mode
+ 1 - bootloader mode
+
+What: /sys/bus/serial/devices/.../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 - Wake-on-LAN power on
+ 0x02 - RTC alarm power on
+ 0x01 - AC recover power on
+
+What: /sys/bus/serial/devices/.../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/.../ac_recovery_status
+Date: September 2020
+Contact: Luka Kovacic <luka.kovacic@sartura.hr>
+Description: (RO) Host platform AC recovery status value
+ Value mapping:
+ 0 - board has not been recovered from power down
+ 1 - board has been recovered from power down
--
2.31.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v9 6/7] Documentation/hwmon: Add iei-wt61p803-puzzle hwmon driver documentation
2021-08-24 12:44 [PATCH v9 0/7] Add support for the IEI WT61P803 PUZZLE MCU Luka Kovacic
` (4 preceding siblings ...)
2021-08-24 12:44 ` [PATCH v9 5/7] Documentation/ABI: Add iei-wt61p803-puzzle driver sysfs interface documentation Luka Kovacic
@ 2021-08-24 12:44 ` Luka Kovacic
2021-08-24 15:00 ` Guenter Roeck
2021-08-24 12:44 ` [PATCH v9 7/7] MAINTAINERS: Add an entry for the IEI WT61P803 PUZZLE driver Luka Kovacic
6 siblings, 1 reply; 16+ messages in thread
From: Luka Kovacic @ 2021-08-24 12:44 UTC (permalink / raw)
To: linux-doc, linux-leds, devicetree, linux-hwmon, linux-kernel,
geert+renesas, Max.Merchel, linux, daniel, shawnguo, sam, arnd,
krzysztof.kozlowski, pavo.banicevic, corbet, lee.jones, pavel,
robh+dt, linux, jdelvare, goran.medic, luka.perkov, robert.marko
Cc: Luka Kovacic
Add the iei-wt61p803-puzzle driver hwmon driver interface documentation.
Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
Signed-off-by: Pavo Banicevic <pavo.banicevic@sartura.hr>
Cc: Luka Perkov <luka.perkov@sartura.hr>
Cc: Robert Marko <robert.marko@sartura.hr>
---
.../hwmon/iei-wt61p803-puzzle-hwmon.rst | 43 +++++++++++++++++++
Documentation/hwmon/index.rst | 1 +
2 files changed, 44 insertions(+)
create mode 100644 Documentation/hwmon/iei-wt61p803-puzzle-hwmon.rst
diff --git a/Documentation/hwmon/iei-wt61p803-puzzle-hwmon.rst b/Documentation/hwmon/iei-wt61p803-puzzle-hwmon.rst
new file mode 100644
index 000000000000..bbbe97645965
--- /dev/null
+++ b/Documentation/hwmon/iei-wt61p803-puzzle-hwmon.rst
@@ -0,0 +1,43 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+Kernel driver iei-wt61p803-puzzle-hwmon
+=======================================
+
+Supported chips:
+ * IEI WT61P803 PUZZLE for IEI Puzzle M801
+
+ Prefix: 'iei-wt61p803-puzzle-hwmon'
+
+Author: Luka Kovacic <luka.kovacic@sartura.hr>
+
+
+Description
+-----------
+
+This driver adds fan and temperature sensor reading for some IEI Puzzle
+series boards.
+
+Sysfs attributes
+----------------
+
+The following attributes are supported:
+
+- IEI WT61P803 PUZZLE for IEI Puzzle M801
+
+/sys files in hwmon subsystem
+-----------------------------
+
+================= == =====================================================
+fan[1-5]_input RO files for fan speed (in RPM)
+pwm[1-2] RW files for fan[1-2] target duty cycle (0..255)
+temp[1-2]_input RO files for temperature sensors, in millidegree Celsius
+================= == =====================================================
+
+/sys files in thermal subsystem
+-------------------------------
+
+================= == =====================================================
+cur_state RW file for current cooling state of the cooling device
+ (0..max_state)
+max_state RO file for maximum cooling state of the cooling device
+================= == =====================================================
diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index bc01601ea81a..4a050a5bc1f8 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -73,6 +73,7 @@ Hardware Monitoring Kernel Drivers
ibmaem
ibm-cffps
ibmpowernv
+ iei-wt61p803-puzzle-hwmon
ina209
ina2xx
ina3221
--
2.31.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v9 7/7] MAINTAINERS: Add an entry for the IEI WT61P803 PUZZLE driver
2021-08-24 12:44 [PATCH v9 0/7] Add support for the IEI WT61P803 PUZZLE MCU Luka Kovacic
` (5 preceding siblings ...)
2021-08-24 12:44 ` [PATCH v9 6/7] Documentation/hwmon: Add iei-wt61p803-puzzle hwmon driver documentation Luka Kovacic
@ 2021-08-24 12:44 ` Luka Kovacic
6 siblings, 0 replies; 16+ messages in thread
From: Luka Kovacic @ 2021-08-24 12:44 UTC (permalink / raw)
To: linux-doc, linux-leds, devicetree, linux-hwmon, linux-kernel,
geert+renesas, Max.Merchel, linux, daniel, shawnguo, sam, arnd,
krzysztof.kozlowski, pavo.banicevic, corbet, lee.jones, pavel,
robh+dt, linux, jdelvare, goran.medic, luka.perkov, robert.marko
Cc: Luka Kovacic
Add an entry for the IEI WT61P803 PUZZLE driver (MFD, HWMON, LED drivers).
Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
Signed-off-by: Pavo Banicevic <pavo.banicevic@sartura.hr>
Cc: Luka Perkov <luka.perkov@sartura.hr>
Cc: Robert Marko <robert.marko@sartura.hr>
---
MAINTAINERS | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index c6b8a720c0bc..131a95aefe3b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8971,6 +8971,22 @@ F: include/net/nl802154.h
F: net/ieee802154/
F: net/mac802154/
+IEI WT61P803 M801 MFD DRIVER
+M: Luka Kovacic <luka.kovacic@sartura.hr>
+M: Luka Perkov <luka.perkov@sartura.hr>
+M: Goran Medic <goran.medic@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: Documentation/hwmon/iei-wt61p803-puzzle-hwmon.rst
+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.31.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v9 6/7] Documentation/hwmon: Add iei-wt61p803-puzzle hwmon driver documentation
2021-08-24 12:44 ` [PATCH v9 6/7] Documentation/hwmon: Add iei-wt61p803-puzzle hwmon driver documentation Luka Kovacic
@ 2021-08-24 15:00 ` Guenter Roeck
0 siblings, 0 replies; 16+ messages in thread
From: Guenter Roeck @ 2021-08-24 15:00 UTC (permalink / raw)
To: Luka Kovacic
Cc: linux-doc, linux-leds, devicetree, linux-hwmon, linux-kernel,
geert+renesas, Max.Merchel, linux, daniel, shawnguo, sam, arnd,
krzysztof.kozlowski, pavo.banicevic, corbet, lee.jones, pavel,
robh+dt, jdelvare, goran.medic, luka.perkov, robert.marko
On Tue, Aug 24, 2021 at 02:44:37PM +0200, Luka Kovacic wrote:
> Add the iei-wt61p803-puzzle driver hwmon driver interface documentation.
>
> Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
> Signed-off-by: Pavo Banicevic <pavo.banicevic@sartura.hr>
> Cc: Luka Perkov <luka.perkov@sartura.hr>
> Cc: Robert Marko <robert.marko@sartura.hr>
Acked-by: Guenter Roeck <linux@roeck-us.net>
> ---
> .../hwmon/iei-wt61p803-puzzle-hwmon.rst | 43 +++++++++++++++++++
> Documentation/hwmon/index.rst | 1 +
> 2 files changed, 44 insertions(+)
> create mode 100644 Documentation/hwmon/iei-wt61p803-puzzle-hwmon.rst
>
> diff --git a/Documentation/hwmon/iei-wt61p803-puzzle-hwmon.rst b/Documentation/hwmon/iei-wt61p803-puzzle-hwmon.rst
> new file mode 100644
> index 000000000000..bbbe97645965
> --- /dev/null
> +++ b/Documentation/hwmon/iei-wt61p803-puzzle-hwmon.rst
> @@ -0,0 +1,43 @@
> +.. SPDX-License-Identifier: GPL-2.0-only
> +
> +Kernel driver iei-wt61p803-puzzle-hwmon
> +=======================================
> +
> +Supported chips:
> + * IEI WT61P803 PUZZLE for IEI Puzzle M801
> +
> + Prefix: 'iei-wt61p803-puzzle-hwmon'
> +
> +Author: Luka Kovacic <luka.kovacic@sartura.hr>
> +
> +
> +Description
> +-----------
> +
> +This driver adds fan and temperature sensor reading for some IEI Puzzle
> +series boards.
> +
> +Sysfs attributes
> +----------------
> +
> +The following attributes are supported:
> +
> +- IEI WT61P803 PUZZLE for IEI Puzzle M801
> +
> +/sys files in hwmon subsystem
> +-----------------------------
> +
> +================= == =====================================================
> +fan[1-5]_input RO files for fan speed (in RPM)
> +pwm[1-2] RW files for fan[1-2] target duty cycle (0..255)
> +temp[1-2]_input RO files for temperature sensors, in millidegree Celsius
> +================= == =====================================================
> +
> +/sys files in thermal subsystem
> +-------------------------------
> +
> +================= == =====================================================
> +cur_state RW file for current cooling state of the cooling device
> + (0..max_state)
> +max_state RO file for maximum cooling state of the cooling device
> +================= == =====================================================
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> index bc01601ea81a..4a050a5bc1f8 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -73,6 +73,7 @@ Hardware Monitoring Kernel Drivers
> ibmaem
> ibm-cffps
> ibmpowernv
> + iei-wt61p803-puzzle-hwmon
> ina209
> ina2xx
> ina3221
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v9 1/7] dt-bindings: Add IEI vendor prefix and IEI WT61P803 PUZZLE driver bindings
2021-08-24 12:44 ` [PATCH v9 1/7] dt-bindings: Add IEI vendor prefix and IEI WT61P803 PUZZLE driver bindings Luka Kovacic
@ 2021-08-24 18:09 ` Rob Herring
0 siblings, 0 replies; 16+ messages in thread
From: Rob Herring @ 2021-08-24 18:09 UTC (permalink / raw)
To: Luka Kovacic
Cc: luka.perkov, linux-doc, robh+dt, Max.Merchel, shawnguo, linux,
krzysztof.kozlowski, pavo.banicevic, jdelvare, linux-leds,
daniel, geert+renesas, linux-hwmon, linux-kernel, goran.medic,
lee.jones, pavel, arnd, sam, robert.marko, linux, devicetree,
corbet
On Tue, 24 Aug 2021 14:44:32 +0200, Luka Kovacic wrote:
> 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>
> Signed-off-by: Pavo Banicevic <pavo.banicevic@sartura.hr>
> Cc: Luka Perkov <luka.perkov@sartura.hr>
> Cc: Robert Marko <robert.marko@sartura.hr>
> ---
> .../hwmon/iei,wt61p803-puzzle-hwmon.yaml | 53 ++++++++++++
> .../leds/iei,wt61p803-puzzle-leds.yaml | 39 +++++++++
> .../bindings/mfd/iei,wt61p803-puzzle.yaml | 82 +++++++++++++++++++
> .../devicetree/bindings/vendor-prefixes.yaml | 2 +
> 4 files changed, 176 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
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v9 2/7] drivers: mfd: Add a driver for IEI WT61P803 PUZZLE MCU
2021-08-24 12:44 ` [PATCH v9 2/7] drivers: mfd: Add a driver for IEI WT61P803 PUZZLE MCU Luka Kovacic
@ 2021-09-22 10:54 ` Lee Jones
2021-09-22 11:42 ` Greg KH
2021-09-22 11:41 ` Greg KH
1 sibling, 1 reply; 16+ messages in thread
From: Lee Jones @ 2021-09-22 10:54 UTC (permalink / raw)
To: Luka Kovacic
Cc: linux-doc, linux-leds, devicetree, linux-hwmon, linux-kernel,
geert+renesas, Max.Merchel, linux, daniel, shawnguo, sam, arnd,
krzysztof.kozlowski, pavo.banicevic, corbet, pavel, robh+dt,
linux, jdelvare, goran.medic, luka.perkov, robert.marko
Greg,
Would you be kind enough to take a look at the SYS imp. please?
I've marked the start with:
###########################################
###########################################
On Tue, 24 Aug 2021, Luka Kovacic wrote:
> Add a driver for the IEI WT61P803 PUZZLE microcontroller, used in some
> IEI Puzzle series devices. The microcontroller controls system power,
> temperature sensors, fans and LEDs.
>
> This driver implements the core functionality for device communication
> over the system serial (serdev bus). It handles MCU messages and the
> internal MCU properties. Some properties can be managed over sysfs.
>
> Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
> Signed-off-by: Pavo Banicevic <pavo.banicevic@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 | 908 ++++++++++++++++++++++++
> include/linux/mfd/iei-wt61p803-puzzle.h | 66 ++
> 4 files changed, 983 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 6a3fd2d75f96..c2183eb0775f 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2170,6 +2170,14 @@ 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.
> +
> config MFD_INTEL_M10_BMC
> tristate "Intel MAX 10 Board Management Controller"
> depends on SPI_MASTER
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 8116c19d5fd4..42b9767ec37a 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -236,6 +236,7 @@ obj-$(CONFIG_MFD_DLN2) += dln2.o
> obj-$(CONFIG_MFD_RT4831) += rt4831.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..596ecbc65627
> --- /dev/null
> +++ b/drivers/mfd/iei-wt61p803-puzzle.c
> @@ -0,0 +1,908 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* IEI WT61P803 PUZZLE MCU Driver
This breaks coding standard conventions.
top line should be empty.
Place a '\n' below it too please.
> + * 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 <linux/atomic.h>
> +#include <linux/delay.h>
> +#include <linux/export.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/iei-wt61p803-puzzle.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/property.h>
> +#include <linux/sched.h>
> +#include <linux/serdev.h>
> +#include <linux/slab.h>
> +#include <linux/sysfs.h>
> +#include <asm/unaligned.h>
Are you 100% sure all of these are in use?
> +/* start, payload and XOR checksum at end */
> +#define IEI_WT61P803_PUZZLE_MAX_COMMAND_LENGTH (1 + 20 + 1)
> +#define IEI_WT61P803_PUZZLE_RESP_BUF_SIZE 512
> +
> +#define IEI_WT61P803_PUZZLE_MAC_LENGTH 17
> +#define IEI_WT61P803_PUZZLE_SN_LENGTH 36
> +#define IEI_WT61P803_PUZZLE_VERSION_LENGTH 6
> +#define IEI_WT61P803_PUZZLE_BUILD_INFO_LENGTH 16
> +#define IEI_WT61P803_PUZZLE_PROTOCOL_VERSION_LENGTH 8
> +#define IEI_WT61P803_PUZZLE_NB_MAC 8
> +
> +/* Use HZ as a timeout value throughout the driver */
> +#define IEI_WT61P803_PUZZLE_GENERAL_TIMEOUT HZ
> +
> +enum iei_wt61p803_puzzle_attribute_type {
> + IEI_WT61P803_PUZZLE_VERSION,
> + IEI_WT61P803_PUZZLE_BUILD_INFO,
> + IEI_WT61P803_PUZZLE_BOOTLOADER_MODE,
> + IEI_WT61P803_PUZZLE_PROTOCOL_VERSION,
> + IEI_WT61P803_PUZZLE_SERIAL_NUMBER,
> + IEI_WT61P803_PUZZLE_MAC_ADDRESS,
> + IEI_WT61P803_PUZZLE_AC_RECOVERY_STATUS,
> + IEI_WT61P803_PUZZLE_POWER_LOSS_RECOVERY,
> + IEI_WT61P803_PUZZLE_POWER_STATUS,
> +};
> +
> +struct iei_wt61p803_puzzle_device_attribute {
> + struct device_attribute dev_attr;
> + enum iei_wt61p803_puzzle_attribute_type type;
> + u8 index;
> +};
Kernel-doc?
> +/**
> + * 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;
> +};
> +
> +/**
> + * 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[IEI_WT61P803_PUZZLE_RESP_BUF_SIZE];
> + 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
> + *
> + * Last element of arrays is reserved for '\0'.
> + */
> +struct iei_wt61p803_puzzle_mcu_version {
> + char version[IEI_WT61P803_PUZZLE_VERSION_LENGTH + 1];
> + char build_info[IEI_WT61P803_PUZZLE_BUILD_INFO_LENGTH + 1];
> + bool bootloader_mode;
> + char protocol_version[IEI_WT61P803_PUZZLE_PROTOCOL_VERSION_LENGTH + 1];
> + char serial_number[IEI_WT61P803_PUZZLE_SN_LENGTH + 1];
> + char mac_address[IEI_WT61P803_PUZZLE_NB_MAC][IEI_WT61P803_PUZZLE_MAC_LENGTH + 1];
> +};
> +
> +/**
> + * struct iei_wt61p803_puzzle - IEI WT61P803 PUZZLE MCU Driver
> + * @serdev: Pointer to underlying serdev device
> + * @dev: Pointer to underlying dev device
> + * @reply_lock: Reply 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 device *dev;
> + struct mutex reply_lock; /* lock to prevent multiple firmware calls */
> + struct iei_wt61p803_puzzle_reply *reply;
> + struct iei_wt61p803_puzzle_mcu_version version;
> + struct iei_wt61p803_puzzle_mcu_status status;
> + unsigned char response_buffer[IEI_WT61P803_PUZZLE_BUF_SIZE];
> + struct mutex lock; /* lock to protect response buffer */
> +};
> +
> +static unsigned char iei_wt61p803_puzzle_checksum(unsigned char *buf, size_t len)
> +{
> + unsigned char checksum = 0;
> + size_t i;
> +
> + for (i = 0; i < len; i++)
> + checksum ^= buf[i];
> + return checksum;
> +}
Are you sure there isn't a generic implementation you can use?
> +static int iei_wt61p803_puzzle_process_resp(struct iei_wt61p803_puzzle *mcu,
> + const unsigned char *raw_resp_data, size_t size)
> +{
> + unsigned char checksum;
> +
> + /* 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))) {
> + if (mcu->reply->size + size >= sizeof(mcu->reply->data))
> + return -EIO;
> +
> + /* Append the frame to existing data */
> + memcpy(mcu->reply->data + mcu->reply->size, raw_resp_data, size);
> + mcu->reply->size += size;
> + } else {
> + if (size >= sizeof(mcu->reply->data))
> + return -EIO;
> +
> + /* Start processing a new frame */
> + memcpy(mcu->reply->data, raw_resp_data, size);
> + mcu->reply->size = size;
> + }
> +
> + 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 */
> + return size;
> + }
> +
> + /* Received all the data */
> + complete(&mcu->reply->received);
> +
> + return 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, data, size);
> + /* Return the number of processed bytes if function returns error,
> + * discard the remaining incoming data, since the frame this data
> + * belongs to is broken anyway
> + */
Place this comment above the function call please.
And fix all of your comments to conform to the kernel Coding Standard.
> + if (ret < 0)
> + return 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, "Command response timed out. Retries: %d\n", retry_count);
> +
> + return -ETIMEDOUT;
> +}
> +EXPORT_SYMBOL_GPL(iei_wt61p803_puzzle_write_command_watchdog);
Don't rely on the export/prototype for forward declaration.
Place this function below iei_wt61p803_puzzle_write_command() please.
> +/**
> + * 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;
> +
> + if (size <= 1 || size > IEI_WT61P803_PUZZLE_MAX_COMMAND_LENGTH)
> + return -EINVAL;
> +
> + mutex_lock(&mcu->reply_lock);
> +
> + cmd[size - 1] = iei_wt61p803_puzzle_checksum(cmd, size - 1);
> +
> + /* Initialize reply struct */
> + reinit_completion(&mcu->reply->received);
> + mcu->reply->size = 0;
'\n' here, as the next line is not related to the comment.
> + usleep_range(2000, 10000);
What's the purpose of the sleep here?
Where does this range come from? Is there a datasheet?
'\n' here.
> + serdev_device_write_flush(mcu->serdev);
> + ret = serdev_device_write_buf(mcu->serdev, cmd, size);
> + if (ret < 0)
> + goto exit;
> +
> + serdev_device_wait_until_sent(mcu->serdev, IEI_WT61P803_PUZZLE_GENERAL_TIMEOUT);
> + ret = wait_for_completion_timeout(&mcu->reply->received,
> + IEI_WT61P803_PUZZLE_GENERAL_TIMEOUT);
> + if (ret == 0) {
> + dev_err(dev, "Command reply receive timeout\n");
Can you turn this into a proper English sentence please?
Perhaps "Timeout command received" or "Command timed out"?
> + ret = -ETIMEDOUT;
> + goto exit;
Tell us the goto's intention, so 'unlock' here.
> + }
> +
> + *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;
read/write calls usually return the size of data read/written.
'\n' here. This code is very bunched up/hard to read.
> +exit:
> + mutex_unlock(&mcu->reply_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(iei_wt61p803_puzzle_write_command);
> +
> +static int iei_wt61p803_puzzle_buzzer(struct iei_wt61p803_puzzle *mcu, bool long_beep)
> +{
> + unsigned char *resp_buf = mcu->response_buffer;
> + unsigned char buzzer_cmd[4] = {};
> + size_t reply_size;
> + int ret;
> +
> + buzzer_cmd[0] = IEI_WT61P803_PUZZLE_CMD_HEADER_START;
> + buzzer_cmd[1] = IEI_WT61P803_PUZZLE_CMD_FUNCTION_SINGLE;
> + buzzer_cmd[2] = long_beep ? '3' : '2'; /* Buzzer 1.5 / 0.5 second beep */
> +
> + mutex_lock(&mcu->lock);
> + ret = iei_wt61p803_puzzle_write_command(mcu, buzzer_cmd, sizeof(buzzer_cmd),
> + 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;
> + }
Why does all this processing have to be conducted under a lock?
'\n'
> +exit:
> + mutex_unlock(&mcu->lock);
> + return ret;
> +}
> +
> +static int iei_wt61p803_puzzle_get_version(struct iei_wt61p803_puzzle *mcu)
> +{
> + unsigned char version_cmd[3] = {
As there are multiple 'version's, please name them all.
Is this the 'hw_version'?
> + 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;
> + 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;
This is different again. Please use 'unlock' throughout.
'\n' after all of these (ret) checks please.
Sometimes it's there, other times it's not. Please be consistent.
> + if (reply_size < 7) {
> + ret = -EIO;
> + goto err;
> + }
> + sprintf(mcu->version.version, "v%c.%.3s", rb[2], &rb[3]);
> +
> + 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;
> + }
I don't like this setup.
Please change the API so that everything is encoded in ret.
If ret is less than 0 then an error occurred else encode the
reply_size into it.
> + sprintf(mcu->version.build_info, "%c%c/%c%c/%.4s %c%c:%c%c",
> + rb[8], rb[9], rb[6], rb[7], &rb[2], 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;
You can drop one of these checks, no?
If bootloader_mode is already initialised you only need the 2nd one.
If it's not, just 'else bootloader_mode = false'.
> + 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]);
'\n'
> +err:
'unlock'
> + 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;
> + 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];
> + }
'\n'
> +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, /* EEPROM read address */
> + 0x24, /* Data length */
All of the magic numbers in this driver should be defined ideally.
> + };
> + size_t reply_size;
> + 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;
> +
> + if (reply_size < IEI_WT61P803_PUZZLE_SN_LENGTH + 4) {
Why 4? Probably better to define it.
> + ret = -EIO;
> + goto err;
> + }
> +
> + sprintf(mcu->version.serial_number, "%.*s",
> + IEI_WT61P803_PUZZLE_SN_LENGTH, resp_buf + 4);
'\n'
> +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, /* EEPROM write address */
> + 0xC, /* Data length */
> + };
> + unsigned char serial_number_cmd[4 + 12 + 1]; /* header, serial number, XOR checksum */
> + int ret, sn_counter;
> + size_t reply_size;
> +
> + /* The MCU can only handle 22 byte messages, send the S/N in 12 byte chunks */
Byte
> + mutex_lock(&mcu->lock);
> + for (sn_counter = 0; sn_counter < 3; sn_counter++) {
Why 3? Please define it.
> + serial_number_header[2] = 0x0 + 0xC * sn_counter;
Define all magic numbers please.
I won't mention this again from here.
> + memcpy(serial_number_cmd, serial_number_header, sizeof(serial_number_header));
> + memcpy(serial_number_cmd + sizeof(serial_number_header),
> + serial_number + 0xC * sn_counter, 0xC);
> +
> + 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_SN_LENGTH, serial_number);
> +err:
> + mutex_unlock(&mcu->lock);
> + return ret;
> +}
> +
> +static int iei_wt61p803_puzzle_get_mac_address(struct iei_wt61p803_puzzle *mcu, int index)
> +{
> + 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, /* EEPROM read address */
> + 0x11, /* Data length */
> + };
> + size_t reply_size;
> + int ret;
> +
> + mutex_lock(&mcu->lock);
> + mac_address_cmd[2] = 0x24 + 0x11 * index;
> +
> + 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 < 22) {
> + ret = -EIO;
> + goto err;
> + }
> +
> + sprintf(mcu->version.mac_address[index], "%.*s",
> + IEI_WT61P803_PUZZLE_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[IEI_WT61P803_PUZZLE_MAC_LENGTH],
> + int mac_address_idx)
> +{
> + unsigned char mac_address_cmd[4 + IEI_WT61P803_PUZZLE_MAC_LENGTH + 1];
> + 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, /* EEPROM write address */
> + 0x11, /* Data length */
> + };
> + size_t reply_size;
> + int ret;
> +
> + if (mac_address_idx < 0 || mac_address_idx >= IEI_WT61P803_PUZZLE_NB_MAC)
> + 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, sizeof(mac_address_header));
> + memcpy(mac_address_cmd + sizeof(mac_address_header), mac_address,
> + IEI_WT61P803_PUZZLE_MAC_LENGTH);
> +
> + 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_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 *resp_buf = mcu->response_buffer;
> + unsigned char power_loss_recovery_cmd[5] = {};
> + size_t reply_size;
> + int ret;
> +
> + if (power_loss_recovery_action < 0 || power_loss_recovery_action > 4)
What are the 5 actions? Are they mentioned/defined/enumed anywhere?
> + return -EINVAL;
> +
> + power_loss_recovery_cmd[0] = IEI_WT61P803_PUZZLE_CMD_HEADER_START;
> + power_loss_recovery_cmd[1] = IEI_WT61P803_PUZZLE_CMD_FUNCTION_OTHER;
> + power_loss_recovery_cmd[2] = IEI_WT61P803_PUZZLE_CMD_FUNCTION_OTHER_POWER_LOSS;
> + power_loss_recovery_cmd[3] = hex_asc[power_loss_recovery_action];
> +
> + 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 to_puzzle_dev_attr(_attr) \
> + container_of(_attr, struct iei_wt61p803_puzzle_device_attribute, dev_attr)
Place this at the top of the file please.
> +static ssize_t show_output(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev);
> + struct iei_wt61p803_puzzle_device_attribute *pattr = to_puzzle_dev_attr(attr);
> + int ret;
> +
> + switch (pattr->type) {
> + case IEI_WT61P803_PUZZLE_VERSION:
> + return scnprintf(buf, PAGE_SIZE, "%s\n", mcu->version.version);
> + case IEI_WT61P803_PUZZLE_BUILD_INFO:
> + return scnprintf(buf, PAGE_SIZE, "%s\n", mcu->version.build_info);
> + case IEI_WT61P803_PUZZLE_BOOTLOADER_MODE:
> + return scnprintf(buf, PAGE_SIZE, "%d\n", mcu->version.bootloader_mode);
> + case IEI_WT61P803_PUZZLE_PROTOCOL_VERSION:
> + return scnprintf(buf, PAGE_SIZE, "%s\n", mcu->version.protocol_version);
> + case IEI_WT61P803_PUZZLE_SERIAL_NUMBER:
> + ret = iei_wt61p803_puzzle_get_serial_number(mcu);
> + if (!ret)
> + ret = scnprintf(buf, PAGE_SIZE, "%s\n", mcu->version.serial_number);
> + else
> + ret = 0;
> + return ret;
> + case IEI_WT61P803_PUZZLE_MAC_ADDRESS:
> + ret = iei_wt61p803_puzzle_get_mac_address(mcu, pattr->index);
> + if (!ret)
> + ret = scnprintf(buf, PAGE_SIZE, "%s\n",
> + mcu->version.mac_address[pattr->index]);
> + else
> + ret = 0;
> + return ret;
> + case IEI_WT61P803_PUZZLE_AC_RECOVERY_STATUS:
> + case IEI_WT61P803_PUZZLE_POWER_LOSS_RECOVERY:
> + case IEI_WT61P803_PUZZLE_POWER_STATUS:
> + ret = iei_wt61p803_puzzle_get_mcu_status(mcu);
> + if (ret)
> + return ret;
> +
> + mutex_lock(&mcu->lock);
> + switch (pattr->type) {
> + case IEI_WT61P803_PUZZLE_AC_RECOVERY_STATUS:
> + ret = scnprintf(buf, PAGE_SIZE, "%x\n",
> + mcu->status.ac_recovery_status_flag);
> + break;
> + case IEI_WT61P803_PUZZLE_POWER_LOSS_RECOVERY:
> + ret = scnprintf(buf, PAGE_SIZE, "%x\n", mcu->status.power_loss_recovery);
> + break;
> + case IEI_WT61P803_PUZZLE_POWER_STATUS:
> + ret = scnprintf(buf, PAGE_SIZE, "%x\n", mcu->status.power_status);
> + break;
> + default:
> + ret = 0;
> + break;
> + }
> + mutex_unlock(&mcu->lock);
> + return ret;
> + default:
> + return 0;
> + }
> +
> + return 0;
> +}
> +
> +static ssize_t store_output(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + unsigned char serial_number[IEI_WT61P803_PUZZLE_SN_LENGTH];
> + unsigned char mac_address[IEI_WT61P803_PUZZLE_MAC_LENGTH];
> + struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev);
> + struct iei_wt61p803_puzzle_device_attribute *pattr = to_puzzle_dev_attr(attr);
> + int power_loss_recovery_action = 0;
> + int ret;
> +
> + switch (pattr->type) {
> + case IEI_WT61P803_PUZZLE_SERIAL_NUMBER:
> + if (len != (size_t)(IEI_WT61P803_PUZZLE_SN_LENGTH + 1))
> + return -EINVAL;
> + memcpy(serial_number, buf, sizeof(serial_number));
> + ret = iei_wt61p803_puzzle_write_serial_number(mcu, serial_number);
> + if (ret)
> + return ret;
> + return len;
> + case IEI_WT61P803_PUZZLE_MAC_ADDRESS:
> + if (len != (size_t)(IEI_WT61P803_PUZZLE_MAC_LENGTH + 1))
> + return -EINVAL;
> +
> + memcpy(mac_address, buf, sizeof(mac_address));
> +
> + if (strlen(attr->attr.name) != 13)
> + return -EIO;
> +
> + ret = iei_wt61p803_puzzle_write_mac_address(mcu, mac_address, pattr->index);
> + if (ret)
> + return ret;
> + return len;
> + case IEI_WT61P803_PUZZLE_POWER_LOSS_RECOVERY:
> + ret = kstrtoint(buf, 10, &power_loss_recovery_action);
> + if (ret)
> + return ret;
> + ret = iei_wt61p803_puzzle_write_power_loss_recovery(mcu,
> + power_loss_recovery_action);
> + if (ret)
> + return ret;
> + return len;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +#define IEI_WT61P803_PUZZLE_ATTR(_name, _mode, _show, _store, _type, _index) \
> + struct iei_wt61p803_puzzle_device_attribute dev_attr_##_name = \
> + { .dev_attr = __ATTR(_name, _mode, _show, _store), \
> + .type = _type, \
> + .index = _index }
> +
> +#define IEI_WT61P803_PUZZLE_ATTR_RO(_name, _type, _id) \
> + IEI_WT61P803_PUZZLE_ATTR(_name, 0444, show_output, NULL, _type, _id)
> +
> +#define IEI_WT61P803_PUZZLE_ATTR_RW(_name, _type, _id) \
> + IEI_WT61P803_PUZZLE_ATTR(_name, 0644, show_output, store_output, _type, _id)
> +
> +static IEI_WT61P803_PUZZLE_ATTR_RO(version, IEI_WT61P803_PUZZLE_VERSION, 0);
> +static IEI_WT61P803_PUZZLE_ATTR_RO(build_info, IEI_WT61P803_PUZZLE_BUILD_INFO, 0);
> +static IEI_WT61P803_PUZZLE_ATTR_RO(bootloader_mode, IEI_WT61P803_PUZZLE_BOOTLOADER_MODE, 0);
> +static IEI_WT61P803_PUZZLE_ATTR_RO(protocol_version, IEI_WT61P803_PUZZLE_PROTOCOL_VERSION, 0);
> +static IEI_WT61P803_PUZZLE_ATTR_RW(serial_number, IEI_WT61P803_PUZZLE_SERIAL_NUMBER, 0);
> +static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_0, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 0);
> +static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_1, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 1);
> +static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_2, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 2);
> +static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_3, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 3);
> +static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_4, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 4);
> +static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_5, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 5);
> +static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_6, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 6);
> +static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_7, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 7);
> +static IEI_WT61P803_PUZZLE_ATTR_RO(ac_recovery_status, IEI_WT61P803_PUZZLE_AC_RECOVERY_STATUS, 0);
> +static IEI_WT61P803_PUZZLE_ATTR_RW(power_loss_recovery, IEI_WT61P803_PUZZLE_POWER_LOSS_RECOVERY, 0);
> +static IEI_WT61P803_PUZZLE_ATTR_RO(power_status, IEI_WT61P803_PUZZLE_POWER_STATUS, 0);
> +
> +static struct attribute *iei_wt61p803_puzzle_attrs[] = {
> + &dev_attr_version.dev_attr.attr,
> + &dev_attr_build_info.dev_attr.attr,
> + &dev_attr_bootloader_mode.dev_attr.attr,
> + &dev_attr_protocol_version.dev_attr.attr,
> + &dev_attr_serial_number.dev_attr.attr,
> + &dev_attr_mac_address_0.dev_attr.attr,
> + &dev_attr_mac_address_1.dev_attr.attr,
> + &dev_attr_mac_address_2.dev_attr.attr,
> + &dev_attr_mac_address_3.dev_attr.attr,
> + &dev_attr_mac_address_4.dev_attr.attr,
> + &dev_attr_mac_address_5.dev_attr.attr,
> + &dev_attr_mac_address_6.dev_attr.attr,
> + &dev_attr_mac_address_7.dev_attr.attr,
> + &dev_attr_ac_recovery_status.dev_attr.attr,
> + &dev_attr_power_loss_recovery.dev_attr.attr,
> + &dev_attr_power_status.dev_attr.attr,
> + NULL
> +};
> +ATTRIBUTE_GROUPS(iei_wt61p803_puzzle);
> +
> +static int iei_wt61p803_puzzle_sysfs_create(struct device *dev,
> + struct iei_wt61p803_puzzle *mcu)
Why do you need 'mcu' here?
> +{
> + int ret;
> +
> + ret = sysfs_create_groups(&mcu->dev->kobj, iei_wt61p803_puzzle_groups);
> + if (ret)
> + mfd_remove_devices(mcu->dev);
You can't remove devices you haven't added!
> + return ret;
> +}
> +
> +static int iei_wt61p803_puzzle_sysfs_remove(struct device *dev,
> + struct iei_wt61p803_puzzle *mcu)
> +{
> + /* Remove sysfs groups */
> + sysfs_remove_groups(&mcu->dev->kobj, iei_wt61p803_puzzle_groups);
> + mfd_remove_devices(mcu->dev);
> +
> + 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");
Same line please.
"'current-speed' device tree property not found"
> + return -EINVAL;
> + }
> + dev_dbg(dev, "Driver baud rate: %d\n", baud);
> +
> + /* Allocate the memory */
Drop this comment.
> + 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;
> +
> + /* Initialize device struct data */
And this please.
> + mcu->serdev = serdev;
> + mcu->dev = dev;
Doesn't serdev already have dev?
If so, you don't need to store both.
> + init_completion(&mcu->reply->received);
> + mutex_init(&mcu->reply_lock);
Why can't you use 'lock' in all cases?
> + mutex_init(&mcu->lock);
> +
> + /* Setup UART interface */
Drop the comment please. It's superfluous.
> + 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;
'\n'
> + serdev_device_set_baudrate(serdev, baud);
> + serdev_device_set_flow_control(serdev, false);
> + ret = serdev_device_set_parity(serdev, SERDEV_PARITY_NONE);
> + if (ret) {
> + dev_err(dev, "Failed to set parity\n");
> + return ret;
> + }
> +
> + ret = iei_wt61p803_puzzle_get_version(mcu);
> + if (ret)
> + return ret;
> +
> + dev_dbg(dev, "MCU version: %s\n", mcu->version.version);
> + dev_dbg(dev, "MCU firmware build info: %s\n", mcu->version.build_info);
> + dev_dbg(dev, "MCU in bootloader mode: %s\n",
> + mcu->version.bootloader_mode ? "true" : "false");
> + dev_dbg(dev, "MCU protocol version: %s\n", mcu->version.protocol_version);
> +
> + if (device_property_read_bool(dev, "enable-beep")) {
> + ret = iei_wt61p803_puzzle_buzzer(mcu, false);
You're beeping just because it's enabled?
> + if (ret)
> + return ret;
> + }
> +
> + ret = iei_wt61p803_puzzle_sysfs_create(dev, mcu);
> + if (ret)
> + return ret;
> +
> + return devm_of_platform_populate(dev);
> +}
> +
> +static void iei_wt61p803_puzzle_remove(struct serdev_device *serdev)
> +{
> + struct device *dev = &serdev->dev;
> + struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev);
> +
> + iei_wt61p803_puzzle_sysfs_remove(dev, mcu);
> +}
> +
> +static const struct of_device_id iei_wt61p803_puzzle_dt_ids[] = {
> + { .compatible = "iei,wt61p803-puzzle" },
> + { }
> +};
> +
Remove this line.
> +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,
No need to tab these out to match the subordinate structure.
> + .driver = {
> + .name = "iei-wt61p803-puzzle",
> + .of_match_table = iei_wt61p803_puzzle_dt_ids,
> + },
> +};
> +
Remove this line.
> +module_serdev_device_driver(iei_wt61p803_puzzle_drv);
> +
> +MODULE_LICENSE("GPL v2");
> +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..7f2da5887dae
> --- /dev/null
> +++ b/include/linux/mfd/iei-wt61p803-puzzle.h
> @@ -0,0 +1,66 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/* IEI WT61P803 PUZZLE MCU Driver
Non-conformant multi-line comment
> + * 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_READ 0x5A /* Z */
> +#define IEI_WT61P803_PUZZLE_CMD_FAN_PWM_WRITE 0x57 /* W */
> +#define IEI_WT61P803_PUZZLE_CMD_FAN_PWM_BASE 0x30
> +#define IEI_WT61P803_PUZZLE_CMD_FAN_RPM_BASE 0x41 /* A */
> +
> +#define IEI_WT61P803_PUZZLE_CMD_FAN_PWM(x) (IEI_WT61P803_PUZZLE_CMD_FAN_PWM_BASE + (x)) /* 0 - 1 */
> +#define IEI_WT61P803_PUZZLE_CMD_FAN_RPM(x) (IEI_WT61P803_PUZZLE_CMD_FAN_RPM_BASE + (x)) /* 0 - 5 */
> +
> +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_ */
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v9 2/7] drivers: mfd: Add a driver for IEI WT61P803 PUZZLE MCU
2021-08-24 12:44 ` [PATCH v9 2/7] drivers: mfd: Add a driver for IEI WT61P803 PUZZLE MCU Luka Kovacic
2021-09-22 10:54 ` Lee Jones
@ 2021-09-22 11:41 ` Greg KH
2021-09-22 12:18 ` Geert Uytterhoeven
1 sibling, 1 reply; 16+ messages in thread
From: Greg KH @ 2021-09-22 11:41 UTC (permalink / raw)
To: Luka Kovacic
Cc: linux-doc, linux-leds, devicetree, linux-hwmon, linux-kernel,
geert+renesas, Max.Merchel, linux, daniel, shawnguo, sam, arnd,
krzysztof.kozlowski, pavo.banicevic, corbet, lee.jones, pavel,
robh+dt, linux, jdelvare, goran.medic, luka.perkov, robert.marko
On Tue, Aug 24, 2021 at 02:44:33PM +0200, Luka Kovacic wrote:
> +++ b/drivers/mfd/iei-wt61p803-puzzle.c
> @@ -0,0 +1,908 @@
> +// 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.
It is 2021 now :(
> + * Author: Luka Kovacic <luka.kovacic@sartura.hr>
> + */
> +
> +#include <linux/atomic.h>
> +#include <linux/delay.h>
> +#include <linux/export.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/iei-wt61p803-puzzle.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/property.h>
> +#include <linux/sched.h>
> +#include <linux/serdev.h>
> +#include <linux/slab.h>
> +#include <linux/sysfs.h>
> +#include <asm/unaligned.h>
Did you run checkpatch.pl on this series?
> +struct iei_wt61p803_puzzle_device_attribute {
> + struct device_attribute dev_attr;
> + enum iei_wt61p803_puzzle_attribute_type type;
> + u8 index;
> +};
Why does a driver need a special attribute structure? What is wrong
with the existing one that this is required?
Your "type" and index can come from the device that the attribute is
called for, right?
> +/**
> + * struct iei_wt61p803_puzzle - IEI WT61P803 PUZZLE MCU Driver
This isn't a driver, it's a device.
> + * @serdev: Pointer to underlying serdev device
> + * @dev: Pointer to underlying dev device
> + * @reply_lock: Reply 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 device *dev;
What controls the lifespan of this object?
> + struct mutex reply_lock; /* lock to prevent multiple firmware calls */
> + struct iei_wt61p803_puzzle_reply *reply;
> + struct iei_wt61p803_puzzle_mcu_version version;
> + struct iei_wt61p803_puzzle_mcu_status status;
> + unsigned char response_buffer[IEI_WT61P803_PUZZLE_BUF_SIZE];
> + struct mutex lock; /* lock to protect response buffer */
> +};
> +static ssize_t show_output(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev);
> + struct iei_wt61p803_puzzle_device_attribute *pattr = to_puzzle_dev_attr(attr);
> + int ret;
> +
> + switch (pattr->type) {
> + case IEI_WT61P803_PUZZLE_VERSION:
> + return scnprintf(buf, PAGE_SIZE, "%s\n", mcu->version.version);
Use sysfs_emit() for all of these please.
> + case IEI_WT61P803_PUZZLE_BUILD_INFO:
> + return scnprintf(buf, PAGE_SIZE, "%s\n", mcu->version.build_info);
> + case IEI_WT61P803_PUZZLE_BOOTLOADER_MODE:
> + return scnprintf(buf, PAGE_SIZE, "%d\n", mcu->version.bootloader_mode);
> + case IEI_WT61P803_PUZZLE_PROTOCOL_VERSION:
> + return scnprintf(buf, PAGE_SIZE, "%s\n", mcu->version.protocol_version);
> + case IEI_WT61P803_PUZZLE_SERIAL_NUMBER:
> + ret = iei_wt61p803_puzzle_get_serial_number(mcu);
> + if (!ret)
> + ret = scnprintf(buf, PAGE_SIZE, "%s\n", mcu->version.serial_number);
> + else
> + ret = 0;
No error? Why not?
> + return ret;
> + case IEI_WT61P803_PUZZLE_MAC_ADDRESS:
> + ret = iei_wt61p803_puzzle_get_mac_address(mcu, pattr->index);
> + if (!ret)
> + ret = scnprintf(buf, PAGE_SIZE, "%s\n",
> + mcu->version.mac_address[pattr->index]);
> + else
> + ret = 0;
> + return ret;
> + case IEI_WT61P803_PUZZLE_AC_RECOVERY_STATUS:
> + case IEI_WT61P803_PUZZLE_POWER_LOSS_RECOVERY:
> + case IEI_WT61P803_PUZZLE_POWER_STATUS:
> + ret = iei_wt61p803_puzzle_get_mcu_status(mcu);
> + if (ret)
> + return ret;
> +
> + mutex_lock(&mcu->lock);
> + switch (pattr->type) {
> + case IEI_WT61P803_PUZZLE_AC_RECOVERY_STATUS:
> + ret = scnprintf(buf, PAGE_SIZE, "%x\n",
> + mcu->status.ac_recovery_status_flag);
> + break;
> + case IEI_WT61P803_PUZZLE_POWER_LOSS_RECOVERY:
> + ret = scnprintf(buf, PAGE_SIZE, "%x\n", mcu->status.power_loss_recovery);
> + break;
> + case IEI_WT61P803_PUZZLE_POWER_STATUS:
> + ret = scnprintf(buf, PAGE_SIZE, "%x\n", mcu->status.power_status);
> + break;
> + default:
> + ret = 0;
No error to return?
> + break;
> + }
> + mutex_unlock(&mcu->lock);
> + return ret;
> + default:
> + return 0;
Again, no error to return?
> + }
> +
> + return 0;
Shouldn't you return the size of the buffer?
> +}
> +
> +static ssize_t store_output(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + unsigned char serial_number[IEI_WT61P803_PUZZLE_SN_LENGTH];
> + unsigned char mac_address[IEI_WT61P803_PUZZLE_MAC_LENGTH];
> + struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev);
> + struct iei_wt61p803_puzzle_device_attribute *pattr = to_puzzle_dev_attr(attr);
> + int power_loss_recovery_action = 0;
> + int ret;
> +
> + switch (pattr->type) {
> + case IEI_WT61P803_PUZZLE_SERIAL_NUMBER:
> + if (len != (size_t)(IEI_WT61P803_PUZZLE_SN_LENGTH + 1))
> + return -EINVAL;
> + memcpy(serial_number, buf, sizeof(serial_number));
> + ret = iei_wt61p803_puzzle_write_serial_number(mcu, serial_number);
> + if (ret)
> + return ret;
> + return len;
> + case IEI_WT61P803_PUZZLE_MAC_ADDRESS:
> + if (len != (size_t)(IEI_WT61P803_PUZZLE_MAC_LENGTH + 1))
> + return -EINVAL;
> +
> + memcpy(mac_address, buf, sizeof(mac_address));
> +
> + if (strlen(attr->attr.name) != 13)
> + return -EIO;
> +
> + ret = iei_wt61p803_puzzle_write_mac_address(mcu, mac_address, pattr->index);
> + if (ret)
> + return ret;
> + return len;
> + case IEI_WT61P803_PUZZLE_POWER_LOSS_RECOVERY:
> + ret = kstrtoint(buf, 10, &power_loss_recovery_action);
> + if (ret)
> + return ret;
> + ret = iei_wt61p803_puzzle_write_power_loss_recovery(mcu,
> + power_loss_recovery_action);
> + if (ret)
> + return ret;
> + return len;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +#define IEI_WT61P803_PUZZLE_ATTR(_name, _mode, _show, _store, _type, _index) \
> + struct iei_wt61p803_puzzle_device_attribute dev_attr_##_name = \
> + { .dev_attr = __ATTR(_name, _mode, _show, _store), \
> + .type = _type, \
> + .index = _index }
Again, don't use your own attribute macro mess. Use the
DEVICE_ATTR_RW() ones instead please.
> +
> +#define IEI_WT61P803_PUZZLE_ATTR_RO(_name, _type, _id) \
> + IEI_WT61P803_PUZZLE_ATTR(_name, 0444, show_output, NULL, _type, _id)
DEVICE_ATTR_RO()
> +
> +#define IEI_WT61P803_PUZZLE_ATTR_RW(_name, _type, _id) \
> + IEI_WT61P803_PUZZLE_ATTR(_name, 0644, show_output, store_output, _type, _id)
DEVICE_ATTR_RW()
> +
> +static IEI_WT61P803_PUZZLE_ATTR_RO(version, IEI_WT61P803_PUZZLE_VERSION, 0);
> +static IEI_WT61P803_PUZZLE_ATTR_RO(build_info, IEI_WT61P803_PUZZLE_BUILD_INFO, 0);
> +static IEI_WT61P803_PUZZLE_ATTR_RO(bootloader_mode, IEI_WT61P803_PUZZLE_BOOTLOADER_MODE, 0);
> +static IEI_WT61P803_PUZZLE_ATTR_RO(protocol_version, IEI_WT61P803_PUZZLE_PROTOCOL_VERSION, 0);
> +static IEI_WT61P803_PUZZLE_ATTR_RW(serial_number, IEI_WT61P803_PUZZLE_SERIAL_NUMBER, 0);
> +static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_0, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 0);
> +static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_1, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 1);
> +static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_2, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 2);
> +static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_3, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 3);
> +static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_4, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 4);
> +static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_5, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 5);
> +static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_6, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 6);
> +static IEI_WT61P803_PUZZLE_ATTR_RW(mac_address_7, IEI_WT61P803_PUZZLE_MAC_ADDRESS, 7);
> +static IEI_WT61P803_PUZZLE_ATTR_RO(ac_recovery_status, IEI_WT61P803_PUZZLE_AC_RECOVERY_STATUS, 0);
> +static IEI_WT61P803_PUZZLE_ATTR_RW(power_loss_recovery, IEI_WT61P803_PUZZLE_POWER_LOSS_RECOVERY, 0);
> +static IEI_WT61P803_PUZZLE_ATTR_RO(power_status, IEI_WT61P803_PUZZLE_POWER_STATUS, 0);
> +
> +static struct attribute *iei_wt61p803_puzzle_attrs[] = {
> + &dev_attr_version.dev_attr.attr,
> + &dev_attr_build_info.dev_attr.attr,
> + &dev_attr_bootloader_mode.dev_attr.attr,
> + &dev_attr_protocol_version.dev_attr.attr,
> + &dev_attr_serial_number.dev_attr.attr,
> + &dev_attr_mac_address_0.dev_attr.attr,
> + &dev_attr_mac_address_1.dev_attr.attr,
> + &dev_attr_mac_address_2.dev_attr.attr,
> + &dev_attr_mac_address_3.dev_attr.attr,
> + &dev_attr_mac_address_4.dev_attr.attr,
> + &dev_attr_mac_address_5.dev_attr.attr,
> + &dev_attr_mac_address_6.dev_attr.attr,
> + &dev_attr_mac_address_7.dev_attr.attr,
> + &dev_attr_ac_recovery_status.dev_attr.attr,
> + &dev_attr_power_loss_recovery.dev_attr.attr,
> + &dev_attr_power_status.dev_attr.attr,
> + NULL
> +};
> +ATTRIBUTE_GROUPS(iei_wt61p803_puzzle);
> +
> +static int iei_wt61p803_puzzle_sysfs_create(struct device *dev,
> + struct iei_wt61p803_puzzle *mcu)
> +{
> + int ret;
> +
> + ret = sysfs_create_groups(&mcu->dev->kobj, iei_wt61p803_puzzle_groups);
You just raced with userspace and lost :(
Please set the default attribute group for your device in the driver and
then the driver core will properly bind and create everything correctly
when the driver binds to the device.
As it is, userspace just did not notice these attributes showing up :(
thanks,
greg k-h
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v9 2/7] drivers: mfd: Add a driver for IEI WT61P803 PUZZLE MCU
2021-09-22 10:54 ` Lee Jones
@ 2021-09-22 11:42 ` Greg KH
2021-09-22 13:04 ` Lee Jones
0 siblings, 1 reply; 16+ messages in thread
From: Greg KH @ 2021-09-22 11:42 UTC (permalink / raw)
To: Lee Jones
Cc: Luka Kovacic, linux-doc, linux-leds, devicetree, linux-hwmon,
linux-kernel, geert+renesas, Max.Merchel, linux, daniel,
shawnguo, sam, arnd, krzysztof.kozlowski, pavo.banicevic, corbet,
pavel, robh+dt, linux, jdelvare, goran.medic, luka.perkov,
robert.marko
On Wed, Sep 22, 2021 at 11:54:53AM +0100, Lee Jones wrote:
> Greg,
>
> Would you be kind enough to take a look at the SYS imp. please?
/me hands Lee some extra characters...
done.
greg k-h
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v9 2/7] drivers: mfd: Add a driver for IEI WT61P803 PUZZLE MCU
2021-09-22 11:41 ` Greg KH
@ 2021-09-22 12:18 ` Geert Uytterhoeven
2021-09-22 13:37 ` Greg KH
0 siblings, 1 reply; 16+ messages in thread
From: Geert Uytterhoeven @ 2021-09-22 12:18 UTC (permalink / raw)
To: Greg KH
Cc: Luka Kovacic, open list:DOCUMENTATION, linux-leds,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-hwmon, Linux Kernel Mailing List, Geert Uytterhoeven,
Max.Merchel, Oleksij Rempel, Daniel Palmer, Shawn Guo,
Sam Ravnborg, Arnd Bergmann, Krzysztof Kozlowski, pavo.banicevic,
Jonathan Corbet, Lee Jones, Pavel Machek, Rob Herring,
Guenter Roeck, Jean Delvare, goran.medic, luka.perkov,
robert.marko
Hi Greg,
On Wed, Sep 22, 2021 at 1:41 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Aug 24, 2021 at 02:44:33PM +0200, Luka Kovacic wrote:
> > +++ b/drivers/mfd/iei-wt61p803-puzzle.c
> > @@ -0,0 +1,908 @@
> > +// 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.
>
> It is 2021 now :(
IANAL, but if the driver was published first in 2020 (v1 of the patch),
"2020" should be fine.
It's not that Disney is allowed to increase the copyright year every time
Snowy White is played in a movie theatre ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v9 2/7] drivers: mfd: Add a driver for IEI WT61P803 PUZZLE MCU
2021-09-22 11:42 ` Greg KH
@ 2021-09-22 13:04 ` Lee Jones
0 siblings, 0 replies; 16+ messages in thread
From: Lee Jones @ 2021-09-22 13:04 UTC (permalink / raw)
To: Greg KH
Cc: Luka Kovacic, linux-doc, linux-leds, devicetree, linux-hwmon,
linux-kernel, geert+renesas, Max.Merchel, linux, daniel,
shawnguo, sam, arnd, krzysztof.kozlowski, pavo.banicevic, corbet,
pavel, robh+dt, linux, jdelvare, goran.medic, luka.perkov,
robert.marko
On Wed, 22 Sep 2021, Greg KH wrote:
> On Wed, Sep 22, 2021 at 11:54:53AM +0100, Lee Jones wrote:
> > Greg,
> >
> > Would you be kind enough to take a look at the SYS imp. please?
>
> /me hands Lee some extra characters...
Fingers faster than brain!
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v9 2/7] drivers: mfd: Add a driver for IEI WT61P803 PUZZLE MCU
2021-09-22 12:18 ` Geert Uytterhoeven
@ 2021-09-22 13:37 ` Greg KH
0 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2021-09-22 13:37 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Luka Kovacic, open list:DOCUMENTATION, linux-leds,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-hwmon, Linux Kernel Mailing List, Geert Uytterhoeven,
Max.Merchel, Oleksij Rempel, Daniel Palmer, Shawn Guo,
Sam Ravnborg, Arnd Bergmann, Krzysztof Kozlowski, pavo.banicevic,
Jonathan Corbet, Lee Jones, Pavel Machek, Rob Herring,
Guenter Roeck, Jean Delvare, goran.medic, luka.perkov,
robert.marko
On Wed, Sep 22, 2021 at 02:18:49PM +0200, Geert Uytterhoeven wrote:
> Hi Greg,
>
> On Wed, Sep 22, 2021 at 1:41 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Tue, Aug 24, 2021 at 02:44:33PM +0200, Luka Kovacic wrote:
> > > +++ b/drivers/mfd/iei-wt61p803-puzzle.c
> > > @@ -0,0 +1,908 @@
> > > +// 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.
> >
> > It is 2021 now :(
>
> IANAL, but if the driver was published first in 2020 (v1 of the patch),
> "2020" should be fine.
THis is a v9, the odds that this file has not been touched at all since
2020 is probably pretty low.
At the least, it better be changed after my review of it :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-09-22 13:37 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-24 12:44 [PATCH v9 0/7] Add support for the IEI WT61P803 PUZZLE MCU Luka Kovacic
2021-08-24 12:44 ` [PATCH v9 1/7] dt-bindings: Add IEI vendor prefix and IEI WT61P803 PUZZLE driver bindings Luka Kovacic
2021-08-24 18:09 ` Rob Herring
2021-08-24 12:44 ` [PATCH v9 2/7] drivers: mfd: Add a driver for IEI WT61P803 PUZZLE MCU Luka Kovacic
2021-09-22 10:54 ` Lee Jones
2021-09-22 11:42 ` Greg KH
2021-09-22 13:04 ` Lee Jones
2021-09-22 11:41 ` Greg KH
2021-09-22 12:18 ` Geert Uytterhoeven
2021-09-22 13:37 ` Greg KH
2021-08-24 12:44 ` [PATCH v9 3/7] drivers: hwmon: Add the IEI WT61P803 PUZZLE HWMON driver Luka Kovacic
2021-08-24 12:44 ` [PATCH v9 4/7] drivers: leds: Add the IEI WT61P803 PUZZLE LED driver Luka Kovacic
2021-08-24 12:44 ` [PATCH v9 5/7] Documentation/ABI: Add iei-wt61p803-puzzle driver sysfs interface documentation Luka Kovacic
2021-08-24 12:44 ` [PATCH v9 6/7] Documentation/hwmon: Add iei-wt61p803-puzzle hwmon driver documentation Luka Kovacic
2021-08-24 15:00 ` Guenter Roeck
2021-08-24 12:44 ` [PATCH v9 7/7] 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).