[PATCH v1 1/1] clk: clk-gpio: add actual gated clock

[Sean Anderson]

On 11/17/23 02:52, Svyatoslav Ryhel wrote:
> Existing gpio-gate-clock driver acts like a simple GPIO switch without any
> effect on gated clock. Add actual clock actions into enable/disable ops and
> implement get_rate op by passing gated clock if it is enabled.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95 at gmail.com>
> ---
>   drivers/clk/clk-gpio.c | 47 +++++++++++++++++++++++++++++++++++-------
>   1 file changed, 40 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/clk/clk-gpio.c b/drivers/clk/clk-gpio.c
> index 26d795b978..a55470cfbf 100644
> --- a/drivers/clk/clk-gpio.c
> +++ b/drivers/clk/clk-gpio.c
> @@ -3,19 +3,23 @@
>    * Copyright (C) 2023 Marek Vasut <marek.vasut+renesas at mailbox.org>
>    */
>   
> -#include <asm/gpio.h>
> -#include <common.h>
> -#include <clk-uclass.h>
> +#include <clk.h>
>   #include <dm.h>
> +#include <clk-uclass.h>
> +#include <linux/clk-provider.h>
> +
> +#include <asm/gpio.h>
>   
>   struct clk_gpio_priv {
>   	struct gpio_desc	enable;
> +	struct clk		*clk;

Please name this "parent".

>   };
>   
>   static int clk_gpio_enable(struct clk *clk)
>   {
>   	struct clk_gpio_priv *priv = dev_get_priv(clk->dev);
>   
> +	clk_prepare_enable(priv->clk);

Just clk_enable is fine. We don't have a separate prepare stage line Linux.

>   	dm_gpio_set_value(&priv->enable, 1);
>   
>   	return 0;
> @@ -26,21 +30,50 @@ static int clk_gpio_disable(struct clk *clk)
>   	struct clk_gpio_priv *priv = dev_get_priv(clk->dev);
>   
>   	dm_gpio_set_value(&priv->enable, 0);
> +	clk_disable_unprepare(priv->clk);
>   
>   	return 0;
>   }
>   
> +static ulong clk_gpio_get_rate(struct clk *clk)
> +{
> +	struct clk_gpio_priv *priv = dev_get_priv(clk->dev);
> +	int ret;
> +
> +	ret = dm_gpio_get_value(&priv->enable);
> +	if (ret)
> +		return clk_get_rate(priv->clk);
> +	else
> +		return -EINVAL;

This should always return the parent rate.

> +}
> +
>   const struct clk_ops clk_gpio_ops = {
>   	.enable		= clk_gpio_enable,
>   	.disable	= clk_gpio_disable,
> +	.get_rate	= clk_gpio_get_rate,
>   };
>   
> -static int clk_gpio_probe(struct udevice *dev)
> +static int clk_gpio_of_to_plat(struct udevice *dev)

This change should be a separate commit.

>   {
>   	struct clk_gpio_priv *priv = dev_get_priv(dev);
> +	int ret;
>   
> -	return gpio_request_by_name(dev, "enable-gpios", 0,
> -				    &priv->enable, GPIOD_IS_OUT);
> +	priv->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(priv->clk)) {
> +		log_err("%s: Could not get gated clock: %ld\n",
> +			__func__, PTR_ERR(priv->clk));
> +		return PTR_ERR(priv->clk);
> +	}
> +
> +	ret = gpio_request_by_name(dev, "enable-gpios", 0,
> +				   &priv->enable, GPIOD_IS_OUT);
> +	if (ret) {
> +		log_err("%s: Could not decode enable-gpios (%d)\n",
> +			__func__, ret);

Use dev_dbg for probe errors to reduce binary size.

> +		return ret;
> +	}
> +
> +	return 0;
>   }
>   
>   /*
> @@ -59,7 +92,7 @@ U_BOOT_DRIVER(gpio_gate_clock) = {
>   	.name		= "gpio_clock",
>   	.id		= UCLASS_CLK,
>   	.of_match	= clk_gpio_match,
> -	.probe		= clk_gpio_probe,
> +	.of_to_plat	= clk_gpio_of_to_plat,
>   	.priv_auto	= sizeof(struct clk_gpio_priv),
>   	.ops		= &clk_gpio_ops,
>   	.flags		= DM_FLAG_PRE_RELOC,

--Sean