[U-Boot] [PATCH v2 0/6] PPC 85xx: Add support for QEMU's ppce500 PV machine

[U-Boot] [PATCH v2 0/6] PPC 85xx: Add support for QEMU's ppce500 PV machine

[Alexander Graf]

In QEMU we implement a PV machine type called "ppce500". That board is able
to run any e500+ FSL cores (e500v2, e500mc, e5500, e6500).

It is heavily inspired by the MPC8544DS SoC and board combination, but
implements only the bare minimum to make Linux happy enough to drive a
virtual machine.

This patch set implements support for this PV machine type in U-Boot, enabling
users to run their virtual machines with netboot, u-boot payload binaries or
other fun things they come up with.

---

v1 -> v2:

  - Write device tree offset directly into global variable
  - use r4 rather than r2 for that
  - access fdt directly from in-memory copy
  - remove unneeded header includes
  - clean up pci enumeration
  - coding style fixes
  - populate and only use fdt_addr_r
  - remove unused exported functions
  - remove unused TLB0 entries
  - make TLB1 I/O maps non-executable
  - remove unused defines in board header
  - make -kernel boot variables more clear
  - remove TLB0 invalidation
  - use tlb1.14 for temporary as=1 map
  - use CONFIG_SYS_MPC85XX_NO_RESETVEC
  - store fdt pointer in gd through cpu_init_early_f()
  - replace fixup_tlb1() with dynamic TLB creation hook
  - find CCSRBAR from device tree
  - find PCI controllers from device tree
  - find CPU speed from device tree

Alexander Graf (6):
  PPC 85xx: Detect e500v2 / e500mc during runtime
  PPC 85xx: Add ELF entry point
  PPC 85xx: Add qemu-ppce500 machine
  PPC 85xx: Find CCSRBAR on ppce500 from device tree
  PPC 85xx: Find PCI host controllers on ppce500 from device tree
  PPC 85xx: Find CPU speed on ppce500 from device tree

 arch/powerpc/cpu/mpc85xx/Makefile           |    2 +
 arch/powerpc/cpu/mpc85xx/cpu_init_early.c   |   10 +-
 arch/powerpc/cpu/mpc85xx/fixed_ivor.S       |   21 +-
 arch/powerpc/cpu/mpc85xx/start.S            |   13 +
 arch/powerpc/cpu/mpc85xx/tlb.c              |    4 +
 arch/powerpc/cpu/mpc85xx/u-boot.lds         |    1 +
 arch/powerpc/include/asm/config_mpc85xx.h   |    4 +
 board/freescale/qemu-ppce500/Makefile       |   10 +
 board/freescale/qemu-ppce500/qemu-ppce500.c |  452 +++++++++++++++++++++++++++
 board/freescale/qemu-ppce500/tlb.c          |   16 +
 boards.cfg                                  |    1 +
 include/configs/qemu-ppce500.h              |  207 ++++++++++++
 12 files changed, 735 insertions(+), 6 deletions(-)
 create mode 100644 board/freescale/qemu-ppce500/Makefile
 create mode 100644 board/freescale/qemu-ppce500/qemu-ppce500.c
 create mode 100644 board/freescale/qemu-ppce500/tlb.c
 create mode 100644 include/configs/qemu-ppce500.h

-- 
1.7.10.4

[U-Boot] [PATCH v2 1/6] PPC 85xx: Detect e500v2 / e500mc during runtime

[Alexander Graf]

With the qemu-ppce500 machine type we can run the same board with
either an e500v2 or an e500mc core plugged in.

This means that the IVOR setup can't be based on compile time decisions,
so instead we have to do a runtime check which CPU generation we're
running on.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/powerpc/cpu/mpc85xx/fixed_ivor.S |   21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/cpu/mpc85xx/fixed_ivor.S b/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
index ebbb8c0..635a97e 100644
--- a/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
+++ b/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
@@ -36,17 +36,25 @@
 	SET_IVOR(14, 0x1e0) /* Instruction TLB Error */
 	SET_IVOR(15, 0x040) /* Debug */
 
-/* e500v1 & e500v2 only */
-#ifndef CONFIG_E500MC
+	/* Check for CPU */
+	mfpvr	r3
+	srwi	r3, r3, 16
+	/* Compare with e500mc PVR major number */
+	li	r4, 0
+	ori	r4, r4, 0x8023
+	cmpw	r3, r4
+
+	/* e500v1 & e500v2 only */
+	bge	1f
 	SET_IVOR(32, 0x200) /* SPE Unavailable */
 	SET_IVOR(33, 0x220) /* Embedded FP Data */
 	SET_IVOR(34, 0x240) /* Embedded FP Round */
-#endif
+1:
 
 	SET_IVOR(35, 0x260) /* Performance monitor */
 
-/* e500mc only */
-#ifdef CONFIG_E500MC
+	/* e500mc only */
+	blt	2f
 	SET_IVOR(36, 0x280) /* Processor doorbell */
 	SET_IVOR(37, 0x2a0) /* Processor doorbell critical */
 	SET_IVOR(38, 0x2c0) /* Guest Processor doorbell */
@@ -54,6 +62,8 @@
 	SET_IVOR(40, 0x300) /* Hypervisor system call */
 	SET_IVOR(41, 0x320) /* Hypervisor Priviledge */
 
+#ifndef CONFIG_QEMU_E500
+	/* QEMU guests runs in guest mode and can't access GIVORs */
 	SET_GIVOR(2, 0x060) /* Guest Data Storage */
 	SET_GIVOR(3, 0x080) /* Guest Instruction Storage */
 	SET_GIVOR(4, 0x0a0) /* Guest External Input */
@@ -61,3 +71,4 @@
 	SET_GIVOR(13, 0x1c0) /* Guest Data TLB Error */
 	SET_GIVOR(14, 0x1e0) /* Guest Instruction TLB Error */
 #endif
+2:
-- 
1.7.10.4

[U-Boot] [PATCH v2 1/6] PPC 85xx: Detect e500v2 / e500mc during runtime

[York Sun]

On 01/31/2014 03:16 AM, Alexander Graf wrote:
> With the qemu-ppce500 machine type we can run the same board with
> either an e500v2 or an e500mc core plugged in.
> 
> This means that the IVOR setup can't be based on compile time decisions,
> so instead we have to do a runtime check which CPU generation we're
> running on.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/powerpc/cpu/mpc85xx/fixed_ivor.S |   21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/cpu/mpc85xx/fixed_ivor.S b/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
> index ebbb8c0..635a97e 100644
> --- a/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
> +++ b/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
> @@ -36,17 +36,25 @@
>  	SET_IVOR(14, 0x1e0) /* Instruction TLB Error */
>  	SET_IVOR(15, 0x040) /* Debug */
>  
> -/* e500v1 & e500v2 only */
> -#ifndef CONFIG_E500MC
> +	/* Check for CPU */
> +	mfpvr	r3
> +	srwi	r3, r3, 16
> +	/* Compare with e500mc PVR major number */
> +	li	r4, 0
> +	ori	r4, r4, 0x8023
> +	cmpw	r3, r4
> +
> +	/* e500v1 & e500v2 only */
> +	bge	1f
>  	SET_IVOR(32, 0x200) /* SPE Unavailable */
>  	SET_IVOR(33, 0x220) /* Embedded FP Data */
>  	SET_IVOR(34, 0x240) /* Embedded FP Round */
> -#endif
> +1:
>  
>  	SET_IVOR(35, 0x260) /* Performance monitor */
>  
> -/* e500mc only */
> -#ifdef CONFIG_E500MC
> +	/* e500mc only */
> +	blt	2f

This "blt" has a risk. Please put a comment warning developers who will modify
the code to be sure the flag has not been updated since last "cmpw".

York

[U-Boot] [PATCH v2 1/6] PPC 85xx: Detect e500v2 / e500mc during runtime

[Scott Wood]

On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
> With the qemu-ppce500 machine type we can run the same board with
> either an e500v2 or an e500mc core plugged in.
> 
> This means that the IVOR setup can't be based on compile time decisions,
> so instead we have to do a runtime check which CPU generation we're
> running on.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/powerpc/cpu/mpc85xx/fixed_ivor.S |   21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/cpu/mpc85xx/fixed_ivor.S b/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
> index ebbb8c0..635a97e 100644
> --- a/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
> +++ b/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
> @@ -36,17 +36,25 @@
>  	SET_IVOR(14, 0x1e0) /* Instruction TLB Error */
>  	SET_IVOR(15, 0x040) /* Debug */
>  
> -/* e500v1 & e500v2 only */
> -#ifndef CONFIG_E500MC
> +	/* Check for CPU */
> +	mfpvr	r3
> +	srwi	r3, r3, 16
> +	/* Compare with e500mc PVR major number */
> +	li	r4, 0
> +	ori	r4, r4, 0x8023
> +	cmpw	r3, r4
> +
> +	/* e500v1 & e500v2 only */
> +	bge	1f
>  	SET_IVOR(32, 0x200) /* SPE Unavailable */
>  	SET_IVOR(33, 0x220) /* Embedded FP Data */
>  	SET_IVOR(34, 0x240) /* Embedded FP Round */
> -#endif
> +1:
>  
>  	SET_IVOR(35, 0x260) /* Performance monitor */
>  
> -/* e500mc only */
> -#ifdef CONFIG_E500MC
> +	/* e500mc only */
> +	blt	2f
>  	SET_IVOR(36, 0x280) /* Processor doorbell */
>  	SET_IVOR(37, 0x2a0) /* Processor doorbell critical */
>  	SET_IVOR(38, 0x2c0) /* Guest Processor doorbell */
> @@ -54,6 +62,8 @@
>  	SET_IVOR(40, 0x300) /* Hypervisor system call */
>  	SET_IVOR(41, 0x320) /* Hypervisor Priviledge */
>  
> +#ifndef CONFIG_QEMU_E500
> +	/* QEMU guests runs in guest mode and can't access GIVORs */
>  	SET_GIVOR(2, 0x060) /* Guest Data Storage */
>  	SET_GIVOR(3, 0x080) /* Guest Instruction Storage */
>  	SET_GIVOR(4, 0x0a0) /* Guest External Input */
> @@ -61,3 +71,4 @@
>  	SET_GIVOR(13, 0x1c0) /* Guest Data TLB Error */
>  	SET_GIVOR(14, 0x1e0) /* Guest Instruction TLB Error */
>  #endif
> +2:

Again, let's please just remove this entire file.

Besides, who's to say that (non-KVM) QEMU doesn't someday start
emulating GIVORs? :-)

-Scott

[U-Boot] [PATCH v2 1/6] PPC 85xx: Detect e500v2 / e500mc during runtime

[Alexander Graf]

On 04.02.2014, at 02:59, Scott Wood <scottwood@freescale.com> wrote:

> On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
>> With the qemu-ppce500 machine type we can run the same board with
>> either an e500v2 or an e500mc core plugged in.
>> 
>> This means that the IVOR setup can't be based on compile time decisions,
>> so instead we have to do a runtime check which CPU generation we're
>> running on.
>> 
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> arch/powerpc/cpu/mpc85xx/fixed_ivor.S |   21 ++++++++++++++++-----
>> 1 file changed, 16 insertions(+), 5 deletions(-)
>> 
>> diff --git a/arch/powerpc/cpu/mpc85xx/fixed_ivor.S b/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
>> index ebbb8c0..635a97e 100644
>> --- a/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
>> +++ b/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
>> @@ -36,17 +36,25 @@
>> 	SET_IVOR(14, 0x1e0) /* Instruction TLB Error */
>> 	SET_IVOR(15, 0x040) /* Debug */
>> 
>> -/* e500v1 & e500v2 only */
>> -#ifndef CONFIG_E500MC
>> +	/* Check for CPU */
>> +	mfpvr	r3
>> +	srwi	r3, r3, 16
>> +	/* Compare with e500mc PVR major number */
>> +	li	r4, 0
>> +	ori	r4, r4, 0x8023
>> +	cmpw	r3, r4
>> +
>> +	/* e500v1 & e500v2 only */
>> +	bge	1f
>> 	SET_IVOR(32, 0x200) /* SPE Unavailable */
>> 	SET_IVOR(33, 0x220) /* Embedded FP Data */
>> 	SET_IVOR(34, 0x240) /* Embedded FP Round */
>> -#endif
>> +1:
>> 
>> 	SET_IVOR(35, 0x260) /* Performance monitor */
>> 
>> -/* e500mc only */
>> -#ifdef CONFIG_E500MC
>> +	/* e500mc only */
>> +	blt	2f
>> 	SET_IVOR(36, 0x280) /* Processor doorbell */
>> 	SET_IVOR(37, 0x2a0) /* Processor doorbell critical */
>> 	SET_IVOR(38, 0x2c0) /* Guest Processor doorbell */
>> @@ -54,6 +62,8 @@
>> 	SET_IVOR(40, 0x300) /* Hypervisor system call */
>> 	SET_IVOR(41, 0x320) /* Hypervisor Priviledge */
>> 
>> +#ifndef CONFIG_QEMU_E500
>> +	/* QEMU guests runs in guest mode and can't access GIVORs */
>> 	SET_GIVOR(2, 0x060) /* Guest Data Storage */
>> 	SET_GIVOR(3, 0x080) /* Guest Instruction Storage */
>> 	SET_GIVOR(4, 0x0a0) /* Guest External Input */
>> @@ -61,3 +71,4 @@
>> 	SET_GIVOR(13, 0x1c0) /* Guest Data TLB Error */
>> 	SET_GIVOR(14, 0x1e0) /* Guest Instruction TLB Error */
>> #endif
>> +2:
> 
> Again, let's please just remove this entire file.

I can remove it, but it's a pretty drastic change from the original behavior that was introduced 4 1/2 years ago:

  http://lists.denx.de/pipermail/u-boot/2009-August/058670.html

I assume the idea was to give OSs one thing less to worry about (IVOR setting). If any OS in between 2009 and now relied on that fact, we'll break it by removing this code.


Alex

[U-Boot] [PATCH v2 1/6] PPC 85xx: Detect e500v2 / e500mc during runtime

[Scott Wood]

On Thu, 2014-02-06 at 12:40 +0100, Alexander Graf wrote:
> On 04.02.2014, at 02:59, Scott Wood <scottwood@freescale.com> wrote:
> 
> > On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
> >> With the qemu-ppce500 machine type we can run the same board with
> >> either an e500v2 or an e500mc core plugged in.
> >> 
> >> This means that the IVOR setup can't be based on compile time decisions,
> >> so instead we have to do a runtime check which CPU generation we're
> >> running on.
> >> 
> >> Signed-off-by: Alexander Graf <agraf@suse.de>
> >> ---
> >> arch/powerpc/cpu/mpc85xx/fixed_ivor.S |   21 ++++++++++++++++-----
> >> 1 file changed, 16 insertions(+), 5 deletions(-)
> >> 
> >> diff --git a/arch/powerpc/cpu/mpc85xx/fixed_ivor.S b/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
> >> index ebbb8c0..635a97e 100644
> >> --- a/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
> >> +++ b/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
> >> @@ -36,17 +36,25 @@
> >> 	SET_IVOR(14, 0x1e0) /* Instruction TLB Error */
> >> 	SET_IVOR(15, 0x040) /* Debug */
> >> 
> >> -/* e500v1 & e500v2 only */
> >> -#ifndef CONFIG_E500MC
> >> +	/* Check for CPU */
> >> +	mfpvr	r3
> >> +	srwi	r3, r3, 16
> >> +	/* Compare with e500mc PVR major number */
> >> +	li	r4, 0
> >> +	ori	r4, r4, 0x8023
> >> +	cmpw	r3, r4
> >> +
> >> +	/* e500v1 & e500v2 only */
> >> +	bge	1f
> >> 	SET_IVOR(32, 0x200) /* SPE Unavailable */
> >> 	SET_IVOR(33, 0x220) /* Embedded FP Data */
> >> 	SET_IVOR(34, 0x240) /* Embedded FP Round */
> >> -#endif
> >> +1:
> >> 
> >> 	SET_IVOR(35, 0x260) /* Performance monitor */
> >> 
> >> -/* e500mc only */
> >> -#ifdef CONFIG_E500MC
> >> +	/* e500mc only */
> >> +	blt	2f
> >> 	SET_IVOR(36, 0x280) /* Processor doorbell */
> >> 	SET_IVOR(37, 0x2a0) /* Processor doorbell critical */
> >> 	SET_IVOR(38, 0x2c0) /* Guest Processor doorbell */
> >> @@ -54,6 +62,8 @@
> >> 	SET_IVOR(40, 0x300) /* Hypervisor system call */
> >> 	SET_IVOR(41, 0x320) /* Hypervisor Priviledge */
> >> 
> >> +#ifndef CONFIG_QEMU_E500
> >> +	/* QEMU guests runs in guest mode and can't access GIVORs */
> >> 	SET_GIVOR(2, 0x060) /* Guest Data Storage */
> >> 	SET_GIVOR(3, 0x080) /* Guest Instruction Storage */
> >> 	SET_GIVOR(4, 0x0a0) /* Guest External Input */
> >> @@ -61,3 +71,4 @@
> >> 	SET_GIVOR(13, 0x1c0) /* Guest Data TLB Error */
> >> 	SET_GIVOR(14, 0x1e0) /* Guest Instruction TLB Error */
> >> #endif
> >> +2:
> > 
> > Again, let's please just remove this entire file.
> 
> I can remove it, but it's a pretty drastic change from the original
> behavior that was introduced 4 1/2 years ago:
> 
>   http://lists.denx.de/pipermail/u-boot/2009-August/058670.html
> 
> I assume the idea was to give OSs one thing less to worry about (IVOR
> setting). If any OS in between 2009 and now relied on that fact, we'll
> break it by removing this code.

Any such OS would already be broken on pre-2009 U-Boot.  Linux doesn't
rely on it.  Neither the code nor the commit message gives a sufficient
rationale, and Kumar didn't answer when asked.  It's incomplete (doesn't
include e6500 IVORs).  It doesn't work on e6500 secondary threads.  The
reset value of these registers is documented as zero, so it's not like
we're just putting things back the way they were.  It only happens
(except on secondary cores) when the bootm/bootz commands are used, not
when using the ELF loader or the go command, nor when using bootm with
IH_TYPE_STANDALONE.

Does anyone know of an OS that actually depends on this?

-Scott

[U-Boot] [PATCH v2 1/6] PPC 85xx: Detect e500v2 / e500mc during runtime

[Alexander Graf]

On 07.02.2014, at 01:51, Scott Wood <scottwood@freescale.com> wrote:

> On Thu, 2014-02-06 at 12:40 +0100, Alexander Graf wrote:
>> On 04.02.2014, at 02:59, Scott Wood <scottwood@freescale.com> wrote:
>> 
>>> On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
>>>> With the qemu-ppce500 machine type we can run the same board with
>>>> either an e500v2 or an e500mc core plugged in.
>>>> 
>>>> This means that the IVOR setup can't be based on compile time decisions,
>>>> so instead we have to do a runtime check which CPU generation we're
>>>> running on.
>>>> 
>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>> ---
>>>> arch/powerpc/cpu/mpc85xx/fixed_ivor.S |   21 ++++++++++++++++-----
>>>> 1 file changed, 16 insertions(+), 5 deletions(-)
>>>> 
>>>> diff --git a/arch/powerpc/cpu/mpc85xx/fixed_ivor.S b/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
>>>> index ebbb8c0..635a97e 100644
>>>> --- a/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
>>>> +++ b/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
>>>> @@ -36,17 +36,25 @@
>>>> 	SET_IVOR(14, 0x1e0) /* Instruction TLB Error */
>>>> 	SET_IVOR(15, 0x040) /* Debug */
>>>> 
>>>> -/* e500v1 & e500v2 only */
>>>> -#ifndef CONFIG_E500MC
>>>> +	/* Check for CPU */
>>>> +	mfpvr	r3
>>>> +	srwi	r3, r3, 16
>>>> +	/* Compare with e500mc PVR major number */
>>>> +	li	r4, 0
>>>> +	ori	r4, r4, 0x8023
>>>> +	cmpw	r3, r4
>>>> +
>>>> +	/* e500v1 & e500v2 only */
>>>> +	bge	1f
>>>> 	SET_IVOR(32, 0x200) /* SPE Unavailable */
>>>> 	SET_IVOR(33, 0x220) /* Embedded FP Data */
>>>> 	SET_IVOR(34, 0x240) /* Embedded FP Round */
>>>> -#endif
>>>> +1:
>>>> 
>>>> 	SET_IVOR(35, 0x260) /* Performance monitor */
>>>> 
>>>> -/* e500mc only */
>>>> -#ifdef CONFIG_E500MC
>>>> +	/* e500mc only */
>>>> +	blt	2f
>>>> 	SET_IVOR(36, 0x280) /* Processor doorbell */
>>>> 	SET_IVOR(37, 0x2a0) /* Processor doorbell critical */
>>>> 	SET_IVOR(38, 0x2c0) /* Guest Processor doorbell */
>>>> @@ -54,6 +62,8 @@
>>>> 	SET_IVOR(40, 0x300) /* Hypervisor system call */
>>>> 	SET_IVOR(41, 0x320) /* Hypervisor Priviledge */
>>>> 
>>>> +#ifndef CONFIG_QEMU_E500
>>>> +	/* QEMU guests runs in guest mode and can't access GIVORs */
>>>> 	SET_GIVOR(2, 0x060) /* Guest Data Storage */
>>>> 	SET_GIVOR(3, 0x080) /* Guest Instruction Storage */
>>>> 	SET_GIVOR(4, 0x0a0) /* Guest External Input */
>>>> @@ -61,3 +71,4 @@
>>>> 	SET_GIVOR(13, 0x1c0) /* Guest Data TLB Error */
>>>> 	SET_GIVOR(14, 0x1e0) /* Guest Instruction TLB Error */
>>>> #endif
>>>> +2:
>>> 
>>> Again, let's please just remove this entire file.
>> 
>> I can remove it, but it's a pretty drastic change from the original
>> behavior that was introduced 4 1/2 years ago:
>> 
>>  http://lists.denx.de/pipermail/u-boot/2009-August/058670.html
>> 
>> I assume the idea was to give OSs one thing less to worry about (IVOR
>> setting). If any OS in between 2009 and now relied on that fact, we'll
>> break it by removing this code.
> 
> Any such OS would already be broken on pre-2009 U-Boot.  Linux doesn't
> rely on it.  Neither the code nor the commit message gives a sufficient
> rationale, and Kumar didn't answer when asked.  It's incomplete (doesn't
> include e6500 IVORs).  It doesn't work on e6500 secondary threads.  The

I thought e6500 has hard coded IVORs which is the whole point of this exercise? Or am I wrong there?


Alex

> reset value of these registers is documented as zero, so it's not like
> we're just putting things back the way they were.  It only happens
> (except on secondary cores) when the bootm/bootz commands are used, not
> when using the ELF loader or the go command, nor when using bootm with
> IH_TYPE_STANDALONE.
> 
> Does anyone know of an OS that actually depends on this?
> 
> -Scott
> 
> 

[U-Boot] [PATCH v2 1/6] PPC 85xx: Detect e500v2 / e500mc during runtime

[Scott Wood]

On Fri, 2014-02-07 at 12:52 +0100, Alexander Graf wrote:
> On 07.02.2014, at 01:51, Scott Wood <scottwood@freescale.com> wrote:
> 
> > On Thu, 2014-02-06 at 12:40 +0100, Alexander Graf wrote:
> >> On 04.02.2014, at 02:59, Scott Wood <scottwood@freescale.com> wrote:
> >> 
> >>> On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
> >>>> With the qemu-ppce500 machine type we can run the same board with
> >>>> either an e500v2 or an e500mc core plugged in.
> >>>> 
> >>>> This means that the IVOR setup can't be based on compile time decisions,
> >>>> so instead we have to do a runtime check which CPU generation we're
> >>>> running on.
> >>>> 
> >>>> Signed-off-by: Alexander Graf <agraf@suse.de>
> >>>> ---
> >>>> arch/powerpc/cpu/mpc85xx/fixed_ivor.S |   21 ++++++++++++++++-----
> >>>> 1 file changed, 16 insertions(+), 5 deletions(-)
> >>>> 
> >>>> diff --git a/arch/powerpc/cpu/mpc85xx/fixed_ivor.S b/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
> >>>> index ebbb8c0..635a97e 100644
> >>>> --- a/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
> >>>> +++ b/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
> >>>> @@ -36,17 +36,25 @@
> >>>> 	SET_IVOR(14, 0x1e0) /* Instruction TLB Error */
> >>>> 	SET_IVOR(15, 0x040) /* Debug */
> >>>> 
> >>>> -/* e500v1 & e500v2 only */
> >>>> -#ifndef CONFIG_E500MC
> >>>> +	/* Check for CPU */
> >>>> +	mfpvr	r3
> >>>> +	srwi	r3, r3, 16
> >>>> +	/* Compare with e500mc PVR major number */
> >>>> +	li	r4, 0
> >>>> +	ori	r4, r4, 0x8023
> >>>> +	cmpw	r3, r4
> >>>> +
> >>>> +	/* e500v1 & e500v2 only */
> >>>> +	bge	1f
> >>>> 	SET_IVOR(32, 0x200) /* SPE Unavailable */
> >>>> 	SET_IVOR(33, 0x220) /* Embedded FP Data */
> >>>> 	SET_IVOR(34, 0x240) /* Embedded FP Round */
> >>>> -#endif
> >>>> +1:
> >>>> 
> >>>> 	SET_IVOR(35, 0x260) /* Performance monitor */
> >>>> 
> >>>> -/* e500mc only */
> >>>> -#ifdef CONFIG_E500MC
> >>>> +	/* e500mc only */
> >>>> +	blt	2f
> >>>> 	SET_IVOR(36, 0x280) /* Processor doorbell */
> >>>> 	SET_IVOR(37, 0x2a0) /* Processor doorbell critical */
> >>>> 	SET_IVOR(38, 0x2c0) /* Guest Processor doorbell */
> >>>> @@ -54,6 +62,8 @@
> >>>> 	SET_IVOR(40, 0x300) /* Hypervisor system call */
> >>>> 	SET_IVOR(41, 0x320) /* Hypervisor Priviledge */
> >>>> 
> >>>> +#ifndef CONFIG_QEMU_E500
> >>>> +	/* QEMU guests runs in guest mode and can't access GIVORs */
> >>>> 	SET_GIVOR(2, 0x060) /* Guest Data Storage */
> >>>> 	SET_GIVOR(3, 0x080) /* Guest Instruction Storage */
> >>>> 	SET_GIVOR(4, 0x0a0) /* Guest External Input */
> >>>> @@ -61,3 +71,4 @@
> >>>> 	SET_GIVOR(13, 0x1c0) /* Guest Data TLB Error */
> >>>> 	SET_GIVOR(14, 0x1e0) /* Guest Instruction TLB Error */
> >>>> #endif
> >>>> +2:
> >>> 
> >>> Again, let's please just remove this entire file.
> >> 
> >> I can remove it, but it's a pretty drastic change from the original
> >> behavior that was introduced 4 1/2 years ago:
> >> 
> >>  http://lists.denx.de/pipermail/u-boot/2009-August/058670.html
> >> 
> >> I assume the idea was to give OSs one thing less to worry about (IVOR
> >> setting). If any OS in between 2009 and now relied on that fact, we'll
> >> break it by removing this code.
> > 
> > Any such OS would already be broken on pre-2009 U-Boot.  Linux doesn't
> > rely on it.  Neither the code nor the commit message gives a sufficient
> > rationale, and Kumar didn't answer when asked.  It's incomplete (doesn't
> > include e6500 IVORs).  It doesn't work on e6500 secondary threads.  The
> 
> I thought e6500 has hard coded IVORs which is the whole point of this exercise? Or am I wrong there?

e6500 does not have fixed interrupt vector offsets.  I think IBM book3e
chips do, and IVORs are phased out in the architecture.

-Scott

[U-Boot] [PATCH v2 2/6] PPC 85xx: Add ELF entry point

[Alexander Graf]

We want to be able to directly execute the ELF binary without going
through the u-boot.bin one.

To know where we have to start executing this ELF binary  we have to
tell the linker where our entry point is.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/powerpc/cpu/mpc85xx/u-boot.lds |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/cpu/mpc85xx/u-boot.lds b/arch/powerpc/cpu/mpc85xx/u-boot.lds
index 2af4c80..b34d212 100644
--- a/arch/powerpc/cpu/mpc85xx/u-boot.lds
+++ b/arch/powerpc/cpu/mpc85xx/u-boot.lds
@@ -13,6 +13,7 @@
 #endif
 
 OUTPUT_ARCH(powerpc)
+ENTRY(_start_e500)
 
 PHDRS
 {
-- 
1.7.10.4

[U-Boot] [PATCH v2 3/6] PPC 85xx: Add qemu-ppce500 machine

[Alexander Graf]

For KVM we have a special PV machine type called "ppce500". This machine
is inspired by the MPC8544DS board, but implements a lot less features
than that one.

It also provides more PCI slots and is supposed to be enumerated by
device tree only.

This patch adds support for the current generation ppce500 machine as
it is implemented today.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - Write device tree offset directly into global variable
  - use r4 rather than r2 for that
  - access fdt directly from in-memory copy
  - remove unneeded header includes
  - clean up pci enumeration
  - coding style fixes
  - populate and only use fdt_addr_r
  - remove unused exported functions
  - remove unused TLB0 entries
  - make TLB1 I/O maps non-executable
  - remove unused defines in board header
  - make -kernel boot variables more clear
  - remove TLB0 invalidation
  - use tlb1.14 for temporary as=1 map
  - use CONFIG_SYS_MPC85XX_NO_RESETVEC
  - store fdt pointer in gd through cpu_init_early_f()
  - replace fixup_tlb1() with dynamic TLB creation hook
---
 arch/powerpc/cpu/mpc85xx/cpu_init_early.c   |   10 +-
 arch/powerpc/cpu/mpc85xx/start.S            |   11 ++
 arch/powerpc/cpu/mpc85xx/tlb.c              |    4 +
 arch/powerpc/include/asm/config_mpc85xx.h   |    4 +
 board/freescale/qemu-ppce500/Makefile       |   10 ++
 board/freescale/qemu-ppce500/qemu-ppce500.c |  169 ++++++++++++++++++++++
 board/freescale/qemu-ppce500/tlb.c          |   29 ++++
 boards.cfg                                  |    1 +
 include/configs/qemu-ppce500.h              |  209 +++++++++++++++++++++++++++
 9 files changed, 446 insertions(+), 1 deletion(-)
 create mode 100644 board/freescale/qemu-ppce500/Makefile
 create mode 100644 board/freescale/qemu-ppce500/qemu-ppce500.c
 create mode 100644 board/freescale/qemu-ppce500/tlb.c
 create mode 100644 include/configs/qemu-ppce500.h

diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init_early.c b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
index 993b8b8..b0ec9ea 100644
--- a/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
+++ b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
@@ -79,7 +79,7 @@ void setup_ifc(void)
 #endif
 
 /* We run cpu_init_early_f in AS = 1 */
-void cpu_init_early_f(void)
+void cpu_init_early_f(void *fdt)
 {
 	u32 mas0, mas1, mas2, mas3, mas7;
 	int i;
@@ -102,6 +102,14 @@ void cpu_init_early_f(void)
 	for (i = 0; i < sizeof(gd_t); i++)
 		((char *)gd)[i] = 0;
 
+#ifdef CONFIG_QEMU_E500
+	/*
+	 * CONFIG_SYS_CCSRBAR_PHYS below will use gd->fdt_blob, so we need to
+	 * populate it before it accesses it.
+	 */
+	gd->fdt_blob = fdt;
+#endif
+
 	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(13);
 	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TS | MAS1_TSIZE(BOOKE_PAGESZ_1M);
 	mas2 = FSL_BOOKE_MAS2(CONFIG_SYS_CCSRBAR, MAS2_I|MAS2_G);
diff --git a/arch/powerpc/cpu/mpc85xx/start.S b/arch/powerpc/cpu/mpc85xx/start.S
index db84d10..0e593d2 100644
--- a/arch/powerpc/cpu/mpc85xx/start.S
+++ b/arch/powerpc/cpu/mpc85xx/start.S
@@ -80,6 +80,11 @@ _start_e500:
 	li	r1,MSR_DE
 	mtmsr 	r1
 
+#ifdef CONFIG_QEMU_E500
+	/* Save our ePAPR device tree pointer before we clobber it */
+	mr	r24, r3
+#endif
+
 #ifdef CONFIG_SYS_FSL_ERRATUM_A004510
 	mfspr	r3,SPRN_SVR
 	rlwinm	r3,r3,0,0xff
@@ -1144,6 +1149,12 @@ _start_cont:
 	mr	r1,r3		/* Transfer to SP(r1) */
 
 	GET_GOT
+
+#ifdef CONFIG_QEMU_E500
+	/* Pass the ePAPR device tree pointer to cpu_init_early_f */
+	mr	r3, r24
+#endif
+
 	bl	cpu_init_early_f
 
 	/* switch back to AS = 0 */
diff --git a/arch/powerpc/cpu/mpc85xx/tlb.c b/arch/powerpc/cpu/mpc85xx/tlb.c
index 8748ecd..c899d9e 100644
--- a/arch/powerpc/cpu/mpc85xx/tlb.c
+++ b/arch/powerpc/cpu/mpc85xx/tlb.c
@@ -36,6 +36,10 @@ void init_tlbs(void)
 			  tlb_table[i].mas7);
 	}
 
+#ifdef CONFIG_USE_DYNAMIC_TLBS
+	init_tlbs_dynamic();
+#endif
+
 	return ;
 }
 
diff --git a/arch/powerpc/include/asm/config_mpc85xx.h b/arch/powerpc/include/asm/config_mpc85xx.h
index 54ce2f0..a0ab453 100644
--- a/arch/powerpc/include/asm/config_mpc85xx.h
+++ b/arch/powerpc/include/asm/config_mpc85xx.h
@@ -776,6 +776,10 @@ defined(CONFIG_PPC_T1020) || defined(CONFIG_PPC_T1022)
 #define CONFIG_SYS_CCSRBAR_DEFAULT	0xff700000
 #define CONFIG_SYS_FSL_ERRATUM_A005125
 
+#elif defined(CONFIG_QEMU_E500)
+#define CONFIG_MAX_CPUS			1
+#define CONFIG_SYS_CCSRBAR_DEFAULT	0xe0000000
+
 #else
 #error Processor type not defined for this platform
 #endif
diff --git a/board/freescale/qemu-ppce500/Makefile b/board/freescale/qemu-ppce500/Makefile
new file mode 100644
index 0000000..193b160
--- /dev/null
+++ b/board/freescale/qemu-ppce500/Makefile
@@ -0,0 +1,10 @@
+#
+# Copyright 2007 Freescale Semiconductor, Inc.
+# (C) Copyright 2001-2006
+# Wolfgang Denk, DENX Software Engineering, wd at denx.de.
+#
+# SPDX-License-Identifier:	GPL-2.0+
+#
+
+obj-y	+= qemu-ppce500.o
+obj-y	+= tlb.o
diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c b/board/freescale/qemu-ppce500/qemu-ppce500.c
new file mode 100644
index 0000000..b33b6b1
--- /dev/null
+++ b/board/freescale/qemu-ppce500/qemu-ppce500.c
@@ -0,0 +1,169 @@
+/*
+ * Copyright 2007,2009-2014 Freescale Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <command.h>
+#include <pci.h>
+#include <asm/processor.h>
+#include <asm/mmu.h>
+#include <asm/fsl_pci.h>
+#include <asm/io.h>
+#include <libfdt.h>
+#include <fdt_support.h>
+#include <netdev.h>
+#include <fdtdec.h>
+#include <errno.h>
+#include <malloc.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+static struct pci_controller pci1_hose;
+
+static const void *get_fdt(void)
+{
+	return gd->fdt_blob;
+}
+
+int checkboard(void)
+{
+	return 0;
+}
+
+void pci_init_board(void)
+{
+	struct fsl_pci_info pci_info;
+	const void *fdt = get_fdt();
+	int pci_node;
+
+	puts("\n");
+
+	pci_node = fdt_path_offset(fdt, "/pci");
+	if (pci_node < 0) {
+		printf("PCI: disabled\n\n");
+		return;
+	}
+
+	SET_STD_PCI_INFO(pci_info, 1);
+
+	fsl_setup_hose(&pci1_hose, pci_info.regs);
+	printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
+		pci_info.regs);
+
+	fsl_pci_init_port(&pci_info, &pci1_hose, 0);
+
+	puts("\n");
+}
+
+int last_stage_init(void)
+{
+	const void *fdt = get_fdt();
+	int len = 0;
+	const uint64_t *prop;
+	int chosen;
+
+	chosen = fdt_path_offset(fdt, "/chosen");
+	if (chosen < 0) {
+		printf("Couldn't find /chosen node in fdt\n");
+		return -EIO;
+	}
+
+	/* -kernel boot */
+	prop = fdt_getprop(fdt, chosen, "qemu,boot-kernel", &len);
+	if (prop && (len >= 8))
+		setenv_hex("qemu_kernel_addr", *prop);
+
+	/* Give the user a variable for the host fdt */
+	setenv_hex("fdt_addr_r", (ulong)fdt);
+
+	return 0;
+}
+
+static uint64_t get_linear_ram_size(void)
+{
+	const void *fdt = get_fdt();
+	const void *prop;
+	int memory;
+	int len;
+
+	memory = fdt_path_offset(fdt, "/memory");
+	prop = fdt_getprop(fdt, memory, "reg", &len);
+
+	if (prop && len >= 16)
+		return *(uint64_t *)(prop+8);
+
+	panic("Couldn't determine RAM size");
+}
+
+int board_eth_init(bd_t *bis)
+{
+	return pci_eth_init(bis);
+}
+
+#if defined(CONFIG_OF_BOARD_SETUP)
+void ft_board_setup(void *blob, bd_t *bd)
+{
+	FT_FSL_PCI_SETUP;
+}
+#endif
+
+void print_laws(void)
+{
+	/* We don't emulate LAWs yet */
+}
+
+phys_size_t fixed_sdram(void)
+{
+	return get_linear_ram_size();
+}
+
+phys_size_t fsl_ddr_sdram_size(void)
+{
+	return get_linear_ram_size();
+}
+
+void init_tlbs_dynamic(void)
+{
+	unsigned long fdt_tlb = (unsigned long)get_fdt() & ~0xffffful;
+	u32 mas0, mas1, mas2, mas3, mas7;
+	phys_size_t ram_size;
+
+	/*
+	 * Create a temporary AS=1 map for the fdt
+	 *
+	 * We use ESEL=0 here to overwrite the previous AS=0 map for ourselves
+	 * which was only 4k big. This way we don't have to clear any other maps.
+	 */
+	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(0);
+	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TS | MAS1_TSIZE(BOOKE_PAGESZ_1M);
+	mas2 = FSL_BOOKE_MAS2(fdt_tlb, 0);
+	mas3 = FSL_BOOKE_MAS3(fdt_tlb, 0, MAS3_SW|MAS3_SR|MAS3_SX);
+	mas7 = FSL_BOOKE_MAS7(fdt_tlb);
+
+	write_tlb(mas0, mas1, mas2, mas3, mas7);
+
+	/* Fetch RAM size from the fdt */
+	ram_size = fixed_sdram();
+
+	/* And remove our fdt map again */
+	disable_tlb(0);
+
+	/* Create a dynamic AS=0 CCSRBAR mapping */
+	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(13);
+	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TSIZE(BOOKE_PAGESZ_1M);
+	mas2 = FSL_BOOKE_MAS2(CONFIG_SYS_CCSRBAR, MAS2_I|MAS2_G);
+	mas3 = FSL_BOOKE_MAS3(CONFIG_SYS_CCSRBAR_PHYS, 0, MAS3_SW|MAS3_SR);
+	mas7 = FSL_BOOKE_MAS7(CONFIG_SYS_CCSRBAR_PHYS);
+
+	write_tlb(mas0, mas1, mas2, mas3, mas7);
+
+	/* Create a RAM map that spans all accessible RAM */
+	init_used_tlb_cams();
+	setup_ddr_tlbs(ram_size >> 20);
+}
+
+void init_laws(void)
+{
+	/* We don't emulate LAWs yet */
+}
diff --git a/board/freescale/qemu-ppce500/tlb.c b/board/freescale/qemu-ppce500/tlb.c
new file mode 100644
index 0000000..a600296
--- /dev/null
+++ b/board/freescale/qemu-ppce500/tlb.c
@@ -0,0 +1,29 @@
+/*
+ * Copyright 2008 Freescale Semiconductor, Inc.
+ *
+ * (C) Copyright 2000
+ * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <asm/mmu.h>
+
+struct fsl_e_tlb_entry tlb_table[] = {
+	/*
+	 * TLB 1:	256M	Non-cacheable, guarded
+	 */
+	SET_TLB_ENTRY(1, CONFIG_SYS_PCI_VIRT, CONFIG_SYS_PCI_PHYS,
+		      MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
+		      0, 1, BOOKE_PAGESZ_256M, 1),
+
+	/*
+	 * TLB 2:	256M	Non-cacheable, guarded
+	 */
+	SET_TLB_ENTRY(1, CONFIG_SYS_PCI_VIRT + 0x10000000, CONFIG_SYS_PCI_PHYS + 0x10000000,
+		      MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
+		      0, 2, BOOKE_PAGESZ_256M, 1),
+};
+
+int num_tlb_entries = ARRAY_SIZE(tlb_table);
diff --git a/boards.cfg b/boards.cfg
index d177f82..ab50982 100644
--- a/boards.cfg
+++ b/boards.cfg
@@ -986,6 +986,7 @@ Active  powerpc     mpc85xx        -           freescale       t2080qds
 Active  powerpc     mpc85xx        -           freescale       t2080qds            T2080QDS_SPIFLASH     T2080QDS:PPC_T2080,RAMBOOT_PBL,SPIFLASH,SYS_TEXT_BASE=0xFFF80000
 Active  powerpc     mpc85xx        -           freescale       t2080qds            T2080QDS_NAND         T2080QDS:PPC_T2080,RAMBOOT_PBL,NAND,SYS_TEXT_BASE=0xFFF80000
 Active  powerpc     mpc85xx        -           freescale       t2080qds            T2080QDS_SRIO_PCIE_BOOT  T2080QDS:PPC_T2080,SRIO_PCIE_BOOT_SLAVE,SYS_TEXT_BASE=0xFFF80000
+Active  powerpc     mpc85xx        -           freescale       qemu-ppce500        qemu-ppce500                         -                                                                                                                                 Alexander Graf <agraf@suse.de>
 Active  powerpc     mpc85xx        -           gdsys           p1022               controlcenterd_36BIT_SDCARD          controlcenterd:36BIT,SDCARD                                                                                                       Dirk Eibach <eibach@gdsys.de>
 Active  powerpc     mpc85xx        -           gdsys           p1022               controlcenterd_36BIT_SDCARD_DEVELOP  controlcenterd:36BIT,SDCARD,DEVELOP                                                                                               Dirk Eibach <eibach@gdsys.de>
 Active  powerpc     mpc85xx        -           gdsys           p1022               controlcenterd_TRAILBLAZER           controlcenterd:TRAILBLAZER,SPIFLASH                                                                                               Dirk Eibach <eibach@gdsys.de>
