linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] mips: bmips: add BCM6328 PCIe support
@ 2020-06-17 10:25 Álvaro Fernández Rojas
  2020-06-17 10:25 ` [PATCH 1/3] mips: bmips: add PCI support Álvaro Fernández Rojas
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Álvaro Fernández Rojas @ 2020-06-17 10:25 UTC (permalink / raw)
  To: bhelgaas, robh+dt, tsbogend, lorenzo.pieralisi, p.zabel,
	jiaxun.yang, paulburton, info, allison, kstewart, tglx,
	jonas.gorski, f.fainelli, bcm-kernel-feedback-list, linux-pci,
	devicetree, linux-kernel, linux-mips
  Cc: Álvaro Fernández Rojas

BCM6328 PCIe host controller is found on BCM6328, BCM6362 and BCM63268 SoCs.

Álvaro Fernández Rojas (3):
  mips: bmips: add PCI support
  dt-bindings: Document BCM6328 PCIe Host Controller
  pci: add BCM6328 PCIe controller support

 .../bindings/pci/brcm,bcm6328-pcie.yaml       | 109 ++++++
 arch/mips/Kconfig                             |   1 +
 arch/mips/pci/Makefile                        |   1 +
 arch/mips/pci/fixup-bmips.c                   |  17 +
 drivers/pci/controller/Kconfig                |   8 +
 drivers/pci/controller/Makefile               |   1 +
 drivers/pci/controller/pcie-bcm6328.c         | 346 ++++++++++++++++++
 7 files changed, 483 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/brcm,bcm6328-pcie.yaml
 create mode 100644 arch/mips/pci/fixup-bmips.c
 create mode 100644 drivers/pci/controller/pcie-bcm6328.c

-- 
2.27.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] mips: bmips: add PCI support
  2020-06-17 10:25 [PATCH 0/3] mips: bmips: add BCM6328 PCIe support Álvaro Fernández Rojas
@ 2020-06-17 10:25 ` Álvaro Fernández Rojas
  2020-06-17 11:09   ` Thomas Bogendoerfer
  2020-06-17 10:25 ` [PATCH 2/3] dt-bindings: Document BCM6328 PCIe Host Controller Álvaro Fernández Rojas
  2020-06-17 10:25 ` [PATCH 3/3] pci: add BCM6328 PCIe controller support Álvaro Fernández Rojas
  2 siblings, 1 reply; 8+ messages in thread
From: Álvaro Fernández Rojas @ 2020-06-17 10:25 UTC (permalink / raw)
  To: bhelgaas, robh+dt, tsbogend, lorenzo.pieralisi, p.zabel,
	jiaxun.yang, paulburton, info, allison, kstewart, tglx,
	jonas.gorski, f.fainelli, bcm-kernel-feedback-list, linux-pci,
	devicetree, linux-kernel, linux-mips
  Cc: Álvaro Fernández Rojas

BMIPS SoCs with PCI: BCM6358, BCM6368.
BMIPS SoCs with PCIe (gen1): BCM6328, BCM6362, BCM63268.
BMIPS SoCs with PCIe (gen2): BCM6318.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 arch/mips/Kconfig           |  1 +
 arch/mips/pci/Makefile      |  1 +
 arch/mips/pci/fixup-bmips.c | 17 +++++++++++++++++
 3 files changed, 19 insertions(+)
 create mode 100644 arch/mips/pci/fixup-bmips.c

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 6fee1a133e9d..357026cb51de 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -230,6 +230,7 @@ config BMIPS_GENERIC
 	select ARCH_HAS_SYNC_DMA_FOR_CPU_ALL
 	select ARCH_HAS_PHYS_TO_DMA
 	select BOOT_RAW
+	select HAVE_PCI
 	select NO_EXCEPT_FILL
 	select USE_OF
 	select CEVT_R4K
diff --git a/arch/mips/pci/Makefile b/arch/mips/pci/Makefile
index 0f68d6849978..e38285c10d45 100644
--- a/arch/mips/pci/Makefile
+++ b/arch/mips/pci/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_PCI_XTALK_BRIDGE)	+= pci-xtalk-bridge.o
 # These are still pretty much in the old state, watch, go blind.
 #
 obj-$(CONFIG_ATH79)		+= fixup-ath79.o
+obj-$(CONFIG_BMIPS_GENERIC)	+= fixup-bmips.o
 obj-$(CONFIG_MIPS_COBALT)	+= fixup-cobalt.o
 obj-$(CONFIG_LEMOTE_FULOONG2E)	+= fixup-fuloong2e.o ops-loongson2.o
 obj-$(CONFIG_LEMOTE_MACH2F)	+= fixup-lemote2f.o ops-loongson2.o
diff --git a/arch/mips/pci/fixup-bmips.c b/arch/mips/pci/fixup-bmips.c
new file mode 100644
index 000000000000..581cff562ead
--- /dev/null
+++ b/arch/mips/pci/fixup-bmips.c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *  Copyright (C) 2020 Álvaro Fernández Rojas <noltari@gmail.com>
+ */
+
+#include <linux/of_pci.h>
+#include <linux/pci.h>
+
+int pcibios_plat_dev_init(struct pci_dev *dev)
+{
+	return PCIBIOS_SUCCESSFUL;
+}
+
+int pcibios_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
+{
+	return of_irq_parse_and_map_pci(dev, slot, pin);
+}
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] dt-bindings: Document BCM6328 PCIe Host Controller
  2020-06-17 10:25 [PATCH 0/3] mips: bmips: add BCM6328 PCIe support Álvaro Fernández Rojas
  2020-06-17 10:25 ` [PATCH 1/3] mips: bmips: add PCI support Álvaro Fernández Rojas
@ 2020-06-17 10:25 ` Álvaro Fernández Rojas
  2020-06-17 17:07   ` Rob Herring
  2020-06-17 21:50   ` Florian Fainelli
  2020-06-17 10:25 ` [PATCH 3/3] pci: add BCM6328 PCIe controller support Álvaro Fernández Rojas
  2 siblings, 2 replies; 8+ messages in thread
From: Álvaro Fernández Rojas @ 2020-06-17 10:25 UTC (permalink / raw)
  To: bhelgaas, robh+dt, tsbogend, lorenzo.pieralisi, p.zabel,
	jiaxun.yang, paulburton, info, allison, kstewart, tglx,
	jonas.gorski, f.fainelli, bcm-kernel-feedback-list, linux-pci,
	devicetree, linux-kernel, linux-mips
  Cc: Álvaro Fernández Rojas

