* [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks @ 2011-07-11 2:53 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 2:53 UTC (permalink / raw) To: linux-arm-kernel I've just been having a go at implementing generic clk API support for an off-SoC device on a slow bus, the clocking module in the wm831x/2x series of PMICs. Unfortunately as I don't currently have access to a platform that has been converted to use the API I've not actually been able to test the code but I'm reasonably optimistic that the code will Just Work(tm) as the API seems fairly straightforward. The biggest issue I ran into was that as the clocks are all registered by name with the API if you've got two instances of the same off-SoC device in the system you'll not be able to disambiguate between the clocks it provides. I've added a simple solution for this in the form of a patch which takes a struct device as an argument and adds that to the name of the registered clock. This isn't ideal but should give stable names which systems can use together with clkdev to match clocks up with their users. I believe that when we have device tree bindings for clocks we should be able to use the device to access the bindings and avoid this mangling - Grant, is that correct? Otherwise everything seems to work pretty well from the driver side for these devices, I've added some minor updates which came up naturally while writing the driver code and seemed fairly obvious. One of these was a change to provide a user visible Kconfig option for building the clock drivers, the idea being to make it easier to do build tests. I did also wonder if it's worth adding a patch to enable the API on x86 (which doesn't currently have a clock API) for coverage. These patches (which include the two I posted yesterday) are against the series you posted in May with the exception of the wm831x patch which also has an additional dependency on "mfd: Add WM831x clock control register definitions" which is due for merge in the next merge window. If this stuff is all OK for you it would be good if you could include the wm831x driver in your tree for this, especially given that there's nothing in -next. Mark Brown (6): clk: Prototype and document clk_register() clk: Provide a dummy clk_unregister() clk: Constify struct clk_hw_ops clk: Add Kconfig option to build all generic clk drivers clk: Support multiple instances of the same clock provider clk: Add initial WM831x clock driver MAINTAINERS | 1 + drivers/clk/Kconfig | 16 ++- drivers/clk/Makefile | 1 + drivers/clk/clk-wm831x.c | 389 ++++++++++++++++++++++++++++++++++++++++++++++ drivers/clk/clk.c | 38 ++++- include/linux/clk.h | 33 ++++ 6 files changed, 472 insertions(+), 6 deletions(-) create mode 100644 drivers/clk/clk-wm831x.c ^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks @ 2011-07-11 2:53 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 2:53 UTC (permalink / raw) To: linux-arm-kernel I've just been having a go at implementing generic clk API support for an off-SoC device on a slow bus, the clocking module in the wm831x/2x series of PMICs. Unfortunately as I don't currently have access to a platform that has been converted to use the API I've not actually been able to test the code but I'm reasonably optimistic that the code will Just Work(tm) as the API seems fairly straightforward. The biggest issue I ran into was that as the clocks are all registered by name with the API if you've got two instances of the same off-SoC device in the system you'll not be able to disambiguate between the clocks it provides. I've added a simple solution for this in the form of a patch which takes a struct device as an argument and adds that to the name of the registered clock. This isn't ideal but should give stable names which systems can use together with clkdev to match clocks up with their users. I believe that when we have device tree bindings for clocks we should be able to use the device to access the bindings and avoid this mangling - Grant, is that correct? Otherwise everything seems to work pretty well from the driver side for these devices, I've added some minor updates which came up naturally while writing the driver code and seemed fairly obvious. One of these was a change to provide a user visible Kconfig option for building the clock drivers, the idea being to make it easier to do build tests. I did also wonder if it's worth adding a patch to enable the API on x86 (which doesn't currently have a clock API) for coverage. These patches (which include the two I posted yesterday) are against the series you posted in May with the exception of the wm831x patch which also has an additional dependency on "mfd: Add WM831x clock control register definitions" which is due for merge in the next merge window. If this stuff is all OK for you it would be good if you could include the wm831x driver in your tree for this, especially given that there's nothing in -next. Mark Brown (6): clk: Prototype and document clk_register() clk: Provide a dummy clk_unregister() clk: Constify struct clk_hw_ops clk: Add Kconfig option to build all generic clk drivers clk: Support multiple instances of the same clock provider clk: Add initial WM831x clock driver MAINTAINERS | 1 + drivers/clk/Kconfig | 16 ++- drivers/clk/Makefile | 1 + drivers/clk/clk-wm831x.c | 389 ++++++++++++++++++++++++++++++++++++++++++++++ drivers/clk/clk.c | 38 ++++- include/linux/clk.h | 33 ++++ 6 files changed, 472 insertions(+), 6 deletions(-) create mode 100644 drivers/clk/clk-wm831x.c ^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks @ 2011-07-11 2:53 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 2:53 UTC (permalink / raw) To: Jeremy Kerr Cc: linux-kernel, linux-arm-kernel, linux-sh, patches, Grant Likely I've just been having a go at implementing generic clk API support for an off-SoC device on a slow bus, the clocking module in the wm831x/2x series of PMICs. Unfortunately as I don't currently have access to a platform that has been converted to use the API I've not actually been able to test the code but I'm reasonably optimistic that the code will Just Work(tm) as the API seems fairly straightforward. The biggest issue I ran into was that as the clocks are all registered by name with the API if you've got two instances of the same off-SoC device in the system you'll not be able to disambiguate between the clocks it provides. I've added a simple solution for this in the form of a patch which takes a struct device as an argument and adds that to the name of the registered clock. This isn't ideal but should give stable names which systems can use together with clkdev to match clocks up with their users. I believe that when we have device tree bindings for clocks we should be able to use the device to access the bindings and avoid this mangling - Grant, is that correct? Otherwise everything seems to work pretty well from the driver side for these devices, I've added some minor updates which came up naturally while writing the driver code and seemed fairly obvious. One of these was a change to provide a user visible Kconfig option for building the clock drivers, the idea being to make it easier to do build tests. I did also wonder if it's worth adding a patch to enable the API on x86 (which doesn't currently have a clock API) for coverage. These patches (which include the two I posted yesterday) are against the series you posted in May with the exception of the wm831x patch which also has an additional dependency on "mfd: Add WM831x clock control register definitions" which is due for merge in the next merge window. If this stuff is all OK for you it would be good if you could include the wm831x driver in your tree for this, especially given that there's nothing in -next. Mark Brown (6): clk: Prototype and document clk_register() clk: Provide a dummy clk_unregister() clk: Constify struct clk_hw_ops clk: Add Kconfig option to build all generic clk drivers clk: Support multiple instances of the same clock provider clk: Add initial WM831x clock driver MAINTAINERS | 1 + drivers/clk/Kconfig | 16 ++- drivers/clk/Makefile | 1 + drivers/clk/clk-wm831x.c | 389 ++++++++++++++++++++++++++++++++++++++++++++++ drivers/clk/clk.c | 38 ++++- include/linux/clk.h | 33 ++++ 6 files changed, 472 insertions(+), 6 deletions(-) create mode 100644 drivers/clk/clk-wm831x.c ^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 1/6] clk: Prototype and document clk_register() 2011-07-11 2:53 ` Mark Brown (?) @ 2011-07-11 2:53 ` Mark Brown -1 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 2:53 UTC (permalink / raw) To: linux-arm-kernel This allows the compiler to ensure drivers are using the correct prototype. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- include/linux/clk.h | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/include/linux/clk.h b/include/linux/clk.h index 7c26135..2ca4f66 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -136,6 +136,18 @@ extern struct clk_hw_ops clk_gate_ops; #endif /* CONFIG_GENERIC_CLK_GATE */ +/** + * clk_register - register and initialize a new clock + * + * @ops: ops for the new clock + * @hw: struct clk_hw to be passed to the ops of the new clock + * @name: name to use for the new clock + * + * Register a new clock with the clk subsytem. Returns either a + * struct clk for the new clock or a NULL pointer. + */ +struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw, + const char *name); #else /* !CONFIG_GENERIC_CLK */ -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 1/6] clk: Prototype and document clk_register() @ 2011-07-11 2:53 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 2:53 UTC (permalink / raw) To: linux-arm-kernel This allows the compiler to ensure drivers are using the correct prototype. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- include/linux/clk.h | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/include/linux/clk.h b/include/linux/clk.h index 7c26135..2ca4f66 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -136,6 +136,18 @@ extern struct clk_hw_ops clk_gate_ops; #endif /* CONFIG_GENERIC_CLK_GATE */ +/** + * clk_register - register and initialize a new clock + * + * @ops: ops for the new clock + * @hw: struct clk_hw to be passed to the ops of the new clock + * @name: name to use for the new clock + * + * Register a new clock with the clk subsytem. Returns either a + * struct clk for the new clock or a NULL pointer. + */ +struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw, + const char *name); #else /* !CONFIG_GENERIC_CLK */ -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 1/6] clk: Prototype and document clk_register() @ 2011-07-11 2:53 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 2:53 UTC (permalink / raw) To: Jeremy Kerr Cc: Grant Likely, linux-kernel, linux-arm-kernel, linux-sh, patches, Mark Brown This allows the compiler to ensure drivers are using the correct prototype. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- include/linux/clk.h | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/include/linux/clk.h b/include/linux/clk.h index 7c26135..2ca4f66 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -136,6 +136,18 @@ extern struct clk_hw_ops clk_gate_ops; #endif /* CONFIG_GENERIC_CLK_GATE */ +/** + * clk_register - register and initialize a new clock + * + * @ops: ops for the new clock + * @hw: struct clk_hw to be passed to the ops of the new clock + * @name: name to use for the new clock + * + * Register a new clock with the clk subsytem. Returns either a + * struct clk for the new clock or a NULL pointer. + */ +struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw, + const char *name); #else /* !CONFIG_GENERIC_CLK */ -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 2/6] clk: Provide a dummy clk_unregister() 2011-07-11 2:53 ` Mark Brown (?) @ 2011-07-11 2:53 ` Mark Brown -1 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 2:53 UTC (permalink / raw) To: linux-arm-kernel Even though unregistration is not actually supported by the clk API it is still useful to provide a clk_unregister() so that drivers can implement their unregistration code. This saves having to go back later and update them once unregistration is possible. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- include/linux/clk.h | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-) diff --git a/include/linux/clk.h b/include/linux/clk.h index 2ca4f66..df5c64f 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -149,6 +149,21 @@ extern struct clk_hw_ops clk_gate_ops; struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw, const char *name); +/** + * clk_unregister - remove a clock + * + * @clk: clock to unregister + * + * Remove a clock from the clk subsystem. This is currently not + * implemented but is provided to allow unregistration code to be + * written in drivers ready for use when an implementation is + * provided. + */ +static inline int clk_unregister(struct clk *clk) +{ + return -ENOTSUPP; +} + #else /* !CONFIG_GENERIC_CLK */ /* -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 2/6] clk: Provide a dummy clk_unregister() @ 2011-07-11 2:53 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 2:53 UTC (permalink / raw) To: linux-arm-kernel Even though unregistration is not actually supported by the clk API it is still useful to provide a clk_unregister() so that drivers can implement their unregistration code. This saves having to go back later and update them once unregistration is possible. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- include/linux/clk.h | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-) diff --git a/include/linux/clk.h b/include/linux/clk.h index 2ca4f66..df5c64f 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -149,6 +149,21 @@ extern struct clk_hw_ops clk_gate_ops; struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw, const char *name); +/** + * clk_unregister - remove a clock + * + * @clk: clock to unregister + * + * Remove a clock from the clk subsystem. This is currently not + * implemented but is provided to allow unregistration code to be + * written in drivers ready for use when an implementation is + * provided. + */ +static inline int clk_unregister(struct clk *clk) +{ + return -ENOTSUPP; +} + #else /* !CONFIG_GENERIC_CLK */ /* -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 2/6] clk: Provide a dummy clk_unregister() @ 2011-07-11 2:53 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 2:53 UTC (permalink / raw) To: Jeremy Kerr Cc: Grant Likely, linux-kernel, linux-arm-kernel, linux-sh, patches, Mark Brown Even though unregistration is not actually supported by the clk API it is still useful to provide a clk_unregister() so that drivers can implement their unregistration code. This saves having to go back later and update them once unregistration is possible. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- include/linux/clk.h | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-) diff --git a/include/linux/clk.h b/include/linux/clk.h index 2ca4f66..df5c64f 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -149,6 +149,21 @@ extern struct clk_hw_ops clk_gate_ops; struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw, const char *name); +/** + * clk_unregister - remove a clock + * + * @clk: clock to unregister + * + * Remove a clock from the clk subsystem. This is currently not + * implemented but is provided to allow unregistration code to be + * written in drivers ready for use when an implementation is + * provided. + */ +static inline int clk_unregister(struct clk *clk) +{ + return -ENOTSUPP; +} + #else /* !CONFIG_GENERIC_CLK */ /* -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 3/6] clk: Constify struct clk_hw_ops 2011-07-11 2:53 ` Mark Brown (?) @ 2011-07-11 2:53 ` Mark Brown -1 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 2:53 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- drivers/clk/clk.c | 4 ++-- include/linux/clk.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 3a4d70e..1df6e23 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -16,7 +16,7 @@ struct clk { const char *name; - struct clk_hw_ops *ops; + const struct clk_hw_ops *ops; struct clk_hw *hw; unsigned int enable_count; unsigned int prepare_count; @@ -252,7 +252,7 @@ int clk_set_parent(struct clk *clk, struct clk *parent) } EXPORT_SYMBOL_GPL(clk_set_parent); -struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw, +struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw, const char *name) { struct clk *clk; diff --git a/include/linux/clk.h b/include/linux/clk.h index df5c64f..fb5e435 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -146,7 +146,7 @@ extern struct clk_hw_ops clk_gate_ops; * Register a new clock with the clk subsytem. Returns either a * struct clk for the new clock or a NULL pointer. */ -struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw, +struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw, const char *name); /** -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 3/6] clk: Constify struct clk_hw_ops @ 2011-07-11 2:53 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 2:53 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- drivers/clk/clk.c | 4 ++-- include/linux/clk.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 3a4d70e..1df6e23 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -16,7 +16,7 @@ struct clk { const char *name; - struct clk_hw_ops *ops; + const struct clk_hw_ops *ops; struct clk_hw *hw; unsigned int enable_count; unsigned int prepare_count; @@ -252,7 +252,7 @@ int clk_set_parent(struct clk *clk, struct clk *parent) } EXPORT_SYMBOL_GPL(clk_set_parent); -struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw, +struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw, const char *name) { struct clk *clk; diff --git a/include/linux/clk.h b/include/linux/clk.h index df5c64f..fb5e435 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -146,7 +146,7 @@ extern struct clk_hw_ops clk_gate_ops; * Register a new clock with the clk subsytem. Returns either a * struct clk for the new clock or a NULL pointer. */ -struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw, +struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw, const char *name); /** -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 3/6] clk: Constify struct clk_hw_ops @ 2011-07-11 2:53 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 2:53 UTC (permalink / raw) To: Jeremy Kerr Cc: Grant Likely, linux-kernel, linux-arm-kernel, linux-sh, patches, Mark Brown Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- drivers/clk/clk.c | 4 ++-- include/linux/clk.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 3a4d70e..1df6e23 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -16,7 +16,7 @@ struct clk { const char *name; - struct clk_hw_ops *ops; + const struct clk_hw_ops *ops; struct clk_hw *hw; unsigned int enable_count; unsigned int prepare_count; @@ -252,7 +252,7 @@ int clk_set_parent(struct clk *clk, struct clk *parent) } EXPORT_SYMBOL_GPL(clk_set_parent); -struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw, +struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw, const char *name) { struct clk *clk; diff --git a/include/linux/clk.h b/include/linux/clk.h index df5c64f..fb5e435 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -146,7 +146,7 @@ extern struct clk_hw_ops clk_gate_ops; * Register a new clock with the clk subsytem. Returns either a * struct clk for the new clock or a NULL pointer. */ -struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw, +struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw, const char *name); /** -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 4/6] clk: Add Kconfig option to build all generic clk drivers 2011-07-11 2:53 ` Mark Brown (?) @ 2011-07-11 2:53 ` Mark Brown -1 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 2:53 UTC (permalink / raw) To: linux-arm-kernel Currently drivers for the generic clk subsystem must be selected by platforms using them in order to enable build. When doing development on the API or generic build time testing it is useful to be able to build unused drivers in order to improve coverage so supply a Kconfig option which allows this. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- drivers/clk/Kconfig | 11 ++++++++++- 1 files changed, 10 insertions(+), 1 deletions(-) diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index 75d2902..1fd0070 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -1,4 +1,3 @@ - config CLKDEV_LOOKUP bool select HAVE_CLK @@ -6,6 +5,16 @@ config CLKDEV_LOOKUP config GENERIC_CLK bool +config GENERIC_CLK_BUILD_TEST + bool "Build all generic clock drivers" + depends on EXPERIMENTAL && GENERIC_CLK + select GENERIC_CLK_FIXED + select GENERIC_CLK_GATE + help + Enable all possible generic clock drivers. This is only + useful for improving build coverage, it is not useful for + production kernel builds. + config GENERIC_CLK_FIXED bool depends on GENERIC_CLK -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 4/6] clk: Add Kconfig option to build all generic clk drivers @ 2011-07-11 2:53 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 2:53 UTC (permalink / raw) To: linux-arm-kernel Currently drivers for the generic clk subsystem must be selected by platforms using them in order to enable build. When doing development on the API or generic build time testing it is useful to be able to build unused drivers in order to improve coverage so supply a Kconfig option which allows this. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- drivers/clk/Kconfig | 11 ++++++++++- 1 files changed, 10 insertions(+), 1 deletions(-) diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index 75d2902..1fd0070 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -1,4 +1,3 @@ - config CLKDEV_LOOKUP bool select HAVE_CLK @@ -6,6 +5,16 @@ config CLKDEV_LOOKUP config GENERIC_CLK bool +config GENERIC_CLK_BUILD_TEST + bool "Build all generic clock drivers" + depends on EXPERIMENTAL && GENERIC_CLK + select GENERIC_CLK_FIXED + select GENERIC_CLK_GATE + help + Enable all possible generic clock drivers. This is only + useful for improving build coverage, it is not useful for + production kernel builds. + config GENERIC_CLK_FIXED bool depends on GENERIC_CLK -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 4/6] clk: Add Kconfig option to build all generic clk drivers @ 2011-07-11 2:53 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 2:53 UTC (permalink / raw) To: Jeremy Kerr Cc: Grant Likely, linux-kernel, linux-arm-kernel, linux-sh, patches, Mark Brown Currently drivers for the generic clk subsystem must be selected by platforms using them in order to enable build. When doing development on the API or generic build time testing it is useful to be able to build unused drivers in order to improve coverage so supply a Kconfig option which allows this. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- drivers/clk/Kconfig | 11 ++++++++++- 1 files changed, 10 insertions(+), 1 deletions(-) diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index 75d2902..1fd0070 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -1,4 +1,3 @@ - config CLKDEV_LOOKUP bool select HAVE_CLK @@ -6,6 +5,16 @@ config CLKDEV_LOOKUP config GENERIC_CLK bool +config GENERIC_CLK_BUILD_TEST + bool "Build all generic clock drivers" + depends on EXPERIMENTAL && GENERIC_CLK + select GENERIC_CLK_FIXED + select GENERIC_CLK_GATE + help + Enable all possible generic clock drivers. This is only + useful for improving build coverage, it is not useful for + production kernel builds. + config GENERIC_CLK_FIXED bool depends on GENERIC_CLK -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 5/6] clk: Support multiple instances of the same clock provider 2011-07-11 2:53 ` Mark Brown (?) @ 2011-07-11 2:53 ` Mark Brown -1 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 2:53 UTC (permalink / raw) To: linux-arm-kernel Currently the generic clk API identifies clocks internally using the name of the clock. This is OK for on-SoC clocks where we have enough control to disambiguate but doesn't work well for clocks provided on external chips where a system design may include more than one instance of the same chip (the Wolfson Speyside system is an example of this) or may have namespace collisions. Address this by allowing the clock provider to supply a struct device for the clock for use in disambiguation. As a first pass if it is provided we prefix the clock name with the dev_name() of the device. With a device tree binding for clocks it should be possible to remove this mangling and instead use the struct device to provide access to the binding information. In order to avoid needless noise in names and memory usage it is strongly recommended that on-SoC clocks do not provide a struct device until the implementation is improved. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- drivers/clk/clk.c | 36 ++++++++++++++++++++++++++++++++---- include/linux/clk.h | 14 ++++++++++---- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 1df6e23..f36f637 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -13,6 +13,7 @@ #include <linux/mutex.h> #include <linux/slab.h> #include <linux/spinlock.h> +#include <linux/device.h> struct clk { const char *name; @@ -252,20 +253,44 @@ int clk_set_parent(struct clk *clk, struct clk *parent) } EXPORT_SYMBOL_GPL(clk_set_parent); -struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw, - const char *name) +struct clk *clk_register(struct device *dev, const struct clk_hw_ops *ops, + struct clk_hw *hw, const char *name) { struct clk *clk; + char *new_name; + size_t name_len; clk = kzalloc(sizeof(*clk), GFP_KERNEL); if (!clk) return NULL; - clk->name = name; clk->ops = ops; clk->hw = hw; hw->clk = clk; + /* Since we currently match clock providers on a purely string + * based method add a prefix based on the device name if a + * device is provided. When we have support for device tree + * based clock matching it should be possible to avoid this + * mangling and instead use the struct device to hook into + * the bindings. + * + * As we don't currently support unregistering clocks we don't + * need to worry about cleanup as yet. + */ + if (dev) { + name_len = strlen(name) + strlen(dev_name(dev)) + 2; + new_name = kzalloc(name_len, GFP_KERNEL); + if (!new_name) + goto err; + + snprintf(new_name, name_len, "%s-%s", dev_name(dev), name); + + clk->name = new_name; + } else { + clk->name = name; + } + /* Query the hardware for parent and initial rate. We may alter * the clock topology, making this clock available from the parent's * children list. So, we need to protect against concurrent @@ -285,7 +310,10 @@ struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw, mutex_unlock(&prepare_lock); - return clk; + +err: + kfree(clk); + return NULL; } EXPORT_SYMBOL_GPL(clk_register); diff --git a/include/linux/clk.h b/include/linux/clk.h index fb5e435..cb1879b 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -139,15 +139,21 @@ extern struct clk_hw_ops clk_gate_ops; /** * clk_register - register and initialize a new clock * + * @dev: device providing the clock or NULL * @ops: ops for the new clock * @hw: struct clk_hw to be passed to the ops of the new clock * @name: name to use for the new clock * - * Register a new clock with the clk subsytem. Returns either a - * struct clk for the new clock or a NULL pointer. + * Register a new clock with the clk subsytem. If dev is provided + * then it will be used to disambiguate between multiple instances of + * the same device in the system, typically this should only be done + * for devices that are not part of the core SoC unless device tree is + * in use. + * + * Returns either a struct clk for the new clock or a NULL pointer. */ -struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw, - const char *name); +struct clk *clk_register(struct device *dev, const struct clk_hw_ops *ops, + struct clk_hw *hw, const char *name); /** * clk_unregister - remove a clock -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 5/6] clk: Support multiple instances of the same clock provider @ 2011-07-11 2:53 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 2:53 UTC (permalink / raw) To: linux-arm-kernel Currently the generic clk API identifies clocks internally using the name of the clock. This is OK for on-SoC clocks where we have enough control to disambiguate but doesn't work well for clocks provided on external chips where a system design may include more than one instance of the same chip (the Wolfson Speyside system is an example of this) or may have namespace collisions. Address this by allowing the clock provider to supply a struct device for the clock for use in disambiguation. As a first pass if it is provided we prefix the clock name with the dev_name() of the device. With a device tree binding for clocks it should be possible to remove this mangling and instead use the struct device to provide access to the binding information. In order to avoid needless noise in names and memory usage it is strongly recommended that on-SoC clocks do not provide a struct device until the implementation is improved. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- drivers/clk/clk.c | 36 ++++++++++++++++++++++++++++++++---- include/linux/clk.h | 14 ++++++++++---- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 1df6e23..f36f637 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -13,6 +13,7 @@ #include <linux/mutex.h> #include <linux/slab.h> #include <linux/spinlock.h> +#include <linux/device.h> struct clk { const char *name; @@ -252,20 +253,44 @@ int clk_set_parent(struct clk *clk, struct clk *parent) } EXPORT_SYMBOL_GPL(clk_set_parent); -struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw, - const char *name) +struct clk *clk_register(struct device *dev, const struct clk_hw_ops *ops, + struct clk_hw *hw, const char *name) { struct clk *clk; + char *new_name; + size_t name_len; clk = kzalloc(sizeof(*clk), GFP_KERNEL); if (!clk) return NULL; - clk->name = name; clk->ops = ops; clk->hw = hw; hw->clk = clk; + /* Since we currently match clock providers on a purely string + * based method add a prefix based on the device name if a + * device is provided. When we have support for device tree + * based clock matching it should be possible to avoid this + * mangling and instead use the struct device to hook into + * the bindings. + * + * As we don't currently support unregistering clocks we don't + * need to worry about cleanup as yet. + */ + if (dev) { + name_len = strlen(name) + strlen(dev_name(dev)) + 2; + new_name = kzalloc(name_len, GFP_KERNEL); + if (!new_name) + goto err; + + snprintf(new_name, name_len, "%s-%s", dev_name(dev), name); + + clk->name = new_name; + } else { + clk->name = name; + } + /* Query the hardware for parent and initial rate. We may alter * the clock topology, making this clock available from the parent's * children list. So, we need to protect against concurrent @@ -285,7 +310,10 @@ struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw, mutex_unlock(&prepare_lock); - return clk; + +err: + kfree(clk); + return NULL; } EXPORT_SYMBOL_GPL(clk_register); diff --git a/include/linux/clk.h b/include/linux/clk.h index fb5e435..cb1879b 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -139,15 +139,21 @@ extern struct clk_hw_ops clk_gate_ops; /** * clk_register - register and initialize a new clock * + * @dev: device providing the clock or NULL * @ops: ops for the new clock * @hw: struct clk_hw to be passed to the ops of the new clock * @name: name to use for the new clock * - * Register a new clock with the clk subsytem. Returns either a - * struct clk for the new clock or a NULL pointer. + * Register a new clock with the clk subsytem. If dev is provided + * then it will be used to disambiguate between multiple instances of + * the same device in the system, typically this should only be done + * for devices that are not part of the core SoC unless device tree is + * in use. + * + * Returns either a struct clk for the new clock or a NULL pointer. */ -struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw, - const char *name); +struct clk *clk_register(struct device *dev, const struct clk_hw_ops *ops, + struct clk_hw *hw, const char *name); /** * clk_unregister - remove a clock -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 5/6] clk: Support multiple instances of the same clock provider @ 2011-07-11 2:53 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 2:53 UTC (permalink / raw) To: Jeremy Kerr Cc: Grant Likely, linux-kernel, linux-arm-kernel, linux-sh, patches, Mark Brown Currently the generic clk API identifies clocks internally using the name of the clock. This is OK for on-SoC clocks where we have enough control to disambiguate but doesn't work well for clocks provided on external chips where a system design may include more than one instance of the same chip (the Wolfson Speyside system is an example of this) or may have namespace collisions. Address this by allowing the clock provider to supply a struct device for the clock for use in disambiguation. As a first pass if it is provided we prefix the clock name with the dev_name() of the device. With a device tree binding for clocks it should be possible to remove this mangling and instead use the struct device to provide access to the binding information. In order to avoid needless noise in names and memory usage it is strongly recommended that on-SoC clocks do not provide a struct device until the implementation is improved. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- drivers/clk/clk.c | 36 ++++++++++++++++++++++++++++++++---- include/linux/clk.h | 14 ++++++++++---- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 1df6e23..f36f637 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -13,6 +13,7 @@ #include <linux/mutex.h> #include <linux/slab.h> #include <linux/spinlock.h> +#include <linux/device.h> struct clk { const char *name; @@ -252,20 +253,44 @@ int clk_set_parent(struct clk *clk, struct clk *parent) } EXPORT_SYMBOL_GPL(clk_set_parent); -struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw, - const char *name) +struct clk *clk_register(struct device *dev, const struct clk_hw_ops *ops, + struct clk_hw *hw, const char *name) { struct clk *clk; + char *new_name; + size_t name_len; clk = kzalloc(sizeof(*clk), GFP_KERNEL); if (!clk) return NULL; - clk->name = name; clk->ops = ops; clk->hw = hw; hw->clk = clk; + /* Since we currently match clock providers on a purely string + * based method add a prefix based on the device name if a + * device is provided. When we have support for device tree + * based clock matching it should be possible to avoid this + * mangling and instead use the struct device to hook into + * the bindings. + * + * As we don't currently support unregistering clocks we don't + * need to worry about cleanup as yet. + */ + if (dev) { + name_len = strlen(name) + strlen(dev_name(dev)) + 2; + new_name = kzalloc(name_len, GFP_KERNEL); + if (!new_name) + goto err; + + snprintf(new_name, name_len, "%s-%s", dev_name(dev), name); + + clk->name = new_name; + } else { + clk->name = name; + } + /* Query the hardware for parent and initial rate. We may alter * the clock topology, making this clock available from the parent's * children list. So, we need to protect against concurrent @@ -285,7 +310,10 @@ struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw, mutex_unlock(&prepare_lock); - return clk; + +err: + kfree(clk); + return NULL; } EXPORT_SYMBOL_GPL(clk_register); diff --git a/include/linux/clk.h b/include/linux/clk.h index fb5e435..cb1879b 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -139,15 +139,21 @@ extern struct clk_hw_ops clk_gate_ops; /** * clk_register - register and initialize a new clock * + * @dev: device providing the clock or NULL * @ops: ops for the new clock * @hw: struct clk_hw to be passed to the ops of the new clock * @name: name to use for the new clock * - * Register a new clock with the clk subsytem. Returns either a - * struct clk for the new clock or a NULL pointer. + * Register a new clock with the clk subsytem. If dev is provided + * then it will be used to disambiguate between multiple instances of + * the same device in the system, typically this should only be done + * for devices that are not part of the core SoC unless device tree is + * in use. + * + * Returns either a struct clk for the new clock or a NULL pointer. */ -struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw, - const char *name); +struct clk *clk_register(struct device *dev, const struct clk_hw_ops *ops, + struct clk_hw *hw, const char *name); /** * clk_unregister - remove a clock -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 72+ messages in thread
* Re: [PATCH 5/6] clk: Support multiple instances of the same clock 2011-07-11 2:53 ` Mark Brown (?) @ 2011-07-11 9:34 ` Russell King - ARM Linux -1 siblings, 0 replies; 72+ messages in thread From: Russell King - ARM Linux @ 2011-07-11 9:34 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 11:53:56AM +0900, Mark Brown wrote: > Currently the generic clk API identifies clocks internally using the name > of the clock. This is OK for on-SoC clocks where we have enough control to > disambiguate but doesn't work well for clocks provided on external chips > where a system design may include more than one instance of the same chip > (the Wolfson Speyside system is an example of this) or may have namespace > collisions. > > Address this by allowing the clock provider to supply a struct device for > the clock for use in disambiguation. As a first pass if it is provided we > prefix the clock name with the dev_name() of the device. With a device > tree binding for clocks it should be possible to remove this mangling and > instead use the struct device to provide access to the binding information. > > In order to avoid needless noise in names and memory usage it is strongly > recommended that on-SoC clocks do not provide a struct device until the > implementation is improved. > > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> > --- > drivers/clk/clk.c | 36 ++++++++++++++++++++++++++++++++---- > include/linux/clk.h | 14 ++++++++++---- > 2 files changed, 42 insertions(+), 8 deletions(-) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index 1df6e23..f36f637 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -13,6 +13,7 @@ > #include <linux/mutex.h> > #include <linux/slab.h> > #include <linux/spinlock.h> > +#include <linux/device.h> > > struct clk { > const char *name; > @@ -252,20 +253,44 @@ int clk_set_parent(struct clk *clk, struct clk *parent) > } > EXPORT_SYMBOL_GPL(clk_set_parent); > > -struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw, > - const char *name) > +struct clk *clk_register(struct device *dev, const struct clk_hw_ops *ops, > + struct clk_hw *hw, const char *name) > { > struct clk *clk; > + char *new_name; > + size_t name_len; > > clk = kzalloc(sizeof(*clk), GFP_KERNEL); > if (!clk) > return NULL; > > - clk->name = name; > clk->ops = ops; > clk->hw = hw; > hw->clk = clk; > > + /* Since we currently match clock providers on a purely string > + * based method add a prefix based on the device name if a > + * device is provided. When we have support for device tree > + * based clock matching it should be possible to avoid this > + * mangling and instead use the struct device to hook into > + * the bindings. > + * > + * As we don't currently support unregistering clocks we don't > + * need to worry about cleanup as yet. > + */ > + if (dev) { > + name_len = strlen(name) + strlen(dev_name(dev)) + 2; > + new_name = kzalloc(name_len, GFP_KERNEL); > + if (!new_name) > + goto err; > + > + snprintf(new_name, name_len, "%s-%s", dev_name(dev), name); > + > + clk->name = new_name; > + } else { > + clk->name = name; > + } This "clk consolidation" is really idiotic. The clk matching mechanism should have _nothing_ to do with the rest of the clk API, especially the consolidation effort. It should not matter whether clkdev is used, or an alternative method to specify this stuff via DT. Keep the clk_get()/clk_put() _separate_ from the consolidation of the rest. ^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 5/6] clk: Support multiple instances of the same clock provider @ 2011-07-11 9:34 ` Russell King - ARM Linux 0 siblings, 0 replies; 72+ messages in thread From: Russell King - ARM Linux @ 2011-07-11 9:34 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 11:53:56AM +0900, Mark Brown wrote: > Currently the generic clk API identifies clocks internally using the name > of the clock. This is OK for on-SoC clocks where we have enough control to > disambiguate but doesn't work well for clocks provided on external chips > where a system design may include more than one instance of the same chip > (the Wolfson Speyside system is an example of this) or may have namespace > collisions. > > Address this by allowing the clock provider to supply a struct device for > the clock for use in disambiguation. As a first pass if it is provided we > prefix the clock name with the dev_name() of the device. With a device > tree binding for clocks it should be possible to remove this mangling and > instead use the struct device to provide access to the binding information. > > In order to avoid needless noise in names and memory usage it is strongly > recommended that on-SoC clocks do not provide a struct device until the > implementation is improved. > > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> > --- > drivers/clk/clk.c | 36 ++++++++++++++++++++++++++++++++---- > include/linux/clk.h | 14 ++++++++++---- > 2 files changed, 42 insertions(+), 8 deletions(-) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index 1df6e23..f36f637 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -13,6 +13,7 @@ > #include <linux/mutex.h> > #include <linux/slab.h> > #include <linux/spinlock.h> > +#include <linux/device.h> > > struct clk { > const char *name; > @@ -252,20 +253,44 @@ int clk_set_parent(struct clk *clk, struct clk *parent) > } > EXPORT_SYMBOL_GPL(clk_set_parent); > > -struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw, > - const char *name) > +struct clk *clk_register(struct device *dev, const struct clk_hw_ops *ops, > + struct clk_hw *hw, const char *name) > { > struct clk *clk; > + char *new_name; > + size_t name_len; > > clk = kzalloc(sizeof(*clk), GFP_KERNEL); > if (!clk) > return NULL; > > - clk->name = name; > clk->ops = ops; > clk->hw = hw; > hw->clk = clk; > > + /* Since we currently match clock providers on a purely string > + * based method add a prefix based on the device name if a > + * device is provided. When we have support for device tree > + * based clock matching it should be possible to avoid this > + * mangling and instead use the struct device to hook into > + * the bindings. > + * > + * As we don't currently support unregistering clocks we don't > + * need to worry about cleanup as yet. > + */ > + if (dev) { > + name_len = strlen(name) + strlen(dev_name(dev)) + 2; > + new_name = kzalloc(name_len, GFP_KERNEL); > + if (!new_name) > + goto err; > + > + snprintf(new_name, name_len, "%s-%s", dev_name(dev), name); > + > + clk->name = new_name; > + } else { > + clk->name = name; > + } This "clk consolidation" is really idiotic. The clk matching mechanism should have _nothing_ to do with the rest of the clk API, especially the consolidation effort. It should not matter whether clkdev is used, or an alternative method to specify this stuff via DT. Keep the clk_get()/clk_put() _separate_ from the consolidation of the rest. ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 5/6] clk: Support multiple instances of the same clock provider @ 2011-07-11 9:34 ` Russell King - ARM Linux 0 siblings, 0 replies; 72+ messages in thread From: Russell King - ARM Linux @ 2011-07-11 9:34 UTC (permalink / raw) To: Mark Brown Cc: Jeremy Kerr, Grant Likely, linux-sh, patches, linux-kernel, linux-arm-kernel On Mon, Jul 11, 2011 at 11:53:56AM +0900, Mark Brown wrote: > Currently the generic clk API identifies clocks internally using the name > of the clock. This is OK for on-SoC clocks where we have enough control to > disambiguate but doesn't work well for clocks provided on external chips > where a system design may include more than one instance of the same chip > (the Wolfson Speyside system is an example of this) or may have namespace > collisions. > > Address this by allowing the clock provider to supply a struct device for > the clock for use in disambiguation. As a first pass if it is provided we > prefix the clock name with the dev_name() of the device. With a device > tree binding for clocks it should be possible to remove this mangling and > instead use the struct device to provide access to the binding information. > > In order to avoid needless noise in names and memory usage it is strongly > recommended that on-SoC clocks do not provide a struct device until the > implementation is improved. > > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> > --- > drivers/clk/clk.c | 36 ++++++++++++++++++++++++++++++++---- > include/linux/clk.h | 14 ++++++++++---- > 2 files changed, 42 insertions(+), 8 deletions(-) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index 1df6e23..f36f637 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -13,6 +13,7 @@ > #include <linux/mutex.h> > #include <linux/slab.h> > #include <linux/spinlock.h> > +#include <linux/device.h> > > struct clk { > const char *name; > @@ -252,20 +253,44 @@ int clk_set_parent(struct clk *clk, struct clk *parent) > } > EXPORT_SYMBOL_GPL(clk_set_parent); > > -struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw, > - const char *name) > +struct clk *clk_register(struct device *dev, const struct clk_hw_ops *ops, > + struct clk_hw *hw, const char *name) > { > struct clk *clk; > + char *new_name; > + size_t name_len; > > clk = kzalloc(sizeof(*clk), GFP_KERNEL); > if (!clk) > return NULL; > > - clk->name = name; > clk->ops = ops; > clk->hw = hw; > hw->clk = clk; > > + /* Since we currently match clock providers on a purely string > + * based method add a prefix based on the device name if a > + * device is provided. When we have support for device tree > + * based clock matching it should be possible to avoid this > + * mangling and instead use the struct device to hook into > + * the bindings. > + * > + * As we don't currently support unregistering clocks we don't > + * need to worry about cleanup as yet. > + */ > + if (dev) { > + name_len = strlen(name) + strlen(dev_name(dev)) + 2; > + new_name = kzalloc(name_len, GFP_KERNEL); > + if (!new_name) > + goto err; > + > + snprintf(new_name, name_len, "%s-%s", dev_name(dev), name); > + > + clk->name = new_name; > + } else { > + clk->name = name; > + } This "clk consolidation" is really idiotic. The clk matching mechanism should have _nothing_ to do with the rest of the clk API, especially the consolidation effort. It should not matter whether clkdev is used, or an alternative method to specify this stuff via DT. Keep the clk_get()/clk_put() _separate_ from the consolidation of the rest. ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 5/6] clk: Support multiple instances of the same clock 2011-07-11 9:34 ` Russell King - ARM Linux (?) @ 2011-07-11 10:53 ` Mark Brown -1 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 10:53 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 10:34:39AM +0100, Russell King - ARM Linux wrote: > On Mon, Jul 11, 2011 at 11:53:56AM +0900, Mark Brown wrote: > > + /* Since we currently match clock providers on a purely string > > + * based method add a prefix based on the device name if a > > + * device is provided. When we have support for device tree > This "clk consolidation" is really idiotic. The clk matching mechanism > should have _nothing_ to do with the rest of the clk API, especially the > consolidation effort. It's not touching clkdev, the comment is somewhat misleading and is mostly based on me thinking about how we'd deploy off-SoC clocks. There's also the diagnostic issues Sacha mentioned, if we don't keep some source information handy it's hard to tell what clock logging is talking about. > It should not matter whether clkdev is used, or an alternative method > to specify this stuff via DT. Keep the clk_get()/clk_put() _separate_ > from the consolidation of the rest. We do need some way to have some idea which clocks we're talking about, and for off-SoC stuff passing around struct clk pointers is rather painful. At some point some bit of code is going to have to get hold of the actual struct clk and then map it onto the devices using it. For device tree we should be able to do that fairly painlessly with just the struct devices, without device tree you either have to have the structs handy or use names. At the minute the infrastructure is somewhat lacking here. ^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 5/6] clk: Support multiple instances of the same clock provider @ 2011-07-11 10:53 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 10:53 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 10:34:39AM +0100, Russell King - ARM Linux wrote: > On Mon, Jul 11, 2011 at 11:53:56AM +0900, Mark Brown wrote: > > + /* Since we currently match clock providers on a purely string > > + * based method add a prefix based on the device name if a > > + * device is provided. When we have support for device tree > This "clk consolidation" is really idiotic. The clk matching mechanism > should have _nothing_ to do with the rest of the clk API, especially the > consolidation effort. It's not touching clkdev, the comment is somewhat misleading and is mostly based on me thinking about how we'd deploy off-SoC clocks. There's also the diagnostic issues Sacha mentioned, if we don't keep some source information handy it's hard to tell what clock logging is talking about. > It should not matter whether clkdev is used, or an alternative method > to specify this stuff via DT. Keep the clk_get()/clk_put() _separate_ > from the consolidation of the rest. We do need some way to have some idea which clocks we're talking about, and for off-SoC stuff passing around struct clk pointers is rather painful. At some point some bit of code is going to have to get hold of the actual struct clk and then map it onto the devices using it. For device tree we should be able to do that fairly painlessly with just the struct devices, without device tree you either have to have the structs handy or use names. At the minute the infrastructure is somewhat lacking here. ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 5/6] clk: Support multiple instances of the same clock provider @ 2011-07-11 10:53 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 10:53 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Jeremy Kerr, Grant Likely, linux-sh, patches, linux-kernel, linux-arm-kernel On Mon, Jul 11, 2011 at 10:34:39AM +0100, Russell King - ARM Linux wrote: > On Mon, Jul 11, 2011 at 11:53:56AM +0900, Mark Brown wrote: > > + /* Since we currently match clock providers on a purely string > > + * based method add a prefix based on the device name if a > > + * device is provided. When we have support for device tree > This "clk consolidation" is really idiotic. The clk matching mechanism > should have _nothing_ to do with the rest of the clk API, especially the > consolidation effort. It's not touching clkdev, the comment is somewhat misleading and is mostly based on me thinking about how we'd deploy off-SoC clocks. There's also the diagnostic issues Sacha mentioned, if we don't keep some source information handy it's hard to tell what clock logging is talking about. > It should not matter whether clkdev is used, or an alternative method > to specify this stuff via DT. Keep the clk_get()/clk_put() _separate_ > from the consolidation of the rest. We do need some way to have some idea which clocks we're talking about, and for off-SoC stuff passing around struct clk pointers is rather painful. At some point some bit of code is going to have to get hold of the actual struct clk and then map it onto the devices using it. For device tree we should be able to do that fairly painlessly with just the struct devices, without device tree you either have to have the structs handy or use names. At the minute the infrastructure is somewhat lacking here. ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 5/6] clk: Support multiple instances of the same clock 2011-07-11 10:53 ` Mark Brown (?) @ 2011-07-11 11:11 ` Russell King - ARM Linux -1 siblings, 0 replies; 72+ messages in thread From: Russell King - ARM Linux @ 2011-07-11 11:11 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 07:53:45PM +0900, Mark Brown wrote: > We do need some way to have some idea which clocks we're talking about, > and for off-SoC stuff passing around struct clk pointers is rather > painful. At some point some bit of code is going to have to get hold of > the actual struct clk and then map it onto the devices using it. As I haven't seen any of this "passing around struct clk pointers" crap recently, I can only guess at what you're saying. I thought all that had been solved with _proper_ use of clkdev. clkdev can provide you with a struct clk for _any_ device in the system where its name is known at build time. For devices where the name is not known at build time, a new cl_lookup entry can be created at runtime with clkdev_alloc() or clk_add_alias(), and dropped with clkdev_drop(). The problem comes when you aren't able to hook into a subsystem which creates an unstable device name (eg, USB). I suspect that's also a problem for DT too because there has to be some way to go from a struct device to something stable. ^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 5/6] clk: Support multiple instances of the same clock provider @ 2011-07-11 11:11 ` Russell King - ARM Linux 0 siblings, 0 replies; 72+ messages in thread From: Russell King - ARM Linux @ 2011-07-11 11:11 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 07:53:45PM +0900, Mark Brown wrote: > We do need some way to have some idea which clocks we're talking about, > and for off-SoC stuff passing around struct clk pointers is rather > painful. At some point some bit of code is going to have to get hold of > the actual struct clk and then map it onto the devices using it. As I haven't seen any of this "passing around struct clk pointers" crap recently, I can only guess at what you're saying. I thought all that had been solved with _proper_ use of clkdev. clkdev can provide you with a struct clk for _any_ device in the system where its name is known at build time. For devices where the name is not known at build time, a new cl_lookup entry can be created at runtime with clkdev_alloc() or clk_add_alias(), and dropped with clkdev_drop(). The problem comes when you aren't able to hook into a subsystem which creates an unstable device name (eg, USB). I suspect that's also a problem for DT too because there has to be some way to go from a struct device to something stable. ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 5/6] clk: Support multiple instances of the same clock provider @ 2011-07-11 11:11 ` Russell King - ARM Linux 0 siblings, 0 replies; 72+ messages in thread From: Russell King - ARM Linux @ 2011-07-11 11:11 UTC (permalink / raw) To: Mark Brown Cc: Jeremy Kerr, Grant Likely, linux-sh, patches, linux-kernel, linux-arm-kernel On Mon, Jul 11, 2011 at 07:53:45PM +0900, Mark Brown wrote: > We do need some way to have some idea which clocks we're talking about, > and for off-SoC stuff passing around struct clk pointers is rather > painful. At some point some bit of code is going to have to get hold of > the actual struct clk and then map it onto the devices using it. As I haven't seen any of this "passing around struct clk pointers" crap recently, I can only guess at what you're saying. I thought all that had been solved with _proper_ use of clkdev. clkdev can provide you with a struct clk for _any_ device in the system where its name is known at build time. For devices where the name is not known at build time, a new cl_lookup entry can be created at runtime with clkdev_alloc() or clk_add_alias(), and dropped with clkdev_drop(). The problem comes when you aren't able to hook into a subsystem which creates an unstable device name (eg, USB). I suspect that's also a problem for DT too because there has to be some way to go from a struct device to something stable. ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 5/6] clk: Support multiple instances of the same clock 2011-07-11 11:11 ` Russell King - ARM Linux (?) @ 2011-07-11 11:41 ` Mark Brown -1 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 11:41 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 12:11:53PM +0100, Russell King - ARM Linux wrote: > On Mon, Jul 11, 2011 at 07:53:45PM +0900, Mark Brown wrote: > > We do need some way to have some idea which clocks we're talking about, > > and for off-SoC stuff passing around struct clk pointers is rather > > painful. At some point some bit of code is going to have to get hold of > > the actual struct clk and then map it onto the devices using it. > As I haven't seen any of this "passing around struct clk pointers" crap > recently, I can only guess at what you're saying. I thought all that > had been solved with _proper_ use of clkdev. The problem is getting the data into clkdev in the first place for the off-SoC devices. > clkdev can provide you with a struct clk for _any_ device in the system > where its name is known at build time. Right, on the consumer side it's all sorted and works really well. The trick is on the provider side where you need to have the clock available to be able to add the mappings for it. Perhaps I'm just missing something about how this should all work - actually, the regulator solution probably works fine here. Pass an array of mappings into either the driver or the clock subsystem and have them add the mappings when the provider registers. Whichever way around should just be some hooks in the new subsystem. ^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 5/6] clk: Support multiple instances of the same clock provider @ 2011-07-11 11:41 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 11:41 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 12:11:53PM +0100, Russell King - ARM Linux wrote: > On Mon, Jul 11, 2011 at 07:53:45PM +0900, Mark Brown wrote: > > We do need some way to have some idea which clocks we're talking about, > > and for off-SoC stuff passing around struct clk pointers is rather > > painful. At some point some bit of code is going to have to get hold of > > the actual struct clk and then map it onto the devices using it. > As I haven't seen any of this "passing around struct clk pointers" crap > recently, I can only guess at what you're saying. I thought all that > had been solved with _proper_ use of clkdev. The problem is getting the data into clkdev in the first place for the off-SoC devices. > clkdev can provide you with a struct clk for _any_ device in the system > where its name is known at build time. Right, on the consumer side it's all sorted and works really well. The trick is on the provider side where you need to have the clock available to be able to add the mappings for it. Perhaps I'm just missing something about how this should all work - actually, the regulator solution probably works fine here. Pass an array of mappings into either the driver or the clock subsystem and have them add the mappings when the provider registers. Whichever way around should just be some hooks in the new subsystem. ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 5/6] clk: Support multiple instances of the same clock provider @ 2011-07-11 11:41 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 11:41 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Jeremy Kerr, Grant Likely, linux-sh, patches, linux-kernel, linux-arm-kernel On Mon, Jul 11, 2011 at 12:11:53PM +0100, Russell King - ARM Linux wrote: > On Mon, Jul 11, 2011 at 07:53:45PM +0900, Mark Brown wrote: > > We do need some way to have some idea which clocks we're talking about, > > and for off-SoC stuff passing around struct clk pointers is rather > > painful. At some point some bit of code is going to have to get hold of > > the actual struct clk and then map it onto the devices using it. > As I haven't seen any of this "passing around struct clk pointers" crap > recently, I can only guess at what you're saying. I thought all that > had been solved with _proper_ use of clkdev. The problem is getting the data into clkdev in the first place for the off-SoC devices. > clkdev can provide you with a struct clk for _any_ device in the system > where its name is known at build time. Right, on the consumer side it's all sorted and works really well. The trick is on the provider side where you need to have the clock available to be able to add the mappings for it. Perhaps I'm just missing something about how this should all work - actually, the regulator solution probably works fine here. Pass an array of mappings into either the driver or the clock subsystem and have them add the mappings when the provider registers. Whichever way around should just be some hooks in the new subsystem. ^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 6/6] clk: Add initial WM831x clock driver 2011-07-11 2:53 ` Mark Brown (?) @ 2011-07-11 2:53 ` Mark Brown -1 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 2:53 UTC (permalink / raw) To: linux-arm-kernel The WM831x and WM832x series of PMICs contain a flexible clocking subsystem intended to provide always on and system core clocks. It features: - A 32.768kHz crystal oscillator which can optionally be used to pass through an externally generated clock. - A FLL which can be clocked from either the 32.768kHz oscillator or the CLKIN pin. - A CLKOUT pin which can bring out either the oscillator or the FLL output. - The 32.768kHz clock can also optionally be brought out on the GPIO pins of the device. This driver fully supports the 32.768kHz oscillator and CLKOUT. The FLL is supported only in AUTO mode, the full flexibility of the FLL cannot currently be used. The use of clock references other than the internal oscillator is not currently supported, and since clk_set_parent() is not implemented in the generic clock API the clock tree configuration cannot be changed at runtime. Due to a lack of access to systems where the core SoC has been converted to use the generic clock API this driver has been compile tested only. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- MAINTAINERS | 1 + drivers/clk/Kconfig | 5 + drivers/clk/Makefile | 1 + drivers/clk/clk-wm831x.c | 389 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 396 insertions(+), 0 deletions(-) create mode 100644 drivers/clk/clk-wm831x.c diff --git a/MAINTAINERS b/MAINTAINERS index ae563fa..c234756 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6969,6 +6969,7 @@ T: git git://opensource.wolfsonmicro.com/linux-2.6-audioplus W: http://opensource.wolfsonmicro.com/content/linux-drivers-wolfson-devices S: Supported F: Documentation/hwmon/wm83?? +F: drivers/clk/clk-wm83*.c F: drivers/leds/leds-wm83*.c F: drivers/mfd/wm8*.c F: drivers/power/wm83*.c diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index 1fd0070..7f6eec2 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -10,6 +10,7 @@ config GENERIC_CLK_BUILD_TEST depends on EXPERIMENTAL && GENERIC_CLK select GENERIC_CLK_FIXED select GENERIC_CLK_GATE + select GENERIC_CLK_WM831X if MFD_WM831X=y help Enable all possible generic clock drivers. This is only useful for improving build coverage, it is not useful for @@ -22,3 +23,7 @@ config GENERIC_CLK_FIXED config GENERIC_CLK_GATE bool depends on GENERIC_CLK + +config GENERIC_CLK_WM831X + tristate + depends on GENERIC_CLK && MFD_WM831X diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index d186446..6628ad5 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -3,3 +3,4 @@ obj-$(CONFIG_CLKDEV_LOOKUP) += clkdev.o obj-$(CONFIG_GENERIC_CLK) += clk.o obj-$(CONFIG_GENERIC_CLK_FIXED) += clk-fixed.o obj-$(CONFIG_GENERIC_CLK_GATE) += clk-gate.o +obj-$(CONFIG_GENERIC_CLK_WM831X) += clk-wm831x.o diff --git a/drivers/clk/clk-wm831x.c b/drivers/clk/clk-wm831x.c new file mode 100644 index 0000000..82782f1 --- /dev/null +++ b/drivers/clk/clk-wm831x.c @@ -0,0 +1,389 @@ +/* + * WM831x clock control + * + * Copyright 2011 Wolfson Microelectronics PLC. + * + * Author: Mark Brown <broonie@opensource.wolfsonmicro.com> + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + * + */ + +#include <linux/clk.h> +#include <linux/module.h> +#include <linux/slab.h> +#include <linux/platform_device.h> +#include <linux/mfd/wm831x/core.h> + +struct wm831x_clk { + struct wm831x *wm831x; + struct clk_hw xtal_hw; + struct clk_hw fll_hw; + struct clk_hw clkout_hw; + bool xtal_ena; +}; + +static int wm831x_xtal_enable(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + xtal_hw); + + if (clkdata->xtal_ena) + return 0; + else + return -EPERM; +} + +static unsigned long wm831x_xtal_recalc_rate(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + xtal_hw); + + if (clkdata->xtal_ena) + return 32768; + else + return 0; +} + +static long wm831x_xtal_round_rate(struct clk_hw *hw, unsigned long rate) +{ + return wm831x_xtal_recalc_rate(hw); +} + +static const struct clk_hw_ops wm831x_xtal_ops = { + .enable = wm831x_xtal_enable, + .recalc_rate = wm831x_xtal_recalc_rate, + .round_rate = wm831x_xtal_round_rate, +}; + +static const unsigned long wm831x_fll_auto_rates[] = { + 2048000, + 11289600, + 12000000, + 12288000, + 19200000, + 22579600, + 24000000, + 24576000, +}; + +static bool wm831x_fll_enabled(struct wm831x *wm831x) +{ + int ret; + + ret = wm831x_reg_read(wm831x, WM831X_FLL_CONTROL_1); + if (ret < 0) { + dev_err(wm831x->dev, "Unable to read FLL_CONTROL_1: %d\n", + ret); + return true; + } + + if (ret & WM831X_FLL_ENA) + return true; + else + return false; +} + +static int wm831x_fll_prepare(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + fll_hw); + struct wm831x *wm831x = clkdata->wm831x; + int ret; + + ret = wm831x_set_bits(wm831x, WM831X_FLL_CONTROL_2, + WM831X_FLL_ENA, WM831X_FLL_ENA); + if (ret != 0) + dev_crit(wm831x->dev, "Failed to enable FLL: %d\n", ret); + + return ret; +} + +static void wm831x_fll_unprepare(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + fll_hw); + struct wm831x *wm831x = clkdata->wm831x; + int ret; + + ret = wm831x_set_bits(wm831x, WM831X_FLL_CONTROL_2, WM831X_FLL_ENA, 0); + if (ret != 0) + dev_crit(wm831x->dev, "Failed to disaable FLL: %d\n", ret); +} + +static unsigned long wm831x_fll_recalc_rate(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + fll_hw); + struct wm831x *wm831x = clkdata->wm831x; + int ret; + + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2); + if (ret < 0) { + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n", + ret); + return 0; + } + + if (ret & WM831X_FLL_AUTO) + return wm831x_fll_auto_rates[ret & WM831X_FLL_AUTO_FREQ_MASK]; + + dev_err(wm831x->dev, "FLL only supported in AUTO mode\n"); + return 0; +} + +static int wm831x_fll_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *parent_rate) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + fll_hw); + struct wm831x *wm831x = clkdata->wm831x; + int i; + + for (i = 0; i < ARRAY_SIZE(wm831x_fll_auto_rates); i++) + if (wm831x_fll_auto_rates[i] = rate) + break; + if (i = ARRAY_SIZE(wm831x_fll_auto_rates)) + return -EINVAL; + + if (wm831x_fll_enabled(wm831x)) + return -EPERM; + + return wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_2, + WM831X_FLL_AUTO_FREQ_MASK, i); +} + +static struct clk *wm831x_fll_get_parent(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + fll_hw); + struct wm831x *wm831x = clkdata->wm831x; + int ret; + + /* AUTO mode is always clocked from the crystal */ + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2); + if (ret < 0) { + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n", + ret); + return NULL; + } + + if (ret & WM831X_FLL_AUTO) + return clkdata->xtal_hw.clk; + + ret = wm831x_reg_read(wm831x, WM831X_FLL_CONTROL_5); + if (ret < 0) { + dev_err(wm831x->dev, "Unable to read FLL_CONTROL_5: %d\n", + ret); + return NULL; + } + + switch (ret & WM831X_FLL_CLK_SRC_MASK) { + case 0: + return clkdata->xtal_hw.clk; + case 1: + dev_warn(wm831x->dev, + "FLL clocked from CLKIN not yet supported\n"); + return NULL; + default: + dev_err(wm831x->dev, "Unsupported FLL clock source %d\n", + ret & WM831X_FLL_CLK_SRC_MASK); + return NULL; + } +} + +static const struct clk_hw_ops wm831x_fll_ops = { + .prepare = wm831x_fll_prepare, + .unprepare = wm831x_fll_unprepare, + .recalc_rate = wm831x_fll_recalc_rate, + .set_rate = wm831x_fll_set_rate, + .get_parent = wm831x_fll_get_parent, +}; + +static int wm831x_clkout_prepare(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + clkout_hw); + struct wm831x *wm831x = clkdata->wm831x; + int ret; + + ret = wm831x_reg_unlock(wm831x); + if (ret != 0) { + dev_crit(wm831x->dev, "Failed to lock registers: %d\n", ret); + return ret; + } + + ret = wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_1, + WM831X_CLKOUT_ENA, WM831X_CLKOUT_ENA); + if (ret != 0) + dev_crit(wm831x->dev, "Failed to enable CLKOUT: %d\n", ret); + + wm831x_reg_lock(wm831x); + + return ret; +} + +static void wm831x_clkout_unprepare(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + clkout_hw); + struct wm831x *wm831x = clkdata->wm831x; + int ret; + + ret = wm831x_reg_unlock(wm831x); + if (ret != 0) { + dev_crit(wm831x->dev, "Failed to lock registers: %d\n", ret); + return; + } + + ret = wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_1, + WM831X_CLKOUT_ENA, 0); + if (ret != 0) + dev_crit(wm831x->dev, "Failed to disable CLKOUT: %d\n", ret); + + wm831x_reg_lock(wm831x); +} + +static unsigned long wm831x_clkout_recalc_rate(struct clk_hw *hw) +{ + return clk_get_rate(clk_get_parent(hw->clk)); +} + +static long wm831x_clkout_round_rate(struct clk_hw *hw, unsigned long rate) +{ + return clk_round_rate(clk_get_parent(hw->clk), rate); +} + +static int wm831x_clkout_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *parent_rate) +{ + *parent_rate = rate; + return CLK_SET_RATE_PROPAGATE; +} + +static struct clk *wm831x_clkout_get_parent(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + clkout_hw); + struct wm831x *wm831x = clkdata->wm831x; + int ret; + + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_1); + if (ret < 0) { + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_1: %d\n", + ret); + return NULL; + } + + if (ret & WM831X_CLKOUT_SRC) + return clkdata->xtal_hw.clk; + else + return clkdata->fll_hw.clk; +} + +static const struct clk_hw_ops wm831x_clkout_ops = { + .prepare = wm831x_clkout_prepare, + .unprepare = wm831x_clkout_unprepare, + .recalc_rate = wm831x_clkout_recalc_rate, + .round_rate = wm831x_clkout_round_rate, + .set_rate = wm831x_clkout_set_rate, + .get_parent = wm831x_clkout_get_parent, +}; + +static __devinit int wm831x_clk_probe(struct platform_device *pdev) +{ + struct wm831x *wm831x = dev_get_drvdata(pdev->dev.parent); + struct wm831x_clk *clkdata; + int ret; + + clkdata = kzalloc(sizeof(*clkdata), GFP_KERNEL); + if (!clkdata) + return -ENOMEM; + + /* XTAL_ENA can only be set via OTP/InstantConfig so just read once */ + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2); + if (ret < 0) { + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n", + ret); + goto err_alloc; + } + clkdata->xtal_ena = ret & WM831X_XTAL_ENA; + + if (!clk_register(wm831x->dev, &wm831x_xtal_ops, &clkdata->xtal_hw, + "xtal")) { + ret = -EINVAL; + goto err_alloc; + } + + if (!clk_register(wm831x->dev, &wm831x_fll_ops, &clkdata->fll_hw, + "fll")) { + ret = -EINVAL; + goto err_xtal; + } + + if (!clk_register(wm831x->dev, &wm831x_clkout_ops, &clkdata->clkout_hw, + "clkout")) { + ret = -EINVAL; + goto err_fll; + } + + dev_set_drvdata(&pdev->dev, clkdata); + + return 0; + +err_fll: + clk_unregister(clkdata->fll_hw.clk); +err_xtal: + clk_unregister(clkdata->xtal_hw.clk); +err_alloc: + kfree(clkdata); + return ret; +} + +static __devexit int wm831x_clk_remove(struct platform_device *pdev) +{ + struct wm831x_clk *clkdata = dev_get_drvdata(&pdev->dev); + + clk_unregister(clkdata->clkout_hw.clk); + clk_unregister(clkdata->fll_hw.clk); + clk_unregister(clkdata->xtal_hw.clk); + kfree(clkdata); + + return 0; +} + +static struct platform_driver wm831x_clk_driver = { + .probe = wm831x_clk_probe, + .remove = __devexit_p(wm831x_clk_remove), + .driver = { + .name = "wm831x-clk", + .owner = THIS_MODULE, + }, +}; + +static int __init wm831x_clk_init(void) +{ + int ret; + + ret = platform_driver_register(&wm831x_clk_driver); + if (ret != 0) + pr_err("Failed to register WM831x clock driver: %d\n", ret); + + return ret; +} +module_init(wm831x_clk_init); + +static void __exit wm831x_clk_exit(void) +{ + platform_driver_unregister(&wm831x_clk_driver); +} +module_exit(wm831x_clk_exit); + +/* Module information */ +MODULE_AUTHOR("Mark Brown <broonie@opensource.wolfsonmicro.com>"); +MODULE_DESCRIPTION("WM831x clock driver"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS("platform:wm831x-clk"); -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 6/6] clk: Add initial WM831x clock driver @ 2011-07-11 2:53 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 2:53 UTC (permalink / raw) To: linux-arm-kernel The WM831x and WM832x series of PMICs contain a flexible clocking subsystem intended to provide always on and system core clocks. It features: - A 32.768kHz crystal oscillator which can optionally be used to pass through an externally generated clock. - A FLL which can be clocked from either the 32.768kHz oscillator or the CLKIN pin. - A CLKOUT pin which can bring out either the oscillator or the FLL output. - The 32.768kHz clock can also optionally be brought out on the GPIO pins of the device. This driver fully supports the 32.768kHz oscillator and CLKOUT. The FLL is supported only in AUTO mode, the full flexibility of the FLL cannot currently be used. The use of clock references other than the internal oscillator is not currently supported, and since clk_set_parent() is not implemented in the generic clock API the clock tree configuration cannot be changed at runtime. Due to a lack of access to systems where the core SoC has been converted to use the generic clock API this driver has been compile tested only. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- MAINTAINERS | 1 + drivers/clk/Kconfig | 5 + drivers/clk/Makefile | 1 + drivers/clk/clk-wm831x.c | 389 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 396 insertions(+), 0 deletions(-) create mode 100644 drivers/clk/clk-wm831x.c diff --git a/MAINTAINERS b/MAINTAINERS index ae563fa..c234756 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6969,6 +6969,7 @@ T: git git://opensource.wolfsonmicro.com/linux-2.6-audioplus W: http://opensource.wolfsonmicro.com/content/linux-drivers-wolfson-devices S: Supported F: Documentation/hwmon/wm83?? +F: drivers/clk/clk-wm83*.c F: drivers/leds/leds-wm83*.c F: drivers/mfd/wm8*.c F: drivers/power/wm83*.c diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index 1fd0070..7f6eec2 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -10,6 +10,7 @@ config GENERIC_CLK_BUILD_TEST depends on EXPERIMENTAL && GENERIC_CLK select GENERIC_CLK_FIXED select GENERIC_CLK_GATE + select GENERIC_CLK_WM831X if MFD_WM831X=y help Enable all possible generic clock drivers. This is only useful for improving build coverage, it is not useful for @@ -22,3 +23,7 @@ config GENERIC_CLK_FIXED config GENERIC_CLK_GATE bool depends on GENERIC_CLK + +config GENERIC_CLK_WM831X + tristate + depends on GENERIC_CLK && MFD_WM831X diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index d186446..6628ad5 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -3,3 +3,4 @@ obj-$(CONFIG_CLKDEV_LOOKUP) += clkdev.o obj-$(CONFIG_GENERIC_CLK) += clk.o obj-$(CONFIG_GENERIC_CLK_FIXED) += clk-fixed.o obj-$(CONFIG_GENERIC_CLK_GATE) += clk-gate.o +obj-$(CONFIG_GENERIC_CLK_WM831X) += clk-wm831x.o diff --git a/drivers/clk/clk-wm831x.c b/drivers/clk/clk-wm831x.c new file mode 100644 index 0000000..82782f1 --- /dev/null +++ b/drivers/clk/clk-wm831x.c @@ -0,0 +1,389 @@ +/* + * WM831x clock control + * + * Copyright 2011 Wolfson Microelectronics PLC. + * + * Author: Mark Brown <broonie@opensource.wolfsonmicro.com> + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + * + */ + +#include <linux/clk.h> +#include <linux/module.h> +#include <linux/slab.h> +#include <linux/platform_device.h> +#include <linux/mfd/wm831x/core.h> + +struct wm831x_clk { + struct wm831x *wm831x; + struct clk_hw xtal_hw; + struct clk_hw fll_hw; + struct clk_hw clkout_hw; + bool xtal_ena; +}; + +static int wm831x_xtal_enable(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + xtal_hw); + + if (clkdata->xtal_ena) + return 0; + else + return -EPERM; +} + +static unsigned long wm831x_xtal_recalc_rate(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + xtal_hw); + + if (clkdata->xtal_ena) + return 32768; + else + return 0; +} + +static long wm831x_xtal_round_rate(struct clk_hw *hw, unsigned long rate) +{ + return wm831x_xtal_recalc_rate(hw); +} + +static const struct clk_hw_ops wm831x_xtal_ops = { + .enable = wm831x_xtal_enable, + .recalc_rate = wm831x_xtal_recalc_rate, + .round_rate = wm831x_xtal_round_rate, +}; + +static const unsigned long wm831x_fll_auto_rates[] = { + 2048000, + 11289600, + 12000000, + 12288000, + 19200000, + 22579600, + 24000000, + 24576000, +}; + +static bool wm831x_fll_enabled(struct wm831x *wm831x) +{ + int ret; + + ret = wm831x_reg_read(wm831x, WM831X_FLL_CONTROL_1); + if (ret < 0) { + dev_err(wm831x->dev, "Unable to read FLL_CONTROL_1: %d\n", + ret); + return true; + } + + if (ret & WM831X_FLL_ENA) + return true; + else + return false; +} + +static int wm831x_fll_prepare(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + fll_hw); + struct wm831x *wm831x = clkdata->wm831x; + int ret; + + ret = wm831x_set_bits(wm831x, WM831X_FLL_CONTROL_2, + WM831X_FLL_ENA, WM831X_FLL_ENA); + if (ret != 0) + dev_crit(wm831x->dev, "Failed to enable FLL: %d\n", ret); + + return ret; +} + +static void wm831x_fll_unprepare(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + fll_hw); + struct wm831x *wm831x = clkdata->wm831x; + int ret; + + ret = wm831x_set_bits(wm831x, WM831X_FLL_CONTROL_2, WM831X_FLL_ENA, 0); + if (ret != 0) + dev_crit(wm831x->dev, "Failed to disaable FLL: %d\n", ret); +} + +static unsigned long wm831x_fll_recalc_rate(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + fll_hw); + struct wm831x *wm831x = clkdata->wm831x; + int ret; + + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2); + if (ret < 0) { + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n", + ret); + return 0; + } + + if (ret & WM831X_FLL_AUTO) + return wm831x_fll_auto_rates[ret & WM831X_FLL_AUTO_FREQ_MASK]; + + dev_err(wm831x->dev, "FLL only supported in AUTO mode\n"); + return 0; +} + +static int wm831x_fll_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *parent_rate) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + fll_hw); + struct wm831x *wm831x = clkdata->wm831x; + int i; + + for (i = 0; i < ARRAY_SIZE(wm831x_fll_auto_rates); i++) + if (wm831x_fll_auto_rates[i] == rate) + break; + if (i == ARRAY_SIZE(wm831x_fll_auto_rates)) + return -EINVAL; + + if (wm831x_fll_enabled(wm831x)) + return -EPERM; + + return wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_2, + WM831X_FLL_AUTO_FREQ_MASK, i); +} + +static struct clk *wm831x_fll_get_parent(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + fll_hw); + struct wm831x *wm831x = clkdata->wm831x; + int ret; + + /* AUTO mode is always clocked from the crystal */ + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2); + if (ret < 0) { + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n", + ret); + return NULL; + } + + if (ret & WM831X_FLL_AUTO) + return clkdata->xtal_hw.clk; + + ret = wm831x_reg_read(wm831x, WM831X_FLL_CONTROL_5); + if (ret < 0) { + dev_err(wm831x->dev, "Unable to read FLL_CONTROL_5: %d\n", + ret); + return NULL; + } + + switch (ret & WM831X_FLL_CLK_SRC_MASK) { + case 0: + return clkdata->xtal_hw.clk; + case 1: + dev_warn(wm831x->dev, + "FLL clocked from CLKIN not yet supported\n"); + return NULL; + default: + dev_err(wm831x->dev, "Unsupported FLL clock source %d\n", + ret & WM831X_FLL_CLK_SRC_MASK); + return NULL; + } +} + +static const struct clk_hw_ops wm831x_fll_ops = { + .prepare = wm831x_fll_prepare, + .unprepare = wm831x_fll_unprepare, + .recalc_rate = wm831x_fll_recalc_rate, + .set_rate = wm831x_fll_set_rate, + .get_parent = wm831x_fll_get_parent, +}; + +static int wm831x_clkout_prepare(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + clkout_hw); + struct wm831x *wm831x = clkdata->wm831x; + int ret; + + ret = wm831x_reg_unlock(wm831x); + if (ret != 0) { + dev_crit(wm831x->dev, "Failed to lock registers: %d\n", ret); + return ret; + } + + ret = wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_1, + WM831X_CLKOUT_ENA, WM831X_CLKOUT_ENA); + if (ret != 0) + dev_crit(wm831x->dev, "Failed to enable CLKOUT: %d\n", ret); + + wm831x_reg_lock(wm831x); + + return ret; +} + +static void wm831x_clkout_unprepare(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + clkout_hw); + struct wm831x *wm831x = clkdata->wm831x; + int ret; + + ret = wm831x_reg_unlock(wm831x); + if (ret != 0) { + dev_crit(wm831x->dev, "Failed to lock registers: %d\n", ret); + return; + } + + ret = wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_1, + WM831X_CLKOUT_ENA, 0); + if (ret != 0) + dev_crit(wm831x->dev, "Failed to disable CLKOUT: %d\n", ret); + + wm831x_reg_lock(wm831x); +} + +static unsigned long wm831x_clkout_recalc_rate(struct clk_hw *hw) +{ + return clk_get_rate(clk_get_parent(hw->clk)); +} + +static long wm831x_clkout_round_rate(struct clk_hw *hw, unsigned long rate) +{ + return clk_round_rate(clk_get_parent(hw->clk), rate); +} + +static int wm831x_clkout_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *parent_rate) +{ + *parent_rate = rate; + return CLK_SET_RATE_PROPAGATE; +} + +static struct clk *wm831x_clkout_get_parent(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + clkout_hw); + struct wm831x *wm831x = clkdata->wm831x; + int ret; + + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_1); + if (ret < 0) { + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_1: %d\n", + ret); + return NULL; + } + + if (ret & WM831X_CLKOUT_SRC) + return clkdata->xtal_hw.clk; + else + return clkdata->fll_hw.clk; +} + +static const struct clk_hw_ops wm831x_clkout_ops = { + .prepare = wm831x_clkout_prepare, + .unprepare = wm831x_clkout_unprepare, + .recalc_rate = wm831x_clkout_recalc_rate, + .round_rate = wm831x_clkout_round_rate, + .set_rate = wm831x_clkout_set_rate, + .get_parent = wm831x_clkout_get_parent, +}; + +static __devinit int wm831x_clk_probe(struct platform_device *pdev) +{ + struct wm831x *wm831x = dev_get_drvdata(pdev->dev.parent); + struct wm831x_clk *clkdata; + int ret; + + clkdata = kzalloc(sizeof(*clkdata), GFP_KERNEL); + if (!clkdata) + return -ENOMEM; + + /* XTAL_ENA can only be set via OTP/InstantConfig so just read once */ + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2); + if (ret < 0) { + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n", + ret); + goto err_alloc; + } + clkdata->xtal_ena = ret & WM831X_XTAL_ENA; + + if (!clk_register(wm831x->dev, &wm831x_xtal_ops, &clkdata->xtal_hw, + "xtal")) { + ret = -EINVAL; + goto err_alloc; + } + + if (!clk_register(wm831x->dev, &wm831x_fll_ops, &clkdata->fll_hw, + "fll")) { + ret = -EINVAL; + goto err_xtal; + } + + if (!clk_register(wm831x->dev, &wm831x_clkout_ops, &clkdata->clkout_hw, + "clkout")) { + ret = -EINVAL; + goto err_fll; + } + + dev_set_drvdata(&pdev->dev, clkdata); + + return 0; + +err_fll: + clk_unregister(clkdata->fll_hw.clk); +err_xtal: + clk_unregister(clkdata->xtal_hw.clk); +err_alloc: + kfree(clkdata); + return ret; +} + +static __devexit int wm831x_clk_remove(struct platform_device *pdev) +{ + struct wm831x_clk *clkdata = dev_get_drvdata(&pdev->dev); + + clk_unregister(clkdata->clkout_hw.clk); + clk_unregister(clkdata->fll_hw.clk); + clk_unregister(clkdata->xtal_hw.clk); + kfree(clkdata); + + return 0; +} + +static struct platform_driver wm831x_clk_driver = { + .probe = wm831x_clk_probe, + .remove = __devexit_p(wm831x_clk_remove), + .driver = { + .name = "wm831x-clk", + .owner = THIS_MODULE, + }, +}; + +static int __init wm831x_clk_init(void) +{ + int ret; + + ret = platform_driver_register(&wm831x_clk_driver); + if (ret != 0) + pr_err("Failed to register WM831x clock driver: %d\n", ret); + + return ret; +} +module_init(wm831x_clk_init); + +static void __exit wm831x_clk_exit(void) +{ + platform_driver_unregister(&wm831x_clk_driver); +} +module_exit(wm831x_clk_exit); + +/* Module information */ +MODULE_AUTHOR("Mark Brown <broonie@opensource.wolfsonmicro.com>"); +MODULE_DESCRIPTION("WM831x clock driver"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS("platform:wm831x-clk"); -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 6/6] clk: Add initial WM831x clock driver @ 2011-07-11 2:53 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 2:53 UTC (permalink / raw) To: Jeremy Kerr Cc: Grant Likely, linux-kernel, linux-arm-kernel, linux-sh, patches, Mark Brown The WM831x and WM832x series of PMICs contain a flexible clocking subsystem intended to provide always on and system core clocks. It features: - A 32.768kHz crystal oscillator which can optionally be used to pass through an externally generated clock. - A FLL which can be clocked from either the 32.768kHz oscillator or the CLKIN pin. - A CLKOUT pin which can bring out either the oscillator or the FLL output. - The 32.768kHz clock can also optionally be brought out on the GPIO pins of the device. This driver fully supports the 32.768kHz oscillator and CLKOUT. The FLL is supported only in AUTO mode, the full flexibility of the FLL cannot currently be used. The use of clock references other than the internal oscillator is not currently supported, and since clk_set_parent() is not implemented in the generic clock API the clock tree configuration cannot be changed at runtime. Due to a lack of access to systems where the core SoC has been converted to use the generic clock API this driver has been compile tested only. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- MAINTAINERS | 1 + drivers/clk/Kconfig | 5 + drivers/clk/Makefile | 1 + drivers/clk/clk-wm831x.c | 389 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 396 insertions(+), 0 deletions(-) create mode 100644 drivers/clk/clk-wm831x.c diff --git a/MAINTAINERS b/MAINTAINERS index ae563fa..c234756 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6969,6 +6969,7 @@ T: git git://opensource.wolfsonmicro.com/linux-2.6-audioplus W: http://opensource.wolfsonmicro.com/content/linux-drivers-wolfson-devices S: Supported F: Documentation/hwmon/wm83?? +F: drivers/clk/clk-wm83*.c F: drivers/leds/leds-wm83*.c F: drivers/mfd/wm8*.c F: drivers/power/wm83*.c diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index 1fd0070..7f6eec2 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -10,6 +10,7 @@ config GENERIC_CLK_BUILD_TEST depends on EXPERIMENTAL && GENERIC_CLK select GENERIC_CLK_FIXED select GENERIC_CLK_GATE + select GENERIC_CLK_WM831X if MFD_WM831X=y help Enable all possible generic clock drivers. This is only useful for improving build coverage, it is not useful for @@ -22,3 +23,7 @@ config GENERIC_CLK_FIXED config GENERIC_CLK_GATE bool depends on GENERIC_CLK + +config GENERIC_CLK_WM831X + tristate + depends on GENERIC_CLK && MFD_WM831X diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index d186446..6628ad5 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -3,3 +3,4 @@ obj-$(CONFIG_CLKDEV_LOOKUP) += clkdev.o obj-$(CONFIG_GENERIC_CLK) += clk.o obj-$(CONFIG_GENERIC_CLK_FIXED) += clk-fixed.o obj-$(CONFIG_GENERIC_CLK_GATE) += clk-gate.o +obj-$(CONFIG_GENERIC_CLK_WM831X) += clk-wm831x.o diff --git a/drivers/clk/clk-wm831x.c b/drivers/clk/clk-wm831x.c new file mode 100644 index 0000000..82782f1 --- /dev/null +++ b/drivers/clk/clk-wm831x.c @@ -0,0 +1,389 @@ +/* + * WM831x clock control + * + * Copyright 2011 Wolfson Microelectronics PLC. + * + * Author: Mark Brown <broonie@opensource.wolfsonmicro.com> + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + * + */ + +#include <linux/clk.h> +#include <linux/module.h> +#include <linux/slab.h> +#include <linux/platform_device.h> +#include <linux/mfd/wm831x/core.h> + +struct wm831x_clk { + struct wm831x *wm831x; + struct clk_hw xtal_hw; + struct clk_hw fll_hw; + struct clk_hw clkout_hw; + bool xtal_ena; +}; + +static int wm831x_xtal_enable(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + xtal_hw); + + if (clkdata->xtal_ena) + return 0; + else + return -EPERM; +} + +static unsigned long wm831x_xtal_recalc_rate(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + xtal_hw); + + if (clkdata->xtal_ena) + return 32768; + else + return 0; +} + +static long wm831x_xtal_round_rate(struct clk_hw *hw, unsigned long rate) +{ + return wm831x_xtal_recalc_rate(hw); +} + +static const struct clk_hw_ops wm831x_xtal_ops = { + .enable = wm831x_xtal_enable, + .recalc_rate = wm831x_xtal_recalc_rate, + .round_rate = wm831x_xtal_round_rate, +}; + +static const unsigned long wm831x_fll_auto_rates[] = { + 2048000, + 11289600, + 12000000, + 12288000, + 19200000, + 22579600, + 24000000, + 24576000, +}; + +static bool wm831x_fll_enabled(struct wm831x *wm831x) +{ + int ret; + + ret = wm831x_reg_read(wm831x, WM831X_FLL_CONTROL_1); + if (ret < 0) { + dev_err(wm831x->dev, "Unable to read FLL_CONTROL_1: %d\n", + ret); + return true; + } + + if (ret & WM831X_FLL_ENA) + return true; + else + return false; +} + +static int wm831x_fll_prepare(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + fll_hw); + struct wm831x *wm831x = clkdata->wm831x; + int ret; + + ret = wm831x_set_bits(wm831x, WM831X_FLL_CONTROL_2, + WM831X_FLL_ENA, WM831X_FLL_ENA); + if (ret != 0) + dev_crit(wm831x->dev, "Failed to enable FLL: %d\n", ret); + + return ret; +} + +static void wm831x_fll_unprepare(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + fll_hw); + struct wm831x *wm831x = clkdata->wm831x; + int ret; + + ret = wm831x_set_bits(wm831x, WM831X_FLL_CONTROL_2, WM831X_FLL_ENA, 0); + if (ret != 0) + dev_crit(wm831x->dev, "Failed to disaable FLL: %d\n", ret); +} + +static unsigned long wm831x_fll_recalc_rate(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + fll_hw); + struct wm831x *wm831x = clkdata->wm831x; + int ret; + + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2); + if (ret < 0) { + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n", + ret); + return 0; + } + + if (ret & WM831X_FLL_AUTO) + return wm831x_fll_auto_rates[ret & WM831X_FLL_AUTO_FREQ_MASK]; + + dev_err(wm831x->dev, "FLL only supported in AUTO mode\n"); + return 0; +} + +static int wm831x_fll_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *parent_rate) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + fll_hw); + struct wm831x *wm831x = clkdata->wm831x; + int i; + + for (i = 0; i < ARRAY_SIZE(wm831x_fll_auto_rates); i++) + if (wm831x_fll_auto_rates[i] == rate) + break; + if (i == ARRAY_SIZE(wm831x_fll_auto_rates)) + return -EINVAL; + + if (wm831x_fll_enabled(wm831x)) + return -EPERM; + + return wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_2, + WM831X_FLL_AUTO_FREQ_MASK, i); +} + +static struct clk *wm831x_fll_get_parent(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + fll_hw); + struct wm831x *wm831x = clkdata->wm831x; + int ret; + + /* AUTO mode is always clocked from the crystal */ + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2); + if (ret < 0) { + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n", + ret); + return NULL; + } + + if (ret & WM831X_FLL_AUTO) + return clkdata->xtal_hw.clk; + + ret = wm831x_reg_read(wm831x, WM831X_FLL_CONTROL_5); + if (ret < 0) { + dev_err(wm831x->dev, "Unable to read FLL_CONTROL_5: %d\n", + ret); + return NULL; + } + + switch (ret & WM831X_FLL_CLK_SRC_MASK) { + case 0: + return clkdata->xtal_hw.clk; + case 1: + dev_warn(wm831x->dev, + "FLL clocked from CLKIN not yet supported\n"); + return NULL; + default: + dev_err(wm831x->dev, "Unsupported FLL clock source %d\n", + ret & WM831X_FLL_CLK_SRC_MASK); + return NULL; + } +} + +static const struct clk_hw_ops wm831x_fll_ops = { + .prepare = wm831x_fll_prepare, + .unprepare = wm831x_fll_unprepare, + .recalc_rate = wm831x_fll_recalc_rate, + .set_rate = wm831x_fll_set_rate, + .get_parent = wm831x_fll_get_parent, +}; + +static int wm831x_clkout_prepare(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + clkout_hw); + struct wm831x *wm831x = clkdata->wm831x; + int ret; + + ret = wm831x_reg_unlock(wm831x); + if (ret != 0) { + dev_crit(wm831x->dev, "Failed to lock registers: %d\n", ret); + return ret; + } + + ret = wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_1, + WM831X_CLKOUT_ENA, WM831X_CLKOUT_ENA); + if (ret != 0) + dev_crit(wm831x->dev, "Failed to enable CLKOUT: %d\n", ret); + + wm831x_reg_lock(wm831x); + + return ret; +} + +static void wm831x_clkout_unprepare(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + clkout_hw); + struct wm831x *wm831x = clkdata->wm831x; + int ret; + + ret = wm831x_reg_unlock(wm831x); + if (ret != 0) { + dev_crit(wm831x->dev, "Failed to lock registers: %d\n", ret); + return; + } + + ret = wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_1, + WM831X_CLKOUT_ENA, 0); + if (ret != 0) + dev_crit(wm831x->dev, "Failed to disable CLKOUT: %d\n", ret); + + wm831x_reg_lock(wm831x); +} + +static unsigned long wm831x_clkout_recalc_rate(struct clk_hw *hw) +{ + return clk_get_rate(clk_get_parent(hw->clk)); +} + +static long wm831x_clkout_round_rate(struct clk_hw *hw, unsigned long rate) +{ + return clk_round_rate(clk_get_parent(hw->clk), rate); +} + +static int wm831x_clkout_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *parent_rate) +{ + *parent_rate = rate; + return CLK_SET_RATE_PROPAGATE; +} + +static struct clk *wm831x_clkout_get_parent(struct clk_hw *hw) +{ + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, + clkout_hw); + struct wm831x *wm831x = clkdata->wm831x; + int ret; + + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_1); + if (ret < 0) { + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_1: %d\n", + ret); + return NULL; + } + + if (ret & WM831X_CLKOUT_SRC) + return clkdata->xtal_hw.clk; + else + return clkdata->fll_hw.clk; +} + +static const struct clk_hw_ops wm831x_clkout_ops = { + .prepare = wm831x_clkout_prepare, + .unprepare = wm831x_clkout_unprepare, + .recalc_rate = wm831x_clkout_recalc_rate, + .round_rate = wm831x_clkout_round_rate, + .set_rate = wm831x_clkout_set_rate, + .get_parent = wm831x_clkout_get_parent, +}; + +static __devinit int wm831x_clk_probe(struct platform_device *pdev) +{ + struct wm831x *wm831x = dev_get_drvdata(pdev->dev.parent); + struct wm831x_clk *clkdata; + int ret; + + clkdata = kzalloc(sizeof(*clkdata), GFP_KERNEL); + if (!clkdata) + return -ENOMEM; + + /* XTAL_ENA can only be set via OTP/InstantConfig so just read once */ + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2); + if (ret < 0) { + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n", + ret); + goto err_alloc; + } + clkdata->xtal_ena = ret & WM831X_XTAL_ENA; + + if (!clk_register(wm831x->dev, &wm831x_xtal_ops, &clkdata->xtal_hw, + "xtal")) { + ret = -EINVAL; + goto err_alloc; + } + + if (!clk_register(wm831x->dev, &wm831x_fll_ops, &clkdata->fll_hw, + "fll")) { + ret = -EINVAL; + goto err_xtal; + } + + if (!clk_register(wm831x->dev, &wm831x_clkout_ops, &clkdata->clkout_hw, + "clkout")) { + ret = -EINVAL; + goto err_fll; + } + + dev_set_drvdata(&pdev->dev, clkdata); + + return 0; + +err_fll: + clk_unregister(clkdata->fll_hw.clk); +err_xtal: + clk_unregister(clkdata->xtal_hw.clk); +err_alloc: + kfree(clkdata); + return ret; +} + +static __devexit int wm831x_clk_remove(struct platform_device *pdev) +{ + struct wm831x_clk *clkdata = dev_get_drvdata(&pdev->dev); + + clk_unregister(clkdata->clkout_hw.clk); + clk_unregister(clkdata->fll_hw.clk); + clk_unregister(clkdata->xtal_hw.clk); + kfree(clkdata); + + return 0; +} + +static struct platform_driver wm831x_clk_driver = { + .probe = wm831x_clk_probe, + .remove = __devexit_p(wm831x_clk_remove), + .driver = { + .name = "wm831x-clk", + .owner = THIS_MODULE, + }, +}; + +static int __init wm831x_clk_init(void) +{ + int ret; + + ret = platform_driver_register(&wm831x_clk_driver); + if (ret != 0) + pr_err("Failed to register WM831x clock driver: %d\n", ret); + + return ret; +} +module_init(wm831x_clk_init); + +static void __exit wm831x_clk_exit(void) +{ + platform_driver_unregister(&wm831x_clk_driver); +} +module_exit(wm831x_clk_exit); + +/* Module information */ +MODULE_AUTHOR("Mark Brown <broonie@opensource.wolfsonmicro.com>"); +MODULE_DESCRIPTION("WM831x clock driver"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS("platform:wm831x-clk"); -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 72+ messages in thread
* Re: [PATCH 6/6] clk: Add initial WM831x clock driver 2011-07-11 2:53 ` Mark Brown (?) @ 2011-07-15 2:53 ` Grant Likely -1 siblings, 0 replies; 72+ messages in thread From: Grant Likely @ 2011-07-15 2:53 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 11:53:57AM +0900, Mark Brown wrote: > The WM831x and WM832x series of PMICs contain a flexible clocking > subsystem intended to provide always on and system core clocks. It > features: > > - A 32.768kHz crystal oscillator which can optionally be used to pass > through an externally generated clock. > - A FLL which can be clocked from either the 32.768kHz oscillator or > the CLKIN pin. > - A CLKOUT pin which can bring out either the oscillator or the FLL > output. > - The 32.768kHz clock can also optionally be brought out on the GPIO > pins of the device. > > This driver fully supports the 32.768kHz oscillator and CLKOUT. The FLL > is supported only in AUTO mode, the full flexibility of the FLL cannot > currently be used. The use of clock references other than the internal > oscillator is not currently supported, and since clk_set_parent() is not > implemented in the generic clock API the clock tree configuration cannot > be changed at runtime. > > Due to a lack of access to systems where the core SoC has been converted > to use the generic clock API this driver has been compile tested only. Generally seems okay. Minor comments below. g. > > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> > --- > MAINTAINERS | 1 + > drivers/clk/Kconfig | 5 + > drivers/clk/Makefile | 1 + > drivers/clk/clk-wm831x.c | 389 ++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 396 insertions(+), 0 deletions(-) > create mode 100644 drivers/clk/clk-wm831x.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index ae563fa..c234756 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6969,6 +6969,7 @@ T: git git://opensource.wolfsonmicro.com/linux-2.6-audioplus > W: http://opensource.wolfsonmicro.com/content/linux-drivers-wolfson-devices > S: Supported > F: Documentation/hwmon/wm83?? > +F: drivers/clk/clk-wm83*.c > F: drivers/leds/leds-wm83*.c > F: drivers/mfd/wm8*.c > F: drivers/power/wm83*.c > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index 1fd0070..7f6eec2 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -10,6 +10,7 @@ config GENERIC_CLK_BUILD_TEST > depends on EXPERIMENTAL && GENERIC_CLK > select GENERIC_CLK_FIXED > select GENERIC_CLK_GATE > + select GENERIC_CLK_WM831X if MFD_WM831X=y Hmmm, this could get unwieldy in a hurry. > help > Enable all possible generic clock drivers. This is only > useful for improving build coverage, it is not useful for > @@ -22,3 +23,7 @@ config GENERIC_CLK_FIXED > config GENERIC_CLK_GATE > bool > depends on GENERIC_CLK > + > +config GENERIC_CLK_WM831X > + tristate > + depends on GENERIC_CLK && MFD_WM831X > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > index d186446..6628ad5 100644 > --- a/drivers/clk/Makefile > +++ b/drivers/clk/Makefile > @@ -3,3 +3,4 @@ obj-$(CONFIG_CLKDEV_LOOKUP) += clkdev.o > obj-$(CONFIG_GENERIC_CLK) += clk.o > obj-$(CONFIG_GENERIC_CLK_FIXED) += clk-fixed.o > obj-$(CONFIG_GENERIC_CLK_GATE) += clk-gate.o > +obj-$(CONFIG_GENERIC_CLK_WM831X) += clk-wm831x.o > diff --git a/drivers/clk/clk-wm831x.c b/drivers/clk/clk-wm831x.c > new file mode 100644 > index 0000000..82782f1 > --- /dev/null > +++ b/drivers/clk/clk-wm831x.c > @@ -0,0 +1,389 @@ > +/* > + * WM831x clock control > + * > + * Copyright 2011 Wolfson Microelectronics PLC. > + * > + * Author: Mark Brown <broonie@opensource.wolfsonmicro.com> > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by the > + * Free Software Foundation; either version 2 of the License, or (at your > + * option) any later version. > + * > + */ > + > +#include <linux/clk.h> > +#include <linux/module.h> > +#include <linux/slab.h> > +#include <linux/platform_device.h> > +#include <linux/mfd/wm831x/core.h> > + > +struct wm831x_clk { > + struct wm831x *wm831x; > + struct clk_hw xtal_hw; > + struct clk_hw fll_hw; > + struct clk_hw clkout_hw; > + bool xtal_ena; > +}; > + > +static int wm831x_xtal_enable(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + xtal_hw); This container of is used 10 times. A static inline would be reasonable. > + > + if (clkdata->xtal_ena) > + return 0; > + else > + return -EPERM; > +} Nit: return clkdata->xtal_ena ? 0 : -EPERM; Just makes for more concise code. > + > +static unsigned long wm831x_xtal_recalc_rate(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + xtal_hw); > + > + if (clkdata->xtal_ena) > + return 32768; > + else > + return 0; > +} > + > +static long wm831x_xtal_round_rate(struct clk_hw *hw, unsigned long rate) > +{ > + return wm831x_xtal_recalc_rate(hw); > +} > + > +static const struct clk_hw_ops wm831x_xtal_ops = { > + .enable = wm831x_xtal_enable, > + .recalc_rate = wm831x_xtal_recalc_rate, > + .round_rate = wm831x_xtal_round_rate, > +}; > + > +static const unsigned long wm831x_fll_auto_rates[] = { > + 2048000, > + 11289600, > + 12000000, > + 12288000, > + 19200000, > + 22579600, > + 24000000, > + 24576000, > +}; > + > +static bool wm831x_fll_enabled(struct wm831x *wm831x) > +{ > + int ret; > + > + ret = wm831x_reg_read(wm831x, WM831X_FLL_CONTROL_1); > + if (ret < 0) { > + dev_err(wm831x->dev, "Unable to read FLL_CONTROL_1: %d\n", > + ret); > + return true; > + } > + > + if (ret & WM831X_FLL_ENA) > + return true; > + else > + return false; similarly, return (ret & WM831X_FLL_ENA) != 0; > +} > + > +static int wm831x_fll_prepare(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + fll_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int ret; > + > + ret = wm831x_set_bits(wm831x, WM831X_FLL_CONTROL_2, > + WM831X_FLL_ENA, WM831X_FLL_ENA); > + if (ret != 0) > + dev_crit(wm831x->dev, "Failed to enable FLL: %d\n", ret); > + > + return ret; > +} > + > +static void wm831x_fll_unprepare(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + fll_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int ret; > + > + ret = wm831x_set_bits(wm831x, WM831X_FLL_CONTROL_2, WM831X_FLL_ENA, 0); > + if (ret != 0) > + dev_crit(wm831x->dev, "Failed to disaable FLL: %d\n", ret); > +} > + > +static unsigned long wm831x_fll_recalc_rate(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + fll_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int ret; > + > + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2); > + if (ret < 0) { > + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n", > + ret); > + return 0; > + } > + > + if (ret & WM831X_FLL_AUTO) > + return wm831x_fll_auto_rates[ret & WM831X_FLL_AUTO_FREQ_MASK]; > + > + dev_err(wm831x->dev, "FLL only supported in AUTO mode\n"); > + return 0; > +} > + > +static int wm831x_fll_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long *parent_rate) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + fll_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(wm831x_fll_auto_rates); i++) > + if (wm831x_fll_auto_rates[i] = rate) > + break; > + if (i = ARRAY_SIZE(wm831x_fll_auto_rates)) > + return -EINVAL; > + > + if (wm831x_fll_enabled(wm831x)) > + return -EPERM; > + > + return wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_2, > + WM831X_FLL_AUTO_FREQ_MASK, i); > +} > + > +static struct clk *wm831x_fll_get_parent(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + fll_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int ret; > + > + /* AUTO mode is always clocked from the crystal */ > + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2); > + if (ret < 0) { > + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n", > + ret); > + return NULL; > + } > + > + if (ret & WM831X_FLL_AUTO) > + return clkdata->xtal_hw.clk; > + > + ret = wm831x_reg_read(wm831x, WM831X_FLL_CONTROL_5); > + if (ret < 0) { > + dev_err(wm831x->dev, "Unable to read FLL_CONTROL_5: %d\n", > + ret); > + return NULL; > + } > + > + switch (ret & WM831X_FLL_CLK_SRC_MASK) { > + case 0: > + return clkdata->xtal_hw.clk; > + case 1: > + dev_warn(wm831x->dev, > + "FLL clocked from CLKIN not yet supported\n"); > + return NULL; > + default: > + dev_err(wm831x->dev, "Unsupported FLL clock source %d\n", > + ret & WM831X_FLL_CLK_SRC_MASK); > + return NULL; > + } > +} > + > +static const struct clk_hw_ops wm831x_fll_ops = { > + .prepare = wm831x_fll_prepare, > + .unprepare = wm831x_fll_unprepare, > + .recalc_rate = wm831x_fll_recalc_rate, > + .set_rate = wm831x_fll_set_rate, > + .get_parent = wm831x_fll_get_parent, > +}; > + > +static int wm831x_clkout_prepare(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + clkout_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int ret; > + > + ret = wm831x_reg_unlock(wm831x); > + if (ret != 0) { > + dev_crit(wm831x->dev, "Failed to lock registers: %d\n", ret); > + return ret; > + } > + > + ret = wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_1, > + WM831X_CLKOUT_ENA, WM831X_CLKOUT_ENA); > + if (ret != 0) > + dev_crit(wm831x->dev, "Failed to enable CLKOUT: %d\n", ret); > + > + wm831x_reg_lock(wm831x); > + > + return ret; > +} > + > +static void wm831x_clkout_unprepare(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + clkout_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int ret; > + > + ret = wm831x_reg_unlock(wm831x); > + if (ret != 0) { > + dev_crit(wm831x->dev, "Failed to lock registers: %d\n", ret); > + return; > + } > + > + ret = wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_1, > + WM831X_CLKOUT_ENA, 0); > + if (ret != 0) > + dev_crit(wm831x->dev, "Failed to disable CLKOUT: %d\n", ret); > + > + wm831x_reg_lock(wm831x); > +} > + > +static unsigned long wm831x_clkout_recalc_rate(struct clk_hw *hw) > +{ > + return clk_get_rate(clk_get_parent(hw->clk)); > +} > + > +static long wm831x_clkout_round_rate(struct clk_hw *hw, unsigned long rate) > +{ > + return clk_round_rate(clk_get_parent(hw->clk), rate); > +} > + > +static int wm831x_clkout_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long *parent_rate) > +{ > + *parent_rate = rate; > + return CLK_SET_RATE_PROPAGATE; > +} > + > +static struct clk *wm831x_clkout_get_parent(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + clkout_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int ret; > + > + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_1); > + if (ret < 0) { > + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_1: %d\n", > + ret); > + return NULL; > + } > + > + if (ret & WM831X_CLKOUT_SRC) > + return clkdata->xtal_hw.clk; > + else > + return clkdata->fll_hw.clk; > +} > + > +static const struct clk_hw_ops wm831x_clkout_ops = { > + .prepare = wm831x_clkout_prepare, > + .unprepare = wm831x_clkout_unprepare, > + .recalc_rate = wm831x_clkout_recalc_rate, > + .round_rate = wm831x_clkout_round_rate, > + .set_rate = wm831x_clkout_set_rate, > + .get_parent = wm831x_clkout_get_parent, > +}; > + > +static __devinit int wm831x_clk_probe(struct platform_device *pdev) > +{ > + struct wm831x *wm831x = dev_get_drvdata(pdev->dev.parent); > + struct wm831x_clk *clkdata; > + int ret; > + > + clkdata = kzalloc(sizeof(*clkdata), GFP_KERNEL); > + if (!clkdata) > + return -ENOMEM; > + > + /* XTAL_ENA can only be set via OTP/InstantConfig so just read once */ > + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2); > + if (ret < 0) { > + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n", > + ret); > + goto err_alloc; > + } > + clkdata->xtal_ena = ret & WM831X_XTAL_ENA; > + > + if (!clk_register(wm831x->dev, &wm831x_xtal_ops, &clkdata->xtal_hw, > + "xtal")) { > + ret = -EINVAL; > + goto err_alloc; > + } > + > + if (!clk_register(wm831x->dev, &wm831x_fll_ops, &clkdata->fll_hw, > + "fll")) { > + ret = -EINVAL; > + goto err_xtal; > + } > + > + if (!clk_register(wm831x->dev, &wm831x_clkout_ops, &clkdata->clkout_hw, > + "clkout")) { > + ret = -EINVAL; > + goto err_fll; > + } How common will this pattern be? Is there need for a clk_register_many() variant? > + > + dev_set_drvdata(&pdev->dev, clkdata); > + > + return 0; > + > +err_fll: > + clk_unregister(clkdata->fll_hw.clk); > +err_xtal: > + clk_unregister(clkdata->xtal_hw.clk); > +err_alloc: > + kfree(clkdata); > + return ret; > +} > + > +static __devexit int wm831x_clk_remove(struct platform_device *pdev) > +{ > + struct wm831x_clk *clkdata = dev_get_drvdata(&pdev->dev); > + > + clk_unregister(clkdata->clkout_hw.clk); > + clk_unregister(clkdata->fll_hw.clk); > + clk_unregister(clkdata->xtal_hw.clk); > + kfree(clkdata); > + > + return 0; > +} > + > +static struct platform_driver wm831x_clk_driver = { > + .probe = wm831x_clk_probe, > + .remove = __devexit_p(wm831x_clk_remove), > + .driver = { > + .name = "wm831x-clk", > + .owner = THIS_MODULE, > + }, > +}; > + > +static int __init wm831x_clk_init(void) > +{ > + int ret; > + > + ret = platform_driver_register(&wm831x_clk_driver); > + if (ret != 0) > + pr_err("Failed to register WM831x clock driver: %d\n", ret); > + > + return ret; > +} Driver registration is very well implemented and debugged. Just do "return platform_driver_register();" > +module_init(wm831x_clk_init); > + > +static void __exit wm831x_clk_exit(void) > +{ > + platform_driver_unregister(&wm831x_clk_driver); > +} > +module_exit(wm831x_clk_exit); > + > +/* Module information */ > +MODULE_AUTHOR("Mark Brown <broonie@opensource.wolfsonmicro.com>"); > +MODULE_DESCRIPTION("WM831x clock driver"); > +MODULE_LICENSE("GPL"); > +MODULE_ALIAS("platform:wm831x-clk"); > -- > 1.7.5.4 > ^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 6/6] clk: Add initial WM831x clock driver @ 2011-07-15 2:53 ` Grant Likely 0 siblings, 0 replies; 72+ messages in thread From: Grant Likely @ 2011-07-15 2:53 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 11:53:57AM +0900, Mark Brown wrote: > The WM831x and WM832x series of PMICs contain a flexible clocking > subsystem intended to provide always on and system core clocks. It > features: > > - A 32.768kHz crystal oscillator which can optionally be used to pass > through an externally generated clock. > - A FLL which can be clocked from either the 32.768kHz oscillator or > the CLKIN pin. > - A CLKOUT pin which can bring out either the oscillator or the FLL > output. > - The 32.768kHz clock can also optionally be brought out on the GPIO > pins of the device. > > This driver fully supports the 32.768kHz oscillator and CLKOUT. The FLL > is supported only in AUTO mode, the full flexibility of the FLL cannot > currently be used. The use of clock references other than the internal > oscillator is not currently supported, and since clk_set_parent() is not > implemented in the generic clock API the clock tree configuration cannot > be changed at runtime. > > Due to a lack of access to systems where the core SoC has been converted > to use the generic clock API this driver has been compile tested only. Generally seems okay. Minor comments below. g. > > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> > --- > MAINTAINERS | 1 + > drivers/clk/Kconfig | 5 + > drivers/clk/Makefile | 1 + > drivers/clk/clk-wm831x.c | 389 ++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 396 insertions(+), 0 deletions(-) > create mode 100644 drivers/clk/clk-wm831x.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index ae563fa..c234756 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6969,6 +6969,7 @@ T: git git://opensource.wolfsonmicro.com/linux-2.6-audioplus > W: http://opensource.wolfsonmicro.com/content/linux-drivers-wolfson-devices > S: Supported > F: Documentation/hwmon/wm83?? > +F: drivers/clk/clk-wm83*.c > F: drivers/leds/leds-wm83*.c > F: drivers/mfd/wm8*.c > F: drivers/power/wm83*.c > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index 1fd0070..7f6eec2 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -10,6 +10,7 @@ config GENERIC_CLK_BUILD_TEST > depends on EXPERIMENTAL && GENERIC_CLK > select GENERIC_CLK_FIXED > select GENERIC_CLK_GATE > + select GENERIC_CLK_WM831X if MFD_WM831X=y Hmmm, this could get unwieldy in a hurry. > help > Enable all possible generic clock drivers. This is only > useful for improving build coverage, it is not useful for > @@ -22,3 +23,7 @@ config GENERIC_CLK_FIXED > config GENERIC_CLK_GATE > bool > depends on GENERIC_CLK > + > +config GENERIC_CLK_WM831X > + tristate > + depends on GENERIC_CLK && MFD_WM831X > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > index d186446..6628ad5 100644 > --- a/drivers/clk/Makefile > +++ b/drivers/clk/Makefile > @@ -3,3 +3,4 @@ obj-$(CONFIG_CLKDEV_LOOKUP) += clkdev.o > obj-$(CONFIG_GENERIC_CLK) += clk.o > obj-$(CONFIG_GENERIC_CLK_FIXED) += clk-fixed.o > obj-$(CONFIG_GENERIC_CLK_GATE) += clk-gate.o > +obj-$(CONFIG_GENERIC_CLK_WM831X) += clk-wm831x.o > diff --git a/drivers/clk/clk-wm831x.c b/drivers/clk/clk-wm831x.c > new file mode 100644 > index 0000000..82782f1 > --- /dev/null > +++ b/drivers/clk/clk-wm831x.c > @@ -0,0 +1,389 @@ > +/* > + * WM831x clock control > + * > + * Copyright 2011 Wolfson Microelectronics PLC. > + * > + * Author: Mark Brown <broonie@opensource.wolfsonmicro.com> > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by the > + * Free Software Foundation; either version 2 of the License, or (at your > + * option) any later version. > + * > + */ > + > +#include <linux/clk.h> > +#include <linux/module.h> > +#include <linux/slab.h> > +#include <linux/platform_device.h> > +#include <linux/mfd/wm831x/core.h> > + > +struct wm831x_clk { > + struct wm831x *wm831x; > + struct clk_hw xtal_hw; > + struct clk_hw fll_hw; > + struct clk_hw clkout_hw; > + bool xtal_ena; > +}; > + > +static int wm831x_xtal_enable(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + xtal_hw); This container of is used 10 times. A static inline would be reasonable. > + > + if (clkdata->xtal_ena) > + return 0; > + else > + return -EPERM; > +} Nit: return clkdata->xtal_ena ? 0 : -EPERM; Just makes for more concise code. > + > +static unsigned long wm831x_xtal_recalc_rate(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + xtal_hw); > + > + if (clkdata->xtal_ena) > + return 32768; > + else > + return 0; > +} > + > +static long wm831x_xtal_round_rate(struct clk_hw *hw, unsigned long rate) > +{ > + return wm831x_xtal_recalc_rate(hw); > +} > + > +static const struct clk_hw_ops wm831x_xtal_ops = { > + .enable = wm831x_xtal_enable, > + .recalc_rate = wm831x_xtal_recalc_rate, > + .round_rate = wm831x_xtal_round_rate, > +}; > + > +static const unsigned long wm831x_fll_auto_rates[] = { > + 2048000, > + 11289600, > + 12000000, > + 12288000, > + 19200000, > + 22579600, > + 24000000, > + 24576000, > +}; > + > +static bool wm831x_fll_enabled(struct wm831x *wm831x) > +{ > + int ret; > + > + ret = wm831x_reg_read(wm831x, WM831X_FLL_CONTROL_1); > + if (ret < 0) { > + dev_err(wm831x->dev, "Unable to read FLL_CONTROL_1: %d\n", > + ret); > + return true; > + } > + > + if (ret & WM831X_FLL_ENA) > + return true; > + else > + return false; similarly, return (ret & WM831X_FLL_ENA) != 0; > +} > + > +static int wm831x_fll_prepare(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + fll_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int ret; > + > + ret = wm831x_set_bits(wm831x, WM831X_FLL_CONTROL_2, > + WM831X_FLL_ENA, WM831X_FLL_ENA); > + if (ret != 0) > + dev_crit(wm831x->dev, "Failed to enable FLL: %d\n", ret); > + > + return ret; > +} > + > +static void wm831x_fll_unprepare(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + fll_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int ret; > + > + ret = wm831x_set_bits(wm831x, WM831X_FLL_CONTROL_2, WM831X_FLL_ENA, 0); > + if (ret != 0) > + dev_crit(wm831x->dev, "Failed to disaable FLL: %d\n", ret); > +} > + > +static unsigned long wm831x_fll_recalc_rate(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + fll_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int ret; > + > + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2); > + if (ret < 0) { > + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n", > + ret); > + return 0; > + } > + > + if (ret & WM831X_FLL_AUTO) > + return wm831x_fll_auto_rates[ret & WM831X_FLL_AUTO_FREQ_MASK]; > + > + dev_err(wm831x->dev, "FLL only supported in AUTO mode\n"); > + return 0; > +} > + > +static int wm831x_fll_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long *parent_rate) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + fll_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(wm831x_fll_auto_rates); i++) > + if (wm831x_fll_auto_rates[i] == rate) > + break; > + if (i == ARRAY_SIZE(wm831x_fll_auto_rates)) > + return -EINVAL; > + > + if (wm831x_fll_enabled(wm831x)) > + return -EPERM; > + > + return wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_2, > + WM831X_FLL_AUTO_FREQ_MASK, i); > +} > + > +static struct clk *wm831x_fll_get_parent(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + fll_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int ret; > + > + /* AUTO mode is always clocked from the crystal */ > + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2); > + if (ret < 0) { > + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n", > + ret); > + return NULL; > + } > + > + if (ret & WM831X_FLL_AUTO) > + return clkdata->xtal_hw.clk; > + > + ret = wm831x_reg_read(wm831x, WM831X_FLL_CONTROL_5); > + if (ret < 0) { > + dev_err(wm831x->dev, "Unable to read FLL_CONTROL_5: %d\n", > + ret); > + return NULL; > + } > + > + switch (ret & WM831X_FLL_CLK_SRC_MASK) { > + case 0: > + return clkdata->xtal_hw.clk; > + case 1: > + dev_warn(wm831x->dev, > + "FLL clocked from CLKIN not yet supported\n"); > + return NULL; > + default: > + dev_err(wm831x->dev, "Unsupported FLL clock source %d\n", > + ret & WM831X_FLL_CLK_SRC_MASK); > + return NULL; > + } > +} > + > +static const struct clk_hw_ops wm831x_fll_ops = { > + .prepare = wm831x_fll_prepare, > + .unprepare = wm831x_fll_unprepare, > + .recalc_rate = wm831x_fll_recalc_rate, > + .set_rate = wm831x_fll_set_rate, > + .get_parent = wm831x_fll_get_parent, > +}; > + > +static int wm831x_clkout_prepare(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + clkout_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int ret; > + > + ret = wm831x_reg_unlock(wm831x); > + if (ret != 0) { > + dev_crit(wm831x->dev, "Failed to lock registers: %d\n", ret); > + return ret; > + } > + > + ret = wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_1, > + WM831X_CLKOUT_ENA, WM831X_CLKOUT_ENA); > + if (ret != 0) > + dev_crit(wm831x->dev, "Failed to enable CLKOUT: %d\n", ret); > + > + wm831x_reg_lock(wm831x); > + > + return ret; > +} > + > +static void wm831x_clkout_unprepare(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + clkout_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int ret; > + > + ret = wm831x_reg_unlock(wm831x); > + if (ret != 0) { > + dev_crit(wm831x->dev, "Failed to lock registers: %d\n", ret); > + return; > + } > + > + ret = wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_1, > + WM831X_CLKOUT_ENA, 0); > + if (ret != 0) > + dev_crit(wm831x->dev, "Failed to disable CLKOUT: %d\n", ret); > + > + wm831x_reg_lock(wm831x); > +} > + > +static unsigned long wm831x_clkout_recalc_rate(struct clk_hw *hw) > +{ > + return clk_get_rate(clk_get_parent(hw->clk)); > +} > + > +static long wm831x_clkout_round_rate(struct clk_hw *hw, unsigned long rate) > +{ > + return clk_round_rate(clk_get_parent(hw->clk), rate); > +} > + > +static int wm831x_clkout_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long *parent_rate) > +{ > + *parent_rate = rate; > + return CLK_SET_RATE_PROPAGATE; > +} > + > +static struct clk *wm831x_clkout_get_parent(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + clkout_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int ret; > + > + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_1); > + if (ret < 0) { > + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_1: %d\n", > + ret); > + return NULL; > + } > + > + if (ret & WM831X_CLKOUT_SRC) > + return clkdata->xtal_hw.clk; > + else > + return clkdata->fll_hw.clk; > +} > + > +static const struct clk_hw_ops wm831x_clkout_ops = { > + .prepare = wm831x_clkout_prepare, > + .unprepare = wm831x_clkout_unprepare, > + .recalc_rate = wm831x_clkout_recalc_rate, > + .round_rate = wm831x_clkout_round_rate, > + .set_rate = wm831x_clkout_set_rate, > + .get_parent = wm831x_clkout_get_parent, > +}; > + > +static __devinit int wm831x_clk_probe(struct platform_device *pdev) > +{ > + struct wm831x *wm831x = dev_get_drvdata(pdev->dev.parent); > + struct wm831x_clk *clkdata; > + int ret; > + > + clkdata = kzalloc(sizeof(*clkdata), GFP_KERNEL); > + if (!clkdata) > + return -ENOMEM; > + > + /* XTAL_ENA can only be set via OTP/InstantConfig so just read once */ > + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2); > + if (ret < 0) { > + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n", > + ret); > + goto err_alloc; > + } > + clkdata->xtal_ena = ret & WM831X_XTAL_ENA; > + > + if (!clk_register(wm831x->dev, &wm831x_xtal_ops, &clkdata->xtal_hw, > + "xtal")) { > + ret = -EINVAL; > + goto err_alloc; > + } > + > + if (!clk_register(wm831x->dev, &wm831x_fll_ops, &clkdata->fll_hw, > + "fll")) { > + ret = -EINVAL; > + goto err_xtal; > + } > + > + if (!clk_register(wm831x->dev, &wm831x_clkout_ops, &clkdata->clkout_hw, > + "clkout")) { > + ret = -EINVAL; > + goto err_fll; > + } How common will this pattern be? Is there need for a clk_register_many() variant? > + > + dev_set_drvdata(&pdev->dev, clkdata); > + > + return 0; > + > +err_fll: > + clk_unregister(clkdata->fll_hw.clk); > +err_xtal: > + clk_unregister(clkdata->xtal_hw.clk); > +err_alloc: > + kfree(clkdata); > + return ret; > +} > + > +static __devexit int wm831x_clk_remove(struct platform_device *pdev) > +{ > + struct wm831x_clk *clkdata = dev_get_drvdata(&pdev->dev); > + > + clk_unregister(clkdata->clkout_hw.clk); > + clk_unregister(clkdata->fll_hw.clk); > + clk_unregister(clkdata->xtal_hw.clk); > + kfree(clkdata); > + > + return 0; > +} > + > +static struct platform_driver wm831x_clk_driver = { > + .probe = wm831x_clk_probe, > + .remove = __devexit_p(wm831x_clk_remove), > + .driver = { > + .name = "wm831x-clk", > + .owner = THIS_MODULE, > + }, > +}; > + > +static int __init wm831x_clk_init(void) > +{ > + int ret; > + > + ret = platform_driver_register(&wm831x_clk_driver); > + if (ret != 0) > + pr_err("Failed to register WM831x clock driver: %d\n", ret); > + > + return ret; > +} Driver registration is very well implemented and debugged. Just do "return platform_driver_register();" > +module_init(wm831x_clk_init); > + > +static void __exit wm831x_clk_exit(void) > +{ > + platform_driver_unregister(&wm831x_clk_driver); > +} > +module_exit(wm831x_clk_exit); > + > +/* Module information */ > +MODULE_AUTHOR("Mark Brown <broonie@opensource.wolfsonmicro.com>"); > +MODULE_DESCRIPTION("WM831x clock driver"); > +MODULE_LICENSE("GPL"); > +MODULE_ALIAS("platform:wm831x-clk"); > -- > 1.7.5.4 > ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 6/6] clk: Add initial WM831x clock driver @ 2011-07-15 2:53 ` Grant Likely 0 siblings, 0 replies; 72+ messages in thread From: Grant Likely @ 2011-07-15 2:53 UTC (permalink / raw) To: Mark Brown Cc: Jeremy Kerr, Grant Likely, linux-kernel, linux-arm-kernel, linux-sh, patches On Mon, Jul 11, 2011 at 11:53:57AM +0900, Mark Brown wrote: > The WM831x and WM832x series of PMICs contain a flexible clocking > subsystem intended to provide always on and system core clocks. It > features: > > - A 32.768kHz crystal oscillator which can optionally be used to pass > through an externally generated clock. > - A FLL which can be clocked from either the 32.768kHz oscillator or > the CLKIN pin. > - A CLKOUT pin which can bring out either the oscillator or the FLL > output. > - The 32.768kHz clock can also optionally be brought out on the GPIO > pins of the device. > > This driver fully supports the 32.768kHz oscillator and CLKOUT. The FLL > is supported only in AUTO mode, the full flexibility of the FLL cannot > currently be used. The use of clock references other than the internal > oscillator is not currently supported, and since clk_set_parent() is not > implemented in the generic clock API the clock tree configuration cannot > be changed at runtime. > > Due to a lack of access to systems where the core SoC has been converted > to use the generic clock API this driver has been compile tested only. Generally seems okay. Minor comments below. g. > > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> > --- > MAINTAINERS | 1 + > drivers/clk/Kconfig | 5 + > drivers/clk/Makefile | 1 + > drivers/clk/clk-wm831x.c | 389 ++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 396 insertions(+), 0 deletions(-) > create mode 100644 drivers/clk/clk-wm831x.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index ae563fa..c234756 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6969,6 +6969,7 @@ T: git git://opensource.wolfsonmicro.com/linux-2.6-audioplus > W: http://opensource.wolfsonmicro.com/content/linux-drivers-wolfson-devices > S: Supported > F: Documentation/hwmon/wm83?? > +F: drivers/clk/clk-wm83*.c > F: drivers/leds/leds-wm83*.c > F: drivers/mfd/wm8*.c > F: drivers/power/wm83*.c > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index 1fd0070..7f6eec2 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -10,6 +10,7 @@ config GENERIC_CLK_BUILD_TEST > depends on EXPERIMENTAL && GENERIC_CLK > select GENERIC_CLK_FIXED > select GENERIC_CLK_GATE > + select GENERIC_CLK_WM831X if MFD_WM831X=y Hmmm, this could get unwieldy in a hurry. > help > Enable all possible generic clock drivers. This is only > useful for improving build coverage, it is not useful for > @@ -22,3 +23,7 @@ config GENERIC_CLK_FIXED > config GENERIC_CLK_GATE > bool > depends on GENERIC_CLK > + > +config GENERIC_CLK_WM831X > + tristate > + depends on GENERIC_CLK && MFD_WM831X > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > index d186446..6628ad5 100644 > --- a/drivers/clk/Makefile > +++ b/drivers/clk/Makefile > @@ -3,3 +3,4 @@ obj-$(CONFIG_CLKDEV_LOOKUP) += clkdev.o > obj-$(CONFIG_GENERIC_CLK) += clk.o > obj-$(CONFIG_GENERIC_CLK_FIXED) += clk-fixed.o > obj-$(CONFIG_GENERIC_CLK_GATE) += clk-gate.o > +obj-$(CONFIG_GENERIC_CLK_WM831X) += clk-wm831x.o > diff --git a/drivers/clk/clk-wm831x.c b/drivers/clk/clk-wm831x.c > new file mode 100644 > index 0000000..82782f1 > --- /dev/null > +++ b/drivers/clk/clk-wm831x.c > @@ -0,0 +1,389 @@ > +/* > + * WM831x clock control > + * > + * Copyright 2011 Wolfson Microelectronics PLC. > + * > + * Author: Mark Brown <broonie@opensource.wolfsonmicro.com> > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by the > + * Free Software Foundation; either version 2 of the License, or (at your > + * option) any later version. > + * > + */ > + > +#include <linux/clk.h> > +#include <linux/module.h> > +#include <linux/slab.h> > +#include <linux/platform_device.h> > +#include <linux/mfd/wm831x/core.h> > + > +struct wm831x_clk { > + struct wm831x *wm831x; > + struct clk_hw xtal_hw; > + struct clk_hw fll_hw; > + struct clk_hw clkout_hw; > + bool xtal_ena; > +}; > + > +static int wm831x_xtal_enable(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + xtal_hw); This container of is used 10 times. A static inline would be reasonable. > + > + if (clkdata->xtal_ena) > + return 0; > + else > + return -EPERM; > +} Nit: return clkdata->xtal_ena ? 0 : -EPERM; Just makes for more concise code. > + > +static unsigned long wm831x_xtal_recalc_rate(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + xtal_hw); > + > + if (clkdata->xtal_ena) > + return 32768; > + else > + return 0; > +} > + > +static long wm831x_xtal_round_rate(struct clk_hw *hw, unsigned long rate) > +{ > + return wm831x_xtal_recalc_rate(hw); > +} > + > +static const struct clk_hw_ops wm831x_xtal_ops = { > + .enable = wm831x_xtal_enable, > + .recalc_rate = wm831x_xtal_recalc_rate, > + .round_rate = wm831x_xtal_round_rate, > +}; > + > +static const unsigned long wm831x_fll_auto_rates[] = { > + 2048000, > + 11289600, > + 12000000, > + 12288000, > + 19200000, > + 22579600, > + 24000000, > + 24576000, > +}; > + > +static bool wm831x_fll_enabled(struct wm831x *wm831x) > +{ > + int ret; > + > + ret = wm831x_reg_read(wm831x, WM831X_FLL_CONTROL_1); > + if (ret < 0) { > + dev_err(wm831x->dev, "Unable to read FLL_CONTROL_1: %d\n", > + ret); > + return true; > + } > + > + if (ret & WM831X_FLL_ENA) > + return true; > + else > + return false; similarly, return (ret & WM831X_FLL_ENA) != 0; > +} > + > +static int wm831x_fll_prepare(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + fll_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int ret; > + > + ret = wm831x_set_bits(wm831x, WM831X_FLL_CONTROL_2, > + WM831X_FLL_ENA, WM831X_FLL_ENA); > + if (ret != 0) > + dev_crit(wm831x->dev, "Failed to enable FLL: %d\n", ret); > + > + return ret; > +} > + > +static void wm831x_fll_unprepare(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + fll_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int ret; > + > + ret = wm831x_set_bits(wm831x, WM831X_FLL_CONTROL_2, WM831X_FLL_ENA, 0); > + if (ret != 0) > + dev_crit(wm831x->dev, "Failed to disaable FLL: %d\n", ret); > +} > + > +static unsigned long wm831x_fll_recalc_rate(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + fll_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int ret; > + > + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2); > + if (ret < 0) { > + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n", > + ret); > + return 0; > + } > + > + if (ret & WM831X_FLL_AUTO) > + return wm831x_fll_auto_rates[ret & WM831X_FLL_AUTO_FREQ_MASK]; > + > + dev_err(wm831x->dev, "FLL only supported in AUTO mode\n"); > + return 0; > +} > + > +static int wm831x_fll_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long *parent_rate) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + fll_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(wm831x_fll_auto_rates); i++) > + if (wm831x_fll_auto_rates[i] == rate) > + break; > + if (i == ARRAY_SIZE(wm831x_fll_auto_rates)) > + return -EINVAL; > + > + if (wm831x_fll_enabled(wm831x)) > + return -EPERM; > + > + return wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_2, > + WM831X_FLL_AUTO_FREQ_MASK, i); > +} > + > +static struct clk *wm831x_fll_get_parent(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + fll_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int ret; > + > + /* AUTO mode is always clocked from the crystal */ > + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2); > + if (ret < 0) { > + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n", > + ret); > + return NULL; > + } > + > + if (ret & WM831X_FLL_AUTO) > + return clkdata->xtal_hw.clk; > + > + ret = wm831x_reg_read(wm831x, WM831X_FLL_CONTROL_5); > + if (ret < 0) { > + dev_err(wm831x->dev, "Unable to read FLL_CONTROL_5: %d\n", > + ret); > + return NULL; > + } > + > + switch (ret & WM831X_FLL_CLK_SRC_MASK) { > + case 0: > + return clkdata->xtal_hw.clk; > + case 1: > + dev_warn(wm831x->dev, > + "FLL clocked from CLKIN not yet supported\n"); > + return NULL; > + default: > + dev_err(wm831x->dev, "Unsupported FLL clock source %d\n", > + ret & WM831X_FLL_CLK_SRC_MASK); > + return NULL; > + } > +} > + > +static const struct clk_hw_ops wm831x_fll_ops = { > + .prepare = wm831x_fll_prepare, > + .unprepare = wm831x_fll_unprepare, > + .recalc_rate = wm831x_fll_recalc_rate, > + .set_rate = wm831x_fll_set_rate, > + .get_parent = wm831x_fll_get_parent, > +}; > + > +static int wm831x_clkout_prepare(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + clkout_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int ret; > + > + ret = wm831x_reg_unlock(wm831x); > + if (ret != 0) { > + dev_crit(wm831x->dev, "Failed to lock registers: %d\n", ret); > + return ret; > + } > + > + ret = wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_1, > + WM831X_CLKOUT_ENA, WM831X_CLKOUT_ENA); > + if (ret != 0) > + dev_crit(wm831x->dev, "Failed to enable CLKOUT: %d\n", ret); > + > + wm831x_reg_lock(wm831x); > + > + return ret; > +} > + > +static void wm831x_clkout_unprepare(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + clkout_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int ret; > + > + ret = wm831x_reg_unlock(wm831x); > + if (ret != 0) { > + dev_crit(wm831x->dev, "Failed to lock registers: %d\n", ret); > + return; > + } > + > + ret = wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_1, > + WM831X_CLKOUT_ENA, 0); > + if (ret != 0) > + dev_crit(wm831x->dev, "Failed to disable CLKOUT: %d\n", ret); > + > + wm831x_reg_lock(wm831x); > +} > + > +static unsigned long wm831x_clkout_recalc_rate(struct clk_hw *hw) > +{ > + return clk_get_rate(clk_get_parent(hw->clk)); > +} > + > +static long wm831x_clkout_round_rate(struct clk_hw *hw, unsigned long rate) > +{ > + return clk_round_rate(clk_get_parent(hw->clk), rate); > +} > + > +static int wm831x_clkout_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long *parent_rate) > +{ > + *parent_rate = rate; > + return CLK_SET_RATE_PROPAGATE; > +} > + > +static struct clk *wm831x_clkout_get_parent(struct clk_hw *hw) > +{ > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > + clkout_hw); > + struct wm831x *wm831x = clkdata->wm831x; > + int ret; > + > + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_1); > + if (ret < 0) { > + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_1: %d\n", > + ret); > + return NULL; > + } > + > + if (ret & WM831X_CLKOUT_SRC) > + return clkdata->xtal_hw.clk; > + else > + return clkdata->fll_hw.clk; > +} > + > +static const struct clk_hw_ops wm831x_clkout_ops = { > + .prepare = wm831x_clkout_prepare, > + .unprepare = wm831x_clkout_unprepare, > + .recalc_rate = wm831x_clkout_recalc_rate, > + .round_rate = wm831x_clkout_round_rate, > + .set_rate = wm831x_clkout_set_rate, > + .get_parent = wm831x_clkout_get_parent, > +}; > + > +static __devinit int wm831x_clk_probe(struct platform_device *pdev) > +{ > + struct wm831x *wm831x = dev_get_drvdata(pdev->dev.parent); > + struct wm831x_clk *clkdata; > + int ret; > + > + clkdata = kzalloc(sizeof(*clkdata), GFP_KERNEL); > + if (!clkdata) > + return -ENOMEM; > + > + /* XTAL_ENA can only be set via OTP/InstantConfig so just read once */ > + ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2); > + if (ret < 0) { > + dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n", > + ret); > + goto err_alloc; > + } > + clkdata->xtal_ena = ret & WM831X_XTAL_ENA; > + > + if (!clk_register(wm831x->dev, &wm831x_xtal_ops, &clkdata->xtal_hw, > + "xtal")) { > + ret = -EINVAL; > + goto err_alloc; > + } > + > + if (!clk_register(wm831x->dev, &wm831x_fll_ops, &clkdata->fll_hw, > + "fll")) { > + ret = -EINVAL; > + goto err_xtal; > + } > + > + if (!clk_register(wm831x->dev, &wm831x_clkout_ops, &clkdata->clkout_hw, > + "clkout")) { > + ret = -EINVAL; > + goto err_fll; > + } How common will this pattern be? Is there need for a clk_register_many() variant? > + > + dev_set_drvdata(&pdev->dev, clkdata); > + > + return 0; > + > +err_fll: > + clk_unregister(clkdata->fll_hw.clk); > +err_xtal: > + clk_unregister(clkdata->xtal_hw.clk); > +err_alloc: > + kfree(clkdata); > + return ret; > +} > + > +static __devexit int wm831x_clk_remove(struct platform_device *pdev) > +{ > + struct wm831x_clk *clkdata = dev_get_drvdata(&pdev->dev); > + > + clk_unregister(clkdata->clkout_hw.clk); > + clk_unregister(clkdata->fll_hw.clk); > + clk_unregister(clkdata->xtal_hw.clk); > + kfree(clkdata); > + > + return 0; > +} > + > +static struct platform_driver wm831x_clk_driver = { > + .probe = wm831x_clk_probe, > + .remove = __devexit_p(wm831x_clk_remove), > + .driver = { > + .name = "wm831x-clk", > + .owner = THIS_MODULE, > + }, > +}; > + > +static int __init wm831x_clk_init(void) > +{ > + int ret; > + > + ret = platform_driver_register(&wm831x_clk_driver); > + if (ret != 0) > + pr_err("Failed to register WM831x clock driver: %d\n", ret); > + > + return ret; > +} Driver registration is very well implemented and debugged. Just do "return platform_driver_register();" > +module_init(wm831x_clk_init); > + > +static void __exit wm831x_clk_exit(void) > +{ > + platform_driver_unregister(&wm831x_clk_driver); > +} > +module_exit(wm831x_clk_exit); > + > +/* Module information */ > +MODULE_AUTHOR("Mark Brown <broonie@opensource.wolfsonmicro.com>"); > +MODULE_DESCRIPTION("WM831x clock driver"); > +MODULE_LICENSE("GPL"); > +MODULE_ALIAS("platform:wm831x-clk"); > -- > 1.7.5.4 > ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 6/6] clk: Add initial WM831x clock driver 2011-07-15 2:53 ` Grant Likely (?) @ 2011-07-15 5:05 ` Mark Brown -1 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-15 5:05 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 14, 2011 at 08:53:39PM -0600, Grant Likely wrote: > On Mon, Jul 11, 2011 at 11:53:57AM +0900, Mark Brown wrote: > > @@ -10,6 +10,7 @@ config GENERIC_CLK_BUILD_TEST > > depends on EXPERIMENTAL && GENERIC_CLK > > select GENERIC_CLK_FIXED > > select GENERIC_CLK_GATE > > + select GENERIC_CLK_WM831X if MFD_WM831X=y > Hmmm, this could get unwieldy in a hurry. It's not really any hassle, we've got a one of these in ASoC. The list gets long but if you keep it sorted it's not an issue for merges and otherwise it's just long not complicated. The ability to get build coverage is *really* useful. > > +static int wm831x_xtal_enable(struct clk_hw *hw) > > +{ > > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > > + xtal_hw); > This container of is used 10 times. A static inline would be > reasonable. Not quite - it's used in three different variants (one for each of the clocks). > > + if (clkdata->xtal_ena) > > + return 0; > > + else > > + return -EPERM; > > +} > Nit: return clkdata->xtal_ena ? 0 : -EPERM; > Just makes for more concise code. I have an extremely strong dislike of the ternery operator, I find it does nothing for legibility. > > + if (!clk_register(wm831x->dev, &wm831x_clkout_ops, &clkdata->clkout_hw, > > + "clkout")) { > > + ret = -EINVAL; > > + goto err_fll; > > + } > How common will this pattern be? Is there need for a > clk_register_many() variant? I dunno, I think a lot of the SoCs may be doing one clk per device. But equally well it's not like it'd be hard if someone starts working on the API again. ^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 6/6] clk: Add initial WM831x clock driver @ 2011-07-15 5:05 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-15 5:05 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 14, 2011 at 08:53:39PM -0600, Grant Likely wrote: > On Mon, Jul 11, 2011 at 11:53:57AM +0900, Mark Brown wrote: > > @@ -10,6 +10,7 @@ config GENERIC_CLK_BUILD_TEST > > depends on EXPERIMENTAL && GENERIC_CLK > > select GENERIC_CLK_FIXED > > select GENERIC_CLK_GATE > > + select GENERIC_CLK_WM831X if MFD_WM831X=y > Hmmm, this could get unwieldy in a hurry. It's not really any hassle, we've got a one of these in ASoC. The list gets long but if you keep it sorted it's not an issue for merges and otherwise it's just long not complicated. The ability to get build coverage is *really* useful. > > +static int wm831x_xtal_enable(struct clk_hw *hw) > > +{ > > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > > + xtal_hw); > This container of is used 10 times. A static inline would be > reasonable. Not quite - it's used in three different variants (one for each of the clocks). > > + if (clkdata->xtal_ena) > > + return 0; > > + else > > + return -EPERM; > > +} > Nit: return clkdata->xtal_ena ? 0 : -EPERM; > Just makes for more concise code. I have an extremely strong dislike of the ternery operator, I find it does nothing for legibility. > > + if (!clk_register(wm831x->dev, &wm831x_clkout_ops, &clkdata->clkout_hw, > > + "clkout")) { > > + ret = -EINVAL; > > + goto err_fll; > > + } > How common will this pattern be? Is there need for a > clk_register_many() variant? I dunno, I think a lot of the SoCs may be doing one clk per device. But equally well it's not like it'd be hard if someone starts working on the API again. ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 6/6] clk: Add initial WM831x clock driver @ 2011-07-15 5:05 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-15 5:05 UTC (permalink / raw) To: Grant Likely Cc: Jeremy Kerr, Grant Likely, linux-kernel, linux-arm-kernel, linux-sh, patches On Thu, Jul 14, 2011 at 08:53:39PM -0600, Grant Likely wrote: > On Mon, Jul 11, 2011 at 11:53:57AM +0900, Mark Brown wrote: > > @@ -10,6 +10,7 @@ config GENERIC_CLK_BUILD_TEST > > depends on EXPERIMENTAL && GENERIC_CLK > > select GENERIC_CLK_FIXED > > select GENERIC_CLK_GATE > > + select GENERIC_CLK_WM831X if MFD_WM831X=y > Hmmm, this could get unwieldy in a hurry. It's not really any hassle, we've got a one of these in ASoC. The list gets long but if you keep it sorted it's not an issue for merges and otherwise it's just long not complicated. The ability to get build coverage is *really* useful. > > +static int wm831x_xtal_enable(struct clk_hw *hw) > > +{ > > + struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk, > > + xtal_hw); > This container of is used 10 times. A static inline would be > reasonable. Not quite - it's used in three different variants (one for each of the clocks). > > + if (clkdata->xtal_ena) > > + return 0; > > + else > > + return -EPERM; > > +} > Nit: return clkdata->xtal_ena ? 0 : -EPERM; > Just makes for more concise code. I have an extremely strong dislike of the ternery operator, I find it does nothing for legibility. > > + if (!clk_register(wm831x->dev, &wm831x_clkout_ops, &clkdata->clkout_hw, > > + "clkout")) { > > + ret = -EINVAL; > > + goto err_fll; > > + } > How common will this pattern be? Is there need for a > clk_register_many() variant? I dunno, I think a lot of the SoCs may be doing one clk per device. But equally well it's not like it'd be hard if someone starts working on the API again. ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 6/6] clk: Add initial WM831x clock driver 2011-07-15 5:05 ` Mark Brown (?) @ 2011-07-15 5:14 ` Ryan Mallon -1 siblings, 0 replies; 72+ messages in thread From: Ryan Mallon @ 2011-07-15 5:14 UTC (permalink / raw) To: linux-arm-kernel On 15/07/11 15:05, Mark Brown wrote: > On Thu, Jul 14, 2011 at 08:53:39PM -0600, Grant Likely wrote: >> On Mon, Jul 11, 2011 at 11:53:57AM +0900, Mark Brown wrote: >>> + if (clkdata->xtal_ena) >>> + return 0; >>> + else >>> + return -EPERM; >>> +} >> Nit: return clkdata->xtal_ena ? 0 : -EPERM; >> Just makes for more concise code. > I have an extremely strong dislike of the ternery operator, I find it > does nothing for legibility. > I prefer this: if (!clkdata->xtal_ena) return -EPERM; return 0; } Which makes the error check clear and makes the success case visible right at the bottom of the function. ~Ryan ^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 6/6] clk: Add initial WM831x clock driver @ 2011-07-15 5:14 ` Ryan Mallon 0 siblings, 0 replies; 72+ messages in thread From: Ryan Mallon @ 2011-07-15 5:14 UTC (permalink / raw) To: linux-arm-kernel On 15/07/11 15:05, Mark Brown wrote: > On Thu, Jul 14, 2011 at 08:53:39PM -0600, Grant Likely wrote: >> On Mon, Jul 11, 2011 at 11:53:57AM +0900, Mark Brown wrote: >>> + if (clkdata->xtal_ena) >>> + return 0; >>> + else >>> + return -EPERM; >>> +} >> Nit: return clkdata->xtal_ena ? 0 : -EPERM; >> Just makes for more concise code. > I have an extremely strong dislike of the ternery operator, I find it > does nothing for legibility. > I prefer this: if (!clkdata->xtal_ena) return -EPERM; return 0; } Which makes the error check clear and makes the success case visible right at the bottom of the function. ~Ryan ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 6/6] clk: Add initial WM831x clock driver @ 2011-07-15 5:14 ` Ryan Mallon 0 siblings, 0 replies; 72+ messages in thread From: Ryan Mallon @ 2011-07-15 5:14 UTC (permalink / raw) To: Mark Brown Cc: Grant Likely, Jeremy Kerr, Grant Likely, linux-kernel, linux-arm-kernel, linux-sh, patches On 15/07/11 15:05, Mark Brown wrote: > On Thu, Jul 14, 2011 at 08:53:39PM -0600, Grant Likely wrote: >> On Mon, Jul 11, 2011 at 11:53:57AM +0900, Mark Brown wrote: >>> + if (clkdata->xtal_ena) >>> + return 0; >>> + else >>> + return -EPERM; >>> +} >> Nit: return clkdata->xtal_ena ? 0 : -EPERM; >> Just makes for more concise code. > I have an extremely strong dislike of the ternery operator, I find it > does nothing for legibility. > I prefer this: if (!clkdata->xtal_ena) return -EPERM; return 0; } Which makes the error check clear and makes the success case visible right at the bottom of the function. ~Ryan ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 1/6] clk: Prototype and document clk_register() 2011-07-11 2:53 ` Mark Brown (?) @ 2011-07-15 2:53 ` Grant Likely -1 siblings, 0 replies; 72+ messages in thread From: Grant Likely @ 2011-07-15 2:53 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 11:53:52AM +0900, Mark Brown wrote: > This allows the compiler to ensure drivers are using the correct prototype. > > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> Acked-by: Grant Likely <grant.likely@secretlab.ca> > --- > include/linux/clk.h | 12 ++++++++++++ > 1 files changed, 12 insertions(+), 0 deletions(-) > > diff --git a/include/linux/clk.h b/include/linux/clk.h > index 7c26135..2ca4f66 100644 > --- a/include/linux/clk.h > +++ b/include/linux/clk.h > @@ -136,6 +136,18 @@ extern struct clk_hw_ops clk_gate_ops; > > #endif /* CONFIG_GENERIC_CLK_GATE */ > > +/** > + * clk_register - register and initialize a new clock > + * > + * @ops: ops for the new clock > + * @hw: struct clk_hw to be passed to the ops of the new clock > + * @name: name to use for the new clock > + * > + * Register a new clock with the clk subsytem. Returns either a > + * struct clk for the new clock or a NULL pointer. > + */ > +struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw, > + const char *name); > > #else /* !CONFIG_GENERIC_CLK */ > > -- > 1.7.5.4 > ^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 1/6] clk: Prototype and document clk_register() @ 2011-07-15 2:53 ` Grant Likely 0 siblings, 0 replies; 72+ messages in thread From: Grant Likely @ 2011-07-15 2:53 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 11:53:52AM +0900, Mark Brown wrote: > This allows the compiler to ensure drivers are using the correct prototype. > > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> Acked-by: Grant Likely <grant.likely@secretlab.ca> > --- > include/linux/clk.h | 12 ++++++++++++ > 1 files changed, 12 insertions(+), 0 deletions(-) > > diff --git a/include/linux/clk.h b/include/linux/clk.h > index 7c26135..2ca4f66 100644 > --- a/include/linux/clk.h > +++ b/include/linux/clk.h > @@ -136,6 +136,18 @@ extern struct clk_hw_ops clk_gate_ops; > > #endif /* CONFIG_GENERIC_CLK_GATE */ > > +/** > + * clk_register - register and initialize a new clock > + * > + * @ops: ops for the new clock > + * @hw: struct clk_hw to be passed to the ops of the new clock > + * @name: name to use for the new clock > + * > + * Register a new clock with the clk subsytem. Returns either a > + * struct clk for the new clock or a NULL pointer. > + */ > +struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw, > + const char *name); > > #else /* !CONFIG_GENERIC_CLK */ > > -- > 1.7.5.4 > ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 1/6] clk: Prototype and document clk_register() @ 2011-07-15 2:53 ` Grant Likely 0 siblings, 0 replies; 72+ messages in thread From: Grant Likely @ 2011-07-15 2:53 UTC (permalink / raw) To: Mark Brown Cc: Jeremy Kerr, Grant Likely, linux-kernel, linux-arm-kernel, linux-sh, patches On Mon, Jul 11, 2011 at 11:53:52AM +0900, Mark Brown wrote: > This allows the compiler to ensure drivers are using the correct prototype. > > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> Acked-by: Grant Likely <grant.likely@secretlab.ca> > --- > include/linux/clk.h | 12 ++++++++++++ > 1 files changed, 12 insertions(+), 0 deletions(-) > > diff --git a/include/linux/clk.h b/include/linux/clk.h > index 7c26135..2ca4f66 100644 > --- a/include/linux/clk.h > +++ b/include/linux/clk.h > @@ -136,6 +136,18 @@ extern struct clk_hw_ops clk_gate_ops; > > #endif /* CONFIG_GENERIC_CLK_GATE */ > > +/** > + * clk_register - register and initialize a new clock > + * > + * @ops: ops for the new clock > + * @hw: struct clk_hw to be passed to the ops of the new clock > + * @name: name to use for the new clock > + * > + * Register a new clock with the clk subsytem. Returns either a > + * struct clk for the new clock or a NULL pointer. > + */ > +struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw, > + const char *name); > > #else /* !CONFIG_GENERIC_CLK */ > > -- > 1.7.5.4 > ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks 2011-07-11 2:53 ` Mark Brown (?) @ 2011-07-11 3:57 ` Mark Brown -1 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 3:57 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: Linus, CCing you in as apparently you're taking over the clock API work. Do you need me to forward all the patches to you? > I've just been having a go at implementing generic clk API support for > an off-SoC device on a slow bus, the clocking module in the wm831x/2x > series of PMICs. Unfortunately as I don't currently have access to a > platform that has been converted to use the API I've not actually been > able to test the code but I'm reasonably optimistic that the code will > Just Work(tm) as the API seems fairly straightforward. > > The biggest issue I ran into was that as the clocks are all registered > by name with the API if you've got two instances of the same off-SoC > device in the system you'll not be able to disambiguate between the > clocks it provides. I've added a simple solution for this in the form > of a patch which takes a struct device as an argument and adds that to > the name of the registered clock. This isn't ideal but should give > stable names which systems can use together with clkdev to match clocks > up with their users. I believe that when we have device tree bindings > for clocks we should be able to use the device to access the bindings > and avoid this mangling - Grant, is that correct? > > Otherwise everything seems to work pretty well from the driver side for > these devices, I've added some minor updates which came up naturally > while writing the driver code and seemed fairly obvious. One of these > was a change to provide a user visible Kconfig option for building the > clock drivers, the idea being to make it easier to do build tests. I > did also wonder if it's worth adding a patch to enable the API on x86 > (which doesn't currently have a clock API) for coverage. > > These patches (which include the two I posted yesterday) are against the > series you posted in May with the exception of the wm831x patch which > also has an additional dependency on "mfd: Add WM831x clock control > register definitions" which is due for merge in the next merge window. > If this stuff is all OK for you it would be good if you could include > the wm831x driver in your tree for this, especially given that there's > nothing in -next. > > Mark Brown (6): > clk: Prototype and document clk_register() > clk: Provide a dummy clk_unregister() > clk: Constify struct clk_hw_ops > clk: Add Kconfig option to build all generic clk drivers > clk: Support multiple instances of the same clock provider > clk: Add initial WM831x clock driver > > MAINTAINERS | 1 + > drivers/clk/Kconfig | 16 ++- > drivers/clk/Makefile | 1 + > drivers/clk/clk-wm831x.c | 389 ++++++++++++++++++++++++++++++++++++++++++++++ > drivers/clk/clk.c | 38 ++++- > include/linux/clk.h | 33 ++++ > 6 files changed, 472 insertions(+), 6 deletions(-) > create mode 100644 drivers/clk/clk-wm831x.c ^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks @ 2011-07-11 3:57 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 3:57 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: Linus, CCing you in as apparently you're taking over the clock API work. Do you need me to forward all the patches to you? > I've just been having a go at implementing generic clk API support for > an off-SoC device on a slow bus, the clocking module in the wm831x/2x > series of PMICs. Unfortunately as I don't currently have access to a > platform that has been converted to use the API I've not actually been > able to test the code but I'm reasonably optimistic that the code will > Just Work(tm) as the API seems fairly straightforward. > > The biggest issue I ran into was that as the clocks are all registered > by name with the API if you've got two instances of the same off-SoC > device in the system you'll not be able to disambiguate between the > clocks it provides. I've added a simple solution for this in the form > of a patch which takes a struct device as an argument and adds that to > the name of the registered clock. This isn't ideal but should give > stable names which systems can use together with clkdev to match clocks > up with their users. I believe that when we have device tree bindings > for clocks we should be able to use the device to access the bindings > and avoid this mangling - Grant, is that correct? > > Otherwise everything seems to work pretty well from the driver side for > these devices, I've added some minor updates which came up naturally > while writing the driver code and seemed fairly obvious. One of these > was a change to provide a user visible Kconfig option for building the > clock drivers, the idea being to make it easier to do build tests. I > did also wonder if it's worth adding a patch to enable the API on x86 > (which doesn't currently have a clock API) for coverage. > > These patches (which include the two I posted yesterday) are against the > series you posted in May with the exception of the wm831x patch which > also has an additional dependency on "mfd: Add WM831x clock control > register definitions" which is due for merge in the next merge window. > If this stuff is all OK for you it would be good if you could include > the wm831x driver in your tree for this, especially given that there's > nothing in -next. > > Mark Brown (6): > clk: Prototype and document clk_register() > clk: Provide a dummy clk_unregister() > clk: Constify struct clk_hw_ops > clk: Add Kconfig option to build all generic clk drivers > clk: Support multiple instances of the same clock provider > clk: Add initial WM831x clock driver > > MAINTAINERS | 1 + > drivers/clk/Kconfig | 16 ++- > drivers/clk/Makefile | 1 + > drivers/clk/clk-wm831x.c | 389 ++++++++++++++++++++++++++++++++++++++++++++++ > drivers/clk/clk.c | 38 ++++- > include/linux/clk.h | 33 ++++ > 6 files changed, 472 insertions(+), 6 deletions(-) > create mode 100644 drivers/clk/clk-wm831x.c ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks @ 2011-07-11 3:57 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 3:57 UTC (permalink / raw) To: Jeremy Kerr, Linus Walleij Cc: linux-kernel, linux-arm-kernel, linux-sh, patches, Grant Likely On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: Linus, CCing you in as apparently you're taking over the clock API work. Do you need me to forward all the patches to you? > I've just been having a go at implementing generic clk API support for > an off-SoC device on a slow bus, the clocking module in the wm831x/2x > series of PMICs. Unfortunately as I don't currently have access to a > platform that has been converted to use the API I've not actually been > able to test the code but I'm reasonably optimistic that the code will > Just Work(tm) as the API seems fairly straightforward. > > The biggest issue I ran into was that as the clocks are all registered > by name with the API if you've got two instances of the same off-SoC > device in the system you'll not be able to disambiguate between the > clocks it provides. I've added a simple solution for this in the form > of a patch which takes a struct device as an argument and adds that to > the name of the registered clock. This isn't ideal but should give > stable names which systems can use together with clkdev to match clocks > up with their users. I believe that when we have device tree bindings > for clocks we should be able to use the device to access the bindings > and avoid this mangling - Grant, is that correct? > > Otherwise everything seems to work pretty well from the driver side for > these devices, I've added some minor updates which came up naturally > while writing the driver code and seemed fairly obvious. One of these > was a change to provide a user visible Kconfig option for building the > clock drivers, the idea being to make it easier to do build tests. I > did also wonder if it's worth adding a patch to enable the API on x86 > (which doesn't currently have a clock API) for coverage. > > These patches (which include the two I posted yesterday) are against the > series you posted in May with the exception of the wm831x patch which > also has an additional dependency on "mfd: Add WM831x clock control > register definitions" which is due for merge in the next merge window. > If this stuff is all OK for you it would be good if you could include > the wm831x driver in your tree for this, especially given that there's > nothing in -next. > > Mark Brown (6): > clk: Prototype and document clk_register() > clk: Provide a dummy clk_unregister() > clk: Constify struct clk_hw_ops > clk: Add Kconfig option to build all generic clk drivers > clk: Support multiple instances of the same clock provider > clk: Add initial WM831x clock driver > > MAINTAINERS | 1 + > drivers/clk/Kconfig | 16 ++- > drivers/clk/Makefile | 1 + > drivers/clk/clk-wm831x.c | 389 ++++++++++++++++++++++++++++++++++++++++++++++ > drivers/clk/clk.c | 38 ++++- > include/linux/clk.h | 33 ++++ > 6 files changed, 472 insertions(+), 6 deletions(-) > create mode 100644 drivers/clk/clk-wm831x.c ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks 2011-07-11 3:57 ` Mark Brown (?) @ 2011-07-11 4:30 ` Mike Frysinger -1 siblings, 0 replies; 72+ messages in thread From: Mike Frysinger @ 2011-07-11 4:30 UTC (permalink / raw) To: linux-arm-kernel [-- Attachment #1: Type: Text/Plain, Size: 569 bytes --] On Sunday, July 10, 2011 23:57:40 Mark Brown wrote: > On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: > > Linus, CCing you in as apparently you're taking over the clock API work. > Do you need me to forward all the patches to you? along these lines, i dont think the new people on the cc noticed my earlier e- mail: for future series, could you cc uclinux-dist-devel@blackfin.uclinux.org ? we dont do clock management on Blackfin parts atm, but it's something we would like to start doing. our hardware can easily benefit from this. -mike [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks @ 2011-07-11 4:30 ` Mike Frysinger 0 siblings, 0 replies; 72+ messages in thread From: Mike Frysinger @ 2011-07-11 4:30 UTC (permalink / raw) To: linux-arm-kernel On Sunday, July 10, 2011 23:57:40 Mark Brown wrote: > On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: > > Linus, CCing you in as apparently you're taking over the clock API work. > Do you need me to forward all the patches to you? along these lines, i dont think the new people on the cc noticed my earlier e- mail: for future series, could you cc uclinux-dist-devel at blackfin.uclinux.org ? we dont do clock management on Blackfin parts atm, but it's something we would like to start doing. our hardware can easily benefit from this. -mike -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part. URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110711/69b913bf/attachment.sig> ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks @ 2011-07-11 4:30 ` Mike Frysinger 0 siblings, 0 replies; 72+ messages in thread From: Mike Frysinger @ 2011-07-11 4:30 UTC (permalink / raw) To: Mark Brown Cc: Jeremy Kerr, Linus Walleij, linux-kernel, linux-arm-kernel, linux-sh, patches, Grant Likely [-- Attachment #1: Type: Text/Plain, Size: 569 bytes --] On Sunday, July 10, 2011 23:57:40 Mark Brown wrote: > On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: > > Linus, CCing you in as apparently you're taking over the clock API work. > Do you need me to forward all the patches to you? along these lines, i dont think the new people on the cc noticed my earlier e- mail: for future series, could you cc uclinux-dist-devel@blackfin.uclinux.org ? we dont do clock management on Blackfin parts atm, but it's something we would like to start doing. our hardware can easily benefit from this. -mike [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks 2011-07-11 4:30 ` Mike Frysinger (?) @ 2011-07-11 4:56 ` Barry Song -1 siblings, 0 replies; 72+ messages in thread From: Barry Song @ 2011-07-11 4:56 UTC (permalink / raw) To: linux-arm-kernel 2011/7/11 Mike Frysinger <vapier@gentoo.org>: > On Sunday, July 10, 2011 23:57:40 Mark Brown wrote: >> On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: >> >> Linus, CCing you in as apparently you're taking over the clock API work. >> Do you need me to forward all the patches to you? > > along these lines, i dont think the new people on the cc noticed my earlier e- > mail: > for future series, could you cc uclinux-dist-devel@blackfin.uclinux.org ? we > dont do clock management on Blackfin parts atm, but it's something we would > like to start doing. our hardware can easily benefit from this. Except that, AFAIK, arch/blackfin/mach-xxx is much similar with arch/arm/mach-xxx with a lot of platform, i2c, spi board information. it is probably we can also benefit from device tree as what ARM is doing. but not sure whether it is really worthwhile for the current situation and long term plan of blackfin. Anyway, there are not so many blackfin SoC and boards as ARM. > -mike > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > ^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks @ 2011-07-11 4:56 ` Barry Song 0 siblings, 0 replies; 72+ messages in thread From: Barry Song @ 2011-07-11 4:56 UTC (permalink / raw) To: linux-arm-kernel 2011/7/11 Mike Frysinger <vapier@gentoo.org>: > On Sunday, July 10, 2011 23:57:40 Mark Brown wrote: >> On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: >> >> Linus, CCing you in as apparently you're taking over the clock API work. >> Do you need me to forward all the patches to you? > > along these lines, i dont think the new people on the cc noticed my earlier e- > mail: > for future series, could you cc uclinux-dist-devel at blackfin.uclinux.org ? ?we > dont do clock management on Blackfin parts atm, but it's something we would > like to start doing. ?our hardware can easily benefit from this. Except that, AFAIK, arch/blackfin/mach-xxx is much similar with arch/arm/mach-xxx with a lot of platform, i2c, spi board information. it is probably we can also benefit from device tree as what ARM is doing. but not sure whether it is really worthwhile for the current situation and long term plan of blackfin. Anyway, there are not so many blackfin SoC and boards as ARM. > -mike > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks @ 2011-07-11 4:56 ` Barry Song 0 siblings, 0 replies; 72+ messages in thread From: Barry Song @ 2011-07-11 4:56 UTC (permalink / raw) To: Mike Frysinger Cc: Mark Brown, Grant Likely, Linus Walleij, linux-sh, patches, linux-kernel, Jeremy Kerr, linux-arm-kernel, uclinux-dist-devel 2011/7/11 Mike Frysinger <vapier@gentoo.org>: > On Sunday, July 10, 2011 23:57:40 Mark Brown wrote: >> On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: >> >> Linus, CCing you in as apparently you're taking over the clock API work. >> Do you need me to forward all the patches to you? > > along these lines, i dont think the new people on the cc noticed my earlier e- > mail: > for future series, could you cc uclinux-dist-devel@blackfin.uclinux.org ? we > dont do clock management on Blackfin parts atm, but it's something we would > like to start doing. our hardware can easily benefit from this. Except that, AFAIK, arch/blackfin/mach-xxx is much similar with arch/arm/mach-xxx with a lot of platform, i2c, spi board information. it is probably we can also benefit from device tree as what ARM is doing. but not sure whether it is really worthwhile for the current situation and long term plan of blackfin. Anyway, there are not so many blackfin SoC and boards as ARM. > -mike > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [uclinux-dist-devel] [PATCH 0/6] clk: Initial feedback for 2011-07-11 4:56 ` Barry Song (?) @ 2011-07-11 5:01 ` Mike Frysinger -1 siblings, 0 replies; 72+ messages in thread From: Mike Frysinger @ 2011-07-11 5:01 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 00:56, Barry Song wrote: > 2011/7/11 Mike Frysinger: >> On Sunday, July 10, 2011 23:57:40 Mark Brown wrote: >>> On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: >>> Linus, CCing you in as apparently you're taking over the clock API work. >>> Do you need me to forward all the patches to you? >> >> along these lines, i dont think the new people on the cc noticed my earlier e- >> mail: >> for future series, could you cc uclinux-dist-devel@blackfin.uclinux.org ? we >> dont do clock management on Blackfin parts atm, but it's something we would >> like to start doing. our hardware can easily benefit from this. > > Except that, AFAIK, arch/blackfin/mach-xxx is much similar with > arch/arm/mach-xxx with a lot of platform, i2c, spi board information. > it is probably we can also benefit from device tree as what ARM is > doing. i dont think device trees are a requirement for the clock api > but not sure whether it is really worthwhile for the current situation > and long term plan of blackfin. Anyway, there are not so many blackfin > SoC and boards as ARM. there are not plans for device tree support. no customers have asked for it, and we arent in the arm situation where we have a distro (like Ubuntu) riding us to have a single build boot on all the different platforms. -mike ^ permalink raw reply [flat|nested] 72+ messages in thread
* [uclinux-dist-devel] [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks @ 2011-07-11 5:01 ` Mike Frysinger 0 siblings, 0 replies; 72+ messages in thread From: Mike Frysinger @ 2011-07-11 5:01 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 00:56, Barry Song wrote: > 2011/7/11 Mike Frysinger: >> On Sunday, July 10, 2011 23:57:40 Mark Brown wrote: >>> On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: >>> Linus, CCing you in as apparently you're taking over the clock API work. >>> Do you need me to forward all the patches to you? >> >> along these lines, i dont think the new people on the cc noticed my earlier e- >> mail: >> for future series, could you cc uclinux-dist-devel at blackfin.uclinux.org ? ?we >> dont do clock management on Blackfin parts atm, but it's something we would >> like to start doing. ?our hardware can easily benefit from this. > > Except that, AFAIK, arch/blackfin/mach-xxx is much similar with > arch/arm/mach-xxx with a lot of platform, i2c, spi board information. > it is probably we can also benefit from device tree as what ARM is > doing. i dont think device trees are a requirement for the clock api > but not sure whether it is really worthwhile for the current situation > and long term plan of blackfin. Anyway, there are not so many blackfin > SoC and boards as ARM. there are not plans for device tree support. no customers have asked for it, and we arent in the arm situation where we have a distro (like Ubuntu) riding us to have a single build boot on all the different platforms. -mike ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [uclinux-dist-devel] [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks @ 2011-07-11 5:01 ` Mike Frysinger 0 siblings, 0 replies; 72+ messages in thread From: Mike Frysinger @ 2011-07-11 5:01 UTC (permalink / raw) To: Barry Song Cc: Grant Likely, Mark Brown, Linus Walleij, linux-sh, patches, linux-kernel, uclinux-dist-devel, Jeremy Kerr, linux-arm-kernel On Mon, Jul 11, 2011 at 00:56, Barry Song wrote: > 2011/7/11 Mike Frysinger: >> On Sunday, July 10, 2011 23:57:40 Mark Brown wrote: >>> On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: >>> Linus, CCing you in as apparently you're taking over the clock API work. >>> Do you need me to forward all the patches to you? >> >> along these lines, i dont think the new people on the cc noticed my earlier e- >> mail: >> for future series, could you cc uclinux-dist-devel@blackfin.uclinux.org ? we >> dont do clock management on Blackfin parts atm, but it's something we would >> like to start doing. our hardware can easily benefit from this. > > Except that, AFAIK, arch/blackfin/mach-xxx is much similar with > arch/arm/mach-xxx with a lot of platform, i2c, spi board information. > it is probably we can also benefit from device tree as what ARM is > doing. i dont think device trees are a requirement for the clock api > but not sure whether it is really worthwhile for the current situation > and long term plan of blackfin. Anyway, there are not so many blackfin > SoC and boards as ARM. there are not plans for device tree support. no customers have asked for it, and we arent in the arm situation where we have a distro (like Ubuntu) riding us to have a single build boot on all the different platforms. -mike ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks 2011-07-11 2:53 ` Mark Brown (?) @ 2011-07-11 9:31 ` Russell King - ARM Linux -1 siblings, 0 replies; 72+ messages in thread From: Russell King - ARM Linux @ 2011-07-11 9:31 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: > The biggest issue I ran into was that as the clocks are all registered > by name with the API if you've got two instances of the same off-SoC > device in the system you'll not be able to disambiguate between the > clocks it provides. Sigh. That sounds like yet more trash. Obviously whoever thought up that doesn't actually understand clks. ^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks @ 2011-07-11 9:31 ` Russell King - ARM Linux 0 siblings, 0 replies; 72+ messages in thread From: Russell King - ARM Linux @ 2011-07-11 9:31 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: > The biggest issue I ran into was that as the clocks are all registered > by name with the API if you've got two instances of the same off-SoC > device in the system you'll not be able to disambiguate between the > clocks it provides. Sigh. That sounds like yet more trash. Obviously whoever thought up that doesn't actually understand clks. ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks @ 2011-07-11 9:31 ` Russell King - ARM Linux 0 siblings, 0 replies; 72+ messages in thread From: Russell King - ARM Linux @ 2011-07-11 9:31 UTC (permalink / raw) To: Mark Brown Cc: Jeremy Kerr, Grant Likely, patches, linux-kernel, linux-arm-kernel, linux-sh On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: > The biggest issue I ran into was that as the clocks are all registered > by name with the API if you've got two instances of the same off-SoC > device in the system you'll not be able to disambiguate between the > clocks it provides. Sigh. That sounds like yet more trash. Obviously whoever thought up that doesn't actually understand clks. ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks 2011-07-11 9:31 ` Russell King - ARM Linux (?) @ 2011-07-11 10:07 ` Sascha Hauer -1 siblings, 0 replies; 72+ messages in thread From: Sascha Hauer @ 2011-07-11 10:07 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 10:31:24AM +0100, Russell King - ARM Linux wrote: > On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: > > The biggest issue I ran into was that as the clocks are all registered > > by name with the API if you've got two instances of the same off-SoC > > device in the system you'll not be able to disambiguate between the > > clocks it provides. > > Sigh. That sounds like yet more trash. Obviously whoever thought > up that doesn't actually understand clks. Nope. In the patches Jeremy posted clocks have a name, but this name is not meant to be used with clk_get. clk_get is still implemented in clkdev, so the matching between clocks and devices is independent of the clock name. In earlier versions of Jeremys patches the clock name was only present when debugfs was compiled in and I think it can be changed back to this. That said the debugfs support (which is not present in Jeremys latest series) would break if two clocks with the same name have the same parent, because the clock core would try to create to debugfs entries with the same name. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks @ 2011-07-11 10:07 ` Sascha Hauer 0 siblings, 0 replies; 72+ messages in thread From: Sascha Hauer @ 2011-07-11 10:07 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 10:31:24AM +0100, Russell King - ARM Linux wrote: > On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: > > The biggest issue I ran into was that as the clocks are all registered > > by name with the API if you've got two instances of the same off-SoC > > device in the system you'll not be able to disambiguate between the > > clocks it provides. > > Sigh. That sounds like yet more trash. Obviously whoever thought > up that doesn't actually understand clks. Nope. In the patches Jeremy posted clocks have a name, but this name is not meant to be used with clk_get. clk_get is still implemented in clkdev, so the matching between clocks and devices is independent of the clock name. In earlier versions of Jeremys patches the clock name was only present when debugfs was compiled in and I think it can be changed back to this. That said the debugfs support (which is not present in Jeremys latest series) would break if two clocks with the same name have the same parent, because the clock core would try to create to debugfs entries with the same name. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks @ 2011-07-11 10:07 ` Sascha Hauer 0 siblings, 0 replies; 72+ messages in thread From: Sascha Hauer @ 2011-07-11 10:07 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Mark Brown, Grant Likely, linux-sh, patches, linux-kernel, Jeremy Kerr, linux-arm-kernel On Mon, Jul 11, 2011 at 10:31:24AM +0100, Russell King - ARM Linux wrote: > On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: > > The biggest issue I ran into was that as the clocks are all registered > > by name with the API if you've got two instances of the same off-SoC > > device in the system you'll not be able to disambiguate between the > > clocks it provides. > > Sigh. That sounds like yet more trash. Obviously whoever thought > up that doesn't actually understand clks. Nope. In the patches Jeremy posted clocks have a name, but this name is not meant to be used with clk_get. clk_get is still implemented in clkdev, so the matching between clocks and devices is independent of the clock name. In earlier versions of Jeremys patches the clock name was only present when debugfs was compiled in and I think it can be changed back to this. That said the debugfs support (which is not present in Jeremys latest series) would break if two clocks with the same name have the same parent, because the clock core would try to create to debugfs entries with the same name. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks 2011-07-11 10:07 ` Sascha Hauer (?) @ 2011-07-11 10:28 ` Russell King - ARM Linux -1 siblings, 0 replies; 72+ messages in thread From: Russell King - ARM Linux @ 2011-07-11 10:28 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 12:07:59PM +0200, Sascha Hauer wrote: > On Mon, Jul 11, 2011 at 10:31:24AM +0100, Russell King - ARM Linux wrote: > > On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: > > > The biggest issue I ran into was that as the clocks are all registered > > > by name with the API if you've got two instances of the same off-SoC > > > device in the system you'll not be able to disambiguate between the > > > clocks it provides. > > > > Sigh. That sounds like yet more trash. Obviously whoever thought > > up that doesn't actually understand clks. > > Nope. In the patches Jeremy posted clocks have a name, but this name > is not meant to be used with clk_get. clk_get is still implemented > in clkdev, so the matching between clocks and devices is independent > of the clock name. > > In earlier versions of Jeremys patches the clock name was only present > when debugfs was compiled in and I think it can be changed back to this. > > That said the debugfs support (which is not present in Jeremys latest > series) would break if two clocks with the same name have the same > parent, because the clock core would try to create to debugfs entries > with the same name. If that's all its for, then can't some other solution for debugfs names (lets stop calling them clock names to avoid confusion) be found - such as sprintf(debugfsname, "%s@%p", clk->debugfsprefix, clk); Or how about using something like this: $debugfs/clkdev/device/connection -> ../../clk/<name-of-clk> $debugfs/clk/<name-of-clk> where <name-of-clk> could just be the address of the struct clk (which eliminates the need to pass any names in) or some prefix plus an incrementing identifier, or just an incrementing number. I did think about introducing such a scheme along with clkdev (and did have some code but that's long gone), but without the $debugfs/clk/ bit being standardized, it wouldn't have worked. ^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks @ 2011-07-11 10:28 ` Russell King - ARM Linux 0 siblings, 0 replies; 72+ messages in thread From: Russell King - ARM Linux @ 2011-07-11 10:28 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 12:07:59PM +0200, Sascha Hauer wrote: > On Mon, Jul 11, 2011 at 10:31:24AM +0100, Russell King - ARM Linux wrote: > > On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: > > > The biggest issue I ran into was that as the clocks are all registered > > > by name with the API if you've got two instances of the same off-SoC > > > device in the system you'll not be able to disambiguate between the > > > clocks it provides. > > > > Sigh. That sounds like yet more trash. Obviously whoever thought > > up that doesn't actually understand clks. > > Nope. In the patches Jeremy posted clocks have a name, but this name > is not meant to be used with clk_get. clk_get is still implemented > in clkdev, so the matching between clocks and devices is independent > of the clock name. > > In earlier versions of Jeremys patches the clock name was only present > when debugfs was compiled in and I think it can be changed back to this. > > That said the debugfs support (which is not present in Jeremys latest > series) would break if two clocks with the same name have the same > parent, because the clock core would try to create to debugfs entries > with the same name. If that's all its for, then can't some other solution for debugfs names (lets stop calling them clock names to avoid confusion) be found - such as sprintf(debugfsname, "%s@%p", clk->debugfsprefix, clk); Or how about using something like this: $debugfs/clkdev/device/connection -> ../../clk/<name-of-clk> $debugfs/clk/<name-of-clk> where <name-of-clk> could just be the address of the struct clk (which eliminates the need to pass any names in) or some prefix plus an incrementing identifier, or just an incrementing number. I did think about introducing such a scheme along with clkdev (and did have some code but that's long gone), but without the $debugfs/clk/ bit being standardized, it wouldn't have worked. ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks @ 2011-07-11 10:28 ` Russell King - ARM Linux 0 siblings, 0 replies; 72+ messages in thread From: Russell King - ARM Linux @ 2011-07-11 10:28 UTC (permalink / raw) To: Sascha Hauer Cc: Mark Brown, Grant Likely, linux-sh, patches, linux-kernel, Jeremy Kerr, linux-arm-kernel On Mon, Jul 11, 2011 at 12:07:59PM +0200, Sascha Hauer wrote: > On Mon, Jul 11, 2011 at 10:31:24AM +0100, Russell King - ARM Linux wrote: > > On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: > > > The biggest issue I ran into was that as the clocks are all registered > > > by name with the API if you've got two instances of the same off-SoC > > > device in the system you'll not be able to disambiguate between the > > > clocks it provides. > > > > Sigh. That sounds like yet more trash. Obviously whoever thought > > up that doesn't actually understand clks. > > Nope. In the patches Jeremy posted clocks have a name, but this name > is not meant to be used with clk_get. clk_get is still implemented > in clkdev, so the matching between clocks and devices is independent > of the clock name. > > In earlier versions of Jeremys patches the clock name was only present > when debugfs was compiled in and I think it can be changed back to this. > > That said the debugfs support (which is not present in Jeremys latest > series) would break if two clocks with the same name have the same > parent, because the clock core would try to create to debugfs entries > with the same name. If that's all its for, then can't some other solution for debugfs names (lets stop calling them clock names to avoid confusion) be found - such as sprintf(debugfsname, "%s@%p", clk->debugfsprefix, clk); Or how about using something like this: $debugfs/clkdev/device/connection -> ../../clk/<name-of-clk> $debugfs/clk/<name-of-clk> where <name-of-clk> could just be the address of the struct clk (which eliminates the need to pass any names in) or some prefix plus an incrementing identifier, or just an incrementing number. I did think about introducing such a scheme along with clkdev (and did have some code but that's long gone), but without the $debugfs/clk/ bit being standardized, it wouldn't have worked. ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks 2011-07-11 10:28 ` Russell King - ARM Linux (?) @ 2011-07-11 10:46 ` Sascha Hauer -1 siblings, 0 replies; 72+ messages in thread From: Sascha Hauer @ 2011-07-11 10:46 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 11:28:56AM +0100, Russell King - ARM Linux wrote: > On Mon, Jul 11, 2011 at 12:07:59PM +0200, Sascha Hauer wrote: > > On Mon, Jul 11, 2011 at 10:31:24AM +0100, Russell King - ARM Linux wrote: > > > On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: > > > > The biggest issue I ran into was that as the clocks are all registered > > > > by name with the API if you've got two instances of the same off-SoC > > > > device in the system you'll not be able to disambiguate between the > > > > clocks it provides. > > > > > > Sigh. That sounds like yet more trash. Obviously whoever thought > > > up that doesn't actually understand clks. > > > > Nope. In the patches Jeremy posted clocks have a name, but this name > > is not meant to be used with clk_get. clk_get is still implemented > > in clkdev, so the matching between clocks and devices is independent > > of the clock name. > > > > In earlier versions of Jeremys patches the clock name was only present > > when debugfs was compiled in and I think it can be changed back to this. > > > > That said the debugfs support (which is not present in Jeremys latest > > series) would break if two clocks with the same name have the same > > parent, because the clock core would try to create to debugfs entries > > with the same name. > > If that's all its for, then can't some other solution for debugfs names > (lets stop calling them clock names to avoid confusion) be found - > such as > > sprintf(debugfsname, "%s@%p", clk->debugfsprefix, clk); > Sounds good. During my tests on i.MX I found it very convenient to have speaking debugfs names, so I vote for this suggestion instead the following ones. > Or how about using something like this: > > $debugfs/clkdev/device/connection -> ../../clk/<name-of-clk> > $debugfs/clk/<name-of-clk> > > where <name-of-clk> could just be the address of the struct clk (which > eliminates the need to pass any names in) or some prefix plus an > incrementing identifier, or just an incrementing number. > > I did think about introducing such a scheme along with clkdev (and did > have some code but that's long gone), but without the $debugfs/clk/ bit > being standardized, it wouldn't have worked. > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks @ 2011-07-11 10:46 ` Sascha Hauer 0 siblings, 0 replies; 72+ messages in thread From: Sascha Hauer @ 2011-07-11 10:46 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 11:28:56AM +0100, Russell King - ARM Linux wrote: > On Mon, Jul 11, 2011 at 12:07:59PM +0200, Sascha Hauer wrote: > > On Mon, Jul 11, 2011 at 10:31:24AM +0100, Russell King - ARM Linux wrote: > > > On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: > > > > The biggest issue I ran into was that as the clocks are all registered > > > > by name with the API if you've got two instances of the same off-SoC > > > > device in the system you'll not be able to disambiguate between the > > > > clocks it provides. > > > > > > Sigh. That sounds like yet more trash. Obviously whoever thought > > > up that doesn't actually understand clks. > > > > Nope. In the patches Jeremy posted clocks have a name, but this name > > is not meant to be used with clk_get. clk_get is still implemented > > in clkdev, so the matching between clocks and devices is independent > > of the clock name. > > > > In earlier versions of Jeremys patches the clock name was only present > > when debugfs was compiled in and I think it can be changed back to this. > > > > That said the debugfs support (which is not present in Jeremys latest > > series) would break if two clocks with the same name have the same > > parent, because the clock core would try to create to debugfs entries > > with the same name. > > If that's all its for, then can't some other solution for debugfs names > (lets stop calling them clock names to avoid confusion) be found - > such as > > sprintf(debugfsname, "%s@%p", clk->debugfsprefix, clk); > Sounds good. During my tests on i.MX I found it very convenient to have speaking debugfs names, so I vote for this suggestion instead the following ones. > Or how about using something like this: > > $debugfs/clkdev/device/connection -> ../../clk/<name-of-clk> > $debugfs/clk/<name-of-clk> > > where <name-of-clk> could just be the address of the struct clk (which > eliminates the need to pass any names in) or some prefix plus an > incrementing identifier, or just an incrementing number. > > I did think about introducing such a scheme along with clkdev (and did > have some code but that's long gone), but without the $debugfs/clk/ bit > being standardized, it wouldn't have worked. > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks @ 2011-07-11 10:46 ` Sascha Hauer 0 siblings, 0 replies; 72+ messages in thread From: Sascha Hauer @ 2011-07-11 10:46 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Grant Likely, patches, linux-sh, Mark Brown, linux-kernel, Jeremy Kerr, linux-arm-kernel On Mon, Jul 11, 2011 at 11:28:56AM +0100, Russell King - ARM Linux wrote: > On Mon, Jul 11, 2011 at 12:07:59PM +0200, Sascha Hauer wrote: > > On Mon, Jul 11, 2011 at 10:31:24AM +0100, Russell King - ARM Linux wrote: > > > On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote: > > > > The biggest issue I ran into was that as the clocks are all registered > > > > by name with the API if you've got two instances of the same off-SoC > > > > device in the system you'll not be able to disambiguate between the > > > > clocks it provides. > > > > > > Sigh. That sounds like yet more trash. Obviously whoever thought > > > up that doesn't actually understand clks. > > > > Nope. In the patches Jeremy posted clocks have a name, but this name > > is not meant to be used with clk_get. clk_get is still implemented > > in clkdev, so the matching between clocks and devices is independent > > of the clock name. > > > > In earlier versions of Jeremys patches the clock name was only present > > when debugfs was compiled in and I think it can be changed back to this. > > > > That said the debugfs support (which is not present in Jeremys latest > > series) would break if two clocks with the same name have the same > > parent, because the clock core would try to create to debugfs entries > > with the same name. > > If that's all its for, then can't some other solution for debugfs names > (lets stop calling them clock names to avoid confusion) be found - > such as > > sprintf(debugfsname, "%s@%p", clk->debugfsprefix, clk); > Sounds good. During my tests on i.MX I found it very convenient to have speaking debugfs names, so I vote for this suggestion instead the following ones. > Or how about using something like this: > > $debugfs/clkdev/device/connection -> ../../clk/<name-of-clk> > $debugfs/clk/<name-of-clk> > > where <name-of-clk> could just be the address of the struct clk (which > eliminates the need to pass any names in) or some prefix plus an > incrementing identifier, or just an incrementing number. > > I did think about introducing such a scheme along with clkdev (and did > have some code but that's long gone), but without the $debugfs/clk/ bit > being standardized, it wouldn't have worked. > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks 2011-07-11 10:46 ` Sascha Hauer (?) @ 2011-07-11 11:43 ` Mark Brown -1 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 11:43 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 12:46:25PM +0200, Sascha Hauer wrote: > On Mon, Jul 11, 2011 at 11:28:56AM +0100, Russell King - ARM Linux wrote: > > If that's all its for, then can't some other solution for debugfs names > > (lets stop calling them clock names to avoid confusion) be found - > > such as > > sprintf(debugfsname, "%s@%p", clk->debugfsprefix, clk); > Sounds good. During my tests on i.MX I found it very convenient to have > speaking debugfs names, so I vote for this suggestion instead the > following ones. FWIW that's pretty much exactly what I added except I used a - instead of @. ^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks @ 2011-07-11 11:43 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 11:43 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 11, 2011 at 12:46:25PM +0200, Sascha Hauer wrote: > On Mon, Jul 11, 2011 at 11:28:56AM +0100, Russell King - ARM Linux wrote: > > If that's all its for, then can't some other solution for debugfs names > > (lets stop calling them clock names to avoid confusion) be found - > > such as > > sprintf(debugfsname, "%s@%p", clk->debugfsprefix, clk); > Sounds good. During my tests on i.MX I found it very convenient to have > speaking debugfs names, so I vote for this suggestion instead the > following ones. FWIW that's pretty much exactly what I added except I used a - instead of @. ^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks @ 2011-07-11 11:43 ` Mark Brown 0 siblings, 0 replies; 72+ messages in thread From: Mark Brown @ 2011-07-11 11:43 UTC (permalink / raw) To: Sascha Hauer Cc: Russell King - ARM Linux, Grant Likely, patches, linux-sh, linux-kernel, Jeremy Kerr, linux-arm-kernel On Mon, Jul 11, 2011 at 12:46:25PM +0200, Sascha Hauer wrote: > On Mon, Jul 11, 2011 at 11:28:56AM +0100, Russell King - ARM Linux wrote: > > If that's all its for, then can't some other solution for debugfs names > > (lets stop calling them clock names to avoid confusion) be found - > > such as > > sprintf(debugfsname, "%s@%p", clk->debugfsprefix, clk); > Sounds good. During my tests on i.MX I found it very convenient to have > speaking debugfs names, so I vote for this suggestion instead the > following ones. FWIW that's pretty much exactly what I added except I used a - instead of @. ^ permalink raw reply [flat|nested] 72+ messages in thread
end of thread, other threads:[~2011-07-15 5:14 UTC | newest] Thread overview: 72+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-07-11 2:53 [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks Mark Brown 2011-07-11 2:53 ` Mark Brown 2011-07-11 2:53 ` Mark Brown 2011-07-11 2:53 ` [PATCH 1/6] clk: Prototype and document clk_register() Mark Brown 2011-07-11 2:53 ` Mark Brown 2011-07-11 2:53 ` Mark Brown 2011-07-11 2:53 ` [PATCH 2/6] clk: Provide a dummy clk_unregister() Mark Brown 2011-07-11 2:53 ` Mark Brown 2011-07-11 2:53 ` Mark Brown 2011-07-11 2:53 ` [PATCH 3/6] clk: Constify struct clk_hw_ops Mark Brown 2011-07-11 2:53 ` Mark Brown 2011-07-11 2:53 ` Mark Brown 2011-07-11 2:53 ` [PATCH 4/6] clk: Add Kconfig option to build all generic clk drivers Mark Brown 2011-07-11 2:53 ` Mark Brown 2011-07-11 2:53 ` Mark Brown 2011-07-11 2:53 ` [PATCH 5/6] clk: Support multiple instances of the same clock provider Mark Brown 2011-07-11 2:53 ` Mark Brown 2011-07-11 2:53 ` Mark Brown 2011-07-11 9:34 ` [PATCH 5/6] clk: Support multiple instances of the same clock Russell King - ARM Linux 2011-07-11 9:34 ` [PATCH 5/6] clk: Support multiple instances of the same clock provider Russell King - ARM Linux 2011-07-11 9:34 ` Russell King - ARM Linux 2011-07-11 10:53 ` [PATCH 5/6] clk: Support multiple instances of the same clock Mark Brown 2011-07-11 10:53 ` [PATCH 5/6] clk: Support multiple instances of the same clock provider Mark Brown 2011-07-11 10:53 ` Mark Brown 2011-07-11 11:11 ` [PATCH 5/6] clk: Support multiple instances of the same clock Russell King - ARM Linux 2011-07-11 11:11 ` [PATCH 5/6] clk: Support multiple instances of the same clock provider Russell King - ARM Linux 2011-07-11 11:11 ` Russell King - ARM Linux 2011-07-11 11:41 ` [PATCH 5/6] clk: Support multiple instances of the same clock Mark Brown 2011-07-11 11:41 ` [PATCH 5/6] clk: Support multiple instances of the same clock provider Mark Brown 2011-07-11 11:41 ` Mark Brown 2011-07-11 2:53 ` [PATCH 6/6] clk: Add initial WM831x clock driver Mark Brown 2011-07-11 2:53 ` Mark Brown 2011-07-11 2:53 ` Mark Brown 2011-07-15 2:53 ` Grant Likely 2011-07-15 2:53 ` Grant Likely 2011-07-15 2:53 ` Grant Likely 2011-07-15 5:05 ` Mark Brown 2011-07-15 5:05 ` Mark Brown 2011-07-15 5:05 ` Mark Brown 2011-07-15 5:14 ` Ryan Mallon 2011-07-15 5:14 ` Ryan Mallon 2011-07-15 5:14 ` Ryan Mallon 2011-07-15 2:53 ` [PATCH 1/6] clk: Prototype and document clk_register() Grant Likely 2011-07-15 2:53 ` Grant Likely 2011-07-15 2:53 ` Grant Likely 2011-07-11 3:57 ` [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks Mark Brown 2011-07-11 3:57 ` Mark Brown 2011-07-11 3:57 ` Mark Brown 2011-07-11 4:30 ` Mike Frysinger 2011-07-11 4:30 ` Mike Frysinger 2011-07-11 4:30 ` Mike Frysinger 2011-07-11 4:56 ` Barry Song 2011-07-11 4:56 ` Barry Song 2011-07-11 4:56 ` Barry Song 2011-07-11 5:01 ` [uclinux-dist-devel] [PATCH 0/6] clk: Initial feedback for Mike Frysinger 2011-07-11 5:01 ` [uclinux-dist-devel] [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks Mike Frysinger 2011-07-11 5:01 ` Mike Frysinger 2011-07-11 9:31 ` Russell King - ARM Linux 2011-07-11 9:31 ` Russell King - ARM Linux 2011-07-11 9:31 ` Russell King - ARM Linux 2011-07-11 10:07 ` Sascha Hauer 2011-07-11 10:07 ` Sascha Hauer 2011-07-11 10:07 ` Sascha Hauer 2011-07-11 10:28 ` Russell King - ARM Linux 2011-07-11 10:28 ` Russell King - ARM Linux 2011-07-11 10:28 ` Russell King - ARM Linux 2011-07-11 10:46 ` Sascha Hauer 2011-07-11 10:46 ` Sascha Hauer 2011-07-11 10:46 ` Sascha Hauer 2011-07-11 11:43 ` Mark Brown 2011-07-11 11:43 ` Mark Brown 2011-07-11 11:43 ` Mark Brown
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.