diff --git a/include/configs/qemu-ppce500.h b/include/configs/qemu-ppce500.h
new file mode 100644
index 0000000..1ebaf51
--- /dev/null
+++ b/include/configs/qemu-ppce500.h
@@ -0,0 +1,209 @@
+/*
+ * Copyright 2011-2014 Freescale Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+/*
+ * Corenet DS style board configuration file
+ */
+#ifndef __QEMU_PPCE500_H
+#define __QEMU_PPCE500_H
+
+#define CONFIG_CMD_REGINFO
+
+/* High Level Configuration Options */
+#define CONFIG_BOOKE
+#define CONFIG_E500			/* BOOKE e500 family */
+#define CONFIG_MPC85xx			/* MPC85xx/PQ3 platform */
+#define CONFIG_QEMU_E500
+
+#undef CONFIG_SYS_TEXT_BASE
+#define CONFIG_SYS_TEXT_BASE	0xf01000 /* 15 MB */
+
+#define CONFIG_SYS_MPC85XX_NO_RESETVEC
+
+#define CONFIG_SYS_RAMBOOT
+
+#define CONFIG_PCI			/* Enable PCI/PCIE */
+#define CONFIG_PCI1		1	/* PCI controller 1 */
+#define CONFIG_FSL_PCI_INIT		/* Use common FSL init code */
+#define CONFIG_SYS_PCI_64BIT		/* enable 64-bit PCI resources */
+
+#define CONFIG_ENV_OVERWRITE
+
+#define CONFIG_ENABLE_36BIT_PHYS
+
+#define CONFIG_ADDR_MAP
+#define CONFIG_SYS_NUM_ADDR_MAP		16	/* number of TLB1 entries */
+
+#define CONFIG_USE_DYNAMIC_TLBS
+
+#define CONFIG_SYS_MEMTEST_START	0x00200000	/* memtest works on */
+#define CONFIG_SYS_MEMTEST_END		0x00400000
+#define CONFIG_SYS_ALT_MEMTEST
+#define CONFIG_PANIC_HANG	/* do not reset board on panic */
+
+#define CONFIG_SYS_CCSRBAR		0xe0000000
+#define CONFIG_SYS_CCSRBAR_PHYS_LOW	CONFIG_SYS_CCSRBAR
+
+/*
+ * DDR Setup
+ */
+#define CONFIG_VERY_BIG_RAM
+#define CONFIG_SYS_DDR_SDRAM_BASE	0x00000000
+#define CONFIG_SYS_SDRAM_BASE		CONFIG_SYS_DDR_SDRAM_BASE
+
+#define CONFIG_CHIP_SELECTS_PER_CTRL	0
+
+/* Get RAM size from device tree */
+#define CONFIG_DDR_SPD
+
+#define CONFIG_SYS_CLK_FREQ        33000000
+
+#define CONFIG_SYS_NO_FLASH
+
+#define CONFIG_SYS_BOOT_BLOCK		0x00000000	/* boot TLB */
+
+#define CONFIG_SYS_MONITOR_BASE		CONFIG_SYS_TEXT_BASE
+
+#define CONFIG_ENV_IS_NOWHERE
+
+#define CONFIG_HWCONFIG
+
+#define CONFIG_SYS_INIT_RAM_ADDR		0x00100000
+#define CONFIG_SYS_INIT_RAM_ADDR_PHYS_HIGH	0x0
+#define CONFIG_SYS_INIT_RAM_ADDR_PHYS_LOW	0x00100000
+/* The assembler doesn't like typecast */
+#define CONFIG_SYS_INIT_RAM_ADDR_PHYS \
+	((CONFIG_SYS_INIT_RAM_ADDR_PHYS_HIGH * 1ull << 32) | \
+	  CONFIG_SYS_INIT_RAM_ADDR_PHYS_LOW)
+#define CONFIG_SYS_INIT_RAM_SIZE		0x00004000
+
+#define CONFIG_SYS_GBL_DATA_OFFSET	(CONFIG_SYS_INIT_RAM_SIZE - \
+					GENERATED_GBL_DATA_SIZE)
+#define CONFIG_SYS_INIT_SP_OFFSET	CONFIG_SYS_GBL_DATA_OFFSET
+
+#define CONFIG_SYS_MONITOR_LEN		(512 * 1024)
+#define CONFIG_SYS_MALLOC_LEN		(4 * 1024 * 1024)
+
+#define CONFIG_CONS_INDEX	1
+#define CONFIG_SYS_NS16550
+#define CONFIG_SYS_NS16550_SERIAL
+#define CONFIG_SYS_NS16550_REG_SIZE	1
+#define CONFIG_SYS_NS16550_CLK		(get_bus_freq(0))
+
+#define CONFIG_SYS_BAUDRATE_TABLE	\
+	{300, 600, 1200, 2400, 4800, 9600, 19200, 38400, 57600, 115200}
+
+#define CONFIG_SYS_NS16550_COM1	(CONFIG_SYS_CCSRBAR+0x4500)
+#define CONFIG_SYS_NS16550_COM2	(CONFIG_SYS_CCSRBAR+0x4600)
+
+/* Use the HUSH parser */
+#define CONFIG_SYS_HUSH_PARSER
+#define CONFIG_SYS_PROMPT_HUSH_PS2 "> "
+
+/* pass open firmware flat tree */
+#define CONFIG_OF_LIBFDT
+#define CONFIG_OF_BOARD_SETUP
+#define CONFIG_OF_STDOUT_VIA_ALIAS
+
+/* new uImage format support */
+#define CONFIG_FIT
+#define CONFIG_FIT_VERBOSE	/* enable fit_format_{error,warning}() */
+
+/*
+ * General PCI
+ * Memory space is mapped 1-1, but I/O space must start from 0.
+ */
+
+#define CONFIG_SYS_PCI_VIRT		0xc0000000	/* 512M PCI TLB */
+#define CONFIG_SYS_PCI_PHYS		0xc0000000	/* 512M PCI TLB */
+
+#define CONFIG_SYS_PCI1_MEM_VIRT	0xc0000000
+#define CONFIG_SYS_PCI1_MEM_BUS	0xc0000000
+#define CONFIG_SYS_PCI1_MEM_PHYS	0xc0000000
+#define CONFIG_SYS_PCI1_MEM_SIZE	0x20000000	/* 512M */
+#define CONFIG_SYS_PCI1_IO_VIRT	0xe1000000
+#define CONFIG_SYS_PCI1_IO_BUS	0x00000000
+#define CONFIG_SYS_PCI1_IO_PHYS	0xe1000000
+#define CONFIG_SYS_PCI1_IO_SIZE	0x00010000	/* 64k */
+
+#ifdef CONFIG_PCI
+#define CONFIG_PCI_INDIRECT_BRIDGE
+#define CONFIG_NET_MULTI
+#define CONFIG_PCI_PNP			/* do pci plug-and-play */
+#define CONFIG_E1000
+
+#define CONFIG_PCI_SCAN_SHOW		/* show pci devices on startup */
+#define CONFIG_DOS_PARTITION
+#endif	/* CONFIG_PCI */
+
+#define CONFIG_LBA48
+#define CONFIG_DOS_PARTITION
+#define CONFIG_CMD_EXT2
+
+/*
+ * Environment
+ */
+#define CONFIG_ENV_SIZE		0x2000
+
+#define CONFIG_LOADS_ECHO		/* echo on for serial download */
+
+#define CONFIG_LAST_STAGE_INIT
+
+/*
+ * Command line configuration.
+ */
+#include <config_cmd_default.h>
+
+#define CONFIG_CMD_DHCP
+#define CONFIG_CMD_ELF
+#define CONFIG_CMD_BOOTZ
+#define CONFIG_CMD_GREPENV
+#define CONFIG_CMD_IRQ
+#define CONFIG_CMD_PING
+#define CONFIG_CMD_SETEXPR
+
+#ifdef CONFIG_PCI
+#define CONFIG_CMD_PCI
+#define CONFIG_CMD_NET
+#endif
+
+/*
+ * Miscellaneous configurable options
+ */
+#define CONFIG_SYS_LONGHELP			/* undef to save memory	*/
+#define CONFIG_CMDLINE_EDITING			/* Command-line editing */
+#define CONFIG_AUTO_COMPLETE			/* add autocompletion support */
+#define CONFIG_SYS_LOAD_ADDR	0x2000000	/* default load address */
+#define CONFIG_SYS_CBSIZE	256		/* Console I/O Buffer Size */
+#define CONFIG_SYS_PBSIZE (CONFIG_SYS_CBSIZE+sizeof(CONFIG_SYS_PROMPT)+16)
+#define CONFIG_SYS_MAXARGS	16		/* max number of command args */
+#define CONFIG_SYS_BARGSIZE	CONFIG_SYS_CBSIZE/* Boot Argument Buffer Size */
+
+/*
+ * For booting Linux, the board info and command line data
+ * have to be in the first 64 MB of memory, since this is
+ * the maximum mapped by the Linux kernel during initialization.
+ */
+#define CONFIG_SYS_BOOTMAPSZ	(64 << 20)	/* Initial map for Linux*/
+#define CONFIG_SYS_BOOTM_LEN	(64 << 20)	/* Increase max gunzip size */
+
+/*
+ * Environment Configuration
+ */
+#define CONFIG_ROOTPATH		"/opt/nfsroot"
+#define CONFIG_BOOTFILE		"uImage"
+#define CONFIG_UBOOTPATH	"u-boot.bin"	/* U-Boot image on TFTP server*/
+
+/* default location for tftp and bootm */
+#define CONFIG_LOADADDR		1000000
+
+#define CONFIG_BAUDRATE	115200
+
+#define CONFIG_BOOTDELAY        1
+#define CONFIG_BOOTCOMMAND		\
+	"test -n \"$qemu_kernel_addr\" && bootm $qemu_kernel_addr - $fdt_addr_r\0"
+
+#endif	/* __QEMU_PPCE500_H */
-- 
1.7.10.4

[U-Boot] [PATCH v2 3/6] PPC 85xx: Add qemu-ppce500 machine

[York Sun]

On 01/31/2014 03:16 AM, Alexander Graf wrote:
> For KVM we have a special PV machine type called "ppce500". This machine
> is inspired by the MPC8544DS board, but implements a lot less features
> than that one.
> 
> It also provides more PCI slots and is supposed to be enumerated by
> device tree only.
> 
> This patch adds support for the current generation ppce500 machine as
> it is implemented today.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> 
> ---
> 
> v1 -> v2:
> 
>   - Write device tree offset directly into global variable
>   - use r4 rather than r2 for that
>   - access fdt directly from in-memory copy
>   - remove unneeded header includes
>   - clean up pci enumeration
>   - coding style fixes
>   - populate and only use fdt_addr_r
>   - remove unused exported functions
>   - remove unused TLB0 entries
>   - make TLB1 I/O maps non-executable
>   - remove unused defines in board header
>   - make -kernel boot variables more clear
>   - remove TLB0 invalidation
>   - use tlb1.14 for temporary as=1 map
>   - use CONFIG_SYS_MPC85XX_NO_RESETVEC
>   - store fdt pointer in gd through cpu_init_early_f()
>   - replace fixup_tlb1() with dynamic TLB creation hook
> ---

<snip>

> diff --git a/arch/powerpc/cpu/mpc85xx/tlb.c b/arch/powerpc/cpu/mpc85xx/tlb.c
> index 8748ecd..c899d9e 100644
> --- a/arch/powerpc/cpu/mpc85xx/tlb.c
> +++ b/arch/powerpc/cpu/mpc85xx/tlb.c
> @@ -36,6 +36,10 @@ void init_tlbs(void)
>  			  tlb_table[i].mas7);
>  	}
>  
> +#ifdef CONFIG_USE_DYNAMIC_TLBS
> +	init_tlbs_dynamic();
> +#endif
> +

You are adding a new CONFIG to common files. Please document it.

>  	return ;
>  }
>  

<snip>

> diff --git a/include/configs/qemu-ppce500.h b/include/configs/qemu-ppce500.h
> new file mode 100644
> index 0000000..1ebaf51
> --- /dev/null
> +++ b/include/configs/qemu-ppce500.h
> @@ -0,0 +1,209 @@
> +/*
> + * Copyright 2011-2014 Freescale Semiconductor, Inc.
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +/*
> + * Corenet DS style board configuration file
> + */
> +#ifndef __QEMU_PPCE500_H
> +#define __QEMU_PPCE500_H
> +
> +#define CONFIG_CMD_REGINFO
> +
> +/* High Level Configuration Options */
> +#define CONFIG_BOOKE
> +#define CONFIG_E500			/* BOOKE e500 family */
> +#define CONFIG_MPC85xx			/* MPC85xx/PQ3 platform */
> +#define CONFIG_QEMU_E500
> +
> +#undef CONFIG_SYS_TEXT_BASE
> +#define CONFIG_SYS_TEXT_BASE	0xf01000 /* 15 MB */
> +
> +#define CONFIG_SYS_MPC85XX_NO_RESETVEC
> +
> +#define CONFIG_SYS_RAMBOOT
> +
> +#define CONFIG_PCI			/* Enable PCI/PCIE */
> +#define CONFIG_PCI1		1	/* PCI controller 1 */
> +#define CONFIG_FSL_PCI_INIT		/* Use common FSL init code */
> +#define CONFIG_SYS_PCI_64BIT		/* enable 64-bit PCI resources */
> +
> +#define CONFIG_ENV_OVERWRITE
> +
> +#define CONFIG_ENABLE_36BIT_PHYS
> +
> +#define CONFIG_ADDR_MAP
> +#define CONFIG_SYS_NUM_ADDR_MAP		16	/* number of TLB1 entries */
> +
> +#define CONFIG_USE_DYNAMIC_TLBS
> +
> +#define CONFIG_SYS_MEMTEST_START	0x00200000	/* memtest works on */
> +#define CONFIG_SYS_MEMTEST_END		0x00400000
> +#define CONFIG_SYS_ALT_MEMTEST
> +#define CONFIG_PANIC_HANG	/* do not reset board on panic */
> +
> +#define CONFIG_SYS_CCSRBAR		0xe0000000
> +#define CONFIG_SYS_CCSRBAR_PHYS_LOW	CONFIG_SYS_CCSRBAR
> +
> +/*
> + * DDR Setup
> + */
> +#define CONFIG_VERY_BIG_RAM
> +#define CONFIG_SYS_DDR_SDRAM_BASE	0x00000000
> +#define CONFIG_SYS_SDRAM_BASE		CONFIG_SYS_DDR_SDRAM_BASE
> +
> +#define CONFIG_CHIP_SELECTS_PER_CTRL	0

I don't know if the qemu has PCI, DDR, etc. Setting the above line to 0 will
actually disable DDR controllers. Is that what you want?

> +
> +/* Get RAM size from device tree */
> +#define CONFIG_DDR_SPD

You enabled SPD but I don't see the I2C address. Did you miss something, or you
don't really use SPD?

York

[U-Boot] [PATCH v2 3/6] PPC 85xx: Add qemu-ppce500 machine

[Scott Wood]

On Fri, 2014-01-31 at 10:05 -0800, York Sun wrote:
> On 01/31/2014 03:16 AM, Alexander Graf wrote:
> > +/*
> > + * DDR Setup
> > + */
> > +#define CONFIG_VERY_BIG_RAM
> > +#define CONFIG_SYS_DDR_SDRAM_BASE	0x00000000
> > +#define CONFIG_SYS_SDRAM_BASE		CONFIG_SYS_DDR_SDRAM_BASE
> > +
> > +#define CONFIG_CHIP_SELECTS_PER_CTRL	0
> 
> I don't know if the qemu has PCI, DDR, etc. Setting the above line to 0 will
> actually disable DDR controllers. Is that what you want?

QEMU does not emulate the DDR controller.

> > +
> > +/* Get RAM size from device tree */
> > +#define CONFIG_DDR_SPD
> 
> You enabled SPD but I don't see the I2C address. Did you miss something, or you
> don't really use SPD?

This should be removed.

-Scott

[U-Boot] [PATCH v2 3/6] PPC 85xx: Add qemu-ppce500 machine

[Scott Wood]

On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
> +void pci_init_board(void)
> +{
> +	struct fsl_pci_info pci_info;
> +	const void *fdt = get_fdt();
> +	int pci_node;
> +
> +	puts("\n");
> +
> +	pci_node = fdt_path_offset(fdt, "/pci");
> +	if (pci_node < 0) {
> +		printf("PCI: disabled\n\n");
> +		return;
> +	}
> +
> +	SET_STD_PCI_INFO(pci_info, 1);
> +
> +	fsl_setup_hose(&pci1_hose, pci_info.regs);
> +	printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
> +		pci_info.regs);

Why hardcode these things in a message?  Just don't print anything if
you don't have the info.

Is QEMU's PCI emulation 32-bit-only?

