Hello Michael,

On 01.10.24 07:14, Heiko Schocher wrote:
Hi Micahel,

On 01.10.24 07:01, Michael Nazzareno Trimarchi wrote:
Hi Heko

On Tue, Oct 1, 2024 at 6:23 AM Heiko Schocher <h...@denx.de> wrote:

Hi Dario,

On 30.09.24 15:20, Dario Binacchi wrote:
Hello Heiko,

On Tue, Sep 24, 2024 at 11:07 AM Heiko Schocher <h...@denx.de> wrote:

Hello Dario,

On 13.09.24 11:55, Dario Binacchi wrote:
From: Michael Trimarchi <mich...@amarulasolutions.com>

Add iMX8 block ctrl driver for displaymix on iMX8MM/iMX8MN and
mediamix on iMX8MP.

To support blk ctrl driver, the power domain driver on iMX8M needs
update to add relevant PGC domains

Signed-off-by: Ye Li <ye...@nxp.com>
Signed-off-by: Michael Trimarchi <mich...@amarulasolutions.com>
Signed-off-by: Dario Binacchi <dario.binac...@amarulasolutions.com>
---

    drivers/power/domain/Kconfig              |   6 +
    drivers/power/domain/Makefile             |   1 +
    drivers/power/domain/imx8m-blk-ctrl.c     | 438 ++++++++++++++++++++++
    drivers/power/domain/imx8m-power-domain.c | 213 ++++++++++-
    4 files changed, 656 insertions(+), 2 deletions(-)
    create mode 100644 drivers/power/domain/imx8m-blk-ctrl.c

diff --git a/drivers/power/domain/Kconfig b/drivers/power/domain/Kconfig
index bd82d2f7044b..fb006b6e8e28 100644
--- a/drivers/power/domain/Kconfig
+++ b/drivers/power/domain/Kconfig
@@ -40,6 +40,12 @@ config IMX8M_POWER_DOMAIN
          Enable support for manipulating NXP i.MX8M on-SoC power domains via
          requests to the ATF.

+config IMX8M_BLK_CTRL
+     bool "Enable i.MX8M block control driver"
+     depends on POWER_DOMAIN && ARCH_IMX8M
+     help
+       Enable support for manipulating NXP i.MX8M on-SoC block control driver
+
    config IMX8MP_HSIOMIX_BLKCTRL
        bool "Enable i.MX8MP HSIOMIX domain driver"
        depends on POWER_DOMAIN && IMX8MP
diff --git a/drivers/power/domain/Makefile b/drivers/power/domain/Makefile
index 2daab73eb758..46849fd2a4db 100644
--- a/drivers/power/domain/Makefile
+++ b/drivers/power/domain/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_APPLE_PMGR_POWER_DOMAIN) += apple-pmgr.o
    obj-$(CONFIG_BCM6328_POWER_DOMAIN) += bcm6328-power-domain.o
    obj-$(CONFIG_IMX8_POWER_DOMAIN) += imx8-power-domain-legacy.o 
imx8-power-domain.o
    obj-$(CONFIG_IMX8M_POWER_DOMAIN) += imx8m-power-domain.o
+obj-$(CONFIG_IMX8M_BLK_CTRL) += imx8m-blk-ctrl.o
    obj-$(CONFIG_IMX8MP_HSIOMIX_BLKCTRL) += imx8mp-hsiomix.o
    obj-$(CONFIG_MTK_POWER_DOMAIN) += mtk-power-domain.o
    obj-$(CONFIG_MESON_GX_VPU_POWER_DOMAIN) += meson-gx-pwrc-vpu.o