BCM6328 PCIe host controller is found on BCM6328, BCM6362 and BCM63268 SoCs.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 .../bindings/pci/brcm,bcm6328-pcie.yaml       | 109 ++++++++++++++++++
 1 file changed, 109 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/brcm,bcm6328-pcie.yaml

diff --git a/Documentation/devicetree/bindings/pci/brcm,bcm6328-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,bcm6328-pcie.yaml
new file mode 100644
index 000000000000..d2bd4933a5fa
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/brcm,bcm6328-pcie.yaml
@@ -0,0 +1,109 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/brcm,bcm6328-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: BCM6328 PCIe Host Controller Device Tree Bindings
+
+maintainers:
+  - Álvaro Fernández Rojas <noltari@gmail.com>
+
+allOf:
+  - $ref: /schemas/pci/pci-bus.yaml#
+
+properties:
+  compatible:
+    enum:
+     - brcm,bcm6328-pcie
+     - brcm,bcm6362-pcie
+     - brcm,bcm63268-pcie
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: pcie
+
+  "#interrupt-cells":
+    const: 1
+
+  interrupt-map-mask:
+    maxItems: 1
+
+  interrupt-map:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  ranges:
+    maxItems: 1
+
+  reg:
+    maxItems: 1
+
+  resets:
+    maxItems: 4
+
+  reset-names:
+    items:
+      - const: pcie
+      - const: pcie-core
+      - const: pcie-ext
+      - const: pcie-hard
+
+required:
+  - brcm,serdes
+  - clocks
+  - clock-names
+  - "#interrupt-cells"
+  - interrupt-map-mask
+  - interrupt-map
+  - ranges
+  - reg
+  - resets
+  - reset-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    serdes_cntl: syscon@10001800 {
+      compatible = "syscon";
+      reg = <0x10001800 0x4>;
+      native-endian;
+    };
+
+    pcie: pcie@10e40000 {
+      compatible = "brcm,bcm6328-pcie";
+      reg = <0x10e40000 0x10000>;
+      #address-cells = <3>;
+      #size-cells = <2>;
+
+      device_type = "pci";
+      bus-range = <0x00 0x01>;
+      ranges = <0x2000000 0 0x10f00000 0x10f00000 0 0x100000>;
+      linux,pci-probe-only = <1>;
+
+      #interrupt-cells = <1>;
+      interrupt-map-mask = <0 0 0 0>;
+      interrupt-map = <0 0 0 0 &periph_intc BCM6328_IRQ_PCIE_RC 0>;
+
+      clocks = <&periph_clk BCM6328_CLK_PCIE>;
+      clock-names = "pcie";
+
+      resets = <&periph_rst BCM6328_RST_PCIE>,
+               <&periph_rst BCM6328_RST_PCIE_EXT>,
+               <&periph_rst BCM6328_RST_PCIE_CORE>,
+               <&periph_rst BCM6328_RST_PCIE_HARD>;
+      reset-names = "pcie",
+                    "pcie-ext",
+                    "pcie-core",
+                    "pcie-hard";
+
+      power-domains = <&periph_pwr BCM6328_POWER_DOMAIN_PCIE>;
+
+      brcm,serdes = <&serdes_cntl>;
+    };
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] pci: add BCM6328 PCIe controller support
  2020-06-17 10:25 [PATCH 0/3] mips: bmips: add BCM6328 PCIe support Álvaro Fernández Rojas
  2020-06-17 10:25 ` [PATCH 1/3] mips: bmips: add PCI support Álvaro Fernández Rojas
  2020-06-17 10:25 ` [PATCH 2/3] dt-bindings: Document BCM6328 PCIe Host Controller Álvaro Fernández Rojas
@ 2020-06-17 10:25 ` Álvaro Fernández Rojas
  2020-06-17 15:50   ` Bjorn Helgaas
  2 siblings, 1 reply; 8+ messages in thread
From: Álvaro Fernández Rojas @ 2020-06-17 10:25 UTC (permalink / raw)
  To: bhelgaas, robh+dt, tsbogend, lorenzo.pieralisi, p.zabel,
	jiaxun.yang, paulburton, info, allison, kstewart, tglx,
	jonas.gorski, f.fainelli, bcm-kernel-feedback-list, linux-pci,
	devicetree, linux-kernel, linux-mips
  Cc: Álvaro Fernández Rojas

BCM6328 PCIe host controller is found on BCM6328, BCM6362 and BCM63268 SoCs.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/pci/controller/Kconfig        |   8 +
 drivers/pci/controller/Makefile       |   1 +
 drivers/pci/controller/pcie-bcm6328.c | 346 ++++++++++++++++++++++++++
 3 files changed, 355 insertions(+)
 create mode 100644 drivers/pci/controller/pcie-bcm6328.c

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index adddf21fa381..7e238c04764e 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -3,6 +3,14 @@
 menu "PCI controller drivers"
 	depends on PCI
 
+config PCIE_BCM6328
+	bool "BCM6328 PCIe controller"
+	depends on BMIPS_GENERIC || COMPILE_TEST
+	depends on OF
+	help
+	  Say Y here if you want support for the PCIe host controller found
+	  on BCM6328, BCM6362 and BCM63268 SoCs.
+
 config PCI_MVEBU
 	bool "Marvell EBU PCIe controller"
 	depends on ARCH_MVEBU || ARCH_DOVE || COMPILE_TEST
diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
index efd9733ead26..1c3e82575845 100644
--- a/drivers/pci/controller/Makefile
+++ b/drivers/pci/controller/Makefile
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_PCIE_BCM6328) += pcie-bcm6328.o
 obj-$(CONFIG_PCIE_CADENCE) += cadence/
 obj-$(CONFIG_PCI_FTPCI100) += pci-ftpci100.o
 obj-$(CONFIG_PCI_HYPERV) += pci-hyperv.o
