mbox series

[hwmon-next,v6,0/3] Add support for MPS Multi-phase mp2888 controller

Message ID 20210507171421.425817-1-vadimp@nvidia.com
Headers show
Series Add support for MPS Multi-phase mp2888 controller | expand

Message

Vadim Pasternak May 7, 2021, 5:14 p.m. UTC
Add driver and documentation for mp2888 device from Monolithic Power
Systems, Inc. (MPS) vendor. This is a digital, multi-phase, pulse-width
modulation controller.

Patch set includes:
Patch #1 - increases maximum number of phases.
Patch #2 - provides mp2888 driver and documentation.
Patch #3 - providesy binding documentation.

Vadim Pasternak (3):
  hwmon: (pmbus) Increase maximum number of phases per page
  hwmon: (pmbus) Add support for MPS Multi-phase mp2888 controller
  dt-bindings: Add MP2888 voltage regulator device

 .../devicetree/bindings/trivial-devices.yaml       |   2 +
 Documentation/hwmon/mp2888.rst                     | 113 ++++++
 drivers/hwmon/pmbus/Kconfig                        |   9 +
 drivers/hwmon/pmbus/Makefile                       |   1 +
 drivers/hwmon/pmbus/mp2888.c                       | 411 +++++++++++++++++++++
 drivers/hwmon/pmbus/pmbus.h                        |   2 +-
 6 files changed, 537 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/hwmon/mp2888.rst
 create mode 100644 drivers/hwmon/pmbus/mp2888.c

Comments

