linux-kernel - Re: [PATCH] hwmon: (it87) Reapply probe path chip registers settings after resume

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sun, 23 Jul 2017 18:11:37 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     "Maciej S. Szmigiero" <mail@...iej.szmigiero.name>,
        Jean Delvare <jdelvare@...e.com>
Cc:     linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] hwmon: (it87) Reapply probe path chip registers settings
 after resume

On 07/23/2017 01:13 PM, Maciej S. Szmigiero wrote:
> On 23.07.2017 22:01, Guenter Roeck wrote:
>> On 07/23/2017 09:12 AM, 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 <mail@...iej.szmigiero.name>
>>
>> I understand the problem, but this looks quite invasive. Can this be solved
>> without changing the driver all over the place ?
> 
> We need to keep track of Super I/O base address (sioaddr), since we need to
> write to IT87_SIO_PINX2_REG register in a resume path.
> This register is only writable via a GPIO interface so we can't change it
> via Environmental Controller I/O port (that this driver normally uses).
> 
> The rest is basically making various internal state initializations in
> it87_init_device() conditional on 'resume' parameter, so they don't execute
> on resume and also about two dozen lines of actual resume handler.
> 
> This patch shouldn't cause any change of behavior in probe path.
> 

The problem is that it makes the code much harder to read. Please try to separate
resume path from normal initialization path. More specifically, please rearrange
the code and and as necessary introduce new functions such that the resume path
can call a different set of functions, and the 'resume' parameter is not necessary.

Thanks,
Guenter