On Wed, Nov 10, 2021 at 04:10:32PM +0200, Jarkko Nikula wrote: > Currently interrupt storm will occur from i2c-i801 after first > transaction if SMB_ALERT signal is enabled and ever asserted. It is > enough if the signal is asserted once even before the driver is loaded > and does not recover because that interrupt is not acknowledged. > > This fix aims to fix it by two ways: > - Add acknowledging for the SMB_ALERT interrupt status > - Disable the SMB_ALERT interrupt on platforms where possible since the > driver currently does not make use for it > > Acknowledging resets the SMB_ALERT interrupt status on all platforms and > also should help to avoid interrupt storm on older platforms where the > SMB_ALERT interrupt disabling is not available. > > For simplicity this fix reuses the host notify feature for disabling and > restoring original register value. With proposed modification (to the text) below, FWIW, Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=177311 > Reported-by: ck+kernelbugzilla@xxxxxxxxxxx > Reported-by: stephane.poignant@xxxxxxxxxxxxxx > Signed-off-by: Jarkko Nikula <jarkko.nikula@xxxxxxxxxxxxxxx> > --- > v3: Add comment from Jean Delvare why SMB_ALERT interrupt is blocked for > now. Simplify comment about SMB_ALERT status clearing > v2: More verbose commenting around interrupt acknowledging, reporting > and SMB_ALERT disabling. Fix typo in commit log and split the first > sentence. > v1: > 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 > interrupt disabling is possible only on ICH3 and later so interrupt > acknowledging should also help those old platforms. > --- > drivers/i2c/busses/i2c-i801.c | 25 +++++++++++++++++++------ > 1 file changed, 19 insertions(+), 6 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c > index ed271274250b..21be28b7955f 100644 > --- a/drivers/i2c/busses/i2c-i801.c > +++ b/drivers/i2c/busses/i2c-i801.c > @@ -191,6 +191,7 @@ > #define SMBSLVSTS_HST_NTFY_STS BIT(0) > > /* Host Notify Command register bits */ > +#define SMBSLVCMD_SMBALERT_DISABLE BIT(2) > #define SMBSLVCMD_HST_NTFY_INTREN BIT(0) > > #define STATUS_ERROR_FLAGS (SMBHSTSTS_FAILED | SMBHSTSTS_BUS_ERR | \ > @@ -642,12 +643,20 @@ static irqreturn_t i801_isr(int irq, void *dev_id) > i801_isr_byte_done(priv); > > /* > - * Clear irq sources and report transaction result. > + * Clear remaining irq sources: Completion of last command, errors irq --> IRQ of last --> of the last > + * and the SMB_ALERT signal. SMB_ALERT status is set after signal > + * assertion independently is the interrupt generation blocked or not is --> if ? > + * so clear it always when the status is set. > + */ > + status &= SMBHSTSTS_INTR | STATUS_ERROR_FLAGS | SMBHSTSTS_SMBALERT_STS; > + if (status) > + outb_p(status, SMBHSTSTS(priv)); > + status &= ~SMBHSTSTS_SMBALERT_STS; /* SMB_ALERT not reported */ > + /* > + * Report transaction result. > * ->status must be cleared before the next transaction is started. > */ > - status &= SMBHSTSTS_INTR | STATUS_ERROR_FLAGS; > if (status) { > - outb_p(status, SMBHSTSTS(priv)); > priv->status = status; > complete(&priv->done); > } > @@ -975,9 +984,13 @@ static void i801_enable_host_notify(struct i2c_adapter *adapter) > if (!(priv->features & FEATURE_HOST_NOTIFY)) > return; > > - if (!(SMBSLVCMD_HST_NTFY_INTREN & priv->original_slvcmd)) > - outb_p(SMBSLVCMD_HST_NTFY_INTREN | priv->original_slvcmd, > - SMBSLVCMD(priv)); > + /* > + * Enable host notify interrupt and block the generation of interrupt > + * from the SMB_ALERT signal because the driver does not support > + * SMBus Alert yet. does not support ... yet --> has not supported ... yet does not support ? > + */ > + outb_p(SMBSLVCMD_HST_NTFY_INTREN | SMBSLVCMD_SMBALERT_DISABLE | > + priv->original_slvcmd, SMBSLVCMD(priv)); > > /* clear Host Notify bit to allow a new notification */ > outb_p(SMBSLVSTS_HST_NTFY_STS, SMBSLVSTS(priv)); > -- > 2.33.0 > -- With Best Regards, Andy Shevchenko
