diff mbox

PCI: add missing DT binding for linux,pci-domain property

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

Commit Message

Lucas Stach Nov. 4, 2014, 11:47 a.m. UTC
This property was added by 41e5c0f81d3e
(of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr())
without the required binding documentation. As this property
will be supported by a number of host bridge drivers going forward,
add it to the common PCI binding doc.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
This is a non-critical fix, but may still qualify for 3.18-rc as
the property was added in this release cycle.
---
 Documentation/devicetree/bindings/pci/pci.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Liviu Dudau Nov. 4, 2014, noon UTC | #1
On Tue, Nov 04, 2014 at 11:47:40AM +0000, Lucas Stach wrote:
> This property was added by 41e5c0f81d3e
> (of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr())
> without the required binding documentation. As this property
> will be supported by a number of host bridge drivers going forward,
> add it to the common PCI binding doc.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> This is a non-critical fix, but may still qualify for 3.18-rc as
> the property was added in this release cycle.

Hi Lucas,

Thanks for taking care of this, sorry it slipped through the cracks
of last minute changes in this area.

> ---
>  Documentation/devicetree/bindings/pci/pci.txt | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/pci.txt b/Documentation/devicetree/bindings/pci/pci.txt
> index 41aeed38926d..b754f786ed5e 100644
> --- a/Documentation/devicetree/bindings/pci/pci.txt
> +++ b/Documentation/devicetree/bindings/pci/pci.txt
> @@ -7,3 +7,13 @@ And for the interrupt mapping part:
>  
>  Open Firmware Recommended Practice: Interrupt Mapping
>  http://www.openfirmware.org/1275/practice/imap/imap0_9d.pdf
> +
> +Additionally to the properties specified in the above standards a host bridge
> +driver implementation may support the following properties:
> +
> +- linux,pci-domain:
> +   If present this property assigns a fixed PCI domain number to a host bridge,
> +   otherwise an unstable (across boots) unique number will be assigned.
> +   It is recommended to either not set this property at all or set it for all
> +   host bridges in the system, otherwise potentially conflicting domain numbers
> +   may be assigned to root buses behind different host bridges.
> -- 
> 2.1.1
> 

While the description is potentially correct, what it fails to explain is that the
choice of using the property or generating an unstable (across boots) unique
number is actually the choice of the host bridge driver at the moment. I know that
my earlier implementations were defaulting to the automatic numbering, but that has
been dropped from the final series as Rob Herring was objecting to it.

There is still scope to adopt a wide policy here, but for now it should say something
to the tune:

   If present this property assigns a fixed PCI domain number to a host bridge,
   otherwise an unstable (across boots) unique number will be assigned.
   If you decide to use the property to assign a fixed PCI domain number to a host
   bridge you have to ensure that all the host bridge drivers present in the system
   follow the same policy. Otherwise, potentially conflicting domain numbers
   may be assigned to root busses behind different host bridges.

Best regards,
Liviu
Arnd Bergmann Nov. 4, 2014, 12:07 p.m. UTC | #2
On Tuesday 04 November 2014 12:00:52 Liviu Dudau wrote:
> 
> While the description is potentially correct, what it fails to explain is that the
> choice of using the property or generating an unstable (across boots) unique
> number is actually the choice of the host bridge driver at the moment. I know that
> my earlier implementations were defaulting to the automatic numbering, but that has
> been dropped from the final series as Rob Herring was objecting to it.
> 
> There is still scope to adopt a wide policy here, but for now it should say something
> to the tune:
> 
>    If present this property assigns a fixed PCI domain number to a host bridge,
>    otherwise an unstable (across boots) unique number will be assigned.
>    If you decide to use the property to assign a fixed PCI domain number to a host
>    bridge you have to ensure that all the host bridge drivers present in the system
>    follow the same policy. Otherwise, potentially conflicting domain numbers
>    may be assigned to root busses behind different host bridges.

But with the latest change to the domain handling, all drivers would implement
this. I would just mention that Linux kernels older than 3.19 are probably
going to ignore this property.

	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
Lucas Stach Nov. 4, 2014, 12:20 p.m. UTC | #3
Am Dienstag, den 04.11.2014, 13:07 +0100 schrieb Arnd Bergmann:
> On Tuesday 04 November 2014 12:00:52 Liviu Dudau wrote:
> > 
> > While the description is potentially correct, what it fails to explain is that the
> > choice of using the property or generating an unstable (across boots) unique
> > number is actually the choice of the host bridge driver at the moment. I know that
> > my earlier implementations were defaulting to the automatic numbering, but that has
> > been dropped from the final series as Rob Herring was objecting to it.
> > 
> > There is still scope to adopt a wide policy here, but for now it should say something
> > to the tune:
> > 
> >    If present this property assigns a fixed PCI domain number to a host bridge,
> >    otherwise an unstable (across boots) unique number will be assigned.
> >    If you decide to use the property to assign a fixed PCI domain number to a host
> >    bridge you have to ensure that all the host bridge drivers present in the system
> >    follow the same policy. Otherwise, potentially conflicting domain numbers
> >    may be assigned to root busses behind different host bridges.
> 
> But with the latest change to the domain handling, all drivers would implement
> this. I would just mention that Linux kernels older than 3.19 are probably
> going to ignore this property.
> 
Right, this further complicates the semantics of this property, so I
would rather leave this out. Host bridge drivers using the generic PCI
domain code will work as documented and IMHO it's reasonable to enforce
this behavior for all new drivers.

