On Wed, Aug 09, 2017 at 12:05:30PM +0200, Maciej S. Szmigiero wrote:
> This commit splits out chip registers setting code on probe path to
> separate functions so they can be reused for setting the device properly
> again when system resumes from suspend.
> 
> While we are at it let's also make clear that on IT8720 and IT8782 it's
> the VCCH5V line that is (possibly) routed to in7.
> This will make it consistent with a similar message that it printed on
> IT8783.
> 
> Signed-off-by: Maciej S. Szmigiero <m...@maciej.szmigiero.name>

Applied to hwmon-next.

Thanks,
Guenter

> ---
> Changes from v1: Move code of common probe / resume steps to new functions
> so we don't need to make large parts of probe function conditional on a
> newly added 'resume' parameter.
> 
> Changes from v2: Code move of common probe / resume steps to new functions
> and actual resume functionality split into two, separate patches.
> 
> Made a message about VCCH5V being routed to in7 consistent across all
> chips.
> 
> Changes from v3, v4: Fix patch formatting.
> 
>  drivers/hwmon/it87.c | 138 
> ++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 88 insertions(+), 50 deletions(-)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
> index 4dfc7238313e..818f123ac475 100644
> --- a/drivers/hwmon/it87.c
> +++ b/drivers/hwmon/it87.c
> @@ -2761,13 +2761,13 @@ static int __init it87_find(int sioaddr, unsigned 
> short *address,
>               uart6 = sio_data->type == it8782 && (reg & BIT(2));
>  
>               /*
> -              * The IT8720F has no VIN7 pin, so VCCH should always be
> +              * The IT8720F has no VIN7 pin, so VCCH5V should always be
>                * routed internally to VIN7 with an internal divider.
>                * Curiously, there still is a configuration bit to control
>                * this, which means it can be set incorrectly. And even
>                * more curiously, many boards out there are improperly
>                * configured, even though the IT8720F datasheet claims
> -              * that the internal routing of VCCH to VIN7 is the default
> +              * that the internal routing of VCCH5V to VIN7 is the default
>                * setting. So we force the internal routing in this case.
>                *
>                * On IT8782F, VIN7 is multiplexed with one of the UART6 pins.
> @@ -2777,7 +2777,7 @@ static int __init it87_find(int sioaddr, unsigned short 
> *address,
>               if ((sio_data->type == it8720 || uart6) && !(reg & BIT(1))) {
>                       reg |= BIT(1);
>                       superio_outb(sioaddr, IT87_SIO_PINX2_REG, reg);
> -                     pr_notice("Routing internal VCCH to in7\n");
> +                     pr_notice("Routing internal VCCH5V to in7\n");
>               }
>               if (reg & BIT(0))
>                       sio_data->internal |= BIT(0);
> @@ -2828,13 +2828,89 @@ static int __init it87_find(int sioaddr, unsigned 
> short *address,
>       return err;
>  }
>  
> +/*
> + * Some chips seem to have default value 0xff for all limit
> + * registers. For low voltage limits it makes no sense and triggers
> + * alarms, so change to 0 instead. For high temperature limits, it
> + * means -1 degree C, which surprisingly doesn't trigger an alarm,
> + * but is still confusing, so change to 127 degrees C.
> + */
> +static void it87_check_limit_regs(struct it87_data *data)
> +{
> +     int i, reg;
> +
> +     for (i = 0; i < NUM_VIN_LIMIT; i++) {
> +             reg = it87_read_value(data, IT87_REG_VIN_MIN(i));
> +             if (reg == 0xff)
> +                     it87_write_value(data, IT87_REG_VIN_MIN(i), 0);
> +     }
> +     for (i = 0; i < NUM_TEMP_LIMIT; i++) {
> +             reg = it87_read_value(data, IT87_REG_TEMP_HIGH(i));
> +             if (reg == 0xff)
> +                     it87_write_value(data, IT87_REG_TEMP_HIGH(i), 127);
> +     }
> +}
> +
> +/* Check if voltage monitors are reset manually or by some reason */
> +static void it87_check_voltage_monitors_reset(struct it87_data *data)
> +{
> +     int reg;
> +
> +     reg = it87_read_value(data, IT87_REG_VIN_ENABLE);
> +     if ((reg & 0xff) == 0) {
> +             /* Enable all voltage monitors */
> +             it87_write_value(data, IT87_REG_VIN_ENABLE, 0xff);
> +     }
> +}
> +
> +/* Check if tachometers are reset manually or by some reason */
> +static void it87_check_tachometers_reset(struct platform_device *pdev)
> +{
> +     struct it87_sio_data *sio_data = dev_get_platdata(&pdev->dev);
> +     struct it87_data *data = platform_get_drvdata(pdev);
> +     u8 mask, fan_main_ctrl;
> +
> +     mask = 0x70 & ~(sio_data->skip_fan << 4);
> +     fan_main_ctrl = it87_read_value(data, IT87_REG_FAN_MAIN_CTRL);
> +     if ((fan_main_ctrl & mask) == 0) {
> +             /* Enable all fan tachometers */
> +             fan_main_ctrl |= mask;
> +             it87_write_value(data, IT87_REG_FAN_MAIN_CTRL,
> +                              fan_main_ctrl);
> +     }
> +}
> +
> +/* Set tachometers to 16-bit mode if needed */
> +static void it87_check_tachometers_16bit_mode(struct platform_device *pdev)
> +{
> +     struct it87_data *data = platform_get_drvdata(pdev);
> +     int reg;
> +
> +     if (!has_fan16_config(data))
> +             return;
> +
> +     reg = it87_read_value(data, IT87_REG_FAN_16BIT);
> +     if (~reg & 0x07 & data->has_fan) {
> +             dev_dbg(&pdev->dev,
> +                     "Setting fan1-3 to 16-bit mode\n");
> +             it87_write_value(data, IT87_REG_FAN_16BIT,
> +                              reg | 0x07);
> +     }
> +}
> +
> +static void it87_start_monitoring(struct it87_data *data)
> +{
> +     it87_write_value(data, IT87_REG_CONFIG,
> +                      (it87_read_value(data, IT87_REG_CONFIG) & 0x3e)
> +                      | (update_vbat ? 0x41 : 0x01));
> +}
> +
>  /* Called when we have found a new IT87. */
>  static void it87_init_device(struct platform_device *pdev)
>  {
>       struct it87_sio_data *sio_data = dev_get_platdata(&pdev->dev);
>       struct it87_data *data = platform_get_drvdata(pdev);
>       int tmp, i;
> -     u8 mask;
>  
>       /*
>        * For each PWM channel:
> @@ -2855,23 +2931,7 @@ static void it87_init_device(struct platform_device 
> *pdev)
>               data->auto_pwm[i][3] = 0x7f;    /* Full speed, hard-coded */
>       }
>  
> -     /*
> -      * Some chips seem to have default value 0xff for all limit
> -      * registers. For low voltage limits it makes no sense and triggers
> -      * alarms, so change to 0 instead. For high temperature limits, it
> -      * means -1 degree C, which surprisingly doesn't trigger an alarm,
> -      * but is still confusing, so change to 127 degrees C.
> -      */
> -     for (i = 0; i < NUM_VIN_LIMIT; i++) {
> -             tmp = it87_read_value(data, IT87_REG_VIN_MIN(i));
> -             if (tmp == 0xff)
> -                     it87_write_value(data, IT87_REG_VIN_MIN(i), 0);
> -     }
> -     for (i = 0; i < NUM_TEMP_LIMIT; i++) {
> -             tmp = it87_read_value(data, IT87_REG_TEMP_HIGH(i));
> -             if (tmp == 0xff)
> -                     it87_write_value(data, IT87_REG_TEMP_HIGH(i), 127);
> -     }
> +     it87_check_limit_regs(data);
>  
>       /*
>        * Temperature channels are not forcibly enabled, as they can be
> @@ -2880,38 +2940,19 @@ static void it87_init_device(struct platform_device 
> *pdev)
>        * run-time through the temp{1-3}_type sysfs accessors if needed.
>        */
>  
> -     /* Check if voltage monitors are reset manually or by some reason */
> -     tmp = it87_read_value(data, IT87_REG_VIN_ENABLE);
> -     if ((tmp & 0xff) == 0) {
> -             /* Enable all voltage monitors */
> -             it87_write_value(data, IT87_REG_VIN_ENABLE, 0xff);
> -     }
> +     it87_check_voltage_monitors_reset(data);
> +
> +     it87_check_tachometers_reset(pdev);
>  
> -     /* Check if tachometers are reset manually or by some reason */
> -     mask = 0x70 & ~(sio_data->skip_fan << 4);
>       data->fan_main_ctrl = it87_read_value(data, IT87_REG_FAN_MAIN_CTRL);
> -     if ((data->fan_main_ctrl & mask) == 0) {
> -             /* Enable all fan tachometers */
> -             data->fan_main_ctrl |= mask;
> -             it87_write_value(data, IT87_REG_FAN_MAIN_CTRL,
> -                              data->fan_main_ctrl);
> -     }
>       data->has_fan = (data->fan_main_ctrl >> 4) & 0x07;
>  
> -     tmp = it87_read_value(data, IT87_REG_FAN_16BIT);
> -
> -     /* Set tachometers to 16-bit mode if needed */
> -     if (has_fan16_config(data)) {
> -             if (~tmp & 0x07 & data->has_fan) {
> -                     dev_dbg(&pdev->dev,
> -                             "Setting fan1-3 to 16-bit mode\n");
> -                     it87_write_value(data, IT87_REG_FAN_16BIT,
> -                                      tmp | 0x07);
> -             }
> -     }
> +     it87_check_tachometers_16bit_mode(pdev);
>  
>       /* Check for additional fans */
>       if (has_five_fans(data)) {
> +             tmp = it87_read_value(data, IT87_REG_FAN_16BIT);
> +
>               if (tmp & BIT(4))
>                       data->has_fan |= BIT(3); /* fan4 enabled */
>               if (tmp & BIT(5))
> @@ -2933,10 +2974,7 @@ static void it87_init_device(struct platform_device 
> *pdev)
>                       sio_data->skip_pwm |= BIT(5);
>       }
>  
> -     /* Start monitoring */
> -     it87_write_value(data, IT87_REG_CONFIG,
> -                      (it87_read_value(data, IT87_REG_CONFIG) & 0x3e)
> -                      | (update_vbat ? 0x41 : 0x01));
> +     it87_start_monitoring(data);
>  }
>  
>  /* Return 1 if and only if the PWM interface is safe to use */
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to