* [PATCH v4 0/2] hwmon: (max127) Add Maxim MAX127 hardware monitoring @ 2020-11-23 7:45 rentao.bupt 2020-11-23 7:45 ` [PATCH v4 1/2] hwmon: (max127) Add Maxim MAX127 hardware monitoring driver rentao.bupt 2020-11-23 7:45 ` [PATCH v4 2/2] docs: hwmon: Document max127 driver rentao.bupt 0 siblings, 2 replies; 7+ messages in thread From: rentao.bupt @ 2020-11-23 7:45 UTC (permalink / raw) To: Jean Delvare, Guenter Roeck, Jonathan Corbet, linux-hwmon, linux-doc, linux-kernel, openbmc, taoren, mikechoi Cc: Tao Ren From: Tao Ren <rentao.bupt@gmail.com> The patch series adds hardware monitoring driver for the Maxim MAX127 chip. Patch #1 adds the max127 hardware monitoring driver, and patch #2 adds documentation for the driver. Tao Ren (2): hwmon: (max127) Add Maxim MAX127 hardware monitoring driver docs: hwmon: Document max127 driver Documentation/hwmon/index.rst | 1 + Documentation/hwmon/max127.rst | 45 +++++ drivers/hwmon/Kconfig | 9 + drivers/hwmon/Makefile | 1 + drivers/hwmon/max127.c | 346 +++++++++++++++++++++++++++++++++ 5 files changed, 402 insertions(+) create mode 100644 Documentation/hwmon/max127.rst create mode 100644 drivers/hwmon/max127.c -- 2.17.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 1/2] hwmon: (max127) Add Maxim MAX127 hardware monitoring driver 2020-11-23 7:45 [PATCH v4 0/2] hwmon: (max127) Add Maxim MAX127 hardware monitoring rentao.bupt @ 2020-11-23 7:45 ` rentao.bupt 2020-11-23 13:18 ` Guenter Roeck 2020-11-23 7:45 ` [PATCH v4 2/2] docs: hwmon: Document max127 driver rentao.bupt 1 sibling, 1 reply; 7+ messages in thread From: rentao.bupt @ 2020-11-23 7:45 UTC (permalink / raw) To: Jean Delvare, Guenter Roeck, Jonathan Corbet, linux-hwmon, linux-doc, linux-kernel, openbmc, taoren, mikechoi Cc: Tao Ren From: Tao Ren <rentao.bupt@gmail.com> Add hardware monitoring driver for the Maxim MAX127 chip. MAX127 min/max range handling code is inspired by the max197 driver. Signed-off-by: Tao Ren <rentao.bupt@gmail.com> --- Changes in v4: - delete unnecessary "#include" lines. - simplify i2c_transfer() error handling. - add mutex to protect ctrl_byte in write_min|max() functions. Changes in v3: - no code change. xdp maintainers were removed from to/cc list. Changes in v2: - replace devm_hwmon_device_register_with_groups() with devm_hwmon_device_register_with_info() API. - divide min/max read and write methods to separate functions. - fix raw-to-vin conversion logic. - refine ctrl_byte handling so mutex is not needed to protect the byte. - improve i2c_transfer() error handling. - a few other improvements (comments, variable naming, and etc.). drivers/hwmon/Kconfig | 9 ++ drivers/hwmon/Makefile | 1 + drivers/hwmon/max127.c | 346 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 356 insertions(+) create mode 100644 drivers/hwmon/max127.c diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 9d600e0c5584..716df51edc87 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -950,6 +950,15 @@ config SENSORS_MAX1111 This driver can also be built as a module. If so, the module will be called max1111. +config SENSORS_MAX127 + tristate "Maxim MAX127 12-bit 8-channel Data Acquisition System" + depends on I2C + help + Say y here to support Maxim's MAX127 DAS chips. + + This driver can also be built as a module. If so, the module + will be called max127. + config SENSORS_MAX16065 tristate "Maxim MAX16065 System Manager and compatibles" depends on I2C diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 1083bbfac779..01ca5d3fbad4 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -127,6 +127,7 @@ obj-$(CONFIG_SENSORS_LTC4260) += ltc4260.o obj-$(CONFIG_SENSORS_LTC4261) += ltc4261.o obj-$(CONFIG_SENSORS_LTQ_CPUTEMP) += ltq-cputemp.o obj-$(CONFIG_SENSORS_MAX1111) += max1111.o +obj-$(CONFIG_SENSORS_MAX127) += max127.o obj-$(CONFIG_SENSORS_MAX16065) += max16065.o obj-$(CONFIG_SENSORS_MAX1619) += max1619.o obj-$(CONFIG_SENSORS_MAX1668) += max1668.o diff --git a/drivers/hwmon/max127.c b/drivers/hwmon/max127.c new file mode 100644 index 000000000000..1c54146b6086 --- /dev/null +++ b/drivers/hwmon/max127.c @@ -0,0 +1,346 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Hardware monitoring driver for MAX127. + * + * Copyright (c) 2020 Facebook Inc. + */ + +#include <linux/err.h> +#include <linux/hwmon.h> +#include <linux/i2c.h> +#include <linux/init.h> +#include <linux/module.h> + +/* + * MAX127 Control Byte. Refer to MAX127 datasheet, Table 1 "Control-Byte + * Format" for details. + */ +#define MAX127_CTRL_START BIT(7) +#define MAX127_CTRL_SEL_SHIFT 4 +#define MAX127_CTRL_RNG BIT(3) +#define MAX127_CTRL_BIP BIT(2) +#define MAX127_CTRL_PD1 BIT(1) +#define MAX127_CTRL_PD0 BIT(0) + +#define MAX127_NUM_CHANNELS 8 +#define MAX127_SET_CHANNEL(ch) (((ch) & 7) << MAX127_CTRL_SEL_SHIFT) + +/* + * MAX127 channel input ranges. Refer to MAX127 datasheet, Table 3 "Range + * and Polarity Selection" for details. + */ +#define MAX127_FULL_RANGE 10000 /* 10V */ +#define MAX127_HALF_RANGE 5000 /* 5V */ + +/* + * MAX127 returns 2 bytes at read: + * - the first byte contains data[11:4]. + * - the second byte contains data[3:0] (MSB) and 4 dummy 0s (LSB). + * Refer to MAX127 datasheet, "Read a Conversion (Read Cycle)" section + * for details. + */ +#define MAX127_DATA_LEN 2 +#define MAX127_DATA_SHIFT 4 + +#define MAX127_SIGN_BIT BIT(11) + +struct max127_data { + struct mutex lock; + struct i2c_client *client; + u8 ctrl_byte[MAX127_NUM_CHANNELS]; +}; + +static int max127_select_channel(struct i2c_client *client, u8 ctrl_byte) +{ + int status; + struct i2c_msg msg = { + .addr = client->addr, + .flags = 0, + .len = sizeof(ctrl_byte), + .buf = &ctrl_byte, + }; + + status = i2c_transfer(client->adapter, &msg, 1); + + return (status == 1) ? 0 : -EIO; +} + +static int max127_read_channel(struct i2c_client *client, long *val) +{ + int status; + u8 i2c_data[MAX127_DATA_LEN]; + struct i2c_msg msg = { + .addr = client->addr, + .flags = I2C_M_RD, + .len = sizeof(i2c_data), + .buf = i2c_data, + }; + + status = i2c_transfer(client->adapter, &msg, 1); + if (status != 1) + return -EIO; + + *val = (i2c_data[1] >> MAX127_DATA_SHIFT) | + ((u16)i2c_data[0] << MAX127_DATA_SHIFT); + return 0; +} + +static long max127_process_raw(u8 ctrl_byte, long raw) +{ + long scale, weight; + + /* + * MAX127's data coding is binary in unipolar mode with 1 LSB = + * (Full-Scale/4096) and two’s complement binary in bipolar mode + * with 1 LSB = [(2 x |FS|)/4096]. + * Refer to MAX127 datasheet, "Transfer Function" section for + * details. + */ + scale = (ctrl_byte & MAX127_CTRL_RNG) ? MAX127_FULL_RANGE : + MAX127_HALF_RANGE; + if (ctrl_byte & MAX127_CTRL_BIP) { + weight = (raw & MAX127_SIGN_BIT); + raw &= ~MAX127_SIGN_BIT; + raw -= weight; + raw *= 2; + } + + return raw * scale / 4096; +} + +static int max127_read_input(struct max127_data *data, int channel, long *val) +{ + long raw; + int status; + struct i2c_client *client = data->client; + u8 ctrl_byte = data->ctrl_byte[channel]; + + mutex_lock(&data->lock); + + status = max127_select_channel(client, ctrl_byte); + if (status) + goto exit; + + status = max127_read_channel(client, &raw); + if (status) + goto exit; + + *val = max127_process_raw(ctrl_byte, raw); + +exit: + mutex_unlock(&data->lock); + return status; +} + +static int max127_read_min(struct max127_data *data, int channel, long *val) +{ + u8 rng_bip = (data->ctrl_byte[channel] >> 2) & 3; + static const int min_input_map[4] = { + 0, /* RNG=0, BIP=0 */ + -MAX127_HALF_RANGE, /* RNG=0, BIP=1 */ + 0, /* RNG=1, BIP=0 */ + -MAX127_FULL_RANGE, /* RNG=1, BIP=1 */ + }; + + *val = min_input_map[rng_bip]; + return 0; +} + +static int max127_read_max(struct max127_data *data, int channel, long *val) +{ + u8 rng_bip = (data->ctrl_byte[channel] >> 2) & 3; + static const int max_input_map[4] = { + MAX127_HALF_RANGE, /* RNG=0, BIP=0 */ + MAX127_HALF_RANGE, /* RNG=0, BIP=1 */ + MAX127_FULL_RANGE, /* RNG=1, BIP=0 */ + MAX127_FULL_RANGE, /* RNG=1, BIP=1 */ + }; + + *val = max_input_map[rng_bip]; + return 0; +} + +static int max127_write_min(struct max127_data *data, int channel, long val) +{ + u8 ctrl; + + mutex_lock(&data->lock); + + ctrl = data->ctrl_byte[channel]; + if (val <= -MAX127_FULL_RANGE) { + ctrl |= (MAX127_CTRL_RNG | MAX127_CTRL_BIP); + } else if (val < 0) { + ctrl |= MAX127_CTRL_BIP; + ctrl &= ~MAX127_CTRL_RNG; + } else { + ctrl &= ~MAX127_CTRL_BIP; + } + data->ctrl_byte[channel] = ctrl; + + mutex_unlock(&data->lock); + + return 0; +} + +static int max127_write_max(struct max127_data *data, int channel, long val) +{ + mutex_lock(&data->lock); + + if (val >= MAX127_FULL_RANGE) + data->ctrl_byte[channel] |= MAX127_CTRL_RNG; + else + data->ctrl_byte[channel] &= ~MAX127_CTRL_RNG; + + mutex_unlock(&data->lock); + + return 0; +} + +static umode_t max127_is_visible(const void *_data, + enum hwmon_sensor_types type, + u32 attr, int channel) +{ + if (type == hwmon_in) { + switch (attr) { + case hwmon_in_input: + return 0444; + + case hwmon_in_min: + case hwmon_in_max: + return 0644; + + default: + break; + } + } + + return 0; +} + +static int max127_read(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long *val) +{ + int status; + struct max127_data *data = dev_get_drvdata(dev); + + if (type != hwmon_in) + return -EOPNOTSUPP; + + switch (attr) { + case hwmon_in_input: + status = max127_read_input(data, channel, val); + break; + + case hwmon_in_min: + status = max127_read_min(data, channel, val); + break; + + case hwmon_in_max: + status = max127_read_max(data, channel, val); + break; + + default: + status = -EOPNOTSUPP; + break; + } + + return status; +} + +static int max127_write(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long val) +{ + int status; + struct max127_data *data = dev_get_drvdata(dev); + + if (type != hwmon_in) + return -EOPNOTSUPP; + + switch (attr) { + case hwmon_in_min: + status = max127_write_min(data, channel, val); + break; + + case hwmon_in_max: + status = max127_write_max(data, channel, val); + break; + + default: + status = -EOPNOTSUPP; + break; + } + + return status; +} + +static const struct hwmon_ops max127_hwmon_ops = { + .is_visible = max127_is_visible, + .read = max127_read, + .write = max127_write, +}; + +static const struct hwmon_channel_info *max127_info[] = { + HWMON_CHANNEL_INFO(in, + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX, + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX, + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX, + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX, + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX, + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX, + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX, + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX), + NULL, +}; + +static const struct hwmon_chip_info max127_chip_info = { + .ops = &max127_hwmon_ops, + .info = max127_info, +}; + +static int max127_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + int i; + struct device *hwmon_dev; + struct max127_data *data; + struct device *dev = &client->dev; + + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + data->client = client; + mutex_init(&data->lock); + for (i = 0; i < ARRAY_SIZE(data->ctrl_byte); i++) + data->ctrl_byte[i] = (MAX127_CTRL_START | + MAX127_SET_CHANNEL(i)); + + hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, + data, + &max127_chip_info, + NULL); + + return PTR_ERR_OR_ZERO(hwmon_dev); +} + +static const struct i2c_device_id max127_id[] = { + { "max127", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, max127_id); + +static struct i2c_driver max127_driver = { + .class = I2C_CLASS_HWMON, + .driver = { + .name = "max127", + }, + .probe = max127_probe, + .id_table = max127_id, +}; + +module_i2c_driver(max127_driver); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Mike Choi <mikechoi@fb.com>"); +MODULE_AUTHOR("Tao Ren <rentao.bupt@gmail.com>"); +MODULE_DESCRIPTION("MAX127 Hardware Monitoring driver"); -- 2.17.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/2] hwmon: (max127) Add Maxim MAX127 hardware monitoring driver 2020-11-23 7:45 ` [PATCH v4 1/2] hwmon: (max127) Add Maxim MAX127 hardware monitoring driver rentao.bupt @ 2020-11-23 13:18 ` Guenter Roeck 0 siblings, 0 replies; 7+ messages in thread From: Guenter Roeck @ 2020-11-23 13:18 UTC (permalink / raw) To: rentao.bupt Cc: Jean Delvare, Jonathan Corbet, linux-hwmon, linux-doc, linux-kernel, openbmc, taoren, mikechoi On Sun, Nov 22, 2020 at 11:45:31PM -0800, rentao.bupt@gmail.com wrote: > From: Tao Ren <rentao.bupt@gmail.com> > > Add hardware monitoring driver for the Maxim MAX127 chip. > > MAX127 min/max range handling code is inspired by the max197 driver. > > Signed-off-by: Tao Ren <rentao.bupt@gmail.com> > --- > Changes in v4: > - delete unnecessary "#include" lines. > - simplify i2c_transfer() error handling. > - add mutex to protect ctrl_byte in write_min|max() functions. > Changes in v3: > - no code change. xdp maintainers were removed from to/cc list. > Changes in v2: > - replace devm_hwmon_device_register_with_groups() with > devm_hwmon_device_register_with_info() API. > - divide min/max read and write methods to separate functions. > - fix raw-to-vin conversion logic. > - refine ctrl_byte handling so mutex is not needed to protect the > byte. > - improve i2c_transfer() error handling. > - a few other improvements (comments, variable naming, and etc.). > > drivers/hwmon/Kconfig | 9 ++ > drivers/hwmon/Makefile | 1 + > drivers/hwmon/max127.c | 346 +++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 356 insertions(+) > create mode 100644 drivers/hwmon/max127.c > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index 9d600e0c5584..716df51edc87 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -950,6 +950,15 @@ config SENSORS_MAX1111 > This driver can also be built as a module. If so, the module > will be called max1111. > > +config SENSORS_MAX127 > + tristate "Maxim MAX127 12-bit 8-channel Data Acquisition System" > + depends on I2C > + help > + Say y here to support Maxim's MAX127 DAS chips. > + > + This driver can also be built as a module. If so, the module > + will be called max127. > + > config SENSORS_MAX16065 > tristate "Maxim MAX16065 System Manager and compatibles" > depends on I2C > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index 1083bbfac779..01ca5d3fbad4 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -127,6 +127,7 @@ obj-$(CONFIG_SENSORS_LTC4260) += ltc4260.o > obj-$(CONFIG_SENSORS_LTC4261) += ltc4261.o > obj-$(CONFIG_SENSORS_LTQ_CPUTEMP) += ltq-cputemp.o > obj-$(CONFIG_SENSORS_MAX1111) += max1111.o > +obj-$(CONFIG_SENSORS_MAX127) += max127.o > obj-$(CONFIG_SENSORS_MAX16065) += max16065.o > obj-$(CONFIG_SENSORS_MAX1619) += max1619.o > obj-$(CONFIG_SENSORS_MAX1668) += max1668.o > diff --git a/drivers/hwmon/max127.c b/drivers/hwmon/max127.c > new file mode 100644 > index 000000000000..1c54146b6086 > --- /dev/null > +++ b/drivers/hwmon/max127.c > @@ -0,0 +1,346 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Hardware monitoring driver for MAX127. > + * > + * Copyright (c) 2020 Facebook Inc. > + */ > + > +#include <linux/err.h> > +#include <linux/hwmon.h> > +#include <linux/i2c.h> > +#include <linux/init.h> > +#include <linux/module.h> > + > +/* > + * MAX127 Control Byte. Refer to MAX127 datasheet, Table 1 "Control-Byte > + * Format" for details. > + */ > +#define MAX127_CTRL_START BIT(7) > +#define MAX127_CTRL_SEL_SHIFT 4 > +#define MAX127_CTRL_RNG BIT(3) > +#define MAX127_CTRL_BIP BIT(2) > +#define MAX127_CTRL_PD1 BIT(1) > +#define MAX127_CTRL_PD0 BIT(0) > + > +#define MAX127_NUM_CHANNELS 8 > +#define MAX127_SET_CHANNEL(ch) (((ch) & 7) << MAX127_CTRL_SEL_SHIFT) > + > +/* > + * MAX127 channel input ranges. Refer to MAX127 datasheet, Table 3 "Range > + * and Polarity Selection" for details. > + */ > +#define MAX127_FULL_RANGE 10000 /* 10V */ > +#define MAX127_HALF_RANGE 5000 /* 5V */ > + > +/* > + * MAX127 returns 2 bytes at read: > + * - the first byte contains data[11:4]. > + * - the second byte contains data[3:0] (MSB) and 4 dummy 0s (LSB). > + * Refer to MAX127 datasheet, "Read a Conversion (Read Cycle)" section > + * for details. > + */ > +#define MAX127_DATA_LEN 2 > +#define MAX127_DATA_SHIFT 4 > + > +#define MAX127_SIGN_BIT BIT(11) > + > +struct max127_data { > + struct mutex lock; > + struct i2c_client *client; > + u8 ctrl_byte[MAX127_NUM_CHANNELS]; > +}; > + > +static int max127_select_channel(struct i2c_client *client, u8 ctrl_byte) > +{ > + int status; > + struct i2c_msg msg = { > + .addr = client->addr, > + .flags = 0, > + .len = sizeof(ctrl_byte), > + .buf = &ctrl_byte, > + }; > + > + status = i2c_transfer(client->adapter, &msg, 1); > + > + return (status == 1) ? 0 : -EIO; This isn't what I said and asked for. It drops the unnecessary else, but now it overwrites an error value. Guenter > +} > + > +static int max127_read_channel(struct i2c_client *client, long *val) > +{ > + int status; > + u8 i2c_data[MAX127_DATA_LEN]; > + struct i2c_msg msg = { > + .addr = client->addr, > + .flags = I2C_M_RD, > + .len = sizeof(i2c_data), > + .buf = i2c_data, > + }; > + > + status = i2c_transfer(client->adapter, &msg, 1); > + if (status != 1) > + return -EIO; This isn't what I asked for. Guenter > + > + *val = (i2c_data[1] >> MAX127_DATA_SHIFT) | > + ((u16)i2c_data[0] << MAX127_DATA_SHIFT); > + return 0; > +} > + > +static long max127_process_raw(u8 ctrl_byte, long raw) > +{ > + long scale, weight; > + > + /* > + * MAX127's data coding is binary in unipolar mode with 1 LSB = > + * (Full-Scale/4096) and two’s complement binary in bipolar mode > + * with 1 LSB = [(2 x |FS|)/4096]. > + * Refer to MAX127 datasheet, "Transfer Function" section for > + * details. > + */ > + scale = (ctrl_byte & MAX127_CTRL_RNG) ? MAX127_FULL_RANGE : > + MAX127_HALF_RANGE; > + if (ctrl_byte & MAX127_CTRL_BIP) { > + weight = (raw & MAX127_SIGN_BIT); > + raw &= ~MAX127_SIGN_BIT; > + raw -= weight; > + raw *= 2; > + } > + > + return raw * scale / 4096; > +} > + > +static int max127_read_input(struct max127_data *data, int channel, long *val) > +{ > + long raw; > + int status; > + struct i2c_client *client = data->client; > + u8 ctrl_byte = data->ctrl_byte[channel]; > + > + mutex_lock(&data->lock); > + > + status = max127_select_channel(client, ctrl_byte); > + if (status) > + goto exit; > + > + status = max127_read_channel(client, &raw); > + if (status) > + goto exit; > + > + *val = max127_process_raw(ctrl_byte, raw); > + > +exit: > + mutex_unlock(&data->lock); > + return status; > +} > + > +static int max127_read_min(struct max127_data *data, int channel, long *val) > +{ > + u8 rng_bip = (data->ctrl_byte[channel] >> 2) & 3; > + static const int min_input_map[4] = { > + 0, /* RNG=0, BIP=0 */ > + -MAX127_HALF_RANGE, /* RNG=0, BIP=1 */ > + 0, /* RNG=1, BIP=0 */ > + -MAX127_FULL_RANGE, /* RNG=1, BIP=1 */ > + }; > + > + *val = min_input_map[rng_bip]; > + return 0; > +} > + > +static int max127_read_max(struct max127_data *data, int channel, long *val) > +{ > + u8 rng_bip = (data->ctrl_byte[channel] >> 2) & 3; > + static const int max_input_map[4] = { > + MAX127_HALF_RANGE, /* RNG=0, BIP=0 */ > + MAX127_HALF_RANGE, /* RNG=0, BIP=1 */ > + MAX127_FULL_RANGE, /* RNG=1, BIP=0 */ > + MAX127_FULL_RANGE, /* RNG=1, BIP=1 */ > + }; > + > + *val = max_input_map[rng_bip]; > + return 0; > +} > + > +static int max127_write_min(struct max127_data *data, int channel, long val) > +{ > + u8 ctrl; > + > + mutex_lock(&data->lock); > + > + ctrl = data->ctrl_byte[channel]; > + if (val <= -MAX127_FULL_RANGE) { > + ctrl |= (MAX127_CTRL_RNG | MAX127_CTRL_BIP); > + } else if (val < 0) { > + ctrl |= MAX127_CTRL_BIP; > + ctrl &= ~MAX127_CTRL_RNG; > + } else { > + ctrl &= ~MAX127_CTRL_BIP; > + } > + data->ctrl_byte[channel] = ctrl; > + > + mutex_unlock(&data->lock); > + > + return 0; > +} > + > +static int max127_write_max(struct max127_data *data, int channel, long val) > +{ > + mutex_lock(&data->lock); > + > + if (val >= MAX127_FULL_RANGE) > + data->ctrl_byte[channel] |= MAX127_CTRL_RNG; > + else > + data->ctrl_byte[channel] &= ~MAX127_CTRL_RNG; > + > + mutex_unlock(&data->lock); > + > + return 0; > +} > + > +static umode_t max127_is_visible(const void *_data, > + enum hwmon_sensor_types type, > + u32 attr, int channel) > +{ > + if (type == hwmon_in) { > + switch (attr) { > + case hwmon_in_input: > + return 0444; > + > + case hwmon_in_min: > + case hwmon_in_max: > + return 0644; > + > + default: > + break; > + } > + } > + > + return 0; > +} > + > +static int max127_read(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long *val) > +{ > + int status; > + struct max127_data *data = dev_get_drvdata(dev); > + > + if (type != hwmon_in) > + return -EOPNOTSUPP; > + > + switch (attr) { > + case hwmon_in_input: > + status = max127_read_input(data, channel, val); > + break; > + > + case hwmon_in_min: > + status = max127_read_min(data, channel, val); > + break; > + > + case hwmon_in_max: > + status = max127_read_max(data, channel, val); > + break; > + > + default: > + status = -EOPNOTSUPP; > + break; > + } > + > + return status; > +} > + > +static int max127_write(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long val) > +{ > + int status; > + struct max127_data *data = dev_get_drvdata(dev); > + > + if (type != hwmon_in) > + return -EOPNOTSUPP; > + > + switch (attr) { > + case hwmon_in_min: > + status = max127_write_min(data, channel, val); > + break; > + > + case hwmon_in_max: > + status = max127_write_max(data, channel, val); > + break; > + > + default: > + status = -EOPNOTSUPP; > + break; > + } > + > + return status; > +} > + > +static const struct hwmon_ops max127_hwmon_ops = { > + .is_visible = max127_is_visible, > + .read = max127_read, > + .write = max127_write, > +}; > + > +static const struct hwmon_channel_info *max127_info[] = { > + HWMON_CHANNEL_INFO(in, > + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX, > + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX, > + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX, > + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX, > + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX, > + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX, > + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX, > + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX), > + NULL, > +}; > + > +static const struct hwmon_chip_info max127_chip_info = { > + .ops = &max127_hwmon_ops, > + .info = max127_info, > +}; > + > +static int max127_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + int i; > + struct device *hwmon_dev; > + struct max127_data *data; > + struct device *dev = &client->dev; > + > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->client = client; > + mutex_init(&data->lock); > + for (i = 0; i < ARRAY_SIZE(data->ctrl_byte); i++) > + data->ctrl_byte[i] = (MAX127_CTRL_START | > + MAX127_SET_CHANNEL(i)); > + > + hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, > + data, > + &max127_chip_info, > + NULL); > + > + return PTR_ERR_OR_ZERO(hwmon_dev); > +} > + > +static const struct i2c_device_id max127_id[] = { > + { "max127", 0 }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, max127_id); > + > +static struct i2c_driver max127_driver = { > + .class = I2C_CLASS_HWMON, > + .driver = { > + .name = "max127", > + }, > + .probe = max127_probe, > + .id_table = max127_id, > +}; > + > +module_i2c_driver(max127_driver); > + > +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("Mike Choi <mikechoi@fb.com>"); > +MODULE_AUTHOR("Tao Ren <rentao.bupt@gmail.com>"); > +MODULE_DESCRIPTION("MAX127 Hardware Monitoring driver"); > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/2] hwmon: (max127) Add Maxim MAX127 hardware monitoring driver @ 2020-11-23 13:18 ` Guenter Roeck 0 siblings, 0 replies; 7+ messages in thread From: Guenter Roeck @ 2020-11-23 13:18 UTC (permalink / raw) To: rentao.bupt Cc: linux-hwmon, Jean Delvare, linux-doc, taoren, openbmc, Jonathan Corbet, linux-kernel, mikechoi On Sun, Nov 22, 2020 at 11:45:31PM -0800, rentao.bupt@gmail.com wrote: > From: Tao Ren <rentao.bupt@gmail.com> > > Add hardware monitoring driver for the Maxim MAX127 chip. > > MAX127 min/max range handling code is inspired by the max197 driver. > > Signed-off-by: Tao Ren <rentao.bupt@gmail.com> > --- > Changes in v4: > - delete unnecessary "#include" lines. > - simplify i2c_transfer() error handling. > - add mutex to protect ctrl_byte in write_min|max() functions. > Changes in v3: > - no code change. xdp maintainers were removed from to/cc list. > Changes in v2: > - replace devm_hwmon_device_register_with_groups() with > devm_hwmon_device_register_with_info() API. > - divide min/max read and write methods to separate functions. > - fix raw-to-vin conversion logic. > - refine ctrl_byte handling so mutex is not needed to protect the > byte. > - improve i2c_transfer() error handling. > - a few other improvements (comments, variable naming, and etc.). > > drivers/hwmon/Kconfig | 9 ++ > drivers/hwmon/Makefile | 1 + > drivers/hwmon/max127.c | 346 +++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 356 insertions(+) > create mode 100644 drivers/hwmon/max127.c > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index 9d600e0c5584..716df51edc87 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -950,6 +950,15 @@ config SENSORS_MAX1111 > This driver can also be built as a module. If so, the module > will be called max1111. > > +config SENSORS_MAX127 > + tristate "Maxim MAX127 12-bit 8-channel Data Acquisition System" > + depends on I2C > + help > + Say y here to support Maxim's MAX127 DAS chips. > + > + This driver can also be built as a module. If so, the module > + will be called max127. > + > config SENSORS_MAX16065 > tristate "Maxim MAX16065 System Manager and compatibles" > depends on I2C > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index 1083bbfac779..01ca5d3fbad4 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -127,6 +127,7 @@ obj-$(CONFIG_SENSORS_LTC4260) += ltc4260.o > obj-$(CONFIG_SENSORS_LTC4261) += ltc4261.o > obj-$(CONFIG_SENSORS_LTQ_CPUTEMP) += ltq-cputemp.o > obj-$(CONFIG_SENSORS_MAX1111) += max1111.o > +obj-$(CONFIG_SENSORS_MAX127) += max127.o > obj-$(CONFIG_SENSORS_MAX16065) += max16065.o > obj-$(CONFIG_SENSORS_MAX1619) += max1619.o > obj-$(CONFIG_SENSORS_MAX1668) += max1668.o > diff --git a/drivers/hwmon/max127.c b/drivers/hwmon/max127.c > new file mode 100644 > index 000000000000..1c54146b6086 > --- /dev/null > +++ b/drivers/hwmon/max127.c > @@ -0,0 +1,346 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Hardware monitoring driver for MAX127. > + * > + * Copyright (c) 2020 Facebook Inc. > + */ > + > +#include <linux/err.h> > +#include <linux/hwmon.h> > +#include <linux/i2c.h> > +#include <linux/init.h> > +#include <linux/module.h> > + > +/* > + * MAX127 Control Byte. Refer to MAX127 datasheet, Table 1 "Control-Byte > + * Format" for details. > + */ > +#define MAX127_CTRL_START BIT(7) > +#define MAX127_CTRL_SEL_SHIFT 4 > +#define MAX127_CTRL_RNG BIT(3) > +#define MAX127_CTRL_BIP BIT(2) > +#define MAX127_CTRL_PD1 BIT(1) > +#define MAX127_CTRL_PD0 BIT(0) > + > +#define MAX127_NUM_CHANNELS 8 > +#define MAX127_SET_CHANNEL(ch) (((ch) & 7) << MAX127_CTRL_SEL_SHIFT) > + > +/* > + * MAX127 channel input ranges. Refer to MAX127 datasheet, Table 3 "Range > + * and Polarity Selection" for details. > + */ > +#define MAX127_FULL_RANGE 10000 /* 10V */ > +#define MAX127_HALF_RANGE 5000 /* 5V */ > + > +/* > + * MAX127 returns 2 bytes at read: > + * - the first byte contains data[11:4]. > + * - the second byte contains data[3:0] (MSB) and 4 dummy 0s (LSB). > + * Refer to MAX127 datasheet, "Read a Conversion (Read Cycle)" section > + * for details. > + */ > +#define MAX127_DATA_LEN 2 > +#define MAX127_DATA_SHIFT 4 > + > +#define MAX127_SIGN_BIT BIT(11) > + > +struct max127_data { > + struct mutex lock; > + struct i2c_client *client; > + u8 ctrl_byte[MAX127_NUM_CHANNELS]; > +}; > + > +static int max127_select_channel(struct i2c_client *client, u8 ctrl_byte) > +{ > + int status; > + struct i2c_msg msg = { > + .addr = client->addr, > + .flags = 0, > + .len = sizeof(ctrl_byte), > + .buf = &ctrl_byte, > + }; > + > + status = i2c_transfer(client->adapter, &msg, 1); > + > + return (status == 1) ? 0 : -EIO; This isn't what I said and asked for. It drops the unnecessary else, but now it overwrites an error value. Guenter > +} > + > +static int max127_read_channel(struct i2c_client *client, long *val) > +{ > + int status; > + u8 i2c_data[MAX127_DATA_LEN]; > + struct i2c_msg msg = { > + .addr = client->addr, > + .flags = I2C_M_RD, > + .len = sizeof(i2c_data), > + .buf = i2c_data, > + }; > + > + status = i2c_transfer(client->adapter, &msg, 1); > + if (status != 1) > + return -EIO; This isn't what I asked for. Guenter > + > + *val = (i2c_data[1] >> MAX127_DATA_SHIFT) | > + ((u16)i2c_data[0] << MAX127_DATA_SHIFT); > + return 0; > +} > + > +static long max127_process_raw(u8 ctrl_byte, long raw) > +{ > + long scale, weight; > + > + /* > + * MAX127's data coding is binary in unipolar mode with 1 LSB = > + * (Full-Scale/4096) and two’s complement binary in bipolar mode > + * with 1 LSB = [(2 x |FS|)/4096]. > + * Refer to MAX127 datasheet, "Transfer Function" section for > + * details. > + */ > + scale = (ctrl_byte & MAX127_CTRL_RNG) ? MAX127_FULL_RANGE : > + MAX127_HALF_RANGE; > + if (ctrl_byte & MAX127_CTRL_BIP) { > + weight = (raw & MAX127_SIGN_BIT); > + raw &= ~MAX127_SIGN_BIT; > + raw -= weight; > + raw *= 2; > + } > + > + return raw * scale / 4096; > +} > + > +static int max127_read_input(struct max127_data *data, int channel, long *val) > +{ > + long raw; > + int status; > + struct i2c_client *client = data->client; > + u8 ctrl_byte = data->ctrl_byte[channel]; > + > + mutex_lock(&data->lock); > + > + status = max127_select_channel(client, ctrl_byte); > + if (status) > + goto exit; > + > + status = max127_read_channel(client, &raw); > + if (status) > + goto exit; > + > + *val = max127_process_raw(ctrl_byte, raw); > + > +exit: > + mutex_unlock(&data->lock); > + return status; > +} > + > +static int max127_read_min(struct max127_data *data, int channel, long *val) > +{ > + u8 rng_bip = (data->ctrl_byte[channel] >> 2) & 3; > + static const int min_input_map[4] = { > + 0, /* RNG=0, BIP=0 */ > + -MAX127_HALF_RANGE, /* RNG=0, BIP=1 */ > + 0, /* RNG=1, BIP=0 */ > + -MAX127_FULL_RANGE, /* RNG=1, BIP=1 */ > + }; > + > + *val = min_input_map[rng_bip]; > + return 0; > +} > + > +static int max127_read_max(struct max127_data *data, int channel, long *val) > +{ > + u8 rng_bip = (data->ctrl_byte[channel] >> 2) & 3; > + static const int max_input_map[4] = { > + MAX127_HALF_RANGE, /* RNG=0, BIP=0 */ > + MAX127_HALF_RANGE, /* RNG=0, BIP=1 */ > + MAX127_FULL_RANGE, /* RNG=1, BIP=0 */ > + MAX127_FULL_RANGE, /* RNG=1, BIP=1 */ > + }; > + > + *val = max_input_map[rng_bip]; > + return 0; > +} > + > +static int max127_write_min(struct max127_data *data, int channel, long val) > +{ > + u8 ctrl; > + > + mutex_lock(&data->lock); > + > + ctrl = data->ctrl_byte[channel]; > + if (val <= -MAX127_FULL_RANGE) { > + ctrl |= (MAX127_CTRL_RNG | MAX127_CTRL_BIP); > + } else if (val < 0) { > + ctrl |= MAX127_CTRL_BIP; > + ctrl &= ~MAX127_CTRL_RNG; > + } else { > + ctrl &= ~MAX127_CTRL_BIP; > + } > + data->ctrl_byte[channel] = ctrl; > + > + mutex_unlock(&data->lock); > + > + return 0; > +} > + > +static int max127_write_max(struct max127_data *data, int channel, long val) > +{ > + mutex_lock(&data->lock); > + > + if (val >= MAX127_FULL_RANGE) > + data->ctrl_byte[channel] |= MAX127_CTRL_RNG; > + else > + data->ctrl_byte[channel] &= ~MAX127_CTRL_RNG; > + > + mutex_unlock(&data->lock); > + > + return 0; > +} > + > +static umode_t max127_is_visible(const void *_data, > + enum hwmon_sensor_types type, > + u32 attr, int channel) > +{ > + if (type == hwmon_in) { > + switch (attr) { > + case hwmon_in_input: > + return 0444; > + > + case hwmon_in_min: > + case hwmon_in_max: > + return 0644; > + > + default: > + break; > + } > + } > + > + return 0; > +} > + > +static int max127_read(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long *val) > +{ > + int status; > + struct max127_data *data = dev_get_drvdata(dev); > + > + if (type != hwmon_in) > + return -EOPNOTSUPP; > + > + switch (attr) { > + case hwmon_in_input: > + status = max127_read_input(data, channel, val); > + break; > + > + case hwmon_in_min: > + status = max127_read_min(data, channel, val); > + break; > + > + case hwmon_in_max: > + status = max127_read_max(data, channel, val); > + break; > + > + default: > + status = -EOPNOTSUPP; > + break; > + } > + > + return status; > +} > + > +static int max127_write(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long val) > +{ > + int status; > + struct max127_data *data = dev_get_drvdata(dev); > + > + if (type != hwmon_in) > + return -EOPNOTSUPP; > + > + switch (attr) { > + case hwmon_in_min: > + status = max127_write_min(data, channel, val); > + break; > + > + case hwmon_in_max: > + status = max127_write_max(data, channel, val); > + break; > + > + default: > + status = -EOPNOTSUPP; > + break; > + } > + > + return status; > +} > + > +static const struct hwmon_ops max127_hwmon_ops = { > + .is_visible = max127_is_visible, > + .read = max127_read, > + .write = max127_write, > +}; > + > +static const struct hwmon_channel_info *max127_info[] = { > + HWMON_CHANNEL_INFO(in, > + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX, > + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX, > + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX, > + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX, > + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX, > + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX, > + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX, > + HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX), > + NULL, > +}; > + > +static const struct hwmon_chip_info max127_chip_info = { > + .ops = &max127_hwmon_ops, > + .info = max127_info, > +}; > + > +static int max127_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + int i; > + struct device *hwmon_dev; > + struct max127_data *data; > + struct device *dev = &client->dev; > + > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->client = client; > + mutex_init(&data->lock); > + for (i = 0; i < ARRAY_SIZE(data->ctrl_byte); i++) > + data->ctrl_byte[i] = (MAX127_CTRL_START | > + MAX127_SET_CHANNEL(i)); > + > + hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, > + data, > + &max127_chip_info, > + NULL); > + > + return PTR_ERR_OR_ZERO(hwmon_dev); > +} > + > +static const struct i2c_device_id max127_id[] = { > + { "max127", 0 }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, max127_id); > + > +static struct i2c_driver max127_driver = { > + .class = I2C_CLASS_HWMON, > + .driver = { > + .name = "max127", > + }, > + .probe = max127_probe, > + .id_table = max127_id, > +}; > + > +module_i2c_driver(max127_driver); > + > +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("Mike Choi <mikechoi@fb.com>"); > +MODULE_AUTHOR("Tao Ren <rentao.bupt@gmail.com>"); > +MODULE_DESCRIPTION("MAX127 Hardware Monitoring driver"); > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/2] hwmon: (max127) Add Maxim MAX127 hardware monitoring driver 2020-11-23 13:18 ` Guenter Roeck @ 2020-11-23 19:03 ` Tao Ren -1 siblings, 0 replies; 7+ messages in thread From: Tao Ren @ 2020-11-23 19:03 UTC (permalink / raw) To: Guenter Roeck Cc: Jean Delvare, Jonathan Corbet, linux-hwmon, linux-doc, linux-kernel, openbmc, taoren, mikechoi On Mon, Nov 23, 2020 at 05:18:32AM -0800, Guenter Roeck wrote: > On Sun, Nov 22, 2020 at 11:45:31PM -0800, rentao.bupt@gmail.com wrote: > > From: Tao Ren <rentao.bupt@gmail.com> > > > > Add hardware monitoring driver for the Maxim MAX127 chip. > > > > MAX127 min/max range handling code is inspired by the max197 driver. > > > > Signed-off-by: Tao Ren <rentao.bupt@gmail.com> > > --- > > Changes in v4: > > - delete unnecessary "#include" lines. > > - simplify i2c_transfer() error handling. > > - add mutex to protect ctrl_byte in write_min|max() functions. > > Changes in v3: > > - no code change. xdp maintainers were removed from to/cc list. > > Changes in v2: > > - replace devm_hwmon_device_register_with_groups() with > > devm_hwmon_device_register_with_info() API. > > - divide min/max read and write methods to separate functions. > > - fix raw-to-vin conversion logic. > > - refine ctrl_byte handling so mutex is not needed to protect the > > byte. > > - improve i2c_transfer() error handling. > > - a few other improvements (comments, variable naming, and etc.). > > > > drivers/hwmon/Kconfig | 9 ++ > > drivers/hwmon/Makefile | 1 + > > drivers/hwmon/max127.c | 346 +++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 356 insertions(+) > > create mode 100644 drivers/hwmon/max127.c > > > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > > index 9d600e0c5584..716df51edc87 100644 > > --- a/drivers/hwmon/Kconfig > > +++ b/drivers/hwmon/Kconfig > > @@ -950,6 +950,15 @@ config SENSORS_MAX1111 > > This driver can also be built as a module. If so, the module > > will be called max1111. > > > > +config SENSORS_MAX127 > > + tristate "Maxim MAX127 12-bit 8-channel Data Acquisition System" > > + depends on I2C > > + help > > + Say y here to support Maxim's MAX127 DAS chips. > > + > > + This driver can also be built as a module. If so, the module > > + will be called max127. > > + > > config SENSORS_MAX16065 > > tristate "Maxim MAX16065 System Manager and compatibles" > > depends on I2C > > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > > index 1083bbfac779..01ca5d3fbad4 100644 > > --- a/drivers/hwmon/Makefile > > +++ b/drivers/hwmon/Makefile > > @@ -127,6 +127,7 @@ obj-$(CONFIG_SENSORS_LTC4260) += ltc4260.o > > obj-$(CONFIG_SENSORS_LTC4261) += ltc4261.o > > obj-$(CONFIG_SENSORS_LTQ_CPUTEMP) += ltq-cputemp.o > > obj-$(CONFIG_SENSORS_MAX1111) += max1111.o > > +obj-$(CONFIG_SENSORS_MAX127) += max127.o > > obj-$(CONFIG_SENSORS_MAX16065) += max16065.o > > obj-$(CONFIG_SENSORS_MAX1619) += max1619.o > > obj-$(CONFIG_SENSORS_MAX1668) += max1668.o > > diff --git a/drivers/hwmon/max127.c b/drivers/hwmon/max127.c > > new file mode 100644 > > index 000000000000..1c54146b6086 > > --- /dev/null > > +++ b/drivers/hwmon/max127.c > > @@ -0,0 +1,346 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Hardware monitoring driver for MAX127. > > + * > > + * Copyright (c) 2020 Facebook Inc. > > + */ > > + > > +#include <linux/err.h> > > +#include <linux/hwmon.h> > > +#include <linux/i2c.h> > > +#include <linux/init.h> > > +#include <linux/module.h> > > + > > +/* > > + * MAX127 Control Byte. Refer to MAX127 datasheet, Table 1 "Control-Byte > > + * Format" for details. > > + */ > > +#define MAX127_CTRL_START BIT(7) > > +#define MAX127_CTRL_SEL_SHIFT 4 > > +#define MAX127_CTRL_RNG BIT(3) > > +#define MAX127_CTRL_BIP BIT(2) > > +#define MAX127_CTRL_PD1 BIT(1) > > +#define MAX127_CTRL_PD0 BIT(0) > > + > > +#define MAX127_NUM_CHANNELS 8 > > +#define MAX127_SET_CHANNEL(ch) (((ch) & 7) << MAX127_CTRL_SEL_SHIFT) > > + > > +/* > > + * MAX127 channel input ranges. Refer to MAX127 datasheet, Table 3 "Range > > + * and Polarity Selection" for details. > > + */ > > +#define MAX127_FULL_RANGE 10000 /* 10V */ > > +#define MAX127_HALF_RANGE 5000 /* 5V */ > > + > > +/* > > + * MAX127 returns 2 bytes at read: > > + * - the first byte contains data[11:4]. > > + * - the second byte contains data[3:0] (MSB) and 4 dummy 0s (LSB). > > + * Refer to MAX127 datasheet, "Read a Conversion (Read Cycle)" section > > + * for details. > > + */ > > +#define MAX127_DATA_LEN 2 > > +#define MAX127_DATA_SHIFT 4 > > + > > +#define MAX127_SIGN_BIT BIT(11) > > + > > +struct max127_data { > > + struct mutex lock; > > + struct i2c_client *client; > > + u8 ctrl_byte[MAX127_NUM_CHANNELS]; > > +}; > > + > > +static int max127_select_channel(struct i2c_client *client, u8 ctrl_byte) > > +{ > > + int status; > > + struct i2c_msg msg = { > > + .addr = client->addr, > > + .flags = 0, > > + .len = sizeof(ctrl_byte), > > + .buf = &ctrl_byte, > > + }; > > + > > + status = i2c_transfer(client->adapter, &msg, 1); > > + > > + return (status == 1) ? 0 : -EIO; > > This isn't what I said and asked for. It drops the unnecessary else, > but now it overwrites an error value. > > Guenter > > > +} > > + > > +static int max127_read_channel(struct i2c_client *client, long *val) > > +{ > > + int status; > > + u8 i2c_data[MAX127_DATA_LEN]; > > + struct i2c_msg msg = { > > + .addr = client->addr, > > + .flags = I2C_M_RD, > > + .len = sizeof(i2c_data), > > + .buf = i2c_data, > > + }; > > + > > + status = i2c_transfer(client->adapter, &msg, 1); > > + if (status != 1) > > + return -EIO; > > This isn't what I asked for. > > Guenter Both places are fixed in v5. Thanks for the clarify, Guenter. Cheers, Tao ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/2] hwmon: (max127) Add Maxim MAX127 hardware monitoring driver @ 2020-11-23 19:03 ` Tao Ren 0 siblings, 0 replies; 7+ messages in thread From: Tao Ren @ 2020-11-23 19:03 UTC (permalink / raw) To: Guenter Roeck Cc: linux-hwmon, Jean Delvare, linux-doc, taoren, openbmc, Jonathan Corbet, linux-kernel, mikechoi On Mon, Nov 23, 2020 at 05:18:32AM -0800, Guenter Roeck wrote: > On Sun, Nov 22, 2020 at 11:45:31PM -0800, rentao.bupt@gmail.com wrote: > > From: Tao Ren <rentao.bupt@gmail.com> > > > > Add hardware monitoring driver for the Maxim MAX127 chip. > > > > MAX127 min/max range handling code is inspired by the max197 driver. > > > > Signed-off-by: Tao Ren <rentao.bupt@gmail.com> > > --- > > Changes in v4: > > - delete unnecessary "#include" lines. > > - simplify i2c_transfer() error handling. > > - add mutex to protect ctrl_byte in write_min|max() functions. > > Changes in v3: > > - no code change. xdp maintainers were removed from to/cc list. > > Changes in v2: > > - replace devm_hwmon_device_register_with_groups() with > > devm_hwmon_device_register_with_info() API. > > - divide min/max read and write methods to separate functions. > > - fix raw-to-vin conversion logic. > > - refine ctrl_byte handling so mutex is not needed to protect the > > byte. > > - improve i2c_transfer() error handling. > > - a few other improvements (comments, variable naming, and etc.). > > > > drivers/hwmon/Kconfig | 9 ++ > > drivers/hwmon/Makefile | 1 + > > drivers/hwmon/max127.c | 346 +++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 356 insertions(+) > > create mode 100644 drivers/hwmon/max127.c > > > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > > index 9d600e0c5584..716df51edc87 100644 > > --- a/drivers/hwmon/Kconfig > > +++ b/drivers/hwmon/Kconfig > > @@ -950,6 +950,15 @@ config SENSORS_MAX1111 > > This driver can also be built as a module. If so, the module > > will be called max1111. > > > > +config SENSORS_MAX127 > > + tristate "Maxim MAX127 12-bit 8-channel Data Acquisition System" > > + depends on I2C > > + help > > + Say y here to support Maxim's MAX127 DAS chips. > > + > > + This driver can also be built as a module. If so, the module > > + will be called max127. > > + > > config SENSORS_MAX16065 > > tristate "Maxim MAX16065 System Manager and compatibles" > > depends on I2C > > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > > index 1083bbfac779..01ca5d3fbad4 100644 > > --- a/drivers/hwmon/Makefile > > +++ b/drivers/hwmon/Makefile > > @@ -127,6 +127,7 @@ obj-$(CONFIG_SENSORS_LTC4260) += ltc4260.o > > obj-$(CONFIG_SENSORS_LTC4261) += ltc4261.o > > obj-$(CONFIG_SENSORS_LTQ_CPUTEMP) += ltq-cputemp.o > > obj-$(CONFIG_SENSORS_MAX1111) += max1111.o > > +obj-$(CONFIG_SENSORS_MAX127) += max127.o > > obj-$(CONFIG_SENSORS_MAX16065) += max16065.o > > obj-$(CONFIG_SENSORS_MAX1619) += max1619.o > > obj-$(CONFIG_SENSORS_MAX1668) += max1668.o > > diff --git a/drivers/hwmon/max127.c b/drivers/hwmon/max127.c > > new file mode 100644 > > index 000000000000..1c54146b6086 > > --- /dev/null > > +++ b/drivers/hwmon/max127.c > > @@ -0,0 +1,346 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Hardware monitoring driver for MAX127. > > + * > > + * Copyright (c) 2020 Facebook Inc. > > + */ > > + > > +#include <linux/err.h> > > +#include <linux/hwmon.h> > > +#include <linux/i2c.h> > > +#include <linux/init.h> > > +#include <linux/module.h> > > + > > +/* > > + * MAX127 Control Byte. Refer to MAX127 datasheet, Table 1 "Control-Byte > > + * Format" for details. > > + */ > > +#define MAX127_CTRL_START BIT(7) > > +#define MAX127_CTRL_SEL_SHIFT 4 > > +#define MAX127_CTRL_RNG BIT(3) > > +#define MAX127_CTRL_BIP BIT(2) > > +#define MAX127_CTRL_PD1 BIT(1) > > +#define MAX127_CTRL_PD0 BIT(0) > > + > > +#define MAX127_NUM_CHANNELS 8 > > +#define MAX127_SET_CHANNEL(ch) (((ch) & 7) << MAX127_CTRL_SEL_SHIFT) > > + > > +/* > > + * MAX127 channel input ranges. Refer to MAX127 datasheet, Table 3 "Range > > + * and Polarity Selection" for details. > > + */ > > +#define MAX127_FULL_RANGE 10000 /* 10V */ > > +#define MAX127_HALF_RANGE 5000 /* 5V */ > > + > > +/* > > + * MAX127 returns 2 bytes at read: > > + * - the first byte contains data[11:4]. > > + * - the second byte contains data[3:0] (MSB) and 4 dummy 0s (LSB). > > + * Refer to MAX127 datasheet, "Read a Conversion (Read Cycle)" section > > + * for details. > > + */ > > +#define MAX127_DATA_LEN 2 > > +#define MAX127_DATA_SHIFT 4 > > + > > +#define MAX127_SIGN_BIT BIT(11) > > + > > +struct max127_data { > > + struct mutex lock; > > + struct i2c_client *client; > > + u8 ctrl_byte[MAX127_NUM_CHANNELS]; > > +}; > > + > > +static int max127_select_channel(struct i2c_client *client, u8 ctrl_byte) > > +{ > > + int status; > > + struct i2c_msg msg = { > > + .addr = client->addr, > > + .flags = 0, > > + .len = sizeof(ctrl_byte), > > + .buf = &ctrl_byte, > > + }; > > + > > + status = i2c_transfer(client->adapter, &msg, 1); > > + > > + return (status == 1) ? 0 : -EIO; > > This isn't what I said and asked for. It drops the unnecessary else, > but now it overwrites an error value. > > Guenter > > > +} > > + > > +static int max127_read_channel(struct i2c_client *client, long *val) > > +{ > > + int status; > > + u8 i2c_data[MAX127_DATA_LEN]; > > + struct i2c_msg msg = { > > + .addr = client->addr, > > + .flags = I2C_M_RD, > > + .len = sizeof(i2c_data), > > + .buf = i2c_data, > > + }; > > + > > + status = i2c_transfer(client->adapter, &msg, 1); > > + if (status != 1) > > + return -EIO; > > This isn't what I asked for. > > Guenter Both places are fixed in v5. Thanks for the clarify, Guenter. Cheers, Tao ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 2/2] docs: hwmon: Document max127 driver 2020-11-23 7:45 [PATCH v4 0/2] hwmon: (max127) Add Maxim MAX127 hardware monitoring rentao.bupt 2020-11-23 7:45 ` [PATCH v4 1/2] hwmon: (max127) Add Maxim MAX127 hardware monitoring driver rentao.bupt @ 2020-11-23 7:45 ` rentao.bupt 1 sibling, 0 replies; 7+ messages in thread From: rentao.bupt @ 2020-11-23 7:45 UTC (permalink / raw) To: Jean Delvare, Guenter Roeck, Jonathan Corbet, linux-hwmon, linux-doc, linux-kernel, openbmc, taoren, mikechoi Cc: Tao Ren From: Tao Ren <rentao.bupt@gmail.com> Add documentation for the max127 hardware monitoring driver. Signed-off-by: Tao Ren <rentao.bupt@gmail.com> Reviewed-by: Guenter Roeck <linux@roeck-us.net> --- Changes in v4: - None. Changes in v3: - no code change. xdp maintainers were removed from to/cc list. Changes in v2: - add more description for min/max sysfs nodes. - convert values from volt to millivolt in the document. Documentation/hwmon/index.rst | 1 + Documentation/hwmon/max127.rst | 45 ++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 Documentation/hwmon/max127.rst diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst index 408760d13813..0a07b6000c20 100644 --- a/Documentation/hwmon/index.rst +++ b/Documentation/hwmon/index.rst @@ -111,6 +111,7 @@ Hardware Monitoring Kernel Drivers ltc4245 ltc4260 ltc4261 + max127 max16064 max16065 max1619 diff --git a/Documentation/hwmon/max127.rst b/Documentation/hwmon/max127.rst new file mode 100644 index 000000000000..dc192dd9c37c --- /dev/null +++ b/Documentation/hwmon/max127.rst @@ -0,0 +1,45 @@ +.. SPDX-License-Identifier: GPL-2.0-or-later + +Kernel driver max127 +==================== + +Author: + + * Tao Ren <rentao.bupt@gmail.com> + +Supported chips: + + * Maxim MAX127 + + Prefix: 'max127' + + Datasheet: https://datasheets.maximintegrated.com/en/ds/MAX127-MAX128.pdf + +Description +----------- + +The MAX127 is a multirange, 12-bit data acquisition system (DAS) providing +8 analog input channels that are independently software programmable for +a variety of ranges. The available ranges are {0,5V}, {0,10V}, {-5,5V} +and {-10,10V}. + +The MAX127 features a 2-wire, I2C-compatible serial interface that allows +communication among multiple devices using SDA and SCL lines. + +Sysfs interface +--------------- + + ============== ============================================================== + in[0-7]_input The input voltage (in mV) of the corresponding channel. + RO + + in[0-7]_min The lower input limit (in mV) for the corresponding channel. + ADC range and LSB will be updated when the limit is changed. + For the MAX127, it will be adjusted to -10000, -5000, or 0. + RW + + in[0-7]_max The higher input limit (in mV) for the corresponding channel. + ADC range and LSB will be updated when the limit is changed. + For the MAX127, it will be adjusted to 0, 5000, or 10000. + RW + ============== ============================================================== -- 2.17.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-11-23 19:05 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-11-23 7:45 [PATCH v4 0/2] hwmon: (max127) Add Maxim MAX127 hardware monitoring rentao.bupt 2020-11-23 7:45 ` [PATCH v4 1/2] hwmon: (max127) Add Maxim MAX127 hardware monitoring driver rentao.bupt 2020-11-23 13:18 ` Guenter Roeck 2020-11-23 13:18 ` Guenter Roeck 2020-11-23 19:03 ` Tao Ren 2020-11-23 19:03 ` Tao Ren 2020-11-23 7:45 ` [PATCH v4 2/2] docs: hwmon: Document max127 driver rentao.bupt
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.