* [PATCH v7 1/9] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Host
2022-11-21 12:43 [PATCH v7 0/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
@ 2022-11-21 12:43 ` Yoshihiro Shimoda
2022-11-21 12:43 ` [PATCH v7 2/9] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Endpoint Yoshihiro Shimoda
` (7 subsequent siblings)
8 siblings, 0 replies; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-21 12:43 UTC (permalink / raw)
To: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt
Cc: 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] 27+ messages in thread* [PATCH v7 2/9] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Endpoint
2022-11-21 12:43 [PATCH v7 0/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
2022-11-21 12:43 ` [PATCH v7 1/9] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Host Yoshihiro Shimoda
@ 2022-11-21 12:43 ` Yoshihiro Shimoda
2022-11-21 12:43 ` [PATCH v7 3/9] PCI: Add PCI_EXP_LNKCAP_MLW macros Yoshihiro Shimoda
` (6 subsequent siblings)
8 siblings, 0 replies; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-21 12:43 UTC (permalink / raw)
To: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt
Cc: 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] 27+ messages in thread* [PATCH v7 3/9] PCI: Add PCI_EXP_LNKCAP_MLW macros
2022-11-21 12:43 [PATCH v7 0/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
2022-11-21 12:43 ` [PATCH v7 1/9] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Host Yoshihiro Shimoda
2022-11-21 12:43 ` [PATCH v7 2/9] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Endpoint Yoshihiro Shimoda
@ 2022-11-21 12:43 ` Yoshihiro Shimoda
2022-11-21 12:43 ` [PATCH v7 4/9] PCI: designware-ep: Expose dw_pcie_ep_exit() to module Yoshihiro Shimoda
` (5 subsequent siblings)
8 siblings, 0 replies; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-21 12:43 UTC (permalink / raw)
To: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt
Cc: 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 1c3591c8e09e..31216c6a07ad 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] 27+ messages in thread* [PATCH v7 4/9] PCI: designware-ep: Expose dw_pcie_ep_exit() to module
2022-11-21 12:43 [PATCH v7 0/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
` (2 preceding siblings ...)
2022-11-21 12:43 ` [PATCH v7 3/9] PCI: Add PCI_EXP_LNKCAP_MLW macros Yoshihiro Shimoda
@ 2022-11-21 12:43 ` Yoshihiro Shimoda
2022-11-21 12:43 ` [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists Yoshihiro Shimoda
` (4 subsequent siblings)
8 siblings, 0 replies; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-21 12:43 UTC (permalink / raw)
To: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt
Cc: 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] 27+ messages in thread* [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
2022-11-21 12:43 [PATCH v7 0/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
` (3 preceding siblings ...)
2022-11-21 12:43 ` [PATCH v7 4/9] PCI: designware-ep: Expose dw_pcie_ep_exit() to module Yoshihiro Shimoda
@ 2022-11-21 12:43 ` Yoshihiro Shimoda
2022-11-22 13:55 ` Manivannan Sadhasivam
2022-11-21 12:43 ` [PATCH v7 6/9] PCI: dwc: Add support for triggering legacy IRQs Yoshihiro Shimoda
` (3 subsequent siblings)
8 siblings, 1 reply; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-21 12:43 UTC (permalink / raw)
To: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt
Cc: marek.vasut+renesas, linux-pci, devicetree, linux-renesas-soc,
Yoshihiro Shimoda
Since reading value of PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL was
0x00000000 on one of SoCs (R-Car S4-8), it cannot find the eDMA.
So, directly read the eDMA register if edma.reg_base is not zero.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
drivers/pci/controller/dwc/pcie-designware.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 637d01807c67..2cc8584da6f4 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -836,8 +836,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
{
u32 val;
- val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
- if (val == 0xFFFFFFFF && pci->edma.reg_base) {
+ if (pci->edma.reg_base) {
pci->edma.mf = EDMA_MF_EDMA_UNROLL;
val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
@@ -845,6 +844,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
pci->edma.mf = EDMA_MF_EDMA_LEGACY;
pci->edma.reg_base = pci->dbi_base + PCIE_DMA_VIEWPORT_BASE;
+ val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
} else {
return -ENODEV;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
2022-11-21 12:43 ` [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists Yoshihiro Shimoda
@ 2022-11-22 13:55 ` Manivannan Sadhasivam
2022-11-27 23:55 ` Serge Semin
0 siblings, 1 reply; 27+ messages in thread
From: Manivannan Sadhasivam @ 2022-11-22 13:55 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt, marek.vasut+renesas,
linux-pci, devicetree, linux-renesas-soc, fancer.lancer,
Sergey.Semin
+ Serge (who authored EDMA support)
Thanks,
Mani
On Mon, Nov 21, 2022 at 09:43:56PM +0900, Yoshihiro Shimoda wrote:
> Since reading value of PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL was
> 0x00000000 on one of SoCs (R-Car S4-8), it cannot find the eDMA.
> So, directly read the eDMA register if edma.reg_base is not zero.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
> drivers/pci/controller/dwc/pcie-designware.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 637d01807c67..2cc8584da6f4 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -836,8 +836,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> {
> u32 val;
>
> - val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> - if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> + if (pci->edma.reg_base) {
> pci->edma.mf = EDMA_MF_EDMA_UNROLL;
>
> val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
> @@ -845,6 +844,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> pci->edma.mf = EDMA_MF_EDMA_LEGACY;
>
> pci->edma.reg_base = pci->dbi_base + PCIE_DMA_VIEWPORT_BASE;
> + val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> } else {
> return -ENODEV;
> }
> --
> 2.25.1
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
2022-11-22 13:55 ` Manivannan Sadhasivam
@ 2022-11-27 23:55 ` Serge Semin
2022-11-28 2:52 ` Yoshihiro Shimoda
0 siblings, 1 reply; 27+ messages in thread
From: Serge Semin @ 2022-11-27 23:55 UTC (permalink / raw)
To: Manivannan Sadhasivam, Yoshihiro Shimoda
Cc: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt, marek.vasut+renesas,
linux-pci, devicetree, linux-renesas-soc, Sergey.Semin
On Tue, Nov 22, 2022 at 07:25:50PM +0530, Manivannan Sadhasivam wrote:
> + Serge (who authored EDMA support)
Thanks @Mani. It's strange to see a fix for a patch which hasn't been even
merged in yet and miss the patch author in the Cc list.)
@Yoshihiro, on the next patchset revisions please don't forget to add
my email address to the copy list.
>
> Thanks,
> Mani
>
> On Mon, Nov 21, 2022 at 09:43:56PM +0900, Yoshihiro Shimoda wrote:
> > Since reading value of PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL was
> > 0x00000000 on one of SoCs (R-Car S4-8), it cannot find the eDMA.
> > So, directly read the eDMA register if edma.reg_base is not zero.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> > drivers/pci/controller/dwc/pcie-designware.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 637d01807c67..2cc8584da6f4 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -836,8 +836,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > {
> > u32 val;
> >
> > - val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> > - if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > + if (pci->edma.reg_base) {
> > pci->edma.mf = EDMA_MF_EDMA_UNROLL;
> >
> > val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
> > @@ -845,6 +844,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > pci->edma.mf = EDMA_MF_EDMA_LEGACY;
> >
> > pci->edma.reg_base = pci->dbi_base + PCIE_DMA_VIEWPORT_BASE;
> > + val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
Look what you suggest here:
< u32 val;
< ...
< if (pci->edma.reg_base) {
< ...
< } else if (val != 0xFFFFFFFF) {
< ...
< } else {
< ...
It would be strange if your compiler didn't warn about 'val' being used
uninitialized here, which in its turn would introduce a regression for
the platforms with the indirectly accessible eDMA registers.
Anyway you can't just drop something what didn't work for you
hardware. The method you suggest to fix here works fine for multiple
DW PCIe IP-cores. Judging by the HW manuals it should work at least up
to v5.30a. Are you sure that your controller is of v5.20a? I see you
overwrite the IP-core version for the PCIe host driver only. Why is
that necessary? Does the version auto-detection procedure work
incorrectly for you? What does the dbi+0x8f8 CSR contain in the host
and EP registers space? Similarly could you also provide a content of
the +0x978 register?
-Sergey
> > } else {
> > return -ENODEV;
> > }
> > --
> > 2.25.1
> >
>
> --
> மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 27+ messages in thread* RE: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
2022-11-27 23:55 ` Serge Semin
@ 2022-11-28 2:52 ` Yoshihiro Shimoda
2022-11-28 11:59 ` Serge Semin
0 siblings, 1 reply; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-28 2:52 UTC (permalink / raw)
To: Serge Semin, Manivannan Sadhasivam
Cc: lpieralisi@kernel.org, robh+dt@kernel.org, kw@linux.com,
bhelgaas@google.com, krzk+dt@kernel.org,
marek.vasut+renesas@gmail.com, linux-pci@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
Sergey.Semin@baikalelectronics.ru
Hi Serge,
> From: Serge Semin, Sent: Monday, November 28, 2022 8:56 AM
>
> On Tue, Nov 22, 2022 at 07:25:50PM +0530, Manivannan Sadhasivam wrote:
> > + Serge (who authored EDMA support)
>
> Thanks @Mani. It's strange to see a fix for a patch which hasn't been even
> merged in yet and miss the patch author in the Cc list.)
>
> @Yoshihiro, on the next patchset revisions please don't forget to add
> my email address to the copy list.
Sure! I'll add your email address on the next patchset revisions.
> > Thanks,
> > Mani
> >
> > On Mon, Nov 21, 2022 at 09:43:56PM +0900, Yoshihiro Shimoda wrote:
> > > Since reading value of PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL was
> > > 0x00000000 on one of SoCs (R-Car S4-8), it cannot find the eDMA.
> > > So, directly read the eDMA register if edma.reg_base is not zero.
> > >
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > ---
> > > drivers/pci/controller/dwc/pcie-designware.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > > index 637d01807c67..2cc8584da6f4 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > @@ -836,8 +836,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > > {
> > > u32 val;
> > >
>
> > > - val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> > > - if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > > + if (pci->edma.reg_base) {
> > > pci->edma.mf = EDMA_MF_EDMA_UNROLL;
> > >
> > > val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
> > > @@ -845,6 +844,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > > pci->edma.mf = EDMA_MF_EDMA_LEGACY;
> > >
> > > pci->edma.reg_base = pci->dbi_base + PCIE_DMA_VIEWPORT_BASE;
> > > + val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
>
> Look what you suggest here:
> < u32 val;
> < ...
> < if (pci->edma.reg_base) {
> < ...
> < } else if (val != 0xFFFFFFFF) {
> < ...
> < } else {
> < ...
>
> It would be strange if your compiler didn't warn about 'val' being used
> uninitialized here, which in its turn would introduce a regression for
> the platforms with the indirectly accessible eDMA registers.
You're correct. It's strange. I don't know why my using compiler [1] didn't
warn about the 'val' though...
[1]
https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.1.0/x86_64-gcc-11.1.0-nolibc-aarch64-linux.tar.xz
> Anyway you can't just drop something what didn't work for you
> hardware. The method you suggest to fix here works fine for multiple
> DW PCIe IP-cores. Judging by the HW manuals it should work at least up
> to v5.30a. Are you sure that your controller is of v5.20a? I see you
> overwrite the IP-core version for the PCIe host driver only. Why is
> that necessary? Does the version auto-detection procedure work
> incorrectly for you?
Thank you for pointed it out! I realized that overwriting the IP-core
Version was not needed. So, I'll drop it on v8.
---
#define DWC_VERSION 0x520a
...
struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev)
{
...
rcar->dw.version = DWC_VERSION;
---
> What does the dbi+0x8f8 CSR contain in the host
> and EP registers space? Similarly could you also provide a content of
> the +0x978 register?
The dbi+0x8f8 and the +0x978 registers' values are 0x00000000.
--------------- (sorry, replace tabs with spaces...)---------------
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -843,6 +843,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
{
u32 val;
+ dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
+ dw_pcie_readl_dma(pci, 0x8f8),
+ dw_pcie_readl_dma(pci, 0x978));
+
if (pci->edma.reg_base) {
pci->edma.mf = EDMA_MF_EDMA_UNROLL;
-------------------------------------------------------------------
----- Output -----
# dmesg | grep edma
[ 1.108709] pcie-rcar-gen4 e65d0000.pcie: dw_pcie_edma_find_chip: +0x8f8 = 00000000, +0x978 = 00000000
------------------
So, should I change the condition like below?
---
- if (val == 0xFFFFFFFF && pci->edma.reg_base) {
+ if ((val == 0xFFFFFFFF || val == 0x00000000) && pci->edma.reg_base) {
...
- } else if (val != 0xFFFFFFFF) {
- } else if (!(val == 0xFFFFFFFF || val == 0x00000000)) {
---
Best regards,
Yoshihiro Shimoda
> -Sergey
>
> > > } else {
> > > return -ENODEV;
> > > }
> > > --
> > > 2.25.1
> > >
> >
> > --
> > மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
2022-11-28 2:52 ` Yoshihiro Shimoda
@ 2022-11-28 11:59 ` Serge Semin
2022-11-28 12:41 ` Yoshihiro Shimoda
0 siblings, 1 reply; 27+ messages in thread
From: Serge Semin @ 2022-11-28 11:59 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Manivannan Sadhasivam, lpieralisi@kernel.org, robh+dt@kernel.org,
kw@linux.com, bhelgaas@google.com, krzk+dt@kernel.org,
marek.vasut+renesas@gmail.com, linux-pci@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
Sergey.Semin@baikalelectronics.ru
On Mon, Nov 28, 2022 at 02:52:56AM +0000, Yoshihiro Shimoda wrote:
> Hi Serge,
>
> > From: Serge Semin, Sent: Monday, November 28, 2022 8:56 AM
> >
> > On Tue, Nov 22, 2022 at 07:25:50PM +0530, Manivannan Sadhasivam wrote:
> > > + Serge (who authored EDMA support)
> >
> > Thanks @Mani. It's strange to see a fix for a patch which hasn't been even
> > merged in yet and miss the patch author in the Cc list.)
> >
> > @Yoshihiro, on the next patchset revisions please don't forget to add
> > my email address to the copy list.
>
> Sure! I'll add your email address on the next patchset revisions.
>
> > > Thanks,
> > > Mani
> > >
> > > On Mon, Nov 21, 2022 at 09:43:56PM +0900, Yoshihiro Shimoda wrote:
> > > > Since reading value of PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL was
> > > > 0x00000000 on one of SoCs (R-Car S4-8), it cannot find the eDMA.
> > > > So, directly read the eDMA register if edma.reg_base is not zero.
> > > >
> > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > > ---
> > > > drivers/pci/controller/dwc/pcie-designware.c | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > > > index 637d01807c67..2cc8584da6f4 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > > @@ -836,8 +836,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > > > {
> > > > u32 val;
> > > >
> >
> > > > - val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> > > > - if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > > > + if (pci->edma.reg_base) {
> > > > pci->edma.mf = EDMA_MF_EDMA_UNROLL;
> > > >
> > > > val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
> > > > @@ -845,6 +844,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > > > pci->edma.mf = EDMA_MF_EDMA_LEGACY;
> > > >
> > > > pci->edma.reg_base = pci->dbi_base + PCIE_DMA_VIEWPORT_BASE;
> > > > + val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> >
> > Look what you suggest here:
> > < u32 val;
> > < ...
> > < if (pci->edma.reg_base) {
> > < ...
> > < } else if (val != 0xFFFFFFFF) {
> > < ...
> > < } else {
> > < ...
> >
> > It would be strange if your compiler didn't warn about 'val' being used
> > uninitialized here, which in its turn would introduce a regression for
> > the platforms with the indirectly accessible eDMA registers.
>
> You're correct. It's strange. I don't know why my using compiler [1] didn't
> warn about the 'val' though...
>
> [1]
> https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.1.0/x86_64-gcc-11.1.0-nolibc-aarch64-linux.tar.xz
>
> > Anyway you can't just drop something what didn't work for you
> > hardware. The method you suggest to fix here works fine for multiple
> > DW PCIe IP-cores. Judging by the HW manuals it should work at least up
> > to v5.30a. Are you sure that your controller is of v5.20a? I see you
> > overwrite the IP-core version for the PCIe host driver only. Why is
> > that necessary? Does the version auto-detection procedure work
> > incorrectly for you?
>
> Thank you for pointed it out! I realized that overwriting the IP-core
> Version was not needed. So, I'll drop it on v8.
> ---
> #define DWC_VERSION 0x520a
> ...
> struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev)
> {
> ...
> rcar->dw.version = DWC_VERSION;
> ---
>
> > What does the dbi+0x8f8 CSR contain in the host
> > and EP registers space? Similarly could you also provide a content of
> > the +0x978 register?
>
> The dbi+0x8f8 and the +0x978 registers' values are 0x00000000.
> --------------- (sorry, replace tabs with spaces...)---------------
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -843,6 +843,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> {
> u32 val;
>
> + dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
> + dw_pcie_readl_dma(pci, 0x8f8),
> + dw_pcie_readl_dma(pci, 0x978));
> +
No, this should have been the dw_pcie_readl_dbi() calls instead of
dw_pcie_readl_!dma!(). What I try to understand from these values is
the real version of your controller (dbi+0x8f8) and whether the legacy
eDMA viewport registers range follows the documented convention of
having FFs in the dbi+0x978 register. My current assumption that
either your IP-core is newer than v5.30a or has some vendor-specific
modification. But let's see the value first.
> if (pci->edma.reg_base) {
> pci->edma.mf = EDMA_MF_EDMA_UNROLL;
> -------------------------------------------------------------------
>
> ----- Output -----
> # dmesg | grep edma
> [ 1.108709] pcie-rcar-gen4 e65d0000.pcie: dw_pcie_edma_find_chip: +0x8f8 = 00000000, +0x978 = 00000000
> ------------------
>
> So, should I change the condition like below?
>
> ---
> - if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> + if ((val == 0xFFFFFFFF || val == 0x00000000) && pci->edma.reg_base) {
> ...
> - } else if (val != 0xFFFFFFFF) {
> - } else if (!(val == 0xFFFFFFFF || val == 0x00000000)) {
> ---
Definitely no. Even though it's impossible to have the eDMA controller
configured with zero number of read and write channels we shouldn't
assume that getting a zero value from the DMA_CTRL_VIEWPORT_OFF offset
means having the unrolled eDMA CSRs mapping. Let's have a look at the
content of the dbi+0x8f8 and dbi+0x978 offsets first. Based on these
values we'll come up with what to do next.
-Serge(y)
>
> Best regards,
> Yoshihiro Shimoda
>
> > -Sergey
> >
> > > > } else {
> > > > return -ENODEV;
> > > > }
> > > > --
> > > > 2.25.1
> > > >
> > >
> > > --
> > > மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 27+ messages in thread* RE: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
2022-11-28 11:59 ` Serge Semin
@ 2022-11-28 12:41 ` Yoshihiro Shimoda
2022-11-28 16:11 ` Serge Semin
0 siblings, 1 reply; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-28 12:41 UTC (permalink / raw)
To: Serge Semin
Cc: Manivannan Sadhasivam, lpieralisi@kernel.org, robh+dt@kernel.org,
kw@linux.com, bhelgaas@google.com, krzk+dt@kernel.org,
marek.vasut+renesas@gmail.com, linux-pci@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
Sergey.Semin@baikalelectronics.ru
Hi Serge,
> From: Serge Semin, Sent: Monday, November 28, 2022 8:59 PM
>
> On Mon, Nov 28, 2022 at 02:52:56AM +0000, Yoshihiro Shimoda wrote:
> > Hi Serge,
> >
> > > From: Serge Semin, Sent: Monday, November 28, 2022 8:56 AM
> > >
<snip>
> > > What does the dbi+0x8f8 CSR contain in the host
> > > and EP registers space? Similarly could you also provide a content of
> > > the +0x978 register?
> >
> > The dbi+0x8f8 and the +0x978 registers' values are 0x00000000.
> > --------------- (sorry, replace tabs with spaces...)---------------
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -843,6 +843,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > {
> > u32 val;
> >
>
> > + dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
> > + dw_pcie_readl_dma(pci, 0x8f8),
> > + dw_pcie_readl_dma(pci, 0x978));
> > +
>
> No, this should have been the dw_pcie_readl_dbi() calls instead of
> dw_pcie_readl_!dma!(). What I try to understand from these values is
> the real version of your controller (dbi+0x8f8) and whether the legacy
> eDMA viewport registers range follows the documented convention of
> having FFs in the dbi+0x978 register. My current assumption that
> either your IP-core is newer than v5.30a or has some vendor-specific
> modification. But let's see the value first.
Oops! I'm sorry for my bad code. After fixed the code, the values are:
---
[ 1.108943] pcie-rcar-gen4 e65d0000.pcie: dw_pcie_edma_find_chip: +0x8f8 = 3532302a, +0x978 = 00000000
---
<snip>
> > So, should I change the condition like below?
> >
> > ---
> > - if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > + if ((val == 0xFFFFFFFF || val == 0x00000000) && pci->edma.reg_base) {
> > ...
> > - } else if (val != 0xFFFFFFFF) {
> > - } else if (!(val == 0xFFFFFFFF || val == 0x00000000)) {
> > ---
>
> Definitely no. Even though it's impossible to have the eDMA controller
> configured with zero number of read and write channels we shouldn't
> assume that getting a zero value from the DMA_CTRL_VIEWPORT_OFF offset
> means having the unrolled eDMA CSRs mapping. Let's have a look at the
> content of the dbi+0x8f8 and dbi+0x978 offsets first. Based on these
> values we'll come up with what to do next.
I got it.
Best regards,
Yoshihiro Shimoda
> -Serge(y)
>
> >
> > Best regards,
> > Yoshihiro Shimoda
> >
> > > -Sergey
> > >
> > > > > } else {
> > > > > return -ENODEV;
> > > > > }
> > > > > --
> > > > > 2.25.1
> > > > >
> > > >
> > > > --
> > > > மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
2022-11-28 12:41 ` Yoshihiro Shimoda
@ 2022-11-28 16:11 ` Serge Semin
2022-11-29 0:21 ` Yoshihiro Shimoda
0 siblings, 1 reply; 27+ messages in thread
From: Serge Semin @ 2022-11-28 16:11 UTC (permalink / raw)
To: Yoshihiro Shimoda, Manivannan Sadhasivam
Cc: lpieralisi@kernel.org, robh+dt@kernel.org, kw@linux.com,
bhelgaas@google.com, krzk+dt@kernel.org,
marek.vasut+renesas@gmail.com, linux-pci@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
Sergey.Semin@baikalelectronics.ru
On Mon, Nov 28, 2022 at 12:41:11PM +0000, Yoshihiro Shimoda wrote:
> Hi Serge,
>
> > From: Serge Semin, Sent: Monday, November 28, 2022 8:59 PM
> >
> > On Mon, Nov 28, 2022 at 02:52:56AM +0000, Yoshihiro Shimoda wrote:
> > > Hi Serge,
> > >
> > > > From: Serge Semin, Sent: Monday, November 28, 2022 8:56 AM
> > > >
> <snip>
> > > > What does the dbi+0x8f8 CSR contain in the host
> > > > and EP registers space? Similarly could you also provide a content of
> > > > the +0x978 register?
> > >
> > > The dbi+0x8f8 and the +0x978 registers' values are 0x00000000.
> > > --------------- (sorry, replace tabs with spaces...)---------------
> > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > @@ -843,6 +843,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > > {
> > > u32 val;
> > >
> >
> > > + dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
> > > + dw_pcie_readl_dma(pci, 0x8f8),
> > > + dw_pcie_readl_dma(pci, 0x978));
> > > +
> >
> > No, this should have been the dw_pcie_readl_dbi() calls instead of
> > dw_pcie_readl_!dma!(). What I try to understand from these values is
> > the real version of your controller (dbi+0x8f8) and whether the legacy
> > eDMA viewport registers range follows the documented convention of
> > having FFs in the dbi+0x978 register. My current assumption that
> > either your IP-core is newer than v5.30a or has some vendor-specific
> > modification. But let's see the value first.
>
> Oops! I'm sorry for my bad code. After fixed the code, the values are:
> ---
> [ 1.108943] pcie-rcar-gen4 e65d0000.pcie: dw_pcie_edma_find_chip: +0x8f8 = 3532302a, +0x978 = 00000000
> ---
@Yoshihiro. Got it. Thanks. Could you please confirm (double-check)
that what the content of the +0x978 CSR was really read by means of the
dw_pcie_readl_dbi() method?
@Mani, could you please have a look at the content of the +0x8f8 and
+0x978 CSRs in the CDM space of the available to you controllers?
I've reproduced the behavior what discovered by @Yoshihiro, but on
v5.40a IP-core only. It was expected for that controller version, but
v5.20a was supposed to have FFs in +0x978 for the unrolled iATU/eDMA
space. It's strange to see it didn't.
-Sergey
>
> <snip>
> > > So, should I change the condition like below?
> > >
> > > ---
> > > - if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > > + if ((val == 0xFFFFFFFF || val == 0x00000000) && pci->edma.reg_base) {
> > > ...
> > > - } else if (val != 0xFFFFFFFF) {
> > > - } else if (!(val == 0xFFFFFFFF || val == 0x00000000)) {
> > > ---
> >
> > Definitely no. Even though it's impossible to have the eDMA controller
> > configured with zero number of read and write channels we shouldn't
> > assume that getting a zero value from the DMA_CTRL_VIEWPORT_OFF offset
> > means having the unrolled eDMA CSRs mapping. Let's have a look at the
> > content of the dbi+0x8f8 and dbi+0x978 offsets first. Based on these
> > values we'll come up with what to do next.
>
> I got it.
>
> Best regards,
> Yoshihiro Shimoda
>
> > -Serge(y)
> >
> > >
> > > Best regards,
> > > Yoshihiro Shimoda
> > >
> > > > -Sergey
> > > >
> > > > > > } else {
> > > > > > return -ENODEV;
> > > > > > }
> > > > > > --
> > > > > > 2.25.1
> > > > > >
> > > > >
> > > > > --
> > > > > மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 27+ messages in thread* RE: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
2022-11-28 16:11 ` Serge Semin
@ 2022-11-29 0:21 ` Yoshihiro Shimoda
2022-12-08 12:26 ` Yoshihiro Shimoda
0 siblings, 1 reply; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-29 0:21 UTC (permalink / raw)
To: Serge Semin, Manivannan Sadhasivam
Cc: lpieralisi@kernel.org, robh+dt@kernel.org, kw@linux.com,
bhelgaas@google.com, krzk+dt@kernel.org,
marek.vasut+renesas@gmail.com, linux-pci@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
Sergey.Semin@baikalelectronics.ru
Hi Serge,
> From: Serge Semin, Sent: Tuesday, November 29, 2022 1:11 AM
>
> On Mon, Nov 28, 2022 at 12:41:11PM +0000, Yoshihiro Shimoda wrote:
> > Hi Serge,
> >
> > > From: Serge Semin, Sent: Monday, November 28, 2022 8:59 PM
> > >
> > > On Mon, Nov 28, 2022 at 02:52:56AM +0000, Yoshihiro Shimoda wrote:
> > > > Hi Serge,
<snip>
> > > No, this should have been the dw_pcie_readl_dbi() calls instead of
> > > dw_pcie_readl_!dma!(). What I try to understand from these values is
> > > the real version of your controller (dbi+0x8f8) and whether the legacy
> > > eDMA viewport registers range follows the documented convention of
> > > having FFs in the dbi+0x978 register. My current assumption that
> > > either your IP-core is newer than v5.30a or has some vendor-specific
> > > modification. But let's see the value first.
> >
>
> > Oops! I'm sorry for my bad code. After fixed the code, the values are:
> > ---
> > [ 1.108943] pcie-rcar-gen4 e65d0000.pcie: dw_pcie_edma_find_chip: +0x8f8 = 3532302a, +0x978 = 00000000
> > ---
>
> @Yoshihiro. Got it. Thanks. Could you please confirm (double-check)
> that what the content of the +0x978 CSR was really read by means of the
> dw_pcie_readl_dbi() method?
Yes, I used dw_pcie_readl_dbi() like below.
--------------- (sorry, tabs replaced with spaces) ---------------
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -843,6 +843,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
{
u32 val;
+ dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
+ dw_pcie_readl_dbi(pci, 0x8f8),
+ dw_pcie_readl_dbi(pci, 0x978));
+
if (pci->edma.reg_base) {
pci->edma.mf = EDMA_MF_EDMA_UNROLL;
------------------------------------------------------------------
Best regards,
Yoshihiro Shimoda
> @Mani, could you please have a look at the content of the +0x8f8 and
> +0x978 CSRs in the CDM space of the available to you controllers?
>
> I've reproduced the behavior what discovered by @Yoshihiro, but on
> v5.40a IP-core only. It was expected for that controller version, but
> v5.20a was supposed to have FFs in +0x978 for the unrolled iATU/eDMA
> space. It's strange to see it didn't.
>
> -Sergey
>
> >
> > <snip>
> > > > So, should I change the condition like below?
> > > >
> > > > ---
> > > > - if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > > > + if ((val == 0xFFFFFFFF || val == 0x00000000) && pci->edma.reg_base) {
> > > > ...
> > > > - } else if (val != 0xFFFFFFFF) {
> > > > - } else if (!(val == 0xFFFFFFFF || val == 0x00000000)) {
> > > > ---
> > >
> > > Definitely no. Even though it's impossible to have the eDMA controller
> > > configured with zero number of read and write channels we shouldn't
> > > assume that getting a zero value from the DMA_CTRL_VIEWPORT_OFF offset
> > > means having the unrolled eDMA CSRs mapping. Let's have a look at the
> > > content of the dbi+0x8f8 and dbi+0x978 offsets first. Based on these
> > > values we'll come up with what to do next.
> >
> > I got it.
> >
> > Best regards,
> > Yoshihiro Shimoda
> >
> > > -Serge(y)
> > >
> > > >
> > > > Best regards,
> > > > Yoshihiro Shimoda
> > > >
> > > > > -Sergey
> > > > >
> > > > > > > } else {
> > > > > > > return -ENODEV;
> > > > > > > }
> > > > > > > --
> > > > > > > 2.25.1
> > > > > > >
> > > > > >
> > > > > > --
> > > > > > மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 27+ messages in thread* RE: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
2022-11-29 0:21 ` Yoshihiro Shimoda
@ 2022-12-08 12:26 ` Yoshihiro Shimoda
2022-12-08 14:01 ` Serge Semin
0 siblings, 1 reply; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-12-08 12:26 UTC (permalink / raw)
To: Serge Semin, Manivannan Sadhasivam
Cc: lpieralisi@kernel.org, robh+dt@kernel.org, kw@linux.com,
bhelgaas@google.com, krzk+dt@kernel.org,
marek.vasut+renesas@gmail.com, linux-pci@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
Sergey.Semin@baikalelectronics.ru
Hi Serge, Manivannan,
> From: Yoshihiro Shimoda, Sent: Tuesday, November 29, 2022 9:22 AM
>
> > From: Serge Semin, Sent: Tuesday, November 29, 2022 1:11 AM
> >
> > On Mon, Nov 28, 2022 at 12:41:11PM +0000, Yoshihiro Shimoda wrote:
> > > Hi Serge,
> > >
> > > > From: Serge Semin, Sent: Monday, November 28, 2022 8:59 PM
> > > >
> > > > On Mon, Nov 28, 2022 at 02:52:56AM +0000, Yoshihiro Shimoda wrote:
> > > > > Hi Serge,
> <snip>
> > > > No, this should have been the dw_pcie_readl_dbi() calls instead of
> > > > dw_pcie_readl_!dma!(). What I try to understand from these values is
> > > > the real version of your controller (dbi+0x8f8) and whether the legacy
> > > > eDMA viewport registers range follows the documented convention of
> > > > having FFs in the dbi+0x978 register. My current assumption that
> > > > either your IP-core is newer than v5.30a or has some vendor-specific
> > > > modification. But let's see the value first.
> > >
> >
> > > Oops! I'm sorry for my bad code. After fixed the code, the values are:
> > > ---
> > > [ 1.108943] pcie-rcar-gen4 e65d0000.pcie: dw_pcie_edma_find_chip: +0x8f8 = 3532302a, +0x978 = 00000000
> > > ---
> >
> > @Yoshihiro. Got it. Thanks. Could you please confirm (double-check)
> > that what the content of the +0x978 CSR was really read by means of the
> > dw_pcie_readl_dbi() method?
>
> Yes, I used dw_pcie_readl_dbi() like below.
>
> --------------- (sorry, tabs replaced with spaces) ---------------
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -843,6 +843,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> {
> u32 val;
>
> + dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
> + dw_pcie_readl_dbi(pci, 0x8f8),
> + dw_pcie_readl_dbi(pci, 0x978));
> +
> if (pci->edma.reg_base) {
> pci->edma.mf = EDMA_MF_EDMA_UNROLL;
> ------------------------------------------------------------------
>
> > @Mani, could you please have a look at the content of the +0x8f8 and
> > +0x978 CSRs in the CDM space of the available to you controllers?
> >
> > I've reproduced the behavior what discovered by @Yoshihiro, but on
> > v5.40a IP-core only. It was expected for that controller version, but
> > v5.20a was supposed to have FFs in +0x978 for the unrolled iATU/eDMA
> > space. It's strange to see it didn't.
So, should I add a quirk flag for my controller like below?
---
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 69665a8a002e..384bd2c0ea75 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -844,7 +844,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
u32 val;
val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
- if (val == 0xFFFFFFFF && pci->edma.reg_base) {
+ if ((pci->no_dma_ctrl_reg || val == 0xFFFFFFFF) && pci->edma.reg_base) {
pci->edma.mf = EDMA_MF_EDMA_UNROLL;
val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
---
Best regards,
Yoshihiro Shimoda
> > -Sergey
> >
> > >
> > > <snip>
> > > > > So, should I change the condition like below?
> > > > >
> > > > > ---
> > > > > - if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > > > > + if ((val == 0xFFFFFFFF || val == 0x00000000) && pci->edma.reg_base) {
> > > > > ...
> > > > > - } else if (val != 0xFFFFFFFF) {
> > > > > - } else if (!(val == 0xFFFFFFFF || val == 0x00000000)) {
> > > > > ---
> > > >
> > > > Definitely no. Even though it's impossible to have the eDMA controller
> > > > configured with zero number of read and write channels we shouldn't
> > > > assume that getting a zero value from the DMA_CTRL_VIEWPORT_OFF offset
> > > > means having the unrolled eDMA CSRs mapping. Let's have a look at the
> > > > content of the dbi+0x8f8 and dbi+0x978 offsets first. Based on these
> > > > values we'll come up with what to do next.
> > >
> > > I got it.
> > >
> > > Best regards,
> > > Yoshihiro Shimoda
> > >
> > > > -Serge(y)
> > > >
> > > > >
> > > > > Best regards,
> > > > > Yoshihiro Shimoda
> > > > >
> > > > > > -Sergey
> > > > > >
> > > > > > > > } else {
> > > > > > > > return -ENODEV;
> > > > > > > > }
> > > > > > > > --
> > > > > > > > 2.25.1
> > > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > மணிவண்ணன் சதாசிவம்
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
2022-12-08 12:26 ` Yoshihiro Shimoda
@ 2022-12-08 14:01 ` Serge Semin
2022-12-09 7:45 ` Yoshihiro Shimoda
0 siblings, 1 reply; 27+ messages in thread
From: Serge Semin @ 2022-12-08 14:01 UTC (permalink / raw)
To: Yoshihiro Shimoda, Frank Li, Manivannan Sadhasivam
Cc: lpieralisi@kernel.org, robh+dt@kernel.org, kw@linux.com,
bhelgaas@google.com, krzk+dt@kernel.org,
marek.vasut+renesas@gmail.com, linux-pci@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
Sergey.Semin@baikalelectronics.ru
Cc += Frank Li
@Frank could you have a look at the thread and check the content of
the CSRs dbi+0x8f8 and dbi+0x978 on available to you DW PCIe +EDMA
devices?
On Thu, Dec 08, 2022 at 12:26:18PM +0000, Yoshihiro Shimoda wrote:
> Hi Serge, Manivannan,
>
> > From: Yoshihiro Shimoda, Sent: Tuesday, November 29, 2022 9:22 AM
> >
> > > From: Serge Semin, Sent: Tuesday, November 29, 2022 1:11 AM
> > >
> > > On Mon, Nov 28, 2022 at 12:41:11PM +0000, Yoshihiro Shimoda wrote:
> > > > Hi Serge,
> > > >
> > > > > From: Serge Semin, Sent: Monday, November 28, 2022 8:59 PM
> > > > >
> > > > > On Mon, Nov 28, 2022 at 02:52:56AM +0000, Yoshihiro Shimoda wrote:
> > > > > > Hi Serge,
> > <snip>
> > > > > No, this should have been the dw_pcie_readl_dbi() calls instead of
> > > > > dw_pcie_readl_!dma!(). What I try to understand from these values is
> > > > > the real version of your controller (dbi+0x8f8) and whether the legacy
> > > > > eDMA viewport registers range follows the documented convention of
> > > > > having FFs in the dbi+0x978 register. My current assumption that
> > > > > either your IP-core is newer than v5.30a or has some vendor-specific
> > > > > modification. But let's see the value first.
> > > >
> > >
> > > > Oops! I'm sorry for my bad code. After fixed the code, the values are:
> > > > ---
> > > > [ 1.108943] pcie-rcar-gen4 e65d0000.pcie: dw_pcie_edma_find_chip: +0x8f8 = 3532302a, +0x978 = 00000000
> > > > ---
> > >
> > > @Yoshihiro. Got it. Thanks. Could you please confirm (double-check)
> > > that what the content of the +0x978 CSR was really read by means of the
> > > dw_pcie_readl_dbi() method?
> >
> > Yes, I used dw_pcie_readl_dbi() like below.
> >
> > --------------- (sorry, tabs replaced with spaces) ---------------
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -843,6 +843,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > {
> > u32 val;
> >
> > + dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
> > + dw_pcie_readl_dbi(pci, 0x8f8),
> > + dw_pcie_readl_dbi(pci, 0x978));
> > +
> > if (pci->edma.reg_base) {
> > pci->edma.mf = EDMA_MF_EDMA_UNROLL;
> > ------------------------------------------------------------------
> >
> > > @Mani, could you please have a look at the content of the +0x8f8 and
> > > +0x978 CSRs in the CDM space of the available to you controllers?
> > >
> > > I've reproduced the behavior what discovered by @Yoshihiro, but on
> > > v5.40a IP-core only. It was expected for that controller version, but
> > > v5.20a was supposed to have FFs in +0x978 for the unrolled iATU/eDMA
> > > space. It's strange to see it didn't.
>
> So, should I add a quirk flag for my controller like below?
> ---
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 69665a8a002e..384bd2c0ea75 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -844,7 +844,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> u32 val;
>
> val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> - if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> + if ((pci->no_dma_ctrl_reg || val == 0xFFFFFFFF) && pci->edma.reg_base) {
> pci->edma.mf = EDMA_MF_EDMA_UNROLL;
>
> val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
Very much no. I have just got rid from such boolean flags from the DW
PCI private data.
Please be patient. Maintainers not always respond as soon as we would
want. Please note my patchset also stalls due to your discovery (DW
eDMA patches still under review).
What we have discovered here is the undocumented behavior. The
HW-manuals from 4.80 up to 5.30 say that the register dbi+0x978 must
have FFs if the register doesn't exist. A similar rule is defined for
the iATU CSRs space and it's working well at least up to IP-core
5.40a. The viewport-based eDMA CSRs space has been removed from the
HW-manuals since IP-core 5.40a and I can assure that IP-core 5.40a has
zeros in dbi+0x978 on the real HW. I do have another devices with eDMA
but all of them have been synthesized with the legacy viewport-based
eDMA access, so I can't check whether the FFs statement is correct for
the devices older than 5.40. You detected the problem on the IP-core
5.20a, but @Mani didn't see it on his devices. Neither does Frank
AFAIU. That's why I asked him and @Frank to check what value the
dbi+0x8f8 and dbi+0x978 registers have in their devices at hand. After
that we'll either add the IP-core version based test here:
< - val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
< + if (dw_pcie_ver_is_ge(pci, 5?0A)) {
< + val = 0xFFFFFFFF;
< + else
< + val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
or add a new capability flag if @Mani has IP-core 5.20a with FFs in
the CSRs dbi+0x978. Like this:
< - val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
< + if (dw_pcie_cap_is(pci, EDMA_UNROLL)) {
< + val = 0xFFFFFFFF;
< + else
< + val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
So the main goal of this activity is to check whether your discovery
is a bug on your particular device or is a bug in the HW-manuals. If
the later is correct then the eDMA CSRs space auto-detection procedure
should be fixed to be executed up to the corresponding IP-core
version. If the former is correct then we'll add a new capability
flag. In anyway having the new boolean field in the device private
data wouldn't be a good option since there is the capabilities API
available in the driver.
-Serge(y)
> ---
>
> Best regards,
> Yoshihiro Shimoda
>
> > > -Sergey
> > >
> > > >
> > > > <snip>
> > > > > > So, should I change the condition like below?
> > > > > >
> > > > > > ---
> > > > > > - if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > > > > > + if ((val == 0xFFFFFFFF || val == 0x00000000) && pci->edma.reg_base) {
> > > > > > ...
> > > > > > - } else if (val != 0xFFFFFFFF) {
> > > > > > - } else if (!(val == 0xFFFFFFFF || val == 0x00000000)) {
> > > > > > ---
> > > > >
> > > > > Definitely no. Even though it's impossible to have the eDMA controller
> > > > > configured with zero number of read and write channels we shouldn't
> > > > > assume that getting a zero value from the DMA_CTRL_VIEWPORT_OFF offset
> > > > > means having the unrolled eDMA CSRs mapping. Let's have a look at the
> > > > > content of the dbi+0x8f8 and dbi+0x978 offsets first. Based on these
> > > > > values we'll come up with what to do next.
> > > >
> > > > I got it.
> > > >
> > > > Best regards,
> > > > Yoshihiro Shimoda
> > > >
> > > > > -Serge(y)
> > > > >
> > > > > >
> > > > > > Best regards,
> > > > > > Yoshihiro Shimoda
> > > > > >
> > > > > > > -Sergey
> > > > > > >
> > > > > > > > > } else {
> > > > > > > > > return -ENODEV;
> > > > > > > > > }
> > > > > > > > > --
> > > > > > > > > 2.25.1
> > > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > > மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 27+ messages in thread* RE: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
2022-12-08 14:01 ` Serge Semin
@ 2022-12-09 7:45 ` Yoshihiro Shimoda
[not found] ` <HE1PR0401MB23319A9F4AF7630A82249D65881C9@HE1PR0401MB2331.eurprd04.prod.outlook.com>
0 siblings, 1 reply; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-12-09 7:45 UTC (permalink / raw)
To: Serge Semin, Frank Li, Manivannan Sadhasivam
Cc: lpieralisi@kernel.org, robh+dt@kernel.org, kw@linux.com,
bhelgaas@google.com, krzk+dt@kernel.org,
marek.vasut+renesas@gmail.com, linux-pci@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
Sergey.Semin@baikalelectronics.ru
Hi Serge,
> From: Serge Semin, Sent: Thursday, December 8, 2022 11:01 PM
>
> Cc += Frank Li
>
> @Frank could you have a look at the thread and check the content of
> the CSRs dbi+0x8f8 and dbi+0x978 on available to you DW PCIe +EDMA
> devices?
>
> On Thu, Dec 08, 2022 at 12:26:18PM +0000, Yoshihiro Shimoda wrote:
> > Hi Serge, Manivannan,
> >
> > > From: Yoshihiro Shimoda, Sent: Tuesday, November 29, 2022 9:22 AM
> > >
> > > > From: Serge Semin, Sent: Tuesday, November 29, 2022 1:11 AM
> > > >
> > > > On Mon, Nov 28, 2022 at 12:41:11PM +0000, Yoshihiro Shimoda wrote:
> > > > > Hi Serge,
> > > > >
> > > > > > From: Serge Semin, Sent: Monday, November 28, 2022 8:59 PM
> > > > > >
> > > > > > On Mon, Nov 28, 2022 at 02:52:56AM +0000, Yoshihiro Shimoda wrote:
> > > > > > > Hi Serge,
> > > <snip>
> > > > > > No, this should have been the dw_pcie_readl_dbi() calls instead of
> > > > > > dw_pcie_readl_!dma!(). What I try to understand from these values is
> > > > > > the real version of your controller (dbi+0x8f8) and whether the legacy
> > > > > > eDMA viewport registers range follows the documented convention of
> > > > > > having FFs in the dbi+0x978 register. My current assumption that
> > > > > > either your IP-core is newer than v5.30a or has some vendor-specific
> > > > > > modification. But let's see the value first.
> > > > >
> > > >
> > > > > Oops! I'm sorry for my bad code. After fixed the code, the values are:
> > > > > ---
> > > > > [ 1.108943] pcie-rcar-gen4 e65d0000.pcie: dw_pcie_edma_find_chip: +0x8f8 = 3532302a, +0x978 = 00000000
> > > > > ---
> > > >
> > > > @Yoshihiro. Got it. Thanks. Could you please confirm (double-check)
> > > > that what the content of the +0x978 CSR was really read by means of the
> > > > dw_pcie_readl_dbi() method?
> > >
> > > Yes, I used dw_pcie_readl_dbi() like below.
> > >
> > > --------------- (sorry, tabs replaced with spaces) ---------------
> > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > @@ -843,6 +843,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > > {
> > > u32 val;
> > >
> > > + dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
> > > + dw_pcie_readl_dbi(pci, 0x8f8),
> > > + dw_pcie_readl_dbi(pci, 0x978));
> > > +
> > > if (pci->edma.reg_base) {
> > > pci->edma.mf = EDMA_MF_EDMA_UNROLL;
> > > ------------------------------------------------------------------
> > >
> > > > @Mani, could you please have a look at the content of the +0x8f8 and
> > > > +0x978 CSRs in the CDM space of the available to you controllers?
> > > >
> > > > I've reproduced the behavior what discovered by @Yoshihiro, but on
> > > > v5.40a IP-core only. It was expected for that controller version, but
> > > > v5.20a was supposed to have FFs in +0x978 for the unrolled iATU/eDMA
> > > > space. It's strange to see it didn't.
> >
>
> > So, should I add a quirk flag for my controller like below?
> > ---
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 69665a8a002e..384bd2c0ea75 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -844,7 +844,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > u32 val;
> >
> > val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
>
> > - if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > + if ((pci->no_dma_ctrl_reg || val == 0xFFFFFFFF) && pci->edma.reg_base) {
> > pci->edma.mf = EDMA_MF_EDMA_UNROLL;
> >
> > val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
>
> Very much no. I have just got rid from such boolean flags from the DW
> PCI private data.
>
> Please be patient. Maintainers not always respond as soon as we would
> want. Please note my patchset also stalls due to your discovery (DW
> eDMA patches still under review).
>
> What we have discovered here is the undocumented behavior. The
> HW-manuals from 4.80 up to 5.30 say that the register dbi+0x978 must
> have FFs if the register doesn't exist. A similar rule is defined for
> the iATU CSRs space and it's working well at least up to IP-core
> 5.40a. The viewport-based eDMA CSRs space has been removed from the
> HW-manuals since IP-core 5.40a and I can assure that IP-core 5.40a has
> zeros in dbi+0x978 on the real HW. I do have another devices with eDMA
> but all of them have been synthesized with the legacy viewport-based
> eDMA access, so I can't check whether the FFs statement is correct for
> the devices older than 5.40. You detected the problem on the IP-core
> 5.20a, but @Mani didn't see it on his devices. Neither does Frank
> AFAIU. That's why I asked him and @Frank to check what value the
> dbi+0x8f8 and dbi+0x978 registers have in their devices at hand. After
> that we'll either add the IP-core version based test here:
> < - val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> < + if (dw_pcie_ver_is_ge(pci, 5?0A)) {
> < + val = 0xFFFFFFFF;
> < + else
> < + val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> or add a new capability flag if @Mani has IP-core 5.20a with FFs in
> the CSRs dbi+0x978. Like this:
> < - val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> < + if (dw_pcie_cap_is(pci, EDMA_UNROLL)) {
> < + val = 0xFFFFFFFF;
> < + else
> < + val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
>
> So the main goal of this activity is to check whether your discovery
> is a bug on your particular device or is a bug in the HW-manuals. If
> the later is correct then the eDMA CSRs space auto-detection procedure
> should be fixed to be executed up to the corresponding IP-core
> version. If the former is correct then we'll add a new capability
> flag. In anyway having the new boolean field in the device private
> data wouldn't be a good option since there is the capabilities API
> available in the driver.
Thank you very much for your detailed explanation! I understood.
# At least, I should have realized that the PCI dwc driver has
# the capabilities API before I send an email...
Best regards,
Yoshihiro Shimoda
> -Serge(y)
>
> > ---
> >
> > Best regards,
> > Yoshihiro Shimoda
> >
> > > > -Sergey
> > > >
> > > > >
> > > > > <snip>
> > > > > > > So, should I change the condition like below?
> > > > > > >
> > > > > > > ---
> > > > > > > - if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > > > > > > + if ((val == 0xFFFFFFFF || val == 0x00000000) && pci->edma.reg_base) {
> > > > > > > ...
> > > > > > > - } else if (val != 0xFFFFFFFF) {
> > > > > > > - } else if (!(val == 0xFFFFFFFF || val == 0x00000000)) {
> > > > > > > ---
> > > > > >
> > > > > > Definitely no. Even though it's impossible to have the eDMA controller
> > > > > > configured with zero number of read and write channels we shouldn't
> > > > > > assume that getting a zero value from the DMA_CTRL_VIEWPORT_OFF offset
> > > > > > means having the unrolled eDMA CSRs mapping. Let's have a look at the
> > > > > > content of the dbi+0x8f8 and dbi+0x978 offsets first. Based on these
> > > > > > values we'll come up with what to do next.
> > > > >
> > > > > I got it.
> > > > >
> > > > > Best regards,
> > > > > Yoshihiro Shimoda
> > > > >
> > > > > > -Serge(y)
> > > > > >
> > > > > > >
> > > > > > > Best regards,
> > > > > > > Yoshihiro Shimoda
> > > > > > >
> > > > > > > > -Sergey
> > > > > > > >
> > > > > > > > > > } else {
> > > > > > > > > > return -ENODEV;
> > > > > > > > > > }
> > > > > > > > > > --
> > > > > > > > > > 2.25.1
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > --
> > > > > > > > > மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v7 6/9] PCI: dwc: Add support for triggering legacy IRQs
2022-11-21 12:43 [PATCH v7 0/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
` (4 preceding siblings ...)
2022-11-21 12:43 ` [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists Yoshihiro Shimoda
@ 2022-11-21 12:43 ` Yoshihiro Shimoda
2022-11-21 12:43 ` [PATCH v7 7/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support Yoshihiro Shimoda
` (2 subsequent siblings)
8 siblings, 0 replies; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-21 12:43 UTC (permalink / raw)
To: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt
Cc: 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 2cc8584da6f4..79ac389b794a 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 028155c03acd..ab50c4577495 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -146,11 +146,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
@@ -243,6 +246,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;
@@ -351,7 +357,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 {
@@ -418,7 +427,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] 27+ messages in thread* [PATCH v7 7/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support
2022-11-21 12:43 [PATCH v7 0/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
` (5 preceding siblings ...)
2022-11-21 12:43 ` [PATCH v7 6/9] PCI: dwc: Add support for triggering legacy IRQs Yoshihiro Shimoda
@ 2022-11-21 12:43 ` Yoshihiro Shimoda
2022-11-22 15:04 ` Bjorn Helgaas
2022-11-21 12:43 ` [PATCH v7 8/9] PCI: rcar-gen4-ep: Add R-Car Gen4 PCIe Endpoint support Yoshihiro Shimoda
2022-11-21 12:44 ` [PATCH v7 9/9] MAINTAINERS: Update PCI DRIVER FOR RENESAS R-CAR for R-Car Gen4 Yoshihiro Shimoda
8 siblings, 1 reply; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-21 12:43 UTC (permalink / raw)
To: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt
Cc: 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.
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.h | 1 +
.../pci/controller/dwc/pcie-rcar-gen4-host.c | 190 ++++++++++++++++++
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 181 +++++++++++++++++
drivers/pci/controller/dwc/pcie-rcar-gen4.h | 63 ++++++
7 files changed, 449 insertions(+)
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 771b8b146623..99717b0f4e50 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -393,4 +393,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_IRQ_DOMAIN
+ 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 d18ff0519a62..122e4bbb98f9 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.h b/drivers/pci/controller/dwc/pcie-designware.h
index ab50c4577495..ca0f9678517f 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -302,6 +302,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..646e8efaaed8
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
@@ -0,0 +1,190 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PCIe host controller driver for Renesas R-Car Gen4 Series SoCs
+ * Copyright (C) 2022 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;
+
+ rcar_gen4_pcie_set_device_type(rcar, true, dw->num_lanes);
+
+ dw_pcie_dbi_ro_wr_en(dw);
+
+ /* Enable L1 Substates */
+ val = dw_pcie_readl_dbi(dw, L1PSCAP(PCI_L1SS_CTL1));
+ val &= ~PCI_L1SS_CTL1_L1SS_MASK;
+ val |= PCI_L1SS_CTL1_PCIPM_L1_2 | PCI_L1SS_CTL1_PCIPM_L1_1 |
+ PCI_L1SS_CTL1_ASPM_L1_2 | PCI_L1SS_CTL1_ASPM_L1_1;
+ dw_pcie_writel_dbi(dw, L1PSCAP(PCI_L1SS_CTL1), val);
+
+ rcar_gen4_pcie_disable_bar(dw, BAR0MASKF);
+ rcar_gen4_pcie_disable_bar(dw, BAR1MASKF);
+
+ /* Set Root Control */
+ val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_RTCTL));
+ val |= PCI_EXP_RTCTL_SECEE | PCI_EXP_RTCTL_SENFEE |
+ PCI_EXP_RTCTL_SEFEE | PCI_EXP_RTCTL_PMEIE |
+ PCI_EXP_RTCTL_CRSSVE;
+ dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_RTCTL), val);
+
+ /* Set Interrupt Disable, SERR# Enable, Parity Error Response */
+ val = dw_pcie_readl_dbi(dw, PCI_COMMAND);
+ val |= PCI_COMMAND_PARITY | PCI_COMMAND_SERR |
+ PCI_COMMAND_INTX_DISABLE;
+ dw_pcie_writel_dbi(dw, PCI_COMMAND, val);
+
+ /* Enable SERR */
+ val = dw_pcie_readb_dbi(dw, PCI_BRIDGE_CONTROL);
+ val |= PCI_BRIDGE_CTL_SERR;
+ dw_pcie_writeb_dbi(dw, PCI_BRIDGE_CONTROL, val);
+
+ /* Device control */
+ val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_DEVCTL));
+ val |= PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE |
+ PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE;
+ dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_DEVCTL), val);
+
+ 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);
+ }
+
+ 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;
+
+ 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);
+}
+
+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..9900d1353998
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -0,0 +1,181 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
+ * Copyright (C) 2022 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)
+
+#define DWC_VERSION 0x520a
+
+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);
+}
+
+void rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
+ int num_lanes)
+{
+ u32 val;
+
+ 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);
+}
+
+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, "%s: failed to resume/get Runtime PM\n", __func__);
+ goto err_resume_and_get;
+ }
+
+ err = reset_control_deassert(rcar->rst);
+ if (err < 0) {
+ dev_err(dev, "%s: failed to deassert reset_control\n", __func__);
+ goto err_deassert;
+ }
+
+ writel(PCIEDMAINTSTSEN_INIT, rcar->base + PCIEDMAINTSTSEN);
+
+ return 0;
+
+err_deassert:
+ pm_runtime_put(dev);
+
+err_resume_and_get:
+ pm_runtime_disable(dev);
+
+ return err;
+}
+
+void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
+{
+ struct device *dev = rcar->dw.dev;
+
+ writel(0, rcar->base + PCIEDMAINTSTSEN);
+ 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;
+ rcar->dw.version = DWC_VERSION;
+
+ 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..db401f884fc8
--- /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 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);
+void 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] 27+ messages in thread* Re: [PATCH v7 7/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support
2022-11-21 12:43 ` [PATCH v7 7/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support Yoshihiro Shimoda
@ 2022-11-22 15:04 ` Bjorn Helgaas
2022-11-25 11:37 ` Yoshihiro Shimoda
0 siblings, 1 reply; 27+ messages in thread
From: Bjorn Helgaas @ 2022-11-22 15:04 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt, marek.vasut+renesas,
linux-pci, devicetree, linux-renesas-soc
On Mon, Nov 21, 2022 at 09:43:58PM +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.
> + /* Enable L1 Substates */
> + val = dw_pcie_readl_dbi(dw, L1PSCAP(PCI_L1SS_CTL1));
> + val &= ~PCI_L1SS_CTL1_L1SS_MASK;
> + val |= PCI_L1SS_CTL1_PCIPM_L1_2 | PCI_L1SS_CTL1_PCIPM_L1_1 |
> + PCI_L1SS_CTL1_ASPM_L1_2 | PCI_L1SS_CTL1_ASPM_L1_1;
> + dw_pcie_writel_dbi(dw, L1PSCAP(PCI_L1SS_CTL1), val);
This seems like something that ought to be done by the PCI core in
pcie/aspm.c. L1.2 also depends on LTR being supported and configured.
If it needs to be enabled here, can you expand the comment to say why
and how LTR is being configured?
> + rcar_gen4_pcie_disable_bar(dw, BAR0MASKF);
> + rcar_gen4_pcie_disable_bar(dw, BAR1MASKF);
> +
> + /* Set Root Control */
> + val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_RTCTL));
> + val |= PCI_EXP_RTCTL_SECEE | PCI_EXP_RTCTL_SENFEE |
> + PCI_EXP_RTCTL_SEFEE | PCI_EXP_RTCTL_PMEIE |
> + PCI_EXP_RTCTL_CRSSVE;
> + dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_RTCTL), val);
> +
> + /* Set Interrupt Disable, SERR# Enable, Parity Error Response */
> + val = dw_pcie_readl_dbi(dw, PCI_COMMAND);
> + val |= PCI_COMMAND_PARITY | PCI_COMMAND_SERR |
> + PCI_COMMAND_INTX_DISABLE;
> + dw_pcie_writel_dbi(dw, PCI_COMMAND, val);
> +
> + /* Enable SERR */
> + val = dw_pcie_readb_dbi(dw, PCI_BRIDGE_CONTROL);
> + val |= PCI_BRIDGE_CTL_SERR;
> + dw_pcie_writeb_dbi(dw, PCI_BRIDGE_CONTROL, val);
> +
> + /* Device control */
> + val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_DEVCTL));
> + val |= PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE |
> + PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE;
> + dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_DEVCTL), val);
The above also looks like things that should be configured by the PCI
core.
> + dev_err(&pdev->dev, "Failed to initialize host\n");
> + dev_err(dev, "failed to request resource: %d\n", err);
Pick a capitalization style.
> + dev_err(dev, "%s: failed to resume/get Runtime PM\n", __func__);
The driver name + device ID + message text printed by dev_err() should
be enough that __func__ isn't needed.
Bjorn
^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: [PATCH v7 7/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support
2022-11-22 15:04 ` Bjorn Helgaas
@ 2022-11-25 11:37 ` Yoshihiro Shimoda
0 siblings, 0 replies; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-25 11:37 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: lpieralisi@kernel.org, robh+dt@kernel.org, kw@linux.com,
bhelgaas@google.com, krzk+dt@kernel.org,
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: Wednesday, November 23, 2022 12:05 AM
>
> On Mon, Nov 21, 2022 at 09:43:58PM +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.
>
> > + /* Enable L1 Substates */
> > + val = dw_pcie_readl_dbi(dw, L1PSCAP(PCI_L1SS_CTL1));
> > + val &= ~PCI_L1SS_CTL1_L1SS_MASK;
> > + val |= PCI_L1SS_CTL1_PCIPM_L1_2 | PCI_L1SS_CTL1_PCIPM_L1_1 |
> > + PCI_L1SS_CTL1_ASPM_L1_2 | PCI_L1SS_CTL1_ASPM_L1_1;
> > + dw_pcie_writel_dbi(dw, L1PSCAP(PCI_L1SS_CTL1), val);
>
> This seems like something that ought to be done by the PCI core in
> pcie/aspm.c. L1.2 also depends on LTR being supported and configured.
>
> If it needs to be enabled here, can you expand the comment to say why
> and how LTR is being configured?
Thank you for your review! I realized that this driver should not enable
it here, as you mentioned. However, I don't know why but it needs to be
enabled here. Otherwise, this driver cannot work. So, I'm investigating
the issue now.
> > + rcar_gen4_pcie_disable_bar(dw, BAR0MASKF);
> > + rcar_gen4_pcie_disable_bar(dw, BAR1MASKF);
> > +
> > + /* Set Root Control */
> > + val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_RTCTL));
> > + val |= PCI_EXP_RTCTL_SECEE | PCI_EXP_RTCTL_SENFEE |
> > + PCI_EXP_RTCTL_SEFEE | PCI_EXP_RTCTL_PMEIE |
> > + PCI_EXP_RTCTL_CRSSVE;
> > + dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_RTCTL), val);
> > +
> > + /* Set Interrupt Disable, SERR# Enable, Parity Error Response */
> > + val = dw_pcie_readl_dbi(dw, PCI_COMMAND);
> > + val |= PCI_COMMAND_PARITY | PCI_COMMAND_SERR |
> > + PCI_COMMAND_INTX_DISABLE;
> > + dw_pcie_writel_dbi(dw, PCI_COMMAND, val);
> > +
> > + /* Enable SERR */
> > + val = dw_pcie_readb_dbi(dw, PCI_BRIDGE_CONTROL);
> > + val |= PCI_BRIDGE_CTL_SERR;
> > + dw_pcie_writeb_dbi(dw, PCI_BRIDGE_CONTROL, val);
> > +
> > + /* Device control */
> > + val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_DEVCTL));
> > + val |= PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE |
> > + PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE;
> > + dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_DEVCTL), val);
>
> The above also looks like things that should be configured by the PCI
> core.
I think so. I realized that the following settings are not needed here.
So, I'll drop them.
> > + dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_RTCTL), val);
> > + dw_pcie_writel_dbi(dw, PCI_COMMAND, val);
> > + dw_pcie_writeb_dbi(dw, PCI_BRIDGE_CONTROL, val);
> > + dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_DEVCTL), val);
> > + dev_err(&pdev->dev, "Failed to initialize host\n");
> > + dev_err(dev, "failed to request resource: %d\n", err);
>
> Pick a capitalization style.
I'll fix the style.
> > + dev_err(dev, "%s: failed to resume/get Runtime PM\n", __func__);
>
> The driver name + device ID + message text printed by dev_err() should
> be enough that __func__ isn't needed.
I got it. I'll fix this on v8.
Best regards,
Yoshihiro Shimoda
> Bjorn
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v7 8/9] PCI: rcar-gen4-ep: Add R-Car Gen4 PCIe Endpoint support
2022-11-21 12:43 [PATCH v7 0/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
` (6 preceding siblings ...)
2022-11-21 12:43 ` [PATCH v7 7/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support Yoshihiro Shimoda
@ 2022-11-21 12:43 ` Yoshihiro Shimoda
2022-11-21 12:44 ` [PATCH v7 9/9] MAINTAINERS: Update PCI DRIVER FOR RENESAS R-CAR for R-Car Gen4 Yoshihiro Shimoda
8 siblings, 0 replies; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-21 12:43 UTC (permalink / raw)
To: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt
Cc: 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 | 182 ++++++++++++++++++
5 files changed, 197 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 99717b0f4e50..4889bde3471b 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -402,4 +402,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..be2b31856c69 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -765,6 +765,9 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
dw_pcie_version_detect(pci);
+ if (ep->ops->ep_pre_init)
+ ep->ops->ep_pre_init(ep);
+
dw_pcie_iatu_detect(pci);
ep->ib_window_map = devm_bitmap_zalloc(dev, pci->num_ib_windows,
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index ca0f9678517f..25c11a591ae2 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -324,6 +324,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..c043844a2acd
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c
@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PCIe Endpoint driver for Renesas R-Car Gen4 Series SoCs
+ * Copyright (C) 2022 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;
+ }
+
+ dw->ops->start_link(dw);
+
+ return 0;
+}
+
+static void rcar_gen4_remove_pcie_ep(struct rcar_gen4_pcie *rcar)
+{
+ 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] 27+ messages in thread* [PATCH v7 9/9] MAINTAINERS: Update PCI DRIVER FOR RENESAS R-CAR for R-Car Gen4
2022-11-21 12:43 [PATCH v7 0/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
` (7 preceding siblings ...)
2022-11-21 12:43 ` [PATCH v7 8/9] PCI: rcar-gen4-ep: Add R-Car Gen4 PCIe Endpoint support Yoshihiro Shimoda
@ 2022-11-21 12:44 ` Yoshihiro Shimoda
8 siblings, 0 replies; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-21 12:44 UTC (permalink / raw)
To: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt
Cc: 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 08b67532e374..4d62fe45032c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15972,6 +15972,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] 27+ messages in thread