linux-hwmon.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] hwmon: (pmbus) Add a NO_PEC flag to probe chips with faulty CAPABILITY
@ 2020-12-21 16:30 Eddie James
  2020-12-21 16:30 ` [PATCH 1/2] " Eddie James
  2020-12-21 16:30 ` [PATCH 2/2] hwmon: (pmbus/ibm-cffps) Set the PMBUS_NO_PEC flag Eddie James
  0 siblings, 2 replies; 6+ messages in thread
From: Eddie James @ 2020-12-21 16:30 UTC (permalink / raw)
  To: linux-hwmon; +Cc: linux-kernel, jdelvare, linux, eajames, bjwyman

Some PMBus chips don't respond with valid data when reading the
CAPABILITY register. For instance the register may report that the
chip supports PEC when in reality it does not. For such chips, PEC
must not be enabled while probing the chip, so this series adds a flag
that allows device drivers to force PEC off. The second patch enables
this flag for the IBM CFFPS driver, which supports power supplies that
report invalid in the CAPABILITY register and must therefore force PEC
off.

Eddie James (2):
  hwmon: (pmbus) Add a NO_PEC flag to probe chips with faulty CAPABILITY
  hwmon: (pmbus/ibm-cffps) Set the PMBUS_NO_PEC flag

 drivers/hwmon/pmbus/ibm-cffps.c  |  2 +-
 drivers/hwmon/pmbus/pmbus_core.c |  8 +++++---
 include/linux/pmbus.h            | 10 ++++++++++
 3 files changed, 16 insertions(+), 4 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] hwmon: (pmbus) Add a NO_PEC flag to probe chips with faulty CAPABILITY
  2020-12-21 16:30 [PATCH 0/2] hwmon: (pmbus) Add a NO_PEC flag to probe chips with faulty CAPABILITY Eddie James
@ 2020-12-21 16:30 ` Eddie James
  2020-12-21 16:54   ` Guenter Roeck
  2020-12-21 16:30 ` [PATCH 2/2] hwmon: (pmbus/ibm-cffps) Set the PMBUS_NO_PEC flag Eddie James
  1 sibling, 1 reply; 6+ messages in thread
From: Eddie James @ 2020-12-21 16:30 UTC (permalink / raw)
  To: linux-hwmon; +Cc: linux-kernel, jdelvare, linux, eajames, bjwyman

Some PMBus chips don't respond with valid data when reading the
CAPABILITY register. For instance the register may report that the
chip supports PEC when in reality it does not. For such chips, PEC
must not be enabled while probing the chip, so add a flag so that
device drivers can force PEC off.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/hwmon/pmbus/pmbus_core.c |  8 +++++---
 include/linux/pmbus.h            | 10 ++++++++++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 192442b3b7a2..3de1657dde35 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -2204,9 +2204,11 @@ static int pmbus_init_common(struct i2c_client *client, struct pmbus_data *data,
 	}
 
 	/* Enable PEC if the controller supports it */
-	ret = i2c_smbus_read_byte_data(client, PMBUS_CAPABILITY);
-	if (ret >= 0 && (ret & PB_CAPABILITY_ERROR_CHECK))
-		client->flags |= I2C_CLIENT_PEC;
+	if (!(data->flags & PMBUS_NO_PEC)) {
+		ret = i2c_smbus_read_byte_data(client, PMBUS_CAPABILITY);
+		if (ret >= 0 && (ret & PB_CAPABILITY_ERROR_CHECK))
+			client->flags |= I2C_CLIENT_PEC;
+	}
 
 	/*
 	 * Check if the chip is write protected. If it is, we can not clear
diff --git a/include/linux/pmbus.h b/include/linux/pmbus.h
index 1ea5bae708a1..9bdc8a581b03 100644
--- a/include/linux/pmbus.h
+++ b/include/linux/pmbus.h
@@ -34,6 +34,16 @@
  */
 #define PMBUS_WRITE_PROTECTED	BIT(1)
 
+/*
+ * PMBUS_NO_PEC
+ *
+ * Some PMBus chips don't respond with valid data when reading the CAPABILITY
+ * register. In this case, the register may report that the chip supports PEC
+ * with bit 7 (PB_CAPABILITY_ERROR_CHECK) when in reality it's not supported.
+ * For such chips, PEC must not be enabled before probing the chip.
+ */
+#define PMBUS_NO_PEC			BIT(2)
+
 struct pmbus_platform_data {
 	u32 flags;		/* Device specific flags */
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] hwmon: (pmbus/ibm-cffps) Set the PMBUS_NO_PEC flag
  2020-12-21 16:30 [PATCH 0/2] hwmon: (pmbus) Add a NO_PEC flag to probe chips with faulty CAPABILITY Eddie James
  2020-12-21 16:30 ` [PATCH 1/2] " Eddie James
