Re: [PATCH 2/3] clk: Introduce 'critical-clocks' property

[Marek Vasut <marex@denx.de>]

On 2/16/22 13:06, Vaittinen, Matti wrote:

Hi,

[...]

>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>> index 8de6a22498e70..1e1686fa76e01 100644
>> --- a/drivers/clk/clk.c
>> +++ b/drivers/clk/clk.c
>> @@ -3872,6 +3872,45 @@ static void clk_core_free_parent_map(struct clk_core *core)
>>    	kfree(core->parents);
>>    }
>>    
>> +static void
>> +__clk_register_critical_clock(struct device_node *np, struct clk_core *core,
>> +			      struct clk_hw *hw)
>> +{
>> +	struct of_phandle_args clkspec;
>> +	u32 clksize, clktotal;
>> +	int ret, i, index;
>> +
>> +	if (!np)
>> +		return;
>> +
>> +	if (!core->ops->match_clkspec)
>> +		return;
>> +
>> +	if (of_property_read_u32(np, "#clock-cells", &clksize))
>> +		return;
>> +
>> +	/* Clock node with #clock-cells = <0> uses critical-clocks; */
>> +	if (clksize == 0) {
>> +		if (of_property_read_bool(np, "critical-clocks") &&
>> +		    !core->ops->match_clkspec(hw, &clkspec))
> 
> I think this is never true as there is
> if (!core->ops->match_clkspec)
> 	return;
> 
> above.

If the driver implements match_clkspec() callback, then the callback 
gets used here to determine whether the clock match this clkspec.

> Anyways, seeing you added a dummy bd71837_match_clkspec in a follow-up
> patch for BD71837 - which has only single clock - I wonder if there is a
> way to omit that dummy callback in controllers which really provide only
> one clock?

Yes, I think we can omit the match_clkspec call for clock controllers 
with clock-cells == 0 altogether.