Re: [REGRESSION] hwmon: (applesmc) avoid overlong udelay()

From: Andreas Kemnade
Date: Wed Nov 04 2020 - 08:22:08 EST


On Tue, 3 Nov 2020 16:56:32 +1100
Brad Campbell <brad@xxxxxxxxxxxxxxx> wrote:

> On 3/11/20 10:56 am, Brad Campbell wrote:
>
> >
> > I've examined the code in VirtualSMC and I'm not convinced we were not waiting on the wrong bits.
> >
> > #define SMC_STATUS_AWAITING_DATA  BIT0  ///< Ready to read data.
> > #define SMC_STATUS_IB_CLOSED      BIT1  /// A write is pending.
> > #define SMC_STATUS_BUSY           BIT2  ///< Busy processing a command.
> > #define SMC_STATUS_GOT_COMMAND    BIT3  ///< The last input was a command.
> > #define SMC_STATUS_UKN_0x16       BIT4
> > #define SMC_STATUS_KEY_DONE       BIT5
> > #define SMC_STATUS_READY          BIT6  // Ready to work
> > #define SMC_STATUS_UKN_0x80       BIT7  // error
> >
> > Any chance you could try this patch? It's ugly, hacked together and currently fairly undocumented, but if it works I'll figure out how to clean it up (suggestions welcome).
> > It works on my MacbookPro 11,1.
>
> I had some time so I spent a bit of time refactoring and trying to clarify the magic numbers.
>
> I also did some fuzzing of the SMC and figured out where we can loosen the masks.
> This has some debug code in it to identify if any wait loops exceed 1 loop and if the SMC is reporting anything other than a clear "I'm waiting" prior to each command.
>
> You might see some of these :
> [ 51.316202] applesmc: wait_status looping 2: 0x44, 0x40, 0x4e
> [ 60.002547] applesmc: wait_status looping 2: 0x44, 0x40, 0x4e
> [ 60.130754] applesmc: wait_status looping 2: 0x44, 0x40, 0x4e
>
> I did some heavy tests and found that with the delays at the bottom of the loop about 50% of calls required no delay at all before a read or write and the other 50% require a single delay.
> I can count on one hand the number of times it's exceeded 1 loop, and the max thus far has been 5 loops.
>
That matches my experience. The only delay is at the end of the
command. Critcal seems to be that there is not too much delay in between.

[...]
> If anyone with a Mac having a conventional SMC and seeing issues on 5.9 could test this it'd be appreciated. I'm not saying this code is "correct", but it "works for me".
>
Seems to work here.
dmesg | grep applesmc

[ 1.350782] applesmc: key=561 fan=1 temp=33 index=33 acc=0 lux=2 kbd=1
[ 1.350922] applesmc applesmc.768: hwmon_device_register() is deprecated. Please convert the driver to use hwmon_device_register_with_info().
[ 17.748504] applesmc: wait_status looping 2: 0x4a, 0x4c, 0x4f
[ 212.008952] applesmc: wait_status looping 2: 0x44, 0x40, 0x4e
[ 213.033930] applesmc: wait_status looping 2: 0x44, 0x40, 0x4e
[ 213.167908] applesmc: wait_status looping 2: 0x44, 0x40, 0x4e
[ 219.087854] applesmc: wait_status looping 2: 0x44, 0x40, 0x4e

Tested it on top of 5.9

Regards,
Andreas