[PATCH v3 0/3] hwmon: (adt7475) pin configuration

[PATCH v3 0/3] hwmon: (adt7475) pin configuration

[Chris Packham]

This series adds support for configuring the ADT7475 based on the
hardware description provided. The ADT7475 driver has always supported
detecting the configuration. This adds support for configuring the chip
based on the hardware design.

Chris Packham (3):
  dt-bindings: hwmon: Document adt7475 pin-function properties
  hwmon: (adt7475) Add support for pin configuration
  hwmon: (adt7475) Use enum chips when loading attenuator settings

 .../devicetree/bindings/hwmon/adt7475.yaml    |  22 ++++
 drivers/hwmon/adt7475.c                       | 119 ++++++++++++++++--
 2 files changed, 128 insertions(+), 13 deletions(-)

-- 
2.35.1

[PATCH v3 1/3] dt-bindings: hwmon: Document adt7475 pin-function properties

[Chris Packham]

The adt7473, adt7475, adt7476 and adt7490 have pins that can be used for
different functions. Add bindings so that it is possible to describe
what pin functions are intended by the hardware design.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
---

Notes:
    Changes in v3:
    - None
    Changes in v2:
    - Add review from Krzysztof

 .../devicetree/bindings/hwmon/adt7475.yaml    | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/adt7475.yaml b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
index 7d9c083632b9..22beb37f1bf1 100644
--- a/Documentation/devicetree/bindings/hwmon/adt7475.yaml
+++ b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
@@ -61,6 +61,26 @@ patternProperties:
     $ref: /schemas/types.yaml#/definitions/uint32
     enum: [0, 1]
 
+  "adi,pin(5|10)-function":
+    description: |
+      Configures the function for pin 5 on the adi,adt7473 and adi,adt7475. Or
+      pin 10 on the adi,adt7476 and adi,adt7490.
+    $ref: /schemas/types.yaml#/definitions/string
+    enum:
+      - pwm2
+      - smbalert#
+
+  "adi,pin(9|14)-function":
+    description: |
+      Configures the function for pin 9 on the adi,adt7473 and adi,adt7475. Or
+      pin 14 on the adi,adt7476 and adi,adt7490
+    $ref: /schemas/types.yaml#/definitions/string
+    enum:
+      - tach4
+      - therm#
+      - smbalert#
+      - gpio
+
 required:
   - compatible
   - reg
@@ -79,6 +99,8 @@ examples:
         adi,bypass-attenuator-in0 = <1>;
         adi,bypass-attenuator-in1 = <0>;
         adi,pwm-active-state = <1 0 1>;
+        adi,pin10-function = "smbalert#";
+        adi,pin14-function = "tach4";
       };
     };
 
-- 
2.35.1

Re: [PATCH v3 1/3] dt-bindings: hwmon: Document adt7475 pin-function properties

[Guenter Roeck]

On Wed, Mar 23, 2022 at 04:40:54PM +1300, Chris Packham wrote:
> The adt7473, adt7475, adt7476 and adt7490 have pins that can be used for
> different functions. Add bindings so that it is possible to describe
> what pin functions are intended by the hardware design.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Still waiting for DT maintainer approval.

Guenter

> ---
> 
> Notes:
>     Changes in v3:
>     - None
>     Changes in v2:
>     - Add review from Krzysztof
> 
>  .../devicetree/bindings/hwmon/adt7475.yaml    | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/adt7475.yaml b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
> index 7d9c083632b9..22beb37f1bf1 100644
> --- a/Documentation/devicetree/bindings/hwmon/adt7475.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
> @@ -61,6 +61,26 @@ patternProperties:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      enum: [0, 1]
>  
> +  "adi,pin(5|10)-function":
> +    description: |
> +      Configures the function for pin 5 on the adi,adt7473 and adi,adt7475. Or
> +      pin 10 on the adi,adt7476 and adi,adt7490.
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum:
> +      - pwm2
> +      - smbalert#
> +
> +  "adi,pin(9|14)-function":
> +    description: |
> +      Configures the function for pin 9 on the adi,adt7473 and adi,adt7475. Or
> +      pin 14 on the adi,adt7476 and adi,adt7490
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum:
> +      - tach4
> +      - therm#
> +      - smbalert#
> +      - gpio
> +
>  required:
>    - compatible
>    - reg
> @@ -79,6 +99,8 @@ examples:
>          adi,bypass-attenuator-in0 = <1>;
>          adi,bypass-attenuator-in1 = <0>;
>          adi,pwm-active-state = <1 0 1>;
> +        adi,pin10-function = "smbalert#";
> +        adi,pin14-function = "tach4";
>        };
>      };
>  

Re: [PATCH v3 1/3] dt-bindings: hwmon: Document adt7475 pin-function properties

[Guenter Roeck]

On Wed, Mar 23, 2022 at 04:40:54PM +1300, Chris Packham wrote:
> The adt7473, adt7475, adt7476 and adt7490 have pins that can be used for
> different functions. Add bindings so that it is possible to describe
> what pin functions are intended by the hardware design.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Applied to hwmon-next.

Thanks,
Guenter

> ---
> 
> Notes:
>     Changes in v3:
>     - None
>     Changes in v2:
>     - Add review from Krzysztof
> 
>  .../devicetree/bindings/hwmon/adt7475.yaml    | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/adt7475.yaml b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
> index 7d9c083632b9..22beb37f1bf1 100644
> --- a/Documentation/devicetree/bindings/hwmon/adt7475.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
> @@ -61,6 +61,26 @@ patternProperties:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      enum: [0, 1]
>  
> +  "adi,pin(5|10)-function":
> +    description: |
> +      Configures the function for pin 5 on the adi,adt7473 and adi,adt7475. Or
> +      pin 10 on the adi,adt7476 and adi,adt7490.
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum:
> +      - pwm2
> +      - smbalert#
> +
> +  "adi,pin(9|14)-function":
> +    description: |
> +      Configures the function for pin 9 on the adi,adt7473 and adi,adt7475. Or
> +      pin 14 on the adi,adt7476 and adi,adt7490
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum:
> +      - tach4
> +      - therm#
> +      - smbalert#
> +      - gpio
> +
>  required:
>    - compatible
>    - reg
> @@ -79,6 +99,8 @@ examples:
>          adi,bypass-attenuator-in0 = <1>;
>          adi,bypass-attenuator-in1 = <0>;
>          adi,pwm-active-state = <1 0 1>;
> +        adi,pin10-function = "smbalert#";
> +        adi,pin14-function = "tach4";
>        };
>      };
>  

[PATCH v3 2/3] hwmon: (adt7475) Add support for pin configuration

[Chris Packham]

The adt7473, adt7475, adt7476 and adt7490 have pins that can be used for
different functions. On the adt7473 and  adt7475 this is pins 5 and 9.
On the adt7476 and adt7490 this is pins 10 and 14.

The first pin can either be PWM2(default) or SMBALERT#. The second pin
can be TACH4(default), THERM#, SMBALERT# or GPIO.

The adt7475 driver has always been able to detect the configuration if
it had been done by an earlier boot stage. Add support for configuring
the pins based on the hardware description in the device tree.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v3:
    - None
    Changes in v2:
    - Use load_config{3,4} instead of load_pin{10,14}_config
    - Handle errors from adt7475_read()
    - Remove obsolete check on chip type
    - Use enum chips instead of int
    - Update error messages

 drivers/hwmon/adt7475.c | 96 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 96 insertions(+)

diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index 9d5b019651f2..6de501de41b2 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -112,6 +112,8 @@
 #define CONFIG3_THERM		0x02
 
 #define CONFIG4_PINFUNC		0x03
+#define CONFIG4_THERM		0x01
+#define CONFIG4_SMBALERT	0x02
 #define CONFIG4_MAXDUTY		0x08
 #define CONFIG4_ATTN_IN10	0x30
 #define CONFIG4_ATTN_IN43	0xC0
@@ -1460,6 +1462,96 @@ static int adt7475_update_limits(struct i2c_client *client)
 	return 0;
 }
 