Regards,
Lucas
Kumar Gala Nov. 4, 2014, 5:05 p.m. UTC | #4
On Nov 4, 2014, at 5:47 AM, Lucas Stach <l.stach@pengutronix.de> wrote:

> This property was added by 41e5c0f81d3e
> (of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr())
> without the required binding documentation. As this property
> will be supported by a number of host bridge drivers going forward,
> add it to the common PCI binding doc.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> This is a non-critical fix, but may still qualify for 3.18-rc as
> the property was added in this release cycle.
> ---
> Documentation/devicetree/bindings/pci/pci.txt | 10 ++++++++++
> 1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/pci.txt b/Documentation/devicetree/bindings/pci/pci.txt
> index 41aeed38926d..b754f786ed5e 100644
> --- a/Documentation/devicetree/bindings/pci/pci.txt
> +++ b/Documentation/devicetree/bindings/pci/pci.txt
> @@ -7,3 +7,13 @@ And for the interrupt mapping part:
> 
> Open Firmware Recommended Practice: Interrupt Mapping
> http://www.openfirmware.org/1275/practice/imap/imap0_9d.pdf
> +
> +Additionally to the properties specified in the above standards a host bridge
> +driver implementation may support the following properties:
> +
> +- linux,pci-domain:
> +   If present this property assigns a fixed PCI domain number to a host bridge,
> +   otherwise an unstable (across boots) unique number will be assigned.
> +   It is recommended to either not set this property at all or set it for all
> +   host bridges in the system, otherwise potentially conflicting domain numbers
> +   may be assigned to root buses behind different host bridges.

Probably should add something about domain numbers being unique across all host bridgse.

- k
Bjorn Helgaas Nov. 5, 2014, 11:17 p.m. UTC | #5
On Tue, Nov 04, 2014 at 12:47:40PM +0100, Lucas Stach wrote:
> This property was added by 41e5c0f81d3e
> (of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr())
> without the required binding documentation. As this property
> will be supported by a number of host bridge drivers going forward,
> add it to the common PCI binding doc.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

I merged 41e5c0f81d3e through my tree, and I could merge something like
this if a consensus develops with some acks.  But I'll just let you guys
handle it unless you poke me again.

> ---
> This is a non-critical fix, but may still qualify for 3.18-rc as
> the property was added in this release cycle.
> ---
>  Documentation/devicetree/bindings/pci/pci.txt | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/pci.txt b/Documentation/devicetree/bindings/pci/pci.txt
> index 41aeed38926d..b754f786ed5e 100644
> --- a/Documentation/devicetree/bindings/pci/pci.txt
> +++ b/Documentation/devicetree/bindings/pci/pci.txt
> @@ -7,3 +7,13 @@ And for the interrupt mapping part:
>  
>  Open Firmware Recommended Practice: Interrupt Mapping
>  http://www.openfirmware.org/1275/practice/imap/imap0_9d.pdf
> +
> +Additionally to the properties specified in the above standards a host bridge
> +driver implementation may support the following properties:
> +
> +- linux,pci-domain:
> +   If present this property assigns a fixed PCI domain number to a host bridge,
> +   otherwise an unstable (across boots) unique number will be assigned.
> +   It is recommended to either not set this property at all or set it for all
> +   host bridges in the system, otherwise potentially conflicting domain numbers
> +   may be assigned to root buses behind different host bridges.
> -- 
> 2.1.1
> 
--
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 Nov. 6, 2014, 10:05 a.m. UTC | #6
On Wed, Nov 05, 2014 at 11:17:43PM +0000, Bjorn Helgaas wrote:
> On Tue, Nov 04, 2014 at 12:47:40PM +0100, Lucas Stach wrote:
> > This property was added by 41e5c0f81d3e
> > (of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr())
> > without the required binding documentation. As this property
> > will be supported by a number of host bridge drivers going forward,
> > add it to the common PCI binding doc.
> > 
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> 
> I merged 41e5c0f81d3e through my tree, and I could merge something like
> this if a consensus develops with some acks.  But I'll just let you guys
> handle it unless you poke me again.

While I think the "linux,pci-domain" property *must* be documented, I
would like to get a consensus first on the usage. If we agree that
the property is mandatory to all host bridge drivers that use OF then
we need to patch existing drivers (partially done through Lorenzo's
patches, but other arches are ignoring it). If we say all *new* drivers
need to use it then we also need to come up with a strategy on how to
deal with old vs new school drivers.

My preferred approach is the 3rd way: "linux,pci-domain" becomes part of
the core PCI infrastructure (and we find the common ground with ACPI).
That way the host bridge drivers don't have to do anything, but the DT
creators have to specify a value.

Pinging Rob to try to get a peek on this thoughts.

Best regards,
Liviu

