Re: [U-Boot] [PATCH v2] pci: Allow for PCI addresses to be 64-bit
On Oct 22, 2008, at 9:19 AM, Kumar Gala wrote: > PCI bus is inherently 64-bit. While not all system require access to > the full 64-bit PCI address range some do. This allows those systems > to enable the full PCI address width via CONFIG_SYS_PCI_64BIT. > > Signed-off-by: Kumar Gala <[EMAIL PROTECTED]> > --- > > Fixed up all the other bits associated with 64-bit PCI support. So, this is looking a bit more complete now :) A few minor comments inline. -B > > > - k > > drivers/pci/pci.c | 37 ++ > drivers/pci/pci_auto.c | 67 > +-- > include/pci.h | 40 +--- > 3 files changed, 91 insertions(+), 53 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 41780db..d13a57e 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > --snip > @@ -319,10 +321,19 @@ int pci_hose_config_device(struct > pci_controller *hose, > io = io + bar_size; > } else { > if ((bar_response & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == > - PCI_BASE_ADDRESS_MEM_TYPE_64) > - found_mem64 = 1; > + PCI_BASE_ADDRESS_MEM_TYPE_64) { > + u32 bar_response_upper; > + u64 bar64; > + pci_hose_write_config_dword(hose, dev, > bar+4, 0x); > + pci_hose_read_config_dword(hose, dev, bar+4, > &bar_response_upper); Fix alignment of code. > > - bar_size = ~(bar_response & PCI_BASE_ADDRESS_MEM_MASK) > + 1; > + bar64 = ((u64)bar_response_upper << 32) | > bar_response; > + > + bar_size = ~(bar64 & PCI_BASE_ADDRESS_MEM_MASK) > + 1; > + found_mem64 = 1; > + } else { > + bar_size = (u32)(~(bar_response & > PCI_BASE_ADDRESS_MEM_MASK) + > 1); > + } > > /* round up region base address to multiple of size */ > mem = ((mem - 1) | (bar_size - 1)) + 1; > @@ -332,11 +343,15 @@ int pci_hose_config_device(struct > pci_controller *hose, > } > > /* Write it out and update our limit */ > - pci_hose_write_config_dword (hose, dev, bar, bar_value); > + pci_hose_write_config_dword (hose, dev, bar, (u32)bar_value); > > > if (found_mem64) { > bar += 4; > +#ifdef CONFIG_SYS_PCI_64BIT > + pci_hose_write_config_dword(hose, dev, bar, > bar_value>>32); spaces around ">>" ? Might want to cast the result to (u32) for consistency. > > +#else > pci_hose_write_config_dword (hose, dev, bar, > 0x); > +#endif > } > } > > diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c > index 3844359..c347e82 100644 > --- a/drivers/pci/pci_auto.c > +++ b/drivers/pci/pci_auto.c > @@ -45,14 +45,14 @@ void pciauto_region_init(struct pci_region* res) > res->bus_lower = res->bus_start ? res->bus_start : 0x1000; > } > > -void pciauto_region_align(struct pci_region *res, unsigned long size) > +void pciauto_region_align(struct pci_region *res, pci_size_t size) > { > res->bus_lower = ((res->bus_lower - 1) | (size - 1)) + 1; > } > > -int pciauto_region_allocate(struct pci_region* res, unsigned int > size, unsigned int *bar) > +int pciauto_region_allocate(struct pci_region* res, pci_size_t > size, pci_addr_t *bar) > { > - unsigned long addr; > + pci_addr_t addr; > > if (!res) { > DEBUGF("No resource"); > @@ -68,13 +68,13 @@ 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", (u64)addr, (u64)res- > >bus_lower); While you're changing this, can we add the "0x" to the bus_lower print? > > > *bar = addr; > return 0; > > error: > - *bar = 0x; > + *bar = (pci_addr_t)-1; > return -1; > } > > @@ -88,7 +88,9 @@ void pciauto_setup_device(struct pci_controller > *hose, > struct pci_region *prefetch, > struct pci_region *io) > { > - unsigned int bar_value, bar_response, bar_size; > + unsigned int bar_response; > + pci_addr_t bar_value; > + pci_size_t bar_size; > unsigned int cmdstat = 0; > struct pci_region *bar_res; > int bar, bar_nr = 0; > @@ -114,33 +116,46 @@ void pciauto_setup_device(struct > pci_controller *hose, > & 0x) + 1; > bar_res = io; > > - DEBUGF("PCI Autoconfig: BAR %d, I/O,
Re: [U-Boot] [PATCH v2] pci: Allow for PCI addresses to be 64-bit
On Oct 22, 2008, at 9:19 AM, Kumar Gala wrote: > PCI bus is inherently 64-bit. While not all system require access to > the full 64-bit PCI address range some do. This allows those systems > to enable the full PCI address width via CONFIG_SYS_PCI_64BIT. > > Signed-off-by: Kumar Gala <[EMAIL PROTECTED]> > --- > > Fixed up all the other bits associated with 64-bit PCI support. guess this is really v3, the next one that fixes some whitespace issues will be v4 - k ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2] pci: Allow for PCI addresses to be 64-bit
PCI bus is inherently 64-bit. While not all system require access to the full 64-bit PCI address range some do. This allows those systems to enable the full PCI address width via CONFIG_SYS_PCI_64BIT. Signed-off-by: Kumar Gala <[EMAIL PROTECTED]> --- Fixed up all the other bits associated with 64-bit PCI support. - k drivers/pci/pci.c | 37 ++ drivers/pci/pci_auto.c | 67 +-- include/pci.h | 40 +--- 3 files changed, 91 insertions(+), 53 deletions(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 41780db..d13a57e 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -218,12 +218,12 @@ pci_dev_t pci_find_device(unsigned int vendor, unsigned int device, int index) * */ -unsigned long pci_hose_phys_to_bus (struct pci_controller *hose, +pci_addr_t pci_hose_phys_to_bus (struct pci_controller *hose, phys_addr_t phys_addr, unsigned long flags) { struct pci_region *res; - unsigned long bus_addr; + pci_addr_t 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, +pci_addr_t bus_addr, unsigned long flags) { struct pci_region *res; @@ -288,15 +288,17 @@ Done: int pci_hose_config_device(struct pci_controller *hose, pci_dev_t dev, unsigned long io, - unsigned long mem, + pci_addr_t mem, unsigned long command) { - unsigned int bar_response, bar_size, bar_value, old_command; + unsigned int bar_response, old_command; + pci_addr_t bar_value; + pci_size_t bar_size; unsigned char pin; int bar, found_mem64; - debug ("PCI Config: I/O=0x%lx, Memory=0x%lx, Command=0x%lx\n", - io, mem, command); + debug ("PCI Config: I/O=0x%lx, Memory=0x%llx, Command=0x%lx\n", + io, (u64)mem, command); pci_hose_write_config_dword (hose, dev, PCI_COMMAND, 0); @@ -319,10 +321,19 @@ int pci_hose_config_device(struct pci_controller *hose, io = io + bar_size; } else { if ((bar_response & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == - PCI_BASE_ADDRESS_MEM_TYPE_64) - found_mem64 = 1; + PCI_BASE_ADDRESS_MEM_TYPE_64) { + u32 bar_response_upper; + u64 bar64; + pci_hose_write_config_dword(hose, dev, bar+4, 0x); + pci_hose_read_config_dword(hose, dev, bar+4, &bar_response_upper); - bar_size = ~(bar_response & PCI_BASE_ADDRESS_MEM_MASK) + 1; + bar64 = ((u64)bar_response_upper << 32) | bar_response; + + bar_size = ~(bar64 & PCI_BASE_ADDRESS_MEM_MASK) + 1; + found_mem64 = 1; + } else { + bar_size = (u32)(~(bar_response & PCI_BASE_ADDRESS_MEM_MASK) + 1); + } /* round up region base address to multiple of size */ mem = ((mem - 1) | (bar_size - 1)) + 1; @@ -332,11 +343,15 @@ int pci_hose_config_device(struct pci_controller *hose, } /* Write it out and update our limit */ - pci_hose_write_config_dword (hose, dev, bar, bar_value); + pci_hose_write_config_dword (hose, dev, bar, (u32)bar_value); if (found_mem64) { bar += 4; +#ifdef CONFIG_SYS_PCI_64BIT + pci_hose_write_config_dword(hose, dev, bar, bar_value>>32); +#else pci_hose_write_config_dword (hose, dev, bar, 0x); +#endif } } diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c index 3844359..c347e82 100644 --- a/drivers/pci/pci_auto.c +++ b/drivers/pci/pci_auto.c @@ -45,14 +45,14 @@ void pciauto_region_init(struct pci_region* res) res->bus_lower = res->bus_start ? res->bus_start : 0x1000; } -void pciauto_region_align(struct pci_region *res, unsigned long size) +void pciauto_region_align(struct pci_region *res, pci_size_t size) { res->bus_lower = ((res->bus_lower - 1) | (size - 1)) + 1; } -int pciauto_region_allocate(struct pci_region* res, unsigned int size, unsigned int *bar) +int pciauto_region_allocate(struct pci_region* res, pci_size_t size, pci_addr_t *bar) { - unsigned long addr; + pci_addr_t addr;
Re: [U-Boot] [PATCH v2] pci: Allow for PCI addresses to be 64-bit
On Oct 21, 2008, at 4:48 PM, Kumar Gala wrote: > PCI bus is inherently 64-bit. While not all system require access to > the full 64-bit PCI address range some do. This allows those systems > to enable the full PCI address width via CONFIG_SYS_PCI_64BIT. I suspect this isn't complete, but I have no proof :) As an example, what are the mem and io args to pci_hose_config_device? Is unsigned long still OK there? (it may well be - I haven't looked at how they're used). A few comments below. > > > Signed-off-by: Kumar Gala <[EMAIL PROTECTED]> > --- > > moved to CONFIG_SYS_PCI_64BIT instead of CONFIG_PCI_64BIT > > - k > > drivers/pci/pci.c |4 ++-- > drivers/pci/pci_auto.c | 22 +++--- > include/pci.h | 24 > 3 files changed, 29 insertions(+), 21 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 41780db..127d50f 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; > + pci_addr_t bus_addr; > int i; > > if (!hose) { Is there a reason pci_hose_phys_to_bus() isn't now returning a pci_addr_t? > > @@ -252,7 +252,7 @@ Done: > } > > phys_addr_t pci_hose_bus_to_phys(struct pci_controller* hose, > - unsigned long bus_addr, > + pci_addr_t 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..547ce5b 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) Is this size really OK as an unsigned int? There are a lot of "unsigned int size" declarations in this file; I haven't looked at them all. - snip -- > diff --git a/include/pci.h b/include/pci.h > index 1c8e216..d79f26a 100644 > --- a/include/pci.h > +++ b/include/pci.h > @@ -312,13 +312,21 @@ > > #include > > +#ifdef CONFIG_SYS_PCI_64BIT > +typedef u64 pci_addr_t; > +typedef u64 pci_size_t; > +#else > +typedef u32 pci_addr_t; > +typedef u32 pci_size_t; > +#endif > + > 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 */ > + pci_addr_t bus_start; /* Start on the bus */ > + phys_addr_t phys_start; /* Start in physical address space */ > + pci_size_t size;/* Size */ > + unsigned long flags;/* Resource flags */ > > - unsigned long bus_lower; > + pci_addr_t bus_lower; > }; > > #define PCI_REGION_MEM0x /* PCI memory space */ > @@ -330,9 +338,9 @@ struct pci_region { > #define PCI_REGION_RO 0x0200 /* Read-only memory */ > > extern __inline__ void pci_set_region(struct pci_region *reg, > - unsigned long bus_start, > + pci_addr_t bus_start, > phys_addr_t phys_start, > - unsigned long size, > + pci_size_t size, > unsigned long flags) { > reg->bus_start = bus_start; > reg->phys_start = phys_start; > @@ -433,7 +441,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); > + pci_addr_t addr, unsigned long flags); > extern unsigned long pci_hose_phys_to_bus(struct pci_controller* hose, > phys_addr_t addr, unsigned long > flags); > Should this return pci_addr_t? Cheers, Becky ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2] pci: Allow for PCI addresses to be 64-bit
PCI bus is inherently 64-bit. While not all system require access to the full 64-bit PCI address range some do. This allows those systems to enable the full PCI address width via CONFIG_SYS_PCI_64BIT. Signed-off-by: Kumar Gala <[EMAIL PROTECTED]> --- moved to CONFIG_SYS_PCI_64BIT instead of CONFIG_PCI_64BIT - k drivers/pci/pci.c |4 ++-- drivers/pci/pci_auto.c | 22 +++--- include/pci.h | 24 3 files changed, 29 insertions(+), 21 deletions(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 41780db..127d50f 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; + pci_addr_t 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, +pci_addr_t 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..547ce5b 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; + pci_addr_t 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", (u64)addr, (u64)res->bus_lower); *bar = addr; return 0; @@ -289,10 +289,10 @@ 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, + (u64)hose->pci_mem->bus_start, + (u64)(hose->pci_mem->bus_start + hose->pci_mem->size - 1), hose->pci_mem->phys_start, hose->pci_mem->phys_start + hose->pci_mem->size - 1); } @@ -300,10 +300,10 @@ 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, + (u64)hose->pci_prefetch->bus_start, + (u64)(hose->pci_prefetch->bus_start + hose->pci_prefetch->size - 1), hose->pci_prefetch->phys_start, hose->pci_prefetch->phys_start + hose->pci_prefetch->size - 1); @@ -312,10 +312,10 @@ 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, + (u64)hose->pci_io->bus_start, + (u64)(hose->pci_io->bus_start + hose->pci_io->size - 1), hose->pci_io->phys_start, hose->pci_io->phys_start + hose->pci_io->size - 1); diff --git a/include/pci.h b/include/pci.h index 1c8e216..d79f26a 100644 --- a/include/pci.h +++ b/include/pci.h @@ -312,13 +312,21 @@ #include +#ifdef CONFIG_SYS_PCI_64BIT +typedef u64 pci_addr_t; +typedef u64 pci_size_t; +#else +typedef u32 pci_addr_t; +typedef u32 pci_size_t; +#endif + 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 */ + pci_addr_t bus_start; /* Start on the bus */ + phys_addr_t phys_start; /* Start in physical add