Re: [RFC] pciehp: Add archdata setting

[Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>]

Hi,

I have some comments.

- I think the patch should be split into two, for microblaze and
  for powerpc.

- The following code looks redundant. I think dev->is_added is
  always false at the BUS_NOTIFY_ADD_DEVICE time.

+		/* Cardbus can call us to add new devices to a bus, so ignore
+		 * those who are already fully discovered
+		 */
+		if (dev->is_added)
+			break;

- You need to add patch description.

Regards,
Kenji Kaneshige


(2012/05/10 19:38), Hiroo Matsumoto wrote:
> Hi, Bjorn
> 
> 
> Refer to your review and comment, I updated bus notifier code.
> This works good. Thanks for your comments.
> And I think this way can be applied to microblaze.
> Please review this patch.
> 
> 
> * Changes
> 1. Moving pcibios_setup_bus_devices code to pcibios_device_change_notifier so that boot and hotplug use same code. This will change boot message on powerpc platform but there is no difference. Please see "Not patched boot message" and "Patched message".
> 2. Calling bus_register_notifier in pcibios_init.
> 3. Using bus notifier not only on powerpc platform but also on microblaze platform because microblaze pcibios_setup_bus_devices is a similer way with powerpc.
> 4. Removing caller and callee of pcibios_setup_bus_devices because bus notifier works instead of pcibios_setup_bus_devices.
> 
> * Not patched boot message
> PCI: Probing PCI hardware
> <snip>
> pci 0000:00:00.0: PCI bridge to [bus 01-ff]
> pci 0000:00:00.0: bridge window [io 0x0000-0x0000] (disabled)
> pci 0000:00:00.0: bridge window [mem 0xa0000000-0xa01fffff]
> pci 0000:00:00.0: bridge window [mem 0x10000000-0x000fffff pref] (disabled)
> irq: irq 4 on host /soc@ffe00000/pic@40000 mapped to virtual irq 16
> <snip>
> pci 0000:01:00.0: PCI bridge to [bus 02-ff]
> pci 0000:01:00.0: bridge window [io 0x1000-0x1fff]
> pci 0000:01:00.0: bridge window [mem 0xa0100000-0xa01fffff]
> pci 0000:01:00.0: bridge window [mem 0x10000000-0x000fffff pref] (disabled)
> irq: irq 5 on host /soc@ffe00000/pic@40000 mapped to virtual irq 17
> irq: irq 7 on host /soc@ffe00000/pic@40000 mapped to virtual irq 18
> 
> * Patched boot message
> PCI: Probing PCI hardware
> <snip>
> pci 0000:00:00.0: PCI bridge to [bus 01-ff]
> pci 0000:00:00.0: bridge window [io 0x0000-0x0000] (disabled)
> pci 0000:00:00.0: bridge window [mem 0xa0000000-0xa01fffff]
> pci 0000:00:00.0: bridge window [mem 0x10000000-0x000fffff pref] (disabled)
> <snip>
> pci 0000:01:00.0: PCI bridge to [bus 02-ff]
> pci 0000:01:00.0: bridge window [io 0x1000-0x1fff]
> pci 0000:01:00.0: bridge window [mem 0xa0100000-0xa01fffff]
> pci 0000:01:00.0: bridge window [mem 0x10000000-0x000fffff pref] (disabled)
> <snip>
> irq: irq 4 on host /soc@ffe00000/pic@40000 mapped to virtual irq 16
> irq: irq 5 on host /soc@ffe00000/pic@40000 mapped to virtual irq 17
> irq: irq 7 on host /soc@ffe00000/pic@40000 mapped to virtual irq 18
> 
> * Not patched pciehp message on powerpc platform
> # echo 1 > /sys/bus/pci/slots/1/power
> <snip>
> pcieport 0000:02:01.0: PCI bridge to [bus 03-03]
> pcieport 0000:02:01.0: bridge window [io 0xff7ee000-0xff7eefff]
> pcieport 0000:02:01.0: bridge window [mem 0xa0100000-0xa01fffff]
> pcieport 0000:02:01.0: bridge window [mem 0xa0200000-0xa02fffff 64bit pref]
> pci 0000:03:00.0: no hotplug settings from platform
> e1000e 0000:03:00.0: Disabling ASPM L1
> e1000e 0000:03:00.0: enabling device (0000 -> 0002)
> e1000e 0000:03:00.0: No usable DMA configuration, aborting
> e1000e: probe of 0000:03:00.0 failed with error -5
> 
> * Patched pciehp message on powerpc platform
> # echo 1 > /sys/bus/pci/slots/1/power
> <snip>
> pcieport 0000:02:01.0: PCI bridge to [bus 03-03]
> pcieport 0000:02:01.0: bridge window [io 0xff7ee000-0xff7eefff]
> pcieport 0000:02:01.0: bridge window [mem 0xa0100000-0xa01fffff]
> pcieport 0000:02:01.0: bridge window [mem 0xa0200000-0xa02fffff 64bit pref]
> pci 0000:03:00.0: no hotplug settings from platform
> e1000e 0000:03:00.0: Disabling ASPM L1
> e1000e 0000:03:00.0: enabling device (0000 -> 0002)
> irq: irq 6 on host /soc@ffe00000/msi@41600 mapped to virtual irq 27
> e1000e 0000:03:00.0: eth0: (PCI Express:2.5GT/s:Width x1) 00:15:17:bf:c0:c9
> e1000e 0000:03:00.0: eth0: Intel(R) PRO/1000 Network Connection
> e1000e 0000:03:00.0: eth0: MAC: 1, PHY: 4, PBA No: D50861-003
> 
> 
> Regards.
> 
> Hiroo MATSUMOTO
> 
> 
>> 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
>>
>>