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

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

[Vyacheslav Yurkov via B4 Relay]

A gpio-locked fixed clock represents an input clock, which state is 
determined by a GPIO signal. 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 v5:
- Use existing DT binding with an additional property instead of adding
  a new one
- The driver is simplified in a way that it represents the actual HW
  desgin without additional unrelated constructs
- Aggregation is removed, now it's one clock input, one clock output
- Link to v4: https://lore.kernel.org/r/20260726-feature-clock-guard-v4-0-e9c8b372b71c@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: clock: gpio-gate-clock: Add a new compatible string
      clk: Add gpio-locked fixed clock driver

 .../devicetree/bindings/clock/gpio-gate-clock.yaml |  32 +++-
 drivers/clk/Makefile                               |   1 +
 drivers/clk/clk-gpio-locked.c                      | 169 +++++++++++++++++++++
 3 files changed, 200 insertions(+), 2 deletions(-)
---
base-commit: 704340f1cd0dcef829eb62f5b48ae95a2ce17bdf
change-id: 20260318-feature-clock-guard-f20a2c35b965

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

[PATCH v5 1/2] dt-bindings: clock: gpio-gate-clock: Add a new compatible string

[Vyacheslav Yurkov via B4 Relay]

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

Extend the binding to be used also for gpio-locked-fixed-clock.
A GPIO-locked fixed clock provider exposes a fixed-rate clock whose
availability depends on one or more GPIO lock-status signals.

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.

Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
---
 .../devicetree/bindings/clock/gpio-gate-clock.yaml | 32 ++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/gpio-gate-clock.yaml b/Documentation/devicetree/bindings/clock/gpio-gate-clock.yaml
index d09d0e3f0c6e..00cf2ad66dd0 100644
--- a/Documentation/devicetree/bindings/clock/gpio-gate-clock.yaml
+++ b/Documentation/devicetree/bindings/clock/gpio-gate-clock.yaml
@@ -11,7 +11,9 @@ maintainers:
 
 properties:
   compatible:
-    const: gpio-gate-clock
+    enum:
+      - gpio-gate-clock
+      - gpio-locked-fixed-clock
 
   clocks:
     maxItems: 1
@@ -23,10 +25,29 @@ properties:
     description: GPIO reference for enabling and disabling the clock.
     maxItems: 1
 
+  locked-gpios:
+    description: GPIO that indicates whether the clock is enabled or disabled.
+    maxItems: 1
+
 required:
   - compatible
   - '#clock-cells'
-  - enable-gpios
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          const: gpio-gate-clock
+    then:
+      required:
+        - enable-gpios
+  - if:
+      properties:
+        compatible:
+          const: gpio-locked-fixed-clock
+    then:
+      required:
+        - locked-gpios
 
 additionalProperties: false
 
@@ -40,3 +61,10 @@ examples:
         #clock-cells = <0>;
         enable-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
     };
+
+    clk_gpio_locked {
+        compatible = "gpio-locked-fixed-clock";
+        #clock-cells = <0>;
+        clocks = <&pll>;
+        locked-gpios = <&gpio0 4 GPIO_ACTIVE_HIGH>;
+    };

-- 
2.34.1

Re: [PATCH v5 1/2] dt-bindings: clock: gpio-gate-clock: Add a new compatible string

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] dt-bindings: clock: locked-gpios maxItems contradicts commit message
- [Low] dt-bindings: clock: schema does not enforce mutually exclusive GPIO properties
- [Low] dt-bindings: clock: example node name uses invalid underscore character
--

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

dt-bindings: clock: gpio-gate-clock: Add a new compatible string

Extend the binding to support a gpio-locked-fixed-clock. This provides a
fixed-rate clock whose availability depends on one or more GPIO lock-status
signals.

> diff --git a/Documentation/devicetree/bindings/clock/gpio-gate-clock.yaml b/Documentation/devicetree/bindings/clock/gpio-gate-clock.yaml
> index d09d0e3f0c6ec..00cf2ad66dd01 100644
> --- a/Documentation/devicetree/bindings/clock/gpio-gate-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/gpio-gate-clock.yaml

