2019-11-12 15:39:25

by Beniamin Bia

[permalink] [raw]
Subject: [PATCH 1/3] iio: Add ADM1177 Hot Swap Controller and Digital Power Monitor driver

From: Michael Hennerich <[email protected]>

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: Michael Hennerich <[email protected]>
Co-developed-by: Beniamin Bia <[email protected]>
Signed-off-by: Beniamin Bia <[email protected]>
---
drivers/iio/adc/Kconfig | 12 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/adm1177.c | 257 ++++++++++++++++++++++++++++++++++++++
3 files changed, 270 insertions(+)
create mode 100644 drivers/iio/adc/adm1177.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 9554890a3200..4db8c6dcb595 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -228,6 +228,18 @@ config ASPEED_ADC
To compile this driver as a module, choose M here: the module will be
called aspeed_adc.

+config ADM1177
+ tristate "Analog Devices ADM1177 Digital Power Monitor driver"
+ depends on I2C
+ help
+ Say yes here to build support for Analog Devices:
+ ADM1177 Hot Swap Controller and Digital Power Monitor
+ with Soft Start Pin. Provides direct access
+ via sysfs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called adm1177.
+
config AT91_ADC
tristate "Atmel AT91 ADC"
depends on ARCH_AT91
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 5ecc481c2967..7899d6a158f3 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_AD7887) += ad7887.o
obj-$(CONFIG_AD7949) += ad7949.o
obj-$(CONFIG_AD799X) += ad799x.o
obj-$(CONFIG_ASPEED_ADC) += aspeed_adc.o
+obj-$(CONFIG_ADM1177) += adm1177.o
obj-$(CONFIG_AT91_ADC) += at91_adc.o
obj-$(CONFIG_AT91_SAMA5D2_ADC) += at91-sama5d2_adc.o
obj-$(CONFIG_AXP20X_ADC) += axp20x_adc.o
diff --git a/drivers/iio/adc/adm1177.c b/drivers/iio/adc/adm1177.c
new file mode 100644
index 000000000000..daec34566a65
--- /dev/null
+++ b/drivers/iio/adc/adm1177.c
@@ -0,0 +1,257 @@
+// 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/device.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/events.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(struct adm1177_state *st, u8 num, u8 *data)
+{
+ struct i2c_client *client = st->client;
+ int ret = 0;
+
+ ret = i2c_master_recv(client, data, num);
+ if (ret < 0) {
+ dev_err(&client->dev, "I2C read error\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+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_write_reg(struct adm1177_state *st, u8 reg, u8 val)
+{
+ return i2c_smbus_write_byte_data(st->client, reg, val);
+}
+
+static int adm1177_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val,
+ int *val2,
+ long mask)
+{
+ struct adm1177_state *st = iio_priv(indio_dev);
+ u8 data[3];
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ adm1177_read(st, 3, data);
+ switch (chan->type) {
+ case IIO_VOLTAGE:
+ *val = (data[0] << 4) | (data[2] >> 4);
+ return IIO_VAL_INT;
+ case IIO_CURRENT:
+ *val = (data[1] << 4) | (data[2] & 0xF);
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+
+ case IIO_CHAN_INFO_SCALE:
+ switch (chan->type) {
+ case IIO_VOLTAGE:
+ if (st->command & ADM1177_CMD_VRANGE)
+ *val = 6650;
+ else
+ *val = 26350;
+
+ *val2 = ADM1177_BITS;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ case IIO_CURRENT:
+ *val = 105840 / st->r_sense_uohm;
+
+ *val2 = ADM1177_BITS;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ default:
+ return -EINVAL;
+ }
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct iio_chan_spec adm1177_channels[] = {
+ {
+ .type = IIO_VOLTAGE,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE),
+ .indexed = 1,
+ .channel = 0,
+ },
+ {
+ .type = IIO_CURRENT,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE),
+ .indexed = 1,
+ .channel = 0,
+ },
+};
+
+static const struct iio_info adm1177_info = {
+ .read_raw = &adm1177_read_raw,
+};
+static void adm1177_remove(void *data)
+{
+ struct adm1177_state *st = data;
+
+ if (st->reg)
+ regulator_disable(st->reg);
+}
+
+static int adm1177_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ int ret;
+ struct device_node *np;
+ struct adm1177_state *st;
+ struct iio_dev *indio_dev;
+
+ indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ st = iio_priv(indio_dev);
+ i2c_set_clientdata(client, indio_dev);
+ st->client = client;
+
+ np = client->dev.of_node;
+
+ 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;
+ }
+
+ of_property_read_u32(np, "adi,r-sense-micro-ohms", &st->r_sense_uohm);
+ of_property_read_u32(np, "adi,shutdown-threshold-microamp",
+ &st->alert_threshold_ua);
+ st->vrange_high = of_property_read_bool(np,
+ "adi,vrange-high-enable");
+ if (st->alert_threshold_ua) {
+ unsigned int val;
+
+ val = (st->alert_threshold_ua * st->r_sense_uohm * 0xFF);
+ val /= 105840000U;
+ if (val > 0xFF) {
+ dev_err(&client->dev,
+ "Requested shutdown current %d uA above limit\n",
+ st->alert_threshold_ua);
+
+ val = 0xFF;
+ }
+ adm1177_write_reg(st, ADM1177_REG_ALERT_TH, val);
+ }
+
+ adm1177_write_cmd(st, ADM1177_CMD_V_CONT |
+ ADM1177_CMD_I_CONT |
+ (st->vrange_high ? 0 : ADM1177_CMD_VRANGE));
+
+ indio_dev->name = id->name;
+ indio_dev->channels = adm1177_channels;
+ indio_dev->num_channels = ARRAY_SIZE(adm1177_channels);
+
+ indio_dev->dev.parent = &client->dev;
+ indio_dev->info = &adm1177_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct i2c_device_id adm1177_ids[] = {
+ { "adm1177", 0 },
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, adm1177_ids);
+
+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 = {
+ .driver = {
+ .name = "adm1177",
+ .of_match_table = adm1177_dt_ids,
+ },
+ .probe = adm1177_probe,
+ .id_table = adm1177_ids,
+};
+module_i2c_driver(adm1177_driver);
+
+MODULE_AUTHOR("Michael Hennerich <[email protected]>");
+MODULE_DESCRIPTION("Analog Devices ADM1177 ADC driver");
+MODULE_LICENSE("GPL v2");
--
2.17.1


2019-11-12 15:40:02

by Beniamin Bia

[permalink] [raw]
Subject: [PATCH 2/3] dt-binding: iio: Add documentation for ADM1177

Documentation for ADM1177 was added.

Signed-off-by: Beniamin Bia <[email protected]>
---
.../bindings/iio/adc/adi,adm1177.yaml | 60 +++++++++++++++++++
1 file changed, 60 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,adm1177.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,adm1177.yaml b/Documentation/devicetree/bindings/iio/adc/adi,adm1177.yaml
new file mode 100644
index 000000000000..69a0230e59f5
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,adm1177.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/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.
+ maximum: 255
+
+ 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
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+ i2c0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ adc@b4 {
+ compatible = "adi,adm1177";
+ reg = <0xb4>;
+ };
+ };
+...
--
2.17.1

2019-11-12 15:40:23

by Beniamin Bia

[permalink] [raw]
Subject: [PATCH 3/3] MAINTAINERS: add entry for ADM1177 driver

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 0fca3b055985..41a34d7a802c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -962,6 +962,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/iio/adc/adm1177.c
+F: Documentation/devicetree/bindings/iio/adc/adi,adm1177.yaml
+
ANALOG DEVICES INC ADP5061 DRIVER
M: Stefan Popa <[email protected]>
L: [email protected]
--
2.17.1

2019-11-12 17:39:28

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 1/3] iio: Add ADM1177 Hot Swap Controller and Digital Power Monitor driver

On Tue, 12 Nov 2019 17:35:50 +0200
Beniamin Bia <[email protected]> wrote:

> From: Michael Hennerich <[email protected]>
>
> 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: Michael Hennerich <[email protected]>
> Co-developed-by: Beniamin Bia <[email protected]>
> Signed-off-by: Beniamin Bia <[email protected]>

Hi Beniamin,

A couple immediate thoughts.

1. That cc list has some rather non obvious people on it. Unless something
fairly surprising is going on, probably better to cut it back a bit.

2. Why IIO? Not entirely obvious to me. From first glance this is definitely
doing hardware monitoring. If there is a reason there should be a clear
statement here on why.

