[PATCH v4 0/7] PCI: qcom: Fix higher MSI vectors handling

[PATCH v4 0/7] PCI: qcom: Fix higher MSI vectors handling

[Dmitry Baryshkov]

I have replied with my Tested-by to the patch at [2], which has landed
in the linux-next as the commit 20f1bfb8dd62 ("PCI: qcom:
Add support for handling MSIs from 8 endpoints"). However lately I
noticed that during the tests I still had 'pcie_pme=nomsi', so the
device was not forced to use higher MSI vectors.

After removing this option I noticed that hight MSI vectors are not
delivered on tested platforms. After additional research I stumbled upon
a patch in msm-4.14 ([1]), which describes that each group of MSI
vectors is mapped to the separate interrupt. Implement corresponding
mapping.

Patchseries dependecies: [2] (landed in pci-next) and [3] (for the
schema change).

Since we can not expect that other platforms will use multi-IRQ scheme
for MSI mapping (e.g. iMX and Tegra map all 256 MSI interrupts to single
IRQ), it's support is implemented directly in pcie-qcom rather than in
the core driver.

Changes since v3:
 - Reimplement MSI handling scheme in the Qualcomm host controller
   driver.

Changes since v2:
 - Fix and rephrase commit message for patch 2.

Changes since v1:
 - Split a huge patch into three patches as suggested by Bjorn Helgaas
 - snps,dw-pcie removal is now part of [3]

[1] https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/commit/671a3d5f129f4bfe477152292ada2194c8440d22
[2] https://lore.kernel.org/linux-arm-msm/20211214101319.25258-1-manivannan.sadhasivam@linaro.org/
[3] https://lore.kernel.org/linux-arm-msm/20220422211002.2012070-1-dmitry.baryshkov@linaro.org/

Dmitry Baryshkov (7):
  PCI: qcom: Revert "PCI: qcom: Add support for handling MSIs from 8
    endpoints"
  PCI: dwc: Correct msi_irq condition in dw_pcie_free_msi()
  PCI: dwc: Add msi_host_deinit callback
  PCI: dwc: Export several functions useful for MSI implentations
  PCI: qcom: Handle MSI IRQs properly
  dt-bindings: pci/qcom,pcie: support additional MSI interrupts
  arm64: dts: qcom: sm8250: provide additional MSI interrupts

 .../devicetree/bindings/pci/qcom,pcie.yaml    |  51 ++++++-
 arch/arm64/boot/dts/qcom/sm8250.dtsi          |  11 +-
 .../pci/controller/dwc/pcie-designware-host.c |  72 +++++----
 drivers/pci/controller/dwc/pcie-designware.h  |  12 ++
 drivers/pci/controller/dwc/pcie-qcom.c        | 138 +++++++++++++++++-
 5 files changed, 252 insertions(+), 32 deletions(-)

-- 
2.35.1

[PATCH v4 1/7] PCI: qcom: Revert "PCI: qcom: Add support for handling MSIs from 8 endpoints"

[Dmitry Baryshkov]

I have replied with my Tested-by to the patch at [2], which has landed
in the linux-next as the commit 20f1bfb8dd62 ("PCI: qcom:
Add support for handling MSIs from 8 endpoints"). However lately I
noticed that during the tests I still had 'pcie_pme=nomsi', so the
device was not forced to use higher MSI vectors.

After removing this option I noticed that hight MSI vectors are not
delivered on tested platforms. Additional research pointed to
a patch in msm-4.14 ([1]), which describes that each group of MSI
vectors is mapped to the separate interrupt.

Without these changes specifying num_verctors can lead to missing MSI
interrupts and thus to devices malfunction.

Fixes: 20f1bfb8dd62 ("PCI: qcom: Add support for handling MSIs from 8 endpoints")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index c940e67d831c..375f27ab9403 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1593,7 +1593,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 	pci->dev = dev;
 	pci->ops = &dw_pcie_ops;
 	pp = &pci->pp;
-	pp->num_vectors = MAX_MSI_IRQS;
 
 	pcie->pci = pci;
 
-- 
2.35.1

[PATCH v4 2/7] PCI: dwc: Correct msi_irq condition in dw_pcie_free_msi()

[Dmitry Baryshkov]

The subdrivers pass -ESOMETHING if they do not want the core to touch
MSI IRQ. dw_pcie_host_init() also checks if (msi_irq > 0) rather than
just if (msi_irq). So let's make dw_pcie_free_msi() also check that
msi_irq is greater than zero.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 2fa86f32d964..43d1d6116007 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -257,7 +257,7 @@ int dw_pcie_allocate_domains(struct pcie_port *pp)
 
 static void dw_pcie_free_msi(struct pcie_port *pp)
 {
-	if (pp->msi_irq)
+	if (pp->msi_irq > 0)
 		irq_set_chained_handler_and_data(pp->msi_irq, NULL, NULL);
 
 	irq_domain_remove(pp->msi_domain);
-- 
2.35.1

[PATCH v4 3/7] PCI: dwc: Add msi_host_deinit callback

[Dmitry Baryshkov]

Add msi_host_deinit() callback as a counterpart to msi_host_init(). It
will tear down MSI support in case host has to run host-specific ops.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 8 ++++++--
 drivers/pci/controller/dwc/pcie-designware.h      | 1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 43d1d6116007..92dcaeabe2bf 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -424,7 +424,9 @@ int dw_pcie_host_init(struct pcie_port *pp)
 		return 0;
 
 err_free_msi:
-	if (pp->has_msi_ctrl)
+	if (pp->ops->msi_host_deinit)
+		pp->ops->msi_host_deinit(pp);
+	else if (pp->has_msi_ctrl)
 		dw_pcie_free_msi(pp);
 	return ret;
 }
@@ -434,7 +436,9 @@ void dw_pcie_host_deinit(struct pcie_port *pp)
 {
 	pci_stop_root_bus(pp->bridge->bus);
 	pci_remove_root_bus(pp->bridge->bus);
-	if (pp->has_msi_ctrl)
+	if (pp->ops->msi_host_deinit)
+		pp->ops->msi_host_deinit(pp);
+	else if (pp->has_msi_ctrl)
 		dw_pcie_free_msi(pp);
 }
 EXPORT_SYMBOL_GPL(dw_pcie_host_deinit);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 7d6e9b7576be..e1c48b71e0d2 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -175,6 +175,7 @@ enum dw_pcie_device_mode {
 struct dw_pcie_host_ops {
 	int (*host_init)(struct pcie_port *pp);
 	int (*msi_host_init)(struct pcie_port *pp);
+	void (*msi_host_deinit)(struct pcie_port *pp);
 };
 
 struct pcie_port {
-- 
2.35.1

[PATCH v4 4/7] PCI: dwc: Export several functions useful for MSI implentations

[Dmitry Baryshkov]

Supporting multiple MSI interrupts on Qualcomm hardware would benefit
from having these functions being exported rather than static. Note that
both designware and qcom driver can not be built as modules, so no need
to use EXPORT_SYMBOL here.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../pci/controller/dwc/pcie-designware-host.c | 62 ++++++++++++-------
 drivers/pci/controller/dwc/pcie-designware.h  | 11 ++++
 2 files changed, 49 insertions(+), 24 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 92dcaeabe2bf..c3b8ab278a00 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -255,7 +255,39 @@ int dw_pcie_allocate_domains(struct pcie_port *pp)
 	return 0;
 }
 
-static void dw_pcie_free_msi(struct pcie_port *pp)
+int dw_pcie_allocate_msi(struct pcie_port *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	int ret;
+
+	ret = dw_pcie_allocate_domains(pp);
+	if (ret)
+		return ret;
+
+	if (pp->msi_irq > 0)
+		irq_set_chained_handler_and_data(pp->msi_irq,
+				dw_chained_msi_isr,
+				pp);
+
+	ret = dma_set_mask(pci->dev, DMA_BIT_MASK(32));
+	if (ret)
+		dev_warn(pci->dev, "Failed to set DMA mask to 32-bit. Devices with only 32-bit MSI support may not work properly\n");
+
+	pp->msi_data = dma_map_single_attrs(pci->dev, &pp->msi_msg,
+			sizeof(pp->msi_msg),
+			DMA_FROM_DEVICE,
+			DMA_ATTR_SKIP_CPU_SYNC);
+	ret = dma_mapping_error(pci->dev, pp->msi_data);
+	if (ret) {
+		dev_err(pci->dev, "Failed to map MSI data\n");
+		pp->msi_data = 0;
+		return ret;
+	}
+
+	return 0;
+}
+
+void dw_pcie_free_msi(struct pcie_port *pp)
 {
 	if (pp->msi_irq > 0)
 		irq_set_chained_handler_and_data(pp->msi_irq, NULL, NULL);
@@ -357,6 +389,9 @@ int dw_pcie_host_init(struct pcie_port *pp)
 			return -EINVAL;
 		}
 
+		/* this can be overridden by msi_host_init() if necessary */
+		pp->msi_irq_chip = &dw_pci_msi_bottom_irq_chip;
+
 		if (pp->ops->msi_host_init) {
 			ret = pp->ops->msi_host_init(pp);
 			if (ret < 0)
@@ -377,30 +412,9 @@ int dw_pcie_host_init(struct pcie_port *pp)
 				}
 			}
 
-			pp->msi_irq_chip = &dw_pci_msi_bottom_irq_chip;
-
-			ret = dw_pcie_allocate_domains(pp);
-			if (ret)
+			ret = dw_pcie_allocate_msi(pp);
+			if (ret < 0)
 				return ret;
-
-			if (pp->msi_irq > 0)
-				irq_set_chained_handler_and_data(pp->msi_irq,
-							    dw_chained_msi_isr,
-							    pp);
-
-			ret = dma_set_mask(pci->dev, DMA_BIT_MASK(32));
-			if (ret)
-				dev_warn(pci->dev, "Failed to set DMA mask to 32-bit. Devices with only 32-bit MSI support may not work properly\n");
-
-			pp->msi_data = dma_map_single_attrs(pci->dev, &pp->msi_msg,
-						      sizeof(pp->msi_msg),
-						      DMA_FROM_DEVICE,
-						      DMA_ATTR_SKIP_CPU_SYNC);
-			if (dma_mapping_error(pci->dev, pp->msi_data)) {
-				dev_err(pci->dev, "Failed to map MSI data\n");
-				pp->msi_data = 0;
-				goto err_free_msi;
-			}
 		}
 	}
 
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index e1c48b71e0d2..f72447f15dc5 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -374,6 +374,8 @@ void dw_pcie_host_deinit(struct pcie_port *pp);
 int dw_pcie_allocate_domains(struct pcie_port *pp);
 void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus, unsigned int devfn,
 				       int where);
