diff mbox series

[V3,3/3] pci: imx: Select RESET_IMX7 by default

Message ID 1595254921-26050-3-git-send-email-Anson.Huang@nxp.com (mailing list archive)
State New, archived
Headers show
Series [V3,1/3] reset: imx7: Support module build | expand

Commit Message

Anson Huang July 20, 2020, 2:22 p.m. UTC
i.MX7 reset driver now supports module build and it is no longer
built in by default, so i.MX PCI driver needs to select it explicitly
due to it is NOT supporting loadable module currently.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No change.
---
 drivers/pci/controller/dwc/Kconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

Lorenzo Pieralisi July 28, 2020, 10:51 a.m. UTC | #1
[CCing IMX6 maintainers]

On Mon, Jul 20, 2020 at 10:22:01PM +0800, Anson Huang wrote:
> i.MX7 reset driver now supports module build and it is no longer
> built in by default, so i.MX PCI driver needs to select it explicitly
> due to it is NOT supporting loadable module currently.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> No change.
> ---
>  drivers/pci/controller/dwc/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 044a376..bcf63ce 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -90,6 +90,7 @@ config PCI_EXYNOS
>  
>  config PCI_IMX6
>  	bool "Freescale i.MX6/7/8 PCIe controller"
> +	select RESET_IMX7
>  	depends on ARCH_MXC || COMPILE_TEST
>  	depends on PCI_MSI_IRQ_DOMAIN
>  	select PCIE_DW_HOST
> -- 
> 2.7.4
>
Rob Herring (Arm) July 29, 2020, 3:26 p.m. UTC | #2
On Mon, Jul 20, 2020 at 8:26 AM Anson Huang <Anson.Huang@nxp.com> wrote:
>
> i.MX7 reset driver now supports module build and it is no longer
> built in by default, so i.MX PCI driver needs to select it explicitly
> due to it is NOT supporting loadable module currently.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> No change.
> ---
>  drivers/pci/controller/dwc/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 044a376..bcf63ce 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -90,6 +90,7 @@ config PCI_EXYNOS
>
>  config PCI_IMX6
>         bool "Freescale i.MX6/7/8 PCIe controller"
> +       select RESET_IMX7

This will break as select will not cause all of RESET_IMX7's
dependencies to be met. It also doesn't scale. Are you going to do the
same thing for clocks, pinctrl, gpio, etc.?

You should make the PCI driver work as a module.

Rob

>         depends on ARCH_MXC || COMPILE_TEST
>         depends on PCI_MSI_IRQ_DOMAIN
>         select PCIE_DW_HOST
> --
> 2.7.4
>
Philipp Zabel July 29, 2020, 3:47 p.m. UTC | #3
On Wed, 2020-07-29 at 09:26 -0600, Rob Herring wrote:
> On Mon, Jul 20, 2020 at 8:26 AM Anson Huang <Anson.Huang@nxp.com> wrote:
> > i.MX7 reset driver now supports module build and it is no longer
> > built in by default, so i.MX PCI driver needs to select it explicitly
> > due to it is NOT supporting loadable module currently.
> > 
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > No change.
> > ---
> >  drivers/pci/controller/dwc/Kconfig | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > index 044a376..bcf63ce 100644
> > --- a/drivers/pci/controller/dwc/Kconfig
> > +++ b/drivers/pci/controller/dwc/Kconfig
> > @@ -90,6 +90,7 @@ config PCI_EXYNOS
> > 
> >  config PCI_IMX6
> >         bool "Freescale i.MX6/7/8 PCIe controller"
> > +       select RESET_IMX7
> 
> This will break as select will not cause all of RESET_IMX7's
> dependencies to be met. It also doesn't scale. Are you going to do the
> same thing for clocks, pinctrl, gpio, etc.?
> 
> You should make the PCI driver work as a module.

Oh, also PCI_IMX6 is used on (surprise) i.MX6, which doesn't need
RESET_IMX7 at all.

How about hiding the RESET_IMX7 option and setting it default y if
PCI_IMX6 is enabled, as an interim solution?

regards
Philipp
Anson Huang July 30, 2020, 2:11 a.m. UTC | #4
Hi, Philipp/Rob


> Subject: Re: [PATCH V3 3/3] pci: imx: Select RESET_IMX7 by default
> 
> On Wed, 2020-07-29 at 09:26 -0600, Rob Herring wrote:
> > On Mon, Jul 20, 2020 at 8:26 AM Anson Huang <Anson.Huang@nxp.com>
> wrote:
> > > i.MX7 reset driver now supports module build and it is no longer
> > > built in by default, so i.MX PCI driver needs to select it
> > > explicitly due to it is NOT supporting loadable module currently.
> > >
> > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > ---
> > > No change.
> > > ---
> > >  drivers/pci/controller/dwc/Kconfig | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/Kconfig
> > > b/drivers/pci/controller/dwc/Kconfig
> > > index 044a376..bcf63ce 100644
> > > --- a/drivers/pci/controller/dwc/Kconfig
> > > +++ b/drivers/pci/controller/dwc/Kconfig
> > > @@ -90,6 +90,7 @@ config PCI_EXYNOS
> > >
> > >  config PCI_IMX6
> > >         bool "Freescale i.MX6/7/8 PCIe controller"
> > > +       select RESET_IMX7
> >
> > This will break as select will not cause all of RESET_IMX7's
> > dependencies to be met. It also doesn't scale. Are you going to do the
> > same thing for clocks, pinctrl, gpio, etc.?
> >
> > You should make the PCI driver work as a module.
> 
> Oh, also PCI_IMX6 is used on (surprise) i.MX6, which doesn't need
> RESET_IMX7 at all.
> 
> How about hiding the RESET_IMX7 option and setting it default y if
> PCI_IMX6 is enabled, as an interim solution?

Like below, RESET_IMX7 is already default y when SOC_IMX7D, now added PCI_IMX6,
let me know if it is OK for you, then I will send new patch for review.

+++ b/drivers/reset/Kconfig
@@ -68,7 +68,7 @@ config RESET_IMX7
        tristate "i.MX7/8 Reset Driver"
        depends on HAS_IOMEM
        depends on SOC_IMX7D || (ARM64 && ARCH_MXC) || COMPILE_TEST
-       default y if SOC_IMX7D
+       default y if (SOC_IMX7D || PCI_IMX6)

Thanks,
Anson
Philipp Zabel July 30, 2020, 7:19 a.m. UTC | #5
Hi Anson,

On Thu, 2020-07-30 at 02:11 +0000, Anson Huang wrote:
> Hi, Philipp/Rob
> 
> > Subject: Re: [PATCH V3 3/3] pci: imx: Select RESET_IMX7 by default
> > 
> > On Wed, 2020-07-29 at 09:26 -0600, Rob Herring wrote:
> > > On Mon, Jul 20, 2020 at 8:26 AM Anson Huang <Anson.Huang@nxp.com>
> > wrote:
> > > > i.MX7 reset driver now supports module build and it is no longer
> > > > built in by default, so i.MX PCI driver needs to select it
> > > > explicitly due to it is NOT supporting loadable module currently.
> > > > 
> > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > > ---
> > > > No change.
> > > > ---
> > > >  drivers/pci/controller/dwc/Kconfig | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/drivers/pci/controller/dwc/Kconfig
> > > > b/drivers/pci/controller/dwc/Kconfig
> > > > index 044a376..bcf63ce 100644
> > > > --- a/drivers/pci/controller/dwc/Kconfig
> > > > +++ b/drivers/pci/controller/dwc/Kconfig
> > > > @@ -90,6 +90,7 @@ config PCI_EXYNOS
> > > > 
> > > >  config PCI_IMX6
> > > >         bool "Freescale i.MX6/7/8 PCIe controller"
> > > > +       select RESET_IMX7
> > > 
> > > This will break as select will not cause all of RESET_IMX7's
> > > dependencies to be met. It also doesn't scale. Are you going to do the
> > > same thing for clocks, pinctrl, gpio, etc.?
> > > 
> > > You should make the PCI driver work as a module.
> > 
> > Oh, also PCI_IMX6 is used on (surprise) i.MX6, which doesn't need
> > RESET_IMX7 at all.
> > 
> > How about hiding the RESET_IMX7 option and setting it default y if
> > PCI_IMX6 is enabled, as an interim solution?
> 
> Like below, RESET_IMX7 is already default y when SOC_IMX7D, now added PCI_IMX6,
> let me know if it is OK for you, then I will send new patch for review.
> 
> +++ b/drivers/reset/Kconfig
> @@ -68,7 +68,7 @@ config RESET_IMX7
>         tristate "i.MX7/8 Reset Driver"

I was thinking something like

	tristate "i.MX7/8 Reset Driver" if COMPILE_TEST || !PCI_IMX6

>         depends on HAS_IOMEM
>         depends on SOC_IMX7D || (ARM64 && ARCH_MXC) || COMPILE_TEST
> -       default y if SOC_IMX7D
> +       default y if (SOC_IMX7D || PCI_IMX6)

Yes, although without the above I think it could still be disabled
manually or via oldconfig.

regards
Philipp
Richard Zhu July 30, 2020, 8:08 a.m. UTC | #6
> -----Original Message-----
> From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Sent: 2020年7月28日 18:51
> To: Anson Huang <anson.huang@nxp.com>; Richard Zhu
> <hongxing.zhu@nxp.com>; Lucas Stach <l.stach@pengutronix.de>
> Cc: catalin.marinas@arm.com; will@kernel.org; robh@kernel.org;
> bhelgaas@google.com; p.zabel@pengutronix.de; shawnguo@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> bjorn.andersson@linaro.org; Leo Li <leoyang.li@nxp.com>; vkoul@kernel.org;
> geert+renesas@glider.be; olof@lixom.net; amurray@thegoodpenguin.co.uk;
> treding@nvidia.com; vidyas@nvidia.com; hayashi.kunihiko@socionext.com;
> jonnyc@amazon.com; eswara.kota@linux.intel.com; krzk@kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> linux-pci@vger.kernel.org
> Subject: [EXT] Re: [PATCH V3 3/3] pci: imx: Select RESET_IMX7 by default
> 
> [CCing IMX6 maintainers]
> 
> On Mon, Jul 20, 2020 at 10:22:01PM +0800, Anson Huang wrote:
> > i.MX7 reset driver now supports module build and it is no longer built
> > in by default, so i.MX PCI driver needs to select it explicitly due to
> > it is NOT supporting loadable module currently.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

