Quoting Dmitry Baryshkov (2021-04-01 18:49:37)
> 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@xxxxxxxxxx>
> >> ---
> >> 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.
>
Yes we shouldn't be adding .name anymore. Can we add the
video_pll0_out_{even,odd} clks? Or if they're not used then can we
remove them from the parent_data and leave some sort of comment
indicating that they may be there?
> >
> >> { 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",
Looks like in this case it would be OK because the array would be length
2 instead of length 5.
> >> };
