linux-kernel - Re: [PATCH v6 12/30] PCI: Introduce pci_host_bridge

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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <55019145.2060504@huawei.com>
Date:	Thu, 12 Mar 2015 21:14:45 +0800
From:	Yijing Wang <wangyijing@...wei.com>
To:	Bjorn Helgaas <bhelgaas@...gle.com>
CC:	Jiang Liu <jiang.liu@...ux.intel.com>, <linux-pci@...r.kernel.org>,
	Yinghai Lu <yinghai@...nel.org>,
	<linux-kernel@...r.kernel.org>,
	Marc Zyngier <marc.zyngier@....com>,
	<linux-arm-kernel@...ts.infradead.org>,
	Russell King <linux@....linux.org.uk>, <x86@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Rusty Russell <rusty@...tcorp.com.au>,
	Tony Luck <tony.luck@...el.com>, <linux-ia64@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>,
	"Guan Xuetao" <gxt@...c.pku.edu.cn>, <linux-alpha@...r.kernel.org>,
	<linux-m68k@...ts.linux-m68k.org>, Liviu Dudau <liviu@...au.co.uk>,
	"Arnd Bergmann" <arnd@...db.de>,
	Geert Uytterhoeven <geert@...ux-m68k.org>
Subject: Re: [PATCH v6 12/30] PCI: Introduce pci_host_bridge_ops to support
 host specific operations

>>  void pci_free_host_bridge(struct pci_host_bridge *host);
>>  #endif /* DRIVERS_PCI_H */
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index c8e074d..59d9be3 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -1889,6 +1889,8 @@ static struct pci_bus *__pci_create_root_bus(
>>  
>>  	bridge->bus = b;
>>  	b->bridge = get_device(&bridge->dev);
>> +	if (bridge->ops && bridge->ops->set_root_bus_speed)
>> +		bridge->ops->set_root_bus_speed(bridge);
> 
> Before this patch, can you do this:
> 
>   - Rename the powerpc pcibios_root_bridge_prepare() to
>     pcibios_set_root_bus_speed() and add a call to it here
> 
>   - Move the pcibios_root_bridge_prepare() call to pci_create_host_bridge()
> 
> Then this patch will make a lot more sense because it will look like this,
> with the host bridge ops call right next to the matching pcibios call:
> 
>   +	if (host->ops && host->ops->prepare) {
>   +		error = host->ops->prepare(host);
>   +		...
>   +	}
> 	pcibios_root_bridge_prepare(...);
> 
>   +	if (bridge->ops && bridge->ops->set_root_bus_speed)
>   +		bridge->ops->set_root_bus_speed(bridge);
> 	pcibios_set_root_bus_speed(...);

OK, will update.

> 
>>  	error = pcibios_root_bridge_prepare(bridge);
>>  	if (error)
>>  		goto err_out;
>> @@ -1954,7 +1956,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, u32 db,
>>  {
>>  	struct pci_host_bridge *host;
>>  
>> -	host = pci_create_host_bridge(parent, db, resources, sysdata);
>> +	host = pci_create_host_bridge(parent, db, resources, sysdata, NULL);
>>  	if (!host)
>>  		return NULL;
>>  
>> @@ -2051,10 +2053,13 @@ static struct pci_bus *__pci_scan_root_bus(
>>  		pci_bus_insert_busn_res(b, b->number, 255);
>>  	}
>>  
>> -	max = pci_scan_child_bus(b);
>> -
>> -	if (!found)
>> -		pci_bus_update_busn_res_end(b, max);
>> +	if (host->ops && host->ops->scan_bus) {
>> +		host->ops->scan_bus(host);
>> +	} else {
>> +		max = pci_scan_child_bus(b);
>> +		if (!found)
>> +			pci_bus_update_busn_res_end(b, max);
>> +	}
> 
> I think host->ops->scan_bus() should have the same prototype as
> pci_scan_child_bus(), and it should return max, and you should do the same
> busn update as when you call pci_scan_child_bus().  So the code would look
> like this:
> 
>     if (host->ops && host->ops->scan_bus)
> 	max = host->ops->scan_bus(b);
>     else
> 	max = pci_scan_child_bus(b);
> 
>     if (!found)
> 	pci_bus_update_busn_res_end(b, max);
> 

OK, I will try to change the of scan_bus(), and make it return the max bus number like pci_scan_child_bus().

>>  
>>  	return b;
>>  }
>> @@ -2064,7 +2069,7 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, u32 db,
>>  {
>>  	struct pci_host_bridge *host;
>>  
>> -	host = pci_create_host_bridge(parent, db, resources, sysdata);
>> +	host = pci_create_host_bridge(parent, db, resources, sysdata, NULL);
>>  	if (!host)
>>  		return NULL;
>>  
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index b621f5b..e9922b1 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -400,6 +400,13 @@ static inline int pci_channel_offline(struct pci_dev *pdev)
>>  	return (pdev->error_state != pci_channel_io_normal);
>>  }
>>  
>> +struct pci_host_bridge;
>> +struct pci_host_bridge_ops {
>> +	void (*set_root_bus_speed)(struct pci_host_bridge *host);
>> +	int (*prepare)(struct pci_host_bridge *host);
>> +	void (*scan_bus)(struct pci_host_bridge *);
> 
> Needs an argument name to match the style of the other ops.

Sorry, I missed it, will add it, thanks.


> 
>> +};
>> +
>>  struct pci_host_bridge {
>>  	u16	domain;
>>  	u16 busnum;
>> @@ -407,6 +414,7 @@ struct pci_host_bridge {
>>  	struct pci_bus *bus;		/* root bus */
>>  	struct list_head windows;	/* resource_entry */
>>  	struct list_head list;
>> +	struct pci_host_bridge_ops *ops;
>>  	void (*release_fn)(struct pci_host_bridge *);
>>  	void *release_data;
>>  };
>> -- 
>> 1.7.1
>>
>> --
>> 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/
> 
> .
> 


-- 
Thanks!
Yijing

--
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/