Re: [PATCH v5 2/2] clk: Add gpio-locked fixed clock driver

[Jerome Brunet]

On jeu. 17 sept. 2026 at 19:05, Vyacheslav Yurkov <uvv.mail@xxxxxxxxx> wrote:

> On 16.09.2026 15:57, Jerome Brunet wrote:
>
>>>>> +/* We can't enable the clock, but the Common Clock Framework calls only
>>>>> + * enable() not is_enabled()
>>>>> + */
>>>>> +static int gpio_locked_clk_enable(struct clk_hw *hw)
>>>>> +{
>>>>> +	return gpio_locked_clk_is_enabled(hw);
>>>>> +}
>> 
>> You need to explain your problem a bit more because this is not OK. Your
>> clock should really just provide .is_enabled() AFAICT
>
> When a peripheral driver uses devm_clk_get_enabled(), this is resolved 
> to clk_prepare() and clk_enable(). Neither of them check for 
> .is_enabled(). Is that a flaw in the CCF or expected behavior?
>

.enable() does return an error on failure. It is up to the provider to
return one or not. In your case, it will be probably be necessary to on
the gpio a little.

Note that if the GPIO can sleep, interacting with the gpio must happen
in prepare (think i2c gpio devices)

>
>>>>> +
>>>>> +/* We have to implement it, but we are not going to control
>>>>> + * parent clock selection
>>>>> + */
>>>>> +static u8 gpio_locked_clk_get_parent(struct clk_hw *hw)
>>>>> +{
>>>>> +	return 0;
>>>>> +}
>> 
>> Same, I dont get why you need that. Not needed if there a single parent
>
> This is a remnant from v4, thanks for spotting it.
>
> Slava

-- 
Jerome