[PATCH v2 0/2] Add adm1281 support

[PATCH v2 0/2] Add adm1281 support

[Jose Ramon San Buenaventura]

This patch removes the extra case added in the adm1275_read_byte_data
for STATUS_CML reads. Upon checking, the reads for the STATUS_CML register
is already handled in the pmbus_core. 

It was also clarified and agreed upon that any other actionable steps
involving the STATUS_CML error flags should be added in the pmbus_core
and not on the specific chip driver.

Jose Ramon San Buenaventura (2):
  dt-bindings: hwmon: adm1275: add adm1281
  hwmon: pmbus: adm1275: add adm1281 support

 .../devicetree/bindings/hwmon/adi,adm1275.yaml     |  4 +++-
 Documentation/hwmon/adm1275.rst                    | 14 +++++++++++---
 drivers/hwmon/pmbus/Kconfig                        |  4 ++--
 drivers/hwmon/pmbus/adm1275.c                      | 11 +++++++++--
 4 files changed, 25 insertions(+), 8 deletions(-)


base-commit: 96fca68c4fbf77a8185eb10f7557e23352732ea2
-- 
2.39.2

[PATCH v2 1/2] dt-bindings: hwmon: adm1275: add adm1281

[Jose Ramon San Buenaventura]

Add support for the adm1281 Hot-Swap Controller and Digital Power
and Energy Monitor

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Jose Ramon San Buenaventura <jose.sanbuenaventura@analog.com>
---
 Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
index b68061294..5b076d677 100644
--- a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
+++ b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
@@ -5,7 +5,7 @@
 $id: http://devicetree.org/schemas/hwmon/adi,adm1275.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Analog Devices ADM1075/ADM127x/ADM129x digital power monitors
+title: Analog Devices ADM1075/ADM127x/ADM1281/ADM129x digital power monitors
 
 maintainers:
   - Krzysztof Kozlowski <krzk@kernel.org>
@@ -27,6 +27,7 @@ properties:
       - adi,adm1275
       - adi,adm1276
       - adi,adm1278
+      - adi,adm1281
       - adi,adm1293
       - adi,adm1294
 
@@ -91,6 +92,7 @@ allOf:
           contains:
             enum:
               - adi,adm1278
+              - adi,adm1281
               - adi,adm1293
               - adi,adm1294
     then:
-- 
2.39.2

[PATCH v2 2/2] hwmon: pmbus: adm1275: add adm1281 support

[Jose Ramon San Buenaventura]

Adding support for adm1281 which is similar to adm1275

Removed extra case for STATUS_CML in adm1275_read_byte_data function.

Signed-off-by: Jose Ramon San Buenaventura <jose.sanbuenaventura@analog.com>
---
 Documentation/hwmon/adm1275.rst | 14 +++++++++++---
 drivers/hwmon/pmbus/Kconfig     |  4 ++--
 drivers/hwmon/pmbus/adm1275.c   | 11 +++++++++--
 3 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/Documentation/hwmon/adm1275.rst b/Documentation/hwmon/adm1275.rst
index 804590eea..467daf8ce 100644
--- a/Documentation/hwmon/adm1275.rst
+++ b/Documentation/hwmon/adm1275.rst
@@ -43,6 +43,14 @@ Supported chips:
 
     Datasheet: www.analog.com/static/imported-files/data_sheets/ADM1278.pdf
 
+  * Analog Devices ADM1281
+
+    Prefix: 'adm1281'
+
+    Addresses scanned: -
+
+    Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adm1281.pdf
+
   * Analog Devices ADM1293/ADM1294
 
     Prefix: 'adm1293', 'adm1294'
@@ -58,10 +66,10 @@ Description
 -----------
 
 This driver supports hardware monitoring for Analog Devices ADM1075, ADM1272,
-ADM1275, ADM1276, ADM1278, ADM1293, and ADM1294 Hot-Swap Controller and
+ADM1275, ADM1276, ADM1278, ADM1281, ADM1293, and ADM1294 Hot-Swap Controller and
 Digital Power Monitors.
 
