[RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

[RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

[Adam Ford]

There are some less-documented registers which control clocks and 
resets for the multimedia block which controls the LCDIF, ISI, MIPI 
CSI, and MIPI DSI.

The i.Mx8M Nano appears to have a subset of the i.MX8MP registers with
a couple shared registers with the i.MX8MM.  This series builds on the
series that have been submitted for both of those other two platforms.

This is an RFC because when enabling the corresponding DTS node, the 
system freezes on power on.  There are a couple of clocks that don't
correspond to either the imx8mp nor the imx8mm, so I might have something
wrong, and I was hoping for some constructive feedback in order to get
the imx8m Nano to a similar point of the Mini and Plus.

Adam Ford (3):
  dt-bindings: clock: imx8mn: Add media blk_ctl clock IDs
  dt-bindings: reset: imx8mn: Add media blk_ctl reset IDs
  clk: imx: Add blk-ctl driver for i.MX8MN

 drivers/clk/imx/clk-blk-ctl-imx8mn.c     | 80 ++++++++++++++++++++++++
 include/dt-bindings/clock/imx8mn-clock.h | 11 ++++
 include/dt-bindings/reset/imx8mn-reset.h | 22 +++++++
 3 files changed, 113 insertions(+)
 create mode 100644 drivers/clk/imx/clk-blk-ctl-imx8mn.c
 create mode 100644 include/dt-bindings/reset/imx8mn-reset.h

-- 
2.25.1

[RFC 1/3] dt-bindings: clock: imx8mn: Add media blk_ctl clock IDs

[Adam Ford]

These will be used by the imx8mn for blk_ctl driver.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 include/dt-bindings/clock/imx8mn-clock.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/dt-bindings/clock/imx8mn-clock.h b/include/dt-bindings/clock/imx8mn-clock.h
index 621ea0e87c67..2baaa2dccfd3 100644
--- a/include/dt-bindings/clock/imx8mn-clock.h
+++ b/include/dt-bindings/clock/imx8mn-clock.h
@@ -236,4 +236,15 @@
 
 #define IMX8MN_CLK_END				215
 
+#define IMX8MN_CLK_MEDIA_BLK_CTL_MIPI_DSI_PCLK	0
+#define IMX8MN_CLK_MEDIA_BLK_CTL_MIPI_DSI_CLKREF	1
+#define IMX8MN_CLK_MEDIA_BLK_CTL_MIPI_CSI_PCLK	2
+#define IMX8MN_CLK_MEDIA_BLK_CTL_MIPI_CSI_ACLK	3
+#define IMX8MN_CLK_MEDIA_BLK_CTL_LCDIF_PIXEL		4
+#define IMX8MN_CLK_MEDIA_BLK_CTL_LCDIF_APB		5
+#define IMX8MN_CLK_MEDIA_BLK_CTL_ISI_PROC		6
+#define IMX8MN_CLK_MEDIA_BLK_CTL_ISI_APB		7
+#define IMX8MN_CLK_MEDIA_BLK_CTL_BUS_BLK		8
+#define IMX8MN_CLK_MEDIA_BLK_CTL_END			9
+
 #endif
-- 
2.25.1

Re: [RFC 1/3] dt-bindings: clock: imx8mn: Add media blk_ctl clock IDs

[Rob Herring]

On Sat, 24 Oct 2020 11:20:13 -0500, Adam Ford wrote:
> These will be used by the imx8mn for blk_ctl driver.
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>
> ---
>  include/dt-bindings/clock/imx8mn-clock.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

[RFC 2/3] dt-bindings: reset: imx8mn: Add media blk_ctl reset IDs

[Adam Ford]

These will be used by the imx8mn for blk_ctl driver.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 include/dt-bindings/reset/imx8mn-reset.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/include/dt-bindings/reset/imx8mn-reset.h b/include/dt-bindings/reset/imx8mn-reset.h
new file mode 100644
index 000000000000..89d402742b96
--- /dev/null
+++ b/include/dt-bindings/reset/imx8mn-reset.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2020 Compass Electronics Group, LLC
+ */
+
+#ifndef DT_BINDING_RESET_IMX8MN_H
+#define DT_BINDING_RESET_IMX8MN_H
+
+#define IMX8MN_MEDIA_BLK_CTL_RESET_MIPI_DSI_PCLK	0
+#define IMX8MN_MEDIA_BLK_CTL_RESET_MIPI_DSI_CLKREF	1
+#define IMX8MN_MEDIA_BLK_CTL_RESET_MIPI_CSI_PCLK	2
+#define IMX8MN_MEDIA_BLK_CTL_RESET_MIPI_CSI_ACLK	3
+#define IMX8MN_MEDIA_BLK_CTL_RESET_LCDIF_PIXEL	4
+#define IMX8MN_MEDIA_BLK_CTL_RESET_LCDIF_APB		5
+#define IMX8MN_MEDIA_BLK_CTL_RESET_ISI_PROC		6
+#define IMX8MN_MEDIA_BLK_CTL_RESET_ISI_APB		7
+#define IMX8MN_MEDIA_BLK_CTL_RESET_BUS_BLK		8
+#define IMX8MN_MEDIA_BLK_CTL_RESET_MIPI_S_RESET	9
+#define IMX8MN_MEDIA_BLK_CTL_RESET_MIPI_M_RESET	10
+#define IMX8MN_MEDIA_BLK_CTL_RESET_NUM		11
+
+#endif
-- 
2.25.1

Re: [RFC 2/3] dt-bindings: reset: imx8mn: Add media blk_ctl reset IDs

[Rob Herring]

On Sat, 24 Oct 2020 11:20:14 -0500, Adam Ford wrote:
> These will be used by the imx8mn for blk_ctl driver.
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>
> ---
>  include/dt-bindings/reset/imx8mn-reset.h | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

[RFC 3/3] clk: imx: Add blk-ctl driver for i.MX8MN

[Adam Ford]

This driver is intended to work with the multimedia block which
contains display and camera subsystems:
  LCDIF
  ISI
  MIPI CSI
  MIPI DSI

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 drivers/clk/imx/clk-blk-ctl-imx8mn.c | 80 ++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/drivers/clk/imx/clk-blk-ctl-imx8mn.c b/drivers/clk/imx/clk-blk-ctl-imx8mn.c
new file mode 100644
index 000000000000..917514212270
--- /dev/null
+++ b/drivers/clk/imx/clk-blk-ctl-imx8mn.c
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 Compass Eletronics Group, LLC
+ */
+
+#include <dt-bindings/clock/imx8mn-clock.h>
+#include <dt-bindings/reset/imx8mn-reset.h>
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#include "clk.h"
+#include "clk-blk-ctl.h"
+
+#define IMX_MEDIA_BLK_CTL_SFT_RSTN		0x0
+#define IMX_MEDIA_BLK_CTL_CLK_EN		0x4
+#define IMX_MEDIA_BLK_CTL_MIPI_RST		0x8
+
+static struct imx_blk_ctl_hw imx8mn_media_blk_ctl_hws[] = {
+	/* clocks */
+	IMX_BLK_CTL_CLK_GATE("mipi_dsi_pclk", IMX8MN_CLK_MEDIA_BLK_CTL_MIPI_DSI_PCLK, 0x4, 0, "dsi_core"),
+	IMX_BLK_CTL_CLK_GATE("mipi_dsi_clkref", IMX8MN_CLK_MEDIA_BLK_CTL_MIPI_DSI_CLKREF, 0x4, 1, "dsi_phy_ref"),
+	IMX_BLK_CTL_CLK_GATE("mipi_csi_pclk", IMX8MN_CLK_MEDIA_BLK_CTL_MIPI_CSI_PCLK, 0x4, 2, "camera_pixel_clk"),
+	IMX_BLK_CTL_CLK_GATE("mipi_csi_aclk", IMX8MN_CLK_MEDIA_BLK_CTL_MIPI_CSI_ACLK, 0x4, 3, "media_cam1_pix_root_clk"),
+	IMX_BLK_CTL_CLK_GATE("lcdif_pixel_clk", IMX8MN_CLK_MEDIA_BLK_CTL_LCDIF_PIXEL, 0x4, 4, "disp_pixel"),
+	IMX_BLK_CTL_CLK_GATE("lcdif_apb_clk", IMX8MN_CLK_MEDIA_BLK_CTL_LCDIF_APB, 0x4, 5, "disp_apb"),
+	IMX_BLK_CTL_CLK_GATE("isi_proc_clk", IMX8MN_CLK_MEDIA_BLK_CTL_ISI_PROC, 0x4, 6, "disp_axi_root_clk"),
+	IMX_BLK_CTL_CLK_GATE("isi_apb_clk", IMX8MN_CLK_MEDIA_BLK_CTL_ISI_APB, 0x4, 7, "disp_apb_root_clk"),
+	IMX_BLK_CTL_CLK_GATE("bus_blk_clk", IMX8MN_CLK_MEDIA_BLK_CTL_BUS_BLK, 0x4, 8, "disp_axi_root_clk"),
+	
+	/* resets */
+	IMX_BLK_CTL_RESET(IMX8MN_MEDIA_BLK_CTL_RESET_MIPI_DSI_PCLK, 0, 0),
+	IMX_BLK_CTL_RESET(IMX8MN_MEDIA_BLK_CTL_RESET_MIPI_DSI_CLKREF, 0, 1),
+	IMX_BLK_CTL_RESET(IMX8MN_MEDIA_BLK_CTL_RESET_MIPI_CSI_PCLK, 0, 2),
+	IMX_BLK_CTL_RESET(IMX8MN_MEDIA_BLK_CTL_RESET_MIPI_CSI_ACLK, 0, 3),
+	IMX_BLK_CTL_RESET(IMX8MN_MEDIA_BLK_CTL_RESET_LCDIF_PIXEL, 0, 4),
+	IMX_BLK_CTL_RESET(IMX8MN_MEDIA_BLK_CTL_RESET_LCDIF_APB, 0, 5),
+	IMX_BLK_CTL_RESET(IMX8MN_MEDIA_BLK_CTL_RESET_ISI_PROC, 0, 6),
+	IMX_BLK_CTL_RESET(IMX8MN_MEDIA_BLK_CTL_RESET_ISI_APB, 0, 7),
+	IMX_BLK_CTL_RESET(IMX8MN_MEDIA_BLK_CTL_RESET_BUS_BLK, 0, 8),
+	IMX_BLK_CTL_RESET(IMX8MN_MEDIA_BLK_CTL_RESET_MIPI_S_RESET, 0x8, 16),
+	IMX_BLK_CTL_RESET(IMX8MN_MEDIA_BLK_CTL_RESET_MIPI_M_RESET, 0x8, 17),
+};
+
+const struct imx_blk_ctl_dev_data imx8mn_media_blk_ctl_dev_data __initconst = {
+	.hws = imx8mn_media_blk_ctl_hws,
+	.hws_num = ARRAY_SIZE(imx8mn_media_blk_ctl_hws),
+	.clocks_max = IMX8MN_CLK_MEDIA_BLK_CTL_END,
+	.resets_max = IMX8MN_MEDIA_BLK_CTL_RESET_NUM,
+	.pm_runtime_saved_regs_num = 3,
+	.pm_runtime_saved_regs = {
+		IMX_MEDIA_BLK_CTL_SFT_RSTN,
+		IMX_MEDIA_BLK_CTL_CLK_EN,
+		IMX_MEDIA_BLK_CTL_MIPI_RST,
+	},
+};
+
+static const struct of_device_id imx8mn_blk_ctl_of_match[] = {
+	{
+		.compatible = "fsl,imx8mn-media-blk-ctl",
+		.data = &imx8mn_media_blk_ctl_dev_data
+	},
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imx8mn_blk_ctl_of_match);
+
+static struct platform_driver imx_blk_ctl_driver = {
+	.probe = imx_blk_ctl_probe,
+	.driver = {
+		.name = "imx8mn-blk-ctl",
+		.of_match_table = of_match_ptr(imx8mn_blk_ctl_of_match),
+		.pm = &imx_blk_ctl_pm_ops,
+	},
+};
+module_platform_driver(imx_blk_ctl_driver);
-- 
2.25.1

Re: [RFC 3/3] clk: imx: Add blk-ctl driver for i.MX8MN

[Marek Vasut]

On 10/24/20 6:20 PM, Adam Ford wrote:
> This driver is intended to work with the multimedia block which
> contains display and camera subsystems:
>    LCDIF
>    ISI
>    MIPI CSI
>    MIPI DSI
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>
> ---
>   drivers/clk/imx/clk-blk-ctl-imx8mn.c | 80 ++++++++++++++++++++++++++++

You seem to be missing the Makefile entries.

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

[Abel Vesa]

On 20-10-24 11:20:12, Adam Ford wrote:
> There are some less-documented registers which control clocks and 
> resets for the multimedia block which controls the LCDIF, ISI, MIPI 
> CSI, and MIPI DSI.
> 
> The i.Mx8M Nano appears to have a subset of the i.MX8MP registers with
> a couple shared registers with the i.MX8MM.  This series builds on the
> series that have been submitted for both of those other two platforms.
> 
> This is an RFC because when enabling the corresponding DTS node, the 
> system freezes on power on.  There are a couple of clocks that don't
> correspond to either the imx8mp nor the imx8mm, so I might have something
> wrong, and I was hoping for some constructive feedback in order to get
> the imx8m Nano to a similar point of the Mini and Plus.
> 

Thanks for the effort.

I'm assuming this relies on the following patchset, right ?
https://lkml.org/lkml/2020/10/24/139

> Adam Ford (3):
>   dt-bindings: clock: imx8mn: Add media blk_ctl clock IDs
>   dt-bindings: reset: imx8mn: Add media blk_ctl reset IDs
>   clk: imx: Add blk-ctl driver for i.MX8MN
> 
>  drivers/clk/imx/clk-blk-ctl-imx8mn.c     | 80 ++++++++++++++++++++++++
>  include/dt-bindings/clock/imx8mn-clock.h | 11 ++++
>  include/dt-bindings/reset/imx8mn-reset.h | 22 +++++++
>  3 files changed, 113 insertions(+)
>  create mode 100644 drivers/clk/imx/clk-blk-ctl-imx8mn.c
>  create mode 100644 include/dt-bindings/reset/imx8mn-reset.h
> 
> -- 
> 2.25.1
> 

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

[Adam Ford]

On Sat, Oct 24, 2020 at 3:23 PM Abel Vesa <abel.vesa@nxp.com> wrote:
>
> On 20-10-24 11:20:12, Adam Ford wrote:
> > There are some less-documented registers which control clocks and
> > resets for the multimedia block which controls the LCDIF, ISI, MIPI
> > CSI, and MIPI DSI.
> >
> > The i.Mx8M Nano appears to have a subset of the i.MX8MP registers with
> > a couple shared registers with the i.MX8MM.  This series builds on the
> > series that have been submitted for both of those other two platforms.
> >
> > This is an RFC because when enabling the corresponding DTS node, the
> > system freezes on power on.  There are a couple of clocks that don't
> > correspond to either the imx8mp nor the imx8mm, so I might have something
> > wrong, and I was hoping for some constructive feedback in order to get
> > the imx8m Nano to a similar point of the Mini and Plus.
> >
>
> Thanks for the effort.

Sure thing!

>
> I'm assuming this relies on the following patchset, right ?
> https://lkml.org/lkml/2020/10/24/139
Abell,

Your link points right back to this e-mail.  ;-)

I based this work off:
https://www.spinics.net/lists/arm-kernel/msg843906.html  from Marek
which I beleive is based on
https://www.spinics.net/lists/arm-kernel/msg836165.html from you.

I also have a GPC patch series located:
https://www.spinics.net/lists/arm-kernel/msg847925.html

Together, both the GPC and the clk-blk driver should be able to pull
the multimedia block out of reset.  Currently, the GPC can handle the
USB OTG and the GPU, but the LCDIF and MIPI DSI appear to be gated by
the clock block

My original patch RFC didn't include the imx8mn node, because it
hangs, but the node I added looks like:

media_blk_ctl: clock-controller@32e28000 {
     compatible = "fsl,imx8mn-media-blk-ctl", "syscon";
     reg = <0x32e28000 0x1000>;
     #clock-cells = <1>;
     #reset-cells = <1>;
};

I was hoping you might have some feedback on the 8mn clk-blk driver
since you did the 8mp clk-blk drive and they appear to be very
similar.

adam


>
> > Adam Ford (3):
> >   dt-bindings: clock: imx8mn: Add media blk_ctl clock IDs
> >   dt-bindings: reset: imx8mn: Add media blk_ctl reset IDs
> >   clk: imx: Add blk-ctl driver for i.MX8MN
> >
> >  drivers/clk/imx/clk-blk-ctl-imx8mn.c     | 80 ++++++++++++++++++++++++
> >  include/dt-bindings/clock/imx8mn-clock.h | 11 ++++
> >  include/dt-bindings/reset/imx8mn-reset.h | 22 +++++++
> >  3 files changed, 113 insertions(+)
> >  create mode 100644 drivers/clk/imx/clk-blk-ctl-imx8mn.c
> >  create mode 100644 include/dt-bindings/reset/imx8mn-reset.h
> >
> > --
> > 2.25.1
> >

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

[Abel Vesa]

On 20-10-24 16:03:17, Adam Ford wrote:
> On Sat, Oct 24, 2020 at 3:23 PM Abel Vesa <abel.vesa@nxp.com> wrote:
> >
> > On 20-10-24 11:20:12, Adam Ford wrote:
> > > There are some less-documented registers which control clocks and
> > > resets for the multimedia block which controls the LCDIF, ISI, MIPI
> > > CSI, and MIPI DSI.
> > >
> > > The i.Mx8M Nano appears to have a subset of the i.MX8MP registers with
> > > a couple shared registers with the i.MX8MM.  This series builds on the
> > > series that have been submitted for both of those other two platforms.
> > >
> > > This is an RFC because when enabling the corresponding DTS node, the
> > > system freezes on power on.  There are a couple of clocks that don't
> > > correspond to either the imx8mp nor the imx8mm, so I might have something
> > > wrong, and I was hoping for some constructive feedback in order to get
> > > the imx8m Nano to a similar point of the Mini and Plus.
> > >
> >
> > Thanks for the effort.
> 
> Sure thing!
> 
> >
> > I'm assuming this relies on the following patchset, right ?
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2020%2F10%2F24%2F139&amp;data=04%7C01%7Cabel.vesa%40nxp.com%7C00007fdffcb44af4bbe808d8786044cf%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637391702150893977%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=RIdR4O2Qx3q7ovfAO7O3kc%2BpXXMPzLQJWPoUAH305%2Bs%3D&amp;reserved=0
> Abell,
> 
> Your link points right back to this e-mail.  ;-)

