[U-Boot] [PATCH v1 4/5] drivers: clk: Fix using assigned-clocks in the node of the clock it sets up

[Simon Glass]

Hi Jean-Jacques,

On Mon, 30 Sep 2019 at 06:29, Jean-Jacques Hiblot <jjhiblot at ti.com> wrote:
>
> This fixes the case where assigned-clocks is used to define a clock
> defaults inside this same clock's node. This is used sometimes to setup a
> default parents and/or rate for a clock.
>
> example:
> muxed_clock: muxed_clock {
>         clocks = <&clk_provider 0>, <&clk_provider 1>;
>         #clock-cells = <0>;
>         assigned-clocks = <&muxed_clock>;
>         assigned-clock-parents = <&clk_provider 1>;
> };
>
> It doesn't work in u-boot because the assigned-clocks are setup *before*

U-Boot

> the clock is probed. (clk_set_parent() will likely crash or fail if called
> before the device probe function)
> Making it work by handling "assigned-clocks" in 2 steps: first before the
> clk device is probed, and then after the clk device is probed.
>
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot at ti.com>
> ---
>
>  drivers/clk/clk-uclass.c | 48 +++++++++++++++++++++++++++++++++++-----
>  drivers/core/device.c    |  2 +-
>  include/clk.h            |  7 ++++--
>  3 files changed, 48 insertions(+), 9 deletions(-)

Reviewed-by: Simon Glass <sjg at chromium.org>

+Stephen Warren who is the expert here

>
> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> index e7ec6347de..75d618a47b 100644
> --- a/drivers/clk/clk-uclass.c
> +++ b/drivers/clk/clk-uclass.c
> @@ -178,7 +178,7 @@ bulk_get_err:
>         return ret;
>  }
>
> -static int clk_set_default_parents(struct udevice *dev)
> +static int clk_set_default_parents(struct udevice *dev, int stage)
>  {
>         struct clk clk, parent_clk;
>         int index;
> @@ -214,8 +214,18 @@ static int clk_set_default_parents(struct udevice *dev)
>                         return ret;
>                 }
>
> -               ret = clk_set_parent(&clk, &parent_clk);
> +               /* This is clk provider device trying to reparent itself

/*
 * This ...

> +                * It cannot be done right now but need to wait after the
> +                * device is probed
> +                */
> +               if (stage == 0 && clk.dev == dev)
> +                       continue;
> +
> +               if (stage > 0 && clk.dev != dev)
> +                       /* do not setup twice the parent clocks */
> +                       continue;
>
> +               ret = clk_set_parent(&clk, &parent_clk);
>                 /*
>                  * Not all drivers may support clock-reparenting (as of now).
>                  * Ignore errors due to this.
> @@ -233,7 +243,7 @@ static int clk_set_default_parents(struct udevice *dev)
>         return 0;
>  }
>
> -static int clk_set_default_rates(struct udevice *dev)
> +static int clk_set_default_rates(struct udevice *dev, int stage)
>  {
>         struct clk clk;
>         int index;
> @@ -268,7 +278,19 @@ static int clk_set_default_rates(struct udevice *dev)
>                         continue;
>                 }
>
> +               /* This is clk provider device trying to program itself
> +                * It cannot be done right now but need to wait after the
> +                * device is probed
> +                */

wait until after?

Regards,
Simon