Hello Mike, On Wed, Jul 2, 2014 at 6:33 AM, Mike Turquette <mturquette@xxxxxxxxxx> wrote: > Quoting Peter Ujfalusi (2014-06-29 22:56:55) >> Hi Javier, >> >> On 06/27/2014 09:23 PM, Javier Martinez Canillas wrote: >> > Hello Peter, >> > >> > On Fri, Jun 27, 2014 at 8:01 AM, Peter Ujfalusi <peter.ujfalusi@xxxxxx> wrote: >> >> Palmas class of devices can provide 32K clock(s) to be used by other devices >> >> on the board. Depending on the actual device the provided clocks can be: >> >> CLK32K_KG and CLK32K_KGAUDIO >> >> or only one: >> >> CLK32K_KG (TPS659039 for example) >> >> >> >> Use separate compatible flags for the two 32K clock. >> >> A system which needs or have only one of the 32k clock from >> >> Palmas will need to add node(s) for each clock as separate section >> >> in the dts file. >> >> The two compatible property is: >> >> "ti,palmas-clk32kg" for clk32kg clock >> >> "ti,palmas-clk32kgaudio" for clk32kgaudio clock >> >> >> >> Apart from the register control of the clocks - which is done via >> >> the clock API there is a posibility to enable the external sleep >> >> control. In this way the clock can be enabled/disabled on demand by the >> >> user of the clock. >> >> >> >> See the documentation for more details. >> >> >> >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@xxxxxx> >> >> Reviewed-by: Nishanth Menon <nm@xxxxxx> >> >> >> +static unsigned long palmas_clks_recalc_rate(struct clk_hw *hw, >> >> + unsigned long parent_rate) >> >> +{ >> >> + return 32768; >> >> +} >> > >> > I see that other clock drivers using a constant rate return 0 if the >> > clock has not been enabled. >> >> and there are examples when similar fixed clock drivers returns only the clock >> value, like clk-max77686. I can not find clear guidelines neither in the >> documentation or around the header/c files for this. >> Mike, what is the appropriate way of handling the recalc_rate? > > You are right that there are no guidelines stating, "don't do that", but > please, "don't do that" ;-) > > clk_enable and clk_set_rate are entirely unrelated operations from the > perspective of the Linux clock framework, and mixing these two classes > of operations is a recipe for pain. > Thanks a lot for the clarification. I just asked since when posting a clock driver I got feedback that recalc_rate should return 0 if the clock was disabled and also saw some drivers doing that, e.g: drivers/clk/clk-s2mps11.c >> >> > So maybe is more correct to have something >> > like the following? >> > >> > if (__clk_is_enabled(hw->clk)) >> > return 32768; >> > else >> > return 0; > > So what happens here if this is gateable clock and later on we call > clk_enable on it? The clocks rate will still be zero since > clk_enable/clk_disable do not touch the rate at all. > Got it, thanks for the explanation. > Regards, > Mike > Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html