Sorry about that. Was kinda late here yesterday.

> 
> I based this work off:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Farm-kernel%2Fmsg843906.html&amp;data=04%7C01%7Cabel.vesa%40nxp.com%7C00007fdffcb44af4bbe808d8786044cf%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637391702150893977%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=6B3UHZNULMB%2FtMzdWHckIOFlqFxEYwYdQclzqDtJ%2FNQ%3D&amp;reserved=0  from Marek
> which I beleive is based on
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Farm-kernel%2Fmsg836165.html&amp;data=04%7C01%7Cabel.vesa%40nxp.com%7C00007fdffcb44af4bbe808d8786044cf%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637391702150893977%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=I%2FfgIjzqwnjuSGOPfVKa%2BPbx950Be008sOon%2FDwSO1o%3D&amp;reserved=0 from you.
> 
> I also have a GPC patch series located:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Farm-kernel%2Fmsg847925.html&amp;data=04%7C01%7Cabel.vesa%40nxp.com%7C00007fdffcb44af4bbe808d8786044cf%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637391702150893977%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=dF%2Bgth7dwopoB2rvQ8IqspTZ7kWxvMYGS0dY%2F0gWgXE%3D&amp;reserved=0
> 
> Together, both the GPC and the clk-blk driver should be able to pull
> the multimedia block out of reset.  Currently, the GPC can handle the
> USB OTG and the GPU, but the LCDIF and MIPI DSI appear to be gated by
> the clock block
> 
> My original patch RFC didn't include the imx8mn node, because it
> hangs, but the node I added looks like:
> 
> media_blk_ctl: clock-controller@32e28000 {
>      compatible = "fsl,imx8mn-media-blk-ctl", "syscon";
>      reg = <0x32e28000 0x1000>;
>      #clock-cells = <1>;
>      #reset-cells = <1>;
> };
> 
> I was hoping you might have some feedback on the 8mn clk-blk driver
> since you did the 8mp clk-blk drive and they appear to be very
> similar.
> 

I'll do you one better still. I'll apply the patch in my tree and give it
a test tomorrow morning.

> adam
> 
> 
> >
> > > Adam Ford (3):
> > >   dt-bindings: clock: imx8mn: Add media blk_ctl clock IDs
> > >   dt-bindings: reset: imx8mn: Add media blk_ctl reset IDs
> > >   clk: imx: Add blk-ctl driver for i.MX8MN
> > >
> > >  drivers/clk/imx/clk-blk-ctl-imx8mn.c     | 80 ++++++++++++++++++++++++
> > >  include/dt-bindings/clock/imx8mn-clock.h | 11 ++++
> > >  include/dt-bindings/reset/imx8mn-reset.h | 22 +++++++
> > >  3 files changed, 113 insertions(+)
> > >  create mode 100644 drivers/clk/imx/clk-blk-ctl-imx8mn.c
> > >  create mode 100644 include/dt-bindings/reset/imx8mn-reset.h
> > >
> > > --
> > > 2.25.1
> > >

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

[Marek Vasut]

On 10/25/20 1:05 PM, Abel Vesa wrote:

[...]

>> Together, both the GPC and the clk-blk driver should be able to pull
>> the multimedia block out of reset.  Currently, the GPC can handle the
>> USB OTG and the GPU, but the LCDIF and MIPI DSI appear to be gated by
>> the clock block
>>
>> My original patch RFC didn't include the imx8mn node, because it
>> hangs, but the node I added looks like:
>>
>> media_blk_ctl: clock-controller@32e28000 {
>>      compatible = "fsl,imx8mn-media-blk-ctl", "syscon";
>>      reg = <0x32e28000 0x1000>;
>>      #clock-cells = <1>;
>>      #reset-cells = <1>;
>> };
>>
>> I was hoping you might have some feedback on the 8mn clk-blk driver
>> since you did the 8mp clk-blk drive and they appear to be very
>> similar.
>>
> 
> I'll do you one better still. I'll apply the patch in my tree and give it
> a test tomorrow morning.

You can also apply the one for 8MM:
https://lore.kernel.org/linux-arm-kernel/20201003224555.163780-5-marex@denx.de/

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

[Adam Ford]

On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut <marex@denx.de> wrote:
>
> On 10/25/20 1:05 PM, Abel Vesa wrote:
>
> [...]
>
> >> Together, both the GPC and the clk-blk driver should be able to pull
> >> the multimedia block out of reset.  Currently, the GPC can handle the
> >> USB OTG and the GPU, but the LCDIF and MIPI DSI appear to be gated by
> >> the clock block
> >>
> >> My original patch RFC didn't include the imx8mn node, because it
> >> hangs, but the node I added looks like:
> >>
> >> media_blk_ctl: clock-controller@32e28000 {
> >>      compatible = "fsl,imx8mn-media-blk-ctl", "syscon";
> >>      reg = <0x32e28000 0x1000>;
> >>      #clock-cells = <1>;
> >>      #reset-cells = <1>;
> >> };
> >>
> >> I was hoping you might have some feedback on the 8mn clk-blk driver
> >> since you did the 8mp clk-blk drive and they appear to be very
> >> similar.
> >>
> >
> > I'll do you one better still. I'll apply the patch in my tree and give it
> > a test tomorrow morning.

I do have some more updates on how to get the system to not hang, and
to enumerate more clocks.
Looking at Marek's work on enabling clocks in the 8MM, he added a
power-domain in dispmix_blk_ctl pointing to the dispmix in the GPC.
By forcing the GPC driver to write 0x1fff  to 32e28004, 0x7f to
32e28000 and 0x30000 to 32e28008, the i.MX8MM can bring the display
clocks out of reset.

