All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Thierry Reding <thierry.reding@gmail.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	Arnd Bergmann <arnd@arndb.de>, Tomasz Nowicki <tn@semihalf.com>,
	Liviu Dudau <liviu.dudau@arm.com>,
	linux-pci@vger.kernel.org, linux-tegra@vger.kernel.org
Subject: Re: [PATCH v2 1/2] PCI: Add new method for registering PCI hosts
Date: Thu, 28 Jul 2016 15:43:28 -0500	[thread overview]
Message-ID: <20160728204328.GA20218@localhost> (raw)
In-Reply-To: <20160630151931.29216-1-thierry.reding@gmail.com>

Hi Thierry and Arnd,

Thanks a lot for pushing this forward.  I would like to have gotten
this into the v4.8 merge window, but I didn't get to it soon enough.

On Thu, Jun 30, 2016 at 05:19:30PM +0200, Thierry Reding wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> This patch makes the existing pci_host_bridge structure a proper device
> that is usable by PCI host drivers in a more standard way. In addition
> to the existing pci_scan_bus, pci_scan_root_bus, pci_scan_root_bus_msi,
> and pci_create_root_bus interfaces, this unfortunately means having to
> add yet another interface doing basically the same thing, and add some
> extra code in the initial step.
> 
> However, this time it's more likely to be extensible enough that we
> won't have to do another one again in the future, and we should be
> able to reduce code much more as a result.
> 
> The main idea is to pull the allocation of 'struct pci_host_bridge' out
> of the registration, and let individual host drivers and architecture
> code fill the members before calling the registration function.
> 
> There are a number of things we can do based on this:
> 
> * Use a single memory allocation for the driver-specific structure
>   and the generic PCI host bridge
> * consolidate the contents of driver specific structures by moving
>   them into pci_host_bridge
> * Add a consistent interface for removing a PCI host bridge again
>   when unloading a host driver module
> * Replace the architecture specific __weak pcibios_* functions with
>   callbacks in a pci_host_bridge device
> * Move common boilerplate code from host drivers into the generic
>   function, based on contents of the structure
> * Extend pci_host_bridge with additional members when needed without
>   having to add arguments to pci_scan_*.
> * Move members of struct pci_bus into pci_host_bridge to avoid
>   having lots of identical copies.
> 
> As mentioned in a previous email, one open question is whether we want
> to export a function for allocating a pci_host_bridge device in
> combination with the per-device structure or let the driver itself
> call kzalloc.

I agree with Arnd that a pci_host_bridge_alloc() would be nice.  The
pointer to alloc_etherdev() was very useful and probably worth
including in the changelog.  It helps a lot if we can copy the style
of some other subsystem, but my head is buried so deep in PCI that I
don't know what they do.

Looking forward to the next iteration.

Bjorn

WARNING: multiple messages have this Message-ID (diff)
From: Bjorn Helgaas <helgaas-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Bjorn Helgaas <bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
	Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>,
	Tomasz Nowicki <tn-nYOzD4b6Jr9Wk0Htik3J/w@public.gmane.org>,
	Liviu Dudau <liviu.dudau-5wv7dgnIgG8@public.gmane.org>,
	linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH v2 1/2] PCI: Add new method for registering PCI hosts
Date: Thu, 28 Jul 2016 15:43:28 -0500	[thread overview]
Message-ID: <20160728204328.GA20218@localhost> (raw)
In-Reply-To: <20160630151931.29216-1-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Hi Thierry and Arnd,

Thanks a lot for pushing this forward.  I would like to have gotten
this into the v4.8 merge window, but I didn't get to it soon enough.

On Thu, Jun 30, 2016 at 05:19:30PM +0200, Thierry Reding wrote:
> From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> 
> This patch makes the existing pci_host_bridge structure a proper device
> that is usable by PCI host drivers in a more standard way. In addition
> to the existing pci_scan_bus, pci_scan_root_bus, pci_scan_root_bus_msi,
> and pci_create_root_bus interfaces, this unfortunately means having to
> add yet another interface doing basically the same thing, and add some
> extra code in the initial step.
> 
> However, this time it's more likely to be extensible enough that we
> won't have to do another one again in the future, and we should be
> able to reduce code much more as a result.
> 
> The main idea is to pull the allocation of 'struct pci_host_bridge' out
> of the registration, and let individual host drivers and architecture
> code fill the members before calling the registration function.
> 
> There are a number of things we can do based on this:
> 
> * Use a single memory allocation for the driver-specific structure
>   and the generic PCI host bridge
> * consolidate the contents of driver specific structures by moving
>   them into pci_host_bridge
> * Add a consistent interface for removing a PCI host bridge again
>   when unloading a host driver module
> * Replace the architecture specific __weak pcibios_* functions with
>   callbacks in a pci_host_bridge device
> * Move common boilerplate code from host drivers into the generic
>   function, based on contents of the structure
> * Extend pci_host_bridge with additional members when needed without
>   having to add arguments to pci_scan_*.
> * Move members of struct pci_bus into pci_host_bridge to avoid
>   having lots of identical copies.
> 
> As mentioned in a previous email, one open question is whether we want
> to export a function for allocating a pci_host_bridge device in
> combination with the per-device structure or let the driver itself
> call kzalloc.

I agree with Arnd that a pci_host_bridge_alloc() would be nice.  The
pointer to alloc_etherdev() was very useful and probably worth
including in the changelog.  It helps a lot if we can copy the style
of some other subsystem, but my head is buried so deep in PCI that I
don't know what they do.

Looking forward to the next iteration.

Bjorn

  parent reply	other threads:[~2016-07-28 20:43 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-30 15:19 [PATCH v2 1/2] PCI: Add new method for registering PCI hosts Thierry Reding
2016-06-30 15:19 ` Thierry Reding
2016-06-30 15:19 ` [PATCH v2 2/2] PCI: tegra: Use new pci_register_host() interface Thierry Reding
2016-06-30 15:37 ` [PATCH v2 1/2] PCI: Add new method for registering PCI hosts Arnd Bergmann
2016-06-30 15:37   ` Arnd Bergmann
2016-07-01 14:14   ` Liviu Dudau
2016-07-01 14:14     ` Liviu Dudau
2016-07-01 14:24     ` Arnd Bergmann
2016-07-01 14:24       ` Arnd Bergmann
2016-07-01 14:52       ` Liviu Dudau
2016-07-01 14:52         ` Liviu Dudau
2016-07-01 15:17         ` Arnd Bergmann
2016-07-01 15:17           ` Arnd Bergmann
2016-07-01 15:40           ` Liviu Dudau
2016-07-01 15:58             ` Arnd Bergmann
2016-07-01 15:58               ` Arnd Bergmann
2016-07-01 14:46 ` Liviu Dudau
2016-07-01 14:46   ` Liviu Dudau
2016-07-01 15:44   ` Arnd Bergmann
2016-07-01 15:44     ` Arnd Bergmann
2016-07-01 16:09     ` Liviu Dudau
2016-07-01 16:30       ` Arnd Bergmann
2016-07-04  9:56         ` Liviu Dudau
2016-07-04 13:46           ` Arnd Bergmann
2016-07-28 20:43 ` Bjorn Helgaas [this message]
2016-07-28 20:43   ` Bjorn Helgaas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160728204328.GA20218@localhost \
    --to=helgaas@kernel.org \
    --cc=arnd@arndb.de \
    --cc=bhelgaas@google.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=liviu.dudau@arm.com \
    --cc=thierry.reding@gmail.com \
    --cc=tn@semihalf.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.