Re: [U-Boot] [PATCH v2] spi: cadence_qspi: support DM_CLK
Vignesh Raghavendra 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
> >>
> >> Support loading clk speed via DM instead of requiring ad-hoc code.
> >>
> >> Signed-off-by: Simon Goldschmidt
> >> Signed-off-by: Simon Goldschmidt
> >> ---
> > [...]
> >> @@ -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, );
> >> +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_free();
> >> +if (IS_ERR_VALUE(ref_clk_hz))
> >> +return ref_clk_hz;
> >> +}
> >>
> [...]
>
> --
> Regards
> Vignesh
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] spi: cadence_qspi: support DM_CLK
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
>>
>> Support loading clk speed via DM instead of requiring ad-hoc code.
>>
>> Signed-off-by: Simon Goldschmidt
>> Signed-off-by: Simon Goldschmidt
>> ---
> [...]
>> @@ -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, );
>> +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_free();
>> +if (IS_ERR_VALUE(ref_clk_hz))
>> +return ref_clk_hz;
>> +}
>>
[...]
--
Regards
Vignesh
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] spi: cadence_qspi: support DM_CLK
Vignesh Raghavendra schrieb am So., 10. Nov. 2019, 12:41:
> Hi Simon,
>
> On 24-Oct-19 11:53 PM, Simon Goldschmidt wrote:
> > From: Simon Goldschmidt
> >
> > Support loading clk speed via DM instead of requiring ad-hoc code.
> >
> > Signed-off-by: Simon Goldschmidt
> > Signed-off-by: Simon Goldschmidt
> > ---
> [...]
> > @@ -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, );
> > + 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_free();
> > + 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);
> >
> >
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] spi: cadence_qspi: support DM_CLK
Hi Simon,
On 24-Oct-19 11:53 PM, Simon Goldschmidt wrote:
> From: Simon Goldschmidt
>
> Support loading clk speed via DM instead of requiring ad-hoc code.
>
> Signed-off-by: Simon Goldschmidt
> Signed-off-by: Simon Goldschmidt
> ---
[...]
> @@ -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, );
> + 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_free();
> + 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);
>
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] spi: cadence_qspi: support DM_CLK
Am 24.10.2019 um 20:23 schrieb Simon Goldschmidt:
From: Simon Goldschmidt
Support loading clk speed via DM instead of requiring ad-hoc code.
Signed-off-by: Simon Goldschmidt
Signed-off-by: Simon Goldschmidt
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
+#include
#include
#include
#include
@@ -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, );
+ 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_free();
+ 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);
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
