Re: [PATCH v2] clk: qcom: Enable clocks which needs to be always on for SDM845

From: Stephen Boyd
Date: Fri Jun 01 2018 - 12:36:32 EST


Quoting Amit Nischal (2018-05-28 23:07:46)
> There are certain clocks which needs to be always enabled for system
> operation. Remove support for such clocks from the clock driver and
> enable them explicitly gcc driver probe. Also fix the target name in
> kernel configuration description for SDM845 gcc driver.

This is why we have CLK_IS_CRITICAL.

>
> Signed-off-by: Amit Nischal <anischal@xxxxxxxxxxxxxx>
> ---
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index fc3edd1..79e9144 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -231,7 +231,7 @@ config SDM_GCC_845
> select QCOM_GDSC
> depends on COMMON_CLK_QCOM
> help
> - Support for the global clock controller on msm8998 devices.
> + Support for the global clock controller on SDM845 devices.
> Say Y if you want to use peripheral devices such as UART, SPI,
> I2C, USB, UFS, SDDC, PCIe, etc.
>

This can be its own patch.

> diff --git a/drivers/clk/qcom/gcc-sdm845.c b/drivers/clk/qcom/gcc-sdm845.c
> index e78e6f5..b40aafc 100644
> --- a/drivers/clk/qcom/gcc-sdm845.c
> +++ b/drivers/clk/qcom/gcc-sdm845.c
> @@ -3433,9 +3327,16 @@ static int gcc_sdm845_probe(struct platform_device *pdev)
> regmap_update_bits(regmap, 0x09ffc, 0x3, 0x3);
> regmap_update_bits(regmap, 0x71028, 0x3, 0x3);
>
> - /* Enable CPUSS clocks */
> - regmap_update_bits(regmap, 0x48190, BIT(0), 0x1);
> - regmap_update_bits(regmap, 0x52004, BIT(22), 0x1);
> + /* Enable clocks which are required to be always ON */
> + regmap_update_bits(regmap, 0xb004, BIT(0), BIT(0));
> + regmap_update_bits(regmap, 0xb008, BIT(0), BIT(0));
> + regmap_update_bits(regmap, 0xb00c, BIT(0), BIT(0));
> + regmap_update_bits(regmap, 0xb028, BIT(0), BIT(0));
> + regmap_update_bits(regmap, 0xb02c, BIT(0), BIT(0));
> + regmap_update_bits(regmap, 0xb030, BIT(0), BIT(0));
> + regmap_update_bits(regmap, 0x48190, BIT(0), BIT(0));
> + regmap_update_bits(regmap, 0x52004, BIT(22), BIT(22));
> + regmap_update_bits(regmap, 0x71004, BIT(0), BIT(0));

Let's do CLK_IS_CRITICAL flag instead. That has the added benefit of
checking to make sure the halt bit toggles correctly so the clks are
verified to be enabled.

>
> return qcom_cc_really_probe(pdev, &gcc_sdm845_desc, regmap);
> }
> diff --git a/include/dt-bindings/clock/qcom,gcc-sdm845.h b/include/dt-bindings/clock/qcom,gcc-sdm845.h
> index aca6126..6330c3f 100644
> --- a/include/dt-bindings/clock/qcom,gcc-sdm845.h
> +++ b/include/dt-bindings/clock/qcom,gcc-sdm845.h
> @@ -13,185 +13,178 @@
> #define GCC_AGGRE_USB3_PRIM_AXI_CLK 3
> #define GCC_AGGRE_USB3_SEC_AXI_CLK 4
> #define GCC_BOOT_ROM_AHB_CLK 5
> -#define GCC_CAMERA_AHB_CLK 6

Don't do this. Things should only be added to here, not removed or
reshuffled.

> -#define GCC_CAMERA_AXI_CLK 7
> -#define GCC_CAMERA_XO_CLK 8
> -#define GCC_CE1_AHB_CLK 9