After a suspend / resume cycle we possibly need to reapply chip registers
settings that we had set or fixed in a probe path, since they might have
been reset to default values or set incorrectly by a BIOS again.

Tested on a Gigabyte M720-US3 board, which requires routing internal VCCH5V
to in7 (and had it wrong again on resume from S3).

Signed-off-by: Maciej S. Szmigiero <m...@maciej.szmigiero.name>
---
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.

 drivers/hwmon/it87.c | 214 +++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 163 insertions(+), 51 deletions(-)

diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
index 4dfc7238313e..b565e29c405c 100644
--- a/drivers/hwmon/it87.c
+++ b/drivers/hwmon/it87.c
@@ -497,6 +497,7 @@ static const struct it87_devices it87_devices[] = {
 #define has_vin3_5v(data)      ((data)->features & FEAT_VIN3_5V)
 
 struct it87_sio_data {
+       int sioaddr;
        enum chips type;
        /* Values read from Super-I/O config space */
        u8 revision;
@@ -517,6 +518,7 @@ struct it87_sio_data {
  */
 struct it87_data {
        const struct attribute_group *groups[7];
+       int sioaddr;
        enum chips type;
        u32 features;
        u8 peci_mask;
@@ -2389,10 +2391,11 @@ static const struct attribute_group it87_group_auto_pwm 
= {
 };
 
 /* SuperIO detection - will change isa_address if a chip is found */
-static int __init it87_find(int sioaddr, unsigned short *address,
+static int __init it87_find(unsigned short *address,
                            struct it87_sio_data *sio_data)
 {
        int err;
+       int sioaddr = sio_data->sioaddr;
        u16 chip_type;
        const char *board_vendor, *board_name;
        const struct it87_devices *config;
@@ -2828,13 +2831,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 +2934,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 +2943,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 +2977,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 */
@@ -2986,8 +3027,6 @@ static int it87_check_pwm(struct device *dev)
                                 "PWM configuration is too broken to be 
fixed\n");
                }
 
-               dev_info(dev,
-                        "Detected broken BIOS defaults, disabling PWM 
interface\n");
                return 0;
        } else if (fix_pwm_polarity) {
                dev_info(dev,
@@ -3020,6 +3059,7 @@ static int it87_probe(struct platform_device *pdev)
                return -ENOMEM;
 
        data->addr = res->start;
+       data->sioaddr = sio_data->sioaddr;
        data->type = sio_data->type;
        data->features = it87_devices[sio_data->type].features;
        data->peci_mask = it87_devices[sio_data->type].peci_mask;
@@ -3058,6 +3098,9 @@ static int it87_probe(struct platform_device *pdev)
 
        /* Check PWM configuration */
        enable_pwm_interface = it87_check_pwm(dev);
+       if (!enable_pwm_interface)
+               dev_info(dev,
+                        "Detected broken BIOS defaults, disabling PWM 
interface\n");
 
        /* Starting with IT8721F, we handle scaling of internal voltages */
        if (has_12mv_adc(data)) {
@@ -3140,9 +3183,77 @@ static int it87_probe(struct platform_device *pdev)
        return PTR_ERR_OR_ZERO(hwmon_dev);
 }
 
+static int it87_resume_sio(struct platform_device *pdev)
+{
+       struct it87_data *data = dev_get_drvdata(&pdev->dev);
+       int err;
+       int reg2c;
+
+       if (!(data->in_internal & BIT(1)))
+               return 0;
+
+       if (has_in7_internal(data))
+               return 0;
+
+       err = superio_enter(data->sioaddr);
+       if (err)
+               return err;
+
+       superio_select(data->sioaddr, GPIO);
+
+       reg2c = superio_inb(data->sioaddr, IT87_SIO_PINX2_REG);
+       if (!(reg2c & BIT(1))) {
+               dev_notice(&pdev->dev,
+                          "Routing internal VCCH5V to in7 again");
+
+               reg2c |= BIT(1);
+               superio_outb(data->sioaddr, IT87_SIO_PINX2_REG,
+                            reg2c);
+       }
+
+       superio_exit(data->sioaddr);
+
+       return 0;
+}
+
+static int __maybe_unused it87_resume(struct device *dev)
+{
+       struct platform_device *pdev = to_platform_device(dev);
+       struct it87_data *data = dev_get_drvdata(dev);
+       int err;
+
+       err = it87_resume_sio(pdev);
+       if (err)
+               dev_err(dev,
+                       "Unable to resume Super I/O (%d)",
+                       err);
+
+       mutex_lock(&data->update_lock);
+
+       it87_check_pwm(dev);
+       it87_check_limit_regs(data);
+       it87_check_voltage_monitors_reset(data);
+       it87_check_tachometers_reset(pdev);
+       it87_check_tachometers_16bit_mode(pdev);
+
+       it87_start_monitoring(data);
+
+       /* force update */
+       data->valid = 0;
+
+       mutex_unlock(&data->update_lock);
+
+       it87_update_device(dev);
+
+       return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(it87_dev_pm_ops, NULL, it87_resume);
+
 static struct platform_driver it87_driver = {
        .driver = {
                .name   = DRVNAME,
+               .pm     = &it87_dev_pm_ops,
        },
        .probe  = it87_probe,
 };
@@ -3208,8 +3319,9 @@ static int __init sm_it87_init(void)
 
        for (i = 0; i < ARRAY_SIZE(sioaddr); i++) {
                memset(&sio_data, 0, sizeof(struct it87_sio_data));
+               sio_data.sioaddr = sioaddr[i];
                isa_address[i] = 0;
-               err = it87_find(sioaddr[i], &isa_address[i], &sio_data);
+               err = it87_find(&isa_address[i], &sio_data);
                if (err || isa_address[i] == 0)
                        continue;
                /*

Reply via email to