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@xxxxxxxxxxx>
> > Tested-by: Chen Wang <unicorn_wang@xxxxxxxxxxx>
> > Reviewed-by: Chen Wang <unicorn_wang@xxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxx>
> > +
> > +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@xxxxxxxxxxx>
> > + *
> > + * 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@xxxxxxxxxxx>");
> > +MODULE_DESCRIPTION("MCU I2C driver for SG2042 soc platform");
> > +MODULE_LICENSE("GPL");
