All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yijing Wang <wangyijing@huawei.com>
To: Arnd Bergmann <arnd@arndb.de>, linux-arm-kernel@lists.infradead.org
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	Liviu Dudau <liviu@dudau.co.uk>, Tony Luck <tony.luck@intel.com>,
	Russell King <linux@arm.linux.org.uk>,
	linux-mips@linux-mips.org, linux-sh@vger.kernel.org,
	linux-pci@vger.kernel.org, linux-xtensa@linux-xtensa.org,
	x86@kernel.org, sparclinux@vger.kernel.org,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	linux-alpha@vger.kernel.org, linux-ia64@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()
Date: Thu, 20 Nov 2014 09:49:05 +0800	[thread overview]
Message-ID: <546D4891.80503@huawei.com> (raw)
In-Reply-To: <1645037.uypArjN50l@wuerfel>

>> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
>> index 076c35c..97f9730 100644
>> --- a/arch/alpha/kernel/pci.c
>> +++ b/arch/alpha/kernel/pci.c
>> @@ -334,6 +334,8 @@ common_init_pci(void)
>>  
>>  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
>>  					hose, &resources);
>> +		if (bus)
>> +			pci_bus_add_devices(bus);
>>  		hose->bus = bus;
>>  		hose->need_domain_info = need_domain_info;
>>  		next_busno = bus->busn_res.end + 1;
> 
> How about making pci_bus_add_devices() handle a NULL argument to save
> the if() here and elsewhere?

This make sense, will update it, thanks!

>> diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
>> index 1bf60b1..f083688 100644
>> --- a/arch/mips/pci/pci.c
>> +++ b/arch/mips/pci/pci.c
>> @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
>>  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
>>  			pci_bus_size_bridges(bus);
>>  			pci_bus_assign_resources(bus);
>> +			pci_bus_add_devices(bus);
>>  		}
>>  	}
>>  }
> 
> This one looks wrong, I think you still want to call pci_bus_add_devices()
> even with PCI_PROBE_ONLY set.

Yes, this is my mistake, :(

> 
>> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
>> index 1f80a88..007466e 100644
>> --- a/arch/tile/kernel/pci.c
>> +++ b/arch/tile/kernel/pci.c
>> @@ -308,6 +308,8 @@ int __init pcibios_init(void)
>>  			pci_add_resource(&resources, &iomem_resource);
>>  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
>>  						controller, &resources);
>> +			if (bus)
>> +				pci_bus_add_devices(bus);
>>  			controller->root_bus = bus;
>>  			controller->last_busno = bus->busn_res.end;
>>  		}
> 
> Should the pci_bus_add_devices come after setting the bus numbers here?

I think it's doesn't matter, but move it backward is ok to me.

> 
> 	Arnd
> 
> .
> 


-- 
Thanks!
Yijing


WARNING: multiple messages have this Message-ID (diff)
From: Yijing Wang <wangyijing@huawei.com>
To: linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()
Date: Thu, 20 Nov 2014 01:49:05 +0000	[thread overview]
Message-ID: <546D4891.80503@huawei.com> (raw)
In-Reply-To: <1645037.uypArjN50l@wuerfel>

>> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
>> index 076c35c..97f9730 100644
>> --- a/arch/alpha/kernel/pci.c
>> +++ b/arch/alpha/kernel/pci.c
>> @@ -334,6 +334,8 @@ common_init_pci(void)
>>  
>>  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
>>  					hose, &resources);
>> +		if (bus)
>> +			pci_bus_add_devices(bus);
>>  		hose->bus = bus;
>>  		hose->need_domain_info = need_domain_info;
>>  		next_busno = bus->busn_res.end + 1;
> 
> How about making pci_bus_add_devices() handle a NULL argument to save
> the if() here and elsewhere?

This make sense, will update it, thanks!

>> diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
>> index 1bf60b1..f083688 100644
>> --- a/arch/mips/pci/pci.c
>> +++ b/arch/mips/pci/pci.c
>> @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
>>  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
>>  			pci_bus_size_bridges(bus);
>>  			pci_bus_assign_resources(bus);
>> +			pci_bus_add_devices(bus);
>>  		}
>>  	}
>>  }
> 
> This one looks wrong, I think you still want to call pci_bus_add_devices()
> even with PCI_PROBE_ONLY set.

