Hi Lorenzo,

Thanks a lot for your comments!

-----Original Message-----
From: Lorenzo Pieralisi <redacted>
Sent: 2019年6月28日 19:36
To: Z.q. Hou <zhiqiang.hou@nxp.com>
Cc: linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com;
l.subrahmanya@mobiveil.co.in; shawnguo@kernel.org; Leo Li
[off-list ref]; catalin.marinas@arm.com; will.deacon@arm.com;
Mingkai Hu [off-list ref]; M.h. Lian [off-list ref];
Xiaowei Bao [off-list ref]
Subject: Re: [PATCHv5 04/20] PCI: mobiveil: Remove the flag
MSI_FLAG_MULTI_PCI_MSI

On Mon, Jun 17, 2019 at 10:34:35AM +0000, Z.q. Hou wrote:

[...]

quoted

quoted

There is nothing obvious. Write what you are fixing in the commit
log and I will apply the patch, I won't write the commit log for
you. Anyone should be able to understand why a patch was needed by
reading the commit log, it is as important as writing the code itself.

With the flag MSI_FLAG_MULTI_PCI_MSI, when the Endpoint allocates
multiple MSI, it will trigger the "WARN_ON(nr_irqs != 1);" in
mobiveil_irq_msi_domain_alloc(), this is the issue this patch want to
fix.

And that's wrong. Marc explained why this controller does not support Multi
MSI and that's what should go in the commit log, triggering a WARN_ON is
the least of the problems (and the WARN_ON can even be removed after
this patch is applied), if it was used as a bandaid to prevent allocating Multi
MSI it is even more broken.

Yes, I agree, the root cause is hardware limitation, is the following changelog
acceptable?

Changelog:
The Mobiveil internal MSI controller uses separate target address per MSI,
so, it is unable to support Multiple MSI feature, which requires the same
target address and incremental MSI_DATA values. This patch is to remove
the flag MSI_FLAG_MULTI_PCI_MSI.

Thanks,
Zhiqiang
 

Lorenzo

quoted

Thanks,
Zhiqiang

quoted

Thanks,
Lorenzo

quoted

Thanks,
Zhiqiang

quoted

Lorenzo

quoted

Subbu, did you test with Endpoint supporting multi MSI?

Thanks,
Zhiqiang

quoted

Thanks,
Lorenzo

quoted

Fixes: 1e913e58335f ("PCI: mobiveil: Add MSI support")
Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
Reviewed-by: Minghuan Lian <Minghuan.Lian@nxp.com>
---
V5:
 - Corrected the subject.

 drivers/pci/controller/pcie-mobiveil.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-mobiveil.c

b/drivers/pci/controller/pcie-mobiveil.c
index 563210e731d3..a0dd337c6214 100644

--- a/drivers/pci/controller/pcie-mobiveil.c
+++ b/drivers/pci/controller/pcie-mobiveil.c

@@ -703,7 +703,7 @@ static struct irq_chip

mobiveil_msi_irq_chip = {

 static struct msi_domain_info mobiveil_msi_domain_info = {
 	.flags	= (MSI_FLAG_USE_DEF_DOM_OPS |

MSI_FLAG_USE_DEF_CHIP_OPS |

quoted

-		   MSI_FLAG_MULTI_PCI_MSI | MSI_FLAG_PCI_MSIX),
+		   MSI_FLAG_PCI_MSIX),
 	.chip	= &mobiveil_msi_irq_chip,
 };

--
2.17.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel