[PATCH v6 03/19] clk: Unconditionally recursively en-/dis-able clocks

[Rick Chen]

Hi Sean

> For clocks not in the CCF, their parents will not have UCLASS_CLK, so we
> just enable them as normal. The enable count is local to the struct clk,
> but this will never result in the actual en-/dis-able op being called
> (unless the same struct clk is enabled twice).
>
> For clocks in the CCF, we always traverse up the tree when enabling.
> Previously, CCF clocks without id set would be skipped, stopping the
> traversal too early.
>
> Signed-off-by: Sean Anderson <seanga2 at gmail.com>
> Acked-by: Lukasz Majewski <lukma at denx.de>
> ---
>
> Changes in v5:
> - Clear enable_count on request
>
> Changes in v4:
> - Lint
>
> Changes in v3:
> - New
>
>  drivers/clk/clk-uclass.c | 60 ++++++++++++++++++----------------------
>  1 file changed, 27 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> index d497737298..3b711395b8 100644
> --- a/drivers/clk/clk-uclass.c
> +++ b/drivers/clk/clk-uclass.c
> @@ -410,6 +410,7 @@ int clk_request(struct udevice *dev, struct clk *clk)
>         ops = clk_dev_ops(dev);
>
>         clk->dev = dev;
> +       clk->enable_count = 0;

Lukasz has taged Acked-by in v3.
Re: [PATCH v3 03/12] clk: Unconditionally recursively en-/dis-able clocks

This patch shall be fixed and freeze.
But you add a new modification (Clear enable_count on request)in v5 and v6.
Although it seem to look fine and nothing big deal.
I just highlight it here, and if Lukasz have no other comment further.
I will pull it via riscv tree later.

Thanks,
Rick



>
>         if (!ops->request)
>                 return 0;
> @@ -522,7 +523,6 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
>  int clk_enable(struct clk *clk)
>  {
>         const struct clk_ops *ops;
> -       struct clk *clkp = NULL;
>         int ret;
>
>         debug("%s(clk=%p \"%s\")\n", __func__, clk, clk->dev->name);
> @@ -531,32 +531,29 @@ int clk_enable(struct clk *clk)
>         ops = clk_dev_ops(clk->dev);
>
>         if (CONFIG_IS_ENABLED(CLK_CCF)) {
> -               /* Take id 0 as a non-valid clk, such as dummy */
> -               if (clk->id && !clk_get_by_id(clk->id, &clkp)) {
> -                       if (clkp->enable_count) {
> -                               clkp->enable_count++;
> -                               return 0;
> -                       }
> -                       if (clkp->dev->parent &&
> -                           device_get_uclass_id(clkp->dev) == UCLASS_CLK) {
> -                               ret = clk_enable(dev_get_clk_ptr(clkp->dev->parent));
> -                               if (ret) {
> -                                       printf("Enable %s failed\n",
> -                                              clkp->dev->parent->name);
> -                                       return ret;
> -                               }
> +               if (clk->enable_count) {
> +                       clk->enable_count++;
> +                       return 0;
> +               }
> +               if (clk->dev->parent &&
> +                   device_get_uclass_id(clk->dev->parent) == UCLASS_CLK) {
> +                       ret = clk_enable(dev_get_clk_ptr(clk->dev->parent));
> +                       if (ret) {
> +                               printf("Enable %s failed\n",
> +                                      clk->dev->parent->name);
> +                               return ret;
>                         }
>                 }
>
>                 if (ops->enable) {
>                         ret = ops->enable(clk);
>                         if (ret) {
> -                               printf("Enable %s failed\n", clk->dev->name);
> +                               printf("Enable %s failed (error %d)\n",
> +                                      clk->dev->name, ret);
>                                 return ret;
>                         }
>                 }
> -               if (clkp)
> -                       clkp->enable_count++;
> +               clk->enable_count++;
>         } else {
>                 if (!ops->enable)
>                         return -ENOSYS;
> @@ -582,7 +579,6 @@ int clk_enable_bulk(struct clk_bulk *bulk)
>  int clk_disable(struct clk *clk)
>  {
>         const struct clk_ops *ops;
> -       struct clk *clkp = NULL;
>         int ret;
>
>         debug("%s(clk=%p)\n", __func__, clk);
> @@ -591,29 +587,27 @@ int clk_disable(struct clk *clk)
>         ops = clk_dev_ops(clk->dev);
>
>         if (CONFIG_IS_ENABLED(CLK_CCF)) {
> -               if (clk->id && !clk_get_by_id(clk->id, &clkp)) {
> -                       if (clkp->enable_count == 0) {
> -                               printf("clk %s already disabled\n",
> -                                      clkp->dev->name);
> -                               return 0;
> -                       }
> -
> -                       if (--clkp->enable_count > 0)
> -                               return 0;
> +               if (clk->enable_count == 0) {
> +                       printf("clk %s already disabled\n",
> +                              clk->dev->name);
> +                       return 0;
>                 }
>
> +               if (--clk->enable_count > 0)
> +                       return 0;
> +
>                 if (ops->disable) {
>                         ret = ops->disable(clk);
>                         if (ret)
>                                 return ret;
>                 }
>
> -               if (clkp && clkp->dev->parent &&
> -                   device_get_uclass_id(clkp->dev) == UCLASS_CLK) {
> -                       ret = clk_disable(dev_get_clk_ptr(clkp->dev->parent));
> +               if (clk->dev->parent &&
> +                   device_get_uclass_id(clk->dev) == UCLASS_CLK) {
> +                       ret = clk_disable(dev_get_clk_ptr(clk->dev->parent));
>                         if (ret) {
> -                               printf("Disable %s failed\n",
> -                                      clkp->dev->parent->name);
> +                               printf("Disable %s failed (error %d)\n",
> +                                      clk->dev->parent->name, ret);
>                                 return ret;
>                         }
>                 }
> --
> 2.25.0
>