diff --git a/drivers/pci/controller/pcie-bcm6328.c b/drivers/pci/controller/pcie-bcm6328.c
new file mode 100644
index 000000000000..5bd86b166336
--- /dev/null
+++ b/drivers/pci/controller/pcie-bcm6328.c
@@ -0,0 +1,346 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * BCM6328 PCIe Controller Driver
+ *
+ * Copyright (C) 2020 Álvaro Fernández Rojas <noltari@gmail.com>
+ * Copyright (C) 2015 Jonas Gorski <jonas.gorski@gmail.com>
+ * Copyright (C) 2008 Maxime Bizon <mbizon@freebox.fr>
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_pci.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+
+#include "../pci.h"
+
+#define SERDES_PCIE_EN			BIT(0)
+#define SERDES_PCIE_EXD_EN		BIT(15)
+
+#define PCIE_BUS_BRIDGE			0
+#define PCIE_BUS_DEVICE			1
+
+#define PCIE_CONFIG2_REG		0x408
+#define CONFIG2_BAR1_SIZE_EN		1
+#define CONFIG2_BAR1_SIZE_MASK		0xf
+
+#define PCIE_IDVAL3_REG			0x43c
+#define IDVAL3_CLASS_CODE_MASK		0xffffff
+#define IDVAL3_SUBCLASS_SHIFT		8
+#define IDVAL3_CLASS_SHIFT		16
+
+#define PCIE_DLSTATUS_REG		0x1048
+#define DLSTATUS_PHYLINKUP		BIT(13)
+
+#define PCIE_BRIDGE_OPT1_REG		0x2820
+#define OPT1_RD_BE_OPT_EN		BIT(7)
+#define OPT1_RD_REPLY_BE_FIX_EN		BIT(9)
+#define OPT1_PCIE_BRIDGE_HOLE_DET_EN	BIT(11)
+#define OPT1_L1_INT_STATUS_MASK_POL	BIT(12)
+
+#define PCIE_BRIDGE_OPT2_REG		0x2824
+#define OPT2_UBUS_UR_DECODE_DIS		BIT(2)
+#define OPT2_TX_CREDIT_CHK_EN		BIT(4)
+#define OPT2_CFG_TYPE1_BD_SEL		BIT(7)
+#define OPT2_CFG_TYPE1_BUS_NO_SHIFT	16
+#define OPT2_CFG_TYPE1_BUS_NO_MASK	(0xff << OPT2_CFG_TYPE1_BUS_NO_SHIFT)
+
+#define PCIE_BRIDGE_BAR0_BASEMASK_REG	0x2828
+#define BASEMASK_REMAP_EN		BIT(0)
+#define BASEMASK_SWAP_EN		BIT(1)
+#define BASEMASK_MASK_SHIFT		4
+#define BASEMASK_MASK_MASK		(0xfff << BASEMASK_MASK_SHIFT)
+#define BASEMASK_BASE_SHIFT		20
+#define BASEMASK_BASE_MASK		(0xfff << BASEMASK_BASE_SHIFT)
+
+#define PCIE_BRIDGE_BAR0_REBASE_ADDR_REG 0x282c
+#define REBASE_ADDR_BASE_SHIFT		20
+#define REBASE_ADDR_BASE_MASK		(0xfff << REBASE_ADDR_BASE_SHIFT)
+
+#define PCIE_BRIDGE_RC_INT_MASK_REG	0x2854
+#define PCIE_RC_INT_A			BIT(0)
+#define PCIE_RC_INT_B			BIT(1)
+#define PCIE_RC_INT_C			BIT(2)
+#define PCIE_RC_INT_D			BIT(3)
+
+#define PCIE_DEVICE_OFFSET		0x8000
+
+struct bcm6328_pcie {
+	void __iomem *base;
+	struct regmap *serdes;
+	struct clk *clk;
+	struct reset_control *reset;
+	struct reset_control *reset_ext;
+	struct reset_control *reset_core;
+	struct reset_control *reset_hard;
+};
+
+static struct bcm6328_pcie bcm6328_pcie;
+
+/*
+ * swizzle 32bits data to return only the needed part
+ */
+static int postprocess_read(u32 data, int where, unsigned int size)
+{
+	u32 ret = 0;
+
+	switch (size) {
+	case 1:
+		ret = (data >> ((where & 3) << 3)) & 0xff;
+		break;
+	case 2:
+		ret = (data >> ((where & 3) << 3)) & 0xffff;
+		break;
+	case 4:
+		ret = data;
+		break;
+	}
+
+	return ret;
+}
+
+static int preprocess_write(u32 orig_data, u32 val, int where,
+			    unsigned int size)
+{
+	u32 ret = 0;
+
+	switch (size) {
+	case 1:
+		ret = (orig_data & ~(0xff << ((where & 3) << 3))) |
+		      (val << ((where & 3) << 3));
+		break;
+	case 2:
+		ret = (orig_data & ~(0xffff << ((where & 3) << 3))) |
+		      (val << ((where & 3) << 3));
+		break;
+	case 4:
+		ret = val;
+		break;
+	}
+
+	return ret;
+}
+
+static int bcm6328_pcie_can_access(struct pci_bus *bus, int devfn)
+{
+	struct bcm6328_pcie *priv = &bcm6328_pcie;
+
+	switch (bus->number) {
+	case PCIE_BUS_BRIDGE:
+		return PCI_SLOT(devfn) == 0;
+	case PCIE_BUS_DEVICE:
+		if (PCI_SLOT(devfn) == 0)
+			return __raw_readl(priv->base + PCIE_DLSTATUS_REG)
+			       & DLSTATUS_PHYLINKUP;
+		/* fall through */
+	default:
+		return false;
+	}
+}
+
+static int bcm6328_pcie_read(struct pci_bus *bus, unsigned int devfn,
+			     int where, int size, u32 *val)
+{
+	struct bcm6328_pcie *priv = &bcm6328_pcie;
+	u32 data;
+	u32 reg = where & ~3;
+
+	if (!bcm6328_pcie_can_access(bus, devfn))
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	if (bus->number == PCIE_BUS_DEVICE)
+		reg += PCIE_DEVICE_OFFSET;
+
+	data = __raw_readl(priv->base + reg);
+	*val = postprocess_read(data, where, size);
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static int bcm6328_pcie_write(struct pci_bus *bus, unsigned int devfn,
+			      int where, int size, u32 val)
+{
+	struct bcm6328_pcie *priv = &bcm6328_pcie;
+	u32 data;
+	u32 reg = where & ~3;
+
+	if (!bcm6328_pcie_can_access(bus, devfn))
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	if (bus->number == PCIE_BUS_DEVICE)
+		reg += PCIE_DEVICE_OFFSET;
+
+	data = __raw_readl(priv->base + reg);
+	data = preprocess_write(data, val, where, size);
+	__raw_writel(data, priv->base + reg);
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static struct pci_ops bcm6328_pcie_ops = {
+	.read = bcm6328_pcie_read,
+	.write = bcm6328_pcie_write,
+};
+
+static struct resource bcm6328_pcie_io_resource;
+static struct resource bcm6328_pcie_mem_resource;
+static struct resource bcm6328_pcie_busn_resource;
+
+static struct pci_controller bcm6328_pcie_controller = {
+	.pci_ops = &bcm6328_pcie_ops,
+	.io_resource = &bcm6328_pcie_io_resource,
+	.mem_resource = &bcm6328_pcie_mem_resource,
+	.busn_resource = &bcm6328_pcie_busn_resource,
+};
+
+static void bcm6328_pcie_reset(struct bcm6328_pcie *priv)
+{
+	regmap_write_bits(priv->serdes, 0,
+			  SERDES_PCIE_EN | SERDES_PCIE_EXD_EN,
+			  SERDES_PCIE_EN | SERDES_PCIE_EXD_EN);
+
+	reset_control_assert(priv->reset);
+	reset_control_assert(priv->reset_core);
+	reset_control_assert(priv->reset_ext);
+	if (priv->reset_hard) {
+		reset_control_assert(priv->reset_hard);
+		mdelay(10);
+		reset_control_deassert(priv->reset_hard);
+	}
+	mdelay(10);
+
+	reset_control_deassert(priv->reset_core);
+	reset_control_deassert(priv->reset);
+	mdelay(10);
+
+	reset_control_deassert(priv->reset_ext);
+	mdelay(200);
+}
+
+static void bcm6328_pcie_setup(struct bcm6328_pcie *priv)
+{
+	u32 val;
+
+	val = __raw_readl(priv->base + PCIE_BRIDGE_OPT1_REG);
+	val |= OPT1_RD_BE_OPT_EN;
+	val |= OPT1_RD_REPLY_BE_FIX_EN;
+	val |= OPT1_PCIE_BRIDGE_HOLE_DET_EN;
+	val |= OPT1_L1_INT_STATUS_MASK_POL;
+	__raw_writel(val, priv->base + PCIE_BRIDGE_OPT1_REG);
+
+	val = __raw_readl(priv->base + PCIE_BRIDGE_RC_INT_MASK_REG);
+	val |= PCIE_RC_INT_A;
+	val |= PCIE_RC_INT_B;
+	val |= PCIE_RC_INT_C;
+	val |= PCIE_RC_INT_D;
+	__raw_writel(val, priv->base + PCIE_BRIDGE_RC_INT_MASK_REG);
+
+	val = __raw_readl(priv->base + PCIE_BRIDGE_OPT2_REG);
+	/* enable credit checking and error checking */
+	val |= OPT2_TX_CREDIT_CHK_EN;
+	val |= OPT2_UBUS_UR_DECODE_DIS;
+	/* set device bus/func for the pcie device */
+	val |= (PCIE_BUS_DEVICE << OPT2_CFG_TYPE1_BUS_NO_SHIFT);
+	val |= OPT2_CFG_TYPE1_BD_SEL;
+	__raw_writel(val, priv->base + PCIE_BRIDGE_OPT2_REG);
+
+	/* setup class code as bridge */
+	val = __raw_readl(priv->base + PCIE_IDVAL3_REG);
+	val &= ~IDVAL3_CLASS_CODE_MASK;
+	val |= (PCI_CLASS_BRIDGE_PCI << IDVAL3_SUBCLASS_SHIFT);
+	__raw_writel(val, priv->base + PCIE_IDVAL3_REG);
+
+	/* disable bar1 size */
+	val = __raw_readl(priv->base + PCIE_CONFIG2_REG);
+	val &= ~CONFIG2_BAR1_SIZE_MASK;
+	__raw_writel(val, priv->base + PCIE_CONFIG2_REG);
+
+	/* set bar0 to little endian */
+	val = (bcm6328_pcie_mem_resource.start >> 20)
+	      << BASEMASK_BASE_SHIFT;
+	val |= (bcm6328_pcie_mem_resource.end >> 20) << BASEMASK_MASK_SHIFT;
+	val |= BASEMASK_REMAP_EN;
+	__raw_writel(val, priv->base + PCIE_BRIDGE_BAR0_BASEMASK_REG);
+
+	val = (bcm6328_pcie_mem_resource.start >> 20)
+	      << REBASE_ADDR_BASE_SHIFT;
+	__raw_writel(val, priv->base + PCIE_BRIDGE_BAR0_REBASE_ADDR_REG);
+}
+
+static int bcm6328_pcie_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct bcm6328_pcie *priv = &bcm6328_pcie;
+	int ret;
+
+	of_pci_check_probe_only();
+
+	priv->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	priv->serdes = syscon_regmap_lookup_by_phandle(np, "brcm,serdes");
+	if (IS_ERR(priv->serdes))
+		return PTR_ERR(priv->serdes);
+
+	priv->reset = devm_reset_control_get_exclusive(dev, "pcie");
+	if (IS_ERR(priv->reset))
+		return PTR_ERR(priv->reset);
+
+	priv->reset_ext = devm_reset_control_get_exclusive(dev, "pcie-ext");
+	if (IS_ERR(priv->reset_ext))
+		return PTR_ERR(priv->reset_ext);
+
+	priv->reset_core = devm_reset_control_get_exclusive(dev, "pcie-core");
+	if (IS_ERR(priv->reset_core))
+		return PTR_ERR(priv->reset_core);
+
+	priv->reset_hard = devm_reset_control_get_optional_exclusive(dev,
+		"pcie-hard");
+	if (IS_ERR(priv->reset_hard))
+		return PTR_ERR(priv->reset_hard);
+
+	priv->clk = devm_clk_get(dev, "pcie");
+	if (IS_ERR(priv->clk))
+		return PTR_ERR(priv->clk);
+
+	ret = clk_prepare_enable(priv->clk);
+	if (ret) {
+		dev_err(dev, "could not enable clock: %d\n", ret);
+		return ret;
+	}
+
+	pci_load_of_ranges(&bcm6328_pcie_controller, np);
+	if (!bcm6328_pcie_mem_resource.start)
+		return -EINVAL;
+
+	of_pci_parse_bus_range(np, &bcm6328_pcie_busn_resource);
+
+	bcm6328_pcie_reset(priv);
+	bcm6328_pcie_setup(priv);
+
+	register_pci_controller(&bcm6328_pcie_controller);
+
+	return 0;
+}
+
+static const struct of_device_id bcm6328_pcie_of_match[] = {
+	{ .compatible = "brcm,bcm6328-pcie", },
+	{ .compatible = "brcm,bcm6362-pcie", },
+	{ .compatible = "brcm,bcm63268-pcie", },
+	{ /* sentinel */ },
+};
+
+static struct platform_driver bcm6328_pcie_driver = {
+	.driver	= {
+		.name = "bcm6328-pcie",
+		.of_match_table = bcm6328_pcie_of_match,
+	},
+	.probe = bcm6328_pcie_probe,
+};
+builtin_platform_driver(bcm6328_pcie_driver);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] mips: bmips: add PCI support
  2020-06-17 10:25 ` [PATCH 1/3] mips: bmips: add PCI support Álvaro Fernández Rojas
@ 2020-06-17 11:09   ` Thomas Bogendoerfer
  0 siblings, 0 replies; 8+ messages in thread
