Re: [PATCH v17 0/6] PCI: dwc: Fix higher MSI vectors handling

[Bjorn Helgaas <helgaas@kernel.org>]

On Thu, Jul 07, 2022 at 04:47:27PM +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.
> 
> Changes since v16:
>  - Fix a typo in the dt schema (noticed and fixed by Johan).
> 
> Changes since v15:
>  - Rebased on top of linux-next to take care of the conflict with the
>    comit 27235cd867cf ("PCI: dwc: Fix MSI msi_msg DMA mapping").
> 
> Changes since v14:
>  - Fixed the dtschema warnings in qcom,pcie.yaml (reported by Rob
>    Herring)
> 
> Changes since v13:
>  - Changed msiX from pointer to the char array (reported by Johan).
> 
> Changes since v12:
>  - Dropped split_msi_names array in favour of generating the msi_name on
>    the fly (Rob),
>  - Dropped separate split MSI ISR as requested by Rob,
>  - Many small syntax & spelling changes as suggested by Johan and Rob,
>  - Moved a revert to be a last patch, as it is now a reminder to
>    Lorenzo,
>  - Renamed series to name dwc rather than qcom, as the are no more
>    actual changes to the qcom PCIe driver (Johan thanks for all
>    suggestions for making the code to work as is).
> 
> 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/
> 
> 
> Dmitry Baryshkov (6):
>   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
>   dt-bindings: PCI: qcom: Support additional MSI interrupts

I applied the above to pci/ctrl/dwc for v5.20, thanks!

I reordered "split MSI stuff to separate function" before "convert to
array" to reduce the complexity first before adding more.

I also dropped the "irq" temporary in "Convert msi_irq to the array",
not because I necessary object to it, but because it's not strictly
related to converting to an array.  Previously we might have left
pp->msi_irq containing an error code (< 0), but the same situation
after the patch would leave pp->msi_irq[0] == 0.

I left the arch/arm64/boot/dts/qcom/sm8250.dtsi change below out
on the assumption it will go via arm-soc.

>   arm64: dts: qcom: sm8250: provide additional MSI interrupts
> 
>  .../devicetree/bindings/pci/qcom,pcie.yaml    |  51 +++++-
>  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 | 164 +++++++++++++-----
>  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 +-
>  9 files changed, 185 insertions(+), 54 deletions(-)
> 
> -- 
> 2.35.1
>