2020-07-20 14:26:43

by Anson Huang

[permalink] [raw]
Subject: [PATCH V3 1/3] reset: imx7: Support module build

Use module_platform_driver(), add module device table, author,
description and license to support module build, and
CONFIG_RESET_IMX7 is changed to default 'y' ONLY for i.MX7D,
other platforms need to select it in defconfig.

Signed-off-by: Anson Huang <[email protected]>
---
Changes since V2:
- use module_platform_driver() instead of builtin_platform_driver().
---
drivers/reset/Kconfig | 5 +++--
drivers/reset/reset-imx7.c | 9 +++++++--
2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index d9efbfd..19f9773 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -65,9 +65,10 @@ config RESET_HSDK
This enables the reset controller driver for HSDK board.

config RESET_IMX7
- bool "i.MX7/8 Reset Driver" if COMPILE_TEST
+ tristate "i.MX7/8 Reset Driver"
depends on HAS_IOMEM
- default SOC_IMX7D || (ARM64 && ARCH_MXC)
+ depends on SOC_IMX7D || (ARM64 && ARCH_MXC) || COMPILE_TEST
+ default y if SOC_IMX7D
select MFD_SYSCON
help
This enables the reset controller driver for i.MX7 SoCs.
diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
index d170fe6..9832033 100644
--- a/drivers/reset/reset-imx7.c
+++ b/drivers/reset/reset-imx7.c
@@ -8,7 +8,7 @@
*/

#include <linux/mfd/syscon.h>
-#include <linux/mod_devicetable.h>
+#include <linux/module.h>
#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/reset-controller.h>
@@ -386,6 +386,7 @@ static const struct of_device_id imx7_reset_dt_ids[] = {
{ .compatible = "fsl,imx8mp-src", .data = &variant_imx8mp },
{ /* sentinel */ },
};
+MODULE_DEVICE_TABLE(of, imx7_reset_dt_ids);

static struct platform_driver imx7_reset_driver = {
.probe = imx7_reset_probe,
@@ -394,4 +395,8 @@ static struct platform_driver imx7_reset_driver = {
.of_match_table = imx7_reset_dt_ids,
},
};
-builtin_platform_driver(imx7_reset_driver);
+module_platform_driver(imx7_reset_driver);
+
+MODULE_AUTHOR("Andrey Smirnov <[email protected]>");
+MODULE_DESCRIPTION("NXP i.MX7 reset driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4


2020-07-20 14:28:48

by Anson Huang

[permalink] [raw]
Subject: [PATCH V3 2/3] arm64: defconfig: Select CONFIG_RESET_IMX7 as module by default

i.MX7 reset driver now supports module build, it is no longer
enabled by default, need to select it explicitly.

Signed-off-by: Anson Huang <[email protected]>
---
No change.
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 39b3ac7..6c764bd 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -921,6 +921,7 @@ CONFIG_PWM_SAMSUNG=y
CONFIG_PWM_SUN4I=m
CONFIG_PWM_TEGRA=m
CONFIG_QCOM_PDC=y
+CONFIG_RESET_IMX7=m
CONFIG_RESET_QCOM_AOSS=y
CONFIG_RESET_QCOM_PDC=m
CONFIG_RESET_TI_SCI=y
--
2.7.4

2020-07-20 14:28:53

by Anson Huang

[permalink] [raw]
Subject: [PATCH V3 3/3] pci: imx: Select RESET_IMX7 by default

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 <[email protected]>
---
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

2020-07-24 08:06:07

by Philipp Zabel

[permalink] [raw]
Subject: Re: [PATCH V3 1/3] reset: imx7: Support module build

On Mon, 2020-07-20 at 22:21 +0800, Anson Huang wrote:
> Use module_platform_driver(), add module device table, author,
> description and license to support module build, and
> CONFIG_RESET_IMX7 is changed to default 'y' ONLY for i.MX7D,
> other platforms need to select it in defconfig.
>
> Signed-off-by: Anson Huang <[email protected]>
> ---
> Changes since V2:
> - use module_platform_driver() instead of builtin_platform_driver().

Thank you, applied to reset/next.

regards
Philipp

2020-07-28 10:52:58

by Lorenzo Pieralisi

[permalink] [raw]
Subject: 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 <[email protected]>
> ---
> 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
>

2020-07-28 10:54:36

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH V3 1/3] reset: imx7: Support module build

