[PATCH v4 0/2] A proposal to add a gpio-locked fixed clock driver.

[PATCH v4 0/2] A proposal to add a gpio-locked fixed clock driver.

[Vyacheslav Yurkov via B4 Relay]

A gpio-locked fixed clock aggregates one or more input clocks and/or one
or more GPIOs. It's similar to a gated-fixed-clock, but GPIO direction is
inverted. Consumers can use the output clock to wait until all input
clocks are locked and only then initialize / access dependent peripherals.

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 check whether clock is actually usable. E.g. some of the IPs might not
have a proper split between registers and IP core, which means that if an
external clock and/or PLL lock is missing and one tries to access the 
registers, the response never comes, thus the CPU stalls.

Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com>
Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
---
Changes in v4:
- Removed driver specifics from DT binding
- Link to v3: https://lore.kernel.org/r/20260603-feature-clock-guard-v3-0-01cca0aa04a5@bruker.com

Changes in v3:
- Removed unnecessary dt bindings
- Improved HW description and commit messages
- Link to v2: https://lore.kernel.org/r/20260510-feature-clock-guard-v2-0-6c25458d5340@bruker.com

Changes in v2:
- Renamed to clk-gpio-locked to express intent.
- Provide enable() / is_enabled() operations so the clock behaves as
  expected
- Fixed DTS errors / warnings
- Link to v1: https://lore.kernel.org/r/20260318-feature-clock-guard-v1-0-6137cb4084b7@bruker.com

---
Vyacheslav Yurkov (2):
      dt-bindings: Add GPIO-locked fixed clock
      clk: Add gpio-locked fixed clock driver

 .../bindings/clock/gpio-locked-fixed-clock.yaml    |  59 ++++
 drivers/clk/Makefile                               |   1 +
 drivers/clk/clk-gpio-locked.c                      | 306 +++++++++++++++++++++
 3 files changed, 366 insertions(+)
---
base-commit: 3dab139d4795f688e4f243e40c7474df00d329d9
change-id: 20260318-feature-clock-guard-f20a2c35b965

Best regards,
-- 
Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>

[PATCH v4 1/2] dt-bindings: Add GPIO-locked fixed clock

[Vyacheslav Yurkov via B4 Relay]

From: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>

Some hardware designs provide fixed-frequency clocks generated outside
software control, such as by FPGA-resident PLLs. While the clock rate is
fixed, a separate GPIO signal indicates whether the clock source is
locked and producing a valid output.

Describe a GPIO-locked fixed clock provider that exposes a fixed-rate
clock whose availability depends on one or more GPIO lock-status
signals.

Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com>
Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
---
 .../bindings/clock/gpio-locked-fixed-clock.yaml    | 59 ++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml b/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml
new file mode 100644
index 000000000000..4d17889c51ad
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/gpio-locked-fixed-clock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GPIO Locked Fixed Clock
+
+maintainers:
+  - Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
+
+properties:
+  compatible:
+    const: gpio-locked-fixed-clock
+
+  "#clock-cells":
+    const: 0
+
+  clocks:
+    description: Input clocks whose validity is monitored by this provider.
+
+  clock-output-names:
+    description: Names of the clock provided by this controller.
+    maxItems: 1
+
+  locked-gpios:
+    description: |
+      GPIOs to check the lock state.
+    minItems: 1
+    maxItems: 32
+
+required:
+  - compatible
+  - "#clock-cells"
+
+anyOf:
+  - required:
+      - clocks
+  - required:
+      - locked-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    clk_gpio_locked: gpio-locked-fixed-clock {
+        compatible = "gpio-locked-fixed-clock";
+        #clock-cells = <0>;
+
+        clocks = <&clk0 0>, <&pll 0>;
+
+        locked-gpios = <&gpio0 4 GPIO_ACTIVE_HIGH>,
+                <&gpio0 5 GPIO_ACTIVE_HIGH>,
+                <&gpio1 2 GPIO_ACTIVE_LOW>;
+
+        clock-output-names = "clkout0";
+    };

-- 
2.34.1

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

[Vyacheslav Yurkov via B4 Relay]

From: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>

