Re: [PATCH v3 4/5] clk: clk-gpio: add driver for gated-fixed-clocks
From: Stephen Boyd <sboyd@kernel.org>
Date: 2024-08-28 18:30:54
Also in:
linux-clk, linux-devicetree, linux-rockchip, lkml
Quoting Heiko Stuebner (2024-08-28 03:15:02)
quoted hunk ↗ jump to hunk
diff --git a/drivers/clk/clk-gpio.c b/drivers/clk/clk-gpio.c index cda362a2eca0..8bcdef340b4c 100644 --- a/drivers/clk/clk-gpio.c +++ b/drivers/clk/clk-gpio.c@@ -239,3 +240,184 @@ static struct platform_driver gpio_clk_driver = { }, }; builtin_platform_driver(gpio_clk_driver); + +/** + * DOC: gated fixed clock, controlled with a gpio output and a regulator + * Traits of this clock: + * prepare - clk_prepare and clk_unprepare are function & control regulator + * optionally a gpio that can sleep + * enable - clk_enable and clk_disable are functional & control gpio + * rate - rate is fixed and set on clock generation
Maybe 'clock registration'
+ * parent - fixed clock is a root clock and has no parent.
Not sure why this one gets the period while other lines above don't.
+ */
+
+/**
+ * struct clk_gate_fixed - gated-fixed-clock
+ *
+ * clk_gpio: instance of clk_gpio for gate-gpio
+ * supply: supply regulator
+ * rate: fixed rate
+ */
+struct clk_gated_fixed {
+ struct clk_gpio clk_gpio;
+ struct regulator *supply;
+ u32 rate;unsigned long rate to match the CCF type please.
+};
+
+#define to_clk_gated_fixed(_clk_gpio) container_of(_clk_gpio, struct clk_gated_fixed, clk_gpio)
+
+static unsigned long clk_gated_fixed_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ return to_clk_gated_fixed(to_clk_gpio(hw))->rate;
+}
+
+static int clk_gated_fixed_prepare(struct clk_hw *hw)
+{
+ struct clk_gated_fixed *clk = to_clk_gated_fixed(to_clk_gpio(hw));
+
+ if (!clk->supply)
+ return 0;
+
+ return regulator_enable(clk->supply);
+}
+
+static void clk_gated_fixed_unprepare(struct clk_hw *hw)
+{
+ struct clk_gated_fixed *clk = to_clk_gated_fixed(to_clk_gpio(hw));
+
+ if (!clk->supply)
+ return;
+
+ regulator_disable(clk->supply);
+}
+
+static int clk_gated_fixed_is_prepared(struct clk_hw *hw)
+{
+ struct clk_gated_fixed *clk = to_clk_gated_fixed(to_clk_gpio(hw));
+
+ if (!clk->supply)
+ return true;
+
+ return regulator_is_enabled(clk->supply);
+}
+
+/*
+ * Fixed gated clock with non-sleeping gpio.
+ *
+ * Prepare operation turns on the supply regulator
+ * and the enable operation switches the enable-gpio.
+ */
+const struct clk_ops clk_gated_fixed_ops = {static
+ .prepare = clk_gated_fixed_prepare,
+ .unprepare = clk_gated_fixed_unprepare,
+ .is_prepared = clk_gated_fixed_is_prepared,
+ .enable = clk_gpio_gate_enable,
+ .disable = clk_gpio_gate_disable,
+ .is_enabled = clk_gpio_gate_is_enabled,
+ .recalc_rate = clk_gated_fixed_recalc_rate,
+};
+
+static int clk_sleeping_gated_fixed_prepare(struct clk_hw *hw)
+{
+ int ret;
+
+ ret = clk_gated_fixed_prepare(hw);
+ if (ret)
+ return ret;
+
+ ret = clk_sleeping_gpio_gate_prepare(hw);
+ if (ret)
+ clk_gated_fixed_unprepare(hw);
+
+ return ret;
+}
+
+static void clk_sleeping_gated_fixed_unprepare(struct clk_hw *hw)
+{
+ clk_gated_fixed_unprepare(hw);
+ clk_sleeping_gpio_gate_unprepare(hw);
+}
+
+/*
+ * Fixed gated clock with non-sleeping gpio.
+ *
+ * Enabling the supply regulator and switching the enable-gpio happens
+ * both in the prepare step.
+ * is_prepared only needs to check the gpio state, as toggling the
+ * gpio is the last step when preparing.
+ */
+const struct clk_ops clk_sleeping_gated_fixed_ops = {static
+ .prepare = clk_sleeping_gated_fixed_prepare,
+ .unprepare = clk_sleeping_gated_fixed_unprepare,
+ .is_prepared = clk_sleeping_gpio_gate_is_prepared,
+ .recalc_rate = clk_gated_fixed_recalc_rate,
+};
+
+static int clk_gated_fixed_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct clk_gated_fixed *clk;
+ const struct clk_ops *ops;
+ const char *clk_name;
+ int ret;
+
+ clk = devm_kzalloc(dev, sizeof(*clk), GFP_KERNEL);
+ if (!clk)
+ return -ENOMEM;
+
+ if (device_property_read_u32(dev, "clock-frequency", &clk->rate))Why not return the error code?
+ return dev_err_probe(dev, -EIO, "failed to get clock-frequency");
Missing newline on printk.
+
+ ret = device_property_read_string(dev, "clock-output-names", &clk_name);
+ if (ret)
+ clk_name = fwnode_get_name(dev->fwnode);
+
+ clk->supply = devm_regulator_get_optional(dev, "vdd");
+ if (IS_ERR(clk->supply)) {
+ if (PTR_ERR(clk->supply) != -ENODEV)
+ return dev_err_probe(dev, PTR_ERR(clk->supply),
+ "failed to get regulator\n");
+ clk->supply = NULL;
+ }
+
+ clk->clk_gpio.gpiod = devm_gpiod_get_optional(dev, "enable",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(clk->clk_gpio.gpiod))
+ return dev_err_probe(dev, PTR_ERR(clk->clk_gpio.gpiod),
+ "failed to get gpio\n");
+
+ if (gpiod_cansleep(clk->clk_gpio.gpiod))
+ ops = &clk_sleeping_gated_fixed_ops;
+ else
+ ops = &clk_gated_fixed_ops;
+
+ clk->clk_gpio.hw.init = CLK_HW_INIT_NO_PARENT(clk_name, ops, 0);
+
+ /* register the clock */
+ ret = devm_clk_hw_register(dev, &clk->clk_gpio.hw);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to register clock\n");
+
+ ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
+ &clk->clk_gpio.hw);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to register clock provider\n");
+
+ return 0;
+}
+
+static const struct of_device_id gated_fixed_clk_match_table[] = {
+ { .compatible = "gated-fixed-clock" },Add a sentinel.
+};
+
+static struct platform_driver gated_fixed_clk_driver = {
+ .probe = clk_gated_fixed_probe,
+ .driver = {
+ .name = "gated-fixed-clk",
+ .of_match_table = gated_fixed_clk_match_table,
+ },
+};
+builtin_platform_driver(gated_fixed_clk_driver);The comment above builtin_platform_driver says "Each driver may only use this macro once". Seems that we need to expand the macro.