+int dw_pcie_allocate_msi(struct pcie_port *pp);
+void dw_pcie_free_msi(struct pcie_port *pp);
 #else
 static inline irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
 {
@@ -403,6 +405,15 @@ static inline void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus,
 {
 	return NULL;
 }
+
+static int dw_pcie_allocate_msi(struct pcie_port *pp)
+{
+	return -EINVAL;
+}
+
+static void dw_pcie_free_msi(struct pcie_port *pp)
+{
+}
 #endif
 
 #ifdef CONFIG_PCIE_DW_EP
-- 
2.35.1

[PATCH v4 5/7] PCI: qcom: Handle MSI IRQs properly

[Dmitry Baryshkov]

On some of Qualcomm platforms each group of 32 MSI vectors is routed to the
separate GIC interrupt. Thus to receive higher MSI vectors properly,
add separate msi_host_init()/msi_host_deinit() handling additional host
IRQs.

Note, that if DT doesn't list extra MSI interrupts, the driver will limit
the amount of supported MSI vectors accordingly (to 32).

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 137 ++++++++++++++++++++++++-
 1 file changed, 136 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 375f27ab9403..ac16353ce5b3 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -194,6 +194,7 @@ struct qcom_pcie_ops {
 
 struct qcom_pcie_cfg {
 	const struct qcom_pcie_ops *ops;
+	unsigned int has_split_msi_irqs:1;
 	unsigned int pipe_clk_need_muxing:1;
 	unsigned int has_tbu_clk:1;
 	unsigned int has_ddrss_sf_tbu_clk:1;
@@ -209,6 +210,7 @@ struct qcom_pcie {
 	struct phy *phy;
 	struct gpio_desc *reset;
 	const struct qcom_pcie_cfg *cfg;
+	int msi_irqs[MAX_MSI_CTRLS];
 };
 
 #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
@@ -1387,6 +1389,124 @@ static int qcom_pcie_config_sid_sm8250(struct qcom_pcie *pcie)
 	return 0;
 }
 
+static void qcom_chained_msi_isr(struct irq_desc *desc)
+{
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	int irq = irq_desc_get_irq(desc);
+	struct pcie_port *pp;
+	int idx, pos;
+	unsigned long val;
+	u32 status, num_ctrls;
+	struct dw_pcie *pci;
+	struct qcom_pcie *pcie;
+
+	chained_irq_enter(chip, desc);
+
+	pp = irq_desc_get_handler_data(desc);
+	pci = to_dw_pcie_from_pp(pp);
+	pcie = to_qcom_pcie(pci);
+
+	/*
+	 * Unlike generic dw_handle_msi_irq we can determine, which group of
+	 * MSIs triggered the IRQ, so process just single group.
+	 */
+	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
+
+	for (idx = 0; idx < num_ctrls; idx++) {
+		if (pcie->msi_irqs[idx] == irq)
+			break;
+	}
+
+	if (WARN_ON_ONCE(unlikely(idx == num_ctrls)))
+		goto out;
+
+	status = dw_pcie_readl_dbi(pci, PCIE_MSI_INTR0_STATUS +
+				   (idx * MSI_REG_CTRL_BLOCK_SIZE));
+	if (!status)
+		goto out;
+
+	val = status;
+	pos = 0;
+	while ((pos = find_next_bit(&val, MAX_MSI_IRQS_PER_CTRL,
+				    pos)) != MAX_MSI_IRQS_PER_CTRL) {
+		generic_handle_domain_irq(pp->irq_domain,
+					  (idx * MAX_MSI_IRQS_PER_CTRL) +
+					  pos);
+		pos++;
+	}
+
+out:
+	chained_irq_exit(chip, desc);
+}
+
+static int qcom_pcie_msi_host_init(struct pcie_port *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct qcom_pcie *pcie = to_qcom_pcie(pci);
+	struct platform_device *pdev = to_platform_device(pci->dev);
+	char irq_name[] = "msiXXX";
+	unsigned int ctrl, num_ctrls;
+	int msi_irq, ret;
+
+	pp->msi_irq = -EINVAL;
+
+	/*
+	 * We provide our own implementation of MSI init/deinit, but rely on
+	 * using the rest of DWC MSI functionality.
+	 */
+	pp->has_msi_ctrl = true;
+
+	msi_irq = platform_get_irq_byname_optional(pdev, "msi");
+	if (msi_irq < 0) {
+		msi_irq = platform_get_irq(pdev, 0);
+		if (msi_irq < 0)
+			return msi_irq;
+	}
+
+	pcie->msi_irqs[0] = msi_irq;
+
+	for (num_ctrls = 1; num_ctrls < MAX_MSI_CTRLS; num_ctrls++) {
+		snprintf(irq_name, sizeof(irq_name), "msi%d", num_ctrls + 1);
+		msi_irq = platform_get_irq_byname_optional(pdev, irq_name);
+		if (msi_irq == -ENXIO)
+			break;
+
+		pcie->msi_irqs[num_ctrls] = msi_irq;
+	}
+
+	pp->num_vectors = num_ctrls * MAX_MSI_IRQS_PER_CTRL;
+	dev_info(&pdev->dev, "Using %d MSI vectors\n", pp->num_vectors);
+	for (ctrl = 0; ctrl < num_ctrls; ctrl++)
+		pp->irq_mask[ctrl] = ~0;
+
+	ret = dw_pcie_allocate_msi(pp);
+	if (ret)
+		return ret;
+
+	for (ctrl = 0; ctrl < num_ctrls; ctrl++)
+		irq_set_chained_handler_and_data(pcie->msi_irqs[ctrl],
+				qcom_chained_msi_isr,
+				pp);
+
+	return 0;
+}
+
+static void qcom_pcie_msi_host_deinit(struct pcie_port *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct qcom_pcie *pcie = to_qcom_pcie(pci);
+	unsigned int ctrl, num_ctrls;
+
+	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
+
+	for (ctrl = 0; ctrl < num_ctrls; ctrl++)
+		irq_set_chained_handler_and_data(pcie->msi_irqs[ctrl],
+				NULL,
+				NULL);
+
+	dw_pcie_free_msi(pp);
+}
+
 static int qcom_pcie_host_init(struct pcie_port *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -1435,6 +1555,12 @@ static const struct dw_pcie_host_ops qcom_pcie_dw_ops = {
 	.host_init = qcom_pcie_host_init,
 };
 
+static const struct dw_pcie_host_ops qcom_pcie_msi_dw_ops = {
+	.host_init = qcom_pcie_host_init,
+	.msi_host_init = qcom_pcie_msi_host_init,
+	.msi_host_deinit = qcom_pcie_msi_host_deinit,
+};
+
 /* Qcom IP rev.: 2.1.0	Synopsys IP rev.: 4.01a */
 static const struct qcom_pcie_ops ops_2_1_0 = {
 	.get_resources = qcom_pcie_get_resources_2_1_0,
@@ -1508,6 +1634,7 @@ static const struct qcom_pcie_cfg ipq8064_cfg = {
 
 static const struct qcom_pcie_cfg msm8996_cfg = {
 	.ops = &ops_2_3_2,
+	.has_split_msi_irqs = true,
 };
 
 static const struct qcom_pcie_cfg ipq8074_cfg = {
@@ -1520,6 +1647,7 @@ static const struct qcom_pcie_cfg ipq4019_cfg = {
 
 static const struct qcom_pcie_cfg sdm845_cfg = {
 	.ops = &ops_2_7_0,
+	.has_split_msi_irqs = true,
 	.has_tbu_clk = true,
 };
 
@@ -1532,12 +1660,14 @@ static const struct qcom_pcie_cfg sm8150_cfg = {
 
 static const struct qcom_pcie_cfg sm8250_cfg = {
 	.ops = &ops_1_9_0,
+	.has_split_msi_irqs = true,
 	.has_tbu_clk = true,
 	.has_ddrss_sf_tbu_clk = true,
 };
 
 static const struct qcom_pcie_cfg sm8450_pcie0_cfg = {
 	.ops = &ops_1_9_0,
+	.has_split_msi_irqs = true,
 	.has_ddrss_sf_tbu_clk = true,
 	.pipe_clk_need_muxing = true,
 	.has_aggre0_clk = true,
@@ -1546,6 +1676,7 @@ static const struct qcom_pcie_cfg sm8450_pcie0_cfg = {
 
 static const struct qcom_pcie_cfg sm8450_pcie1_cfg = {
 	.ops = &ops_1_9_0,
+	.has_split_msi_irqs = true,
 	.has_ddrss_sf_tbu_clk = true,
 	.pipe_clk_need_muxing = true,
 	.has_aggre1_clk = true,
@@ -1553,6 +1684,7 @@ static const struct qcom_pcie_cfg sm8450_pcie1_cfg = {
 
 static const struct qcom_pcie_cfg sc7280_cfg = {
 	.ops = &ops_1_9_0,
+	.has_split_msi_irqs = true,
 	.has_tbu_clk = true,
 	.pipe_clk_need_muxing = true,
 };
@@ -1626,7 +1758,10 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_pm_runtime_put;
 
-	pp->ops = &qcom_pcie_dw_ops;
+	if (pcie->cfg->has_split_msi_irqs)
+		pp->ops = &qcom_pcie_msi_dw_ops;
+	else
+		pp->ops = &qcom_pcie_dw_ops;
 
 	ret = phy_init(pcie->phy);
 	if (ret) {
-- 
2.35.1

[PATCH v4 6/7] dt-bindings: pci/qcom,pcie: support additional MSI interrupts

[Dmitry Baryshkov]

On Qualcomm platforms each group of 32 MSI vectors is routed to the
separate GIC interrupt. Document mapping of additional interrupts.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../devicetree/bindings/pci/qcom,pcie.yaml    | 51 ++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index 0b69b12b849e..a8f99bca389e 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -43,11 +43,20 @@ properties:
     maxItems: 5
 
   interrupts:
-    maxItems: 1
+    minItems: 1
+    maxItems: 8
 
   interrupt-names:
+    minItems: 1
     items:
       - const: msi
+      - const: msi2
+      - const: msi3
+      - const: msi4
+      - const: msi5
+      - const: msi6
+      - const: msi7
+      - const: msi8
 
   # Common definitions for clocks, clock-names and reset.
   # Platform constraints are described later.
@@ -623,6 +632,46 @@ allOf:
         - resets
         - reset-names
 
+    # On newer chipsets support either 1 or 8 msi interrupts
+    # On older chipsets it's always 1 msi interrupt
+  - if:
+      properties:
+        compatibles:
+          contains:
+            enum:
+              - qcom,pcie-msm8996
+              - qcom,pcie-sc7280
+              - qcom,pcie-sc8180x
+              - qcom,pcie-sdm845
+              - qcom,pcie-sm8150
+              - qcom,pcie-sm8250
+              - qcom,pcie-sm8450-pcie0
+              - qcom,pcie-sm8450-pcie1
+    then:
+      oneOf:
+        - properties:
+            interrupts:
+              minItems: 1
+              maxItems: 1
+            interrupt-names:
+              minItems: 1
+              maxItems: 1
+        - properties:
+            interrupts:
+              minItems: 8
+              maxItems: 8
+            interrupt-names:
+              minItems: 8
+              maxItems: 8
+    else:
+      properties:
+        interrupts:
+          minItems: 1
+          maxItems: 1
+        interrupt-names:
+          minItems: 1
+          maxItems: 1
+
 unevaluatedProperties: false
 
 examples:
-- 
2.35.1

Re: [PATCH v4 6/7] dt-bindings: pci/qcom,pcie: support additional MSI interrupts

[Krzysztof Kozlowski]

On 28/04/2022 13:59, Dmitry Baryshkov wrote:
> On Qualcomm platforms each group of 32 MSI vectors is routed to the
> separate GIC interrupt. Document mapping of additional interrupts.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../devicetree/bindings/pci/qcom,pcie.yaml    | 51 ++++++++++++++++++-
>  1 file changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index 0b69b12b849e..a8f99bca389e 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -43,11 +43,20 @@ properties:
>      maxItems: 5
>  
>    interrupts:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 8
>  
>    interrupt-names:
> +    minItems: 1
>      items:
>        - const: msi
> +      - const: msi2
> +      - const: msi3
> +      - const: msi4
> +      - const: msi5
> +      - const: msi6
> +      - const: msi7
> +      - const: msi8
>  
>    # Common definitions for clocks, clock-names and reset.
>    # Platform constraints are described later.
> @@ -623,6 +632,46 @@ allOf:
>          - resets
>          - reset-names
>  
> +    # On newer chipsets support either 1 or 8 msi interrupts
> +    # On older chipsets it's always 1 msi interrupt
> +  - if:
> +      properties:
> +        compatibles:
> +          contains:
> +            enum:
> +              - qcom,pcie-msm8996
> +              - qcom,pcie-sc7280
> +              - qcom,pcie-sc8180x
> +              - qcom,pcie-sdm845
> +              - qcom,pcie-sm8150
> +              - qcom,pcie-sm8250
> +              - qcom,pcie-sm8450-pcie0
> +              - qcom,pcie-sm8450-pcie1
> +    then:
> +      oneOf:
> +        - properties:
> +            interrupts:
> +              minItems: 1

minItems should not be needed here and in places below, because it is
equal to maxItems.

> +              maxItems: 1
> +            interrupt-names:
> +              minItems: 1
> +              maxItems: 1
> +        - properties:
> +            interrupts:
> +              minItems: 8
> +              maxItems: 8
> +            interrupt-names:
> +              minItems: 8
> +              maxItems: 8
> +    else:
> +      properties:
> +        interrupts:
> +          minItems: 1
> +          maxItems: 1
> +        interrupt-names:
> +          minItems: 1
> +          maxItems: 1
> +
>  unevaluatedProperties: false
>  
>  examples:


Best regards,
Krzysztof

Re: [PATCH v4 6/7] dt-bindings: pci/qcom,pcie: support additional MSI interrupts

[Dmitry Baryshkov]

On Thu, 28 Apr 2022 at 15:08, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 28/04/2022 13:59, Dmitry Baryshkov wrote:
> > On Qualcomm platforms each group of 32 MSI vectors is routed to the
> > separate GIC interrupt. Document mapping of additional interrupts.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  .../devicetree/bindings/pci/qcom,pcie.yaml    | 51 ++++++++++++++++++-
> >  1 file changed, 50 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > index 0b69b12b849e..a8f99bca389e 100644
> > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > @@ -43,11 +43,20 @@ properties:
> >      maxItems: 5
> >
> >    interrupts:
> > -    maxItems: 1
> > +    minItems: 1
> > +    maxItems: 8
> >
> >    interrupt-names:
> > +    minItems: 1
> >      items:
> >        - const: msi
> > +      - const: msi2
> > +      - const: msi3
> > +      - const: msi4
> > +      - const: msi5
> > +      - const: msi6
> > +      - const: msi7
> > +      - const: msi8
> >
> >    # Common definitions for clocks, clock-names and reset.
> >    # Platform constraints are described later.
> > @@ -623,6 +632,46 @@ allOf:
> >          - resets
> >          - reset-names
> >
> > +    # On newer chipsets support either 1 or 8 msi interrupts
> > +    # On older chipsets it's always 1 msi interrupt
> > +  - if:
> > +      properties:
> > +        compatibles:
> > +          contains:
> > +            enum:
> > +              - qcom,pcie-msm8996
> > +              - qcom,pcie-sc7280
> > +              - qcom,pcie-sc8180x
> > +              - qcom,pcie-sdm845
> > +              - qcom,pcie-sm8150
> > +              - qcom,pcie-sm8250
> > +              - qcom,pcie-sm8450-pcie0
> > +              - qcom,pcie-sm8450-pcie1
> > +    then:
> > +      oneOf:
> > +        - properties:
> > +            interrupts:
> > +              minItems: 1
>
> minItems should not be needed here and in places below, because it is
> equal to maxItems.

Maybe it's a misunderstanding from my side. In the top level we have
the min = 1, max = 8.
How does that interfere with these entries? In other words, if we e.g.
omit minItems here, which setting would preveal: implicit minItems = 8
(from maxItems = 8) or minItems = 1 in the top level?

> > +              maxItems: 1
> > +            interrupt-names:
> > +              minItems: 1
> > +              maxItems: 1
> > +        - properties:
> > +            interrupts:
> > +              minItems: 8
> > +              maxItems: 8
> > +            interrupt-names:
> > +              minItems: 8
> > +              maxItems: 8
> > +    else:
> > +      properties:
> > +        interrupts:
> > +          minItems: 1
> > +          maxItems: 1
> > +        interrupt-names:
> > +          minItems: 1
> > +          maxItems: 1
> > +
> >  unevaluatedProperties: false
> >
> >  examples:
>
>
> Best regards,
> Krzysztof



-- 
With best wishes
Dmitry

Re: [PATCH v4 6/7] dt-bindings: pci/qcom,pcie: support additional MSI interrupts

[Krzysztof Kozlowski]

On 28/04/2022 15:57, Dmitry Baryshkov wrote:
> On Thu, 28 Apr 2022 at 15:08, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 28/04/2022 13:59, Dmitry Baryshkov wrote:
>>> On Qualcomm platforms each group of 32 MSI vectors is routed to the
>>> separate GIC interrupt. Document mapping of additional interrupts.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>>  .../devicetree/bindings/pci/qcom,pcie.yaml    | 51 ++++++++++++++++++-
>>>  1 file changed, 50 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>> index 0b69b12b849e..a8f99bca389e 100644
>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>> @@ -43,11 +43,20 @@ properties:
>>>      maxItems: 5
>>>
>>>    interrupts:
>>> -    maxItems: 1
>>> +    minItems: 1
>>> +    maxItems: 8
>>>
>>>    interrupt-names:
>>> +    minItems: 1
>>>      items:
>>>        - const: msi
>>> +      - const: msi2
>>> +      - const: msi3
>>> +      - const: msi4
>>> +      - const: msi5
>>> +      - const: msi6
>>> +      - const: msi7
>>> +      - const: msi8
>>>
>>>    # Common definitions for clocks, clock-names and reset.
>>>    # Platform constraints are described later.
>>> @@ -623,6 +632,46 @@ allOf:
>>>          - resets
>>>          - reset-names
>>>
>>> +    # On newer chipsets support either 1 or 8 msi interrupts
>>> +    # On older chipsets it's always 1 msi interrupt
>>> +  - if:
>>> +      properties:
>>> +        compatibles:
>>> +          contains:
>>> +            enum:
>>> +              - qcom,pcie-msm8996
>>> +              - qcom,pcie-sc7280
>>> +              - qcom,pcie-sc8180x
>>> +              - qcom,pcie-sdm845
>>> +              - qcom,pcie-sm8150
>>> +              - qcom,pcie-sm8250
>>> +              - qcom,pcie-sm8450-pcie0
>>> +              - qcom,pcie-sm8450-pcie1
>>> +    then:
>>> +      oneOf:
>>> +        - properties:
>>> +            interrupts:
>>> +              minItems: 1
>>
>> minItems should not be needed here and in places below, because it is
>> equal to maxItems.
> 
> Maybe it's a misunderstanding from my side. In the top level we have
> the min = 1, max = 8.
> How does that interfere with these entries? In other words, if we e.g.
> omit minItems here, which setting would preveal: implicit minItems = 8
> (from maxItems = 8) or minItems = 1 in the top level?
> 
>>> +              maxItems: 1

I don't propose to skip it for the case with maxItems:8, but only here.
minItems:1 is set in toplevel. Where is that implicit minItems:8?


Best regards,
Krzysztof

Re: [PATCH v4 6/7] dt-bindings: pci/qcom,pcie: support additional MSI interrupts

[Dmitry Baryshkov]

On 28/04/2022 17:06, Krzysztof Kozlowski wrote:
> On 28/04/2022 15:57, Dmitry Baryshkov wrote:
>> On Thu, 28 Apr 2022 at 15:08, Krzysztof Kozlowski
>> <krzysztof.kozlowski@linaro.org> wrote:
>>>
>>> On 28/04/2022 13:59, Dmitry Baryshkov wrote:
>>>> On Qualcomm platforms each group of 32 MSI vectors is routed to the
>>>> separate GIC interrupt. Document mapping of additional interrupts.
>>>>
>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>> ---
>>>>   .../devicetree/bindings/pci/qcom,pcie.yaml    | 51 ++++++++++++++++++-
>>>>   1 file changed, 50 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>>> index 0b69b12b849e..a8f99bca389e 100644
>>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>>> @@ -43,11 +43,20 @@ properties:
>>>>       maxItems: 5
>>>>
>>>>     interrupts:
>>>> -    maxItems: 1
>>>> +    minItems: 1
>>>> +    maxItems: 8
>>>>
>>>>     interrupt-names:
>>>> +    minItems: 1
>>>>       items:
>>>>         - const: msi
>>>> +      - const: msi2
>>>> +      - const: msi3
>>>> +      - const: msi4
>>>> +      - const: msi5
>>>> +      - const: msi6
>>>> +      - const: msi7
>>>> +      - const: msi8
>>>>
>>>>     # Common definitions for clocks, clock-names and reset.
>>>>     # Platform constraints are described later.
>>>> @@ -623,6 +632,46 @@ allOf:
>>>>           - resets
>>>>           - reset-names
>>>>
>>>> +    # On newer chipsets support either 1 or 8 msi interrupts
>>>> +    # On older chipsets it's always 1 msi interrupt
>>>> +  - if:
>>>> +      properties:
>>>> +        compatibles:
>>>> +          contains:
>>>> +            enum:
>>>> +              - qcom,pcie-msm8996
>>>> +              - qcom,pcie-sc7280
>>>> +              - qcom,pcie-sc8180x
>>>> +              - qcom,pcie-sdm845
>>>> +              - qcom,pcie-sm8150
>>>> +              - qcom,pcie-sm8250
>>>> +              - qcom,pcie-sm8450-pcie0
>>>> +              - qcom,pcie-sm8450-pcie1
>>>> +    then:
>>>> +      oneOf:
>>>> +        - properties:
>>>> +            interrupts:
>>>> +              minItems: 1
>>>
>>> minItems should not be needed here and in places below, because it is
>>> equal to maxItems.
>>
>> Maybe it's a misunderstanding from my side. In the top level we have
>> the min = 1, max = 8.
>> How does that interfere with these entries? In other words, if we e.g.
>> omit minItems here, which setting would preveal: implicit minItems = 8
>> (from maxItems = 8) or minItems = 1 in the top level?
>>
>>>> +              maxItems: 1
> 
> I don't propose to skip it for the case with maxItems:8, but only here.
> minItems:1 is set in toplevel. Where is that implicit minItems:8?

maxItems:8? Maybe I just misunderstand this part of yaml/jsonschema.

-- 
With best wishes
Dmitry

Re: [PATCH v4 6/7] dt-bindings: pci/qcom,pcie: support additional MSI interrupts

[Krzysztof Kozlowski]

On 28/04/2022 16:45, Dmitry Baryshkov wrote:
> On 28/04/2022 17:06, Krzysztof Kozlowski wrote:
>> On 28/04/2022 15:57, Dmitry Baryshkov wrote:
>>> On Thu, 28 Apr 2022 at 15:08, Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 28/04/2022 13:59, Dmitry Baryshkov wrote:
>>>>> On Qualcomm platforms each group of 32 MSI vectors is routed to the
>>>>> separate GIC interrupt. Document mapping of additional interrupts.
>>>>>
>>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>> ---
>>>>>   .../devicetree/bindings/pci/qcom,pcie.yaml    | 51 ++++++++++++++++++-
>>>>>   1 file changed, 50 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>>>> index 0b69b12b849e..a8f99bca389e 100644
>>>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>>>> @@ -43,11 +43,20 @@ properties:
>>>>>       maxItems: 5
>>>>>
>>>>>     interrupts:
>>>>> -    maxItems: 1
>>>>> +    minItems: 1
>>>>> +    maxItems: 8
>>>>>
>>>>>     interrupt-names:
>>>>> +    minItems: 1
>>>>>       items:
>>>>>         - const: msi
>>>>> +      - const: msi2
>>>>> +      - const: msi3
>>>>> +      - const: msi4
>>>>> +      - const: msi5
>>>>> +      - const: msi6
>>>>> +      - const: msi7
>>>>> +      - const: msi8
>>>>>
>>>>>     # Common definitions for clocks, clock-names and reset.
>>>>>     # Platform constraints are described later.
>>>>> @@ -623,6 +632,46 @@ allOf:
>>>>>           - resets
>>>>>           - reset-names
>>>>>
>>>>> +    # On newer chipsets support either 1 or 8 msi interrupts
>>>>> +    # On older chipsets it's always 1 msi interrupt
>>>>> +  - if:
>>>>> +      properties:
>>>>> +        compatibles:
>>>>> +          contains:
>>>>> +            enum:
>>>>> +              - qcom,pcie-msm8996
>>>>> +              - qcom,pcie-sc7280
>>>>> +              - qcom,pcie-sc8180x
>>>>> +              - qcom,pcie-sdm845
>>>>> +              - qcom,pcie-sm8150
>>>>> +              - qcom,pcie-sm8250
>>>>> +              - qcom,pcie-sm8450-pcie0
>>>>> +              - qcom,pcie-sm8450-pcie1
>>>>> +    then:
>>>>> +      oneOf:
>>>>> +        - properties:
>>>>> +            interrupts:
>>>>> +              minItems: 1
>>>>
>>>> minItems should not be needed here and in places below, because it is
>>>> equal to maxItems.
>>>
>>> Maybe it's a misunderstanding from my side. In the top level we have
>>> the min = 1, max = 8.
>>> How does that interfere with these entries? In other words, if we e.g.
>>> omit minItems here, which setting would preveal: implicit minItems = 8
>>> (from maxItems = 8) or minItems = 1 in the top level?
>>>
>>>>> +              maxItems: 1
>>
>> I don't propose to skip it for the case with maxItems:8, but only here.
>> minItems:1 is set in toplevel. Where is that implicit minItems:8?
> 
> maxItems:8? Maybe I just misunderstand this part of yaml/jsonschema.

The top level defines minItems=1, maxItems=8, so it cannot mean
implicitly "minItems=1, maxItems=8, minItems=maxItems".

There is no other place in the bindings which would implicitly set here
minItems=maxItems.


Best regards,
Krzysztof

[PATCH v4 7/7] arm64: dts: qcom: sm8250: provide additional MSI interrupts

[Dmitry Baryshkov]

On SM8250 each group of MSI interrupts is mapped to the separate host
interrupt. Describe each of interrupts in the device tree for PCIe0
host.

Tested on Qualcomm RB5 platform with first group of MSI interrupts being
used by the PME and attached ath11k WiFi chip using second group of MSI
interrupts.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8250.dtsi | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 410272a1e19b..0659ac45c651 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -1807,8 +1807,15 @@ pcie0: pci@1c00000 {
 			ranges = <0x01000000 0x0 0x60200000 0 0x60200000 0x0 0x100000>,
 				 <0x02000000 0x0 0x60300000 0 0x60300000 0x0 0x3d00000>;
 
-			interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>;
-			interrupt-names = "msi";
+			interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 142 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 143 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 144 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 146 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "msi", "msi2", "msi3", "msi4", "msi5", "msi6", "msi7", "msi8";
 			#interrupt-cells = <1>;
 			interrupt-map-mask = <0 0 0 0x7>;
 			interrupt-map = <0 0 0 1 &intc 0 149 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
-- 
2.35.1