From: Thomas Bogendoerfer @ 2020-06-17 11:09 UTC (permalink / raw)
  To: Álvaro Fernández Rojas
  Cc: bhelgaas, robh+dt, lorenzo.pieralisi, p.zabel, jiaxun.yang,
	paulburton, info, allison, kstewart, tglx, jonas.gorski,
	f.fainelli, bcm-kernel-feedback-list, linux-pci, devicetree,
	linux-kernel, linux-mips

On Wed, Jun 17, 2020 at 12:25:54PM +0200, Álvaro Fernández Rojas wrote:
> BMIPS SoCs with PCI: BCM6358, BCM6368.
> BMIPS SoCs with PCIe (gen1): BCM6328, BCM6362, BCM63268.
> BMIPS SoCs with PCIe (gen2): BCM6318.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>  arch/mips/Kconfig           |  1 +
>  arch/mips/pci/Makefile      |  1 +
>  arch/mips/pci/fixup-bmips.c | 17 +++++++++++++++++
>  3 files changed, 19 insertions(+)
>  create mode 100644 arch/mips/pci/fixup-bmips.c
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 6fee1a133e9d..357026cb51de 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -230,6 +230,7 @@ config BMIPS_GENERIC
>  	select ARCH_HAS_SYNC_DMA_FOR_CPU_ALL
>  	select ARCH_HAS_PHYS_TO_DMA
>  	select BOOT_RAW
> +	select HAVE_PCI
>  	select NO_EXCEPT_FILL
>  	select USE_OF
>  	select CEVT_R4K

