* Re: [lm-sensors] [PATCH] Added support for Lattice's POWR1220 power manager IC.
@ 2014-06-10 0:33 Guenter Roeck
2014-06-10 14:25 ` Scott Kanowitz
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: Guenter Roeck @ 2014-06-10 0:33 UTC (permalink / raw)
To: lm-sensors
On 06/09/2014 09:08 AM, Scott Kanowitz wrote:
> This patch adds support for Lattice's POWR1220 power manager IC. Read/write
> access to all the registers and ADCs on the chip are supported through the
> hwmon sysfs files.
>
> Signed-off-by: Scott Kanowitz <skanowitz@echo360.com>
Hi Scott,
please read and follow
Documentation/hwmon/hwmon-kernel-api.txt
Documentation/hwmon/sysfs-interface
Documentation/hwmon/submitting-patches
Also please make sure that the mailing list is included in your patch submission.
This ensures that it is copied to patchwork and does not get lost.
> ---
> drivers/hwmon/Kconfig | 12 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/powr1220.c | 624 +++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 637 insertions(+)
> mode change 100644 => 100755 drivers/hwmon/Kconfig
> create mode 100755 drivers/hwmon/powr1220.c
Configuration and source files are not executable. Please make sure
that your patch passes checkpatch without errors and warnings.
I won't review in detail since literally none of your attributes
follows the hwmon ABI. Some general comments, though.
- Attributes are displayed in decimal.
- There is no need to add blocking around a single i2c function call.
The i2c subsystem does its own locking.
- Single assignments don't need locking either.
- GPIO attributes are not acceptable as hwmon attributes. Feel free to
register GPIO attributes with the GPIO subsystem if you like. You can
also write separate mfd and gpio drivers, but IMO that would be overkill.
- No forward declarations, please.
- Please use device managed functions where possible.
- Configuration parameters need to be set via platform data and/or
devicetree data, not with sysfs attributes.
- Your driver does not use any smbus word access functions. Checking for
I2C_FUNC_SMBUS_WORD_DATA seems quite unnecessary.
- At your choice you might want to consider using regmap for value caching
instead of powr1220_update_device().
Thanks,
Guenter
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> old mode 100644
> new mode 100755
> index 4af0da9..fc4d7f1
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -608,6 +608,18 @@ config SENSORS_JC42
> This driver can also be built as a module. If so, the module
> will be called jc42.
>
> +config SENSORS_POWR1220
> + tristate "Lattice POWR1220 Power Monitoring"
> + depends on I2C
> + default n
> + help
> + If you say yes here you get access to the hardware monitoring
> + functions of the Lattice POWR1220 isp Power Supply Monitoring,
> + Sequencing and Margining Controller.
> +
> + This driver can also be built as a module. If so, the module
> + will be called powr1220.
> +
> config SENSORS_LINEAGE
> tristate "Lineage Compact Power Line Power Entry Module"
> depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index c48f987..842e6b3 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -119,6 +119,7 @@ obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o
> obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
> obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
> obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
> +obj-$(CONFIG_SENSORS_POWR1220) += powr1220.o
> obj-$(CONFIG_SENSORS_S3C) += s3c-hwmon.o
> obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o
> obj-$(CONFIG_SENSORS_SCH5627) += sch5627.o
> diff --git a/drivers/hwmon/powr1220.c b/drivers/hwmon/powr1220.c
> new file mode 100755
> index 0000000..2452e29
> --- /dev/null
> +++ b/drivers/hwmon/powr1220.c
> @@ -0,0 +1,624 @@
> +/*
> + * powr1220.c - Driver for the Lattice POWR1220 programmable power supply
> + * and monitor. Users can read all the values from the device, reset
> + * the device, read all the ADC inputs and adjust the trim values
> + * using the sysfs nodes.
> + *
> + * Copyright (c) 2014 Echo360 http://www.echo360.com
> + * Scott Kanowitz <skanowitz@echo360.com> <scott.kanowitz@gmail.com>
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/jiffies.h>
> +#include <linux/i2c.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/err.h>
> +#include <linux/mutex.h>
> +#include <linux/delay.h>
> +
> +
> +#define ADC_STEP_MV 2
> +
> +enum powr1220_regs {
> + VMON_STATUS0,
> + VMON_STATUS1,
> + VMON_STATUS2,
> + OUTPUT_STATUS0,
> + OUTPUT_STATUS1,
> + OUTPUT_STATUS2,
> + INPUT_STATUS,
> + ADC_VALUE_LOW,
> + ADC_VALUE_HIGH,
> + ADC_MUX,
> + UES_BYTE0,
> + UES_BYTE1,
> + UES_BYTE2,
> + UES_BYTE3,
> + GP_OUTPUT1,
> + GP_OUTPUT2,
> + GP_OUTPUT3,
> + INPUT_VALUE,
> + RESET,
> + TRIM1_TRIM,
> + TRIM2_TRIM,
> + TRIM3_TRIM,
> + TRIM4_TRIM,
> + TRIM5_TRIM,
> + TRIM6_TRIM,
> + TRIM7_TRIM,
> + TRIM8_TRIM,
> + MAX_POWR1220_REGS
> +};
> +
> +
> +enum powr1220_adc_values {
> + VMON1,
> + VMON2,
> + VMON3,
> + VMON4,
> + VMON5,
> + VMON6,
> + VMON7,
> + VMON8,
> + VMON9,
> + VMON10,
> + VMON11,
> + VMON12,
> + VCCA,
> + VCCINP,
> + MAX_POWR1220_ADC_VALUES
> +};
> +
> +struct powr1220_data {
> + struct device *hwmon_dev;
> + struct mutex update_lock;
> + bool valid;
> + bool adc_valid[MAX_POWR1220_ADC_VALUES];
> + /* the next two values are in jiffies */
> + unsigned long last_updated;
> + unsigned long adc_last_updated[MAX_POWR1220_ADC_VALUES];
> +
> + /* values */
> + unsigned char adc_range;
> + int adc_values[MAX_POWR1220_ADC_VALUES];
> + int vmon_status[3];
> + int input_status;
> + int input_value;
> + int hvout;
> + int output_status;
> + int gp_output;
> + int ues;
> + int trim[8];
> +};
> +
> +
> +static struct powr1220_data *powr1220_update_device(struct device *dev);
> +static int powr1220_read_adc(struct device *dev, int ch_num);
> +static int powr1220_probe(struct i2c_client *client,
> + const struct i2c_device_id *id);
> +static int powr1220_remove(struct i2c_client *client);
> +
> +
> +/* Resets the device with a write to the reset register */
> +static ssize_t powr1220_reset(struct device *dev,
> + struct device_attribute *dev_attr, const char *buf, size_t count)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct powr1220_data *data = i2c_get_clientdata(client);
> +
> + mutex_lock(&data->update_lock);
> + i2c_smbus_write_byte_data(client, RESET, 1);
> + mutex_unlock(&data->update_lock);
> +
> + return count;
> +}
> +
> +
> +/* Shows the voltage associated with the specified ADC channel */
> +static ssize_t powr1220_show_voltage(struct device *dev,
> + struct device_attribute *dev_attr, char *buf)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
> + int adc_val = powr1220_read_adc(dev, attr->index);
> +
> + return sprintf(buf, "%04d\n", adc_val);
> +}
> +
> +
> +/* Shows the status associated with the specified vmon register */
> +static ssize_t powr1220_show_vmon_status(struct device *dev,
> + struct device_attribute *dev_attr, char *buf)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
> + struct powr1220_data *data = powr1220_update_device(dev);
> +
> + return sprintf(buf, "0x%02X\n", data->vmon_status[attr->index]);
> +}
> +
> +
> +/* Shows the value of the input status register */
> +static ssize_t powr1220_show_input_status(struct device *dev,
> + struct device_attribute *dev_attr, char *buf)
> +{
> + struct powr1220_data *data = powr1220_update_device(dev);
> +
> + return sprintf(buf, "0x%02X\n", data->input_status);
> +}
> +
> +
> +/* Shows the value of the input value register */
> +static ssize_t powr1220_show_input_value(struct device *dev,
> + struct device_attribute *dev_attr, char *buf)
> +{
> + struct powr1220_data *data = powr1220_update_device(dev);
> +
> + return sprintf(buf, "0x%02X\n", data->input_value);
> +}
> +
> +
> +/* Sets the value of the input_value register. Input value is always hex */
> +static ssize_t powr1220_set_input_value(struct device *dev,
> + struct device_attribute *dev_attr, const char *buf, size_t count)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct powr1220_data *data = i2c_get_clientdata(client);
> + unsigned long value;
> + int error;
> +
> + error = kstrtol(buf, 16, &value);
> + if (error)
> + return error;
> +
> + value &= 0xFE;
> +
> + mutex_lock(&data->update_lock);
> + i2c_smbus_write_byte_data(client, INPUT_VALUE, (u8)value);
> + mutex_unlock(&data->update_lock);
> +
> + return count;
> +}
> +
> +
> +/* Shows the value of the output status */
> +static ssize_t powr1220_show_output_status(struct device *dev,
> + struct device_attribute *dev_attr, char *buf)
> +{
> + struct powr1220_data *data = powr1220_update_device(dev);
> +
> + return sprintf(buf, "0x%06X\n", data->output_status);
> +}
> +
> +
> +/* Shows the value of the gp output */
> +static ssize_t powr1220_show_gp_output(struct device *dev,
> + struct device_attribute *dev_attr, char *buf)
> +{
> + struct powr1220_data *data = powr1220_update_device(dev);
> +
> + return sprintf(buf, "0x%06X\n", data->gp_output);
> +}
> +
> +
> +/* Sets the value of the gp_output registers. Input value is always hex */
> +static ssize_t powr1220_set_gp_output(struct device *dev,
> + struct device_attribute *dev_attr, const char *buf, size_t count)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct powr1220_data *data = i2c_get_clientdata(client);
> + unsigned long value;
> + int error;
> + unsigned char regs[3];
> +
> + error = kstrtol(buf, 16, &value);
> + if (error)
> + return error;
> +
> + regs[0] = value & 0x0000FF;
> + regs[1] = (value & 0x00FF00) >> 8;
> + regs[2] = (value & 0x0F0000) >> 16;
> +
> + mutex_lock(&data->update_lock);
> + i2c_smbus_write_byte_data(client, GP_OUTPUT1, regs[0]);
> + i2c_smbus_write_byte_data(client, GP_OUTPUT2, regs[1]);
> + i2c_smbus_write_byte_data(client, GP_OUTPUT3, regs[2]);
> + mutex_unlock(&data->update_lock);
> +
> + return count;
> +}
> +
> +
> +/* Shows the value of the hvout */
> +static ssize_t powr1220_show_hvout(struct device *dev,
> + struct device_attribute *dev_attr, char *buf)
> +{
> + struct powr1220_data *data = powr1220_update_device(dev);
> +
> + return sprintf(buf, "0x%02X\n", data->hvout);
> +}
> +
> +
> +/* Shows the value of ues */
> +static ssize_t powr1220_show_ues(struct device *dev,
> + struct device_attribute *dev_attr, char *buf)
> +{
> + struct powr1220_data *data = powr1220_update_device(dev);
> +
> + return sprintf(buf, "0x%08X\n", data->ues);
> +}
> +
> +
> +/* Shows the value of the specified trim value */
> +static ssize_t powr1220_show_trim(struct device *dev,
> + struct device_attribute *dev_attr, char *buf)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
> + struct powr1220_data *data = powr1220_update_device(dev);
> +
> + return sprintf(buf, "0x%02X\n", data->trim[attr->index]);
> +}
> +
> +
> +/* Sets the value of the specified trim register. Input value is always hex */
> +static ssize_t powr1220_set_trim(struct device *dev,
> + struct device_attribute *dev_attr, const char *buf, size_t count)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
> + struct powr1220_data *data = i2c_get_clientdata(client);
> + unsigned long value;
> + int error;
> +
> + error = kstrtol(buf, 16, &value);
> + if (error)
> + return error;
> +
> + mutex_lock(&data->update_lock);
> + i2c_smbus_write_byte_data(client, TRIM1_TRIM + attr->index, (u8)value);
> + mutex_unlock(&data->update_lock);
> +
> + return count;
> +}
> +
> +
> +/* Shows the value of the adc range (0 or 1) */
> +static ssize_t powr1220_show_adc_range(struct device *dev,
> + struct device_attribute *dev_attr, char *buf)
> +{
> + struct powr1220_data *data = powr1220_update_device(dev);
> +
> + return sprintf(buf, "%d\n", (data->adc_range ? 1 : 0));
> +}
> +
> +
> +/* Sets the value of the adc range (0 or 1) */
> +static ssize_t powr1220_set_adc_range(struct device *dev,
> + struct device_attribute *dev_attr, const char *buf, size_t count)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct powr1220_data *data = i2c_get_clientdata(client);
> + unsigned long range;
> + int error;
> +
> + error = kstrtol(buf, 10, &range);
> + if (error)
> + return error;
> +
> + mutex_lock(&data->update_lock);
> + data->adc_range = (range ? 1 : 0);
> + mutex_unlock(&data->update_lock);
> +
> + return count;
> +}
> +
> +
> +
> +/* Reads the specified ADC channel */
> +static int powr1220_read_adc(struct device *dev, int ch_num)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct powr1220_data *data = i2c_get_clientdata(client);
> + unsigned char reg;
> + int reading;
> +
> + mutex_lock(&data->update_lock);
> +
> + if (time_after(jiffies, data->adc_last_updated[ch_num] + HZ) ||
> + !data->adc_valid[ch_num]) {
> + /* set the attenuator and mux */
> + reg = ((data->adc_range & 1) << 4) | ch_num;
> + i2c_smbus_write_byte_data(client, ADC_MUX, reg);
> +
> + /* wait at least Tconvert time (200 us) for the
> + * conversion to complete */
> + udelay(200);
> +
> + /* check if the done bit is set */
> + reg = i2c_smbus_read_byte_data(client, ADC_VALUE_LOW);
> + if (reg & 0x01) {
> + /* we have a valid conversion to read */
> + reading = reg >> 4;
> + reg = i2c_smbus_read_byte_data(client, ADC_VALUE_HIGH);
> + reading |= (unsigned int)reg << 4;
> +
> + /* now convert the reading to a voltage */
> + data->adc_values[ch_num] = reading * ADC_STEP_MV;
> + }
> +
> + data->adc_last_updated[ch_num] = jiffies;
> + data->adc_valid[ch_num] = 1;
> + }
> +
> + mutex_unlock(&data->update_lock);
> +
> + return data->adc_values[ch_num];
> +}
> +
> +
> +/* Reads from the device and updates the local data if after the timeout */
> +static struct powr1220_data *powr1220_update_device(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct powr1220_data *data = i2c_get_clientdata(client);
> + unsigned char regs[MAX_POWR1220_REGS];
> + int i;
> +
> + mutex_lock(&data->update_lock);
> +
> + if (time_after(jiffies, data->last_updated + HZ) || !data->valid) {
> + /* read the full register bank */
> + for (i = 0; i < MAX_POWR1220_REGS; i++)
> + regs[i] = i2c_smbus_read_byte_data(client, i);
> +
> + /* get the values of vmon A and B */
> + for (i = 0; i < ARRAY_SIZE(data->vmon_status); i++)
> + data->vmon_status[i] = regs[VMON_STATUS0 + i];
> +
> + /* get the input status and value */
> + data->input_status = regs[INPUT_STATUS] & 0x3F;
> + data->input_value = regs[INPUT_VALUE] & 0x3E;
> +
> + /* get the output status */
> + data->hvout = regs[OUTPUT_STATUS0] & 0x0F;
> + data->output_status = (regs[OUTPUT_STATUS0] >> 4) |
> + ((int)regs[OUTPUT_STATUS1] << 4) |
> + ((int)regs[OUTPUT_STATUS2] << 12);
> +
> + /* get the output */
> + data->gp_output = regs[GP_OUTPUT1] |
> + ((int)regs[GP_OUTPUT2] << 8) |
> + ((int)(regs[GP_OUTPUT3] & 0x0F) << 16);
> +
> + /* get the user signature*/
> + data->ues = regs[UES_BYTE0] |
> + ((int)regs[UES_BYTE1] << 8) |
> + ((int)regs[UES_BYTE2] << 16) |
> + ((int)regs[UES_BYTE3] << 24);
> +
> + /* get the trim values */
> + for (i = 0; i < ARRAY_SIZE(data->trim); i++)
> + data->trim[i] = regs[TRIM1_TRIM + i];
> +
> + data->last_updated = jiffies;
> + data->valid = 1;
> + }
> +
> + mutex_unlock(&data->update_lock);
> +
> + return data;
> +}
> +
> +
> +
> +static SENSOR_DEVICE_ATTR(vmon1, S_IRUGO, powr1220_show_voltage, NULL, VMON1);
> +static SENSOR_DEVICE_ATTR(vmon2, S_IRUGO, powr1220_show_voltage, NULL, VMON2);
> +static SENSOR_DEVICE_ATTR(vmon3, S_IRUGO, powr1220_show_voltage, NULL, VMON3);
> +static SENSOR_DEVICE_ATTR(vmon4, S_IRUGO, powr1220_show_voltage, NULL, VMON4);
> +static SENSOR_DEVICE_ATTR(vmon5, S_IRUGO, powr1220_show_voltage, NULL, VMON5);
> +static SENSOR_DEVICE_ATTR(vmon6, S_IRUGO, powr1220_show_voltage, NULL, VMON6);
> +static SENSOR_DEVICE_ATTR(vmon7, S_IRUGO, powr1220_show_voltage, NULL, VMON7);
> +static SENSOR_DEVICE_ATTR(vmon8, S_IRUGO, powr1220_show_voltage, NULL, VMON8);
> +static SENSOR_DEVICE_ATTR(vmon9, S_IRUGO, powr1220_show_voltage, NULL, VMON9);
> +static SENSOR_DEVICE_ATTR(vmon10, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON10);
> +static SENSOR_DEVICE_ATTR(vmon11, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON11);
> +static SENSOR_DEVICE_ATTR(vmon12, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON12);
> +static SENSOR_DEVICE_ATTR(vcca, S_IRUGO, powr1220_show_voltage, NULL, VCCA);
> +static SENSOR_DEVICE_ATTR(vccinp, S_IRUGO, powr1220_show_voltage, NULL,
> + VCCINP);
> +
> +static SENSOR_DEVICE_ATTR(reset, S_IWUSR, NULL, powr1220_reset, 0);
> +
> +static SENSOR_DEVICE_ATTR(vmon_status0, S_IRUGO, powr1220_show_vmon_status,
> + NULL, 0);
> +static SENSOR_DEVICE_ATTR(vmon_status1, S_IRUGO, powr1220_show_vmon_status,
> + NULL, 1);
> +static SENSOR_DEVICE_ATTR(vmon_status2, S_IRUGO, powr1220_show_vmon_status,
> + NULL, 2);
> +
> +static SENSOR_DEVICE_ATTR(input_status, S_IRUGO, powr1220_show_input_status,
> + NULL, 0);
> +static SENSOR_DEVICE_ATTR(input_value, S_IWUSR | S_IRUGO,
> + powr1220_show_input_value, powr1220_set_input_value, 0);
> +static SENSOR_DEVICE_ATTR(output_status, S_IRUGO, powr1220_show_output_status,
> + NULL, 0);
> +static SENSOR_DEVICE_ATTR(gp_output, S_IWUSR | S_IRUGO,
> + powr1220_show_gp_output, powr1220_set_gp_output, 0);
> +static SENSOR_DEVICE_ATTR(hvout, S_IRUGO, powr1220_show_hvout,
> + NULL, 0);
> +static SENSOR_DEVICE_ATTR(ues, S_IRUGO, powr1220_show_ues, NULL, 0);
> +
> +static SENSOR_DEVICE_ATTR(trim1, S_IWUSR | S_IRUGO, powr1220_show_trim,
> + powr1220_set_trim, 0);
> +static SENSOR_DEVICE_ATTR(trim2, S_IWUSR | S_IRUGO, powr1220_show_trim,
> + powr1220_set_trim, 1);
> +static SENSOR_DEVICE_ATTR(trim3, S_IWUSR | S_IRUGO, powr1220_show_trim,
> + powr1220_set_trim, 2);
> +static SENSOR_DEVICE_ATTR(trim4, S_IWUSR | S_IRUGO, powr1220_show_trim,
> + powr1220_set_trim, 3);
> +static SENSOR_DEVICE_ATTR(trim5, S_IWUSR | S_IRUGO, powr1220_show_trim,
> + powr1220_set_trim, 4);
> +static SENSOR_DEVICE_ATTR(trim6, S_IWUSR | S_IRUGO, powr1220_show_trim,
> + powr1220_set_trim, 5);
> +static SENSOR_DEVICE_ATTR(trim7, S_IWUSR | S_IRUGO, powr1220_show_trim,
> + powr1220_set_trim, 6);
> +static SENSOR_DEVICE_ATTR(trim8, S_IWUSR | S_IRUGO, powr1220_show_trim,
> + powr1220_set_trim, 7);
> +
> +static SENSOR_DEVICE_ATTR(adc_range, S_IWUSR | S_IRUGO,
> + powr1220_show_adc_range, powr1220_set_adc_range, 0);
> +
> +
> +
> +static struct attribute *powr1220_attributes[] = {
> + &sensor_dev_attr_vmon1.dev_attr.attr,
> + &sensor_dev_attr_vmon2.dev_attr.attr,
> + &sensor_dev_attr_vmon3.dev_attr.attr,
> + &sensor_dev_attr_vmon4.dev_attr.attr,
> + &sensor_dev_attr_vmon5.dev_attr.attr,
> + &sensor_dev_attr_vmon6.dev_attr.attr,
> + &sensor_dev_attr_vmon7.dev_attr.attr,
> + &sensor_dev_attr_vmon8.dev_attr.attr,
> + &sensor_dev_attr_vmon9.dev_attr.attr,
> + &sensor_dev_attr_vmon10.dev_attr.attr,
> + &sensor_dev_attr_vmon11.dev_attr.attr,
> + &sensor_dev_attr_vmon12.dev_attr.attr,
> + &sensor_dev_attr_vcca.dev_attr.attr,
> + &sensor_dev_attr_vccinp.dev_attr.attr,
> +
> + &sensor_dev_attr_reset.dev_attr.attr,
> +
> + &sensor_dev_attr_vmon_status0.dev_attr.attr,
> + &sensor_dev_attr_vmon_status1.dev_attr.attr,
> + &sensor_dev_attr_vmon_status2.dev_attr.attr,
> +
> + &sensor_dev_attr_input_status.dev_attr.attr,
> + &sensor_dev_attr_input_value.dev_attr.attr,
> + &sensor_dev_attr_output_status.dev_attr.attr,
> + &sensor_dev_attr_gp_output.dev_attr.attr,
> + &sensor_dev_attr_hvout.dev_attr.attr,
> + &sensor_dev_attr_ues.dev_attr.attr,
> +
> + &sensor_dev_attr_trim1.dev_attr.attr,
> + &sensor_dev_attr_trim2.dev_attr.attr,
> + &sensor_dev_attr_trim3.dev_attr.attr,
> + &sensor_dev_attr_trim4.dev_attr.attr,
> + &sensor_dev_attr_trim5.dev_attr.attr,
> + &sensor_dev_attr_trim6.dev_attr.attr,
> + &sensor_dev_attr_trim7.dev_attr.attr,
> + &sensor_dev_attr_trim8.dev_attr.attr,
> +
> + &sensor_dev_attr_adc_range.dev_attr.attr,
> +
> + NULL
> +};
> +
> +static const struct attribute_group powr1220_group = {
> + .attrs = powr1220_attributes,
> +};
> +
> +
> +static int powr1220_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct powr1220_data *data;
> + int status;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA))
> + return -EIO;
> +
> + data = kzalloc(sizeof(struct powr1220_data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + i2c_set_clientdata(client, data);
> + mutex_init(&data->update_lock);
> +
> + status = sysfs_create_group(&client->dev.kobj, &powr1220_group);
> + if (status)
> + goto exit_free;
> +
> + data->hwmon_dev = hwmon_device_register(&client->dev);
> + if (IS_ERR(data->hwmon_dev)) {
> + status = PTR_ERR(data->hwmon_dev);
> + goto exit_remove_files;
> + }
> +
> + dev_info(&client->dev, "initialized\n");
> +
> + return 0;
> +
> +exit_remove_files:
> + sysfs_remove_group(&client->dev.kobj, &powr1220_group);
> +exit_free:
> + kfree(data);
> + return status;
> +}
> +
> +
> +static int powr1220_remove(struct i2c_client *client)
> +{
> + struct powr1220_data *data = i2c_get_clientdata(client);
> +
> + hwmon_device_unregister(data->hwmon_dev);
> + sysfs_remove_group(&client->dev.kobj, &powr1220_group);
> + kfree(data);
> + return 0;
> +}
> +
> +
> +
> +static const struct i2c_device_id powr1220_ids[] = {
> + { "powr1220", 0, },
> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, powr1220_ids);
> +
> +static struct i2c_driver powr1220_driver = {
> + .class = I2C_CLASS_HWMON,
> + .driver = {
> + .name = "powr1220",
> + },
> + .probe = powr1220_probe,
> + .remove = powr1220_remove,
> + .id_table = powr1220_ids,
> +};
> +
> +
> +static int __init powr1220_init(void)
> +{
> + return i2c_add_driver(&powr1220_driver);
> +}
> +
> +static void __exit powr1220_exit(void)
> +{
> + i2c_del_driver(&powr1220_driver);
> +}
> +
> +
> +module_init(powr1220_init);
> +module_exit(powr1220_exit);
> +
> +MODULE_AUTHOR("Scott Kanowitz");
> +MODULE_DESCRIPTION("POWR1220 driver");
> +MODULE_LICENSE("GPL");
>
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [lm-sensors] [PATCH] Added support for Lattice's POWR1220 power manager IC.
2014-06-10 0:33 [lm-sensors] [PATCH] Added support for Lattice's POWR1220 power manager IC Guenter Roeck
@ 2014-06-10 14:25 ` Scott Kanowitz
2014-06-11 20:35 ` Scott Kanowitz
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Scott Kanowitz @ 2014-06-10 14:25 UTC (permalink / raw)
To: lm-sensors
Guenter,
Thanks for the feedback. I'll update the driver and resubmit it.
--Scott
-----Original Message-----
From: Guenter Roeck [mailto:linux@roeck-us.net]
Sent: Monday, June 09, 2014 8:33 PM
To: Scott Kanowitz; jdelvare@suse.de; LM Sensors
Subject: Re: [PATCH] Added support for Lattice's POWR1220 power manager IC.
On 06/09/2014 09:08 AM, Scott Kanowitz wrote:
> This patch adds support for Lattice's POWR1220 power manager IC.
> Read/write access to all the registers and ADCs on the chip are
> supported through the hwmon sysfs files.
>
> Signed-off-by: Scott Kanowitz <skanowitz@echo360.com>
Hi Scott,
please read and follow
Documentation/hwmon/hwmon-kernel-api.txt
Documentation/hwmon/sysfs-interface
Documentation/hwmon/submitting-patches
Also please make sure that the mailing list is included in your patch submission.
This ensures that it is copied to patchwork and does not get lost.
> ---
> drivers/hwmon/Kconfig | 12 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/powr1220.c | 624 +++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 637 insertions(+)
> mode change 100644 => 100755 drivers/hwmon/Kconfig
> create mode 100755 drivers/hwmon/powr1220.c
Configuration and source files are not executable. Please make sure that your patch passes checkpatch without errors and warnings.
I won't review in detail since literally none of your attributes follows the hwmon ABI. Some general comments, though.
- Attributes are displayed in decimal.
- There is no need to add blocking around a single i2c function call.
The i2c subsystem does its own locking.
- Single assignments don't need locking either.
- GPIO attributes are not acceptable as hwmon attributes. Feel free to
register GPIO attributes with the GPIO subsystem if you like. You can
also write separate mfd and gpio drivers, but IMO that would be overkill.
- No forward declarations, please.
- Please use device managed functions where possible.
- Configuration parameters need to be set via platform data and/or
devicetree data, not with sysfs attributes.
- Your driver does not use any smbus word access functions. Checking for
I2C_FUNC_SMBUS_WORD_DATA seems quite unnecessary.
- At your choice you might want to consider using regmap for value caching
instead of powr1220_update_device().
Thanks,
Guenter
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig old mode
> 100644 new mode 100755 index 4af0da9..fc4d7f1
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -608,6 +608,18 @@ config SENSORS_JC42
> This driver can also be built as a module. If so, the module
> will be called jc42.
>
> +config SENSORS_POWR1220
> + tristate "Lattice POWR1220 Power Monitoring"
> + depends on I2C
> + default n
> + help
> + If you say yes here you get access to the hardware monitoring
> + functions of the Lattice POWR1220 isp Power Supply Monitoring,
> + Sequencing and Margining Controller.
> +
> + This driver can also be built as a module. If so, the module
> + will be called powr1220.
> +
> config SENSORS_LINEAGE
> tristate "Lineage Compact Power Line Power Entry Module"
> depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index
> c48f987..842e6b3 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -119,6 +119,7 @@ obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o
> obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
> obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
> obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
> +obj-$(CONFIG_SENSORS_POWR1220) += powr1220.o
> obj-$(CONFIG_SENSORS_S3C) += s3c-hwmon.o
> obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o
> obj-$(CONFIG_SENSORS_SCH5627) += sch5627.o
> diff --git a/drivers/hwmon/powr1220.c b/drivers/hwmon/powr1220.c new
> file mode 100755 index 0000000..2452e29
> --- /dev/null
> +++ b/drivers/hwmon/powr1220.c
> @@ -0,0 +1,624 @@
> +/*
> + * powr1220.c - Driver for the Lattice POWR1220 programmable power
> +supply
> + * and monitor. Users can read all the values from the device, reset
> + * the device, read all the ADC inputs and adjust the trim values
> + * using the sysfs nodes.
> + *
> + * Copyright (c) 2014 Echo360 http://www.echo360.com
> + * Scott Kanowitz <skanowitz@echo360.com> <scott.kanowitz@gmail.com>
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/jiffies.h>
> +#include <linux/i2c.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/err.h>
> +#include <linux/mutex.h>
> +#include <linux/delay.h>
> +
> +
> +#define ADC_STEP_MV 2
> +
> +enum powr1220_regs {
> + VMON_STATUS0,
> + VMON_STATUS1,
> + VMON_STATUS2,
> + OUTPUT_STATUS0,
> + OUTPUT_STATUS1,
> + OUTPUT_STATUS2,
> + INPUT_STATUS,
> + ADC_VALUE_LOW,
> + ADC_VALUE_HIGH,
> + ADC_MUX,
> + UES_BYTE0,
> + UES_BYTE1,
> + UES_BYTE2,
> + UES_BYTE3,
> + GP_OUTPUT1,
> + GP_OUTPUT2,
> + GP_OUTPUT3,
> + INPUT_VALUE,
> + RESET,
> + TRIM1_TRIM,
> + TRIM2_TRIM,
> + TRIM3_TRIM,
> + TRIM4_TRIM,
> + TRIM5_TRIM,
> + TRIM6_TRIM,
> + TRIM7_TRIM,
> + TRIM8_TRIM,
> + MAX_POWR1220_REGS
> +};
> +
> +
> +enum powr1220_adc_values {
> + VMON1,
> + VMON2,
> + VMON3,
> + VMON4,
> + VMON5,
> + VMON6,
> + VMON7,
> + VMON8,
> + VMON9,
> + VMON10,
> + VMON11,
> + VMON12,
> + VCCA,
> + VCCINP,
> + MAX_POWR1220_ADC_VALUES
> +};
> +
> +struct powr1220_data {
> + struct device *hwmon_dev;
> + struct mutex update_lock;
> + bool valid;
> + bool adc_valid[MAX_POWR1220_ADC_VALUES];
> + /* the next two values are in jiffies */
> + unsigned long last_updated;
> + unsigned long adc_last_updated[MAX_POWR1220_ADC_VALUES];
> +
> + /* values */
> + unsigned char adc_range;
> + int adc_values[MAX_POWR1220_ADC_VALUES];
> + int vmon_status[3];
> + int input_status;
> + int input_value;
> + int hvout;
> + int output_status;
> + int gp_output;
> + int ues;
> + int trim[8];
> +};
> +
> +
> +static struct powr1220_data *powr1220_update_device(struct device
> +*dev); static int powr1220_read_adc(struct device *dev, int ch_num);
> +static int powr1220_probe(struct i2c_client *client,
> + const struct i2c_device_id *id);
> +static int powr1220_remove(struct i2c_client *client);
> +
> +
> +/* Resets the device with a write to the reset register */ static
> +ssize_t powr1220_reset(struct device *dev,
> + struct device_attribute *dev_attr, const char *buf, size_t count) {
> + struct i2c_client *client = to_i2c_client(dev);
> + struct powr1220_data *data = i2c_get_clientdata(client);
> +
> + mutex_lock(&data->update_lock);
> + i2c_smbus_write_byte_data(client, RESET, 1);
> + mutex_unlock(&data->update_lock);
> +
> + return count;
> +}
> +
> +
> +/* Shows the voltage associated with the specified ADC channel */
> +static ssize_t powr1220_show_voltage(struct device *dev,
> + struct device_attribute *dev_attr, char *buf) {
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
> + int adc_val = powr1220_read_adc(dev, attr->index);
> +
> + return sprintf(buf, "%04d\n", adc_val); }
> +
> +
> +/* Shows the status associated with the specified vmon register */
> +static ssize_t powr1220_show_vmon_status(struct device *dev,
> + struct device_attribute *dev_attr, char *buf) {
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
> + struct powr1220_data *data = powr1220_update_device(dev);
> +
> + return sprintf(buf, "0x%02X\n", data->vmon_status[attr->index]); }
> +
> +
> +/* Shows the value of the input status register */ static ssize_t
> +powr1220_show_input_status(struct device *dev,
> + struct device_attribute *dev_attr, char *buf) {
> + struct powr1220_data *data = powr1220_update_device(dev);
> +
> + return sprintf(buf, "0x%02X\n", data->input_status); }
> +
> +
> +/* Shows the value of the input value register */ static ssize_t
> +powr1220_show_input_value(struct device *dev,
> + struct device_attribute *dev_attr, char *buf) {
> + struct powr1220_data *data = powr1220_update_device(dev);
> +
> + return sprintf(buf, "0x%02X\n", data->input_value); }
> +
> +
> +/* Sets the value of the input_value register. Input value is always
> +hex */ static ssize_t powr1220_set_input_value(struct device *dev,
> + struct device_attribute *dev_attr, const char *buf, size_t count) {
> + struct i2c_client *client = to_i2c_client(dev);
> + struct powr1220_data *data = i2c_get_clientdata(client);
> + unsigned long value;
> + int error;
> +
> + error = kstrtol(buf, 16, &value);
> + if (error)
> + return error;
> +
> + value &= 0xFE;
> +
> + mutex_lock(&data->update_lock);
> + i2c_smbus_write_byte_data(client, INPUT_VALUE, (u8)value);
> + mutex_unlock(&data->update_lock);
> +
> + return count;
> +}
> +
> +
> +/* Shows the value of the output status */ static ssize_t
> +powr1220_show_output_status(struct device *dev,
> + struct device_attribute *dev_attr, char *buf) {
> + struct powr1220_data *data = powr1220_update_device(dev);
> +
> + return sprintf(buf, "0x%06X\n", data->output_status); }
> +
> +
> +/* Shows the value of the gp output */ static ssize_t
> +powr1220_show_gp_output(struct device *dev,
> + struct device_attribute *dev_attr, char *buf) {
> + struct powr1220_data *data = powr1220_update_device(dev);
> +
> + return sprintf(buf, "0x%06X\n", data->gp_output); }
> +
> +
> +/* Sets the value of the gp_output registers. Input value is always
> +hex */ static ssize_t powr1220_set_gp_output(struct device *dev,
> + struct device_attribute *dev_attr, const char *buf, size_t count) {
> + struct i2c_client *client = to_i2c_client(dev);
> + struct powr1220_data *data = i2c_get_clientdata(client);
> + unsigned long value;
> + int error;
> + unsigned char regs[3];
> +
> + error = kstrtol(buf, 16, &value);
> + if (error)
> + return error;
> +
> + regs[0] = value & 0x0000FF;
> + regs[1] = (value & 0x00FF00) >> 8;
> + regs[2] = (value & 0x0F0000) >> 16;
> +
> + mutex_lock(&data->update_lock);
> + i2c_smbus_write_byte_data(client, GP_OUTPUT1, regs[0]);
> + i2c_smbus_write_byte_data(client, GP_OUTPUT2, regs[1]);
> + i2c_smbus_write_byte_data(client, GP_OUTPUT3, regs[2]);
> + mutex_unlock(&data->update_lock);
> +
> + return count;
> +}
> +
> +
> +/* Shows the value of the hvout */
> +static ssize_t powr1220_show_hvout(struct device *dev,
> + struct device_attribute *dev_attr, char *buf) {
> + struct powr1220_data *data = powr1220_update_device(dev);
> +
> + return sprintf(buf, "0x%02X\n", data->hvout); }
> +
> +
> +/* Shows the value of ues */
> +static ssize_t powr1220_show_ues(struct device *dev,
> + struct device_attribute *dev_attr, char *buf) {
> + struct powr1220_data *data = powr1220_update_device(dev);
> +
> + return sprintf(buf, "0x%08X\n", data->ues); }
> +
> +
> +/* Shows the value of the specified trim value */ static ssize_t
> +powr1220_show_trim(struct device *dev,
> + struct device_attribute *dev_attr, char *buf) {
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
> + struct powr1220_data *data = powr1220_update_device(dev);
> +
> + return sprintf(buf, "0x%02X\n", data->trim[attr->index]); }
> +
> +
> +/* Sets the value of the specified trim register. Input value is
> +always hex */ static ssize_t powr1220_set_trim(struct device *dev,
> + struct device_attribute *dev_attr, const char *buf, size_t count) {
> + struct i2c_client *client = to_i2c_client(dev);
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
> + struct powr1220_data *data = i2c_get_clientdata(client);
> + unsigned long value;
> + int error;
> +
> + error = kstrtol(buf, 16, &value);
> + if (error)
> + return error;
> +
> + mutex_lock(&data->update_lock);
> + i2c_smbus_write_byte_data(client, TRIM1_TRIM + attr->index, (u8)value);
> + mutex_unlock(&data->update_lock);
> +
> + return count;
> +}
> +
> +
> +/* Shows the value of the adc range (0 or 1) */ static ssize_t
> +powr1220_show_adc_range(struct device *dev,
> + struct device_attribute *dev_attr, char *buf) {
> + struct powr1220_data *data = powr1220_update_device(dev);
> +
> + return sprintf(buf, "%d\n", (data->adc_range ? 1 : 0)); }
> +
> +
> +/* Sets the value of the adc range (0 or 1) */ static ssize_t
> +powr1220_set_adc_range(struct device *dev,
> + struct device_attribute *dev_attr, const char *buf, size_t count) {
> + struct i2c_client *client = to_i2c_client(dev);
> + struct powr1220_data *data = i2c_get_clientdata(client);
> + unsigned long range;
> + int error;
> +
> + error = kstrtol(buf, 10, &range);
> + if (error)
> + return error;
> +
> + mutex_lock(&data->update_lock);
> + data->adc_range = (range ? 1 : 0);
> + mutex_unlock(&data->update_lock);
> +
> + return count;
> +}
> +
> +
> +
> +/* Reads the specified ADC channel */ static int
> +powr1220_read_adc(struct device *dev, int ch_num) {
> + struct i2c_client *client = to_i2c_client(dev);
> + struct powr1220_data *data = i2c_get_clientdata(client);
> + unsigned char reg;
> + int reading;
> +
> + mutex_lock(&data->update_lock);
> +
> + if (time_after(jiffies, data->adc_last_updated[ch_num] + HZ) ||
> + !data->adc_valid[ch_num]) {
> + /* set the attenuator and mux */
> + reg = ((data->adc_range & 1) << 4) | ch_num;
> + i2c_smbus_write_byte_data(client, ADC_MUX, reg);
> +
> + /* wait at least Tconvert time (200 us) for the
> + * conversion to complete */
> + udelay(200);
> +
> + /* check if the done bit is set */
> + reg = i2c_smbus_read_byte_data(client, ADC_VALUE_LOW);
> + if (reg & 0x01) {
> + /* we have a valid conversion to read */
> + reading = reg >> 4;
> + reg = i2c_smbus_read_byte_data(client, ADC_VALUE_HIGH);
> + reading |= (unsigned int)reg << 4;
> +
> + /* now convert the reading to a voltage */
> + data->adc_values[ch_num] = reading * ADC_STEP_MV;
> + }
> +
> + data->adc_last_updated[ch_num] = jiffies;
> + data->adc_valid[ch_num] = 1;
> + }
> +
> + mutex_unlock(&data->update_lock);
> +
> + return data->adc_values[ch_num];
> +}
> +
> +
> +/* Reads from the device and updates the local data if after the
> +timeout */ static struct powr1220_data *powr1220_update_device(struct
> +device *dev) {
> + struct i2c_client *client = to_i2c_client(dev);
> + struct powr1220_data *data = i2c_get_clientdata(client);
> + unsigned char regs[MAX_POWR1220_REGS];
> + int i;
> +
> + mutex_lock(&data->update_lock);
> +
> + if (time_after(jiffies, data->last_updated + HZ) || !data->valid) {
> + /* read the full register bank */
> + for (i = 0; i < MAX_POWR1220_REGS; i++)
> + regs[i] = i2c_smbus_read_byte_data(client, i);
> +
> + /* get the values of vmon A and B */
> + for (i = 0; i < ARRAY_SIZE(data->vmon_status); i++)
> + data->vmon_status[i] = regs[VMON_STATUS0 + i];
> +
> + /* get the input status and value */
> + data->input_status = regs[INPUT_STATUS] & 0x3F;
> + data->input_value = regs[INPUT_VALUE] & 0x3E;
> +
> + /* get the output status */
> + data->hvout = regs[OUTPUT_STATUS0] & 0x0F;
> + data->output_status = (regs[OUTPUT_STATUS0] >> 4) |
> + ((int)regs[OUTPUT_STATUS1] << 4) |
> + ((int)regs[OUTPUT_STATUS2] << 12);
> +
> + /* get the output */
> + data->gp_output = regs[GP_OUTPUT1] |
> + ((int)regs[GP_OUTPUT2] << 8) |
> + ((int)(regs[GP_OUTPUT3] & 0x0F) << 16);
> +
> + /* get the user signature*/
> + data->ues = regs[UES_BYTE0] |
> + ((int)regs[UES_BYTE1] << 8) |
> + ((int)regs[UES_BYTE2] << 16) |
> + ((int)regs[UES_BYTE3] << 24);
> +
> + /* get the trim values */
> + for (i = 0; i < ARRAY_SIZE(data->trim); i++)
> + data->trim[i] = regs[TRIM1_TRIM + i];
> +
> + data->last_updated = jiffies;
> + data->valid = 1;
> + }
> +
> + mutex_unlock(&data->update_lock);
> +
> + return data;
> +}
> +
> +
> +
> +static SENSOR_DEVICE_ATTR(vmon1, S_IRUGO, powr1220_show_voltage,
> +NULL, VMON1); static SENSOR_DEVICE_ATTR(vmon2, S_IRUGO,
> +powr1220_show_voltage, NULL, VMON2); static SENSOR_DEVICE_ATTR(vmon3,
> +S_IRUGO, powr1220_show_voltage, NULL, VMON3); static
> +SENSOR_DEVICE_ATTR(vmon4, S_IRUGO, powr1220_show_voltage, NULL,
> +VMON4); static SENSOR_DEVICE_ATTR(vmon5, S_IRUGO,
> +powr1220_show_voltage, NULL, VMON5); static SENSOR_DEVICE_ATTR(vmon6,
> +S_IRUGO, powr1220_show_voltage, NULL, VMON6); static
> +SENSOR_DEVICE_ATTR(vmon7, S_IRUGO, powr1220_show_voltage, NULL,
> +VMON7); static SENSOR_DEVICE_ATTR(vmon8, S_IRUGO, powr1220_show_voltage, NULL, VMON8); static SENSOR_DEVICE_ATTR(vmon9, S_IRUGO, powr1220_show_voltage, NULL, VMON9); static SENSOR_DEVICE_ATTR(vmon10, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON10);
> +static SENSOR_DEVICE_ATTR(vmon11, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON11);
> +static SENSOR_DEVICE_ATTR(vmon12, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON12);
> +static SENSOR_DEVICE_ATTR(vcca, S_IRUGO, powr1220_show_voltage, NULL,
> +VCCA); static SENSOR_DEVICE_ATTR(vccinp, S_IRUGO, powr1220_show_voltage, NULL,
> + VCCINP);
> +
> +static SENSOR_DEVICE_ATTR(reset, S_IWUSR, NULL, powr1220_reset, 0);
> +
> +static SENSOR_DEVICE_ATTR(vmon_status0, S_IRUGO, powr1220_show_vmon_status,
> + NULL, 0);
> +static SENSOR_DEVICE_ATTR(vmon_status1, S_IRUGO, powr1220_show_vmon_status,
> + NULL, 1);
> +static SENSOR_DEVICE_ATTR(vmon_status2, S_IRUGO, powr1220_show_vmon_status,
> + NULL, 2);
> +
> +static SENSOR_DEVICE_ATTR(input_status, S_IRUGO, powr1220_show_input_status,
> + NULL, 0);
> +static SENSOR_DEVICE_ATTR(input_value, S_IWUSR | S_IRUGO,
> + powr1220_show_input_value, powr1220_set_input_value, 0); static
> +SENSOR_DEVICE_ATTR(output_status, S_IRUGO, powr1220_show_output_status,
> + NULL, 0);
> +static SENSOR_DEVICE_ATTR(gp_output, S_IWUSR | S_IRUGO,
> + powr1220_show_gp_output, powr1220_set_gp_output, 0); static
> +SENSOR_DEVICE_ATTR(hvout, S_IRUGO, powr1220_show_hvout,
> + NULL, 0);
> +static SENSOR_DEVICE_ATTR(ues, S_IRUGO, powr1220_show_ues, NULL, 0);
> +
> +static SENSOR_DEVICE_ATTR(trim1, S_IWUSR | S_IRUGO, powr1220_show_trim,
> + powr1220_set_trim, 0);
> +static SENSOR_DEVICE_ATTR(trim2, S_IWUSR | S_IRUGO, powr1220_show_trim,
> + powr1220_set_trim, 1);
> +static SENSOR_DEVICE_ATTR(trim3, S_IWUSR | S_IRUGO, powr1220_show_trim,
> + powr1220_set_trim, 2);
> +static SENSOR_DEVICE_ATTR(trim4, S_IWUSR | S_IRUGO, powr1220_show_trim,
> + powr1220_set_trim, 3);
> +static SENSOR_DEVICE_ATTR(trim5, S_IWUSR | S_IRUGO, powr1220_show_trim,
> + powr1220_set_trim, 4);
> +static SENSOR_DEVICE_ATTR(trim6, S_IWUSR | S_IRUGO, powr1220_show_trim,
> + powr1220_set_trim, 5);
> +static SENSOR_DEVICE_ATTR(trim7, S_IWUSR | S_IRUGO, powr1220_show_trim,
> + powr1220_set_trim, 6);
> +static SENSOR_DEVICE_ATTR(trim8, S_IWUSR | S_IRUGO, powr1220_show_trim,
> + powr1220_set_trim, 7);
> +
> +static SENSOR_DEVICE_ATTR(adc_range, S_IWUSR | S_IRUGO,
> + powr1220_show_adc_range, powr1220_set_adc_range, 0);
> +
> +
> +
> +static struct attribute *powr1220_attributes[] = {
> + &sensor_dev_attr_vmon1.dev_attr.attr,
> + &sensor_dev_attr_vmon2.dev_attr.attr,
> + &sensor_dev_attr_vmon3.dev_attr.attr,
> + &sensor_dev_attr_vmon4.dev_attr.attr,
> + &sensor_dev_attr_vmon5.dev_attr.attr,
> + &sensor_dev_attr_vmon6.dev_attr.attr,
> + &sensor_dev_attr_vmon7.dev_attr.attr,
> + &sensor_dev_attr_vmon8.dev_attr.attr,
> + &sensor_dev_attr_vmon9.dev_attr.attr,
> + &sensor_dev_attr_vmon10.dev_attr.attr,
> + &sensor_dev_attr_vmon11.dev_attr.attr,
> + &sensor_dev_attr_vmon12.dev_attr.attr,
> + &sensor_dev_attr_vcca.dev_attr.attr,
> + &sensor_dev_attr_vccinp.dev_attr.attr,
> +
> + &sensor_dev_attr_reset.dev_attr.attr,
> +
> + &sensor_dev_attr_vmon_status0.dev_attr.attr,
> + &sensor_dev_attr_vmon_status1.dev_attr.attr,
> + &sensor_dev_attr_vmon_status2.dev_attr.attr,
> +
> + &sensor_dev_attr_input_status.dev_attr.attr,
> + &sensor_dev_attr_input_value.dev_attr.attr,
> + &sensor_dev_attr_output_status.dev_attr.attr,
> + &sensor_dev_attr_gp_output.dev_attr.attr,
> + &sensor_dev_attr_hvout.dev_attr.attr,
> + &sensor_dev_attr_ues.dev_attr.attr,
> +
> + &sensor_dev_attr_trim1.dev_attr.attr,
> + &sensor_dev_attr_trim2.dev_attr.attr,
> + &sensor_dev_attr_trim3.dev_attr.attr,
> + &sensor_dev_attr_trim4.dev_attr.attr,
> + &sensor_dev_attr_trim5.dev_attr.attr,
> + &sensor_dev_attr_trim6.dev_attr.attr,
> + &sensor_dev_attr_trim7.dev_attr.attr,
> + &sensor_dev_attr_trim8.dev_attr.attr,
> +
> + &sensor_dev_attr_adc_range.dev_attr.attr,
> +
> + NULL
> +};
> +
> +static const struct attribute_group powr1220_group = {
> + .attrs = powr1220_attributes,
> +};
> +
> +
> +static int powr1220_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct powr1220_data *data;
> + int status;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA))
> + return -EIO;
> +
> + data = kzalloc(sizeof(struct powr1220_data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + i2c_set_clientdata(client, data);
> + mutex_init(&data->update_lock);
> +
> + status = sysfs_create_group(&client->dev.kobj, &powr1220_group);
> + if (status)
> + goto exit_free;
> +
> + data->hwmon_dev = hwmon_device_register(&client->dev);
> + if (IS_ERR(data->hwmon_dev)) {
> + status = PTR_ERR(data->hwmon_dev);
> + goto exit_remove_files;
> + }
> +
> + dev_info(&client->dev, "initialized\n");
> +
> + return 0;
> +
> +exit_remove_files:
> + sysfs_remove_group(&client->dev.kobj, &powr1220_group);
> +exit_free:
> + kfree(data);
> + return status;
> +}
> +
> +
> +static int powr1220_remove(struct i2c_client *client) {
> + struct powr1220_data *data = i2c_get_clientdata(client);
> +
> + hwmon_device_unregister(data->hwmon_dev);
> + sysfs_remove_group(&client->dev.kobj, &powr1220_group);
> + kfree(data);
> + return 0;
> +}
> +
> +
> +
> +static const struct i2c_device_id powr1220_ids[] = {
> + { "powr1220", 0, },
> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, powr1220_ids);
> +
> +static struct i2c_driver powr1220_driver = {
> + .class = I2C_CLASS_HWMON,
> + .driver = {
> + .name = "powr1220",
> + },
> + .probe = powr1220_probe,
> + .remove = powr1220_remove,
> + .id_table = powr1220_ids,
> +};
> +
> +
> +static int __init powr1220_init(void) {
> + return i2c_add_driver(&powr1220_driver); }
> +
> +static void __exit powr1220_exit(void) {
> + i2c_del_driver(&powr1220_driver);
> +}
> +
> +
> +module_init(powr1220_init);
> +module_exit(powr1220_exit);
> +
> +MODULE_AUTHOR("Scott Kanowitz");
> +MODULE_DESCRIPTION("POWR1220 driver"); MODULE_LICENSE("GPL");
>
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 7+ messages in thread
* [lm-sensors] [PATCH] Added support for Lattice's POWR1220 power manager IC.
2014-06-10 0:33 [lm-sensors] [PATCH] Added support for Lattice's POWR1220 power manager IC Guenter Roeck
2014-06-10 14:25 ` Scott Kanowitz
@ 2014-06-11 20:35 ` Scott Kanowitz
2014-06-11 22:26 ` Guenter Roeck
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Scott Kanowitz @ 2014-06-11 20:35 UTC (permalink / raw)
To: lm-sensors
This patch adds support for Lattice's POWR1220 power manager IC. Read
access to all the ADCs on the chip are supported through the hwmon
sysfs files.
Signed-off-by: Scott Kanowitz <skanowitz@echo360.com>
---
Documentation/hwmon/powr1220 | 45 +++++
drivers/hwmon/Kconfig | 12 ++
drivers/hwmon/Makefile | 1 +
drivers/hwmon/powr1220.c | 416 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 474 insertions(+)
create mode 100644 Documentation/hwmon/powr1220
create mode 100644 drivers/hwmon/powr1220.c
diff --git a/Documentation/hwmon/powr1220 b/Documentation/hwmon/powr1220
new file mode 100644
index 0000000..21e44f7
--- /dev/null
+++ b/Documentation/hwmon/powr1220
@@ -0,0 +1,45 @@
+Kernel driver powr1220
+=========
+
+Supported chips:
+ * Lattice POWR1220AT8
+ Prefix: 'powr1220'
+ Addresses scanned: none
+ Datasheet: Publicly available at the Lattice website
+ http://www.latticesemi.com/
+
+Author: Scott Kanowitz <scott.kanowitz@gmail.com>
+
+Description
+-----------
+
+This driver supports the Lattice POWR1220AT8 chip. The POWR1220
+includes voltage monitoring for 14 inputs as well as trim settings
+for output voltages and GPIOs. This driver implements the voltage
+monitoring portion of the chip.
+
+Voltages are sampled by a 12-bit ADC with a step size of 2 mV.
+An in-line attenuator allows measurements from 0 to 6 V. The
+attenuator is enabled or disabled depending on the setting of the
+input's max value. The driver will enable the attenuator for any
+value over the low measurement range maximum of 2 V.
+
+The input naming convention is as follows:
+
+driver name pin name
+in0 VMON1
+in1 VMON2
+in2 VMON3
+in2 VMON4
+in4 VMON5
+in5 VMON6
+in6 VMON7
+in7 VMON8
+in8 VMON9
+in9 VMON10
+in10 VMON11
+in11 VMON12
+in12 VCCA
+in13 VCCINP
+
+The ADC readings are updated on request with a minimum period of 1s.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 4af0da9..fc4d7f1 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -608,6 +608,18 @@ config SENSORS_JC42
This driver can also be built as a module. If so, the module
will be called jc42.
+config SENSORS_POWR1220
+ tristate "Lattice POWR1220 Power Monitoring"
+ depends on I2C
+ default n
+ help
+ If you say yes here you get access to the hardware monitoring
+ functions of the Lattice POWR1220 isp Power Supply Monitoring,
+ Sequencing and Margining Controller.
+
+ This driver can also be built as a module. If so, the module
+ will be called powr1220.
+
config SENSORS_LINEAGE
tristate "Lineage Compact Power Line Power Entry Module"
depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index c48f987..842e6b3 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -119,6 +119,7 @@ obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o
obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
+obj-$(CONFIG_SENSORS_POWR1220) += powr1220.o
obj-$(CONFIG_SENSORS_S3C) += s3c-hwmon.o
obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o
obj-$(CONFIG_SENSORS_SCH5627) += sch5627.o
diff --git a/drivers/hwmon/powr1220.c b/drivers/hwmon/powr1220.c
new file mode 100644
index 0000000..59e5ff8
--- /dev/null
+++ b/drivers/hwmon/powr1220.c
@@ -0,0 +1,416 @@
+/*
+ * powr1220.c - Driver for the Lattice POWR1220 programmable power supply
+ * and monitor. Users can read all ADC inputs along with their labels
+ * using the sysfs nodes.
+ *
+ * Copyright (c) 2014 Echo360 http://www.echo360.com
+ * Scott Kanowitz <skanowitz@echo360.com> <scott.kanowitz@gmail.com>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/jiffies.h>
+#include <linux/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/delay.h>
+
+
+#define ADC_STEP_MV 2
+#define ADC_MAX_LOW_MEASUREMENT_MV 2000
+#define ADC_MAX_HIGH_MEASUREMENT_MV 6000
+
+enum powr1220_regs {
+ VMON_STATUS0,
+ VMON_STATUS1,
+ VMON_STATUS2,
+ OUTPUT_STATUS0,
+ OUTPUT_STATUS1,
+ OUTPUT_STATUS2,
+ INPUT_STATUS,
+ ADC_VALUE_LOW,
+ ADC_VALUE_HIGH,
+ ADC_MUX,
+ UES_BYTE0,
+ UES_BYTE1,
+ UES_BYTE2,
+ UES_BYTE3,
+ GP_OUTPUT1,
+ GP_OUTPUT2,
+ GP_OUTPUT3,
+ INPUT_VALUE,
+ RESET,
+ TRIM1_TRIM,
+ TRIM2_TRIM,
+ TRIM3_TRIM,
+ TRIM4_TRIM,
+ TRIM5_TRIM,
+ TRIM6_TRIM,
+ TRIM7_TRIM,
+ TRIM8_TRIM,
+ MAX_POWR1220_REGS
+};
+
+
+enum powr1220_adc_values {
+ VMON1,
+ VMON2,
+ VMON3,
+ VMON4,
+ VMON5,
+ VMON6,
+ VMON7,
+ VMON8,
+ VMON9,
+ VMON10,
+ VMON11,
+ VMON12,
+ VCCA,
+ VCCINP,
+ MAX_POWR1220_ADC_VALUES
+};
+
+
+struct powr1220_data {
+ struct i2c_client *client;
+ struct mutex update_lock;
+ bool adc_valid[MAX_POWR1220_ADC_VALUES];
+ /* the next value is in jiffies */
+ unsigned long adc_last_updated[MAX_POWR1220_ADC_VALUES];
+
+ /* values */
+ int adc_maxes[MAX_POWR1220_ADC_VALUES];
+ int adc_values[MAX_POWR1220_ADC_VALUES];
+};
+
+
+static const char * const input_names[] = {
+ [VMON1] = "vmon1",
+ [VMON2] = "vmon2",
+ [VMON3] = "vmon3",
+ [VMON4] = "vmon4",
+ [VMON5] = "vmon5",
+ [VMON6] = "vmon6",
+ [VMON7] = "vmon7",
+ [VMON8] = "vmon8",
+ [VMON9] = "vmon9",
+ [VMON10] = "vmon10",
+ [VMON11] = "vmon11",
+ [VMON12] = "vmon12",
+ [VCCA] = "vcca",
+ [VCCINP] = "vccinp",
+};
+
+
+/* Reads the specified ADC channel */
+static int powr1220_read_adc(struct device *dev, int ch_num)
+{
+ struct powr1220_data *data = dev_get_drvdata(dev);
+ unsigned char reg;
+ int reading;
+ unsigned char adc_range = 0;
+
+ mutex_lock(&data->update_lock);
+
+ if (time_after(jiffies, data->adc_last_updated[ch_num] + HZ) ||
+ !data->adc_valid[ch_num]) {
+ /* figure out if we need to use the attenuator for
+ * high inputs */
+ if (data->adc_maxes[ch_num] > ADC_MAX_LOW_MEASUREMENT_MV)
+ adc_range = 1;
+
+ /* set the attenuator and mux */
+ reg = (adc_range << 4) | ch_num;
+ i2c_smbus_write_byte_data(data->client, ADC_MUX, reg);
+
+ /* wait at least Tconvert time (200 us) for the
+ * conversion to complete */
+ udelay(200);
+
+ /* check if the done bit is set */
+ reg = i2c_smbus_read_byte_data(data->client, ADC_VALUE_LOW);
+ if (reg & 0x01) {
+ /* we have a valid conversion to read */
+ reading = reg >> 4;
+ reg = i2c_smbus_read_byte_data(data->client,
+ ADC_VALUE_HIGH);
+ reading |= (unsigned int)reg << 4;
+
+ /* now convert the reading to a voltage */
+ data->adc_values[ch_num] = reading * ADC_STEP_MV;
+ }
+
+ data->adc_last_updated[ch_num] = jiffies;
+ data->adc_valid[ch_num] = 1;
+ }
+
+ mutex_unlock(&data->update_lock);
+
+ return data->adc_values[ch_num];
+}
+
+
+/* Shows the voltage associated with the specified ADC channel */
+static ssize_t powr1220_show_voltage(struct device *dev,
+ struct device_attribute *dev_attr, char *buf)
+{
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
+ int adc_val = powr1220_read_adc(dev, attr->index);
+
+ return sprintf(buf, "%d\n", adc_val);
+}
+
+
+/* Shows the maximum setting associated with the specified ADC channel */
+static ssize_t powr1220_show_max(struct device *dev,
+ struct device_attribute *dev_attr, char *buf)
+{
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
+ struct powr1220_data *data = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%d\n", data->adc_maxes[attr->index]);
+}
+
+
+/* Sets the maximum setting associated with the specified ADC channel */
+static ssize_t powr1220_set_max(struct device *dev,
+ struct device_attribute *dev_attr, const char *buf, size_t count)
+{
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
+ struct powr1220_data *data = dev_get_drvdata(dev);
+ unsigned long max;
+ int error;
+
+ error = kstrtol(buf, 10, &max);
+ if (error)
+ return error;
+
+ max = clamp_val(max, 0, ADC_MAX_HIGH_MEASUREMENT_MV);
+
+ data->adc_maxes[attr->index] = max;
+
+ return count;
+}
+
+
+/* Shows the label associated with the specified ADC channel */
+static ssize_t powr1220_show_label(struct device *dev,
+ struct device_attribute *dev_attr, char *buf)
+{
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
+
+ return sprintf(buf, "%s\n", input_names[attr->index]);
+}
+
+
+static SENSOR_DEVICE_ATTR(in0_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON1);
+static SENSOR_DEVICE_ATTR(in1_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON2);
+static SENSOR_DEVICE_ATTR(in2_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON3);
+static SENSOR_DEVICE_ATTR(in3_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON4);
+static SENSOR_DEVICE_ATTR(in4_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON5);
+static SENSOR_DEVICE_ATTR(in5_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON6);
+static SENSOR_DEVICE_ATTR(in6_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON7);
+static SENSOR_DEVICE_ATTR(in7_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON8);
+static SENSOR_DEVICE_ATTR(in8_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON9);
+static SENSOR_DEVICE_ATTR(in9_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON10);
+static SENSOR_DEVICE_ATTR(in10_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON11);
+static SENSOR_DEVICE_ATTR(in11_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON12);
+static SENSOR_DEVICE_ATTR(in12_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VCCA);
+static SENSOR_DEVICE_ATTR(in13_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VCCINP);
+
+static SENSOR_DEVICE_ATTR(in0_max, S_IWUSR | S_IRUGO, powr1220_show_max,
+ powr1220_set_max, VMON1);
+static SENSOR_DEVICE_ATTR(in1_max, S_IWUSR | S_IRUGO, powr1220_show_max,
+ powr1220_set_max, VMON2);
+static SENSOR_DEVICE_ATTR(in2_max, S_IWUSR | S_IRUGO, powr1220_show_max,
+ powr1220_set_max, VMON3);
+static SENSOR_DEVICE_ATTR(in3_max, S_IWUSR | S_IRUGO, powr1220_show_max,
+ powr1220_set_max, VMON4);
+static SENSOR_DEVICE_ATTR(in4_max, S_IWUSR | S_IRUGO, powr1220_show_max,
+ powr1220_set_max, VMON5);
+static SENSOR_DEVICE_ATTR(in5_max, S_IWUSR | S_IRUGO, powr1220_show_max,
+ powr1220_set_max, VMON6);
+static SENSOR_DEVICE_ATTR(in6_max, S_IWUSR | S_IRUGO, powr1220_show_max,
+ powr1220_set_max, VMON7);
+static SENSOR_DEVICE_ATTR(in7_max, S_IWUSR | S_IRUGO, powr1220_show_max,
+ powr1220_set_max, VMON8);
+static SENSOR_DEVICE_ATTR(in8_max, S_IWUSR | S_IRUGO, powr1220_show_max,
+ powr1220_set_max, VMON9);
+static SENSOR_DEVICE_ATTR(in9_max, S_IWUSR | S_IRUGO, powr1220_show_max,
+ powr1220_set_max, VMON10);
+static SENSOR_DEVICE_ATTR(in10_max, S_IWUSR | S_IRUGO, powr1220_show_max,
+ powr1220_set_max, VMON11);
+static SENSOR_DEVICE_ATTR(in11_max, S_IWUSR | S_IRUGO, powr1220_show_max,
+ powr1220_set_max, VMON12);
+static SENSOR_DEVICE_ATTR(in12_max, S_IWUSR | S_IRUGO, powr1220_show_max,
+ powr1220_set_max, VCCA);
+static SENSOR_DEVICE_ATTR(in13_max, S_IWUSR | S_IRUGO, powr1220_show_max,
+ powr1220_set_max, VCCINP);
+
+static SENSOR_DEVICE_ATTR(in0_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON1);
+static SENSOR_DEVICE_ATTR(in1_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON2);
+static SENSOR_DEVICE_ATTR(in2_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON3);
+static SENSOR_DEVICE_ATTR(in3_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON4);
+static SENSOR_DEVICE_ATTR(in4_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON5);
+static SENSOR_DEVICE_ATTR(in5_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON6);
+static SENSOR_DEVICE_ATTR(in6_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON7);
+static SENSOR_DEVICE_ATTR(in7_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON8);
+static SENSOR_DEVICE_ATTR(in8_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON9);
+static SENSOR_DEVICE_ATTR(in9_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON10);
+static SENSOR_DEVICE_ATTR(in10_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON11);
+static SENSOR_DEVICE_ATTR(in11_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON12);
+static SENSOR_DEVICE_ATTR(in12_label, S_IRUGO, powr1220_show_label, NULL,
+ VCCA);
+static SENSOR_DEVICE_ATTR(in13_label, S_IRUGO, powr1220_show_label, NULL,
+ VCCINP);
+
+
+
+static struct attribute *powr1220_attrs[] = {
+ &sensor_dev_attr_in0_input.dev_attr.attr,
+ &sensor_dev_attr_in1_input.dev_attr.attr,
+ &sensor_dev_attr_in2_input.dev_attr.attr,
+ &sensor_dev_attr_in3_input.dev_attr.attr,
+ &sensor_dev_attr_in4_input.dev_attr.attr,
+ &sensor_dev_attr_in5_input.dev_attr.attr,
+ &sensor_dev_attr_in6_input.dev_attr.attr,
+ &sensor_dev_attr_in7_input.dev_attr.attr,
+ &sensor_dev_attr_in8_input.dev_attr.attr,
+ &sensor_dev_attr_in9_input.dev_attr.attr,
+ &sensor_dev_attr_in10_input.dev_attr.attr,
+ &sensor_dev_attr_in11_input.dev_attr.attr,
+ &sensor_dev_attr_in12_input.dev_attr.attr,
+ &sensor_dev_attr_in13_input.dev_attr.attr,
+
+ &sensor_dev_attr_in0_max.dev_attr.attr,
+ &sensor_dev_attr_in1_max.dev_attr.attr,
+ &sensor_dev_attr_in2_max.dev_attr.attr,
+ &sensor_dev_attr_in3_max.dev_attr.attr,
+ &sensor_dev_attr_in4_max.dev_attr.attr,
+ &sensor_dev_attr_in5_max.dev_attr.attr,
+ &sensor_dev_attr_in6_max.dev_attr.attr,
+ &sensor_dev_attr_in7_max.dev_attr.attr,
+ &sensor_dev_attr_in8_max.dev_attr.attr,
+ &sensor_dev_attr_in9_max.dev_attr.attr,
+ &sensor_dev_attr_in10_max.dev_attr.attr,
+ &sensor_dev_attr_in11_max.dev_attr.attr,
+ &sensor_dev_attr_in12_max.dev_attr.attr,
+ &sensor_dev_attr_in13_max.dev_attr.attr,
+
+ &sensor_dev_attr_in0_label.dev_attr.attr,
+ &sensor_dev_attr_in1_label.dev_attr.attr,
+ &sensor_dev_attr_in2_label.dev_attr.attr,
+ &sensor_dev_attr_in3_label.dev_attr.attr,
+ &sensor_dev_attr_in4_label.dev_attr.attr,
+ &sensor_dev_attr_in5_label.dev_attr.attr,
+ &sensor_dev_attr_in6_label.dev_attr.attr,
+ &sensor_dev_attr_in7_label.dev_attr.attr,
+ &sensor_dev_attr_in8_label.dev_attr.attr,
+ &sensor_dev_attr_in9_label.dev_attr.attr,
+ &sensor_dev_attr_in10_label.dev_attr.attr,
+ &sensor_dev_attr_in11_label.dev_attr.attr,
+ &sensor_dev_attr_in12_label.dev_attr.attr,
+ &sensor_dev_attr_in13_label.dev_attr.attr,
+
+ NULL
+};
+
+ATTRIBUTE_GROUPS(powr1220);
+
+
+static int powr1220_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct powr1220_data *data;
+ struct device *hwmon_dev;
+
+ if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
+ return -EIO;
+
+ data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ mutex_init(&data->update_lock);
+ data->client = client;
+
+ hwmon_dev = devm_hwmon_device_register_with_groups(&client->dev,
+ client->name, data, powr1220_groups);
+
+ return PTR_ERR_OR_ZERO(hwmon_dev);
+}
+
+
+static const struct i2c_device_id powr1220_ids[] = {
+ { "powr1220", 0, },
+ { }
+};
+
+MODULE_DEVICE_TABLE(i2c, powr1220_ids);
+
+static struct i2c_driver powr1220_driver = {
+ .class = I2C_CLASS_HWMON,
+ .driver = {
+ .name = "powr1220",
+ },
+ .probe = powr1220_probe,
+ .id_table = powr1220_ids,
+};
+
+
+static int __init powr1220_init(void)
+{
+ return i2c_add_driver(&powr1220_driver);
+}
+
+static void __exit powr1220_exit(void)
+{
+ i2c_del_driver(&powr1220_driver);
+}
+
+
+module_init(powr1220_init);
+module_exit(powr1220_exit);
+
+MODULE_AUTHOR("Scott Kanowitz");
+MODULE_DESCRIPTION("POWR1220 driver");
+MODULE_LICENSE("GPL");
--
1.8.3.2
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [lm-sensors] [PATCH] Added support for Lattice's POWR1220 power manager IC.
2014-06-10 0:33 [lm-sensors] [PATCH] Added support for Lattice's POWR1220 power manager IC Guenter Roeck
2014-06-10 14:25 ` Scott Kanowitz
2014-06-11 20:35 ` Scott Kanowitz
@ 2014-06-11 22:26 ` Guenter Roeck
2014-06-12 3:16 ` Scott Kanowitz
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2014-06-11 22:26 UTC (permalink / raw)
To: lm-sensors
On 06/11/2014 01:35 PM, Scott Kanowitz wrote:
> This patch adds support for Lattice's POWR1220 power manager IC. Read
> access to all the ADCs on the chip are supported through the hwmon
> sysfs files.
>
> Signed-off-by: Scott Kanowitz <skanowitz@echo360.com>
Hi Scott,
that was a pretty quick turnaround for such a major rewrite.
See inline for comments.
Thanks,
Guenter
> ---
> Documentation/hwmon/powr1220 | 45 +++++
> drivers/hwmon/Kconfig | 12 ++
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/powr1220.c | 416 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 474 insertions(+)
> create mode 100644 Documentation/hwmon/powr1220
> create mode 100644 drivers/hwmon/powr1220.c
>
> diff --git a/Documentation/hwmon/powr1220 b/Documentation/hwmon/powr1220
> new file mode 100644
> index 0000000..21e44f7
> --- /dev/null
> +++ b/Documentation/hwmon/powr1220
> @@ -0,0 +1,45 @@
> +Kernel driver powr1220
> +=========
> +
> +Supported chips:
> + * Lattice POWR1220AT8
> + Prefix: 'powr1220'
> + Addresses scanned: none
> + Datasheet: Publicly available at the Lattice website
> + http://www.latticesemi.com/
> +
> +Author: Scott Kanowitz <scott.kanowitz@gmail.com>
> +
> +Description
> +-----------
> +
> +This driver supports the Lattice POWR1220AT8 chip. The POWR1220
> +includes voltage monitoring for 14 inputs as well as trim settings
> +for output voltages and GPIOs. This driver implements the voltage
> +monitoring portion of the chip.
> +
> +Voltages are sampled by a 12-bit ADC with a step size of 2 mV.
> +An in-line attenuator allows measurements from 0 to 6 V. The
> +attenuator is enabled or disabled depending on the setting of the
> +input's max value. The driver will enable the attenuator for any
> +value over the low measurement range maximum of 2 V.
> +
> +The input naming convention is as follows:
> +
> +driver name pin name
> +in0 VMON1
> +in1 VMON2
> +in2 VMON3
> +in2 VMON4
> +in4 VMON5
> +in5 VMON6
> +in6 VMON7
> +in7 VMON8
> +in8 VMON9
> +in9 VMON10
> +in10 VMON11
> +in11 VMON12
> +in12 VCCA
> +in13 VCCINP
> +
> +The ADC readings are updated on request with a minimum period of 1s.
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 4af0da9..fc4d7f1 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -608,6 +608,18 @@ config SENSORS_JC42
> This driver can also be built as a module. If so, the module
> will be called jc42.
>
> +config SENSORS_POWR1220
> + tristate "Lattice POWR1220 Power Monitoring"
> + depends on I2C
> + default n
> + help
> + If you say yes here you get access to the hardware monitoring
> + functions of the Lattice POWR1220 isp Power Supply Monitoring,
> + Sequencing and Margining Controller.
> +
> + This driver can also be built as a module. If so, the module
> + will be called powr1220.
> +
> config SENSORS_LINEAGE
> tristate "Lineage Compact Power Line Power Entry Module"
> depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index c48f987..842e6b3 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -119,6 +119,7 @@ obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o
> obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
> obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
> obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
> +obj-$(CONFIG_SENSORS_POWR1220) += powr1220.o
> obj-$(CONFIG_SENSORS_S3C) += s3c-hwmon.o
> obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o
> obj-$(CONFIG_SENSORS_SCH5627) += sch5627.o
> diff --git a/drivers/hwmon/powr1220.c b/drivers/hwmon/powr1220.c
> new file mode 100644
> index 0000000..59e5ff8
> --- /dev/null
> +++ b/drivers/hwmon/powr1220.c
> @@ -0,0 +1,416 @@
> +/*
> + * powr1220.c - Driver for the Lattice POWR1220 programmable power supply
> + * and monitor. Users can read all ADC inputs along with their labels
> + * using the sysfs nodes.
> + *
> + * Copyright (c) 2014 Echo360 http://www.echo360.com
> + * Scott Kanowitz <skanowitz@echo360.com> <scott.kanowitz@gmail.com>
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/jiffies.h>
> +#include <linux/i2c.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/err.h>
> +#include <linux/mutex.h>
> +#include <linux/delay.h>
> +
> +
Only one empty line please.
> +#define ADC_STEP_MV 2
> +#define ADC_MAX_LOW_MEASUREMENT_MV 2000
> +#define ADC_MAX_HIGH_MEASUREMENT_MV 6000
Please use a tab after _MV.
> +
> +enum powr1220_regs {
> + VMON_STATUS0,
> + VMON_STATUS1,
> + VMON_STATUS2,
> + OUTPUT_STATUS0,
> + OUTPUT_STATUS1,
> + OUTPUT_STATUS2,
> + INPUT_STATUS,
> + ADC_VALUE_LOW,
> + ADC_VALUE_HIGH,
> + ADC_MUX,
> + UES_BYTE0,
> + UES_BYTE1,
> + UES_BYTE2,
> + UES_BYTE3,
> + GP_OUTPUT1,
> + GP_OUTPUT2,
> + GP_OUTPUT3,
> + INPUT_VALUE,
> + RESET,
> + TRIM1_TRIM,
> + TRIM2_TRIM,
> + TRIM3_TRIM,
> + TRIM4_TRIM,
> + TRIM5_TRIM,
> + TRIM6_TRIM,
> + TRIM7_TRIM,
> + TRIM8_TRIM,
> + MAX_POWR1220_REGS
> +};
> +
> +
One empty line
> +enum powr1220_adc_values {
> + VMON1,
> + VMON2,
> + VMON3,
> + VMON4,
> + VMON5,
> + VMON6,
> + VMON7,
> + VMON8,
> + VMON9,
> + VMON10,
> + VMON11,
> + VMON12,
> + VCCA,
> + VCCINP,
> + MAX_POWR1220_ADC_VALUES
> +};
> +
> +
One empty line
> +struct powr1220_data {
> + struct i2c_client *client;
> + struct mutex update_lock;
> + bool adc_valid[MAX_POWR1220_ADC_VALUES];
> + /* the next value is in jiffies */
> + unsigned long adc_last_updated[MAX_POWR1220_ADC_VALUES];
> +
> + /* values */
> + int adc_maxes[MAX_POWR1220_ADC_VALUES];
> + int adc_values[MAX_POWR1220_ADC_VALUES];
> +};
> +
> +
One empty line
> +static const char * const input_names[] = {
> + [VMON1] = "vmon1",
> + [VMON2] = "vmon2",
> + [VMON3] = "vmon3",
> + [VMON4] = "vmon4",
> + [VMON5] = "vmon5",
> + [VMON6] = "vmon6",
> + [VMON7] = "vmon7",
> + [VMON8] = "vmon8",
> + [VMON9] = "vmon9",
> + [VMON10] = "vmon10",
> + [VMON11] = "vmon11",
> + [VMON12] = "vmon12",
> + [VCCA] = "vcca",
> + [VCCINP] = "vccinp",
> +};
> +
> +
Single empty line
> +/* Reads the specified ADC channel */
> +static int powr1220_read_adc(struct device *dev, int ch_num)
> +{
> + struct powr1220_data *data = dev_get_drvdata(dev);
> + unsigned char reg;
> + int reading;
> + unsigned char adc_range = 0;
> +
> + mutex_lock(&data->update_lock);
> +
> + if (time_after(jiffies, data->adc_last_updated[ch_num] + HZ) ||
> + !data->adc_valid[ch_num]) {
> + /* figure out if we need to use the attenuator for
> + * high inputs */
/*
* Please follow coding style for multi-line comments
*/
> + if (data->adc_maxes[ch_num] > ADC_MAX_LOW_MEASUREMENT_MV)
> + adc_range = 1;
> +
> + /* set the attenuator and mux */
> + reg = (adc_range << 4) | ch_num;
If you assign (1 << 4) to adc_range above, you don't need to shift here
and make life a bit easier on the compiler.
> + i2c_smbus_write_byte_data(data->client, ADC_MUX, reg);
error check ?
> +
> + /* wait at least Tconvert time (200 us) for the
> + * conversion to complete */
multi-line comment
> + udelay(200);
> +
> + /* check if the done bit is set */
> + reg = i2c_smbus_read_byte_data(data->client, ADC_VALUE_LOW);
error check ? (reg needs to be int for that to work)
> + if (reg & 0x01) {
Datasheet says you don't need to check this bit after waiting for the maximum Tconvert.
> + /* we have a valid conversion to read */
> + reading = reg >> 4;
> + reg = i2c_smbus_read_byte_data(data->client,
> + ADC_VALUE_HIGH);
This can return an error (and is then negative). You should check for that condition
and handle it. Since the function always returns values >= 0 if there is no error,
you can return the negative error code and report it to the user.
> + reading |= (unsigned int)reg << 4;
Unnecessary typecast.
> +
> + /* now convert the reading to a voltage */
> + data->adc_values[ch_num] = reading * ADC_STEP_MV;
Is the factor the same for both low and high range ? Shouldn't the factor
be 6 for the high range ?
> + }
else error (though the conditional should not be necessary per above) ?
> +
> + data->adc_last_updated[ch_num] = jiffies;
> + data->adc_valid[ch_num] = 1;
= true;
and please move inside the if() statement. Otherwise the value is treated as updated
even if it wasn't.
> + }
> +
> + mutex_unlock(&data->update_lock);
> +
> + return data->adc_values[ch_num];
> +}
> +
> +
Single empty line
> +/* Shows the voltage associated with the specified ADC channel */
> +static ssize_t powr1220_show_voltage(struct device *dev,
> + struct device_attribute *dev_attr, char *buf)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
> + int adc_val = powr1220_read_adc(dev, attr->index);
> +
> + return sprintf(buf, "%d\n", adc_val);
> +}
> +
> +
> +/* Shows the maximum setting associated with the specified ADC channel */
> +static ssize_t powr1220_show_max(struct device *dev,
> + struct device_attribute *dev_attr, char *buf)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
> + struct powr1220_data *data = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%d\n", data->adc_maxes[attr->index]);
> +}
> +
> +
Single empty line
> +/* Sets the maximum setting associated with the specified ADC channel */
> +static ssize_t powr1220_set_max(struct device *dev,
> + struct device_attribute *dev_attr, const char *buf, size_t count)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
> + struct powr1220_data *data = dev_get_drvdata(dev);
> + unsigned long max;
> + int error;
> +
> + error = kstrtol(buf, 10, &max);
> + if (error)
> + return error;
> +
> + max = clamp_val(max, 0, ADC_MAX_HIGH_MEASUREMENT_MV);
> +
> + data->adc_maxes[attr->index] = max;
> +
It might make sense to set adc_maxes[] to either ADC_MAX_LOW_MEASUREMENT_MV
or ADC_MAX_HIGH_MEASUREMENT_MV depending on the entered value. Otherwise
returns a pretty much random number which doesn't really make much sense.
If you set it to one of the two values, at least the user will have an idea
if the value was too low.
Alternative (which I would prefer) would be to auto-configure the max
value and get rid of the max attributes. This should be quite simple:
- start with the large range.
- if the reported voltage is below ADC_MAX_LOW_MEASUREMENT_MV,
remember it and use the low range next time.
- if, with the range set to ADC_MAX_LOW_MEASUREMENT_MV, the reported value
gets close to ADC_MAX_LOW_MEASUREMENT_MV, switch to the larger range.
> + return count;
> +}
> +
> +
Single empty line
> +/* Shows the label associated with the specified ADC channel */
> +static ssize_t powr1220_show_label(struct device *dev,
> + struct device_attribute *dev_attr, char *buf)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
> +
> + return sprintf(buf, "%s\n", input_names[attr->index]);
> +}
> +
> +
Single empty line
> +static SENSOR_DEVICE_ATTR(in0_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON1);
> +static SENSOR_DEVICE_ATTR(in1_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON2);
> +static SENSOR_DEVICE_ATTR(in2_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON3);
> +static SENSOR_DEVICE_ATTR(in3_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON4);
> +static SENSOR_DEVICE_ATTR(in4_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON5);
> +static SENSOR_DEVICE_ATTR(in5_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON6);
> +static SENSOR_DEVICE_ATTR(in6_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON7);
> +static SENSOR_DEVICE_ATTR(in7_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON8);
> +static SENSOR_DEVICE_ATTR(in8_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON9);
> +static SENSOR_DEVICE_ATTR(in9_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON10);
> +static SENSOR_DEVICE_ATTR(in10_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON11);
> +static SENSOR_DEVICE_ATTR(in11_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON12);
> +static SENSOR_DEVICE_ATTR(in12_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VCCA);
> +static SENSOR_DEVICE_ATTR(in13_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VCCINP);
> +
> +static SENSOR_DEVICE_ATTR(in0_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON1);
> +static SENSOR_DEVICE_ATTR(in1_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON2);
> +static SENSOR_DEVICE_ATTR(in2_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON3);
> +static SENSOR_DEVICE_ATTR(in3_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON4);
> +static SENSOR_DEVICE_ATTR(in4_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON5);
> +static SENSOR_DEVICE_ATTR(in5_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON6);
> +static SENSOR_DEVICE_ATTR(in6_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON7);
> +static SENSOR_DEVICE_ATTR(in7_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON8);
> +static SENSOR_DEVICE_ATTR(in8_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON9);
> +static SENSOR_DEVICE_ATTR(in9_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON10);
> +static SENSOR_DEVICE_ATTR(in10_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON11);
> +static SENSOR_DEVICE_ATTR(in11_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON12);
> +static SENSOR_DEVICE_ATTR(in12_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VCCA);
> +static SENSOR_DEVICE_ATTR(in13_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VCCINP);
> +
> +static SENSOR_DEVICE_ATTR(in0_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON1);
> +static SENSOR_DEVICE_ATTR(in1_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON2);
> +static SENSOR_DEVICE_ATTR(in2_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON3);
> +static SENSOR_DEVICE_ATTR(in3_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON4);
> +static SENSOR_DEVICE_ATTR(in4_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON5);
> +static SENSOR_DEVICE_ATTR(in5_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON6);
> +static SENSOR_DEVICE_ATTR(in6_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON7);
> +static SENSOR_DEVICE_ATTR(in7_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON8);
> +static SENSOR_DEVICE_ATTR(in8_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON9);
> +static SENSOR_DEVICE_ATTR(in9_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON10);
> +static SENSOR_DEVICE_ATTR(in10_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON11);
> +static SENSOR_DEVICE_ATTR(in11_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON12);
> +static SENSOR_DEVICE_ATTR(in12_label, S_IRUGO, powr1220_show_label, NULL,
> + VCCA);
> +static SENSOR_DEVICE_ATTR(in13_label, S_IRUGO, powr1220_show_label, NULL,
> + VCCINP);
> +
> +
> +
Single empty line
> +static struct attribute *powr1220_attrs[] = {
> + &sensor_dev_attr_in0_input.dev_attr.attr,
> + &sensor_dev_attr_in1_input.dev_attr.attr,
> + &sensor_dev_attr_in2_input.dev_attr.attr,
> + &sensor_dev_attr_in3_input.dev_attr.attr,
> + &sensor_dev_attr_in4_input.dev_attr.attr,
> + &sensor_dev_attr_in5_input.dev_attr.attr,
> + &sensor_dev_attr_in6_input.dev_attr.attr,
> + &sensor_dev_attr_in7_input.dev_attr.attr,
> + &sensor_dev_attr_in8_input.dev_attr.attr,
> + &sensor_dev_attr_in9_input.dev_attr.attr,
> + &sensor_dev_attr_in10_input.dev_attr.attr,
> + &sensor_dev_attr_in11_input.dev_attr.attr,
> + &sensor_dev_attr_in12_input.dev_attr.attr,
> + &sensor_dev_attr_in13_input.dev_attr.attr,
> +
> + &sensor_dev_attr_in0_max.dev_attr.attr,
> + &sensor_dev_attr_in1_max.dev_attr.attr,
> + &sensor_dev_attr_in2_max.dev_attr.attr,
> + &sensor_dev_attr_in3_max.dev_attr.attr,
> + &sensor_dev_attr_in4_max.dev_attr.attr,
> + &sensor_dev_attr_in5_max.dev_attr.attr,
> + &sensor_dev_attr_in6_max.dev_attr.attr,
> + &sensor_dev_attr_in7_max.dev_attr.attr,
> + &sensor_dev_attr_in8_max.dev_attr.attr,
> + &sensor_dev_attr_in9_max.dev_attr.attr,
> + &sensor_dev_attr_in10_max.dev_attr.attr,
> + &sensor_dev_attr_in11_max.dev_attr.attr,
> + &sensor_dev_attr_in12_max.dev_attr.attr,
> + &sensor_dev_attr_in13_max.dev_attr.attr,
> +
> + &sensor_dev_attr_in0_label.dev_attr.attr,
> + &sensor_dev_attr_in1_label.dev_attr.attr,
> + &sensor_dev_attr_in2_label.dev_attr.attr,
> + &sensor_dev_attr_in3_label.dev_attr.attr,
> + &sensor_dev_attr_in4_label.dev_attr.attr,
> + &sensor_dev_attr_in5_label.dev_attr.attr,
> + &sensor_dev_attr_in6_label.dev_attr.attr,
> + &sensor_dev_attr_in7_label.dev_attr.attr,
> + &sensor_dev_attr_in8_label.dev_attr.attr,
> + &sensor_dev_attr_in9_label.dev_attr.attr,
> + &sensor_dev_attr_in10_label.dev_attr.attr,
> + &sensor_dev_attr_in11_label.dev_attr.attr,
> + &sensor_dev_attr_in12_label.dev_attr.attr,
> + &sensor_dev_attr_in13_label.dev_attr.attr,
> +
> + NULL
> +};
> +
> +ATTRIBUTE_GROUPS(powr1220);
> +
> +
Single empty line
> +static int powr1220_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct powr1220_data *data;
> + struct device *hwmon_dev;
> +
> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> + return -EIO;
-ENODEV is commonly used here.
> +
> + data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + mutex_init(&data->update_lock);
> + data->client = client;
> +
> + hwmon_dev = devm_hwmon_device_register_with_groups(&client->dev,
> + client->name, data, powr1220_groups);
> +
> + return PTR_ERR_OR_ZERO(hwmon_dev);
> +}
> +
> +
Single empty line.
> +static const struct i2c_device_id powr1220_ids[] = {
> + { "powr1220", 0, },
> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, powr1220_ids);
> +
> +static struct i2c_driver powr1220_driver = {
> + .class = I2C_CLASS_HWMON,
> + .driver = {
> + .name = "powr1220",
> + },
> + .probe = powr1220_probe,
> + .id_table = powr1220_ids,
> +};
> +
> +
> +static int __init powr1220_init(void)
> +{
> + return i2c_add_driver(&powr1220_driver);
> +}
> +
> +static void __exit powr1220_exit(void)
> +{
> + i2c_del_driver(&powr1220_driver);
> +}
> +
> +
> +module_init(powr1220_init);
> +module_exit(powr1220_exit);
> +
Please use module_i2c_driver().
> +MODULE_AUTHOR("Scott Kanowitz");
> +MODULE_DESCRIPTION("POWR1220 driver");
> +MODULE_LICENSE("GPL");
>
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [lm-sensors] [PATCH] Added support for Lattice's POWR1220 power manager IC.
2014-06-10 0:33 [lm-sensors] [PATCH] Added support for Lattice's POWR1220 power manager IC Guenter Roeck
` (2 preceding siblings ...)
2014-06-11 22:26 ` Guenter Roeck
@ 2014-06-12 3:16 ` Scott Kanowitz
2014-06-12 20:22 ` Scott Kanowitz
2014-06-13 18:10 ` Guenter Roeck
5 siblings, 0 replies; 7+ messages in thread
From: Scott Kanowitz @ 2014-06-12 3:16 UTC (permalink / raw)
To: lm-sensors
Hi Guenter,
I'll update with your changes and resubmit the patch.
To answer your question:
> Is the factor the same for both low and high range ? Shouldn't the factor be 6 for the high range ?
The factor is always 2 mV. The attenuator circuitry implements a divide by three before the ADC followed by a multiply by three. Therefore the step size is always 2 mV, you just lose resolution with the voltages in the higher range.
--Scott
-----Original Message-----
From: Guenter Roeck [mailto:linux@roeck-us.net]
Sent: Wednesday, June 11, 2014 6:26 PM
To: Scott Kanowitz
Cc: jdelvare@suse.de; lm-sensors@lm-sensors.org
Subject: Re: [PATCH] Added support for Lattice's POWR1220 power manager IC.
On 06/11/2014 01:35 PM, Scott Kanowitz wrote:
> This patch adds support for Lattice's POWR1220 power manager IC. Read
> access to all the ADCs on the chip are supported through the hwmon
> sysfs files.
>
> Signed-off-by: Scott Kanowitz <skanowitz@echo360.com>
Hi Scott,
that was a pretty quick turnaround for such a major rewrite.
See inline for comments.
Thanks,
Guenter
> ---
> Documentation/hwmon/powr1220 | 45 +++++
> drivers/hwmon/Kconfig | 12 ++
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/powr1220.c | 416 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 474 insertions(+)
> create mode 100644 Documentation/hwmon/powr1220
> create mode 100644 drivers/hwmon/powr1220.c
>
> diff --git a/Documentation/hwmon/powr1220
> b/Documentation/hwmon/powr1220 new file mode 100644 index
> 0000000..21e44f7
> --- /dev/null
> +++ b/Documentation/hwmon/powr1220
> @@ -0,0 +1,45 @@
> +Kernel driver powr1220
> +=========
> +
> +Supported chips:
> + * Lattice POWR1220AT8
> + Prefix: 'powr1220'
> + Addresses scanned: none
> + Datasheet: Publicly available at the Lattice website
> + http://www.latticesemi.com/
> +
> +Author: Scott Kanowitz <scott.kanowitz@gmail.com>
> +
> +Description
> +-----------
> +
> +This driver supports the Lattice POWR1220AT8 chip. The POWR1220
> +includes voltage monitoring for 14 inputs as well as trim settings
> +for output voltages and GPIOs. This driver implements the voltage
> +monitoring portion of the chip.
> +
> +Voltages are sampled by a 12-bit ADC with a step size of 2 mV.
> +An in-line attenuator allows measurements from 0 to 6 V. The
> +attenuator is enabled or disabled depending on the setting of the
> +input's max value. The driver will enable the attenuator for any
> +value over the low measurement range maximum of 2 V.
> +
> +The input naming convention is as follows:
> +
> +driver name pin name
> +in0 VMON1
> +in1 VMON2
> +in2 VMON3
> +in2 VMON4
> +in4 VMON5
> +in5 VMON6
> +in6 VMON7
> +in7 VMON8
> +in8 VMON9
> +in9 VMON10
> +in10 VMON11
> +in11 VMON12
> +in12 VCCA
> +in13 VCCINP
> +
> +The ADC readings are updated on request with a minimum period of 1s.
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index
> 4af0da9..fc4d7f1 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -608,6 +608,18 @@ config SENSORS_JC42
> This driver can also be built as a module. If so, the module
> will be called jc42.
>
> +config SENSORS_POWR1220
> + tristate "Lattice POWR1220 Power Monitoring"
> + depends on I2C
> + default n
> + help
> + If you say yes here you get access to the hardware monitoring
> + functions of the Lattice POWR1220 isp Power Supply Monitoring,
> + Sequencing and Margining Controller.
> +
> + This driver can also be built as a module. If so, the module
> + will be called powr1220.
> +
> config SENSORS_LINEAGE
> tristate "Lineage Compact Power Line Power Entry Module"
> depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index
> c48f987..842e6b3 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -119,6 +119,7 @@ obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o
> obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
> obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
> obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
> +obj-$(CONFIG_SENSORS_POWR1220) += powr1220.o
> obj-$(CONFIG_SENSORS_S3C) += s3c-hwmon.o
> obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o
> obj-$(CONFIG_SENSORS_SCH5627) += sch5627.o
> diff --git a/drivers/hwmon/powr1220.c b/drivers/hwmon/powr1220.c new
> file mode 100644 index 0000000..59e5ff8
> --- /dev/null
> +++ b/drivers/hwmon/powr1220.c
> @@ -0,0 +1,416 @@
> +/*
> + * powr1220.c - Driver for the Lattice POWR1220 programmable power
> +supply
> + * and monitor. Users can read all ADC inputs along with their labels
> + * using the sysfs nodes.
> + *
> + * Copyright (c) 2014 Echo360 http://www.echo360.com
> + * Scott Kanowitz <skanowitz@echo360.com> <scott.kanowitz@gmail.com>
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/jiffies.h>
> +#include <linux/i2c.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/err.h>
> +#include <linux/mutex.h>
> +#include <linux/delay.h>
> +
> +
Only one empty line please.
> +#define ADC_STEP_MV 2
> +#define ADC_MAX_LOW_MEASUREMENT_MV 2000 #define
> +ADC_MAX_HIGH_MEASUREMENT_MV 6000
Please use a tab after _MV.
> +
> +enum powr1220_regs {
> + VMON_STATUS0,
> + VMON_STATUS1,
> + VMON_STATUS2,
> + OUTPUT_STATUS0,
> + OUTPUT_STATUS1,
> + OUTPUT_STATUS2,
> + INPUT_STATUS,
> + ADC_VALUE_LOW,
> + ADC_VALUE_HIGH,
> + ADC_MUX,
> + UES_BYTE0,
> + UES_BYTE1,
> + UES_BYTE2,
> + UES_BYTE3,
> + GP_OUTPUT1,
> + GP_OUTPUT2,
> + GP_OUTPUT3,
> + INPUT_VALUE,
> + RESET,
> + TRIM1_TRIM,
> + TRIM2_TRIM,
> + TRIM3_TRIM,
> + TRIM4_TRIM,
> + TRIM5_TRIM,
> + TRIM6_TRIM,
> + TRIM7_TRIM,
> + TRIM8_TRIM,
> + MAX_POWR1220_REGS
> +};
> +
> +
One empty line
> +enum powr1220_adc_values {
> + VMON1,
> + VMON2,
> + VMON3,
> + VMON4,
> + VMON5,
> + VMON6,
> + VMON7,
> + VMON8,
> + VMON9,
> + VMON10,
> + VMON11,
> + VMON12,
> + VCCA,
> + VCCINP,
> + MAX_POWR1220_ADC_VALUES
> +};
> +
> +
One empty line
> +struct powr1220_data {
> + struct i2c_client *client;
> + struct mutex update_lock;
> + bool adc_valid[MAX_POWR1220_ADC_VALUES];
> + /* the next value is in jiffies */
> + unsigned long adc_last_updated[MAX_POWR1220_ADC_VALUES];
> +
> + /* values */
> + int adc_maxes[MAX_POWR1220_ADC_VALUES];
> + int adc_values[MAX_POWR1220_ADC_VALUES];
> +};
> +
> +
One empty line
> +static const char * const input_names[] = {
> + [VMON1] = "vmon1",
> + [VMON2] = "vmon2",
> + [VMON3] = "vmon3",
> + [VMON4] = "vmon4",
> + [VMON5] = "vmon5",
> + [VMON6] = "vmon6",
> + [VMON7] = "vmon7",
> + [VMON8] = "vmon8",
> + [VMON9] = "vmon9",
> + [VMON10] = "vmon10",
> + [VMON11] = "vmon11",
> + [VMON12] = "vmon12",
> + [VCCA] = "vcca",
> + [VCCINP] = "vccinp",
> +};
> +
> +
Single empty line
> +/* Reads the specified ADC channel */ static int
> +powr1220_read_adc(struct device *dev, int ch_num) {
> + struct powr1220_data *data = dev_get_drvdata(dev);
> + unsigned char reg;
> + int reading;
> + unsigned char adc_range = 0;
> +
> + mutex_lock(&data->update_lock);
> +
> + if (time_after(jiffies, data->adc_last_updated[ch_num] + HZ) ||
> + !data->adc_valid[ch_num]) {
> + /* figure out if we need to use the attenuator for
> + * high inputs */
/*
* Please follow coding style for multi-line comments
*/
> + if (data->adc_maxes[ch_num] > ADC_MAX_LOW_MEASUREMENT_MV)
> + adc_range = 1;
> +
> + /* set the attenuator and mux */
> + reg = (adc_range << 4) | ch_num;
If you assign (1 << 4) to adc_range above, you don't need to shift here and make life a bit easier on the compiler.
> + i2c_smbus_write_byte_data(data->client, ADC_MUX, reg);
error check ?
> +
> + /* wait at least Tconvert time (200 us) for the
> + * conversion to complete */
multi-line comment
> + udelay(200);
> +
> + /* check if the done bit is set */
> + reg = i2c_smbus_read_byte_data(data->client, ADC_VALUE_LOW);
error check ? (reg needs to be int for that to work)
> + if (reg & 0x01) {
Datasheet says you don't need to check this bit after waiting for the maximum Tconvert.
> + /* we have a valid conversion to read */
> + reading = reg >> 4;
> + reg = i2c_smbus_read_byte_data(data->client,
> + ADC_VALUE_HIGH);
This can return an error (and is then negative). You should check for that condition and handle it. Since the function always returns values >= 0 if there is no error, you can return the negative error code and report it to the user.
> + reading |= (unsigned int)reg << 4;
Unnecessary typecast.
> +
> + /* now convert the reading to a voltage */
> + data->adc_values[ch_num] = reading * ADC_STEP_MV;
Is the factor the same for both low and high range ? Shouldn't the factor be 6 for the high range ?
> + }
else error (though the conditional should not be necessary per above) ?
> +
> + data->adc_last_updated[ch_num] = jiffies;
> + data->adc_valid[ch_num] = 1;
= true;
and please move inside the if() statement. Otherwise the value is treated as updated even if it wasn't.
> + }
> +
> + mutex_unlock(&data->update_lock);
> +
> + return data->adc_values[ch_num];
> +}
> +
> +
Single empty line
> +/* Shows the voltage associated with the specified ADC channel */
> +static ssize_t powr1220_show_voltage(struct device *dev,
> + struct device_attribute *dev_attr, char *buf) {
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
> + int adc_val = powr1220_read_adc(dev, attr->index);
> +
> + return sprintf(buf, "%d\n", adc_val); }
> +
> +
> +/* Shows the maximum setting associated with the specified ADC
> +channel */ static ssize_t powr1220_show_max(struct device *dev,
> + struct device_attribute *dev_attr, char *buf) {
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
> + struct powr1220_data *data = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%d\n", data->adc_maxes[attr->index]); }
> +
> +
Single empty line
> +/* Sets the maximum setting associated with the specified ADC channel
> +*/ static ssize_t powr1220_set_max(struct device *dev,
> + struct device_attribute *dev_attr, const char *buf, size_t count) {
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
> + struct powr1220_data *data = dev_get_drvdata(dev);
> + unsigned long max;
> + int error;
> +
> + error = kstrtol(buf, 10, &max);
> + if (error)
> + return error;
> +
> + max = clamp_val(max, 0, ADC_MAX_HIGH_MEASUREMENT_MV);
> +
> + data->adc_maxes[attr->index] = max;
> +
It might make sense to set adc_maxes[] to either ADC_MAX_LOW_MEASUREMENT_MV or ADC_MAX_HIGH_MEASUREMENT_MV depending on the entered value. Otherwise returns a pretty much random number which doesn't really make much sense.
If you set it to one of the two values, at least the user will have an idea if the value was too low.
Alternative (which I would prefer) would be to auto-configure the max value and get rid of the max attributes. This should be quite simple:
- start with the large range.
- if the reported voltage is below ADC_MAX_LOW_MEASUREMENT_MV,
remember it and use the low range next time.
- if, with the range set to ADC_MAX_LOW_MEASUREMENT_MV, the reported value
gets close to ADC_MAX_LOW_MEASUREMENT_MV, switch to the larger range.
> + return count;
> +}
> +
> +
Single empty line
> +/* Shows the label associated with the specified ADC channel */
> +static ssize_t powr1220_show_label(struct device *dev,
> + struct device_attribute *dev_attr, char *buf) {
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
> +
> + return sprintf(buf, "%s\n", input_names[attr->index]); }
> +
> +
Single empty line
> +static SENSOR_DEVICE_ATTR(in0_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON1);
> +static SENSOR_DEVICE_ATTR(in1_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON2);
> +static SENSOR_DEVICE_ATTR(in2_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON3);
> +static SENSOR_DEVICE_ATTR(in3_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON4);
> +static SENSOR_DEVICE_ATTR(in4_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON5);
> +static SENSOR_DEVICE_ATTR(in5_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON6);
> +static SENSOR_DEVICE_ATTR(in6_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON7);
> +static SENSOR_DEVICE_ATTR(in7_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON8);
> +static SENSOR_DEVICE_ATTR(in8_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON9);
> +static SENSOR_DEVICE_ATTR(in9_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON10);
> +static SENSOR_DEVICE_ATTR(in10_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON11);
> +static SENSOR_DEVICE_ATTR(in11_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VMON12);
> +static SENSOR_DEVICE_ATTR(in12_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VCCA);
> +static SENSOR_DEVICE_ATTR(in13_input, S_IRUGO, powr1220_show_voltage, NULL,
> + VCCINP);
> +
> +static SENSOR_DEVICE_ATTR(in0_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON1);
> +static SENSOR_DEVICE_ATTR(in1_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON2);
> +static SENSOR_DEVICE_ATTR(in2_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON3);
> +static SENSOR_DEVICE_ATTR(in3_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON4);
> +static SENSOR_DEVICE_ATTR(in4_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON5);
> +static SENSOR_DEVICE_ATTR(in5_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON6);
> +static SENSOR_DEVICE_ATTR(in6_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON7);
> +static SENSOR_DEVICE_ATTR(in7_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON8);
> +static SENSOR_DEVICE_ATTR(in8_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON9);
> +static SENSOR_DEVICE_ATTR(in9_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON10);
> +static SENSOR_DEVICE_ATTR(in10_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON11);
> +static SENSOR_DEVICE_ATTR(in11_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VMON12);
> +static SENSOR_DEVICE_ATTR(in12_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VCCA);
> +static SENSOR_DEVICE_ATTR(in13_max, S_IWUSR | S_IRUGO, powr1220_show_max,
> + powr1220_set_max, VCCINP);
> +
> +static SENSOR_DEVICE_ATTR(in0_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON1);
> +static SENSOR_DEVICE_ATTR(in1_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON2);
> +static SENSOR_DEVICE_ATTR(in2_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON3);
> +static SENSOR_DEVICE_ATTR(in3_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON4);
> +static SENSOR_DEVICE_ATTR(in4_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON5);
> +static SENSOR_DEVICE_ATTR(in5_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON6);
> +static SENSOR_DEVICE_ATTR(in6_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON7);
> +static SENSOR_DEVICE_ATTR(in7_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON8);
> +static SENSOR_DEVICE_ATTR(in8_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON9);
> +static SENSOR_DEVICE_ATTR(in9_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON10);
> +static SENSOR_DEVICE_ATTR(in10_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON11);
> +static SENSOR_DEVICE_ATTR(in11_label, S_IRUGO, powr1220_show_label, NULL,
> + VMON12);
> +static SENSOR_DEVICE_ATTR(in12_label, S_IRUGO, powr1220_show_label, NULL,
> + VCCA);
> +static SENSOR_DEVICE_ATTR(in13_label, S_IRUGO, powr1220_show_label, NULL,
> + VCCINP);
> +
> +
> +
Single empty line
> +static struct attribute *powr1220_attrs[] = {
> + &sensor_dev_attr_in0_input.dev_attr.attr,
> + &sensor_dev_attr_in1_input.dev_attr.attr,
> + &sensor_dev_attr_in2_input.dev_attr.attr,
> + &sensor_dev_attr_in3_input.dev_attr.attr,
> + &sensor_dev_attr_in4_input.dev_attr.attr,
> + &sensor_dev_attr_in5_input.dev_attr.attr,
> + &sensor_dev_attr_in6_input.dev_attr.attr,
> + &sensor_dev_attr_in7_input.dev_attr.attr,
> + &sensor_dev_attr_in8_input.dev_attr.attr,
> + &sensor_dev_attr_in9_input.dev_attr.attr,
> + &sensor_dev_attr_in10_input.dev_attr.attr,
> + &sensor_dev_attr_in11_input.dev_attr.attr,
> + &sensor_dev_attr_in12_input.dev_attr.attr,
> + &sensor_dev_attr_in13_input.dev_attr.attr,
> +
> + &sensor_dev_attr_in0_max.dev_attr.attr,
> + &sensor_dev_attr_in1_max.dev_attr.attr,
> + &sensor_dev_attr_in2_max.dev_attr.attr,
> + &sensor_dev_attr_in3_max.dev_attr.attr,
> + &sensor_dev_attr_in4_max.dev_attr.attr,
> + &sensor_dev_attr_in5_max.dev_attr.attr,
> + &sensor_dev_attr_in6_max.dev_attr.attr,
> + &sensor_dev_attr_in7_max.dev_attr.attr,
> + &sensor_dev_attr_in8_max.dev_attr.attr,
> + &sensor_dev_attr_in9_max.dev_attr.attr,
> + &sensor_dev_attr_in10_max.dev_attr.attr,
> + &sensor_dev_attr_in11_max.dev_attr.attr,
> + &sensor_dev_attr_in12_max.dev_attr.attr,
> + &sensor_dev_attr_in13_max.dev_attr.attr,
> +
> + &sensor_dev_attr_in0_label.dev_attr.attr,
> + &sensor_dev_attr_in1_label.dev_attr.attr,
> + &sensor_dev_attr_in2_label.dev_attr.attr,
> + &sensor_dev_attr_in3_label.dev_attr.attr,
> + &sensor_dev_attr_in4_label.dev_attr.attr,
> + &sensor_dev_attr_in5_label.dev_attr.attr,
> + &sensor_dev_attr_in6_label.dev_attr.attr,
> + &sensor_dev_attr_in7_label.dev_attr.attr,
> + &sensor_dev_attr_in8_label.dev_attr.attr,
> + &sensor_dev_attr_in9_label.dev_attr.attr,
> + &sensor_dev_attr_in10_label.dev_attr.attr,
> + &sensor_dev_attr_in11_label.dev_attr.attr,
> + &sensor_dev_attr_in12_label.dev_attr.attr,
> + &sensor_dev_attr_in13_label.dev_attr.attr,
> +
> + NULL
> +};
> +
> +ATTRIBUTE_GROUPS(powr1220);
> +
> +
Single empty line
> +static int powr1220_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct powr1220_data *data;
> + struct device *hwmon_dev;
> +
> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> + return -EIO;
-ENODEV is commonly used here.
> +
> + data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + mutex_init(&data->update_lock);
> + data->client = client;
> +
> + hwmon_dev = devm_hwmon_device_register_with_groups(&client->dev,
> + client->name, data, powr1220_groups);
> +
> + return PTR_ERR_OR_ZERO(hwmon_dev);
> +}
> +
> +
Single empty line.
> +static const struct i2c_device_id powr1220_ids[] = {
> + { "powr1220", 0, },
> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, powr1220_ids);
> +
> +static struct i2c_driver powr1220_driver = {
> + .class = I2C_CLASS_HWMON,
> + .driver = {
> + .name = "powr1220",
> + },
> + .probe = powr1220_probe,
> + .id_table = powr1220_ids,
> +};
> +
> +
> +static int __init powr1220_init(void) {
> + return i2c_add_driver(&powr1220_driver); }
> +
> +static void __exit powr1220_exit(void) {
> + i2c_del_driver(&powr1220_driver);
> +}
> +
> +
> +module_init(powr1220_init);
> +module_exit(powr1220_exit);
> +
Please use module_i2c_driver().
> +MODULE_AUTHOR("Scott Kanowitz");
> +MODULE_DESCRIPTION("POWR1220 driver"); MODULE_LICENSE("GPL");
>
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 7+ messages in thread
* [lm-sensors] [PATCH] Added support for Lattice's POWR1220 power manager IC.
2014-06-10 0:33 [lm-sensors] [PATCH] Added support for Lattice's POWR1220 power manager IC Guenter Roeck
` (3 preceding siblings ...)
2014-06-12 3:16 ` Scott Kanowitz
@ 2014-06-12 20:22 ` Scott Kanowitz
2014-06-13 18:10 ` Guenter Roeck
5 siblings, 0 replies; 7+ messages in thread
From: Scott Kanowitz @ 2014-06-12 20:22 UTC (permalink / raw)
To: lm-sensors
This patch adds support for Lattice's POWR1220 power manager IC. Read
access to all the ADCs on the chip are supported through the hwmon
sysfs files.
Signed-off-by: Scott Kanowitz <skanowitz@echo360.com>
---
Documentation/hwmon/powr1220 | 45 +++++
drivers/hwmon/Kconfig | 12 ++
drivers/hwmon/Makefile | 1 +
drivers/hwmon/powr1220.c | 391 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 449 insertions(+)
create mode 100644 Documentation/hwmon/powr1220
create mode 100644 drivers/hwmon/powr1220.c
diff --git a/Documentation/hwmon/powr1220 b/Documentation/hwmon/powr1220
new file mode 100644
index 0000000..21e44f7
--- /dev/null
+++ b/Documentation/hwmon/powr1220
@@ -0,0 +1,45 @@
+Kernel driver powr1220
+=========
+
+Supported chips:
+ * Lattice POWR1220AT8
+ Prefix: 'powr1220'
+ Addresses scanned: none
+ Datasheet: Publicly available at the Lattice website
+ http://www.latticesemi.com/
+
+Author: Scott Kanowitz <scott.kanowitz@gmail.com>
+
+Description
+-----------
+
+This driver supports the Lattice POWR1220AT8 chip. The POWR1220
+includes voltage monitoring for 14 inputs as well as trim settings
+for output voltages and GPIOs. This driver implements the voltage
+monitoring portion of the chip.
+
+Voltages are sampled by a 12-bit ADC with a step size of 2 mV.
+An in-line attenuator allows measurements from 0 to 6 V. The
+attenuator is enabled or disabled depending on the setting of the
+input's max value. The driver will enable the attenuator for any
+value over the low measurement range maximum of 2 V.
+
+The input naming convention is as follows:
+
+driver name pin name
+in0 VMON1
+in1 VMON2
+in2 VMON3
+in2 VMON4
+in4 VMON5
+in5 VMON6
+in6 VMON7
+in7 VMON8
+in8 VMON9
+in9 VMON10
+in10 VMON11
+in11 VMON12
+in12 VCCA
+in13 VCCINP
+
+The ADC readings are updated on request with a minimum period of 1s.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 4af0da9..fc4d7f1 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -608,6 +608,18 @@ config SENSORS_JC42
This driver can also be built as a module. If so, the module
will be called jc42.
+config SENSORS_POWR1220
+ tristate "Lattice POWR1220 Power Monitoring"
+ depends on I2C
+ default n
+ help
+ If you say yes here you get access to the hardware monitoring
+ functions of the Lattice POWR1220 isp Power Supply Monitoring,
+ Sequencing and Margining Controller.
+
+ This driver can also be built as a module. If so, the module
+ will be called powr1220.
+
config SENSORS_LINEAGE
tristate "Lineage Compact Power Line Power Entry Module"
depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index c48f987..842e6b3 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -119,6 +119,7 @@ obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o
obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
+obj-$(CONFIG_SENSORS_POWR1220) += powr1220.o
obj-$(CONFIG_SENSORS_S3C) += s3c-hwmon.o
obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o
obj-$(CONFIG_SENSORS_SCH5627) += sch5627.o
diff --git a/drivers/hwmon/powr1220.c b/drivers/hwmon/powr1220.c
new file mode 100644
index 0000000..bea36d1
--- /dev/null
+++ b/drivers/hwmon/powr1220.c
@@ -0,0 +1,391 @@
+/*
+ * powr1220.c - Driver for the Lattice POWR1220 programmable power supply
+ * and monitor. Users can read all ADC inputs along with their labels
+ * using the sysfs nodes.
+ *
+ * Copyright (c) 2014 Echo360 http://www.echo360.com
+ * Scott Kanowitz <skanowitz@echo360.com> <scott.kanowitz@gmail.com>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/jiffies.h>
+#include <linux/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/delay.h>
+
+#define ADC_STEP_MV 2
+#define ADC_MAX_LOW_MEASUREMENT_MV 2000
+
+enum powr1220_regs {
+ VMON_STATUS0,
+ VMON_STATUS1,
+ VMON_STATUS2,
+ OUTPUT_STATUS0,
+ OUTPUT_STATUS1,
+ OUTPUT_STATUS2,
+ INPUT_STATUS,
+ ADC_VALUE_LOW,
+ ADC_VALUE_HIGH,
+ ADC_MUX,
+ UES_BYTE0,
+ UES_BYTE1,
+ UES_BYTE2,
+ UES_BYTE3,
+ GP_OUTPUT1,
+ GP_OUTPUT2,
+ GP_OUTPUT3,
+ INPUT_VALUE,
+ RESET,
+ TRIM1_TRIM,
+ TRIM2_TRIM,
+ TRIM3_TRIM,
+ TRIM4_TRIM,
+ TRIM5_TRIM,
+ TRIM6_TRIM,
+ TRIM7_TRIM,
+ TRIM8_TRIM,
+ MAX_POWR1220_REGS
+};
+
+enum powr1220_adc_values {
+ VMON1,
+ VMON2,
+ VMON3,
+ VMON4,
+ VMON5,
+ VMON6,
+ VMON7,
+ VMON8,
+ VMON9,
+ VMON10,
+ VMON11,
+ VMON12,
+ VCCA,
+ VCCINP,
+ MAX_POWR1220_ADC_VALUES
+};
+
+struct powr1220_data {
+ struct i2c_client *client;
+ struct mutex update_lock;
+ bool adc_valid[MAX_POWR1220_ADC_VALUES];
+ /* the next value is in jiffies */
+ unsigned long adc_last_updated[MAX_POWR1220_ADC_VALUES];
+
+ /* values */
+ int adc_maxes[MAX_POWR1220_ADC_VALUES];
+ int adc_values[MAX_POWR1220_ADC_VALUES];
+};
+
+static const char * const input_names[] = {
+ [VMON1] = "vmon1",
+ [VMON2] = "vmon2",
+ [VMON3] = "vmon3",
+ [VMON4] = "vmon4",
+ [VMON5] = "vmon5",
+ [VMON6] = "vmon6",
+ [VMON7] = "vmon7",
+ [VMON8] = "vmon8",
+ [VMON9] = "vmon9",
+ [VMON10] = "vmon10",
+ [VMON11] = "vmon11",
+ [VMON12] = "vmon12",
+ [VCCA] = "vcca",
+ [VCCINP] = "vccinp",
+};
+
+/* Reads the specified ADC channel */
+static int powr1220_read_adc(struct device *dev, int ch_num)
+{
+ struct powr1220_data *data = dev_get_drvdata(dev);
+ int reading;
+ int result;
+ int adc_range = 0;
+
+ mutex_lock(&data->update_lock);
+
+ if (time_after(jiffies, data->adc_last_updated[ch_num] + HZ) ||
+ !data->adc_valid[ch_num]) {
+ /*
+ * figure out if we need to use the attenuator for
+ * high inputs or inputs that we don't yet have a measurement
+ * for. We dynamically set the attenuator depending on the
+ * max reading.
+ */
+ if ((data->adc_maxes[ch_num] > ADC_MAX_LOW_MEASUREMENT_MV) ||
+ (data->adc_maxes[ch_num] = 0))
+ adc_range = (1 << 4);
+
+ /* set the attenuator and mux */
+ result = i2c_smbus_write_byte_data(data->client, ADC_MUX,
+ (adc_range | ch_num));
+ if (result)
+ goto exit;
+
+ /*
+ * wait at least Tconvert time (200 us) for the
+ * conversion to complete
+ */
+ udelay(200);
+
+ /* get the ADC reading */
+ result = i2c_smbus_read_byte_data(data->client, ADC_VALUE_LOW);
+ if (result < 0)
+ goto exit;
+
+ reading = result >> 4;
+
+ /* get the upper half of the reading */
+ result = i2c_smbus_read_byte_data(data->client, ADC_VALUE_HIGH);
+ if (result < 0)
+ goto exit;
+
+ reading |= result << 4;
+
+ /* now convert the reading to a voltage */
+ reading *= ADC_STEP_MV;
+ data->adc_values[ch_num] = reading;
+ data->adc_valid[ch_num] = true;
+ data->adc_last_updated[ch_num] = jiffies;
+ result = reading;
+
+ if (reading > data->adc_maxes[ch_num])
+ data->adc_maxes[ch_num] = reading;
+ } else {
+ result = data->adc_values[ch_num];
+ }
+
+exit:
+ mutex_unlock(&data->update_lock);
+
+ return result;
+}
+
+/* Shows the voltage associated with the specified ADC channel */
+static ssize_t powr1220_show_voltage(struct device *dev,
+ struct device_attribute *dev_attr, char *buf)
+{
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
+ int adc_val = powr1220_read_adc(dev, attr->index);
+
+ if (adc_val < 0)
+ return adc_val;
+
+ return sprintf(buf, "%d\n", adc_val);
+}
+
+/* Shows the maximum setting associated with the specified ADC channel */
+static ssize_t powr1220_show_max(struct device *dev,
+ struct device_attribute *dev_attr, char *buf)
+{
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
+ struct powr1220_data *data = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%d\n", data->adc_maxes[attr->index]);
+}
+
+/* Shows the label associated with the specified ADC channel */
+static ssize_t powr1220_show_label(struct device *dev,
+ struct device_attribute *dev_attr, char *buf)
+{
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
+
+ return sprintf(buf, "%s\n", input_names[attr->index]);
+}
+
+static SENSOR_DEVICE_ATTR(in0_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON1);
+static SENSOR_DEVICE_ATTR(in1_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON2);
+static SENSOR_DEVICE_ATTR(in2_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON3);
+static SENSOR_DEVICE_ATTR(in3_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON4);
+static SENSOR_DEVICE_ATTR(in4_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON5);
+static SENSOR_DEVICE_ATTR(in5_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON6);
+static SENSOR_DEVICE_ATTR(in6_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON7);
+static SENSOR_DEVICE_ATTR(in7_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON8);
+static SENSOR_DEVICE_ATTR(in8_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON9);
+static SENSOR_DEVICE_ATTR(in9_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON10);
+static SENSOR_DEVICE_ATTR(in10_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON11);
+static SENSOR_DEVICE_ATTR(in11_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VMON12);
+static SENSOR_DEVICE_ATTR(in12_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VCCA);
+static SENSOR_DEVICE_ATTR(in13_input, S_IRUGO, powr1220_show_voltage, NULL,
+ VCCINP);
+
+static SENSOR_DEVICE_ATTR(in0_highest, S_IRUGO, powr1220_show_max, NULL,
+ VMON1);
+static SENSOR_DEVICE_ATTR(in1_highest, S_IRUGO, powr1220_show_max, NULL,
+ VMON2);
+static SENSOR_DEVICE_ATTR(in2_highest, S_IRUGO, powr1220_show_max, NULL,
+ VMON3);
+static SENSOR_DEVICE_ATTR(in3_highest, S_IRUGO, powr1220_show_max, NULL,
+ VMON4);
+static SENSOR_DEVICE_ATTR(in4_highest, S_IRUGO, powr1220_show_max, NULL,
+ VMON5);
+static SENSOR_DEVICE_ATTR(in5_highest, S_IRUGO, powr1220_show_max, NULL,
+ VMON6);
+static SENSOR_DEVICE_ATTR(in6_highest, S_IRUGO, powr1220_show_max, NULL,
+ VMON7);
+static SENSOR_DEVICE_ATTR(in7_highest, S_IRUGO, powr1220_show_max, NULL,
+ VMON8);
+static SENSOR_DEVICE_ATTR(in8_highest, S_IRUGO, powr1220_show_max, NULL,
+ VMON9);
+static SENSOR_DEVICE_ATTR(in9_highest, S_IRUGO, powr1220_show_max, NULL,
+ VMON10);
+static SENSOR_DEVICE_ATTR(in10_highest, S_IRUGO, powr1220_show_max, NULL,
+ VMON11);
+static SENSOR_DEVICE_ATTR(in11_highest, S_IRUGO, powr1220_show_max, NULL,
+ VMON12);
+static SENSOR_DEVICE_ATTR(in12_highest, S_IRUGO, powr1220_show_max, NULL,
+ VCCA);
+static SENSOR_DEVICE_ATTR(in13_highest, S_IRUGO, powr1220_show_max, NULL,
+ VCCINP);
+
+static SENSOR_DEVICE_ATTR(in0_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON1);
+static SENSOR_DEVICE_ATTR(in1_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON2);
+static SENSOR_DEVICE_ATTR(in2_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON3);
+static SENSOR_DEVICE_ATTR(in3_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON4);
+static SENSOR_DEVICE_ATTR(in4_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON5);
+static SENSOR_DEVICE_ATTR(in5_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON6);
+static SENSOR_DEVICE_ATTR(in6_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON7);
+static SENSOR_DEVICE_ATTR(in7_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON8);
+static SENSOR_DEVICE_ATTR(in8_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON9);
+static SENSOR_DEVICE_ATTR(in9_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON10);
+static SENSOR_DEVICE_ATTR(in10_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON11);
+static SENSOR_DEVICE_ATTR(in11_label, S_IRUGO, powr1220_show_label, NULL,
+ VMON12);
+static SENSOR_DEVICE_ATTR(in12_label, S_IRUGO, powr1220_show_label, NULL,
+ VCCA);
+static SENSOR_DEVICE_ATTR(in13_label, S_IRUGO, powr1220_show_label, NULL,
+ VCCINP);
+
+static struct attribute *powr1220_attrs[] = {
+ &sensor_dev_attr_in0_input.dev_attr.attr,
+ &sensor_dev_attr_in1_input.dev_attr.attr,
+ &sensor_dev_attr_in2_input.dev_attr.attr,
+ &sensor_dev_attr_in3_input.dev_attr.attr,
+ &sensor_dev_attr_in4_input.dev_attr.attr,
+ &sensor_dev_attr_in5_input.dev_attr.attr,
+ &sensor_dev_attr_in6_input.dev_attr.attr,
+ &sensor_dev_attr_in7_input.dev_attr.attr,
+ &sensor_dev_attr_in8_input.dev_attr.attr,
+ &sensor_dev_attr_in9_input.dev_attr.attr,
+ &sensor_dev_attr_in10_input.dev_attr.attr,
+ &sensor_dev_attr_in11_input.dev_attr.attr,
+ &sensor_dev_attr_in12_input.dev_attr.attr,
+ &sensor_dev_attr_in13_input.dev_attr.attr,
+
+ &sensor_dev_attr_in0_highest.dev_attr.attr,
+ &sensor_dev_attr_in1_highest.dev_attr.attr,
+ &sensor_dev_attr_in2_highest.dev_attr.attr,
+ &sensor_dev_attr_in3_highest.dev_attr.attr,
+ &sensor_dev_attr_in4_highest.dev_attr.attr,
+ &sensor_dev_attr_in5_highest.dev_attr.attr,
+ &sensor_dev_attr_in6_highest.dev_attr.attr,
+ &sensor_dev_attr_in7_highest.dev_attr.attr,
+ &sensor_dev_attr_in8_highest.dev_attr.attr,
+ &sensor_dev_attr_in9_highest.dev_attr.attr,
+ &sensor_dev_attr_in10_highest.dev_attr.attr,
+ &sensor_dev_attr_in11_highest.dev_attr.attr,
+ &sensor_dev_attr_in12_highest.dev_attr.attr,
+ &sensor_dev_attr_in13_highest.dev_attr.attr,
+
+ &sensor_dev_attr_in0_label.dev_attr.attr,
+ &sensor_dev_attr_in1_label.dev_attr.attr,
+ &sensor_dev_attr_in2_label.dev_attr.attr,
+ &sensor_dev_attr_in3_label.dev_attr.attr,
+ &sensor_dev_attr_in4_label.dev_attr.attr,
+ &sensor_dev_attr_in5_label.dev_attr.attr,
+ &sensor_dev_attr_in6_label.dev_attr.attr,
+ &sensor_dev_attr_in7_label.dev_attr.attr,
+ &sensor_dev_attr_in8_label.dev_attr.attr,
+ &sensor_dev_attr_in9_label.dev_attr.attr,
+ &sensor_dev_attr_in10_label.dev_attr.attr,
+ &sensor_dev_attr_in11_label.dev_attr.attr,
+ &sensor_dev_attr_in12_label.dev_attr.attr,
+ &sensor_dev_attr_in13_label.dev_attr.attr,
+
+ NULL
+};
+
+ATTRIBUTE_GROUPS(powr1220);
+
+static int powr1220_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct powr1220_data *data;
+ struct device *hwmon_dev;
+
+ if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
+ return -ENODEV;
+
+ data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ mutex_init(&data->update_lock);
+ data->client = client;
+
+ hwmon_dev = devm_hwmon_device_register_with_groups(&client->dev,
+ client->name, data, powr1220_groups);
+
+ return PTR_ERR_OR_ZERO(hwmon_dev);
+}
+
+static const struct i2c_device_id powr1220_ids[] = {
+ { "powr1220", 0, },
+ { }
+};
+
+MODULE_DEVICE_TABLE(i2c, powr1220_ids);
+
+static struct i2c_driver powr1220_driver = {
+ .class = I2C_CLASS_HWMON,
+ .driver = {
+ .name = "powr1220",
+ },
+ .probe = powr1220_probe,
+ .id_table = powr1220_ids,
+};
+
+module_i2c_driver(powr1220_driver);
+
+MODULE_AUTHOR("Scott Kanowitz");
+MODULE_DESCRIPTION("POWR1220 driver");
+MODULE_LICENSE("GPL");
--
1.8.3.2
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [lm-sensors] [PATCH] Added support for Lattice's POWR1220 power manager IC.
2014-06-10 0:33 [lm-sensors] [PATCH] Added support for Lattice's POWR1220 power manager IC Guenter Roeck
` (4 preceding siblings ...)
2014-06-12 20:22 ` Scott Kanowitz
@ 2014-06-13 18:10 ` Guenter Roeck
5 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2014-06-13 18:10 UTC (permalink / raw)
To: lm-sensors
On 06/12/2014 01:22 PM, Scott Kanowitz wrote:
> This patch adds support for Lattice's POWR1220 power manager IC. Read
> access to all the ADCs on the chip are supported through the hwmon
> sysfs files.
>
> Signed-off-by: Scott Kanowitz <skanowitz@echo360.com>
Hi Scott,
Nicely done, though next time it would be great if you can add a change log.
There are a couple of unneeded ( ), which I removed. No need to resend.
Applied to -next.
Thanks,
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-06-13 18:10 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-10 0:33 [lm-sensors] [PATCH] Added support for Lattice's POWR1220 power manager IC Guenter Roeck
2014-06-10 14:25 ` Scott Kanowitz
2014-06-11 20:35 ` Scott Kanowitz
2014-06-11 22:26 ` Guenter Roeck
2014-06-12 3:16 ` Scott Kanowitz
2014-06-12 20:22 ` Scott Kanowitz
2014-06-13 18:10 ` Guenter Roeck
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.