devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Add support for AMD MDB IP as Root Port
@ 2024-12-03 12:36 Thippeswamy Havalige
  2024-12-03 12:36 ` [PATCH v2 1/2] dt-bindings: PCI: amd-mdb: Add AMD Versal2 MDB PCIe Root Port Bridge Thippeswamy Havalige
  2024-12-03 12:36 ` [PATCH v2 2/2] PCI: amd-mdb: Add AMD MDB Root Port driver Thippeswamy Havalige
  0 siblings, 2 replies; 7+ messages in thread
From: Thippeswamy Havalige @ 2024-12-03 12:36 UTC (permalink / raw)
  To: bhelgaas, lpieralisi, kw, manivannan.sadhasivam, robh, krzk+dt,
	conor+dt
  Cc: linux-pci, devicetree, linux-kernel, jingoohan1, michal.simek,
	bharat.kumar.gogada, Thippeswamy Havalige

This series of patch add support for AMD MDB IP as Root Port.

The AMD MDB IP support's 32 bit and 64bit BAR's at Gen5 speed.
As Root Port it supports MSI and legacy interrupts.

Thippeswamy Havalige (2):
  dt-bindings: PCI: amd-mdb: Add AMD Versal2 MDB PCIe Root Port Bridge
  PCI: amd-mdb: Add AMD MDB Root Port driver

 .../bindings/pci/amd,versal2-mdb-host.yaml    | 132 ++++++
 drivers/pci/controller/dwc/Kconfig            |  10 +
 drivers/pci/controller/dwc/Makefile           |   1 +
 drivers/pci/controller/dwc/pcie-amd-mdb.c     | 439 ++++++++++++++++++
 4 files changed, 582 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml
 create mode 100644 drivers/pci/controller/dwc/pcie-amd-mdb.c

-- 
2.34.1


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

* [PATCH v2 1/2] dt-bindings: PCI: amd-mdb: Add AMD Versal2 MDB PCIe Root Port Bridge
  2024-12-03 12:36 [PATCH v2 0/2] Add support for AMD MDB IP as Root Port Thippeswamy Havalige
@ 2024-12-03 12:36 ` Thippeswamy Havalige
  2024-12-03 13:24   ` Rob Herring (Arm)
  2024-12-03 14:41   ` Rob Herring
  2024-12-03 12:36 ` [PATCH v2 2/2] PCI: amd-mdb: Add AMD MDB Root Port driver Thippeswamy Havalige
  1 sibling, 2 replies; 7+ messages in thread
From: Thippeswamy Havalige @ 2024-12-03 12:36 UTC (permalink / raw)
  To: bhelgaas, lpieralisi, kw, manivannan.sadhasivam, robh, krzk+dt,
	conor+dt
  Cc: linux-pci, devicetree, linux-kernel, jingoohan1, michal.simek,
	bharat.kumar.gogada, Thippeswamy Havalige

Add AMD Versal2 MDB (Multimedia DMA Bridge) PCIe Root Port Bridge.

Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
---
Changes in v2:
-------------
- Modify patch subject.
- Add pcie host bridge reference.
- Modify filename as per compatible string.
- Remove standard PCI properties.
- Modify interrupt controller description.
- Indentation
---
 .../bindings/pci/amd,versal2-mdb-host.yaml    | 132 ++++++++++++++++++
 1 file changed, 132 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml

diff --git a/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml b/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml
new file mode 100644
index 000000000000..75795bab8254
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml
@@ -0,0 +1,132 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/amd,mdb-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMD Versal2 MDB(Multimedia DMA Bridge) Host Controller
+
+maintainers:
+  - Thippeswamy Havalige <thippeswamy.havalige@amd.com>
+
+allOf:
+  - $ref: /schemas/pci/pci-host-bridge.yaml#
+
+properties:
+  compatible:
+    const: amd,versal2-mdb-host
+
+  reg:
+    items:
+      - description: MDB PCIe controller 0 SLCR
+      - description: configuration region
+      - description: data bus interface
+      - description: address translation unit register
+
+  reg-names:
+    items:
+      - const: mdb_pcie_slcr
+      - const: config
+      - const: dbi
+      - const: atu
+
+  ranges:
+    maxItems: 2
+
+  msi-map:
+    maxItems: 1
+
+  bus-range:
+    maxItems: 1
+
+  "#address-cells":
+    const: 3
+
+  "#size-cells":
+    const: 2
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-map-mask:
+    items:
+      - const: 0
+      - const: 0
+      - const: 0
+      - const: 7
+
+  interrupt-map:
+    maxItems: 4
+
+  "#interrupt-cells":
+    const: 1
+
+  interrupt-controller:
+    description: identifies the node as an interrupt controller
+    type: object
+    properties:
+      interrupt-controller: true
+
+      "#address-cells":
+        const: 0
+
+      "#interrupt-cells":
+        const: 1
+
+    required:
+      - interrupt-controller
+      - "#address-cells"
+      - "#interrupt-cells"
+
+    additionalProperties: false
+
+required:
+  - reg
+  - reg-names
+  - interrupts
+  - interrupt-map
+  - interrupt-map-mask
+  - msi-map
+  - ranges
+  - "#interrupt-cells"
+  - interrupt-controller
+
+unevaluatedProperties: false
+
+examples:
+
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+        pci@ed931000 {
+            compatible = "amd,versal2-mdb-host";
+            reg = <0x0 0xed931000 0x0 0x2000>,
+                  <0x1000 0x100000 0x0 0xff00000>,
+                  <0x1000 0x0 0x0 0x100000>,
+                  <0x0 0xed860000 0x0 0x2000>;
+            reg-names = "mdb_pcie_slcr", "config", "dbi", "atu";
+            ranges = <0x2000000 0x00 0xa8000000 0x00 0xa8000000 0x00 0x10000000>,
+                     <0x43000000 0x1100 0x00 0x1100 0x00 0x00 0x1000000>;
+            interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-parent = <&gic>;
+            interrupt-map-mask = <0 0 0 7>;
+            interrupt-map = <0 0 0 1 &pcie_intc_0 0>,
+                            <0 0 0 2 &pcie_intc_0 1>,
+                            <0 0 0 3 &pcie_intc_0 2>,
+                            <0 0 0 4 &pcie_intc_0 3>;
+            msi-map = <0x0 &gic_its 0x00 0x10000>;
+            #address-cells = <3>;
+            #size-cells = <2>;
+            #interrupt-cells = <1>;
+            device_type = "pci";
+            pcie_intc_0: interrupt-controller {
+                #address-cells = <0>;
+                #interrupt-cells = <1>;
+                interrupt-controller;
+           };
+        };
+    };
-- 
2.34.1


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