as everything is using DT in your patch, can't you use PCI_DRIVERS_GENERIC
here and drop fixup-bmips ?

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] pci: add BCM6328 PCIe controller support
  2020-06-17 10:25 ` [PATCH 3/3] pci: add BCM6328 PCIe controller support Álvaro Fernández Rojas
@ 2020-06-17 15:50   ` Bjorn Helgaas
  0 siblings, 0 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2020-06-17 15:50 UTC (permalink / raw)
  To: Álvaro Fernández Rojas
  Cc: bhelgaas, robh+dt, tsbogend, lorenzo.pieralisi, p.zabel,
	jiaxun.yang, paulburton, info, allison, kstewart, tglx,
	jonas.gorski, f.fainelli, bcm-kernel-feedback-list, linux-pci,
	devicetree, linux-kernel, linux-mips

"git log --oneline drivers/pci/controller/" tells you that the
conventional style for the subject would be:

  PCI: bcm6328: Add BCM6328 PCIe host controller support

On Wed, Jun 17, 2020 at 12:25:56PM +0200, Álvaro Fernández Rojas wrote:
> BCM6328 PCIe host controller is found on BCM6328, BCM6362 and BCM63268 SoCs.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>  drivers/pci/controller/Kconfig        |   8 +
>  drivers/pci/controller/Makefile       |   1 +
>  drivers/pci/controller/pcie-bcm6328.c | 346 ++++++++++++++++++++++++++
>  3 files changed, 355 insertions(+)
>  create mode 100644 drivers/pci/controller/pcie-bcm6328.c
> 
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index adddf21fa381..7e238c04764e 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -3,6 +3,14 @@
>  menu "PCI controller drivers"
>  	depends on PCI
>  
> +config PCIE_BCM6328
> +	bool "BCM6328 PCIe controller"
> +	depends on BMIPS_GENERIC || COMPILE_TEST
> +	depends on OF
> +	help
> +	  Say Y here if you want support for the PCIe host controller found
> +	  on BCM6328, BCM6362 and BCM63268 SoCs.
> +
>  config PCI_MVEBU
>  	bool "Marvell EBU PCIe controller"
>  	depends on ARCH_MVEBU || ARCH_DOVE || COMPILE_TEST
> diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
> index efd9733ead26..1c3e82575845 100644
> --- a/drivers/pci/controller/Makefile
> +++ b/drivers/pci/controller/Makefile
> @@ -1,4 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_PCIE_BCM6328) += pcie-bcm6328.o
>  obj-$(CONFIG_PCIE_CADENCE) += cadence/
>  obj-$(CONFIG_PCI_FTPCI100) += pci-ftpci100.o
>  obj-$(CONFIG_PCI_HYPERV) += pci-hyperv.o
> diff --git a/drivers/pci/controller/pcie-bcm6328.c b/drivers/pci/controller/pcie-bcm6328.c
> new file mode 100644
> index 000000000000..5bd86b166336
> --- /dev/null
> +++ b/drivers/pci/controller/pcie-bcm6328.c
> @@ -0,0 +1,346 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * BCM6328 PCIe Controller Driver
> + *
> + * Copyright (C) 2020 Álvaro Fernández Rojas <noltari@gmail.com>
> + * Copyright (C) 2015 Jonas Gorski <jonas.gorski@gmail.com>
> + * Copyright (C) 2008 Maxime Bizon <mbizon@freebox.fr>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of_pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +
> +#include "../pci.h"
> +
> +#define SERDES_PCIE_EN			BIT(0)
> +#define SERDES_PCIE_EXD_EN		BIT(15)
> +
> +#define PCIE_BUS_BRIDGE			0
> +#define PCIE_BUS_DEVICE			1
> +
> +#define PCIE_CONFIG2_REG		0x408
> +#define CONFIG2_BAR1_SIZE_EN		1
> +#define CONFIG2_BAR1_SIZE_MASK		0xf
> +
> +#define PCIE_IDVAL3_REG			0x43c
> +#define IDVAL3_CLASS_CODE_MASK		0xffffff
> +#define IDVAL3_SUBCLASS_SHIFT		8
> +#define IDVAL3_CLASS_SHIFT		16
> +
> +#define PCIE_DLSTATUS_REG		0x1048
> +#define DLSTATUS_PHYLINKUP		BIT(13)
> +
> +#define PCIE_BRIDGE_OPT1_REG		0x2820
> +#define OPT1_RD_BE_OPT_EN		BIT(7)
> +#define OPT1_RD_REPLY_BE_FIX_EN		BIT(9)
> +#define OPT1_PCIE_BRIDGE_HOLE_DET_EN	BIT(11)
> +#define OPT1_L1_INT_STATUS_MASK_POL	BIT(12)
> +
> +#define PCIE_BRIDGE_OPT2_REG		0x2824
> +#define OPT2_UBUS_UR_DECODE_DIS		BIT(2)
> +#define OPT2_TX_CREDIT_CHK_EN		BIT(4)
> +#define OPT2_CFG_TYPE1_BD_SEL		BIT(7)
> +#define OPT2_CFG_TYPE1_BUS_NO_SHIFT	16
> +#define OPT2_CFG_TYPE1_BUS_NO_MASK	(0xff << OPT2_CFG_TYPE1_BUS_NO_SHIFT)
> +
> +#define PCIE_BRIDGE_BAR0_BASEMASK_REG	0x2828
> +#define BASEMASK_REMAP_EN		BIT(0)
> +#define BASEMASK_SWAP_EN		BIT(1)
> +#define BASEMASK_MASK_SHIFT		4
> +#define BASEMASK_MASK_MASK		(0xfff << BASEMASK_MASK_SHIFT)
> +#define BASEMASK_BASE_SHIFT		20
> +#define BASEMASK_BASE_MASK		(0xfff << BASEMASK_BASE_SHIFT)
> +
> +#define PCIE_BRIDGE_BAR0_REBASE_ADDR_REG 0x282c
> +#define REBASE_ADDR_BASE_SHIFT		20
> +#define REBASE_ADDR_BASE_MASK		(0xfff << REBASE_ADDR_BASE_SHIFT)
> +
> +#define PCIE_BRIDGE_RC_INT_MASK_REG	0x2854
> +#define PCIE_RC_INT_A			BIT(0)
> +#define PCIE_RC_INT_B			BIT(1)
> +#define PCIE_RC_INT_C			BIT(2)
> +#define PCIE_RC_INT_D			BIT(3)
> +
> +#define PCIE_DEVICE_OFFSET		0x8000
> +
> +struct bcm6328_pcie {
> +	void __iomem *base;
> +	struct regmap *serdes;
> +	struct clk *clk;
> +	struct reset_control *reset;
> +	struct reset_control *reset_ext;
> +	struct reset_control *reset_core;
> +	struct reset_control *reset_hard;
> +};
> +
> +static struct bcm6328_pcie bcm6328_pcie;

It would be better to dynamically allocate this like all the other
drivers in driver/pci/controller/ do.

> +/*
> + * swizzle 32bits data to return only the needed part
> + */
> +static int postprocess_read(u32 data, int where, unsigned int size)
> +{
> +	u32 ret = 0;
> +
> +	switch (size) {
> +	case 1:
> +		ret = (data >> ((where & 3) << 3)) & 0xff;
> +		break;
> +	case 2:
> +		ret = (data >> ((where & 3) << 3)) & 0xffff;
> +		break;
> +	case 4:
> +		ret = data;
> +		break;
> +	}
> +
> +	return ret;

No need for "ret" here; just return the values directly.

> +}
> +
> +static int preprocess_write(u32 orig_data, u32 val, int where,
> +			    unsigned int size)
> +{
> +	u32 ret = 0;
> +
> +	switch (size) {
> +	case 1:
> +		ret = (orig_data & ~(0xff << ((where & 3) << 3))) |
> +		      (val << ((where & 3) << 3));
> +		break;
> +	case 2:
> +		ret = (orig_data & ~(0xffff << ((where & 3) << 3))) |
> +		      (val << ((where & 3) << 3));
> +		break;
> +	case 4:
> +		ret = val;
> +		break;
> +	}
> +
> +	return ret;

No need for "ret" here either.

> +}
> +
> +static int bcm6328_pcie_can_access(struct pci_bus *bus, int devfn)
> +{
> +	struct bcm6328_pcie *priv = &bcm6328_pcie;
> +
> +	switch (bus->number) {
> +	case PCIE_BUS_BRIDGE:
> +		return PCI_SLOT(devfn) == 0;
> +	case PCIE_BUS_DEVICE:
> +		if (PCI_SLOT(devfn) == 0)
> +			return __raw_readl(priv->base + PCIE_DLSTATUS_REG)
> +			       & DLSTATUS_PHYLINKUP;

Checking for link up is problematic.  The link may go down after we
check but before we do the actual config read.  We have to be able to
handle that case anyway, so you should not check whether the link is
up.

> +		/* fall through */
> +	default:
> +		return false;
> +	}

Why do you need this function at all?  Normally a config read to a
device that doesn't exist returns ~0 data, and the PCI core knows how
to interpret that.

The implication is that this controller is severely limited and can
only support:

  bus 0, device 0, functions 0-7 (presumably root ports), and
  bus 1, device 0, functions 0-7 (presumably endpoints)?

It cannot support any switches or slots?  And apparently the root bus
number is not programmable?

> +}
> +
> +static int bcm6328_pcie_read(struct pci_bus *bus, unsigned int devfn,
> +			     int where, int size, u32 *val)
> +{
> +	struct bcm6328_pcie *priv = &bcm6328_pcie;
> +	u32 data;
> +	u32 reg = where & ~3;
> +
> +	if (!bcm6328_pcie_can_access(bus, devfn))
> +		return PCIBIOS_DEVICE_NOT_FOUND;
> +
> +	if (bus->number == PCIE_BUS_DEVICE)
> +		reg += PCIE_DEVICE_OFFSET;
> +
> +	data = __raw_readl(priv->base + reg);

Why do you need __raw_readl() and __raw_writel?  This would be the
first use in drivers/pci/, so you shouldn't need them unless this
hardware is really unique in some way.

> +	*val = postprocess_read(data, where, size);
> +
> +	return PCIBIOS_SUCCESSFUL;
> +}
> +
> +static int bcm6328_pcie_write(struct pci_bus *bus, unsigned int devfn,
> +			      int where, int size, u32 val)
> +{
> +	struct bcm6328_pcie *priv = &bcm6328_pcie;
> +	u32 data;
> +	u32 reg = where & ~3;
> +
> +	if (!bcm6328_pcie_can_access(bus, devfn))
> +		return PCIBIOS_DEVICE_NOT_FOUND;
> +
> +	if (bus->number == PCIE_BUS_DEVICE)
> +		reg += PCIE_DEVICE_OFFSET;
> +
> +	data = __raw_readl(priv->base + reg);
> +	data = preprocess_write(data, val, where, size);
> +	__raw_writel(data, priv->base + reg);

So this hardware is unable to perform 1- or 2-byte config writes on
PCI?  That's a hardware defect that can cause corruption of config
registers.

See pci_generic_config_write32() and add a similar warning here if
this hardware is broken in this way.

> +	return PCIBIOS_SUCCESSFUL;
> +}
> +
> +static struct pci_ops bcm6328_pcie_ops = {
> +	.read = bcm6328_pcie_read,
> +	.write = bcm6328_pcie_write,
> +};
> +
> +static struct resource bcm6328_pcie_io_resource;
> +static struct resource bcm6328_pcie_mem_resource;
> +static struct resource bcm6328_pcie_busn_resource;
> +
> +static struct pci_controller bcm6328_pcie_controller = {
> +	.pci_ops = &bcm6328_pcie_ops,
> +	.io_resource = &bcm6328_pcie_io_resource,
> +	.mem_resource = &bcm6328_pcie_mem_resource,
> +	.busn_resource = &bcm6328_pcie_busn_resource,
> +};
> +
> +static void bcm6328_pcie_reset(struct bcm6328_pcie *priv)
> +{
> +	regmap_write_bits(priv->serdes, 0,
> +			  SERDES_PCIE_EN | SERDES_PCIE_EXD_EN,
> +			  SERDES_PCIE_EN | SERDES_PCIE_EXD_EN);
> +
> +	reset_control_assert(priv->reset);
> +	reset_control_assert(priv->reset_core);
> +	reset_control_assert(priv->reset_ext);
> +	if (priv->reset_hard) {
> +		reset_control_assert(priv->reset_hard);
> +		mdelay(10);
> +		reset_control_deassert(priv->reset_hard);
> +	}
> +	mdelay(10);
> +
> +	reset_control_deassert(priv->reset_core);
> +	reset_control_deassert(priv->reset);
> +	mdelay(10);
> +
> +	reset_control_deassert(priv->reset_ext);
> +	mdelay(200);
> +}
> +
> +static void bcm6328_pcie_setup(struct bcm6328_pcie *priv)
> +{
> +	u32 val;
> +
> +	val = __raw_readl(priv->base + PCIE_BRIDGE_OPT1_REG);
> +	val |= OPT1_RD_BE_OPT_EN;
> +	val |= OPT1_RD_REPLY_BE_FIX_EN;
> +	val |= OPT1_PCIE_BRIDGE_HOLE_DET_EN;
> +	val |= OPT1_L1_INT_STATUS_MASK_POL;
> +	__raw_writel(val, priv->base + PCIE_BRIDGE_OPT1_REG);
> +
> +	val = __raw_readl(priv->base + PCIE_BRIDGE_RC_INT_MASK_REG);
> +	val |= PCIE_RC_INT_A;
> +	val |= PCIE_RC_INT_B;
> +	val |= PCIE_RC_INT_C;
> +	val |= PCIE_RC_INT_D;
> +	__raw_writel(val, priv->base + PCIE_BRIDGE_RC_INT_MASK_REG);
> +
> +	val = __raw_readl(priv->base + PCIE_BRIDGE_OPT2_REG);
> +	/* enable credit checking and error checking */
> +	val |= OPT2_TX_CREDIT_CHK_EN;
> +	val |= OPT2_UBUS_UR_DECODE_DIS;
> +	/* set device bus/func for the pcie device */
> +	val |= (PCIE_BUS_DEVICE << OPT2_CFG_TYPE1_BUS_NO_SHIFT);
> +	val |= OPT2_CFG_TYPE1_BD_SEL;
> +	__raw_writel(val, priv->base + PCIE_BRIDGE_OPT2_REG);
> +
> +	/* setup class code as bridge */
> +	val = __raw_readl(priv->base + PCIE_IDVAL3_REG);
> +	val &= ~IDVAL3_CLASS_CODE_MASK;
> +	val |= (PCI_CLASS_BRIDGE_PCI << IDVAL3_SUBCLASS_SHIFT);
> +	__raw_writel(val, priv->base + PCIE_IDVAL3_REG);
> +
> +	/* disable bar1 size */
> +	val = __raw_readl(priv->base + PCIE_CONFIG2_REG);
> +	val &= ~CONFIG2_BAR1_SIZE_MASK;
> +	__raw_writel(val, priv->base + PCIE_CONFIG2_REG);
> +
> +	/* set bar0 to little endian */
> +	val = (bcm6328_pcie_mem_resource.start >> 20)
> +	      << BASEMASK_BASE_SHIFT;
> +	val |= (bcm6328_pcie_mem_resource.end >> 20) << BASEMASK_MASK_SHIFT;
> +	val |= BASEMASK_REMAP_EN;
> +	__raw_writel(val, priv->base + PCIE_BRIDGE_BAR0_BASEMASK_REG);
> +
> +	val = (bcm6328_pcie_mem_resource.start >> 20)
> +	      << REBASE_ADDR_BASE_SHIFT;
> +	__raw_writel(val, priv->base + PCIE_BRIDGE_BAR0_REBASE_ADDR_REG);
> +}
> +
> +static int bcm6328_pcie_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = dev->of_node;
> +	struct bcm6328_pcie *priv = &bcm6328_pcie;
> +	int ret;
> +
> +	of_pci_check_probe_only();