[ ... ]

> @@ -23,10 +25,29 @@ properties:
>      description: GPIO reference for enabling and disabling the clock.
>      maxItems: 1
>  
> +  locked-gpios:
> +    description: GPIO that indicates whether the clock is enabled or disabled.
> +    maxItems: 1

[Severity: Medium]
The commit message states that the clock's availability depends on "one or
more GPIO lock-status signals".

Looking at the schema definition, the locked-gpios property is strictly
restricted to exactly one item via maxItems: 1.

Does this maxItems constraint need to be increased to support multiple signals,
or should the commit message be updated to reflect that only a single GPIO is
supported?

> +
>  required:
>    - compatible
>    - '#clock-cells'
> -  - enable-gpios
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          const: gpio-gate-clock
> +    then:
> +      required:
> +        - enable-gpios

[Severity: Low]
Since additionalProperties: false only enforces that properties must be
defined in the top-level properties block, both enable-gpios and locked-gpios
are now allowed for any compatible matching this file.

Should this conditional block explicitly reject locked-gpios by setting it
to false? Without this, a device tree node with compatible = "gpio-gate-clock"
can include locked-gpios and inappropriately pass schema validation.

> +  - if:
> +      properties:
> +        compatible:
> +          const: gpio-locked-fixed-clock
> +    then:
> +      required:
> +        - locked-gpios

[ ... ]

> @@ -40,3 +61,10 @@ examples:
>          #clock-cells = <0>;
>          enable-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
>      };
> +
> +    clk_gpio_locked {
> +        compatible = "gpio-locked-fixed-clock";

[Severity: Low]
Does this example node name need to be updated to use a hyphen instead of an
underscore?

Device tree node names should ideally use hyphens and a generic class name
like clock. Using an underscore violates device tree naming conventions and
will likely trigger a node_name_chars_strict warning during compilation.

> +        #clock-cells = <0>;
> +        clocks = <&pll>;
> +        locked-gpios = <&gpio0 4 GPIO_ACTIVE_HIGH>;
> +    };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-feature-clock-guard-v5-0-42ab5dc3a6aa@bruker.com?part=1

Re: [PATCH v5 1/2] dt-bindings: clock: gpio-gate-clock: Add a new compatible string

[Conor Dooley]

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

On Tue, Sep 15, 2026 at 09:27:39AM +0000, Vyacheslav Yurkov via B4 Relay wrote:
> From: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
> 
> Extend the binding to be used also for gpio-locked-fixed-clock.
> A GPIO-locked fixed clock provider exposes a fixed-rate clock whose
> availability depends on one or more GPIO lock-status signals.
> 
> 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.
> 
> Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

Cheers,
Conor.

> ---
>  .../devicetree/bindings/clock/gpio-gate-clock.yaml | 32 ++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/gpio-gate-clock.yaml b/Documentation/devicetree/bindings/clock/gpio-gate-clock.yaml
> index d09d0e3f0c6e..00cf2ad66dd0 100644
> --- a/Documentation/devicetree/bindings/clock/gpio-gate-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/gpio-gate-clock.yaml
> @@ -11,7 +11,9 @@ maintainers:
>  
>  properties:
>    compatible:
> -    const: gpio-gate-clock
> +    enum:
> +      - gpio-gate-clock
> +      - gpio-locked-fixed-clock
>  
>    clocks:
>      maxItems: 1
> @@ -23,10 +25,29 @@ properties:
>      description: GPIO reference for enabling and disabling the clock.
>      maxItems: 1
>  
> +  locked-gpios:
> +    description: GPIO that indicates whether the clock is enabled or disabled.
> +    maxItems: 1
> +
>  required:
>    - compatible
>    - '#clock-cells'
> -  - enable-gpios
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          const: gpio-gate-clock
> +    then:
> +      required:
> +        - enable-gpios
> +  - if:
> +      properties:
> +        compatible:
> +          const: gpio-locked-fixed-clock
> +    then:
> +      required:
> +        - locked-gpios
>  
>  additionalProperties: false
>  
> @@ -40,3 +61,10 @@ examples:
>          #clock-cells = <0>;
>          enable-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
>      };
> +
> +    clk_gpio_locked {
> +        compatible = "gpio-locked-fixed-clock";
> +        #clock-cells = <0>;
> +        clocks = <&pll>;
> +        locked-gpios = <&gpio0 4 GPIO_ACTIVE_HIGH>;
> +    };
> 
> -- 
> 2.34.1
> 
> 

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

