| Message ID | 20191111214251.4112-1-simon.k.r.goldschmidt@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | Simon Goldschmidt |
| Headers | show |
| Series | [U-Boot,v3] spi: cadence_qspi: support DM_CLK | expand |
> -----Original Message----- > From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > Sent: Tuesday, November 12, 2019 5:43 AM > To: Jagan Teki <jagan@amarulasolutions.com> > Cc: Marek Vasut <marex@denx.de>; Tan, Ley Foon > <ley.foon.tan@intel.com>; Vignesh Raghavendra <vigneshr@ti.com>; Simon > Goldschmidt <simon.k.r.goldschmidt@gmail.com>; u-boot@lists.denx.de > Subject: [PATCH v3] spi: cadence_qspi: support DM_CLK > > Support loading clk speed via DM instead of requiring ad-hoc code. > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > --- > > Changes in v3: > - load ref_clk_hz only once in cadence_spi_ofdata_to_platdata instead > of loading it every time in cadence_spi_write_speed > > Changes in v2: > - check return value of clk_get_rate for error > > drivers/spi/cadence_qspi.c | 21 +++++++++++++++++++-- > drivers/spi/cadence_qspi.h | 1 + > 2 files changed, 20 insertions(+), 2 deletions(-) > > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index > e2e54cd277..8fd23a7702 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> > @@ -24,10 +25,10 @@ static int cadence_spi_write_speed(struct udevice > *bus, uint hz) > struct cadence_spi_priv *priv = dev_get_priv(bus); > > cadence_qspi_apb_config_baudrate_div(priv->regbase, > - CONFIG_CQSPI_REF_CLK, hz); > + plat->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, plat->ref_clk_hz, hz, > plat->tshsl_ns, plat->tsd2d_ns, > plat->tchsh_ns, plat->tslch_ns); > > @@ -294,6 +295,8 @@ 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_index(bus, 1); @@ -325,6 > +328,20 @@ 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); > Did you compile this with platform without clock DM before? Eg: Stratix10. You need add check for CONFIG_CLK enabled to call clock DM functions here. Regards Ley Foon > + 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); > diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h index > 20cceca239..99dee75bbd 100644 > --- a/drivers/spi/cadence_qspi.h > +++ b/drivers/spi/cadence_qspi.h > @@ -16,6 +16,7 @@ > #define CQSPI_READ_CAPTURE_MAX_DELAY 16 > > struct cadence_spi_platdata { > + unsigned int ref_clk_hz; > unsigned int max_hz; > void *regbase; > void *ahbbase; > -- > 2.20.1
On Tue, Nov 12, 2019 at 9:59 AM Tan, Ley Foon <ley.foon.tan@intel.com> wrote: > > > > > -----Original Message----- > > From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > > Sent: Tuesday, November 12, 2019 5:43 AM > > To: Jagan Teki <jagan@amarulasolutions.com> > > Cc: Marek Vasut <marex@denx.de>; Tan, Ley Foon > > <ley.foon.tan@intel.com>; Vignesh Raghavendra <vigneshr@ti.com>; Simon > > Goldschmidt <simon.k.r.goldschmidt@gmail.com>; u-boot@lists.denx.de > > Subject: [PATCH v3] spi: cadence_qspi: support DM_CLK > > > > Support loading clk speed via DM instead of requiring ad-hoc code. > > > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > > --- > > > > Changes in v3: > > - load ref_clk_hz only once in cadence_spi_ofdata_to_platdata instead > > of loading it every time in cadence_spi_write_speed > > > > Changes in v2: > > - check return value of clk_get_rate for error > > > > drivers/spi/cadence_qspi.c | 21 +++++++++++++++++++-- > > drivers/spi/cadence_qspi.h | 1 + > > 2 files changed, 20 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index > > e2e54cd277..8fd23a7702 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> > > @@ -24,10 +25,10 @@ static int cadence_spi_write_speed(struct udevice > > *bus, uint hz) > > struct cadence_spi_priv *priv = dev_get_priv(bus); > > > > cadence_qspi_apb_config_baudrate_div(priv->regbase, > > - CONFIG_CQSPI_REF_CLK, hz); > > + plat->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, plat->ref_clk_hz, hz, > > plat->tshsl_ns, plat->tsd2d_ns, > > plat->tchsh_ns, plat->tslch_ns); > > > > @@ -294,6 +295,8 @@ 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_index(bus, 1); @@ -325,6 > > +328,20 @@ 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); > > > Did you compile this with platform without clock DM before? Eg: Stratix10. > You need add check for CONFIG_CLK enabled to call clock DM functions here. Unless I'm mistaken, those functions are prototyped when CLK is not enabled: https://elixir.bootlin.com/u-boot/latest/source/include/clk.h#L172 That should be enough, no? And yes, I did test this on the current state of gen5 which does not have a CLK driver, yet. Regards, Simon > > Regards > Ley Foon > > > + 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); > > diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h index > > 20cceca239..99dee75bbd 100644 > > --- a/drivers/spi/cadence_qspi.h > > +++ b/drivers/spi/cadence_qspi.h > > @@ -16,6 +16,7 @@ > > #define CQSPI_READ_CAPTURE_MAX_DELAY 16 > > > > struct cadence_spi_platdata { > > + unsigned int ref_clk_hz; > > unsigned int max_hz; > > void *regbase; > > void *ahbbase; > > -- > > 2.20.1 >
On 12/11/19 2:44 PM, Simon Goldschmidt wrote: > On Tue, Nov 12, 2019 at 9:59 AM Tan, Ley Foon <ley.foon.tan@intel.com> wrote: >> >> >> >>> -----Original Message----- >>> From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> >>> Sent: Tuesday, November 12, 2019 5:43 AM >>> To: Jagan Teki <jagan@amarulasolutions.com> >>> Cc: Marek Vasut <marex@denx.de>; Tan, Ley Foon >>> <ley.foon.tan@intel.com>; Vignesh Raghavendra <vigneshr@ti.com>; Simon >>> Goldschmidt <simon.k.r.goldschmidt@gmail.com>; u-boot@lists.denx.de >>> Subject: [PATCH v3] spi: cadence_qspi: support DM_CLK >>> >>> Support loading clk speed via DM instead of requiring ad-hoc code. >>> >>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> >>> --- >>> >>> Changes in v3: >>> - load ref_clk_hz only once in cadence_spi_ofdata_to_platdata instead >>> of loading it every time in cadence_spi_write_speed >>> >>> Changes in v2: >>> - check return value of clk_get_rate for error >>> >>> drivers/spi/cadence_qspi.c | 21 +++++++++++++++++++-- >>> drivers/spi/cadence_qspi.h | 1 + >>> 2 files changed, 20 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index >>> e2e54cd277..8fd23a7702 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> >>> @@ -24,10 +25,10 @@ static int cadence_spi_write_speed(struct udevice >>> *bus, uint hz) >>> struct cadence_spi_priv *priv = dev_get_priv(bus); >>> >>> cadence_qspi_apb_config_baudrate_div(priv->regbase, >>> - CONFIG_CQSPI_REF_CLK, hz); >>> + plat->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, plat->ref_clk_hz, hz, >>> plat->tshsl_ns, plat->tsd2d_ns, >>> plat->tchsh_ns, plat->tslch_ns); >>> >>> @@ -294,6 +295,8 @@ 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_index(bus, 1); @@ -325,6 >>> +328,20 @@ 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); >>> >> Did you compile this with platform without clock DM before? Eg: Stratix10. >> You need add check for CONFIG_CLK enabled to call clock DM functions here. > > Unless I'm mistaken, those functions are prototyped when CLK is not enabled: > > https://elixir.bootlin.com/u-boot/latest/source/include/clk.h#L172 > But, unfortunately, such stub does not exists for clk_get_rate(). So on platforms w/o CONFIG_CLK set: arm-linux-gnueabihf-ld.bfd: drivers/spi/built-in.o: in function `cadence_spi_probe': /home/a0132425/workspace/u-boot/drivers/spi/cadence_qspi.c:184: undefined reference to `clk_get_rate' Makefile:1647: recipe for target 'u-boot' failed make: *** [u-boot] Error 1 Regards Vignesh > That should be enough, no? And yes, I did test this on the current state of > gen5 which does not have a CLK driver, yet. > > Regards, > Simon > >> >> Regards >> Ley Foon >> >>> + 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); >>> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h index >>> 20cceca239..99dee75bbd 100644 >>> --- a/drivers/spi/cadence_qspi.h >>> +++ b/drivers/spi/cadence_qspi.h >>> @@ -16,6 +16,7 @@ >>> #define CQSPI_READ_CAPTURE_MAX_DELAY 16 >>> >>> struct cadence_spi_platdata { >>> + unsigned int ref_clk_hz; >>> unsigned int max_hz; >>> void *regbase; >>> void *ahbbase; >>> -- >>> 2.20.1 >>
> -----Original Message----- > From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > Sent: Tuesday, November 12, 2019 5:14 PM > To: Tan, Ley Foon <ley.foon.tan@intel.com> > Cc: Jagan Teki <jagan@amarulasolutions.com>; Marek Vasut > <marex@denx.de>; Vignesh Raghavendra <vigneshr@ti.com>; u- > boot@lists.denx.de > Subject: Re: [PATCH v3] spi: cadence_qspi: support DM_CLK > > On Tue, Nov 12, 2019 at 9:59 AM Tan, Ley Foon <ley.foon.tan@intel.com> > wrote: > > > > > > > > > -----Original Message----- > > > From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > > > Sent: Tuesday, November 12, 2019 5:43 AM > > > To: Jagan Teki <jagan@amarulasolutions.com> > > > Cc: Marek Vasut <marex@denx.de>; Tan, Ley Foon > > > <ley.foon.tan@intel.com>; Vignesh Raghavendra <vigneshr@ti.com>; > > > Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>; > > > u-boot@lists.denx.de > > > Subject: [PATCH v3] spi: cadence_qspi: support DM_CLK > > > > > > Support loading clk speed via DM instead of requiring ad-hoc code. > > > > > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > > > --- > > > > > > Changes in v3: > > > - load ref_clk_hz only once in cadence_spi_ofdata_to_platdata instead > > > of loading it every time in cadence_spi_write_speed > > > > > > Changes in v2: > > > - check return value of clk_get_rate for error > > > > > > drivers/spi/cadence_qspi.c | 21 +++++++++++++++++++-- > > > drivers/spi/cadence_qspi.h | 1 + > > > 2 files changed, 20 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > > > index > > > e2e54cd277..8fd23a7702 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> > > > @@ -24,10 +25,10 @@ static int cadence_spi_write_speed(struct > > > udevice *bus, uint hz) > > > struct cadence_spi_priv *priv = dev_get_priv(bus); > > > > > > cadence_qspi_apb_config_baudrate_div(priv->regbase, > > > - CONFIG_CQSPI_REF_CLK, hz); > > > + plat->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, plat->ref_clk_hz, hz, > > > plat->tshsl_ns, plat->tsd2d_ns, > > > plat->tchsh_ns, plat->tslch_ns); > > > > > > @@ -294,6 +295,8 @@ 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_index(bus, 1); @@ > > > -325,6 > > > +328,20 @@ 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); > > > > > Did you compile this with platform without clock DM before? Eg: Stratix10. > > You need add check for CONFIG_CLK enabled to call clock DM functions > here. > > Unless I'm mistaken, those functions are prototyped when CLK is not enabled: > > https://elixir.bootlin.com/u-boot/latest/source/include/clk.h#L172 > > That should be enough, no? And yes, I did test this on the current state of > gen5 which does not have a CLK driver, yet. > I also saw related clock issue in another patch review: https://patchwork.ozlabs.org/cover/1191936/ Some functions in clock DM doesn't have non-DM version functions wrapper, eg: clk_get_rate Regards Ley Foon > > > > > Regards > > Ley Foon > > > > > + 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); > > > diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h > > > index 20cceca239..99dee75bbd 100644 > > > --- a/drivers/spi/cadence_qspi.h > > > +++ b/drivers/spi/cadence_qspi.h > > > @@ -16,6 +16,7 @@ > > > #define CQSPI_READ_CAPTURE_MAX_DELAY 16 > > > > > > struct cadence_spi_platdata { > > > + unsigned int ref_clk_hz; > > > unsigned int max_hz; > > > void *regbase; > > > void *ahbbase; > > > -- > > > 2.20.1 > >
On Tue, Nov 12, 2019 at 10:22 AM Vignesh Raghavendra <vigneshr@ti.com> wrote: > > > > On 12/11/19 2:44 PM, Simon Goldschmidt wrote: > > On Tue, Nov 12, 2019 at 9:59 AM Tan, Ley Foon <ley.foon.tan@intel.com> wrote: > >> > >> > >> > >>> -----Original Message----- > >>> From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > >>> Sent: Tuesday, November 12, 2019 5:43 AM > >>> To: Jagan Teki <jagan@amarulasolutions.com> > >>> Cc: Marek Vasut <marex@denx.de>; Tan, Ley Foon > >>> <ley.foon.tan@intel.com>; Vignesh Raghavendra <vigneshr@ti.com>; Simon > >>> Goldschmidt <simon.k.r.goldschmidt@gmail.com>; u-boot@lists.denx.de > >>> Subject: [PATCH v3] spi: cadence_qspi: support DM_CLK > >>> > >>> Support loading clk speed via DM instead of requiring ad-hoc code. > >>> > >>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > >>> --- > >>> > >>> Changes in v3: > >>> - load ref_clk_hz only once in cadence_spi_ofdata_to_platdata instead > >>> of loading it every time in cadence_spi_write_speed > >>> > >>> Changes in v2: > >>> - check return value of clk_get_rate for error > >>> > >>> drivers/spi/cadence_qspi.c | 21 +++++++++++++++++++-- > >>> drivers/spi/cadence_qspi.h | 1 + > >>> 2 files changed, 20 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index > >>> e2e54cd277..8fd23a7702 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> > >>> @@ -24,10 +25,10 @@ static int cadence_spi_write_speed(struct udevice > >>> *bus, uint hz) > >>> struct cadence_spi_priv *priv = dev_get_priv(bus); > >>> > >>> cadence_qspi_apb_config_baudrate_div(priv->regbase, > >>> - CONFIG_CQSPI_REF_CLK, hz); > >>> + plat->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, plat->ref_clk_hz, hz, > >>> plat->tshsl_ns, plat->tsd2d_ns, > >>> plat->tchsh_ns, plat->tslch_ns); > >>> > >>> @@ -294,6 +295,8 @@ 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_index(bus, 1); @@ -325,6 > >>> +328,20 @@ 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); > >>> > >> Did you compile this with platform without clock DM before? Eg: Stratix10. > >> You need add check for CONFIG_CLK enabled to call clock DM functions here. > > > > Unless I'm mistaken, those functions are prototyped when CLK is not enabled: > > > > https://elixir.bootlin.com/u-boot/latest/source/include/clk.h#L172 > > > > But, unfortunately, such stub does not exists for clk_get_rate(). > So on platforms w/o CONFIG_CLK set: > > arm-linux-gnueabihf-ld.bfd: drivers/spi/built-in.o: in function `cadence_spi_probe': > /home/a0132425/workspace/u-boot/drivers/spi/cadence_qspi.c:184: undefined reference to `clk_get_rate' > Makefile:1647: recipe for target 'u-boot' failed > make: *** [u-boot] Error 1 So why did it compile for me? Probably because the linker knows it doesn't need 'clk_get_rate' since this branch will never be executed? Regards, Simon > > Regards > Vignesh > > > That should be enough, no? And yes, I did test this on the current state of > > gen5 which does not have a CLK driver, yet. > > > > Regards, > > Simon > > > >> > >> Regards > >> Ley Foon > >> > >>> + 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); > >>> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h index > >>> 20cceca239..99dee75bbd 100644 > >>> --- a/drivers/spi/cadence_qspi.h > >>> +++ b/drivers/spi/cadence_qspi.h > >>> @@ -16,6 +16,7 @@ > >>> #define CQSPI_READ_CAPTURE_MAX_DELAY 16 > >>> > >>> struct cadence_spi_platdata { > >>> + unsigned int ref_clk_hz; > >>> unsigned int max_hz; > >>> void *regbase; > >>> void *ahbbase; > >>> -- > >>> 2.20.1 > >>
> -----Original Message----- > From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > Sent: Tuesday, November 12, 2019 5:27 PM > To: Vignesh Raghavendra <vigneshr@ti.com> > Cc: Tan, Ley Foon <ley.foon.tan@intel.com>; Jagan Teki > <jagan@amarulasolutions.com>; Marek Vasut <marex@denx.de>; u- > boot@lists.denx.de > Subject: Re: [PATCH v3] spi: cadence_qspi: support DM_CLK > > On Tue, Nov 12, 2019 at 10:22 AM Vignesh Raghavendra <vigneshr@ti.com> > wrote: > > > > > > > > On 12/11/19 2:44 PM, Simon Goldschmidt wrote: > > > On Tue, Nov 12, 2019 at 9:59 AM Tan, Ley Foon <ley.foon.tan@intel.com> > wrote: > > >> > > >> > > >> > > >>> -----Original Message----- > > >>> From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > > >>> Sent: Tuesday, November 12, 2019 5:43 AM > > >>> To: Jagan Teki <jagan@amarulasolutions.com> > > >>> Cc: Marek Vasut <marex@denx.de>; Tan, Ley Foon > > >>> <ley.foon.tan@intel.com>; Vignesh Raghavendra <vigneshr@ti.com>; > > >>> Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>; > > >>> u-boot@lists.denx.de > > >>> Subject: [PATCH v3] spi: cadence_qspi: support DM_CLK > > >>> > > >>> Support loading clk speed via DM instead of requiring ad-hoc code. > > >>> > > >>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > > >>> --- > > >>> > > >>> Changes in v3: > > >>> - load ref_clk_hz only once in cadence_spi_ofdata_to_platdata instead > > >>> of loading it every time in cadence_spi_write_speed > > >>> > > >>> Changes in v2: > > >>> - check return value of clk_get_rate for error > > >>> > > >>> drivers/spi/cadence_qspi.c | 21 +++++++++++++++++++-- > > >>> drivers/spi/cadence_qspi.h | 1 + > > >>> 2 files changed, 20 insertions(+), 2 deletions(-) > > >>> > > >>> diff --git a/drivers/spi/cadence_qspi.c > > >>> b/drivers/spi/cadence_qspi.c index > > >>> e2e54cd277..8fd23a7702 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> > > >>> @@ -24,10 +25,10 @@ static int cadence_spi_write_speed(struct > > >>> udevice *bus, uint hz) > > >>> struct cadence_spi_priv *priv = dev_get_priv(bus); > > >>> > > >>> cadence_qspi_apb_config_baudrate_div(priv->regbase, > > >>> - CONFIG_CQSPI_REF_CLK, hz); > > >>> + plat->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, plat->ref_clk_hz, hz, > > >>> plat->tshsl_ns, plat->tsd2d_ns, > > >>> plat->tchsh_ns, plat->tslch_ns); > > >>> > > >>> @@ -294,6 +295,8 @@ 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_index(bus, 1); @@ > > >>> -325,6 > > >>> +328,20 @@ 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); > > >>> > > >> Did you compile this with platform without clock DM before? Eg: > Stratix10. > > >> You need add check for CONFIG_CLK enabled to call clock DM functions > here. > > > > > > Unless I'm mistaken, those functions are prototyped when CLK is not > enabled: > > > > > > https://elixir.bootlin.com/u-boot/latest/source/include/clk.h#L172 > > > > > > > But, unfortunately, such stub does not exists for clk_get_rate(). > > So on platforms w/o CONFIG_CLK set: > > > > arm-linux-gnueabihf-ld.bfd: drivers/spi/built-in.o: in function > `cadence_spi_probe': > > /home/a0132425/workspace/u-boot/drivers/spi/cadence_qspi.c:184: > undefined reference to `clk_get_rate' > > Makefile:1647: recipe for target 'u-boot' failed > > make: *** [u-boot] Error 1 > > So why did it compile for me? Probably because the linker knows it doesn't > need 'clk_get_rate' since this branch will never be executed? Maybe you can try compile from clean build. Run "make mrproper" before compile. Regards Ley Foon > > Regards, > Simon > > > > > Regards > > Vignesh > > > > > That should be enough, no? And yes, I did test this on the current > > > state of > > > gen5 which does not have a CLK driver, yet. > > > > > > Regards, > > > Simon > > > > > >> > > >> Regards > > >> Ley Foon > > >> > > >>> + 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); > > >>> diff --git a/drivers/spi/cadence_qspi.h > > >>> b/drivers/spi/cadence_qspi.h index 20cceca239..99dee75bbd 100644 > > >>> --- a/drivers/spi/cadence_qspi.h > > >>> +++ b/drivers/spi/cadence_qspi.h > > >>> @@ -16,6 +16,7 @@ > > >>> #define CQSPI_READ_CAPTURE_MAX_DELAY 16 > > >>> > > >>> struct cadence_spi_platdata { > > >>> + unsigned int ref_clk_hz; > > >>> unsigned int max_hz; > > >>> void *regbase; > > >>> void *ahbbase; > > >>> -- > > >>> 2.20.1 > > >>
On Tue, Nov 12, 2019 at 10:30 AM Tan, Ley Foon <ley.foon.tan@intel.com> wrote: > > > > > -----Original Message----- > > From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > > Sent: Tuesday, November 12, 2019 5:27 PM > > To: Vignesh Raghavendra <vigneshr@ti.com> > > Cc: Tan, Ley Foon <ley.foon.tan@intel.com>; Jagan Teki > > <jagan@amarulasolutions.com>; Marek Vasut <marex@denx.de>; u- > > boot@lists.denx.de > > Subject: Re: [PATCH v3] spi: cadence_qspi: support DM_CLK > > > > On Tue, Nov 12, 2019 at 10:22 AM Vignesh Raghavendra <vigneshr@ti.com> > > wrote: > > > > > > > > > > > > On 12/11/19 2:44 PM, Simon Goldschmidt wrote: > > > > On Tue, Nov 12, 2019 at 9:59 AM Tan, Ley Foon <ley.foon.tan@intel.com> > > wrote: > > > >> > > > >> > > > >> > > > >>> -----Original Message----- > > > >>> From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > > > >>> Sent: Tuesday, November 12, 2019 5:43 AM > > > >>> To: Jagan Teki <jagan@amarulasolutions.com> > > > >>> Cc: Marek Vasut <marex@denx.de>; Tan, Ley Foon > > > >>> <ley.foon.tan@intel.com>; Vignesh Raghavendra <vigneshr@ti.com>; > > > >>> Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>; > > > >>> u-boot@lists.denx.de > > > >>> Subject: [PATCH v3] spi: cadence_qspi: support DM_CLK > > > >>> > > > >>> Support loading clk speed via DM instead of requiring ad-hoc code. > > > >>> > > > >>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > > > >>> --- > > > >>> > > > >>> Changes in v3: > > > >>> - load ref_clk_hz only once in cadence_spi_ofdata_to_platdata instead > > > >>> of loading it every time in cadence_spi_write_speed > > > >>> > > > >>> Changes in v2: > > > >>> - check return value of clk_get_rate for error > > > >>> > > > >>> drivers/spi/cadence_qspi.c | 21 +++++++++++++++++++-- > > > >>> drivers/spi/cadence_qspi.h | 1 + > > > >>> 2 files changed, 20 insertions(+), 2 deletions(-) > > > >>> > > > >>> diff --git a/drivers/spi/cadence_qspi.c > > > >>> b/drivers/spi/cadence_qspi.c index > > > >>> e2e54cd277..8fd23a7702 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> > > > >>> @@ -24,10 +25,10 @@ static int cadence_spi_write_speed(struct > > > >>> udevice *bus, uint hz) > > > >>> struct cadence_spi_priv *priv = dev_get_priv(bus); > > > >>> > > > >>> cadence_qspi_apb_config_baudrate_div(priv->regbase, > > > >>> - CONFIG_CQSPI_REF_CLK, hz); > > > >>> + plat->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, plat->ref_clk_hz, hz, > > > >>> plat->tshsl_ns, plat->tsd2d_ns, > > > >>> plat->tchsh_ns, plat->tslch_ns); > > > >>> > > > >>> @@ -294,6 +295,8 @@ 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_index(bus, 1); @@ > > > >>> -325,6 > > > >>> +328,20 @@ 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); > > > >>> > > > >> Did you compile this with platform without clock DM before? Eg: > > Stratix10. > > > >> You need add check for CONFIG_CLK enabled to call clock DM functions > > here. > > > > > > > > Unless I'm mistaken, those functions are prototyped when CLK is not > > enabled: > > > > > > > > https://elixir.bootlin.com/u-boot/latest/source/include/clk.h#L172 > > > > > > > > > > But, unfortunately, such stub does not exists for clk_get_rate(). > > > So on platforms w/o CONFIG_CLK set: > > > > > > arm-linux-gnueabihf-ld.bfd: drivers/spi/built-in.o: in function > > `cadence_spi_probe': > > > /home/a0132425/workspace/u-boot/drivers/spi/cadence_qspi.c:184: > > undefined reference to `clk_get_rate' > > > Makefile:1647: recipe for target 'u-boot' failed > > > make: *** [u-boot] Error 1 > > > > So why did it compile for me? Probably because the linker knows it doesn't > > need 'clk_get_rate' since this branch will never be executed? > Maybe you can try compile from clean build. Run "make mrproper" before compile. Of course I did that, and I just did it again. It *does* compile. Can anyone tell me a config/setup where it doesn't compile? Or does this complain only come from reading the sources? Regards, Simon > > Regards > Ley Foon > > > > Regards, > > Simon > > > > > > > > Regards > > > Vignesh > > > > > > > That should be enough, no? And yes, I did test this on the current > > > > state of > > > > gen5 which does not have a CLK driver, yet. > > > > > > > > Regards, > > > > Simon > > > > > > > >> > > > >> Regards > > > >> Ley Foon > > > >> > > > >>> + 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); > > > >>> diff --git a/drivers/spi/cadence_qspi.h > > > >>> b/drivers/spi/cadence_qspi.h index 20cceca239..99dee75bbd 100644 > > > >>> --- a/drivers/spi/cadence_qspi.h > > > >>> +++ b/drivers/spi/cadence_qspi.h > > > >>> @@ -16,6 +16,7 @@ > > > >>> #define CQSPI_READ_CAPTURE_MAX_DELAY 16 > > > >>> > > > >>> struct cadence_spi_platdata { > > > >>> + unsigned int ref_clk_hz; > > > >>> unsigned int max_hz; > > > >>> void *regbase; > > > >>> void *ahbbase; > > > >>> -- > > > >>> 2.20.1 > > > >>
On 12/11/19 4:57 PM, Simon Goldschmidt wrote: > On Tue, Nov 12, 2019 at 10:30 AM Tan, Ley Foon <ley.foon.tan@intel.com> wrote: >> [...] >>>> But, unfortunately, such stub does not exists for clk_get_rate(). >>>> So on platforms w/o CONFIG_CLK set: >>>> >>>> arm-linux-gnueabihf-ld.bfd: drivers/spi/built-in.o: in function >>> `cadence_spi_probe': >>>> /home/a0132425/workspace/u-boot/drivers/spi/cadence_qspi.c:184: >>> undefined reference to `clk_get_rate' >>>> Makefile:1647: recipe for target 'u-boot' failed >>>> make: *** [u-boot] Error 1 >>> >>> So why did it compile for me? Probably because the linker knows it doesn't >>> need 'clk_get_rate' since this branch will never be executed? >> Maybe you can try compile from clean build. Run "make mrproper" before compile. > > Of course I did that, and I just did it again. It *does* compile. > > Can anyone tell me a config/setup where it doesn't compile? Or does > this complain only come from reading the sources? > I see above error with k2g_evm_defconfig and compiler is: arm-linux-gnueabihf-gcc (GNU Toolchain for the A-profile Architecture 8.3-2019.03 (arm-rel-8.36)) 8.3.0 Regards Vignesh > Regards, > Simon > >> >> Regards >> Ley Foon >>> >>> Regards, >>> Simon >>> >>>> >>>> Regards >>>> Vignesh >>>> >>>>> That should be enough, no? And yes, I did test this on the current >>>>> state of >>>>> gen5 which does not have a CLK driver, yet. >>>>> >>>>> Regards, >>>>> Simon >>>>> >>>>>> >>>>>> Regards >>>>>> Ley Foon >>>>>> >>>>>>> + 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); >>>>>>> diff --git a/drivers/spi/cadence_qspi.h >>>>>>> b/drivers/spi/cadence_qspi.h index 20cceca239..99dee75bbd 100644 >>>>>>> --- a/drivers/spi/cadence_qspi.h >>>>>>> +++ b/drivers/spi/cadence_qspi.h >>>>>>> @@ -16,6 +16,7 @@ >>>>>>> #define CQSPI_READ_CAPTURE_MAX_DELAY 16 >>>>>>> >>>>>>> struct cadence_spi_platdata { >>>>>>> + unsigned int ref_clk_hz; >>>>>>> unsigned int max_hz; >>>>>>> void *regbase; >>>>>>> void *ahbbase; >>>>>>> -- >>>>>>> 2.20.1 >>>>>>
On Tue, Nov 12, 2019 at 12:40 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: > > > > On 12/11/19 4:57 PM, Simon Goldschmidt wrote: > > On Tue, Nov 12, 2019 at 10:30 AM Tan, Ley Foon <ley.foon.tan@intel.com> wrote: > >> > [...] > >>>> But, unfortunately, such stub does not exists for clk_get_rate(). > >>>> So on platforms w/o CONFIG_CLK set: > >>>> > >>>> arm-linux-gnueabihf-ld.bfd: drivers/spi/built-in.o: in function > >>> `cadence_spi_probe': > >>>> /home/a0132425/workspace/u-boot/drivers/spi/cadence_qspi.c:184: > >>> undefined reference to `clk_get_rate' > >>>> Makefile:1647: recipe for target 'u-boot' failed > >>>> make: *** [u-boot] Error 1 > >>> > >>> So why did it compile for me? Probably because the linker knows it doesn't > >>> need 'clk_get_rate' since this branch will never be executed? > >> Maybe you can try compile from clean build. Run "make mrproper" before compile. > > > > Of course I did that, and I just did it again. It *does* compile. > > > > Can anyone tell me a config/setup where it doesn't compile? Or does > > this complain only come from reading the sources? > > > > I see above error with k2g_evm_defconfig and compiler is: Ok, just tested that config and it works for me :-( > > arm-linux-gnueabihf-gcc (GNU Toolchain for the A-profile Architecture > 8.3-2019.03 (arm-rel-8.36)) 8.3.0 I'm using 6.3.0 from debian stretch, but have also tested my patch with newest Ubuntu (which has a 9.x cross compiler). So while I think that difference is disturbing, Maybe it's really best to inline-define all clock functions as you mentioned to Patrick in the other thread... Regards, Simon > > Regards > Vignesh > > > > Regards, > > Simon > > > >> > >> Regards > >> Ley Foon > >>> > >>> Regards, > >>> Simon > >>> > >>>> > >>>> Regards > >>>> Vignesh > >>>> > >>>>> That should be enough, no? And yes, I did test this on the current > >>>>> state of > >>>>> gen5 which does not have a CLK driver, yet. > >>>>> > >>>>> Regards, > >>>>> Simon > >>>>> > >>>>>> > >>>>>> Regards > >>>>>> Ley Foon > >>>>>> > >>>>>>> + 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); > >>>>>>> diff --git a/drivers/spi/cadence_qspi.h > >>>>>>> b/drivers/spi/cadence_qspi.h index 20cceca239..99dee75bbd 100644 > >>>>>>> --- a/drivers/spi/cadence_qspi.h > >>>>>>> +++ b/drivers/spi/cadence_qspi.h > >>>>>>> @@ -16,6 +16,7 @@ > >>>>>>> #define CQSPI_READ_CAPTURE_MAX_DELAY 16 > >>>>>>> > >>>>>>> struct cadence_spi_platdata { > >>>>>>> + unsigned int ref_clk_hz; > >>>>>>> unsigned int max_hz; > >>>>>>> void *regbase; > >>>>>>> void *ahbbase; > >>>>>>> -- > >>>>>>> 2.20.1 > >>>>>> > > -- > Regards > Vignesh
Marek, Am 12.11.2019 um 12:57 schrieb Simon Goldschmidt: > On Tue, Nov 12, 2019 at 12:40 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: >> >> >> >> On 12/11/19 4:57 PM, Simon Goldschmidt wrote: >>> On Tue, Nov 12, 2019 at 10:30 AM Tan, Ley Foon <ley.foon.tan@intel.com> wrote: >>>> >> [...] >>>>>> But, unfortunately, such stub does not exists for clk_get_rate(). >>>>>> So on platforms w/o CONFIG_CLK set: >>>>>> >>>>>> arm-linux-gnueabihf-ld.bfd: drivers/spi/built-in.o: in function >>>>> `cadence_spi_probe': >>>>>> /home/a0132425/workspace/u-boot/drivers/spi/cadence_qspi.c:184: >>>>> undefined reference to `clk_get_rate' >>>>>> Makefile:1647: recipe for target 'u-boot' failed >>>>>> make: *** [u-boot] Error 1 >>>>> >>>>> So why did it compile for me? Probably because the linker knows it doesn't >>>>> need 'clk_get_rate' since this branch will never be executed? >>>> Maybe you can try compile from clean build. Run "make mrproper" before compile. >>> >>> Of course I did that, and I just did it again. It *does* compile. >>> >>> Can anyone tell me a config/setup where it doesn't compile? Or does >>> this complain only come from reading the sources? >>> >> >> I see above error with k2g_evm_defconfig and compiler is: > > Ok, just tested that config and it works for me :-( After having a successful travis run for this on top of u-boot-socfpga/master, could you take this and re-send the PR of last week? Travis run is here: https://travis-ci.org/goldsimon/u-boot/builds/614187977 Regards, Simon > >> >> arm-linux-gnueabihf-gcc (GNU Toolchain for the A-profile Architecture >> 8.3-2019.03 (arm-rel-8.36)) 8.3.0 > > I'm using 6.3.0 from debian stretch, but have also tested my patch > with newest Ubuntu (which has a 9.x cross compiler). > > So while I think that difference is disturbing, Maybe it's really best > to inline-define all clock functions as you mentioned to Patrick > in the other thread... > > Regards, > Simon > >> >> Regards >> Vignesh >> >> >>> Regards, >>> Simon >>> >>>> >>>> Regards >>>> Ley Foon >>>>> >>>>> Regards, >>>>> Simon >>>>> >>>>>> >>>>>> Regards >>>>>> Vignesh >>>>>> >>>>>>> That should be enough, no? And yes, I did test this on the current >>>>>>> state of >>>>>>> gen5 which does not have a CLK driver, yet. >>>>>>> >>>>>>> Regards, >>>>>>> Simon >>>>>>> >>>>>>>> >>>>>>>> Regards >>>>>>>> Ley Foon >>>>>>>> >>>>>>>>> + 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); >>>>>>>>> diff --git a/drivers/spi/cadence_qspi.h >>>>>>>>> b/drivers/spi/cadence_qspi.h index 20cceca239..99dee75bbd 100644 >>>>>>>>> --- a/drivers/spi/cadence_qspi.h >>>>>>>>> +++ b/drivers/spi/cadence_qspi.h >>>>>>>>> @@ -16,6 +16,7 @@ >>>>>>>>> #define CQSPI_READ_CAPTURE_MAX_DELAY 16 >>>>>>>>> >>>>>>>>> struct cadence_spi_platdata { >>>>>>>>> + unsigned int ref_clk_hz; >>>>>>>>> unsigned int max_hz; >>>>>>>>> void *regbase; >>>>>>>>> void *ahbbase; >>>>>>>>> -- >>>>>>>>> 2.20.1 >>>>>>>> >> >> -- >> Regards >> Vignesh
On 11/20/19 8:49 PM, Simon Goldschmidt wrote: > Marek, > > Am 12.11.2019 um 12:57 schrieb Simon Goldschmidt: >> On Tue, Nov 12, 2019 at 12:40 PM Vignesh Raghavendra >> wrote: >>> >>> >>> >>> On 12/11/19 4:57 PM, Simon Goldschmidt wrote: >>>> On Tue, Nov 12, 2019 at 10:30 AM Tan, Ley Foon >>>> <ley.foon.tan@intel.com> wrote: >>>>> >>> [...] >>>>>>> But, unfortunately, such stub does not exists for clk_get_rate(). >>>>>>> So on platforms w/o CONFIG_CLK set: >>>>>>> >>>>>>> arm-linux-gnueabihf-ld.bfd: drivers/spi/built-in.o: in function >>>>>> `cadence_spi_probe': >>>>>>> /home/a0132425/workspace/u-boot/drivers/spi/cadence_qspi.c:184: >>>>>> undefined reference to `clk_get_rate' >>>>>>> Makefile:1647: recipe for target 'u-boot' failed >>>>>>> make: *** [u-boot] Error 1 >>>>>> >>>>>> So why did it compile for me? Probably because the linker knows it >>>>>> doesn't >>>>>> need 'clk_get_rate' since this branch will never be executed? >>>>> Maybe you can try compile from clean build. Run "make mrproper" >>>>> before compile. >>>> >>>> Of course I did that, and I just did it again. It *does* compile. >>>> >>>> Can anyone tell me a config/setup where it doesn't compile? Or does >>>> this complain only come from reading the sources? >>>> >>> >>> I see above error with k2g_evm_defconfig and compiler is: >> >> Ok, just tested that config and it works for me :-( > > After having a successful travis run for this on top of > u-boot-socfpga/master, could you take this and re-send the PR of last week? > > Travis run is here: > > https://travis-ci.org/goldsimon/u-boot/builds/614187977 Wait, so that PR was good all along ? Also, can you just resend this one ?
Am 20.11.2019 um 21:51 schrieb Marek Vasut: > On 11/20/19 8:49 PM, Simon Goldschmidt wrote: >> Marek, >> >> Am 12.11.2019 um 12:57 schrieb Simon Goldschmidt: >>> On Tue, Nov 12, 2019 at 12:40 PM Vignesh Raghavendra >>> wrote: >>>> >>>> >>>> >>>> On 12/11/19 4:57 PM, Simon Goldschmidt wrote: >>>>> On Tue, Nov 12, 2019 at 10:30 AM Tan, Ley Foon >>>>> <ley.foon.tan@intel.com> wrote: >>>>>> >>>> [...] >>>>>>>> But, unfortunately, such stub does not exists for clk_get_rate(). >>>>>>>> So on platforms w/o CONFIG_CLK set: >>>>>>>> >>>>>>>> arm-linux-gnueabihf-ld.bfd: drivers/spi/built-in.o: in function >>>>>>> `cadence_spi_probe': >>>>>>>> /home/a0132425/workspace/u-boot/drivers/spi/cadence_qspi.c:184: >>>>>>> undefined reference to `clk_get_rate' >>>>>>>> Makefile:1647: recipe for target 'u-boot' failed >>>>>>>> make: *** [u-boot] Error 1 >>>>>>> >>>>>>> So why did it compile for me? Probably because the linker knows it >>>>>>> doesn't >>>>>>> need 'clk_get_rate' since this branch will never be executed? >>>>>> Maybe you can try compile from clean build. Run "make mrproper" >>>>>> before compile. >>>>> >>>>> Of course I did that, and I just did it again. It *does* compile. >>>>> >>>>> Can anyone tell me a config/setup where it doesn't compile? Or does >>>>> this complain only come from reading the sources? >>>>> >>>> >>>> I see above error with k2g_evm_defconfig and compiler is: >>> >>> Ok, just tested that config and it works for me :-( >> >> After having a successful travis run for this on top of >> u-boot-socfpga/master, could you take this and re-send the PR of last week? >> >> Travis run is here: >> >> https://travis-ci.org/goldsimon/u-boot/builds/614187977 > > Wait, so that PR was good all along ? D'oh, no, sorry I phrased that wrong, sorry. That PR had v2 and this is v3. I meant use this one instead v2 and send another PR. > > Also, can you just resend this one ? Yes, I'll do that. Regards, Simon
On 11/20/19 9:52 PM, Simon Goldschmidt wrote: > Am 20.11.2019 um 21:51 schrieb Marek Vasut: >> On 11/20/19 8:49 PM, Simon Goldschmidt wrote: >>> Marek, >>> >>> Am 12.11.2019 um 12:57 schrieb Simon Goldschmidt: >>>> On Tue, Nov 12, 2019 at 12:40 PM Vignesh Raghavendra >>>> wrote: >>>>> >>>>> >>>>> >>>>> On 12/11/19 4:57 PM, Simon Goldschmidt wrote: >>>>>> On Tue, Nov 12, 2019 at 10:30 AM Tan, Ley Foon >>>>>> <ley.foon.tan@intel.com> wrote: >>>>>>> >>>>> [...] >>>>>>>>> But, unfortunately, such stub does not exists for clk_get_rate(). >>>>>>>>> So on platforms w/o CONFIG_CLK set: >>>>>>>>> >>>>>>>>> arm-linux-gnueabihf-ld.bfd: drivers/spi/built-in.o: in function >>>>>>>> `cadence_spi_probe': >>>>>>>>> /home/a0132425/workspace/u-boot/drivers/spi/cadence_qspi.c:184: >>>>>>>> undefined reference to `clk_get_rate' >>>>>>>>> Makefile:1647: recipe for target 'u-boot' failed >>>>>>>>> make: *** [u-boot] Error 1 >>>>>>>> >>>>>>>> So why did it compile for me? Probably because the linker knows it >>>>>>>> doesn't >>>>>>>> need 'clk_get_rate' since this branch will never be executed? >>>>>>> Maybe you can try compile from clean build. Run "make mrproper" >>>>>>> before compile. >>>>>> >>>>>> Of course I did that, and I just did it again. It *does* compile. >>>>>> >>>>>> Can anyone tell me a config/setup where it doesn't compile? Or does >>>>>> this complain only come from reading the sources? >>>>>> >>>>> >>>>> I see above error with k2g_evm_defconfig and compiler is: >>>> >>>> Ok, just tested that config and it works for me :-( >>> >>> After having a successful travis run for this on top of >>> u-boot-socfpga/master, could you take this and re-send the PR of last >>> week? >>> >>> Travis run is here: >>> >>> https://travis-ci.org/goldsimon/u-boot/builds/614187977 >> >> Wait, so that PR was good all along ? > > D'oh, no, sorry I phrased that wrong, sorry. That PR had v2 and this is > v3. I meant use this one instead v2 and send another PR. > >> >> Also, can you just resend this one ? > > Yes, I'll do that. OK, thanks.
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index e2e54cd277..8fd23a7702 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> @@ -24,10 +25,10 @@ static int cadence_spi_write_speed(struct udevice *bus, uint hz) struct cadence_spi_priv *priv = dev_get_priv(bus); cadence_qspi_apb_config_baudrate_div(priv->regbase, - CONFIG_CQSPI_REF_CLK, hz); + plat->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, plat->ref_clk_hz, hz, plat->tshsl_ns, plat->tsd2d_ns, plat->tchsh_ns, plat->tslch_ns); @@ -294,6 +295,8 @@ 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_index(bus, 1); @@ -325,6 +328,20 @@ 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); diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h index 20cceca239..99dee75bbd 100644 --- a/drivers/spi/cadence_qspi.h +++ b/drivers/spi/cadence_qspi.h @@ -16,6 +16,7 @@ #define CQSPI_READ_CAPTURE_MAX_DELAY 16 struct cadence_spi_platdata { + unsigned int ref_clk_hz; unsigned int max_hz; void *regbase; void *ahbbase;
Support loading clk speed via DM instead of requiring ad-hoc code. Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> --- Changes in v3: - load ref_clk_hz only once in cadence_spi_ofdata_to_platdata instead of loading it every time in cadence_spi_write_speed Changes in v2: - check return value of clk_get_rate for error drivers/spi/cadence_qspi.c | 21 +++++++++++++++++++-- drivers/spi/cadence_qspi.h | 1 + 2 files changed, 20 insertions(+), 2 deletions(-)