* [PATCH v9 1/8] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Host
2023-02-10 13:49 [PATCH v9 0/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
@ 2023-02-10 13:49 ` Yoshihiro Shimoda
2023-02-10 13:49 ` [PATCH v9 2/8] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Endpoint Yoshihiro Shimoda
` (6 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: Yoshihiro Shimoda @ 2023-02-10 13:49 UTC (permalink / raw)
To: lpieralisi, robh+dt, kw, bhelgaas, jingoohan1, gustavo.pimentel
Cc: Sergey.Semin, marek.vasut+renesas, linux-pci, devicetree,
linux-renesas-soc, Yoshihiro Shimoda, Rob Herring
Document bindings for Renesas R-Car Gen4 and R-Car S4-8 (R8A779F0)
PCIe host module.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../bindings/pci/rcar-gen4-pci-host.yaml | 90 +++++++++++++++++++
1 file changed, 90 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
diff --git a/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
new file mode 100644
index 000000000000..1bd7612101d8
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
@@ -0,0 +1,90 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2022 Renesas Electronics Corp.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/rcar-gen4-pci-host.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas R-Car Gen4 PCIe Host
+
+maintainers:
+ - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
+
+allOf:
+ - $ref: snps,dw-pcie.yaml#
+
+properties:
+ compatible:
+ items:
+ - const: renesas,r8a779f0-pcie # R-Car S4-8
+ - const: renesas,rcar-gen4-pcie # R-Car Gen4
+
+ interrupts:
+ maxItems: 4
+
+ interrupt-names:
+ items:
+ - const: msi
+ - const: dma
+ - const: sft_ce
+ - const: app
+
+ clocks:
+ maxItems: 1
+
+ power-domains:
+ maxItems: 1
+
+ resets:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - power-domains
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/r8a779f0-cpg-mssr.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/power/r8a779f0-sysc.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ pcie: pcie@e65d0000 {
+ compatible = "renesas,r8a779f0-pcie", "renesas,rcar-gen4-pcie";
+ reg = <0 0xe65d0000 0 0x3000>, <0 0xe65d3000 0 0x2000>,
+ <0 0xe65d6200 0 0x0e00>, <0 0xfe000000 0 0x400000>;
+ reg-names = "dbi", "atu", "app", "config";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ bus-range = <0x00 0xff>;
+ device_type = "pci";
+ ranges = <0x81000000 0 0x00000000 0 0xfe000000 0 0x00010000
+ 0x82000000 0 0x30000000 0 0x30000000 0 0x10000000>;
+ dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x80000000>;
+ interrupts = <GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 417 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 418 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "msi", "dma", "sft_ce", "app";
+ #interrupt-cells = <1>;
+ interrupt-map-mask = <0 0 0 7>;
+ interrupt-map = <0 0 0 1 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH
+ 0 0 0 2 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH
+ 0 0 0 3 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH
+ 0 0 0 4 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_MOD 624>;
+ power-domains = <&sysc R8A779F0_PD_ALWAYS_ON>;
+ resets = <&cpg 624>;
+ num-lanes = <2>;
+ snps,enable-cdm-check;
+ max-link-speed = <2>;
+ };
+ };
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v9 2/8] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Endpoint
2023-02-10 13:49 [PATCH v9 0/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
2023-02-10 13:49 ` [PATCH v9 1/8] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Host Yoshihiro Shimoda
@ 2023-02-10 13:49 ` Yoshihiro Shimoda
2023-02-12 21:11 ` Serge Semin
2023-02-10 13:49 ` [PATCH v9 3/8] PCI: Add PCI_EXP_LNKCAP_MLW macros Yoshihiro Shimoda
` (5 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Yoshihiro Shimoda @ 2023-02-10 13:49 UTC (permalink / raw)
To: lpieralisi, robh+dt, kw, bhelgaas, jingoohan1, gustavo.pimentel
Cc: Sergey.Semin, marek.vasut+renesas, linux-pci, devicetree,
linux-renesas-soc, Yoshihiro Shimoda, Rob Herring
Document bindings for Renesas R-Car Gen4 and R-Car S4-8 (R8A779F0)
PCIe endpoint module.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../bindings/pci/rcar-gen4-pci-ep.yaml | 90 +++++++++++++++++++
1 file changed, 90 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
diff --git a/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
new file mode 100644
index 000000000000..4b10d67e4336
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
@@ -0,0 +1,90 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2022 Renesas Electronics Corp.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/rcar-gen4-pci-ep.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas R-Car Gen4 PCIe Endpoint
+
+maintainers:
+ - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
+
+allOf:
+ - $ref: snps,dw-pcie-ep.yaml#
+
+properties:
+ compatible:
+ items:
+ - const: renesas,r8a779f0-pcie-ep # R-Car S4-8
+ - const: renesas,rcar-gen4-pcie-ep # R-Car Gen4
+
+ reg:
+ maxItems: 4
+
+ reg-names:
+ items:
+ - const: dbi
+ - const: atu
+ - const: appl
+ - const: addr_space
+
+ interrupts:
+ maxItems: 3
+
+ interrupt-names:
+ items:
+ - const: dma
+ - const: sft_ce
+ - const: app
+
+ power-domains:
+ maxItems: 1
+
+ resets:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ max-link-speed: true
+
+ num-lanes: true
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - interrupts
+ - resets
+ - power-domains
+ - clocks
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/r8a779f0-cpg-mssr.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/power/r8a779f0-sysc.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ pcie0_ep: pcie-ep@e65d0000 {
+ compatible = "renesas,r8a779f0-pcie-ep", "renesas,rcar-gen4-pcie-ep";
+ reg = <0 0xe65d0000 0 0x1000>, <0 0xe65d1000 0 0x1000>,
+ <0 0xe65d3000 0 0x2000>, <0 0xfe000000 0 0x400000>;
+ reg-names = "dbi", "atu", "appl", "addr_space";
+ interrupts = <GIC_SPI 417 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 418 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "dma", "sft_ce", "app";
+ clocks = <&cpg CPG_MOD 624>;
+ power-domains = <&sysc R8A779F0_PD_ALWAYS_ON>;
+ resets = <&cpg 624>;
+ num-lanes = <2>;
+ max-link-speed = <2>;
+ };
+ };
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v9 2/8] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Endpoint
2023-02-10 13:49 ` [PATCH v9 2/8] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Endpoint Yoshihiro Shimoda
@ 2023-02-12 21:11 ` Serge Semin
2023-02-13 4:54 ` Yoshihiro Shimoda
0 siblings, 1 reply; 20+ messages in thread
From: Serge Semin @ 2023-02-12 21:11 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: lpieralisi, robh+dt, kw, bhelgaas, jingoohan1, gustavo.pimentel,
Sergey.Semin, marek.vasut+renesas, linux-pci, devicetree,
linux-renesas-soc, Rob Herring
On Fri, Feb 10, 2023 at 10:49:11PM +0900, Yoshihiro Shimoda wrote:
> Document bindings for Renesas R-Car Gen4 and R-Car S4-8 (R8A779F0)
> PCIe endpoint module.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> .../bindings/pci/rcar-gen4-pci-ep.yaml | 90 +++++++++++++++++++
> 1 file changed, 90 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
>
> diff --git a/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
> new file mode 100644
> index 000000000000..4b10d67e4336
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
> @@ -0,0 +1,90 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2022 Renesas Electronics Corp.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/rcar-gen4-pci-ep.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas R-Car Gen4 PCIe Endpoint
> +
> +maintainers:
> + - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> +
> +allOf:
> + - $ref: snps,dw-pcie-ep.yaml#
> +
> +properties:
> + compatible:
> + items:
> + - const: renesas,r8a779f0-pcie-ep # R-Car S4-8
> + - const: renesas,rcar-gen4-pcie-ep # R-Car Gen4
> +
> + reg:
> + maxItems: 4
> +
> + reg-names:
> + items:
> + - const: dbi
> + - const: atu
> + - const: appl
Please, use "elbi" or "app" instead. No need in using the
vendor-specific names if there is the generic ones.
(* @Rob, that's why I was insisting in failing the DT-bindings
evaluation for such cases...)
See Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml:98
> + - const: addr_space
> +
> + interrupts:
> + maxItems: 3
> +
> + interrupt-names:
> + items:
> + - const: dma
Are you sure there is a single IRQ line for all eDMA channels?
Judging by the DW PCIe HW manual the eDMA events are signaled by the
wires: edma_int[((CC_NUM_DMA_RD_CHAN+CC_NUM_DMA_WR_CHAN)-1):0]. If you
have a single signal then they must have been combined on the way to
the GIC though...
> + - const: sft_ce
> + - const: app
> +
> + power-domains:
> + maxItems: 1
> +
> + resets:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + max-link-speed: true
This prop is determined by the CX_MAX_PCIE_SPEED IP-core synthesize
parameter.
> +
> + num-lanes: true
This is determined by the CX_NL IP-core synthesize parameter.
Thus you can provide at least the 'maximum' constraint for
the properties above.
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - interrupts
> + - resets
> + - power-domains
> + - clocks
> +
> +additionalProperties: false
Are you sure that none of the next properties will be ever used in
the R-Car PCIe End-point DT-nodes?
max-functions
max-virtual-functions
phys
phy-names
reset-gpios
snps,enable-cdm-check
dma-coherent
* etc
I am pretty much sure that the reset-gpios (platform-specific),
max-{virtual-}functions (determined by the CX_NFUNC IP-core synthesize
parameter), phys/phy-names (you had a PHYs CSR space in the
DT-bindings example) and dma-coherent properties are relevant for your
device. At the very least the 'max-functions' prop constraint could be
explicitly added to your DT-bindings file. You must be aware of how
many functions the R-Car PCIe EP support, right? The rest of the
properties could be implicitly evaluated by replacing the
"additionalProperties: false" flag with the "unevaluatedProperties:
false" statement.
-Serge(y)
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/r8a779f0-cpg-mssr.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/power/r8a779f0-sysc.h>
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + pcie0_ep: pcie-ep@e65d0000 {
> + compatible = "renesas,r8a779f0-pcie-ep", "renesas,rcar-gen4-pcie-ep";
> + reg = <0 0xe65d0000 0 0x1000>, <0 0xe65d1000 0 0x1000>,
> + <0 0xe65d3000 0 0x2000>, <0 0xfe000000 0 0x400000>;
> + reg-names = "dbi", "atu", "appl", "addr_space";
> + interrupts = <GIC_SPI 417 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 418 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "dma", "sft_ce", "app";
> + clocks = <&cpg CPG_MOD 624>;
> + power-domains = <&sysc R8A779F0_PD_ALWAYS_ON>;
> + resets = <&cpg 624>;
> + num-lanes = <2>;
> + max-link-speed = <2>;
> + };
> + };
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread* RE: [PATCH v9 2/8] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Endpoint
2023-02-12 21:11 ` Serge Semin
@ 2023-02-13 4:54 ` Yoshihiro Shimoda
0 siblings, 0 replies; 20+ messages in thread
From: Yoshihiro Shimoda @ 2023-02-13 4:54 UTC (permalink / raw)
To: Serge Semin
Cc: lpieralisi@kernel.org, robh+dt@kernel.org, kw@linux.com,
bhelgaas@google.com, jingoohan1@gmail.com,
gustavo.pimentel@synopsys.com, Sergey.Semin@baikalelectronics.ru,
marek.vasut+renesas@gmail.com, linux-pci@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
Rob Herring
Hi Serge,
> From: Serge Semin, Sent: Monday, February 13, 2023 6:11 AM
>
> On Fri, Feb 10, 2023 at 10:49:11PM +0900, Yoshihiro Shimoda wrote:
> > Document bindings for Renesas R-Car Gen4 and R-Car S4-8 (R8A779F0)
> > PCIe endpoint module.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > ---
> > .../bindings/pci/rcar-gen4-pci-ep.yaml | 90 +++++++++++++++++++
> > 1 file changed, 90 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
> b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
> > new file mode 100644
> > index 000000000000..4b10d67e4336
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
> > @@ -0,0 +1,90 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright (C) 2022 Renesas Electronics Corp.
> > +%YAML 1.2
> > +---
> > +$id:
<snip URL>
> > +$schema:
<snip URL>
> > +
> > +title: Renesas R-Car Gen4 PCIe Endpoint
> > +
> > +maintainers:
> > + - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > +
> > +allOf:
> > + - $ref: snps,dw-pcie-ep.yaml#
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - const: renesas,r8a779f0-pcie-ep # R-Car S4-8
> > + - const: renesas,rcar-gen4-pcie-ep # R-Car Gen4
> > +
> > + reg:
> > + maxItems: 4
> > +
> > + reg-names:
> > + items:
> > + - const: dbi
> > + - const: atu
>
> > + - const: appl
>
> Please, use "elbi" or "app" instead. No need in using the
> vendor-specific names if there is the generic ones.
> (* @Rob, that's why I was insisting in failing the DT-bindings
> evaluation for such cases...)
> See Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml:98
I got it. I'll fix it.
> > + - const: addr_space
> > +
> > + interrupts:
> > + maxItems: 3
> > +
> > + interrupt-names:
> > + items:
>
> > + - const: dma
>
> Are you sure there is a single IRQ line for all eDMA channels?
Yes.
> Judging by the DW PCIe HW manual the eDMA events are signaled by the
> wires: edma_int[((CC_NUM_DMA_RD_CHAN+CC_NUM_DMA_WR_CHAN)-1):0]. If you
> have a single signal then they must have been combined on the way to
> the GIC though...
I think so.
The drivers/dma/dw-edma/dw-edma-core.c seems to support nr_irqs == 1.
> > + - const: sft_ce
> > + - const: app
> > +
> > + power-domains:
> > + maxItems: 1
> > +
> > + resets:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
>
> > + max-link-speed: true
>
> This prop is determined by the CX_MAX_PCIE_SPEED IP-core synthesize
> parameter.
>
> > +
> > + num-lanes: true
>
> This is determined by the CX_NL IP-core synthesize parameter.
>
> Thus you can provide at least the 'maximum' constraint for
> the properties above.
I'll add 'maximum' to each property.
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - reg-names
> > + - interrupts
> > + - resets
> > + - power-domains
> > + - clocks
> > +
> > +additionalProperties: false
>
> Are you sure that none of the next properties will be ever used in
> the R-Car PCIe End-point DT-nodes?
> max-functions
> max-virtual-functions
> phys
> phy-names
> reset-gpios
> snps,enable-cdm-check
> dma-coherent
> * etc
>
> I am pretty much sure that the reset-gpios (platform-specific),
> max-{virtual-}functions (determined by the CX_NFUNC IP-core synthesize
> parameter), phys/phy-names (you had a PHYs CSR space in the
> DT-bindings example) and dma-coherent properties are relevant for your
> device. At the very least the 'max-functions' prop constraint could be
> explicitly added to your DT-bindings file. You must be aware of how
> many functions the R-Car PCIe EP support, right?
The R-Car PCIe EP supports two functions. So, I'll add 'max-functions'
property with 'maximum: 2'.
> The rest of the
> properties could be implicitly evaluated by replacing the
> "additionalProperties: false" flag with the "unevaluatedProperties:
> false" statement.
I got it. I'll change "additionalProperties: false" to
"unevaluatedProperties: false".
Best regards,
Yoshihiro Shimoda
> -Serge(y)
>
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/r8a779f0-cpg-mssr.h>
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #include <dt-bindings/power/r8a779f0-sysc.h>
> > +
> > + soc {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > +
> > + pcie0_ep: pcie-ep@e65d0000 {
> > + compatible = "renesas,r8a779f0-pcie-ep", "renesas,rcar-gen4-pcie-ep";
> > + reg = <0 0xe65d0000 0 0x1000>, <0 0xe65d1000 0 0x1000>,
> > + <0 0xe65d3000 0 0x2000>, <0 0xfe000000 0 0x400000>;
> > + reg-names = "dbi", "atu", "appl", "addr_space";
> > + interrupts = <GIC_SPI 417 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 418 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>;
> > + interrupt-names = "dma", "sft_ce", "app";
> > + clocks = <&cpg CPG_MOD 624>;
> > + power-domains = <&sysc R8A779F0_PD_ALWAYS_ON>;
> > + resets = <&cpg 624>;
> > + num-lanes = <2>;
> > + max-link-speed = <2>;
> > + };
> > + };
> > --
> > 2.25.1
> >
> >
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v9 3/8] PCI: Add PCI_EXP_LNKCAP_MLW macros
2023-02-10 13:49 [PATCH v9 0/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
2023-02-10 13:49 ` [PATCH v9 1/8] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Host Yoshihiro Shimoda
2023-02-10 13:49 ` [PATCH v9 2/8] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Endpoint Yoshihiro Shimoda
@ 2023-02-10 13:49 ` Yoshihiro Shimoda
2023-02-10 13:49 ` [PATCH v9 4/8] PCI: designware-ep: Expose dw_pcie_ep_exit() to module Yoshihiro Shimoda
` (4 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: Yoshihiro Shimoda @ 2023-02-10 13:49 UTC (permalink / raw)
To: lpieralisi, robh+dt, kw, bhelgaas, jingoohan1, gustavo.pimentel
Cc: Sergey.Semin, marek.vasut+renesas, linux-pci, devicetree,
linux-renesas-soc, Yoshihiro Shimoda
Add macros defining Maximum Link Width bits in Link Capabilities
Register.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
include/uapi/linux/pci_regs.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 85ab1278811e..c6fb2420cb17 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -538,6 +538,12 @@
#define PCI_EXP_LNKCAP_SLS_16_0GB 0x00000004 /* LNKCAP2 SLS Vector bit 3 */
#define PCI_EXP_LNKCAP_SLS_32_0GB 0x00000005 /* LNKCAP2 SLS Vector bit 4 */
#define PCI_EXP_LNKCAP_SLS_64_0GB 0x00000006 /* LNKCAP2 SLS Vector bit 5 */
+#define PCI_EXP_LNKCAP_MLW_X1 0x00000010 /* Maximum Link Width x1 */
+#define PCI_EXP_LNKCAP_MLW_X2 0x00000020 /* Maximum Link Width x2 */
+#define PCI_EXP_LNKCAP_MLW_X4 0x00000040 /* Maximum Link Width x4 */
+#define PCI_EXP_LNKCAP_MLW_X8 0x00000080 /* Maximum Link Width x8 */
+#define PCI_EXP_LNKCAP_MLW_X12 0x000000c0 /* Maximum Link Width x12 */
+#define PCI_EXP_LNKCAP_MLW_X16 0x00000100 /* Maximum Link Width x16 */
#define PCI_EXP_LNKCAP_MLW 0x000003f0 /* Maximum Link Width */
#define PCI_EXP_LNKCAP_ASPMS 0x00000c00 /* ASPM Support */
#define PCI_EXP_LNKCAP_ASPM_L0S 0x00000400 /* ASPM L0s Support */
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v9 4/8] PCI: designware-ep: Expose dw_pcie_ep_exit() to module
2023-02-10 13:49 [PATCH v9 0/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
` (2 preceding siblings ...)
2023-02-10 13:49 ` [PATCH v9 3/8] PCI: Add PCI_EXP_LNKCAP_MLW macros Yoshihiro Shimoda
@ 2023-02-10 13:49 ` Yoshihiro Shimoda
2023-02-10 13:49 ` [PATCH v9 5/8] PCI: dwc: Add support for triggering legacy IRQs Yoshihiro Shimoda
` (3 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: Yoshihiro Shimoda @ 2023-02-10 13:49 UTC (permalink / raw)
To: lpieralisi, robh+dt, kw, bhelgaas, jingoohan1, gustavo.pimentel
Cc: Sergey.Semin, marek.vasut+renesas, linux-pci, devicetree,
linux-renesas-soc, Yoshihiro Shimoda
Expose dw_pcie_ep_exit() to module.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
drivers/pci/controller/dwc/pcie-designware-ep.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index f9182f8d552f..95efe14f1036 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -622,6 +622,7 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
pci_epc_mem_exit(epc);
}
+EXPORT_SYMBOL_GPL(dw_pcie_ep_exit);
static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
{
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v9 5/8] PCI: dwc: Add support for triggering legacy IRQs
2023-02-10 13:49 [PATCH v9 0/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
` (3 preceding siblings ...)
2023-02-10 13:49 ` [PATCH v9 4/8] PCI: designware-ep: Expose dw_pcie_ep_exit() to module Yoshihiro Shimoda
@ 2023-02-10 13:49 ` Yoshihiro Shimoda
2023-02-15 15:19 ` Serge Semin
2023-02-10 13:49 ` [PATCH v9 6/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support Yoshihiro Shimoda
` (2 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Yoshihiro Shimoda @ 2023-02-10 13:49 UTC (permalink / raw)
To: lpieralisi, robh+dt, kw, bhelgaas, jingoohan1, gustavo.pimentel
Cc: Sergey.Semin, marek.vasut+renesas, linux-pci, devicetree,
linux-renesas-soc, Yoshihiro Shimoda
Add support for triggering legacy IRQs by using outbound ATU.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
.../pci/controller/dwc/pcie-designware-ep.c | 66 +++++++++++++++++--
drivers/pci/controller/dwc/pcie-designware.c | 25 ++++---
drivers/pci/controller/dwc/pcie-designware.h | 12 +++-
3 files changed, 87 insertions(+), 16 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 95efe14f1036..886483bf378b 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -6,6 +6,7 @@
* Author: Kishon Vijay Abraham I <kishon@ti.com>
*/
+#include <linux/delay.h>
#include <linux/of.h>
#include <linux/platform_device.h>
@@ -182,8 +183,8 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
return 0;
}
-static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
- phys_addr_t phys_addr,
+static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
+ u8 code, u8 routing, phys_addr_t phys_addr,
u64 pci_addr, size_t size)
{
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
@@ -196,8 +197,9 @@ static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
return -EINVAL;
}
- ret = dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win, PCIE_ATU_TYPE_MEM,
- phys_addr, pci_addr, size);
+ ret = dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win, type,
+ code, routing, phys_addr, pci_addr,
+ size);
if (ret)
return ret;
@@ -306,7 +308,8 @@ static int dw_pcie_ep_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
struct dw_pcie_ep *ep = epc_get_drvdata(epc);
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
- ret = dw_pcie_ep_outbound_atu(ep, func_no, addr, pci_addr, size);
+ ret = dw_pcie_ep_outbound_atu(ep, func_no, PCIE_ATU_TYPE_MEM, 0, 0,
+ addr, pci_addr, size);
if (ret) {
dev_err(pci->dev, "Failed to enable address\n");
return ret;
@@ -479,11 +482,43 @@ static const struct pci_epc_ops epc_ops = {
.get_features = dw_pcie_ep_get_features,
};
+static int dw_pcie_ep_send_msg(struct dw_pcie_ep *ep, u8 func_no, u8 code,
+ u8 routing)
+{
+ struct pci_epc *epc = ep->epc;
+ int ret;
+
+ ret = dw_pcie_ep_outbound_atu(ep, func_no, PCIE_ATU_TYPE_MSG, code,
+ routing, ep->intx_mem_phys, 0,
+ epc->mem->window.page_size);
+ if (ret)
+ return ret;
+ writel(0, ep->intx_mem);
+ dw_pcie_ep_unmap_addr(epc, func_no, 0, ep->intx_mem_phys);
+
+ return 0;
+}
+
+static int __dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no,
+ int intx)
+{
+ int ret;
+
+ ret = dw_pcie_ep_send_msg(ep, func_no, PCIE_MSG_ASSERT_INTA + intx, 0x04);
+ if (ret)
+ return ret;
+ usleep_range(1000, 2000);
+ return dw_pcie_ep_send_msg(ep, func_no, PCIE_MSG_DEASSERT_INTA + intx, 0x04);
+}
+
int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no)
{
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
struct device *dev = pci->dev;
+ if (ep->intx_by_atu)
+ return __dw_pcie_ep_raise_legacy_irq(ep, func_no, 0);
+
dev_err(dev, "EP cannot trigger legacy IRQs\n");
return -EINVAL;
@@ -617,6 +652,10 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
dw_pcie_edma_remove(pci);
+ if (ep->intx_by_atu)
+ pci_epc_mem_free_addr(epc, ep->intx_mem_phys, ep->intx_mem,
+ epc->mem->window.page_size);
+
pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
epc->mem->window.page_size);
@@ -789,9 +828,19 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
goto err_exit_epc_mem;
}
+ if (ep->intx_by_atu) {
+ ep->intx_mem = pci_epc_mem_alloc_addr(epc, &ep->intx_mem_phys,
+ epc->mem->window.page_size);
+ if (!ep->intx_mem) {
+ ret = -ENOMEM;
+ dev_err(dev, "Failed to reserve memory for INTx\n");
+ goto err_free_epc_mem;
+ }
+ }
+
ret = dw_pcie_edma_detect(pci);
if (ret)
- goto err_free_epc_mem;
+ goto err_free_epc_mem_intx;
if (ep->ops->get_features) {
epc_features = ep->ops->get_features(ep);
@@ -808,6 +857,11 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
err_remove_edma:
dw_pcie_edma_remove(pci);
+err_free_epc_mem_intx:
+ if (ep->intx_by_atu)
+ pci_epc_mem_free_addr(epc, ep->intx_mem_phys, ep->intx_mem,
+ epc->mem->window.page_size);
+
err_free_epc_mem:
pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
epc->mem->window.page_size);
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 53a16b8b6ac2..b4315cf84340 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -465,8 +465,8 @@ static inline u32 dw_pcie_enable_ecrc(u32 val)
}
static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
- int index, int type, u64 cpu_addr,
- u64 pci_addr, u64 size)
+ int index, int type, u8 code, u8 routing,
+ u64 cpu_addr, u64 pci_addr, u64 size)
{
u32 retries, val;
u64 limit_addr;
@@ -498,7 +498,7 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_UPPER_TARGET,
upper_32_bits(pci_addr));
- val = type | PCIE_ATU_FUNC_NUM(func_no);
+ val = type | routing | PCIE_ATU_FUNC_NUM(func_no);
if (upper_32_bits(limit_addr) > upper_32_bits(cpu_addr) &&
dw_pcie_ver_is_ge(pci, 460A))
val |= PCIE_ATU_INCREASE_REGION_SIZE;
@@ -506,7 +506,14 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
val = dw_pcie_enable_ecrc(val);
dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL1, val);
- dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2, PCIE_ATU_ENABLE);
+ if (code)
+ dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2,
+ PCIE_ATU_ENABLE |
+ PCIE_ATU_INHIBIT_PAYLOAD |
+ PCIE_ATU_HEADER_SUB_ENABLE | code);
+ else
+ dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2,
+ PCIE_ATU_ENABLE);
/*
* Make sure ATU enable takes effect before any subsequent config
@@ -528,16 +535,16 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
u64 cpu_addr, u64 pci_addr, u64 size)
{
- return __dw_pcie_prog_outbound_atu(pci, 0, index, type,
+ return __dw_pcie_prog_outbound_atu(pci, 0, index, type, 0, 0,
cpu_addr, pci_addr, size);
}
int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
- int type, u64 cpu_addr, u64 pci_addr,
- u64 size)
+ int type, u8 code, u8 routing, u64 cpu_addr,
+ u64 pci_addr, u64 size)
{
- return __dw_pcie_prog_outbound_atu(pci, func_no, index, type,
- cpu_addr, pci_addr, size);
+ return __dw_pcie_prog_outbound_atu(pci, func_no, index, type, code,
+ routing, cpu_addr, pci_addr, size);
}
static inline u32 dw_pcie_readl_atu_ib(struct dw_pcie *pci, u32 index, u32 reg)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 79713ce075cc..1a6e7e9489ee 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -147,11 +147,14 @@
#define PCIE_ATU_TYPE_IO 0x2
#define PCIE_ATU_TYPE_CFG0 0x4
#define PCIE_ATU_TYPE_CFG1 0x5
+#define PCIE_ATU_TYPE_MSG 0x10
#define PCIE_ATU_TD BIT(8)
#define PCIE_ATU_FUNC_NUM(pf) ((pf) << 20)
#define PCIE_ATU_REGION_CTRL2 0x004
#define PCIE_ATU_ENABLE BIT(31)
#define PCIE_ATU_BAR_MODE_ENABLE BIT(30)
+#define PCIE_ATU_INHIBIT_PAYLOAD BIT(22)
+#define PCIE_ATU_HEADER_SUB_ENABLE BIT(21)
#define PCIE_ATU_FUNC_NUM_MATCH_EN BIT(19)
#define PCIE_ATU_LOWER_BASE 0x008
#define PCIE_ATU_UPPER_BASE 0x00C
@@ -244,6 +247,9 @@
/* Default eDMA LLP memory size */
#define DMA_LLP_MEM_SIZE PAGE_SIZE
+#define PCIE_MSG_ASSERT_INTA 0x20
+#define PCIE_MSG_DEASSERT_INTA 0x24
+
struct dw_pcie;
struct dw_pcie_rp;
struct dw_pcie_ep;
@@ -352,7 +358,10 @@ struct dw_pcie_ep {
unsigned long *ob_window_map;
void __iomem *msi_mem;
phys_addr_t msi_mem_phys;
+ void __iomem *intx_mem;
+ phys_addr_t intx_mem_phys;
struct pci_epf_bar *epf_bar[PCI_STD_NUM_BARS];
+ bool intx_by_atu;
};
struct dw_pcie_ops {
@@ -419,7 +428,8 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci);
int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
u64 cpu_addr, u64 pci_addr, u64 size);
int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
- int type, u64 cpu_addr, u64 pci_addr, u64 size);
+ int type, u8 code, u8 routing, u64 cpu_addr,
+ u64 pci_addr, u64 size);
int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int type,
u64 cpu_addr, u64 pci_addr, u64 size);
int dw_pcie_prog_ep_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v9 5/8] PCI: dwc: Add support for triggering legacy IRQs
2023-02-10 13:49 ` [PATCH v9 5/8] PCI: dwc: Add support for triggering legacy IRQs Yoshihiro Shimoda
@ 2023-02-15 15:19 ` Serge Semin
2023-02-16 9:56 ` Yoshihiro Shimoda
0 siblings, 1 reply; 20+ messages in thread
From: Serge Semin @ 2023-02-15 15:19 UTC (permalink / raw)
To: Yoshihiro Shimoda, Rob Herring, Bjorn Helgaas
Cc: lpieralisi, robh+dt, kw, bhelgaas, jingoohan1, gustavo.pimentel,
Sergey.Semin, marek.vasut+renesas, linux-pci, devicetree,
linux-renesas-soc
On Fri, Feb 10, 2023 at 10:49:14PM +0900, Yoshihiro Shimoda wrote:
> Add support for triggering legacy IRQs by using outbound ATU.
I supposed a little more details would nice, like outbound iATU is
utilized to send assert and de-assert INTx TLPs. The message is
generated based on the payloadless Msg TLP with type 0x14, where 0x4
is the routing code implying the terminated at Receiver message. The
message code is specified as b1000xx for the INTx assertion and
b1001xx for the INTx de-assertion. Etc...
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
> .../pci/controller/dwc/pcie-designware-ep.c | 66 +++++++++++++++++--
> drivers/pci/controller/dwc/pcie-designware.c | 25 ++++---
> drivers/pci/controller/dwc/pcie-designware.h | 12 +++-
> 3 files changed, 87 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 95efe14f1036..886483bf378b 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -6,6 +6,7 @@
> * Author: Kishon Vijay Abraham I <kishon@ti.com>
> */
>
> +#include <linux/delay.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
>
> @@ -182,8 +183,8 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
> return 0;
> }
>
> -static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
> - phys_addr_t phys_addr,
> +static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
> + u8 code, u8 routing, phys_addr_t phys_addr,
> u64 pci_addr, size_t size)
> {
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> @@ -196,8 +197,9 @@ static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
> return -EINVAL;
> }
>
> - ret = dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win, PCIE_ATU_TYPE_MEM,
> - phys_addr, pci_addr, size);
> + ret = dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win, type,
> + code, routing, phys_addr, pci_addr,
> + size);
> if (ret)
> return ret;
>
> @@ -306,7 +308,8 @@ static int dw_pcie_ep_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>
> - ret = dw_pcie_ep_outbound_atu(ep, func_no, addr, pci_addr, size);
> + ret = dw_pcie_ep_outbound_atu(ep, func_no, PCIE_ATU_TYPE_MEM, 0, 0,
> + addr, pci_addr, size);
> if (ret) {
> dev_err(pci->dev, "Failed to enable address\n");
> return ret;
> @@ -479,11 +482,43 @@ static const struct pci_epc_ops epc_ops = {
> .get_features = dw_pcie_ep_get_features,
> };
>
> +static int dw_pcie_ep_send_msg(struct dw_pcie_ep *ep, u8 func_no, u8 code,
> + u8 routing)
> +{
> + struct pci_epc *epc = ep->epc;
> + int ret;
> +
> + ret = dw_pcie_ep_outbound_atu(ep, func_no, PCIE_ATU_TYPE_MSG, code,
> + routing, ep->intx_mem_phys, 0,
> + epc->mem->window.page_size);
> + if (ret)
> + return ret;
> + writel(0, ep->intx_mem);
> + dw_pcie_ep_unmap_addr(epc, func_no, 0, ep->intx_mem_phys);
> +
> + return 0;
> +}
> +
> +static int __dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no,
> + int intx)
> +{
> + int ret;
> +
> + ret = dw_pcie_ep_send_msg(ep, func_no, PCIE_MSG_ASSERT_INTA + intx, 0x04);
> + if (ret)
> + return ret;
> + usleep_range(1000, 2000);
> + return dw_pcie_ep_send_msg(ep, func_no, PCIE_MSG_DEASSERT_INTA + intx, 0x04);
> +}
> +
> int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no)
> {
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> struct device *dev = pci->dev;
>
> + if (ep->intx_by_atu)
IMO the flag is redundant. Your implementation is generic enough to be
useful for all the DW PCIe EPs. If you are afraid to break things,
then just make it optional. If no outbound physical memory could be
allocated then initialize intx_mem with NULL and move on with further
initializations. As before the Legacy IRQ raise method shall return
EINVAL in that case.
> + return __dw_pcie_ep_raise_legacy_irq(ep, func_no, 0);
> +
> dev_err(dev, "EP cannot trigger legacy IRQs\n");
>
> return -EINVAL;
> @@ -617,6 +652,10 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
>
> dw_pcie_edma_remove(pci);
>
> + if (ep->intx_by_atu)
> + pci_epc_mem_free_addr(epc, ep->intx_mem_phys, ep->intx_mem,
> + epc->mem->window.page_size);
> +
> pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
> epc->mem->window.page_size);
>
> @@ -789,9 +828,19 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> goto err_exit_epc_mem;
> }
>
> + if (ep->intx_by_atu) {
> + ep->intx_mem = pci_epc_mem_alloc_addr(epc, &ep->intx_mem_phys,
> + epc->mem->window.page_size);
> + if (!ep->intx_mem) {
> + ret = -ENOMEM;
> + dev_err(dev, "Failed to reserve memory for INTx\n");
As I suggested above you can just dev_warn() here and move on with
EP initialization.
> + goto err_free_epc_mem;
> + }
> + }
> +
> ret = dw_pcie_edma_detect(pci);
> if (ret)
> - goto err_free_epc_mem;
> + goto err_free_epc_mem_intx;
>
> if (ep->ops->get_features) {
> epc_features = ep->ops->get_features(ep);
> @@ -808,6 +857,11 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> err_remove_edma:
> dw_pcie_edma_remove(pci);
>
> +err_free_epc_mem_intx:
> + if (ep->intx_by_atu)
> + pci_epc_mem_free_addr(epc, ep->intx_mem_phys, ep->intx_mem,
> + epc->mem->window.page_size);
> +
> err_free_epc_mem:
> pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
> epc->mem->window.page_size);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 53a16b8b6ac2..b4315cf84340 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -465,8 +465,8 @@ static inline u32 dw_pcie_enable_ecrc(u32 val)
> }
>
> static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
> - int index, int type, u64 cpu_addr,
> - u64 pci_addr, u64 size)
> + int index, int type, u8 code, u8 routing,
> + u64 cpu_addr, u64 pci_addr, u64 size)
The implementation gets to be too complicated especially with having
nine arguments now. Seven args have been not that good either, but at
the very least it was more-or-less coherent with respect to the Mem/IO
outbound TLPs generations. Now in addition to that it will be also
responsible for the MSG TLPs mapping. What we could simplify here is:
< either 1. Drop separate routing arg. It can be merged into the type
argument seeing it's a part of one by specification. Thus we'll have
only eight arguments left. That will simplify the prototype, but not
the implementation.
< or 2. Split up the outbound mem/IO and MSG windows setups.
In the later case you'll need only the next data for the MSG TLPs
mapping: function #, MW index, message code, CPU base address, MW size.
< or 3. Convert __dw_pcie_prog_outbound_atu() to accepting a
dw_pcie_outbound_atu(-ish) structure with all the arguments listed as
fields: MW index, function #, message type, routing type, message
code (valid if MSG type specified), CPU base address, PCIe base
address, MW size.
What do you think? @Rob, @Bjorn?
(Please don't forget to define the macros for the routing and message
code values.)
> {
> u32 retries, val;
> u64 limit_addr;
> @@ -498,7 +498,7 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
> dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_UPPER_TARGET,
> upper_32_bits(pci_addr));
>
> - val = type | PCIE_ATU_FUNC_NUM(func_no);
> + val = type | routing | PCIE_ATU_FUNC_NUM(func_no);
> if (upper_32_bits(limit_addr) > upper_32_bits(cpu_addr) &&
> dw_pcie_ver_is_ge(pci, 460A))
> val |= PCIE_ATU_INCREASE_REGION_SIZE;
> @@ -506,7 +506,14 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
> val = dw_pcie_enable_ecrc(val);
> dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL1, val);
>
> - dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2, PCIE_ATU_ENABLE);
> + if (code)
> + dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2,
> + PCIE_ATU_ENABLE |
> + PCIE_ATU_INHIBIT_PAYLOAD |
> + PCIE_ATU_HEADER_SUB_ENABLE | code);
AFAICS the setup above is only specific to the Msg TLPs. If so then it
would have been more generic to use if(type == ?) conditional
statement here. What do you think?
> + else
> + dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2,
> + PCIE_ATU_ENABLE);
The next modification seems to be looking better in this case:
+ val = PCIE_ATU_ENABLE;
+ if (type == PCIE_ATU_TYPE_MSG)
+ val |= PCIE_ATU_INHIBIT_PAYLOAD | PCIE_ATU_HEADER_SUB_ENABLE | code;
+
+ dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2, val);
-Serge(y)
>
> /*
> * Make sure ATU enable takes effect before any subsequent config
> @@ -528,16 +535,16 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
> int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> u64 cpu_addr, u64 pci_addr, u64 size)
> {
> - return __dw_pcie_prog_outbound_atu(pci, 0, index, type,
> + return __dw_pcie_prog_outbound_atu(pci, 0, index, type, 0, 0,
> cpu_addr, pci_addr, size);
> }
>
> int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> - int type, u64 cpu_addr, u64 pci_addr,
> - u64 size)
> + int type, u8 code, u8 routing, u64 cpu_addr,
> + u64 pci_addr, u64 size)
> {
> - return __dw_pcie_prog_outbound_atu(pci, func_no, index, type,
> - cpu_addr, pci_addr, size);
> + return __dw_pcie_prog_outbound_atu(pci, func_no, index, type, code,
> + routing, cpu_addr, pci_addr, size);
> }
>
> static inline u32 dw_pcie_readl_atu_ib(struct dw_pcie *pci, u32 index, u32 reg)
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 79713ce075cc..1a6e7e9489ee 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -147,11 +147,14 @@
> #define PCIE_ATU_TYPE_IO 0x2
> #define PCIE_ATU_TYPE_CFG0 0x4
> #define PCIE_ATU_TYPE_CFG1 0x5
> +#define PCIE_ATU_TYPE_MSG 0x10
> #define PCIE_ATU_TD BIT(8)
> #define PCIE_ATU_FUNC_NUM(pf) ((pf) << 20)
> #define PCIE_ATU_REGION_CTRL2 0x004
> #define PCIE_ATU_ENABLE BIT(31)
> #define PCIE_ATU_BAR_MODE_ENABLE BIT(30)
> +#define PCIE_ATU_INHIBIT_PAYLOAD BIT(22)
> +#define PCIE_ATU_HEADER_SUB_ENABLE BIT(21)
> #define PCIE_ATU_FUNC_NUM_MATCH_EN BIT(19)
> #define PCIE_ATU_LOWER_BASE 0x008
> #define PCIE_ATU_UPPER_BASE 0x00C
> @@ -244,6 +247,9 @@
> /* Default eDMA LLP memory size */
> #define DMA_LLP_MEM_SIZE PAGE_SIZE
>
> +#define PCIE_MSG_ASSERT_INTA 0x20
> +#define PCIE_MSG_DEASSERT_INTA 0x24
> +
> struct dw_pcie;
> struct dw_pcie_rp;
> struct dw_pcie_ep;
> @@ -352,7 +358,10 @@ struct dw_pcie_ep {
> unsigned long *ob_window_map;
> void __iomem *msi_mem;
> phys_addr_t msi_mem_phys;
> + void __iomem *intx_mem;
> + phys_addr_t intx_mem_phys;
> struct pci_epf_bar *epf_bar[PCI_STD_NUM_BARS];
> + bool intx_by_atu;
> };
>
> struct dw_pcie_ops {
> @@ -419,7 +428,8 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci);
> int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> u64 cpu_addr, u64 pci_addr, u64 size);
> int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> - int type, u64 cpu_addr, u64 pci_addr, u64 size);
> + int type, u8 code, u8 routing, u64 cpu_addr,
> + u64 pci_addr, u64 size);
> int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int type,
> u64 cpu_addr, u64 pci_addr, u64 size);
> int dw_pcie_prog_ep_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread* RE: [PATCH v9 5/8] PCI: dwc: Add support for triggering legacy IRQs
2023-02-15 15:19 ` Serge Semin
@ 2023-02-16 9:56 ` Yoshihiro Shimoda
0 siblings, 0 replies; 20+ messages in thread
From: Yoshihiro Shimoda @ 2023-02-16 9:56 UTC (permalink / raw)
To: Serge Semin, Rob Herring, Bjorn Helgaas
Cc: lpieralisi@kernel.org, robh+dt@kernel.org, kw@linux.com,
bhelgaas@google.com, jingoohan1@gmail.com,
gustavo.pimentel@synopsys.com, Sergey.Semin@baikalelectronics.ru,
marek.vasut+renesas@gmail.com, linux-pci@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org
Hi Serge,
> From: Serge Semin, Sent: Thursday, February 16, 2023 12:19 AM
>
> On Fri, Feb 10, 2023 at 10:49:14PM +0900, Yoshihiro Shimoda wrote:
> > Add support for triggering legacy IRQs by using outbound ATU.
>
> I supposed a little more details would nice, like outbound iATU is
> utilized to send assert and de-assert INTx TLPs. The message is
> generated based on the payloadless Msg TLP with type 0x14, where 0x4
> is the routing code implying the terminated at Receiver message. The
> message code is specified as b1000xx for the INTx assertion and
> b1001xx for the INTx de-assertion. Etc...
Thank you for your comments. The descriptions are very nice to me.
So, I'll add the descriptions.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> > .../pci/controller/dwc/pcie-designware-ep.c | 66 +++++++++++++++++--
> > drivers/pci/controller/dwc/pcie-designware.c | 25 ++++---
> > drivers/pci/controller/dwc/pcie-designware.h | 12 +++-
> > 3 files changed, 87 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index 95efe14f1036..886483bf378b 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -6,6 +6,7 @@
> > * Author: Kishon Vijay Abraham I <kishon@ti.com>
> > */
> >
> > +#include <linux/delay.h>
> > #include <linux/of.h>
> > #include <linux/platform_device.h>
> >
> > @@ -182,8 +183,8 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
> > return 0;
> > }
> >
> > -static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
> > - phys_addr_t phys_addr,
> > +static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
> > + u8 code, u8 routing, phys_addr_t phys_addr,
> > u64 pci_addr, size_t size)
> > {
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > @@ -196,8 +197,9 @@ static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
> > return -EINVAL;
> > }
> >
> > - ret = dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win, PCIE_ATU_TYPE_MEM,
> > - phys_addr, pci_addr, size);
> > + ret = dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win, type,
> > + code, routing, phys_addr, pci_addr,
> > + size);
> > if (ret)
> > return ret;
> >
> > @@ -306,7 +308,8 @@ static int dw_pcie_ep_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> >
> > - ret = dw_pcie_ep_outbound_atu(ep, func_no, addr, pci_addr, size);
> > + ret = dw_pcie_ep_outbound_atu(ep, func_no, PCIE_ATU_TYPE_MEM, 0, 0,
> > + addr, pci_addr, size);
> > if (ret) {
> > dev_err(pci->dev, "Failed to enable address\n");
> > return ret;
> > @@ -479,11 +482,43 @@ static const struct pci_epc_ops epc_ops = {
> > .get_features = dw_pcie_ep_get_features,
> > };
> >
> > +static int dw_pcie_ep_send_msg(struct dw_pcie_ep *ep, u8 func_no, u8 code,
> > + u8 routing)
> > +{
> > + struct pci_epc *epc = ep->epc;
> > + int ret;
> > +
> > + ret = dw_pcie_ep_outbound_atu(ep, func_no, PCIE_ATU_TYPE_MSG, code,
> > + routing, ep->intx_mem_phys, 0,
> > + epc->mem->window.page_size);
> > + if (ret)
> > + return ret;
> > + writel(0, ep->intx_mem);
> > + dw_pcie_ep_unmap_addr(epc, func_no, 0, ep->intx_mem_phys);
> > +
> > + return 0;
> > +}
> > +
> > +static int __dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no,
> > + int intx)
> > +{
> > + int ret;
> > +
> > + ret = dw_pcie_ep_send_msg(ep, func_no, PCIE_MSG_ASSERT_INTA + intx, 0x04);
> > + if (ret)
> > + return ret;
> > + usleep_range(1000, 2000);
> > + return dw_pcie_ep_send_msg(ep, func_no, PCIE_MSG_DEASSERT_INTA + intx, 0x04);
> > +}
> > +
> > int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no)
> > {
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > struct device *dev = pci->dev;
> >
>
> > + if (ep->intx_by_atu)
>
> IMO the flag is redundant. Your implementation is generic enough to be
> useful for all the DW PCIe EPs. If you are afraid to break things,
Yes, I'm afraid to break things.
> then just make it optional. If no outbound physical memory could be
> allocated then initialize intx_mem with NULL and move on with further
> initializations. As before the Legacy IRQ raise method shall return
> EINVAL in that case.
I got it. I'll modify this patch like that.
> > + return __dw_pcie_ep_raise_legacy_irq(ep, func_no, 0);
> > +
> > dev_err(dev, "EP cannot trigger legacy IRQs\n");
> >
> > return -EINVAL;
> > @@ -617,6 +652,10 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
> >
> > dw_pcie_edma_remove(pci);
> >
> > + if (ep->intx_by_atu)
> > + pci_epc_mem_free_addr(epc, ep->intx_mem_phys, ep->intx_mem,
> > + epc->mem->window.page_size);
> > +
> > pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
> > epc->mem->window.page_size);
> >
> > @@ -789,9 +828,19 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> > goto err_exit_epc_mem;
> > }
> >
>
> > + if (ep->intx_by_atu) {
> > + ep->intx_mem = pci_epc_mem_alloc_addr(epc, &ep->intx_mem_phys,
> > + epc->mem->window.page_size);
> > + if (!ep->intx_mem) {
> > + ret = -ENOMEM;
> > + dev_err(dev, "Failed to reserve memory for INTx\n");
>
> As I suggested above you can just dev_warn() here and move on with
> EP initialization.
I got it.
> > + goto err_free_epc_mem;
> > + }
> > + }
> > +
> > ret = dw_pcie_edma_detect(pci);
> > if (ret)
> > - goto err_free_epc_mem;
> > + goto err_free_epc_mem_intx;
> >
> > if (ep->ops->get_features) {
> > epc_features = ep->ops->get_features(ep);
> > @@ -808,6 +857,11 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> > err_remove_edma:
> > dw_pcie_edma_remove(pci);
> >
> > +err_free_epc_mem_intx:
> > + if (ep->intx_by_atu)
> > + pci_epc_mem_free_addr(epc, ep->intx_mem_phys, ep->intx_mem,
> > + epc->mem->window.page_size);
> > +
> > err_free_epc_mem:
> > pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
> > epc->mem->window.page_size);
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 53a16b8b6ac2..b4315cf84340 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -465,8 +465,8 @@ static inline u32 dw_pcie_enable_ecrc(u32 val)
> > }
> >
>
> > static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
> > - int index, int type, u64 cpu_addr,
> > - u64 pci_addr, u64 size)
> > + int index, int type, u8 code, u8 routing,
> > + u64 cpu_addr, u64 pci_addr, u64 size)
>
> The implementation gets to be too complicated especially with having
> nine arguments now. Seven args have been not that good either, but at
> the very least it was more-or-less coherent with respect to the Mem/IO
> outbound TLPs generations. Now in addition to that it will be also
> responsible for the MSG TLPs mapping. What we could simplify here is:
>
> < either 1. Drop separate routing arg. It can be merged into the type
> argument seeing it's a part of one by specification. Thus we'll have
> only eight arguments left. That will simplify the prototype, but not
> the implementation.
>
> < or 2. Split up the outbound mem/IO and MSG windows setups.
> In the later case you'll need only the next data for the MSG TLPs
> mapping: function #, MW index, message code, CPU base address, MW size.
>
> < or 3. Convert __dw_pcie_prog_outbound_atu() to accepting a
> dw_pcie_outbound_atu(-ish) structure with all the arguments listed as
> fields: MW index, function #, message type, routing type, message
> code (valid if MSG type specified), CPU base address, PCIe base
> address, MW size.
>
> What do you think? @Rob, @Bjorn?
I don't have a strong opinion, but I prefer the 3 above.
> (Please don't forget to define the macros for the routing and message
> code values.)
>
> > {
> > u32 retries, val;
> > u64 limit_addr;
> > @@ -498,7 +498,7 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
> > dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_UPPER_TARGET,
> > upper_32_bits(pci_addr));
> >
> > - val = type | PCIE_ATU_FUNC_NUM(func_no);
> > + val = type | routing | PCIE_ATU_FUNC_NUM(func_no);
> > if (upper_32_bits(limit_addr) > upper_32_bits(cpu_addr) &&
> > dw_pcie_ver_is_ge(pci, 460A))
> > val |= PCIE_ATU_INCREASE_REGION_SIZE;
> > @@ -506,7 +506,14 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
> > val = dw_pcie_enable_ecrc(val);
> > dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL1, val);
> >
> > - dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2, PCIE_ATU_ENABLE);
>
> > + if (code)
> > + dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2,
> > + PCIE_ATU_ENABLE |
> > + PCIE_ATU_INHIBIT_PAYLOAD |
> > + PCIE_ATU_HEADER_SUB_ENABLE | code);
>
> AFAICS the setup above is only specific to the Msg TLPs. If so then it
> would have been more generic to use if(type == ?) conditional
> statement here. What do you think?
>
> > + else
> > + dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2,
> > + PCIE_ATU_ENABLE);
>
> The next modification seems to be looking better in this case:
> + val = PCIE_ATU_ENABLE;
> + if (type == PCIE_ATU_TYPE_MSG)
> + val |= PCIE_ATU_INHIBIT_PAYLOAD | PCIE_ATU_HEADER_SUB_ENABLE | code;
> +
> + dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2, val);
Thank you for the suggestion. It looks better than my patch.
So, I'll modify it.
Best regards,
Yoshihiro Shimoda
> -Serge(y)
>
> >
> > /*
> > * Make sure ATU enable takes effect before any subsequent config
> > @@ -528,16 +535,16 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
> > int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> > u64 cpu_addr, u64 pci_addr, u64 size)
> > {
> > - return __dw_pcie_prog_outbound_atu(pci, 0, index, type,
> > + return __dw_pcie_prog_outbound_atu(pci, 0, index, type, 0, 0,
> > cpu_addr, pci_addr, size);
> > }
> >
> > int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> > - int type, u64 cpu_addr, u64 pci_addr,
> > - u64 size)
> > + int type, u8 code, u8 routing, u64 cpu_addr,
> > + u64 pci_addr, u64 size)
> > {
> > - return __dw_pcie_prog_outbound_atu(pci, func_no, index, type,
> > - cpu_addr, pci_addr, size);
> > + return __dw_pcie_prog_outbound_atu(pci, func_no, index, type, code,
> > + routing, cpu_addr, pci_addr, size);
> > }
> >
> > static inline u32 dw_pcie_readl_atu_ib(struct dw_pcie *pci, u32 index, u32 reg)
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index 79713ce075cc..1a6e7e9489ee 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -147,11 +147,14 @@
> > #define PCIE_ATU_TYPE_IO 0x2
> > #define PCIE_ATU_TYPE_CFG0 0x4
> > #define PCIE_ATU_TYPE_CFG1 0x5
> > +#define PCIE_ATU_TYPE_MSG 0x10
> > #define PCIE_ATU_TD BIT(8)
> > #define PCIE_ATU_FUNC_NUM(pf) ((pf) << 20)
> > #define PCIE_ATU_REGION_CTRL2 0x004
> > #define PCIE_ATU_ENABLE BIT(31)
> > #define PCIE_ATU_BAR_MODE_ENABLE BIT(30)
> > +#define PCIE_ATU_INHIBIT_PAYLOAD BIT(22)
> > +#define PCIE_ATU_HEADER_SUB_ENABLE BIT(21)
> > #define PCIE_ATU_FUNC_NUM_MATCH_EN BIT(19)
> > #define PCIE_ATU_LOWER_BASE 0x008
> > #define PCIE_ATU_UPPER_BASE 0x00C
> > @@ -244,6 +247,9 @@
> > /* Default eDMA LLP memory size */
> > #define DMA_LLP_MEM_SIZE PAGE_SIZE
> >
> > +#define PCIE_MSG_ASSERT_INTA 0x20
> > +#define PCIE_MSG_DEASSERT_INTA 0x24
> > +
> > struct dw_pcie;
> > struct dw_pcie_rp;
> > struct dw_pcie_ep;
> > @@ -352,7 +358,10 @@ struct dw_pcie_ep {
> > unsigned long *ob_window_map;
> > void __iomem *msi_mem;
> > phys_addr_t msi_mem_phys;
> > + void __iomem *intx_mem;
> > + phys_addr_t intx_mem_phys;
> > struct pci_epf_bar *epf_bar[PCI_STD_NUM_BARS];
> > + bool intx_by_atu;
> > };
> >
> > struct dw_pcie_ops {
> > @@ -419,7 +428,8 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci);
> > int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> > u64 cpu_addr, u64 pci_addr, u64 size);
> > int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> > - int type, u64 cpu_addr, u64 pci_addr, u64 size);
> > + int type, u8 code, u8 routing, u64 cpu_addr,
> > + u64 pci_addr, u64 size);
> > int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int type,
> > u64 cpu_addr, u64 pci_addr, u64 size);
> > int dw_pcie_prog_ep_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> > --
> > 2.25.1
> >
> >
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v9 6/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support
2023-02-10 13:49 [PATCH v9 0/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
` (4 preceding siblings ...)
2023-02-10 13:49 ` [PATCH v9 5/8] PCI: dwc: Add support for triggering legacy IRQs Yoshihiro Shimoda
@ 2023-02-10 13:49 ` Yoshihiro Shimoda
2023-02-10 17:22 ` Rob Herring
` (2 more replies)
2023-02-10 13:49 ` [PATCH v9 7/8] PCI: rcar-gen4-ep: Add R-Car Gen4 PCIe Endpoint support Yoshihiro Shimoda
2023-02-10 13:49 ` [PATCH v9 8/8] MAINTAINERS: Update PCI DRIVER FOR RENESAS R-CAR for R-Car Gen4 Yoshihiro Shimoda
7 siblings, 3 replies; 20+ messages in thread
From: Yoshihiro Shimoda @ 2023-02-10 13:49 UTC (permalink / raw)
To: lpieralisi, robh+dt, kw, bhelgaas, jingoohan1, gustavo.pimentel
Cc: Sergey.Semin, marek.vasut+renesas, linux-pci, devicetree,
linux-renesas-soc, Yoshihiro Shimoda
Add R-Car Gen4 PCIe Host support. This controller is based on
Synopsys DesignWare PCIe.
This controller doesn't support MSI-X interrupt. So, introduce
no_msix flag in struct dw_pcie_host_ops to clear MSI_FLAG_PCI_MSIX
from dw_pcie_msi_domain_info.
Note that this controller on R-Car S4-8 has an unexpected register
value on the dbi+0x97b register. So, add a new capability flag
which would force the unrolled eDMA mapping for the problematic
device, as suggested by Serge Semin.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
drivers/pci/controller/dwc/Kconfig | 9 +
drivers/pci/controller/dwc/Makefile | 2 +
.../pci/controller/dwc/pcie-designware-host.c | 3 +
drivers/pci/controller/dwc/pcie-designware.c | 8 +-
drivers/pci/controller/dwc/pcie-designware.h | 6 +-
.../pci/controller/dwc/pcie-rcar-gen4-host.c | 165 +++++++++++++++++
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 166 ++++++++++++++++++
drivers/pci/controller/dwc/pcie-rcar-gen4.h | 63 +++++++
8 files changed, 419 insertions(+), 3 deletions(-)
create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.c
create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.h
diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 434f6a4f4041..94805ec31a8f 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -414,4 +414,13 @@ config PCIE_FU740
Say Y here if you want PCIe controller support for the SiFive
FU740.
+config PCIE_RCAR_GEN4
+ tristate "Renesas R-Car Gen4 PCIe Host controller"
+ depends on ARCH_RENESAS || COMPILE_TEST
+ depends on PCI_MSI
+ select PCIE_DW_HOST
+ help
+ Say Y here if you want PCIe host controller support on R-Car Gen4 SoCs.
+ This uses the DesignWare core.
+
endmenu
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index bf5c311875a1..486cf706b53d 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -26,6 +26,8 @@ obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
+pcie-rcar-gen4-host-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-host.o
+obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4-host-drv.o
# The following drivers are for devices that use the generic ACPI
# pci_root.c driver but don't support standard ECAM config access.
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 9952057c8819..5aefeec15c9b 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -242,6 +242,9 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
irq_domain_update_bus_token(pp->irq_domain, DOMAIN_BUS_NEXUS);
+ if (pp->no_msix)
+ dw_pcie_msi_domain_info.flags &= ~MSI_FLAG_PCI_MSIX;
+
pp->msi_domain = pci_msi_create_irq_domain(fwnode,
&dw_pcie_msi_domain_info,
pp->irq_domain);
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index b4315cf84340..7d649ba387f8 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -847,8 +847,14 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
* Indirect eDMA CSRs access has been completely removed since v5.40a
* thus no space is now reserved for the eDMA channels viewport and
* former DMA CTRL register is no longer fixed to FFs.
+ *
+ * Note some devices for unknown reason may have zeros in the eDMA CTRL
+ * register even though the HW-manual explicitly states there must FFs
+ * if the unrolled mapping is enabled. For such cases the low-level
+ * drivers are supposed to manually activate the unrolled mapping to
+ * bypass the auto-detection procedure.
*/
- if (dw_pcie_ver_is_ge(pci, 540A))
+ if (dw_pcie_ver_is_ge(pci, 540A) || dw_pcie_cap_is(pci, EDMA_UNROLL))
val = 0xFFFFFFFF;
else
val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 1a6e7e9489ee..1b1af514b849 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -51,8 +51,9 @@
/* DWC PCIe controller capabilities */
#define DW_PCIE_CAP_REQ_RES 0
-#define DW_PCIE_CAP_IATU_UNROLL 1
-#define DW_PCIE_CAP_CDM_CHECK 2
+#define DW_PCIE_CAP_EDMA_UNROLL 1
+#define DW_PCIE_CAP_IATU_UNROLL 2
+#define DW_PCIE_CAP_CDM_CHECK 3
#define dw_pcie_cap_is(_pci, _cap) \
test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
@@ -303,6 +304,7 @@ struct dw_pcie_host_ops {
struct dw_pcie_rp {
bool has_msi_ctrl:1;
bool cfg0_io_shared:1;
+ bool no_msix:1;
u64 cfg0_base;
void __iomem *va_cfg0_base;
u32 cfg0_size;
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
new file mode 100644
index 000000000000..d60f4895ffe9
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
@@ -0,0 +1,165 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PCIe host controller driver for Renesas R-Car Gen4 Series SoCs
+ * Copyright (C) 2022-2023 Renesas Electronics Corporation
+ */
+
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+
+#include "pcie-rcar-gen4.h"
+#include "pcie-designware.h"
+
+static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
+{
+ struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
+ struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
+ int ret;
+ u32 val;
+
+ ret = rcar_gen4_pcie_set_device_type(rcar, true, dw->num_lanes);
+ if (ret < 0)
+ return ret;
+
+ dw_pcie_dbi_ro_wr_en(dw);
+
+ rcar_gen4_pcie_disable_bar(dw, BAR0MASKF);
+ rcar_gen4_pcie_disable_bar(dw, BAR1MASKF);
+
+ dw_pcie_dbi_ro_wr_dis(dw);
+
+ if (IS_ENABLED(CONFIG_PCI_MSI)) {
+ /* Enable MSI interrupt signal */
+ val = readl(rcar->base + PCIEINTSTS0EN);
+ val |= MSI_CTRL_INT;
+ writel(val, rcar->base + PCIEINTSTS0EN);
+ }
+
+ gpiod_set_value_cansleep(dw->pe_rst, 0);
+
+ dw_pcie_setup_rc(pp);
+
+ dw_pcie_dbi_ro_wr_en(dw);
+ rcar_gen4_pcie_set_max_link_width(dw, dw->num_lanes);
+ dw_pcie_dbi_ro_wr_dis(dw);
+
+ if (!dw_pcie_link_up(dw)) {
+ ret = dw->ops->start_link(dw);
+ if (ret)
+ return ret;
+ }
+
+ /* Ignore errors, the link may come up later */
+ if (dw_pcie_wait_for_link(dw))
+ dev_info(dw->dev, "PCIe link down\n");
+
+ return 0;
+}
+
+static const struct dw_pcie_host_ops rcar_gen4_pcie_host_ops = {
+ .host_init = rcar_gen4_pcie_host_init,
+};
+
+static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar,
+ struct platform_device *pdev)
+{
+ struct dw_pcie *dw = &rcar->dw;
+ struct dw_pcie_rp *pp = &dw->pp;
+ int ret;
+
+ pp->ops = &rcar_gen4_pcie_host_ops;
+ pp->no_msix = true;
+ dw_pcie_cap_set(dw, REQ_RES);
+
+ ret = dw_pcie_host_init(pp);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to initialize host\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
+{
+ dw_pcie_host_deinit(&rcar->dw.pp);
+ gpiod_set_value_cansleep(rcar->dw.pe_rst, 1);
+}
+
+static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar,
+ struct platform_device *pdev)
+{
+ struct dw_pcie *dw = &rcar->dw;
+
+ /* Renesas-specific registers */
+ rcar->base = devm_platform_ioremap_resource_byname(pdev, "app");
+ if (IS_ERR(rcar->base))
+ return PTR_ERR(rcar->base);
+
+ return rcar_gen4_pcie_devm_reset_get(rcar, dw->dev);
+}
+
+static int rcar_gen4_pcie_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct rcar_gen4_pcie *rcar;
+ int err;
+
+ rcar = rcar_gen4_pcie_devm_alloc(dev);
+ if (!rcar)
+ return -ENOMEM;
+
+ err = rcar_gen4_pcie_get_resources(rcar, pdev);
+ if (err < 0) {
+ dev_err(dev, "Failed to request resource: %d\n", err);
+ return err;
+ }
+
+ platform_set_drvdata(pdev, rcar);
+
+ err = rcar_gen4_pcie_prepare(rcar);
+ if (err < 0)
+ return err;
+
+ err = rcar_gen4_add_dw_pcie_rp(rcar, pdev);
+ if (err < 0)
+ goto err_add;
+
+ return 0;
+
+err_add:
+ rcar_gen4_pcie_unprepare(rcar);
+
+ return err;
+}
+
+static int rcar_gen4_pcie_remove(struct platform_device *pdev)
+{
+ struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
+
+ rcar_gen4_remove_dw_pcie_rp(rcar);
+ rcar_gen4_pcie_unprepare(rcar);
+
+ return 0;
+}
+
+static const struct of_device_id rcar_gen4_pcie_of_match[] = {
+ { .compatible = "renesas,rcar-gen4-pcie", },
+ {},
+};
+
+static struct platform_driver rcar_gen4_pcie_driver = {
+ .driver = {
+ .name = "pcie-rcar-gen4",
+ .of_match_table = rcar_gen4_pcie_of_match,
+ },
+ .probe = rcar_gen4_pcie_probe,
+ .remove = rcar_gen4_pcie_remove,
+};
+module_platform_driver(rcar_gen4_pcie_driver);
+
+MODULE_DESCRIPTION("Renesas R-Car Gen4 PCIe host controller driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
new file mode 100644
index 000000000000..a6a29d6125c8
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -0,0 +1,166 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
+ * Copyright (C) 2022-2023 Renesas Electronics Corporation
+ */
+
+#include <linux/io.h>
+#include <linux/of_device.h>
+#include <linux/pci.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+
+#include "pcie-rcar-gen4.h"
+#include "pcie-designware.h"
+
+/* Renesas-specific */
+#define PCIERSTCTRL1 0x0014
+#define APP_HOLD_PHY_RST BIT(16)
+#define APP_LTSSM_ENABLE BIT(0)
+
+static void rcar_gen4_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar,
+ bool enable)
+{
+ u32 val;
+
+ val = readl(rcar->base + PCIERSTCTRL1);
+ if (enable) {
+ val |= APP_LTSSM_ENABLE;
+ val &= ~APP_HOLD_PHY_RST;
+ } else {
+ val &= ~APP_LTSSM_ENABLE;
+ val |= APP_HOLD_PHY_RST;
+ }
+ writel(val, rcar->base + PCIERSTCTRL1);
+}
+
+static int rcar_gen4_pcie_link_up(struct dw_pcie *dw)
+{
+ struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
+ u32 val, mask;
+
+ val = readl(rcar->base + PCIEINTSTS0);
+ mask = RDLH_LINK_UP | SMLH_LINK_UP;
+
+ return (val & mask) == mask;
+}
+
+static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
+{
+ struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
+
+ rcar_gen4_pcie_ltssm_enable(rcar, true);
+
+ return 0;
+}
+
+static void rcar_gen4_pcie_stop_link(struct dw_pcie *dw)
+{
+ struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
+
+ rcar_gen4_pcie_ltssm_enable(rcar, false);
+}
+
+int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
+ int num_lanes)
+{
+ u32 val;
+
+ /* Note: Assume the reset is asserted here */
+ val = readl(rcar->base + PCIEMSR0);
+ if (rc)
+ val |= DEVICE_TYPE_RC;
+ else
+ val |= DEVICE_TYPE_EP;
+ if (num_lanes < 4)
+ val |= BIFUR_MOD_SET_ON;
+ writel(val, rcar->base + PCIEMSR0);
+
+ return reset_control_deassert(rcar->rst);
+}
+
+void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg)
+{
+ dw_pcie_writel_dbi(dw, SHADOW_REG(bar_mask_reg), 0x0);
+}
+
+void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *dw, int num_lanes)
+{
+ u32 val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_LNKCAP));
+
+ val &= ~PCI_EXP_LNKCAP_MLW;
+ switch (num_lanes) {
+ case 1:
+ val |= PCI_EXP_LNKCAP_MLW_X1;
+ break;
+ case 2:
+ val |= PCI_EXP_LNKCAP_MLW_X2;
+ break;
+ case 4:
+ val |= PCI_EXP_LNKCAP_MLW_X4;
+ break;
+ default:
+ dev_info(dw->dev, "Invalid num-lanes %d\n", num_lanes);
+ val |= PCI_EXP_LNKCAP_MLW_X1;
+ break;
+ }
+ dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_LNKCAP), val);
+}
+
+int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *rcar)
+{
+ struct device *dev = rcar->dw.dev;
+ int err;
+
+ pm_runtime_enable(dev);
+ err = pm_runtime_resume_and_get(dev);
+ if (err < 0) {
+ dev_err(dev, "Failed to resume/get Runtime PM\n");
+ pm_runtime_disable(dev);
+ }
+
+ return err;
+}
+
+void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
+{
+ struct device *dev = rcar->dw.dev;
+
+ if (!reset_control_status(rcar->rst))
+ reset_control_assert(rcar->rst);
+ pm_runtime_put(dev);
+ pm_runtime_disable(dev);
+}
+
+int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *rcar,
+ struct device *dev)
+{
+ rcar->rst = devm_reset_control_get(dev, NULL);
+ if (IS_ERR(rcar->rst)) {
+ dev_err(dev, "Failed to get Cold-reset\n");
+ return PTR_ERR(rcar->rst);
+ }
+
+ return 0;
+}
+
+static const struct dw_pcie_ops dw_pcie_ops = {
+ .start_link = rcar_gen4_pcie_start_link,
+ .stop_link = rcar_gen4_pcie_stop_link,
+ .link_up = rcar_gen4_pcie_link_up,
+};
+
+struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev)
+{
+ struct rcar_gen4_pcie *rcar;
+
+ rcar = devm_kzalloc(dev, sizeof(*rcar), GFP_KERNEL);
+ if (!rcar)
+ return NULL;
+
+ rcar->dw.dev = dev;
+ rcar->dw.ops = &dw_pcie_ops;
+ dw_pcie_cap_set(&rcar->dw, EDMA_UNROLL);
+
+ return rcar;
+}
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.h b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
new file mode 100644
index 000000000000..1cdce0cf7dac
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
@@ -0,0 +1,63 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
+ * Copyright (C) 2022-2023 Renesas Electronics Corporation
+ */
+
+#ifndef _PCIE_RCAR_GEN4_H_
+#define _PCIE_RCAR_GEN4_H_
+
+#include <linux/io.h>
+#include <linux/pci.h>
+#include <linux/reset.h>
+
+#include "pcie-designware.h"
+
+/* PCI Express capability */
+#define EXPCAP(x) (0x0070 + (x))
+/* ASPM L1 PM Substates */
+#define L1PSCAP(x) (0x01bc + (x))
+/* PCI Shadow offset */
+#define SHADOW_REG(x) (0x2000 + (x))
+/* BAR Mask registers */
+#define BAR0MASKF 0x0010
+#define BAR1MASKF 0x0014
+#define BAR2MASKF 0x0018
+#define BAR3MASKF 0x001c
+#define BAR4MASKF 0x0020
+#define BAR5MASKF 0x0024
+
+/* Renesas-specific */
+#define PCIEMSR0 0x0000
+#define BIFUR_MOD_SET_ON BIT(0)
+#define DEVICE_TYPE_EP 0
+#define DEVICE_TYPE_RC BIT(4)
+
+#define PCIEINTSTS0 0x0084
+#define PCIEINTSTS0EN 0x0310
+#define MSI_CTRL_INT BIT(26)
+#define SMLH_LINK_UP BIT(7)
+#define RDLH_LINK_UP BIT(6)
+#define PCIEDMAINTSTSEN 0x0314
+#define PCIEDMAINTSTSEN_INIT GENMASK(15, 0)
+
+struct rcar_gen4_pcie {
+ struct dw_pcie dw;
+ void __iomem *base;
+ struct reset_control *rst;
+};
+#define to_rcar_gen4_pcie(x) dev_get_drvdata((x)->dev)
+
+u32 rcar_gen4_pcie_readl(struct rcar_gen4_pcie *pcie, u32 reg);
+void rcar_gen4_pcie_writel(struct rcar_gen4_pcie *pcie, u32 reg, u32 val);
+int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
+ int num_lanes);
+void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg);
+void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *pci, int num_lanes);
+int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *pcie);
+void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *pcie);
+int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *pcie,
+ struct device *dev);
+struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev);
+
+#endif /* _PCIE_RCAR_GEN4_H_ */
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v9 6/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support
2023-02-10 13:49 ` [PATCH v9 6/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support Yoshihiro Shimoda
@ 2023-02-10 17:22 ` Rob Herring
2023-02-13 2:25 ` Yoshihiro Shimoda
2023-02-10 22:31 ` Bjorn Helgaas
2023-02-15 18:33 ` Serge Semin
2 siblings, 1 reply; 20+ messages in thread
From: Rob Herring @ 2023-02-10 17:22 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: lpieralisi, kw, bhelgaas, jingoohan1, gustavo.pimentel,
Sergey.Semin, marek.vasut+renesas, linux-pci, devicetree,
linux-renesas-soc
On Fri, Feb 10, 2023 at 7:49 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
>
> Add R-Car Gen4 PCIe Host support. This controller is based on
> Synopsys DesignWare PCIe.
>
> This controller doesn't support MSI-X interrupt. So, introduce
> no_msix flag in struct dw_pcie_host_ops to clear MSI_FLAG_PCI_MSIX
> from dw_pcie_msi_domain_info.
>
> Note that this controller on R-Car S4-8 has an unexpected register
> value on the dbi+0x97b register. So, add a new capability flag
> which would force the unrolled eDMA mapping for the problematic
> device, as suggested by Serge Semin.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
> drivers/pci/controller/dwc/Kconfig | 9 +
> drivers/pci/controller/dwc/Makefile | 2 +
> .../pci/controller/dwc/pcie-designware-host.c | 3 +
> drivers/pci/controller/dwc/pcie-designware.c | 8 +-
> drivers/pci/controller/dwc/pcie-designware.h | 6 +-
> .../pci/controller/dwc/pcie-rcar-gen4-host.c | 165 +++++++++++++++++
> drivers/pci/controller/dwc/pcie-rcar-gen4.c | 166 ++++++++++++++++++
> drivers/pci/controller/dwc/pcie-rcar-gen4.h | 63 +++++++
> 8 files changed, 419 insertions(+), 3 deletions(-)
> create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.c
> create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.h
>
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 434f6a4f4041..94805ec31a8f 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -414,4 +414,13 @@ config PCIE_FU740
> Say Y here if you want PCIe controller support for the SiFive
> FU740.
>
> +config PCIE_RCAR_GEN4
> + tristate "Renesas R-Car Gen4 PCIe Host controller"
> + depends on ARCH_RENESAS || COMPILE_TEST
> + depends on PCI_MSI
> + select PCIE_DW_HOST
> + help
> + Say Y here if you want PCIe host controller support on R-Car Gen4 SoCs.
> + This uses the DesignWare core.
> +
> endmenu
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index bf5c311875a1..486cf706b53d 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -26,6 +26,8 @@ obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
> obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
> obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
> obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
> +pcie-rcar-gen4-host-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-host.o
> +obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4-host-drv.o
>
> # The following drivers are for devices that use the generic ACPI
> # pci_root.c driver but don't support standard ECAM config access.
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 9952057c8819..5aefeec15c9b 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -242,6 +242,9 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
>
> irq_domain_update_bus_token(pp->irq_domain, DOMAIN_BUS_NEXUS);
>
> + if (pp->no_msix)
> + dw_pcie_msi_domain_info.flags &= ~MSI_FLAG_PCI_MSIX;
> +
> pp->msi_domain = pci_msi_create_irq_domain(fwnode,
> &dw_pcie_msi_domain_info,
> pp->irq_domain);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index b4315cf84340..7d649ba387f8 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -847,8 +847,14 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> * Indirect eDMA CSRs access has been completely removed since v5.40a
> * thus no space is now reserved for the eDMA channels viewport and
> * former DMA CTRL register is no longer fixed to FFs.
> + *
> + * Note some devices for unknown reason may have zeros in the eDMA CTRL
> + * register even though the HW-manual explicitly states there must FFs
> + * if the unrolled mapping is enabled. For such cases the low-level
> + * drivers are supposed to manually activate the unrolled mapping to
> + * bypass the auto-detection procedure.
> */
> - if (dw_pcie_ver_is_ge(pci, 540A))
> + if (dw_pcie_ver_is_ge(pci, 540A) || dw_pcie_cap_is(pci, EDMA_UNROLL))
> val = 0xFFFFFFFF;
> else
> val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 1a6e7e9489ee..1b1af514b849 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -51,8 +51,9 @@
>
> /* DWC PCIe controller capabilities */
> #define DW_PCIE_CAP_REQ_RES 0
> -#define DW_PCIE_CAP_IATU_UNROLL 1
> -#define DW_PCIE_CAP_CDM_CHECK 2
> +#define DW_PCIE_CAP_EDMA_UNROLL 1
> +#define DW_PCIE_CAP_IATU_UNROLL 2
> +#define DW_PCIE_CAP_CDM_CHECK 3
>
> #define dw_pcie_cap_is(_pci, _cap) \
> test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
> @@ -303,6 +304,7 @@ struct dw_pcie_host_ops {
> struct dw_pcie_rp {
> bool has_msi_ctrl:1;
> bool cfg0_io_shared:1;
> + bool no_msix:1;
> u64 cfg0_base;
> void __iomem *va_cfg0_base;
> u32 cfg0_size;
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> new file mode 100644
> index 000000000000..d60f4895ffe9
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> @@ -0,0 +1,165 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * PCIe host controller driver for Renesas R-Car Gen4 Series SoCs
> + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +
> +#include "pcie-rcar-gen4.h"
> +#include "pcie-designware.h"
> +
> +static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
> +{
> + struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
> + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> + int ret;
> + u32 val;
> +
> + ret = rcar_gen4_pcie_set_device_type(rcar, true, dw->num_lanes);
> + if (ret < 0)
> + return ret;
> +
> + dw_pcie_dbi_ro_wr_en(dw);
> +
> + rcar_gen4_pcie_disable_bar(dw, BAR0MASKF);
> + rcar_gen4_pcie_disable_bar(dw, BAR1MASKF);
> +
> + dw_pcie_dbi_ro_wr_dis(dw);
> +
> + if (IS_ENABLED(CONFIG_PCI_MSI)) {
> + /* Enable MSI interrupt signal */
> + val = readl(rcar->base + PCIEINTSTS0EN);
> + val |= MSI_CTRL_INT;
> + writel(val, rcar->base + PCIEINTSTS0EN);
> + }
> +
> + gpiod_set_value_cansleep(dw->pe_rst, 0);
> +
> + dw_pcie_setup_rc(pp);
> +
> + dw_pcie_dbi_ro_wr_en(dw);
> + rcar_gen4_pcie_set_max_link_width(dw, dw->num_lanes);
> + dw_pcie_dbi_ro_wr_dis(dw);
> +
> + if (!dw_pcie_link_up(dw)) {
> + ret = dw->ops->start_link(dw);
The DW core code does this for you.
> + if (ret)
> + return ret;
> + }
> +
> + /* Ignore errors, the link may come up later */
> + if (dw_pcie_wait_for_link(dw))
> + dev_info(dw->dev, "PCIe link down\n");
Same here.
> +
> + return 0;
> +}
> +
> +static const struct dw_pcie_host_ops rcar_gen4_pcie_host_ops = {
> + .host_init = rcar_gen4_pcie_host_init,
> +};
> +
> +static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar,
> + struct platform_device *pdev)
> +{
> + struct dw_pcie *dw = &rcar->dw;
> + struct dw_pcie_rp *pp = &dw->pp;
> + int ret;
> +
> + pp->ops = &rcar_gen4_pcie_host_ops;
> + pp->no_msix = true;
> + dw_pcie_cap_set(dw, REQ_RES);
> +
> + ret = dw_pcie_host_init(pp);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to initialize host\n");
If this failed, dw_pcie_host_init() should have printed an error. No
reason for every caller to print something.
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
> +{
> + dw_pcie_host_deinit(&rcar->dw.pp);
> + gpiod_set_value_cansleep(rcar->dw.pe_rst, 1);
> +}
> +
> +static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar,
> + struct platform_device *pdev)
> +{
> + struct dw_pcie *dw = &rcar->dw;
> +
> + /* Renesas-specific registers */
> + rcar->base = devm_platform_ioremap_resource_byname(pdev, "app");
> + if (IS_ERR(rcar->base))
> + return PTR_ERR(rcar->base);
> +
> + return rcar_gen4_pcie_devm_reset_get(rcar, dw->dev);
> +}
> +
> +static int rcar_gen4_pcie_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct rcar_gen4_pcie *rcar;
> + int err;
> +
> + rcar = rcar_gen4_pcie_devm_alloc(dev);
> + if (!rcar)
> + return -ENOMEM;
> +
> + err = rcar_gen4_pcie_get_resources(rcar, pdev);
> + if (err < 0) {
> + dev_err(dev, "Failed to request resource: %d\n", err);
> + return err;
> + }
> +
> + platform_set_drvdata(pdev, rcar);
> +
> + err = rcar_gen4_pcie_prepare(rcar);
> + if (err < 0)
> + return err;
> +
> + err = rcar_gen4_add_dw_pcie_rp(rcar, pdev);
> + if (err < 0)
> + goto err_add;
If you didn't add it, then you shouldn't remove it. You are calling
remove on both failed and successful add.
> +
> + return 0;
> +
> +err_add:
> + rcar_gen4_pcie_unprepare(rcar);
> +
> + return err;
> +}
> +
> +static int rcar_gen4_pcie_remove(struct platform_device *pdev)
> +{
> + struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
> +
> + rcar_gen4_remove_dw_pcie_rp(rcar);
> + rcar_gen4_pcie_unprepare(rcar);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id rcar_gen4_pcie_of_match[] = {
> + { .compatible = "renesas,rcar-gen4-pcie", },
> + {},
> +};
> +
> +static struct platform_driver rcar_gen4_pcie_driver = {
> + .driver = {
> + .name = "pcie-rcar-gen4",
> + .of_match_table = rcar_gen4_pcie_of_match,
> + },
> + .probe = rcar_gen4_pcie_probe,
> + .remove = rcar_gen4_pcie_remove,
> +};
> +module_platform_driver(rcar_gen4_pcie_driver);
> +
> +MODULE_DESCRIPTION("Renesas R-Car Gen4 PCIe host controller driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> new file mode 100644
> index 000000000000..a6a29d6125c8
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -0,0 +1,166 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> + */
> +
> +#include <linux/io.h>
> +#include <linux/of_device.h>
> +#include <linux/pci.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> +
> +#include "pcie-rcar-gen4.h"
> +#include "pcie-designware.h"
> +
> +/* Renesas-specific */
> +#define PCIERSTCTRL1 0x0014
> +#define APP_HOLD_PHY_RST BIT(16)
> +#define APP_LTSSM_ENABLE BIT(0)
> +
> +static void rcar_gen4_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar,
> + bool enable)
> +{
> + u32 val;
> +
> + val = readl(rcar->base + PCIERSTCTRL1);
> + if (enable) {
> + val |= APP_LTSSM_ENABLE;
> + val &= ~APP_HOLD_PHY_RST;
> + } else {
> + val &= ~APP_LTSSM_ENABLE;
> + val |= APP_HOLD_PHY_RST;
> + }
> + writel(val, rcar->base + PCIERSTCTRL1);
> +}
> +
> +static int rcar_gen4_pcie_link_up(struct dw_pcie *dw)
> +{
> + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> + u32 val, mask;
> +
> + val = readl(rcar->base + PCIEINTSTS0);
> + mask = RDLH_LINK_UP | SMLH_LINK_UP;
> +
> + return (val & mask) == mask;
> +}
> +
> +static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
> +{
> + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> +
> + rcar_gen4_pcie_ltssm_enable(rcar, true);
> +
> + return 0;
> +}
> +
> +static void rcar_gen4_pcie_stop_link(struct dw_pcie *dw)
> +{
> + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> +
> + rcar_gen4_pcie_ltssm_enable(rcar, false);
> +}
> +
> +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> + int num_lanes)
> +{
> + u32 val;
> +
> + /* Note: Assume the reset is asserted here */
> + val = readl(rcar->base + PCIEMSR0);
> + if (rc)
> + val |= DEVICE_TYPE_RC;
> + else
> + val |= DEVICE_TYPE_EP;
> + if (num_lanes < 4)
> + val |= BIFUR_MOD_SET_ON;
> + writel(val, rcar->base + PCIEMSR0);
> +
> + return reset_control_deassert(rcar->rst);
> +}
> +
> +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg)
> +{
> + dw_pcie_writel_dbi(dw, SHADOW_REG(bar_mask_reg), 0x0);
> +}
> +
> +void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *dw, int num_lanes)
> +{
> + u32 val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_LNKCAP));
> +
> + val &= ~PCI_EXP_LNKCAP_MLW;
> + switch (num_lanes) {
> + case 1:
> + val |= PCI_EXP_LNKCAP_MLW_X1;
> + break;
> + case 2:
> + val |= PCI_EXP_LNKCAP_MLW_X2;
> + break;
> + case 4:
> + val |= PCI_EXP_LNKCAP_MLW_X4;
> + break;
> + default:
> + dev_info(dw->dev, "Invalid num-lanes %d\n", num_lanes);
> + val |= PCI_EXP_LNKCAP_MLW_X1;
> + break;
The register field is just the number of lanes, why not do a macro
that just uses directly instead of essentially a lookup table:
val |= PCI_EXP_LNKCAP_MLW(num_lanes);
You shouldn't need to validate 'num_lanes' here except for a max
value. Though dtschema does that (or should). If you want to validate
it further in the kernel, do that somewhere common so everyone
benefits.
Or better yet, refactor this to be common. We already have Tegra194
doing the same thing.
> + }
> + dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_LNKCAP), val);
> +}
> +
> +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *rcar)
> +{
> + struct device *dev = rcar->dw.dev;
> + int err;
> +
> + pm_runtime_enable(dev);
> + err = pm_runtime_resume_and_get(dev);
> + if (err < 0) {
> + dev_err(dev, "Failed to resume/get Runtime PM\n");
> + pm_runtime_disable(dev);
> + }
> +
> + return err;
> +}
> +
> +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
> +{
> + struct device *dev = rcar->dw.dev;
> +
> + if (!reset_control_status(rcar->rst))
> + reset_control_assert(rcar->rst);
> + pm_runtime_put(dev);
> + pm_runtime_disable(dev);
> +}
> +
> +int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *rcar,
> + struct device *dev)
> +{
> + rcar->rst = devm_reset_control_get(dev, NULL);
> + if (IS_ERR(rcar->rst)) {
> + dev_err(dev, "Failed to get Cold-reset\n");
> + return PTR_ERR(rcar->rst);
> + }
> +
> + return 0;
> +}
> +
> +static const struct dw_pcie_ops dw_pcie_ops = {
> + .start_link = rcar_gen4_pcie_start_link,
> + .stop_link = rcar_gen4_pcie_stop_link,
> + .link_up = rcar_gen4_pcie_link_up,
> +};
> +
> +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev)
> +{
> + struct rcar_gen4_pcie *rcar;
> +
> + rcar = devm_kzalloc(dev, sizeof(*rcar), GFP_KERNEL);
> + if (!rcar)
> + return NULL;
> +
> + rcar->dw.dev = dev;
> + rcar->dw.ops = &dw_pcie_ops;
> + dw_pcie_cap_set(&rcar->dw, EDMA_UNROLL);
> +
> + return rcar;
> +}
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.h b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> new file mode 100644
> index 000000000000..1cdce0cf7dac
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> @@ -0,0 +1,63 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> + */
> +
> +#ifndef _PCIE_RCAR_GEN4_H_
> +#define _PCIE_RCAR_GEN4_H_
> +
> +#include <linux/io.h>
> +#include <linux/pci.h>
> +#include <linux/reset.h>
> +
> +#include "pcie-designware.h"
> +
> +/* PCI Express capability */
> +#define EXPCAP(x) (0x0070 + (x))
This offset is discoverable, don't hardcode it.
> +/* ASPM L1 PM Substates */
> +#define L1PSCAP(x) (0x01bc + (x))
> +/* PCI Shadow offset */
> +#define SHADOW_REG(x) (0x2000 + (x))
> +/* BAR Mask registers */
> +#define BAR0MASKF 0x0010
> +#define BAR1MASKF 0x0014
> +#define BAR2MASKF 0x0018
> +#define BAR3MASKF 0x001c
> +#define BAR4MASKF 0x0020
> +#define BAR5MASKF 0x0024
> +
> +/* Renesas-specific */
> +#define PCIEMSR0 0x0000
> +#define BIFUR_MOD_SET_ON BIT(0)
> +#define DEVICE_TYPE_EP 0
> +#define DEVICE_TYPE_RC BIT(4)
> +
> +#define PCIEINTSTS0 0x0084
> +#define PCIEINTSTS0EN 0x0310
> +#define MSI_CTRL_INT BIT(26)
> +#define SMLH_LINK_UP BIT(7)
> +#define RDLH_LINK_UP BIT(6)
> +#define PCIEDMAINTSTSEN 0x0314
> +#define PCIEDMAINTSTSEN_INIT GENMASK(15, 0)
> +
> +struct rcar_gen4_pcie {
> + struct dw_pcie dw;
> + void __iomem *base;
> + struct reset_control *rst;
> +};
> +#define to_rcar_gen4_pcie(x) dev_get_drvdata((x)->dev)
> +
> +u32 rcar_gen4_pcie_readl(struct rcar_gen4_pcie *pcie, u32 reg);
> +void rcar_gen4_pcie_writel(struct rcar_gen4_pcie *pcie, u32 reg, u32 val);
> +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> + int num_lanes);
> +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg);
> +void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *pci, int num_lanes);
> +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *pcie);
> +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *pcie);
> +int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *pcie,
> + struct device *dev);
> +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev);
> +
> +#endif /* _PCIE_RCAR_GEN4_H_ */
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 20+ messages in thread* RE: [PATCH v9 6/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support
2023-02-10 17:22 ` Rob Herring
@ 2023-02-13 2:25 ` Yoshihiro Shimoda
0 siblings, 0 replies; 20+ messages in thread
From: Yoshihiro Shimoda @ 2023-02-13 2:25 UTC (permalink / raw)
To: Rob Herring
Cc: lpieralisi@kernel.org, kw@linux.com, bhelgaas@google.com,
jingoohan1@gmail.com, gustavo.pimentel@synopsys.com,
Sergey.Semin@baikalelectronics.ru, marek.vasut+renesas@gmail.com,
linux-pci@vger.kernel.org, devicetree@vger.kernel.org,
linux-renesas-soc@vger.kernel.org
Hi Rob,
> From: Rob Herring, Sent: Saturday, February 11, 2023 2:22 AM
>
> On Fri, Feb 10, 2023 at 7:49 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> >
> > Add R-Car Gen4 PCIe Host support. This controller is based on
> > Synopsys DesignWare PCIe.
> >
> > This controller doesn't support MSI-X interrupt. So, introduce
> > no_msix flag in struct dw_pcie_host_ops to clear MSI_FLAG_PCI_MSIX
> > from dw_pcie_msi_domain_info.
> >
> > Note that this controller on R-Car S4-8 has an unexpected register
> > value on the dbi+0x97b register. So, add a new capability flag
> > which would force the unrolled eDMA mapping for the problematic
> > device, as suggested by Serge Semin.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> > drivers/pci/controller/dwc/Kconfig | 9 +
> > drivers/pci/controller/dwc/Makefile | 2 +
> > .../pci/controller/dwc/pcie-designware-host.c | 3 +
> > drivers/pci/controller/dwc/pcie-designware.c | 8 +-
> > drivers/pci/controller/dwc/pcie-designware.h | 6 +-
> > .../pci/controller/dwc/pcie-rcar-gen4-host.c | 165 +++++++++++++++++
> > drivers/pci/controller/dwc/pcie-rcar-gen4.c | 166 ++++++++++++++++++
> > drivers/pci/controller/dwc/pcie-rcar-gen4.h | 63 +++++++
> > 8 files changed, 419 insertions(+), 3 deletions(-)
> > create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> > create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.h
> >
> > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > index 434f6a4f4041..94805ec31a8f 100644
> > --- a/drivers/pci/controller/dwc/Kconfig
> > +++ b/drivers/pci/controller/dwc/Kconfig
> > @@ -414,4 +414,13 @@ config PCIE_FU740
> > Say Y here if you want PCIe controller support for the SiFive
> > FU740.
> >
> > +config PCIE_RCAR_GEN4
> > + tristate "Renesas R-Car Gen4 PCIe Host controller"
> > + depends on ARCH_RENESAS || COMPILE_TEST
> > + depends on PCI_MSI
> > + select PCIE_DW_HOST
> > + help
> > + Say Y here if you want PCIe host controller support on R-Car Gen4 SoCs.
> > + This uses the DesignWare core.
> > +
> > endmenu
> > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> > index bf5c311875a1..486cf706b53d 100644
> > --- a/drivers/pci/controller/dwc/Makefile
> > +++ b/drivers/pci/controller/dwc/Makefile
> > @@ -26,6 +26,8 @@ obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
> > obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
> > obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
> > obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
> > +pcie-rcar-gen4-host-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-host.o
> > +obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4-host-drv.o
> >
> > # The following drivers are for devices that use the generic ACPI
> > # pci_root.c driver but don't support standard ECAM config access.
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index 9952057c8819..5aefeec15c9b 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -242,6 +242,9 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
> >
> > irq_domain_update_bus_token(pp->irq_domain, DOMAIN_BUS_NEXUS);
> >
> > + if (pp->no_msix)
> > + dw_pcie_msi_domain_info.flags &= ~MSI_FLAG_PCI_MSIX;
> > +
> > pp->msi_domain = pci_msi_create_irq_domain(fwnode,
> > &dw_pcie_msi_domain_info,
> > pp->irq_domain);
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index b4315cf84340..7d649ba387f8 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -847,8 +847,14 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > * Indirect eDMA CSRs access has been completely removed since v5.40a
> > * thus no space is now reserved for the eDMA channels viewport and
> > * former DMA CTRL register is no longer fixed to FFs.
> > + *
> > + * Note some devices for unknown reason may have zeros in the eDMA CTRL
> > + * register even though the HW-manual explicitly states there must FFs
> > + * if the unrolled mapping is enabled. For such cases the low-level
> > + * drivers are supposed to manually activate the unrolled mapping to
> > + * bypass the auto-detection procedure.
> > */
> > - if (dw_pcie_ver_is_ge(pci, 540A))
> > + if (dw_pcie_ver_is_ge(pci, 540A) || dw_pcie_cap_is(pci, EDMA_UNROLL))
> > val = 0xFFFFFFFF;
> > else
> > val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index 1a6e7e9489ee..1b1af514b849 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -51,8 +51,9 @@
> >
> > /* DWC PCIe controller capabilities */
> > #define DW_PCIE_CAP_REQ_RES 0
> > -#define DW_PCIE_CAP_IATU_UNROLL 1
> > -#define DW_PCIE_CAP_CDM_CHECK 2
> > +#define DW_PCIE_CAP_EDMA_UNROLL 1
> > +#define DW_PCIE_CAP_IATU_UNROLL 2
> > +#define DW_PCIE_CAP_CDM_CHECK 3
> >
> > #define dw_pcie_cap_is(_pci, _cap) \
> > test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
> > @@ -303,6 +304,7 @@ struct dw_pcie_host_ops {
> > struct dw_pcie_rp {
> > bool has_msi_ctrl:1;
> > bool cfg0_io_shared:1;
> > + bool no_msix:1;
> > u64 cfg0_base;
> > void __iomem *va_cfg0_base;
> > u32 cfg0_size;
> > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> > new file mode 100644
> > index 000000000000..d60f4895ffe9
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> > @@ -0,0 +1,165 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * PCIe host controller driver for Renesas R-Car Gen4 Series SoCs
> > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > + */
> > +
> > +#include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +#include <linux/pci.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include "pcie-rcar-gen4.h"
> > +#include "pcie-designware.h"
> > +
> > +static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
> > +{
> > + struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
> > + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > + int ret;
> > + u32 val;
> > +
> > + ret = rcar_gen4_pcie_set_device_type(rcar, true, dw->num_lanes);
> > + if (ret < 0)
> > + return ret;
> > +
> > + dw_pcie_dbi_ro_wr_en(dw);
> > +
> > + rcar_gen4_pcie_disable_bar(dw, BAR0MASKF);
> > + rcar_gen4_pcie_disable_bar(dw, BAR1MASKF);
> > +
> > + dw_pcie_dbi_ro_wr_dis(dw);
> > +
> > + if (IS_ENABLED(CONFIG_PCI_MSI)) {
> > + /* Enable MSI interrupt signal */
> > + val = readl(rcar->base + PCIEINTSTS0EN);
> > + val |= MSI_CTRL_INT;
> > + writel(val, rcar->base + PCIEINTSTS0EN);
> > + }
> > +
> > + gpiod_set_value_cansleep(dw->pe_rst, 0);
> > +
> > + dw_pcie_setup_rc(pp);
> > +
> > + dw_pcie_dbi_ro_wr_en(dw);
> > + rcar_gen4_pcie_set_max_link_width(dw, dw->num_lanes);
> > + dw_pcie_dbi_ro_wr_dis(dw);
> > +
> > + if (!dw_pcie_link_up(dw)) {
> > + ret = dw->ops->start_link(dw);
>
> The DW core code does this for you.
I got it. I'll check whether removing this also works or not.
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + /* Ignore errors, the link may come up later */
> > + if (dw_pcie_wait_for_link(dw))
> > + dev_info(dw->dev, "PCIe link down\n");
>
> Same here.
Same here.
> > +
> > + return 0;
> > +}
> > +
> > +static const struct dw_pcie_host_ops rcar_gen4_pcie_host_ops = {
> > + .host_init = rcar_gen4_pcie_host_init,
> > +};
> > +
> > +static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar,
> > + struct platform_device *pdev)
> > +{
> > + struct dw_pcie *dw = &rcar->dw;
> > + struct dw_pcie_rp *pp = &dw->pp;
> > + int ret;
> > +
> > + pp->ops = &rcar_gen4_pcie_host_ops;
> > + pp->no_msix = true;
> > + dw_pcie_cap_set(dw, REQ_RES);
> > +
> > + ret = dw_pcie_host_init(pp);
> > + if (ret) {
> > + dev_err(&pdev->dev, "Failed to initialize host\n");
>
> If this failed, dw_pcie_host_init() should have printed an error. No
> reason for every caller to print something.
I got it. I will remove it.
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
> > +{
> > + dw_pcie_host_deinit(&rcar->dw.pp);
> > + gpiod_set_value_cansleep(rcar->dw.pe_rst, 1);
> > +}
> > +
> > +static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar,
> > + struct platform_device *pdev)
> > +{
> > + struct dw_pcie *dw = &rcar->dw;
> > +
> > + /* Renesas-specific registers */
> > + rcar->base = devm_platform_ioremap_resource_byname(pdev, "app");
> > + if (IS_ERR(rcar->base))
> > + return PTR_ERR(rcar->base);
> > +
> > + return rcar_gen4_pcie_devm_reset_get(rcar, dw->dev);
> > +}
> > +
> > +static int rcar_gen4_pcie_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct rcar_gen4_pcie *rcar;
> > + int err;
> > +
> > + rcar = rcar_gen4_pcie_devm_alloc(dev);
> > + if (!rcar)
> > + return -ENOMEM;
> > +
> > + err = rcar_gen4_pcie_get_resources(rcar, pdev);
> > + if (err < 0) {
> > + dev_err(dev, "Failed to request resource: %d\n", err);
> > + return err;
> > + }
> > +
> > + platform_set_drvdata(pdev, rcar);
> > +
> > + err = rcar_gen4_pcie_prepare(rcar);
> > + if (err < 0)
> > + return err;
> > +
> > + err = rcar_gen4_add_dw_pcie_rp(rcar, pdev);
> > + if (err < 0)
> > + goto err_add;
>
> If you didn't add it, then you shouldn't remove it. You are calling
> remove on both failed and successful add.
I'm afraid but, I could not understand why/where I'm calling remove on
both failed and successful add. rcar_gen4_remove_dw_pcie_rp(rcar) is
called from rcar_gen4_pcie_remove() only, and IIUC, the .remove() function
is only called after .probe() was successful.
> > +
> > + return 0;
> > +
> > +err_add:
> > + rcar_gen4_pcie_unprepare(rcar);
> > +
> > + return err;
> > +}
> > +
> > +static int rcar_gen4_pcie_remove(struct platform_device *pdev)
> > +{
> > + struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
> > +
> > + rcar_gen4_remove_dw_pcie_rp(rcar);
> > + rcar_gen4_pcie_unprepare(rcar);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id rcar_gen4_pcie_of_match[] = {
> > + { .compatible = "renesas,rcar-gen4-pcie", },
> > + {},
> > +};
> > +
> > +static struct platform_driver rcar_gen4_pcie_driver = {
> > + .driver = {
> > + .name = "pcie-rcar-gen4",
> > + .of_match_table = rcar_gen4_pcie_of_match,
> > + },
> > + .probe = rcar_gen4_pcie_probe,
> > + .remove = rcar_gen4_pcie_remove,
> > +};
> > +module_platform_driver(rcar_gen4_pcie_driver);
> > +
> > +MODULE_DESCRIPTION("Renesas R-Car Gen4 PCIe host controller driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > new file mode 100644
> > index 000000000000..a6a29d6125c8
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > @@ -0,0 +1,166 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > + */
> > +
> > +#include <linux/io.h>
> > +#include <linux/of_device.h>
> > +#include <linux/pci.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/reset.h>
> > +
> > +#include "pcie-rcar-gen4.h"
> > +#include "pcie-designware.h"
> > +
> > +/* Renesas-specific */
> > +#define PCIERSTCTRL1 0x0014
> > +#define APP_HOLD_PHY_RST BIT(16)
> > +#define APP_LTSSM_ENABLE BIT(0)
> > +
> > +static void rcar_gen4_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar,
> > + bool enable)
> > +{
> > + u32 val;
> > +
> > + val = readl(rcar->base + PCIERSTCTRL1);
> > + if (enable) {
> > + val |= APP_LTSSM_ENABLE;
> > + val &= ~APP_HOLD_PHY_RST;
> > + } else {
> > + val &= ~APP_LTSSM_ENABLE;
> > + val |= APP_HOLD_PHY_RST;
> > + }
> > + writel(val, rcar->base + PCIERSTCTRL1);
> > +}
> > +
> > +static int rcar_gen4_pcie_link_up(struct dw_pcie *dw)
> > +{
> > + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > + u32 val, mask;
> > +
> > + val = readl(rcar->base + PCIEINTSTS0);
> > + mask = RDLH_LINK_UP | SMLH_LINK_UP;
> > +
> > + return (val & mask) == mask;
> > +}
> > +
> > +static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
> > +{
> > + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > +
> > + rcar_gen4_pcie_ltssm_enable(rcar, true);
> > +
> > + return 0;
> > +}
> > +
> > +static void rcar_gen4_pcie_stop_link(struct dw_pcie *dw)
> > +{
> > + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > +
> > + rcar_gen4_pcie_ltssm_enable(rcar, false);
> > +}
> > +
> > +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> > + int num_lanes)
> > +{
> > + u32 val;
> > +
> > + /* Note: Assume the reset is asserted here */
> > + val = readl(rcar->base + PCIEMSR0);
> > + if (rc)
> > + val |= DEVICE_TYPE_RC;
> > + else
> > + val |= DEVICE_TYPE_EP;
> > + if (num_lanes < 4)
> > + val |= BIFUR_MOD_SET_ON;
> > + writel(val, rcar->base + PCIEMSR0);
> > +
> > + return reset_control_deassert(rcar->rst);
> > +}
> > +
> > +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg)
> > +{
> > + dw_pcie_writel_dbi(dw, SHADOW_REG(bar_mask_reg), 0x0);
> > +}
> > +
> > +void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *dw, int num_lanes)
> > +{
> > + u32 val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_LNKCAP));
> > +
> > + val &= ~PCI_EXP_LNKCAP_MLW;
> > + switch (num_lanes) {
> > + case 1:
> > + val |= PCI_EXP_LNKCAP_MLW_X1;
> > + break;
> > + case 2:
> > + val |= PCI_EXP_LNKCAP_MLW_X2;
> > + break;
> > + case 4:
> > + val |= PCI_EXP_LNKCAP_MLW_X4;
> > + break;
> > + default:
> > + dev_info(dw->dev, "Invalid num-lanes %d\n", num_lanes);
> > + val |= PCI_EXP_LNKCAP_MLW_X1;
> > + break;
>
> The register field is just the number of lanes, why not do a macro
> that just uses directly instead of essentially a lookup table:
>
> val |= PCI_EXP_LNKCAP_MLW(num_lanes);
I see. I should have assumed I should use PCI_EXP_LNKCAP_MLW_Xxx macros.
> You shouldn't need to validate 'num_lanes' here except for a max
> value. Though dtschema does that (or should). If you want to validate
> it further in the kernel, do that somewhere common so everyone
> benefits.
I got it. I'll revise dt-bindings doc for it.
> Or better yet, refactor this to be common. We already have Tegra194
> doing the same thing.
Thank you for your suggestion. I'll check it.
> > + }
> > + dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_LNKCAP), val);
> > +}
> > +
> > +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *rcar)
> > +{
> > + struct device *dev = rcar->dw.dev;
> > + int err;
> > +
> > + pm_runtime_enable(dev);
> > + err = pm_runtime_resume_and_get(dev);
> > + if (err < 0) {
> > + dev_err(dev, "Failed to resume/get Runtime PM\n");
> > + pm_runtime_disable(dev);
> > + }
> > +
> > + return err;
> > +}
> > +
> > +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
> > +{
> > + struct device *dev = rcar->dw.dev;
> > +
> > + if (!reset_control_status(rcar->rst))
> > + reset_control_assert(rcar->rst);
> > + pm_runtime_put(dev);
> > + pm_runtime_disable(dev);
> > +}
> > +
> > +int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *rcar,
> > + struct device *dev)
> > +{
> > + rcar->rst = devm_reset_control_get(dev, NULL);
> > + if (IS_ERR(rcar->rst)) {
> > + dev_err(dev, "Failed to get Cold-reset\n");
> > + return PTR_ERR(rcar->rst);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static const struct dw_pcie_ops dw_pcie_ops = {
> > + .start_link = rcar_gen4_pcie_start_link,
> > + .stop_link = rcar_gen4_pcie_stop_link,
> > + .link_up = rcar_gen4_pcie_link_up,
> > +};
> > +
> > +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev)
> > +{
> > + struct rcar_gen4_pcie *rcar;
> > +
> > + rcar = devm_kzalloc(dev, sizeof(*rcar), GFP_KERNEL);
> > + if (!rcar)
> > + return NULL;
> > +
> > + rcar->dw.dev = dev;
> > + rcar->dw.ops = &dw_pcie_ops;
> > + dw_pcie_cap_set(&rcar->dw, EDMA_UNROLL);
> > +
> > + return rcar;
> > +}
> > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.h b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> > new file mode 100644
> > index 000000000000..1cdce0cf7dac
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> > @@ -0,0 +1,63 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > + */
> > +
> > +#ifndef _PCIE_RCAR_GEN4_H_
> > +#define _PCIE_RCAR_GEN4_H_
> > +
> > +#include <linux/io.h>
> > +#include <linux/pci.h>
> > +#include <linux/reset.h>
> > +
> > +#include "pcie-designware.h"
> > +
> > +/* PCI Express capability */
> > +#define EXPCAP(x) (0x0070 + (x))
>
> This offset is discoverable, don't hardcode it.
I got it. I'll fix it.
Best regards,
Yoshihiro Shimoda
> > +/* ASPM L1 PM Substates */
> > +#define L1PSCAP(x) (0x01bc + (x))
> > +/* PCI Shadow offset */
> > +#define SHADOW_REG(x) (0x2000 + (x))
> > +/* BAR Mask registers */
> > +#define BAR0MASKF 0x0010
> > +#define BAR1MASKF 0x0014
> > +#define BAR2MASKF 0x0018
> > +#define BAR3MASKF 0x001c
> > +#define BAR4MASKF 0x0020
> > +#define BAR5MASKF 0x0024
> > +
> > +/* Renesas-specific */
> > +#define PCIEMSR0 0x0000
> > +#define BIFUR_MOD_SET_ON BIT(0)
> > +#define DEVICE_TYPE_EP 0
> > +#define DEVICE_TYPE_RC BIT(4)
> > +
> > +#define PCIEINTSTS0 0x0084
> > +#define PCIEINTSTS0EN 0x0310
> > +#define MSI_CTRL_INT BIT(26)
> > +#define SMLH_LINK_UP BIT(7)
> > +#define RDLH_LINK_UP BIT(6)
> > +#define PCIEDMAINTSTSEN 0x0314
> > +#define PCIEDMAINTSTSEN_INIT GENMASK(15, 0)
> > +
> > +struct rcar_gen4_pcie {
> > + struct dw_pcie dw;
> > + void __iomem *base;
> > + struct reset_control *rst;
> > +};
> > +#define to_rcar_gen4_pcie(x) dev_get_drvdata((x)->dev)
> > +
> > +u32 rcar_gen4_pcie_readl(struct rcar_gen4_pcie *pcie, u32 reg);
> > +void rcar_gen4_pcie_writel(struct rcar_gen4_pcie *pcie, u32 reg, u32 val);
> > +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> > + int num_lanes);
> > +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg);
> > +void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *pci, int num_lanes);
> > +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *pcie);
> > +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *pcie);
> > +int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *pcie,
> > + struct device *dev);
> > +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev);
> > +
> > +#endif /* _PCIE_RCAR_GEN4_H_ */
> > --
> > 2.25.1
> >
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v9 6/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support
2023-02-10 13:49 ` [PATCH v9 6/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support Yoshihiro Shimoda
2023-02-10 17:22 ` Rob Herring
@ 2023-02-10 22:31 ` Bjorn Helgaas
2023-02-13 2:25 ` Yoshihiro Shimoda
2023-02-15 18:33 ` Serge Semin
2 siblings, 1 reply; 20+ messages in thread
From: Bjorn Helgaas @ 2023-02-10 22:31 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: lpieralisi, robh+dt, kw, bhelgaas, jingoohan1, gustavo.pimentel,
Sergey.Semin, marek.vasut+renesas, linux-pci, devicetree,
linux-renesas-soc
On Fri, Feb 10, 2023 at 10:49:15PM +0900, Yoshihiro Shimoda wrote:
> Add R-Car Gen4 PCIe Host support. This controller is based on
> Synopsys DesignWare PCIe.
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> +/* ASPM L1 PM Substates */
> +#define L1PSCAP(x) (0x01bc + (x))
Doesn't appear to be used; please remove it.
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH v9 6/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support
2023-02-10 22:31 ` Bjorn Helgaas
@ 2023-02-13 2:25 ` Yoshihiro Shimoda
0 siblings, 0 replies; 20+ messages in thread
From: Yoshihiro Shimoda @ 2023-02-13 2:25 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: lpieralisi@kernel.org, robh+dt@kernel.org, kw@linux.com,
bhelgaas@google.com, jingoohan1@gmail.com,
gustavo.pimentel@synopsys.com, Sergey.Semin@baikalelectronics.ru,
marek.vasut+renesas@gmail.com, linux-pci@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org
Hi Bjorn,
> From: Bjorn Helgaas, Sent: Saturday, February 11, 2023 7:31 AM
>
> On Fri, Feb 10, 2023 at 10:49:15PM +0900, Yoshihiro Shimoda wrote:
> > Add R-Car Gen4 PCIe Host support. This controller is based on
> > Synopsys DesignWare PCIe.
>
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
>
> > +/* ASPM L1 PM Substates */
> > +#define L1PSCAP(x) (0x01bc + (x))
>
> Doesn't appear to be used; please remove it.
Oops. I'll remove it.
Best regards,
Yoshihiro Shimoda
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v9 6/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support
2023-02-10 13:49 ` [PATCH v9 6/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support Yoshihiro Shimoda
2023-02-10 17:22 ` Rob Herring
2023-02-10 22:31 ` Bjorn Helgaas
@ 2023-02-15 18:33 ` Serge Semin
2023-02-16 11:33 ` Yoshihiro Shimoda
2 siblings, 1 reply; 20+ messages in thread
From: Serge Semin @ 2023-02-15 18:33 UTC (permalink / raw)
To: Yoshihiro Shimoda, Rob Herring, Bjorn Helgaas
Cc: lpieralisi, robh+dt, kw, bhelgaas, jingoohan1, gustavo.pimentel,
Sergey.Semin, marek.vasut+renesas, linux-pci, devicetree,
linux-renesas-soc
On Fri, Feb 10, 2023 at 10:49:15PM +0900, Yoshihiro Shimoda wrote:
> Add R-Car Gen4 PCIe Host support. This controller is based on
> Synopsys DesignWare PCIe.
>
> This controller doesn't support MSI-X interrupt. So, introduce
> no_msix flag in struct dw_pcie_host_ops to clear MSI_FLAG_PCI_MSIX
> from dw_pcie_msi_domain_info.
>
> Note that this controller on R-Car S4-8 has an unexpected register
> value on the dbi+0x97b register. So, add a new capability flag
> which would force the unrolled eDMA mapping for the problematic
> device, as suggested by Serge Semin.
Please move the noted fix to a separate pre-requisite patch seeing your
update depends on it. Thus this patch will be a bit simpler to review
with no harm to the changes atomicity.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
> drivers/pci/controller/dwc/Kconfig | 9 +
> drivers/pci/controller/dwc/Makefile | 2 +
> .../pci/controller/dwc/pcie-designware-host.c | 3 +
> drivers/pci/controller/dwc/pcie-designware.c | 8 +-
> drivers/pci/controller/dwc/pcie-designware.h | 6 +-
> .../pci/controller/dwc/pcie-rcar-gen4-host.c | 165 +++++++++++++++++
> drivers/pci/controller/dwc/pcie-rcar-gen4.c | 166 ++++++++++++++++++
> drivers/pci/controller/dwc/pcie-rcar-gen4.h | 63 +++++++
> 8 files changed, 419 insertions(+), 3 deletions(-)
> create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.c
> create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.h
>
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 434f6a4f4041..94805ec31a8f 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -414,4 +414,13 @@ config PCIE_FU740
> Say Y here if you want PCIe controller support for the SiFive
> FU740.
>
> +config PCIE_RCAR_GEN4
> + tristate "Renesas R-Car Gen4 PCIe Host controller"
> + depends on ARCH_RENESAS || COMPILE_TEST
> + depends on PCI_MSI
> + select PCIE_DW_HOST
> + help
> + Say Y here if you want PCIe host controller support on R-Car Gen4 SoCs.
> + This uses the DesignWare core.
> +
> endmenu
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index bf5c311875a1..486cf706b53d 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -26,6 +26,8 @@ obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
> obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
> obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
> obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
> +pcie-rcar-gen4-host-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-host.o
> +obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4-host-drv.o
>
> # The following drivers are for devices that use the generic ACPI
> # pci_root.c driver but don't support standard ECAM config access.
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 9952057c8819..5aefeec15c9b 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -242,6 +242,9 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
>
> irq_domain_update_bus_token(pp->irq_domain, DOMAIN_BUS_NEXUS);
>
> + if (pp->no_msix)
> + dw_pcie_msi_domain_info.flags &= ~MSI_FLAG_PCI_MSIX;
Changing static data field in the probe path doesn't seem like
correct. Is it really that necessary to clear out that flag? The rest
of the LLDDs don't seem to be bothered with that (mine included). On
the other hand it seems to me that the iMSI-RX engine doesn't really
support true MSI-X messages. It always stick to a single base address
with up to 256 IRQs in total and up to 32 IRQs per function. Do you
suggest to drop that flag then for all DW PCIe hosts or just for ones
with iMSI-RX engine in-use or just forget about it?
@Rob, @Bjorn?
> +
> pp->msi_domain = pci_msi_create_irq_domain(fwnode,
> &dw_pcie_msi_domain_info,
> pp->irq_domain);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index b4315cf84340..7d649ba387f8 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -847,8 +847,14 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> * Indirect eDMA CSRs access has been completely removed since v5.40a
> * thus no space is now reserved for the eDMA channels viewport and
> * former DMA CTRL register is no longer fixed to FFs.
> + *
> + * Note some devices for unknown reason may have zeros in the eDMA CTRL
> + * register even though the HW-manual explicitly states there must FFs
> + * if the unrolled mapping is enabled. For such cases the low-level
> + * drivers are supposed to manually activate the unrolled mapping to
> + * bypass the auto-detection procedure.
> */
> - if (dw_pcie_ver_is_ge(pci, 540A))
> + if (dw_pcie_ver_is_ge(pci, 540A) || dw_pcie_cap_is(pci, EDMA_UNROLL))
> val = 0xFFFFFFFF;
> else
> val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
As I suggested in the head of this message please move this to a
separate pre-requisite patch.
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 1a6e7e9489ee..1b1af514b849 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -51,8 +51,9 @@
>
> /* DWC PCIe controller capabilities */
> #define DW_PCIE_CAP_REQ_RES 0
> -#define DW_PCIE_CAP_IATU_UNROLL 1
> -#define DW_PCIE_CAP_CDM_CHECK 2
> +#define DW_PCIE_CAP_EDMA_UNROLL 1
> +#define DW_PCIE_CAP_IATU_UNROLL 2
> +#define DW_PCIE_CAP_CDM_CHECK 3
ditto
>
> #define dw_pcie_cap_is(_pci, _cap) \
> test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
> @@ -303,6 +304,7 @@ struct dw_pcie_host_ops {
> struct dw_pcie_rp {
> bool has_msi_ctrl:1;
> bool cfg0_io_shared:1;
> + bool no_msix:1;
> u64 cfg0_base;
> void __iomem *va_cfg0_base;
> u32 cfg0_size;
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> new file mode 100644
> index 000000000000..d60f4895ffe9
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> @@ -0,0 +1,165 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * PCIe host controller driver for Renesas R-Car Gen4 Series SoCs
> + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +
> +#include "pcie-rcar-gen4.h"
> +#include "pcie-designware.h"
> +
> +static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
> +{
> + struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
> + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> + int ret;
> + u32 val;
> +
> + ret = rcar_gen4_pcie_set_device_type(rcar, true, dw->num_lanes);
> + if (ret < 0)
> + return ret;
> +
> + dw_pcie_dbi_ro_wr_en(dw);
> +
> + rcar_gen4_pcie_disable_bar(dw, BAR0MASKF);
> + rcar_gen4_pcie_disable_bar(dw, BAR1MASKF);
> +
> + dw_pcie_dbi_ro_wr_dis(dw);
> +
> + if (IS_ENABLED(CONFIG_PCI_MSI)) {
> + /* Enable MSI interrupt signal */
> + val = readl(rcar->base + PCIEINTSTS0EN);
> + val |= MSI_CTRL_INT;
> + writel(val, rcar->base + PCIEINTSTS0EN);
> + }
> +
> + gpiod_set_value_cansleep(dw->pe_rst, 0);
> +
> + dw_pcie_setup_rc(pp);
> +
> + dw_pcie_dbi_ro_wr_en(dw);
> + rcar_gen4_pcie_set_max_link_width(dw, dw->num_lanes);
> + dw_pcie_dbi_ro_wr_dis(dw);
> +
> + if (!dw_pcie_link_up(dw)) {
> + ret = dw->ops->start_link(dw);
> + if (ret)
> + return ret;
> + }
> +
> + /* Ignore errors, the link may come up later */
> + if (dw_pcie_wait_for_link(dw))
> + dev_info(dw->dev, "PCIe link down\n");
This whole procedure is done in the dw_pcie_host_init() method (@Rob
already noted that a bit earlier). The only exception is the
rcar_gen4_pcie_set_max_link_width() method, which I suggest to move
the generic code (see further for more details).
> +
> + return 0;
> +}
> +
> +static const struct dw_pcie_host_ops rcar_gen4_pcie_host_ops = {
> + .host_init = rcar_gen4_pcie_host_init,
> +};
> +
> +static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar,
> + struct platform_device *pdev)
> +{
> + struct dw_pcie *dw = &rcar->dw;
> + struct dw_pcie_rp *pp = &dw->pp;
> + int ret;
> +
> + pp->ops = &rcar_gen4_pcie_host_ops;
> + pp->no_msix = true;
> + dw_pcie_cap_set(dw, REQ_RES);
> +
> + ret = dw_pcie_host_init(pp);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to initialize host\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
> +{
> + dw_pcie_host_deinit(&rcar->dw.pp);
> + gpiod_set_value_cansleep(rcar->dw.pe_rst, 1);
> +}
> +
> +static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar,
> + struct platform_device *pdev)
> +{
> + struct dw_pcie *dw = &rcar->dw;
> +
> + /* Renesas-specific registers */
> + rcar->base = devm_platform_ioremap_resource_byname(pdev, "app");
> + if (IS_ERR(rcar->base))
> + return PTR_ERR(rcar->base);
> +
> + return rcar_gen4_pcie_devm_reset_get(rcar, dw->dev);
> +}
Please note that after fixing the RCAR PCIe EP DT-bindings to permit
the "app" instead of "appl" named reg property (see my note the RCAR
PCie EP DT-bindings patch) the method above will be identical in the
RCAR RP and EP driver implementations. So you'll be able to move it
to the pcie-rcar-gen4.c driver.
> +
> +static int rcar_gen4_pcie_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct rcar_gen4_pcie *rcar;
> + int err;
> +
> + rcar = rcar_gen4_pcie_devm_alloc(dev);
> + if (!rcar)
> + return -ENOMEM;
> +
> + err = rcar_gen4_pcie_get_resources(rcar, pdev);
> + if (err < 0) {
> + dev_err(dev, "Failed to request resource: %d\n", err);
> + return err;
> + }
> +
> + platform_set_drvdata(pdev, rcar);
> +
> + err = rcar_gen4_pcie_prepare(rcar);
> + if (err < 0)
> + return err;
> +
> + err = rcar_gen4_add_dw_pcie_rp(rcar, pdev);
> + if (err < 0)
> + goto err_add;
> +
> + return 0;
> +
> +err_add:
> + rcar_gen4_pcie_unprepare(rcar);
> +
> + return err;
> +}
> +
> +static int rcar_gen4_pcie_remove(struct platform_device *pdev)
> +{
> + struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
> +
> + rcar_gen4_remove_dw_pcie_rp(rcar);
> + rcar_gen4_pcie_unprepare(rcar);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id rcar_gen4_pcie_of_match[] = {
> + { .compatible = "renesas,rcar-gen4-pcie", },
> + {},
> +};
> +
> +static struct platform_driver rcar_gen4_pcie_driver = {
> + .driver = {
> + .name = "pcie-rcar-gen4",
> + .of_match_table = rcar_gen4_pcie_of_match,
> + },
> + .probe = rcar_gen4_pcie_probe,
> + .remove = rcar_gen4_pcie_remove,
> +};
> +module_platform_driver(rcar_gen4_pcie_driver);
> +
> +MODULE_DESCRIPTION("Renesas R-Car Gen4 PCIe host controller driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> new file mode 100644
> index 000000000000..a6a29d6125c8
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -0,0 +1,166 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> + */
> +
> +#include <linux/io.h>
> +#include <linux/of_device.h>
> +#include <linux/pci.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> +
> +#include "pcie-rcar-gen4.h"
> +#include "pcie-designware.h"
> +
> +/* Renesas-specific */
> +#define PCIERSTCTRL1 0x0014
> +#define APP_HOLD_PHY_RST BIT(16)
> +#define APP_LTSSM_ENABLE BIT(0)
> +
> +static void rcar_gen4_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar,
> + bool enable)
> +{
> + u32 val;
> +
> + val = readl(rcar->base + PCIERSTCTRL1);
> + if (enable) {
> + val |= APP_LTSSM_ENABLE;
> + val &= ~APP_HOLD_PHY_RST;
> + } else {
> + val &= ~APP_LTSSM_ENABLE;
> + val |= APP_HOLD_PHY_RST;
> + }
> + writel(val, rcar->base + PCIERSTCTRL1);
> +}
> +
> +static int rcar_gen4_pcie_link_up(struct dw_pcie *dw)
> +{
> + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> + u32 val, mask;
> +
> + val = readl(rcar->base + PCIEINTSTS0);
> + mask = RDLH_LINK_UP | SMLH_LINK_UP;
> +
> + return (val & mask) == mask;
> +}
> +
> +static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
> +{
> + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> +
> + rcar_gen4_pcie_ltssm_enable(rcar, true);
> +
> + return 0;
> +}
> +
> +static void rcar_gen4_pcie_stop_link(struct dw_pcie *dw)
> +{
> + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> +
> + rcar_gen4_pcie_ltssm_enable(rcar, false);
> +}
> +
> +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> + int num_lanes)
> +{
> + u32 val;
> +
> + /* Note: Assume the reset is asserted here */
> + val = readl(rcar->base + PCIEMSR0);
> + if (rc)
> + val |= DEVICE_TYPE_RC;
> + else
> + val |= DEVICE_TYPE_EP;
> + if (num_lanes < 4)
> + val |= BIFUR_MOD_SET_ON;
> + writel(val, rcar->base + PCIEMSR0);
> +
> + return reset_control_deassert(rcar->rst);
> +}
> +
> +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg)
> +{
> + dw_pcie_writel_dbi(dw, SHADOW_REG(bar_mask_reg), 0x0);
> +}
> +
Hm, this seems like a DBI2/DBI_CS2 CSRs. By default the DW PCIe core
driver assumes that the DBI2 is defined within the 0x1000 offset with
respect to the DBI space (see dw_pcie_get_resources()). In your case
it must be within 0x2000 offset (judging by the SHADOW_REG() macro).
What about either defining the "dbi2" reg-named DT-property in the
DT-bindings (see DW PCIe RP/EP generic DT-bindings) or re-defining
the dw_pcie.dbi_base2 field in your LLDD?
Note the dbi_cs2 space is currently utilized in the DW PCIe EP driver
only in the framework of the BARs mapping setup procedure. Are you
sure it's working well in your case seeing the DBI2 base address is
most likely incorrectly initialized? Anyway if you get to define the
dbi_base2 field properly you'll be able to use the
dw_pcie_ep_reset_bar() method in the EP driver instead of the function
above (please see the way it's utilized in the rest of the DW PCIe EP
low-level drivers).
Another question. Are you sure the method above is relevant to the DW
PCIe Root Port controller? I don't see any shadow registers defined
for the host BARs in any HW-manual I've got (4.60, 4.70, 4.90, 5.20,
5.30, 5.40). What are they used for anyway?
> +void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *dw, int num_lanes)
> +{
> + u32 val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_LNKCAP));
> +
> + val &= ~PCI_EXP_LNKCAP_MLW;
> + switch (num_lanes) {
> + case 1:
> + val |= PCI_EXP_LNKCAP_MLW_X1;
> + break;
> + case 2:
> + val |= PCI_EXP_LNKCAP_MLW_X2;
> + break;
> + case 4:
> + val |= PCI_EXP_LNKCAP_MLW_X4;
> + break;
> + default:
> + dev_info(dw->dev, "Invalid num-lanes %d\n", num_lanes);
> + val |= PCI_EXP_LNKCAP_MLW_X1;
> + break;
> + }
> + dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_LNKCAP), val);
> +}
This seems to be a generic action. What about moving it to
pcie-designware.c implementing a function similar to
dw_pcie_link_set_max_speed(), which would be called in
dw_pcie_setup()?
It could be named as dw_pcie_link_set_max_(link_)?width() and besides
of the code above would consist of the PCIE_PORT_LINK_CONTROL and
PCIE_LINK_WIDTH_SPEED_CONTROL CSR initializations performed in
dw_pcie_setup().
* Please do that in a separate pre-requisite patch.
> +
> +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *rcar)
> +{
> + struct device *dev = rcar->dw.dev;
> + int err;
> +
> + pm_runtime_enable(dev);
> + err = pm_runtime_resume_and_get(dev);
> + if (err < 0) {
> + dev_err(dev, "Failed to resume/get Runtime PM\n");
> + pm_runtime_disable(dev);
> + }
> +
> + return err;
> +}
> +
> +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
> +{
> + struct device *dev = rcar->dw.dev;
> +
> + if (!reset_control_status(rcar->rst))
> + reset_control_assert(rcar->rst);
> + pm_runtime_put(dev);
> + pm_runtime_disable(dev);
> +}
> +
> +int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *rcar,
> + struct device *dev)
> +{
> + rcar->rst = devm_reset_control_get(dev, NULL);
> + if (IS_ERR(rcar->rst)) {
> + dev_err(dev, "Failed to get Cold-reset\n");
> + return PTR_ERR(rcar->rst);
> + }
> +
> + return 0;
> +}
> +
> +static const struct dw_pcie_ops dw_pcie_ops = {
> + .start_link = rcar_gen4_pcie_start_link,
> + .stop_link = rcar_gen4_pcie_stop_link,
> + .link_up = rcar_gen4_pcie_link_up,
> +};
> +
> +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev)
> +{
> + struct rcar_gen4_pcie *rcar;
> +
> + rcar = devm_kzalloc(dev, sizeof(*rcar), GFP_KERNEL);
> + if (!rcar)
> + return NULL;
> +
> + rcar->dw.dev = dev;
> + rcar->dw.ops = &dw_pcie_ops;
> + dw_pcie_cap_set(&rcar->dw, EDMA_UNROLL);
> +
> + return rcar;
> +}
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.h b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> new file mode 100644
> index 000000000000..1cdce0cf7dac
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> @@ -0,0 +1,63 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> + */
> +
> +#ifndef _PCIE_RCAR_GEN4_H_
> +#define _PCIE_RCAR_GEN4_H_
> +
> +#include <linux/io.h>
> +#include <linux/pci.h>
> +#include <linux/reset.h>
> +
> +#include "pcie-designware.h"
> +
> +/* PCI Express capability */
> +#define EXPCAP(x) (0x0070 + (x))
> +/* ASPM L1 PM Substates */
> +#define L1PSCAP(x) (0x01bc + (x))
> +/* PCI Shadow offset */
> +#define SHADOW_REG(x) (0x2000 + (x))
> +/* BAR Mask registers */
> +#define BAR0MASKF 0x0010
> +#define BAR1MASKF 0x0014
> +#define BAR2MASKF 0x0018
> +#define BAR3MASKF 0x001c
> +#define BAR4MASKF 0x0020
> +#define BAR5MASKF 0x0024
> +
* If you get to take some of my notes above into account you'll be able to
drop these macros...
> +/* Renesas-specific */
> +#define PCIEMSR0 0x0000
> +#define BIFUR_MOD_SET_ON BIT(0)
> +#define DEVICE_TYPE_EP 0
> +#define DEVICE_TYPE_RC BIT(4)
> +
> +#define PCIEINTSTS0 0x0084
> +#define PCIEINTSTS0EN 0x0310
> +#define MSI_CTRL_INT BIT(26)
> +#define SMLH_LINK_UP BIT(7)
> +#define RDLH_LINK_UP BIT(6)
> +#define PCIEDMAINTSTSEN 0x0314
> +#define PCIEDMAINTSTSEN_INIT GENMASK(15, 0)
Vendor-specific prefix would make the code using these macros much
more readable.
> +
> +struct rcar_gen4_pcie {
> + struct dw_pcie dw;
> + void __iomem *base;
> + struct reset_control *rst;
> +};
> +#define to_rcar_gen4_pcie(x) dev_get_drvdata((x)->dev)
> +
> +u32 rcar_gen4_pcie_readl(struct rcar_gen4_pcie *pcie, u32 reg);
> +void rcar_gen4_pcie_writel(struct rcar_gen4_pcie *pcie, u32 reg, u32 val);
I don't see these two methods being defined in your driver. Are these
artefacts from the previous patch revisions?
-Serge(y)
> +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> + int num_lanes);
> +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg);
> +void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *pci, int num_lanes);
> +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *pcie);
> +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *pcie);
> +int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *pcie,
> + struct device *dev);
> +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev);
> +
> +#endif /* _PCIE_RCAR_GEN4_H_ */
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread* RE: [PATCH v9 6/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support
2023-02-15 18:33 ` Serge Semin
@ 2023-02-16 11:33 ` Yoshihiro Shimoda
2023-02-17 11:15 ` Serge Semin
0 siblings, 1 reply; 20+ messages in thread
From: Yoshihiro Shimoda @ 2023-02-16 11:33 UTC (permalink / raw)
To: Serge Semin, Rob Herring, Bjorn Helgaas, lpieralisi@kernel.org
Cc: robh+dt@kernel.org, kw@linux.com, bhelgaas@google.com,
jingoohan1@gmail.com, gustavo.pimentel@synopsys.com,
Sergey.Semin@baikalelectronics.ru, marek.vasut+renesas@gmail.com,
linux-pci@vger.kernel.org, devicetree@vger.kernel.org,
linux-renesas-soc@vger.kernel.org
Hi Serge,
> From: Serge Semin, Sent: Thursday, February 16, 2023 3:33 AM
>
> On Fri, Feb 10, 2023 at 10:49:15PM +0900, Yoshihiro Shimoda wrote:
> > Add R-Car Gen4 PCIe Host support. This controller is based on
> > Synopsys DesignWare PCIe.
> >
> > This controller doesn't support MSI-X interrupt. So, introduce
> > no_msix flag in struct dw_pcie_host_ops to clear MSI_FLAG_PCI_MSIX
> > from dw_pcie_msi_domain_info.
> >
>
> > Note that this controller on R-Car S4-8 has an unexpected register
> > value on the dbi+0x97b register. So, add a new capability flag
> > which would force the unrolled eDMA mapping for the problematic
> > device, as suggested by Serge Semin.
>
> Please move the noted fix to a separate pre-requisite patch seeing your
> update depends on it. Thus this patch will be a bit simpler to review
> with no harm to the changes atomicity.
There is other patch though, I think Lorenzo prefers non separated patch:
https://lore.kernel.org/all/Y245ZtqqqelXif+Y@lpieralisi/
But, Lorenzo, what do you think?
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> > drivers/pci/controller/dwc/Kconfig | 9 +
> > drivers/pci/controller/dwc/Makefile | 2 +
> > .../pci/controller/dwc/pcie-designware-host.c | 3 +
> > drivers/pci/controller/dwc/pcie-designware.c | 8 +-
> > drivers/pci/controller/dwc/pcie-designware.h | 6 +-
> > .../pci/controller/dwc/pcie-rcar-gen4-host.c | 165 +++++++++++++++++
> > drivers/pci/controller/dwc/pcie-rcar-gen4.c | 166 ++++++++++++++++++
> > drivers/pci/controller/dwc/pcie-rcar-gen4.h | 63 +++++++
> > 8 files changed, 419 insertions(+), 3 deletions(-)
> > create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> > create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.h
> >
> > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > index 434f6a4f4041..94805ec31a8f 100644
> > --- a/drivers/pci/controller/dwc/Kconfig
> > +++ b/drivers/pci/controller/dwc/Kconfig
> > @@ -414,4 +414,13 @@ config PCIE_FU740
> > Say Y here if you want PCIe controller support for the SiFive
> > FU740.
> >
> > +config PCIE_RCAR_GEN4
> > + tristate "Renesas R-Car Gen4 PCIe Host controller"
> > + depends on ARCH_RENESAS || COMPILE_TEST
> > + depends on PCI_MSI
> > + select PCIE_DW_HOST
> > + help
> > + Say Y here if you want PCIe host controller support on R-Car Gen4 SoCs.
> > + This uses the DesignWare core.
> > +
> > endmenu
> > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> > index bf5c311875a1..486cf706b53d 100644
> > --- a/drivers/pci/controller/dwc/Makefile
> > +++ b/drivers/pci/controller/dwc/Makefile
> > @@ -26,6 +26,8 @@ obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
> > obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
> > obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
> > obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
> > +pcie-rcar-gen4-host-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-host.o
> > +obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4-host-drv.o
> >
> > # The following drivers are for devices that use the generic ACPI
> > # pci_root.c driver but don't support standard ECAM config access.
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index 9952057c8819..5aefeec15c9b 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -242,6 +242,9 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
> >
> > irq_domain_update_bus_token(pp->irq_domain, DOMAIN_BUS_NEXUS);
> >
>
> > + if (pp->no_msix)
> > + dw_pcie_msi_domain_info.flags &= ~MSI_FLAG_PCI_MSIX;
>
> Changing static data field in the probe path doesn't seem like
> correct. Is it really that necessary to clear out that flag?
I didn't investigate the detail, but even if clearing this flag
disappeared, it seemed to work.
> The rest
> of the LLDDs don't seem to be bothered with that (mine included). On
> the other hand it seems to me that the iMSI-RX engine doesn't really
> support true MSI-X messages. It always stick to a single base address
> with up to 256 IRQs in total and up to 32 IRQs per function. Do you
> suggest to drop that flag then for all DW PCIe hosts or just for ones
> with iMSI-RX engine in-use or just forget about it?
I assumed almost all controller supported MSI-X, so that I suggested
to drop the MSI_FLAG_PCI_MSIX flag if needed.
> @Rob, @Bjorn?
>
> > +
> > pp->msi_domain = pci_msi_create_irq_domain(fwnode,
> > &dw_pcie_msi_domain_info,
> > pp->irq_domain);
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index b4315cf84340..7d649ba387f8 100644
>
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -847,8 +847,14 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > * Indirect eDMA CSRs access has been completely removed since v5.40a
> > * thus no space is now reserved for the eDMA channels viewport and
> > * former DMA CTRL register is no longer fixed to FFs.
> > + *
> > + * Note some devices for unknown reason may have zeros in the eDMA CTRL
> > + * register even though the HW-manual explicitly states there must FFs
> > + * if the unrolled mapping is enabled. For such cases the low-level
> > + * drivers are supposed to manually activate the unrolled mapping to
> > + * bypass the auto-detection procedure.
> > */
> > - if (dw_pcie_ver_is_ge(pci, 540A))
> > + if (dw_pcie_ver_is_ge(pci, 540A) || dw_pcie_cap_is(pci, EDMA_UNROLL))
> > val = 0xFFFFFFFF;
> > else
> > val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
>
> As I suggested in the head of this message please move this to a
> separate pre-requisite patch.
I would like to know Lorenzo's preference.
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index 1a6e7e9489ee..1b1af514b849 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -51,8 +51,9 @@
> >
> > /* DWC PCIe controller capabilities */
> > #define DW_PCIE_CAP_REQ_RES 0
>
> > -#define DW_PCIE_CAP_IATU_UNROLL 1
> > -#define DW_PCIE_CAP_CDM_CHECK 2
> > +#define DW_PCIE_CAP_EDMA_UNROLL 1
> > +#define DW_PCIE_CAP_IATU_UNROLL 2
> > +#define DW_PCIE_CAP_CDM_CHECK 3
>
> ditto
>
> >
> > #define dw_pcie_cap_is(_pci, _cap) \
> > test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
> > @@ -303,6 +304,7 @@ struct dw_pcie_host_ops {
> > struct dw_pcie_rp {
> > bool has_msi_ctrl:1;
> > bool cfg0_io_shared:1;
> > + bool no_msix:1;
> > u64 cfg0_base;
> > void __iomem *va_cfg0_base;
> > u32 cfg0_size;
> > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> > new file mode 100644
> > index 000000000000..d60f4895ffe9
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> > @@ -0,0 +1,165 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * PCIe host controller driver for Renesas R-Car Gen4 Series SoCs
> > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > + */
> > +
> > +#include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +#include <linux/pci.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include "pcie-rcar-gen4.h"
> > +#include "pcie-designware.h"
> > +
> > +static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
> > +{
> > + struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
> > + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > + int ret;
> > + u32 val;
> > +
> > + ret = rcar_gen4_pcie_set_device_type(rcar, true, dw->num_lanes);
> > + if (ret < 0)
> > + return ret;
> > +
> > + dw_pcie_dbi_ro_wr_en(dw);
> > +
> > + rcar_gen4_pcie_disable_bar(dw, BAR0MASKF);
> > + rcar_gen4_pcie_disable_bar(dw, BAR1MASKF);
> > +
> > + dw_pcie_dbi_ro_wr_dis(dw);
> > +
> > + if (IS_ENABLED(CONFIG_PCI_MSI)) {
> > + /* Enable MSI interrupt signal */
> > + val = readl(rcar->base + PCIEINTSTS0EN);
> > + val |= MSI_CTRL_INT;
> > + writel(val, rcar->base + PCIEINTSTS0EN);
> > + }
> > +
> > + gpiod_set_value_cansleep(dw->pe_rst, 0);
> > +
>
> > + dw_pcie_setup_rc(pp);
> > +
> > + dw_pcie_dbi_ro_wr_en(dw);
> > + rcar_gen4_pcie_set_max_link_width(dw, dw->num_lanes);
> > + dw_pcie_dbi_ro_wr_dis(dw);
> > +
> > + if (!dw_pcie_link_up(dw)) {
> > + ret = dw->ops->start_link(dw);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + /* Ignore errors, the link may come up later */
> > + if (dw_pcie_wait_for_link(dw))
> > + dev_info(dw->dev, "PCIe link down\n");
>
> This whole procedure is done in the dw_pcie_host_init() method (@Rob
> already noted that a bit earlier). The only exception is the
> rcar_gen4_pcie_set_max_link_width() method, which I suggest to move
> the generic code (see further for more details).
I investigated this and then I found they can be just removed.
# This is related to the following patch. After I applied the following,
# I can remove the above codes.
# https://lore.kernel.org/linux-pci/20230216092012.3256440-1-yoshihiro.shimoda.uh@renesas.com/
> > +
> > + return 0;
> > +}
> > +
> > +static const struct dw_pcie_host_ops rcar_gen4_pcie_host_ops = {
> > + .host_init = rcar_gen4_pcie_host_init,
> > +};
> > +
> > +static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar,
> > + struct platform_device *pdev)
> > +{
> > + struct dw_pcie *dw = &rcar->dw;
> > + struct dw_pcie_rp *pp = &dw->pp;
> > + int ret;
> > +
> > + pp->ops = &rcar_gen4_pcie_host_ops;
> > + pp->no_msix = true;
> > + dw_pcie_cap_set(dw, REQ_RES);
> > +
> > + ret = dw_pcie_host_init(pp);
> > + if (ret) {
> > + dev_err(&pdev->dev, "Failed to initialize host\n");
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
> > +{
> > + dw_pcie_host_deinit(&rcar->dw.pp);
> > + gpiod_set_value_cansleep(rcar->dw.pe_rst, 1);
> > +}
> > +
>
> > +static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar,
> > + struct platform_device *pdev)
> > +{
> > + struct dw_pcie *dw = &rcar->dw;
> > +
> > + /* Renesas-specific registers */
> > + rcar->base = devm_platform_ioremap_resource_byname(pdev, "app");
> > + if (IS_ERR(rcar->base))
> > + return PTR_ERR(rcar->base);
> > +
> > + return rcar_gen4_pcie_devm_reset_get(rcar, dw->dev);
> > +}
>
> Please note that after fixing the RCAR PCIe EP DT-bindings to permit
> the "app" instead of "appl" named reg property (see my note the RCAR
> PCie EP DT-bindings patch) the method above will be identical in the
> RCAR RP and EP driver implementations. So you'll be able to move it
> to the pcie-rcar-gen4.c driver.
I think so. I'll modify it.
> > +
> > +static int rcar_gen4_pcie_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct rcar_gen4_pcie *rcar;
> > + int err;
> > +
> > + rcar = rcar_gen4_pcie_devm_alloc(dev);
> > + if (!rcar)
> > + return -ENOMEM;
> > +
> > + err = rcar_gen4_pcie_get_resources(rcar, pdev);
> > + if (err < 0) {
> > + dev_err(dev, "Failed to request resource: %d\n", err);
> > + return err;
> > + }
> > +
> > + platform_set_drvdata(pdev, rcar);
> > +
> > + err = rcar_gen4_pcie_prepare(rcar);
> > + if (err < 0)
> > + return err;
> > +
> > + err = rcar_gen4_add_dw_pcie_rp(rcar, pdev);
> > + if (err < 0)
> > + goto err_add;
> > +
> > + return 0;
> > +
> > +err_add:
> > + rcar_gen4_pcie_unprepare(rcar);
> > +
> > + return err;
> > +}
> > +
> > +static int rcar_gen4_pcie_remove(struct platform_device *pdev)
> > +{
> > + struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
> > +
> > + rcar_gen4_remove_dw_pcie_rp(rcar);
> > + rcar_gen4_pcie_unprepare(rcar);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id rcar_gen4_pcie_of_match[] = {
> > + { .compatible = "renesas,rcar-gen4-pcie", },
> > + {},
> > +};
> > +
> > +static struct platform_driver rcar_gen4_pcie_driver = {
> > + .driver = {
> > + .name = "pcie-rcar-gen4",
> > + .of_match_table = rcar_gen4_pcie_of_match,
> > + },
> > + .probe = rcar_gen4_pcie_probe,
> > + .remove = rcar_gen4_pcie_remove,
> > +};
> > +module_platform_driver(rcar_gen4_pcie_driver);
> > +
> > +MODULE_DESCRIPTION("Renesas R-Car Gen4 PCIe host controller driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > new file mode 100644
> > index 000000000000..a6a29d6125c8
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > @@ -0,0 +1,166 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > + */
> > +
> > +#include <linux/io.h>
> > +#include <linux/of_device.h>
> > +#include <linux/pci.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/reset.h>
> > +
> > +#include "pcie-rcar-gen4.h"
> > +#include "pcie-designware.h"
> > +
> > +/* Renesas-specific */
> > +#define PCIERSTCTRL1 0x0014
> > +#define APP_HOLD_PHY_RST BIT(16)
> > +#define APP_LTSSM_ENABLE BIT(0)
> > +
> > +static void rcar_gen4_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar,
> > + bool enable)
> > +{
> > + u32 val;
> > +
> > + val = readl(rcar->base + PCIERSTCTRL1);
> > + if (enable) {
> > + val |= APP_LTSSM_ENABLE;
> > + val &= ~APP_HOLD_PHY_RST;
> > + } else {
> > + val &= ~APP_LTSSM_ENABLE;
> > + val |= APP_HOLD_PHY_RST;
> > + }
> > + writel(val, rcar->base + PCIERSTCTRL1);
> > +}
> > +
> > +static int rcar_gen4_pcie_link_up(struct dw_pcie *dw)
> > +{
> > + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > + u32 val, mask;
> > +
> > + val = readl(rcar->base + PCIEINTSTS0);
> > + mask = RDLH_LINK_UP | SMLH_LINK_UP;
> > +
> > + return (val & mask) == mask;
> > +}
> > +
> > +static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
> > +{
> > + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > +
> > + rcar_gen4_pcie_ltssm_enable(rcar, true);
> > +
> > + return 0;
> > +}
> > +
> > +static void rcar_gen4_pcie_stop_link(struct dw_pcie *dw)
> > +{
> > + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > +
> > + rcar_gen4_pcie_ltssm_enable(rcar, false);
> > +}
> > +
> > +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> > + int num_lanes)
> > +{
> > + u32 val;
> > +
> > + /* Note: Assume the reset is asserted here */
> > + val = readl(rcar->base + PCIEMSR0);
> > + if (rc)
> > + val |= DEVICE_TYPE_RC;
> > + else
> > + val |= DEVICE_TYPE_EP;
> > + if (num_lanes < 4)
> > + val |= BIFUR_MOD_SET_ON;
> > + writel(val, rcar->base + PCIEMSR0);
> > +
> > + return reset_control_deassert(rcar->rst);
> > +}
> > +
>
> > +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg)
> > +{
> > + dw_pcie_writel_dbi(dw, SHADOW_REG(bar_mask_reg), 0x0);
> > +}
> > +
>
> Hm, this seems like a DBI2/DBI_CS2 CSRs. By default the DW PCIe core
> driver assumes that the DBI2 is defined within the 0x1000 offset with
> respect to the DBI space (see dw_pcie_get_resources()). In your case
> it must be within 0x2000 offset (judging by the SHADOW_REG() macro).
> What about either defining the "dbi2" reg-named DT-property in the
> DT-bindings (see DW PCIe RP/EP generic DT-bindings) or re-defining
> the dw_pcie.dbi_base2 field in your LLDD?
>
> Note the dbi_cs2 space is currently utilized in the DW PCIe EP driver
> only in the framework of the BARs mapping setup procedure. Are you
> sure it's working well in your case seeing the DBI2 base address is
> most likely incorrectly initialized? Anyway if you get to define the
> dbi_base2 field properly you'll be able to use the
> dw_pcie_ep_reset_bar() method in the EP driver instead of the function
> above (please see the way it's utilized in the rest of the DW PCIe EP
> low-level drivers).
Thank you for your comments. I'll investigate it.
> Another question. Are you sure the method above is relevant to the DW
> PCIe Root Port controller? I don't see any shadow registers defined
> for the host BARs in any HW-manual I've got (4.60, 4.70, 4.90, 5.20,
> 5.30, 5.40). What are they used for anyway?
To be honest, I reuse this code from our BSP which other guy made.
So, I'll investigate it.
> > +void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *dw, int num_lanes)
> > +{
> > + u32 val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_LNKCAP));
> > +
> > + val &= ~PCI_EXP_LNKCAP_MLW;
> > + switch (num_lanes) {
> > + case 1:
> > + val |= PCI_EXP_LNKCAP_MLW_X1;
> > + break;
> > + case 2:
> > + val |= PCI_EXP_LNKCAP_MLW_X2;
> > + break;
> > + case 4:
> > + val |= PCI_EXP_LNKCAP_MLW_X4;
> > + break;
> > + default:
> > + dev_info(dw->dev, "Invalid num-lanes %d\n", num_lanes);
> > + val |= PCI_EXP_LNKCAP_MLW_X1;
> > + break;
> > + }
> > + dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_LNKCAP), val);
> > +}
>
> This seems to be a generic action. What about moving it to
> pcie-designware.c implementing a function similar to
> dw_pcie_link_set_max_speed(), which would be called in
> dw_pcie_setup()?
Rob also suggests it. So, I'll investigate it.
> It could be named as dw_pcie_link_set_max_(link_)?width() and besides
> of the code above would consist of the PCIE_PORT_LINK_CONTROL and
> PCIE_LINK_WIDTH_SPEED_CONTROL CSR initializations performed in
> dw_pcie_setup().
Thank you for your comments.
> * Please do that in a separate pre-requisite patch.
>
> > +
> > +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *rcar)
> > +{
> > + struct device *dev = rcar->dw.dev;
> > + int err;
> > +
> > + pm_runtime_enable(dev);
> > + err = pm_runtime_resume_and_get(dev);
> > + if (err < 0) {
> > + dev_err(dev, "Failed to resume/get Runtime PM\n");
> > + pm_runtime_disable(dev);
> > + }
> > +
> > + return err;
> > +}
> > +
> > +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
> > +{
> > + struct device *dev = rcar->dw.dev;
> > +
> > + if (!reset_control_status(rcar->rst))
> > + reset_control_assert(rcar->rst);
> > + pm_runtime_put(dev);
> > + pm_runtime_disable(dev);
> > +}
> > +
> > +int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *rcar,
> > + struct device *dev)
> > +{
> > + rcar->rst = devm_reset_control_get(dev, NULL);
> > + if (IS_ERR(rcar->rst)) {
> > + dev_err(dev, "Failed to get Cold-reset\n");
> > + return PTR_ERR(rcar->rst);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static const struct dw_pcie_ops dw_pcie_ops = {
> > + .start_link = rcar_gen4_pcie_start_link,
> > + .stop_link = rcar_gen4_pcie_stop_link,
> > + .link_up = rcar_gen4_pcie_link_up,
> > +};
> > +
> > +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev)
> > +{
> > + struct rcar_gen4_pcie *rcar;
> > +
> > + rcar = devm_kzalloc(dev, sizeof(*rcar), GFP_KERNEL);
> > + if (!rcar)
> > + return NULL;
> > +
> > + rcar->dw.dev = dev;
> > + rcar->dw.ops = &dw_pcie_ops;
> > + dw_pcie_cap_set(&rcar->dw, EDMA_UNROLL);
> > +
> > + return rcar;
> > +}
> > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.h b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> > new file mode 100644
> > index 000000000000..1cdce0cf7dac
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> > @@ -0,0 +1,63 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > + */
> > +
> > +#ifndef _PCIE_RCAR_GEN4_H_
> > +#define _PCIE_RCAR_GEN4_H_
> > +
> > +#include <linux/io.h>
> > +#include <linux/pci.h>
> > +#include <linux/reset.h>
> > +
> > +#include "pcie-designware.h"
> > +
>
> > +/* PCI Express capability */
> > +#define EXPCAP(x) (0x0070 + (x))
> > +/* ASPM L1 PM Substates */
> > +#define L1PSCAP(x) (0x01bc + (x))
> > +/* PCI Shadow offset */
> > +#define SHADOW_REG(x) (0x2000 + (x))
> > +/* BAR Mask registers */
> > +#define BAR0MASKF 0x0010
> > +#define BAR1MASKF 0x0014
> > +#define BAR2MASKF 0x0018
> > +#define BAR3MASKF 0x001c
> > +#define BAR4MASKF 0x0020
> > +#define BAR5MASKF 0x0024
> > +
>
> * If you get to take some of my notes above into account you'll be able to
> drop these macros...
I got it.
> > +/* Renesas-specific */
> > +#define PCIEMSR0 0x0000
> > +#define BIFUR_MOD_SET_ON BIT(0)
> > +#define DEVICE_TYPE_EP 0
> > +#define DEVICE_TYPE_RC BIT(4)
> > +
> > +#define PCIEINTSTS0 0x0084
> > +#define PCIEINTSTS0EN 0x0310
> > +#define MSI_CTRL_INT BIT(26)
> > +#define SMLH_LINK_UP BIT(7)
> > +#define RDLH_LINK_UP BIT(6)
> > +#define PCIEDMAINTSTSEN 0x0314
> > +#define PCIEDMAINTSTSEN_INIT GENMASK(15, 0)
>
> Vendor-specific prefix would make the code using these macros much
> more readable.
I got it. I'll rename them.
> > +
> > +struct rcar_gen4_pcie {
> > + struct dw_pcie dw;
> > + void __iomem *base;
> > + struct reset_control *rst;
> > +};
> > +#define to_rcar_gen4_pcie(x) dev_get_drvdata((x)->dev)
> > +
>
> > +u32 rcar_gen4_pcie_readl(struct rcar_gen4_pcie *pcie, u32 reg);
> > +void rcar_gen4_pcie_writel(struct rcar_gen4_pcie *pcie, u32 reg, u32 val);
>
> I don't see these two methods being defined in your driver. Are these
> artefacts from the previous patch revisions?
Oops. I'll remove them.
Best regards,
Yoshihiro Shimoda
> -Serge(y)
>
> > +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> > + int num_lanes);
>
> > +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg);
> > +void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *pci, int num_lanes);
> > +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *pcie);
> > +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *pcie);
> > +int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *pcie,
> > + struct device *dev);
> > +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev);
> > +
> > +#endif /* _PCIE_RCAR_GEN4_H_ */
> > --
> > 2.25.1
> >
> >
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v9 6/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support
2023-02-16 11:33 ` Yoshihiro Shimoda
@ 2023-02-17 11:15 ` Serge Semin
0 siblings, 0 replies; 20+ messages in thread
From: Serge Semin @ 2023-02-17 11:15 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Rob Herring, Bjorn Helgaas, lpieralisi@kernel.org,
robh+dt@kernel.org, kw@linux.com, bhelgaas@google.com,
jingoohan1@gmail.com, gustavo.pimentel@synopsys.com,
Sergey.Semin@baikalelectronics.ru, marek.vasut+renesas@gmail.com,
linux-pci@vger.kernel.org, devicetree@vger.kernel.org,
linux-renesas-soc@vger.kernel.org
On Thu, Feb 16, 2023 at 11:33:16AM +0000, Yoshihiro Shimoda wrote:
> Hi Serge,
>
> > From: Serge Semin, Sent: Thursday, February 16, 2023 3:33 AM
> >
> > On Fri, Feb 10, 2023 at 10:49:15PM +0900, Yoshihiro Shimoda wrote:
> > > Add R-Car Gen4 PCIe Host support. This controller is based on
> > > Synopsys DesignWare PCIe.
> > >
> > > This controller doesn't support MSI-X interrupt. So, introduce
> > > no_msix flag in struct dw_pcie_host_ops to clear MSI_FLAG_PCI_MSIX
> > > from dw_pcie_msi_domain_info.
> > >
> >
> > > Note that this controller on R-Car S4-8 has an unexpected register
> > > value on the dbi+0x97b register. So, add a new capability flag
> > > which would force the unrolled eDMA mapping for the problematic
> > > device, as suggested by Serge Semin.
> >
> > Please move the noted fix to a separate pre-requisite patch seeing your
> > update depends on it. Thus this patch will be a bit simpler to review
> > with no harm to the changes atomicity.
>
> There is other patch though, I think Lorenzo prefers non separated patch:
> https://lore.kernel.org/all/Y245ZtqqqelXif+Y@lpieralisi/
>
> But, Lorenzo, what do you think?
>
> > >
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > ---
> > > drivers/pci/controller/dwc/Kconfig | 9 +
> > > drivers/pci/controller/dwc/Makefile | 2 +
> > > .../pci/controller/dwc/pcie-designware-host.c | 3 +
> > > drivers/pci/controller/dwc/pcie-designware.c | 8 +-
> > > drivers/pci/controller/dwc/pcie-designware.h | 6 +-
> > > .../pci/controller/dwc/pcie-rcar-gen4-host.c | 165 +++++++++++++++++
> > > drivers/pci/controller/dwc/pcie-rcar-gen4.c | 166 ++++++++++++++++++
> > > drivers/pci/controller/dwc/pcie-rcar-gen4.h | 63 +++++++
> > > 8 files changed, 419 insertions(+), 3 deletions(-)
> > > create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> > > create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > > create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.h
> > >
> > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > > index 434f6a4f4041..94805ec31a8f 100644
> > > --- a/drivers/pci/controller/dwc/Kconfig
> > > +++ b/drivers/pci/controller/dwc/Kconfig
> > > @@ -414,4 +414,13 @@ config PCIE_FU740
> > > Say Y here if you want PCIe controller support for the SiFive
> > > FU740.
> > >
> > > +config PCIE_RCAR_GEN4
> > > + tristate "Renesas R-Car Gen4 PCIe Host controller"
> > > + depends on ARCH_RENESAS || COMPILE_TEST
> > > + depends on PCI_MSI
> > > + select PCIE_DW_HOST
> > > + help
> > > + Say Y here if you want PCIe host controller support on R-Car Gen4 SoCs.
> > > + This uses the DesignWare core.
> > > +
> > > endmenu
> > > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> > > index bf5c311875a1..486cf706b53d 100644
> > > --- a/drivers/pci/controller/dwc/Makefile
> > > +++ b/drivers/pci/controller/dwc/Makefile
> > > @@ -26,6 +26,8 @@ obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
> > > obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
> > > obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
> > > obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
> > > +pcie-rcar-gen4-host-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-host.o
> > > +obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4-host-drv.o
> > >
> > > # The following drivers are for devices that use the generic ACPI
> > > # pci_root.c driver but don't support standard ECAM config access.
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > index 9952057c8819..5aefeec15c9b 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > @@ -242,6 +242,9 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
> > >
> > > irq_domain_update_bus_token(pp->irq_domain, DOMAIN_BUS_NEXUS);
> > >
> >
> > > + if (pp->no_msix)
> > > + dw_pcie_msi_domain_info.flags &= ~MSI_FLAG_PCI_MSIX;
> >
> > Changing static data field in the probe path doesn't seem like
> > correct. Is it really that necessary to clear out that flag?
>
> I didn't investigate the detail, but even if clearing this flag
> disappeared, it seemed to work.
Yeah, until we need to allocate more than 32 IRQ vectors per
hierarchy.) That can be tuned by either increasing the
MSI_DEF_NUM_VECTORS macros value to 256 or re-initializing the
num_vectors field in the LLDD with MAX_MSI_IRQS. That's what iMSI-RX
engine is capable of. But anyways it isn't near to what the
MSI-X-capable controller is supposed to support.
>
> > The rest
> > of the LLDDs don't seem to be bothered with that (mine included). On
> > the other hand it seems to me that the iMSI-RX engine doesn't really
> > support true MSI-X messages. It always stick to a single base address
> > with up to 256 IRQs in total and up to 32 IRQs per function. Do you
> > suggest to drop that flag then for all DW PCIe hosts or just for ones
> > with iMSI-RX engine in-use or just forget about it?
>
> I assumed almost all controller supported MSI-X, so that I suggested
> to drop the MSI_FLAG_PCI_MSIX flag if needed.
AFAICS iMSI-RX engine supports just MSI only at very least due to
having a single CSR for the MSI target address meanwhile MSI-X is
supposed to support multiple target addresses.
>
> > @Rob, @Bjorn?
> >
> > > +
> > > pp->msi_domain = pci_msi_create_irq_domain(fwnode,
> > > &dw_pcie_msi_domain_info,
> > > pp->irq_domain);
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > > index b4315cf84340..7d649ba387f8 100644
> >
> > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > @@ -847,8 +847,14 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > > * Indirect eDMA CSRs access has been completely removed since v5.40a
> > > * thus no space is now reserved for the eDMA channels viewport and
> > > * former DMA CTRL register is no longer fixed to FFs.
> > > + *
> > > + * Note some devices for unknown reason may have zeros in the eDMA CTRL
> > > + * register even though the HW-manual explicitly states there must FFs
> > > + * if the unrolled mapping is enabled. For such cases the low-level
> > > + * drivers are supposed to manually activate the unrolled mapping to
> > > + * bypass the auto-detection procedure.
> > > */
> > > - if (dw_pcie_ver_is_ge(pci, 540A))
> > > + if (dw_pcie_ver_is_ge(pci, 540A) || dw_pcie_cap_is(pci, EDMA_UNROLL))
> > > val = 0xFFFFFFFF;
> > > else
> > > val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> >
> > As I suggested in the head of this message please move this to a
> > separate pre-requisite patch.
>
> I would like to know Lorenzo's preference.
>
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > > index 1a6e7e9489ee..1b1af514b849 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > > @@ -51,8 +51,9 @@
> > >
> > > /* DWC PCIe controller capabilities */
> > > #define DW_PCIE_CAP_REQ_RES 0
> >
> > > -#define DW_PCIE_CAP_IATU_UNROLL 1
> > > -#define DW_PCIE_CAP_CDM_CHECK 2
> > > +#define DW_PCIE_CAP_EDMA_UNROLL 1
> > > +#define DW_PCIE_CAP_IATU_UNROLL 2
> > > +#define DW_PCIE_CAP_CDM_CHECK 3
> >
> > ditto
> >
> > >
> > > #define dw_pcie_cap_is(_pci, _cap) \
> > > test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
> > > @@ -303,6 +304,7 @@ struct dw_pcie_host_ops {
> > > struct dw_pcie_rp {
> > > bool has_msi_ctrl:1;
> > > bool cfg0_io_shared:1;
> > > + bool no_msix:1;
> > > u64 cfg0_base;
> > > void __iomem *va_cfg0_base;
> > > u32 cfg0_size;
> > > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> > > new file mode 100644
> > > index 000000000000..d60f4895ffe9
> > > --- /dev/null
> > > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> > > @@ -0,0 +1,165 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * PCIe host controller driver for Renesas R-Car Gen4 Series SoCs
> > > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > > + */
> > > +
> > > +#include <linux/interrupt.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of_device.h>
> > > +#include <linux/pci.h>
> > > +#include <linux/platform_device.h>
> > > +
> > > +#include "pcie-rcar-gen4.h"
> > > +#include "pcie-designware.h"
> > > +
> > > +static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
> > > +{
> > > + struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
> > > + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > > + int ret;
> > > + u32 val;
> > > +
> > > + ret = rcar_gen4_pcie_set_device_type(rcar, true, dw->num_lanes);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + dw_pcie_dbi_ro_wr_en(dw);
> > > +
> > > + rcar_gen4_pcie_disable_bar(dw, BAR0MASKF);
> > > + rcar_gen4_pcie_disable_bar(dw, BAR1MASKF);
> > > +
> > > + dw_pcie_dbi_ro_wr_dis(dw);
> > > +
> > > + if (IS_ENABLED(CONFIG_PCI_MSI)) {
> > > + /* Enable MSI interrupt signal */
> > > + val = readl(rcar->base + PCIEINTSTS0EN);
> > > + val |= MSI_CTRL_INT;
> > > + writel(val, rcar->base + PCIEINTSTS0EN);
> > > + }
> > > +
> > > + gpiod_set_value_cansleep(dw->pe_rst, 0);
> > > +
> >
> > > + dw_pcie_setup_rc(pp);
> > > +
> > > + dw_pcie_dbi_ro_wr_en(dw);
> > > + rcar_gen4_pcie_set_max_link_width(dw, dw->num_lanes);
> > > + dw_pcie_dbi_ro_wr_dis(dw);
> > > +
> > > + if (!dw_pcie_link_up(dw)) {
> > > + ret = dw->ops->start_link(dw);
> > > + if (ret)
> > > + return ret;
> > > + }
> > > +
> > > + /* Ignore errors, the link may come up later */
> > > + if (dw_pcie_wait_for_link(dw))
> > > + dev_info(dw->dev, "PCIe link down\n");
> >
> > This whole procedure is done in the dw_pcie_host_init() method (@Rob
> > already noted that a bit earlier). The only exception is the
> > rcar_gen4_pcie_set_max_link_width() method, which I suggest to move
> > the generic code (see further for more details).
>
> I investigated this and then I found they can be just removed.
> # This is related to the following patch. After I applied the following,
> # I can remove the above codes.
> # https://lore.kernel.org/linux-pci/20230216092012.3256440-1-yoshihiro.shimoda.uh@renesas.com/
Yeah, my mistake.( I should have been more attentive. Sorry about
that.
-Serge(y)
>
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static const struct dw_pcie_host_ops rcar_gen4_pcie_host_ops = {
> > > + .host_init = rcar_gen4_pcie_host_init,
> > > +};
> > > +
> > > +static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar,
> > > + struct platform_device *pdev)
> > > +{
> > > + struct dw_pcie *dw = &rcar->dw;
> > > + struct dw_pcie_rp *pp = &dw->pp;
> > > + int ret;
> > > +
> > > + pp->ops = &rcar_gen4_pcie_host_ops;
> > > + pp->no_msix = true;
> > > + dw_pcie_cap_set(dw, REQ_RES);
> > > +
> > > + ret = dw_pcie_host_init(pp);
> > > + if (ret) {
> > > + dev_err(&pdev->dev, "Failed to initialize host\n");
> > > + return ret;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
> > > +{
> > > + dw_pcie_host_deinit(&rcar->dw.pp);
> > > + gpiod_set_value_cansleep(rcar->dw.pe_rst, 1);
> > > +}
> > > +
> >
> > > +static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar,
> > > + struct platform_device *pdev)
> > > +{
> > > + struct dw_pcie *dw = &rcar->dw;
> > > +
> > > + /* Renesas-specific registers */
> > > + rcar->base = devm_platform_ioremap_resource_byname(pdev, "app");
> > > + if (IS_ERR(rcar->base))
> > > + return PTR_ERR(rcar->base);
> > > +
> > > + return rcar_gen4_pcie_devm_reset_get(rcar, dw->dev);
> > > +}
> >
> > Please note that after fixing the RCAR PCIe EP DT-bindings to permit
> > the "app" instead of "appl" named reg property (see my note the RCAR
> > PCie EP DT-bindings patch) the method above will be identical in the
> > RCAR RP and EP driver implementations. So you'll be able to move it
> > to the pcie-rcar-gen4.c driver.
>
> I think so. I'll modify it.
>
> > > +
> > > +static int rcar_gen4_pcie_probe(struct platform_device *pdev)
> > > +{
> > > + struct device *dev = &pdev->dev;
> > > + struct rcar_gen4_pcie *rcar;
> > > + int err;
> > > +
> > > + rcar = rcar_gen4_pcie_devm_alloc(dev);
> > > + if (!rcar)
> > > + return -ENOMEM;
> > > +
> > > + err = rcar_gen4_pcie_get_resources(rcar, pdev);
> > > + if (err < 0) {
> > > + dev_err(dev, "Failed to request resource: %d\n", err);
> > > + return err;
> > > + }
> > > +
> > > + platform_set_drvdata(pdev, rcar);
> > > +
> > > + err = rcar_gen4_pcie_prepare(rcar);
> > > + if (err < 0)
> > > + return err;
> > > +
> > > + err = rcar_gen4_add_dw_pcie_rp(rcar, pdev);
> > > + if (err < 0)
> > > + goto err_add;
> > > +
> > > + return 0;
> > > +
> > > +err_add:
> > > + rcar_gen4_pcie_unprepare(rcar);
> > > +
> > > + return err;
> > > +}
> > > +
> > > +static int rcar_gen4_pcie_remove(struct platform_device *pdev)
> > > +{
> > > + struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
> > > +
> > > + rcar_gen4_remove_dw_pcie_rp(rcar);
> > > + rcar_gen4_pcie_unprepare(rcar);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static const struct of_device_id rcar_gen4_pcie_of_match[] = {
> > > + { .compatible = "renesas,rcar-gen4-pcie", },
> > > + {},
> > > +};
> > > +
> > > +static struct platform_driver rcar_gen4_pcie_driver = {
> > > + .driver = {
> > > + .name = "pcie-rcar-gen4",
> > > + .of_match_table = rcar_gen4_pcie_of_match,
> > > + },
> > > + .probe = rcar_gen4_pcie_probe,
> > > + .remove = rcar_gen4_pcie_remove,
> > > +};
> > > +module_platform_driver(rcar_gen4_pcie_driver);
> > > +
> > > +MODULE_DESCRIPTION("Renesas R-Car Gen4 PCIe host controller driver");
> > > +MODULE_LICENSE("GPL");
> > > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > > new file mode 100644
> > > index 000000000000..a6a29d6125c8
> > > --- /dev/null
> > > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > > @@ -0,0 +1,166 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> > > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > > + */
> > > +
> > > +#include <linux/io.h>
> > > +#include <linux/of_device.h>
> > > +#include <linux/pci.h>
> > > +#include <linux/pm_runtime.h>
> > > +#include <linux/reset.h>
> > > +
> > > +#include "pcie-rcar-gen4.h"
> > > +#include "pcie-designware.h"
> > > +
> > > +/* Renesas-specific */
> > > +#define PCIERSTCTRL1 0x0014
> > > +#define APP_HOLD_PHY_RST BIT(16)
> > > +#define APP_LTSSM_ENABLE BIT(0)
> > > +
> > > +static void rcar_gen4_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar,
> > > + bool enable)
> > > +{
> > > + u32 val;
> > > +
> > > + val = readl(rcar->base + PCIERSTCTRL1);
> > > + if (enable) {
> > > + val |= APP_LTSSM_ENABLE;
> > > + val &= ~APP_HOLD_PHY_RST;
> > > + } else {
> > > + val &= ~APP_LTSSM_ENABLE;
> > > + val |= APP_HOLD_PHY_RST;
> > > + }
> > > + writel(val, rcar->base + PCIERSTCTRL1);
> > > +}
> > > +
> > > +static int rcar_gen4_pcie_link_up(struct dw_pcie *dw)
> > > +{
> > > + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > > + u32 val, mask;
> > > +
> > > + val = readl(rcar->base + PCIEINTSTS0);
> > > + mask = RDLH_LINK_UP | SMLH_LINK_UP;
> > > +
> > > + return (val & mask) == mask;
> > > +}
> > > +
> > > +static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
> > > +{
> > > + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > > +
> > > + rcar_gen4_pcie_ltssm_enable(rcar, true);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static void rcar_gen4_pcie_stop_link(struct dw_pcie *dw)
> > > +{
> > > + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > > +
> > > + rcar_gen4_pcie_ltssm_enable(rcar, false);
> > > +}
> > > +
> > > +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> > > + int num_lanes)
> > > +{
> > > + u32 val;
> > > +
> > > + /* Note: Assume the reset is asserted here */
> > > + val = readl(rcar->base + PCIEMSR0);
> > > + if (rc)
> > > + val |= DEVICE_TYPE_RC;
> > > + else
> > > + val |= DEVICE_TYPE_EP;
> > > + if (num_lanes < 4)
> > > + val |= BIFUR_MOD_SET_ON;
> > > + writel(val, rcar->base + PCIEMSR0);
> > > +
> > > + return reset_control_deassert(rcar->rst);
> > > +}
> > > +
> >
> > > +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg)
> > > +{
> > > + dw_pcie_writel_dbi(dw, SHADOW_REG(bar_mask_reg), 0x0);
> > > +}
> > > +
> >
> > Hm, this seems like a DBI2/DBI_CS2 CSRs. By default the DW PCIe core
> > driver assumes that the DBI2 is defined within the 0x1000 offset with
> > respect to the DBI space (see dw_pcie_get_resources()). In your case
> > it must be within 0x2000 offset (judging by the SHADOW_REG() macro).
> > What about either defining the "dbi2" reg-named DT-property in the
> > DT-bindings (see DW PCIe RP/EP generic DT-bindings) or re-defining
> > the dw_pcie.dbi_base2 field in your LLDD?
> >
> > Note the dbi_cs2 space is currently utilized in the DW PCIe EP driver
> > only in the framework of the BARs mapping setup procedure. Are you
> > sure it's working well in your case seeing the DBI2 base address is
> > most likely incorrectly initialized? Anyway if you get to define the
> > dbi_base2 field properly you'll be able to use the
> > dw_pcie_ep_reset_bar() method in the EP driver instead of the function
> > above (please see the way it's utilized in the rest of the DW PCIe EP
> > low-level drivers).
>
> Thank you for your comments. I'll investigate it.
>
> > Another question. Are you sure the method above is relevant to the DW
> > PCIe Root Port controller? I don't see any shadow registers defined
> > for the host BARs in any HW-manual I've got (4.60, 4.70, 4.90, 5.20,
> > 5.30, 5.40). What are they used for anyway?
>
> To be honest, I reuse this code from our BSP which other guy made.
> So, I'll investigate it.
>
> > > +void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *dw, int num_lanes)
> > > +{
> > > + u32 val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_LNKCAP));
> > > +
> > > + val &= ~PCI_EXP_LNKCAP_MLW;
> > > + switch (num_lanes) {
> > > + case 1:
> > > + val |= PCI_EXP_LNKCAP_MLW_X1;
> > > + break;
> > > + case 2:
> > > + val |= PCI_EXP_LNKCAP_MLW_X2;
> > > + break;
> > > + case 4:
> > > + val |= PCI_EXP_LNKCAP_MLW_X4;
> > > + break;
> > > + default:
> > > + dev_info(dw->dev, "Invalid num-lanes %d\n", num_lanes);
> > > + val |= PCI_EXP_LNKCAP_MLW_X1;
> > > + break;
> > > + }
> > > + dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_LNKCAP), val);
> > > +}
> >
> > This seems to be a generic action. What about moving it to
> > pcie-designware.c implementing a function similar to
> > dw_pcie_link_set_max_speed(), which would be called in
> > dw_pcie_setup()?
>
> Rob also suggests it. So, I'll investigate it.
>
> > It could be named as dw_pcie_link_set_max_(link_)?width() and besides
> > of the code above would consist of the PCIE_PORT_LINK_CONTROL and
> > PCIE_LINK_WIDTH_SPEED_CONTROL CSR initializations performed in
> > dw_pcie_setup().
>
> Thank you for your comments.
>
> > * Please do that in a separate pre-requisite patch.
> >
> > > +
> > > +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *rcar)
> > > +{
> > > + struct device *dev = rcar->dw.dev;
> > > + int err;
> > > +
> > > + pm_runtime_enable(dev);
> > > + err = pm_runtime_resume_and_get(dev);
> > > + if (err < 0) {
> > > + dev_err(dev, "Failed to resume/get Runtime PM\n");
> > > + pm_runtime_disable(dev);
> > > + }
> > > +
> > > + return err;
> > > +}
> > > +
> > > +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
> > > +{
> > > + struct device *dev = rcar->dw.dev;
> > > +
> > > + if (!reset_control_status(rcar->rst))
> > > + reset_control_assert(rcar->rst);
> > > + pm_runtime_put(dev);
> > > + pm_runtime_disable(dev);
> > > +}
> > > +
> > > +int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *rcar,
> > > + struct device *dev)
> > > +{
> > > + rcar->rst = devm_reset_control_get(dev, NULL);
> > > + if (IS_ERR(rcar->rst)) {
> > > + dev_err(dev, "Failed to get Cold-reset\n");
> > > + return PTR_ERR(rcar->rst);
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static const struct dw_pcie_ops dw_pcie_ops = {
> > > + .start_link = rcar_gen4_pcie_start_link,
> > > + .stop_link = rcar_gen4_pcie_stop_link,
> > > + .link_up = rcar_gen4_pcie_link_up,
> > > +};
> > > +
> > > +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev)
> > > +{
> > > + struct rcar_gen4_pcie *rcar;
> > > +
> > > + rcar = devm_kzalloc(dev, sizeof(*rcar), GFP_KERNEL);
> > > + if (!rcar)
> > > + return NULL;
> > > +
> > > + rcar->dw.dev = dev;
> > > + rcar->dw.ops = &dw_pcie_ops;
> > > + dw_pcie_cap_set(&rcar->dw, EDMA_UNROLL);
> > > +
> > > + return rcar;
> > > +}
> > > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.h b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> > > new file mode 100644
> > > index 000000000000..1cdce0cf7dac
> > > --- /dev/null
> > > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> > > @@ -0,0 +1,63 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +/*
> > > + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> > > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > > + */
> > > +
> > > +#ifndef _PCIE_RCAR_GEN4_H_
> > > +#define _PCIE_RCAR_GEN4_H_
> > > +
> > > +#include <linux/io.h>
> > > +#include <linux/pci.h>
> > > +#include <linux/reset.h>
> > > +
> > > +#include "pcie-designware.h"
> > > +
> >
> > > +/* PCI Express capability */
> > > +#define EXPCAP(x) (0x0070 + (x))
> > > +/* ASPM L1 PM Substates */
> > > +#define L1PSCAP(x) (0x01bc + (x))
> > > +/* PCI Shadow offset */
> > > +#define SHADOW_REG(x) (0x2000 + (x))
> > > +/* BAR Mask registers */
> > > +#define BAR0MASKF 0x0010
> > > +#define BAR1MASKF 0x0014
> > > +#define BAR2MASKF 0x0018
> > > +#define BAR3MASKF 0x001c
> > > +#define BAR4MASKF 0x0020
> > > +#define BAR5MASKF 0x0024
> > > +
> >
> > * If you get to take some of my notes above into account you'll be able to
> > drop these macros...
>
> I got it.
>
> > > +/* Renesas-specific */
> > > +#define PCIEMSR0 0x0000
> > > +#define BIFUR_MOD_SET_ON BIT(0)
> > > +#define DEVICE_TYPE_EP 0
> > > +#define DEVICE_TYPE_RC BIT(4)
> > > +
> > > +#define PCIEINTSTS0 0x0084
> > > +#define PCIEINTSTS0EN 0x0310
> > > +#define MSI_CTRL_INT BIT(26)
> > > +#define SMLH_LINK_UP BIT(7)
> > > +#define RDLH_LINK_UP BIT(6)
> > > +#define PCIEDMAINTSTSEN 0x0314
> > > +#define PCIEDMAINTSTSEN_INIT GENMASK(15, 0)
> >
> > Vendor-specific prefix would make the code using these macros much
> > more readable.
>
> I got it. I'll rename them.
>
> > > +
> > > +struct rcar_gen4_pcie {
> > > + struct dw_pcie dw;
> > > + void __iomem *base;
> > > + struct reset_control *rst;
> > > +};
> > > +#define to_rcar_gen4_pcie(x) dev_get_drvdata((x)->dev)
> > > +
> >
> > > +u32 rcar_gen4_pcie_readl(struct rcar_gen4_pcie *pcie, u32 reg);
> > > +void rcar_gen4_pcie_writel(struct rcar_gen4_pcie *pcie, u32 reg, u32 val);
> >
> > I don't see these two methods being defined in your driver. Are these
> > artefacts from the previous patch revisions?
>
> Oops. I'll remove them.
>
> Best regards,
> Yoshihiro Shimoda
>
> > -Serge(y)
> >
> > > +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> > > + int num_lanes);
> >
> > > +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg);
> > > +void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *pci, int num_lanes);
> > > +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *pcie);
> > > +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *pcie);
> > > +int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *pcie,
> > > + struct device *dev);
> > > +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev);
> > > +
> > > +#endif /* _PCIE_RCAR_GEN4_H_ */
> > > --
> > > 2.25.1
> > >
> > >
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v9 7/8] PCI: rcar-gen4-ep: Add R-Car Gen4 PCIe Endpoint support
2023-02-10 13:49 [PATCH v9 0/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
` (5 preceding siblings ...)
2023-02-10 13:49 ` [PATCH v9 6/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support Yoshihiro Shimoda
@ 2023-02-10 13:49 ` Yoshihiro Shimoda
2023-02-10 13:49 ` [PATCH v9 8/8] MAINTAINERS: Update PCI DRIVER FOR RENESAS R-CAR for R-Car Gen4 Yoshihiro Shimoda
7 siblings, 0 replies; 20+ messages in thread
From: Yoshihiro Shimoda @ 2023-02-10 13:49 UTC (permalink / raw)
To: lpieralisi, robh+dt, kw, bhelgaas, jingoohan1, gustavo.pimentel
Cc: Sergey.Semin, marek.vasut+renesas, linux-pci, devicetree,
linux-renesas-soc, Yoshihiro Shimoda
Add R-Car Gen4 PCIe Endpoint support. This controller is based on
Synopsys DesignWare PCIe.
This controller requires vender-specific initialization before
.ep_init(). To use dw->dbi and dw->num-lanes in the initialization
code, introduce .ep_pre_init() into struct dw_pcie_ep_ops.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
drivers/pci/controller/dwc/Kconfig | 9 +
drivers/pci/controller/dwc/Makefile | 2 +
.../pci/controller/dwc/pcie-designware-ep.c | 3 +
drivers/pci/controller/dwc/pcie-designware.h | 1 +
.../pci/controller/dwc/pcie-rcar-gen4-ep.c | 185 ++++++++++++++++++
5 files changed, 200 insertions(+)
create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c
diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 94805ec31a8f..f33e403c7b61 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -423,4 +423,13 @@ config PCIE_RCAR_GEN4
Say Y here if you want PCIe host controller support on R-Car Gen4 SoCs.
This uses the DesignWare core.
+config PCIE_RCAR_GEN4_EP
+ tristate "Renesas R-Car Gen4 PCIe Endpoint controller"
+ depends on ARCH_RENESAS || COMPILE_TEST
+ depends on PCI_ENDPOINT
+ select PCIE_DW_EP
+ help
+ Say Y here if you want PCIe endpoint controller support on R-Car Gen4
+ SoCs. This uses the DesignWare core.
+
endmenu
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index 486cf706b53d..0fb0bde26ac4 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -28,6 +28,8 @@ obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
pcie-rcar-gen4-host-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-host.o
obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4-host-drv.o
+pcie-rcar-gen4-ep-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-ep.o
+obj-$(CONFIG_PCIE_RCAR_GEN4_EP) += pcie-rcar-gen4-ep-drv.o
# The following drivers are for devices that use the generic ACPI
# pci_root.c driver but don't support standard ECAM config access.
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 886483bf378b..6ce418fd3bfe 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -763,6 +763,9 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
ep->phys_base = res->start;
ep->addr_size = resource_size(res);
+ if (ep->ops->ep_pre_init)
+ ep->ops->ep_pre_init(ep);
+
dw_pcie_version_detect(pci);
dw_pcie_iatu_detect(pci);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 1b1af514b849..63b272587eec 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -326,6 +326,7 @@ struct dw_pcie_rp {
};
struct dw_pcie_ep_ops {
+ void (*ep_pre_init)(struct dw_pcie_ep *ep);
void (*ep_init)(struct dw_pcie_ep *ep);
int (*raise_irq)(struct dw_pcie_ep *ep, u8 func_no,
enum pci_epc_irq_type type, u16 interrupt_num);
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c b/drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c
new file mode 100644
index 000000000000..ecffbac62f85
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c
@@ -0,0 +1,185 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PCIe Endpoint driver for Renesas R-Car Gen4 Series SoCs
+ * Copyright (C) 2022-2023 Renesas Electronics Corporation
+ */
+
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+
+#include "pcie-rcar-gen4.h"
+#include "pcie-designware.h"
+
+/* Configuration */
+#define PCICONF3 0x000c
+#define MULTI_FUNC BIT(23)
+
+static void rcar_gen4_pcie_ep_pre_init(struct dw_pcie_ep *ep)
+{
+ struct dw_pcie *dw = to_dw_pcie_from_ep(ep);
+ struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
+ int val;
+
+ rcar_gen4_pcie_set_device_type(rcar, false, dw->num_lanes);
+
+ dw_pcie_dbi_ro_wr_en(dw);
+
+ /* Single function */
+ val = dw_pcie_readl_dbi(dw, PCICONF3);
+ val &= ~MULTI_FUNC;
+ dw_pcie_writel_dbi(dw, PCICONF3, val);
+
+ rcar_gen4_pcie_disable_bar(dw, BAR5MASKF);
+
+ rcar_gen4_pcie_set_max_link_width(dw, dw->num_lanes);
+
+ dw_pcie_dbi_ro_wr_dis(dw);
+}
+
+static int rcar_gen4_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
+ enum pci_epc_irq_type type,
+ u16 interrupt_num)
+{
+ struct dw_pcie *dw = to_dw_pcie_from_ep(ep);
+
+ switch (type) {
+ case PCI_EPC_IRQ_LEGACY:
+ return dw_pcie_ep_raise_legacy_irq(ep, func_no);
+ case PCI_EPC_IRQ_MSI:
+ return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
+ default:
+ dev_err(dw->dev, "UNKNOWN IRQ type\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static const struct pci_epc_features rcar_gen4_pcie_epc_features = {
+ .linkup_notifier = false,
+ .msi_capable = true,
+ .msix_capable = false,
+ .reserved_bar = 1 << BAR_5,
+ .align = SZ_1M,
+};
+
+static const struct pci_epc_features*
+rcar_gen4_pcie_ep_get_features(struct dw_pcie_ep *ep)
+{
+ return &rcar_gen4_pcie_epc_features;
+}
+
+static const struct dw_pcie_ep_ops pcie_ep_ops = {
+ .ep_pre_init = rcar_gen4_pcie_ep_pre_init,
+ .raise_irq = rcar_gen4_pcie_ep_raise_irq,
+ .get_features = rcar_gen4_pcie_ep_get_features,
+};
+
+static int rcar_gen4_add_pcie_ep(struct rcar_gen4_pcie *rcar,
+ struct platform_device *pdev)
+{
+ struct dw_pcie *dw = &rcar->dw;
+ struct dw_pcie_ep *ep;
+ int ret;
+
+ ep = &dw->ep;
+ ep->ops = &pcie_ep_ops;
+ ep->intx_by_atu = true;
+
+ ret = dw_pcie_ep_init(ep);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to initialize endpoint\n");
+ return ret;
+ }
+
+ writel(PCIEDMAINTSTSEN_INIT, rcar->base + PCIEDMAINTSTSEN);
+
+ dw->ops->start_link(dw);
+
+ return 0;
+}
+
+static void rcar_gen4_remove_pcie_ep(struct rcar_gen4_pcie *rcar)
+{
+ writel(0, rcar->base + PCIEDMAINTSTSEN);
+ dw_pcie_ep_exit(&rcar->dw.ep);
+}
+
+static int rcar_gen4_pcie_ep_get_resources(struct rcar_gen4_pcie *rcar,
+ struct platform_device *pdev)
+{
+ struct dw_pcie *dw = &rcar->dw;
+ struct device *dev = dw->dev;
+
+ /* Renesas-specific registers */
+ rcar->base = devm_platform_ioremap_resource_byname(pdev, "appl");
+ if (IS_ERR(rcar->base))
+ return PTR_ERR(rcar->base);
+
+ return rcar_gen4_pcie_devm_reset_get(rcar, dev);
+}
+
+static int rcar_gen4_pcie_ep_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct rcar_gen4_pcie *rcar;
+ int err;
+
+ rcar = rcar_gen4_pcie_devm_alloc(dev);
+ if (!rcar)
+ return -ENOMEM;
+
+ err = rcar_gen4_pcie_ep_get_resources(rcar, pdev);
+ if (err < 0) {
+ dev_err(dev, "failed to request resource: %d\n", err);
+ return err;
+ }
+
+ platform_set_drvdata(pdev, rcar);
+
+ err = rcar_gen4_pcie_prepare(rcar);
+ if (err < 0)
+ return err;
+
+ err = rcar_gen4_add_pcie_ep(rcar, pdev);
+ if (err < 0)
+ goto err_add;
+
+ return 0;
+
+err_add:
+ rcar_gen4_pcie_unprepare(rcar);
+
+ return err;
+}
+
+static int rcar_gen4_pcie_ep_remove(struct platform_device *pdev)
+{
+ struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
+
+ rcar_gen4_remove_pcie_ep(rcar);
+ rcar_gen4_pcie_unprepare(rcar);
+
+ return 0;
+}
+
+static const struct of_device_id rcar_gen4_pcie_of_match[] = {
+ { .compatible = "renesas,rcar-gen4-pcie-ep", },
+ {},
+};
+
+static struct platform_driver rcar_gen4_pcie_ep_driver = {
+ .driver = {
+ .name = "pcie-rcar-gen4-ep",
+ .of_match_table = rcar_gen4_pcie_of_match,
+ },
+ .probe = rcar_gen4_pcie_ep_probe,
+ .remove = rcar_gen4_pcie_ep_remove,
+};
+module_platform_driver(rcar_gen4_pcie_ep_driver);
+
+MODULE_DESCRIPTION("Renesas R-Car Gen4 PCIe endpoint controller driver");
+MODULE_LICENSE("GPL");
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v9 8/8] MAINTAINERS: Update PCI DRIVER FOR RENESAS R-CAR for R-Car Gen4
2023-02-10 13:49 [PATCH v9 0/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
` (6 preceding siblings ...)
2023-02-10 13:49 ` [PATCH v9 7/8] PCI: rcar-gen4-ep: Add R-Car Gen4 PCIe Endpoint support Yoshihiro Shimoda
@ 2023-02-10 13:49 ` Yoshihiro Shimoda
7 siblings, 0 replies; 20+ messages in thread
From: Yoshihiro Shimoda @ 2023-02-10 13:49 UTC (permalink / raw)
To: lpieralisi, robh+dt, kw, bhelgaas, jingoohan1, gustavo.pimentel
Cc: Sergey.Semin, marek.vasut+renesas, linux-pci, devicetree,
linux-renesas-soc, Yoshihiro Shimoda
Update this entry for R-Car Gen4's source code.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 57d1d6ecb33d..d5392527d073 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16001,6 +16001,7 @@ L: linux-renesas-soc@vger.kernel.org
S: Maintained
F: Documentation/devicetree/bindings/pci/*rcar*
F: drivers/pci/controller/*rcar*
+F: drivers/pci/controller/dwc/*rcar*
PCI DRIVER FOR SAMSUNG EXYNOS
M: Jingoo Han <jingoohan1@gmail.com>
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread