diff mbox series

[4/8] hwmon: (pmbus/mp2975) Simplify VOUT code

Message ID 20230712114754.500477-4-Naresh.Solanki@9elements.com (mailing list archive)
State Superseded
Headers show
Series [1/8] hwmon: (pmbus/mp2975) Fix whitespace error | expand

Commit Message

Naresh Solanki July 12, 2023, 11:47 a.m. UTC
From: Patrick Rudolph <patrick.rudolph@9elements.com>

In order to upstream MP2973/MP2971 simplify the code by removing support
for various VOUT formats. The MP2973 and MP2971 supports all PMBUS
supported formats for VOUT, while the MP2975 only support DIRECT and
VID for VOUT.

In DIRECT mode all chips report the voltage in 1mV/LSB.

Configure the chip to use DIRECT format for VOUT and drop the code
conversion code for other formats. The to be added chips MP2973/MP2971
will be configured to also report VOUT in DIRECT format.

The maximum voltage that can be reported in DIRECT format is 32768mV.
This is sufficient as the maximum output voltage for VR12/VR13 is
3040 mV.

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
---
 drivers/hwmon/pmbus/mp2975.c | 54 ++++++------------------------------
 1 file changed, 8 insertions(+), 46 deletions(-)

Comments

Guenter Roeck July 12, 2023, 3:49 p.m. UTC | #1
On 7/12/23 04:47, Naresh Solanki wrote:
> From: Patrick Rudolph <patrick.rudolph@9elements.com>
> 
> In order to upstream MP2973/MP2971 simplify the code by removing support
> for various VOUT formats. The MP2973 and MP2971 supports all PMBUS
> supported formats for VOUT, while the MP2975 only support DIRECT and
> VID for VOUT.
> 
> In DIRECT mode all chips report the voltage in 1mV/LSB.
> 
> Configure the chip to use DIRECT format for VOUT and drop the code
> conversion code for other formats. The to be added chips MP2973/MP2971
> will be configured to also report VOUT in DIRECT format.
> 
> The maximum voltage that can be reported in DIRECT format is 32768mV.
> This is sufficient as the maximum output voltage for VR12/VR13 is
> 3040 mV.
> 
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
> ---
>   drivers/hwmon/pmbus/mp2975.c | 54 ++++++------------------------------
>   1 file changed, 8 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/mp2975.c b/drivers/hwmon/pmbus/mp2975.c
> index 04778f2dcbdb..ebc9a84b8ad3 100644
> --- a/drivers/hwmon/pmbus/mp2975.c
> +++ b/drivers/hwmon/pmbus/mp2975.c
> @@ -70,7 +70,6 @@ struct mp2975_data {
>   	int vref_off[MP2975_PAGE_NUM];
>   	int vout_max[MP2975_PAGE_NUM];
>   	int vout_ov_fixed[MP2975_PAGE_NUM];
> -	int vout_format[MP2975_PAGE_NUM];
>   	int curr_sense_gain[MP2975_PAGE_NUM];
>   };
>   
> @@ -83,22 +82,6 @@ MODULE_DEVICE_TABLE(i2c, mp2975_id);
>   
>   #define to_mp2975_data(x)  container_of(x, struct mp2975_data, info)
>   
> -static int mp2975_read_byte_data(struct i2c_client *client, int page, int reg)
> -{
> -	switch (reg) {
> -	case PMBUS_VOUT_MODE:
> -		/*
> -		 * Enforce VOUT direct format, since device allows to set the
> -		 * different formats for the different rails. Conversion from
> -		 * VID to direct provided by driver internally, in case it is
> -		 * necessary.
> -		 */
> -		return PB_VOUT_MODE_DIRECT;
> -	default:
> -		return -ENODATA;
> -	}
> -}
> -
>   static int
>   mp2975_read_word_helper(struct i2c_client *client, int page, int phase, u8 reg,
>   			u16 mask)
> @@ -273,24 +256,6 @@ static int mp2975_read_word_data(struct i2c_client *client, int page,
>   		ret = DIV_ROUND_CLOSEST(data->vref[page] * 10 - 50 *
>   					(ret + 1) * data->vout_scale, 10);
>   		break;
> -	case PMBUS_READ_VOUT:
> -		ret = mp2975_read_word_helper(client, page, phase, reg,
> -					      GENMASK(11, 0));
> -		if (ret < 0)
> -			return ret;
> -
> -		/*
> -		 * READ_VOUT can be provided in VID or direct format. The
> -		 * format type is specified by bit 15 of the register
> -		 * MP2975_MFR_DC_LOOP_CTRL. The driver enforces VOUT direct
> -		 * format, since device allows to set the different formats for
> -		 * the different rails and also all VOUT limits registers are
> -		 * provided in a direct format. In case format is VID - convert
> -		 * to direct.
> -		 */
> -		if (data->vout_format[page] == vid)
> -			ret = mp2975_vid2direct(info->vrm_version[page], ret);
> -		break;
>   	case PMBUS_VIRT_READ_POUT_MAX:
>   		ret = mp2975_read_word_helper(client, page, phase,
>   					      MP2975_MFR_READ_POUT_PK,
> @@ -578,20 +543,18 @@ mp2975_vout_max_get(struct i2c_client *client, struct mp2975_data *data,
>   }
>   
>   static int
> -mp2975_identify_vout_format(struct i2c_client *client,
> -			    struct mp2975_data *data, int page)
> +mp2975_set_vout_format(struct i2c_client *client,
> +		       struct mp2975_data *data, int page)
>   {
>   	int ret;
>   
>   	ret = i2c_smbus_read_word_data(client, MP2975_MFR_DC_LOOP_CTRL);
>   	if (ret < 0)
>   		return ret;
> -
> -	if (ret & MP2975_VOUT_FORMAT)
> -		data->vout_format[page] = vid;
> -	else
> -		data->vout_format[page] = direct;
> -	return 0;
> +	/* Enable DIRECT VOUT format 1mV/LSB */
> +	ret &= ~MP2975_VOUT_FORMAT;
> +	ret = i2c_smbus_write_word_data(client, MP2975_MFR_DC_LOOP_CTRL, ret);

Writing this back is only needed if MP2975_VOUT_FORMAT was not already cleared.

> +	return ret;
>   }
>   
>   static int
> @@ -650,11 +613,11 @@ mp2975_vout_per_rail_config_get(struct i2c_client *client,
>   			return ret;
>   
>   		/*
> -		 * Get VOUT format for READ_VOUT command : VID or direct.
> +		 * Set VOUT format for READ_VOUT command : direct.
>   		 * Pages on same device can be configured with different
>   		 * formats.

Not sure if this comment still makes sense.

>   		 */
> -		ret = mp2975_identify_vout_format(client, data, i);
> +		ret = mp2975_set_vout_format(client, data, i);
>   		if (ret < 0)
>   			return ret;
>   
> @@ -689,7 +652,6 @@ static struct pmbus_driver_info mp2975_info = {
>   		PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
>   		PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP | PMBUS_HAVE_POUT |
>   		PMBUS_HAVE_PIN | PMBUS_HAVE_STATUS_INPUT | PMBUS_PHASE_VIRTUAL,
> -	.read_byte_data = mp2975_read_byte_data,
>   	.read_word_data = mp2975_read_word_data,
>   };
>
Naresh Solanki July 14, 2023, 11:23 a.m. UTC | #2
Hi Guenter,

On Wed, 12 Jul 2023 at 17:50, Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 7/12/23 04:47, Naresh Solanki wrote:
> > From: Patrick Rudolph <patrick.rudolph@9elements.com>
> >
> > In order to upstream MP2973/MP2971 simplify the code by removing support
> > for various VOUT formats. The MP2973 and MP2971 supports all PMBUS
> > supported formats for VOUT, while the MP2975 only support DIRECT and
> > VID for VOUT.
> >
> > In DIRECT mode all chips report the voltage in 1mV/LSB.
> >
> > Configure the chip to use DIRECT format for VOUT and drop the code
> > conversion code for other formats. The to be added chips MP2973/MP2971
> > will be configured to also report VOUT in DIRECT format.
> >
> > The maximum voltage that can be reported in DIRECT format is 32768mV.
> > This is sufficient as the maximum output voltage for VR12/VR13 is
> > 3040 mV.
> >
> > Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> > Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
> > ---
> >   drivers/hwmon/pmbus/mp2975.c | 54 ++++++------------------------------
> >   1 file changed, 8 insertions(+), 46 deletions(-)
> >
> > diff --git a/drivers/hwmon/pmbus/mp2975.c b/drivers/hwmon/pmbus/mp2975.c
> > index 04778f2dcbdb..ebc9a84b8ad3 100644
> > --- a/drivers/hwmon/pmbus/mp2975.c
> > +++ b/drivers/hwmon/pmbus/mp2975.c
> > @@ -70,7 +70,6 @@ struct mp2975_data {
> >       int vref_off[MP2975_PAGE_NUM];
> >       int vout_max[MP2975_PAGE_NUM];
> >       int vout_ov_fixed[MP2975_PAGE_NUM];
> > -     int vout_format[MP2975_PAGE_NUM];
> >       int curr_sense_gain[MP2975_PAGE_NUM];
> >   };
> >
> > @@ -83,22 +82,6 @@ MODULE_DEVICE_TABLE(i2c, mp2975_id);
> >
> >   #define to_mp2975_data(x)  container_of(x, struct mp2975_data, info)
> >
> > -static int mp2975_read_byte_data(struct i2c_client *client, int page, int reg)
> > -{
> > -     switch (reg) {
> > -     case PMBUS_VOUT_MODE:
> > -             /*
> > -              * Enforce VOUT direct format, since device allows to set the
> > -              * different formats for the different rails. Conversion from
> > -              * VID to direct provided by driver internally, in case it is
> > -              * necessary.
> > -              */
> > -             return PB_VOUT_MODE_DIRECT;
> > -     default:
> > -             return -ENODATA;
> > -     }
> > -}
> > -
> >   static int
> >   mp2975_read_word_helper(struct i2c_client *client, int page, int phase, u8 reg,
> >                       u16 mask)
> > @@ -273,24 +256,6 @@ static int mp2975_read_word_data(struct i2c_client *client, int page,
> >               ret = DIV_ROUND_CLOSEST(data->vref[page] * 10 - 50 *
> >                                       (ret + 1) * data->vout_scale, 10);
> >               break;
> > -     case PMBUS_READ_VOUT:
> > -             ret = mp2975_read_word_helper(client, page, phase, reg,
> > -                                           GENMASK(11, 0));
> > -             if (ret < 0)
> > -                     return ret;
> > -
> > -             /*
> > -              * READ_VOUT can be provided in VID or direct format. The
> > -              * format type is specified by bit 15 of the register
> > -              * MP2975_MFR_DC_LOOP_CTRL. The driver enforces VOUT direct
> > -              * format, since device allows to set the different formats for
> > -              * the different rails and also all VOUT limits registers are
> > -              * provided in a direct format. In case format is VID - convert
> > -              * to direct.
> > -              */
> > -             if (data->vout_format[page] == vid)
> > -                     ret = mp2975_vid2direct(info->vrm_version[page], ret);
> > -             break;
> >       case PMBUS_VIRT_READ_POUT_MAX:
> >               ret = mp2975_read_word_helper(client, page, phase,
> >                                             MP2975_MFR_READ_POUT_PK,
> > @@ -578,20 +543,18 @@ mp2975_vout_max_get(struct i2c_client *client, struct mp2975_data *data,
> >   }
> >
> >   static int
> > -mp2975_identify_vout_format(struct i2c_client *client,
> > -                         struct mp2975_data *data, int page)
> > +mp2975_set_vout_format(struct i2c_client *client,
> > +                    struct mp2975_data *data, int page)
> >   {
> >       int ret;
> >
> >       ret = i2c_smbus_read_word_data(client, MP2975_MFR_DC_LOOP_CTRL);
> >       if (ret < 0)
> >               return ret;
> > -
> > -     if (ret & MP2975_VOUT_FORMAT)
> > -             data->vout_format[page] = vid;
> > -     else
> > -             data->vout_format[page] = direct;
> > -     return 0;
> > +     /* Enable DIRECT VOUT format 1mV/LSB */
> > +     ret &= ~MP2975_VOUT_FORMAT;
> > +     ret = i2c_smbus_write_word_data(client, MP2975_MFR_DC_LOOP_CTRL, ret);
>
> Writing this back is only needed if MP2975_VOUT_FORMAT was not already cleared.
Yes. Will optimize it as:
if (ret & MP2975_VOUT_FORMAT) {
ret &= ~MP2975_VOUT_FORMAT;
ret = i2c_smbus_write_word_data(client, MP2975_MFR_DC_LOOP_CTRL, ret);
}


>
> > +     return ret;
> >   }
> >
> >   static int
> > @@ -650,11 +613,11 @@ mp2975_vout_per_rail_config_get(struct i2c_client *client,
> >                       return ret;
> >
> >               /*
> > -              * Get VOUT format for READ_VOUT command : VID or direct.
> > +              * Set VOUT format for READ_VOUT command : direct.
> >                * Pages on same device can be configured with different
> >                * formats.
>
> Not sure if this comment still makes sense.
Yes. Updated the comment as below:
/* Set VOUT format for READ_VOUT command : direct. */
ret = mp2975_set_vout_format(....

>
> >                */
> > -             ret = mp2975_identify_vout_format(client, data, i);
> > +             ret = mp2975_set_vout_format(client, data, i);
> >               if (ret < 0)
> >                       return ret;
> >
> > @@ -689,7 +652,6 @@ static struct pmbus_driver_info mp2975_info = {
> >               PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
> >               PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP | PMBUS_HAVE_POUT |
> >               PMBUS_HAVE_PIN | PMBUS_HAVE_STATUS_INPUT | PMBUS_PHASE_VIRTUAL,
> > -     .read_byte_data = mp2975_read_byte_data,
> >       .read_word_data = mp2975_read_word_data,
> >   };
> >
>

Regards,
Naresh Solanki
diff mbox series

Patch

diff --git a/drivers/hwmon/pmbus/mp2975.c b/drivers/hwmon/pmbus/mp2975.c
index 04778f2dcbdb..ebc9a84b8ad3 100644
--- a/drivers/hwmon/pmbus/mp2975.c
+++ b/drivers/hwmon/pmbus/mp2975.c
@@ -70,7 +70,6 @@  struct mp2975_data {
 	int vref_off[MP2975_PAGE_NUM];
 	int vout_max[MP2975_PAGE_NUM];
 	int vout_ov_fixed[MP2975_PAGE_NUM];
-	int vout_format[MP2975_PAGE_NUM];
 	int curr_sense_gain[MP2975_PAGE_NUM];
 };
 
@@ -83,22 +82,6 @@  MODULE_DEVICE_TABLE(i2c, mp2975_id);
 
 #define to_mp2975_data(x)  container_of(x, struct mp2975_data, info)
 
-static int mp2975_read_byte_data(struct i2c_client *client, int page, int reg)
-{
-	switch (reg) {
-	case PMBUS_VOUT_MODE:
-		/*
-		 * Enforce VOUT direct format, since device allows to set the
-		 * different formats for the different rails. Conversion from
-		 * VID to direct provided by driver internally, in case it is
-		 * necessary.
-		 */
-		return PB_VOUT_MODE_DIRECT;
-	default:
-		return -ENODATA;
-	}
-}
-
 static int
 mp2975_read_word_helper(struct i2c_client *client, int page, int phase, u8 reg,
 			u16 mask)
@@ -273,24 +256,6 @@  static int mp2975_read_word_data(struct i2c_client *client, int page,
 		ret = DIV_ROUND_CLOSEST(data->vref[page] * 10 - 50 *
 					(ret + 1) * data->vout_scale, 10);
 		break;
-	case PMBUS_READ_VOUT:
-		ret = mp2975_read_word_helper(client, page, phase, reg,
-					      GENMASK(11, 0));
-		if (ret < 0)
-			return ret;
-
-		/*
-		 * READ_VOUT can be provided in VID or direct format. The
-		 * format type is specified by bit 15 of the register
-		 * MP2975_MFR_DC_LOOP_CTRL. The driver enforces VOUT direct
-		 * format, since device allows to set the different formats for
-		 * the different rails and also all VOUT limits registers are
-		 * provided in a direct format. In case format is VID - convert
-		 * to direct.
-		 */
-		if (data->vout_format[page] == vid)
-			ret = mp2975_vid2direct(info->vrm_version[page], ret);
-		break;
 	case PMBUS_VIRT_READ_POUT_MAX:
 		ret = mp2975_read_word_helper(client, page, phase,
 					      MP2975_MFR_READ_POUT_PK,
@@ -578,20 +543,18 @@  mp2975_vout_max_get(struct i2c_client *client, struct mp2975_data *data,
 }
 
 static int
-mp2975_identify_vout_format(struct i2c_client *client,
-			    struct mp2975_data *data, int page)
+mp2975_set_vout_format(struct i2c_client *client,
+		       struct mp2975_data *data, int page)
 {
 	int ret;
 
 	ret = i2c_smbus_read_word_data(client, MP2975_MFR_DC_LOOP_CTRL);
 	if (ret < 0)
 		return ret;
-
-	if (ret & MP2975_VOUT_FORMAT)
-		data->vout_format[page] = vid;
-	else
-		data->vout_format[page] = direct;
-	return 0;
+	/* Enable DIRECT VOUT format 1mV/LSB */
+	ret &= ~MP2975_VOUT_FORMAT;
+	ret = i2c_smbus_write_word_data(client, MP2975_MFR_DC_LOOP_CTRL, ret);
+	return ret;
 }
 
 static int
@@ -650,11 +613,11 @@  mp2975_vout_per_rail_config_get(struct i2c_client *client,
 			return ret;
 
 		/*
-		 * Get VOUT format for READ_VOUT command : VID or direct.
+		 * Set VOUT format for READ_VOUT command : direct.
 		 * Pages on same device can be configured with different
 		 * formats.
 		 */
-		ret = mp2975_identify_vout_format(client, data, i);
+		ret = mp2975_set_vout_format(client, data, i);
 		if (ret < 0)
 			return ret;
 
@@ -689,7 +652,6 @@  static struct pmbus_driver_info mp2975_info = {
 		PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
 		PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP | PMBUS_HAVE_POUT |
 		PMBUS_HAVE_PIN | PMBUS_HAVE_STATUS_INPUT | PMBUS_PHASE_VIRTUAL,
-	.read_byte_data = mp2975_read_byte_data,
 	.read_word_data = mp2975_read_word_data,
 };