On Fri, Jul 24, 2020 at 10:03:11AM +0200, Philipp Zabel wrote:
> On Mon, 2020-07-20 at 22:21 +0800, Anson Huang wrote:
> > Use module_platform_driver(), add module device table, author,
> > description and license to support module build, and
> > CONFIG_RESET_IMX7 is changed to default 'y' ONLY for i.MX7D,
> > other platforms need to select it in defconfig.
> >
> > Signed-off-by: Anson Huang <[email protected]>
> > ---
> > Changes since V2:
> > - use module_platform_driver() instead of builtin_platform_driver().
>
> Thank you, applied to reset/next.

I think you should pick up patch (3) as well please if PCI_IMX6
maintainers ACK it - merging just patch(1) will trigger regressions
AFAICS.

Lorenzo

2020-07-29 15:28:59

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH V3 3/3] pci: imx: Select RESET_IMX7 by default

On Mon, Jul 20, 2020 at 8:26 AM Anson Huang <[email protected]> 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 <[email protected]>
> ---
> 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
>

2020-07-29 15:50:51

by Philipp Zabel

[permalink] [raw]
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 <[email protected]> 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 <[email protected]>
> > ---
> > 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

2020-07-29 15:51:30

by Philipp Zabel

[permalink] [raw]
Subject: Re: [PATCH V3 1/3] reset: imx7: Support module build

On Tue, 2020-07-28 at 11:53 +0100, Lorenzo Pieralisi wrote:
> On Fri, Jul 24, 2020 at 10:03:11AM +0200, Philipp Zabel wrote:
> > On Mon, 2020-07-20 at 22:21 +0800, Anson Huang wrote:
> > > Use module_platform_driver(), add module device table, author,
> > > description and license to support module build, and
> > > CONFIG_RESET_IMX7 is changed to default 'y' ONLY for i.MX7D,
> > > other platforms need to select it in defconfig.
> > >
> > > Signed-off-by: Anson Huang <[email protected]>
> > > ---
> > > Changes since V2:
> > > - use module_platform_driver() instead of builtin_platform_driver().
> >
> > Thank you, applied to reset/next.
>
> I think you should pick up patch (3) as well please if PCI_IMX6
> maintainers ACK it - merging just patch(1) will trigger regressions
> AFAICS.

Thank you for raising this, I'll put these patches on hold until the
PCI_IMX6 issue is resolved.

regards
Philipp

2020-07-30 02:12:02

by Anson Huang

[permalink] [raw]
Subject: RE: [PATCH V3 3/3] pci: imx: Select RESET_IMX7 by default

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 <[email protected]>
> 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 <[email protected]>
> > > ---
> > > 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

2020-07-30 07:23:03

by Philipp Zabel

[permalink] [raw]
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 <[email protected]>
> > 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 <[email protected]>
> > > > ---
> > > > 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

2020-07-30 08:21:18

by Richard Zhu

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH V3 3/3] pci: imx: Select RESET_IMX7 by default

> -----Original Message-----
> From: Lorenzo Pieralisi <[email protected]>
> Sent: 2020??7??28?? 18:51
> To: Anson Huang <[email protected]>; Richard Zhu
> <[email protected]>; Lucas Stach <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; Leo Li <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]
> 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 <[email protected]>

I'm okay with this change. Acked-by: Richard Zhu <[email protected]>
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
> >

2020-07-30 13:12:35

by Anson Huang

[permalink] [raw]
Subject: RE: [PATCH V3 3/3] pci: imx: Select RESET_IMX7 by default

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
> <[email protected]>
> > > 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 <[email protected]>
> > > > > ---
> > > > > 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