G'day All,
Versions 1-3 of this patch were various attempts to try and simplify/clarify
the communication to the SMC in order to remove the timing sensitivity which
was exposed by Commit fff2d0f701e6 ("hwmon: (applesmc) avoid overlong
udelay()"). As with the original author(s), we were limited in not having any
documentation and relying on a "poke it and see what happens".
Whilst version 3 ticked all the boxes from a performance and reliability
standpoint it still wasn't clear why it worked and why retries were required
when sending a command to the SMC. I dug into the OSX driver to try and seek
some clarity around that and found a very simple "state corrector" for want of
a better word, whereby any interaction with the SMC was preceded by a simple 3
step process (found in smc_sane()) which ensured the SMC was in the right state
to accept a transaction (or ultimately reported as broken). My testing has
shown this routine is generally only required once on driver load to sync up
the state machine, however it's purpose is to pull the SMC back into line if
its internal state machine gets out of sync. This was likely happening with the
command retry loop in the earlier versions, however due to the way the status
bits were being checked it was unclear.
The other thing that was clear is outside of the "state corrector", all
checking of SMC status bits happened before a read/write, not after. By
re-working the comms to follow this protocol I believe we've simplified the
code, made the actual operation clearer and removed *all* timing dependencies.
This has also allowed a simplification of the timing in wait_status and a
reduction in the waits incurred.
Henrik further simplified wait_status by leaving the minimum wait in
usleep_range as 8uS. Testing on multiple machines has borne this out resulting
in less loops/sleeps and no apparent impact in the performance of the driver
(and in fact an increase on my iMac12,2 with a _very_ slow SMC). It also
allowed for the removal of the jiffy based timeout as worst case it's going to
sleep for a couple of seconds. The OSX driver puts a 10s timeout on every wait.
So, after much testing I'll submit v4 for comment and further testing.
Regards,
Brad
