On Tue, Apr 10, 2018 at 2:28 PM, Jean Delvare <jdelvare@xxxxxxx> wrote:
> On Tue, 10 Apr 2018 11:57:00 -0400, Jason Andryuk wrote:
>> > On Tue, Apr 10, 2018 at 9:44 AM, Jean Delvare <jdelvare@xxxxxxx> wrote:
>> >> Jason, please test this patch and let us know if it fixes your problem.
>>
>> Unfortunately, no. For a normal power off, the subsequent boot hangs
>> as I was seeing before. However, if I rmmod the driver first and then
>> power off, the subsequent boot does not hang. Maybe i801_remove is
>> only called on rmmod, but not shutdown? Do we need to define
>> shutdown() for struct pci_driver?
>
> I'm not sure. The current i2c-i801 driver code clearly assumes that the
> remove function is called at shutdown time. But on the other hand there
> must be a reason why a separate shutdown callback is supported at the
> driver core level.
>
> That would be easy enough to test...
>
> ---
> drivers/i2c/busses/i2c-i801.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> --- linux-4.15.orig/drivers/i2c/busses/i2c-i801.c 2018-04-10 19:01:57.085549960 +0200
> +++ linux-4.15/drivers/i2c/busses/i2c-i801.c 2018-04-10 19:06:04.820155530 +0200
> @@ -1700,6 +1700,16 @@ static void i801_remove(struct pci_dev *
> */
> }
>
> +static void i801_shutdown(struct pci_dev *dev)
> +{
> + struct i801_priv *priv = pci_get_drvdata(dev);
> +
> + dev_info(&dev->dev, "i801_shutdown is called\n");
> +
> + i801_disable_host_notify(priv);
> + pci_write_config_byte(dev, SMBHSTCFG, priv->original_hstcfg);
> +}
> +
> #ifdef CONFIG_PM
> static int i801_suspend(struct device *dev)
> {
> @@ -1729,6 +1739,7 @@ static struct pci_driver i801_driver = {
> .id_table = i801_ids,
> .probe = i801_probe,
> .remove = i801_remove,
> + .shutdown = i801_shutdown,
> .driver = {
> .pm = &i801_pm_ops,
> },
>
> Note that I can't see the log message in my own tests, but I think this
> is because systemd gets down before the kernel drivers are shut down,
> so nobody is there anymore to write the message to the disk. I would
> have to setup a serial console to test properly, but I don't have time
> to do that at the moment.
This shutdown patch on top of the "i2c: i801: Save register SMBSLVCMD
value only once" fixes the boot hang bug. I also included the
SMBSLVCMD debug print patch to log the values.
At power-off, I see:
i2c i2c-6: i801_enable_host_notify: SMBSLVCMD value before initialization: 01
i2c i2c-6: i801_enable_host_notify: SMBSLVCMD value after initialization: 01
i801_smbus 0000:00:1f.4: i801_shutdown is called
i2c i2c-6: i801_disable_host_notify: SMBSLVCMD value at removal time: 01
i2c i2c-6: i801_disable_host_notify: SMBSLVCMD reset to: 00
Thanks for your help!
Regards,
Jason