I'm okay with this change.  Acked-by: Richard Zhu <hongxing.zhu@nxp.com>
Thanks.
Best Regards
Richard Zhu
> > ---
> > No change.
> > ---
> >  drivers/pci/controller/dwc/Kconfig | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/pci/controller/dwc/Kconfig
> > b/drivers/pci/controller/dwc/Kconfig
> > index 044a376..bcf63ce 100644
> > --- a/drivers/pci/controller/dwc/Kconfig
> > +++ b/drivers/pci/controller/dwc/Kconfig
> > @@ -90,6 +90,7 @@ config PCI_EXYNOS
> >
> >  config PCI_IMX6
> >       bool "Freescale i.MX6/7/8 PCIe controller"
> > +     select RESET_IMX7
> >       depends on ARCH_MXC || COMPILE_TEST
> >       depends on PCI_MSI_IRQ_DOMAIN
> >       select PCIE_DW_HOST
> > --
> > 2.7.4
> >
Anson Huang July 30, 2020, 1:11 p.m. UTC | #7
Hi, Philipp


> Subject: Re: [PATCH V3 3/3] pci: imx: Select RESET_IMX7 by default
> 
> Hi Anson,
> 
> On Thu, 2020-07-30 at 02:11 +0000, Anson Huang wrote:
> > Hi, Philipp/Rob
> >
> > > Subject: Re: [PATCH V3 3/3] pci: imx: Select RESET_IMX7 by default
> > >
> > > On Wed, 2020-07-29 at 09:26 -0600, Rob Herring wrote:
> > > > On Mon, Jul 20, 2020 at 8:26 AM Anson Huang
> <Anson.Huang@nxp.com>
> > > wrote:
> > > > > i.MX7 reset driver now supports module build and it is no longer
> > > > > built in by default, so i.MX PCI driver needs to select it
> > > > > explicitly due to it is NOT supporting loadable module currently.
> > > > >
> > > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > > > ---
> > > > > No change.
> > > > > ---
> > > > >  drivers/pci/controller/dwc/Kconfig | 1 +
> > > > >  1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git a/drivers/pci/controller/dwc/Kconfig
> > > > > b/drivers/pci/controller/dwc/Kconfig
> > > > > index 044a376..bcf63ce 100644
> > > > > --- a/drivers/pci/controller/dwc/Kconfig
> > > > > +++ b/drivers/pci/controller/dwc/Kconfig
> > > > > @@ -90,6 +90,7 @@ config PCI_EXYNOS
> > > > >
> > > > >  config PCI_IMX6
> > > > >         bool "Freescale i.MX6/7/8 PCIe controller"
> > > > > +       select RESET_IMX7
> > > >
> > > > This will break as select will not cause all of RESET_IMX7's
> > > > dependencies to be met. It also doesn't scale. Are you going to do
> > > > the same thing for clocks, pinctrl, gpio, etc.?
> > > >
> > > > You should make the PCI driver work as a module.
> > >
> > > Oh, also PCI_IMX6 is used on (surprise) i.MX6, which doesn't need
> > > RESET_IMX7 at all.
> > >
> > > How about hiding the RESET_IMX7 option and setting it default y if
> > > PCI_IMX6 is enabled, as an interim solution?
> >
> > Like below, RESET_IMX7 is already default y when SOC_IMX7D, now added
> > PCI_IMX6, let me know if it is OK for you, then I will send new patch for
> review.
> >
> > +++ b/drivers/reset/Kconfig
> > @@ -68,7 +68,7 @@ config RESET_IMX7
> >         tristate "i.MX7/8 Reset Driver"
> 
> I was thinking something like
> 
> 	tristate "i.MX7/8 Reset Driver" if COMPILE_TEST || !PCI_IMX6
> 
> >         depends on HAS_IOMEM
> >         depends on SOC_IMX7D || (ARM64 && ARCH_MXC) ||
> COMPILE_TEST
> > -       default y if SOC_IMX7D
> > +       default y if (SOC_IMX7D || PCI_IMX6)
> 
> Yes, although without the above I think it could still be disabled manually or
> via oldconfig.

Then should I send a new version with below? As we already did it this way for i.MX7D,
adding it for i.MX6 makes more sense?

+       default y if (SOC_IMX7D || PCI_IMX6)

Thanks,
Anson
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 044a376..bcf63ce 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -90,6 +90,7 @@  config PCI_EXYNOS
 
 config PCI_IMX6
 	bool "Freescale i.MX6/7/8 PCIe controller"
+	select RESET_IMX7
 	depends on ARCH_MXC || COMPILE_TEST
 	depends on PCI_MSI_IRQ_DOMAIN
 	select PCIE_DW_HOST