All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Chris Packham <chris.packham@alliedtelesis.co.nz>
Cc: jdelvare@suse.com, linux-hwmon@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] hwmon: (pmbus/bpa-rs600) Remove duplicate defininitions
Date: Wed, 11 Aug 2021 12:53:46 -0700	[thread overview]
Message-ID: <20210811195346.GA966404@roeck-us.net> (raw)
In-Reply-To: <20210811041738.15061-2-chris.packham@alliedtelesis.co.nz>

On Wed, Aug 11, 2021 at 04:17:37PM +1200, Chris Packham wrote:
> Commit 787c095edaa9 ("hwmon: (pmbus/core) Add support for rated
> attributes") added definitions for MFR_VIN_MIN etc so we can remove the
> local definitions of these from the bpa-rs600 driver.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>  drivers/hwmon/pmbus/bpa-rs600.c | 25 ++++++++-----------------
>  1 file changed, 8 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/bpa-rs600.c b/drivers/hwmon/pmbus/bpa-rs600.c
> index d205b41540ce..d495faa89799 100644
> --- a/drivers/hwmon/pmbus/bpa-rs600.c
> +++ b/drivers/hwmon/pmbus/bpa-rs600.c
> @@ -12,15 +12,6 @@
>  #include <linux/pmbus.h>
>  #include "pmbus.h"
>  
> -#define BPARS600_MFR_VIN_MIN	0xa0
> -#define BPARS600_MFR_VIN_MAX	0xa1
> -#define BPARS600_MFR_IIN_MAX	0xa2
> -#define BPARS600_MFR_PIN_MAX	0xa3
> -#define BPARS600_MFR_VOUT_MIN	0xa4
> -#define BPARS600_MFR_VOUT_MAX	0xa5
> -#define BPARS600_MFR_IOUT_MAX	0xa6
> -#define BPARS600_MFR_POUT_MAX	0xa7
> -
>  enum chips { bpa_rs600, bpd_rs600 };
>  
>  static int bpa_rs600_read_byte_data(struct i2c_client *client, int page, int reg)
> @@ -83,28 +74,28 @@ static int bpa_rs600_read_word_data(struct i2c_client *client, int page, int pha
>  
>  	switch (reg) {
>  	case PMBUS_VIN_UV_WARN_LIMIT:
> -		ret = pmbus_read_word_data(client, 0, 0xff, BPARS600_MFR_VIN_MIN);
> +		ret = pmbus_read_word_data(client, 0, 0xff, PMBUS_MFR_VIN_MIN);
>  		break;
>  	case PMBUS_VIN_OV_WARN_LIMIT:
> -		ret = pmbus_read_word_data(client, 0, 0xff, BPARS600_MFR_VIN_MAX);
> +		ret = pmbus_read_word_data(client, 0, 0xff, PMBUS_MFR_VIN_MAX);
>  		break;
>  	case PMBUS_VOUT_UV_WARN_LIMIT:
> -		ret = pmbus_read_word_data(client, 0, 0xff, BPARS600_MFR_VOUT_MIN);
> +		ret = pmbus_read_word_data(client, 0, 0xff, PMBUS_MFR_VOUT_MIN);
>  		break;
>  	case PMBUS_VOUT_OV_WARN_LIMIT:
> -		ret = pmbus_read_word_data(client, 0, 0xff, BPARS600_MFR_VOUT_MAX);
> +		ret = pmbus_read_word_data(client, 0, 0xff, PMBUS_MFR_VOUT_MAX);
>  		break;
>  	case PMBUS_IIN_OC_WARN_LIMIT:
> -		ret = pmbus_read_word_data(client, 0, 0xff, BPARS600_MFR_IIN_MAX);
> +		ret = pmbus_read_word_data(client, 0, 0xff, PMBUS_MFR_IIN_MAX);
>  		break;
>  	case PMBUS_IOUT_OC_WARN_LIMIT:
> -		ret = pmbus_read_word_data(client, 0, 0xff, BPARS600_MFR_IOUT_MAX);
> +		ret = pmbus_read_word_data(client, 0, 0xff, PMBUS_MFR_IOUT_MAX);
>  		break;
>  	case PMBUS_PIN_OP_WARN_LIMIT:
> -		ret = pmbus_read_word_data(client, 0, 0xff, BPARS600_MFR_PIN_MAX);
> +		ret = pmbus_read_word_data(client, 0, 0xff, PMBUS_MFR_PIN_MAX);
>  		break;
>  	case PMBUS_POUT_OP_WARN_LIMIT:
> -		ret = pmbus_read_word_data(client, 0, 0xff, BPARS600_MFR_POUT_MAX);
> +		ret = pmbus_read_word_data(client, 0, 0xff, PMBUS_MFR_POUT_MAX);

If the above is correct, the driver reports the wrong attributes. For example,
PMBUS_MFR_PIN_MAX is supposed to report the rated limit, not the warning limit.
What does the datasheet say ?

Guenter

>  		break;
>  	case PMBUS_VIN_UV_FAULT_LIMIT:
>  	case PMBUS_VIN_OV_FAULT_LIMIT:
> -- 
> 2.32.0
> 

  reply	other threads:[~2021-08-11 19:53 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-11  4:17 [PATCH 0/2] hwmon: (pmbus/bpa-rs600) cleanup and workaround Chris Packham
2021-08-11  4:17 ` [PATCH 1/2] hwmon: (pmbus/bpa-rs600) Remove duplicate defininitions Chris Packham
2021-08-11 19:53   ` Guenter Roeck [this message]
2021-08-11 21:58     ` Chris Packham
2021-08-11  4:17 ` [PATCH 2/2] hwmon: (pmbus/bpa-rs600) Add workaround for incorrect Pin max Chris Packham
2021-08-11 19:53   ` Guenter Roeck
2021-08-11 22:19     ` Chris Packham
2021-08-11 23:18       ` Guenter Roeck
2021-08-11 23:25         ` Chris Packham
2021-08-12  0:41           ` Guenter Roeck
2021-08-12  3:15 ` [PATCH 0/2] hwmon: (pmbus/bpa-rs600) cleanup and workaround Chris Packham
2021-08-12  4:29   ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210811195346.GA966404@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=chris.packham@alliedtelesis.co.nz \
    --cc=jdelvare@suse.com \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.