diff mbox series

[v3,04/14] PCI: dwc: Kconfig: Add iMX PCIe EP mode support

Message ID 1663913220-9523-5-git-send-email-hongxing.zhu@nxp.com
State New
Headers show
Series Add iMX PCIe EP mode support | expand

Commit Message

Hongxing Zhu Sept. 23, 2022, 6:06 a.m. UTC
Since i.MX PCIe is one dual mode PCIe controller.
Add i.MX PCIe EP mode support, and split the PCIe modes to the Root
Complex mode and Endpoint mode.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 drivers/pci/controller/dwc/Kconfig | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

Comments

Bjorn Helgaas Sept. 23, 2022, 2:15 p.m. UTC | #1
On Fri, Sep 23, 2022 at 02:06:50PM +0800, Richard Zhu wrote:
> Since i.MX PCIe is one dual mode PCIe controller.

This is not a sentence.

> Add i.MX PCIe EP mode support, and split the PCIe modes to the Root
> Complex mode and Endpoint mode.

Add blank lines between paragraphs or rewrap into a single paragraph
that fills 75 columns.

I think you should split "[12/14] PCI: imx6: Add iMX8MM PCIe EP mode"
into:

  - A patch that adds the generic endpoint infrastructure, e.g.,
    imx6_pcie_ep_init(), imx6_pcie_ep_raise_irq(), imx6_add_pcie_ep().

  - A second patch that adds the i.MX8MM identifiers.

That way the i.MX8MM patch will be analogous to the i.MX8MQ and
i.MX8MP patches.

Then you could squash this Kconfig patch into the generic endpoint
infrastructure patch because this patch is what selects PCIE_DW_EP,
which is what ensures that dw_pcie_ep_reset_bar(),
dw_pcie_ep_raise_legacy_irq(), etc., are available.

> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -92,10 +92,33 @@ config PCI_EXYNOS
>  	  functions to implement the driver.
>  
>  config PCI_IMX6
> -	bool "Freescale i.MX6/7/8 PCIe controller"
> +	bool
> +
> +config PCI_IMX6_HOST
> +	bool "Freescale i.MX6/7/8 PCIe controller host mode"
>  	depends on ARCH_MXC || COMPILE_TEST
>  	depends on PCI_MSI_IRQ_DOMAIN
>  	select PCIE_DW_HOST
> +	select PCI_IMX6
> +	help
> +	  Enables support for the PCIe controller Root Complex mode in the
> +	  iMX6/7/8 SoCs.

> +	  This controller can work either as EP or RC. In order to enable
> +	  host-specific features PCIE_DW_HOST must be selected and in order
> +	  to enable device-specific features PCIE_DW_EP must be selected.

I don't think these three lines are useful to the user.  They only
describe what Kconfig does when PCI_IMX6_HOST is enabled, which is
really an internal implementation detail.

> +config PCI_IMX6_EP
> +	bool "Freescale i.MX6/7/8 PCIe controller endpoint mode"
> +	depends on ARCH_MXC || COMPILE_TEST
> +	depends on PCI_ENDPOINT
> +	select PCIE_DW_EP
> +	select PCI_IMX6
> +	help
> +	  Enables support for the PCIe controller endpoint mode in the
> +	  iMX6/7/8 SoCs.
> +	  This controller can work either as EP or RC. In order to enable
> +	  host-specific features PCIE_DW_HOST must be selected and in order
> +	  to enable device-specific features PCIE_DW_EP must be selected.

Ditto.

>  config PCIE_SPEAR13XX
>  	bool "STMicroelectronics SPEAr PCIe controller"
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hongxing Zhu Sept. 26, 2022, 5:24 a.m. UTC | #2
> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 2022年9月23日 22:15
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: l.stach@pengutronix.de; bhelgaas@google.com; robh+dt@kernel.org;
> lorenzo.pieralisi@arm.com; shawnguo@kernel.org; kishon@ti.com;
> kw@linux.com; Frank Li <frank.li@nxp.com>; linux-pci@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; kernel@pengutronix.de; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: Re: [PATCH v3 04/14] PCI: dwc: Kconfig: Add iMX PCIe EP mode
> support
> 
> On Fri, Sep 23, 2022 at 02:06:50PM +0800, Richard Zhu wrote:
> > Since i.MX PCIe is one dual mode PCIe controller.
> 
> This is not a sentence.
Okay, would be changed.

> 
> > Add i.MX PCIe EP mode support, and split the PCIe modes to the Root
> > Complex mode and Endpoint mode.
> 
> Add blank lines between paragraphs or rewrap into a single paragraph that fills
> 75 columns.
Okay, would refine the commit log later.

