2022-07-28 14:54:31

by Serge Semin

[permalink] [raw]
Subject: [PATCH v4 00/17] PCI: dwc: Add generic resources and Baikal-T1 support

This patchset is a third one in the series created in the framework of
my Baikal-T1 PCIe/eDMA-related work:

[1: Done v5] PCI: dwc: Various fixes and cleanups
Link: https://lore.kernel.org/linux-pci/[email protected]/
Link: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/?h=pci/ctrl/dwc-fixes
[2: Done v4] PCI: dwc: Add hw version and dma-ranges support
Link: https://lore.kernel.org/linux-pci/[email protected]
[3: In-review v4] PCI: dwc: Add generic resources and Baikal-T1 support
Link: ---you are looking at it---
[4: Done v4] dmaengine: dw-edma: Add RP/EP local DMA support
Link: https://lore.kernel.org/linux-pci/[email protected]/

Note it is very recommended to merge the patchsets in the same order as
they are listed in the set above in order to have them applied smoothly.
Nothing prevents them from being reviewed synchronously though.

@Bjorn, as we agreed here:
Link: https://lore.kernel.org/linux-pci/20220616163533.GA1094478@bhelgaas
could you please merge this series into the 'pci/edma' branch of your repo:
Link: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/log/?h=pci/edma
Thanks.

Originally the patches submitted in this patchset were a part of the series:
Link: https://lore.kernel.org/linux-pci/[email protected]/
but due to the reviewers requests the series was expanded to about 30
patches which made it too bulky for a comfortable review. So I decided to
split it up into two patchsets: 2. and 3. in the table above.

Regarding the series content. This patchset is mainly about adding new DW
PCIe platform support - Baikal-T1 PCIe of DW PCIe v4.60a IP-core. But a
set of feature-reach preparations are done first. It starts from
converting the currently available DT-schema into a more flexible schemas
hierarchy with separately defined regs, clocks, resets and interrupts
properties. As a result the common schema can be easily re-used by all the
currently available platforms while the named properties above can be
either re-defined or used as is if the platforms support they. In the
framework of that modification we also suggest to add a set of generic
regs, clocks, resets and interrupts resource names in accordance with what
the DW PCIe hardware reference manual describes and what the DW PCIe core
driver already expects to be specified. Thus the new platform driver will
be able to re-use the common resources infrastructure.

Link: https://lore.kernel.org/linux-pci/[email protected]/
Changelog v2:
- Rename 'syscon' property to 'baikal,bt1-syscon'. (@Rob)
- Move the iATU region selection procedure into a helper function (@Rob).
- Rebase from kernel v5.17 onto v5.18-rc3 since the later kernel has
already DT bindings converted. (@Rob)
- Use 'definitions' property instead of the '$defs' one. It fixes the
dt-validate error: 'X is not of type array.'
- Drop 'interrupts' and 'interrupt-names' property from being required
for the native DW PCIe host.
- Evaluate the 'snps,dw-pcie-common.yaml' schema in the
'socionext,uniphier-pcie-ep.yaml' DT-bindings since the later has
platform-specific names defined.

Link: https://lore.kernel.org/linux-pci/[email protected]
Changelog v3:
- Split up the patch "dt-bindings: PCI: dwc: Define common and native DT
bindings" into a series of modifications. (@Rob)
- Detach this series of the patches into a dedicated patchset.
- Add a new feature patch: "PCI: dwc: Introduce generic controller
capabilities interface".
- Add a new feature patch: "PCI: dwc: Introduce generic resources getter".
- Add a new cleanup patch: "PCI: dwc: Combine iATU detection procedures".
- Add a method to at least request the generic clocks and resets. (@Rob)
- Add GPIO-based PERST# signal support to the core module.
- Redefine Baikal-T1 PCIe host bridge config space accessors with the
pci_generic_config_read32() and pci_generic_config_write32() methods.
(@Rob)
- Drop synonymous from the names list in the common DT-schema since the
device sub-schemas create their own enumerations anyway.
- Rebase onto kernel v5.18.

Link: https://lore.kernel.org/linux-pci/[email protected]/
Changelog v4:
- Drop PCIBIOS_* macros usage. (@Rob)
- Add "static const" to the dw_pcie_ops and dw_pcie_host_ops structure
instances. (@Bjorn)
- Rename bt1_pcie_dw_ops to bt1_pcie_ops. (@Bjorn)
- Rename bt1_pcie_ops to bt1_pci_ops. (@Bjorn)
- Use start_link/stop_link suffixes in the Baikal-T1 PCIe
start/stop link callbacks. (@Bjorn)
- Change the get_res() method suffix to being get_resources(). (@Bjorn)
- Change *_{add,del}_dw_port() method to *_{add,del}_port(). (@Bjorn)
- Drop dma_coerce_mask_and_coherent() applied to the PCI host bridge
kernel device instance. (@Bjorn)
- Add the comment above the dma_set_mask_and_coherent() method usage
regarding the controller eDMA feature. (@Bjorn)
- Fix the comment above the core reset controls assertion. (@Bjorn)
- Replace delays and timeout numeric literals with macros. (@Bjorn)
- Convert the method name from dw_pcie_get_res() to
dw_pcie_get_resources(). (@Bjorn)
- Rebase onto the kernel v5.19-rcX.

Signed-off-by: Serge Semin <[email protected]>
Cc: Alexey Malahov <[email protected]>
Cc: Pavel Parkhomenko <[email protected]>
Cc: "Krzysztof WilczyƄski" <[email protected]>
Cc: Frank Li <[email protected]>
Cc: Manivannan Sadhasivam <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]

Serge Semin (17):
dt-bindings: PCI: dwc: Detach common RP/EP DT bindings
dt-bindings: PCI: dwc: Remove bus node from the examples
dt-bindings: PCI: dwc: Add phys/phy-names common properties
dt-bindings: PCI: dwc: Add max-link-speed common property
dt-bindings: PCI: dwc: Stop selecting generic bindings by default
dt-bindings: PCI: dwc: Add max-functions EP property
dt-bindings: PCI: dwc: Add interrupts/interrupt-names common
properties
dt-bindings: PCI: dwc: Add reg/reg-names common properties
dt-bindings: PCI: dwc: Add clocks/resets common properties
dt-bindings: PCI: dwc: Add dma-coherent property
dt-bindings: PCI: dwc: Apply common schema to Rockchip DW PCIe nodes
dt-bindings: PCI: dwc: Add Baikal-T1 PCIe Root Port bindings
PCI: dwc: Introduce generic controller capabilities interface
PCI: dwc: Introduce generic resources getter
PCI: dwc: Combine iATU detection procedures
PCI: dwc: Introduce generic platform clocks and resets
PCI: dwc: Add Baikal-T1 PCIe controller support

.../bindings/pci/baikal,bt1-pcie.yaml | 154 +++++
.../bindings/pci/fsl,imx6q-pcie.yaml | 3 +-
.../bindings/pci/hisilicon,kirin-pcie.yaml | 3 +-
.../bindings/pci/rockchip-dw-pcie.yaml | 3 +-
.../bindings/pci/samsung,exynos-pcie.yaml | 3 +-
.../bindings/pci/sifive,fu740-pcie.yaml | 3 +-
.../bindings/pci/snps,dw-pcie-common.yaml | 323 +++++++++
.../bindings/pci/snps,dw-pcie-ep.yaml | 155 +++--
.../devicetree/bindings/pci/snps,dw-pcie.yaml | 208 ++++--
.../pci/socionext,uniphier-pcie-ep.yaml | 12 +-
.../bindings/pci/toshiba,visconti-pcie.yaml | 3 +-
drivers/pci/controller/dwc/Kconfig | 9 +
drivers/pci/controller/dwc/Makefile | 1 +
drivers/pci/controller/dwc/pcie-bt1.c | 653 ++++++++++++++++++
.../pci/controller/dwc/pcie-designware-ep.c | 26 +-
.../pci/controller/dwc/pcie-designware-host.c | 15 +-
drivers/pci/controller/dwc/pcie-designware.c | 206 ++++--
drivers/pci/controller/dwc/pcie-designware.h | 57 +-
18 files changed, 1628 insertions(+), 209 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml
create mode 100644 Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
create mode 100644 drivers/pci/controller/dwc/pcie-bt1.c

--
2.35.1


2022-07-28 14:55:19

by Serge Semin

[permalink] [raw]
Subject: [PATCH v4 11/17] dt-bindings: PCI: dwc: Apply common schema to Rockchip DW PCIe nodes

As the DT-bindings description states the Rockchip PCIe controller is
based on the DW PCIe RP IP-core thus its DT-nodes are supposed to be
compatible with the common DW PCIe controller schema. Let's make sure they
evaluated against it by referring to the snps,dw-pcie-common.yaml schema
in the allOf sub-schemas composition.

Signed-off-by: Serge Semin <[email protected]>

---

Changelog v3:
- This is a new patch created on v3 lap of the series.
---
Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
index bc0a9d1db750..2afdc43a27ed 100644
--- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
@@ -14,10 +14,11 @@ maintainers:
description: |+
RK3568 SoC PCIe host controller is based on the Synopsys DesignWare
PCIe IP and thus inherits all the common properties defined in
- designware-pcie.txt.
+ snps,dw-pcie-common.yaml.

allOf:
- $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/snps,dw-pcie-common.yaml#

properties:
compatible:
--
2.35.1

2022-07-28 14:55:24

by Serge Semin

[permalink] [raw]
Subject: [PATCH v4 04/17] dt-bindings: PCI: dwc: Add max-link-speed common property

In accordance with [1] DW PCIe controllers support up to Gen5 link speed.
Let's add the max-link-speed property upper bound to 5 then. The DT
bindings of the particular devices are expected to setup more strict
constraint on that parameter.

[1] Synopsys DesignWare Cores PCI Express Controller Databook, Version
5.40a, March 2019, p. 27

Signed-off-by: Serge Semin <[email protected]>

---

Changelog v3:
- This is a new patch unpinned from the next one:
https://lore.kernel.org/linux-pci/[email protected]/
by the Rob' request. (@Rob)
---
Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml | 3 +++
Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml | 2 ++
Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml | 1 +
3 files changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
index 627a5d6625ba..b2fbe886981b 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
@@ -45,6 +45,9 @@ properties:
the peripheral devices available on the PCIe bus.
maxItems: 1

+ max-link-speed:
+ maximum: 5
+
num-lanes:
description:
Number of PCIe link lanes to use. Can be omitted should the already
diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
index dcd521aed213..fc3b5d4ac245 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
@@ -55,4 +55,6 @@ examples:

phys = <&pcie_phy0>, <&pcie_phy1>, <&pcie_phy2>, <&pcie_phy3>;
phy-names = "pcie0", "pcie1", "pcie2", "pcie3";
+
+ max-link-speed = <3>;
};
diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
index 4a5c8b933b52..01cedf51e0f8 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
@@ -74,4 +74,5 @@ examples:
phy-names = "pcie";

num-lanes = <1>;
+ max-link-speed = <3>;
};
--
2.35.1

2022-07-28 14:55:42

by Serge Semin

[permalink] [raw]
Subject: [PATCH v4 15/17] PCI: dwc: Combine iATU detection procedures

Since the iATU CSR region is now retrieved in the DW PCIe resources getter
there is no much benefits in the iATU detection procedures splitting up.
Therefore let's join the iATU unroll/viewport detection procedure with the
rest of the iATU parameters detection code. The resultant method will be
as coherent as before, while the redundant functions will be eliminated
thus producing more readable code.

Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Rob Herring <[email protected]>

---

Changelog v3:
- This is a new patch created on v3 lap of the series.
---
drivers/pci/controller/dwc/pcie-designware.c | 39 +++++---------------
1 file changed, 10 insertions(+), 29 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 7625e43a97cb..d1e57ed17f5a 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -631,26 +631,21 @@ static void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen)

}

-static bool dw_pcie_iatu_unroll_enabled(struct dw_pcie *pci)
-{
- u32 val;
-
- val = dw_pcie_readl_dbi(pci, PCIE_ATU_VIEWPORT);
- if (val == 0xffffffff)
- return true;
-
- return false;
-}
-
-static void dw_pcie_iatu_detect_regions(struct dw_pcie *pci)
+void dw_pcie_iatu_detect(struct dw_pcie *pci)
{
int max_region, ob, ib;
u32 val, min, dir;
u64 max;

- if (dw_pcie_cap_is(pci, IATU_UNROLL)) {
+ val = dw_pcie_readl_dbi(pci, PCIE_ATU_VIEWPORT);
+ if (val == 0xFFFFFFFF) {
+ dw_pcie_cap_set(pci, IATU_UNROLL);
+
max_region = min((int)pci->atu_size / 512, 256);
} else {
+ pci->atu_base = pci->dbi_base + PCIE_ATU_VIEWPORT_BASE;
+ pci->atu_size = PCIE_ATU_VIEWPORT_SIZE;
+
dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT, 0xFF);
max_region = dw_pcie_readl_dbi(pci, PCIE_ATU_VIEWPORT) + 1;
}
@@ -692,23 +687,9 @@ static void dw_pcie_iatu_detect_regions(struct dw_pcie *pci)
pci->num_ib_windows = ib;
pci->region_align = 1 << fls(min);
pci->region_limit = (max << 32) | (SZ_4G - 1);
-}
-
-void dw_pcie_iatu_detect(struct dw_pcie *pci)
-{
- if (dw_pcie_iatu_unroll_enabled(pci)) {
- dw_pcie_cap_set(pci, IATU_UNROLL);
- } else {
- pci->atu_base = pci->dbi_base + PCIE_ATU_VIEWPORT_BASE;
- pci->atu_size = PCIE_ATU_VIEWPORT_SIZE;
- }
-
- dw_pcie_iatu_detect_regions(pci);
-
- dev_info(pci->dev, "iATU unroll: %s\n", dw_pcie_cap_is(pci, IATU_UNROLL) ?
- "enabled" : "disabled");

- dev_info(pci->dev, "iATU regions: %u ob, %u ib, align %uK, limit %lluG\n",
+ dev_info(pci->dev, "iATU: unroll %s, %u ob, %u ib, align %uK, limit %lluG\n",
+ dw_pcie_cap_is(pci, IATU_UNROLL) ? "T" : "F",
pci->num_ob_windows, pci->num_ib_windows,
pci->region_align / SZ_1K, (pci->region_limit + 1) / SZ_1G);
}
--
2.35.1

2022-07-28 14:55:44

by Serge Semin

[permalink] [raw]
Subject: [PATCH v4 06/17] dt-bindings: PCI: dwc: Add max-functions EP property

In accordance with [1] the CX_NFUNC IP-core synthesize parameter is
responsible for the number of physical functions to support in the EP
mode. Its upper limit is 32. Let's use it to constrain the number of
PCIe functions the DW PCIe EP DT-nodes can advertise.

[1] Synopsys DesignWare Cores PCI Express Controller Databook - DWC PCIe
Endpoint, Version 5.40a, March 2019, p. 887.

Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Rob Herring <[email protected]>

---

Changelog v3:
- This is a new patch unpinned from the next one:
https://lore.kernel.org/linux-pci/[email protected]/
by the Rob' request. (@Rob)
---
Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
index b04ce7ddb796..9411366d6ca7 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
@@ -53,6 +53,9 @@ properties:
items:
enum: [dbi, dbi2, config, atu, addr_space, link, atu_dma, appl]

+ max-functions:
+ maximum: 32
+
required:
- compatible
- reg
@@ -73,4 +76,5 @@ examples:
phy-names = "pcie0", "pcie1", "pcie2", "pcie3";

max-link-speed = <3>;
+ max-functions = /bits/ 8 <4>;
};
--
2.35.1

2022-07-28 14:55:53

by Serge Semin

[permalink] [raw]
Subject: [PATCH v4 03/17] dt-bindings: PCI: dwc: Add phys/phy-names common properties

It's normal to have the DW PCIe RP/EP DT-nodes equipped with the explicit
PHY phandle references. There can be up to 16 PHYs attach in accordance
with the maximum number of supported PCIe lanes. Let's extend the common
DW PCIe controller schema with the 'phys' and 'phy-names' properties
definition. The PHY names are defined with the regexp pattern
'^pcie([0-9]+|-?phy[0-9]*)?$' so to match the names currently supported by
the DW PCIe platform drivers ("pcie": meson; "pciephy": qcom, imx6;
"pcie-phy": uniphier, rockchip, spear13xx; "pcie": intel-gw; "pcie-phy%d":
keystone, dra7xx; "pcie": histb, etc). Though the "pcie%d" format would
the most preferable in this case.

Signed-off-by: Serge Semin <[email protected]>

---

Changelog v3:
- This is a new patch unpinned from the next one:
https://lore.kernel.org/linux-pci/[email protected]/
by the Rob' request. (@Rob)
---
.../bindings/pci/snps,dw-pcie-common.yaml | 15 +++++++++++++++
.../devicetree/bindings/pci/snps,dw-pcie-ep.yaml | 3 +++
.../devicetree/bindings/pci/snps,dw-pcie.yaml | 3 +++
3 files changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
index 3e992b653d12..627a5d6625ba 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
@@ -17,6 +17,21 @@ description:
select: false

properties:
+ phys:
+ description:
+ There can be up to the number of possible lanes PHYs specified.
+ Obviously each specified PHY is supposed to be able to work in the
+ PCIe mode with a speed implied by the DWC PCIe controller it is
+ attached to.
+ minItems: 1
+ maxItems: 16
+
+ phy-names:
+ minItems: 1
+ maxItems: 16
+ items:
+ pattern: '^pcie([0-9]+|-?phy[0-9]*)?$'
+
reset-gpio:
deprecated: true
description:
diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
index 7d05dcba419b..dcd521aed213 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
@@ -52,4 +52,7 @@ examples:
<0xdfc01000 0x0001000>, /* IP registers 2 */
<0xd0000000 0x2000000>; /* Configuration space */
reg-names = "dbi", "dbi2", "addr_space";
+
+ phys = <&pcie_phy0>, <&pcie_phy1>, <&pcie_phy2>, <&pcie_phy3>;
+ phy-names = "pcie0", "pcie1", "pcie2", "pcie3";
};
diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
index 2810e9b5cc8d..4a5c8b933b52 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
@@ -70,5 +70,8 @@ examples:

reset-gpios = <&port0 0 1>;

+ phys = <&pcie_phy>;
+ phy-names = "pcie";
+
num-lanes = <1>;
};
--
2.35.1

2022-07-28 14:56:02

by Serge Semin

[permalink] [raw]
Subject: [PATCH v4 17/17] PCI: dwc: Add Baikal-T1 PCIe controller support

Baikal-T1 SoC is equipped with DWC PCIe v4.60a host controller. It can be
trained to work up to Gen.3 speed over up to x4 lanes. The host controller
is attached to the DW PCIe 3.0 PCS via the PIPE-4 interface, which in its
turn is connected to the DWC 10G PHY. The whole system is supposed to be
fed up with four clock sources: DBI peripheral clock, AXI application
clocks and external PHY/core reference clock generating the 100MHz signal.
In addition to that the platform provide a way to reset each part of the
controller: sticky/non-sticky bits, host controller core, PIPE interface,
PCS/PHY and Hot/Power reset signal. The driver also provides a way to
handle the GPIO-based PERST# signal.

Note due to the Baikal-T1 MMIO peculiarity we have to implement the DBI
interface accessors which make sure the IO operations are dword-aligned.

Signed-off-by: Serge Semin <[email protected]>

---

Changelog v2:
- Rename 'syscon' property to 'baikal,bt1-syscon'.

Changelog v3:
- Use the clocks/resets handlers defined in the DW PCIe core descriptor.
(@Rob)
- Redefine PCI host bridge config space accessors with the generic
pci_generic_config_read32() and pci_generic_config_write32() methods.
(@Rob)

Changelog v4:
- Drop PCIBIOS_* macros usage. (@Rob)
- Add "static const" to the dw_pcie_ops and dw_pcie_host_ops structure
instances. (@Bjorn)
- Rename bt1_pcie_dw_ops to bt1_pcie_ops. (@Bjorn)
- Rename bt1_pcie_ops to bt1_pci_ops. (@Bjorn)
- Use start_link/stop_link suffixes in the corresponding callbacks.
(@Bjorn)
- Change the get_res() method suffix to being get_resources(). (@Bjorn)
- Change *_{add,del}_dw_port() method to *_{add,del}_port(). (@Bjorn)
- Drop dma_coerce_mask_and_coherent() applied to the PCI host bridge
kernel device instance. (@Bjorn)
- Add the comment above the dma_set_mask_and_coherent() method usage
regarding the controller eDMA feature. (@Bjorn)
- Fix the comment above the core reset controls assertion. (@Bjorn)
- Replace delays and timeout numeric literals with macros. (@Bjorn)
---
drivers/pci/controller/dwc/Kconfig | 9 +
drivers/pci/controller/dwc/Makefile | 1 +
drivers/pci/controller/dwc/pcie-bt1.c | 653 ++++++++++++++++++++++++++
3 files changed, 663 insertions(+)
create mode 100644 drivers/pci/controller/dwc/pcie-bt1.c

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 62ce3abf0f19..771b8b146623 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -222,6 +222,15 @@ config PCIE_ARTPEC6_EP
Enables support for the PCIe controller in the ARTPEC-6 SoC to work in
endpoint mode. This uses the DesignWare core.

