Re: [v3, 1/2] hwmon: (it87) Split out chip registers setting code on probe path

2017-08-09 Thread Maciej S. Szmigiero

On 09.08.2017 06:27, Guenter Roeck wrote:

On Wed, Aug 02, 2017 at 01:06:23AM +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 


I finally got to this and tried to apply it, but it fails spectacularly.
Even patch fails to apply each and every chunk. No idea what is going on.
How did you generate the patch, and what was your baseline ?


It was generated from
git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git
(hwmon branch), but it looks like my MUA mangled it.

Will try to send again in a moment.
 

Guenter


Maciej


---
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.

   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

--
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


Re: [v3, 1/2] hwmon: (it87) Split out chip registers setting code on probe path

2017-08-08 Thread Guenter Roeck
On Wed, Aug 02, 2017 at 01:06:23AM +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 

I finally got to this and tried to apply it, but it fails spectacularly.
Even patch fails to apply each and every chunk. No idea what is going on.
How did you generate the patch, and what was your baseline ?

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.
> 
>   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->ski