* [PATCH 1/3] hwmon: adm1177: Add ADM1177 Hot Swap Controller and Digital Power Monitor driver @ 2019-12-03 13:57 Beniamin Bia 2019-12-03 13:57 ` [PATCH 2/3] dt-binding: iio: Add documentation for ADM1177 Beniamin Bia ` (2 more replies) 0 siblings, 3 replies; 5+ messages in thread From: Beniamin Bia @ 2019-12-03 13:57 UTC (permalink / raw) To: linux-hwmon Cc: Michael.Hennerich, linux-kernel, jdelvare, linux, mark.rutland, lgirdwood, broonie, devicetree, biabeniamin, Beniamin Bia ADM1177 is a Hot Swap Controller and Digital Power Monitor with Soft Start Pin. Datasheet: Link: https://www.analog.com/media/en/technical-documentation/data-sheets/ADM1177.pdf Signed-off-by: Beniamin Bia <beniamin.bia@analog.com> --- drivers/hwmon/Kconfig | 10 ++ drivers/hwmon/Makefile | 1 + drivers/hwmon/adm1177.c | 274 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 285 insertions(+) create mode 100644 drivers/hwmon/adm1177.c diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 5308c59d7001..3db8f5752675 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -164,6 +164,16 @@ config SENSORS_ADM1031 This driver can also be built as a module. If so, the module will be called adm1031. +config SENSORS_ADM1177 + tristate "Analog Devices ADM1177 and compatibles" + depends on I2C + help + If you say yes here you get support for Analog Devices ADM1177 + sensor chips. + + This driver can also be built as a module. If so, the module + will be called adm1177. + config SENSORS_ADM9240 tristate "Analog Devices ADM9240 and compatibles" depends on I2C diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 40c036ea45e6..27d04eab1be4 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -34,6 +34,7 @@ obj-$(CONFIG_SENSORS_ADM1025) += adm1025.o obj-$(CONFIG_SENSORS_ADM1026) += adm1026.o obj-$(CONFIG_SENSORS_ADM1029) += adm1029.o obj-$(CONFIG_SENSORS_ADM1031) += adm1031.o +obj-$(CONFIG_SENSORS_ADM1177) += adm1177.o obj-$(CONFIG_SENSORS_ADM9240) += adm9240.o obj-$(CONFIG_SENSORS_ADS7828) += ads7828.o obj-$(CONFIG_SENSORS_ADS7871) += ads7871.o diff --git a/drivers/hwmon/adm1177.c b/drivers/hwmon/adm1177.c new file mode 100644 index 000000000000..08950cecc9f9 --- /dev/null +++ b/drivers/hwmon/adm1177.c @@ -0,0 +1,274 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * ADM1177 Hot Swap Controller and Digital Power Monitor with Soft Start Pin + * + * Copyright 2015-2019 Analog Devices Inc. + */ + +#include <linux/module.h> +#include <linux/device.h> +#include <linux/init.h> +#include <linux/i2c.h> +#include <linux/hwmon.h> +#include <linux/regulator/consumer.h> + +/* Command Byte Operations */ +#define ADM1177_CMD_V_CONT BIT(0) +#define ADM1177_CMD_V_ONCE BIT(1) +#define ADM1177_CMD_I_CONT BIT(2) +#define ADM1177_CMD_I_ONCE BIT(3) +#define ADM1177_CMD_VRANGE BIT(4) +#define ADM1177_CMD_STATUS_RD BIT(6) + +/* Extended Register */ +#define ADM1177_REG_ALERT_EN 1 +#define ADM1177_REG_ALERT_TH 2 +#define ADM1177_REG_CONTROL 3 + +/* ADM1177_REG_ALERT_EN */ +#define ADM1177_EN_ADC_OC1 BIT(0) +#define ADM1177_EN_ADC_OC4 BIT(1) +#define ADM1177_EN_HS_ALERT BIT(2) +#define ADM1177_EN_OFF_ALERT BIT(3) +#define ADM1177_CLEAR BIT(4) + +/* ADM1177_REG_CONTROL */ +#define ADM1177_SWOFF BIT(0) + +#define ADM1177_BITS 12 + +/** + * struct adm1177_state - driver instance specific data + * @client pointer to i2c client + * @reg regulator info for the the power supply of the device + * @command internal control register + * @r_sense_uohm current sense resistor value + * @alert_threshold_ua current limit for shutdown + * @vrange_high internal voltage divider + */ +struct adm1177_state { + struct i2c_client *client; + struct regulator *reg; + u8 command; + u32 r_sense_uohm; + u32 alert_threshold_ua; + bool vrange_high; +}; + +static int adm1177_read_raw(struct adm1177_state *st, u8 num, u8 *data) +{ + struct i2c_client *client = st->client; + + return i2c_master_recv(client, data, num); +} + +static int adm1177_write_cmd(struct adm1177_state *st, u8 cmd) +{ + st->command = cmd; + return i2c_smbus_write_byte(st->client, cmd); +} + +static int adm1177_read(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long *val) +{ + struct adm1177_state *st = dev_get_drvdata(dev); + u8 data[3]; + long dummy; + int ret; + + switch (type) { + case hwmon_curr: + ret = adm1177_read_raw(st, 3, data); + if (ret < 0) + return ret; + dummy = (data[1] << 4) | (data[2] & 0xF); + /* + * convert in milliamperes + * ((105.84mV / 4096) x raw) / senseResistor(ohm) + */ + *val = div_u64((105840000ull * dummy), 4096 * st->r_sense_uohm); + return 0; + case hwmon_in: + ret = adm1177_read_raw(st, 3, data); + if (ret < 0) + return ret; + dummy = (data[0] << 4) | (data[2] >> 4); + /* + * convert in millivolts based on resistor devision + * (V_fullscale / 4096) * raw + */ + if (st->command & ADM1177_CMD_VRANGE) + *val = 6650; + else + *val = 26350; + + *val = ((*val * dummy) / 4096); + return 0; + default: + return -EOPNOTSUPP; + } +} + +static umode_t adm1177_is_visible(const void *data, + enum hwmon_sensor_types type, + u32 attr, int channel) +{ + const struct adm1177_state *st = data; + + switch (type) { + case hwmon_in: + switch (attr) { + case hwmon_in_input: + return 0444; + } + break; + case hwmon_curr: + switch (attr) { + case hwmon_curr_input: + if (st->r_sense_uohm) + return 0444; + return 0; + } + break; + default: + break; + } + return 0; +} + +static const u32 adm1177_curr_config[] = { + HWMON_C_INPUT, + 0 +}; + +static const struct hwmon_channel_info adm1177_curr = { + .type = hwmon_curr, + .config = adm1177_curr_config, +}; + +static const u32 adm1177_in_config[] = { + HWMON_I_INPUT, + 0 +}; + +static const struct hwmon_channel_info adm1177_in = { + .type = hwmon_in, + .config = adm1177_in_config, +}; + +static const struct hwmon_channel_info *adm1177_info[] = { + &adm1177_curr, + &adm1177_in, + NULL +}; + +static const struct hwmon_ops adm1177_hwmon_ops = { + .is_visible = adm1177_is_visible, + .read = adm1177_read, +}; + +static const struct hwmon_chip_info adm1177_chip_info = { + .ops = &adm1177_hwmon_ops, + .info = adm1177_info, +}; + +static void adm1177_remove(void *data) +{ + struct adm1177_state *st = data; + + regulator_disable(st->reg); +} + +static int adm1177_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct device *dev = &client->dev; + struct device *hwmon_dev; + struct adm1177_state *st; + int ret; + + st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL); + if (!st) + return -ENOMEM; + + st->client = client; + + st->reg = devm_regulator_get_optional(&client->dev, "vref"); + if (IS_ERR(st->reg)) { + if (PTR_ERR(st->reg) == -EPROBE_DEFER) + return -EPROBE_DEFER; + + st->reg = NULL; + } else { + ret = regulator_enable(st->reg); + if (ret) + return ret; + ret = devm_add_action_or_reset(&client->dev, adm1177_remove, + st); + if (ret) + return ret; + } + + if (device_property_read_u32(dev, "adi,r-sense-micro-ohms", + &st->r_sense_uohm)) + st->r_sense_uohm = 1; + if (device_property_read_u32(dev, "adi,shutdown-threshold-microamp", + &st->alert_threshold_ua)) + st->alert_threshold_ua = 0; + st->vrange_high = device_property_read_bool(dev, + "adi,vrange-high-enable"); + if (st->alert_threshold_ua) { + u64 val; + + val = (0xFFUL * st->alert_threshold_ua * st->r_sense_uohm); + val = div_u64(val, 105840000U); + if (val > 0xFF) { + dev_warn(&client->dev, + "Requested shutdown current %d uA above limit\n", + st->alert_threshold_ua); + + val = 0xFF; + } + i2c_smbus_write_byte_data(st->client, ADM1177_REG_ALERT_TH, + val); + } + + ret = adm1177_write_cmd(st, ADM1177_CMD_V_CONT | + ADM1177_CMD_I_CONT | + (st->vrange_high ? 0 : ADM1177_CMD_VRANGE)); + if (ret) + return ret; + + hwmon_dev = + devm_hwmon_device_register_with_info(dev, client->name, st, + &adm1177_chip_info, NULL); + return PTR_ERR_OR_ZERO(hwmon_dev); +} + +static const struct i2c_device_id adm1177_id[] = { + {"adm1177", 0}, + {} +}; +MODULE_DEVICE_TABLE(i2c, adm1177_id); + +static const struct of_device_id adm1177_dt_ids[] = { + { .compatible = "adi,adm1177" }, + {}, +}; +MODULE_DEVICE_TABLE(of, adm1177_dt_ids); + +static struct i2c_driver adm1177_driver = { + .class = I2C_CLASS_HWMON, + .driver = { + .name = "adm1177", + .of_match_table = adm1177_dt_ids, + }, + .probe = adm1177_probe, + .id_table = adm1177_id, +}; +module_i2c_driver(adm1177_driver); + +MODULE_AUTHOR("Beniamin Bia <beniamin.bia@analog.com>"); +MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>"); +MODULE_DESCRIPTION("Analog Devices ADM1177 ADC driver"); +MODULE_LICENSE("GPL v2"); -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] dt-binding: iio: Add documentation for ADM1177 2019-12-03 13:57 [PATCH 1/3] hwmon: adm1177: Add ADM1177 Hot Swap Controller and Digital Power Monitor driver Beniamin Bia @ 2019-12-03 13:57 ` Beniamin Bia 2019-12-03 15:06 ` Guenter Roeck 2019-12-03 13:57 ` [PATCH 3/3] MAINTAINERS: add entry for ADM1177 driver Beniamin Bia 2019-12-03 19:28 ` [PATCH 1/3] hwmon: adm1177: Add ADM1177 Hot Swap Controller and Digital Power Monitor driver Guenter Roeck 2 siblings, 1 reply; 5+ messages in thread From: Beniamin Bia @ 2019-12-03 13:57 UTC (permalink / raw) To: linux-hwmon Cc: Michael.Hennerich, linux-kernel, jdelvare, linux, mark.rutland, lgirdwood, broonie, devicetree, biabeniamin, Beniamin Bia Documentation for ADM1177 was added. Signed-off-by: Beniamin Bia <beniamin.bia@analog.com> --- .../bindings/hwmon/adi,adm1177.yaml | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/adi,adm1177.yaml diff --git a/Documentation/devicetree/bindings/hwmon/adi,adm1177.yaml b/Documentation/devicetree/bindings/hwmon/adi,adm1177.yaml new file mode 100644 index 000000000000..abd9354217ba --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/adi,adm1177.yaml @@ -0,0 +1,65 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hwmon/adi,adm1177.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices ADM1177 Hot Swap Controller and Digital Power Monitor + +maintainers: + - Michael Hennerich <michael.hennerich@analog.com> + - Beniamin Bia <beniamin.bia@analog.com> + +description: | + Analog Devices ADM1177 Hot Swap Controller and Digital Power Monitor + https://www.analog.com/media/en/technical-documentation/data-sheets/ADM1177.pdf + +properties: + compatible: + enum: + - adi,adm1177 + + reg: + maxItems: 1 + + avcc-supply: + description: + Phandle to the Avcc power supply + + adi,r-sense-micro-ohms: + description: + The value of curent sense resistor in microohms. + + adi,shutdown-threshold-microamp: + description: + Specifies the current level at which an over current alert occurs. + + adi,vrange-high-enable: + description: + Specifies which internal voltage divider to be used. A 1 selects + a 7:2 voltage divider while a 0 selects a 14:1 voltage divider. + type: boolean + +required: + - compatible + - reg + - adi,r-sense-micro-ohms + - adi,shutdown-threshold-microamp + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/interrupt-controller/irq.h> + i2c0 { + #address-cells = <1>; + #size-cells = <0>; + + pwmon@5a { + compatible = "adi,adm1177"; + reg = <0x5a>; + adi,r-sense-micro-ohms = <50000>; /* 50 mOhm */ + adi,shutdown-threshold-microamp = <1059000>; /* 1.059 A */ + adi,vrange-high-enable; + }; + }; +... -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/3] dt-binding: iio: Add documentation for ADM1177 2019-12-03 13:57 ` [PATCH 2/3] dt-binding: iio: Add documentation for ADM1177 Beniamin Bia @ 2019-12-03 15:06 ` Guenter Roeck 0 siblings, 0 replies; 5+ messages in thread From: Guenter Roeck @ 2019-12-03 15:06 UTC (permalink / raw) To: Beniamin Bia, linux-hwmon Cc: Michael.Hennerich, linux-kernel, jdelvare, mark.rutland, lgirdwood, broonie, devicetree, biabeniamin On 12/3/19 5:57 AM, Beniamin Bia wrote: > Documentation for ADM1177 was added. > Subject still points to iio. > Signed-off-by: Beniamin Bia <beniamin.bia@analog.com> > --- > .../bindings/hwmon/adi,adm1177.yaml | 65 +++++++++++++++++++ > 1 file changed, 65 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/adi,adm1177.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/adi,adm1177.yaml b/Documentation/devicetree/bindings/hwmon/adi,adm1177.yaml > new file mode 100644 > index 000000000000..abd9354217ba > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/adi,adm1177.yaml > @@ -0,0 +1,65 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/hwmon/adi,adm1177.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices ADM1177 Hot Swap Controller and Digital Power Monitor > + > +maintainers: > + - Michael Hennerich <michael.hennerich@analog.com> > + - Beniamin Bia <beniamin.bia@analog.com> > + > +description: | > + Analog Devices ADM1177 Hot Swap Controller and Digital Power Monitor > + https://www.analog.com/media/en/technical-documentation/data-sheets/ADM1177.pdf > + > +properties: > + compatible: > + enum: > + - adi,adm1177 > + > + reg: > + maxItems: 1 > + > + avcc-supply: > + description: > + Phandle to the Avcc power supply > + > + adi,r-sense-micro-ohms: > + description: > + The value of curent sense resistor in microohms. There is a standard property for that (shunt-resistor-micro-ohms). > + > + adi,shutdown-threshold-microamp: > + description: > + Specifies the current level at which an over current alert occurs. > + > + adi,vrange-high-enable: > + description: > + Specifies which internal voltage divider to be used. A 1 selects > + a 7:2 voltage divider while a 0 selects a 14:1 voltage divider. > + type: boolean > + > +required: > + - compatible > + - reg > + - adi,r-sense-micro-ohms > + - adi,shutdown-threshold-microamp > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + i2c0 { > + #address-cells = <1>; > + #size-cells = <0>; > + > + pwmon@5a { > + compatible = "adi,adm1177"; > + reg = <0x5a>; > + adi,r-sense-micro-ohms = <50000>; /* 50 mOhm */ > + adi,shutdown-threshold-microamp = <1059000>; /* 1.059 A */ > + adi,vrange-high-enable; > + }; > + }; > +... > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 3/3] MAINTAINERS: add entry for ADM1177 driver 2019-12-03 13:57 [PATCH 1/3] hwmon: adm1177: Add ADM1177 Hot Swap Controller and Digital Power Monitor driver Beniamin Bia 2019-12-03 13:57 ` [PATCH 2/3] dt-binding: iio: Add documentation for ADM1177 Beniamin Bia @ 2019-12-03 13:57 ` Beniamin Bia 2019-12-03 19:28 ` [PATCH 1/3] hwmon: adm1177: Add ADM1177 Hot Swap Controller and Digital Power Monitor driver Guenter Roeck 2 siblings, 0 replies; 5+ messages in thread From: Beniamin Bia @ 2019-12-03 13:57 UTC (permalink / raw) To: linux-hwmon Cc: Michael.Hennerich, linux-kernel, jdelvare, linux, mark.rutland, lgirdwood, broonie, devicetree, biabeniamin, Beniamin Bia Add Beniamin Bia and Michael Hennerich as a maintainer for ADM1177 ADC. Signed-off-by: Beniamin Bia <beniamin.bia@analog.com> --- MAINTAINERS | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 3ef731fc753b..bc19b624fcd5 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -978,6 +978,15 @@ W: http://ez.analog.com/community/linux-device-drivers F: drivers/iio/imu/adis16460.c F: Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml +ANALOG DEVICES INC ADM1177 DRIVER +M: Beniamin Bia <beniamin.bia@analog.com> +M: Michael Hennerich <Michael.Hennerich@analog.com> +L: linux-hwmon@vger.kernel.org +W: http://ez.analog.com/community/linux-device-drivers +S: Supported +F: drivers/hwmon/adm1177.c +F: Documentation/devicetree/bindings/hwmon/adi,adm1177.yaml + ANALOG DEVICES INC ADP5061 DRIVER M: Stefan Popa <stefan.popa@analog.com> L: linux-pm@vger.kernel.org -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] hwmon: adm1177: Add ADM1177 Hot Swap Controller and Digital Power Monitor driver 2019-12-03 13:57 [PATCH 1/3] hwmon: adm1177: Add ADM1177 Hot Swap Controller and Digital Power Monitor driver Beniamin Bia 2019-12-03 13:57 ` [PATCH 2/3] dt-binding: iio: Add documentation for ADM1177 Beniamin Bia 2019-12-03 13:57 ` [PATCH 3/3] MAINTAINERS: add entry for ADM1177 driver Beniamin Bia @ 2019-12-03 19:28 ` Guenter Roeck 2 siblings, 0 replies; 5+ messages in thread From: Guenter Roeck @ 2019-12-03 19:28 UTC (permalink / raw) To: Beniamin Bia Cc: linux-hwmon, Michael.Hennerich, linux-kernel, jdelvare, mark.rutland, lgirdwood, broonie, devicetree, biabeniamin On Tue, Dec 03, 2019 at 03:57:09PM +0200, Beniamin Bia wrote: > ADM1177 is a Hot Swap Controller and Digital Power Monitor with > Soft Start Pin. > > Datasheet: > Link: https://www.analog.com/media/en/technical-documentation/data-sheets/ADM1177.pdf > High level question: The chip supports setting the current limit, and it reports the overcurrent status. Any reason for not providing the respective sysfs entries ? > Signed-off-by: Beniamin Bia <beniamin.bia@analog.com> > --- > drivers/hwmon/Kconfig | 10 ++ > drivers/hwmon/Makefile | 1 + > drivers/hwmon/adm1177.c | 274 ++++++++++++++++++++++++++++++++++++++++ Please also provide Documentation/hwmon/adm1177.rst. > 3 files changed, 285 insertions(+) > create mode 100644 drivers/hwmon/adm1177.c > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index 5308c59d7001..3db8f5752675 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -164,6 +164,16 @@ config SENSORS_ADM1031 > This driver can also be built as a module. If so, the module > will be called adm1031. > > +config SENSORS_ADM1177 > + tristate "Analog Devices ADM1177 and compatibles" > + depends on I2C > + help > + If you say yes here you get support for Analog Devices ADM1177 > + sensor chips. > + > + This driver can also be built as a module. If so, the module > + will be called adm1177. > + > config SENSORS_ADM9240 > tristate "Analog Devices ADM9240 and compatibles" > depends on I2C > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index 40c036ea45e6..27d04eab1be4 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -34,6 +34,7 @@ obj-$(CONFIG_SENSORS_ADM1025) += adm1025.o > obj-$(CONFIG_SENSORS_ADM1026) += adm1026.o > obj-$(CONFIG_SENSORS_ADM1029) += adm1029.o > obj-$(CONFIG_SENSORS_ADM1031) += adm1031.o > +obj-$(CONFIG_SENSORS_ADM1177) += adm1177.o > obj-$(CONFIG_SENSORS_ADM9240) += adm9240.o > obj-$(CONFIG_SENSORS_ADS7828) += ads7828.o > obj-$(CONFIG_SENSORS_ADS7871) += ads7871.o > diff --git a/drivers/hwmon/adm1177.c b/drivers/hwmon/adm1177.c > new file mode 100644 > index 000000000000..08950cecc9f9 > --- /dev/null > +++ b/drivers/hwmon/adm1177.c > @@ -0,0 +1,274 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * ADM1177 Hot Swap Controller and Digital Power Monitor with Soft Start Pin > + * > + * Copyright 2015-2019 Analog Devices Inc. > + */ > + > +#include <linux/module.h> > +#include <linux/device.h> > +#include <linux/init.h> > +#include <linux/i2c.h> > +#include <linux/hwmon.h> > +#include <linux/regulator/consumer.h> Alphabetic include file order, please. > + > +/* Command Byte Operations */ > +#define ADM1177_CMD_V_CONT BIT(0) > +#define ADM1177_CMD_V_ONCE BIT(1) > +#define ADM1177_CMD_I_CONT BIT(2) > +#define ADM1177_CMD_I_ONCE BIT(3) > +#define ADM1177_CMD_VRANGE BIT(4) > +#define ADM1177_CMD_STATUS_RD BIT(6) > + Requires #include <linux/bits.h>. ADM1177_CMD_STATUS_RD is not used. > +/* Extended Register */ > +#define ADM1177_REG_ALERT_EN 1 ADM1177_REG_ALERT_EN is not used. > +#define ADM1177_REG_ALERT_TH 2 > +#define ADM1177_REG_CONTROL 3 ADM1177_REG_CONTROL is not used. > + > +/* ADM1177_REG_ALERT_EN */ > +#define ADM1177_EN_ADC_OC1 BIT(0) > +#define ADM1177_EN_ADC_OC4 BIT(1) > +#define ADM1177_EN_HS_ALERT BIT(2) > +#define ADM1177_EN_OFF_ALERT BIT(3) > +#define ADM1177_CLEAR BIT(4) > + > +/* ADM1177_REG_CONTROL */ > +#define ADM1177_SWOFF BIT(0) > + > +#define ADM1177_BITS 12 The above defines are not used. > + > +/** > + * struct adm1177_state - driver instance specific data > + * @client pointer to i2c client > + * @reg regulator info for the the power supply of the device > + * @command internal control register > + * @r_sense_uohm current sense resistor value > + * @alert_threshold_ua current limit for shutdown > + * @vrange_high internal voltage divider > + */ > +struct adm1177_state { > + struct i2c_client *client; > + struct regulator *reg; > + u8 command; > + u32 r_sense_uohm; > + u32 alert_threshold_ua; > + bool vrange_high; > +}; > + > +static int adm1177_read_raw(struct adm1177_state *st, u8 num, u8 *data) > +{ > + struct i2c_client *client = st->client; > + That variable seems unncessary. Just use st->client directly, like below. > + return i2c_master_recv(client, data, num); > +} > + > +static int adm1177_write_cmd(struct adm1177_state *st, u8 cmd) > +{ > + st->command = cmd; This is an odd way to record and check if vrange is high or low, especially since there is already a flag indicating that/if it is high. Why not just use "if (st->vrange_high) instead of "if (st->command & ADM1177_CMD_VRANGE") ? > + return i2c_smbus_write_byte(st->client, cmd); > +} > + > +static int adm1177_read(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long *val) > +{ > + struct adm1177_state *st = dev_get_drvdata(dev); > + u8 data[3]; > + long dummy; > + int ret; > + > + switch (type) { > + case hwmon_curr: > + ret = adm1177_read_raw(st, 3, data); > + if (ret < 0) > + return ret; > + dummy = (data[1] << 4) | (data[2] & 0xF); > + /* > + * convert in milliamperes convert to > + * ((105.84mV / 4096) x raw) / senseResistor(ohm) > + */ > + *val = div_u64((105840000ull * dummy), 4096 * st->r_sense_uohm); > + return 0; > + case hwmon_in: > + ret = adm1177_read_raw(st, 3, data); > + if (ret < 0) > + return ret; > + dummy = (data[0] << 4) | (data[2] >> 4); > + /* > + * convert in millivolts based on resistor devision > + * (V_fullscale / 4096) * raw > + */ > + if (st->command & ADM1177_CMD_VRANGE) > + *val = 6650; > + else > + *val = 26350; > + Dereferencing val several times isn't really very efficient. How about the following ? if (st->vrange_high) dummy *= 26350; else dummy *= 6650; *val = DIV_ROUND_CLOSEST(dummy, 4096); > + *val = ((*val * dummy) / 4096); > + return 0; > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static umode_t adm1177_is_visible(const void *data, > + enum hwmon_sensor_types type, > + u32 attr, int channel) > +{ > + const struct adm1177_state *st = data; > + > + switch (type) { > + case hwmon_in: > + switch (attr) { > + case hwmon_in_input: > + return 0444; > + } > + break; > + case hwmon_curr: > + switch (attr) { > + case hwmon_curr_input: > + if (st->r_sense_uohm) > + return 0444; > + return 0; > + } > + break; > + default: > + break; > + } > + return 0; > +} > + > +static const u32 adm1177_curr_config[] = { > + HWMON_C_INPUT, > + 0 > +}; > + > +static const struct hwmon_channel_info adm1177_curr = { > + .type = hwmon_curr, > + .config = adm1177_curr_config, > +}; > + > +static const u32 adm1177_in_config[] = { > + HWMON_I_INPUT, > + 0 > +}; > + > +static const struct hwmon_channel_info adm1177_in = { > + .type = hwmon_in, > + .config = adm1177_in_config, > +}; > + > +static const struct hwmon_channel_info *adm1177_info[] = { > + &adm1177_curr, > + &adm1177_in, > + NULL > +}; Please use the HWMON_CHANNEL_INFO macro to declare the above. > + > +static const struct hwmon_ops adm1177_hwmon_ops = { > + .is_visible = adm1177_is_visible, > + .read = adm1177_read, > +}; > + > +static const struct hwmon_chip_info adm1177_chip_info = { > + .ops = &adm1177_hwmon_ops, > + .info = adm1177_info, > +}; > + > +static void adm1177_remove(void *data) > +{ > + struct adm1177_state *st = data; > + > + regulator_disable(st->reg); > +} > + > +static int adm1177_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct device *dev = &client->dev; > + struct device *hwmon_dev; > + struct adm1177_state *st; > + int ret; > + > + st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL); > + if (!st) > + return -ENOMEM; > + > + st->client = client; > + > + st->reg = devm_regulator_get_optional(&client->dev, "vref"); > + if (IS_ERR(st->reg)) { > + if (PTR_ERR(st->reg) == -EPROBE_DEFER) > + return -EPROBE_DEFER; > + > + st->reg = NULL; > + } else { > + ret = regulator_enable(st->reg); > + if (ret) > + return ret; > + ret = devm_add_action_or_reset(&client->dev, adm1177_remove, > + st); > + if (ret) > + return ret; > + } > + > + if (device_property_read_u32(dev, "adi,r-sense-micro-ohms", > + &st->r_sense_uohm)) > + st->r_sense_uohm = 1; Does the default make sense ? A default of 1 mOhm seems to make more sense. I assume that you explicitly want to be able to disable current sensing by setting r_sense_uohm to 0. If so, that needs to be documented in the devicetree file. Also, as written, the property is optional. I think this also needs to be documented. > + if (device_property_read_u32(dev, "adi,shutdown-threshold-microamp", > + &st->alert_threshold_ua)) > + st->alert_threshold_ua = 0; > + st->vrange_high = device_property_read_bool(dev, > + "adi,vrange-high-enable"); > + if (st->alert_threshold_ua) { > + u64 val; > + > + val = (0xFFUL * st->alert_threshold_ua * st->r_sense_uohm); Unnecessary ( ) The above sets val to 0 if st->r_sense_uohm is 0. That means the register value is set to 0. Does that mean "disable" for the chip ? > + val = div_u64(val, 105840000U); > + if (val > 0xFF) { > + dev_warn(&client->dev, > + "Requested shutdown current %d uA above limit\n", > + st->alert_threshold_ua); > + > + val = 0xFF; > + } > + i2c_smbus_write_byte_data(st->client, ADM1177_REG_ALERT_TH, > + val); > + } Does the chip have a default value ? > + > + ret = adm1177_write_cmd(st, ADM1177_CMD_V_CONT | > + ADM1177_CMD_I_CONT | > + (st->vrange_high ? 0 : ADM1177_CMD_VRANGE)); > + if (ret) > + return ret; > + > + hwmon_dev = > + devm_hwmon_device_register_with_info(dev, client->name, st, > + &adm1177_chip_info, NULL); > + return PTR_ERR_OR_ZERO(hwmon_dev); > +} > + > +static const struct i2c_device_id adm1177_id[] = { > + {"adm1177", 0}, > + {} > +}; > +MODULE_DEVICE_TABLE(i2c, adm1177_id); > + > +static const struct of_device_id adm1177_dt_ids[] = { > + { .compatible = "adi,adm1177" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, adm1177_dt_ids); > + > +static struct i2c_driver adm1177_driver = { > + .class = I2C_CLASS_HWMON, > + .driver = { > + .name = "adm1177", > + .of_match_table = adm1177_dt_ids, > + }, > + .probe = adm1177_probe, > + .id_table = adm1177_id, > +}; > +module_i2c_driver(adm1177_driver); > + > +MODULE_AUTHOR("Beniamin Bia <beniamin.bia@analog.com>"); > +MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>"); > +MODULE_DESCRIPTION("Analog Devices ADM1177 ADC driver"); > +MODULE_LICENSE("GPL v2"); > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-12-03 19:29 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-03 13:57 [PATCH 1/3] hwmon: adm1177: Add ADM1177 Hot Swap Controller and Digital Power Monitor driver Beniamin Bia 2019-12-03 13:57 ` [PATCH 2/3] dt-binding: iio: Add documentation for ADM1177 Beniamin Bia 2019-12-03 15:06 ` Guenter Roeck 2019-12-03 13:57 ` [PATCH 3/3] MAINTAINERS: add entry for ADM1177 driver Beniamin Bia 2019-12-03 19:28 ` [PATCH 1/3] hwmon: adm1177: Add ADM1177 Hot Swap Controller and Digital Power Monitor driver Guenter Roeck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).