[PATCH v9 16/16] PCI: imx6: Add iMX95 Endpoint (EP) support

Tue Feb 13 07:35:50 PST 2024

On Tue, Feb 13, 2024 at 01:23:45PM +0000, Robin Murphy wrote:
> On 13/02/2024 10:38 am, Lorenzo Pieralisi wrote:
> > [+Christoph, Robin - just checking with you if the DMA mask handling is
> > what you expect from drivers, don't want to merge code that breaks your
> > expectations]
> 
> I don't really understand how all the endpoint stuff fits together, but my
> hunch would be that setting the DMA masks in imx6_add_pcie_ep() might be
> more sensible, unless perhaps there's an implied intent to account for eDMA

pci_epc_mem_alloc_addr() ioremap PCIe EP side outbound memory. Without it
it will be mapped to lower 4G space / SWIOTLB. Generally, outbound is quite
big in some chips, such imx95, (4GB). So it need set dmamask to make sure
it can be mapped to > 4G space.

> channels in root complex mode as well (and so assuming that eDMA and
> endpoint mode play nicely together sharing the same platform device) - as

There are eDMA in host side, but still not find a usecase yet. So it is not
enabled yet. I just need add flags IMX6_PCIE_FLAG_SUPPORT_64BIT in imx95
host if needs.

Anyways, if you like, I can move it into imx6_add_pcie_ep() at next
version.

Frank

> we've discussed before across various threads, setting DMA masks for a host
> bridge itself doesn't really make sense (and hence it leaves the gap where
> we can get away with co-opting them for the MSI address hack); it's any
> additional DMA-initiating functionality within a root complex which should
> own that responsibility.
> 
> FWIW it looks like the equivalent change for Layerscape *is* specific to
> endpoint mode, but I don't know how relevant that is to i.MX given that
> they're unrelated hardware configurations.

Previous i.MX PCI outbound space < 4G space. And only have 32bit address
access in system bus. So needn't set DMA mask. i.MX95 change it and PCIe
part is inheriate from layerscape.

Frank
> 
> Thanks,
> Robin.
> 
> > On Fri, Jan 19, 2024 at 12:11:22PM -0500, Frank Li wrote:
> > > Add iMX95 EP support and add 64bit address support. Internal bus bridge for
> > > PCI support 64bit dma address in iMX95. Hence, call
> > > dma_set_mask_and_coherent() to set 64 bit DMA mask.
> > > 
> > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam at linaro.org>
> > > Signed-off-by: Frank Li <Frank.Li at nxp.com>
> > > ---
> > > 
> > > Notes:
> > >      Change from v8 to v9
> > >      - update fixme comments
> > >      - update BAR1 comments
> > >      - Add mani's review tag
> > >      Change from v7 to v8
> > >      - Update commit message
> > >      - Using Fixme
> > >      - Update clks_cnts by ARRAY_SIZE
> > >      Change from v4 to v7
> > >      - none
> > >      Change from v3 to v4
> > >      - change align to 4k for imx95
> > >      Change from v1 to v3
> > >      - new patches at v3
> > > 
> > >   drivers/pci/controller/dwc/pci-imx6.c | 47 +++++++++++++++++++++++++++
> > >   1 file changed, 47 insertions(+)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > > index bbaa45c08323b..7889318f6555a 100644
> > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > @@ -75,6 +75,7 @@ enum imx6_pcie_variants {
> > >   	IMX8MQ_EP,
> > >   	IMX8MM_EP,
> > >   	IMX8MP_EP,
> > > +	IMX95_EP,
> > >   };
> > >   #define IMX6_PCIE_FLAG_IMX6_PHY			BIT(0)
> > > @@ -84,6 +85,7 @@ enum imx6_pcie_variants {
> > >   #define IMX6_PCIE_FLAG_HAS_APP_RESET		BIT(4)
> > >   #define IMX6_PCIE_FLAG_HAS_PHY_RESET		BIT(5)
> > >   #define IMX6_PCIE_FLAG_HAS_SERDES		BIT(6)
> > > +#define IMX6_PCIE_FLAG_SUPPORT_64BIT		BIT(7)
> > >   #define imx6_check_flag(pci, val)     (pci->drvdata->flags & val)
> > > @@ -616,6 +618,7 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
> > >   		break;
> > >   	case IMX7D:
> > >   	case IMX95:
> > > +	case IMX95_EP:
> > >   		break;
> > >   	case IMX8MM:
> > >   	case IMX8MM_EP:
> > > @@ -1050,6 +1053,23 @@ static const struct pci_epc_features imx8m_pcie_epc_features = {
> > >   	.align = SZ_64K,
> > >   };
> > > +/*
> > > + * BAR#	| Default BAR enable	| Default BAR Type	| Default BAR Size	| BAR Sizing Scheme
> > > + * ================================================================================================
> > > + * BAR0	| Enable		| 64-bit		| 1 MB			| Programmable Size
> > > + * BAR1	| Disable		| 32-bit		| 64 KB			| Fixed Size
> > > + *        BAR1 should be disabled if BAR0 is 64bit.
> > > + * BAR2	| Enable		| 32-bit		| 1 MB			| Programmable Size
> > > + * BAR3	| Enable		| 32-bit		| 64 KB			| Programmable Size
> > > + * BAR4	| Enable		| 32-bit		| 1M			| Programmable Size
> > > + * BAR5	| Enable		| 32-bit		| 64 KB			| Programmable Size
> > > + */
> > > +static const struct pci_epc_features imx95_pcie_epc_features = {
> > > +	.msi_capable = true,
> > > +	.bar_fixed_size[1] = SZ_64K,
> > > +	.align = SZ_4K,
> > > +};
> > > +
> > >   static const struct pci_epc_features*
> > >   imx6_pcie_ep_get_features(struct dw_pcie_ep *ep)
> > >   {
> > > @@ -1092,6 +1112,15 @@ static int imx6_add_pcie_ep(struct imx6_pcie *imx6_pcie,
> > >   	pci->dbi_base2 = pci->dbi_base + pcie_dbi2_offset;
> > > +	/*
> > > +	 * FIXME: Ideally, dbi2 base address should come from DT. But since only IMX95 is defining
> > > +	 * "dbi2" in DT, "dbi_base2" is set to NULL here for that platform alone so that the DWC
> > > +	 * core code can fetch that from DT. But once all platform DTs were fixed, this and the
> > > +	 * above "dbi_base2" setting should be removed.
> > > +	 */
> > > +	if (imx6_pcie->drvdata->variant == IMX95_EP)
> > > +		pci->dbi_base2 = NULL;
> > > +
> > >   	ret = dw_pcie_ep_init(ep);
> > >   	if (ret) {
> > >   		dev_err(dev, "failed to initialize endpoint\n");
> > > @@ -1345,6 +1374,9 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> > >   					     "unable to find iomuxc registers\n");
> > >   	}
> > > +	if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_SUPPORT_64BIT))
> > > +		dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> > > +
> > >   	/* Grab PCIe PHY Tx Settings */
> > >   	if (of_property_read_u32(node, "fsl,tx-deemph-gen1",
> > >   				 &imx6_pcie->tx_deemph_gen1))
> > > @@ -1563,6 +1595,20 @@ static const struct imx6_pcie_drvdata drvdata[] = {
> > >   		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
> > >   		.epc_features = &imx8m_pcie_epc_features,
> > >   	},
> > > +	[IMX95_EP] = {
> > > +		.variant = IMX95_EP,
> > > +		.flags = IMX6_PCIE_FLAG_HAS_SERDES |
> > > +			 IMX6_PCIE_FLAG_SUPPORT_64BIT,
> > > +		.clk_names = imx8mq_clks,
> > > +		.clks_cnt = ARRAY_SIZE(imx8mq_clks),
> > > +		.ltssm_off = IMX95_PE0_GEN_CTRL_3,
> > > +		.ltssm_mask = IMX95_PCIE_LTSSM_EN,
> > > +		.mode_off[0]  = IMX95_PE0_GEN_CTRL_1,
> > > +		.mode_mask[0] = IMX95_PCIE_DEVICE_TYPE,
> > > +		.init_phy = imx95_pcie_init_phy,
> > > +		.epc_features = &imx95_pcie_epc_features,
> > > +		.mode = DW_PCIE_EP_TYPE,
> > > +	},
> > >   };
> > >   static const struct of_device_id imx6_pcie_of_match[] = {
> > > @@ -1577,6 +1623,7 @@ static const struct of_device_id imx6_pcie_of_match[] = {
> > >   	{ .compatible = "fsl,imx8mq-pcie-ep", .data = &drvdata[IMX8MQ_EP], },
> > >   	{ .compatible = "fsl,imx8mm-pcie-ep", .data = &drvdata[IMX8MM_EP], },
> > >   	{ .compatible = "fsl,imx8mp-pcie-ep", .data = &drvdata[IMX8MP_EP], },
> > > +	{ .compatible = "fsl,imx95-pcie-ep", .data = &drvdata[IMX95_EP], },
> > >   	{},
> > >   };
> > > -- 
> > > 2.34.1
> > > 