-ADM1075, ADM1272, ADM1275, ADM1276, ADM1278, ADM1293, and ADM1294 are hot-swap
+ADM1075, ADM1272, ADM1275, ADM1276, ADM1278, ADM1281, ADM1293, and ADM1294 are hot-swap
 controllers that allow a circuit board to be removed from or inserted into
 a live backplane. They also feature current and voltage readback via an
 integrated 12 bit analog-to-digital converter (ADC), accessed using a
@@ -144,5 +152,5 @@ temp1_highest		Highest observed temperature.
 temp1_reset_history	Write any value to reset history.
 
 			Temperature attributes are supported on ADM1272 and
-			ADM1278.
+			ADM1278, and ADM1281.
 ======================= =======================================================
diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index 557ae0c41..9c1d0d7d5 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -51,8 +51,8 @@ config SENSORS_ADM1275
 	tristate "Analog Devices ADM1275 and compatibles"
 	help
 	  If you say yes here you get hardware monitoring support for Analog
-	  Devices ADM1075, ADM1272, ADM1275, ADM1276, ADM1278, ADM1293,
-	  and ADM1294 Hot-Swap Controller and Digital Power Monitors.
+	  Devices ADM1075, ADM1272, ADM1275, ADM1276, ADM1278, ADM1281,
+	  ADM1293, and ADM1294 Hot-Swap Controller and Digital Power Monitors.
 
 	  This driver can also be built as a module. If so, the module will
 	  be called adm1275.
diff --git a/drivers/hwmon/pmbus/adm1275.c b/drivers/hwmon/pmbus/adm1275.c
index e2c61d6fa..2baeee8be 100644
--- a/drivers/hwmon/pmbus/adm1275.c
+++ b/drivers/hwmon/pmbus/adm1275.c
@@ -18,7 +18,7 @@
 #include <linux/log2.h>
 #include "pmbus.h"
 
-enum chips { adm1075, adm1272, adm1275, adm1276, adm1278, adm1293, adm1294 };
+enum chips { adm1075, adm1272, adm1275, adm1276, adm1278, adm1281, adm1293, adm1294 };
 
 #define ADM1275_MFR_STATUS_IOUT_WARN2	BIT(0)
 #define ADM1293_MFR_STATUS_VAUX_UV_WARN	BIT(5)
@@ -101,6 +101,7 @@ struct adm1275_data {
 	bool have_pin_max;
 	bool have_temp_max;
 	bool have_power_sampling;
+	bool have_status_cml;
 	struct pmbus_driver_info info;
 };
 
@@ -482,6 +483,7 @@ static const struct i2c_device_id adm1275_id[] = {
 	{ "adm1275", adm1275 },
 	{ "adm1276", adm1276 },
 	{ "adm1278", adm1278 },
+	{ "adm1281", adm1281 },
 	{ "adm1293", adm1293 },
 	{ "adm1294", adm1294 },
 	{ }
@@ -555,7 +557,8 @@ static int adm1275_probe(struct i2c_client *client)
 			   client->name, mid->name);
 
 	if (mid->driver_data == adm1272 || mid->driver_data == adm1278 ||
-	    mid->driver_data == adm1293 || mid->driver_data == adm1294)
+	    mid->driver_data == adm1281 || mid->driver_data == adm1293 ||
+	    mid->driver_data == adm1294)
 		config_read_fn = i2c_smbus_read_word_data;
 	else
 		config_read_fn = i2c_smbus_read_byte_data;
@@ -703,6 +706,10 @@ static int adm1275_probe(struct i2c_client *client)
 			  PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT;
 		break;
 	case adm1278:
+	case adm1281:
+		if (data->id == adm1281)
+			data->have_status_cml = true;
+
 		data->have_vout = true;
 		data->have_pin_max = true;
 		data->have_temp_max = true;
-- 
2.39.2

Re: [PATCH v2 2/2] hwmon: pmbus: adm1275: add adm1281 support

[Nuno Sá]

Hi Jose,

On Mon, 2024-04-22 at 10:51 +0800, Jose Ramon San Buenaventura wrote:
> Adding support for adm1281 which is similar to adm1275
> 
> Removed extra case for STATUS_CML in adm1275_read_byte_data function.
> 

Remove the above line. As nothing to do with supporting the new device. This is what
it should be in your v2 changelog :)

