[PATCH v12 0/8] PCI: qcom: Fix higher MSI vectors handling

[PATCH v12 0/8] 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.

The first patch in the series is a revert of  [2] (landed in pci-next).
Either both patches should be applied or both should be dropped.

Patchseries dependecies: [3] (for the schema change).

Changes since v11 (suggested by Johan):
 - Added back reporting errors for the "msi0" interrupt,
 - Stopped overriding num_vectors field if it is less than the amount of
   MSI vectors deduced from interrupt list,
 - Added a warning (and an override) if the host specifies more MSI
   vectors than available,
 - Moved has_split_msi_irq variable to the patch where it is used.

Changes since v10:
 - Remove has_split_msi_irqs flag. Trust DT and use split MSI IRQs if
   they are described in the DT. This removes the need for the
   pcie-qcom.c changes (everything is handled by the core (suggested by
   Johan).
 - Rebased on top of Lorenzo's DWC branch

Changes since v9:
 - Relax requirements and stop validating the DT. If the has_split_msi
   was specified, parse as many msiN irqs as specified in DT. If there
   are none, fallback to the single "msi" IRQ.

Changes since v8:
 - Fix typos noted by Bjorn Helgaas
 - Add missing links to the patch 1 (revert)
 - Fix sm8250 interrupt-names (Johan)
 - Specify num_vectors in qcom configuration data (Johan)
 - Rework parsing of MSI IRQs (Johan)

Changes since v7:
 - Move code back to the dwc core driver (as required by Rob),
 - Change dt schema to require either a single "msi" interrupt or an
   array of "msi0", "msi1", ... "msi7" IRQs. Disallow specifying a
   part of the array (the DT should specify the exact amount of MSI IRQs
   allowing fallback to a single "msi" IRQ),
 - Fix in the DWC init code for the dma_mapping_error() return value.

Changes since v6:
 - Fix indentation of the arguments as requested by Stanimir

Changes since v5:
 - Fixed commit subject and in-comment code according to Bjorn's
   suggestion,
 - Changed variable idx to i to follow dw_handle_msi_irq() style.

Changes since v4:
 - Fix the minItems/maxItems properties in the YAML schema.

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 (8):
  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: Convert msi_irq to the array
  PCI: dwc: split MSI IRQ parsing/allocation to a separate function
  PCI: dwc: Handle MSIs routed to multiple GIC interrupts
  PCI: dwc: Implement special ISR handler for split MSI IRQ setup
  dt-bindings: PCI: qcom: Support additional MSI interrupts
  arm64: dts: qcom: sm8250: provide additional MSI interrupts

 .../devicetree/bindings/pci/qcom,pcie.yaml    |  53 +++-
 arch/arm64/boot/dts/qcom/sm8250.dtsi          |  12 +-
 drivers/pci/controller/dwc/pci-dra7xx.c       |   2 +-
 drivers/pci/controller/dwc/pci-exynos.c       |   2 +-
 .../pci/controller/dwc/pcie-designware-host.c | 243 +++++++++++++-----
 drivers/pci/controller/dwc/pcie-designware.h  |   2 +-
 drivers/pci/controller/dwc/pcie-keembay.c     |   2 +-
 drivers/pci/controller/dwc/pcie-qcom.c        |   1 -
 drivers/pci/controller/dwc/pcie-spear13xx.c   |   2 +-
 drivers/pci/controller/dwc/pcie-tegra194.c    |   2 +-
 10 files changed, 246 insertions(+), 75 deletions(-)

-- 
2.35.1

[PATCH v12 1/8] 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 high 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_vectors can lead to missing MSI
interrupts and thus to devices malfunction.

[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/

Fixes: 20f1bfb8dd62 ("PCI: qcom: Add support for handling MSIs from 8 endpoints")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.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 f9a61ad6d1f0..2e5464edc36e 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1587,7 +1587,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 v12 2/8] 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.

Reviewed-by: Rob Herring <robh@kernel.org>
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 9979302532b7..af91fe69f542 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

Re: [PATCH v12 2/8] PCI: dwc: Correct msi_irq condition in dw_pcie_free_msi()

[Johan Hovold]

On Mon, May 23, 2022 at 09:18:30PM +0300, Dmitry Baryshkov wrote:
> 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.
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> 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 9979302532b7..af91fe69f542 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);

Looks good.

Reviewed-by: Johan Hovold <johan+linaro@kernel.org>

[PATCH v12 3/8] PCI: dwc: Convert msi_irq to the array

[Dmitry Baryshkov]

Qualcomm version of DWC PCIe controller supports more than 32 MSI
interrupts, but they are routed to separate interrupts in groups of 32
vectors. To support such configuration, change the msi_irq field into an
array. Let the DWC core handle all interrupts that were set in this
array.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/pci/controller/dwc/pci-dra7xx.c       |  2 +-
 drivers/pci/controller/dwc/pci-exynos.c       |  2 +-
 .../pci/controller/dwc/pcie-designware-host.c | 30 +++++++++++--------
 drivers/pci/controller/dwc/pcie-designware.h  |  2 +-
 drivers/pci/controller/dwc/pcie-keembay.c     |  2 +-
 drivers/pci/controller/dwc/pcie-spear13xx.c   |  2 +-
 drivers/pci/controller/dwc/pcie-tegra194.c    |  2 +-
 7 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
index dfcdeb432dc8..0919c96dcdbd 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -483,7 +483,7 @@ static int dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
 		return pp->irq;
 
 	/* MSI IRQ is muxed */
-	pp->msi_irq = -ENODEV;
+	pp->msi_irq[0] = -ENODEV;
 
 	ret = dra7xx_pcie_init_irq_domain(pp);
 	if (ret < 0)
diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
index 467c8d1cd7e4..4f2010bd9cd7 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -292,7 +292,7 @@ static int exynos_add_pcie_port(struct exynos_pcie *ep,
 	}
 
 	pp->ops = &exynos_pcie_host_ops;
-	pp->msi_irq = -ENODEV;
+	pp->msi_irq[0] = -ENODEV;
 
 	ret = dw_pcie_host_init(pp);
 	if (ret) {
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index af91fe69f542..8dd913f69de7 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -257,8 +257,11 @@ int dw_pcie_allocate_domains(struct pcie_port *pp)
 
 static 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);
+	u32 ctrl;
+
+	for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++)
+		if (pp->msi_irq[ctrl] > 0)
+			irq_set_chained_handler_and_data(pp->msi_irq[ctrl], NULL, NULL);
 
 	irq_domain_remove(pp->msi_domain);
 	irq_domain_remove(pp->irq_domain);
@@ -368,13 +371,15 @@ int dw_pcie_host_init(struct pcie_port *pp)
 			for (ctrl = 0; ctrl < num_ctrls; ctrl++)
 				pp->irq_mask[ctrl] = ~0;
 
