Hello,
On 6/2/26 6:09 AM, Johannes Schneider wrote:
> From: Thomas Haemmerle <[email protected]>
>
> The i.MX8MP LCDIF2 controller lives inside the MEDIAMIX power domain,
> which has its own blk-ctrl to gate clocks and control resets. Without
> enabling the MEDIAMIX bus clock and de-asserting the block-level resets
> before the first register access, any read or write to LCDIF2 registers
> causes an AXI bus hang and barebox locks up on startup.
>
> Extend the existing blk-ctrl driver (currently HSIO-only) to support the
> media blk-ctrl (fsl,imx8mp-media-blk-ctrl) by:
> - Separating the HSIO ADB handshake into the HSIO-specific power_on hook
> so the generic path is clean for other blk-ctrls without such handshake.
> - Adding null checks on bc->power_on/off to allow instances without custom
> callbacks.
> - Increasing DOMAIN_MAX_CLKS from 2 to 3 (LCDIF2 needs disp2, axi, apb).
> - Adding media blk-ctrl probe that enables the MEDIAMIX bus clock and
> de-asserts LCDIF2 clocks/resets, mirroring what the Linux kernel does
> in imx8mp_media_power_notifier.
>
> The media_blk_ctrl DTS node retains its syscon compatible so that
> syscon_node_to_regmap() continues to work for the LDB bridge sub-device.
>
> Assisted-by: Claude:claude-sonnet-4-6
> Signed-of-by: Thomas Haemmerle <[email protected]>
> ---
> drivers/pmdomain/imx/imx8mp-blk-ctrl.c | 117 +++++++++++++++++++++++--
> 1 file changed, 108 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> b/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> index 3d302cbde7..87e0114c22 100644
> --- a/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> +++ b/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> @@ -17,6 +17,7 @@
>
> #include <dt-bindings/power/imx8mp-power.h>
>
> +/* HSIO blk-ctrl registers */
> #define GPR_REG0 0x0
> #define PCIE_CLOCK_MODULE_EN BIT(0)
> #define USB_CLOCK_MODULE_EN BIT(1)
> @@ -32,6 +33,11 @@
> #define PLL_CKE BIT(17)
> #define PLL_RST BIT(31)
>
> +/* Media blk-ctrl registers */
> +#define LCDIF_ARCACHE_CTRL 0x40
> +#define LCDIF_1_RD_HURRY GENMASK(6, 4)
> +#define LCDIF_0_RD_HURRY GENMASK(2, 0)
> +
> struct imx8mp_blk_ctrl_domain;
>
> struct imx8mp_blk_ctrl {
> @@ -51,7 +57,7 @@ struct imx8mp_blk_ctrl_domain_data {
> const char *gpc_name;
> };
>
> -#define DOMAIN_MAX_CLKS 2
> +#define DOMAIN_MAX_CLKS 3
>
> struct imx8mp_blk_ctrl_domain {
> struct generic_pm_domain genpd;
> @@ -155,9 +161,14 @@ static int imx8mp_hsio_blk_ctrl_probe(struct
> imx8mp_blk_ctrl *bc)
> return of_clk_add_hw_provider(dev_of_node(bc->dev),
> of_clk_hw_simple_get, hw);
> }
>
> +static int imx8mp_hsio_propagate_adb_handshake(struct imx8mp_blk_ctrl *bc);
> +
> static void imx8mp_hsio_blk_ctrl_power_on(struct imx8mp_blk_ctrl *bc,
> struct imx8mp_blk_ctrl_domain *domain)
> {
> + /* propagate ADB handshake once before any HSIO sub-domain is enabled */
> + imx8mp_hsio_propagate_adb_handshake(bc);
This used to happen _before_ enabling upstream clocks, but now it
happens _after_. The commit message doesn't explain why this would be ok.
Maybe Marco or Lucas have a thought on this?
Cheers,
Ahmad
> +
> switch (domain->id) {
> case IMX8MP_HSIOBLK_PD_USB:
> regmap_set_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> @@ -259,6 +270,66 @@ static const struct imx8mp_blk_ctrl_data
> imx8mp_hsio_blk_ctl_dev_data = {
> .num_domains = ARRAY_SIZE(imx8mp_hsio_domain_data),
> };
>
> +/* Media blk-ctrl */
> +
> +/*
> + * MEDIAMIX BLK_CTRL register offsets (from i.MX 8M Plus RM, section 13).
> + * Bit SET = reset de-asserted / clock enabled.
> + */
> +#define BLK_SFT_RSTN 0x00
> +#define BLK_CLK_EN 0x04
> +
> +/* BIT(8): bus/APB clock and reset for the MEDIAMIX interconnect */
> +#define MEDIAMIX_BUS_CLK_RST BIT(8)
> +
> +/*
> + * LCDIF2 (mediablk-lcdif-2) bits in BLK_SFT_RSTN and BLK_CLK_EN:
> + * BIT(11): lcdif2-axi, BIT(12): lcdif2-apb, BIT(24): disp2 pixel clock
> + * (from Linux kernel imx8m-blk-ctrl.c IMX8MP_MEDIABLK_PD_LCDIF_2)
> + */
> +#define LCDIF2_CLK_RST_MASK (BIT(11) | BIT(12) | BIT(24))
> +
> +static int imx8mp_media_blk_ctrl_probe(struct imx8mp_blk_ctrl *bc)
> +{
> + /* Set panic read hurry level for LCDIF interfaces to 7 */
> + regmap_update_bits(bc->regmap, LCDIF_ARCACHE_CTRL,
> + FIELD_PREP(LCDIF_1_RD_HURRY, 7) |
> + FIELD_PREP(LCDIF_0_RD_HURRY, 7),
> + FIELD_PREP(LCDIF_1_RD_HURRY, 7) |
> + FIELD_PREP(LCDIF_0_RD_HURRY, 7));
> +
> + /*
> + * Enable the MEDIAMIX bus clock and de-assert its reset so the
> + * internal AHB/APB fabric is up before sub-module access.
> + * (Mirrors Linux kernel imx8mp_media_power_notifier BIT(8) writes.)
> + */
> + regmap_set_bits(bc->regmap, BLK_CLK_EN, MEDIAMIX_BUS_CLK_RST);
> + regmap_set_bits(bc->regmap, BLK_SFT_RSTN, MEDIAMIX_BUS_CLK_RST);
> + udelay(5); /* wait for ADB handshake, as Linux kernel does */
> +
> + /*
> + * Enable LCDIF2 clocks and de-assert its reset within MEDIAMIX.
> + * Without this, LCDIF2 registers are inaccessible (AXI bus hangs).
> + * (Mirrors Linux kernel imx8mp_blk_ctrl_power_on for LCDIF_2 domain.)
> + */
> + regmap_set_bits(bc->regmap, BLK_CLK_EN, LCDIF2_CLK_RST_MASK);
> + regmap_set_bits(bc->regmap, BLK_SFT_RSTN, LCDIF2_CLK_RST_MASK);
> +
> + return 0;
> +}
> +
> +static const struct imx8mp_blk_ctrl_data imx8mp_media_blk_ctl_dev_data = {
> + .max_reg = 0x138,
> + .probe = imx8mp_media_blk_ctrl_probe,
> + /*
> + * num_domains intentionally omitted (= 0): skip GPC power domain
> + * management for the media blk-ctrl. MEDIAMIX is already powered
> + * on by the boot ROM/SPL, so no GPC sequencing is needed in the
> + * bootloader. With barebox deep-probe enabled, lcdif2
> + * silently ignores the missing genpd provider and probes directly.
> + */
> +};
> +
> static int imx8mp_blk_ctrl_power_on(struct generic_pm_domain *genpd)
> {
> struct imx8mp_blk_ctrl_domain *domain =
> to_imx8mp_blk_ctrl_domain(genpd);
> @@ -273,12 +344,6 @@ static int imx8mp_blk_ctrl_power_on(struct
> generic_pm_domain *genpd)
> return ret;
> }
>
> - ret = imx8mp_hsio_propagate_adb_handshake(bc);
> - if (ret) {
> - dev_err(bc->dev, "failed to propagate adb handshake\n");
> - goto bus_put;
> - }
> -
> /* enable upstream clocks */
> ret = clk_bulk_prepare_enable(data->num_clks, domain->clks);
> if (ret) {
> @@ -287,7 +352,8 @@ static int imx8mp_blk_ctrl_power_on(struct
> generic_pm_domain *genpd)
> }
>
> /* domain specific blk-ctrl manipulation */
> - bc->power_on(bc, domain);
> + if (bc->power_on)
> + bc->power_on(bc, domain);
>
> /* power up upstream GPC domain */
> ret = pm_runtime_resume_and_get_genpd(domain->power_dev);
> @@ -322,7 +388,8 @@ static int imx8mp_blk_ctrl_power_off(struct
> generic_pm_domain *genpd)
> }
>
> /* domain specific blk-ctrl manipulation */
> - bc->power_off(bc, domain);
> + if (bc->power_off)
> + bc->power_off(bc, domain);
>
> clk_bulk_disable_unprepare(data->num_clks, domain->clks);
>
> @@ -380,6 +447,35 @@ static int imx8mp_blk_ctrl_probe(struct device *dev)
> if (!bc->onecell_data.domains)
> return -ENOMEM;
>
> + /*
> + * Skip GPC power domain management when num_domains == 0.
> + * This is used for blk-ctrl instances (e.g. media) where we only
> + * want the regmap/probe side-effects and not genpd provider
> + * registration, which could trigger GPC power-on sequences at
> + * unexpected times during boot.
> + */
> + if (num_domains == 0) {
> + /*
> + * For the media blk-ctrl we skip genpd provider registration to
> + * avoid triggering full GPC power sequencing for every
> consumer.
> + * However we still need the MEDIAMIX domain to be powered
> before
> + * accessing any of its registers (including blk-ctrl itself).
> + * Power it on via the "bus" domain and keep it on.
> + */
> + bc->bus_power_dev = dev_pm_domain_attach_by_name(dev, "bus");
> + if (!IS_ERR_OR_NULL(bc->bus_power_dev)) {
> + ret =
> pm_runtime_resume_and_get_genpd(bc->bus_power_dev);
> + if (ret < 0)
> + dev_warn(dev, "failed to power on MEDIAMIX
> (ignoring): %d\n", ret);
> + }
> + if (bc_data->probe) {
> + ret = bc_data->probe(bc);
> + if (ret)
> + return ret;
> + }
> + return 0;
> + }
> +
> bc->bus_power_dev = dev_pm_domain_attach_by_name(dev, "bus");
> if (IS_ERR(bc->bus_power_dev))
> return dev_err_probe(dev, PTR_ERR(bc->bus_power_dev),
> @@ -461,6 +557,9 @@ static const struct of_device_id
> imx8mp_blk_ctrl_of_match[] = {
> {
> .compatible = "fsl,imx8mp-hsio-blk-ctrl",
> .data = &imx8mp_hsio_blk_ctl_dev_data,
> + }, {
> + .compatible = "fsl,imx8mp-media-blk-ctrl",
> + .data = &imx8mp_media_blk_ctl_dev_data,
> }, {
> /* Sentinel */
> }
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
