Re: [Resend] pcibios_add_platform_entries usage

[Sebastian Ott <sebott@linux.vnet.ibm.com>]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2243 bytes --]

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