Re: Regression in -git / [PATCH] i2c-i801.c: don't pci_disable_device() after it was just enabled

[Jean Delvare <khali@linux-fr.org>]

Hi Daniel,

I see that your patch was already merged, but I would like to reply
anyway.

> [PATCH] i2c-i801.c: don't pci_disable_device() after it was just enabled
> 
> Commit 02dd7ae2892e5ceff111d032769c78d3377df970:
> 	[PATCH] i2c-i801: Merge setup function
> has a missing return 0 in the _probe() function. this means the error
> path is always executed and pci_disable_device() is called even when
> the device just got successfully enabled.

Oops, good catch, thanks. I'm quite ashamed for letting this go
through :(

> having the SMBus device disabled makes some systems (eg. Fujitsu-Siemens
> Lifebook E8010) hang hard during power-off.
> 
> Intead of reverting the whole commit this patch fixes it up:
> - don't ever call pci_disable_device(), also not in the _remove() function
>   to avoid hangs

This is weird, and would certainly deserve additional investigation.
Disabling the PCI device when we no more need it is the right thing to
do and almost all pci drivers do that by now - except I2C bus drivers,
this is the first one I was attempting to convert.

Do you have any idea why disabling the SMBus causes the problem you
observe? Could be that your BIOS attempts to use the SMBus at power
down time, but I wonder what for. Do you have anything special on this
SMBus? Proprietary EEPROM? Real-time clock? I have two laptops using
this driver (one Sony, one Dell) and none exhibited the problem you
described.

> - fix missing pci_release_region() in error path

Another bug I let go through :( Thanks again.

-- 
Jean Delvare
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/