> +void init_tlbs_dynamic(void)
> +{
> +	unsigned long fdt_tlb = (unsigned long)get_fdt() & ~0xffffful;
> +	u32 mas0, mas1, mas2, mas3, mas7;
> +	phys_size_t ram_size;
> +
> +	/*
> +	 * Create a temporary AS=1 map for the fdt
> +	 *
> +	 * We use ESEL=0 here to overwrite the previous AS=0 map for ourselves
> +	 * which was only 4k big. This way we don't have to clear any other maps.
> +	 */
> +	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(0);
> +	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TS | MAS1_TSIZE(BOOKE_PAGESZ_1M);
> +	mas2 = FSL_BOOKE_MAS2(fdt_tlb, 0);
> +	mas3 = FSL_BOOKE_MAS3(fdt_tlb, 0, MAS3_SW|MAS3_SR|MAS3_SX);
> +	mas7 = FSL_BOOKE_MAS7(fdt_tlb);
> +
> +	write_tlb(mas0, mas1, mas2, mas3, mas7);

How do you know you're not creating an overlapping TLB entry?  You
should map this to a fixed virtual address that you know is safe.

> +	/* Create a dynamic AS=0 CCSRBAR mapping */
> +	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(13);
> +	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TSIZE(BOOKE_PAGESZ_1M);
> +	mas2 = FSL_BOOKE_MAS2(CONFIG_SYS_CCSRBAR, MAS2_I|MAS2_G);
> +	mas3 = FSL_BOOKE_MAS3(CONFIG_SYS_CCSRBAR_PHYS, 0, MAS3_SW|MAS3_SR);
> +	mas7 = FSL_BOOKE_MAS7(CONFIG_SYS_CCSRBAR_PHYS);

CCSRBAR may be 1M or 16M (assuming qemu-ppce500 sticks with a CCSR-ish
layout at all).  Really we should be creating explicit maps for
everything we find in the device tree that we care about.

> +#define CONFIG_SYS_CCSRBAR		0xe0000000
> +#define CONFIG_SYS_CCSRBAR_PHYS_LOW	CONFIG_SYS_CCSRBAR

Set CONFIG_SYS_CCSR_DO_NOT_RELOCATE instead of
CONFIG_SYS_CCSRBAR_PHYS_LOW.

> +/* Get RAM size from device tree */
> +#define CONFIG_DDR_SPD

That's not what CONFIG_DDR_SPD means.

-Scott

[U-Boot] [PATCH v2 3/6] PPC 85xx: Add qemu-ppce500 machine

[Alexander Graf]

On 04.02.2014, at 03:19, Scott Wood <scottwood@freescale.com> wrote:

> On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
>> +void pci_init_board(void)
>> +{
>> +	struct fsl_pci_info pci_info;
>> +	const void *fdt = get_fdt();
>> +	int pci_node;
>> +
>> +	puts("\n");
>> +
>> +	pci_node = fdt_path_offset(fdt, "/pci");
>> +	if (pci_node < 0) {
>> +		printf("PCI: disabled\n\n");
>> +		return;
>> +	}
>> +
>> +	SET_STD_PCI_INFO(pci_info, 1);
>> +
>> +	fsl_setup_hose(&pci1_hose, pci_info.regs);
>> +	printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
>> +		pci_info.regs);
> 
> Why hardcode these things in a message?  Just don't print anything if
> you don't have the info.

To make it look more akin to a real e500 board. But I'll change it.

> Is QEMU's PCI emulation 32-bit-only?
> 
>> +void init_tlbs_dynamic(void)
>> +{
>> +	unsigned long fdt_tlb = (unsigned long)get_fdt() & ~0xffffful;
>> +	u32 mas0, mas1, mas2, mas3, mas7;
>> +	phys_size_t ram_size;
>> +
>> +	/*
>> +	 * Create a temporary AS=1 map for the fdt
>> +	 *
>> +	 * We use ESEL=0 here to overwrite the previous AS=0 map for ourselves
>> +	 * which was only 4k big. This way we don't have to clear any other maps.
>> +	 */
>> +	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(0);
>> +	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TS | MAS1_TSIZE(BOOKE_PAGESZ_1M);
>> +	mas2 = FSL_BOOKE_MAS2(fdt_tlb, 0);
>> +	mas3 = FSL_BOOKE_MAS3(fdt_tlb, 0, MAS3_SW|MAS3_SR|MAS3_SX);
>> +	mas7 = FSL_BOOKE_MAS7(fdt_tlb);
>> +
>> +	write_tlb(mas0, mas1, mas2, mas3, mas7);
> 
> How do you know you're not creating an overlapping TLB entry?  You
> should map this to a fixed virtual address that you know is safe.

Ok. I'll map it behind CCSRBAR.

> 
>> +	/* Create a dynamic AS=0 CCSRBAR mapping */
>> +	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(13);
>> +	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TSIZE(BOOKE_PAGESZ_1M);
>> +	mas2 = FSL_BOOKE_MAS2(CONFIG_SYS_CCSRBAR, MAS2_I|MAS2_G);
>> +	mas3 = FSL_BOOKE_MAS3(CONFIG_SYS_CCSRBAR_PHYS, 0, MAS3_SW|MAS3_SR);
>> +	mas7 = FSL_BOOKE_MAS7(CONFIG_SYS_CCSRBAR_PHYS);
> 
> CCSRBAR may be 1M or 16M (assuming qemu-ppce500 sticks with a CCSR-ish
> layout at all).  Really we should be creating explicit maps for
> everything we find in the device tree that we care about.

I don't understand that part. We do create explicit maps for everything we care about, no?

> 
>> +#define CONFIG_SYS_CCSRBAR		0xe0000000
>> +#define CONFIG_SYS_CCSRBAR_PHYS_LOW	CONFIG_SYS_CCSRBAR
> 
> Set CONFIG_SYS_CCSR_DO_NOT_RELOCATE instead of
> CONFIG_SYS_CCSRBAR_PHYS_LOW.

Ok.

> 
>> +/* Get RAM size from device tree */
>> +#define CONFIG_DDR_SPD
> 
> That's not what CONFIG_DDR_SPD means.

I've changed the #ifdef I care about to cover CONFIG_QEMU_E500 as well and removed this define now.


Alex

[U-Boot] [PATCH v2 3/6] PPC 85xx: Add qemu-ppce500 machine

[Scott Wood]

On Thu, 2014-02-06 at 13:48 +0100, Alexander Graf wrote:
> On 04.02.2014, at 03:19, Scott Wood <scottwood@freescale.com> wrote:
> 
> > On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
> >> +void pci_init_board(void)
> >> +{
> >> +	struct fsl_pci_info pci_info;
> >> +	const void *fdt = get_fdt();
> >> +	int pci_node;
> >> +
> >> +	puts("\n");
> >> +
> >> +	pci_node = fdt_path_offset(fdt, "/pci");
> >> +	if (pci_node < 0) {
> >> +		printf("PCI: disabled\n\n");
> >> +		return;
> >> +	}
> >> +
> >> +	SET_STD_PCI_INFO(pci_info, 1);
> >> +
> >> +	fsl_setup_hose(&pci1_hose, pci_info.regs);
> >> +	printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
> >> +		pci_info.regs);
> > 
> > Why hardcode these things in a message?  Just don't print anything if
> > you don't have the info.
> 
> To make it look more akin to a real e500 board. But I'll change it.

It's a paravirt target...

> >> +void init_tlbs_dynamic(void)
> >> +{
> >> +	unsigned long fdt_tlb = (unsigned long)get_fdt() & ~0xffffful;
> >> +	u32 mas0, mas1, mas2, mas3, mas7;
> >> +	phys_size_t ram_size;
> >> +
> >> +	/*
> >> +	 * Create a temporary AS=1 map for the fdt
> >> +	 *
> >> +	 * We use ESEL=0 here to overwrite the previous AS=0 map for ourselves
> >> +	 * which was only 4k big. This way we don't have to clear any other maps.
> >> +	 */
> >> +	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(0);
> >> +	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TS | MAS1_TSIZE(BOOKE_PAGESZ_1M);
> >> +	mas2 = FSL_BOOKE_MAS2(fdt_tlb, 0);
> >> +	mas3 = FSL_BOOKE_MAS3(fdt_tlb, 0, MAS3_SW|MAS3_SR|MAS3_SX);
> >> +	mas7 = FSL_BOOKE_MAS7(fdt_tlb);
> >> +
> >> +	write_tlb(mas0, mas1, mas2, mas3, mas7);
> > 
> > How do you know you're not creating an overlapping TLB entry?  You
> > should map this to a fixed virtual address that you know is safe.
> 
> Ok. I'll map it behind CCSRBAR.

I'd give it its own CONFIG symbol to be explicitly located in the header
file.

> >> +	/* Create a dynamic AS=0 CCSRBAR mapping */
> >> +	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(13);
> >> +	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TSIZE(BOOKE_PAGESZ_1M);
> >> +	mas2 = FSL_BOOKE_MAS2(CONFIG_SYS_CCSRBAR, MAS2_I|MAS2_G);
> >> +	mas3 = FSL_BOOKE_MAS3(CONFIG_SYS_CCSRBAR_PHYS, 0, MAS3_SW|MAS3_SR);
> >> +	mas7 = FSL_BOOKE_MAS7(CONFIG_SYS_CCSRBAR_PHYS);
> > 
> > CCSRBAR may be 1M or 16M (assuming qemu-ppce500 sticks with a CCSR-ish
> > layout at all).  Really we should be creating explicit maps for
> > everything we find in the device tree that we care about.
> 
> I don't understand that part. We do create explicit maps for everything we care about, no?

No, you map CCSR as a whole, rather than the UART, PCI controller regs,
etc.  If you want to that, ideally the size of CCSR should come from the
device tree as well as the address.

-Scott

[U-Boot] [PATCH v2 3/6] PPC 85xx: Add qemu-ppce500 machine

[Alexander Graf]

On 06.02.2014, at 23:55, Scott Wood <scottwood@freescale.com> wrote:

> On Thu, 2014-02-06 at 13:48 +0100, Alexander Graf wrote:
>> On 04.02.2014, at 03:19, Scott Wood <scottwood@freescale.com> wrote:
>> 
>>> On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
>>>> +void pci_init_board(void)
>>>> +{
>>>> +	struct fsl_pci_info pci_info;
>>>> +	const void *fdt = get_fdt();
>>>> +	int pci_node;
>>>> +
>>>> +	puts("\n");
>>>> +
>>>> +	pci_node = fdt_path_offset(fdt, "/pci");
>>>> +	if (pci_node < 0) {
>>>> +		printf("PCI: disabled\n\n");
>>>> +		return;
>>>> +	}
>>>> +
>>>> +	SET_STD_PCI_INFO(pci_info, 1);
>>>> +
>>>> +	fsl_setup_hose(&pci1_hose, pci_info.regs);
>>>> +	printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
>>>> +		pci_info.regs);
>>> 
>>> Why hardcode these things in a message?  Just don't print anything if
>>> you don't have the info.
>> 
>> To make it look more akin to a real e500 board. But I'll change it.
> 
> It's a paravirt target...

So? It should still look "normal" to someone who runs it.

> 
>>>> +void init_tlbs_dynamic(void)
>>>> +{
>>>> +	unsigned long fdt_tlb = (unsigned long)get_fdt() & ~0xffffful;
>>>> +	u32 mas0, mas1, mas2, mas3, mas7;
>>>> +	phys_size_t ram_size;
>>>> +
>>>> +	/*
>>>> +	 * Create a temporary AS=1 map for the fdt
>>>> +	 *
>>>> +	 * We use ESEL=0 here to overwrite the previous AS=0 map for ourselves
>>>> +	 * which was only 4k big. This way we don't have to clear any other maps.
>>>> +	 */
>>>> +	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(0);
>>>> +	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TS | MAS1_TSIZE(BOOKE_PAGESZ_1M);
>>>> +	mas2 = FSL_BOOKE_MAS2(fdt_tlb, 0);
>>>> +	mas3 = FSL_BOOKE_MAS3(fdt_tlb, 0, MAS3_SW|MAS3_SR|MAS3_SX);
>>>> +	mas7 = FSL_BOOKE_MAS7(fdt_tlb);
>>>> +
>>>> +	write_tlb(mas0, mas1, mas2, mas3, mas7);
>>> 
>>> How do you know you're not creating an overlapping TLB entry?  You
>>> should map this to a fixed virtual address that you know is safe.
>> 
>> Ok. I'll map it behind CCSRBAR.
> 
> I'd give it its own CONFIG symbol to be explicitly located in the header
> file.

It's only a temporary map that is alive for a few dozen instructions. I think a new CONFIG symbol is excessive here :).

> 
>>>> +	/* Create a dynamic AS=0 CCSRBAR mapping */
>>>> +	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(13);
>>>> +	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TSIZE(BOOKE_PAGESZ_1M);
>>>> +	mas2 = FSL_BOOKE_MAS2(CONFIG_SYS_CCSRBAR, MAS2_I|MAS2_G);
>>>> +	mas3 = FSL_BOOKE_MAS3(CONFIG_SYS_CCSRBAR_PHYS, 0, MAS3_SW|MAS3_SR);
>>>> +	mas7 = FSL_BOOKE_MAS7(CONFIG_SYS_CCSRBAR_PHYS);
>>> 
>>> CCSRBAR may be 1M or 16M (assuming qemu-ppce500 sticks with a CCSR-ish
>>> layout at all).  Really we should be creating explicit maps for
>>> everything we find in the device tree that we care about.
>> 
>> I don't understand that part. We do create explicit maps for everything we care about, no?
> 
> No, you map CCSR as a whole, rather than the UART, PCI controller regs,
> etc.  If you want to that, ideally the size of CCSR should come from the
> device tree as well as the address.

Ah, ok. I can certainly try and pull the size out of the device tree.


Alex

[U-Boot] [PATCH v2 3/6] PPC 85xx: Add qemu-ppce500 machine

[Scott Wood]

On Fri, 2014-02-07 at 00:28 +0100, Alexander Graf wrote:
> On 06.02.2014, at 23:55, Scott Wood <scottwood@freescale.com> wrote:
> 
> > On Thu, 2014-02-06 at 13:48 +0100, Alexander Graf wrote:
> >> On 04.02.2014, at 03:19, Scott Wood <scottwood@freescale.com> wrote:
> >> 
> >>> On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
> >>>> +void pci_init_board(void)
> >>>> +{
> >>>> +	struct fsl_pci_info pci_info;
> >>>> +	const void *fdt = get_fdt();
> >>>> +	int pci_node;
> >>>> +
> >>>> +	puts("\n");
> >>>> +
> >>>> +	pci_node = fdt_path_offset(fdt, "/pci");
> >>>> +	if (pci_node < 0) {
> >>>> +		printf("PCI: disabled\n\n");
> >>>> +		return;
> >>>> +	}
> >>>> +
> >>>> +	SET_STD_PCI_INFO(pci_info, 1);
> >>>> +
> >>>> +	fsl_setup_hose(&pci1_hose, pci_info.regs);
> >>>> +	printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
> >>>> +		pci_info.regs);
> >>> 
> >>> Why hardcode these things in a message?  Just don't print anything if
> >>> you don't have the info.
> >> 
> >> To make it look more akin to a real e500 board. But I'll change it.
> > 
> > It's a paravirt target...
> 
> So? It should still look "normal" to someone who runs it.

Not to the extent of making up info that isn't there.

> >>> How do you know you're not creating an overlapping TLB entry?  You
> >>> should map this to a fixed virtual address that you know is safe.
> >> 
> >> Ok. I'll map it behind CCSRBAR.
> > 
> > I'd give it its own CONFIG symbol to be explicitly located in the header
> > file.
> 
> It's only a temporary map that is alive for a few dozen instructions. I
> think a new CONFIG symbol is excessive here :).

It's address space that should be visible in the map to avoid potential
conflicts.  Whether you want to call it something with FDT in the name,
or call it something that indicates a multi-purpose temporary mapping
location, is up to you.

-Scott

[U-Boot] [PATCH v2 4/6] PPC 85xx: Find CCSRBAR on ppce500 from device tree

[Alexander Graf]

The definition of our ppce500 PV machine is that every address is dynamically
determined through device tree bindings.

So don't hardcode where CCSR is in our physical memory layout but instead
read it dynamically from the device tree we get passed on boot.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/powerpc/cpu/mpc85xx/start.S            |    2 +
 board/freescale/qemu-ppce500/qemu-ppce500.c |   83 +++++++++++++++++++++++++++
 include/configs/qemu-ppce500.h              |   12 +++-
 3 files changed, 96 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/cpu/mpc85xx/start.S b/arch/powerpc/cpu/mpc85xx/start.S
index 0e593d2..2672c20 100644
--- a/arch/powerpc/cpu/mpc85xx/start.S
+++ b/arch/powerpc/cpu/mpc85xx/start.S
@@ -519,6 +519,7 @@ nexti:	mflr	r1		/* R1 = our PC */
  * As a general rule, TLB0 is used for short-term TLBs, and TLB1 is used for
  * long-term TLBs, so we use TLB0 here.
  */
+#if !defined(CONFIG_DYNAMIC_CCSRBAR)
 #if (CONFIG_SYS_CCSRBAR_DEFAULT != CONFIG_SYS_CCSRBAR_PHYS)
 
 #if !defined(CONFIG_SYS_CCSRBAR_PHYS_HIGH) || !defined(CONFIG_SYS_CCSRBAR_PHYS_LOW)
@@ -703,6 +704,7 @@ delete_temp_tlbs:
 	delete_tlb0_entry 1, CONFIG_SYS_CCSRBAR + 0x1000, MAS2_I|MAS2_G, r3
 
 #endif /* #if (CONFIG_SYS_CCSRBAR_DEFAULT != CONFIG_SYS_CCSRBAR_PHYS) */
+#endif /* #if !defined(CONFIG_DYNAMIC_CCSRBAR) */
 
 #if defined(CONFIG_SYS_FSL_QORIQ_CHASSIS2) && defined(CONFIG_E6500)
 create_ccsr_l2_tlb:
diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c b/board/freescale/qemu-ppce500/qemu-ppce500.c
index b33b6b1..6491ae9 100644
--- a/board/freescale/qemu-ppce500/qemu-ppce500.c
+++ b/board/freescale/qemu-ppce500/qemu-ppce500.c
@@ -26,6 +26,89 @@ static const void *get_fdt(void)
 	return gd->fdt_blob;
 }
 
+uint64_t get_phys_ccsrbar_addr(void)
+{
+	const void *fdt = get_fdt();
+	int root_node = fdt_path_offset(fdt, "/");
+	int soc_node = fdt_path_offset(fdt, "/soc");
+	const uint32_t *prop;
+	const uint64_t *prop64;
+	int len;
+	int root_address_cells = 1;
+	int address_cells = 1;
+	uint64_t r = 0;
+
+	/* Read CCSRBAR address length and size from device tree */
+	prop = fdt_getprop(fdt, root_node, "#address-cells", &len);
+	if (prop && (len >= 4))
+		root_address_cells = prop[0];
+
+	prop = fdt_getprop(fdt, soc_node, "#address-cells", &len);
+	if (prop && (len >= 4))
+		address_cells = prop[0];
+
+	/* Read CCSRBAR address from device tree */
+	prop = fdt_getprop(fdt, soc_node, "ranges", &len);
+	if (prop && (len >= ((address_cells + root_address_cells) * 4))) {
+		/* The physical address starts after the child's offset */
+		prop += address_cells;
+		prop64 = (const uint64_t *)prop;
+
+		if (root_address_cells == 1)
+			r = prop[0];
+		else if (root_address_cells == 2)
+			r = prop64[0];
+	}
+
+	if (!r)
+		panic("Couldn't find CCSR in FDT");
+
+	return r;
+}
+
+uint64_t get_phys_ccsrbar_addr_early(void)
+{
+	u32 mas0, mas1, mas2, mas3, mas7;
+	ulong fdt = (ulong)get_fdt();
+	uint64_t r;
+
+	/*
+	 * To be able to read the FDT we need to create a temporary TLB
+	 * map for it.
+	 */
+
+	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(10);
+	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TS | MAS1_TSIZE(BOOKE_PAGESZ_1M);
+	mas2 = FSL_BOOKE_MAS2(fdt, 0);
+	mas3 = FSL_BOOKE_MAS3(fdt, 0, MAS3_SW|MAS3_SR);
+	mas7 = FSL_BOOKE_MAS7(fdt);
+
+	write_tlb(mas0, mas1, mas2, mas3, mas7);
+
+	r = get_phys_ccsrbar_addr();
+
+	disable_tlb(10);
+
+	return r;
+}
+
+int board_early_init_f(void)
+{
+	u32 mas0, mas1, mas2, mas3, mas7;
+	uint64_t phys_ccsr_address = get_phys_ccsrbar_addr();
+
+	/* Extend the CCSR map from cpu_init_early_f() */
+	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(13);
+	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TSIZE(BOOKE_PAGESZ_64M);
+	mas2 = FSL_BOOKE_MAS2(CONFIG_SYS_CCSRBAR, MAS2_I|MAS2_G);
+	mas3 = FSL_BOOKE_MAS3(phys_ccsr_address, 0, MAS3_SW|MAS3_SR);
+	mas7 = FSL_BOOKE_MAS7(phys_ccsr_address);
+
+	write_tlb(mas0, mas1, mas2, mas3, mas7);
+
+	return 0;
+}
+
 int checkboard(void)
 {
 	return 0;
diff --git a/include/configs/qemu-ppce500.h b/include/configs/qemu-ppce500.h
index 1ebaf51..7ef7235 100644
--- a/include/configs/qemu-ppce500.h
+++ b/include/configs/qemu-ppce500.h
@@ -44,8 +44,18 @@
 #define CONFIG_SYS_ALT_MEMTEST
 #define CONFIG_PANIC_HANG	/* do not reset board on panic */
 
+/* Needed to fill the ccsrbar pointer */
+#define CONFIG_BOARD_EARLY_INIT_F
+
+/* Virtual address to CCSRBAR */
 #define CONFIG_SYS_CCSRBAR		0xe0000000
-#define CONFIG_SYS_CCSRBAR_PHYS_LOW	CONFIG_SYS_CCSRBAR
+/* Physical address should be a function call */
+#ifndef __ASSEMBLY__
+extern unsigned long long get_phys_ccsrbar_addr_early(void);
+#endif
+#define CONFIG_SYS_CCSRBAR_PHYS_LOW	((uint32_t)get_phys_ccsrbar_addr_early())
+#define CONFIG_SYS_CCSRBAR_PHYS_HIGH	(get_phys_ccsrbar_addr_early() >> 32)
+#define CONFIG_DYNAMIC_CCSRBAR
 
 /*
  * DDR Setup
-- 
1.7.10.4

[U-Boot] [PATCH v2 4/6] PPC 85xx: Find CCSRBAR on ppce500 from device tree

[Scott Wood]

On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
> The definition of our ppce500 PV machine is that every address is dynamically
> determined through device tree bindings.
> 
> So don't hardcode where CCSR is in our physical memory layout but instead
> read it dynamically from the device tree we get passed on boot.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/powerpc/cpu/mpc85xx/start.S            |    2 +
>  board/freescale/qemu-ppce500/qemu-ppce500.c |   83 +++++++++++++++++++++++++++
>  include/configs/qemu-ppce500.h              |   12 +++-
>  3 files changed, 96 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/cpu/mpc85xx/start.S b/arch/powerpc/cpu/mpc85xx/start.S
> index 0e593d2..2672c20 100644
> --- a/arch/powerpc/cpu/mpc85xx/start.S
> +++ b/arch/powerpc/cpu/mpc85xx/start.S
> @@ -519,6 +519,7 @@ nexti:	mflr	r1		/* R1 = our PC */
>   * As a general rule, TLB0 is used for short-term TLBs, and TLB1 is used for
>   * long-term TLBs, so we use TLB0 here.
>   */
> +#if !defined(CONFIG_DYNAMIC_CCSRBAR)
>  #if (CONFIG_SYS_CCSRBAR_DEFAULT != CONFIG_SYS_CCSRBAR_PHYS)
>  
>  #if !defined(CONFIG_SYS_CCSRBAR_PHYS_HIGH) || !defined(CONFIG_SYS_CCSRBAR_PHYS_LOW)
> @@ -703,6 +704,7 @@ delete_temp_tlbs:
>  	delete_tlb0_entry 1, CONFIG_SYS_CCSRBAR + 0x1000, MAS2_I|MAS2_G, r3
>  
>  #endif /* #if (CONFIG_SYS_CCSRBAR_DEFAULT != CONFIG_SYS_CCSRBAR_PHYS) */
> +#endif /* #if !defined(CONFIG_DYNAMIC_CCSRBAR) */
>  
>  #if defined(CONFIG_SYS_FSL_QORIQ_CHASSIS2) && defined(CONFIG_E6500)
>  create_ccsr_l2_tlb:
> diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c b/board/freescale/qemu-ppce500/qemu-ppce500.c
> index b33b6b1..6491ae9 100644
> --- a/board/freescale/qemu-ppce500/qemu-ppce500.c
> +++ b/board/freescale/qemu-ppce500/qemu-ppce500.c
> @@ -26,6 +26,89 @@ static const void *get_fdt(void)
>  	return gd->fdt_blob;
>  }
>  
> +uint64_t get_phys_ccsrbar_addr(void)
> +{
> +	const void *fdt = get_fdt();
> +	int root_node = fdt_path_offset(fdt, "/");
> +	int soc_node = fdt_path_offset(fdt, "/soc");
> +	const uint32_t *prop;
> +	const uint64_t *prop64;
> +	int len;
> +	int root_address_cells = 1;
> +	int address_cells = 1;
> +	uint64_t r = 0;
> +
> +	/* Read CCSRBAR address length and size from device tree */
> +	prop = fdt_getprop(fdt, root_node, "#address-cells", &len);
> +	if (prop && (len >= 4))
> +		root_address_cells = prop[0];
> +
> +	prop = fdt_getprop(fdt, soc_node, "#address-cells", &len);
> +	if (prop && (len >= 4))
> +		address_cells = prop[0];
> +
> +	/* Read CCSRBAR address from device tree */
> +	prop = fdt_getprop(fdt, soc_node, "ranges", &len);
> +	if (prop && (len >= ((address_cells + root_address_cells) * 4))) {
> +		/* The physical address starts after the child's offset */
> +		prop += address_cells;
> +		prop64 = (const uint64_t *)prop;
> +
> +		if (root_address_cells == 1)
> +			r = prop[0];
> +		else if (root_address_cells == 2)
> +			r = prop64[0];
> +	}
> +
> +	if (!r)
> +		panic("Couldn't find CCSR in FDT");
> +
> +	return r;
> +}
> +
> +uint64_t get_phys_ccsrbar_addr_early(void)
> +{
> +	u32 mas0, mas1, mas2, mas3, mas7;
> +	ulong fdt = (ulong)get_fdt();
> +	uint64_t r;
> +
> +	/*
> +	 * To be able to read the FDT we need to create a temporary TLB
> +	 * map for it.
> +	 */
> +
> +	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(10);
> +	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TS | MAS1_TSIZE(BOOKE_PAGESZ_1M);
> +	mas2 = FSL_BOOKE_MAS2(fdt, 0);
> +	mas3 = FSL_BOOKE_MAS3(fdt, 0, MAS3_SW|MAS3_SR);
> +	mas7 = FSL_BOOKE_MAS7(fdt);
> +
> +	write_tlb(mas0, mas1, mas2, mas3, mas7);
> +
> +	r = get_phys_ccsrbar_addr();
> +
> +	disable_tlb(10);
> +
> +	return r;
> +}
> +
> +int board_early_init_f(void)
> +{
> +	u32 mas0, mas1, mas2, mas3, mas7;
> +	uint64_t phys_ccsr_address = get_phys_ccsrbar_addr();
> +
> +	/* Extend the CCSR map from cpu_init_early_f() */
> +	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(13);
> +	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TSIZE(BOOKE_PAGESZ_64M);
> +	mas2 = FSL_BOOKE_MAS2(CONFIG_SYS_CCSRBAR, MAS2_I|MAS2_G);
> +	mas3 = FSL_BOOKE_MAS3(phys_ccsr_address, 0, MAS3_SW|MAS3_SR);
> +	mas7 = FSL_BOOKE_MAS7(phys_ccsr_address);
> +
> +	write_tlb(mas0, mas1, mas2, mas3, mas7);
> +
> +	return 0;
> +}
> +
>  int checkboard(void)
>  {
>  	return 0;

You're adding new code to map CCSR and FDT, but not removing the code to
do the same thing you added in patch 3/6.

Why 64M?

> diff --git a/include/configs/qemu-ppce500.h b/include/configs/qemu-ppce500.h
> index 1ebaf51..7ef7235 100644
> --- a/include/configs/qemu-ppce500.h
> +++ b/include/configs/qemu-ppce500.h
> @@ -44,8 +44,18 @@
>  #define CONFIG_SYS_ALT_MEMTEST
>  #define CONFIG_PANIC_HANG	/* do not reset board on panic */
>  
> +/* Needed to fill the ccsrbar pointer */
> +#define CONFIG_BOARD_EARLY_INIT_F
> +
> +/* Virtual address to CCSRBAR */
>  #define CONFIG_SYS_CCSRBAR		0xe0000000
> -#define CONFIG_SYS_CCSRBAR_PHYS_LOW	CONFIG_SYS_CCSRBAR
> +/* Physical address should be a function call */
> +#ifndef __ASSEMBLY__
> +extern unsigned long long get_phys_ccsrbar_addr_early(void);
> +#endif
> +#define CONFIG_SYS_CCSRBAR_PHYS_LOW	((uint32_t)get_phys_ccsrbar_addr_early())
> +#define CONFIG_SYS_CCSRBAR_PHYS_HIGH	(get_phys_ccsrbar_addr_early() >> 32)
> +#define CONFIG_DYNAMIC_CCSRBAR
>  

What cares about CONFIG_SYS_CCSRBAR_PHYS* other than the code you
skipped with an ifndef in start.S, and related header ifdeffery (which
could be skipped by the existing CONFIG_SYS_CCSR_DO_NOT_RELOCATE
instead)?

-Scott

[U-Boot] [PATCH v2 4/6] PPC 85xx: Find CCSRBAR on ppce500 from device tree

[Alexander Graf]

On 04.02.2014, at 03:24, Scott Wood <scottwood@freescale.com> wrote:

> On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
>> The definition of our ppce500 PV machine is that every address is dynamically
>> determined through device tree bindings.
>> 
>> So don't hardcode where CCSR is in our physical memory layout but instead
>> read it dynamically from the device tree we get passed on boot.
>> 
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> arch/powerpc/cpu/mpc85xx/start.S            |    2 +
>> board/freescale/qemu-ppce500/qemu-ppce500.c |   83 +++++++++++++++++++++++++++
>> include/configs/qemu-ppce500.h              |   12 +++-
>> 3 files changed, 96 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/powerpc/cpu/mpc85xx/start.S b/arch/powerpc/cpu/mpc85xx/start.S
>> index 0e593d2..2672c20 100644
>> --- a/arch/powerpc/cpu/mpc85xx/start.S
>> +++ b/arch/powerpc/cpu/mpc85xx/start.S
>> @@ -519,6 +519,7 @@ nexti:	mflr	r1		/* R1 = our PC */
>>  * As a general rule, TLB0 is used for short-term TLBs, and TLB1 is used for
>>  * long-term TLBs, so we use TLB0 here.
>>  */
>> +#if !defined(CONFIG_DYNAMIC_CCSRBAR)
>> #if (CONFIG_SYS_CCSRBAR_DEFAULT != CONFIG_SYS_CCSRBAR_PHYS)
>> 
>> #if !defined(CONFIG_SYS_CCSRBAR_PHYS_HIGH) || !defined(CONFIG_SYS_CCSRBAR_PHYS_LOW)
>> @@ -703,6 +704,7 @@ delete_temp_tlbs:
>> 	delete_tlb0_entry 1, CONFIG_SYS_CCSRBAR + 0x1000, MAS2_I|MAS2_G, r3
>> 
>> #endif /* #if (CONFIG_SYS_CCSRBAR_DEFAULT != CONFIG_SYS_CCSRBAR_PHYS) */
>> +#endif /* #if !defined(CONFIG_DYNAMIC_CCSRBAR) */
>> 
>> #if defined(CONFIG_SYS_FSL_QORIQ_CHASSIS2) && defined(CONFIG_E6500)
>> create_ccsr_l2_tlb:
>> diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c b/board/freescale/qemu-ppce500/qemu-ppce500.c
>> index b33b6b1..6491ae9 100644
>> --- a/board/freescale/qemu-ppce500/qemu-ppce500.c
>> +++ b/board/freescale/qemu-ppce500/qemu-ppce500.c
>> @@ -26,6 +26,89 @@ static const void *get_fdt(void)
>> 	return gd->fdt_blob;
>> }
>> 
>> +uint64_t get_phys_ccsrbar_addr(void)
>> +{
>> +	const void *fdt = get_fdt();
>> +	int root_node = fdt_path_offset(fdt, "/");
>> +	int soc_node = fdt_path_offset(fdt, "/soc");
>> +	const uint32_t *prop;
>> +	const uint64_t *prop64;
>> +	int len;
>> +	int root_address_cells = 1;
>> +	int address_cells = 1;
>> +	uint64_t r = 0;
>> +
>> +	/* Read CCSRBAR address length and size from device tree */
>> +	prop = fdt_getprop(fdt, root_node, "#address-cells", &len);
>> +	if (prop && (len >= 4))
>> +		root_address_cells = prop[0];
>> +
>> +	prop = fdt_getprop(fdt, soc_node, "#address-cells", &len);
>> +	if (prop && (len >= 4))
>> +		address_cells = prop[0];
>> +
>> +	/* Read CCSRBAR address from device tree */
>> +	prop = fdt_getprop(fdt, soc_node, "ranges", &len);
>> +	if (prop && (len >= ((address_cells + root_address_cells) * 4))) {
>> +		/* The physical address starts after the child's offset */
>> +		prop += address_cells;
>> +		prop64 = (const uint64_t *)prop;
>> +
>> +		if (root_address_cells == 1)
>> +			r = prop[0];
>> +		else if (root_address_cells == 2)
>> +			r = prop64[0];
>> +	}
>> +
>> +	if (!r)
>> +		panic("Couldn't find CCSR in FDT");
>> +
>> +	return r;
>> +}
>> +
>> +uint64_t get_phys_ccsrbar_addr_early(void)
>> +{
>> +	u32 mas0, mas1, mas2, mas3, mas7;
>> +	ulong fdt = (ulong)get_fdt();
>> +	uint64_t r;
>> +
>> +	/*
>> +	 * To be able to read the FDT we need to create a temporary TLB
>> +	 * map for it.
>> +	 */
>> +
>> +	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(10);
>> +	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TS | MAS1_TSIZE(BOOKE_PAGESZ_1M);
>> +	mas2 = FSL_BOOKE_MAS2(fdt, 0);
>> +	mas3 = FSL_BOOKE_MAS3(fdt, 0, MAS3_SW|MAS3_SR);
>> +	mas7 = FSL_BOOKE_MAS7(fdt);
>> +
>> +	write_tlb(mas0, mas1, mas2, mas3, mas7);
>> +
>> +	r = get_phys_ccsrbar_addr();
>> +
>> +	disable_tlb(10);
>> +
>> +	return r;
>> +}
>> +
>> +int board_early_init_f(void)
>> +{
>> +	u32 mas0, mas1, mas2, mas3, mas7;
>> +	uint64_t phys_ccsr_address = get_phys_ccsrbar_addr();
>> +
>> +	/* Extend the CCSR map from cpu_init_early_f() */
>> +	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(13);
>> +	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TSIZE(BOOKE_PAGESZ_64M);
>> +	mas2 = FSL_BOOKE_MAS2(CONFIG_SYS_CCSRBAR, MAS2_I|MAS2_G);
>> +	mas3 = FSL_BOOKE_MAS3(phys_ccsr_address, 0, MAS3_SW|MAS3_SR);
>> +	mas7 = FSL_BOOKE_MAS7(phys_ccsr_address);
>> +
>> +	write_tlb(mas0, mas1, mas2, mas3, mas7);
>> +
>> +	return 0;
>> +}
>> +
>> int checkboard(void)
>> {
>> 	return 0;
> 
> You're adding new code to map CCSR and FDT, but not removing the code to
> do the same thing you added in patch 3/6.

Good point, This chunk can go now.

> Why 64M?

Because that was the TLB entry it replaced:

+       /*
+        * TLB 3:       64M     Non-cacheable, guarded
+        * 0xe000_0000  1M      CCSRBAR
+        * 0xe100_0000  255M    PCI IO range
+        */
+       SET_TLB_ENTRY(1, CONFIG_SYS_CCSRBAR, CONFIG_SYS_CCSRBAR_PHYS,
+                     MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
+                     0, 3, BOOKE_PAGESZ_64M, 1),

which I just copied from

  board/freescale/mpc8544ds/tlb.c

but yeah, it's not really necessary to be that big.

> 
>> diff --git a/include/configs/qemu-ppce500.h b/include/configs/qemu-ppce500.h
>> index 1ebaf51..7ef7235 100644
>> --- a/include/configs/qemu-ppce500.h
>> +++ b/include/configs/qemu-ppce500.h
>> @@ -44,8 +44,18 @@
>> #define CONFIG_SYS_ALT_MEMTEST
>> #define CONFIG_PANIC_HANG	/* do not reset board on panic */
>> 
>> +/* Needed to fill the ccsrbar pointer */
>> +#define CONFIG_BOARD_EARLY_INIT_F
>> +
>> +/* Virtual address to CCSRBAR */
>> #define CONFIG_SYS_CCSRBAR		0xe0000000
>> -#define CONFIG_SYS_CCSRBAR_PHYS_LOW	CONFIG_SYS_CCSRBAR
>> +/* Physical address should be a function call */
>> +#ifndef __ASSEMBLY__
>> +extern unsigned long long get_phys_ccsrbar_addr_early(void);
>> +#endif
>> +#define CONFIG_SYS_CCSRBAR_PHYS_LOW	((uint32_t)get_phys_ccsrbar_addr_early())
>> +#define CONFIG_SYS_CCSRBAR_PHYS_HIGH	(get_phys_ccsrbar_addr_early() >> 32)
>> +#define CONFIG_DYNAMIC_CCSRBAR
>> 
> 
> What cares about CONFIG_SYS_CCSRBAR_PHYS* other than the code you
> skipped with an ifndef in start.S, and related header ifdeffery (which
> could be skipped by the existing CONFIG_SYS_CCSR_DO_NOT_RELOCATE
> instead)?

Nothing. I'll replace it.


Alex

[U-Boot] [PATCH v2 5/6] PPC 85xx: Find PCI host controllers on ppce500 from device tree

[Alexander Graf]

The definition of our ppce500 PV machine is that every address is dynamically
determined through device tree bindings.

So don't hardcode where PCI devices are in our physical memory layout but instead
read them dynamically from the device tree we get passed on boot.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 board/freescale/qemu-ppce500/qemu-ppce500.c |  193 ++++++++++++++++++++++++---
 board/freescale/qemu-ppce500/tlb.c          |   13 --
 include/configs/qemu-ppce500.h              |   12 --
 3 files changed, 175 insertions(+), 43 deletions(-)

diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c b/board/freescale/qemu-ppce500/qemu-ppce500.c
index 6491ae9..5d4dd64 100644
--- a/board/freescale/qemu-ppce500/qemu-ppce500.c
+++ b/board/freescale/qemu-ppce500/qemu-ppce500.c
@@ -19,7 +19,51 @@
 #include <malloc.h>
 
 DECLARE_GLOBAL_DATA_PTR;
-static struct pci_controller pci1_hose;
+
+static uint64_t myfdt_readcells(const void *fdt, int node, const char *property,
+				int num, int off, uint64_t defval)
+{
+	int len;
+	const uint32_t *prop;
+
+	prop = fdt_getprop(fdt, node, property, &len);
+
+	if (!prop)
+		return defval;
+
+	if (len < ((off + num) * sizeof(uint32_t)))
+		panic("Invalid fdt");
+
+	prop += off;
+
+	switch (num) {
+	case 1:
+		return *prop;
+	case 2:
+		return *(const uint64_t *)prop;
+	}
+
+	panic("Invalid cell size");
+}
+
+static uint32_t myfdt_one_cell(const void *fdt, int node, const char *property,
+			       uint32_t defval)
+{
+	return myfdt_readcells(fdt, node, property, 1, 0, defval);
+}
+
+static int myfdt_count_compatibles(const void *fdt, const char *compat)
+{
+	int node, num = 0;
+
+	node = fdt_node_offset_by_compatible(fdt, -1, compat);
+	while (node != -FDT_ERR_NOTFOUND) {
+		node = fdt_node_offset_by_compatible(fdt, node, compat);
+		num++;
+	}
+
+	return num;
+}
 
 static const void *get_fdt(void)
 {
@@ -39,13 +83,9 @@ uint64_t get_phys_ccsrbar_addr(void)
 	uint64_t r = 0;
 
 	/* Read CCSRBAR address length and size from device tree */
-	prop = fdt_getprop(fdt, root_node, "#address-cells", &len);
-	if (prop && (len >= 4))
-		root_address_cells = prop[0];
-
-	prop = fdt_getprop(fdt, soc_node, "#address-cells", &len);
-	if (prop && (len >= 4))
-		address_cells = prop[0];
+	root_address_cells = myfdt_one_cell(fdt, root_node, "#address-cells", 1);
+	address_cells = myfdt_one_cell(fdt, soc_node, "#address-cells",
+				       root_address_cells);
 
 	/* Read CCSRBAR address from device tree */
 	prop = fdt_getprop(fdt, soc_node, "ranges", &len);
@@ -114,27 +154,144 @@ int checkboard(void)
 	return 0;
 }
 
+static void map_tlb1_io(ulong virt_addr, uint64_t phys_addr, uint64_t size)
+{
+	unsigned int max_cam, tsize_mask;
+	int i;
+
+	if ((mfspr(SPRN_MMUCFG) & MMUCFG_MAVN) == MMUCFG_MAVN_V1) {
+		/* Convert (4^max) kB to (2^max) bytes */
+		max_cam = ((mfspr(SPRN_TLB1CFG) >> 16) & 0xf) * 2 + 10;
+		tsize_mask = ~1U;
+	} else {
+		/* Convert (2^max) kB to (2^max) bytes */
+		max_cam = __ilog2(mfspr(SPRN_TLB1PS)) + 10;
+		tsize_mask = ~0U;
+	}
+
+	for (i = 0; size && i < 8; i++) {
+		int tlb_index = find_free_tlbcam();
+		u32 camsize = __ilog2_u64(size) & tsize_mask;
+		u32 align = __ilog2(virt_addr) & tsize_mask;
+		unsigned int tlb_size;
+
+		if (tlb_index == -1)
+			break;
+
+		if (align == -2) align = max_cam;
+		if (camsize > align)
+			camsize = align;
+
+		if (camsize > max_cam)
+			camsize = max_cam;
+
+		tlb_size = camsize - 10;
+
+		set_tlb(1, virt_addr, phys_addr,
+			MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
+			0, tlb_index, tlb_size, 1);
+
+		/* Remember the mapping in our address map */
+		addrmap_set_entry(virt_addr, phys_addr, 1ULL << camsize,
+				  tlb_index);
+
+		size -= 1ULL << camsize;
+		virt_addr += 1UL << camsize;
+		phys_addr += 1UL << camsize;
+	}
+
+}
+
 void pci_init_board(void)
 {
-	struct fsl_pci_info pci_info;
+	struct pci_controller *pci_hoses;
 	const void *fdt = get_fdt();
 	int pci_node;
+	int pci_num = 0;
+	int pci_count;
+	const char *compat = "fsl,mpc8540-pci";
+	ulong map_addr;
 
 	puts("\n");
 
-	pci_node = fdt_path_offset(fdt, "/pci");
-	if (pci_node < 0) {
+	/* Start MMIO and PIO range maps above RAM */
+	map_addr = CONFIG_MAX_MEM_MAPPED;
+
+	/* Count and allocate PCI buses */
+	pci_count = myfdt_count_compatibles(fdt, compat);
+
+	if (pci_count) {
+		pci_hoses = malloc(sizeof(struct pci_controller) * pci_count);
+	} else {
 		printf("PCI: disabled\n\n");
 		return;
 	}
 
-	SET_STD_PCI_INFO(pci_info, 1);
-
-	fsl_setup_hose(&pci1_hose, pci_info.regs);
-	printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
-		pci_info.regs);
-
-	fsl_pci_init_port(&pci_info, &pci1_hose, 0);
+	/* Spawn PCI buses based on device tree */
+	pci_node = fdt_node_offset_by_compatible(fdt, -1, compat);
+	while (pci_node != -FDT_ERR_NOTFOUND) {
+		struct fsl_pci_info pci_info = { };
+		uint64_t phys_addr;
+		int pnode = fdt_parent_offset(fdt, pci_node);
+		int paddress_cells;
+		int address_cells;
+		int size_cells;
+		int off = 0;
+
+		paddress_cells = myfdt_one_cell(fdt, pnode, "#address-cells", 1);
+		address_cells = myfdt_one_cell(fdt, pci_node, "#address-cells", 1);
+		size_cells = myfdt_one_cell(fdt, pci_node, "#size-cells", 1);
+
+		pci_info.regs = myfdt_readcells(fdt, pci_node, "reg",
+						paddress_cells, 0, 0);
+
+		/* MMIO range */
+		off += address_cells;
+		phys_addr = myfdt_readcells(fdt, pci_node, "ranges",
+					    paddress_cells, off, 0);
+		off += paddress_cells;
+		pci_info.mem_size = myfdt_readcells(fdt, pci_node, "ranges",
+			size_cells, off, 0);
+		off += size_cells;
+
+		/* Align virtual region */
+		map_addr += pci_info.mem_size - 1;
+		map_addr &= ~(pci_info.mem_size - 1);
+		/* Map virtual memory for MMIO range */
+		map_tlb1_io(map_addr, phys_addr, pci_info.mem_size);
+		pci_info.mem_bus = phys_addr;
+		pci_info.mem_phys = phys_addr;
+		map_addr += pci_info.mem_size;
+
+		/* PIO range */
+		off += address_cells;
+		pci_info.io_phys = myfdt_readcells(fdt, pci_node, "ranges",
+			paddress_cells, off, 0);
+		off += paddress_cells;
+		pci_info.io_size = myfdt_readcells(fdt, pci_node, "ranges",
+			size_cells, off, 0);
+
+		/* Align virtual region */
+		map_addr += pci_info.io_size - 1;
+		map_addr &= ~(pci_info.io_size - 1);
+		/* Map virtual memory for MMIO range */
+		map_tlb1_io(map_addr, pci_info.io_phys, pci_info.io_size);
+		pci_info.io_bus = map_addr;
+		map_addr += pci_info.io_size;
+
+		/* Instantiate */
+		pci_info.pci_num = pci_num + 1;
+
+		fsl_setup_hose(&pci_hoses[pci_num], pci_info.regs);
+		printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
+			pci_info.regs);
+
+		fsl_pci_init_port(&pci_info, &pci_hoses[pci_num], pci_num);
+
+		/* Jump to next PCI node */
+		pci_node = fdt_node_offset_by_compatible(fdt, pci_node, compat);
+		pci_num++;
+	}
 
 	puts("\n");
 }
diff --git a/board/freescale/qemu-ppce500/tlb.c b/board/freescale/qemu-ppce500/tlb.c
index a600296..cf51d0e 100644
--- a/board/freescale/qemu-ppce500/tlb.c
+++ b/board/freescale/qemu-ppce500/tlb.c
@@ -11,19 +11,6 @@
 #include <asm/mmu.h>
 
 struct fsl_e_tlb_entry tlb_table[] = {
-	/*
-	 * TLB 1:	256M	Non-cacheable, guarded
-	 */
-	SET_TLB_ENTRY(1, CONFIG_SYS_PCI_VIRT, CONFIG_SYS_PCI_PHYS,
-		      MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
-		      0, 1, BOOKE_PAGESZ_256M, 1),
-
-	/*
-	 * TLB 2:	256M	Non-cacheable, guarded
-	 */
-	SET_TLB_ENTRY(1, CONFIG_SYS_PCI_VIRT + 0x10000000, CONFIG_SYS_PCI_PHYS + 0x10000000,
-		      MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
-		      0, 2, BOOKE_PAGESZ_256M, 1),
 };
 
 int num_tlb_entries = ARRAY_SIZE(tlb_table);
diff --git a/include/configs/qemu-ppce500.h b/include/configs/qemu-ppce500.h
index 7ef7235..f7e59bc 100644
--- a/include/configs/qemu-ppce500.h
+++ b/include/configs/qemu-ppce500.h
@@ -127,18 +127,6 @@ extern unsigned long long get_phys_ccsrbar_addr_early(void);
  * Memory space is mapped 1-1, but I/O space must start from 0.
  */
 
-#define CONFIG_SYS_PCI_VIRT		0xc0000000	/* 512M PCI TLB */
-#define CONFIG_SYS_PCI_PHYS		0xc0000000	/* 512M PCI TLB */
-
-#define CONFIG_SYS_PCI1_MEM_VIRT	0xc0000000
-#define CONFIG_SYS_PCI1_MEM_BUS	0xc0000000
-#define CONFIG_SYS_PCI1_MEM_PHYS	0xc0000000
-#define CONFIG_SYS_PCI1_MEM_SIZE	0x20000000	/* 512M */
-#define CONFIG_SYS_PCI1_IO_VIRT	0xe1000000
-#define CONFIG_SYS_PCI1_IO_BUS	0x00000000
-#define CONFIG_SYS_PCI1_IO_PHYS	0xe1000000
-#define CONFIG_SYS_PCI1_IO_SIZE	0x00010000	/* 64k */
-
 #ifdef CONFIG_PCI
 #define CONFIG_PCI_INDIRECT_BRIDGE
 #define CONFIG_NET_MULTI
-- 
1.7.10.4

[U-Boot] [PATCH v2 5/6] PPC 85xx: Find PCI host controllers on ppce500 from device tree

[Scott Wood]

On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
> The definition of our ppce500 PV machine is that every address is dynamically
> determined through device tree bindings.
> 
> So don't hardcode where PCI devices are in our physical memory layout but instead
> read them dynamically from the device tree we get passed on boot.

Would it be difficult to make the QEMU emulation properly implement
access windows?

> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  board/freescale/qemu-ppce500/qemu-ppce500.c |  193 ++++++++++++++++++++++++---
>  board/freescale/qemu-ppce500/tlb.c          |   13 --
>  include/configs/qemu-ppce500.h              |   12 --
>  3 files changed, 175 insertions(+), 43 deletions(-)
> 
> diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c b/board/freescale/qemu-ppce500/qemu-ppce500.c
> index 6491ae9..5d4dd64 100644
> --- a/board/freescale/qemu-ppce500/qemu-ppce500.c
> +++ b/board/freescale/qemu-ppce500/qemu-ppce500.c
> @@ -19,7 +19,51 @@
>  #include <malloc.h>
>  
>  DECLARE_GLOBAL_DATA_PTR;
> -static struct pci_controller pci1_hose;
> +
> +static uint64_t myfdt_readcells(const void *fdt, int node, const char *property,
> +				int num, int off, uint64_t defval)

"my" fdt?

> +{
> +	int len;
> +	const uint32_t *prop;
> +
> +	prop = fdt_getprop(fdt, node, property, &len);
> +
> +	if (!prop)
> +		return defval;
> +
> +	if (len < ((off + num) * sizeof(uint32_t)))
> +		panic("Invalid fdt");
> +
> +	prop += off;
> +
> +	switch (num) {
> +	case 1:
> +		return *prop;
> +	case 2:
> +		return *(const uint64_t *)prop;
> +	}
> +

What about this function is specific to qemu-ppce500?

+	panic("Invalid cell size");
+}

s/cell size/cell count/

> +static uint32_t myfdt_one_cell(const void *fdt, int node, const char *property,
> +			       uint32_t defval)
> +{
> +	return myfdt_readcells(fdt, node, property, 1, 0, defval);
> +}

This looks a lot like fdt_getprop_u32_default(), except for "int node"
versus path.

> +static void map_tlb1_io(ulong virt_addr, uint64_t phys_addr, uint64_t size)
> +{
> +	unsigned int max_cam, tsize_mask;
> +	int i;
> +
> +	if ((mfspr(SPRN_MMUCFG) & MMUCFG_MAVN) == MMUCFG_MAVN_V1) {
> +		/* Convert (4^max) kB to (2^max) bytes */
> +		max_cam = ((mfspr(SPRN_TLB1CFG) >> 16) & 0xf) * 2 + 10;
> +		tsize_mask = ~1U;
> +	} else {
> +		/* Convert (2^max) kB to (2^max) bytes */
> +		max_cam = __ilog2(mfspr(SPRN_TLB1PS)) + 10;
> +		tsize_mask = ~0U;
> +	}
> +
> +	for (i = 0; size && i < 8; i++) {
> +		int tlb_index = find_free_tlbcam();
> +		u32 camsize = __ilog2_u64(size) & tsize_mask;
> +		u32 align = __ilog2(virt_addr) & tsize_mask;
> +		unsigned int tlb_size;
> +
> +		if (tlb_index == -1)
> +			break;
> +
> +		if (align == -2) align = max_cam;

-2?  Besides align being unsigned, if this is meant to handle the case
where virt_addr is zero, that's undefined for __ilog2() (don't rely on
it being implemented with cntlzw), and you're not handling the MMUv2
case.

Plus, whitespace.

> +		if (camsize > align)
> +			camsize = align;
> +
> +		if (camsize > max_cam)
> +			camsize = max_cam;

What about min_cam?

> +	}
> +
> +}

Whitespace.

> +
>  void pci_init_board(void)
>  {
> -	struct fsl_pci_info pci_info;
> +	struct pci_controller *pci_hoses;
>  	const void *fdt = get_fdt();
>  	int pci_node;
> +	int pci_num = 0;
> +	int pci_count;
> +	const char *compat = "fsl,mpc8540-pci";
> +	ulong map_addr;
>  
>  	puts("\n");
>  
> -	pci_node = fdt_path_offset(fdt, "/pci");
> -	if (pci_node < 0) {
> +	/* Start MMIO and PIO range maps above RAM */
> +	map_addr = CONFIG_MAX_MEM_MAPPED;
> +
> +	/* Count and allocate PCI buses */
> +	pci_count = myfdt_count_compatibles(fdt, compat);
> +
> +	if (pci_count) {
> +		pci_hoses = malloc(sizeof(struct pci_controller) * pci_count);
> +	} else {
>  		printf("PCI: disabled\n\n");
>  		return;
>  	}
>  
> -	SET_STD_PCI_INFO(pci_info, 1);
> -
> -	fsl_setup_hose(&pci1_hose, pci_info.regs);
> -	printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
> -		pci_info.regs);
> -
> -	fsl_pci_init_port(&pci_info, &pci1_hose, 0);
> +	/* Spawn PCI buses based on device tree */
> +	pci_node = fdt_node_offset_by_compatible(fdt, -1, compat);
> +	while (pci_node != -FDT_ERR_NOTFOUND) {
> +		struct fsl_pci_info pci_info = { };
> +		uint64_t phys_addr;
> +		int pnode = fdt_parent_offset(fdt, pci_node);
> +		int paddress_cells;
> +		int address_cells;
> +		int size_cells;
> +		int off = 0;
> +
> +		paddress_cells = myfdt_one_cell(fdt, pnode, "#address-cells", 1);
> +		address_cells = myfdt_one_cell(fdt, pci_node, "#address-cells", 1);
> +		size_cells = myfdt_one_cell(fdt, pci_node, "#size-cells", 1);
> +
> +		pci_info.regs = myfdt_readcells(fdt, pci_node, "reg",
> +						paddress_cells, 0, 0);
> +
> +		/* MMIO range */
> +		off += address_cells;
> +		phys_addr = myfdt_readcells(fdt, pci_node, "ranges",
> +					    paddress_cells, off, 0);
> +		off += paddress_cells;
> +		pci_info.mem_size = myfdt_readcells(fdt, pci_node, "ranges",
> +			size_cells, off, 0);
> +		off += size_cells;
> +
> +		/* Align virtual region */
> +		map_addr += pci_info.mem_size - 1;
> +		map_addr &= ~(pci_info.mem_size - 1);
> +		/* Map virtual memory for MMIO range */
> +		map_tlb1_io(map_addr, phys_addr, pci_info.mem_size);
> +		pci_info.mem_bus = phys_addr;
> +		pci_info.mem_phys = phys_addr;
> +		map_addr += pci_info.mem_size;
> +
> +		/* PIO range */
> +		off += address_cells;
> +		pci_info.io_phys = myfdt_readcells(fdt, pci_node, "ranges",
> +			paddress_cells, off, 0);
> +		off += paddress_cells;
> +		pci_info.io_size = myfdt_readcells(fdt, pci_node, "ranges",
> +			size_cells, off, 0);
> +
> +		/* Align virtual region */
> +		map_addr += pci_info.io_size - 1;
> +		map_addr &= ~(pci_info.io_size - 1);
> +		/* Map virtual memory for MMIO range */
> +		map_tlb1_io(map_addr, pci_info.io_phys, pci_info.io_size);
> +		pci_info.io_bus = map_addr;
> +		map_addr += pci_info.io_size;
> +
> +		/* Instantiate */
> +		pci_info.pci_num = pci_num + 1;
> +
> +		fsl_setup_hose(&pci_hoses[pci_num], pci_info.regs);
> +		printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
> +			pci_info.regs);
> +
> +		fsl_pci_init_port(&pci_info, &pci_hoses[pci_num], pci_num);
> +
> +		/* Jump to next PCI node */
> +		pci_node = fdt_node_offset_by_compatible(fdt, pci_node, compat);
> +		pci_num++;
> +	}
>  
>  	puts("\n");
>  }

