Re: [PATCH v2 3/5] hwmon: pmbus: mpq8785: Add support for MPM3695 family

[Krzysztof Kozlowski <krzk@kernel.org>]

On 09/05/2025 08:51, Pawel Dembicki wrote:
> Add support for the Monolithic Power Systems MPM3695 family.
> It contains four devices with suffixes: -10, -20, -25 and -100.
> The device is PMBus compliant and shares characteristics with the
> MPM82504.
> 
> MPM3695-25 has different VOLTAGE_SCALE_LOOP register size [11:0]
> and it needs to be separated because it will be configured in the next
> commit.

This should be *this* commit. Add proper support for a device in one
commit. Not half-broken commit, being fixed later.

> 
> Tested with device tree based matching (MPM3695-10).
> 
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> 
> ---
> v2:
>   - Added whole MPM3695 family
> ---
>  Documentation/hwmon/mpq8785.rst | 13 +++++++++----
>  drivers/hwmon/pmbus/mpq8785.c   |  8 +++++++-
>  2 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/hwmon/mpq8785.rst b/Documentation/hwmon/mpq8785.rst
> index be228ee58ce2..7093e4db4f55 100644
> --- a/Documentation/hwmon/mpq8785.rst
> +++ b/Documentation/hwmon/mpq8785.rst
> @@ -7,6 +7,7 @@ Supported chips:
>  
>    * MPS MPQ8785
>    * MPS MPM82504
> +  * MPS MPM3695 family
>  
>      Prefix: 'mpq8785'
>  
> @@ -29,6 +30,14 @@ The device can also operate in parallel with the MPM3695-100 and additional
>  MPM82504 devices to provide a higher output current. The MPM82504 operates
>  at high efficiency across a wide load range.
>  
> +The MPM3695 family is a scalable, ultra-thin, fully integrated power module with
> +a PMBus interface. It offers a complete power solution that achieves up to
> +10A (-10 variant), 20A (-25 variant), 25A (-20 variant), 100A (-100 variant)
> +of output current with excellent load and line regulation across a wide input
> +voltage range. It operates at high efficiency over a wide load range, and can
> +be parallled to deliver higher current. Variants -10,-20 and -100 have different
> +voltage scale configuration register range (10 bits) than -25 version (11 bits).
> +
>  The PMBus interface provides converter configurations and key parameters
>  monitoring.
>  
> @@ -42,10 +51,6 @@ Fully integrated protection features include over-current protection (OCP),
>  over-voltage protection (OVP), under-voltage protection (UVP), and
>  over-temperature protection (OTP).
>  
> -All supported modules require a minimal number of readily available, standard
> -external components. The MPQ8785 is available in a TLGA (5mmx6mm) package
> -and the MPM82504 is available in a BGA (15mmx30mmx5.18mm) package.
> -
>  Device compliant with:
>  
>  - PMBus rev 1.3 interface.
> diff --git a/drivers/hwmon/pmbus/mpq8785.c b/drivers/hwmon/pmbus/mpq8785.c
> index 9a4a211b2aeb..34245d0d2125 100644
> --- a/drivers/hwmon/pmbus/mpq8785.c
> +++ b/drivers/hwmon/pmbus/mpq8785.c
> @@ -10,7 +10,7 @@
>  
>  #define PMBUS_READ_TEMPERATURE_1_SIGN	BIT(9)
>  
> -enum chips { mpq8785, mpm82504 };
> +enum chips { mpq8785, mpm82504, mpm3695, mpm3695_25 };
>  
>  static int mpq8785_identify(struct i2c_client *client,
>  			    struct pmbus_driver_info *info)
> @@ -79,6 +79,8 @@ static struct pmbus_driver_info mpq8785_info = {
>  static const struct i2c_device_id mpq8785_id[] = {
>  	{ "mpq8785", mpq8785 },
>  	{ "mpm82504", mpm82504 },
> +	{ "mpm3695", mpm3695 },
> +	{ "mpm3695-25", mpm3695_25 },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(i2c, mpq8785_id);
> @@ -86,6 +88,8 @@ MODULE_DEVICE_TABLE(i2c, mpq8785_id);
>  static const struct of_device_id __maybe_unused mpq8785_of_match[] = {
>  	{ .compatible = "mps,mpq8785", .data = (void *)mpq8785 },
>  	{ .compatible = "mps,mpm82504", .data = (void *)mpm82504 },
> +	{ .compatible = "mps,mpm3695", .data = (void *)mpm3695 },
> +	{ .compatible = "mps,mpm3695-25", .data = (void *)mpm3695_25 },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, mpq8785_of_match);
> @@ -110,6 +114,8 @@ static int mpq8785_probe(struct i2c_client *client)
>  		info->identify = mpq8785_identify;
>  		break;
>  	case mpm82504:
> +	case mpm3695:
> +	case mpm3695_25:

So are these fully compatible? Looks like, so why aren't you expressing
it in the binding and here? No need for these redundant ID table entries.

>  		info->format[PSC_VOLTAGE_OUT] = direct;
>  		info->m[PSC_VOLTAGE_OUT] = 8;
>  		info->b[PSC_VOLTAGE_OUT] = 0;


Best regards,
Krzysztof