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

[PATCH v3 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 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    |  70 +++++
 drivers/clk/Makefile                               |   1 +
 drivers/clk/clk-gpio-locked.c                      | 306 +++++++++++++++++++++
 3 files changed, 377 insertions(+)
---
base-commit: ba3e43a9e601636f5edb54e259a74f96ca3b8fd8
change-id: 20260318-feature-clock-guard-f20a2c35b965

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

[PATCH v3 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    | 70 ++++++++++++++++++++++
 1 file changed, 70 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..9106b800b673
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml
@@ -0,0 +1,70 @@
+# 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>
+
+description: |
+  Provides a clock output whose availability depends on a set of
+  prerequisite conditions. These conditions include the presence of
+  one or more parent clocks and the asserted state of one or more
+  GPIO lock indicators. An example of such clocks is FPGA clock that
+  are outside CPU control, with the lock status exposed through GPIO
+  signal.
+
+  The output clock is considered available only when all configured
+  prerequisites are satisfied.
+
+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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] The driver fails to probe with `-EINVAL` because it incorrectly uses `devm_clk_bulk_get_all` IDs as CCF `parent_names`.
- [High] The driver registers multiple required input clocks as CCF parents, causing `clk_enable()` to permanently fail.
- [High] The binding lacks a `clock-frequency` property, resulting in a 0 Hz clock when no parents are specified.
- [High] The .is_enabled callback incorrectly returns 0 when the clock is ready and a negative error code when it is not.
- [Medium] The driver reads the 'gpio-names' property which is missing from the dt-binding schema.
- [Low] The `clocks` property is missing an array size constraint (`maxItems` or `items`).
- [Low] The example uses a device-specific node name instead of a generic node name.
--

commit f442ab0fb77267ab8b660fa86301ab0aa027e186
Author: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
Subject: dt-bindings: Add GPIO-locked fixed clock

This commit introduces a new device tree binding and corresponding driver
support for a GPIO-locked fixed clock provider. The clock's availability
depends on prerequisite conditions, including parent clocks and GPIO lock
indicators. This allows modeling FPGA-generated clocks that expose lock
status through GPIOs.

> 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
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml
> @@ -0,0 +1,70 @@
[ ... ]
> +properties:
> +  compatible:
> +    const: gpio-locked-fixed-clock
> +
> +  "#clock-cells":
> +    const: 0
> +
> +  clocks:
> +    description: Input clocks whose validity is monitored by this provider.

[Severity: Low]
Does the clocks property need an array size constraint such as maxItems
or items? Without it, dt_binding_check might emit a meta-schema validation
error.

[ ... ]
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    clk_gpio_locked: gpio-locked-fixed-clock {

[Severity: Low]
Is gpio-locked-fixed-clock the appropriate node name here? Standard device
tree rules typically require generic node names (e.g., clock-controller { or
clock {) rather than device-specific names.

> +        compatible = "gpio-locked-fixed-clock";
[ ... ]
> diff --git a/drivers/clk/clk-gpio-locked.c b/drivers/clk/clk-gpio-locked.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/clk/clk-gpio-locked.c
> @@ -0,0 +1,306 @@
[ ... ]
> +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: High]
Does .is_enabled correctly return the clock status here?

The Common Clock Framework expects this callback to return a non-zero value
(true) when the clock is ready, and 0 (false) when it is disabled. Returning
0 on success and negative error codes (like -EBUSY) on failure might cause
CCF to implicitly cast the negative error codes to true, falsely assuming
the clock is enabled when it is actually unavailable.

[ ... ]
> +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,
> +};

[Severity: High]
The commit message mentions this provider exposes a fixed-rate clock, but
the clock-frequency property is missing from the binding, and .recalc_rate
is not implemented here. 

If the clocks property is omitted in the device tree (which the schema
allows), won't the root clock default to 0 Hz? How will downstream consumers
know the correct clock rate without a way to define it?

[ ... ]
> +static int gpio_locked_clk_parse_gpios(struct gpio_locked_clk_priv *priv)
> +{
[ ... ]
> +	for (i = 0; i < priv->num_gpios; i++) {
> +		of_property_read_string_index(np, "gpio-names", i,
> +				&priv->gpio_names[i]);

[Severity: Medium]
Will this logic cause dt_binding_check to fail if a device tree includes
gpio-names?

The driver attempts to read the gpio-names property, but it is missing
from the dt-binding schema, which strictly enforces additionalProperties:
false. If it is omitted from the device tree to pass validation, won't
priv->gpio_names[i] remain NULL and cause debug warnings?

[ ... ]
> +static int gpio_locked_clk_parse_outputs(struct gpio_locked_clk_priv *priv)
> +{
[ ... ]
> +	init.name = priv->output_clock_name;
> +	init.ops = &gpio_locked_clk_ops;
> +	init.flags = 0;
> +	init.num_parents = priv->num_clks;

[Severity: High]
Can clk_enable() permanently fail if there are multiple input clocks?

By setting init.num_parents = priv->num_clks, this configures the clock as
a CCF multiplexer. However, CCF's clk_core_enable() only enables the
single active parent returned by .get_parent() (which the driver hardcodes
to return 0). Since the .is_enabled() callback mandates that all input
clocks must be enabled, won't the check fail and return -EBUSY for the other
unenabled parents?

> +
> +	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]
Is it safe to assign priv->clks[j].id to parent_names[j] here?

Since devm_clk_bulk_get_all() populates clks[i].id from the clock-names
property, and clock-names is explicitly omitted from the DT binding schema
(and forbidden by additionalProperties: false), won't clks[i].id evaluate
to NULL?

This could cause clk_core_populate_parent_map() to emit a WARN and return
-EINVAL during registration. Would using .parent_hws instead of
.parent_names be a more reliable approach?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603-feature-clock-guard-v3-0-01cca0aa04a5@bruker.com?part=1

[PATCH v3 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 a3e2862ebd7e..464ca1c1d8a8 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 v3 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:
- [High] The driver reuses its `.enable` function for `.is_enabled`, returning 0 for ready and `-EBUSY` for not ready, which inverts CCF's `.is_enabled` boolean semantics.
- [High] The driver returns `-EBUSY` immediately from `.enable()` instead of internally waiting for the clock to lock, inappropriately forcing consumers to poll.
- [High] The driver attempts to aggregate multiple input clocks using `num_parents > 1`, but CCF treats multiple parents as a multiplexer (MUX), not a logical AND gate.
- [High] The driver populates `init.parent_names` with local consumer connection IDs instead of globally registered clock names, breaking parent clock resolution.
- [High] The driver violates API boundaries by using `__clk_get_hw()` on consumer `struct clk` pointers to check `clk_hw_is_enabled()`, which only checks software state and risks use-after-free.
- [High] Calling `gpiod_get_array_value()` from `.enable()` and `.is_enabled()` triggers a sleeping-in-atomic BUG if the GPIOs are on a slow bus.
- [Medium] Input GPIOs are requested with `GPIOD_ASIS` instead of `GPIOD_IN`, failing to guarantee the lines are configured as inputs.
--

commit a891e5fe0dbdc27c3cfbbf69bfe6b147ef67ce9c
Author: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>

clk: Add gpio-locked fixed clock driver

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.

> 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
[ ... ]
> +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);

[Severity: High]
Will this trigger a sleeping in atomic context warning if the GPIOs are on a
slow bus?

CCF .enable() and .is_enabled() callbacks execute in an atomic context
under the global enable_lock spinlock with IRQs disabled. If the GPIOs
are on an I2C/SPI expander, gpiod_get_array_value() requires a sleepable
context.

Should .prepare() and .is_prepared() be used instead?

> +
> +	if (ret) {
> +		dev_err(priv->dev, "Failed to read GPIOs");
> +		return -EIO;
> +	}
[ ... ]
> +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;
> +	}

[Severity: High]
Does this logic invert the CCF state machine?

The framework expects .is_enabled() to return non-zero if the clock is
enabled and 0 if disabled. This function returns 0 when ready, and -EBUSY
when not ready. Because -EBUSY (-16) is non-zero, CCF interprets a 'not
ready' state as 'enabled', and a 'ready' state as 'disabled'.

> +
> +	// 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)) {

[Severity: High]
Is it safe to use __clk_get_hw() on arbitrary consumer struct clk pointers?

This internal CCF macro returns a raw pointer that could be freed if the
provider unbinds, creating a use-after-free risk. Additionally,
clk_hw_is_enabled() only checks the software enable_count rather than
actual hardware PLL lock states.

> +			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);
> +}

[Severity: High]
Can this cause consumers to fail instantly without waiting for the clock
to lock?

Standard clock consumers expect clk_prepare_enable() to block until the
clock is stable or return a timeout error. By returning -EBUSY immediately
without polling, consumer drivers calling clk_prepare_enable() during probe
will instantly fail if the clock isn't already locked.

Should the clock provider poll the lock status internally (preferably within
its .prepare() callback)?

> +
> +/* 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 CCF treat num_parents > 1 as a multiplexer rather than an AND-gate?

When CCF propagates clk_enable(), it only enables the single active parent
determined by .get_parent(), which hardcodes to 0 here. This means input
clocks 1 through N-1 are never automatically enabled, causing the checks in
gpio_locked_clk_is_enabled() to permanently fail unless those clocks happen
to be enabled externally.

> +
> +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: Medium]
Does requesting with GPIOD_ASIS guarantee the lines are configured as inputs?

If the bootloader left the pins in an output state, reading them might return
the internal output latch value instead of the external signal. Should
GPIOD_IN be used instead?

> +	if (IS_ERR(priv->gpios)) {
> +		dev_err(dev, "failed to get GPIO array: %ld\n",
> +				PTR_ERR(priv->gpios));
> +		return 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;
[ ... ]
> +	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;

[Severity: High]
Does this assign local consumer connection names to init.parent_names
instead of globally registered clock names?

devm_clk_bulk_get_all() fills the id field with the local consumer
connection name from the clock-names DT property, but init.parent_names
expects global provider names. This could prevent CCF from linking the
clock hierarchy correctly.

Could init.parent_hws or init.parent_data be used instead?

> +	}
> +
> +	priv->output_hw_clk.init = &init;
> +
> +	ret = devm_clk_hw_register(dev, &priv->output_hw_clk);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603-feature-clock-guard-v3-0-01cca0aa04a5@bruker.com?part=2