Unfortunately, the i.MX8MN needs to have 0x100 written to both
32e28000 and 32e28004, and the values written for the 8MM are not
compatible.
By forcing the GPC to write those values, I can get  lcdif_pixel_clk
and the mipi_dsi_clkref  appearing on the Nano.

 video_pll1_ref_sel                0        0        0    24000000
     0     0  50000
       video_pll1                     0        0        0   594000000
        0     0  50000
          video_pll1_bypass           0        0        0   594000000
        0     0  50000
             video_pll1_out           0        0        0   594000000
        0     0  50000
                disp_pixel            0        0        0   594000000
        0     0  50000
                   lcdif_pixel_clk       0        0        0
594000000          0     0  50000
                   disp_pixel_clk       0        0        0
594000000          0     0  50000
                dsi_phy_ref           0        0        0    27000000
        0     0  50000
                   mipi_dsi_clkref       0        0        0
27000000          0     0  50000

I am not 100% certain the clock parents  in the clk block driver for
the 8MN are correct, and I am not seeing the mipi_dsi_pclk

Once the dust settles on the GPC decision for Mini and Nano, I think
we'll need a more generic way to pass the bits we need to set in clock
block to the GPC.

adam
>
> You can also apply the one for 8MM:
> https://lore.kernel.org/linux-arm-kernel/20201003224555.163780-5-marex@denx.de/

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

[Abel Vesa]

On 20-10-25 11:05:32, Adam Ford wrote:
> On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut <marex@denx.de> wrote:
> >
> > On 10/25/20 1:05 PM, Abel Vesa wrote:
> >
> > [...]
> >
> > >> Together, both the GPC and the clk-blk driver should be able to pull
> > >> the multimedia block out of reset.  Currently, the GPC can handle the
> > >> USB OTG and the GPU, but the LCDIF and MIPI DSI appear to be gated by
> > >> the clock block
> > >>
> > >> My original patch RFC didn't include the imx8mn node, because it
> > >> hangs, but the node I added looks like:
> > >>
> > >> media_blk_ctl: clock-controller@32e28000 {
> > >>      compatible = "fsl,imx8mn-media-blk-ctl", "syscon";
> > >>      reg = <0x32e28000 0x1000>;
> > >>      #clock-cells = <1>;
> > >>      #reset-cells = <1>;
> > >> };
> > >>
> > >> I was hoping you might have some feedback on the 8mn clk-blk driver
> > >> since you did the 8mp clk-blk drive and they appear to be very
> > >> similar.
> > >>
> > >
> > > I'll do you one better still. I'll apply the patch in my tree and give it
> > > a test tomorrow morning.
> 
> I do have some more updates on how to get the system to not hang, and
> to enumerate more clocks.
> Looking at Marek's work on enabling clocks in the 8MM, he added a
> power-domain in dispmix_blk_ctl pointing to the dispmix in the GPC.
> By forcing the GPC driver to write 0x1fff  to 32e28004, 0x7f to
> 32e28000 and 0x30000 to 32e28008, the i.MX8MM can bring the display
> clocks out of reset.
> 

Yeah, that makes sense. Basically, it was trying to disable unused clocks
(see clk_disable_unused) but in order to disable the clocks from the
media BLK_CTL (which I think should be renamed in display BLK_CTL) the
PD need to be on. Since you initially didn't give it any PD, it was trying
to blindly write/read the gate bit and therefore freeze.

> Unfortunately, the i.MX8MN needs to have 0x100 written to both
> 32e28000 and 32e28004, and the values written for the 8MM are not
> compatible.
> By forcing the GPC to write those values, I can get  lcdif_pixel_clk
> and the mipi_dsi_clkref  appearing on the Nano.

I'm trying to make a branch with all the patches for all i.MX8M so I
can keep track of it all. On this branch I've also applied the 
following patchset from Lucas Stach:
https://www.spinics.net/lists/arm-kernel/msg843007.html
but I'm getting the folowing errors:

[   16.690885] imx-pgc imx-pgc-domain.3: failed to power up ADB400
[   16.716839] imx-pgc imx-pgc-domain.3: failed to power up ADB400
[   16.730500] imx-pgc imx-pgc-domain.3: failed to power up ADB400

Lucas, any thoughts?

Maybe it's something related to 8MN.

Will dig further, see what pops out.

> 
>  video_pll1_ref_sel                0        0        0    24000000
>      0     0  50000
>        video_pll1                     0        0        0   594000000
>         0     0  50000
>           video_pll1_bypass           0        0        0   594000000
>         0     0  50000
>              video_pll1_out           0        0        0   594000000
>         0     0  50000
>                 disp_pixel            0        0        0   594000000
>         0     0  50000
>                    lcdif_pixel_clk       0        0        0
> 594000000          0     0  50000
>                    disp_pixel_clk       0        0        0
> 594000000          0     0  50000
>                 dsi_phy_ref           0        0        0    27000000
>         0     0  50000
>                    mipi_dsi_clkref       0        0        0
> 27000000          0     0  50000
> 
> I am not 100% certain the clock parents  in the clk block driver for
> the 8MN are correct, and I am not seeing the mipi_dsi_pclk
> 
> Once the dust settles on the GPC decision for Mini and Nano, I think
> we'll need a more generic way to pass the bits we need to set in clock
> block to the GPC.
> 
> adam
> >
> > You can also apply the one for 8MM:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flinux-arm-kernel%2F20201003224555.163780-5-marex%40denx.de%2F&amp;data=04%7C01%7Cabel.vesa%40nxp.com%7Cae966cce11204214fb1908d878ffd492%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637392387462398200%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=M944BaOI7Sa1RmI0nwrshKaM7MGMEN5pWgjmYqXZkns%3D&amp;reserved=0

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

[Adam Ford]

On Mon, Oct 26, 2020 at 9:55 AM Abel Vesa <abel.vesa@nxp.com> wrote:
>
> On 20-10-25 11:05:32, Adam Ford wrote:
> > On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut <marex@denx.de> wrote:
> > >
> > > On 10/25/20 1:05 PM, Abel Vesa wrote:
> > >
> > > [...]
> > >
> > > >> Together, both the GPC and the clk-blk driver should be able to pull
> > > >> the multimedia block out of reset.  Currently, the GPC can handle the
> > > >> USB OTG and the GPU, but the LCDIF and MIPI DSI appear to be gated by
> > > >> the clock block
> > > >>
> > > >> My original patch RFC didn't include the imx8mn node, because it
> > > >> hangs, but the node I added looks like:
> > > >>
> > > >> media_blk_ctl: clock-controller@32e28000 {
> > > >>      compatible = "fsl,imx8mn-media-blk-ctl", "syscon";
> > > >>      reg = <0x32e28000 0x1000>;
> > > >>      #clock-cells = <1>;
> > > >>      #reset-cells = <1>;
> > > >> };
> > > >>
> > > >> I was hoping you might have some feedback on the 8mn clk-blk driver
> > > >> since you did the 8mp clk-blk drive and they appear to be very
> > > >> similar.
> > > >>
> > > >
> > > > I'll do you one better still. I'll apply the patch in my tree and give it
> > > > a test tomorrow morning.
> >
> > I do have some more updates on how to get the system to not hang, and
> > to enumerate more clocks.
> > Looking at Marek's work on enabling clocks in the 8MM, he added a
> > power-domain in dispmix_blk_ctl pointing to the dispmix in the GPC.
> > By forcing the GPC driver to write 0x1fff  to 32e28004, 0x7f to
> > 32e28000 and 0x30000 to 32e28008, the i.MX8MM can bring the display
> > clocks out of reset.
> >
>
> Yeah, that makes sense. Basically, it was trying to disable unused clocks
> (see clk_disable_unused) but in order to disable the clocks from the
> media BLK_CTL (which I think should be renamed in display BLK_CTL) the
> PD need to be on. Since you initially didn't give it any PD, it was trying
> to blindly write/read the gate bit and therefore freeze.
>
> > Unfortunately, the i.MX8MN needs to have 0x100 written to both
> > 32e28000 and 32e28004, and the values written for the 8MM are not
> > compatible.
> > By forcing the GPC to write those values, I can get  lcdif_pixel_clk
> > and the mipi_dsi_clkref  appearing on the Nano.
>
> I'm trying to make a branch with all the patches for all i.MX8M so I
> can keep track of it all. On this branch I've also applied the
> following patchset from Lucas Stach:
> https://www.spinics.net/lists/arm-kernel/msg843007.html
> but I'm getting the folowing errors:
>
> [   16.690885] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> [   16.716839] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> [   16.730500] imx-pgc imx-pgc-domain.3: failed to power up ADB400
>
> Lucas, any thoughts?
>
> Maybe it's something related to 8MN.
>
I will go back and double check this now that we have both the
blt_crl->power-domain and the power-domain->blk_ctl.

> Will dig further, see what pops out.

I wasn't sure which direction to go with the name.  I can rename the
media_blk_ctl  driver to display_blk_ctl.  I used Media based on the
imx8mp naming convention and the fact that it's controlling both the
display and the camera interface, however it's depending on the
dispmix GPC.

I'll submit a RFC V2 with the cross referencing to the GPC based on
Marek's Mini patch set, but we'll still have an issue where the Mini
and Nano have different syscon values to enable the clocks, and
Marek's branch has it card-coded, so my patch would effectively break
the Mini in order to make the Nano operate until we find a better
solution.

adam

>
> >
> >  video_pll1_ref_sel                0        0        0    24000000
> >      0     0  50000
> >        video_pll1                     0        0        0   594000000
> >         0     0  50000
> >           video_pll1_bypass           0        0        0   594000000
> >         0     0  50000
> >              video_pll1_out           0        0        0   594000000
> >         0     0  50000
> >                 disp_pixel            0        0        0   594000000
> >         0     0  50000
> >                    lcdif_pixel_clk       0        0        0
> > 594000000          0     0  50000
> >                    disp_pixel_clk       0        0        0
> > 594000000          0     0  50000
> >                 dsi_phy_ref           0        0        0    27000000
> >         0     0  50000
> >                    mipi_dsi_clkref       0        0        0
> > 27000000          0     0  50000
> >
> > I am not 100% certain the clock parents  in the clk block driver for
> > the 8MN are correct, and I am not seeing the mipi_dsi_pclk
> >
> > Once the dust settles on the GPC decision for Mini and Nano, I think
> > we'll need a more generic way to pass the bits we need to set in clock
> > block to the GPC.
> >
> > adam
> > >
> > > You can also apply the one for 8MM:
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flinux-arm-kernel%2F20201003224555.163780-5-marex%40denx.de%2F&amp;data=04%7C01%7Cabel.vesa%40nxp.com%7Cae966cce11204214fb1908d878ffd492%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637392387462398200%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=M944BaOI7Sa1RmI0nwrshKaM7MGMEN5pWgjmYqXZkns%3D&amp;reserved=0

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

[Lucas Stach]

