Re: [PATCH V2 RESEND 3/6] clk: qcom: videocc-sm8550: Add SM8650 video clock controller

[Jagadeesh Kona <quic_jkona@xxxxxxxxxxx>]

On 3/21/2024 3:33 PM, Dmitry Baryshkov wrote:
> On Thu, 21 Mar 2024 at 11:27, Jagadeesh Kona <quic_jkona@xxxxxxxxxxx> wrote:
> > Add support to the SM8650 video clock controller by extending
> > the SM8550 video clock controller, which is mostly identical
> > but SM8650 has few additional clocks and minor differences.
> >
> > Signed-off-by: Jagadeesh Kona <quic_jkona@xxxxxxxxxxx>
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
>
>
>
> > @@ -411,6 +540,7 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
> >   {
> >          struct regmap *regmap;
> >          int ret;
> > +       u32 offset = 0x8140;
>
> Nit: this variable seems misnamed. Please rename to something like
> sleep_clk_offset;

Thanks Dmitry for your review.

Yes, will rename this in the next series.

Thanks,
Jagadeesh

> >          ret = devm_pm_runtime_enable(&pdev->dev);
> >          if (ret)
> > @@ -426,12 +556,27 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
> >                  return PTR_ERR(regmap);
> >          }
> >
> > +       if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8650-videocc")) {
> > +               offset = 0x8150;
> > +               video_cc_pll0_config.l = 0x1e;
> > +               video_cc_pll0_config.alpha = 0xa000;
> > +               video_cc_pll1_config.l = 0x2b;
> > +               video_cc_pll1_config.alpha = 0xc000;
> > +               video_cc_mvs0_clk_src.freq_tbl = ftbl_video_cc_mvs0_clk_src_sm8650;
> > +               video_cc_mvs1_clk_src.freq_tbl = ftbl_video_cc_mvs1_clk_src_sm8650;
> > +               video_cc_sm8550_clocks[VIDEO_CC_MVS0_SHIFT_CLK] = &video_cc_mvs0_shift_clk.clkr;
> > +               video_cc_sm8550_clocks[VIDEO_CC_MVS0C_SHIFT_CLK] = &video_cc_mvs0c_shift_clk.clkr;
> > +               video_cc_sm8550_clocks[VIDEO_CC_MVS1_SHIFT_CLK] = &video_cc_mvs1_shift_clk.clkr;
> > +               video_cc_sm8550_clocks[VIDEO_CC_MVS1C_SHIFT_CLK] = &video_cc_mvs1c_shift_clk.clkr;
> > +               video_cc_sm8550_clocks[VIDEO_CC_XO_CLK_SRC] = &video_cc_xo_clk_src.clkr;
> > +       }
> > +
> >          clk_lucid_ole_pll_configure(&video_cc_pll0, regmap, &video_cc_pll0_config);
> >          clk_lucid_ole_pll_configure(&video_cc_pll1, regmap, &video_cc_pll1_config);
> >
> >          /* Keep some clocks always-on */
> >          qcom_branch_set_clk_en(regmap, 0x80f4); /* VIDEO_CC_AHB_CLK */
> > -       qcom_branch_set_clk_en(regmap, 0x8140); /* VIDEO_CC_SLEEP_CLK */
> > +       qcom_branch_set_clk_en(regmap, offset); /* VIDEO_CC_SLEEP_CLK */
> >          qcom_branch_set_clk_en(regmap, 0x8124); /* VIDEO_CC_XO_CLK */
> >
> >          ret = qcom_cc_really_probe(pdev, &video_cc_sm8550_desc, regmap);
> > --
> > 2.43.0