* [PATCH v3 0/3] Add W83773G hwmon sensor driver and doc @ 2017-11-09 7:08 ` Lei YU 0 siblings, 0 replies; 8+ messages in thread From: Lei YU @ 2017-11-09 7:08 UTC (permalink / raw) To: Mark Rutland, Jean Delvare, Guenter Roeck, Jonathan Corbet, Jiri Kosina Cc: Lei YU, devicetree, linux-kernel, linux-hwmon, Joel Stanley, Andrew Jeffery Nuvoton W83773G is a hardware monitoring chip, which integrates two remote and one local temperature sensors. --- v2: - The driver is re-written as v1's comment, so the author is changed to me. - Added the device to trivial-devices.txt v3: - Update the driver to use new API devm_hwmon_device_register_with_info() Lei YU (3): DT: i2c: W83773G is a trivial device drivers: hwmon: Add W83773G driver hwmon: (w83773g) Add documentation .../devicetree/bindings/trivial-devices.txt | 1 + Documentation/hwmon/w83773g | 33 ++ drivers/hwmon/Kconfig | 10 + drivers/hwmon/Makefile | 1 + drivers/hwmon/w83773g.c | 348 +++++++++++++++++++++ 5 files changed, 393 insertions(+) create mode 100644 Documentation/hwmon/w83773g create mode 100644 drivers/hwmon/w83773g.c -- 1.9.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 0/3] Add W83773G hwmon sensor driver and doc @ 2017-11-09 7:08 ` Lei YU 0 siblings, 0 replies; 8+ messages in thread From: Lei YU @ 2017-11-09 7:08 UTC (permalink / raw) To: Mark Rutland, Jean Delvare, Guenter Roeck, Jonathan Corbet, Jiri Kosina Cc: Lei YU, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-hwmon-u79uwXL29TY76Z2rM5mHXA, Joel Stanley, Andrew Jeffery Nuvoton W83773G is a hardware monitoring chip, which integrates two remote and one local temperature sensors. --- v2: - The driver is re-written as v1's comment, so the author is changed to me. - Added the device to trivial-devices.txt v3: - Update the driver to use new API devm_hwmon_device_register_with_info() Lei YU (3): DT: i2c: W83773G is a trivial device drivers: hwmon: Add W83773G driver hwmon: (w83773g) Add documentation .../devicetree/bindings/trivial-devices.txt | 1 + Documentation/hwmon/w83773g | 33 ++ drivers/hwmon/Kconfig | 10 + drivers/hwmon/Makefile | 1 + drivers/hwmon/w83773g.c | 348 +++++++++++++++++++++ 5 files changed, 393 insertions(+) create mode 100644 Documentation/hwmon/w83773g create mode 100644 drivers/hwmon/w83773g.c -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 1/3] DT: i2c: W83773G is a trivial device 2017-11-09 7:08 ` Lei YU (?) @ 2017-11-09 7:08 ` Lei YU -1 siblings, 0 replies; 8+ messages in thread From: Lei YU @ 2017-11-09 7:08 UTC (permalink / raw) To: Mark Rutland, Jean Delvare, Guenter Roeck, Jonathan Corbet, Jiri Kosina Cc: Lei YU, devicetree, linux-kernel, linux-hwmon, Joel Stanley, Andrew Jeffery Signed-off-by: Lei YU <mine260309@gmail.com> --- Documentation/devicetree/bindings/trivial-devices.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/trivial-devices.txt b/Documentation/devicetree/bindings/trivial-devices.txt index af284fb..63ad2f1 100644 --- a/Documentation/devicetree/bindings/trivial-devices.txt +++ b/Documentation/devicetree/bindings/trivial-devices.txt @@ -188,3 +188,4 @@ ti,tmp103 Low Power Digital Temperature Sensor with SMBUS/Two Wire Serial Inter ti,tmp275 Digital Temperature Sensor winbond,w83793 Winbond/Nuvoton H/W Monitor winbond,wpct301 i2c trusted platform module (TPM) +nuvoton,w83773g Nuvoton Temperature Sensor -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 2/3] drivers: hwmon: Add W83773G driver 2017-11-09 7:08 ` Lei YU (?) (?) @ 2017-11-09 7:09 ` Lei YU 2017-11-10 14:49 ` Guenter Roeck -1 siblings, 1 reply; 8+ messages in thread From: Lei YU @ 2017-11-09 7:09 UTC (permalink / raw) To: Mark Rutland, Jean Delvare, Guenter Roeck, Jonathan Corbet, Jiri Kosina Cc: Lei YU, devicetree, linux-kernel, linux-hwmon, Joel Stanley, Andrew Jeffery Nuvoton W83773G is a hardware monitor IC providing one local temperature and two remote temperature sensors. Signed-off-by: Lei YU <mine260309@gmail.com> --- v2: - Rewrite the driver using regmap - Add offset and update_interval v3: - Use devm_hwmon_device_register_with_info() with is_visible/read/write functions. --- drivers/hwmon/Kconfig | 10 ++ drivers/hwmon/Makefile | 1 + drivers/hwmon/w83773g.c | 348 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 359 insertions(+) create mode 100644 drivers/hwmon/w83773g.c diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index d654314..11c6248 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1710,6 +1710,16 @@ config SENSORS_VT8231 This driver can also be built as a module. If so, the module will be called vt8231. +config SENSORS_W83773G + tristate "Nuvoton W83773G" + depends on I2C + help + If you say yes here you get support for the Nuvoton W83773G hardware + monitoring chip. + + This driver can also be built as a module. If so, the module + will be called w83773g. + config SENSORS_W83781D tristate "Winbond W83781D, W83782D, W83783S, Asus AS99127F" depends on I2C diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index c84d978..0649ad8 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -13,6 +13,7 @@ obj-$(CONFIG_SENSORS_ATK0110) += asus_atk0110.o # asb100, then w83781d go first, as they can override other drivers' addresses. obj-$(CONFIG_SENSORS_ASB100) += asb100.o obj-$(CONFIG_SENSORS_W83627HF) += w83627hf.o +obj-$(CONFIG_SENSORS_W83773G) += w83773g.o obj-$(CONFIG_SENSORS_W83792D) += w83792d.o obj-$(CONFIG_SENSORS_W83793) += w83793.o obj-$(CONFIG_SENSORS_W83795) += w83795.o diff --git a/drivers/hwmon/w83773g.c b/drivers/hwmon/w83773g.c new file mode 100644 index 0000000..58ac45b --- /dev/null +++ b/drivers/hwmon/w83773g.c @@ -0,0 +1,348 @@ +/* + * Copyright (C) 2017 IBM Corp. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * Driver for the Nuvoton W83773G SMBus temperature sensor IC. + * Supported models: W83773G + */ + +#include <linux/module.h> +#include <linux/init.h> +#include <linux/i2c.h> +#include <linux/hwmon.h> +#include <linux/hwmon-sysfs.h> +#include <linux/err.h> +#include <linux/of_device.h> +#include <linux/regmap.h> + +/* Addresses to scan */ +static const unsigned short normal_i2c[] = { 0x4c, 0x4d, I2C_CLIENT_END }; + +/* W83773 has 3 channels */ +#define W83773_CHANNELS 3 + +/* The W83773 registers */ +#define W83773_CONVERSION_RATE_REG_READ 0x04 +#define W83773_CONVERSION_RATE_REG_WRITE 0x0A +#define W83773_MANUFACTURER_ID_REG 0xFE +#define W83773_LOCAL_TEMP 0x00 + +static const u8 W83773_STATUS[2] = { 0x02, 0x17 }; + +static const u8 W83773_TEMP_LSB[2] = { 0x10, 0x25 }; +static const u8 W83773_TEMP_MSB[2] = { 0x01, 0x24 }; + +static const u8 W83773_OFFSET_LSB[2] = { 0x12, 0x16 }; +static const u8 W83773_OFFSET_MSB[2] = { 0x11, 0x15 }; + +/* this is the number of sensors in the device */ +static const struct i2c_device_id w83773_id[] = { + { "w83773g" }, + { } +}; + +MODULE_DEVICE_TABLE(i2c, w83773_id); + +static const struct of_device_id w83773_of_match[] = { + { + .compatible = "nuvoton,w83773g" + }, + { }, +}; +MODULE_DEVICE_TABLE(of, w83773_of_match); + +static inline long temp_of_local(s8 reg) +{ + return reg * 1000; +} + +static inline long temp_of_remote(s8 hb, u8 lb) +{ + return (hb << 3 | lb >> 5) * 125; +} + +static int get_local_temp(struct regmap *regmap, long *val) +{ + unsigned int regval; + int ret; + + ret = regmap_read(regmap, W83773_LOCAL_TEMP, ®val); + if (ret < 0) + return ret; + + *val = temp_of_local(regval); + return 0; +} + +static int get_remote_temp(struct regmap *regmap, int index, long *val) +{ + unsigned int regval_high; + unsigned int regval_low; + int ret; + + ret = regmap_read(regmap, W83773_TEMP_MSB[index], ®val_high); + if (ret < 0) + return ret; + + ret = regmap_read(regmap, W83773_TEMP_LSB[index], ®val_low); + if (ret < 0) + return ret; + + *val = temp_of_remote(regval_high, regval_low); + return 0; +} + +static int get_fault(struct regmap *regmap, int index, long *val) +{ + unsigned int regval; + int ret; + + ret = regmap_read(regmap, W83773_STATUS[index], ®val); + + if (ret < 0) + return ret; + + *val = (u8)regval & 0x04 >> 2; + return 0; +} + +static int get_offset(struct regmap *regmap, int index, long *val) +{ + unsigned int regval_high; + unsigned int regval_low; + int ret; + + ret = regmap_read(regmap, W83773_OFFSET_MSB[index], ®val_high); + if (ret < 0) + return ret; + + ret = regmap_read(regmap, W83773_OFFSET_LSB[index], ®val_low); + if (ret < 0) + return ret; + + *val = temp_of_remote(regval_high, regval_low); + return 0; +} + +static int set_offset(struct regmap *regmap, int index, long val) +{ + int ret; + u8 high_byte; + u8 low_byte; + + val = clamp_val(val, -127825, 127825); + /* offset value equals to (high_byte << 3 | low_byte >> 5) * 125 */ + val /= 125; + high_byte = val >> 3; + low_byte = (val & 0x07) << 5; + + ret = regmap_write(regmap, W83773_OFFSET_MSB[index], high_byte); + if (ret < 0) + return ret; + + ret = regmap_write(regmap, W83773_OFFSET_LSB[index], low_byte); + if (ret < 0) + return ret; + + return 0; +} + +static int get_update_interval(struct regmap *regmap, long *val) +{ + unsigned int regval; + int ret; + + ret = regmap_read(regmap, W83773_CONVERSION_RATE_REG_READ, ®val); + if (ret < 0) + return ret; + + *val = 16000 >> regval; + return 0; +} + +static int set_update_interval(struct regmap *regmap, long val) +{ + int ret; + int rate; + + if (val <= 0) + return -EINVAL; + + /* + * For valid rates, interval can be calculated as + * interval = (1 << (8 - rate)) * 62.5; + * Rounded rate is therefore + * rate = 8 - __fls(interval * 8 / (62.5 * 7)); + * Use clamp_val() to avoid overflows, and to ensure valid input + * for __fls. + */ + val = clamp_val(val, 62, 16000) * 10; + rate = 8 - __fls((val * 8 / (625 * 7))); + ret = regmap_write(regmap, W83773_CONVERSION_RATE_REG_WRITE, rate); + if (ret < 0) + return ret; + return 0; +} + +static int w83773_read(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long *val) +{ + struct regmap *regmap = dev_get_drvdata(dev); + + if (type == hwmon_chip) { + if (attr == hwmon_chip_update_interval) + return get_update_interval(regmap, val); + else + return -EOPNOTSUPP; + } + + switch (attr) { + case hwmon_temp_input: + if (channel == 0) + return get_local_temp(regmap, val); + else + return get_remote_temp(regmap, channel - 1, val); + case hwmon_temp_fault: + return get_fault(regmap, channel - 1, val); + case hwmon_temp_offset: + return get_offset(regmap, channel - 1, val); + default: + return -EOPNOTSUPP; + } +} + +static int w83773_write(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long val) +{ + struct regmap *regmap = dev_get_drvdata(dev); + + if (type == hwmon_chip && attr == hwmon_chip_update_interval) + return set_update_interval(regmap, val); + + if (type == hwmon_temp && attr == hwmon_temp_offset) + return set_offset(regmap, channel - 1, val); + + return -EOPNOTSUPP; +} + +static umode_t w83773_is_visible(const void *data, enum hwmon_sensor_types type, + u32 attr, int channel) +{ + switch (type) { + case hwmon_chip: + switch (attr) { + case hwmon_chip_update_interval: + return 0644; + } + break; + case hwmon_temp: + switch (attr) { + case hwmon_temp_input: + return 0444; + case hwmon_temp_fault: + return 0444; + case hwmon_temp_offset: + return 0644; + } + break; + default: + break; + } + return 0; +} + +static const u32 w83773_chip_config[] = { + HWMON_C_REGISTER_TZ | HWMON_C_UPDATE_INTERVAL, + 0 +}; + +static const struct hwmon_channel_info w83773_chip = { + .type = hwmon_chip, + .config = w83773_chip_config, +}; + +static const u32 w83773_temp_config[] = { + HWMON_T_INPUT, + HWMON_T_INPUT | HWMON_T_FAULT | HWMON_T_OFFSET, + HWMON_T_INPUT | HWMON_T_FAULT | HWMON_T_OFFSET, + 0 +}; + +static const struct hwmon_channel_info w83773_temp = { + .type = hwmon_temp, + .config = w83773_temp_config, +}; + +static const struct hwmon_channel_info *w83773_info[] = { + &w83773_chip, + &w83773_temp, + NULL +}; + +static const struct hwmon_ops w83773_ops = { + .is_visible = w83773_is_visible, + .read = w83773_read, + .write = w83773_write, +}; + +static const struct hwmon_chip_info w83773_chip_info = { + .ops = &w83773_ops, + .info = w83773_info, +}; + +static const struct regmap_config w83773_regmap_config = { + .reg_bits = 8, + .val_bits = 8, +}; + +static int w83773_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct device *dev = &client->dev; + struct device *hwmon_dev; + struct regmap *regmap; + int ret; + + regmap = devm_regmap_init_i2c(client, &w83773_regmap_config); + if (IS_ERR(regmap)) { + dev_err(dev, "failed to allocate register map\n"); + return PTR_ERR(regmap); + } + + /* Set the conversion rate to 2 Hz */ + ret = regmap_write(regmap, W83773_CONVERSION_RATE_REG_WRITE, 0x05); + if (ret < 0) { + dev_err(&client->dev, "error writing config rate register\n"); + return ret; + } + + i2c_set_clientdata(client, regmap); + + hwmon_dev = devm_hwmon_device_register_with_info(dev, + client->name, + regmap, + &w83773_chip_info, + NULL); + return PTR_ERR_OR_ZERO(hwmon_dev); +} + +static struct i2c_driver w83773_driver = { + .class = I2C_CLASS_HWMON, + .driver = { + .name = "w83773g", + .of_match_table = of_match_ptr(w83773_of_match), + }, + .probe = w83773_probe, + .id_table = w83773_id, + .address_list = normal_i2c, +}; + +module_i2c_driver(w83773_driver); + +MODULE_AUTHOR("Lei YU <mine260309@gmail.com>"); +MODULE_DESCRIPTION("W83773G temperature sensor driver"); +MODULE_LICENSE("GPL"); -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/3] drivers: hwmon: Add W83773G driver 2017-11-09 7:09 ` [PATCH v3 2/3] drivers: hwmon: Add W83773G driver Lei YU @ 2017-11-10 14:49 ` Guenter Roeck 2017-11-13 2:52 ` Lei YU 0 siblings, 1 reply; 8+ messages in thread From: Guenter Roeck @ 2017-11-10 14:49 UTC (permalink / raw) To: Lei YU, Mark Rutland, Jean Delvare, Jonathan Corbet, Jiri Kosina Cc: devicetree, linux-kernel, linux-hwmon, Joel Stanley, Andrew Jeffery On 11/08/2017 11:09 PM, Lei YU wrote: > Nuvoton W83773G is a hardware monitor IC providing one local > temperature and two remote temperature sensors. > > Signed-off-by: Lei YU <mine260309@gmail.com> Nicely done. Couple of nitpicks left. Thanks! Guenter > --- > v2: > - Rewrite the driver using regmap > - Add offset and update_interval > v3: > - Use devm_hwmon_device_register_with_info() with is_visible/read/write > functions. > --- > drivers/hwmon/Kconfig | 10 ++ > drivers/hwmon/Makefile | 1 + > drivers/hwmon/w83773g.c | 348 ++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 359 insertions(+) > create mode 100644 drivers/hwmon/w83773g.c > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index d654314..11c6248 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -1710,6 +1710,16 @@ config SENSORS_VT8231 > This driver can also be built as a module. If so, the module > will be called vt8231. > > +config SENSORS_W83773G > + tristate "Nuvoton W83773G" > + depends on I2C > + help > + If you say yes here you get support for the Nuvoton W83773G hardware > + monitoring chip. > + > + This driver can also be built as a module. If so, the module > + will be called w83773g. > + > config SENSORS_W83781D > tristate "Winbond W83781D, W83782D, W83783S, Asus AS99127F" > depends on I2C > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index c84d978..0649ad8 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -13,6 +13,7 @@ obj-$(CONFIG_SENSORS_ATK0110) += asus_atk0110.o > # asb100, then w83781d go first, as they can override other drivers' addresses. > obj-$(CONFIG_SENSORS_ASB100) += asb100.o > obj-$(CONFIG_SENSORS_W83627HF) += w83627hf.o > +obj-$(CONFIG_SENSORS_W83773G) += w83773g.o > obj-$(CONFIG_SENSORS_W83792D) += w83792d.o > obj-$(CONFIG_SENSORS_W83793) += w83793.o > obj-$(CONFIG_SENSORS_W83795) += w83795.o > diff --git a/drivers/hwmon/w83773g.c b/drivers/hwmon/w83773g.c > new file mode 100644 > index 0000000..58ac45b > --- /dev/null > +++ b/drivers/hwmon/w83773g.c > @@ -0,0 +1,348 @@ > +/* > + * Copyright (C) 2017 IBM Corp. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * Driver for the Nuvoton W83773G SMBus temperature sensor IC. > + * Supported models: W83773G > + */ > + > +#include <linux/module.h> > +#include <linux/init.h> > +#include <linux/i2c.h> > +#include <linux/hwmon.h> > +#include <linux/hwmon-sysfs.h> > +#include <linux/err.h> > +#include <linux/of_device.h> > +#include <linux/regmap.h> > + > +/* Addresses to scan */ > +static const unsigned short normal_i2c[] = { 0x4c, 0x4d, I2C_CLIENT_END }; > + > +/* W83773 has 3 channels */ > +#define W83773_CHANNELS 3 > + > +/* The W83773 registers */ > +#define W83773_CONVERSION_RATE_REG_READ 0x04 > +#define W83773_CONVERSION_RATE_REG_WRITE 0x0A > +#define W83773_MANUFACTURER_ID_REG 0xFE > +#define W83773_LOCAL_TEMP 0x00 > + > +static const u8 W83773_STATUS[2] = { 0x02, 0x17 }; > + > +static const u8 W83773_TEMP_LSB[2] = { 0x10, 0x25 }; > +static const u8 W83773_TEMP_MSB[2] = { 0x01, 0x24 }; > + > +static const u8 W83773_OFFSET_LSB[2] = { 0x12, 0x16 }; > +static const u8 W83773_OFFSET_MSB[2] = { 0x11, 0x15 }; > + > +/* this is the number of sensors in the device */ > +static const struct i2c_device_id w83773_id[] = { > + { "w83773g" }, > + { } > +}; > + > +MODULE_DEVICE_TABLE(i2c, w83773_id); > + > +static const struct of_device_id w83773_of_match[] = { > + { > + .compatible = "nuvoton,w83773g" > + }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, w83773_of_match); > + > +static inline long temp_of_local(s8 reg) > +{ > + return reg * 1000; > +} > + > +static inline long temp_of_remote(s8 hb, u8 lb) > +{ > + return (hb << 3 | lb >> 5) * 125; > +} > + > +static int get_local_temp(struct regmap *regmap, long *val) > +{ > + unsigned int regval; > + int ret; > + > + ret = regmap_read(regmap, W83773_LOCAL_TEMP, ®val); > + if (ret < 0) > + return ret; > + > + *val = temp_of_local(regval); > + return 0; > +} > + > +static int get_remote_temp(struct regmap *regmap, int index, long *val) > +{ > + unsigned int regval_high; > + unsigned int regval_low; > + int ret; > + > + ret = regmap_read(regmap, W83773_TEMP_MSB[index], ®val_high); > + if (ret < 0) > + return ret; > + > + ret = regmap_read(regmap, W83773_TEMP_LSB[index], ®val_low); > + if (ret < 0) > + return ret; > + > + *val = temp_of_remote(regval_high, regval_low); > + return 0; > +} > + > +static int get_fault(struct regmap *regmap, int index, long *val) > +{ > + unsigned int regval; > + int ret; > + > + ret = regmap_read(regmap, W83773_STATUS[index], ®val); > + Drop the empty line please. > + if (ret < 0) > + return ret; > + > + *val = (u8)regval & 0x04 >> 2; > + return 0; > +} > + > +static int get_offset(struct regmap *regmap, int index, long *val) > +{ > + unsigned int regval_high; > + unsigned int regval_low; > + int ret; > + > + ret = regmap_read(regmap, W83773_OFFSET_MSB[index], ®val_high); > + if (ret < 0) > + return ret; > + > + ret = regmap_read(regmap, W83773_OFFSET_LSB[index], ®val_low); > + if (ret < 0) > + return ret; > + > + *val = temp_of_remote(regval_high, regval_low); > + return 0; > +} > + > +static int set_offset(struct regmap *regmap, int index, long val) > +{ > + int ret; > + u8 high_byte; > + u8 low_byte; > + > + val = clamp_val(val, -127825, 127825); > + /* offset value equals to (high_byte << 3 | low_byte >> 5) * 125 */ > + val /= 125; > + high_byte = val >> 3; > + low_byte = (val & 0x07) << 5; > + > + ret = regmap_write(regmap, W83773_OFFSET_MSB[index], high_byte); > + if (ret < 0) > + return ret; > + > + ret = regmap_write(regmap, W83773_OFFSET_LSB[index], low_byte); > + if (ret < 0) > + return ret; > + > + return 0; For the second call, return regmap_write(regmap, W83773_OFFSET_LSB[index], low_byte); > +} > + > +static int get_update_interval(struct regmap *regmap, long *val) > +{ > + unsigned int regval; > + int ret; > + > + ret = regmap_read(regmap, W83773_CONVERSION_RATE_REG_READ, ®val); > + if (ret < 0) > + return ret; > + > + *val = 16000 >> regval; > + return 0; > +} > + > +static int set_update_interval(struct regmap *regmap, long val) > +{ > + int ret; > + int rate; > + > + if (val <= 0) > + return -EINVAL; > + Not really needed since you are clamping below. For consistency I would suggest to drop it (otherwise 0 returns -EINVAL, 1 translates to 62). > + /* > + * For valid rates, interval can be calculated as > + * interval = (1 << (8 - rate)) * 62.5; > + * Rounded rate is therefore > + * rate = 8 - __fls(interval * 8 / (62.5 * 7)); > + * Use clamp_val() to avoid overflows, and to ensure valid input > + * for __fls. > + */ > + val = clamp_val(val, 62, 16000) * 10; > + rate = 8 - __fls((val * 8 / (625 * 7))); > + ret = regmap_write(regmap, W83773_CONVERSION_RATE_REG_WRITE, rate); > + if (ret < 0) > + return ret; > + return 0; just return regmap_write(regmap, W83773_CONVERSION_RATE_REG_WRITE, rate); > +} > + > +static int w83773_read(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long *val) > +{ > + struct regmap *regmap = dev_get_drvdata(dev); > + > + if (type == hwmon_chip) { > + if (attr == hwmon_chip_update_interval) > + return get_update_interval(regmap, val); > + else else not needed after return > + return -EOPNOTSUPP; > + } > + > + switch (attr) { > + case hwmon_temp_input: > + if (channel == 0) > + return get_local_temp(regmap, val); > + else else not needed after return > + return get_remote_temp(regmap, channel - 1, val); > + case hwmon_temp_fault: > + return get_fault(regmap, channel - 1, val); > + case hwmon_temp_offset: > + return get_offset(regmap, channel - 1, val); > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static int w83773_write(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long val) > +{ > + struct regmap *regmap = dev_get_drvdata(dev); > + > + if (type == hwmon_chip && attr == hwmon_chip_update_interval) > + return set_update_interval(regmap, val); > + > + if (type == hwmon_temp && attr == hwmon_temp_offset) > + return set_offset(regmap, channel - 1, val); > + > + return -EOPNOTSUPP; > +} > + > +static umode_t w83773_is_visible(const void *data, enum hwmon_sensor_types type, > + u32 attr, int channel) > +{ > + switch (type) { > + case hwmon_chip: > + switch (attr) { > + case hwmon_chip_update_interval: > + return 0644; > + } > + break; > + case hwmon_temp: > + switch (attr) { > + case hwmon_temp_input: > + return 0444; > + case hwmon_temp_fault: The above case statements can be combined. > + return 0444; > + case hwmon_temp_offset: > + return 0644; Wonder if static analyzers complain here and above because of the missing default: case. Any idea ? > + } > + break; > + default: > + break; > + } > + return 0; > +} > + > +static const u32 w83773_chip_config[] = { > + HWMON_C_REGISTER_TZ | HWMON_C_UPDATE_INTERVAL, > + 0 > +}; > + > +static const struct hwmon_channel_info w83773_chip = { > + .type = hwmon_chip, > + .config = w83773_chip_config, > +}; > + > +static const u32 w83773_temp_config[] = { > + HWMON_T_INPUT, > + HWMON_T_INPUT | HWMON_T_FAULT | HWMON_T_OFFSET, > + HWMON_T_INPUT | HWMON_T_FAULT | HWMON_T_OFFSET, > + 0 > +}; > + > +static const struct hwmon_channel_info w83773_temp = { > + .type = hwmon_temp, > + .config = w83773_temp_config, > +}; > + > +static const struct hwmon_channel_info *w83773_info[] = { > + &w83773_chip, > + &w83773_temp, > + NULL > +}; > + > +static const struct hwmon_ops w83773_ops = { > + .is_visible = w83773_is_visible, > + .read = w83773_read, > + .write = w83773_write, > +}; > + > +static const struct hwmon_chip_info w83773_chip_info = { > + .ops = &w83773_ops, > + .info = w83773_info, > +}; > + > +static const struct regmap_config w83773_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > +}; > + > +static int w83773_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct device *dev = &client->dev; > + struct device *hwmon_dev; > + struct regmap *regmap; > + int ret; > + > + regmap = devm_regmap_init_i2c(client, &w83773_regmap_config); > + if (IS_ERR(regmap)) { > + dev_err(dev, "failed to allocate register map\n"); > + return PTR_ERR(regmap); > + } > + > + /* Set the conversion rate to 2 Hz */ > + ret = regmap_write(regmap, W83773_CONVERSION_RATE_REG_WRITE, 0x05); > + if (ret < 0) { > + dev_err(&client->dev, "error writing config rate register\n"); > + return ret; > + } > + > + i2c_set_clientdata(client, regmap); > + > + hwmon_dev = devm_hwmon_device_register_with_info(dev, > + client->name, > + regmap, > + &w83773_chip_info, > + NULL); > + return PTR_ERR_OR_ZERO(hwmon_dev); > +} > + > +static struct i2c_driver w83773_driver = { > + .class = I2C_CLASS_HWMON, > + .driver = { > + .name = "w83773g", > + .of_match_table = of_match_ptr(w83773_of_match), > + }, > + .probe = w83773_probe, > + .id_table = w83773_id, > + .address_list = normal_i2c, address_list is only used if there is a detect function, so you can drop it. > +}; > + > +module_i2c_driver(w83773_driver); > + > +MODULE_AUTHOR("Lei YU <mine260309@gmail.com>"); > +MODULE_DESCRIPTION("W83773G temperature sensor driver"); > +MODULE_LICENSE("GPL"); > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/3] drivers: hwmon: Add W83773G driver @ 2017-11-13 2:52 ` Lei YU 0 siblings, 0 replies; 8+ messages in thread From: Lei YU @ 2017-11-13 2:52 UTC (permalink / raw) To: Guenter Roeck Cc: Mark Rutland, Jean Delvare, Jonathan Corbet, Jiri Kosina, devicetree, linux-kernel, linux-hwmon, Joel Stanley, Andrew Jeffery On Fri, Nov 10, 2017 at 10:49 PM, Guenter Roeck <linux@roeck-us.net> wrote: > On 11/08/2017 11:09 PM, Lei YU wrote: >> >> Nuvoton W83773G is a hardware monitor IC providing one local >> temperature and two remote temperature sensors. >> >> Signed-off-by: Lei YU <mine260309@gmail.com> > > > Nicely done. Couple of nitpicks left. > > Thanks! > Guenter > > >> --- >> v2: >> - Rewrite the driver using regmap >> - Add offset and update_interval >> v3: >> - Use devm_hwmon_device_register_with_info() with is_visible/read/write >> functions. >> --- >> drivers/hwmon/Kconfig | 10 ++ >> drivers/hwmon/Makefile | 1 + >> drivers/hwmon/w83773g.c | 348 >> ++++++++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 359 insertions(+) >> create mode 100644 drivers/hwmon/w83773g.c >> >> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig >> index d654314..11c6248 100644 >> --- a/drivers/hwmon/Kconfig >> +++ b/drivers/hwmon/Kconfig >> @@ -1710,6 +1710,16 @@ config SENSORS_VT8231 >> This driver can also be built as a module. If so, the module >> will be called vt8231. >> +config SENSORS_W83773G >> + tristate "Nuvoton W83773G" >> + depends on I2C >> + help >> + If you say yes here you get support for the Nuvoton W83773G >> hardware >> + monitoring chip. >> + >> + This driver can also be built as a module. If so, the module >> + will be called w83773g. >> + >> config SENSORS_W83781D >> tristate "Winbond W83781D, W83782D, W83783S, Asus AS99127F" >> depends on I2C >> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile >> index c84d978..0649ad8 100644 >> --- a/drivers/hwmon/Makefile >> +++ b/drivers/hwmon/Makefile >> @@ -13,6 +13,7 @@ obj-$(CONFIG_SENSORS_ATK0110) += asus_atk0110.o >> # asb100, then w83781d go first, as they can override other drivers' >> addresses. >> obj-$(CONFIG_SENSORS_ASB100) += asb100.o >> obj-$(CONFIG_SENSORS_W83627HF) += w83627hf.o >> +obj-$(CONFIG_SENSORS_W83773G) += w83773g.o >> obj-$(CONFIG_SENSORS_W83792D) += w83792d.o >> obj-$(CONFIG_SENSORS_W83793) += w83793.o >> obj-$(CONFIG_SENSORS_W83795) += w83795.o >> diff --git a/drivers/hwmon/w83773g.c b/drivers/hwmon/w83773g.c >> new file mode 100644 >> index 0000000..58ac45b >> --- /dev/null >> +++ b/drivers/hwmon/w83773g.c >> @@ -0,0 +1,348 @@ >> +/* >> + * Copyright (C) 2017 IBM Corp. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * Driver for the Nuvoton W83773G SMBus temperature sensor IC. >> + * Supported models: W83773G >> + */ >> + >> +#include <linux/module.h> >> +#include <linux/init.h> >> +#include <linux/i2c.h> >> +#include <linux/hwmon.h> >> +#include <linux/hwmon-sysfs.h> >> +#include <linux/err.h> >> +#include <linux/of_device.h> >> +#include <linux/regmap.h> >> + >> +/* Addresses to scan */ >> +static const unsigned short normal_i2c[] = { 0x4c, 0x4d, I2C_CLIENT_END >> }; >> + >> +/* W83773 has 3 channels */ >> +#define W83773_CHANNELS 3 >> + >> +/* The W83773 registers */ >> +#define W83773_CONVERSION_RATE_REG_READ 0x04 >> +#define W83773_CONVERSION_RATE_REG_WRITE 0x0A >> +#define W83773_MANUFACTURER_ID_REG 0xFE >> +#define W83773_LOCAL_TEMP 0x00 >> + >> +static const u8 W83773_STATUS[2] = { 0x02, 0x17 }; >> + >> +static const u8 W83773_TEMP_LSB[2] = { 0x10, 0x25 }; >> +static const u8 W83773_TEMP_MSB[2] = { 0x01, 0x24 }; >> + >> +static const u8 W83773_OFFSET_LSB[2] = { 0x12, 0x16 }; >> +static const u8 W83773_OFFSET_MSB[2] = { 0x11, 0x15 }; >> + >> +/* this is the number of sensors in the device */ >> +static const struct i2c_device_id w83773_id[] = { >> + { "w83773g" }, >> + { } >> +}; >> + >> +MODULE_DEVICE_TABLE(i2c, w83773_id); >> + >> +static const struct of_device_id w83773_of_match[] = { >> + { >> + .compatible = "nuvoton,w83773g" >> + }, >> + { }, >> +}; >> +MODULE_DEVICE_TABLE(of, w83773_of_match); >> + >> +static inline long temp_of_local(s8 reg) >> +{ >> + return reg * 1000; >> +} >> + >> +static inline long temp_of_remote(s8 hb, u8 lb) >> +{ >> + return (hb << 3 | lb >> 5) * 125; >> +} >> + >> +static int get_local_temp(struct regmap *regmap, long *val) >> +{ >> + unsigned int regval; >> + int ret; >> + >> + ret = regmap_read(regmap, W83773_LOCAL_TEMP, ®val); >> + if (ret < 0) >> + return ret; >> + >> + *val = temp_of_local(regval); >> + return 0; >> +} >> + >> +static int get_remote_temp(struct regmap *regmap, int index, long *val) >> +{ >> + unsigned int regval_high; >> + unsigned int regval_low; >> + int ret; >> + >> + ret = regmap_read(regmap, W83773_TEMP_MSB[index], ®val_high); >> + if (ret < 0) >> + return ret; >> + >> + ret = regmap_read(regmap, W83773_TEMP_LSB[index], ®val_low); >> + if (ret < 0) >> + return ret; >> + >> + *val = temp_of_remote(regval_high, regval_low); >> + return 0; >> +} >> + >> +static int get_fault(struct regmap *regmap, int index, long *val) >> +{ >> + unsigned int regval; >> + int ret; >> + >> + ret = regmap_read(regmap, W83773_STATUS[index], ®val); >> + > > Drop the empty line please. > Will be fixed in v4. > >> + if (ret < 0) >> + return ret; >> + >> + *val = (u8)regval & 0x04 >> 2; >> + return 0; >> +} >> + >> +static int get_offset(struct regmap *regmap, int index, long *val) >> +{ >> + unsigned int regval_high; >> + unsigned int regval_low; >> + int ret; >> + >> + ret = regmap_read(regmap, W83773_OFFSET_MSB[index], ®val_high); >> + if (ret < 0) >> + return ret; >> + >> + ret = regmap_read(regmap, W83773_OFFSET_LSB[index], ®val_low); >> + if (ret < 0) >> + return ret; >> + >> + *val = temp_of_remote(regval_high, regval_low); >> + return 0; >> +} >> + >> +static int set_offset(struct regmap *regmap, int index, long val) >> +{ >> + int ret; >> + u8 high_byte; >> + u8 low_byte; >> + >> + val = clamp_val(val, -127825, 127825); >> + /* offset value equals to (high_byte << 3 | low_byte >> 5) * 125 >> */ >> + val /= 125; >> + high_byte = val >> 3; >> + low_byte = (val & 0x07) << 5; >> + >> + ret = regmap_write(regmap, W83773_OFFSET_MSB[index], high_byte); >> + if (ret < 0) >> + return ret; >> + >> + ret = regmap_write(regmap, W83773_OFFSET_LSB[index], low_byte); >> + if (ret < 0) >> + return ret; >> + >> + return 0; > > > For the second call, > > return regmap_write(regmap, W83773_OFFSET_LSB[index], low_byte); > Will be fixed in v4. >> +} >> + >> +static int get_update_interval(struct regmap *regmap, long *val) >> +{ >> + unsigned int regval; >> + int ret; >> + >> + ret = regmap_read(regmap, W83773_CONVERSION_RATE_REG_READ, >> ®val); >> + if (ret < 0) >> + return ret; >> + >> + *val = 16000 >> regval; >> + return 0; >> +} >> + >> +static int set_update_interval(struct regmap *regmap, long val) >> +{ >> + int ret; >> + int rate; >> + >> + if (val <= 0) >> + return -EINVAL; >> + > > Not really needed since you are clamping below. For consistency I would > suggest > to drop it (otherwise 0 returns -EINVAL, 1 translates to 62). > My initial idea is that user should not set a value equal or less than 0, because it does not make sense. And thus user get EINVAL for values <= 0, and get interval 62 if he sets a value 1. But yes, it can be removed for consistency, so user always get 62 for values less than 62. Will be fixed in v4. >> + /* >> + * For valid rates, interval can be calculated as >> + * interval = (1 << (8 - rate)) * 62.5; >> + * Rounded rate is therefore >> + * rate = 8 - __fls(interval * 8 / (62.5 * 7)); >> + * Use clamp_val() to avoid overflows, and to ensure valid input >> + * for __fls. >> + */ >> + val = clamp_val(val, 62, 16000) * 10; >> + rate = 8 - __fls((val * 8 / (625 * 7))); >> + ret = regmap_write(regmap, W83773_CONVERSION_RATE_REG_WRITE, >> rate); >> + if (ret < 0) >> + return ret; >> + return 0; > > > just > return regmap_write(regmap, W83773_CONVERSION_RATE_REG_WRITE, rate); > Will be fixed in v4. >> +} >> + >> +static int w83773_read(struct device *dev, enum hwmon_sensor_types type, >> + u32 attr, int channel, long *val) >> +{ >> + struct regmap *regmap = dev_get_drvdata(dev); >> + >> + if (type == hwmon_chip) { >> + if (attr == hwmon_chip_update_interval) >> + return get_update_interval(regmap, val); >> + else > > > else not needed after return > Will be fixed in v4. >> + return -EOPNOTSUPP; >> + } >> + >> + switch (attr) { >> + case hwmon_temp_input: >> + if (channel == 0) >> + return get_local_temp(regmap, val); >> + else > > > else not needed after return > Will be fixed in v4. > >> + return get_remote_temp(regmap, channel - 1, val); >> + case hwmon_temp_fault: >> + return get_fault(regmap, channel - 1, val); >> + case hwmon_temp_offset: >> + return get_offset(regmap, channel - 1, val); >> + default: >> + return -EOPNOTSUPP; >> + } >> +} >> + >> +static int w83773_write(struct device *dev, enum hwmon_sensor_types type, >> + u32 attr, int channel, long val) >> +{ >> + struct regmap *regmap = dev_get_drvdata(dev); >> + >> + if (type == hwmon_chip && attr == hwmon_chip_update_interval) >> + return set_update_interval(regmap, val); >> + >> + if (type == hwmon_temp && attr == hwmon_temp_offset) >> + return set_offset(regmap, channel - 1, val); >> + >> + return -EOPNOTSUPP; >> +} >> + >> +static umode_t w83773_is_visible(const void *data, enum >> hwmon_sensor_types type, >> + u32 attr, int channel) >> +{ >> + switch (type) { >> + case hwmon_chip: >> + switch (attr) { >> + case hwmon_chip_update_interval: >> + return 0644; >> + } >> + break; >> + case hwmon_temp: >> + switch (attr) { >> + case hwmon_temp_input: >> + return 0444; >> + case hwmon_temp_fault: > > > The above case statements can be combined. Will be fixed in v4. > >> + return 0444; >> + case hwmon_temp_offset: >> + return 0644; > > > Wonder if static analyzers complain here and above because > of the missing default: case. Any idea ? hmm, I did not use static analyzer to parse this code so did not know. My idea is that it's safe here because it always goes to the "return 0" below so it is safe here. > > >> + } >> + break; >> + default: >> + break; >> + } >> + return 0; >> +} >> + >> +static const u32 w83773_chip_config[] = { >> + HWMON_C_REGISTER_TZ | HWMON_C_UPDATE_INTERVAL, >> + 0 >> +}; >> + >> +static const struct hwmon_channel_info w83773_chip = { >> + .type = hwmon_chip, >> + .config = w83773_chip_config, >> +}; >> + >> +static const u32 w83773_temp_config[] = { >> + HWMON_T_INPUT, >> + HWMON_T_INPUT | HWMON_T_FAULT | HWMON_T_OFFSET, >> + HWMON_T_INPUT | HWMON_T_FAULT | HWMON_T_OFFSET, >> + 0 >> +}; >> + >> +static const struct hwmon_channel_info w83773_temp = { >> + .type = hwmon_temp, >> + .config = w83773_temp_config, >> +}; >> + >> +static const struct hwmon_channel_info *w83773_info[] = { >> + &w83773_chip, >> + &w83773_temp, >> + NULL >> +}; >> + >> +static const struct hwmon_ops w83773_ops = { >> + .is_visible = w83773_is_visible, >> + .read = w83773_read, >> + .write = w83773_write, >> +}; >> + >> +static const struct hwmon_chip_info w83773_chip_info = { >> + .ops = &w83773_ops, >> + .info = w83773_info, >> +}; >> + >> +static const struct regmap_config w83773_regmap_config = { >> + .reg_bits = 8, >> + .val_bits = 8, >> +}; >> + >> +static int w83773_probe(struct i2c_client *client, >> + const struct i2c_device_id *id) >> +{ >> + struct device *dev = &client->dev; >> + struct device *hwmon_dev; >> + struct regmap *regmap; >> + int ret; >> + >> + regmap = devm_regmap_init_i2c(client, &w83773_regmap_config); >> + if (IS_ERR(regmap)) { >> + dev_err(dev, "failed to allocate register map\n"); >> + return PTR_ERR(regmap); >> + } >> + >> + /* Set the conversion rate to 2 Hz */ >> + ret = regmap_write(regmap, W83773_CONVERSION_RATE_REG_WRITE, >> 0x05); >> + if (ret < 0) { >> + dev_err(&client->dev, "error writing config rate >> register\n"); >> + return ret; >> + } >> + >> + i2c_set_clientdata(client, regmap); >> + >> + hwmon_dev = devm_hwmon_device_register_with_info(dev, >> + client->name, >> + regmap, >> + >> &w83773_chip_info, >> + NULL); >> + return PTR_ERR_OR_ZERO(hwmon_dev); >> +} >> + >> +static struct i2c_driver w83773_driver = { >> + .class = I2C_CLASS_HWMON, >> + .driver = { >> + .name = "w83773g", >> + .of_match_table = of_match_ptr(w83773_of_match), >> + }, >> + .probe = w83773_probe, >> + .id_table = w83773_id, >> + .address_list = normal_i2c, > > > address_list is only used if there is a detect function, so you can drop it. > Will be fixed in v4. > >> +}; >> + >> +module_i2c_driver(w83773_driver); >> + >> +MODULE_AUTHOR("Lei YU <mine260309@gmail.com>"); >> +MODULE_DESCRIPTION("W83773G temperature sensor driver"); >> +MODULE_LICENSE("GPL"); >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/3] drivers: hwmon: Add W83773G driver @ 2017-11-13 2:52 ` Lei YU 0 siblings, 0 replies; 8+ messages in thread From: Lei YU @ 2017-11-13 2:52 UTC (permalink / raw) To: Guenter Roeck Cc: Mark Rutland, Jean Delvare, Jonathan Corbet, Jiri Kosina, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-hwmon-u79uwXL29TY76Z2rM5mHXA, Joel Stanley, Andrew Jeffery On Fri, Nov 10, 2017 at 10:49 PM, Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org> wrote: > On 11/08/2017 11:09 PM, Lei YU wrote: >> >> Nuvoton W83773G is a hardware monitor IC providing one local >> temperature and two remote temperature sensors. >> >> Signed-off-by: Lei YU <mine260309-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > > > Nicely done. Couple of nitpicks left. > > Thanks! > Guenter > > >> --- >> v2: >> - Rewrite the driver using regmap >> - Add offset and update_interval >> v3: >> - Use devm_hwmon_device_register_with_info() with is_visible/read/write >> functions. >> --- >> drivers/hwmon/Kconfig | 10 ++ >> drivers/hwmon/Makefile | 1 + >> drivers/hwmon/w83773g.c | 348 >> ++++++++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 359 insertions(+) >> create mode 100644 drivers/hwmon/w83773g.c >> >> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig >> index d654314..11c6248 100644 >> --- a/drivers/hwmon/Kconfig >> +++ b/drivers/hwmon/Kconfig >> @@ -1710,6 +1710,16 @@ config SENSORS_VT8231 >> This driver can also be built as a module. If so, the module >> will be called vt8231. >> +config SENSORS_W83773G >> + tristate "Nuvoton W83773G" >> + depends on I2C >> + help >> + If you say yes here you get support for the Nuvoton W83773G >> hardware >> + monitoring chip. >> + >> + This driver can also be built as a module. If so, the module >> + will be called w83773g. >> + >> config SENSORS_W83781D >> tristate "Winbond W83781D, W83782D, W83783S, Asus AS99127F" >> depends on I2C >> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile >> index c84d978..0649ad8 100644 >> --- a/drivers/hwmon/Makefile >> +++ b/drivers/hwmon/Makefile >> @@ -13,6 +13,7 @@ obj-$(CONFIG_SENSORS_ATK0110) += asus_atk0110.o >> # asb100, then w83781d go first, as they can override other drivers' >> addresses. >> obj-$(CONFIG_SENSORS_ASB100) += asb100.o >> obj-$(CONFIG_SENSORS_W83627HF) += w83627hf.o >> +obj-$(CONFIG_SENSORS_W83773G) += w83773g.o >> obj-$(CONFIG_SENSORS_W83792D) += w83792d.o >> obj-$(CONFIG_SENSORS_W83793) += w83793.o >> obj-$(CONFIG_SENSORS_W83795) += w83795.o >> diff --git a/drivers/hwmon/w83773g.c b/drivers/hwmon/w83773g.c >> new file mode 100644 >> index 0000000..58ac45b >> --- /dev/null >> +++ b/drivers/hwmon/w83773g.c >> @@ -0,0 +1,348 @@ >> +/* >> + * Copyright (C) 2017 IBM Corp. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * Driver for the Nuvoton W83773G SMBus temperature sensor IC. >> + * Supported models: W83773G >> + */ >> + >> +#include <linux/module.h> >> +#include <linux/init.h> >> +#include <linux/i2c.h> >> +#include <linux/hwmon.h> >> +#include <linux/hwmon-sysfs.h> >> +#include <linux/err.h> >> +#include <linux/of_device.h> >> +#include <linux/regmap.h> >> + >> +/* Addresses to scan */ >> +static const unsigned short normal_i2c[] = { 0x4c, 0x4d, I2C_CLIENT_END >> }; >> + >> +/* W83773 has 3 channels */ >> +#define W83773_CHANNELS 3 >> + >> +/* The W83773 registers */ >> +#define W83773_CONVERSION_RATE_REG_READ 0x04 >> +#define W83773_CONVERSION_RATE_REG_WRITE 0x0A >> +#define W83773_MANUFACTURER_ID_REG 0xFE >> +#define W83773_LOCAL_TEMP 0x00 >> + >> +static const u8 W83773_STATUS[2] = { 0x02, 0x17 }; >> + >> +static const u8 W83773_TEMP_LSB[2] = { 0x10, 0x25 }; >> +static const u8 W83773_TEMP_MSB[2] = { 0x01, 0x24 }; >> + >> +static const u8 W83773_OFFSET_LSB[2] = { 0x12, 0x16 }; >> +static const u8 W83773_OFFSET_MSB[2] = { 0x11, 0x15 }; >> + >> +/* this is the number of sensors in the device */ >> +static const struct i2c_device_id w83773_id[] = { >> + { "w83773g" }, >> + { } >> +}; >> + >> +MODULE_DEVICE_TABLE(i2c, w83773_id); >> + >> +static const struct of_device_id w83773_of_match[] = { >> + { >> + .compatible = "nuvoton,w83773g" >> + }, >> + { }, >> +}; >> +MODULE_DEVICE_TABLE(of, w83773_of_match); >> + >> +static inline long temp_of_local(s8 reg) >> +{ >> + return reg * 1000; >> +} >> + >> +static inline long temp_of_remote(s8 hb, u8 lb) >> +{ >> + return (hb << 3 | lb >> 5) * 125; >> +} >> + >> +static int get_local_temp(struct regmap *regmap, long *val) >> +{ >> + unsigned int regval; >> + int ret; >> + >> + ret = regmap_read(regmap, W83773_LOCAL_TEMP, ®val); >> + if (ret < 0) >> + return ret; >> + >> + *val = temp_of_local(regval); >> + return 0; >> +} >> + >> +static int get_remote_temp(struct regmap *regmap, int index, long *val) >> +{ >> + unsigned int regval_high; >> + unsigned int regval_low; >> + int ret; >> + >> + ret = regmap_read(regmap, W83773_TEMP_MSB[index], ®val_high); >> + if (ret < 0) >> + return ret; >> + >> + ret = regmap_read(regmap, W83773_TEMP_LSB[index], ®val_low); >> + if (ret < 0) >> + return ret; >> + >> + *val = temp_of_remote(regval_high, regval_low); >> + return 0; >> +} >> + >> +static int get_fault(struct regmap *regmap, int index, long *val) >> +{ >> + unsigned int regval; >> + int ret; >> + >> + ret = regmap_read(regmap, W83773_STATUS[index], ®val); >> + > > Drop the empty line please. > Will be fixed in v4. > >> + if (ret < 0) >> + return ret; >> + >> + *val = (u8)regval & 0x04 >> 2; >> + return 0; >> +} >> + >> +static int get_offset(struct regmap *regmap, int index, long *val) >> +{ >> + unsigned int regval_high; >> + unsigned int regval_low; >> + int ret; >> + >> + ret = regmap_read(regmap, W83773_OFFSET_MSB[index], ®val_high); >> + if (ret < 0) >> + return ret; >> + >> + ret = regmap_read(regmap, W83773_OFFSET_LSB[index], ®val_low); >> + if (ret < 0) >> + return ret; >> + >> + *val = temp_of_remote(regval_high, regval_low); >> + return 0; >> +} >> + >> +static int set_offset(struct regmap *regmap, int index, long val) >> +{ >> + int ret; >> + u8 high_byte; >> + u8 low_byte; >> + >> + val = clamp_val(val, -127825, 127825); >> + /* offset value equals to (high_byte << 3 | low_byte >> 5) * 125 >> */ >> + val /= 125; >> + high_byte = val >> 3; >> + low_byte = (val & 0x07) << 5; >> + >> + ret = regmap_write(regmap, W83773_OFFSET_MSB[index], high_byte); >> + if (ret < 0) >> + return ret; >> + >> + ret = regmap_write(regmap, W83773_OFFSET_LSB[index], low_byte); >> + if (ret < 0) >> + return ret; >> + >> + return 0; > > > For the second call, > > return regmap_write(regmap, W83773_OFFSET_LSB[index], low_byte); > Will be fixed in v4. >> +} >> + >> +static int get_update_interval(struct regmap *regmap, long *val) >> +{ >> + unsigned int regval; >> + int ret; >> + >> + ret = regmap_read(regmap, W83773_CONVERSION_RATE_REG_READ, >> ®val); >> + if (ret < 0) >> + return ret; >> + >> + *val = 16000 >> regval; >> + return 0; >> +} >> + >> +static int set_update_interval(struct regmap *regmap, long val) >> +{ >> + int ret; >> + int rate; >> + >> + if (val <= 0) >> + return -EINVAL; >> + > > Not really needed since you are clamping below. For consistency I would > suggest > to drop it (otherwise 0 returns -EINVAL, 1 translates to 62). > My initial idea is that user should not set a value equal or less than 0, because it does not make sense. And thus user get EINVAL for values <= 0, and get interval 62 if he sets a value 1. But yes, it can be removed for consistency, so user always get 62 for values less than 62. Will be fixed in v4. >> + /* >> + * For valid rates, interval can be calculated as >> + * interval = (1 << (8 - rate)) * 62.5; >> + * Rounded rate is therefore >> + * rate = 8 - __fls(interval * 8 / (62.5 * 7)); >> + * Use clamp_val() to avoid overflows, and to ensure valid input >> + * for __fls. >> + */ >> + val = clamp_val(val, 62, 16000) * 10; >> + rate = 8 - __fls((val * 8 / (625 * 7))); >> + ret = regmap_write(regmap, W83773_CONVERSION_RATE_REG_WRITE, >> rate); >> + if (ret < 0) >> + return ret; >> + return 0; > > > just > return regmap_write(regmap, W83773_CONVERSION_RATE_REG_WRITE, rate); > Will be fixed in v4. >> +} >> + >> +static int w83773_read(struct device *dev, enum hwmon_sensor_types type, >> + u32 attr, int channel, long *val) >> +{ >> + struct regmap *regmap = dev_get_drvdata(dev); >> + >> + if (type == hwmon_chip) { >> + if (attr == hwmon_chip_update_interval) >> + return get_update_interval(regmap, val); >> + else > > > else not needed after return > Will be fixed in v4. >> + return -EOPNOTSUPP; >> + } >> + >> + switch (attr) { >> + case hwmon_temp_input: >> + if (channel == 0) >> + return get_local_temp(regmap, val); >> + else > > > else not needed after return > Will be fixed in v4. > >> + return get_remote_temp(regmap, channel - 1, val); >> + case hwmon_temp_fault: >> + return get_fault(regmap, channel - 1, val); >> + case hwmon_temp_offset: >> + return get_offset(regmap, channel - 1, val); >> + default: >> + return -EOPNOTSUPP; >> + } >> +} >> + >> +static int w83773_write(struct device *dev, enum hwmon_sensor_types type, >> + u32 attr, int channel, long val) >> +{ >> + struct regmap *regmap = dev_get_drvdata(dev); >> + >> + if (type == hwmon_chip && attr == hwmon_chip_update_interval) >> + return set_update_interval(regmap, val); >> + >> + if (type == hwmon_temp && attr == hwmon_temp_offset) >> + return set_offset(regmap, channel - 1, val); >> + >> + return -EOPNOTSUPP; >> +} >> + >> +static umode_t w83773_is_visible(const void *data, enum >> hwmon_sensor_types type, >> + u32 attr, int channel) >> +{ >> + switch (type) { >> + case hwmon_chip: >> + switch (attr) { >> + case hwmon_chip_update_interval: >> + return 0644; >> + } >> + break; >> + case hwmon_temp: >> + switch (attr) { >> + case hwmon_temp_input: >> + return 0444; >> + case hwmon_temp_fault: > > > The above case statements can be combined. Will be fixed in v4. > >> + return 0444; >> + case hwmon_temp_offset: >> + return 0644; > > > Wonder if static analyzers complain here and above because > of the missing default: case. Any idea ? hmm, I did not use static analyzer to parse this code so did not know. My idea is that it's safe here because it always goes to the "return 0" below so it is safe here. > > >> + } >> + break; >> + default: >> + break; >> + } >> + return 0; >> +} >> + >> +static const u32 w83773_chip_config[] = { >> + HWMON_C_REGISTER_TZ | HWMON_C_UPDATE_INTERVAL, >> + 0 >> +}; >> + >> +static const struct hwmon_channel_info w83773_chip = { >> + .type = hwmon_chip, >> + .config = w83773_chip_config, >> +}; >> + >> +static const u32 w83773_temp_config[] = { >> + HWMON_T_INPUT, >> + HWMON_T_INPUT | HWMON_T_FAULT | HWMON_T_OFFSET, >> + HWMON_T_INPUT | HWMON_T_FAULT | HWMON_T_OFFSET, >> + 0 >> +}; >> + >> +static const struct hwmon_channel_info w83773_temp = { >> + .type = hwmon_temp, >> + .config = w83773_temp_config, >> +}; >> + >> +static const struct hwmon_channel_info *w83773_info[] = { >> + &w83773_chip, >> + &w83773_temp, >> + NULL >> +}; >> + >> +static const struct hwmon_ops w83773_ops = { >> + .is_visible = w83773_is_visible, >> + .read = w83773_read, >> + .write = w83773_write, >> +}; >> + >> +static const struct hwmon_chip_info w83773_chip_info = { >> + .ops = &w83773_ops, >> + .info = w83773_info, >> +}; >> + >> +static const struct regmap_config w83773_regmap_config = { >> + .reg_bits = 8, >> + .val_bits = 8, >> +}; >> + >> +static int w83773_probe(struct i2c_client *client, >> + const struct i2c_device_id *id) >> +{ >> + struct device *dev = &client->dev; >> + struct device *hwmon_dev; >> + struct regmap *regmap; >> + int ret; >> + >> + regmap = devm_regmap_init_i2c(client, &w83773_regmap_config); >> + if (IS_ERR(regmap)) { >> + dev_err(dev, "failed to allocate register map\n"); >> + return PTR_ERR(regmap); >> + } >> + >> + /* Set the conversion rate to 2 Hz */ >> + ret = regmap_write(regmap, W83773_CONVERSION_RATE_REG_WRITE, >> 0x05); >> + if (ret < 0) { >> + dev_err(&client->dev, "error writing config rate >> register\n"); >> + return ret; >> + } >> + >> + i2c_set_clientdata(client, regmap); >> + >> + hwmon_dev = devm_hwmon_device_register_with_info(dev, >> + client->name, >> + regmap, >> + >> &w83773_chip_info, >> + NULL); >> + return PTR_ERR_OR_ZERO(hwmon_dev); >> +} >> + >> +static struct i2c_driver w83773_driver = { >> + .class = I2C_CLASS_HWMON, >> + .driver = { >> + .name = "w83773g", >> + .of_match_table = of_match_ptr(w83773_of_match), >> + }, >> + .probe = w83773_probe, >> + .id_table = w83773_id, >> + .address_list = normal_i2c, > > > address_list is only used if there is a detect function, so you can drop it. > Will be fixed in v4. > >> +}; >> + >> +module_i2c_driver(w83773_driver); >> + >> +MODULE_AUTHOR("Lei YU <mine260309-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>"); >> +MODULE_DESCRIPTION("W83773G temperature sensor driver"); >> +MODULE_LICENSE("GPL"); >> > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 3/3] hwmon: (w83773g) Add documentation 2017-11-09 7:08 ` Lei YU ` (2 preceding siblings ...) (?) @ 2017-11-09 7:09 ` Lei YU -1 siblings, 0 replies; 8+ messages in thread From: Lei YU @ 2017-11-09 7:09 UTC (permalink / raw) To: Mark Rutland, Jean Delvare, Guenter Roeck, Jonathan Corbet, Jiri Kosina Cc: Lei YU, devicetree, linux-kernel, linux-hwmon, Joel Stanley, Andrew Jeffery Add documentation for the w83773g driver. Signed-off-by: Lei YU <mine260309@gmail.com> --- v2: - Add notes for offset and update_interval --- Documentation/hwmon/w83773g | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 Documentation/hwmon/w83773g diff --git a/Documentation/hwmon/w83773g b/Documentation/hwmon/w83773g new file mode 100644 index 0000000..4cc6c0b --- /dev/null +++ b/Documentation/hwmon/w83773g @@ -0,0 +1,33 @@ +Kernel driver w83773g +==================== + +Supported chips: + * Nuvoton W83773G + Prefix: 'w83773g' + Addresses scanned: I2C 0x4c and 0x4d + Datasheet: https://www.nuvoton.com/resource-files/W83773G_SG_DatasheetV1_2.pdf + +Authors: + Lei YU <mine260309@gmail.com> + +Description +----------- + +This driver implements support for Nuvoton W83773G temperature sensor +chip. This chip implements one local and two remote sensors. +The chip also features offsets for the two remote sensors which get added to +the input readings. The chip does all the scaling by itself and the driver +therefore reports true temperatures that don't need any user-space adjustments. +Temperature is measured in degrees Celsius. +The chip is wired over I2C/SMBus and specified over a temperature +range of -40 to +125 degrees Celsius (for local sensor) and -40 to +127 +degrees Celsius (for remote sensors). +Resolution for both the local and remote channels is 0.125 degree C. + +The chip supports only temperature measurement. The driver exports +the temperature values via the following sysfs files: + +temp[1-3]_input +temp[2-3]_fault +temp[2-3]_offset +update_interval -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-11-13 2:52 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-11-09 7:08 [PATCH v3 0/3] Add W83773G hwmon sensor driver and doc Lei YU 2017-11-09 7:08 ` Lei YU 2017-11-09 7:08 ` [PATCH v3 1/3] DT: i2c: W83773G is a trivial device Lei YU 2017-11-09 7:09 ` [PATCH v3 2/3] drivers: hwmon: Add W83773G driver Lei YU 2017-11-10 14:49 ` Guenter Roeck 2017-11-13 2:52 ` Lei YU 2017-11-13 2:52 ` Lei YU 2017-11-09 7:09 ` [PATCH v3 3/3] hwmon: (w83773g) Add documentation Lei YU
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.