> 
> > ---
> > This is a non-critical fix, but may still qualify for 3.18-rc as
> > the property was added in this release cycle.
> > ---
> >  Documentation/devicetree/bindings/pci/pci.txt | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/pci.txt b/Documentation/devicetree/bindings/pci/pci.txt
> > index 41aeed38926d..b754f786ed5e 100644
> > --- a/Documentation/devicetree/bindings/pci/pci.txt
> > +++ b/Documentation/devicetree/bindings/pci/pci.txt
> > @@ -7,3 +7,13 @@ And for the interrupt mapping part:
> >  
> >  Open Firmware Recommended Practice: Interrupt Mapping
> >  http://www.openfirmware.org/1275/practice/imap/imap0_9d.pdf
> > +
> > +Additionally to the properties specified in the above standards a host bridge
> > +driver implementation may support the following properties:
> > +
> > +- linux,pci-domain:
> > +   If present this property assigns a fixed PCI domain number to a host bridge,
> > +   otherwise an unstable (across boots) unique number will be assigned.
> > +   It is recommended to either not set this property at all or set it for all
> > +   host bridges in the system, otherwise potentially conflicting domain numbers
> > +   may be assigned to root buses behind different host bridges.
> > -- 
> > 2.1.1
> > 
>
Arnd Bergmann Nov. 6, 2014, 11:42 a.m. UTC | #7
On Thursday 06 November 2014 10:05:18 Liviu Dudau wrote:
> On Wed, Nov 05, 2014 at 11:17:43PM +0000, Bjorn Helgaas wrote:
> > On Tue, Nov 04, 2014 at 12:47:40PM +0100, Lucas Stach wrote:
> > > This property was added by 41e5c0f81d3e
> > > (of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr())
> > > without the required binding documentation. As this property
> > > will be supported by a number of host bridge drivers going forward,
> > > add it to the common PCI binding doc.
> > > 
> > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > 
> > I merged 41e5c0f81d3e through my tree, and I could merge something like
> > this if a consensus develops with some acks.  But I'll just let you guys
> > handle it unless you poke me again.
> 
> While I think the "linux,pci-domain" property *must* be documented, I
> would like to get a consensus first on the usage. If we agree that
> the property is mandatory to all host bridge drivers that use OF then
> we need to patch existing drivers (partially done through Lorenzo's
> patches, but other arches are ignoring it). If we say all *new* drivers
> need to use it then we also need to come up with a strategy on how to
> deal with old vs new school drivers.
> 
> My preferred approach is the 3rd way: "linux,pci-domain" becomes part of
> the core PCI infrastructure (and we find the common ground with ACPI).
> That way the host bridge drivers don't have to do anything, but the DT
> creators have to specify a value.
> 
> Pinging Rob to try to get a peek on this thoughts.

Parsing "linux,pci-domain" from the PCI core code seems like the best
solution to me, but we still have to support dtbs that don't contain
it. Lorenzo's patch gets this right I think.

ACPI is easy here, because it already requires the domain to be
explicit.

	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
Liviu Dudau Nov. 6, 2014, 12:36 p.m. UTC | #8
On Thu, Nov 06, 2014 at 11:42:41AM +0000, Arnd Bergmann wrote:
> On Thursday 06 November 2014 10:05:18 Liviu Dudau wrote:
> > On Wed, Nov 05, 2014 at 11:17:43PM +0000, Bjorn Helgaas wrote:
> > > On Tue, Nov 04, 2014 at 12:47:40PM +0100, Lucas Stach wrote:
> > > > This property was added by 41e5c0f81d3e
> > > > (of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr())
> > > > without the required binding documentation. As this property
> > > > will be supported by a number of host bridge drivers going forward,
> > > > add it to the common PCI binding doc.
> > > > 
> > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > > 
> > > I merged 41e5c0f81d3e through my tree, and I could merge something like
> > > this if a consensus develops with some acks.  But I'll just let you guys
> > > handle it unless you poke me again.
> > 
> > While I think the "linux,pci-domain" property *must* be documented, I
> > would like to get a consensus first on the usage. If we agree that
> > the property is mandatory to all host bridge drivers that use OF then
> > we need to patch existing drivers (partially done through Lorenzo's
> > patches, but other arches are ignoring it). If we say all *new* drivers
> > need to use it then we also need to come up with a strategy on how to
> > deal with old vs new school drivers.
> > 
> > My preferred approach is the 3rd way: "linux,pci-domain" becomes part of
> > the core PCI infrastructure (and we find the common ground with ACPI).
> > That way the host bridge drivers don't have to do anything, but the DT
> > creators have to specify a value.
> > 
> > Pinging Rob to try to get a peek on this thoughts.
> 
> Parsing "linux,pci-domain" from the PCI core code seems like the best
> solution to me, but we still have to support dtbs that don't contain
> it. Lorenzo's patch gets this right I think.

Agree. But we are not covering other architectures that use OF yet.

> 
> ACPI is easy here, because it already requires the domain to be
> explicit.

I was thinking more along the line of code sharing.

Best regards,
Liviu

> 
> 	Arnd
> 
>
Arnd Bergmann Nov. 6, 2014, 1:29 p.m. UTC | #9
On Thursday 06 November 2014 12:36:42 Liviu Dudau wrote:
> > ACPI is easy here, because it already requires the domain to be
> > explicit.
> 
> I was thinking more along the line of code sharing.
> 

I don't think it's worth it here. In general, it's a good idea,
but the way that ACPI represents PCI is very different. In particular,
it doesn't support multiple kinds of host controllers, only ECAM
config space registers with no special setup code for other hardware
registers. The domain is part of the addressing mechanism for finding
the right ECAM register.

	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
Rob Herring Nov. 6, 2014, 2:57 p.m. UTC | #10
On Thu, Nov 6, 2014 at 4:05 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> On Wed, Nov 05, 2014 at 11:17:43PM +0000, Bjorn Helgaas wrote:
>> On Tue, Nov 04, 2014 at 12:47:40PM +0100, Lucas Stach wrote:
>> > This property was added by 41e5c0f81d3e
>> > (of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr())
>> > without the required binding documentation. As this property
>> > will be supported by a number of host bridge drivers going forward,
>> > add it to the common PCI binding doc.
>> >
>> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
>>
>> I merged 41e5c0f81d3e through my tree, and I could merge something like
>> this if a consensus develops with some acks.  But I'll just let you guys
>> handle it unless you poke me again.
>
> While I think the "linux,pci-domain" property *must* be documented, I
> would like to get a consensus first on the usage. If we agree that
> the property is mandatory to all host bridge drivers that use OF then
> we need to patch existing drivers (partially done through Lorenzo's
> patches, but other arches are ignoring it). If we say all *new* drivers
> need to use it then we also need to come up with a strategy on how to
> deal with old vs new school drivers.
>
> My preferred approach is the 3rd way: "linux,pci-domain" becomes part of
> the core PCI infrastructure (and we find the common ground with ACPI).
> That way the host bridge drivers don't have to do anything, but the DT
> creators have to specify a value.

I'm okay with it being in the core. It was the mixture of using the
property and automatic numbering that I had issues with. Any mixture
whether in DT or in drivers should be an error. Also, I think having a
mixture of root bus host drivers would be rare, so I'm not too
concerned about some drivers supporting the property and others not.
In any case, these issues are all with the kernel and not really the
concern for the binding. For the binding, simply all hosts set the
domain or none of them do.

Rob
--
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
Lucas Stach Nov. 6, 2014, 3:08 p.m. UTC | #11
Am Dienstag, den 04.11.2014, 13:07 +0100 schrieb Arnd Bergmann:
> On Tuesday 04 November 2014 12:00:52 Liviu Dudau wrote:
> > 
> > While the description is potentially correct, what it fails to explain is that the
> > choice of using the property or generating an unstable (across boots) unique
> > number is actually the choice of the host bridge driver at the moment. I know that
> > my earlier implementations were defaulting to the automatic numbering, but that has
> > been dropped from the final series as Rob Herring was objecting to it.
> > 
> > There is still scope to adopt a wide policy here, but for now it should say something
> > to the tune:
> > 
> >    If present this property assigns a fixed PCI domain number to a host bridge,
> >    otherwise an unstable (across boots) unique number will be assigned.
> >    If you decide to use the property to assign a fixed PCI domain number to a host
> >    bridge you have to ensure that all the host bridge drivers present in the system
> >    follow the same policy. Otherwise, potentially conflicting domain numbers
> >    may be assigned to root busses behind different host bridges.
> 
> But with the latest change to the domain handling, all drivers would implement
> this. I would just mention that Linux kernels older than 3.19 are probably
> going to ignore this property.
> 
Hm, I don't think we should stick those things into the binding docs, as
those should not be Linux specific. IMHO the time when the parsing of a
property gets implemented is a implementation detail that has nothing to
do with the binding. Besides vendors always screw with this timeframes
by doing backports.

Regards,
Lucas
Liviu Dudau Nov. 6, 2014, 3:30 p.m. UTC | #12
On Thu, Nov 06, 2014 at 02:57:35PM +0000, Rob Herring wrote:
> On Thu, Nov 6, 2014 at 4:05 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> > On Wed, Nov 05, 2014 at 11:17:43PM +0000, Bjorn Helgaas wrote:
> >> On Tue, Nov 04, 2014 at 12:47:40PM +0100, Lucas Stach wrote:
> >> > This property was added by 41e5c0f81d3e
> >> > (of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr())
> >> > without the required binding documentation. As this property
> >> > will be supported by a number of host bridge drivers going forward,
> >> > add it to the common PCI binding doc.
> >> >
> >> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> >>
> >> I merged 41e5c0f81d3e through my tree, and I could merge something like
> >> this if a consensus develops with some acks.  But I'll just let you guys
> >> handle it unless you poke me again.
> >
> > While I think the "linux,pci-domain" property *must* be documented, I
> > would like to get a consensus first on the usage. If we agree that
> > the property is mandatory to all host bridge drivers that use OF then
> > we need to patch existing drivers (partially done through Lorenzo's
> > patches, but other arches are ignoring it). If we say all *new* drivers
> > need to use it then we also need to come up with a strategy on how to
> > deal with old vs new school drivers.
> >
> > My preferred approach is the 3rd way: "linux,pci-domain" becomes part of
> > the core PCI infrastructure (and we find the common ground with ACPI).
> > That way the host bridge drivers don't have to do anything, but the DT
> > creators have to specify a value.
> 
> I'm okay with it being in the core. It was the mixture of using the
> property and automatic numbering that I had issues with. Any mixture
> whether in DT or in drivers should be an error. Also, I think having a
> mixture of root bus host drivers would be rare, so I'm not too
> concerned about some drivers supporting the property and others not.
> In any case, these issues are all with the kernel and not really the
> concern for the binding. For the binding, simply all hosts set the
> domain or none of them do.

Repeating what you've said to verify my understanding: you are OK with
the "linux,pci-domain" being handled in the PCI framework and mandatory
to all OF-based host bridges and architectures. Failure to include
the property should be an error and no host bridge driver should default
to the auto-generation of domain numbers.

Is that correct?

Thanks,
Liviu

> 
> Rob
>
Rob Herring Nov. 6, 2014, 7:46 p.m. UTC | #13
On Thu, Nov 6, 2014 at 9:30 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> On Thu, Nov 06, 2014 at 02:57:35PM +0000, Rob Herring wrote:
>> On Thu, Nov 6, 2014 at 4:05 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
>> > On Wed, Nov 05, 2014 at 11:17:43PM +0000, Bjorn Helgaas wrote:
>> >> On Tue, Nov 04, 2014 at 12:47:40PM +0100, Lucas Stach wrote:
>> >> > This property was added by 41e5c0f81d3e
>> >> > (of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr())
>> >> > without the required binding documentation. As this property
>> >> > will be supported by a number of host bridge drivers going forward,
>> >> > add it to the common PCI binding doc.
>> >> >
>> >> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
>> >>
>> >> I merged 41e5c0f81d3e through my tree, and I could merge something like
>> >> this if a consensus develops with some acks.  But I'll just let you guys
>> >> handle it unless you poke me again.
>> >
>> > While I think the "linux,pci-domain" property *must* be documented, I
>> > would like to get a consensus first on the usage. If we agree that
>> > the property is mandatory to all host bridge drivers that use OF then
>> > we need to patch existing drivers (partially done through Lorenzo's
>> > patches, but other arches are ignoring it). If we say all *new* drivers
>> > need to use it then we also need to come up with a strategy on how to
>> > deal with old vs new school drivers.
>> >
>> > My preferred approach is the 3rd way: "linux,pci-domain" becomes part of
>> > the core PCI infrastructure (and we find the common ground with ACPI).
>> > That way the host bridge drivers don't have to do anything, but the DT
>> > creators have to specify a value.
>>
>> I'm okay with it being in the core. It was the mixture of using the
>> property and automatic numbering that I had issues with. Any mixture
>> whether in DT or in drivers should be an error. Also, I think having a
>> mixture of root bus host drivers would be rare, so I'm not too
>> concerned about some drivers supporting the property and others not.
>> In any case, these issues are all with the kernel and not really the
>> concern for the binding. For the binding, simply all hosts set the
>> domain or none of them do.
>
> Repeating what you've said to verify my understanding: you are OK with
> the "linux,pci-domain" being handled in the PCI framework and mandatory
> to all OF-based host bridges and architectures. Failure to include
> the property should be an error and no host bridge driver should default
> to the auto-generation of domain numbers.
>
> Is that correct?

Not exactly. It is only mandatory when you have multiple root buses.
But we can't say an existing dtb is in error, so it has to remain
optional for compatibility. Also, given it is a Linux property, you
can't really say it is mandatory for all PCI bindings from a DT
perspective.

While we could have issues in theory if this is handled in the
drivers, I don't think we will in practice as having root buses with
different drivers is unlikely.

Rob
--
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 Nov. 6, 2014, 7:55 p.m. UTC | #14
On Thursday 06 November 2014 16:08:57 Lucas Stach wrote:
> Am Dienstag, den 04.11.2014, 13:07 +0100 schrieb Arnd Bergmann:
> > On Tuesday 04 November 2014 12:00:52 Liviu Dudau wrote:
> > > 
> > > While the description is potentially correct, what it fails to explain is that the
> > > choice of using the property or generating an unstable (across boots) unique
> > > number is actually the choice of the host bridge driver at the moment. I know that
> > > my earlier implementations were defaulting to the automatic numbering, but that has
> > > been dropped from the final series as Rob Herring was objecting to it.
> > > 
> > > There is still scope to adopt a wide policy here, but for now it should say something
> > > to the tune:
> > > 
> > >    If present this property assigns a fixed PCI domain number to a host bridge,
> > >    otherwise an unstable (across boots) unique number will be assigned.
> > >    If you decide to use the property to assign a fixed PCI domain number to a host
> > >    bridge you have to ensure that all the host bridge drivers present in the system
> > >    follow the same policy. Otherwise, potentially conflicting domain numbers
> > >    may be assigned to root busses behind different host bridges.
> > 
> > But with the latest change to the domain handling, all drivers would implement
> > this. I would just mention that Linux kernels older than 3.19 are probably
> > going to ignore this property.
> > 
> Hm, I don't think we should stick those things into the binding docs, as
> those should not be Linux specific. IMHO the time when the parsing of a
> property gets implemented is a implementation detail that has nothing to
> do with the binding. Besides vendors always screw with this timeframes
> by doing backports.

Well, unlike most properties, this one is explicitly Linux-specific,
at least that is what the name implies.

	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
Liviu Dudau Nov. 7, 2014, 10:17 a.m. UTC | #15
On Thu, Nov 06, 2014 at 07:46:33PM +0000, Rob Herring wrote:
> On Thu, Nov 6, 2014 at 9:30 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> > On Thu, Nov 06, 2014 at 02:57:35PM +0000, Rob Herring wrote:
> >> On Thu, Nov 6, 2014 at 4:05 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> >> > On Wed, Nov 05, 2014 at 11:17:43PM +0000, Bjorn Helgaas wrote:
> >> >> On Tue, Nov 04, 2014 at 12:47:40PM +0100, Lucas Stach wrote:
> >> >> > This property was added by 41e5c0f81d3e
> >> >> > (of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr())
> >> >> > without the required binding documentation. As this property
> >> >> > will be supported by a number of host bridge drivers going forward,
> >> >> > add it to the common PCI binding doc.
> >> >> >
> >> >> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> >> >>
> >> >> I merged 41e5c0f81d3e through my tree, and I could merge something like
> >> >> this if a consensus develops with some acks.  But I'll just let you guys
> >> >> handle it unless you poke me again.
> >> >
> >> > While I think the "linux,pci-domain" property *must* be documented, I
> >> > would like to get a consensus first on the usage. If we agree that
> >> > the property is mandatory to all host bridge drivers that use OF then
> >> > we need to patch existing drivers (partially done through Lorenzo's
> >> > patches, but other arches are ignoring it). If we say all *new* drivers
> >> > need to use it then we also need to come up with a strategy on how to
> >> > deal with old vs new school drivers.
> >> >
> >> > My preferred approach is the 3rd way: "linux,pci-domain" becomes part of
> >> > the core PCI infrastructure (and we find the common ground with ACPI).
> >> > That way the host bridge drivers don't have to do anything, but the DT
> >> > creators have to specify a value.
> >>
> >> I'm okay with it being in the core. It was the mixture of using the
> >> property and automatic numbering that I had issues with. Any mixture
> >> whether in DT or in drivers should be an error. Also, I think having a
> >> mixture of root bus host drivers would be rare, so I'm not too
> >> concerned about some drivers supporting the property and others not.
> >> In any case, these issues are all with the kernel and not really the
> >> concern for the binding. For the binding, simply all hosts set the
> >> domain or none of them do.
> >
> > Repeating what you've said to verify my understanding: you are OK with
> > the "linux,pci-domain" being handled in the PCI framework and mandatory
> > to all OF-based host bridges and architectures. Failure to include
> > the property should be an error and no host bridge driver should default
> > to the auto-generation of domain numbers.
> >
> > Is that correct?
> 
> Not exactly. It is only mandatory when you have multiple root buses.
> But we can't say an existing dtb is in error, so it has to remain
> optional for compatibility. Also, given it is a Linux property, you
> can't really say it is mandatory for all PCI bindings from a DT
> perspective.
> 
> While we could have issues in theory if this is handled in the
> drivers, I don't think we will in practice as having root buses with
> different drivers is unlikely.

That's correct. However, I would like to bring to you attention that as
long as the property is treated as optional in certain cases we will need,
in the framework code, to mix the assignment of a domain number coming from
parsing of the property with the auto-numbering scheme in order to support
old DT files. And that was one of your major concerns when reviewing the
series. Any suggestions or clarifications?

Best regards,
Liviu

> 
> Rob
>
Rob Herring Nov. 7, 2014, 2 p.m. UTC | #16
On Fri, Nov 7, 2014 at 4:17 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> On Thu, Nov 06, 2014 at 07:46:33PM +0000, Rob Herring wrote:
>> On Thu, Nov 6, 2014 at 9:30 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
>> > On Thu, Nov 06, 2014 at 02:57:35PM +0000, Rob Herring wrote:
>> >> On Thu, Nov 6, 2014 at 4:05 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
>> >> > On Wed, Nov 05, 2014 at 11:17:43PM +0000, Bjorn Helgaas wrote:
>> >> >> On Tue, Nov 04, 2014 at 12:47:40PM +0100, Lucas Stach wrote:
>> >> >> > This property was added by 41e5c0f81d3e
>> >> >> > (of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr())
>> >> >> > without the required binding documentation. As this property
>> >> >> > will be supported by a number of host bridge drivers going forward,
>> >> >> > add it to the common PCI binding doc.
>> >> >> >
>> >> >> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
>> >> >>
>> >> >> I merged 41e5c0f81d3e through my tree, and I could merge something like
>> >> >> this if a consensus develops with some acks.  But I'll just let you guys
>> >> >> handle it unless you poke me again.
>> >> >
>> >> > While I think the "linux,pci-domain" property *must* be documented, I
>> >> > would like to get a consensus first on the usage. If we agree that
>> >> > the property is mandatory to all host bridge drivers that use OF then
>> >> > we need to patch existing drivers (partially done through Lorenzo's
>> >> > patches, but other arches are ignoring it). If we say all *new* drivers
>> >> > need to use it then we also need to come up with a strategy on how to
>> >> > deal with old vs new school drivers.
>> >> >
>> >> > My preferred approach is the 3rd way: "linux,pci-domain" becomes part of
>> >> > the core PCI infrastructure (and we find the common ground with ACPI).
>> >> > That way the host bridge drivers don't have to do anything, but the DT
>> >> > creators have to specify a value.
>> >>
>> >> I'm okay with it being in the core. It was the mixture of using the
>> >> property and automatic numbering that I had issues with. Any mixture
>> >> whether in DT or in drivers should be an error. Also, I think having a
>> >> mixture of root bus host drivers would be rare, so I'm not too
>> >> concerned about some drivers supporting the property and others not.
>> >> In any case, these issues are all with the kernel and not really the
>> >> concern for the binding. For the binding, simply all hosts set the
>> >> domain or none of them do.
>> >
>> > Repeating what you've said to verify my understanding: you are OK with
>> > the "linux,pci-domain" being handled in the PCI framework and mandatory
>> > to all OF-based host bridges and architectures. Failure to include
>> > the property should be an error and no host bridge driver should default
>> > to the auto-generation of domain numbers.
>> >
>> > Is that correct?
>>
>> Not exactly. It is only mandatory when you have multiple root buses.
>> But we can't say an existing dtb is in error, so it has to remain
>> optional for compatibility. Also, given it is a Linux property, you
>> can't really say it is mandatory for all PCI bindings from a DT
>> perspective.
>>
>> While we could have issues in theory if this is handled in the
>> drivers, I don't think we will in practice as having root buses with
>> different drivers is unlikely.
>
> That's correct. However, I would like to bring to you attention that as
> long as the property is treated as optional in certain cases we will need,
> in the framework code, to mix the assignment of a domain number coming from
> parsing of the property with the auto-numbering scheme in order to support
> old DT files. And that was one of your major concerns when reviewing the
> series. Any suggestions or clarifications?

We have to support both allocation schemes, but not on the same
system. A mixture on a given system is an error. So the cases to
handle are like this:

1 root bus w/ domain -> use domain prop
1 root bus w/o domain -> auto numbering
N root buses w/ domains -> use domain prop
N root buses w/o domains -> auto numbering
N root buses w/ and w/o domains -> error

I also had issues just around the implementation details, but we can
discuss those when there is another version.

Rob
--
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 Nov. 7, 2014, 3:23 p.m. UTC | #17
On Fri, Nov 07, 2014 at 02:00:56PM +0000, Rob Herring wrote:
> On Fri, Nov 7, 2014 at 4:17 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> > On Thu, Nov 06, 2014 at 07:46:33PM +0000, Rob Herring wrote:
> >> On Thu, Nov 6, 2014 at 9:30 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> >> > On Thu, Nov 06, 2014 at 02:57:35PM +0000, Rob Herring wrote:
> >> >> On Thu, Nov 6, 2014 at 4:05 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> >> >> > On Wed, Nov 05, 2014 at 11:17:43PM +0000, Bjorn Helgaas wrote:
> >> >> >> On Tue, Nov 04, 2014 at 12:47:40PM +0100, Lucas Stach wrote:
> >> >> >> > This property was added by 41e5c0f81d3e
> >> >> >> > (of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr())
> >> >> >> > without the required binding documentation. As this property
> >> >> >> > will be supported by a number of host bridge drivers going forward,
> >> >> >> > add it to the common PCI binding doc.
> >> >> >> >
> >> >> >> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> >> >> >>
> >> >> >> I merged 41e5c0f81d3e through my tree, and I could merge something like
> >> >> >> this if a consensus develops with some acks.  But I'll just let you guys
> >> >> >> handle it unless you poke me again.
> >> >> >
> >> >> > While I think the "linux,pci-domain" property *must* be documented, I
> >> >> > would like to get a consensus first on the usage. If we agree that
> >> >> > the property is mandatory to all host bridge drivers that use OF then
> >> >> > we need to patch existing drivers (partially done through Lorenzo's
> >> >> > patches, but other arches are ignoring it). If we say all *new* drivers
> >> >> > need to use it then we also need to come up with a strategy on how to
> >> >> > deal with old vs new school drivers.
> >> >> >
> >> >> > My preferred approach is the 3rd way: "linux,pci-domain" becomes part of
> >> >> > the core PCI infrastructure (and we find the common ground with ACPI).
> >> >> > That way the host bridge drivers don't have to do anything, but the DT
> >> >> > creators have to specify a value.
> >> >>
> >> >> I'm okay with it being in the core. It was the mixture of using the
> >> >> property and automatic numbering that I had issues with. Any mixture
> >> >> whether in DT or in drivers should be an error. Also, I think having a
> >> >> mixture of root bus host drivers would be rare, so I'm not too
> >> >> concerned about some drivers supporting the property and others not.
> >> >> In any case, these issues are all with the kernel and not really the
> >> >> concern for the binding. For the binding, simply all hosts set the
> >> >> domain or none of them do.
> >> >
> >> > Repeating what you've said to verify my understanding: you are OK with
> >> > the "linux,pci-domain" being handled in the PCI framework and mandatory
> >> > to all OF-based host bridges and architectures. Failure to include
> >> > the property should be an error and no host bridge driver should default
> >> > to the auto-generation of domain numbers.
> >> >
> >> > Is that correct?
> >>
> >> Not exactly. It is only mandatory when you have multiple root buses.
> >> But we can't say an existing dtb is in error, so it has to remain
> >> optional for compatibility. Also, given it is a Linux property, you
> >> can't really say it is mandatory for all PCI bindings from a DT
> >> perspective.
> >>
> >> While we could have issues in theory if this is handled in the
> >> drivers, I don't think we will in practice as having root buses with
> >> different drivers is unlikely.
> >
> > That's correct. However, I would like to bring to you attention that as
> > long as the property is treated as optional in certain cases we will need,
> > in the framework code, to mix the assignment of a domain number coming from
> > parsing of the property with the auto-numbering scheme in order to support
> > old DT files. And that was one of your major concerns when reviewing the
> > series. Any suggestions or clarifications?
> 
> We have to support both allocation schemes, but not on the same
> system. A mixture on a given system is an error. So the cases to
> handle are like this:
> 
> 1 root bus w/ domain -> use domain prop
> 1 root bus w/o domain -> auto numbering
> N root buses w/ domains -> use domain prop
> N root buses w/o domains -> auto numbering
> N root buses w/ and w/o domains -> error

Agree.

> 
> I also had issues just around the implementation details, but we can
> discuss those when there is another version.

Well, there is a version already in arch/arm64/kernel/pci.c:
pci_bus_assign_domain_nr() and I believe it works on all the cases
listed above. Question is if you agree on making this the default
implementation for OF-based architectures. Lorenzo has already
copied it into arch/arm.

Best regards,
Liviu

> 
> Rob
>
Rob Herring Nov. 7, 2014, 3:37 p.m. UTC | #18
On Fri, Nov 7, 2014 at 9:23 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> On Fri, Nov 07, 2014 at 02:00:56PM +0000, Rob Herring wrote:
>> On Fri, Nov 7, 2014 at 4:17 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
>> > On Thu, Nov 06, 2014 at 07:46:33PM +0000, Rob Herring wrote:
>> >> On Thu, Nov 6, 2014 at 9:30 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
>> >> > On Thu, Nov 06, 2014 at 02:57:35PM +0000, Rob Herring wrote:
>> >> >> On Thu, Nov 6, 2014 at 4:05 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
>> >> >> > On Wed, Nov 05, 2014 at 11:17:43PM +0000, Bjorn Helgaas wrote:
>> >> >> >> On Tue, Nov 04, 2014 at 12:47:40PM +0100, Lucas Stach wrote:
>> >> >> >> > This property was added by 41e5c0f81d3e
>> >> >> >> > (of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr())
>> >> >> >> > without the required binding documentation. As this property
>> >> >> >> > will be supported by a number of host bridge drivers going forward,
>> >> >> >> > add it to the common PCI binding doc.
>> >> >> >> >
>> >> >> >> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
>> >> >> >>
>> >> >> >> I merged 41e5c0f81d3e through my tree, and I could merge something like
>> >> >> >> this if a consensus develops with some acks.  But I'll just let you guys
>> >> >> >> handle it unless you poke me again.
>> >> >> >
>> >> >> > While I think the "linux,pci-domain" property *must* be documented, I
>> >> >> > would like to get a consensus first on the usage. If we agree that
>> >> >> > the property is mandatory to all host bridge drivers that use OF then
>> >> >> > we need to patch existing drivers (partially done through Lorenzo's
>> >> >> > patches, but other arches are ignoring it). If we say all *new* drivers
>> >> >> > need to use it then we also need to come up with a strategy on how to
>> >> >> > deal with old vs new school drivers.
>> >> >> >
>> >> >> > My preferred approach is the 3rd way: "linux,pci-domain" becomes part of
>> >> >> > the core PCI infrastructure (and we find the common ground with ACPI).
>> >> >> > That way the host bridge drivers don't have to do anything, but the DT
>> >> >> > creators have to specify a value.
>> >> >>
>> >> >> I'm okay with it being in the core. It was the mixture of using the
>> >> >> property and automatic numbering that I had issues with. Any mixture
>> >> >> whether in DT or in drivers should be an error. Also, I think having a
>> >> >> mixture of root bus host drivers would be rare, so I'm not too
>> >> >> concerned about some drivers supporting the property and others not.
>> >> >> In any case, these issues are all with the kernel and not really the
>> >> >> concern for the binding. For the binding, simply all hosts set the
>> >> >> domain or none of them do.
>> >> >
>> >> > Repeating what you've said to verify my understanding: you are OK with
>> >> > the "linux,pci-domain" being handled in the PCI framework and mandatory
>> >> > to all OF-based host bridges and architectures. Failure to include
>> >> > the property should be an error and no host bridge driver should default
>> >> > to the auto-generation of domain numbers.
>> >> >
>> >> > Is that correct?
>> >>
>> >> Not exactly. It is only mandatory when you have multiple root buses.
>> >> But we can't say an existing dtb is in error, so it has to remain
>> >> optional for compatibility. Also, given it is a Linux property, you
>> >> can't really say it is mandatory for all PCI bindings from a DT
>> >> perspective.
>> >>
>> >> While we could have issues in theory if this is handled in the
>> >> drivers, I don't think we will in practice as having root buses with
>> >> different drivers is unlikely.
>> >
>> > That's correct. However, I would like to bring to you attention that as
>> > long as the property is treated as optional in certain cases we will need,
>> > in the framework code, to mix the assignment of a domain number coming from
>> > parsing of the property with the auto-numbering scheme in order to support
>> > old DT files. And that was one of your major concerns when reviewing the
>> > series. Any suggestions or clarifications?
>>
>> We have to support both allocation schemes, but not on the same
>> system. A mixture on a given system is an error. So the cases to
>> handle are like this:
>>
>> 1 root bus w/ domain -> use domain prop
>> 1 root bus w/o domain -> auto numbering
>> N root buses w/ domains -> use domain prop
>> N root buses w/o domains -> auto numbering
>> N root buses w/ and w/o domains -> error
>
> Agree.
>
>>
>> I also had issues just around the implementation details, but we can
>> discuss those when there is another version.
>
> Well, there is a version already in arch/arm64/kernel/pci.c:
> pci_bus_assign_domain_nr() and I believe it works on all the cases
> listed above. Question is if you agree on making this the default
> implementation for OF-based architectures. Lorenzo has already
> copied it into arch/arm.

I agree it is correct implementation for correct DTs, but it doesn't
handle some error cases. I'll comment on Lorenzo's ARM version about
specifics.

Rob
--
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/Documentation/devicetree/bindings/pci/pci.txt b/Documentation/devicetree/bindings/pci/pci.txt
index 41aeed38926d..b754f786ed5e 100644
--- a/Documentation/devicetree/bindings/pci/pci.txt
+++ b/Documentation/devicetree/bindings/pci/pci.txt
@@ -7,3 +7,13 @@  And for the interrupt mapping part:
 
 Open Firmware Recommended Practice: Interrupt Mapping
 http://www.openfirmware.org/1275/practice/imap/imap0_9d.pdf
+
+Additionally to the properties specified in the above standards a host bridge
+driver implementation may support the following properties:
+
+- linux,pci-domain:
+   If present this property assigns a fixed PCI domain number to a host bridge,
+   otherwise an unstable (across boots) unique number will be assigned.
+   It is recommended to either not set this property at all or set it for all
+   host bridges in the system, otherwise potentially conflicting domain numbers
+   may be assigned to root buses behind different host bridges.