[PATCH v10 0/4] riscv: sophgo: Add SG2042 external hardware monitor support

[PATCH v10 0/4] riscv: sophgo: Add SG2042 external hardware monitor support

[Inochi Amaoto]

Add support for the onboard hardware monitor for SG2042.
Can be tested with OpenSBI v1.5.

The patch require the following i2c patch:
https://lore.kernel.org/all/IA1PR20MB4953DB82FB7D75BF8409FFF4BBB72@IA1PR20MB4953.namprd20.prod.outlook.com/

Changed from v9:
1. remove missed wrong return value.
2. add documentation for the other interface of the driver.
3. rename repower_action to repower_policy.
4. use break to unify "return 0" in is_visible function.

Changed from v8:
1. rename critical_action to repower_action, and add some comment
2. remove unnecessary wrap function
3. use guard macro to simplify mutex process.

Changed from v7:
1. add mutex protected and fix the return value when writing
"critical_action"

Changed from v6:
1. restore the driver name to sg2042-mcu
2. remove unnecessary wrap function and check in the driver.
3. add dts and config entry.

Changed from v5:
1. rename driver name to sgmcu as it will support more sophgo chip.
2. move some attr to debugfs.
3. add standard crit_hyst support
4. add documentation

Changed from v4:
1. use fix patch for binding ref.
2. use unevaluatedProperties instead of additionalProperties for binding

Changed from v3:
1. add thermal-sensor check.
2. change node type from syscon to hwmon

Changed from v2:
1. fix bindings id path.

Changed from v1:
1. Move patch from soc to hwmon.
2. Fix typo.

Inochi Amaoto (4):
  dt-bindings: hwmon: Add Sophgo SG2042 external hardware monitor
    support
  drivers: hwmon: sophgo: Add SG2042 external hardware monitor support
  riscv: dts: sophgo: Add mcu device for Milk-V Pioneer
  riscv: defconfig: Enable MCU support for SG2042

 .../hwmon/sophgo,sg2042-hwmon-mcu.yaml        |  43 ++
 Documentation/hwmon/index.rst                 |   1 +
 Documentation/hwmon/sg2042-mcu.rst            |  77 ++++
 .../boot/dts/sophgo/sg2042-milkv-pioneer.dts  |  60 +++
 arch/riscv/configs/defconfig                  |   1 +
 drivers/hwmon/Kconfig                         |  11 +
 drivers/hwmon/Makefile                        |   1 +
 drivers/hwmon/sg2042-mcu.c                    | 388 ++++++++++++++++++
 8 files changed, 582 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/sophgo,sg2042-hwmon-mcu.yaml
 create mode 100644 Documentation/hwmon/sg2042-mcu.rst
 create mode 100644 drivers/hwmon/sg2042-mcu.c


base-commit: d2d190621b60d77496d36659e1621b20c897b25d
prerequisite-patch-id: 5e7221ed1c63bb4d68e4a2a0141440e32d29e397
--
2.46.0

[PATCH v10 1/4] dt-bindings: hwmon: Add Sophgo SG2042 external hardware monitor support

[Inochi Amaoto]

Due to the design, Sophgo SG2042 use an external MCU to provide
hardware information, thermal information and reset control.

Add bindings for this monitor device.

Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Tested-by: Chen Wang <unicorn_wang@outlook.com>
Reviewed-by: Chen Wang <unicorn_wang@outlook.com>
---
 .../hwmon/sophgo,sg2042-hwmon-mcu.yaml        | 43 +++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/sophgo,sg2042-hwmon-mcu.yaml

diff --git a/Documentation/devicetree/bindings/hwmon/sophgo,sg2042-hwmon-mcu.yaml b/Documentation/devicetree/bindings/hwmon/sophgo,sg2042-hwmon-mcu.yaml
new file mode 100644
index 000000000000..f0667ac41d75
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/sophgo,sg2042-hwmon-mcu.yaml
@@ -0,0 +1,43 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/sophgo,sg2042-hwmon-mcu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo SG2042 onboard MCU support
+
+maintainers:
+  - Inochi Amaoto <inochiama@outlook.com>
+
+properties:
+  compatible:
+    const: sophgo,sg2042-hwmon-mcu
+
+  reg:
+    maxItems: 1
+
+  "#thermal-sensor-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - "#thermal-sensor-cells"
+
+allOf:
+  - $ref: /schemas/thermal/thermal-sensor.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        hwmon@17 {
+            compatible = "sophgo,sg2042-hwmon-mcu";
+            reg = <0x17>;
+            #thermal-sensor-cells = <1>;
+        };
+    };
-- 
2.46.0

[PATCH v10 2/4] drivers: hwmon: sophgo: Add SG2042 external hardware monitor support

[Inochi Amaoto]

SG2042 use an external MCU to provide basic hardware information
and thermal sensors.

Add driver support for the onboard MCU of SG2042.

Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
---
 Documentation/hwmon/index.rst      |   1 +
 Documentation/hwmon/sg2042-mcu.rst |  77 ++++++
 drivers/hwmon/Kconfig              |  11 +
 drivers/hwmon/Makefile             |   1 +
 drivers/hwmon/sg2042-mcu.c         | 388 +++++++++++++++++++++++++++++
 5 files changed, 478 insertions(+)
 create mode 100644 Documentation/hwmon/sg2042-mcu.rst
 create mode 100644 drivers/hwmon/sg2042-mcu.c

diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index 913c11390a45..ea3b5be8fe4f 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -206,6 +206,7 @@ Hardware Monitoring Kernel Drivers
    sch5636
    scpi-hwmon
    sfctemp
+   sg2042-mcu
    sht15
    sht21
    sht3x
