Hello Bjorn, On Mon, 14 Apr 2014, Bjorn Helgaas wrote: > [+cc Greg] > > On Mon, Apr 14, 2014 at 11:03:42AM +0200, Sebastian Ott wrote: > > Hello Bjorn, > > > > for pci on s390 we currently use pcibios_add_platform_entries to add > > some arch specific attributes to pdevs. This has 2 downsides - it will > > race with userspace which is triggered by udev events and expecting > > these attributes (but that's a theoretical issue). More important to > > me is that one cannot use attribute_groups with this. Both issues could > > be addressed by using pdev->dev.groups and let the driver core handle > > attribute creation. > > > > So would it be ok if we set pdev->dev.groups in pcibios_add_device? > > (It should be since it's not used by pci common code which uses bus_type, > > dev_type, and class groups). > > Hi Sebastian, > > Sorry, I meant to respond to this earlier, but forgot. This sounds > reasonable to me, but Greg can give you a much better answer than I can. > > Documentation/driver-model/device.txt says the dev->groups pointer > should be set before calling device_register(). PCI calls > device_initialize() and device_add() instead of using device_register(), > and pcibios_add_device() looks like it happens at the right time: > > pci_scan_root_bus > pci_scan_child_bus > pci_scan_slot > pci_scan_single_device > pci_device_add > device_initialize > pcibios_add_device # <--- > device_add > pci_bus_add_devices > pci_bus_add_device > pci_create_sysfs_dev_files > pcibios_add_platform_entries # 8d4cd0833107 (benh) > device_attach > > I'm not sure why pci_create_sysfs_dev_files() is done later. It seems > like that should be done before device_add() as well. Maybe it's > because BARs might not be valid yet (that doesn't seem like a very good > excuse, but it's all I can think of). > > I assume that if you change s390, you'll also change microblaze and > powerpc? They look structurally similar to s390. Yes, that sounds like a plan - this way we can get rid of pcibios_add_platform_entries altogether. I'll send these patches soon. Regards, Sebastian