-			if (!pp->msi_irq) {
-				pp->msi_irq = platform_get_irq_byname_optional(pdev, "msi");
-				if (pp->msi_irq < 0) {
-					pp->msi_irq = platform_get_irq(pdev, 0);
-					if (pp->msi_irq < 0)
-						return pp->msi_irq;
+			if (!pp->msi_irq[0]) {
+				int irq = platform_get_irq_byname_optional(pdev, "msi");
+
+				if (irq < 0) {
+					irq = platform_get_irq(pdev, 0);
+					if (irq < 0)
+						return irq;
 				}
+				pp->msi_irq[0] = irq;
 			}
 
 			pp->msi_irq_chip = &dw_pci_msi_bottom_irq_chip;
@@ -383,10 +388,11 @@ int dw_pcie_host_init(struct pcie_port *pp)
 			if (ret)
 				return ret;
 
-			if (pp->msi_irq > 0)
-				irq_set_chained_handler_and_data(pp->msi_irq,
-							    dw_chained_msi_isr,
-							    pp);
+			for (ctrl = 0; ctrl < num_ctrls; ctrl++)
+				if (pp->msi_irq[ctrl] > 0)
+					irq_set_chained_handler_and_data(pp->msi_irq[ctrl],
+									 dw_chained_msi_isr,
+									 pp);
 
 			ret = dma_set_mask(pci->dev, DMA_BIT_MASK(32));
 			if (ret)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 7d6e9b7576be..9c1a38b0a6b3 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -187,7 +187,7 @@ struct pcie_port {
 	u32			io_size;
 	int			irq;
 	const struct dw_pcie_host_ops *ops;
-	int			msi_irq;
+	int			msi_irq[MAX_MSI_CTRLS];
 	struct irq_domain	*irq_domain;
 	struct irq_domain	*msi_domain;
 	u16			msi_msg;
diff --git a/drivers/pci/controller/dwc/pcie-keembay.c b/drivers/pci/controller/dwc/pcie-keembay.c
index 1ac29a6eef22..297e6e926c00 100644
--- a/drivers/pci/controller/dwc/pcie-keembay.c
+++ b/drivers/pci/controller/dwc/pcie-keembay.c
@@ -338,7 +338,7 @@ static int keembay_pcie_add_pcie_port(struct keembay_pcie *pcie,
 	int ret;
 
 	pp->ops = &keembay_pcie_host_ops;
-	pp->msi_irq = -ENODEV;
+	pp->msi_irq[0] = -ENODEV;
 
 	ret = keembay_pcie_setup_msi_irq(pcie);
 	if (ret)
diff --git a/drivers/pci/controller/dwc/pcie-spear13xx.c b/drivers/pci/controller/dwc/pcie-spear13xx.c
index 1569e82b5568..cc7776833810 100644
--- a/drivers/pci/controller/dwc/pcie-spear13xx.c
+++ b/drivers/pci/controller/dwc/pcie-spear13xx.c
@@ -172,7 +172,7 @@ static int spear13xx_add_pcie_port(struct spear13xx_pcie *spear13xx_pcie,
 	}
 
 	pp->ops = &spear13xx_pcie_host_ops;
-	pp->msi_irq = -ENODEV;
+	pp->msi_irq[0] = -ENODEV;
 
 	ret = dw_pcie_host_init(pp);
 	if (ret) {
diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index cc2678490162..7056072637ab 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -2262,7 +2262,7 @@ static void tegra194_pcie_shutdown(struct platform_device *pdev)
 
 	disable_irq(pcie->pci.pp.irq);
 	if (IS_ENABLED(CONFIG_PCI_MSI))
-		disable_irq(pcie->pci.pp.msi_irq);
+		disable_irq(pcie->pci.pp.msi_irq[0]);
 
 	tegra194_pcie_pme_turnoff(pcie);
 	tegra_pcie_unconfig_controller(pcie);
-- 
2.35.1

Re: [PATCH v12 3/8] PCI: dwc: Convert msi_irq to the array

[Rob Herring]

On Mon, May 23, 2022 at 09:18:31PM +0300, Dmitry Baryshkov wrote:
> Qualcomm version of DWC PCIe controller supports more than 32 MSI
> interrupts, but they are routed to separate interrupts in groups of 32
> vectors. To support such configuration, change the msi_irq field into an
> array. Let the DWC core handle all interrupts that were set in this
> array.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/pci/controller/dwc/pci-dra7xx.c       |  2 +-
>  drivers/pci/controller/dwc/pci-exynos.c       |  2 +-
>  .../pci/controller/dwc/pcie-designware-host.c | 30 +++++++++++--------
>  drivers/pci/controller/dwc/pcie-designware.h  |  2 +-
>  drivers/pci/controller/dwc/pcie-keembay.c     |  2 +-
>  drivers/pci/controller/dwc/pcie-spear13xx.c   |  2 +-
>  drivers/pci/controller/dwc/pcie-tegra194.c    |  2 +-
>  7 files changed, 24 insertions(+), 18 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v12 3/8] PCI: dwc: Convert msi_irq to the array

[Johan Hovold]

On Mon, May 23, 2022 at 09:18:31PM +0300, Dmitry Baryshkov wrote:
> Qualcomm version of DWC PCIe controller supports more than 32 MSI
> interrupts, but they are routed to separate interrupts in groups of 32
> vectors. To support such configuration, change the msi_irq field into an
> array. Let the DWC core handle all interrupts that were set in this
> array.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/pci/controller/dwc/pci-dra7xx.c       |  2 +-
>  drivers/pci/controller/dwc/pci-exynos.c       |  2 +-
>  .../pci/controller/dwc/pcie-designware-host.c | 30 +++++++++++--------
>  drivers/pci/controller/dwc/pcie-designware.h  |  2 +-
>  drivers/pci/controller/dwc/pcie-keembay.c     |  2 +-
>  drivers/pci/controller/dwc/pcie-spear13xx.c   |  2 +-
>  drivers/pci/controller/dwc/pcie-tegra194.c    |  2 +-
>  7 files changed, 24 insertions(+), 18 deletions(-)

> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index af91fe69f542..8dd913f69de7 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -257,8 +257,11 @@ int dw_pcie_allocate_domains(struct pcie_port *pp)
>  
>  static 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);
> +	u32 ctrl;
> +
> +	for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++)
> +		if (pp->msi_irq[ctrl] > 0)
> +			irq_set_chained_handler_and_data(pp->msi_irq[ctrl], NULL, NULL);

I'd use brackets around multline statements for readability.

>  	irq_domain_remove(pp->msi_domain);
>  	irq_domain_remove(pp->irq_domain);

> @@ -383,10 +388,11 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  			if (ret)
>  				return ret;
>  
> -			if (pp->msi_irq > 0)
> -				irq_set_chained_handler_and_data(pp->msi_irq,
> -							    dw_chained_msi_isr,
> -							    pp);
> +			for (ctrl = 0; ctrl < num_ctrls; ctrl++)
> +				if (pp->msi_irq[ctrl] > 0)
> +					irq_set_chained_handler_and_data(pp->msi_irq[ctrl],
> +									 dw_chained_msi_isr,
> +									 pp);

Same here (even if the old code didn't use it).

>  			ret = dma_set_mask(pci->dev, DMA_BIT_MASK(32));
>  			if (ret)

Looks good otherwise.

Reviewed-by: Johan Hovold <johan+linaro@kernel.org>

Johan

[PATCH v12 4/8] PCI: dwc: split MSI IRQ parsing/allocation to a separate function

[Dmitry Baryshkov]

Split handling of MSI host IRQs to a separate dw_pcie_msi_host_init()
function. The code is complex enough to warrant a separate function.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../pci/controller/dwc/pcie-designware-host.c | 98 +++++++++++--------
 1 file changed, 56 insertions(+), 42 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 8dd913f69de7..a076abe6611c 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -288,6 +288,60 @@ static void dw_pcie_msi_init(struct pcie_port *pp)
 	dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_HI, upper_32_bits(msi_target));
 }
 
+static int dw_pcie_msi_host_init(struct pcie_port *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct device *dev = pci->dev;
+	struct platform_device *pdev = to_platform_device(dev);
+	int ret;
+	u32 ctrl, num_ctrls;
+
+	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
+	for (ctrl = 0; ctrl < num_ctrls; ctrl++)
+		pp->irq_mask[ctrl] = ~0;
+
+	if (!pp->msi_irq[0]) {
+		int irq = platform_get_irq_byname_optional(pdev, "msi");
+
+		if (irq < 0) {
+			irq = platform_get_irq(pdev, 0);
+			if (irq < 0)
+				return irq;
+		}
+		pp->msi_irq[0] = irq;
+	}
+
+	pp->msi_irq_chip = &dw_pci_msi_bottom_irq_chip;
+
+	ret = dw_pcie_allocate_domains(pp);
+	if (ret)
+		return ret;
+
+	for (ctrl = 0; ctrl < num_ctrls; ctrl++)
+		if (pp->msi_irq[ctrl] > 0)
+			irq_set_chained_handler_and_data(pp->msi_irq[ctrl],
+							 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;
+		dw_pcie_free_msi(pp);
+		return ret;
+	}
+
+	return 0;
+}
+
 int dw_pcie_host_init(struct pcie_port *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -365,49 +419,9 @@ int dw_pcie_host_init(struct pcie_port *pp)
 			if (ret < 0)
 				return ret;
 		} else if (pp->has_msi_ctrl) {
-			u32 ctrl, num_ctrls;
-
-			num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
-			for (ctrl = 0; ctrl < num_ctrls; ctrl++)
-				pp->irq_mask[ctrl] = ~0;
-
-			if (!pp->msi_irq[0]) {
-				int irq = platform_get_irq_byname_optional(pdev, "msi");
-
-				if (irq < 0) {
-					irq = platform_get_irq(pdev, 0);
-					if (irq < 0)
-						return irq;
-				}
-				pp->msi_irq[0] = irq;
-			}
-
-			pp->msi_irq_chip = &dw_pci_msi_bottom_irq_chip;
-
-			ret = dw_pcie_allocate_domains(pp);
-			if (ret)
+			ret = dw_pcie_msi_host_init(pp);
+			if (ret < 0)
 				return ret;
-
-			for (ctrl = 0; ctrl < num_ctrls; ctrl++)
-				if (pp->msi_irq[ctrl] > 0)
-					irq_set_chained_handler_and_data(pp->msi_irq[ctrl],
-									 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;
-				goto err_free_msi;
-			}
 		}
 	}
 