+CC Guenter and Jean as hwmon maintainers.

Whilst I'm here, a very quick review below.

> ---
> drivers/iio/adc/Kconfig | 12 ++
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/adm1177.c | 257 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 270 insertions(+)
> create mode 100644 drivers/iio/adc/adm1177.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 9554890a3200..4db8c6dcb595 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -228,6 +228,18 @@ config ASPEED_ADC
> To compile this driver as a module, choose M here: the module will be
> called aspeed_adc.
>
> +config ADM1177
> + tristate "Analog Devices ADM1177 Digital Power Monitor driver"
> + depends on I2C
> + help
> + Say yes here to build support for Analog Devices:
> + ADM1177 Hot Swap Controller and Digital Power Monitor
> + with Soft Start Pin. Provides direct access
> + via sysfs.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called adm1177.
> +
> config AT91_ADC
> tristate "Atmel AT91 ADC"
> depends on ARCH_AT91
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 5ecc481c2967..7899d6a158f3 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_AD7887) += ad7887.o
> obj-$(CONFIG_AD7949) += ad7949.o
> obj-$(CONFIG_AD799X) += ad799x.o
> obj-$(CONFIG_ASPEED_ADC) += aspeed_adc.o
> +obj-$(CONFIG_ADM1177) += adm1177.o
> obj-$(CONFIG_AT91_ADC) += at91_adc.o
> obj-$(CONFIG_AT91_SAMA5D2_ADC) += at91-sama5d2_adc.o
> obj-$(CONFIG_AXP20X_ADC) += axp20x_adc.o
> diff --git a/drivers/iio/adc/adm1177.c b/drivers/iio/adc/adm1177.c
> new file mode 100644
> index 000000000000..daec34566a65
> --- /dev/null
> +++ b/drivers/iio/adc/adm1177.c
> @@ -0,0 +1,257 @@
> +// 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/device.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +#include <linux/sysfs.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/events.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(struct adm1177_state *st, u8 num, u8 *data)
> +{
> + struct i2c_client *client = st->client;
> + int ret = 0;
> +
> + ret = i2c_master_recv(client, data, num);
> + if (ret < 0) {
> + dev_err(&client->dev, "I2C read error\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +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_write_reg(struct adm1177_state *st, u8 reg, u8 val)
> +{
> + return i2c_smbus_write_byte_data(st->client, reg, val);

These wrappers don't really add anything. I'd just make the i2c
calls directly inline. They will be self explanatory.

> +}
> +
> +static int adm1177_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val,
> + int *val2,
> + long mask)
> +{
> + struct adm1177_state *st = iio_priv(indio_dev);
> + u8 data[3];
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + adm1177_read(st, 3, data);
> + switch (chan->type) {
> + case IIO_VOLTAGE:
> + *val = (data[0] << 4) | (data[2] >> 4);
> + return IIO_VAL_INT;
> + case IIO_CURRENT:
> + *val = (data[1] << 4) | (data[2] & 0xF);
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +
> + case IIO_CHAN_INFO_SCALE:
> + switch (chan->type) {
> + case IIO_VOLTAGE:
> + if (st->command & ADM1177_CMD_VRANGE)
> + *val = 6650;
> + else
> + *val = 26350;
> +
> + *val2 = ADM1177_BITS;
> + return IIO_VAL_FRACTIONAL_LOG2;
> + case IIO_CURRENT:
> + *val = 105840 / st->r_sense_uohm;
> +
> + *val2 = ADM1177_BITS;
> + return IIO_VAL_FRACTIONAL_LOG2;
> + default:
> + return -EINVAL;
> + }
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static const struct iio_chan_spec adm1177_channels[] = {
> + {
> + .type = IIO_VOLTAGE,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE),
> + .indexed = 1,
> + .channel = 0,
> + },
> + {
> + .type = IIO_CURRENT,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE),
> + .indexed = 1,
> + .channel = 0,
> + },
> +};
> +
> +static const struct iio_info adm1177_info = {
> + .read_raw = &adm1177_read_raw,
> +};

blank line here.

> +static void adm1177_remove(void *data)
> +{
> + struct adm1177_state *st = data;
> +
> + if (st->reg)
> + regulator_disable(st->reg);

Probe and remove should be mirror images of each other.
As you have a mixture of managed and non managed calls that
isn't the case here.

devm_add_action_or_reset will sort this out for you if
used to turn off the regulator at the right point on removal.

> +}
> +
> +static int adm1177_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + int ret;
> + struct device_node *np;
> + struct adm1177_state *st;
> + struct iio_dev *indio_dev;
> +
> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*st));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + st = iio_priv(indio_dev);
> + i2c_set_clientdata(client, indio_dev);
> + st->client = client;
> +
> + np = client->dev.of_node;
> +
> + 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;
> + }
> +
> + of_property_read_u32(np, "adi,r-sense-micro-ohms", &st->r_sense_uohm);
> + of_property_read_u32(np, "adi,shutdown-threshold-microamp",
> + &st->alert_threshold_ua);
> + st->vrange_high = of_property_read_bool(np,
> + "adi,vrange-high-enable");
> + if (st->alert_threshold_ua) {
> + unsigned int val;
> +
> + val = (st->alert_threshold_ua * st->r_sense_uohm * 0xFF);
> + val /= 105840000U;
> + if (val > 0xFF) {
> + dev_err(&client->dev,
> + "Requested shutdown current %d uA above limit\n",
> + st->alert_threshold_ua);
> +
> + val = 0xFF;
> + }
> + adm1177_write_reg(st, ADM1177_REG_ALERT_TH, val);
> + }
> +
> + adm1177_write_cmd(st, ADM1177_CMD_V_CONT |
> + ADM1177_CMD_I_CONT |
> + (st->vrange_high ? 0 : ADM1177_CMD_VRANGE));
> +
> + indio_dev->name = id->name;
> + indio_dev->channels = adm1177_channels;
> + indio_dev->num_channels = ARRAY_SIZE(adm1177_channels);
> +
> + indio_dev->dev.parent = &client->dev;
> + indio_dev->info = &adm1177_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + return devm_iio_device_register(&client->dev, indio_dev);
> +}
> +
> +static const struct i2c_device_id adm1177_ids[] = {
> + { "adm1177", 0 },
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, adm1177_ids);
> +
> +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 = {
> + .driver = {
> + .name = "adm1177",
> + .of_match_table = adm1177_dt_ids,
> + },
> + .probe = adm1177_probe,
> + .id_table = adm1177_ids,
> +};
> +module_i2c_driver(adm1177_driver);
> +
> +MODULE_AUTHOR("Michael Hennerich <[email protected]>");
> +MODULE_DESCRIPTION("Analog Devices ADM1177 ADC driver");
> +MODULE_LICENSE("GPL v2");


