Re: [PATCH v3] applesmc: Re-work SMC comms

[Henrik Rydberg]

On 2020-11-08 12:57, Brad Campbell wrote:
> On 8/11/20 9:14 pm, Henrik Rydberg wrote:
> > On Sun, Nov 08, 2020 at 09:35:28AM +0100, Henrik Rydberg wrote:
> > > Hi Brad,
> > >
> > > On 2020-11-08 02:00, Brad Campbell wrote:
> > > > G'day Henrik,
> > > >
> > > > I noticed you'd also loosened up the requirement for SMC_STATUS_BUSY in read_smc(). I assume
> > > > that causes problems on the early Macbook. This is revised on the one sent earlier.
> > > > If you could test this on your Air1,1 it'd be appreciated.
> > >
> > > No, I managed to screw up the patch; you can see that I carefully added the
> > > same treatment for the read argument, being unsure if the BUSY state would
> > > remain during the AVAILABLE data phase. I can check that again, but
> > > unfortunately the patch in this email shows the same problem.
> > >
> > > I think it may be worthwhile to rethink the behavior of wait_status() here.
> > > If one machine shows no change after a certain status bit change, then
> > > perhaps the others share that behavior, and we are waiting in vain. Just
> > > imagine how many years of cpu that is, combined. ;-)
> >
> > Here is a modification along that line.
> >
> > Compared to your latest version, this one has wait_status() return the
> > actual status on success. Instead of waiting for BUSY, it waits for
> > the other status bits, and checks BUSY afterwards. So as not to wait
> > unneccesarily, the udelay() is placed together with the single
> > outb(). The return value of send_byte_data() is augmented with
> > -EAGAIN, which is then used in send_command() to create the resend
> > loop.
> >
> > I reach 41 reads per second on the MBA1,1 with this version, which is
> > getting close to the performance prior to the problems.
>
> G'day Henrik,
>
> I like this one. It's slower on my laptop (40 rps vs 50 on the MacbookPro11,1) and the same 17 rps on the iMac 12,2 but it's as reliable
> and if it works for both of yours then I think it's a winner. I can't really diagnose the iMac properly as I'm 2,800KM away and have
> nobody to reboot it if I kill it. 5.7.2 gives 20 rps, so 17 is ok for me.
>
> Andreas, could I ask you to test this one?
>
> Odd my original version worked on your Air3,1 and the other 3 machines without retry.
> I wonder how many commands require retries, how many retires are actually required, and what we are going wrong on the Air1,1 that requires
> one or more retries.
>
> I just feels like a brute force approach because there's something we're missing.

I would think you are right. There should be a way to follow the status 
changes in realtime, so one can determine handshake and processing from 
that information. At least, with this change, we are making the blunt 
instrument a little smaller.

Cheers,
Henrik