[PATCH v2 0/2] hwmon: (adt7475) Added attenuator bypass support

[PATCH v2 0/2] hwmon: (adt7475) Added attenuator bypass support

[Logan Shaw]

The ADT7476 and ADT7490 chips support bypassing voltage input
attenuators on voltage inputs 0, 1, 3, and 4. This can be useful
to improve measurement resolution when measuring voltages 0 V - 2.25 V.

These attenuators can be bypassed in two ways. First, bit 5 of register
0x73, if set, will bypass attenuators on all above mentioned voltage
inputs. Secondly, bits [7:4] control individually bypassing attenuators
on the above mentioned voltage inputs.

This patch adds 5 optional devicetree properties to the adt7475
driver, which if present in the devicetree and the device is one of
{ADT7476, ADT7490} will set the appropriate bits to bypass the attenuator
on the relevant voltage input.

* v2
- removed sysfs changes from patch
- removed adt7475_write macro from patch and replaced it by using
	the i2c_smbus_write_byte_data function directly in code.
- removed config4_attenuate_index function from patch and replaced it
	by modifying the function  load_individual_bypass_attenuators
	to use hard coded bit values.
- modified function load_individual_bypass_attenuators to use 4 if
	statements, one for each voltage input, replacing the for loop.
- modified function adt7475_probe to check the device is a ADT7476 or
	ADT7490 (other devices do not support bypassing all or
	individual attenuators), and only then set the relevant bits.
- added new file adt7475.txt to document the new devicetree properties.
- removed c++ style comments. 

Logan Shaw (2):
  hwmon: (adt7475) Added attenuator bypass support
  hwmon: (adt7475) Added attenuator bypass support

 .../devicetree/bindings/hwmon/adt7475.txt     | 35 +++++++++
 drivers/hwmon/adt7475.c                       | 73 +++++++++++++++++++
 2 files changed, 108 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/adt7475.txt

-- 
2.23.0

[PATCH v2 1/2] hwmon: (adt7475) Added attenuator bypass support

[Logan Shaw]

Added support for reading DTS properties to set attenuators on
device probe. Only bypasses attenuators on the ADT7476 and ADT7490
(other chips do not support this).

Signed-off-by: Logan Shaw <logan.shaw@alliedtelesis.co.nz>
---
---
 drivers/hwmon/adt7475.c | 73 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index 6c64d50c9aae..1e6ca1bc28ce 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -19,6 +19,7 @@
 #include <linux/hwmon-vid.h>
 #include <linux/err.h>
 #include <linux/jiffies.h>
+#include <linux/of.h>
 #include <linux/util_macros.h>
 
 /* Indexes for the sysfs hooks */
@@ -1457,6 +1458,68 @@ static int adt7475_update_limits(struct i2c_client *client)
 	return 0;
 }
 
+/**
+ * Reads individual voltage input bypass attenuator properties from the DTS,
+ * and if the property is present the corresponding bit is set in the
+ * register. Only the ADT7476 and ADT7490 support bypassing individual
+ * attenuators.
+ *
+ * Properties must be in the form of "bypass-attenuator-inx", where x is an
+ * integer from the set {0, 1, 3, 4} (can not bypass in2 attenuator).
+ *
+ * Returns a negative error code if there was an error writing to the register.
+ */
+static int load_individual_bypass_attenuators(const struct i2c_client *client,
+					      u8 *config4)
+{
+	u8 config4_copy = *config4;
+
+	if (of_get_property(client->dev.of_node, "bypass-attenuator-in0", NULL))
+		config4_copy |= (1 << 4);
+
+	if (of_get_property(client->dev.of_node, "bypass-attenuator-in1", NULL))
+		config4_copy |= (1 << 5);
+
+	if (of_get_property(client->dev.of_node, "bypass-attenuator-in3", NULL))
+		config4_copy |= (1 << 6);
+
+	if (of_get_property(client->dev.of_node, "bypass-attenuator-in4", NULL))
+		config4_copy |= (1 << 7);
+
+	if (i2c_smbus_write_byte_data(client, REG_CONFIG4, config4_copy) < 0)
+		return -EREMOTEIO;
+
+	*config4 = config4_copy;
+
+	return 0;
+}
+
+/**
+ * Sets the bypass all attenuators bit, if the "bypass-attenuator-all"
+ * property exists in the DTS.
+ *
+ * Returns a negative error code if there was an error writing to the
+ * register.
+ */
+static int load_all_bypass_attenuator(const struct i2c_client *client,
+				      u8 *config2)
+{
+	u8 config2_copy = *config2;
+
+	if (!of_get_property(client->dev.of_node,
+			     "bypass-attenuator-all", NULL))
+		return 0;
+
+	config2_copy |= (1 << 5);
+
+	if (i2c_smbus_write_byte_data(client, REG_CONFIG2, config2_copy) < 0)
+		return -EREMOTEIO;
+
+	*config2 = config2_copy;
+
+	return 0;
+}
+
 static int adt7475_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
