Re: [PATCH v2 1/5] clk: imx6: remove quirky clk_set_parent(LDB_diN_sel, pll5_video_div)

[Philipp Zabel <p.zabel@pengutronix.de>]

On Tue, 2019-05-21 at 17:56 +0200, Ahmad Fatoum wrote:
> barebox has inherited the clk_set_parent(ldb_diN_sel, pll5_video_div)
> from upstream kernel commit 32f3b8da22 ("ARM i.MX6q: set the LDB serial
> clock parent to the video PLL"), where it was enabled for all i.MX6Q
> revisions after 1.0. It was applied whenever CONFIG_DRIVER_VIDEO_IMX_IPUV3
> was defined.
> 
> The kernel removed this reparenting again as a preventive measure
> against ERR009219 in 03d576f202 ("clk: imx6: Make the LDB_DI0 and
> LDB_DI1 clocks read-only").
> 
> As the kernel used the device tree compatible, which is the same
> for e.g. i.MX6Solo and DualLite, but barebox queried ANATOP which
> lists different CPU types for i.MX6Solo and DualLite and because
> i.MX6QuadPlus wasn't supported at first, barebox grew to do
> the reparenting on the odd set of:
>   - Quad and Dual rev >1.0
>   - DualLite
>   - Solo rev >1.0
>   - QuadPlus and DualPlus rev >1.0
> 
> On all of these, except for the QuadPlus and the DualPlus, this
> reparenting may glitch the LDB so that it permanently locks up.
> 
> By removing the reparenting in this commit, producing this glitch is
> avoided, unless the device tree[1] reinstates the old behavior:
> 
> 	&clks {
> 		assigned-clocks = <&clks IMX6QDL_CLK_LDB_DI0_SEL>,
> 				  <&clks IMX6QDL_CLK_LDB_DI1_SEL>;
> 		assigned-clock-parents = <&clks IMX6QDL_CLK_PLL5_VIDEO_DIV>,
> 					 <&clks IMX6QDL_CLK_PLL5_VIDEO_DIV>;
> 	};
> 
> Follow-up patches will explicitly check for this and do the reparenting
> in a glitch-free manner.
> 
> As of v2019.03.0, following mainline boards are potentially broken
        ^^^^^^^^^^
This Barebox version is out of date.

> by this (i.e. they're supported by barebox, are in the list above,
> had a LDB enabled and might be defining CONFIG_DRIVER_VIDEO_IMX_IPUV3):
>   - imx6qdl-zii-rdu2.dtsi
>   - imx6qdl-udoo.dtsi
>   - imx6qdl-mba6x.dtsi
>   - imx6q-var-custom.dts
>   - imx6q-guf-santaro.dts
>   - imx6q-embedsky-e9.dtsi

Besides RDU2 and Santaro these are all development boards.
UDOO, E9, and MBa6x have monitor connectors, so using PLL5 for LVDS may
be the wrong thing anyway, depending on the connected panel, and on
whether the HDMI/DVI connector is going to be used.

For RDU2 and Santaro I think we need the above DT change. Both don't
have HDMI connectors, which makes PLL5 the correct clock to use, and I
can't see any other place where LDB_DI clocks are switched to it.

> [1]: If barebox is configured to show a boot splash screen, this snippet
>      should exist in the barebox device tree. If barebox acts on it, the
>      kernel will show following warning:
>          ccm: ldb_di0_sel already changed from reset value: 0
>          ccm: ldb_di1_sel already changed from reset value: 0
>      This warning is safe to ignore.
> 
> Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Raphael Poggi <poggi.raph@gmail.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Apart from that,
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox