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.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.
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 >