Hi Tony, On Wed, Apr 10, 2013 at 1:49 PM, Tony Lindgren <tony@xxxxxxxxxxx> wrote: > * Nishanth Menon <nm@xxxxxx> [130410 10:44]: >> Details in the patch below (Tony, I have added you as collaborator for >> helping in getting this working-clk_add_alias was'nt needed in the >> internal patch discussion we had - I have taken a bit of freedom in >> adding your contributions to the patch below) > > OK thanks. Noticed few minor things, see below. Thanks on the checkup > >> Folks, this does seem to be the best compromise we can achieve at this >> point in time. feedback on this approach is much appreciated - if folks >> are ok, I can post this as an formal patch series. >> >> From 130a41821bf57081ca45ef654029175d173135e6 Mon Sep 17 00:00:00 2001 >> From: Nishanth Menon <nm@xxxxxx> >> Date: Tue, 9 Apr 2013 19:26:40 -0500 >> Subject: [RFC PATCH] clk: OMAP: introduce device tree binding to kernel clock >> data >> >> OMAP clock data is located in arch/arm/mach-omap2/cclockXYZ_data.c. >> However, this presents an obstacle for using these clock nodes in >> Device Tree definitions. There are many possible approaches to this >> problem as discussed in the following thread: >> http://marc.info/?t=136370325600009&r=1&w=2 > > It might be worth clarifying that this is especially for the board > specific clocks initially. The fixed clocks are currently found via > the clock aliases table. Ack. [...] > >> +Example #2: describing clock node for auxilary clock #3 on OMAP443x platform: >> +Ref: arch/arm/mach-omap2/cclock44xx_data.c >> +describes the auxclk3 clock to be as follows: >> + CLK(NULL, "auxclk3_ck", &auxclk3_ck, CK_443X), >> +Corresponding binding will be: >> + auxclk3: auxclk3 { >> + #clock-cells = <1>; >> + compatible = "ti,omap-clock"; >> + }; >> +And it's usage will be: >> + clocks = <&auxclk3>; > > The #clock-cells in the auxclk3 example should also be 0 instead of 1 > AFAIK. We should only use #clock-cells = <1> when the same physical > clock provides multiple outputs. I believe the auxclocks are all separate, > but that needs to be verified. Oops.. my bad. you are correct here - auxclk is single output. all of them. will fix. [...] >> +static int omap_clk_probe(struct platform_device *pdev) >> +{ >> + struct clk *clk; >> + int res; >> + >> + const struct of_device_id *match; >> + struct device_node *np = pdev->dev.of_node; >> + char clk_name[32]; >> + >> + match = of_match_device(omap_clk_of_match, &pdev->dev); >> + >> + /* Set up things so consumer can call clk_get() with name */ >> + snprintf(clk_name, 32, "%s_ck", np->name); >> + clk = clk_get(NULL, clk_name); >> + if (IS_ERR(clk)) { >> + res = PTR_ERR(clk); >> + dev_err(&pdev->dev, "could not get clock %s (%d)\n", >> + clk_name, res); >> + goto out; >> + } > > It seems that at least for now we can assume the naming will stay > that way, then if we need other rules for finding clocks, we can > add omap_match_clock() function or something. ack. > >> + /* This allows the driver to of_clk_get() */ >> + res = of_clk_add_provider(np, of_clk_src_simple_get, clk); >> + if (res) >> + dev_err(&pdev->dev, "could not add provider for %s (%d)\n", >> + clk_name, res); >> + >> + clk_put(clk); >> +out: >> + return res; >> +} > > We can avoid the concern of storing the struct clk * and do the > look up lazily on consumer driver probe by setting a dummy struct > clk * here. Then replace of_clk_src_simple_get() with a custom > omap_clk_src_get() that does the lookup and replaces the struct > clk * with the real one. Hmm.. this is interesting. will give it a try. Thanks on the suggestion. Regards, Nishanth Menon -- 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
