[PATCH 06/22] clk: get clock pointer before proceeding

[Claudiu.Beznea at microchip.com]

On 04.08.2020 05:00, Simon Glass wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Claudiu,
> 
> On Wed, 29 Jul 2020 at 08:52, Claudiu Beznea
> <claudiu.beznea at microchip.com> wrote:
>>
>> clk_get_by_indexed_prop() retrieves a clock with dev member being set
>> with the pointer to the udevice for the clock controller driver. But
>> in case of CCF each struct clk object has set in dev member the reference
>> to its parent (the root of the clock tree is a fixed clock, every
>> node in clock tree is a clock registered with clk_register()). In this
>> case the subsequent operations like dev_get_clk_ptr() on clocks
>> retrieved by clk_get_by_indexed_prop() will fail. For this, get the
>> pointer to the proper clock registered (with clk_register()) using
>> clk_get_by_id() before proceeding.
>>
>> Fixes: 1d7993d1d0ef ("clk: Port Linux common clock framework [CCF] for imx6q to U-boot (tag: v5.1.12)")
>> Signed-off-by: Claudiu Beznea <claudiu.beznea at microchip.com>
>> ---
>>  drivers/clk/clk-uclass.c | 41 +++++++++++++++++++++++++++++++++++++----
>>  1 file changed, 37 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
>> index 958a9490bee2..8f926aad12cf 100644
>> --- a/drivers/clk/clk-uclass.c
>> +++ b/drivers/clk/clk-uclass.c
>> @@ -186,7 +186,7 @@ bulk_get_err:
>>
>>  static int clk_set_default_parents(struct udevice *dev, int stage)
>>  {
>> -       struct clk clk, parent_clk;
>> +       struct clk clk, parent_clk, *c, *p;
>>         int index;
>>         int num_parents;
>>         int ret;
>> @@ -212,6 +212,17 @@ static int clk_set_default_parents(struct udevice *dev, int stage)
>>                         return ret;
>>                 }
>>
>> +               if (CONFIG_IS_ENABLED(CLK_CCF)) {
>> +                       ret = clk_get_by_id(parent_clk.id, &p);
>> +                       if (ret) {
>> +                               debug("%s(): could not get parent clock pointer, id %lu, for %s\n",
>> +                                     __func__, parent_clk.id, dev_read_name(dev));
>> +                               return ret;
>> +                       }
>> +               } else {
>> +                       p = &parent_clk;
>> +               }
>> +
>>                 ret = clk_get_by_indexed_prop(dev, "assigned-clocks",
>>                                               index, &clk);
>>                 if (ret) {
>> @@ -231,7 +242,18 @@ static int clk_set_default_parents(struct udevice *dev, int stage)
>>                         /* do not setup twice the parent clocks */
>>                         continue;
>>
>> -               ret = clk_set_parent(&clk, &parent_clk);
>> +               if (CONFIG_IS_ENABLED(CLK_CCF)) {
>> +                       ret = clk_get_by_id(clk.id, &c);
>> +                       if (ret) {
>> +                               debug("%s(): could not get clock pointer, id %lu, for %s\n",
>> +                                     __func__, clk.id, dev_read_name(dev));
>> +                               return ret;
>> +                       }
>> +               } else {
>> +                       c = &clk;
>> +               }
> 
> Could this code go in a function? It seems to be repeated three times.

Sure, it will!

Thank you reviewing this,
Claudiu Beznea

> 
>> +
>> +               ret = clk_set_parent(c, p);
>>                 /*
>>                  * Not all drivers may support clock-reparenting (as of now).
>>                  * Ignore errors due to this.
>> @@ -251,7 +273,7 @@ static int clk_set_default_parents(struct udevice *dev, int stage)
>>
>>  static int clk_set_default_rates(struct udevice *dev, int stage)
>>  {
>> -       struct clk clk;
>> +       struct clk clk, *c;
>>         int index;
>>         int num_rates;
>>         int size;
>> @@ -295,7 +317,18 @@ static int clk_set_default_rates(struct udevice *dev, int stage)
>>                         /* do not setup twice the parent clocks */
>>                         continue;
>>
>> -               ret = clk_set_rate(&clk, rates[index]);
>> +               if (CONFIG_IS_ENABLED(CLK_CCF)) {
>> +                       ret = clk_get_by_id(clk.id, &c);
>> +                       if (ret) {
>> +                               debug("%s(): could not get clock pointer, id %lu, for %s\n",
>> +                                     __func__, clk.id, dev_read_name(dev));
>> +                               return ret;
>> +                       }
>> +               } else {
>> +                       c = &clk;
>> +               }
>> +
>> +               ret = clk_set_rate(c, rates[index]);
>>
>>                 if (ret < 0) {
>>                         debug("%s: failed to set rate on clock index %d (%ld) for %s\n",
>> --
>> 2.7.4
>>
> 
> Regards,
> Simon
>