Re: [PATCH v1 13/15] clk: qcom: videocc-sdm845: remove unsupported clock sources

[Dmitry Baryshkov <dmitry.baryshkov@linaro.org>]

On 02/04/2021 04:23, Taniya Das wrote:
> Hi Dmitry,
> 
> On 3/25/2021 4:41 PM, Dmitry Baryshkov wrote:
>> video_pll0_out_even/_odd are not supported neither in the upstream nor
>> in the downstream kernels, so drop those clock sources.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   drivers/clk/qcom/videocc-sdm845.c | 8 +-------
>>   1 file changed, 1 insertion(+), 7 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/videocc-sdm845.c 
>> b/drivers/clk/qcom/videocc-sdm845.c
>> index 5d6a7724a194..7153f044504f 100644
>> --- a/drivers/clk/qcom/videocc-sdm845.c
>> +++ b/drivers/clk/qcom/videocc-sdm845.c
>> @@ -21,24 +21,18 @@
>>   enum {
>>       P_BI_TCXO,
>>       P_CORE_BI_PLL_TEST_SE,
>> -    P_VIDEO_PLL0_OUT_EVEN,
>>       P_VIDEO_PLL0_OUT_MAIN,
>> -    P_VIDEO_PLL0_OUT_ODD,
>>   };
>>   static const struct parent_map video_cc_parent_map_0[] = {
>>       { P_BI_TCXO, 0 },
>>       { P_VIDEO_PLL0_OUT_MAIN, 1 },
>> -    { P_VIDEO_PLL0_OUT_EVEN, 2 },
>> -    { P_VIDEO_PLL0_OUT_ODD, 3 },
> 
> These are supported from the design, please do not remove them. It is 
> just that in SW currently it is not being used.
> But SW can decide to use them as they want. As said earlier these are 
> defined in the HW plans and thus do not want them to be updated manually 
> to create a mismatch.

The problem arises during conversion of these drivers to use parent_data 
instead of parent_names. You see, video_pll0_odd/_even are clocks which 
should be referenced using .hw (and thus defined inside the videocc 
driver) as we do for "video_pll0" parent. However there are no clk_hw 
entities defined for those clocks. For now I'd just use the { .name = 
video_pll0_out_odd" } entry for those clocks, however I still think this 
is not correct.

> 
>>       { P_CORE_BI_PLL_TEST_SE, 4 },
>>   };
>>   static const char * const video_cc_parent_names_0[] = {
>>       "bi_tcxo",
>>       "video_pll0",
>> -    "video_pll0_out_even",
>> -    "video_pll0_out_odd",
>>       "core_bi_pll_test_se",
>>   };
>> @@ -79,7 +73,7 @@ static struct clk_rcg2 video_cc_venus_clk_src = {
>>       .clkr.hw.init = &(struct clk_init_data){
>>           .name = "video_cc_venus_clk_src",
>>           .parent_names = video_cc_parent_names_0,
>> -        .num_parents = 5,
>> +        .num_parents = 3,
>>           .flags = CLK_SET_RATE_PARENT,
>>           .ops = &clk_rcg2_shared_ops,
>>       },
>>
> 


-- 
With best wishes
Dmitry