diff --git a/drivers/power/domain/imx8m-blk-ctrl.c 
b/drivers/power/domain/imx8m-blk-ctrl.c
new file mode 100644
index 000000000000..4c89078b991b
--- /dev/null
+++ b/drivers/power/domain/imx8m-blk-ctrl.c
@@ -0,0 +1,438 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2023 NXP
+ */
+
+#include <dm.h>
+#include <malloc.h>
+#include <power-domain-uclass.h>
+#include <asm/io.h>
+#include <dm/device-internal.h>
+#include <dm/device.h>
+#include <dt-bindings/power/imx8mm-power.h>
+#include <dt-bindings/power/imx8mn-power.h>
+#include <dt-bindings/power/imx8mp-power.h>
+#include <clk.h>
+#include <linux/delay.h>
+
+#define BLK_SFT_RSTN 0x0
+#define BLK_CLK_EN   0x4
+#define BLK_MIPI_RESET_DIV   0x8 /* Mini/Nano/Plus DISPLAY_BLK_CTRL only */
+
+#define DOMAIN_MAX_CLKS 4
+
+struct imx8m_blk_ctrl_domain {
+     struct clk clks[DOMAIN_MAX_CLKS];
+     struct power_domain power_dev;
+};
+
+struct imx8m_blk_ctrl {
+     void __iomem *base;
+     struct power_domain bus_power_dev;
+     struct imx8m_blk_ctrl_domain *domains;
+};
+
+struct imx8m_blk_ctrl_domain_data {
+     const char *name;
+     const char * const *clk_names;
+     const char *gpc_name;
+     int num_clks;
+     u32 rst_mask;
+     u32 clk_mask;
+     u32 mipi_phy_rst_mask;
+};
+
+struct imx8m_blk_ctrl_data {
+     int max_reg;
+     const struct imx8m_blk_ctrl_domain_data *domains;
+     int num_domains;
+     u32 bus_rst_mask;
+     u32 bus_clk_mask;
+};
+
+static int imx8m_blk_ctrl_request(struct power_domain *power_domain)
+{
+     return 0;
+}
+
+static int imx8m_blk_ctrl_free(struct power_domain *power_domain)
+{
+     return 0;
+}
+
+static int imx8m_blk_ctrl_enable_domain_clk(struct udevice *dev, ulong 
domain_id, bool enable)
+{
+     int ret, i;
+     struct imx8m_blk_ctrl *priv = (struct imx8m_blk_ctrl *)dev_get_priv(dev);
+     struct imx8m_blk_ctrl_data *drv_data =
+             (struct imx8m_blk_ctrl_data *)dev_get_driver_data(dev);
+
+     debug("%s num_clk %u\n", __func__, drv_data->domains[domain_id].num_clks);
+
+     for (i = 0; i < drv_data->domains[domain_id].num_clks; i++) {
+             debug("%s clk %s\n", __func__, 
drv_data->domains[domain_id].clk_names[i]);
+             if (enable)
+                     ret = clk_enable(&priv->domains[domain_id].clks[i]);
+             else
+                     ret = clk_disable(&priv->domains[domain_id].clks[i]);
+             if (ret && ret != -ENOENT) {
+                     printf("Failed to %s domain clk %s\n", enable ? "enable" : 
"disable",
+                            drv_data->domains[domain_id].clk_names[i]);
+                     return ret;
+             }
+     }
+
+     return 0;
+}
+
+static int imx8m_blk_ctrl_power_on(struct power_domain *power_domain)
+{
+     struct udevice *dev = power_domain->dev;
+     struct imx8m_blk_ctrl *priv = (struct imx8m_blk_ctrl *)dev_get_priv(dev);
+     struct imx8m_blk_ctrl_data *drv_data =
+             (struct imx8m_blk_ctrl_data *)dev_get_driver_data(dev);
+     int ret;
+
+     debug("%s, id %lu\n", __func__, power_domain->id);
+
+     if (!priv->domains[power_domain->id].power_dev.dev)
+             return -ENODEV;
+
+     ret = power_domain_on(&priv->bus_power_dev);
+     if (ret < 0) {
+             printf("Failed to power up bus domain %d\n", ret);
+             return ret;
+     }
+
+     /* Enable bus clock and deassert bus reset */
+     setbits_le32(priv->base + BLK_CLK_EN, drv_data->bus_clk_mask);
+     setbits_le32(priv->base + BLK_SFT_RSTN, drv_data->bus_rst_mask);
+
+     /* wait for reset to propagate */
+     udelay(5);
+
+     /* put devices into reset */
+     clrbits_le32(priv->base + BLK_SFT_RSTN, 
drv_data->domains[power_domain->id].rst_mask);
+     if (drv_data->domains[power_domain->id].mipi_phy_rst_mask)
+             clrbits_le32(priv->base + BLK_MIPI_RESET_DIV, d
+                          
rv_data->domains[power_domain->id].mipi_phy_rst_mask);

Does this build for you?

I needed the following fix:

diff --git a/drivers/power/domain/imx8m-blk-ctrl.c 
b/drivers/power/domain/imx8m-blk-ctrl.c
index 4c89078b99..b772d50480 100644
--- a/drivers/power/domain/imx8m-blk-ctrl.c
+++ b/drivers/power/domain/imx8m-blk-ctrl.c
@@ -114,8 +114,8 @@ static int imx8m_blk_ctrl_power_on(struct power_domain 
*power_domain)
           /* put devices into reset */
           clrbits_le32(priv->base + BLK_SFT_RSTN, 
drv_data->domains[power_domain->id].rst_mask);
           if (drv_data->domains[power_domain->id].mipi_phy_rst_mask)
-               clrbits_le32(priv->base + BLK_MIPI_RESET_DIV, d
-                            
rv_data->domains[power_domain->id].mipi_phy_rst_mask);
+               clrbits_le32(priv->base + BLK_MIPI_RESET_DIV,
+                            
drv_data->domains[power_domain->id].mipi_phy_rst_mask);

           /* enable upstream and blk-ctrl clocks to allow reset to propagate */
           ret = imx8m_blk_ctrl_enable_domain_clk(dev, power_domain->id, true);

to get it building.

Yes, you're right. I made some changes suggested by patman, and before
applying the
   patch, I didn't recompile, so I didn't notice.


BTW: Just also working on bootlogo support for an imx8mp based board!

I now applied your patches:

clk: Propagate clk_set_rate() if CLK_SET_PARENT_RATE present for gate and mux
clk: clk-uclass: Implement CLK_OPS_PARENT_ENABLE

And added in my adapted /drivers/clk/imx/clk-imx8mp.c (imported from linux)

clk_dm(IMX8MP_CLK_MEDIA_AXI, imx8m_clk_composite_flags("media_axi", 
imx8mp_media_axi_sels,
ARRAY_SIZE(imx8mp_media_axi_sels), base + 0x8a00, CLK_IS_CRITICAL));

instead of using imx8m_clk_composite, and dropped my approach to
get clocks up and working.

Also dropped my similiar approach for mediablock and used your

power: Add iMX8M block ctrl driver for dispmix

And with this 3 patches, bootlogo works also/still fine for me!

I did not applied/need your patches:

clk: imx8mn: Prevent clock critical path from disabling during reparent and 
set_rate
clk: imx8mm: Prevent clock critical path from disabling during reparent and 
set_rate
clk: imx8mm: Mark IMX8MM_SYS_PLL2 and IMX8MM_SYS_PLL3 as enabled
clk: imx8mn: Mark IMX8MN_SYS_PLL2 and IMX8MN_SYS_PLL3 as enabled

Of course, they are for imx8mm, but I mean I do not need similiar patches
for imx8mp!

Also not applied (as for imx8mm)
clk: imx8mn: add video clocks support

but as said, have similiar patch for clk-imx8mp.c

May you check if using imx8m_clk_composite_flags() is working for you?

I will do it


I did not applied your patches 09/26 and the following patches from
your series, as I made a video bridge driver based on

linux driver drivers/gpu/drm/bridge/fsl-ldb.c

and a lcdif driver based on linux:/drivers/gpu/drm/mxsfb/lcdif_drv.c


Thank you for you feedback Heiko

You are welcome!

Hmm.. unfortunately ... I had applied your 2 clock patches, which
fixed a problem with enabling parent clocks ... but they broke booting
on a carrier which has fec ethernet! After "Net: " output the board hang...

I reverted your 2 clock patches and it bootet again ... so there is
a problem ... try to get some more time to look into...


We have a fec, but we had I think two patches more on it. I forget to
answer Marek
about them because I don't have my board now and because he is
partially right (or maybe right).
Anyway when we boot we could have and we have clocks that are enabled
by bootrom or SPL that
we need to declare as enable like PLL2/PLL3 those clocks are parts or
could be part of reparent so,
you need to have a reference counter on them that allow to not disable
during the down chain disable
and up chain enable. I think that what happens to your ethernet it's
that you disable some clock that is
critical to the board to survive. I had a patch merged by Tom that

Yep, thats what I think too! If you access registers in a block for
which the clock is not enabled ... it just hang...

prints the clock name so if you enable
the debug of the clock you will find that your board stops working
during one of this reparinting.

I currently work on 2024.07... will rebase if 2024.10 is out...

Ah, you mean:

commit a70d991212c9684e09ed80ece69ce1ff7bfd9f08
Author: Michael Trimarchi <mich...@amarulasolutions.com>
Date:   Tue Jul 9 08:28:13 2024 +0200

     clk: clk-uclass: Print clk name in clk_enable/clk_disable

     Print clk name in clk_enable and clk_disable. Make sense to know
     what clock get disabled/enabled before a system crash or system
     hang.

     Signed-off-by: Michael Trimarchi <mich...@amarulasolutions.com>

I have the same change when I debug :-P

IIRC I did not see the clock names ... but I will recheck!

I see with your patch the clock names, so fine... and see [1]

Hmm... I am on imx8mp, and I think the changes the patchset do in

"clk: imx8mn: Prevent clock critical path from disabling during reparent and 
set_rate"

are in clk-imx8mp already ...

Ported the patch from patchset

"[PATCH 05/26] clk: imx8mm: Mark IMX8MM_SYS_PLL2 and IMX8MM_SYS_PLL3 as enabled"

to imx8mp [2] and fec ethernet works again for me on imx8mp!

Could you add this if you post a v2 ?

bye,
Heiko
[1]
│   │    <> Net:   clk_set_defaults(gpio@30230000)
│ │ <> clk_set_default_parents: could not read assigned-clock-parents for 00000000fee74d90 name=gpio@30230000
│   │    <> clk_set_defaults(aristainetos3-fec-rgmii-grp)
│ │ <> clk_set_default_parents: could not read assigned-clock-parents for 00000000fee75780 name=aristainetos3-fec-rgmii-grp
│   │    <> clk_set_defaults(ethernet@30be0000)
│   │    <> clk_get_by_indexed_prop(dev=00000000fee7ad20, index=0, 
clk=00000000fee5e3b8)
│   │    <> clk_of_xlate_default(clk=00000000fee5e3b8)
│   │    <> clk_request(dev=00000000fee77480, clk=00000000fee5e3b8)
│   │    <> clk_get_by_indexed_prop(dev=00000000fee7ad20, index=0, 
clk=00000000fee5e390)
│   │    <> clk_of_xlate_default(clk=00000000fee5e390)
│   │    <> clk_request(dev=00000000fee77480, clk=00000000fee5e390)
│   │    <> clk_set_parent(clk=00000000fee825c0, parent=00000000fee80680)
│   │    <> clk_get_parent(clk=00000000fee825c0)
│   │    <> clk_enable(clk=00000000fee743d0 name=clock-osc-24m)
│   │    <> clk_enable(clk=00000000fee80680 name=sys_pll1_266m)
│   │    <> clk_enable(clk=00000000fee7fb40 name=sys_pll1_out)
│   │    <> clk_disable(clk=00000000fee80680 name=sys_pll1_266m)
│   │    <> clk_disable(clk=00000000fee7fb40 name=sys_pll1_out)
│   │    <> clk_disable(clk=00000000fee743d0 name=clock-osc-24m)
│   │    <> clk_get_by_indexed_prop(dev=00000000fee7ad20, index=1, 
clk=00000000fee5e3b8)
│   │    <> clk_of_xlate_default(clk=00000000fee5e3b8)
│   │    <> clk_request(dev=00000000fee77480, clk=00000000fee5e3b8)
│   │    <> clk_get_by_indexed_prop(dev=00000000fee7ad20, index=1, 
clk=00000000fee5e390)
│   │    <> clk_of_xlate_default(clk=00000000fee5e390)
│   │    <> clk_request(dev=00000000fee77480, clk=00000000fee5e390)
│   │    <> clk_set_parent(clk=00000000fee85b80, parent=00000000fee80b80)
│   │    <> clk_get_parent(clk=00000000fee85b80)
│   │    <> clk_enable(clk=00000000fee743d0 name=clock-osc-24m)
│   │    <> clk_enable(clk=00000000fee80b80 name=sys_pll2_100m)
│   │    <> clk_enable(clk=00000000fee7fc80 name=sys_pll2_out)
│   │    <> clk_enable(clk=00000000fee7f480 name=sys_pll2_bypass)
│   │    <> clk_enable(clk=00000000fee7eb80 name=sys_pll2)
│   │    <> clk_enable(clk=00000000fee7e280 name=sys_pll2_ref_sel)
│   │    <> clk_enable(clk=00000000fee743d0 name=clock-osc-24m)
│   │    <> clk_disable(clk=00000000fee80b80 name=sys_pll2_100m)
│   │    <> clk_disable(clk=00000000fee7fc80 name=sys_pll2_out)
│   │    <> clk_disable(clk=00000000fee7f480 name=sys_pll2_bypass)
│   │    <> clk_disable(clk=00000000fee7eb80 name=sys_pll2)

[2]
diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
index d6865509cb..f6fa66094d 100644
--- a/drivers/clk/imx/clk-imx8mp.c
+++ b/drivers/clk/imx/clk-imx8mp.c
@@ -235,6 +235,7 @@ static const char * const imx8mp_clkout_sels[] = 
{"audio_pll1_out", "audio_pll2_
 static int imx8mp_clk_probe(struct udevice *dev)
 {
        struct clk osc_24m_clk, osc_32k_clk;
+       struct clk *clk;
        void __iomem *base;
        int ret;

@@ -303,6 +304,14 @@ static int imx8mp_clk_probe(struct udevice *dev)
                return ret;
        clk_dm(IMX8MP_CLK_32K, dev_get_clk_ptr(osc_32k_clk.dev));

+       ret = clk_get_by_id(IMX8MP_SYS_PLL2, &clk);
+       if (!ret)
+               clk_enable(clk);
+
+       ret = clk_get_by_id(IMX8MP_SYS_PLL3, &clk);
+       if (!ret)
+               clk_enable(clk);
+
        base = dev_read_addr_ptr(dev);
        if (!base)
                return -EINVAL;
--
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: h...@denx.de

Reply via email to