linux-kernel - Re: [PATCH 1/2] hwmon: (pmbus) Add a NO_PEC flag to probe chips with faulty CAPABILITY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f96ded73-93de-dc7d-36af-7f100e71817a@roeck-us.net>
Date:   Mon, 21 Dec 2020 08:54:13 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     Eddie James <eajames@...ux.ibm.com>, linux-hwmon@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org, jdelvare@...e.com, bjwyman@...il.com
Subject: Re: [PATCH 1/2] hwmon: (pmbus) Add a NO_PEC flag to probe chips with
 faulty CAPABILITY

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@...ux.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 */
>  
>