> Signed-off-by: Jose Ramon San Buenaventura <jose.sanbuenaventura@analog.com>
> ---
>  Documentation/hwmon/adm1275.rst | 14 +++++++++++---
>  drivers/hwmon/pmbus/Kconfig     |  4 ++--
>  drivers/hwmon/pmbus/adm1275.c   | 11 +++++++++--
>  3 files changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/hwmon/adm1275.rst b/Documentation/hwmon/adm1275.rst
> index 804590eea..467daf8ce 100644
> --- a/Documentation/hwmon/adm1275.rst
> +++ b/Documentation/hwmon/adm1275.rst
> @@ -43,6 +43,14 @@ Supported chips:
>  
>      Datasheet: www.analog.com/static/imported-files/data_sheets/ADM1278.pdf
>  
> +  * Analog Devices ADM1281
> +
> +    Prefix: 'adm1281'
> +
> +    Addresses scanned: -
> +
> +    Datasheet:
> https://www.analog.com/media/en/technical-documentation/data-sheets/adm1281.pdf
> +
>    * Analog Devices ADM1293/ADM1294
>  
>      Prefix: 'adm1293', 'adm1294'
> @@ -58,10 +66,10 @@ Description
>  -----------
>  
>  This driver supports hardware monitoring for Analog Devices ADM1075, ADM1272,
> -ADM1275, ADM1276, ADM1278, ADM1293, and ADM1294 Hot-Swap Controller and
> +ADM1275, ADM1276, ADM1278, ADM1281, ADM1293, and ADM1294 Hot-Swap Controller and
>  Digital Power Monitors.
>  
> -ADM1075, ADM1272, ADM1275, ADM1276, ADM1278, ADM1293, and ADM1294 are hot-swap
> +ADM1075, ADM1272, ADM1275, ADM1276, ADM1278, ADM1281, ADM1293, and ADM1294 are
> hot-swap
>  controllers that allow a circuit board to be removed from or inserted into
>  a live backplane. They also feature current and voltage readback via an
>  integrated 12 bit analog-to-digital converter (ADC), accessed using a
> @@ -144,5 +152,5 @@ temp1_highest		Highest observed temperature.
>  temp1_reset_history	Write any value to reset history.
>  
>  			Temperature attributes are supported on ADM1272 and
> -			ADM1278.
> +			ADM1278, and ADM1281.
>  ======================= =======================================================
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 557ae0c41..9c1d0d7d5 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -51,8 +51,8 @@ config SENSORS_ADM1275
>  	tristate "Analog Devices ADM1275 and compatibles"
>  	help
>  	  If you say yes here you get hardware monitoring support for Analog
> -	  Devices ADM1075, ADM1272, ADM1275, ADM1276, ADM1278, ADM1293,
> -	  and ADM1294 Hot-Swap Controller and Digital Power Monitors.
> +	  Devices ADM1075, ADM1272, ADM1275, ADM1276, ADM1278, ADM1281,
> +	  ADM1293, and ADM1294 Hot-Swap Controller and Digital Power Monitors.
>  
>  	  This driver can also be built as a module. If so, the module will
>  	  be called adm1275.
> diff --git a/drivers/hwmon/pmbus/adm1275.c b/drivers/hwmon/pmbus/adm1275.c
> index e2c61d6fa..2baeee8be 100644
> --- a/drivers/hwmon/pmbus/adm1275.c
> +++ b/drivers/hwmon/pmbus/adm1275.c
> @@ -18,7 +18,7 @@
>  #include <linux/log2.h>
>  #include "pmbus.h"
>  
> -enum chips { adm1075, adm1272, adm1275, adm1276, adm1278, adm1293, adm1294 };
> +enum chips { adm1075, adm1272, adm1275, adm1276, adm1278, adm1281, adm1293,
> adm1294 };
>  
>  #define ADM1275_MFR_STATUS_IOUT_WARN2	BIT(0)
>  #define ADM1293_MFR_STATUS_VAUX_UV_WARN	BIT(5)
> @@ -101,6 +101,7 @@ struct adm1275_data {
>  	bool have_pin_max;
>  	bool have_temp_max;
>  	bool have_power_sampling;
> +	bool have_status_cml;

don't think we need this...

>  	struct pmbus_driver_info info;
>  };
>  
> @@ -482,6 +483,7 @@ static const struct i2c_device_id adm1275_id[] = {
>  	{ "adm1275", adm1275 },
>  	{ "adm1276", adm1276 },
>  	{ "adm1278", adm1278 },
> +	{ "adm1281", adm1281 },
>  	{ "adm1293", adm1293 },
>  	{ "adm1294", adm1294 },
>  	{ }
> @@ -555,7 +557,8 @@ static int adm1275_probe(struct i2c_client *client)
>  			   client->name, mid->name);
>  
>  	if (mid->driver_data == adm1272 || mid->driver_data == adm1278 ||
> -	    mid->driver_data == adm1293 || mid->driver_data == adm1294)
> +	    mid->driver_data == adm1281 || mid->driver_data == adm1293 ||
> +	    mid->driver_data == adm1294)
>  		config_read_fn = i2c_smbus_read_word_data;
>  	else
>  		config_read_fn = i2c_smbus_read_byte_data;
> @@ -703,6 +706,10 @@ static int adm1275_probe(struct i2c_client *client)
>  			  PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT;
>  		break;
>  	case adm1278:
> +	case adm1281:
> +		if (data->id == adm1281)
> +			data->have_status_cml = true;
> +

