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