How about using fdt_translate_address() or other existing functionality?

-Scott

[U-Boot] [PATCH v2 5/6] PPC 85xx: Find PCI host controllers on ppce500 from device tree

[Alexander Graf]

On 04.02.2014, at 03:47, Scott Wood <scottwood@freescale.com> wrote:

> On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
>> The definition of our ppce500 PV machine is that every address is dynamically
>> determined through device tree bindings.
>> 
>> So don't hardcode where PCI devices are in our physical memory layout but instead
>> read them dynamically from the device tree we get passed on boot.
> 
> Would it be difficult to make the QEMU emulation properly implement
> access windows?

What are access windows? You mean BAR region offsets? Not too hard I suppose, but it adds complexity which we were trying to avoid, no?

> 
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> board/freescale/qemu-ppce500/qemu-ppce500.c |  193 ++++++++++++++++++++++++---
>> board/freescale/qemu-ppce500/tlb.c          |   13 --
>> include/configs/qemu-ppce500.h              |   12 --
>> 3 files changed, 175 insertions(+), 43 deletions(-)
>> 
>> diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c b/board/freescale/qemu-ppce500/qemu-ppce500.c
>> index 6491ae9..5d4dd64 100644
>> --- a/board/freescale/qemu-ppce500/qemu-ppce500.c
>> +++ b/board/freescale/qemu-ppce500/qemu-ppce500.c
>> @@ -19,7 +19,51 @@
>> #include <malloc.h>
>> 
>> DECLARE_GLOBAL_DATA_PTR;
>> -static struct pci_controller pci1_hose;
>> +
>> +static uint64_t myfdt_readcells(const void *fdt, int node, const char *property,
>> +				int num, int off, uint64_t defval)
> 
> "my" fdt?

