diff mbox series

[v6,02/10] PCI: imx6: Fix i.MX8MP PCIe EP's occasional failure to trigger MSI

Message ID 20240617-pci2_upstream-v6-2-e0821238f997@nxp.com (mailing list archive)
State Superseded
Headers show
Series PCI: imx6: Fix\rename\clean up and add lut information for imx95 | expand

Commit Message

Frank Li June 17, 2024, 8:16 p.m. UTC 
From: Richard Zhu <hongxing.zhu@nxp.com>

Correct occasional MSI triggering failures in i.MX8MP PCIe EP by apply 64KB
hardware alignment requirement.

MSI triggering fail if the outbound MSI memory region (ep->msi_mem) is not
aligned to 64KB.

In dw_pcie_ep_init():

ep->msi_mem = pci_epc_mem_alloc_addr(epc, &ep->msi_mem_phys,
				     epc->mem->window.page_size);

Set ep->page_size to match drvdata::epc_features::align since different
SOCs have different alignment requirements.

Fixes: 1bd0d43dcf3b ("PCI: imx6: Clean up addr_space retrieval code")
Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Acked-by: Jason Liu <jason.hui.liu@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Manivannan Sadhasivam June 29, 2024, 1:05 p.m. UTC | #1 
On Mon, Jun 17, 2024 at 04:16:38PM -0400, Frank Li wrote:
> From: Richard Zhu <hongxing.zhu@nxp.com>
> 
> Correct occasional MSI triggering failures in i.MX8MP PCIe EP by apply 64KB
> hardware alignment requirement.
> 
> MSI triggering fail if the outbound MSI memory region (ep->msi_mem) is not
> aligned to 64KB.
> 
> In dw_pcie_ep_init():
> 
> ep->msi_mem = pci_epc_mem_alloc_addr(epc, &ep->msi_mem_phys,
> 				     epc->mem->window.page_size);
> 

So this is an alignment restriction w.r.t iATU. In that case, we should be
passing 'pci_epc_features::align' instead?

- Mani

> Set ep->page_size to match drvdata::epc_features::align since different
> SOCs have different alignment requirements.
> 
> Fixes: 1bd0d43dcf3b ("PCI: imx6: Clean up addr_space retrieval code")
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> Acked-by: Jason Liu <jason.hui.liu@nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 9a71b8aa09b3c..ca9a000c9a96d 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1118,6 +1118,8 @@ static int imx6_add_pcie_ep(struct imx6_pcie *imx6_pcie,
>  	if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_SUPPORT_64BIT))
>  		dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
>  
> +	ep->page_size = imx6_pcie->drvdata->epc_features->align;
> +
>  	ret = dw_pcie_ep_init(ep);
>  	if (ret) {
>  		dev_err(dev, "failed to initialize endpoint\n");
> 
> -- 
> 2.34.1
>
Frank Li July 1, 2024, 6:32 p.m. UTC | #2 
On Sat, Jun 29, 2024 at 06:35:25PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Jun 17, 2024 at 04:16:38PM -0400, Frank Li wrote:
> > From: Richard Zhu <hongxing.zhu@nxp.com>
> > 
> > Correct occasional MSI triggering failures in i.MX8MP PCIe EP by apply 64KB
> > hardware alignment requirement.
> > 
> > MSI triggering fail if the outbound MSI memory region (ep->msi_mem) is not
> > aligned to 64KB.
> > 
> > In dw_pcie_ep_init():
> > 
> > ep->msi_mem = pci_epc_mem_alloc_addr(epc, &ep->msi_mem_phys,
> > 				     epc->mem->window.page_size);
> > 
> 
> So this is an alignment restriction w.r.t iATU. In that case, we should be
> passing 'pci_epc_features::align' instead?

pci_epc_features::align already set.

pci_epc_mem_alloc_addr(
	...
	align_size = ALIGN(size, mem->window.page_size);
	order = pci_epc_mem_get_order(mem, align_size);
	...
}

but pci_epc_mem_alloc_addr() align to page_size, instead of
pci_epc_features::align.

Frank

> 
> - Mani
> 
> > Set ep->page_size to match drvdata::epc_features::align since different
> > SOCs have different alignment requirements.
> > 
> > Fixes: 1bd0d43dcf3b ("PCI: imx6: Clean up addr_space retrieval code")
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > Acked-by: Jason Liu <jason.hui.liu@nxp.com>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > index 9a71b8aa09b3c..ca9a000c9a96d 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -1118,6 +1118,8 @@ static int imx6_add_pcie_ep(struct imx6_pcie *imx6_pcie,
> >  	if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_SUPPORT_64BIT))
> >  		dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> >  
> > +	ep->page_size = imx6_pcie->drvdata->epc_features->align;
> > +
> >  	ret = dw_pcie_ep_init(ep);
> >  	if (ret) {
> >  		dev_err(dev, "failed to initialize endpoint\n");
> > 
> > -- 
> > 2.34.1
> > 
> 
> -- 
> மணிவண்ணன் சதாசிவம்
Manivannan Sadhasivam July 6, 2024, 5:39 p.m. UTC | #3 
On Mon, Jul 01, 2024 at 02:32:59PM -0400, Frank Li wrote:
> On Sat, Jun 29, 2024 at 06:35:25PM +0530, Manivannan Sadhasivam wrote:
> > On Mon, Jun 17, 2024 at 04:16:38PM -0400, Frank Li wrote:
> > > From: Richard Zhu <hongxing.zhu@nxp.com>
> > > 
> > > Correct occasional MSI triggering failures in i.MX8MP PCIe EP by apply 64KB
> > > hardware alignment requirement.
> > > 
> > > MSI triggering fail if the outbound MSI memory region (ep->msi_mem) is not
> > > aligned to 64KB.
> > > 
> > > In dw_pcie_ep_init():
> > > 
> > > ep->msi_mem = pci_epc_mem_alloc_addr(epc, &ep->msi_mem_phys,
> > > 				     epc->mem->window.page_size);
> > > 
> > 
> > So this is an alignment restriction w.r.t iATU. In that case, we should be
> > passing 'pci_epc_features::align' instead?
> 
> pci_epc_features::align already set.
> 
> pci_epc_mem_alloc_addr(
> 	...
> 	align_size = ALIGN(size, mem->window.page_size);
> 	order = pci_epc_mem_get_order(mem, align_size);
> 	...
> }
> 
> but pci_epc_mem_alloc_addr() align to page_size, instead of
> pci_epc_features::align.
> 

'window.page_size' is set to what is passed as 'page_size' argument to
pci_epc_mem_init(). In this case, 'ep->page_size' is passed which corresponds to
size of pages that can be allocated within the memory window.

Default value of 'ep->page_size' is PAGE_SIZE which is most likely 4K. So if
your hardware cannot allocate 4K pages within the memory window, then it
doesn't support splitting this OB region into 4K pages.

But this has nothing to do with alignment AFAIU since epc_features::align is
used for IB memory. This 'page_size' argument was introduced for some TI SoC
that doesn't handle PAGE_SIZE splitting of OB memory window. Reference:

52c9285d4745 ("PCI: endpoint: Add support for configurable page size")

Can you check if your SoC also suffers from the same limitation? If so, then you
should modify the commit message to make it clear.

- Mani
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 9a71b8aa09b3c..ca9a000c9a96d 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1118,6 +1118,8 @@  static int imx6_add_pcie_ep(struct imx6_pcie *imx6_pcie,
 	if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_SUPPORT_64BIT))
 		dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
 
+	ep->page_size = imx6_pcie->drvdata->epc_features->align;
+
 	ret = dw_pcie_ep_init(ep);
 	if (ret) {
 		dev_err(dev, "failed to initialize endpoint\n");