[U-Boot] [PATCH] pci: Treat all PCI bus addresses as 64-bit
PCI bus is inherently 64-bit. We should treat all PCI related bus addresses as 64-bit quanities. This allows us to have the ability to support devices or memory on the PCI bus above the 32-bit boundary.
Signed-off-by: Kumar Gala galak@kernel.crashing.org --- drivers/pci/pci.c | 4 ++-- drivers/pci/pci_auto.c | 10 +++++----- include/pci.h | 16 ++++++++-------- 3 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 41780db..253e1f5 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -223,7 +223,7 @@ unsigned long pci_hose_phys_to_bus (struct pci_controller *hose, unsigned long flags) { struct pci_region *res; - unsigned long bus_addr; + u64 bus_addr; int i;
if (!hose) { @@ -252,7 +252,7 @@ Done: }
phys_addr_t pci_hose_bus_to_phys(struct pci_controller* hose, - unsigned long bus_addr, + u64 bus_addr, unsigned long flags) { struct pci_region *res; diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c index 3844359..ea362a9 100644 --- a/drivers/pci/pci_auto.c +++ b/drivers/pci/pci_auto.c @@ -52,7 +52,7 @@ void pciauto_region_align(struct pci_region *res, unsigned long size)
int pciauto_region_allocate(struct pci_region* res, unsigned int size, unsigned int *bar) { - unsigned long addr; + u64 addr;
if (!res) { DEBUGF("No resource"); @@ -68,7 +68,7 @@ int pciauto_region_allocate(struct pci_region* res, unsigned int size, unsigned
res->bus_lower = addr + size;
- DEBUGF("address=0x%lx bus_lower=%x", addr, res->bus_lower); + DEBUGF("address=0x%llx bus_lower=%llx", addr, res->bus_lower);
*bar = addr; return 0; @@ -289,7 +289,7 @@ void pciauto_config_init(struct pci_controller *hose) if (hose->pci_mem) { pciauto_region_init(hose->pci_mem);
- DEBUGF("PCI Autoconfig: Bus Memory region: [%lx-%lx],\n" + DEBUGF("PCI Autoconfig: Bus Memory region: [%llx-%llx],\n" "\t\tPhysical Memory [%x-%x]\n", hose->pci_mem->bus_start, hose->pci_mem->bus_start + hose->pci_mem->size - 1, @@ -300,7 +300,7 @@ void pciauto_config_init(struct pci_controller *hose) if (hose->pci_prefetch) { pciauto_region_init(hose->pci_prefetch);
- DEBUGF("PCI Autoconfig: Bus Prefetchable Mem: [%lx-%lx],\n" + DEBUGF("PCI Autoconfig: Bus Prefetchable Mem: [%llx-%llx],\n" "\t\tPhysical Memory [%x-%x]\n", hose->pci_prefetch->bus_start, hose->pci_prefetch->bus_start + hose->pci_prefetch->size - 1, @@ -312,7 +312,7 @@ void pciauto_config_init(struct pci_controller *hose) if (hose->pci_io) { pciauto_region_init(hose->pci_io);
- DEBUGF("PCI Autoconfig: Bus I/O region: [%lx-%lx],\n" + DEBUGF("PCI Autoconfig: Bus I/O region: [%llx-%llx],\n" "\t\tPhysical Memory: [%x-%x]\n", hose->pci_io->bus_start, hose->pci_io->bus_start + hose->pci_io->size - 1, diff --git a/include/pci.h b/include/pci.h index 1c8e216..2689640 100644 --- a/include/pci.h +++ b/include/pci.h @@ -313,12 +313,12 @@ #include <pci_ids.h>
struct pci_region { - unsigned long bus_start; /* Start on the bus */ - phys_addr_t phys_start; /* Start in physical address space */ - unsigned long size; /* Size */ - unsigned long flags; /* Resource flags */ + u64 bus_start; /* Start on the bus */ + phys_addr_t phys_start; /* Start in physical address space */ + u64 size; /* Size */ + unsigned long flags; /* Resource flags */
- unsigned long bus_lower; + u64 bus_lower; };
#define PCI_REGION_MEM 0x00000000 /* PCI memory space */ @@ -330,9 +330,9 @@ struct pci_region { #define PCI_REGION_RO 0x00000200 /* Read-only memory */
extern __inline__ void pci_set_region(struct pci_region *reg, - unsigned long bus_start, + u64 bus_start, phys_addr_t phys_start, - unsigned long size, + u64 size, unsigned long flags) { reg->bus_start = bus_start; reg->phys_start = phys_start; @@ -433,7 +433,7 @@ extern __inline__ void pci_set_ops(struct pci_controller *hose, extern void pci_setup_indirect(struct pci_controller* hose, u32 cfg_addr, u32 cfg_data);
extern phys_addr_t pci_hose_bus_to_phys(struct pci_controller* hose, - unsigned long addr, unsigned long flags); + u64 addr, unsigned long flags); extern unsigned long pci_hose_phys_to_bus(struct pci_controller* hose, phys_addr_t addr, unsigned long flags);
Dear Kumar Gala,
In message 1224598531-2698-1-git-send-email-galak@kernel.crashing.org you wrote:
PCI bus is inherently 64-bit. We should treat all PCI related bus addresses as 64-bit quanities. This allows us to have the ability to support devices or memory on the PCI bus above the 32-bit boundary.
I don't think this is a good idea. There are pure 32 bit systems out there which will never use more than 32 bit for the PCI resources, so why load them with the additional memory size and execution time?
Should we not enable this only for such systems that actually need it?
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
Dear Kumar Gala,
In message 1224598531-2698-1-git-send-email-galak@kernel.crashing.org you wrote:
PCI bus is inherently 64-bit. We should treat all PCI related bus addresses as 64-bit quanities. This allows us to have the ability to support devices or memory on the PCI bus above the 32-bit boundary.
I don't think this is a good idea. There are pure 32 bit systems out there which will never use more than 32 bit for the PCI resources, so why load them with the additional memory size and execution time?
Should we not enable this only for such systems that actually need it?
Best regards,
Wolfgang Denk
Why would we not use phys_addr_t and phys_size_t for the PCI addresses?
Best regards, gvb
On Oct 21, 2008, at 9:55 AM, Jerry Van Baren wrote:
Wolfgang Denk wrote:
Dear Kumar Gala, In message <1224598531-2698-1-git-send-email-galak@kernel.crashing.org
you wrote: PCI bus is inherently 64-bit. We should treat all PCI related bus addresses as 64-bit quanities. This allows us to have the ability to support devices or memory on the PCI bus above the 32-bit boundary.
I don't think this is a good idea. There are pure 32 bit systems out there which will never use more than 32 bit for the PCI resources, so why load them with the additional memory size and execution time? Should we not enable this only for such systems that actually need it? Best regards, Wolfgang Denk
Why would we not use phys_addr_t and phys_size_t for the PCI addresses?
Because they aren't coupled. I can reasonable want 64-bit PCI accessible on a pure 32-bit system.
If we want to do this we should introduce a pci_addr_t/pci_size_t.
- k
Dear Jerry Van Baren,
In message 48FDED5D.3010403@ge.com you wrote:
Should we not enable this only for such systems that actually need it?
...
Why would we not use phys_addr_t and phys_size_t for the PCI addresses?
Good point.
Best regards,
Wolfgang Denk
On Oct 21, 2008, at 9:46 AM, Wolfgang Denk wrote:
Dear Kumar Gala,
In message <1224598531-2698-1-git-send-email- galak@kernel.crashing.org> you wrote:
PCI bus is inherently 64-bit. We should treat all PCI related bus addresses as 64-bit quanities. This allows us to have the ability to support devices or memory on the PCI bus above the 32-bit boundary.
I don't think this is a good idea. There are pure 32 bit systems out there which will never use more than 32 bit for the PCI resources, so why load them with the additional memory size and execution time?
Should we not enable this only for such systems that actually need it?
We could, however it seems the impact is pretty small on those systems with pure 32-bit PCI. I think the different in testing etc may not be worth it:
text data bss dec hex filename 350588 44964 41909 437461 6acd5 u-boot (32-bit pci structs) 351100 44964 42349 438413 6b08d u-boot (64-bit pci structs) ---------------------------------------------- 512 0 440 952 3b8
If this is desired to be configurable should I introduce a pci_addr_t/ pci_size_t? Coupling to phys_addr_t/phys_size_t doesn't make sense to me because its perfectly reasonable to have a 64-bit PCI device work in a 32-bit system.
- k
Dear Kumar Gala,
In message A16160A4-977E-48A0-83D1-FBAB41E1C0B6@kernel.crashing.org you wrote:
If this is desired to be configurable should I introduce a pci_addr_t/ pci_size_t? Coupling to phys_addr_t/phys_size_t doesn't make sense to me because its perfectly reasonable to have a 64-bit PCI device work in a 32-bit system.
I think using pci_addr_t/pci_size_t is the most reasonable suggestion, then (but it may turn into fighting a lot of new warnings about incompatible types for one board or another).
Best regards,
Wolfgang Denk
On Oct 21, 2008, at 10:06 AM, Wolfgang Denk wrote:
Dear Kumar Gala,
In message <A16160A4-977E-48A0-83D1- FBAB41E1C0B6@kernel.crashing.org> you wrote:
If this is desired to be configurable should I introduce a pci_addr_t/ pci_size_t? Coupling to phys_addr_t/phys_size_t doesn't make sense to me because its perfectly reasonable to have a 64-bit PCI device work in a 32-bit system.
I think using pci_addr_t/pci_size_t is the most reasonable suggestion, then (but it may turn into fighting a lot of new warnings about incompatible types for one board or another).
Ok, I'll switch over to these types and post a new patch.
- k
participants (3)
-
Jerry Van Baren -
Kumar Gala -
Wolfgang Denk