linux-kernel - Re: [RESEND 2/2] clk: Add driver for Palmas clk32kg and clk32kgaudio clocks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CABxcv==Ut40bQg22hWuXmMgHF7b_iLfJuLHrdGds1B3wXb74ug@mail.gmail.com>
Date:	Wed, 2 Jul 2014 11:16:53 +0200
From:	Javier Martinez Canillas <javier@...hile0.org>
To:	Mike Turquette <mturquette@...aro.org>
Cc:	Peter Ujfalusi <peter.ujfalusi@...com>,
	Grant Likely <grant.likely@...aro.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	"linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>,
	Laxman Dewangan <ldewangan@...dia.com>,
	linux-doc@...r.kernel.org, Nishanth Menon <nm@...com>,
	Rob Herring <robh+dt@...nel.org>
Subject: Re: [RESEND 2/2] clk: Add driver for Palmas clk32kg and clk32kgaudio clocks

Hello Mike,

On Wed, Jul 2, 2014 at 6:33 AM, Mike Turquette <mturquette@...aro.org> 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@...com> 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@...com>
>> >> Reviewed-by: Nishanth Menon <nm@...com>
>>
>> >> +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 linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/