On Mon, 9 Apr 2018 18:20:14 +0200, Jean Delvare wrote:
> What is pretty interesting in your previous post, and I failed to
> notice before, is that bit 0 of register 0xefb1 was still set after the
> module was unloaded. The original register value is supposed to be
> restored in i801_disable_host_notify(), and the dump you just provided
> indicates that Host Notify is not enabled by the BIOS on your machine,
> so the value of this bit should have been saved, and then restored, to
> 0. This is not happening.
>
> I think it would be worth instrumenting the i2c-i801 driver to check
> what exactly is going on. Something like:
>
> ---
> drivers/i2c/busses/i2c-i801.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> --- linux-4.16.orig/drivers/i2c/busses/i2c-i801.c 2018-04-09 14:54:05.179509697 +0200
> +++ linux-4.16/drivers/i2c/busses/i2c-i801.c 2018-04-09 18:09:28.843867439 +0200
> @@ -967,6 +967,8 @@ static void i801_enable_host_notify(stru
> return;
>
> priv->original_slvcmd = inb_p(SMBSLVCMD(priv));
> + dev_info(&adapter->dev, "Original SMBSLVCMD value: %02X\n",
> + (unsigned int)priv->original_slvcmd);
>
> if (!(SMBSLVCMD_HST_NTFY_INTREN & priv->original_slvcmd))
> outb_p(SMBSLVCMD_HST_NTFY_INTREN | priv->original_slvcmd,
> @@ -974,6 +976,9 @@ static void i801_enable_host_notify(stru
>
> /* clear Host Notify bit to allow a new notification */
> outb_p(SMBSLVSTS_HST_NTFY_STS, SMBSLVSTS(priv));
> +
> + dev_info(&adapter->dev, "SMBSLVCMD value after initialization: %02X\n",
> + (unsigned int)inb_p(SMBSLVCMD(priv)));
> }
>
> static void i801_disable_host_notify(struct i801_priv *priv)
> @@ -981,7 +986,11 @@ static void i801_disable_host_notify(str
> if (!(priv->features & FEATURE_HOST_NOTIFY))
> return;
>
> + dev_info(&priv->adapter.dev, "SMBSLVCMD value at removal time: %02X\n",
> + (unsigned int)inb_p(SMBSLVCMD(priv)));
> outb_p(priv->original_slvcmd, SMBSLVCMD(priv));
> + dev_info(&priv->adapter.dev, "SMBSLVCMD reset to: %02X\n",
> + (unsigned int)inb_p(SMBSLVCMD(priv)));
> }
>
> static const struct i2c_algorithm smbus_algorithm = {
>
> Could you try this with the Host Notify feature enabled, and report the
> values that are logged?
OK, I think I nailed it. I applied the patch above, and it turns out
that i801_enable_host_notify() is being called *twice* when I load
i2c-i801: once from i801_probe(), as expected, and once from
i801_resume(), which I did not expect. My kernel config has
CONFIG_PM_TEST_SUSPEND=n, so I'm not sure what this happens.
Anyway, looking at the code, it is clear that saving
priv->original_slvcmd in i801_enable_host_notify() is wrong. This
function is called at resume time and we have no idea what will be the
value of the SMBSLVCMD register at that point: it could be the hardware
default value, or a value set by the BIOS, or our own value from
suspend time.
So the register value needs to be saved once and only once, at probe
time. I'll send a patch in a minute.
--
Jean Delvare
SUSE L3 Support
