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
Regards,
Dario
> bye,
> Heiko
> --
> 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
--
Dario Binacchi
Senior Embedded Linux Developer
dario.binac...@amarulasolutions.com
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
i...@amarulasolutions.com
www.amarulasolutions.com
