| Message ID | 1596034301-5428-7-git-send-email-claudiu.beznea@microchip.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Eugen Hristev |
| Headers | show |
| Series | clk: at91: add sama7g5 support | expand |
Hi Claudiu, On Wed, 29 Jul 2020 at 08:52, Claudiu Beznea <claudiu.beznea@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@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
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@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@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 >
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; + } + + 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",
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@microchip.com> --- drivers/clk/clk-uclass.c | 41 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 4 deletions(-)