On Fri, Oct 18, 2024 at 11:14:00PM +0530, Taniya Das wrote:
>
>
> On 10/16/2024 3:46 PM, Dmitry Baryshkov wrote:
> > On Wed, Oct 16, 2024 at 09:40:07AM +0530, Taniya Das wrote:
> > >
> > >
> > > On 9/20/2024 4:33 PM, Dmitry Baryshkov wrote:
> > > > On Fri, Sep 20, 2024 at 04:08:18PM GMT, Taniya Das wrote:
> > > > > Add the global clock controller support for QCS615 SoC.
> > > > >
> > > > > Signed-off-by: Taniya Das <quic_tdas@xxxxxxxxxxx>
> > > > > ---
> > > > > drivers/clk/qcom/Kconfig | 9 +
> > > > > drivers/clk/qcom/Makefile | 1 +
> > > > > drivers/clk/qcom/gcc-qcs615.c | 3035 +++++++++++++++++++++++++++++++++++++++++
> > > > > 3 files changed, 3045 insertions(+)
>
> > > > > +};
> > > > > +
> > > > > +static struct clk_alpha_pll gpll0 = {
> > > > > + .offset = 0x0,
> > > > > + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
> > > > > + .clkr = {
> > > > > + .enable_reg = 0x52000,
> > > > > + .enable_mask = BIT(0),
> > > > > + .hw.init = &(const struct clk_init_data) {
> > > > > + .name = "gpll0",
> > > > > + .parent_data = &(const struct clk_parent_data) {
> > > > > + .index = DT_BI_TCXO,
> > > > > + },
> > > > > + .num_parents = 1,
> > > > > + .ops = &clk_alpha_pll_ops,
> > > > > + },
> > > > > + },
> > > > > +};
> > > > > +
> > > > > +static struct clk_fixed_factor gpll0_out_aux2_div = {
> > > > > + .mult = 1,
> > > > > + .div = 2,
> > > > > + .hw.init = &(struct clk_init_data) {
> > > > > + .name = "gpll0_out_aux2_div",
> > > > > + .parent_data = &(const struct clk_parent_data) {
> > > > > + .hw = &gpll0.clkr.hw,
> > > > > + },
> > > > > + .num_parents = 1,
> > > > > + .ops = &clk_fixed_factor_ops,
> > > > > + },
> > > > > +};
> > > >
> > > > Should it be clk_alpha_pll_postdiv_foo_ops ?
> > > >
> > >
> > > This is not the PLL output, but it is a fixed divider which is placed as
> > > input to the RCG.
> > > That is the reason to use the fixed factor.
> >
> > Usually OUT_AUX2 is the PLL output, isn't it? Even by its name. See
> > gcc-qcm2290 / gcc-sm6115 and most of other clock controller drivers,
> > except gcc-sm6125. Maybe I don't understand the difference between the
> > two usecases. Is there a difference in the GCC / PLL design?
> >
>
> Yes, your understanding is correct out_aux2/out_main are the PLL leaf
> outputs. But on QCS615 the PLL dividers are not used and thus the aux2 and
> the other leaf outputs are at the same frequency as the main output of the
> VCO and instead there was a fixed divider placed after the PLL to divide the
> VCO output. There was a GCC design change required to meet timing closures.
Ack, please add a comment that this PLL uses fixed divider instead of
a normal postdiv (and to other out_aux2 clocks too).
> > > >
> > > > > +
> > > > > +static struct clk_branch gcc_pcie_0_pipe_clk = {
> > > > > + .halt_reg = 0x6b024,
> > > > > + .halt_check = BRANCH_HALT_SKIP,
> > > > > + .clkr = {
> > > > > + .enable_reg = 0x5200c,
> > > > > + .enable_mask = BIT(4),
> > > > > + .hw.init = &(const struct clk_init_data) {
> > > > > + .name = "gcc_pcie_0_pipe_clk",
> > > > > + .ops = &clk_branch2_ops,
> > > > > + },
> > > > > + },
> > > > > +};
> > > >
> > > > No corresponding gcc_pcie_0_pipe_clk_src?
> > > >
> > >
> > > On QCS615 the pipe clock source is not required to be modelled as the mux is
> > > default Power on reset is set to external pipe clock.
> >
> > And do we need to toggle the source of the clk_src together with the
> > GDSC toggling?
> >
>
> AFAIR, QCS615 didn't require toggling for GDSC, as even on downstream kernel
> we do not have the pipe_clk_src modelled in our driver.
OK, thanks for the explanation.
--
With best wishes
Dmitry