Yeah, didn't want to clash with the fdt namespace.

> 
>> +{
>> +	int len;
>> +	const uint32_t *prop;
>> +
>> +	prop = fdt_getprop(fdt, node, property, &len);
>> +
>> +	if (!prop)
>> +		return defval;
>> +
>> +	if (len < ((off + num) * sizeof(uint32_t)))
>> +		panic("Invalid fdt");
>> +
>> +	prop += off;
>> +
>> +	switch (num) {
>> +	case 1:
>> +		return *prop;
>> +	case 2:
>> +		return *(const uint64_t *)prop;
>> +	}
>> +
> 
> What about this function is specific to qemu-ppce500?

Nothing. But the less common code I touch the less I can break. There seems to be an fdt helper framework that's only targeted at a few ARM devices - not sure what to make of that.

> 
> +	panic("Invalid cell size");
> +}
> 
> s/cell size/cell count/
> 
>> +static uint32_t myfdt_one_cell(const void *fdt, int node, const char *property,
>> +			       uint32_t defval)
>> +{
>> +	return myfdt_readcells(fdt, node, property, 1, 0, defval);
>> +}
> 
> This looks a lot like fdt_getprop_u32_default(), except for "int node"
> versus path.

Well, I use it inside of a loop where I don't have the path :).

> 
>> +static void map_tlb1_io(ulong virt_addr, uint64_t phys_addr, uint64_t size)
>> +{
>> +	unsigned int max_cam, tsize_mask;
>> +	int i;
>> +
>> +	if ((mfspr(SPRN_MMUCFG) & MMUCFG_MAVN) == MMUCFG_MAVN_V1) {
>> +		/* Convert (4^max) kB to (2^max) bytes */
>> +		max_cam = ((mfspr(SPRN_TLB1CFG) >> 16) & 0xf) * 2 + 10;
>> +		tsize_mask = ~1U;
>> +	} else {
>> +		/* Convert (2^max) kB to (2^max) bytes */
>> +		max_cam = __ilog2(mfspr(SPRN_TLB1PS)) + 10;
>> +		tsize_mask = ~0U;
>> +	}
>> +
>> +	for (i = 0; size && i < 8; i++) {
>> +		int tlb_index = find_free_tlbcam();
>> +		u32 camsize = __ilog2_u64(size) & tsize_mask;
>> +		u32 align = __ilog2(virt_addr) & tsize_mask;
>> +		unsigned int tlb_size;
>> +
>> +		if (tlb_index == -1)
>> +			break;
>> +
>> +		if (align == -2) align = max_cam;
> 
> -2?  Besides align being unsigned, if this is meant to handle the case
> where virt_addr is zero, that's undefined for __ilog2() (don't rely on
> it being implemented with cntlzw), and you're not handling the MMUv2
> case.

I merely copied this from tlb.c's setup_ddr_tlbs_phys() and adjusted it slightly to let me choose the target virt address.

Would you prefer if I generalize setup_ddr_tlbs_phys() inside tlb.c and export that function there?

> 
> Plus, whitespace.
> 
>> +		if (camsize > align)
>> +			camsize = align;
>> +
>> +		if (camsize > max_cam)
>> +			camsize = max_cam;
> 
> What about min_cam?
> 
>> +	}
>> +
>> +}
> 
> Whitespace.
> 
>> +
>> void pci_init_board(void)
>> {
>> -	struct fsl_pci_info pci_info;
>> +	struct pci_controller *pci_hoses;
>> 	const void *fdt = get_fdt();
>> 	int pci_node;
>> +	int pci_num = 0;
>> +	int pci_count;
>> +	const char *compat = "fsl,mpc8540-pci";
>> +	ulong map_addr;
>> 
>> 	puts("\n");
>> 
>> -	pci_node = fdt_path_offset(fdt, "/pci");
>> -	if (pci_node < 0) {
>> +	/* Start MMIO and PIO range maps above RAM */
>> +	map_addr = CONFIG_MAX_MEM_MAPPED;
>> +
>> +	/* Count and allocate PCI buses */
>> +	pci_count = myfdt_count_compatibles(fdt, compat);
>> +
>> +	if (pci_count) {
>> +		pci_hoses = malloc(sizeof(struct pci_controller) * pci_count);
>> +	} else {
>> 		printf("PCI: disabled\n\n");
>> 		return;
>> 	}
>> 
>> -	SET_STD_PCI_INFO(pci_info, 1);
>> -
>> -	fsl_setup_hose(&pci1_hose, pci_info.regs);
>> -	printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
>> -		pci_info.regs);
>> -
>> -	fsl_pci_init_port(&pci_info, &pci1_hose, 0);
>> +	/* Spawn PCI buses based on device tree */
>> +	pci_node = fdt_node_offset_by_compatible(fdt, -1, compat);
>> +	while (pci_node != -FDT_ERR_NOTFOUND) {
>> +		struct fsl_pci_info pci_info = { };
>> +		uint64_t phys_addr;
>> +		int pnode = fdt_parent_offset(fdt, pci_node);
>> +		int paddress_cells;
>> +		int address_cells;
>> +		int size_cells;
>> +		int off = 0;
>> +
>> +		paddress_cells = myfdt_one_cell(fdt, pnode, "#address-cells", 1);
>> +		address_cells = myfdt_one_cell(fdt, pci_node, "#address-cells", 1);
>> +		size_cells = myfdt_one_cell(fdt, pci_node, "#size-cells", 1);
>> +
>> +		pci_info.regs = myfdt_readcells(fdt, pci_node, "reg",
>> +						paddress_cells, 0, 0);
>> +
>> +		/* MMIO range */
>> +		off += address_cells;
>> +		phys_addr = myfdt_readcells(fdt, pci_node, "ranges",
>> +					    paddress_cells, off, 0);
>> +		off += paddress_cells;
>> +		pci_info.mem_size = myfdt_readcells(fdt, pci_node, "ranges",
>> +			size_cells, off, 0);
>> +		off += size_cells;
>> +
>> +		/* Align virtual region */
>> +		map_addr += pci_info.mem_size - 1;
>> +		map_addr &= ~(pci_info.mem_size - 1);
>> +		/* Map virtual memory for MMIO range */
>> +		map_tlb1_io(map_addr, phys_addr, pci_info.mem_size);
>> +		pci_info.mem_bus = phys_addr;
>> +		pci_info.mem_phys = phys_addr;
>> +		map_addr += pci_info.mem_size;
>> +
>> +		/* PIO range */
>> +		off += address_cells;
>> +		pci_info.io_phys = myfdt_readcells(fdt, pci_node, "ranges",
>> +			paddress_cells, off, 0);
>> +		off += paddress_cells;
>> +		pci_info.io_size = myfdt_readcells(fdt, pci_node, "ranges",
>> +			size_cells, off, 0);
>> +
>> +		/* Align virtual region */
>> +		map_addr += pci_info.io_size - 1;
>> +		map_addr &= ~(pci_info.io_size - 1);
>> +		/* Map virtual memory for MMIO range */
>> +		map_tlb1_io(map_addr, pci_info.io_phys, pci_info.io_size);
>> +		pci_info.io_bus = map_addr;
>> +		map_addr += pci_info.io_size;
>> +
>> +		/* Instantiate */
>> +		pci_info.pci_num = pci_num + 1;
>> +
>> +		fsl_setup_hose(&pci_hoses[pci_num], pci_info.regs);
>> +		printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
>> +			pci_info.regs);
>> +
>> +		fsl_pci_init_port(&pci_info, &pci_hoses[pci_num], pci_num);
>> +
>> +		/* Jump to next PCI node */
>> +		pci_node = fdt_node_offset_by_compatible(fdt, pci_node, compat);
>> +		pci_num++;
>> +	}
>> 
>> 	puts("\n");
>> }
> 
> How about using fdt_translate_address() or other existing functionality?

Mind to show exactly how?


Alex

[U-Boot] [PATCH v2 5/6] PPC 85xx: Find PCI host controllers on ppce500 from device tree

[Scott Wood]

On Thu, 2014-02-06 at 14:26 +0100, Alexander Graf wrote:
> On 04.02.2014, at 03:47, Scott Wood <scottwood@freescale.com> wrote:
> 
> > On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
> >> The definition of our ppce500 PV machine is that every address is dynamically
> >> determined through device tree bindings.
> >> 
> >> So don't hardcode where PCI devices are in our physical memory layout but instead
> >> read them dynamically from the device tree we get passed on boot.
> > 
> > Would it be difficult to make the QEMU emulation properly implement
> > access windows?
> 
> What are access windows? You mean BAR region offsets? Not too hard I
> suppose, but it adds complexity which we were trying to avoid, no?

It would remove U-Boot complexity (unlike the LAW stuff where we just
skip it) because we wouldn't need to care about QEMU's default settings.
It should be easier to do than LAW support, and more useful (e.g. Linux
currently programs this as well, we just get lucky that it misuses the
device tree as configuration information).
 
> >> +{
> >> +	int len;
> >> +	const uint32_t *prop;
> >> +
> >> +	prop = fdt_getprop(fdt, node, property, &len);
> >> +
> >> +	if (!prop)
> >> +		return defval;
> >> +
> >> +	if (len < ((off + num) * sizeof(uint32_t)))
> >> +		panic("Invalid fdt");
> >> +
> >> +	prop += off;
> >> +
> >> +	switch (num) {
> >> +	case 1:
> >> +		return *prop;
> >> +	case 2:
> >> +		return *(const uint64_t *)prop;
> >> +	}
> >> +
> > 
> > What about this function is specific to qemu-ppce500?
> 
> Nothing. But the less common code I touch the less I can break.

The more that line of thought is applied, the uglier the codebase we'll
end up with. :-)

>  There seems to be an fdt helper framework that's only targeted at a few ARM
> devices - not sure what to make of that.

Make use of whatever parts you can, and extend it with the missing bits
you need.  There's also common/fdt_support.c which is definitely not
just used by ARM.

> > +	panic("Invalid cell size");
> > +}
> > 
> > s/cell size/cell count/
> > 
> >> +static uint32_t myfdt_one_cell(const void *fdt, int node, const char *property,
> >> +			       uint32_t defval)
> >> +{
> >> +	return myfdt_readcells(fdt, node, property, 1, 0, defval);
> >> +}
> > 
> > This looks a lot like fdt_getprop_u32_default(), except for "int node"
> > versus path.
> 
> Well, I use it inside of a loop where I don't have the path :).

It still indicates where the proper place for code like this is. :-)

> >> +static void map_tlb1_io(ulong virt_addr, uint64_t phys_addr, uint64_t size)
> >> +{
> >> +	unsigned int max_cam, tsize_mask;
> >> +	int i;
> >> +
> >> +	if ((mfspr(SPRN_MMUCFG) & MMUCFG_MAVN) == MMUCFG_MAVN_V1) {
> >> +		/* Convert (4^max) kB to (2^max) bytes */
> >> +		max_cam = ((mfspr(SPRN_TLB1CFG) >> 16) & 0xf) * 2 + 10;
> >> +		tsize_mask = ~1U;
> >> +	} else {
> >> +		/* Convert (2^max) kB to (2^max) bytes */
> >> +		max_cam = __ilog2(mfspr(SPRN_TLB1PS)) + 10;
> >> +		tsize_mask = ~0U;
> >> +	}
> >> +
> >> +	for (i = 0; size && i < 8; i++) {
> >> +		int tlb_index = find_free_tlbcam();
> >> +		u32 camsize = __ilog2_u64(size) & tsize_mask;
> >> +		u32 align = __ilog2(virt_addr) & tsize_mask;
> >> +		unsigned int tlb_size;
> >> +
> >> +		if (tlb_index == -1)
> >> +			break;
> >> +
> >> +		if (align == -2) align = max_cam;
> > 
> > -2?  Besides align being unsigned, if this is meant to handle the case
> > where virt_addr is zero, that's undefined for __ilog2() (don't rely on
> > it being implemented with cntlzw), and you're not handling the MMUv2
> > case.
> 
> I merely copied this from tlb.c's setup_ddr_tlbs_phys() and adjusted it
> slightly to let me choose the target virt address.
> 
> Would you prefer if I generalize setup_ddr_tlbs_phys() inside tlb.c and
> export that function there?

Yes.

And maybe fix that align == -2 bug while you're at it. :-)

> >> void pci_init_board(void)
> >> {
> >> -	struct fsl_pci_info pci_info;
> >> +	struct pci_controller *pci_hoses;
> >> 	const void *fdt = get_fdt();
> >> 	int pci_node;
> >> +	int pci_num = 0;
> >> +	int pci_count;
> >> +	const char *compat = "fsl,mpc8540-pci";
> >> +	ulong map_addr;
> >> 
> >> 	puts("\n");
> >> 
> >> -	pci_node = fdt_path_offset(fdt, "/pci");
> >> -	if (pci_node < 0) {
> >> +	/* Start MMIO and PIO range maps above RAM */
> >> +	map_addr = CONFIG_MAX_MEM_MAPPED;
> >> +
> >> +	/* Count and allocate PCI buses */
> >> +	pci_count = myfdt_count_compatibles(fdt, compat);
> >> +
> >> +	if (pci_count) {
> >> +		pci_hoses = malloc(sizeof(struct pci_controller) * pci_count);
> >> +	} else {
> >> 		printf("PCI: disabled\n\n");
> >> 		return;
> >> 	}
> >> 
> >> -	SET_STD_PCI_INFO(pci_info, 1);
> >> -
> >> -	fsl_setup_hose(&pci1_hose, pci_info.regs);
> >> -	printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
> >> -		pci_info.regs);
> >> -
> >> -	fsl_pci_init_port(&pci_info, &pci1_hose, 0);
> >> +	/* Spawn PCI buses based on device tree */
> >> +	pci_node = fdt_node_offset_by_compatible(fdt, -1, compat);
> >> +	while (pci_node != -FDT_ERR_NOTFOUND) {
> >> +		struct fsl_pci_info pci_info = { };
> >> +		uint64_t phys_addr;
> >> +		int pnode = fdt_parent_offset(fdt, pci_node);
> >> +		int paddress_cells;
> >> +		int address_cells;
> >> +		int size_cells;
> >> +		int off = 0;
> >> +
> >> +		paddress_cells = myfdt_one_cell(fdt, pnode, "#address-cells", 1);
> >> +		address_cells = myfdt_one_cell(fdt, pci_node, "#address-cells", 1);
> >> +		size_cells = myfdt_one_cell(fdt, pci_node, "#size-cells", 1);
> >> +
> >> +		pci_info.regs = myfdt_readcells(fdt, pci_node, "reg",
> >> +						paddress_cells, 0, 0);
> >> +
> >> +		/* MMIO range */
> >> +		off += address_cells;
> >> +		phys_addr = myfdt_readcells(fdt, pci_node, "ranges",
> >> +					    paddress_cells, off, 0);
> >> +		off += paddress_cells;
> >> +		pci_info.mem_size = myfdt_readcells(fdt, pci_node, "ranges",
> >> +			size_cells, off, 0);
> >> +		off += size_cells;
> >> +
> >> +		/* Align virtual region */
> >> +		map_addr += pci_info.mem_size - 1;
> >> +		map_addr &= ~(pci_info.mem_size - 1);
> >> +		/* Map virtual memory for MMIO range */
> >> +		map_tlb1_io(map_addr, phys_addr, pci_info.mem_size);
> >> +		pci_info.mem_bus = phys_addr;
> >> +		pci_info.mem_phys = phys_addr;
> >> +		map_addr += pci_info.mem_size;
> >> +
> >> +		/* PIO range */
> >> +		off += address_cells;
> >> +		pci_info.io_phys = myfdt_readcells(fdt, pci_node, "ranges",
> >> +			paddress_cells, off, 0);
> >> +		off += paddress_cells;
> >> +		pci_info.io_size = myfdt_readcells(fdt, pci_node, "ranges",
> >> +			size_cells, off, 0);
> >> +
> >> +		/* Align virtual region */
> >> +		map_addr += pci_info.io_size - 1;
> >> +		map_addr &= ~(pci_info.io_size - 1);
> >> +		/* Map virtual memory for MMIO range */
> >> +		map_tlb1_io(map_addr, pci_info.io_phys, pci_info.io_size);
> >> +		pci_info.io_bus = map_addr;
> >> +		map_addr += pci_info.io_size;
> >> +
> >> +		/* Instantiate */
> >> +		pci_info.pci_num = pci_num + 1;
> >> +
> >> +		fsl_setup_hose(&pci_hoses[pci_num], pci_info.regs);
> >> +		printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
> >> +			pci_info.regs);
> >> +
> >> +		fsl_pci_init_port(&pci_info, &pci_hoses[pci_num], pci_num);
> >> +
> >> +		/* Jump to next PCI node */
> >> +		pci_node = fdt_node_offset_by_compatible(fdt, pci_node, compat);
> >> +		pci_num++;
> >> +	}
> >> 
> >> 	puts("\n");
> >> }
> > 
> > How about using fdt_translate_address() or other existing functionality?
> 
> Mind to show exactly how?

