diff mbox

[3/4] PCI: designware: remove pci_assign_unassigned_resources call

Message ID 1406137961-14684-4-git-send-email-l.stach@pengutronix.de (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Lucas Stach July 23, 2014, 5:52 p.m. UTC
The pci_common_init_dev() call right before will already
handle the device resource allocation, so this call
was a no-op.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/pci/host/pcie-designware.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Pratyush Anand Aug. 16, 2014, 1:13 p.m. UTC | #1
On Wed, Jul 23, 2014 at 11:22 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
> The pci_common_init_dev() call right before will already
> handle the device resource allocation, so this call
> was a no-op.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  drivers/pci/host/pcie-designware.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index b869e202367c..dde5e6d4afa2 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -568,7 +568,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>         dw_pci.private_data = (void **)&pp;
>
>         pci_common_init_dev(pp->dev, &dw_pci);
> -       pci_assign_unassigned_resources();

I am not sure here. IIRC, then calling  pci_assign_unassigned_resources after
pci_common_init_dev was needed to handle few PCIe cards with EP behind
bridge.

I do not have good understanding of pci resource allocation code.
@Yinghai: Can you please help(rather teach) with the description of
different resource allocator
available in setup-bus.c. Can try reading code, but if a documentation
exists, that would
be helpful.

 ~Pratyush
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yinghai Lu Aug. 17, 2014, 2:59 a.m. UTC | #2
On Sat, Aug 16, 2014 at 6:13 AM, Pratyush Anand
<pratyush.anand@gmail.com> wrote:
> On Wed, Jul 23, 2014 at 11:22 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
>
> I am not sure here. IIRC, then calling  pci_assign_unassigned_resources after
> pci_common_init_dev was needed to handle few PCIe cards with EP behind
> bridge.
>
> I do not have good understanding of pci resource allocation code.
> @Yinghai: Can you please help(rather teach) with the description of
> different resource allocator
> available in setup-bus.c. Can try reading code, but if a documentation
> exists, that would
> be helpful.

pci_assign_unassigned_resources() should try several times to make sure
assign resource to PCI bars that are not assigned by BIOS or not valid value
from BIOS.
so it will honor the setting from BIOS.

in your arm case, pci_common_init_dev() is doing sizing and assign.
so you should not need that pci_assign_unassigned_resources() anymore.

Or your setup have PCI_PROBE_ONLY ?

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pratyush Anand Aug. 17, 2014, 6:20 a.m. UTC | #3
On Sun, Aug 17, 2014 at 8:29 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Sat, Aug 16, 2014 at 6:13 AM, Pratyush Anand
> <pratyush.anand@gmail.com> wrote:
>> On Wed, Jul 23, 2014 at 11:22 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
>>
>> I am not sure here. IIRC, then calling  pci_assign_unassigned_resources after
>> pci_common_init_dev was needed to handle few PCIe cards with EP behind
>> bridge.
>>
>> I do not have good understanding of pci resource allocation code.
>> @Yinghai: Can you please help(rather teach) with the description of
>> different resource allocator
>> available in setup-bus.c. Can try reading code, but if a documentation
>> exists, that would
>> be helpful.
>
> pci_assign_unassigned_resources() should try several times to make sure
> assign resource to PCI bars that are not assigned by BIOS or not valid value
> from BIOS.
> so it will honor the setting from BIOS.
>
> in your arm case, pci_common_init_dev() is doing sizing and assign.
> so you should not need that pci_assign_unassigned_resources() anymore.
>
> Or your setup have PCI_PROBE_ONLY ?

Thanks for the clarification.

I think none of the designware based platform boots with *firmware*, so none of
the setup should have PCI_PROBE_ONLY. Then patch seems fine.

~Pratyush

>
> Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Murali Karicheri Aug. 18, 2014, 3:39 p.m. UTC | #4
On 08/17/2014 02:20 AM, Pratyush Anand wrote:
> On Sun, Aug 17, 2014 at 8:29 AM, Yinghai Lu<yinghai@kernel.org>  wrote:
>> On Sat, Aug 16, 2014 at 6:13 AM, Pratyush Anand
>> <pratyush.anand@gmail.com>  wrote:
>>> On Wed, Jul 23, 2014 at 11:22 PM, Lucas Stach<l.stach@pengutronix.de>  wrote:
>>>
>>> I am not sure here. IIRC, then calling  pci_assign_unassigned_resources after
>>> pci_common_init_dev was needed to handle few PCIe cards with EP behind
>>> bridge.
>>>
>>> I do not have good understanding of pci resource allocation code.
>>> @Yinghai: Can you please help(rather teach) with the description of
>>> different resource allocator
>>> available in setup-bus.c. Can try reading code, but if a documentation
>>> exists, that would
>>> be helpful.
>>
>> pci_assign_unassigned_resources() should try several times to make sure
>> assign resource to PCI bars that are not assigned by BIOS or not valid value
>> from BIOS.
>> so it will honor the setting from BIOS.
>>
>> in your arm case, pci_common_init_dev() is doing sizing and assign.
>> so you should not need that pci_assign_unassigned_resources() anymore.
>>
>> Or your setup have PCI_PROBE_ONLY ?
>
> Thanks for the clarification.
>
> I think none of the designware based platform boots with *firmware*, so none of
> the setup should have PCI_PROBE_ONLY. Then patch seems fine.
>
There are customers using Keystone platform on which u-boot sets up BAR 
and use firmware option to honor BAR assignment done by the boot loader. 
How does this change that use case? If this still works with this 
change, no issues. If not, I would suggest this shouldn't be removed. 
Any expert here who can comment on this? Keystone driver is just getting 
merged to upstream branch and is based on designware.

Murali
> ~Pratyush
>
>>
>> Yinghai
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index b869e202367c..dde5e6d4afa2 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -568,7 +568,6 @@  int __init dw_pcie_host_init(struct pcie_port *pp)
 	dw_pci.private_data = (void **)&pp;
 
 	pci_common_init_dev(pp->dev, &dw_pci);
-	pci_assign_unassigned_resources();
 #ifdef CONFIG_PCI_DOMAINS
 	dw_pci.domain++;
 #endif