diff mbox

[U-Boot] pci: fix some warnings related to assumptions about

Message ID 1348066056-12965-1-git-send-email-galak@kernel.crashing.org
State Accepted
Commit cf5787f
Delegated to: Anatolij Gustschin
Headers show

Commit Message

Kumar Gala Sept. 19, 2012, 2:47 p.m. UTC
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(-)

Comments

Robert Thorhuus Sept. 19, 2012, 3:28 p.m. UTC | #1
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
Kumar Gala Sept. 20, 2012, 5:27 p.m. UTC | #2
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
Robert Thorhuus Sept. 20, 2012, 8:24 p.m. UTC | #3
> -----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
Tabi Timur-B04825 Sept. 20, 2012, 10:37 p.m. UTC | #4
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?
Robert Thorhuus Sept. 21, 2012, 10:21 a.m. UTC | #5
> -----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
Tabi Timur-B04825 Sept. 21, 2012, 11:37 a.m. UTC | #6
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!
Robert Thorhuus Sept. 21, 2012, 11:54 a.m. UTC | #7
> -----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
Kumar Gala Sept. 21, 2012, 12:49 p.m. UTC | #8
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
Robert Thorhuus Sept. 21, 2012, 1:21 p.m. UTC | #9
> -----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
Timur Tabi Sept. 21, 2012, 1:28 p.m. UTC | #10
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.
Timur Tabi Sept. 21, 2012, 1:30 p.m. UTC | #11
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.
Robert Thorhuus Sept. 21, 2012, 1:34 p.m. UTC | #12
> -----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 Sept. 21, 2012, 1:56 p.m. UTC | #13
> -----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
Timur Tabi Sept. 21, 2012, 2:15 p.m. UTC | #14
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.
Anatolij Gustschin Sept. 21, 2012, 10:34 p.m. UTC | #15
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
Kumar Gala Sept. 22, 2012, 1:57 p.m. UTC | #16
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
Tom Rini Sept. 22, 2012, 5:45 p.m. UTC | #17
-----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
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQXfkuAAoJENk4IS6UOR1W2AUP/A9YxyD6u5F8bI6uQbFjCgvj
BEgEhc2KENHWX8gUYPvQMFQosCwDvAB4EOrb9a9ouC5ppHS7flzUZ1HwbCiSFim1
lBVT/MrpQGbxDwfbqNltEB3LbPWVY4uRnszRiUvW0y6FL9ikYx+v8JZZ5PVlAarE
dDaR1YoM7T+ePdyv3+fXegfBYIKQFEBFr3FRaF9KGwn6L8cqQKI3/9O33ofI1uiS
6devhuIQc7IT/vJPWoG5dFVDJSB+0EyDbhxqKXXDGhUIFs9wyywhgAb1qbHwZ4ec
vRBYstPbVAkWXxrWwxLBfvvwYSPVCZD8T9oiCIZay2XqjkEmIE7RdgVgNzidyb6e
1oSUvrHL+AZVbEhEdudbb6rmkXyb8hEcaiKWNkKhm6eoKIZVy5ZhqefuoSIdMPrV
IyN79zgmnOVOez0cTYJgzvV70zF4at2Xs335Aznb7AbvjB+pg0bm/NtY40aFIOMQ
UlniQDCCk5vXSnGymS0LbDy42F69rLtRw9zpR0oJkqiP/rT0+bKgHzCT3+O41p/7
ExuHzs7FP14i8lCS5Tnlvcgu1HxFvWnZZAqLZuDDL6aYr3XDDzPL3itSfNi3uZmD
Mc322A6A6wOldbYX3JvEyIZnNSpiikt0zHT6fjtTA/Inv/WlhpGOOMqJwtKyNavU
l4XjadVB8XPhEohWnm/v
=MKhp
-----END PGP SIGNATURE-----
diff mbox

Patch

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;