[U-Boot] [PATCH] pci: fix some warnings related to assumptions about
The following commit introduced some warnings associated with using pci_addr_t instead of a proper 32-bit data type.
commit af778c6d9e2b945ee03cbc53bb976238a3374f33 Author: Andrew Sharp andywyse6@gmail.com Date: Wed Aug 1 12:27:16 2012 +0000
pci: fix errant data types and corresponding access functions
On some platforms pci_addr_t is defined as a 64-bit data type so its not proper to use with pci_{read,write}_config_dword.
Signed-off-by: Kumar Gala galak@kernel.crashing.org --- drivers/pci/pci.c | 6 +++--- drivers/pci/pci_auto.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 2a6d0a7..d864f13 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -118,11 +118,11 @@ PCI_WRITE_VIA_DWORD_OP(word, u16, 0x02, 0x0000ffff) void *pci_map_bar(pci_dev_t pdev, int bar, int flags) { pci_addr_t pci_bus_addr; - pci_addr_t bar_response; + u32 bar_response;
/* read BAR address */ pci_read_config_dword(pdev, bar, &bar_response); - pci_bus_addr = bar_response & ~0xf; + pci_bus_addr = (pci_addr_t)(bar_response & ~0xf);
/* * Pass "0" as the length argument to pci_bus_to_virt. The arg @@ -389,7 +389,7 @@ int pci_hose_config_device(struct pci_controller *hose, pci_addr_t mem, unsigned long command) { - pci_addr_t bar_response; + u32 bar_response; unsigned int old_command; pci_addr_t bar_value; pci_size_t bar_size; diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c index ae61e24..cd78030 100644 --- a/drivers/pci/pci_auto.c +++ b/drivers/pci/pci_auto.c @@ -89,7 +89,7 @@ void pciauto_setup_device(struct pci_controller *hose, struct pci_region *prefetch, struct pci_region *io) { - pci_addr_t bar_response; + u32 bar_response; pci_size_t bar_size; u16 cmdstat = 0; int bar, bar_nr = 0;
Hello!
I'm having problem with my U-Boot for a PowerPC system. I think the root cause is that the function get_effective_memsize() (in file arch/powerpc/lib/board.c) returns 0.
ulong get_effective_memsize(void) { #ifndef CONFIG_VERY_BIG_RAM return gd->ram_size; #else /* limit stack to what we can reasonable map */ return ((gd->ram_size > CONFIG_MAX_MEM_MAPPED) ? CONFIG_MAX_MEM_MAPPED : gd->ram_size); #endif }
Since the system have 24GB of RAM I have fiddled around so both CONFIG_MAX_MEM_MAPPED is actually 24GB and also gd->ram_size is 24GB.
Is it just me who have completely missed how U-Boot works or what?
Thanks for help BR Robert
On Sep 19, 2012, at 10:28 AM, Robert Thorhuus wrote:
Hello!
I'm having problem with my U-Boot for a PowerPC system. I think the root cause is that the function get_effective_memsize() (in file arch/powerpc/lib/board.c) returns 0.
ulong get_effective_memsize(void) { #ifndef CONFIG_VERY_BIG_RAM return gd->ram_size; #else /* limit stack to what we can reasonable map */ return ((gd->ram_size > CONFIG_MAX_MEM_MAPPED) ? CONFIG_MAX_MEM_MAPPED : gd->ram_size); #endif }
Since the system have 24GB of RAM I have fiddled around so both CONFIG_MAX_MEM_MAPPED is actually 24GB and also gd->ram_size is 24GB.
Is it just me who have completely missed how U-Boot works or what?
What kinda of PPC system is this?
You probably don't want to change CONFIG_MAX_MEM_MAPPED. The idea is this is how much of your 24GB is directly accessible by u-boot. For large mem systems this is usually 2GB.
- k
-----Original Message----- From: Kumar Gala [mailto:galak@kernel.crashing.org] Sent: den 20 september 2012 19:27 To: Robert Thorhuus Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] arch/powerpc/lib/board.c:get_effective_memsize() for 4GB+ systems
On Sep 19, 2012, at 10:28 AM, Robert Thorhuus wrote:
Hello!
I'm having problem with my U-Boot for a PowerPC system. I
think the root cause is that the function get_effective_memsize() (in file arch/powerpc/lib/board.c) returns 0.
ulong get_effective_memsize(void) { #ifndef CONFIG_VERY_BIG_RAM return gd->ram_size; #else /* limit stack to what we can reasonable map */ return ((gd->ram_size > CONFIG_MAX_MEM_MAPPED) ? CONFIG_MAX_MEM_MAPPED : gd->ram_size); #endif }
Since the system have 24GB of RAM I have fiddled around so
both CONFIG_MAX_MEM_MAPPED is actually 24GB and also gd->ram_size is 24GB.
Is it just me who have completely missed how U-Boot works or what?
What kinda of PPC system is this?
You probably don't want to change CONFIG_MAX_MEM_MAPPED. The idea is this is how much of your 24GB is directly accessible by u-boot. For large mem systems this is usually 2GB.
- k
Thanks for the answer Kumar!
I'm using 64 bit mpc85xx cores.
Mapping the whole memory from the beginning was of course just an initial thought. But for me it looks like the code is just not able to deal with more than 3.99GB mapped. U-Boot will not be able to be relocated.
BR Robert
On Thu, Sep 20, 2012 at 3:24 PM, Robert Thorhuus robert.thorhuus@ericsson.com wrote:
I'm using 64 bit mpc85xx cores.
U-Boot runs in 32-bit mode even on a 64-bit code.
Mapping the whole memory from the beginning was of course just an initial thought. But for me it looks like the code is just not able to deal with more than 3.99GB mapped. U-Boot will not be able to be relocated.
I would be very surprised if U-Boot could handle a value of CONFIG_MAX_MEM_MAPPED larger than 2GB, no matter what. We use the address space above 2GB for I/O addresses, like CCSR and PCI memory. You would need to reshuffle your I/O memory mappings to use a larger value. And even then, we've never tested that, and so who knows what obscure bugs are in the code.
Is there a reason why you need access to more than 2GB of RAM from within U-Boot?
-----Original Message----- From: Tabi Timur-B04825 [mailto:B04825@freescale.com] Sent: den 21 september 2012 00:38 To: Robert Thorhuus Cc: Kumar Gala; u-boot@lists.denx.de Subject: Re: [U-Boot] arch/powerpc/lib/board.c:get_effective_memsize() for 4GB+ systems
On Thu, Sep 20, 2012 at 3:24 PM, Robert Thorhuus robert.thorhuus@ericsson.com wrote:
I'm using 64 bit mpc85xx cores.
U-Boot runs in 32-bit mode even on a 64-bit code.
Mapping the whole memory from the beginning was of course
just an initial thought.
But for me it looks like the code is just not able to deal
with more than 3.99GB mapped. U-Boot will not be able to be relocated.
I would be very surprised if U-Boot could handle a value of CONFIG_MAX_MEM_MAPPED larger than 2GB, no matter what. We use the address space above 2GB for I/O addresses, like CCSR and PCI memory. You would need to reshuffle your I/O memory mappings to use a larger value. And even then, we've never tested that, and so who knows what obscure bugs are in the code.
Is there a reason why you need access to more than 2GB of RAM from within U-Boot?
-- Timur Tabi Linux kernel developer at Freescale
Hello Timur!
Ok. Point taken.
I really see the advantage of keeping this map you have. It is of course more compatible with different OSEs and you do not need to do anything special with 32/64 bit cores. But at the same time you never take advantage of a 64 bit core with this approach.
How should I access my 24GB in U-Boot? So really you are saying that I should have a 2GB map window in my 32-bit address space and then move this window depending on what memory I want to access? A bit cumbersome I must say. But ok.
How about the U-Boot relocation? As I see the code, it is not easy to decide where it should relocate. It will be either relocated to end of RAM or if we have more than 4G it will be at 4GB end. What if you want to place U-Boot at 16MB for instance? Or if you do not want a memory map hole at 4GB just because U-Boot doesn't handle more than that?
Last AND least I just want to say I'm used to creating memory maps in bootloader which holds for the OS. It seems the time has come to annihilate my illusion...
Thanks BR Robert
Robert Thorhuus wrote:
I really see the advantage of keeping this map you have. It is of course more compatible with different OSEs and you do not need to do anything special with 32/64 bit cores. But at the same time you never take advantage of a 64 bit core with this approach.
U-Boot is a boot loader, not an operating system. What is U-Boot supposed to do with more than 2GB of RAM?
How should I access my 24GB in U-Boot?
You should not!
So really you are saying that I should have a 2GB map window in my 32-bit address space and then move this window depending on what memory I want to access? A bit cumbersome I must say. But ok.
Again, you're doing the wrong thing with U-Boot. It's a boot loader. It's supposed to find your OS, load it into memory, and then boot it.
How about the U-Boot relocation?
As I see the code, it is not easy to decide where it should relocate. It will be either relocated to end of RAM or if we have more than 4G it will be at 4GB end.
It relocates to the end of RAM or the end of 2GB, whichever is lower. It ignores all memory above 2GB.
What if you want to place U-Boot at 16MB for instance? Or if you do not want a memory map hole at 4GB just because U-Boot doesn't handle more than that?
Again, you're missing the point about U-boot.
Last AND least I just want to say I'm used to creating memory maps in bootloader which holds for the OS. It seems the time has come to annihilate my illusion...
Yes, please kill it with fire!
-----Original Message----- From: Tabi Timur-B04825 [mailto:B04825@freescale.com] Sent: den 21 september 2012 13:37 To: Robert Thorhuus Cc: Kumar Gala; u-boot@lists.denx.de Subject: Re: [U-Boot] arch/powerpc/lib/board.c:get_effective_memsize() for 4GB+ systems
Robert Thorhuus wrote:
I really see the advantage of keeping this map you have. It is of course more compatible with different OSEs and you do not
need to do
anything special with 32/64 bit cores. But at the same time
you never
take advantage of a 64 bit core with this approach.
U-Boot is a boot loader, not an operating system. What is U-Boot supposed to do with more than 2GB of RAM?
How should I access my 24GB in U-Boot?
You should not!
So really you are saying that I should have a 2GB map window in my 32-bit address space and then move this window depending on what memory I want to access? A bit cumbersome I must say. But ok.
Again, you're doing the wrong thing with U-Boot. It's a boot loader. It's supposed to find your OS, load it into memory, and then boot it.
How about the U-Boot relocation?
As I see the code, it is not easy to decide where it should
relocate.
It will be either relocated to end of RAM or if we have
more than 4G
it will be at 4GB end.
It relocates to the end of RAM or the end of 2GB, whichever is lower. It ignores all memory above 2GB.
What if you want to place U-Boot at 16MB for instance? Or if you do not want a memory map hole at 4GB just because U-Boot
doesn't handle
more than that?
Again, you're missing the point about U-boot.
Last AND least I just want to say I'm used to creating
memory maps in
bootloader which holds for the OS. It seems the time has come to annihilate my illusion...
Yes, please kill it with fire!
-- Timur Tabi Linux kernel developer at Freescale
Hello Timur!
You really used the machine gun there ;)
Ok. I'll just answer your 2GB+ usage question:
Testing! No I will not be using much memory at all for functionality. But the memory needs to be tested. What is your proposal for that then? And I see U-Boot as the first software place for test and debug. Maybe I want to read out RAM contents?
BR Robert
On Sep 21, 2012, at 6:54 AM, Robert Thorhuus wrote:
-----Original Message----- From: Tabi Timur-B04825 [mailto:B04825@freescale.com] Sent: den 21 september 2012 13:37 To: Robert Thorhuus Cc: Kumar Gala; u-boot@lists.denx.de Subject: Re: [U-Boot] arch/powerpc/lib/board.c:get_effective_memsize() for 4GB+ systems
Robert Thorhuus wrote:
I really see the advantage of keeping this map you have. It is of course more compatible with different OSEs and you do not
need to do
anything special with 32/64 bit cores. But at the same time
you never
take advantage of a 64 bit core with this approach.
U-Boot is a boot loader, not an operating system. What is U-Boot supposed to do with more than 2GB of RAM?
How should I access my 24GB in U-Boot?
You should not!
So really you are saying that I should have a 2GB map window in my 32-bit address space and then move this window depending on what memory I want to access? A bit cumbersome I must say. But ok.
Again, you're doing the wrong thing with U-Boot. It's a boot loader. It's supposed to find your OS, load it into memory, and then boot it.
How about the U-Boot relocation?
As I see the code, it is not easy to decide where it should
relocate.
It will be either relocated to end of RAM or if we have
more than 4G
it will be at 4GB end.
It relocates to the end of RAM or the end of 2GB, whichever is lower. It ignores all memory above 2GB.
What if you want to place U-Boot at 16MB for instance? Or if you do not want a memory map hole at 4GB just because U-Boot
doesn't handle
more than that?
Again, you're missing the point about U-boot.
Last AND least I just want to say I'm used to creating
memory maps in
bootloader which holds for the OS. It seems the time has come to annihilate my illusion...
Yes, please kill it with fire!
-- Timur Tabi Linux kernel developer at Freescale
Hello Timur!
You really used the machine gun there ;)
Ok. I'll just answer your 2GB+ usage question:
Testing! No I will not be using much memory at all for functionality. But the memory needs to be tested. What is your proposal for that then? And I see U-Boot as the first software place for test and debug. Maybe I want to read out RAM contents?
Robert,
We have seen some cases for u-boot to access >4G of memory directly. However, the effort to get u-boot to be a 64-bit clean just has never warranted the investment for the few minor cases people have raised for wanting to address >4G directly.
The one case I usually hear about is DDR test. We suggest its easier to try and getting DDR testing to utilize a window scheme into memory than it is for one to go make u-boot 64-bit.
:)
- k
-----Original Message----- From: Kumar Gala [mailto:galak@kernel.crashing.org] Sent: den 21 september 2012 14:50 To: Robert Thorhuus Cc: 'Tabi Timur-B04825'; u-boot@lists.denx.de Subject: Re: [U-Boot] arch/powerpc/lib/board.c:get_effective_memsize() for 4GB+ systems
On Sep 21, 2012, at 6:54 AM, Robert Thorhuus wrote:
-----Original Message----- From: Tabi Timur-B04825 [mailto:B04825@freescale.com] Sent: den 21 september 2012 13:37 To: Robert Thorhuus Cc: Kumar Gala; u-boot@lists.denx.de Subject: Re: [U-Boot] arch/powerpc/lib/board.c:get_effective_memsize() for 4GB+ systems
Robert Thorhuus wrote:
I really see the advantage of keeping this map you have. It is of course more compatible with different OSEs and you do not
need to do
anything special with 32/64 bit cores. But at the same time
you never
take advantage of a 64 bit core with this approach.
U-Boot is a boot loader, not an operating system. What is U-Boot supposed to do with more than 2GB of RAM?
How should I access my 24GB in U-Boot?
You should not!
So really you are saying that I should have a 2GB map
window in my
32-bit address space and then move this window depending on what memory I want to access? A bit cumbersome I must say. But ok.
Again, you're doing the wrong thing with U-Boot. It's a
boot loader.
It's supposed to find your OS, load it into memory, and
then boot it.
How about the U-Boot relocation?
As I see the code, it is not easy to decide where it should
relocate.
It will be either relocated to end of RAM or if we have
more than 4G
it will be at 4GB end.
It relocates to the end of RAM or the end of 2GB,
whichever is lower.
It ignores all memory above 2GB.
What if you want to place U-Boot at 16MB for instance? Or
if you do
not want a memory map hole at 4GB just because U-Boot
doesn't handle
more than that?
Again, you're missing the point about U-boot.
Last AND least I just want to say I'm used to creating
memory maps in
bootloader which holds for the OS. It seems the time has come to annihilate my illusion...
Yes, please kill it with fire!
-- Timur Tabi Linux kernel developer at Freescale
Hello Timur!
You really used the machine gun there ;)
Ok. I'll just answer your 2GB+ usage question:
Testing! No I will not be using much memory at all for
functionality. But the memory needs to be tested. What is your proposal for that then?
And I see U-Boot as the first software place for test and
debug. Maybe I want to read out RAM contents?
Robert,
We have seen some cases for u-boot to access >4G of memory directly. However, the effort to get u-boot to be a 64-bit clean just has never warranted the investment for the few minor cases people have raised for wanting to address >4G directly.
The one case I usually hear about is DDR test. We suggest its easier to try and getting DDR testing to utilize a window scheme into memory than it is for one to go make u-boot 64-bit.
:)
- k
Hello Kumar!
Ok. Since this seems to be the verified way I'll walk it. I'll change the DDR testing to be "small" window based.
Still I wonder why the choice was made to have U-Boot relocate in high memory rather than low memory and also not making it easy to configure the relocation.
BR Robert
Robert Thorhuus wrote:
Still I wonder why the choice was made to have U-Boot relocate in high memory rather than low memory and also not making it easy to configure the relocation.
U-Boot relocates in high memory so that you can load your operating system at address 0.
-----Original Message----- From: Timur Tabi [mailto:timur@freescale.com] Sent: den 21 september 2012 15:30 To: Robert Thorhuus Cc: 'Kumar Gala'; u-boot@lists.denx.de Subject: Re: [U-Boot] arch/powerpc/lib/board.c:get_effective_memsize() for 4GB+ systems
Robert Thorhuus wrote:
Still I wonder why the choice was made to have U-Boot
relocate in high
memory rather than low memory and also not making it easy
to configure
the relocation.
U-Boot relocates in high memory so that you can load your operating system at address 0.
-- Timur Tabi Linux kernel developer at Freescale
Ok. BR Robert
Robert Thorhuus wrote:
No I will not be using much memory at all for functionality. But the memory needs to be tested. What is your proposal for that then?
We have that already. Look at CONFIG_SYS_POST_MEMORY. It uses sliding 2GB TLBs to test all of DDR.
And I see U-Boot as the first software place for test and debug. Maybe I want to read out RAM contents?
It's a boot loader, not a testing platform.
-----Original Message----- From: Timur Tabi [mailto:timur@freescale.com] Sent: den 21 september 2012 15:29 To: Robert Thorhuus Cc: Kumar Gala; u-boot@lists.denx.de Subject: Re: [U-Boot] arch/powerpc/lib/board.c:get_effective_memsize() for 4GB+ systems
Robert Thorhuus wrote:
No I will not be using much memory at all for
functionality. But the
memory needs to be tested. What is your proposal for that then?
We have that already. Look at CONFIG_SYS_POST_MEMORY. It uses sliding 2GB TLBs to test all of DDR.
Ok. I'll look at that. Thanks.
And I see U-Boot as the first software place for test and
debug. Maybe I want to read out RAM contents?
It's a boot loader, not a testing platform.
Sorry. But this is were I disagree with you. Of course its prime function is to boot an operating system. And usually in a desktop environment that is what you need. But if you have newly developed hardware it is very seldom everything works all the time. There are a lot of debugging hardware sessions before you have stable hardware. So in embedded systems I would say U-Boot can very well be a primary choice of testing platform in the beginning of a development. Mainly because sucessfully booting U-Boot demands less of the hardware than sucessfully boot an OS. Say you have a NOR and 1 MB L3 cache but no DDR3, U-Boot is set, OS not.
And what test platform would you suggest? And what if your extended testing needs to be a viable option at every boot and that you have boot time requirements?
If you have suggestions I will be happy because I'd like to move my testing from U-Boot for several reasons but currently the restraints are tough.
BR
Robert Thorhuus wrote:
Sorry. But this is were I disagree with you. Of course its prime function is to boot an operating system. And usually in a desktop environment that is what you need. But if you have newly developed hardware it is very seldom everything works all the time. There are a lot of debugging hardware sessions before you have stable hardware. So in embedded systems I would say U-Boot can very well be a primary choice of testing platform in the beginning of a development. Mainly because sucessfully booting U-Boot demands less of the hardware than sucessfully boot an OS. Say you have a NOR and 1 MB L3 cache but no DDR3, U-Boot is set, OS not.
If you really want to turn U-Boot into a testing platform, feel free to post patches that implement those features. But don't be surprised if your patches are rejected.
And what test platform would you suggest? And what if your extended testing needs to be a viable option at every boot and that you have boot time requirements?
You can create U-boot "applications" that test your hardware, if you want to test from within U-Boot.
Hi,
On Wed, 19 Sep 2012 09:47:36 -0500 Kumar Gala galak@kernel.crashing.org wrote:
The following commit introduced some warnings associated with using pci_addr_t instead of a proper 32-bit data type.
commit af778c6d9e2b945ee03cbc53bb976238a3374f33 Author: Andrew Sharp andywyse6@gmail.com Date: Wed Aug 1 12:27:16 2012 +0000
pci: fix errant data types and corresponding access functionsOn some platforms pci_addr_t is defined as a 64-bit data type so its not proper to use with pci_{read,write}_config_dword.
Signed-off-by: Kumar Gala galak@kernel.crashing.org
drivers/pci/pci.c | 6 +++--- drivers/pci/pci_auto.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)
Applied to staging/agust@denx.de.
Thanks, Anatolij
On Sep 21, 2012, at 5:34 PM, Anatolij Gustschin wrote:
Hi,
On Wed, 19 Sep 2012 09:47:36 -0500 Kumar Gala galak@kernel.crashing.org wrote:
The following commit introduced some warnings associated with using pci_addr_t instead of a proper 32-bit data type.
commit af778c6d9e2b945ee03cbc53bb976238a3374f33 Author: Andrew Sharp andywyse6@gmail.com Date: Wed Aug 1 12:27:16 2012 +0000
pci: fix errant data types and corresponding access functions
On some platforms pci_addr_t is defined as a 64-bit data type so its not proper to use with pci_{read,write}_config_dword.
Signed-off-by: Kumar Gala galak@kernel.crashing.org
drivers/pci/pci.c | 6 +++--- drivers/pci/pci_auto.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)
Applied to staging/agust@denx.de.
Thanks, Anatolij
Thanks,
This should get in before v2012.10 is released
- k
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 09/22/12 06:57, Kumar Gala wrote:
On Sep 21, 2012, at 5:34 PM, Anatolij Gustschin wrote:
Hi,
On Wed, 19 Sep 2012 09:47:36 -0500 Kumar Gala galak@kernel.crashing.org wrote:
The following commit introduced some warnings associated with using pci_addr_t instead of a proper 32-bit data type.
commit af778c6d9e2b945ee03cbc53bb976238a3374f33 Author: Andrew Sharp andywyse6@gmail.com Date: Wed Aug 1 12:27:16 2012 +0000
pci: fix errant data types and corresponding access functions
On some platforms pci_addr_t is defined as a 64-bit data type so its not proper to use with pci_{read,write}_config_dword.
Signed-off-by: Kumar Gala galak@kernel.crashing.org --- drivers/pci/pci.c | 6 +++--- drivers/pci/pci_auto.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)
Applied to staging/agust@denx.de.
Thanks, Anatolij
Thanks,
This should get in before v2012.10 is released
Staging branch was included in -rc1 so we should be good (and I noticed the builds with warning set of powerpc went away).
- -- Tom
participants (6)
-
Anatolij Gustschin -
Kumar Gala -
Robert Thorhuus -
Tabi Timur-B04825 -
Timur Tabi -
Tom Rini