On 12/22/2013 07:52 PM, Gerhard Sittig wrote:
[ dropped devicetree, we're clock specific here ]

On Fri, Dec 20, 2013 at 18:34 +0200, Tero Kristo wrote:

Divider clock can now be registered to use low level register access ops.
Preferred initialization method is via clock description.

Signed-off-by: Tero Kristo <[email protected]>
---
  drivers/clk/clk-divider.c    |   22 +++++++++++++++++++---
  include/linux/clk-provider.h |    4 ++++
  2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index 8cfed5c..887e2d8 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -108,7 +108,12 @@ static unsigned long clk_divider_recalc_rate(struct clk_hw 
*hw,
        struct clk_divider *divider = to_clk_divider(hw);
        unsigned int div, val;

-       val = clk_readl(divider->reg) >> divider->shift;
+       if (divider->ll_ops)
+               val = divider->ll_ops->clk_readl(divider->reg);
+       else
+               val = clk_readl(divider->reg);

Should this not better always use an ll_ops structure, which
either is individual to the clock item, or is "global" for a
platform, yet can get re-registered at runtime (see the comment
on 06/49)?  And why are you referencing clk_readl() instead of
clk_readl_default() which you specifically have introduced in the
previous patch?  Adding a copy of the routine and using both the
copy and the original doesn't look right.

In some cases, the clock data is defined statically during compile time and here, ll_ops can be (and for OMAP cases at least is) NULL. I had kind of a global ll_ops definition in some of the earlier versions of this series, but it was frowned upon by some of the maintainers thus I dropped it.

-Tero

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to