A gpio-locked clock aggregates one or more input clocks and/or one or
more GPIOs. It's a FPGA-assisted clocking design where peripheral clocks
are generated by FPGA PLLs that are outside CPU control, with
clock-valid/PLL-lock status exposed through GPIO signals.
Consumers can use the output clock to wait until all input
clocks are locked and only then initialize dependent peripherals.

Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com>
Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
---
 drivers/clk/Makefile          |   1 +
 drivers/clk/clk-gpio-locked.c | 306 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 307 insertions(+)

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index cc108a75a900..846383ed0358 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_COMMON_CLK)	+= clk-gpio-locked.o
 endif
 
 # KUnit specific helpers
diff --git a/drivers/clk/clk-gpio-locked.c b/drivers/clk/clk-gpio-locked.c
new file mode 100644
index 000000000000..79098f9b6532
--- /dev/null
+++ b/drivers/clk/clk-gpio-locked.c
@@ -0,0 +1,306 @@
+// 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>
+
+#define MAX_INPUT_GPIO_COUNT 32
+
+/**
+ * struct gpio_locked_clk_priv - private state for the whole driver
+ * @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 gpio_locked_clk_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_gpio_locked_clk_priv(_hw) \
+	container_of(_hw, struct gpio_locked_clk_priv, output_hw_clk)
+
+static int is_gpio_ready(struct gpio_locked_clk_priv *priv)
+{
+	unsigned long values[BITS_TO_LONGS(MAX_INPUT_GPIO_COUNT)] = {0};
+	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 gpio_locked_clk_is_enabled(struct clk_hw *hw)
+{
+	struct gpio_locked_clk_priv *priv = to_gpio_locked_clk_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_enabled(hw_clk)) {
+			dev_dbg(priv->dev, "Clock %i (%s) is not ready",
+				i, priv->clks[i].id);
+			return -EBUSY;
+		}
+	}
+
+	return 0;
+}
+
+/* We can't enable the clock, but the Common Clock Framework calls only
+ * enable() not is_enabled()
+ */
+static int gpio_locked_clk_enable(struct clk_hw *hw)
+{
+	return gpio_locked_clk_is_enabled(hw);
+}
+
+/* We have to implement it, but we are not going to control
+ * parent clock selection
+ */
+static u8 gpio_locked_clk_get_parent(struct clk_hw *hw)
+{
+	return 0;
+}
+
+static const struct clk_ops gpio_locked_clk_ops = {
+	.enable =	gpio_locked_clk_enable,
+	.is_enabled =	gpio_locked_clk_is_enabled,
+	.get_parent =	gpio_locked_clk_get_parent,
+};
+
+static int gpio_locked_clk_parse_inputs(struct gpio_locked_clk_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 gpio_locked_clk_parse_gpios(struct gpio_locked_clk_priv *priv)
+{
+	struct device *dev = priv->dev;
+	struct device_node *np = dev->of_node;
+	int i;
+
+	priv->gpios = devm_gpiod_get_array_optional(dev, "locked", 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 > MAX_INPUT_GPIO_COUNT) {
+		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 gpio_locked_clk_parse_outputs(struct gpio_locked_clk_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 = &gpio_locked_clk_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 gpio_locked_clk_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct gpio_locked_clk_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 = gpio_locked_clk_parse_inputs(priv);
+	if (ret)
+		return ret;
+
+	ret = gpio_locked_clk_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 = gpio_locked_clk_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 gpio_locked_clk_remove(struct platform_device *pdev)
+{
+	dev_dbg(&pdev->dev, "removed\n");
+}
+
+static const struct of_device_id gpio_locked_clk_of_match[] = {
+	{ .compatible = "gpio-locked-fixed-clock" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, gpio_locked_clk_of_match);
+
+static struct platform_driver gpio_locked_clk_driver = {
+	.probe  = gpio_locked_clk_probe,
+	.remove = gpio_locked_clk_remove,
+	.driver = {
+		.name           = "gpio-locked-fixed-clock",
+		.of_match_table = gpio_locked_clk_of_match,
+	},
+};
+module_platform_driver(gpio_locked_clk_driver);
+
+MODULE_AUTHOR("Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>");
+MODULE_DESCRIPTION("GPIO-locked clock driver");
+MODULE_LICENSE("Dual BSD/GPL");

-- 
2.34.1

Re: [PATCH v4 1/2] dt-bindings: Add GPIO-locked fixed clock

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The schema's `anyOf` constraint makes the essential `locked-gpios` property optional, and the binding is missing `clock-frequency`, breaking validation for fixed clocks.
--

--- Patch [1]: [PATCH v4 1/2] dt-bindings: Add GPIO-locked fixed clock ---
commit fe524126bff4b0741b4910f69b3229a3794694dd
Author: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>

dt-bindings: Add GPIO-locked fixed clock

This commit adds a device tree binding for a fixed-rate clock whose
availability depends on one or more GPIO lock-status signals. Describe a
GPIO-locked fixed clock provider that exposes a fixed-rate clock whose
availability is tied to a lock-status signal.

> diff --git a/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml b/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml
> new file mode 100644
> index 0000000000000..4d17889c51ad9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml
[ ... ]
> +properties:
> +  compatible:
> +    const: gpio-locked-fixed-clock
> +
> +  "#clock-cells":
> +    const: 0
> +

[Severity: Medium]
Is clock-frequency intentionally omitted from the properties list? Because
additionalProperties is set to false, a clock node that omits the clocks
property will have no valid way to specify its rate, which would cause
standalone fixed clocks to fail validation.

> +  clocks:
> +    description: Input clocks whose validity is monitored by this provider.
> +
> +  clock-output-names:
> +    description: Names of the clock provided by this controller.
> +    maxItems: 1
> +
> +  locked-gpios:
> +    description: |
> +      GPIOs to check the lock state.
> +    minItems: 1
> +    maxItems: 32
> +
> +required:
> +  - compatible
> +  - "#clock-cells"
> +
> +anyOf:
> +  - required:
> +      - clocks
> +  - required:
> +      - locked-gpios

[Severity: Medium]
Does this anyOf condition inadvertently make locked-gpios optional? If a node
specifies clocks but omits locked-gpios, the first branch is satisfied and
the schema will silently pass validation. This seems to allow configurations
that bypass the core purpose of a GPIO-locked clock provider.

> +
> +additionalProperties: false

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260726-feature-clock-guard-v4-0-e9c8b372b71c@bruker.com?part=1

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [Critical] The `.is_enabled` callback completely inverts the clock status logic, returning error codes instead of boolean state.
- [High] The driver attempts to aggregate multiple dependent input clocks by registering them as CCF parents, causing a TOCTOU race and failing to enable parents.
- [High] Using local DT connection IDs to populate `init.parent_names` causes global clock matching to fail and risks NULL pointer dereferences.
- [High] The `enable` callback returns `-EBUSY` immediately rather than waiting for the hardware lock, contradicting CCF semantics.
- [High] The driver requests the lock GPIOs with `GPIOD_ASIS` rather than `GPIOD_IN`, potentially failing to read the pin state correctly.
- [High] The driver fails to validate that provided GPIOs are non-sleeping, risking a kernel panic if slow GPIO controllers are used.
- [Medium] Calling `dev_warn` from a spinlocked, atomic context causes severe latencies.
--

--- Patch [2]: [PATCH v4 2/2] clk: Add gpio-locked fixed clock driver ---
commit 86eabe35b2750726ea6024147b3e2f51a67773ca
Author: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>

clk: Add gpio-locked fixed clock driver

This commit introduces a new clock driver that aggregates input clocks and
GPIOs to expose a clock-valid/PLL-lock status for FPGA-assisted clocking
designs.

> diff --git a/drivers/clk/clk-gpio-locked.c b/drivers/clk/clk-gpio-locked.c
> new file mode 100644
> index 0000000000000..79098f9b65326
> --- /dev/null
> +++ b/drivers/clk/clk-gpio-locked.c
> @@ -0,0 +1,306 @@
[ ... ]
> +static int is_gpio_ready(struct gpio_locked_clk_priv *priv)
> +{
> +	unsigned long values[BITS_TO_LONGS(MAX_INPUT_GPIO_COUNT)] = {0};
> +	int ret = 0;
[ ... ]
> +	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]);

[Severity: Medium]
Is it safe to call dev_warn() while holding the clock framework's enable_lock?

Because is_gpio_ready() is called from the .enable and .is_enabled callbacks,
it executes under a spinlock with hard interrupts disabled. Unconditionally
logging to the console from this context can cause severe IRQ latencies or
hard lockups, especially if a consumer polls the clock state.

> +			return -EBUSY;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int gpio_locked_clk_is_enabled(struct clk_hw *hw)
> +{
> +	struct gpio_locked_clk_priv *priv = to_gpio_locked_clk_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_enabled(hw_clk)) {
> +			dev_dbg(priv->dev, "Clock %i (%s) is not ready",
> +				i, priv->clks[i].id);
> +			return -EBUSY;
> +		}
> +	}
> +
> +	return 0;

[Severity: Critical]
Does this function invert the expected return logic for .is_enabled?

The clock framework expects .is_enabled to return a non-zero value when the
clock is enabled, and 0 when it is disabled. By returning -EBUSY when not
ready and 0 when ready, the framework will consider the clock enabled when
it is actually not ready, and disabled when it is ready.

> +}
> +
> +/* We can't enable the clock, but the Common Clock Framework calls only
> + * enable() not is_enabled()
> + */
> +static int gpio_locked_clk_enable(struct clk_hw *hw)
> +{
> +	return gpio_locked_clk_is_enabled(hw);

[Severity: High]
Should the enable callback block until the hardware lock is acquired?

Standard clock consumers do not poll clk_prepare_enable(). By doing a single
non-blocking check and failing immediately with -EBUSY, consumer driver probes
will fail if the hardware lock is not instantaneously ready at the moment of
enablement.

> +}
> +
> +/* We have to implement it, but we are not going to control
> + * parent clock selection
> + */
> +static u8 gpio_locked_clk_get_parent(struct clk_hw *hw)
> +{
> +	return 0;

[Severity: High]
Does hardcoding the parent to index 0 break the aggregation of multiple inputs?

When a clock is registered with num_parents > 1 and implements get_parent,
the clock framework treats it as a multiplexer where only one parent is active.

By always returning 0, the framework is told to abandon tracking for all other
parents. This means parents 1 through N will not be enabled when this clock
is enabled.

> +}
> +
> +static const struct clk_ops gpio_locked_clk_ops = {
> +	.enable =	gpio_locked_clk_enable,
> +	.is_enabled =	gpio_locked_clk_is_enabled,
> +	.get_parent =	gpio_locked_clk_get_parent,
> +};
[ ... ]
> +static int gpio_locked_clk_parse_gpios(struct gpio_locked_clk_priv *priv)
> +{
> +	struct device *dev = priv->dev;
> +	struct device_node *np = dev->of_node;
> +	int i;
> +
> +	priv->gpios = devm_gpiod_get_array_optional(dev, "locked", GPIOD_ASIS);

[Severity: High]
Could requesting the GPIO array with GPIOD_ASIS leave the pins in an undefined
state?

Without enforcing the input direction using GPIOD_IN, the hardware
configuration relies on the bootloader. If the bootloader leaves the pins
floating or configured as outputs, reading the pin state may fail to detect
the lock correctly.

[Severity: High]
Does the driver need to verify that these GPIOs are non-sleeping?

The clock framework's .enable and .is_enabled callbacks run in an atomic
context with interrupts disabled. If the device tree provides a sleeping GPIO
(such as an I2C or SPI expander), calling gpiod_get_array_value() from
is_gpio_ready() will trigger a scheduling-while-atomic BUG.

Could the probe function check !gpiod_cansleep() to reject sleeping GPIOs?

> +	if (IS_ERR(priv->gpios)) {
> +		dev_err(dev, "failed to get GPIO array: %ld\n",
> +				PTR_ERR(priv->gpios));
[ ... ]
> +static int gpio_locked_clk_parse_outputs(struct gpio_locked_clk_priv *priv)
> +{
> +	struct device *dev = priv->dev;
> +	struct device_node *np = dev->of_node;
> +	struct clk_init_data init = {};
> +	int ret;
[ ... ]
> +	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;

[Severity: High]
Does this assignment pass local device tree connection IDs to the clock
framework instead of globally registered provider names?

During probe, devm_clk_bulk_get_all() populates the id field with local
consumer names from the device tree. The clock framework expects
init.parent_names to contain global provider names. This mismatch will
cause the output clock to fail to match its parents.

Additionally, if the optional clock-names property is missing in the DT,
priv->clks[j].id will be NULL, causing a NULL pointer dereference when the
framework evaluates init.parent_names.

> +
> +		init.parent_names = parent_names;
> +	}
> +
> +	priv->output_hw_clk.init = &init;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260726-feature-clock-guard-v4-0-e9c8b372b71c@bruker.com?part=2

Re: [PATCH v4 1/2] dt-bindings: Add GPIO-locked fixed clock

[Rob Herring]

On Sun, Jul 26, 2026 at 05:40:24PM +0000, Vyacheslav Yurkov wrote:
> Some hardware designs provide fixed-frequency clocks generated outside
> software control, such as by FPGA-resident PLLs. While the clock rate is
> fixed, a separate GPIO signal indicates whether the clock source is
> locked and producing a valid output.
> 
> Describe a GPIO-locked fixed clock provider that exposes a fixed-rate
> clock whose availability depends on one or more GPIO lock-status
> signals.
> 
> Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com>

I think you can drop this. Just update your author email.

> Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
> ---
>  .../bindings/clock/gpio-locked-fixed-clock.yaml    | 59 ++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml b/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml
> new file mode 100644
> index 000000000000..4d17889c51ad
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/gpio-locked-fixed-clock.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: GPIO Locked Fixed Clock
> +
> +maintainers:
> +  - Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
> +
> +properties:
> +  compatible:
> +    const: gpio-locked-fixed-clock
> +
> +  "#clock-cells":
> +    const: 0
> +
> +  clocks:
> +    description: Input clocks whose validity is monitored by this provider.
> +
> +  clock-output-names:
> +    description: Names of the clock provided by this controller.
> +    maxItems: 1
> +
> +  locked-gpios:
> +    description: |

Don't need '|' if no formatting.

> +      GPIOs to check the lock state.
> +    minItems: 1
> +    maxItems: 32

Please explain better how multiple GPIOs work? Why would you ever have 
more than 1 lock signal per clock? And you can only have 1 clock given 
#clock-cells==0.

Do you have an immediate need for more than 1? If not, I'd drop 
support for it for now.

Rob

Re: [PATCH v4 1/2] dt-bindings: Add GPIO-locked fixed clock

[Rob Herring]

On Mon, Aug 10, 2026 at 11:40:00AM -0500, Rob Herring wrote:
> On Sun, Jul 26, 2026 at 05:40:24PM +0000, Vyacheslav Yurkov wrote:
> > Some hardware designs provide fixed-frequency clocks generated outside
> > software control, such as by FPGA-resident PLLs. While the clock rate is
> > fixed, a separate GPIO signal indicates whether the clock source is
> > locked and producing a valid output.
> > 
> > Describe a GPIO-locked fixed clock provider that exposes a fixed-rate
> > clock whose availability depends on one or more GPIO lock-status
> > signals.
> > 
> > Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com>
> 
> I think you can drop this. Just update your author email.
> 
> > Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
> > ---
> >  .../bindings/clock/gpio-locked-fixed-clock.yaml    | 59 ++++++++++++++++++++++
> >  1 file changed, 59 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml b/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml
> > new file mode 100644
> > index 000000000000..4d17889c51ad
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml
> > @@ -0,0 +1,59 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/gpio-locked-fixed-clock.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: GPIO Locked Fixed Clock
> > +
> > +maintainers:
> > +  - Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: gpio-locked-fixed-clock
> > +
> > +  "#clock-cells":
> > +    const: 0
> > +
> > +  clocks:
> > +    description: Input clocks whose validity is monitored by this provider.
> > +
> > +  clock-output-names:
> > +    description: Names of the clock provided by this controller.
> > +    maxItems: 1
> > +
> > +  locked-gpios:
> > +    description: |
> 
> Don't need '|' if no formatting.
> 
> > +      GPIOs to check the lock state.
> > +    minItems: 1
> > +    maxItems: 32
> 
> Please explain better how multiple GPIOs work? Why would you ever have 
> more than 1 lock signal per clock? And you can only have 1 clock given 
> #clock-cells==0.
> 
> Do you have an immediate need for more than 1? If not, I'd drop 
> support for it for now.

I missed that this is N input clocks and 1 output clock. But that leads 
to other questions. You've implemented a clock mux then? I still don't 
understand for what h/w that makes sense. Which input clock is selected? 
The locked one?

Rob

Re: [PATCH v4 1/2] dt-bindings: Add GPIO-locked fixed clock

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2985 bytes --]

On Mon, Aug 10, 2026 at 11:54:03AM -0500, Rob Herring wrote:
> On Mon, Aug 10, 2026 at 11:40:00AM -0500, Rob Herring wrote:
> > On Sun, Jul 26, 2026 at 05:40:24PM +0000, Vyacheslav Yurkov wrote:
> > > Some hardware designs provide fixed-frequency clocks generated outside
> > > software control, such as by FPGA-resident PLLs. While the clock rate is
> > > fixed, a separate GPIO signal indicates whether the clock source is
> > > locked and producing a valid output.
> > > 
> > > Describe a GPIO-locked fixed clock provider that exposes a fixed-rate
> > > clock whose availability depends on one or more GPIO lock-status
> > > signals.
> > > 
> > > Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com>
> > 
> > I think you can drop this. Just update your author email.
> > 
> > > Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
> > > ---
> > >  .../bindings/clock/gpio-locked-fixed-clock.yaml    | 59 ++++++++++++++++++++++
> > >  1 file changed, 59 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml b/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml
> > > new file mode 100644
> > > index 000000000000..4d17889c51ad
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml
> > > @@ -0,0 +1,59 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/clock/gpio-locked-fixed-clock.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: GPIO Locked Fixed Clock
> > > +
> > > +maintainers:
> > > +  - Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: gpio-locked-fixed-clock
> > > +
> > > +  "#clock-cells":
> > > +    const: 0
> > > +
> > > +  clocks:
> > > +    description: Input clocks whose validity is monitored by this provider.
> > > +
> > > +  clock-output-names:
> > > +    description: Names of the clock provided by this controller.
> > > +    maxItems: 1
> > > +
> > > +  locked-gpios:
> > > +    description: |
> > 
> > Don't need '|' if no formatting.
> > 
> > > +      GPIOs to check the lock state.
> > > +    minItems: 1
> > > +    maxItems: 32
> > 
> > Please explain better how multiple GPIOs work? Why would you ever have 
> > more than 1 lock signal per clock? And you can only have 1 clock given 
> > #clock-cells==0.
> > 
> > Do you have an immediate need for more than 1? If not, I'd drop 
> > support for it for now.
> 
> I missed that this is N input clocks and 1 output clock. But that leads 
> to other questions. You've implemented a clock mux then? I still don't 
> understand for what h/w that makes sense. Which input clock is selected? 
> The locked one?

Yeah, I thought this was n inputs and n outputs, with each gpio
signalling that an individual PLL had locked.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 1/2] dt-bindings: Add GPIO-locked fixed clock

[Vyacheslav Yurkov]

On 10.08.2026 18:58, Conor Dooley wrote:
> On Mon, Aug 10, 2026 at 11:54:03AM -0500, Rob Herring wrote:
>> On Mon, Aug 10, 2026 at 11:40:00AM -0500, Rob Herring wrote:
>>
>> I missed that this is N input clocks and 1 output clock. But that leads
>> to other questions. You've implemented a clock mux then? I still don't
>> understand for what h/w that makes sense. Which input clock is selected?
>> The locked one?
> 
> Yeah, I thought this was n inputs and n outputs, with each gpio
> signalling that an individual PLL had locked.


It is n input clocks and 1 output clock. It is kind of a mux, but the 
CPU doesn't control the clocks or GPIO signals. The whole idea is that 
peripherals check the output clock, when it's locked that means _all_ 
the clocks are locked and GPIOs are in expected state. That's why the 
selection operation is not really implemented.

Actually the number of input clocks don't have to correspond to the 
number of the GPIOs, because the GPIO signals indicate the locked state 
of the clocks that are not accessible to the CPU.

Slava

Re: [PATCH v4 1/2] dt-bindings: Add GPIO-locked fixed clock

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2158 bytes --]

On Thu, Aug 27, 2026 at 10:23:20AM +0200, Vyacheslav Yurkov wrote:
> On 10.08.2026 18:58, Conor Dooley wrote:
> > On Mon, Aug 10, 2026 at 11:54:03AM -0500, Rob Herring wrote:
> > > On Mon, Aug 10, 2026 at 11:40:00AM -0500, Rob Herring wrote:
> > > 
> > > I missed that this is N input clocks and 1 output clock. But that leads
> > > to other questions. You've implemented a clock mux then? I still don't
> > > understand for what h/w that makes sense. Which input clock is selected?
> > > The locked one?
> > 
> > Yeah, I thought this was n inputs and n outputs, with each gpio
> > signalling that an individual PLL had locked.
> 
> 
> It is n input clocks and 1 output clock. It is kind of a mux, but the CPU
> doesn't control the clocks or GPIO signals. The whole idea is that
> peripherals check the output clock, when it's locked that means _all_ the
> clocks are locked and GPIOs are in expected state. That's why the selection
> operation is not really implemented.
> 
> Actually the number of input clocks don't have to correspond to the number
> of the GPIOs, because the GPIO signals indicate the locked state of the
> clocks that are not accessible to the CPU.

I'm not entirely sure what you mean by this, but it is starting to sound
like you're only having one output because that's the minimum you need to do
to ensure that this driver has probed before the peripheral(s) using the
N input clocks. Requiring other input clocks to be stable before
declaring the input that's actually connected to the output stable
appears to be a shortcut/hack rather than an accurate description of the
hardware. If that's the case, I'd be much happier with this if this was
implemented as either a) N inputs with N gpios and N outputs, or b) 1 input,
M gpios (if multiple represent the stability of that input) and 1 output,
with N instances, one for each clock.
On the other hand, if this is genuinely a mux, then the binding should
reflect that, rather than only describe a subset of what you can do and
the driver should only check the actual parent out the output, rather
than the N-1 other inputs.

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 1/2] dt-bindings: Add GPIO-locked fixed clock