-- 
2.35.1

Re: [PATCH v12 4/8] PCI: dwc: split MSI IRQ parsing/allocation to a separate function

[Rob Herring]

On Mon, May 23, 2022 at 09:18:32PM +0300, Dmitry Baryshkov wrote:
> Split handling of MSI host IRQs to a separate dw_pcie_msi_host_init()
> function. The code is complex enough to warrant a separate function.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../pci/controller/dwc/pcie-designware-host.c | 98 +++++++++++--------
>  1 file changed, 56 insertions(+), 42 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

Note that we should probably apply this[1] or whatever fix we end up 
with first.

Rob

[1] https://lore.kernel.org/all/20220525223316.388490-1-willmcvicker@google.com/

Re: [PATCH v12 4/8] PCI: dwc: split MSI IRQ parsing/allocation to a separate function

[Dmitry Baryshkov]

On Thu, 26 May 2022 at 21:09, Rob Herring <robh@kernel.org> wrote:
>
> On Mon, May 23, 2022 at 09:18:32PM +0300, Dmitry Baryshkov wrote:
> > Split handling of MSI host IRQs to a separate dw_pcie_msi_host_init()
> > function. The code is complex enough to warrant a separate function.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  .../pci/controller/dwc/pcie-designware-host.c | 98 +++++++++++--------
> >  1 file changed, 56 insertions(+), 42 deletions(-)
>
> Reviewed-by: Rob Herring <robh@kernel.org>
>
> Note that we should probably apply this[1] or whatever fix we end up
> with first.

Ack, I will rebase this patch series the fix gets merged.

> [1] https://lore.kernel.org/all/20220525223316.388490-1-willmcvicker@google.com/

-- 
With best wishes
Dmitry

Re: [PATCH v12 4/8] PCI: dwc: split MSI IRQ parsing/allocation to a separate function

[Johan Hovold]

On Mon, May 23, 2022 at 09:18:32PM +0300, Dmitry Baryshkov wrote:
> Split handling of MSI host IRQs to a separate dw_pcie_msi_host_init()
> function. The code is complex enough to warrant a separate function.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../pci/controller/dwc/pcie-designware-host.c | 98 +++++++++++--------
>  1 file changed, 56 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 8dd913f69de7..a076abe6611c 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -288,6 +288,60 @@ static void dw_pcie_msi_init(struct pcie_port *pp)
>  	dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_HI, upper_32_bits(msi_target));
>  }
>  
> +static int dw_pcie_msi_host_init(struct pcie_port *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct device *dev = pci->dev;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	int ret;
> +	u32 ctrl, num_ctrls;
> +
> +	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
> +	for (ctrl = 0; ctrl < num_ctrls; ctrl++)
> +		pp->irq_mask[ctrl] = ~0;
> +
> +	if (!pp->msi_irq[0]) {
> +		int irq = platform_get_irq_byname_optional(pdev, "msi");
> +
> +		if (irq < 0) {
> +			irq = platform_get_irq(pdev, 0);
> +			if (irq < 0)
> +				return irq;
> +		}
> +		pp->msi_irq[0] = irq;
> +	}
> +
> +	pp->msi_irq_chip = &dw_pci_msi_bottom_irq_chip;
> +
> +	ret = dw_pcie_allocate_domains(pp);
> +	if (ret)
> +		return ret;
> +
> +	for (ctrl = 0; ctrl < num_ctrls; ctrl++)
> +		if (pp->msi_irq[ctrl] > 0)
> +			irq_set_chained_handler_and_data(pp->msi_irq[ctrl],
> +							 dw_chained_msi_isr,
> +							 pp);

Reminder: brackets.

Reviewed-by: Johan Hovold <johan+linaro@kernel.org>

[PATCH v12 5/8] PCI: dwc: Handle MSIs routed to multiple GIC interrupts

[Dmitry Baryshkov]

On some of Qualcomm platforms each group of 32 MSI vectors is routed to the
separate GIC interrupt. Implement support for such configurations by
parsing "msi0" ... "msiN" interrupts and attaching them to the chained
handler.

Note, that if DT doesn't list an array of MSI interrupts and uses single
"msi" IRQ, the driver will limit the amount of supported MSI vectors
accordingly (to 32).

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../pci/controller/dwc/pcie-designware-host.c | 61 +++++++++++++++++--
 1 file changed, 57 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index a076abe6611c..98a57249ecaf 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -288,6 +288,47 @@ static void dw_pcie_msi_init(struct pcie_port *pp)
 	dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_HI, upper_32_bits(msi_target));
 }
 
