On 05/22/2012 09:33 PM, Hiroo Matsumoto wrote:
> +static int pcibios_device_change_notifier(struct notifier_block *nb,
> + unsigned long action, void *data)
> +{
> + struct pci_dev *dev = to_pci_dev(data);
> +
In the general case, we don't know what *data contains until we evaluate
'action'. This conversion should probably be moved inside the case:
statement.
> + switch (action) {
> + case BUS_NOTIFY_ADD_DEVICE:
> + /* Setup OF node pointer in the device */
> + dev->dev.of_node = pci_device_to_OF_node(dev);
> +
> + /* Fixup NUMA node as it may not be setup yet by the generic
> + * code and is needed by the DMA init
> + */
> + 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);
> +
> + /* Read default IRQs and fixup if necessary */
> + pci_read_irq_line(dev);
> + if (ppc_md.pci_irq_fixup)
> + ppc_md.pci_irq_fixup(dev);
> +
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static struct notifier_block device_nb = {
> + .notifier_call = pcibios_device_change_notifier,
> +};
This is just a nit pick, but I think the naming of
pcibios_device_change_notifier() is a bit misleading. It doesn't
actually notify anything, but instead it *handles* notifications.
Perhaps a better name would be pcibios_device_change_handler() or
pcibios_device_change_callback()?
Sincerely,
Jesse Larrew
Software Engineer, Linux on Power Kernel Team
IBM Linux Technology Center
Phone: (512) 973-2052 (T/L: 363-2052)
[email protected]
_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://lists.ozlabs.org/listinfo/linuxppc-dev