and this...

- Nuno Sá

Re: [PATCH v2 0/2] Add adm1281 support

[Krzysztof Kozlowski]

On 22/04/2024 04:51, Jose Ramon San Buenaventura wrote:
> This patch removes the extra case added in the adm1275_read_byte_data
> for STATUS_CML reads. Upon checking, the reads for the STATUS_CML register
> is already handled in the pmbus_core. 
> 
> It was also clarified and agreed upon that any other actionable steps
> involving the STATUS_CML error flags should be added in the pmbus_core
> and not on the specific chip driver.
> 

Where is the changelog? It's v2, so what happened here?

Best regards,
Krzysztof

Re: [PATCH v2 0/2] Add adm1281 support

[Guenter Roeck]

On 4/21/24 20:28, Krzysztof Kozlowski wrote:
> On 22/04/2024 04:51, Jose Ramon San Buenaventura wrote:
>> This patch removes the extra case added in the adm1275_read_byte_data
>> for STATUS_CML reads. Upon checking, the reads for the STATUS_CML register
>> is already handled in the pmbus_core.
>>
>> It was also clarified and agreed upon that any other actionable steps
>> involving the STATUS_CML error flags should be added in the pmbus_core
>> and not on the specific chip driver.
>>
> 
> Where is the changelog? It's v2, so what happened here?
> 

Same question.

Guenter

RE: [PATCH v2 0/2] Add adm1281 support

[SanBuenaventura, Jose]

> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Monday, April 22, 2024 11:45 AM
> To: Krzysztof Kozlowski <krzk@kernel.org>; SanBuenaventura, Jose
> <Jose.SanBuenaventura@analog.com>; linux-hwmon@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> doc@vger.kernel.org; linux-i2c@vger.kernel.org
> Cc: Jean Delvare <jdelvare@suse.com>; Rob Herring <robh@kernel.org>;
> Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Jonathan Corbet <corbet@lwn.net>; Delphine CC
> Chiu <Delphine_CC_Chiu@Wiwynn.com>
> Subject: Re: [PATCH v2 0/2] Add adm1281 support
> 
> [External]
> 
> On 4/21/24 20:28, Krzysztof Kozlowski wrote:
> > On 22/04/2024 04:51, Jose Ramon San Buenaventura wrote:
> >> This patch removes the extra case added in the adm1275_read_byte_data
> >> for STATUS_CML reads. Upon checking, the reads for the STATUS_CML
> >> register is already handled in the pmbus_core.
> >>
> >> It was also clarified and agreed upon that any other actionable steps
> >> involving the STATUS_CML error flags should be added in the
> >> pmbus_core and not on the specific chip driver.
> >>
> >
> > Where is the changelog? It's v2, so what happened here?
> >
> 
> Same question.
> 
> Guenter
> 

Hello,

The only change from v1 to v2 is the removal of the additional case
for STATUS_CML in the adm1275_read_byte_data function in the adm1275.c
driver code. No changes were done in the other files, only in the driver code.

I apologize for the confusion.

For clarification, do I need to resend the patches and include a changelog
in the patch that specifies the changes done i.e., v1 -> v2: removed case?

Joram