On 10/27/21 12:28 AM, Jean Delvare wrote:
Hi Jarkko,
On Tue, 19 Oct 2021 17:17:00 +0300, Jarkko Nikula wrote:
Currently interrupt storm will occur from i2-i801 after first transaction
Typo: i2c-i801.
Will fix.
Hi Conrad and Stephane. This patch is otherwise the same than the one I
had in bugzilla but this adds also acknowledging for the SMB_ALERT
interrupt. There is short time window during driver load and unload
where interrupt storm will still occur if signal was asserted. Also
The storm only starts with the first transaction, loading the driver
does not start it, so I can't see the window at load time.
For driver unload time, maybe we should restore SMBHSTCNT_INTREN to
its original value at that time, to prevent an interrupt storm from
happening. If we don't, then I suspect we'll have a storm not only for
a short time window but actually forever.
I had to hack and retest how I got that load and unload time interrupts
but managed to do it with a code that doesn't acknowledge SMB_ALERT. It
wasn't actually straight after boot but hacking with rmmod/modprobe,
transactions and SMB_ALERT during runtime. Here in simplest form:
0. Boot
1. Do transaction and get the SMBHSTCNT_INTREN enabled
2. SMB_ALERT or rmmod
- if SMB_ALERT first, then interrupts during this unload
- if rmmod first, then interrupts during next unload
3. modprobe
- SMBHSTCNT_INTREN already set
-> PCI_STATUS_INTERRUPT set, "An interrupt is pending!" followed by
interrupts between devm_request_irq and i801_enable_host_notify where
SMB_ALERT disabled by the code change.
- status &= SMBHSTSTS_INTR | STATUS_ERROR_FLAGS;
- if (status) {
+ status &= SMBHSTSTS_INTR | STATUS_ERROR_FLAGS | SMBHSTSTS_SMBALERT_STS;
+ if (status)
outb_p(status, SMBHSTSTS(priv));
Might be worth a comment.
...
- if (!(SMBSLVCMD_HST_NTFY_INTREN & priv->original_slvcmd))
- outb_p(SMBSLVCMD_HST_NTFY_INTREN | priv->original_slvcmd,
- SMBSLVCMD(priv));
+ /* Enable host notify interrupt and disable SMB_ALERT signal */
+ outb_p(SMBSLVCMD_HST_NTFY_INTREN | SMBSLVCMD_SMBALERT_DISABLE |
+ priv->original_slvcmd, SMBSLVCMD(priv));
A more verbose comment would be welcome too. Right now we know why we
are doing that, but in a few years we won't remember.
Agree to both, will update.
/* clear Host Notify bit to allow a new notification */
outb_p(SMBSLVSTS_HST_NTFY_STS, SMBSLVSTS(priv));
Tested-by: Jean Delvare <jdelvare@xxxxxxx>
(But my system was not affected by the issue in the first place, so
it's only a non-regression test.)
I was fortunate to find a reference design in our lab where SMB_ALERT
wasn't connected anywhere else than a pull-up and was able to
relatively safely pull it down by a grounded wire. Just have to be
careful to not touch anything else with the wire :-)
Jarkko
