On Thu, Dec 11, 2008 at 10:35:01AM -0800, Jesse Barnes wrote: > On Thursday, December 11, 2008 10:16 am Alex Chiang wrote: > > From: Justin Chen <justin.chen@xxxxxx> > > > > Certain HP machines require the full 64 bits of _SUN as allowed > > by the ACPI spec. Without this change, we get name collisions in > > the lower 32 bits of the _SUN returned by firmware. > > > > Signed-off-by: Justin Chen <justin.chen@xxxxxx> > > Signed-off-by: Alex Chiang <achiang@xxxxxx> > > It's very late in the cycle, so I'd like to get acks from at least a couple of > other testers for this one... Any volunteers? Testers or reviewers? Here's my review (done thinking out loud style): $ grep -w sun drivers/pci/hotplug/acpiphp* 1. drivers/pci/hotplug/acpiphp_core.c: snprintf(name, SLOT_NAME_SIZE, "%u", slot->acpi_slot->sun); 2. drivers/pci/hotplug/acpiphp_glue.c: unsigned long long adr, sun; 3. drivers/pci/hotplug/acpiphp_glue.c: status = acpi_evaluate_integer(handle, "_SUN", NULL, &sun); 4. drivers/pci/hotplug/acpiphp_glue.c: sun = bridge->nr_slots+1; 5. drivers/pci/hotplug/acpiphp_glue.c: if (slot->sun != sun) 6. drivers/pci/hotplug/acpiphp_glue.c: slot->sun = sun; 7. drivers/pci/hotplug/acpiphp_glue.c: slot->sun, pci_domain_nr(bridge->pci_bus), 8. drivers/pci/hotplug/acpiphp_glue.c: "hotplug driver\n", slot->sun); 9. drivers/pci/hotplug/acpiphp.h: u32 sun; /* ACPI _SUN (slot unique number) */ 10. drivers/pci/hotplug/acpiphp_ibm.c:#define hpslot_to_sun(A) (((struct slot *)((A)->private))->acpi_slot->sun) 11. drivers/pci/hotplug/acpiphp_ibm.c: u8 sun; #1 is a printk ... string changed to %llu. Fine. #2 is a declaration of a local variable, not related. #3 is a use of said local variable, fine. #4 ditto #5 compares said local variable to the newly widened element. Good. #6 is an assignment. Good. #7 is also a printk. Also changed to %llu. Good. #8 Ditto. #9 is the change in question. #10 and #11 are in the acpiphp_ibm driver. This driver will truncate the stored value of 'sun' to 32-bit ... however, it then compares the result for equality with an 8-bit value. We do no harm here by storing a 64-bit value in sun instead of a 32-bit value. If IBM want to produce machines that use more than 8 bits for slot number, they will have to change their firmware interface anyway -- their ACPI table uses 8-bit wide data types for it. Acked-by: Matthew Wilcox <willy@xxxxxxxxxxxxxxx> -- Matthew Wilcox Intel Open Source Technology Centre "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