> 
> I think you should split "[12/14] PCI: imx6: Add iMX8MM PCIe EP mode"
> into:
> 
>   - A patch that adds the generic endpoint infrastructure, e.g.,
>     imx6_pcie_ep_init(), imx6_pcie_ep_raise_irq(), imx6_add_pcie_ep().
> 
>   - A second patch that adds the i.MX8MM identifiers.
> 
> That way the i.MX8MM patch will be analogous to the i.MX8MQ and i.MX8MP
> patches.
> 
> Then you could squash this Kconfig patch into the generic endpoint
> infrastructure patch because this patch is what selects PCIE_DW_EP, which is
> what ensures that dw_pcie_ep_reset_bar(), dw_pcie_ep_raise_legacy_irq(), etc.,
> are available.
Good suggestion. Thanks a lot.
Would change it following this way.

> 
> > --- a/drivers/pci/controller/dwc/Kconfig
> > +++ b/drivers/pci/controller/dwc/Kconfig
> > @@ -92,10 +92,33 @@ config PCI_EXYNOS
> >  	  functions to implement the driver.
> >
> >  config PCI_IMX6
> > -	bool "Freescale i.MX6/7/8 PCIe controller"
> > +	bool
> > +
> > +config PCI_IMX6_HOST
> > +	bool "Freescale i.MX6/7/8 PCIe controller host mode"
> >  	depends on ARCH_MXC || COMPILE_TEST
> >  	depends on PCI_MSI_IRQ_DOMAIN
> >  	select PCIE_DW_HOST
> > +	select PCI_IMX6
> > +	help
> > +	  Enables support for the PCIe controller Root Complex mode in the
> > +	  iMX6/7/8 SoCs.
> 
> > +	  This controller can work either as EP or RC. In order to enable
> > +	  host-specific features PCIE_DW_HOST must be selected and in order
> > +	  to enable device-specific features PCIE_DW_EP must be selected.
> 
> I don't think these three lines are useful to the user.  They only describe what
> Kconfig does when PCI_IMX6_HOST is enabled, which is really an internal
> implementation detail.
Okay, would refine them later.

> 
> > +config PCI_IMX6_EP
> > +	bool "Freescale i.MX6/7/8 PCIe controller endpoint mode"
> > +	depends on ARCH_MXC || COMPILE_TEST
> > +	depends on PCI_ENDPOINT
> > +	select PCIE_DW_EP
> > +	select PCI_IMX6
> > +	help
> > +	  Enables support for the PCIe controller endpoint mode in the
> > +	  iMX6/7/8 SoCs.
> > +	  This controller can work either as EP or RC. In order to enable
> > +	  host-specific features PCIE_DW_HOST must be selected and in order
> > +	  to enable device-specific features PCIE_DW_EP must be selected.
> 
> Ditto.
Okay, would refine them later.

Best Regards
Richard Zhu

> 
> >  config PCIE_SPEAR13XX
> >  	bool "STMicroelectronics SPEAr PCIe controller"
> > --
> > 2.25.1
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists
> > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&amp;data=05%7
> C0
> >
> 1%7Chongxing.zhu%40nxp.com%7Cc9b4cdb1fe6048bcbf2808da9d6e1245%7
> C686ea1
> >
> d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637995393327822891%7CUnk
> nown%7CTW
> >
> FpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXV
> CI6
> >
> Mn0%3D%7C3000%7C%7C%7C&amp;sdata=mtOA1T3wSvr4IRfbF8WpMlu%2B
> pMh6vQDPKab
> > 17Y4zdeE%3D&amp;reserved=0
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 62ce3abf0f19..a24d8cacf1be 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -92,10 +92,33 @@  config PCI_EXYNOS
 	  functions to implement the driver.
 
 config PCI_IMX6
-	bool "Freescale i.MX6/7/8 PCIe controller"
+	bool
+
+config PCI_IMX6_HOST
+	bool "Freescale i.MX6/7/8 PCIe controller host mode"
 	depends on ARCH_MXC || COMPILE_TEST
 	depends on PCI_MSI_IRQ_DOMAIN
 	select PCIE_DW_HOST
+	select PCI_IMX6
+	help
+	  Enables support for the PCIe controller Root Complex mode in the
+	  iMX6/7/8 SoCs.
+	  This controller can work either as EP or RC. In order to enable
+	  host-specific features PCIE_DW_HOST must be selected and in order
+	  to enable device-specific features PCIE_DW_EP must be selected.
+
+config PCI_IMX6_EP
+	bool "Freescale i.MX6/7/8 PCIe controller endpoint mode"
+	depends on ARCH_MXC || COMPILE_TEST
+	depends on PCI_ENDPOINT
+	select PCIE_DW_EP
+	select PCI_IMX6
+	help
+	  Enables support for the PCIe controller endpoint mode in the
+	  iMX6/7/8 SoCs.
+	  This controller can work either as EP or RC. In order to enable
+	  host-specific features PCIE_DW_HOST must be selected and in order
+	  to enable device-specific features PCIE_DW_EP must be selected.
 
 config PCIE_SPEAR13XX
 	bool "STMicroelectronics SPEAr PCIe controller"