[Vyacheslav Yurkov]

On 27.08.2026 18:55, Conor Dooley wrote:

>> It is n input clocks and 1 output clock. It is kind of a mux, but the CPU
>> doesn't control the clocks or GPIO signals. The whole idea is that
>> peripherals check the output clock, when it's locked that means _all_ the
>> clocks are locked and GPIOs are in expected state. That's why the selection
>> operation is not really implemented.
>>
>> Actually the number of input clocks don't have to correspond to the number
>> of the GPIOs, because the GPIO signals indicate the locked state of the
>> clocks that are not accessible to the CPU.
> 
> I'm not entirely sure what you mean by this, but it is starting to sound
> like you're only having one output because that's the minimum you need to do
> to ensure that this driver has probed before the peripheral(s) using the
> N input clocks.

That's exactly the idea. How else I would ensure in peripheral's probe 
driver that clocks are locked?

> Requiring other input clocks to be stable before
> declaring the input that's actually connected to the output stable
> appears to be a shortcut/hack rather than an accurate description of the
> hardware. If that's the case, I'd be much happier with this if this was
> implemented as either a) N inputs with N gpios and N outputs, or b) 1 input,
> M gpios (if multiple represent the stability of that input) and 1 output,
> with N instances, one for each clock.

How having N outputs would help? It looks like it would just move the 
job I do here into each peripheral driver instead.

