diff mbox series

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

Message ID 55183b0a7c466528361802fabef65a57f969d07b.1574922435.git.shubhrajyoti.datta@xilinx.com (mailing list archive)
State Changes Requested, archived
Headers show
Series clk: clk-wizard: clock-wizard: Driver updates | expand

Commit Message

Shubhrajyoti Datta Nov. 28, 2019, 6:36 a.m. UTC
From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>

Update the fixed factor clock registration to register the divisors.

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
 drivers/clk/clk-xlnx-clock-wizard.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

Comments

Stephen Boyd Jan. 5, 2020, 8 p.m. UTC | #1
Quoting shubhrajyoti.datta@gmail.com (2019-11-27 22:36:14)
> From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> 
> Update the fixed factor clock registration to register the divisors.
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@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.

>         clk_name = kasprintf(GFP_KERNEL, "%s_mul_div", dev_name(&pdev->dev));
>         if (!clk_name) {
>                 ret = -ENOMEM;
>                 goto err_rm_int_clk;
>         }
>
Shubhrajyoti Datta Jan. 6, 2020, 4:17 a.m. UTC | #2
On Mon, Jan 6, 2020 at 1:30 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting shubhrajyoti.datta@gmail.com (2019-11-27 22:36:14)
> > From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> >
> > Update the fixed factor clock registration to register the divisors.
> >
> > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@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.

I will update the description in the next version.
>
> >         clk_name = kasprintf(GFP_KERNEL, "%s_mul_div", dev_name(&pdev->dev));
> >         if (!clk_name) {
> >                 ret = -ENOMEM;
> >                 goto err_rm_int_clk;
> >         }
> >
diff mbox series

Patch

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;
 	clk_name = kasprintf(GFP_KERNEL, "%s_mul_div", dev_name(&pdev->dev));
 	if (!clk_name) {
 		ret = -ENOMEM;
 		goto err_rm_int_clk;
 	}
 
-	clk_wzrd->clks_internal[wzrd_clk_mul_div] = clk_register_fixed_factor
+	ctrl_reg = clk_wzrd->base + WZRD_CLK_CFG_REG(0);
+	/* register div */
+	clk_wzrd->clks_internal[wzrd_clk_mul_div] = clk_register_divider
 			(&pdev->dev, clk_name,
 			 __clk_get_name(clk_wzrd->clks_internal[wzrd_clk_mul]),
-			 0, 1, reg);
+			flags, ctrl_reg, 0, 8, CLK_DIVIDER_ONE_BASED |
+			CLK_DIVIDER_ALLOW_ZERO, &clkwzrd_lock);
 	if (IS_ERR(clk_wzrd->clks_internal[wzrd_clk_mul_div])) {
 		dev_err(&pdev->dev, "unable to register divider clock\n");
 		ret = PTR_ERR(clk_wzrd->clks_internal[wzrd_clk_mul_div]);
@@ -597,7 +602,7 @@  static int clk_wzrd_probe(struct platform_device *pdev)
 		if (!i)
 			clk_wzrd->clkout[i] = clk_wzrd_register_divf
 				(&pdev->dev, clkout_name,
-				clk_name, 0,
+				clk_name, flags,
 				clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12),
 				WZRD_CLKOUT_DIVIDE_SHIFT,
 				WZRD_CLKOUT_DIVIDE_WIDTH,