Messages in this thread | | | From | Brad Campbell <> | Subject | Re: [PATCH v5 1/1] applesmc: Re-work SMC comms | Date | Thu, 12 Nov 2020 10:28:50 +1100 |
| |
On 12/11/20 7:05 am, Henrik Rydberg wrote: > On 2020-11-11 14:06, Brad Campbell wrote: >> Commit fff2d0f701e6 ("hwmon: (applesmc) avoid overlong udelay()") >> introduced an issue whereby communication with the SMC became >> unreliable with write errors like : >> >> [ 120.378614] applesmc: send_byte(0x00, 0x0300) fail: 0x40 >> [ 120.378621] applesmc: LKSB: write data fail >> [ 120.512782] applesmc: send_byte(0x00, 0x0300) fail: 0x40 >> [ 120.512787] applesmc: LKSB: write data fail >> >> The original code appeared to be timing sensitive and was not reliable >> with the timing changes in the aforementioned commit. >> >> This patch re-factors the SMC communication to remove the timing >> dependencies and restore function with the changes previously >> committed. Logic changes based on inspection of the Apple SMC kext. >> >> Tested on : MacbookAir6,2 MacBookPro11,1 iMac12,2, MacBookAir1,1, >> MacBookAir3,1 >> >> Fixes: fff2d0f701e6 ("hwmon: (applesmc) avoid overlong udelay()") >> Reported-by: Andreas Kemnade <andreas@kemnade.info> >> Tested-by: Andreas Kemnade <andreas@kemnade.info> # MacBookAir6,2 >> Acked-by: Arnd Bergmann <arnd@arndb.de> >> Signed-off-by: Brad Campbell <brad@fnarfbargle.com> >> Signed-off-by: Henrik Rydberg <rydberg@bitmath.org> >> >> --- >> Changelog : >> v1 : Initial attempt >> v2 : Address logic and coding style >> v3 : Removed some debug hangover. Added tested-by. Modifications for MacBookAir1,1 >> v4 : Re-factored logic based on Apple driver. Simplified wait_status loop >> v5 : Re-wrote status loop. Simplified busy check in send_byte(). Fixed formatting > > Hi Brad, > > This version is still working fine on the MBA1,1, at 50 reads per second. >
Cheers Henrik,
I did 5.6 million reads overnight and had 3 failures. I suspect it's this : status = inb(APPLESMC_CMD_PORT) & SMC_STATUS_BUSY; if (!status) return -EIO;
When I used wait_status() previously I didn't see any read errors. With hindsight I probably shouldn't have made that simplification.
I'll do some more testing and probably submit a v6 with just this change. I think it's just about right provided that wait_status loop gets across the line.
Regards, Brad
| |