[PATCH] hwmon: (nct6775) Fix incorrect parenthesization in nct6775_write_fan_div()

[PATCH] hwmon: (nct6775) Fix incorrect parenthesization in nct6775_write_fan_div()

[Zev Weiss]

Commit 4ef2774511dc ("hwmon: (nct6775) Convert register access to
regmap API") fumbled the shifting & masking of the fan_div values such
that odd-numbered fan divisors would always be set to zero.  Fix it so
that we actually OR in the bits we meant to.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
Fixes: 4ef2774511dc ("hwmon: (nct6775) Convert register access to regmap API")
Cc: stable@kernel.org # v5.19+
---

This bug got noticed during review [0] and the fix was incorporated
into v3 of the patch series [1], but then I somehow managed to
accidentally revert the fix in v4 [2] that actually got applied.

I'm fairly confident this is correct, but I don't have any nct6775
hardware to test on (this code path is only used on that specific
chip, not others in the family).  If any one who does can try it out
and report back that would of course be welcome.

[0] https://lore.kernel.org/linux-hwmon/YiiMfJV3bjUmoUcV@hatter.bewilderbeest.net/
[1] https://lore.kernel.org/linux-hwmon/20220426071848.11619-3-zev@bewilderbeest.net/
[2] https://lore.kernel.org/linux-hwmon/20220427010154.29749-3-zev@bewilderbeest.net/

 drivers/hwmon/nct6775-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/nct6775-core.c b/drivers/hwmon/nct6775-core.c
index da9ec6983e13..c54233f0369b 100644
--- a/drivers/hwmon/nct6775-core.c
+++ b/drivers/hwmon/nct6775-core.c
@@ -1150,7 +1150,7 @@ static int nct6775_write_fan_div(struct nct6775_data *data, int nr)
 	if (err)
 		return err;
 	reg &= 0x70 >> oddshift;
-	reg |= data->fan_div[nr] & (0x7 << oddshift);
+	reg |= (data->fan_div[nr] & 0x7) << oddshift;
 	return nct6775_write_value(data, fandiv_reg, reg);
 }
 
-- 
2.39.0

Re: [PATCH] hwmon: (nct6775) Fix incorrect parenthesization in nct6775_write_fan_div()

[Guenter Roeck]

On Mon, Jan 02, 2023 at 01:28:57PM -0800, Zev Weiss wrote:
> Commit 4ef2774511dc ("hwmon: (nct6775) Convert register access to
> regmap API") fumbled the shifting & masking of the fan_div values such
> that odd-numbered fan divisors would always be set to zero.  Fix it so
> that we actually OR in the bits we meant to.
> 
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> Fixes: 4ef2774511dc ("hwmon: (nct6775) Convert register access to regmap API")
> Cc: stable@kernel.org # v5.19+

Applied.

Thanks,
Guenter

> ---
> 
> This bug got noticed during review [0] and the fix was incorporated
> into v3 of the patch series [1], but then I somehow managed to
> accidentally revert the fix in v4 [2] that actually got applied.
> 
> I'm fairly confident this is correct, but I don't have any nct6775
> hardware to test on (this code path is only used on that specific
> chip, not others in the family).  If any one who does can try it out
> and report back that would of course be welcome.
> 
> [0] https://lore.kernel.org/linux-hwmon/YiiMfJV3bjUmoUcV@hatter.bewilderbeest.net/
> [1] https://lore.kernel.org/linux-hwmon/20220426071848.11619-3-zev@bewilderbeest.net/
> [2] https://lore.kernel.org/linux-hwmon/20220427010154.29749-3-zev@bewilderbeest.net/
> 
>  drivers/hwmon/nct6775-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/nct6775-core.c b/drivers/hwmon/nct6775-core.c
> index da9ec6983e13..c54233f0369b 100644
> --- a/drivers/hwmon/nct6775-core.c
> +++ b/drivers/hwmon/nct6775-core.c
> @@ -1150,7 +1150,7 @@ static int nct6775_write_fan_div(struct nct6775_data *data, int nr)
>  	if (err)
>  		return err;
>  	reg &= 0x70 >> oddshift;
> -	reg |= data->fan_div[nr] & (0x7 << oddshift);
> +	reg |= (data->fan_div[nr] & 0x7) << oddshift;
>  	return nct6775_write_value(data, fandiv_reg, reg);
>  }
>