* [PATCH 0/2] A proposal to add a virtual clock controller guard.
@ 2026-03-18 17:43 Vyacheslav Yurkov via B4 Relay
2026-03-18 17:43 ` [PATCH 1/2] clk: Add " Vyacheslav Yurkov via B4 Relay
2026-03-18 17:43 ` [PATCH 2/2] dt-bindings: Add clock guard DT description Vyacheslav Yurkov via B4 Relay
0 siblings, 2 replies; 17+ messages in thread
From: Vyacheslav Yurkov via B4 Relay @ 2026-03-18 17:43 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-kernel, linux-clk, devicetree, Vyacheslav Yurkov,
Vyacheslav Yurkov
The clock controller guard driver acts as clock provider and provides only
one clock that consumers can check to make sure whether all other conditions
are met in order to enable other peripehrals. This can be seen as 1-to-N
clock relation, thus consumers care only about one clock and not about N.
The usage example for such a driver is when peripherals depend on PLLs in
a FPGA, which can't be directly accessed by the CPU, but need a GPIO pin
to chekc whether clock is actually usable.
Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com>
Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
---
Vyacheslav Yurkov (2):
clk: Add clock controller guard
dt-bindings: Add clock guard DT description
.../bindings/clock/clock-controller-guard.yaml | 79 +++++
drivers/clk/Kconfig | 12 +
drivers/clk/Makefile | 1 +
drivers/clk/clkctrl-guard.c | 334 +++++++++++++++++++++
4 files changed, 426 insertions(+)
---
base-commit: 4f3df2e5ea69f5717d2721922aff263c31957548
change-id: 20260318-feature-clock-guard-f20a2c35b965
Best regards,
--
Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 1/2] clk: Add clock controller guard 2026-03-18 17:43 [PATCH 0/2] A proposal to add a virtual clock controller guard Vyacheslav Yurkov via B4 Relay @ 2026-03-18 17:43 ` Vyacheslav Yurkov via B4 Relay 2026-03-19 8:15 ` kernel test robot 2026-03-18 17:43 ` [PATCH 2/2] dt-bindings: Add clock guard DT description Vyacheslav Yurkov via B4 Relay 1 sibling, 1 reply; 17+ messages in thread From: Vyacheslav Yurkov via B4 Relay @ 2026-03-18 17:43 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-kernel, linux-clk, devicetree, Vyacheslav Yurkov, Vyacheslav Yurkov From: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com> A clock controller guard driver acts as clock provider and provides only one clock that consumers can check to make sure whether all other conditions are met in order to enable other peripehrals. This can be seen as 1:N clock relation, thus consumers care only about one clock and not about N. Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com> Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com> --- drivers/clk/Kconfig | 12 ++ drivers/clk/Makefile | 1 + drivers/clk/clkctrl-guard.c | 334 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 347 insertions(+) diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index 3d803b4cf5c1..4ce61014754b 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -499,6 +499,18 @@ config COMMON_CLK_RPMI Support for clocks based on the clock service group defined by the RISC-V platform management interface (RPMI) specification. +config CLKCTRL_GUARD + tristate "Clock controller guard" + depends on COMMON_CLK && OF + default n + help + A virtual clock controller that can be used on platfroms where + several clocks and GPIO signals are required to be enabled first + before peripheral initialization. The signals can be routed to this + controller, which simplifies peripheral driver's probe procedure. + + If unsure, say N. + source "drivers/clk/actions/Kconfig" source "drivers/clk/analogbits/Kconfig" source "drivers/clk/aspeed/Kconfig" diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index f7bce3951a30..7a3adc341c0b 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -35,6 +35,7 @@ obj-$(CONFIG_CLK_FD_KUNIT_TEST) += clk-fractional-divider_test.o obj-$(CONFIG_COMMON_CLK) += clk-gpio.o ifeq ($(CONFIG_OF), y) obj-$(CONFIG_COMMON_CLK) += clk-conf.o +obj-$(CONFIG_CLKCTRL_GUARD) += clkctrl-guard.o endif # KUnit specific helpers diff --git a/drivers/clk/clkctrl-guard.c b/drivers/clk/clkctrl-guard.c new file mode 100644 index 000000000000..6978c36543de --- /dev/null +++ b/drivers/clk/clkctrl-guard.c @@ -0,0 +1,334 @@ +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +/* + * Clock Controller Guard Driver + * + * Copyright 2026 Bruker Corporation + */ + +#include <linux/clk.h> +#include <linux/clk-provider.h> +#include <linux/device.h> +#include <linux/gpio/consumer.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/slab.h> +#include <linux/version.h> + +#define MAX_INPUT_GPIO_COUNT 32 + +/** + * struct clkctrl_guard_priv - private state for the whole controller + * @dev: platform device + * + * @clks: array of input clock descriptors + * @num_clks: number of entries in @inputs + * + * @gpios: array of GPIO descriptors + * @gpio_names: GPIO names + * @num_gpios: number of input GPIOs + * + * @output_hw_clk: output clock HW descriptor + * @output_clock_name: output clock name + */ +struct clkctrl_guard_priv { + struct device *dev; + + struct clk_bulk_data *clks; + int num_clks; + + struct gpio_descs *gpios; + const char **gpio_names; + int num_gpios; + + struct clk_hw output_hw_clk; + const char *output_clock_name; +}; + +#define to_clkctrl_guard_priv(_hw) \ + container_of(_hw, struct clkctrl_guard_priv, output_hw_clk) + +static int clkctrl_guard_enable(struct clk_hw *hw) +{ + struct clkctrl_guard_priv *priv = to_clkctrl_guard_priv(hw); + + dev_dbg(priv->dev, "enable output clk '%s'\n", priv->output_clock_name); + + return clk_bulk_enable(priv->num_clks, priv->clks); +} + +static void clkctrl_guard_disable(struct clk_hw *hw) +{ + struct clkctrl_guard_priv *priv = to_clkctrl_guard_priv(hw); + + dev_dbg(priv->dev, "disable output clk '%s'\n", priv->output_clock_name); + + clk_bulk_disable(priv->num_clks, priv->clks); +} + +static int clkctrl_guard_prepare(struct clk_hw *hw) +{ + struct clkctrl_guard_priv *priv = to_clkctrl_guard_priv(hw); + + return clk_bulk_prepare(priv->num_clks, priv->clks); +} + +static void clkctrl_guard_unprepare(struct clk_hw *hw) +{ + struct clkctrl_guard_priv *priv = to_clkctrl_guard_priv(hw); + + clk_bulk_unprepare(priv->num_clks, priv->clks); +} + +static int is_gpio_ready(struct clkctrl_guard_priv *priv) +{ + unsigned long values[BITS_TO_LONGS(MAX_INPUT_GPIO_COUNT)]; + int ret = 0; + + if (priv->num_gpios == 0) + return 0; + + ret = gpiod_get_array_value(priv->gpios->ndescs, + priv->gpios->desc, + priv->gpios->info, + values); + + if (ret) { + dev_err(priv->dev, "Failed to read GPIOs"); + return -EIO; + } + + for (int i = 0; i < priv->gpios->ndescs; i++) { + if (!test_bit(i, values)) { + dev_warn(priv->dev, "GPIO %s is not ready", priv->gpio_names[i]); + return -EBUSY; + } + } + + return 0; +} + +static int clkctrl_guard_is_prepared(struct clk_hw *hw) +{ + struct clkctrl_guard_priv *priv = to_clkctrl_guard_priv(hw); + int ret = 0; + + if (priv->num_gpios > 0) { + ret = is_gpio_ready(priv); + if (ret < 0) + return ret; + } + + // Now check for the clocks + for (int i = 0; i < priv->num_clks; i++) { + struct clk_hw *hw_clk = __clk_get_hw(priv->clks[i].clk); + + if (!clk_hw_is_prepared(hw_clk)) { + dev_dbg(priv->dev, "Clock %i (%s) is not ready", + i, priv->clks[i].id); + return -EBUSY; + } + } + + return 0; +} + +/* We have to implement it, but we are not going to control + * parent clock selection + */ +static u8 clkctrl_guard_get_parent(struct clk_hw *hw) +{ + return 0; +} + +static const struct clk_ops clkctrl_guard_ops = { + .enable = clkctrl_guard_enable, + .disable = clkctrl_guard_disable, + .prepare = clkctrl_guard_prepare, + .unprepare = clkctrl_guard_unprepare, + .is_prepared = clkctrl_guard_is_prepared, + .get_parent = clkctrl_guard_get_parent, +}; + +static int clkctrl_guard_parse_inputs(struct clkctrl_guard_priv *priv) +{ + struct device *dev = priv->dev; + int ret; + + ret = devm_clk_bulk_get_all(dev, &priv->clks); + if (ret < 0) { + dev_err(dev, "failed to get input clocks: %d\n", ret); + return ret ? ret : -ENOENT; + } + + priv->num_clks = ret; + + if (priv->num_clks == 0) + dev_info(dev, "No input clocks provided\n"); + + for (int i = 0; i < priv->num_clks; i++) + dev_dbg(dev, "input clk[%d]: name='%s' rate=%lu Hz\n", + i, priv->clks[i].id, + clk_get_rate(priv->clks[i].clk)); + + return 0; +} + +static int clkctrl_guard_parse_gpios(struct clkctrl_guard_priv *priv) +{ + struct device *dev = priv->dev; + struct device_node *np = dev->of_node; + int i; + + priv->gpios = devm_gpiod_get_array_optional(dev, NULL, GPIOD_ASIS); + if (IS_ERR(priv->gpios)) { + dev_err(dev, "failed to get GPIO array: %ld\n", + PTR_ERR(priv->gpios)); + return PTR_ERR(priv->gpios); + } + + if (!priv->gpios) { + dev_info(dev, "No GPIOs provided, continue\n"); + priv->num_gpios = 0; + return 0; + } + + priv->num_gpios = priv->gpios->ndescs; + if (priv->num_gpios > 32) { + dev_err(priv->dev, "Maximum number of input GPIOs is 32\n"); + return -EINVAL; + } + + /* gpio_descs carries no names, so read "gpio-names" separately */ + priv->gpio_names = devm_kcalloc(dev, priv->num_gpios, sizeof(*priv->gpio_names), + GFP_KERNEL); + if (!priv->gpio_names) + return -ENOMEM; + + for (i = 0; i < priv->num_gpios; i++) { + of_property_read_string_index(np, "gpio-names", i, + &priv->gpio_names[i]); + + dev_dbg(dev, "gpio[%d]: name='%s'\n", + i, priv->gpio_names[i] ? priv->gpio_names[i] : "(unnamed)"); + } + + return 0; +} + +static int clkctrl_guard_parse_outputs(struct clkctrl_guard_priv *priv) +{ + struct device *dev = priv->dev; + struct device_node *np = dev->of_node; + struct clk_init_data init = {}; + int ret; + + of_property_read_string_index(np, "clock-output-names", 0, + &priv->output_clock_name); + + if (!priv->output_clock_name) + priv->output_clock_name = dev_name(priv->dev); + + init.name = priv->output_clock_name; + init.ops = &clkctrl_guard_ops; + init.flags = 0; + init.num_parents = priv->num_clks; + + if (priv->num_clks) { + const char **parent_names; + int j; + + parent_names = devm_kcalloc(dev, priv->num_clks, + sizeof(*parent_names), + GFP_KERNEL); + if (!parent_names) + return -ENOMEM; + + for (j = 0; j < priv->num_clks; j++) + parent_names[j] = priv->clks[j].id; + + init.parent_names = parent_names; + } + + priv->output_hw_clk.init = &init; + + ret = devm_clk_hw_register(dev, &priv->output_hw_clk); + if (ret) { + dev_err(dev, "failed to register output clk'%s': %d\n", + priv->output_clock_name, ret); + return ret; + } + + dev_info(priv->dev, "Output clock '%s' registered\n", priv->output_clock_name); + + return 0; +} + +static int clkctrl_guard_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct clkctrl_guard_priv *priv; + int ret; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->dev = dev; + platform_set_drvdata(pdev, priv); + + ret = clkctrl_guard_parse_inputs(priv); + if (ret) + return ret; + + ret = clkctrl_guard_parse_gpios(priv); + if (ret) + return ret; + + if (priv->num_clks == 0 && priv->num_gpios == 0) { + dev_err(priv->dev, "At least 1 input clock or input GPIO is required\n"); + return -EINVAL; + } + + ret = clkctrl_guard_parse_outputs(priv); + if (ret) + return ret; + + ret = devm_of_clk_add_hw_provider(priv->dev, of_clk_hw_simple_get, + &priv->output_hw_clk); + if (ret) { + dev_err(priv->dev, "failed to register clock provider '%s': %d\n", + priv->output_clock_name, ret); + return ret; + } + + dev_info(dev, "registered %u input clocks, %u GPIOs\n", + priv->num_clks, priv->num_gpios); + + return 0; +} + +static void clkctrl_guard_remove(struct platform_device *pdev) +{ + dev_dbg(&pdev->dev, "removed\n"); +} + +static const struct of_device_id clkctrl_guard_of_match[] = { + { .compatible = "clock-controller-guard" }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, clkctrl_guard_of_match); + +static struct platform_driver clkctrl_guard_driver = { + .probe = clkctrl_guard_probe, + .remove = clkctrl_guard_remove, + .driver = { + .name = "clock-controller-guard", + .of_match_table = clkctrl_guard_of_match, + }, +}; +module_platform_driver(clkctrl_guard_driver); + +MODULE_AUTHOR("Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>"); +MODULE_DESCRIPTION("GPIO-controlled clock controller driver"); +MODULE_LICENSE("Dual BSD/GPL"); -- 2.34.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] clk: Add clock controller guard 2026-03-18 17:43 ` [PATCH 1/2] clk: Add " Vyacheslav Yurkov via B4 Relay @ 2026-03-19 8:15 ` kernel test robot 0 siblings, 0 replies; 17+ messages in thread From: kernel test robot @ 2026-03-19 8:15 UTC (permalink / raw) To: Vyacheslav Yurkov via B4 Relay, Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: oe-kbuild-all, linux-kernel, linux-clk, devicetree, Vyacheslav Yurkov Hi Vyacheslav, kernel test robot noticed the following build warnings: [auto build test WARNING on 4f3df2e5ea69f5717d2721922aff263c31957548] url: https://github.com/intel-lab-lkp/linux/commits/Vyacheslav-Yurkov-via-B4-Relay/clk-Add-clock-controller-guard/20260319-151001 base: 4f3df2e5ea69f5717d2721922aff263c31957548 patch link: https://lore.kernel.org/r/20260318-feature-clock-guard-v1-1-6137cb4084b7%40bruker.com patch subject: [PATCH 1/2] clk: Add clock controller guard reproduce: (https://download.01.org/0day-ci/archive/20260319/202603190902.jG6JLJIo-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202603190902.jG6JLJIo-lkp@intel.com/ versioncheck warnings: (new ones prefixed by >>) INFO PATH=/opt/cross/rustc-1.88.0-bindgen-0.72.1/cargo/bin:/opt/cross/clang-20/bin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin /usr/bin/timeout -k 100 3h /usr/bin/make KCFLAGS=\ -fno-crash-diagnostics\ -Wno-error=return-type\ -Wreturn-type\ -funsigned-char\ -Wundef\ -falign-functions=64 W=1 --keep-going LLVM=1 -j32 ARCH=x86_64 versioncheck find ./* \( -name SCCS -o -name BitKeeper -o -name .svn -o -name CVS -o -name .pc -o -name .hg -o -name .git \) -prune -o \ -name '*.[hcS]' -type f -print | sort \ | xargs perl -w ./scripts/checkversion.pl >> ./drivers/clk/clkctrl-guard.c: 16 linux/version.h not needed. ./samples/bpf/spintest.bpf.c: 8 linux/version.h not needed. ./tools/lib/bpf/bpf_helpers.h: 446: need linux/version.h ./tools/testing/selftests/bpf/progs/dev_cgroup.c: 9 linux/version.h not needed. ./tools/testing/selftests/bpf/progs/netcnt_prog.c: 3 linux/version.h not needed. ./tools/testing/selftests/bpf/progs/test_map_lock.c: 4 linux/version.h not needed. ./tools/testing/selftests/bpf/progs/test_send_signal_kern.c: 4 linux/version.h not needed. ./tools/testing/selftests/bpf/progs/test_spin_lock.c: 4 linux/version.h not needed. ./tools/testing/selftests/bpf/progs/test_tcp_estats.c: 37 linux/version.h not needed. ./tools/testing/selftests/wireguard/qemu/init.c: 27 linux/version.h not needed. -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/2] dt-bindings: Add clock guard DT description 2026-03-18 17:43 [PATCH 0/2] A proposal to add a virtual clock controller guard Vyacheslav Yurkov via B4 Relay 2026-03-18 17:43 ` [PATCH 1/2] clk: Add " Vyacheslav Yurkov via B4 Relay @ 2026-03-18 17:43 ` Vyacheslav Yurkov via B4 Relay 2026-03-18 19:33 ` Rob Herring (Arm) 2026-03-18 22:55 ` Rob Herring 1 sibling, 2 replies; 17+ messages in thread From: Vyacheslav Yurkov via B4 Relay @ 2026-03-18 17:43 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-kernel, linux-clk, devicetree, Vyacheslav Yurkov, Vyacheslav Yurkov From: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com> Describe device tree binding for virtual clock controller guard. Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com> Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com> --- .../bindings/clock/clock-controller-guard.yaml | 79 ++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/Documentation/devicetree/bindings/clock/clock-controller-guard.yaml b/Documentation/devicetree/bindings/clock/clock-controller-guard.yaml new file mode 100644 index 000000000000..71c2d80de1f0 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/clock-controller-guard.yaml @@ -0,0 +1,79 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/clock/clock-controller-guard.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Clock Controller Guard + +maintainers: + - Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com> + +description: | + Clock controller that guards upstream clocks and/or GPIO + signals and exposes them as a single clock output. + +properties: + compatible: + const: clock-controller-guard + + "#clock-cells": + const: 1 + + clocks: + description: Input clocks that will be guarded. + minItems: 0 + + clock-names: Input clock names. + minItems: 0 + + clock-output-names: + description: Names of the clock provided by this controller. + minItems: 1 + items: + type: string + + gpios: + description: | + GPIOs used to control or guard the clocks. + minItems: 0 + maxItems: 32 + + gpio-names: + description: Names corresponding to each GPIO. + minItems: 0 + maxItems: 32 + + items: + type: string + +required: + - compatible + - "#clock-cells" +anyOf: + - required: + - clocks + - required: + - gpios +dependencies: + gpio-names: [gpios] + clock-names: [clocks] + +additionalProperties: false + +examples: + - | + clkctrl: clock-controller { + compatible = "clock-controller-guard"; + #clock-cells = <1>; + + clocks = <&clk0 0>, <&pll 0>; + + gpios = <&gpio0 4 GPIO_ACTIVE_HIGH>, + <&gpio0 5 GPIO_ACTIVE_HIGH>, + <&gpio1 2 GPIO_ACTIVE_LOW>; + + gpio-names = "gpio0", "gpio1", "gpio2"; + + clock-output-names = "clkout0"; + }; -- 2.34.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] dt-bindings: Add clock guard DT description 2026-03-18 17:43 ` [PATCH 2/2] dt-bindings: Add clock guard DT description Vyacheslav Yurkov via B4 Relay @ 2026-03-18 19:33 ` Rob Herring (Arm) 2026-03-18 22:55 ` Rob Herring 1 sibling, 0 replies; 17+ messages in thread From: Rob Herring (Arm) @ 2026-03-18 19:33 UTC (permalink / raw) To: Vyacheslav Yurkov Cc: Michael Turquette, linux-clk, Stephen Boyd, Vyacheslav Yurkov, devicetree, linux-kernel, Krzysztof Kozlowski, Conor Dooley On Wed, 18 Mar 2026 17:43:40 +0000, Vyacheslav Yurkov wrote: > Describe device tree binding for virtual clock controller guard. > > Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com> > Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com> > --- > .../bindings/clock/clock-controller-guard.yaml | 79 ++++++++++++++++++++++ > 1 file changed, 79 insertions(+) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: ./Documentation/devicetree/bindings/clock/clock-controller-guard.yaml:4:6: [error] string value is redundantly quoted with any quotes (quoted-strings) ./Documentation/devicetree/bindings/clock/clock-controller-guard.yaml:5:10: [error] string value is redundantly quoted with any quotes (quoted-strings) ./Documentation/devicetree/bindings/clock/clock-controller-guard.yaml:28:13: [error] syntax error: mapping values are not allowed here (syntax) dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/clock-controller-guard.yaml: ignoring, error parsing file ./Documentation/devicetree/bindings/clock/clock-controller-guard.yaml:28:13: mapping values are not allowed here make[2]: *** Deleting file 'Documentation/devicetree/bindings/clock/clock-controller-guard.example.dts' Documentation/devicetree/bindings/clock/clock-controller-guard.yaml:28:13: mapping values are not allowed here make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/clock/clock-controller-guard.example.dts] Error 1 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1601: dt_binding_check] Error 2 make: *** [Makefile:248: __sub-make] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.kernel.org/project/devicetree/patch/20260318-feature-clock-guard-v1-2-6137cb4084b7@bruker.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] dt-bindings: Add clock guard DT description 2026-03-18 17:43 ` [PATCH 2/2] dt-bindings: Add clock guard DT description Vyacheslav Yurkov via B4 Relay 2026-03-18 19:33 ` Rob Herring (Arm) @ 2026-03-18 22:55 ` Rob Herring 2026-03-19 5:50 ` Vyacheslav Yurkov 1 sibling, 1 reply; 17+ messages in thread From: Rob Herring @ 2026-03-18 22:55 UTC (permalink / raw) To: Vyacheslav Yurkov Cc: Michael Turquette, Stephen Boyd, Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-clk, devicetree, Vyacheslav Yurkov On Wed, Mar 18, 2026 at 05:43:40PM +0000, Vyacheslav Yurkov wrote: > Describe device tree binding for virtual clock controller guard. No idea what this means. Please explain how I would identify this h/w. We generally don't do bindings for virtual devices and we don't do single clock bindings (other than some we are stuck with). > > Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com> > Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com> > --- > .../bindings/clock/clock-controller-guard.yaml | 79 ++++++++++++++++++++++ > 1 file changed, 79 insertions(+) > > diff --git a/Documentation/devicetree/bindings/clock/clock-controller-guard.yaml b/Documentation/devicetree/bindings/clock/clock-controller-guard.yaml > new file mode 100644 > index 000000000000..71c2d80de1f0 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/clock-controller-guard.yaml > @@ -0,0 +1,79 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/clock/clock-controller-guard.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > + > +title: Clock Controller Guard > + > +maintainers: > + - Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com> > + > +description: | > + Clock controller that guards upstream clocks and/or GPIO > + signals and exposes them as a single clock output. > + > +properties: > + compatible: > + const: clock-controller-guard > + > + "#clock-cells": > + const: 1 > + > + clocks: > + description: Input clocks that will be guarded. > + minItems: 0 > + > + clock-names: Input clock names. > + minItems: 0 > + > + clock-output-names: > + description: Names of the clock provided by this controller. > + minItems: 1 > + items: > + type: string > + > + gpios: > + description: | > + GPIOs used to control or guard the clocks. > + minItems: 0 > + maxItems: 32 > + > + gpio-names: > + description: Names corresponding to each GPIO. > + minItems: 0 > + maxItems: 32 > + > + items: > + type: string > + > +required: > + - compatible > + - "#clock-cells" > +anyOf: > + - required: > + - clocks > + - required: > + - gpios > +dependencies: > + gpio-names: [gpios] > + clock-names: [clocks] > + > +additionalProperties: false > + > +examples: > + - | > + clkctrl: clock-controller { > + compatible = "clock-controller-guard"; > + #clock-cells = <1>; > + > + clocks = <&clk0 0>, <&pll 0>; > + > + gpios = <&gpio0 4 GPIO_ACTIVE_HIGH>, > + <&gpio0 5 GPIO_ACTIVE_HIGH>, > + <&gpio1 2 GPIO_ACTIVE_LOW>; > + > + gpio-names = "gpio0", "gpio1", "gpio2"; > + > + clock-output-names = "clkout0"; > + }; > > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] dt-bindings: Add clock guard DT description 2026-03-18 22:55 ` Rob Herring @ 2026-03-19 5:50 ` Vyacheslav Yurkov 2026-03-19 16:50 ` Conor Dooley 0 siblings, 1 reply; 17+ messages in thread From: Vyacheslav Yurkov @ 2026-03-19 5:50 UTC (permalink / raw) To: Rob Herring, Vyacheslav Yurkov Cc: Michael Turquette, Stephen Boyd, Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-clk, devicetree On 18.03.2026 23:55, Rob Herring wrote: > On Wed, Mar 18, 2026 at 05:43:40PM +0000, Vyacheslav Yurkov wrote: >> Describe device tree binding for virtual clock controller guard. > No idea what this means. Please explain how I would identify this h/w. > > We generally don't do bindings for virtual devices and we don't do > single clock bindings (other than some we are stuck with). > I described a use case in my cover letter (PATCH 0). Perhaps our approach to tackle the issue is not correct in the first place. The term "virtual clock controller guard" is something we named it, but it's literally just a clock provider which combines several other clocks and input GPIO signals in order for the consumers to check whether they are allowed to probe already or have to wait until the input clocks are enabled. So in essence it's like a helper driver to simplify consumers probe procedure. Does it make sense? If you don't do bindings for virtual HW, how else would you approach this? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] dt-bindings: Add clock guard DT description 2026-03-19 5:50 ` Vyacheslav Yurkov @ 2026-03-19 16:50 ` Conor Dooley 2026-03-23 13:52 ` Vyacheslav Yurkov 0 siblings, 1 reply; 17+ messages in thread From: Conor Dooley @ 2026-03-19 16:50 UTC (permalink / raw) To: Vyacheslav Yurkov Cc: Rob Herring, Vyacheslav Yurkov, Michael Turquette, Stephen Boyd, Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-clk, devicetree [-- Attachment #1: Type: text/plain, Size: 1275 bytes --] On Thu, Mar 19, 2026 at 06:50:52AM +0100, Vyacheslav Yurkov wrote: > On 18.03.2026 23:55, Rob Herring wrote: > > On Wed, Mar 18, 2026 at 05:43:40PM +0000, Vyacheslav Yurkov wrote: > > > Describe device tree binding for virtual clock controller guard. > > No idea what this means. Please explain how I would identify this h/w. > > > > We generally don't do bindings for virtual devices and we don't do > > single clock bindings (other than some we are stuck with). > > > > I described a use case in my cover letter (PATCH 0). Perhaps our approach to > tackle the issue is not correct in the first place. The term "virtual clock > controller guard" is something we named it, but it's literally just a clock > provider which combines several other clocks and input GPIO signals in order > for the consumers to check whether they are allowed to probe already or have > to wait until the input clocks are enabled. Can you explain how this is different to gpio-gate-clock? AFAICT, you're trying to support clocks that are enabled by a gpio, and that's what it is for. > > So in essence it's like a helper driver to simplify consumers probe > procedure. Does it make sense? If you don't do bindings for virtual HW, how > else would you approach this? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] dt-bindings: Add clock guard DT description 2026-03-19 16:50 ` Conor Dooley @ 2026-03-23 13:52 ` Vyacheslav Yurkov 2026-03-23 20:14 ` Conor Dooley 0 siblings, 1 reply; 17+ messages in thread From: Vyacheslav Yurkov @ 2026-03-23 13:52 UTC (permalink / raw) To: Conor Dooley Cc: Rob Herring, Vyacheslav Yurkov, Michael Turquette, Stephen Boyd, Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-clk, devicetree On 19.03.2026 17:50, Conor Dooley wrote: >> I described a use case in my cover letter (PATCH 0). Perhaps our approach to >> tackle the issue is not correct in the first place. The term "virtual clock >> controller guard" is something we named it, but it's literally just a clock >> provider which combines several other clocks and input GPIO signals in order >> for the consumers to check whether they are allowed to probe already or have >> to wait until the input clocks are enabled. > > Can you explain how this is different to gpio-gate-clock? AFAICT, you're > trying to support clocks that are enabled by a gpio, and that's what it > is for. > It partially covers the similar use case, but differs in the sense that gpio-gate-clock controls the clock via GPIO (enable/disable), the clock-controller-guard gets the GPIO status signals whether the clock _was_ enabled externally because a CPU has no direct access to the clock. So perhaps the terminology I came up with is not so self-explanatory, that's why I posted it for review and other opinions. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] dt-bindings: Add clock guard DT description 2026-03-23 13:52 ` Vyacheslav Yurkov @ 2026-03-23 20:14 ` Conor Dooley 2026-03-26 9:54 ` Vyacheslav Yurkov 0 siblings, 1 reply; 17+ messages in thread From: Conor Dooley @ 2026-03-23 20:14 UTC (permalink / raw) To: Vyacheslav Yurkov Cc: Rob Herring, Vyacheslav Yurkov, Michael Turquette, Stephen Boyd, Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-clk, devicetree [-- Attachment #1: Type: text/plain, Size: 2282 bytes --] On Mon, Mar 23, 2026 at 02:52:21PM +0100, Vyacheslav Yurkov wrote: > On 19.03.2026 17:50, Conor Dooley wrote: > > > > I described a use case in my cover letter (PATCH 0). Perhaps our approach to > > > tackle the issue is not correct in the first place. The term "virtual clock > > > controller guard" is something we named it, but it's literally just a clock > > > provider which combines several other clocks and input GPIO signals in order > > > for the consumers to check whether they are allowed to probe already or have > > > to wait until the input clocks are enabled. > > > > Can you explain how this is different to gpio-gate-clock? AFAICT, you're > > trying to support clocks that are enabled by a gpio, and that's what it > > is for. > > > It partially covers the similar use case, but differs in the sense that > gpio-gate-clock controls the clock via GPIO (enable/disable), the > clock-controller-guard gets the GPIO status signals whether the clock _was_ > enabled externally because a CPU has no direct access to the clock. So > perhaps the terminology I came up with is not so self-explanatory, that's > why I posted it for review and other opinions. The binding you've got says "GPIOs used to control or guard the clocks", which is not what you're saying that is going on in this mail. A more suitable description would be "GPIOs used to check the status of the clocks". I want to see an example dts user for this please. TBH, I don't understand your driver implementation either and why it has +static const struct clk_ops clkctrl_guard_ops = { + .enable = clkctrl_guard_enable, + .disable = clkctrl_guard_disable, + .prepare = clkctrl_guard_prepare, + .unprepare = clkctrl_guard_unprepare, + .is_prepared = clkctrl_guard_is_prepared, any of these 4 implemented when you have no control over the clock. I didn't think it was required to call your parent clocks enables in your own enable either, thought that was handled by the core recursively calling clk_enable() on clk->parent. The one thing I would expect you to have implemented ops wise is is_enabled, which you don't have. Also no sign of any rate acquisition functions, which I thought were mandatory. + .get_parent = clkctrl_guard_get_parent, +}; [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] dt-bindings: Add clock guard DT description 2026-03-23 20:14 ` Conor Dooley @ 2026-03-26 9:54 ` Vyacheslav Yurkov 2026-03-26 10:08 ` Krzysztof Kozlowski 2026-03-26 10:44 ` Conor Dooley 0 siblings, 2 replies; 17+ messages in thread From: Vyacheslav Yurkov @ 2026-03-26 9:54 UTC (permalink / raw) To: Conor Dooley Cc: Rob Herring, Vyacheslav Yurkov, Michael Turquette, Stephen Boyd, Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-clk, devicetree On 23.03.2026 21:14, Conor Dooley wrote: > > The binding you've got says "GPIOs used to control or guard the clocks", > which is not what you're saying that is going on in this mail. A more > suitable description would be "GPIOs used to check the status of the > clocks". Agree, the description I provided is not very accurate. > I want to see an example dts user for this please. DTS example: clock_guard: clock_controller_guard { compatible = "clock-controller-guard"; #clock-cells = <1>; clocks = <&h2f_clk 0>, <&clk_fgpa_rx 0>, <clk_fpga_tx 0>; clock-names = "h2f_clk0", "clk_fpga_rx", "clk_fpga_tx"; gpios = <&fpga_ip 0 GPIO_ACTIVE_HIGH>, <&fpga_ip 1 GPIO_ACTIVE_HIGH>; gpio-names = "gpio-input0", "gpio-input1"; clock-output-names = "clkctrl-guard"; }; custom_device { compatible = "..."; ... #clock-cells = <1>; clocks = <&clock_guard 0>; clock-names = "clock-guard"; }; The driver usage exaple: clk = devm_clk_get(dev, "clock-guard"); if (IS_ERR(clk)) return dev_err_probe(dev, PTR_ERR(clk), "failed to get clock\n"); ret = clk_prepare_enable(clk); if (ret) { dev_warn(dev, "Clock is not ready, %d\n", ret); return -EPROBE_DEFER; } > TBH, I don't understand your driver implementation either and why it has > > +static const struct clk_ops clkctrl_guard_ops = { > > + .enable = clkctrl_guard_enable, > + .disable = clkctrl_guard_disable, > + .prepare = clkctrl_guard_prepare, > + .unprepare = clkctrl_guard_unprepare, > + .is_prepared = clkctrl_guard_is_prepared, > > any of these 4 implemented when you have no control over the clock. > I didn't think it was required to call your parent clocks enables in > your own enable either, thought that was handled by the core recursively > calling clk_enable() on clk->parent. The one thing I would expect you to > have implemented ops wise is is_enabled, which you don't have. > Also no sign of any rate acquisition functions, which I thought were > mandatory. > > + .get_parent = clkctrl_guard_get_parent, > +}; Good point on .is_enabled, I indeed missed that. As for the rate acquisition functions I referred to this table https://docs.kernel.org/driver-api/clk.html#id4 , and it see that .set_rate is actually optional. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] dt-bindings: Add clock guard DT description 2026-03-26 9:54 ` Vyacheslav Yurkov @ 2026-03-26 10:08 ` Krzysztof Kozlowski 2026-03-26 13:39 ` Vyacheslav Yurkov 2026-03-26 10:44 ` Conor Dooley 1 sibling, 1 reply; 17+ messages in thread From: Krzysztof Kozlowski @ 2026-03-26 10:08 UTC (permalink / raw) To: Vyacheslav Yurkov, Conor Dooley Cc: Rob Herring, Vyacheslav Yurkov, Michael Turquette, Stephen Boyd, Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-clk, devicetree On 26/03/2026 10:54, Vyacheslav Yurkov wrote: > On 23.03.2026 21:14, Conor Dooley wrote: > >> >> The binding you've got says "GPIOs used to control or guard the clocks", >> which is not what you're saying that is going on in this mail. A more >> suitable description would be "GPIOs used to check the status of the >> clocks". > > Agree, the description I provided is not very accurate. > >> I want to see an example dts user for this please. > > DTS example: > clock_guard: clock_controller_guard { > compatible = "clock-controller-guard"; > #clock-cells = <1>; > clocks = <&h2f_clk 0>, <&clk_fgpa_rx 0>, <clk_fpga_tx 0>; > clock-names = "h2f_clk0", "clk_fpga_rx", "clk_fpga_tx"; > gpios = <&fpga_ip 0 GPIO_ACTIVE_HIGH>, <&fpga_ip 1 GPIO_ACTIVE_HIGH>; > gpio-names = "gpio-input0", "gpio-input1"; > clock-output-names = "clkctrl-guard"; > }; > > custom_device { > compatible = "..."; > ... > #clock-cells = <1>; > clocks = <&clock_guard 0>; > clock-names = "clock-guard"; > }; So a pure SW construct? Device has specific clock inputs but you do not model them and instead replace with one fake-guard-input. I don't see how this represents the hardware at all. Maybe some diagrams would help, assuming we still talk about hardware. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] dt-bindings: Add clock guard DT description 2026-03-26 10:08 ` Krzysztof Kozlowski @ 2026-03-26 13:39 ` Vyacheslav Yurkov 2026-03-26 13:49 ` Krzysztof Kozlowski 2026-03-26 18:32 ` Conor Dooley 0 siblings, 2 replies; 17+ messages in thread From: Vyacheslav Yurkov @ 2026-03-26 13:39 UTC (permalink / raw) To: Krzysztof Kozlowski, Conor Dooley Cc: Rob Herring, Vyacheslav Yurkov, Michael Turquette, Stephen Boyd, Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-clk, devicetree On 26.03.2026 11:08, Krzysztof Kozlowski wrote: >> >> DTS example: >> clock_guard: clock_controller_guard { >> compatible = "clock-controller-guard"; >> #clock-cells = <1>; >> clocks = <&h2f_clk 0>, <&clk_fgpa_rx 0>, <clk_fpga_tx 0>; >> clock-names = "h2f_clk0", "clk_fpga_rx", "clk_fpga_tx"; >> gpios = <&fpga_ip 0 GPIO_ACTIVE_HIGH>, <&fpga_ip 1 GPIO_ACTIVE_HIGH>; >> gpio-names = "gpio-input0", "gpio-input1"; >> clock-output-names = "clkctrl-guard"; >> }; >> >> custom_device { >> compatible = "..."; >> ... >> #clock-cells = <1>; >> clocks = <&clock_guard 0>; >> clock-names = "clock-guard"; >> }; > > So a pure SW construct? Device has specific clock inputs but you do not > model them and instead replace with one fake-guard-input. > > I don't see how this represents the hardware at all. > > Maybe some diagrams would help, assuming we still talk about hardware. > > Best regards, > Krzysztof Techincally that's correct, it's a software construct. If this is not a right place to submit such a helper driver, I'd appreciate a hint what subsystem is the right one. I was not sure how to provide a diagram in the mailing list, so I posted in on Github https://github.com/OSS-Keepers/clock-controller-guard/issues/1 It is a driver which models dependencies for other drivers. These are soft or "indirect" dependencies, because we cannot access the FPGA unless the FPGA_PLL_locked, and GPIO is telling us we are good to go. Conor, I think this should answer your question as well. Thanks, Slava ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] dt-bindings: Add clock guard DT description 2026-03-26 13:39 ` Vyacheslav Yurkov @ 2026-03-26 13:49 ` Krzysztof Kozlowski 2026-03-26 18:32 ` Conor Dooley 1 sibling, 0 replies; 17+ messages in thread From: Krzysztof Kozlowski @ 2026-03-26 13:49 UTC (permalink / raw) To: Vyacheslav Yurkov, Conor Dooley Cc: Rob Herring, Vyacheslav Yurkov, Michael Turquette, Stephen Boyd, Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-clk, devicetree On 26/03/2026 14:39, Vyacheslav Yurkov wrote: > On 26.03.2026 11:08, Krzysztof Kozlowski wrote: > >>> >>> DTS example: >>> clock_guard: clock_controller_guard { >>> compatible = "clock-controller-guard"; >>> #clock-cells = <1>; >>> clocks = <&h2f_clk 0>, <&clk_fgpa_rx 0>, <clk_fpga_tx 0>; >>> clock-names = "h2f_clk0", "clk_fpga_rx", "clk_fpga_tx"; >>> gpios = <&fpga_ip 0 GPIO_ACTIVE_HIGH>, <&fpga_ip 1 GPIO_ACTIVE_HIGH>; >>> gpio-names = "gpio-input0", "gpio-input1"; >>> clock-output-names = "clkctrl-guard"; >>> }; >>> >>> custom_device { >>> compatible = "..."; >>> ... >>> #clock-cells = <1>; >>> clocks = <&clock_guard 0>; >>> clock-names = "clock-guard"; >>> }; >> >> So a pure SW construct? Device has specific clock inputs but you do not >> model them and instead replace with one fake-guard-input. >> >> I don't see how this represents the hardware at all. >> >> Maybe some diagrams would help, assuming we still talk about hardware. >> >> Best regards, >> Krzysztof > > Techincally that's correct, it's a software construct. If this is not a > right place to submit such a helper driver, I'd appreciate a hint what > subsystem is the right one. > > I was not sure how to provide a diagram in the mailing list, so I posted > in on Github https://github.com/OSS-Keepers/clock-controller-guard/issues/1 Diagram of hardware. We talk here about hardware. > > It is a driver which models dependencies for other drivers. These are > soft or "indirect" dependencies, because we cannot access the FPGA > unless the FPGA_PLL_locked, and GPIO is telling us we are good to go. DT is not for drivers. It's not about subsystem, but concept. DT describes hardware. Certain generic bindings which solve real and common hardware problems are accepted, but so far I miss what hardware problem are you solving here. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] dt-bindings: Add clock guard DT description 2026-03-26 13:39 ` Vyacheslav Yurkov 2026-03-26 13:49 ` Krzysztof Kozlowski @ 2026-03-26 18:32 ` Conor Dooley 2026-03-28 2:58 ` Vyacheslav Yurkov 1 sibling, 1 reply; 17+ messages in thread From: Conor Dooley @ 2026-03-26 18:32 UTC (permalink / raw) To: Vyacheslav Yurkov Cc: Krzysztof Kozlowski, Rob Herring, Vyacheslav Yurkov, Michael Turquette, Stephen Boyd, Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-clk, devicetree [-- Attachment #1: Type: text/plain, Size: 3243 bytes --] On Thu, Mar 26, 2026 at 02:39:22PM +0100, Vyacheslav Yurkov wrote: > On 26.03.2026 11:08, Krzysztof Kozlowski wrote: > > > > > > > DTS example: > > > clock_guard: clock_controller_guard { > > > compatible = "clock-controller-guard"; > > > #clock-cells = <1>; > > > clocks = <&h2f_clk 0>, <&clk_fgpa_rx 0>, <clk_fpga_tx 0>; > > > clock-names = "h2f_clk0", "clk_fpga_rx", "clk_fpga_tx"; > > > gpios = <&fpga_ip 0 GPIO_ACTIVE_HIGH>, <&fpga_ip 1 GPIO_ACTIVE_HIGH>; > > > gpio-names = "gpio-input0", "gpio-input1"; > > > clock-output-names = "clkctrl-guard"; > > > }; > > > > > > custom_device { > > > compatible = "..."; > > > ... > > > #clock-cells = <1>; > > > clocks = <&clock_guard 0>; > > > clock-names = "clock-guard"; > > > }; > > > > So a pure SW construct? Device has specific clock inputs but you do not > > model them and instead replace with one fake-guard-input. > > > > I don't see how this represents the hardware at all. > > > > Maybe some diagrams would help, assuming we still talk about hardware. > > > > Best regards, > > Krzysztof > > Techincally that's correct, it's a software construct. If this is not a Is it a software construct? I assume that the PLL status is going to be some lock bit in a register, and you're got some "hardware" in your FPGA fabric that reads that bit and sets a GPIO when it gets locked? Or maybe it's even simpler, and your GPIO just gets set once your custom HDL comes out of reset, which happens when the PLL locks? If that's an approximation of what you have, that's not a software construct. > right place to submit such a helper driver, I'd appreciate a hint what > subsystem is the right one. > > I was not sure how to provide a diagram in the mailing list, so I posted in > on Github https://github.com/OSS-Keepers/clock-controller-guard/issues/1 > > It is a driver which models dependencies for other drivers. These are soft > or "indirect" dependencies, because we cannot access the FPGA unless the > FPGA_PLL_locked, and GPIO is telling us we are good to go. > > Conor, I think this should answer your question as well. Not really, but it gets part of the way there. I want to know what this provider actually is. I now know it is a PLL, not an off-chip oscillator, but I know nothing about the interface that you have to it (or if you have one at all). What compatible string/kernel driver does it use? Because SoC-FPGAs can route GPIOs from the SoC part to the FPGA fabric and use them as if interacting with something off-chip, I'm not sure if we are dealing with an separate FPGA or a SoC-FPGA. Which is it? Effectively I want to understand why you cannot just read the lock bit from the PLL directly. In my experience with *SoC*-FPGAs, things like PLLs that must lock for the fabric to be usable have a register interface from which the lock bit can be read, that is of course not clocked by the PLL output clock and therefore accessible before the PLL has locked. I think more info is needed here to guide you on where such a "helper driver" should be located and what the dt represetation should be. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] dt-bindings: Add clock guard DT description 2026-03-26 18:32 ` Conor Dooley @ 2026-03-28 2:58 ` Vyacheslav Yurkov 0 siblings, 0 replies; 17+ messages in thread From: Vyacheslav Yurkov @ 2026-03-28 2:58 UTC (permalink / raw) To: Conor Dooley Cc: Krzysztof Kozlowski, Rob Herring, Vyacheslav Yurkov, Michael Turquette, Stephen Boyd, Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-clk, devicetree On 26.03.2026 19:32, Conor Dooley wrote: >> I was not sure how to provide a diagram in the mailing list, so I posted in >> on Github https://github.com/OSS-Keepers/clock-controller-guard/issues/1 >> >> It is a driver which models dependencies for other drivers. These are soft >> or "indirect" dependencies, because we cannot access the FPGA unless the >> FPGA_PLL_locked, and GPIO is telling us we are good to go. >> >> Conor, I think this should answer your question as well. > > Not really, but it gets part of the way there. I want to know what this > provider actually is. I now know it is a PLL, not an off-chip > oscillator, but I know nothing about the interface that you have to it > (or if you have one at all). What compatible string/kernel driver does > it use? > > Because SoC-FPGAs can route GPIOs from the SoC part to the FPGA fabric > and use them as if interacting with something off-chip, I'm not sure if > we are dealing with an separate FPGA or a SoC-FPGA. Which is it? > Effectively I want to understand why you cannot just read the lock bit > from the PLL directly. In my experience with *SoC*-FPGAs, things like > PLLs that must lock for the fabric to be usable have a register > interface from which the lock bit can be read, that is of course not > clocked by the PLL output clock and therefore accessible before the > PLL has locked. > > I think more info is needed here to guide you on where such a "helper > driver" should be located and what the dt represetation should be. I really appreciate your feedback on this. Here's an attempt to provide a better exlanation. We have various use cases. Most of the time it's a PLL in the FPGA but it can also be some signal from a custom FPGA IP used to indicate if some preconditions are met and the IP is ready to be used (some kind of inverted reset but exposed by the IP). For a PLL we typically get the signal connected either to a GPIO IP block (altr,pio-1.0) OR to a bit in a custom IP register. In addition, some of the IPs in our design do not have a proper split between registers and IP core, which means that if an external clock and/or PLL lock is missing and we access the registers we won’t ever get an answer and thus stall the CPU. We are using a SoC-FPGA and use some GPIO IP within the FPGA (altr,pio-1.0 for example). The PLL itself doesn't have any registers but the signal indicating that it is locked is available and routed to such a GPIO. The point is that we will have several IPs/drivers that will depend on the same preconditions (clk, gpios being high or low) and we want to use this clk_guard driver as an aggregator for those pre-conditions. Define once, reuse a lot. Slava ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] dt-bindings: Add clock guard DT description 2026-03-26 9:54 ` Vyacheslav Yurkov 2026-03-26 10:08 ` Krzysztof Kozlowski @ 2026-03-26 10:44 ` Conor Dooley 1 sibling, 0 replies; 17+ messages in thread From: Conor Dooley @ 2026-03-26 10:44 UTC (permalink / raw) To: Vyacheslav Yurkov Cc: Rob Herring, Vyacheslav Yurkov, Michael Turquette, Stephen Boyd, Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-clk, devicetree [-- Attachment #1: Type: text/plain, Size: 3132 bytes --] On Thu, Mar 26, 2026 at 10:54:52AM +0100, Vyacheslav Yurkov wrote: > On 23.03.2026 21:14, Conor Dooley wrote: > > > > > The binding you've got says "GPIOs used to control or guard the clocks", > > which is not what you're saying that is going on in this mail. A more > > suitable description would be "GPIOs used to check the status of the > > clocks". > > Agree, the description I provided is not very accurate. > > > I want to see an example dts user for this please. > > DTS example: > clock_guard: clock_controller_guard { > compatible = "clock-controller-guard"; > #clock-cells = <1>; > clocks = <&h2f_clk 0>, <&clk_fgpa_rx 0>, <clk_fpga_tx 0>; Unfortunately, this doesn't contain the part that I wanted to see - who the providers of these clocks here actually are. To be frank, I am not sure how this block would know that these clocks are enabled but their providers do not. I can think of a few ideas for how this block would know, but I don't understand why the providers themselves don't, and therefore why you need this gpio to tell you. > clock-names = "h2f_clk0", "clk_fpga_rx", "clk_fpga_tx"; > gpios = <&fpga_ip 0 GPIO_ACTIVE_HIGH>, <&fpga_ip 1 GPIO_ACTIVE_HIGH>; > gpio-names = "gpio-input0", "gpio-input1"; > clock-output-names = "clkctrl-guard"; > }; > > custom_device { > compatible = "..."; > ... > #clock-cells = <1>; > clocks = <&clock_guard 0>; > clock-names = "clock-guard"; > }; > > The driver usage exaple: > > clk = devm_clk_get(dev, "clock-guard"); > if (IS_ERR(clk)) > return dev_err_probe(dev, PTR_ERR(clk), "failed to get clock\n"); > > ret = clk_prepare_enable(clk); > if (ret) { > dev_warn(dev, "Clock is not ready, %d\n", ret); > return -EPROBE_DEFER; > } > > > > TBH, I don't understand your driver implementation either and why it has > > > > +static const struct clk_ops clkctrl_guard_ops = { > > > > + .enable = clkctrl_guard_enable, > > + .disable = clkctrl_guard_disable, > > + .prepare = clkctrl_guard_prepare, > > + .unprepare = clkctrl_guard_unprepare, > > + .is_prepared = clkctrl_guard_is_prepared, > > > > any of these 4 implemented when you have no control over the clock. > > I didn't think it was required to call your parent clocks enables in > > your own enable either, thought that was handled by the core recursively > > calling clk_enable() on clk->parent. The one thing I would expect you to > > have implemented ops wise is is_enabled, which you don't have. > > Also no sign of any rate acquisition functions, which I thought were > > mandatory. > > > > + .get_parent = clkctrl_guard_get_parent, > > +}; > > Good point on .is_enabled, I indeed missed that. As for the rate acquisition > functions I referred to this table > https://docs.kernel.org/driver-api/clk.html#id4 , and it see that .set_rate > is actually optional. .set_rate is not rate acquisition. .round_rate and .determine_rate are. I thought they were mandatory, but for a gate clock I guess they are not and the parent rate gets used automatically. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2026-03-28 2:58 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-18 17:43 [PATCH 0/2] A proposal to add a virtual clock controller guard Vyacheslav Yurkov via B4 Relay 2026-03-18 17:43 ` [PATCH 1/2] clk: Add " Vyacheslav Yurkov via B4 Relay 2026-03-19 8:15 ` kernel test robot 2026-03-18 17:43 ` [PATCH 2/2] dt-bindings: Add clock guard DT description Vyacheslav Yurkov via B4 Relay 2026-03-18 19:33 ` Rob Herring (Arm) 2026-03-18 22:55 ` Rob Herring 2026-03-19 5:50 ` Vyacheslav Yurkov 2026-03-19 16:50 ` Conor Dooley 2026-03-23 13:52 ` Vyacheslav Yurkov 2026-03-23 20:14 ` Conor Dooley 2026-03-26 9:54 ` Vyacheslav Yurkov 2026-03-26 10:08 ` Krzysztof Kozlowski 2026-03-26 13:39 ` Vyacheslav Yurkov 2026-03-26 13:49 ` Krzysztof Kozlowski 2026-03-26 18:32 ` Conor Dooley 2026-03-28 2:58 ` Vyacheslav Yurkov 2026-03-26 10:44 ` Conor Dooley
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox