Messages in this thread | | | From | David Brownell <> | Subject | Re: [PATCH 0/5] Clocklib: generic clocks framework | Date | Fri, 25 Apr 2008 15:46:51 -0700 |
| |
On Monday 21 April 2008, Dmitry wrote: > > - I don't think that I understand the clk_functions part of your code. > > Is this a shorthand to construct aliases to other struct clks? > > Yes, that's one of usages for it. E.g. current AT91 code has same > functionality named at91_clock_associate.
As in:
at91_clock_associate("usart0_clk", &pdev0->dev, "usart"); at91_clock_associate("usart1_clk", &pdev1->dev, "usart"); at91_clock_associate("usart2_clk", &pdev2->dev, "usart");
That essentially maps a device and logical clock name to a specific clock (those "usartX_clk" identifiers are global), so that drivers can clk_get(dev, "usart") and get the right clock. It decouples clock and device declarations, which should help when integrating external clocks too.
The assumption that clk_get(NULL, "usartX_clk") works is mostly just a convenience, although it's probably a fair assumption for many other SOC platforms. Conceptually that first parameter is a clock, not a name.
> > +struct clk_function { > > + const char *parent; > > + void *priv; > > + struct clk clk; > > +};
That doesn't really seem attuned to the task of associating logical clock names to devices. Wouldn't it be better to have something directly addressing that core mechanism?
> Also once we get to multiple chips providers/users, we'll see, > that the clock simply can't have just one record in the clocks tree.
I don't follow. Why not? If a clock has multiple records, I'd expect its refcounts would easily get borked. I think I don't like your notion of a "wrapper clock".
> It's provided by some > pin (provider_name) and then consumed by several devices (several > consumer_name + consumer_device pairs). That is it.
There would still be just one clock though. It shouldn't matter how clk_get(this_dev, "c1") and clk_get(that_dev, "c2") find it. Hashtable, linked list, divination, ... so long as it works.
Those other records should just hold {dev, name} ==> clock mappings; they wouldn't be records for the clock itself. But those "functions" do not seem to record just mappings.
- Dave
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |