lkml.org 
[lkml]   [2022]   [Sep]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v3 04/14] PCI: dwc: Kconfig: Add iMX PCIe EP mode support
Date
> -----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
\
 
 \ /
  Last update: 2022-09-26 07:26    [W:0.062 / U:0.524 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site