hwmon: (npcm750-pwm-fan) serialize PWM register updates

npcm7xx_pwm_config_set() writes the channel's CMR register and then
does a read-modify-write of the CR register that holds the enable and
invert bits of all four channels of a PWM module. It is called from the
hwmon pwm write path and from npcm7xx_pwm_cz_set_cur_state(), the
set_cur_state callback of the thermal cooling devices the driver
registers for channels with "cooling-levels".

The per-module pwm_lock serializing these updates was dropped on the
assumption that the hwmon core serializes all attribute accesses. That
only covers the hwmon attributes: the cooling devices are registered
directly with the thermal core, which only holds the lock of the
cooling device being updated. So a thermal zone update or a write to a
cooling device's cur_state can run concurrently with a pwm write, or
with the cooling device of another channel of the same module, and one
CR update overwrites the other:

    CPU0 (pwmN write)               CPU1 (cooling dev, same module)
    tmp = ioread32(CR)
                                    tmp = ioread32(CR)
                                    tmp |= ch1 enable
                                    iowrite32(tmp, CR)
    tmp |= ch0 enable
    iowrite32(tmp, CR)              /* ch1 enable bit lost */

leaving a fan stopped or running at a duty cycle that no longer matches
its enable state.

The bug was found by code inspection and has not been reproduced.

Fix by restoring a per-module mutex around the register updates; a
driver lock is used instead of hwmon_lock() because the cooling devices
are registered before the hwmon device exists.

Fixes: abfb050851 ("hwmon: (corsair-psu) Rely on subsystem locking")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
This commit is contained in:
Josef Bacik 2026-10-03 18:16:42 +00:00 committed by Guenter Roeck
parent b8249a3b6b
commit e1b3c2630d

View File

@ -7,6 +7,7 @@
#include <linux/interrupt.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/of_address.h>
#include <linux/of_irq.h>
#include <linux/platform_device.h>
@ -197,6 +198,8 @@ struct npcm7xx_pwm_fan_data {
int pwm_modules;
struct clk *pwm_clk;
struct clk *fan_clk;
/* serializes CMR/CR updates from hwmon and the cooling devices */
struct mutex pwm_lock[NPCM7XX_PWM_MAX_MODULES];
spinlock_t fan_lock[NPCM7XX_FAN_MAX_MODULE];
int fan_irq[NPCM7XX_FAN_MAX_MODULE];
bool pwm_present[NPCM7XX_PWM_MAX_CHN_NUM];
@ -216,6 +219,8 @@ static int npcm7xx_pwm_config_set(struct npcm7xx_pwm_fan_data *data,
u32 module = (channel / NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
u32 tmp_buf, ctrl_en_bit, env_bit;
guard(mutex)(&data->pwm_lock[module]);
/*
* Config PWM Comparator register for setting duty cycle
*/
@ -986,6 +991,12 @@ static int npcm7xx_pwm_fan_probe(struct platform_device *pdev)
output_freq = npcm7xx_pwm_init(data);
npcm7xx_fan_init(data);
for (i = 0; i < ARRAY_SIZE(data->pwm_lock); i++) {
ret = devm_mutex_init(dev, &data->pwm_lock[i]);
if (ret)
return ret;
}
for (i = 0; i < NPCM7XX_FAN_MAX_MODULE; i++) {
spin_lock_init(&data->fan_lock[i]);