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 <[email protected]>
---
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 <[email protected]>");
+MODULE_AUTHOR("Michael Hennerich <[email protected]>");
+MODULE_DESCRIPTION("Analog Devices ADM1177 ADC driver");
+MODULE_LICENSE("GPL v2");
--
2.17.1
Add Beniamin Bia and Michael Hennerich as a maintainer for ADM1177 ADC.
Signed-off-by: Beniamin Bia <[email protected]>
---
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 <[email protected]>
+M: Michael Hennerich <[email protected]>
+L: [email protected]
+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 <[email protected]>
L: [email protected]
--
2.17.1
Documentation for ADM1177 was added.
Signed-off-by: Beniamin Bia <[email protected]>
---
.../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 <[email protected]>
+ - Beniamin Bia <[email protected]>
+
+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
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 <[email protected]>
> ---
> .../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 <[email protected]>
> + - Beniamin Bia <[email protected]>
> +
> +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;
> + };
> + };
> +...
>
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 <[email protected]>
> ---
> 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 <[email protected]>");
> +MODULE_AUTHOR("Michael Hennerich <[email protected]>");
> +MODULE_DESCRIPTION("Analog Devices ADM1177 ADC driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.17.1
>