[PATCH v5 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 exposes a clock, which status is determined by a
GPIO signal. The common use-case is 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 the input clock is locked
and only then initialize dependent peripherals.

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

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index b18af485d7f0..b904f292d683 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -46,6 +46,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..b648f8763922
--- /dev/null
+++ b/drivers/clk/clk-gpio-locked.c
@@ -0,0 +1,169 @@
+// 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>
+
+/**
+ * struct gpio_locked_clk_priv - private state for the whole driver
+ * @dev:		platform device
+ *
+ * @input_clk	input clock
+ * @gpios:		input GPIO descriptor
+ *
+ * @output_hw_clk:      output clock HW descriptor
+ * @output_clock_name:  output clock name
+ */
+struct gpio_locked_clk_priv {
+	struct device *dev;
+
+	struct clk *input_clk;
+	struct gpio_desc *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 gpio_locked_clk_is_enabled(struct clk_hw *hw)
+{
+	struct gpio_locked_clk_priv *priv = to_gpio_locked_clk_priv(hw);
+
+	int data = gpiod_get_value(priv->gpios);
+
+	if (data < 0) {
+		dev_err(priv->dev, "Failed to get data gpio val: %d\n",
+				data);
+		return data;
+	} else if (!data) {
+		dev_warn(priv->dev, "GPIO is not ready");
+		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_outputs(struct gpio_locked_clk_priv *priv)
+{
+	struct device *dev = priv->dev;
+	struct device_node *np = dev->of_node;
+	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);
+
+	priv->output_hw_clk.init =
+		CLK_HW_INIT_FW_NAME(priv->output_clock_name,
+				    __clk_get_name(priv->input_clk),
+				    &gpio_locked_clk_ops, 0);
+
+	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);
+
+	priv->input_clk = devm_clk_get_enabled(priv->dev, NULL);
+	if (IS_ERR(priv->input_clk))
+		return dev_err_probe(priv->dev, PTR_ERR(priv->input_clk),
+				"Failed to get locked fixed clock, not yet ready\n");
+
+	priv->gpios = devm_gpiod_get(priv->dev, "locked", GPIOD_ASIS);
+	if (IS_ERR(priv->gpios)) {
+		dev_err(dev, "failed to get locked gpios: %ld\n",
+				PTR_ERR(priv->gpios));
+		return PTR_ERR(priv->gpios);
+	}
+
+	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 %s gpio locked clock\n",
+		 clk_hw_get_name(&priv->output_hw_clk));
+
+	return 0;
+}
+
+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,
+	.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 v5 2/2] clk: Add gpio-locked fixed clock driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] clk: gpio-locked-fixed-clock breaks topology by passing global name to fw_name
- [High] clk: inverted boolean returns in .is_enabled corrupt clock state
- [High] clk: sleeping gpiod_get_value called from atomic .enable callback
- [Medium] clk: parent clock unconditionally enabled defeating PM, no lock polling
--

commit 5585fb0b2689bcfcfc380b86553df6e0d756293f
Author: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
Subject: clk: Add gpio-locked fixed clock driver

This commit adds a gpio-locked clock driver, which exposes a clock whose
status is determined by a GPIO signal. It is intended for FPGA-assisted
clocking designs, allowing consumers to wait until the input clock is
locked before initializing dependent peripherals.