* [PATCH v2 2/2] PCI: amd-mdb: Add AMD MDB Root Port driver
  2024-12-03 12:36 [PATCH v2 0/2] Add support for AMD MDB IP as Root Port Thippeswamy Havalige
  2024-12-03 12:36 ` [PATCH v2 1/2] dt-bindings: PCI: amd-mdb: Add AMD Versal2 MDB PCIe Root Port Bridge Thippeswamy Havalige
@ 2024-12-03 12:36 ` Thippeswamy Havalige
  1 sibling, 0 replies; 7+ messages in thread
From: Thippeswamy Havalige @ 2024-12-03 12:36 UTC (permalink / raw)
  To: bhelgaas, lpieralisi, kw, manivannan.sadhasivam, robh, krzk+dt,
	conor+dt
  Cc: linux-pci, devicetree, linux-kernel, jingoohan1, michal.simek,
	bharat.kumar.gogada, Thippeswamy Havalige

Add support for AMD MDB(Multimedia DMA Bridge) IP core as Root Port.

The Versal2 devices include MDB Module. The integrated block for MDB along
with the integrated bridge can function as PCIe Root Port controller at
Gen5 32-Gb/s operation per lane.

Bridge supports error and legacy interrupts and are handled using platform
specific interrupt line in Versal2.

Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
---
changes in v2:
-------------
- Update Gen5 speed in the patch description.
- Modify Kconfig file.
- Update string _leg_ to intx.
- Get platform structure through automic variables.
- Remove _rp_ in function.
---
 drivers/pci/controller/dwc/Kconfig        |  10 +
 drivers/pci/controller/dwc/Makefile       |   1 +
 drivers/pci/controller/dwc/pcie-amd-mdb.c | 439 ++++++++++++++++++++++
 3 files changed, 450 insertions(+)
 create mode 100644 drivers/pci/controller/dwc/pcie-amd-mdb.c

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index b6d6778b0698..30d0a3eadf1d 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -27,6 +27,16 @@ config PCIE_AL
 	  required only for DT-based platforms. ACPI platforms with the
 	  Annapurna Labs PCIe controller don't need to enable this.
 
+config PCIE_AMD_MDB
+	bool "AMD PCIe controller (host mode)"
+	depends on OF || COMPILE_TEST
+	depends on PCI && PCI_MSI
+	select PCIE_DW_HOST
+	help
+	  Say Y here to enable PCIe controller support on AMD SoCs. The
+	  PCIe controller is based on DesignWare Hardware and uses AMD
+	  hardware wrappers.
+
 config PCI_MESON
 	tristate "Amlogic Meson PCIe controller"
 	default m if ARCH_MESON
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index a8308d9ea986..ae27eda6ec5e 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_AMD_MDB) += pcie-amd-mdb.o
 obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o
 obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
 obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
