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 hs@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 hs@denx.de wrote:
Hello Dario,
On 13.09.24 11:55, Dario Binacchi wrote:
From: Michael Trimarchi michael@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.li@nxp.com Signed-off-by: Michael Trimarchi michael@amarulasolutions.com Signed-off-by: Dario Binacchi dario.binacchi@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 michael@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 michael@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;