Re: [PATCH 1/3] PCI: add a callback to struct pci_host_bridge for adding a new device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Aug 17, 2018 at 5:25 PM Lorenzo Pieralisi
<lorenzo.pieralisi@xxxxxxx> wrote:
>
> On Thu, Aug 16, 2018 at 11:04:53PM +0200, Arnd Bergmann wrote:
> > On Thu, Aug 16, 2018 at 10:59 PM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
> > > On Wed, Aug 15, 2018 at 02:52:28PM -0500, Bjorn Helgaas wrote:
> > > > On Thu, Aug 02, 2018 at 05:54:30PM +0100, Lorenzo Pieralisi wrote:
> >
> > > > This patch seems OK to me.
> > > >
> > > > I don't really care about the prototype.  There's only one
> > > > pcibios_add_device() implementation (x86) that returns anything other
> > > > than 0, and that's a pretty obscure error case related to f9a37be0f02a
> > > > ("x86: Use PCI setup data"), which lets us use ROM data from boot
> > > > services.  Even then the only thing that happens is a WARN_ON().  A
> > > > more descriptive printk would be a lot more useful.
> > >
> > > Thinking about this some more, I'm not so sure about the connection
> > > with removing pcibios_add_device().  This host_bridge->add_dev() hook
> > > would be for host bridge-specific things, while pcibios_add_device()
> > > is for arch-specific things.
> > >
> > > I'd still love to get rid of pcibios_add_device() (especially the
> > > non-arch-specific things like the pci_claim_resource() in s390); I'm
> > > just not sure yet whether this particular patch is the vehicle.
> >
> > I think most of the arch-specific pcibios_* calls are actually
> > host bridge specific after all, it just so happens that they are
> > implemented on architectures that only have one specific
> > host bridge implementation, or that they are used on an
> > architecture that does something odd in one place and needs
> > to do something else in another place.
> >
> > For pci_claim_resource() we seem to be doing this in a number
> > of different places, but there isn't strictly a reason for that.
>
> pci_claim_resource() is needed if either arch code or the host
> controller driver does not trigger a resources assignment (which claims
> them while at it); in theory that's arch agnostic but it turned out to
> be very arch/platform specific - aka if we move s390 code to core code
> we will notice :) so pci_claim_resource() in a pcibios call is
> unfortunately legitimate - whether it can be moved out of it to
> generic code that's a very complicated problem.

The generic pci_host_probe() already does either pci_bus_claim_resources() or
pci_bus_size_bridges()/pci_bus_assign_resources()/pcie_bus_configure_settings()

I would hope that we can move a lot of implementations over to just
call this function, with the appropriate PCI_PROBE_ONLY setting.

       Arnd



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux