Re: [Pcihpd-discuss] [PATCH 5/6]: hotplug/ia64: SN Hotplug Driver - SN Hotplug Driver code

From: Prarit Bhargava <prarit_at_sgi.com>
Date: 2005-05-18 21:42:56
Rolf Eike Beer wrote:

> 
> 
> 
>>+	if (rc == PCI_SLOT_ALREADY_UP) {
>>+		dev_dbg(slot->pci_bus->self, "is already active\n");
>>+		return -EPERM;
>>+	}
> 
> 
> IIRC most other drivers handle this case as success. Greg, your opinion?
> 

Other drivers handle this by returning 1.  I changed the return value to 1.

> 
> 
>>+		dev_dbg(slot->pci_bus->self, "Slot %s already inactive\n");
>>+		return -ENODEV;
>>+	}
> 
> 
> Again this might better be a success.
> 

Other drivers handle this by returning 1.  I changed the return value to 1.

>>+	num_funcs = pci_scan_slot(slot->pci_bus, PCI_DEVFN(slot->device_num+1,
> 
> 
> Add spaces before and after '+'. I don't feel good with this "+1" at all, this 
> is some kind of strange.
> 

Aside from being used in the above calculation, the slot->device_num is also a 
bitmask.  The 0th bit corresponds to the first device that is active/inactive on 
a slot.

In the calculation above, the slot we're scanning is PCI_DEVFN(1,0) -- the first 
device in the slot.

I could flip things around and use slot->device_num to a one-based calculation, 
but that leads to more "device_num - 1" statements than "device_num" statements 
in the code.

P.
-
To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Received on Wed May 18 07:43:43 2005

This archive was generated by hypermail 2.1.8 : 2005-08-02 09:20:39 EST