+static int load_config3(const struct i2c_client *client, const char *propname)
+{
+	const char *function;
+	u8 config3;
+	int ret;
+
+	ret = of_property_read_string(client->dev.of_node, propname, &function);
+	if (!ret) {
+		ret = adt7475_read(REG_CONFIG3);
+		if (ret < 0)
+			return ret;
+
+		config3 = ret & ~CONFIG3_SMBALERT;
+		if (!strcmp("pwm2", function))
+			;
+		else if (!strcmp("smbalert#", function))
+			config3 |= CONFIG3_SMBALERT;
+		else
+			return -EINVAL;
+
+		return i2c_smbus_write_byte_data(client, REG_CONFIG3, config3);
+	}
+
+	return 0;
+}
+
+static int load_config4(const struct i2c_client *client, const char *propname)
+{
+	const char *function;
+	u8 config4;
+	int ret;
+
+	ret = of_property_read_string(client->dev.of_node, propname, &function);
+	if (!ret) {
+		ret = adt7475_read(REG_CONFIG4);
+		if (ret < 0)
+			return ret;
+
+		config4 = ret & ~CONFIG4_PINFUNC;
+
+		if (!strcmp("tach4", function))
+			;
+		else if (!strcmp("therm#", function))
+			config4 |= CONFIG4_THERM;
+		else if (!strcmp("smbalert#", function))
+			config4 |= CONFIG4_SMBALERT;
+		else if (!strcmp("gpio", function))
+			config4 |= CONFIG4_PINFUNC;
+		else
+			return -EINVAL;
+
+		return i2c_smbus_write_byte_data(client, REG_CONFIG4, config4);
+	}
+
+	return 0;
+}
+
+static int load_config(const struct i2c_client *client, enum chips chip)
+{
+	int err;
+	const char *prop1, *prop2;
+
+	switch (chip) {
+	case adt7473:
+	case adt7475:
+		prop1 = "adi,pin5-function";
+		prop2 = "adi,pin9-function";
+		break;
+	case adt7476:
+	case adt7490:
+		prop1 = "adi,pin10-function";
+		prop2 = "adi,pin14-function";
+		break;
+	}
+
+	err = load_config3(client, prop1);
+	if (err) {
+		dev_err(&client->dev, "failed to configure %s\n", prop1);
+		return err;
+	}
+
+	err = load_config4(client, prop2);
+	if (err) {
+		dev_err(&client->dev, "failed to configure %s\n", prop2);
+		return err;
+	}
+
+	return 0;
+}
+
 static int set_property_bit(const struct i2c_client *client, char *property,
 			    u8 *config, u8 bit_index)
 {
@@ -1585,6 +1677,10 @@ static int adt7475_probe(struct i2c_client *client)
 		revision = adt7475_read(REG_DEVID2) & 0x07;
 	}
 
+	ret = load_config(client, chip);
+	if (ret)
+		return ret;
+
 	config3 = adt7475_read(REG_CONFIG3);
 	/* Pin PWM2 may alternatively be used for ALERT output */
 	if (!(config3 & CONFIG3_SMBALERT))
-- 
2.35.1

Re: [PATCH v3 2/3] hwmon: (adt7475) Add support for pin configuration

[Guenter Roeck]

On Wed, Mar 23, 2022 at 04:40:55PM +1300, Chris Packham wrote:
> The adt7473, adt7475, adt7476 and adt7490 have pins that can be used for
> different functions. On the adt7473 and  adt7475 this is pins 5 and 9.
> On the adt7476 and adt7490 this is pins 10 and 14.
> 
> The first pin can either be PWM2(default) or SMBALERT#. The second pin
> can be TACH4(default), THERM#, SMBALERT# or GPIO.
> 
> The adt7475 driver has always been able to detect the configuration if
> it had been done by an earlier boot stage. Add support for configuring
> the pins based on the hardware description in the device tree.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

