[PATCH 1/1] clk: introduce of_clk_detect_critical helper

[PATCH 1/1] clk: introduce of_clk_detect_critical helper

[Maxim Kochetkov]

This helper find clock-critiacl property in dts and assign
CLK_IS_CRITICAL flag to the clock.

Signed-off-by: Maxim Kochetkov <fido_max@inbox.ru>
---
 drivers/clk/clk-uclass.c | 17 +++++++++++++++++
 drivers/core/of_access.c | 22 ++++++++++++++++++++++
 include/clk.h            | 28 ++++++++++++++++++++++++++++
 include/dm/of_access.h   | 17 +++++++++++++++++
 4 files changed, 84 insertions(+)

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index 7262e89b512..4f35b90382f 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -835,6 +835,23 @@ struct clk *devm_clk_get(struct udevice *dev, const char *id)
 	return clk;
 }
 
+int of_clk_detect_critical(struct device_node *np, int index,
+			   unsigned long *flags)
+{
+	struct property *prop;
+	const __be32 *cur;
+	uint32_t idx;
+
+	if (!np || !flags)
+		return -EINVAL;
+
+	of_property_for_each_u32(np, "clock-critical", prop, cur, idx)
+		if (index == idx)
+			*flags |= CLK_IS_CRITICAL;
+
+	return 0;
+}
+
 int clk_uclass_post_probe(struct udevice *dev)
 {
 	/*
diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c
index b11e36202c1..59dda3ae152 100644
--- a/drivers/core/of_access.c
+++ b/drivers/core/of_access.c
@@ -666,6 +666,28 @@ int of_property_read_string_helper(const struct device_node *np,
 	return i <= 0 ? -ENODATA : i;
 }
 
+const __be32 *of_prop_next_u32(struct property *prop, const __be32 *cur,
+			       u32 *pu)
+{
+	const void *curv = cur;
+
+	if (!prop)
+		return NULL;
+
+	if (!cur) {
+		curv = prop->value;
+		goto out_val;
+	}
+
+	curv += sizeof(*cur);
+	if (curv >= prop->value + prop->length)
+		return NULL;
+
+out_val:
+	*pu = be32_to_cpup(curv);
+	return curv;
+}
+
 static int __of_root_parse_phandle_with_args(struct device_node *root,
 					     const struct device_node *np,
 					     const char *list_name,
diff --git a/include/clk.h b/include/clk.h
index f94135ff778..ea708f54fd5 100644
--- a/include/clk.h
+++ b/include/clk.h
@@ -589,6 +589,27 @@ bool clk_dev_binded(struct clk *clk);
  */
 ulong clk_get_id(const struct clk *clk);
 
+/**
+ * of_clk_detect_critical() - set CLK_IS_CRITICAL flag from Device Tree
+ * @np: Device node pointer associated with clock provider
+ * @index: clock index
+ * @flags: pointer to top-level framework flags
+ *
+ * Detects if the clock-critical property exists and, if so, sets the
+ * corresponding CLK_IS_CRITICAL flag.
+ *
+ * Do not use this function. It exists only for legacy Device Tree
+ * bindings, such as the one-clock-per-node style that are outdated.
+ * Those bindings typically put all clock data into .dts and the Linux
+ * driver has no clock data, thus making it impossible to set this flag
+ * correctly from the driver. Only those drivers may call
+ * of_clk_detect_critical from their setup functions.
+ *
+ * Return: error code or zero on success
+ */
+int of_clk_detect_critical(struct device_node *np, int index,
+			   unsigned long *flags);
+
 #else /* CONFIG_IS_ENABLED(CLK) */
 
 static inline int clk_request(struct udevice *dev, struct clk *clk)
@@ -665,6 +686,13 @@ static inline ulong clk_get_id(const struct clk *clk)
 {
 	return 0;
 }
+
+int of_clk_detect_critical(struct device_node *np, int index,
+			   unsigned long *flags)
+{
+	return 0;
+}
+
 #endif /* CONFIG_IS_ENABLED(CLK) */
 
 /**
diff --git a/include/dm/of_access.h b/include/dm/of_access.h
index 44143a5a391..dba725498f5 100644
--- a/include/dm/of_access.h
+++ b/include/dm/of_access.h
@@ -405,6 +405,17 @@ int of_property_read_string_helper(const struct device_node *np,
 				   const char *propname, const char **out_strs,
 				   size_t sz, int index);
 
+/*
+ * struct property *prop;
+ * const __be32 *p;
+ * u32 u;
+ *
+ * of_property_for_each_u32(np, "propname", prop, p, u)
+ *         printk("U32 value: %x\n", u);
+ */
+const __be32 *of_prop_next_u32(struct property *prop, const __be32 *cur,
+			       u32 *pu);
+
 /**
  * of_property_read_string_index() - Find and read a string from a multiple
  * strings property.
@@ -701,4 +712,10 @@ int of_remove_property(struct device_node *np, struct property *prop);
  */
 int of_remove_node(struct device_node *to_remove);
 
+#define of_property_for_each_u32(np, propname, prop, p, u)	\
+	for (prop = of_find_property(np, propname, NULL),	\
+		p = of_prop_next_u32(prop, NULL, &u);		\
+		p;						\
+		p = of_prop_next_u32(prop, p, &u))
+
 #endif
-- 
2.50.0

Re: [PATCH 1/1] clk: introduce of_clk_detect_critical helper

[Christian Marangi]

On Tue, Aug 12, 2025 at 07:29:46PM +0300, Maxim Kochetkov wrote:
> This helper find clock-critiacl property in dts and assign

clock-critiacl -> typo

> CLK_IS_CRITICAL flag to the clock.
> 
> Signed-off-by: Maxim Kochetkov <fido_max@inbox.ru>
> ---
>  drivers/clk/clk-uclass.c | 17 +++++++++++++++++
>  drivers/core/of_access.c | 22 ++++++++++++++++++++++
>  include/clk.h            | 28 ++++++++++++++++++++++++++++
>  include/dm/of_access.h   | 17 +++++++++++++++++
>  4 files changed, 84 insertions(+)
> 
> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> index 7262e89b512..4f35b90382f 100644
> --- a/drivers/clk/clk-uclass.c
> +++ b/drivers/clk/clk-uclass.c
> @@ -835,6 +835,23 @@ struct clk *devm_clk_get(struct udevice *dev, const char *id)
>  	return clk;
>  }
>  
> +int of_clk_detect_critical(struct device_node *np, int index,
> +			   unsigned long *flags)
> +{
> +	struct property *prop;
> +	const __be32 *cur;
> +	uint32_t idx;
> +
> +	if (!np || !flags)
> +		return -EINVAL;
> +
> +	of_property_for_each_u32(np, "clock-critical", prop, cur, idx)
> +		if (index == idx)
> +			*flags |= CLK_IS_CRITICAL;
> +
> +	return 0;
> +}
> +
>  int clk_uclass_post_probe(struct udevice *dev)
>  {
>  	/*
> diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c
> index b11e36202c1..59dda3ae152 100644
> --- a/drivers/core/of_access.c
> +++ b/drivers/core/of_access.c
> @@ -666,6 +666,28 @@ int of_property_read_string_helper(const struct device_node *np,
>  	return i <= 0 ? -ENODATA : i;
>  }
>  
> +const __be32 *of_prop_next_u32(struct property *prop, const __be32 *cur,
> +			       u32 *pu)
> +{
> +	const void *curv = cur;
> +
> +	if (!prop)
> +		return NULL;
> +
> +	if (!cur) {
> +		curv = prop->value;
> +		goto out_val;
> +	}
> +
> +	curv += sizeof(*cur);
> +	if (curv >= prop->value + prop->length)
> +		return NULL;
> +
> +out_val:
> +	*pu = be32_to_cpup(curv);
> +	return curv;
> +}
> +
>  static int __of_root_parse_phandle_with_args(struct device_node *root,
>  					     const struct device_node *np,
>  					     const char *list_name,
> diff --git a/include/clk.h b/include/clk.h
> index f94135ff778..ea708f54fd5 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -589,6 +589,27 @@ bool clk_dev_binded(struct clk *clk);
>   */
>  ulong clk_get_id(const struct clk *clk);
>  
> +/**
> + * of_clk_detect_critical() - set CLK_IS_CRITICAL flag from Device Tree
> + * @np: Device node pointer associated with clock provider
> + * @index: clock index
> + * @flags: pointer to top-level framework flags
> + *
> + * Detects if the clock-critical property exists and, if so, sets the
> + * corresponding CLK_IS_CRITICAL flag.
> + *
> + * Do not use this function. It exists only for legacy Device Tree
> + * bindings, such as the one-clock-per-node style that are outdated.
> + * Those bindings typically put all clock data into .dts and the Linux
> + * driver has no clock data, thus making it impossible to set this flag
> + * correctly from the driver. Only those drivers may call
> + * of_clk_detect_critical from their setup functions.
> + *
> + * Return: error code or zero on success
> + */
> +int of_clk_detect_critical(struct device_node *np, int index,
> +			   unsigned long *flags);
> +
>  #else /* CONFIG_IS_ENABLED(CLK) */
>  
>  static inline int clk_request(struct udevice *dev, struct clk *clk)
> @@ -665,6 +686,13 @@ static inline ulong clk_get_id(const struct clk *clk)
>  {
>  	return 0;
>  }
> +
> +int of_clk_detect_critical(struct device_node *np, int index,
> +			   unsigned long *flags)
> +{
> +	return 0;
> +}
> +
>  #endif /* CONFIG_IS_ENABLED(CLK) */
>  
>  /**
> diff --git a/include/dm/of_access.h b/include/dm/of_access.h
> index 44143a5a391..dba725498f5 100644
> --- a/include/dm/of_access.h
> +++ b/include/dm/of_access.h
> @@ -405,6 +405,17 @@ int of_property_read_string_helper(const struct device_node *np,
>  				   const char *propname, const char **out_strs,
>  				   size_t sz, int index);
>  
> +/*
> + * struct property *prop;
> + * const __be32 *p;
> + * u32 u;
> + *
> + * of_property_for_each_u32(np, "propname", prop, p, u)
> + *         printk("U32 value: %x\n", u);
> + */
> +const __be32 *of_prop_next_u32(struct property *prop, const __be32 *cur,
> +			       u32 *pu);
> +
>  /**
>   * of_property_read_string_index() - Find and read a string from a multiple
>   * strings property.
> @@ -701,4 +712,10 @@ int of_remove_property(struct device_node *np, struct property *prop);
>   */
>  int of_remove_node(struct device_node *to_remove);
>  
> +#define of_property_for_each_u32(np, propname, prop, p, u)	\
> +	for (prop = of_find_property(np, propname, NULL),	\
> +		p = of_prop_next_u32(prop, NULL, &u);		\
> +		p;						\
> +		p = of_prop_next_u32(prop, p, &u))
> +
>  #endif
> -- 

I would split these of additional function to a separate commit also
where they come from? They are ported from Linux Kernel?

Also would be ideal to have some scenario where it's needed to declare a
clock in DT that is critical. Isn't that handled directly by the clock
driver internally?

-- 
	Ansuel

Re: [PATCH 1/1] clk: introduce of_clk_detect_critical helper

[Maxim Kochetkov]

12.08.2025 19:41, Christian Marangi wrote:
> On Tue, Aug 12, 2025 at 07:29:46PM +0300, Maxim Kochetkov wrote:
>> This helper find clock-critiacl property in dts and assign
> 
> clock-critiacl -> typo


Thanks. Will fix it in v2

> 
>> CLK_IS_CRITICAL flag to the clock.
>>
>> Signed-off-by: Maxim Kochetkov <fido_max@inbox.ru>
>> ---
>>   drivers/clk/clk-uclass.c | 17 +++++++++++++++++
>>   drivers/core/of_access.c | 22 ++++++++++++++++++++++
>>   include/clk.h            | 28 ++++++++++++++++++++++++++++
>>   include/dm/of_access.h   | 17 +++++++++++++++++
>>   4 files changed, 84 insertions(+)
>>
>> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
>> index 7262e89b512..4f35b90382f 100644
>> --- a/drivers/clk/clk-uclass.c
>> +++ b/drivers/clk/clk-uclass.c
>> @@ -835,6 +835,23 @@ struct clk *devm_clk_get(struct udevice *dev, const char *id)
>>   	return clk;
>>   }
>>   
>> +int of_clk_detect_critical(struct device_node *np, int index,
>> +			   unsigned long *flags)
>> +{
>> +	struct property *prop;
>> +	const __be32 *cur;
>> +	uint32_t idx;
>> +
>> +	if (!np || !flags)
>> +		return -EINVAL;
>> +
>> +	of_property_for_each_u32(np, "clock-critical", prop, cur, idx)
>> +		if (index == idx)
>> +			*flags |= CLK_IS_CRITICAL;
>> +
>> +	return 0;
>> +}
>> +
>>   int clk_uclass_post_probe(struct udevice *dev)
>>   {
>>   	/*
>> diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c
>> index b11e36202c1..59dda3ae152 100644
>> --- a/drivers/core/of_access.c
>> +++ b/drivers/core/of_access.c
>> @@ -666,6 +666,28 @@ int of_property_read_string_helper(const struct device_node *np,
>>   	return i <= 0 ? -ENODATA : i;
>>   }
>>   
>> +const __be32 *of_prop_next_u32(struct property *prop, const __be32 *cur,
>> +			       u32 *pu)
>> +{
>> +	const void *curv = cur;
>> +
>> +	if (!prop)
>> +		return NULL;
>> +
>> +	if (!cur) {
>> +		curv = prop->value;
>> +		goto out_val;
>> +	}
>> +
>> +	curv += sizeof(*cur);
>> +	if (curv >= prop->value + prop->length)
>> +		return NULL;
>> +
>> +out_val:
>> +	*pu = be32_to_cpup(curv);
>> +	return curv;
>> +}
>> +
>>   static int __of_root_parse_phandle_with_args(struct device_node *root,
>>   					     const struct device_node *np,
>>   					     const char *list_name,
>> diff --git a/include/clk.h b/include/clk.h
>> index f94135ff778..ea708f54fd5 100644
>> --- a/include/clk.h
>> +++ b/include/clk.h
>> @@ -589,6 +589,27 @@ bool clk_dev_binded(struct clk *clk);
>>    */
>>   ulong clk_get_id(const struct clk *clk);
>>   
>> +/**
>> + * of_clk_detect_critical() - set CLK_IS_CRITICAL flag from Device Tree
>> + * @np: Device node pointer associated with clock provider
>> + * @index: clock index
>> + * @flags: pointer to top-level framework flags
>> + *
>> + * Detects if the clock-critical property exists and, if so, sets the
>> + * corresponding CLK_IS_CRITICAL flag.
>> + *
>> + * Do not use this function. It exists only for legacy Device Tree
>> + * bindings, such as the one-clock-per-node style that are outdated.
>> + * Those bindings typically put all clock data into .dts and the Linux
>> + * driver has no clock data, thus making it impossible to set this flag
>> + * correctly from the driver. Only those drivers may call
>> + * of_clk_detect_critical from their setup functions.
>> + *
>> + * Return: error code or zero on success
>> + */
>> +int of_clk_detect_critical(struct device_node *np, int index,
>> +			   unsigned long *flags);
>> +
>>   #else /* CONFIG_IS_ENABLED(CLK) */
>>   
>>   static inline int clk_request(struct udevice *dev, struct clk *clk)
>> @@ -665,6 +686,13 @@ static inline ulong clk_get_id(const struct clk *clk)
>>   {
>>   	return 0;
>>   }
>> +
>> +int of_clk_detect_critical(struct device_node *np, int index,
>> +			   unsigned long *flags)
>> +{
>> +	return 0;
>> +}
>> +
>>   #endif /* CONFIG_IS_ENABLED(CLK) */
>>   
>>   /**
>> diff --git a/include/dm/of_access.h b/include/dm/of_access.h
>> index 44143a5a391..dba725498f5 100644
>> --- a/include/dm/of_access.h
>> +++ b/include/dm/of_access.h
>> @@ -405,6 +405,17 @@ int of_property_read_string_helper(const struct device_node *np,
>>   				   const char *propname, const char **out_strs,
>>   				   size_t sz, int index);
>>   
>> +/*
>> + * struct property *prop;
>> + * const __be32 *p;
>> + * u32 u;
>> + *
>> + * of_property_for_each_u32(np, "propname", prop, p, u)
>> + *         printk("U32 value: %x\n", u);
>> + */
>> +const __be32 *of_prop_next_u32(struct property *prop, const __be32 *cur,
>> +			       u32 *pu);
>> +
>>   /**
>>    * of_property_read_string_index() - Find and read a string from a multiple
>>    * strings property.
>> @@ -701,4 +712,10 @@ int of_remove_property(struct device_node *np, struct property *prop);
>>    */
>>   int of_remove_node(struct device_node *to_remove);
>>   
>> +#define of_property_for_each_u32(np, propname, prop, p, u)	\
>> +	for (prop = of_find_property(np, propname, NULL),	\
>> +		p = of_prop_next_u32(prop, NULL, &u);		\
>> +		p;						\
>> +		p = of_prop_next_u32(prop, p, &u))
>> +
>>   #endif
>> -- 
> 
> I would split these of additional function to a separate commit also
> where they come from? They are ported from Linux Kernel?

Ok, I will split it.
Yes these function are ported from Linux Kernel.

> 
> Also would be ideal to have some scenario where it's needed to declare a
> clock in DT that is critical. Isn't that handled directly by the clock
> driver internally?
> 

Yes. Now we can use this property from DT by custom drivers to mark 
critical clocks.

Re: [PATCH 1/1] clk: introduce of_clk_detect_critical helper

[Quentin Schulz]

Hi Maxim,

On 8/12/25 8:56 PM, Maxim Kochetkov wrote:
> [You don't often get email from fido_max@inbox.ru. Learn why this is 
> important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> 12.08.2025 19:41, Christian Marangi wrote:
>> On Tue, Aug 12, 2025 at 07:29:46PM +0300, Maxim Kochetkov wrote:
>>> This helper find clock-critiacl property in dts and assign
>>
>> clock-critiacl -> typo
> 
> 
> Thanks. Will fix it in v2
> 
>>
>>> CLK_IS_CRITICAL flag to the clock.
>>>
>>> Signed-off-by: Maxim Kochetkov <fido_max@inbox.ru>
>>> ---
>>>   drivers/clk/clk-uclass.c | 17 +++++++++++++++++
>>>   drivers/core/of_access.c | 22 ++++++++++++++++++++++
>>>   include/clk.h            | 28 ++++++++++++++++++++++++++++
>>>   include/dm/of_access.h   | 17 +++++++++++++++++
>>>   4 files changed, 84 insertions(+)
>>>
>>> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
>>> index 7262e89b512..4f35b90382f 100644
>>> --- a/drivers/clk/clk-uclass.c
>>> +++ b/drivers/clk/clk-uclass.c
>>> @@ -835,6 +835,23 @@ struct clk *devm_clk_get(struct udevice *dev, 
>>> const char *id)
>>>      return clk;
>>>   }
>>>
>>> +int of_clk_detect_critical(struct device_node *np, int index,
>>> +                       unsigned long *flags)
>>> +{
>>> +    struct property *prop;
>>> +    const __be32 *cur;
>>> +    uint32_t idx;
>>> +
>>> +    if (!np || !flags)
>>> +            return -EINVAL;
>>> +
>>> +    of_property_for_each_u32(np, "clock-critical", prop, cur, idx)
>>> +            if (index == idx)
>>> +                    *flags |= CLK_IS_CRITICAL;
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>   int clk_uclass_post_probe(struct udevice *dev)
>>>   {
>>>      /*
>>> diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c
>>> index b11e36202c1..59dda3ae152 100644
>>> --- a/drivers/core/of_access.c
>>> +++ b/drivers/core/of_access.c
>>> @@ -666,6 +666,28 @@ int of_property_read_string_helper(const struct 
>>> device_node *np,
>>>      return i <= 0 ? -ENODATA : i;
>>>   }
>>>
>>> +const __be32 *of_prop_next_u32(struct property *prop, const __be32 
>>> *cur,
>>> +                           u32 *pu)
>>> +{
>>> +    const void *curv = cur;
>>> +
>>> +    if (!prop)
>>> +            return NULL;
>>> +
>>> +    if (!cur) {
>>> +            curv = prop->value;
>>> +            goto out_val;
>>> +    }
>>> +
>>> +    curv += sizeof(*cur);
>>> +    if (curv >= prop->value + prop->length)
>>> +            return NULL;
>>> +
>>> +out_val:
>>> +    *pu = be32_to_cpup(curv);
>>> +    return curv;
>>> +}
>>> +
>>>   static int __of_root_parse_phandle_with_args(struct device_node *root,
>>>                                           const struct device_node *np,
>>>                                           const char *list_name,
>>> diff --git a/include/clk.h b/include/clk.h
>>> index f94135ff778..ea708f54fd5 100644
>>> --- a/include/clk.h
>>> +++ b/include/clk.h
>>> @@ -589,6 +589,27 @@ bool clk_dev_binded(struct clk *clk);
>>>    */
>>>   ulong clk_get_id(const struct clk *clk);
>>>
>>> +/**
>>> + * of_clk_detect_critical() - set CLK_IS_CRITICAL flag from Device Tree
>>> + * @np: Device node pointer associated with clock provider
>>> + * @index: clock index
>>> + * @flags: pointer to top-level framework flags
>>> + *
>>> + * Detects if the clock-critical property exists and, if so, sets the
>>> + * corresponding CLK_IS_CRITICAL flag.
>>> + *
>>> + * Do not use this function. It exists only for legacy Device Tree
>>> + * bindings, such as the one-clock-per-node style that are outdated.
>>> + * Those bindings typically put all clock data into .dts and the Linux
>>> + * driver has no clock data, thus making it impossible to set this flag
>>> + * correctly from the driver. Only those drivers may call
>>> + * of_clk_detect_critical from their setup functions.
>>> + *
>>> + * Return: error code or zero on success
>>> + */
>>> +int of_clk_detect_critical(struct device_node *np, int index,
>>> +                       unsigned long *flags);
>>> +
>>>   #else /* CONFIG_IS_ENABLED(CLK) */
>>>
>>>   static inline int clk_request(struct udevice *dev, struct clk *clk)
>>> @@ -665,6 +686,13 @@ static inline ulong clk_get_id(const struct clk 
>>> *clk)
>>>   {
>>>      return 0;
>>>   }
>>> +
>>> +int of_clk_detect_critical(struct device_node *np, int index,
>>> +                       unsigned long *flags)
>>> +{
>>> +    return 0;
>>> +}
>>> +
>>>   #endif /* CONFIG_IS_ENABLED(CLK) */
>>>
>>>   /**
>>> diff --git a/include/dm/of_access.h b/include/dm/of_access.h
>>> index 44143a5a391..dba725498f5 100644
>>> --- a/include/dm/of_access.h
>>> +++ b/include/dm/of_access.h
>>> @@ -405,6 +405,17 @@ int of_property_read_string_helper(const struct 
>>> device_node *np,
>>>                                 const char *propname, const char 
>>> **out_strs,
>>>                                 size_t sz, int index);
>>>
>>> +/*
>>> + * struct property *prop;
>>> + * const __be32 *p;
>>> + * u32 u;
>>> + *
>>> + * of_property_for_each_u32(np, "propname", prop, p, u)
>>> + *         printk("U32 value: %x\n", u);
>>> + */
>>> +const __be32 *of_prop_next_u32(struct property *prop, const __be32 
>>> *cur,
>>> +                           u32 *pu);
>>> +
>>>   /**
>>>    * of_property_read_string_index() - Find and read a string from a 
>>> multiple
>>>    * strings property.
>>> @@ -701,4 +712,10 @@ int of_remove_property(struct device_node *np, 
>>> struct property *prop);
>>>    */
>>>   int of_remove_node(struct device_node *to_remove);
>>>
>>> +#define of_property_for_each_u32(np, propname, prop, p, u)  \
>>> +    for (prop = of_find_property(np, propname, NULL),       \
>>> +            p = of_prop_next_u32(prop, NULL, &u);           \
>>> +            p;                                              \
>>> +            p = of_prop_next_u32(prop, p, &u))
>>> +
>>>   #endif
>>> -- 
>>
>> I would split these of additional function to a separate commit also
>> where they come from? They are ported from Linux Kernel?
> 
> Ok, I will split it.
> Yes these function are ported from Linux Kernel.
> 
>>
>> Also would be ideal to have some scenario where it's needed to declare a
>> clock in DT that is critical. Isn't that handled directly by the clock
>> driver internally?
>>
> 
> Yes. Now we can use this property from DT by custom drivers to mark
> critical clocks.
> 

How this is typically handled today is by hardcoding the clock that is 
critical in the SoC clock controller driver directly, c.f. 
https://lore.kernel.org/linux-rockchip/20241214224820.200665-1-heiko@sntech.de/

This is of course then forced-enabled for all boards based on this SoC. 
If that is something that isn't desirable, then we need to properly need 
to represent this clock in the Device Tree. You haven't really explained 
the real life example that would benefit from this. Is there one? What 
is it?

Finally, we do not accept Device Tree properties anymore if they aren't 
part of the Device Tree binding coming from the kernel, the Device Tree 
specification or the dt-schema. So it needs to go through those channels 
first before we can make use of this property here in U-Boot. Our 
ultimate goal is to have the exact same Device Tree for U-Boot than in 
the Linux kernel (and I assume even further than that, anything that 
requires DT use the same one across all SW stack).

Cheers,
Quentin

Re: [PATCH 1/1] clk: introduce of_clk_detect_critical helper

[Patrick DELAUNAY]

Hi,

On 8/19/25 09:41, Quentin Schulz wrote:
> Hi Maxim,
>
> On 8/12/25 8:56 PM, Maxim Kochetkov wrote:
>> [You don't often get email from fido_max@inbox.ru. Learn why this is 
>> important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> 12.08.2025 19:41, Christian Marangi wrote:
>>> On Tue, Aug 12, 2025 at 07:29:46PM +0300, Maxim Kochetkov wrote:
>>>> This helper find clock-critiacl property in dts and assign
>>>
>>> clock-critiacl -> typo
>>
>>
>> Thanks. Will fix it in v2
>>
>>>
>>>> CLK_IS_CRITICAL flag to the clock.
>>>>
>>>> Signed-off-by: Maxim Kochetkov <fido_max@inbox.ru>
>>>> ---
>>>>   drivers/clk/clk-uclass.c | 17 +++++++++++++++++
>>>>   drivers/core/of_access.c | 22 ++++++++++++++++++++++
>>>>   include/clk.h            | 28 ++++++++++++++++++++++++++++
>>>>   include/dm/of_access.h   | 17 +++++++++++++++++
>>>>   4 files changed, 84 insertions(+)
>>>>
>>>> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
>>>> index 7262e89b512..4f35b90382f 100644
>>>> --- a/drivers/clk/clk-uclass.c
>>>> +++ b/drivers/clk/clk-uclass.c
>>>> @@ -835,6 +835,23 @@ struct clk *devm_clk_get(struct udevice *dev, 
>>>> const char *id)
>>>>      return clk;
>>>>   }
>>>>
>>>> +int of_clk_detect_critical(struct device_node *np, int index,
>>>> +                       unsigned long *flags)
>>>> +{
>>>> +    struct property *prop;
>>>> +    const __be32 *cur;
>>>> +    uint32_t idx;
>>>> +
>>>> +    if (!np || !flags)
>>>> +            return -EINVAL;
>>>> +
>>>> +    of_property_for_each_u32(np, "clock-critical", prop, cur, idx)
>>>> +            if (index == idx)
>>>> +                    *flags |= CLK_IS_CRITICAL;
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>>   int clk_uclass_post_probe(struct udevice *dev)
>>>>   {
>>>>      /*
>>>> diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c
>>>> index b11e36202c1..59dda3ae152 100644
>>>> --- a/drivers/core/of_access.c
>>>> +++ b/drivers/core/of_access.c
>>>> @@ -666,6 +666,28 @@ int of_property_read_string_helper(const 
>>>> struct device_node *np,
>>>>      return i <= 0 ? -ENODATA : i;
>>>>   }
>>>>
>>>> +const __be32 *of_prop_next_u32(struct property *prop, const __be32 
>>>> *cur,
>>>> +                           u32 *pu)
>>>> +{
>>>> +    const void *curv = cur;
>>>> +
>>>> +    if (!prop)
>>>> +            return NULL;
>>>> +
>>>> +    if (!cur) {
>>>> +            curv = prop->value;
>>>> +            goto out_val;
>>>> +    }
>>>> +
>>>> +    curv += sizeof(*cur);
>>>> +    if (curv >= prop->value + prop->length)
>>>> +            return NULL;
>>>> +
>>>> +out_val:
>>>> +    *pu = be32_to_cpup(curv);
>>>> +    return curv;
>>>> +}
>>>> +
>>>>   static int __of_root_parse_phandle_with_args(struct device_node 
>>>> *root,
>>>>                                           const struct device_node 
>>>> *np,
>>>>                                           const char *list_name,
>>>> diff --git a/include/clk.h b/include/clk.h
>>>> index f94135ff778..ea708f54fd5 100644
>>>> --- a/include/clk.h
>>>> +++ b/include/clk.h
>>>> @@ -589,6 +589,27 @@ bool clk_dev_binded(struct clk *clk);
>>>>    */
>>>>   ulong clk_get_id(const struct clk *clk);
>>>>
>>>> +/**
>>>> + * of_clk_detect_critical() - set CLK_IS_CRITICAL flag from Device 
>>>> Tree
>>>> + * @np: Device node pointer associated with clock provider
>>>> + * @index: clock index
>>>> + * @flags: pointer to top-level framework flags
>>>> + *
>>>> + * Detects if the clock-critical property exists and, if so, sets the
>>>> + * corresponding CLK_IS_CRITICAL flag.
>>>> + *
>>>> + * Do not use this function. It exists only for legacy Device Tree
>>>> + * bindings, such as the one-clock-per-node style that are outdated.
>>>> + * Those bindings typically put all clock data into .dts and the 
>>>> Linux
>>>> + * driver has no clock data, thus making it impossible to set this 
>>>> flag
>>>> + * correctly from the driver. Only those drivers may call
>>>> + * of_clk_detect_critical from their setup functions.
>>>> + *
>>>> + * Return: error code or zero on success
>>>> + */
>>>> +int of_clk_detect_critical(struct device_node *np, int index,
>>>> +                       unsigned long *flags);
>>>> +
>>>>   #else /* CONFIG_IS_ENABLED(CLK) */
>>>>
>>>>   static inline int clk_request(struct udevice *dev, struct clk *clk)
>>>> @@ -665,6 +686,13 @@ static inline ulong clk_get_id(const struct 
>>>> clk *clk)
>>>>   {
>>>>      return 0;
>>>>   }
>>>> +
>>>> +int of_clk_detect_critical(struct device_node *np, int index,
>>>> +                       unsigned long *flags)
>>>> +{
>>>> +    return 0;
>>>> +}
>>>> +
>>>>   #endif /* CONFIG_IS_ENABLED(CLK) */
>>>>
>>>>   /**
>>>> diff --git a/include/dm/of_access.h b/include/dm/of_access.h
>>>> index 44143a5a391..dba725498f5 100644
>>>> --- a/include/dm/of_access.h
>>>> +++ b/include/dm/of_access.h
>>>> @@ -405,6 +405,17 @@ int of_property_read_string_helper(const 
>>>> struct device_node *np,
>>>>                                 const char *propname, const char 
>>>> **out_strs,
>>>>                                 size_t sz, int index);
>>>>
>>>> +/*
>>>> + * struct property *prop;
>>>> + * const __be32 *p;
>>>> + * u32 u;
>>>> + *
>>>> + * of_property_for_each_u32(np, "propname", prop, p, u)
>>>> + *         printk("U32 value: %x\n", u);
>>>> + */
>>>> +const __be32 *of_prop_next_u32(struct property *prop, const __be32 
>>>> *cur,
>>>> +                           u32 *pu);
>>>> +
>>>>   /**
>>>>    * of_property_read_string_index() - Find and read a string from 
>>>> a multiple
>>>>    * strings property.
>>>> @@ -701,4 +712,10 @@ int of_remove_property(struct device_node *np, 
>>>> struct property *prop);
>>>>    */
>>>>   int of_remove_node(struct device_node *to_remove);
>>>>
>>>> +#define of_property_for_each_u32(np, propname, prop, p, u) \
>>>> +    for (prop = of_find_property(np, propname, NULL), \
>>>> +            p = of_prop_next_u32(prop, NULL, &u);           \
>>>> +            p; \
>>>> +            p = of_prop_next_u32(prop, p, &u))
>>>> +
>>>>   #endif
>>>> -- 
>>>
>>> I would split these of additional function to a separate commit also
>>> where they come from? They are ported from Linux Kernel?
>>
>> Ok, I will split it.
>> Yes these function are ported from Linux Kernel.
>>
>>>
>>> Also would be ideal to have some scenario where it's needed to 
>>> declare a
>>> clock in DT that is critical. Isn't that handled directly by the clock
>>> driver internally?
>>>
>>
>> Yes. Now we can use this property from DT by custom drivers to mark
>> critical clocks.
>>
>
> How this is typically handled today is by hardcoding the clock that is 
> critical in the SoC clock controller driver directly, c.f. 
> https://lore.kernel.org/linux-rockchip/20241214224820.200665-1-heiko@sntech.de/ 
>
>
> This is of course then forced-enabled for all boards based on this 
> SoC. If that is something that isn't desirable, then we need to 
> properly need to represent this clock in the Device Tree. You haven't 
> really explained the real life example that would benefit from this. 
> Is there one? What is it?
>
> Finally, we do not accept Device Tree properties anymore if they 
> aren't part of the Device Tree binding coming from the kernel, the 
> Device Tree specification or the dt-schema. So it needs to go through 
> those channels first before we can make use of this property here in 
> U-Boot. Our ultimate goal is to have the exact same Device Tree for 
> U-Boot than in the Linux kernel (and I assume even further than that, 
> anything that requires DT use the same one across all SW stack).
>
> Cheers,
> Quentin


See Linux commit d56f8994b6fb

it is a legacy feature for Linux side, not part of generic clock binding

and no  used in Linux's device tree

-----------------------------------------------------------

Author: Lee Jones <lee@kernel.org>
Date:   Thu Feb 11 13:19:11 2016 -0800

     clk: Provide OF helper to mark clocks as CRITICAL

     This call matches clocks which have been marked as critical in DT
     and sets the appropriate flag.  These flags can then be used to
     mark the clock core flags appropriately prior to registration.

     Legacy bindings requiring this feature must add the clock-critical
     property to their binding descriptions, as it is not a part of
     common-clock binding.

     Cc: devicetree@vger.kernel.org
     Signed-off-by: Lee Jones <lee.jones@linaro.org>
     Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
     Signed-off-by: Michael Turquette <mturquette@baylibre.com>
     Link: 
lkml.kernel.org/r/1455225554-13267-4-git-send-email-mturquette@baylibre.com

Regards.

---------------------------------------------------------


=> custom drivers never mark a clock critical ...

       is a is a clock driver decision (common for all board, for 
mandatory clock in SoC,

       root clock or CPU clock for example = no need to be cutomized in 
DT for each board)


for information:  with CLK_CCF, this custom driver (device or board) can 
enable a clock found

in the device tree,  and its user counter become >1 (@enable_count),

the clock is always enable for your use case (if enable/disabled are 
correctly paired in other

drivers and with activate CCF) .... and you have no need to set it 
critical at SoC / Clock driver level


Regards


Patrick