+static const char * const split_msi_names[] = {
+	"msi0", "msi1", "msi2", "msi3",
+	"msi4", "msi5", "msi6", "msi7",
+};
+
+static int dw_pcie_parse_split_msi_irq(struct pcie_port *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct device *dev = pci->dev;
+	struct platform_device *pdev = to_platform_device(dev);
+	int irq;
+	u32 ctrl, max_vectors;
+
+	/* Parse as many IRQs as described in the devicetree. */
+	for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++) {
+		irq = platform_get_irq_byname_optional(pdev, split_msi_names[ctrl]);
+		if (irq == -ENXIO)
+			break;
+		if (irq < 0)
+			return dev_err_probe(dev, irq,
+					     "Failed to parse MSI IRQ '%s'\n",
+					     split_msi_names[ctrl]);
+
+		pp->msi_irq[ctrl] = irq;
+	}
+
+	/* If there were no "msiN" IRQs at all, fallback to the standard "msi" IRQ. */
+	if (ctrl == 0)
+		return -ENXIO;
+
+	max_vectors = ctrl * MAX_MSI_IRQS_PER_CTRL;
+	if (pp->num_vectors > max_vectors) {
+		dev_warn(dev, "Exceeding number of MSI vectors, limiting to %d\n", max_vectors);
+		pp->num_vectors = max_vectors;
+	}
+	if (!pp->num_vectors)
+		pp->num_vectors = max_vectors;
+
+	return 0;
+}
+
 static int dw_pcie_msi_host_init(struct pcie_port *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -296,21 +337,32 @@ static int dw_pcie_msi_host_init(struct pcie_port *pp)
 	int ret;
 	u32 ctrl, num_ctrls;
 
-	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
-	for (ctrl = 0; ctrl < num_ctrls; ctrl++)
+	for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++)
 		pp->irq_mask[ctrl] = ~0;
 
+	if (!pp->msi_irq[0]) {
+		ret = dw_pcie_parse_split_msi_irq(pp);
+		if (ret < 0 && ret != -ENXIO)
+			return ret;
+	}
+
+	if (!pp->num_vectors)
+		pp->num_vectors = MSI_DEF_NUM_VECTORS;
+	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
+
 	if (!pp->msi_irq[0]) {
 		int irq = platform_get_irq_byname_optional(pdev, "msi");
 
 		if (irq < 0) {
 			irq = platform_get_irq(pdev, 0);
 			if (irq < 0)
-				return irq;
+				return dev_err_probe(dev, irq, "Failed to parse MSI irq\n");
 		}
 		pp->msi_irq[0] = irq;
 	}
 
+	dev_dbg(dev, "Using %d MSI vectors\n", pp->num_vectors);
+
 	pp->msi_irq_chip = &dw_pci_msi_bottom_irq_chip;
 
 	ret = dw_pcie_allocate_domains(pp);
@@ -407,7 +459,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
 				     of_property_read_bool(np, "msi-parent") ||
 				     of_property_read_bool(np, "msi-map"));
 
-		if (!pp->num_vectors) {
+		/* for the has_msi_ctrl the default assignment is handled inside dw_pcie_msi_host_init() */
+		if (!pp->has_msi_ctrl && !pp->num_vectors) {
 			pp->num_vectors = MSI_DEF_NUM_VECTORS;
 		} else if (pp->num_vectors > MAX_MSI_IRQS) {
 			dev_err(dev, "Invalid number of vectors\n");
-- 
2.35.1

Re: [PATCH v12 5/8] PCI: dwc: Handle MSIs routed to multiple GIC interrupts

[Rob Herring]

On Mon, May 23, 2022 at 09:18:33PM +0300, Dmitry Baryshkov wrote:
> On some of Qualcomm platforms each group of 32 MSI vectors is routed to the
> separate GIC interrupt. Implement support for such configurations by
> parsing "msi0" ... "msiN" interrupts and attaching them to the chained
> handler.
> 
> Note, that if DT doesn't list an array of MSI interrupts and uses single
> "msi" IRQ, the driver will limit the amount of supported MSI vectors
> accordingly (to 32).
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../pci/controller/dwc/pcie-designware-host.c | 61 +++++++++++++++++--
>  1 file changed, 57 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index a076abe6611c..98a57249ecaf 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -288,6 +288,47 @@ static void dw_pcie_msi_init(struct pcie_port *pp)
>  	dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_HI, upper_32_bits(msi_target));
>  }
>  
> +static const char * const split_msi_names[] = {
> +	"msi0", "msi1", "msi2", "msi3",
> +	"msi4", "msi5", "msi6", "msi7",
> +};
> +
> +static int dw_pcie_parse_split_msi_irq(struct pcie_port *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct device *dev = pci->dev;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	int irq;
> +	u32 ctrl, max_vectors;
> +
> +	/* Parse as many IRQs as described in the devicetree. */
> +	for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++) {
> +		irq = platform_get_irq_byname_optional(pdev, split_msi_names[ctrl]);

I'd do this instead:

char *msi_name = "msiX";
msi_name[3] = '0' + ctrl;

Otherwise,

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v12 5/8] PCI: dwc: Handle MSIs routed to multiple GIC interrupts

[Johan Hovold]

