Re: [PATCH 0/5] more clk-next fixes
Note that the Orion patches aren't here, but I figure that Andrew L. probably wants to check those against the final clk-next before I pull them. Hi Mike Here is a pull request. There is only one small change since the previous version. The last patch no longer has select COMMON_CLK_DISABLE_UNUSED. Thanks Andrew The following changes since commit d49d10779b9b9d1c07dc61bf58a7995835b7d32f: clk: clk_set_rate() must fail if CLK_SET_RATE_GATE is set and clk is enabled (2012-05-07 09:57:58 +0200) are available in the git repository at: git://github.com/lunn/orion-clk.git v3.4-rc5-clk-next-orion-v2 Andrew Lunn (14): [ARM: Orion] Add clocks using the generic clk infrastructure. [ARM: Orion: SPI] Add clk/clkdev support. [ARM: Orion: Eth] Add clk/clkdev support. [ARM: Orion: WDT] Add clk/clkdev support [ARM: Orion: UART] Get the clock rate via clk_get_rate(). [ARM: Orion: SATA] Add per channel clk/clkdev support. [ARM: Orion: EHCI] Add support for enabling clocks [ARM: Orion: NAND] Add support for clk, if there is one. [ARM: Orion: SDIO] Add support for clk. [ARM: Orion: CESA] Add support for clk [ARM: Orion: XOR] Add support for clk [ARM: Orion: PCIE] Add support for clk [ARM: Orion: Audio] Add clk/clkdev support [ARM: Kirkwood] Replace clock gating arch/arm/Kconfig |2 + arch/arm/mach-dove/common.c | 40 ++- arch/arm/mach-dove/dove-db-setup.c|1 - arch/arm/mach-kirkwood/board-dreamplug.c |1 - arch/arm/mach-kirkwood/board-dt.c |3 + arch/arm/mach-kirkwood/common.c | 274 ++--- arch/arm/mach-kirkwood/common.h |1 + arch/arm/mach-kirkwood/include/mach/bridge-regs.h | 16 ++ arch/arm/mach-kirkwood/mv88f6281gtw_ge-setup.c|1 - arch/arm/mach-kirkwood/pcie.c | 25 ++- arch/arm/mach-kirkwood/rd88f6192-nas-setup.c |1 - arch/arm/mach-kirkwood/t5325-setup.c |1 - arch/arm/mach-kirkwood/tsx1x-common.c |1 - arch/arm/mach-mv78xx0/common.c| 45 +++-- arch/arm/mach-orion5x/common.c| 27 ++- arch/arm/mach-orion5x/rd88f6183ap-ge-setup.c |1 - arch/arm/plat-orion/common.c | 104 + arch/arm/plat-orion/include/plat/common.h | 34 ++-- arch/arm/plat-orion/include/plat/orion_wdt.h | 18 -- arch/arm/plat-orion/pcie.c|4 +- drivers/ata/sata_mv.c | 40 +++- drivers/crypto/mv_cesa.c | 14 + drivers/dma/mv_xor.c | 15 ++ drivers/dma/mv_xor.h |1 + drivers/mmc/host/mvsdio.c | 14 + drivers/mtd/nand/orion_nand.c | 18 ++ drivers/net/ethernet/marvell/mv643xx_eth.c| 42 +++- drivers/spi/spi-orion.c | 30 ++- drivers/usb/host/ehci-orion.c | 16 ++ drivers/watchdog/orion_wdt.c | 16 +- include/linux/mv643xx_eth.h |1 - include/linux/spi/orion_spi.h | 17 -- sound/soc/kirkwood/kirkwood-i2s.c | 13 + sound/soc/kirkwood/kirkwood.h |1 + 34 files changed, 575 insertions(+), 263 deletions(-) delete mode 100644 arch/arm/plat-orion/include/plat/orion_wdt.h delete mode 100644 include/linux/spi/orion_spi.h ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v6 3/3] clk: basic clock hardware types
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. Hi Mike Please feel free to add: Reviewed-by: Andrew Lunn and...@lunn.ch Tested-by: Andrew Lunn and...@lunn.ch Andrew ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v6 2/3] clk: introduce the common clock framework
On Fri, Mar 09, 2012 at 11:54:23PM -0800, Mike Turquette wrote: The common clock framework defines a common struct clk useful across most platforms as well as an implementation of the clk api that drivers can use safely for managing clocks. Hi Mike Please feel free to add: Tested-by: Andrew Lunn and...@lunn.ch Andrew ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v6 1/3] Documentation: common clk API
On Fri, Mar 09, 2012 at 11:54:22PM -0800, Mike Turquette wrote: Provide documentation for the common clk structures and APIs. This code can be found in drivers/clk/ and include/linux/clk*.h. Hi Mike Please feel free to add: Reviewed-by: Andrew Lunn and...@lunn.ch Andrew ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v5 3/4] clk: introduce the common clock framework
I'd say use the nonstatic ones. I think using the static initializers
will cause us much pain in the future. I've been through several rebases
on the i.MX clock rework and everytime I wish my sed foo would be
better. Now imagine what happens when it turns out that the internal
struct clk layout or the structs for the muxes/dividers/gates have to
be changed.
/*
* CLK tree
/
static DEFINE_SPINLOCK(gating_lock);
#define DEFINE_KIRKWOOD_CLK_GATE(_name, _bit) \
DEFINE_CLK_GATE(_name, tclk, NULL, 0, \
(void __iomem *)CLOCK_GATING_CTRL, \
_bit, 0, gating_lock)
DEFINE_KIRKWOOD_CLK_GATE(clk_ge0,CGC_BIT_GE0);
DEFINE_KIRKWOOD_CLK_GATE(clk_pex0, CGC_BIT_PEX0);
DEFINE_KIRKWOOD_CLK_GATE(clk_usb0, CGC_BIT_USB0);
DEFINE_KIRKWOOD_CLK_GATE(clk_sdio, CGC_BIT_SDIO);
DEFINE_KIRKWOOD_CLK_GATE(clk_tsu,CGC_BIT_TSU);
DEFINE_KIRKWOOD_CLK_GATE(clk_dunit, CGC_BIT_DUNIT);
DEFINE_KIRKWOOD_CLK_GATE(clk_runit, CGC_BIT_RUNIT);
I've so far not had any problems, and not needed an sed foo. I do
only have a dozen or so clocks, which helps. But even so, all the real
pain is hidden inside DEFINE_CLK_GATE() which Mike maintains.
I guess the problem comes when you are not using the basic clk
providers, but your own provider. What might help is if
linux/clk-provider.h could provide some macros to do most of the
generic definitions. Something like:
#define DEFINE_CLK_GENERIC(_name, _flags, _ops) \
static struct clk _name;\
static char *_name##_parent_names[] = {}; \
static struct clk _name = { \
.name = #_name, \
.ops = _ops, \
.hw = _name##_hw.hw, \
.parent_names = _name##_parent_names, \
.num_parents = \
ARRAY_SIZE(_name##_parent_names), \
.flags = _flags,\
};
and then you have something like
#define DEFINE_CLK_IMX(_name, _flags, _foo, _bar) \
static struct clk_imx _name##_hw = {\
.hw = { \
.clk = _name, \
}, \
.foo = _foo,\
.bar = _bar,\
}; \
DEFINE_CLK_GENERIC(_name, _flags, clk_imx_ops)
Andrew
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v5 3/4] clk: introduce the common clock framework
Assuming that some day OMAP code can be refactored to allow for lazy (or at least initcall-based) registration of clocks then perhaps your suggestion can take root. Which leads me to this question: are there any other platforms out there that require the level of expose to struct clk present in this patchset? OMAP does, for now, but if that changes then I need to know if others require this as well. Hi Mike For kirkwood, i use static clk's for all but my root clock. I cannot statically know the rate of the root clock, so i have to determine it at boot time using heuristics, PCI ID, etc. I used statics thinking it would be less code. No idea if it actually is, and there is nothing stopping me moving to creating the clocks after creating the root clock. One comment i have about the current static clks. I completely missing you need to call __clk_init() on them and so ended up with lots of division by zero errors, since they did not have a parent, and so the code seemed not be to able to determine the rate. So 1) Please add __clk_init() to the documentation in the section about static clocks. 2) Should maybe the name change? It seems strange having to call a __X() function. If this is a function which is supposed to be used, drop the __. Maybe clk_static_register()? 3) Maybe, if the parent is missing, clk_get_rate() should return an error? Andrew ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v5 4/4] clk: basic clock hardware types
On Sun, Mar 04, 2012 at 04:30:08PM -0800, Turquette, Mike wrote:
On Sun, Mar 4, 2012 at 9:42 AM, Andrew Lunn and...@lunn.ch wrote:
On Sat, Mar 03, 2012 at 12:29:01AM -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.
Hi Mike
These basic clocks don't allow the use of prepare/unprepare, from the
side of the clock provider. I think i need this.
This is an interesting point and might help us nail down exactly what
we expect from clk_prepare/clk_unprepare.
The Orion Kirkwood SoC has clock gating for most of its on chip
peripherals, which the other older Orion devices don't have. The SATA
and PCIe also allows the PHY to be shut down, again which older Orion
devices don't have. The current code links the clock and the PHY
together, shutting both down are the same time. So i would like to
perform the PHY shutdown in the unprepare() function of the clk
driver.
Do you feel it is The Right Thing to enable/disable the phy from
clk_prepare/clk_unprepare?
Humm, not sure yet. I don't know all the different possibilities,
which is why i tried to describe my use case, rather than just assume
i need prepare/unprepare.
I also realized i did not explain my use case properly.
At boot, uboot is turning on various clocks and SATA/PCIe PHYs etc, in
order to get the device booted. Linux takes over, and the
Orion/kirkwood board files, ask the kirkwood or generic Orion code to
create platform_data structures for the different devices that the
board uses. The kirkwood code keeps a bitmap of devices for which it
creates platform data for which there is a gated clock. Then in a
lateinit call, it turns on clocks which are needed, and also turns off
clocks which are no longer needed, because the board did not ask for a
driver binding for that device. If it turns off a SATA or PCIe clock,
it also turns off the PHY associated with it.
So we are talking about turning off hardware for which there is no
driver. This seems to exclude pm_runtime_get(_sync), which is about
hardware with drivers.
We touched on this subject a couple of months ago, at least with
respect to clocks. You said that is what the flag CLK_IGNORE_UNUSED
will be used for. In a lateinit call, you plan to iterate over all
clocks and turn off any which don't have CLK_IGNORE_UNUSED and have
not been enabled. I assume you will call both disable() and
unprepare(), and if i've can put code into the unprepare to turn the
PHY off, all is good.
Is allowing to pass prepare/unprepare functions to basic clocks
something you want to support? If i prepare a patch would you consider
it?
My original instinct was no. The simple gate clock was always
supposed to be simple and if you need more than it provides then it
might be best for your platform to implement a specific clock type.
Especially since the purpose of clk_prepare is still up in the air.
I think i can wrap your simple gate clock, to make my complex gate
clock. What would help is if you would EXPORT_SYMBOL_GPL
clk_gate_enable() and clk_gate_disable(), since they do exactly what i
want. I can then build my own clk_ops structure, with my own
unprepare() function. I would probably use DEFINE_CLK_GATE as is, and
then at run time, before calling __clk_init() overwrite the .ops with
my own version.
However, if others have the need for {un}prepare(), it would be good
to have a generic solution.
Andrew
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v5 4/4] clk: basic clock hardware types
I think i can wrap your simple gate clock, to make my complex gate clock. What would help is if you would EXPORT_SYMBOL_GPL clk_gate_enable() and clk_gate_disable(), since they do exactly what i want. I can then build my own clk_ops structure, with my own unprepare() function. I would probably use DEFINE_CLK_GATE as is, and then at run time, before calling __clk_init() overwrite the .ops with my own version. Maybe I don't get your point, but clk_unprepare should be used when you have to sleep to disable your clock. When clk_gate_disable() is exactly why do you want to use clk_unprepare instead of clk_disable? I'm trying to avoid having to implement a new clock provider. The whole point of the generic clk code is to consolidate code. It seems silly to create a new clk provider which is 95% identical to Mike's gated provider, if i can avoid it. If i stuff it into clk_disable(), it means i cannot use the basic gate clock Mike provides in the generic clock framework. Which is a shame, since it does exactly what i want in terms of gating the clock. If i can use unprepare(), which basic gate does not use, i can use Mikes code, and just extend it. It is there, it is unused, so why not use it? Andrew ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v5 4/4] clk: basic clock hardware types
+#define DEFINE_CLK_GATE(_name, _parent_name, _parent_ptr,\
+ _flags, _reg, _bit_idx, \
+ _gate_flags, _lock) \
+ static struct clk _name;\
+ static char *_name##_parent_names[] = { \
+ _parent_name, \
+ }; \
+ static struct clk *_name##_parents[] = {\
+ _parent_ptr,\
+ }; \
+ static struct clk_gate _name##_hw = { \
+ .hw = { \
+ .clk = _name, \
+ }, \
+ .reg = _reg,\
+ .bit_idx = _bit_idx,\
+ .flags = _gate_flags\
+ .lock = _lock, \
+ }; \
+ static struct clk _name = { \
+ .name = #_name, \
+ .ops = clk_gate_ops, \
+ .hw = _name##_hw.hw, \
+ .parent_names = _name##_parent_names, \
+ .num_parents = \
+ ARRAY_SIZE(_name##parent_names),\
Hi Mike
This should be _name##_parent_names, i.e. you are missing a _.
With this and the previous change, i get something which at least
compiles...
Andrew
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH] clk: Fix compile errors in DEFINE_CLK_GATE
From 71e9a676b2b2f0dc2bb0cc395e8325cf38f4808b Mon Sep 17 00:00:00 2001
From: Andrew Lunn and...@lunn.ch
Date: Sun, 4 Mar 2012 16:31:14 +0100
Subject: [PATCH] [clk] Fix compile errors in DEFINE_CLK_GATE()
Signed-off-by: Andrew Lunn and...@lunn.ch
---
include/linux/clk-private.h |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h
index d06e6fb..9d5c4b1 100644
--- a/include/linux/clk-private.h
+++ b/include/linux/clk-private.h
@@ -95,7 +95,7 @@ extern struct clk_ops clk_gate_ops;
}, \
.reg = _reg,\
.bit_idx = _bit_idx,\
- .flags = _gate_flags\
+ .flags = _gate_flags, \
.lock = _lock, \
}; \
static struct clk _name = { \
@@ -104,7 +104,7 @@ extern struct clk_ops clk_gate_ops;
.hw = _name##_hw.hw, \
.parent_names = _name##_parent_names, \
.num_parents = \
- ARRAY_SIZE(_name##parent_names),\
+ ARRAY_SIZE(_name##_parent_names), \
.parents = _name##_parents, \
.flags = _flags,\
};
--
1.7.2.5
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v5 4/4] clk: basic clock hardware types
On Sat, Mar 03, 2012 at 12:29:01AM -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. Hi Mike These basic clocks don't allow the use of prepare/unprepare, from the side of the clock provider. I think i need this. The Orion Kirkwood SoC has clock gating for most of its on chip peripherals, which the other older Orion devices don't have. The SATA and PCIe also allows the PHY to be shut down, again which older Orion devices don't have. The current code links the clock and the PHY together, shutting both down are the same time. So i would like to perform the PHY shutdown in the unprepare() function of the clk driver. Is allowing to pass prepare/unprepare functions to basic clocks something you want to support? If i prepare a patch would you consider it? Thanks Andrew ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v3 4/5] clk: basic gateable and fixed-rate clks
Hi Mike +int clk_register_gate(struct device *dev, const char *name, unsigned long flags, + struct clk *fixed_parent, void __iomem *reg, u8 bit_idx, +int set_to_enable) + How do you suggest handling gated clocks which are already running when calling the register function? On my kirkwood bases system, the bootloader has already turned on a number of clocks. It does not seem right to start messing with clk-enable_count and clk-prepare_count. Could clk_register_gate() have one more parameter, a bool indicating running? The kirkwood mach code keeps a bitmap of which platform_data init functions are called from the board file. In a late_initcall function it then enables and disables clocks as needed. What i was thinking is i can ask the hardware what clocks are already running before i register them and register them as running/not running. Then let the driver probe functions use the API to enable clocks which are really needed. In a late_initcall function, i would then call clk_disable(), clk_unprepare() on clocks which the boot loader started, thus turning them off if no driver has claimed them. Is this how you envisage it working? Thanks Andrew ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v3 4/5] clk: basic gateable and fixed-rate clks
I don't like this approach. If the bool for a particular clk is statically defined then it could be wrong (bootloader/kernel mismatch). I've been thinking of adding a clk-ops-init callback in clk_init, which is defined for a platform to use however the author sees fit. There have been a few cases where it would be nice to init a clk only once, when it is registered. That code could also handle detecting if a clk is enabled or not. On the other hand we already have a .get_parent callback which is only good for figuring out which parent a mux clk is using... maybe a .is_enabled or .get_enabled would be a good idea which also served the purpose of dynamically determining whether a clk is disabled or running. In general though I think we should try to keep the solution in the core code, not by having platform code pass in a bool. Hi Mike How about simply reading the bit in the control register? You are already doing a read/modify/write when enabling/disabling the clock, so it seems reasonably safe to assume the read gives you the current state. For those platforms which this does not work, you could add another flag disabling this read to get the initial state. Andrew ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