For my reference:

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> 
> Notes:
>     Changes in v3:
>     - None
>     Changes in v2:
>     - Use load_config{3,4} instead of load_pin{10,14}_config
>     - Handle errors from adt7475_read()
>     - Remove obsolete check on chip type
>     - Use enum chips instead of int
>     - Update error messages
> 
>  drivers/hwmon/adt7475.c | 96 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 96 insertions(+)
> 
> diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
> index 9d5b019651f2..6de501de41b2 100644
> --- a/drivers/hwmon/adt7475.c
> +++ b/drivers/hwmon/adt7475.c
> @@ -112,6 +112,8 @@
>  #define CONFIG3_THERM		0x02
>  
>  #define CONFIG4_PINFUNC		0x03
> +#define CONFIG4_THERM		0x01
> +#define CONFIG4_SMBALERT	0x02
>  #define CONFIG4_MAXDUTY		0x08
>  #define CONFIG4_ATTN_IN10	0x30
>  #define CONFIG4_ATTN_IN43	0xC0
> @@ -1460,6 +1462,96 @@ static int adt7475_update_limits(struct i2c_client *client)
>  	return 0;
>  }
>  
> +static int load_config3(const struct i2c_client *client, const char *propname)
> +{
> +	const char *function;
> +	u8 config3;
> +	int ret;
> +
> +	ret = of_property_read_string(client->dev.of_node, propname, &function);
> +	if (!ret) {
> +		ret = adt7475_read(REG_CONFIG3);
> +		if (ret < 0)
> +			return ret;
> +
> +		config3 = ret & ~CONFIG3_SMBALERT;
> +		if (!strcmp("pwm2", function))
> +			;
> +		else if (!strcmp("smbalert#", function))
> +			config3 |= CONFIG3_SMBALERT;
> +		else
> +			return -EINVAL;
> +
> +		return i2c_smbus_write_byte_data(client, REG_CONFIG3, config3);
> +	}
> +
> +	return 0;
> +}
> +
> +static int load_config4(const struct i2c_client *client, const char *propname)
> +{
> +	const char *function;
> +	u8 config4;
> +	int ret;
> +
> +	ret = of_property_read_string(client->dev.of_node, propname, &function);
> +	if (!ret) {
> +		ret = adt7475_read(REG_CONFIG4);
> +		if (ret < 0)
> +			return ret;
> +
> +		config4 = ret & ~CONFIG4_PINFUNC;
> +
> +		if (!strcmp("tach4", function))
> +			;
> +		else if (!strcmp("therm#", function))
> +			config4 |= CONFIG4_THERM;
> +		else if (!strcmp("smbalert#", function))
> +			config4 |= CONFIG4_SMBALERT;
> +		else if (!strcmp("gpio", function))
> +			config4 |= CONFIG4_PINFUNC;
> +		else
> +			return -EINVAL;
> +
> +		return i2c_smbus_write_byte_data(client, REG_CONFIG4, config4);
> +	}
> +
> +	return 0;
> +}
> +
> +static int load_config(const struct i2c_client *client, enum chips chip)
> +{
> +	int err;
> +	const char *prop1, *prop2;
> +
> +	switch (chip) {
> +	case adt7473:
> +	case adt7475:
> +		prop1 = "adi,pin5-function";
> +		prop2 = "adi,pin9-function";
> +		break;
> +	case adt7476:
> +	case adt7490:
> +		prop1 = "adi,pin10-function";
> +		prop2 = "adi,pin14-function";
> +		break;
> +	}
> +
> +	err = load_config3(client, prop1);
> +	if (err) {
> +		dev_err(&client->dev, "failed to configure %s\n", prop1);
> +		return err;
> +	}
> +
> +	err = load_config4(client, prop2);
> +	if (err) {
> +		dev_err(&client->dev, "failed to configure %s\n", prop2);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
>  static int set_property_bit(const struct i2c_client *client, char *property,
>  			    u8 *config, u8 bit_index)
>  {
> @@ -1585,6 +1677,10 @@ static int adt7475_probe(struct i2c_client *client)
>  		revision = adt7475_read(REG_DEVID2) & 0x07;
>  	}
>  
> +	ret = load_config(client, chip);
> +	if (ret)
> +		return ret;
> +
>  	config3 = adt7475_read(REG_CONFIG3);
>  	/* Pin PWM2 may alternatively be used for ALERT output */
>  	if (!(config3 & CONFIG3_SMBALERT))

Re: [PATCH v3 2/3] hwmon: (adt7475) Add support for pin configuration

[Guenter Roeck]

On Wed, Mar 23, 2022 at 04:40:55PM +1300, Chris Packham wrote:
> The adt7473, adt7475, adt7476 and adt7490 have pins that can be used for
> different functions. On the adt7473 and  adt7475 this is pins 5 and 9.
> On the adt7476 and adt7490 this is pins 10 and 14.
> 
> The first pin can either be PWM2(default) or SMBALERT#. The second pin
> can be TACH4(default), THERM#, SMBALERT# or GPIO.
> 
> The adt7475 driver has always been able to detect the configuration if
> it had been done by an earlier boot stage. Add support for configuring
> the pins based on the hardware description in the device tree.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Applied to hwmon-next.

Thanks,
Guenter

> ---
> 
> Notes:
>     Changes in v3:
>     - None
>     Changes in v2:
>     - Use load_config{3,4} instead of load_pin{10,14}_config
>     - Handle errors from adt7475_read()
>     - Remove obsolete check on chip type
>     - Use enum chips instead of int
>     - Update error messages
> 
>  drivers/hwmon/adt7475.c | 96 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 96 insertions(+)
> 
> diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
> index 9d5b019651f2..6de501de41b2 100644
> --- a/drivers/hwmon/adt7475.c
> +++ b/drivers/hwmon/adt7475.c
> @@ -112,6 +112,8 @@
>  #define CONFIG3_THERM		0x02
>  
>  #define CONFIG4_PINFUNC		0x03
> +#define CONFIG4_THERM		0x01
> +#define CONFIG4_SMBALERT	0x02
>  #define CONFIG4_MAXDUTY		0x08
>  #define CONFIG4_ATTN_IN10	0x30
>  #define CONFIG4_ATTN_IN43	0xC0
> @@ -1460,6 +1462,96 @@ static int adt7475_update_limits(struct i2c_client *client)
>  	return 0;
>  }
>  
> +static int load_config3(const struct i2c_client *client, const char *propname)
> +{
> +	const char *function;
> +	u8 config3;
> +	int ret;
> +
> +	ret = of_property_read_string(client->dev.of_node, propname, &function);
> +	if (!ret) {
> +		ret = adt7475_read(REG_CONFIG3);
> +		if (ret < 0)
> +			return ret;
> +
> +		config3 = ret & ~CONFIG3_SMBALERT;
> +		if (!strcmp("pwm2", function))
> +			;
> +		else if (!strcmp("smbalert#", function))
> +			config3 |= CONFIG3_SMBALERT;
> +		else
> +			return -EINVAL;
> +
> +		return i2c_smbus_write_byte_data(client, REG_CONFIG3, config3);
> +	}
> +
> +	return 0;
> +}
> +
> +static int load_config4(const struct i2c_client *client, const char *propname)
> +{
> +	const char *function;
> +	u8 config4;
> +	int ret;
> +
> +	ret = of_property_read_string(client->dev.of_node, propname, &function);
> +	if (!ret) {
> +		ret = adt7475_read(REG_CONFIG4);
> +		if (ret < 0)
> +			return ret;
> +
> +		config4 = ret & ~CONFIG4_PINFUNC;
> +
> +		if (!strcmp("tach4", function))
> +			;
> +		else if (!strcmp("therm#", function))
> +			config4 |= CONFIG4_THERM;
> +		else if (!strcmp("smbalert#", function))
> +			config4 |= CONFIG4_SMBALERT;
> +		else if (!strcmp("gpio", function))
> +			config4 |= CONFIG4_PINFUNC;
> +		else
> +			return -EINVAL;
> +
> +		return i2c_smbus_write_byte_data(client, REG_CONFIG4, config4);
> +	}
> +
> +	return 0;
> +}
> +
> +static int load_config(const struct i2c_client *client, enum chips chip)
> +{
> +	int err;
> +	const char *prop1, *prop2;
> +
> +	switch (chip) {
> +	case adt7473:
> +	case adt7475:
> +		prop1 = "adi,pin5-function";
> +		prop2 = "adi,pin9-function";
> +		break;
> +	case adt7476:
> +	case adt7490:
> +		prop1 = "adi,pin10-function";
> +		prop2 = "adi,pin14-function";
> +		break;
> +	}
> +
> +	err = load_config3(client, prop1);
> +	if (err) {
> +		dev_err(&client->dev, "failed to configure %s\n", prop1);
> +		return err;
> +	}
> +
> +	err = load_config4(client, prop2);
> +	if (err) {
> +		dev_err(&client->dev, "failed to configure %s\n", prop2);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
>  static int set_property_bit(const struct i2c_client *client, char *property,
>  			    u8 *config, u8 bit_index)
>  {
> @@ -1585,6 +1677,10 @@ static int adt7475_probe(struct i2c_client *client)
>  		revision = adt7475_read(REG_DEVID2) & 0x07;
>  	}
>  
> +	ret = load_config(client, chip);
> +	if (ret)
> +		return ret;
> +
>  	config3 = adt7475_read(REG_CONFIG3);
>  	/* Pin PWM2 may alternatively be used for ALERT output */
>  	if (!(config3 & CONFIG3_SMBALERT))

[PATCH v3 3/3] hwmon: (adt7475) Use enum chips when loading attenuator settings

[Chris Packham]

Make use of enum chips and use a switch statement in load_attenuators()
so that the compiler can tell us if we've failed to cater for a
supported chip.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v3:
    - Reword commit message
    - Use switch instead of if/else
    Changes in v2:
    - New

 drivers/hwmon/adt7475.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index 6de501de41b2..ac480e6e4818 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -1569,12 +1569,12 @@ static int set_property_bit(const struct i2c_client *client, char *property,
 	return ret;
 }
 
-static int load_attenuators(const struct i2c_client *client, int chip,
+static int load_attenuators(const struct i2c_client *client, enum chips chip,
 			    struct adt7475_data *data)
 {
-	int ret;
-
-	if (chip == adt7476 || chip == adt7490) {
+	switch (chip) {
+	case adt7476:
+	case adt7490:
 		set_property_bit(client, "adi,bypass-attenuator-in0",
 				 &data->config4, 4);
 		set_property_bit(client, "adi,bypass-attenuator-in1",
@@ -1584,18 +1584,15 @@ static int load_attenuators(const struct i2c_client *client, int chip,
 		set_property_bit(client, "adi,bypass-attenuator-in4",
 				 &data->config4, 7);
 
-		ret = i2c_smbus_write_byte_data(client, REG_CONFIG4,
-						data->config4);
-		if (ret < 0)
-			return ret;
-	} else if (chip == adt7473 || chip == adt7475) {
+		return i2c_smbus_write_byte_data(client, REG_CONFIG4,
+						 data->config4);
+	case adt7473:
+	case adt7475:
 		set_property_bit(client, "adi,bypass-attenuator-in1",
 				 &data->config2, 5);
 
-		ret = i2c_smbus_write_byte_data(client, REG_CONFIG2,
-						data->config2);
-		if (ret < 0)
-			return ret;
+		return i2c_smbus_write_byte_data(client, REG_CONFIG2,
+						 data->config2);
 	}
 
 	return 0;
-- 
2.35.1

Re: [PATCH v3 3/3] hwmon: (adt7475) Use enum chips when loading attenuator settings

[Guenter Roeck]

On Wed, Mar 23, 2022 at 04:40:56PM +1300, Chris Packham wrote:
> Make use of enum chips and use a switch statement in load_attenuators()
> so that the compiler can tell us if we've failed to cater for a
> supported chip.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

For my reference:

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Guenter

> ---
> 
> Notes:
>     Changes in v3:
>     - Reword commit message
>     - Use switch instead of if/else
>     Changes in v2:
>     - New
> 
>  drivers/hwmon/adt7475.c | 23 ++++++++++-------------
>  1 file changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
> index 6de501de41b2..ac480e6e4818 100644
> --- a/drivers/hwmon/adt7475.c
> +++ b/drivers/hwmon/adt7475.c
> @@ -1569,12 +1569,12 @@ static int set_property_bit(const struct i2c_client *client, char *property,
>  	return ret;
>  }
>  
> -static int load_attenuators(const struct i2c_client *client, int chip,
> +static int load_attenuators(const struct i2c_client *client, enum chips chip,
>  			    struct adt7475_data *data)
>  {
> -	int ret;
> -
> -	if (chip == adt7476 || chip == adt7490) {
> +	switch (chip) {
> +	case adt7476:
> +	case adt7490:
>  		set_property_bit(client, "adi,bypass-attenuator-in0",
>  				 &data->config4, 4);
>  		set_property_bit(client, "adi,bypass-attenuator-in1",
> @@ -1584,18 +1584,15 @@ static int load_attenuators(const struct i2c_client *client, int chip,
>  		set_property_bit(client, "adi,bypass-attenuator-in4",
>  				 &data->config4, 7);
>  
> -		ret = i2c_smbus_write_byte_data(client, REG_CONFIG4,
> -						data->config4);
> -		if (ret < 0)
> -			return ret;
> -	} else if (chip == adt7473 || chip == adt7475) {
> +		return i2c_smbus_write_byte_data(client, REG_CONFIG4,
> +						 data->config4);
> +	case adt7473:
> +	case adt7475:
>  		set_property_bit(client, "adi,bypass-attenuator-in1",
>  				 &data->config2, 5);
>  
> -		ret = i2c_smbus_write_byte_data(client, REG_CONFIG2,
> -						data->config2);
> -		if (ret < 0)
> -			return ret;
> +		return i2c_smbus_write_byte_data(client, REG_CONFIG2,
> +						 data->config2);
>  	}
>  
>  	return 0;

Re: [PATCH v3 3/3] hwmon: (adt7475) Use enum chips when loading attenuator settings

[Guenter Roeck]

On Wed, Mar 23, 2022 at 04:40:56PM +1300, Chris Packham wrote:
> Make use of enum chips and use a switch statement in load_attenuators()
> so that the compiler can tell us if we've failed to cater for a
> supported chip.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Applied to hwmon-next.

Thanks,
Guenter

> ---
> 
> Notes:
>     Changes in v3:
>     - Reword commit message
>     - Use switch instead of if/else
>     Changes in v2:
>     - New
> 
>  drivers/hwmon/adt7475.c | 23 ++++++++++-------------
>  1 file changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
> index 6de501de41b2..ac480e6e4818 100644
> --- a/drivers/hwmon/adt7475.c
> +++ b/drivers/hwmon/adt7475.c
> @@ -1569,12 +1569,12 @@ static int set_property_bit(const struct i2c_client *client, char *property,
>  	return ret;
>  }
>  
> -static int load_attenuators(const struct i2c_client *client, int chip,
> +static int load_attenuators(const struct i2c_client *client, enum chips chip,
>  			    struct adt7475_data *data)
>  {
> -	int ret;
> -
> -	if (chip == adt7476 || chip == adt7490) {
> +	switch (chip) {
> +	case adt7476:
> +	case adt7490:
>  		set_property_bit(client, "adi,bypass-attenuator-in0",
>  				 &data->config4, 4);
>  		set_property_bit(client, "adi,bypass-attenuator-in1",
> @@ -1584,18 +1584,15 @@ static int load_attenuators(const struct i2c_client *client, int chip,
>  		set_property_bit(client, "adi,bypass-attenuator-in4",
>  				 &data->config4, 7);
>  
> -		ret = i2c_smbus_write_byte_data(client, REG_CONFIG4,
> -						data->config4);
> -		if (ret < 0)
> -			return ret;
> -	} else if (chip == adt7473 || chip == adt7475) {
> +		return i2c_smbus_write_byte_data(client, REG_CONFIG4,
> +						 data->config4);
> +	case adt7473:
> +	case adt7475:
>  		set_property_bit(client, "adi,bypass-attenuator-in1",
>  				 &data->config2, 5);
>  
> -		ret = i2c_smbus_write_byte_data(client, REG_CONFIG2,
> -						data->config2);
> -		if (ret < 0)
> -			return ret;
> +		return i2c_smbus_write_byte_data(client, REG_CONFIG2,
> +						 data->config2);
>  	}
>  
>  	return 0;