On Mon, May 23, 2022 at 09:18:33PM +0300, Dmitry Baryshkov wrote:
> On some of Qualcomm platforms each group of 32 MSI vectors is routed to the
> separate GIC interrupt. Implement support for such configurations by
> parsing "msi0" ... "msiN" interrupts and attaching them to the chained
> handler.
> 
> Note, that if DT doesn't list an array of MSI interrupts and uses single
> "msi" IRQ, the driver will limit the amount of supported MSI vectors
> accordingly (to 32).
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../pci/controller/dwc/pcie-designware-host.c | 61 +++++++++++++++++--
>  1 file changed, 57 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index a076abe6611c..98a57249ecaf 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -288,6 +288,47 @@ static void dw_pcie_msi_init(struct pcie_port *pp)
>  	dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_HI, upper_32_bits(msi_target));
>  }
>  
> +static const char * const split_msi_names[] = {
> +	"msi0", "msi1", "msi2", "msi3",
> +	"msi4", "msi5", "msi6", "msi7",
> +};
> +
> +static int dw_pcie_parse_split_msi_irq(struct pcie_port *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct device *dev = pci->dev;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	int irq;
> +	u32 ctrl, max_vectors;
> +
> +	/* Parse as many IRQs as described in the devicetree. */
> +	for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++) {
> +		irq = platform_get_irq_byname_optional(pdev, split_msi_names[ctrl]);
> +		if (irq == -ENXIO)
> +			break;
> +		if (irq < 0)
> +			return dev_err_probe(dev, irq,
> +					     "Failed to parse MSI IRQ '%s'\n",
> +					     split_msi_names[ctrl]);
> +
> +		pp->msi_irq[ctrl] = irq;
> +	}
> +
> +	/* If there were no "msiN" IRQs at all, fallback to the standard "msi" IRQ. */
> +	if (ctrl == 0)
> +		return -ENXIO;
> +
> +	max_vectors = ctrl * MAX_MSI_IRQS_PER_CTRL;
> +	if (pp->num_vectors > max_vectors) {
> +		dev_warn(dev, "Exceeding number of MSI vectors, limiting to %d\n", max_vectors);

%u

break line after last comma?

> +		pp->num_vectors = max_vectors;
> +	}
> +	if (!pp->num_vectors)
> +		pp->num_vectors = max_vectors;
> +
> +	return 0;
> +}
> +
>  static int dw_pcie_msi_host_init(struct pcie_port *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -296,21 +337,32 @@ static int dw_pcie_msi_host_init(struct pcie_port *pp)
>  	int ret;
>  	u32 ctrl, num_ctrls;
>  
> -	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
> -	for (ctrl = 0; ctrl < num_ctrls; ctrl++)
> +	for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++)
>  		pp->irq_mask[ctrl] = ~0;
>  
> +	if (!pp->msi_irq[0]) {
> +		ret = dw_pcie_parse_split_msi_irq(pp);
> +		if (ret < 0 && ret != -ENXIO)
> +			return ret;
> +	}
> +
> +	if (!pp->num_vectors)
> +		pp->num_vectors = MSI_DEF_NUM_VECTORS;
> +	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
> +
>  	if (!pp->msi_irq[0]) {
>  		int irq = platform_get_irq_byname_optional(pdev, "msi");
>  
>  		if (irq < 0) {
>  			irq = platform_get_irq(pdev, 0);
>  			if (irq < 0)
> -				return irq;
> +				return dev_err_probe(dev, irq, "Failed to parse MSI irq\n");
>  		}
>  		pp->msi_irq[0] = irq;
>  	}
>  
> +	dev_dbg(dev, "Using %d MSI vectors\n", pp->num_vectors);
> +
>  	pp->msi_irq_chip = &dw_pci_msi_bottom_irq_chip;
>  
>  	ret = dw_pcie_allocate_domains(pp);
> @@ -407,7 +459,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  				     of_property_read_bool(np, "msi-parent") ||
>  				     of_property_read_bool(np, "msi-map"));
>  
> -		if (!pp->num_vectors) {
> +		/* for the has_msi_ctrl the default assignment is handled inside dw_pcie_msi_host_init() */

Add the missing "case" after "has_msi_ctrl".

s/inside/in/

Please make this a multiline comment split at < 80 chars.

And follow the comment style of the driver and start with a capital
letter.

> +		if (!pp->has_msi_ctrl && !pp->num_vectors) {
>  			pp->num_vectors = MSI_DEF_NUM_VECTORS;
>  		} else if (pp->num_vectors > MAX_MSI_IRQS) {
>  			dev_err(dev, "Invalid number of vectors\n");

Looks good now otherwise. 

But please consider Rob's suggestion for generating the interrupt names.

Reviewed-by: Johan Hovold <johan+linaro@kernel.org>

[PATCH v12 6/8] PCI: dwc: Implement special ISR handler for split MSI IRQ setup

[Dmitry Baryshkov]

If the PCIe DWC controller uses split MSI IRQs for reporting MSI
vectors, it is possible to detect, which group triggered the interrupt.
Provide an optimized version of MSI ISR handler that will handle just a
single MSI group instead of handling all of them.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../pci/controller/dwc/pcie-designware-host.c | 86 ++++++++++++++-----
 1 file changed, 65 insertions(+), 21 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 98a57249ecaf..2b2de517301a 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -52,34 +52,42 @@ static struct msi_domain_info dw_pcie_msi_domain_info = {
 	.chip	= &dw_pcie_msi_irq_chip,
 };
 
+static inline irqreturn_t dw_handle_single_msi_group(struct pcie_port *pp, int i)
+{
+	int pos;
+	unsigned long val;
+	u32 status;
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+
+	status = dw_pcie_readl_dbi(pci, PCIE_MSI_INTR0_STATUS +
+				   (i * MSI_REG_CTRL_BLOCK_SIZE));
+	if (!status)
+		return IRQ_NONE;
+
+	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,
+					  (i * MAX_MSI_IRQS_PER_CTRL) +
+					  pos);
+		pos++;
+	}
+
+	return IRQ_HANDLED;
+}
+
 /* MSI int handler */
 irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
 {
-	int i, pos;
-	unsigned long val;
-	u32 status, num_ctrls;
+	int i;
+	u32 num_ctrls;
 	irqreturn_t ret = IRQ_NONE;
-	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 
 	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
 
-	for (i = 0; i < num_ctrls; i++) {
-		status = dw_pcie_readl_dbi(pci, PCIE_MSI_INTR0_STATUS +
-					   (i * MSI_REG_CTRL_BLOCK_SIZE));
-		if (!status)
-			continue;
-
-		ret = IRQ_HANDLED;
-		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,
-						  (i * MAX_MSI_IRQS_PER_CTRL) +
-						  pos);
-			pos++;
-		}
-	}
+	for (i = 0; i < num_ctrls; i++)
+		ret |= dw_handle_single_msi_group(pp, i);
 
 	return ret;
 }
@@ -98,6 +106,38 @@ static void dw_chained_msi_isr(struct irq_desc *desc)
 	chained_irq_exit(chip, desc);
 }
 
