[PATCH 1/1] clk: introduce of_clk_detect_critical helper
Patrick DELAUNAY
patrick.delaunay at foss.st.com
Wed Aug 27 19:08:00 CEST 2025
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 at 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 at 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 at 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 at vger.kernel.org
Signed-off-by: Lee Jones <lee.jones at linaro.org>
Reviewed-by: Stephen Boyd <sboyd at codeaurora.org>
Signed-off-by: Michael Turquette <mturquette at baylibre.com>
Link:
lkml.kernel.org/r/1455225554-13267-4-git-send-email-mturquette at 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
More information about the U-Boot
mailing list