[PATCH v6 3/3] clk: basic clock hardware types

[Rob Herring]

On 03/14/2012 01:23 PM, Sascha Hauer wrote:
> On Fri, Mar 09, 2012 at 11:54:24PM -0800, Mike Turquette wrote:
>> Many platforms support simple gateable clocks, fixed-rate clocks,
>> adjustable divider clocks and multi-parent multiplexer clocks.
>>
>> This patch introduces basic clock types for the above-mentioned hardware
>> which share some common characteristics.
>>
>> Based on original work by Jeremy Kerr and contribution by Jamie Iles.
>> Dividers and multiplexor clocks originally contributed by Richard Zhao &
>> Sascha Hauer.
>>
>> Signed-off-by: Mike Turquette <mturquette at linaro.org>
>> Signed-off-by: Mike Turquette <mturquette at ti.com>
>> Cc: Russell King <linux at arm.linux.org.uk>
>> Cc: Jeremy Kerr <jeremy.kerr at canonical.com>
>> Cc: Thomas Gleixner <tglx at linutronix.de>
>> Cc: Arnd Bergman <arnd.bergmann at linaro.org>
>> Cc: Paul Walmsley <paul at pwsan.com>
>> Cc: Shawn Guo <shawn.guo at freescale.com>
>> Cc: Sascha Hauer <s.hauer at pengutronix.de>
>> Cc: Jamie Iles <jamie at jamieiles.com>
>> Cc: Richard Zhao <richard.zhao at linaro.org>
>> Cc: Saravana Kannan <skannan at codeaurora.org>
>> Cc: Magnus Damm <magnus.damm at gmail.com>
>> Cc: Rob Herring <rob.herring at calxeda.com>
>> Cc: Mark Brown <broonie at opensource.wolfsonmicro.com>
>> Cc: Linus Walleij <linus.walleij at stericsson.com>
>> Cc: Stephen Boyd <sboyd at codeaurora.org>
>> Cc: Amit Kucheria <amit.kucheria at linaro.org>
>> Cc: Deepak Saxena <dsaxena at linaro.org>
>> Cc: Grant Likely <grant.likely at secretlab.ca>
>> Cc: Andrew Lunn <andrew at lunn.ch>
>> ---
>> Changes since v5:
>>  * standardized the hw-specific locking in the basic clock types
>>  * export the individual ops for each basic clock type
>>  * improve registration for single-parent basic clocks (thanks Sascha)
>>  * fixed bugs in gate clock's static initializers (thanks Andrew)
>>
>>  drivers/clk/Makefile         |    3 +-
>>  drivers/clk/clk-divider.c    |  204 ++++++++++++++++++++++++++++++++++++++++++
>>  drivers/clk/clk-fixed-rate.c |   82 +++++++++++++++++
>>  drivers/clk/clk-gate.c       |  157 ++++++++++++++++++++++++++++++++
>>  drivers/clk/clk-mux.c        |  123 +++++++++++++++++++++++++
>>  include/linux/clk-private.h  |  124 +++++++++++++++++++++++++
>>  include/linux/clk-provider.h |  127 ++++++++++++++++++++++++++
>>  7 files changed, 819 insertions(+), 1 deletions(-)
>>  create mode 100644 drivers/clk/clk-divider.c
>>  create mode 100644 drivers/clk/clk-fixed-rate.c
>>  create mode 100644 drivers/clk/clk-gate.c
>>  create mode 100644 drivers/clk/clk-mux.c
>>
>> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
>> index ff362c4..1f736bc 100644
>> --- a/drivers/clk/Makefile
>> +++ b/drivers/clk/Makefile
>> @@ -1,3 +1,4 @@
>>  
>>  obj-$(CONFIG_CLKDEV_LOOKUP)	+= clkdev.o
>> -obj-$(CONFIG_COMMON_CLK)	+= clk.o
>> +obj-$(CONFIG_COMMON_CLK)	+= clk.o clk-fixed-rate.o clk-gate.o \
>> +				   clk-mux.o clk-divider.o
>> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
>> new file mode 100644
>> index 0000000..c0c4e0b
>> --- /dev/null
>> +++ b/drivers/clk/clk-divider.c
>> @@ -0,0 +1,204 @@
>> +/*
>> + * Copyright (C) 2011 Sascha Hauer, Pengutronix <s.hauer at pengutronix.de>
>> + * Copyright (C) 2011 Richard Zhao, Linaro <richard.zhao at linaro.org>
>> + * Copyright (C) 2011-2012 Mike Turquette, Linaro Ltd <mturquette at linaro.org>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * Adjustable divider clock implementation
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/module.h>
>> +#include <linux/slab.h>
>> +#include <linux/io.h>
>> +#include <linux/err.h>
>> +#include <linux/string.h>
>> +
>> +/*
>> + * DOC: basic adjustable divider clock that cannot gate
>> + *
>> + * Traits of this clock:
>> + * prepare - clk_prepare only ensures that parents are prepared
>> + * enable - clk_enable only ensures that parents are enabled
>> + * rate - rate is adjustable.  clk->rate = parent->rate / divisor
>> + * parent - fixed parent.  No clk_set_parent support
>> + */
>> +
>> +#define to_clk_divider(_hw) container_of(_hw, struct clk_divider, hw)
>> +
>> +static unsigned long clk_divider_recalc_rate(struct clk_hw *hw,
>> +		unsigned long parent_rate)
>> +{
>> +	struct clk_divider *divider = to_clk_divider(hw);
>> +	unsigned int div;
>> +	unsigned long flags = 0;
>> +
>> +	if (divider->lock)
>> +		spin_lock_irqsave(divider->lock, flags);
>> +
>> +	div = readl(divider->reg) >> divider->shift;
>> +	div &= (1 << divider->width) - 1;
>> +
>> +	if (divider->lock)
>> +		spin_unlock_irqrestore(divider->lock, flags);
>> +
>> +	if (!(divider->flags & CLK_DIVIDER_ONE_BASED))
>> +		div++;
>> +
>> +	return parent_rate / div;
>> +}
>> +EXPORT_SYMBOL_GPL(clk_divider_recalc_rate);
>> +
>> +static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
>> +		unsigned long *best_parent_rate)
>> +{
>> +	struct clk_divider *divider = to_clk_divider(hw);
>> +	int i, bestdiv = 0;
>> +	unsigned long parent_rate, best = 0, now, maxdiv;
>> +
>> +	maxdiv = (1 << divider->width);
> 
> We need a check for rate == 0 here:
> 
> 	if (rate == 0)
> 		rate = 1;
> 
> Otherwise we divide by zero below.
> 
>> +
>> +	if (divider->flags & CLK_DIVIDER_ONE_BASED)
>> +		maxdiv--;
>> +
>> +	if (!(__clk_get_flags(hw->clk) & CLK_SET_RATE_PARENT)) {
>> +		parent_rate = __clk_get_rate(__clk_get_parent(hw->clk));
>> +		bestdiv = parent_rate / rate;
> 
> It's a bit strange but due to integer maths you must use DIV_ROUND_UP
> in round_rate and div = parent_rate / rate in set_rate.
> 
> Example:
> 
> parent_rate = 100
> rate = 34
> 
> Now the best divider would be 3 (100/3 = 33), but the above
> results in 2. With DIV_ROUND_UP round_rate correctly returns
> 33. Now when you use DIV_ROUND_UP in set_rate the resulting
> divider would be 4 which is wrong whereas 100/33 returns the correct
> result.

This gets into the problem with round_rate. You can't specify whether to
round up or down. For something like an SD card you would want to pass
in the max freq for the card and get back something less than or equal
to it. For something like an LCD pixel clock, you need a frequency
greater than or equal to the requested freq to maintain a minimum
refresh rate. There's been some discussion about this some time back.

But for now we shouldn't fix round_rate, and your solution is right (or
at least safer).

Rob