[PATCH v3 0/2] Add adm1281 support

[PATCH v3 0/2] Add adm1281 support

[Jose Ramon San Buenaventura]

CHANGELOG:

v2 -> v3:
        - removed extra boolean member in adm1275_data structure
        - refactored commit message        

v1 -> v2:
        - removed STATUS_CML case in read_byte_data function 

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                      |  7 +++++--
 4 files changed, 21 insertions(+), 8 deletions(-)


base-commit: 96fca68c4fbf77a8185eb10f7557e23352732ea2
-- 
2.39.2

[PATCH v3 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

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1719 bytes --]

On Thu, Apr 25, 2024 at 03:09:47PM +0800, Jose Ramon San Buenaventura wrote:
> 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>

I acked this, but I'm having second thoughts. The 1281 follows the same
codepaths in the driver as the 1278, why is the 1278 not used as a
fallback compatible here?

Thanks,
Conor.

> ---
>  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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Guenter Roeck]

On 4/25/24 00:19, Conor Dooley wrote:
> On Thu, Apr 25, 2024 at 03:09:47PM +0800, Jose Ramon San Buenaventura wrote:
>> 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>
> 
> I acked this, but I'm having second thoughts. The 1281 follows the same
> codepaths in the driver as the 1278, why is the 1278 not used as a
> fallback compatible here?
> 

That is rare for those devices es but true in this case. Technically possible,
though it would result in a "Device mismatch" notification by the driver
since it compares the configured vs. actual device IDs. I would prefer to
avoid that, if for nothing else to avoid having to deal with concerned users.

Thanks,
Guenter

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

[Guenter Roeck]

On Thu, Apr 25, 2024 at 03:09:47PM +0800, Jose Ramon San Buenaventura wrote:
> 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>

Applied.

Thanks,
Guenter

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

[Jose Ramon San Buenaventura]

Adding support for adm1281 which is similar to adm1275

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   |  7 +++++--
 3 files changed, 18 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..59ffc0828 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)
@@ -482,6 +482,7 @@ static const struct i2c_device_id adm1275_id[] = {
 	{ "adm1275", adm1275 },
 	{ "adm1276", adm1276 },
 	{ "adm1278", adm1278 },
+	{ "adm1281", adm1281 },
 	{ "adm1293", adm1293 },
 	{ "adm1294", adm1294 },
 	{ }
@@ -555,7 +556,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 +705,7 @@ static int adm1275_probe(struct i2c_client *client)
 			  PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT;
 		break;
 	case adm1278:
+	case adm1281:
 		data->have_vout = true;
 		data->have_pin_max = true;
 		data->have_temp_max = true;
-- 
2.39.2

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

[Guenter Roeck]

On 4/25/24 00:09, Jose Ramon San Buenaventura wrote:
> Adding support for adm1281 which is similar to adm1275
> 
> Signed-off-by: Jose Ramon San Buenaventura <jose.sanbuenaventura@analog.com>

Patch is fine, but we'll need to wait for Conor's feedback
regarding the compatible fallback before I can apply it.

Thanks,
Guenter

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 758 bytes --]

On Thu, Apr 25, 2024 at 10:52:03AM -0700, Guenter Roeck wrote:
> On 4/25/24 00:09, Jose Ramon San Buenaventura wrote:
> > Adding support for adm1281 which is similar to adm1275
> > 
> > Signed-off-by: Jose Ramon San Buenaventura <jose.sanbuenaventura@analog.com>
> 
> Patch is fine, but we'll need to wait for Conor's feedback
> regarding the compatible fallback before I can apply it.

Oh, I didn't realise you were waiting for me. I had deleted the thread
from my queue given I had already acked the patch.

I'm not really a fan of the warn when IDs don't match thing though,
seems a bit unhelpful, unless you've running into scenarios where the
firmware tells you there's a device present but actually there's some
other incompatible one.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Guenter Roeck]

On 4/26/24 10:01, Conor Dooley wrote:
> On Thu, Apr 25, 2024 at 10:52:03AM -0700, Guenter Roeck wrote:
>> On 4/25/24 00:09, Jose Ramon San Buenaventura wrote:
>>> Adding support for adm1281 which is similar to adm1275
>>>
>>> Signed-off-by: Jose Ramon San Buenaventura <jose.sanbuenaventura@analog.com>
>>
>> Patch is fine, but we'll need to wait for Conor's feedback
>> regarding the compatible fallback before I can apply it.
> 
> Oh, I didn't realise you were waiting for me. I had deleted the thread
> from my queue given I had already acked the patch.
> 
> I'm not really a fan of the warn when IDs don't match thing though,
> seems a bit unhelpful, unless you've running into scenarios where the
> firmware tells you there's a device present but actually there's some
> other incompatible one.

That is pretty much what we do, because many of the chips supported by the
driver are not or not fully compatible to each other. Sure, we could add
more information about fully compatible chips, but in my opinion that would
just add complexity to the driver for little if any gain.

Thanks,
Guenter

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

[Guenter Roeck]

On Thu, Apr 25, 2024 at 03:09:48PM +0800, Jose Ramon San Buenaventura wrote:
> Adding support for adm1281 which is similar to adm1275
> 
> Signed-off-by: Jose Ramon San Buenaventura <jose.sanbuenaventura@analog.com>

Applied.

Thanks,
Guenter