> On the other hand, if this is genuinely a mux, then the binding should
> reflect that, rather than only describe a subset of what you can do and
> the driver should only check the actual parent out the output, rather
> than the N-1 other inputs.
> 
> Thanks,
> Conor.


Slava

Re: [PATCH v4 1/2] dt-bindings: Add GPIO-locked fixed clock

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4558 bytes --]

On Fri, Aug 28, 2026 at 07:31:10AM +0200, Vyacheslav Yurkov wrote:
> On 27.08.2026 18:55, Conor Dooley wrote:
> 
> > > It is n input clocks and 1 output clock. It is kind of a mux, but the CPU
> > > doesn't control the clocks or GPIO signals. The whole idea is that
> > > peripherals check the output clock, when it's locked that means _all_ the
> > > clocks are locked and GPIOs are in expected state. That's why the selection
> > > operation is not really implemented.
> > > 
> > > Actually the number of input clocks don't have to correspond to the number
> > > of the GPIOs, because the GPIO signals indicate the locked state of the
> > > clocks that are not accessible to the CPU.
> > 
> > I'm not entirely sure what you mean by this, but it is starting to sound
> > like you're only having one output because that's the minimum you need to do
> > to ensure that this driver has probed before the peripheral(s) using the
> > N input clocks.
> 
> That's exactly the idea. How else I would ensure in peripheral's probe
> driver that clocks are locked?

