| Message ID | 20220707134733.2436629-1-dmitry.baryshkov@linaro.org |
|---|---|
| Headers | show |
| Series | PCI: dwc: Fix higher MSI vectors handling | expand |
On 07/07/2022 16:47, 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. [skipped] Gracious ping. Does this series stand a chance of getting into 5.20? > 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 > 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(-) >
On 14/07/2022 14:58, Dmitry Baryshkov wrote: > On 07/07/2022 16:47, 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. > > [skipped] > > Gracious ping. Does this series stand a chance of getting into 5.20? Bjorn, please excuse my insistence. Given that it's not merged (yet), probably it will not make it into 5.20. Is there anything preventing it from being accepted for 5.21? dwc patches were reviewed by Rob, Mani and Johan. Stanimir has acked the bindings patch. The dts patch should probably go via the arm-soc tree (together with additional patches adding multiple MSI IRQs to other Qualcomm SoCs). > >> 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 >> 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(-) >> > >
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 >
On Fri, 22 Jul 2022 at 20:09, Bjorn Helgaas <helgaas@kernel.org> wrote: > > 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. Ack, thank you. I do not have strong preference, so let it be so. > > I left the arch/arm64/boot/dts/qcom/sm8250.dtsi change below out > on the assumption it will go via arm-soc. Yes. Thanks a lot! > > > 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 > >
On Thu, 7 Jul 2022 16:47:27 +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. > > [...] Applied, thanks! [6/6] arm64: dts: qcom: sm8250: provide additional MSI interrupts commit: f2819650aab5b037e5e730c88abcd971e96a1637 Best regards,
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 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(-)