On 11.12.19 00:25, Bjorn Helgaas wrote: Hi, > 2) I despise the pci_get_device() interfaces because they're > inefficient, not hotplug-safe, they circumvent the device model > claim mechanism, and it's hard to do the reference counting > correctly. Agreed, but for that we'd have to edac and hwmo driver together into one (or maybe a MFD). That would be a major rewrite, and I don't have the necessary HW to test it. > 3) There are several things going on in this patch and it would be > easier to read if you could split them into separate patches: > > - Removing the redundancy between chipset_ids[] and i5k_amb_ids[]. > This seems like a nice change. > > - The "chipset_ids[i].fbd0 + 1" thing was weird and the new > ".driver_data + 1" is still weird. Those are PCI device IDs, > and addition is not a valid operation on those IDs. IMHO both > PCI_DEVICE_ID_INTEL_5000_FBD0 and PCI_DEVICE_ID_INTEL_5000_FBD1 > should be listed explicitly in the driver instead of trying to > compute PCI_DEVICE_ID_INTEL_5000_FBD1. Ok, good point. > - Replacing the hard-coding of PCI_VENDOR_ID_INTEL with the vendor > ID from i5k_amb_ids[] seems worthwhile and should be its own > separate patch (if possible). > > - Changing to use pci_get_device_by_id(). This should be trivial > to verify, like the other patches. Ok, I'll try to split it up more clearly, once I've got some more spare time. --mtx --- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@xxxxxxxxx -- +49-151-27565287
