| Message ID | 1467934090-12834-1-git-send-email-niklass@axis.com |
|---|---|
| State | Changes Requested |
| Headers | show |
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 -- 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
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. -- 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
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 -- 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 --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>;