* [PATCH v4 0/2] A proposal to add a gpio-locked fixed clock driver.
@ 2026-07-26 17:40 ` Vyacheslav Yurkov
0 siblings, 0 replies; 8+ messages in thread
From: Vyacheslav Yurkov via B4 Relay @ 2026-07-26 17:40 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Brian Masney
Cc: linux-kernel, linux-clk, devicetree, Vyacheslav Yurkov,
Vyacheslav Yurkov
A gpio-locked fixed clock aggregates one or more input clocks and/or one
or more GPIOs. It's similar to a gated-fixed-clock, but GPIO direction is
inverted. Consumers can use the output clock to wait until all input
clocks are locked and only then initialize / access dependent peripherals.
The usage example for such a driver is when peripherals depend on PLLs in
a FPGA, which can't be directly accessed by the CPU, but need a GPIO pin
to check whether clock is actually usable. E.g. some of the IPs might not
have a proper split between registers and IP core, which means that if an
external clock and/or PLL lock is missing and one tries to access the
registers, the response never comes, thus the CPU stalls.
Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com>
Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
---
Changes in v4:
- Removed driver specifics from DT binding
- Link to v3: https://lore.kernel.org/r/20260603-feature-clock-guard-v3-0-01cca0aa04a5@bruker.com
Changes in v3:
- Removed unnecessary dt bindings
- Improved HW description and commit messages
- Link to v2: https://lore.kernel.org/r/20260510-feature-clock-guard-v2-0-6c25458d5340@bruker.com
Changes in v2:
- Renamed to clk-gpio-locked to express intent.
- Provide enable() / is_enabled() operations so the clock behaves as
expected
- Fixed DTS errors / warnings
- Link to v1: https://lore.kernel.org/r/20260318-feature-clock-guard-v1-0-6137cb4084b7@bruker.com
---
Vyacheslav Yurkov (2):
dt-bindings: Add GPIO-locked fixed clock
clk: Add gpio-locked fixed clock driver
.../bindings/clock/gpio-locked-fixed-clock.yaml | 59 ++++
drivers/clk/Makefile | 1 +
drivers/clk/clk-gpio-locked.c | 306 +++++++++++++++++++++
3 files changed, 366 insertions(+)
---
base-commit: 3dab139d4795f688e4f243e40c7474df00d329d9
change-id: 20260318-feature-clock-guard-f20a2c35b965
Best regards,
--
Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v4 0/2] A proposal to add a gpio-locked fixed clock driver. @ 2026-07-26 17:40 ` Vyacheslav Yurkov 0 siblings, 0 replies; 8+ messages in thread From: Vyacheslav Yurkov @ 2026-07-26 17:40 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brian Masney Cc: linux-kernel, linux-clk, devicetree, Vyacheslav Yurkov, Vyacheslav Yurkov A gpio-locked fixed clock aggregates one or more input clocks and/or one or more GPIOs. It's similar to a gated-fixed-clock, but GPIO direction is inverted. Consumers can use the output clock to wait until all input clocks are locked and only then initialize / access dependent peripherals. The usage example for such a driver is when peripherals depend on PLLs in a FPGA, which can't be directly accessed by the CPU, but need a GPIO pin to check whether clock is actually usable. E.g. some of the IPs might not have a proper split between registers and IP core, which means that if an external clock and/or PLL lock is missing and one tries to access the registers, the response never comes, thus the CPU stalls. Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com> Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com> --- Changes in v4: - Removed driver specifics from DT binding - Link to v3: https://lore.kernel.org/r/20260603-feature-clock-guard-v3-0-01cca0aa04a5@bruker.com Changes in v3: - Removed unnecessary dt bindings - Improved HW description and commit messages - Link to v2: https://lore.kernel.org/r/20260510-feature-clock-guard-v2-0-6c25458d5340@bruker.com Changes in v2: - Renamed to clk-gpio-locked to express intent. - Provide enable() / is_enabled() operations so the clock behaves as expected - Fixed DTS errors / warnings - Link to v1: https://lore.kernel.org/r/20260318-feature-clock-guard-v1-0-6137cb4084b7@bruker.com --- Vyacheslav Yurkov (2): dt-bindings: Add GPIO-locked fixed clock clk: Add gpio-locked fixed clock driver .../bindings/clock/gpio-locked-fixed-clock.yaml | 59 ++++ drivers/clk/Makefile | 1 + drivers/clk/clk-gpio-locked.c | 306 +++++++++++++++++++++ 3 files changed, 366 insertions(+) --- base-commit: 3dab139d4795f688e4f243e40c7474df00d329d9 change-id: 20260318-feature-clock-guard-f20a2c35b965 Best regards, -- Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v4 1/2] dt-bindings: Add GPIO-locked fixed clock 2026-07-26 17:40 ` Vyacheslav Yurkov @ 2026-07-26 17:40 ` Vyacheslav Yurkov -1 siblings, 0 replies; 8+ messages in thread From: Vyacheslav Yurkov via B4 Relay @ 2026-07-26 17:40 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brian Masney Cc: linux-kernel, linux-clk, devicetree, Vyacheslav Yurkov, Vyacheslav Yurkov From: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com> Some hardware designs provide fixed-frequency clocks generated outside software control, such as by FPGA-resident PLLs. While the clock rate is fixed, a separate GPIO signal indicates whether the clock source is locked and producing a valid output. Describe a GPIO-locked fixed clock provider that exposes a fixed-rate clock whose availability depends on one or more GPIO lock-status signals. Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com> Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com> --- .../bindings/clock/gpio-locked-fixed-clock.yaml | 59 ++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml b/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml new file mode 100644 index 000000000000..4d17889c51ad --- /dev/null +++ b/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml @@ -0,0 +1,59 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/gpio-locked-fixed-clock.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: GPIO Locked Fixed Clock + +maintainers: + - Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com> + +properties: + compatible: + const: gpio-locked-fixed-clock + + "#clock-cells": + const: 0 + + clocks: + description: Input clocks whose validity is monitored by this provider. + + clock-output-names: + description: Names of the clock provided by this controller. + maxItems: 1 + + locked-gpios: + description: | + GPIOs to check the lock state. + minItems: 1 + maxItems: 32 + +required: + - compatible + - "#clock-cells" + +anyOf: + - required: + - clocks + - required: + - locked-gpios + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + clk_gpio_locked: gpio-locked-fixed-clock { + compatible = "gpio-locked-fixed-clock"; + #clock-cells = <0>; + + clocks = <&clk0 0>, <&pll 0>; + + locked-gpios = <&gpio0 4 GPIO_ACTIVE_HIGH>, + <&gpio0 5 GPIO_ACTIVE_HIGH>, + <&gpio1 2 GPIO_ACTIVE_LOW>; + + clock-output-names = "clkout0"; + }; -- 2.34.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v4 1/2] dt-bindings: Add GPIO-locked fixed clock @ 2026-07-26 17:40 ` Vyacheslav Yurkov 0 siblings, 0 replies; 8+ messages in thread From: Vyacheslav Yurkov @ 2026-07-26 17:40 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brian Masney Cc: linux-kernel, linux-clk, devicetree, Vyacheslav Yurkov, Vyacheslav Yurkov Some hardware designs provide fixed-frequency clocks generated outside software control, such as by FPGA-resident PLLs. While the clock rate is fixed, a separate GPIO signal indicates whether the clock source is locked and producing a valid output. Describe a GPIO-locked fixed clock provider that exposes a fixed-rate clock whose availability depends on one or more GPIO lock-status signals. Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com> Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com> --- .../bindings/clock/gpio-locked-fixed-clock.yaml | 59 ++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml b/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml new file mode 100644 index 000000000000..4d17889c51ad --- /dev/null +++ b/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml @@ -0,0 +1,59 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/gpio-locked-fixed-clock.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: GPIO Locked Fixed Clock + +maintainers: + - Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com> + +properties: + compatible: + const: gpio-locked-fixed-clock + + "#clock-cells": + const: 0 + + clocks: + description: Input clocks whose validity is monitored by this provider. + + clock-output-names: + description: Names of the clock provided by this controller. + maxItems: 1 + + locked-gpios: + description: | + GPIOs to check the lock state. + minItems: 1 + maxItems: 32 + +required: + - compatible + - "#clock-cells" + +anyOf: + - required: + - clocks + - required: + - locked-gpios + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + clk_gpio_locked: gpio-locked-fixed-clock { + compatible = "gpio-locked-fixed-clock"; + #clock-cells = <0>; + + clocks = <&clk0 0>, <&pll 0>; + + locked-gpios = <&gpio0 4 GPIO_ACTIVE_HIGH>, + <&gpio0 5 GPIO_ACTIVE_HIGH>, + <&gpio1 2 GPIO_ACTIVE_LOW>; + + clock-output-names = "clkout0"; + }; -- 2.34.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: Add GPIO-locked fixed clock 2026-07-26 17:40 ` Vyacheslav Yurkov (?) @ 2026-07-26 17:45 ` sashiko-bot -1 siblings, 0 replies; 8+ messages in thread From: sashiko-bot @ 2026-07-26 17:45 UTC (permalink / raw) To: Vyacheslav Yurkov; +Cc: devicetree, robh, conor+dt Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The schema's `anyOf` constraint makes the essential `locked-gpios` property optional, and the binding is missing `clock-frequency`, breaking validation for fixed clocks. -- --- Patch [1]: [PATCH v4 1/2] dt-bindings: Add GPIO-locked fixed clock --- commit fe524126bff4b0741b4910f69b3229a3794694dd Author: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com> dt-bindings: Add GPIO-locked fixed clock This commit adds a device tree binding for a fixed-rate clock whose availability depends on one or more GPIO lock-status signals. Describe a GPIO-locked fixed clock provider that exposes a fixed-rate clock whose availability is tied to a lock-status signal. > diff --git a/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml b/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml > new file mode 100644 > index 0000000000000..4d17889c51ad9 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/gpio-locked-fixed-clock.yaml [ ... ] > +properties: > + compatible: > + const: gpio-locked-fixed-clock > + > + "#clock-cells": > + const: 0 > + [Severity: Medium] Is clock-frequency intentionally omitted from the properties list? Because additionalProperties is set to false, a clock node that omits the clocks property will have no valid way to specify its rate, which would cause standalone fixed clocks to fail validation. > + clocks: > + description: Input clocks whose validity is monitored by this provider. > + > + clock-output-names: > + description: Names of the clock provided by this controller. > + maxItems: 1 > + > + locked-gpios: > + description: | > + GPIOs to check the lock state. > + minItems: 1 > + maxItems: 32 > + > +required: > + - compatible > + - "#clock-cells" > + > +anyOf: > + - required: > + - clocks > + - required: > + - locked-gpios [Severity: Medium] Does this anyOf condition inadvertently make locked-gpios optional? If a node specifies clocks but omits locked-gpios, the first branch is satisfied and the schema will silently pass validation. This seems to allow configurations that bypass the core purpose of a GPIO-locked clock provider. > + > +additionalProperties: false -- Sashiko AI review · https://sashiko.dev/#/patchset/20260726-feature-clock-guard-v4-0-e9c8b372b71c@bruker.com?part=1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v4 2/2] clk: Add gpio-locked fixed clock driver 2026-07-26 17:40 ` Vyacheslav Yurkov @ 2026-07-26 17:40 ` Vyacheslav Yurkov -1 siblings, 0 replies; 8+ messages in thread From: Vyacheslav Yurkov via B4 Relay @ 2026-07-26 17:40 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brian Masney Cc: linux-kernel, linux-clk, devicetree, Vyacheslav Yurkov, Vyacheslav Yurkov From: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com> A gpio-locked clock aggregates one or more input clocks and/or one or more GPIOs. It's a FPGA-assisted clocking design where peripheral clocks are generated by FPGA PLLs that are outside CPU control, with clock-valid/PLL-lock status exposed through GPIO signals. Consumers can use the output clock to wait until all input clocks are locked and only then initialize dependent peripherals. Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com> Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com> --- drivers/clk/Makefile | 1 + drivers/clk/clk-gpio-locked.c | 306 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 307 insertions(+) diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index cc108a75a900..846383ed0358 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -35,6 +35,7 @@ obj-$(CONFIG_CLK_FD_KUNIT_TEST) += clk-fractional-divider_test.o obj-$(CONFIG_COMMON_CLK) += clk-gpio.o ifeq ($(CONFIG_OF), y) obj-$(CONFIG_COMMON_CLK) += clk-conf.o +obj-$(CONFIG_COMMON_CLK) += clk-gpio-locked.o endif # KUnit specific helpers diff --git a/drivers/clk/clk-gpio-locked.c b/drivers/clk/clk-gpio-locked.c new file mode 100644 index 000000000000..79098f9b6532 --- /dev/null +++ b/drivers/clk/clk-gpio-locked.c @@ -0,0 +1,306 @@ +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +/* + * Clock Controller Guard Driver + * + * Copyright 2026 Bruker Corporation + */ + +#include <linux/clk.h> +#include <linux/clk-provider.h> +#include <linux/device.h> +#include <linux/gpio/consumer.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/slab.h> + +#define MAX_INPUT_GPIO_COUNT 32 + +/** + * struct gpio_locked_clk_priv - private state for the whole driver + * @dev: platform device + * + * @clks: array of input clock descriptors + * @num_clks: number of entries in @inputs + * + * @gpios: array of GPIO descriptors + * @gpio_names: GPIO names + * @num_gpios: number of input GPIOs + * + * @output_hw_clk: output clock HW descriptor + * @output_clock_name: output clock name + */ +struct gpio_locked_clk_priv { + struct device *dev; + + struct clk_bulk_data *clks; + int num_clks; + + struct gpio_descs *gpios; + const char **gpio_names; + int num_gpios; + + struct clk_hw output_hw_clk; + const char *output_clock_name; +}; + +#define to_gpio_locked_clk_priv(_hw) \ + container_of(_hw, struct gpio_locked_clk_priv, output_hw_clk) + +static int is_gpio_ready(struct gpio_locked_clk_priv *priv) +{ + unsigned long values[BITS_TO_LONGS(MAX_INPUT_GPIO_COUNT)] = {0}; + int ret = 0; + + if (priv->num_gpios == 0) + return 0; + + ret = gpiod_get_array_value(priv->gpios->ndescs, + priv->gpios->desc, + priv->gpios->info, + values); + + if (ret) { + dev_err(priv->dev, "Failed to read GPIOs"); + return -EIO; + } + + for (int i = 0; i < priv->gpios->ndescs; i++) { + if (!test_bit(i, values)) { + dev_warn(priv->dev, "GPIO %s is not ready", priv->gpio_names[i]); + return -EBUSY; + } + } + + return 0; +} + +static int gpio_locked_clk_is_enabled(struct clk_hw *hw) +{ + struct gpio_locked_clk_priv *priv = to_gpio_locked_clk_priv(hw); + int ret = 0; + + if (priv->num_gpios > 0) { + ret = is_gpio_ready(priv); + if (ret < 0) + return ret; + } + + // Now check for the clocks + for (int i = 0; i < priv->num_clks; i++) { + struct clk_hw *hw_clk = __clk_get_hw(priv->clks[i].clk); + + if (!clk_hw_is_enabled(hw_clk)) { + dev_dbg(priv->dev, "Clock %i (%s) is not ready", + i, priv->clks[i].id); + return -EBUSY; + } + } + + return 0; +} + +/* We can't enable the clock, but the Common Clock Framework calls only + * enable() not is_enabled() + */ +static int gpio_locked_clk_enable(struct clk_hw *hw) +{ + return gpio_locked_clk_is_enabled(hw); +} + +/* We have to implement it, but we are not going to control + * parent clock selection + */ +static u8 gpio_locked_clk_get_parent(struct clk_hw *hw) +{ + return 0; +} + +static const struct clk_ops gpio_locked_clk_ops = { + .enable = gpio_locked_clk_enable, + .is_enabled = gpio_locked_clk_is_enabled, + .get_parent = gpio_locked_clk_get_parent, +}; + +static int gpio_locked_clk_parse_inputs(struct gpio_locked_clk_priv *priv) +{ + struct device *dev = priv->dev; + int ret; + + ret = devm_clk_bulk_get_all(dev, &priv->clks); + if (ret < 0) { + dev_err(dev, "failed to get input clocks: %d\n", ret); + return ret ? ret : -ENOENT; + } + + priv->num_clks = ret; + + if (priv->num_clks == 0) + dev_info(dev, "No input clocks provided\n"); + + for (int i = 0; i < priv->num_clks; i++) + dev_dbg(dev, "input clk[%d]: name='%s' rate=%lu Hz\n", + i, priv->clks[i].id, + clk_get_rate(priv->clks[i].clk)); + + return 0; +} + +static int gpio_locked_clk_parse_gpios(struct gpio_locked_clk_priv *priv) +{ + struct device *dev = priv->dev; + struct device_node *np = dev->of_node; + int i; + + priv->gpios = devm_gpiod_get_array_optional(dev, "locked", GPIOD_ASIS); + if (IS_ERR(priv->gpios)) { + dev_err(dev, "failed to get GPIO array: %ld\n", + PTR_ERR(priv->gpios)); + return PTR_ERR(priv->gpios); + } + + if (!priv->gpios) { + dev_info(dev, "No GPIOs provided, continue\n"); + priv->num_gpios = 0; + return 0; + } + + priv->num_gpios = priv->gpios->ndescs; + if (priv->num_gpios > MAX_INPUT_GPIO_COUNT) { + dev_err(priv->dev, "Maximum number of input GPIOs is 32\n"); + return -EINVAL; + } + + /* gpio_descs carries no names, so read "gpio-names" separately */ + priv->gpio_names = devm_kcalloc(dev, priv->num_gpios, sizeof(*priv->gpio_names), + GFP_KERNEL); + if (!priv->gpio_names) + return -ENOMEM; + + for (i = 0; i < priv->num_gpios; i++) { + of_property_read_string_index(np, "gpio-names", i, + &priv->gpio_names[i]); + + dev_dbg(dev, "gpio[%d]: name='%s'\n", + i, priv->gpio_names[i] ? priv->gpio_names[i] : "(unnamed)"); + } + + return 0; +} + +static int gpio_locked_clk_parse_outputs(struct gpio_locked_clk_priv *priv) +{ + struct device *dev = priv->dev; + struct device_node *np = dev->of_node; + struct clk_init_data init = {}; + int ret; + + of_property_read_string_index(np, "clock-output-names", 0, + &priv->output_clock_name); + + if (!priv->output_clock_name) + priv->output_clock_name = dev_name(priv->dev); + + init.name = priv->output_clock_name; + init.ops = &gpio_locked_clk_ops; + init.flags = 0; + init.num_parents = priv->num_clks; + + if (priv->num_clks) { + const char **parent_names; + int j; + + parent_names = devm_kcalloc(dev, priv->num_clks, + sizeof(*parent_names), + GFP_KERNEL); + if (!parent_names) + return -ENOMEM; + + for (j = 0; j < priv->num_clks; j++) + parent_names[j] = priv->clks[j].id; + + init.parent_names = parent_names; + } + + priv->output_hw_clk.init = &init; + + ret = devm_clk_hw_register(dev, &priv->output_hw_clk); + if (ret) { + dev_err(dev, "failed to register output clk'%s': %d\n", + priv->output_clock_name, ret); + return ret; + } + + dev_info(priv->dev, "Output clock '%s' registered\n", priv->output_clock_name); + + return 0; +} + +static int gpio_locked_clk_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct gpio_locked_clk_priv *priv; + int ret; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->dev = dev; + platform_set_drvdata(pdev, priv); + + ret = gpio_locked_clk_parse_inputs(priv); + if (ret) + return ret; + + ret = gpio_locked_clk_parse_gpios(priv); + if (ret) + return ret; + + if (priv->num_clks == 0 && priv->num_gpios == 0) { + dev_err(priv->dev, "At least 1 input clock or input GPIO is required\n"); + return -EINVAL; + } + + ret = gpio_locked_clk_parse_outputs(priv); + if (ret) + return ret; + + ret = devm_of_clk_add_hw_provider(priv->dev, of_clk_hw_simple_get, + &priv->output_hw_clk); + if (ret) { + dev_err(priv->dev, "failed to register clock provider '%s': %d\n", + priv->output_clock_name, ret); + return ret; + } + + dev_info(dev, "registered %u input clocks, %u GPIOs\n", + priv->num_clks, priv->num_gpios); + + return 0; +} + +static void gpio_locked_clk_remove(struct platform_device *pdev) +{ + dev_dbg(&pdev->dev, "removed\n"); +} + +static const struct of_device_id gpio_locked_clk_of_match[] = { + { .compatible = "gpio-locked-fixed-clock" }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, gpio_locked_clk_of_match); + +static struct platform_driver gpio_locked_clk_driver = { + .probe = gpio_locked_clk_probe, + .remove = gpio_locked_clk_remove, + .driver = { + .name = "gpio-locked-fixed-clock", + .of_match_table = gpio_locked_clk_of_match, + }, +}; +module_platform_driver(gpio_locked_clk_driver); + +MODULE_AUTHOR("Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>"); +MODULE_DESCRIPTION("GPIO-locked clock driver"); +MODULE_LICENSE("Dual BSD/GPL"); -- 2.34.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v4 2/2] clk: Add gpio-locked fixed clock driver @ 2026-07-26 17:40 ` Vyacheslav Yurkov 0 siblings, 0 replies; 8+ messages in thread From: Vyacheslav Yurkov @ 2026-07-26 17:40 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brian Masney Cc: linux-kernel, linux-clk, devicetree, Vyacheslav Yurkov, Vyacheslav Yurkov A gpio-locked clock aggregates one or more input clocks and/or one or more GPIOs. It's a FPGA-assisted clocking design where peripheral clocks are generated by FPGA PLLs that are outside CPU control, with clock-valid/PLL-lock status exposed through GPIO signals. Consumers can use the output clock to wait until all input clocks are locked and only then initialize dependent peripherals. Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com> Signed-off-by: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com> --- drivers/clk/Makefile | 1 + drivers/clk/clk-gpio-locked.c | 306 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 307 insertions(+) diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index cc108a75a900..846383ed0358 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -35,6 +35,7 @@ obj-$(CONFIG_CLK_FD_KUNIT_TEST) += clk-fractional-divider_test.o obj-$(CONFIG_COMMON_CLK) += clk-gpio.o ifeq ($(CONFIG_OF), y) obj-$(CONFIG_COMMON_CLK) += clk-conf.o +obj-$(CONFIG_COMMON_CLK) += clk-gpio-locked.o endif # KUnit specific helpers diff --git a/drivers/clk/clk-gpio-locked.c b/drivers/clk/clk-gpio-locked.c new file mode 100644 index 000000000000..79098f9b6532 --- /dev/null +++ b/drivers/clk/clk-gpio-locked.c @@ -0,0 +1,306 @@ +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +/* + * Clock Controller Guard Driver + * + * Copyright 2026 Bruker Corporation + */ + +#include <linux/clk.h> +#include <linux/clk-provider.h> +#include <linux/device.h> +#include <linux/gpio/consumer.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/slab.h> + +#define MAX_INPUT_GPIO_COUNT 32 + +/** + * struct gpio_locked_clk_priv - private state for the whole driver + * @dev: platform device + * + * @clks: array of input clock descriptors + * @num_clks: number of entries in @inputs + * + * @gpios: array of GPIO descriptors + * @gpio_names: GPIO names + * @num_gpios: number of input GPIOs + * + * @output_hw_clk: output clock HW descriptor + * @output_clock_name: output clock name + */ +struct gpio_locked_clk_priv { + struct device *dev; + + struct clk_bulk_data *clks; + int num_clks; + + struct gpio_descs *gpios; + const char **gpio_names; + int num_gpios; + + struct clk_hw output_hw_clk; + const char *output_clock_name; +}; + +#define to_gpio_locked_clk_priv(_hw) \ + container_of(_hw, struct gpio_locked_clk_priv, output_hw_clk) + +static int is_gpio_ready(struct gpio_locked_clk_priv *priv) +{ + unsigned long values[BITS_TO_LONGS(MAX_INPUT_GPIO_COUNT)] = {0}; + int ret = 0; + + if (priv->num_gpios == 0) + return 0; + + ret = gpiod_get_array_value(priv->gpios->ndescs, + priv->gpios->desc, + priv->gpios->info, + values); + + if (ret) { + dev_err(priv->dev, "Failed to read GPIOs"); + return -EIO; + } + + for (int i = 0; i < priv->gpios->ndescs; i++) { + if (!test_bit(i, values)) { + dev_warn(priv->dev, "GPIO %s is not ready", priv->gpio_names[i]); + return -EBUSY; + } + } + + return 0; +} + +static int gpio_locked_clk_is_enabled(struct clk_hw *hw) +{ + struct gpio_locked_clk_priv *priv = to_gpio_locked_clk_priv(hw); + int ret = 0; + + if (priv->num_gpios > 0) { + ret = is_gpio_ready(priv); + if (ret < 0) + return ret; + } + + // Now check for the clocks + for (int i = 0; i < priv->num_clks; i++) { + struct clk_hw *hw_clk = __clk_get_hw(priv->clks[i].clk); + + if (!clk_hw_is_enabled(hw_clk)) { + dev_dbg(priv->dev, "Clock %i (%s) is not ready", + i, priv->clks[i].id); + return -EBUSY; + } + } + + return 0; +} + +/* We can't enable the clock, but the Common Clock Framework calls only + * enable() not is_enabled() + */ +static int gpio_locked_clk_enable(struct clk_hw *hw) +{ + return gpio_locked_clk_is_enabled(hw); +} + +/* We have to implement it, but we are not going to control + * parent clock selection + */ +static u8 gpio_locked_clk_get_parent(struct clk_hw *hw) +{ + return 0; +} + +static const struct clk_ops gpio_locked_clk_ops = { + .enable = gpio_locked_clk_enable, + .is_enabled = gpio_locked_clk_is_enabled, + .get_parent = gpio_locked_clk_get_parent, +}; + +static int gpio_locked_clk_parse_inputs(struct gpio_locked_clk_priv *priv) +{ + struct device *dev = priv->dev; + int ret; + + ret = devm_clk_bulk_get_all(dev, &priv->clks); + if (ret < 0) { + dev_err(dev, "failed to get input clocks: %d\n", ret); + return ret ? ret : -ENOENT; + } + + priv->num_clks = ret; + + if (priv->num_clks == 0) + dev_info(dev, "No input clocks provided\n"); + + for (int i = 0; i < priv->num_clks; i++) + dev_dbg(dev, "input clk[%d]: name='%s' rate=%lu Hz\n", + i, priv->clks[i].id, + clk_get_rate(priv->clks[i].clk)); + + return 0; +} + +static int gpio_locked_clk_parse_gpios(struct gpio_locked_clk_priv *priv) +{ + struct device *dev = priv->dev; + struct device_node *np = dev->of_node; + int i; + + priv->gpios = devm_gpiod_get_array_optional(dev, "locked", GPIOD_ASIS); + if (IS_ERR(priv->gpios)) { + dev_err(dev, "failed to get GPIO array: %ld\n", + PTR_ERR(priv->gpios)); + return PTR_ERR(priv->gpios); + } + + if (!priv->gpios) { + dev_info(dev, "No GPIOs provided, continue\n"); + priv->num_gpios = 0; + return 0; + } + + priv->num_gpios = priv->gpios->ndescs; + if (priv->num_gpios > MAX_INPUT_GPIO_COUNT) { + dev_err(priv->dev, "Maximum number of input GPIOs is 32\n"); + return -EINVAL; + } + + /* gpio_descs carries no names, so read "gpio-names" separately */ + priv->gpio_names = devm_kcalloc(dev, priv->num_gpios, sizeof(*priv->gpio_names), + GFP_KERNEL); + if (!priv->gpio_names) + return -ENOMEM; + + for (i = 0; i < priv->num_gpios; i++) { + of_property_read_string_index(np, "gpio-names", i, + &priv->gpio_names[i]); + + dev_dbg(dev, "gpio[%d]: name='%s'\n", + i, priv->gpio_names[i] ? priv->gpio_names[i] : "(unnamed)"); + } + + return 0; +} + +static int gpio_locked_clk_parse_outputs(struct gpio_locked_clk_priv *priv) +{ + struct device *dev = priv->dev; + struct device_node *np = dev->of_node; + struct clk_init_data init = {}; + int ret; + + of_property_read_string_index(np, "clock-output-names", 0, + &priv->output_clock_name); + + if (!priv->output_clock_name) + priv->output_clock_name = dev_name(priv->dev); + + init.name = priv->output_clock_name; + init.ops = &gpio_locked_clk_ops; + init.flags = 0; + init.num_parents = priv->num_clks; + + if (priv->num_clks) { + const char **parent_names; + int j; + + parent_names = devm_kcalloc(dev, priv->num_clks, + sizeof(*parent_names), + GFP_KERNEL); + if (!parent_names) + return -ENOMEM; + + for (j = 0; j < priv->num_clks; j++) + parent_names[j] = priv->clks[j].id; + + init.parent_names = parent_names; + } + + priv->output_hw_clk.init = &init; + + ret = devm_clk_hw_register(dev, &priv->output_hw_clk); + if (ret) { + dev_err(dev, "failed to register output clk'%s': %d\n", + priv->output_clock_name, ret); + return ret; + } + + dev_info(priv->dev, "Output clock '%s' registered\n", priv->output_clock_name); + + return 0; +} + +static int gpio_locked_clk_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct gpio_locked_clk_priv *priv; + int ret; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->dev = dev; + platform_set_drvdata(pdev, priv); + + ret = gpio_locked_clk_parse_inputs(priv); + if (ret) + return ret; + + ret = gpio_locked_clk_parse_gpios(priv); + if (ret) + return ret; + + if (priv->num_clks == 0 && priv->num_gpios == 0) { + dev_err(priv->dev, "At least 1 input clock or input GPIO is required\n"); + return -EINVAL; + } + + ret = gpio_locked_clk_parse_outputs(priv); + if (ret) + return ret; + + ret = devm_of_clk_add_hw_provider(priv->dev, of_clk_hw_simple_get, + &priv->output_hw_clk); + if (ret) { + dev_err(priv->dev, "failed to register clock provider '%s': %d\n", + priv->output_clock_name, ret); + return ret; + } + + dev_info(dev, "registered %u input clocks, %u GPIOs\n", + priv->num_clks, priv->num_gpios); + + return 0; +} + +static void gpio_locked_clk_remove(struct platform_device *pdev) +{ + dev_dbg(&pdev->dev, "removed\n"); +} + +static const struct of_device_id gpio_locked_clk_of_match[] = { + { .compatible = "gpio-locked-fixed-clock" }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, gpio_locked_clk_of_match); + +static struct platform_driver gpio_locked_clk_driver = { + .probe = gpio_locked_clk_probe, + .remove = gpio_locked_clk_remove, + .driver = { + .name = "gpio-locked-fixed-clock", + .of_match_table = gpio_locked_clk_of_match, + }, +}; +module_platform_driver(gpio_locked_clk_driver); + +MODULE_AUTHOR("Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>"); +MODULE_DESCRIPTION("GPIO-locked clock driver"); +MODULE_LICENSE("Dual BSD/GPL"); -- 2.34.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v4 2/2] clk: Add gpio-locked fixed clock driver 2026-07-26 17:40 ` Vyacheslav Yurkov (?) @ 2026-07-26 17:51 ` sashiko-bot -1 siblings, 0 replies; 8+ messages in thread From: sashiko-bot @ 2026-07-26 17:51 UTC (permalink / raw) To: Vyacheslav Yurkov; +Cc: conor+dt, devicetree, robh Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider: - [Critical] The `.is_enabled` callback completely inverts the clock status logic, returning error codes instead of boolean state. - [High] The driver attempts to aggregate multiple dependent input clocks by registering them as CCF parents, causing a TOCTOU race and failing to enable parents. - [High] Using local DT connection IDs to populate `init.parent_names` causes global clock matching to fail and risks NULL pointer dereferences. - [High] The `enable` callback returns `-EBUSY` immediately rather than waiting for the hardware lock, contradicting CCF semantics. - [High] The driver requests the lock GPIOs with `GPIOD_ASIS` rather than `GPIOD_IN`, potentially failing to read the pin state correctly. - [High] The driver fails to validate that provided GPIOs are non-sleeping, risking a kernel panic if slow GPIO controllers are used. - [Medium] Calling `dev_warn` from a spinlocked, atomic context causes severe latencies. -- --- Patch [2]: [PATCH v4 2/2] clk: Add gpio-locked fixed clock driver --- commit 86eabe35b2750726ea6024147b3e2f51a67773ca Author: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com> clk: Add gpio-locked fixed clock driver This commit introduces a new clock driver that aggregates input clocks and GPIOs to expose a clock-valid/PLL-lock status for FPGA-assisted clocking designs. > diff --git a/drivers/clk/clk-gpio-locked.c b/drivers/clk/clk-gpio-locked.c > new file mode 100644 > index 0000000000000..79098f9b65326 > --- /dev/null > +++ b/drivers/clk/clk-gpio-locked.c > @@ -0,0 +1,306 @@ [ ... ] > +static int is_gpio_ready(struct gpio_locked_clk_priv *priv) > +{ > + unsigned long values[BITS_TO_LONGS(MAX_INPUT_GPIO_COUNT)] = {0}; > + int ret = 0; [ ... ] > + for (int i = 0; i < priv->gpios->ndescs; i++) { > + if (!test_bit(i, values)) { > + dev_warn(priv->dev, "GPIO %s is not ready", priv->gpio_names[i]); [Severity: Medium] Is it safe to call dev_warn() while holding the clock framework's enable_lock? Because is_gpio_ready() is called from the .enable and .is_enabled callbacks, it executes under a spinlock with hard interrupts disabled. Unconditionally logging to the console from this context can cause severe IRQ latencies or hard lockups, especially if a consumer polls the clock state. > + return -EBUSY; > + } > + } > + > + return 0; > +} > + > +static int gpio_locked_clk_is_enabled(struct clk_hw *hw) > +{ > + struct gpio_locked_clk_priv *priv = to_gpio_locked_clk_priv(hw); > + int ret = 0; > + > + if (priv->num_gpios > 0) { > + ret = is_gpio_ready(priv); > + if (ret < 0) > + return ret; > + } > + > + // Now check for the clocks > + for (int i = 0; i < priv->num_clks; i++) { > + struct clk_hw *hw_clk = __clk_get_hw(priv->clks[i].clk); > + > + if (!clk_hw_is_enabled(hw_clk)) { > + dev_dbg(priv->dev, "Clock %i (%s) is not ready", > + i, priv->clks[i].id); > + return -EBUSY; > + } > + } > + > + return 0; [Severity: Critical] Does this function invert the expected return logic for .is_enabled? The clock framework expects .is_enabled to return a non-zero value when the clock is enabled, and 0 when it is disabled. By returning -EBUSY when not ready and 0 when ready, the framework will consider the clock enabled when it is actually not ready, and disabled when it is ready. > +} > + > +/* We can't enable the clock, but the Common Clock Framework calls only > + * enable() not is_enabled() > + */ > +static int gpio_locked_clk_enable(struct clk_hw *hw) > +{ > + return gpio_locked_clk_is_enabled(hw); [Severity: High] Should the enable callback block until the hardware lock is acquired? Standard clock consumers do not poll clk_prepare_enable(). By doing a single non-blocking check and failing immediately with -EBUSY, consumer driver probes will fail if the hardware lock is not instantaneously ready at the moment of enablement. > +} > + > +/* We have to implement it, but we are not going to control > + * parent clock selection > + */ > +static u8 gpio_locked_clk_get_parent(struct clk_hw *hw) > +{ > + return 0; [Severity: High] Does hardcoding the parent to index 0 break the aggregation of multiple inputs? When a clock is registered with num_parents > 1 and implements get_parent, the clock framework treats it as a multiplexer where only one parent is active. By always returning 0, the framework is told to abandon tracking for all other parents. This means parents 1 through N will not be enabled when this clock is enabled. > +} > + > +static const struct clk_ops gpio_locked_clk_ops = { > + .enable = gpio_locked_clk_enable, > + .is_enabled = gpio_locked_clk_is_enabled, > + .get_parent = gpio_locked_clk_get_parent, > +}; [ ... ] > +static int gpio_locked_clk_parse_gpios(struct gpio_locked_clk_priv *priv) > +{ > + struct device *dev = priv->dev; > + struct device_node *np = dev->of_node; > + int i; > + > + priv->gpios = devm_gpiod_get_array_optional(dev, "locked", GPIOD_ASIS); [Severity: High] Could requesting the GPIO array with GPIOD_ASIS leave the pins in an undefined state? Without enforcing the input direction using GPIOD_IN, the hardware configuration relies on the bootloader. If the bootloader leaves the pins floating or configured as outputs, reading the pin state may fail to detect the lock correctly. [Severity: High] Does the driver need to verify that these GPIOs are non-sleeping? The clock framework's .enable and .is_enabled callbacks run in an atomic context with interrupts disabled. If the device tree provides a sleeping GPIO (such as an I2C or SPI expander), calling gpiod_get_array_value() from is_gpio_ready() will trigger a scheduling-while-atomic BUG. Could the probe function check !gpiod_cansleep() to reject sleeping GPIOs? > + if (IS_ERR(priv->gpios)) { > + dev_err(dev, "failed to get GPIO array: %ld\n", > + PTR_ERR(priv->gpios)); [ ... ] > +static int gpio_locked_clk_parse_outputs(struct gpio_locked_clk_priv *priv) > +{ > + struct device *dev = priv->dev; > + struct device_node *np = dev->of_node; > + struct clk_init_data init = {}; > + int ret; [ ... ] > + init.flags = 0; > + init.num_parents = priv->num_clks; > + > + if (priv->num_clks) { > + const char **parent_names; > + int j; > + > + parent_names = devm_kcalloc(dev, priv->num_clks, > + sizeof(*parent_names), > + GFP_KERNEL); > + if (!parent_names) > + return -ENOMEM; > + > + for (j = 0; j < priv->num_clks; j++) > + parent_names[j] = priv->clks[j].id; [Severity: High] Does this assignment pass local device tree connection IDs to the clock framework instead of globally registered provider names? During probe, devm_clk_bulk_get_all() populates the id field with local consumer names from the device tree. The clock framework expects init.parent_names to contain global provider names. This mismatch will cause the output clock to fail to match its parents. Additionally, if the optional clock-names property is missing in the DT, priv->clks[j].id will be NULL, causing a NULL pointer dereference when the framework evaluates init.parent_names. > + > + init.parent_names = parent_names; > + } > + > + priv->output_hw_clk.init = &init; -- Sashiko AI review · https://sashiko.dev/#/patchset/20260726-feature-clock-guard-v4-0-e9c8b372b71c@bruker.com?part=2 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-07-26 17:51 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-26 17:40 [PATCH v4 0/2] A proposal to add a gpio-locked fixed clock driver Vyacheslav Yurkov via B4 Relay 2026-07-26 17:40 ` Vyacheslav Yurkov 2026-07-26 17:40 ` [PATCH v4 1/2] dt-bindings: Add GPIO-locked fixed clock Vyacheslav Yurkov via B4 Relay 2026-07-26 17:40 ` Vyacheslav Yurkov 2026-07-26 17:45 ` sashiko-bot 2026-07-26 17:40 ` [PATCH v4 2/2] clk: Add gpio-locked fixed clock driver Vyacheslav Yurkov via B4 Relay 2026-07-26 17:40 ` Vyacheslav Yurkov 2026-07-26 17:51 ` sashiko-bot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.