Quoting Heiko Stuebner (2024-07-15 04:02:50)
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 4abe16c8ccdfe..ca7b7b7ddfd8d 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -79,6 +79,7 @@ obj-$(CONFIG_COMMON_CLK_SI521XX) += clk-si521xx.o
> obj-$(CONFIG_COMMON_CLK_VC3) += clk-versaclock3.o
> obj-$(CONFIG_COMMON_CLK_VC5) += clk-versaclock5.o
> obj-$(CONFIG_COMMON_CLK_VC7) += clk-versaclock7.o
> +obj-$(CONFIG_COMMON_CLK_VCO) += clk-vco.o
Wrong section. It's basically a common clk type.
> obj-$(CONFIG_COMMON_CLK_WM831X) += clk-wm831x.o
> obj-$(CONFIG_COMMON_CLK_XGENE) += clk-xgene.o
>
> diff --git a/drivers/clk/clk-vco.c b/drivers/clk/clk-vco.c
> new file mode 100644
> index 0000000000000..f7fe2bc627f36
> --- /dev/null
> +++ b/drivers/clk/clk-vco.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2024 Heiko Stuebner <heiko@xxxxxxxxx>
> + *
> + * Generic voltage controlled oscillator
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +
> +struct clk_vco {
> + struct device *dev;
> + struct clk_hw hw;
> + u32 rate;
> + struct regulator *supply;
> + struct gpio_desc *enable_gpio;
> +};
> +
> +#define to_clk_vco(_hw) container_of(_hw, struct clk_vco, hw)
> +
> +static int clk_vco_prepare(struct clk_hw *hw)
> +{
> + return regulator_enable(to_clk_vco(hw)->supply);
> +}
> +
> +static void clk_vco_unprepare(struct clk_hw *hw)
> +{
> + regulator_disable(to_clk_vco(hw)->supply);
> +}
> +
> +static int clk_vco_enable(struct clk_hw *hw)
> +{
> + gpiod_set_value(to_clk_vco(hw)->enable_gpio, 1);
> + return 0;
> +}
> +
> +static void clk_vco_disable(struct clk_hw *hw)
> +{
> + gpiod_set_value(to_clk_vco(hw)->enable_gpio, 0);
> +}
It looks similar to clk-gpio.c code, but not as complete because it
assumes gpios can't sleep. Please look into reusing that code somehow,
possibly exporting 'clk_gpio_gate_ops' and struct clk_gpio for use in
this new driver. It would be good to fold the sleepable gpio bit as well
somehow, maybe with a new function to get a device's gpiod along with
returning a const pointer to the clk_ops that can be copied and amended
with the regulator part.
> +
> +static unsigned long clk_vco_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + return to_clk_vco(hw)->rate;
> +}
> +
> +const struct clk_ops clk_vco_ops = {
> + .prepare = clk_vco_prepare,
> + .unprepare = clk_vco_unprepare,
> + .enable = clk_vco_enable,
> + .disable = clk_vco_disable,
> + .recalc_rate = clk_vco_recalc_rate,
> +};
> +
> +static int clk_vco_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct clk_vco *clkgen;
> + const char *clk_name;
> + int ret;
> +
> + clkgen = devm_kzalloc(dev, sizeof(*clkgen), GFP_KERNEL);
> + if (!clkgen)
> + return -ENOMEM;
> +
> + clkgen->dev = dev;
Is this used outside of probe? Why stash it?
> +
> + if (device_property_read_u32(dev, "clock-frequency", &clkgen->rate))
> + return dev_err_probe(dev, -EIO, "failed to get clock-frequency");
> +
> + ret = device_property_read_string(dev, "clock-output-names", &clk_name);
> + if (ret)
> + clk_name = fwnode_get_name(dev->fwnode);
> +
> + clkgen->supply = devm_regulator_get_optional(dev, "vdd");
> + if (IS_ERR(clkgen->supply)) {
> + if (PTR_ERR(clkgen->supply) != -ENODEV)
> + return dev_err_probe(dev, PTR_ERR(clkgen->supply),
> + "failed to get regulator\n");
> + clkgen->supply = NULL;
> + }
> +
> + clkgen->enable_gpio = devm_gpiod_get_optional(dev, "enable",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(clkgen->enable_gpio))
> + return dev_err_probe(dev, PTR_ERR(clkgen->enable_gpio),
> + "failed to get gpio\n");
> +
> + ret = gpiod_direction_output(clkgen->enable_gpio, 0);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "failed to set gpio output");
Missing newline.
> +
> + clkgen->hw.init = CLK_HW_INIT_NO_PARENT(clk_name, &clk_vco_ops, 0);
> +
> + /* register the clock */
> + ret = devm_clk_hw_register(dev, &clkgen->hw);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to register clock\n");
> +
