Hi Claudiu, On Wed, 29 Jul 2020 at 08:52, Claudiu Beznea <claudiu.bez...@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.bez...@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. > + > + 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