[U-Boot] [PATCH v3 2/2] armv7m: Fix larger builds

[Phil Edworthy]

The branch instruction only has an 11-bit relative target address, which
is sometimes not enough.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
v3:
 - Move CONFIG_ARM_ASM_UNIFIED and CONFIG_THUMB2_KERNEL to Kconfig
v2:
 - Use W(b) instead of ldr+mov. Using this macro requires
   CONFIG_ARM_ASM_UNIFIED and CONFIG_THUMB2_KERNEL to be defined.
---
 arch/arm/cpu/armv7m/start.S | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/cpu/armv7m/start.S b/arch/arm/cpu/armv7m/start.S
index 49f2720..890c773 100644
--- a/arch/arm/cpu/armv7m/start.S
+++ b/arch/arm/cpu/armv7m/start.S
@@ -5,10 +5,12 @@
  * SPDX-License-Identifier:GPL-2.0+
  */
 
+#include 
+
 .globl reset
 .type reset, %function
 reset:
-   b   _main
+   W(b)_main
 
 .globl c_runtime_cpu_setup
 c_runtime_cpu_setup:
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 1/2] arm: Add Kconfig symbols used for Linux asm compatibility

[Phil Edworthy]

Rather than change asm files that come from Linux, add the symbols
to Kconfig. Since one of the symbols is for thumb2 builds, make
CPU_V7M always select them.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 arch/arm/Kconfig  | 10 ++
 arch/arm/lib/Makefile |  2 --
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 2a3a36e..2793651 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -19,6 +19,15 @@ config HAS_VBAR
 config HAS_THUMB2
bool
 
+# Used for compatibility with asm files copied from the kernel
+config ARM_ASM_UNIFIED
+   bool
+   default y
+
+# Used for compatibility with asm files copied from the kernel
+config THUMB2_KERNEL
+   bool
+
 # If set, the workarounds for these ARM errata are applied early during U-Boot
 # startup. Note that in general these options force the workarounds to be
 # applied; no CPU-type/version detection exists, unlike the similar options in
@@ -128,6 +137,7 @@ config CPU_V7
 config CPU_V7M
bool
select HAS_THUMB2
+   select THUMB2_KERNEL
select SYS_CACHE_SHIFT_5
 
 config CPU_PXA
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
index f162c14..6e1c436 100644
--- a/arch/arm/lib/Makefile
+++ b/arch/arm/lib/Makefile
@@ -72,8 +72,6 @@ ifneq (,$(findstring -mabi=aapcs-linux,$(PLATFORM_CPPFLAGS)))
 extra-y+= eabi_compat.o
 endif
 
-asflags-y += -DCONFIG_ARM_ASM_UNIFIED
-
 # some files can only build in ARM or THUMB2, not THUMB1
 
 ifdef CONFIG_$(SPL_)SYS_THUMB_BUILD
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2] armv7m: Fix larger builds

[Phil Edworthy]

Hi Tom,

On 31 May 2017 14:10 Tom Rini wrote:
> On Wed, May 31, 2017 at 08:27:05AM +0100, Phil Edworthy wrote:
> 
> > The branch instruction only has an 11-bit relative target address,
> > which is sometimes not enough.
> >
> > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > ---
> > v2:
> >  - Use W(b) instead of ldr+mov. Using this macro requires
> >CONFIG_ARM_ASM_UNIFIED and CONFIG_THUMB2_KERNEL to be
> defined.
> > ---
> >  arch/arm/cpu/armv7m/Makefile | 3 +++
> >  arch/arm/cpu/armv7m/start.S  | 4 +++-
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/cpu/armv7m/Makefile
> > b/arch/arm/cpu/armv7m/Makefile index 257fc7f..df1fc95 100644
> > --- a/arch/arm/cpu/armv7m/Makefile
> > +++ b/arch/arm/cpu/armv7m/Makefile
> > @@ -8,3 +8,6 @@
> >  extra-y := start.o
> >  obj-y += cpu.o cache.o mpu.o
> >  obj-$(CONFIG_SYS_ARCH_TIMER) += systick-timer.o
> > +
> > +asflags-y += -DCONFIG_ARM_ASM_UNIFIED asflags-y +=
> > +-DCONFIG_THUMB2_KERNEL
> 
> Lets move these two symbols to Kconfig and select them on CPU_V7M,
> thanks!

These two symbols come from Linux asm files that have been simply
copied over to U-Boot, they are not configuration options in U-Boot.
They are used by both CPU_V7M and others.

Perhaps I have misunderstood, but for non-CPU_V7M, since
CONFIG_THUMB2_KERNEL depends on CONFIG_SYS_THUMB_BUILD,
are you asking me to add Kconfig for CONFIG_SYS_THUMB_BUILD?

Thanks
Phil

> --
> Tom
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v2] armv7m: Fix larger builds

[Phil Edworthy]

The branch instruction only has an 11-bit relative target address, which
is sometimes not enough.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
v2:
 - Use W(b) instead of ldr+mov. Using this macro requires
   CONFIG_ARM_ASM_UNIFIED and CONFIG_THUMB2_KERNEL to be defined.
---
 arch/arm/cpu/armv7m/Makefile | 3 +++
 arch/arm/cpu/armv7m/start.S  | 4 +++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/cpu/armv7m/Makefile b/arch/arm/cpu/armv7m/Makefile
index 257fc7f..df1fc95 100644
--- a/arch/arm/cpu/armv7m/Makefile
+++ b/arch/arm/cpu/armv7m/Makefile
@@ -8,3 +8,6 @@
 extra-y := start.o
 obj-y += cpu.o cache.o mpu.o
 obj-$(CONFIG_SYS_ARCH_TIMER) += systick-timer.o
+
+asflags-y += -DCONFIG_ARM_ASM_UNIFIED
+asflags-y += -DCONFIG_THUMB2_KERNEL
diff --git a/arch/arm/cpu/armv7m/start.S b/arch/arm/cpu/armv7m/start.S
index 49f2720..890c773 100644
--- a/arch/arm/cpu/armv7m/start.S
+++ b/arch/arm/cpu/armv7m/start.S
@@ -5,10 +5,12 @@
  * SPDX-License-Identifier:GPL-2.0+
  */
 
+#include 
+
 .globl reset
 .type reset, %function
 reset:
-   b   _main
+   W(b)_main
 
 .globl c_runtime_cpu_setup
 c_runtime_cpu_setup:
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] armv7m: Fix larger builds

[Phil Edworthy]

Hi Vikas,

On 27 May 2017 00:55 Vikas MANOCHA wrote:
> On Friday, May 26, 2017 1:27 AM Phil Edworthy wrote:
> > On 26 May 2017 00:58 Vikas MANOCHA wrote:
> > > On Thursday, May 25, 2017 6:58 AM Phil Edworthy wrote:
> > > > On 25 May 2017 10:16 Phil Edworthy wrote:
> > > > > > On 24 May 2017 18:32 Vikas MANOCHA wrote:
> > > > > > Hi Phil,
> > > > > >
> > > > > > > On Wednesday, May 24, 2017 7:34 AM Phil Edworthy wrote:
> > > > > > > The branch instruction only has an 11-bit relative target
> > > > > > > address, which is
> > > > > > sometimes not enough.
> > > > > > >
> > > > > > > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > > > > > > ---
> > > > > > >  arch/arm/cpu/armv7m/start.S | 3 ++-
> > > > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/arch/arm/cpu/armv7m/start.S
> > > > > b/arch/arm/cpu/armv7m/start.S
> > > > > > > index 49f2720..d79adb5 100644
> > > > > > > --- a/arch/arm/cpu/armv7m/start.S
> > > > > > > +++ b/arch/arm/cpu/armv7m/start.S
> > > > > > > @@ -8,7 +8,8 @@
> > > > > > >  .globl   reset
> > > > > > >  .type reset, %function
> > > > > > >  reset:
> > > > > > > - b   _main
> > > > > > > + ldr r0, =_main
> > > > > > > + mov pc, r0
> > > > > >
> > > > > > How about using W(b) for wider range ?
> > > > > Yes, that makes better sense!
> > > > Hmm, if I use W(b) it complains with:
> > > > arch/arm/cpu/armv7m/start.S:14:(.text+0x0): relocation truncated
> > > > to
> > > > fit: R_ARM_THM_JUMP11 against symbol `_main' defined in .text
> > > > section in arch/arm/lib/built-in.o
> > >
> > > Seems like the generated branch instruction is still 16 bit, you can
> > > check the disassembly.  Which compiler & version you are using ?
> > > Are you getting the same issue with "b_main" also. If no, compare
> the
> > > disassembly.
> > I'm using Linaro GCC 6.3-2017.02
> >
> > b   _main
> > or
> > W(b)_main
> > Disassembly is the same:
> >0:   e7feb.n 0 <_main>
> >
> > The problem appears to be that __ASSEMBLY__ is defined and so in
> arch/arm/include/asm/unified.h, W(x) does nothing.
> 
> __ASSEMBLY__ is fine, flag CONFIG_THUMB2_KERNEL is required.
>
> > Having said
> > that, if I simply change the code to
> > b.w _main
> > then I get a build error "Error: bad instruction `b.w _main'"
> 
> Here also flag CONFIG_ARM_ASM_UNIFIED is required for unified assembly.

Thanks for the pointer, I thought CONFIG_THUMB2_KERNEL was defined, but 
is currently only done for files in arch/arm/lib.

I'll re-send a patch for this.

Thanks
Phil

> Cheers,
> Vikas
> 
> >
> > Sorry, I'm not very familiar with ARM asm :)
> >
> > Thanks
> > Phil
> >
> > > Cheers,
> > > Vikas
> > >
> > > >
> > > > Any ideas why?
> > > > Phil
> > > >
> > > > > Thanks
> > > > > Phil
> > > > >
> > > > > > Cheers,
> > > > > > Vikas
> > > > > >
> > > > > > >
> > > > > > >  .globl   c_runtime_cpu_setup
> > > > > > >  c_runtime_cpu_setup:
> > > > > > > --
> > > > > > > 2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] armv7m: Fix larger builds

[Phil Edworthy]

Hi Vikas,

On 26 May 2017 00:58 Vikas MANOCHA wrote:
> On Thursday, May 25, 2017 6:58 AM Phil Edworthy wrote:
> > On 25 May 2017 10:16 Phil Edworthy wrote:
> > > > On 24 May 2017 18:32 Vikas MANOCHA wrote:
> > > > Hi Phil,
> > > >
> > > > > On Wednesday, May 24, 2017 7:34 AM Phil Edworthy wrote:
> > > > > The branch instruction only has an 11-bit relative target
> > > > > address, which is
> > > > sometimes not enough.
> > > > >
> > > > > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > > > > ---
> > > > >  arch/arm/cpu/armv7m/start.S | 3 ++-
> > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/arch/arm/cpu/armv7m/start.S
> > > b/arch/arm/cpu/armv7m/start.S
> > > > > index 49f2720..d79adb5 100644
> > > > > --- a/arch/arm/cpu/armv7m/start.S
> > > > > +++ b/arch/arm/cpu/armv7m/start.S
> > > > > @@ -8,7 +8,8 @@
> > > > >  .globl   reset
> > > > >  .type reset, %function
> > > > >  reset:
> > > > > - b   _main
> > > > > + ldr r0, =_main
> > > > > + mov pc, r0
> > > >
> > > > How about using W(b) for wider range ?
> > > Yes, that makes better sense!
> > Hmm, if I use W(b) it complains with:
> > arch/arm/cpu/armv7m/start.S:14:(.text+0x0): relocation truncated to
> > fit: R_ARM_THM_JUMP11 against symbol `_main' defined in .text section
> > in arch/arm/lib/built-in.o
> 
> Seems like the generated branch instruction is still 16 bit, you can check the
> disassembly.  Which compiler & version you are using ?
> Are you getting the same issue with "b_main" also. If no, compare the
> disassembly.
I'm using Linaro GCC 6.3-2017.02

b   _main
or
W(b)_main
Disassembly is the same:
   0:   e7feb.n 0 <_main>

The problem appears to be that __ASSEMBLY__ is defined and so in
arch/arm/include/asm/unified.h, W(x) does nothing. Having said that,
if I simply change the code to
b.w _main
then I get a build error "Error: bad instruction `b.w _main'"

Sorry, I'm not very familiar with ARM asm :)

Thanks
Phil

> Cheers,
> Vikas
> 
> >
> > Any ideas why?
> > Phil
> >
> > > Thanks
> > > Phil
> > >
> > > > Cheers,
> > > > Vikas
> > > >
> > > > >
> > > > >  .globl   c_runtime_cpu_setup
> > > > >  c_runtime_cpu_setup:
> > > > > --
> > > > > 2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] mmc: Set the initial clock speed to 400KHz

[Phil Edworthy]

Hi Jaehoon Chung,

On 26 May 2017 04:38 Jaehoon Chung wrote:
> On 05/25/2017 11:14 PM, Phil Edworthy wrote:
> > On 25 May 2017 15:10 Jaehoon Chung wrote:
> >> On 05/25/2017 11:02 PM, Phil Edworthy wrote:
> >>> On 25 May 2017 14:50 Jaehoon Chung wrote:
> >>>> On 05/24/2017 10:54 PM, Phil Edworthy wrote:
> >>>>> The code currently defaults to the slowest clock speed that can be
> >>>>> achieved, which can be significantly lower than the SD spec.
> >>>>
> >>>> Is there any problem..As i know, it should be changed from 1 to
> min_clk.
> >>> The only problem is that the initial SD clock can be very slow so it
> >>> increases the time to start SD. Admittedly, it's a very small
> >>> increase in time, but we should use the correct initial clock speed.
> >>
> >> Well..i didn't agree yet...
> >>
> >> If mmc_set_clock(mmc, 400K) and mmc->cfg->f_min is 300K?
> >> Initial clock should be always 400K..but spec is mentioned "initial
> >> clock is maximum 400K.."
> >> It means the clock can be the value under 400K.
> > I'm not sure I follow you.
> > The spec means that all SD cards must support an initial clock speed of
> 400KHz, right?
> 
> No..clock frequency range shall be 100KHz - 400KHz during initialization
> sequence.
> Some targets should not work fine with 400KHz..So it needs to check f_min
> value.
> otherwise, it needs to find the initial clock value like Linux kernel scheme.
f_min is not the frequency that must be used during init, it is the minimum
frequency that the Controller can operate at. If a controller has f_min=300KHz
then there is no problem attempting to set the MMC clock to 400KHz. If the
controller cannot do exactly 400KHz, the driver should set the clock to the
closest frequency _under_ 400KHz.

For example, SDHCI v3 controllers set f_min to f_max / 2046. In my particular
case, SDHCI v3 controller f_max is 50MHz, so f_min is approximately 25KHz.
Therefore, when the code does this:
mmc_set_clock(mmc, 1);
it will set the clock rate to approximately 25KHz.
Clearly this is outside the valid 100-400KHz range that you quote.

Thanks
Phil

> > If so, then there is no harm in setting it to 400KHz.
> >
> > Thanks
> > Phil
> >
> >>>>> Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> >>>>> ---
> >>>>>  drivers/mmc/mmc.c | 2 +-
> >>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index
> >>>>> 72fc177..dff1be3 100644
> >>>>> --- a/drivers/mmc/mmc.c
> >>>>> +++ b/drivers/mmc/mmc.c
> >>>>> @@ -1676,7 +1676,7 @@ int mmc_start_init(struct mmc *mmc)
> #endif
> >>>>> mmc->ddr_mode = 0;
> >>>>> mmc_set_bus_width(mmc, 1);
> >>>>> -   mmc_set_clock(mmc, 1);
> >>>>> +   mmc_set_clock(mmc, 40);
> >>>>>
> >>>>> /* Reset the Card */
> >>>>> err = mmc_go_idle(mmc);
> >>>>>
> >>>
> >>>
> >>>
> >>>
> >
> >
> >
> >

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] mmc: Set the initial clock speed to 400KHz

[Phil Edworthy]

Hi Jaehoon Chung,

On 25 May 2017 15:10 Jaehoon Chung wrote:
> On 05/25/2017 11:02 PM, Phil Edworthy wrote:
> > On 25 May 2017 14:50 Jaehoon Chung wrote:
> >> On 05/24/2017 10:54 PM, Phil Edworthy wrote:
> >>> The code currently defaults to the slowest clock speed that can be
> >>> achieved, which can be significantly lower than the SD spec.
> >>
> >> Is there any problem..As i know, it should be changed from 1 to min_clk.
> > The only problem is that the initial SD clock can be very slow so it
> > increases the time to start SD. Admittedly, it's a very small increase
> > in time, but we should use the correct initial clock speed.
> 
> Well..i didn't agree yet...
> 
> If mmc_set_clock(mmc, 400K) and mmc->cfg->f_min is 300K?
> Initial clock should be always 400K..but spec is mentioned "initial clock is
> maximum 400K.."
> It means the clock can be the value under 400K.
I'm not sure I follow you.
The spec means that all SD cards must support an initial clock speed of 400KHz, 
right?
If so, then there is no harm in setting it to 400KHz.

Thanks
Phil

> >>> Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> >>> ---
> >>>  drivers/mmc/mmc.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index
> >>> 72fc177..dff1be3 100644
> >>> --- a/drivers/mmc/mmc.c
> >>> +++ b/drivers/mmc/mmc.c
> >>> @@ -1676,7 +1676,7 @@ int mmc_start_init(struct mmc *mmc)  #endif
> >>>   mmc->ddr_mode = 0;
> >>>   mmc_set_bus_width(mmc, 1);
> >>> - mmc_set_clock(mmc, 1);
> >>> + mmc_set_clock(mmc, 40);
> >>>
> >>>   /* Reset the Card */
> >>>   err = mmc_go_idle(mmc);
> >>>
> >
> >
> >
> >

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] mmc: Set the initial clock speed to 400KHz

[Phil Edworthy]

Hi Jaehoon Chung,

On 25 May 2017 14:50 Jaehoon Chung wrote:
> Hi,
> 
> On 05/24/2017 10:54 PM, Phil Edworthy wrote:
> > The code currently defaults to the slowest clock speed that can be
> > achieved, which can be significantly lower than the SD spec.
> 
> Is there any problem..As i know, it should be changed from 1 to min_clk.
The only problem is that the initial SD clock can be very slow so it increases
the time to start SD. Admittedly, it's a very small increase in time, but we
should use the correct initial clock speed.

Thanks
Phil

> >
> > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > ---
> >  drivers/mmc/mmc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index
> > 72fc177..dff1be3 100644
> > --- a/drivers/mmc/mmc.c
> > +++ b/drivers/mmc/mmc.c
> > @@ -1676,7 +1676,7 @@ int mmc_start_init(struct mmc *mmc)  #endif
> > mmc->ddr_mode = 0;
> > mmc_set_bus_width(mmc, 1);
> > -   mmc_set_clock(mmc, 1);
> > +   mmc_set_clock(mmc, 40);
> >
> > /* Reset the Card */
> > err = mmc_go_idle(mmc);
> >

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] armv7m: Fix larger builds

[Phil Edworthy]

Hi Vikas,

On 25 May 2017 10:16 Phil Edworthy wrote:
> > On 24 May 2017 18:32 Vikas MANOCHA wrote:
> > Hi Phil,
> >
> > > On Wednesday, May 24, 2017 7:34 AM Phil Edworthy wrote:
> > > The branch instruction only has an 11-bit relative target address, which 
> > > is
> > sometimes not enough.
> > >
> > > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > > ---
> > >  arch/arm/cpu/armv7m/start.S | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm/cpu/armv7m/start.S
> b/arch/arm/cpu/armv7m/start.S
> > > index 49f2720..d79adb5 100644
> > > --- a/arch/arm/cpu/armv7m/start.S
> > > +++ b/arch/arm/cpu/armv7m/start.S
> > > @@ -8,7 +8,8 @@
> > >  .globl   reset
> > >  .type reset, %function
> > >  reset:
> > > - b   _main
> > > + ldr r0, =_main
> > > + mov pc, r0
> >
> > How about using W(b) for wider range ?
> Yes, that makes better sense!
Hmm, if I use W(b) it complains with:
arch/arm/cpu/armv7m/start.S:14:(.text+0x0): relocation truncated to fit: 
R_ARM_THM_JUMP11 against symbol `_main' defined in .text section in 
arch/arm/lib/built-in.o

Any ideas why?
Phil

> Thanks
> Phil
> 
> > Cheers,
> > Vikas
> >
> > >
> > >  .globl   c_runtime_cpu_setup
> > >  c_runtime_cpu_setup:
> > > --
> > > 2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] armv7m: Fix larger builds

[Phil Edworthy]

Hi Vikas,

> On 24 May 2017 18:32 Vikas MANOCHA wrote:
> Hi Phil,
> 
> > On Wednesday, May 24, 2017 7:34 AM Phil Edworthy wrote:
> > The branch instruction only has an 11-bit relative target address, which is
> sometimes not enough.
> >
> > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > ---
> >  arch/arm/cpu/armv7m/start.S | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/cpu/armv7m/start.S b/arch/arm/cpu/armv7m/start.S
> > index 49f2720..d79adb5 100644
> > --- a/arch/arm/cpu/armv7m/start.S
> > +++ b/arch/arm/cpu/armv7m/start.S
> > @@ -8,7 +8,8 @@
> >  .globl reset
> >  .type reset, %function
> >  reset:
> > -   b   _main
> > +   ldr r0, =_main
> > +   mov pc, r0
> 
> How about using W(b) for wider range ?
Yes, that makes better sense!

Thanks
Phil
 
> Cheers,
> Vikas
> 
> >
> >  .globl c_runtime_cpu_setup
> >  c_runtime_cpu_setup:
> > --
> > 2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH] armv7m: Fix larger builds

[Phil Edworthy]

The branch instruction only has an 11-bit relative target address, which
is sometimes not enough.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 arch/arm/cpu/armv7m/start.S | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/cpu/armv7m/start.S b/arch/arm/cpu/armv7m/start.S
index 49f2720..d79adb5 100644
--- a/arch/arm/cpu/armv7m/start.S
+++ b/arch/arm/cpu/armv7m/start.S
@@ -8,7 +8,8 @@
 .globl reset
 .type reset, %function
 reset:
-   b   _main
+   ldr r0, =_main
+   mov pc, r0
 
 .globl c_runtime_cpu_setup
 c_runtime_cpu_setup:
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [RESEND PATCH] dfu: dfu_sf: Fix read offset

[Phil Edworthy]

The offset was applied to write, but not read, now its applied to
both.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
This was previously reviewed and Acked, but vanished somewhere...
See https://patchwork.ozlabs.org/patch/694574
---
 drivers/dfu/dfu_sf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dfu/dfu_sf.c b/drivers/dfu/dfu_sf.c
index 9702eee..b6d5fe2 100644
--- a/drivers/dfu/dfu_sf.c
+++ b/drivers/dfu/dfu_sf.c
@@ -20,7 +20,8 @@ static long dfu_get_medium_size_sf(struct dfu_entity *dfu)
 static int dfu_read_medium_sf(struct dfu_entity *dfu, u64 offset, void *buf,
long *len)
 {
-   return spi_flash_read(dfu->data.sf.dev, offset, *len, buf);
+   return spi_flash_read(dfu->data.sf.dev, dfu->data.sf.start + offset,
+   *len, buf);
 }
 
 static u64 find_sector(struct dfu_entity *dfu, u64 start, u64 offset)
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH] mmc: Set the initial clock speed to 400KHz

[Phil Edworthy]

The code currently defaults to the slowest clock speed that can be
achieved, which can be significantly lower than the SD spec.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 drivers/mmc/mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 72fc177..dff1be3 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -1676,7 +1676,7 @@ int mmc_start_init(struct mmc *mmc)
 #endif
mmc->ddr_mode = 0;
mmc_set_bus_width(mmc, 1);
-   mmc_set_clock(mmc, 1);
+   mmc_set_clock(mmc, 40);
 
/* Reset the Card */
err = mmc_go_idle(mmc);
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v3] armv7m: Add SysTick timer driver

[Phil Edworthy]

The SysTick is a 24-bit down counter that is found on all ARM Cortex
M3, M4, M7 devices and is always located at a fixed address.

The number of reference clock ticks that correspond to 10ms is normally
defined in the SysTick Calibration register's TENMS field. However, on some
devices this is wrong, so this driver allows the clock rate to be defined
using CONFIG_SYS_HZ_CLOCK.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
v3:
 - Add comments about where we get the ref clock rate from.

v2:
 - Variables & constant renamed.
 - Use the calibration reg to determine if we use the cpu or ref clk
 - Use the calibration reg to get the clk rate, unless specified
---
 arch/arm/cpu/armv7m/Makefile|   2 +
 arch/arm/cpu/armv7m/systick-timer.c | 116 
 2 files changed, 118 insertions(+)
 create mode 100644 arch/arm/cpu/armv7m/systick-timer.c

diff --git a/arch/arm/cpu/armv7m/Makefile b/arch/arm/cpu/armv7m/Makefile
index aff60e8..e1a6c40 100644
--- a/arch/arm/cpu/armv7m/Makefile
+++ b/arch/arm/cpu/armv7m/Makefile
@@ -7,3 +7,5 @@
 
 extra-y := start.o
 obj-y += cpu.o
+
+obj-$(CONFIG_SYS_ARCH_TIMER) += systick-timer.o
diff --git a/arch/arm/cpu/armv7m/systick-timer.c 
b/arch/arm/cpu/armv7m/systick-timer.c
new file mode 100644
index 000..23244c3
--- /dev/null
+++ b/arch/arm/cpu/armv7m/systick-timer.c
@@ -0,0 +1,116 @@
+/*
+ * ARM Cortex M3/M4/M7 SysTick timer driver
+ * (C) Copyright 2017 Renesas Electronics Europe Ltd
+ *
+ * Based on arch/arm/mach-stm32/stm32f1/timer.c
+ * (C) Copyright 2015
+ * Kamil Lulko, <kamil.lu...@gmail.com>
+ *
+ * Copyright 2015 ATS Advanced Telematics Systems GmbH
+ * Copyright 2015 Konsulko Group, Matt Porter <mpor...@konsulko.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ *
+ * The SysTick timer is a 24-bit count down timer. The clock can be either the
+ * CPU clock or a reference clock. Since the timer will wrap around very 
quickly
+ * when using the CPU clock, and we do not handle the timer interrupts, it is
+ * expected that this driver is only ever used with a slow reference clock.
+ *
+ * The number of reference clock ticks that correspond to 10ms is normally
+ * defined in the SysTick Calibration register's TENMS field. However, on some
+ * devices this is wrong, so this driver allows the clock rate to be defined
+ * using CONFIG_SYS_HZ_CLOCK.
+ */
+
+#include 
+#include 
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/* SysTick Base Address - fixed for all Cortex M3, M4 and M7 devices */
+#define SYSTICK_BASE   0xE000E010
+
+struct cm3_systick {
+   uint32_t ctrl;
+   uint32_t reload_val;
+   uint32_t current_val;
+   uint32_t calibration;
+};
+
+#define TIMER_MAX_VAL  0x00FF
+#define SYSTICK_CTRL_ENBIT(0)
+/* Clock source: 0 = Ref clock, 1 = CPU clock */
+#define SYSTICK_CTRL_CPU_CLK   BIT(2)
+#define SYSTICK_CAL_NOREF  BIT(31)
+#define SYSTICK_CAL_SKEW   BIT(30)
+#define SYSTICK_CAL_TENMS_MASK 0x00FF
+
+/* read the 24-bit timer */
+static ulong read_timer(void)
+{
+   struct cm3_systick *systick = (struct cm3_systick *)SYSTICK_BASE;
+
+   /* The timer counts down, therefore convert to an incrementing timer */
+   return TIMER_MAX_VAL - readl(>current_val);
+}
+
+int timer_init(void)
+{
+   struct cm3_systick *systick = (struct cm3_systick *)SYSTICK_BASE;
+   u32 cal;
+
+   writel(TIMER_MAX_VAL, >reload_val);
+   /* Any write to current_val reg clears it to 0 */
+   writel(0, >current_val);
+
+   cal = readl(>calibration);
+   if (cal & SYSTICK_CAL_NOREF)
+   /* Use CPU clock, no interrupts */
+   writel(SYSTICK_CTRL_EN | SYSTICK_CTRL_CPU_CLK, >ctrl);
+   else
+   /* Use external clock, no interrupts */
+   writel(SYSTICK_CTRL_EN, >ctrl);
+
+   /*
+* If the TENMS field is inexact or wrong, specify the clock rate using
+* CONFIG_SYS_HZ_CLOCK.
+*/
+#if defined(CONFIG_SYS_HZ_CLOCK)
+   gd->arch.timer_rate_hz = CONFIG_SYS_HZ_CLOCK;
+#else
+   gd->arch.timer_rate_hz = (cal & SYSTICK_CAL_TENMS_MASK) * 100;
+#endif
+
+   gd->arch.tbl = 0;
+   gd->arch.tbu = 0;
+   gd->arch.lastinc = read_timer();
+
+   return 0;
+}
+
+/* return milli-seconds timer value */
+ulong get_timer(ulong base)
+{
+   unsigned long long t = get_ticks() * 1000;
+
+   return (ulong)((t / gd->arch.timer_rate_hz)) - base;
+}
+
+unsigned long long get_ticks(void)
+{
+   u32 now = read_timer();
+
+   if (now >= gd->arch.lastinc)
+   gd->arch.tbl += (now - gd->arch.lastinc);
+   else
+   gd->arch.tbl += (TIMER_MAX_VAL - gd->arch.lastinc) + now;
+
+   gd->arch.lastinc = now;
+
+   return gd->arch.tbl;
+}
+
+ulong get_tbclk(void)
+{
+   return gd->arch.timer_rate_hz;
+}
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2] armv7m: Add SysTick timer driver

[Phil Edworthy]

Hi Vikas,

On 15 February 2017 23:50, Vikas MANOCHA wrote:
> Hi Phil,
> 
> > -Original Message-
> > From: Phil Edworthy [mailto:phil.edwor...@renesas.com]
> > Sent: Monday, February 13, 2017 11:48 PM
> > To: Albert Aribaud <albert.u.b...@aribaud.net>
> > Cc: Tom Rini <tr...@konsulko.com>; Vikas MANOCHA
> <vikas.mano...@st.com>; Kamil Lulko <kamil.lu...@gmail.com>; Michael
> > Kurz <michi.k...@gmail.com>; u-boot@lists.denx.de; Phil Edworthy
> <phil.edwor...@renesas.com>
> > Subject: [PATCH v2] armv7m: Add SysTick timer driver
> >
> > The SysTick is a 24-bit down counter that is found on all ARM Cortex M3, M4,
> M7 devices and is always located at a fixed address.
> >
> > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > ---
> > v2:
> >  - Variables & constant renamed.
> >  - Use the calibration reg to determine if we use the cpu or ref clk
> >  - Use the calibration reg to get the clk rate, unless specified
> > ---
> >  arch/arm/cpu/armv7m/Makefile|   2 +
> >  arch/arm/cpu/armv7m/systick-timer.c | 107
> 
> >  2 files changed, 109 insertions(+)
> >  create mode 100644 arch/arm/cpu/armv7m/systick-timer.c
> >
> > diff --git a/arch/arm/cpu/armv7m/Makefile b/arch/arm/cpu/armv7m/Makefile
> index aff60e8..e1a6c40 100644
> > --- a/arch/arm/cpu/armv7m/Makefile
> > +++ b/arch/arm/cpu/armv7m/Makefile
> > @@ -7,3 +7,5 @@
> >
> >  extra-y := start.o
> >  obj-y += cpu.o
> > +
> > +obj-$(CONFIG_SYS_ARCH_TIMER) += systick-timer.o
> > diff --git a/arch/arm/cpu/armv7m/systick-timer.c
> b/arch/arm/cpu/armv7m/systick-timer.c
> > new file mode 100644
> > index 000..62308f9
> > --- /dev/null
> > +++ b/arch/arm/cpu/armv7m/systick-timer.c
> > @@ -0,0 +1,107 @@
> > +/*
> > + * ARM Cortex M3/M4/M7 SysTick timer driver
> > + * (C) Copyright 2017 Renesas Electronics Europe Ltd
> > + *
> > + * Based on arch/arm/mach-stm32/stm32f1/timer.c
> > + * (C) Copyright 2015
> > + * Kamil Lulko, <kamil.lu...@gmail.com>
> > + *
> > + * Copyright 2015 ATS Advanced Telematics Systems GmbH
> > + * Copyright 2015 Konsulko Group, Matt Porter <mpor...@konsulko.com>
> > + *
> > + * SPDX-License-Identifier: GPL-2.0+
> > + *
> > + * The SysTick timer is a 24-bit count down timer. The clock can be
> > +either the
> > + * CPU clock or a reference clock. Since the timer will wrap around
> > +very quickly
> > + * when using the CPU clock, and we do not handle the timer interrupts,
> > +it is
> > + * expected that this driver is only ever used with a slow reference clock.
> > + */
> > +
> > +#include 
> > +#include 
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +/* SysTick Base Address - fixed for all Cortex M3, M4 and M7 devices */
> > +#define SYSTICK_BASE   0xE000E010
> > +
> > +struct cm3_systick {
> > +   uint32_t ctrl;
> > +   uint32_t reload_val;
> > +   uint32_t current_val;
> > +   uint32_t calibration;
> > +};
> > +
> > +#define TIMER_MAX_VAL  0x00FF
> > +#define SYSTICK_CTRL_ENBIT(0)
> > +/* Clock source: 0 = Ref clock, 1 = CPU clock */
> > +#define SYSTICK_CTRL_CPU_CLK   BIT(2)
> > +#define SYSTICK_CAL_NOREF  BIT(31)
> > +#define SYSTICK_CAL_SKEW   BIT(30)
> > +#define SYSTICK_CAL_TENMS_MASK 0x00FF
> > +
> > +/* read the 24-bit timer */
> > +static ulong read_timer(void)
> > +{
> > +   struct cm3_systick *systick = (struct cm3_systick *)SYSTICK_BASE;
> > +
> > +   /* The timer counts down, therefore convert to an incrementing timer */
> > +   return TIMER_MAX_VAL - readl(>current_val); }
> > +
> > +int timer_init(void)
> > +{
> > +   struct cm3_systick *systick = (struct cm3_systick *)SYSTICK_BASE;
> > +   u32 cal;
> > +
> > +   writel(TIMER_MAX_VAL, >reload_val);
> > +   /* Any write to current_val reg clears it to 0 */
> > +   writel(0, >current_val);
> > +
> > +   cal = readl(>calibration);
> > +   if (cal & SYSTICK_CAL_NOREF)
> 
> Good.
> 
> > +   /* Use CPU clock, no interrupts */
> > +   writel(SYSTICK_CTRL_EN | SYSTICK_CTRL_CPU_CLK, 
> >ctrl);
> > +   else
> > +   /* Use external clock, no interrupts */
> > +   writel(SYSTICK_CTRL_EN, >ctrl);
> > +
> > +#if defined(CONFIG_SYS_HZ_CLOCK)
> > +   gd->arch.timer_rate_hz = CONFIG_SY

[U-Boot] [PATCH v2] armv7m: Add SysTick timer driver

[Phil Edworthy]

The SysTick is a 24-bit down counter that is found on all ARM Cortex
M3, M4, M7 devices and is always located at a fixed address.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
v2:
 - Variables & constant renamed.
 - Use the calibration reg to determine if we use the cpu or ref clk
 - Use the calibration reg to get the clk rate, unless specified
---
 arch/arm/cpu/armv7m/Makefile|   2 +
 arch/arm/cpu/armv7m/systick-timer.c | 107 
 2 files changed, 109 insertions(+)
 create mode 100644 arch/arm/cpu/armv7m/systick-timer.c

diff --git a/arch/arm/cpu/armv7m/Makefile b/arch/arm/cpu/armv7m/Makefile
index aff60e8..e1a6c40 100644
--- a/arch/arm/cpu/armv7m/Makefile
+++ b/arch/arm/cpu/armv7m/Makefile
@@ -7,3 +7,5 @@
 
 extra-y := start.o
 obj-y += cpu.o
+
+obj-$(CONFIG_SYS_ARCH_TIMER) += systick-timer.o
diff --git a/arch/arm/cpu/armv7m/systick-timer.c 
b/arch/arm/cpu/armv7m/systick-timer.c
new file mode 100644
index 000..62308f9
--- /dev/null
+++ b/arch/arm/cpu/armv7m/systick-timer.c
@@ -0,0 +1,107 @@
+/*
+ * ARM Cortex M3/M4/M7 SysTick timer driver
+ * (C) Copyright 2017 Renesas Electronics Europe Ltd
+ *
+ * Based on arch/arm/mach-stm32/stm32f1/timer.c
+ * (C) Copyright 2015
+ * Kamil Lulko, <kamil.lu...@gmail.com>
+ *
+ * Copyright 2015 ATS Advanced Telematics Systems GmbH
+ * Copyright 2015 Konsulko Group, Matt Porter <mpor...@konsulko.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ *
+ * The SysTick timer is a 24-bit count down timer. The clock can be either the
+ * CPU clock or a reference clock. Since the timer will wrap around very 
quickly
+ * when using the CPU clock, and we do not handle the timer interrupts, it is
+ * expected that this driver is only ever used with a slow reference clock.
+ */
+
+#include 
+#include 
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/* SysTick Base Address - fixed for all Cortex M3, M4 and M7 devices */
+#define SYSTICK_BASE   0xE000E010
+
+struct cm3_systick {
+   uint32_t ctrl;
+   uint32_t reload_val;
+   uint32_t current_val;
+   uint32_t calibration;
+};
+
+#define TIMER_MAX_VAL  0x00FF
+#define SYSTICK_CTRL_ENBIT(0)
+/* Clock source: 0 = Ref clock, 1 = CPU clock */
+#define SYSTICK_CTRL_CPU_CLK   BIT(2)
+#define SYSTICK_CAL_NOREF  BIT(31)
+#define SYSTICK_CAL_SKEW   BIT(30)
+#define SYSTICK_CAL_TENMS_MASK 0x00FF
+
+/* read the 24-bit timer */
+static ulong read_timer(void)
+{
+   struct cm3_systick *systick = (struct cm3_systick *)SYSTICK_BASE;
+
+   /* The timer counts down, therefore convert to an incrementing timer */
+   return TIMER_MAX_VAL - readl(>current_val);
+}
+
+int timer_init(void)
+{
+   struct cm3_systick *systick = (struct cm3_systick *)SYSTICK_BASE;
+   u32 cal;
+
+   writel(TIMER_MAX_VAL, >reload_val);
+   /* Any write to current_val reg clears it to 0 */
+   writel(0, >current_val);
+
+   cal = readl(>calibration);
+   if (cal & SYSTICK_CAL_NOREF)
+   /* Use CPU clock, no interrupts */
+   writel(SYSTICK_CTRL_EN | SYSTICK_CTRL_CPU_CLK, >ctrl);
+   else
+   /* Use external clock, no interrupts */
+   writel(SYSTICK_CTRL_EN, >ctrl);
+
+#if defined(CONFIG_SYS_HZ_CLOCK)
+   gd->arch.timer_rate_hz = CONFIG_SYS_HZ_CLOCK;
+#else
+   gd->arch.timer_rate_hz = (cal & SYSTICK_CAL_TENMS_MASK) * 100;
+#endif
+
+   gd->arch.tbl = 0;
+   gd->arch.tbu = 0;
+   gd->arch.lastinc = read_timer();
+
+   return 0;
+}
+
+/* return milli-seconds timer value */
+ulong get_timer(ulong base)
+{
+   unsigned long long t = get_ticks() * 1000;
+
+   return (ulong)((t / gd->arch.timer_rate_hz)) - base;
+}
+
+unsigned long long get_ticks(void)
+{
+   u32 now = read_timer();
+
+   if (now >= gd->arch.lastinc)
+   gd->arch.tbl += (now - gd->arch.lastinc);
+   else
+   gd->arch.tbl += (TIMER_MAX_VAL - gd->arch.lastinc) + now;
+
+   gd->arch.lastinc = now;
+
+   return gd->arch.tbl;
+}
+
+ulong get_tbclk(void)
+{
+   return gd->arch.timer_rate_hz;
+}
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] armv7m: Add SysTick timer driver

[Phil Edworthy]

Hi Vikas,

On 12 February 2017 21:10, Vikas MANOCHA wrote:
> Hi Phil,
> 
> > -Original Message-
> > From: Phil Edworthy [mailto:phil.edwor...@renesas.com]
> > Sent: Tuesday, February 07, 2017 6:34 AM
> > To: Tom Rini <tr...@konsulko.com>
> > Cc: Kamil Lulko <kamil.lu...@gmail.com>; Vikas MANOCHA
> <vikas.mano...@st.com>; Michael Kurz <michi.k...@gmail.com>;
> > Albert Aribaud <albert.u.b...@aribaud.net>; U-Boot Mailing List  b...@lists.denx.de>
> > Subject: RE: [U-Boot] [PATCH] armv7m: Add SysTick timer driver
> >
> > Hi Tom,
> >
> > On 07 February 2017 14:23, Tom Rini wrote:
> > > On Tue, Feb 07, 2017 at 02:19:39PM +, Phil Edworthy wrote:
> > > > Hi Kamil,
> > > >
> > > > On 07 February 2017 14:12, Kamil Lulko wrote:
> > > > > On Mon, Feb 6, 2017 at 12:16 AM, Tom Rini <tr...@konsulko.com> wrote:
> > > > > > On Fri, Feb 03, 2017 at 02:48:40PM +, Phil Edworthy wrote:
> > > > > >
> > > > > > > The SysTick is a 24-bit down counter that is found on all ARM
> > > > > > > Cortex M3, M4, M7 devices and is always located at a fixed 
> > > > > > > address.
> > > > > > >
> > > > > > > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > > > > > > ---
> > > > > > >  arch/arm/cpu/armv7m/Makefile|  2 +
> > > > > > >  arch/arm/cpu/armv7m/systick-timer.c | 91
> > > > > +
> > > > > > >  2 files changed, 93 insertions(+)  create mode 100644
> > > > > > > arch/arm/cpu/armv7m/systick-timer.c
> > > > > > >
> > > > > > > diff --git a/arch/arm/cpu/armv7m/Makefile
> > > > > b/arch/arm/cpu/armv7m/Makefile
> > > > > > > index aff60e8..e1a6c40 100644
> > > > > > > --- a/arch/arm/cpu/armv7m/Makefile
> > > > > > > +++ b/arch/arm/cpu/armv7m/Makefile
> > > > > > > @@ -7,3 +7,5 @@
> > > > > > >
> > > > > > >  extra-y := start.o
> > > > > > >  obj-y += cpu.o
> > > > > > > +
> > > > > > > +obj-$(CONFIG_SYS_ARCH_TIMER) += systick-timer.o
> > > > > > > diff --git a/arch/arm/cpu/armv7m/systick-timer.c
> > > > > b/arch/arm/cpu/armv7m/systick-timer.c
> > > > > > > new file mode 100644
> > > > > > > index 000..6ccc2fb
> > > > > > > --- /dev/null
> > > > > > > +++ b/arch/arm/cpu/armv7m/systick-timer.c
> > > > > > > @@ -0,0 +1,91 @@
> > > > > > > +/*
> > > > > > > + * ARM Cortex M3/M4/M7 SysTick timer driver
> > > > > > > + * (C) Copyright 2017 Renesas Electronics Europe Ltd
> > > > > > > + *
> > > > > > > + * Based on arch/arm/mach-stm32/stm32f1/timer.c
> > > > > > > + * (C) Copyright 2015
> > > > > > > + * Kamil Lulko, <kamil.lu...@gmail.com>
> > > > > > > + *
> > > > > > > + * Copyright 2015 ATS Advanced Telematics Systems GmbH
> > > > > > > + * Copyright 2015 Konsulko Group, Matt Porter
> > > <mpor...@konsulko.com>
> > > > > > > + *
> > > > > > > + * SPDX-License-Identifier: GPL-2.0+
> > > > > > > + */
> > > > > > > +
> > > > > > > +#include 
> > > > > > > +#include 
> > > > > > > +
> > > > > > > +DECLARE_GLOBAL_DATA_PTR;
> > > > > > > +
> > > > > > > +/* SysTick Base Address - fixed for all Cortex M3, M4 and M7 
> > > > > > > devices
> */
> > > > > > > +#define SYSTICK_BASE 0xE000E010
> > > > > > > +
> > > > > > > +struct cm3_systick {
> > > > > > > + uint32_t ctrl;
> > > > > > > + uint32_t reload;
> > > > > > > + uint32_t val;
> > > > > > > + uint32_t calibration;
> > > > > > > +};
> > > > > > > +
> > > > > > > +#define TIMER_LOAD_VAL   0x00FF
> > > > > > > +#define SYSTICK_CTRL_EN  BIT(0)

Re: [U-Boot] [PATCH] armv7m: Add SysTick timer driver

[Phil Edworthy]

Hi Vikas,

On 12 February 2017 20:53, Vikas MANOCHA wrote:
> > On Fri, Feb 03, 2017 at 02:48:40PM +, Phil Edworthy wrote:
> >
> > > The SysTick is a 24-bit down counter that is found on all ARM Cortex
> > > M3, M4, M7 devices and is always located at a fixed address.
> > >
> > > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > > ---
> > >  arch/arm/cpu/armv7m/Makefile|  2 +
> > >  arch/arm/cpu/armv7m/systick-timer.c | 91
> > > +
> > >  2 files changed, 93 insertions(+)
> > >  create mode 100644 arch/arm/cpu/armv7m/systick-timer.c
> > >
> > > diff --git a/arch/arm/cpu/armv7m/Makefile
> > > b/arch/arm/cpu/armv7m/Makefile index aff60e8..e1a6c40 100644
> > > --- a/arch/arm/cpu/armv7m/Makefile
> > > +++ b/arch/arm/cpu/armv7m/Makefile
> > > @@ -7,3 +7,5 @@
> > >
> > >  extra-y := start.o
> > >  obj-y += cpu.o
> > > +
> > > +obj-$(CONFIG_SYS_ARCH_TIMER) += systick-timer.o
> > > diff --git a/arch/arm/cpu/armv7m/systick-timer.c
> > > b/arch/arm/cpu/armv7m/systick-timer.c
> > > new file mode 100644
> > > index 000..6ccc2fb
> > > --- /dev/null
> > > +++ b/arch/arm/cpu/armv7m/systick-timer.c
> > > @@ -0,0 +1,91 @@
> > > +/*
> > > + * ARM Cortex M3/M4/M7 SysTick timer driver
> > > + * (C) Copyright 2017 Renesas Electronics Europe Ltd
> > > + *
> > > + * Based on arch/arm/mach-stm32/stm32f1/timer.c
> > > + * (C) Copyright 2015
> > > + * Kamil Lulko, <kamil.lu...@gmail.com>
> > > + *
> > > + * Copyright 2015 ATS Advanced Telematics Systems GmbH
> > > + * Copyright 2015 Konsulko Group, Matt Porter <mpor...@konsulko.com>
> > > + *
> > > + * SPDX-License-Identifier: GPL-2.0+
> > > + */
> > > +
> > > +#include 
> > > +#include 
> > > +
> > > +DECLARE_GLOBAL_DATA_PTR;
> > > +
> > > +/* SysTick Base Address - fixed for all Cortex M3, M4 and M7 devices */
> > > +#define SYSTICK_BASE 0xE000E010
> > > +
> > > +struct cm3_systick {
> > > + uint32_t ctrl;
> > > + uint32_t reload;
> > > + uint32_t val;
> 
> This register is current timer value, write of any value clears it.
> Rename to cur_val would avoid any confusion.
Ok, though I'm not sure that cur_val is any clearer :)

> > > + uint32_t calibration;
> > > +};
> > > +
> > > +#define TIMER_LOAD_VAL   0x00FF
> > > +#define SYSTICK_CTRL_EN  BIT(0)
> > > +/* Clock source: 0 = Ref clock, 1 = CPU clock */
> > > +#define SYSTICK_CTRL_CPU_CLK BIT(2)
> > > +
> > > +/* read the 24-bit timer */
> > > +static ulong read_timer(void)
> > > +{
> > > + struct cm3_systick *systick = (struct cm3_systick *)SYSTICK_BASE;
> > > +
> > > + /* The timer counts down, therefore convert to an incrementing timer */
> > > + return TIMER_LOAD_VAL - readl(>val); }
> 
> use mask to be sure of 24bit timer counter value.
Why? It's a 24-bit timer, the other bits are always zero.
 
> > > +
> > > +int timer_init(void)
> > > +{
> > > + struct cm3_systick *systick = (struct cm3_systick *)SYSTICK_BASE;
> > > +
> > > + writel(TIMER_LOAD_VAL, >reload);
> > > + writel(TIMER_LOAD_VAL, >val);
> 
> Writing load value gives the impression that it is load register, in fact it 
> is only
> clearing countflag.
Ok

> > > +
> > > +#ifdef CONFIG_ARMCORTEXM3_SYSTICK_CPU
> 
> Cortex M3 systick has only cpu clock source ?
No, it can have cpu clock or another clock external to the M3 core.
In our case we use the external clock because it is much slower than the
cpu clock, making the timer usable without interrupts. I can't imagine
anyone using the cpu clock unless they add code to handle the interrupt.
I'll add a comment to that effect.

Thanks
Phil

> Cheers,
> Vikas
> 
> > > + /* Use CPU clock, no interrupts */
> > > + writel(SYSTICK_CTRL_EN | SYSTICK_CTRL_CPU_CLK, >ctrl);
> > > +#else
> > > + /* Use external clock, no interrupts */
> > > + writel(SYSTICK_CTRL_EN, >ctrl); #endif
> > > +
> > > + gd->arch.tbl = 0;
> > > + gd->arch.tbu = 0;
> > > + gd->arch.lastinc = read_timer();
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/* return milli-seconds timer value */ ulong get_timer(ulong base) {
> > > + unsigned long long t = get_ticks() * 1000;
> > > +
> > > + return (ulong)((t / CONFIG_SYS_HZ_CLOCK)) - base; }
> > > +
> > > +unsigned long long get_ticks(void)
> > > +{
> > > + u32 now = read_timer();
> > > +
> > > + if (now >= gd->arch.lastinc)
> > > + gd->arch.tbl += (now - gd->arch.lastinc);
> > > + else
> > > + gd->arch.tbl += (TIMER_LOAD_VAL - gd->arch.lastinc) + now;
> > > +
> > > + gd->arch.lastinc = now;
> > > +
> > > + return gd->arch.tbl;
> > > +}
> > > +
> > > +ulong get_tbclk(void)
> > > +{
> > > + return CONFIG_SYS_HZ_CLOCK;
> > > +}
> >
> > And (cc'ing maintainers) we could use this in place of arch/arm/mach-
> stm32/*/timer.c I assume, yes?  Thanks!
> >
> > --
> > Tom
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] armv7m: Add SysTick timer driver

[Phil Edworthy]

Hi Tom,

On 07 February 2017 14:23, Tom Rini wrote:
> On Tue, Feb 07, 2017 at 02:19:39PM +0000, Phil Edworthy wrote:
> > Hi Kamil,
> >
> > On 07 February 2017 14:12, Kamil Lulko wrote:
> > > On Mon, Feb 6, 2017 at 12:16 AM, Tom Rini <tr...@konsulko.com> wrote:
> > > > On Fri, Feb 03, 2017 at 02:48:40PM +, Phil Edworthy wrote:
> > > >
> > > > > The SysTick is a 24-bit down counter that is found on all ARM Cortex
> > > > > M3, M4, M7 devices and is always located at a fixed address.
> > > > >
> > > > > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > > > > ---
> > > > >  arch/arm/cpu/armv7m/Makefile|  2 +
> > > > >  arch/arm/cpu/armv7m/systick-timer.c | 91
> > > +
> > > > >  2 files changed, 93 insertions(+)
> > > > >  create mode 100644 arch/arm/cpu/armv7m/systick-timer.c
> > > > >
> > > > > diff --git a/arch/arm/cpu/armv7m/Makefile
> > > b/arch/arm/cpu/armv7m/Makefile
> > > > > index aff60e8..e1a6c40 100644
> > > > > --- a/arch/arm/cpu/armv7m/Makefile
> > > > > +++ b/arch/arm/cpu/armv7m/Makefile
> > > > > @@ -7,3 +7,5 @@
> > > > >
> > > > >  extra-y := start.o
> > > > >  obj-y += cpu.o
> > > > > +
> > > > > +obj-$(CONFIG_SYS_ARCH_TIMER) += systick-timer.o
> > > > > diff --git a/arch/arm/cpu/armv7m/systick-timer.c
> > > b/arch/arm/cpu/armv7m/systick-timer.c
> > > > > new file mode 100644
> > > > > index 000..6ccc2fb
> > > > > --- /dev/null
> > > > > +++ b/arch/arm/cpu/armv7m/systick-timer.c
> > > > > @@ -0,0 +1,91 @@
> > > > > +/*
> > > > > + * ARM Cortex M3/M4/M7 SysTick timer driver
> > > > > + * (C) Copyright 2017 Renesas Electronics Europe Ltd
> > > > > + *
> > > > > + * Based on arch/arm/mach-stm32/stm32f1/timer.c
> > > > > + * (C) Copyright 2015
> > > > > + * Kamil Lulko, <kamil.lu...@gmail.com>
> > > > > + *
> > > > > + * Copyright 2015 ATS Advanced Telematics Systems GmbH
> > > > > + * Copyright 2015 Konsulko Group, Matt Porter
> <mpor...@konsulko.com>
> > > > > + *
> > > > > + * SPDX-License-Identifier: GPL-2.0+
> > > > > + */
> > > > > +
> > > > > +#include 
> > > > > +#include 
> > > > > +
> > > > > +DECLARE_GLOBAL_DATA_PTR;
> > > > > +
> > > > > +/* SysTick Base Address - fixed for all Cortex M3, M4 and M7 devices 
> > > > > */
> > > > > +#define SYSTICK_BASE 0xE000E010
> > > > > +
> > > > > +struct cm3_systick {
> > > > > + uint32_t ctrl;
> > > > > + uint32_t reload;
> > > > > + uint32_t val;
> > > > > + uint32_t calibration;
> > > > > +};
> > > > > +
> > > > > +#define TIMER_LOAD_VAL   0x00FF
> > > > > +#define SYSTICK_CTRL_EN  BIT(0)
> > > > > +/* Clock source: 0 = Ref clock, 1 = CPU clock */
> > > > > +#define SYSTICK_CTRL_CPU_CLK BIT(2)
> > > > > +
> > > > > +/* read the 24-bit timer */
> > > > > +static ulong read_timer(void)
> > > > > +{
> > > > > + struct cm3_systick *systick = (struct cm3_systick 
> > > > > *)SYSTICK_BASE;
> > > > > +
> > > > > + /* The timer counts down, therefore convert to an incrementing 
> > > > > timer
> */
> > > > > + return TIMER_LOAD_VAL - readl(>val);
> > > > > +}
> > > > > +
> > > > > +int timer_init(void)
> > > > > +{
> > > > > + struct cm3_systick *systick = (struct cm3_systick 
> > > > > *)SYSTICK_BASE;
> > > > > +
> > > > > + writel(TIMER_LOAD_VAL, >reload);
> > > > > + writel(TIMER_LOAD_VAL, >val);
> > > > > +
> > > > > +#ifdef CONFIG_ARMCORTEXM3_SYSTICK_CPU
> > > > > + /* Use CPU clock, no interrupts */
> > > > > + writel(SYSTICK_CTRL_EN | SYSTICK_CTRL_CPU_CLK, >ctrl);
> > > > > +#else
> > > > > + /* Use external clock, no i

Re: [U-Boot] [PATCH] armv7m: Add SysTick timer driver

[Phil Edworthy]

Hi Kamil,

On 07 February 2017 14:12, Kamil Lulko wrote:
> On Mon, Feb 6, 2017 at 12:16 AM, Tom Rini <tr...@konsulko.com> wrote:
> > On Fri, Feb 03, 2017 at 02:48:40PM +, Phil Edworthy wrote:
> >
> > > The SysTick is a 24-bit down counter that is found on all ARM Cortex
> > > M3, M4, M7 devices and is always located at a fixed address.
> > >
> > > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > > ---
> > >  arch/arm/cpu/armv7m/Makefile|  2 +
> > >  arch/arm/cpu/armv7m/systick-timer.c | 91
> +
> > >  2 files changed, 93 insertions(+)
> > >  create mode 100644 arch/arm/cpu/armv7m/systick-timer.c
> > >
> > > diff --git a/arch/arm/cpu/armv7m/Makefile
> b/arch/arm/cpu/armv7m/Makefile
> > > index aff60e8..e1a6c40 100644
> > > --- a/arch/arm/cpu/armv7m/Makefile
> > > +++ b/arch/arm/cpu/armv7m/Makefile
> > > @@ -7,3 +7,5 @@
> > >
> > >  extra-y := start.o
> > >  obj-y += cpu.o
> > > +
> > > +obj-$(CONFIG_SYS_ARCH_TIMER) += systick-timer.o
> > > diff --git a/arch/arm/cpu/armv7m/systick-timer.c
> b/arch/arm/cpu/armv7m/systick-timer.c
> > > new file mode 100644
> > > index 000..6ccc2fb
> > > --- /dev/null
> > > +++ b/arch/arm/cpu/armv7m/systick-timer.c
> > > @@ -0,0 +1,91 @@
> > > +/*
> > > + * ARM Cortex M3/M4/M7 SysTick timer driver
> > > + * (C) Copyright 2017 Renesas Electronics Europe Ltd
> > > + *
> > > + * Based on arch/arm/mach-stm32/stm32f1/timer.c
> > > + * (C) Copyright 2015
> > > + * Kamil Lulko, <kamil.lu...@gmail.com>
> > > + *
> > > + * Copyright 2015 ATS Advanced Telematics Systems GmbH
> > > + * Copyright 2015 Konsulko Group, Matt Porter <mpor...@konsulko.com>
> > > + *
> > > + * SPDX-License-Identifier: GPL-2.0+
> > > + */
> > > +
> > > +#include 
> > > +#include 
> > > +
> > > +DECLARE_GLOBAL_DATA_PTR;
> > > +
> > > +/* SysTick Base Address - fixed for all Cortex M3, M4 and M7 devices */
> > > +#define SYSTICK_BASE 0xE000E010
> > > +
> > > +struct cm3_systick {
> > > + uint32_t ctrl;
> > > + uint32_t reload;
> > > + uint32_t val;
> > > + uint32_t calibration;
> > > +};
> > > +
> > > +#define TIMER_LOAD_VAL   0x00FF
> > > +#define SYSTICK_CTRL_EN  BIT(0)
> > > +/* Clock source: 0 = Ref clock, 1 = CPU clock */
> > > +#define SYSTICK_CTRL_CPU_CLK BIT(2)
> > > +
> > > +/* read the 24-bit timer */
> > > +static ulong read_timer(void)
> > > +{
> > > + struct cm3_systick *systick = (struct cm3_systick *)SYSTICK_BASE;
> > > +
> > > + /* The timer counts down, therefore convert to an incrementing 
> > > timer */
> > > + return TIMER_LOAD_VAL - readl(>val);
> > > +}
> > > +
> > > +int timer_init(void)
> > > +{
> > > + struct cm3_systick *systick = (struct cm3_systick *)SYSTICK_BASE;
> > > +
> > > + writel(TIMER_LOAD_VAL, >reload);
> > > + writel(TIMER_LOAD_VAL, >val);
> > > +
> > > +#ifdef CONFIG_ARMCORTEXM3_SYSTICK_CPU
> > > + /* Use CPU clock, no interrupts */
> > > + writel(SYSTICK_CTRL_EN | SYSTICK_CTRL_CPU_CLK, >ctrl);
> > > +#else
> > > + /* Use external clock, no interrupts */
> > > + writel(SYSTICK_CTRL_EN, >ctrl);
> > > +#endif
> > > +
> > > + gd->arch.tbl = 0;
> > > + gd->arch.tbu = 0;
> > > + gd->arch.lastinc = read_timer();
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/* return milli-seconds timer value */
> > > +ulong get_timer(ulong base)
> > > +{
> > > + unsigned long long t = get_ticks() * 1000;
> > > +
> > > + return (ulong)((t / CONFIG_SYS_HZ_CLOCK)) - base;
> > > +}
> > > +
> > > +unsigned long long get_ticks(void)
> > > +{
> > > + u32 now = read_timer();
> > > +
> > > + if (now >= gd->arch.lastinc)
> > > + gd->arch.tbl += (now - gd->arch.lastinc);
> > > + else
> > > + gd->arch.tbl += (TIMER_LOAD_VAL - gd->arch.lastinc) + now;
> > > +
> > > + gd->arch.lastinc = now;
> > > +
> > > + return gd->arch.tbl;
> > > +}
> > > +
> > > +ulong get_tbclk(void)
> > > +{
> > > + return CONFIG_SYS_HZ_CLOCK;
> > > +}
> >
> > And (cc'ing maintainers) we could use this in place of
> > arch/arm/mach-stm32/*/timer.c I assume, yes?  Thanks!
> >
> > --
> > Tom
> 
> Yes, however as far as I remember the systick is only 24-bit and
> usually runs on CPU clock so it will roll over too fast to be useful
> without using interrupts. Maybe I have missed something?

Yes, it is only a 24-bit counter. On the device I am using, the clock
is not the cpu clock, but a low speed clock at 6.25MHz. Even at that
low clock it will still wrap every ~2.6 seconds.

> /Kamil

Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] armv7m: Make reset_cpu() weak

[Phil Edworthy]

Some devices/boards have their own way to reset the cpu.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 arch/arm/cpu/armv7m/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/cpu/armv7m/cpu.c b/arch/arm/cpu/armv7m/cpu.c
index 58cde93..c3e4734 100644
--- a/arch/arm/cpu/armv7m/cpu.c
+++ b/arch/arm/cpu/armv7m/cpu.c
@@ -24,7 +24,7 @@ int cleanup_before_linux(void)
 /*
  * Perform the low-level reset.
  */
-void reset_cpu(ulong addr)
+__weak void reset_cpu(ulong addr)
 {
/*
 * Perform reset but keep priority group unchanged.
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] armv7m: Add SysTick timer driver

[Phil Edworthy]

The SysTick is a 24-bit down counter that is found on all ARM Cortex
M3, M4, M7 devices and is always located at a fixed address.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
Resend with Albert's new email address - get_maintainer.pl gave the old one.
---
 arch/arm/cpu/armv7m/Makefile|  2 +
 arch/arm/cpu/armv7m/systick-timer.c | 91 +
 2 files changed, 93 insertions(+)
 create mode 100644 arch/arm/cpu/armv7m/systick-timer.c

diff --git a/arch/arm/cpu/armv7m/Makefile b/arch/arm/cpu/armv7m/Makefile
index aff60e8..e1a6c40 100644
--- a/arch/arm/cpu/armv7m/Makefile
+++ b/arch/arm/cpu/armv7m/Makefile
@@ -7,3 +7,5 @@
 
 extra-y := start.o
 obj-y += cpu.o
+
+obj-$(CONFIG_SYS_ARCH_TIMER) += systick-timer.o
diff --git a/arch/arm/cpu/armv7m/systick-timer.c 
b/arch/arm/cpu/armv7m/systick-timer.c
new file mode 100644
index 000..6ccc2fb
--- /dev/null
+++ b/arch/arm/cpu/armv7m/systick-timer.c
@@ -0,0 +1,91 @@
+/*
+ * ARM Cortex M3/M4/M7 SysTick timer driver
+ * (C) Copyright 2017 Renesas Electronics Europe Ltd
+ *
+ * Based on arch/arm/mach-stm32/stm32f1/timer.c
+ * (C) Copyright 2015
+ * Kamil Lulko, <kamil.lu...@gmail.com>
+ *
+ * Copyright 2015 ATS Advanced Telematics Systems GmbH
+ * Copyright 2015 Konsulko Group, Matt Porter <mpor...@konsulko.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include 
+#include 
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/* SysTick Base Address - fixed for all Cortex M3, M4 and M7 devices */
+#define SYSTICK_BASE   0xE000E010
+
+struct cm3_systick {
+   uint32_t ctrl;
+   uint32_t reload;
+   uint32_t val;
+   uint32_t calibration;
+};
+
+#define TIMER_LOAD_VAL 0x00FF
+#define SYSTICK_CTRL_ENBIT(0)
+/* Clock source: 0 = Ref clock, 1 = CPU clock */
+#define SYSTICK_CTRL_CPU_CLK   BIT(2)
+
+/* read the 24-bit timer */
+static ulong read_timer(void)
+{
+   struct cm3_systick *systick = (struct cm3_systick *)SYSTICK_BASE;
+
+   /* The timer counts down, therefore convert to an incrementing timer */
+   return TIMER_LOAD_VAL - readl(>val);
+}
+
+int timer_init(void)
+{
+   struct cm3_systick *systick = (struct cm3_systick *)SYSTICK_BASE;
+
+   writel(TIMER_LOAD_VAL, >reload);
+   writel(TIMER_LOAD_VAL, >val);
+
+#ifdef CONFIG_ARMCORTEXM3_SYSTICK_CPU
+   /* Use CPU clock, no interrupts */
+   writel(SYSTICK_CTRL_EN | SYSTICK_CTRL_CPU_CLK, >ctrl);
+#else
+   /* Use external clock, no interrupts */
+   writel(SYSTICK_CTRL_EN, >ctrl);
+#endif
+
+   gd->arch.tbl = 0;
+   gd->arch.tbu = 0;
+   gd->arch.lastinc = read_timer();
+
+   return 0;
+}
+
+/* return milli-seconds timer value */
+ulong get_timer(ulong base)
+{
+   unsigned long long t = get_ticks() * 1000;
+
+   return (ulong)((t / CONFIG_SYS_HZ_CLOCK)) - base;
+}
+
+unsigned long long get_ticks(void)
+{
+   u32 now = read_timer();
+
+   if (now >= gd->arch.lastinc)
+   gd->arch.tbl += (now - gd->arch.lastinc);
+   else
+   gd->arch.tbl += (TIMER_LOAD_VAL - gd->arch.lastinc) + now;
+
+   gd->arch.lastinc = now;
+
+   return gd->arch.tbl;
+}
+
+ulong get_tbclk(void)
+{
+   return CONFIG_SYS_HZ_CLOCK;
+}
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] armv7m: Add SysTick timer driver

[Phil Edworthy]

The SysTick is a 24-bit down counter that is found on all ARM Cortex
M3, M4, M7 devices and is always located at a fixed address.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 arch/arm/cpu/armv7m/Makefile|  2 +
 arch/arm/cpu/armv7m/systick-timer.c | 91 +
 2 files changed, 93 insertions(+)
 create mode 100644 arch/arm/cpu/armv7m/systick-timer.c

diff --git a/arch/arm/cpu/armv7m/Makefile b/arch/arm/cpu/armv7m/Makefile
index aff60e8..e1a6c40 100644
--- a/arch/arm/cpu/armv7m/Makefile
+++ b/arch/arm/cpu/armv7m/Makefile
@@ -7,3 +7,5 @@
 
 extra-y := start.o
 obj-y += cpu.o
+
+obj-$(CONFIG_SYS_ARCH_TIMER) += systick-timer.o
diff --git a/arch/arm/cpu/armv7m/systick-timer.c 
b/arch/arm/cpu/armv7m/systick-timer.c
new file mode 100644
index 000..6ccc2fb
--- /dev/null
+++ b/arch/arm/cpu/armv7m/systick-timer.c
@@ -0,0 +1,91 @@
+/*
+ * ARM Cortex M3/M4/M7 SysTick timer driver
+ * (C) Copyright 2017 Renesas Electronics Europe Ltd
+ *
+ * Based on arch/arm/mach-stm32/stm32f1/timer.c
+ * (C) Copyright 2015
+ * Kamil Lulko, <kamil.lu...@gmail.com>
+ *
+ * Copyright 2015 ATS Advanced Telematics Systems GmbH
+ * Copyright 2015 Konsulko Group, Matt Porter <mpor...@konsulko.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include 
+#include 
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/* SysTick Base Address - fixed for all Cortex M3, M4 and M7 devices */
+#define SYSTICK_BASE   0xE000E010
+
+struct cm3_systick {
+   uint32_t ctrl;
+   uint32_t reload;
+   uint32_t val;
+   uint32_t calibration;
+};
+
+#define TIMER_LOAD_VAL 0x00FF
+#define SYSTICK_CTRL_ENBIT(0)
+/* Clock source: 0 = Ref clock, 1 = CPU clock */
+#define SYSTICK_CTRL_CPU_CLK   BIT(2)
+
+/* read the 24-bit timer */
+static ulong read_timer(void)
+{
+   struct cm3_systick *systick = (struct cm3_systick *)SYSTICK_BASE;
+
+   /* The timer counts down, therefore convert to an incrementing timer */
+   return TIMER_LOAD_VAL - readl(>val);
+}
+
+int timer_init(void)
+{
+   struct cm3_systick *systick = (struct cm3_systick *)SYSTICK_BASE;
+
+   writel(TIMER_LOAD_VAL, >reload);
+   writel(TIMER_LOAD_VAL, >val);
+
+#ifdef CONFIG_ARMCORTEXM3_SYSTICK_CPU
+   /* Use CPU clock, no interrupts */
+   writel(SYSTICK_CTRL_EN | SYSTICK_CTRL_CPU_CLK, >ctrl);
+#else
+   /* Use external clock, no interrupts */
+   writel(SYSTICK_CTRL_EN, >ctrl);
+#endif
+
+   gd->arch.tbl = 0;
+   gd->arch.tbu = 0;
+   gd->arch.lastinc = read_timer();
+
+   return 0;
+}
+
+/* return milli-seconds timer value */
+ulong get_timer(ulong base)
+{
+   unsigned long long t = get_ticks() * 1000;
+
+   return (ulong)((t / CONFIG_SYS_HZ_CLOCK)) - base;
+}
+
+unsigned long long get_ticks(void)
+{
+   u32 now = read_timer();
+
+   if (now >= gd->arch.lastinc)
+   gd->arch.tbl += (now - gd->arch.lastinc);
+   else
+   gd->arch.tbl += (TIMER_LOAD_VAL - gd->arch.lastinc) + now;
+
+   gd->arch.lastinc = now;
+
+   return gd->arch.tbl;
+}
+
+ulong get_tbclk(void)
+{
+   return CONFIG_SYS_HZ_CLOCK;
+}
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] keystone2: Rename local CONFIG_ symbol

[Phil Edworthy]

CONFIG_SPL_STACK_SIZE is not a config option, so rename it.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
Not tested at all since I don't have the HW
---
 include/configs/ti_armv7_keystone2.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/configs/ti_armv7_keystone2.h 
b/include/configs/ti_armv7_keystone2.h
index d120c69..5d4ef58 100644
--- a/include/configs/ti_armv7_keystone2.h
+++ b/include/configs/ti_armv7_keystone2.h
@@ -47,11 +47,11 @@
 #define CONFIG_SYS_SPL_MALLOC_START(CONFIG_SPL_BSS_START_ADDR + \
CONFIG_SPL_BSS_MAX_SIZE)
 #define CONFIG_SYS_SPL_MALLOC_SIZE (32 * 1024)
-#define CONFIG_SPL_STACK_SIZE  (8 * 1024)
+#define KEYSTONE_SPL_STACK_SIZE(8 * 1024)
 #define CONFIG_SPL_STACK   (CONFIG_SYS_SPL_MALLOC_START + \
CONFIG_SYS_SPL_MALLOC_SIZE + \
SPL_MALLOC_F_SIZE + \
-   CONFIG_SPL_STACK_SIZE - 4)
+   KEYSTONE_SPL_STACK_SIZE - 4)
 #define CONFIG_SPL_SPI_LOAD
 #define CONFIG_SYS_SPI_U_BOOT_OFFS CONFIG_SPL_PAD_TO
 
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] microblaze: Rename local CONFIG_ symbol

[Phil Edworthy]

CONFIG_SPL_STACK_SIZE is not a config option, so rename it.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 include/configs/microblaze-generic.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/configs/microblaze-generic.h 
b/include/configs/microblaze-generic.h
index 24de528..cff054a 100644
--- a/include/configs/microblaze-generic.h
+++ b/include/configs/microblaze-generic.h
@@ -305,13 +305,13 @@
 CONFIG_SYS_MALLOC_F_LEN)
 
 /* Just for sure that there is a space for stack */
-#define CONFIG_SPL_STACK_SIZE  0x100
+#define MICROBLAZE_SPL_STACK_SIZE  0x100
 
 #define CONFIG_SYS_UBOOT_START CONFIG_SYS_TEXT_BASE
 
 #define CONFIG_SPL_MAX_FOOTPRINT   (CONFIG_SYS_INIT_RAM_SIZE - \
 CONFIG_SYS_INIT_RAM_ADDR - \
 CONFIG_SYS_MALLOC_F_LEN - \
-CONFIG_SPL_STACK_SIZE)
+MICROBLAZE_SPL_STACK_SIZE)
 
 #endif /* __CONFIG_H */
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] sf: ids: Correct Micron n25q128 flags

[Phil Edworthy]

Hi Jagan,

On 21 December 2016 11:21, Jagan Teki wrote:
> On Wed, Dec 21, 2016 at 12:15 PM, Phil Edworthy
> <phil.edwor...@renesas.com> wrote:
> > On 21 December 2016 11:10, Jagan Teki wrote:
> >> On Fri, Dec 16, 2016 at 11:38 AM, Phil Edworthy
> >> <phil.edwor...@renesas.com> wrote:
> >> > The n25q128 devices support 4K erase.
> >>
> >> It's OK, but go with 64K erase.
> > Sorry, I don’t understand your comment.
> > Do you mean we should stick with 64K erase?
> > If so, then what about all the other Micron devices that have SECT_4K?
> 
> It's because we have tested 64K on these parts and it's fine why can't
> we move 4K because 64K look faster, did you find any issue with 64K on
> this specific part?
No, I didn’t find any problems with it at 64K erase.
As you say, erasing 64K is much faster than 16 x 4K erase. So should we
move the other Micron parts to 64K?

Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] sf: ids: Correct Micron n25q128 flags

[Phil Edworthy]

Hi Jagan,

On 21 December 2016 11:10, Jagan Teki wrote:
> On Fri, Dec 16, 2016 at 11:38 AM, Phil Edworthy
> <phil.edwor...@renesas.com> wrote:
> > The n25q128 devices support 4K erase.
> 
> It's OK, but go with 64K erase.
Sorry, I don’t understand your comment.
Do you mean we should stick with 64K erase?
If so, then what about all the other Micron devices that have SECT_4K?

Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] sf: ids: Correct Micron n25q128 flags

[Phil Edworthy]

The n25q128 devices support 4K erase.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 drivers/mtd/spi/spi_flash_ids.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi/spi_flash_ids.c b/drivers/mtd/spi/spi_flash_ids.c
index edca94e..e31131e 100644
--- a/drivers/mtd/spi/spi_flash_ids.c
+++ b/drivers/mtd/spi/spi_flash_ids.c
@@ -123,8 +123,8 @@ const struct spi_flash_info spi_flash_ids[] = {
{"n25q32a",INFO(0x20bb16, 0x0,  64 * 1024,64, RD_FULL | 
WR_QPP | SECT_4K) },
{"n25q64", INFO(0x20ba17, 0x0,  64 * 1024,   128, RD_FULL | 
WR_QPP | SECT_4K) },
{"n25q64a",INFO(0x20bb17, 0x0,  64 * 1024,   128, RD_FULL | 
WR_QPP | SECT_4K) },
-   {"n25q128",INFO(0x20ba18, 0x0,  64 * 1024,   256, RD_FULL | 
WR_QPP) },
-   {"n25q128a",   INFO(0x20bb18, 0x0,  64 * 1024,   256, RD_FULL | 
WR_QPP) },
+   {"n25q128",INFO(0x20ba18, 0x0,  64 * 1024,   256, RD_FULL | 
WR_QPP | SECT_4K) },
+   {"n25q128a",   INFO(0x20bb18, 0x0,  64 * 1024,   256, RD_FULL | 
WR_QPP | SECT_4K) },
{"n25q256",INFO(0x20ba19, 0x0,  64 * 1024,   512, RD_FULL | 
WR_QPP | SECT_4K) },
{"n25q256a",   INFO(0x20bb19, 0x0,  64 * 1024,   512, RD_FULL | 
WR_QPP | SECT_4K) },
{"n25q512",INFO(0x20ba20, 0x0,  64 * 1024,  1024, RD_FULL | 
WR_QPP | E_FSR | SECT_4K) },
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] Problem with qspi when using micron devices

[Phil Edworthy]

Hi Jagan,

On 07 December 2016 08:03, Phil Edworthy wrote:
> Hi Jagan,
> 
> When using Micron devices, SPI flash with quad mode does not work since
> commit
> c56ae7519f141523ba1248b22b5b5169b21772fe "sf: Fix quad bit set for micron
> devices".
> 
> This has been pointed out before, details about why the patch does work are
> here:
> http://lists.denx.de/pipermail/u-boot/2016-March/248566.html
> 
> I know you are working on spi-nor, but it really would be good to revert the
> patch.

Is there any reason why you cannot revert 
c56ae7519f141523ba1248b22b5b5169b21772fe?
>From what I see, this patch has caused a few problems already:
http://lists.denx.de/pipermail/u-boot/2016-March/248566.html
http://lists.denx.de/pipermail/u-boot/2016-July/261005.html 
http://lists.denx.de/pipermail/u-boot/2016-November/271634.html

Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] net: phy: vitesse: Fix cis8204 RGMII_ID code

[Phil Edworthy]

Commit 79e86ccb3786c8b20004db3fa10a70049456f580 "vitesse: remove duplicated
argument to ||" correctly removed a redundant check.

However, I believe that the original code was simply wrong, and should have
been checking against RGMII_ID.

To fix this and avoid similar problems in the future, use the
phy_interface_is_rgmii helper function.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
This has _not_ been tested in any way, shape or form! It was picked
up when converting PHY code to use the phy_interface_is_rgmii helper
function.
---
 drivers/net/phy/vitesse.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/phy/vitesse.c b/drivers/net/phy/vitesse.c
index 2635b82..11f4b66 100644
--- a/drivers/net/phy/vitesse.c
+++ b/drivers/net/phy/vitesse.c
@@ -128,9 +128,7 @@ static int cis8204_config(struct phy_device *phydev)
 
genphy_config_aneg(phydev);
 
-   if ((phydev->interface == PHY_INTERFACE_MODE_RGMII) ||
-   (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) ||
-   (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID))
+   if (phy_interface_is_rgmii(phydev))
phy_write(phydev, MDIO_DEVAD_NONE, MIIM_CIS8204_EPHY_CON,
MIIM_CIS8204_EPHYCON_INIT |
MIIM_CIS8204_EPHYCON_RGMII);
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 3/3] net: phy: Marvell: Use phy_interface_is_rgmii helper function

[Phil Edworthy]

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
Reviewed-by: Stefan Roese <s...@denx.de>
Acked-by: Joe Hershberger <joe.hershber...@ni.com>
---
v3:
 No changes
v2:
 No changes
---
 drivers/net/phy/marvell.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 4ddb85d..00d0719 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -177,10 +177,7 @@ static int m88es_config(struct phy_device *phydev)
 {
int reg;
 
-   if ((phydev->interface == PHY_INTERFACE_MODE_RGMII) ||
-   (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
-   (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) ||
-   (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)) {
+   if (phy_interface_is_rgmii(phydev)) {
reg = phy_read(phydev,
MDIO_DEVAD_NONE, MIIM_88E_PHY_EXT_CR);
if ((phydev->interface == PHY_INTERFACE_MODE_RGMII) ||
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 2/3] net: phy: Add support for Marvell M88E1512

[Phil Edworthy]

This device also works with the 88E1518 code, so we just adjust
the UID mask accordingly.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
v3:
 Correct the mask.
v2:
 Don't add a new entry, just adjust the UID mask.
---
 drivers/net/phy/marvell.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 06029c0..4ddb85d 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -560,10 +560,15 @@ static struct phy_driver M88E1510_driver = {
.shutdown = _shutdown,
 };
 
+/*
+ * This supports:
+ *  88E1518, uid 0x1410dd1
+ *  88E1512, uid 0x1410dd4
+ */
 static struct phy_driver M88E1518_driver = {
.name = "Marvell 88E1518",
-   .uid = 0x1410dd1,
-   .mask = 0xfff,
+   .uid = 0x1410dd0,
+   .mask = 0xffa,
.features = PHY_GBIT_FEATURES,
.config = _config,
.startup = _startup,
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 0/3] net: phy: Add Marvell M88E1512

[Phil Edworthy]

These patches add support for the Marvell M88E1512 PHY.

It turns out that it behaves exactly the same as 88E1518, so it's
just a matter of fixing up the uid/mask.

Phil Edworthy (3):
  net: phy: Fix mask so that we can identify Marvell 88E1518
  net: phy: Add support for Marvell M88E1512
  net: phy: Marvell: Use phy_interface_is_rgmii helper function

 drivers/net/phy/marvell.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 1/3] net: phy: Fix mask so that we can identify Marvell 88E1518

[Phil Edworthy]

The mask for the 88E1510 meant that the 88E1518 code would never be
used.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
Reviewed-by: Stefan Roese <s...@denx.de>
Acked-by: Joe Hershberger <joe.hershber...@ni.com>
---
Note: This has only been tested on a board that uses a Marvell 88E1512
PHY, see subsequent patches.

v3:
 No changes
v2:
 No changes
---
 drivers/net/phy/marvell.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 4eeb0f6..06029c0 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -553,7 +553,7 @@ static struct phy_driver M88E1149S_driver = {
 static struct phy_driver M88E1510_driver = {
.name = "Marvell 88E1510",
.uid = 0x1410dd0,
-   .mask = 0xff0,
+   .mask = 0xfff,
.features = PHY_GBIT_FEATURES,
.config = _config,
.startup = _startup,
@@ -563,7 +563,7 @@ static struct phy_driver M88E1510_driver = {
 static struct phy_driver M88E1518_driver = {
.name = "Marvell 88E1518",
.uid = 0x1410dd1,
-   .mask = 0xff0,
+   .mask = 0xfff,
.features = PHY_GBIT_FEATURES,
.config = _config,
.startup = _startup,
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 3/5] net: phy: Marvell 88E151x: Add support for RGMII

[Phil Edworthy]

Hi Joe

On 12 December 2016 10:15, Phil Edworthy wrote:
> On 12 December 2016 09:25, Phil Edworthy wrote:
> > On 09 December 2016 18:59, Joe Hershberger wrote:
> > > On Fri, Dec 9, 2016 at 7:38 AM, Phil Edworthy <phil.edwor...@renesas.com>
> > > wrote:
> > > > This has been tested with a Marvell 88E1512 PHY.
> > > >
> > > > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > > > Reviewed-by: Stefan Roese <s...@denx.de>
> > > > ---
> > > > v2:
> > > >  Rebased on top of Joe's code to use macros
> > > > ---
> > > >  drivers/net/phy/marvell.c | 17 +
> > > >  1 file changed, 13 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> > > > index 5adfe7d..646b00d 100644
> > > > --- a/drivers/net/phy/marvell.c
> > > > +++ b/drivers/net/phy/marvell.c
> > > > @@ -94,6 +94,7 @@
> > > >  #define MIIM_88E151x_INT_EN_OFFS   7
> > > >  /* Page 18 registers */
> > > >  #define MIIM_88E151x_GENERAL_CTRL  20
> > > > +#define MIIM_88E151x_MODE_RGMII0
> > >
> > > I'm not sure why this is needed. The HW reset value for the mode is
> > > 0x7 (RGMII auto detect mode). Would it not be sufficient to simply not
> > > change it if the mode is RGMII?
> > I don’t have a manual for any Marvell devices, just some small bits
> > of information. All I can tell you is that without this patch, RGMII on
> > 1512 doesn't work.
> I messed around with the code a bit and found that I don’t need to
> write to the MIIM_88E151x_GENERAL_CTRL register to set it to RGMII,
> but I must do a soft reset of the PHY before calling m88es_config().
> 
> Would you prefer something like this instead?
> @@ -340,6 +333,9 @@ static int m88e1518_config(struct phy_device *phydev)
> udelay(100);
> }
> 
> +   if (phy_interface_is_rgmii(phydev))
> +   phy_reset(phydev);
> +
> return m88es_config(phydev);
>  }

Sometimes I hate PHYs! My v2 patch for detecting the 1512 is wrong; the
PHY still worked to some extent since there was still power on the board.
I _thought_ the power switch was cutting power to the PHY, but no it's
not.

After fixing up the 1512 detection again, there doesn't appear to be a
problem with detecting RGMII. The PHY was only gets a 10M link,  I was
only getting a 1Gb link by forcing MIIM_88E151x_MODE bits to 0, as per
this patch. And the reason for the wrong speed is that I used the wrong
PHY mode; RGMII instead of RGMII_ID.

So the upshot of all this is that this patch is not needed at all... I'll send
a new patch that changes the uid/mask to support the 88E1512.


> > > >  #define MIIM_88E151x_MODE_SGMII1
> > > >  #define MIIM_88E151x_RESET_OFFS15
> > > >
> > > > @@ -315,14 +316,22 @@ static int m88e1518_config(struct phy_device
> > > *phydev)
> > > > phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2159);
> > > > phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE,
> > > 0x);
> > > >
> > > > -   /* SGMII-to-Copper mode initialization */
> > > > -   if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
> > > > +   /* SGMII/RGMII-to-Copper mode initialization */
> > > > +   if ((phydev->interface == PHY_INTERFACE_MODE_SGMII) ||
> > > > +   phy_interface_is_rgmii(phydev)) {
> > > > +   u16 mode;
> > > > +
> > > > +   if (phydev->interface == PHY_INTERFACE_MODE_SGMII)
> > > > +   mode = MIIM_88E151x_MODE_SGMII;
> > > > +   else
> > > > +   mode = MIIM_88E151x_MODE_RGMII;
> > > > +
> > > > /* Select page 18 */
> > > > phy_write(phydev, MDIO_DEVAD_NONE,
> > MIIM_88E1118_PHY_PAGE,
> > > 18);
> > > >
> > > > -   /* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
> > > > +   /* In reg 20, write MODE[2:0], SGMII or RGMII to Copper 
> > > > */
> > > > m88e1518_phy_writebits(phydev, 
> > > > MIIM_88E151x_GENERAL_CTRL,
> > > > -  0, 3, MIIM_88E151x_MODE_SGMII);
> > > > +  0, 3, mode);
> > > >
> > > > /* PHY reset is necessary after changing MODE[2:0] */
> > > > m88e1518_phy_writebits(phydev, 
> > > > MIIM_88E151x_GENERAL_CTRL,
> > > > --
> > > > 2.7.4

Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 3/5] net: phy: Marvell 88E151x: Add support for RGMII

[Phil Edworthy]

Hi Joe,

On 12 December 2016 09:25, Phil Edworthy wrote:
> On 09 December 2016 18:59, Joe Hershberger wrote:
> > On Fri, Dec 9, 2016 at 7:38 AM, Phil Edworthy <phil.edwor...@renesas.com>
> > wrote:
> > > This has been tested with a Marvell 88E1512 PHY.
> > >
> > > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > > Reviewed-by: Stefan Roese <s...@denx.de>
> > > ---
> > > v2:
> > >  Rebased on top of Joe's code to use macros
> > > ---
> > >  drivers/net/phy/marvell.c | 17 +
> > >  1 file changed, 13 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> > > index 5adfe7d..646b00d 100644
> > > --- a/drivers/net/phy/marvell.c
> > > +++ b/drivers/net/phy/marvell.c
> > > @@ -94,6 +94,7 @@
> > >  #define MIIM_88E151x_INT_EN_OFFS   7
> > >  /* Page 18 registers */
> > >  #define MIIM_88E151x_GENERAL_CTRL  20
> > > +#define MIIM_88E151x_MODE_RGMII0
> >
> > I'm not sure why this is needed. The HW reset value for the mode is
> > 0x7 (RGMII auto detect mode). Would it not be sufficient to simply not
> > change it if the mode is RGMII?
> I don’t have a manual for any Marvell devices, just some small bits
> of information. All I can tell you is that without this patch, RGMII on
> 1512 doesn't work.
I messed around with the code a bit and found that I don’t need to
write to the MIIM_88E151x_GENERAL_CTRL register to set it to RGMII,
but I must do a soft reset of the PHY before calling m88es_config().

Would you prefer something like this instead?
@@ -340,6 +333,9 @@ static int m88e1518_config(struct phy_device *phydev)
udelay(100);
}
 
+   if (phy_interface_is_rgmii(phydev))
+   phy_reset(phydev);
+
return m88es_config(phydev);
 }


> > >  #define MIIM_88E151x_MODE_SGMII1
> > >  #define MIIM_88E151x_RESET_OFFS15
> > >
> > > @@ -315,14 +316,22 @@ static int m88e1518_config(struct phy_device
> > *phydev)
> > > phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2159);
> > > phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE,
> > 0x);
> > >
> > > -   /* SGMII-to-Copper mode initialization */
> > > -   if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
> > > +   /* SGMII/RGMII-to-Copper mode initialization */
> > > +   if ((phydev->interface == PHY_INTERFACE_MODE_SGMII) ||
> > > +   phy_interface_is_rgmii(phydev)) {
> > > +   u16 mode;
> > > +
> > > +   if (phydev->interface == PHY_INTERFACE_MODE_SGMII)
> > > +   mode = MIIM_88E151x_MODE_SGMII;
> > > +   else
> > > +   mode = MIIM_88E151x_MODE_RGMII;
> > > +
> > > /* Select page 18 */
> > > phy_write(phydev, MDIO_DEVAD_NONE,
> MIIM_88E1118_PHY_PAGE,
> > 18);
> > >
> > > -   /* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
> > > +   /* In reg 20, write MODE[2:0], SGMII or RGMII to Copper */
> > > m88e1518_phy_writebits(phydev, MIIM_88E151x_GENERAL_CTRL,
> > > -  0, 3, MIIM_88E151x_MODE_SGMII);
> > > +  0, 3, mode);
> > >
> > > /* PHY reset is necessary after changing MODE[2:0] */
> > > m88e1518_phy_writebits(phydev, MIIM_88E151x_GENERAL_CTRL,
> > > --
> > > 2.7.4

Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 3/5] net: phy: Marvell 88E151x: Add support for RGMII

[Phil Edworthy]

Hi Joe,

On 09 December 2016 18:59, Joe Hershberger wrote:
> On Fri, Dec 9, 2016 at 7:38 AM, Phil Edworthy <phil.edwor...@renesas.com>
> wrote:
> > This has been tested with a Marvell 88E1512 PHY.
> >
> > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > Reviewed-by: Stefan Roese <s...@denx.de>
> > ---
> > v2:
> >  Rebased on top of Joe's code to use macros
> > ---
> >  drivers/net/phy/marvell.c | 17 +
> >  1 file changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> > index 5adfe7d..646b00d 100644
> > --- a/drivers/net/phy/marvell.c
> > +++ b/drivers/net/phy/marvell.c
> > @@ -94,6 +94,7 @@
> >  #define MIIM_88E151x_INT_EN_OFFS   7
> >  /* Page 18 registers */
> >  #define MIIM_88E151x_GENERAL_CTRL  20
> > +#define MIIM_88E151x_MODE_RGMII0
> 
> I'm not sure why this is needed. The HW reset value for the mode is
> 0x7 (RGMII auto detect mode). Would it not be sufficient to simply not
> change it if the mode is RGMII?
I don’t have a manual for any Marvell devices, just some small bits
of information. All I can tell you is that without this patch, RGMII on
1512 doesn't work.


> >  #define MIIM_88E151x_MODE_SGMII1
> >  #define MIIM_88E151x_RESET_OFFS15
> >
> > @@ -315,14 +316,22 @@ static int m88e1518_config(struct phy_device
> *phydev)
> > phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2159);
> > phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE,
> 0x);
> >
> > -   /* SGMII-to-Copper mode initialization */
> > -   if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
> > +   /* SGMII/RGMII-to-Copper mode initialization */
> > +   if ((phydev->interface == PHY_INTERFACE_MODE_SGMII) ||
> > +   phy_interface_is_rgmii(phydev)) {
> > +   u16 mode;
> > +
> > +   if (phydev->interface == PHY_INTERFACE_MODE_SGMII)
> > +   mode = MIIM_88E151x_MODE_SGMII;
> > +   else
> > +   mode = MIIM_88E151x_MODE_RGMII;
> > +
> > /* Select page 18 */
> > phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE,
> 18);
> >
> > -   /* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
> > +   /* In reg 20, write MODE[2:0], SGMII or RGMII to Copper */
> > m88e1518_phy_writebits(phydev, MIIM_88E151x_GENERAL_CTRL,
> > -  0, 3, MIIM_88E151x_MODE_SGMII);
> > +  0, 3, mode);
> >
> > /* PHY reset is necessary after changing MODE[2:0] */
> > m88e1518_phy_writebits(phydev, MIIM_88E151x_GENERAL_CTRL,
> > --
> > 2.7.4

Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] sf: Do not force the DT memory map size to exactly match the device

[Phil Edworthy]

As long as the memory mapped size specifeid in the DT is the same or
bigger than the device size, it will work. So do not force the sizes
to be identical.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 drivers/mtd/spi/spi_flash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index 6e595c8..fee930b 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -902,7 +902,7 @@ int spi_flash_decode_fdt(const void *blob, struct spi_flash 
*flash)
return 0;
}
 
-   if (flash->size != size) {
+   if (flash->size > size) {
debug("%s: Memory map must cover entire device\n", __func__);
return -1;
}
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 5/5] net: phy: Marvell: Use phy_interface_is_rgmii helper function

[Phil Edworthy]

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
Reviewed-by: Stefan Roese <s...@denx.de>
---
v2:
 No changes
---
 drivers/net/phy/marvell.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index b6c005f..f640a81 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -193,10 +193,7 @@ static int m88es_config(struct phy_device *phydev)
 {
int reg;
 
-   if ((phydev->interface == PHY_INTERFACE_MODE_RGMII) ||
-   (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
-   (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) ||
-   (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)) {
+   if (phy_interface_is_rgmii(phydev)) {
reg = phy_read(phydev,
MDIO_DEVAD_NONE, MIIM_88E_PHY_EXT_CR);
if ((phydev->interface == PHY_INTERFACE_MODE_RGMII) ||
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 3/5] net: phy: Marvell 88E151x: Add support for RGMII

[Phil Edworthy]

This has been tested with a Marvell 88E1512 PHY.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
Reviewed-by: Stefan Roese <s...@denx.de>
---
v2:
 Rebased on top of Joe's code to use macros
---
 drivers/net/phy/marvell.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 5adfe7d..646b00d 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -94,6 +94,7 @@
 #define MIIM_88E151x_INT_EN_OFFS   7
 /* Page 18 registers */
 #define MIIM_88E151x_GENERAL_CTRL  20
+#define MIIM_88E151x_MODE_RGMII0
 #define MIIM_88E151x_MODE_SGMII1
 #define MIIM_88E151x_RESET_OFFS15
 
@@ -315,14 +316,22 @@ static int m88e1518_config(struct phy_device *phydev)
phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2159);
phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE, 0x);
 
-   /* SGMII-to-Copper mode initialization */
-   if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
+   /* SGMII/RGMII-to-Copper mode initialization */
+   if ((phydev->interface == PHY_INTERFACE_MODE_SGMII) ||
+   phy_interface_is_rgmii(phydev)) {
+   u16 mode;
+
+   if (phydev->interface == PHY_INTERFACE_MODE_SGMII)
+   mode = MIIM_88E151x_MODE_SGMII;
+   else
+   mode = MIIM_88E151x_MODE_RGMII;
+
/* Select page 18 */
phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE, 18);
 
-   /* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
+   /* In reg 20, write MODE[2:0], SGMII or RGMII to Copper */
m88e1518_phy_writebits(phydev, MIIM_88E151x_GENERAL_CTRL,
-  0, 3, MIIM_88E151x_MODE_SGMII);
+  0, 3, mode);
 
/* PHY reset is necessary after changing MODE[2:0] */
m88e1518_phy_writebits(phydev, MIIM_88E151x_GENERAL_CTRL,
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 4/5] net: phy: Marvell 88E151x: Increase delay after init

[Phil Edworthy]

On Marvell 88E1512, the delay is not enough when connected
to some external switches (e.g. Netgear GS108E).

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
Reviewed-by: Stefan Roese <s...@denx.de>
---
v2:
 No changes
---
 drivers/net/phy/marvell.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 646b00d..b6c005f 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -340,7 +340,7 @@ static int m88e1518_config(struct phy_device *phydev)
/* Reset page selection */
phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE, 0);
 
-   udelay(100);
+   udelay(500);
}
 
return m88es_config(phydev);
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 2/5] net: phy: Add support for Marvell M88E1512

[Phil Edworthy]

This device also works with the 88E1518 code, so we just adjust
the UID mask accordingly.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
v2:
 Don't add a new entry, just adjust the UID mask.
---
 drivers/net/phy/marvell.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 95d1492..5adfe7d 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -585,10 +585,14 @@ static struct phy_driver M88E1510_driver = {
.shutdown = _shutdown,
 };
 
+/* This supports:
+ *  88E1518, uid 0x1410dd1
+ *  88E1512, uid 0x1410dd4
+ */
 static struct phy_driver M88E1518_driver = {
.name = "Marvell 88E1518",
.uid = 0x1410dd1,
-   .mask = 0xfff,
+   .mask = 0xffb,
.features = PHY_GBIT_FEATURES,
.config = _config,
.startup = _startup,
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 1/5] net: phy: Fix mask so that we can identify Marvell 88E1518

[Phil Edworthy]

The mask for the 88E1510 meant that the 88E1518 code would never be
used.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
Reviewed-by: Stefan Roese <s...@denx.de>
---
Note: This has only been tested on a board that uses a Marvell 88E1512
PHY, see subsequent patches.

v2:
 No changes
---
 drivers/net/phy/marvell.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index ac13a67..95d1492 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -578,7 +578,7 @@ static struct phy_driver M88E1149S_driver = {
 static struct phy_driver M88E1510_driver = {
.name = "Marvell 88E1510",
.uid = 0x1410dd0,
-   .mask = 0xff0,
+   .mask = 0xfff,
.features = PHY_GBIT_FEATURES,
.config = _config,
.startup = _startup,
@@ -588,7 +588,7 @@ static struct phy_driver M88E1510_driver = {
 static struct phy_driver M88E1518_driver = {
.name = "Marvell 88E1518",
.uid = 0x1410dd1,
-   .mask = 0xff0,
+   .mask = 0xfff,
.features = PHY_GBIT_FEATURES,
.config = _config,
.startup = _startup,
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 0/5] net: phy: Add Marvell M88E1512

[Phil Edworthy]

These patches add support for the Marvell M88E1512 PHY.

Phil Edworthy (5):
  net: phy: Fix mask so that we can identify Marvell 88E1518
  net: phy: Add support for Marvell M88E1512
  net: phy: Marvell 88E151x: Add support for RGMII
  net: phy: Marvell 88E151x: Increase delay after init
  net: phy: Marvell: Use phy_interface_is_rgmii helper function

 drivers/net/phy/marvell.c | 32 +---
 1 file changed, 21 insertions(+), 11 deletions(-)

-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] net: phy: Improve the Marvell 151x constants

[Phil Edworthy]

Hi Joe,

On 02 December 2016 05:50, Joe Hershberger wrote:
> On 01.12.2016 18:08, Joe Hershberger wrote:
> > Use some constants for the phy configuration instead of so many magic
> > numbers.
> >
> > Signed-off-by: Joe Hershberger 
> > ---
> >
> >  drivers/net/phy/marvell.c | 47 -
> --
> >  1 file changed, 36 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> > index 4eeb0f6..e06fdac 100644
> > --- a/drivers/net/phy/marvell.c
> > +++ b/drivers/net/phy/marvell.c
> > @@ -82,6 +82,21 @@
> >  #define MIIM_88E1310_PHY_RGMII_CTRL21
> >  #define MIIM_88E1310_PHY_PAGE  22
> >
> > +/* 88E151x PHY defines */
> > +/* Page 3 registers */
> > +#define MIIM_88E151x_LED_FUNC_CTRL 16
> > +#define MIIM_88E151x_LED_FLD_SZ4
> > +#define MIIM_88E151x_LED0_OFFS (0 *
> MIIM_88E151x_LED_FLD_SIZE)
> > +#define MIIM_88E151x_LED1_OFFS (1 *
> MIIM_88E151x_LED_FLD_SIZE)

Build error: 
marvell.c:89:38: error: 'MIIM_88E151x_LED_FLD_SIZE' undeclared
I guess it should use 'MIIM_88E151x_LED_FLD_SZ' instead!


> > +#define MIIM_88E151x_LED0_ACT  3
> > +#define MIIM_88E151x_LED1_100_1000_LINK6
> > +#define MIIM_88E151x_LED_TIMER_CTRL18
> > +#define MIIM_88E151x_INT_EN_OFFS   7
> > +/* Page 18 registers */
> > +#define MIIM_88E151x_GENERAL_CTRL  20
> > +#define MIIM_88E151x_MODE_SGMII1
> > +#define MIIM_88E151x_RESET_OFFS15
> > +
> >  /* Marvell 88E1011S */
> >  static int m88e1011s_config(struct phy_device *phydev)
> >  {
> > @@ -289,7 +304,7 @@ static int m88e1518_config(struct phy_device *phydev)
> >  */
> >
> > /* EEE initialization */
> > -   phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x00ff);
> > +   phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE,
> 0x00ff);
> > phy_write(phydev, MDIO_DEVAD_NONE, 17, 0x214B);
> > phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2144);
> > phy_write(phydev, MDIO_DEVAD_NONE, 17, 0x0C28);
> > @@ -298,21 +313,23 @@ static int m88e1518_config(struct phy_device
> *phydev)
> > phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x214D);
> > phy_write(phydev, MDIO_DEVAD_NONE, 17, 0xCC0C);
> > phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2159);
> > -   phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x);
> > +   phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE,
> 0x);
> >
> > /* SGMII-to-Copper mode initialization */
> > if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
> > /* Select page 18 */
> > -   phy_write(phydev, MDIO_DEVAD_NONE, 22, 18);
> > +   phy_write(phydev, MDIO_DEVAD_NONE,
> MIIM_88E1118_PHY_PAGE, 18);
> >
> > /* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
> > -   m88e1518_phy_writebits(phydev, 20, 0, 3, 1);
> > +   m88e1518_phy_writebits(phydev,
> MIIM_88E151x_GENERAL_CTRL,
> > +  0, 3, MIIM_88E151x_MODE_SGMII);
> >
> > /* PHY reset is necessary after changing MODE[2:0] */
> > -   m88e1518_phy_writebits(phydev, 20, 15, 1, 1);
> > +   m88e1518_phy_writebits(phydev,
> MIIM_88E151x_GENERAL_CTRL,
> > +  MIIM_88E151x_RESET_OFFS, 1, 1);
> >
> > /* Reset page selection */
> > -   phy_write(phydev, MDIO_DEVAD_NONE, 22, 0);
> > +   phy_write(phydev, MDIO_DEVAD_NONE,
> MIIM_88E1118_PHY_PAGE, 0);
> >
> > udelay(100);
> > }
> > @@ -324,17 +341,25 @@ static int m88e1518_config(struct phy_device
> *phydev)
> >  static int m88e1510_config(struct phy_device *phydev)
> >  {
> > /* Select page 3 */
> > -   phy_write(phydev, MDIO_DEVAD_NONE, 22, 3);
> > +   phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE,
> > + MIIM_88E1118_PHY_LED_PAGE);
> >
> > /* Enable INTn output on LED[2] */
> > -   m88e1518_phy_writebits(phydev, 18, 7, 1, 1);
> > +   m88e1518_phy_writebits(phydev, MIIM_88E151x_LED_TIMER_CTRL,
> > +  MIIM_88E151x_INT_EN_OFFS, 1, 1);
> >
> > /* Configure LEDs */
> > -   m88e1518_phy_writebits(phydev, 16, 0, 4, 3); /* LED[0]:0011 (ACT) */
> > -   m88e1518_phy_writebits(phydev, 16, 4, 4, 6); /* LED[1]:0110 (LINK) */
> > +   /* LED[0]:0011 (ACT) */
> > +   m88e1518_phy_writebits(phydev, MIIM_88E151x_LED_FUNC_CTRL,
> > +  MIIM_88E151x_LED0_OFFS,
> MIIM_88E151x_LED_FLD_SZ,
> > +  MIIM_88E151x_LED0_ACT);
> > +   /* LED[1]:0110 (LINK 100/1000 Mbps) */
> > +   m88e1518_phy_writebits(phydev, MIIM_88E151x_LED_FUNC_CTRL,
> > +  MIIM_88E151x_LED1_OFFS,
> MIIM_88E151x_LED_FLD_SZ,
> > +  MIIM_88E151x_LED1_100_1000_LINK);
> >
> > /* Reset page selection */
> > -   phy_write(phydev, MDIO_DEVAD_NONE, 22, 0);
> > +   phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE, 0);
> >
> > return 

Re: [U-Boot] [PATCH 3/5] net: phy: Marvell 88E151x: Add support for RGMII

[Phil Edworthy]

Hi Stefan,

On 09 December 2016 12:19, Stefan Roese wrote:
> On 09.12.2016 11:41, Phil Edworthy wrote:
> > This has been tested with a Marvell 88E1512 PHY.
> >
> > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > ---
> >  drivers/net/phy/marvell.c | 14 ++
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> > index a7ea435..8214760 100644
> > --- a/drivers/net/phy/marvell.c
> > +++ b/drivers/net/phy/marvell.c
> > @@ -300,13 +300,19 @@ static int m88e1518_config(struct phy_device
> *phydev)
> > phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2159);
> > phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x);
> >
> > -   /* SGMII-to-Copper mode initialization */
> > -   if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
> > +   /* SGMII/RGMII-to-Copper mode initialization */
> > +   if ((phydev->interface == PHY_INTERFACE_MODE_SGMII) ||
> > +   phy_interface_is_rgmii(phydev)) {
> > /* Select page 18 */
> > phy_write(phydev, MDIO_DEVAD_NONE, 22, 18);
> >
> > -   /* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
> > -   m88e1518_phy_writebits(phydev, 20, 0, 3, 1);
> > +   if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
> > +   /* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
> > +   m88e1518_phy_writebits(phydev, 20, 0, 3, 1);
> > +   } else {
> > +   /* In reg 20, write MODE[2:0] = 0x0 (RGMII to Copper) */
> > +   m88e1518_phy_writebits(phydev, 20, 0, 3, 0);
> > +   }
> >
> > /* PHY reset is necessary after changing MODE[2:0] */
> > m88e1518_phy_writebits(phydev, 20, 15, 1, 1);
> >
> 
> Joe has recently sent a patch, adding some nice macros for the
> some "standard" Marvell PHY register, like PAGE [1]. So this patch
> will most likely generate some merge problems. Please rebase it
> on top of Joe's patch.
Will do!

Thanks
Phil
> Other than this:
> 
> Reviewed-by: Stefan Roese <s...@denx.de>
> 
> Thanks,
> Stefan
> 
> [1] https://patchwork.ozlabs.org/patch/701613/
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 2/5] net: phy: Add support for Marvell M88E1512

[Phil Edworthy]

Hi Stefan,

On 09 December 2016 12:16, Stefan Roese wrote:
> On 09.12.2016 11:40, Phil Edworthy wrote:
> > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > ---
> >  drivers/net/phy/marvell.c | 11 +++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> > index 06029c0..a7ea435 100644
> > --- a/drivers/net/phy/marvell.c
> > +++ b/drivers/net/phy/marvell.c
> > @@ -560,6 +560,16 @@ static struct phy_driver M88E1510_driver = {
> > .shutdown = _shutdown,
> >  };
> >
> > +static struct phy_driver M88E1512_driver = {
> > +   .name = "Marvell 88E1512",
> > +   .uid = 0x01410dd4,
> > +   .mask = 0xfff,
> > +   .features = PHY_GBIT_FEATURES,
> > +   .config = _config,
> > +   .startup = _startup,
> > +   .shutdown = _shutdown,
> > +};
> > +
> >  static struct phy_driver M88E1518_driver = {
> > .name = "Marvell 88E1518",
> > .uid = 0x1410dd1,
> > @@ -591,6 +601,7 @@ int phy_marvell_init(void)
> > phy_register(_driver);
> > phy_register(_driver);
> > phy_register(_driver);
> > +   phy_register(_driver);
> > phy_register(_driver);
> 
> Do you need some special handling for the M88E1512 or is it identical
> to the 1518 handling? If its identical, why do we need a new entry
> for this PHY?
Good point, they are the same except for the uid so no need for a new
entry. I'll change the mask of the 1518 to cover it.

Btw, what's the best way to indicate that the code supports the 1512
as well? It's not so easy to see if the code supports a particular device
ahead of hardware arriving, other than trying to match the uid's
manually.

Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] net: phy: ti: Fix dp83867 RGMII_TXID interface path

[Phil Edworthy]

There is code that is specifically for RGMII_TXID interface, but this
will never get used because the code checks that the RGMII interface
is RGMII_ID to RGMII_RXID; RGMII_TXID is after this.

To fix this and avoid similar problems in the future, use the
phy_interface_is_rgmii helper function.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
This has _not_ been tested in any way, shape or form! It was picked
up when converting PHY code to use the phy_interface_is_rgmii helper
function.
---
 drivers/net/phy/ti.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/phy/ti.c b/drivers/net/phy/ti.c
index c55dd97..5e2b2dd 100644
--- a/drivers/net/phy/ti.c
+++ b/drivers/net/phy/ti.c
@@ -246,8 +246,7 @@ static int dp83867_config(struct phy_device *phydev)
phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83867_BISCR, 0x0);
}
 
-   if ((phydev->interface >= PHY_INTERFACE_MODE_RGMII_ID) &&
-   (phydev->interface <= PHY_INTERFACE_MODE_RGMII_RXID)) {
+   if (phy_interface_is_rgmii(phydev)) {
val = phy_read_mmd_indirect(phydev, DP83867_RGMIICTL,
DP83867_DEVADDR, phydev->addr);
 
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 4/5] net: phy: Marvell 88E151x: Increase delay after init

[Phil Edworthy]

On Marvell 88E1512, the delay is not enough when connected
to some external switches (e.g. Netgear GS108E).

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 drivers/net/phy/marvell.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 8214760..48ebb50 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -320,7 +320,7 @@ static int m88e1518_config(struct phy_device *phydev)
/* Reset page selection */
phy_write(phydev, MDIO_DEVAD_NONE, 22, 0);
 
-   udelay(100);
+   udelay(500);
}
 
return m88es_config(phydev);
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 3/5] net: phy: Marvell 88E151x: Add support for RGMII

[Phil Edworthy]

This has been tested with a Marvell 88E1512 PHY.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 drivers/net/phy/marvell.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index a7ea435..8214760 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -300,13 +300,19 @@ static int m88e1518_config(struct phy_device *phydev)
phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2159);
phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x);
 
-   /* SGMII-to-Copper mode initialization */
-   if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
+   /* SGMII/RGMII-to-Copper mode initialization */
+   if ((phydev->interface == PHY_INTERFACE_MODE_SGMII) ||
+   phy_interface_is_rgmii(phydev)) {
/* Select page 18 */
phy_write(phydev, MDIO_DEVAD_NONE, 22, 18);
 
-   /* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
-   m88e1518_phy_writebits(phydev, 20, 0, 3, 1);
+   if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
+   /* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
+   m88e1518_phy_writebits(phydev, 20, 0, 3, 1);
+   } else {
+   /* In reg 20, write MODE[2:0] = 0x0 (RGMII to Copper) */
+   m88e1518_phy_writebits(phydev, 20, 0, 3, 0);
+   }
 
/* PHY reset is necessary after changing MODE[2:0] */
m88e1518_phy_writebits(phydev, 20, 15, 1, 1);
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 5/5] net: phy: Marvell: Use phy_interface_is_rgmii helper function

[Phil Edworthy]

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 drivers/net/phy/marvell.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 48ebb50..dc1d25f 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -177,10 +177,7 @@ static int m88es_config(struct phy_device *phydev)
 {
int reg;
 
-   if ((phydev->interface == PHY_INTERFACE_MODE_RGMII) ||
-   (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
-   (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) ||
-   (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)) {
+   if (phy_interface_is_rgmii(phydev)) {
reg = phy_read(phydev,
MDIO_DEVAD_NONE, MIIM_88E_PHY_EXT_CR);
if ((phydev->interface == PHY_INTERFACE_MODE_RGMII) ||
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 2/5] net: phy: Add support for Marvell M88E1512

[Phil Edworthy]

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 drivers/net/phy/marvell.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 06029c0..a7ea435 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -560,6 +560,16 @@ static struct phy_driver M88E1510_driver = {
.shutdown = _shutdown,
 };
 
+static struct phy_driver M88E1512_driver = {
+   .name = "Marvell 88E1512",
+   .uid = 0x01410dd4,
+   .mask = 0xfff,
+   .features = PHY_GBIT_FEATURES,
+   .config = _config,
+   .startup = _startup,
+   .shutdown = _shutdown,
+};
+
 static struct phy_driver M88E1518_driver = {
.name = "Marvell 88E1518",
.uid = 0x1410dd1,
@@ -591,6 +601,7 @@ int phy_marvell_init(void)
phy_register(_driver);
phy_register(_driver);
phy_register(_driver);
+   phy_register(_driver);
phy_register(_driver);
 
return 0;
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 1/5] net: phy: Fix mask so that we can identify Marvell 88E1518

[Phil Edworthy]

The mask for the 88E1510 meant that the 88E1518 code would never be
used.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
Note: This has only been tested on a board that uses a Marvell 88E1512
PHY, see subsequent patches.
---
 drivers/net/phy/marvell.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 4eeb0f6..06029c0 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -553,7 +553,7 @@ static struct phy_driver M88E1149S_driver = {
 static struct phy_driver M88E1510_driver = {
.name = "Marvell 88E1510",
.uid = 0x1410dd0,
-   .mask = 0xff0,
+   .mask = 0xfff,
.features = PHY_GBIT_FEATURES,
.config = _config,
.startup = _startup,
@@ -563,7 +563,7 @@ static struct phy_driver M88E1510_driver = {
 static struct phy_driver M88E1518_driver = {
.name = "Marvell 88E1518",
.uid = 0x1410dd1,
-   .mask = 0xff0,
+   .mask = 0xfff,
.features = PHY_GBIT_FEATURES,
.config = _config,
.startup = _startup,
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 0/5] net: phy: Add Marvell M88E1512

[Phil Edworthy]

These patches add support for the Marvell M88E1512 PHY.

Phil Edworthy (5):
  net: phy: Fix mask so that we can identify Marvell 88E1518
  net: phy: Add support for Marvell M88E1512
  net: phy: Marvell 88E151x: Add support for RGMII
  net: phy: Marvell 88E151x: Increase delay after init
  net: phy: Marvell: Use phy_interface_is_rgmii helper function

 drivers/net/phy/marvell.c | 36 +---
 1 file changed, 25 insertions(+), 11 deletions(-)

-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] Problem with qspi when using micron devices

[Phil Edworthy]

Hi Jagan,

When using Micron devices, SPI flash with quad mode does not work since commit
c56ae7519f141523ba1248b22b5b5169b21772fe "sf: Fix quad bit set for micron 
devices".

This has been pointed out before, details about why the patch does work are 
here:
http://lists.denx.de/pipermail/u-boot/2016-March/248566.html

I know you are working on spi-nor, but it really would be good to revert the 
patch.

Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 11/11] spi: cadence_qspi: Support specifying the sample edge used

[Phil Edworthy]

Hi Jagan,

On 06 December 2016 17:24 Jagan Teki wrote:
> On Tue, Dec 6, 2016 at 6:00 PM, Phil Edworthy <phil.edwor...@renesas.com>
> wrote:
> > Hi Jagan, Marek,
> >
> > On 06 December 2016 12:39 Marek Vasut wrote:
> >> On 12/06/2016 11:25 AM, Phil Edworthy wrote:
> >> > On 05 December 2016 13:31, Marek Vasut wrote:
> >> >> On 12/05/2016 11:46 AM, Phil Edworthy wrote:
> >> >>> On 05 December 2016 10:42, Jagan Teki wrote:
> >> >>>> On Mon, Dec 5, 2016 at 11:31 AM, Phil Edworthy
> >> >>>> <phil.edwor...@renesas.com> wrote:
> >> >>>>> On 05 December 2016 10:26, Jagan Teki wrote:
> >> >>>>>> On Mon, Dec 5, 2016 at 11:09 AM, Phil Edworthy
> >> >>>>>> <phil.edwor...@renesas.com> wrote:
> >> >>>>>>> On 02 December 2016 14:23, Jagan Teki wrote:
> >> >>>>>>>> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
> >> >>>>>>>> <phil.edwor...@renesas.com> wrote:
> >> >>>>>>>>> Introduce a new DT property to specify whether the QSPI
> Controller
> >> >>>>>>>>> samples the data on a rising edge. The default is falling edge.
> >> >>>>>>>>> Some versions of the QSPI Controller do not implement this bit, 
> >> >>>>>>>>> in
> >> >>>>>>>>> which case the property should be omitted.
> > 
> >
> >> >>>>>>>> Code look reasonable, but how Linux handling this with the same
> dt
> >> >>>>>>>> property, any idea? I couldn't find it either.
> >> >>>>>>> The Linux driver does not yet have this property. Is there a 
> >> >>>>>>> policy to
> add
> >> >> new
> >> >>>>>>> props to Linux first?
> >> >>>>>>
> >> >>>>>> If the same/equal code used in Linux better to have the same
> property
> >> >>>>>> instead of another name used in U-boot?
> >> >>>>> Of course, but I cannot see this in Linux:
> >> >>>>> https://git.kernel.org/cgit/linux/kernel/git/next/linux-
> >> >>>> next.git/tree/Documentation/devicetree/bindings/spi/spi-cadence.txt
> >> >>>>
> >> >>>> Yeah, I saw this. Do you have any idea how Linux handling this sample
> >> edge?
> >> >>> The same way U-Boot currently handles it, i.e. it does nothing with 
> >> >>> this.
> >> >> Intel/Altera
> >> >>> (Chin Liang) said that they do not have this bit in their version of 
> >> >>> the
> Cadence
> >> >> QSPI
> >> >>> Controller.
> >> >>>
> >> >>> We are using a later version that has had this bit added.
> >> >>
> >> >> You were looking at the wrong bindings:
> >> >> https://git.kernel.org/cgit/linux/kernel/git/next/linux-
> >> >> next.git/tree/Documentation/devicetree/bindings/mtd/cadence-
> quadspi.txt
> >> > Thanks for pointing that out!
> >> >
> >> >> but yes, Linux does not do support the data edge toggling. I think there
> >> >> was another QSPI patch in Linux which tried adding such property, so
> >> >> check linux-mtd for it. Generic one would be great.
> >> > I had a search around, but couldn't find anything.
> >>
> >> Look for negative_edge here:
> >> http://www.spinics.net/lists/devicetree/msg153582.html
> >>
> >> >> And no, there is no policy for pushing new props to linux first. New DT
> >> >> props should ideally get approved via devicetree@vger though, but that's
> >> >> about it. Also, while I tried backporting the Linux CQSPI driver to
> >> >> U-Boot, but unfortunately, it turned out to be extremely difficult due
> >> >> significant differences between the Linux and U-Boot SPI NOR  framework.
> >> > OK, thanks for the information.
> >
> > Since it will take a bit more time to get a generic prop for the sample 
> > edge to
> > be ack'd by devicetree@vger, would it make sense to drop it from this 
> > series,
> > so we can get the rest in?
> 
> I can drop 10 and 11 from the series, is that OK?
Yes please!

Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 11/11] spi: cadence_qspi: Support specifying the sample edge used

[Phil Edworthy]

Hi Jagan, Marek,

On 06 December 2016 12:39 Marek Vasut wrote:
> On 12/06/2016 11:25 AM, Phil Edworthy wrote:
> > On 05 December 2016 13:31, Marek Vasut wrote:
> >> On 12/05/2016 11:46 AM, Phil Edworthy wrote:
> >>> On 05 December 2016 10:42, Jagan Teki wrote:
> >>>> On Mon, Dec 5, 2016 at 11:31 AM, Phil Edworthy
> >>>> <phil.edwor...@renesas.com> wrote:
> >>>>> On 05 December 2016 10:26, Jagan Teki wrote:
> >>>>>> On Mon, Dec 5, 2016 at 11:09 AM, Phil Edworthy
> >>>>>> <phil.edwor...@renesas.com> wrote:
> >>>>>>> On 02 December 2016 14:23, Jagan Teki wrote:
> >>>>>>>> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
> >>>>>>>> <phil.edwor...@renesas.com> wrote:
> >>>>>>>>> Introduce a new DT property to specify whether the QSPI Controller
> >>>>>>>>> samples the data on a rising edge. The default is falling edge.
> >>>>>>>>> Some versions of the QSPI Controller do not implement this bit, in
> >>>>>>>>> which case the property should be omitted.


> >>>>>>>> Code look reasonable, but how Linux handling this with the same dt
> >>>>>>>> property, any idea? I couldn't find it either.
> >>>>>>> The Linux driver does not yet have this property. Is there a policy 
> >>>>>>> to add
> >> new
> >>>>>>> props to Linux first?
> >>>>>>
> >>>>>> If the same/equal code used in Linux better to have the same property
> >>>>>> instead of another name used in U-boot?
> >>>>> Of course, but I cannot see this in Linux:
> >>>>> https://git.kernel.org/cgit/linux/kernel/git/next/linux-
> >>>> next.git/tree/Documentation/devicetree/bindings/spi/spi-cadence.txt
> >>>>
> >>>> Yeah, I saw this. Do you have any idea how Linux handling this sample
> edge?
> >>> The same way U-Boot currently handles it, i.e. it does nothing with this.
> >> Intel/Altera
> >>> (Chin Liang) said that they do not have this bit in their version of the 
> >>> Cadence
> >> QSPI
> >>> Controller.
> >>>
> >>> We are using a later version that has had this bit added.
> >>
> >> You were looking at the wrong bindings:
> >> https://git.kernel.org/cgit/linux/kernel/git/next/linux-
> >> next.git/tree/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> > Thanks for pointing that out!
> >
> >> but yes, Linux does not do support the data edge toggling. I think there
> >> was another QSPI patch in Linux which tried adding such property, so
> >> check linux-mtd for it. Generic one would be great.
> > I had a search around, but couldn't find anything.
> 
> Look for negative_edge here:
> http://www.spinics.net/lists/devicetree/msg153582.html
> 
> >> And no, there is no policy for pushing new props to linux first. New DT
> >> props should ideally get approved via devicetree@vger though, but that's
> >> about it. Also, while I tried backporting the Linux CQSPI driver to
> >> U-Boot, but unfortunately, it turned out to be extremely difficult due
> >> significant differences between the Linux and U-Boot SPI NOR  framework.
> > OK, thanks for the information.

Since it will take a bit more time to get a generic prop for the sample edge to
be ack'd by devicetree@vger, would it make sense to drop it from this series,
so we can get the rest in?

Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 11/11] spi: cadence_qspi: Support specifying the sample edge used

[Phil Edworthy]

Hi Marek,

On 05 December 2016 13:31, Marek Vasut wrote:
> On 12/05/2016 11:46 AM, Phil Edworthy wrote:
> > On 05 December 2016 10:42, Jagan Teki wrote:
> >> On Mon, Dec 5, 2016 at 11:31 AM, Phil Edworthy
> >> <phil.edwor...@renesas.com> wrote:
> >>> On 05 December 2016 10:26, Jagan Teki wrote:
> >>>> On Mon, Dec 5, 2016 at 11:09 AM, Phil Edworthy
> >>>> <phil.edwor...@renesas.com> wrote:
> >>>>> On 02 December 2016 14:23, Jagan Teki wrote:
> >>>>>> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
> >>>>>> <phil.edwor...@renesas.com> wrote:
> >>>>>>> Introduce a new DT property to specify whether the QSPI Controller
> >>>>>>> samples the data on a rising edge. The default is falling edge.
> >>>>>>> Some versions of the QSPI Controller do not implement this bit, in
> >>>>>>> which case the property should be omitted.
> >>>>>>>
> >>>>>>> Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> >>>>>>> ---
> >>>>>>>  v3:
> >>>>>>>   - Patch split so this one only has code related to the subject.
> >>>>>>>   - Commit message updated.
> >>>>>>>  v2:
> >>>>>>>   - Change name of DT prop and provide details of what it does.
> >>>>>>> Whilst at it, move the code to read the "sram-size" property
> >>>>>>> into the other code that reads properties from the node, rather
> >>>>>>> than the SF subnode.
> >>>>>>>
> >>>>>>> Also change the code to use a bool for the bypass arg.
> >>>>>>> ---
> >>>>>>>  doc/device-tree-bindings/spi/spi-cadence.txt |  2 ++
> >>>>>>>  drivers/spi/cadence_qspi.c   | 10 +++---
> >>>>>>>  drivers/spi/cadence_qspi.h   |  3 ++-
> >>>>>>>  drivers/spi/cadence_qspi_apb.c   |  8 +++-
> >>>>>>>  4 files changed, 18 insertions(+), 5 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/doc/device-tree-bindings/spi/spi-cadence.txt 
> >>>>>>> b/doc/device-
> >> tree-
> >>>>>> bindings/spi/spi-cadence.txt
> >>>>>>> index c1e2233..94c800b 100644
> >>>>>>> --- a/doc/device-tree-bindings/spi/spi-cadence.txt
> >>>>>>> +++ b/doc/device-tree-bindings/spi/spi-cadence.txt
> >>>>>>> @@ -26,3 +26,5 @@ connected flash properties
> >>>>>>>   select (n_ss_out).
> >>>>>>>  - tslch-ns : Delay in master reference clocks between 
> >>>>>>> setting
> >>>>>>>   n_ss_out low and first bit transfer
> >>>>>>> +- sample-edge-rising   : Data outputs from flash memory will be
> sampled
> >> on
> >>>>>> the
> >>>>>>> + rising edge. Default is falling edge.
> >>>>>>
> >>>>>> Code look reasonable, but how Linux handling this with the same dt
> >>>>>> property, any idea? I couldn't find it either.
> >>>>> The Linux driver does not yet have this property. Is there a policy to 
> >>>>> add
> new
> >>>>> props to Linux first?
> >>>>
> >>>> If the same/equal code used in Linux better to have the same property
> >>>> instead of another name used in U-boot?
> >>> Of course, but I cannot see this in Linux:
> >>> https://git.kernel.org/cgit/linux/kernel/git/next/linux-
> >> next.git/tree/Documentation/devicetree/bindings/spi/spi-cadence.txt
> >>
> >> Yeah, I saw this. Do you have any idea how Linux handling this sample edge?
> > The same way U-Boot currently handles it, i.e. it does nothing with this.
> Intel/Altera
> > (Chin Liang) said that they do not have this bit in their version of the 
> > Cadence
> QSPI
> > Controller.
> >
> > We are using a later version that has had this bit added.
> 
> You were looking at the wrong bindings:
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-
> next.git/tree/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
Thanks for pointing that out!

> but yes, Linux does not do support the data edge toggling. I think there
> was another QSPI patch in Linux which tried adding such property, so
> check linux-mtd for it. Generic one would be great.
I had a search around, but couldn't find anything.

> And no, there is no policy for pushing new props to linux first. New DT
> props should ideally get approved via devicetree@vger though, but that's
> about it. Also, while I tried backporting the Linux CQSPI driver to
> U-Boot, but unfortunately, it turned out to be extremely difficult due
> significant differences between the Linux and U-Boot SPI NOR  framework.
OK, thanks for the information.

Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 11/11] spi: cadence_qspi: Support specifying the sample edge used

[Phil Edworthy]

Hi Jagan,

On 05 December 2016 10:42, Jagan Teki wrote:
> On Mon, Dec 5, 2016 at 11:31 AM, Phil Edworthy
> <phil.edwor...@renesas.com> wrote:
> > HI Jagan,
> >
> > On 05 December 2016 10:26, Jagan Teki wrote:
> >> On Mon, Dec 5, 2016 at 11:09 AM, Phil Edworthy
> >> <phil.edwor...@renesas.com> wrote:
> >> > Hi Jagan,
> >> >
> >> > On 02 December 2016 14:23, Jagan Teki wrote:
> >> >> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
> >> >> <phil.edwor...@renesas.com> wrote:
> >> >> > Introduce a new DT property to specify whether the QSPI Controller
> >> >> > samples the data on a rising edge. The default is falling edge.
> >> >> > Some versions of the QSPI Controller do not implement this bit, in
> >> >> > which case the property should be omitted.
> >> >> >
> >> >> > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> >> >> > ---
> >> >> >  v3:
> >> >> >   - Patch split so this one only has code related to the subject.
> >> >> >   - Commit message updated.
> >> >> >  v2:
> >> >> >   - Change name of DT prop and provide details of what it does.
> >> >> > Whilst at it, move the code to read the "sram-size" property
> >> >> > into the other code that reads properties from the node, rather
> >> >> > than the SF subnode.
> >> >> >
> >> >> > Also change the code to use a bool for the bypass arg.
> >> >> > ---
> >> >> >  doc/device-tree-bindings/spi/spi-cadence.txt |  2 ++
> >> >> >  drivers/spi/cadence_qspi.c   | 10 +++---
> >> >> >  drivers/spi/cadence_qspi.h   |  3 ++-
> >> >> >  drivers/spi/cadence_qspi_apb.c   |  8 +++-
> >> >> >  4 files changed, 18 insertions(+), 5 deletions(-)
> >> >> >
> >> >> > diff --git a/doc/device-tree-bindings/spi/spi-cadence.txt 
> >> >> > b/doc/device-
> tree-
> >> >> bindings/spi/spi-cadence.txt
> >> >> > index c1e2233..94c800b 100644
> >> >> > --- a/doc/device-tree-bindings/spi/spi-cadence.txt
> >> >> > +++ b/doc/device-tree-bindings/spi/spi-cadence.txt
> >> >> > @@ -26,3 +26,5 @@ connected flash properties
> >> >> >   select (n_ss_out).
> >> >> >  - tslch-ns : Delay in master reference clocks between 
> >> >> > setting
> >> >> >   n_ss_out low and first bit transfer
> >> >> > +- sample-edge-rising   : Data outputs from flash memory will be 
> >> >> > sampled
> on
> >> >> the
> >> >> > + rising edge. Default is falling edge.
> >> >>
> >> >> Code look reasonable, but how Linux handling this with the same dt
> >> >> property, any idea? I couldn't find it either.
> >> > The Linux driver does not yet have this property. Is there a policy to 
> >> > add new
> >> > props to Linux first?
> >>
> >> If the same/equal code used in Linux better to have the same property
> >> instead of another name used in U-boot?
> > Of course, but I cannot see this in Linux:
> > https://git.kernel.org/cgit/linux/kernel/git/next/linux-
> next.git/tree/Documentation/devicetree/bindings/spi/spi-cadence.txt
> 
> Yeah, I saw this. Do you have any idea how Linux handling this sample edge?
The same way U-Boot currently handles it, i.e. it does nothing with this. 
Intel/Altera
(Chin Liang) said that they do not have this bit in their version of the 
Cadence QSPI
Controller.

We are using a later version that has had this bit added.

BR
Phil

> thanks!
> --
> Jagan Teki
> Free Software Engineer | www.openedev.com
> U-Boot, Linux | Upstream Maintainer
> Hyderabad, India.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 10/11] spi: cadence_qspi: Change readdata_capture() arg to bool

[Phil Edworthy]

Hi Jagan,

On 05 December 2016 10:29, Jagan Teki wrote:
> On Mon, Dec 5, 2016 at 11:07 AM, Phil Edworthy
> <phil.edwor...@renesas.com> wrote:
> > Hi Jagan,
> >
> > On 02 December 2016 14:20, Jagan Teki wrote:
> >> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
> >> <phil.edwor...@renesas.com> wrote:
> >> > This is in preparation for adding another arg.
> >>
> >> ?? proper reason for changing arg to bool.
> > Purely because the patch 11 adds another arg that is a bool (which is the 
> > natural
> > type as it is read from a dtb). Then having this bypass arg as something 
> > other
> > than a bool look a bit odd.
> 
> Can't we make this as 11 and saying the reason for bool which is
> used/compatible with previous dt patch (I mean 11th patch in the
> current case)?
Do you mean swap patches 10 and 11? Then this commit msg is basically
to say it is changed to bool to match the other arg?
If so, then sure, no problem.

Thanks
Phil

> thanks!
> --
> Jagan Teki
> Free Software Engineer | www.openedev.com
> U-Boot, Linux | Upstream Maintainer
> Hyderabad, India.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 11/11] spi: cadence_qspi: Support specifying the sample edge used

[Phil Edworthy]

HI Jagan,

On 05 December 2016 10:26, Jagan Teki wrote:
> On Mon, Dec 5, 2016 at 11:09 AM, Phil Edworthy
> <phil.edwor...@renesas.com> wrote:
> > Hi Jagan,
> >
> > On 02 December 2016 14:23, Jagan Teki wrote:
> >> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
> >> <phil.edwor...@renesas.com> wrote:
> >> > Introduce a new DT property to specify whether the QSPI Controller
> >> > samples the data on a rising edge. The default is falling edge.
> >> > Some versions of the QSPI Controller do not implement this bit, in
> >> > which case the property should be omitted.
> >> >
> >> > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> >> > ---
> >> >  v3:
> >> >   - Patch split so this one only has code related to the subject.
> >> >   - Commit message updated.
> >> >  v2:
> >> >   - Change name of DT prop and provide details of what it does.
> >> > Whilst at it, move the code to read the "sram-size" property
> >> > into the other code that reads properties from the node, rather
> >> > than the SF subnode.
> >> >
> >> > Also change the code to use a bool for the bypass arg.
> >> > ---
> >> >  doc/device-tree-bindings/spi/spi-cadence.txt |  2 ++
> >> >  drivers/spi/cadence_qspi.c   | 10 +++---
> >> >  drivers/spi/cadence_qspi.h   |  3 ++-
> >> >  drivers/spi/cadence_qspi_apb.c   |  8 +++-
> >> >  4 files changed, 18 insertions(+), 5 deletions(-)
> >> >
> >> > diff --git a/doc/device-tree-bindings/spi/spi-cadence.txt 
> >> > b/doc/device-tree-
> >> bindings/spi/spi-cadence.txt
> >> > index c1e2233..94c800b 100644
> >> > --- a/doc/device-tree-bindings/spi/spi-cadence.txt
> >> > +++ b/doc/device-tree-bindings/spi/spi-cadence.txt
> >> > @@ -26,3 +26,5 @@ connected flash properties
> >> >   select (n_ss_out).
> >> >  - tslch-ns : Delay in master reference clocks between 
> >> > setting
> >> >   n_ss_out low and first bit transfer
> >> > +- sample-edge-rising   : Data outputs from flash memory will be sampled 
> >> > on
> >> the
> >> > + rising edge. Default is falling edge.
> >>
> >> Code look reasonable, but how Linux handling this with the same dt
> >> property, any idea? I couldn't find it either.
> > The Linux driver does not yet have this property. Is there a policy to add 
> > new
> > props to Linux first?
> 
> If the same/equal code used in Linux better to have the same property
> instead of another name used in U-boot?
Of course, but I cannot see this in Linux:
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/spi/spi-cadence.txt

BR
Phil
 
> thanks!
> --
> Jagan Teki
> Free Software Engineer | www.openedev.com
> U-Boot, Linux | Upstream Maintainer
> Hyderabad, India.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 11/11] spi: cadence_qspi: Support specifying the sample edge used

[Phil Edworthy]

Hi Jagan,

On 02 December 2016 14:23, Jagan Teki wrote:
> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
> <phil.edwor...@renesas.com> wrote:
> > Introduce a new DT property to specify whether the QSPI Controller
> > samples the data on a rising edge. The default is falling edge.
> > Some versions of the QSPI Controller do not implement this bit, in
> > which case the property should be omitted.
> >
> > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > ---
> >  v3:
> >   - Patch split so this one only has code related to the subject.
> >   - Commit message updated.
> >  v2:
> >   - Change name of DT prop and provide details of what it does.
> > Whilst at it, move the code to read the "sram-size" property
> > into the other code that reads properties from the node, rather
> > than the SF subnode.
> >
> > Also change the code to use a bool for the bypass arg.
> > ---
> >  doc/device-tree-bindings/spi/spi-cadence.txt |  2 ++
> >  drivers/spi/cadence_qspi.c   | 10 +++---
> >  drivers/spi/cadence_qspi.h   |  3 ++-
> >  drivers/spi/cadence_qspi_apb.c   |  8 +++-
> >  4 files changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/doc/device-tree-bindings/spi/spi-cadence.txt b/doc/device-tree-
> bindings/spi/spi-cadence.txt
> > index c1e2233..94c800b 100644
> > --- a/doc/device-tree-bindings/spi/spi-cadence.txt
> > +++ b/doc/device-tree-bindings/spi/spi-cadence.txt
> > @@ -26,3 +26,5 @@ connected flash properties
> >   select (n_ss_out).
> >  - tslch-ns : Delay in master reference clocks between setting
> >   n_ss_out low and first bit transfer
> > +- sample-edge-rising   : Data outputs from flash memory will be sampled on
> the
> > + rising edge. Default is falling edge.
> 
> Code look reasonable, but how Linux handling this with the same dt
> property, any idea? I couldn't find it either.
The Linux driver does not yet have this property. Is there a policy to add new
props to Linux first?

Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 10/11] spi: cadence_qspi: Change readdata_capture() arg to bool

[Phil Edworthy]

Hi Jagan,

On 02 December 2016 14:20, Jagan Teki wrote:
> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
> <phil.edwor...@renesas.com> wrote:
> > This is in preparation for adding another arg.
> 
> ?? proper reason for changing arg to bool.
Purely because the patch 11 adds another arg that is a bool (which is the 
natural
type as it is read from a dtb). Then having this bypass arg as something other
than a bool look a bit odd.

Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 04/11] spi: cadence_qspi: Use #define for bits instead of bit shifts

[Phil Edworthy]

Hi Jagan,

On 30 November 2016 04:59, Jagan Teki wrote:
> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
> <phil.edwor...@renesas.com> wrote:
> > Most of the code already uses #defines for the bit value, rather
> > than the shift required to get the value. This changes the remaining
> > code over.
> >
> > Whislt at it, fix the names of the "Rd Data Capture" register defs.
> >
> > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > Acked-by: Marek Vasut <marek.va...@gmail.com>
> > ---
> >  v3:
> >  - Remove brackets that aren't needed.
> >  v2:
> >  - No change.
> > ---
> >  drivers/spi/cadence_qspi_apb.c | 37 +++--
> >  1 file changed, 19 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> > index 0a2963d..b41f36b 100644
> > --- a/drivers/spi/cadence_qspi_apb.c
> > +++ b/drivers/spi/cadence_qspi_apb.c
> > @@ -57,9 +57,9 @@
> >   * Controller's configuration and status register (offset from QSPI_BASE)
> >
> **
> **/
> >  #defineCQSPI_REG_CONFIG0x00
> > -#defineCQSPI_REG_CONFIG_CLK_POL_LSB1
> > -#defineCQSPI_REG_CONFIG_CLK_PHA_LSB2
> >  #defineCQSPI_REG_CONFIG_ENABLE_MASKBIT(0)
> > +#defineCQSPI_REG_CONFIG_CLK_POLBIT(1)
> > +#defineCQSPI_REG_CONFIG_CLK_PHABIT(2)
> >  #defineCQSPI_REG_CONFIG_DIRECT_MASKBIT(7)
> >  #defineCQSPI_REG_CONFIG_DECODE_MASKBIT(9)
> >  #defineCQSPI_REG_CONFIG_XIP_IMM_MASK   BIT(18)
> > @@ -94,10 +94,10 @@
> >  #defineCQSPI_REG_DELAY_TSD2D_MASK  0xFF
> >  #defineCQSPI_REG_DELAY_TSHSL_MASK  0xFF
> >
> > -#defineCQSPI_READLCAPTURE  0x10
> > -#defineCQSPI_READLCAPTURE_BYPASS_LSB   0
> > -#defineCQSPI_READLCAPTURE_DELAY_LSB1
> > -#defineCQSPI_READLCAPTURE_DELAY_MASK   0xF
> > +#defineCQSPI_REG_RD_DATA_CAPTURE   0x10
> > +#defineCQSPI_REG_RD_DATA_CAPTURE_BYPASSBIT(0)
> > +#defineCQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB 1
> > +#defineCQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK0xF
> >
> >  #defineCQSPI_REG_SIZE  0x14
> >  #defineCQSPI_REG_SIZE_ADDRESS_LSB  0
> > @@ -244,20 +244,20 @@ void cadence_qspi_apb_readdata_capture(void
> *reg_base,
> > unsigned int reg;
> > cadence_qspi_apb_controller_disable(reg_base);
> >
> > -   reg = readl(reg_base + CQSPI_READLCAPTURE);
> > +   reg = readl(reg_base + CQSPI_REG_RD_DATA_CAPTURE);
> >
> > if (bypass)
> > -   reg |= (1 << CQSPI_READLCAPTURE_BYPASS_LSB);
> > +   reg |= CQSPI_REG_RD_DATA_CAPTURE_BYPASS;
> > else
> > -   reg &= ~(1 << CQSPI_READLCAPTURE_BYPASS_LSB);
> > +   reg &= ~CQSPI_REG_RD_DATA_CAPTURE_BYPASS;
> >
> > -   reg &= ~(CQSPI_READLCAPTURE_DELAY_MASK
> > -   << CQSPI_READLCAPTURE_DELAY_LSB);
> > +   reg &= ~(CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK
> > +   << CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB);
> >
> > -   reg |= ((delay & CQSPI_READLCAPTURE_DELAY_MASK)
> > -   << CQSPI_READLCAPTURE_DELAY_LSB);
> > +   reg |= (delay & CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK)
> > +   << CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB;
> >
> > -   writel(reg, reg_base + CQSPI_READLCAPTURE);
> > +   writel(reg, reg_base + CQSPI_REG_RD_DATA_CAPTURE);
> >
> > cadence_qspi_apb_controller_enable(reg_base);
> > return;
> > @@ -301,11 +301,12 @@ void cadence_qspi_apb_set_clk_mode(void
> *reg_base,
> >
> > cadence_qspi_apb_controller_disable(reg_base);
> > reg = readl(reg_base + CQSPI_REG_CONFIG);
> > -   reg &= ~(1 << CQSPI_REG_CONFIG_CLK_POL_LSB);
> > -   reg &= ~(1 << CQSPI_REG_CONFIG_CLK_PHA_LSB);
> > +   reg &= ~(CQSPI_REG_CONFIG_CLK_POL |
> CQSPI_REG_CONFIG_CLK_PHA);
> >
> > -   reg |= ((clk_pol & 0x1) << CQSPI_REG_CONFIG_CLK_POL_LSB);
> > -   reg |= ((clk_pha & 0x1) << CQSPI_REG_CONFIG_CLK_PHA_LSB);
> > +  

[U-Boot] [PATCH v3 11/11] spi: cadence_qspi: Support specifying the sample edge used

[Phil Edworthy]

Introduce a new DT property to specify whether the QSPI Controller
samples the data on a rising edge. The default is falling edge.
Some versions of the QSPI Controller do not implement this bit, in
which case the property should be omitted.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 v3:
  - Patch split so this one only has code related to the subject.
  - Commit message updated.
 v2:
  - Change name of DT prop and provide details of what it does.
Whilst at it, move the code to read the "sram-size" property
into the other code that reads properties from the node, rather
than the SF subnode.

Also change the code to use a bool for the bypass arg.
---
 doc/device-tree-bindings/spi/spi-cadence.txt |  2 ++
 drivers/spi/cadence_qspi.c   | 10 +++---
 drivers/spi/cadence_qspi.h   |  3 ++-
 drivers/spi/cadence_qspi_apb.c   |  8 +++-
 4 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/doc/device-tree-bindings/spi/spi-cadence.txt 
b/doc/device-tree-bindings/spi/spi-cadence.txt
index c1e2233..94c800b 100644
--- a/doc/device-tree-bindings/spi/spi-cadence.txt
+++ b/doc/device-tree-bindings/spi/spi-cadence.txt
@@ -26,3 +26,5 @@ connected flash properties
  select (n_ss_out).
 - tslch-ns : Delay in master reference clocks between setting
  n_ss_out low and first bit transfer
+- sample-edge-rising   : Data outputs from flash memory will be sampled on the
+ rising edge. Default is falling edge.
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
index 1c4ed33..6fa917d 100644
--- a/drivers/spi/cadence_qspi.c
+++ b/drivers/spi/cadence_qspi.c
@@ -39,8 +39,10 @@ static int cadence_spi_write_speed(struct udevice *bus, uint 
hz)
 /* Calibration sequence to determine the read data capture delay register */
 static int spi_calibration(struct udevice *bus, uint hz)
 {
+   struct cadence_spi_platdata *plat = bus->platdata;
struct cadence_spi_priv *priv = dev_get_priv(bus);
void *base = priv->regbase;
+   bool edge = plat->sample_edge_rising;
u8 opcode_rdid = 0x9F;
unsigned int idcode = 0, temp = 0;
int err = 0, i, range_lo = -1, range_hi = -1;
@@ -49,7 +51,7 @@ static int spi_calibration(struct udevice *bus, uint hz)
cadence_spi_write_speed(bus, 100);
 
/* configure the read data capture delay register to 0 */
-   cadence_qspi_apb_readdata_capture(base, true, 0);
+   cadence_qspi_apb_readdata_capture(base, true, edge, 0);
 
/* Enable QSPI */
cadence_qspi_apb_controller_enable(base);
@@ -69,7 +71,7 @@ static int spi_calibration(struct udevice *bus, uint hz)
cadence_qspi_apb_controller_disable(base);
 
/* reconfigure the read data capture delay register */
-   cadence_qspi_apb_readdata_capture(base, true, i);
+   cadence_qspi_apb_readdata_capture(base, true, edge, i);
 
/* Enable back QSPI */
cadence_qspi_apb_controller_enable(base);
@@ -105,7 +107,7 @@ static int spi_calibration(struct udevice *bus, uint hz)
cadence_qspi_apb_controller_disable(base);
 
/* configure the final value for read data capture delay register */
-   cadence_qspi_apb_readdata_capture(base, true,
+   cadence_qspi_apb_readdata_capture(base, true, edge,
(range_hi + range_lo) / 2);
debug("SF: Read data capture delay calibrated to %i (%i - %i)\n",
  (range_hi + range_lo) / 2, range_lo, range_hi);
@@ -317,6 +319,8 @@ static int cadence_spi_ofdata_to_platdata(struct udevice 
*bus)
plat->tsd2d_ns = fdtdec_get_int(blob, subnode, "tsd2d-ns", 255);
plat->tchsh_ns = fdtdec_get_int(blob, subnode, "tchsh-ns", 20);
plat->tslch_ns = fdtdec_get_int(blob, subnode, "tslch-ns", 20);
+   plat->sample_edge_rising = fdtdec_get_bool(blob, subnode,
+   "sample-edge-rising");
 
debug("%s: regbase=%p ahbbase=%p max-frequency=%d page-size=%d\n",
  __func__, plat->regbase, plat->ahbbase, plat->max_hz,
diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h
index 50c6e7c..897e5f0 100644
--- a/drivers/spi/cadence_qspi.h
+++ b/drivers/spi/cadence_qspi.h
@@ -26,6 +26,7 @@ struct cadence_spi_platdata {
u32 tchsh_ns;
u32 tslch_ns;
u32 sram_size;
+   boolsample_edge_rising;
 };
 
 struct cadence_spi_priv {
@@ -72,6 +73,6 @@ void cadence_qspi_apb_delay(void *reg_base,
unsigned int tchsh_ns, unsigned int tslch_ns);
 void cadence_qspi_apb_enter_xip(void *reg_base, char xip_dummy);
 void cadence_qspi_apb_readdata_capture(void *reg_base,
-   bool bypass, unsigned int delay);
+   bool bypass, bool e

[U-Boot] [PATCH v3 10/11] spi: cadence_qspi: Change readdata_capture() arg to bool

[Phil Edworthy]

This is in preparation for adding another arg.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 v3:
  - New patch to split changes.
---
 drivers/spi/cadence_qspi.c | 7 ---
 drivers/spi/cadence_qspi.h | 2 +-
 drivers/spi/cadence_qspi_apb.c | 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
index f16f90d..1c4ed33 100644
--- a/drivers/spi/cadence_qspi.c
+++ b/drivers/spi/cadence_qspi.c
@@ -49,7 +49,7 @@ static int spi_calibration(struct udevice *bus, uint hz)
cadence_spi_write_speed(bus, 100);
 
/* configure the read data capture delay register to 0 */
-   cadence_qspi_apb_readdata_capture(base, 1, 0);
+   cadence_qspi_apb_readdata_capture(base, true, 0);
 
/* Enable QSPI */
cadence_qspi_apb_controller_enable(base);
@@ -69,7 +69,7 @@ static int spi_calibration(struct udevice *bus, uint hz)
cadence_qspi_apb_controller_disable(base);
 
/* reconfigure the read data capture delay register */
-   cadence_qspi_apb_readdata_capture(base, 1, i);
+   cadence_qspi_apb_readdata_capture(base, true, i);
 
/* Enable back QSPI */
cadence_qspi_apb_controller_enable(base);
@@ -105,7 +105,8 @@ static int spi_calibration(struct udevice *bus, uint hz)
cadence_qspi_apb_controller_disable(base);
 
/* configure the final value for read data capture delay register */
-   cadence_qspi_apb_readdata_capture(base, 1, (range_hi + range_lo) / 2);
+   cadence_qspi_apb_readdata_capture(base, true,
+   (range_hi + range_lo) / 2);
debug("SF: Read data capture delay calibrated to %i (%i - %i)\n",
  (range_hi + range_lo) / 2, range_lo, range_hi);
 
diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h
index d1927a4..50c6e7c 100644
--- a/drivers/spi/cadence_qspi.h
+++ b/drivers/spi/cadence_qspi.h
@@ -72,6 +72,6 @@ void cadence_qspi_apb_delay(void *reg_base,
unsigned int tchsh_ns, unsigned int tslch_ns);
 void cadence_qspi_apb_enter_xip(void *reg_base, char xip_dummy);
 void cadence_qspi_apb_readdata_capture(void *reg_base,
-   unsigned int bypass, unsigned int delay);
+   bool bypass, unsigned int delay);
 
 #endif /* __CADENCE_QSPI_H__ */
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index df6a91f..f2cd852 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -234,7 +234,7 @@ static unsigned int cadence_qspi_wait_idle(void *reg_base)
 }
 
 void cadence_qspi_apb_readdata_capture(void *reg_base,
-   unsigned int bypass, unsigned int delay)
+   bool bypass, unsigned int delay)
 {
unsigned int reg;
cadence_qspi_apb_controller_disable(reg_base);
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 08/11] spi: cadence_qspi: Fix CS timings

[Phil Edworthy]

The Cadence QSPI controller has specified overheads for the various CS
times that are in addition to those programmed in to the Device Delay
register. The overheads are different for the delays.

In addition, the existing code does not handle the case when the delay
is less than a SCLK period.

This change accurately calculates the additional delays in Ref clocks.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 v3:
  - No change.
 v2:
 Was "spi: cadence_qspi: Fix CQSPI_CAL_DELAY calculation"
 Note only did the existing code not cope with the delay less than
 an SCLK period, but the calculation didn't round properly, and
 didn't take into account the delays that the QSPI Controller adds
 to those programmed into the Device Delay reg.
---
 drivers/spi/cadence_qspi_apb.c | 23 ---
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 39e31f6..df6a91f 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -169,9 +169,6 @@
((readl(base + CQSPI_REG_CONFIG) >> \
CQSPI_REG_CONFIG_IDLE_LSB) & 0x1)
 
-#define CQSPI_CAL_DELAY(tdelay_ns, tref_ns, tsclk_ns)  \
-   tdelay_ns) - (tsclk_ns)) / (tref_ns)))
-
 #define CQSPI_GET_RD_SRAM_LEVEL(reg_base)  \
(((readl(reg_base + CQSPI_REG_SDRAMLEVEL)) >>   \
CQSPI_REG_SDRAMLEVEL_RD_LSB) & CQSPI_REG_SDRAMLEVEL_RD_MASK)
@@ -355,16 +352,20 @@ void cadence_qspi_apb_delay(void *reg_base,
cadence_qspi_apb_controller_disable(reg_base);
 
/* Convert to ns. */
-   ref_clk_ns = (10) / ref_clk;
+   ref_clk_ns = DIV_ROUND_UP(10, ref_clk);
 
/* Convert to ns. */
-   sclk_ns = (10) / sclk_hz;
-
-   /* Plus 1 to round up 1 clock cycle. */
-   tshsl = CQSPI_CAL_DELAY(tshsl_ns, ref_clk_ns, sclk_ns) + 1;
-   tchsh = CQSPI_CAL_DELAY(tchsh_ns, ref_clk_ns, sclk_ns) + 1;
-   tslch = CQSPI_CAL_DELAY(tslch_ns, ref_clk_ns, sclk_ns) + 1;
-   tsd2d = CQSPI_CAL_DELAY(tsd2d_ns, ref_clk_ns, sclk_ns) + 1;
+   sclk_ns = DIV_ROUND_UP(10, sclk_hz);
+
+   /* The controller adds additional delay to that programmed in the reg */
+   if (tshsl_ns >= sclk_ns + ref_clk_ns)
+   tshsl_ns -= sclk_ns + ref_clk_ns;
+   if (tchsh_ns >= sclk_ns + 3 * ref_clk_ns)
+   tchsh_ns -= sclk_ns + 3 * ref_clk_ns;
+   tshsl = DIV_ROUND_UP(tshsl_ns, ref_clk_ns);
+   tchsh = DIV_ROUND_UP(tchsh_ns, ref_clk_ns);
+   tslch = DIV_ROUND_UP(tslch_ns, ref_clk_ns);
+   tsd2d = DIV_ROUND_UP(tsd2d_ns, ref_clk_ns);
 
reg = ((tshsl & CQSPI_REG_DELAY_TSHSL_MASK)
<< CQSPI_REG_DELAY_TSHSL_LSB);
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 09/11] spi: cadence_qspi: Move DT prop code to match layout

[Phil Edworthy]

Move the code to read the "sram-size" property into the other code
that reads properties from the node, rather than the SF subnode.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 v3:
  - New patch to split changes.
---
 drivers/spi/cadence_qspi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
index 55192d6..f16f90d 100644
--- a/drivers/spi/cadence_qspi.c
+++ b/drivers/spi/cadence_qspi.c
@@ -296,6 +296,7 @@ static int cadence_spi_ofdata_to_platdata(struct udevice 
*bus)
 
plat->regbase = (void *)data[0];
plat->ahbbase = (void *)data[2];
+   plat->sram_size = fdtdec_get_int(blob, node, "sram-size", 128);
 
/* All other paramters are embedded in the child node */
subnode = fdt_first_subnode(blob, node);
@@ -315,7 +316,6 @@ static int cadence_spi_ofdata_to_platdata(struct udevice 
*bus)
plat->tsd2d_ns = fdtdec_get_int(blob, subnode, "tsd2d-ns", 255);
plat->tchsh_ns = fdtdec_get_int(blob, subnode, "tchsh-ns", 20);
plat->tslch_ns = fdtdec_get_int(blob, subnode, "tslch-ns", 20);
-   plat->sram_size = fdtdec_get_int(blob, node, "sram-size", 128);
 
debug("%s: regbase=%p ahbbase=%p max-frequency=%d page-size=%d\n",
  __func__, plat->regbase, plat->ahbbase, plat->max_hz,
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 07/11] spi: cadence_qspi: Remove returns from end of void functions

[Phil Edworthy]

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
Acked-by: Marek Vasut <marek.va...@gmail.com>
---
 v3:
  - No change.
 v2:
  - No change.
---
 drivers/spi/cadence_qspi_apb.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index e81d678..39e31f6 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -199,7 +199,6 @@ void cadence_qspi_apb_controller_enable(void *reg_base)
reg = readl(reg_base + CQSPI_REG_CONFIG);
reg |= CQSPI_REG_CONFIG_ENABLE;
writel(reg, reg_base + CQSPI_REG_CONFIG);
-   return;
 }
 
 void cadence_qspi_apb_controller_disable(void *reg_base)
@@ -208,7 +207,6 @@ void cadence_qspi_apb_controller_disable(void *reg_base)
reg = readl(reg_base + CQSPI_REG_CONFIG);
reg &= ~CQSPI_REG_CONFIG_ENABLE;
writel(reg, reg_base + CQSPI_REG_CONFIG);
-   return;
 }
 
 /* Return 1 if idle, otherwise return 0 (busy). */
@@ -260,7 +258,6 @@ void cadence_qspi_apb_readdata_capture(void *reg_base,
writel(reg, reg_base + CQSPI_REG_RD_DATA_CAPTURE);
 
cadence_qspi_apb_controller_enable(reg_base);
-   return;
 }
 
 void cadence_qspi_apb_config_baudrate_div(void *reg_base,
@@ -291,7 +288,6 @@ void cadence_qspi_apb_config_baudrate_div(void *reg_base,
writel(reg, reg_base + CQSPI_REG_CONFIG);
 
cadence_qspi_apb_controller_enable(reg_base);
-   return;
 }
 
 void cadence_qspi_apb_set_clk_mode(void *reg_base, uint mode)
@@ -310,7 +306,6 @@ void cadence_qspi_apb_set_clk_mode(void *reg_base, uint 
mode)
writel(reg, reg_base + CQSPI_REG_CONFIG);
 
cadence_qspi_apb_controller_enable(reg_base);
-   return;
 }
 
 void cadence_qspi_apb_chipselect(void *reg_base,
@@ -345,7 +340,6 @@ void cadence_qspi_apb_chipselect(void *reg_base,
writel(reg, reg_base + CQSPI_REG_CONFIG);
 
cadence_qspi_apb_controller_enable(reg_base);
-   return;
 }
 
 void cadence_qspi_apb_delay(void *reg_base,
@@ -383,7 +377,6 @@ void cadence_qspi_apb_delay(void *reg_base,
writel(reg, reg_base + CQSPI_REG_DELAY);
 
cadence_qspi_apb_controller_enable(reg_base);
-   return;
 }
 
 void cadence_qspi_apb_controller_init(struct cadence_spi_platdata *plat)
@@ -411,7 +404,6 @@ void cadence_qspi_apb_controller_init(struct 
cadence_spi_platdata *plat)
writel(0, plat->regbase + CQSPI_REG_IRQMASK);
 
cadence_qspi_apb_controller_enable(plat->regbase);
-   return;
 }
 
 static int cadence_qspi_apb_exec_flash_cmd(void *reg_base,
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 06/11] spi: cadence_qspi: Use spi mode at the point it is needed

[Phil Edworthy]

Instead of extracting mode settings and passing them as separate
args to another function, just pass the SPI mode as an arg.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 v3:
  - New patch introduced to address comments.
---
 drivers/spi/cadence_qspi.c | 4 +---
 drivers/spi/cadence_qspi.h | 3 +--
 drivers/spi/cadence_qspi_apb.c | 7 +++
 3 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
index 1051afb..55192d6 100644
--- a/drivers/spi/cadence_qspi.c
+++ b/drivers/spi/cadence_qspi.c
@@ -170,14 +170,12 @@ static int cadence_spi_probe(struct udevice *bus)
 static int cadence_spi_set_mode(struct udevice *bus, uint mode)
 {
struct cadence_spi_priv *priv = dev_get_priv(bus);
-   unsigned int clk_pol = (mode & SPI_CPOL) ? 1 : 0;
-   unsigned int clk_pha = (mode & SPI_CPHA) ? 1 : 0;
 
/* Disable QSPI */
cadence_qspi_apb_controller_disable(priv->regbase);
 
/* Set SPI mode */
-   cadence_qspi_apb_set_clk_mode(priv->regbase, clk_pol, clk_pha);
+   cadence_qspi_apb_set_clk_mode(priv->regbase, mode);
 
/* Enable QSPI */
cadence_qspi_apb_controller_enable(priv->regbase);
diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h
index a849f7b..d1927a4 100644
--- a/drivers/spi/cadence_qspi.h
+++ b/drivers/spi/cadence_qspi.h
@@ -63,8 +63,7 @@ int cadence_qspi_apb_indirect_write_execute(struct 
cadence_spi_platdata *plat,
 
 void cadence_qspi_apb_chipselect(void *reg_base,
unsigned int chip_select, unsigned int decoder_enable);
-void cadence_qspi_apb_set_clk_mode(void *reg_base_addr,
-   unsigned int clk_pol, unsigned int clk_pha);
+void cadence_qspi_apb_set_clk_mode(void *reg_base, uint mode);
 void cadence_qspi_apb_config_baudrate_div(void *reg_base,
unsigned int ref_clk_hz, unsigned int sclk_hz);
 void cadence_qspi_apb_delay(void *reg_base,
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 634a857..e81d678 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -294,8 +294,7 @@ void cadence_qspi_apb_config_baudrate_div(void *reg_base,
return;
 }
 
-void cadence_qspi_apb_set_clk_mode(void *reg_base,
-   unsigned int clk_pol, unsigned int clk_pha)
+void cadence_qspi_apb_set_clk_mode(void *reg_base, uint mode)
 {
unsigned int reg;
 
@@ -303,9 +302,9 @@ void cadence_qspi_apb_set_clk_mode(void *reg_base,
reg = readl(reg_base + CQSPI_REG_CONFIG);
reg &= ~(CQSPI_REG_CONFIG_CLK_POL | CQSPI_REG_CONFIG_CLK_PHA);
 
-   if (clk_pol)
+   if (mode & SPI_CPOL)
reg |= CQSPI_REG_CONFIG_CLK_POL;
-   if (clk_pha)
+   if (mode & SPI_CPHA)
reg |= CQSPI_REG_CONFIG_CLK_PHA;
 
writel(reg, reg_base + CQSPI_REG_CONFIG);
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 05/11] spi: cadence_qspi: Clean up the #define names

[Phil Edworthy]

A lot of the #defines are for single bits in a register, where the
name has _MASK on the end. Since this can be used for both a mask
and the value, remove _MASK from them.

Whilst doing so, also remove the unnecessary brackets around the
constants.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
Acked-by: Marek Vasut <marek.va...@gmail.com>
---
 v3:
  - No change.
 v2:
  - No change.
---
 drivers/spi/cadence_qspi_apb.c | 86 +-
 1 file changed, 43 insertions(+), 43 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index b41f36b..634a857 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -32,37 +32,37 @@
 #include 
 #include "cadence_qspi.h"
 
-#define CQSPI_REG_POLL_US  (1) /* 1us */
-#define CQSPI_REG_RETRY(1)
-#define CQSPI_POLL_IDLE_RETRY  (3)
+#define CQSPI_REG_POLL_US  1 /* 1us */
+#define CQSPI_REG_RETRY1
+#define CQSPI_POLL_IDLE_RETRY  3
 
-#define CQSPI_FIFO_WIDTH   (4)
+#define CQSPI_FIFO_WIDTH   4
 
-#define CQSPI_REG_SRAM_THRESHOLD_WORDS (50)
+#define CQSPI_REG_SRAM_THRESHOLD_WORDS 50
 
 /* Transfer mode */
-#define CQSPI_INST_TYPE_SINGLE (0)
-#define CQSPI_INST_TYPE_DUAL   (1)
-#define CQSPI_INST_TYPE_QUAD   (2)
+#define CQSPI_INST_TYPE_SINGLE 0
+#define CQSPI_INST_TYPE_DUAL   1
+#define CQSPI_INST_TYPE_QUAD   2
 
-#define CQSPI_STIG_DATA_LEN_MAX(8)
-
-#define CQSPI_DUMMY_CLKS_PER_BYTE  (8)
-#define CQSPI_DUMMY_BYTES_MAX  (4)
+#define CQSPI_STIG_DATA_LEN_MAX8
 
+#define CQSPI_DUMMY_CLKS_PER_BYTE  8
+#define CQSPI_DUMMY_BYTES_MAX  4
 
 #define CQSPI_REG_SRAM_FILL_THRESHOLD  \
((CQSPI_REG_SRAM_SIZE_WORD / 2) * CQSPI_FIFO_WIDTH)
+
 /
  * Controller's configuration and status register (offset from QSPI_BASE)
  /
 #defineCQSPI_REG_CONFIG0x00
-#defineCQSPI_REG_CONFIG_ENABLE_MASKBIT(0)
+#defineCQSPI_REG_CONFIG_ENABLE BIT(0)
 #defineCQSPI_REG_CONFIG_CLK_POLBIT(1)
 #defineCQSPI_REG_CONFIG_CLK_PHABIT(2)
-#defineCQSPI_REG_CONFIG_DIRECT_MASKBIT(7)
-#defineCQSPI_REG_CONFIG_DECODE_MASKBIT(9)
-#defineCQSPI_REG_CONFIG_XIP_IMM_MASK   BIT(18)
+#defineCQSPI_REG_CONFIG_DIRECT BIT(7)
+#defineCQSPI_REG_CONFIG_DECODE BIT(9)
+#defineCQSPI_REG_CONFIG_XIP_IMMBIT(18)
 #defineCQSPI_REG_CONFIG_CHIPSELECT_LSB 10
 #defineCQSPI_REG_CONFIG_BAUD_LSB   19
 #defineCQSPI_REG_CONFIG_IDLE_LSB   31
@@ -123,18 +123,18 @@
 #defineCQSPI_REG_IRQMASK   0x44
 
 #defineCQSPI_REG_INDIRECTRD0x60
-#defineCQSPI_REG_INDIRECTRD_START_MASK BIT(0)
-#defineCQSPI_REG_INDIRECTRD_CANCEL_MASKBIT(1)
-#defineCQSPI_REG_INDIRECTRD_INPROGRESS_MASKBIT(2)
-#defineCQSPI_REG_INDIRECTRD_DONE_MASK  BIT(5)
+#defineCQSPI_REG_INDIRECTRD_START  BIT(0)
+#defineCQSPI_REG_INDIRECTRD_CANCEL BIT(1)
+#defineCQSPI_REG_INDIRECTRD_INPROGRESS BIT(2)
+#defineCQSPI_REG_INDIRECTRD_DONE   BIT(5)
 
 #defineCQSPI_REG_INDIRECTRDWATERMARK   0x64
 #defineCQSPI_REG_INDIRECTRDSTARTADDR   0x68
 #defineCQSPI_REG_INDIRECTRDBYTES   0x6C
 
 #defineCQSPI_REG_CMDCTRL   0x90
-#defineCQSPI_REG_CMDCTRL_EXECUTE_MASK  BIT(0)
-#defineCQSPI_REG_CMDCTRL_INPROGRESS_MASK   BIT(1)
+#defineCQSPI_REG_CMDCTRL_EXECUTE   BIT(0)
+#defineCQSPI_REG_CMDCTRL_INPROGRESSBIT(1)
 #defineCQSPI_REG_CMDCTRL_DUMMY_LSB 7
 #defineCQSPI_REG_CMDCTRL_WR_BYTES_LSB  12
 #defineCQSPI_REG_CMDCTRL_WR_EN_LSB 15
@@ -150,10 +150,10 @@
 #defineCQSPI_REG_CMDCTRL_OPCODE_MASK   0xFF
 
 #defineCQSPI_REG_INDIRECTWR0x70
-#defineCQSPI_REG_INDIRECTWR_START_MASK BIT(0)
-#defineCQSPI_REG_INDIRECTWR_CANCEL_MASKBIT(1)
-#defineCQSPI_REG_INDIRECTWR_INPROGRESS_MASKBIT(2)
-#defineCQSPI_REG_INDIRECTWR_DONE_MASK  BIT(5)
+#defineCQSPI_REG_INDIRECTWR_START   

[U-Boot] [PATCH v3 03/11] spi: cadence_qspi: Better debug information on the SPI clock rate

[Phil Edworthy]

Show what the output clock rate actually is.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
Acked-by: Marek Vasut <marek.va...@gmail.com>
---
 v3:
  - No change.
 v2:
  - No change.
---
 drivers/spi/cadence_qspi_apb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index b5c664f..0a2963d 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -280,13 +280,13 @@ void cadence_qspi_apb_config_baudrate_div(void *reg_base,
 */
div = DIV_ROUND_UP(ref_clk_hz, sclk_hz * 2) - 1;
 
-   debug("%s: ref_clk %dHz sclk %dHz Div 0x%x\n", __func__,
- ref_clk_hz, sclk_hz, div);
-
/* ensure the baud rate doesn't exceed the max value */
if (div > CQSPI_REG_CONFIG_BAUD_MASK)
div = CQSPI_REG_CONFIG_BAUD_MASK;
 
+   debug("%s: ref_clk %dHz sclk %dHz Div 0x%x, actual %dHz\n", __func__,
+ ref_clk_hz, sclk_hz, div, ref_clk_hz / (2 * (div + 1)));
+
reg |= (div << CQSPI_REG_CONFIG_BAUD_LSB);
writel(reg, reg_base + CQSPI_REG_CONFIG);
 
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 04/11] spi: cadence_qspi: Use #define for bits instead of bit shifts

[Phil Edworthy]

Most of the code already uses #defines for the bit value, rather
than the shift required to get the value. This changes the remaining
code over.

Whislt at it, fix the names of the "Rd Data Capture" register defs.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
Acked-by: Marek Vasut <marek.va...@gmail.com>
---
 v3:
 - Remove brackets that aren't needed.
 v2:
 - No change.
---
 drivers/spi/cadence_qspi_apb.c | 37 +++--
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 0a2963d..b41f36b 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -57,9 +57,9 @@
  * Controller's configuration and status register (offset from QSPI_BASE)
  /
 #defineCQSPI_REG_CONFIG0x00
-#defineCQSPI_REG_CONFIG_CLK_POL_LSB1
-#defineCQSPI_REG_CONFIG_CLK_PHA_LSB2
 #defineCQSPI_REG_CONFIG_ENABLE_MASKBIT(0)
+#defineCQSPI_REG_CONFIG_CLK_POLBIT(1)
+#defineCQSPI_REG_CONFIG_CLK_PHABIT(2)
 #defineCQSPI_REG_CONFIG_DIRECT_MASKBIT(7)
 #defineCQSPI_REG_CONFIG_DECODE_MASKBIT(9)
 #defineCQSPI_REG_CONFIG_XIP_IMM_MASK   BIT(18)
@@ -94,10 +94,10 @@
 #defineCQSPI_REG_DELAY_TSD2D_MASK  0xFF
 #defineCQSPI_REG_DELAY_TSHSL_MASK  0xFF
 
-#defineCQSPI_READLCAPTURE  0x10
-#defineCQSPI_READLCAPTURE_BYPASS_LSB   0
-#defineCQSPI_READLCAPTURE_DELAY_LSB1
-#defineCQSPI_READLCAPTURE_DELAY_MASK   0xF
+#defineCQSPI_REG_RD_DATA_CAPTURE   0x10
+#defineCQSPI_REG_RD_DATA_CAPTURE_BYPASSBIT(0)
+#defineCQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB 1
+#defineCQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK0xF
 
 #defineCQSPI_REG_SIZE  0x14
 #defineCQSPI_REG_SIZE_ADDRESS_LSB  0
@@ -244,20 +244,20 @@ void cadence_qspi_apb_readdata_capture(void *reg_base,
unsigned int reg;
cadence_qspi_apb_controller_disable(reg_base);
 
-   reg = readl(reg_base + CQSPI_READLCAPTURE);
+   reg = readl(reg_base + CQSPI_REG_RD_DATA_CAPTURE);
 
if (bypass)
-   reg |= (1 << CQSPI_READLCAPTURE_BYPASS_LSB);
+   reg |= CQSPI_REG_RD_DATA_CAPTURE_BYPASS;
else
-   reg &= ~(1 << CQSPI_READLCAPTURE_BYPASS_LSB);
+   reg &= ~CQSPI_REG_RD_DATA_CAPTURE_BYPASS;
 
-   reg &= ~(CQSPI_READLCAPTURE_DELAY_MASK
-   << CQSPI_READLCAPTURE_DELAY_LSB);
+   reg &= ~(CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK
+   << CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB);
 
-   reg |= ((delay & CQSPI_READLCAPTURE_DELAY_MASK)
-   << CQSPI_READLCAPTURE_DELAY_LSB);
+   reg |= (delay & CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK)
+   << CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB;
 
-   writel(reg, reg_base + CQSPI_READLCAPTURE);
+   writel(reg, reg_base + CQSPI_REG_RD_DATA_CAPTURE);
 
cadence_qspi_apb_controller_enable(reg_base);
return;
@@ -301,11 +301,12 @@ void cadence_qspi_apb_set_clk_mode(void *reg_base,
 
cadence_qspi_apb_controller_disable(reg_base);
reg = readl(reg_base + CQSPI_REG_CONFIG);
-   reg &= ~(1 << CQSPI_REG_CONFIG_CLK_POL_LSB);
-   reg &= ~(1 << CQSPI_REG_CONFIG_CLK_PHA_LSB);
+   reg &= ~(CQSPI_REG_CONFIG_CLK_POL | CQSPI_REG_CONFIG_CLK_PHA);
 
-   reg |= ((clk_pol & 0x1) << CQSPI_REG_CONFIG_CLK_POL_LSB);
-   reg |= ((clk_pha & 0x1) << CQSPI_REG_CONFIG_CLK_PHA_LSB);
+   if (clk_pol)
+   reg |= CQSPI_REG_CONFIG_CLK_POL;
+   if (clk_pha)
+   reg |= CQSPI_REG_CONFIG_CLK_PHA;
 
writel(reg, reg_base + CQSPI_REG_CONFIG);
 
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 02/11] spi: cadence_qspi: Fix baud rate calculation

[Phil Edworthy]

With the existing code, when the requested SPI clock rate is near
to the lowest that can be achieved by the hardware (max divider
of the ref clock is 32), the generated clock rate is wrong.
For example, with a 50MHz ref clock, when asked for anything less
than a 1.5MHz SPI clock, the code sets up the divider to generate
25MHz.

This change fixes the calculation.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
v3:
 - Use single line DIV_ROUND_UP instead of two
v2:
 - Use the DIV_ROUND_UP macro
---
 drivers/spi/cadence_qspi_apb.c | 22 ++
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 2403e71..b5c664f 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -273,22 +273,12 @@ void cadence_qspi_apb_config_baudrate_div(void *reg_base,
reg = readl(reg_base + CQSPI_REG_CONFIG);
reg &= ~(CQSPI_REG_CONFIG_BAUD_MASK << CQSPI_REG_CONFIG_BAUD_LSB);
 
-   div = ref_clk_hz / sclk_hz;
-
-   if (div > 32)
-   div = 32;
-
-   /* Check if even number. */
-   if ((div & 1)) {
-   div = (div / 2);
-   } else {
-   if (ref_clk_hz % sclk_hz)
-   /* ensure generated SCLK doesn't exceed user
-   specified sclk_hz */
-   div = (div / 2);
-   else
-   div = (div / 2) - 1;
-   }
+   /*
+* The baud_div field in the config reg is 4 bits, and the ref clock is
+* divided by 2 * (baud_div + 1). Round up the divider to ensure the
+* SPI clock rate is less than or equal to the requested clock rate.
+*/
+   div = DIV_ROUND_UP(ref_clk_hz, sclk_hz * 2) - 1;
 
debug("%s: ref_clk %dHz sclk %dHz Div 0x%x\n", __func__,
  ref_clk_hz, sclk_hz, div);
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 00/11] SF: Cadence QSPI driver fixes and clean up

[Phil Edworthy]

This series has fixes, patches to clean the code up, and add support for
specifying the sampling edge.

Main changes in v3:
  1. Added "spi: cadence_qspi: Use spi mode at the point it is needed" to
 address comments for SPI pol/phase code.
  2. "spi: cadence_qspi: Support specifying the sample edge used" has been
 split into 3 separate patches.


Phil Edworthy (11):
  spi: cadence_qspi: Fix clearing of pol/pha bits
  spi: cadence_qspi: Fix baud rate calculation
  spi: cadence_qspi: Better debug information on the SPI clock rate
  spi: cadence_qspi: Use #define for bits instead of bit shifts
  spi: cadence_qspi: Clean up the #define names
  spi: cadence_qspi: Use spi mode at the point it is needed
  spi: cadence_qspi: Remove returns from end of void functions
  spi: cadence_qspi: Fix CS timings
  spi: cadence_qspi: Move DT prop code to match layout
  spi: cadence_qspi: Change readdata_capture() arg to bool
  spi: cadence_qspi: Support specifying the sample edge used

 doc/device-tree-bindings/spi/spi-cadence.txt |   2 +
 drivers/spi/cadence_qspi.c   |  17 ++-
 drivers/spi/cadence_qspi.h   |   6 +-
 drivers/spi/cadence_qspi_apb.c   | 193 +--
 4 files changed, 106 insertions(+), 112 deletions(-)

-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 01/11] spi: cadence_qspi: Fix clearing of pol/pha bits

[Phil Edworthy]

Or'ing together bit positions is clearly wrong.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
Acked-by: Marek Vasut <marek.va...@gmail.com>
---
 v3:
  - No change.
 v2:
  - No change.
---
 drivers/spi/cadence_qspi_apb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index e285d3c..2403e71 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -311,8 +311,8 @@ void cadence_qspi_apb_set_clk_mode(void *reg_base,
 
cadence_qspi_apb_controller_disable(reg_base);
reg = readl(reg_base + CQSPI_REG_CONFIG);
-   reg &= ~(1 <<
-   (CQSPI_REG_CONFIG_CLK_POL_LSB | CQSPI_REG_CONFIG_CLK_PHA_LSB));
+   reg &= ~(1 << CQSPI_REG_CONFIG_CLK_POL_LSB);
+   reg &= ~(1 << CQSPI_REG_CONFIG_CLK_PHA_LSB);
 
reg |= ((clk_pol & 0x1) << CQSPI_REG_CONFIG_CLK_POL_LSB);
reg |= ((clk_pha & 0x1) << CQSPI_REG_CONFIG_CLK_PHA_LSB);
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 7/8] spi: cadence_qspi: Fix CS timings

[Phil Edworthy]

Hi Chin Liang,

On 28 November 2016 12:49 See, Chin Liang wrote:
> On Jum, 2016-11-25 at 14:38 +0000, Phil Edworthy wrote:
> >
> > The Cadence QSPI controller has specified overheads for the various
> > CS
> > times that are in addition to those programmed in to the Device Delay
> > register. The overheads are different for the delays.
> >
> > In addition, the existing code does not handle the case when the
> > delay
> > is less than a SCLK period.
> >
> > This change accurately calculates the additional delays in Ref
> > clocks.
> >
> > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > ---
> > v2:
> >  Was "spi: cadence_qspi: Fix CQSPI_CAL_DELAY calculation"
> >  Note only did the existing code not cope with the delay less than
> >  an SCLK period, but the calculation didn't round properly, and
> >  didn't take into account the delays that the QSPI Controller adds
> >  to those programmed into the Device Delay reg.
> > ---
> >  drivers/spi/cadence_qspi_apb.c | 23 ---
> >  1 file changed, 12 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/spi/cadence_qspi_apb.c
> > b/drivers/spi/cadence_qspi_apb.c
> > index 1cd636a..56ad952 100644
> > --- a/drivers/spi/cadence_qspi_apb.c
> > +++ b/drivers/spi/cadence_qspi_apb.c
> > @@ -169,9 +169,6 @@
> > ((readl(base + CQSPI_REG_CONFIG) >> \
> > CQSPI_REG_CONFIG_IDLE_LSB) & 0x1)
> >
> > -#define CQSPI_CAL_DELAY(tdelay_ns, tref_ns, tsclk_ns)  \
> > -   tdelay_ns) - (tsclk_ns)) / (tref_ns)))
> > -
> >  #define CQSPI_GET_RD_SRAM_LEVEL(reg_base)  \
> > (((readl(reg_base + CQSPI_REG_SDRAMLEVEL)) >>   \
> > CQSPI_REG_SDRAMLEVEL_RD_LSB) &
> CQSPI_REG_SDRAMLEVEL_RD_MASK)
> > @@ -357,16 +354,20 @@ void cadence_qspi_apb_delay(void *reg_base,
> > cadence_qspi_apb_controller_disable(reg_base);
> >
> > /* Convert to ns. */
> > -   ref_clk_ns = (10) / ref_clk;
> > +   ref_clk_ns = DIV_ROUND_UP(10, ref_clk);
> >
> > /* Convert to ns. */
> > -   sclk_ns = (10) / sclk_hz;
> > -
> > -   /* Plus 1 to round up 1 clock cycle. */
> > -   tshsl = CQSPI_CAL_DELAY(tshsl_ns, ref_clk_ns, sclk_ns) + 1;
> > -   tchsh = CQSPI_CAL_DELAY(tchsh_ns, ref_clk_ns, sclk_ns) + 1;
> > -   tslch = CQSPI_CAL_DELAY(tslch_ns, ref_clk_ns, sclk_ns) + 1;
> > -   tsd2d = CQSPI_CAL_DELAY(tsd2d_ns, ref_clk_ns, sclk_ns) + 1;
> > +   sclk_ns = DIV_ROUND_UP(10, sclk_hz);
> > +
> > +   /* The controller adds additional delay to that programmed in
> > the reg */
> > +   if (tshsl_ns >= sclk_ns + ref_clk_ns)
> > +   tshsl_ns -= sclk_ns + ref_clk_ns;
> > +   if (tchsh_ns >= sclk_ns + 3 * ref_clk_ns)
> > +   tchsh_ns -= sclk_ns + 3 * ref_clk_ns;
> Any reason we need this?
> The DIV_ROUND_UP or previous + 1 in algo will ensure its more than a
> SCLK period.
In general, all of these CS timing should be optimal. I measured differences
in throughput with the sub-optimal setting. Admittedly, the difference is
small but still we should set it correctly.

> Thanks
> Chin Liang
> 
> >
> > +   tshsl = DIV_ROUND_UP(tshsl_ns, ref_clk_ns);
> > +   tchsh = DIV_ROUND_UP(tchsh_ns, ref_clk_ns);
> > +   tslch = DIV_ROUND_UP(tslch_ns, ref_clk_ns);
> > +   tsd2d = DIV_ROUND_UP(tsd2d_ns, ref_clk_ns);
> >
> > reg = ((tshsl & CQSPI_REG_DELAY_TSHSL_MASK)
> > << CQSPI_REG_DELAY_TSHSL_LSB);
> > --
> > 2.7.4
> >

Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 2/8] spi: cadence_qspi: Fix baud rate calculation

[Phil Edworthy]

Hi Jagan,

On 25 November 2016 15:42 Jagan Teki wrote:
> On Fri, Nov 25, 2016 at 8:08 PM, Phil Edworthy
> <phil.edwor...@renesas.com> wrote:
> > With the existing code, when the requested SPI clock rate is near
> > to the lowest that can be achieved by the hardware (max divider
> > of the ref clock is 32), the generated clock rate is wrong.
> > For example, with a 50MHz ref clock, when asked for anything less
> > than a 1.5MHz SPI clock, the code sets up the divider to generate
> > 25MHz.
> >
> > This change fixes the calculation.
> >
> > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > ---
> > v2:
> >  - Use the DIV_ROUND_UP macro
> > ---
> >  drivers/spi/cadence_qspi_apb.c | 23 +++
> >  1 file changed, 7 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> > index 2403e71..b9e0df7 100644
> > --- a/drivers/spi/cadence_qspi_apb.c
> > +++ b/drivers/spi/cadence_qspi_apb.c
> > @@ -273,22 +273,13 @@ void cadence_qspi_apb_config_baudrate_div(void
> *reg_base,
> > reg = readl(reg_base + CQSPI_REG_CONFIG);
> > reg &= ~(CQSPI_REG_CONFIG_BAUD_MASK <<
> CQSPI_REG_CONFIG_BAUD_LSB);
> >
> > -   div = ref_clk_hz / sclk_hz;
> > -
> > -   if (div > 32)
> > -   div = 32;
> > -
> > -   /* Check if even number. */
> > -   if ((div & 1)) {
> > -   div = (div / 2);
> > -   } else {
> > -   if (ref_clk_hz % sclk_hz)
> > -   /* ensure generated SCLK doesn't exceed user
> > -   specified sclk_hz */
> > -   div = (div / 2);
> > -   else
> > -   div = (div / 2) - 1;
> > -   }
> > +   /*
> > +* The baud_div field in the config reg is 4 bits, and the ref 
> > clock is
> > +* divided by 2 * (baud_div + 1). Round up the divider to ensure the
> > +* SPI clock rate is less than or equal to the requested clock rate.
> > +*/
> > +   div = DIV_ROUND_UP(ref_clk_hz, sclk_hz);
> > +   div = DIV_ROUND_UP(div, 2) - 1;
> 
> What about div = (ref / (sclk * 2)) -1 that means
> div = DIV_ROUND_UP(ref_clk_hz, sclk_hz * 2) -1;
Yes, that is also the same result. I'll change the code.

> thanks!
> --
> Jagan Teki
> Free Software Engineer | www.openedev.com
> U-Boot, Linux | Upstream Maintainer
> Hyderabad, India.

Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 1/8] spi: cadence_qspi: Fix clearing of pol/pha bits

[Phil Edworthy]

Hi Jagan,

On 25 November 2016 15:29 Jagan Teki wrote:
> On Fri, Nov 25, 2016 at 8:08 PM, Phil Edworthy
> <phil.edwor...@renesas.com> wrote:
> > Or'ing together bit positions is clearly wrong.
> >
> > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > ---
> >  drivers/spi/cadence_qspi_apb.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> > index e285d3c..2403e71 100644
> > --- a/drivers/spi/cadence_qspi_apb.c
> > +++ b/drivers/spi/cadence_qspi_apb.c
> > @@ -311,8 +311,8 @@ void cadence_qspi_apb_set_clk_mode(void *reg_base,
> >
> > cadence_qspi_apb_controller_disable(reg_base);
> > reg = readl(reg_base + CQSPI_REG_CONFIG);
> > -   reg &= ~(1 <<
> > -   (CQSPI_REG_CONFIG_CLK_POL_LSB |
> CQSPI_REG_CONFIG_CLK_PHA_LSB));
> > +   reg &= ~(1 << CQSPI_REG_CONFIG_CLK_POL_LSB);
> > +   reg &= ~(1 << CQSPI_REG_CONFIG_CLK_PHA_LSB);
> 
> OK, but see below
> 
> >
> > reg |= ((clk_pol & 0x1) << CQSPI_REG_CONFIG_CLK_POL_LSB);
> > reg |= ((clk_pha & 0x1) << CQSPI_REG_CONFIG_CLK_PHA_LSB);
> 
> This existing code look not easy to understand for me with doing & and
> << operations with numeric 0x1. what about this
> 
> Say for example POL and PHA on cadence has with bit 1 and 2
> CQSPI_REG_CONFIG_CLK_POL_LSB   BIT(1)
> CQSPI_REG_CONFIG_CLK_PHA_LSB   BIT(2)
> 
> reg &= ~(CQSPI_REG_CONFIG_CLK_PHA_LSB |
> CQSPI_REG_CONFIG_CLK_POL_LSB);
> 
> if (mode & SPI_CPHA)
>  reg |= CQSPI_REG_CONFIG_CLK_PHA_LSB;
> if (mode & SPI_CPOL)
>  reg |= CQSPI_REG_CONFIG_CLK_POL_LSB;

I completely agree. This is addressed in patch 4 along with the other
code that defines shifts.

> thanks!
> --
> Jagan Teki
> Free Software Engineer | www.openedev.com
> U-Boot, Linux | Upstream Maintainer
> Hyderabad, India.

Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 8/8] spi: cadence_qspi: Support specifying the sample edge used

[Phil Edworthy]

Hi Marek,

On 25 November 2016 15:06 Marek Vasut wrote:
> On 11/25/2016 03:38 PM, Phil Edworthy wrote:
> > Whilst at it, move the code to read the "sram-size" property
> > into the other code that reads properties from the node, rather
> > than the SF subnode.
> 
> The commit message does not match the subject, but I think this needs
> splitting into two patches.
Ah, right the subject was covering it.
I'll split the patch in two.

> > Also change the code to use a bool for the bypass arg.
> >
> > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> >
> > ---
> > v2:
> >  Change name of DT prop and provide details of what it does.
> >  Whilst at it, move the code to read the "sram-size" property
> >  into the other code that reads properties from the node, rather
> >  than the SF subnode.
> >
> >  Also change the code to use a bool for the bypass arg.
> > ---
> >  doc/device-tree-bindings/spi/spi-cadence.txt |  2 ++
> >  drivers/spi/cadence_qspi.c   | 13 +
> >  drivers/spi/cadence_qspi.h   |  3 ++-
> >  drivers/spi/cadence_qspi_apb.c   |  8 +++-
> >  4 files changed, 20 insertions(+), 6 deletions(-)
> >
> > diff --git a/doc/device-tree-bindings/spi/spi-cadence.txt b/doc/device-tree-
> bindings/spi/spi-cadence.txt
> > index c1e2233..94c800b 100644
> > --- a/doc/device-tree-bindings/spi/spi-cadence.txt
> > +++ b/doc/device-tree-bindings/spi/spi-cadence.txt
> > @@ -26,3 +26,5 @@ connected flash properties
> >   select (n_ss_out).
> >  - tslch-ns : Delay in master reference clocks between setting
> >   n_ss_out low and first bit transfer
> > +- sample-edge-rising   : Data outputs from flash memory will be sampled
> on the
> > + rising edge. Default is falling edge.
> > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
> > index 1051afb..16fb18e 100644
> > --- a/drivers/spi/cadence_qspi.c
> > +++ b/drivers/spi/cadence_qspi.c
> > @@ -39,8 +39,10 @@ static int cadence_spi_write_speed(struct udevice *bus,
> uint hz)
> >  /* Calibration sequence to determine the read data capture delay register 
> > */
> >  static int spi_calibration(struct udevice *bus, uint hz)
> >  {
> > +   struct cadence_spi_platdata *plat = bus->platdata;
> > struct cadence_spi_priv *priv = dev_get_priv(bus);
> > void *base = priv->regbase;
> > +   bool edge = plat->sample_edge_rising;
> > u8 opcode_rdid = 0x9F;
> > unsigned int idcode = 0, temp = 0;
> > int err = 0, i, range_lo = -1, range_hi = -1;
> > @@ -49,7 +51,7 @@ static int spi_calibration(struct udevice *bus, uint hz)
> > cadence_spi_write_speed(bus, 100);
> >
> > /* configure the read data capture delay register to 0 */
> > -   cadence_qspi_apb_readdata_capture(base, 1, 0);
> > +   cadence_qspi_apb_readdata_capture(base, true, edge, 0);
> >
> > /* Enable QSPI */
> > cadence_qspi_apb_controller_enable(base);
> > @@ -69,7 +71,7 @@ static int spi_calibration(struct udevice *bus, uint hz)
> > cadence_qspi_apb_controller_disable(base);
> >
> > /* reconfigure the read data capture delay register */
> > -   cadence_qspi_apb_readdata_capture(base, 1, i);
> > +   cadence_qspi_apb_readdata_capture(base, true, edge, i);
> >
> > /* Enable back QSPI */
> > cadence_qspi_apb_controller_enable(base);
> > @@ -105,7 +107,8 @@ static int spi_calibration(struct udevice *bus, uint hz)
> > cadence_qspi_apb_controller_disable(base);
> >
> > /* configure the final value for read data capture delay register */
> > -   cadence_qspi_apb_readdata_capture(base, 1, (range_hi + range_lo) / 2);
> > +   cadence_qspi_apb_readdata_capture(base, true, edge,
> > +   (range_hi + range_lo) / 2);
> > debug("SF: Read data capture delay calibrated to %i (%i - %i)\n",
> >   (range_hi + range_lo) / 2, range_lo, range_hi);
> >
> > @@ -298,6 +301,7 @@ static int cadence_spi_ofdata_to_platdata(struct
> udevice *bus)
> >
> > plat->regbase = (void *)data[0];
> > plat->ahbbase = (void *)data[2];
> > +   plat->sram_size = fdtdec_get_int(blob, node, "sram-size", 128);
> >
> > /* All other paramters are embedded in the child node */
> > subnode = fdt_first_subnode(blob, node);
> > @@ -317,7 +321,8 @@ static int cadence_spi_ofdata_to_platda

Re: [U-Boot] [PATCH v2 4/8] spi: cadence_qspi: Use #define for bits instead of bit shifts

[Phil Edworthy]

Hi Marek,

On 25 November 2016 15:00 Marek Vasut wrote:
> On 11/25/2016 03:38 PM, Phil Edworthy wrote:
> > Most of the code already uses #defines for the bit value, rather
> > than the shift required to get the value. This changes the remaining
> > code over.
> >
> > Whislt at it, fix the names of the "Rd Data Capture" register defs.
> >
> > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > ---
> >  drivers/spi/cadence_qspi_apb.c | 37 +++--
> >  1 file changed, 19 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> > index 3ae4b5a..cd46a15 100644
> > --- a/drivers/spi/cadence_qspi_apb.c
> > +++ b/drivers/spi/cadence_qspi_apb.c
> > @@ -57,9 +57,9 @@
> >   * Controller's configuration and status register (offset from QSPI_BASE)
> >
> **
> **/
> >  #defineCQSPI_REG_CONFIG0x00
> > -#defineCQSPI_REG_CONFIG_CLK_POL_LSB1
> > -#defineCQSPI_REG_CONFIG_CLK_PHA_LSB2
> >  #defineCQSPI_REG_CONFIG_ENABLE_MASKBIT(0)
> > +#defineCQSPI_REG_CONFIG_CLK_POLBIT(1)
> > +#defineCQSPI_REG_CONFIG_CLK_PHABIT(2)
> >  #defineCQSPI_REG_CONFIG_DIRECT_MASKBIT(7)
> >  #defineCQSPI_REG_CONFIG_DECODE_MASKBIT(9)
> >  #defineCQSPI_REG_CONFIG_XIP_IMM_MASK   BIT(18)
> > @@ -94,10 +94,10 @@
> >  #defineCQSPI_REG_DELAY_TSD2D_MASK  0xFF
> >  #defineCQSPI_REG_DELAY_TSHSL_MASK  0xFF
> >
> > -#defineCQSPI_READLCAPTURE  0x10
> > -#defineCQSPI_READLCAPTURE_BYPASS_LSB   0
> > -#defineCQSPI_READLCAPTURE_DELAY_LSB1
> > -#defineCQSPI_READLCAPTURE_DELAY_MASK   0xF
> > +#defineCQSPI_REG_RD_DATA_CAPTURE   0x10
> > +#defineCQSPI_REG_RD_DATA_CAPTURE_BYPASSBIT(0)
> > +#defineCQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB 1
> > +#defineCQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK0xF
> >
> >  #defineCQSPI_REG_SIZE  0x14
> >  #defineCQSPI_REG_SIZE_ADDRESS_LSB  0
> > @@ -244,20 +244,20 @@ void cadence_qspi_apb_readdata_capture(void
> *reg_base,
> > unsigned int reg;
> > cadence_qspi_apb_controller_disable(reg_base);
> >
> > -   reg = readl(reg_base + CQSPI_READLCAPTURE);
> > +   reg = readl(reg_base + CQSPI_REG_RD_DATA_CAPTURE);
> >
> > if (bypass)
> > -   reg |= (1 << CQSPI_READLCAPTURE_BYPASS_LSB);
> > +   reg |= CQSPI_REG_RD_DATA_CAPTURE_BYPASS;
> > else
> > -   reg &= ~(1 << CQSPI_READLCAPTURE_BYPASS_LSB);
> > +   reg &= ~CQSPI_REG_RD_DATA_CAPTURE_BYPASS;
> >
> > -   reg &= ~(CQSPI_READLCAPTURE_DELAY_MASK
> > -   << CQSPI_READLCAPTURE_DELAY_LSB);
> > +   reg &= ~(CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK
> > +   << CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB);
> >
> > -   reg |= ((delay & CQSPI_READLCAPTURE_DELAY_MASK)
> > -   << CQSPI_READLCAPTURE_DELAY_LSB);
> > +   reg |= ((delay & CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK)
> > +   << CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB);
> 
> You can also drop the () around the whole expression here.
Ok, will do.

> >
> > -   writel(reg, reg_base + CQSPI_READLCAPTURE);
> > +   writel(reg, reg_base + CQSPI_REG_RD_DATA_CAPTURE);
> >
> > cadence_qspi_apb_controller_enable(reg_base);
> > return;
> > @@ -302,11 +302,12 @@ void cadence_qspi_apb_set_clk_mode(void
> *reg_base,
> >
> > cadence_qspi_apb_controller_disable(reg_base);
> > reg = readl(reg_base + CQSPI_REG_CONFIG);
> > -   reg &= ~(1 << CQSPI_REG_CONFIG_CLK_POL_LSB);
> > -   reg &= ~(1 << CQSPI_REG_CONFIG_CLK_PHA_LSB);
> > +   reg &= ~(CQSPI_REG_CONFIG_CLK_POL |
> CQSPI_REG_CONFIG_CLK_PHA);
> >
> > -   reg |= ((clk_pol & 0x1) << CQSPI_REG_CONFIG_CLK_POL_LSB);
> > -   reg |= ((clk_pha & 0x1) << CQSPI_REG_CONFIG_CLK_PHA_LSB);
> > +   if (clk_pol)
> > +   reg |= CQSPI_REG_CONFIG_CLK_POL;
> > +   if (clk_pha)
> > +   reg |= CQSPI_REG_CONFIG_CLK_PHA;
> 
> Except the minor nit above,
> 
> Acked-by: Marek Vasut <marek.va...@gmail.com>
> 
> > writel(reg, reg_base + CQSPI_REG_CONFIG);
> >
> >
> 
> 
> --
> Best regards,
> Marek Vasut

Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 2/8] spi: cadence_qspi: Fix baud rate calculation

[Phil Edworthy]

Hi Marek,

On 25 November 2016 14:58 Marek Vasut wrote:
> On 11/25/2016 03:38 PM, Phil Edworthy wrote:
> > With the existing code, when the requested SPI clock rate is near
> > to the lowest that can be achieved by the hardware (max divider
> > of the ref clock is 32), the generated clock rate is wrong.
> > For example, with a 50MHz ref clock, when asked for anything less
> > than a 1.5MHz SPI clock, the code sets up the divider to generate
> > 25MHz.
> >
> > This change fixes the calculation.
> >
> > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > ---
> > v2:
> >  - Use the DIV_ROUND_UP macro
> > ---
> >  drivers/spi/cadence_qspi_apb.c | 23 +++
> >  1 file changed, 7 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> > index 2403e71..b9e0df7 100644
> > --- a/drivers/spi/cadence_qspi_apb.c
> > +++ b/drivers/spi/cadence_qspi_apb.c
> > @@ -273,22 +273,13 @@ void cadence_qspi_apb_config_baudrate_div(void
> *reg_base,
> > reg = readl(reg_base + CQSPI_REG_CONFIG);
> > reg &= ~(CQSPI_REG_CONFIG_BAUD_MASK <<
> CQSPI_REG_CONFIG_BAUD_LSB);
> >
> > -   div = ref_clk_hz / sclk_hz;
> > -
> > -   if (div > 32)
> > -   div = 32;
> > -
> > -   /* Check if even number. */
> > -   if ((div & 1)) {
> > -   div = (div / 2);
> > -   } else {
> > -   if (ref_clk_hz % sclk_hz)
> > -   /* ensure generated SCLK doesn't exceed user
> > -   specified sclk_hz */
> > -   div = (div / 2);
> > -   else
> > -   div = (div / 2) - 1;
> > -   }
> > +   /*
> > +* The baud_div field in the config reg is 4 bits, and the ref clock is
> > +* divided by 2 * (baud_div + 1). Round up the divider to ensure the
> > +* SPI clock rate is less than or equal to the requested clock rate.
> > +*/
> > +   div = DIV_ROUND_UP(ref_clk_hz, sclk_hz);
> > +   div = DIV_ROUND_UP(div, 2) - 1;
> 
> Same as you did for u-boot, right ?
Eh? This is u-boot :)

> Acked-by: Marek Vasut <marek.va...@gmail.com>
> 
> > debug("%s: ref_clk %dHz sclk %dHz Div 0x%x\n", __func__,
> >   ref_clk_hz, sclk_hz, div);
> >
> 
> 
> --
> Best regards,
> Marek Vasut

Thanks
Phil

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 8/8] spi: cadence_qspi: Support specifying the sample edge used

[Phil Edworthy]

Whilst at it, move the code to read the "sram-size" property
into the other code that reads properties from the node, rather
than the SF subnode.

Also change the code to use a bool for the bypass arg.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>

---
v2:
 Change name of DT prop and provide details of what it does.
 Whilst at it, move the code to read the "sram-size" property
 into the other code that reads properties from the node, rather
 than the SF subnode.

 Also change the code to use a bool for the bypass arg.
---
 doc/device-tree-bindings/spi/spi-cadence.txt |  2 ++
 drivers/spi/cadence_qspi.c   | 13 +
 drivers/spi/cadence_qspi.h   |  3 ++-
 drivers/spi/cadence_qspi_apb.c   |  8 +++-
 4 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/doc/device-tree-bindings/spi/spi-cadence.txt 
b/doc/device-tree-bindings/spi/spi-cadence.txt
index c1e2233..94c800b 100644
--- a/doc/device-tree-bindings/spi/spi-cadence.txt
+++ b/doc/device-tree-bindings/spi/spi-cadence.txt
@@ -26,3 +26,5 @@ connected flash properties
  select (n_ss_out).
 - tslch-ns : Delay in master reference clocks between setting
  n_ss_out low and first bit transfer
+- sample-edge-rising   : Data outputs from flash memory will be sampled on the
+ rising edge. Default is falling edge.
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
index 1051afb..16fb18e 100644
--- a/drivers/spi/cadence_qspi.c
+++ b/drivers/spi/cadence_qspi.c
@@ -39,8 +39,10 @@ static int cadence_spi_write_speed(struct udevice *bus, uint 
hz)
 /* Calibration sequence to determine the read data capture delay register */
 static int spi_calibration(struct udevice *bus, uint hz)
 {
+   struct cadence_spi_platdata *plat = bus->platdata;
struct cadence_spi_priv *priv = dev_get_priv(bus);
void *base = priv->regbase;
+   bool edge = plat->sample_edge_rising;
u8 opcode_rdid = 0x9F;
unsigned int idcode = 0, temp = 0;
int err = 0, i, range_lo = -1, range_hi = -1;
@@ -49,7 +51,7 @@ static int spi_calibration(struct udevice *bus, uint hz)
cadence_spi_write_speed(bus, 100);
 
/* configure the read data capture delay register to 0 */
-   cadence_qspi_apb_readdata_capture(base, 1, 0);
+   cadence_qspi_apb_readdata_capture(base, true, edge, 0);
 
/* Enable QSPI */
cadence_qspi_apb_controller_enable(base);
@@ -69,7 +71,7 @@ static int spi_calibration(struct udevice *bus, uint hz)
cadence_qspi_apb_controller_disable(base);
 
/* reconfigure the read data capture delay register */
-   cadence_qspi_apb_readdata_capture(base, 1, i);
+   cadence_qspi_apb_readdata_capture(base, true, edge, i);
 
/* Enable back QSPI */
cadence_qspi_apb_controller_enable(base);
@@ -105,7 +107,8 @@ static int spi_calibration(struct udevice *bus, uint hz)
cadence_qspi_apb_controller_disable(base);
 
/* configure the final value for read data capture delay register */
-   cadence_qspi_apb_readdata_capture(base, 1, (range_hi + range_lo) / 2);
+   cadence_qspi_apb_readdata_capture(base, true, edge,
+   (range_hi + range_lo) / 2);
debug("SF: Read data capture delay calibrated to %i (%i - %i)\n",
  (range_hi + range_lo) / 2, range_lo, range_hi);
 
@@ -298,6 +301,7 @@ static int cadence_spi_ofdata_to_platdata(struct udevice 
*bus)
 
plat->regbase = (void *)data[0];
plat->ahbbase = (void *)data[2];
+   plat->sram_size = fdtdec_get_int(blob, node, "sram-size", 128);
 
/* All other paramters are embedded in the child node */
subnode = fdt_first_subnode(blob, node);
@@ -317,7 +321,8 @@ static int cadence_spi_ofdata_to_platdata(struct udevice 
*bus)
plat->tsd2d_ns = fdtdec_get_int(blob, subnode, "tsd2d-ns", 255);
plat->tchsh_ns = fdtdec_get_int(blob, subnode, "tchsh-ns", 20);
plat->tslch_ns = fdtdec_get_int(blob, subnode, "tslch-ns", 20);
-   plat->sram_size = fdtdec_get_int(blob, node, "sram-size", 128);
+   plat->sample_edge_rising = fdtdec_get_bool(blob, subnode,
+   "sample-edge-rising");
 
debug("%s: regbase=%p ahbbase=%p max-frequency=%d page-size=%d\n",
  __func__, plat->regbase, plat->ahbbase, plat->max_hz,
diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h
index a849f7b..ad1dbae 100644
--- a/drivers/spi/cadence_qspi.h
+++ b/drivers/spi/cadence_qspi.h
@@ -26,6 +26,7 @@ struct cadence_spi_platdata {
u32 tchsh_ns;
u32 tslch_ns;
u32 sram_size;
+   boolsample_edge_rising;

[U-Boot] [PATCH v2 7/8] spi: cadence_qspi: Fix CS timings

[Phil Edworthy]

The Cadence QSPI controller has specified overheads for the various CS
times that are in addition to those programmed in to the Device Delay
register. The overheads are different for the delays.

In addition, the existing code does not handle the case when the delay
is less than a SCLK period.

This change accurately calculates the additional delays in Ref clocks.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
v2:
 Was "spi: cadence_qspi: Fix CQSPI_CAL_DELAY calculation"
 Note only did the existing code not cope with the delay less than
 an SCLK period, but the calculation didn't round properly, and
 didn't take into account the delays that the QSPI Controller adds
 to those programmed into the Device Delay reg.
---
 drivers/spi/cadence_qspi_apb.c | 23 ---
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 1cd636a..56ad952 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -169,9 +169,6 @@
((readl(base + CQSPI_REG_CONFIG) >> \
CQSPI_REG_CONFIG_IDLE_LSB) & 0x1)
 
-#define CQSPI_CAL_DELAY(tdelay_ns, tref_ns, tsclk_ns)  \
-   tdelay_ns) - (tsclk_ns)) / (tref_ns)))
-
 #define CQSPI_GET_RD_SRAM_LEVEL(reg_base)  \
(((readl(reg_base + CQSPI_REG_SDRAMLEVEL)) >>   \
CQSPI_REG_SDRAMLEVEL_RD_LSB) & CQSPI_REG_SDRAMLEVEL_RD_MASK)
@@ -357,16 +354,20 @@ void cadence_qspi_apb_delay(void *reg_base,
cadence_qspi_apb_controller_disable(reg_base);
 
/* Convert to ns. */
-   ref_clk_ns = (10) / ref_clk;
+   ref_clk_ns = DIV_ROUND_UP(10, ref_clk);
 
/* Convert to ns. */
-   sclk_ns = (10) / sclk_hz;
-
-   /* Plus 1 to round up 1 clock cycle. */
-   tshsl = CQSPI_CAL_DELAY(tshsl_ns, ref_clk_ns, sclk_ns) + 1;
-   tchsh = CQSPI_CAL_DELAY(tchsh_ns, ref_clk_ns, sclk_ns) + 1;
-   tslch = CQSPI_CAL_DELAY(tslch_ns, ref_clk_ns, sclk_ns) + 1;
-   tsd2d = CQSPI_CAL_DELAY(tsd2d_ns, ref_clk_ns, sclk_ns) + 1;
+   sclk_ns = DIV_ROUND_UP(10, sclk_hz);
+
+   /* The controller adds additional delay to that programmed in the reg */
+   if (tshsl_ns >= sclk_ns + ref_clk_ns)
+   tshsl_ns -= sclk_ns + ref_clk_ns;
+   if (tchsh_ns >= sclk_ns + 3 * ref_clk_ns)
+   tchsh_ns -= sclk_ns + 3 * ref_clk_ns;
+   tshsl = DIV_ROUND_UP(tshsl_ns, ref_clk_ns);
+   tchsh = DIV_ROUND_UP(tchsh_ns, ref_clk_ns);
+   tslch = DIV_ROUND_UP(tslch_ns, ref_clk_ns);
+   tsd2d = DIV_ROUND_UP(tsd2d_ns, ref_clk_ns);
 
reg = ((tshsl & CQSPI_REG_DELAY_TSHSL_MASK)
<< CQSPI_REG_DELAY_TSHSL_LSB);
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 5/8] spi: cadence_qspi: Clean up the #define names

[Phil Edworthy]

A lot of the #defines are for single bits in a register, where the
name has _MASK on the end. Since this can be used for both a mask
and the value, remove _MASK from them.

Whilst doing so, also remove the unnecessary brackets around the
constants.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 drivers/spi/cadence_qspi_apb.c | 86 +-
 1 file changed, 43 insertions(+), 43 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index cd46a15..e7d8320 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -32,37 +32,37 @@
 #include 
 #include "cadence_qspi.h"
 
-#define CQSPI_REG_POLL_US  (1) /* 1us */
-#define CQSPI_REG_RETRY(1)
-#define CQSPI_POLL_IDLE_RETRY  (3)
+#define CQSPI_REG_POLL_US  1 /* 1us */
+#define CQSPI_REG_RETRY1
+#define CQSPI_POLL_IDLE_RETRY  3
 
-#define CQSPI_FIFO_WIDTH   (4)
+#define CQSPI_FIFO_WIDTH   4
 
-#define CQSPI_REG_SRAM_THRESHOLD_WORDS (50)
+#define CQSPI_REG_SRAM_THRESHOLD_WORDS 50
 
 /* Transfer mode */
-#define CQSPI_INST_TYPE_SINGLE (0)
-#define CQSPI_INST_TYPE_DUAL   (1)
-#define CQSPI_INST_TYPE_QUAD   (2)
+#define CQSPI_INST_TYPE_SINGLE 0
+#define CQSPI_INST_TYPE_DUAL   1
+#define CQSPI_INST_TYPE_QUAD   2
 
-#define CQSPI_STIG_DATA_LEN_MAX(8)
-
-#define CQSPI_DUMMY_CLKS_PER_BYTE  (8)
-#define CQSPI_DUMMY_BYTES_MAX  (4)
+#define CQSPI_STIG_DATA_LEN_MAX8
 
+#define CQSPI_DUMMY_CLKS_PER_BYTE  8
+#define CQSPI_DUMMY_BYTES_MAX  4
 
 #define CQSPI_REG_SRAM_FILL_THRESHOLD  \
((CQSPI_REG_SRAM_SIZE_WORD / 2) * CQSPI_FIFO_WIDTH)
+
 /
  * Controller's configuration and status register (offset from QSPI_BASE)
  /
 #defineCQSPI_REG_CONFIG0x00
-#defineCQSPI_REG_CONFIG_ENABLE_MASKBIT(0)
+#defineCQSPI_REG_CONFIG_ENABLE BIT(0)
 #defineCQSPI_REG_CONFIG_CLK_POLBIT(1)
 #defineCQSPI_REG_CONFIG_CLK_PHABIT(2)
-#defineCQSPI_REG_CONFIG_DIRECT_MASKBIT(7)
-#defineCQSPI_REG_CONFIG_DECODE_MASKBIT(9)
-#defineCQSPI_REG_CONFIG_XIP_IMM_MASK   BIT(18)
+#defineCQSPI_REG_CONFIG_DIRECT BIT(7)
+#defineCQSPI_REG_CONFIG_DECODE BIT(9)
+#defineCQSPI_REG_CONFIG_XIP_IMMBIT(18)
 #defineCQSPI_REG_CONFIG_CHIPSELECT_LSB 10
 #defineCQSPI_REG_CONFIG_BAUD_LSB   19
 #defineCQSPI_REG_CONFIG_IDLE_LSB   31
@@ -123,18 +123,18 @@
 #defineCQSPI_REG_IRQMASK   0x44
 
 #defineCQSPI_REG_INDIRECTRD0x60
-#defineCQSPI_REG_INDIRECTRD_START_MASK BIT(0)
-#defineCQSPI_REG_INDIRECTRD_CANCEL_MASKBIT(1)
-#defineCQSPI_REG_INDIRECTRD_INPROGRESS_MASKBIT(2)
-#defineCQSPI_REG_INDIRECTRD_DONE_MASK  BIT(5)
+#defineCQSPI_REG_INDIRECTRD_START  BIT(0)
+#defineCQSPI_REG_INDIRECTRD_CANCEL BIT(1)
+#defineCQSPI_REG_INDIRECTRD_INPROGRESS BIT(2)
+#defineCQSPI_REG_INDIRECTRD_DONE   BIT(5)
 
 #defineCQSPI_REG_INDIRECTRDWATERMARK   0x64
 #defineCQSPI_REG_INDIRECTRDSTARTADDR   0x68
 #defineCQSPI_REG_INDIRECTRDBYTES   0x6C
 
 #defineCQSPI_REG_CMDCTRL   0x90
-#defineCQSPI_REG_CMDCTRL_EXECUTE_MASK  BIT(0)
-#defineCQSPI_REG_CMDCTRL_INPROGRESS_MASK   BIT(1)
+#defineCQSPI_REG_CMDCTRL_EXECUTE   BIT(0)
+#defineCQSPI_REG_CMDCTRL_INPROGRESSBIT(1)
 #defineCQSPI_REG_CMDCTRL_DUMMY_LSB 7
 #defineCQSPI_REG_CMDCTRL_WR_BYTES_LSB  12
 #defineCQSPI_REG_CMDCTRL_WR_EN_LSB 15
@@ -150,10 +150,10 @@
 #defineCQSPI_REG_CMDCTRL_OPCODE_MASK   0xFF
 
 #defineCQSPI_REG_INDIRECTWR0x70
-#defineCQSPI_REG_INDIRECTWR_START_MASK BIT(0)
-#defineCQSPI_REG_INDIRECTWR_CANCEL_MASKBIT(1)
-#defineCQSPI_REG_INDIRECTWR_INPROGRESS_MASKBIT(2)
-#defineCQSPI_REG_INDIRECTWR_DONE_MASK  BIT(5)
+#defineCQSPI_REG_INDIRECTWR_START  BIT(0)
+#defineCQSPI_REG_INDIRECTWR_CANCEL

[U-Boot] [PATCH v2 6/8] spi: cadence_qspi: Remove returns from end of void functions

[Phil Edworthy]

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 drivers/spi/cadence_qspi_apb.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index e7d8320..1cd636a 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -199,7 +199,6 @@ void cadence_qspi_apb_controller_enable(void *reg_base)
reg = readl(reg_base + CQSPI_REG_CONFIG);
reg |= CQSPI_REG_CONFIG_ENABLE;
writel(reg, reg_base + CQSPI_REG_CONFIG);
-   return;
 }
 
 void cadence_qspi_apb_controller_disable(void *reg_base)
@@ -208,7 +207,6 @@ void cadence_qspi_apb_controller_disable(void *reg_base)
reg = readl(reg_base + CQSPI_REG_CONFIG);
reg &= ~CQSPI_REG_CONFIG_ENABLE;
writel(reg, reg_base + CQSPI_REG_CONFIG);
-   return;
 }
 
 /* Return 1 if idle, otherwise return 0 (busy). */
@@ -260,7 +258,6 @@ void cadence_qspi_apb_readdata_capture(void *reg_base,
writel(reg, reg_base + CQSPI_REG_RD_DATA_CAPTURE);
 
cadence_qspi_apb_controller_enable(reg_base);
-   return;
 }
 
 void cadence_qspi_apb_config_baudrate_div(void *reg_base,
@@ -292,7 +289,6 @@ void cadence_qspi_apb_config_baudrate_div(void *reg_base,
writel(reg, reg_base + CQSPI_REG_CONFIG);
 
cadence_qspi_apb_controller_enable(reg_base);
-   return;
 }
 
 void cadence_qspi_apb_set_clk_mode(void *reg_base,
@@ -312,7 +308,6 @@ void cadence_qspi_apb_set_clk_mode(void *reg_base,
writel(reg, reg_base + CQSPI_REG_CONFIG);
 
cadence_qspi_apb_controller_enable(reg_base);
-   return;
 }
 
 void cadence_qspi_apb_chipselect(void *reg_base,
@@ -347,7 +342,6 @@ void cadence_qspi_apb_chipselect(void *reg_base,
writel(reg, reg_base + CQSPI_REG_CONFIG);
 
cadence_qspi_apb_controller_enable(reg_base);
-   return;
 }
 
 void cadence_qspi_apb_delay(void *reg_base,
@@ -385,7 +379,6 @@ void cadence_qspi_apb_delay(void *reg_base,
writel(reg, reg_base + CQSPI_REG_DELAY);
 
cadence_qspi_apb_controller_enable(reg_base);
-   return;
 }
 
 void cadence_qspi_apb_controller_init(struct cadence_spi_platdata *plat)
@@ -413,7 +406,6 @@ void cadence_qspi_apb_controller_init(struct 
cadence_spi_platdata *plat)
writel(0, plat->regbase + CQSPI_REG_IRQMASK);
 
cadence_qspi_apb_controller_enable(plat->regbase);
-   return;
 }
 
 static int cadence_qspi_apb_exec_flash_cmd(void *reg_base,
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 2/8] spi: cadence_qspi: Fix baud rate calculation

[Phil Edworthy]

With the existing code, when the requested SPI clock rate is near
to the lowest that can be achieved by the hardware (max divider
of the ref clock is 32), the generated clock rate is wrong.
For example, with a 50MHz ref clock, when asked for anything less
than a 1.5MHz SPI clock, the code sets up the divider to generate
25MHz.

This change fixes the calculation.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
v2:
 - Use the DIV_ROUND_UP macro
---
 drivers/spi/cadence_qspi_apb.c | 23 +++
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 2403e71..b9e0df7 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -273,22 +273,13 @@ void cadence_qspi_apb_config_baudrate_div(void *reg_base,
reg = readl(reg_base + CQSPI_REG_CONFIG);
reg &= ~(CQSPI_REG_CONFIG_BAUD_MASK << CQSPI_REG_CONFIG_BAUD_LSB);
 
-   div = ref_clk_hz / sclk_hz;
-
-   if (div > 32)
-   div = 32;
-
-   /* Check if even number. */
-   if ((div & 1)) {
-   div = (div / 2);
-   } else {
-   if (ref_clk_hz % sclk_hz)
-   /* ensure generated SCLK doesn't exceed user
-   specified sclk_hz */
-   div = (div / 2);
-   else
-   div = (div / 2) - 1;
-   }
+   /*
+* The baud_div field in the config reg is 4 bits, and the ref clock is
+* divided by 2 * (baud_div + 1). Round up the divider to ensure the
+* SPI clock rate is less than or equal to the requested clock rate.
+*/
+   div = DIV_ROUND_UP(ref_clk_hz, sclk_hz);
+   div = DIV_ROUND_UP(div, 2) - 1;
 
debug("%s: ref_clk %dHz sclk %dHz Div 0x%x\n", __func__,
  ref_clk_hz, sclk_hz, div);
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 3/8] spi: cadence_qspi: Better debug information on the SPI clock rate

[Phil Edworthy]

Show what the output clock rate actually is.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 drivers/spi/cadence_qspi_apb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index b9e0df7..3ae4b5a 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -281,13 +281,13 @@ void cadence_qspi_apb_config_baudrate_div(void *reg_base,
div = DIV_ROUND_UP(ref_clk_hz, sclk_hz);
div = DIV_ROUND_UP(div, 2) - 1;
 
-   debug("%s: ref_clk %dHz sclk %dHz Div 0x%x\n", __func__,
- ref_clk_hz, sclk_hz, div);
-
/* ensure the baud rate doesn't exceed the max value */
if (div > CQSPI_REG_CONFIG_BAUD_MASK)
div = CQSPI_REG_CONFIG_BAUD_MASK;
 
+   debug("%s: ref_clk %dHz sclk %dHz Div 0x%x, actual %dHz\n", __func__,
+ ref_clk_hz, sclk_hz, div, ref_clk_hz / (2 * (div + 1)));
+
reg |= (div << CQSPI_REG_CONFIG_BAUD_LSB);
writel(reg, reg_base + CQSPI_REG_CONFIG);
 
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 4/8] spi: cadence_qspi: Use #define for bits instead of bit shifts

[Phil Edworthy]

Most of the code already uses #defines for the bit value, rather
than the shift required to get the value. This changes the remaining
code over.

Whislt at it, fix the names of the "Rd Data Capture" register defs.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 drivers/spi/cadence_qspi_apb.c | 37 +++--
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 3ae4b5a..cd46a15 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -57,9 +57,9 @@
  * Controller's configuration and status register (offset from QSPI_BASE)
  /
 #defineCQSPI_REG_CONFIG0x00
-#defineCQSPI_REG_CONFIG_CLK_POL_LSB1
-#defineCQSPI_REG_CONFIG_CLK_PHA_LSB2
 #defineCQSPI_REG_CONFIG_ENABLE_MASKBIT(0)
+#defineCQSPI_REG_CONFIG_CLK_POLBIT(1)
+#defineCQSPI_REG_CONFIG_CLK_PHABIT(2)
 #defineCQSPI_REG_CONFIG_DIRECT_MASKBIT(7)
 #defineCQSPI_REG_CONFIG_DECODE_MASKBIT(9)
 #defineCQSPI_REG_CONFIG_XIP_IMM_MASK   BIT(18)
@@ -94,10 +94,10 @@
 #defineCQSPI_REG_DELAY_TSD2D_MASK  0xFF
 #defineCQSPI_REG_DELAY_TSHSL_MASK  0xFF
 
-#defineCQSPI_READLCAPTURE  0x10
-#defineCQSPI_READLCAPTURE_BYPASS_LSB   0
-#defineCQSPI_READLCAPTURE_DELAY_LSB1
-#defineCQSPI_READLCAPTURE_DELAY_MASK   0xF
+#defineCQSPI_REG_RD_DATA_CAPTURE   0x10
+#defineCQSPI_REG_RD_DATA_CAPTURE_BYPASSBIT(0)
+#defineCQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB 1
+#defineCQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK0xF
 
 #defineCQSPI_REG_SIZE  0x14
 #defineCQSPI_REG_SIZE_ADDRESS_LSB  0
@@ -244,20 +244,20 @@ void cadence_qspi_apb_readdata_capture(void *reg_base,
unsigned int reg;
cadence_qspi_apb_controller_disable(reg_base);
 
-   reg = readl(reg_base + CQSPI_READLCAPTURE);
+   reg = readl(reg_base + CQSPI_REG_RD_DATA_CAPTURE);
 
if (bypass)
-   reg |= (1 << CQSPI_READLCAPTURE_BYPASS_LSB);
+   reg |= CQSPI_REG_RD_DATA_CAPTURE_BYPASS;
else
-   reg &= ~(1 << CQSPI_READLCAPTURE_BYPASS_LSB);
+   reg &= ~CQSPI_REG_RD_DATA_CAPTURE_BYPASS;
 
-   reg &= ~(CQSPI_READLCAPTURE_DELAY_MASK
-   << CQSPI_READLCAPTURE_DELAY_LSB);
+   reg &= ~(CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK
+   << CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB);
 
-   reg |= ((delay & CQSPI_READLCAPTURE_DELAY_MASK)
-   << CQSPI_READLCAPTURE_DELAY_LSB);
+   reg |= ((delay & CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK)
+   << CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB);
 
-   writel(reg, reg_base + CQSPI_READLCAPTURE);
+   writel(reg, reg_base + CQSPI_REG_RD_DATA_CAPTURE);
 
cadence_qspi_apb_controller_enable(reg_base);
return;
@@ -302,11 +302,12 @@ void cadence_qspi_apb_set_clk_mode(void *reg_base,
 
cadence_qspi_apb_controller_disable(reg_base);
reg = readl(reg_base + CQSPI_REG_CONFIG);
-   reg &= ~(1 << CQSPI_REG_CONFIG_CLK_POL_LSB);
-   reg &= ~(1 << CQSPI_REG_CONFIG_CLK_PHA_LSB);
+   reg &= ~(CQSPI_REG_CONFIG_CLK_POL | CQSPI_REG_CONFIG_CLK_PHA);
 
-   reg |= ((clk_pol & 0x1) << CQSPI_REG_CONFIG_CLK_POL_LSB);
-   reg |= ((clk_pha & 0x1) << CQSPI_REG_CONFIG_CLK_PHA_LSB);
+   if (clk_pol)
+   reg |= CQSPI_REG_CONFIG_CLK_POL;
+   if (clk_pha)
+   reg |= CQSPI_REG_CONFIG_CLK_PHA;
 
writel(reg, reg_base + CQSPI_REG_CONFIG);
 
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 1/8] spi: cadence_qspi: Fix clearing of pol/pha bits

[Phil Edworthy]

Or'ing together bit positions is clearly wrong.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 drivers/spi/cadence_qspi_apb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index e285d3c..2403e71 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -311,8 +311,8 @@ void cadence_qspi_apb_set_clk_mode(void *reg_base,
 
cadence_qspi_apb_controller_disable(reg_base);
reg = readl(reg_base + CQSPI_REG_CONFIG);
-   reg &= ~(1 <<
-   (CQSPI_REG_CONFIG_CLK_POL_LSB | CQSPI_REG_CONFIG_CLK_PHA_LSB));
+   reg &= ~(1 << CQSPI_REG_CONFIG_CLK_POL_LSB);
+   reg &= ~(1 << CQSPI_REG_CONFIG_CLK_PHA_LSB);
 
reg |= ((clk_pol & 0x1) << CQSPI_REG_CONFIG_CLK_POL_LSB);
reg |= ((clk_pha & 0x1) << CQSPI_REG_CONFIG_CLK_PHA_LSB);
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 0/8] SF: Cadence QSPI driver fixes and clean up

[Phil Edworthy]

This series has fixes, patches to clean the code up, and add support for
specifying the sampling edge.

Changed in v2:
  spi: cadence_qspi: Fix baud rate calculation
  spi: cadence_qspi: Fix CS timings (was "Fix CQSPI_CAL_DELAY calculation")
  spi: cadence_qspi: Support specifying the sample edge used

Added in v2:
  spi: cadence_qspi: Better debug information on the SPI clock rate

Phil Edworthy (8):
  spi: cadence_qspi: Fix clearing of pol/pha bits
  spi: cadence_qspi: Fix baud rate calculation
  spi: cadence_qspi: Better debug information on the SPI clock rate
  spi: cadence_qspi: Use #define for bits instead of bit shifts
  spi: cadence_qspi: Clean up the #define names
  spi: cadence_qspi: Remove returns from end of void functions
  spi: cadence_qspi: Fix CS timings
  spi: cadence_qspi: Support specifying the sample edge used

 doc/device-tree-bindings/spi/spi-cadence.txt |   2 +
 drivers/spi/cadence_qspi.c   |  13 +-
 drivers/spi/cadence_qspi.h   |   3 +-
 drivers/spi/cadence_qspi_apb.c   | 191 +--
 4 files changed, 104 insertions(+), 105 deletions(-)

-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 0/7] SF: Cadence QSPI driver fixes and clean up

[Phil Edworthy]

Hi Jagan,

On 02 November 2016 15:16, Phil wrote:
> This series has fixes, patches to clean the code up, and add support for
> specifying the sampling edge.
> 
> Phil Edworthy (7):
>   spi: cadence_qspi: Fix clearing of pol/pha bits
>   spi: cadence_qspi: Fix baud rate calculation
>   spi: cadence_qspi: Use #define for bits instead of bit shifts
>   spi: cadence_qspi: Clean up the #define names
>   spi: cadence_qspi: Remove returns from end of void functions
>   spi: cadence_qspi: Fix CQSPI_CAL_DELAY calculation
>   spi: cadence_qspi: Support specifying the sample edge used
> 
>  doc/device-tree-bindings/spi/spi-cadence.txt |   2 +
>  drivers/spi/cadence_qspi.c   |  10 +-
>  drivers/spi/cadence_qspi.h   |   3 +-
>  drivers/spi/cadence_qspi_apb.c   | 164 
> +--
>  4 files changed, 88 insertions(+), 91 deletions(-)

I've sent a couple of new versions of some of these patches based on feedback
and something I noticed, but most have not had any comments.
Would you like me to send the whole series again with the latest versions?

Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 6/7] spi: cadence_qspi: Fix CS timings

[Phil Edworthy]

The Cadence QSPI controller has specified overheads for the various CS
times that are in addition to those programmed in to the Device Delay
register. The overheads are different for the delays.

In addition, the existing code does not handle the case when the delay
is less than a SCLK period.

This change accurately calculates the additional delays in Ref clocks.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
v2:
 Was "spi: cadence_qspi: Fix CQSPI_CAL_DELAY calculation"
 Note only did the existing code not cope with the delay less than
 an SCLK period, but the calculation didn't round properly, and
 didn't take into account the delays that the QSPI Controller adds
 to those programmed into the Device Delay reg.
---
 drivers/spi/cadence_qspi_apb.c | 23 ---
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 72c7c79..4d53aad 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -169,9 +169,6 @@
((readl(base + CQSPI_REG_CONFIG) >> \
CQSPI_REG_CONFIG_IDLE_LSB) & 0x1)
 
-#define CQSPI_CAL_DELAY(tdelay_ns, tref_ns, tsclk_ns)  \
-   tdelay_ns) - (tsclk_ns)) / (tref_ns)))
-
 #define CQSPI_GET_RD_SRAM_LEVEL(reg_base)  \
(((readl(reg_base + CQSPI_REG_SDRAMLEVEL)) >>   \
CQSPI_REG_SDRAMLEVEL_RD_LSB) & CQSPI_REG_SDRAMLEVEL_RD_MASK)
@@ -357,16 +354,20 @@ void cadence_qspi_apb_delay(void *reg_base,
cadence_qspi_apb_controller_disable(reg_base);
 
/* Convert to ns. */
-   ref_clk_ns = (10) / ref_clk;
+   ref_clk_ns = DIV_ROUND_UP(10, ref_clk);
 
/* Convert to ns. */
-   sclk_ns = (10) / sclk_hz;
-
-   /* Plus 1 to round up 1 clock cycle. */
-   tshsl = CQSPI_CAL_DELAY(tshsl_ns, ref_clk_ns, sclk_ns) + 1;
-   tchsh = CQSPI_CAL_DELAY(tchsh_ns, ref_clk_ns, sclk_ns) + 1;
-   tslch = CQSPI_CAL_DELAY(tslch_ns, ref_clk_ns, sclk_ns) + 1;
-   tsd2d = CQSPI_CAL_DELAY(tsd2d_ns, ref_clk_ns, sclk_ns) + 1;
+   sclk_ns = DIV_ROUND_UP(10, sclk_hz);
+
+   /* The controller adds additional delay to that programmed in the reg */
+   if (tshsl_ns >= sclk_ns + ref_clk_ns)
+   tshsl_ns -= sclk_ns + ref_clk_ns;
+   if (tchsh_ns >= sclk_ns + 3 * ref_clk_ns)
+   tchsh_ns -= sclk_ns + 3 * ref_clk_ns;
+   tshsl = DIV_ROUND_UP(tshsl_ns, ref_clk_ns);
+   tchsh = DIV_ROUND_UP(tchsh_ns, ref_clk_ns);
+   tslch = DIV_ROUND_UP(tslch_ns, ref_clk_ns);
+   tsd2d = DIV_ROUND_UP(tsd2d_ns, ref_clk_ns);
 
reg = ((tshsl & CQSPI_REG_DELAY_TSHSL_MASK)
<< CQSPI_REG_DELAY_TSHSL_LSB);
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] dfu: dfu_sf: Fix read offset

[Phil Edworthy]

The offset was applied to write, but not read, now its applied to
both.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
---
 drivers/dfu/dfu_sf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dfu/dfu_sf.c b/drivers/dfu/dfu_sf.c
index 9702eee..b6d5fe2 100644
--- a/drivers/dfu/dfu_sf.c
+++ b/drivers/dfu/dfu_sf.c
@@ -20,7 +20,8 @@ static long dfu_get_medium_size_sf(struct dfu_entity *dfu)
 static int dfu_read_medium_sf(struct dfu_entity *dfu, u64 offset, void *buf,
long *len)
 {
-   return spi_flash_read(dfu->data.sf.dev, offset, *len, buf);
+   return spi_flash_read(dfu->data.sf.dev, dfu->data.sf.start + offset,
+   *len, buf);
 }
 
 static u64 find_sector(struct dfu_entity *dfu, u64 start, u64 offset)
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 ] spi: cadence_qspi: Support specifying the sample edge used

[Phil Edworthy]

Whilst at it, move the code to read the "sram-size" property
into the other code that reads properties from the node, rather
than the SF subnode.

Also change the code to use a bool for the bypass arg.

Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>

---
v2:
 Change name of DT prop and provide details of what it does.
 Whilst at it, move the code to read the "sram-size" property
 into the other code that reads properties from the node, rather
 than the SF subnode.

 Also changed the code to use a bool for the bypass arg.

 Note: I didn't explicitly state that the prop is optional as all of the props
 listed in that section are optional.
---
 doc/device-tree-bindings/spi/spi-cadence.txt |  2 ++
 drivers/spi/cadence_qspi.c   | 13 +
 drivers/spi/cadence_qspi.h   |  3 ++-
 drivers/spi/cadence_qspi_apb.c   |  8 +++-
 4 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/doc/device-tree-bindings/spi/spi-cadence.txt 
b/doc/device-tree-bindings/spi/spi-cadence.txt
index c1e2233..94c800b 100644
--- a/doc/device-tree-bindings/spi/spi-cadence.txt
+++ b/doc/device-tree-bindings/spi/spi-cadence.txt
@@ -26,3 +26,5 @@ connected flash properties
  select (n_ss_out).
 - tslch-ns : Delay in master reference clocks between setting
  n_ss_out low and first bit transfer
+- sample-edge-rising   : Data outputs from flash memory will be sampled on the
+ rising edge. Default is falling edge.
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
index 1051afb..16fb18e 100644
--- a/drivers/spi/cadence_qspi.c
+++ b/drivers/spi/cadence_qspi.c
@@ -39,8 +39,10 @@ static int cadence_spi_write_speed(struct udevice *bus, uint 
hz)
 /* Calibration sequence to determine the read data capture delay register */
 static int spi_calibration(struct udevice *bus, uint hz)
 {
+   struct cadence_spi_platdata *plat = bus->platdata;
struct cadence_spi_priv *priv = dev_get_priv(bus);
void *base = priv->regbase;
+   bool edge = plat->sample_edge_rising;
u8 opcode_rdid = 0x9F;
unsigned int idcode = 0, temp = 0;
int err = 0, i, range_lo = -1, range_hi = -1;
@@ -49,7 +51,7 @@ static int spi_calibration(struct udevice *bus, uint hz)
cadence_spi_write_speed(bus, 100);
 
/* configure the read data capture delay register to 0 */
-   cadence_qspi_apb_readdata_capture(base, 1, 0);
+   cadence_qspi_apb_readdata_capture(base, true, edge, 0);
 
/* Enable QSPI */
cadence_qspi_apb_controller_enable(base);
@@ -69,7 +71,7 @@ static int spi_calibration(struct udevice *bus, uint hz)
cadence_qspi_apb_controller_disable(base);
 
/* reconfigure the read data capture delay register */
-   cadence_qspi_apb_readdata_capture(base, 1, i);
+   cadence_qspi_apb_readdata_capture(base, true, edge, i);
 
/* Enable back QSPI */
cadence_qspi_apb_controller_enable(base);
@@ -105,7 +107,8 @@ static int spi_calibration(struct udevice *bus, uint hz)
cadence_qspi_apb_controller_disable(base);
 
/* configure the final value for read data capture delay register */
-   cadence_qspi_apb_readdata_capture(base, 1, (range_hi + range_lo) / 2);
+   cadence_qspi_apb_readdata_capture(base, true, edge,
+   (range_hi + range_lo) / 2);
debug("SF: Read data capture delay calibrated to %i (%i - %i)\n",
  (range_hi + range_lo) / 2, range_lo, range_hi);
 
@@ -298,6 +301,7 @@ static int cadence_spi_ofdata_to_platdata(struct udevice 
*bus)
 
plat->regbase = (void *)data[0];
plat->ahbbase = (void *)data[2];
+   plat->sram_size = fdtdec_get_int(blob, node, "sram-size", 128);
 
/* All other paramters are embedded in the child node */
subnode = fdt_first_subnode(blob, node);
@@ -317,7 +321,8 @@ static int cadence_spi_ofdata_to_platdata(struct udevice 
*bus)
plat->tsd2d_ns = fdtdec_get_int(blob, subnode, "tsd2d-ns", 255);
plat->tchsh_ns = fdtdec_get_int(blob, subnode, "tchsh-ns", 20);
plat->tslch_ns = fdtdec_get_int(blob, subnode, "tslch-ns", 20);
-   plat->sram_size = fdtdec_get_int(blob, node, "sram-size", 128);
+   plat->sample_edge_rising = fdtdec_get_bool(blob, subnode,
+   "sample-edge-rising");
 
debug("%s: regbase=%p ahbbase=%p max-frequency=%d page-size=%d\n",
  __func__, plat->regbase, plat->ahbbase, plat->max_hz,
diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h
index a849f7b..ad1dbae 100644
--- a/drivers/spi/cadence_qspi.h
+++ b/drivers/spi/cadence_qspi.h
@@ -26,6 +26,7 @@ struct cadence_spi_platdata {
u32 tchsh

Re: [U-Boot] [PATCH 7/7] spi: cadence_qspi: Support specifying the sample edge used

[Phil Edworthy]

Hi Vignesh,

On 04 November 2016 05:57, Vignesh R wrote:
> On Wednesday 02 November 2016 08:45 PM, Phil Edworthy wrote:
> > The HW manual does not give details about what the register
> > value for this bit actually does, other than "Choose edge on
> > which data outputs from flash memory will be sampled".
> >
> > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> >
> > ---
> > Our HW engineers tell me that it needs to be set for our hardware.
> > ---
> >  doc/device-tree-bindings/spi/spi-cadence.txt |  2 ++
> >  drivers/spi/cadence_qspi.c   | 10 +++---
> >  drivers/spi/cadence_qspi.h   |  3 ++-
> >  drivers/spi/cadence_qspi_apb.c   |  8 +++-
> >  4 files changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/doc/device-tree-bindings/spi/spi-cadence.txt b/doc/device-tree-
> bindings/spi/spi-cadence.txt
> > index c1e2233..71aa06a 100644
> > --- a/doc/device-tree-bindings/spi/spi-cadence.txt
> > +++ b/doc/device-tree-bindings/spi/spi-cadence.txt
> > @@ -26,3 +26,5 @@ connected flash properties
> >   select (n_ss_out).
> >  - tslch-ns : Delay in master reference clocks between setting
> >   n_ss_out low and first bit transfer
> > +- sample-edge  : The edge on which data outputs from flash
> memory will
> > + be sampled.
> 
> Could this be changed to bool property say sample-edge-rising?
> 
> sample-edge-rising(optional): data outputs from flash memory will
> be sampled at the rising edge. Default is falling edge
Whilst this makes sense, the trouble I have is that I don't know if the
register setting  is making it use rising or falling edge sampling. I'll try to
find out again...
 
> --
> Regards
> Vignesh

Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] gpio: dwapb: Add support for port B

[Phil Edworthy]

Hi Marek,

On 03 November 2016 17:49, Marek Vasut wrote:
> On 11/03/2016 02:56 PM, Simon Glass wrote:
> > On 2 November 2016 at 13:38, Marek Vasut <ma...@denx.de
> > <mailto:ma...@denx.de>> wrote:
> >> On 11/02/2016 04:24 PM, Phil Edworthy wrote:

> Please no c-structure register definitions, this does not scale :(
I think this is a long standing issue... For some IP blocks using a
C-structure for register definitions is painful. On the other hand,
for some others it may make the code more readable.

In this case, I would tend to agree with Simon. However, I'll happily
use whatever you two agree to.

Best regards
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] gpio: dwapb: Add support for port B

[Phil Edworthy]

Hi Simon,

On 03 November 2016 15:49, Simon Glass wrote:
> On 3 November 2016 at 08:02, Phil Edworthy <phil.edwor...@renesas.com>
> wrote:
> > >On 2 November 2016 at 13:38, Marek Vasut <ma...@denx.de> wrote:
> > >> On 11/02/2016 04:24 PM, Phil Edworthy wrote:
> > >>> The IP supports two ports, A and B, each providing up to 32 gpios.
> > >>> The driver already creates a 2nd gpio bank by reading the 2nd node
> > >>> from DT, so this is quite a simple change to support the 2nd bank.
> > >>>
> > >>> Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > >>> ---
> > >>> drivers/gpio/dwapb_gpio.c | 40
> +---
> > >>> 1 file changed, 33 insertions(+), 7 deletions(-)
> > >>>
> > >>> diff --git a/drivers/gpio/dwapb_gpio.c b/drivers/gpio/dwapb_gpio.c
> > >>> index 471e18a..dda0b42 100644
> > >>> --- a/drivers/gpio/dwapb_gpio.c
> > >>> +++ b/drivers/gpio/dwapb_gpio.c
> > >>> @@ -21,6 +21,8 @@ DECLARE_GLOBAL_DATA_PTR;
> > >>>
> > >>> #define GPIO_SWPORTA_DR 0x00
> > >>> #define GPIO_SWPORTA_DDR 0x04
> > >>> +#define GPIO_SWPORTB_DR 0x0C
> > >>> +#define GPIO_SWPORTB_DDR 0x10
> > >>> #define GPIO_INTEN 0x30
> > >>> #define GPIO_INTMASK 0x34
> > >>> #define GPIO_INTTYPE_LEVEL 0x38
> > >>> @@ -29,6 +31,7 @@ DECLARE_GLOBAL_DATA_PTR;
> > >>> #define GPIO_PORTA_DEBOUNCE 0x48
> > >>> #define GPIO_PORTA_EOI 0x4c
> > >>> #define GPIO_EXT_PORTA 0x50
> > >>> +#define GPIO_EXT_PORTB 0x54
> > >>>
> > >>> struct gpio_dwapb_platdata {
> > >>> const char *name;
> > >>> @@ -41,7 +44,11 @@ static int dwapb_gpio_direction_input(struct udevice
> *dev, unsigned pin)
> > >>> {
> > >>> struct gpio_dwapb_platdata *plat = dev_get_platdata(dev);
> > >>>
> > >>> - clrbits_le32(plat->base + GPIO_SWPORTA_DDR, 1 << pin);
> > >>> + if (plat->bank == 0)
> > >>> + clrbits_le32(plat->base + GPIO_SWPORTA_DDR, 1 << pin);
> > >>> + else
> > >>> + clrbits_le32(plat->base + GPIO_SWPORTB_DDR, 1 << pin);
> > >>
> > >> What about doing plat->base + plat->offset + GPIO_SWPORT_DDR ? Then
> you
> > >> don't need this if-else stuff.
> > >
> > >Yes. In fact this seems to be crying out to use a C structure instead.
> > >
> > >struct gpio_port {
> > >uint32_t dr;
> > >uint32_t ddr;
> > >};
> > >
> > >struct gpio_regs {
> > >struct gpio_port port[2];
> > >uint32_t reserved[8];
> > >uint32_t inten;
> > >...
> > >}
> > Unfortunately not because the registers for each port are spread over the
> place,
> > for example see the GPIO_EXT_PORTA/B registers.
> 
> What do you mean by 'spread all over the place'?
The registers for port A are 0x0, 0x4, 0x50, whereas port B are 0xc, 0x10, 0x54,
so you can't use a 'struct gpio_port' simply in the way you have suggested. Of
course it can be done, just not so cleanly.

> > After Marek's feedback, I think we now have a reasonably clean solution.
> 
> Well I don't like it much, but it's up to you.
Ok, I'll change it.

> Regards,
> Simon
Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] gpio: dwapb: Add support for port B

[Phil Edworthy]

Hi Simon,

>On 2 November 2016 at 13:38, Marek Vasut <ma...@denx.de> wrote:
>> On 11/02/2016 04:24 PM, Phil Edworthy wrote:
>>> The IP supports two ports, A and B, each providing up to 32 gpios.
>>> The driver already creates a 2nd gpio bank by reading the 2nd node
>>> from DT, so this is quite a simple change to support the 2nd bank.
>>>
>>> Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
>>> ---
>>> drivers/gpio/dwapb_gpio.c | 40 +---
>>> 1 file changed, 33 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpio/dwapb_gpio.c b/drivers/gpio/dwapb_gpio.c
>>> index 471e18a..dda0b42 100644
>>> --- a/drivers/gpio/dwapb_gpio.c
>>> +++ b/drivers/gpio/dwapb_gpio.c
>>> @@ -21,6 +21,8 @@ DECLARE_GLOBAL_DATA_PTR;
>>>
>>> #define GPIO_SWPORTA_DR 0x00
>>> #define GPIO_SWPORTA_DDR 0x04
>>> +#define GPIO_SWPORTB_DR 0x0C
>>> +#define GPIO_SWPORTB_DDR 0x10
>>> #define GPIO_INTEN 0x30
>>> #define GPIO_INTMASK 0x34
>>> #define GPIO_INTTYPE_LEVEL 0x38
>>> @@ -29,6 +31,7 @@ DECLARE_GLOBAL_DATA_PTR;
>>> #define GPIO_PORTA_DEBOUNCE 0x48
>>> #define GPIO_PORTA_EOI 0x4c
>>> #define GPIO_EXT_PORTA 0x50
>>> +#define GPIO_EXT_PORTB 0x54
>>>
>>> struct gpio_dwapb_platdata {
>>> const char *name;
>>> @@ -41,7 +44,11 @@ static int dwapb_gpio_direction_input(struct udevice 
>>> *dev, unsigned pin)
>>> {
>>> struct gpio_dwapb_platdata *plat = dev_get_platdata(dev);
>>>
>>> - clrbits_le32(plat->base + GPIO_SWPORTA_DDR, 1 << pin);
>>> + if (plat->bank == 0)
>>> + clrbits_le32(plat->base + GPIO_SWPORTA_DDR, 1 << pin);
>>> + else
>>> + clrbits_le32(plat->base + GPIO_SWPORTB_DDR, 1 << pin);
>>
>> What about doing plat->base + plat->offset + GPIO_SWPORT_DDR ? Then you
>> don't need this if-else stuff.
>
>Yes. In fact this seems to be crying out to use a C structure instead.
>
>struct gpio_port {
>uint32_t dr;
>uint32_t ddr;
>};
>
>struct gpio_regs {
>struct gpio_port port[2];
>uint32_t reserved[8];
>uint32_t inten;
>...
>}
Unfortunately not because the registers for each port are spread over the place,
for example see the GPIO_EXT_PORTA/B registers.

After Marek's feedback, I think we now have a reasonably clean solution.

>Regards,
>Simon

Thanks
Phil
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot