On Tue 05 May 2020 at 10:17, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote: > [...] > >> >> > + >> >> > + return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, >> >> > + onecell_data); >> >> >> >> I think registering a provider for a module that does not provide clocks >> >> to any other device is a bit overkill. >> >> >> >> I understand the matter is getting the per-user clk* pointer. >> >> Since this is the module registering the clock, you can use clk_hw->clk >> >> to get it. >> >> >> >> Once you have the clk* of the leaf clocks, you don't even need to keep >> >> track of the clk_hw* since you are using devm_ >> >> >> >> Afterward, we should propably discuss with Stephen if something should >> >> be added in CCF to get a struct clk* from struct clk_hw*. >> >> >> > >> > [...] >> > >> > Hmm. >> > >> > I am not sure the above is a good idea, at all. Unless, I am >> > misunderstanding your point, which may be the case. >> > >> > I think above "shortcuts" could lead to abuse of the clock framework >> > and its internal data structures. When going forward, this could make >> > it unnecessary harder to maintain the clock framework. >> > >> > I know, it's not my responsibility, but from my experience with MMC >> > and SDIO interfaces, is that those have been too easy abuse - since >> > most of the data structures and interfaces have been exported. Now, >> > it's hard to roll back that, if you see what I mean. >> >> Indeed, it worth clarifying this first. >> >> With clk_register deprecated in favor of clk_hw_register, we are likely >> to see that case rise elsewhere. >> > > So, according to the separate discussion [1], I think we can let > Martin decide what option to implement at this point. > > 1. Implement the "clk_hw_get_clk()" approach. The preferred option, > but requires wider changes of the clock subsystem as well. > > 2. Keep the existing approach, with devm_clk_get(). I am fine with > this as well, we can always switch to 1) later on. I have a problem with this approach. The dt-bindings would include "#clock-cells = <1>" for a device that does not actually provide and only needs it has a temporary work around. Those bindings are supposed to be stable ... I have proposed 2 other short term solutions, let's see how it goes > > [...] > > Kind regards > Uffe > > [1] > https://www.spinics.net/lists/linux-clk/msg48373.html