Am Montag, den 26.10.2020, 10:12 -0500 schrieb Adam Ford:
> On Mon, Oct 26, 2020 at 9:55 AM Abel Vesa <abel.vesa@nxp.com> wrote:
> > On 20-10-25 11:05:32, Adam Ford wrote:
> > > On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut <marex@denx.de> wrote:
> > > > On 10/25/20 1:05 PM, Abel Vesa wrote:
> > > > 
> > > > [...]
> > > > 
> > > > > > Together, both the GPC and the clk-blk driver should be able to pull
> > > > > > the multimedia block out of reset.  Currently, the GPC can handle the
> > > > > > USB OTG and the GPU, but the LCDIF and MIPI DSI appear to be gated by
> > > > > > the clock block
> > > > > > 
> > > > > > My original patch RFC didn't include the imx8mn node, because it
> > > > > > hangs, but the node I added looks like:
> > > > > > 
> > > > > > media_blk_ctl: clock-controller@32e28000 {
> > > > > >      compatible = "fsl,imx8mn-media-blk-ctl", "syscon";
> > > > > >      reg = <0x32e28000 0x1000>;
> > > > > >      #clock-cells = <1>;
> > > > > >      #reset-cells = <1>;
> > > > > > };
> > > > > > 
> > > > > > I was hoping you might have some feedback on the 8mn clk-blk driver
> > > > > > since you did the 8mp clk-blk drive and they appear to be very
> > > > > > similar.
> > > > > > 
> > > > > 
> > > > > I'll do you one better still. I'll apply the patch in my tree and give it
> > > > > a test tomorrow morning.
> > > 
> > > I do have some more updates on how to get the system to not hang, and
> > > to enumerate more clocks.
> > > Looking at Marek's work on enabling clocks in the 8MM, he added a
> > > power-domain in dispmix_blk_ctl pointing to the dispmix in the GPC.
> > > By forcing the GPC driver to write 0x1fff  to 32e28004, 0x7f to
> > > 32e28000 and 0x30000 to 32e28008, the i.MX8MM can bring the display
> > > clocks out of reset.
> > > 
> > 
> > Yeah, that makes sense. Basically, it was trying to disable unused clocks
> > (see clk_disable_unused) but in order to disable the clocks from the
> > media BLK_CTL (which I think should be renamed in display BLK_CTL) the
> > PD need to be on. Since you initially didn't give it any PD, it was trying
> > to blindly write/read the gate bit and therefore freeze.
> > 
> > > Unfortunately, the i.MX8MN needs to have 0x100 written to both
> > > 32e28000 and 32e28004, and the values written for the 8MM are not
> > > compatible.
> > > By forcing the GPC to write those values, I can get  lcdif_pixel_clk
> > > and the mipi_dsi_clkref  appearing on the Nano.
> > 
> > I'm trying to make a branch with all the patches for all i.MX8M so I
> > can keep track of it all. On this branch I've also applied the
> > following patchset from Lucas Stach:
> > https://www.spinics.net/lists/arm-kernel/msg843007.html
> > but I'm getting the folowing errors:
> > 
> > [   16.690885] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > [   16.716839] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > [   16.730500] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > 
> > Lucas, any thoughts?
> > 
> > Maybe it's something related to 8MN.
> > 
> I will go back and double check this now that we have both the
> blt_crl->power-domain and the power-domain->blk_ctl.
> 
> > Will dig further, see what pops out.
> 
> I wasn't sure which direction to go with the name.  I can rename the
> media_blk_ctl  driver to display_blk_ctl.  I used Media based on the
> imx8mp naming convention and the fact that it's controlling both the
> display and the camera interface, however it's depending on the
> dispmix GPC.
> 
> I'll submit a RFC V2 with the cross referencing to the GPC based on
> Marek's Mini patch set, but we'll still have an issue where the Mini
> and Nano have different syscon values to enable the clocks, and
> Marek's branch has it card-coded, so my patch would effectively break
> the Mini in order to make the Nano operate until we find a better
> solution.

The GPC should not write into the BLK_CTL region via syscon, but
instead use the clocks and resets as exposed by the BLK_CTL driver.
Doing it via syscon is a hack to get things going. The clocks and
resets should properly be hooked up to the GPC domains via the clocks
and resets DT properties.

For the clocks there is one complication: if the clocks are controlled
via BLK_CTL we can only enable them once the domain is powered up,
however the earlier designs using the GPCv2 assert resets as part of
the power up sequence, which needs the clocks to be running for the
reset to propagate. So depending on whether we have a power domain with
a BLK_CTL or not we need to enable the clocks before or after powering
up the domain. I guess we need a new DT property to specify which way
the domain needs to handled.

Regards,
Lucas

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

[Adam Ford]

On Mon, Oct 26, 2020 at 10:44 AM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Am Montag, den 26.10.2020, 10:12 -0500 schrieb Adam Ford:
> > On Mon, Oct 26, 2020 at 9:55 AM Abel Vesa <abel.vesa@nxp.com> wrote:
> > > On 20-10-25 11:05:32, Adam Ford wrote:
> > > > On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut <marex@denx.de> wrote:
> > > > > On 10/25/20 1:05 PM, Abel Vesa wrote:
> > > > >
> > > > > [...]
> > > > >
> > > > > > > Together, both the GPC and the clk-blk driver should be able to pull
> > > > > > > the multimedia block out of reset.  Currently, the GPC can handle the
> > > > > > > USB OTG and the GPU, but the LCDIF and MIPI DSI appear to be gated by
> > > > > > > the clock block
> > > > > > >
> > > > > > > My original patch RFC didn't include the imx8mn node, because it
> > > > > > > hangs, but the node I added looks like:
> > > > > > >
> > > > > > > media_blk_ctl: clock-controller@32e28000 {
> > > > > > >      compatible = "fsl,imx8mn-media-blk-ctl", "syscon";
> > > > > > >      reg = <0x32e28000 0x1000>;
> > > > > > >      #clock-cells = <1>;
> > > > > > >      #reset-cells = <1>;
> > > > > > > };
> > > > > > >
> > > > > > > I was hoping you might have some feedback on the 8mn clk-blk driver
> > > > > > > since you did the 8mp clk-blk drive and they appear to be very
> > > > > > > similar.
> > > > > > >
> > > > > >
> > > > > > I'll do you one better still. I'll apply the patch in my tree and give it
> > > > > > a test tomorrow morning.
> > > >
> > > > I do have some more updates on how to get the system to not hang, and
> > > > to enumerate more clocks.
> > > > Looking at Marek's work on enabling clocks in the 8MM, he added a
> > > > power-domain in dispmix_blk_ctl pointing to the dispmix in the GPC.
> > > > By forcing the GPC driver to write 0x1fff  to 32e28004, 0x7f to
> > > > 32e28000 and 0x30000 to 32e28008, the i.MX8MM can bring the display
> > > > clocks out of reset.
> > > >
> > >
> > > Yeah, that makes sense. Basically, it was trying to disable unused clocks
> > > (see clk_disable_unused) but in order to disable the clocks from the
> > > media BLK_CTL (which I think should be renamed in display BLK_CTL) the
> > > PD need to be on. Since you initially didn't give it any PD, it was trying
> > > to blindly write/read the gate bit and therefore freeze.
> > >
> > > > Unfortunately, the i.MX8MN needs to have 0x100 written to both
> > > > 32e28000 and 32e28004, and the values written for the 8MM are not
> > > > compatible.
> > > > By forcing the GPC to write those values, I can get  lcdif_pixel_clk
> > > > and the mipi_dsi_clkref  appearing on the Nano.
> > >
> > > I'm trying to make a branch with all the patches for all i.MX8M so I
> > > can keep track of it all. On this branch I've also applied the
> > > following patchset from Lucas Stach:
> > > https://www.spinics.net/lists/arm-kernel/msg843007.html
> > > but I'm getting the folowing errors:
> > >
> > > [   16.690885] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > > [   16.716839] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > > [   16.730500] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > >
> > > Lucas, any thoughts?
> > >
> > > Maybe it's something related to 8MN.
> > >
> > I will go back and double check this now that we have both the
> > blt_crl->power-domain and the power-domain->blk_ctl.
> >
> > > Will dig further, see what pops out.
> >
> > I wasn't sure which direction to go with the name.  I can rename the
> > media_blk_ctl  driver to display_blk_ctl.  I used Media based on the
> > imx8mp naming convention and the fact that it's controlling both the
> > display and the camera interface, however it's depending on the
> > dispmix GPC.
> >
> > I'll submit a RFC V2 with the cross referencing to the GPC based on
> > Marek's Mini patch set, but we'll still have an issue where the Mini
> > and Nano have different syscon values to enable the clocks, and
> > Marek's branch has it card-coded, so my patch would effectively break
> > the Mini in order to make the Nano operate until we find a better
> > solution.
>
> The GPC should not write into the BLK_CTL region via syscon, but
> instead use the clocks and resets as exposed by the BLK_CTL driver.
> Doing it via syscon is a hack to get things going. The clocks and
> resets should properly be hooked up to the GPC domains via the clocks
> and resets DT properties.
>
> For the clocks there is one complication: if the clocks are controlled
> via BLK_CTL we can only enable them once the domain is powered up,
> however the earlier designs using the GPCv2 assert resets as part of
> the power up sequence, which needs the clocks to be running for the
> reset to propagate. So depending on whether we have a power domain with
> a BLK_CTL or not we need to enable the clocks before or after powering
> up the domain. I guess we need a new DT property to specify which way
> the domain needs to handled.

So in the case of Nano, could we create two blocks instead of one?
The first block would enable the bus clock and reset that correspond
to writing 0x100 to avoid writing to syscon.  From there, we reference
that reset and clock from the GPC displaymix_pd to enable the access.
Once that's done, we point the 2nd block power-domain to the
dispmix_pd to unlock the remaining clocks.

Would that work?  I can try it later today, but I'm not near the hardware now.

adam

>
> Regards,
> Lucas
>

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

[Lucas Stach]

Am Montag, den 26.10.2020, 11:23 -0500 schrieb Adam Ford:
> On Mon, Oct 26, 2020 at 10:44 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> > Am Montag, den 26.10.2020, 10:12 -0500 schrieb Adam Ford:
> > > On Mon, Oct 26, 2020 at 9:55 AM Abel Vesa <abel.vesa@nxp.com> wrote:
> > > > On 20-10-25 11:05:32, Adam Ford wrote:
> > > > > On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut <marex@denx.de> wrote:
> > > > > > On 10/25/20 1:05 PM, Abel Vesa wrote:
> > > > > > 
> > > > > > [...]
> > > > > > 
> > > > > > > > Together, both the GPC and the clk-blk driver should be able to pull
> > > > > > > > the multimedia block out of reset.  Currently, the GPC can handle the
> > > > > > > > USB OTG and the GPU, but the LCDIF and MIPI DSI appear to be gated by
> > > > > > > > the clock block
> > > > > > > > 
> > > > > > > > My original patch RFC didn't include the imx8mn node, because it
> > > > > > > > hangs, but the node I added looks like:
> > > > > > > > 
> > > > > > > > media_blk_ctl: clock-controller@32e28000 {
> > > > > > > >      compatible = "fsl,imx8mn-media-blk-ctl", "syscon";
> > > > > > > >      reg = <0x32e28000 0x1000>;
> > > > > > > >      #clock-cells = <1>;
> > > > > > > >      #reset-cells = <1>;
> > > > > > > > };
> > > > > > > > 
> > > > > > > > I was hoping you might have some feedback on the 8mn clk-blk driver
> > > > > > > > since you did the 8mp clk-blk drive and they appear to be very
> > > > > > > > similar.
> > > > > > > > 
> > > > > > > 
> > > > > > > I'll do you one better still. I'll apply the patch in my tree and give it
> > > > > > > a test tomorrow morning.
> > > > > 
> > > > > I do have some more updates on how to get the system to not hang, and
> > > > > to enumerate more clocks.
> > > > > Looking at Marek's work on enabling clocks in the 8MM, he added a
> > > > > power-domain in dispmix_blk_ctl pointing to the dispmix in the GPC.
> > > > > By forcing the GPC driver to write 0x1fff  to 32e28004, 0x7f to
> > > > > 32e28000 and 0x30000 to 32e28008, the i.MX8MM can bring the display
> > > > > clocks out of reset.
> > > > > 
> > > > 
> > > > Yeah, that makes sense. Basically, it was trying to disable unused clocks
> > > > (see clk_disable_unused) but in order to disable the clocks from the
> > > > media BLK_CTL (which I think should be renamed in display BLK_CTL) the
> > > > PD need to be on. Since you initially didn't give it any PD, it was trying
> > > > to blindly write/read the gate bit and therefore freeze.
> > > > 
> > > > > Unfortunately, the i.MX8MN needs to have 0x100 written to both
> > > > > 32e28000 and 32e28004, and the values written for the 8MM are not
> > > > > compatible.
> > > > > By forcing the GPC to write those values, I can get  lcdif_pixel_clk
> > > > > and the mipi_dsi_clkref  appearing on the Nano.
> > > > 
> > > > I'm trying to make a branch with all the patches for all i.MX8M so I
> > > > can keep track of it all. On this branch I've also applied the
> > > > following patchset from Lucas Stach:
> > > > https://www.spinics.net/lists/arm-kernel/msg843007.html
> > > > but I'm getting the folowing errors:
> > > > 
> > > > [   16.690885] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > > > [   16.716839] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > > > [   16.730500] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > > > 
> > > > Lucas, any thoughts?
> > > > 
> > > > Maybe it's something related to 8MN.
> > > > 
> > > I will go back and double check this now that we have both the
> > > blt_crl->power-domain and the power-domain->blk_ctl.
> > > 
> > > > Will dig further, see what pops out.
> > > 
> > > I wasn't sure which direction to go with the name.  I can rename the
> > > media_blk_ctl  driver to display_blk_ctl.  I used Media based on the
> > > imx8mp naming convention and the fact that it's controlling both the
> > > display and the camera interface, however it's depending on the
> > > dispmix GPC.
> > > 
> > > I'll submit a RFC V2 with the cross referencing to the GPC based on
> > > Marek's Mini patch set, but we'll still have an issue where the Mini
> > > and Nano have different syscon values to enable the clocks, and
> > > Marek's branch has it card-coded, so my patch would effectively break
> > > the Mini in order to make the Nano operate until we find a better
> > > solution.
> > 
> > The GPC should not write into the BLK_CTL region via syscon, but
> > instead use the clocks and resets as exposed by the BLK_CTL driver.
> > Doing it via syscon is a hack to get things going. The clocks and
> > resets should properly be hooked up to the GPC domains via the clocks
> > and resets DT properties.
> > 
> > For the clocks there is one complication: if the clocks are controlled
> > via BLK_CTL we can only enable them once the domain is powered up,
> > however the earlier designs using the GPCv2 assert resets as part of
> > the power up sequence, which needs the clocks to be running for the
> > reset to propagate. So depending on whether we have a power domain with
> > a BLK_CTL or not we need to enable the clocks before or after powering
> > up the domain. I guess we need a new DT property to specify which way
> > the domain needs to handled.
> 
> So in the case of Nano, could we create two blocks instead of one?
> The first block would enable the bus clock and reset that correspond
> to writing 0x100 to avoid writing to syscon.  From there, we reference
> that reset and clock from the GPC displaymix_pd to enable the access.
> Once that's done, we point the 2nd block power-domain to the
> dispmix_pd to unlock the remaining clocks.
> 
> Would that work?  I can try it later today, but I'm not near the hardware now.