@ 2020-12-21 16:30 ` Eddie James
  1 sibling, 0 replies; 6+ messages in thread
From: Eddie James @ 2020-12-21 16:30 UTC (permalink / raw)
  To: linux-hwmon; +Cc: linux-kernel, jdelvare, linux, eajames, bjwyman

Several power supplies supported by the IBM CFFPS driver don't
report valid data in the CAPABILITY register, or support PEC only
for certain PMBus registers. Since the automatic version detection
of the driver might fail on some supplies with PEC enabled, just
disable PEC entirely for this driver.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/hwmon/pmbus/ibm-cffps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/ibm-cffps.c b/drivers/hwmon/pmbus/ibm-cffps.c
index d6bbbb223871..f8e3ae989e99 100644
--- a/drivers/hwmon/pmbus/ibm-cffps.c
+++ b/drivers/hwmon/pmbus/ibm-cffps.c
@@ -472,7 +472,7 @@ static struct pmbus_driver_info ibm_cffps_info[] = {
 };
 
 static struct pmbus_platform_data ibm_cffps_pdata = {
-	.flags = PMBUS_SKIP_STATUS_CHECK,
+	.flags = PMBUS_SKIP_STATUS_CHECK | PMBUS_NO_PEC,
 };
 
 static int ibm_cffps_probe(struct i2c_client *client)
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] hwmon: (pmbus) Add a NO_PEC flag to probe chips with faulty CAPABILITY
  2020-12-21 16:30 ` [PATCH 1/2] " Eddie James
@ 2020-12-21 16:54   ` Guenter Roeck
  2020-12-21 18:32     ` Eddie James
  0 siblings, 1 reply; 6+ messages in thread
From: Guenter Roeck @ 2020-12-21 16:54 UTC (permalink / raw)
  To: Eddie James, linux-hwmon; +Cc: linux-kernel, jdelvare, bjwyman

On 12/21/20 8:30 AM, Eddie James wrote:
> Some PMBus chips don't respond with valid data when reading the
> CAPABILITY register. For instance the register may report that the
> chip supports PEC when in reality it does not. For such chips, PEC
> must not be enabled while probing the chip, so add a flag so that
> device drivers can force PEC off.
> 

I think the flag should indicate that the capability register
shall not be read/used. That the capability register is currently
only used to check for PEC is secondary. We might,for example,
start using it to check for alert support or to check the numeric
format.

Thanks,
Guenter

> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>  drivers/hwmon/pmbus/pmbus_core.c |  8 +++++---
>  include/linux/pmbus.h            | 10 ++++++++++
>  2 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 192442b3b7a2..3de1657dde35 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -2204,9 +2204,11 @@ static int pmbus_init_common(struct i2c_client *client, struct pmbus_data *data,
>  	}
>  
>  	/* Enable PEC if the controller supports it */
> -	ret = i2c_smbus_read_byte_data(client, PMBUS_CAPABILITY);
> -	if (ret >= 0 && (ret & PB_CAPABILITY_ERROR_CHECK))
> -		client->flags |= I2C_CLIENT_PEC;
> +	if (!(data->flags & PMBUS_NO_PEC)) {
> +		ret = i2c_smbus_read_byte_data(client, PMBUS_CAPABILITY);
> +		if (ret >= 0 && (ret & PB_CAPABILITY_ERROR_CHECK))
> +			client->flags |= I2C_CLIENT_PEC;
> +	}
>  
>  	/*
>  	 * Check if the chip is write protected. If it is, we can not clear
> diff --git a/include/linux/pmbus.h b/include/linux/pmbus.h
> index 1ea5bae708a1..9bdc8a581b03 100644
> --- a/include/linux/pmbus.h
> +++ b/include/linux/pmbus.h
> @@ -34,6 +34,16 @@
>   */
>  #define PMBUS_WRITE_PROTECTED	BIT(1)
>  
> +/*
> + * PMBUS_NO_PEC
> + *
> + * Some PMBus chips don't respond with valid data when reading the CAPABILITY
> + * register. In this case, the register may report that the chip supports PEC
> + * with bit 7 (PB_CAPABILITY_ERROR_CHECK) when in reality it's not supported.
> + * For such chips, PEC must not be enabled before probing the chip.
> + */
> +#define PMBUS_NO_PEC			BIT(2)
> +
>  struct pmbus_platform_data {
>  	u32 flags;		/* Device specific flags */
>  
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] hwmon: (pmbus) Add a NO_PEC flag to probe chips with faulty CAPABILITY
  2020-12-21 16:54   ` Guenter Roeck
@ 2020-12-21 18:32     ` Eddie James
  2020-12-21 18:53       ` Guenter Roeck
  0 siblings, 1 reply; 6+ messages in thread
From: Eddie James @ 2020-12-21 18:32 UTC (permalink / raw)
  To: Guenter Roeck, linux-hwmon; +Cc: linux-kernel, jdelvare, bjwyman

On Mon, 2020-12-21 at 08:54 -0800, Guenter Roeck wrote:
> On 12/21/20 8:30 AM, Eddie James wrote:
> > Some PMBus chips don't respond with valid data when reading the
> > CAPABILITY register. For instance the register may report that the
> > chip supports PEC when in reality it does not. For such chips, PEC
> > must not be enabled while probing the chip, so add a flag so that
> > device drivers can force PEC off.
> > 
> 
> I think the flag should indicate that the capability register
> shall not be read/used. That the capability register is currently
> only used to check for PEC is secondary. We might,for example,
> start using it to check for alert support or to check the numeric
> format.

OK, that makes sense. I'll rename the flag in v2, how does
PMBUS_NO_CAPABILITY sound?

Thanks for the quick reply,
Eddie

> 
> Thanks,
> Guenter
> 
> > Signed-off-by: Eddie James <eajames@linux.ibm.com>
> > ---
> >  drivers/hwmon/pmbus/pmbus_core.c |  8 +++++---
> >  include/linux/pmbus.h            | 10 ++++++++++
> >  2 files changed, 15 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/hwmon/pmbus/pmbus_core.c
> > b/drivers/hwmon/pmbus/pmbus_core.c
> > index 192442b3b7a2..3de1657dde35 100644
> > --- a/drivers/hwmon/pmbus/pmbus_core.c
> > +++ b/drivers/hwmon/pmbus/pmbus_core.c
> > @@ -2204,9 +2204,11 @@ static int pmbus_init_common(struct
> > i2c_client *client, struct pmbus_data *data,
> >  	}
> >  
> >  	/* Enable PEC if the controller supports it */
> > -	ret = i2c_smbus_read_byte_data(client, PMBUS_CAPABILITY);
> > -	if (ret >= 0 && (ret & PB_CAPABILITY_ERROR_CHECK))
> > -		client->flags |= I2C_CLIENT_PEC;
> > +	if (!(data->flags & PMBUS_NO_PEC)) {
> > +		ret = i2c_smbus_read_byte_data(client,
> > PMBUS_CAPABILITY);
> > +		if (ret >= 0 && (ret & PB_CAPABILITY_ERROR_CHECK))
> > +			client->flags |= I2C_CLIENT_PEC;
> > +	}
> >  
> >  	/*
> >  	 * Check if the chip is write protected. If it is, we can not
> > clear
> > diff --git a/include/linux/pmbus.h b/include/linux/pmbus.h
> > index 1ea5bae708a1..9bdc8a581b03 100644
> > --- a/include/linux/pmbus.h
> > +++ b/include/linux/pmbus.h
> > @@ -34,6 +34,16 @@
> >   */
> >  #define PMBUS_WRITE_PROTECTED	BIT(1)
> >  
> > +/*
> > + * PMBUS_NO_PEC
> > + *
> > + * Some PMBus chips don't respond with valid data when reading the
> > CAPABILITY
> > + * register. In this case, the register may report that the chip
> > supports PEC
> > + * with bit 7 (PB_CAPABILITY_ERROR_CHECK) when in reality it's not
> > supported.
> > + * For such chips, PEC must not be enabled before probing the
> > chip.
> > + */
> > +#define PMBUS_NO_PEC			BIT(2)
> > +
> >  struct pmbus_platform_data {
> >  	u32 flags;		/* Device specific flags */
> >  
> > 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] hwmon: (pmbus) Add a NO_PEC flag to probe chips with faulty CAPABILITY
  2020-12-21 18:32     ` Eddie James
