Received-SPF: pass (google.com: domain of linux-kernel+bounces-147803-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Sender: Guenter Roeck <groeck7@gmail.com>
Date: Tue, 16 Apr 2024 17:32:46 -0700
From: Guenter Roeck <linux@roeck-us.net>
To: Jose Ramon San Buenaventura <jose.sanbuenaventura@analog.com>
Cc: linux-hwmon@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-i2c@vger.kernel.org, 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 2/2] hwmon: pmbus: adm1275: add adm1281 support
Message-ID: <b36db2c0-db31-4304-8e58-aa358ab811c5@roeck-us.net>
References: <20240417000722.919-1-jose.sanbuenaventura@analog.com>
 <20240417000722.919-3-jose.sanbuenaventura@analog.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240417000722.919-3-jose.sanbuenaventura@analog.com>

On Wed, Apr 17, 2024 at 08:07:22AM +0800, Jose Ramon San Buenaventura wrote:
> Adding support for adm1281 which is similar to adm1275
> 
> ADM1281 has STATUS_CML read support which is also being added.
> 
> 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   | 27 +++++++++++++++++++++++++--
>  3 files changed, 38 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..6c3e8840f 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;
>  };
>  
> @@ -469,6 +470,22 @@ static int adm1275_read_byte_data(struct i2c_client *client, int page, int reg)
>  				ret |= PB_VOLTAGE_UV_WARNING;
>  		}
>  		break;
> +	case PMBUS_STATUS_CML:
> +		if (!data->have_status_cml)
> +			return -ENXIO;
> +
> +		ret = pmbus_read_byte_data(client, page, PMBUS_STATUS_BYTE);
> +		if (ret < 0)
> +			break;

You'll have to explain why this additional status byte read
is necessary (while it isn't necessary for all other chips supporting
PMBUS_STATUS_CML).

Thanks,
Guenter