| Message ID | 20230808013157.80913-1-mark.tomlinson@alliedtelesis.co.nz (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/2] hwmon: Add driver for EMC181x temperature sensors | expand |
On 8/7/23 18:31, Mark Tomlinson wrote: > This patch adds a HWMON driver for the EMC1812, EMC1813, EMC1814, > EMC1815 and EMC1833 temperature sensor chips from microchip. Does not Microchip > currently support the alert outputs. > It doesn't support limit registers either. Odd, but I guess it is better to have some driver support than nothing. checkpatch --strict reports a couple of problems. Please fix. > Signed-off-by: Mark Tomlinson <mark.tomlinson@alliedtelesis.co.nz> > Co-developed-by: Mathew McBride <matt@traverse.com.au> > Signed-off-by: Mathew McBride <matt@traverse.com.au> > --- > drivers/hwmon/Kconfig | 10 ++ > drivers/hwmon/Makefile | 1 + > drivers/hwmon/emc181x.c | 296 ++++++++++++++++++++++++++++++++++++++++ Documentation missing. Also, please no wildcards in file names. EMC1833 doesn't even fit the pattern, and the driver doesn't support ENC18[01] or EMC181[6-9]. Use one of the chips as name. The datasheet says "EMC1812 family", which just as well works here. > 3 files changed, 307 insertions(+) > create mode 100644 drivers/hwmon/emc181x.c > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index 307477b8a371..196d91494536 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -1820,6 +1820,16 @@ config SENSORS_EMC1403 > Threshold values can be configured using sysfs. > Data from the different diodes are accessible via sysfs. > > +config SENSORS_EMC181X > + tristate "SMSC EMC181X thermal sensor" Again, no wildcards here. Also, s/SMSC/Microchip/ > + depends on I2C > + select REGMAP_I2C > + help > + If you say yes here you get support for the SMSC EMC181X > + temperature monitoring chip. EMC181X does not exist. List the supported chips. > + > + Data from the different diodes are accessible via sysfs. That has no value. > + > config SENSORS_EMC2103 > tristate "SMSC EMC2103" > depends on I2C > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index 3f4b0fda0998..bcea887dfe17 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -69,6 +69,7 @@ obj-$(CONFIG_SENSORS_DRIVETEMP) += drivetemp.o > obj-$(CONFIG_SENSORS_DS620) += ds620.o > obj-$(CONFIG_SENSORS_DS1621) += ds1621.o > obj-$(CONFIG_SENSORS_EMC1403) += emc1403.o > +obj-$(CONFIG_SENSORS_EMC181X) += emc181x.o > obj-$(CONFIG_SENSORS_EMC2103) += emc2103.o > obj-$(CONFIG_SENSORS_EMC2305) += emc2305.o > obj-$(CONFIG_SENSORS_EMC6W201) += emc6w201.o > diff --git a/drivers/hwmon/emc181x.c b/drivers/hwmon/emc181x.c > new file mode 100644 > index 000000000000..364d2bfb15ca > --- /dev/null > +++ b/drivers/hwmon/emc181x.c > @@ -0,0 +1,296 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Driver for the Microchip/SMSC EMC181x Temperature monitor > + * > + * Copyright (C) 2018-2019 Traverse Technologies > + * Author: Mathew McBride <matt@traverse.com.au> > + * > + * Copyright (C) 2023 Allied Telesis Labs > + * Author: Mark Tomlinson <mark.tomlinson@alliedtelesis.co.nz> > + * > + */ > + > +#include <linux/err.h> > +#include <linux/hwmon.h> > +#include <linux/hwmon-sysfs.h> Not needed > +#include <linux/of_device.h> > +#include <linux/i2c.h> > +#include <linux/kernel.h> > +#include <linux/module.h> Alphabetic order > + > +#define EMC1812_ID 0x81 > +#define EMC1813_ID 0x87 > +#define EMC1814_ID 0x84 > +#define EMC1815_ID 0x85 > +#define EMC1833_ID 0x83 > + > +#define MICROCHIP_ID 0x54 > + > +#define EMC181X_STATUS_REG 0x02 > +#define EMC181X_CONFIG_REG 0x03 > +#define EMC181X_DIODE_DATA_BASE 0x60 > +#define EMC181X_PID_REG 0xfd > +#define EMC181X_SMSC_ID_REG 0xfe > +#define EMC181X_REVISION_REG 0xff Some of the above are not used. Drop unused definitions. > + > +/* Adjustable address type is 0x7c, 0x5c, 0x4c, 0x6c, 0x1c, 0x3c */ > +/* Fixed address is either 0x4c or 0x45 */ Irrelevant here. > +static const unsigned short emc181x_address_list[] = { > + 0x7c, 0x5c, 0x4c, 0x6c, 0x1c, 0x3c, 0x45, I2C_CLIENT_END Numeric order, please. > +}; > + > +enum chips { > + emc1812, emc1813, > + emc1814, emc1815, > + emc1833, One line is enough. > +}; > + > +struct emc181x_data { > + struct i2c_client *i2c; > + enum chips type; > + bool is_extended_range; > +}; > + > +static int emc181x_read(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long *val) > +{ > + struct emc181x_data *data = dev_get_drvdata(dev); > + struct i2c_client *i2c = data->i2c; > + Unnecessary empty line. > + u8 channel_reg = 0; > + s32 channel_deg = 0; > + s32 channel_frac = 0; No unnecessary initializations, please. > + > + if (type != hwmon_temp) > + return -EOPNOTSUPP; > + if (channel > 4) > + return -EINVAL; > + > + channel_reg = EMC181X_DIODE_DATA_BASE + (channel * 0x02); > + channel_deg = i2c_smbus_read_byte_data(i2c, channel_reg); > + if (channel_deg < 0) > + return channel_deg; > + if (data->is_extended_range) > + channel_deg -= 64; > + > + channel_frac = i2c_smbus_read_byte_data(i2c, channel_reg + 0x01); > + if (channel_frac < 0) > + return channel_frac; > + The above either needs to be mutex protected, or i2c_smbus_read_word_data() needs to be used. > + *val = ((channel_deg << 3) | (channel_frac >> 5)) * 125; > + return 0; > +} > + > +static umode_t emc181x_is_visible(const void *drvdata, enum hwmon_sensor_types type, > + u32 attr, int channel) > +{ > + if (type == hwmon_temp && attr == hwmon_temp_input) > + return 0444; > + else else after return is pointless. > + return 0; > +} > + > +static const struct hwmon_channel_info *emc1812_info[] = { > + HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ), > + HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT, HWMON_T_INPUT), > + NULL > +}; > + > +static const struct hwmon_channel_info *emc1813_info[] = { > + HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ), > + HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT, HWMON_T_INPUT, HWMON_T_INPUT), > + NULL > +}; > + > +static const struct hwmon_channel_info *emc1814_info[] = { > + HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ), > + HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT, HWMON_T_INPUT, HWMON_T_INPUT, > + HWMON_T_INPUT), > + NULL > +}; > + > +static const struct hwmon_channel_info *emc1815_info[] = { > + HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ), > + HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT, HWMON_T_INPUT, HWMON_T_INPUT, > + HWMON_T_INPUT, HWMON_T_INPUT), > + NULL > +}; Only one of those is needed. The others should be handled with the is_visible() function. > + > +static const struct hwmon_ops emc181x_ops = { > + .is_visible = emc181x_is_visible, > + .read = emc181x_read, > +}; > + > +static const struct hwmon_chip_info emc181x_chip_info[] = { > + [emc1812] = { > + .ops = &emc181x_ops, > + .info = emc1812_info, > + }, > + [emc1813] = { > + .ops = &emc181x_ops, > + .info = emc1813_info, > + }, > + [emc1814] = { > + .ops = &emc181x_ops, > + .info = emc1814_info, > + }, > + [emc1815] = { > + .ops = &emc181x_ops, > + .info = emc1815_info, > + }, > + [emc1833] = { > + .ops = &emc181x_ops, > + .info = emc1813_info, > + }, > +}; > + > +static const struct i2c_device_id emc181x_i2c_id[] = { > + { "emc1812", emc1812 }, > + { "emc1813", emc1813 }, > + { "emc1814", emc1814 }, > + { "emc1815", emc1815 }, > + { "emc1833", emc1833 }, > + {} > +}; > + > +MODULE_DEVICE_TABLE(i2c, emc181x_i2c_id); > + > +/* Return 0 if detection is successful, -ENODEV otherwise */ ... which is violated below. > +static int emc181x_i2c_detect(struct i2c_client *client, > + struct i2c_board_info *info) > +{ > + struct i2c_adapter *adapter = client->adapter; > + int pid, id, rev; > + const char *name; > + > + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA | > + I2C_FUNC_SMBUS_WORD_DATA)) While the driver should use I2C_FUNC_SMBUS_WORD_DATA, it currently doesn't. > + return -ENODEV; > + > + /* Determine the chip type */ Unnecessary comment > + id = i2c_smbus_read_byte_data(client, EMC181X_SMSC_ID_REG); > + if (id < 0) > + return id; -ENODEV should be returned here; if this is not an expected chip there may be no register at this address. > + if (id != MICROCHIP_ID) > + return -ENODEV; > + pid = i2c_smbus_read_byte_data(client, EMC181X_PID_REG); > + if (pid < 0) > + return pid; > + rev = i2c_smbus_read_byte_data(client, EMC181X_REVISION_REG); > + if (rev < 0) > + return rev; > + > + switch (pid) { > + case EMC1812_ID: > + name = emc181x_i2c_id[emc1812].name; > + break; > + case EMC1813_ID: > + name = emc181x_i2c_id[emc1813].name; > + break; > + case EMC1814_ID: > + name = emc181x_i2c_id[emc1814].name; > + break; > + case EMC1815_ID: > + name = emc181x_i2c_id[emc1815].name; > + break; > + case EMC1833_ID: > + name = emc181x_i2c_id[emc1833].name; > + break; > + default: > + return -ENODEV; > + } > + strscpy(info->type, name, I2C_NAME_SIZE); > + > + dev_dbg(&client->dev, > + "Detected device %s at 0x%02x with COMPANY: 0x%02x and PID: 0x%02x REV: 0x%02x\n", > + name, client->addr, id, pid, rev); COMPANY is pretty pointless here. > + > + return 0; > +} > + > +static int emc181x_i2c_probe(struct i2c_client *client) > +{ > + struct device *dev = &client->dev; > + struct device *hwmon_dev; > + struct emc181x_data *data; > + s32 regval; > + u8 config; > + > + data = devm_kzalloc(dev, sizeof(struct emc181x_data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->i2c = client; > + if (dev_fwnode(dev)) { > + data->type = (enum chips)device_get_match_data(dev); > + data->is_extended_range = device_property_present(dev, "emc181x,extended-range"); > + } else > + data->type = i2c_match_id(emc181x_i2c_id, client)->driver_data; > + > + regval = i2c_smbus_read_byte_data(client, EMC181X_CONFIG_REG); > + if (regval < 0) { > + dev_dbg(dev, "Failed to read config reg %d", regval); > + return regval; > + } > + > + /* By default, extended range is disabled in the EMC181X, > + * if enabled we need to set this in the CONFIG register. > + */ This is not the networking subsystem. Please use standard multi-line comments. > + config = regval & ~0x04; > + if (data->is_extended_range) > + config |= 0x04; > + > + dev_dbg(dev, "EMC181X setting CONFIG to %d\n", config); > + if (config != regval) > + i2c_smbus_write_byte_data(client, EMC181X_CONFIG_REG, config); Error handling missing. > + > + hwmon_dev = devm_hwmon_device_register_with_info(dev, > + client->name, > + data, > + &emc181x_chip_info[data->type], > + NULL); Limit the number of continuation lines to what is necessary. > + > + return PTR_ERR_OR_ZERO(hwmon_dev); > +} > + > +static const struct of_device_id __maybe_unused emc181x_of_match[] = { Drop __maybe_unused > + { > + .compatible = "microchip,emc1812", > + .data = (void *)emc1812 > + }, > + { > + .compatible = "microchip,emc1813", > + .data = (void *)emc1813 > + }, > + { > + .compatible = "microchip,emc1814", > + .data = (void *)emc1814 > + }, > + { > + .compatible = "microchip,emc1815", > + .data = (void *)emc1815 > + }, > + { > + .compatible = "microchip,emc1833", > + .data = (void *)emc1833 > + }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, emc181x_of_match); > + > +static struct i2c_driver emc181x_i2c_driver = { > + .driver = { > + .name = "emc181x", > + .of_match_table = of_match_ptr(emc181x_of_match), Drop of_match_ptr > + }, > + .probe = emc181x_i2c_probe, > + .id_table = emc181x_i2c_id, > + .detect = emc181x_i2c_detect, > + .address_list = emc181x_address_list, > +}; > + > +module_i2c_driver(emc181x_i2c_driver); > + > +MODULE_DESCRIPTION("EMC181X Sensor Driver"); > +MODULE_AUTHOR("Mark Tomlinson <mark.tomlinson@alliedtelesis.co.nz>"); > +MODULE_LICENSE("GPL");
On 08/08/2023 03:31, Mark Tomlinson wrote: > This patch adds a HWMON driver for the EMC1812, EMC1813, EMC1814, > EMC1815 and EMC1833 temperature sensor chips from microchip. Does not > currently support the alert outputs. > > Signed-off-by: Mark Tomlinson <mark.tomlinson@alliedtelesis.co.nz> > Co-developed-by: Mathew McBride <matt@traverse.com.au> > Signed-off-by: Mathew McBride <matt@traverse.com.au> The order of these tags is clearly not correct. It says Mathew is sending, but you are not Mathew? > --- > drivers/hwmon/Kconfig | 10 ++ > drivers/hwmon/Makefile | 1 + > drivers/hwmon/emc181x.c | 296 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 307 insertions(+) > create mode 100644 drivers/hwmon/emc181x.c ... > + > +static const struct i2c_device_id emc181x_i2c_id[] = { > + { "emc1812", emc1812 }, > + { "emc1813", emc1813 }, > + { "emc1814", emc1814 }, > + { "emc1815", emc1815 }, > + { "emc1833", emc1833 }, > + {} > +}; > + > +MODULE_DEVICE_TABLE(i2c, emc181x_i2c_id); Keep the entire table next to OF device ID table. > + > +/* Return 0 if detection is successful, -ENODEV otherwise */ > +static int emc181x_i2c_detect(struct i2c_client *client, > + struct i2c_board_info *info) > +{ > + struct i2c_adapter *adapter = client->adapter; > + int pid, id, rev; > + const char *name; > + > + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA | > + I2C_FUNC_SMBUS_WORD_DATA)) > + return -ENODEV; > + > + /* Determine the chip type */ > + id = i2c_smbus_read_byte_data(client, EMC181X_SMSC_ID_REG); > + if (id < 0) > + return id; > + if (id != MICROCHIP_ID) > + return -ENODEV; > + pid = i2c_smbus_read_byte_data(client, EMC181X_PID_REG); > + if (pid < 0) > + return pid; > + rev = i2c_smbus_read_byte_data(client, EMC181X_REVISION_REG); > + if (rev < 0) > + return rev; > + > + switch (pid) { > + case EMC1812_ID: > + name = emc181x_i2c_id[emc1812].name; > + break; > + case EMC1813_ID: > + name = emc181x_i2c_id[emc1813].name; > + break; > + case EMC1814_ID: > + name = emc181x_i2c_id[emc1814].name; > + break; > + case EMC1815_ID: > + name = emc181x_i2c_id[emc1815].name; > + break; > + case EMC1833_ID: > + name = emc181x_i2c_id[emc1833].name; > + break; > + default: > + return -ENODEV; > + } > + strscpy(info->type, name, I2C_NAME_SIZE); > + > + dev_dbg(&client->dev, > + "Detected device %s at 0x%02x with COMPANY: 0x%02x and PID: 0x%02x REV: 0x%02x\n", > + name, client->addr, id, pid, rev); If you have detection, why not using it for device matching? All devices should be OF-compatible with one fallback. > + > + return 0; > +} > + > +static int emc181x_i2c_probe(struct i2c_client *client) > +{ > + struct device *dev = &client->dev; > + struct device *hwmon_dev; > + struct emc181x_data *data; > + s32 regval; > + u8 config; > + > + data = devm_kzalloc(dev, sizeof(struct emc181x_data), GFP_KERNEL); sizeof(*) > + if (!data) > + return -ENOMEM; > + > + data->i2c = client; > + if (dev_fwnode(dev)) { > + data->type = (enum chips)device_get_match_data(dev); > + data->is_extended_range = device_property_present(dev, "emc181x,extended-range"); > + } else > + data->type = i2c_match_id(emc181x_i2c_id, client)->driver_data; Please run scripts/checkpatch.pl and fix reported warnings. Some warnings can be ignored, but the code here looks like it needs a fix. Feel free to get in touch if the warning is not clear. > + > + regval = i2c_smbus_read_byte_data(client, EMC181X_CONFIG_REG); > + if (regval < 0) { > + dev_dbg(dev, "Failed to read config reg %d", regval); > + return regval; > + } > + > + /* By default, extended range is disabled in the EMC181X, /* * ... > + * if enabled we need to set this in the CONFIG register. > + */ > + config = regval & ~0x04; > + if (data->is_extended_range) > + config |= 0x04; > + > + dev_dbg(dev, "EMC181X setting CONFIG to %d\n", config); > + if (config != regval) > + i2c_smbus_write_byte_data(client, EMC181X_CONFIG_REG, config); > + > + hwmon_dev = devm_hwmon_device_register_with_info(dev, > + client->name, > + data, > + &emc181x_chip_info[data->type], > + NULL); > + > + return PTR_ERR_OR_ZERO(hwmon_dev); > +} > + > +static const struct of_device_id __maybe_unused emc181x_of_match[] = { > + { > + .compatible = "microchip,emc1812", Bindings go before the user. > + .data = (void *)emc1812 > + }, > + { > + .compatible = "microchip,emc1813", > + .data = (void *)emc1813 > + }, > + { > + .compatible = "microchip,emc1814", > + .data = (void *)emc1814 > + }, > + { > + .compatible = "microchip,emc1815", > + .data = (void *)emc1815 > + }, > + { > + .compatible = "microchip,emc1833", > + .data = (void *)emc1833 > + }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, emc181x_of_match); > + > +static struct i2c_driver emc181x_i2c_driver = { > + .driver = { > + .name = "emc181x", > + .of_match_table = of_match_ptr(emc181x_of_match), > + }, Drop of_match_ptr(). This will cause warnings. > + .probe = emc181x_i2c_probe, > + .id_table = emc181x_i2c_id, > + .detect = emc181x_i2c_detect, > + .address_list = emc181x_address_list, > +}; > + > +module_i2c_driver(emc181x_i2c_driver); > + > +MODULE_DESCRIPTION("EMC181X Sensor Driver"); > +MODULE_AUTHOR("Mark Tomlinson <mark.tomlinson@alliedtelesis.co.nz>"); > +MODULE_LICENSE("GPL"); Best regards, Krzysztof
Hi Krzysztof, On Tue, Aug 8, 2023, at 5:38 PM, Krzysztof Kozlowski wrote: > On 08/08/2023 03:31, Mark Tomlinson wrote: > > This patch adds a HWMON driver for the EMC1812, EMC1813, EMC1814, > > EMC1815 and EMC1833 temperature sensor chips from microchip. Does not > > currently support the alert outputs. > > > > Signed-off-by: Mark Tomlinson <mark.tomlinson@alliedtelesis.co.nz> > > Co-developed-by: Mathew McBride <matt@traverse.com.au> > > Signed-off-by: Mathew McBride <matt@traverse.com.au> > > The order of these tags is clearly not correct. It says Mathew is > sending, but you are not Mathew? Just to clarify, Mark has developed this version based off something I wrote a while ago: https://gitlab.com/traversetech/ls1088firmware/traverse-sensors/-/blob/master/emc181x/emc181x.c?ref_type=heads Hence my copyright is listed in the header. I advised him to list the authors in that order following the example in the kernel's "submitting patches" guide [2] which provides an example of such a situation: https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by If I interpreted the guidelines incorrectly, happy to be corrected. Best Regards, Matt
On 08/08/2023 09:54, Mathew McBride wrote: > Hi Krzysztof, > > On Tue, Aug 8, 2023, at 5:38 PM, Krzysztof Kozlowski wrote: >> On 08/08/2023 03:31, Mark Tomlinson wrote: >>> This patch adds a HWMON driver for the EMC1812, EMC1813, EMC1814, >>> EMC1815 and EMC1833 temperature sensor chips from microchip. Does not >>> currently support the alert outputs. >>> >>> Signed-off-by: Mark Tomlinson <mark.tomlinson@alliedtelesis.co.nz> >>> Co-developed-by: Mathew McBride <matt@traverse.com.au> >>> Signed-off-by: Mathew McBride <matt@traverse.com.au> >> >> The order of these tags is clearly not correct. It says Mathew is >> sending, but you are not Mathew? > > Just to clarify, Mark has developed this version based off something I wrote a while ago: > > https://gitlab.com/traversetech/ls1088firmware/traverse-sensors/-/blob/master/emc181x/emc181x.c?ref_type=heads > > Hence my copyright is listed in the header. > > I advised him to list the authors in that order following the example in the kernel's "submitting patches" guide [2] which provides an example of such a situation: > > https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by The last entry is: Signed-off-by: From Author <from@author.example.org> So it is expected to be Mark. Best regards, Krzysztof
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 307477b8a371..196d91494536 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1820,6 +1820,16 @@ config SENSORS_EMC1403 Threshold values can be configured using sysfs. Data from the different diodes are accessible via sysfs. +config SENSORS_EMC181X + tristate "SMSC EMC181X thermal sensor" + depends on I2C + select REGMAP_I2C + help + If you say yes here you get support for the SMSC EMC181X + temperature monitoring chip. + + Data from the different diodes are accessible via sysfs. + config SENSORS_EMC2103 tristate "SMSC EMC2103" depends on I2C diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 3f4b0fda0998..bcea887dfe17 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -69,6 +69,7 @@ obj-$(CONFIG_SENSORS_DRIVETEMP) += drivetemp.o obj-$(CONFIG_SENSORS_DS620) += ds620.o obj-$(CONFIG_SENSORS_DS1621) += ds1621.o obj-$(CONFIG_SENSORS_EMC1403) += emc1403.o +obj-$(CONFIG_SENSORS_EMC181X) += emc181x.o obj-$(CONFIG_SENSORS_EMC2103) += emc2103.o obj-$(CONFIG_SENSORS_EMC2305) += emc2305.o obj-$(CONFIG_SENSORS_EMC6W201) += emc6w201.o diff --git a/drivers/hwmon/emc181x.c b/drivers/hwmon/emc181x.c new file mode 100644 index 000000000000..364d2bfb15ca --- /dev/null +++ b/drivers/hwmon/emc181x.c @@ -0,0 +1,296 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Driver for the Microchip/SMSC EMC181x Temperature monitor + * + * Copyright (C) 2018-2019 Traverse Technologies + * Author: Mathew McBride <matt@traverse.com.au> + * + * Copyright (C) 2023 Allied Telesis Labs + * Author: Mark Tomlinson <mark.tomlinson@alliedtelesis.co.nz> + * + */ + +#include <linux/err.h> +#include <linux/hwmon.h> +#include <linux/hwmon-sysfs.h> +#include <linux/of_device.h> +#include <linux/i2c.h> +#include <linux/kernel.h> +#include <linux/module.h> + +#define EMC1812_ID 0x81 +#define EMC1813_ID 0x87 +#define EMC1814_ID 0x84 +#define EMC1815_ID 0x85 +#define EMC1833_ID 0x83 + +#define MICROCHIP_ID 0x54 + +#define EMC181X_STATUS_REG 0x02 +#define EMC181X_CONFIG_REG 0x03 +#define EMC181X_DIODE_DATA_BASE 0x60 +#define EMC181X_PID_REG 0xfd +#define EMC181X_SMSC_ID_REG 0xfe +#define EMC181X_REVISION_REG 0xff + +/* Adjustable address type is 0x7c, 0x5c, 0x4c, 0x6c, 0x1c, 0x3c */ +/* Fixed address is either 0x4c or 0x45 */ +static const unsigned short emc181x_address_list[] = { + 0x7c, 0x5c, 0x4c, 0x6c, 0x1c, 0x3c, 0x45, I2C_CLIENT_END +}; + +enum chips { + emc1812, emc1813, + emc1814, emc1815, + emc1833, +}; + +struct emc181x_data { + struct i2c_client *i2c; + enum chips type; + bool is_extended_range; +}; + +static int emc181x_read(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long *val) +{ + struct emc181x_data *data = dev_get_drvdata(dev); + struct i2c_client *i2c = data->i2c; + + u8 channel_reg = 0; + s32 channel_deg = 0; + s32 channel_frac = 0; + + if (type != hwmon_temp) + return -EOPNOTSUPP; + if (channel > 4) + return -EINVAL; + + channel_reg = EMC181X_DIODE_DATA_BASE + (channel * 0x02); + channel_deg = i2c_smbus_read_byte_data(i2c, channel_reg); + if (channel_deg < 0) + return channel_deg; + if (data->is_extended_range) + channel_deg -= 64; + + channel_frac = i2c_smbus_read_byte_data(i2c, channel_reg + 0x01); + if (channel_frac < 0) + return channel_frac; + + *val = ((channel_deg << 3) | (channel_frac >> 5)) * 125; + return 0; +} + +static umode_t emc181x_is_visible(const void *drvdata, enum hwmon_sensor_types type, + u32 attr, int channel) +{ + if (type == hwmon_temp && attr == hwmon_temp_input) + return 0444; + else + return 0; +} + +static const struct hwmon_channel_info *emc1812_info[] = { + HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ), + HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT, HWMON_T_INPUT), + NULL +}; + +static const struct hwmon_channel_info *emc1813_info[] = { + HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ), + HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT, HWMON_T_INPUT, HWMON_T_INPUT), + NULL +}; + +static const struct hwmon_channel_info *emc1814_info[] = { + HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ), + HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT, HWMON_T_INPUT, HWMON_T_INPUT, + HWMON_T_INPUT), + NULL +}; + +static const struct hwmon_channel_info *emc1815_info[] = { + HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ), + HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT, HWMON_T_INPUT, HWMON_T_INPUT, + HWMON_T_INPUT, HWMON_T_INPUT), + NULL +}; + +static const struct hwmon_ops emc181x_ops = { + .is_visible = emc181x_is_visible, + .read = emc181x_read, +}; + +static const struct hwmon_chip_info emc181x_chip_info[] = { + [emc1812] = { + .ops = &emc181x_ops, + .info = emc1812_info, + }, + [emc1813] = { + .ops = &emc181x_ops, + .info = emc1813_info, + }, + [emc1814] = { + .ops = &emc181x_ops, + .info = emc1814_info, + }, + [emc1815] = { + .ops = &emc181x_ops, + .info = emc1815_info, + }, + [emc1833] = { + .ops = &emc181x_ops, + .info = emc1813_info, + }, +}; + +static const struct i2c_device_id emc181x_i2c_id[] = { + { "emc1812", emc1812 }, + { "emc1813", emc1813 }, + { "emc1814", emc1814 }, + { "emc1815", emc1815 }, + { "emc1833", emc1833 }, + {} +}; + +MODULE_DEVICE_TABLE(i2c, emc181x_i2c_id); + +/* Return 0 if detection is successful, -ENODEV otherwise */ +static int emc181x_i2c_detect(struct i2c_client *client, + struct i2c_board_info *info) +{ + struct i2c_adapter *adapter = client->adapter; + int pid, id, rev; + const char *name; + + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA | + I2C_FUNC_SMBUS_WORD_DATA)) + return -ENODEV; + + /* Determine the chip type */ + id = i2c_smbus_read_byte_data(client, EMC181X_SMSC_ID_REG); + if (id < 0) + return id; + if (id != MICROCHIP_ID) + return -ENODEV; + pid = i2c_smbus_read_byte_data(client, EMC181X_PID_REG); + if (pid < 0) + return pid; + rev = i2c_smbus_read_byte_data(client, EMC181X_REVISION_REG); + if (rev < 0) + return rev; + + switch (pid) { + case EMC1812_ID: + name = emc181x_i2c_id[emc1812].name; + break; + case EMC1813_ID: + name = emc181x_i2c_id[emc1813].name; + break; + case EMC1814_ID: + name = emc181x_i2c_id[emc1814].name; + break; + case EMC1815_ID: + name = emc181x_i2c_id[emc1815].name; + break; + case EMC1833_ID: + name = emc181x_i2c_id[emc1833].name; + break; + default: + return -ENODEV; + } + strscpy(info->type, name, I2C_NAME_SIZE); + + dev_dbg(&client->dev, + "Detected device %s at 0x%02x with COMPANY: 0x%02x and PID: 0x%02x REV: 0x%02x\n", + name, client->addr, id, pid, rev); + + return 0; +} + +static int emc181x_i2c_probe(struct i2c_client *client) +{ + struct device *dev = &client->dev; + struct device *hwmon_dev; + struct emc181x_data *data; + s32 regval; + u8 config; + + data = devm_kzalloc(dev, sizeof(struct emc181x_data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + data->i2c = client; + if (dev_fwnode(dev)) { + data->type = (enum chips)device_get_match_data(dev); + data->is_extended_range = device_property_present(dev, "emc181x,extended-range"); + } else + data->type = i2c_match_id(emc181x_i2c_id, client)->driver_data; + + regval = i2c_smbus_read_byte_data(client, EMC181X_CONFIG_REG); + if (regval < 0) { + dev_dbg(dev, "Failed to read config reg %d", regval); + return regval; + } + + /* By default, extended range is disabled in the EMC181X, + * if enabled we need to set this in the CONFIG register. + */ + config = regval & ~0x04; + if (data->is_extended_range) + config |= 0x04; + + dev_dbg(dev, "EMC181X setting CONFIG to %d\n", config); + if (config != regval) + i2c_smbus_write_byte_data(client, EMC181X_CONFIG_REG, config); + + hwmon_dev = devm_hwmon_device_register_with_info(dev, + client->name, + data, + &emc181x_chip_info[data->type], + NULL); + + return PTR_ERR_OR_ZERO(hwmon_dev); +} + +static const struct of_device_id __maybe_unused emc181x_of_match[] = { + { + .compatible = "microchip,emc1812", + .data = (void *)emc1812 + }, + { + .compatible = "microchip,emc1813", + .data = (void *)emc1813 + }, + { + .compatible = "microchip,emc1814", + .data = (void *)emc1814 + }, + { + .compatible = "microchip,emc1815", + .data = (void *)emc1815 + }, + { + .compatible = "microchip,emc1833", + .data = (void *)emc1833 + }, + { }, +}; +MODULE_DEVICE_TABLE(of, emc181x_of_match); + +static struct i2c_driver emc181x_i2c_driver = { + .driver = { + .name = "emc181x", + .of_match_table = of_match_ptr(emc181x_of_match), + }, + .probe = emc181x_i2c_probe, + .id_table = emc181x_i2c_id, + .detect = emc181x_i2c_detect, + .address_list = emc181x_address_list, +}; + +module_i2c_driver(emc181x_i2c_driver); + +MODULE_DESCRIPTION("EMC181X Sensor Driver"); +MODULE_AUTHOR("Mark Tomlinson <mark.tomlinson@alliedtelesis.co.nz>"); +MODULE_LICENSE("GPL");