diff mbox

[v12,10/12] PCI: Assign unassigned bus resources in pci_scan_root_bus()

Message ID 1411498874-9864-11-git-send-email-Liviu.Dudau@arm.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Liviu Dudau Sept. 23, 2014, 7:01 p.m. UTC
If the firmware has not assigned all the bus resources and
we are not just probing the PCIe busses, it makes sense to
assign the unassigned resources in pci_scan_root_bus().

Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Cc: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
---
 drivers/pci/probe.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Liviu Dudau Sept. 24, 2014, 1:18 a.m. UTC | #1
On Tue, Sep 23, 2014 at 08:01:12PM +0100, Liviu Dudau wrote:
> If the firmware has not assigned all the bus resources and
> we are not just probing the PCIe busses, it makes sense to
> assign the unassigned resources in pci_scan_root_bus().
> 
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> ---
>  drivers/pci/probe.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index ef891d2..508cf61 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1953,6 +1953,9 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
>  	if (!found)
>  		pci_bus_update_busn_res_end(b, max);
>  
> +	if (!pci_has_flag(PCI_PROBE_ONLY))
> +		pci_assign_unassigned_bus_resources(b);
> +
>  	pci_bus_add_devices(b);
>  	return b;
>  }
> -- 
> 2.1.0
> 

Bjorn,

If you are OK with this patch, can you let me know how do you feel about
making pci_scan_root_bus() set up the bus->msi pointer as well?

Side note: I don't know exactly how to do it now, but setting bus->msi is
needed by platforms that are based on my patches and want to use MSI,
otherwise they have to open code pci_scan_root_bus() to set it. I haven't
made any attempts to do it in this series as the GICv2m patches are not
ready yet, but I can see the need arriving soon.

Best regards,
Liviu

> --
> 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
>
Bjorn Helgaas Sept. 24, 2014, 1:41 a.m. UTC | #2
On Tue, Sep 23, 2014 at 7:18 PM, Liviu Dudau <liviu@dudau.co.uk> wrote:
> On Tue, Sep 23, 2014 at 08:01:12PM +0100, Liviu Dudau wrote:
>> If the firmware has not assigned all the bus resources and
>> we are not just probing the PCIe busses, it makes sense to
>> assign the unassigned resources in pci_scan_root_bus().
>>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
>> ---
>>  drivers/pci/probe.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index ef891d2..508cf61 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -1953,6 +1953,9 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
>>       if (!found)
>>               pci_bus_update_busn_res_end(b, max);
>>
>> +     if (!pci_has_flag(PCI_PROBE_ONLY))
>> +             pci_assign_unassigned_bus_resources(b);
>> +
>>       pci_bus_add_devices(b);
>>       return b;
>>  }
>> --
>> 2.1.0
>>
>
> Bjorn,
>
> If you are OK with this patch, can you let me know how do you feel about
> making pci_scan_root_bus() set up the bus->msi pointer as well?

I'm not opposed to it, but I have the same question as for setting up
the domain: how does pci_scan_root_bus() learn what to assign to
bus->msi?  It currently only gets a "void *sysdata" so there's no
obvious place to put it there.  You could add a pcibios interface to
retrieve it, I suppose, but I'm starting to get uncomfortable with
adding more of those because we have such a mess of them already.

Bjorn

> Side note: I don't know exactly how to do it now, but setting bus->msi is
> needed by platforms that are based on my patches and want to use MSI,
> otherwise they have to open code pci_scan_root_bus() to set it. I haven't
> made any attempts to do it in this series as the GICv2m patches are not
> ready yet, but I can see the need arriving soon.
>
> Best regards,
> Liviu
>
>> --
>> 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
>>
>
> --
> -------------------
>    .oooO
>    (   )
>     \ (  Oooo.
>      \_) (   )
>           ) /
>          (_/
>
>  One small step
>    for me ...
>
--
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
Liviu Dudau Sept. 24, 2014, 1:48 a.m. UTC | #3
On Tue, Sep 23, 2014 at 07:41:35PM -0600, Bjorn Helgaas wrote:
> On Tue, Sep 23, 2014 at 7:18 PM, Liviu Dudau <liviu@dudau.co.uk> wrote:
> > On Tue, Sep 23, 2014 at 08:01:12PM +0100, Liviu Dudau wrote:
> >> If the firmware has not assigned all the bus resources and
> >> we are not just probing the PCIe busses, it makes sense to
> >> assign the unassigned resources in pci_scan_root_bus().
> >>
> >> Cc: Bjorn Helgaas <bhelgaas@google.com>
> >> Cc: Arnd Bergmann <arnd@arndb.de>
> >> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> >> Cc: Rob Herring <robh+dt@kernel.org>
> >> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> >> ---
> >>  drivers/pci/probe.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> >> index ef891d2..508cf61 100644
> >> --- a/drivers/pci/probe.c
> >> +++ b/drivers/pci/probe.c
> >> @@ -1953,6 +1953,9 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
> >>       if (!found)
> >>               pci_bus_update_busn_res_end(b, max);
> >>
> >> +     if (!pci_has_flag(PCI_PROBE_ONLY))
> >> +             pci_assign_unassigned_bus_resources(b);
> >> +
> >>       pci_bus_add_devices(b);
> >>       return b;
> >>  }
> >> --
> >> 2.1.0
> >>
> >
> > Bjorn,
> >
> > If you are OK with this patch, can you let me know how do you feel about
> > making pci_scan_root_bus() set up the bus->msi pointer as well?
> 
> I'm not opposed to it, but I have the same question as for setting up
> the domain: how does pci_scan_root_bus() learn what to assign to
> bus->msi?  It currently only gets a "void *sysdata" so there's no
> obvious place to put it there.  You could add a pcibios interface to
> retrieve it, I suppose, but I'm starting to get uncomfortable with
> adding more of those because we have such a mess of them already.

My long term view is that the creation of the pci_host_bridge structure
needs to be separated from the creation and scanning of the root bus.
Then we can add host bridge ops or some other API to the structure
that should allow for less friction in passing information to the bridge.

Best regards,
Liviu

> 
> Bjorn
> 
> > Side note: I don't know exactly how to do it now, but setting bus->msi is
> > needed by platforms that are based on my patches and want to use MSI,
> > otherwise they have to open code pci_scan_root_bus() to set it. I haven't
> > made any attempts to do it in this series as the GICv2m patches are not
> > ready yet, but I can see the need arriving soon.
> >
> > Best regards,
> > Liviu
> >
> >> --
> >> 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
Suravee Suthikulpanit Sept. 28, 2014, 9:16 p.m. UTC | #4
On 09/23/2014 08:41 PM, Bjorn Helgaas wrote:
> On Tue, Sep 23, 2014 at 7:18 PM, Liviu Dudau <liviu@dudau.co.uk> wrote:
>> On Tue, Sep 23, 2014 at 08:01:12PM +0100, Liviu Dudau wrote:
>>> If the firmware has not assigned all the bus resources and
>>> we are not just probing the PCIe busses, it makes sense to
>>> assign the unassigned resources in pci_scan_root_bus().
>>>
>>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
>>> Cc: Rob Herring <robh+dt@kernel.org>
>>> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
>>> ---
>>>   drivers/pci/probe.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>>> index ef891d2..508cf61 100644
>>> --- a/drivers/pci/probe.c
>>> +++ b/drivers/pci/probe.c
>>> @@ -1953,6 +1953,9 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
>>>        if (!found)
>>>                pci_bus_update_busn_res_end(b, max);
>>>
>>> +     if (!pci_has_flag(PCI_PROBE_ONLY))
>>> +             pci_assign_unassigned_bus_resources(b);
>>> +
>>>        pci_bus_add_devices(b);
>>>        return b;
>>>   }
>>> --
>>> 2.1.0
>>>
>>
>> Bjorn,
>>
>> If you are OK with this patch, can you let me know how do you feel about
>> making pci_scan_root_bus() set up the bus->msi pointer as well?
>
> I'm not opposed to it, but I have the same question as for setting up
> the domain: how does pci_scan_root_bus() learn what to assign to
> bus->msi?  It currently only gets a "void *sysdata" so there's no
> obvious place to put it there.  You could add a pcibios interface to
> retrieve it, I suppose, but I'm starting to get uncomfortable with
> adding more of those because we have such a mess of them already.
>
> Bjorn

[Suravee] Liviu and I had a talk during Linaro Connect, and we came up
with a new binding for the pcie controller called "msi-parent" which is
supposed to contain phandle to the corresponded msi-controller.

>
>> Side note: I don't know exactly how to do it now, but setting bus->msi is
>> needed by platforms that are based on my patches and want to use MSI,
>> otherwise they have to open code pci_scan_root_bus() to set it. I haven't
>> made any attempts to do it in this series as the GICv2m patches are not
>> ready yet, but I can see the need arriving soon.

[Suravee] I just sent out an RFC which implements the described binding 
here.

https://lkml.org/lkml/2014/9/28/149

Any feedback/comments are welcome :)

Thanks,

Suravee
--
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
Arnd Bergmann Sept. 29, 2014, 6:18 p.m. UTC | #5
On Sunday 28 September 2014 16:16:21 Suravee Suthikulpanit wrote:
> >
> > I'm not opposed to it, but I have the same question as for setting up
> > the domain: how does pci_scan_root_bus() learn what to assign to
> > bus->msi?  It currently only gets a "void *sysdata" so there's no
> > obvious place to put it there.  You could add a pcibios interface to
> > retrieve it, I suppose, but I'm starting to get uncomfortable with
> > adding more of those because we have such a mess of them already.
> >
> 
> [Suravee] Liviu and I had a talk during Linaro Connect, and we came up
> with a new binding for the pcie controller called "msi-parent" which is
> supposed to contain phandle to the corresponded msi-controller.

Is this based on the property that we have in the pci-mvebu driver?

I hope this is meant to be compatible so we can share the implementation.

	Arnd
--
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/probe.c b/drivers/pci/probe.c
index ef891d2..508cf61 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1953,6 +1953,9 @@  struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
 	if (!found)
 		pci_bus_update_busn_res_end(b, max);
 
+	if (!pci_has_flag(PCI_PROBE_ONLY))
+		pci_assign_unassigned_bus_resources(b);
+
 	pci_bus_add_devices(b);
 	return b;
 }