[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

[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

[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