| Message ID | 1598621996-31040-6-git-send-email-shubhrajyoti.datta@xilinx.com |
|---|---|
| State | Accepted |
| Commit | 91d695d71841ab4ed7d26e27ee194aed03328095 |
| Headers | show |
| Series | None | expand |
Quoting Shubhrajyoti Datta (2020-08-28 06:39:53) > Currently the set rate granularity is to integral divisors. > Add support for the fractional divisors. > Only the first output0 is fractional in the hardware. > > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> Getting closer. > diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c > index 8dfcec8..1af59a4 100644 > --- a/drivers/clk/clk-xlnx-clock-wizard.c > +++ b/drivers/clk/clk-xlnx-clock-wizard.c > @@ -185,6 +191,134 @@ static const struct clk_ops clk_wzrd_clk_divider_ops = { > .recalc_rate = clk_wzrd_recalc_rate, > }; > > +static unsigned long clk_wzrd_recalc_ratef(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + unsigned int val; > + u32 div, frac; > + struct clk_wzrd_divider *divider = to_clk_wzrd_divider(hw); > + void __iomem *div_addr = divider->base + divider->offset; > + > + val = readl(div_addr); > + div = val & div_mask(divider->width); > + frac = (val >> WZRD_CLKOUT_FRAC_SHIFT) & WZRD_CLKOUT_FRAC_MASK; > + > + return ((parent_rate * 1000) / ((div * 1000) + frac)); Please remove extra parenthesis. And is this mult_frac()? > +} > + > +static int clk_wzrd_dynamic_reconfig_f(struct clk_hw *hw, unsigned long rate, > + unsigned long parent_rate) > +{ > + int err; > + u32 value, pre; > + unsigned long rate_div, f, clockout0_div; > + struct clk_wzrd_divider *divider = to_clk_wzrd_divider(hw); > + void __iomem *div_addr = divider->base + divider->offset; > + > + rate_div = ((parent_rate * 1000) / rate); > + clockout0_div = rate_div / 1000; > + > + pre = DIV_ROUND_CLOSEST((parent_rate * 1000), rate); > + f = (u32)(pre - (clockout0_div * 1000)); > + f = f & WZRD_CLKOUT_FRAC_MASK; > + > + value = ((f << WZRD_CLKOUT_DIVIDE_WIDTH) | (clockout0_div & > + WZRD_CLKOUT_DIVIDE_MASK)); Please split this to multiple lines. > + > + /* Set divisor and clear phase offset */ > + writel(value, div_addr); > + writel(0x0, div_addr + WZRD_DR_DIV_TO_PHASE_OFFSET); > + > + /* Check status register */ > + err= readl_poll_timeout(divider->base + WZRD_DR_STATUS_REG_OFFSET, value, > + value & WZRD_DR_LOCK_BIT_MASK, > + WZRD_USEC_POLL, WZRD_TIMEOUT_POLL); > + if (err) > + return err; > + > + /* Initiate reconfiguration */ > + writel(WZRD_DR_BEGIN_DYNA_RECONF, > + divider->base + WZRD_DR_INIT_REG_OFFSET); > + > + /* Check status register */ > + err= readl_poll_timeout(divider->base + WZRD_DR_STATUS_REG_OFFSET, value, > + value & WZRD_DR_LOCK_BIT_MASK, > + WZRD_USEC_POLL, WZRD_TIMEOUT_POLL); > + > + return err; Just return readl_poll_timeout() please. > +} > + > +static long clk_wzrd_round_rate_f(struct clk_hw *hw, unsigned long rate, > + unsigned long *prate) > +{ > + return rate; Can every rate be supported? This function is supposed to tell the clk framework what rate will be achieved if we call clk_set_rate() with 'rate' passed to this function. Almost always returning 'rate' is not the case. > > + > +static const struct clk_ops clk_wzrd_clk_divider_ops_f = { > + .round_rate = clk_wzrd_round_rate_f, > + .set_rate = clk_wzrd_dynamic_reconfig_f, > + .recalc_rate = clk_wzrd_recalc_ratef, > +}; > + > +static struct clk *clk_wzrd_register_divf(struct device *dev, > + const char *name, > + const char *parent_name, > + unsigned long flags, > + void __iomem *base, u16 offset, > + u8 shift, u8 width, > + u8 clk_divider_flags, > + const struct clk_div_table *table, > + spinlock_t *lock) > +{ > + struct clk_wzrd_divider *div; > + struct clk_hw *hw; > + struct clk_init_data init; > + int ret; > + > + if (clk_divider_flags & CLK_DIVIDER_HIWORD_MASK) { Is this used? It's a rockchip specific flag mostly so probably not? > + if (width + shift > 16) { > + pr_warn("divider value exceeds LOWORD field\n"); > + return ERR_PTR(-EINVAL); > + } > + } > + > + /* allocate the divider */ Please remove useless comments like this. > + div = kzalloc(sizeof(*div), GFP_KERNEL); > + if (!div) > + return ERR_PTR(-ENOMEM); > + > + init.name = name; > + > + if (clk_divider_flags & CLK_DIVIDER_READ_ONLY) Is this flag used? > + init.ops = &clk_divider_ro_ops; > + else > + init.ops = &clk_wzrd_clk_divider_ops_f; > + > + init.flags = flags; > + init.parent_names = (parent_name ? &parent_name : NULL); > + init.num_parents = (parent_name ? 1 : 0); Do you have cases where there isn't a parent? Hopefully not, so this can be simplified. > > + /* struct clk_divider assignments */ Drop this comment? > + div->base = base; > + div->offset = offset; > + div->shift = shift; > + div->width = width; > + div->flags = clk_divider_flags; > + div->lock = lock; > + div->hw.init = &init; > + div->table = table; > + > + /* register the clock */ Drop this comment? > + hw = &div->hw; > + ret = clk_hw_register(dev, hw); Any reason we can't use devm_clk_hw_register() here? > + if (ret) { > + kfree(div); > + return ERR_PTR(ret); > + } > + > + return hw->clk; > +} > + > static struct clk *clk_wzrd_register_divider(struct device *dev, > const char *name, > const char *parent_name, > @@ -355,17 +489,13 @@ static int clk_wzrd_probe(struct platform_device *pdev) > goto err_disable_clk; > } > > - /* we don't support fractional div/mul yet */ > - reg = readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) & > - WZRD_CLKFBOUT_FRAC_EN; > - reg |= readl(clk_wzrd->base + WZRD_CLK_CFG_REG(2)) & > - WZRD_CLKOUT0_FRAC_EN; > - if (reg) > - dev_warn(&pdev->dev, "fractional div/mul not supported\n"); > - > /* register multiplier */ > reg = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) & > WZRD_CLKFBOUT_MULT_MASK) >> WZRD_CLKFBOUT_MULT_SHIFT; > + reg_f = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) & > + WZRD_CLKFBOUT_FRAC_MASK) >> WZRD_CLKFBOUT_FRAC_SHIFT; Use two lines please. Read into variable on one line, shift on another. > + > + mult = ((reg * 1000) + reg_f); Please remove extra parenthesis. > clk_name = kasprintf(GFP_KERNEL, "%s_mul", dev_name(&pdev->dev)); > if (!clk_name) { > ret = -ENOMEM; > @@ -413,8 +543,18 @@ static int clk_wzrd_probe(struct platform_device *pdev) > ret = -EINVAL; > goto err_rm_int_clks; > } > - clk_wzrd->clkout[i] = clk_wzrd_register_divider(&pdev->dev, > - clkout_name, > + if (!i) > + clk_wzrd->clkout[i] = clk_wzrd_register_divf > + (&pdev->dev, clkout_name, > + clk_name, 0, > + clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12), > + WZRD_CLKOUT_DIVIDE_SHIFT, > + WZRD_CLKOUT_DIVIDE_WIDTH, > + CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO, > + NULL, &clkwzrd_lock); > + else > + clk_wzrd->clkout[i] = clk_wzrd_register_divider > + (&pdev->dev, clkout_name, > clk_name, 0, > clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12), > WZRD_CLKOUT_DIVIDE_SHIFT, > I wonder if a do-while loop with flags set to ONE_BASED and ALLOW_ZERO could work and then flags gets overwritten to be just DIVIDE_SHIFT? Then we don't have to duplicate the registration line.
On Fri, Aug 28, 2020 at 07:09:53PM +0530, Shubhrajyoti Datta wrote: > + > + /* Check status register */ > + err= readl_poll_timeout(divider->base + WZRD_DR_STATUS_REG_OFFSET, value, > + value & WZRD_DR_LOCK_BIT_MASK, > + WZRD_USEC_POLL, WZRD_TIMEOUT_POLL); Checkpatch will catch the missing space in "err= readl_". regards, dan carpenter
Hi , Thanks for the review. > -----Original Message----- > From: Stephen Boyd <sboyd@kernel.org> > Sent: Tuesday, September 22, 2020 2:48 AM > To: Shubhrajyoti Datta <shubhraj@xilinx.com>; linux-clk@vger.kernel.org > Cc: devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; > devel@driverdev.osuosl.org; robh+dt@kernel.org; > gregkh@linuxfoundation.org; mturquette@baylibre.com; Shubhrajyoti > Datta <shubhraj@xilinx.com> > Subject: Re: [PATCH v6 5/8] clk: clock-wizard: Add support for fractional > support > > Quoting Shubhrajyoti Datta (2020-08-28 06:39:53) > > Currently the set rate granularity is to integral divisors. > > Add support for the fractional divisors. > > Only the first output0 is fractional in the hardware. > > > > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> > > Getting closer. > > > diff --git a/drivers/clk/clk-xlnx-clock-wizard.c > > b/drivers/clk/clk-xlnx-clock-wizard.c > > index 8dfcec8..1af59a4 100644 > > --- a/drivers/clk/clk-xlnx-clock-wizard.c > > +++ b/drivers/clk/clk-xlnx-clock-wizard.c > > @@ -185,6 +191,134 @@ static const struct clk_ops > clk_wzrd_clk_divider_ops = { > > .recalc_rate = clk_wzrd_recalc_rate, }; > > > > +static unsigned long clk_wzrd_recalc_ratef(struct clk_hw *hw, > > + unsigned long parent_rate) > > +{ > > + unsigned int val; > > + u32 div, frac; > > + struct clk_wzrd_divider *divider = to_clk_wzrd_divider(hw); > > + void __iomem *div_addr = divider->base + divider->offset; > > + > > + val = readl(div_addr); > > + div = val & div_mask(divider->width); > > + frac = (val >> WZRD_CLKOUT_FRAC_SHIFT) & > > + WZRD_CLKOUT_FRAC_MASK; > > + > > + return ((parent_rate * 1000) / ((div * 1000) + frac)); > > Please remove extra parenthesis. And is this mult_frac()? > Will fix > > +} > > + > > +static int clk_wzrd_dynamic_reconfig_f(struct clk_hw *hw, unsigned long > rate, > > + unsigned long parent_rate) { > > + int err; > > + u32 value, pre; > > + unsigned long rate_div, f, clockout0_div; > > + struct clk_wzrd_divider *divider = to_clk_wzrd_divider(hw); > > + void __iomem *div_addr = divider->base + divider->offset; > > + > > + rate_div = ((parent_rate * 1000) / rate); > > + clockout0_div = rate_div / 1000; > > + > > + pre = DIV_ROUND_CLOSEST((parent_rate * 1000), rate); > > + f = (u32)(pre - (clockout0_div * 1000)); > > + f = f & WZRD_CLKOUT_FRAC_MASK; > > + > > + value = ((f << WZRD_CLKOUT_DIVIDE_WIDTH) | (clockout0_div & > > + WZRD_CLKOUT_DIVIDE_MASK)); > > Please split this to multiple lines. Will fix > > > + > > + /* Set divisor and clear phase offset */ > > + writel(value, div_addr); > > + writel(0x0, div_addr + WZRD_DR_DIV_TO_PHASE_OFFSET); > > + > > + /* Check status register */ > > + err= readl_poll_timeout(divider->base + > WZRD_DR_STATUS_REG_OFFSET, value, > > + value & WZRD_DR_LOCK_BIT_MASK, > > + WZRD_USEC_POLL, WZRD_TIMEOUT_POLL); > > + if (err) > > + return err; > > + > > + /* Initiate reconfiguration */ > > + writel(WZRD_DR_BEGIN_DYNA_RECONF, > > + divider->base + WZRD_DR_INIT_REG_OFFSET); > > + > > + /* Check status register */ > > + err= readl_poll_timeout(divider->base + > WZRD_DR_STATUS_REG_OFFSET, value, > > + value & WZRD_DR_LOCK_BIT_MASK, > > + WZRD_USEC_POLL, WZRD_TIMEOUT_POLL); > > + > > + return err; > > Just return readl_poll_timeout() please. Will fix > > > +} > > + > > +static long clk_wzrd_round_rate_f(struct clk_hw *hw, unsigned long > rate, > > + unsigned long *prate) { > > + return rate; > > Can every rate be supported? This function is supposed to tell the clk > framework what rate will be achieved if we call clk_set_rate() with 'rate' > passed to this function. Almost always returning 'rate' is not the case. > We can support rate upto 3 decimal places to prevent truncation here we are Returning rate. > > > > + > > +static const struct clk_ops clk_wzrd_clk_divider_ops_f = { > > + .round_rate = clk_wzrd_round_rate_f, > > + .set_rate = clk_wzrd_dynamic_reconfig_f, > > + .recalc_rate = clk_wzrd_recalc_ratef, }; > > + > > +static struct clk *clk_wzrd_register_divf(struct device *dev, > > + const char *name, > > + const char *parent_name, > > + unsigned long flags, > > + void __iomem *base, u16 offset, > > + u8 shift, u8 width, > > + u8 clk_divider_flags, > > + const struct clk_div_table *table, > > + spinlock_t *lock) { > > + struct clk_wzrd_divider *div; > > + struct clk_hw *hw; > > + struct clk_init_data init; > > + int ret; > > + > > + if (clk_divider_flags & CLK_DIVIDER_HIWORD_MASK) { > > Is this used? It's a rockchip specific flag mostly so probably not? > > > + if (width + shift > 16) { > > + pr_warn("divider value exceeds LOWORD field\n"); > > + return ERR_PTR(-EINVAL); > > + } > > + } > > + > > + /* allocate the divider */ > > Please remove useless comments like this. Will fix > > > + div = kzalloc(sizeof(*div), GFP_KERNEL); > > + if (!div) > > + return ERR_PTR(-ENOMEM); > > + > > + init.name = name; > > + > > + if (clk_divider_flags & CLK_DIVIDER_READ_ONLY) > > Is this flag used? Will fix > > > + init.ops = &clk_divider_ro_ops; > > + else > > + init.ops = &clk_wzrd_clk_divider_ops_f; > > + > > + init.flags = flags; > > + init.parent_names = (parent_name ? &parent_name : NULL); > > + init.num_parents = (parent_name ? 1 : 0); > > Do you have cases where there isn't a parent? Hopefully not, so this can be > simplified. > Will fix > > > > + /* struct clk_divider assignments */ > > Drop this comment? Will fix > > > + div->base = base; > > + div->offset = offset; > > + div->shift = shift; > > + div->width = width; > > + div->flags = clk_divider_flags; > > + div->lock = lock; > > + div->hw.init = &init; > > + div->table = table; > > + > > + /* register the clock */ > > Drop this comment? Will fix > > > + hw = &div->hw; > > + ret = clk_hw_register(dev, hw); > > Any reason we can't use devm_clk_hw_register() here? > Will do > > + if (ret) { > > + kfree(div); > > + return ERR_PTR(ret); > > + } > > + > > + return hw->clk; > > +} > > + > > static struct clk *clk_wzrd_register_divider(struct device *dev, > > const char *name, > > const char *parent_name, > > @@ -355,17 +489,13 @@ static int clk_wzrd_probe(struct > platform_device *pdev) > > goto err_disable_clk; > > } > > > > - /* we don't support fractional div/mul yet */ > > - reg = readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) & > > - WZRD_CLKFBOUT_FRAC_EN; > > - reg |= readl(clk_wzrd->base + WZRD_CLK_CFG_REG(2)) & > > - WZRD_CLKOUT0_FRAC_EN; > > - if (reg) > > - dev_warn(&pdev->dev, "fractional div/mul not supported\n"); > > - > > /* register multiplier */ > > reg = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) & > > WZRD_CLKFBOUT_MULT_MASK) >> > > WZRD_CLKFBOUT_MULT_SHIFT; > > + reg_f = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) & > > + WZRD_CLKFBOUT_FRAC_MASK) >> > > + WZRD_CLKFBOUT_FRAC_SHIFT; > > Use two lines please. Read into variable on one line, shift on another. > Will fix > > + > > + mult = ((reg * 1000) + reg_f); > > Please remove extra parenthesis. > > > clk_name = kasprintf(GFP_KERNEL, "%s_mul", dev_name(&pdev- > >dev)); > > if (!clk_name) { > > ret = -ENOMEM; > > @@ -413,8 +543,18 @@ static int clk_wzrd_probe(struct platform_device > *pdev) > > ret = -EINVAL; > > goto err_rm_int_clks; > > } > > - clk_wzrd->clkout[i] = clk_wzrd_register_divider(&pdev->dev, > > - clkout_name, > > + if (!i) > > + clk_wzrd->clkout[i] = clk_wzrd_register_divf > > + (&pdev->dev, clkout_name, > > + clk_name, 0, > > + clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12), > > + WZRD_CLKOUT_DIVIDE_SHIFT, > > + WZRD_CLKOUT_DIVIDE_WIDTH, > > + CLK_DIVIDER_ONE_BASED | > CLK_DIVIDER_ALLOW_ZERO, > > + NULL, &clkwzrd_lock); > > + else > > + clk_wzrd->clkout[i] = clk_wzrd_register_divider > > + (&pdev->dev, clkout_name, > > clk_name, 0, > > clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12), > > WZRD_CLKOUT_DIVIDE_SHIFT, > > > > I wonder if a do-while loop with flags set to ONE_BASED and ALLOW_ZERO > could work and then flags gets overwritten to be just DIVIDE_SHIFT? Then > we don't have to duplicate the registration line. I did not understand this comment in one case I am registering for the fractional operations In another we are using the integral operations
diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c index 8dfcec8..1af59a4 100644 --- a/drivers/clk/clk-xlnx-clock-wizard.c +++ b/drivers/clk/clk-xlnx-clock-wizard.c @@ -29,20 +29,25 @@ #define WZRD_CLKFBOUT_MULT_SHIFT 8 #define WZRD_CLKFBOUT_MULT_MASK (0xff << WZRD_CLKFBOUT_MULT_SHIFT) +#define WZRD_CLKFBOUT_FRAC_SHIFT 16 +#define WZRD_CLKFBOUT_FRAC_MASK (0x3ff << WZRD_CLKFBOUT_FRAC_SHIFT) #define WZRD_DIVCLK_DIVIDE_SHIFT 0 #define WZRD_DIVCLK_DIVIDE_MASK (0xff << WZRD_DIVCLK_DIVIDE_SHIFT) #define WZRD_CLKOUT_DIVIDE_SHIFT 0 #define WZRD_CLKOUT_DIVIDE_WIDTH 8 #define WZRD_CLKOUT_DIVIDE_MASK (0xff << WZRD_DIVCLK_DIVIDE_SHIFT) +#define WZRD_CLKOUT_FRAC_SHIFT 8 +#define WZRD_CLKOUT_FRAC_MASK 0x3ff #define WZRD_DR_MAX_INT_DIV_VALUE 255 -#define WZRD_DR_NUM_RETRIES 10000 #define WZRD_DR_STATUS_REG_OFFSET 0x04 #define WZRD_DR_LOCK_BIT_MASK 0x00000001 #define WZRD_DR_INIT_REG_OFFSET 0x25C #define WZRD_DR_DIV_TO_PHASE_OFFSET 4 #define WZRD_DR_BEGIN_DYNA_RECONF 0x03 +#define WZRD_USEC_POLL 10 +#define WZRD_TIMEOUT_POLL 1000 /* Get the mask from width */ #define div_mask(width) ((1 << (width)) - 1) @@ -52,6 +57,7 @@ enum clk_wzrd_int_clks { wzrd_clk_mul, wzrd_clk_mul_div, + wzrd_clk_mul_frac, wzrd_clk_int_max }; @@ -185,6 +191,134 @@ static const struct clk_ops clk_wzrd_clk_divider_ops = { .recalc_rate = clk_wzrd_recalc_rate, }; +static unsigned long clk_wzrd_recalc_ratef(struct clk_hw *hw, + unsigned long parent_rate) +{ + unsigned int val; + u32 div, frac; + struct clk_wzrd_divider *divider = to_clk_wzrd_divider(hw); + void __iomem *div_addr = divider->base + divider->offset; + + val = readl(div_addr); + div = val & div_mask(divider->width); + frac = (val >> WZRD_CLKOUT_FRAC_SHIFT) & WZRD_CLKOUT_FRAC_MASK; + + return ((parent_rate * 1000) / ((div * 1000) + frac)); +} + +static int clk_wzrd_dynamic_reconfig_f(struct clk_hw *hw, unsigned long rate, + unsigned long parent_rate) +{ + int err; + u32 value, pre; + unsigned long rate_div, f, clockout0_div; + struct clk_wzrd_divider *divider = to_clk_wzrd_divider(hw); + void __iomem *div_addr = divider->base + divider->offset; + + rate_div = ((parent_rate * 1000) / rate); + clockout0_div = rate_div / 1000; + + pre = DIV_ROUND_CLOSEST((parent_rate * 1000), rate); + f = (u32)(pre - (clockout0_div * 1000)); + f = f & WZRD_CLKOUT_FRAC_MASK; + + value = ((f << WZRD_CLKOUT_DIVIDE_WIDTH) | (clockout0_div & + WZRD_CLKOUT_DIVIDE_MASK)); + + /* Set divisor and clear phase offset */ + writel(value, div_addr); + writel(0x0, div_addr + WZRD_DR_DIV_TO_PHASE_OFFSET); + + /* Check status register */ + err= readl_poll_timeout(divider->base + WZRD_DR_STATUS_REG_OFFSET, value, + value & WZRD_DR_LOCK_BIT_MASK, + WZRD_USEC_POLL, WZRD_TIMEOUT_POLL); + if (err) + return err; + + /* Initiate reconfiguration */ + writel(WZRD_DR_BEGIN_DYNA_RECONF, + divider->base + WZRD_DR_INIT_REG_OFFSET); + + /* Check status register */ + err= readl_poll_timeout(divider->base + WZRD_DR_STATUS_REG_OFFSET, value, + value & WZRD_DR_LOCK_BIT_MASK, + WZRD_USEC_POLL, WZRD_TIMEOUT_POLL); + + return err; +} + +static long clk_wzrd_round_rate_f(struct clk_hw *hw, unsigned long rate, + unsigned long *prate) +{ + return rate; +} + +static const struct clk_ops clk_wzrd_clk_divider_ops_f = { + .round_rate = clk_wzrd_round_rate_f, + .set_rate = clk_wzrd_dynamic_reconfig_f, + .recalc_rate = clk_wzrd_recalc_ratef, +}; + +static struct clk *clk_wzrd_register_divf(struct device *dev, + const char *name, + const char *parent_name, + unsigned long flags, + void __iomem *base, u16 offset, + u8 shift, u8 width, + u8 clk_divider_flags, + const struct clk_div_table *table, + spinlock_t *lock) +{ + struct clk_wzrd_divider *div; + struct clk_hw *hw; + struct clk_init_data init; + int ret; + + if (clk_divider_flags & CLK_DIVIDER_HIWORD_MASK) { + if (width + shift > 16) { + pr_warn("divider value exceeds LOWORD field\n"); + return ERR_PTR(-EINVAL); + } + } + + /* allocate the divider */ + div = kzalloc(sizeof(*div), GFP_KERNEL); + if (!div) + return ERR_PTR(-ENOMEM); + + init.name = name; + + if (clk_divider_flags & CLK_DIVIDER_READ_ONLY) + init.ops = &clk_divider_ro_ops; + else + init.ops = &clk_wzrd_clk_divider_ops_f; + + init.flags = flags; + init.parent_names = (parent_name ? &parent_name : NULL); + init.num_parents = (parent_name ? 1 : 0); + + /* struct clk_divider assignments */ + div->base = base; + div->offset = offset; + div->shift = shift; + div->width = width; + div->flags = clk_divider_flags; + div->lock = lock; + div->hw.init = &init; + div->table = table; + + /* register the clock */ + hw = &div->hw; + ret = clk_hw_register(dev, hw); + if (ret) { + kfree(div); + return ERR_PTR(ret); + } + + return hw->clk; +} + static struct clk *clk_wzrd_register_divider(struct device *dev, const char *name, const char *parent_name, @@ -303,7 +437,7 @@ static SIMPLE_DEV_PM_OPS(clk_wzrd_dev_pm_ops, clk_wzrd_suspend, static int clk_wzrd_probe(struct platform_device *pdev) { int i, ret; - u32 reg; + u32 reg, reg_f, mult; unsigned long rate; const char *clk_name; struct clk_wzrd *clk_wzrd; @@ -355,17 +489,13 @@ static int clk_wzrd_probe(struct platform_device *pdev) goto err_disable_clk; } - /* we don't support fractional div/mul yet */ - reg = readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) & - WZRD_CLKFBOUT_FRAC_EN; - reg |= readl(clk_wzrd->base + WZRD_CLK_CFG_REG(2)) & - WZRD_CLKOUT0_FRAC_EN; - if (reg) - dev_warn(&pdev->dev, "fractional div/mul not supported\n"); - /* register multiplier */ reg = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) & WZRD_CLKFBOUT_MULT_MASK) >> WZRD_CLKFBOUT_MULT_SHIFT; + reg_f = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) & + WZRD_CLKFBOUT_FRAC_MASK) >> WZRD_CLKFBOUT_FRAC_SHIFT; + + mult = ((reg * 1000) + reg_f); clk_name = kasprintf(GFP_KERNEL, "%s_mul", dev_name(&pdev->dev)); if (!clk_name) { ret = -ENOMEM; @@ -374,7 +504,7 @@ static int clk_wzrd_probe(struct platform_device *pdev) clk_wzrd->clks_internal[wzrd_clk_mul] = clk_register_fixed_factor (&pdev->dev, clk_name, __clk_get_name(clk_wzrd->clk_in1), - 0, reg, 1); + 0, mult, 1000); kfree(clk_name); if (IS_ERR(clk_wzrd->clks_internal[wzrd_clk_mul])) { dev_err(&pdev->dev, "unable to register fixed-factor clock\n"); @@ -413,8 +543,18 @@ static int clk_wzrd_probe(struct platform_device *pdev) ret = -EINVAL; goto err_rm_int_clks; } - clk_wzrd->clkout[i] = clk_wzrd_register_divider(&pdev->dev, - clkout_name, + if (!i) + clk_wzrd->clkout[i] = clk_wzrd_register_divf + (&pdev->dev, clkout_name, + clk_name, 0, + clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12), + WZRD_CLKOUT_DIVIDE_SHIFT, + WZRD_CLKOUT_DIVIDE_WIDTH, + CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO, + NULL, &clkwzrd_lock); + else + clk_wzrd->clkout[i] = clk_wzrd_register_divider + (&pdev->dev, clkout_name, clk_name, 0, clk_wzrd->base, (WZRD_CLK_CFG_REG(2) + i * 12), WZRD_CLKOUT_DIVIDE_SHIFT,
Currently the set rate granularity is to integral divisors. Add support for the fractional divisors. Only the first output0 is fractional in the hardware. Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> --- v6: remove unnecessary typecast remove unnecessary locks use polled timeout drivers/clk/clk-xlnx-clock-wizard.c | 166 +++++++++++++++++++++++++++++++++--- 1 file changed, 153 insertions(+), 13 deletions(-)