+static void dw_split_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 i;
+	u32 num_ctrls;
+	struct dw_pcie *pci;
+
+	chained_irq_enter(chip, desc);
+
+	pp = irq_desc_get_handler_data(desc);
+	pci = to_dw_pcie_from_pp(pp);
+
+	/*
+	 * Unlike generic dw_handle_msi_irq(), we can determine which group of
+	 * MSIs triggered the IRQ, so process just that group.
+	 */
+	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
+
+	for (i = 0; i < num_ctrls; i++) {
+		if (pp->msi_irq[i] == irq) {
+			dw_handle_single_msi_group(pp, i);
+			break;
+		}
+	}
+
+	WARN_ON_ONCE(i == num_ctrls);
+
+	chained_irq_exit(chip, desc);
+}
+
 static void dw_pci_setup_msi_msg(struct irq_data *d, struct msi_msg *msg)
 {
 	struct pcie_port *pp = irq_data_get_irq_chip_data(d);
@@ -336,6 +376,7 @@ static int dw_pcie_msi_host_init(struct pcie_port *pp)
 	struct platform_device *pdev = to_platform_device(dev);
 	int ret;
 	u32 ctrl, num_ctrls;
+	bool has_split_msi_irq = false;
 
 	for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++)
 		pp->irq_mask[ctrl] = ~0;
@@ -344,6 +385,8 @@ static int dw_pcie_msi_host_init(struct pcie_port *pp)
 		ret = dw_pcie_parse_split_msi_irq(pp);
 		if (ret < 0 && ret != -ENXIO)
 			return ret;
+		else if (!ret)
+			has_split_msi_irq = true;
 	}
 
 	if (!pp->num_vectors)
@@ -372,6 +415,7 @@ static int dw_pcie_msi_host_init(struct pcie_port *pp)
 	for (ctrl = 0; ctrl < num_ctrls; ctrl++)
 		if (pp->msi_irq[ctrl] > 0)
 			irq_set_chained_handler_and_data(pp->msi_irq[ctrl],
+							 has_split_msi_irq ? dw_split_msi_isr :
 							 dw_chained_msi_isr,
 							 pp);
 
-- 
2.35.1

Re: [PATCH v12 6/8] PCI: dwc: Implement special ISR handler for split MSI IRQ setup

[kernel test robot]

Hi Dmitry,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on helgaas-pci/next]
[cannot apply to robh/for-next v5.18]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Dmitry-Baryshkov/PCI-qcom-Fix-higher-MSI-vectors-handling/20220524-024956
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: mips-allyesconfig (https://download.01.org/0day-ci/archive/20220524/202205240801.7l6SF2Hv-lkp@intel.com/config)
compiler: mips-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/35aa0471e2ee4f0f21b01fbcfe6a4a425e968596
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Dmitry-Baryshkov/PCI-qcom-Fix-higher-MSI-vectors-handling/20220524-024956
        git checkout 35aa0471e2ee4f0f21b01fbcfe6a4a425e968596
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/pci/controller/dwc/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/pci/controller/dwc/pcie-designware-host.c: In function 'dw_split_msi_isr':
>> drivers/pci/controller/dwc/pcie-designware-host.c:116:25: warning: variable 'pci' set but not used [-Wunused-but-set-variable]
     116 |         struct dw_pcie *pci;
         |                         ^~~


vim +/pci +116 drivers/pci/controller/dwc/pcie-designware-host.c

   108	
   109	static void dw_split_msi_isr(struct irq_desc *desc)
   110	{
   111		struct irq_chip *chip = irq_desc_get_chip(desc);
   112		int irq = irq_desc_get_irq(desc);
   113		struct pcie_port *pp;
   114		int i;
   115		u32 num_ctrls;
 > 116		struct dw_pcie *pci;
   117	
   118		chained_irq_enter(chip, desc);
   119	
   120		pp = irq_desc_get_handler_data(desc);
   121		pci = to_dw_pcie_from_pp(pp);
   122	
   123		/*
   124		 * Unlike generic dw_handle_msi_irq(), we can determine which group of
   125		 * MSIs triggered the IRQ, so process just that group.
   126		 */
   127		num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
   128	
   129		for (i = 0; i < num_ctrls; i++) {
   130			if (pp->msi_irq[i] == irq) {
   131				dw_handle_single_msi_group(pp, i);
   132				break;
   133			}
   134		}
   135	
   136		WARN_ON_ONCE(i == num_ctrls);
   137	
   138		chained_irq_exit(chip, desc);
   139	}
   140	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH v12 6/8] PCI: dwc: Implement special ISR handler for split MSI IRQ setup

[Rob Herring]

On Mon, May 23, 2022 at 09:18:34PM +0300, Dmitry Baryshkov wrote:
> If the PCIe DWC controller uses split MSI IRQs for reporting MSI
> vectors, it is possible to detect, which group triggered the interrupt.
> Provide an optimized version of MSI ISR handler that will handle just a
> single MSI group instead of handling all of them.

A lot more complexity to save 7 register reads...

> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../pci/controller/dwc/pcie-designware-host.c | 86 ++++++++++++++-----
>  1 file changed, 65 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 98a57249ecaf..2b2de517301a 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -52,34 +52,42 @@ static struct msi_domain_info dw_pcie_msi_domain_info = {
>  	.chip	= &dw_pcie_msi_irq_chip,
>  };
>  
> +static inline irqreturn_t dw_handle_single_msi_group(struct pcie_port *pp, int i)
> +{
> +	int pos;
> +	unsigned long val;
> +	u32 status;
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +
> +	status = dw_pcie_readl_dbi(pci, PCIE_MSI_INTR0_STATUS +
> +				   (i * MSI_REG_CTRL_BLOCK_SIZE));
> +	if (!status)
> +		return IRQ_NONE;
> +
> +	val = status;
> +	pos = 0;
> +	while ((pos = find_next_bit(&val, MAX_MSI_IRQS_PER_CTRL,
> +				    pos)) != MAX_MSI_IRQS_PER_CTRL) {

for_each_set_bit() doesn't work here?

> +		generic_handle_domain_irq(pp->irq_domain,
> +					  (i * MAX_MSI_IRQS_PER_CTRL) +
> +					  pos);
> +		pos++;
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
>  /* MSI int handler */
>  irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
>  {
> -	int i, pos;
> -	unsigned long val;
> -	u32 status, num_ctrls;
> +	int i;
> +	u32 num_ctrls;
>  	irqreturn_t ret = IRQ_NONE;
> -	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>  
>  	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
>  
> -	for (i = 0; i < num_ctrls; i++) {
> -		status = dw_pcie_readl_dbi(pci, PCIE_MSI_INTR0_STATUS +
> -					   (i * MSI_REG_CTRL_BLOCK_SIZE));
> -		if (!status)
> -			continue;
> -
> -		ret = IRQ_HANDLED;
> -		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,
> -						  (i * MAX_MSI_IRQS_PER_CTRL) +
> -						  pos);
> -			pos++;
> -		}
> -	}
> +	for (i = 0; i < num_ctrls; i++)
> +		ret |= dw_handle_single_msi_group(pp, i);
>  
>  	return ret;
>  }
> @@ -98,6 +106,38 @@ static void dw_chained_msi_isr(struct irq_desc *desc)
>  	chained_irq_exit(chip, desc);
>  }
>  
> +static void dw_split_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 i;
> +	u32 num_ctrls;
> +	struct dw_pcie *pci;
> +
> +	chained_irq_enter(chip, desc);
> +
> +	pp = irq_desc_get_handler_data(desc);
> +	pci = to_dw_pcie_from_pp(pp);
> +
> +	/*
> +	 * Unlike generic dw_handle_msi_irq(), we can determine which group of
> +	 * MSIs triggered the IRQ, so process just that group.
> +	 */
> +	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
> +
> +	for (i = 0; i < num_ctrls; i++) {
> +		if (pp->msi_irq[i] == irq) {
> +			dw_handle_single_msi_group(pp, i);
> +			break;
> +		}
> +	}
> +
> +	WARN_ON_ONCE(i == num_ctrls);
> +
> +	chained_irq_exit(chip, desc);
> +}
> +
>  static void dw_pci_setup_msi_msg(struct irq_data *d, struct msi_msg *msg)
>  {
>  	struct pcie_port *pp = irq_data_get_irq_chip_data(d);
> @@ -336,6 +376,7 @@ static int dw_pcie_msi_host_init(struct pcie_port *pp)
>  	struct platform_device *pdev = to_platform_device(dev);
>  	int ret;
>  	u32 ctrl, num_ctrls;
> +	bool has_split_msi_irq = false;
>  
>  	for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++)
>  		pp->irq_mask[ctrl] = ~0;
> @@ -344,6 +385,8 @@ static int dw_pcie_msi_host_init(struct pcie_port *pp)
>  		ret = dw_pcie_parse_split_msi_irq(pp);
>  		if (ret < 0 && ret != -ENXIO)
>  			return ret;
> +		else if (!ret)
> +			has_split_msi_irq = true;
>  	}
>  
>  	if (!pp->num_vectors)
> @@ -372,6 +415,7 @@ static int dw_pcie_msi_host_init(struct pcie_port *pp)
>  	for (ctrl = 0; ctrl < num_ctrls; ctrl++)
>  		if (pp->msi_irq[ctrl] > 0)
>  			irq_set_chained_handler_and_data(pp->msi_irq[ctrl],
> +							 has_split_msi_irq ? dw_split_msi_isr :
>  							 dw_chained_msi_isr,
>  							 pp);
>  
> -- 
> 2.35.1
> 

Re: [PATCH v12 6/8] PCI: dwc: Implement special ISR handler for split MSI IRQ setup

[Dmitry Baryshkov]

On Thu, 26 May 2022 at 21:42, Rob Herring <robh@kernel.org> wrote:
>
> On Mon, May 23, 2022 at 09:18:34PM +0300, Dmitry Baryshkov wrote:
> > If the PCIe DWC controller uses split MSI IRQs for reporting MSI
> > vectors, it is possible to detect, which group triggered the interrupt.
> > Provide an optimized version of MSI ISR handler that will handle just a
> > single MSI group instead of handling all of them.
>
> A lot more complexity to save 7 register reads...

Thus it is a separate patch. It can be dropped w/o any issues.

>
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  .../pci/controller/dwc/pcie-designware-host.c | 86 ++++++++++++++-----
> >  1 file changed, 65 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index 98a57249ecaf..2b2de517301a 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -52,34 +52,42 @@ static struct msi_domain_info dw_pcie_msi_domain_info = {
> >       .chip   = &dw_pcie_msi_irq_chip,
> >  };
> >
> > +static inline irqreturn_t dw_handle_single_msi_group(struct pcie_port *pp, int i)
> > +{
> > +     int pos;
> > +     unsigned long val;
> > +     u32 status;
> > +     struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > +
> > +     status = dw_pcie_readl_dbi(pci, PCIE_MSI_INTR0_STATUS +
> > +                                (i * MSI_REG_CTRL_BLOCK_SIZE));
> > +     if (!status)
> > +             return IRQ_NONE;
> > +
> > +     val = status;
> > +     pos = 0;
> > +     while ((pos = find_next_bit(&val, MAX_MSI_IRQS_PER_CTRL,
> > +                                 pos)) != MAX_MSI_IRQS_PER_CTRL) {
>
> for_each_set_bit() doesn't work here?

Good question, I just moved the existing DWC code.

>
> > +             generic_handle_domain_irq(pp->irq_domain,
> > +                                       (i * MAX_MSI_IRQS_PER_CTRL) +
> > +                                       pos);
> > +             pos++;
> > +     }
> > +
> > +     return IRQ_HANDLED;
> > +}
> > +
> >  /* MSI int handler */
> >  irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
> >  {
> > -     int i, pos;
> > -     unsigned long val;
> > -     u32 status, num_ctrls;
> > +     int i;
> > +     u32 num_ctrls;
> >       irqreturn_t ret = IRQ_NONE;
> > -     struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> >
> >       num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
> >
> > -     for (i = 0; i < num_ctrls; i++) {
> > -             status = dw_pcie_readl_dbi(pci, PCIE_MSI_INTR0_STATUS +
> > -                                        (i * MSI_REG_CTRL_BLOCK_SIZE));
> > -             if (!status)
> > -                     continue;
> > -
> > -             ret = IRQ_HANDLED;
> > -             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,
> > -                                               (i * MAX_MSI_IRQS_PER_CTRL) +
> > -                                               pos);
> > -                     pos++;
> > -             }
> > -     }
> > +     for (i = 0; i < num_ctrls; i++)
> > +             ret |= dw_handle_single_msi_group(pp, i);
> >
> >       return ret;
> >  }



-- 
With best wishes
Dmitry

[PATCH v12 7/8] dt-bindings: PCI: qcom: 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.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../devicetree/bindings/pci/qcom,pcie.yaml    | 53 +++++++++++++++++--
 1 file changed, 50 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index 0b69b12b849e..fe8f9a62a665 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -43,11 +43,12 @@ properties:
     maxItems: 5
 
   interrupts:
-    maxItems: 1
+    minItems: 1
+    maxItems: 8
 
   interrupt-names:
-    items:
-      - const: msi
+    minItems: 1
+    maxItems: 8
 
   # Common definitions for clocks, clock-names and reset.
   # Platform constraints are described later.
@@ -623,6 +624,52 @@ 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:
+              maxItems: 1
+            interrupt-names:
+              maxItems: 1
+              items:
+                - const: msi
+        - properties:
+            interrupts:
+              minItems: 8
+            interrupt-names:
+              minItems: 8
+              items:
+                - const: msi0
+                - const: msi1
+                - const: msi2
+                - const: msi3
+                - const: msi4
+                - const: msi5
+                - const: msi6
+                - const: msi7
+    else:
+      properties:
+        interrupts:
+          maxItems: 1
+        interrupt-names:
+          items:
+            - const: msi
+
 unevaluatedProperties: false
 
 examples:
-- 
2.35.1