+config PCIE_BT1
+ tristate "Baikal-T1 PCIe controller"
+ depends on MIPS_BAIKAL_T1 || COMPILE_TEST
+ depends on PCI_MSI_IRQ_DOMAIN
+ select PCIE_DW_HOST
+ help
+ Enables support for the PCIe controller in the Baikal-T1 SoC to work
+ in host mode. It's based on the Synopsys DWC PCIe v4.60a IP-core.
+
config PCIE_ROCKCHIP_DW_HOST
bool "Rockchip DesignWare PCIe controller"
select PCIE_DW
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index 8ba7b67f5e50..bf5c311875a1 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o
obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
+obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o
obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
obj-$(CONFIG_PCIE_FU740) += pcie-fu740.o
diff --git a/drivers/pci/controller/dwc/pcie-bt1.c b/drivers/pci/controller/dwc/pcie-bt1.c
new file mode 100644
index 000000000000..86b230575ddc
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-bt1.c
@@ -0,0 +1,653 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2021 BAIKAL ELECTRONICS, JSC
+ *
+ * Authors:
+ * Vadim Vlasov <[email protected]>
+ * Serge Semin <[email protected]>
+ *
+ * Baikal-T1 PCIe controller driver
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/types.h>
+
+#include "pcie-designware.h"
+
+/* Baikal-T1 System CCU control registers */
+#define BT1_CCU_PCIE_CLKC 0x140
+#define BT1_CCU_PCIE_REQ_PCS_CLK BIT(16)
+#define BT1_CCU_PCIE_REQ_MAC_CLK BIT(17)
+#define BT1_CCU_PCIE_REQ_PIPE_CLK BIT(18)
+
+#define BT1_CCU_PCIE_RSTC 0x144
+#define BT1_CCU_PCIE_REQ_LINK_RST BIT(13)
+#define BT1_CCU_PCIE_REQ_SMLH_RST BIT(14)
+#define BT1_CCU_PCIE_REQ_PHY_RST BIT(16)
+#define BT1_CCU_PCIE_REQ_CORE_RST BIT(24)
+#define BT1_CCU_PCIE_REQ_STICKY_RST BIT(26)
+#define BT1_CCU_PCIE_REQ_NSTICKY_RST BIT(27)
+
+#define BT1_CCU_PCIE_PMSC 0x148
+#define BT1_CCU_PCIE_LTSSM_STATE_MASK GENMASK(5, 0)
+#define BT1_CCU_PCIE_LTSSM_DET_QUIET 0x00
+#define BT1_CCU_PCIE_LTSSM_DET_ACT 0x01
+#define BT1_CCU_PCIE_LTSSM_POLL_ACT 0x02
+#define BT1_CCU_PCIE_LTSSM_POLL_COMP 0x03
+#define BT1_CCU_PCIE_LTSSM_POLL_CONF 0x04
+#define BT1_CCU_PCIE_LTSSM_PRE_DET_QUIET 0x05
+#define BT1_CCU_PCIE_LTSSM_DET_WAIT 0x06
+#define BT1_CCU_PCIE_LTSSM_CFG_LNKWD_START 0x07
+#define BT1_CCU_PCIE_LTSSM_CFG_LNKWD_ACEPT 0x08
+#define BT1_CCU_PCIE_LTSSM_CFG_LNNUM_WAIT 0x09
+#define BT1_CCU_PCIE_LTSSM_CFG_LNNUM_ACEPT 0x0a
+#define BT1_CCU_PCIE_LTSSM_CFG_COMPLETE 0x0b
+#define BT1_CCU_PCIE_LTSSM_CFG_IDLE 0x0c
+#define BT1_CCU_PCIE_LTSSM_RCVR_LOCK 0x0d
+#define BT1_CCU_PCIE_LTSSM_RCVR_SPEED 0x0e
+#define BT1_CCU_PCIE_LTSSM_RCVR_RCVRCFG 0x0f
+#define BT1_CCU_PCIE_LTSSM_RCVR_IDLE 0x10
+#define BT1_CCU_PCIE_LTSSM_L0 0x11
+#define BT1_CCU_PCIE_LTSSM_L0S 0x12
+#define BT1_CCU_PCIE_LTSSM_L123_SEND_IDLE 0x13
+#define BT1_CCU_PCIE_LTSSM_L1_IDLE 0x14
+#define BT1_CCU_PCIE_LTSSM_L2_IDLE 0x15
+#define BT1_CCU_PCIE_LTSSM_L2_WAKE 0x16
+#define BT1_CCU_PCIE_LTSSM_DIS_ENTRY 0x17
+#define BT1_CCU_PCIE_LTSSM_DIS_IDLE 0x18
+#define BT1_CCU_PCIE_LTSSM_DISABLE 0x19
+#define BT1_CCU_PCIE_LTSSM_LPBK_ENTRY 0x1a
+#define BT1_CCU_PCIE_LTSSM_LPBK_ACTIVE 0x1b
+#define BT1_CCU_PCIE_LTSSM_LPBK_EXIT 0x1c
+#define BT1_CCU_PCIE_LTSSM_LPBK_EXIT_TOUT 0x1d
+#define BT1_CCU_PCIE_LTSSM_HOT_RST_ENTRY 0x1e
+#define BT1_CCU_PCIE_LTSSM_HOT_RST 0x1f
+#define BT1_CCU_PCIE_LTSSM_RCVR_EQ0 0x20
+#define BT1_CCU_PCIE_LTSSM_RCVR_EQ1 0x21
+#define BT1_CCU_PCIE_LTSSM_RCVR_EQ2 0x22
+#define BT1_CCU_PCIE_LTSSM_RCVR_EQ3 0x23
+#define BT1_CCU_PCIE_SMLH_LINKUP BIT(6)
+#define BT1_CCU_PCIE_RDLH_LINKUP BIT(7)
+#define BT1_CCU_PCIE_PM_LINKSTATE_L0S BIT(8)
+#define BT1_CCU_PCIE_PM_LINKSTATE_L1 BIT(9)
+#define BT1_CCU_PCIE_PM_LINKSTATE_L2 BIT(10)
+#define BT1_CCU_PCIE_L1_PENDING BIT(12)
+#define BT1_CCU_PCIE_REQ_EXIT_L1 BIT(14)
+#define BT1_CCU_PCIE_LTSSM_RCVR_EQ BIT(15)
+#define BT1_CCU_PCIE_PM_DSTAT_MASK GENMASK(18, 16)
+#define BT1_CCU_PCIE_PM_PME_EN BIT(20)
+#define BT1_CCU_PCIE_PM_PME_STATUS BIT(21)
+#define BT1_CCU_PCIE_AUX_PM_EN BIT(22)
+#define BT1_CCU_PCIE_AUX_PWR_DET BIT(23)
+#define BT1_CCU_PCIE_WAKE_DET BIT(24)
+#define BT1_CCU_PCIE_TURNOFF_REQ BIT(30)
+#define BT1_CCU_PCIE_TURNOFF_ACK BIT(31)
+
+#define BT1_CCU_PCIE_GENC 0x14c
+#define BT1_CCU_PCIE_LTSSM_EN BIT(1)
+#define BT1_CCU_PCIE_DBI2_MODE BIT(2)
+#define BT1_CCU_PCIE_MGMT_EN BIT(3)
+#define BT1_CCU_PCIE_RXLANE_FLIP_EN BIT(16)
+#define BT1_CCU_PCIE_TXLANE_FLIP_EN BIT(17)
+#define BT1_CCU_PCIE_SLV_XFER_PEND BIT(24)
+#define BT1_CCU_PCIE_RCV_XFER_PEND BIT(25)
+#define BT1_CCU_PCIE_DBI_XFER_PEND BIT(26)
+#define BT1_CCU_PCIE_DMA_XFER_PEND BIT(27)
+
+#define BT1_CCU_PCIE_LTSSM_LINKUP(_pmsc) \
+({ \
+ int __state = FIELD_GET(BT1_CCU_PCIE_LTSSM_STATE_MASK, _pmsc); \
+ __state >= BT1_CCU_PCIE_LTSSM_L0 && __state <= BT1_CCU_PCIE_LTSSM_L2_WAKE; \
+})
+
+/* Baikal-T1 PCIe specific control registers */
+#define BT1_PCIE_AXI2MGM_LANENUM 0xd04
+#define BT1_PCIE_AXI2MGM_LANESEL_MASK GENMASK(3, 0)
+
+#define BT1_PCIE_AXI2MGM_ADDRCTL 0xd08
+#define BT1_PCIE_AXI2MGM_PHYREG_ADDR_MASK GENMASK(20, 0)
+#define BT1_PCIE_AXI2MGM_READ_FLAG BIT(29)
+#define BT1_PCIE_AXI2MGM_DONE BIT(30)
+#define BT1_PCIE_AXI2MGM_BUSY BIT(31)
+
+#define BT1_PCIE_AXI2MGM_WRITEDATA 0xd0c
+#define BT1_PCIE_AXI2MGM_WDATA GENMASK(15, 0)
+
+#define BT1_PCIE_AXI2MGM_READDATA 0xd10
+#define BT1_PCIE_AXI2MGM_RDATA GENMASK(15, 0)
+
+/* Generic Baikal-T1 PCIe interface resources */
+#define BT1_PCIE_NUM_APP_CLKS ARRAY_SIZE(bt1_pcie_app_clks)
+#define BT1_PCIE_NUM_CORE_CLKS ARRAY_SIZE(bt1_pcie_core_clks)
+#define BT1_PCIE_NUM_APP_RSTS ARRAY_SIZE(bt1_pcie_app_rsts)
+#define BT1_PCIE_NUM_CORE_RSTS ARRAY_SIZE(bt1_pcie_core_rsts)
+
+/* PCIe bus setup delays and timeouts */
+#define BT1_PCIE_RST_DELAY_MS 100
+#define BT1_PCIE_RUN_DELAY_US 100
+#define BT1_PCIE_REQ_DELAY_US 1
+#define BT1_PCIE_REQ_TIMEOUT_US 1000
+#define BT1_PCIE_LNK_DELAY_US 1000
+#define BT1_PCIE_LNK_TIMEOUT_US 1000000
+
+static const enum dw_pcie_app_clk bt1_pcie_app_clks[] = {
+ DW_PCIE_DBI_CLK, DW_PCIE_MSTR_CLK, DW_PCIE_SLV_CLK,
+};
+
+static const enum dw_pcie_core_clk bt1_pcie_core_clks[] = {
+ DW_PCIE_REF_CLK,
+};
+
+static const enum dw_pcie_app_rst bt1_pcie_app_rsts[] = {
+ DW_PCIE_MSTR_RST, DW_PCIE_SLV_RST,
+};
+
+static const enum dw_pcie_core_rst bt1_pcie_core_rsts[] = {
+ DW_PCIE_NON_STICKY_RST, DW_PCIE_STICKY_RST, DW_PCIE_CORE_RST,
+ DW_PCIE_PIPE_RST, DW_PCIE_PHY_RST, DW_PCIE_HOT_RST, DW_PCIE_PWR_RST,
+};
+
+struct bt1_pcie {
+ struct dw_pcie dw;
+ struct platform_device *pdev;
+ struct regmap *sys_regs;
+};
+#define to_bt1_pcie(_dw) container_of(_dw, struct bt1_pcie, dw)
+
+/*
+ * Baikal-T1 MMIO space must be read/written by the dword-aligned
+ * instructions. Note the methods are optimized to have the dword operations
+ * performed with minimum overhead as the most frequently used ones.
+ */
+static int bt1_pcie_read_mmio(void __iomem *addr, int size, u32 *val)
+{
+ unsigned int ofs = (uintptr_t)addr & 0x3;
+
+ if (!IS_ALIGNED((uintptr_t)addr, size))
+ return -EINVAL;
+
+ *val = readl(addr - ofs) >> ofs * BITS_PER_BYTE;
+ if (size == 4) {
+ return 0;
+ } else if (size == 2) {
+ *val &= 0xffff;
+ return 0;
+ } else if (size == 1) {
+ *val &= 0xff;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static int bt1_pcie_write_mmio(void __iomem *addr, int size, u32 val)
+{
+ unsigned int ofs = (uintptr_t)addr & 0x3;
+ u32 tmp, mask;
+
+ if (!IS_ALIGNED((uintptr_t)addr, size))
+ return -EINVAL;
+
+ if (size == 4) {
+ writel(val, addr);
+ return 0;
+ } else if (size == 2 || size == 1) {
+ mask = GENMASK(size * BITS_PER_BYTE - 1, 0);
+ tmp = readl(addr - ofs) & ~(mask << ofs * BITS_PER_BYTE);
+ tmp |= (val & mask) << ofs * BITS_PER_BYTE;
+ writel(tmp, addr - ofs);
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static u32 bt1_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base, u32 reg,
+ size_t size)
+{
+ int ret;
+ u32 val;
+
+ ret = bt1_pcie_read_mmio(base + reg, size, &val);
+ if (ret) {
+ dev_err(pci->dev, "Read DBI address failed\n");
+ return ~0U;
+ }
+
+ return val;
+}
+
+static void bt1_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base, u32 reg,
+ size_t size, u32 val)
+{
+ int ret;
+
+ ret = bt1_pcie_write_mmio(base + reg, size, val);
+ if (ret)
+ dev_err(pci->dev, "Write DBI address failed\n");
+}
+
+static void bt1_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base, u32 reg,
+ size_t size, u32 val)
+{
+ struct bt1_pcie *btpci = to_bt1_pcie(pci);
+ int ret;
+
+ regmap_update_bits(btpci->sys_regs, BT1_CCU_PCIE_GENC,
+ BT1_CCU_PCIE_DBI2_MODE, BT1_CCU_PCIE_DBI2_MODE);
+
+ ret = bt1_pcie_write_mmio(base + reg, size, val);
+ if (ret)
+ dev_err(pci->dev, "Write DBI2 address failed\n");
+
+ regmap_update_bits(btpci->sys_regs, BT1_CCU_PCIE_GENC,
+ BT1_CCU_PCIE_DBI2_MODE, 0);
+}
+
+static int bt1_pcie_start_link(struct dw_pcie *pci)
+{
+ struct bt1_pcie *btpci = to_bt1_pcie(pci);
+ u32 val;
+ int ret;
+
+ /*
+ * Enable LTSSM and make sure it was able to establish both PHY and
+ * data links. This procedure shall work fine to reach 2.5 GT/s speed.
+ */
+ regmap_update_bits(btpci->sys_regs, BT1_CCU_PCIE_GENC,
+ BT1_CCU_PCIE_LTSSM_EN, BT1_CCU_PCIE_LTSSM_EN);
+
+ ret = regmap_read_poll_timeout(btpci->sys_regs, BT1_CCU_PCIE_PMSC, val,
+ (val & BT1_CCU_PCIE_SMLH_LINKUP),
+ BT1_PCIE_LNK_DELAY_US, BT1_PCIE_LNK_TIMEOUT_US);
+ if (ret) {
+ dev_err(pci->dev, "LTSSM failed to set PHY link up\n");
+ return ret;
+ }
+
+ ret = regmap_read_poll_timeout(btpci->sys_regs, BT1_CCU_PCIE_PMSC, val,
+ (val & BT1_CCU_PCIE_RDLH_LINKUP),
+ BT1_PCIE_LNK_DELAY_US, BT1_PCIE_LNK_TIMEOUT_US);
+ if (ret) {
+ dev_err(pci->dev, "LTSSM failed to set data link up\n");
+ return ret;
+ }
+
+ /*
+ * Activate direct speed change after the link is established in an
+ * attempt to reach a higher bus performance (up to Gen.3 - 8.0 GT/s).
+ * This is required at least to get 8.0 GT/s speed.
+ */
+ val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
+ val |= PORT_LOGIC_SPEED_CHANGE;
+ dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
+
+ ret = regmap_read_poll_timeout(btpci->sys_regs, BT1_CCU_PCIE_PMSC, val,
+ BT1_CCU_PCIE_LTSSM_LINKUP(val),
+ BT1_PCIE_LNK_DELAY_US, BT1_PCIE_LNK_TIMEOUT_US);
+ if (ret)
+ dev_err(pci->dev, "LTSSM failed to get into L0 state\n");
+
+ return ret;
+}
+
+static void bt1_pcie_stop_link(struct dw_pcie *pci)
+{
+ struct bt1_pcie *btpci = to_bt1_pcie(pci);
+
+ regmap_update_bits(btpci->sys_regs, BT1_CCU_PCIE_GENC,
+ BT1_CCU_PCIE_LTSSM_EN, 0);
+}
+
+static const struct dw_pcie_ops bt1_pcie_ops = {
+ .read_dbi = bt1_pcie_read_dbi,
+ .write_dbi = bt1_pcie_write_dbi,
+ .write_dbi2 = bt1_pcie_write_dbi2,
+ .start_link = bt1_pcie_start_link,
+ .stop_link = bt1_pcie_stop_link,
+};
+
+static struct pci_ops bt1_pci_ops = {
+ .map_bus = dw_pcie_own_conf_map_bus,
+ .read = pci_generic_config_read32,
+ .write = pci_generic_config_write32,
+};
+
+static int bt1_pcie_get_resources(struct bt1_pcie *btpci)
+{
+ struct device *dev = btpci->dw.dev;
+ int i;
+
+ /* DBI access is supposed to be performed by the dword-aligned IOs */
+ btpci->dw.pp.bridge->ops = &bt1_pci_ops;
+
+ /* These CSRs are in MMIO so we won't check the regmap-methods status */
+ btpci->sys_regs =
+ syscon_regmap_lookup_by_phandle(dev->of_node, "baikal,bt1-syscon");
+ if (IS_ERR(btpci->sys_regs))
+ return dev_err_probe(dev, PTR_ERR(btpci->sys_regs),
+ "Failed to get syscon\n");
+
+ /* Make sure all the required resources have been specified */
+ for (i = 0; i < BT1_PCIE_NUM_APP_CLKS; i++) {
+ if (!btpci->dw.app_clks[bt1_pcie_app_clks[i]].clk) {
+ dev_err(dev, "App clocks set is incomplete\n");
+ return -ENOENT;
+ }
+ }
+
+ for (i = 0; i < BT1_PCIE_NUM_CORE_CLKS; i++) {
+ if (!btpci->dw.core_clks[bt1_pcie_core_clks[i]].clk) {
+ dev_err(dev, "Core clocks set is incomplete\n");
+ return -ENOENT;
+ }
+ }
+
+ for (i = 0; i < BT1_PCIE_NUM_APP_RSTS; i++) {
+ if (!btpci->dw.app_rsts[bt1_pcie_app_rsts[i]].rstc) {
+ dev_err(dev, "App resets set is incomplete\n");
+ return -ENOENT;
+ }
+ }
+
+ for (i = 0; i < BT1_PCIE_NUM_CORE_RSTS; i++) {
+ if (!btpci->dw.core_rsts[bt1_pcie_core_rsts[i]].rstc) {
+ dev_err(dev, "Core resets set is incomplete\n");
+ return -ENOENT;
+ }
+ }
+
+ return 0;
+}
+
+static void bt1_pcie_full_stop_bus(struct bt1_pcie *btpci, bool init)
+{
+ struct device *dev = btpci->dw.dev;
+ struct dw_pcie *pci = &btpci->dw;
+ int ret;
+
+ /* Disable LTSSM for sure */
+ regmap_update_bits(btpci->sys_regs, BT1_CCU_PCIE_GENC,
+ BT1_CCU_PCIE_LTSSM_EN, 0);
+
+ /*
+ * Application reset controls are trigger-based so assert the core
+ * resets only.
+ */
+ ret = reset_control_bulk_assert(DW_PCIE_NUM_CORE_RSTS, pci->core_rsts);
+ if (ret)
+ dev_err(dev, "Failed to assert core resets\n");
+
+ /*
+ * Clocks are disabled by default at least in accordance with the clk
+ * enable counter value on init stage.
+ */
+ if (!init) {
+ clk_bulk_disable_unprepare(DW_PCIE_NUM_CORE_CLKS, pci->core_clks);
+
+ clk_bulk_disable_unprepare(DW_PCIE_NUM_APP_CLKS, pci->app_clks);
+ }
+
+ /* The peripheral devices are unavailable anyway so reset them too */
+ gpiod_set_value_cansleep(pci->pe_rst, 1);
+
+ /* Make sure all the resets are settled */
+ msleep(BT1_PCIE_RST_DELAY_MS);
+}
+
+/*
+ * Implements the cold reset procedure in accordance with the reference manual
+ * and available PM signals.
+ */
+static int bt1_pcie_cold_start_bus(struct bt1_pcie *btpci)
+{
+ struct device *dev = btpci->dw.dev;
+ struct dw_pcie *pci = &btpci->dw;
+ u32 val;
+ int ret;
+
+ /* First get out of the Power/Hot reset state */
+ ret = reset_control_deassert(pci->core_rsts[DW_PCIE_PWR_RST].rstc);
+ if (ret) {
+ dev_err(dev, "Failed to deassert PHY reset\n");
+ return ret;
+ }
+
+ ret = reset_control_deassert(pci->core_rsts[DW_PCIE_HOT_RST].rstc);
+ if (ret) {
+ dev_err(dev, "Failed to deassert hot reset\n");
+ goto err_assert_pwr_rst;
+ }
+
+ /* Wait for the PM-core to stop requesting the PHY reset */
+ ret = regmap_read_poll_timeout(btpci->sys_regs, BT1_CCU_PCIE_RSTC, val,
+ !(val & BT1_CCU_PCIE_REQ_PHY_RST),
+ BT1_PCIE_REQ_DELAY_US, BT1_PCIE_REQ_TIMEOUT_US);
+ if (ret) {
+ dev_err(dev, "Timed out waiting for PM to stop PHY resetting\n");
+ goto err_assert_hot_rst;
+ }
+
+ ret = reset_control_deassert(pci->core_rsts[DW_PCIE_PHY_RST].rstc);
+ if (ret) {
+ dev_err(dev, "Failed to deassert PHY reset\n");
+ goto err_assert_hot_rst;
+ }
+
+ /* Clocks can be now enabled, but the ref one is crucial at this stage */
+ ret = clk_bulk_prepare_enable(DW_PCIE_NUM_APP_CLKS, pci->app_clks);
+ if (ret) {
+ dev_err(dev, "Failed to enable app clocks\n");
+ goto err_assert_phy_rst;
+ }
+
+ ret = clk_bulk_prepare_enable(DW_PCIE_NUM_CORE_CLKS, pci->core_clks);
+ if (ret) {
+ dev_err(dev, "Failed to enable ref clocks\n");
+ goto err_disable_app_clk;
+ }
+
+ /* Wait for the PM to stop requesting the controller core reset */
+ ret = regmap_read_poll_timeout(btpci->sys_regs, BT1_CCU_PCIE_RSTC, val,
+ !(val & BT1_CCU_PCIE_REQ_CORE_RST),
+ BT1_PCIE_REQ_DELAY_US, BT1_PCIE_REQ_TIMEOUT_US);
+ if (ret) {
+ dev_err(dev, "Timed out waiting for PM to stop core resetting\n");
+ goto err_disable_core_clk;
+ }
+
+ /* PCS-PIPE interface and controller core can be now activated */
+ ret = reset_control_deassert(pci->core_rsts[DW_PCIE_PIPE_RST].rstc);
+ if (ret) {
+ dev_err(dev, "Failed to deassert PIPE reset\n");
+ goto err_disable_core_clk;
+ }
+
+ ret = reset_control_deassert(pci->core_rsts[DW_PCIE_CORE_RST].rstc);
+ if (ret) {
+ dev_err(dev, "Failed to deassert core reset\n");
+ goto err_assert_pipe_rst;
+ }
+
+ /* It's recommended to reset the core and application logic together */
+ ret = reset_control_bulk_reset(DW_PCIE_NUM_APP_RSTS, pci->app_rsts);
+ if (ret) {
+ dev_err(dev, "Failed to reset app domain\n");
+ goto err_assert_core_rst;
+ }
+
+ /* Sticky/Non-sticky CSR flags can be now unreset too */
+ ret = reset_control_deassert(pci->core_rsts[DW_PCIE_STICKY_RST].rstc);
+ if (ret) {
+ dev_err(dev, "Failed to deassert sticky reset\n");
+ goto err_assert_core_rst;
+ }
+
+ ret = reset_control_deassert(pci->core_rsts[DW_PCIE_NON_STICKY_RST].rstc);
+ if (ret) {
+ dev_err(dev, "Failed to deassert non-sticky reset\n");
+ goto err_assert_sticky_rst;
+ }
+
+ /* Activate the PCIe bus peripheral devices */
+ gpiod_set_value_cansleep(pci->pe_rst, 0);
+
+ /* Make sure the state is settled (LTSSM is still disabled though) */
+ usleep_range(BT1_PCIE_RUN_DELAY_US, BT1_PCIE_RUN_DELAY_US + 100);
+
+ return 0;
+
+err_assert_sticky_rst:
+ reset_control_assert(pci->core_rsts[DW_PCIE_STICKY_RST].rstc);
+
+err_assert_core_rst:
+ reset_control_assert(pci->core_rsts[DW_PCIE_CORE_RST].rstc);
+
+err_assert_pipe_rst:
+ reset_control_assert(pci->core_rsts[DW_PCIE_PIPE_RST].rstc);
+
+err_disable_core_clk:
+ clk_bulk_disable_unprepare(DW_PCIE_NUM_CORE_CLKS, pci->core_clks);
+
+err_disable_app_clk:
+ clk_bulk_disable_unprepare(DW_PCIE_NUM_APP_CLKS, pci->app_clks);
+
+err_assert_phy_rst:
+ reset_control_assert(pci->core_rsts[DW_PCIE_PHY_RST].rstc);
+
+err_assert_hot_rst:
+ reset_control_assert(pci->core_rsts[DW_PCIE_HOT_RST].rstc);
+
+err_assert_pwr_rst:
+ reset_control_assert(pci->core_rsts[DW_PCIE_PWR_RST].rstc);
+
+ return ret;
+}
+
+static int bt1_pcie_host_init(struct dw_pcie_rp *pp)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+ struct bt1_pcie *btpci = to_bt1_pcie(pci);
+ int ret;
+
+ ret = bt1_pcie_get_resources(btpci);
+ if (ret)
+ return ret;
+
+ bt1_pcie_full_stop_bus(btpci, true);
+
+ return bt1_pcie_cold_start_bus(btpci);
+}
+
+static void bt1_pcie_host_deinit(struct dw_pcie_rp *pp)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+ struct bt1_pcie *btpci = to_bt1_pcie(pci);
+
+ bt1_pcie_full_stop_bus(btpci, false);
+}
+
+static const struct dw_pcie_host_ops bt1_pcie_host_ops = {
+ .host_init = bt1_pcie_host_init,
+ .host_deinit = bt1_pcie_host_deinit,
+};
+
+static struct bt1_pcie *bt1_pcie_create_data(struct platform_device *pdev)
+{
+ struct bt1_pcie *btpci;
+
+ btpci = devm_kzalloc(&pdev->dev, sizeof(*btpci), GFP_KERNEL);
+ if (!btpci)
+ return ERR_PTR(-ENOMEM);
+
+ btpci->pdev = pdev;
+
+ platform_set_drvdata(pdev, btpci);
+
+ return btpci;
+}
+
+static int bt1_pcie_add_port(struct bt1_pcie *btpci)
+{
+ struct device *dev = &btpci->pdev->dev;
+ int ret;
+
+ /*
+ * DW PCIe Root Port controller is equipped with eDMA capable of
+ * working with the 64-bit memory addresses.
+ */
+ ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+ if (ret)
+ return ret;
+
+ btpci->dw.version = DW_PCIE_VER_460A;
+ btpci->dw.dev = dev;
+ btpci->dw.ops = &bt1_pcie_ops;
+
+ btpci->dw.pp.num_vectors = MAX_MSI_IRQS;
+ btpci->dw.pp.ops = &bt1_pcie_host_ops;
+
+ dw_pcie_cap_set(&btpci->dw, REQ_RES);
+
+ ret = dw_pcie_host_init(&btpci->dw.pp);
+ if (ret)
+ dev_err_probe(dev, ret, "Failed to initialize DWC PCIe host\n");
+
+ return ret;
+}
+
+static void bt1_pcie_del_port(struct bt1_pcie *btpci)
+{
+ dw_pcie_host_deinit(&btpci->dw.pp);
+}
+
+static int bt1_pcie_probe(struct platform_device *pdev)
+{
+ struct bt1_pcie *btpci;
+
+ btpci = bt1_pcie_create_data(pdev);
+ if (IS_ERR(btpci))
+ return PTR_ERR(btpci);
+
+ return bt1_pcie_add_port(btpci);
+}
+
+static int bt1_pcie_remove(struct platform_device *pdev)
+{
+ struct bt1_pcie *btpci = platform_get_drvdata(pdev);
+
+ bt1_pcie_del_port(btpci);
+
+ return 0;
+}
+
+static const struct of_device_id bt1_pcie_of_match[] = {
+ { .compatible = "baikal,bt1-pcie" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, bt1_pcie_of_match);
+
+static struct platform_driver bt1_pcie_driver = {
+ .probe = bt1_pcie_probe,
+ .remove = bt1_pcie_remove,
+ .driver = {
+ .name = "bt1-pcie",
+ .of_match_table = bt1_pcie_of_match,
+ },
+};
+module_platform_driver(bt1_pcie_driver);
+
+MODULE_AUTHOR("Serge Semin <[email protected]>");
+MODULE_DESCRIPTION("Baikal-T1 PCIe driver");
+MODULE_LICENSE("GPL");
--
2.35.1

2022-07-28 14:57:25

by Serge Semin

[permalink] [raw]
Subject: [PATCH v4 14/17] PCI: dwc: Introduce generic resources getter

Currently the DW PCIe Root Port and Endpoint CSR spaces are retrieved in
the separate parts of the DW PCIe core driver. It doesn't really make
sense since the both controller types have identical set of the core CSR
regions: DBI, DBI CS2 and iATU/eDMA. Thus we can simplify the DW PCIe Host
and EP initialization methods by moving the platform-specific registers
space getting and mapping into a common method. It gets to be even more
justified seeing the CSRs base address pointers are preserved in the
common DW PCIe descriptor. Note all the OF-based common DW PCIe settings
initialization will be moved to the new method too in order to have a
single function for all the generic platform properties handling in single
place.

A nice side-effect of this change is that the pcie-designware-host.c and
pcie-designware-ep.c drivers are cleaned up from all the direct dw_pcie
storage modification, which makes the DW PCIe core, Root Port and Endpoint
modules more coherent.

Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Rob Herring <[email protected]>

---

Changelog v3:
- This is a new patch created on v3 lap of the series.

Changelog v4:
- Convert the method name from dw_pcie_get_res() to
dw_pcie_get_resources(). (@Bjorn)
---
.../pci/controller/dwc/pcie-designware-ep.c | 26 +------
.../pci/controller/dwc/pcie-designware-host.c | 15 +---
drivers/pci/controller/dwc/pcie-designware.c | 75 ++++++++++++++-----
drivers/pci/controller/dwc/pcie-designware.h | 3 +
4 files changed, 65 insertions(+), 54 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 441feff1917a..d4db48e4d46a 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -13,8 +13,6 @@
#include <linux/pci-epc.h>
#include <linux/pci-epf.h>

-#include "../../pci.h"
-
void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
{
struct pci_epc *epc = ep->epc;
@@ -680,29 +678,14 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
struct device *dev = pci->dev;
struct platform_device *pdev = to_platform_device(dev);
- struct device_node *np = dev->of_node;
const struct pci_epc_features *epc_features;
struct dw_pcie_ep_func *ep_func;

INIT_LIST_HEAD(&ep->func_list);

- if (!pci->dbi_base) {
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
- pci->dbi_base = devm_pci_remap_cfg_resource(dev, res);
- if (IS_ERR(pci->dbi_base))
- return PTR_ERR(pci->dbi_base);
- }
-
- if (!pci->dbi_base2) {
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi2");
- if (!res) {
- pci->dbi_base2 = pci->dbi_base + SZ_4K;
- } else {
- pci->dbi_base2 = devm_pci_remap_cfg_resource(dev, res);
- if (IS_ERR(pci->dbi_base2))
- return PTR_ERR(pci->dbi_base2);
- }
- }
+ ret = dw_pcie_get_resources(pci);
+ if (ret)
+ return ret;

res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space");
if (!res)
@@ -735,9 +718,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
return -ENOMEM;
ep->outbound_addr = addr;

- if (pci->link_gen < 1)
- pci->link_gen = of_pci_get_max_link_speed(np);
-
epc = devm_pci_epc_create(dev, &epc_ops);
if (IS_ERR(epc)) {
dev_err(dev, "Failed to create epc device\n");
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 2fbe9dc11634..6d201ed1297a 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -16,7 +16,6 @@
#include <linux/pci_regs.h>
#include <linux/platform_device.h>

-#include "../../pci.h"
#include "pcie-designware.h"

static struct pci_ops dw_pcie_ops;
@@ -298,6 +297,10 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)

raw_spin_lock_init(&pp->lock);

+ ret = dw_pcie_get_resources(pci);
+ if (ret)
+ return ret;
+
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
if (res) {
pp->cfg0_size = resource_size(res);
@@ -311,13 +314,6 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
return -ENODEV;
}

- if (!pci->dbi_base) {
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
- pci->dbi_base = devm_pci_remap_cfg_resource(dev, res);
- if (IS_ERR(pci->dbi_base))
- return PTR_ERR(pci->dbi_base);
- }
-
bridge = devm_pci_alloc_host_bridge(dev, 0);
if (!bridge)
return -ENOMEM;
@@ -332,9 +328,6 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
pp->io_base = pci_pio_to_address(win->res->start);
}

- if (pci->link_gen < 1)
- pci->link_gen = of_pci_get_max_link_speed(np);
-
/* Set default bus ops */
bridge->ops = &dw_pcie_ops;
bridge->child_ops = &dw_child_pcie_ops;
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 479eafcdbcb7..7625e43a97cb 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -11,6 +11,7 @@
#include <linux/align.h>
#include <linux/bitops.h>
#include <linux/delay.h>
+#include <linux/ioport.h>
#include <linux/of.h>
#include <linux/of_platform.h>
#include <linux/sizes.h>
@@ -19,6 +20,59 @@
#include "../../pci.h"
#include "pcie-designware.h"

+int dw_pcie_get_resources(struct dw_pcie *pci)
+{
+ struct platform_device *pdev = to_platform_device(pci->dev);
+ struct device_node *np = dev_of_node(pci->dev);
+ struct resource *res;
+
+ if (!pci->dbi_base) {
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
+ pci->dbi_base = devm_pci_remap_cfg_resource(pci->dev, res);
+ if (IS_ERR(pci->dbi_base))
+ return PTR_ERR(pci->dbi_base);
+ }
+
+ /* DBI2 is mainly useful for the endpoint controller */
+ if (!pci->dbi_base2) {
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi2");
+ if (res) {
+ pci->dbi_base2 = devm_pci_remap_cfg_resource(pci->dev, res);
+ if (IS_ERR(pci->dbi_base2))
+ return PTR_ERR(pci->dbi_base2);
+ } else {
+ pci->dbi_base2 = pci->dbi_base + SZ_4K;
+ }
+ }
+
+ /* For non-unrolled iATU/eDMA platforms this range will be ignored */
+ if (!pci->atu_base) {
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "atu");
+ if (res) {
+ pci->atu_size = resource_size(res);
+ pci->atu_base = devm_ioremap_resource(pci->dev, res);
+ if (IS_ERR(pci->atu_base))
+ return PTR_ERR(pci->atu_base);
+ } else {
+ pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET;
+ }
+ }
+
+ /* Set a default value suitable for at most 8 in and 8 out windows */
+ if (!pci->atu_size)
+ pci->atu_size = SZ_4K;
+
+ if (pci->link_gen < 1)
+ pci->link_gen = of_pci_get_max_link_speed(np);
+
+ of_property_read_u32(np, "num-lanes", &pci->num_lanes);
+
+ if (of_property_read_bool(np, "snps,enable-cdm-check"))
+ dw_pcie_cap_set(pci, CDM_CHECK);
+
+ return 0;
+}
+
void dw_pcie_version_detect(struct dw_pcie *pci)
{
u32 ver;
@@ -642,25 +696,8 @@ static void dw_pcie_iatu_detect_regions(struct dw_pcie *pci)

void dw_pcie_iatu_detect(struct dw_pcie *pci)
{
- struct platform_device *pdev = to_platform_device(pci->dev);
-
if (dw_pcie_iatu_unroll_enabled(pci)) {
dw_pcie_cap_set(pci, IATU_UNROLL);
-
- if (!pci->atu_base) {
- struct resource *res =
- platform_get_resource_byname(pdev, IORESOURCE_MEM, "atu");
- if (res) {
- pci->atu_size = resource_size(res);
- pci->atu_base = devm_ioremap_resource(pci->dev, res);
- }
- if (!pci->atu_base || IS_ERR(pci->atu_base))
- pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET;
- }
-
- if (!pci->atu_size)
- /* Pick a minimal default, enough for 8 in and 8 out windows */
- pci->atu_size = SZ_4K;
} else {
pci->atu_base = pci->dbi_base + PCIE_ATU_VIEWPORT_BASE;
pci->atu_size = PCIE_ATU_VIEWPORT_SIZE;
@@ -678,7 +715,6 @@ void dw_pcie_iatu_detect(struct dw_pcie *pci)

void dw_pcie_setup(struct dw_pcie *pci)
{
- struct device_node *np = pci->dev->of_node;
u32 val;

if (pci->link_gen > 0)
@@ -706,14 +742,13 @@ void dw_pcie_setup(struct dw_pcie *pci)
val |= PORT_LINK_DLL_LINK_EN;
dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val);

- if (of_property_read_bool(np, "snps,enable-cdm-check")) {
+ if (dw_pcie_cap_is(pci, CDM_CHECK)) {
val = dw_pcie_readl_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS);
val |= PCIE_PL_CHK_REG_CHK_REG_CONTINUOUS |
PCIE_PL_CHK_REG_CHK_REG_START;
dw_pcie_writel_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS, val);
}

- of_property_read_u32(np, "num-lanes", &pci->num_lanes);
if (!pci->num_lanes) {
dev_dbg(pci->dev, "Using h/w default number of lanes\n");
return;
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index e3b839ec0ccf..4c46a9f9694d 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -46,6 +46,7 @@

/* DWC PCIe controller capabilities */
#define DW_PCIE_CAP_IATU_UNROLL 1
+#define DW_PCIE_CAP_CDM_CHECK 2

#define dw_pcie_cap_is(_pci, _cap) \
test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
@@ -300,6 +301,8 @@ struct dw_pcie {
#define to_dw_pcie_from_ep(endpoint) \
container_of((endpoint), struct dw_pcie, ep)

+int dw_pcie_get_resources(struct dw_pcie *pci);
+
void dw_pcie_version_detect(struct dw_pcie *pci);

u8 dw_pcie_find_capability(struct dw_pcie *pci, u8 cap);
--
2.35.1

2022-07-28 14:58:08

by Serge Semin

[permalink] [raw]
Subject: [PATCH v4 07/17] dt-bindings: PCI: dwc: Add interrupts/interrupt-names common properties

Currently the 'interrupts' and 'interrupt-names' are defined being too
generic to really describe any actual IRQ interface. Moreover the DW PCIe
End-point devices are left with no IRQ signals. All of that can be fixed
by adding the IRQ-related properties to the common DW PCIe DT-schema and
defining a common and device-specific set of the IRQ names in accordance
with the hardware reference manual. Seeing there are common and dedicated
IRQ signals for DW PCIe Root Port and End-point controllers we suggest to
split the IRQ names up into two sets: common definitions available in the
snps,dw-pcie-common.yaml schema and Root Port specific names defined in
the snps,dw-pcie.yaml schema. The former one will be applied to both DW
PCIe RP and EP controllers, while the later one - for the RP only.

Signed-off-by: Serge Semin <[email protected]>

---

Changelog v3:
- This is a new patch unpinned from the next one:
https://lore.kernel.org/linux-pci/[email protected]/
by the Rob' request. (@Rob)
---
.../bindings/pci/snps,dw-pcie-common.yaml | 51 +++++++++++++++
.../bindings/pci/snps,dw-pcie-ep.yaml | 17 +++++
.../devicetree/bindings/pci/snps,dw-pcie.yaml | 63 ++++++++++++++++++-
3 files changed, 128 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
index b2fbe886981b..0a524e916a9f 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
@@ -17,6 +17,25 @@ description:
select: false

properties:
+ interrupts:
+ description:
+ There are two main sub-blocks which are normally capable of
+ generating interrupts. It's System Information Interface and MSI
+ interface. While the former one has some common for the Host and
+ Endpoint controllers IRQ-signals, the later interface is obviously
+ Root Complex specific since it's responsible for the incoming MSI
+ messages signalling. The System Information IRQ signals are mainly
+ responsible for reporting the generic PCIe hierarchy and Root
+ Complex events like VPD IO request, general AER, PME, Hot-plug, link
+ bandwidth change, link equalization request, INTx asserted/deasserted
+ Message detection, embedded DMA Tx/Rx/Error.
+ minItems: 1
+ maxItems: 26
+
+ interrupt-names:
+ minItems: 1
+ maxItems: 26
+
phys:
description:
There can be up to the number of possible lanes PHYs specified.
@@ -91,4 +110,36 @@ properties:

additionalProperties: true

+definitions:
+ interrupt-names:
+ description:
+ IRQ signal names common for the DWC PCIe Root Port and Endpoint
+ controllers.
+ oneOf:
+ - description:
+ Controller request to read or write virtual product data
+ from/to the VPD capability registers.
+ const: vpd
+ - description:
+ Link Equalization Request flag is set in the Link Status 2
+ register (applicable if the corresponding IRQ is enabled in
+ the Link Control 3 register).
+ const: l_eq
+ - description:
+ Indicates that the eDMA Tx/Rx transfer is complete or that an
+ error has occurred on the corresponding channel. eDMA can have
+ eight Tx (Write) and Rx (Read) eDMA channels thus supporting up
+ to 16 IRQ signals all together. Write eDMA channels shall go
+ first in the ordered row as per default edma_int[*] bus setup.
+ pattern: '^dma([0-9]|1[0-5])?$'
+ - description:
+ PCIe protocol correctable error or a Data Path protection
+ correctable error is detected by the automotive/safety
+ feature.
+ const: sft_ce
+ - description:
+ Indicates that the internal safety mechanism detected and
+ uncorrectable error.
+ const: sft_ue
+
...
diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
index 9411366d6ca7..5f12a6ac08d8 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
@@ -53,6 +53,20 @@ properties:
items:
enum: [dbi, dbi2, config, atu, addr_space, link, atu_dma, appl]

+ interrupts:
+ description:
+ There is no mandatory IRQ signals for the normal controller functioning,
+ but in addition to the native set the platforms may have a link- or
+ PM-related IRQs specified.
+ minItems: 1
+ maxItems: 20
+
+ interrupt-names:
+ minItems: 1
+ maxItems: 20
+ items:
+ $ref: /schemas/pci/snps,dw-pcie-common.yaml#/definitions/interrupt-names
+
max-functions:
maximum: 32

@@ -72,6 +86,9 @@ examples:
<0xd0000000 0x2000000>; /* Configuration space */
reg-names = "dbi", "dbi2", "addr_space";

+ interrupts = <23>, <24>;
+ interrupt-names = "dma0", "dma1";
+
phys = <&pcie_phy0>, <&pcie_phy1>, <&pcie_phy2>, <&pcie_phy3>;
phy-names = "pcie0", "pcie1", "pcie2", "pcie3";

diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
index 8b2e3210e3e2..e0020b72288a 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
@@ -54,9 +54,23 @@ properties:
enum: [ dbi, dbi2, config, atu, app, elbi, mgmt, ctrl, parf, cfg, link,
ulreg, smu, mpu, apb, phy ]

- interrupts: true
-
- interrupt-names: true
+ interrupts:
+ description:
+ At least MSI interrupt signal is supposed to be specified for
+ the DWC PCIe host controller.
+ minItems: 1
+ maxItems: 26
+
+ interrupt-names:
+ minItems: 1
+ maxItems: 26
+ items:
+ anyOf:
+ - $ref: /schemas/pci/snps,dw-pcie-common.yaml#/definitions/interrupt-names
+ - $ref: '#/definitions/interrupt-names'
+ allOf:
+ - contains:
+ const: msi

clocks: true

@@ -67,6 +81,48 @@ required:
- reg
- reg-names

+definitions:
+ interrupt-names:
+ description:
+ DWC PCIe Root Port/Complex specific IRQ signal names.
+ oneOf:
+ - description:
+ DSP AXI MSI Interrupt detected. It gets de-asserted when there is
+ no more MSI interrupt pending. The interrupt is relevant to the
+ iMSI-RX - Integrated MSI Receiver (AXI bridge).
+ const: msi
+ - description:
+ Legacy A/B/C/D interrupt signal. Basically it's triggered by
+ receiving a Assert_INT{A,B,C,D}/Desassert_INT{A,B,C,D} message
+ from the downstream device.
+ pattern: "^int(a|b|c|d)$"
+ - description:
+ Error condition detected and a bit is set in the Root Error Status
+ register of the AER capability. It's asserted when the RC
+ internally generated an error or an error message is received by
+ the RC.
+ const: aer
+ - description:
+ PME message is received by the port. That means having the PME
+ status bit set in the Root Status register (the event is
+ supposed to be unmasked in the Root Control register).
+ const: pme
+ - description:
+ Hot-plug event is detected. That is a bit has been set in the
+ Slot Status register and the corresponding event is enabled in
+ the Slot Control register.
+ const: hp
+ - description:
+ Link Autonomous Bandwidth Status flag has been set in the Link
+ Status register (the event is supposed to be unmasked in the
+ Link Control register).
+ const: bw_au
+ - description:
+ Bandwidth Management Status flag has been set in the Link
+ Status register (the event is supposed to be unmasked in the
+ Link Control register).
+ const: bw_mg
+
examples:
- |
pcie@dfc00000 {
@@ -82,6 +138,7 @@ examples:
bus-range = <0x0 0xff>;

interrupts = <25>, <24>;
+ interrupt-names = "msi", "hp";
#interrupt-cells = <1>;

reset-gpios = <&port0 0 1>;
--
2.35.1

2022-07-28 14:58:26

by Serge Semin

[permalink] [raw]
Subject: [PATCH v4 16/17] PCI: dwc: Introduce generic platform clocks and resets

Currently almost each platform driver uses its own resets and clocks
naming in order to get the corresponding descriptors. It makes the code
harder to maintain and comprehend especially seeing the DWC PCIe core main
resets and clocks signals set hasn't changed much for about at least one
major IP-core release. So in order to organize things around these signals
we suggest to create a generic interface for them in accordance with the
naming introduced in the DWC PCIe IP-core reference manual:

Application clocks:
- "dbi" - data bus interface clock (on some DWC PCIe platforms it's
referred as "pclk", "pcie", "sys", "ahb", "cfg", "iface",
"gio", "reg", "pcie_apb_sys");
- "mstr" - AXI-bus master interface clock (some DWC PCIe glue drivers
refer to this clock as "port", "bus", "pcie_bus",
"bus_master/master_bus/axi_m", "pcie_aclk");
- "slv" - AXI-bus slave interface clock (also called as "port", "bus",
"pcie_bus", "bus_slave/slave_bus/axi_s", "pcie_aclk",
"pcie_inbound_axi").

Core clocks:
- "pipe" - core-PCS PIPE interface clock coming from external PHY (it's
normally named by the platform drivers as just "pipe");
- "core" - primary clock of the controller (none of the platform drivers
declare such a clock but in accordance with the ref. manual
the devices may have it separately specified);
- "aux" - auxiliary PMC domain clock (it is named by some platforms as
"pcie_aux" and just "aux");
- "ref" - Generic reference clock (it is a generic clock source, which
can be used as a signal source for multiple interfaces, some
platforms call it as "ref", "general", "pcie_phy",
"pcie_phy_ref").

Application resets:
- "dbi" - Data-bus interface reset (it's CSR interface clock and is
normally called as "apb" though technically it's not APB but
DWC PCIe-specific interface);
- "mstr" - AXI-bus master reset (some platforms call it as "port", "apps",
"bus", "axi_m");
- "slv" - ABI-bus slave reset (some platforms call it as "port", "apps",
"bus", "axi_s").

Core resets:
- "non-sticky" - non-sticky CSR flags reset;
- "sticky" - sticky CSR flags reset;
- "pipe" - PIPE-interface (Core-PCS) logic reset (some platforms
call it just "pipe");
- "core" - controller primary reset (resets everything except PMC
module, some platforms refer to this signal as "soft",
"pci");
- "phy" - PCS/PHY block reset (strictly speaking it is normally
connected to the input of an external block, but the
reference manual says it must be available for the PMC
working correctly, some existing platforms call it
"pciephy", "phy", "link");
- "hot" - PMC hot reset signal (also called as "sleep");
- "pwr" - cold reset signal (can be referred as "pwr", "turnoff").

Bus reset:
- "perst" - PCIe standard signal used to reset the PCIe peripheral
devices.

As you can see each platform uses it's own naming for basically the same
set of the signals. In the framework of this commit we suggest to add a
set of the clocks and reset signals resources, corresponding names and
identifiers for each denoted entity. At current stage the platforms will
be able to use the provided infrastructure to automatically request all
these resources and manipulate with them in the Host/EP init callbacks.
Alas it isn't that easy to create a common cold/hot reset procedure due to
too many platform-specifics in the procedure, like the external flags
exposure and the delays requirement.

Signed-off-by: Serge Semin <[email protected]>

---

Changelog v3:
- Add a method to at least request the generic clocks and resets. (@Rob)
- Add GPIO-based PERST# signal support.
---
drivers/pci/controller/dwc/pcie-designware.c | 91 ++++++++++++++++++++
drivers/pci/controller/dwc/pcie-designware.h | 42 +++++++++
2 files changed, 133 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index d1e57ed17f5a..4b4dc1136687 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -10,7 +10,9 @@

#include <linux/align.h>
#include <linux/bitops.h>
+#include <linux/clk.h>
#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
#include <linux/ioport.h>
#include <linux/of.h>
#include <linux/of_platform.h>
@@ -20,11 +22,89 @@
#include "../../pci.h"
#include "pcie-designware.h"

+static const char * const dw_pcie_app_clks[DW_PCIE_NUM_APP_CLKS] = {
+ [DW_PCIE_DBI_CLK] = "dbi",
+ [DW_PCIE_MSTR_CLK] = "mstr",
+ [DW_PCIE_SLV_CLK] = "slv",
+};
+
+static const char * const dw_pcie_core_clks[DW_PCIE_NUM_CORE_CLKS] = {
+ [DW_PCIE_PIPE_CLK] = "pipe",
+ [DW_PCIE_CORE_CLK] = "core",
+ [DW_PCIE_AUX_CLK] = "aux",
+ [DW_PCIE_REF_CLK] = "ref",
+};
+
+static const char * const dw_pcie_app_rsts[DW_PCIE_NUM_APP_RSTS] = {
+ [DW_PCIE_DBI_RST] = "dbi",
+ [DW_PCIE_MSTR_RST] = "mstr",
+ [DW_PCIE_SLV_RST] = "slv",
+};
+
+static const char * const dw_pcie_core_rsts[DW_PCIE_NUM_CORE_RSTS] = {
+ [DW_PCIE_NON_STICKY_RST] = "non-sticky",
+ [DW_PCIE_STICKY_RST] = "sticky",
+ [DW_PCIE_CORE_RST] = "core",
+ [DW_PCIE_PIPE_RST] = "pipe",
+ [DW_PCIE_PHY_RST] = "phy",
+ [DW_PCIE_HOT_RST] = "hot",
+ [DW_PCIE_PWR_RST] = "pwr",
+};
+
+static int dw_pcie_get_clocks(struct dw_pcie *pci)
+{
+ int i, ret;
+
+ for (i = 0; i < DW_PCIE_NUM_APP_CLKS; i++)
+ pci->app_clks[i].id = dw_pcie_app_clks[i];
+
+ for (i = 0; i < DW_PCIE_NUM_CORE_CLKS; i++)
+ pci->core_clks[i].id = dw_pcie_core_clks[i];
+
+ ret = devm_clk_bulk_get_optional(pci->dev, DW_PCIE_NUM_APP_CLKS,
+ pci->app_clks);
+ if (ret)
+ return ret;
+
+ return devm_clk_bulk_get_optional(pci->dev, DW_PCIE_NUM_CORE_CLKS,
+ pci->core_clks);
+}
+
+static int dw_pcie_get_resets(struct dw_pcie *pci)
+{
+ int i, ret;
+
+ for (i = 0; i < DW_PCIE_NUM_APP_RSTS; i++)
+ pci->app_rsts[i].id = dw_pcie_app_rsts[i];
+
+ for (i = 0; i < DW_PCIE_NUM_CORE_RSTS; i++)
+ pci->core_rsts[i].id = dw_pcie_core_rsts[i];
+
+ ret = devm_reset_control_bulk_get_optional_shared(pci->dev,
+ DW_PCIE_NUM_APP_RSTS,
+ pci->app_rsts);
+ if (ret)
+ return ret;
+
+ ret = devm_reset_control_bulk_get_optional_exclusive(pci->dev,
+ DW_PCIE_NUM_CORE_RSTS,
+ pci->core_rsts);
+ if (ret)
+ return ret;
+
+ pci->pe_rst = devm_gpiod_get_optional(pci->dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(pci->pe_rst))
+ return PTR_ERR(pci->pe_rst);
+
+ return 0;
+}
+
int dw_pcie_get_resources(struct dw_pcie *pci)
{
struct platform_device *pdev = to_platform_device(pci->dev);
struct device_node *np = dev_of_node(pci->dev);
struct resource *res;
+ int ret;

if (!pci->dbi_base) {
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
@@ -62,6 +142,17 @@ int dw_pcie_get_resources(struct dw_pcie *pci)
if (!pci->atu_size)
pci->atu_size = SZ_4K;

+ /* LLDD is supposed to manually switch the clocks and resets state */
+ if (dw_pcie_cap_is(pci, REQ_RES)) {
+ ret = dw_pcie_get_clocks(pci);
+ if (ret)
+ return ret;
+
+ ret = dw_pcie_get_resets(pci);
+ if (ret)
+ return ret;
+ }
+
if (pci->link_gen < 1)
pci->link_gen = of_pci_get_max_link_speed(np);

diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 4c46a9f9694d..bfe5581edd2e 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -13,10 +13,13 @@

#include <linux/bitfield.h>
#include <linux/bitops.h>
+#include <linux/clk.h>
#include <linux/dma-mapping.h>
+#include <linux/gpio/consumer.h>
#include <linux/irq.h>
#include <linux/msi.h>
#include <linux/pci.h>
+#include <linux/reset.h>

#include <linux/pci-epc.h>
#include <linux/pci-epf.h>
@@ -45,6 +48,7 @@
__dw_pcie_ver_cmp(_pci, TYPE_ ## _type, >=))

/* DWC PCIe controller capabilities */
+#define DW_PCIE_CAP_REQ_RES 0
#define DW_PCIE_CAP_IATU_UNROLL 1
#define DW_PCIE_CAP_CDM_CHECK 2

@@ -194,6 +198,39 @@ enum dw_pcie_device_mode {
DW_PCIE_RC_TYPE,
};

+enum dw_pcie_app_clk {
+ DW_PCIE_DBI_CLK,
+ DW_PCIE_MSTR_CLK,
+ DW_PCIE_SLV_CLK,
+ DW_PCIE_NUM_APP_CLKS
+};
+
+enum dw_pcie_core_clk {
+ DW_PCIE_PIPE_CLK,
+ DW_PCIE_CORE_CLK,
+ DW_PCIE_AUX_CLK,
+ DW_PCIE_REF_CLK,
+ DW_PCIE_NUM_CORE_CLKS
+};
+
+enum dw_pcie_app_rst {
+ DW_PCIE_DBI_RST,
+ DW_PCIE_MSTR_RST,
+ DW_PCIE_SLV_RST,
+ DW_PCIE_NUM_APP_RSTS
+};
+
+enum dw_pcie_core_rst {
+ DW_PCIE_NON_STICKY_RST,
+ DW_PCIE_STICKY_RST,
+ DW_PCIE_CORE_RST,
+ DW_PCIE_PIPE_RST,
+ DW_PCIE_PHY_RST,
+ DW_PCIE_HOT_RST,
+ DW_PCIE_PWR_RST,
+ DW_PCIE_NUM_CORE_RSTS
+};
+
struct dw_pcie_host_ops {
int (*host_init)(struct dw_pcie_rp *pp);
void (*host_deinit)(struct dw_pcie_rp *pp);
@@ -294,6 +331,11 @@ struct dw_pcie {
int num_lanes;
int link_gen;
u8 n_fts[2];
+ struct clk_bulk_data app_clks[DW_PCIE_NUM_APP_CLKS];
+ struct clk_bulk_data core_clks[DW_PCIE_NUM_CORE_CLKS];
+ struct reset_control_bulk_data app_rsts[DW_PCIE_NUM_APP_RSTS];
+ struct reset_control_bulk_data core_rsts[DW_PCIE_NUM_CORE_RSTS];
+ struct gpio_desc *pe_rst;
};

#define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp)
--
2.35.1

2022-07-28 15:10:12

by Serge Semin

[permalink] [raw]
Subject: [PATCH v4 08/17] dt-bindings: PCI: dwc: Add reg/reg-names common properties

Even though there is a more-or-less limited set of the CSR spaces can be
defined for each DW PCIe controller the generic DT schema currently
doesn't specify much limitations on the reg-space names used for one or
another range. In order to prevent the vendor-specific controller schemas
further deviation from the generic interface let's fix that by introducing
the reg-names definition in the common DW PCIe DT-schema and preserving a
generic "reg" and "reg-names" properties in there. New DW PCIe device
DT-bindings are encouraged to use the generic set of the CSR spaces
defined in the generic DW PCie DT-bindings, while the already available
vendor-specific DT-bindings can refer to the common schema and define
their own reg-spaces.

Note the number of reg/reg-names items need to be changed in the DW PCIe
EP DT-schema since aside with the "dbi" CSRs space these arrays can
have "dbi2", "addr_space", "atu", etc ranges.

Signed-off-by: Serge Semin <[email protected]>

---

Changelog v3:
- This is a new patch unpinned from the next one:
https://lore.kernel.org/linux-pci/[email protected]/
by the Rob' request. (@Rob)
- Split up reg-names in the same way as the interrupt-names: common,
Root Port and Endpoint specific names. (@Rob)
- Drop synonymous from the names list since the device schemas create
their own enumerations anyway.
---
.../bindings/pci/snps,dw-pcie-common.yaml | 76 +++++++++++++++++++
.../bindings/pci/snps,dw-pcie-ep.yaml | 34 +++++++--
.../devicetree/bindings/pci/snps,dw-pcie.yaml | 27 +++++--
3 files changed, 124 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
index 0a524e916a9f..c6a55f90ddd5 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
@@ -17,6 +17,28 @@ description:
select: false

properties:
+ reg:
+ description:
+ DWC PCIe CSR space is normally accessed over the dedicated Data Bus
+ Interface - DBI. In accordance with the reference manual the register
+ configuration space belongs to the Configuration-Dependent Module (CDM)
+ and is split up into several sub-parts Standard PCIe configuration
+ space, Port Logic Registers (PL), Shadow Config-space Registers,
+ iATU/eDMA registers. The particular sub-space is selected by the
+ CDM/ELBI (dbi_cs) and CS2 (dbi_cs2) signals (selector bits). Such
+ configuration provides a flexible interface for the system engineers to
+ either map the particular space at a desired MMIO address or just leave
+ them in a contiguous memory space if pure Native or AXI Bridge DBI access
+ is selected. Note the PCIe CFG-space, PL and Shadow registers are
+ specific for each activated function, while the rest of the sub-spaces
+ are common for all of them (if there are more than one).
+ minItems: 2
+ maxItems: 6
+
+ reg-names:
+ minItems: 2
+ maxItems: 6
+
interrupts:
description:
There are two main sub-blocks which are normally capable of
@@ -111,6 +133,60 @@ properties:
additionalProperties: true

definitions:
+ reg-names:
+ description:
+ CSR space names common for the DWC PCIe Root Port and Endpoint
+ controllers.
+ oneOf:
+ - description:
+ Basic DWC PCIe controller configuration-space accessible over
+ the DBI interface. This memory space is either activated with
+ CDM/ELBI = 0 and CS2 = 0 or is a contiguous memory region
+ with all spaces. Note iATU/eDMA CSRs are indirectly accessible
+ via the PL viewports on the DWC PCIe controllers older than
+ v4.80a.
+ const: dbi
+ - description:
+ Shadow DWC PCIe config-space registers. This space is selected
+ by setting CDM/ELBI = 0 and CS2 = 1. This is an intermix of
+ the PCI-SIG PCIe CFG-space with the shadow registers for some
+ PCI Header space, PCI Standard and Extended Structures. It's
+ mainly relevant for the end-point controller configuration,
+ but still there are some shadow registers available for the
+ Root Port mode too.
+ const: dbi2
+ - description:
+ External Local Bus registers. It's an application-dependent
+ registers normally defined by the platform engineers. The space
+ can be selected by setting CDM/ELBI = 1 and CS2 = 0 wires or can
+ be accessed over some platform-specific means (for instance
+ as a part of a system controller).
+ enum: [ elbi, app ]
+ - description:
+ iATU/eDMA registers common for all device functions. It's an
+ unrolled memory space with the internal Address Translation
+ Unit and Enhanced DMA, which is selected by setting CDM/ELBI = 1
+ and CS2 = 1. For IP-core releases prior v4.80a, these registers
+ have been programmed via an indirect addressing scheme using a
+ set of viewport CSRs mapped into the PL space. Note iATU is
+ normally mapped to the 0x0 address of this region, while eDMA
+ is available at 0x80000 base address.
+ const: atu
+ - description:
+ Platform-specific eDMA registers. Some platforms may have eDMA
+ CSRs mapped in a non-standard base address. The registers offset
+ can be changed or the MS/LS-bits of the address can be attached
+ in an additional RTL block before the MEM-IO transactions reach
+ the DW PCIe slave interface.
+ const: dma
+ - description:
+ PHY/PCS configuration registers. Some platforms can have the
+ PCS and PHY CSRs accessible over a dedicated memory mapped
+ region, but mainly these registers are indirectly accessible
+ either by means of the embedded PHY viewport schema or by some
+ platform-specific method.
+ const: phy
+
interrupt-names:
description:
IRQ signal names common for the DWC PCIe Root Port and Endpoint
diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
index 5f12a6ac08d8..bfff723b529d 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
@@ -40,18 +40,25 @@ properties:
const: snps,dw-pcie-ep

reg:
- description: |
- It should contain Data Bus Interface (dbi) and config registers for all
- versions.
- For designware core version >= 4.80, it may contain ATU address space.
+ description:
+ DBI, DBI2 reg-spaces and outbound memory window are required for the
+ normal controller functioning. iATU memory IO region is also required
+ if the space is unrolled (IP-core version >= 4.80a).
minItems: 2
- maxItems: 4
+ maxItems: 5

reg-names:
minItems: 2
- maxItems: 4
+ maxItems: 5
items:
- enum: [dbi, dbi2, config, atu, addr_space, link, atu_dma, appl]
+ anyOf:
+ - $ref: /schemas/pci/snps,dw-pcie-common.yaml#/definitions/reg-names
+ - $ref: '#/definitions/reg-names'
+ allOf:
+ - contains:
+ const: dbi
+ - contains:
+ const: addr_space

interrupts:
description:
@@ -77,6 +84,19 @@ required:

additionalProperties: true

+definitions:
+ reg-names:
+ description:
+ DWC PCIe Endpoint specific CSR space names.
+ oneOf:
+ - description:
+ Outbound iATU-capable memory-region which will be used to
+ generate various application-specific traffic on the PCIe bus
+ hierarchy. It's usage scenario depends on the endpoint
+ functionality, for instance it can be used to create MSI(X)
+ messages.
+ const: addr_space
+
examples:
- |
pcie-ep@dfd00000 {
diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
index e0020b72288a..9e96c6d0ef48 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
@@ -40,10 +40,10 @@ properties:
const: snps,dw-pcie

reg:
- description: |
- It should contain Data Bus Interface (dbi) and config registers for all
- versions.
- For designware core version >= 4.80, it may contain ATU address space.
+ description:
+ At least DBI reg-space and peripheral devices CFG-space outbound window
+ are required for the normal controller work. iATU memory IO region is
+ also required if the space is unrolled (IP-core version >= 4.80a).
minItems: 2
maxItems: 5

@@ -51,8 +51,14 @@ properties:
minItems: 2
maxItems: 5
items:
- enum: [ dbi, dbi2, config, atu, app, elbi, mgmt, ctrl, parf, cfg, link,
- ulreg, smu, mpu, apb, phy ]
+ anyOf:
+ - $ref: /schemas/pci/snps,dw-pcie-common.yaml#/definitions/reg-names
+ - $ref: '#/definitions/reg-names'
+ allOf:
+ - contains:
+ const: dbi
+ - contains:
+ const: config

interrupts:
description:
@@ -82,6 +88,15 @@ required:
- reg-names

definitions:
+ reg-names:
+ description:
+ DWC PCIe Root Port/Complex specific CSR space names.
+ oneOf:
+ - description:
+ Outbound iATU-capable memory-region which will be used to access
+ the peripheral PCIe devices configuration space.
+ const: config
+
interrupt-names:
description:
DWC PCIe Root Port/Complex specific IRQ signal names.
--
2.35.1

2022-07-28 15:10:40

by Serge Semin

[permalink] [raw]
Subject: [PATCH v4 12/17] dt-bindings: PCI: dwc: Add Baikal-T1 PCIe Root Port bindings

Baikal-T1 SoC is equipped with DWC PCIe v4.60a Root Port controller, which
link can be trained to work on up to Gen.3 speed over up to x4 lanes. The
controller is supposed to be fed up with four clock sources: DBI
peripheral clock, AXI application Tx/Rx clocks and external PHY/core
reference clock generating the 100MHz signal. In addition to that the
platform provide a way to reset each part of the controller:
sticky/non-sticky bits, host controller core, PIPE interface, PCS/PHY and
Hot/Power reset signal. The Root Port controller is equipped with multiple
IRQ lines like MSI, system AER, PME, HP, Bandwidth change, Link
equalization request and eDMA ones. The registers space is accessed over
the DBI interface. There can be no more than four inbound or outbound iATU
windows configured.

Signed-off-by: Serge Semin <[email protected]>

---

Changelog v2:
- Rename 'syscon' property to 'baikal,bt1-syscon'.
- Fix the 'compatible' property definition to being more specific about
what strings are supposed to be used. Due to that we had to add the
select property to evaluate the schema against the Baikal-T1 PCIe DT
nodes only.
---
.../bindings/pci/baikal,bt1-pcie.yaml | 154 ++++++++++++++++++
1 file changed, 154 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml

diff --git a/Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml b/Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml
new file mode 100644
index 000000000000..23bd1d0aa5c5
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml
@@ -0,0 +1,154 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/baikal,bt1-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Baikal-T1 PCIe Root Port Controller
+
+maintainers:
+ - Serge Semin <[email protected]>
+
+description:
+ Embedded into Baikal-T1 SoC Root Complex controller. It's based on the
+ DWC RC PCIe v4.60a IP-core, which is configured to have just a single Root
+ Port function and is capable of establishing the link up to Gen.3 speed
+ on x4 lanes. It doesn't have embedded clock and reset control module, so
+ the proper interface initialization is supposed to be performed by software.
+
+select:
+ properties:
+ compatible:
+ contains:
+ const: baikal,bt1-pcie
+
+ required:
+ - compatible
+
+allOf:
+ - $ref: /schemas/pci/snps,dw-pcie.yaml#
+
+properties:
+ compatible:
+ items:
+ - const: baikal,bt1-pcie
+ - const: snps,dw-pcie-4.60a
+ - const: snps,dw-pcie
+
+ reg:
+ description:
+ DBI, DBI2 and at least 4KB outbound iATU-capable region.
+ maxItems: 3
+
+ reg-names:
+ minItems: 3
+ maxItems: 3
+ items:
+ enum: [ dbi, dbi2, config ]
+
+ interrupts:
+ description:
+ MSI, AER, PME, Hot-plug, Link Bandwidth Management, Link Equalization
+ request and eight Read/Write eDMA IRQ lines are available.
+ maxItems: 14
+
+ interrupt-names:
+ minItems: 14
+ maxItems: 14
+ items:
+ oneOf:
+ - pattern: '^dma[0-7]$'
+ - enum: [ msi, aer, pme, hp, bw_mg, l_eq ]
+
+ clocks:
+ description:
+ DBI (attached to the APB bus), AXI-bus master and slave interfaces
+ are fed up by the dedicated application clocks. A common reference
+ clock signal is supposed to be attached to the corresponding Ref-pad
+ of the SoC. It will be redistributed amongst the controller core
+ sub-modules (pipe, core, aux, etc).
+ minItems: 4
+ maxItems: 4
+
+ clock-names:
+ minItems: 4
+ maxItems: 4
+ items:
+ enum: [ dbi, mstr, slv, ref ]
+
+ resets:
+ description:
+ A comprehensive controller reset logic is supposed to be implemented
+ by software, so almost all the possible application and core reset
+ signals are exposed via the system CCU module.
+ minItems: 9
+ maxItems: 9
+
+ reset-names:
+ minItems: 9
+ maxItems: 9
+ items:
+ enum: [ mstr, slv, pwr, hot, phy, core, pipe, sticky, non-sticky ]
+
+ baikal,bt1-syscon:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ Phandle to the Baikal-T1 System Controller DT node. It's required to
+ access some additional PM, Reset-related and LTSSM signals.
+
+ num-lanes:
+ maximum: 4
+
+ max-link-speed:
+ maximum: 3
+
+ num-ob-windows:
+ const: 4
+
+ num-ib-windows:
+ const: 4
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - interrupts
+ - interrupt-names
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ pcie@1f052000 {
+ compatible = "baikal,bt1-pcie", "snps,dw-pcie-4.60a", "snps,dw-pcie";
+ device_type = "pci";
+ reg = <0x1f052000 0x1000>, <0x1f053000 0x1000>, <0x1bdbf000 0x1000>;
+ reg-names = "dbi", "dbi2", "config";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges = <0x81000000 0 0x00000000 0x1bdb0000 0 0x00008000>,
+ <0x82000000 0 0x20000000 0x08000000 0 0x13db0000>;
+ bus-range = <0x0 0xff>;
+
+ interrupts = <0 80 4>, <0 81 4>, <0 82 4>, <0 83 4>,
+ <0 84 4>, <0 85 4>, <0 86 4>, <0 87 4>,
+ <0 88 4>, <0 89 4>, <0 90 4>, <0 91 4>,
+ <0 92 4>, <0 93 4>;
+ interrupt-names = "dma0", "dma1", "dma2", "dma3", "dma4", "dma5", "dma6",
+ "dma7", "msi", "aer", "pme", "hp", "bw_mg", "l_eq";
+
+ clocks = <&ccu_sys 1>, <&ccu_axi 6>, <&ccu_axi 7>, <&clk_pcie>;
+ clock-names = "dbi", "mstr", "slv", "ref";
+
+ resets = <&ccu_axi 6>, <&ccu_axi 7>, <&ccu_sys 7>, <&ccu_sys 10>,
+ <&ccu_sys 4>, <&ccu_sys 6>, <&ccu_sys 5>, <&ccu_sys 8>,
+ <&ccu_sys 9>;
+ reset-names = "mstr", "slv", "pwr", "hot", "phy", "core", "pipe",
+ "sticky", "non-sticky";
+
+ reset-gpios = <&port0 0 1>;
+
+ num-lanes = <4>;
+ max-link-speed = <3>;
+ };
+...
--
2.35.1

2022-07-28 15:11:13

by Serge Semin

[permalink] [raw]
Subject: [PATCH v4 13/17] PCI: dwc: Introduce generic controller capabilities interface

Since in addition to the already available iATU unrolled mapping we are
about to add a few more DW PCIe platform-specific capabilities (CDM-check
and generic clocks/resets resources) let's add a generic interface to set
and get the flags indicating their availability. The new interface shall
improve maintainability of the platform-specific code.

Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Rob Herring <[email protected]>

---

Note the DW_PCIE_CAP_IATU_UNROLL macro is intentionally set to 1 since
being added afterwards capability will be more suitable to be identified
with position 0.

Changelog v3:
- This is a new patch created on v3 lap of the series.
---
drivers/pci/controller/dwc/pcie-designware.c | 11 ++++++-----
drivers/pci/controller/dwc/pcie-designware.h | 12 +++++++++++-
2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 7a5be3c4f8e0..479eafcdbcb7 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -213,7 +213,7 @@ static inline void __iomem *dw_pcie_select_atu(struct dw_pcie *pci, u32 dir,
{
void __iomem *base = pci->atu_base;

- if (pci->iatu_unroll_enabled)
+ if (dw_pcie_cap_is(pci, IATU_UNROLL))
base += PCIE_ATU_UNROLL_BASE(dir, index);
else
dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT, dir | index);
@@ -594,7 +594,7 @@ static void dw_pcie_iatu_detect_regions(struct dw_pcie *pci)
u32 val, min, dir;
u64 max;

- if (pci->iatu_unroll_enabled) {
+ if (dw_pcie_cap_is(pci, IATU_UNROLL)) {
max_region = min((int)pci->atu_size / 512, 256);
} else {
dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT, 0xFF);
@@ -644,8 +644,9 @@ void dw_pcie_iatu_detect(struct dw_pcie *pci)
{
struct platform_device *pdev = to_platform_device(pci->dev);

- pci->iatu_unroll_enabled = dw_pcie_iatu_unroll_enabled(pci);
- if (pci->iatu_unroll_enabled) {
+ if (dw_pcie_iatu_unroll_enabled(pci)) {
+ dw_pcie_cap_set(pci, IATU_UNROLL);
+
if (!pci->atu_base) {
struct resource *res =
platform_get_resource_byname(pdev, IORESOURCE_MEM, "atu");
@@ -667,7 +668,7 @@ void dw_pcie_iatu_detect(struct dw_pcie *pci)

dw_pcie_iatu_detect_regions(pci);

- dev_info(pci->dev, "iATU unroll: %s\n", pci->iatu_unroll_enabled ?
+ dev_info(pci->dev, "iATU unroll: %s\n", dw_pcie_cap_is(pci, IATU_UNROLL) ?
"enabled" : "disabled");

dev_info(pci->dev, "iATU regions: %u ob, %u ib, align %uK, limit %lluG\n",
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 5954e8cf9eec..e3b839ec0ccf 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -12,6 +12,7 @@
#define _PCIE_DESIGNWARE_H

#include <linux/bitfield.h>
+#include <linux/bitops.h>
#include <linux/dma-mapping.h>
#include <linux/irq.h>
#include <linux/msi.h>
@@ -43,6 +44,15 @@
(__dw_pcie_ver_cmp(_pci, _ver, ==) && \
__dw_pcie_ver_cmp(_pci, TYPE_ ## _type, >=))

+/* DWC PCIe controller capabilities */
+#define DW_PCIE_CAP_IATU_UNROLL 1
+
+#define dw_pcie_cap_is(_pci, _cap) \
+ test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
+
+#define dw_pcie_cap_set(_pci, _cap) \
+ set_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
+
/* Parameters for the waiting for link up routine */
#define LINK_WAIT_MAX_RETRIES 10
#define LINK_WAIT_USLEEP_MIN 90000
@@ -279,10 +289,10 @@ struct dw_pcie {
const struct dw_pcie_ops *ops;
u32 version;
u32 type;
+ unsigned long caps;
int num_lanes;
int link_gen;
u8 n_fts[2];
- bool iatu_unroll_enabled: 1;
};

#define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp)
--
2.35.1

2022-07-28 15:11:48

by Serge Semin

[permalink] [raw]
Subject: [PATCH v4 02/17] dt-bindings: PCI: dwc: Remove bus node from the examples

It's absolutely redundant seeing by default each node is embedded into its
own example-X node with address and size cells set to 1.

Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Rob Herring <[email protected]>

---

Changelog v3:
- This is a new patch unpinned from the next one:
https://lore.kernel.org/linux-pci/[email protected]/
by the Rob' request. (@Rob)
---
.../bindings/pci/snps,dw-pcie-ep.yaml | 16 ++++-----
.../devicetree/bindings/pci/snps,dw-pcie.yaml | 35 ++++++++++---------
2 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
index eae60901d60e..7d05dcba419b 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
@@ -46,14 +46,10 @@ additionalProperties: true

examples:
- |
- bus {
- #address-cells = <1>;
- #size-cells = <1>;
- pcie-ep@dfd00000 {
- compatible = "snps,dw-pcie-ep";
- reg = <0xdfc00000 0x0001000>, /* IP registers 1 */
- <0xdfc01000 0x0001000>, /* IP registers 2 */
- <0xd0000000 0x2000000>; /* Configuration space */
- reg-names = "dbi", "dbi2", "addr_space";
- };
+ pcie-ep@dfd00000 {
+ compatible = "snps,dw-pcie-ep";
+ reg = <0xdfc00000 0x0001000>, /* IP registers 1 */
+ <0xdfc01000 0x0001000>, /* IP registers 2 */
+ <0xd0000000 0x2000000>; /* Configuration space */
+ reg-names = "dbi", "dbi2", "addr_space";
};
diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
index 75ff715a0153..2810e9b5cc8d 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
@@ -53,21 +53,22 @@ required:

examples:
- |
- bus {
- #address-cells = <1>;
- #size-cells = <1>;
- pcie@dfc00000 {
- device_type = "pci";
- compatible = "snps,dw-pcie";
- reg = <0xdfc00000 0x0001000>, /* IP registers */
- <0xd0000000 0x0002000>; /* Configuration space */
- reg-names = "dbi", "config";
- #address-cells = <3>;
- #size-cells = <2>;
- ranges = <0x81000000 0 0x00000000 0xde000000 0 0x00010000>,
- <0x82000000 0 0xd0400000 0xd0400000 0 0x0d000000>;
- interrupts = <25>, <24>;
- #interrupt-cells = <1>;
- num-lanes = <1>;
- };
+ pcie@dfc00000 {
+ compatible = "snps,dw-pcie";
+ device_type = "pci";
+ reg = <0xdfc00000 0x0001000>, /* IP registers */
+ <0xd0000000 0x0002000>; /* Configuration space */
+ reg-names = "dbi", "config";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges = <0x81000000 0 0x00000000 0xde000000 0 0x00010000>,
+ <0x82000000 0 0xd0400000 0xd0400000 0 0x0d000000>;
+ bus-range = <0x0 0xff>;
+
+ interrupts = <25>, <24>;
+ #interrupt-cells = <1>;
+
+ reset-gpios = <&port0 0 1>;
+
+ num-lanes = <1>;
};
--
2.35.1

2022-07-28 15:11:57

by Serge Semin

[permalink] [raw]
Subject: [PATCH v4 05/17] dt-bindings: PCI: dwc: Stop selecting generic bindings by default

It's highly encouraged to have the separate DT schema for each available
particular device, while the generic schema should be left untouched
representing just a set of the common device properties (mainly advertised
by the IP-core reference manual). Seeing there is no currently DW PCIe
RP/EP dts nodes with only generic compatible string and since there isn't
any vendor-specific compatible string added to the generic DT schema,
before it's too late let's mark the snps,dw-pcie.yaml and
snps,dw-pcie-ep.yaml schemas not selected for checking by default and add
the explicit requirement to have the compatible string containing the
generic device name.

Note due to this modification we need to switch some of the DW PCIe-based
DT-bindings to referring to the common DT-schema instead of evaluating
against the generic DW PCIe DT-bindings. They are already defined as
having the vendor-specific compatible string only. So we can't change that
semantic.

Signed-off-by: Serge Semin <[email protected]>

---

Changelog v3:
- This is a new patch unpinned from the next one:
https://lore.kernel.org/linux-pci/[email protected]/
by the Rob' request. (@Rob)
- Fix compatible property schema so one would work as expected: string
must contain either generic DW PCIe IP-core name or both generic and
equipped with IP-core version names.
---
.../bindings/pci/fsl,imx6q-pcie.yaml | 3 ++-
.../bindings/pci/hisilicon,kirin-pcie.yaml | 3 ++-
.../bindings/pci/sifive,fu740-pcie.yaml | 3 ++-
.../bindings/pci/snps,dw-pcie-ep.yaml | 24 +++++++++++++++----
.../devicetree/bindings/pci/snps,dw-pcie.yaml | 24 +++++++++++++++----
.../pci/socionext,uniphier-pcie-ep.yaml | 9 +++----
.../bindings/pci/toshiba,visconti-pcie.yaml | 3 ++-
7 files changed, 53 insertions(+), 16 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
index 252e5b72aee0..6f99baa445a6 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
@@ -15,7 +15,8 @@ description: |+
and thus inherits all the common properties defined in snps,dw-pcie.yaml.

allOf:
- - $ref: /schemas/pci/snps,dw-pcie.yaml#
+ - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/snps,dw-pcie-common.yaml#

properties:
compatible:
diff --git a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
index c9f04999c9cf..f0d5314f340f 100644
--- a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
@@ -17,7 +17,8 @@ description: |
Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml.

allOf:
- - $ref: /schemas/pci/snps,dw-pcie.yaml#
+ - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/snps,dw-pcie-common.yaml#

properties:
compatible:
diff --git a/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml b/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
index 195e6afeb169..b0cf8ce99ce3 100644
--- a/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
@@ -17,7 +17,8 @@ maintainers:
- Greentime Hu <[email protected]>

allOf:
- - $ref: /schemas/pci/snps,dw-pcie.yaml#
+ - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/snps,dw-pcie-common.yaml#

properties:
compatible:
diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
index fc3b5d4ac245..b04ce7ddb796 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
@@ -13,6 +13,12 @@ maintainers:
description: |
Synopsys DesignWare PCIe host controller endpoint

+# Please create a separate DT-schema for the particular DWC PCIe Endpoint
+# controller and make sure it's assigned with the vendor-specific
+# compatible string together with the generic Synopsys DWC PCIe strings so
+# the bindings would be evaluated against that schema.
+select: false
+
allOf:
- $ref: /schemas/pci/pci-ep.yaml#
- $ref: /schemas/pci/snps,dw-pcie-common.yaml#
@@ -20,8 +26,18 @@ allOf:
properties:
compatible:
anyOf:
- - {}
- - const: snps,dw-pcie-ep
+ - description:
+ DWC PCIe Endpoint controller (IP-core version is explicitly
+ specified in the additional compatible string)
+ contains:
+ allOf:
+ - pattern: '^snps,dw-pcie-ep-[0-9]+\.[0-9]+a?$'
+ - const: snps,dw-pcie-ep
+ - description:
+ DWC PCIe Endpoint controller (IP-core version is either unknown
+ or can be read from the PCIe version register of the PL reg-space)
+ contains:
+ const: snps,dw-pcie-ep

reg:
description: |
@@ -38,16 +54,16 @@ properties:
enum: [dbi, dbi2, config, atu, addr_space, link, atu_dma, appl]

required:
+ - compatible
- reg
- reg-names
- - compatible

additionalProperties: true

examples:
- |
pcie-ep@dfd00000 {
- compatible = "snps,dw-pcie-ep";
+ compatible = "vendor,soc-pcie", "snps,dw-pcie-ep";
reg = <0xdfc00000 0x0001000>, /* IP registers 1 */
<0xdfc01000 0x0001000>, /* IP registers 2 */
<0xd0000000 0x2000000>; /* Configuration space */
diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
index 01cedf51e0f8..8b2e3210e3e2 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
@@ -13,6 +13,12 @@ maintainers:
description: |
Synopsys DesignWare PCIe host controller

+# Please create a separate DT-schema for the particular DWC PCIe Root Port
+# controller and make sure it's assigned with the vendor-specific
+# compatible string together with the generic Synopsys DWC PCIe strings so
+# the bindings would be evaluated against that schema.
+select: false
+
allOf:
- $ref: /schemas/pci/pci-bus.yaml#
- $ref: /schemas/pci/snps,dw-pcie-common.yaml#
@@ -20,8 +26,18 @@ allOf:
properties:
compatible:
anyOf:
- - {}
- - const: snps,dw-pcie
+ - description:
+ DWC PCIe Root Port controller (IP-core version is explicitly
+ specified in the additional compatible string)
+ contains:
+ allOf:
+ - pattern: '^snps,dw-pcie-[0-9]+\.[0-9]+a?$'
+ - const: snps,dw-pcie
+ - description:
+ DWC PCIe Root Port controller (IP-core version is either unknown
+ or can be read from the PCIe version register of the PL reg-space)
+ contains:
+ const: snps,dw-pcie

reg:
description: |
@@ -47,14 +63,14 @@ properties:
additionalProperties: true

required:
+ - compatible
- reg
- reg-names
- - compatible

examples:
- |
pcie@dfc00000 {
- compatible = "snps,dw-pcie";
+ compatible = "vendor,soc-pcie", "snps,dw-pcie";
device_type = "pci";
reg = <0xdfc00000 0x0001000>, /* IP registers */
<0xd0000000 0x0002000>; /* Configuration space */
diff --git a/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie-ep.yaml
index 437e61618d06..1719a36952c0 100644
--- a/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie-ep.yaml
@@ -20,9 +20,10 @@ allOf:

properties:
compatible:
- enum:
- - socionext,uniphier-pro5-pcie-ep
- - socionext,uniphier-nx1-pcie-ep
+ contains:
+ enum:
+ - socionext,uniphier-pro5-pcie-ep
+ - socionext,uniphier-nx1-pcie-ep

reg:
minItems: 4
@@ -92,7 +93,7 @@ unevaluatedProperties: false
examples:
- |
pcie_ep: pcie-ep@66000000 {
- compatible = "socionext,uniphier-pro5-pcie-ep";
+ compatible = "socionext,uniphier-pro5-pcie-ep", "snps,dw-pcie-ep";
reg-names = "dbi", "dbi2", "link", "addr_space";
reg = <0x66000000 0x1000>, <0x66001000 0x1000>,
<0x66010000 0x10000>, <0x67000000 0x400000>;
diff --git a/Documentation/devicetree/bindings/pci/toshiba,visconti-pcie.yaml b/Documentation/devicetree/bindings/pci/toshiba,visconti-pcie.yaml
index 30b6396d83c8..a08002ce9119 100644
--- a/Documentation/devicetree/bindings/pci/toshiba,visconti-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/toshiba,visconti-pcie.yaml
@@ -13,7 +13,8 @@ description:
Toshiba Visconti5 SoC PCIe host controller is based on the Synopsys DesignWare PCIe IP.

allOf:
- - $ref: /schemas/pci/snps,dw-pcie.yaml#
+ - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/snps,dw-pcie-common.yaml#

properties:
compatible:
--
2.35.1

2022-07-28 15:12:48

by Serge Semin

[permalink] [raw]
Subject: [PATCH v4 09/17] dt-bindings: PCI: dwc: Add clocks/resets common properties

DW PCIe RP/EP reference manuals explicit define all the clocks and reset
requirements in [1] and [2]. Seeing the DW PCIe vendor-specific
DT-bindings have already started assigning random names to the same set of
the clocks and resets lines, let's define a generic names sets and add
them to the DW PCIe definitions in the common DT-schema. These definitions
will be used in the generic DW PCIe DT-schema and can be referenced in the
particular DW PCIe DT-bindings if they are compatible with them, otherwise
the platforms can be left with already defined clocks/resets properties.

[1] Synopsys DesignWare Cores PCI Express Controller Databook - DWC PCIe
Root Port, Version 5.40a, March 2019, p.55 - 78.
[2] Synopsys DesignWare Cores PCI Express Controller Databook - DWC PCIe
Endpoint, Version 5.40a, March 2019, p.58 - 81.

Signed-off-by: Serge Semin <[email protected]>

---

Changelog v3:
- This is a new patch unpinned from the next one:
https://lore.kernel.org/linux-pci/[email protected]/
by the Rob' request. (@Rob)
- Drop synonymous from the names list since the device schemas create
their own enumerations anyway.
---
.../bindings/pci/samsung,exynos-pcie.yaml | 3 +-
.../bindings/pci/snps,dw-pcie-common.yaml | 100 ++++++++++++++++++
.../bindings/pci/snps,dw-pcie-ep.yaml | 26 +++++
.../devicetree/bindings/pci/snps,dw-pcie.yaml | 20 +++-
.../pci/socionext,uniphier-pcie-ep.yaml | 3 +-
5 files changed, 149 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
index 445eed94b53f..fedb774938f4 100644
--- a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
@@ -16,7 +16,8 @@ description: |+
snps,dw-pcie.yaml.

allOf:
- - $ref: /schemas/pci/snps,dw-pcie.yaml#
+ - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/snps,dw-pcie-common.yaml#

properties:
compatible:
diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
index c6a55f90ddd5..f22fb01c9bd0 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
@@ -58,6 +58,36 @@ properties:
minItems: 1
maxItems: 26

+ clocks:
+ description:
+ DWC PCIe reference manual explicitly defines a set of the clocks required
+ to get the controller working correctly. In general all of them can
+ be divided into two groups':' application and core clocks. Note the
+ platforms may have some of the clock sources unspecified in case if the
+ corresponding domains are fed up from a common clock source.
+ minItems: 1
+ maxItems: 7
+
+ clock-names:
+ minItems: 1
+ maxItems: 7
+
+ resets:
+ description:
+ DWC PCIe reference manual explicitly defines a set of the reset
+ signals required to be de-asserted to properly activate the controller
+ sub-parts. All of these signals can be divided into two sub-groups':'
+ application and core resets with respect to the main sub-domains they
+ are supposed to reset. Note the platforms may have some of these signals
+ unspecified in case if they are automatically handled or aggregated into
+ a comprehensive control module.
+ minItems: 1
+ maxItems: 10
+
+ reset-names:
+ minItems: 1
+ maxItems: 10
+
phys:
description:
There can be up to the number of possible lanes PHYs specified.
@@ -218,4 +248,74 @@ definitions:
uncorrectable error.
const: sft_ue

+ clock-names:
+ description:
+ Reference clock names common for the DWC PCIe Root Port and Endpoint
+ controllers.
+ anyOf:
+ - description:
+ Data Bus Interface (DBI) clock. Clock signal for the AXI-bus
+ interface of the Configuration-Dependent Module, which is
+ basically the set of the controller CSRs.
+ const: dbi
+ - description:
+ Application AXI-bus Master interface clock. Basically this is
+ a clock for the controller DMA interface (PCI-to-CPU).
+ const: mstr
+ - description:
+ Application AXI-bus Slave interface clock. This is a clock for
+ the CPU-to-PCI memory IO interface.
+ const: slv
+ - description:
+ Controller Core-PCS PIPE interface clock. It's normally
+ supplied by an external PCS-PHY.
+ const: pipe
+ - description:
+ Controller Primary clock. It's assumed that all controller input
+ signals (except resets) are synchronous to this clock.
+ const: core
+ - description:
+ Auxiliary clock for the controller PMC domain. The controller
+ partitioning implies having some parts to operate with this
+ clock in some power management states.
+ const: aux
+ - description:
+ Generic reference clock. In case if there are several
+ interfaces fed up with a common clock source it's advisable to
+ define it with this name (for instance pipe, core and aux can
+ be connected to a single source of the periodic signal).
+ const: ref
+ - description:
+ Clock for the PHY registers interface. Originally this is
+ a PHY-viewport-based interface, but some platform may have
+ specifically designed one.
+ const: phy_reg
+
+ reset-names:
+ description:
+ Reset signal names common for the DWC PCIe Root Port and Endpoint
+ controllers.
+ anyOf:
+ - description: Data Bus Interface (DBI) domain reset
+ const: dbi
+ - description: AXI-bus Master interface reset
+ const: mstr
+ - description: AXI-bus Slave interface reset
+ const: slv
+ - description: Controller Non-sticky CSR flags reset
+ const: non-sticky
+ - description: Controller sticky CSR flags reset
+ const: sticky
+ - description: PIPE-interface (Core-PCS) logic reset
+ const: pipe
+ - description:
+ Controller primary reset (resets everything except PMC module)
+ const: core
+ - description: PCS/PHY block reset
+ const: phy
+ - description: PMC hot reset signal
+ const: hot
+ - description: Cold reset signal
+ const: pwr
+
...
diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
index bfff723b529d..bb7e4381d392 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
@@ -74,6 +74,26 @@ properties:
items:
$ref: /schemas/pci/snps,dw-pcie-common.yaml#/definitions/interrupt-names

+ clocks:
+ minItems: 1
+ maxItems: 7
+
+ clock-names:
+ minItems: 1
+ maxItems: 7
+ items:
+ $ref: /schemas/pci/snps,dw-pcie-common.yaml#/definitions/clock-names
+
+ resets:
+ minItems: 1
+ maxItems: 10
+
+ reset-names:
+ minItems: 1
+ maxItems: 10
+ items:
+ $ref: /schemas/pci/snps,dw-pcie-common.yaml#/definitions/reset-names
+
max-functions:
maximum: 32

@@ -109,6 +129,12 @@ examples:
interrupts = <23>, <24>;
interrupt-names = "dma0", "dma1";

+ clocks = <&sys_clk 12>, <&sys_clk 24>;
+ clock-names = "dbi", "ref";
+
+ resets = <&sys_rst 12>, <&sys_rst 24>;
+ reset-names = "dbi", "phy";
+
phys = <&pcie_phy0>, <&pcie_phy1>, <&pcie_phy2>, <&pcie_phy3>;
phy-names = "pcie0", "pcie1", "pcie2", "pcie3";

diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
index 9e96c6d0ef48..518fa5626c11 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
@@ -78,7 +78,25 @@ properties:
- contains:
const: msi

- clocks: true
+ clocks:
+ minItems: 1
+ maxItems: 7
+
+ clock-names:
+ minItems: 1
+ maxItems: 7
+ items:
+ $ref: /schemas/pci/snps,dw-pcie-common.yaml#/definitions/clock-names
+
+ resets:
+ minItems: 1
+ maxItems: 10
+
+ reset-names:
+ minItems: 1
+ maxItems: 10
+ items:
+ $ref: /schemas/pci/snps,dw-pcie-common.yaml#/definitions/reset-names

additionalProperties: true

diff --git a/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie-ep.yaml
index 1719a36952c0..8c2a8e8f96f9 100644
--- a/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie-ep.yaml
@@ -16,7 +16,8 @@ maintainers:
- Kunihiko Hayashi <[email protected]>

allOf:
- - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
+ - $ref: /schemas/pci/pci-ep.yaml#
+ - $ref: /schemas/pci/snps,dw-pcie-common.yaml#

properties:
compatible:
--
2.35.1

2022-07-28 22:51:11

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 11/17] dt-bindings: PCI: dwc: Apply common schema to Rockchip DW PCIe nodes

On Thu, 28 Jul 2022 17:34:21 +0300, Serge Semin wrote:
> As the DT-bindings description states the Rockchip PCIe controller is
> based on the DW PCIe RP IP-core thus its DT-nodes are supposed to be
> compatible with the common DW PCIe controller schema. Let's make sure they
> evaluated against it by referring to the snps,dw-pcie-common.yaml schema
> in the allOf sub-schemas composition.
>
> Signed-off-by: Serge Semin <[email protected]>
>
> ---
>
> Changelog v3:
> - This is a new patch created on v3 lap of the series.
> ---
> Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/pci/snps,dw-pcie-common.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.example.dtb: pcie@fe280000: False schema does not allow {'compatible': ['rockchip,rk3568-pcie'], 'reg': [[3, 3229614080, 0, 3735552], [0, 4264034304, 0, 65536], [3, 2147483648, 0, 1048576]], 'reg-names': ['dbi', 'apb', 'config'], 'bus-range': [[32, 47]], 'clocks': [[4294967295, 143], [4294967295, 144], [4294967295, 145], [4294967295, 146], [4294967295, 147]], 'clock-names': ['aclk_mst', 'aclk_slv', 'aclk_dbi', 'pclk', 'aux'], 'device_type': ['pci'], 'linux,pci-domain': [[2]], 'max-link-speed': [[2]], 'msi-map': [[8192, 4294967295, 8192, 4096]], 'num-lanes': [[2]], 'phys': [[4294967295]], 'phy-names': ['pcie-phy'], 'power-domains': [[4294967295, 15]], 'ranges': [[2164260864, 0, 2155872256, 3, 2155872256, 0, 1048576], [2197815296, 0, 2156920832, 3, 2156920832, 0, 1064304640]], 'resets': [[4294967295, 193]], 'reset-names': ['pipe'], '#address-cells': [[3]], '#size-cells': [[2]], '$nodename': ['pcie
@fe280000']}
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

2022-07-28 22:51:16

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 05/17] dt-bindings: PCI: dwc: Stop selecting generic bindings by default

On Thu, 28 Jul 2022 17:34:15 +0300, Serge Semin wrote:
> It's highly encouraged to have the separate DT schema for each available
> particular device, while the generic schema should be left untouched
> representing just a set of the common device properties (mainly advertised
> by the IP-core reference manual). Seeing there is no currently DW PCIe
> RP/EP dts nodes with only generic compatible string and since there isn't
> any vendor-specific compatible string added to the generic DT schema,
> before it's too late let's mark the snps,dw-pcie.yaml and
> snps,dw-pcie-ep.yaml schemas not selected for checking by default and add
> the explicit requirement to have the compatible string containing the
> generic device name.
>
> Note due to this modification we need to switch some of the DW PCIe-based
> DT-bindings to referring to the common DT-schema instead of evaluating
> against the generic DW PCIe DT-bindings. They are already defined as
> having the vendor-specific compatible string only. So we can't change that
> semantic.
>
> Signed-off-by: Serge Semin <[email protected]>
>
> ---
>
> Changelog v3:
> - This is a new patch unpinned from the next one:
> https://lore.kernel.org/linux-pci/[email protected]/
> by the Rob' request. (@Rob)
> - Fix compatible property schema so one would work as expected: string
> must contain either generic DW PCIe IP-core name or both generic and
> equipped with IP-core version names.
> ---
> .../bindings/pci/fsl,imx6q-pcie.yaml | 3 ++-
> .../bindings/pci/hisilicon,kirin-pcie.yaml | 3 ++-
> .../bindings/pci/sifive,fu740-pcie.yaml | 3 ++-
> .../bindings/pci/snps,dw-pcie-ep.yaml | 24 +++++++++++++++----
> .../devicetree/bindings/pci/snps,dw-pcie.yaml | 24 +++++++++++++++----
> .../pci/socionext,uniphier-pcie-ep.yaml | 9 +++----
> .../bindings/pci/toshiba,visconti-pcie.yaml | 3 ++-
> 7 files changed, 53 insertions(+), 16 deletions(-)
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.example.dtb: pcie@e00000000: Unevaluated properties are not allowed ('clocks', 'interrupts' were unexpected)
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/toshiba,visconti-pcie.example.dtb: pcie@28400000: Unevaluated properties are not allowed ('interrupt-names' was unexpected)
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/toshiba,visconti-pcie.yaml
Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.example.dtb:0:0: /example-0/pcie-ep@dfd00000: failed to match any schema with compatible: ['vendor,soc-pcie', 'snps,dw-pcie-ep']
Documentation/devicetree/bindings/pci/snps,dw-pcie.example.dtb:0:0: /example-0/pcie@dfc00000: failed to match any schema with compatible: ['vendor,soc-pcie', 'snps,dw-pcie']
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.example.dtb: pcie@15700000: compatible: 'anyOf' conditional failed, one must be fixed:
['samsung,exynos5433-pcie'] does not contain items matching the given schema
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.example.dtb: pcie@15700000: Unevaluated properties are not allowed ('#address-cells', '#interrupt-cells', '#size-cells', 'bus-range', 'device_type', 'interrupt-map', 'interrupt-map-mask', 'ranges' were unexpected)
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie.example.dtb: pcie@66000000: compatible: 'anyOf' conditional failed, one must be fixed:
['socionext,uniphier-pcie'] does not contain items matching the given schema
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie.example.dtb: pcie@66000000: Unevaluated properties are not allowed ('#address-cells', '#interrupt-cells', '#size-cells', 'bus-range', 'device_type', 'interrupt-map', 'interrupt-map-mask', 'interrupt-names', 'interrupts', 'ranges' were unexpected)
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.example.dtb: pcie@f4000000: Unevaluated properties are not allowed ('interrupt-names', 'interrupts' were unexpected)
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.example.dtb: pcie@f5000000: Unevaluated properties are not allowed ('interrupt-names', 'interrupts' were unexpected)
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

2022-08-01 18:48:25

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 03/17] dt-bindings: PCI: dwc: Add phys/phy-names common properties

On Thu, Jul 28, 2022 at 05:34:13PM +0300, Serge Semin wrote:
> It's normal to have the DW PCIe RP/EP DT-nodes equipped with the explicit
> PHY phandle references. There can be up to 16 PHYs attach in accordance
> with the maximum number of supported PCIe lanes. Let's extend the common
> DW PCIe controller schema with the 'phys' and 'phy-names' properties
> definition. The PHY names are defined with the regexp pattern
> '^pcie([0-9]+|-?phy[0-9]*)?$' so to match the names currently supported by
> the DW PCIe platform drivers ("pcie": meson; "pciephy": qcom, imx6;
> "pcie-phy": uniphier, rockchip, spear13xx; "pcie": intel-gw; "pcie-phy%d":
> keystone, dra7xx; "pcie": histb, etc). Though the "pcie%d" format would
> the most preferable in this case.

No phy-names is my preference. Some string plus an index is utterly
pointless. Oh well...

>
> Signed-off-by: Serge Semin <[email protected]>
>
> ---
>
> Changelog v3:
> - This is a new patch unpinned from the next one:
> https://lore.kernel.org/linux-pci/[email protected]/
> by the Rob' request. (@Rob)
> ---
> .../bindings/pci/snps,dw-pcie-common.yaml | 15 +++++++++++++++
> .../devicetree/bindings/pci/snps,dw-pcie-ep.yaml | 3 +++
> .../devicetree/bindings/pci/snps,dw-pcie.yaml | 3 +++
> 3 files changed, 21 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> index 3e992b653d12..627a5d6625ba 100644
> --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> @@ -17,6 +17,21 @@ description:
> select: false
>
> properties:
> + phys:
> + description:
> + There can be up to the number of possible lanes PHYs specified.

This needs something about being in order of lane number.

> + Obviously each specified PHY is supposed to be able to work in the
> + PCIe mode with a speed implied by the DWC PCIe controller it is
> + attached to.
> + minItems: 1
> + maxItems: 16
> +
> + phy-names:
> + minItems: 1
> + maxItems: 16
> + items:
> + pattern: '^pcie([0-9]+|-?phy[0-9]*)?$'

Please comment here that pcie[0-9] is the preferred form.

Rob

2022-08-01 18:49:40

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 12/17] dt-bindings: PCI: dwc: Add Baikal-T1 PCIe Root Port bindings

On Thu, Jul 28, 2022 at 05:34:22PM +0300, Serge Semin wrote:
> Baikal-T1 SoC is equipped with DWC PCIe v4.60a Root Port controller, which
> link can be trained to work on up to Gen.3 speed over up to x4 lanes. The
> controller is supposed to be fed up with four clock sources: DBI
> peripheral clock, AXI application Tx/Rx clocks and external PHY/core
> reference clock generating the 100MHz signal. In addition to that the
> platform provide a way to reset each part of the controller:
> sticky/non-sticky bits, host controller core, PIPE interface, PCS/PHY and
> Hot/Power reset signal. The Root Port controller is equipped with multiple
> IRQ lines like MSI, system AER, PME, HP, Bandwidth change, Link
> equalization request and eDMA ones. The registers space is accessed over
> the DBI interface. There can be no more than four inbound or outbound iATU
> windows configured.
>
> Signed-off-by: Serge Semin <[email protected]>
>
> ---
>
> Changelog v2:
> - Rename 'syscon' property to 'baikal,bt1-syscon'.
> - Fix the 'compatible' property definition to being more specific about
> what strings are supposed to be used. Due to that we had to add the
> select property to evaluate the schema against the Baikal-T1 PCIe DT
> nodes only.
> ---
> .../bindings/pci/baikal,bt1-pcie.yaml | 154 ++++++++++++++++++
> 1 file changed, 154 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml
>
> diff --git a/Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml b/Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml
> new file mode 100644
> index 000000000000..23bd1d0aa5c5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml
> @@ -0,0 +1,154 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/baikal,bt1-pcie.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Baikal-T1 PCIe Root Port Controller
> +
> +maintainers:
> + - Serge Semin <[email protected]>
> +
> +description:
> + Embedded into Baikal-T1 SoC Root Complex controller. It's based on the
> + DWC RC PCIe v4.60a IP-core, which is configured to have just a single Root
> + Port function and is capable of establishing the link up to Gen.3 speed
> + on x4 lanes. It doesn't have embedded clock and reset control module, so
> + the proper interface initialization is supposed to be performed by software.
> +
> +select:
> + properties:
> + compatible:
> + contains:
> + const: baikal,bt1-pcie
> +
> + required:
> + - compatible
> +
> +allOf:
> + - $ref: /schemas/pci/snps,dw-pcie.yaml#
> +
> +properties:
> + compatible:
> + items:
> + - const: baikal,bt1-pcie
> + - const: snps,dw-pcie-4.60a
> + - const: snps,dw-pcie

Again, these fallbacks simply aren't useful.

> +
> + reg:
> + description:
> + DBI, DBI2 and at least 4KB outbound iATU-capable region.

'iATU-capable region' means config space? That's not very clear.

> + maxItems: 3
> +
> + reg-names:
> + minItems: 3
> + maxItems: 3
> + items:
> + enum: [ dbi, dbi2, config ]

Define the order. Here, and the rest.

> +
> + interrupts:
> + description:
> + MSI, AER, PME, Hot-plug, Link Bandwidth Management, Link Equalization
> + request and eight Read/Write eDMA IRQ lines are available.
> + maxItems: 14
> +
> + interrupt-names:
> + minItems: 14
> + maxItems: 14
> + items:
> + oneOf:
> + - pattern: '^dma[0-7]$'
> + - enum: [ msi, aer, pme, hp, bw_mg, l_eq ]
> +
> + clocks:
> + description:
> + DBI (attached to the APB bus), AXI-bus master and slave interfaces
> + are fed up by the dedicated application clocks. A common reference
> + clock signal is supposed to be attached to the corresponding Ref-pad
> + of the SoC. It will be redistributed amongst the controller core
> + sub-modules (pipe, core, aux, etc).
> + minItems: 4
> + maxItems: 4
> +
> + clock-names:
> + minItems: 4
> + maxItems: 4
> + items:
> + enum: [ dbi, mstr, slv, ref ]
> +
> + resets:
> + description:
> + A comprehensive controller reset logic is supposed to be implemented
> + by software, so almost all the possible application and core reset
> + signals are exposed via the system CCU module.
> + minItems: 9
> + maxItems: 9
> +
> + reset-names:
> + minItems: 9
> + maxItems: 9
> + items:
> + enum: [ mstr, slv, pwr, hot, phy, core, pipe, sticky, non-sticky ]
> +
> + baikal,bt1-syscon:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to the Baikal-T1 System Controller DT node. It's required to
> + access some additional PM, Reset-related and LTSSM signals.
> +
> + num-lanes:
> + maximum: 4
> +
> + max-link-speed:
> + maximum: 3
> +

> + num-ob-windows:
> + const: 4
> +
> + num-ib-windows:
> + const: 4

These are deprecated. Don't add them for a new binding.


> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - interrupts
> + - interrupt-names
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + pcie@1f052000 {
> + compatible = "baikal,bt1-pcie", "snps,dw-pcie-4.60a", "snps,dw-pcie";
> + device_type = "pci";
> + reg = <0x1f052000 0x1000>, <0x1f053000 0x1000>, <0x1bdbf000 0x1000>;
> + reg-names = "dbi", "dbi2", "config";
> + #address-cells = <3>;
> + #size-cells = <2>;
> + ranges = <0x81000000 0 0x00000000 0x1bdb0000 0 0x00008000>,
> + <0x82000000 0 0x20000000 0x08000000 0 0x13db0000>;
> + bus-range = <0x0 0xff>;
> +
> + interrupts = <0 80 4>, <0 81 4>, <0 82 4>, <0 83 4>,
> + <0 84 4>, <0 85 4>, <0 86 4>, <0 87 4>,
> + <0 88 4>, <0 89 4>, <0 90 4>, <0 91 4>,
> + <0 92 4>, <0 93 4>;
> + interrupt-names = "dma0", "dma1", "dma2", "dma3", "dma4", "dma5", "dma6",
> + "dma7", "msi", "aer", "pme", "hp", "bw_mg", "l_eq";
> +
> + clocks = <&ccu_sys 1>, <&ccu_axi 6>, <&ccu_axi 7>, <&clk_pcie>;
> + clock-names = "dbi", "mstr", "slv", "ref";
> +
> + resets = <&ccu_axi 6>, <&ccu_axi 7>, <&ccu_sys 7>, <&ccu_sys 10>,
> + <&ccu_sys 4>, <&ccu_sys 6>, <&ccu_sys 5>, <&ccu_sys 8>,
> + <&ccu_sys 9>;
> + reset-names = "mstr", "slv", "pwr", "hot", "phy", "core", "pipe",
> + "sticky", "non-sticky";
> +
> + reset-gpios = <&port0 0 1>;
> +
> + num-lanes = <4>;
> + max-link-speed = <3>;
> + };
> +...
> --
> 2.35.1
>
>

2022-08-01 19:01:35

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 04/17] dt-bindings: PCI: dwc: Add max-link-speed common property

On Thu, 28 Jul 2022 17:34:14 +0300, Serge Semin wrote:
> In accordance with [1] DW PCIe controllers support up to Gen5 link speed.
> Let's add the max-link-speed property upper bound to 5 then. The DT
> bindings of the particular devices are expected to setup more strict
> constraint on that parameter.
>
> [1] Synopsys DesignWare Cores PCI Express Controller Databook, Version
> 5.40a, March 2019, p. 27
>
> Signed-off-by: Serge Semin <[email protected]>
>
> ---
>
> Changelog v3:
> - This is a new patch unpinned from the next one:
> https://lore.kernel.org/linux-pci/[email protected]/
> by the Rob' request. (@Rob)
> ---
> Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml | 3 +++
> Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml | 2 ++
> Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml | 1 +
> 3 files changed, 6 insertions(+)
>

Reviewed-by: Rob Herring <[email protected]>

2022-08-08 10:59:21

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v4 03/17] dt-bindings: PCI: dwc: Add phys/phy-names common properties

On Mon, Aug 01, 2022 at 11:56:36AM -0600, Rob Herring wrote:
> On Thu, Jul 28, 2022 at 05:34:13PM +0300, Serge Semin wrote:
> > It's normal to have the DW PCIe RP/EP DT-nodes equipped with the explicit
> > PHY phandle references. There can be up to 16 PHYs attach in accordance
> > with the maximum number of supported PCIe lanes. Let's extend the common
> > DW PCIe controller schema with the 'phys' and 'phy-names' properties
> > definition. The PHY names are defined with the regexp pattern
> > '^pcie([0-9]+|-?phy[0-9]*)?$' so to match the names currently supported by
> > the DW PCIe platform drivers ("pcie": meson; "pciephy": qcom, imx6;
> > "pcie-phy": uniphier, rockchip, spear13xx; "pcie": intel-gw; "pcie-phy%d":
> > keystone, dra7xx; "pcie": histb, etc). Though the "pcie%d" format would
> > the most preferable in this case.
>

> No phy-names is my preference. Some string plus an index is utterly
> pointless. Oh well...

Mine too, but we have no choice in this case since the named
PHY-properties support has already been advertised by the platform
drivers. This patch just provides the bindings for them. Just note
the string patterns have been designed in a way to match these bindings.

Anyway thanks for telling about the preferred option. Keeping that in
mind I won't have doubts what approach to select for the new
driver/bindings development.

>
> >
> > Signed-off-by: Serge Semin <[email protected]>
> >
> > ---
> >
> > Changelog v3:
> > - This is a new patch unpinned from the next one:
> > https://lore.kernel.org/linux-pci/[email protected]/
> > by the Rob' request. (@Rob)
> > ---
> > .../bindings/pci/snps,dw-pcie-common.yaml | 15 +++++++++++++++
> > .../devicetree/bindings/pci/snps,dw-pcie-ep.yaml | 3 +++
> > .../devicetree/bindings/pci/snps,dw-pcie.yaml | 3 +++
> > 3 files changed, 21 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> > index 3e992b653d12..627a5d6625ba 100644
> > --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> > +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> > @@ -17,6 +17,21 @@ description:
> > select: false
> >
> > properties:
> > + phys:
> > + description:
> > + There can be up to the number of possible lanes PHYs specified.
>

> This needs something about being in order of lane number.

Ok.

>
> > + Obviously each specified PHY is supposed to be able to work in the
> > + PCIe mode with a speed implied by the DWC PCIe controller it is
> > + attached to.
> > + minItems: 1
> > + maxItems: 16
> > +
> > + phy-names:
> > + minItems: 1
> > + maxItems: 16
> > + items:
> > + pattern: '^pcie([0-9]+|-?phy[0-9]*)?$'
>

> Please comment here that pcie[0-9] is the preferred form.

What about a bit more sophisticated update?
phy-names:
minItems: 1
maxItems: 16
oneOf:
- items:
pattern: '^pcie[0-9]+$'
- deprecated: true
items:
pattern: '^pcie(-?phy[0-9]*)?$'

-Sergey

>
> Rob

2022-08-08 16:03:14

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 03/17] dt-bindings: PCI: dwc: Add phys/phy-names common properties

On Mon, Aug 8, 2022 at 4:36 AM Serge Semin <[email protected]> wrote:
>
> On Mon, Aug 01, 2022 at 11:56:36AM -0600, Rob Herring wrote:
> > On Thu, Jul 28, 2022 at 05:34:13PM +0300, Serge Semin wrote:
> > > It's normal to have the DW PCIe RP/EP DT-nodes equipped with the explicit
> > > PHY phandle references. There can be up to 16 PHYs attach in accordance
> > > with the maximum number of supported PCIe lanes. Let's extend the common
> > > DW PCIe controller schema with the 'phys' and 'phy-names' properties
> > > definition. The PHY names are defined with the regexp pattern
> > > '^pcie([0-9]+|-?phy[0-9]*)?$' so to match the names currently supported by
> > > the DW PCIe platform drivers ("pcie": meson; "pciephy": qcom, imx6;
> > > "pcie-phy": uniphier, rockchip, spear13xx; "pcie": intel-gw; "pcie-phy%d":
> > > keystone, dra7xx; "pcie": histb, etc). Though the "pcie%d" format would
> > > the most preferable in this case.
> >
>
> > No phy-names is my preference. Some string plus an index is utterly
> > pointless. Oh well...
>
> Mine too, but we have no choice in this case since the named
> PHY-properties support has already been advertised by the platform
> drivers. This patch just provides the bindings for them. Just note
> the string patterns have been designed in a way to match these bindings.
>
> Anyway thanks for telling about the preferred option. Keeping that in
> mind I won't have doubts what approach to select for the new
> driver/bindings development.
>
> >
> > >
> > > Signed-off-by: Serge Semin <[email protected]>
> > >
> > > ---
> > >
> > > Changelog v3:
> > > - This is a new patch unpinned from the next one:
> > > https://lore.kernel.org/linux-pci/[email protected]/
> > > by the Rob' request. (@Rob)
> > > ---
> > > .../bindings/pci/snps,dw-pcie-common.yaml | 15 +++++++++++++++
> > > .../devicetree/bindings/pci/snps,dw-pcie-ep.yaml | 3 +++
> > > .../devicetree/bindings/pci/snps,dw-pcie.yaml | 3 +++
> > > 3 files changed, 21 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> > > index 3e992b653d12..627a5d6625ba 100644
> > > --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> > > +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> > > @@ -17,6 +17,21 @@ description:
> > > select: false
> > >
> > > properties:
> > > + phys:
> > > + description:
> > > + There can be up to the number of possible lanes PHYs specified.
> >
>
> > This needs something about being in order of lane number.
>
> Ok.
>
> >
> > > + Obviously each specified PHY is supposed to be able to work in the
> > > + PCIe mode with a speed implied by the DWC PCIe controller it is
> > > + attached to.
> > > + minItems: 1
> > > + maxItems: 16
> > > +
> > > + phy-names:
> > > + minItems: 1
> > > + maxItems: 16
> > > + items:
> > > + pattern: '^pcie([0-9]+|-?phy[0-9]*)?$'
> >
>
> > Please comment here that pcie[0-9] is the preferred form.
>
> What about a bit more sophisticated update?
> phy-names:
> minItems: 1
> maxItems: 16
> oneOf:
> - items:
> pattern: '^pcie[0-9]+$'
> - deprecated: true
> items:
> pattern: '^pcie(-?phy[0-9]*)?$'

Sure.

Rob

2022-08-08 16:47:29

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v4 12/17] dt-bindings: PCI: dwc: Add Baikal-T1 PCIe Root Port bindings

On Mon, Aug 01, 2022 at 12:13:11PM -0600, Rob Herring wrote:
> On Thu, Jul 28, 2022 at 05:34:22PM +0300, Serge Semin wrote:
> > Baikal-T1 SoC is equipped with DWC PCIe v4.60a Root Port controller, which
> > link can be trained to work on up to Gen.3 speed over up to x4 lanes. The
> > controller is supposed to be fed up with four clock sources: DBI
> > peripheral clock, AXI application Tx/Rx clocks and external PHY/core
> > reference clock generating the 100MHz signal. In addition to that the
> > platform provide a way to reset each part of the controller:
> > sticky/non-sticky bits, host controller core, PIPE interface, PCS/PHY and
> > Hot/Power reset signal. The Root Port controller is equipped with multiple
> > IRQ lines like MSI, system AER, PME, HP, Bandwidth change, Link
> > equalization request and eDMA ones. The registers space is accessed over
> > the DBI interface. There can be no more than four inbound or outbound iATU
> > windows configured.
> >
> > Signed-off-by: Serge Semin <[email protected]>
> >
> > ---
> >
> > Changelog v2:
> > - Rename 'syscon' property to 'baikal,bt1-syscon'.
> > - Fix the 'compatible' property definition to being more specific about
> > what strings are supposed to be used. Due to that we had to add the
> > select property to evaluate the schema against the Baikal-T1 PCIe DT
> > nodes only.
> > ---
> > .../bindings/pci/baikal,bt1-pcie.yaml | 154 ++++++++++++++++++
> > 1 file changed, 154 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml b/Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml
> > new file mode 100644
> > index 000000000000..23bd1d0aa5c5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml
> > @@ -0,0 +1,154 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pci/baikal,bt1-pcie.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Baikal-T1 PCIe Root Port Controller
> > +
> > +maintainers:
> > + - Serge Semin <[email protected]>
> > +
> > +description:
> > + Embedded into Baikal-T1 SoC Root Complex controller. It's based on the
> > + DWC RC PCIe v4.60a IP-core, which is configured to have just a single Root
> > + Port function and is capable of establishing the link up to Gen.3 speed
> > + on x4 lanes. It doesn't have embedded clock and reset control module, so
> > + the proper interface initialization is supposed to be performed by software.
> > +
> > +select:
> > + properties:
> > + compatible:
> > + contains:
> > + const: baikal,bt1-pcie
> > +
> > + required:
> > + - compatible
> > +
> > +allOf:
> > + - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - const: baikal,bt1-pcie
> > + - const: snps,dw-pcie-4.60a
> > + - const: snps,dw-pcie
>

> Again, these fallbacks simply aren't useful.

Ok. I give up. You are the boss. I'll drop them =)

>
> > +
> > + reg:
> > + description:
> > + DBI, DBI2 and at least 4KB outbound iATU-capable region.
>

> 'iATU-capable region' means config space? That's not very clear.

No 'iATU-capable region' means the region, which can be used as a
source address for the iATU engine. In general it can be used for any
accesses (IO, MEM, CFG). But the DW PCIe driver will indeed use it for
the config-space accesses.

IMO the 'config' reg space is kind of virtual. Due to the outbound
iATU capability the driver could use any free outbound iATU region it
found instead.

>
> > + maxItems: 3
> > +
> > + reg-names:
> > + minItems: 3
> > + maxItems: 3
> > + items:
> > + enum: [ dbi, dbi2, config ]
>

> Define the order. Here, and the rest.

Ok. I will, but please answer to my question, I asked you in the
previous email thread:

Serge Semin wrote:
> Rob Herring wrote:
> > ...
> > Tell me why you need random order.
>
> Because I don't see a need in constraining the order. If we get to set
> the order requirement, then why do we need to have the "*-names"
> property at all?
> IMO having "reg" with max/minItems restriction plus generic
> description and "reg-names" with possible values enumerated seems very
> suitable pattern in this case. Don't you think?

In addition to that what about optional names? How would you suggest
to handle such case without the non-ordered pattern?

>
> > +
> > + interrupts:
> > + description:
> > + MSI, AER, PME, Hot-plug, Link Bandwidth Management, Link Equalization
> > + request and eight Read/Write eDMA IRQ lines are available.
> > + maxItems: 14
> > +
> > + interrupt-names:
> > + minItems: 14
> > + maxItems: 14
> > + items:
> > + oneOf:
> > + - pattern: '^dma[0-7]$'
> > + - enum: [ msi, aer, pme, hp, bw_mg, l_eq ]
> > +
> > + clocks:
> > + description:
> > + DBI (attached to the APB bus), AXI-bus master and slave interfaces
> > + are fed up by the dedicated application clocks. A common reference
> > + clock signal is supposed to be attached to the corresponding Ref-pad
> > + of the SoC. It will be redistributed amongst the controller core
> > + sub-modules (pipe, core, aux, etc).
> > + minItems: 4
> > + maxItems: 4
> > +
> > + clock-names:
> > + minItems: 4
> > + maxItems: 4
> > + items:
> > + enum: [ dbi, mstr, slv, ref ]
> > +
> > + resets:
> > + description:
> > + A comprehensive controller reset logic is supposed to be implemented
> > + by software, so almost all the possible application and core reset
> > + signals are exposed via the system CCU module.
> > + minItems: 9
> > + maxItems: 9
> > +
> > + reset-names:
> > + minItems: 9
> > + maxItems: 9
> > + items:
> > + enum: [ mstr, slv, pwr, hot, phy, core, pipe, sticky, non-sticky ]
> > +
> > + baikal,bt1-syscon:
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + description:
> > + Phandle to the Baikal-T1 System Controller DT node. It's required to
> > + access some additional PM, Reset-related and LTSSM signals.
> > +
> > + num-lanes:
> > + maximum: 4
> > +
> > + max-link-speed:
> > + maximum: 3
> > +
>
> > + num-ob-windows:
> > + const: 4
> > +
> > + num-ib-windows:
> > + const: 4
>

> These are deprecated. Don't add them for a new binding.

Ok.

-Sergey

>
>
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - reg-names
> > + - interrupts
> > + - interrupt-names
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > + - |
> > + pcie@1f052000 {
> > + compatible = "baikal,bt1-pcie", "snps,dw-pcie-4.60a", "snps,dw-pcie";
> > + device_type = "pci";
> > + reg = <0x1f052000 0x1000>, <0x1f053000 0x1000>, <0x1bdbf000 0x1000>;
> > + reg-names = "dbi", "dbi2", "config";
> > + #address-cells = <3>;
> > + #size-cells = <2>;
> > + ranges = <0x81000000 0 0x00000000 0x1bdb0000 0 0x00008000>,
> > + <0x82000000 0 0x20000000 0x08000000 0 0x13db0000>;
> > + bus-range = <0x0 0xff>;
> > +
> > + interrupts = <0 80 4>, <0 81 4>, <0 82 4>, <0 83 4>,
> > + <0 84 4>, <0 85 4>, <0 86 4>, <0 87 4>,
> > + <0 88 4>, <0 89 4>, <0 90 4>, <0 91 4>,
> > + <0 92 4>, <0 93 4>;
> > + interrupt-names = "dma0", "dma1", "dma2", "dma3", "dma4", "dma5", "dma6",
> > + "dma7", "msi", "aer", "pme", "hp", "bw_mg", "l_eq";
> > +
> > + clocks = <&ccu_sys 1>, <&ccu_axi 6>, <&ccu_axi 7>, <&clk_pcie>;
> > + clock-names = "dbi", "mstr", "slv", "ref";
> > +
> > + resets = <&ccu_axi 6>, <&ccu_axi 7>, <&ccu_sys 7>, <&ccu_sys 10>,
> > + <&ccu_sys 4>, <&ccu_sys 6>, <&ccu_sys 5>, <&ccu_sys 8>,
> > + <&ccu_sys 9>;
> > + reset-names = "mstr", "slv", "pwr", "hot", "phy", "core", "pipe",
> > + "sticky", "non-sticky";
> > +
> > + reset-gpios = <&port0 0 1>;
> > +
> > + num-lanes = <4>;
> > + max-link-speed = <3>;
> > + };
> > +...
> > --
> > 2.35.1
> >
> >

2022-08-09 15:21:48

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 12/17] dt-bindings: PCI: dwc: Add Baikal-T1 PCIe Root Port bindings

On Mon, Aug 8, 2022 at 10:01 AM Serge Semin <[email protected]> wrote:
>
> On Mon, Aug 01, 2022 at 12:13:11PM -0600, Rob Herring wrote:
> > On Thu, Jul 28, 2022 at 05:34:22PM +0300, Serge Semin wrote:
> > > Baikal-T1 SoC is equipped with DWC PCIe v4.60a Root Port controller, which
> > > link can be trained to work on up to Gen.3 speed over up to x4 lanes. The
> > > controller is supposed to be fed up with four clock sources: DBI
> > > peripheral clock, AXI application Tx/Rx clocks and external PHY/core
> > > reference clock generating the 100MHz signal. In addition to that the
> > > platform provide a way to reset each part of the controller:
> > > sticky/non-sticky bits, host controller core, PIPE interface, PCS/PHY and
> > > Hot/Power reset signal. The Root Port controller is equipped with multiple
> > > IRQ lines like MSI, system AER, PME, HP, Bandwidth change, Link
> > > equalization request and eDMA ones. The registers space is accessed over
> > > the DBI interface. There can be no more than four inbound or outbound iATU
> > > windows configured.
> > >
> > > Signed-off-by: Serge Semin <[email protected]>
> > >
> > > ---
> > >
> > > Changelog v2:
> > > - Rename 'syscon' property to 'baikal,bt1-syscon'.
> > > - Fix the 'compatible' property definition to being more specific about
> > > what strings are supposed to be used. Due to that we had to add the
> > > select property to evaluate the schema against the Baikal-T1 PCIe DT
> > > nodes only.
> > > ---
> > > .../bindings/pci/baikal,bt1-pcie.yaml | 154 ++++++++++++++++++
> > > 1 file changed, 154 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml b/Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml
> > > new file mode 100644
> > > index 000000000000..23bd1d0aa5c5
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml
> > > @@ -0,0 +1,154 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/pci/baikal,bt1-pcie.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Baikal-T1 PCIe Root Port Controller
> > > +
> > > +maintainers:
> > > + - Serge Semin <[email protected]>
> > > +
> > > +description:
> > > + Embedded into Baikal-T1 SoC Root Complex controller. It's based on the
> > > + DWC RC PCIe v4.60a IP-core, which is configured to have just a single Root
> > > + Port function and is capable of establishing the link up to Gen.3 speed
> > > + on x4 lanes. It doesn't have embedded clock and reset control module, so
> > > + the proper interface initialization is supposed to be performed by software.
> > > +
> > > +select:
> > > + properties:
> > > + compatible:
> > > + contains:
> > > + const: baikal,bt1-pcie
> > > +
> > > + required:
> > > + - compatible
> > > +
> > > +allOf:
> > > + - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > +
> > > +properties:
> > > + compatible:
> > > + items:
> > > + - const: baikal,bt1-pcie
> > > + - const: snps,dw-pcie-4.60a
> > > + - const: snps,dw-pcie
> >
>
> > Again, these fallbacks simply aren't useful.
>
> Ok. I give up. You are the boss. I'll drop them =)
>
> >
> > > +
> > > + reg:
> > > + description:
> > > + DBI, DBI2 and at least 4KB outbound iATU-capable region.
> >
>
> > 'iATU-capable region' means config space? That's not very clear.
>
> No 'iATU-capable region' means the region, which can be used as a
> source address for the iATU engine. In general it can be used for any
> accesses (IO, MEM, CFG). But the DW PCIe driver will indeed use it for
> the config-space accesses.
>
> IMO the 'config' reg space is kind of virtual. Due to the outbound
> iATU capability the driver could use any free outbound iATU region it
> found instead.

It is and in hindsight, we maybe should have described the whole
address space and let the OS alloc the config space out of it. But
then again, original OpenFirmware PCI binding reflects what the
firmware discovered AND how it is configured. So specifying where
config space is makes sense.

Bottom line is the binding defines putting the config space region in
'reg', not an iATU region.

> > > + maxItems: 3
> > > +
> > > + reg-names:
> > > + minItems: 3
> > > + maxItems: 3
> > > + items:
> > > + enum: [ dbi, dbi2, config ]
> >
>
> > Define the order. Here, and the rest.
>
> Ok. I will, but please answer to my question, I asked you in the
> previous email thread:
>
> Serge Semin wrote:
> > Rob Herring wrote:
> > > ...
> > > Tell me why you need random order.
> >
> > Because I don't see a need in constraining the order. If we get to set
> > the order requirement, then why do we need to have the "*-names"
> > property at all?

Originally, it was for cases where you have a variable number of
entries and can't determine what each entry is. IOW, when you have
optional entries in the middle of required entries. But then everyone
*loves* -names even when not needed or useful such as 'phy-names =
"pcie"' (the phy subsys requiring names was part of the problem there,
but that's been fixed).

> > IMO having "reg" with max/minItems restriction plus generic
> > description and "reg-names" with possible values enumerated seems very
> > suitable pattern in this case. Don't you think?

No, I think this is just as concise and defines the order too:

reg-names:
items:
- const: dbi
- const: dbi2
- const: config

>
> In addition to that what about optional names? How would you suggest
> to handle such case without the non-ordered pattern?

Sorry, I don't follow.

Rob

2022-08-09 19:35:45

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v4 12/17] dt-bindings: PCI: dwc: Add Baikal-T1 PCIe Root Port bindings

On Tue, Aug 09, 2022 at 09:12:31AM -0600, Rob Herring wrote:
> On Mon, Aug 8, 2022 at 10:01 AM Serge Semin <[email protected]> wrote:
> >
> > On Mon, Aug 01, 2022 at 12:13:11PM -0600, Rob Herring wrote:
> > > On Thu, Jul 28, 2022 at 05:34:22PM +0300, Serge Semin wrote:
> > > > Baikal-T1 SoC is equipped with DWC PCIe v4.60a Root Port controller, which
> > > > link can be trained to work on up to Gen.3 speed over up to x4 lanes. The
> > > > controller is supposed to be fed up with four clock sources: DBI
> > > > peripheral clock, AXI application Tx/Rx clocks and external PHY/core
> > > > reference clock generating the 100MHz signal. In addition to that the
> > > > platform provide a way to reset each part of the controller:
> > > > sticky/non-sticky bits, host controller core, PIPE interface, PCS/PHY and
> > > > Hot/Power reset signal. The Root Port controller is equipped with multiple
> > > > IRQ lines like MSI, system AER, PME, HP, Bandwidth change, Link
> > > > equalization request and eDMA ones. The registers space is accessed over
> > > > the DBI interface. There can be no more than four inbound or outbound iATU
> > > > windows configured.
> > > >
> > > > Signed-off-by: Serge Semin <[email protected]>
> > > >
> > > > ---
> > > >
> > > > Changelog v2:
> > > > - Rename 'syscon' property to 'baikal,bt1-syscon'.
> > > > - Fix the 'compatible' property definition to being more specific about
> > > > what strings are supposed to be used. Due to that we had to add the
> > > > select property to evaluate the schema against the Baikal-T1 PCIe DT
> > > > nodes only.
> > > > ---
> > > > .../bindings/pci/baikal,bt1-pcie.yaml | 154 ++++++++++++++++++
> > > > 1 file changed, 154 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml b/Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml
> > > > new file mode 100644
> > > > index 000000000000..23bd1d0aa5c5
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml
> > > > @@ -0,0 +1,154 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/pci/baikal,bt1-pcie.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Baikal-T1 PCIe Root Port Controller
> > > > +
> > > > +maintainers:
> > > > + - Serge Semin <[email protected]>
> > > > +
> > > > +description:
> > > > + Embedded into Baikal-T1 SoC Root Complex controller. It's based on the
> > > > + DWC RC PCIe v4.60a IP-core, which is configured to have just a single Root
> > > > + Port function and is capable of establishing the link up to Gen.3 speed
> > > > + on x4 lanes. It doesn't have embedded clock and reset control module, so
> > > > + the proper interface initialization is supposed to be performed by software.
> > > > +
> > > > +select:
> > > > + properties:
> > > > + compatible:
> > > > + contains:
> > > > + const: baikal,bt1-pcie
> > > > +
> > > > + required:
> > > > + - compatible
> > > > +
> > > > +allOf:
> > > > + - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + items:
> > > > + - const: baikal,bt1-pcie
> > > > + - const: snps,dw-pcie-4.60a
> > > > + - const: snps,dw-pcie
> > >
> >
> > > Again, these fallbacks simply aren't useful.
> >
> > Ok. I give up. You are the boss. I'll drop them =)
> >
> > >
> > > > +
> > > > + reg:
> > > > + description:
> > > > + DBI, DBI2 and at least 4KB outbound iATU-capable region.
> > >
> >
> > > 'iATU-capable region' means config space? That's not very clear.
> >
> > No 'iATU-capable region' means the region, which can be used as a
> > source address for the iATU engine. In general it can be used for any
> > accesses (IO, MEM, CFG). But the DW PCIe driver will indeed use it for
> > the config-space accesses.
> >
> > IMO the 'config' reg space is kind of virtual. Due to the outbound
> > iATU capability the driver could use any free outbound iATU region it
> > found instead.
>
> It is and in hindsight, we maybe should have described the whole
> address space and let the OS alloc the config space out of it. But
> then again, original OpenFirmware PCI binding reflects what the
> firmware discovered AND how it is configured. So specifying where
> config space is makes sense.
>
> Bottom line is the binding defines putting the config space region in
> 'reg', not an iATU region.
>
> > > > + maxItems: 3
> > > > +
> > > > + reg-names:
> > > > + minItems: 3
> > > > + maxItems: 3
> > > > + items:
> > > > + enum: [ dbi, dbi2, config ]
> > >
> >
> > > Define the order. Here, and the rest.
> >
> > Ok. I will, but please answer to my question, I asked you in the
> > previous email thread:
> >
> > Serge Semin wrote:
> > > Rob Herring wrote:
> > > > ...
> > > > Tell me why you need random order.
> > >
> > > Because I don't see a need in constraining the order. If we get to set
> > > the order requirement, then why do we need to have the "*-names"
> > > property at all?
>
> Originally, it was for cases where you have a variable number of
> entries and can't determine what each entry is. IOW, when you have
> optional entries in the middle of required entries. But then everyone
> *loves* -names even when not needed or useful such as 'phy-names =
> "pcie"' (the phy subsys requiring names was part of the problem there,
> but that's been fixed).
>
> > > IMO having "reg" with max/minItems restriction plus generic
> > > description and "reg-names" with possible values enumerated seems very
> > > suitable pattern in this case. Don't you think?
>
> No, I think this is just as concise and defines the order too:
>
> reg-names:
> items:
> - const: dbi
> - const: dbi2
> - const: config
>
> >
> > In addition to that what about optional names? How would you suggest
> > to handle such case without the non-ordered pattern?
>

> Sorry, I don't follow.

I meant exactly the case you've described as the main goal of the
named properties. My worry was that by using the pattern:

reg-names:
items:
- const: name
- const: another_name
- const: one_more_name

you get to fix the names order, which they were invented to get rid
from. If you get to use that pattern the only optional names could be
the names at the tail of the array, which isn't always applicable. In
that case you'd have no choice but to use the pattern suggested by
me.

-Sergey

>
> Rob

2022-08-09 20:22:43

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 12/17] dt-bindings: PCI: dwc: Add Baikal-T1 PCIe Root Port bindings

On Tue, Aug 9, 2022 at 1:28 PM Serge Semin <[email protected]> wrote:
>
> On Tue, Aug 09, 2022 at 09:12:31AM -0600, Rob Herring wrote:
> > On Mon, Aug 8, 2022 at 10:01 AM Serge Semin <[email protected]> wrote:
> > >
> > > On Mon, Aug 01, 2022 at 12:13:11PM -0600, Rob Herring wrote:
> > > > On Thu, Jul 28, 2022 at 05:34:22PM +0300, Serge Semin wrote:
> > > > > Baikal-T1 SoC is equipped with DWC PCIe v4.60a Root Port controller, which
> > > > > link can be trained to work on up to Gen.3 speed over up to x4 lanes. The
> > > > > controller is supposed to be fed up with four clock sources: DBI
> > > > > peripheral clock, AXI application Tx/Rx clocks and external PHY/core
> > > > > reference clock generating the 100MHz signal. In addition to that the
> > > > > platform provide a way to reset each part of the controller:
> > > > > sticky/non-sticky bits, host controller core, PIPE interface, PCS/PHY and
> > > > > Hot/Power reset signal. The Root Port controller is equipped with multiple
> > > > > IRQ lines like MSI, system AER, PME, HP, Bandwidth change, Link
> > > > > equalization request and eDMA ones. The registers space is accessed over
> > > > > the DBI interface. There can be no more than four inbound or outbound iATU
> > > > > windows configured.
> > > > >
> > > > > Signed-off-by: Serge Semin <[email protected]>
> > > > >

[...]

> > > > > + reg-names:
> > > > > + minItems: 3
> > > > > + maxItems: 3
> > > > > + items:
> > > > > + enum: [ dbi, dbi2, config ]
> > > >
> > >
> > > > Define the order. Here, and the rest.
> > >
> > > Ok. I will, but please answer to my question, I asked you in the
> > > previous email thread:
> > >
> > > Serge Semin wrote:
> > > > Rob Herring wrote:
> > > > > ...
> > > > > Tell me why you need random order.
> > > >
> > > > Because I don't see a need in constraining the order. If we get to set
> > > > the order requirement, then why do we need to have the "*-names"
> > > > property at all?
> >
> > Originally, it was for cases where you have a variable number of
> > entries and can't determine what each entry is. IOW, when you have
> > optional entries in the middle of required entries. But then everyone
> > *loves* -names even when not needed or useful such as 'phy-names =
> > "pcie"' (the phy subsys requiring names was part of the problem there,
> > but that's been fixed).
> >

> > > > IMO having "reg" with max/minItems restriction plus generic
> > > > description and "reg-names" with possible values enumerated seems very
> > > > suitable pattern in this case. Don't you think?
> >
> > No, I think this is just as concise and defines the order too:
> >
> > reg-names:
> > items:
> > - const: dbi
> > - const: dbi2
> > - const: config
> >
> > >
> > > In addition to that what about optional names? How would you suggest
> > > to handle such case without the non-ordered pattern?
> >
>
> > Sorry, I don't follow.
>
> I meant exactly the case you've described as the main goal of the
> named properties. My worry was that by using the pattern:
>
> reg-names:
> items:
> - const: name
> - const: another_name
> - const: one_more_name
>
> you get to fix the names order, which they were invented to get rid
> from. If you get to use that pattern the only optional names could be
> the names at the tail of the array, which isn't always applicable. In
> that case you'd have no choice but to use the pattern suggested by
> me.

For this binding, we use reg-names because the order and what's
present varies by platform. But for a given platform the order is
fixed.

Rob

2022-08-09 21:15:57

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v4 12/17] dt-bindings: PCI: dwc: Add Baikal-T1 PCIe Root Port bindings

On Tue, Aug 09, 2022 at 02:06:16PM -0600, Rob Herring wrote:
> On Tue, Aug 9, 2022 at 1:28 PM Serge Semin <[email protected]> wrote:
> >
> > On Tue, Aug 09, 2022 at 09:12:31AM -0600, Rob Herring wrote:
> > > On Mon, Aug 8, 2022 at 10:01 AM Serge Semin <[email protected]> wrote:
> > > >
> > > > On Mon, Aug 01, 2022 at 12:13:11PM -0600, Rob Herring wrote:
> > > > > On Thu, Jul 28, 2022 at 05:34:22PM +0300, Serge Semin wrote:
> > > > > > Baikal-T1 SoC is equipped with DWC PCIe v4.60a Root Port controller, which
> > > > > > link can be trained to work on up to Gen.3 speed over up to x4 lanes. The
> > > > > > controller is supposed to be fed up with four clock sources: DBI
> > > > > > peripheral clock, AXI application Tx/Rx clocks and external PHY/core
> > > > > > reference clock generating the 100MHz signal. In addition to that the
> > > > > > platform provide a way to reset each part of the controller:
> > > > > > sticky/non-sticky bits, host controller core, PIPE interface, PCS/PHY and
> > > > > > Hot/Power reset signal. The Root Port controller is equipped with multiple
> > > > > > IRQ lines like MSI, system AER, PME, HP, Bandwidth change, Link
> > > > > > equalization request and eDMA ones. The registers space is accessed over
> > > > > > the DBI interface. There can be no more than four inbound or outbound iATU
> > > > > > windows configured.
> > > > > >
> > > > > > Signed-off-by: Serge Semin <[email protected]>
> > > > > >
>
> [...]
>
> > > > > > + reg-names:
> > > > > > + minItems: 3
> > > > > > + maxItems: 3
> > > > > > + items:
> > > > > > + enum: [ dbi, dbi2, config ]
> > > > >
> > > >
> > > > > Define the order. Here, and the rest.
> > > >
> > > > Ok. I will, but please answer to my question, I asked you in the
> > > > previous email thread:
> > > >
> > > > Serge Semin wrote:
> > > > > Rob Herring wrote:
> > > > > > ...
> > > > > > Tell me why you need random order.
> > > > >
> > > > > Because I don't see a need in constraining the order. If we get to set
> > > > > the order requirement, then why do we need to have the "*-names"
> > > > > property at all?
> > >
> > > Originally, it was for cases where you have a variable number of
> > > entries and can't determine what each entry is. IOW, when you have
> > > optional entries in the middle of required entries. But then everyone
> > > *loves* -names even when not needed or useful such as 'phy-names =
> > > "pcie"' (the phy subsys requiring names was part of the problem there,
> > > but that's been fixed).
> > >
>
> > > > > IMO having "reg" with max/minItems restriction plus generic
> > > > > description and "reg-names" with possible values enumerated seems very
> > > > > suitable pattern in this case. Don't you think?
> > >
> > > No, I think this is just as concise and defines the order too:
> > >
> > > reg-names:
> > > items:
> > > - const: dbi
> > > - const: dbi2
> > > - const: config
> > >
> > > >
> > > > In addition to that what about optional names? How would you suggest
> > > > to handle such case without the non-ordered pattern?
> > >
> >
> > > Sorry, I don't follow.
> >
> > I meant exactly the case you've described as the main goal of the
> > named properties. My worry was that by using the pattern:
> >
> > reg-names:
> > items:
> > - const: name
> > - const: another_name
> > - const: one_more_name
> >
> > you get to fix the names order, which they were invented to get rid
> > from. If you get to use that pattern the only optional names could be
> > the names at the tail of the array, which isn't always applicable. In
> > that case you'd have no choice but to use the pattern suggested by
> > me.
>
> For this binding, we use reg-names because the order and what's
> present varies by platform. But for a given platform the order is
> fixed.

Got it. Thanks for your time and for the detailed case clarification.

-Sergey

>
> Rob