> diff --git a/drivers/clk/clk-gpio-locked.c b/drivers/clk/clk-gpio-locked.c
> new file mode 100644
> index 0000000000000..b648f8763922e
> --- /dev/null
> +++ b/drivers/clk/clk-gpio-locked.c

[ ... ]

> +static int gpio_locked_clk_is_enabled(struct clk_hw *hw)
> +{
> +	struct gpio_locked_clk_priv *priv = to_gpio_locked_clk_priv(hw);
> +
> +	int data = gpiod_get_value(priv->gpios);

[Severity: High]
Could this trigger a sleeping-while-atomic panic if the GPIO is on a slow
bus?

The core clock framework executes the .is_enabled and .enable callbacks
with the enable_lock spinlock held (interrupts disabled). 

Since the probe function doesn't check gpiod_cansleep(), accessing a
potentially sleepable GPIO here could result in a crash.

Would it be better to either use .prepare and .is_prepared instead, which
execute under a mutex, or explicitly reject sleepable GPIOs during probe?

> +
> +	if (data < 0) {
> +		dev_err(priv->dev, "Failed to get data gpio val: %d\n",
> +				data);
> +		return data;
> +	} else if (!data) {
> +		dev_warn(priv->dev, "GPIO is not ready");
> +		return -EBUSY;
> +	}
> +
> +	return 0;
> +}

[Severity: High]
Does this logic invert the clock state for the common clock framework?

The framework expects .is_enabled to return a boolean-equivalent value
(non-zero for enabled, 0 for disabled). 

Returning -EBUSY evaluates to true (enabled), and 0 evaluates to false
(disabled). 

Could this break clock state tracking and bypass protection mechanisms like
clk_disable_unused by telling the framework the clock is enabled when it
is actually locked, 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: Medium]
Does this instantly fail instead of waiting for the lock as the commit
message suggests?

The commit message indicates consumers can wait until the input clock is
locked. However, because this immediately returns -EBUSY if the GPIO is low
rather than polling or blocking, consumers cannot natively wait for the
lock.

Could we implement a polling loop with a timeout inside a .prepare callback
instead?

[ ... ]

> +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;
> +	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);
> +
> +	priv->output_hw_clk.init =
> +		CLK_HW_INIT_FW_NAME(priv->output_clock_name,
> +				    __clk_get_name(priv->input_clk),
> +				    &gpio_locked_clk_ops, 0);

[Severity: High]
Does using the global clock name with CLK_HW_INIT_FW_NAME break the
parent-child topology?

The CLK_HW_INIT_FW_NAME macro assigns the provided string to the .fw_name
field, which the clock framework uses to look up the parent via
of_parse_clkspec() against the local device tree's clock-names property.

Since __clk_get_name() returns a globally unique provider name instead of a
local device tree name, the lookup will fail and register this clock as an
orphan. 

Since the driver already has the input clock pointer, would it work better
to use CLK_HW_INIT_HW and pass __clk_get_hw(priv->input_clk) instead?

[ ... ]

> +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);
> +
> +	priv->input_clk = devm_clk_get_enabled(priv->dev, NULL);

[Severity: Medium]
Does acquiring an irreversible enable reference here at probe time defeat
dynamic power management?

By permanently enabling the parent clock at probe time, it seems the 
framework loses the ability to disable the parent clock when the
gpio-locked clock is unused.

Would it be better to use devm_clk_get() instead, relying on the consumers
to trigger the enable path when needed?

> +	if (IS_ERR(priv->input_clk))
> +		return dev_err_probe(priv->dev, PTR_ERR(priv->input_clk),
> +				"Failed to get locked fixed clock, not yet ready\n");
> +
> +	priv->gpios = devm_gpiod_get(priv->dev, "locked", GPIOD_ASIS);

[Severity: High]
Should this check if the GPIO is safe to use in atomic context?

If the driver intends to use this GPIO inside the atomic .enable callback,
should it verify gpiod_cansleep() is false here to avoid potential panics
when reading the value later?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-feature-clock-guard-v5-0-42ab5dc3a6aa@bruker.com?part=2

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

[Jerome Brunet]