Re: [PATCH v12 7/8] dt-bindings: PCI: qcom: Support additional MSI interrupts

[Rob Herring]

On Mon, May 23, 2022 at 09:18:35PM +0300, Dmitry Baryshkov wrote:
> On Qualcomm platforms each group of 32 MSI vectors is routed to the
> separate GIC interrupt. Document mapping of additional interrupts.
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../devicetree/bindings/pci/qcom,pcie.yaml    | 53 +++++++++++++++++--
>  1 file changed, 50 insertions(+), 3 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH v12 8/8] 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.

Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8250.dtsi | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 410272a1e19b..523a035ffc5f 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -1807,8 +1807,16 @@ 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 = "msi0", "msi1", "msi2", "msi3",
+					  "msi4", "msi5", "msi6", "msi7";
 			#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

Re: [PATCH v12 0/8] PCI: qcom: Fix higher MSI vectors handling

[Bjorn Helgaas]

On Mon, May 23, 2022 at 09:18:28PM +0300, Dmitry Baryshkov wrote:
> 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.
> 
> The first patch in the series is a revert of  [2] (landed in pci-next).
> Either both patches should be applied or both should be dropped.

20f1bfb8dd62 is currently on Lorenzo's pci/qcom branch:

  $ git log --oneline remotes/lorenzo/pci/qcom
  bddedfeb1315 dt-bindings: PCI: qcom: Add schema for sc7280 chipset
  a6f2d6b1b349 dt-bindings: PCI: qcom: Specify reg-names explicitly
  81dab110d351 dt-bindings: PCI: qcom: Do not require resets on msm8996 platforms
  5383d16f0607 dt-bindings: PCI: qcom: Convert to YAML
  3ae93c5a9718 PCI: qcom: Fix unbalanced PHY init on probe errors
  b986db29edbb PCI: qcom: Fix runtime PM imbalance on probe errors
  dcd9011f591a PCI: qcom: Fix pipe clock imbalance
  3007ba831ccd PCI: qcom: Add SM8150 SoC support
  f52d2a0f0d32 dt-bindings: pci: qcom: Document PCIe bindings for SM8150 SoC
  20f1bfb8dd62 PCI: qcom: Add support for handling MSIs from 8 endpoints
  312310928417 Linux 5.18-rc1

Is it safe for me to just drop that single patch before sending the
pull request for v5.19?  Then target the rest of this series for
v5.20?

Bjorn

Re: [PATCH v12 0/8] PCI: qcom: Fix higher MSI vectors handling

[Dmitry Baryshkov]

On Tue, 24 May 2022 at 17:53, Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Mon, May 23, 2022 at 09:18:28PM +0300, Dmitry Baryshkov wrote:
> > 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.
> >
> > The first patch in the series is a revert of  [2] (landed in pci-next).
> > Either both patches should be applied or both should be dropped.
>
> 20f1bfb8dd62 is currently on Lorenzo's pci/qcom branch:
>
>   $ git log --oneline remotes/lorenzo/pci/qcom
>   bddedfeb1315 dt-bindings: PCI: qcom: Add schema for sc7280 chipset
>   a6f2d6b1b349 dt-bindings: PCI: qcom: Specify reg-names explicitly
>   81dab110d351 dt-bindings: PCI: qcom: Do not require resets on msm8996 platforms
>   5383d16f0607 dt-bindings: PCI: qcom: Convert to YAML
>   3ae93c5a9718 PCI: qcom: Fix unbalanced PHY init on probe errors
>   b986db29edbb PCI: qcom: Fix runtime PM imbalance on probe errors
>   dcd9011f591a PCI: qcom: Fix pipe clock imbalance
>   3007ba831ccd PCI: qcom: Add SM8150 SoC support
>   f52d2a0f0d32 dt-bindings: pci: qcom: Document PCIe bindings for SM8150 SoC
>   20f1bfb8dd62 PCI: qcom: Add support for handling MSIs from 8 endpoints
>   312310928417 Linux 5.18-rc1
>
> Is it safe for me to just drop that single patch before sending the
> pull request for v5.19?  Then target the rest of this series for
> v5.20?

Yes and yes.

-- 
With best wishes
Dmitry

Re: [PATCH v12 0/8] PCI: qcom: Fix higher MSI vectors handling

[Bjorn Helgaas]

On Tue, May 24, 2022 at 07:17:42PM +0300, Dmitry Baryshkov wrote:
> On Tue, 24 May 2022 at 17:53, Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > On Mon, May 23, 2022 at 09:18:28PM +0300, Dmitry Baryshkov wrote:
> > > 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.
> > >
> > > The first patch in the series is a revert of  [2] (landed in pci-next).
> > > Either both patches should be applied or both should be dropped.
> >
> > 20f1bfb8dd62 is currently on Lorenzo's pci/qcom branch:
> >
> >   $ git log --oneline remotes/lorenzo/pci/qcom
> >   bddedfeb1315 dt-bindings: PCI: qcom: Add schema for sc7280 chipset
> >   a6f2d6b1b349 dt-bindings: PCI: qcom: Specify reg-names explicitly
> >   81dab110d351 dt-bindings: PCI: qcom: Do not require resets on msm8996 platforms
> >   5383d16f0607 dt-bindings: PCI: qcom: Convert to YAML
> >   3ae93c5a9718 PCI: qcom: Fix unbalanced PHY init on probe errors
> >   b986db29edbb PCI: qcom: Fix runtime PM imbalance on probe errors
> >   dcd9011f591a PCI: qcom: Fix pipe clock imbalance
> >   3007ba831ccd PCI: qcom: Add SM8150 SoC support
> >   f52d2a0f0d32 dt-bindings: pci: qcom: Document PCIe bindings for SM8150 SoC
> >   20f1bfb8dd62 PCI: qcom: Add support for handling MSIs from 8 endpoints
> >   312310928417 Linux 5.18-rc1
> >
> > Is it safe for me to just drop that single patch before sending the
> > pull request for v5.19?  Then target the rest of this series for
> > v5.20?
> 
> Yes and yes.

Great, thank you!  I have dropped that one from my "next" branch.

Re: [PATCH v12 0/8] PCI: qcom: Fix higher MSI vectors handling

[Johan Hovold]

On Mon, May 23, 2022 at 09:18:28PM +0300, Dmitry Baryshkov wrote:
> 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.
> 
> The first patch in the series is a revert of  [2] (landed in pci-next).
> Either both patches should be applied or both should be dropped.
> 
> Patchseries dependecies: [3] (for the schema change).
> 
> Changes since v11 (suggested by Johan):
>  - Added back reporting errors for the "msi0" interrupt,
>  - Stopped overriding num_vectors field if it is less than the amount of
>    MSI vectors deduced from interrupt list,
>  - Added a warning (and an override) if the host specifies more MSI
>    vectors than available,
>  - Moved has_split_msi_irq variable to the patch where it is used.

You forgot to CC me this version. Please remember to keep reviewers on
CC.

Johan