linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andreas Kemnade <andreas@kemnade.info>
To: Brad Campbell <brad@fnarfbargle.com>
Cc: Guenter Roeck <linux@roeck-us.net>, Arnd Bergmann <arnd@arndb.de>,
	rydberg@bitmath.org, Jean Delvare <jdelvare@suse.com>,
	linux-hwmon@vger.kernel.org,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	hns@goldelico.com
Subject: Re: [REGRESSION] hwmon: (applesmc) avoid overlong udelay()
Date: Wed, 4 Nov 2020 14:20:57 +0100	[thread overview]
Message-ID: <20201104142057.62493c12@aktux> (raw)
In-Reply-To: <68467f1b-cea1-47ea-a4d4-8319214b072a@fnarfbargle.com>

On Tue, 3 Nov 2020 16:56:32 +1100
Brad Campbell <brad@fnarfbargle.com> 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

  reply	other threads:[~2020-11-04 13:21 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-30  8:54 [REGRESSION] hwmon: (applesmc) avoid overlong udelay() Andreas Kemnade
2020-09-30 16:44 ` Guenter Roeck
2020-09-30 20:00   ` Arnd Bergmann
2020-10-01 22:22     ` Andreas Kemnade
2020-10-02  4:07       ` Guenter Roeck
2020-10-06  7:02         ` Andreas Kemnade
2020-11-02 23:56           ` Brad Campbell
2020-11-03  5:56             ` Brad Campbell
2020-11-04 13:20               ` Andreas Kemnade [this message]
2020-11-05  2:18                 ` Brad Campbell
2020-11-05  4:22                   ` Brad Campbell
2020-11-05  4:43                   ` Guenter Roeck
2020-11-05  5:05                     ` Brad Campbell
2020-11-05  5:26                       ` Guenter Roeck
2020-11-05  5:47                         ` [PATCH] applesmc: Re-work SMC comms v1 Brad Campbell
2020-11-05  7:26                           ` [PATCH] applesmc: Re-work SMC comms v2 Brad Campbell
2020-11-05  7:56                             ` Henrik Rydberg
2020-11-05  8:15                               ` Andreas Kemnade
2020-11-05  8:30                               ` Brad Campbell
2020-11-05 10:31                                 ` Henrik Rydberg
2020-11-06 16:26                                   ` Henrik Rydberg
2020-11-06 20:02                                     ` Henrik Rydberg
2020-11-07 18:31                                       ` Henrik Rydberg
2020-11-08  0:09                                         ` Brad Campbell
2020-11-08  8:22                                           ` Henrik Rydberg
2020-11-08  1:00                                         ` [PATCH v3] applesmc: Re-work SMC comms Brad Campbell
2020-11-08  8:35                                           ` Henrik Rydberg
2020-11-08 10:14                                             ` Henrik Rydberg
2020-11-08 11:57                                               ` Brad Campbell
2020-11-08 12:04                                                 ` Henrik Rydberg
2020-11-09 13:06                                                   ` Brad Campbell
2020-11-09 17:08                                                     ` Henrik Rydberg
2020-11-09 22:52                                                       ` Brad Campbell
2020-11-08 16:06                                               ` Guenter Roeck
2020-11-09  0:25                                                 ` Brad Campbell
2020-11-10  2:04                                                 ` Brad Campbell
2020-11-10  4:55                                                   ` Guenter Roeck
2020-11-10  5:40                                                     ` Brad Campbell
2020-11-10 16:02                                                       ` Guenter Roeck
2020-11-09  8:44                                               ` Andreas Kemnade
2020-11-09  9:51                                                 ` Brad Campbell
2020-11-11  3:37                                           ` [PATCH v4 0/1] " Brad Campbell
2020-11-11  4:55                                             ` [PATCH v1] applesmc: Cleanups on top of re-work comms Brad Campbell
2020-11-11  3:38                                           ` [PATCH v4 1/1] applesmc: Re-work SMC comms Brad Campbell
2020-11-11  5:56                                             ` Guenter Roeck
2020-11-11  7:05                                               ` Brad Campbell
2020-11-11 13:06                                               ` [PATCH v5 " Brad Campbell
2020-11-11 20:05                                                 ` Henrik Rydberg
2020-11-11 23:28                                                   ` Brad Campbell
2020-11-12  3:08                                                 ` [PATCH v6 " Brad Campbell
2020-11-12 17:20                                                   ` Guenter Roeck
2020-11-06 23:11                                     ` [PATCH] applesmc: Re-work SMC comms v2 Brad Campbell
2020-11-05  8:12                             ` Andreas Kemnade
2020-11-05 16:12                             ` Guenter Roeck
2020-11-06  0:02                               ` Brad Campbell
2020-11-06  3:08                                 ` Guenter Roeck
2020-11-09  9:27                           ` [PATCH] applesmc: Re-work SMC comms v1 kernel test robot
2020-11-05  9:48                       ` [REGRESSION] hwmon: (applesmc) avoid overlong udelay() Arnd Bergmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201104142057.62493c12@aktux \
    --to=andreas@kemnade.info \
    --cc=arnd@arndb.de \
    --cc=brad@fnarfbargle.com \
    --cc=hns@goldelico.com \
    --cc=jdelvare@suse.com \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=rydberg@bitmath.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).