Splitting the PD into 2 staged domains might actually work well to get
around the cyclic dependency between GPC and BLK_CTL. It's not totally
to my liking, as the DT description doesn't map 1:1 to hardware
anymore, but it seems to be the most elegant solution to get around the
dependency.

I'll try to implement this on the i.MX8MM today or tomorrow to see if
it holds up in reality or if there are some hidden warts.

Regards,
Lucas

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

[Abel Vesa]

On 20-10-29 12:54:58, Lucas Stach wrote:
> Am Montag, den 26.10.2020, 11:23 -0500 schrieb Adam Ford:
> > On Mon, Oct 26, 2020 at 10:44 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> > > Am Montag, den 26.10.2020, 10:12 -0500 schrieb Adam Ford:
> > > > On Mon, Oct 26, 2020 at 9:55 AM Abel Vesa <abel.vesa@nxp.com> wrote:
> > > > > On 20-10-25 11:05:32, Adam Ford wrote:
> > > > > > On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut <marex@denx.de> wrote:
> > > > > > > On 10/25/20 1:05 PM, Abel Vesa wrote:
> > > > > > > 
> > > > > > > [...]
> > > > > > > 
> > > > > > > > > Together, both the GPC and the clk-blk driver should be able to pull
> > > > > > > > > the multimedia block out of reset.  Currently, the GPC can handle the
> > > > > > > > > USB OTG and the GPU, but the LCDIF and MIPI DSI appear to be gated by
> > > > > > > > > the clock block
> > > > > > > > > 
> > > > > > > > > My original patch RFC didn't include the imx8mn node, because it
> > > > > > > > > hangs, but the node I added looks like:
> > > > > > > > > 
> > > > > > > > > media_blk_ctl: clock-controller@32e28000 {
> > > > > > > > >      compatible = "fsl,imx8mn-media-blk-ctl", "syscon";
> > > > > > > > >      reg = <0x32e28000 0x1000>;
> > > > > > > > >      #clock-cells = <1>;
> > > > > > > > >      #reset-cells = <1>;
> > > > > > > > > };
> > > > > > > > > 
> > > > > > > > > I was hoping you might have some feedback on the 8mn clk-blk driver
> > > > > > > > > since you did the 8mp clk-blk drive and they appear to be very
> > > > > > > > > similar.
> > > > > > > > > 
> > > > > > > > 
> > > > > > > > I'll do you one better still. I'll apply the patch in my tree and give it
> > > > > > > > a test tomorrow morning.
> > > > > > 
> > > > > > I do have some more updates on how to get the system to not hang, and
> > > > > > to enumerate more clocks.
> > > > > > Looking at Marek's work on enabling clocks in the 8MM, he added a
> > > > > > power-domain in dispmix_blk_ctl pointing to the dispmix in the GPC.
> > > > > > By forcing the GPC driver to write 0x1fff  to 32e28004, 0x7f to
> > > > > > 32e28000 and 0x30000 to 32e28008, the i.MX8MM can bring the display
> > > > > > clocks out of reset.
> > > > > > 
> > > > > 
> > > > > Yeah, that makes sense. Basically, it was trying to disable unused clocks
> > > > > (see clk_disable_unused) but in order to disable the clocks from the
> > > > > media BLK_CTL (which I think should be renamed in display BLK_CTL) the
> > > > > PD need to be on. Since you initially didn't give it any PD, it was trying
> > > > > to blindly write/read the gate bit and therefore freeze.
> > > > > 
> > > > > > Unfortunately, the i.MX8MN needs to have 0x100 written to both
> > > > > > 32e28000 and 32e28004, and the values written for the 8MM are not
> > > > > > compatible.
> > > > > > By forcing the GPC to write those values, I can get  lcdif_pixel_clk
> > > > > > and the mipi_dsi_clkref  appearing on the Nano.
> > > > > 
> > > > > I'm trying to make a branch with all the patches for all i.MX8M so I
> > > > > can keep track of it all. On this branch I've also applied the
> > > > > following patchset from Lucas Stach:
> > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Farm-kernel%2Fmsg843007.html&amp;data=04%7C01%7Cabel.vesa%40nxp.com%7C7ee028d464cf451e0e2d08d87c0177cd%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637395693031971288%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=yKGtS%2FTyKjGHZ2xcXSI8%2F74R9vUjPmXgT9FSQfUfZB4%3D&amp;reserved=0
> > > > > but I'm getting the folowing errors:
> > > > > 
> > > > > [   16.690885] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > > > > [   16.716839] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > > > > [   16.730500] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > > > > 
> > > > > Lucas, any thoughts?
> > > > > 
> > > > > Maybe it's something related to 8MN.
> > > > > 
> > > > I will go back and double check this now that we have both the
> > > > blt_crl->power-domain and the power-domain->blk_ctl.
> > > > 
> > > > > Will dig further, see what pops out.
> > > > 
> > > > I wasn't sure which direction to go with the name.  I can rename the
> > > > media_blk_ctl  driver to display_blk_ctl.  I used Media based on the
> > > > imx8mp naming convention and the fact that it's controlling both the
> > > > display and the camera interface, however it's depending on the
> > > > dispmix GPC.
> > > > 
> > > > I'll submit a RFC V2 with the cross referencing to the GPC based on
> > > > Marek's Mini patch set, but we'll still have an issue where the Mini
> > > > and Nano have different syscon values to enable the clocks, and
> > > > Marek's branch has it card-coded, so my patch would effectively break
> > > > the Mini in order to make the Nano operate until we find a better
> > > > solution.
> > > 
> > > The GPC should not write into the BLK_CTL region via syscon, but
> > > instead use the clocks and resets as exposed by the BLK_CTL driver.
> > > Doing it via syscon is a hack to get things going. The clocks and
> > > resets should properly be hooked up to the GPC domains via the clocks
> > > and resets DT properties.
> > > 
> > > For the clocks there is one complication: if the clocks are controlled
> > > via BLK_CTL we can only enable them once the domain is powered up,
> > > however the earlier designs using the GPCv2 assert resets as part of
> > > the power up sequence, which needs the clocks to be running for the
> > > reset to propagate. So depending on whether we have a power domain with
> > > a BLK_CTL or not we need to enable the clocks before or after powering
> > > up the domain. I guess we need a new DT property to specify which way
> > > the domain needs to handled.
> > 
> > So in the case of Nano, could we create two blocks instead of one?
> > The first block would enable the bus clock and reset that correspond
> > to writing 0x100 to avoid writing to syscon.  From there, we reference
> > that reset and clock from the GPC displaymix_pd to enable the access.
> > Once that's done, we point the 2nd block power-domain to the
> > dispmix_pd to unlock the remaining clocks.
> > 
> > Would that work?  I can try it later today, but I'm not near the hardware now.
> 
> Splitting the PD into 2 staged domains might actually work well to get
> around the cyclic dependency between GPC and BLK_CTL. It's not totally
> to my liking, as the DT description doesn't map 1:1 to hardware
> anymore, but it seems to be the most elegant solution to get around the
> dependency.
> 
> I'll try to implement this on the i.MX8MM today or tomorrow to see if
> it holds up in reality or if there are some hidden warts.
> 

I started looking into this yesterday, on 8MN. Splitting doesn't solve the issue.
I tried splitting into dispmix and dispmix_adb400. Added the bus_blk reset and
clock to the dispmix_adb400 PD. Trouble is, you can't add the dispmix_adb400 PD
to the blk_ctl because of the cyclic dependency.

> Regards,
> Lucas
> 

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

[Adam Ford]

On Thu, Oct 29, 2020 at 6:55 AM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Am Montag, den 26.10.2020, 11:23 -0500 schrieb Adam Ford:
> > On Mon, Oct 26, 2020 at 10:44 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> > > Am Montag, den 26.10.2020, 10:12 -0500 schrieb Adam Ford:
> > > > On Mon, Oct 26, 2020 at 9:55 AM Abel Vesa <abel.vesa@nxp.com> wrote:
> > > > > On 20-10-25 11:05:32, Adam Ford wrote:
> > > > > > On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut <marex@denx.de> wrote:
> > > > > > > On 10/25/20 1:05 PM, Abel Vesa wrote:
> > > > > > >
> > > > > > > [...]
> > > > > > >
> > > > > > > > > Together, both the GPC and the clk-blk driver should be able to pull
> > > > > > > > > the multimedia block out of reset.  Currently, the GPC can handle the
> > > > > > > > > USB OTG and the GPU, but the LCDIF and MIPI DSI appear to be gated by
> > > > > > > > > the clock block
> > > > > > > > >
> > > > > > > > > My original patch RFC didn't include the imx8mn node, because it
> > > > > > > > > hangs, but the node I added looks like:
> > > > > > > > >
> > > > > > > > > media_blk_ctl: clock-controller@32e28000 {
> > > > > > > > >      compatible = "fsl,imx8mn-media-blk-ctl", "syscon";
> > > > > > > > >      reg = <0x32e28000 0x1000>;
> > > > > > > > >      #clock-cells = <1>;
> > > > > > > > >      #reset-cells = <1>;
> > > > > > > > > };
> > > > > > > > >
> > > > > > > > > I was hoping you might have some feedback on the 8mn clk-blk driver
> > > > > > > > > since you did the 8mp clk-blk drive and they appear to be very
> > > > > > > > > similar.
> > > > > > > > >
> > > > > > > >
> > > > > > > > I'll do you one better still. I'll apply the patch in my tree and give it
> > > > > > > > a test tomorrow morning.
> > > > > >
> > > > > > I do have some more updates on how to get the system to not hang, and
> > > > > > to enumerate more clocks.
> > > > > > Looking at Marek's work on enabling clocks in the 8MM, he added a
> > > > > > power-domain in dispmix_blk_ctl pointing to the dispmix in the GPC.
> > > > > > By forcing the GPC driver to write 0x1fff  to 32e28004, 0x7f to
> > > > > > 32e28000 and 0x30000 to 32e28008, the i.MX8MM can bring the display
> > > > > > clocks out of reset.
> > > > > >
> > > > >
> > > > > Yeah, that makes sense. Basically, it was trying to disable unused clocks
> > > > > (see clk_disable_unused) but in order to disable the clocks from the
> > > > > media BLK_CTL (which I think should be renamed in display BLK_CTL) the
> > > > > PD need to be on. Since you initially didn't give it any PD, it was trying
> > > > > to blindly write/read the gate bit and therefore freeze.
> > > > >
> > > > > > Unfortunately, the i.MX8MN needs to have 0x100 written to both
> > > > > > 32e28000 and 32e28004, and the values written for the 8MM are not
> > > > > > compatible.
> > > > > > By forcing the GPC to write those values, I can get  lcdif_pixel_clk
> > > > > > and the mipi_dsi_clkref  appearing on the Nano.
> > > > >
> > > > > I'm trying to make a branch with all the patches for all i.MX8M so I
> > > > > can keep track of it all. On this branch I've also applied the
> > > > > following patchset from Lucas Stach:
> > > > > https://www.spinics.net/lists/arm-kernel/msg843007.html
> > > > > but I'm getting the folowing errors:
> > > > >
> > > > > [   16.690885] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > > > > [   16.716839] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > > > > [   16.730500] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > > > >
> > > > > Lucas, any thoughts?
> > > > >
> > > > > Maybe it's something related to 8MN.
> > > > >
> > > > I will go back and double check this now that we have both the
> > > > blt_crl->power-domain and the power-domain->blk_ctl.
> > > >
> > > > > Will dig further, see what pops out.
> > > >
> > > > I wasn't sure which direction to go with the name.  I can rename the
> > > > media_blk_ctl  driver to display_blk_ctl.  I used Media based on the
> > > > imx8mp naming convention and the fact that it's controlling both the
> > > > display and the camera interface, however it's depending on the
> > > > dispmix GPC.
> > > >
> > > > I'll submit a RFC V2 with the cross referencing to the GPC based on
> > > > Marek's Mini patch set, but we'll still have an issue where the Mini
> > > > and Nano have different syscon values to enable the clocks, and
> > > > Marek's branch has it card-coded, so my patch would effectively break
> > > > the Mini in order to make the Nano operate until we find a better
> > > > solution.
> > >
> > > The GPC should not write into the BLK_CTL region via syscon, but
> > > instead use the clocks and resets as exposed by the BLK_CTL driver.
> > > Doing it via syscon is a hack to get things going. The clocks and
> > > resets should properly be hooked up to the GPC domains via the clocks
> > > and resets DT properties.
> > >
> > > For the clocks there is one complication: if the clocks are controlled
> > > via BLK_CTL we can only enable them once the domain is powered up,
> > > however the earlier designs using the GPCv2 assert resets as part of
> > > the power up sequence, which needs the clocks to be running for the
> > > reset to propagate. So depending on whether we have a power domain with
> > > a BLK_CTL or not we need to enable the clocks before or after powering
> > > up the domain. I guess we need a new DT property to specify which way
> > > the domain needs to handled.
> >
> > So in the case of Nano, could we create two blocks instead of one?
> > The first block would enable the bus clock and reset that correspond
> > to writing 0x100 to avoid writing to syscon.  From there, we reference
> > that reset and clock from the GPC displaymix_pd to enable the access.
> > Once that's done, we point the 2nd block power-domain to the
> > dispmix_pd to unlock the remaining clocks.
> >
> > Would that work?  I can try it later today, but I'm not near the hardware now.
>
> Splitting the PD into 2 staged domains might actually work well to get
> around the cyclic dependency between GPC and BLK_CTL. It's not totally
> to my liking, as the DT description doesn't map 1:1 to hardware
> anymore, but it seems to be the most elegant solution to get around the
> dependency.
>
> I'll try to implement this on the i.MX8MM today or tomorrow to see if
> it holds up in reality or if there are some hidden warts.