diff --git a/Documentation/hwmon/sg2042-mcu.rst b/Documentation/hwmon/sg2042-mcu.rst
new file mode 100644
index 000000000000..18a3578ac213
--- /dev/null
+++ b/Documentation/hwmon/sg2042-mcu.rst
@@ -0,0 +1,77 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+Kernel driver sg2042-mcu
+========================
+
+Supported chips:
+
+  * Onboard MCU for sg2042
+
+    Addresses scanned: -
+
+    Prefix: 'sg2042-mcu'
+
+Authors:
+
+  - Inochi Amaoto <inochiama@outlook.com>
+
+Description
+-----------
+
+This driver supprts hardware monitoring for onboard MCU with
+i2c interface.
+
+Usage Notes
+-----------
+
+This driver does not auto-detect devices. You will have to instantiate
+the devices explicitly.
+Please see Documentation/i2c/instantiating-devices.rst for details.
+
+Sysfs Attributes
+----------------
+
+The following table shows the standard entries support by the driver:
+
+================= =====================================================
+Name              Description
+================= =====================================================
+temp1_input       Measured temperature of SoC
+temp1_crit        Critical high temperature
+temp1_crit_hyst   hysteresis temperature restore from Critical
+temp2_input       Measured temperature of the base board
+================= =====================================================
+
+The following table shows the extra entries support by the platform:
+
+================= ======= =============================================
+Name              Perm    Description
+================= ======= =============================================
+reset_count       RO      Reset count of the SoC
+uptime            RO      Seconds after the MCU is powered
+reset_reason      RO      Reset reason for the last reset
+repower_policy    RW      Execution policy when triggering repower
+================= ======= =============================================
+
+``repower_policy``
+  The repower is triggered when the temperature of the SoC falls below
+  the hysteresis temperature after triggering a shutdown due to
+  reaching the critical temperature.
+  The valid value for this entry is "repower" or "keep". "keep" will
+  leave the SoC down when the triggering repower, and "repower" will
+  boot the SoC.
+
+Debugfs Interfaces
+------------------
+
+If debugfs is available, this driver exposes some hardware specific
+data in ``/sys/kernel/debug/sg2042-mcu/*/``.
+
+================= ======= =============================================
+Name              Format  Description
+================= ======= =============================================
+firmware_version  0x%02x  firmware version of the MCU
+pcb_version       0x%02x  version number of the base board
+board_type        0x%02x  identifiers for the base board
+mcu_type          %d      type of the MCU: 0 is STM32, 1 is GD32
+================= ======= =============================================
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index b60fe2e58ad6..7aa6c3f322e5 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2066,6 +2066,17 @@ config SENSORS_SFCTEMP
 	  This driver can also be built as a module.  If so, the module
 	  will be called sfctemp.
 
+config SENSORS_SG2042_MCU
+	tristate "Sophgo onboard MCU support"
+	depends on I2C
+	depends on ARCH_SOPHGO || COMPILE_TEST
+	help
+	  Support for onboard MCU of Sophgo SG2042 SoCs. This mcu provides
+	  power control and some basic information.
+
+	  This driver can be built as a module. If so, the module
+	  will be called sg2042-mcu.
+
 config SENSORS_SURFACE_FAN
 	tristate "Surface Fan Driver"
 	depends on SURFACE_AGGREGATOR
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index b1c7056c37db..0bbe812a67ae 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -194,6 +194,7 @@ obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o
 obj-$(CONFIG_SENSORS_SCH5627)	+= sch5627.o
 obj-$(CONFIG_SENSORS_SCH5636)	+= sch5636.o
 obj-$(CONFIG_SENSORS_SFCTEMP)	+= sfctemp.o
+obj-$(CONFIG_SENSORS_SG2042_MCU) += sg2042-mcu.o
 obj-$(CONFIG_SENSORS_SL28CPLD)	+= sl28cpld-hwmon.o
 obj-$(CONFIG_SENSORS_SHT15)	+= sht15.o
 obj-$(CONFIG_SENSORS_SHT21)	+= sht21.o
diff --git a/drivers/hwmon/sg2042-mcu.c b/drivers/hwmon/sg2042-mcu.c
new file mode 100644
index 000000000000..0a2e072511b6
--- /dev/null
+++ b/drivers/hwmon/sg2042-mcu.c
@@ -0,0 +1,388 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2024 Inochi Amaoto <inochiama@outlook.com>
+ *
+ * Sophgo power control mcu for SG2042
+ */
+
+#include <linux/cleanup.h>
+#include <linux/debugfs.h>
+#include <linux/err.h>
+#include <linux/hwmon.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+
+/* fixed MCU registers */
+#define REG_BOARD_TYPE				0x00
+#define REG_MCU_FIRMWARE_VERSION		0x01
+#define REG_PCB_VERSION				0x02
+#define REG_PWR_CTRL				0x03
+#define REG_SOC_TEMP				0x04
+#define REG_BOARD_TEMP				0x05
+#define REG_RST_COUNT				0x0a
+#define REG_UPTIME				0x0b
+#define REG_RESET_REASON			0x0d
+#define REG_MCU_TYPE				0x18
+#define REG_REPOWER_POLICY			0x65
+#define REG_CRITICAL_TEMP			0x66
+#define REG_REPOWER_TEMP			0x67
+
+#define REPOWER_POLICY_REBOOT			1
+#define REPOWER_POLICY_KEEP_OFF			2
+
+#define MCU_POWER_MAX				0xff
+
+#define DEFINE_MCU_DEBUG_ATTR(_name, _reg, _format)			\
+	static int _name##_show(struct seq_file *seqf,			\
+				    void *unused)			\
+	{								\
+		struct sg2042_mcu_data *mcu = seqf->private;		\
+		int ret;						\
+		ret = i2c_smbus_read_byte_data(mcu->client, (_reg));	\
+		if (ret < 0)						\
+			return ret;					\
+		seq_printf(seqf, _format "\n", ret);			\
+		return 0;						\
+	}								\
+	DEFINE_SHOW_ATTRIBUTE(_name)					\
+
+struct sg2042_mcu_data {
+	struct i2c_client	*client;
+	struct dentry		*debugfs;
+	struct mutex		mutex;
+};
+
+static struct dentry *sgmcu_debugfs;
+
+static ssize_t reset_count_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct sg2042_mcu_data *mcu = dev_get_drvdata(dev);
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(mcu->client, REG_RST_COUNT);
+	if (ret < 0)
+		return ret;
+
+	return sprintf(buf, "%d\n", ret);
+}
+
+static ssize_t uptime_show(struct device *dev,
+			   struct device_attribute *attr,
+			   char *buf)
+{
+	struct sg2042_mcu_data *mcu = dev_get_drvdata(dev);
+	u8 time_val[2];
+	int ret;
+
+	ret = i2c_smbus_read_i2c_block_data(mcu->client, REG_UPTIME,
+					    sizeof(time_val), time_val);
+	if (ret < 0)
+		return ret;
+
+	return sprintf(buf, "%d\n",
+		       (time_val[0]) | (time_val[1] << 8));
+}
+
+static ssize_t reset_reason_show(struct device *dev,
+				 struct device_attribute *attr,
+				 char *buf)
+{
+	struct sg2042_mcu_data *mcu = dev_get_drvdata(dev);
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(mcu->client, REG_RESET_REASON);
+	if (ret < 0)
+		return ret;
+
+	return sprintf(buf, "0x%02x\n", ret);
+}
+
+static ssize_t repower_policy_show(struct device *dev,
+				   struct device_attribute *attr,
+				   char *buf)
+{
+	struct sg2042_mcu_data *mcu = dev_get_drvdata(dev);
+	int ret;
+	const char *action;
+
+	ret = i2c_smbus_read_byte_data(mcu->client, REG_REPOWER_POLICY);
+	if (ret < 0)
+		return ret;
+
+	if (ret == REPOWER_POLICY_REBOOT)
+		action = "repower";
+	else if (ret == REPOWER_POLICY_KEEP_OFF)
+		action = "keep";
+	else
+		action = "unknown";
+
+	return sprintf(buf, "%s\n", action);
+}
+
+static ssize_t repower_policy_store(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t count)
+{
+	struct sg2042_mcu_data *mcu = dev_get_drvdata(dev);
+	u8 value;
+	int ret;
+
+	if (sysfs_streq("repower", buf))
+		value = REPOWER_POLICY_REBOOT;
+	else if (sysfs_streq("keep", buf))
+		value = REPOWER_POLICY_KEEP_OFF;
+	else
+		return -EINVAL;
+
+	ret = i2c_smbus_write_byte_data(mcu->client,
+					REG_REPOWER_POLICY, value);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static DEVICE_ATTR_RO(reset_count);
+static DEVICE_ATTR_RO(uptime);
+static DEVICE_ATTR_RO(reset_reason);
+static DEVICE_ATTR_RW(repower_policy);
+
+DEFINE_MCU_DEBUG_ATTR(firmware_version, REG_MCU_FIRMWARE_VERSION, "0x%02x");
+DEFINE_MCU_DEBUG_ATTR(pcb_version, REG_PCB_VERSION, "0x%02x");
+DEFINE_MCU_DEBUG_ATTR(board_type, REG_BOARD_TYPE, "0x%02x");
+DEFINE_MCU_DEBUG_ATTR(mcu_type, REG_MCU_TYPE, "%d");
+
+static struct attribute *sg2042_mcu_attrs[] = {
+	&dev_attr_reset_count.attr,
+	&dev_attr_uptime.attr,
+	&dev_attr_reset_reason.attr,
+	&dev_attr_repower_policy.attr,
+	NULL
+};
+
+static const struct attribute_group sg2042_mcu_attr_group = {
+	.attrs	= sg2042_mcu_attrs,
+};
+
+static const struct attribute_group *sg2042_mcu_groups[] = {
+	&sg2042_mcu_attr_group,
+	NULL
+};
+
+static const struct hwmon_channel_info * const sg2042_mcu_info[] = {
+	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
+	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_CRIT |
+					HWMON_T_CRIT_HYST,
+				 HWMON_T_INPUT),
+	NULL
+};
+
+static int sg2042_mcu_read(struct device *dev,
+			   enum hwmon_sensor_types type,
+			   u32 attr, int channel, long *val)
+{
+	struct sg2042_mcu_data *mcu = dev_get_drvdata(dev);
+	int tmp;
+	u8 reg;
+
+	switch (attr) {
+	case hwmon_temp_input:
+		reg = channel ? REG_BOARD_TEMP : REG_SOC_TEMP;
+		break;
+	case hwmon_temp_crit:
+		reg = REG_CRITICAL_TEMP;
+		break;
+	case hwmon_temp_crit_hyst:
+		reg = REG_REPOWER_TEMP;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	tmp = i2c_smbus_read_byte_data(mcu->client, reg);
+	if (tmp < 0)
+		return tmp;
+	*val = tmp * 1000;
+
+	return 0;
+}
+
+static int sg2042_mcu_write(struct device *dev,
+			    enum hwmon_sensor_types type,
+			    u32 attr, int channel, long val)
+{
+	struct sg2042_mcu_data *mcu = dev_get_drvdata(dev);
+	int temp = val / 1000;
+	int hyst_temp, crit_temp;
+	u8 reg;
+
+	temp = clamp_val(temp, 0, MCU_POWER_MAX);
+
+	guard(mutex)(&mcu->mutex);
+
+	switch (attr) {
+	case hwmon_temp_crit:
+		hyst_temp = i2c_smbus_read_byte_data(mcu->client,
+						     REG_REPOWER_TEMP);
+		if (hyst_temp < 0)
+			return hyst_temp;
+
+		crit_temp = temp;
+		reg = REG_CRITICAL_TEMP;
+		break;
+	case hwmon_temp_crit_hyst:
+		crit_temp = i2c_smbus_read_byte_data(mcu->client,
+						     REG_CRITICAL_TEMP);
+		if (crit_temp < 0)
+			return crit_temp;
+
+		hyst_temp = temp;
+		reg = REG_REPOWER_TEMP;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	/*
+	 * ensure hyst_temp is smaller to avoid MCU from
+	 * keeping triggering repower event.
+	 */
+	if (crit_temp < hyst_temp)
+		return -EINVAL;
+
+	return i2c_smbus_write_byte_data(mcu->client, reg, temp);
+}
+
+static umode_t sg2042_mcu_is_visible(const void *_data,
+				     enum hwmon_sensor_types type,
+				     u32 attr, int channel)
+{
+	switch (type) {
+	case hwmon_temp:
+		switch (attr) {
+		case hwmon_temp_input:
+			return 0444;
+		case hwmon_temp_crit:
+		case hwmon_temp_crit_hyst:
+			if (channel == 0)
+				return 0664;
+			break;
+		default:
+			break;
+		}
+		break;
+	default:
+			break;
+	}
+	return 0;
+}
+
+static const struct hwmon_ops sg2042_mcu_ops = {
+	.is_visible = sg2042_mcu_is_visible,
+	.read = sg2042_mcu_read,
+	.write = sg2042_mcu_write,
+};
+
+static const struct hwmon_chip_info sg2042_mcu_chip_info = {
+	.ops = &sg2042_mcu_ops,
+	.info = sg2042_mcu_info,
+};
+
+static void sg2042_mcu_debugfs_init(struct sg2042_mcu_data *mcu,
+				    struct device *dev)
+{
+	mcu->debugfs = debugfs_create_dir(dev_name(dev), sgmcu_debugfs);
+
+	debugfs_create_file("firmware_version", 0444, mcu->debugfs,
+			    mcu, &firmware_version_fops);
+	debugfs_create_file("pcb_version", 0444, mcu->debugfs, mcu,
+			    &pcb_version_fops);
+	debugfs_create_file("mcu_type", 0444, mcu->debugfs, mcu,
+			    &mcu_type_fops);
+	debugfs_create_file("board_type", 0444, mcu->debugfs, mcu,
+			    &board_type_fops);
+}
+
+static int sg2042_mcu_i2c_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct sg2042_mcu_data *mcu;
+	struct device *hwmon_dev;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA |
+						I2C_FUNC_SMBUS_BLOCK_DATA))
+		return -ENODEV;
+
+	mcu = devm_kmalloc(dev, sizeof(*mcu), GFP_KERNEL);
+	if (!mcu)
+		return -ENOMEM;
+
+	mutex_init(&mcu->mutex);
+	mcu->client = client;
+
+	i2c_set_clientdata(client, mcu);
+
+	hwmon_dev = devm_hwmon_device_register_with_info(dev, "sg2042_mcu",
+							 mcu,
+							 &sg2042_mcu_chip_info,
+							 NULL);
+	if (IS_ERR(hwmon_dev))
+		return PTR_ERR(hwmon_dev);
+
+	sg2042_mcu_debugfs_init(mcu, dev);
+
+	return 0;
+}
+
+static void sg2042_mcu_i2c_remove(struct i2c_client *client)
+{
+	struct sg2042_mcu_data *mcu = i2c_get_clientdata(client);
+
+	debugfs_remove_recursive(mcu->debugfs);
+}
+
+static const struct i2c_device_id sg2042_mcu_id[] = {
+	{ "sg2042-hwmon-mcu", 0 },
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, sg2042_mcu_id);
+
+static const struct of_device_id sg2042_mcu_of_id[] = {
+	{ .compatible = "sophgo,sg2042-hwmon-mcu" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sg2042_mcu_of_id);
+
+static struct i2c_driver sg2042_mcu_driver = {
+	.driver = {
+		.name = "sg2042-mcu",
+		.of_match_table = sg2042_mcu_of_id,
+		.dev_groups = sg2042_mcu_groups,
+	},
+	.probe = sg2042_mcu_i2c_probe,
+	.remove = sg2042_mcu_i2c_remove,
+	.id_table = sg2042_mcu_id,
+};
+
+static int __init sg2042_mcu_init(void)
+{
+	sgmcu_debugfs = debugfs_create_dir("sg2042-mcu", NULL);
+	return i2c_add_driver(&sg2042_mcu_driver);
+}
+
+static void __exit sg2042_mcu_exit(void)
+{
+	debugfs_remove_recursive(sgmcu_debugfs);
+	i2c_del_driver(&sg2042_mcu_driver);
+}
+
+module_init(sg2042_mcu_init);
+module_exit(sg2042_mcu_exit);
+
+MODULE_AUTHOR("Inochi Amaoto <inochiama@outlook.com>");
+MODULE_DESCRIPTION("MCU I2C driver for SG2042 soc platform");
+MODULE_LICENSE("GPL");
-- 
2.46.0

[PATCH v10 3/4] riscv: dts: sophgo: Add mcu device for Milk-V Pioneer

[Inochi Amaoto]

Add mcu device and thermal zones node for Milk-V Pioneer.

Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
Tested-by: Chen Wang <unicorn_wang@outlook.com>
Reviewed-by: Chen Wang <unicorn_wang@outlook.com>
---
 .../boot/dts/sophgo/sg2042-milkv-pioneer.dts  | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts b/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
index 80cb017974d8..76375e25a495 100644
--- a/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
+++ b/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
@@ -26,6 +26,66 @@ &cgi_dpll1 {
 	clock-frequency = <25000000>;
 };
 
+&i2c1 {
+	status = "okay";
+
+	mcu: syscon@17 {
+		compatible = "sophgo,sg2042-hwmon-mcu";
+		reg = <0x17>;
+		#thermal-sensor-cells = <1>;
+	};
+};
+
 &uart0 {
 	status = "okay";
 };
+
+/ {
+	thermal-zones {
+		soc-thermal {
+			polling-delay-passive = <1000>;
+			polling-delay = <1000>;
+			thermal-sensors = <&mcu 0>;
+
+			trips {
+				soc_active1: soc-active1 {
+					temperature = <30000>;
+					hysteresis = <8000>;
+					type = "active";
+				};
+
+				soc_active2: soc-active2 {
+					temperature = <58000>;
+					hysteresis = <12000>;
+					type = "active";
+				};
+
+				soc_active3: soc-active3 {
+					temperature = <70000>;
+					hysteresis = <10000>;
+					type = "active";
+				};
+
+				soc_hot: soc-hot {
+					temperature = <80000>;
+					hysteresis = <5000>;
+					type = "hot";
+				};
+			};
+		};
+
+		board-thermal {
+			polling-delay-passive = <1000>;
+			polling-delay = <1000>;
+			thermal-sensors = <&mcu 1>;
+
+			trips {
+				board_active: board-active {
+					temperature = <75000>;
+					hysteresis = <8000>;
+					type = "active";
+				};
+			};
+		};
+	};
+};
-- 
2.46.0

[PATCH v10 4/4] riscv: defconfig: Enable MCU support for SG2042

[Inochi Amaoto]

Enable MCU driver for SG2042 to provide thermal and reboot support.

Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
Tested-by: Chen Wang <unicorn_wang@outlook.com>
Reviewed-by: Chen Wang <unicorn_wang@outlook.com>
---
 arch/riscv/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
index 0d678325444f..a0f346301df6 100644
--- a/arch/riscv/configs/defconfig
+++ b/arch/riscv/configs/defconfig
@@ -170,6 +170,7 @@ CONFIG_SPI_SUN6I=y
 CONFIG_GPIO_SIFIVE=y
 CONFIG_POWER_RESET_GPIO_RESTART=y
 CONFIG_SENSORS_SFCTEMP=m
+CONFIG_SENSORS_SG2042_MCU=y
 CONFIG_CPU_THERMAL=y
 CONFIG_DEVFREQ_THERMAL=y
 CONFIG_RZG2L_THERMAL=y
-- 
2.46.0

Re: [PATCH v10 2/4] drivers: hwmon: sophgo: Add SG2042 external hardware monitor support

[Chen Wang]

On 2024/8/10 16:03, Inochi Amaoto wrote:
> SG2042 use an external MCU to provide basic hardware information
> and thermal sensors.
>
> Add driver support for the onboard MCU of SG2042.
>
> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>

Tested-by: Chen Wang <unicorn_wang@outlook.com>

Reviewed-by: Chen Wang <unicorn_wang@outlook.com>

> ---
>   Documentation/hwmon/index.rst      |   1 +
>   Documentation/hwmon/sg2042-mcu.rst |  77 ++++++
>   drivers/hwmon/Kconfig              |  11 +
>   drivers/hwmon/Makefile             |   1 +
>   drivers/hwmon/sg2042-mcu.c         | 388 +++++++++++++++++++++++++++++
>   5 files changed, 478 insertions(+)
>   create mode 100644 Documentation/hwmon/sg2042-mcu.rst
>   create mode 100644 drivers/hwmon/sg2042-mcu.c

[......]

Re: [PATCH v10 0/4] riscv: sophgo: Add SG2042 external hardware monitor support

[Chen Wang]

On 2024/8/10 16:03, Inochi Amaoto wrote:
> Add support for the onboard hardware monitor for SG2042.
> Can be tested with OpenSBI v1.5.
>
> The patch require the following i2c patch:
> https://lore.kernel.org/all/IA1PR20MB4953DB82FB7D75BF8409FFF4BBB72@IA1PR20MB4953.namprd20.prod.outlook.com/
>
> Changed from v9:
> 1. remove missed wrong return value.
> 2. add documentation for the other interface of the driver.
> 3. rename repower_action to repower_policy.
> 4. use break to unify "return 0" in is_visible function.
>
> Changed from v8:
> 1. rename critical_action to repower_action, and add some comment
> 2. remove unnecessary wrap function
> 3. use guard macro to simplify mutex process.
>
> Changed from v7:
> 1. add mutex protected and fix the return value when writing
> "critical_action"
>
> Changed from v6:
> 1. restore the driver name to sg2042-mcu
> 2. remove unnecessary wrap function and check in the driver.
> 3. add dts and config entry.
>
> Changed from v5:
> 1. rename driver name to sgmcu as it will support more sophgo chip.
> 2. move some attr to debugfs.
> 3. add standard crit_hyst support
> 4. add documentation
>
> Changed from v4:
> 1. use fix patch for binding ref.
> 2. use unevaluatedProperties instead of additionalProperties for binding
>
> Changed from v3:
> 1. add thermal-sensor check.
> 2. change node type from syscon to hwmon
>
> Changed from v2:
> 1. fix bindings id path.
>
> Changed from v1:
> 1. Move patch from soc to hwmon.
> 2. Fix typo.
>
> Inochi Amaoto (4):
>    dt-bindings: hwmon: Add Sophgo SG2042 external hardware monitor
>      support
>    drivers: hwmon: sophgo: Add SG2042 external hardware monitor support
>    riscv: dts: sophgo: Add mcu device for Milk-V Pioneer
>    riscv: defconfig: Enable MCU support for SG2042
>
>   .../hwmon/sophgo,sg2042-hwmon-mcu.yaml        |  43 ++
>   Documentation/hwmon/index.rst                 |   1 +
>   Documentation/hwmon/sg2042-mcu.rst            |  77 ++++
>   .../boot/dts/sophgo/sg2042-milkv-pioneer.dts  |  60 +++
>   arch/riscv/configs/defconfig                  |   1 +
>   drivers/hwmon/Kconfig                         |  11 +
>   drivers/hwmon/Makefile                        |   1 +
>   drivers/hwmon/sg2042-mcu.c                    | 388 ++++++++++++++++++
>   8 files changed, 582 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/hwmon/sophgo,sg2042-hwmon-mcu.yaml
>   create mode 100644 Documentation/hwmon/sg2042-mcu.rst
>   create mode 100644 drivers/hwmon/sg2042-mcu.c
>
>
> base-commit: d2d190621b60d77496d36659e1621b20c897b25d
> prerequisite-patch-id: 5e7221ed1c63bb4d68e4a2a0141440e32d29e397
In fact, I did not find these two commit ids on master or sophgo/for-next.
When testing, I applied this patchset based on the latest sophgo/for-next.
> --
> 2.46.0
>

Re: [PATCH v10 0/4] riscv: sophgo: Add SG2042 external hardware monitor support

[Inochi Amaoto]

On Tue, Aug 13, 2024 at 09:24:19AM GMT, Chen Wang wrote:
> 
> On 2024/8/10 16:03, Inochi Amaoto wrote:
> > Add support for the onboard hardware monitor for SG2042.
> > Can be tested with OpenSBI v1.5.
> > 
> > The patch require the following i2c patch:
> > https://lore.kernel.org/all/IA1PR20MB4953DB82FB7D75BF8409FFF4BBB72@IA1PR20MB4953.namprd20.prod.outlook.com/
> > 
> > Changed from v9:
> > 1. remove missed wrong return value.
> > 2. add documentation for the other interface of the driver.
> > 3. rename repower_action to repower_policy.
> > 4. use break to unify "return 0" in is_visible function.
> > 
> > Changed from v8:
> > 1. rename critical_action to repower_action, and add some comment
> > 2. remove unnecessary wrap function
> > 3. use guard macro to simplify mutex process.
> > 
> > Changed from v7:
> > 1. add mutex protected and fix the return value when writing
> > "critical_action"
> > 
> > Changed from v6:
> > 1. restore the driver name to sg2042-mcu
> > 2. remove unnecessary wrap function and check in the driver.
> > 3. add dts and config entry.
> > 
> > Changed from v5:
> > 1. rename driver name to sgmcu as it will support more sophgo chip.
> > 2. move some attr to debugfs.
> > 3. add standard crit_hyst support
> > 4. add documentation
> > 
> > Changed from v4:
> > 1. use fix patch for binding ref.
> > 2. use unevaluatedProperties instead of additionalProperties for binding
> > 
> > Changed from v3:
> > 1. add thermal-sensor check.
> > 2. change node type from syscon to hwmon
> > 
> > Changed from v2:
> > 1. fix bindings id path.
> > 
> > Changed from v1:
> > 1. Move patch from soc to hwmon.
> > 2. Fix typo.
> > 
> > Inochi Amaoto (4):
> >    dt-bindings: hwmon: Add Sophgo SG2042 external hardware monitor
> >      support
> >    drivers: hwmon: sophgo: Add SG2042 external hardware monitor support
> >    riscv: dts: sophgo: Add mcu device for Milk-V Pioneer
> >    riscv: defconfig: Enable MCU support for SG2042
> > 
> >   .../hwmon/sophgo,sg2042-hwmon-mcu.yaml        |  43 ++
> >   Documentation/hwmon/index.rst                 |   1 +
> >   Documentation/hwmon/sg2042-mcu.rst            |  77 ++++
> >   .../boot/dts/sophgo/sg2042-milkv-pioneer.dts  |  60 +++
> >   arch/riscv/configs/defconfig                  |   1 +
> >   drivers/hwmon/Kconfig                         |  11 +
> >   drivers/hwmon/Makefile                        |   1 +
> >   drivers/hwmon/sg2042-mcu.c                    | 388 ++++++++++++++++++
> >   8 files changed, 582 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/hwmon/sophgo,sg2042-hwmon-mcu.yaml
> >   create mode 100644 Documentation/hwmon/sg2042-mcu.rst
> >   create mode 100644 drivers/hwmon/sg2042-mcu.c
> > 
> > 
> > base-commit: d2d190621b60d77496d36659e1621b20c897b25d
> > prerequisite-patch-id: 5e7221ed1c63bb4d68e4a2a0141440e32d29e397
> In fact, I did not find these two commit ids on master or sophgo/for-next.
> When testing, I applied this patchset based on the latest sophgo/for-next.

This two id comes from my tree, not the sophgo one. But I
think it should be recognized by the git. For testing, you 
can just pick the sophgo/for-next tree and test it.

Regards,
Inochi.

Re: [PATCH v10 0/4] riscv: sophgo: Add SG2042 external hardware monitor support

[Chen Wang]

On 2024/8/13 9:53, Inochi Amaoto wrote:
> On Tue, Aug 13, 2024 at 09:24:19AM GMT, Chen Wang wrote:
>> On 2024/8/10 16:03, Inochi Amaoto wrote:
>>> Add support for the onboard hardware monitor for SG2042.
>>> Can be tested with OpenSBI v1.5.
>>>
>>> The patch require the following i2c patch:
>>> https://lore.kernel.org/all/IA1PR20MB4953DB82FB7D75BF8409FFF4BBB72@IA1PR20MB4953.namprd20.prod.outlook.com/
>>>
>>> Changed from v9:
>>> 1. remove missed wrong return value.
>>> 2. add documentation for the other interface of the driver.
>>> 3. rename repower_action to repower_policy.
>>> 4. use break to unify "return 0" in is_visible function.
>>>
>>> Changed from v8:
>>> 1. rename critical_action to repower_action, and add some comment
>>> 2. remove unnecessary wrap function
>>> 3. use guard macro to simplify mutex process.
>>>
>>> Changed from v7:
>>> 1. add mutex protected and fix the return value when writing
>>> "critical_action"
>>>
>>> Changed from v6:
>>> 1. restore the driver name to sg2042-mcu
>>> 2. remove unnecessary wrap function and check in the driver.
>>> 3. add dts and config entry.
>>>
>>> Changed from v5:
>>> 1. rename driver name to sgmcu as it will support more sophgo chip.
>>> 2. move some attr to debugfs.
>>> 3. add standard crit_hyst support
>>> 4. add documentation
>>>
>>> Changed from v4:
>>> 1. use fix patch for binding ref.
>>> 2. use unevaluatedProperties instead of additionalProperties for binding
>>>
>>> Changed from v3:
>>> 1. add thermal-sensor check.
>>> 2. change node type from syscon to hwmon
>>>
>>> Changed from v2:
>>> 1. fix bindings id path.
>>>
>>> Changed from v1:
>>> 1. Move patch from soc to hwmon.
>>> 2. Fix typo.
>>>
>>> Inochi Amaoto (4):
>>>     dt-bindings: hwmon: Add Sophgo SG2042 external hardware monitor
>>>       support
>>>     drivers: hwmon: sophgo: Add SG2042 external hardware monitor support
>>>     riscv: dts: sophgo: Add mcu device for Milk-V Pioneer
>>>     riscv: defconfig: Enable MCU support for SG2042
>>>
>>>    .../hwmon/sophgo,sg2042-hwmon-mcu.yaml        |  43 ++
>>>    Documentation/hwmon/index.rst                 |   1 +
>>>    Documentation/hwmon/sg2042-mcu.rst            |  77 ++++
>>>    .../boot/dts/sophgo/sg2042-milkv-pioneer.dts  |  60 +++
>>>    arch/riscv/configs/defconfig                  |   1 +
>>>    drivers/hwmon/Kconfig                         |  11 +
>>>    drivers/hwmon/Makefile                        |   1 +
>>>    drivers/hwmon/sg2042-mcu.c                    | 388 ++++++++++++++++++
>>>    8 files changed, 582 insertions(+)
>>>    create mode 100644 Documentation/devicetree/bindings/hwmon/sophgo,sg2042-hwmon-mcu.yaml
>>>    create mode 100644 Documentation/hwmon/sg2042-mcu.rst
>>>    create mode 100644 drivers/hwmon/sg2042-mcu.c
>>>
>>>
>>> base-commit: d2d190621b60d77496d36659e1621b20c897b25d
>>> prerequisite-patch-id: 5e7221ed1c63bb4d68e4a2a0141440e32d29e397
>> In fact, I did not find these two commit ids on master or sophgo/for-next.
>> When testing, I applied this patchset based on the latest sophgo/for-next.
> This two id comes from my tree, not the sophgo one. But I
> think it should be recognized by the git. For testing, you
> can just pick the sophgo/for-next tree and test it.
Don't use commit id from your local git, use public git so other people 
can find your base. Generally we use official label, such as 6.11-rc1.
>
> Regards,
> Inochi.

Re: [PATCH v10 2/4] drivers: hwmon: sophgo: Add SG2042 external hardware monitor support

[Guenter Roeck]

On Sat, Aug 10, 2024 at 04:03:51PM +0800, Inochi Amaoto wrote:
> SG2042 use an external MCU to provide basic hardware information
> and thermal sensors.
> 
> Add driver support for the onboard MCU of SG2042.
> 
> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
> Tested-by: Chen Wang <unicorn_wang@outlook.com>
> Reviewed-by: Chen Wang <unicorn_wang@outlook.com>
> ---
>  Documentation/hwmon/index.rst      |   1 +
>  Documentation/hwmon/sg2042-mcu.rst |  77 ++++++
>  drivers/hwmon/Kconfig              |  11 +
>  drivers/hwmon/Makefile             |   1 +
>  drivers/hwmon/sg2042-mcu.c         | 388 +++++++++++++++++++++++++++++
>  5 files changed, 478 insertions(+)
>  create mode 100644 Documentation/hwmon/sg2042-mcu.rst
>  create mode 100644 drivers/hwmon/sg2042-mcu.c
> 
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> index 913c11390a45..ea3b5be8fe4f 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -206,6 +206,7 @@ Hardware Monitoring Kernel Drivers
>     sch5636
>     scpi-hwmon
>     sfctemp
> +   sg2042-mcu
>     sht15
>     sht21
>     sht3x
> diff --git a/Documentation/hwmon/sg2042-mcu.rst b/Documentation/hwmon/sg2042-mcu.rst
> new file mode 100644
> index 000000000000..18a3578ac213
> --- /dev/null
> +++ b/Documentation/hwmon/sg2042-mcu.rst
> @@ -0,0 +1,77 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +Kernel driver sg2042-mcu
> +========================
> +
> +Supported chips:
> +
> +  * Onboard MCU for sg2042
> +
> +    Addresses scanned: -
> +
> +    Prefix: 'sg2042-mcu'
> +
> +Authors:
> +
> +  - Inochi Amaoto <inochiama@outlook.com>
> +
> +Description
> +-----------
> +
> +This driver supprts hardware monitoring for onboard MCU with
> +i2c interface.
> +
> +Usage Notes
> +-----------
> +
> +This driver does not auto-detect devices. You will have to instantiate
> +the devices explicitly.
> +Please see Documentation/i2c/instantiating-devices.rst for details.
> +
> +Sysfs Attributes
> +----------------
> +
> +The following table shows the standard entries support by the driver:
> +
> +================= =====================================================
> +Name              Description
> +================= =====================================================
> +temp1_input       Measured temperature of SoC
> +temp1_crit        Critical high temperature
> +temp1_crit_hyst   hysteresis temperature restore from Critical
> +temp2_input       Measured temperature of the base board
> +================= =====================================================
> +
> +The following table shows the extra entries support by the platform:
> +

Those are attached to the i2c device, which should be mentioned.
The terms "driver" and "platform" are really misleading. The attributes
above are hardware monitoring device attributes, the attributes below are
i2c device attributes. Both are implemented and thus supported by the
driver.

> +================= ======= =============================================
> +Name              Perm    Description
> +================= ======= =============================================
> +reset_count       RO      Reset count of the SoC
> +uptime            RO      Seconds after the MCU is powered
> +reset_reason      RO      Reset reason for the last reset
> +repower_policy    RW      Execution policy when triggering repower
> +================= ======= =============================================
> +
> +``repower_policy``
> +  The repower is triggered when the temperature of the SoC falls below
> +  the hysteresis temperature after triggering a shutdown due to
> +  reaching the critical temperature.
> +  The valid value for this entry is "repower" or "keep". "keep" will
> +  leave the SoC down when the triggering repower, and "repower" will
> +  boot the SoC.
> +
> +Debugfs Interfaces
> +------------------
> +
> +If debugfs is available, this driver exposes some hardware specific
> +data in ``/sys/kernel/debug/sg2042-mcu/*/``.
> +
> +================= ======= =============================================
> +Name              Format  Description
> +================= ======= =============================================
> +firmware_version  0x%02x  firmware version of the MCU
> +pcb_version       0x%02x  version number of the base board
> +board_type        0x%02x  identifiers for the base board
> +mcu_type          %d      type of the MCU: 0 is STM32, 1 is GD32
> +================= ======= =============================================
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index b60fe2e58ad6..7aa6c3f322e5 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -2066,6 +2066,17 @@ config SENSORS_SFCTEMP
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called sfctemp.
>  
> +config SENSORS_SG2042_MCU
> +	tristate "Sophgo onboard MCU support"
> +	depends on I2C
> +	depends on ARCH_SOPHGO || COMPILE_TEST
> +	help
> +	  Support for onboard MCU of Sophgo SG2042 SoCs. This mcu provides
> +	  power control and some basic information.
> +
> +	  This driver can be built as a module. If so, the module
> +	  will be called sg2042-mcu.
> +
>  config SENSORS_SURFACE_FAN
>  	tristate "Surface Fan Driver"
>  	depends on SURFACE_AGGREGATOR
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index b1c7056c37db..0bbe812a67ae 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -194,6 +194,7 @@ obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o
>  obj-$(CONFIG_SENSORS_SCH5627)	+= sch5627.o
>  obj-$(CONFIG_SENSORS_SCH5636)	+= sch5636.o
>  obj-$(CONFIG_SENSORS_SFCTEMP)	+= sfctemp.o
> +obj-$(CONFIG_SENSORS_SG2042_MCU) += sg2042-mcu.o
>  obj-$(CONFIG_SENSORS_SL28CPLD)	+= sl28cpld-hwmon.o
>  obj-$(CONFIG_SENSORS_SHT15)	+= sht15.o
>  obj-$(CONFIG_SENSORS_SHT21)	+= sht21.o
> diff --git a/drivers/hwmon/sg2042-mcu.c b/drivers/hwmon/sg2042-mcu.c
> new file mode 100644
> index 000000000000..0a2e072511b6
> --- /dev/null
> +++ b/drivers/hwmon/sg2042-mcu.c
> @@ -0,0 +1,388 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2024 Inochi Amaoto <inochiama@outlook.com>
> + *
> + * Sophgo power control mcu for SG2042
> + */
> +
> +#include <linux/cleanup.h>
> +#include <linux/debugfs.h>
> +#include <linux/err.h>
> +#include <linux/hwmon.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +
> +/* fixed MCU registers */
> +#define REG_BOARD_TYPE				0x00
> +#define REG_MCU_FIRMWARE_VERSION		0x01
> +#define REG_PCB_VERSION				0x02
> +#define REG_PWR_CTRL				0x03
> +#define REG_SOC_TEMP				0x04
> +#define REG_BOARD_TEMP				0x05
> +#define REG_RST_COUNT				0x0a
> +#define REG_UPTIME				0x0b
> +#define REG_RESET_REASON			0x0d
> +#define REG_MCU_TYPE				0x18
> +#define REG_REPOWER_POLICY			0x65
> +#define REG_CRITICAL_TEMP			0x66
> +#define REG_REPOWER_TEMP			0x67
> +
> +#define REPOWER_POLICY_REBOOT			1
> +#define REPOWER_POLICY_KEEP_OFF			2
> +
> +#define MCU_POWER_MAX				0xff
> +
> +#define DEFINE_MCU_DEBUG_ATTR(_name, _reg, _format)			\
> +	static int _name##_show(struct seq_file *seqf,			\
> +				    void *unused)			\
> +	{								\
> +		struct sg2042_mcu_data *mcu = seqf->private;		\
> +		int ret;						\
> +		ret = i2c_smbus_read_byte_data(mcu->client, (_reg));	\
> +		if (ret < 0)						\
> +			return ret;					\
> +		seq_printf(seqf, _format "\n", ret);			\
> +		return 0;						\
> +	}								\
> +	DEFINE_SHOW_ATTRIBUTE(_name)					\
> +
> +struct sg2042_mcu_data {
> +	struct i2c_client	*client;
> +	struct dentry		*debugfs;
> +	struct mutex		mutex;
> +};
> +
> +static struct dentry *sgmcu_debugfs;
> +
> +static ssize_t reset_count_show(struct device *dev,
> +				struct device_attribute *attr,
> +				char *buf)
> +{
> +	struct sg2042_mcu_data *mcu = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = i2c_smbus_read_byte_data(mcu->client, REG_RST_COUNT);
> +	if (ret < 0)
> +		return ret;
> +
> +	return sprintf(buf, "%d\n", ret);
> +}
> +
> +static ssize_t uptime_show(struct device *dev,
> +			   struct device_attribute *attr,
> +			   char *buf)
> +{
> +	struct sg2042_mcu_data *mcu = dev_get_drvdata(dev);
> +	u8 time_val[2];
> +	int ret;
> +
> +	ret = i2c_smbus_read_i2c_block_data(mcu->client, REG_UPTIME,
> +					    sizeof(time_val), time_val);
> +	if (ret < 0)
> +		return ret;
> +
> +	return sprintf(buf, "%d\n",
> +		       (time_val[0]) | (time_val[1] << 8));
> +}
> +
> +static ssize_t reset_reason_show(struct device *dev,
> +				 struct device_attribute *attr,
> +				 char *buf)
> +{
> +	struct sg2042_mcu_data *mcu = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = i2c_smbus_read_byte_data(mcu->client, REG_RESET_REASON);
> +	if (ret < 0)
> +		return ret;
> +
> +	return sprintf(buf, "0x%02x\n", ret);
> +}
> +
> +static ssize_t repower_policy_show(struct device *dev,
> +				   struct device_attribute *attr,
> +				   char *buf)
> +{
> +	struct sg2042_mcu_data *mcu = dev_get_drvdata(dev);
> +	int ret;
> +	const char *action;
> +
> +	ret = i2c_smbus_read_byte_data(mcu->client, REG_REPOWER_POLICY);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (ret == REPOWER_POLICY_REBOOT)
> +		action = "repower";
> +	else if (ret == REPOWER_POLICY_KEEP_OFF)
> +		action = "keep";
> +	else
> +		action = "unknown";
> +
> +	return sprintf(buf, "%s\n", action);
> +}
> +
> +static ssize_t repower_policy_store(struct device *dev,
> +				    struct device_attribute *attr,
> +				    const char *buf, size_t count)
> +{
> +	struct sg2042_mcu_data *mcu = dev_get_drvdata(dev);
> +	u8 value;
> +	int ret;
> +
> +	if (sysfs_streq("repower", buf))
> +		value = REPOWER_POLICY_REBOOT;
> +	else if (sysfs_streq("keep", buf))
> +		value = REPOWER_POLICY_KEEP_OFF;
> +	else
> +		return -EINVAL;
> +
> +	ret = i2c_smbus_write_byte_data(mcu->client,
> +					REG_REPOWER_POLICY, value);
> +	if (ret < 0)
> +		return ret;
> +
> +	return count;
> +}
> +
> +static DEVICE_ATTR_RO(reset_count);
> +static DEVICE_ATTR_RO(uptime);
> +static DEVICE_ATTR_RO(reset_reason);
> +static DEVICE_ATTR_RW(repower_policy);
> +
> +DEFINE_MCU_DEBUG_ATTR(firmware_version, REG_MCU_FIRMWARE_VERSION, "0x%02x");
> +DEFINE_MCU_DEBUG_ATTR(pcb_version, REG_PCB_VERSION, "0x%02x");
> +DEFINE_MCU_DEBUG_ATTR(board_type, REG_BOARD_TYPE, "0x%02x");
> +DEFINE_MCU_DEBUG_ATTR(mcu_type, REG_MCU_TYPE, "%d");
> +
> +static struct attribute *sg2042_mcu_attrs[] = {
> +	&dev_attr_reset_count.attr,
> +	&dev_attr_uptime.attr,
> +	&dev_attr_reset_reason.attr,
> +	&dev_attr_repower_policy.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group sg2042_mcu_attr_group = {
> +	.attrs	= sg2042_mcu_attrs,
> +};
> +
> +static const struct attribute_group *sg2042_mcu_groups[] = {
> +	&sg2042_mcu_attr_group,
> +	NULL
> +};
> +
> +static const struct hwmon_channel_info * const sg2042_mcu_info[] = {
> +	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
> +	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_CRIT |
> +					HWMON_T_CRIT_HYST,
> +				 HWMON_T_INPUT),
> +	NULL
> +};
> +
> +static int sg2042_mcu_read(struct device *dev,
> +			   enum hwmon_sensor_types type,
> +			   u32 attr, int channel, long *val)
> +{
> +	struct sg2042_mcu_data *mcu = dev_get_drvdata(dev);
> +	int tmp;
> +	u8 reg;
> +
> +	switch (attr) {
> +	case hwmon_temp_input:
> +		reg = channel ? REG_BOARD_TEMP : REG_SOC_TEMP;
> +		break;
> +	case hwmon_temp_crit:
> +		reg = REG_CRITICAL_TEMP;
> +		break;
> +	case hwmon_temp_crit_hyst:
> +		reg = REG_REPOWER_TEMP;
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	tmp = i2c_smbus_read_byte_data(mcu->client, reg);
> +	if (tmp < 0)
> +		return tmp;
> +	*val = tmp * 1000;
> +
> +	return 0;
> +}
> +
> +static int sg2042_mcu_write(struct device *dev,
> +			    enum hwmon_sensor_types type,
> +			    u32 attr, int channel, long val)
> +{
> +	struct sg2042_mcu_data *mcu = dev_get_drvdata(dev);
> +	int temp = val / 1000;
> +	int hyst_temp, crit_temp;
> +	u8 reg;
> +
> +	temp = clamp_val(temp, 0, MCU_POWER_MAX);
> +
> +	guard(mutex)(&mcu->mutex);
> +
> +	switch (attr) {
> +	case hwmon_temp_crit:
> +		hyst_temp = i2c_smbus_read_byte_data(mcu->client,
> +						     REG_REPOWER_TEMP);
> +		if (hyst_temp < 0)
> +			return hyst_temp;
> +
> +		crit_temp = temp;
> +		reg = REG_CRITICAL_TEMP;
> +		break;
> +	case hwmon_temp_crit_hyst:
> +		crit_temp = i2c_smbus_read_byte_data(mcu->client,
> +						     REG_CRITICAL_TEMP);
> +		if (crit_temp < 0)
> +			return crit_temp;
> +
> +		hyst_temp = temp;
> +		reg = REG_REPOWER_TEMP;
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	/*
> +	 * ensure hyst_temp is smaller to avoid MCU from
> +	 * keeping triggering repower event.
> +	 */
> +	if (crit_temp < hyst_temp)
> +		return -EINVAL;
> +
> +	return i2c_smbus_write_byte_data(mcu->client, reg, temp);
> +}
> +
> +static umode_t sg2042_mcu_is_visible(const void *_data,
> +				     enum hwmon_sensor_types type,
> +				     u32 attr, int channel)
> +{
> +	switch (type) {
> +	case hwmon_temp:
> +		switch (attr) {
> +		case hwmon_temp_input:
> +			return 0444;
> +		case hwmon_temp_crit:
> +		case hwmon_temp_crit_hyst:
> +			if (channel == 0)
> +				return 0664;

0644, please. Always. We don't permit group write access for sysfs
attributes.



> +			break;
> +		default:
> +			break;
> +		}
> +		break;
> +	default:
> +			break;
> +	}
> +	return 0;
> +}
> +
> +static const struct hwmon_ops sg2042_mcu_ops = {
> +	.is_visible = sg2042_mcu_is_visible,
> +	.read = sg2042_mcu_read,
> +	.write = sg2042_mcu_write,
> +};
> +
> +static const struct hwmon_chip_info sg2042_mcu_chip_info = {
> +	.ops = &sg2042_mcu_ops,
> +	.info = sg2042_mcu_info,
> +};
> +
> +static void sg2042_mcu_debugfs_init(struct sg2042_mcu_data *mcu,
> +				    struct device *dev)
> +{
> +	mcu->debugfs = debugfs_create_dir(dev_name(dev), sgmcu_debugfs);
> +
> +	debugfs_create_file("firmware_version", 0444, mcu->debugfs,
> +			    mcu, &firmware_version_fops);
> +	debugfs_create_file("pcb_version", 0444, mcu->debugfs, mcu,
> +			    &pcb_version_fops);
> +	debugfs_create_file("mcu_type", 0444, mcu->debugfs, mcu,
> +			    &mcu_type_fops);
> +	debugfs_create_file("board_type", 0444, mcu->debugfs, mcu,
> +			    &board_type_fops);
> +}
> +
> +static int sg2042_mcu_i2c_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct sg2042_mcu_data *mcu;
> +	struct device *hwmon_dev;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA |
> +						I2C_FUNC_SMBUS_BLOCK_DATA))
> +		return -ENODEV;
> +
> +	mcu = devm_kmalloc(dev, sizeof(*mcu), GFP_KERNEL);
> +	if (!mcu)
> +		return -ENOMEM;
> +
> +	mutex_init(&mcu->mutex);
> +	mcu->client = client;
> +
> +	i2c_set_clientdata(client, mcu);
> +
> +	hwmon_dev = devm_hwmon_device_register_with_info(dev, "sg2042_mcu",
> +							 mcu,
> +							 &sg2042_mcu_chip_info,
> +							 NULL);
> +	if (IS_ERR(hwmon_dev))
> +		return PTR_ERR(hwmon_dev);
> +
> +	sg2042_mcu_debugfs_init(mcu, dev);
> +
> +	return 0;
> +}
> +
> +static void sg2042_mcu_i2c_remove(struct i2c_client *client)
> +{
> +	struct sg2042_mcu_data *mcu = i2c_get_clientdata(client);
> +
> +	debugfs_remove_recursive(mcu->debugfs);
> +}
> +
> +static const struct i2c_device_id sg2042_mcu_id[] = {
> +	{ "sg2042-hwmon-mcu", 0 },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(i2c, sg2042_mcu_id);
> +
> +static const struct of_device_id sg2042_mcu_of_id[] = {
> +	{ .compatible = "sophgo,sg2042-hwmon-mcu" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, sg2042_mcu_of_id);
> +
> +static struct i2c_driver sg2042_mcu_driver = {
> +	.driver = {
> +		.name = "sg2042-mcu",
> +		.of_match_table = sg2042_mcu_of_id,
> +		.dev_groups = sg2042_mcu_groups,
> +	},
> +	.probe = sg2042_mcu_i2c_probe,
> +	.remove = sg2042_mcu_i2c_remove,
> +	.id_table = sg2042_mcu_id,
> +};
> +
> +static int __init sg2042_mcu_init(void)
> +{
> +	sgmcu_debugfs = debugfs_create_dir("sg2042-mcu", NULL);
> +	return i2c_add_driver(&sg2042_mcu_driver);
> +}
> +
> +static void __exit sg2042_mcu_exit(void)
> +{
> +	debugfs_remove_recursive(sgmcu_debugfs);
> +	i2c_del_driver(&sg2042_mcu_driver);
> +}
> +
> +module_init(sg2042_mcu_init);
> +module_exit(sg2042_mcu_exit);
> +
> +MODULE_AUTHOR("Inochi Amaoto <inochiama@outlook.com>");
> +MODULE_DESCRIPTION("MCU I2C driver for SG2042 soc platform");
> +MODULE_LICENSE("GPL");

Re: [PATCH v10 2/4] drivers: hwmon: sophgo: Add SG2042 external hardware monitor support

[Inochi Amaoto]

On Tue, Aug 13, 2024 at 11:27:11AM GMT, Guenter Roeck wrote:
> On Sat, Aug 10, 2024 at 04:03:51PM +0800, Inochi Amaoto wrote:
> > SG2042 use an external MCU to provide basic hardware information
> > and thermal sensors.
> > 
> > Add driver support for the onboard MCU of SG2042.
> > 
> > Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
> > Tested-by: Chen Wang <unicorn_wang@outlook.com>
> > Reviewed-by: Chen Wang <unicorn_wang@outlook.com>
> > ---
> >  Documentation/hwmon/index.rst      |   1 +
> >  Documentation/hwmon/sg2042-mcu.rst |  77 ++++++
> >  drivers/hwmon/Kconfig              |  11 +
> >  drivers/hwmon/Makefile             |   1 +
> >  drivers/hwmon/sg2042-mcu.c         | 388 +++++++++++++++++++++++++++++
> >  5 files changed, 478 insertions(+)
> >  create mode 100644 Documentation/hwmon/sg2042-mcu.rst
> >  create mode 100644 drivers/hwmon/sg2042-mcu.c
> > 
> > diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> > index 913c11390a45..ea3b5be8fe4f 100644
> > --- a/Documentation/hwmon/index.rst
> > +++ b/Documentation/hwmon/index.rst
> > @@ -206,6 +206,7 @@ Hardware Monitoring Kernel Drivers
> >     sch5636
> >     scpi-hwmon
> >     sfctemp
> > +   sg2042-mcu
> >     sht15
> >     sht21
> >     sht3x
> > diff --git a/Documentation/hwmon/sg2042-mcu.rst b/Documentation/hwmon/sg2042-mcu.rst
> > new file mode 100644
> > index 000000000000..18a3578ac213
> > --- /dev/null
> > +++ b/Documentation/hwmon/sg2042-mcu.rst
> > @@ -0,0 +1,77 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +Kernel driver sg2042-mcu
> > +========================
> > +
> > +Supported chips:
> > +
> > +  * Onboard MCU for sg2042
> > +
> > +    Addresses scanned: -
> > +
> > +    Prefix: 'sg2042-mcu'
> > +
> > +Authors:
> > +
> > +  - Inochi Amaoto <inochiama@outlook.com>
> > +
> > +Description
> > +-----------
> > +
> > +This driver supprts hardware monitoring for onboard MCU with
> > +i2c interface.
> > +
> > +Usage Notes
> > +-----------
> > +
> > +This driver does not auto-detect devices. You will have to instantiate
> > +the devices explicitly.
> > +Please see Documentation/i2c/instantiating-devices.rst for details.
> > +
> > +Sysfs Attributes
> > +----------------
> > +
> > +The following table shows the standard entries support by the driver:
> > +
> > +================= =====================================================
> > +Name              Description
> > +================= =====================================================
> > +temp1_input       Measured temperature of SoC
> > +temp1_crit        Critical high temperature
> > +temp1_crit_hyst   hysteresis temperature restore from Critical
> > +temp2_input       Measured temperature of the base board
> > +================= =====================================================
> > +
> > +The following table shows the extra entries support by the platform:
> > +
> 
> Those are attached to the i2c device, which should be mentioned.

I have mentioned this in the Description. Does this need to be 
mentioned here again?

> The terms "driver" and "platform" are really misleading. The attributes
> above are hardware monitoring device attributes, the attributes below are
> i2c device attributes. Both are implemented and thus supported by the
> driver.
> 

Thanks.

> > +================= ======= =============================================
> > +Name              Perm    Description
> > +================= ======= =============================================
> > +reset_count       RO      Reset count of the SoC
> > +uptime            RO      Seconds after the MCU is powered
> > +reset_reason      RO      Reset reason for the last reset
> > +repower_policy    RW      Execution policy when triggering repower
> > +================= ======= =============================================
> > +
> > +``repower_policy``
> > +  The repower is triggered when the temperature of the SoC falls below
> > +  the hysteresis temperature after triggering a shutdown due to
> > +  reaching the critical temperature.
> > +  The valid value for this entry is "repower" or "keep". "keep" will
> > +  leave the SoC down when the triggering repower, and "repower" will
> > +  boot the SoC.
> > +
> > +Debugfs Interfaces
> > +------------------
> > +
> > +If debugfs is available, this driver exposes some hardware specific
> > +data in ``/sys/kernel/debug/sg2042-mcu/*/``.
> > +
> > +================= ======= =============================================
> > +Name              Format  Description
> > +================= ======= =============================================
> > +firmware_version  0x%02x  firmware version of the MCU
> > +pcb_version       0x%02x  version number of the base board
> > +board_type        0x%02x  identifiers for the base board
> > +mcu_type          %d      type of the MCU: 0 is STM32, 1 is GD32
> > +================= ======= =============================================
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index b60fe2e58ad6..7aa6c3f322e5 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -2066,6 +2066,17 @@ config SENSORS_SFCTEMP
> >  	  This driver can also be built as a module.  If so, the module
> >  	  will be called sfctemp.
> >  
> > +config SENSORS_SG2042_MCU
> > +	tristate "Sophgo onboard MCU support"
> > +	depends on I2C
> > +	depends on ARCH_SOPHGO || COMPILE_TEST
> > +	help
> > +	  Support for onboard MCU of Sophgo SG2042 SoCs. This mcu provides
> > +	  power control and some basic information.
> > +
> > +	  This driver can be built as a module. If so, the module
> > +	  will be called sg2042-mcu.
> > +
> >  config SENSORS_SURFACE_FAN
> >  	tristate "Surface Fan Driver"
> >  	depends on SURFACE_AGGREGATOR
> > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> > index b1c7056c37db..0bbe812a67ae 100644
> > --- a/drivers/hwmon/Makefile
> > +++ b/drivers/hwmon/Makefile
> > @@ -194,6 +194,7 @@ obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o
> >  obj-$(CONFIG_SENSORS_SCH5627)	+= sch5627.o
> >  obj-$(CONFIG_SENSORS_SCH5636)	+= sch5636.o
> >  obj-$(CONFIG_SENSORS_SFCTEMP)	+= sfctemp.o
> > +obj-$(CONFIG_SENSORS_SG2042_MCU) += sg2042-mcu.o
> >  obj-$(CONFIG_SENSORS_SL28CPLD)	+= sl28cpld-hwmon.o
> >  obj-$(CONFIG_SENSORS_SHT15)	+= sht15.o
> >  obj-$(CONFIG_SENSORS_SHT21)	+= sht21.o
> > diff --git a/drivers/hwmon/sg2042-mcu.c b/drivers/hwmon/sg2042-mcu.c
> > new file mode 100644
> > index 000000000000..0a2e072511b6
> > --- /dev/null
> > +++ b/drivers/hwmon/sg2042-mcu.c
> > @@ -0,0 +1,388 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2024 Inochi Amaoto <inochiama@outlook.com>
> > + *
> > + * Sophgo power control mcu for SG2042
> > + */
> > +
> > +#include <linux/cleanup.h>
> > +#include <linux/debugfs.h>
> > +#include <linux/err.h>
> > +#include <linux/hwmon.h>
> > +#include <linux/i2c.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +
> > +/* fixed MCU registers */
> > +#define REG_BOARD_TYPE				0x00
> > +#define REG_MCU_FIRMWARE_VERSION		0x01
> > +#define REG_PCB_VERSION				0x02
> > +#define REG_PWR_CTRL				0x03
> > +#define REG_SOC_TEMP				0x04
> > +#define REG_BOARD_TEMP				0x05
> > +#define REG_RST_COUNT				0x0a
> > +#define REG_UPTIME				0x0b
> > +#define REG_RESET_REASON			0x0d
> > +#define REG_MCU_TYPE				0x18
> > +#define REG_REPOWER_POLICY			0x65
> > +#define REG_CRITICAL_TEMP			0x66
> > +#define REG_REPOWER_TEMP			0x67
> > +
> > +#define REPOWER_POLICY_REBOOT			1
> > +#define REPOWER_POLICY_KEEP_OFF			2
> > +
> > +#define MCU_POWER_MAX				0xff
> > +
> > +#define DEFINE_MCU_DEBUG_ATTR(_name, _reg, _format)			\
> > +	static int _name##_show(struct seq_file *seqf,			\
> > +				    void *unused)			\
> > +	{								\
> > +		struct sg2042_mcu_data *mcu = seqf->private;		\
> > +		int ret;						\
> > +		ret = i2c_smbus_read_byte_data(mcu->client, (_reg));	\
> > +		if (ret < 0)						\
> > +			return ret;					\
> > +		seq_printf(seqf, _format "\n", ret);			\
> > +		return 0;						\
> > +	}								\
> > +	DEFINE_SHOW_ATTRIBUTE(_name)					\
> > +
> > +struct sg2042_mcu_data {
> > +	struct i2c_client	*client;
> > +	struct dentry		*debugfs;
> > +	struct mutex		mutex;
> > +};
> > +
> > +static struct dentry *sgmcu_debugfs;
> > +
> > +static ssize_t reset_count_show(struct device *dev,
> > +				struct device_attribute *attr,
> > +				char *buf)
> > +{
> > +	struct sg2042_mcu_data *mcu = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	ret = i2c_smbus_read_byte_data(mcu->client, REG_RST_COUNT);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return sprintf(buf, "%d\n", ret);
> > +}
> > +
> > +static ssize_t uptime_show(struct device *dev,
> > +			   struct device_attribute *attr,
> > +			   char *buf)
> > +{
> > +	struct sg2042_mcu_data *mcu = dev_get_drvdata(dev);
> > +	u8 time_val[2];
> > +	int ret;
> > +
> > +	ret = i2c_smbus_read_i2c_block_data(mcu->client, REG_UPTIME,
> > +					    sizeof(time_val), time_val);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return sprintf(buf, "%d\n",
> > +		       (time_val[0]) | (time_val[1] << 8));
> > +}
> > +
> > +static ssize_t reset_reason_show(struct device *dev,
> > +				 struct device_attribute *attr,
> > +				 char *buf)
> > +{
> > +	struct sg2042_mcu_data *mcu = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	ret = i2c_smbus_read_byte_data(mcu->client, REG_RESET_REASON);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return sprintf(buf, "0x%02x\n", ret);
> > +}
> > +
> > +static ssize_t repower_policy_show(struct device *dev,
> > +				   struct device_attribute *attr,
> > +				   char *buf)
> > +{
> > +	struct sg2042_mcu_data *mcu = dev_get_drvdata(dev);
> > +	int ret;
> > +	const char *action;
> > +
> > +	ret = i2c_smbus_read_byte_data(mcu->client, REG_REPOWER_POLICY);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	if (ret == REPOWER_POLICY_REBOOT)
> > +		action = "repower";
> > +	else if (ret == REPOWER_POLICY_KEEP_OFF)
> > +		action = "keep";
> > +	else
> > +		action = "unknown";
> > +
> > +	return sprintf(buf, "%s\n", action);
> > +}
> > +
> > +static ssize_t repower_policy_store(struct device *dev,
> > +				    struct device_attribute *attr,
> > +				    const char *buf, size_t count)
> > +{
> > +	struct sg2042_mcu_data *mcu = dev_get_drvdata(dev);
> > +	u8 value;
> > +	int ret;
> > +
> > +	if (sysfs_streq("repower", buf))
> > +		value = REPOWER_POLICY_REBOOT;
> > +	else if (sysfs_streq("keep", buf))
> > +		value = REPOWER_POLICY_KEEP_OFF;
> > +	else
> > +		return -EINVAL;
> > +
> > +	ret = i2c_smbus_write_byte_data(mcu->client,
> > +					REG_REPOWER_POLICY, value);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return count;
> > +}
> > +
> > +static DEVICE_ATTR_RO(reset_count);
> > +static DEVICE_ATTR_RO(uptime);
> > +static DEVICE_ATTR_RO(reset_reason);
> > +static DEVICE_ATTR_RW(repower_policy);
> > +
> > +DEFINE_MCU_DEBUG_ATTR(firmware_version, REG_MCU_FIRMWARE_VERSION, "0x%02x");
> > +DEFINE_MCU_DEBUG_ATTR(pcb_version, REG_PCB_VERSION, "0x%02x");
> > +DEFINE_MCU_DEBUG_ATTR(board_type, REG_BOARD_TYPE, "0x%02x");
> > +DEFINE_MCU_DEBUG_ATTR(mcu_type, REG_MCU_TYPE, "%d");
> > +
> > +static struct attribute *sg2042_mcu_attrs[] = {
> > +	&dev_attr_reset_count.attr,
> > +	&dev_attr_uptime.attr,
> > +	&dev_attr_reset_reason.attr,
> > +	&dev_attr_repower_policy.attr,
> > +	NULL
> > +};
> > +
> > +static const struct attribute_group sg2042_mcu_attr_group = {
> > +	.attrs	= sg2042_mcu_attrs,
> > +};
> > +
> > +static const struct attribute_group *sg2042_mcu_groups[] = {
> > +	&sg2042_mcu_attr_group,
> > +	NULL
> > +};
> > +
> > +static const struct hwmon_channel_info * const sg2042_mcu_info[] = {
> > +	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
> > +	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_CRIT |
> > +					HWMON_T_CRIT_HYST,
> > +				 HWMON_T_INPUT),
> > +	NULL
> > +};
> > +
> > +static int sg2042_mcu_read(struct device *dev,
> > +			   enum hwmon_sensor_types type,
> > +			   u32 attr, int channel, long *val)
> > +{
> > +	struct sg2042_mcu_data *mcu = dev_get_drvdata(dev);
> > +	int tmp;
> > +	u8 reg;
> > +
> > +	switch (attr) {
> > +	case hwmon_temp_input:
> > +		reg = channel ? REG_BOARD_TEMP : REG_SOC_TEMP;
> > +		break;
> > +	case hwmon_temp_crit:
> > +		reg = REG_CRITICAL_TEMP;
> > +		break;
> > +	case hwmon_temp_crit_hyst:
> > +		reg = REG_REPOWER_TEMP;
> > +		break;
> > +	default:
> > +		return -EOPNOTSUPP;
> > +	}
> > +
> > +	tmp = i2c_smbus_read_byte_data(mcu->client, reg);
> > +	if (tmp < 0)
> > +		return tmp;
> > +	*val = tmp * 1000;
> > +
> > +	return 0;
> > +}
> > +
> > +static int sg2042_mcu_write(struct device *dev,
> > +			    enum hwmon_sensor_types type,
> > +			    u32 attr, int channel, long val)
> > +{
> > +	struct sg2042_mcu_data *mcu = dev_get_drvdata(dev);
> > +	int temp = val / 1000;
> > +	int hyst_temp, crit_temp;
> > +	u8 reg;
> > +
> > +	temp = clamp_val(temp, 0, MCU_POWER_MAX);
> > +
> > +	guard(mutex)(&mcu->mutex);
> > +
> > +	switch (attr) {
> > +	case hwmon_temp_crit:
> > +		hyst_temp = i2c_smbus_read_byte_data(mcu->client,
> > +						     REG_REPOWER_TEMP);
> > +		if (hyst_temp < 0)
> > +			return hyst_temp;
> > +
> > +		crit_temp = temp;
> > +		reg = REG_CRITICAL_TEMP;
> > +		break;
> > +	case hwmon_temp_crit_hyst:
> > +		crit_temp = i2c_smbus_read_byte_data(mcu->client,
> > +						     REG_CRITICAL_TEMP);
> > +		if (crit_temp < 0)
> > +			return crit_temp;
> > +
> > +		hyst_temp = temp;
> > +		reg = REG_REPOWER_TEMP;
> > +		break;
> > +	default:
> > +		return -EOPNOTSUPP;
> > +	}
> > +
> > +	/*
> > +	 * ensure hyst_temp is smaller to avoid MCU from
> > +	 * keeping triggering repower event.
> > +	 */
> > +	if (crit_temp < hyst_temp)
> > +		return -EINVAL;
> > +
> > +	return i2c_smbus_write_byte_data(mcu->client, reg, temp);
> > +}
> > +
> > +static umode_t sg2042_mcu_is_visible(const void *_data,
> > +				     enum hwmon_sensor_types type,
> > +				     u32 attr, int channel)
> > +{
> > +	switch (type) {
> > +	case hwmon_temp:
> > +		switch (attr) {
> > +		case hwmon_temp_input:
> > +			return 0444;
> > +		case hwmon_temp_crit:
> > +		case hwmon_temp_crit_hyst:
> > +			if (channel == 0)
> > +				return 0664;
> 
> 0644, please. Always. We don't permit group write access for sysfs
> attributes.
> 
> 
> 

Thank, it is my mistake.

> > +			break;
> > +		default:
> > +			break;
> > +		}
> > +		break;
> > +	default:
> > +			break;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static const struct hwmon_ops sg2042_mcu_ops = {
> > +	.is_visible = sg2042_mcu_is_visible,
> > +	.read = sg2042_mcu_read,
> > +	.write = sg2042_mcu_write,
> > +};
> > +
> > +static const struct hwmon_chip_info sg2042_mcu_chip_info = {
> > +	.ops = &sg2042_mcu_ops,
> > +	.info = sg2042_mcu_info,
> > +};
> > +
> > +static void sg2042_mcu_debugfs_init(struct sg2042_mcu_data *mcu,
> > +				    struct device *dev)
> > +{
> > +	mcu->debugfs = debugfs_create_dir(dev_name(dev), sgmcu_debugfs);
> > +
> > +	debugfs_create_file("firmware_version", 0444, mcu->debugfs,
> > +			    mcu, &firmware_version_fops);
> > +	debugfs_create_file("pcb_version", 0444, mcu->debugfs, mcu,
> > +			    &pcb_version_fops);
> > +	debugfs_create_file("mcu_type", 0444, mcu->debugfs, mcu,
> > +			    &mcu_type_fops);
> > +	debugfs_create_file("board_type", 0444, mcu->debugfs, mcu,
> > +			    &board_type_fops);
> > +}
> > +
> > +static int sg2042_mcu_i2c_probe(struct i2c_client *client)
> > +{
> > +	struct device *dev = &client->dev;
> > +	struct sg2042_mcu_data *mcu;
> > +	struct device *hwmon_dev;
> > +
> > +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA |
> > +						I2C_FUNC_SMBUS_BLOCK_DATA))
> > +		return -ENODEV;
> > +
> > +	mcu = devm_kmalloc(dev, sizeof(*mcu), GFP_KERNEL);
> > +	if (!mcu)
> > +		return -ENOMEM;
> > +
> > +	mutex_init(&mcu->mutex);
> > +	mcu->client = client;
> > +
> > +	i2c_set_clientdata(client, mcu);
> > +
> > +	hwmon_dev = devm_hwmon_device_register_with_info(dev, "sg2042_mcu",
> > +							 mcu,
> > +							 &sg2042_mcu_chip_info,
> > +							 NULL);
> > +	if (IS_ERR(hwmon_dev))
> > +		return PTR_ERR(hwmon_dev);
> > +
> > +	sg2042_mcu_debugfs_init(mcu, dev);
> > +
> > +	return 0;
> > +}
> > +
> > +static void sg2042_mcu_i2c_remove(struct i2c_client *client)
> > +{
> > +	struct sg2042_mcu_data *mcu = i2c_get_clientdata(client);
> > +
> > +	debugfs_remove_recursive(mcu->debugfs);
> > +}
> > +
> > +static const struct i2c_device_id sg2042_mcu_id[] = {
> > +	{ "sg2042-hwmon-mcu", 0 },
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(i2c, sg2042_mcu_id);
> > +
> > +static const struct of_device_id sg2042_mcu_of_id[] = {
> > +	{ .compatible = "sophgo,sg2042-hwmon-mcu" },
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, sg2042_mcu_of_id);
> > +
> > +static struct i2c_driver sg2042_mcu_driver = {
> > +	.driver = {
> > +		.name = "sg2042-mcu",
> > +		.of_match_table = sg2042_mcu_of_id,
> > +		.dev_groups = sg2042_mcu_groups,
> > +	},
> > +	.probe = sg2042_mcu_i2c_probe,
> > +	.remove = sg2042_mcu_i2c_remove,
> > +	.id_table = sg2042_mcu_id,
> > +};
> > +
> > +static int __init sg2042_mcu_init(void)
> > +{
> > +	sgmcu_debugfs = debugfs_create_dir("sg2042-mcu", NULL);
> > +	return i2c_add_driver(&sg2042_mcu_driver);
> > +}
> > +
> > +static void __exit sg2042_mcu_exit(void)
> > +{
> > +	debugfs_remove_recursive(sgmcu_debugfs);
> > +	i2c_del_driver(&sg2042_mcu_driver);
> > +}
> > +
> > +module_init(sg2042_mcu_init);
> > +module_exit(sg2042_mcu_exit);
> > +
> > +MODULE_AUTHOR("Inochi Amaoto <inochiama@outlook.com>");
> > +MODULE_DESCRIPTION("MCU I2C driver for SG2042 soc platform");
> > +MODULE_LICENSE("GPL");

Re: [PATCH v10 2/4] drivers: hwmon: sophgo: Add SG2042 external hardware monitor support

[Guenter Roeck]

On 8/13/24 17:40, Inochi Amaoto wrote:
> On Tue, Aug 13, 2024 at 11:27:11AM GMT, Guenter Roeck wrote:
>> On Sat, Aug 10, 2024 at 04:03:51PM +0800, Inochi Amaoto wrote:
>>> SG2042 use an external MCU to provide basic hardware information
>>> and thermal sensors.
>>>
>>> Add driver support for the onboard MCU of SG2042.
>>>
>>> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
>>> Tested-by: Chen Wang <unicorn_wang@outlook.com>
>>> Reviewed-by: Chen Wang <unicorn_wang@outlook.com>
>>> ---
>>>   Documentation/hwmon/index.rst      |   1 +
>>>   Documentation/hwmon/sg2042-mcu.rst |  77 ++++++
>>>   drivers/hwmon/Kconfig              |  11 +
>>>   drivers/hwmon/Makefile             |   1 +
>>>   drivers/hwmon/sg2042-mcu.c         | 388 +++++++++++++++++++++++++++++
>>>   5 files changed, 478 insertions(+)
>>>   create mode 100644 Documentation/hwmon/sg2042-mcu.rst
>>>   create mode 100644 drivers/hwmon/sg2042-mcu.c
>>>
>>> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
>>> index 913c11390a45..ea3b5be8fe4f 100644
>>> --- a/Documentation/hwmon/index.rst
>>> +++ b/Documentation/hwmon/index.rst
>>> @@ -206,6 +206,7 @@ Hardware Monitoring Kernel Drivers
>>>      sch5636
>>>      scpi-hwmon
>>>      sfctemp
>>> +   sg2042-mcu
>>>      sht15
>>>      sht21
>>>      sht3x
>>> diff --git a/Documentation/hwmon/sg2042-mcu.rst b/Documentation/hwmon/sg2042-mcu.rst
>>> new file mode 100644
>>> index 000000000000..18a3578ac213
>>> --- /dev/null
>>> +++ b/Documentation/hwmon/sg2042-mcu.rst
>>> @@ -0,0 +1,77 @@
>>> +.. SPDX-License-Identifier: GPL-2.0
>>> +
>>> +Kernel driver sg2042-mcu
>>> +========================
>>> +
>>> +Supported chips:
>>> +
>>> +  * Onboard MCU for sg2042
>>> +
>>> +    Addresses scanned: -
>>> +
>>> +    Prefix: 'sg2042-mcu'
>>> +
>>> +Authors:
>>> +
>>> +  - Inochi Amaoto <inochiama@outlook.com>
>>> +
>>> +Description
>>> +-----------
>>> +
>>> +This driver supprts hardware monitoring for onboard MCU with
>>> +i2c interface.
>>> +
>>> +Usage Notes
>>> +-----------
>>> +
>>> +This driver does not auto-detect devices. You will have to instantiate
>>> +the devices explicitly.
>>> +Please see Documentation/i2c/instantiating-devices.rst for details.
>>> +
>>> +Sysfs Attributes
>>> +----------------
>>> +
>>> +The following table shows the standard entries support by the driver:
>>> +
>>> +================= =====================================================
>>> +Name              Description
>>> +================= =====================================================
>>> +temp1_input       Measured temperature of SoC
>>> +temp1_crit        Critical high temperature
>>> +temp1_crit_hyst   hysteresis temperature restore from Critical
>>> +temp2_input       Measured temperature of the base board
>>> +================= =====================================================
>>> +
>>> +The following table shows the extra entries support by the platform:
>>> +
>>
>> Those are attached to the i2c device, which should be mentioned.
> 
> I have mentioned this in the Description. Does this need to be
> mentioned here again?
> 

Here is where it is needed.

Guenter

Re: [PATCH v10 2/4] drivers: hwmon: sophgo: Add SG2042 external hardware monitor support

[Inochi Amaoto]

On Tue, Aug 13, 2024 at 05:51:05PM GMT, Guenter Roeck wrote:
> On 8/13/24 17:40, Inochi Amaoto wrote:
> > On Tue, Aug 13, 2024 at 11:27:11AM GMT, Guenter Roeck wrote:
> > > On Sat, Aug 10, 2024 at 04:03:51PM +0800, Inochi Amaoto wrote:
> > > > SG2042 use an external MCU to provide basic hardware information
> > > > and thermal sensors.
> > > > 
> > > > Add driver support for the onboard MCU of SG2042.
> > > > 
> > > > Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
> > > > Tested-by: Chen Wang <unicorn_wang@outlook.com>
> > > > Reviewed-by: Chen Wang <unicorn_wang@outlook.com>
> > > > ---
> > > >   Documentation/hwmon/index.rst      |   1 +
> > > >   Documentation/hwmon/sg2042-mcu.rst |  77 ++++++
> > > >   drivers/hwmon/Kconfig              |  11 +
> > > >   drivers/hwmon/Makefile             |   1 +
> > > >   drivers/hwmon/sg2042-mcu.c         | 388 +++++++++++++++++++++++++++++
> > > >   5 files changed, 478 insertions(+)
> > > >   create mode 100644 Documentation/hwmon/sg2042-mcu.rst
> > > >   create mode 100644 drivers/hwmon/sg2042-mcu.c
> > > > 
> > > > diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> > > > index 913c11390a45..ea3b5be8fe4f 100644
> > > > --- a/Documentation/hwmon/index.rst
> > > > +++ b/Documentation/hwmon/index.rst
> > > > @@ -206,6 +206,7 @@ Hardware Monitoring Kernel Drivers
> > > >      sch5636
> > > >      scpi-hwmon
> > > >      sfctemp
> > > > +   sg2042-mcu
> > > >      sht15
> > > >      sht21
> > > >      sht3x
> > > > diff --git a/Documentation/hwmon/sg2042-mcu.rst b/Documentation/hwmon/sg2042-mcu.rst
> > > > new file mode 100644
> > > > index 000000000000..18a3578ac213
> > > > --- /dev/null
> > > > +++ b/Documentation/hwmon/sg2042-mcu.rst
> > > > @@ -0,0 +1,77 @@
> > > > +.. SPDX-License-Identifier: GPL-2.0
> > > > +
> > > > +Kernel driver sg2042-mcu
> > > > +========================
> > > > +
> > > > +Supported chips:
> > > > +
> > > > +  * Onboard MCU for sg2042
> > > > +
> > > > +    Addresses scanned: -
> > > > +
> > > > +    Prefix: 'sg2042-mcu'
> > > > +
> > > > +Authors:
> > > > +
> > > > +  - Inochi Amaoto <inochiama@outlook.com>
> > > > +
> > > > +Description
> > > > +-----------
> > > > +
> > > > +This driver supprts hardware monitoring for onboard MCU with
> > > > +i2c interface.
> > > > +
> > > > +Usage Notes
> > > > +-----------
> > > > +
> > > > +This driver does not auto-detect devices. You will have to instantiate
> > > > +the devices explicitly.
> > > > +Please see Documentation/i2c/instantiating-devices.rst for details.
> > > > +
> > > > +Sysfs Attributes
> > > > +----------------
> > > > +
> > > > +The following table shows the standard entries support by the driver:
> > > > +
> > > > +================= =====================================================
> > > > +Name              Description
> > > > +================= =====================================================
> > > > +temp1_input       Measured temperature of SoC
> > > > +temp1_crit        Critical high temperature
> > > > +temp1_crit_hyst   hysteresis temperature restore from Critical
> > > > +temp2_input       Measured temperature of the base board
> > > > +================= =====================================================
> > > > +
> > > > +The following table shows the extra entries support by the platform:
> > > > +
> > > 
> > > Those are attached to the i2c device, which should be mentioned.
> > 
> > I have mentioned this in the Description. Does this need to be
> > mentioned here again?
> > 
> 
> Here is where it is needed.
> 
> Guenter
> 

OK, thanks.

Regard,
Inochi