2022-04-23 19:01:09

by Peter Geis

[permalink] [raw]
Subject: [PATCH v8 0/5] Enable rk356x PCIe controller

This series enables the DesignWare based PCIe controller on the rk356x
series of chips.
We drop the fallback to the core driver due to compatibility issues.
We reset the PCIe controller at driver probe to prevent issues in the
future when firmware / kexec leaves the controller in an unknown state.
We add support for legacy interrupts for cards that lack MSI support
(which is partially broken currently).
We then add the device tree nodes to enable PCIe on the Quartz64 Model
A.

Patch 1 drops the snps,dw,pcie fallback from the dt-binding
Patch 2 resets the PCIe controller to prevent configuration bugs
Patch 3 adds legacy interrupt support to the driver
Patch 4 adds the device tree binding to the rk356x.dtsi
Patch 5 enables the PCIe controller on the Quartz64-A

Changelog:
v8:
- add core reset patch
- simplify irq enable/disable functions
- drop spinlock
- only enable/disable irq requested
- only pass the irq register bits used to irq functions

Changelog:
v7:
- drop assigned-clocks

v6:
- fix a ranges issue
- point to gic instead of its

v5:
- fix incorrect series (apologies for the v4 spam)

v4:
- drop the ITS modification, poor compatibility is better than
completely broken

v3:
- drop select node from dt-binding
- convert to for_each_set_bit
- convert to generic_handle_domain_irq
- drop unncessary dev_err
- reorder irq_chip items
- change to level_irq
- install the handler after initializing the domain

v2:
- Define PCIE_CLIENT_INTR_STATUS_LEGACY
- Fix PCIE_LEGACY_INT_ENABLE to only enable the RC interrupts
- Add legacy interrupt enable/disable support

Peter Geis (5):
dt-bindings: pci: remove fallback from Rockchip DesignWare binding
PCI: dwc: rockchip: reset core at driver probe
PCI: dwc: rockchip: add legacy interrupt support
arm64: dts: rockchip: add rk3568 pcie2x1 controller
arm64: dts: rockchip: enable pcie controller on quartz64-a

.../bindings/pci/rockchip-dw-pcie.yaml | 12 +-
.../boot/dts/rockchip/rk3566-quartz64-a.dts | 34 ++++++
arch/arm64/boot/dts/rockchip/rk356x.dtsi | 52 ++++++++
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 114 +++++++++++++++---
4 files changed, 185 insertions(+), 27 deletions(-)

--
2.25.1


2022-04-24 01:32:50

by Peter Geis

[permalink] [raw]
Subject: [PATCH v8 3/5] PCI: dwc: rockchip: add legacy interrupt support

The legacy interrupts on the rk356x pcie controller are handled by a
single muxed interrupt. Add irq domain support to the pcie-dw-rockchip
driver to support the virtual domain.

Signed-off-by: Peter Geis <[email protected]>
---
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 94 ++++++++++++++++++-
1 file changed, 91 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index d67ed811e752..b00832d653ea 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -10,9 +10,12 @@

#include <linux/clk.h>
#include <linux/gpio/consumer.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
#include <linux/mfd/syscon.h>
#include <linux/module.h>
#include <linux/of_device.h>
+#include <linux/of_irq.h>
#include <linux/phy/phy.h>
#include <linux/platform_device.h>
#include <linux/regmap.h>
@@ -26,6 +29,7 @@
*/
#define HIWORD_UPDATE(mask, val) (((mask) << 16) | (val))
#define HIWORD_UPDATE_BIT(val) HIWORD_UPDATE(val, val)
+#define HIWORD_DISABLE_BIT(val) HIWORD_UPDATE(val, ~val)

#define to_rockchip_pcie(x) dev_get_drvdata((x)->dev)

