On 03/03/2013 01:08 PM, Heiko Stübner wrote:
Am Sonntag, 3. März 2013, 12:45:01 schrieb Tomasz Figa:
On Sunday 03 of March 2013 12:17:29 Sylwester Nawrocki wrote:
On 03/03/2013 02:08 AM, Heiko Stübner wrote:
But is there an easy way to define more than one alias? On the s3c2416
for example the hsmmc hclk is the "hsmmc" io-clock, as well as the
source for the "mmc_busclk.0". Same for the "uart" pclk, that is also
a baud clock source.
This driver currently provides for only one additional clkdev lookup
entry per a platform clock. I pointed out this desing issue in the
early version of the patch set. It's because a machine clock definition
is coupled with a clock consumer definition. And IMO various
samsung_clock_register_* functions should not have
clk_register_clkdev() inside them. I.e. first step could be registering
all machine clocks and in the second one clkdev lookup entries could be
created. This is how most (all?) existing SoC clock drivers are
working.
But those multiple aliases are important only for machines with device
tree support, aren't they ?
I suppose you meant _without_ device tree support, right?
Yes, my mistake, sorry for the confusion.
The aliases are only needed for the non-dt case. But as I think common clk
support will be a requirement for dt support in the future, similar to
pinctrl, without the correct handling of the aliases it will be hard to
incrementally convert the other platforms (i.e. s3c24xx before s3c2443, etc).
Yes, indeed. That's a very valid point to have the handling of the aliases
implemented correctly, not assuming it will be needed temporarily only.
For the time being I've added my own register_alias function to Thomas' core
code, hijacking the clk_table for this - attached for reference below.
The patch looks good to me. It would make sense to handle all clkdev
entries like this.
I hope this patch series gets merged early to linux-next in the 3.10
cycle so the multiple accumulated fixup patches for this clock driver
can be merged as well and issues like that you pointed out can be
resolved with incremental patches.
Yes, I hope so too.
me too. Following all this still out-of-tree stuff makes me dizzy :-)
Yeah, especially that it is not always clear what tag the patch series
are based of off. For a long patch series like these, touching the core
subsystems, it would be nice to have a corresponding git tree so it is
possible to actually use and test the patches without much trouble.
Heiko
diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
index d36cdd5..7f1b5bc 100644
--- a/drivers/clk/samsung/clk.c
+++ b/drivers/clk/samsung/clk.c
@@ -57,14 +57,15 @@ void __init samsung_clk_init(struct device_node *np, void __iomem *base,
unsigned long nr_rdump)
{
reg_base = base;
- if (!np)
- return;
-#ifdef CONFIG_OF
clk_table = kzalloc(sizeof(struct clk *) * nr_clks, GFP_KERNEL);
if (!clk_table)
panic("could not allocate clock lookup table\n");
+ if (!np)
+ return;
+
+#ifdef CONFIG_OF
clk_data.clks = clk_table;
clk_data.clk_num = nr_clks;
of_clk_add_provider(np, of_clk_src_onecell_get,&clk_data);
@@ -96,6 +97,46 @@ void samsung_clk_add_lookup(struct clk *clk, unsigned int id)
clk_table[id] = clk;
}
+/* register a list of aliases */
+void __init samsung_clk_register_alias(struct samsung_clock_alias *list,
+ unsigned int nr_clk)
+{
+ struct clk *clk;
+ unsigned int idx, ret;
+
+ if (!clk_table) {
+ pr_err("%s: clock table missing\n", __func__);
+ return;
+ }
+
+ for (idx = 0; idx< nr_clk; idx++, list++) {
+ if (!list->id) {
+ pr_err("%s: clock id missing for index %d\n", __func__,
+ idx);
+ continue;
+ }
+
+ clk = clk_table[list->id];
+ if (!clk) {
+ pr_err("%s: failed to find clock %d\n", __func__,
+ list->id);
+ continue;
+ }
+
+ /* register a clock lookup only if a clock alias is specified */
+ if (!list->alias) {
+ pr_err("%s: no alias defined for clock %d\n", __func__,
+ list->id);
I wouldn't print that error in general. It might be a clock with NULL
conn_id.
It's not an error condition.
+ continue;
+ }
+
+ ret = clk_register_clkdev(clk, list->alias, list->dev_name);
+ if (ret)
+ pr_err("%s: failed to register lookup %s\n",
+ __func__, list->alias);
+ }
+}
+
/* register a list of fixed clocks */
void __init samsung_clk_register_fixed_rate(
struct samsung_fixed_rate_clock *list, unsigned int nr_clk)
diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
index 175a9d0..8be9248 100644
--- a/drivers/clk/samsung/clk.h
+++ b/drivers/clk/samsung/clk.h
@@ -23,6 +23,25 @@
#include<mach/map.h>
/**
+ * struct samsung_clock_alias: information about mux clock
+ * @id: platform specific id of the clock.
+ * @dev_name: name of the device to which this clock belongs.
+ * @alias: optional clock alias name to be assigned to this clock.
+ */
+struct samsung_clock_alias {
+ unsigned int id;
+ const char *dev_name;
+ const char *alias;
+};
+
+#define ALIAS(_id, dname, a) \
+ { \
+ .id = _id, \
+ .dev_name = dname, \
+ .alias = a, \
+ }
+
+/**
* struct samsung_fixed_rate_clock: information about fixed-rate clock
* @id: platform specific id of the clock.
* @name: name of this fixed-rate clock.
@@ -260,6 +282,8 @@ extern void __init samsung_clk_of_register_fixed_ext(
extern void samsung_clk_add_lookup(struct clk *clk, unsigned int id);
+extern void samsung_clk_register_alias(struct samsung_clock_alias *list,
+ unsigned int nr_clk);
extern void __init samsung_clk_register_fixed_rate(
struct samsung_fixed_rate_clock *clk_list, unsigned int nr_clk);
extern void __init samsung_clk_register_fixed_factor(
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
