[PATCH v3 07/10] clk: clock-wizard: Update the fixed factor divisors

Stephen Boyd sboyd at kernel.org
Mon Jan 6 06:10:43 UTC 2020


Quoting Shubhrajyoti Datta (2020-01-05 20:17:16)
> On Mon, Jan 6, 2020 at 1:30 AM Stephen Boyd <sboyd at kernel.org> wrote:
> >
> > Quoting shubhrajyoti.datta at gmail.com (2019-11-27 22:36:14)
> > > From: Shubhrajyoti Datta <shubhrajyoti.datta at xilinx.com>
> > >
> > > Update the fixed factor clock registration to register the divisors.
> > >
> > > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta at xilinx.com>
> > > ---
> > >  drivers/clk/clk-xlnx-clock-wizard.c | 17 +++++++++++------
> > >  1 file changed, 11 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
> > > index 4c6155b..75ea745 100644
> > > --- a/drivers/clk/clk-xlnx-clock-wizard.c
> > > +++ b/drivers/clk/clk-xlnx-clock-wizard.c
> > > @@ -491,9 +491,11 @@ static int clk_wzrd_probe(struct platform_device *pdev)
> > >         u32 reg, reg_f, mult;
> > >         unsigned long rate;
> > >         const char *clk_name;
> > > +       void __iomem *ctrl_reg;
> > >         struct clk_wzrd *clk_wzrd;
> > >         struct resource *mem;
> > >         int outputs;
> > > +       unsigned long flags = 0;
> > >         struct device_node *np = pdev->dev.of_node;
> > >
> > >         clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL);
> > > @@ -564,19 +566,22 @@ static int clk_wzrd_probe(struct platform_device *pdev)
> > >                 goto err_disable_clk;
> > >         }
> > >
> > > -       /* register div */
> > > -       reg = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
> > > -                       WZRD_DIVCLK_DIVIDE_MASK) >> WZRD_DIVCLK_DIVIDE_SHIFT;
> > > +       outputs = of_property_count_strings(np, "clock-output-names");
> > > +       if (outputs == 1)
> > > +               flags = CLK_SET_RATE_PARENT;
> >
> > What does the number of clk outputs have to do with the ability to
> > change the rate of a parent clk? The commit text doesn't inform me of
> > what this is for either. Please help us understand.
> 
> If there are multiple clocks then changing the rate of the parent
> changes the rate of all the
> outputs so we donot allow changing the rate of the parent if there are
> multiple clocks.
> If there is only one output then that is not an issue.

Maybe your downstream consumers should use the rate locking APIs instead
of having this restriction in the provider driver? Look at
clk_set_rate_exclusive() and associated APIs.

> 
> I will update the description in the next version.


More information about the devel mailing list