Why do you need this?

> +	priv->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(priv->base))
> +		return PTR_ERR(priv->base);
> +
> +	priv->serdes = syscon_regmap_lookup_by_phandle(np, "brcm,serdes");
> +	if (IS_ERR(priv->serdes))
> +		return PTR_ERR(priv->serdes);
> +
> +	priv->reset = devm_reset_control_get_exclusive(dev, "pcie");
> +	if (IS_ERR(priv->reset))
> +		return PTR_ERR(priv->reset);
> +
> +	priv->reset_ext = devm_reset_control_get_exclusive(dev, "pcie-ext");
> +	if (IS_ERR(priv->reset_ext))
> +		return PTR_ERR(priv->reset_ext);
> +
> +	priv->reset_core = devm_reset_control_get_exclusive(dev, "pcie-core");
> +	if (IS_ERR(priv->reset_core))
> +		return PTR_ERR(priv->reset_core);
> +
> +	priv->reset_hard = devm_reset_control_get_optional_exclusive(dev,
> +		"pcie-hard");
> +	if (IS_ERR(priv->reset_hard))
> +		return PTR_ERR(priv->reset_hard);
> +
> +	priv->clk = devm_clk_get(dev, "pcie");
> +	if (IS_ERR(priv->clk))
> +		return PTR_ERR(priv->clk);
> +
> +	ret = clk_prepare_enable(priv->clk);
> +	if (ret) {
> +		dev_err(dev, "could not enable clock: %d\n", ret);
> +		return ret;
> +	}
> +
> +	pci_load_of_ranges(&bcm6328_pcie_controller, np);

Please use the generic pci_parse_request_of_pci_ranges() as most PCIe
host drivers do, instead of the MIPS-specific pci_load_of_ranges().

I know that's probably not a trivial change, but it would help remove
a lot of unnecessarily MIPS-specific code from this driver and its
dependencies.

