linux-kernel - Re: [RFC PATCH 07/16] PCI: Separate pci_host_bridge creation out of pci_create_root

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2507218.mHliopJb05@wuerfel>
Date:	Mon, 17 Nov 2014 11:56:36 +0100
From:	Arnd Bergmann <arnd@...db.de>
To:	linuxppc-dev@...ts.ozlabs.org
Cc:	Yijing Wang <wangyijing@...wei.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Liviu Dudau <liviu@...au.co.uk>,
	Tony Luck <tony.luck@...el.com>,
	Russell King <linux@....linux.org.uk>,
	linux-pci@...r.kernel.org, x86@...nel.org,
	linux-kernel@...r.kernel.org, Xinwei Hu <huxinwei@...wei.com>,
	Thierry Reding <thierry.reding@...il.com>,
	Suravee.Suthikulpanit@....com, linux-ia64@...r.kernel.org,
	Thomas Gleixner <tglx@...utronix.de>,
	Wuyun <wuyun.wu@...wei.com>, linux-arm-kernel@...ts.infradead.org
Subject: Re: [RFC PATCH 07/16] PCI: Separate pci_host_bridge creation out of pci_create_root_bus()

On Monday 17 November 2014 18:21:41 Yijing Wang wrote:
> There are some common PCI infos like domain, msi_controller, these
> infos are saved in arch PCI sysdata, and lots arch specific functions
> like pci_domain_nr() and pcibios_msi_controller() required.
> We could separate pci_host_bridge creation out of pci_create_root_bus(),
> then we could put the common infos in, then we could eliminate
> the arch specifc functions.
> 
> Signed-off-by: Yijing Wang <wangyijing@...wei.com>
> ---
>  drivers/pci/host-bridge.c |   99 +++++++++++++++++++++++++++++++++
>  drivers/pci/probe.c       |  134 ++++++++++++++++----------------------------
>  include/linux/pci.h       |   11 +++-
>  3 files changed, 158 insertions(+), 86 deletions(-)
> 
> diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
> index 0e5f3c9..e31604f 100644
> --- a/drivers/pci/host-bridge.c
> +++ b/drivers/pci/host-bridge.c
> @@ -8,6 +8,105 @@
>  
>  #include "pci.h"
>  
> +LIST_HEAD(pci_host_bridge_list);
> +DECLARE_RWSEM(pci_host_bridge_sem);

Unless the pci_host_bridge_sem is accessed thousands of times per second,
it's normally better to use a simple mutex instead.

> +static struct resource busn_resource = {
> +	.name	= "PCI busn",
> +	.start	= 0,
> +	.end	= 255,
> +	.flags	= IORESOURCE_BUS,
> +};

I think it would be better to require callers to pass the bus resource
down to the function.

> +struct pci_host_bridge *pci_create_host_bridge(
> +		struct device *parent, u32 db, 
> +		struct pci_ops *ops, void *sysdata, 
> +		struct list_head *resources)
> +{

Do we still need to pass the 'sysdata' in here? If we are guaranteed to
have a device pointer, we should always be able to get the driver
private data from dev_get_drvdata(host->dev->parent).

> +	host = kzalloc(sizeof(*host), GFP_KERNEL);
> +	if (!host)
> +		return NULL;

devm_kzalloc maybe?

> +	if (!resources) {
> +		/* Use default IO/MEM/BUS resources*/
> +		pci_add_resource(&host->windows, &ioport_resource);
> +		pci_add_resource(&host->windows, &iomem_resource);
> +		pci_add_resource(&host->windows, &busn_resource);
> +	} else {
> +		list_for_each_entry_safe(window, n, resources, list)
> +			list_move_tail(&window->list, &host->windows);
> +	}

I think we should assume that the correct resources are passed. You
could add a wrapper around this function to convert old platforms
though.

> +EXPORT_SYMBOL(pci_create_host_bridge);

EXPORT_SYMBOL_GPL() maybe?

> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 8b11b38..daa7f40 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -402,7 +402,12 @@ struct pci_host_bridge_window {
>  struct pci_host_bridge {
>  	struct device dev;
>  	struct pci_bus *bus;		/* root bus */
> +	struct list_head list;
>  	struct list_head windows;	/* pci_host_bridge_windows */
> +	int busnum;

The busnum should already be implied through the bus resource.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/