Yes, this is my mistake, :(

> 
>> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
>> index 1f80a88..007466e 100644
>> --- a/arch/tile/kernel/pci.c
>> +++ b/arch/tile/kernel/pci.c
>> @@ -308,6 +308,8 @@ int __init pcibios_init(void)
>>  			pci_add_resource(&resources, &iomem_resource);
>>  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
>>  						controller, &resources);
>> +			if (bus)
>> +				pci_bus_add_devices(bus);
>>  			controller->root_bus = bus;
>>  			controller->last_busno = bus->busn_res.end;
>>  		}
> 
> Should the pci_bus_add_devices come after setting the bus numbers here?

I think it's doesn't matter, but move it backward is ok to me.

> 
> 	Arnd
> 
> .
> 


-- 
Thanks!
Yijing


WARNING: multiple messages have this Message-ID (diff)
From: Yijing Wang <wangyijing@huawei.com>
To: Arnd Bergmann <arnd@arndb.de>, <linux-arm-kernel@lists.infradead.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	Liviu Dudau <liviu@dudau.co.uk>,
	"Tony Luck" <tony.luck@intel.com>,
	Russell King <linux@arm.linux.org.uk>,
	<linux-mips@linux-mips.org>, <linux-sh@vger.kernel.org>,
	<linux-pci@vger.kernel.org>, <linux-xtensa@linux-xtensa.org>,
	<x86@kernel.org>, <sparclinux@vger.kernel.org>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	<linux-alpha@vger.kernel.org>, <linux-ia64@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()
Date: Thu, 20 Nov 2014 09:49:05 +0800	[thread overview]
Message-ID: <546D4891.80503@huawei.com> (raw)
In-Reply-To: <1645037.uypArjN50l@wuerfel>

>> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
>> index 076c35c..97f9730 100644
>> --- a/arch/alpha/kernel/pci.c
>> +++ b/arch/alpha/kernel/pci.c
>> @@ -334,6 +334,8 @@ common_init_pci(void)
>>  
>>  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
>>  					hose, &resources);
>> +		if (bus)
>> +			pci_bus_add_devices(bus);
>>  		hose->bus = bus;
>>  		hose->need_domain_info = need_domain_info;
>>  		next_busno = bus->busn_res.end + 1;
> 
> How about making pci_bus_add_devices() handle a NULL argument to save
> the if() here and elsewhere?

This make sense, will update it, thanks!

>> diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
>> index 1bf60b1..f083688 100644
>> --- a/arch/mips/pci/pci.c
>> +++ b/arch/mips/pci/pci.c
>> @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
>>  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
>>  			pci_bus_size_bridges(bus);
>>  			pci_bus_assign_resources(bus);
>> +			pci_bus_add_devices(bus);
>>  		}
>>  	}
>>  }
> 
> This one looks wrong, I think you still want to call pci_bus_add_devices()
> even with PCI_PROBE_ONLY set.

Yes, this is my mistake, :(

> 
>> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
>> index 1f80a88..007466e 100644
>> --- a/arch/tile/kernel/pci.c
>> +++ b/arch/tile/kernel/pci.c
>> @@ -308,6 +308,8 @@ int __init pcibios_init(void)
>>  			pci_add_resource(&resources, &iomem_resource);
>>  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
>>  						controller, &resources);
>> +			if (bus)
>> +				pci_bus_add_devices(bus);
>>  			controller->root_bus = bus;
>>  			controller->last_busno = bus->busn_res.end;
>>  		}
> 
> Should the pci_bus_add_devices come after setting the bus numbers here?

I think it's doesn't matter, but move it backward is ok to me.

> 
> 	Arnd
> 
> .
> 


-- 
Thanks!
Yijing

WARNING: multiple messages have this Message-ID (diff)
From: wangyijing@huawei.com (Yijing Wang)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()
Date: Thu, 20 Nov 2014 09:49:05 +0800	[thread overview]
Message-ID: <546D4891.80503@huawei.com> (raw)
In-Reply-To: <1645037.uypArjN50l@wuerfel>

>> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
>> index 076c35c..97f9730 100644
>> --- a/arch/alpha/kernel/pci.c
>> +++ b/arch/alpha/kernel/pci.c
>> @@ -334,6 +334,8 @@ common_init_pci(void)
>>  
>>  		bus = pci_scan_root_bus(NULL, next_busno, alpha_mv.pci_ops,
>>  					hose, &resources);
>> +		if (bus)
>> +			pci_bus_add_devices(bus);
>>  		hose->bus = bus;
>>  		hose->need_domain_info = need_domain_info;
>>  		next_busno = bus->busn_res.end + 1;
> 
> How about making pci_bus_add_devices() handle a NULL argument to save
> the if() here and elsewhere?

This make sense, will update it, thanks!

>> diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
>> index 1bf60b1..f083688 100644
>> --- a/arch/mips/pci/pci.c
>> +++ b/arch/mips/pci/pci.c
>> @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
>>  		if (!pci_has_flag(PCI_PROBE_ONLY)) {
>>  			pci_bus_size_bridges(bus);
>>  			pci_bus_assign_resources(bus);
>> +			pci_bus_add_devices(bus);
>>  		}
>>  	}
>>  }
> 
> This one looks wrong, I think you still want to call pci_bus_add_devices()
> even with PCI_PROBE_ONLY set.

Yes, this is my mistake, :(

> 
>> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
>> index 1f80a88..007466e 100644
>> --- a/arch/tile/kernel/pci.c
>> +++ b/arch/tile/kernel/pci.c
>> @@ -308,6 +308,8 @@ int __init pcibios_init(void)
>>  			pci_add_resource(&resources, &iomem_resource);
>>  			bus = pci_scan_root_bus(NULL, 0, controller->ops,
>>  						controller, &resources);
>> +			if (bus)
>> +				pci_bus_add_devices(bus);
>>  			controller->root_bus = bus;
>>  			controller->last_busno = bus->busn_res.end;
>>  		}
> 
> Should the pci_bus_add_devices come after setting the bus numbers here?

I think it's doesn't matter, but move it backward is ok to me.

> 
> 	Arnd
> 
> .
> 


-- 
Thanks!
Yijing

  parent reply	other threads:[~2014-11-20  1:49 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-19  6:52 [PATCH 0/5] PCI scan interfaces clean up Yijing Wang
2014-11-19  7:32 ` Yijing Wang
2014-11-19  7:32 ` Yijing Wang
2014-11-19  6:52 ` [PATCH 2/5] PCI: Introduce pci_scan_bus_simple() Yijing Wang
2014-11-19  7:32   ` Yijing Wang
2014-11-19  7:32   ` Yijing Wang
2014-11-19  6:52 ` [PATCH 4/5] PCI: Remove pci_scan_bus() and pci_scan_bus_parented() Yijing Wang
2014-11-19  7:32   ` Yijing Wang
2014-11-19  7:32   ` Yijing Wang
2014-11-19  6:52 ` [PATCH 3/5] PCI: Use pci_scan_bus_simple() in simple scene Yijing Wang
2014-11-19  7:32   ` Yijing Wang
2014-11-19  7:32   ` Yijing Wang
2014-11-19 12:28   ` Arnd Bergmann
2014-11-19 12:28     ` Arnd Bergmann
2014-11-19 12:28     ` Arnd Bergmann
2014-11-20  1:53     ` Yijing Wang
2014-11-20  1:53       ` Yijing Wang
2014-11-20  1:53       ` Yijing Wang
2014-11-19  6:52 ` [PATCH 5/5] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus() Yijing Wang
2014-11-19  7:32   ` Yijing Wang
2014-11-19  7:32   ` Yijing Wang
2014-11-19  7:32   ` Yijing Wang
2014-11-19  6:52   ` Yijing Wang
2014-11-19 11:49   ` Lorenzo Pieralisi
2014-11-19 11:49     ` Lorenzo Pieralisi
2014-11-19 11:49     ` Lorenzo Pieralisi
2014-11-20  1:14     ` Yijing Wang
2014-11-20  1:14       ` Yijing Wang
2014-11-20  1:14       ` Yijing Wang
2014-11-19 12:11   ` Arnd Bergmann
2014-11-19 12:11     ` Arnd Bergmann
2014-11-19 12:11     ` Arnd Bergmann
2014-11-19 19:03     ` Liviu Dudau
2014-11-19 19:03       ` Liviu Dudau
2014-11-19 19:03       ` Liviu Dudau
2014-11-20  1:49     ` Yijing Wang [this message]
2014-11-20  1:49       ` Yijing Wang
2014-11-20  1:49       ` Yijing Wang
2014-11-20  1:49       ` Yijing Wang
2014-11-19  6:52 ` [PATCH 1/5] PCI: Rip out pci_bus_add_devices() from pci_scan_bus() Yijing Wang
2014-11-19  7:32   ` Yijing Wang
2014-11-19  7:32   ` Yijing Wang
2014-11-19  7:32   ` Yijing Wang
2014-11-19  6:52   ` Yijing Wang

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=546D4891.80503@huawei.com \
    --to=wangyijing@huawei.com \
    --cc=arnd@arndb.de \
    --cc=bhelgaas@google.com \
    --cc=geert@linux-m68k.org \
    --cc=linux-alpha@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-ia64@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=linux-xtensa@linux-xtensa.org \
    --cc=linux@arm.linux.org.uk \
    --cc=liviu@dudau.co.uk \
    --cc=sparclinux@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    /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.