[U-Boot] [PATCH v3] spi: cadence_qspi: support DM_CLK
Simon Goldschmidt
simon.k.r.goldschmidt at gmail.com
Tue Nov 12 11:27:18 UTC 2019
On Tue, Nov 12, 2019 at 10:30 AM Tan, Ley Foon <ley.foon.tan at intel.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
> > Sent: Tuesday, November 12, 2019 5:27 PM
> > To: Vignesh Raghavendra <vigneshr at ti.com>
> > Cc: Tan, Ley Foon <ley.foon.tan at intel.com>; Jagan Teki
> > <jagan at amarulasolutions.com>; Marek Vasut <marex at denx.de>; u-
> > boot at 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 at 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 at intel.com>
> > wrote:
> > > >>
> > > >>
> > > >>
> > > >>> -----Original Message-----
> > > >>> From: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
> > > >>> Sent: Tuesday, November 12, 2019 5:43 AM
> > > >>> To: Jagan Teki <jagan at amarulasolutions.com>
> > > >>> Cc: Marek Vasut <marex at denx.de>; Tan, Ley Foon
> > > >>> <ley.foon.tan at intel.com>; Vignesh Raghavendra <vigneshr at ti.com>;
> > > >>> Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>;
> > > >>> u-boot at 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 at 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
> > > >>
More information about the U-Boot
mailing list