@@ -36,10 +40,12 @@
#define PCIE_LINKUP (PCIE_SMLH_LINKUP | PCIE_RDLH_LINKUP)
#define PCIE_L0S_ENTRY 0x11
#define PCIE_CLIENT_GENERAL_CONTROL 0x0
+#define PCIE_CLIENT_INTR_STATUS_LEGACY 0x8
+#define PCIE_CLIENT_INTR_MASK_LEGACY 0x1c
#define PCIE_CLIENT_GENERAL_DEBUG 0x104
-#define PCIE_CLIENT_HOT_RESET_CTRL 0x180
+#define PCIE_CLIENT_HOT_RESET_CTRL 0x180
#define PCIE_CLIENT_LTSSM_STATUS 0x300
-#define PCIE_LTSSM_ENABLE_ENHANCE BIT(4)
+#define PCIE_LTSSM_ENABLE_ENHANCE BIT(4)
#define PCIE_LTSSM_STATUS_MASK GENMASK(5, 0)

struct rockchip_pcie {
@@ -51,6 +57,7 @@ struct rockchip_pcie {
struct reset_control *rst;
struct gpio_desc *rst_gpio;
struct regulator *vpcie3v3;
+ struct irq_domain *irq_domain;
};

static int rockchip_pcie_readl_apb(struct rockchip_pcie *rockchip,
@@ -65,6 +72,76 @@ static void rockchip_pcie_writel_apb(struct rockchip_pcie *rockchip,
writel_relaxed(val, rockchip->apb_base + reg);
}

+static void rockchip_pcie_legacy_int_handler(struct irq_desc *desc)
+{
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ struct rockchip_pcie *rockchip = irq_desc_get_handler_data(desc);
+ unsigned long reg, hwirq;
+
+ chained_irq_enter(chip, desc);
+
+ reg = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_INTR_STATUS_LEGACY);
+
+ for_each_set_bit(hwirq, &reg, 4)
+ generic_handle_domain_irq(rockchip->irq_domain, hwirq);
+
+ chained_irq_exit(chip, desc);
+}
+
+static void rockchip_intx_mask(struct irq_data *data)
+{
+ rockchip_pcie_writel_apb(irq_data_get_irq_chip_data(data), HIWORD_UPDATE_BIT(BIT(data->hwirq)),
+ PCIE_CLIENT_INTR_MASK_LEGACY);
+};
+
+static void rockchip_intx_unmask(struct irq_data *data)
+{
+ rockchip_pcie_writel_apb(irq_data_get_irq_chip_data(data), HIWORD_DISABLE_BIT(BIT(data->hwirq)),
+ PCIE_CLIENT_INTR_MASK_LEGACY);
+};
+
+static struct irq_chip rockchip_intx_irq_chip = {
+ .name = "INTx",
+ .irq_mask = rockchip_intx_mask,
+ .irq_unmask = rockchip_intx_unmask,
+ .flags = IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND,
+};
+
+static int rockchip_pcie_intx_map(struct irq_domain *domain, unsigned int irq,
+ irq_hw_number_t hwirq)
+{
+ irq_set_chip_and_handler(irq, &rockchip_intx_irq_chip, handle_level_irq);
+ irq_set_chip_data(irq, domain->host_data);
+
+ return 0;
+}
+
+static const struct irq_domain_ops intx_domain_ops = {
+ .map = rockchip_pcie_intx_map,
+};
+
+static int rockchip_pcie_init_irq_domain(struct rockchip_pcie *rockchip)
+{
+ struct device *dev = rockchip->pci.dev;
+ struct device_node *intc;
+
+ intc = of_get_child_by_name(dev->of_node, "legacy-interrupt-controller");
+ if (!intc) {
+ dev_err(dev, "missing child interrupt-controller node\n");
+ return -EINVAL;
+ }
+
+ rockchip->irq_domain = irq_domain_add_linear(intc, PCI_NUM_INTX,
+ &intx_domain_ops, rockchip);
+ of_node_put(intc);
+ if (!rockchip->irq_domain) {
+ dev_err(dev, "failed to get a INTx IRQ domain\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static void rockchip_pcie_enable_ltssm(struct rockchip_pcie *rockchip)
{
rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_ENABLE_LTSSM,
@@ -111,7 +188,19 @@ static int rockchip_pcie_host_init(struct pcie_port *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
+ struct device *dev = rockchip->pci.dev;
u32 val = HIWORD_UPDATE_BIT(PCIE_LTSSM_ENABLE_ENHANCE);
+ int irq, ret;
+
+ irq = of_irq_get_byname(dev->of_node, "legacy");
+ if (irq < 0)
+ return irq;
+
+ ret = rockchip_pcie_init_irq_domain(rockchip);
+ if (ret < 0)
+ dev_err(dev, "failed to init irq domain\n");
+
+ irq_set_chained_handler_and_data(irq, rockchip_pcie_legacy_int_handler, rockchip);

/* LTSSM enable control mode */
rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL);
@@ -158,7 +247,6 @@ static int rockchip_pcie_resource_get(struct platform_device *pdev,
"failed to get reset lines\n");

return reset_control_assert(rockchip->rst);
-
}

static int rockchip_pcie_phy_init(struct rockchip_pcie *rockchip)
--
2.25.1

2022-04-24 06:22:15

by Peter Geis

[permalink] [raw]
Subject: [PATCH v8 4/5] arm64: dts: rockchip: add rk3568 pcie2x1 controller

The pcie2x1 controller is common between the rk3568 and rk3566. It is a
single lane pcie2 compliant controller.

Signed-off-by: Peter Geis <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk356x.dtsi | 52 ++++++++++++++++++++++++
1 file changed, 52 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index 7cdef800cb3c..aea5d9255235 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -689,6 +689,58 @@ qos_vop_m1: qos@fe1a8100 {
reg = <0x0 0xfe1a8100 0x0 0x20>;
};

+ pcie2x1: pcie@fe260000 {
+ compatible = "rockchip,rk3568-pcie";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ bus-range = <0x0 0xf>;
+ clocks = <&cru ACLK_PCIE20_MST>, <&cru ACLK_PCIE20_SLV>,
+ <&cru ACLK_PCIE20_DBI>, <&cru PCLK_PCIE20>,
+ <&cru CLK_PCIE20_AUX_NDFT>;
+ clock-names = "aclk_mst", "aclk_slv",
+ "aclk_dbi", "pclk", "aux";
+ device_type = "pci";
+ interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "sys", "pmc", "msi", "legacy", "err";
+ #interrupt-cells = <1>;
+ interrupt-map-mask = <0 0 0 7>;
+ interrupt-map = <0 0 0 1 &pcie_intc 0>,
+ <0 0 0 2 &pcie_intc 1>,
+ <0 0 0 3 &pcie_intc 2>,
+ <0 0 0 4 &pcie_intc 3>;
+ linux,pci-domain = <0>;
+ num-ib-windows = <6>;
+ num-ob-windows = <2>;
+ max-link-speed = <2>;
+ msi-map = <0x0 &gic 0x0 0x1000>;
+ num-lanes = <1>;
+ phys = <&combphy2 PHY_TYPE_PCIE>;
+ phy-names = "pcie-phy";
+ power-domains = <&power RK3568_PD_PIPE>;
+ reg = <0x3 0xc0000000 0x0 0x00400000>,
+ <0x0 0xfe260000 0x0 0x00010000>,
+ <0x3 0x00000000 0x0 0x01000000>;
+ ranges = <0x01000000 0x0 0x01000000 0x3 0x01000000 0x0 0x00100000
+ 0x02000000 0x0 0x02000000 0x3 0x01100000 0x0 0x3ef00000>;
+ reg-names = "dbi", "apb", "config";
+ resets = <&cru SRST_PCIE20_POWERUP>;
+ reset-names = "pipe";
+ status = "disabled";
+
+ pcie_intc: legacy-interrupt-controller {
+ #address-cells = <0>;
+ #interrupt-cells = <1>;
+ interrupt-controller;
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_SPI 72 IRQ_TYPE_EDGE_RISING>;
+ };
+
+ };
+
sdmmc0: mmc@fe2b0000 {
compatible = "rockchip,rk3568-dw-mshc", "rockchip,rk3288-dw-mshc";
reg = <0x0 0xfe2b0000 0x0 0x4000>;
--
2.25.1

2022-04-24 13:56:06

by Peter Geis

[permalink] [raw]
Subject: [PATCH v8 1/5] dt-bindings: pci: remove fallback from Rockchip DesignWare binding

The snps,dw-pcie binds to a standalone driver.
It is not fully compatible with the Rockchip implementation and causes a
hang if it binds to the device.

Remove this binding as a valid fallback.

Signed-off-by: Peter Geis <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
.../devicetree/bindings/pci/rockchip-dw-pcie.yaml | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
index 142bbe577763..bc0a9d1db750 100644
--- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
@@ -19,20 +19,10 @@ description: |+
allOf:
- $ref: /schemas/pci/pci-bus.yaml#

-# We need a select here so we don't match all nodes with 'snps,dw-pcie'
-select:
- properties:
- compatible:
- contains:
- const: rockchip,rk3568-pcie
- required:
- - compatible
-
properties:
compatible:
items:
- const: rockchip,rk3568-pcie
- - const: snps,dw-pcie

reg:
items:
@@ -110,7 +100,7 @@ examples:
#size-cells = <2>;

pcie3x2: pcie@fe280000 {
- compatible = "rockchip,rk3568-pcie", "snps,dw-pcie";
+ compatible = "rockchip,rk3568-pcie";
reg = <0x3 0xc0800000 0x0 0x390000>,
<0x0 0xfe280000 0x0 0x10000>,
<0x3 0x80000000 0x0 0x100000>;
--
2.25.1

2022-04-25 07:51:56

by Peter Geis

[permalink] [raw]
Subject: [PATCH v8 2/5] PCI: dwc: rockchip: reset core at driver probe

The PCIe controller is in an unknown state at driver probe. This can
lead to undesireable effects when the driver attempts to configure the
controller.

Prevent issues in the future by resetting the core during probe.

Signed-off-by: Peter Geis <[email protected]>
---
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 22 +++++++------------
1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index c9b341e55cbb..d67ed811e752 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -152,7 +152,13 @@ static int rockchip_pcie_resource_get(struct platform_device *pdev,
if (IS_ERR(rockchip->rst_gpio))
return PTR_ERR(rockchip->rst_gpio);

- return 0;
+ rockchip->rst = devm_reset_control_array_get_exclusive(&pdev->dev);
+ if (IS_ERR(rockchip->rst))
+ return dev_err_probe(&pdev->dev, PTR_ERR(rockchip->rst),
+ "failed to get reset lines\n");
+
+ return reset_control_assert(rockchip->rst);
+
}

static int rockchip_pcie_phy_init(struct rockchip_pcie *rockchip)
@@ -182,18 +188,6 @@ static void rockchip_pcie_phy_deinit(struct rockchip_pcie *rockchip)
phy_power_off(rockchip->phy);
}

-static int rockchip_pcie_reset_control_release(struct rockchip_pcie *rockchip)
-{
- struct device *dev = rockchip->pci.dev;
-
- rockchip->rst = devm_reset_control_array_get_exclusive(dev);
- if (IS_ERR(rockchip->rst))
- return dev_err_probe(dev, PTR_ERR(rockchip->rst),
- "failed to get reset lines\n");
-
- return reset_control_deassert(rockchip->rst);
-}
-
static const struct dw_pcie_ops dw_pcie_ops = {
.link_up = rockchip_pcie_link_up,
.start_link = rockchip_pcie_start_link,
@@ -241,7 +235,7 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
if (ret)
goto disable_regulator;

- ret = rockchip_pcie_reset_control_release(rockchip);
+ ret = reset_control_deassert(rockchip->rst);
if (ret)
goto deinit_phy;

--
2.25.1

2022-04-25 09:27:53

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v8 0/5] Enable rk356x PCIe controller

Looks like your cover letter lacked a "To:" line, which breaks group
reply, at least for mutt.

On Sat, Apr 23, 2022 at 11:23:58AM -0400, Peter Geis wrote:
> This series enables the DesignWare based PCIe controller on the rk356x
> series of chips.
> We drop the fallback to the core driver due to compatibility issues.
> We reset the PCIe controller at driver probe to prevent issues in the
> future when firmware / kexec leaves the controller in an unknown state.
> We add support for legacy interrupts for cards that lack MSI support
> (which is partially broken currently).
> We then add the device tree nodes to enable PCIe on the Quartz64 Model
> A.
>
> Patch 1 drops the snps,dw,pcie fallback from the dt-binding
> Patch 2 resets the PCIe controller to prevent configuration bugs
> Patch 3 adds legacy interrupt support to the driver
> Patch 4 adds the device tree binding to the rk356x.dtsi
> Patch 5 enables the PCIe controller on the Quartz64-A
>
> Changelog:
> v8:
> - add core reset patch
> - simplify irq enable/disable functions
> - drop spinlock
> - only enable/disable irq requested
> - only pass the irq register bits used to irq functions
>
> Changelog:
> v7:
> - drop assigned-clocks
>
> v6:
> - fix a ranges issue
> - point to gic instead of its
>
> v5:
> - fix incorrect series (apologies for the v4 spam)
>
> v4:
> - drop the ITS modification, poor compatibility is better than
> completely broken
>
> v3:
> - drop select node from dt-binding
> - convert to for_each_set_bit
> - convert to generic_handle_domain_irq
> - drop unncessary dev_err
> - reorder irq_chip items
> - change to level_irq
> - install the handler after initializing the domain
>
> v2:
> - Define PCIE_CLIENT_INTR_STATUS_LEGACY
> - Fix PCIE_LEGACY_INT_ENABLE to only enable the RC interrupts
> - Add legacy interrupt enable/disable support
>
> Peter Geis (5):
> dt-bindings: pci: remove fallback from Rockchip DesignWare binding
> PCI: dwc: rockchip: reset core at driver probe
> PCI: dwc: rockchip: add legacy interrupt support
> arm64: dts: rockchip: add rk3568 pcie2x1 controller
> arm64: dts: rockchip: enable pcie controller on quartz64-a

Please make your subjects and commit logs match previous history:

PCI: fu740: Remove unused assignments
PCI: kirin: Remove unused assignments
PCI: fu740: Force 2.5GT/s for initial device probe
PCI: imx6: Assert i.MX8MM CLKREQ# even if no device present
PCI: imx6: Invoke the PHY exit function after PHY power off
PCI: dwc: Restore MSI Receiver mask during resume
PCI: fu740: Drop redundant '-gpios' from DT GPIO lookup
PCI: imx6: Enable i.MX6QP PCIe power management support
PCI: qcom: Add SM8450 PCIe support
PCI: qcom: Add ddrss_sf_tbu flag
PCI: qcom: Remove redundancy between qcom_pcie and qcom_pcie_cfg

No "dwc:" (no need to include all path elements; "dwc" isn't relevant
unless changing the dwc core itself). Capitalize first word after the
driver name ("Reset", "Add").

Wrap commit logs to fill 75 columns.

Use blank lines to separate paragraphs.

In subjects, commit logs, comments, log messages, etc:

s/pcie/PCIe/
s/irq/IRQ/

Wrap code to fit in 80 columns to match the rest of the file (except
things like printk strings where it would reduce greppability).

> .../bindings/pci/rockchip-dw-pcie.yaml | 12 +-
> .../boot/dts/rockchip/rk3566-quartz64-a.dts | 34 ++++++
> arch/arm64/boot/dts/rockchip/rk356x.dtsi | 52 ++++++++
> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 114 +++++++++++++++---
> 4 files changed, 185 insertions(+), 27 deletions(-)
>
> --
> 2.25.1
>

2022-04-25 17:58:40

by Philipp Zabel

[permalink] [raw]
Subject: Re: [PATCH v8 2/5] PCI: dwc: rockchip: reset core at driver probe

Hi Peter,

On Sa, 2022-04-23 at 11:24 -0400, Peter Geis wrote:
> The PCIe controller is in an unknown state at driver probe. This can
> lead to undesireable effects when the driver attempts to configure the
> controller.
>
> Prevent issues in the future by resetting the core during probe.
>
> Signed-off-by: Peter Geis <[email protected]>
> ---
>  drivers/pci/controller/dwc/pcie-dw-rockchip.c | 22 +++++++------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index c9b341e55cbb..d67ed811e752 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -152,7 +152,13 @@ static int rockchip_pcie_resource_get(struct platform_device *pdev,
>   if (IS_ERR(rockchip->rst_gpio))
>   return PTR_ERR(rockchip->rst_gpio);
>
> - return 0;
> + rockchip->rst = devm_reset_control_array_get_exclusive(&pdev->dev);
> + if (IS_ERR(rockchip->rst))
> + return dev_err_probe(&pdev->dev, PTR_ERR(rockchip->rst),
> + "failed to get reset lines\n");
> +
> + return reset_control_assert(rockchip->rst);

This makes "rockchip_pcie_resource_get" a bit of a misnomer, maybe move
this out into rockchip_pcie_probe()?

> +

Superfluous whitespace.

regards
Philipp

2022-04-26 22:59:06

by Peter Geis

[permalink] [raw]
Subject: Re: [PATCH v8 2/5] PCI: dwc: rockchip: reset core at driver probe

On Mon, Apr 25, 2022 at 5:20 AM Philipp Zabel <[email protected]> wrote:
>
> Hi Peter,
>
> On Sa, 2022-04-23 at 11:24 -0400, Peter Geis wrote:
> > The PCIe controller is in an unknown state at driver probe. This can
> > lead to undesireable effects when the driver attempts to configure the
> > controller.
> >
> > Prevent issues in the future by resetting the core during probe.
> >
> > Signed-off-by: Peter Geis <[email protected]>
> > ---
> > drivers/pci/controller/dwc/pcie-dw-rockchip.c | 22 +++++++------------
> > 1 file changed, 8 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > index c9b341e55cbb..d67ed811e752 100644
> > --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > @@ -152,7 +152,13 @@ static int rockchip_pcie_resource_get(struct platform_device *pdev,
> > if (IS_ERR(rockchip->rst_gpio))
> > return PTR_ERR(rockchip->rst_gpio);
> >
> > - return 0;
> > + rockchip->rst = devm_reset_control_array_get_exclusive(&pdev->dev);
> > + if (IS_ERR(rockchip->rst))
> > + return dev_err_probe(&pdev->dev, PTR_ERR(rockchip->rst),
> > + "failed to get reset lines\n");
> > +
> > + return reset_control_assert(rockchip->rst);
>
> This makes "rockchip_pcie_resource_get" a bit of a misnomer, maybe move
> this out into rockchip_pcie_probe()?

Ah, yes that does make sense, thanks.

>
> > +
>
> Superfluous whitespace.
>
> regards
> Philipp
>

2022-04-27 10:04:22

by Nicolas Frattaroli

[permalink] [raw]
Subject: Re: [PATCH v8 0/5] Enable rk356x PCIe controller

On Samstag, 23. April 2022 17:23:58 CEST Peter Geis wrote:
> This series enables the DesignWare based PCIe controller on the rk356x
> series of chips.
> We drop the fallback to the core driver due to compatibility issues.
> We reset the PCIe controller at driver probe to prevent issues in the
> future when firmware / kexec leaves the controller in an unknown state.
> We add support for legacy interrupts for cards that lack MSI support
> (which is partially broken currently).
> We then add the device tree nodes to enable PCIe on the Quartz64 Model
> A.

Tested-by: Nicolas Frattaroli <[email protected]>

Tested on a PINE64 Quartz64 Model A. The series was applied to 5.18-rc4,
and two devices were tested:

Device #1: ASMedia Technology Inc. ASM1142 USB 3.1 Host Controller

A USB 3.1 flash drive was plugged into the PCIe USB controller card.
Then, the block device was read. Performance was nominal, no errors
showed up in dmesg.

Device #2: NEC Corporation uPD720200 USB 3.0 Host Controller (rev 03)
behind a PLX Technology, Inc. PEX 8608 8-lane, 8-Port PCI Express Gen 2
(5.0 GT/s) Switch (rev ba) PCIe switch.

(it's a weird card I grabbed off an auction site with both USB and SATA
behind a PCIe switch, it's best not to worry about the twisted mind
that came up with it.)

A USB 3.1 flash drive was plugged into the PCIe controller card's
USB 3.0 port. Then, the block device was read. Performance was nominal,
no errors appeared in dmesg.

512 megabytes of /dev/urandom were redirected into a file. The file was
SHA1 checksummed. The file was then copied onto the mounted USB 3.1 drive
which was connected to the PCIe card. The drive was unmounted, then
re-mounted, and then a sha1sum of the file on the drive was calculated.
The checksums matched.

Based on these tests it is my understanding that this patch series is
functional for the use cases I have covered.

Regards,
Nicolas Frattaroli