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

[Amit Nischal <anischal@codeaurora.org>]

On 2018-06-01 22:06, Stephen Boyd wrote:
> 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@codeaurora.org>
>> ---
>> 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.
> 

Thanks for fixing above as this is already in the clk-next branch
under commit "77122d".

>> 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.
> 

I will submit the next patch for all above-listed clocks with
CLK_IS_CRITICAL flag.

>> 
>>         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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html