Re: [PATCH 3/3] clk: sunxi-ng: a64: stop force-selecting PLL-MIPI as TCON0 parent

[Dragan Simic <dsimic@manjaro.org>]

Hello Vasily,

On 2024-12-15 06:34, Vasily Khoruzhick wrote:
> Force selecting PLL-MIPI as TCON0 parent breaks video output on 
> Pinebook
> that uses RGB to eDP bridge.
> 
> TCON0 clock parent will be selected in the device tree instead.
> 
> Fixes: ca1170b69968 ("clk: sunxi-ng: a64: force select PLL_MIPI in 
> TCON0 mux")
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>

Looking good to me, as a patch that completes the panel bugfix.
Thanks once again for the patches!

I'd suggest that the patch description is improved further a bit,
by incorporating some parts of the good description of the issue
that's already in the cover letter.  In particular, I'd suggest
that you describe that the patch partially reverts an earlier
commit, etc.

Additionally, I'd suggest that the comment block deleted below is
actually adjusted and copied to the addition to the A64 SoC dtsi,
which is performed in the first patch in your series.  That might
be of high value later.

With that addressed, please feel free to include

Reviewed-by: Dragan Simic <dsimic@manjaro.org>

> ---
>  drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 11 -----------
>  1 file changed, 11 deletions(-)
> 
> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> index 3a7d61c81667..cc8de0bfbc67 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> @@ -534,12 +534,6 @@ static SUNXI_CCU_M_WITH_MUX_GATE(de_clk, "de", 
> de_parents,
>  				 0x104, 0, 4, 24, 3, BIT(31),
>  				 CLK_SET_RATE_PARENT);
> 
> -/*
> - * DSI output seems to work only when PLL_MIPI selected. Set it and 
> prevent
> - * the mux from reparenting.
> - */
> -#define SUN50I_A64_TCON0_CLK_REG	0x118
> -
>  static const char * const tcon0_parents[] = { "pll-mipi", 
> "pll-video0-2x" };
>  static const u8 tcon0_table[] = { 0, 2, };
>  static SUNXI_CCU_MUX_TABLE_WITH_GATE_CLOSEST(tcon0_clk, "tcon0", 
> tcon0_parents,
> @@ -959,11 +953,6 @@ static int sun50i_a64_ccu_probe(struct
> platform_device *pdev)
> 
>  	writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
> 
> -	/* Set PLL MIPI as parent for TCON0 */
> -	val = readl(reg + SUN50I_A64_TCON0_CLK_REG);
> -	val &= ~GENMASK(26, 24);
> -	writel(val | (0 << 24), reg + SUN50I_A64_TCON0_CLK_REG);
> -
>  	ret = devm_sunxi_ccu_probe(&pdev->dev, reg, &sun50i_a64_ccu_desc);
>  	if (ret)
>  		return ret;