[<prev] [next>] [day] [month] [year] [list]
Message-ID: <20200624163823.GA215518@roeck-us.net>
Date: Wed, 24 Jun 2020 09:38:23 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Chu Lin <linchuyuan@...gle.com>
Cc: Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org,
Jason Ling <jasonling@...gle.com>,
Kais Belgaied <belgaied@...gle.com>,
Zhongqi Li <zhongqil@...gle.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] hwmon:max6697: Make sure the OVERT mask is set correctly
On Tue, Jun 23, 2020 at 10:13:08PM +0000, Chu Lin wrote:
> Per the datasheet for max6697, OVERT mask and ALERT mask is different.
> For example, the 7th bit of OVERT is the local channel but for alert
> mask, the 6th bit is the local channel. Therefore, we can't apply the
> same mask for both reg. In addition to that, max6697 driver is suppose
> to be compatibale with different models. I mannually went over all the
> listed chip and made sure all the chip type has the same layout.
>
> Testing;
> mask value of 0x9 should map to 0x44 for ALERT and 0x84 for OVERT.
> I used iotool to read the reg value back to verify. I only tested this
> change on max6581
>
> Reference:
> https://datasheets.maximintegrated.com/en/ds/MAX6581.pdf
> https://datasheets.maximintegrated.com/en/ds/MAX6697.pdf
> https://datasheets.maximintegrated.com/en/ds/MAX6699.pdf
>
> Signed-off-by: Chu Lin <linchuyuan@...gle.com>
Nice catch.
Applied, thanks
Guenter
> ---
> drivers/hwmon/max6697.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hwmon/max6697.c b/drivers/hwmon/max6697.c
> index 743752a2467a..64122eb38060 100644
> --- a/drivers/hwmon/max6697.c
> +++ b/drivers/hwmon/max6697.c
> @@ -38,8 +38,9 @@ static const u8 MAX6697_REG_CRIT[] = {
> * Map device tree / platform data register bit map to chip bit map.
> * Applies to alert register and over-temperature register.
> */
> -#define MAX6697_MAP_BITS(reg) ((((reg) & 0x7e) >> 1) | \
> +#define MAX6697_ALERT_MAP_BITS(reg) ((((reg) & 0x7e) >> 1) | \
> (((reg) & 0x01) << 6) | ((reg) & 0x80))
> +#define MAX6697_OVERT_MAP_BITS(reg) (((reg) >> 1) | (((reg) & 0x01) << 7))
>
> #define MAX6697_REG_STAT(n) (0x44 + (n))
>
> @@ -562,12 +563,12 @@ static int max6697_init_chip(struct max6697_data *data,
> return ret;
>
> ret = i2c_smbus_write_byte_data(client, MAX6697_REG_ALERT_MASK,
> - MAX6697_MAP_BITS(pdata->alert_mask));
> + MAX6697_ALERT_MAP_BITS(pdata->alert_mask));
> if (ret < 0)
> return ret;
>
> ret = i2c_smbus_write_byte_data(client, MAX6697_REG_OVERT_MASK,
> - MAX6697_MAP_BITS(pdata->over_temperature_mask));
> + MAX6697_OVERT_MAP_BITS(pdata->over_temperature_mask));
> if (ret < 0)
> return ret;
>
> --
> 2.27.0.111.gc72c7da667-goog
>
Powered by blists - more mailing lists