I guess you can't use that when you don't know the bus address, but
still this is the wrong place to implement it.  If getting PCI range
info from the device tree is something we want to do, then it should be
a new common code helper.

-Scott

[U-Boot] [PATCH v2 5/6] PPC 85xx: Find PCI host controllers on ppce500 from device tree

[Alexander Graf]

On 06.02.2014, at 23:52, Scott Wood <scottwood@freescale.com> wrote:

> On Thu, 2014-02-06 at 14:26 +0100, Alexander Graf wrote:
>> On 04.02.2014, at 03:47, Scott Wood <scottwood@freescale.com> wrote:
>> 
>>> On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
>>>> The definition of our ppce500 PV machine is that every address is dynamically
>>>> determined through device tree bindings.
>>>> 
>>>> So don't hardcode where PCI devices are in our physical memory layout but instead
>>>> read them dynamically from the device tree we get passed on boot.
>>> 
>>> Would it be difficult to make the QEMU emulation properly implement
>>> access windows?
>> 
>> What are access windows? You mean BAR region offsets? Not too hard I
>> suppose, but it adds complexity which we were trying to avoid, no?
> 
> It would remove U-Boot complexity (unlike the LAW stuff where we just
> skip it) because we wouldn't need to care about QEMU's default settings.
> It should be easier to do than LAW support, and more useful (e.g. Linux
> currently programs this as well, we just get lucky that it misuses the
> device tree as configuration information).

What complexity would it remove? We would still need to find the configuration space for the access windows, configure them and then even go as far as modifying the original device tree so we expose the new windows.

> 
>>>> +{
>>>> +	int len;
>>>> +	const uint32_t *prop;
>>>> +
>>>> +	prop = fdt_getprop(fdt, node, property, &len);
>>>> +
>>>> +	if (!prop)
>>>> +		return defval;
>>>> +
>>>> +	if (len < ((off + num) * sizeof(uint32_t)))
>>>> +		panic("Invalid fdt");
>>>> +
>>>> +	prop += off;
>>>> +
>>>> +	switch (num) {
>>>> +	case 1:
>>>> +		return *prop;
>>>> +	case 2:
>>>> +		return *(const uint64_t *)prop;
>>>> +	}
>>>> +
>>> 
>>> What about this function is specific to qemu-ppce500?
>> 
>> Nothing. But the less common code I touch the less I can break.
> 
> The more that line of thought is applied, the uglier the codebase we'll
> end up with. :-)
> 
>> There seems to be an fdt helper framework that's only targeted at a few ARM
>> devices - not sure what to make of that.
> 
> Make use of whatever parts you can, and extend it with the missing bits
> you need.  There's also common/fdt_support.c which is definitely not
> just used by ARM.
> 
>>> +	panic("Invalid cell size");
>>> +}
>>> 
>>> s/cell size/cell count/
>>> 
>>>> +static uint32_t myfdt_one_cell(const void *fdt, int node, const char *property,
>>>> +			       uint32_t defval)
>>>> +{
>>>> +	return myfdt_readcells(fdt, node, property, 1, 0, defval);
>>>> +}
>>> 
>>> This looks a lot like fdt_getprop_u32_default(), except for "int node"
>>> versus path.
>> 
>> Well, I use it inside of a loop where I don't have the path :).
> 
> It still indicates where the proper place for code like this is. :-)
> 
>>>> +static void map_tlb1_io(ulong virt_addr, uint64_t phys_addr, uint64_t size)
>>>> +{
>>>> +	unsigned int max_cam, tsize_mask;
>>>> +	int i;
>>>> +
>>>> +	if ((mfspr(SPRN_MMUCFG) & MMUCFG_MAVN) == MMUCFG_MAVN_V1) {
>>>> +		/* Convert (4^max) kB to (2^max) bytes */
>>>> +		max_cam = ((mfspr(SPRN_TLB1CFG) >> 16) & 0xf) * 2 + 10;
>>>> +		tsize_mask = ~1U;
>>>> +	} else {
>>>> +		/* Convert (2^max) kB to (2^max) bytes */
>>>> +		max_cam = __ilog2(mfspr(SPRN_TLB1PS)) + 10;
>>>> +		tsize_mask = ~0U;
>>>> +	}
>>>> +
>>>> +	for (i = 0; size && i < 8; i++) {
>>>> +		int tlb_index = find_free_tlbcam();
>>>> +		u32 camsize = __ilog2_u64(size) & tsize_mask;
>>>> +		u32 align = __ilog2(virt_addr) & tsize_mask;
>>>> +		unsigned int tlb_size;
>>>> +
>>>> +		if (tlb_index == -1)
>>>> +			break;
>>>> +
>>>> +		if (align == -2) align = max_cam;
>>> 
>>> -2?  Besides align being unsigned, if this is meant to handle the case
>>> where virt_addr is zero, that's undefined for __ilog2() (don't rely on
>>> it being implemented with cntlzw), and you're not handling the MMUv2
>>> case.
>> 
>> I merely copied this from tlb.c's setup_ddr_tlbs_phys() and adjusted it
>> slightly to let me choose the target virt address.
>> 
>> Would you prefer if I generalize setup_ddr_tlbs_phys() inside tlb.c and
>> export that function there?
> 
> Yes.
> 
> And maybe fix that align == -2 bug while you're at it. :-)

I'll leave that to you :P


Alex

[U-Boot] [PATCH v2 5/6] PPC 85xx: Find PCI host controllers on ppce500 from device tree

[Scott Wood]

On Fri, 2014-02-07 at 13:25 +0100, Alexander Graf wrote:
> On 06.02.2014, at 23:52, Scott Wood <scottwood@freescale.com> wrote:
> 
> > On Thu, 2014-02-06 at 14:26 +0100, Alexander Graf wrote:
> >> On 04.02.2014, at 03:47, Scott Wood <scottwood@freescale.com> wrote:
> >> 
> >>> On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
> >>>> The definition of our ppce500 PV machine is that every address is dynamically
> >>>> determined through device tree bindings.
> >>>> 
> >>>> So don't hardcode where PCI devices are in our physical memory layout but instead
> >>>> read them dynamically from the device tree we get passed on boot.
> >>> 
> >>> Would it be difficult to make the QEMU emulation properly implement
> >>> access windows?
> >> 
> >> What are access windows? You mean BAR region offsets? Not too hard I
> >> suppose, but it adds complexity which we were trying to avoid, no?
> > 
> > It would remove U-Boot complexity (unlike the LAW stuff where we just
> > skip it) because we wouldn't need to care about QEMU's default settings.
> > It should be easier to do than LAW support, and more useful (e.g. Linux
> > currently programs this as well, we just get lucky that it misuses the
> > device tree as configuration information).
> 
> What complexity would it remove?

Getting the PCI translation window addresses from the device tree.

> We would still need to find the configuration space for the access
> windows,

That's easier -- just a standard reg lookup.

> configure them

That code's already there.

> and then even go as far as modifying the original device tree so we
> expose the new windows.

It would be nice if we did this regardless of QEMU.  As is, IIRC we have
some device trees that don't match what U-Boot does, which works (at
least in Linux) because Linux reprograms it to match the device tree.

-Scott

[U-Boot] [PATCH v2 5/6] PPC 85xx: Find PCI host controllers on ppce500 from device tree

[Alexander Graf]

On 06.02.2014, at 23:52, Scott Wood <scottwood@freescale.com> wrote:

> On Thu, 2014-02-06 at 14:26 +0100, Alexander Graf wrote:
>> On 04.02.2014, at 03:47, Scott Wood <scottwood@freescale.com> wrote:
>> 
>>> On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
>>>> The definition of our ppce500 PV machine is that every address is dynamically
>>>> determined through device tree bindings.
>>>> 
>>>> So don't hardcode where PCI devices are in our physical memory layout but instead
>>>> read them dynamically from the device tree we get passed on boot.
>>> 
>>> Would it be difficult to make the QEMU emulation properly implement
>>> access windows?
>> 
>> What are access windows? You mean BAR region offsets? Not too hard I
>> suppose, but it adds complexity which we were trying to avoid, no?
> 
> It would remove U-Boot complexity (unlike the LAW stuff where we just
> skip it) because we wouldn't need to care about QEMU's default settings.
> It should be easier to do than LAW support, and more useful (e.g. Linux
> currently programs this as well, we just get lucky that it misuses the
> device tree as configuration information).
> 
>>>> +{
>>>> +	int len;
>>>> +	const uint32_t *prop;
>>>> +
>>>> +	prop = fdt_getprop(fdt, node, property, &len);
>>>> +
>>>> +	if (!prop)
>>>> +		return defval;
>>>> +
>>>> +	if (len < ((off + num) * sizeof(uint32_t)))
>>>> +		panic("Invalid fdt");
>>>> +
>>>> +	prop += off;
>>>> +
>>>> +	switch (num) {
>>>> +	case 1:
>>>> +		return *prop;
>>>> +	case 2:
>>>> +		return *(const uint64_t *)prop;
>>>> +	}
>>>> +
>>> 
>>> What about this function is specific to qemu-ppce500?
>> 
>> Nothing. But the less common code I touch the less I can break.
> 
> The more that line of thought is applied, the uglier the codebase we'll
> end up with. :-)
> 
>> There seems to be an fdt helper framework that's only targeted at a few ARM
>> devices - not sure what to make of that.
> 
> Make use of whatever parts you can, and extend it with the missing bits
> you need.  There's also common/fdt_support.c which is definitely not
> just used by ARM.
> 
>>> +	panic("Invalid cell size");
>>> +}
>>> 
>>> s/cell size/cell count/
>>> 
>>>> +static uint32_t myfdt_one_cell(const void *fdt, int node, const char *property,
>>>> +			       uint32_t defval)
>>>> +{
>>>> +	return myfdt_readcells(fdt, node, property, 1, 0, defval);
>>>> +}
>>> 
>>> This looks a lot like fdt_getprop_u32_default(), except for "int node"
>>> versus path.
>> 
>> Well, I use it inside of a loop where I don't have the path :).
> 
> It still indicates where the proper place for code like this is. :-)
> 
>>>> +static void map_tlb1_io(ulong virt_addr, uint64_t phys_addr, uint64_t size)
>>>> +{
>>>> +	unsigned int max_cam, tsize_mask;
>>>> +	int i;
>>>> +
>>>> +	if ((mfspr(SPRN_MMUCFG) & MMUCFG_MAVN) == MMUCFG_MAVN_V1) {
>>>> +		/* Convert (4^max) kB to (2^max) bytes */
>>>> +		max_cam = ((mfspr(SPRN_TLB1CFG) >> 16) & 0xf) * 2 + 10;
>>>> +		tsize_mask = ~1U;
>>>> +	} else {
>>>> +		/* Convert (2^max) kB to (2^max) bytes */
>>>> +		max_cam = __ilog2(mfspr(SPRN_TLB1PS)) + 10;
>>>> +		tsize_mask = ~0U;
>>>> +	}
>>>> +
>>>> +	for (i = 0; size && i < 8; i++) {
>>>> +		int tlb_index = find_free_tlbcam();
>>>> +		u32 camsize = __ilog2_u64(size) & tsize_mask;
>>>> +		u32 align = __ilog2(virt_addr) & tsize_mask;
>>>> +		unsigned int tlb_size;
>>>> +
>>>> +		if (tlb_index == -1)
>>>> +			break;
>>>> +
>>>> +		if (align == -2) align = max_cam;
>>> 
>>> -2?  Besides align being unsigned, if this is meant to handle the case
>>> where virt_addr is zero, that's undefined for __ilog2() (don't rely on
>>> it being implemented with cntlzw), and you're not handling the MMUv2
>>> case.
>> 
>> I merely copied this from tlb.c's setup_ddr_tlbs_phys() and adjusted it
>> slightly to let me choose the target virt address.
>> 
>> Would you prefer if I generalize setup_ddr_tlbs_phys() inside tlb.c and
>> export that function there?
> 
> Yes.
> 
> And maybe fix that align == -2 bug while you're at it. :-)
> 
>>>> void pci_init_board(void)
>>>> {
>>>> -	struct fsl_pci_info pci_info;
>>>> +	struct pci_controller *pci_hoses;
>>>> 	const void *fdt = get_fdt();
>>>> 	int pci_node;
>>>> +	int pci_num = 0;
>>>> +	int pci_count;
>>>> +	const char *compat = "fsl,mpc8540-pci";
>>>> +	ulong map_addr;
>>>> 
>>>> 	puts("\n");
>>>> 
>>>> -	pci_node = fdt_path_offset(fdt, "/pci");
>>>> -	if (pci_node < 0) {
>>>> +	/* Start MMIO and PIO range maps above RAM */
>>>> +	map_addr = CONFIG_MAX_MEM_MAPPED;
>>>> +
>>>> +	/* Count and allocate PCI buses */
>>>> +	pci_count = myfdt_count_compatibles(fdt, compat);
>>>> +
>>>> +	if (pci_count) {
>>>> +		pci_hoses = malloc(sizeof(struct pci_controller) * pci_count);
>>>> +	} else {
>>>> 		printf("PCI: disabled\n\n");
>>>> 		return;
>>>> 	}
>>>> 
>>>> -	SET_STD_PCI_INFO(pci_info, 1);
>>>> -
>>>> -	fsl_setup_hose(&pci1_hose, pci_info.regs);
>>>> -	printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
>>>> -		pci_info.regs);
>>>> -
>>>> -	fsl_pci_init_port(&pci_info, &pci1_hose, 0);
>>>> +	/* Spawn PCI buses based on device tree */
>>>> +	pci_node = fdt_node_offset_by_compatible(fdt, -1, compat);
>>>> +	while (pci_node != -FDT_ERR_NOTFOUND) {
>>>> +		struct fsl_pci_info pci_info = { };
>>>> +		uint64_t phys_addr;
>>>> +		int pnode = fdt_parent_offset(fdt, pci_node);
>>>> +		int paddress_cells;
>>>> +		int address_cells;
>>>> +		int size_cells;
>>>> +		int off = 0;
>>>> +
>>>> +		paddress_cells = myfdt_one_cell(fdt, pnode, "#address-cells", 1);
>>>> +		address_cells = myfdt_one_cell(fdt, pci_node, "#address-cells", 1);
>>>> +		size_cells = myfdt_one_cell(fdt, pci_node, "#size-cells", 1);
>>>> +
>>>> +		pci_info.regs = myfdt_readcells(fdt, pci_node, "reg",
>>>> +						paddress_cells, 0, 0);
>>>> +
>>>> +		/* MMIO range */
>>>> +		off += address_cells;
>>>> +		phys_addr = myfdt_readcells(fdt, pci_node, "ranges",
>>>> +					    paddress_cells, off, 0);
>>>> +		off += paddress_cells;
>>>> +		pci_info.mem_size = myfdt_readcells(fdt, pci_node, "ranges",
>>>> +			size_cells, off, 0);
>>>> +		off += size_cells;
>>>> +
>>>> +		/* Align virtual region */
>>>> +		map_addr += pci_info.mem_size - 1;
>>>> +		map_addr &= ~(pci_info.mem_size - 1);
>>>> +		/* Map virtual memory for MMIO range */
>>>> +		map_tlb1_io(map_addr, phys_addr, pci_info.mem_size);
>>>> +		pci_info.mem_bus = phys_addr;
>>>> +		pci_info.mem_phys = phys_addr;
>>>> +		map_addr += pci_info.mem_size;
>>>> +
>>>> +		/* PIO range */
>>>> +		off += address_cells;
>>>> +		pci_info.io_phys = myfdt_readcells(fdt, pci_node, "ranges",
>>>> +			paddress_cells, off, 0);
>>>> +		off += paddress_cells;
>>>> +		pci_info.io_size = myfdt_readcells(fdt, pci_node, "ranges",
>>>> +			size_cells, off, 0);
>>>> +
>>>> +		/* Align virtual region */
>>>> +		map_addr += pci_info.io_size - 1;
>>>> +		map_addr &= ~(pci_info.io_size - 1);
>>>> +		/* Map virtual memory for MMIO range */
>>>> +		map_tlb1_io(map_addr, pci_info.io_phys, pci_info.io_size);
>>>> +		pci_info.io_bus = map_addr;
>>>> +		map_addr += pci_info.io_size;
>>>> +
>>>> +		/* Instantiate */
>>>> +		pci_info.pci_num = pci_num + 1;
>>>> +
>>>> +		fsl_setup_hose(&pci_hoses[pci_num], pci_info.regs);
>>>> +		printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
>>>> +			pci_info.regs);
>>>> +
>>>> +		fsl_pci_init_port(&pci_info, &pci_hoses[pci_num], pci_num);
>>>> +
>>>> +		/* Jump to next PCI node */
>>>> +		pci_node = fdt_node_offset_by_compatible(fdt, pci_node, compat);
>>>> +		pci_num++;
>>>> +	}
>>>> 
>>>> 	puts("\n");
>>>> }
>>> 
>>> How about using fdt_translate_address() or other existing functionality?
>> 
>> Mind to show exactly how?
> 
> I guess you can't use that when you don't know the bus address, but
> still this is the wrong place to implement it.  If getting PCI range
> info from the device tree is something we want to do, then it should be
> a new common code helper.

The more I think about this the less of an idea I have how to do any generic API for this at all. And I'm not convinced anything generic is going to help anyone. Do a git grep on fdt_translate_address over the code base and you will find a _single_ caller.

So even if we come up with something now, nobody's going to use it.


Alex

[U-Boot] [Qemu-ppc] [PATCH v2 5/6] PPC 85xx: Find PCI host controllers on ppce500 from device tree

[Alexander Graf]

On 07.02.2014, at 15:54, Alexander Graf <agraf@suse.de> wrote:

> 
> On 06.02.2014, at 23:52, Scott Wood <scottwood@freescale.com> wrote:
> 
>> On Thu, 2014-02-06 at 14:26 +0100, Alexander Graf wrote:
>>>> 
>>>> How about using fdt_translate_address() or other existing functionality?
>>> 
>>> Mind to show exactly how?
>> 
>> I guess you can't use that when you don't know the bus address, but
>> still this is the wrong place to implement it.  If getting PCI range
>> info from the device tree is something we want to do, then it should be
>> a new common code helper.
> 
> The more I think about this the less of an idea I have how to do any generic API for this at all. And I'm not convinced anything generic is going to help anyone. Do a git grep on fdt_translate_address over the code base and you will find a _single_ caller.
> 
> So even if we come up with something now, nobody's going to use it.

Hrm. Maybe fdt_get_base_address() is the answer?


Alex

[U-Boot] [Qemu-ppc] [PATCH v2 5/6] PPC 85xx: Find PCI host controllers on ppce500 from device tree

[Scott Wood]

On Fri, 2014-02-07 at 16:00 +0100, Alexander Graf wrote:
> On 07.02.2014, at 15:54, Alexander Graf <agraf@suse.de> wrote:
> 
> > 
> > On 06.02.2014, at 23:52, Scott Wood <scottwood@freescale.com> wrote:
> > 
> >> On Thu, 2014-02-06 at 14:26 +0100, Alexander Graf wrote:
> >>>> 
> >>>> How about using fdt_translate_address() or other existing functionality?
> >>> 
> >>> Mind to show exactly how?
> >> 
> >> I guess you can't use that when you don't know the bus address, but
> >> still this is the wrong place to implement it.  If getting PCI range
> >> info from the device tree is something we want to do, then it should be
> >> a new common code helper.
> > 
> > The more I think about this the less of an idea I have how to do any generic API for this at all. And I'm not convinced anything generic is going to help anyone. Do a git grep on fdt_translate_address over the code base and you will find a _single_ caller.
> > 
> > So even if we come up with something now, nobody's going to use it.
> 
> Hrm. Maybe fdt_get_base_address() is the answer?

Yes, that looks close to what you want.  It'd have to be extended to
look for the various types of PCI ranges and return a (cpu address, bus
address, size) tuple rather than just a base address.

It's moot though if we can reprogram the PCI windows.

-Scott

[U-Boot] [PATCH v2 6/6] PPC 85xx: Find CPU speed on ppce500 from device tree

[Alexander Graf]