I tried it last night on the Nano without success.  :-(

I was just getting ready to e-mail the group when I saw this come in.

adam

>
> Regards,
> Lucas
>

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

[Lucas Stach]

Am Donnerstag, den 29.10.2020, 07:18 -0500 schrieb Adam Ford:
> On Thu, Oct 29, 2020 at 6:55 AM Lucas Stach <l.stach@pengutronix.de>
> wrote:
> > Am Montag, den 26.10.2020, 11:23 -0500 schrieb Adam Ford:
> > > On Mon, Oct 26, 2020 at 10:44 AM Lucas Stach <
> > > l.stach@pengutronix.de> wrote:
> > > > Am Montag, den 26.10.2020, 10:12 -0500 schrieb Adam Ford:
> > > > > On Mon, Oct 26, 2020 at 9:55 AM Abel Vesa <abel.vesa@nxp.com>
> > > > > wrote:
> > > > > > On 20-10-25 11:05:32, Adam Ford wrote:
> > > > > > > On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut <
> > > > > > > marex@denx.de> wrote:
> > > > > > > > On 10/25/20 1:05 PM, Abel Vesa wrote:
> > > > > > > > 
> > > > > > > > [...]
> > > > > > > > 
> > > > > > > > > > Together, both the GPC and the clk-blk driver
> > > > > > > > > > should be able to pull
> > > > > > > > > > the multimedia block out of reset.  Currently, the
> > > > > > > > > > GPC can handle the
> > > > > > > > > > USB OTG and the GPU, but the LCDIF and MIPI DSI
> > > > > > > > > > appear to be gated by
> > > > > > > > > > the clock block
> > > > > > > > > > 
> > > > > > > > > > My original patch RFC didn't include the imx8mn
> > > > > > > > > > node, because it
> > > > > > > > > > hangs, but the node I added looks like:
> > > > > > > > > > 
> > > > > > > > > > media_blk_ctl: clock-controller@32e28000 {
> > > > > > > > > >      compatible = "fsl,imx8mn-media-blk-ctl",
> > > > > > > > > > "syscon";
> > > > > > > > > >      reg = <0x32e28000 0x1000>;
> > > > > > > > > >      #clock-cells = <1>;
> > > > > > > > > >      #reset-cells = <1>;
> > > > > > > > > > };
> > > > > > > > > > 
> > > > > > > > > > I was hoping you might have some feedback on the
> > > > > > > > > > 8mn clk-blk driver
> > > > > > > > > > since you did the 8mp clk-blk drive and they appear
> > > > > > > > > > to be very
> > > > > > > > > > similar.
> > > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > I'll do you one better still. I'll apply the patch in
> > > > > > > > > my tree and give it
> > > > > > > > > a test tomorrow morning.
> > > > > > > 
> > > > > > > I do have some more updates on how to get the system to
> > > > > > > not hang, and
> > > > > > > to enumerate more clocks.
> > > > > > > Looking at Marek's work on enabling clocks in the 8MM, he
> > > > > > > added a
> > > > > > > power-domain in dispmix_blk_ctl pointing to the dispmix
> > > > > > > in the GPC.
> > > > > > > By forcing the GPC driver to write 0x1fff  to 32e28004,
> > > > > > > 0x7f to
> > > > > > > 32e28000 and 0x30000 to 32e28008, the i.MX8MM can bring
> > > > > > > the display
> > > > > > > clocks out of reset.
> > > > > > > 
> > > > > > 
> > > > > > Yeah, that makes sense. Basically, it was trying to disable
> > > > > > unused clocks
> > > > > > (see clk_disable_unused) but in order to disable the clocks
> > > > > > from the
> > > > > > media BLK_CTL (which I think should be renamed in display
> > > > > > BLK_CTL) the
> > > > > > PD need to be on. Since you initially didn't give it any
> > > > > > PD, it was trying
> > > > > > to blindly write/read the gate bit and therefore freeze.
> > > > > > 
> > > > > > > Unfortunately, the i.MX8MN needs to have 0x100 written to
> > > > > > > both
> > > > > > > 32e28000 and 32e28004, and the values written for the 8MM
> > > > > > > are not
> > > > > > > compatible.
> > > > > > > By forcing the GPC to write those values, I can
> > > > > > > get  lcdif_pixel_clk
> > > > > > > and the mipi_dsi_clkref  appearing on the Nano.
> > > > > > 
> > > > > > I'm trying to make a branch with all the patches for all
> > > > > > i.MX8M so I
> > > > > > can keep track of it all. On this branch I've also applied
> > > > > > the
> > > > > > following patchset from Lucas Stach:
> > > > > > https://www.spinics.net/lists/arm-kernel/msg843007.html
> > > > > > but I'm getting the folowing errors:
> > > > > > 
> > > > > > [   16.690885] imx-pgc imx-pgc-domain.3: failed to power up
> > > > > > ADB400
> > > > > > [   16.716839] imx-pgc imx-pgc-domain.3: failed to power up
> > > > > > ADB400
> > > > > > [   16.730500] imx-pgc imx-pgc-domain.3: failed to power up
> > > > > > ADB400
> > > > > > 
> > > > > > Lucas, any thoughts?
> > > > > > 
> > > > > > Maybe it's something related to 8MN.
> > > > > > 
> > > > > I will go back and double check this now that we have both
> > > > > the
> > > > > blt_crl->power-domain and the power-domain->blk_ctl.
> > > > > 
> > > > > > Will dig further, see what pops out.
> > > > > 
> > > > > I wasn't sure which direction to go with the name.  I can
> > > > > rename the
> > > > > media_blk_ctl  driver to display_blk_ctl.  I used Media based
> > > > > on the
> > > > > imx8mp naming convention and the fact that it's controlling
> > > > > both the
> > > > > display and the camera interface, however it's depending on
> > > > > the
> > > > > dispmix GPC.
> > > > > 
> > > > > I'll submit a RFC V2 with the cross referencing to the GPC
> > > > > based on
> > > > > Marek's Mini patch set, but we'll still have an issue where
> > > > > the Mini
> > > > > and Nano have different syscon values to enable the clocks,
> > > > > and
> > > > > Marek's branch has it card-coded, so my patch would
> > > > > effectively break
> > > > > the Mini in order to make the Nano operate until we find a
> > > > > better
> > > > > solution.
> > > > 
> > > > The GPC should not write into the BLK_CTL region via syscon,
> > > > but
> > > > instead use the clocks and resets as exposed by the BLK_CTL
> > > > driver.
> > > > Doing it via syscon is a hack to get things going. The clocks
> > > > and
> > > > resets should properly be hooked up to the GPC domains via the
> > > > clocks
> > > > and resets DT properties.
> > > > 
> > > > For the clocks there is one complication: if the clocks are
> > > > controlled
> > > > via BLK_CTL we can only enable them once the domain is powered
> > > > up,
> > > > however the earlier designs using the GPCv2 assert resets as
> > > > part of
> > > > the power up sequence, which needs the clocks to be running for
> > > > the
> > > > reset to propagate. So depending on whether we have a power
> > > > domain with
> > > > a BLK_CTL or not we need to enable the clocks before or after
> > > > powering
> > > > up the domain. I guess we need a new DT property to specify
> > > > which way
> > > > the domain needs to handled.
> > > 
> > > So in the case of Nano, could we create two blocks instead of
> > > one?
> > > The first block would enable the bus clock and reset that
> > > correspond
> > > to writing 0x100 to avoid writing to syscon.  From there, we
> > > reference
> > > that reset and clock from the GPC displaymix_pd to enable the
> > > access.
> > > Once that's done, we point the 2nd block power-domain to the
> > > dispmix_pd to unlock the remaining clocks.
> > > 
> > > Would that work?  I can try it later today, but I'm not near the
> > > hardware now.
> > 
> > Splitting the PD into 2 staged domains might actually work well to
> > get
> > around the cyclic dependency between GPC and BLK_CTL. It's not
> > totally
> > to my liking, as the DT description doesn't map 1:1 to hardware
> > anymore, but it seems to be the most elegant solution to get around
> > the
> > dependency.
> > 
> > I'll try to implement this on the i.MX8MM today or tomorrow to see
> > if
> > it holds up in reality or if there are some hidden warts.
> 
> I tried it last night on the Nano without success.  :-(
> 
> I was just getting ready to e-mail the group when I saw this come in.

I was thinking the other way around, keeping a single BLK_CTL, but
splitting the PD to reflect the power-up sequence requirement. I'll let
you know how this works out.

Regards,
Lucas

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

[Abel Vesa]

On 20-10-26 16:44:13, Lucas Stach wrote:
> Am Montag, den 26.10.2020, 10:12 -0500 schrieb Adam Ford:
> > On Mon, Oct 26, 2020 at 9:55 AM Abel Vesa <abel.vesa@nxp.com> wrote:
> > > On 20-10-25 11:05:32, Adam Ford wrote:
> > > > On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut <marex@denx.de> wrote:
> > > > > On 10/25/20 1:05 PM, Abel Vesa wrote:
> > > > > 
> > > > > [...]
> > > > > 
> > > > > > > Together, both the GPC and the clk-blk driver should be able to pull
> > > > > > > the multimedia block out of reset.  Currently, the GPC can handle the
> > > > > > > USB OTG and the GPU, but the LCDIF and MIPI DSI appear to be gated by
> > > > > > > the clock block
> > > > > > > 
> > > > > > > My original patch RFC didn't include the imx8mn node, because it
> > > > > > > hangs, but the node I added looks like:
> > > > > > > 
> > > > > > > media_blk_ctl: clock-controller@32e28000 {
> > > > > > >      compatible = "fsl,imx8mn-media-blk-ctl", "syscon";
> > > > > > >      reg = <0x32e28000 0x1000>;
> > > > > > >      #clock-cells = <1>;
> > > > > > >      #reset-cells = <1>;
> > > > > > > };
> > > > > > > 
> > > > > > > I was hoping you might have some feedback on the 8mn clk-blk driver
> > > > > > > since you did the 8mp clk-blk drive and they appear to be very
> > > > > > > similar.
> > > > > > > 
> > > > > > 
> > > > > > I'll do you one better still. I'll apply the patch in my tree and give it
> > > > > > a test tomorrow morning.
> > > > 
> > > > I do have some more updates on how to get the system to not hang, and
> > > > to enumerate more clocks.
> > > > Looking at Marek's work on enabling clocks in the 8MM, he added a
> > > > power-domain in dispmix_blk_ctl pointing to the dispmix in the GPC.
> > > > By forcing the GPC driver to write 0x1fff  to 32e28004, 0x7f to
> > > > 32e28000 and 0x30000 to 32e28008, the i.MX8MM can bring the display
> > > > clocks out of reset.
> > > > 
> > > 
> > > Yeah, that makes sense. Basically, it was trying to disable unused clocks
> > > (see clk_disable_unused) but in order to disable the clocks from the
> > > media BLK_CTL (which I think should be renamed in display BLK_CTL) the
> > > PD need to be on. Since you initially didn't give it any PD, it was trying
> > > to blindly write/read the gate bit and therefore freeze.
> > > 
> > > > Unfortunately, the i.MX8MN needs to have 0x100 written to both
> > > > 32e28000 and 32e28004, and the values written for the 8MM are not
> > > > compatible.
> > > > By forcing the GPC to write those values, I can get  lcdif_pixel_clk
> > > > and the mipi_dsi_clkref  appearing on the Nano.
> > > 
> > > I'm trying to make a branch with all the patches for all i.MX8M so I
> > > can keep track of it all. On this branch I've also applied the
> > > following patchset from Lucas Stach:
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Farm-kernel%2Fmsg843007.html&amp;data=04%7C01%7Cabel.vesa%40nxp.com%7C02bcfb84d35f4c41a05e08d879c5fe57%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637393238611075979%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=fQnrRpGOnk0B4tFCFsfadKK2qozZwxK4ddycmv4VPls%3D&amp;reserved=0
> > > but I'm getting the folowing errors:
> > > 
> > > [   16.690885] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > > [   16.716839] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > > [   16.730500] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > > 
> > > Lucas, any thoughts?
> > > 
> > > Maybe it's something related to 8MN.
> > > 
> > I will go back and double check this now that we have both the
> > blt_crl->power-domain and the power-domain->blk_ctl.
> > 
> > > Will dig further, see what pops out.
> > 
> > I wasn't sure which direction to go with the name.  I can rename the
> > media_blk_ctl  driver to display_blk_ctl.  I used Media based on the
> > imx8mp naming convention and the fact that it's controlling both the
> > display and the camera interface, however it's depending on the
> > dispmix GPC.
> > 
> > I'll submit a RFC V2 with the cross referencing to the GPC based on
> > Marek's Mini patch set, but we'll still have an issue where the Mini
> > and Nano have different syscon values to enable the clocks, and
> > Marek's branch has it card-coded, so my patch would effectively break
> > the Mini in order to make the Nano operate until we find a better
> > solution.
> 
> The GPC should not write into the BLK_CTL region via syscon, but
> instead use the clocks and resets as exposed by the BLK_CTL driver.
> Doing it via syscon is a hack to get things going. The clocks and
> resets should properly be hooked up to the GPC domains via the clocks
> and resets DT properties.
> 

Totally agree. The syscon is there for other GPRs of any BLK_CTL which
do not fit in a clock or reset controller. So please do not ever use the
syscon for touching clocks or reset. Actually, I'm thinking of a way to
make sure the use of syscon for clocks and resets in the BLK_CTL would
be protected against.

> For the clocks there is one complication: if the clocks are controlled
> via BLK_CTL we can only enable them once the domain is powered up,
> however the earlier designs using the GPCv2 assert resets as part of
> the power up sequence, which needs the clocks to be running for the
> reset to propagate. So depending on whether we have a power domain with
> a BLK_CTL or not we need to enable the clocks before or after powering
> up the domain. I guess we need a new DT property to specify which way
> the domain needs to handled.
> 

All the BLK_CTLs have their own power domains. No question there. Now if
there are resets anc clocks that are part of the BLK_CTL and need to be
asserted/enabled on runtime resume, I think we can implement a generic
way to do that in the pm_runtime callbacks. Something similar to the 
.pm_runtime_saved_regs, maybe.

> Regards,
> Lucas
> 

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

[Lucas Stach]

Am Montag, den 26.10.2020, 16:55 +0200 schrieb Abel Vesa:
> On 20-10-25 11:05:32, Adam Ford wrote:
> > On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut <marex@denx.de> wrote:
> > > On 10/25/20 1:05 PM, Abel Vesa wrote:
> > > 
> > > [...]
> > > 
> > > > > Together, both the GPC and the clk-blk driver should be able to pull
> > > > > the multimedia block out of reset.  Currently, the GPC can handle the
> > > > > USB OTG and the GPU, but the LCDIF and MIPI DSI appear to be gated by
> > > > > the clock block
> > > > > 
> > > > > My original patch RFC didn't include the imx8mn node, because it
> > > > > hangs, but the node I added looks like:
> > > > > 
> > > > > media_blk_ctl: clock-controller@32e28000 {
> > > > >      compatible = "fsl,imx8mn-media-blk-ctl", "syscon";
> > > > >      reg = <0x32e28000 0x1000>;
> > > > >      #clock-cells = <1>;
> > > > >      #reset-cells = <1>;
> > > > > };
> > > > > 
> > > > > I was hoping you might have some feedback on the 8mn clk-blk driver
> > > > > since you did the 8mp clk-blk drive and they appear to be very
> > > > > similar.
> > > > > 
> > > > 
> > > > I'll do you one better still. I'll apply the patch in my tree and give it
> > > > a test tomorrow morning.
> > 
> > I do have some more updates on how to get the system to not hang, and
> > to enumerate more clocks.
> > Looking at Marek's work on enabling clocks in the 8MM, he added a
> > power-domain in dispmix_blk_ctl pointing to the dispmix in the GPC.
> > By forcing the GPC driver to write 0x1fff  to 32e28004, 0x7f to
> > 32e28000 and 0x30000 to 32e28008, the i.MX8MM can bring the display
> > clocks out of reset.
> > 
> 
> Yeah, that makes sense. Basically, it was trying to disable unused clocks
> (see clk_disable_unused) but in order to disable the clocks from the
> media BLK_CTL (which I think should be renamed in display BLK_CTL) the
> PD need to be on. Since you initially didn't give it any PD, it was trying
> to blindly write/read the gate bit and therefore freeze.
> 
> > Unfortunately, the i.MX8MN needs to have 0x100 written to both
> > 32e28000 and 32e28004, and the values written for the 8MM are not
> > compatible.
> > By forcing the GPC to write those values, I can get  lcdif_pixel_clk
> > and the mipi_dsi_clkref  appearing on the Nano.
> 
> I'm trying to make a branch with all the patches for all i.MX8M so I
> can keep track of it all. On this branch I've also applied the 
> following patchset from Lucas Stach:
> https://www.spinics.net/lists/arm-kernel/msg843007.html
> but I'm getting the folowing errors:
> 
> [   16.690885] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> [   16.716839] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> [   16.730500] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> 
> Lucas, any thoughts?
> 
> Maybe it's something related to 8MN.

The ADB is apparently clocked by one of the BLK_CTL clocks, so the ADB
handshake ack will only work when the BLK_CTL clocks are enabled. So I
guess the GPC driver should enable those clocks and assert the resets
at the right time in the power-up sequencing. Unfortunately this means
we can't properly put the BLK_CTL driver in the power-domain without
having a cyclic dependency in the DT. I'm still thinking about how to
solve this properly.

Regards,
Lucas

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

[Abel Vesa]

On 20-10-26 16:37:51, Lucas Stach wrote:
> Am Montag, den 26.10.2020, 16:55 +0200 schrieb Abel Vesa:
> > On 20-10-25 11:05:32, Adam Ford wrote:
> > > On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut <marex@denx.de> wrote:
> > > > On 10/25/20 1:05 PM, Abel Vesa wrote:
> > > > 
> > > > [...]
> > > > 
> > > > > > Together, both the GPC and the clk-blk driver should be able to pull
> > > > > > the multimedia block out of reset.  Currently, the GPC can handle the
> > > > > > USB OTG and the GPU, but the LCDIF and MIPI DSI appear to be gated by
> > > > > > the clock block
> > > > > > 
> > > > > > My original patch RFC didn't include the imx8mn node, because it
> > > > > > hangs, but the node I added looks like:
> > > > > > 
> > > > > > media_blk_ctl: clock-controller@32e28000 {
> > > > > >      compatible = "fsl,imx8mn-media-blk-ctl", "syscon";
> > > > > >      reg = <0x32e28000 0x1000>;
> > > > > >      #clock-cells = <1>;
> > > > > >      #reset-cells = <1>;
> > > > > > };
> > > > > > 
> > > > > > I was hoping you might have some feedback on the 8mn clk-blk driver
> > > > > > since you did the 8mp clk-blk drive and they appear to be very
> > > > > > similar.
> > > > > > 
> > > > > 
> > > > > I'll do you one better still. I'll apply the patch in my tree and give it
> > > > > a test tomorrow morning.
> > > 
> > > I do have some more updates on how to get the system to not hang, and
> > > to enumerate more clocks.
> > > Looking at Marek's work on enabling clocks in the 8MM, he added a
> > > power-domain in dispmix_blk_ctl pointing to the dispmix in the GPC.
> > > By forcing the GPC driver to write 0x1fff  to 32e28004, 0x7f to
> > > 32e28000 and 0x30000 to 32e28008, the i.MX8MM can bring the display
> > > clocks out of reset.
> > > 
> > 
> > Yeah, that makes sense. Basically, it was trying to disable unused clocks
> > (see clk_disable_unused) but in order to disable the clocks from the
> > media BLK_CTL (which I think should be renamed in display BLK_CTL) the
> > PD need to be on. Since you initially didn't give it any PD, it was trying
> > to blindly write/read the gate bit and therefore freeze.
> > 
> > > Unfortunately, the i.MX8MN needs to have 0x100 written to both
> > > 32e28000 and 32e28004, and the values written for the 8MM are not
> > > compatible.
> > > By forcing the GPC to write those values, I can get  lcdif_pixel_clk
> > > and the mipi_dsi_clkref  appearing on the Nano.
> > 
> > I'm trying to make a branch with all the patches for all i.MX8M so I
> > can keep track of it all. On this branch I've also applied the 
> > following patchset from Lucas Stach:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Farm-kernel%2Fmsg843007.html&amp;data=04%7C01%7Cabel.vesa%40nxp.com%7Cc930b0f523c44946a93f08d879c51c3f%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637393234789815116%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=ZzzUpEiEVcWqQQ5azAx8dkjHgiSMwkK04tqi32uLKbU%3D&amp;reserved=0
> > but I'm getting the folowing errors:
> > 
> > [   16.690885] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > [   16.716839] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > [   16.730500] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > 
> > Lucas, any thoughts?
> > 
> > Maybe it's something related to 8MN.
> 
> The ADB is apparently clocked by one of the BLK_CTL clocks, so the ADB
> handshake ack will only work when the BLK_CTL clocks are enabled. So I
> guess the GPC driver should enable those clocks and assert the resets
> at the right time in the power-up sequencing. Unfortunately this means
> we can't properly put the BLK_CTL driver in the power-domain without
> having a cyclic dependency in the DT. I'm still thinking about how to
> solve this properly.
> 

I remember we had something similar in our internal tree with the
bus_blk_clk on 8MP, which was added by the media BLK_CTL. What I did was to
just drop the registration of that clock entirely. My rationale was that if
the clock is part of the BLK_CTL but also needed by the BLK_CTL to work,
I can leave it alone (that is, enabled by default) since when the PD will be
powered off the clock will gated too. I guess another option would be to 
mark it as critical, that way, it will never be disabled (will be left alone
by the clk_disable_unused too) but at the same time will be visible in the
clock hierarchy.

> Regards,
> Lucas
> 

Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

[Abel Vesa]

On 20-10-27 11:31:10, Abel Vesa wrote:
> On 20-10-26 16:37:51, Lucas Stach wrote:
> > Am Montag, den 26.10.2020, 16:55 +0200 schrieb Abel Vesa:
> > > On 20-10-25 11:05:32, Adam Ford wrote:
> > > > On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut <marex@denx.de> wrote:
> > > > > On 10/25/20 1:05 PM, Abel Vesa wrote:
> > > > > 
> > > > > [...]
> > > > > 
> > > > > > > Together, both the GPC and the clk-blk driver should be able to pull
> > > > > > > the multimedia block out of reset.  Currently, the GPC can handle the
> > > > > > > USB OTG and the GPU, but the LCDIF and MIPI DSI appear to be gated by
> > > > > > > the clock block
> > > > > > > 
> > > > > > > My original patch RFC didn't include the imx8mn node, because it
> > > > > > > hangs, but the node I added looks like:
> > > > > > > 
> > > > > > > media_blk_ctl: clock-controller@32e28000 {
> > > > > > >      compatible = "fsl,imx8mn-media-blk-ctl", "syscon";
> > > > > > >      reg = <0x32e28000 0x1000>;
> > > > > > >      #clock-cells = <1>;
> > > > > > >      #reset-cells = <1>;
> > > > > > > };
> > > > > > > 
> > > > > > > I was hoping you might have some feedback on the 8mn clk-blk driver
> > > > > > > since you did the 8mp clk-blk drive and they appear to be very
> > > > > > > similar.
> > > > > > > 
> > > > > > 
> > > > > > I'll do you one better still. I'll apply the patch in my tree and give it
> > > > > > a test tomorrow morning.
> > > > 
> > > > I do have some more updates on how to get the system to not hang, and
> > > > to enumerate more clocks.
> > > > Looking at Marek's work on enabling clocks in the 8MM, he added a
> > > > power-domain in dispmix_blk_ctl pointing to the dispmix in the GPC.
> > > > By forcing the GPC driver to write 0x1fff  to 32e28004, 0x7f to
> > > > 32e28000 and 0x30000 to 32e28008, the i.MX8MM can bring the display
> > > > clocks out of reset.
> > > > 
> > > 
> > > Yeah, that makes sense. Basically, it was trying to disable unused clocks
> > > (see clk_disable_unused) but in order to disable the clocks from the
> > > media BLK_CTL (which I think should be renamed in display BLK_CTL) the
> > > PD need to be on. Since you initially didn't give it any PD, it was trying
> > > to blindly write/read the gate bit and therefore freeze.
> > > 
> > > > Unfortunately, the i.MX8MN needs to have 0x100 written to both
> > > > 32e28000 and 32e28004, and the values written for the 8MM are not
> > > > compatible.
> > > > By forcing the GPC to write those values, I can get  lcdif_pixel_clk
> > > > and the mipi_dsi_clkref  appearing on the Nano.
> > > 
> > > I'm trying to make a branch with all the patches for all i.MX8M so I
> > > can keep track of it all. On this branch I've also applied the 
> > > following patchset from Lucas Stach:
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Farm-kernel%2Fmsg843007.html&amp;data=04%7C01%7Cabel.vesa%40nxp.com%7C5ff46189143747fce45908d87a5b4281%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637393879674506099%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=ELDCbfLvxrB6FLwnsA6VyGlU5V3qpA2ImfPAbZnWyzI%3D&amp;reserved=0
> > > but I'm getting the folowing errors:
> > > 
> > > [   16.690885] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > > [   16.716839] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > > [   16.730500] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > > 
> > > Lucas, any thoughts?
> > > 
> > > Maybe it's something related to 8MN.
> > 
> > The ADB is apparently clocked by one of the BLK_CTL clocks, so the ADB
> > handshake ack will only work when the BLK_CTL clocks are enabled. So I
> > guess the GPC driver should enable those clocks and assert the resets
> > at the right time in the power-up sequencing. Unfortunately this means
> > we can't properly put the BLK_CTL driver in the power-domain without
> > having a cyclic dependency in the DT. I'm still thinking about how to
> > solve this properly.
> > 
> 
> I remember we had something similar in our internal tree with the
> bus_blk_clk on 8MP, which was added by the media BLK_CTL. What I did was to
> just drop the registration of that clock entirely. My rationale was that if
> the clock is part of the BLK_CTL but also needed by the BLK_CTL to work,
> I can leave it alone (that is, enabled by default) since when the PD will be
> powered off the clock will gated too. I guess another option would be to 
> mark it as critical, that way, it will never be disabled (will be left alone
> by the clk_disable_unused too) but at the same time will be visible in the
> clock hierarchy.
> 

Do ignore evrything I said about the bus_blk_ctl, that did work on our tree since
the whole PD power on/off "magic" is done in TF-A.

So the problem, as I understand it now, is the fact that the blk_ctl driver won't
probe because it needs its PD, but the PD is not registered because the ADB400
can't power up since it needs the bus_blk_ctl clock enabled, clock which is registered
by the blk_ctl. 

> > Regards,
> > Lucas
> > 

RE: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN

[Jacky Bai]

> -----Original Message-----
> From: Abel Vesa [mailto:abel.vesa@nxp.com]
> Sent: Tuesday, October 27, 2020 7:55 PM
> To: Lucas Stach <l.stach@pengutronix.de>
> Cc: Adam Ford <aford173@gmail.com>; Marek Vasut <marex@denx.de>;
> devicetree <devicetree@vger.kernel.org>; Sascha Hauer
> <s.hauer@pengutronix.de>; Philipp Zabel <p.zabel@pengutronix.de>;
> Stephen Boyd <sboyd@kernel.org>; Fabio Estevam <festevam@gmail.com>;
> Michael Turquette <mturquette@baylibre.com>; Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org>; Rob Herring <robh+dt@kernel.org>;
> dl-linux-imx <linux-imx@nxp.com>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Shawn Guo <shawnguo@kernel.org>; linux-clk
> <linux-clk@vger.kernel.org>; arm-soc <linux-arm-kernel@lists.infradead.org>
> Subject: Re: [RFC 0/3] clk: imx: Implement blk-ctl driver for i.MX8MN
> 
> On 20-10-27 11:31:10, Abel Vesa wrote:
> > On 20-10-26 16:37:51, Lucas Stach wrote:
> > > Am Montag, den 26.10.2020, 16:55 +0200 schrieb Abel Vesa:
> > > > On 20-10-25 11:05:32, Adam Ford wrote:
> > > > > On Sun, Oct 25, 2020 at 7:19 AM Marek Vasut <marex@denx.de>
> wrote:
> > > > > > On 10/25/20 1:05 PM, Abel Vesa wrote:
> > > > > >
> > > > > > [...]
> > > > > >
> > > > > > > > Together, both the GPC and the clk-blk driver should be
> > > > > > > > able to pull the multimedia block out of reset.
> > > > > > > > Currently, the GPC can handle the USB OTG and the GPU, but
> > > > > > > > the LCDIF and MIPI DSI appear to be gated by the clock
> > > > > > > > block
> > > > > > > >
> > > > > > > > My original patch RFC didn't include the imx8mn node,
> > > > > > > > because it hangs, but the node I added looks like:
> > > > > > > >
> > > > > > > > media_blk_ctl: clock-controller@32e28000 {
> > > > > > > >      compatible = "fsl,imx8mn-media-blk-ctl", "syscon";
> > > > > > > >      reg = <0x32e28000 0x1000>;
> > > > > > > >      #clock-cells = <1>;
> > > > > > > >      #reset-cells = <1>;
> > > > > > > > };
> > > > > > > >
> > > > > > > > I was hoping you might have some feedback on the 8mn
> > > > > > > > clk-blk driver since you did the 8mp clk-blk drive and
> > > > > > > > they appear to be very similar.
> > > > > > > >
> > > > > > >
> > > > > > > I'll do you one better still. I'll apply the patch in my
> > > > > > > tree and give it a test tomorrow morning.
> > > > >
> > > > > I do have some more updates on how to get the system to not
> > > > > hang, and to enumerate more clocks.
> > > > > Looking at Marek's work on enabling clocks in the 8MM, he added
> > > > > a power-domain in dispmix_blk_ctl pointing to the dispmix in the GPC.
> > > > > By forcing the GPC driver to write 0x1fff  to 32e28004, 0x7f to
> > > > > 32e28000 and 0x30000 to 32e28008, the i.MX8MM can bring the
> > > > > display clocks out of reset.
> > > > >
> > > >
> > > > Yeah, that makes sense. Basically, it was trying to disable unused
> > > > clocks (see clk_disable_unused) but in order to disable the clocks
> > > > from the media BLK_CTL (which I think should be renamed in display
> > > > BLK_CTL) the PD need to be on. Since you initially didn't give it
> > > > any PD, it was trying to blindly write/read the gate bit and therefore
> freeze.
> > > >
> > > > > Unfortunately, the i.MX8MN needs to have 0x100 written to both
> > > > > 32e28000 and 32e28004, and the values written for the 8MM are
> > > > > not compatible.
> > > > > By forcing the GPC to write those values, I can get
> > > > > lcdif_pixel_clk and the mipi_dsi_clkref  appearing on the Nano.
> > > >
> > > > I'm trying to make a branch with all the patches for all i.MX8M so
> > > > I can keep track of it all. On this branch I've also applied the
> > > > following patchset from Lucas Stach:
> > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > > >
> www.spinics.net%2Flists%2Farm-kernel%2Fmsg843007.html&amp;data=04%
> > > >
> 7C01%7Cping.bai%40nxp.com%7C0c54623294334a04a01208d87a6f3163%7
> C686
> > > >
> ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637393965282215014%7C
> Unkno
> > > >
> wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
> a
> > > >
> WwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=vFbBn94CPsShV72rOCtbTcz5u0
> qh7Au
> > > > o44Sb2%2BiBlrE%3D&amp;reserved=0 but I'm getting the folowing
> > > > errors:
> > > >
> > > > [   16.690885] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > > > [   16.716839] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > > > [   16.730500] imx-pgc imx-pgc-domain.3: failed to power up ADB400
> > > >
> > > > Lucas, any thoughts?
> > > >
> > > > Maybe it's something related to 8MN.
> > >
> > > The ADB is apparently clocked by one of the BLK_CTL clocks, so the
> > > ADB handshake ack will only work when the BLK_CTL clocks are
> > > enabled. So I guess the GPC driver should enable those clocks and
> > > assert the resets at the right time in the power-up sequencing.
> > > Unfortunately this means we can't properly put the BLK_CTL driver in
> > > the power-domain without having a cyclic dependency in the DT. I'm
> > > still thinking about how to solve this properly.
> > >
> >
> > I remember we had something similar in our internal tree with the
> > bus_blk_clk on 8MP, which was added by the media BLK_CTL. What I did
> > was to just drop the registration of that clock entirely. My rationale
> > was that if the clock is part of the BLK_CTL but also needed by the
> > BLK_CTL to work, I can leave it alone (that is, enabled by default)
> > since when the PD will be powered off the clock will gated too. I
> > guess another option would be to mark it as critical, that way, it
> > will never be disabled (will be left alone by the clk_disable_unused
> > too) but at the same time will be visible in the clock hierarchy.
> >
> 
> Do ignore evrything I said about the bus_blk_ctl, that did work on our tree
> since the whole PD power on/off "magic" is done in TF-A.
> 
> So the problem, as I understand it now, is the fact that the blk_ctl driver won't
> probe because it needs its PD, but the PD is not registered because the
> ADB400 can't power up since it needs the bus_blk_ctl clock enabled, clock
> which is registered by the blk_ctl.

1. For some MIX's BLK_CTL GPRs, it can only be accessed when the MIX PD is on
2. for some MIX's ADB handshake, need to config some BLK_CTL clock_en bit to enable the MIX internal bus clock.

That's why I have concern on implementing such MIX GPR as clock & reset driver, and implementing GPC PD in linux kernel.
It will introduce some chicken-egg issue that not easy to handle in linux kernel.


BR
Jacky Bai

> 
> > > Regards,
> > > Lucas
> > >