2019-11-12 18:20:55

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH 1/3] iio: Add ADM1177 Hot Swap Controller and Digital Power Monitor driver

On Tue, Nov 12, 2019 at 05:37:57PM +0000, Jonathan Cameron wrote:
> On Tue, 12 Nov 2019 17:35:50 +0200
> Beniamin Bia <[email protected]> wrote:
>
> > From: Michael Hennerich <[email protected]>
> >
> > 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: Michael Hennerich <[email protected]>
> > Co-developed-by: Beniamin Bia <[email protected]>
> > Signed-off-by: Beniamin Bia <[email protected]>
>
> Hi Beniamin,
>
> A couple immediate thoughts.
>
> 1. That cc list has some rather non obvious people on it. Unless something
> fairly surprising is going on, probably better to cut it back a bit.
>
> 2. Why IIO? Not entirely obvious to me. From first glance this is definitely
> doing hardware monitoring. If there is a reason there should be a clear
> statement here on why.
>

I don't see why this is implemented as iio driver. I think it should be implemented
as hardware monitoring driver.

Some more comments below.

Guenter

> +CC Guenter and Jean as hwmon maintainers.
>
> Whilst I'm here, a very quick review below.
>
> > ---
> > drivers/iio/adc/Kconfig | 12 ++
> > drivers/iio/adc/Makefile | 1 +
> > drivers/iio/adc/adm1177.c | 257 ++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 270 insertions(+)
> > create mode 100644 drivers/iio/adc/adm1177.c
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 9554890a3200..4db8c6dcb595 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -228,6 +228,18 @@ config ASPEED_ADC
> > To compile this driver as a module, choose M here: the module will be
> > called aspeed_adc.
> >
> > +config ADM1177
> > + tristate "Analog Devices ADM1177 Digital Power Monitor driver"
> > + depends on I2C
> > + help
> > + Say yes here to build support for Analog Devices:
> > + ADM1177 Hot Swap Controller and Digital Power Monitor
> > + with Soft Start Pin. Provides direct access
> > + via sysfs.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called adm1177.
> > +
> > config AT91_ADC
> > tristate "Atmel AT91 ADC"
> > depends on ARCH_AT91
> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > index 5ecc481c2967..7899d6a158f3 100644
> > --- a/drivers/iio/adc/Makefile
> > +++ b/drivers/iio/adc/Makefile
> > @@ -24,6 +24,7 @@ obj-$(CONFIG_AD7887) += ad7887.o
> > obj-$(CONFIG_AD7949) += ad7949.o
> > obj-$(CONFIG_AD799X) += ad799x.o
> > obj-$(CONFIG_ASPEED_ADC) += aspeed_adc.o
> > +obj-$(CONFIG_ADM1177) += adm1177.o
> > obj-$(CONFIG_AT91_ADC) += at91_adc.o
> > obj-$(CONFIG_AT91_SAMA5D2_ADC) += at91-sama5d2_adc.o
> > obj-$(CONFIG_AXP20X_ADC) += axp20x_adc.o
> > diff --git a/drivers/iio/adc/adm1177.c b/drivers/iio/adc/adm1177.c
> > new file mode 100644
> > index 000000000000..daec34566a65
> > --- /dev/null
> > +++ b/drivers/iio/adc/adm1177.c
> > @@ -0,0 +1,257 @@
> > +// 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/device.h>
> > +#include <linux/err.h>
> > +#include <linux/i2c.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/slab.h>
> > +#include <linux/sysfs.h>
> > +
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/sysfs.h>
> > +#include <linux/iio/events.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(struct adm1177_state *st, u8 num, u8 *data)
> > +{
> > + struct i2c_client *client = st->client;
> > + int ret = 0;
> > +
> > + ret = i2c_master_recv(client, data, num);
> > + if (ret < 0) {
> > + dev_err(&client->dev, "I2C read error\n");

Seems a bit noisy (and inconsistent). It would make much more sense to
report the error to userspace instead of ignoring it and reporting a random
value (ie the stack content).

> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +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_write_reg(struct adm1177_state *st, u8 reg, u8 val)
> > +{
> > + return i2c_smbus_write_byte_data(st->client, reg, val);
>
> These wrappers don't really add anything. I'd just make the i2c
> calls directly inline. They will be self explanatory.
>

Plus, their return value is never checked.

> > +}
> > +
> > +static int adm1177_read_raw(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan,
> > + int *val,
> > + int *val2,
> > + long mask)
> > +{
> > + struct adm1177_state *st = iio_priv(indio_dev);
> > + u8 data[3];
> > +
> > + switch (mask) {
> > + case IIO_CHAN_INFO_RAW:
> > + adm1177_read(st, 3, data);
> > + switch (chan->type) {
> > + case IIO_VOLTAGE:
> > + *val = (data[0] << 4) | (data[2] >> 4);
> > + return IIO_VAL_INT;
> > + case IIO_CURRENT:
> > + *val = (data[1] << 4) | (data[2] & 0xF);
> > + return IIO_VAL_INT;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + case IIO_CHAN_INFO_SCALE:
> > + switch (chan->type) {
> > + case IIO_VOLTAGE:
> > + if (st->command & ADM1177_CMD_VRANGE)
> > + *val = 6650;
> > + else
> > + *val = 26350;
> > +
> > + *val2 = ADM1177_BITS;
> > + return IIO_VAL_FRACTIONAL_LOG2;
> > + case IIO_CURRENT:
> > + *val = 105840 / st->r_sense_uohm;
> > +
> > + *val2 = ADM1177_BITS;
> > + return IIO_VAL_FRACTIONAL_LOG2;
> > + default:
> > + return -EINVAL;
> > + }
> > + default:
> > + return -EINVAL;
> > + }
> > +}
> > +
> > +static const struct iio_chan_spec adm1177_channels[] = {
> > + {
> > + .type = IIO_VOLTAGE,
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > + BIT(IIO_CHAN_INFO_SCALE),
> > + .indexed = 1,
> > + .channel = 0,
> > + },
> > + {
> > + .type = IIO_CURRENT,
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > + BIT(IIO_CHAN_INFO_SCALE),
> > + .indexed = 1,
> > + .channel = 0,
> > + },
> > +};
> > +
> > +static const struct iio_info adm1177_info = {
> > + .read_raw = &adm1177_read_raw,
> > +};
>
> blank line here.
>
> > +static void adm1177_remove(void *data)
> > +{
> > + struct adm1177_state *st = data;
> > +
> > + if (st->reg)
> > + regulator_disable(st->reg);
>
> Probe and remove should be mirror images of each other.
> As you have a mixture of managed and non managed calls that
> isn't the case here.
>
> devm_add_action_or_reset will sort this out for you if
> used to turn off the regulator at the right point on removal.
>

On top of that, this function is added with devm_add_action_or_reset(),
and it is only ever called if st->reg is not NULL.

> > +}
> > +
> > +static int adm1177_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + int ret;
> > + struct device_node *np;
> > + struct adm1177_state *st;
> > + struct iio_dev *indio_dev;
> > +
> > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*st));
> > + if (!indio_dev)
> > + return -ENOMEM;
> > +
> > + st = iio_priv(indio_dev);
> > + i2c_set_clientdata(client, indio_dev);
> > + st->client = client;
> > +
> > + np = client->dev.of_node;
> > +
> > + 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;
> > + }
> > +
> > + of_property_read_u32(np, "adi,r-sense-micro-ohms", &st->r_sense_uohm);

shunt-resistor-micro-ohms

> > + of_property_read_u32(np, "adi,shutdown-threshold-microamp",
> > + &st->alert_threshold_ua);
> > + st->vrange_high = of_property_read_bool(np,
> > + "adi,vrange-high-enable");
> > + if (st->alert_threshold_ua) {
> > + unsigned int val;
> > +
> > + val = (st->alert_threshold_ua * st->r_sense_uohm * 0xFF);
> > + val /= 105840000U;
> > + if (val > 0xFF) {
> > + dev_err(&client->dev,
> > + "Requested shutdown current %d uA above limit\n",
> > + st->alert_threshold_ua);
> > +
> > + val = 0xFF;
> > + }
> > + adm1177_write_reg(st, ADM1177_REG_ALERT_TH, val);
> > + }
> > +
> > + adm1177_write_cmd(st, ADM1177_CMD_V_CONT |
> > + ADM1177_CMD_I_CONT |
> > + (st->vrange_high ? 0 : ADM1177_CMD_VRANGE));
> > +
> > + indio_dev->name = id->name;
> > + indio_dev->channels = adm1177_channels;
> > + indio_dev->num_channels = ARRAY_SIZE(adm1177_channels);
> > +
> > + indio_dev->dev.parent = &client->dev;
> > + indio_dev->info = &adm1177_info;
> > + indio_dev->modes = INDIO_DIRECT_MODE;
> > +
> > + return devm_iio_device_register(&client->dev, indio_dev);
> > +}
> > +
> > +static const struct i2c_device_id adm1177_ids[] = {
> > + { "adm1177", 0 },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(i2c, adm1177_ids);
> > +
> > +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 = {
> > + .driver = {
> > + .name = "adm1177",
> > + .of_match_table = adm1177_dt_ids,
> > + },
> > + .probe = adm1177_probe,
> > + .id_table = adm1177_ids,
> > +};
> > +module_i2c_driver(adm1177_driver);
> > +
> > +MODULE_AUTHOR("Michael Hennerich <[email protected]>");
> > +MODULE_DESCRIPTION("Analog Devices ADM1177 ADC driver");
> > +MODULE_LICENSE("GPL v2");
>
>

2019-11-13 08:14:17

by Hennerich, Michael

[permalink] [raw]
Subject: RE: [PATCH 1/3] iio: Add ADM1177 Hot Swap Controller and Digital Power Monitor driver



> -----Original Message-----
> From: Guenter Roeck <[email protected]> On Behalf Of Guenter Roeck
> Sent: Dienstag, 12. November 2019 20:18
> To: Jonathan Cameron <[email protected]>
> Cc: Bia, Beniamin <[email protected]>; [email protected];
> [email protected]; Hennerich, Michael <[email protected]>;
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; Jean Delvare
> <[email protected]>
> Subject: Re: [PATCH 1/3] iio: Add ADM1177 Hot Swap Controller and Digital
> Power Monitor driver
>
> On Tue, Nov 12, 2019 at 05:37:57PM +0000, Jonathan Cameron wrote:
> > On Tue, 12 Nov 2019 17:35:50 +0200
> > Beniamin Bia <[email protected]> wrote:
> >
> > > From: Michael Hennerich <[email protected]>
> > >
> > > 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: Michael Hennerich <[email protected]>
> > > Co-developed-by: Beniamin Bia <[email protected]>
> > > Signed-off-by: Beniamin Bia <[email protected]>
> >
> > Hi Beniamin,
> >
> > A couple immediate thoughts.
> >
> > 1. That cc list has some rather non obvious people on it. Unless something
> > fairly surprising is going on, probably better to cut it back a bit.
> >
> > 2. Why IIO? Not entirely obvious to me. From first glance this is definitely
> > doing hardware monitoring. If there is a reason there should be a clear
> > statement here on why.
> >
>
> I don't see why this is implemented as iio driver. I think it should be
> implemented as hardware monitoring driver.

Totally agree that this driver could have been implemented as HWMON driver.
Well we use this device as USB supply monitor on our embedded portably kits, to detect low VBUS or excess current draw.

ADALM-PLUTO and ADALM2000:
https://www.analog.com/en/design-center/evaluation-hardware-and-software/evaluation-boards-kits/adalm-pluto.html

https://www.analog.com/en/design-center/evaluation-hardware-and-software/evaluation-boards-kits/ADALM2000.html

The only connectivity to the host PC is via IIO/libiio USB Gadget FS and Ethernet backends.

We recommend people to read the IIO attributes whenever they report an issue.
Unless libiio supports directly HWMON or HWMON adds an IIO bridge we would prefer this driver being exposed as IIO device, since HWMON users still can use the IIO/HWMON bridge.

-Michael

2019-11-13 22:40:41

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH 1/3] iio: Add ADM1177 Hot Swap Controller and Digital Power Monitor driver

On Wed, Nov 13, 2019 at 08:10:50AM +0000, Hennerich, Michael wrote:
>
>
> > -----Original Message-----
> > From: Guenter Roeck <[email protected]> On Behalf Of Guenter Roeck
> > Sent: Dienstag, 12. November 2019 20:18
> > To: Jonathan Cameron <[email protected]>
> > Cc: Bia, Beniamin <[email protected]>; [email protected];
> > [email protected]; Hennerich, Michael <[email protected]>;
> > [email protected]; [email protected]; linux-
> > [email protected]; [email protected]; linux-
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected]; Jean Delvare
> > <[email protected]>
> > Subject: Re: [PATCH 1/3] iio: Add ADM1177 Hot Swap Controller and Digital
> > Power Monitor driver
> >
> > On Tue, Nov 12, 2019 at 05:37:57PM +0000, Jonathan Cameron wrote:
> > > On Tue, 12 Nov 2019 17:35:50 +0200
> > > Beniamin Bia <[email protected]> wrote:
> > >
> > > > From: Michael Hennerich <[email protected]>
> > > >
> > > > 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: Michael Hennerich <[email protected]>
> > > > Co-developed-by: Beniamin Bia <[email protected]>
> > > > Signed-off-by: Beniamin Bia <[email protected]>
> > >
> > > Hi Beniamin,
> > >
> > > A couple immediate thoughts.
> > >
> > > 1. That cc list has some rather non obvious people on it. Unless something
> > > fairly surprising is going on, probably better to cut it back a bit.
> > >
> > > 2. Why IIO? Not entirely obvious to me. From first glance this is definitely
> > > doing hardware monitoring. If there is a reason there should be a clear
> > > statement here on why.
> > >
> >
> > I don't see why this is implemented as iio driver. I think it should be
> > implemented as hardware monitoring driver.
>
> Totally agree that this driver could have been implemented as HWMON driver.
> Well we use this device as USB supply monitor on our embedded portably kits, to detect low VBUS or excess current draw.
>
> ADALM-PLUTO and ADALM2000:
> https://www.analog.com/en/design-center/evaluation-hardware-and-software/evaluation-boards-kits/adalm-pluto.html
>
> https://www.analog.com/en/design-center/evaluation-hardware-and-software/evaluation-boards-kits/ADALM2000.html
>
> The only connectivity to the host PC is via IIO/libiio USB Gadget FS and Ethernet backends.
>
> We recommend people to read the IIO attributes whenever they report an issue.
> Unless libiio supports directly HWMON or HWMON adds an IIO bridge we would prefer this driver being exposed as IIO device, since HWMON users still can use the IIO/HWMON bridge.
>

I do not think this is a valid argument.

- This is a hardware monitoring chip.
- Implementing kernel support as IIO driver, keeping it out of tree for years,
establishing an iio based use case, and then pressuring kernel maintainers
to accept an iio driver seems inappropriate.
- The argument of "we need libiio support for this chip" could effectively
be used to re-implement pretty much all hwmon drivers as iio drivers.
- The iio->hwmon bridge would add complexity for the majority of potential
users of this chip. Focus should be on the majority, not on one use case.
- Userspace may as well use libsensors and/or sensors to do the necessary
access - or implement it if neded. Or add a libsensors based backend to
libiio (or to iiod).
- Last but not least, it would be more appropriate to implement a generic
hwmon->iio bridge for iio use cases for chips supported by hwmon drivers,
similar to the hwmon->thermal bridge.

Guenter

2019-11-18 17:51:43

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 2/3] dt-binding: iio: Add documentation for ADM1177

On Tue, Nov 12, 2019 at 05:35:51PM +0200, Beniamin Bia wrote:
> Documentation for ADM1177 was added.
>
> Signed-off-by: Beniamin Bia <[email protected]>
> ---
> .../bindings/iio/adc/adi,adm1177.yaml | 60 +++++++++++++++++++
> 1 file changed, 60 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,adm1177.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,adm1177.yaml b/Documentation/devicetree/bindings/iio/adc/adi,adm1177.yaml
> new file mode 100644
> index 000000000000..69a0230e59f5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,adm1177.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/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.

s/curent/current/

Is there a range of values allowed?

> +
> + adi,shutdown-threshold-microamp:
> + description:
> + Specifies the current level at which an over current alert occurs.
> + maximum: 255
> +
> + 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
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> + i2c0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@b4 {
> + compatible = "adi,adm1177";
> + reg = <0xb4>;
> + };
> + };
> +...
> --
> 2.17.1
>