Re: [PATCH v7] hwmon: (it87) Reapply probe path chip registers settings after resume
On 09.08.2017 19:39, Guenter Roeck wrote: > On Wed, Aug 09, 2017 at 05:15:46PM +0200, Maciej S. Szmigiero wrote: (..) >> >> Changes from v6: Add __maybe_unused to it87_resume_sio(), move a message >> about failure to enter Super I/O on resume to this function, make it clear >> what had actually failed and downgrade it to a warning. >> > Return value from it87_resume_sio() is now unused, thus it is pointless > to have it return an error. > > No need to resend; applied after making that change. > > Thanks, > Guenter Thanks, Maciej
Re: [PATCH v7] hwmon: (it87) Reapply probe path chip registers settings after resume
On Wed, Aug 09, 2017 at 05:15:46PM +0200, Maciej S. Szmigiero wrote: > 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 > --- > 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. > > Changed message level about VCCH5V being routed to in7 again on resume to > debug. > > Added a "need_in7_reroute" flag and made VCCH5V to in7 rerouting on resume > explicitly conditional on it. > > Didn't finally add check for enable_pwm_interface being set before calling > it87_check_pwm() in the resume function since this variable is it87_probe() > function local and so adding this check would require adding it to device > instance structure. > > Changes from v3, v4: Fix patch formatting. > > Changes from v5: Use 'bool' for on / off variables, don't change it87_find() > function arguments to minimize the changes. > > Changes from v6: Add __maybe_unused to it87_resume_sio(), move a message > about failure to enter Super I/O on resume to this function, make it clear > what had actually failed and downgrade it to a warning. > Return value from it87_resume_sio() is now unused, thus it is pointless to have it return an error. No need to resend; applied after making that change. Thanks, Guenter > drivers/hwmon/it87.c | 78 > ++-- > 1 file changed, 76 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c > index 818f123ac475..a36d5947c1f6 100644 > --- a/drivers/hwmon/it87.c > +++ b/drivers/hwmon/it87.c > @@ -497,12 +497,14 @@ 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; > u8 vid_value; > u8 beep_pin; > u8 internal;/* Internal sensors can be labeled */ > + bool need_in7_reroute; > /* Features skipped based on config or DMI */ > u16 skip_in; > u8 skip_vid; > @@ -517,6 +519,7 @@ struct it87_sio_data { > */ > struct it87_data { > const struct attribute_group *groups[7]; > + int sioaddr; > enum chips type; > u32 features; > u8 peci_mask; > @@ -532,6 +535,7 @@ struct it87_data { > u16 in_internal;/* Bitfield, internal sensors (for labels) */ > u16 has_in; /* Bitfield, voltage sensors enabled */ > u8 in[NUM_VIN][3]; /* [nr][0]=in, [1]=min, [2]=max */ > + bool need_in7_reroute; > u8 has_fan; /* Bitfield, fans enabled */ > u16 fan[NUM_FAN][2];/* Register values, [nr][0]=fan, [1]=min */ > u8 has_temp;/* Bitfield, temp sensors enabled */ > @@ -2487,6 +2491,7 @@ static int __init it87_find(int sioaddr, unsigned short > *address, > } > > err = 0; > + sio_data->sioaddr = sioaddr; > sio_data->revision = superio_inb(sioaddr, DEVREV) & 0x0f; > pr_info("Found IT%04x%s chip at 0x%x, revision %d\n", chip_type, > it87_devices[sio_data->type].suffix, > @@ -2575,6 +2580,7 @@ static int __init it87_find(int sioaddr, unsigned short > *address, > reg2c |= BIT(1); > superio_outb(sioaddr, IT87_SIO_PINX2_REG, >reg2c); > + sio_data->need_in7_reroute = true; > pr_notice("Routing internal VCCH5V to in7.\n"); > } > pr_notice("in7 routed to internal voltage divider, with > external pin disabled.\n"); > @@ -2777,6 +2783,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); > + sio_data->need_in7_reroute = true; > pr_notice("Routing internal VCCH5V to in7\n"); > } > if (reg & BIT(0)) > @@ -3024,8 +3031,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, dis
[PATCH v7] hwmon: (it87) Reapply probe path chip registers settings after resume
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 --- 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. Changed message level about VCCH5V being routed to in7 again on resume to debug. Added a "need_in7_reroute" flag and made VCCH5V to in7 rerouting on resume explicitly conditional on it. Didn't finally add check for enable_pwm_interface being set before calling it87_check_pwm() in the resume function since this variable is it87_probe() function local and so adding this check would require adding it to device instance structure. Changes from v3, v4: Fix patch formatting. Changes from v5: Use 'bool' for on / off variables, don't change it87_find() function arguments to minimize the changes. Changes from v6: Add __maybe_unused to it87_resume_sio(), move a message about failure to enter Super I/O on resume to this function, make it clear what had actually failed and downgrade it to a warning. drivers/hwmon/it87.c | 78 ++-- 1 file changed, 76 insertions(+), 2 deletions(-) diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c index 818f123ac475..a36d5947c1f6 100644 --- a/drivers/hwmon/it87.c +++ b/drivers/hwmon/it87.c @@ -497,12 +497,14 @@ 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; u8 vid_value; u8 beep_pin; u8 internal;/* Internal sensors can be labeled */ + bool need_in7_reroute; /* Features skipped based on config or DMI */ u16 skip_in; u8 skip_vid; @@ -517,6 +519,7 @@ struct it87_sio_data { */ struct it87_data { const struct attribute_group *groups[7]; + int sioaddr; enum chips type; u32 features; u8 peci_mask; @@ -532,6 +535,7 @@ struct it87_data { u16 in_internal;/* Bitfield, internal sensors (for labels) */ u16 has_in; /* Bitfield, voltage sensors enabled */ u8 in[NUM_VIN][3]; /* [nr][0]=in, [1]=min, [2]=max */ + bool need_in7_reroute; u8 has_fan; /* Bitfield, fans enabled */ u16 fan[NUM_FAN][2];/* Register values, [nr][0]=fan, [1]=min */ u8 has_temp;/* Bitfield, temp sensors enabled */ @@ -2487,6 +2491,7 @@ static int __init it87_find(int sioaddr, unsigned short *address, } err = 0; + sio_data->sioaddr = sioaddr; sio_data->revision = superio_inb(sioaddr, DEVREV) & 0x0f; pr_info("Found IT%04x%s chip at 0x%x, revision %d\n", chip_type, it87_devices[sio_data->type].suffix, @@ -2575,6 +2580,7 @@ static int __init it87_find(int sioaddr, unsigned short *address, reg2c |= BIT(1); superio_outb(sioaddr, IT87_SIO_PINX2_REG, reg2c); + sio_data->need_in7_reroute = true; pr_notice("Routing internal VCCH5V to in7.\n"); } pr_notice("in7 routed to internal voltage divider, with external pin disabled.\n"); @@ -2777,6 +2783,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); + sio_data->need_in7_reroute = true; pr_notice("Routing internal VCCH5V to in7\n"); } if (reg & BIT(0)) @@ -3024,8 +3031,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, @@ -3058,6 +3063,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