Thread (39 messages) 39 messages, 12 authors, 2024-12-26

Re: [PATCH v3 6/7] hwmon: Add Nuvoton NCT6694 HWMON support

From: Vincent Mailhol <hidden>
Date: 2024-12-12 16:20:04
Also in: linux-can, linux-gpio, linux-hwmon, linux-i2c, linux-rtc, linux-watchdog, lkml

On 10/12/2024 at 19:45, Ming Yu wrote:
quoted hunk ↗ jump to hunk
This driver supports Hardware monitor functionality for NCT6694 MFD
device based on USB interface.

Signed-off-by: Ming Yu <tmyu0@nuvoton.com>
---
 MAINTAINERS                   |   1 +
 drivers/hwmon/Kconfig         |  10 +
 drivers/hwmon/Makefile        |   1 +
 drivers/hwmon/nct6694-hwmon.c | 768 ++++++++++++++++++++++++++++++++++
 4 files changed, 780 insertions(+)
 create mode 100644 drivers/hwmon/nct6694-hwmon.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 496fe7d5a23f..d6414eea0463 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16546,6 +16546,7 @@ M:      Ming Yu <tmyu0@nuvoton.com>
 L:     linux-kernel@vger.kernel.org
 S:     Supported
 F:     drivers/gpio/gpio-nct6694.c
+F:     drivers/hwmon/nct6694-hwmon.c
 F:     drivers/i2c/busses/i2c-nct6694.c
 F:     drivers/mfd/nct6694.c
 F:     drivers/net/can/nct6694_canfd.c
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index dd376602f3f1..df40986424bd 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1636,6 +1636,16 @@ config SENSORS_NCT6683
          This driver can also be built as a module. If so, the module
          will be called nct6683.

+config SENSORS_NCT6694
+       tristate "Nuvoton NCT6694 Hardware Monitor support"
+       depends on MFD_NCT6694
+       help
+         Say Y here to support Nuvoton NCT6694 hardware monitoring
+         functionality.
+
+         This driver can also be built as a module. If so, the module
+         will be called nct6694-hwmon.
+
 config SENSORS_NCT6775_CORE
        tristate
        select REGMAP
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index b827b92f2a78..27a43e67cdb7 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -168,6 +168,7 @@ obj-$(CONFIG_SENSORS_MLXREG_FAN) += mlxreg-fan.o
 obj-$(CONFIG_SENSORS_MENF21BMC_HWMON) += menf21bmc_hwmon.o
 obj-$(CONFIG_SENSORS_MR75203)  += mr75203.o
 obj-$(CONFIG_SENSORS_NCT6683)  += nct6683.o
+obj-$(CONFIG_SENSORS_NCT6694)  += nct6694-hwmon.o
 obj-$(CONFIG_SENSORS_NCT6775_CORE) += nct6775-core.o
 nct6775-objs                   := nct6775-platform.o
 obj-$(CONFIG_SENSORS_NCT6775)  += nct6775.o
diff --git a/drivers/hwmon/nct6694-hwmon.c b/drivers/hwmon/nct6694-hwmon.c
new file mode 100644
index 000000000000..b2320d64090b
--- /dev/null
+++ b/drivers/hwmon/nct6694-hwmon.c
@@ -0,0 +1,768 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Nuvoton NCT6694 HWMON driver based on USB interface.
+ *
+ * Copyright (C) 2024 Nuvoton Technology Corp.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/hwmon.h>
+#include <linux/kernel.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/nct6694.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+/* Host interface */
+#define NCT6694_RPT_MOD                        0xFF
+#define NCT6694_HWMON_MOD              0x00
+#define NCT6694_PWM_MOD                        0x01
+
+/* Report Channel */
+#define NCT6694_VIN_IDX(x)             (0x00 + (x))
+#define NCT6694_TIN_IDX(x)                     \
+       ({ typeof(x) (_x) = (x);                \
+        ((_x) < 10) ? (0x10 + ((_x) * 2)) :    \
+        (0x30 + (((_x) - 10) * 2)); })
+#define NCT6694_FIN_IDX(x)             (0x50 + ((x) * 2))
+#define NCT6694_PWM_IDX(x)             (0x70 + (x))
+#define NCT6694_VIN_STS(x)             (0x68 + (x))
+#define NCT6694_TIN_STS(x)             (0x6A + (x))
+#define NCT6694_FIN_STS(x)             (0x6E + (x))
+
+/* Message Channel*/
+/* HWMON Command */
+/* Command 00h */
+#define NCT6694_HWMON_CMD0_LEN         0x40
+#define NCT6694_HWMON_CMD0_OFFSET      0x0000  /* OFFSET = SEL|CMD */
+#define NCT6694_VIN_EN(x)              (0x00 + (x))
+#define NCT6694_TIN_EN(x)              (0x02 + (x))
+#define NCT6694_FIN_EN(x)              (0x04 + (x))
+#define NCT6694_PWM_EN(x)              (0x06 + (x))
+#define NCT6694_PWM_FREQ_IDX(x)                (0x30 + (x))
+/* Command 02h */
+#define NCT6694_HWMON_CMD2_LEN         0x90
+#define NCT6694_HWMON_CMD2_OFFSET      0x0002  /* OFFSET = SEL|CMD */
+#define NCT6694_SMI_CTRL_IDX           0x00
+#define NCT6694_VIN_HL(x)              (0x10 + ((x) * 2))
+#define NCT6694_VIN_LL(x)              (0x11 + ((x) * 2))
+#define NCT6694_TIN_HYST(x)            (0x30 + ((x) * 2))
+#define NCT6694_TIN_HL(x)              (0x31 + ((x) * 2))
+#define NCT6694_FIN_HL(x)              (0x70 + ((x) * 2))
+#define NCT6694_FIN_LL(x)              (0x71 + ((x) * 2))
+/* PWM Command */
+#define NCT6694_PWM_CMD1_LEN           0x18
+#define NCT6694_PWM_CMD1_OFFSET                0x0001
+#define NCT6694_MAL_VAL(x)             (0x02 + (x))
+
+#define NCT6694_FREQ_FROM_REG(reg)     ((reg) * 25000 / 255)
+#define NCT6694_FREQ_TO_REG(val)       \
+       (DIV_ROUND_CLOSEST(clamp_val((val), 100, 25000) * 255, 25000))
+
+#define NCT6694_LSB_REG_MASK           GENMASK(7, 5)
+#define NCT6694_TIN_HYST_MASK          GENMASK(7, 5)
+
+static inline long in_from_reg(u8 reg)
+{
+       return reg * 16;
+}
+
+static inline u8 in_to_reg(long val)
+{
+       if (val <= 0)
+               return 0;
+       return val / 16;
+}
+
+static inline long temp_from_reg(u8 reg)
+{
+       return reg * 1000;
+}
+
+static inline u8 temp_to_reg(long val)
+{
+       return val / 1000;
+}
+
+struct nct6694_hwmon_data {
+       struct nct6694 *nct6694;
+       struct mutex lock;
+       unsigned char *xmit_buf;
+       unsigned char hwmon_en[NCT6694_HWMON_CMD0_LEN];
+};
A global comment on this series: do not declare your buffers as some
opaque unsigned char arrays. Instead, make it a structure (or an array
of structures if needed) using the little endian types for the
different fields.

You already applied this change to the CAN driver after I made a
comment, please do the same throughout the series.

The same applies with any other comments made by anyone else: do not
only apply to the patch where the comment is made, but apply it
broadly to the series.

Thank you.


Yours sincerely,
Vincent Mailhol
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help