Re: [PATCHv9 05/43] CLK: TI: add DT alias clock registration mechanism
On Sat, Nov 2, 2013 at 8:49 AM, Tomasz Figa tomasz.f...@gmail.com wrote: Hi Matt, I have one question here, what makes this part of the patch TI-specific at all except the definition of the structure ti_dt_clk? Mapping DT clocks to generic clkdev legacy namings seems like it would be a necessary evil across *all* SoC platforms. SoC platforms that are fully converted to Device Tree do not need any legacy clkdev namings, because they have all clkdev look-ups performed using data from DT (see generic clock bindings). So this would be not *all* SoC platforms, but instead SoC platforms, which are currently undergoing conversion to DT or conversion of which is yet to be started. That is well understood here, I have a long-running hobby project to do this for i.MX5 and i.MX6 - I have a tree from months ago where I restructured the way clocks are probed for both these (moving from mach-imx to drivers/clk and putting in the right places) and successfully moved 8 clocks (on i.MX51, 3 core PLLs, some fixed clocks which got done in the meantime upstream, and one in the CCM - the CPU freq divider) with the correct references and it all operated really well. In the meantime I had to add a bunch of weird hacks to the drivers to deal with the missing connection names, so this being generic would allow the core clock work to be finished while the drivers are brought up to the new convention. Still this might be a good enough justification for having a generic solution for this problem, but I would wait with this until the real need on some of such platforms shows up. I don't see anything wrong in implementing this first as TI-specific quirk and then possibly making it generic if such need shows up. Indeed.. and I would like to make use of it but I'm not coding on a TI platform or a Samsung platform:) By the way, we already have something like this in clk/samsung/clk.c . This is called aliases there and allows registering clkdev look-ups for particular clocks. However this is specific to our clock drivers that register all clocks inside a single clock privder, based on static ID assignment and all clocks statically listed in clock driver - see clk/samsung/clk-s3c64xx.c for an example. Right. I'll keep it in mind as I do this. Thanks, this does make my life a lot easier, means I might have it working and submitted before the end of the month instead of sometime next year, I was dreading reworking all these drivers up front ;) Matt Sealey n...@bakuhatsu.net -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv9 05/43] CLK: TI: add DT alias clock registration mechanism
Hi Matt,
On Tuesday 29 of October 2013 12:50:43 Matt Sealey wrote:
On Fri, Oct 25, 2013 at 10:56 AM, Tero Kristo t-kri...@ti.com wrote:
Some devices require their clocks to be available with a specific
dev-id con-id mapping. With DT, the clocks can be found by default
only with their name, or alternatively through the device node of
the consumer. With drivers, that don't support DT fully yet, add
mechanism to register specific clock names.
Signed-off-by: Tero Kristo t-kri...@ti.com
Tested-by: Nishanth Menon n...@ti.com
Acked-by: Tony Lindgren t...@atomide.com
You guys are wonderful, by the way ;)
+void __init ti_dt_clocks_register(struct ti_dt_clk oclks[])
+{
+ struct ti_dt_clk *c;
+ struct device_node *node;
+ struct clk *clk;
+ struct of_phandle_args clkspec;
+
+ for (c = oclks; c-node_name != NULL; c++) {
+ node = of_find_node_by_name(NULL, c-node_name);
+ clkspec.np = node;
+ clk = of_clk_get_from_provider(clkspec);
+
+ if (!IS_ERR(clk)) {
+ c-lk.clk = clk;
+ clkdev_add(c-lk);
+ } else {
+ pr_warn(%s: failed to lookup clock node
%s\n,
+ __func__, c-node_name);
+ }
+ }
+}
I have one question here, what makes this part of the patch
TI-specific at all except the definition of the structure ti_dt_clk?
Mapping DT clocks to generic clkdev legacy namings seems like it would
be a necessary evil across *all* SoC platforms.
SoC platforms that are fully converted to Device Tree do not need any
legacy clkdev namings, because they have all clkdev look-ups performed
using data from DT (see generic clock bindings).
So this would be not *all* SoC platforms, but instead SoC platforms,
which are currently undergoing conversion to DT or conversion of which is
yet to be started.
Still this might be a good enough justification for having a generic
solution for this problem, but I would wait with this until the real need
on some of such platforms shows up. I don't see anything wrong in
implementing this first as TI-specific quirk and then possibly making it
generic if such need shows up.
By the way, we already have something like this in clk/samsung/clk.c .
This is called aliases there and allows registering clkdev look-ups for
particular clocks. However this is specific to our clock drivers that
register all clocks inside a single clock privder, based on static ID
assignment and all clocks statically listed in clock driver - see
clk/samsung/clk-s3c64xx.c for an example.
Best regards,
Tomasz
I would say there is a good case for this being generic and used by
all platforms waiting for con-id/dev-id to be moved to DT-awareness,
the structure itself is very generic within clkdev as long as the
drivers conformed anyway (and if they didn't, none of this helps) and
I don't think it really needs to be a TI-only thing. The next thing
that's going to happen when another platform arrives that wants to do
the same thing is duplication or consolidation - so it would probably
be a good idea to make this generic to start.
While there the names could be a bit more descriptive -
dt_clk_name_lookup_map_register(), struct dt_clk_name_lookup_map,
DT_CLK_LOOKUP_MAP() ..? The current namespace makes it look like this
is the absolute correct thing to do under all circumstances, but in
fact this is purely for mapping the old way to the new, better way..
in the event a brand new platform arrives, with all new drivers (or
only compliant drivers), none of this code should be implemented for
it and it should be presented this way.
Thanks,
Matt Sealey n...@bakuhatsu.net
___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv9 05/43] CLK: TI: add DT alias clock registration mechanism
On 10/30/2013 07:38 PM, Matt Sealey wrote: On Wed, Oct 30, 2013 at 3:29 AM, Tero Kristo t-kri...@ti.com wrote: On 10/29/2013 07:50 PM, Matt Sealey wrote: I have one question here, what makes this part of the patch TI-specific at all except the definition of the structure ti_dt_clk? Mapping DT clocks to generic clkdev legacy namings seems like it would be a necessary evil across *all* SoC platforms. I would say there is a good case for this being generic An earlier implementation of this patch (or at least a patch that tries to accomplish the same as this does, add support for legacy devices) did try to add generic solution. There was some discussion on it and I decided to drop it due to bad feedback. See: http://www.spinics.net/lists/linux-omap/msg95147.html I don't see anything that says this should not be generic, just that it was implemented relatively poorly at the time... I agree with Tomasz, find_by_name in device trees is basically evil (it's only supposed to be used if you already know the name because something else passed it to you, and just want it's phandle, but names CAN be ambiguous. The real fix is always pass phandles and not strings of the name property), but that's not to say this thing needs to be TI-specific.. Ah well. If it ends up getting used elsewhere, it can be quickly patched into a generic version anyway, so.. yay! Yeah, this can be converted to generic version easily if need be. The thing with this patch is also that it is kind of temporary solution, it should not be needed anymore once all the drivers are converted to DT. I'm really interested in this all going in - but I would love to see where this is just all patched into a tree already. Pulling things out against a bunch of trees gave me more merge failures and weird happenings than I care to deal with. Is there a TI clock development tree with all this in somewhere publically accessible? I'd like to get a feel for how the DT looks and what's going on, I can't see it from the patches. Once it's there.. I'll spend a week or 5 doing all of the i.MX chips and you'll have a second test case for any problems :) Did you try the fully integrated test tree I mentioned in the cover letter? tree: https://github.com/t-kristo/linux-pm.git branch: 3.12-rc6-dt-clks-v9 This has everything in it, and compiles cleanly. -Tero -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv9 05/43] CLK: TI: add DT alias clock registration mechanism
On 10/29/2013 07:50 PM, Matt Sealey wrote:
On Fri, Oct 25, 2013 at 10:56 AM, Tero Kristo t-kri...@ti.com wrote:
Some devices require their clocks to be available with a specific
dev-id con-id mapping. With DT, the clocks can be found by default
only with their name, or alternatively through the device node of
the consumer. With drivers, that don't support DT fully yet, add
mechanism to register specific clock names.
Signed-off-by: Tero Kristo t-kri...@ti.com
Tested-by: Nishanth Menon n...@ti.com
Acked-by: Tony Lindgren t...@atomide.com
You guys are wonderful, by the way ;)
+void __init ti_dt_clocks_register(struct ti_dt_clk oclks[])
+{
+ struct ti_dt_clk *c;
+ struct device_node *node;
+ struct clk *clk;
+ struct of_phandle_args clkspec;
+
+ for (c = oclks; c-node_name != NULL; c++) {
+ node = of_find_node_by_name(NULL, c-node_name);
+ clkspec.np = node;
+ clk = of_clk_get_from_provider(clkspec);
+
+ if (!IS_ERR(clk)) {
+ c-lk.clk = clk;
+ clkdev_add(c-lk);
+ } else {
+ pr_warn(%s: failed to lookup clock node %s\n,
+ __func__, c-node_name);
+ }
+ }
+}
I have one question here, what makes this part of the patch
TI-specific at all except the definition of the structure ti_dt_clk?
Mapping DT clocks to generic clkdev legacy namings seems like it would
be a necessary evil across *all* SoC platforms.
I would say there is a good case for this being generic and used by
all platforms waiting for con-id/dev-id to be moved to DT-awareness,
the structure itself is very generic within clkdev as long as the
drivers conformed anyway (and if they didn't, none of this helps) and
I don't think it really needs to be a TI-only thing. The next thing
that's going to happen when another platform arrives that wants to do
the same thing is duplication or consolidation - so it would probably
be a good idea to make this generic to start.
While there the names could be a bit more descriptive -
dt_clk_name_lookup_map_register(), struct dt_clk_name_lookup_map,
DT_CLK_LOOKUP_MAP() ..? The current namespace makes it look like this
is the absolute correct thing to do under all circumstances, but in
fact this is purely for mapping the old way to the new, better way..
in the event a brand new platform arrives, with all new drivers (or
only compliant drivers), none of this code should be implemented for
it and it should be presented this way.
An earlier implementation of this patch (or at least a patch that tries
to accomplish the same as this does, add support for legacy devices) did
try to add generic solution. There was some discussion on it and I
decided to drop it due to bad feedback.
See: http://www.spinics.net/lists/linux-omap/msg95147.html
-Tero
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv9 05/43] CLK: TI: add DT alias clock registration mechanism
On Wed, Oct 30, 2013 at 3:29 AM, Tero Kristo t-kri...@ti.com wrote: On 10/29/2013 07:50 PM, Matt Sealey wrote: I have one question here, what makes this part of the patch TI-specific at all except the definition of the structure ti_dt_clk? Mapping DT clocks to generic clkdev legacy namings seems like it would be a necessary evil across *all* SoC platforms. I would say there is a good case for this being generic An earlier implementation of this patch (or at least a patch that tries to accomplish the same as this does, add support for legacy devices) did try to add generic solution. There was some discussion on it and I decided to drop it due to bad feedback. See: http://www.spinics.net/lists/linux-omap/msg95147.html I don't see anything that says this should not be generic, just that it was implemented relatively poorly at the time... I agree with Tomasz, find_by_name in device trees is basically evil (it's only supposed to be used if you already know the name because something else passed it to you, and just want it's phandle, but names CAN be ambiguous. The real fix is always pass phandles and not strings of the name property), but that's not to say this thing needs to be TI-specific.. Ah well. If it ends up getting used elsewhere, it can be quickly patched into a generic version anyway, so.. yay! I'm really interested in this all going in - but I would love to see where this is just all patched into a tree already. Pulling things out against a bunch of trees gave me more merge failures and weird happenings than I care to deal with. Is there a TI clock development tree with all this in somewhere publically accessible? I'd like to get a feel for how the DT looks and what's going on, I can't see it from the patches. Once it's there.. I'll spend a week or 5 doing all of the i.MX chips and you'll have a second test case for any problems :) Ta, Matt Sealey n...@bakuhatsu.net -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv9 05/43] CLK: TI: add DT alias clock registration mechanism
On Fri, Oct 25, 2013 at 10:56 AM, Tero Kristo t-kri...@ti.com wrote:
Some devices require their clocks to be available with a specific
dev-id con-id mapping. With DT, the clocks can be found by default
only with their name, or alternatively through the device node of
the consumer. With drivers, that don't support DT fully yet, add
mechanism to register specific clock names.
Signed-off-by: Tero Kristo t-kri...@ti.com
Tested-by: Nishanth Menon n...@ti.com
Acked-by: Tony Lindgren t...@atomide.com
You guys are wonderful, by the way ;)
+void __init ti_dt_clocks_register(struct ti_dt_clk oclks[])
+{
+ struct ti_dt_clk *c;
+ struct device_node *node;
+ struct clk *clk;
+ struct of_phandle_args clkspec;
+
+ for (c = oclks; c-node_name != NULL; c++) {
+ node = of_find_node_by_name(NULL, c-node_name);
+ clkspec.np = node;
+ clk = of_clk_get_from_provider(clkspec);
+
+ if (!IS_ERR(clk)) {
+ c-lk.clk = clk;
+ clkdev_add(c-lk);
+ } else {
+ pr_warn(%s: failed to lookup clock node %s\n,
+ __func__, c-node_name);
+ }
+ }
+}
I have one question here, what makes this part of the patch
TI-specific at all except the definition of the structure ti_dt_clk?
Mapping DT clocks to generic clkdev legacy namings seems like it would
be a necessary evil across *all* SoC platforms.
I would say there is a good case for this being generic and used by
all platforms waiting for con-id/dev-id to be moved to DT-awareness,
the structure itself is very generic within clkdev as long as the
drivers conformed anyway (and if they didn't, none of this helps) and
I don't think it really needs to be a TI-only thing. The next thing
that's going to happen when another platform arrives that wants to do
the same thing is duplication or consolidation - so it would probably
be a good idea to make this generic to start.
While there the names could be a bit more descriptive -
dt_clk_name_lookup_map_register(), struct dt_clk_name_lookup_map,
DT_CLK_LOOKUP_MAP() ..? The current namespace makes it look like this
is the absolute correct thing to do under all circumstances, but in
fact this is purely for mapping the old way to the new, better way..
in the event a brand new platform arrives, with all new drivers (or
only compliant drivers), none of this code should be implemented for
it and it should be presented this way.
Thanks,
Matt Sealey n...@bakuhatsu.net
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv9 05/43] CLK: TI: add DT alias clock registration mechanism
Some devices require their clocks to be available with a specific
dev-id con-id mapping. With DT, the clocks can be found by default
only with their name, or alternatively through the device node of
the consumer. With drivers, that don't support DT fully yet, add
mechanism to register specific clock names.
Signed-off-by: Tero Kristo t-kri...@ti.com
Tested-by: Nishanth Menon n...@ti.com
Acked-by: Tony Lindgren t...@atomide.com
---
drivers/clk/ti/Makefile |2 +-
drivers/clk/ti/clk.c| 52 +++
include/linux/clk/ti.h | 23 +
3 files changed, 76 insertions(+), 1 deletion(-)
create mode 100644 drivers/clk/ti/clk.c
diff --git a/drivers/clk/ti/Makefile b/drivers/clk/ti/Makefile
index 93177987..05af5d8 100644
--- a/drivers/clk/ti/Makefile
+++ b/drivers/clk/ti/Makefile
@@ -1,3 +1,3 @@
ifneq ($(CONFIG_OF),)
-obj-y += dpll.o
+obj-y += clk.o dpll.o
endif
diff --git a/drivers/clk/ti/clk.c b/drivers/clk/ti/clk.c
new file mode 100644
index 000..ad58b01
--- /dev/null
+++ b/drivers/clk/ti/clk.c
@@ -0,0 +1,52 @@
+/*
+ * TI clock support
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ *
+ * Tero Kristo t-kri...@ti.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed as is WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/clk-provider.h
+#include linux/clkdev.h
+#include linux/clk/ti.h
+#include linux/of.h
+
+/**
+ * ti_dt_clocks_register - register DT duplicate clocks during boot
+ * @oclks: list of clocks to register
+ *
+ * Register duplicate or non-standard DT clock entries during boot. By
+ * default, DT clocks are found based on their node name. If any
+ * additional con-id / dev-id - clock mapping is required, use this
+ * function to list these.
+ */
+void __init ti_dt_clocks_register(struct ti_dt_clk oclks[])
+{
+ struct ti_dt_clk *c;
+ struct device_node *node;
+ struct clk *clk;
+ struct of_phandle_args clkspec;
+
+ for (c = oclks; c-node_name != NULL; c++) {
+ node = of_find_node_by_name(NULL, c-node_name);
+ clkspec.np = node;
+ clk = of_clk_get_from_provider(clkspec);
+
+ if (!IS_ERR(clk)) {
+ c-lk.clk = clk;
+ clkdev_add(c-lk);
+ } else {
+ pr_warn(%s: failed to lookup clock node %s\n,
+ __func__, c-node_name);
+ }
+ }
+}
diff --git a/include/linux/clk/ti.h b/include/linux/clk/ti.h
index c187023..9786752 100644
--- a/include/linux/clk/ti.h
+++ b/include/linux/clk/ti.h
@@ -15,6 +15,8 @@
#ifndef __LINUX_CLK_TI_H__
#define __LINUX_CLK_TI_H__
+#include linux/clkdev.h
+
/**
* struct dpll_data - DPLL registers and integration data
* @mult_div1_reg: register containing the DPLL M and N bitfields
@@ -135,6 +137,25 @@ struct clk_hw_omap {
/* DPLL Type and DCO Selection Flags */
#define DPLL_J_TYPE0x1
+/**
+ * struct ti_dt_clk - OMAP DT clock alias declarations
+ * @lk: clock lookup definition
+ * @node_name: clock DT node to map to
+ */
+struct ti_dt_clk {
+ struct clk_lookup lk;
+ char*node_name;
+};
+
+#define DT_CLK(dev, con, name) \
+ { \
+ .lk = { \
+ .dev_id = dev, \
+ .con_id = con, \
+ }, \
+ .node_name = name, \
+ }
+
void omap2_init_clk_hw_omap_clocks(struct clk *clk);
int omap3_noncore_dpll_enable(struct clk_hw *hw);
void omap3_noncore_dpll_disable(struct clk_hw *hw);
@@ -155,6 +176,8 @@ unsigned long omap3_clkoutx2_recalc(struct clk_hw *hw,
int omap3_dpll4_set_rate(struct clk_hw *clk, unsigned long rate,
unsigned long parent_rate);
+void ti_dt_clocks_register(struct ti_dt_clk *oclks);
+
extern const struct clk_hw_omap_ops clkhwops_omap3_dpll;
extern const struct clk_hw_omap_ops clkhwops_omap4_dpllmx;
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