Every peripheral should already contain a reference to all of the clocks
that are connected to it, and the peripheral drivers should request
those clocks. If you do that for all clocks a peripheral uses, you don't
need to have an artificial check for the gpios representing clocks
b,c,d,e,f,g... in the enable function for clock a, because their own
{devm,}clk_get_enabled() will interrogate the relevant bit.

At present, I'm expecting that you have a dt like

clk_gpio_locked: gpio-locked-fixed-clock {
    compatible = "gpio-locked-fixed-clock";
    #clock-cells = <0>;

    clocks = <&clk0 0>, <&pll 0>;

    locked-gpios = <&gpio0 4 GPIO_ACTIVE_HIGH>,
            <&gpio0 5 GPIO_ACTIVE_HIGH>,
            <&gpio1 2 GPIO_ACTIVE_LOW>;

    clock-output-names = "clkout0";
};

peripheral {
	clocks = <&clk_gpio_locked>, <&clk0 0>, <&pll 0>;
	//or this, depending on whether clkout0 actually produces a
	//rate, which I guess it does given it's a passthrough for the
	//first clocks entry?
	clocks = <&clk_gpio_locked>, <&pll 0>;
};

when really you should have something like

clk_gpio_locked: gpio-locked-fixed-clock {
    compatible = "gpio-locked-fixed-clock";
    #clock-cells = <1>;

    clocks = <&clk0 0>, <&pll 0>, <more clocks here>;

    locked-gpios = <&gpio0 4 GPIO_ACTIVE_HIGH>,
            <&gpio0 5 GPIO_ACTIVE_HIGH>,
            <&gpio1 2 GPIO_ACTIVE_LOW>;

    clock-output-names = "clkout0", "clkout1", "more clocks here";
};

