Per-user clock constraints
Subsystems (15)
v7 cover Per-user clock constraints
Hi,
in this v7 of the patchset I have only rebased on top of 3.17rc1, with no other changes. I have had to do a fair amount of fixing due to the rebase, more details below. Follows the original cover letter blurb:
I'm retaking Rabin's patches [0] for splitting the clk API in two: one API for clk consumers and another for providers. The consumer API uses a clk structure that just keeps track of the consumer and has a reference to the actual clk_core struct, which is used internally.
I have kept a patch from Rabin that aims to aid in debugging nested enable/disable calls, though my personal aim is to allow more than one consumer to influence the final, effective frequency rate. For now this is limited to setting floor and ceiling constraints, with the short-term aim of allowing devfreq and thermal drivers to set floor and ceiling frequencies on the memory clock, respectively.
For those functions in the consumer clk API that were called from providers, I have added variants to clk-provider.h that are the same only that accept a clk_core instead. These functions are prefixed with clk_provider_.
Patch 1/8 just adds a bunch of defines with the goal of having all the renames in their own commit while preserving git-bisectability, with patch 5/8 containing the rename itself as generated by the Coccinelle script in [1]. Patches 2/8 and 3/8 remove an instance of a clock consumer using what is currently provider-only API. Patch 4/8 is needed because sound/soc/mxs/mxs-saif.c calls both the consumer and the provider API. The actual implementation of the API split comes in patch 6/8. I will be happy to organize the refactoring differently if anybody has a better idea.
Patch 7/8 warns when there's an unbalanced usage of the enable and disable APIs, and patch 8/8 adds the API for setting floor and ceiling frequencies, per consumer.
[0] http://thread.gmane.org/gmane.linux.kernel/1402006
[1] http://cgit.collabora.com/git/user/tomeu/linux.git/commit/?id=da9c7e34d9
Thanks,
Tomeu
Sebastian Hesselbarth (1):
clk: provide public clk_is_enabled function
Tomeu Vizoso (7):
clk: Add temporary mapping to the existing API
cpufreq: kirkwood: Remove use of the clk provider API
ASoC: mxs-saif: fix mixed use of public and provider clk API
clk: Move all drivers to use internal API
clk: use struct clk only for external API
clk: per-user clock accounting for debug
clk: Add floor and ceiling constraints to clock rates
[detailed diffstat omitted for brevity sake]
256 files changed, 2267 insertions(+), 1732 deletions(-)
Signature · 1 line
1.9.3
PATCH v7 1/8 clk: Add temporary mapping to the existing API
To preserve git-bisectability, add aliases from the future provider API to the existing public API.
Also includes clk-provider.h and clk-dev.h in a few places so the right functions are defined.
---
v7: * Add mappings for clk_notifier_[un]register
* Add more clk-provider.h includes to clk implementations
v4: * Add more clk-provider.h includes to clk implementations
* Add mapping for clk_provider_round_rate
--- arch/arm/mach-omap2/display.c | 1 + arch/arm/mach-omap2/omap_device.c | 1 + arch/arm/mach-shmobile/clock.c | 1 + arch/arm/plat-orion/common.c | 1 + drivers/clk/berlin/bg2.c | 1 + drivers/clk/berlin/bg2q.c | 1 + drivers/clk/clk-conf.c | 1 + drivers/clk/clkdev.c | 1 + drivers/media/platform/exynos4-is/media-dev.c | 1 + include/linux/clk-provider.h | 25 +++++++++++++++++++++++++ include/linux/clk/zynq.h | 1 + 11 files changed, 35 insertions(+)
arch/arm/mach-omap2/display.c +1 -0
@@ -21,6 +21,7 @@#include <linux/platform_device.h> #include <linux/io.h> #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/err.h> #include <linux/delay.h> #include <linux/of.h>
arch/arm/mach-omap2/omap_device.c +1 -0
@@ -32,6 +32,7 @@#include <linux/io.h> #include <linux/clk.h> #include <linux/clkdev.h> +#include <linux/clk-provider.h> #include <linux/pm_runtime.h> #include <linux/of.h> #include <linux/notifier.h>
arch/arm/mach-shmobile/clock.c +1 -0
@@ -24,6 +24,7 @@#ifdef CONFIG_COMMON_CLK #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/clkdev.h> #include "clock.h"
arch/arm/plat-orion/common.c +1 -0
@@ -15,6 +15,7 @@#include <linux/serial_8250.h> #include <linux/ata_platform.h> #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/clkdev.h> #include <linux/mv643xx_eth.h> #include <linux/mv643xx_i2c.h>
drivers/clk/berlin/bg2.c +1 -0
@@ -19,6 +19,7 @@#include <linux/clk.h> #include <linux/clk-provider.h> +#include <linux/clkdev.h> #include <linux/kernel.h> #include <linux/of.h> #include <linux/of_address.h>
drivers/clk/berlin/bg2q.c +1 -0
@@ -19,6 +19,7 @@#include <linux/clk.h> #include <linux/clk-provider.h> +#include <linux/clkdev.h> #include <linux/kernel.h> #include <linux/of.h> #include <linux/of_address.h>
drivers/clk/clk-conf.c +1 -0
@@ -8,6 +8,7 @@*/ #include <linux/clk.h> +#include <linux/clkdev.h> #include <linux/clk-provider.h> #include <linux/clk/clk-conf.h> #include <linux/device.h>
drivers/clk/clkdev.c +1 -0
@@ -18,6 +18,7 @@#include <linux/string.h> #include <linux/mutex.h> #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/clkdev.h> #include <linux/of.h>
drivers/media/platform/exynos4-is/media-dev.c +1 -0
@@ -13,6 +13,7 @@#include <linux/bug.h> #include <linux/clk.h> #include <linux/clk-provider.h> +#include <linux/clkdev.h> #include <linux/device.h> #include <linux/errno.h> #include <linux/i2c.h>
include/linux/clk-provider.h +25 -0
@@ -16,6 +16,31 @@#ifdef CONFIG_COMMON_CLK +/* Temporarily map the to-be-added API to the old API, just so stuff compiles */ +#define clk_core clk + +#define __clk_create_clk + +#define clk_provider_get clk_get +#define clk_provider_get_sys clk_get_sys +#define devm_clk_provider_get devm_clk_get +#define of_clk_provider_get of_clk_get +#define of_clk_provider_get_by_name of_clk_get_by_name + +#define clk_provider_set_rate clk_set_rate +#define clk_provider_get_rate clk_get_rate +#define clk_provider_round_rate clk_round_rate +#define clk_provider_set_parent clk_set_parent +#define clk_provider_get_parent clk_get_parent +#define clk_provider_prepare clk_prepare +#define clk_provider_unprepare clk_unprepare +#define clk_provider_enable clk_enable +#define clk_provider_disable clk_disable +#define clk_provider_prepare_enable clk_prepare_enable +#define clk_provider_disable_unprepare clk_unprepare +#define clk_provider_notifier_register clk_notifier_register +#define clk_provider_notifier_unregister clk_notifier_unregister +/* * flags used across common struct clk. these flags should only affect the * top-level framework. custom flags for dealing with hardware specifics
include/linux/clk/zynq.h +1 -0
@@ -21,6 +21,7 @@#define __LINUX_CLK_ZYNQ_H_#include <linux/spinlock.h> +#include <linux/clk-provider.h> void zynq_clock_init (void );
Signature · 1 line
1.9.3
PATCH v7 1/8 clk: Add temporary mapping to the existing API
Quoting Tomeu Vizoso (2014-08-18 08:30:27)
Quoted 7 lines
To preserve git-bisectability, add aliases from the future provider API to the existing public API.
Also includes
clk-provider.handclk-dev.hin a few places so the right functions are defined.Signed-off-by Tomeu Vizoso <[email protected]>
Cc'ing Tony for the OMAP2+ parts, Simon & Magnus for the SHMobile parts, Jason & Andrew for the Orion parts, Mauro & Kukjin for the Exynos parts.
This change is super trivial but it's best not to touch these files without a heads-up for the owners.
Regards,
Mike
Quoted 180 lines
---
v7: * Add mappings for clk_notifier_[un]register
* Add moreclk-provider.hincludes to clk implementationsv4: * Add more
clk-provider.hincludes to clk implementations
* Add mapping forclk_provider_round_rate--- arch/arm/mach-omap2/display.c | 1 + arch/arm/mach-omap2/omap_device.c | 1 + arch/arm/mach-shmobile/clock.c | 1 + arch/arm/plat-orion/common.c | 1 + drivers/clk/berlin/bg2.c | 1 + drivers/clk/berlin/bg2q.c | 1 + drivers/clk/clk-conf.c | 1 + drivers/clk/clkdev.c | 1 + drivers/media/platform/exynos4-is/media-dev.c | 1 + include/linux/clk-provider.h | 25 +++++++++++++++++++++++++ include/linux/clk/zynq.h | 1 + 11 files changed, 35 insertions(+)arch/arm/mach-omap2/display.c +1
-0@@ -21,6 +21,7 @@#include <linux/platform_device.h> #include <linux/io.h> #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/err.h> #include <linux/delay.h> #include <linux/of.h> arch/arm/mach-omap2/omap_device.c +1
-0@@ -32,6 +32,7 @@#include <linux/io.h> #include <linux/clk.h> #include <linux/clkdev.h> +#include <linux/clk-provider.h> #include <linux/pm_runtime.h> #include <linux/of.h> #include <linux/notifier.h> arch/arm/mach-shmobile/clock.c +1
-0@@ -24,6 +24,7 @@#ifdef CONFIG_COMMON_CLK #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/clkdev.h> #include "clock.h" arch/arm/plat-orion/common.c +1
-0@@ -15,6 +15,7 @@#include <linux/serial_8250.h> #include <linux/ata_platform.h> #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/clkdev.h> #include <linux/mv643xx_eth.h> #include <linux/mv643xx_i2c.h> drivers/clk/berlin/bg2.c +1
-0@@ -19,6 +19,7 @@#include <linux/clk.h> #include <linux/clk-provider.h> +#include <linux/clkdev.h> #include <linux/kernel.h> #include <linux/of.h> #include <linux/of_address.h> drivers/clk/berlin/bg2q.c +1
-0@@ -19,6 +19,7 @@#include <linux/clk.h> #include <linux/clk-provider.h> +#include <linux/clkdev.h> #include <linux/kernel.h> #include <linux/of.h> #include <linux/of_address.h> drivers/clk/clk-conf.c +1
-0@@ -8,6 +8,7 @@*/ #include <linux/clk.h> +#include <linux/clkdev.h> #include <linux/clk-provider.h> #include <linux/clk/clk-conf.h> #include <linux/device.h> drivers/clk/clkdev.c +1
-0@@ -18,6 +18,7 @@#include <linux/string.h> #include <linux/mutex.h> #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/clkdev.h> #include <linux/of.h> drivers/media/platform/exynos4-is/media-dev.c +1
-0@@ -13,6 +13,7 @@#include <linux/bug.h> #include <linux/clk.h> #include <linux/clk-provider.h> +#include <linux/clkdev.h> #include <linux/device.h> #include <linux/errno.h> #include <linux/i2c.h> include/linux/clk-provider.h +25
-0@@ -16,6 +16,31 @@#ifdef CONFIG_COMMON_CLK +/* Temporarily map the to-be-added API to the old API, just so stuff compiles */ +#define clk_core clk + +#define __clk_create_clk + +#define clk_provider_get clk_get +#define clk_provider_get_sys clk_get_sys +#define devm_clk_provider_get devm_clk_get +#define of_clk_provider_get of_clk_get +#define of_clk_provider_get_by_name of_clk_get_by_name + +#define clk_provider_set_rate clk_set_rate +#define clk_provider_get_rate clk_get_rate +#define clk_provider_round_rate clk_round_rate +#define clk_provider_set_parent clk_set_parent +#define clk_provider_get_parent clk_get_parent +#define clk_provider_prepare clk_prepare +#define clk_provider_unprepare clk_unprepare +#define clk_provider_enable clk_enable +#define clk_provider_disable clk_disable +#define clk_provider_prepare_enable clk_prepare_enable +#define clk_provider_disable_unprepare clk_unprepare +#define clk_provider_notifier_register clk_notifier_register +#define clk_provider_notifier_unregister clk_notifier_unregister +/* * flags used across common struct clk. these flags should only affect the * top-level framework. custom flags for dealing with hardware specifics include/linux/clk/zynq.h +1
-0@@ -21,6 +21,7 @@#define __LINUX_CLK_ZYNQ_H_#include <linux/spinlock.h> +#include <linux/clk-provider.h> void zynq_clock_init (void );Signature · 1 line
1.9.3
Re: [PATCH v7 1/8] clk: Add temporary mapping to the existing API
Quoted 7 lines · Mike Turquette
Quoting Tomeu Vizoso (2014-08-18 08:30:27)
Quoted 6 lines
To preserve git-bisectability, add aliases from the future provider API to the existing public API.
Also includes
clk-provider.handclk-dev.hin a few places so the right functions are defined.
Quoted 7 lines
Quoted 1 line
Signed-off-by Tomeu Vizoso <[email protected]>Cc'ing Tony for the OMAP2+ parts, Simon & Magnus for the SHMobile parts, Jason & Andrew for the Orion parts, Mauro & Kukjin for the Exynos parts.
This change is super trivial but it's best not to touch these files without a heads-up for the owners.
As long as it compiles omap2plus_defconfig it seems safe to me:
Quoted 179 lines
Quoted 179 lines
---
v7: * Add mappings for clk_notifier_[un]register
* Add moreclk-provider.hincludes to clk implementationsv4: * Add more
clk-provider.hincludes to clk implementations
* Add mapping forclk_provider_round_rate--- arch/arm/mach-omap2/display.c | 1 + arch/arm/mach-omap2/omap_device.c | 1 + arch/arm/mach-shmobile/clock.c | 1 + arch/arm/plat-orion/common.c | 1 + drivers/clk/berlin/bg2.c | 1 + drivers/clk/berlin/bg2q.c | 1 + drivers/clk/clk-conf.c | 1 + drivers/clk/clkdev.c | 1 + drivers/media/platform/exynos4-is/media-dev.c | 1 + include/linux/clk-provider.h | 25 +++++++++++++++++++++++++ include/linux/clk/zynq.h | 1 + 11 files changed, 35 insertions(+)arch/arm/mach-omap2/display.c +1
-0@@ -21,6 +21,7 @@#include <linux/platform_device.h> #include <linux/io.h> #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/err.h> #include <linux/delay.h> #include <linux/of.h> arch/arm/mach-omap2/omap_device.c +1
-0@@ -32,6 +32,7 @@#include <linux/io.h> #include <linux/clk.h> #include <linux/clkdev.h> +#include <linux/clk-provider.h> #include <linux/pm_runtime.h> #include <linux/of.h> #include <linux/notifier.h> arch/arm/mach-shmobile/clock.c +1
-0@@ -24,6 +24,7 @@#ifdef CONFIG_COMMON_CLK #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/clkdev.h> #include "clock.h" arch/arm/plat-orion/common.c +1
-0@@ -15,6 +15,7 @@#include <linux/serial_8250.h> #include <linux/ata_platform.h> #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/clkdev.h> #include <linux/mv643xx_eth.h> #include <linux/mv643xx_i2c.h> drivers/clk/berlin/bg2.c +1
-0@@ -19,6 +19,7 @@#include <linux/clk.h> #include <linux/clk-provider.h> +#include <linux/clkdev.h> #include <linux/kernel.h> #include <linux/of.h> #include <linux/of_address.h> drivers/clk/berlin/bg2q.c +1
-0@@ -19,6 +19,7 @@#include <linux/clk.h> #include <linux/clk-provider.h> +#include <linux/clkdev.h> #include <linux/kernel.h> #include <linux/of.h> #include <linux/of_address.h> drivers/clk/clk-conf.c +1
-0@@ -8,6 +8,7 @@*/ #include <linux/clk.h> +#include <linux/clkdev.h> #include <linux/clk-provider.h> #include <linux/clk/clk-conf.h> #include <linux/device.h> drivers/clk/clkdev.c +1
-0@@ -18,6 +18,7 @@#include <linux/string.h> #include <linux/mutex.h> #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/clkdev.h> #include <linux/of.h> drivers/media/platform/exynos4-is/media-dev.c +1
-0@@ -13,6 +13,7 @@#include <linux/bug.h> #include <linux/clk.h> #include <linux/clk-provider.h> +#include <linux/clkdev.h> #include <linux/device.h> #include <linux/errno.h> #include <linux/i2c.h> include/linux/clk-provider.h +25
-0@@ -16,6 +16,31 @@#ifdef CONFIG_COMMON_CLK +/* Temporarily map the to-be-added API to the old API, just so stuff compiles */ +#define clk_core clk + +#define __clk_create_clk + +#define clk_provider_get clk_get +#define clk_provider_get_sys clk_get_sys +#define devm_clk_provider_get devm_clk_get +#define of_clk_provider_get of_clk_get +#define of_clk_provider_get_by_name of_clk_get_by_name + +#define clk_provider_set_rate clk_set_rate +#define clk_provider_get_rate clk_get_rate +#define clk_provider_round_rate clk_round_rate +#define clk_provider_set_parent clk_set_parent +#define clk_provider_get_parent clk_get_parent +#define clk_provider_prepare clk_prepare +#define clk_provider_unprepare clk_unprepare +#define clk_provider_enable clk_enable +#define clk_provider_disable clk_disable +#define clk_provider_prepare_enable clk_prepare_enable +#define clk_provider_disable_unprepare clk_unprepare +#define clk_provider_notifier_register clk_notifier_register +#define clk_provider_notifier_unregister clk_notifier_unregister +/* * flags used across common struct clk. these flags should only affect the * top-level framework. custom flags for dealing with hardware specifics include/linux/clk/zynq.h +1
-0@@ -21,6 +21,7 @@#define __LINUX_CLK_ZYNQ_H_#include <linux/spinlock.h> +#include <linux/clk-provider.h> void zynq_clock_init (void );Signature · 1 line
1.9.3
Jason Cooper
Acked-byRe: [PATCH v7 1/8] clk: Add temporary mapping to the existing API
Quoted 13 lines · Tony Lindgren
Quoted 12 lines · Mike Turquette
Quoting Tomeu Vizoso (2014-08-18 08:30:27)
Quoted 7 lines
To preserve git-bisectability, add aliases from the future provider API to the existing public API.
Also includes
clk-provider.handclk-dev.hin a few places so the right functions are defined.Signed-off-by Tomeu Vizoso <[email protected]>Cc'ing Tony for the OMAP2+ parts, Simon & Magnus for the SHMobile parts, Jason & Andrew for the Orion parts, Mauro & Kukjin for the Exynos parts.
Quoted 6 lines
Quoted 2 lines
This change is super trivial but it's best not to touch these files without a heads-up for the owners.
As long as it compiles
omap2plus_defconfigit seems safe to me:Acked-by Tony Lindgren <[email protected]>
Same here for orion5x_defconfig, dove_defconfig, and mv78xx_defconfig.
Also, added Sebastian to the Cc as he's the maintainer of mach-berlin.
thx,
Jason.
Quoted 180 lines
Quoted 180 lines
Quoted 180 lines
---
v7: * Add mappings for clk_notifier_[un]register
* Add moreclk-provider.hincludes to clk implementationsv4: * Add more
clk-provider.hincludes to clk implementations
* Add mapping forclk_provider_round_rate--- arch/arm/mach-omap2/display.c | 1 + arch/arm/mach-omap2/omap_device.c | 1 + arch/arm/mach-shmobile/clock.c | 1 + arch/arm/plat-orion/common.c | 1 + drivers/clk/berlin/bg2.c | 1 + drivers/clk/berlin/bg2q.c | 1 + drivers/clk/clk-conf.c | 1 + drivers/clk/clkdev.c | 1 + drivers/media/platform/exynos4-is/media-dev.c | 1 + include/linux/clk-provider.h | 25 +++++++++++++++++++++++++ include/linux/clk/zynq.h | 1 + 11 files changed, 35 insertions(+)arch/arm/mach-omap2/display.c +1
-0@@ -21,6 +21,7 @@#include <linux/platform_device.h> #include <linux/io.h> #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/err.h> #include <linux/delay.h> #include <linux/of.h> arch/arm/mach-omap2/omap_device.c +1
-0@@ -32,6 +32,7 @@#include <linux/io.h> #include <linux/clk.h> #include <linux/clkdev.h> +#include <linux/clk-provider.h> #include <linux/pm_runtime.h> #include <linux/of.h> #include <linux/notifier.h> arch/arm/mach-shmobile/clock.c +1
-0@@ -24,6 +24,7 @@#ifdef CONFIG_COMMON_CLK #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/clkdev.h> #include "clock.h" arch/arm/plat-orion/common.c +1
-0@@ -15,6 +15,7 @@#include <linux/serial_8250.h> #include <linux/ata_platform.h> #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/clkdev.h> #include <linux/mv643xx_eth.h> #include <linux/mv643xx_i2c.h> drivers/clk/berlin/bg2.c +1
-0@@ -19,6 +19,7 @@#include <linux/clk.h> #include <linux/clk-provider.h> +#include <linux/clkdev.h> #include <linux/kernel.h> #include <linux/of.h> #include <linux/of_address.h> drivers/clk/berlin/bg2q.c +1
-0@@ -19,6 +19,7 @@#include <linux/clk.h> #include <linux/clk-provider.h> +#include <linux/clkdev.h> #include <linux/kernel.h> #include <linux/of.h> #include <linux/of_address.h> drivers/clk/clk-conf.c +1
-0@@ -8,6 +8,7 @@*/ #include <linux/clk.h> +#include <linux/clkdev.h> #include <linux/clk-provider.h> #include <linux/clk/clk-conf.h> #include <linux/device.h> drivers/clk/clkdev.c +1
-0@@ -18,6 +18,7 @@#include <linux/string.h> #include <linux/mutex.h> #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/clkdev.h> #include <linux/of.h> drivers/media/platform/exynos4-is/media-dev.c +1
-0@@ -13,6 +13,7 @@#include <linux/bug.h> #include <linux/clk.h> #include <linux/clk-provider.h> +#include <linux/clkdev.h> #include <linux/device.h> #include <linux/errno.h> #include <linux/i2c.h> include/linux/clk-provider.h +25
-0@@ -16,6 +16,31 @@#ifdef CONFIG_COMMON_CLK +/* Temporarily map the to-be-added API to the old API, just so stuff compiles */ +#define clk_core clk + +#define __clk_create_clk + +#define clk_provider_get clk_get +#define clk_provider_get_sys clk_get_sys +#define devm_clk_provider_get devm_clk_get +#define of_clk_provider_get of_clk_get +#define of_clk_provider_get_by_name of_clk_get_by_name + +#define clk_provider_set_rate clk_set_rate +#define clk_provider_get_rate clk_get_rate +#define clk_provider_round_rate clk_round_rate +#define clk_provider_set_parent clk_set_parent +#define clk_provider_get_parent clk_get_parent +#define clk_provider_prepare clk_prepare +#define clk_provider_unprepare clk_unprepare +#define clk_provider_enable clk_enable +#define clk_provider_disable clk_disable +#define clk_provider_prepare_enable clk_prepare_enable +#define clk_provider_disable_unprepare clk_unprepare +#define clk_provider_notifier_register clk_notifier_register +#define clk_provider_notifier_unregister clk_notifier_unregister +/* * flags used across common struct clk. these flags should only affect the * top-level framework. custom flags for dealing with hardware specifics include/linux/clk/zynq.h +1
-0@@ -21,6 +21,7 @@#define __LINUX_CLK_ZYNQ_H_#include <linux/spinlock.h> +#include <linux/clk-provider.h> void zynq_clock_init (void );Signature · 1 line
1.9.3
Simon Horman
Acked-byRe: [PATCH v7 1/8] clk: Add temporary mapping to the existing API
Quoted 19 lines · Jason Cooper
Quoted 18 lines · Tony Lindgren
Quoted 7 lines · Mike Turquette
Quoting Tomeu Vizoso (2014-08-18 08:30:27)
Quoted 6 lines
To preserve git-bisectability, add aliases from the future provider API to the existing public API.
Also includes
clk-provider.handclk-dev.hin a few places so the right functions are defined.Quoted 7 lines
Quoted 1 line
Signed-off-by Tomeu Vizoso <[email protected]>Cc'ing Tony for the OMAP2+ parts, Simon & Magnus for the SHMobile parts, Jason & Andrew for the Orion parts, Mauro & Kukjin for the Exynos parts.
This change is super trivial but it's best not to touch these files without a heads-up for the owners.
As long as it compiles
omap2plus_defconfigit seems safe to me:
Quoted 7 lines
Quoted 1 line
Acked-by Tony Lindgren <[email protected]>Same here for
orion5x_defconfig,dove_defconfig, andmv78xx_defconfig.Acked-by Jason Cooper <[email protected]>Also, added Sebastian to the Cc as he's the maintainer of mach-berlin.
I will add my usual 2c worth about global changes vs adding things to infrastructure, updating the platforms, then removing things from infrastructure, in separate patches picked up by the relevant maintainers. But if everyone else is happy with doing things this way I won't say no.
Ideally I'd like the shmobile portion patch to apply cleanly to linux-next (it looks like it should) and for the following to still compile:ape6evm_defconfig, armadillo800eva_defconfig, bockw_defconfig, koelsch_defconfig, kzm9g_defconfig, lager_defconfig, mackerel_defconfig, marzen_defconfig, shmobile_defconfig.
With all that said and done:
Quoted 185 lines
thx,
Jason.
Quoted 180 lines
Quoted 180 lines
Quoted 180 lines
---
v7: * Add mappings for clk_notifier_[un]register
* Add moreclk-provider.hincludes to clk implementationsv4: * Add more
clk-provider.hincludes to clk implementations
* Add mapping forclk_provider_round_rate--- arch/arm/mach-omap2/display.c | 1 + arch/arm/mach-omap2/omap_device.c | 1 + arch/arm/mach-shmobile/clock.c | 1 + arch/arm/plat-orion/common.c | 1 + drivers/clk/berlin/bg2.c | 1 + drivers/clk/berlin/bg2q.c | 1 + drivers/clk/clk-conf.c | 1 + drivers/clk/clkdev.c | 1 + drivers/media/platform/exynos4-is/media-dev.c | 1 + include/linux/clk-provider.h | 25 +++++++++++++++++++++++++ include/linux/clk/zynq.h | 1 + 11 files changed, 35 insertions(+)arch/arm/mach-omap2/display.c +1
-0@@ -21,6 +21,7 @@#include <linux/platform_device.h> #include <linux/io.h> #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/err.h> #include <linux/delay.h> #include <linux/of.h> arch/arm/mach-omap2/omap_device.c +1
-0@@ -32,6 +32,7 @@#include <linux/io.h> #include <linux/clk.h> #include <linux/clkdev.h> +#include <linux/clk-provider.h> #include <linux/pm_runtime.h> #include <linux/of.h> #include <linux/notifier.h> arch/arm/mach-shmobile/clock.c +1
-0@@ -24,6 +24,7 @@#ifdef CONFIG_COMMON_CLK #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/clkdev.h> #include "clock.h" arch/arm/plat-orion/common.c +1
-0@@ -15,6 +15,7 @@#include <linux/serial_8250.h> #include <linux/ata_platform.h> #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/clkdev.h> #include <linux/mv643xx_eth.h> #include <linux/mv643xx_i2c.h> drivers/clk/berlin/bg2.c +1
-0@@ -19,6 +19,7 @@#include <linux/clk.h> #include <linux/clk-provider.h> +#include <linux/clkdev.h> #include <linux/kernel.h> #include <linux/of.h> #include <linux/of_address.h> drivers/clk/berlin/bg2q.c +1
-0@@ -19,6 +19,7 @@#include <linux/clk.h> #include <linux/clk-provider.h> +#include <linux/clkdev.h> #include <linux/kernel.h> #include <linux/of.h> #include <linux/of_address.h> drivers/clk/clk-conf.c +1
-0@@ -8,6 +8,7 @@*/ #include <linux/clk.h> +#include <linux/clkdev.h> #include <linux/clk-provider.h> #include <linux/clk/clk-conf.h> #include <linux/device.h> drivers/clk/clkdev.c +1
-0@@ -18,6 +18,7 @@#include <linux/string.h> #include <linux/mutex.h> #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/clkdev.h> #include <linux/of.h> drivers/media/platform/exynos4-is/media-dev.c +1
-0@@ -13,6 +13,7 @@#include <linux/bug.h> #include <linux/clk.h> #include <linux/clk-provider.h> +#include <linux/clkdev.h> #include <linux/device.h> #include <linux/errno.h> #include <linux/i2c.h> include/linux/clk-provider.h +25
-0@@ -16,6 +16,31 @@#ifdef CONFIG_COMMON_CLK +/* Temporarily map the to-be-added API to the old API, just so stuff compiles */ +#define clk_core clk + +#define __clk_create_clk + +#define clk_provider_get clk_get +#define clk_provider_get_sys clk_get_sys +#define devm_clk_provider_get devm_clk_get +#define of_clk_provider_get of_clk_get +#define of_clk_provider_get_by_name of_clk_get_by_name + +#define clk_provider_set_rate clk_set_rate +#define clk_provider_get_rate clk_get_rate +#define clk_provider_round_rate clk_round_rate +#define clk_provider_set_parent clk_set_parent +#define clk_provider_get_parent clk_get_parent +#define clk_provider_prepare clk_prepare +#define clk_provider_unprepare clk_unprepare +#define clk_provider_enable clk_enable +#define clk_provider_disable clk_disable +#define clk_provider_prepare_enable clk_prepare_enable +#define clk_provider_disable_unprepare clk_unprepare +#define clk_provider_notifier_register clk_notifier_register +#define clk_provider_notifier_unregister clk_notifier_unregister +/* * flags used across common struct clk. these flags should only affect the * top-level framework. custom flags for dealing with hardware specifics include/linux/clk/zynq.h +1
-0@@ -21,6 +21,7 @@#define __LINUX_CLK_ZYNQ_H_#include <linux/spinlock.h> +#include <linux/clk-provider.h> void zynq_clock_init (void );Signature · 1 line
1.9.3
Re: [PATCH v7 1/8] clk: Add temporary mapping to the existing API
In reply to Jason CooperQuoted 19 lines · Jason Cooper
Quoted 18 lines · Tony Lindgren
Quoted 7 lines · Mike Turquette
Quoting Tomeu Vizoso (2014-08-18 08:30:27)
Quoted 6 lines
To preserve git-bisectability, add aliases from the future provider API to the existing public API.
Also includes
clk-provider.handclk-dev.hin a few places so the right functions are defined.Quoted 7 lines
Quoted 1 line
Signed-off-by Tomeu Vizoso <[email protected]>Cc'ing Tony for the OMAP2+ parts, Simon & Magnus for the SHMobile parts, Jason & Andrew for the Orion parts, Mauro & Kukjin for the Exynos parts.
This change is super trivial but it's best not to touch these files without a heads-up for the owners.
As long as it compiles
omap2plus_defconfigit seems safe to me:
Quoted 7 lines
Quoted 1 line
Acked-by Tony Lindgren <[email protected]>Same here for
orion5x_defconfig,dove_defconfig, andmv78xx_defconfig.Acked-by Jason Cooper <[email protected]>Also, added Sebastian to the Cc as he's the maintainer of mach-berlin.
If it still compiles for multi_v7,
I'll come back to mach-berlin later this week.
Sebastian
PATCH v7 2/8 clk: provide public clk_is_enabled function
In reply to Tomeu VizosoFrom: Sebastian Hesselbarth <[email protected]>
To determine if a clk has been previously enabled, provide a public clk_is_enabled function. This is especially helpful to check the state of clk-gate without actually changing the state of the gate.
--- drivers/clk/clk.c | 18 ++++++++++++++++++ include/linux/clk.h | 13 +++++++++++++ 2 files changed, 31 insertions(+)
drivers/clk/clk.c +18 -0
@@ -953,6 +953,24 @@ int clk_enable(struct clk *clk)EXPORT_SYMBOL_GPL (clk_enable);/** +* clk_is_enabled - return the enabled state of a clk +* @clk: the clk whose enabled state gets returned +* +* Returns true if the clock is enabled, false otherwise. +*/ +bool clk_is_enabled (struct clk *clk) +{ +bool is_en; + +clk_prepare_lock (); + is_en =__clk_is_enabled (clk); +clk_prepare_unlock (); + +return is_en; +} +EXPORT_SYMBOL_GPL (clk_is_enabled); + +/** * __clk_round_rate - round the given rate for a clk * @clk: round the rate of this clock * @rate: the rate which is to be rounded
include/linux/clk.h +13 -0
@@ -218,6 +218,14 @@ int clk_enable(struct clk *clk);void clk_disable (struct clk *clk);/** +* clk_is_enabled - return the enabled state of a clk +* @clk: the clk whose enabled state gets returned +* +* Returns true if the clock is enabled, false otherwise. +*/ +bool clk_is_enabled (struct clk *clk); + +/** * clk_get_rate - obtain the current clock rate (in Hz) for a clock source. * This is only valid once the clock source has been enabled. * @clk: clock source
@@ -330,6 +338,11 @@ static inline int clk_enable(struct clk *clk)static inline void clk_disable (struct clk *clk) {} +static inline bool clk_is_enabled (struct clk *clk) +{ +return false; +} +static inline unsigned long clk_get_rate (struct clk *clk) {return 0 ;
Signature · 1 line
1.9.3
PATCH v7 3/8 cpufreq: kirkwood: Remove use of the clk provider API
In reply to Tomeu Vizoso--- drivers/cpufreq/kirkwood-cpufreq.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/cpufreq/kirkwood-cpufreq.c +1 -2
@@ -12,7 +12,6 @@#include <linux/kernel.h> #include <linux/module.h> #include <linux/clk.h> -#include <linux/clk-provider.h> #include <linux/cpufreq.h> #include <linux/of_device.h> #include <linux/platform_device.h>
@@ -50,7 +49,7 @@ static struct cpufreq_frequency_table kirkwood_freq_table[] = {static unsigned int kirkwood_cpufreq_get_cpu_frequency (unsigned int cpu) { -if (__clk_is_enabled (priv.powersave_clk)) +if (clk_is_enabled (priv.powersave_clk))return kirkwood_freq_table[1 ].frequency;return kirkwood_freq_table[0 ].frequency; }
Signature · 1 line
1.9.3
A message that is not in the archive (sent off-list or not yet received): 20140820225513.5251.284@quantum
Tomeu Vizoso
Reviewed-byRe: [PATCH v7 3/8] cpufreq: kirkwood: Remove use of the clk provider API
Quoted 39 lines · Mike Turquette
Quoting Tomeu Vizoso (2014-08-18 08:30:29)
Quoted 27 lines
Signed-off-by Tomeu Vizoso <[email protected]>--- drivers/cpufreq/kirkwood-cpufreq.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)drivers/cpufreq/kirkwood-cpufreq.c +1
-3@@ -12,7 +12,6 @@#include <linux/kernel.h> #include <linux/module.h> #include <linux/clk.h> -#include <linux/clk-provider.h> #include <linux/cpufreq.h> #include <linux/of_device.h> #include <linux/platform_device.h> @@ -50,7 +49,7 @@ static struct cpufreq_frequency_table kirkwood_freq_table[] = {static unsigned int kirkwood_cpufreq_get_cpu_frequency (unsigned int cpu) { -if (__clk_is_enabled (priv.powersave_clk)) +if (clk_is_enabled (priv.powersave_clk))return kirkwood_freq_table[1 ].frequency;return kirkwood_freq_table[0 ].frequency; } --Quoted 2 lines
1.9.3
Tomeu,
After taking a second look at
clk_is_enabledand the Kirkwood driver, I would prefer to not implementclk_is_enabled. The main reason is that it is racey, since the clock's status could of course change as soon as as that call completes. Furthermore I am worried that drivers might do something like:
Quoted 7 lines
if (!clk_is_enabled(clk))
clk_enable(clk);Which is crap and the driver should just call
clk_enableany time it needs the clock. To that end I propose to drop "clk: provide publicclk_is_enabledfunction" and replace your update tokirkwood-cpufreq.cwith the following patch. Let me know what you think.
Yep, this looks much better to me. I will replace 2/8 and 3/8 with this one for v8.
Thanks!
Tomeu
Quoted 75 lines
Regards,
MikeFrom
713b46fd66abd26a4e4d584a0b8a2a28c0459a60Mon Sep 17 00:00:00 2001
From: Mike Turquette <[email protected]>
Date: Mon, 18 Aug 2014 17:30:29 +0200
Subject: [PATCH] cpufreq: kirkwood: Remove use of the clk provider APIInstead of checking the clock framework to see if the clock is enabled, track the cpu frequency within the driver as the
powersave_clkis enabled and disabled.Cc Tomeu Vizoso <[email protected]>Cc "Rafael J. Wysocki" <[email protected]>Cc Viresh Kumar <[email protected]>Signed-off-by Mike Turquette <[email protected]>--- drivers/cpufreq/kirkwood-cpufreq.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)drivers/cpufreq/kirkwood-cpufreq.c +6
-4@@ -12,7 +12,6 @@#include <linux/kernel.h> #include <linux/module.h> #include <linux/clk.h> -#include <linux/clk-provider.h> #include <linux/cpufreq.h> #include <linux/of_device.h> #include <linux/platform_device.h> @@ -33,6 +32,8 @@ static struct priv#define STATE_CPU_FREQ 0x01#define STATE_DDR_FREQ 0x02 +static unsigned long cpu_frequency =0 ; +/* * Kirkwood can swap the clock to the CPU between two clocks: * @@ -50,9 +51,7 @@ static struct cpufreq_frequency_table kirkwood_freq_table[] = {static unsigned int kirkwood_cpufreq_get_cpu_frequency (unsigned int cpu) { -if (__clk_is_enabled (priv.powersave_clk)) -return kirkwood_freq_table[1 ].frequency; -return kirkwood_freq_table[0 ].frequency; +return cpu_frequency; } static int kirkwood_cpufreq_target(struct cpufreq_policy *policy,@@ -71,9 +70,11 @@ static int kirkwood_cpufreq_target(struct cpufreq_policy *policy,switch (state) {case STATE_CPU_FREQ :clk_disable (priv.powersave_clk); + cpu_frequency = kirkwood_freq_table[0 ].frequency;break ;case STATE_DDR_FREQ :clk_enable (priv.powersave_clk); + cpu_frequency = kirkwood_freq_table[1 ].frequency;break ; }@@ -133,6 +134,7 @@ static int kirkwood_cpufreq_probe(struct platform_device *pdev)clk_prepare_enable (priv.cpu_clk); kirkwood_freq_table[0 ].frequency =clk_get_rate (priv.cpu_clk) /1000 ; + cpu_frequency = kirkwood_freq_table[0 ].frequency; priv.ddr_clk =of_clk_get_by_name (np,"ddrclk" );if (IS_ERR (priv.ddr_clk)) {
Andrew Lunn
maintainerRe: [PATCH v7 3/8] cpufreq: kirkwood: Remove use of the clk provider API
Quoted 40 lines · Tomeu Vizoso
Quoted 39 lines · Mike Turquette
Quoting Tomeu Vizoso (2014-08-18 08:30:29)
Quoted 27 lines
Signed-off-by Tomeu Vizoso <[email protected]>--- drivers/cpufreq/kirkwood-cpufreq.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)drivers/cpufreq/kirkwood-cpufreq.c +1
-3@@ -12,7 +12,6 @@#include <linux/kernel.h> #include <linux/module.h> #include <linux/clk.h> -#include <linux/clk-provider.h> #include <linux/cpufreq.h> #include <linux/of_device.h> #include <linux/platform_device.h> @@ -50,7 +49,7 @@ static struct cpufreq_frequency_table kirkwood_freq_table[] = {static unsigned int kirkwood_cpufreq_get_cpu_frequency (unsigned int cpu) { -if (__clk_is_enabled (priv.powersave_clk)) +if (clk_is_enabled (priv.powersave_clk))return kirkwood_freq_table[1 ].frequency;return kirkwood_freq_table[0 ].frequency; } --Quoted 2 lines
1.9.3
Tomeu,
After taking a second look at
clk_is_enabledand the Kirkwood driver, I would prefer to not implementclk_is_enabled. The main reason is that it is racey, since the clock's status could of course change as soon as as that call completes. Furthermore I am worried that drivers might do something like:
Quoted 7 lines
Quoted 7 lines
if (!clk_is_enabled(clk))
clk_enable(clk);Which is crap and the driver should just call
clk_enableany time it needs the clock. To that end I propose to drop "clk: provide publicclk_is_enabledfunction" and replace your update tokirkwood-cpufreq.cwith the following patch. Let me know what you think.
Hi Mike, Tomeu
Quoted 1 line
Quoted 1 line
+static unsigned long
cpu_frequency= 0;
This has a problem. You are making an assumption about the initial state. The way the hardware works, is you change the state of the clock and then perform a Wait For Interrupt. Once the hardware has finished adjusting its PLL, it raises an interrupt and things continue.
However, if you don't cause an actual state change, the WFI never returns. If this assumption is wrong, your box is dead the first time it tries to change cpu frequency.
This is why the code reads the hardware register to find the real current state, rather than assume it.
Andrew
Quoted 42 lines
Quoted 29 lines
drivers/cpufreq/kirkwood-cpufreq.c +4
-3+/* * Kirkwood can swap the clock to the CPU between two clocks: * @@ -50,9 +51,7 @@ static struct cpufreq_frequency_table kirkwood_freq_table[] = {static unsigned int kirkwood_cpufreq_get_cpu_frequency (unsigned int cpu) { -if (__clk_is_enabled (priv.powersave_clk)) -return kirkwood_freq_table[1 ].frequency; -return kirkwood_freq_table[0 ].frequency; +return cpu_frequency; } static int kirkwood_cpufreq_target(struct cpufreq_policy *policy,@@ -71,9 +70,11 @@ static int kirkwood_cpufreq_target(struct cpufreq_policy *policy,switch (state) {case STATE_CPU_FREQ :clk_disable (priv.powersave_clk); + cpu_frequency = kirkwood_freq_table[0 ].frequency;break ;case STATE_DDR_FREQ :clk_enable (priv.powersave_clk); + cpu_frequency = kirkwood_freq_table[1 ].frequency;break ; }drivers/cpufreq/kirkwood-cpufreq.c static int kirkwood_cpufreq_probe(struct platform_device *pdev) · line 140
Quoted 7 lines
drivers/cpufreq/kirkwood-cpufreq.c +1
-0@@ -133,6 +134,7 @@ static int kirkwood_cpufreq_probe(struct platform_device *pdev)clk_prepare_enable (priv.cpu_clk); kirkwood_freq_table[0 ].frequency =clk_get_rate (priv.cpu_clk) /1000 ; + cpu_frequency = kirkwood_freq_table[0 ].frequency; priv.ddr_clk =of_clk_get_by_name (np,"ddrclk" );if (IS_ERR (priv.ddr_clk)) {_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v7 3/8] cpufreq: kirkwood: Remove use of the clk provider API
Quoting Andrew Lunn (2014-08-21 06:38:25)
Quoted 51 lines · Tomeu Vizoso
Quoted 40 lines · Mike Turquette
Quoted 39 lines
Quoting Tomeu Vizoso (2014-08-18 08:30:29)
Quoted 27 lines
Signed-off-by Tomeu Vizoso <[email protected]>--- drivers/cpufreq/kirkwood-cpufreq.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)drivers/cpufreq/kirkwood-cpufreq.c +1
-3@@ -12,7 +12,6 @@#include <linux/kernel.h> #include <linux/module.h> #include <linux/clk.h> -#include <linux/clk-provider.h> #include <linux/cpufreq.h> #include <linux/of_device.h> #include <linux/platform_device.h> @@ -50,7 +49,7 @@ static struct cpufreq_frequency_table kirkwood_freq_table[] = {static unsigned int kirkwood_cpufreq_get_cpu_frequency (unsigned int cpu) { -if (__clk_is_enabled (priv.powersave_clk)) +if (clk_is_enabled (priv.powersave_clk))return kirkwood_freq_table[1 ].frequency;return kirkwood_freq_table[0 ].frequency; } --Quoted 2 lines
1.9.3
Tomeu,
After taking a second look at
clk_is_enabledand the Kirkwood driver, I would prefer to not implementclk_is_enabled. The main reason is that it is racey, since the clock's status could of course change as soon as as that call completes. Furthermore I am worried that drivers might do something like:Quoted 7 lines
Quoted 7 lines
if (!clk_is_enabled(clk))
clk_enable(clk);Which is crap and the driver should just call
clk_enableany time it needs the clock. To that end I propose to drop "clk: provide publicclk_is_enabledfunction" and replace your update tokirkwood-cpufreq.cwith the following patch. Let me know what you think.Hi Mike, Tomeu
Quoted 8 lines
Quoted 1 line
Quoted 1 line
+static unsigned long
cpu_frequency= 0;This has a problem. You are making an assumption about the initial state. The way the hardware works, is you change the state of the clock and then perform a Wait For Interrupt. Once the hardware has finished adjusting its PLL, it raises an interrupt and things continue.
Andrew,
Thanks for reviewing. I think my patch is equivalent to the old way of doing things, with one exception that I will address later below.
struct cpufreq_frequency_table kirkwood_freq_table has clock rates initialized to zero, so there is no regression compared to my unsigned long cpu_frequency variable used for tracking the clock rate. Both implementations start with unknown rates in hardware and initialize a variable to zero until that rate can be discovered later on in kirkwood_cpufreq_probe().
kirkwood_cpufreq_get_cpu_frequency() returns the frequency based on the state of the clock. As best I can tell, this clock is only touched by this cpufreq driver and nowhere else, so the driver knows the state of the clock implicitly and doesn't need to read any hardware registers to see if it is enabled or not. Every time we enable or disable the clock we can know the cpu frequency.
Quoted 4 lines
However, if you don't cause an actual state change, the WFI never returns. If this assumption is wrong, your box is dead the first time it tries to change cpu frequency.
So if a state change in hardware never occurs, the cpu will not wake up? That sounds like a bad situation but I do not understand how it relates to the changes I made in the driver. Could you explain how tracking cpu_frequency in the driver would result in the cpu not waking up from wfi?
Quoted 3 lines
This is why the code reads the hardware register to find the real current state, rather than assume it.
OK, so this is the point that I referenced above that needs to be addressed. The *only* difference between my implementation and yours is that you do read the enable bit on the powersave clock every time you query the frequency. Note that this driver controls the state of the powersave clock directly via calls to clk_enable & clk_disable.
Is there ever a case where hardware will change the state of the clock behind our backs? If the driver calls clk_enable(priv.powersave_clk), then is there ever a possibility that the clock will in fact be disabled? Likewise if we disable the clock with a call to clk_disable(priv.powersave_clk), is there ever an instance where the hardware will re-enable that clock without telling us? Can the driver's view of the clock status be out of sync with the actual hardware?
Thanks,
Mike
Quoted 45 lines
Andrew
Quoted 42 lines
Quoted 29 lines
drivers/cpufreq/kirkwood-cpufreq.c +4
-3+/* * Kirkwood can swap the clock to the CPU between two clocks: * @@ -50,9 +51,7 @@ static struct cpufreq_frequency_table kirkwood_freq_table[] = {static unsigned int kirkwood_cpufreq_get_cpu_frequency (unsigned int cpu) { -if (__clk_is_enabled (priv.powersave_clk)) -return kirkwood_freq_table[1 ].frequency; -return kirkwood_freq_table[0 ].frequency; +return cpu_frequency; } static int kirkwood_cpufreq_target(struct cpufreq_policy *policy,@@ -71,9 +70,11 @@ static int kirkwood_cpufreq_target(struct cpufreq_policy *policy,switch (state) {case STATE_CPU_FREQ :clk_disable (priv.powersave_clk); + cpu_frequency = kirkwood_freq_table[0 ].frequency;break ;case STATE_DDR_FREQ :clk_enable (priv.powersave_clk); + cpu_frequency = kirkwood_freq_table[1 ].frequency;break ; }drivers/cpufreq/kirkwood-cpufreq.c static int kirkwood_cpufreq_probe(struct platform_device *pdev) · line 140
Quoted 7 lines
drivers/cpufreq/kirkwood-cpufreq.c +1
-0@@ -133,6 +134,7 @@ static int kirkwood_cpufreq_probe(struct platform_device *pdev)clk_prepare_enable (priv.cpu_clk); kirkwood_freq_table[0 ].frequency =clk_get_rate (priv.cpu_clk) /1000 ; + cpu_frequency = kirkwood_freq_table[0 ].frequency; priv.ddr_clk =of_clk_get_by_name (np,"ddrclk" );if (IS_ERR (priv.ddr_clk)) {_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Andrew Lunn
maintainerRe: [PATCH v7 3/8] cpufreq: kirkwood: Remove use of the clk provider API
Quoted 10 lines
Thanks for reviewing. I think my patch is equivalent to the old way of doing things, with one exception that I will address later below.
struct
cpufreq_frequency_tablekirkwood_freq_tablehas clock rates initialized to zero, so there is no regression compared to my unsigned longcpu_frequencyvariable used for tracking the clock rate. Both implementations start with unknown rates in hardware and initialize a variable to zero until that rate can be discovered later on inkirkwood_cpufreq_probe().
Quoted 3 lines
kirkwood_cpufreq_get_cpu_frequency()returns the frequency based on the state of the clock. As best I can tell, this clock is only touched by this cpufreq driver and nowhere else
Not quite true. u-boot might of touch the clock. Weird things happen with some kirkwood boards. Some don't have the ability to control there power supplies. So some boards implement "power off" by rebooting, and letting u-boot spin until a button is pressed. I hope such a u-boot powers off as much as possible, and e.g. drops the CPU clock to the lower frequency. One would also hope it puts it back to high speed before calling the kernel.
There is also the somewhat unlikely case that somebody uses kexec to go from one kernel to another. You have no idea what state the previous kernel left the clock in.
Quoted 11 lines
, so the driver knows the state of
the clock implicitly and doesn't need to read any hardware registers to see if it is enabled or not. Every time we enable or disable the clock we can know the cpu frequency.Quoted 4 lines
However, if you don't cause an actual state change, the WFI never returns. If this assumption is wrong, your box is dead the first time it tries to change cpu frequency.
So if a state change in hardware never occurs, the cpu will not wake up?
Correct. I had that situation a few times while developing this driver. And it is not obvious what has happened, since it does not happen immediately, but when the governor decides the CPU load passes a threshold and the frequency can be changed. I had it stop dead while i assume it executed some sleep in an /etc/init.d script.
Quoted 4 lines
That sounds like a bad situation but I do not understand how it relates to the changes I made in the driver. Could you explain how tracking
cpu_frequencyin the driver would result in the cpu not waking up from wfi?
It was clearer in earlier versions of the driver, but code has been refactored into the cpufreq core. The core should call kirkwood_cpufreq_get_cpu_frequency() in order to get the current frequency, and only perform a change if the requested frequency is different. In the current code, kirkwood_cpufreq_get_cpu_frequency() reads from the hardware what the current frequency is. So we are guaranteed to only call kirkwood_cpufreq_target() when there is a real change.
[snip]
Quoted 2 lines
Can the driver's
view of the clock status be out of sync with the actual hardware?
Yes, at startup, as explained above, we have no idea what the current state of the hardware is. Once we do know the real state, we can track it within the driver, but we need to get that initial state correct, or we WFI forever on the first frequency change.
Andrew
Andrew Lunn
maintainerRe: [PATCH v7 3/8] cpufreq: kirkwood: Remove use of the clk provider API
Quoted 8 lines
It was clearer in earlier versions of the driver, but code has been refactored into the cpufreq core. The core should call
kirkwood_cpufreq_get_cpu_frequency()in order to get the current frequency, and only perform a change if the requested frequency is different. In the current code,kirkwood_cpufreq_get_cpu_frequency()reads from the hardware what the current frequency is. So we are guaranteed to only callkirkwood_cpufreq_target()when there is a real change.
Hi Mike
I went looking at the core.
drivers/cpufreq/cpufreq.c:__cpufreq_add_dev() contains:
if (cpufreq_driver->get && !cpufreq_driver->setpolicy) {
policy->cur = cpufreq_driver->get(policy->cpu);
if (!policy->cur) {
pr_err("%s: ->get() failed\n", __func__);
goto err_get_freq;
}
}So this gets the current frequency from the driver, when the driver is added. So for the current code, this gets the real state of the hardware.
and
drivers/cpufreq/cpufreq.c:__cpufreq_driver_target() contains:
/*
* This might look like a redundant call as we are checking it again
* after finding index. But it is left intentionally for cases where
* exactly same freq is called again and so we can save on few function
* calls.
*/
if (target_freq == policy->cur)
return 0;
/* Save last value to restore later on errors */
policy->restore_freq = policy->cur;
if (cpufreq_driver->target)
retval = cpufreq_driver->target(policy, target_freq, relation);and here it will only call the function to change the frequency, if it is different from the current frequency.
Andrew
PATCH v7 3/8 cpufreq: kirkwood: Remove use of the clk provider API
In reply to Andrew LunnQuoting Andrew Lunn (2014-08-22 13:11:12)
Quoted 14 lines
Quoted 10 lines
Thanks for reviewing. I think my patch is equivalent to the old way of doing things, with one exception that I will address later below.
struct
cpufreq_frequency_tablekirkwood_freq_tablehas clock rates initialized to zero, so there is no regression compared to my unsigned longcpu_frequencyvariable used for tracking the clock rate. Both implementations start with unknown rates in hardware and initialize a variable to zero until that rate can be discovered later on inkirkwood_cpufreq_probe().Quoted 3 lines
kirkwood_cpufreq_get_cpu_frequency()returns the frequency based on the state of the clock. As best I can tell, this clock is only touched by this cpufreq driver and nowhere else
Quoted 7 lines
Not quite true. u-boot might of touch the clock. Weird things happen with some kirkwood boards. Some don't have the ability to control there power supplies. So some boards implement "power off" by rebooting, and letting u-boot spin until a button is pressed. I hope such a u-boot powers off as much as possible, and e.g. drops the CPU clock to the lower frequency. One would also hope it puts it back to high speed before calling the kernel.
I have a doubt about this.
The powersave clock in drivers/clk/mvebu/kirkwood.c does not set CLK_IGNORE_UNUSED, nor do any of the clocks in kirkwood_gating_desc[].
So regardless of what U-boot does, if no driver has called clk_enable() on powersave_clk by late_initcall-time then clk_disable_unused() will disable it as a power-saving mechanism.
So are kirkwood systems that use cpufreq simply getting lucky and not hanging?
Regards,
Mike
Quoted 48 lines
There is also the somewhat unlikely case that somebody uses kexec to go from one kernel to another. You have no idea what state the previous kernel left the clock in.
Quoted 11 lines
, so the driver knows the state of
the clock implicitly and doesn't need to read any hardware registers to see if it is enabled or not. Every time we enable or disable the clock we can know the cpu frequency.Quoted 4 lines
However, if you don't cause an actual state change, the WFI never returns. If this assumption is wrong, your box is dead the first time it tries to change cpu frequency.
So if a state change in hardware never occurs, the cpu will not wake up?
Correct. I had that situation a few times while developing this driver. And it is not obvious what has happened, since it does not happen immediately, but when the governor decides the CPU load passes a threshold and the frequency can be changed. I had it stop dead while i assume it executed some sleep in an
/etc/init.dscript.Quoted 4 lines
That sounds like a bad situation but I do not understand how it relates to the changes I made in the driver. Could you explain how tracking
cpu_frequencyin the driver would result in the cpu not waking up from wfi?It was clearer in earlier versions of the driver, but code has been refactored into the cpufreq core. The core should call
kirkwood_cpufreq_get_cpu_frequency()in order to get the current frequency, and only perform a change if the requested frequency is different. In the current code,kirkwood_cpufreq_get_cpu_frequency()reads from the hardware what the current frequency is. So we are guaranteed to only callkirkwood_cpufreq_target()when there is a real change.[snip]
Quoted 2 lines
Can the driver's
view of the clock status be out of sync with the actual hardware?Yes, at startup, as explained above, we have no idea what the current state of the hardware is. Once we do know the real state, we can track it within the driver, but we need to get that initial state correct, or we WFI forever on the first frequency change.
Andrew
Andrew Lunn
maintainerRe: [PATCH v7 3/8] cpufreq: kirkwood: Remove use of the clk provider API
Quoted 13 lines
Quoted 7 lines
Not quite true. u-boot might of touch the clock. Weird things happen with some kirkwood boards. Some don't have the ability to control there power supplies. So some boards implement "power off" by rebooting, and letting u-boot spin until a button is pressed. I hope such a u-boot powers off as much as possible, and e.g. drops the CPU clock to the lower frequency. One would also hope it puts it back to high speed before calling the kernel.
I have a doubt about this.
The powersave clock in
drivers/clk/mvebu/kirkwood.cdoes not setCLK_IGNORE_UNUSED, nor do any of the clocks in kirkwood_gating_desc[].
Quoted 6 lines
So regardless of what U-boot does, if no driver has called
clk_enable()onpowersave_clkby late_initcall-time thenclk_disable_unused()will disable it as a power-saving mechanism.So are kirkwood systems that use cpufreq simply getting lucky and not hanging?
Hi Mike
Its a good question.
However, the reset value of the clock is off. off means the CPU is running at its high speed. Turning this clock on, actually reduces the clock speed! So for 99% of the time, the late_initcall does nothing.
It gets more interesting when uboot, or a previous kernel has turned the clock on. I admit, i don't expect this to happen very often, but if it does, and there is no cpufreq driver, interesting things could happen. The cpufreq driver can only be builtin, not a module. So if it is available, it should be guaranteed to claim the clock before the late_initcall could turn it off. And since it reads the hardware state, it will do the right thing.
Andrew
PATCH v7 3/8 cpufreq: kirkwood: Remove use of the clk provider API
Quoting Andrew Lunn (2014-08-26 15:36:37)
Quoted 28 lines
Quoted 13 lines
Quoted 7 lines
Not quite true. u-boot might of touch the clock. Weird things happen with some kirkwood boards. Some don't have the ability to control there power supplies. So some boards implement "power off" by rebooting, and letting u-boot spin until a button is pressed. I hope such a u-boot powers off as much as possible, and e.g. drops the CPU clock to the lower frequency. One would also hope it puts it back to high speed before calling the kernel.
I have a doubt about this.
The powersave clock in
drivers/clk/mvebu/kirkwood.cdoes not setCLK_IGNORE_UNUSED, nor do any of the clocks in kirkwood_gating_desc[].Quoted 6 lines
So regardless of what U-boot does, if no driver has called
clk_enable()onpowersave_clkby late_initcall-time thenclk_disable_unused()will disable it as a power-saving mechanism.So are kirkwood systems that use cpufreq simply getting lucky and not hanging?
Hi Mike
Its a good question.
However, the reset value of the clock is off. off means the CPU is running at its high speed. Turning this clock on, actually reduces the clock speed! So for 99% of the time, the
late_initcalldoes nothing.
Quoted 7 lines
It gets more interesting when uboot, or a previous kernel has turned the clock on. I admit, i don't expect this to happen very often, but if it does, and there is no cpufreq driver, interesting things could happen. The cpufreq driver can only be builtin, not a module. So if it is available, it should be guaranteed to claim the clock before the
late_initcallcould turn it off. And since it reads the hardware state, it will do the right thing.
Hi Andrew,
Thanks for the response. That all makes sense. I still think my solution is OK for both of your cases:
1) for the U-boot-enables-powersave-clk case, if we do not have driver that claims the clock and does something with it then that is out of scope of our concerns
2) for the kexec-kernel-case, the responsibility is on the first kernel to set things up in a good state for the second kernel, with the exception of using kexec to debug/examime/recover from a kernel crash, in which case you likely don't care about this stuff as much
But anyways I do understand the case you are trying to prevent.
One final thought I have had is that it might be a good idea to have a mux clock which represents the clock signal that feeds into the cpu. It seems that a mux is exactly what is going on here with cpuclk rate and ddrclk rate.
I even wonder if it is even appropriate to model this transition with a clock enable operation? Maybe it is only a multiplex operation, or perhaps a combination of enabling the powersave clock and changing the parent input to the cpu?
My idea is instead of relying on a cpufreq driver to parse the state of your clocks and understand the multiplexing, you can use the clock framework for that. In fact that might help you get one step closer to using the cpufreq-cpu0.c/cpufreq-generic.c implementation.
Regards,
Mike
Quoted 2 lines
Andrew
Andrew Lunn
maintainerRe: [PATCH v7 3/8] cpufreq: kirkwood: Remove use of the clk provider API
Quoted 4 lines
One final thought I have had is that it might be a good idea to have a mux clock which represents the clock signal that feeds into the cpu. It seems that a mux is exactly what is going on here with cpuclk rate and ddrclk rate.
I did think of this when i implemented the cpufreq driver. What makes it hard is that this bit is mixed in the same 32 bit register as the gate clocks. It would mean two clock providers sharing the same register, sharing a spinlock, etc. And the gating provider is shared with all mvebu platforms, dove, kirkword, orion5x, and four different armada SoCs. So i'm pushing complexity into this shared code, which none of the others need. Does the standard mux clock do what is needed? Or would i have to write a new clock provider?
Quoted 9 lines
I even wonder if it is even appropriate to model this transition with a clock enable operation? Maybe it is only a multiplex operation, or perhaps a combination of enabling the powersave clock and changing the parent input to the cpu?
My idea is instead of relying on a cpufreq driver to parse the state of your clocks and understand the multiplexing, you can use the clock framework for that. In fact that might help you get one step closer to using the
cpufreq-cpu0.c/cpufreq-generic.cimplementation.
So you want the whole disabling of interrupt delivery to the cpu, flipping the mux, wait for interrupt and re-enabling of interrupt delivery to the cpu inside the clock provider? That is way past a simple mux clock.
Andrew
Re: [PATCH v7 3/8] cpufreq: kirkwood: Remove use of the clk provider API
Quoted 5 lines · Andrew Lunn
Quoted 4 lines
One final thought I have had is that it might be a good idea to have a mux clock which represents the clock signal that feeds into the cpu. It seems that a mux is exactly what is going on here with cpuclk rate and ddrclk rate.
Quoted 8 lines
I did think of this when i implemented the cpufreq driver. What makes it hard is that this bit is mixed in the same 32 bit register as the gate clocks. It would mean two clock providers sharing the same register, sharing a spinlock, etc. And the gating provider is shared with all mvebu platforms, dove, kirkword, orion5x, and four different armada SoCs. So i'm pushing complexity into this shared code, which none of the others need. Does the standard mux clock do what is needed? Or would i have to write a new clock provider?
Well I think that the mux-clock type should suffice. Both the standard gate and mux can have a spinlock passed in at registration-time, so it could be a shared spinlock. The standard mux clock expects a bitfield in a register, similar to the single-bit approach taken by the gate clock. So I think it could do just fine.
Quoted 11 lines
Quoted 10 lines
I even wonder if it is even appropriate to model this transition with a clock enable operation? Maybe it is only a multiplex operation, or perhaps a combination of enabling the powersave clock and changing the parent input to the cpu?
My idea is instead of relying on a cpufreq driver to parse the state of your clocks and understand the multiplexing, you can use the clock framework for that. In fact that might help you get one step closer to using the
cpufreq-cpu0.c/cpufreq-generic.cimplementation.
Quoted 4 lines
So you want the whole disabling of interrupt delivery to the cpu, flipping the mux, wait for interrupt and re-enabling of interrupt delivery to the cpu inside the clock provider? That is way past a simple mux clock.
No way! I said, "one step closer" for a reason. The interrupt stuff is clearly out of scope.
Regards,
Mike
Quoted 2 lines
Andrew
Re: [PATCH v7 3/8] cpufreq: kirkwood: Remove use of the clk provider API
In reply to Mike TurquetteQuoted 4 lines · Mike Turquette
2) for the kexec-kernel-case, the responsibility is on the first kernel to set things up in a good state for the second kernel, with the exception of using kexec to debug/examime/recover from a kernel crash, in which case you likely don't care about this stuff as much
FWIW, we frequently use a kexec flow on Kirkwood for development here
- and I have not been able to get the initial kernel to cleanly shut down before kexec'ing the second kernel.
The flow we've had to use involved including a pre-kernel stub in the kexec flow that goes around and cleans up all the registers enough so that the 2nd kernel will work properly. Critically it does things like turn off ethernet DMA, because the initial kernel won't even do that :|
There is some kind of support for doing this, but I ran out of time unraveling the mess of config options to actually turn it on for kirkwood.. It is tied to PM support which was/is missing elements on Kirkwood..
Jason
PATCH v7 4/8 ASoC: mxs-saif: fix mixed use of public and provider clk API
In reply to Tomeu VizosoIn preparation to changing the clk provider API to use struct clk_core instead of struct clk.
--- sound/soc/mxs/mxs-saif.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
sound/soc/mxs/mxs-saif.c +2 -2
@@ -682,11 +682,11 @@ static int mxs_saif_mclk_init(struct platform_device *pdev) {struct mxs_saif *saif =platform_get_drvdata (pdev);struct device_node *np = pdev->dev.of_node; -struct clk *clk; +struct clk_core *clk;int ret; clk =clk_register_divider (&pdev->dev,"mxs_saif_mclk" , -__clk_get_name (saif->clk),0 , +clk_get_name (saif->clk),0 , saif->base +SAIF_CTRL ,BP_SAIF_CTRL_BITCLK_MULT_RATE ,3 ,0 ,NULL );
Signature · 1 line
1.9.3
PATCH v7 6/8 clk: use struct clk only for external API
In reply to Tomeu VizosoIn order to provide per-user accounting, this separates the struct clk used in the common clock framework into two structures 'struct clk_core' and 'struct clk'. struct clk_core will be used for internal manipulation and struct clk will be used in the clock API implementation.
In this patch, struct clk is simply renamed to struct clk_core and a new struct clk is implemented which simply wraps it. In the next patch, the new struct clk will be used to implement per-user clock enable accounting.
Based on previous work by Rabin Vincent <[email protected]>.
NOTE: with this patch, clk_get_parent() behaves like clk_get(), i.e. it needs to be matched with a clk_put(). Otherwise, memory will leak.
---
v7: * convert clk_is_enabled to clk_core
* add clk_provider_notifier_[un]register
v4: * export clk_to_clk_core so mach-msm can use it for clk_reset
* add clk_provider_round_rate, for mach-imx
* fix build with !CONFIG_COMMON_CLK
* keep handling NULL struct clk gracefully as before
v3: * Allocate and release the per-user struct clk
* Have clk_core_to_clk return any error it's passed, so calls to it can be chained
* Add provider API for enable and disable
* Remove clk_free_clk for now
* Have clk_to_clk_core be an inline function to benefit of type-checking
* Have devres wrap the clk struct instead of clk_core
* Rename clk_core_to_clk to __clk_create_clk to make more clear that it allocates
--- arch/arm/mach-msm/clock.c | 2 +- drivers/clk/clk-devres.c | 31 ++ drivers/clk/clk.c | 674 ++++++++++++++++++++++++++----------------- drivers/clk/clk.h | 6 + drivers/clk/clkdev.c | 125 ++++++-- include/linux/clk-private.h | 38 +-- include/linux/clk-provider.h | 162 ++++++----- include/linux/clk.h | 32 +- include/linux/clkdev.h | 24 +- 9 files changed, 707 insertions(+), 387 deletions(-)
arch/arm/mach-msm/clock.c +1 -1
@@ -21,7 +21,7 @@int clk_reset (struct clk *clk,enum clk_reset_action action) { -struct clk_hw *hw =__clk_get_hw (clk); +struct clk_hw *hw =__clk_get_hw (clk_to_clk_core (clk));struct msm_clk *m =to_msm_clk (hw);return m->reset (hw, action); }
drivers/clk/clk-devres.c +31 -0
@@ -5,10 +5,14 @@*/ #include <linux/clk.h> +#include <linux/clkdev.h> +#include <linux/clk-provider.h> #include <linux/device.h> #include <linux/export.h> #include <linux/gfp.h> +#include "clk.h" +static void devm_clk_release (struct device *dev,void *res) {clk_put (*(struct clk **)res);
@@ -34,6 +38,33 @@ struct clk *devm_clk_get(struct device *dev, const char *id) }EXPORT_SYMBOL (devm_clk_get); +#if defined(CONFIG_COMMON_CLK ) +static void devm_clk_core_release (struct device *dev,void *res) +{ +__clk_put (*(struct clk_core **)res); +} + +struct clk_core *devm_clk_provider_get (struct device *dev,const char *id) +{ +struct clk_core **ptr, *clk; + + ptr =devres_alloc (devm_clk_core_release,sizeof (*ptr),GFP_KERNEL ); +if (!ptr) +return ERR_PTR (-ENOMEM ); + + clk =clk_provider_get (dev, id); +if (!IS_ERR (clk)) { + *ptr = clk; +devres_add (dev, ptr); + }else { +devres_free (ptr); + } + +return clk; +} +EXPORT_SYMBOL (devm_clk_provider_get); +#endif +static int devm_clk_match (struct device *dev,void *res,void *data) {struct clk **c = res;
drivers/clk/clk.c +411 -263
1,381 lines not on this page · raw message
drivers/clk/clk.h +6 -0
@@ -9,9 +9,15 @@* published by the Free Software Foundation. */ +#include <linux/clk-private.h> +#if defined(CONFIG_OF ) && defined(CONFIG_COMMON_CLK )struct clk_core *of_clk_get_by_clkspec (struct of_phandle_args *clkspec);struct clk_core *__of_clk_get_from_provider (struct of_phandle_args *clkspec);void of_clk_lock (void );void of_clk_unlock (void );#endif + +#if defined(CONFIG_COMMON_CLK ) +struct clk *__clk_create_clk (struct clk_core *clk_core); +#endif
drivers/clk/clkdev.c +100 -25
248 lines not on this page · raw message
include/linux/clk-private.h +21 -17
@@ -28,20 +28,20 @@struct module ; -struct clk { +struct clk_core {const char *name;const struct clk_ops *ops;struct clk_hw *hw;struct module *owner; -struct clk *parent; +struct clk_core *parent;const char **parent_names; -struct clk **parents; +struct clk_core **parents;u8 num_parents;u8 new_parent_index;unsigned long rate;unsigned long new_rate; -struct clk *new_parent; -struct clk *new_child; +struct clk_core *new_parent; +struct clk_core *new_child;unsigned long flags;unsigned int enable_count;unsigned int prepare_count;
@@ -55,6 +55,10 @@ struct clk {struct kref ref; }; +struct clk { +struct clk_core *core; +}; +/* * DOC: Basic clock implementations common to many platforms *
@@ -66,7 +70,7 @@ struct clk {#define DEFINE_CLK (_name, _ops, _flags, _parent_names, \ _parents) \ - static struct clk _name = { \ + static struct clk_core _name = { \ .name = #_name, \ .ops = &_ops, \ .hw = &_name##_hw.hw, \
@@ -78,7 +82,7 @@ struct clk {#define DEFINE_CLK_FIXED_RATE (_name, _flags, _rate, \ _fixed_rate_flags) \ - static struct clk _name; \ + static struct clk_core _name; \ static const char *_name##_parent_names[] = {}; \ static struct clk_fixed_rate _name##_hw = { \ .hw = { \
@@ -93,11 +97,11 @@ struct clk {#define DEFINE_CLK_GATE (_name, _parent_name, _parent_ptr, \ _flags, _reg, _bit_idx, \ _gate_flags, _lock) \ - static struct clk _name; \ + static struct clk_core _name; \ static const char *_name##_parent_names[] = { \ _parent_name, \ }; \ - static struct clk *_name##_parents[] = { \ + static struct clk_core *_name##_parents[] = { \ _parent_ptr, \ }; \ static struct clk_gate _name##_hw = { \
@@ -115,11 +119,11 @@ struct clk {#define _DEFINE_CLK_DIVIDER (_name, _parent_name, _parent_ptr, \ _flags, _reg, _shift, _width, \ _divider_flags, _table, _lock) \ - static struct clk _name; \ + static struct clk_core _name; \ static const char *_name##_parent_names[] = { \ _parent_name, \ }; \ - static struct clk *_name##_parents[] = { \ + static struct clk_core *_name##_parents[] = { \ _parent_ptr, \ }; \ static struct clk_divider _name##_hw = { \
@@ -154,7 +158,7 @@ struct clk {#define DEFINE_CLK_MUX (_name, _parent_names, _parents, _flags, \ _reg, _shift, _width, \ _mux_flags, _lock) \ - static struct clk _name; \ + static struct clk_core _name; \ static struct clk_mux _name##_hw = { \ .hw = { \ .clk = &_name, \
@@ -171,11 +175,11 @@ struct clk {#define DEFINE_CLK_FIXED_FACTOR (_name, _parent_name, \ _parent_ptr, _flags, \ _mult, _div) \ - static struct clk _name; \ + static struct clk_core _name; \ static const char *_name##_parent_names[] = { \ _parent_name, \ }; \ - static struct clk *_name##_parents[] = { \ + static struct clk_core *_name##_parents[] = { \ _parent_ptr, \ }; \ static struct clk_fixed_factor _name##_hw = { \
@@ -196,7 +200,7 @@ struct clk {* Initializes the lists in struct clk, queries the hardware for the * parent and rate and sets them both. * -* Any struct clk passed into __clk_init must have the following members +* Any struct clk_core passed into __clk_init must have the following members * populated: * .name * .ops
@@ -210,9 +214,9 @@ struct clk {* * Returns 0 on success, otherwise an error code. */ -int __clk_init (struct device *dev,struct clk *clk); +int __clk_init (struct device *dev,struct clk_core *clk); -struct clk *__clk_register (struct device *dev,struct clk_hw *hw); +struct clk_core *__clk_register (struct device *dev,struct clk_hw *hw);#endif /* CONFIG_COMMON_CLK */ #endif /* CLK_PRIVATE_H */
include/linux/clk-provider.h +90 -72
306 lines not on this page · raw message
include/linux/clk.h +29 -3
@@ -22,6 +22,8 @@ struct clk;#ifdef CONFIG_COMMON_CLK +struct clk_core ; +/** * DOC: clk notifier callback types *
@@ -56,7 +58,7 @@ struct clk;* @notifier_head. */ struct clk_notifier { -struct clk *clk; +struct clk_core *clk;struct srcu_notifier_head notifier_head;struct list_head node; };
@@ -73,7 +75,7 @@ struct clk_notifier {* current rate (this was done to optimize the implementation). */ struct clk_notifier_data { -struct clk *clk; +struct clk_core *clk;unsigned long old_rate;unsigned long new_rate; };
@@ -315,6 +317,14 @@ struct clk *clk_get_parent(struct clk *clk);*/ struct clk *clk_get_sys (const char *dev_id,const char *con_id); +/** +* clk_get_name - get a clock's name +* @clk: clock source +* +* Returns the name of the provided clock. +*/ +const char *clk_get_name (struct clk *clk); +#else /* !CONFIG_HAVE_CLK */ static inline struct clk *clk_get (struct device *dev,const char *id)
@@ -411,7 +421,8 @@ struct of_phandle_args;#if defined(CONFIG_OF ) && defined(CONFIG_COMMON_CLK )struct clk *of_clk_get (struct device_node *np,int index);struct clk *of_clk_get_by_name (struct device_node *np,const char *name); -struct clk *of_clk_get_from_provider (struct of_phandle_args *clkspec); +struct clk_core *of_clk_provider_get_by_name (struct device_node *np,const char *name); +struct clk_core *of_clk_get_from_provider (struct of_phandle_args *clkspec);#else static inline struct clk *of_clk_get (struct device_node *np,int index) {
@@ -422,6 +433,21 @@ static inline struct clk *of_clk_get_by_name(struct device_node *np, {return ERR_PTR (-ENOENT ); } + +#if defined(CONFIG_COMMON_CLK ) +static inline struct clk_core *of_clk_provider_get_by_name (struct device_node *np, +const char *name) +{ +return ERR_PTR (-ENOENT ); +} +#else +static inline struct clk *of_clk_provider_get_by_name (struct device_node *np, +const char *name) +{ +return ERR_PTR (-ENOENT ); +} +#endif /* CONFIG_COMMON_CLK */ +#endif #endif
include/linux/clkdev.h +18 -6
@@ -17,11 +17,23 @@struct clk ;struct device ; +/* +* To avoid a mass-rename of all non-common clock implementations (spread out +* in arch-specific code), we let them use struct clk for both the internal and +* external view. +*/ +#ifdef CONFIG_COMMON_CLK +struct clk_core ; +#define clk_core_t struct clk_core +#else +#define clk_core_t struct clk +#endif +struct clk_lookup {struct list_head node;const char *dev_id;const char *con_id; -struct clk *clk; +clk_core_t *clk; };#define CLKDEV_INIT (d, n, c) \
@@ -31,7 +43,7 @@ struct clk_lookup { .clk = c, \ } -struct clk_lookup *clkdev_alloc (struct clk *clk,const char *con_id, +struct clk_lookup *clkdev_alloc (clk_core_t *clk,const char *con_id,const char *dev_fmt, ...);void clkdev_add (struct clk_lookup *cl);
@@ -40,12 +52,12 @@ void clkdev_drop(struct clk_lookup *cl);void clkdev_add_table (struct clk_lookup *,size_t );int clk_add_alias (const char *,const char *,char *,struct device *); -int clk_register_clkdev (struct clk *,const char *,const char *, ...); -int clk_register_clkdevs (struct clk *,struct clk_lookup *,size_t ); +int clk_register_clkdev (clk_core_t *,const char *,const char *, ...); +int clk_register_clkdevs (clk_core_t *,struct clk_lookup *,size_t );#ifdef CONFIG_COMMON_CLK -int __clk_get (struct clk *clk); -void __clk_put (struct clk *clk); +int __clk_get (struct clk_core *clk); +void __clk_put (struct clk_core *clk);#endif #endif
Signature · 1 line
1.9.3
PATCH v7 7/8 clk: per-user clock accounting for debug
In reply to Tomeu VizosoWhen a clock has multiple users, the WARNING on imbalance of enable/disable may not show the guilty party since although they may have commited the error earlier, the warning is emitted later when some other user, presumably innocent, disables the clock.
Provide per-user clock enable/disable accounting and disabler tracking in order to help debug these problems.
Based on previous work by Rabin Vincent <[email protected]>.
--- drivers/clk/clk.c | 38 ++++++++++++++++++++++++++++++++++---- drivers/clk/clk.h | 3 ++- drivers/clk/clkdev.c | 12 ++++++++---- include/linux/clk-private.h | 4 ++++ 4 files changed, 48 insertions(+), 9 deletions(-)
drivers/clk/clk.c +34 -4
@@ -543,7 +543,8 @@ static int clk_disable_unused(void) }late_initcall_sync (clk_disable_unused); -struct clk *__clk_create_clk (struct clk_core *clk_core) +struct clk *__clk_create_clk (struct clk_core *clk_core,const char *dev, +const char *con) {struct clk *clk;
@@ -556,6 +557,8 @@ struct clk *__clk_create_clk(struct clk_core *clk_core)return ERR_PTR (-ENOMEM ); clk->core = clk_core; + clk->dev_id = dev; + clk->con_id = con;return clk; }
@@ -936,10 +939,25 @@ EXPORT_SYMBOL_GPL(clk_provider_disable);*/ void clk_disable (struct clk *clk_user) { +struct clk_core *clk; +unsigned long flags; +if (IS_ERR_OR_NULL (clk_user))return ; -clk_provider_disable (clk_to_clk_core (clk_user)); + clk =clk_to_clk_core (clk_user); + + flags =clk_enable_lock (); +if (!WARN (clk_user->enable_count ==0 , +"incorrect disable clk dev %s con %s last disabler %pF\n" , + clk_user->dev_id, clk_user->con_id, clk_user->last_disable)) { + + clk_user->last_disable =__builtin_return_address (0 ); + clk_user->enable_count--; + +__clk_disable (clk); + } +clk_enable_unlock (flags); }EXPORT_SYMBOL_GPL (clk_disable);
@@ -1000,10 +1018,22 @@ EXPORT_SYMBOL_GPL(clk_provider_enable);*/ int clk_enable (struct clk *clk_user) { +struct clk_core *clk; +unsigned long flags; +int ret; +if (!clk_user)return 0 ; -return clk_provider_enable (clk_to_clk_core (clk_user)); + clk =clk_to_clk_core (clk_user); + + flags =clk_enable_lock (); + ret =__clk_enable (clk); +if (!ret) + clk_user->enable_count++; +clk_enable_unlock (flags); + +return ret; }EXPORT_SYMBOL_GPL (clk_enable);
@@ -1719,7 +1749,7 @@ struct clk *clk_get_parent(struct clk *clk_user) clk =clk_to_clk_core (clk_user); parent =clk_provider_get_parent (clk); -return __clk_create_clk (parent); +return __clk_create_clk (parent, clk_user->dev_id, clk_user->con_id); }EXPORT_SYMBOL_GPL (clk_get_parent);
drivers/clk/clk.h +2 -1
@@ -19,5 +19,6 @@ void of_clk_unlock(void);#endif #if defined(CONFIG_COMMON_CLK ) -struct clk *__clk_create_clk (struct clk_core *clk_core); +struct clk *__clk_create_clk (struct clk_core *clk_core,const char *dev, +const char *con);#endif
drivers/clk/clkdev.c +8 -4
@@ -76,7 +76,9 @@ struct clk_core *of_clk_provider_get(struct device_node *np, int index)struct clk *of_clk_get (struct device_node *np,int index) { -return __clk_create_clk (of_clk_provider_get (np, index)); +struct clk_core *clk =of_clk_provider_get (np, index); + +return __clk_create_clk (clk, np->full_name,NULL ); }EXPORT_SYMBOL (of_clk_get);
@@ -129,7 +131,9 @@ struct clk_core *of_clk_provider_get_by_name(struct device_node *np, const char*/ struct clk *of_clk_get_by_name (struct device_node *np,const char *name) { -return __clk_create_clk (of_clk_provider_get_by_name (np, name)); +struct clk_core *clk =of_clk_provider_get_by_name (np, name); + +return __clk_create_clk (clk, np->full_name,NULL ); }EXPORT_SYMBOL (of_clk_get_by_name);#endif
@@ -201,7 +205,7 @@ struct clk *clk_get_sys(const char *dev_id, const char *con_id)#if defined(CONFIG_COMMON_CLK )struct clk_core *clk =clk_provider_get_sys (dev_id, con_id); -return __clk_create_clk (clk); +return __clk_create_clk (clk, dev_id, con_id);#else struct clk_lookup *cl;
@@ -243,7 +247,7 @@ struct clk *clk_get(struct device *dev, const char *con_id)#if defined(CONFIG_COMMON_CLK )const char *dev_id = dev ?dev_name (dev) :NULL ; -return __clk_create_clk (clk_provider_get (dev, con_id)); +return __clk_create_clk (clk_provider_get (dev, con_id), dev_id, con_id);#else const char *dev_id = dev ?dev_name (dev) :NULL ;struct clk *clk;
include/linux/clk-private.h +4 -0
@@ -57,6 +57,10 @@ struct clk_core {struct clk {struct clk_core *core; +unsigned int enable_count; +const char *dev_id; +const char *con_id; +void *last_disable; };/*
Signature · 1 line
1.9.3
PATCH v7 7/8 clk: per-user clock accounting for debug
Quoting Tomeu Vizoso (2014-08-18 08:30:32)
Quoted 41 lines
drivers/clk/clk.c +22
-2@@ -936,10 +939,25 @@ EXPORT_SYMBOL_GPL(clk_provider_disable);*/ void clk_disable (struct clk *clk_user) { +struct clk_core *clk; +unsigned long flags; +if (IS_ERR_OR_NULL (clk_user))return ; -clk_provider_disable (clk_to_clk_core (clk_user)); + clk =clk_to_clk_core (clk_user); + + flags =clk_enable_lock (); +if (!WARN (clk_user->enable_count ==0 , +"incorrect disable clk dev %s con %s last disabler %pF\n" , + clk_user->dev_id, clk_user->con_id, clk_user->last_disable)) { + + clk_user->last_disable =__builtin_return_address (0 ); + clk_user->enable_count--; + +__clk_disable (clk); + } +clk_enable_unlock (flags); }EXPORT_SYMBOL_GPL (clk_disable);@@ -1000,10 +1018,22 @@ EXPORT_SYMBOL_GPL(clk_provider_enable);*/ int clk_enable (struct clk *clk_user) { +struct clk_core *clk; +unsigned long flags; +int ret; +if (!clk_user)return 0 ; -return clk_provider_enable (clk_to_clk_core (clk_user)); + clk =clk_to_clk_core (clk_user); +
drivers/clk/clk.c EXPORT_SYMBOL_GPL(clk_provider_enable); · line 1039
Quoted 10 lines
drivers/clk/clk.c +7
-0+ flags =clk_enable_lock (); + ret =__clk_enable (clk); +if (!ret) + clk_user->enable_count++; +clk_enable_unlock (flags); + +return ret; }EXPORT_SYMBOL_GPL (clk_enable);
While we're at it, why not do per-using accounting for clk_{un}prepare?
Regards,
Mike
PATCH v7 8/8 clk: Add floor and ceiling constraints to clock rates
In reply to Tomeu VizosoAdds a way for clock consumers to set maximum and minimum rates. This can be used for thermal drivers to set ceiling rates, or by misc. drivers to set floor rates to assure a minimum performance level.
---
v5: * Move the storage of constraints to the per-user clk struct, as suggested
by Stephen Warren.
--- drivers/clk/clk.c | 36 ++++++++++++++++++++++++++++++++++++ drivers/clk/clk.h | 1 + drivers/clk/clkdev.c | 2 +- include/linux/clk-private.h | 4 ++++ include/linux/clk.h | 18 ++++++++++++++++++ 5 files changed, 60 insertions(+), 1 deletion(-)
drivers/clk/clk.c +36 -0
@@ -560,6 +560,8 @@ struct clk *__clk_create_clk(struct clk_core *clk_core, const char *dev, clk->dev_id = dev; clk->con_id = con; +hlist_add_head (&clk->child_node, &clk_core->per_user_clks); +return clk; }
@@ -1646,6 +1648,7 @@ static void clk_change_rate(struct clk_core *clk)int clk_provider_set_rate (struct clk_core *clk,unsigned long rate) {struct clk_core *top, *fail_clk; +struct clk *clk_user;int ret =0 ;if (!clk)
@@ -1654,6 +1657,13 @@ int clk_provider_set_rate(struct clk_core *clk, unsigned long rate)/* prevent racing with updates to the clock topology */ clk_prepare_lock (); +hlist_for_each_entry (clk_user, &clk->per_user_clks, child_node) { + rate =max (rate, clk_user->floor_constraint); + +if (clk_user->ceiling_constraint >0 ) + rate =min (rate, clk_user->ceiling_constraint); + } +/* bail early if nothing to do */ if (rate ==clk_provider_get_rate (clk)) goto out;
@@ -1720,6 +1730,24 @@ int clk_set_rate(struct clk *clk_user, unsigned long rate) }EXPORT_SYMBOL_GPL (clk_set_rate); +int clk_set_floor_rate (struct clk *clk_user,unsigned long rate) +{ +struct clk_core *clk =clk_to_clk_core (clk_user); + + clk_user->floor_constraint = rate; +return clk_provider_set_rate (clk,clk_provider_get_rate (clk)); +} +EXPORT_SYMBOL_GPL (clk_set_floor_rate); + +int clk_set_ceiling_rate (struct clk *clk_user,unsigned long rate) +{ +struct clk_core *clk =clk_to_clk_core (clk_user); + + clk_user->ceiling_constraint = rate; +return clk_provider_set_rate (clk,clk_provider_get_rate (clk)); +} +EXPORT_SYMBOL_GPL (clk_set_ceiling_rate); +struct clk_core *clk_provider_get_parent (struct clk_core *clk) {struct clk_core *parent;
@@ -2064,6 +2092,8 @@ int __clk_init(struct device *dev, struct clk_core *clk) } } +INIT_HLIST_HEAD (&clk->per_user_clks); +/* * optional platform-specific magic *
@@ -2514,6 +2544,12 @@ int clk_notifier_unregister(struct clk *clk_user, struct notifier_block *nb) }EXPORT_SYMBOL_GPL (clk_notifier_unregister); +void __clk_free_clk (struct clk *clk_user) +{ +hlist_del (&clk_user->child_node); +kfree (clk_user); +} +#ifdef CONFIG_OF /** * struct of_clk_provider - Clock provider registration structure
drivers/clk/clk.h +1 -0
@@ -21,4 +21,5 @@ void of_clk_unlock(void);#if defined(CONFIG_COMMON_CLK )struct clk *__clk_create_clk (struct clk_core *clk_core,const char *dev,const char *con); +void __clk_free_clk (struct clk *clk_user);#endif
drivers/clk/clkdev.c +1 -1
@@ -270,7 +270,7 @@ void clk_put(struct clk *clk)#if defined(CONFIG_COMMON_CLK )clk_core_t *core =clk_to_clk_core (clk); -kfree (clk); +__clk_free_clk (clk);__clk_put (core);#else __clk_put (clk);
include/linux/clk-private.h +4 -0
@@ -48,6 +48,7 @@ struct clk_core {unsigned long accuracy;struct hlist_head children;struct hlist_node child_node; +struct hlist_head per_user_clks;unsigned int notifier_count;#ifdef CONFIG_DEBUG_FS struct dentry *dentry;
@@ -61,6 +62,9 @@ struct clk {const char *dev_id;const char *con_id;void *last_disable; +unsigned long floor_constraint; +unsigned long ceiling_constraint; +struct hlist_node child_node; };/*
include/linux/clk.h +18 -0
@@ -283,6 +283,24 @@ long clk_round_rate(struct clk *clk, unsigned long rate);int clk_set_rate (struct clk *clk,unsigned long rate);/** +* clk_set_floor_rate - set a minimum clock rate for a clock source +* @clk: clock source +* @rate: desired minimum clock rate in Hz +* +* Returns success (0) or negative errno. +*/ +int clk_set_floor_rate (struct clk *clk,unsigned long rate); + +/** +* clk_set_ceiling_rate - set a maximum clock rate for a clock source +* @clk: clock source +* @rate: desired maximum clock rate in Hz +* +* Returns success (0) or negative errno. +*/ +int clk_set_ceiling_rate (struct clk *clk,unsigned long rate); + +/** * clk_set_parent - set the parent clock source for this clock * @clk: clock source * @parent: parent clock source
Signature · 1 line
1.9.3
Andrew Lunn
maintainerQuoted 5 lines · Tomeu Vizoso
Hi,
in this v7 of the patchset I have only rebased on top of 3.17rc1, with no other changes. I have had to do a fair amount of fixing due to the rebase, more details below. Follows the original cover letter blurb:
Hi Tomeu
I would like to test this patchset, but patch 5 never made it to the list. Do you have a git tree i can clone?
Thanks
Andrew
Quoted 11 lines · Andrew Lunn
Quoted 5 lines · Tomeu Vizoso
Hi,
in this v7 of the patchset I have only rebased on top of 3.17rc1, with no other changes. I have had to do a fair amount of fixing due to the rebase, more details below. Follows the original cover letter blurb:
Hi Tomeu
I would like to test this patchset, but patch 5 never made it to the list. Do you have a git tree i can clone?
Sure:
http://cgit.collabora.com/git/user/tomeu/linux.git/log/?h=clk-refactoring-7
Though it seems to have made into at least one mailing list (the others seem to have rejected it because of either its size, or the number of recipients):
http://article.gmane.org/gmane.linux.ports.ppc64.devel/104392
Thanks for the interest!
Tomeu
Quoted 7 lines
Thanks
Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Hi Tomeu,
Am Montag, 18. August 2014, 17:30:26 schrieb Tomeu Vizoso:
Quoted 11 lines
Hi,
in this v7 of the patchset I have only rebased on top of 3.17rc1, with no other changes. I have had to do a fair amount of fixing due to the rebase, more details below. Follows the original cover letter blurb:
I'm retaking Rabin's patches [0] for splitting the clk API in two: one API for clk consumers and another for providers. The consumer API uses a clk structure that just keeps track of the consumer and has a reference to the actual
clk_corestruct, which is used internally.
Quoted 6 lines
I have kept a patch from Rabin that aims to aid in debugging nested enable/disable calls, though my personal aim is to allow more than one consumer to influence the final, effective frequency rate. For now this is limited to setting floor and ceiling constraints, with the short-term aim of allowing devfreq and thermal drivers to set floor and ceiling frequencies on the memory clock, respectively.
sounds interesting. I've tested your clk-refactoring-7 branch on a Rockchip rk3188-radxarock and rk3288-evb-rk808 board, so
Heiko