Re: [PATCH 3/5] clk: imx8mp: Allow adjusting the video PLL rates for the LDB clock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] clk: imx8mp: media_ldb registered with imx8m_clk_hw_composite_bus_flags()
--

commit 0eadafd0202ef3a4c96dd126be3ffec604e5e8eb
Author: Miquel Raynal <miquel.raynal@bootlin.com>

clk: imx8mp: Allow adjusting the video PLL rates for the LDB clock

This commit adds the CLK_SET_RATE_PARENT flag to the IMX8MP_CLK_MEDIA_LDB
clock so that rate changes can propagate to the parent video PLLs.

> diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
> index fe6dac70f1a15..3540992b89409 100644
> --- a/drivers/clk/imx/clk-imx8mp.c
> +++ b/drivers/clk/imx/clk-imx8mp.c
> @@ -753,7 +753,7 @@ static int imx8mp_clocks_probe(struct platform_device *pdev)
>  	hws[IMX8MP_CLK_MEDIA_MIPI_PHY1_REF] = imx8m_clk_hw_composite("media_mipi_phy1_ref", imx8mp_media_mipi_phy1_ref_sels, ccm_base + 0xbd80);
>  	hws[IMX8MP_CLK_MEDIA_DISP1_PIX] = imx8m_clk_hw_composite_bus_flags("media_disp1_pix", imx8mp_media_disp_pix_sels, ccm_base + 0xbe00, CLK_SET_RATE_PARENT);
>  	hws[IMX8MP_CLK_MEDIA_CAM2_PIX] = imx8m_clk_hw_composite("media_cam2_pix", imx8mp_media_cam2_pix_sels, ccm_base + 0xbe80);
> -	hws[IMX8MP_CLK_MEDIA_LDB] = imx8m_clk_hw_composite("media_ldb", imx8mp_media_ldb_sels, ccm_base + 0xbf00);
> +	hws[IMX8MP_CLK_MEDIA_LDB] = imx8m_clk_hw_composite_bus_flags("media_ldb", imx8mp_media_ldb_sels, ccm_base + 0xbf00, CLK_SET_RATE_PARENT);

[Severity: Medium]
Should IMX8MP_CLK_MEDIA_LDB in imx8mp_clocks_probe() be registered with
imx8m_clk_hw_composite_flags() instead of
imx8m_clk_hw_composite_bus_flags()?

In the i.MX8MP CCM register map, offsets 0xa000..0xc300 (including
media_ldb at ccm_base + 0xbf00) are IP peripheral clock slices rather than
bus clock slices (0x8800..0x9300).

Using imx8m_clk_hw_composite_bus_flags() passes IMX_COMPOSITE_BUS to
__imx8m_clk_hw_composite() in drivers/clk/imx/clk-composite-8m.c:

	} else if (composite_flags & IMX_COMPOSITE_BUS) {
		div->shift = PCG_PREDIV_SHIFT;
		div->width = PCG_PREDIV_WIDTH;
		divider_ops = &imx8m_clk_composite_divider_ops;
		mux_ops = &imx8m_clk_composite_mux_ops;
	} else {
		div->shift = PCG_PREDIV_SHIFT;
		div->width = PCG_PREDIV_WIDTH;
		divider_ops = &imx8m_clk_composite_divider_ops;
		mux_ops = &clk_mux_ops;
		if (!(composite_flags & IMX_COMPOSITE_FW_MANAGED))
			flags |= CLK_SET_PARENT_GATE;
	}

Passing IMX_COMPOSITE_BUS selects imx8m_clk_composite_mux_ops (intended for
the dual SEL_A/SEL_B glitchless mux on core and bus slices) instead of
clk_mux_ops, and drops CLK_SET_PARENT_GATE (added in commit 936c383673b9 to
gate IP peripheral clock slices during reparenting and avoid clock
glitches).

In drivers/clk/imx/clk.h, imx8m_clk_hw_composite_flags() is available for IP
peripheral clock slices that need extra clock flags while keeping
composite_flags set to 0:

#define imx8m_clk_hw_composite_flags(name, parent_names, reg, flags) \
	_imx8m_clk_hw_composite(name, parent_names, reg, \
			0, IMX_COMPOSITE_CLK_FLAGS_DEFAULT |  flags)

>  	hws[IMX8MP_CLK_MEMREPAIR] = imx8m_clk_hw_composite_critical("mem_repair", imx8mp_memrepair_sels, ccm_base + 0xbf80);
>  	hws[IMX8MP_CLK_MEDIA_MIPI_TEST_BYTE] = imx8m_clk_hw_composite("media_mipi_test_byte", imx8mp_media_mipi_test_byte_sels, ccm_base + 0xc100);
>  	hws[IMX8MP_CLK_ECSPI3] = imx8m_clk_hw_composite("ecspi3", imx8mp_ecspi3_sels, ccm_base + 0xc180);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261021-ge-clk-lcm-v1-0-246cc35b5ffb@bootlin.com?part=3