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

[Prarit Bhargava <prarit@sgi.com>]

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.