Guenter Roeck May 10, 2021, 4:48 p.m. UTC | #1
On Fri, May 07, 2021 at 08:14:20PM +0300, Vadim Pasternak wrote:
> Add support for mp2888 device from Monolithic Power Systems, Inc. (MPS)
> vendor. This is a digital, multi-phase, pulse-width modulation
> controller.
> 
> This device supports:
> - One power rail.
> - Programmable Multi-Phase up to 10 Phases.
> - PWM-VID Interface
> - One pages 0 for telemetry.
> - Programmable pins for PMBus Address.
> - Built-In EEPROM to Store Custom Configurations.
> - Can configured VOUT readout in direct or VID format and allows
>   setting of different formats on rails 1 and 2. For VID the following
>   protocols are available: VR13 mode with 5-mV DAC; VR13 mode with
>   10-mV DAC, IMVP9 mode with 5-mV DAC.
> 
> Signed-off-by: Vadim Pasternak <vadimp@nvidia.com>
> ---
> v5->v6
>  Comments pointed out by Guenter:
>  - Fix comments for limits in mp2888_read_word_data().
>  - Remove unnecessary typecasts from mp2888_write_word_data().
> 
> v4->v5
>  Comments pointed out by Guenter:
>  - Fix calculation of PMBUS_READ_VIN.
>  - Add mp2888_write_word_data() for limits setting.
>  - Treat PMBUS_POUT_OP_WARN_LIMIT in separate case.
>  - Drop tuning of "m[PSC_POWER]" and "m[PSC_CURRENT_OUT]" from probing
>    routine.
>  Fixes from Vadim:
>  - Treat PMBUS_IOUT_OC_WARN_LIMIT in separate case.
> 
> v3->v4
>  Comments pointed out by Guenter:
>   - Fix PMBUS_READ_VIN and limits calculations.
>   - Add comment for PMBUS_OT_WARN_LIMIT scaling.
>   - Fix PMBUS_READ_IOUT, PMBUS_READ_POUT, PMBUS_READ_PIN calculations.
>   - Enable PMBUS_IOUT_OC_WARN_LIMIT and PMBUS_POUT_OP_WARN_LIMIT.
>  Fixes from Vadim:
>   - Disable PMBUS_POUT_MAX. Device uses this register for different
>     purpose.
>   - Disable PMBUS_MFR_VOU_MIN. Device doe not implement this register.
>   - Update documentation file.
> 
> v2->v3
>  Comments pointed out by Guenter:
>  - Fix precision for PMBUS_READ_VIN (it requires adding scale for
>    PMBUS_VIN_OV_FAULT_LIMIT and PMBUS_VIN_UV_WARN_LIMIT.
>  - Fix precision for PMBUS_READ_TEMPERATURE_1 (it requires adding
>    scale for PMBUS_OT_WARN_LIMIT).
>  - Use DIV_ROUND_CLOSEST_ULL for scaling PMBUS_READ_POUT,
>    PMBUS_READ_PIN readouts.
>  Notes and fixes from Vadim:
>   - READ_IOUT in linear11 does not give write calculation (tested with
>     external load), while in direct format readouts are correct.
>   - Disable non-configured phases in mp2888_identify_multiphase().
> 
> v1->v2:
>  Comments pointed out by Guenter:
>   - Use standard access for getting PMBUS_OT_WARN_LIMIT,
>     PMBUS_VIN_OV_FAULT_LIMIT, PMBUS_VIN_UV_WARN_LIMIT.
>   - Use linear11 conversion for PMBUS_READ_VIN, PMBUS_READ_POUT,
>     PMBUS_READ_PIN, PMBUS_READ_TEMPERATURE_1 and adjust coefficients.
>   - Add reading phases current from the dedicated registers.
>   - Add comment for not implemented or implemented not according to the
> 	spec registers, for which "ENXIO" code is returned.
>   - Set PMBUS_HAVE_IOUT" statically.
>   Notes from Vadim:
>   - READ_IOUT uses direct format, so I did not adjust it like the below
>     registers.
> ---

[ ... ]

> +
> +static int mp2888_write_word_data(struct i2c_client *client, int page, int reg, u16 word)
> +{
> +	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
> +	struct mp2888_data *data = to_mp2888_data(info);
> +
> +	switch (reg) {
> +	case PMBUS_OT_WARN_LIMIT:
> +		word = DIV_ROUND_CLOSEST((int)word, MP2888_TEMP_UNIT);

Sorry if I am missing something, but why those typecasts here and below ?

> +		/* Drop unused bits 15:8. */
> +		word = clamp_val(word, 0, GENMASK(7, 0));
> +		break;
> +	case PMBUS_IOUT_OC_WARN_LIMIT:
> +		/* Fix limit according to total curent resolution. */
> +		word = data->total_curr_resolution ? DIV_ROUND_CLOSEST((int)word, 8) :
> +		       DIV_ROUND_CLOSEST((int)word, 4);
> +		/* Drop unused bits 15:10. */
> +		word = clamp_val(word, 0, GENMASK(9, 0));
> +		break;
> +	case PMBUS_POUT_OP_WARN_LIMIT:
> +		/* Fix limit according to total curent resolution. */
> +		word = data->total_curr_resolution ? DIV_ROUND_CLOSEST((int)word, 4) :
> +		       DIV_ROUND_CLOSEST((int)word, 2);
> +		/* Drop unused bits 15:10. */
> +		word = clamp_val(word, 0, GENMASK(9, 0));
> +		break;
> +	default:
> +		return -ENODATA;
> +	}
> +	return pmbus_write_word_data(client, page, reg, word);
> +}
> +
> +static int
> +mp2888_identify_multiphase(struct i2c_client *client, struct mp2888_data *data,
> +			   struct pmbus_driver_info *info)
> +{
> +	int i, ret;
> +
> +	ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Identify multiphase number - could be from 1 to 10. */
> +	ret = i2c_smbus_read_word_data(client, MP2888_MFR_VR_CONFIG1);
> +	if (ret <= 0)
> +		return ret;
> +
> +	info->phases[0] = ret & GENMASK(3, 0);
> +
> +	/*
> +	 * The device provides a total of 10 PWM pins, and can be configured to different phase
> +	 * count applications for rail.
> +	 */
> +	if (info->phases[0] > MP2888_MAX_PHASE)
> +		return -EINVAL;
> +
> +	/* Disable non-configured phases. */
> +	for (i = info->phases[0]; i < MP2888_MAX_PHASE; i++)
> +		info->pfunc[i] = 0;

Not that it matters much, but this is unnecessary since the pmbus core
won't look at phase data beyond info->phases[].

Guenter
Vadim Pasternak May 10, 2021, 6:28 p.m. UTC | #2
> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Monday, May 10, 2021 7:48 PM
> To: Vadim Pasternak <vadimp@nvidia.com>
> Cc: robh+dt@kernel.org; linux-hwmon@vger.kernel.org;
> devicetree@vger.kernel.org
> Subject: Re: [PATCH hwmon-next v6 2/3] hwmon: (pmbus) Add support for
> MPS Multi-phase mp2888 controller
> 
> On Fri, May 07, 2021 at 08:14:20PM +0300, Vadim Pasternak wrote:
> > Add support for mp2888 device from Monolithic Power Systems, Inc.
> > (MPS) vendor. This is a digital, multi-phase, pulse-width modulation
> > controller.
> >
> > This device supports:
> > - One power rail.
> > - Programmable Multi-Phase up to 10 Phases.
> > - PWM-VID Interface
> > - One pages 0 for telemetry.
> > - Programmable pins for PMBus Address.
> > - Built-In EEPROM to Store Custom Configurations.
> > - Can configured VOUT readout in direct or VID format and allows
> >   setting of different formats on rails 1 and 2. For VID the following
> >   protocols are available: VR13 mode with 5-mV DAC; VR13 mode with
> >   10-mV DAC, IMVP9 mode with 5-mV DAC.
> >
> > Signed-off-by: Vadim Pasternak <vadimp@nvidia.com>
> > ---
> > v5->v6
> >  Comments pointed out by Guenter:
> >  - Fix comments for limits in mp2888_read_word_data().
> >  - Remove unnecessary typecasts from mp2888_write_word_data().
> >
> > v4->v5
> >  Comments pointed out by Guenter:
> >  - Fix calculation of PMBUS_READ_VIN.
> >  - Add mp2888_write_word_data() for limits setting.
> >  - Treat PMBUS_POUT_OP_WARN_LIMIT in separate case.
> >  - Drop tuning of "m[PSC_POWER]" and "m[PSC_CURRENT_OUT]" from
> probing
> >    routine.
> >  Fixes from Vadim:
> >  - Treat PMBUS_IOUT_OC_WARN_LIMIT in separate case.
> >
> > v3->v4
> >  Comments pointed out by Guenter:
> >   - Fix PMBUS_READ_VIN and limits calculations.
> >   - Add comment for PMBUS_OT_WARN_LIMIT scaling.
> >   - Fix PMBUS_READ_IOUT, PMBUS_READ_POUT, PMBUS_READ_PIN
> calculations.
> >   - Enable PMBUS_IOUT_OC_WARN_LIMIT and
> PMBUS_POUT_OP_WARN_LIMIT.
> >  Fixes from Vadim:
> >   - Disable PMBUS_POUT_MAX. Device uses this register for different
> >     purpose.
> >   - Disable PMBUS_MFR_VOU_MIN. Device doe not implement this
> register.
> >   - Update documentation file.
> >
> > v2->v3
> >  Comments pointed out by Guenter:
> >  - Fix precision for PMBUS_READ_VIN (it requires adding scale for
> >    PMBUS_VIN_OV_FAULT_LIMIT and PMBUS_VIN_UV_WARN_LIMIT.
> >  - Fix precision for PMBUS_READ_TEMPERATURE_1 (it requires adding
> >    scale for PMBUS_OT_WARN_LIMIT).
> >  - Use DIV_ROUND_CLOSEST_ULL for scaling PMBUS_READ_POUT,
> >    PMBUS_READ_PIN readouts.
> >  Notes and fixes from Vadim:
> >   - READ_IOUT in linear11 does not give write calculation (tested with
> >     external load), while in direct format readouts are correct.
> >   - Disable non-configured phases in mp2888_identify_multiphase().
> >
> > v1->v2:
> >  Comments pointed out by Guenter:
> >   - Use standard access for getting PMBUS_OT_WARN_LIMIT,
> >     PMBUS_VIN_OV_FAULT_LIMIT, PMBUS_VIN_UV_WARN_LIMIT.
> >   - Use linear11 conversion for PMBUS_READ_VIN, PMBUS_READ_POUT,
> >     PMBUS_READ_PIN, PMBUS_READ_TEMPERATURE_1 and adjust
> coefficients.
> >   - Add reading phases current from the dedicated registers.
> >   - Add comment for not implemented or implemented not according to the
> > 	spec registers, for which "ENXIO" code is returned.
> >   - Set PMBUS_HAVE_IOUT" statically.
> >   Notes from Vadim:
> >   - READ_IOUT uses direct format, so I did not adjust it like the below
> >     registers.
> > ---
> 
> [ ... ]
> 
> > +
> > +static int mp2888_write_word_data(struct i2c_client *client, int
> > +page, int reg, u16 word) {
> > +	const struct pmbus_driver_info *info =
> pmbus_get_driver_info(client);
> > +	struct mp2888_data *data = to_mp2888_data(info);
> > +
> > +	switch (reg) {
> > +	case PMBUS_OT_WARN_LIMIT:
> > +		word = DIV_ROUND_CLOSEST((int)word,
> MP2888_TEMP_UNIT);
> 
> Sorry if I am missing something, but why those typecasts here and below ?

I forgot to remove it. Sorry.

> 
> > +		/* Drop unused bits 15:8. */
> > +		word = clamp_val(word, 0, GENMASK(7, 0));
> > +		break;
> > +	case PMBUS_IOUT_OC_WARN_LIMIT:
> > +		/* Fix limit according to total curent resolution. */
> > +		word = data->total_curr_resolution ?
> DIV_ROUND_CLOSEST((int)word, 8) :
> > +		       DIV_ROUND_CLOSEST((int)word, 4);
> > +		/* Drop unused bits 15:10. */
> > +		word = clamp_val(word, 0, GENMASK(9, 0));
> > +		break;
> > +	case PMBUS_POUT_OP_WARN_LIMIT:
> > +		/* Fix limit according to total curent resolution. */
> > +		word = data->total_curr_resolution ?
> DIV_ROUND_CLOSEST((int)word, 4) :
> > +		       DIV_ROUND_CLOSEST((int)word, 2);
> > +		/* Drop unused bits 15:10. */
> > +		word = clamp_val(word, 0, GENMASK(9, 0));
> > +		break;
> > +	default:
> > +		return -ENODATA;
> > +	}
> > +	return pmbus_write_word_data(client, page, reg, word); }
> > +
> > +static int
> > +mp2888_identify_multiphase(struct i2c_client *client, struct mp2888_data
> *data,
> > +			   struct pmbus_driver_info *info) {
> > +	int i, ret;
> > +
> > +	ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/* Identify multiphase number - could be from 1 to 10. */
> > +	ret = i2c_smbus_read_word_data(client,
> MP2888_MFR_VR_CONFIG1);
> > +	if (ret <= 0)
> > +		return ret;
> > +
> > +	info->phases[0] = ret & GENMASK(3, 0);
> > +
> > +	/*
> > +	 * The device provides a total of 10 PWM pins, and can be configured
> to different phase
> > +	 * count applications for rail.
> > +	 */
> > +	if (info->phases[0] > MP2888_MAX_PHASE)
> > +		return -EINVAL;
> > +
> > +	/* Disable non-configured phases. */
> > +	for (i = info->phases[0]; i < MP2888_MAX_PHASE; i++)
> > +		info->pfunc[i] = 0;
> 
> Not that it matters much, but this is unnecessary since the pmbus core won't
> look at phase data beyond info->phases[].

I see. So I'll drop these two lines.

> 
> Guenter