Re: [PATCHv9 05/43] CLK: TI: add DT alias clock registration mechanism

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/29/2013 07:50 PM, Matt Sealey wrote:
On Fri, Oct 25, 2013 at 10:56 AM, Tero Kristo <t-kristo@xxxxxx> 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-kristo@xxxxxx>
Tested-by: Nishanth Menon <nm@xxxxxx>
Acked-by: Tony Lindgren <tony@xxxxxxxxxxx>

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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux