linux-kernel - Re: [PATCH v1 7/7] clk: qcom: Add video clock controller driver for SC7180

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20191116055118.2FCD120729@mail.kernel.org>
Date:   Fri, 15 Nov 2019 21:51:17 -0800
From:   Stephen Boyd <sboyd@...nel.org>
To:     Michael Turquette <mturquette@...libre.com>,
        Taniya Das <tdas@...eaurora.org>
Cc:     David Brown <david.brown@...aro.org>,
        Rajendra Nayak <rnayak@...eaurora.org>,
        linux-arm-msm@...r.kernel.org, linux-soc@...r.kernel.org,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
        Andy Gross <agross@...nel.org>, devicetree@...r.kernel.org,
        robh@...nel.org, robh+dt@...nel.org
Subject: Re: [PATCH v1 7/7] clk: qcom: Add video clock controller driver for SC7180

Quoting Taniya Das (2019-11-15 00:11:31)
> Hi Stephen,
> 
> Thanks for the review.
> 
> On 11/6/2019 6:09 AM, Stephen Boyd wrote:
> > Quoting Taniya Das (2019-10-31 05:21:13)
> >> diff --git a/drivers/clk/qcom/videocc-sc7180.c b/drivers/clk/qcom/videocc-sc7180.c
> >> new file mode 100644
> >> index 0000000..bef034b
> >> --- /dev/null
> >> +++ b/drivers/clk/qcom/videocc-sc7180.c
> >> @@ -0,0 +1,263 @@
> > 
> >> +       video_pll0_config.alpha = 0x4000;
> >> +       video_pll0_config.user_ctl_val = 0x00000001;
> >> +       video_pll0_config.user_ctl_hi_val = 0x00004805;
> > 
> > Same question, why on stack?
> > 
> 
> Will update the same.

Sounds like nothing to do.

> 
> >> +
> >> +       clk_fabia_pll_configure(&video_pll0, regmap, &video_pll0_config);
> >> +
> >> +       /* video_cc_xo_clk */
> > 
> > What are we doing? Enabling it?
> > 
> 
> yes, enabling it. Did not model and mark it CRITICAL.

Ok. Please describe that in the comment.

> >> +}
> >> +module_exit(video_cc_sc7180_exit);
> > 
> > Same question, module platform driver perhaps?
> > 
> 
> I will move it to subsys_initcall().
> 

Hmm ok.