On Sun, May 07, 2023 at 08:41:05PM +1000, Frank Crawford wrote:
> Add feature flag FEAT_FANCTL_ONOFF for chips that support configuration
> bits for management of fan control off.
>
> Signed-off-by: Frank Crawford <frank@xxxxxxxxxxxxxxxxxx>
This patch would leave the driver broken until the next patch is applied,
which is a no-go.
You have a number of options:
- Negate the flag and apply it to 8603 only to minimize the changes,
and make all changes in a single patch.
- Split the two patches into three, first to introduce the flag,
then apply it to affected chips, then make the actual code
changes.
- Combine the two patches into one.
Guenter
> ---
> drivers/hwmon/it87.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
> index 4c3641d28a6a..6fa9e928177e 100644
> --- a/drivers/hwmon/it87.c
> +++ b/drivers/hwmon/it87.c
> @@ -320,6 +320,7 @@ struct it87_devices {
> #define FEAT_FOUR_FANS BIT(20) /* Supports four fans */
> #define FEAT_FOUR_PWM BIT(21) /* Supports four fan controls */
> #define FEAT_FOUR_TEMP BIT(22)
> +#define FEAT_FANCTL_ONOFF BIT(23) /* chip has FAN_CTL ON/OFF */
>
> static const struct it87_devices it87_devices[] = {
> [it87] = {
> @@ -534,6 +535,7 @@ static const struct it87_devices it87_devices[] = {
> #define has_conf_noexit(data) ((data)->features & FEAT_CONF_NOEXIT)
> #define has_scaling(data) ((data)->features & (FEAT_12MV_ADC | \
> FEAT_10_9MV_ADC))
> +#define has_fanctl_onoff(data) ((data)->features & FEAT_FANCTL_ONOFF)
>
> struct it87_sio_data {
> int sioaddr;
> @@ -1240,11 +1242,12 @@ static SENSOR_DEVICE_ATTR(temp3_type, S_IRUGO | S_IWUSR, show_temp_type,
>
> static int pwm_mode(const struct it87_data *data, int nr)
> {
> - if (data->type != it8603 && nr < 3 && !(data->fan_main_ctrl & BIT(nr)))
> - return 0; /* Full speed */
> + if (has_fanctl_onoff(data) && nr < 3 &&
> + !(data->fan_main_ctrl & BIT(nr)))
> + return 0; /* Full speed */
> if (data->pwm_ctrl[nr] & 0x80)
> - return 2; /* Automatic mode */
> - if ((data->type == it8603 || nr >= 3) &&
> + return 2; /* Automatic mode */
> + if ((!has_fanctl_onoff(data) || nr >= 3) &&
> data->pwm_duty[nr] == pwm_to_reg(data, 0xff))
> return 0; /* Full speed */
>
> @@ -1481,7 +1484,7 @@ static ssize_t set_pwm_enable(struct device *dev, struct device_attribute *attr,
> return err;
>
> if (val == 0) {
> - if (nr < 3 && data->type != it8603) {
> + if (nr < 3 && has_fanctl_onoff(data)) {
> int tmp;
> /* make sure the fan is on when in on/off mode */
> tmp = it87_read_value(data, IT87_REG_FAN_CTL);
> @@ -1521,7 +1524,7 @@ static ssize_t set_pwm_enable(struct device *dev, struct device_attribute *attr,
> data->pwm_ctrl[nr] = ctrl;
> it87_write_value(data, IT87_REG_PWM[nr], ctrl);
>
> - if (data->type != it8603 && nr < 3) {
> + if (has_fanctl_onoff(data) && nr < 3) {
> /* set SmartGuardian mode */
> data->fan_main_ctrl |= BIT(nr);
> it87_write_value(data, IT87_REG_FAN_MAIN_CTRL,
