PowerPC: Add 64-bit phys addr support to 32-bit pci.
RFC PATCH 1/1 PowerPC: Add 64-bit phys addr support to 32-bit pci.
Add 64-bit physical address support to PCI.
--- arch/powerpc/kernel/iomap.c | 4 +-- arch/powerpc/kernel/pci_32.c | 56 +++++++++++++++++++++++++++++-------------- 2 files changed, 41 insertions(+), 19 deletions(-)
arch/powerpc/kernel/iomap.c +2 -2
@@ -119,8 +119,8 @@void __iomem *pci_iomap (struct pci_dev *dev,int bar,unsigned long max) { -unsigned long start =pci_resource_start (dev, bar); -unsigned long len =pci_resource_len (dev, bar); +resource_size_t start =pci_resource_start (dev, bar); +resource_size_t len =pci_resource_len (dev, bar);unsigned long flags =pci_resource_flags (dev, bar);if (!len)
arch/powerpc/kernel/pci_32.c +39 -17
136 lines not on this page · raw message
Quoted 1 line · Valentine Barshak
Add 64-bit physical address support to PCI.
Can you expand on your commit message. for example why do this? is it address a bug fix, etc..
Quoted 7 lines
Signed-off-by Valentine Barshak <[email protected]>--- arch/powerpc/kernel/iomap.c | 4 +-- arch/powerpc/kernel/pci_32.c | 56 ++++++++++++++++++++++++++++ +-------------- 2 files changed, 41 insertions(+), 19 deletions(-)
- k
Quoted 8 lines · Kumar Gala
Quoted 1 line · Valentine Barshak
Add 64-bit physical address support to PCI.
Can you expand on your commit message. for example why do this? is it address a bug fix, etc..
Quoted 1 line
Quoted 9 lines
Quoted 6 lines
Signed-off-by Valentine Barshak <[email protected]>--- arch/powerpc/kernel/iomap.c | 4 +-- arch/powerpc/kernel/pci_32.c | 56 +++++++++++++++++++++++++++++-------------- 2 files changed, 41 insertions(+), 19 deletions(-)- k
Currently 32-bit pci code doesn't support 64-bit physical addresses.
We have to add 64-bit addr support in order to bring-up pci on PowerPC 440. Actually, this refers to the conversation started here:
http://ozlabs.org/pipermail/linuxppc-dev/2007-September/042267.html
Thanks,
Valentine.
Quoted 11 lines · Valentine Barshak
Quoted 10 lines · Kumar Gala
Quoted 1 line · Valentine Barshak
Add 64-bit physical address support to PCI.
Can you expand on your commit message. for example why do this? is it address a bug fix, etc..
Quoted 7 lines
Signed-off-by Valentine Barshak <[email protected]>--- arch/powerpc/kernel/iomap.c | 4 +-- arch/powerpc/kernel/pci_32.c | 56 ++++++++++++++++++++++++++++ +-------------- 2 files changed, 41 insertions(+), 19 deletions(-)
Quoted 4 lines
Quoted 1 line
- k
Currently 32-bit pci code doesn't support 64-bit physical addresses.
We have to add 64-bit addr support in order to bring-up pci on PowerPC 440. Actually, this refers to the conversation started here:
I can guess as to why, but its useful for the commit messages to be more descriptive so when some goes back a year from now they know why someone added this patch.
- k
RFC PATCH 1/1 PowerPC: Add 64-bit phys addr support to 32-bit pci.
Currently pci_32 doesn't support 64-bit physical addresses, while PowerPC440 platform has PCI space typically mapped above 4GB range. The patch adds 64-bit physical address support to 32-bit PCI code in order to bring-up PCI on 44x platform.
--- arch/powerpc/kernel/iomap.c | 4 +-- arch/powerpc/kernel/pci_32.c | 56 +++++++++++++++++++++++++++++-------------- 2 files changed, 41 insertions(+), 19 deletions(-)
arch/powerpc/kernel/iomap.c +2 -2
@@ -119,8 +119,8 @@void __iomem *pci_iomap (struct pci_dev *dev,int bar,unsigned long max) { -unsigned long start =pci_resource_start (dev, bar); -unsigned long len =pci_resource_len (dev, bar); +resource_size_t start =pci_resource_start (dev, bar); +resource_size_t len =pci_resource_len (dev, bar);unsigned long flags =pci_resource_flags (dev, bar);if (!len)
arch/powerpc/kernel/pci_32.c +39 -17
136 lines not on this page · raw message
Hello Valentine,
Quoted 5 lines · On Tue, 18 Sep 2007 20:07:54 +0400 Valentine Barshak wrote:
Currently
pci_32doesn't support 64-bit physical addresses, while PowerPC440 platform has PCI space typically mapped above 4GB range. The patch adds 64-bit physical address support to 32-bit PCI code in order to bring-up PCI on 44x platform.
The approach looks interesting, and resource_size_t<>ulong apparently makes sense, but I would strongly disagree we need to add more nuts and bolts into ranges parse func:
Quoted 1 line
Signed-off-by Valentine Barshak <[email protected]>
[...]
Quoted 31 lines
void
__init
pci_process_bridge_OF_ranges(structpci_controller*hose,arch/powerpc/kernel/pci_32.c +10
-5struct device_node *dev, int primary) {static unsigned int static_lc_ranges[256 ]__initdata ;const unsigned int *dt_ranges; -unsigned int *lc_ranges, *ranges, *prev, size; +unsigned int *lc_ranges, *ranges, *prev;int rlen =0 , orig_rlen;int memno =0 ;struct resource *res; +u32 prev_pci_space, pci_space; +u64 prev_pci_addr, pci_addr; +u64 prev_size, size; +phys_addr_t cpu_phys_addr; +int np, na =of_n_addr_cells (dev); np = na +5 ;@@ -879,11 +891,18 @@ prev =NULL ;while ((rlen -= np *sizeof (unsigned int )) >=0 ) {if (prev) { -if (prev[0 ] == ranges[0 ] && prev[1 ] == ranges[1 ] && - (prev[2 ] + prev[na+4 ]) == ranges[2 ] && - (prev[na+2 ] + prev[na+4 ]) == ranges[na+2 ]) { - prev[na+4 ] += ranges[na+4 ]; + prev_pci_space = prev[0 ]; + prev_pci_addr =pci_get_range64 (&prev[1 ]); + prev_size =pci_get_range64 (&prev[na+3 ]); + pci_space = ranges[0 ];
arch/powerpc/kernel/pci_32.c line 907
Quoted 10 lines
arch/powerpc/kernel/pci_32.c +7
-0+ pci_addr =pci_get_range64 (&ranges[1 ]); +if ((prev_pci_space == pci_space) && + ((prev_pci_addr + prev_size) == pci_addr)) { + size =pci_get_range64 (&ranges[na+3 ]); + prev_size += size; ranges[0 ] =0 ; + prev[na+3 ] = (u32)((prev_size >>32 ) &0xffffffff ); + prev[na+4 ] = (u32)(prev_size &0xffffffff ); ranges += np;continue ;
I do think that ranges hacking (even on a copy) to cope with contiguous ranges is not a good deed. And nobody would object the upper looks horrible from the maintenance POV.
arch/powerpc/kernel/pci_32.c line 926
Quoted 8 lines
arch/powerpc/kernel/pci_32.c +1
-2}@@ -904,21 +923,22 @@ rlen = orig_rlen;while (ranges && (rlen -= np *sizeof (unsigned int )) >=0 ) { res =NULL ; - size = ranges[na+4 ]; -switch ((ranges[0 ] >>24 ) &0x3 ) { + size =pci_get_range64 (&ranges[na+3 ]);
This is not correct - it should depend on #ac of the parent node.
But I'll stop right here - there is no deep mutual difference between 32 & 64 bit so that to keep 2 similar implementations, none of which (esp 32bit) really comply the spec. It should work the same way, and, if there are differences, they should be handled explicitly, and, of course, reconsidered if they make sense (like isa_io_base, absense of io_size in ppc32 and so on)
I am *not* telling here that my implementation is the only true way around. But we need to improve and make code cleaner rather then just extend existing error-prone approach.
Signature · 1 line
Sincerely, Vitaly
Quoted 17 lines
Quoted 17 lines
arch/powerpc/kernel/pci_32.c +8
-4@@ -879,11 +891,18 @@ prev =NULL ;while ((rlen -= np *sizeof (unsigned int )) >=0 ) {if (prev) { -if (prev[0 ] == ranges[0 ] && prev[1 ] == ranges[1 ] && - (prev[2 ] + prev[na+4 ]) == ranges[2 ] && - (prev[na+2 ] + prev[na+4 ]) == ranges[na+2 ]) { - prev[na+4 ] += ranges[na+4 ]; + prev_pci_space = prev[0 ]; + prev_pci_addr =pci_get_range64 (&prev[1 ]); + prev_size =pci_get_range64 (&prev[na+3 ]); + pci_space = ranges[0 ]; + pci_addr =pci_get_range64 (&ranges[1 ]); +if ((prev_pci_space == pci_space) && + ((prev_pci_addr + prev_size) == pci_addr)) { + size =pci_get_range64 (&ranges[na+3 ]);
Quoted 9 lines
arch/powerpc/kernel/pci_32.c line 907
Quoted 6 lines
arch/powerpc/kernel/pci_32.c +3
-0+ prev_size += size; ranges[0 ] =0 ; + prev[na+3 ] = (u32)((prev_size >>32 ) &0xffffffff ); + prev[na+4 ] = (u32)(prev_size &0xffffffff ); ranges += np;continue ;I do think that ranges hacking (even on a copy) to cope with contiguous ranges is not a good deed. And nobody would object the upper looks horrible from the maintenance POV.
Quoted 1 line
Yes, hacking of ranges shouldn't be done there. PowerMac does range coalescing separately, we can move this to some generic place maybe and use it where needed.
Just make everything use resource_size_t in the PCI bits and u64 in the parsing bits.
Quoted 5 lines
This is not correct - it should depend on #ac of the parent node.
But I'll stop right here - there is no deep mutual difference between 32 & 64 bit so that to keep 2 similar implementations, none of which (esp 32bit) really comply the spec. It should work the same way, and, if there are differences, they should be handled explicitly, and, of course, reconsidered if they make sense (likeisa_io_base, absense ofio_sizein ppc32 and so on)I am *not* telling here that my implementation is the only true way around. But we need to improve and make code cleaner rather then just extend existing error-prone approach.
We need to merge 32 and 64 bits here for sure.
Ben.