diff --git a/drivers/pci/controller/dwc/pcie-amd-mdb.c b/drivers/pci/controller/dwc/pcie-amd-mdb.c
new file mode 100644
index 000000000000..3947aad13ea0
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-amd-mdb.c
@@ -0,0 +1,439 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PCIe host controller driver for AMD MDB PCIe Bridge
+ *
+ * Copyright (C) 2024-2025, Advanced Micro Devices, Inc.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/interrupt.h>
+#include <linux/irqdomain.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/of_device.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/resource.h>
+#include <linux/types.h>
+
+#include "pcie-designware.h"
+
+#define AMD_MDB_TLP_IR_STATUS_MISC		0x4C0
+#define AMD_MDB_TLP_IR_MASK_MISC		0x4C4
+#define AMD_MDB_TLP_IR_ENABLE_MISC		0x4C8
+
+#define AMD_MDB_PCIE_IDRN_SHIFT			16
+
+/* Interrupt registers definitions */
+#define AMD_MDB_PCIE_INTR_CMPL_TIMEOUT		15
+#define AMD_MDB_PCIE_INTR_PM_PME_RCVD		24
+#define AMD_MDB_PCIE_INTR_PME_TO_ACK_RCVD	25
+#define AMD_MDB_PCIE_INTR_MISC_CORRECTABLE	26
+#define AMD_MDB_PCIE_INTR_NONFATAL		27
+#define AMD_MDB_PCIE_INTR_FATAL			28
+
+#define IMR(x) BIT(AMD_MDB_PCIE_INTR_ ##x)
+#define AMD_MDB_PCIE_IMR_ALL_MASK			\
+	(						\
+		IMR(CMPL_TIMEOUT)	|		\
+		IMR(PM_PME_RCVD)	|		\
+		IMR(PME_TO_ACK_RCVD)	|		\
+		IMR(MISC_CORRECTABLE)	|		\
+		IMR(NONFATAL)		|		\
+		IMR(FATAL)				\
+	)
+
+/**
+ * struct amd_mdb_pcie - PCIe port information
+ * @pci: DesignWare PCIe controller structure
+ * @mdb_base: MDB System Level Control and Status Register(SLCR) Base
+ * @intx_domain: Legacy IRQ domain pointer
+ * @mdb_domain: MDB IRQ domain pointer
+ */
+struct amd_mdb_pcie {
+	struct dw_pcie			pci;
+	void __iomem			*mdb_base;
+	struct irq_domain		*intx_domain;
+	struct irq_domain		*mdb_domain;
+};
+
+static const struct dw_pcie_host_ops amd_mdb_pcie_host_ops = {
+};
+
+static inline u32 pcie_read(struct amd_mdb_pcie *pcie, u32 reg)
+{
+	return readl_relaxed(pcie->mdb_base + reg);
+}
+
+static inline void pcie_write(struct amd_mdb_pcie *pcie,
+			      u32 val, u32 reg)
+{
+	writel_relaxed(val, pcie->mdb_base + reg);
+}
+
+static void amd_mdb_mask_intx_irq(struct irq_data *data)
+{
+	struct amd_mdb_pcie *pcie = irq_data_get_irq_chip_data(data);
+	struct dw_pcie *pci = &pcie->pci;
+	struct dw_pcie_rp *port = &pci->pp;
+	unsigned long flags;
+	u32 mask, val;
+
+	mask = BIT(data->hwirq + AMD_MDB_PCIE_IDRN_SHIFT);
+	raw_spin_lock_irqsave(&port->lock, flags);
+
+	val = pcie_read(pcie, AMD_MDB_TLP_IR_STATUS_MISC);
+	pcie_write(pcie, (val & (~mask)), AMD_MDB_TLP_IR_STATUS_MISC);
+
+	raw_spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static void amd_mdb_unmask_intx_irq(struct irq_data *data)
+{
+	struct amd_mdb_pcie *pcie = irq_data_get_irq_chip_data(data);
+	struct dw_pcie *pci = &pcie->pci;
+	struct dw_pcie_rp *port = &pci->pp;
+	unsigned long flags;
+	u32 mask;
+	u32 val;
+
+	mask = BIT(data->hwirq + AMD_MDB_PCIE_IDRN_SHIFT);
+	raw_spin_lock_irqsave(&port->lock, flags);
+
+	val = pcie_read(pcie, AMD_MDB_TLP_IR_STATUS_MISC);
+	pcie_write(pcie, (val | mask), AMD_MDB_TLP_IR_STATUS_MISC);
+
+	raw_spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static struct irq_chip amd_mdb_intx_irq_chip = {
+	.name		= "INTx",
+	.irq_mask	= amd_mdb_mask_intx_irq,
+	.irq_unmask	= amd_mdb_unmask_intx_irq,
+};
+
+/**
+ * amd_mdb_pcie_intx_map - Set the handler for the INTx and mark IRQ
+ * as valid
+ * @domain: IRQ domain
+ * @irq: Virtual IRQ number
+ * @hwirq: HW interrupt number
+ *
+ * Return: Always returns 0.
+ */
+static int amd_mdb_pcie_intx_map(struct irq_domain *domain,
+				 unsigned int irq, irq_hw_number_t hwirq)
+{
+	irq_set_chip_and_handler(irq, &amd_mdb_intx_irq_chip,
+				 handle_level_irq);
+	irq_set_chip_data(irq, domain->host_data);
+	irq_set_status_flags(irq, IRQ_LEVEL);
+
+	return 0;
+}
+
+/* INTx IRQ Domain operations */
+static const struct irq_domain_ops amd_intx_domain_ops = {
+	.map = amd_mdb_pcie_intx_map,
+};
+
+/**
+ * amd_mdb_pcie_init_port - Initialize hardware
+ * @pcie: PCIe port information
+ * @pdev: platform device
+ */
+static int amd_mdb_pcie_init_port(struct amd_mdb_pcie *pcie,
+				  struct platform_device *pdev)
+{
+	int val;
+
+	/* Disable all TLP Interrupts */
+	pcie_write(pcie, pcie_read(pcie, AMD_MDB_TLP_IR_ENABLE_MISC) &
+		   ~AMD_MDB_PCIE_IMR_ALL_MASK,
+		   AMD_MDB_TLP_IR_ENABLE_MISC);
+
+	/* Clear pending TLP interrupts */
+	pcie_write(pcie, pcie_read(pcie, AMD_MDB_TLP_IR_STATUS_MISC) &
+		   AMD_MDB_PCIE_IMR_ALL_MASK,
+		   AMD_MDB_TLP_IR_STATUS_MISC);
+
+	/* Enable all TLP Interrupts */
+	val = pcie_read(pcie, AMD_MDB_TLP_IR_ENABLE_MISC);
+	pcie_write(pcie, (val | AMD_MDB_PCIE_IMR_ALL_MASK),
+		   AMD_MDB_TLP_IR_ENABLE_MISC);
+
+	return 0;
+}
+
+static irqreturn_t amd_mdb_pcie_event_flow(int irq, void *args)
+{
+	struct amd_mdb_pcie *pcie = args;
+	unsigned long val;
+	int i;
+
+	val =  pcie_read(pcie, AMD_MDB_TLP_IR_STATUS_MISC);
+	val &= ~pcie_read(pcie, AMD_MDB_TLP_IR_MASK_MISC);
+	for_each_set_bit(i, &val, 32)
+		generic_handle_domain_irq(pcie->mdb_domain, i);
+	pcie_write(pcie, val, AMD_MDB_TLP_IR_STATUS_MISC);
+
+	return IRQ_HANDLED;
+}
+
+#define _IC(x, s)[AMD_MDB_PCIE_INTR_ ## x] = { __stringify(x), s }
+
+static const struct {
+	const char	*sym;
+	const char	*str;
+} intr_cause[32] = {
+	_IC(CMPL_TIMEOUT,	"completion timeout"),
+	_IC(PM_PME_RCVD,	"PM_PME message received"),
+	_IC(PME_TO_ACK_RCVD,	"PME_TO_ACK message received"),
+	_IC(MISC_CORRECTABLE,	"Correctable error message"),
+	_IC(NONFATAL,		"Non fatal error message"),
+	_IC(FATAL,		"Fatal error message"),
+};
+
+static void amd_mdb_mask_event_irq(struct irq_data *d)
+{
+	struct amd_mdb_pcie *pcie = irq_data_get_irq_chip_data(d);
+	struct dw_pcie *pci = &pcie->pci;
+	struct dw_pcie_rp *port = &pci->pp;
+	u32 val;
+
+	raw_spin_lock(&port->lock);
+	val = pcie_read(pcie, AMD_MDB_TLP_IR_STATUS_MISC);
+	val &= ~BIT(d->hwirq);
+	pcie_write(pcie, val, AMD_MDB_TLP_IR_STATUS_MISC);
+	raw_spin_unlock(&port->lock);
+}
+
+static void amd_mdb_unmask_event_irq(struct irq_data *d)
+{
+	struct amd_mdb_pcie *pcie = irq_data_get_irq_chip_data(d);
+	struct dw_pcie *pci = &pcie->pci;
+	struct dw_pcie_rp *port = &pci->pp;
+	u32 val;
+
+	raw_spin_lock(&port->lock);
+	val = pcie_read(pcie, AMD_MDB_TLP_IR_STATUS_MISC);
+	val |= BIT(d->hwirq);
+	pcie_write(pcie, val, AMD_MDB_TLP_IR_STATUS_MISC);
+	raw_spin_unlock(&port->lock);
+}
+
+static struct irq_chip amd_mdb_event_irq_chip = {
+	.name		= "RC-Event",
+	.irq_mask	= amd_mdb_mask_event_irq,
+	.irq_unmask	= amd_mdb_unmask_event_irq,
+};
+
+static int amd_mdb_pcie_event_map(struct irq_domain *domain,
+				  unsigned int irq, irq_hw_number_t hwirq)
+{
+	irq_set_chip_and_handler(irq, &amd_mdb_event_irq_chip,
+				 handle_level_irq);
+	irq_set_chip_data(irq, domain->host_data);
+	irq_set_status_flags(irq, IRQ_LEVEL);
+	return 0;
+}
+
+static const struct irq_domain_ops event_domain_ops = {
+	.map = amd_mdb_pcie_event_map,
+};
+
+static void amd_mdb_pcie_free_irq_domains(struct amd_mdb_pcie *pcie)
+{
+	if (pcie->intx_domain) {
+		irq_domain_remove(pcie->intx_domain);
+		pcie->intx_domain = NULL;
+	}
+
+	if (pcie->mdb_domain) {
+		irq_domain_remove(pcie->mdb_domain);
+		pcie->mdb_domain = NULL;
+	}
+}
+
+/**
+ * amd_mdb_pcie_init_irq_domains - Initialize IRQ domain
+ * @pcie: PCIe port information
+ * @pdev: platform device
+ * Return: '0' on success and error value on failure
+ */
+static int amd_mdb_pcie_init_irq_domains(struct amd_mdb_pcie *pcie,
+					 struct platform_device *pdev)
+{
+	struct dw_pcie *pci = &pcie->pci;
+	struct dw_pcie_rp *pp = &pci->pp;
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	struct device_node *pcie_intc_node;
+
+	/* Setup INTx */
+	pcie_intc_node = of_get_next_child(node, NULL);
+	if (!pcie_intc_node) {
+		dev_err(dev, "No PCIe Intc node found\n");
+		return -EINVAL;
+	}
+
+	pcie->mdb_domain = irq_domain_add_linear(pcie_intc_node, 32,
+						 &event_domain_ops,
+					       pcie);
+	if (!pcie->mdb_domain)
+		goto out;
+
+	irq_domain_update_bus_token(pcie->mdb_domain, DOMAIN_BUS_NEXUS);
+
+	pcie->intx_domain = irq_domain_add_linear(pcie_intc_node, PCI_NUM_INTX,
+						  &amd_intx_domain_ops, pcie);
+	if (!pcie->intx_domain)
+		goto mdb_out;
+
+	irq_domain_update_bus_token(pcie->intx_domain, DOMAIN_BUS_WIRED);
+
+	of_node_put(pcie_intc_node);
+	raw_spin_lock_init(&pp->lock);
+
+	return 0;
+mdb_out:
+	amd_mdb_pcie_free_irq_domains(pcie);
+out:
+	of_node_put(pcie_intc_node);
+	dev_err(dev, "Failed to allocate IRQ domains\n");
+
+	return -ENOMEM;
+}
+
+static irqreturn_t amd_mdb_pcie_intr_handler(int irq, void *args)
+{
+	struct amd_mdb_pcie *pcie = args;
+	struct device *dev;
+	struct irq_data *d;
+
+	dev = pcie->pci.dev;
+
+	d = irq_domain_get_irq_data(pcie->mdb_domain, irq);
+	if (intr_cause[d->hwirq].str)
+		dev_warn(dev, "%s\n", intr_cause[d->hwirq].str);
+	else
+		dev_warn(dev, "Unknown IRQ %ld\n", d->hwirq);
+
+	return IRQ_HANDLED;
+}
+
+static int amd_mdb_setup_irq(struct amd_mdb_pcie *pcie,
+			     struct platform_device *pdev)
+{
+	struct dw_pcie *pci = &pcie->pci;
+	struct dw_pcie_rp *pp = &pci->pp;
+	struct device *dev = &pdev->dev;
+	int i, irq, err;
+
+	pp->irq = platform_get_irq(pdev, 0);
+	if (pp->irq < 0)
+		return pp->irq;
+
+	for (i = 0; i < ARRAY_SIZE(intr_cause); i++) {
+		if (!intr_cause[i].str)
+			continue;
+		irq = irq_create_mapping(pcie->mdb_domain, i);
+		if (!irq) {
+			dev_err(dev, "Failed to map mdb domain interrupt\n");
+			return -ENXIO;
+		}
+		err = devm_request_irq(dev, irq, amd_mdb_pcie_intr_handler,
+				       IRQF_SHARED | IRQF_NO_THREAD,
+				       intr_cause[i].sym, pcie);
+		if (err) {
+			dev_err(dev, "Failed to request IRQ %d\n", irq);
+			return err;
+		}
+	}
+
+	/* Plug the main event chained handler */
+	err = devm_request_irq(dev, pp->irq, amd_mdb_pcie_event_flow,
+			       IRQF_SHARED | IRQF_NO_THREAD, "pcie_irq", pcie);
+	if (err) {
+		dev_err(dev, "Failed to request event IRQ %d\n", pp->irq);
+		return err;
+	}
+
+	return 0;
+}
+
+static int amd_mdb_add_pcie_port(struct amd_mdb_pcie *pcie,
+				 struct platform_device *pdev)
+{
+	struct dw_pcie *pci = &pcie->pci;
+	struct dw_pcie_rp *pp = &pci->pp;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	pcie->mdb_base = devm_platform_ioremap_resource_byname(pdev, "mdb_pcie_slcr");
+	if (IS_ERR(pcie->mdb_base))
+		return PTR_ERR(pcie->mdb_base);
+
+	ret = amd_mdb_pcie_init_irq_domains(pcie, pdev);
+	if (ret)
+		return ret;
+
+	amd_mdb_pcie_init_port(pcie, pdev);
+
+	ret = amd_mdb_setup_irq(pcie, pdev);
+	if (ret) {
+		dev_err(dev, "Failed to set up interrupts\n");
+		goto out;
+	}
+
+	pp->ops = &amd_mdb_pcie_host_ops;
+
+	ret = dw_pcie_host_init(pp);
+	if (ret) {
+		dev_err(dev, "Failed to initialize host\n");
+		goto out;
+	}
+
+	return 0;
+
+out:
+	amd_mdb_pcie_free_irq_domains(pcie);
+	return ret;
+}
+
+static int amd_mdb_pcie_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct amd_mdb_pcie *pcie;
+	struct dw_pcie *pci;
+
+	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
+	if (!pcie)
+		return -ENOMEM;
+
+	pci = &pcie->pci;
+	pci->dev = dev;
+
+	platform_set_drvdata(pdev, pcie);
+
+	return amd_mdb_add_pcie_port(pcie, pdev);
+}
+
+static const struct of_device_id amd_mdb_pcie_of_match[] = {
+	{
+		.compatible = "amd,versal2-mdb-host",
+	},
+	{},
+};
+
+static struct platform_driver amd_mdb_pcie_driver = {
+	.driver = {
+		.name	= "amd-mdb-pcie",
+		.of_match_table = amd_mdb_pcie_of_match,
+		.suppress_bind_attrs = true,
+	},
+	.probe = amd_mdb_pcie_probe,
+};
+builtin_platform_driver(amd_mdb_pcie_driver);
-- 
2.34.1


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

* Re: [PATCH v2 1/2] dt-bindings: PCI: amd-mdb: Add AMD Versal2 MDB PCIe Root Port Bridge
  2024-12-03 12:36 ` [PATCH v2 1/2] dt-bindings: PCI: amd-mdb: Add AMD Versal2 MDB PCIe Root Port Bridge Thippeswamy Havalige
@ 2024-12-03 13:24   ` Rob Herring (Arm)
  2024-12-03 14:41   ` Rob Herring
  1 sibling, 0 replies; 7+ messages in thread
From: Rob Herring (Arm) @ 2024-12-03 13:24 UTC (permalink / raw)
  To: Thippeswamy Havalige
  Cc: linux-kernel, manivannan.sadhasivam, bharat.kumar.gogada,
	lpieralisi, linux-pci, krzk+dt, conor+dt, bhelgaas, devicetree,
	michal.simek, kw, jingoohan1


On Tue, 03 Dec 2024 18:06:07 +0530, Thippeswamy Havalige wrote:
> Add AMD Versal2 MDB (Multimedia DMA Bridge) PCIe Root Port Bridge.
> 
> Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
> ---
> Changes in v2:
> -------------
> - Modify patch subject.
> - Add pcie host bridge reference.
> - Modify filename as per compatible string.
> - Remove standard PCI properties.
> - Modify interrupt controller description.
> - Indentation
> ---
>  .../bindings/pci/amd,versal2-mdb-host.yaml    | 132 ++++++++++++++++++
>  1 file changed, 132 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml
> 

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

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml: $id: Cannot determine base path from $id, relative path/filename doesn't match actual path or filename
 	 $id: http://devicetree.org/schemas/pci/amd,mdb-pcie.yaml
 	file: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241203123608.2944662-2-thippeswamy.havalige@amd.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


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

* Re: [PATCH v2 1/2] dt-bindings: PCI: amd-mdb: Add AMD Versal2 MDB PCIe Root Port Bridge
  2024-12-03 12:36 ` [PATCH v2 1/2] dt-bindings: PCI: amd-mdb: Add AMD Versal2 MDB PCIe Root Port Bridge Thippeswamy Havalige
  2024-12-03 13:24   ` Rob Herring (Arm)
@ 2024-12-03 14:41   ` Rob Herring
  2024-12-04  6:32     ` Havalige, Thippeswamy
  1 sibling, 1 reply; 7+ messages in thread
From: Rob Herring @ 2024-12-03 14:41 UTC (permalink / raw)
  To: Thippeswamy Havalige
  Cc: bhelgaas, lpieralisi, kw, manivannan.sadhasivam, krzk+dt,
	conor+dt, linux-pci, devicetree, linux-kernel, jingoohan1,
	michal.simek, bharat.kumar.gogada

On Tue, Dec 03, 2024 at 06:06:07PM +0530, Thippeswamy Havalige wrote:
> Add AMD Versal2 MDB (Multimedia DMA Bridge) PCIe Root Port Bridge.
> 
> Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
> ---
> Changes in v2:
> -------------
> - Modify patch subject.
> - Add pcie host bridge reference.
> - Modify filename as per compatible string.
> - Remove standard PCI properties.
> - Modify interrupt controller description.
> - Indentation
> ---
>  .../bindings/pci/amd,versal2-mdb-host.yaml    | 132 ++++++++++++++++++
>  1 file changed, 132 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml b/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml
> new file mode 100644
> index 000000000000..75795bab8254
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml
> @@ -0,0 +1,132 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/amd,mdb-pcie.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: AMD Versal2 MDB(Multimedia DMA Bridge) Host Controller
> +
> +maintainers:
> +  - Thippeswamy Havalige <thippeswamy.havalige@amd.com>
> +
> +allOf:
> +  - $ref: /schemas/pci/pci-host-bridge.yaml#
> +
> +properties:
> +  compatible:
> +    const: amd,versal2-mdb-host
> +
> +  reg:
> +    items:
> +      - description: MDB PCIe controller 0 SLCR

SLCR is not defined anywhere.

> +      - description: configuration region
> +      - description: data bus interface
> +      - description: address translation unit register
> +
> +  reg-names:
> +    items:
> +      - const: mdb_pcie_slcr
> +      - const: config
> +      - const: dbi
> +      - const: atu

DWC based it seems. You need to reference the DWC schema.

> +
> +  ranges:
> +    maxItems: 2
> +
> +  msi-map:
> +    maxItems: 1
> +
> +  bus-range:
> +    maxItems: 1

Already defined in the common schema. Plus you obviously didn't test 
anything with this because bus-range must be exactly 2 entries. 1 is not 
valid.

> +
> +  "#address-cells":
> +    const: 3
> +
> +  "#size-cells":
> +    const: 2

Both of these are also already defined in the pci-host-bridge.yaml.

> +
> +  interrupts:
> +    maxItems: 1
> +
> +  interrupt-map-mask:
> +    items:
> +      - const: 0
> +      - const: 0
> +      - const: 0
> +      - const: 7
> +
> +  interrupt-map:
> +    maxItems: 4
> +
> +  "#interrupt-cells":
> +    const: 1
> +
> +  interrupt-controller:
> +    description: identifies the node as an interrupt controller
> +    type: object
> +    properties:
> +      interrupt-controller: true
> +
> +      "#address-cells":
> +        const: 0
> +
> +      "#interrupt-cells":
> +        const: 1
> +
> +    required:
> +      - interrupt-controller
> +      - "#address-cells"
> +      - "#interrupt-cells"
> +
> +    additionalProperties: false

Move this before 'properties'.

> +
> +required:
> +  - reg
> +  - reg-names
> +  - interrupts
> +  - interrupt-map
> +  - interrupt-map-mask
> +  - msi-map
> +  - ranges

Already required by common schema.

> +  - "#interrupt-cells"
> +  - interrupt-controller
> +
> +unevaluatedProperties: false
> +
> +examples:
> +

Drop blank line.

> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +        pci@ed931000 {

pcie@...

> +            compatible = "amd,versal2-mdb-host";
> +            reg = <0x0 0xed931000 0x0 0x2000>,
> +                  <0x1000 0x100000 0x0 0xff00000>,
> +                  <0x1000 0x0 0x0 0x100000>,
> +                  <0x0 0xed860000 0x0 0x2000>;
> +            reg-names = "mdb_pcie_slcr", "config", "dbi", "atu";
> +            ranges = <0x2000000 0x00 0xa8000000 0x00 0xa8000000 0x00 0x10000000>,
> +                     <0x43000000 0x1100 0x00 0x1100 0x00 0x00 0x1000000>;
> +            interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupt-parent = <&gic>;
> +            interrupt-map-mask = <0 0 0 7>;
> +            interrupt-map = <0 0 0 1 &pcie_intc_0 0>,
> +                            <0 0 0 2 &pcie_intc_0 1>,
> +                            <0 0 0 3 &pcie_intc_0 2>,
> +                            <0 0 0 4 &pcie_intc_0 3>;
> +            msi-map = <0x0 &gic_its 0x00 0x10000>;
> +            #address-cells = <3>;
> +            #size-cells = <2>;
> +            #interrupt-cells = <1>;
> +            device_type = "pci";
> +            pcie_intc_0: interrupt-controller {
> +                #address-cells = <0>;
> +                #interrupt-cells = <1>;
> +                interrupt-controller;
> +           };
> +        };
> +    };
> -- 
> 2.34.1
> 

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

* RE: [PATCH v2 1/2] dt-bindings: PCI: amd-mdb: Add AMD Versal2 MDB PCIe Root Port Bridge
  2024-12-03 14:41   ` Rob Herring
@ 2024-12-04  6:32     ` Havalige, Thippeswamy
  2024-12-10  6:37       ` Havalige, Thippeswamy
  0 siblings, 1 reply; 7+ messages in thread
From: Havalige, Thippeswamy @ 2024-12-04  6:32 UTC (permalink / raw)
  To: Rob Herring
  Cc: bhelgaas@google.com, lpieralisi@kernel.org, kw@linux.com,
	manivannan.sadhasivam@linaro.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, linux-pci@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	jingoohan1@gmail.com, Simek, Michal, Gogada, Bharat Kumar

Hi Bjorn,

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Tuesday, December 3, 2024 8:11 PM
> To: Havalige, Thippeswamy <thippeswamy.havalige@amd.com>
> Cc: bhelgaas@google.com; lpieralisi@kernel.org; kw@linux.com;
> manivannan.sadhasivam@linaro.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; jingoohan1@gmail.com; Simek, Michal
> <michal.simek@amd.com>; Gogada, Bharat Kumar
> <bharat.kumar.gogada@amd.com>
> Subject: Re: [PATCH v2 1/2] dt-bindings: PCI: amd-mdb: Add AMD Versal2 MDB
> PCIe Root Port Bridge
> 
> On Tue, Dec 03, 2024 at 06:06:07PM +0530, Thippeswamy Havalige wrote:
> > Add AMD Versal2 MDB (Multimedia DMA Bridge) PCIe Root Port Bridge.
> >
> > Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
> > ---
> > Changes in v2:
> > -------------
> > - Modify patch subject.
> > - Add pcie host bridge reference.
> > - Modify filename as per compatible string.
> > - Remove standard PCI properties.
> > - Modify interrupt controller description.
> > - Indentation
> > ---
> >  .../bindings/pci/amd,versal2-mdb-host.yaml    | 132 ++++++++++++++++++
> >  1 file changed, 132 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml
> > b/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml
> > new file mode 100644
> > index 000000000000..75795bab8254
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml
> > @@ -0,0 +1,132 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pci/amd,mdb-pcie.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: AMD Versal2 MDB(Multimedia DMA Bridge) Host Controller
> > +
> > +maintainers:
> > +  - Thippeswamy Havalige <thippeswamy.havalige@amd.com>
> > +
> > +allOf:
> > +  - $ref: /schemas/pci/pci-host-bridge.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    const: amd,versal2-mdb-host
> > +
> > +  reg:
> > +    items:
> > +      - description: MDB PCIe controller 0 SLCR
> 
> SLCR is not defined anywhere.
Thanks for review, Here SLCR refers to mdb_pcie_slcr should I modify it to lower case? 
> 
> > +      - description: configuration region
> > +      - description: data bus interface
> > +      - description: address translation unit register
> > +
> > +  reg-names:
> > +    items:
> > +      - const: mdb_pcie_slcr
> > +      - const: config
> > +      - const: dbi
> > +      - const: atu
> 
> DWC based it seems. You need to reference the DWC schema.
- Thanks for the review, Here should I add both dwc and pci-host-bridge host bridge schema.
> > +
> > +  ranges:
> > +    maxItems: 2
> > +
> > +  msi-map:
> > +    maxItems: 1
> > +
> > +  bus-range:
> > +    maxItems: 1
> 
> Already defined in the common schema. Plus you obviously didn't test anything with
> this because bus-range must be exactly 2 entries. 1 is not valid.
- Thanks for the review, Will remove it in next patch.
> 
> > +
> > +  "#address-cells":
> > +    const: 3
> > +
> > +  "#size-cells":
> > +    const: 2
> 
> Both of these are also already defined in the pci-host-bridge.yaml.
Thanks for the review, Will update in next patch.
> 
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  interrupt-map-mask:
> > +    items:
> > +      - const: 0
> > +      - const: 0
> > +      - const: 0
> > +      - const: 7
> > +
> > +  interrupt-map:
> > +    maxItems: 4
> > +
> > +  "#interrupt-cells":
> > +    const: 1
> > +
> > +  interrupt-controller:
> > +    description: identifies the node as an interrupt controller
> > +    type: object
> > +    properties:
> > +      interrupt-controller: true
> > +
> > +      "#address-cells":
> > +        const: 0
> > +
> > +      "#interrupt-cells":
> > +        const: 1
> > +
> > +    required:
> > +      - interrupt-controller
> > +      - "#address-cells"
> > +      - "#interrupt-cells"
> > +
> > +    additionalProperties: false
> 
> Move this before 'properties'.
- Thanks for the review, I will update in next patch.
> 
> > +
> > +required:
> > +  - reg
> > +  - reg-names
> > +  - interrupts
> > +  - interrupt-map
> > +  - interrupt-map-mask
> > +  - msi-map
> > +  - ranges
> 
> Already required by common schema.
Thanks for the review, will update in next patch.
> 
> > +  - "#interrupt-cells"
> > +  - interrupt-controller
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +
> 
> Drop blank line.
Thanks for the review, will update in next patch.
> 
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +
> > +    soc {
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;
> > +        pci@ed931000 {
> 
> pcie@...
Thanks for the review, will update in next patch.
> 
> > +            compatible = "amd,versal2-mdb-host";
> > +            reg = <0x0 0xed931000 0x0 0x2000>,
> > +                  <0x1000 0x100000 0x0 0xff00000>,
> > +                  <0x1000 0x0 0x0 0x100000>,
> > +                  <0x0 0xed860000 0x0 0x2000>;
> > +            reg-names = "mdb_pcie_slcr", "config", "dbi", "atu";
> > +            ranges = <0x2000000 0x00 0xa8000000 0x00 0xa8000000 0x00
> 0x10000000>,
> > +                     <0x43000000 0x1100 0x00 0x1100 0x00 0x00 0x1000000>;
> > +            interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>;
> > +            interrupt-parent = <&gic>;
> > +            interrupt-map-mask = <0 0 0 7>;
> > +            interrupt-map = <0 0 0 1 &pcie_intc_0 0>,
> > +                            <0 0 0 2 &pcie_intc_0 1>,
> > +                            <0 0 0 3 &pcie_intc_0 2>,
> > +                            <0 0 0 4 &pcie_intc_0 3>;
> > +            msi-map = <0x0 &gic_its 0x00 0x10000>;
> > +            #address-cells = <3>;
> > +            #size-cells = <2>;
> > +            #interrupt-cells = <1>;
> > +            device_type = "pci";
> > +            pcie_intc_0: interrupt-controller {
> > +                #address-cells = <0>;
> > +                #interrupt-cells = <1>;
> > +                interrupt-controller;
> > +           };
> > +        };
> > +    };
> > --
> > 2.34.1
> >
Regards,
Thippeswamy H

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

* RE: [PATCH v2 1/2] dt-bindings: PCI: amd-mdb: Add AMD Versal2 MDB PCIe Root Port Bridge
  2024-12-04  6:32     ` Havalige, Thippeswamy
@ 2024-12-10  6:37       ` Havalige, Thippeswamy
  0 siblings, 0 replies; 7+ messages in thread
From: Havalige, Thippeswamy @ 2024-12-10  6:37 UTC (permalink / raw)
  To: Rob Herring
  Cc: bhelgaas@google.com, lpieralisi@kernel.org, kw@linux.com,
	manivannan.sadhasivam@linaro.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, linux-pci@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	jingoohan1@gmail.com, Simek, Michal, Gogada, Bharat Kumar

Hi Robh,

Thank you for your comments. I mistakenly mentioned Bjorn in my earlier message.
My apologies for the oversight.

Regards,
Thippeswamy H

> -----Original Message-----
> From: Havalige, Thippeswamy
> Sent: Wednesday, December 4, 2024 12:02 PM
> To: Rob Herring <robh@kernel.org>
> Cc: bhelgaas@google.com; lpieralisi@kernel.org; kw@linux.com;
> manivannan.sadhasivam@linaro.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; jingoohan1@gmail.com; Simek, Michal
> <michal.simek@amd.com>; Gogada, Bharat Kumar
> <bharat.kumar.gogada@amd.com>
> Subject: RE: [PATCH v2 1/2] dt-bindings: PCI: amd-mdb: Add AMD Versal2 MDB
> PCIe Root Port Bridge
> 
> Hi Bjorn,
> 
> > -----Original Message-----
> > From: Rob Herring <robh@kernel.org>
> > Sent: Tuesday, December 3, 2024 8:11 PM
> > To: Havalige, Thippeswamy <thippeswamy.havalige@amd.com>
> > Cc: bhelgaas@google.com; lpieralisi@kernel.org; kw@linux.com;
> > manivannan.sadhasivam@linaro.org; krzk+dt@kernel.org;
> > conor+dt@kernel.org; linux-pci@vger.kernel.org;
> > devicetree@vger.kernel.org; linux- kernel@vger.kernel.org;
> > jingoohan1@gmail.com; Simek, Michal <michal.simek@amd.com>; Gogada,
> > Bharat Kumar <bharat.kumar.gogada@amd.com>
> > Subject: Re: [PATCH v2 1/2] dt-bindings: PCI: amd-mdb: Add AMD Versal2
> > MDB PCIe Root Port Bridge
> >
> > On Tue, Dec 03, 2024 at 06:06:07PM +0530, Thippeswamy Havalige wrote:
> > > Add AMD Versal2 MDB (Multimedia DMA Bridge) PCIe Root Port Bridge.
> > >
> > > Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
> > > ---
> > > Changes in v2:
> > > -------------
> > > - Modify patch subject.
> > > - Add pcie host bridge reference.
> > > - Modify filename as per compatible string.
> > > - Remove standard PCI properties.
> > > - Modify interrupt controller description.
> > > - Indentation
> > > ---
> > >  .../bindings/pci/amd,versal2-mdb-host.yaml    | 132 ++++++++++++++++++
> > >  1 file changed, 132 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml
> > > b/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml
> > > new file mode 100644
> > > index 000000000000..75795bab8254
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yam
> > > +++ l
> > > @@ -0,0 +1,132 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/pci/amd,mdb-pcie.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: AMD Versal2 MDB(Multimedia DMA Bridge) Host Controller
> > > +
> > > +maintainers:
> > > +  - Thippeswamy Havalige <thippeswamy.havalige@amd.com>
> > > +
> > > +allOf:
> > > +  - $ref: /schemas/pci/pci-host-bridge.yaml#
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: amd,versal2-mdb-host
> > > +
> > > +  reg:
> > > +    items:
> > > +      - description: MDB PCIe controller 0 SLCR
> >
> > SLCR is not defined anywhere.
> Thanks for review, Here SLCR refers to mdb_pcie_slcr should I modify it to lower
> case?
> >
> > > +      - description: configuration region
> > > +      - description: data bus interface
> > > +      - description: address translation unit register
> > > +
> > > +  reg-names:
> > > +    items:
> > > +      - const: mdb_pcie_slcr
> > > +      - const: config
> > > +      - const: dbi
> > > +      - const: atu
> >
> > DWC based it seems. You need to reference the DWC schema.
> - Thanks for the review, Here should I add both dwc and pci-host-bridge host bridge
> schema.
> > > +
> > > +  ranges:
> > > +    maxItems: 2
> > > +
> > > +  msi-map:
> > > +    maxItems: 1
> > > +
> > > +  bus-range:
> > > +    maxItems: 1
> >
> > Already defined in the common schema. Plus you obviously didn't test
> > anything with this because bus-range must be exactly 2 entries. 1 is not valid.
> - Thanks for the review, Will remove it in next patch.
> >
> > > +
> > > +  "#address-cells":
> > > +    const: 3
> > > +
> > > +  "#size-cells":
> > > +    const: 2
> >
> > Both of these are also already defined in the pci-host-bridge.yaml.
> Thanks for the review, Will update in next patch.
> >
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> > > +
> > > +  interrupt-map-mask:
> > > +    items:
> > > +      - const: 0
> > > +      - const: 0
> > > +      - const: 0
> > > +      - const: 7
> > > +
> > > +  interrupt-map:
> > > +    maxItems: 4
> > > +
> > > +  "#interrupt-cells":
> > > +    const: 1
> > > +
> > > +  interrupt-controller:
> > > +    description: identifies the node as an interrupt controller
> > > +    type: object
> > > +    properties:
> > > +      interrupt-controller: true
> > > +
> > > +      "#address-cells":
> > > +        const: 0
> > > +
> > > +      "#interrupt-cells":
> > > +        const: 1
> > > +
> > > +    required:
> > > +      - interrupt-controller
> > > +      - "#address-cells"
> > > +      - "#interrupt-cells"
> > > +
> > > +    additionalProperties: false
> >
> > Move this before 'properties'.
> - Thanks for the review, I will update in next patch.
> >
> > > +
> > > +required:
> > > +  - reg
> > > +  - reg-names
> > > +  - interrupts
> > > +  - interrupt-map
> > > +  - interrupt-map-mask
> > > +  - msi-map
> > > +  - ranges
> >
> > Already required by common schema.
> Thanks for the review, will update in next patch.
> >
> > > +  - "#interrupt-cells"
> > > +  - interrupt-controller
> > > +
> > > +unevaluatedProperties: false
> > > +
> > > +examples:
> > > +
> >
> > Drop blank line.
> Thanks for the review, will update in next patch.
> >
> > > +  - |
> > > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > +    #include <dt-bindings/interrupt-controller/irq.h>
> > > +
> > > +    soc {
> > > +        #address-cells = <2>;
> > > +        #size-cells = <2>;
> > > +        pci@ed931000 {
> >
> > pcie@...
> Thanks for the review, will update in next patch.
> >
> > > +            compatible = "amd,versal2-mdb-host";
> > > +            reg = <0x0 0xed931000 0x0 0x2000>,
> > > +                  <0x1000 0x100000 0x0 0xff00000>,
> > > +                  <0x1000 0x0 0x0 0x100000>,
> > > +                  <0x0 0xed860000 0x0 0x2000>;
> > > +            reg-names = "mdb_pcie_slcr", "config", "dbi", "atu";
> > > +            ranges = <0x2000000 0x00 0xa8000000 0x00 0xa8000000
> > > + 0x00
> > 0x10000000>,
> > > +                     <0x43000000 0x1100 0x00 0x1100 0x00 0x00 0x1000000>;
> > > +            interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>;
> > > +            interrupt-parent = <&gic>;
> > > +            interrupt-map-mask = <0 0 0 7>;
> > > +            interrupt-map = <0 0 0 1 &pcie_intc_0 0>,
> > > +                            <0 0 0 2 &pcie_intc_0 1>,
> > > +                            <0 0 0 3 &pcie_intc_0 2>,
> > > +                            <0 0 0 4 &pcie_intc_0 3>;
> > > +            msi-map = <0x0 &gic_its 0x00 0x10000>;
> > > +            #address-cells = <3>;
> > > +            #size-cells = <2>;
> > > +            #interrupt-cells = <1>;
> > > +            device_type = "pci";
> > > +            pcie_intc_0: interrupt-controller {
> > > +                #address-cells = <0>;
> > > +                #interrupt-cells = <1>;
> > > +                interrupt-controller;
> > > +           };
> > > +        };
> > > +    };
> > > --
> > > 2.34.1
> > >
> Regards,
> Thippeswamy H

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

end of thread, other threads:[~2024-12-10  6:37 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-03 12:36 [PATCH v2 0/2] Add support for AMD MDB IP as Root Port Thippeswamy Havalige
2024-12-03 12:36 ` [PATCH v2 1/2] dt-bindings: PCI: amd-mdb: Add AMD Versal2 MDB PCIe Root Port Bridge Thippeswamy Havalige
2024-12-03 13:24   ` Rob Herring (Arm)
2024-12-03 14:41   ` Rob Herring
2024-12-04  6:32     ` Havalige, Thippeswamy
2024-12-10  6:37       ` Havalige, Thippeswamy
2024-12-03 12:36 ` [PATCH v2 2/2] PCI: amd-mdb: Add AMD MDB Root Port driver Thippeswamy Havalige

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).