The only thing we know in our PV machine through device tree is the clock
speed of the CPUs. Take that as CPU speed, system speed and ddr speed so that
we have some meaningful values there at all.

The CPU speed is important because our timing loops get determined based on it.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/powerpc/cpu/mpc85xx/Makefile           |    2 ++
 board/freescale/qemu-ppce500/qemu-ppce500.c |   43 +++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/arch/powerpc/cpu/mpc85xx/Makefile b/arch/powerpc/cpu/mpc85xx/Makefile
index ef7637a..4094785 100644
--- a/arch/powerpc/cpu/mpc85xx/Makefile
+++ b/arch/powerpc/cpu/mpc85xx/Makefile
@@ -102,7 +102,9 @@ obj-y	+= cpu.o
 obj-y	+= cpu_init.o
 obj-y	+= cpu_init_early.o
 obj-y	+= interrupts.o
+ifneq ($(CONFIG_QEMU_E500),y)
 obj-y	+= speed.o
+endif
 obj-y	+= tlb.o
 obj-y	+= traps.o
 
diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c b/board/freescale/qemu-ppce500/qemu-ppce500.c
index 5d4dd64..9e9d688 100644
--- a/board/freescale/qemu-ppce500/qemu-ppce500.c
+++ b/board/freescale/qemu-ppce500/qemu-ppce500.c
@@ -407,3 +407,46 @@ void init_laws(void)
 {
 	/* We don't emulate LAWs yet */
 }
+
+static uint32_t get_cpu_freq(void)
+{
+	const void *fdt = get_fdt();
+	int cpus_node = fdt_path_offset(fdt, "/cpus");
+	int cpu_node = fdt_first_subnode(fdt, cpus_node);
+	return myfdt_one_cell(fdt, cpu_node, "clock-frequency", 0);
+}
+
+void get_sys_info(sys_info_t *sys_info)
+{
+	int freq = get_cpu_freq();
+
+	memset(sys_info, 0, sizeof(sys_info_t));
+	sys_info->freq_systembus = freq;
+	sys_info->freq_ddrbus = freq;
+	sys_info->freq_processor[0] = freq;
+}
+
+int get_clocks (void)
+{
+	sys_info_t sys_info;
+
+	get_sys_info(&sys_info);
+
+	gd->cpu_clk = sys_info.freq_processor[0];
+	gd->bus_clk = sys_info.freq_systembus;
+	gd->mem_clk = sys_info.freq_ddrbus;
+	gd->arch.lbc_clk = sys_info.freq_ddrbus;
+
+	return 0;
+}
+
+/********************************************
+ * get_bus_freq
+ * return system bus freq in Hz
+ *********************************************/
+ulong get_bus_freq (ulong dummy)
+{
+	sys_info_t sys_info;
+	get_sys_info(&sys_info);
+	return sys_info.freq_systembus;
+}
-- 
1.7.10.4

[U-Boot] [PATCH v2 6/6] PPC 85xx: Find CPU speed on ppce500 from device tree

[Scott Wood]

On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
> The only thing we know in our PV machine through device tree is the clock
> speed of the CPUs. Take that as CPU speed, system speed and ddr speed so that
> we have some meaningful values there at all.
> 
> The CPU speed is important because our timing loops get determined based on it.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/powerpc/cpu/mpc85xx/Makefile           |    2 ++
>  board/freescale/qemu-ppce500/qemu-ppce500.c |   43 +++++++++++++++++++++++++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/arch/powerpc/cpu/mpc85xx/Makefile b/arch/powerpc/cpu/mpc85xx/Makefile
> index ef7637a..4094785 100644
> --- a/arch/powerpc/cpu/mpc85xx/Makefile
> +++ b/arch/powerpc/cpu/mpc85xx/Makefile
> @@ -102,7 +102,9 @@ obj-y	+= cpu.o
>  obj-y	+= cpu_init.o
>  obj-y	+= cpu_init_early.o
>  obj-y	+= interrupts.o
> +ifneq ($(CONFIG_QEMU_E500),y)
>  obj-y	+= speed.o
> +endif
>  obj-y	+= tlb.o
>  obj-y	+= traps.o
>  
> diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c b/board/freescale/qemu-ppce500/qemu-ppce500.c
> index 5d4dd64..9e9d688 100644
> --- a/board/freescale/qemu-ppce500/qemu-ppce500.c
> +++ b/board/freescale/qemu-ppce500/qemu-ppce500.c
> @@ -407,3 +407,46 @@ void init_laws(void)
>  {
>  	/* We don't emulate LAWs yet */
>  }
> +
> +static uint32_t get_cpu_freq(void)
> +{
> +	const void *fdt = get_fdt();
> +	int cpus_node = fdt_path_offset(fdt, "/cpus");
> +	int cpu_node = fdt_first_subnode(fdt, cpus_node);
> +	return myfdt_one_cell(fdt, cpu_node, "clock-frequency", 0);
> +}
> +
> +void get_sys_info(sys_info_t *sys_info)
> +{
> +	int freq = get_cpu_freq();
> +
> +	memset(sys_info, 0, sizeof(sys_info_t));
> +	sys_info->freq_systembus = freq;
> +	sys_info->freq_ddrbus = freq;
> +	sys_info->freq_processor[0] = freq;
> +}
> +
> +int get_clocks (void)
> +{
> +	sys_info_t sys_info;
> +
> +	get_sys_info(&sys_info);
> +
> +	gd->cpu_clk = sys_info.freq_processor[0];
> +	gd->bus_clk = sys_info.freq_systembus;
> +	gd->mem_clk = sys_info.freq_ddrbus;
> +	gd->arch.lbc_clk = sys_info.freq_ddrbus;
> +
> +	return 0;
> +}

This probably decreases the accuracy of the timebase frequency, since
you'll be basing it on the CPU frequency rather than the bus frequency.

If you're doing this, why not override get_tbclk() as well?

-Scott

[U-Boot] [PATCH v2 6/6] PPC 85xx: Find CPU speed on ppce500 from device tree

[Alexander Graf]

On 04.02.2014, at 03:52, Scott Wood <scottwood@freescale.com> wrote:

> On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
>> The only thing we know in our PV machine through device tree is the clock
>> speed of the CPUs. Take that as CPU speed, system speed and ddr speed so that
>> we have some meaningful values there at all.
>> 
>> The CPU speed is important because our timing loops get determined based on it.
>> 
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> arch/powerpc/cpu/mpc85xx/Makefile           |    2 ++
>> board/freescale/qemu-ppce500/qemu-ppce500.c |   43 +++++++++++++++++++++++++++
>> 2 files changed, 45 insertions(+)
>> 
>> diff --git a/arch/powerpc/cpu/mpc85xx/Makefile b/arch/powerpc/cpu/mpc85xx/Makefile
>> index ef7637a..4094785 100644
>> --- a/arch/powerpc/cpu/mpc85xx/Makefile
>> +++ b/arch/powerpc/cpu/mpc85xx/Makefile
>> @@ -102,7 +102,9 @@ obj-y	+= cpu.o
>> obj-y	+= cpu_init.o
>> obj-y	+= cpu_init_early.o
>> obj-y	+= interrupts.o
>> +ifneq ($(CONFIG_QEMU_E500),y)
>> obj-y	+= speed.o
>> +endif
>> obj-y	+= tlb.o
>> obj-y	+= traps.o
>> 
>> diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c b/board/freescale/qemu-ppce500/qemu-ppce500.c
>> index 5d4dd64..9e9d688 100644
>> --- a/board/freescale/qemu-ppce500/qemu-ppce500.c
>> +++ b/board/freescale/qemu-ppce500/qemu-ppce500.c
>> @@ -407,3 +407,46 @@ void init_laws(void)
>> {
>> 	/* We don't emulate LAWs yet */
>> }
>> +
>> +static uint32_t get_cpu_freq(void)
>> +{
>> +	const void *fdt = get_fdt();
>> +	int cpus_node = fdt_path_offset(fdt, "/cpus");
>> +	int cpu_node = fdt_first_subnode(fdt, cpus_node);
>> +	return myfdt_one_cell(fdt, cpu_node, "clock-frequency", 0);
>> +}
>> +
>> +void get_sys_info(sys_info_t *sys_info)
>> +{
>> +	int freq = get_cpu_freq();
>> +
>> +	memset(sys_info, 0, sizeof(sys_info_t));
>> +	sys_info->freq_systembus = freq;
>> +	sys_info->freq_ddrbus = freq;
>> +	sys_info->freq_processor[0] = freq;
>> +}
>> +
>> +int get_clocks (void)
>> +{
>> +	sys_info_t sys_info;
>> +
>> +	get_sys_info(&sys_info);
>> +
>> +	gd->cpu_clk = sys_info.freq_processor[0];
>> +	gd->bus_clk = sys_info.freq_systembus;
>> +	gd->mem_clk = sys_info.freq_ddrbus;
>> +	gd->arch.lbc_clk = sys_info.freq_ddrbus;
>> +
>> +	return 0;
>> +}
> 
> This probably decreases the accuracy of the timebase frequency, since
> you'll be basing it on the CPU frequency rather than the bus frequency.
> 
> If you're doing this, why not override get_tbclk() as well?

That one only uses variables I already control, no?

unsigned long get_tbclk (void)
{
        unsigned long tbclk_div = CONFIG_SYS_FSL_TBCLK_DIV;

        return (gd->bus_clk + (tbclk_div >> 1)) / tbclk_div;
}


Alex

[U-Boot] [PATCH v2 6/6] PPC 85xx: Find CPU speed on ppce500 from device tree

[Scott Wood]

On Thu, 2014-02-06 at 12:11 +0100, Alexander Graf wrote:
> On 04.02.2014, at 03:52, Scott Wood <scottwood@freescale.com> wrote:
> 
> > On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
> >> The only thing we know in our PV machine through device tree is the clock
> >> speed of the CPUs. Take that as CPU speed, system speed and ddr speed so that
> >> we have some meaningful values there at all.
> >> 
> >> The CPU speed is important because our timing loops get determined based on it.
> >> 
> >> Signed-off-by: Alexander Graf <agraf@suse.de>
> >> ---
> >> arch/powerpc/cpu/mpc85xx/Makefile           |    2 ++
> >> board/freescale/qemu-ppce500/qemu-ppce500.c |   43 +++++++++++++++++++++++++++
> >> 2 files changed, 45 insertions(+)
> >> 
> >> diff --git a/arch/powerpc/cpu/mpc85xx/Makefile b/arch/powerpc/cpu/mpc85xx/Makefile
> >> index ef7637a..4094785 100644
> >> --- a/arch/powerpc/cpu/mpc85xx/Makefile
> >> +++ b/arch/powerpc/cpu/mpc85xx/Makefile
> >> @@ -102,7 +102,9 @@ obj-y	+= cpu.o
> >> obj-y	+= cpu_init.o
> >> obj-y	+= cpu_init_early.o
> >> obj-y	+= interrupts.o
> >> +ifneq ($(CONFIG_QEMU_E500),y)
> >> obj-y	+= speed.o
> >> +endif
> >> obj-y	+= tlb.o
> >> obj-y	+= traps.o
> >> 
> >> diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c b/board/freescale/qemu-ppce500/qemu-ppce500.c
> >> index 5d4dd64..9e9d688 100644
> >> --- a/board/freescale/qemu-ppce500/qemu-ppce500.c
> >> +++ b/board/freescale/qemu-ppce500/qemu-ppce500.c
> >> @@ -407,3 +407,46 @@ void init_laws(void)
> >> {
> >> 	/* We don't emulate LAWs yet */
> >> }
> >> +
> >> +static uint32_t get_cpu_freq(void)
> >> +{
> >> +	const void *fdt = get_fdt();
> >> +	int cpus_node = fdt_path_offset(fdt, "/cpus");
> >> +	int cpu_node = fdt_first_subnode(fdt, cpus_node);
> >> +	return myfdt_one_cell(fdt, cpu_node, "clock-frequency", 0);
> >> +}
> >> +
> >> +void get_sys_info(sys_info_t *sys_info)
> >> +{
> >> +	int freq = get_cpu_freq();
> >> +
> >> +	memset(sys_info, 0, sizeof(sys_info_t));
> >> +	sys_info->freq_systembus = freq;
> >> +	sys_info->freq_ddrbus = freq;
> >> +	sys_info->freq_processor[0] = freq;
> >> +}
> >> +
> >> +int get_clocks (void)
> >> +{
> >> +	sys_info_t sys_info;
> >> +
> >> +	get_sys_info(&sys_info);
> >> +
> >> +	gd->cpu_clk = sys_info.freq_processor[0];
> >> +	gd->bus_clk = sys_info.freq_systembus;
> >> +	gd->mem_clk = sys_info.freq_ddrbus;
> >> +	gd->arch.lbc_clk = sys_info.freq_ddrbus;
> >> +
> >> +	return 0;
> >> +}
> > 
> > This probably decreases the accuracy of the timebase frequency, since
> > you'll be basing it on the CPU frequency rather than the bus frequency.
> > 
> > If you're doing this, why not override get_tbclk() as well?
> 
> That one only uses variables I already control, no?
> 
> unsigned long get_tbclk (void)
> {
>         unsigned long tbclk_div = CONFIG_SYS_FSL_TBCLK_DIV;
> 
>         return (gd->bus_clk + (tbclk_div >> 1)) / tbclk_div;
> }

What did you set CONFIG_SYS_FSL_TBCLK_DIV to?

-Scott

[U-Boot] [PATCH v2 6/6] PPC 85xx: Find CPU speed on ppce500 from device tree

[Alexander Graf]

On 07.02.2014, at 02:33, Scott Wood <scottwood@freescale.com> wrote:

> On Thu, 2014-02-06 at 12:11 +0100, Alexander Graf wrote:
>> On 04.02.2014, at 03:52, Scott Wood <scottwood@freescale.com> wrote:
>> 
>>> On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
>>>> The only thing we know in our PV machine through device tree is the clock
>>>> speed of the CPUs. Take that as CPU speed, system speed and ddr speed so that
>>>> we have some meaningful values there at all.
>>>> 
>>>> The CPU speed is important because our timing loops get determined based on it.
>>>> 
>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>> ---
>>>> arch/powerpc/cpu/mpc85xx/Makefile           |    2 ++
>>>> board/freescale/qemu-ppce500/qemu-ppce500.c |   43 +++++++++++++++++++++++++++
>>>> 2 files changed, 45 insertions(+)
>>>> 
>>>> diff --git a/arch/powerpc/cpu/mpc85xx/Makefile b/arch/powerpc/cpu/mpc85xx/Makefile
>>>> index ef7637a..4094785 100644
>>>> --- a/arch/powerpc/cpu/mpc85xx/Makefile
>>>> +++ b/arch/powerpc/cpu/mpc85xx/Makefile
>>>> @@ -102,7 +102,9 @@ obj-y	+= cpu.o
>>>> obj-y	+= cpu_init.o
>>>> obj-y	+= cpu_init_early.o
>>>> obj-y	+= interrupts.o
>>>> +ifneq ($(CONFIG_QEMU_E500),y)
>>>> obj-y	+= speed.o
>>>> +endif
>>>> obj-y	+= tlb.o
>>>> obj-y	+= traps.o
>>>> 
>>>> diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c b/board/freescale/qemu-ppce500/qemu-ppce500.c
>>>> index 5d4dd64..9e9d688 100644
>>>> --- a/board/freescale/qemu-ppce500/qemu-ppce500.c
>>>> +++ b/board/freescale/qemu-ppce500/qemu-ppce500.c
>>>> @@ -407,3 +407,46 @@ void init_laws(void)
>>>> {
>>>> 	/* We don't emulate LAWs yet */
>>>> }
>>>> +
>>>> +static uint32_t get_cpu_freq(void)
>>>> +{
>>>> +	const void *fdt = get_fdt();
>>>> +	int cpus_node = fdt_path_offset(fdt, "/cpus");
>>>> +	int cpu_node = fdt_first_subnode(fdt, cpus_node);
>>>> +	return myfdt_one_cell(fdt, cpu_node, "clock-frequency", 0);
>>>> +}
>>>> +
>>>> +void get_sys_info(sys_info_t *sys_info)
>>>> +{
>>>> +	int freq = get_cpu_freq();
>>>> +
>>>> +	memset(sys_info, 0, sizeof(sys_info_t));
>>>> +	sys_info->freq_systembus = freq;
>>>> +	sys_info->freq_ddrbus = freq;
>>>> +	sys_info->freq_processor[0] = freq;
>>>> +}
>>>> +
>>>> +int get_clocks (void)
>>>> +{
>>>> +	sys_info_t sys_info;
>>>> +
>>>> +	get_sys_info(&sys_info);
>>>> +
>>>> +	gd->cpu_clk = sys_info.freq_processor[0];
>>>> +	gd->bus_clk = sys_info.freq_systembus;
>>>> +	gd->mem_clk = sys_info.freq_ddrbus;
>>>> +	gd->arch.lbc_clk = sys_info.freq_ddrbus;
>>>> +
>>>> +	return 0;
>>>> +}
>>> 
>>> This probably decreases the accuracy of the timebase frequency, since
>>> you'll be basing it on the CPU frequency rather than the bus frequency.
>>> 
>>> If you're doing this, why not override get_tbclk() as well?
>> 
>> That one only uses variables I already control, no?
>> 
>> unsigned long get_tbclk (void)
>> {
>>        unsigned long tbclk_div = CONFIG_SYS_FSL_TBCLK_DIV;
>> 
>>        return (gd->bus_clk + (tbclk_div >> 1)) / tbclk_div;
>> }
> 
> What did you set CONFIG_SYS_FSL_TBCLK_DIV to?

Nothing which is why it's defaulting to 8.


Alex

[U-Boot] [PATCH v2 6/6] PPC 85xx: Find CPU speed on ppce500 from device tree

[Scott Wood]

On Fri, 2014-02-07 at 12:50 +0100, Alexander Graf wrote:
> On 07.02.2014, at 02:33, Scott Wood <scottwood@freescale.com> wrote:
> 
> > On Thu, 2014-02-06 at 12:11 +0100, Alexander Graf wrote:
> >> On 04.02.2014, at 03:52, Scott Wood <scottwood@freescale.com> wrote:
> >> 
> >>> On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
> >>>> The only thing we know in our PV machine through device tree is the clock
> >>>> speed of the CPUs. Take that as CPU speed, system speed and ddr speed so that
> >>>> we have some meaningful values there at all.
> >>>> 
> >>>> The CPU speed is important because our timing loops get determined based on it.
> >>>> 
> >>>> Signed-off-by: Alexander Graf <agraf@suse.de>
> >>>> ---
> >>>> arch/powerpc/cpu/mpc85xx/Makefile           |    2 ++
> >>>> board/freescale/qemu-ppce500/qemu-ppce500.c |   43 +++++++++++++++++++++++++++
> >>>> 2 files changed, 45 insertions(+)
> >>>> 
> >>>> diff --git a/arch/powerpc/cpu/mpc85xx/Makefile b/arch/powerpc/cpu/mpc85xx/Makefile
> >>>> index ef7637a..4094785 100644
> >>>> --- a/arch/powerpc/cpu/mpc85xx/Makefile
> >>>> +++ b/arch/powerpc/cpu/mpc85xx/Makefile
> >>>> @@ -102,7 +102,9 @@ obj-y	+= cpu.o
> >>>> obj-y	+= cpu_init.o
> >>>> obj-y	+= cpu_init_early.o
> >>>> obj-y	+= interrupts.o
> >>>> +ifneq ($(CONFIG_QEMU_E500),y)
> >>>> obj-y	+= speed.o
> >>>> +endif
> >>>> obj-y	+= tlb.o
> >>>> obj-y	+= traps.o
> >>>> 
> >>>> diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c b/board/freescale/qemu-ppce500/qemu-ppce500.c
> >>>> index 5d4dd64..9e9d688 100644
> >>>> --- a/board/freescale/qemu-ppce500/qemu-ppce500.c
> >>>> +++ b/board/freescale/qemu-ppce500/qemu-ppce500.c
> >>>> @@ -407,3 +407,46 @@ void init_laws(void)
> >>>> {
> >>>> 	/* We don't emulate LAWs yet */
> >>>> }
> >>>> +
> >>>> +static uint32_t get_cpu_freq(void)
> >>>> +{
> >>>> +	const void *fdt = get_fdt();
> >>>> +	int cpus_node = fdt_path_offset(fdt, "/cpus");
> >>>> +	int cpu_node = fdt_first_subnode(fdt, cpus_node);
> >>>> +	return myfdt_one_cell(fdt, cpu_node, "clock-frequency", 0);
> >>>> +}
> >>>> +
> >>>> +void get_sys_info(sys_info_t *sys_info)
> >>>> +{
> >>>> +	int freq = get_cpu_freq();
> >>>> +
> >>>> +	memset(sys_info, 0, sizeof(sys_info_t));
> >>>> +	sys_info->freq_systembus = freq;
> >>>> +	sys_info->freq_ddrbus = freq;
> >>>> +	sys_info->freq_processor[0] = freq;
> >>>> +}
> >>>> +
> >>>> +int get_clocks (void)
> >>>> +{
> >>>> +	sys_info_t sys_info;
> >>>> +
> >>>> +	get_sys_info(&sys_info);
> >>>> +
> >>>> +	gd->cpu_clk = sys_info.freq_processor[0];
> >>>> +	gd->bus_clk = sys_info.freq_systembus;
> >>>> +	gd->mem_clk = sys_info.freq_ddrbus;
> >>>> +	gd->arch.lbc_clk = sys_info.freq_ddrbus;
> >>>> +
> >>>> +	return 0;
> >>>> +}
> >>> 
> >>> This probably decreases the accuracy of the timebase frequency, since
> >>> you'll be basing it on the CPU frequency rather than the bus frequency.
> >>> 
> >>> If you're doing this, why not override get_tbclk() as well?
> >> 
> >> That one only uses variables I already control, no?
> >> 
> >> unsigned long get_tbclk (void)
> >> {
> >>        unsigned long tbclk_div = CONFIG_SYS_FSL_TBCLK_DIV;
> >> 
> >>        return (gd->bus_clk + (tbclk_div >> 1)) / tbclk_div;
> >> }
> > 
> > What did you set CONFIG_SYS_FSL_TBCLK_DIV to?
> 
> Nothing which is why it's defaulting to 8.

Hence why I suggested overriding get_tbclk() rather than trying to fake
up a divider (which could be difficult if the CPU clock is not a
multiple of the bus clock).

-Scott