| Message ID | 20191024182318.10295-1-goldsimon@gmx.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [U-Boot,v2] spi: cadence_qspi: support DM_CLK | expand |
Am 24.10.2019 um 20:23 schrieb Simon Goldschmidt: > From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > > Support loading clk speed via DM instead of requiring ad-hoc code. > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de> That gmx adress somehow slipped in after cloning u-boot-spi. Can you remove it when applying or should I resend? Regards, Simon > --- > > Changes in v2: > - check return value of clk_get_rate for error > > drivers/spi/cadence_qspi.c | 22 ++++++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > index e2e54cd277..60af99a16a 100644 > --- a/drivers/spi/cadence_qspi.c > +++ b/drivers/spi/cadence_qspi.c > @@ -5,6 +5,7 @@ > */ > > #include <common.h> > +#include <clk.h> > #include <dm.h> > #include <fdtdec.h> > #include <malloc.h> > @@ -22,12 +23,29 @@ static int cadence_spi_write_speed(struct udevice *bus, uint hz) > { > struct cadence_spi_platdata *plat = bus->platdata; > struct cadence_spi_priv *priv = dev_get_priv(bus); > + unsigned int ref_clk_hz; > + struct clk clk; > + int ret; > + > + ret = clk_get_by_index(bus, 0, &clk); > + if (ret) { > +#ifdef CONFIG_CQSPI_REF_CLK > + ref_clk_hz = CONFIG_CQSPI_REF_CLK; > +#else > + return ret; > +#endif > + } else { > + ref_clk_hz = clk_get_rate(&clk); > + clk_free(&clk); > + if (IS_ERR_VALUE(ref_clk_hz)) > + return ref_clk_hz; > + } > > cadence_qspi_apb_config_baudrate_div(priv->regbase, > - CONFIG_CQSPI_REF_CLK, hz); > + ref_clk_hz, hz); > > /* Reconfigure delay timing if speed is changed. */ > - cadence_qspi_apb_delay(priv->regbase, CONFIG_CQSPI_REF_CLK, hz, > + cadence_qspi_apb_delay(priv->regbase, ref_clk_hz, hz, > plat->tshsl_ns, plat->tsd2d_ns, > plat->tchsh_ns, plat->tslch_ns); > >
Hi Simon, On 24-Oct-19 11:53 PM, Simon Goldschmidt wrote: > From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > > Support loading clk speed via DM instead of requiring ad-hoc code. > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de> > --- [...] > @@ -22,12 +23,29 @@ static int cadence_spi_write_speed(struct udevice *bus, uint hz) > { > struct cadence_spi_platdata *plat = bus->platdata; > struct cadence_spi_priv *priv = dev_get_priv(bus); > + unsigned int ref_clk_hz; > + struct clk clk; > + int ret; > + > + ret = clk_get_by_index(bus, 0, &clk); > + if (ret) { > +#ifdef CONFIG_CQSPI_REF_CLK > + ref_clk_hz = CONFIG_CQSPI_REF_CLK; > +#else > + return ret; > +#endif > + } else { > + ref_clk_hz = clk_get_rate(&clk); > + clk_free(&clk); > + if (IS_ERR_VALUE(ref_clk_hz)) > + return ref_clk_hz; > + } > Can this be moved to probe function instead? cadence_spi_write_speed() is called multiple times from spi_calibration() and doing clk_get_by_index() and clk_get_rate() each time seems to be additional overhead. Regards Vignesh > cadence_qspi_apb_config_baudrate_div(priv->regbase, > - CONFIG_CQSPI_REF_CLK, hz); > + ref_clk_hz, hz); > > /* Reconfigure delay timing if speed is changed. */ > - cadence_qspi_apb_delay(priv->regbase, CONFIG_CQSPI_REF_CLK, hz, > + cadence_qspi_apb_delay(priv->regbase, ref_clk_hz, hz, > plat->tshsl_ns, plat->tsd2d_ns, > plat->tchsh_ns, plat->tslch_ns); > >
Vignesh Raghavendra <vigneshr@ti.com> schrieb am So., 10. Nov. 2019, 12:41: > Hi Simon, > > On 24-Oct-19 11:53 PM, Simon Goldschmidt wrote: > > From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > > > > Support loading clk speed via DM instead of requiring ad-hoc code. > > > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > > Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de> > > --- > [...] > > @@ -22,12 +23,29 @@ static int cadence_spi_write_speed(struct udevice > *bus, uint hz) > > { > > struct cadence_spi_platdata *plat = bus->platdata; > > struct cadence_spi_priv *priv = dev_get_priv(bus); > > + unsigned int ref_clk_hz; > > + struct clk clk; > > + int ret; > > + > > + ret = clk_get_by_index(bus, 0, &clk); > > + if (ret) { > > +#ifdef CONFIG_CQSPI_REF_CLK > > + ref_clk_hz = CONFIG_CQSPI_REF_CLK; > > +#else > > + return ret; > > +#endif > > + } else { > > + ref_clk_hz = clk_get_rate(&clk); > > + clk_free(&clk); > > + if (IS_ERR_VALUE(ref_clk_hz)) > > + return ref_clk_hz; > > + } > > > > Can this be moved to probe function instead? cadence_spi_write_speed() > is called multiple times from spi_calibration() and doing > clk_get_by_index() and clk_get_rate() each time seems to be additional > overhead. > Sure, that would indeed be better. Regards, Simon > Regards > Vignesh > > > > cadence_qspi_apb_config_baudrate_div(priv->regbase, > > - CONFIG_CQSPI_REF_CLK, hz); > > + ref_clk_hz, hz); > > > > /* Reconfigure delay timing if speed is changed. */ > > - cadence_qspi_apb_delay(priv->regbase, CONFIG_CQSPI_REF_CLK, hz, > > + cadence_qspi_apb_delay(priv->regbase, ref_clk_hz, hz, > > plat->tshsl_ns, plat->tsd2d_ns, > > plat->tchsh_ns, plat->tslch_ns); > > > > >
On 10/11/19 5:11 PM, Vignesh Raghavendra wrote: > Hi Simon, > > On 24-Oct-19 11:53 PM, Simon Goldschmidt wrote: >> From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> >> >> Support loading clk speed via DM instead of requiring ad-hoc code. >> >> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> >> Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de> >> --- > [...] >> @@ -22,12 +23,29 @@ static int cadence_spi_write_speed(struct udevice *bus, uint hz) >> { >> struct cadence_spi_platdata *plat = bus->platdata; >> struct cadence_spi_priv *priv = dev_get_priv(bus); >> + unsigned int ref_clk_hz; >> + struct clk clk; >> + int ret; >> + >> + ret = clk_get_by_index(bus, 0, &clk); >> + if (ret) { >> +#ifdef CONFIG_CQSPI_REF_CLK We also could get rid of CONFIG_CQSPI_REF_CLK altogether. Instead pass frequency from DT or platdata using "clock-frequency" property like serial drivers do, assuming all platforms now use DT or platdata (all TI platforms using this driver support DT). But that can be done in a separate patch series... >> + ref_clk_hz = CONFIG_CQSPI_REF_CLK; >> +#else >> + return ret; >> +#endif >> + } else { >> + ref_clk_hz = clk_get_rate(&clk); >> + clk_free(&clk); >> + if (IS_ERR_VALUE(ref_clk_hz)) >> + return ref_clk_hz; >> + } >> [...]
Vignesh Raghavendra <vigneshr@ti.com> schrieb am Mo., 11. Nov. 2019, 05:22: > > > On 10/11/19 5:11 PM, Vignesh Raghavendra wrote: > > Hi Simon, > > > > On 24-Oct-19 11:53 PM, Simon Goldschmidt wrote: > >> From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > >> > >> Support loading clk speed via DM instead of requiring ad-hoc code. > >> > >> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > >> Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de> > >> --- > > [...] > >> @@ -22,12 +23,29 @@ static int cadence_spi_write_speed(struct udevice > *bus, uint hz) > >> { > >> struct cadence_spi_platdata *plat = bus->platdata; > >> struct cadence_spi_priv *priv = dev_get_priv(bus); > >> + unsigned int ref_clk_hz; > >> + struct clk clk; > >> + int ret; > >> + > >> + ret = clk_get_by_index(bus, 0, &clk); > >> + if (ret) { > >> +#ifdef CONFIG_CQSPI_REF_CLK > > We also could get rid of CONFIG_CQSPI_REF_CLK altogether. Instead pass > frequency from DT or platdata using "clock-frequency" property like > serial drivers do, assuming all platforms now use DT or platdata (all TI > platforms using this driver support DT). > But that can be done in a separate patch series... > My next step for socfpga is to provide a DM_CLK driver, so that would remove the need for this define altogether (for that platform). Regards, Simon > > >> + ref_clk_hz = CONFIG_CQSPI_REF_CLK; > >> +#else > >> + return ret; > >> +#endif > >> + } else { > >> + ref_clk_hz = clk_get_rate(&clk); > >> + clk_free(&clk); > >> + if (IS_ERR_VALUE(ref_clk_hz)) > >> + return ref_clk_hz; > >> + } > >> > [...] > > -- > Regards > Vignesh >
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index e2e54cd277..60af99a16a 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -5,6 +5,7 @@ */ #include <common.h> +#include <clk.h> #include <dm.h> #include <fdtdec.h> #include <malloc.h> @@ -22,12 +23,29 @@ static int cadence_spi_write_speed(struct udevice *bus, uint hz) { struct cadence_spi_platdata *plat = bus->platdata; struct cadence_spi_priv *priv = dev_get_priv(bus); + unsigned int ref_clk_hz; + struct clk clk; + int ret; + + ret = clk_get_by_index(bus, 0, &clk); + if (ret) { +#ifdef CONFIG_CQSPI_REF_CLK + ref_clk_hz = CONFIG_CQSPI_REF_CLK; +#else + return ret; +#endif + } else { + ref_clk_hz = clk_get_rate(&clk); + clk_free(&clk); + if (IS_ERR_VALUE(ref_clk_hz)) + return ref_clk_hz; + } cadence_qspi_apb_config_baudrate_div(priv->regbase, - CONFIG_CQSPI_REF_CLK, hz); + ref_clk_hz, hz); /* Reconfigure delay timing if speed is changed. */ - cadence_qspi_apb_delay(priv->regbase, CONFIG_CQSPI_REF_CLK, hz, + cadence_qspi_apb_delay(priv->regbase, ref_clk_hz, hz, plat->tshsl_ns, plat->tsd2d_ns, plat->tchsh_ns, plat->tslch_ns);