Re: [PATCH 1/1] reset: imx7: Add support for i.MX8MQ

2018-11-26 Thread Andrey Smirnov
On Mon, Nov 19, 2018 at 6:32 AM Philipp Zabel  wrote:
>
> Hi Andrey,
>
> thank you for the patch.
>
> In general, sharing the lookup table with i.MX7 is fine iff it is a
> strict superset. But I don't think that is the case (see below).
> Even so, this will change if there ever is another i.MX7 or i.MX8M
> variant that is also a superset of i.MX7, but not a superset of
> i.MX8M. Also, just listing all resets in order of appearance would make
> review a bit easier.
>
> I don't like increasing IMX7_RESET_NUM though, i.MX8M should have its
> own nr_resets.
>
> On Sat, 2018-11-17 at 10:11 -0800, Andrey Smirnov wrote:
> > SRC IP block used in i.MX8MQ is a superset of what is found in i.MX7D,
>
> Is this true, though?
>
> i.MX7 has SRC_ERCR at offset 0x14 and HSICPHY_RCR at offset 0x1c.
> According to documentation, i.MX8M is missing those at least.
>

OK, I'll convert the patch to use separate LUTs in v2.

> > so add all of the definitions necessary to support both.
> >
> > Cc: p.za...@pengutronix.de
> > Cc: Fabio Estevam 
> > Cc: cphe...@gmail.com
> > Cc: l.st...@pengutronix.de
> > Cc: Leonard Crestez 
> > Cc: "A.s. Dong" 
> > Cc: Richard Zhu 
> > Cc: linux-...@nxp.com
> > Cc: linux-arm-ker...@lists.infradead.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Andrey Smirnov 
> > ---
> >  drivers/reset/Kconfig   |  2 +-
> >  drivers/reset/reset-imx7.c  | 66 -
> >  include/dt-bindings/reset/imx7-reset.h  | 15 +-
> >  include/dt-bindings/reset/imx8m-reset.h | 47 ++
> >  4 files changed, 127 insertions(+), 3 deletions(-)
> >  create mode 100644 include/dt-bindings/reset/imx8m-reset.h
> >
> > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> > index c21da9fe51ec..4909aab7401b 100644
> > --- a/drivers/reset/Kconfig
> > +++ b/drivers/reset/Kconfig
> > @@ -50,7 +50,7 @@ config RESET_HSDK
> >  config RESET_IMX7
> >   bool "i.MX7 Reset Driver" if COMPILE_TEST
> >   depends on HAS_IOMEM
> > - default SOC_IMX7D
> > + default SOC_IMX7D || SOC_IMX8MQ
> >   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 77911fa8f31d..dffad618f805 100644
> > --- a/drivers/reset/reset-imx7.c
> > +++ b/drivers/reset/reset-imx7.c
> > @@ -21,14 +21,16 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  struct imx7_src {
> >   struct reset_controller_dev rcdev;
> >   struct regmap *regmap;
> >  };
> >
> > -enum imx7_src_registers {
> > +enum imx_src_registers {
> >   SRC_A7RCR0  = 0x0004,
> > + SRC_A53RCR0 = 0x0004,
> >   SRC_M4RCR   = 0x000c,
> >   SRC_ERCR= 0x0014,
> >   SRC_HSICPHY_RCR = 0x001c,
> > @@ -36,7 +38,9 @@ enum imx7_src_registers {
> >   SRC_USBOPHY2_RCR= 0x0024,
> >   SRC_MIPIPHY_RCR = 0x0028,
> >   SRC_PCIEPHY_RCR = 0x002c,
>
> These aren't complete. I see at least SRC_HDMI_RCR, SRC_DISP_RCR,
> SRC_GPU_RCR, and SRC_VPU_RCR missing.
>
> > + SRC_PCIE2PHY_RCR= 0x0048,
>
> And SRC_MIPIPHY1_RCR, SRC_MIPIPHY2_RCR. Please check the reference
> manual and complete the list of registers that contain resets.
>
> >   SRC_DDRC_RCR= 0x1000,
> > +
>
> Whitespace.
>
> >  };
> >
> >  struct imx7_src_signal {
> > @@ -67,11 +71,67 @@ static const struct imx7_src_signal 
> > imx7_src_signals[IMX7_RESET_NUM] = {
> >   [IMX7_RESET_PCIEPHY]= { SRC_PCIEPHY_RCR, BIT(2) | BIT(1) 
> > },
> >   [IMX7_RESET_PCIEPHY_PERST]  = { SRC_PCIEPHY_RCR, BIT(3) },
> >   [IMX7_RESET_PCIE_CTRL_APPS_EN]  = { SRC_PCIEPHY_RCR, BIT(6) },
> > + [IMX7_RESET_PCIE_CTRL_APPS_CLK_REQ] = { SRC_PCIEPHY_RCR, BIT(4) },
>
> Now I feel like CTRL_APPS_EN shouldn't have been added here either. But
> while there I wasn't really sure about whether that is actually a valid
> reset, I'm pretty sure that this one isn't.
> I think that i.MX8M either needs a clock driver to control this bit and
> the corresponding IOMUXC GPR bit to enable the PCIe refclk, or the PCIe
> driver has to access this register via syscon. I really don't want to
> see a clock enable signal described in the device tree as a reset.
>

OK, will drop in v2.

> >   [IMX7_RESET_PCIE_CTRL_APPS_TURNOFF] = { SRC_PCIEPHY_RCR, BIT(11) },
> >   [IMX7_RESET_DDRC_PRST]  = { SRC_DDRC_RCR, BIT(0) },
> >   [IMX7_RESET_DDRC_CORE_RST]  = { SRC_DDRC_RCR, BIT(1) },
> > +
> > + [IMX8M_RESET_A53_CORE_POR_RESET2] = { SRC_A53RCR0, BIT(2) },
> > + [IMX8M_RESET_A53_CORE_POR_RESET3] = { SRC_A53RCR0, BIT(3) },
> > + [IMX8M_RESET_A53_CORE_RESET2] = { SRC_A53RCR0, BIT(6) },
> > + [IMX8M_RESET_A53_CORE_RESET3] = { SRC_A53RCR0, BIT(7) },
>
> Missing IMX8M_RESET_A53_DBG_RESET2,3.
>
> > + [IMX8M_RESET_A53_ETM_RESET2]  = { SRC_A53RCR0, BIT(14) },
> > +

Re: [PATCH 1/1] reset: imx7: Add support for i.MX8MQ

2018-11-26 Thread Andrey Smirnov
On Mon, Nov 19, 2018 at 6:32 AM Philipp Zabel  wrote:
>
> Hi Andrey,
>
> thank you for the patch.
>
> In general, sharing the lookup table with i.MX7 is fine iff it is a
> strict superset. But I don't think that is the case (see below).
> Even so, this will change if there ever is another i.MX7 or i.MX8M
> variant that is also a superset of i.MX7, but not a superset of
> i.MX8M. Also, just listing all resets in order of appearance would make
> review a bit easier.
>
> I don't like increasing IMX7_RESET_NUM though, i.MX8M should have its
> own nr_resets.
>
> On Sat, 2018-11-17 at 10:11 -0800, Andrey Smirnov wrote:
> > SRC IP block used in i.MX8MQ is a superset of what is found in i.MX7D,
>
> Is this true, though?
>
> i.MX7 has SRC_ERCR at offset 0x14 and HSICPHY_RCR at offset 0x1c.
> According to documentation, i.MX8M is missing those at least.
>

OK, I'll convert the patch to use separate LUTs in v2.

> > so add all of the definitions necessary to support both.
> >
> > Cc: p.za...@pengutronix.de
> > Cc: Fabio Estevam 
> > Cc: cphe...@gmail.com
> > Cc: l.st...@pengutronix.de
> > Cc: Leonard Crestez 
> > Cc: "A.s. Dong" 
> > Cc: Richard Zhu 
> > Cc: linux-...@nxp.com
> > Cc: linux-arm-ker...@lists.infradead.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Andrey Smirnov 
> > ---
> >  drivers/reset/Kconfig   |  2 +-
> >  drivers/reset/reset-imx7.c  | 66 -
> >  include/dt-bindings/reset/imx7-reset.h  | 15 +-
> >  include/dt-bindings/reset/imx8m-reset.h | 47 ++
> >  4 files changed, 127 insertions(+), 3 deletions(-)
> >  create mode 100644 include/dt-bindings/reset/imx8m-reset.h
> >
> > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> > index c21da9fe51ec..4909aab7401b 100644
> > --- a/drivers/reset/Kconfig
> > +++ b/drivers/reset/Kconfig
> > @@ -50,7 +50,7 @@ config RESET_HSDK
> >  config RESET_IMX7
> >   bool "i.MX7 Reset Driver" if COMPILE_TEST
> >   depends on HAS_IOMEM
> > - default SOC_IMX7D
> > + default SOC_IMX7D || SOC_IMX8MQ
> >   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 77911fa8f31d..dffad618f805 100644
> > --- a/drivers/reset/reset-imx7.c
> > +++ b/drivers/reset/reset-imx7.c
> > @@ -21,14 +21,16 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  struct imx7_src {
> >   struct reset_controller_dev rcdev;
> >   struct regmap *regmap;
> >  };
> >
> > -enum imx7_src_registers {
> > +enum imx_src_registers {
> >   SRC_A7RCR0  = 0x0004,
> > + SRC_A53RCR0 = 0x0004,
> >   SRC_M4RCR   = 0x000c,
> >   SRC_ERCR= 0x0014,
> >   SRC_HSICPHY_RCR = 0x001c,
> > @@ -36,7 +38,9 @@ enum imx7_src_registers {
> >   SRC_USBOPHY2_RCR= 0x0024,
> >   SRC_MIPIPHY_RCR = 0x0028,
> >   SRC_PCIEPHY_RCR = 0x002c,
>
> These aren't complete. I see at least SRC_HDMI_RCR, SRC_DISP_RCR,
> SRC_GPU_RCR, and SRC_VPU_RCR missing.
>
> > + SRC_PCIE2PHY_RCR= 0x0048,
>
> And SRC_MIPIPHY1_RCR, SRC_MIPIPHY2_RCR. Please check the reference
> manual and complete the list of registers that contain resets.
>
> >   SRC_DDRC_RCR= 0x1000,
> > +
>
> Whitespace.
>
> >  };
> >
> >  struct imx7_src_signal {
> > @@ -67,11 +71,67 @@ static const struct imx7_src_signal 
> > imx7_src_signals[IMX7_RESET_NUM] = {
> >   [IMX7_RESET_PCIEPHY]= { SRC_PCIEPHY_RCR, BIT(2) | BIT(1) 
> > },
> >   [IMX7_RESET_PCIEPHY_PERST]  = { SRC_PCIEPHY_RCR, BIT(3) },
> >   [IMX7_RESET_PCIE_CTRL_APPS_EN]  = { SRC_PCIEPHY_RCR, BIT(6) },
> > + [IMX7_RESET_PCIE_CTRL_APPS_CLK_REQ] = { SRC_PCIEPHY_RCR, BIT(4) },
>
> Now I feel like CTRL_APPS_EN shouldn't have been added here either. But
> while there I wasn't really sure about whether that is actually a valid
> reset, I'm pretty sure that this one isn't.
> I think that i.MX8M either needs a clock driver to control this bit and
> the corresponding IOMUXC GPR bit to enable the PCIe refclk, or the PCIe
> driver has to access this register via syscon. I really don't want to
> see a clock enable signal described in the device tree as a reset.
>

OK, will drop in v2.

> >   [IMX7_RESET_PCIE_CTRL_APPS_TURNOFF] = { SRC_PCIEPHY_RCR, BIT(11) },
> >   [IMX7_RESET_DDRC_PRST]  = { SRC_DDRC_RCR, BIT(0) },
> >   [IMX7_RESET_DDRC_CORE_RST]  = { SRC_DDRC_RCR, BIT(1) },
> > +
> > + [IMX8M_RESET_A53_CORE_POR_RESET2] = { SRC_A53RCR0, BIT(2) },
> > + [IMX8M_RESET_A53_CORE_POR_RESET3] = { SRC_A53RCR0, BIT(3) },
> > + [IMX8M_RESET_A53_CORE_RESET2] = { SRC_A53RCR0, BIT(6) },
> > + [IMX8M_RESET_A53_CORE_RESET3] = { SRC_A53RCR0, BIT(7) },
>
> Missing IMX8M_RESET_A53_DBG_RESET2,3.
>
> > + [IMX8M_RESET_A53_ETM_RESET2]  = { SRC_A53RCR0, BIT(14) },
> > +

Re: [PATCH 1/1] reset: imx7: Add support for i.MX8MQ

2018-11-19 Thread Philipp Zabel
Hi Andrey,

thank you for the patch.

In general, sharing the lookup table with i.MX7 is fine iff it is a
strict superset. But I don't think that is the case (see below).
Even so, this will change if there ever is another i.MX7 or i.MX8M
variant that is also a superset of i.MX7, but not a superset of
i.MX8M. Also, just listing all resets in order of appearance would make
review a bit easier.

I don't like increasing IMX7_RESET_NUM though, i.MX8M should have its
own nr_resets.

On Sat, 2018-11-17 at 10:11 -0800, Andrey Smirnov wrote:
> SRC IP block used in i.MX8MQ is a superset of what is found in i.MX7D,

Is this true, though?

i.MX7 has SRC_ERCR at offset 0x14 and HSICPHY_RCR at offset 0x1c.
According to documentation, i.MX8M is missing those at least.

> so add all of the definitions necessary to support both.
>
> Cc: p.za...@pengutronix.de
> Cc: Fabio Estevam 
> Cc: cphe...@gmail.com
> Cc: l.st...@pengutronix.de
> Cc: Leonard Crestez 
> Cc: "A.s. Dong" 
> Cc: Richard Zhu 
> Cc: linux-...@nxp.com
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Andrey Smirnov 
> ---
>  drivers/reset/Kconfig   |  2 +-
>  drivers/reset/reset-imx7.c  | 66 -
>  include/dt-bindings/reset/imx7-reset.h  | 15 +-
>  include/dt-bindings/reset/imx8m-reset.h | 47 ++
>  4 files changed, 127 insertions(+), 3 deletions(-)
>  create mode 100644 include/dt-bindings/reset/imx8m-reset.h
> 
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index c21da9fe51ec..4909aab7401b 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -50,7 +50,7 @@ config RESET_HSDK
>  config RESET_IMX7
>   bool "i.MX7 Reset Driver" if COMPILE_TEST
>   depends on HAS_IOMEM
> - default SOC_IMX7D
> + default SOC_IMX7D || SOC_IMX8MQ
>   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 77911fa8f31d..dffad618f805 100644
> --- a/drivers/reset/reset-imx7.c
> +++ b/drivers/reset/reset-imx7.c
> @@ -21,14 +21,16 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  struct imx7_src {
>   struct reset_controller_dev rcdev;
>   struct regmap *regmap;
>  };
>  
> -enum imx7_src_registers {
> +enum imx_src_registers {
>   SRC_A7RCR0  = 0x0004,
> + SRC_A53RCR0 = 0x0004,
>   SRC_M4RCR   = 0x000c,
>   SRC_ERCR= 0x0014,
>   SRC_HSICPHY_RCR = 0x001c,
> @@ -36,7 +38,9 @@ enum imx7_src_registers {
>   SRC_USBOPHY2_RCR= 0x0024,
>   SRC_MIPIPHY_RCR = 0x0028,
>   SRC_PCIEPHY_RCR = 0x002c,

These aren't complete. I see at least SRC_HDMI_RCR, SRC_DISP_RCR,
SRC_GPU_RCR, and SRC_VPU_RCR missing.

> + SRC_PCIE2PHY_RCR= 0x0048,

And SRC_MIPIPHY1_RCR, SRC_MIPIPHY2_RCR. Please check the reference
manual and complete the list of registers that contain resets.

>   SRC_DDRC_RCR= 0x1000,
> +

Whitespace.

>  };
>  
>  struct imx7_src_signal {
> @@ -67,11 +71,67 @@ static const struct imx7_src_signal 
> imx7_src_signals[IMX7_RESET_NUM] = {
>   [IMX7_RESET_PCIEPHY]= { SRC_PCIEPHY_RCR, BIT(2) | BIT(1) },
>   [IMX7_RESET_PCIEPHY_PERST]  = { SRC_PCIEPHY_RCR, BIT(3) },
>   [IMX7_RESET_PCIE_CTRL_APPS_EN]  = { SRC_PCIEPHY_RCR, BIT(6) },
> + [IMX7_RESET_PCIE_CTRL_APPS_CLK_REQ] = { SRC_PCIEPHY_RCR, BIT(4) },

Now I feel like CTRL_APPS_EN shouldn't have been added here either. But
while there I wasn't really sure about whether that is actually a valid
reset, I'm pretty sure that this one isn't.
I think that i.MX8M either needs a clock driver to control this bit and
the corresponding IOMUXC GPR bit to enable the PCIe refclk, or the PCIe
driver has to access this register via syscon. I really don't want to
see a clock enable signal described in the device tree as a reset.

>   [IMX7_RESET_PCIE_CTRL_APPS_TURNOFF] = { SRC_PCIEPHY_RCR, BIT(11) },
>   [IMX7_RESET_DDRC_PRST]  = { SRC_DDRC_RCR, BIT(0) },
>   [IMX7_RESET_DDRC_CORE_RST]  = { SRC_DDRC_RCR, BIT(1) },
> +
> + [IMX8M_RESET_A53_CORE_POR_RESET2] = { SRC_A53RCR0, BIT(2) },
> + [IMX8M_RESET_A53_CORE_POR_RESET3] = { SRC_A53RCR0, BIT(3) },
> + [IMX8M_RESET_A53_CORE_RESET2] = { SRC_A53RCR0, BIT(6) },
> + [IMX8M_RESET_A53_CORE_RESET3] = { SRC_A53RCR0, BIT(7) },

Missing IMX8M_RESET_A53_DBG_RESET2,3.

> + [IMX8M_RESET_A53_ETM_RESET2]  = { SRC_A53RCR0, BIT(14) },
> + [IMX8M_RESET_A53_ETM_RESET3]  = { SRC_A53RCR0, BIT(15) },

Missing some reset registers here.

> + [IMX8M_RESET_PCIE2PHY]  = { SRC_PCIEPHY_RCR, BIT(2) | BIT(1) },
> + [IMX8M_RESET_PCIE2PHY_PERST]  = { SRC_PCIE2PHY_RCR, BIT(3) },
> + [IMX8M_RESET_PCIE2_CTRL_APPS_EN]  = { SRC_PCIE2PHY_RCR, BIT(6) },
> + 

Re: [PATCH 1/1] reset: imx7: Add support for i.MX8MQ

2018-11-19 Thread Philipp Zabel
Hi Andrey,

thank you for the patch.

In general, sharing the lookup table with i.MX7 is fine iff it is a
strict superset. But I don't think that is the case (see below).
Even so, this will change if there ever is another i.MX7 or i.MX8M
variant that is also a superset of i.MX7, but not a superset of
i.MX8M. Also, just listing all resets in order of appearance would make
review a bit easier.

I don't like increasing IMX7_RESET_NUM though, i.MX8M should have its
own nr_resets.

On Sat, 2018-11-17 at 10:11 -0800, Andrey Smirnov wrote:
> SRC IP block used in i.MX8MQ is a superset of what is found in i.MX7D,

Is this true, though?

i.MX7 has SRC_ERCR at offset 0x14 and HSICPHY_RCR at offset 0x1c.
According to documentation, i.MX8M is missing those at least.

> so add all of the definitions necessary to support both.
>
> Cc: p.za...@pengutronix.de
> Cc: Fabio Estevam 
> Cc: cphe...@gmail.com
> Cc: l.st...@pengutronix.de
> Cc: Leonard Crestez 
> Cc: "A.s. Dong" 
> Cc: Richard Zhu 
> Cc: linux-...@nxp.com
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Andrey Smirnov 
> ---
>  drivers/reset/Kconfig   |  2 +-
>  drivers/reset/reset-imx7.c  | 66 -
>  include/dt-bindings/reset/imx7-reset.h  | 15 +-
>  include/dt-bindings/reset/imx8m-reset.h | 47 ++
>  4 files changed, 127 insertions(+), 3 deletions(-)
>  create mode 100644 include/dt-bindings/reset/imx8m-reset.h
> 
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index c21da9fe51ec..4909aab7401b 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -50,7 +50,7 @@ config RESET_HSDK
>  config RESET_IMX7
>   bool "i.MX7 Reset Driver" if COMPILE_TEST
>   depends on HAS_IOMEM
> - default SOC_IMX7D
> + default SOC_IMX7D || SOC_IMX8MQ
>   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 77911fa8f31d..dffad618f805 100644
> --- a/drivers/reset/reset-imx7.c
> +++ b/drivers/reset/reset-imx7.c
> @@ -21,14 +21,16 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  struct imx7_src {
>   struct reset_controller_dev rcdev;
>   struct regmap *regmap;
>  };
>  
> -enum imx7_src_registers {
> +enum imx_src_registers {
>   SRC_A7RCR0  = 0x0004,
> + SRC_A53RCR0 = 0x0004,
>   SRC_M4RCR   = 0x000c,
>   SRC_ERCR= 0x0014,
>   SRC_HSICPHY_RCR = 0x001c,
> @@ -36,7 +38,9 @@ enum imx7_src_registers {
>   SRC_USBOPHY2_RCR= 0x0024,
>   SRC_MIPIPHY_RCR = 0x0028,
>   SRC_PCIEPHY_RCR = 0x002c,

These aren't complete. I see at least SRC_HDMI_RCR, SRC_DISP_RCR,
SRC_GPU_RCR, and SRC_VPU_RCR missing.

> + SRC_PCIE2PHY_RCR= 0x0048,

And SRC_MIPIPHY1_RCR, SRC_MIPIPHY2_RCR. Please check the reference
manual and complete the list of registers that contain resets.

>   SRC_DDRC_RCR= 0x1000,
> +

Whitespace.

>  };
>  
>  struct imx7_src_signal {
> @@ -67,11 +71,67 @@ static const struct imx7_src_signal 
> imx7_src_signals[IMX7_RESET_NUM] = {
>   [IMX7_RESET_PCIEPHY]= { SRC_PCIEPHY_RCR, BIT(2) | BIT(1) },
>   [IMX7_RESET_PCIEPHY_PERST]  = { SRC_PCIEPHY_RCR, BIT(3) },
>   [IMX7_RESET_PCIE_CTRL_APPS_EN]  = { SRC_PCIEPHY_RCR, BIT(6) },
> + [IMX7_RESET_PCIE_CTRL_APPS_CLK_REQ] = { SRC_PCIEPHY_RCR, BIT(4) },

Now I feel like CTRL_APPS_EN shouldn't have been added here either. But
while there I wasn't really sure about whether that is actually a valid
reset, I'm pretty sure that this one isn't.
I think that i.MX8M either needs a clock driver to control this bit and
the corresponding IOMUXC GPR bit to enable the PCIe refclk, or the PCIe
driver has to access this register via syscon. I really don't want to
see a clock enable signal described in the device tree as a reset.

>   [IMX7_RESET_PCIE_CTRL_APPS_TURNOFF] = { SRC_PCIEPHY_RCR, BIT(11) },
>   [IMX7_RESET_DDRC_PRST]  = { SRC_DDRC_RCR, BIT(0) },
>   [IMX7_RESET_DDRC_CORE_RST]  = { SRC_DDRC_RCR, BIT(1) },
> +
> + [IMX8M_RESET_A53_CORE_POR_RESET2] = { SRC_A53RCR0, BIT(2) },
> + [IMX8M_RESET_A53_CORE_POR_RESET3] = { SRC_A53RCR0, BIT(3) },
> + [IMX8M_RESET_A53_CORE_RESET2] = { SRC_A53RCR0, BIT(6) },
> + [IMX8M_RESET_A53_CORE_RESET3] = { SRC_A53RCR0, BIT(7) },

Missing IMX8M_RESET_A53_DBG_RESET2,3.

> + [IMX8M_RESET_A53_ETM_RESET2]  = { SRC_A53RCR0, BIT(14) },
> + [IMX8M_RESET_A53_ETM_RESET3]  = { SRC_A53RCR0, BIT(15) },

Missing some reset registers here.

> + [IMX8M_RESET_PCIE2PHY]  = { SRC_PCIEPHY_RCR, BIT(2) | BIT(1) },
> + [IMX8M_RESET_PCIE2PHY_PERST]  = { SRC_PCIE2PHY_RCR, BIT(3) },
> + [IMX8M_RESET_PCIE2_CTRL_APPS_EN]  = { SRC_PCIE2PHY_RCR, BIT(6) },
> + 

[PATCH 1/1] reset: imx7: Add support for i.MX8MQ

2018-11-17 Thread Andrey Smirnov
SRC IP block used in i.MX8MQ is a superset of what is found in i.MX7D,
so add all of the definitions necessary to support both.

Cc: p.za...@pengutronix.de
Cc: Fabio Estevam 
Cc: cphe...@gmail.com
Cc: l.st...@pengutronix.de
Cc: Leonard Crestez 
Cc: "A.s. Dong" 
Cc: Richard Zhu 
Cc: linux-...@nxp.com
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Andrey Smirnov 
---
 drivers/reset/Kconfig   |  2 +-
 drivers/reset/reset-imx7.c  | 66 -
 include/dt-bindings/reset/imx7-reset.h  | 15 +-
 include/dt-bindings/reset/imx8m-reset.h | 47 ++
 4 files changed, 127 insertions(+), 3 deletions(-)
 create mode 100644 include/dt-bindings/reset/imx8m-reset.h

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index c21da9fe51ec..4909aab7401b 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -50,7 +50,7 @@ config RESET_HSDK
 config RESET_IMX7
bool "i.MX7 Reset Driver" if COMPILE_TEST
depends on HAS_IOMEM
-   default SOC_IMX7D
+   default SOC_IMX7D || SOC_IMX8MQ
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 77911fa8f31d..dffad618f805 100644
--- a/drivers/reset/reset-imx7.c
+++ b/drivers/reset/reset-imx7.c
@@ -21,14 +21,16 @@
 #include 
 #include 
 #include 
+#include 
 
 struct imx7_src {
struct reset_controller_dev rcdev;
struct regmap *regmap;
 };
 
-enum imx7_src_registers {
+enum imx_src_registers {
SRC_A7RCR0  = 0x0004,
+   SRC_A53RCR0 = 0x0004,
SRC_M4RCR   = 0x000c,
SRC_ERCR= 0x0014,
SRC_HSICPHY_RCR = 0x001c,
@@ -36,7 +38,9 @@ enum imx7_src_registers {
SRC_USBOPHY2_RCR= 0x0024,
SRC_MIPIPHY_RCR = 0x0028,
SRC_PCIEPHY_RCR = 0x002c,
+   SRC_PCIE2PHY_RCR= 0x0048,
SRC_DDRC_RCR= 0x1000,
+
 };
 
 struct imx7_src_signal {
@@ -67,11 +71,67 @@ static const struct imx7_src_signal 
imx7_src_signals[IMX7_RESET_NUM] = {
[IMX7_RESET_PCIEPHY]= { SRC_PCIEPHY_RCR, BIT(2) | BIT(1) },
[IMX7_RESET_PCIEPHY_PERST]  = { SRC_PCIEPHY_RCR, BIT(3) },
[IMX7_RESET_PCIE_CTRL_APPS_EN]  = { SRC_PCIEPHY_RCR, BIT(6) },
+   [IMX7_RESET_PCIE_CTRL_APPS_CLK_REQ] = { SRC_PCIEPHY_RCR, BIT(4) },
[IMX7_RESET_PCIE_CTRL_APPS_TURNOFF] = { SRC_PCIEPHY_RCR, BIT(11) },
[IMX7_RESET_DDRC_PRST]  = { SRC_DDRC_RCR, BIT(0) },
[IMX7_RESET_DDRC_CORE_RST]  = { SRC_DDRC_RCR, BIT(1) },
+
+   [IMX8M_RESET_A53_CORE_POR_RESET2] = { SRC_A53RCR0, BIT(2) },
+   [IMX8M_RESET_A53_CORE_POR_RESET3] = { SRC_A53RCR0, BIT(3) },
+   [IMX8M_RESET_A53_CORE_RESET2] = { SRC_A53RCR0, BIT(6) },
+   [IMX8M_RESET_A53_CORE_RESET3] = { SRC_A53RCR0, BIT(7) },
+   [IMX8M_RESET_A53_ETM_RESET2]  = { SRC_A53RCR0, BIT(14) },
+   [IMX8M_RESET_A53_ETM_RESET3]  = { SRC_A53RCR0, BIT(15) },
+   [IMX8M_RESET_PCIE2PHY]  = { SRC_PCIEPHY_RCR, BIT(2) | BIT(1) },
+   [IMX8M_RESET_PCIE2PHY_PERST]  = { SRC_PCIE2PHY_RCR, BIT(3) },
+   [IMX8M_RESET_PCIE2_CTRL_APPS_EN]  = { SRC_PCIE2PHY_RCR, BIT(6) },
+   [IMX8M_RESET_PCIE2_CTRL_APPS_CLK_REQ] = { SRC_PCIE2PHY_RCR, BIT(4) },
+   [IMX8M_RESET_PCIE2_CTRL_APPS_TURNOFF] = { SRC_PCIE2PHY_RCR, BIT(11) },
 };
 
+static inline void imx7_src_check_definitions(void)
+{
+   BUILD_BUG_ON(IMX8M_RESET_A53_CORE_POR_RESET0 !=
+IMX7_RESET_A7_CORE_POR_RESET0);
+   BUILD_BUG_ON(IMX8M_RESET_A53_CORE_POR_RESET1 !=
+IMX7_RESET_A7_CORE_POR_RESET1);
+   BUILD_BUG_ON(IMX8M_RESET_A53_CORE_RESET0 !=
+IMX7_RESET_A7_CORE_RESET0);
+   BUILD_BUG_ON(IMX8M_RESET_A53_CORE_RESET1 !=
+IMX7_RESET_A7_CORE_RESET1);
+   BUILD_BUG_ON(IMX8M_RESET_A53_DBG_RESET0 !=
+IMX7_RESET_A7_DBG_RESET0);
+   BUILD_BUG_ON(IMX8M_RESET_A53_DBG_RESET1 !=
+IMX7_RESET_A7_DBG_RESET1);
+   BUILD_BUG_ON(IMX8M_RESET_A53_ETM_RESET0 !=
+IMX7_RESET_A7_ETM_RESET0);
+   BUILD_BUG_ON(IMX8M_RESET_A53_ETM_RESET1 !=
+IMX7_RESET_A7_ETM_RESET1);
+   BUILD_BUG_ON(IMX8M_RESET_A53_SOC_DBG_RESET !=
+IMX7_RESET_A7_SOC_DBG_RESET);
+   BUILD_BUG_ON(IMX8M_RESET_A53_L2RESET != IMX7_RESET_A7_L2RESET);
+   BUILD_BUG_ON(IMX8M_RESET_SW_M4C_RST != IMX7_RESET_SW_M4C_RST);
+   BUILD_BUG_ON(IMX8M_RESET_SW_M4P_RST != IMX7_RESET_SW_M4P_RST);
+   BUILD_BUG_ON(IMX8M_RESET_EIM_RST != IMX7_RESET_EIM_RST);
+   BUILD_BUG_ON(IMX8M_RESET_HSICPHY_PORT_RST !=
+IMX7_RESET_HSICPHY_PORT_RST);
+   BUILD_BUG_ON(IMX8M_RESET_USBPHY1_POR != IMX7_RESET_USBPHY1_POR);
+   

[PATCH 1/1] reset: imx7: Add support for i.MX8MQ

2018-11-17 Thread Andrey Smirnov
SRC IP block used in i.MX8MQ is a superset of what is found in i.MX7D,
so add all of the definitions necessary to support both.

Cc: p.za...@pengutronix.de
Cc: Fabio Estevam 
Cc: cphe...@gmail.com
Cc: l.st...@pengutronix.de
Cc: Leonard Crestez 
Cc: "A.s. Dong" 
Cc: Richard Zhu 
Cc: linux-...@nxp.com
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Andrey Smirnov 
---
 drivers/reset/Kconfig   |  2 +-
 drivers/reset/reset-imx7.c  | 66 -
 include/dt-bindings/reset/imx7-reset.h  | 15 +-
 include/dt-bindings/reset/imx8m-reset.h | 47 ++
 4 files changed, 127 insertions(+), 3 deletions(-)
 create mode 100644 include/dt-bindings/reset/imx8m-reset.h

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index c21da9fe51ec..4909aab7401b 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -50,7 +50,7 @@ config RESET_HSDK
 config RESET_IMX7
bool "i.MX7 Reset Driver" if COMPILE_TEST
depends on HAS_IOMEM
-   default SOC_IMX7D
+   default SOC_IMX7D || SOC_IMX8MQ
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 77911fa8f31d..dffad618f805 100644
--- a/drivers/reset/reset-imx7.c
+++ b/drivers/reset/reset-imx7.c
@@ -21,14 +21,16 @@
 #include 
 #include 
 #include 
+#include 
 
 struct imx7_src {
struct reset_controller_dev rcdev;
struct regmap *regmap;
 };
 
-enum imx7_src_registers {
+enum imx_src_registers {
SRC_A7RCR0  = 0x0004,
+   SRC_A53RCR0 = 0x0004,
SRC_M4RCR   = 0x000c,
SRC_ERCR= 0x0014,
SRC_HSICPHY_RCR = 0x001c,
@@ -36,7 +38,9 @@ enum imx7_src_registers {
SRC_USBOPHY2_RCR= 0x0024,
SRC_MIPIPHY_RCR = 0x0028,
SRC_PCIEPHY_RCR = 0x002c,
+   SRC_PCIE2PHY_RCR= 0x0048,
SRC_DDRC_RCR= 0x1000,
+
 };
 
 struct imx7_src_signal {
@@ -67,11 +71,67 @@ static const struct imx7_src_signal 
imx7_src_signals[IMX7_RESET_NUM] = {
[IMX7_RESET_PCIEPHY]= { SRC_PCIEPHY_RCR, BIT(2) | BIT(1) },
[IMX7_RESET_PCIEPHY_PERST]  = { SRC_PCIEPHY_RCR, BIT(3) },
[IMX7_RESET_PCIE_CTRL_APPS_EN]  = { SRC_PCIEPHY_RCR, BIT(6) },
+   [IMX7_RESET_PCIE_CTRL_APPS_CLK_REQ] = { SRC_PCIEPHY_RCR, BIT(4) },
[IMX7_RESET_PCIE_CTRL_APPS_TURNOFF] = { SRC_PCIEPHY_RCR, BIT(11) },
[IMX7_RESET_DDRC_PRST]  = { SRC_DDRC_RCR, BIT(0) },
[IMX7_RESET_DDRC_CORE_RST]  = { SRC_DDRC_RCR, BIT(1) },
+
+   [IMX8M_RESET_A53_CORE_POR_RESET2] = { SRC_A53RCR0, BIT(2) },
+   [IMX8M_RESET_A53_CORE_POR_RESET3] = { SRC_A53RCR0, BIT(3) },
+   [IMX8M_RESET_A53_CORE_RESET2] = { SRC_A53RCR0, BIT(6) },
+   [IMX8M_RESET_A53_CORE_RESET3] = { SRC_A53RCR0, BIT(7) },
+   [IMX8M_RESET_A53_ETM_RESET2]  = { SRC_A53RCR0, BIT(14) },
+   [IMX8M_RESET_A53_ETM_RESET3]  = { SRC_A53RCR0, BIT(15) },
+   [IMX8M_RESET_PCIE2PHY]  = { SRC_PCIEPHY_RCR, BIT(2) | BIT(1) },
+   [IMX8M_RESET_PCIE2PHY_PERST]  = { SRC_PCIE2PHY_RCR, BIT(3) },
+   [IMX8M_RESET_PCIE2_CTRL_APPS_EN]  = { SRC_PCIE2PHY_RCR, BIT(6) },
+   [IMX8M_RESET_PCIE2_CTRL_APPS_CLK_REQ] = { SRC_PCIE2PHY_RCR, BIT(4) },
+   [IMX8M_RESET_PCIE2_CTRL_APPS_TURNOFF] = { SRC_PCIE2PHY_RCR, BIT(11) },
 };
 
+static inline void imx7_src_check_definitions(void)
+{
+   BUILD_BUG_ON(IMX8M_RESET_A53_CORE_POR_RESET0 !=
+IMX7_RESET_A7_CORE_POR_RESET0);
+   BUILD_BUG_ON(IMX8M_RESET_A53_CORE_POR_RESET1 !=
+IMX7_RESET_A7_CORE_POR_RESET1);
+   BUILD_BUG_ON(IMX8M_RESET_A53_CORE_RESET0 !=
+IMX7_RESET_A7_CORE_RESET0);
+   BUILD_BUG_ON(IMX8M_RESET_A53_CORE_RESET1 !=
+IMX7_RESET_A7_CORE_RESET1);
+   BUILD_BUG_ON(IMX8M_RESET_A53_DBG_RESET0 !=
+IMX7_RESET_A7_DBG_RESET0);
+   BUILD_BUG_ON(IMX8M_RESET_A53_DBG_RESET1 !=
+IMX7_RESET_A7_DBG_RESET1);
+   BUILD_BUG_ON(IMX8M_RESET_A53_ETM_RESET0 !=
+IMX7_RESET_A7_ETM_RESET0);
+   BUILD_BUG_ON(IMX8M_RESET_A53_ETM_RESET1 !=
+IMX7_RESET_A7_ETM_RESET1);
+   BUILD_BUG_ON(IMX8M_RESET_A53_SOC_DBG_RESET !=
+IMX7_RESET_A7_SOC_DBG_RESET);
+   BUILD_BUG_ON(IMX8M_RESET_A53_L2RESET != IMX7_RESET_A7_L2RESET);
+   BUILD_BUG_ON(IMX8M_RESET_SW_M4C_RST != IMX7_RESET_SW_M4C_RST);
+   BUILD_BUG_ON(IMX8M_RESET_SW_M4P_RST != IMX7_RESET_SW_M4P_RST);
+   BUILD_BUG_ON(IMX8M_RESET_EIM_RST != IMX7_RESET_EIM_RST);
+   BUILD_BUG_ON(IMX8M_RESET_HSICPHY_PORT_RST !=
+IMX7_RESET_HSICPHY_PORT_RST);
+   BUILD_BUG_ON(IMX8M_RESET_USBPHY1_POR != IMX7_RESET_USBPHY1_POR);
+