[PATCH v2 0/2] Add pcibios_device_change_notifier
matsumoto.hiroo at jp.fujitsu.com
Thu Jun 7 22:45:11 EST 2012
I apologize for my late reply.
> On Wed, 2012-05-23 at 11:33 +0900, Hiroo Matsumoto wrote:
>> This patchset is for PCI hotplug.
>> pcibios_setup_bus_devices which sets DMA and IRQs of PCI device is called
>> only when boot. DMA setting in probe for PCI driver, like dma_set_mask,
>> does not work on powerpc platform. So it is need to set DMA and IRQs of
>> PCI device when hotplug.
>> 1. Moving pcibios_setup_bus_devices code to pcibios_device_change_notifier
>> which is registered to bus notifier in pcibios_init.
>> 2. Removing caller and callee of pcibios_setup_bus_devices bus notifier
>> works instead of pcibios_setup_bus_devices.
>> 3. Using this bus notifier for microblaze because microblaze/PCI is similer
>> with powerpc/PCI.
> This makes me a bit nervous (that doesn't mean it's not right, but
> we need some careful auditing & testing here, which I won't be
> able to do until I'm back from leave). Mostly due to the change in when
> we do the work.
> pcibios_fixup_bus() used to be called early on in the initial scan pass.
> Your code causes the code to be called -much- later when registering the
> device with the device model. Are we 100% certain nothing will happen in
> between that might rely on the stuff being setup already ? It might well
> be ok, but I want us to triple check that.
> Now, if we are ok to do the setup that late (basically right before the
> driver probe() routine gets called), would it make sense to simplify
> things even further ... and do it from pcibios_enable_device() ? Thus
> avoiding the notifier business completely or is that pushing it too
> far ?
As you said, there are times between pcibios_fixup_bus and device_add.
I'm agree with you.
But it is difficult for me to verify these by myself.
Can I ask for your help?
I will wait that you are back from leave.
> Also you seem to add:
> + /* Setup OF node pointer in the device */
> + dev->dev.of_node = pci_device_to_OF_node(dev);
> This shouldn't be needed anymore, the device node should be setup by the
> core nowadays. Is this just a remnant of you rebasing an old patch or do
> you have a good reason to add this statement ?
No, I don't have a good reason to add this statement.
I just tried to make this code be same with pcibios_setup_bus_devices.
Thanks for your review and comment.
More information about the Linuxppc-dev