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

[Guenter Roeck]

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