| Message ID | 20200224071051.19331-1-p.yadav@ti.com |
|---|---|
| State | Accepted |
| Commit | 0a9c2874978a8468c92ed0dafe7a0cf239dc6a35 |
| Headers | show |
| Series | spi: cadence-qspi: Move ref clock calculation to probe | expand |
+Simon who converted driver to use clk_get* APIs On 24/02/20 12:40 pm, Pratyush Yadav wrote: > "assigned-clock-parents" and "assigned-clock-rates" DT properties take > effect only after ofdata_to_platdata() when clk_set_defaults() is called > in device_probe(). Therefore clk get rate() would return a wrong value > in ofdata_to_platdata() when compared with probe. Hence it needs to be > moved to probe. > > Tested on u-boot-ti/next. > Acked-by: Vignesh Raghavendra <vigneshr at ti.com> Regards Vignesh > Signed-off-by: Pratyush Yadav <p.yadav at ti.com> > --- > drivers/spi/cadence_qspi.c | 33 +++++++++++++++++---------------- > 1 file changed, 17 insertions(+), 16 deletions(-) > > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > index 83b114ffe7..994a5948f1 100644 > --- a/drivers/spi/cadence_qspi.c > +++ b/drivers/spi/cadence_qspi.c > @@ -166,11 +166,28 @@ static int cadence_spi_probe(struct udevice *bus) > { > struct cadence_spi_platdata *plat = bus->platdata; > struct cadence_spi_priv *priv = dev_get_priv(bus); > + struct clk clk; > int ret; > > priv->regbase = plat->regbase; > priv->ahbbase = plat->ahbbase; > > + if (plat->ref_clk_hz == 0) { > + ret = clk_get_by_index(bus, 0, &clk); > + if (ret) { > +#ifdef CONFIG_CQSPI_REF_CLK > + plat->ref_clk_hz = CONFIG_CQSPI_REF_CLK; > +#else > + return ret; > +#endif > + } else { > + plat->ref_clk_hz = clk_get_rate(&clk); > + clk_free(&clk); > + if (IS_ERR_VALUE(plat->ref_clk_hz)) > + return plat->ref_clk_hz; > + } > + } > + > ret = reset_get_bulk(bus, &priv->resets); > if (ret) > dev_warn(bus, "Can't get reset: %d\n", ret); > @@ -268,8 +285,6 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) > { > struct cadence_spi_platdata *plat = bus->platdata; > ofnode subnode; > - struct clk clk; > - int ret; > > plat->regbase = (void *)devfdt_get_addr_index(bus, 0); > plat->ahbbase = (void *)devfdt_get_addr_size_index(bus, 1, > @@ -305,20 +320,6 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) > plat->tchsh_ns = ofnode_read_u32_default(subnode, "cdns,tchsh-ns", 20); > plat->tslch_ns = ofnode_read_u32_default(subnode, "cdns,tslch-ns", 20); > > - ret = clk_get_by_index(bus, 0, &clk); > - if (ret) { > -#ifdef CONFIG_CQSPI_REF_CLK > - plat->ref_clk_hz = CONFIG_CQSPI_REF_CLK; > -#else > - return ret; > -#endif > - } else { > - plat->ref_clk_hz = clk_get_rate(&clk); > - clk_free(&clk); > - if (IS_ERR_VALUE(plat->ref_clk_hz)) > - return plat->ref_clk_hz; > - } > - > debug("%s: regbase=%p ahbbase=%p max-frequency=%d page-size=%d\n", > __func__, plat->regbase, plat->ahbbase, plat->max_hz, > plat->page_size); >
Vignesh Raghavendra <vigneshr at ti.com> schrieb am Mi., 26. Feb. 2020, 08:29: > +Simon who converted driver to use clk_get* APIs > > On 24/02/20 12:40 pm, Pratyush Yadav wrote: > > "assigned-clock-parents" and "assigned-clock-rates" DT properties take > > effect only after ofdata_to_platdata() when clk_set_defaults() is called > > in device_probe(). Therefore clk get rate() would return a wrong value > > in ofdata_to_platdata() when compared with probe. Hence it needs to be > > moved to probe. > > > > Tested on u-boot-ti/next. > > > > Acked-by: Vignesh Raghavendra <vigneshr at ti.com> > Fine by me. I actually moved it there after someone requested me to :-) I first had it in the set_rate function... Acked-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com> > > Regards > Vignesh > > > Signed-off-by: Pratyush Yadav <p.yadav at ti.com> > > --- > > drivers/spi/cadence_qspi.c | 33 +++++++++++++++++---------------- > > 1 file changed, 17 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > > index 83b114ffe7..994a5948f1 100644 > > --- a/drivers/spi/cadence_qspi.c > > +++ b/drivers/spi/cadence_qspi.c > > @@ -166,11 +166,28 @@ static int cadence_spi_probe(struct udevice *bus) > > { > > struct cadence_spi_platdata *plat = bus->platdata; > > struct cadence_spi_priv *priv = dev_get_priv(bus); > > + struct clk clk; > > int ret; > > > > priv->regbase = plat->regbase; > > priv->ahbbase = plat->ahbbase; > > > > + if (plat->ref_clk_hz == 0) { > > + ret = clk_get_by_index(bus, 0, &clk); > > + if (ret) { > > +#ifdef CONFIG_CQSPI_REF_CLK > > + plat->ref_clk_hz = CONFIG_CQSPI_REF_CLK; > > +#else > > + return ret; > > +#endif > > + } else { > > + plat->ref_clk_hz = clk_get_rate(&clk); > > + clk_free(&clk); > > + if (IS_ERR_VALUE(plat->ref_clk_hz)) > > + return plat->ref_clk_hz; > > + } > > + } > > + > > ret = reset_get_bulk(bus, &priv->resets); > > if (ret) > > dev_warn(bus, "Can't get reset: %d\n", ret); > > @@ -268,8 +285,6 @@ static int cadence_spi_ofdata_to_platdata(struct > udevice *bus) > > { > > struct cadence_spi_platdata *plat = bus->platdata; > > ofnode subnode; > > - struct clk clk; > > - int ret; > > > > plat->regbase = (void *)devfdt_get_addr_index(bus, 0); > > plat->ahbbase = (void *)devfdt_get_addr_size_index(bus, 1, > > @@ -305,20 +320,6 @@ static int cadence_spi_ofdata_to_platdata(struct > udevice *bus) > > plat->tchsh_ns = ofnode_read_u32_default(subnode, "cdns,tchsh-ns", > 20); > > plat->tslch_ns = ofnode_read_u32_default(subnode, "cdns,tslch-ns", > 20); > > > > - ret = clk_get_by_index(bus, 0, &clk); > > - if (ret) { > > -#ifdef CONFIG_CQSPI_REF_CLK > > - plat->ref_clk_hz = CONFIG_CQSPI_REF_CLK; > > -#else > > - return ret; > > -#endif > > - } else { > > - plat->ref_clk_hz = clk_get_rate(&clk); > > - clk_free(&clk); > > - if (IS_ERR_VALUE(plat->ref_clk_hz)) > > - return plat->ref_clk_hz; > > - } > > - > > debug("%s: regbase=%p ahbbase=%p max-frequency=%d page-size=%d\n", > > __func__, plat->regbase, plat->ahbbase, plat->max_hz, > > plat->page_size); > > > > >
On Wed, Feb 26, 2020 at 1:02 PM Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com> wrote: > > > > Vignesh Raghavendra <vigneshr at ti.com> schrieb am Mi., 26. Feb. 2020, 08:29: >> >> +Simon who converted driver to use clk_get* APIs >> >> On 24/02/20 12:40 pm, Pratyush Yadav wrote: >> > "assigned-clock-parents" and "assigned-clock-rates" DT properties take >> > effect only after ofdata_to_platdata() when clk_set_defaults() is called >> > in device_probe(). Therefore clk get rate() would return a wrong value >> > in ofdata_to_platdata() when compared with probe. Hence it needs to be >> > moved to probe. >> > >> > Tested on u-boot-ti/next. >> > >> >> Acked-by: Vignesh Raghavendra <vigneshr at ti.com> > > > Fine by me. I actually moved it there after someone requested me to :-) I first had it in the set_rate function... > > Acked-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com> So, does this fixing something or will it work even for wrong value? I'm thinking of picking it up for release, if associated boards got tested. Jagan.
On Mon, Feb 24, 2020 at 12:40 PM Pratyush Yadav <p.yadav at ti.com> wrote: > > "assigned-clock-parents" and "assigned-clock-rates" DT properties take > effect only after ofdata_to_platdata() when clk_set_defaults() is called > in device_probe(). Therefore clk get rate() would return a wrong value > in ofdata_to_platdata() when compared with probe. Hence it needs to be > moved to probe. > > Tested on u-boot-ti/next. > > Signed-off-by: Pratyush Yadav <p.yadav at ti.com> > --- Applied to u-boot-spi/master
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 83b114ffe7..994a5948f1 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -166,11 +166,28 @@ static int cadence_spi_probe(struct udevice *bus) { struct cadence_spi_platdata *plat = bus->platdata; struct cadence_spi_priv *priv = dev_get_priv(bus); + struct clk clk; int ret; priv->regbase = plat->regbase; priv->ahbbase = plat->ahbbase; + if (plat->ref_clk_hz == 0) { + ret = clk_get_by_index(bus, 0, &clk); + if (ret) { +#ifdef CONFIG_CQSPI_REF_CLK + plat->ref_clk_hz = CONFIG_CQSPI_REF_CLK; +#else + return ret; +#endif + } else { + plat->ref_clk_hz = clk_get_rate(&clk); + clk_free(&clk); + if (IS_ERR_VALUE(plat->ref_clk_hz)) + return plat->ref_clk_hz; + } + } + ret = reset_get_bulk(bus, &priv->resets); if (ret) dev_warn(bus, "Can't get reset: %d\n", ret); @@ -268,8 +285,6 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) { struct cadence_spi_platdata *plat = bus->platdata; ofnode subnode; - struct clk clk; - int ret; plat->regbase = (void *)devfdt_get_addr_index(bus, 0); plat->ahbbase = (void *)devfdt_get_addr_size_index(bus, 1, @@ -305,20 +320,6 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) plat->tchsh_ns = ofnode_read_u32_default(subnode, "cdns,tchsh-ns", 20); plat->tslch_ns = ofnode_read_u32_default(subnode, "cdns,tslch-ns", 20); - ret = clk_get_by_index(bus, 0, &clk); - if (ret) { -#ifdef CONFIG_CQSPI_REF_CLK - plat->ref_clk_hz = CONFIG_CQSPI_REF_CLK; -#else - return ret; -#endif - } else { - plat->ref_clk_hz = clk_get_rate(&clk); - clk_free(&clk); - if (IS_ERR_VALUE(plat->ref_clk_hz)) - return plat->ref_clk_hz; - } - debug("%s: regbase=%p ahbbase=%p max-frequency=%d page-size=%d\n", __func__, plat->regbase, plat->ahbbase, plat->max_hz, plat->page_size);
"assigned-clock-parents" and "assigned-clock-rates" DT properties take effect only after ofdata_to_platdata() when clk_set_defaults() is called in device_probe(). Therefore clk get rate() would return a wrong value in ofdata_to_platdata() when compared with probe. Hence it needs to be moved to probe. Tested on u-boot-ti/next. Signed-off-by: Pratyush Yadav <p.yadav at ti.com> --- drivers/spi/cadence_qspi.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-)