> +	if (!bcm6328_pcie_mem_resource.start)
> +		return -EINVAL;
> +
> +	of_pci_parse_bus_range(np, &bcm6328_pcie_busn_resource);
> +
> +	bcm6328_pcie_reset(priv);
> +	bcm6328_pcie_setup(priv);
> +
> +	register_pci_controller(&bcm6328_pcie_controller);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id bcm6328_pcie_of_match[] = {
> +	{ .compatible = "brcm,bcm6328-pcie", },
> +	{ .compatible = "brcm,bcm6362-pcie", },
> +	{ .compatible = "brcm,bcm63268-pcie", },
> +	{ /* sentinel */ },
> +};
> +
> +static struct platform_driver bcm6328_pcie_driver = {
> +	.driver	= {
> +		.name = "bcm6328-pcie",
> +		.of_match_table = bcm6328_pcie_of_match,
> +	},
> +	.probe = bcm6328_pcie_probe,
> +};
> +builtin_platform_driver(bcm6328_pcie_driver);
> -- 
> 2.27.0
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] dt-bindings: Document BCM6328 PCIe Host Controller
  2020-06-17 10:25 ` [PATCH 2/3] dt-bindings: Document BCM6328 PCIe Host Controller Álvaro Fernández Rojas
@ 2020-06-17 17:07   ` Rob Herring
  2020-06-17 21:50   ` Florian Fainelli
  1 sibling, 0 replies; 8+ messages in thread
From: Rob Herring @ 2020-06-17 17:07 UTC (permalink / raw)
  To: Álvaro Fernández Rojas
  Cc: paulburton, tsbogend, devicetree, kstewart, f.fainelli,
	bcm-kernel-feedback-list, lorenzo.pieralisi, p.zabel, robh+dt,
	jonas.gorski, linux-kernel, info, tglx, allison, linux-pci,
	jiaxun.yang, bhelgaas, linux-mips

On Wed, 17 Jun 2020 12:25:55 +0200, Álvaro Fernández Rojas wrote:
> BCM6328 PCIe host controller is found on BCM6328, BCM6362 and BCM63268 SoCs.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>  .../bindings/pci/brcm,bcm6328-pcie.yaml       | 109 ++++++++++++++++++
>  1 file changed, 109 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/brcm,bcm6328-pcie.yaml
> 


My bot found errors running 'make dt_binding_check' on your patch:

Error: Documentation/devicetree/bindings/pci/brcm,bcm6328-pcie.example.dts:38.49-50 syntax error
FATAL ERROR: Unable to parse input tree
scripts/Makefile.lib:315: recipe for target 'Documentation/devicetree/bindings/pci/brcm,bcm6328-pcie.example.dt.yaml' failed
make[1]: *** [Documentation/devicetree/bindings/pci/brcm,bcm6328-pcie.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
Makefile:1347: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2


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

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

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] dt-bindings: Document BCM6328 PCIe Host Controller
  2020-06-17 10:25 ` [PATCH 2/3] dt-bindings: Document BCM6328 PCIe Host Controller Álvaro Fernández Rojas
  2020-06-17 17:07   ` Rob Herring
@ 2020-06-17 21:50   ` Florian Fainelli
  1 sibling, 0 replies; 8+ messages in thread
From: Florian Fainelli @ 2020-06-17 21:50 UTC (permalink / raw)
  To: Álvaro Fernández Rojas, bhelgaas, robh+dt, tsbogend,
	lorenzo.pieralisi, p.zabel, jiaxun.yang, paulburton, info,
	allison, kstewart, tglx, jonas.gorski, bcm-kernel-feedback-list,
	linux-pci, devicetree, linux-kernel, linux-mips



On 6/17/2020 3:25 AM, Álvaro Fernández Rojas wrote:
> BCM6328 PCIe host controller is found on BCM6328, BCM6362 and BCM63268 SoCs.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>  .../bindings/pci/brcm,bcm6328-pcie.yaml       | 109 ++++++++++++++++++
>  1 file changed, 109 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/brcm,bcm6328-pcie.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pci/brcm,bcm6328-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,bcm6328-pcie.yaml
> new file mode 100644
> index 000000000000..d2bd4933a5fa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/brcm,bcm6328-pcie.yaml
> @@ -0,0 +1,109 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/brcm,bcm6328-pcie.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: BCM6328 PCIe Host Controller Device Tree Bindings
> +
> +maintainers:
> +  - Álvaro Fernández Rojas <noltari@gmail.com>
> +
> +allOf:
> +  - $ref: /schemas/pci/pci-bus.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +     - brcm,bcm6328-pcie
> +     - brcm,bcm6362-pcie
> +     - brcm,bcm63268-pcie
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: pcie
> +
> +  "#interrupt-cells":
> +    const: 1
> +
> +  interrupt-map-mask:
> +    maxItems: 1
> +
> +  interrupt-map:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  ranges:
> +    maxItems: 1
> +
> +  reg:
> +    maxItems: 1
> +
> +  resets:
> +    maxItems: 4
> +
> +  reset-names:
> +    items:
> +      - const: pcie
> +      - const: pcie-core
> +      - const: pcie-ext
> +      - const: pcie-hard
> +
> +required:
> +  - brcm,serdes
> +  - clocks
> +  - clock-names
> +  - "#interrupt-cells"
> +  - interrupt-map-mask
> +  - interrupt-map
> +  - ranges
> +  - reg
> +  - resets
> +  - reset-names
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    serdes_cntl: syscon@10001800 {
> +      compatible = "syscon";
> +      reg = <0x10001800 0x4>;
> +      native-endian;
> +    };

I believe you could be modelling the SerDes as generic PHY driver which
would be a little cleaner than the syscon approach. In newer chips like
6318 it looks like you should be able to use pcie-brcmstb.c since the
controller appears to be nearly the same and the PHY abstraction would
work nicely there.
-- 
Florian

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-06-17 21:50 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-17 10:25 [PATCH 0/3] mips: bmips: add BCM6328 PCIe support Álvaro Fernández Rojas
2020-06-17 10:25 ` [PATCH 1/3] mips: bmips: add PCI support Álvaro Fernández Rojas
2020-06-17 11:09   ` Thomas Bogendoerfer
2020-06-17 10:25 ` [PATCH 2/3] dt-bindings: Document BCM6328 PCIe Host Controller Álvaro Fernández Rojas
2020-06-17 17:07   ` Rob Herring
2020-06-17 21:50   ` Florian Fainelli
2020-06-17 10:25 ` [PATCH 3/3] pci: add BCM6328 PCIe controller support Álvaro Fernández Rojas
2020-06-17 15:50   ` Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).