On mar. 15 sept. 2026 at 09:27, Vyacheslav Yurkov via B4 Relay <devnull+V.Yurkov.EXT.bruker.com@kernel.org> wrote:

> From: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
>
> A gpio-locked clock exposes a clock, which status is determined by a
> GPIO signal. The common use-case is 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 the input clock is locked
> and only then initialize dependent peripherals.
>

We already have gpio gate driver in drivers/clk/clk-gpio.c

It would be much better if you could just extend that one that take
optionally take a clock input like you do here.

> Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
> ---
>  drivers/clk/Makefile          |   1 +
>  drivers/clk/clk-gpio-locked.c | 169 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 170 insertions(+)
>
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index b18af485d7f0..b904f292d683 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -46,6 +46,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..b648f8763922
> --- /dev/null
> +++ b/drivers/clk/clk-gpio-locked.c
> @@ -0,0 +1,169 @@
> +// 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>
> +
> +/**
> + * struct gpio_locked_clk_priv - private state for the whole driver
> + * @dev:		platform device
> + *
> + * @input_clk	input clock
> + * @gpios:		input GPIO descriptor
> + *
> + * @output_hw_clk:      output clock HW descriptor
> + * @output_clock_name:  output clock name
> + */
> +struct gpio_locked_clk_priv {
> +	struct device *dev;
> +
> +	struct clk *input_clk;
> +	struct gpio_desc *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 gpio_locked_clk_is_enabled(struct clk_hw *hw)
> +{
> +	struct gpio_locked_clk_priv *priv = to_gpio_locked_clk_priv(hw);
> +
> +	int data = gpiod_get_value(priv->gpios);
> +
> +	if (data < 0) {
> +		dev_err(priv->dev, "Failed to get data gpio val: %d\n",
> +				data);
> +		return data;
> +	} else if (!data) {
> +		dev_warn(priv->dev, "GPIO is not ready");
> +		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_outputs(struct gpio_locked_clk_priv *priv)
> +{
> +	struct device *dev = priv->dev;
> +	struct device_node *np = dev->of_node;
> +	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);
> +
> +	priv->output_hw_clk.init =
> +		CLK_HW_INIT_FW_NAME(priv->output_clock_name,
> +				    __clk_get_name(priv->input_clk),
> +				    &gpio_locked_clk_ops, 0);
> +
> +	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);
> +
> +	priv->input_clk = devm_clk_get_enabled(priv->dev, NULL);
> +	if (IS_ERR(priv->input_clk))
> +		return dev_err_probe(priv->dev, PTR_ERR(priv->input_clk),
> +				"Failed to get locked fixed clock, not yet ready\n");

In your use case it might be fixed, but nothing says it is in general

> +
> +	priv->gpios = devm_gpiod_get(priv->dev, "locked", GPIOD_ASIS);
> +	if (IS_ERR(priv->gpios)) {
> +		dev_err(dev, "failed to get locked gpios: %ld\n",
> +				PTR_ERR(priv->gpios));
> +		return PTR_ERR(priv->gpios);
> +	}
> +
> +	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 %s gpio locked clock\n",
> +		 clk_hw_get_name(&priv->output_hw_clk));
> +
> +	return 0;
> +}
> +
> +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,
> +	.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
>
>

pw-bot: changes-requested

-- 
Jerome

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

[Vyacheslav Yurkov]

On 15.09.2026 12:09, Jerome Brunet wrote:
> On mar. 15 sept. 2026 at 09:27, Vyacheslav Yurkov via B4 Relay <devnull+V.Yurkov.EXT.bruker.com@kernel.org> wrote:
> 
>> From: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
>>
>> A gpio-locked clock exposes a clock, which status is determined by a
>> GPIO signal. The common use-case is 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 the input clock is locked
>> and only then initialize dependent peripherals.
>>
> 
> We already have gpio gate driver in drivers/clk/clk-gpio.c
> 
> It would be much better if you could just extend that one that take
> optionally take a clock input like you do here.