@@ -1545,7 +1608,17 @@ static int adt7475_probe(struct i2c_client *client,
 	}
 
 	/* Voltage attenuators can be bypassed, globally or individually */
+	if (chip == adt7476 || chip == adt7490)
+		if (load_individual_bypass_attenuators(client,
+							&(data->config4)) < 0)
+			dev_warn(&client->dev,
+				 "Error setting bypass attenuator bits\n");
+
 	config2 = adt7475_read(REG_CONFIG2);
+	if (chip == adt7476 || chip == adt7490)
+		if (load_all_bypass_attenuator(client, &config2) < 0)
+			dev_warn(&client->dev, "Error setting bypass all attenuator\n");
+
 	if (config2 & CONFIG2_ATTN) {
 		data->bypass_attn = (0x3 << 3) | 0x3;
 	} else {
-- 
2.23.0

[PATCH v2 2/2] hwmon: (adt7475) Added attenuator bypass support

[Logan Shaw]

Added a new file documenting the adt7475 devicetree and added the five
new properties to it.

Signed-off-by: Logan Shaw <logan.shaw@alliedtelesis.co.nz>
---
---
 .../devicetree/bindings/hwmon/adt7475.txt     | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/adt7475.txt

diff --git a/Documentation/devicetree/bindings/hwmon/adt7475.txt b/Documentation/devicetree/bindings/hwmon/adt7475.txt
new file mode 100644
index 000000000000..388dd718a246
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/adt7475.txt
@@ -0,0 +1,35 @@
+*ADT7475 hwmon sensor.
+
+Required properties:
+- compatible: One of
+	"adi,adt7473"
+	"adi,adt7475"
+	"adi,adt7476"
+	"adi,adt7490"
+
+- reg: I2C address
+
+optional properties:
+
+- bypass-attenuator-all: Configures bypassing all voltage input attenuators.
+	This is only supported on the ADT7476 and ADT7490, this property does
+	nothing on other chips.
+		property present: Bit set to bypass all voltage input attenuators.
+		property absent: Registers left unchanged.
+
+- bypass-attenuator-inx: Configures bypassing individual voltage input
+	attenuators, where x is an integer from the set {0, 1, 3, 4}. This
+	is only supported on the ADT7476 and ADT7490, this property does nothing
+	on other chips.
+		property present: Bit set to bypass specific voltage input attenuator
+			for voltage input x.
+		property absent: Registers left unchanged.
+
+Example:
+
+hwmon@2e {
+	compatible = "adi,adt7475";
+	reg = <0x2e>;
+	bypass-attenuator-all;
+	bypass-attenuator-in1;
+};
\ No newline at end of file
-- 
2.23.0

Re: [PATCH v2 2/2] hwmon: (adt7475) Added attenuator bypass support

[Guenter Roeck]

On 12/18/19 7:32 PM, Logan Shaw wrote:
> Added a new file documenting the adt7475 devicetree and added the five
> new properties to it.
> 
> Signed-off-by: Logan Shaw <logan.shaw@alliedtelesis.co.nz>
> ---
> ---
>   .../devicetree/bindings/hwmon/adt7475.txt     | 35 +++++++++++++++++++
>   1 file changed, 35 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/hwmon/adt7475.txt
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/adt7475.txt b/Documentation/devicetree/bindings/hwmon/adt7475.txt
> new file mode 100644
> index 000000000000..388dd718a246
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/adt7475.txt
> @@ -0,0 +1,35 @@
> +*ADT7475 hwmon sensor.
> +
> +Required properties:
> +- compatible: One of
> +	"adi,adt7473"
> +	"adi,adt7475"
> +	"adi,adt7476"
> +	"adi,adt7490"
> +
> +- reg: I2C address
> +
> +optional properties:
> +
> +- bypass-attenuator-all: Configures bypassing all voltage input attenuators.
> +	This is only supported on the ADT7476 and ADT7490, this property does
> +	nothing on other chips.

Both adt7473 and adt7475 do support configuring an attenuator on VCCP

> +		property present: Bit set to bypass all voltage input attenuators.
> +		property absent: Registers left unchanged.
> +
> +- bypass-attenuator-inx: Configures bypassing individual voltage input
> +	attenuators, where x is an integer from the set {0, 1, 3, 4}. This
> +	is only supported on the ADT7476 and ADT7490, this property does nothing
> +	on other chips.
> +		property present: Bit set to bypass specific voltage input attenuator
> +			for voltage input x.
> +		property absent: Registers left unchanged.
> +
This is interesting. It essentially means "retain configuration from ROM
Monitor", but leaves no means to _disable_ the bypass.

> +Example:
> +
> +hwmon@2e {
> +	compatible = "adi,adt7475";
> +	reg = <0x2e>;
> +	bypass-attenuator-all;
> +	bypass-attenuator-in1;

What would be the purpose of specifying both all and in1 ?

> +};
> \ No newline at end of file
> 

Re: [PATCH v2 2/2] hwmon: (adt7475) Added attenuator bypass support

[Logan Shaw]

On Wed, 2019-12-18 at 19:53 -0800, Guenter Roeck wrote:
> On 12/18/19 7:32 PM, Logan Shaw wrote:
> > Added a new file documenting the adt7475 devicetree and added the
> > five
> > new properties to it.
> > 
> > Signed-off-by: Logan Shaw <logan.shaw@alliedtelesis.co.nz>
> > ---
> > ---
> >   .../devicetree/bindings/hwmon/adt7475.txt     | 35
> > +++++++++++++++++++
> >   1 file changed, 35 insertions(+)
> >   create mode 100644
> > Documentation/devicetree/bindings/hwmon/adt7475.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/hwmon/adt7475.txt
> > b/Documentation/devicetree/bindings/hwmon/adt7475.txt
> > new file mode 100644
> > index 000000000000..388dd718a246
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/adt7475.txt
> > @@ -0,0 +1,35 @@
> > +*ADT7475 hwmon sensor.
> > +
> > +Required properties:
> > +- compatible: One of
> > +	"adi,adt7473"
> > +	"adi,adt7475"
> > +	"adi,adt7476"
> > +	"adi,adt7490"
> > +
> > +- reg: I2C address
> > +
> > +optional properties:
> > +
> > +- bypass-attenuator-all: Configures bypassing all voltage input
> > attenuators.
> > +	This is only supported on the ADT7476 and ADT7490, this
> > property does
> > +	nothing on other chips.
> 
> Both adt7473 and adt7475 do support configuring an attenuator on VCCP
> 

That is true, but as the function of register 0x73 bit 5 differs
between the two set of hardware ({adt7473, adt7475} and {adt7476,
adt7490}) a solution which allows bypassing VCCP on both gets more
complicated which I was trying to avoid.

Is it acceptable to split the function
load_individual_bypass_attenuators into two, one for each set of
chips, and call the appropriate function for the chip? That way adding
"bypass-attenuator-in1" to any of the four adt chips DTS will result in
the attenuator for VCCP being bypassed (albeit by setting different
bits depending on the specific bit).

> > +		property present: Bit set to bypass all voltage input
> > attenuators.
> > +		property absent: Registers left unchanged.
> > +
> > +- bypass-attenuator-inx: Configures bypassing individual voltage
> > input
> > +	attenuators, where x is an integer from the set {0, 1, 3, 4}.
> > This
> > +	is only supported on the ADT7476 and ADT7490, this property
> > does nothing
> > +	on other chips.
> > +		property present: Bit set to bypass specific voltage
> > input attenuator
> > +			for voltage input x.
> > +		property absent: Registers left unchanged.
> > +
> 
> This is interesting. It essentially means "retain configuration from
> ROM
> Monitor", but leaves no means to _disable_ the bypass.
> 

Only a power cycle (after removing the properties from the DTS) will
set the affected bits back to 0. Removing the properties, but only
softly restarting the system (no interrupted power supply to the adt
chip), will not set the bits back to 0.

I decided against setting the bits to 0 by default (if no property was
present) as doing so might break compatibility with systems that expect
the bits to remain unchanged. 

A compromise would be to change the "bypass-attenuator-inx" property to
"bypass-attenuator-inx = <y>", where y = 1 bypasses the attenuator and
y = 0 does not. If the property is not present then the register is
left unchanged. It would make sense to do the same to the "bypass-
attenuator-all" property. Would these changes be acceptable?

> > +Example:
> > +
> > +hwmon@2e {
> > +	compatible = "adi,adt7475";
> > +	reg = <0x2e>;
> > +	bypass-attenuator-all;
> > +	bypass-attenuator-in1;
> 
> What would be the purpose of specifying both all and in1 ?

There would be no practical purpose, it was to keep the example
compact. Instead "bypass-attenuator-in1" could be removed and added to 
second device hwmon@2d. This would show off the syntax for each set of
properties in a more practical way.

> 
> > +};
> > \ No newline at end of file
> > 
> 
> 

Re: [PATCH v2 2/2] hwmon: (adt7475) Added attenuator bypass support

[Logan Shaw]

A gentle reminder that this patch exists, as it has (understandably) gone silent over the holiday period. I would appreciate some feedback on my ideas before modifying the patch, so I know I am headed in the right direction.

On Wed, 2019-12-18 at 19:53 -0800, Guenter Roeck wrote:
> On 12/18/19 7:32 PM, Logan Shaw wrote:
> > Added a new file documenting the adt7475 devicetree and added the
> > five
> > new properties to it.
> >
> > Signed-off-by: Logan Shaw <logan.shaw@alliedtelesis.co.nz>
> > ---
> > ---
> >   .../devicetree/bindings/hwmon/adt7475.txt     | 35
> > +++++++++++++++++++
> >   1 file changed, 35 insertions(+)
> >   create mode 100644
> > Documentation/devicetree/bindings/hwmon/adt7475.txt
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/adt7475.txt
> > b/Documentation/devicetree/bindings/hwmon/adt7475.txt
> > new file mode 100644
> > index 000000000000..388dd718a246
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/adt7475.txt
> > @@ -0,0 +1,35 @@
> > +*ADT7475 hwmon sensor.
> > +
> > +Required properties:
> > +- compatible: One of
> > +   "adi,adt7473"
> > +   "adi,adt7475"
> > +   "adi,adt7476"
> > +   "adi,adt7490"
> > +
> > +- reg: I2C address
> > +
> > +optional properties:
> > +
> > +- bypass-attenuator-all: Configures bypassing all voltage input
> > attenuators.
> > +   This is only supported on the ADT7476 and ADT7490, this
> > property does
> > +   nothing on other chips.
>
> Both adt7473 and adt7475 do support configuring an attenuator on VCCP
>

That is true, but as the function of register 0x73 bit 5 differs
between the two set of hardware ({adt7473, adt7475} and {adt7476,
adt7490}) a solution which allows bypassing VCCP on both gets more
complicated which I was trying to avoid.

Is it acceptable to split the function
load_individual_bypass_attenuators into two, one for each set of
chips, and call the appropriate function for the chip? That way adding
"bypass-attenuator-in1" to any of the four adt chips DTS will result in
the attenuator for VCCP being bypassed (albeit by setting different
bits depending on the specific bit).

> > +           property present: Bit set to bypass all voltage input
> > attenuators.
> > +           property absent: Registers left unchanged.
> > +
> > +- bypass-attenuator-inx: Configures bypassing individual voltage
> > input
> > +   attenuators, where x is an integer from the set {0, 1, 3, 4}.
> > This
> > +   is only supported on the ADT7476 and ADT7490, this property
> > does nothing
> > +   on other chips.
> > +           property present: Bit set to bypass specific voltage
> > input attenuator
> > +                   for voltage input x.
> > +           property absent: Registers left unchanged.
> > +
>
> This is interesting. It essentially means "retain configuration from
> ROM
> Monitor", but leaves no means to _disable_ the bypass.
>

Only a power cycle (after removing the properties from the DTS) will
set the affected bits back to 0. Removing the properties, but only
softly restarting the system (no interrupted power supply to the adt
chip), will not set the bits back to 0.

I decided against setting the bits to 0 by default (if no property was
present) as doing so might break compatibility with systems that expect
the bits to remain unchanged.

A compromise would be to change the "bypass-attenuator-inx" property to
"bypass-attenuator-inx = <y>", where y = 1 bypasses the attenuator and
y = 0 does not. If the property is not present then the register is
left unchanged. It would make sense to do the same to the "bypass-
attenuator-all" property. Would these changes be acceptable?

> > +Example:
> > +
> > +hwmon@2e {
> > +   compatible = "adi,adt7475";
> > +   reg = <0x2e>;
> > +   bypass-attenuator-all;
> > +   bypass-attenuator-in1;
>
> What would be the purpose of specifying both all and in1 ?

There would be no practical purpose, it was to keep the example
compact. Instead "bypass-attenuator-in1" could be removed and added to
second device hwmon@2d. This would show off the syntax for each set of
properties in a more practical way.

>
> > +};
> > \ No newline at end of file
> >
>
>

Re: [PATCH v2 2/2] hwmon: (adt7475) Added attenuator bypass support

[Guenter Roeck]

On Fri, Dec 27, 2019 at 02:53:16AM +0000, Logan Shaw wrote:
> On Wed, 2019-12-18 at 19:53 -0800, Guenter Roeck wrote:
> > On 12/18/19 7:32 PM, Logan Shaw wrote:
> > > Added a new file documenting the adt7475 devicetree and added the
> > > five
> > > new properties to it.
> > > 
> > > Signed-off-by: Logan Shaw <logan.shaw@alliedtelesis.co.nz>
> > > ---
> > > ---
> > >   .../devicetree/bindings/hwmon/adt7475.txt     | 35
> > > +++++++++++++++++++
> > >   1 file changed, 35 insertions(+)
> > >   create mode 100644
> > > Documentation/devicetree/bindings/hwmon/adt7475.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/hwmon/adt7475.txt
> > > b/Documentation/devicetree/bindings/hwmon/adt7475.txt
> > > new file mode 100644
> > > index 000000000000..388dd718a246
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/hwmon/adt7475.txt
> > > @@ -0,0 +1,35 @@
> > > +*ADT7475 hwmon sensor.
> > > +
> > > +Required properties:
> > > +- compatible: One of
> > > +	"adi,adt7473"
> > > +	"adi,adt7475"
> > > +	"adi,adt7476"
> > > +	"adi,adt7490"
> > > +
> > > +- reg: I2C address
> > > +
> > > +optional properties:
> > > +
> > > +- bypass-attenuator-all: Configures bypassing all voltage input
> > > attenuators.
> > > +	This is only supported on the ADT7476 and ADT7490, this
> > > property does
> > > +	nothing on other chips.
> > 
> > Both adt7473 and adt7475 do support configuring an attenuator on VCCP
> > 
> 
> That is true, but as the function of register 0x73 bit 5 differs
> between the two set of hardware ({adt7473, adt7475} and {adt7476,
> adt7490}) a solution which allows bypassing VCCP on both gets more
> complicated which I was trying to avoid.
> 
> Is it acceptable to split the function
> load_individual_bypass_attenuators into two, one for each set of
> chips, and call the appropriate function for the chip? That way adding
> "bypass-attenuator-in1" to any of the four adt chips DTS will result in
> the attenuator for VCCP being bypassed (albeit by setting different
> bits depending on the specific bit).
> 
You could have per-chip functions, or per-chip mask data in the local data
structure.

> > > +		property present: Bit set to bypass all voltage input
> > > attenuators.
> > > +		property absent: Registers left unchanged.
> > > +
> > > +- bypass-attenuator-inx: Configures bypassing individual voltage
> > > input
> > > +	attenuators, where x is an integer from the set {0, 1, 3, 4}.
> > > This
> > > +	is only supported on the ADT7476 and ADT7490, this property
> > > does nothing
> > > +	on other chips.
> > > +		property present: Bit set to bypass specific voltage
> > > input attenuator
> > > +			for voltage input x.
> > > +		property absent: Registers left unchanged.
> > > +
> > 
> > This is interesting. It essentially means "retain configuration from
> > ROM
> > Monitor", but leaves no means to _disable_ the bypass.
> > 
> 
> Only a power cycle (after removing the properties from the DTS) will
> set the affected bits back to 0. Removing the properties, but only
> softly restarting the system (no interrupted power supply to the adt
> chip), will not set the bits back to 0.
> 
> I decided against setting the bits to 0 by default (if no property was
> present) as doing so might break compatibility with systems that expect
> the bits to remain unchanged. 
> 
> A compromise would be to change the "bypass-attenuator-inx" property to
> "bypass-attenuator-inx = <y>", where y = 1 bypasses the attenuator and
> y = 0 does not. If the property is not present then the register is
> left unchanged. It would make sense to do the same to the "bypass-
> attenuator-all" property. Would these changes be acceptable?
> 
That goes into devicetree object details. Rob might have an opinion
on that.

> > > +Example:
> > > +
> > > +hwmon@2e {
> > > +	compatible = "adi,adt7475";
> > > +	reg = <0x2e>;
> > > +	bypass-attenuator-all;
> > > +	bypass-attenuator-in1;
> > 
> > What would be the purpose of specifying both all and in1 ?
> 
> There would be no practical purpose, it was to keep the example
> compact. Instead "bypass-attenuator-in1" could be removed and added to 
> second device hwmon@2d. This would show off the syntax for each set of
> properties in a more practical way.
> 
That would make more sense.

> > 
> > > +};
> > > \ No newline at end of file

Note that you might want to fix that.

Guenter

Re: [PATCH v2 2/2] hwmon: (adt7475) Added attenuator bypass support

[Chris Packham]

(CC Rob and devicetree)

On Wed, 2019-12-18 at 19:53 -0800, Guenter Roeck wrote:
> On 12/18/19 7:32 PM, Logan Shaw wrote:
> > Added a new file documenting the adt7475 devicetree and added the five
> > new properties to it.
> > 
> > Signed-off-by: Logan Shaw <logan.shaw@alliedtelesis.co.nz>
> > ---
> > ---
> >   .../devicetree/bindings/hwmon/adt7475.txt     | 35 +++++++++++++++++++
> >   1 file changed, 35 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/hwmon/adt7475.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/hwmon/adt7475.txt b/Documentation/devicetree/bindings/hwmon/adt7475.txt
> > new file mode 100644
> > index 000000000000..388dd718a246
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/adt7475.txt

There's an effort underway to convert the device tree bindings to yaml
so it'd be better for new bindings to start off that way. It should be
relatively straight forward, there are a couple of existing examples
in  Documentation/devicetree/bindings/hwmon/ to follow.

Also there is an existing entry in
Documentation/devicetree/bindings/trivial-devices.yaml for the adt7475
and friends that should be removed as part of the patch that adds the
new binding.

> > @@ -0,0 +1,35 @@
> > +*ADT7475 hwmon sensor.
> > +
> > +Required properties:
> > +- compatible: One of
> > +	"adi,adt7473"
> > +	"adi,adt7475"
> > +	"adi,adt7476"
> > +	"adi,adt7490"
> > +
> > +- reg: I2C address
> > +
> > +optional properties:
> > +
> > +- bypass-attenuator-all: Configures bypassing all voltage input attenuators.
> > +	This is only supported on the ADT7476 and ADT7490, this property does
> > +	nothing on other chips.

I don't know that there's any point in supporting bypass-attenuator-all 
even though the adt7475 can support it configuring per VIN seems more
useful.

> 
> Both adt7473 and adt7475 do support configuring an attenuator on VCCP
> 
> > +		property present: Bit set to bypass all voltage input attenuators.
> > +		property absent: Registers left unchanged.
> > +
> > +- bypass-attenuator-inx: Configures bypassing individual voltage input
> > +	attenuators, where x is an integer from the set {0, 1, 3, 4}. This
> > +	is only supported on the ADT7476 and ADT7490, this property does nothing
> > +	on other chips.
> > +		property present: Bit set to bypass specific voltage input attenuator
> > +			for voltage input x.
> > +		property absent: Registers left unchanged.
> > +
> 
> This is interesting. It essentially means "retain configuration from ROM
> Monitor", but leaves no means to _disable_ the bypass.
> 

For our systems Linux is generally the ROM monitor, at least as far as
the hwmon devices are concerned. Overriding the HW default makes sense
for that case.

Do we want the ability to override the configuration from the ROM? It
should be easily doable by using an integer property instead of a
boolean.

> > +Example:
> > +
> > +hwmon@2e {
> > +	compatible = "adi,adt7475";
> > +	reg = <0x2e>;
> > +	bypass-attenuator-all;
> > +	bypass-attenuator-in1;
> 
> What would be the purpose of specifying both all and in1 ?
> 
> > +};
> > \ No newline at end of file
> > 
> 
>