Re: [PATCH v3] hwmon: Add LTC2990 sensor driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 13, 2016 at 03:45:01PM +0100, Mike Looijmans wrote:
> This adds support for the Linear Technology LTC2990  I2C System Monitor.
> The LTC2990 supports a combination of voltage, current and temperature
> monitoring. This driver currently only supports reading two currents
> by measuring two differential voltages across series resistors, in
> addition to the Vcc supply voltage and internal temperature.
> 
> This is sufficient to support the Topic Miami SOM which uses this chip
> to monitor the currents flowing into the FPGA and the CPU parts.
> 
> Signed-off-by: Mike Looijmans <mike.looijmans@xxxxxxxx>

Hi Mike,

almost there. Please see inline.

Thanks,
Guenter

> ---
> v3: Remove unused includes.
>     Remove (most) unused register defines.
>     Also check on SMBUS WORD capability.
>     Use register defines as value indices.
>     Alignment fixups with "(".
> v2: Processed all review comments.
>      Put chip into continuous measurement mode.
>      Added ducumentation.
>      Use standard hwmon interfaces and macros.
> 
>  Documentation/hwmon/ltc2990 |  44 ++++++++++++
>  drivers/hwmon/Kconfig       |  14 ++++
>  drivers/hwmon/Makefile      |   1 +
>  drivers/hwmon/ltc2990.c     | 159 ++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 218 insertions(+)
>  create mode 100644 Documentation/hwmon/ltc2990
>  create mode 100644 drivers/hwmon/ltc2990.c
> 
> diff --git a/Documentation/hwmon/ltc2990 b/Documentation/hwmon/ltc2990
> new file mode 100644
> index 0000000..838b74e
> --- /dev/null
> +++ b/Documentation/hwmon/ltc2990
> @@ -0,0 +1,44 @@
> +Kernel driver ltc2990
> +=====================
> +
> +Supported chips:
> +  * Linear Technology LTC2990
> +    Prefix: 'ltc2990'
> +    Addresses scanned: -
> +    Datasheet: http://www.linear.com/product/ltc2990
> +
> +Author: Mike Looijmans <mike.looijmans@xxxxxxxx>
> +
> +
> +Description
> +-----------
> +
> +LTC2990 is a Quad I2C Voltage, Current and Temperature Monitor.
> +The chip's inputs can measure 4 voltages, or two inputs together (1+2 and 3+4)
> +can be combined to measure a differential voltage, which is typically used to
> +measure current through a series resistor, or a temperature.
> +
> +This driver currently uses the 2x differential mode only. In order to support
> +other modes, the driver will need to be expanded.
> +
> +
> +Usage Notes
> +-----------
> +
> +This driver does not probe for PMBus devices. You will have to instantiate
> +devices explicitly.
> +
> +
> +Sysfs attributes
> +----------------
> +
> +The "curr*_input" measurements actually report the voltage drop across the
> +input pins in microvolts. This is equivalent to the current through a 1mOhm
> +sense resistor. Divide the reported value by the actual sense resistor value
> +in mOhm to get the actual value.
> +
> +in0_input     Voltage at Vcc pin in millivolt (range 2.5V to 5V)
> +temp1_input   Internal chip temperature in millidegrees Celcius
> +curr1_input   Current in mA across v1-v2 assuming a 1mOhm sense resistor.
> +curr2_input   Current in mA across v3-v4 assuming a 1mOhm sense resistor.
> +
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 80a73bf..8a31d64 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -685,6 +685,20 @@ config SENSORS_LTC2945
>  	  This driver can also be built as a module. If so, the module will
>  	  be called ltc2945.
>  
> +config SENSORS_LTC2990
> +	tristate "Linear Technology LTC2990 (current monitoring mode only)"
> +	depends on I2C
> +	help
> +	  If you say yes here you get support for Linear Technology LTC2990
> +	  I2C System Monitor. The LTC2990 supports a combination of voltage,
> +	  current and temperature monitoring, but in addition to the Vcc supply
> +	  voltage and chip temperature, this driver currently only supports
> +	  reading two currents by measuring two differential voltages across
> +	  series resistors.
> +
> +	  This driver can also be built as a module. If so, the module will
> +	  be called ltc2990.
> +
>  config SENSORS_LTC4151
>  	tristate "Linear Technology LTC4151"
>  	depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 12a3239..e4bd15b 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -101,6 +101,7 @@ obj-$(CONFIG_SENSORS_LM95234)	+= lm95234.o
>  obj-$(CONFIG_SENSORS_LM95241)	+= lm95241.o
>  obj-$(CONFIG_SENSORS_LM95245)	+= lm95245.o
>  obj-$(CONFIG_SENSORS_LTC2945)	+= ltc2945.o
> +obj-$(CONFIG_SENSORS_LTC2990)	+= ltc2990.o
>  obj-$(CONFIG_SENSORS_LTC4151)	+= ltc4151.o
>  obj-$(CONFIG_SENSORS_LTC4215)	+= ltc4215.o
>  obj-$(CONFIG_SENSORS_LTC4222)	+= ltc4222.o
> diff --git a/drivers/hwmon/ltc2990.c b/drivers/hwmon/ltc2990.c
> new file mode 100644
> index 0000000..37ca5f4
> --- /dev/null
> +++ b/drivers/hwmon/ltc2990.c
> @@ -0,0 +1,159 @@
> +/*
> + * Driver for Linear Technology LTC2990 power monitor
> + *
> + * Copyright (C) 2014 Topic Embedded Products
> + * Author: Mike Looijmans <mike.looijmans@xxxxxxxx>
> + *
> + * License: GPLv2
> + *
> + * This driver assumes the chip is wired as a dual current monitor, and
> + * reports the voltage drop across two series resistors. It also reports
> + * the chip's internal temperature and Vcc power supply voltage.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +
> +#define LTC2990_STATUS	0x00
> +#define LTC2990_CONTROL	0x01
> +#define LTC2990_TRIGGER	0x02
> +#define LTC2990_TINT_MSB	0x04
> +#define LTC2990_V1_MSB	0x06
> +#define LTC2990_V2_MSB	0x08
> +#define LTC2990_V3_MSB	0x0A
> +#define LTC2990_V4_MSB	0x0C
> +#define LTC2990_VCC_MSB	0x0E
> +
> +#define LTC2990_CONTROL_KELVIN		BIT(7)
> +#define LTC2990_CONTROL_SINGLE		BIT(6)
> +#define LTC2990_CONTROL_MEASURE_ALL	(0x3 << 3)
> +#define LTC2990_CONTROL_MODE_CURRENT	0x06
> +#define LTC2990_CONTROL_MODE_VOLTAGE	0x07
> +
> +/* convert raw register value to sign-extended integer in 16-bit range */
> +static int ltc2990_voltage_to_int(int raw)
> +{
> +	if (raw & BIT(14)) {
> +		return -(0x4000 - (raw & 0x3FFF)) << 2;
> +	} else {
> +		return (raw & 0x3FFF) << 2;
> +	}

Unnecessary { }. Please see checkpatch warning.

> +}
> +
> +/* Return the converted value from the given register in uV or mC */
> +static int ltc2990_get_value(struct i2c_client *i2c, u8 reg)
> +{
> +	int val;
> +	int result;
> +
> +	val = i2c_smbus_read_word_swapped(i2c, reg);
> +	if (unlikely(val < 0))
> +		return val;

This suggests the function returns a value < 0 on error ...

> +	switch (reg) {
> +	case LTC2990_TINT_MSB:
> +		/* internal temp, 0.0625 degrees/LSB, 13-bit  */
> +		val = (val & 0x1FFF) << 3;
> +		result = (val * 1000) >> 7;
> +		break;
> +	case LTC2990_V1_MSB:
> +	case LTC2990_V3_MSB:
> +		 /* Vx-Vy, 19.42uV/LSB. Depends on mode. */
> +		result = ltc2990_voltage_to_int(val) * 1942 / (4 * 100);
> +		break;
> +	case LTC2990_VCC_MSB:
> +		/* Vcc, 305.18μV/LSB, 2.5V offset */
> +		result = ltc2990_voltage_to_int(val) * 30518 / (4 * 100 * 1000);
> +		result += 2500;
> +		break;
> +	default:
> +		result = 0; /* won't happen, keep compiler happy */

Given that, we can return some error here, and ...

> +		break;
> +	}
> +
> +	return result;
> +}
> +
> +static ssize_t ltc2990_show_value(struct device *dev,
> +				  struct device_attribute *da, char *buf)
> +{
> +	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> +	int value;
> +
> +	value = ltc2990_get_value(dev_get_drvdata(dev), attr->index);

... we should actually check the error return here.

	if (value < 0)
		return value;

> +	return snprintf(buf, PAGE_SIZE, "%d\n", value);
> +}
> +
> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, ltc2990_show_value, NULL,
> +			  LTC2990_TINT_MSB);
> +static SENSOR_DEVICE_ATTR(curr1_input, S_IRUGO, ltc2990_show_value, NULL,
> +			  LTC2990_V1_MSB);
> +static SENSOR_DEVICE_ATTR(curr2_input, S_IRUGO, ltc2990_show_value, NULL,
> +			  LTC2990_V3_MSB);
> +static SENSOR_DEVICE_ATTR(in0_input, S_IRUGO, ltc2990_show_value, NULL,
> +			  LTC2990_VCC_MSB);
> +
> +static struct attribute *ltc2990_attrs[] = {
> +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> +	&sensor_dev_attr_curr1_input.dev_attr.attr,
> +	&sensor_dev_attr_curr2_input.dev_attr.attr,
> +	&sensor_dev_attr_in0_input.dev_attr.attr,
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(ltc2990);
> +
> +static int ltc2990_i2c_probe(struct i2c_client *i2c,
> +			     const struct i2c_device_id *id)
> +{
> +	int ret;
> +	struct device *hwmon_dev;
> +
> +	if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_SMBUS_BYTE_DATA |
> +				     I2C_FUNC_SMBUS_WORD_DATA))
> +		return -ENODEV;
> +
> +	/* Setup continuous mode, current monitor */
> +	ret = i2c_smbus_write_byte_data(i2c, LTC2990_CONTROL,
> +					LTC2990_CONTROL_MEASURE_ALL |
> +					LTC2990_CONTROL_MODE_CURRENT);
> +	if (ret < 0) {
> +		dev_err(&i2c->dev, "Error: Failed to set control mode.\n");
> +		return ret;
> +	}
> +	/* Trigger once to start continuous conversion */
> +	ret = i2c_smbus_write_byte_data(i2c, LTC2990_TRIGGER, 1);
> +	if (ret < 0) {
> +		dev_err(&i2c->dev, "Error: Failed to start acquisition.\n");
> +		return ret;
> +	}
> +
> +	hwmon_dev = devm_hwmon_device_register_with_groups(&i2c->dev,
> +							   i2c->name,
> +							   i2c,
> +							   ltc2990_groups);
> +
> +	return PTR_ERR_OR_ZERO(hwmon_dev);
> +}
> +
> +static const struct i2c_device_id ltc2990_i2c_id[] = {
> +	{ "ltc2990", 0 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, ltc2990_i2c_id);
> +
> +static struct i2c_driver ltc2990_i2c_driver = {
> +	.driver = {
> +		.name = "ltc2990",
> +	},
> +	.probe    = ltc2990_i2c_probe,
> +	.id_table = ltc2990_i2c_id,
> +};
> +
> +module_i2c_driver(ltc2990_i2c_driver);
> +
> +MODULE_DESCRIPTION("LTC2990 Sensor Driver");
> +MODULE_AUTHOR("Topic Embedded Products");
> +MODULE_LICENSE("GPL v2");
> -- 
> 1.9.1
> 

_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors




[Index of Archives]     [Linux Kernel]     [Linux Hardware Monitoring]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux