[PATCH] bindings: PCI: artpec: correct pci binding example

[PATCH] bindings: PCI: artpec: correct pci binding example

[Niklas Cassel]

From: Niklas Cassel <niklas.cassel@axis.com>

 - Increase config size. When using a PCIe switch,
   the previous config size only had room for one device.
 - Add bus range. Inherited optional property.
 - Map downstream I/O to PCI address 0. We can map it to any
   address, but let's be consistent with other drivers.

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
 Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt b/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
index 330a45b..5ecaea1 100644
--- a/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
@@ -24,16 +24,17 @@ Example:
 		compatible = "axis,artpec6-pcie", "snps,dw-pcie";
 		reg = <0xf8050000 0x2000
 		       0xf8040000 0x1000
-		       0xc0000000 0x1000>;
+		       0xc0000000 0x2000>;
 		reg-names = "dbi", "phy", "config";
 		#address-cells = <3>;
 		#size-cells = <2>;
 		device_type = "pci";
 			  /* downstream I/O */
-		ranges = <0x81000000 0 0x00010000 0xc0010000 0 0x00010000
+		ranges = <0x81000000 0 0 0xc0002000 0 0x00010000
 			  /* non-prefetchable memory */
-			  0x82000000 0 0xc0020000 0xc0020000 0 0x1ffe0000>;
+			  0x82000000 0 0xc0012000 0xc0012000 0 0x1ffee000>;
 		num-lanes = <2>;
+		bus-range = <0x00 0xff>;
 		interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
 		interrupt-names = "msi";
 		#interrupt-cells = <1>;
-- 
2.1.4

Re: [PATCH] bindings: PCI: artpec: correct pci binding example

[Arnd Bergmann]

On Friday, July 8, 2016 1:28:10 AM CEST Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@axis.com>
> 
>  - Increase config size. When using a PCIe switch,
>    the previous config size only had room for one device.
>  - Add bus range. Inherited optional property.
>  - Map downstream I/O to PCI address 0. We can map it to any
>    address, but let's be consistent with other drivers.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> ---
>  Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt b/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
> index 330a45b..5ecaea1 100644
> --- a/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
> @@ -24,16 +24,17 @@ Example:
>  		compatible = "axis,artpec6-pcie", "snps,dw-pcie";
>  		reg = <0xf8050000 0x2000
>  		       0xf8040000 0x1000
> -		       0xc0000000 0x1000>;
> +		       0xc0000000 0x2000>;

If this is your config space size

>  		num-lanes = <2>;
> +		bus-range = <0x00 0xff>;

then the bus range looks too large. These two are typically connected.
I couldn't immediately see which config space access function is
used, but if you have 0x1000 bytes per bus, then the bus range matching
a 0x2000 byte config space would be either <0x00 0x01> or <0x00 0x02>
depending whether the root bus is part of that range.

	Arnd

Re: [PATCH] bindings: PCI: artpec: correct pci binding example

[Niklas Cassel]

On 07/08/2016 11:39 AM, Arnd Bergmann wrote:
> On Friday, July 8, 2016 1:28:10 AM CEST Niklas Cassel wrote:
>> From: Niklas Cassel <niklas.cassel@axis.com>
>>
>>  - Increase config size. When using a PCIe switch,
>>    the previous config size only had room for one device.
>>  - Add bus range. Inherited optional property.
>>  - Map downstream I/O to PCI address 0. We can map it to any
>>    address, but let's be consistent with other drivers.
>>
>> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
>> ---
>>  Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt b/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
>> index 330a45b..5ecaea1 100644
>> --- a/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
>> +++ b/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
>> @@ -24,16 +24,17 @@ Example:
>>  		compatible = "axis,artpec6-pcie", "snps,dw-pcie";
>>  		reg = <0xf8050000 0x2000
>>  		       0xf8040000 0x1000
>> -		       0xc0000000 0x1000>;
>> +		       0xc0000000 0x2000>;
> If this is your config space size
>
>>  		num-lanes = <2>;
>> +		bus-range = <0x00 0xff>;
> then the bus range looks too large. These two are typically connected.
> I couldn't immediately see which config space access function is
> used, but if you have 0x1000 bytes per bus, then the bus range matching
> a 0x2000 byte config space would be either <0x00 0x01> or <0x00 0x02>
> depending whether the root bus is part of that range.

I see your point, a config space size of 0x2000 is only enough to
hold two configuration space headers.

However, all other PCIe controllers based on Synopsys DesignWare IP
with a config space size of 0x2000, uses a bus-range of 0x0-0xff.
(Except hisilicon which for some reason uses 0x0-0xf).
Isn't it better to be consistent with the other DesignWare based controllers?

Also, isn't is possible to have a device behind bus 1, then nothing behind
bus 2, and then a device behind bus 3?
Not sure if this could actually happen though, since Linux appears to assign
the bus numbers, unless already defined by BIOS.

Re: [PATCH] bindings: PCI: artpec: correct pci binding example

[Arnd Bergmann]

On Thursday, July 14, 2016 1:54:20 AM CEST Niklas Cassel wrote:
> On 07/08/2016 11:39 AM, Arnd Bergmann wrote:
> > On Friday, July 8, 2016 1:28:10 AM CEST Niklas Cassel wrote:
> >> From: Niklas Cassel <niklas.cassel@axis.com>
> >>
> >>  - Increase config size. When using a PCIe switch,
> >>    the previous config size only had room for one device.
> >>  - Add bus range. Inherited optional property.
> >>  - Map downstream I/O to PCI address 0. We can map it to any
> >>    address, but let's be consistent with other drivers.
> >>
> >> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> >> ---
> >>  Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt | 7 ++++---
> >>  1 file changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt b/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
> >> index 330a45b..5ecaea1 100644
> >> --- a/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
> >> +++ b/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
> >> @@ -24,16 +24,17 @@ Example:
> >>              compatible = "axis,artpec6-pcie", "snps,dw-pcie";
> >>              reg = <0xf8050000 0x2000
> >>                     0xf8040000 0x1000
> >> -                   0xc0000000 0x1000>;
> >> +                   0xc0000000 0x2000>;
> > If this is your config space size
> >
> >>              num-lanes = <2>;
> >> +            bus-range = <0x00 0xff>;
> > then the bus range looks too large. These two are typically connected.
> > I couldn't immediately see which config space access function is
> > used, but if you have 0x1000 bytes per bus, then the bus range matching
> > a 0x2000 byte config space would be either <0x00 0x01> or <0x00 0x02>
> > depending whether the root bus is part of that range.
> 
> I see your point, a config space size of 0x2000 is only enough to
> hold two configuration space headers.
> 
> However, all other PCIe controllers based on Synopsys DesignWare IP
> with a config space size of 0x2000, uses a bus-range of 0x0-0xff.
> (Except hisilicon which for some reason uses 0x0-0xf).
> Isn't it better to be consistent with the other DesignWare based controllers?

No, if we found a bug, we should fix it for all of them.

> Also, isn't is possible to have a device behind bus 1, then nothing behind
> bus 2, and then a device behind bus 3?
> Not sure if this could actually happen though, since Linux appears to assign
> the bus numbers, unless already defined by BIOS.

I think this might happen with hotplugged devices.

	Arnd