Re: [PATCH 2/2] clk: qcom: gcc-sc8280xp: Don't keep display AHB clocks always-on

[Johan Hovold <johan@kernel.org>]

On Fri, Sep 29, 2023 at 03:38:53PM +0200, Konrad Dybcio wrote:
> These clocks are consumed by the dispcc[01] clock controllers, so there's
> no reason to keep them on from gcc probe. Remove that hack.

Eh, how did you test this patch?

The GCC_DISP_AHB_CLK clocks are not modelled by the clock driver
currently so nothing is guaranteeing them to be enabled if we were to
apply this patch. They just happen to be left on by the bootloader on
some machines currently (well at least one of them is on one machine).

So this series is broken and should not be applied.

NAK.

Also, please CC me on sc8280xp and X13s related patches.
 
> Fixes: d65d005f9a6c ("clk: qcom: add sc8280xp GCC driver")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  drivers/clk/qcom/gcc-sc8280xp.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clk/qcom/gcc-sc8280xp.c b/drivers/clk/qcom/gcc-sc8280xp.c
> index bfb77931e868..bf95f82a3818 100644
> --- a/drivers/clk/qcom/gcc-sc8280xp.c
> +++ b/drivers/clk/qcom/gcc-sc8280xp.c
> @@ -7545,18 +7545,16 @@ static int gcc_sc8280xp_probe(struct platform_device *pdev)
>  
>  	/*
>  	 * Keep the clocks always-ON
> -	 * GCC_CAMERA_AHB_CLK, GCC_CAMERA_XO_CLK, GCC_DISP_AHB_CLK,
> +	 * GCC_CAMERA_AHB_CLK, GCC_CAMERA_XO_CLK,
>  	 * GCC_DISP_XO_CLK, GCC_GPU_CFG_AHB_CLK, GCC_VIDEO_AHB_CLK,
> -	 * GCC_VIDEO_XO_CLK, GCC_DISP1_AHB_CLK, GCC_DISP1_XO_CLK
> +	 * GCC_VIDEO_XO_CLK, GCC_DISP1_XO_CLK
>  	 */
>  	regmap_update_bits(regmap, 0x26004, BIT(0), BIT(0));
>  	regmap_update_bits(regmap, 0x26020, BIT(0), BIT(0));
> -	regmap_update_bits(regmap, 0x27004, BIT(0), BIT(0));
>  	regmap_update_bits(regmap, 0x27028, BIT(0), BIT(0));
>  	regmap_update_bits(regmap, 0x71004, BIT(0), BIT(0));
>  	regmap_update_bits(regmap, 0x28004, BIT(0), BIT(0));
>  	regmap_update_bits(regmap, 0x28028, BIT(0), BIT(0));
> -	regmap_update_bits(regmap, 0xbb004, BIT(0), BIT(0));
>  	regmap_update_bits(regmap, 0xbb028, BIT(0), BIT(0));

Johan