lkml.org 
[lkml]   [2017]   [Apr]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH v2] clk/axs10x: introduce AXS10X pll driver
    Date
    Hi Stephen,

    On Tue, 2017-04-04 at 18:35 -0700, Stephen Boyd wrote:
    > > +     .pll_table = (struct pll_of_table []){
    > > +             {
    > > +                     .prate = 27000000,
    >
    > Can this be another clk in the framework instead of hardcoding
    > the parent rate?

    In fact there is another clk in the framework that represents this parent clock. But this field is needed to get
    appropriate pll_cfg_table as it depends on parent clock frequency. Below in pll_cfg_get function we are searching for
    the correct table comparing .parent_node field with real hardware parent clock frequency:
    ---------------------------------->8------------------------------------
    for (i = 0; pll_table[i].prate != 0; i++)
        if (pll_table[i].prate == prate)
            return pll_table[i].pll_cfg_table;
    ---------------------------------->8------------------------------------

    >
    > > +                     .pll_cfg_table = (struct pll_cfg []){
    > > +                             { 25200000, 1, 84, 90 },
    > > +                             { 50000000, 1, 100, 54 },
    > > +                             { 74250000, 1, 44, 16 },
    > > +                             { },
    > > +                     },
    > > +             },
    > > +             /* Used as list limiter */
    > > +             { },
    >
    > There's only ever one, so I'm confused why we're making a list.

    By this patch we only add support of core arc pll and pgu pll and today they are clocked by the only parent clocks
    introduced here. But other plls on axs10x may be driven by different or configurable clocks, so in such cases we will
    have more than one entry in this list. And we are going to add more supported plls to this driver in the nearest future.

    > > +
    > > +     clk = clk_register(NULL, &pll_clk->hw);
    > > +     if (IS_ERR(clk)) {
    > > +             pr_err("failed to register %s clock (%ld)\n",
    > > +                             node->name, PTR_ERR(clk));
    > > +             kfree(pll_clk);
    > > +             return;
    > > +     }
    > > +
    > > +     of_clk_add_provider(node, of_clk_src_simple_get, clk);
    >
    > Can you please use the clk_hw based provider and clk registration
    > functions?

    Sure. Could you be so kind to explain what is the difference between hw and non-hw based provider and clk registration
    functions please? In which cases they are preferred? 

    >
    > > +}
    > > +
    > > +CLK_OF_DECLARE(axs10x_pll_clock, "snps,axs10x-arc-pll-clock", of_pll_clk_setup);
    >
    > Does this need to be CLK_OF_DECLARE_DRIVER? I mean does the
    > driver need to probe and also have this of declare happen? Is the
    > PLL special and needs to be used for the timers?

    It is special and is used for the timers, so we have to CLK_OF_DECLARE it. On the other hand similar pll is used to
    drive PGU clock frequency and other subsystems and so we add usual probe func.

    --
    Best regards,
    Vlad Zakharov <vzakhar@synopsys.com>
    \
     
     \ /
      Last update: 2017-04-05 18:07    [W:3.155 / U:0.988 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site