peripheral {
	clocks = <&clk_gpio_locked 0>, <&clk_gpio_locked 1>;
};

> > Requiring other input clocks to be stable before
> > declaring the input that's actually connected to the output stable
> > appears to be a shortcut/hack rather than an accurate description of the
> > hardware. If that's the case, I'd be much happier with this if this was
> > implemented as either a) N inputs with N gpios and N outputs, or b) 1 input,
> > M gpios (if multiple represent the stability of that input) and 1 output,
> > with N instances, one for each clock.
> 
> How having N outputs would help? It looks like it would just move the job I
> do here into each peripheral driver instead.

Two reasons. Firstly because what you've got here suits you just fine,
but since you're adding it to a generic binding it should be written in
the most generic manner possible. Some other similar use case might not
want to wait for clocks used by other peripherals to lock before
allowing the driver for what they do care about to probe.
Secondly because it's a bit of a hack currently, you've got one clock
representing the status for a bunch of other clocks. Each clock should
represent it's own status. If this was done in the hardware it'd be one
thing, but you've got a pure software construct blocking one clock from
being locked because other clocks that may be unrelated to it entirely
are not yet locked.
The argument for including this depends on it not being a pure software
construct, and having what appears to be a hack so that you can rely on
the status of one clock rather than correctly determining the locked
state of individual clocks undermines your own argument for inclusion.

Cheers,
Conor.

> > On the other hand, if this is genuinely a mux, then the binding should
> > reflect that, rather than only describe a subset of what you can do and
> > the driver should only check the actual parent out the output, rather
> > than the N-1 other inputs.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]