It is a bit more than that. The gated clock requires "enable-gpios" 
property, while gpio-locked clock needs "locked-gpios". Technically I 
could re-use the "enable-gpios", but that might lead to a confusion, 
because semantically they are used for different purpose. Do you think 
the extension of clk-gpio.c would be still better in this case?

>> Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
>> ---
>>   drivers/clk/Makefile          |   1 +
>>   drivers/clk/clk-gpio-locked.c | 169 ++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 170 insertions(+)
>>
>> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
>> index b18af485d7f0..b904f292d683 100644
>> --- a/drivers/clk/Makefile
>> +++ b/drivers/clk/Makefile
>> @@ -46,6 +46,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..b648f8763922
>> --- /dev/null
>> +++ b/drivers/clk/clk-gpio-locked.c
>> @@ -0,0 +1,169 @@
>> +// 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>
>> +
>> +/**
>> + * struct gpio_locked_clk_priv - private state for the whole driver
>> + * @dev:		platform device
>> + *
>> + * @input_clk	input clock
>> + * @gpios:		input GPIO descriptor
>> + *
>> + * @output_hw_clk:      output clock HW descriptor
>> + * @output_clock_name:  output clock name
>> + */
>> +struct gpio_locked_clk_priv {
>> +	struct device *dev;
>> +
>> +	struct clk *input_clk;
>> +	struct gpio_desc *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 gpio_locked_clk_is_enabled(struct clk_hw *hw)
>> +{
>> +	struct gpio_locked_clk_priv *priv = to_gpio_locked_clk_priv(hw);
>> +
>> +	int data = gpiod_get_value(priv->gpios);
>> +
>> +	if (data < 0) {
>> +		dev_err(priv->dev, "Failed to get data gpio val: %d\n",
>> +				data);
>> +		return data;
>> +	} else if (!data) {
>> +		dev_warn(priv->dev, "GPIO is not ready");
>> +		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_outputs(struct gpio_locked_clk_priv *priv)
>> +{
>> +	struct device *dev = priv->dev;
>> +	struct device_node *np = dev->of_node;
>> +	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);
>> +
>> +	priv->output_hw_clk.init =
>> +		CLK_HW_INIT_FW_NAME(priv->output_clock_name,
>> +				    __clk_get_name(priv->input_clk),
>> +				    &gpio_locked_clk_ops, 0);
>> +
>> +	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);
>> +
>> +	priv->input_clk = devm_clk_get_enabled(priv->dev, NULL);
>> +	if (IS_ERR(priv->input_clk))
>> +		return dev_err_probe(priv->dev, PTR_ERR(priv->input_clk),
>> +				"Failed to get locked fixed clock, not yet ready\n");
> 
> In your use case it might be fixed, but nothing says it is in general
> 
Good point, thanks.

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

[Jerome Brunet]

On mar. 15 sept. 2026 at 15:58, Vyacheslav Yurkov <uvv.mail@gmail.com> wrote:

> On 15.09.2026 12:09, Jerome Brunet wrote:
>> On mar. 15 sept. 2026 at 09:27, Vyacheslav Yurkov via B4 Relay <devnull+V.Yurkov.EXT.bruker.com@kernel.org> wrote:
>> 
>>> From: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
>>>
>>> A gpio-locked clock exposes a clock, which status is determined by a
>>> GPIO signal. The common use-case is 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 the input clock is locked
>>> and only then initialize dependent peripherals.
>>>
>> 
>> We already have gpio gate driver in drivers/clk/clk-gpio.c
>> 
>> It would be much better if you could just extend that one that take
>> optionally take a clock input like you do here.
>
> It is a bit more than that. The gated clock requires "enable-gpios" 
> property, while gpio-locked clock needs "locked-gpios". Technically I 
> could re-use the "enable-gpios", but that might lead to a confusion, 
> because semantically they are used for different purpose. Do you think 
> the extension of clk-gpio.c would be still better in this case?

Apologies, I've mis-read you initial submission.
Basically this is a read-only gate controlled by a gpio.

I think it still belongs in clk-gpio with its own ops, along with the
gate and mux that are already there.

enabled-gpio for the pin maybe ?

>
>>> Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
>>> ---
>>>   drivers/clk/Makefile          |   1 +
>>>   drivers/clk/clk-gpio-locked.c | 169 ++++++++++++++++++++++++++++++++++++++++++
>>>   2 files changed, 170 insertions(+)
>>>
>>> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
>>> index b18af485d7f0..b904f292d683 100644
>>> --- a/drivers/clk/Makefile
>>> +++ b/drivers/clk/Makefile
>>> @@ -46,6 +46,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

Drop every reference to locked ... this is very PLL oriented and the
driver you are proposing is more generic than that


>>>   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..b648f8763922
>>> --- /dev/null
>>> +++ b/drivers/clk/clk-gpio-locked.c
>>> @@ -0,0 +1,169 @@
>>> +// 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>
>>> +
>>> +/**
>>> + * struct gpio_locked_clk_priv - private state for the whole driver
>>> + * @dev:		platform device
>>> + *
>>> + * @input_clk	input clock
>>> + * @gpios:		input GPIO descriptor
>>> + *
>>> + * @output_hw_clk:      output clock HW descriptor
>>> + * @output_clock_name:  output clock name
>>> + */
>>> +struct gpio_locked_clk_priv {
>>> +	struct device *dev;
>>> +
>>> +	struct clk *input_clk;
>>> +	struct gpio_desc *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 gpio_locked_clk_is_enabled(struct clk_hw *hw)
>>> +{
>>> +	struct gpio_locked_clk_priv *priv = to_gpio_locked_clk_priv(hw);
>>> +
>>> +	int data = gpiod_get_value(priv->gpios);
>>> +
>>> +	if (data < 0) {
>>> +		dev_err(priv->dev, "Failed to get data gpio val: %d\n",
>>> +				data);
>>> +		return data;
>>> +	} else if (!data) {
>>> +		dev_warn(priv->dev, "GPIO is not ready");
>>> +		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);
>>> +}

You need to explain your problem a bit more because this is not OK. Your
clock should really just provide .is_enabled() AFAICT

>>> +
>>> +/* 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;
>>> +}

Same, I dont get why you need that. Not needed if there a single parent

>>> +
>>> +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_outputs(struct gpio_locked_clk_priv *priv)
>>> +{
>>> +	struct device *dev = priv->dev;
>>> +	struct device_node *np = dev->of_node;
>>> +	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);
>>> +
>>> +	priv->output_hw_clk.init =
>>> +		CLK_HW_INIT_FW_NAME(priv->output_clock_name,
>>> +				    __clk_get_name(priv->input_clk),

No, just put the name as it is in DT (input?) or go for index 0 possibly
?

No call to devm_clk_get_enabled() from this driver to its input. A clock
controller should not do that. The consuming device will get this clock,
enable it and the enable will trickle down to the provider.


>>> +				    &gpio_locked_clk_ops, 0);
>>> +
>>> +	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);
>>> +

Drop those prints

>>> +	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);
>>> +
>>> +	priv->input_clk = devm_clk_get_enabled(priv->dev, NULL);
>>> +	if (IS_ERR(priv->input_clk))
>>> +		return dev_err_probe(priv->dev, PTR_ERR(priv->input_clk),
>>> +				"Failed to get locked fixed clock, not yet ready\n");
>> 
>> In your use case it might be fixed, but nothing says it is in general
>> 
> Good point, thanks.

-- 
Jerome

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

[Vyacheslav Yurkov]

On 16.09.2026 15:57, Jerome Brunet wrote:

>>>> +/* 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);
>>>> +}
> 
> You need to explain your problem a bit more because this is not OK. Your
> clock should really just provide .is_enabled() AFAICT

When a peripheral driver uses devm_clk_get_enabled(), this is resolved 
to clk_prepare() and clk_enable(). Neither of them check for 
.is_enabled(). Is that a flaw in the CCF or expected behavior?


>>>> +
>>>> +/* 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;
>>>> +}
> 
> Same, I dont get why you need that. Not needed if there a single parent

This is a remnant from v4, thanks for spotting it.

Slava

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

[Jerome Brunet]

On jeu. 17 sept. 2026 at 19:05, Vyacheslav Yurkov <uvv.mail@gmail.com> wrote:

> On 16.09.2026 15:57, Jerome Brunet wrote:
>
>>>>> +/* 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);
>>>>> +}
>> 
>> You need to explain your problem a bit more because this is not OK. Your
>> clock should really just provide .is_enabled() AFAICT
>
> When a peripheral driver uses devm_clk_get_enabled(), this is resolved 
> to clk_prepare() and clk_enable(). Neither of them check for 
> .is_enabled(). Is that a flaw in the CCF or expected behavior?
>

.enable() does return an error on failure. It is up to the provider to
return one or not. In your case, it will be probably be necessary to on
the gpio a little.

Note that if the GPIO can sleep, interacting with the gpio must happen
in prepare (think i2c gpio devices)

>
>>>>> +
>>>>> +/* 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;
>>>>> +}
>> 
>> Same, I dont get why you need that. Not needed if there a single parent
>
> This is a remnant from v4, thanks for spotting it.
>
> Slava

-- 
Jerome

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

[Vyacheslav Yurkov]

On 25.09.2026 13:57, Jerome Brunet wrote:
> On jeu. 17 sept. 2026 at 19:05, Vyacheslav Yurkov <uvv.mail@gmail.com> wrote:
> 
>> On 16.09.2026 15:57, Jerome Brunet wrote:
>>
>>>>>> +/* 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);
>>>>>> +}
>>>
>>> You need to explain your problem a bit more because this is not OK. Your
>>> clock should really just provide .is_enabled() AFAICT
>>
>> When a peripheral driver uses devm_clk_get_enabled(), this is resolved
>> to clk_prepare() and clk_enable(). Neither of them check for
>> .is_enabled(). Is that a flaw in the CCF or expected behavior?
>>
> 
> .enable() does return an error on failure. It is up to the provider to
> return one or not. In your case, it will be probably be necessary to on
> the gpio a little.

I'm not sure I understand what you meant.
> Note that if the GPIO can sleep, interacting with the gpio must happen
> in prepare (think i2c gpio devices)

In other words, I should only provide prepare/unprepare instead? Do you 
think is_enabled can also be replaced by is_prepared in this case?

Slava

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

[Jerome Brunet]

On ven. 25 sept. 2026 at 15:15, Vyacheslav Yurkov <uvv.mail@gmail.com> wrote:

> On 25.09.2026 13:57, Jerome Brunet wrote:
>> On jeu. 17 sept. 2026 at 19:05, Vyacheslav Yurkov <uvv.mail@gmail.com> wrote:
>> 
>>> On 16.09.2026 15:57, Jerome Brunet wrote:
>>>
>>>>>>> +/* 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);
>>>>>>> +}
>>>>
>>>> You need to explain your problem a bit more because this is not OK. Your
>>>> clock should really just provide .is_enabled() AFAICT
>>>
>>> When a peripheral driver uses devm_clk_get_enabled(), this is resolved
>>> to clk_prepare() and clk_enable(). Neither of them check for
>>> .is_enabled(). Is that a flaw in the CCF or expected behavior?
>>>
>> 
>> .enable() does return an error on failure. It is up to the provider to
>> return one or not. In your case, it will be probably be necessary to on
>> the gpio a little.

it will probably be necessary to poll on the gpio a little. (sorry)

>
> I'm not sure I understand what you meant.

>> Note that if the GPIO can sleep, interacting with the gpio must happen
>> in prepare (think i2c gpio devices)
>
> In other words, I should only provide prepare/unprepare instead? Do you 
> think is_enabled can also be replaced by is_prepared in this case?

It depends on the GPIO API used. have a look at clk-gpio.c

>
> Slava

-- 
Jerome