Re: M7101

[Matthew Wilcox <matthew@wil.cx>]

On Sun, Feb 06, 2005 at 03:26:15PM +0100, Jean Delvare wrote:
> Maarten Deprez then converted it to the proper kernel coding-style:
> http://marc.theaimsgroup.com/?l=linux-kernel&m=110726276414532
> 
> I invite you to test the new patch and confirm that it works for you.
> 
> Any chance we could get the PCI folks to review the code and push it
> upwards if it is OK?

Looks pretty good to me.  For clarity, I'd change:

-	m7101 = pci_scan_single_device(dev->bus, 0x18);
+	m7101 = pci_scan_single_device(dev->bus, PCI_DEVFN(3, 0));

and then test m7101 for being NULL -- if it fails, you'll get a NULL
ptr dereference from pci_read_config_byte():

-	if (pci_read_config_byte(m7101, 0x5B, &val)) {
+	if (!m7101 || pci_read_config_byte(m7101, 0x5B, &val)) {

I don't think you need to bother checking the subsequent write for failure:

 	if (val & 0x06) {
-		if (pci_write_config_byte(m7101, 0x5B, val & ~0x06)) {
-			printk(KERN_INFO "PCI: Failed to enable M7101 SMBus Controller\n");
-			return;
-		}
+		pci_write_config_byte(m7101, 0x5B, val & ~0x06);
 	}

So conceptually, I approve, just some details need fixing.

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain