On Sun, Apr 28, 2024 at 12:00:15AM GMT, Guenter Roeck wrote: > On 4/27/24 18:30, 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> > > Why does this driver reside in drivers/soc and not in drivers/hwmon ? > > NACK from my perspective. > > Guenter > In fact, I am pretty unsure which one is better. This driver also provides something rather than the hardware monitor. For example, this device also acts as a reset controller (now only used in SBI). Is it even better to move the driver into hwmon subsystem in this case? Regards, Inochi > > --- > > drivers/soc/Kconfig | 1 + > > drivers/soc/Makefile | 1 + > > drivers/soc/sophgo/Kconfig | 12 + > > drivers/soc/sophgo/Makefile | 5 + > > drivers/soc/sophgo/sg2042-hwmon-mcu.c | 531 ++++++++++++++++++++++++++ > > 5 files changed, 550 insertions(+) > > create mode 100644 drivers/soc/sophgo/Kconfig > > create mode 100644 drivers/soc/sophgo/Makefile > > create mode 100644 drivers/soc/sophgo/sg2042-hwmon-mcu.c > > > > diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig > > index 5d924e946507..19050f094996 100644 > > --- a/drivers/soc/Kconfig > > +++ b/drivers/soc/Kconfig > > @@ -22,6 +22,7 @@ source "drivers/soc/qcom/Kconfig" > > source "drivers/soc/renesas/Kconfig" > > source "drivers/soc/rockchip/Kconfig" > > source "drivers/soc/samsung/Kconfig" > > +source "drivers/soc/sophgo/Kconfig" > > source "drivers/soc/sunxi/Kconfig" > > source "drivers/soc/tegra/Kconfig" > > source "drivers/soc/ti/Kconfig" > > diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile > > index ba8f5b5460e1..6948e6617316 100644 > > --- a/drivers/soc/Makefile > > +++ b/drivers/soc/Makefile > > @@ -28,6 +28,7 @@ obj-y += qcom/ > > obj-y += renesas/ > > obj-y += rockchip/ > > obj-$(CONFIG_SOC_SAMSUNG) += samsung/ > > +obj-y += sophgo/ > > obj-y += sunxi/ > > obj-$(CONFIG_ARCH_TEGRA) += tegra/ > > obj-y += ti/ > > diff --git a/drivers/soc/sophgo/Kconfig b/drivers/soc/sophgo/Kconfig > > new file mode 100644 > > index 000000000000..de9842d1c287 > > --- /dev/null > > +++ b/drivers/soc/sophgo/Kconfig > > @@ -0,0 +1,12 @@ > > +# SPDX-License-Identifier: GPL-2.0-only > > +# > > +# Sophgo SoC drivers > > +# > > + > > +config SG2042_HWMON_MCU > > + tristate "SG2042 onboard MCU support" > > + depends on RISCV > > + help > > + Support for SG2042 onboard MCU. This mcu provides power > > + control and some basic information for SG2042 chip. > > + If unsure, say Y. > > diff --git a/drivers/soc/sophgo/Makefile b/drivers/soc/sophgo/Makefile > > new file mode 100644 > > index 000000000000..c72729ce61aa > > --- /dev/null > > +++ b/drivers/soc/sophgo/Makefile > > @@ -0,0 +1,5 @@ > > +# SPDX-License-Identifier: GPL-2.0-only > > +# > > +# Sophgo SoC drivers > > +# > > +obj-$(CONFIG_SG2042_HWMON_MCU) += sg2042-hwmon-mcu.o > > diff --git a/drivers/soc/sophgo/sg2042-hwmon-mcu.c b/drivers/soc/sophgo/sg2042-hwmon-mcu.c > > new file mode 100644 > > index 000000000000..3413b1a0f5e3 > > --- /dev/null > > +++ b/drivers/soc/sophgo/sg2042-hwmon-mcu.c > > @@ -0,0 +1,531 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Copyright (c) 2024 Inochi Amaoto <inochiama@xxxxxxxxxxx> > > + * > > + * Sophgo power control mcu for SG2042 > > + */ > > + > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/i2c.h> > > +#include <linux/err.h> > > +#include <linux/hwmon.h> > > +#include <linux/time.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_CRITICAL_ACTIONS 0x65 > > +#define REG_CRITICAL_TEMP 0x66 > > +#define REG_REPOWER_TEMP 0x67 > > + > > +#define CRITICAL_ACTION_REBOOT 0x1 > > +#define CRITICAL_ACTION_POWEROFF 0x2 > > + > > +#define DEFAULT_REPOWER_TEMP 60 > > +#define MAX_REPOWER_TEMP 100 > > + > > +#define sg2042_mcu_read_byte(client, reg) \ > > + i2c_smbus_read_byte_data(client, reg) > > +#define sg2042_mcu_write_byte(client, reg, value) \ > > + i2c_smbus_write_byte_data(client, reg, value) > > +#define sg2042_mcu_read_block(client, reg, array) \ > > + i2c_smbus_read_i2c_block_data(client, reg, sizeof(array), array) > > + > > +#define DEFINE_MCU_ATTR_READ_FUNC(_name, _type, _format) \ > > + static ssize_t _name##_show(struct device *dev, \ > > + struct device_attribute *attr, \ > > + char *buf) \ > > + { \ > > + struct sg2042_mcu_data *mcu = dev_get_drvdata(dev); \ > > + _type ret; \ > > + ret = sg2042_mcu_get_##_name(mcu->client); \ > > + if (ret < 0) \ > > + return ret; \ > > + return sprintf(buf, _format "\n", ret); \ > > + } > > + > > +struct sg2042_mcu_board_data { > > + u8 id; > > + const char *name; > > +}; > > + > > +struct sg2042_mcu_data { > > + struct i2c_client *client; > > + const struct sg2042_mcu_board_data *board_info; > > +}; > > + > > +static const struct sg2042_mcu_board_data sg2042_boards_data[] = { > > + { > > + .id = 0x80, > > + .name = "SG2042 evb x8", > > + }, > > + { > > + .id = 0x81, > > + .name = "SG2042R evb", > > + }, > > + { > > + .id = 0x83, > > + .name = "SG2042 evb x4", > > + }, > > + { > > + .id = 0x90, > > + .name = "Milk-V Pioneer", > > + }, > > +}; > > + > > +static const char *sg2042_mcu_reset_reason[8] = { > > + "Power supply overheat", > > + "Power supply failure", > > + "12V power supply failure", > > + "Reset commant", > > + "Unknown", > > + "Unknown", > > + "Unknown", > > + "SoC overheat", > > +}; > > + > > +static int sg2042_mcu_get_board_type(struct i2c_client *client) > > +{ > > + return sg2042_mcu_read_byte(client, REG_BOARD_TYPE); > > +} > > + > > +static int sg2042_mcu_get_firmware_version(struct i2c_client *client) > > +{ > > + return sg2042_mcu_read_byte(client, REG_MCU_FIRMWARE_VERSION); > > +} > > + > > +static int sg2042_mcu_get_pcb_version(struct i2c_client *client) > > +{ > > + return sg2042_mcu_read_byte(client, REG_PCB_VERSION); > > +} > > + > > +static int sg2042_mcu_get_soc_temp(struct i2c_client *client) > > +{ > > + return sg2042_mcu_read_byte(client, REG_SOC_TEMP); > > +} > > + > > +static int sg2042_mcu_get_board_temp(struct i2c_client *client) > > +{ > > + return sg2042_mcu_read_byte(client, REG_BOARD_TEMP); > > +} > > + > > +static int sg2042_mcu_get_reset_count(struct i2c_client *client) > > +{ > > + return sg2042_mcu_read_byte(client, REG_RST_COUNT); > > +} > > + > > +static s32 sg2042_mcu_get_uptime(struct i2c_client *client) > > +{ > > + int ret; > > + u8 time_val[2]; > > + > > + ret = sg2042_mcu_read_block(client, REG_UPTIME, time_val); > > + if (ret < 0) > > + return ret; > > + > > + return (s32)(time_val[0]) + ((s32)(time_val[1]) << 8); > > +} > > + > > +static int sg2042_mcu_get_reset_reason(struct i2c_client *client) > > +{ > > + return sg2042_mcu_read_byte(client, REG_RESET_REASON); > > +} > > + > > +static int sg2042_mcu_get_mcu_type(struct i2c_client *client) > > +{ > > + return sg2042_mcu_read_byte(client, REG_MCU_TYPE); > > +} > > + > > +static int sg2042_mcu_get_critical_action(struct i2c_client *client) > > +{ > > + return sg2042_mcu_read_byte(client, REG_CRITICAL_ACTIONS); > > +} > > + > > +static int sg2042_mcu_get_critical_temp(struct i2c_client *client) > > +{ > > + return sg2042_mcu_read_byte(client, REG_CRITICAL_TEMP); > > +} > > + > > +static int sg2042_mcu_get_repower_temp(struct i2c_client *client) > > +{ > > + return sg2042_mcu_read_byte(client, REG_REPOWER_TEMP); > > +} > > + > > +static int sg2042_mcu_set_critical_action(struct i2c_client *client, > > + u8 value) > > +{ > > + return sg2042_mcu_write_byte(client, REG_CRITICAL_ACTIONS, value); > > +} > > + > > +static int sg2042_mcu_set_critical_temp(struct i2c_client *client, > > + u8 value) > > +{ > > + return sg2042_mcu_write_byte(client, REG_CRITICAL_TEMP, value); > > +} > > + > > +static int sg2042_mcu_set_repower_temp(struct i2c_client *client, > > + u8 value) > > +{ > > + return sg2042_mcu_write_byte(client, REG_REPOWER_TEMP, value); > > +} > > + > > +static ssize_t board_type_show(struct device *dev, > > + struct device_attribute *attr, > > + char *buf) > > +{ > > + struct sg2042_mcu_data *mcu = dev_get_drvdata(dev); > > + > > + return sprintf(buf, "%s\n", mcu->board_info->name ?: "Unknown"); > > +} > > + > > +DEFINE_MCU_ATTR_READ_FUNC(firmware_version, int, "0x%02x"); > > +DEFINE_MCU_ATTR_READ_FUNC(pcb_version, int, "0x%02x"); > > +DEFINE_MCU_ATTR_READ_FUNC(reset_count, int, "%d"); > > +DEFINE_MCU_ATTR_READ_FUNC(uptime, s32, "%d"); > > + > > +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, val, i; > > + > > + val = sg2042_mcu_get_reset_reason(mcu->client); > > + if (val < 0) > > + return val; > > + > > + ret = sprintf(buf, "Reason: 0x%02x\n", val); > > + > > + for (i = 0; i < ARRAY_SIZE(sg2042_mcu_reset_reason); i++) { > > + if (val & BIT(i)) > > + ret += sprintf(buf + ret, "bit %d: %s\n", i, > > + sg2042_mcu_reset_reason[i]); > > + } > > + > > + return ret; > > +} > > + > > +static ssize_t mcu_type_show(struct device *dev, > > + struct device_attribute *attr, > > + char *buf) > > +{ > > + struct sg2042_mcu_data *mcu = dev_get_drvdata(dev); > > + int ret; > > + > > + ret = sg2042_mcu_get_mcu_type(mcu->client); > > + if (ret < 0) > > + return ret; > > + > > + return sprintf(buf, ret ? "GD32\n" : "STM32\n"); > > +} > > + > > +static ssize_t critical_action_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 = sg2042_mcu_get_critical_action(mcu->client); > > + if (ret < 0) > > + return ret; > > + > > + if (ret == CRITICAL_ACTION_REBOOT) > > + action = "reboot"; > > + else if (ret == CRITICAL_ACTION_POWEROFF) > > + action = "poweroff"; > > + else > > + action = "unknown"; > > + > > + return sprintf(buf, "%s\n", action); > > +} > > + > > +static ssize_t critical_action_store(struct device *dev, > > + struct device_attribute *attr, > > + const char *buf, size_t count) > > +{ > > + struct sg2042_mcu_data *mcu = dev_get_drvdata(dev); > > + int value; > > + > > + if (sysfs_streq("reboot", buf)) > > + value = CRITICAL_ACTION_REBOOT; > > + else if (sysfs_streq("poweroff", buf)) > > + value = CRITICAL_ACTION_POWEROFF; > > + else > > + return -EINVAL; > > + > > + return sg2042_mcu_set_critical_action(mcu->client, value); > > +} > > + > > +DEFINE_MCU_ATTR_READ_FUNC(repower_temp, u32, "%u"); > > + > > +static ssize_t repower_temp_store(struct device *dev, > > + struct device_attribute *attr, > > + const char *buf, size_t count) > > +{ > > + struct sg2042_mcu_data *mcu = dev_get_drvdata(dev); > > + u8 val; > > + int ret; > > + > > + ret = kstrtou8(buf, 10, &val); > > + if (ret < 0) > > + return ret; > > + > > + if (val == 0 || val > MAX_REPOWER_TEMP) > > + return -EINVAL; > > + > > + return sg2042_mcu_set_repower_temp(mcu->client, val); > > +} > > + > > +static DEVICE_ATTR_RO(board_type); > > +static DEVICE_ATTR_RO(firmware_version); > > +static DEVICE_ATTR_RO(pcb_version); > > +static DEVICE_ATTR_RO(reset_count); > > +static DEVICE_ATTR_RO(uptime); > > +static DEVICE_ATTR_RO(reset_reason); > > +static DEVICE_ATTR_RO(mcu_type); > > +static DEVICE_ATTR_RW(critical_action); > > +static DEVICE_ATTR_RW(repower_temp); > > + > > +static struct attribute *sg2042_mcu_attrs[] = { > > + &dev_attr_board_type.attr, > > + &dev_attr_firmware_version.attr, > > + &dev_attr_pcb_version.attr, > > + &dev_attr_reset_count.attr, > > + &dev_attr_uptime.attr, > > + &dev_attr_reset_reason.attr, > > + &dev_attr_mcu_type.attr, > > + &dev_attr_critical_action.attr, > > + &dev_attr_repower_temp.attr, > > + NULL > > +}; > > + > > +static const struct attribute_group sg2042_mcu_attr_group = { > > + .attrs = sg2042_mcu_attrs, > > +}; > > + > > +static const struct hwmon_channel_info * const sg2042_mcu_info[] = { > > + HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ | HWMON_C_UPDATE_INTERVAL), > > + HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_CRIT, > > + HWMON_T_INPUT), > > + NULL > > +}; > > + > > +static int sg2042_mcu_read_temp(struct device *dev, > > + u32 attr, int channel, > > + long *val) > > +{ > > + struct sg2042_mcu_data *mcu = dev_get_drvdata(dev); > > + long tmp; > > + > > + switch (attr) { > > + case hwmon_temp_input: > > + switch (channel) { > > + case 0: > > + tmp = sg2042_mcu_get_soc_temp(mcu->client); > > + if (tmp < 0) > > + return tmp; > > + *val = tmp * 1000; > > + break; > > + case 1: > > + tmp = sg2042_mcu_get_board_temp(mcu->client); > > + if (tmp < 0) > > + return tmp; > > + *val = tmp * 1000; > > + break; > > + default: > > + return -EOPNOTSUPP; > > + } > > + break; > > + case hwmon_temp_crit: > > + if (channel) > > + return -EOPNOTSUPP; > > + > > + tmp = sg2042_mcu_get_critical_temp(mcu->client); > > + if (tmp < 0) > > + return tmp; > > + *val = tmp * 1000; > > + break; > > + default: > > + return -EOPNOTSUPP; > > + } > > + return 0; > > +} > > + > > +static int sg2042_mcu_read(struct device *dev, > > + enum hwmon_sensor_types type, > > + u32 attr, int channel, long *val) > > +{ > > + switch (type) { > > + case hwmon_chip: > > + if (attr != hwmon_chip_update_interval) > > + return -EOPNOTSUPP; > > + *val = 1000; > > + break; > > + case hwmon_temp: > > + return sg2042_mcu_read_temp(dev, attr, channel, val); > > + default: > > + return -EOPNOTSUPP; > > + } > > + 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); > > + u8 down_temp, repower_temp; > > + int ret; > > + > > + if (type != hwmon_temp || attr != hwmon_temp_crit || !channel) > > + return -EOPNOTSUPP; > > + > > + ret = sg2042_mcu_get_repower_temp(mcu->client); > > + if (ret < 0) > > + repower_temp = DEFAULT_REPOWER_TEMP; > > + else > > + repower_temp = ret; > > + > > + down_temp = val / 1000; > > + if (down_temp < repower_temp) > > + return -EINVAL; > > + > > + return sg2042_mcu_set_critical_temp(mcu->client, (u8)(val / 1000)); > > +} > > + > > +static umode_t sg2042_mcu_is_visible(const void *_data, > > + enum hwmon_sensor_types type, > > + u32 attr, int channel) > > +{ > > + switch (type) { > > + case hwmon_chip: > > + if (attr == hwmon_chip_update_interval) > > + return 0444; > > + break; > > + case hwmon_temp: > > + switch (attr) { > > + case hwmon_temp_input: > > + if (channel < 2) > > + return 0444; > > + break; > > + case hwmon_temp_crit: > > + if (channel == 0) > > + return 0664; > > + break; > > + default: > > + return 0; > > + } > > + break; > > + default: > > + return 0; > > + } > > + 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 int sg2042_mcu_check_board(u8 id) > > +{ > > + int i; > > + > > + for (i = 0; i < ARRAY_SIZE(sg2042_boards_data); i++) { > > + if (sg2042_boards_data[i].id == id) > > + return i; > > + } > > + > > + return -ENODEV; > > +} > > + > > +static int sg2042_mcu_i2c_probe(struct i2c_client *client) > > +{ > > + int ret; > > + 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 -EIO; > > + > > + ret = sg2042_mcu_get_board_type(client); > > + if (ret < 0) > > + return ret; > > + > > + ret = sg2042_mcu_check_board(ret); > > + if (ret < 0) > > + return ret; > > + > > + mcu = devm_kmalloc(dev, sizeof(*mcu), GFP_KERNEL); > > + if (!mcu) > > + return -ENOMEM; > > + > > + mcu->client = client; > > + mcu->board_info = &sg2042_boards_data[ret]; > > + > > + ret = sysfs_create_group(&dev->kobj, &sg2042_mcu_attr_group); > > + if (ret < 0) > > + return ret; > > + > > + i2c_set_clientdata(client, mcu); > > + > > + hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, > > + mcu, > > + &sg2042_mcu_chip_info, > > + NULL); > > + > > + return PTR_ERR_OR_ZERO(hwmon_dev); > > +} > > + > > +static void sg2042_mcu_i2c_remove(struct i2c_client *client) > > +{ > > + struct device *dev = &client->dev; > > + > > + sysfs_remove_group(&dev->kobj, &sg2042_mcu_attr_group); > > +} > > + > > +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, > > + }, > > + .probe = sg2042_mcu_i2c_probe, > > + .remove = sg2042_mcu_i2c_remove, > > + .id_table = sg2042_mcu_id, > > +}; > > + > > +module_i2c_driver(sg2042_mcu_driver); > > + > > +MODULE_DESCRIPTION("MCU I2C driver for bm16xx soc platform"); > > +MODULE_LICENSE("GPL"); > > -- > > 2.44.0 > > >