@ 2020-12-21 18:53       ` Guenter Roeck
  0 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2020-12-21 18:53 UTC (permalink / raw)
  To: Eddie James, linux-hwmon; +Cc: linux-kernel, jdelvare, bjwyman

On 12/21/20 10:32 AM, Eddie James wrote:
> On Mon, 2020-12-21 at 08:54 -0800, Guenter Roeck wrote:
>> On 12/21/20 8:30 AM, Eddie James wrote:
>>> Some PMBus chips don't respond with valid data when reading the
>>> CAPABILITY register. For instance the register may report that the
>>> chip supports PEC when in reality it does not. For such chips, PEC
>>> must not be enabled while probing the chip, so add a flag so that
>>> device drivers can force PEC off.
>>>
>>
>> I think the flag should indicate that the capability register
>> shall not be read/used. That the capability register is currently
>> only used to check for PEC is secondary. We might,for example,
>> start using it to check for alert support or to check the numeric
>> format.
> 
> OK, that makes sense. I'll rename the flag in v2, how does
> PMBUS_NO_CAPABILITY sound?
> 

sgtm

Thanks,
Guenter

> Thanks for the quick reply,
> Eddie
> 
>>
>> Thanks,
>> Guenter
>>
>>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>>> ---
>>>  drivers/hwmon/pmbus/pmbus_core.c |  8 +++++---
>>>  include/linux/pmbus.h            | 10 ++++++++++
>>>  2 files changed, 15 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/hwmon/pmbus/pmbus_core.c
>>> b/drivers/hwmon/pmbus/pmbus_core.c
>>> index 192442b3b7a2..3de1657dde35 100644
>>> --- a/drivers/hwmon/pmbus/pmbus_core.c
>>> +++ b/drivers/hwmon/pmbus/pmbus_core.c
>>> @@ -2204,9 +2204,11 @@ static int pmbus_init_common(struct
>>> i2c_client *client, struct pmbus_data *data,
>>>  	}
>>>  
>>>  	/* Enable PEC if the controller supports it */
>>> -	ret = i2c_smbus_read_byte_data(client, PMBUS_CAPABILITY);
>>> -	if (ret >= 0 && (ret & PB_CAPABILITY_ERROR_CHECK))
>>> -		client->flags |= I2C_CLIENT_PEC;
>>> +	if (!(data->flags & PMBUS_NO_PEC)) {
>>> +		ret = i2c_smbus_read_byte_data(client,
>>> PMBUS_CAPABILITY);
>>> +		if (ret >= 0 && (ret & PB_CAPABILITY_ERROR_CHECK))
>>> +			client->flags |= I2C_CLIENT_PEC;
>>> +	}
>>>  
>>>  	/*
>>>  	 * Check if the chip is write protected. If it is, we can not
>>> clear
>>> diff --git a/include/linux/pmbus.h b/include/linux/pmbus.h
>>> index 1ea5bae708a1..9bdc8a581b03 100644
>>> --- a/include/linux/pmbus.h
>>> +++ b/include/linux/pmbus.h
>>> @@ -34,6 +34,16 @@
>>>   */
>>>  #define PMBUS_WRITE_PROTECTED	BIT(1)
>>>  
>>> +/*
>>> + * PMBUS_NO_PEC
>>> + *
>>> + * Some PMBus chips don't respond with valid data when reading the
>>> CAPABILITY
>>> + * register. In this case, the register may report that the chip
>>> supports PEC
>>> + * with bit 7 (PB_CAPABILITY_ERROR_CHECK) when in reality it's not
>>> supported.
>>> + * For such chips, PEC must not be enabled before probing the
>>> chip.
>>> + */
>>> +#define PMBUS_NO_PEC			BIT(2)
>>> +
>>>  struct pmbus_platform_data {
>>>  	u32 flags;		/* Device specific flags */
>>>  
>>>
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-12-21 18:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-21 16:30 [PATCH 0/2] hwmon: (pmbus) Add a NO_PEC flag to probe chips with faulty CAPABILITY Eddie James
2020-12-21 16:30 ` [PATCH 1/2] " Eddie James
2020-12-21 16:54   ` Guenter Roeck
2020-12-21 18:32     ` Eddie James
2020-12-21 18:53       ` Guenter Roeck
2020-12-21 16:30 ` [PATCH 2/2] hwmon: (pmbus/ibm-cffps) Set the PMBUS_NO_PEC flag Eddie James

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).