Hi, Bjorn
Thanks a lot for your review and comment.
I will rethink again.
Regards.
Hiroo MATSUMOTO
On Mon, Apr 16, 2012 at 12:32 AM, Hiroo Matsumoto
<matsumoto.hiroo@xxxxxxxxxxxxxx> wrote:
Hi, Bjorn
I wrote a code with bus_register_notifier().
A function that calls bus_register_notifer() is called from rootfs_initcall
as well as amd_iommu is.
Please review this.
Regards.
Hiroo MATSUMOTO
Hi, Bjorn
Thanks for your research and comment.
As you said, a way of registering code with bus_register_notifier(), which
will be called in device_add(), is better one than pcibios_enable_device().
I will try to write code with bus_register_notifier().
Regards.
Hiroo MATSUMOTO
On Wed, Apr 11, 2012 at 11:00 PM, Hiroo Matsumoto
<matsumoto.hiroo@xxxxxxxxxxxxxx> wrote:
Hi, Kaneshige
You are right!
Setting dma_ops in pcibios_enable_device is a nice way.
In PCI driver, pci_enable_device_xxx that calls pcibios_enable_device is
called before checking archdata.dma_ops.
I added code of checking and setting dma_ops to pcibios_enable_device in
arch/powerpc/kernel/pci-common.c.
It works good.
When I researched this, I thought the best route was to use the
bus_register_notifier() path, as amd_iommu_init_notifier() does.
We're filling in a struct device field, not a PCI field, and
bus_register_notifier() seems like a more generic path than relying on
pcibios_enable_device().
Bjorn
Signed-off-by: Hiroo MATSUMOTO <matsumoto.hiroo@xxxxxxxxxxxxxx>
diff --git a/arch/powerpc/kernel/pci-common.c
b/arch/powerpc/kernel/pci-common.c
index 32656f1..1fe1e25 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1083,6 +1083,17 @@ void __devinit pcibios_setup_bus_self(struct pci_bus
*bus)
ppc_md.pci_dma_bus_setup(bus);
}
+static inline void pcibios_set_dma_ops(struct pci_dev *dev)
+{
+ /* Hook up default DMA ops */
+ set_dma_ops(&dev->dev, pci_dma_ops);
+ set_dma_offset(&dev->dev, PCI_DRAM_OFFSET);
+
+ /* Additional platform DMA/iommu setup */
+ if (ppc_md.pci_dma_dev_setup)
+ ppc_md.pci_dma_dev_setup(dev);
+}
+
void __devinit pcibios_setup_bus_devices(struct pci_bus *bus)
{
struct pci_dev *dev;
@@ -1102,13 +1113,7 @@ void __devinit pcibios_setup_bus_devices(struct
pci_bus *bus)
*/
set_dev_node(&dev->dev, pcibus_to_node(dev->bus));
- /* Hook up default DMA ops */
- set_dma_ops(&dev->dev, pci_dma_ops);
- set_dma_offset(&dev->dev, PCI_DRAM_OFFSET);
-
- /* Additional platform DMA/iommu setup */
- if (ppc_md.pci_dma_dev_setup)
- ppc_md.pci_dma_dev_setup(dev);
+ pcibios_set_dma_ops(dev);
/* Read default IRQs and fixup if necessary */
pci_read_irq_line(dev);
@@ -1749,3 +1754,33 @@ static void fixup_hide_host_resource_fsl(struct
pci_dev *dev)
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID,
fixup_hide_host_resource_fsl);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID,
fixup_hide_host_resource_fsl);
+
+static int pcibios_device_change_notifier(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct pci_dev *dev = to_pci_dev(data);
+
+ if (!dev)
+ return 0;
I don't think you need this check.
+
+ switch (action) {
+ case BUS_NOTIFY_ADD_DEVICE:
+ if (!get_dma_ops(&dev->dev))
+ pcibios_set_dma_ops(dev);
+ break;
+ default:
+ break;
The "default:" case is superfluous.
+ }
+
+ return 0;
+}
+
+static struct notifier_block device_nb = {
+ .notifier_call = pcibios_device_change_notifier,
+};
+
+static int pcibios_bus_notifier_init(void)
+{
+ return bus_register_notifier(&pci_bus_type, &device_nb);
+}
+rootfs_initcall(pcibios_bus_notifier_init);
Instead of making this a rootfs_initcall(), can you call
bus_register_notifier() explicitly in pcibios_init()? That way
pcibios_device_change_notifier() should get called for *all* new PCI
devices, whether we find them at boot-time or they're hot-added later.
If you do that, I think you can move all the
pcibios_setup_bus_devices() code into the
pcibios_device_change_notifier() path, including the NUMA node and IRQ
fixups.
Your patch will also need a changelog.
Bjorn
--
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
