Re: [PATCH v7 5/7] PCI: qcom: Handle MSIs routed to multiple GIC interrupts

From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Rob Herring <robh@kernel.org>
Cc: Andy Gross <agross@kernel.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Jingoo Han <jingoohan1@gmail.com>,
	Gustavo Pimentel <gustavo.pimentel@synopsys.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Stanimir Varbanov <svarbanov@mm-sol.com>,
	Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
	Vinod Koul <vkoul@kernel.org>,
	linux-arm-msm@vger.kernel.org, linux-pci@vger.kernel.org,
	devicetree@vger.kernel.org
Subject: Re: [PATCH v7 5/7] PCI: qcom: Handle MSIs routed to multiple GIC interrupts
Date: Fri, 6 May 2022 10:40:56 +0300	[thread overview]
Message-ID: <b334a2e6-69ae-690d-8560-25f8a1319e5c@linaro.org> (raw)
In-Reply-To: <YnRA//LbCW+IVi3o@robh.at.kernel.org>

On 06/05/2022 00:26, Rob Herring wrote:
> On Thu, May 05, 2022 at 04:54:05PM +0300, Dmitry Baryshkov wrote:
>> 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.
> 
> msi_host_init() has 1 user (keystone) as it doesn't use the DWC MSI
> controller. But QCom does given the access to PCIE_MSI_INTR0_STATUS,
> so mutiple MSI IRQ outputs must have been added in newer versions of the
> DWC IP. If so, it's only a matter of time for another platform to
> do the same thing. Maybe someone from Synopsys could confirm?

This is a valid question, and if you check, first iterations of this 
patchset had this in the dwc core ([1], [2]). Exactly for the reason 
this might be usable for other platforms.

Then I did some research for other platforms using DWC PCIe IP core. For 
example, both Tegra Xavier and iMX6 support up to 256 MSI vectors, they 
use DWC MSI IRQ controller. The iMX6 TRM explicitly describes using 
different MSI groups for different endpoints. The diagram shows 8 MSI 
IRQ signal lines. However in the end the signals from all groups are 
OR'ed to form a single host msi_ctrl_int. Thus currently I suppose that 
using multiple MSI IRQs is a peculiarity of Qualcomm platform.

> 
> Therefore this should all be handled in the DWC core. In general, I
> don't want to see more users nor more ops if we don't have to. Let's not
> create ops for what can be handled as data. AFAICT, this is just number
> of MSIs and # of MSIs per IRQ. It seems plausible another platform could
> do something similar and supporting it in the core code wouldn't
> negatively impact other platforms.

I wanted to balance adding additional ops vs complicating the core for 
other platforms. And I still suppose that platform specifics should go 
to the platform driver. However if you prefer [1] and [2], we can go 
back to that implementation.

[1]: 
https://lore.kernel.org/linux-arm-msm/20220427121653.3158569-2-dmitry.baryshkov@linaro.org/[2]: 
https://lore.kernel.org/linux-arm-msm/20220427121653.3158569-3-dmitry.baryshkov@linaro.org/

-- 
With best wishes
Dmitry