Add pcibios_device_change_notifier

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.

Quoted 8 lines

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.

Quoted 6 lines

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 ?

Also you seem to add:

(quoted diff) +2 -0

+               /* 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 ?

Cheers,
Ben.

Quoted 6 lines

Also you seem to add:

(quoted diff) +2 -0

+               /* Setup OF node pointer in the device */
+               dev->dev.of_node = pci_device_to_OF_node(dev);

Quoted 3 lines

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.

Quoted 8 lines · Hiroo Matsumoto

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.

Quoted 6 lines

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 ?

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 ?

Quoted 11 lines

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.

Here's my theory on this: we're setting up DMA and IRQ stuff. DMA and IRQ usage is device-specific, so the core can't do anything with them.
Only drivers know how to use them. Drivers can't find the device
until it's registered with the device model. So it seems like it should be safe to move it later. Subject to thinkos and testing in the real world, of course :)

Quoted 5 lines

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 ?

Kenji-san actually suggested using pcibios_enable_device() early on, and I'm the one who suggested the notifiers instead. I think I suggested that because I was copying the amd_iommu_init_notifier() style.

But I now think that might have been a mistake. Notifiers are definitely more complicated, and a pcibios_*() hook seems straightforward. It could be in pcibios_enable_device(), though we only need it to be called once, and the enable_device() path may be called many times, e.g., every time a driver claims it. My new vote is a pcibios_device_add(), with an empty weak definition in drivers/pci, and a non-empty definition for microblaze and powerpc.

Quoted 8 lines

Also you seem to add:

(quoted diff) +2 -0

+               /* 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 ?

It sounds like you want to remove this line in any case, so I'll wait for updated patches.