* [PATCH v3 1/6] dt-bindings: PCI: dwc: rockchip: Add mandatory atu reg
2023-10-27 14:54 [PATCH v3 0/6] rockchip DWC PCIe improvements Niklas Cassel
@ 2023-10-27 14:54 ` Niklas Cassel
2023-10-27 15:58 ` Rob Herring
2023-10-27 14:54 ` [PATCH v3 2/6] dt-bindings: PCI: dwc: rockchip: Add optional dma interrupts Niklas Cassel
` (4 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Niklas Cassel @ 2023-10-27 14:54 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Shawn Lin, Simon Xue
Cc: Damien Le Moal, Sebastian Reichel, Serge Semin, Niklas Cassel,
linux-pci, devicetree, linux-arm-kernel, linux-rockchip
From: Niklas Cassel <niklas.cassel@wdc.com>
Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
using:
allOf:
- $ref: /schemas/pci/snps,dw-pcie.yaml#
and snps,dw-pcie.yaml does have the atu reg defined, in order to be
able to use this reg, while still making sure 'make CHECK_DTBS=y'
pass, we need to add this reg to rockchip-dw-pcie.yaml.
All compatible strings (rockchip,rk3568-pcie and rockchip,rk3588-pcie)
should have this reg.
The regs in the example are updated to actually match pcie3x2 on rk3568.
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
.../devicetree/bindings/pci/rockchip-dw-pcie.yaml | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
index 1ae8dcfa072c..6ca87ff4ae20 100644
--- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
@@ -33,12 +33,14 @@ properties:
- description: Data Bus Interface (DBI) registers
- description: Rockchip designed configuration registers
- description: Config registers
+ - description: iATU/eDMA registers
reg-names:
items:
- const: dbi
- const: apb
- const: config
+ - const: atu
clocks:
minItems: 5
@@ -171,10 +173,11 @@ examples:
pcie3x2: pcie@fe280000 {
compatible = "rockchip,rk3568-pcie";
- reg = <0x3 0xc0800000 0x0 0x390000>,
- <0x0 0xfe280000 0x0 0x10000>,
- <0x3 0x80000000 0x0 0x100000>;
- reg-names = "dbi", "apb", "config";
+ reg = <0x3 0xc0800000 0x0 0x00300000>,
+ <0x0 0xfe280000 0x0 0x00010000>,
+ <0x0 0xf0000000 0x0 0x00100000>,
+ <0x3 0xc0b00000 0x0 0x00100000>;
+ reg-names = "dbi", "apb", "config", "atu";
bus-range = <0x20 0x2f>;
clocks = <&cru 143>, <&cru 144>,
<&cru 145>, <&cru 146>,
--
2.41.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/6] dt-bindings: PCI: dwc: rockchip: Add mandatory atu reg
2023-10-27 14:54 ` [PATCH v3 1/6] dt-bindings: PCI: dwc: rockchip: Add mandatory atu reg Niklas Cassel
@ 2023-10-27 15:58 ` Rob Herring
2023-10-27 16:14 ` Niklas Cassel
0 siblings, 1 reply; 18+ messages in thread
From: Rob Herring @ 2023-10-27 15:58 UTC (permalink / raw)
To: Niklas Cassel
Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, Shawn Lin,
Simon Xue, Damien Le Moal, Sebastian Reichel, Serge Semin,
Niklas Cassel, linux-pci, devicetree, linux-arm-kernel,
linux-rockchip
On Fri, Oct 27, 2023 at 9:56 AM Niklas Cassel <nks@flawful.org> wrote:
>
> From: Niklas Cassel <niklas.cassel@wdc.com>
>
> Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> using:
>
> allOf:
> - $ref: /schemas/pci/snps,dw-pcie.yaml#
>
> and snps,dw-pcie.yaml does have the atu reg defined, in order to be
> able to use this reg, while still making sure 'make CHECK_DTBS=y'
> pass, we need to add this reg to rockchip-dw-pcie.yaml.
>
> All compatible strings (rockchip,rk3568-pcie and rockchip,rk3588-pcie)
> should have this reg.
>
> The regs in the example are updated to actually match pcie3x2 on rk3568.
Breaking compatibility on these platforms is okay because ...?
>
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
> .../devicetree/bindings/pci/rockchip-dw-pcie.yaml | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/6] dt-bindings: PCI: dwc: rockchip: Add mandatory atu reg
2023-10-27 15:58 ` Rob Herring
@ 2023-10-27 16:14 ` Niklas Cassel
2023-10-27 17:03 ` Rob Herring
2023-10-27 17:07 ` Heiko Stübner
0 siblings, 2 replies; 18+ messages in thread
From: Niklas Cassel @ 2023-10-27 16:14 UTC (permalink / raw)
To: Rob Herring
Cc: Niklas Cassel, Lorenzo Pieralisi, Krzysztof Wilczyński,
Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Shawn Lin, Simon Xue, Damien Le Moal, Sebastian Reichel,
Serge Semin, linux-pci@vger.kernel.org,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org
Hello Rob,
On Fri, Oct 27, 2023 at 10:58:28AM -0500, Rob Herring wrote:
> On Fri, Oct 27, 2023 at 9:56 AM Niklas Cassel <nks@flawful.org> wrote:
> >
> > From: Niklas Cassel <niklas.cassel@wdc.com>
> >
> > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > using:
> >
> > allOf:
> > - $ref: /schemas/pci/snps,dw-pcie.yaml#
> >
> > and snps,dw-pcie.yaml does have the atu reg defined, in order to be
> > able to use this reg, while still making sure 'make CHECK_DTBS=y'
> > pass, we need to add this reg to rockchip-dw-pcie.yaml.
> >
> > All compatible strings (rockchip,rk3568-pcie and rockchip,rk3588-pcie)
> > should have this reg.
> >
> > The regs in the example are updated to actually match pcie3x2 on rk3568.
>
> Breaking compatibility on these platforms is okay because ...?
I don't follow, could you please elaborate?
Kind regards,
Niklas
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/6] dt-bindings: PCI: dwc: rockchip: Add mandatory atu reg
2023-10-27 16:14 ` Niklas Cassel
@ 2023-10-27 17:03 ` Rob Herring
2023-10-28 12:11 ` Niklas Cassel
2023-10-27 17:07 ` Heiko Stübner
1 sibling, 1 reply; 18+ messages in thread
From: Rob Herring @ 2023-10-27 17:03 UTC (permalink / raw)
To: Niklas Cassel
Cc: Niklas Cassel, Lorenzo Pieralisi, Krzysztof Wilczyński,
Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Shawn Lin, Simon Xue, Damien Le Moal, Sebastian Reichel,
Serge Semin, linux-pci@vger.kernel.org,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org
On Fri, Oct 27, 2023 at 11:15 AM Niklas Cassel <Niklas.Cassel@wdc.com> wrote:
>
> Hello Rob,
>
> On Fri, Oct 27, 2023 at 10:58:28AM -0500, Rob Herring wrote:
> > On Fri, Oct 27, 2023 at 9:56 AM Niklas Cassel <nks@flawful.org> wrote:
> > >
> > > From: Niklas Cassel <niklas.cassel@wdc.com>
> > >
> > > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > > using:
> > >
> > > allOf:
> > > - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > >
> > > and snps,dw-pcie.yaml does have the atu reg defined, in order to be
> > > able to use this reg, while still making sure 'make CHECK_DTBS=y'
> > > pass, we need to add this reg to rockchip-dw-pcie.yaml.
> > >
> > > All compatible strings (rockchip,rk3568-pcie and rockchip,rk3588-pcie)
> > > should have this reg.
> > >
> > > The regs in the example are updated to actually match pcie3x2 on rk3568.
> >
> > Breaking compatibility on these platforms is okay because ...?
>
> I don't follow, could you please elaborate?
A DT which was valid without 'atu' region is now not valid with this
change. If I'm reading this schema with the change, then not having
'atu' is an error and the OS can treat it as an error. If it does,
then it wouldn't work with existing DTs. You cannot add new required
properties or required entries.
If you can say no users of the affected platforms care (e.g. only you
have a board) or the binding is not yet in use, then it's fine. But
you have to state that justification. I suspect that is not the case
here.
Rob
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/6] dt-bindings: PCI: dwc: rockchip: Add mandatory atu reg
2023-10-27 17:03 ` Rob Herring
@ 2023-10-28 12:11 ` Niklas Cassel
2023-10-30 17:39 ` Rob Herring
0 siblings, 1 reply; 18+ messages in thread
From: Niklas Cassel @ 2023-10-28 12:11 UTC (permalink / raw)
To: Rob Herring
Cc: Niklas Cassel, Lorenzo Pieralisi, Krzysztof Wilczyński,
Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Shawn Lin, Simon Xue, Damien Le Moal, Sebastian Reichel,
Serge Semin, linux-pci@vger.kernel.org,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org
On Fri, Oct 27, 2023 at 12:03:15PM -0500, Rob Herring wrote:
> On Fri, Oct 27, 2023 at 11:15 AM Niklas Cassel <Niklas.Cassel@wdc.com> wrote:
> >
> > Hello Rob,
> >
> > On Fri, Oct 27, 2023 at 10:58:28AM -0500, Rob Herring wrote:
> > > On Fri, Oct 27, 2023 at 9:56 AM Niklas Cassel <nks@flawful.org> wrote:
> > > >
> > > > From: Niklas Cassel <niklas.cassel@wdc.com>
> > > >
> > > > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > > > using:
> > > >
> > > > allOf:
> > > > - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > >
> > > > and snps,dw-pcie.yaml does have the atu reg defined, in order to be
> > > > able to use this reg, while still making sure 'make CHECK_DTBS=y'
> > > > pass, we need to add this reg to rockchip-dw-pcie.yaml.
> > > >
> > > > All compatible strings (rockchip,rk3568-pcie and rockchip,rk3588-pcie)
> > > > should have this reg.
> > > >
> > > > The regs in the example are updated to actually match pcie3x2 on rk3568.
> > >
> > > Breaking compatibility on these platforms is okay because ...?
> >
> > I don't follow, could you please elaborate?
>
> A DT which was valid without 'atu' region is now not valid with this
> change. If I'm reading this schema with the change, then not having
> 'atu' is an error and the OS can treat it as an error. If it does,
> then it wouldn't work with existing DTs. You cannot add new required
> properties or required entries.
>
> If you can say no users of the affected platforms care (e.g. only you
> have a board) or the binding is not yet in use, then it's fine. But
> you have to state that justification. I suspect that is not the case
> here.
FWIW, I had "atu" as optional in v2 of this series.
Since I made the effort in v3 to add "atu" to all the existing users of the
rockchip binding, I thought that marking it required in the rockchip binding
would help ensure that any future rockchip platform does not forget to add
"atu".
I know that DT has to be backwards compatible, but the device driver works
fine with a DT that lacks "atu" (even though you will not be able to detect
all iATUs), so an old DT would still work.
But yes, running make CHECK_DTBS=y with the new binding, would not pass for
an old DT.
I get your point, you can never add a required property or entries to an
existing compatible (this is in use).
If we look at snps,dw-pcie.yaml, "atu" is optional, so that correlates to
the device driver being able to work without "atu" specified.
Since the rockchip driver doesn't get "atu" itself, but relies on the common
code to get it, it should obviously be optional also for the rockchip binding.
I guess my problem is that I just want to inherit the entry from the common
binding...
Is there no DT keyword to extend an existing binding, so that you inherit all
the properties/entries from that common binding, while still allowing you to
define (or overload) additional properties/entries?
Even if there is no way to inherit all properties/entries from a common binding,
it would be nice to be able to inherit a specific property/entry using a
"inherit" keyword, such that you get the same definition (optional/required) as
the parent binding.
Kind regards,
Niklas
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/6] dt-bindings: PCI: dwc: rockchip: Add mandatory atu reg
2023-10-28 12:11 ` Niklas Cassel
@ 2023-10-30 17:39 ` Rob Herring
0 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2023-10-30 17:39 UTC (permalink / raw)
To: Niklas Cassel
Cc: Niklas Cassel, Lorenzo Pieralisi, Krzysztof Wilczyński,
Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Shawn Lin, Simon Xue, Damien Le Moal, Sebastian Reichel,
Serge Semin, linux-pci@vger.kernel.org,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org
On Sat, Oct 28, 2023 at 12:11:02PM +0000, Niklas Cassel wrote:
> On Fri, Oct 27, 2023 at 12:03:15PM -0500, Rob Herring wrote:
> > On Fri, Oct 27, 2023 at 11:15 AM Niklas Cassel <Niklas.Cassel@wdc.com> wrote:
> > >
> > > Hello Rob,
> > >
> > > On Fri, Oct 27, 2023 at 10:58:28AM -0500, Rob Herring wrote:
> > > > On Fri, Oct 27, 2023 at 9:56 AM Niklas Cassel <nks@flawful.org> wrote:
> > > > >
> > > > > From: Niklas Cassel <niklas.cassel@wdc.com>
> > > > >
> > > > > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > > > > using:
> > > > >
> > > > > allOf:
> > > > > - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > > >
> > > > > and snps,dw-pcie.yaml does have the atu reg defined, in order to be
> > > > > able to use this reg, while still making sure 'make CHECK_DTBS=y'
> > > > > pass, we need to add this reg to rockchip-dw-pcie.yaml.
> > > > >
> > > > > All compatible strings (rockchip,rk3568-pcie and rockchip,rk3588-pcie)
> > > > > should have this reg.
> > > > >
> > > > > The regs in the example are updated to actually match pcie3x2 on rk3568.
> > > >
> > > > Breaking compatibility on these platforms is okay because ...?
> > >
> > > I don't follow, could you please elaborate?
> >
> > A DT which was valid without 'atu' region is now not valid with this
> > change. If I'm reading this schema with the change, then not having
> > 'atu' is an error and the OS can treat it as an error. If it does,
> > then it wouldn't work with existing DTs. You cannot add new required
> > properties or required entries.
> >
> > If you can say no users of the affected platforms care (e.g. only you
> > have a board) or the binding is not yet in use, then it's fine. But
> > you have to state that justification. I suspect that is not the case
> > here.
>
> FWIW, I had "atu" as optional in v2 of this series.
Right, that was "correct".
> Since I made the effort in v3 to add "atu" to all the existing users of the
> rockchip binding, I thought that marking it required in the rockchip binding
> would help ensure that any future rockchip platform does not forget to add
> "atu".
It would nice to test both this kind of thing and compatibility. The
only way I know of to do that is with 'deprecated' keyword which still
needs support in dtschema to remove all the deprecated schemas (which
would then cause warnings). That's not hard, I just haven't done it
yet.
To use 'deprecated' here, you could do something like this:
items:
- ...
- ...
- ...
- ...
allOf:
- deprecated: true
minItems: 3
However, that would also not work because we implicitly add 'minItems:
4', but that could be fixed.
For 'required', we'd need a 'oneOf' with 2 lists of required properties
which is kind of ugly.
> I know that DT has to be backwards compatible, but the device driver works
> fine with a DT that lacks "atu" (even though you will not be able to detect
> all iATUs), so an old DT would still work.
>
> But yes, running make CHECK_DTBS=y with the new binding, would not pass for
> an old DT.
>
> I get your point, you can never add a required property or entries to an
> existing compatible (this is in use).
We may test for this at some point. I want to be able to test for ABI
breaks rather than catch them in reviews. Right now I'm just kicking
around ideas in my head on how to do that...
> If we look at snps,dw-pcie.yaml, "atu" is optional, so that correlates to
> the device driver being able to work without "atu" specified.
>
> Since the rockchip driver doesn't get "atu" itself, but relies on the common
> code to get it, it should obviously be optional also for the rockchip binding.
The way this binding works is snps,dw-pcie.yaml defines the set of
common properties. The SoC specific binding still has to define which
ones it uses. It's some duplication, but there's not really any way
around it on these licensed IPs unless the bindings are complete up
front.
> I guess my problem is that I just want to inherit the entry from the common
> binding...
>
> Is there no DT keyword to extend an existing binding, so that you inherit all
> the properties/entries from that common binding, while still allowing you to
> define (or overload) additional properties/entries?
>
> Even if there is no way to inherit all properties/entries from a common binding,
> it would be nice to be able to inherit a specific property/entry using a
> "inherit" keyword, such that you get the same definition (optional/required) as
> the parent binding.
The only way to inherit is via a $ref. We can only add more constraints
to a parent/referenced binding.
You could have a $ref to
"snps,dw-pcie.yaml#/properties/reg-names/items/oneOf/3" to get the 4th
reg-name in the list of common properties. I wouldn't recommend doing
that because I think it will be fragile and difficult to maintain.
Rob
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/6] dt-bindings: PCI: dwc: rockchip: Add mandatory atu reg
2023-10-27 16:14 ` Niklas Cassel
2023-10-27 17:03 ` Rob Herring
@ 2023-10-27 17:07 ` Heiko Stübner
1 sibling, 0 replies; 18+ messages in thread
From: Heiko Stübner @ 2023-10-27 17:07 UTC (permalink / raw)
To: Rob Herring, Niklas Cassel
Cc: Niklas Cassel, Lorenzo Pieralisi, Krzysztof Wilczyński,
Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Shawn Lin,
Simon Xue, Damien Le Moal, Sebastian Reichel, Serge Semin,
linux-pci@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org
Hi Niklas,
Am Freitag, 27. Oktober 2023, 18:14:32 CEST schrieb Niklas Cassel:
> On Fri, Oct 27, 2023 at 10:58:28AM -0500, Rob Herring wrote:
> > On Fri, Oct 27, 2023 at 9:56 AM Niklas Cassel <nks@flawful.org> wrote:
> > >
> > > From: Niklas Cassel <niklas.cassel@wdc.com>
> > >
> > > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > > using:
> > >
> > > allOf:
> > > - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > >
> > > and snps,dw-pcie.yaml does have the atu reg defined, in order to be
> > > able to use this reg, while still making sure 'make CHECK_DTBS=y'
> > > pass, we need to add this reg to rockchip-dw-pcie.yaml.
> > >
> > > All compatible strings (rockchip,rk3568-pcie and rockchip,rk3588-pcie)
> > > should have this reg.
> > >
> > > The regs in the example are updated to actually match pcie3x2 on rk3568.
> >
> > Breaking compatibility on these platforms is okay because ...?
>
> I don't follow, could you please elaborate?
you're adding the atu reg unconditionally as required element.
Newer kernel versions (strongly) _should_ work with older devicetrees.
So a kernel with that change should also work with a dtb build from the
old style.
DTBs are essentially part of the device firmware, so while some devices
can update theirs easily, you can't really require a dtb update.
I guess you could something like:
reg-names:
oneOf:
- deprecated: true
items:
- const: dbi
- const: apb
- const: config
- items:
- const: dbi
- const: apb
- const: config
- const: atu
(may not be accurate and to spec yet)
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 2/6] dt-bindings: PCI: dwc: rockchip: Add optional dma interrupts
2023-10-27 14:54 [PATCH v3 0/6] rockchip DWC PCIe improvements Niklas Cassel
2023-10-27 14:54 ` [PATCH v3 1/6] dt-bindings: PCI: dwc: rockchip: Add mandatory atu reg Niklas Cassel
@ 2023-10-27 14:54 ` Niklas Cassel
2023-10-30 17:40 ` Rob Herring
2023-10-31 1:10 ` Serge Semin
2023-10-27 14:54 ` [PATCH v3 3/6] arm64: dts: rockchip: drop unused properties num-{ib,ob}-windows Niklas Cassel
` (3 subsequent siblings)
5 siblings, 2 replies; 18+ messages in thread
From: Niklas Cassel @ 2023-10-27 14:54 UTC (permalink / raw)
To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Shawn Lin, Simon Xue
Cc: Damien Le Moal, Sebastian Reichel, Serge Semin, Niklas Cassel,
linux-pci, devicetree, linux-arm-kernel, linux-rockchip
From: Niklas Cassel <niklas.cassel@wdc.com>
Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
using:
allOf:
- $ref: /schemas/pci/snps,dw-pcie.yaml#
and snps,dw-pcie.yaml does have the dma interrupts defined, in order to be
able to use these interrupts, while still making sure 'make CHECK_DTBS=y'
pass, we need to add these interrupts to rockchip-dw-pcie.yaml.
These dedicated interrupts for the eDMA are not always wired to all the
PCIe controllers on the same SoC. In other words, even for a specific
compatible, e.g. rockchip,rk3588-pcie, these dedicated eDMA interrupts
may or may not be wired.
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
.../bindings/pci/rockchip-dw-pcie.yaml | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
index 6ca87ff4ae20..7ad6e1283784 100644
--- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
@@ -63,6 +63,7 @@ properties:
- const: pipe
interrupts:
+ minItems: 5
items:
- description:
Combined system interrupt, which is used to signal the following
@@ -86,14 +87,31 @@ properties:
interrupts - aer_rc_err, aer_rc_err_msi, rx_cpl_timeout,
tx_cpl_timeout, cor_err_sent, nf_err_sent, f_err_sent, cor_err_rx,
nf_err_rx, f_err_rx, radm_qoverflow
+ - description:
+ Indicates that the eDMA Tx/Rx transfer is complete or that an
+ error has occurred on the corresponding channel.
+ - description:
+ Indicates that the eDMA Tx/Rx transfer is complete or that an
+ error has occurred on the corresponding channel.
+ - description:
+ Indicates that the eDMA Tx/Rx transfer is complete or that an
+ error has occurred on the corresponding channel.
+ - description:
+ Indicates that the eDMA Tx/Rx transfer is complete or that an
+ error has occurred on the corresponding channel.
interrupt-names:
+ minItems: 5
items:
- const: sys
- const: pmc
- const: msg
- const: legacy
- const: err
+ - const: dma0
+ - const: dma1
+ - const: dma2
+ - const: dma3
legacy-interrupt-controller:
description: Interrupt controller node for handling legacy PCI interrupts.
--
2.41.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3 2/6] dt-bindings: PCI: dwc: rockchip: Add optional dma interrupts
2023-10-27 14:54 ` [PATCH v3 2/6] dt-bindings: PCI: dwc: rockchip: Add optional dma interrupts Niklas Cassel
@ 2023-10-30 17:40 ` Rob Herring
2023-10-31 1:10 ` Serge Semin
1 sibling, 0 replies; 18+ messages in thread
From: Rob Herring @ 2023-10-30 17:40 UTC (permalink / raw)
To: Niklas Cassel
Cc: Lorenzo Pieralisi, Damien Le Moal, linux-arm-kernel, linux-pci,
linux-rockchip, Niklas Cassel, Simon Xue, Krzysztof Kozlowski,
Bjorn Helgaas, Heiko Stuebner, Serge Semin, Conor Dooley,
Krzysztof Wilczyński, Sebastian Reichel, devicetree,
Shawn Lin
On Fri, 27 Oct 2023 16:54:14 +0200, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
>
> Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> using:
>
> allOf:
> - $ref: /schemas/pci/snps,dw-pcie.yaml#
>
> and snps,dw-pcie.yaml does have the dma interrupts defined, in order to be
> able to use these interrupts, while still making sure 'make CHECK_DTBS=y'
> pass, we need to add these interrupts to rockchip-dw-pcie.yaml.
>
> These dedicated interrupts for the eDMA are not always wired to all the
> PCIe controllers on the same SoC. In other words, even for a specific
> compatible, e.g. rockchip,rk3588-pcie, these dedicated eDMA interrupts
> may or may not be wired.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
> .../bindings/pci/rockchip-dw-pcie.yaml | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 2/6] dt-bindings: PCI: dwc: rockchip: Add optional dma interrupts
2023-10-27 14:54 ` [PATCH v3 2/6] dt-bindings: PCI: dwc: rockchip: Add optional dma interrupts Niklas Cassel
2023-10-30 17:40 ` Rob Herring
@ 2023-10-31 1:10 ` Serge Semin
2023-10-31 15:51 ` Niklas Cassel
1 sibling, 1 reply; 18+ messages in thread
From: Serge Semin @ 2023-10-31 1:10 UTC (permalink / raw)
To: Niklas Cassel
Cc: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Shawn Lin, Simon Xue, Damien Le Moal, Sebastian Reichel,
Niklas Cassel, linux-pci, devicetree, linux-arm-kernel,
linux-rockchip
On Fri, Oct 27, 2023 at 04:54:14PM +0200, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
>
> Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> using:
>
> allOf:
> - $ref: /schemas/pci/snps,dw-pcie.yaml#
>
> and snps,dw-pcie.yaml does have the dma interrupts defined, in order to be
> able to use these interrupts, while still making sure 'make CHECK_DTBS=y'
> pass, we need to add these interrupts to rockchip-dw-pcie.yaml.
>
> These dedicated interrupts for the eDMA are not always wired to all the
> PCIe controllers on the same SoC. In other words, even for a specific
> compatible, e.g. rockchip,rk3588-pcie, these dedicated eDMA interrupts
> may or may not be wired.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
> .../bindings/pci/rockchip-dw-pcie.yaml | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> index 6ca87ff4ae20..7ad6e1283784 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> @@ -63,6 +63,7 @@ properties:
> - const: pipe
>
> interrupts:
> + minItems: 5
> items:
> - description:
> Combined system interrupt, which is used to signal the following
> @@ -86,14 +87,31 @@ properties:
> interrupts - aer_rc_err, aer_rc_err_msi, rx_cpl_timeout,
> tx_cpl_timeout, cor_err_sent, nf_err_sent, f_err_sent, cor_err_rx,
> nf_err_rx, f_err_rx, radm_qoverflow
> + - description:
> + Indicates that the eDMA Tx/Rx transfer is complete or that an
> + error has occurred on the corresponding channel.
> + - description:
> + Indicates that the eDMA Tx/Rx transfer is complete or that an
> + error has occurred on the corresponding channel.
> + - description:
> + Indicates that the eDMA Tx/Rx transfer is complete or that an
> + error has occurred on the corresponding channel.
> + - description:
> + Indicates that the eDMA Tx/Rx transfer is complete or that an
> + error has occurred on the corresponding channel.
>
> interrupt-names:
> + minItems: 5
> items:
> - const: sys
> - const: pmc
> - const: msg
> - const: legacy
> - const: err
> + - const: dma0
> + - const: dma1
> + - const: dma2
> + - const: dma3
No. As you said yourself here
https://lore.kernel.org/linux-pci/ZTl1ZsHvk3DDHWsm@x1-carbon/
and in the response to my last message in v2, which for some reason
hasn't got to the lore archive:
On Fri, Oct 27, 2023 at 05:51:14PM +0200, Niklas Cassel wrote:
> However, e.g. rk3568 only has one channel for reads and one for writes.
> (Now this SoC doesn't have dedicated IRQs for the eDMA, but let's pretend
> that it did.)
>
> So for rk3568, it would then instead be:
> dma0: wr0
> dma1: rd0
> dma2: <unused>
> dma3: <unused>
rk3568 doesn't have IRQs supplied in a normal way, as separate
signals. Instead they are combined in the 'sys' IRQ. So you should
define the IRQs constraint being device-specific by using for example
the "allOf: if-else" pattern.
-Serge(y)
>
> legacy-interrupt-controller:
> description: Interrupt controller node for handling legacy PCI interrupts.
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 2/6] dt-bindings: PCI: dwc: rockchip: Add optional dma interrupts
2023-10-31 1:10 ` Serge Semin
@ 2023-10-31 15:51 ` Niklas Cassel
2023-11-01 22:23 ` Bjorn Helgaas
0 siblings, 1 reply; 18+ messages in thread
From: Niklas Cassel @ 2023-10-31 15:51 UTC (permalink / raw)
To: Serge Semin
Cc: Niklas Cassel, Bjorn Helgaas, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, Shawn Lin, Simon Xue,
Damien Le Moal, Sebastian Reichel, linux-pci@vger.kernel.org,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org
On Tue, Oct 31, 2023 at 04:10:17AM +0300, Serge Semin wrote:
> On Fri, Oct 27, 2023 at 05:51:14PM +0200, Niklas Cassel wrote:
> > However, e.g. rk3568 only has one channel for reads and one for writes.
> > (Now this SoC doesn't have dedicated IRQs for the eDMA, but let's pretend
> > that it did.)
> >
> > So for rk3568, it would then instead be:
> > dma0: wr0
> > dma1: rd0
> > dma2: <unused>
> > dma3: <unused>
>
> rk3568 doesn't have IRQs supplied in a normal way, as separate
> signals. Instead they are combined in the 'sys' IRQ. So you should
> define the IRQs constraint being device-specific by using for example
> the "allOf: if-else" pattern.
Thank you for your review comment.
I agree. Will fix this in next version.
Kind regards,
Niklas
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 2/6] dt-bindings: PCI: dwc: rockchip: Add optional dma interrupts
2023-10-31 15:51 ` Niklas Cassel
@ 2023-11-01 22:23 ` Bjorn Helgaas
0 siblings, 0 replies; 18+ messages in thread
From: Bjorn Helgaas @ 2023-11-01 22:23 UTC (permalink / raw)
To: Niklas Cassel
Cc: Serge Semin, Niklas Cassel, Bjorn Helgaas, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, Shawn Lin, Simon Xue,
Damien Le Moal, Sebastian Reichel, linux-pci@vger.kernel.org,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org
On Tue, Oct 31, 2023 at 03:51:03PM +0000, Niklas Cassel wrote:
> On Tue, Oct 31, 2023 at 04:10:17AM +0300, Serge Semin wrote:
> > On Fri, Oct 27, 2023 at 05:51:14PM +0200, Niklas Cassel wrote:
> > > However, e.g. rk3568 only has one channel for reads and one for writes.
> > > (Now this SoC doesn't have dedicated IRQs for the eDMA, but let's pretend
> > > that it did.)
> > >
> > > So for rk3568, it would then instead be:
> > > dma0: wr0
> > > dma1: rd0
> > > dma2: <unused>
> > > dma3: <unused>
> >
> > rk3568 doesn't have IRQs supplied in a normal way, as separate
> > signals. Instead they are combined in the 'sys' IRQ. So you should
> > define the IRQs constraint being device-specific by using for example
> > the "allOf: if-else" pattern.
>
> Thank you for your review comment.
>
> I agree. Will fix this in next version.
When you do, would you mind capitalizing "ATU", "DMA", etc in your
subject lines, commit logs, comments, etc? Then it'll be more obvious
that these aren't ordinary English words.
Bjorn
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 3/6] arm64: dts: rockchip: drop unused properties num-{ib,ob}-windows
2023-10-27 14:54 [PATCH v3 0/6] rockchip DWC PCIe improvements Niklas Cassel
2023-10-27 14:54 ` [PATCH v3 1/6] dt-bindings: PCI: dwc: rockchip: Add mandatory atu reg Niklas Cassel
2023-10-27 14:54 ` [PATCH v3 2/6] dt-bindings: PCI: dwc: rockchip: Add optional dma interrupts Niklas Cassel
@ 2023-10-27 14:54 ` Niklas Cassel
2023-10-31 1:14 ` Serge Semin
2023-10-27 14:54 ` [PATCH v3 4/6] arm64: dts: rockchip: add missing mandatory rk3568 PCIe atu reg Niklas Cassel
` (2 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Niklas Cassel @ 2023-10-27 14:54 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner
Cc: Damien Le Moal, Sebastian Reichel, Rob Herring, Serge Semin,
Niklas Cassel, devicetree, linux-arm-kernel, linux-rockchip
From: Niklas Cassel <niklas.cassel@wdc.com>
The properties num-ib-windows and num-ob-windows have been deprecated for
a long time, and since commit 281f1f99cf3a ("PCI: dwc: Detect number of
iATU windows"), these properties are no longer used by the driver.
The correct number of inbound and outbound iATUs are now detected at
runtime. Thus, drop these unused properties.
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
arch/arm64/boot/dts/rockchip/rk3568-nanopi-r5s.dts | 2 --
arch/arm64/boot/dts/rockchip/rk3568.dtsi | 4 ----
arch/arm64/boot/dts/rockchip/rk356x.dtsi | 2 --
3 files changed, 8 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3568-nanopi-r5s.dts b/arch/arm64/boot/dts/rockchip/rk3568-nanopi-r5s.dts
index b6ad8328c7eb..da4927a35142 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-nanopi-r5s.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-nanopi-r5s.dts
@@ -102,8 +102,6 @@ &pcie3x1 {
&pcie3x2 {
num-lanes = <1>;
- num-ib-windows = <8>;
- num-ob-windows = <8>;
reset-gpios = <&gpio2 RK_PD6 GPIO_ACTIVE_HIGH>;
vpcie3v3-supply = <&vcc3v3_pcie>;
status = "okay";
diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
index f1be76a54ceb..4487754065b7 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
@@ -84,8 +84,6 @@ pcie3x1: pcie@fe270000 {
<0 0 0 3 &pcie3x1_intc 2>,
<0 0 0 4 &pcie3x1_intc 3>;
linux,pci-domain = <1>;
- num-ib-windows = <6>;
- num-ob-windows = <2>;
max-link-speed = <3>;
msi-map = <0x0 &gic 0x1000 0x1000>;
num-lanes = <1>;
@@ -137,8 +135,6 @@ pcie3x2: pcie@fe280000 {
<0 0 0 3 &pcie3x2_intc 2>,
<0 0 0 4 &pcie3x2_intc 3>;
linux,pci-domain = <2>;
- num-ib-windows = <6>;
- num-ob-windows = <2>;
max-link-speed = <3>;
msi-map = <0x0 &gic 0x2000 0x1000>;
num-lanes = <2>;
diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index abee88911982..e2d99613109b 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -985,8 +985,6 @@ pcie2x1: pcie@fe260000 {
<0 0 0 3 &pcie_intc 2>,
<0 0 0 4 &pcie_intc 3>;
linux,pci-domain = <0>;
- num-ib-windows = <6>;
- num-ob-windows = <2>;
max-link-speed = <2>;
msi-map = <0x0 &gic 0x0 0x1000>;
num-lanes = <1>;
--
2.41.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3 3/6] arm64: dts: rockchip: drop unused properties num-{ib,ob}-windows
2023-10-27 14:54 ` [PATCH v3 3/6] arm64: dts: rockchip: drop unused properties num-{ib,ob}-windows Niklas Cassel
@ 2023-10-31 1:14 ` Serge Semin
0 siblings, 0 replies; 18+ messages in thread
From: Serge Semin @ 2023-10-31 1:14 UTC (permalink / raw)
To: Niklas Cassel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Damien Le Moal, Sebastian Reichel, Rob Herring, Niklas Cassel,
devicetree, linux-arm-kernel, linux-rockchip
On Fri, Oct 27, 2023 at 04:54:15PM +0200, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
>
> The properties num-ib-windows and num-ob-windows have been deprecated for
> a long time, and since commit 281f1f99cf3a ("PCI: dwc: Detect number of
> iATU windows"), these properties are no longer used by the driver.
>
> The correct number of inbound and outbound iATUs are now detected at
> runtime. Thus, drop these unused properties.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
-Serge(y)
> ---
> arch/arm64/boot/dts/rockchip/rk3568-nanopi-r5s.dts | 2 --
> arch/arm64/boot/dts/rockchip/rk3568.dtsi | 4 ----
> arch/arm64/boot/dts/rockchip/rk356x.dtsi | 2 --
> 3 files changed, 8 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-nanopi-r5s.dts b/arch/arm64/boot/dts/rockchip/rk3568-nanopi-r5s.dts
> index b6ad8328c7eb..da4927a35142 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-nanopi-r5s.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-nanopi-r5s.dts
> @@ -102,8 +102,6 @@ &pcie3x1 {
>
> &pcie3x2 {
> num-lanes = <1>;
> - num-ib-windows = <8>;
> - num-ob-windows = <8>;
> reset-gpios = <&gpio2 RK_PD6 GPIO_ACTIVE_HIGH>;
> vpcie3v3-supply = <&vcc3v3_pcie>;
> status = "okay";
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> index f1be76a54ceb..4487754065b7 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> @@ -84,8 +84,6 @@ pcie3x1: pcie@fe270000 {
> <0 0 0 3 &pcie3x1_intc 2>,
> <0 0 0 4 &pcie3x1_intc 3>;
> linux,pci-domain = <1>;
> - num-ib-windows = <6>;
> - num-ob-windows = <2>;
> max-link-speed = <3>;
> msi-map = <0x0 &gic 0x1000 0x1000>;
> num-lanes = <1>;
> @@ -137,8 +135,6 @@ pcie3x2: pcie@fe280000 {
> <0 0 0 3 &pcie3x2_intc 2>,
> <0 0 0 4 &pcie3x2_intc 3>;
> linux,pci-domain = <2>;
> - num-ib-windows = <6>;
> - num-ob-windows = <2>;
> max-link-speed = <3>;
> msi-map = <0x0 &gic 0x2000 0x1000>;
> num-lanes = <2>;
> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> index abee88911982..e2d99613109b 100644
> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> @@ -985,8 +985,6 @@ pcie2x1: pcie@fe260000 {
> <0 0 0 3 &pcie_intc 2>,
> <0 0 0 4 &pcie_intc 3>;
> linux,pci-domain = <0>;
> - num-ib-windows = <6>;
> - num-ob-windows = <2>;
> max-link-speed = <2>;
> msi-map = <0x0 &gic 0x0 0x1000>;
> num-lanes = <1>;
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 4/6] arm64: dts: rockchip: add missing mandatory rk3568 PCIe atu reg
2023-10-27 14:54 [PATCH v3 0/6] rockchip DWC PCIe improvements Niklas Cassel
` (2 preceding siblings ...)
2023-10-27 14:54 ` [PATCH v3 3/6] arm64: dts: rockchip: drop unused properties num-{ib,ob}-windows Niklas Cassel
@ 2023-10-27 14:54 ` Niklas Cassel
2023-10-27 14:54 ` [PATCH v3 5/6] arm64: dts: rockchip: add missing mandatory rk3588 " Niklas Cassel
2023-10-27 14:54 ` [PATCH v3 6/6] arm64: dts: rockchip: add missing rk3588 PCIe eDMA interrupts Niklas Cassel
5 siblings, 0 replies; 18+ messages in thread
From: Niklas Cassel @ 2023-10-27 14:54 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner
Cc: Damien Le Moal, Sebastian Reichel, Rob Herring, Serge Semin,
Niklas Cassel, devicetree, linux-arm-kernel, linux-rockchip
From: Niklas Cassel <niklas.cassel@wdc.com>
From the snps,dw-pcie.yaml devicetree binding:
"At least DBI reg-space and peripheral devices CFG-space outbound window
are required for the normal controller work. iATU memory IO region is
also required if the space is unrolled (IP-core version >= 4.80a)."
All the PCIe controllers in rk3568 are using the iATU unroll feature,
and thus have to supply the atu reg in the device tree node.
Without this patch, the driver will not be able to detect all the inbound
and outbound iATUs. (The default iATU range that is used by by the driver,
when no atu reg is found, allows the driver to detect up to a maximum
of 8 inbound and 8 outbound iATUs.)
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
arch/arm64/boot/dts/rockchip/rk3568.dtsi | 14 ++++++++------
arch/arm64/boot/dts/rockchip/rk356x.dtsi | 7 ++++---
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
index 4487754065b7..019429891288 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
@@ -90,13 +90,14 @@ pcie3x1: pcie@fe270000 {
phys = <&pcie30phy>;
phy-names = "pcie-phy";
power-domains = <&power RK3568_PD_PIPE>;
- reg = <0x3 0xc0400000 0x0 0x00400000>,
+ reg = <0x3 0xc0400000 0x0 0x00300000>,
<0x0 0xfe270000 0x0 0x00010000>,
- <0x0 0xf2000000 0x0 0x00100000>;
+ <0x0 0xf2000000 0x0 0x00100000>,
+ <0x3 0xc0700000 0x0 0x00100000>;
+ reg-names = "dbi", "apb", "config", "atu";
ranges = <0x01000000 0x0 0xf2100000 0x0 0xf2100000 0x0 0x00100000>,
<0x02000000 0x0 0xf2200000 0x0 0xf2200000 0x0 0x01e00000>,
<0x03000000 0x0 0x40000000 0x3 0x40000000 0x0 0x40000000>;
- reg-names = "dbi", "apb", "config";
resets = <&cru SRST_PCIE30X1_POWERUP>;
reset-names = "pipe";
/* bifurcation; lane1 when using 1+1 */
@@ -141,13 +142,14 @@ pcie3x2: pcie@fe280000 {
phys = <&pcie30phy>;
phy-names = "pcie-phy";
power-domains = <&power RK3568_PD_PIPE>;
- reg = <0x3 0xc0800000 0x0 0x00400000>,
+ reg = <0x3 0xc0800000 0x0 0x00300000>,
<0x0 0xfe280000 0x0 0x00010000>,
- <0x0 0xf0000000 0x0 0x00100000>;
+ <0x0 0xf0000000 0x0 0x00100000>,
+ <0x3 0xc0b00000 0x0 0x00100000>;
+ reg-names = "dbi", "apb", "config", "atu";
ranges = <0x01000000 0x0 0xf0100000 0x0 0xf0100000 0x0 0x00100000>,
<0x02000000 0x0 0xf0200000 0x0 0xf0200000 0x0 0x01e00000>,
<0x03000000 0x0 0x40000000 0x3 0x80000000 0x0 0x40000000>;
- reg-names = "dbi", "apb", "config";
resets = <&cru SRST_PCIE30X2_POWERUP>;
reset-names = "pipe";
/* bifurcation; lane0 when using 1+1 */
diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index e2d99613109b..872c6bc28559 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -961,10 +961,11 @@ qos_vop_m1: qos@fe1a8100 {
pcie2x1: pcie@fe260000 {
compatible = "rockchip,rk3568-pcie";
- reg = <0x3 0xc0000000 0x0 0x00400000>,
+ reg = <0x3 0xc0000000 0x0 0x00300000>,
<0x0 0xfe260000 0x0 0x00010000>,
- <0x0 0xf4000000 0x0 0x00100000>;
- reg-names = "dbi", "apb", "config";
+ <0x0 0xf4000000 0x0 0x00100000>,
+ <0x3 0xc0300000 0x0 0x00100000>;
+ reg-names = "dbi", "apb", "config", "atu";
interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>,
--
2.41.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 5/6] arm64: dts: rockchip: add missing mandatory rk3588 PCIe atu reg
2023-10-27 14:54 [PATCH v3 0/6] rockchip DWC PCIe improvements Niklas Cassel
` (3 preceding siblings ...)
2023-10-27 14:54 ` [PATCH v3 4/6] arm64: dts: rockchip: add missing mandatory rk3568 PCIe atu reg Niklas Cassel
@ 2023-10-27 14:54 ` Niklas Cassel
2023-10-27 14:54 ` [PATCH v3 6/6] arm64: dts: rockchip: add missing rk3588 PCIe eDMA interrupts Niklas Cassel
5 siblings, 0 replies; 18+ messages in thread
From: Niklas Cassel @ 2023-10-27 14:54 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Jagan Teki, Sebastian Reichel, Kever Yang
Cc: Damien Le Moal, Rob Herring, Serge Semin, Niklas Cassel,
devicetree, linux-arm-kernel, linux-rockchip
From: Niklas Cassel <niklas.cassel@wdc.com>
From the snps,dw-pcie.yaml devicetree binding:
"At least DBI reg-space and peripheral devices CFG-space outbound window
are required for the normal controller work. iATU memory IO region is
also required if the space is unrolled (IP-core version >= 4.80a)."
All the PCIe controllers in rk3588 are using the iATU unroll feature,
and thus have to supply the atu reg in the device tree node.
Without this patch, the driver will not be able to detect all the inbound
and outbound iATUs. (The default iATU range that is used by by the driver,
when no atu reg is found, allows the driver to detect up to a maximum
of 8 inbound and 8 outbound iATUs.)
On the rk3588 based rock-5b board:
Before this patch, dw_pcie_iatu_detect() fails to detect all iATUs:
rockchip-dw-pcie a40000000.pcie: iATU: unroll T, 8 ob, 8 ib, align 64K, limit 8G
rockchip-dw-pcie a41000000.pcie: iATU: unroll T, 8 ob, 8 ib, align 64K, limit 8G
rockchip-dw-pcie a40800000.pcie: iATU: unroll T, 8 ob, 8 ib, align 64K, limit 8G
After this patch, dw_pcie_iatu_detect() succeeds to detect all iATUs:
rockchip-dw-pcie a40000000.pcie: iATU: unroll T, 16 ob, 16 ib, align 64K, limit 8G
rockchip-dw-pcie a41000000.pcie: iATU: unroll T, 8 ob, 8 ib, align 64K, limit 8G
rockchip-dw-pcie a40800000.pcie: iATU: unroll T, 8 ob, 8 ib, align 64K, limit 8G
Fixes: 8d81b77f4c49 ("arm64: dts: rockchip: add rk3588 PCIe2 support")
Fixes: 0acf4fa7f187 ("arm64: dts: rockchip: add PCIe3 support for rk3588")
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
arch/arm64/boot/dts/rockchip/rk3588.dtsi | 21 ++++++++++++---------
arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 14 ++++++++------
2 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588.dtsi b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
index 5519c1430cb7..28955acda9f2 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
@@ -119,10 +119,11 @@ pcie3x4: pcie@fe150000 {
ranges = <0x01000000 0x0 0xf0100000 0x0 0xf0100000 0x0 0x00100000>,
<0x02000000 0x0 0xf0200000 0x0 0xf0200000 0x0 0x00e00000>,
<0x03000000 0x0 0x40000000 0x9 0x00000000 0x0 0x40000000>;
- reg = <0xa 0x40000000 0x0 0x00400000>,
+ reg = <0xa 0x40000000 0x0 0x00300000>,
<0x0 0xfe150000 0x0 0x00010000>,
- <0x0 0xf0000000 0x0 0x00100000>;
- reg-names = "dbi", "apb", "config";
+ <0x0 0xf0000000 0x0 0x00100000>,
+ <0xa 0x40300000 0x0 0x00100000>;
+ reg-names = "dbi", "apb", "config", "atu";
resets = <&cru SRST_PCIE0_POWER_UP>, <&cru SRST_P_PCIE0>;
reset-names = "pwr", "pipe";
status = "disabled";
@@ -170,10 +171,11 @@ pcie3x2: pcie@fe160000 {
ranges = <0x01000000 0x0 0xf1100000 0x0 0xf1100000 0x0 0x00100000>,
<0x02000000 0x0 0xf1200000 0x0 0xf1200000 0x0 0x00e00000>,
<0x03000000 0x0 0x40000000 0x9 0x40000000 0x0 0x40000000>;
- reg = <0xa 0x40400000 0x0 0x00400000>,
+ reg = <0xa 0x40400000 0x0 0x00300000>,
<0x0 0xfe160000 0x0 0x00010000>,
- <0x0 0xf1000000 0x0 0x00100000>;
- reg-names = "dbi", "apb", "config";
+ <0x0 0xf1000000 0x0 0x00100000>,
+ <0xa 0x40700000 0x0 0x00100000>;
+ reg-names = "dbi", "apb", "config", "atu";
resets = <&cru SRST_PCIE1_POWER_UP>, <&cru SRST_P_PCIE1>;
reset-names = "pwr", "pipe";
status = "disabled";
@@ -219,10 +221,11 @@ pcie2x1l0: pcie@fe170000 {
ranges = <0x01000000 0x0 0xf2100000 0x0 0xf2100000 0x0 0x00100000>,
<0x02000000 0x0 0xf2200000 0x0 0xf2200000 0x0 0x00e00000>,
<0x03000000 0x0 0x40000000 0x9 0x80000000 0x0 0x40000000>;
- reg = <0xa 0x40800000 0x0 0x00400000>,
+ reg = <0xa 0x40800000 0x0 0x00300000>,
<0x0 0xfe170000 0x0 0x00010000>,
- <0x0 0xf2000000 0x0 0x00100000>;
- reg-names = "dbi", "apb", "config";
+ <0x0 0xf2000000 0x0 0x00100000>,
+ <0xa 0x40b00000 0x0 0x00100000>;
+ reg-names = "dbi", "apb", "config", "atu";
resets = <&cru SRST_PCIE2_POWER_UP>, <&cru SRST_P_PCIE2>;
reset-names = "pwr", "pipe";
#address-cells = <3>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
index 5544f66c6ff4..60e02a5145e7 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
@@ -1259,10 +1259,11 @@ pcie2x1l1: pcie@fe180000 {
ranges = <0x01000000 0x0 0xf3100000 0x0 0xf3100000 0x0 0x00100000>,
<0x02000000 0x0 0xf3200000 0x0 0xf3200000 0x0 0x00e00000>,
<0x03000000 0x0 0x40000000 0x9 0xc0000000 0x0 0x40000000>;
- reg = <0xa 0x40c00000 0x0 0x00400000>,
+ reg = <0xa 0x40c00000 0x0 0x00300000>,
<0x0 0xfe180000 0x0 0x00010000>,
- <0x0 0xf3000000 0x0 0x00100000>;
- reg-names = "dbi", "apb", "config";
+ <0x0 0xf3000000 0x0 0x00100000>,
+ <0xa 0x40f00000 0x0 0x00100000>;
+ reg-names = "dbi", "apb", "config", "atu";
resets = <&cru SRST_PCIE3_POWER_UP>, <&cru SRST_P_PCIE3>;
reset-names = "pwr", "pipe";
#address-cells = <3>;
@@ -1310,10 +1311,11 @@ pcie2x1l2: pcie@fe190000 {
ranges = <0x01000000 0x0 0xf4100000 0x0 0xf4100000 0x0 0x00100000>,
<0x02000000 0x0 0xf4200000 0x0 0xf4200000 0x0 0x00e00000>,
<0x03000000 0x0 0x40000000 0xa 0x00000000 0x0 0x40000000>;
- reg = <0xa 0x41000000 0x0 0x00400000>,
+ reg = <0xa 0x41000000 0x0 0x00300000>,
<0x0 0xfe190000 0x0 0x00010000>,
- <0x0 0xf4000000 0x0 0x00100000>;
- reg-names = "dbi", "apb", "config";
+ <0x0 0xf4000000 0x0 0x00100000>,
+ <0xa 0x41300000 0x0 0x00100000>;
+ reg-names = "dbi", "apb", "config", "atu";
resets = <&cru SRST_PCIE4_POWER_UP>, <&cru SRST_P_PCIE4>;
reset-names = "pwr", "pipe";
#address-cells = <3>;
--
2.41.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 6/6] arm64: dts: rockchip: add missing rk3588 PCIe eDMA interrupts
2023-10-27 14:54 [PATCH v3 0/6] rockchip DWC PCIe improvements Niklas Cassel
` (4 preceding siblings ...)
2023-10-27 14:54 ` [PATCH v3 5/6] arm64: dts: rockchip: add missing mandatory rk3588 " Niklas Cassel
@ 2023-10-27 14:54 ` Niklas Cassel
5 siblings, 0 replies; 18+ messages in thread
From: Niklas Cassel @ 2023-10-27 14:54 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner
Cc: Damien Le Moal, Sebastian Reichel, Rob Herring, Serge Semin,
Niklas Cassel, devicetree, linux-arm-kernel, linux-rockchip
From: Niklas Cassel <niklas.cassel@wdc.com>
The rk3588 has 5 PCIe controllers, however, according the the rk3588 TRM
(Technical Reference Manual), only pcie3x4 has dedicated interrupts wired
to embedded DMA controller (eDMA) on the DWC PCIe controller.
The eDMA on pcie3x4 has two DMA read channels and two DMA write channels,
and according to snps,dw-pcie.yaml, the IRQs for the write channels have
to be specified before the IRQs for the read channels in "interrupts".
On the rk3588 based rock-5b board, when building with CONFIG_DW_EDMA=y:
Before this patch, only the iATUs are detected:
rockchip-dw-pcie a40000000.pcie: iATU: unroll T, 16 ob, 16 ib, align 64K, limit 8G
After this patch, both the iATUs and the eDMA channels are detected:
rockchip-dw-pcie a40000000.pcie: iATU: unroll T, 16 ob, 16 ib, align 64K, limit 8G
rockchip-dw-pcie a40000000.pcie: eDMA: unroll T, 2 wr, 2 rd
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
arch/arm64/boot/dts/rockchip/rk3588.dtsi | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588.dtsi b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
index 28955acda9f2..9b042f97f8c8 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
@@ -101,8 +101,13 @@ pcie3x4: pcie@fe150000 {
<GIC_SPI 262 IRQ_TYPE_LEVEL_HIGH 0>,
<GIC_SPI 261 IRQ_TYPE_LEVEL_HIGH 0>,
<GIC_SPI 260 IRQ_TYPE_LEVEL_HIGH 0>,
- <GIC_SPI 259 IRQ_TYPE_LEVEL_HIGH 0>;
- interrupt-names = "sys", "pmc", "msg", "legacy", "err";
+ <GIC_SPI 259 IRQ_TYPE_LEVEL_HIGH 0>,
+ <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH 0>,
+ <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH 0>,
+ <GIC_SPI 269 IRQ_TYPE_LEVEL_HIGH 0>,
+ <GIC_SPI 270 IRQ_TYPE_LEVEL_HIGH 0>;
+ interrupt-names = "sys", "pmc", "msg", "legacy", "err",
+ "dma0", "dma1", "dma2", "dma3";
#interrupt-cells = <1>;
interrupt-map-mask = <0 0 0 7>;
interrupt-map = <0 0 0 1 &pcie3x4_intc 0>,
--
2.41.0
^ permalink raw reply related [flat|nested] 18+ messages in thread