[U-Boot] [PATCH v2] pci: Allow for PCI addresses to be 64-bit

Becky Bruce becky.bruce at freescale.com
Wed Oct 22 19:11:45 CEST 2008


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 <galak at kernel.crashing.org>
> ---
>
> 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, 0xffffffff);
> +				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, 0x00000000);
> +#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 = 0xffffffff;
> +	*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,
> 				   & 0xffff) + 1;
> 			bar_res = io;
>
> -			DEBUGF("PCI Autoconfig: BAR %d, I/O, size=0x%x, ", bar_nr,  
> bar_size);
> +			DEBUGF("PCI Autoconfig: BAR %d, I/O, size=0x%llx, ", bar_nr,  
> (u64)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, 0xffffffff);
> +				pci_hose_read_config_dword(hose, dev, bar+4,  
> &bar_response_upper);
> +
> +				bar64 = ((u64)bar_response_upper << 32) | bar_response;
>
> -			bar_size = ~(bar_response & PCI_BASE_ADDRESS_MEM_MASK) + 1;
> +				bar_size = ~(bar64 & PCI_BASE_ADDRESS_MEM_MASK) + 1;
> +				found_mem64 = 1;
> +			} else {
> +				bar_size = (u32)(~(bar_response & PCI_BASE_ADDRESS_MEM_MASK) +  
> 1);
> +			}
> 			if (prefetch && (bar_response & PCI_BASE_ADDRESS_MEM_PREFETCH))
> 				bar_res = prefetch;
> 			else
> 				bar_res = mem;
>
> -			DEBUGF("PCI Autoconfig: BAR %d, Mem, size=0x%x, ", bar_nr,  
> bar_size);
> +			DEBUGF("PCI Autoconfig: BAR %d, Mem, size=0x%llx, ", bar_nr,  
> (u64)bar_size);
> 		}
>
> 		if (pciauto_region_allocate(bar_res, bar_size, &bar_value) == 0) {
> 			/* 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 we are a 64-bit decoder then increment to the
> -			 * upper 32 bits of the bar and force it to locate
> -			 * in the lower 4GB of memory.
> -			 */
> 			if (found_mem64) {
> 				bar += 4;
> +#ifdef CONFIG_SYS_PCI_64BIT
> +				pci_hose_write_config_dword(hose, dev, bar, bar_value>>32);

same as earlier.....

>
> +#else
> +				/*
> +				 * If we are a 64-bit decoder then increment to the
> +				 * upper 32 bits of the bar and force it to locate
> +				 * in the lower 4GB of memory.
> +				 */
> 				pci_hose_write_config_dword(hose, dev, bar, 0x00000000);
> +#endif
> 			}
>
> 			cmdstat |= (bar_response & PCI_BASE_ADDRESS_SPACE) ?
> @@ -289,10 +304,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);
> 	}

Don't the phys_start and size need to print as 64b?  The region  
elements are defined as phys_addr_t and pci_size_t now.  Also would  
prefer to see a "0x" prefix on those hex prints, since you're changing  
this anyway.

>
> @@ -300,10 +315,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);

64b prints of phys_start and size? "0x" prefix?

>
> @@ -312,10 +327,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);

phys_start/size should print as 64b? "0x" prefix?

>
>
> diff --git a/include/pci.h b/include/pci.h
> index 1c8e216..eebe8a8 100644
> --- a/include/pci.h
> +++ b/include/pci.h
> @@ -101,8 +101,8 @@
> #define  PCI_BASE_ADDRESS_MEM_TYPE_1M	0x02	/* Below 1M [obsolete] */
> #define  PCI_BASE_ADDRESS_MEM_TYPE_64	0x04	/* 64 bit address */
> #define  PCI_BASE_ADDRESS_MEM_PREFETCH	0x08	/* prefetchable? */
> -#define  PCI_BASE_ADDRESS_MEM_MASK	(~0x0fUL)
> -#define  PCI_BASE_ADDRESS_IO_MASK	(~0x03UL)
> +#define  PCI_BASE_ADDRESS_MEM_MASK	(~0x0fULL)
> +#define  PCI_BASE_ADDRESS_IO_MASK	(~0x03ULL)
> /* bit 1 is reserved if address_space = 1 */
>
> /* Header type 0 (normal devices) */
> @@ -111,7 +111,7 @@
> #define PCI_SUBSYSTEM_ID	0x2e
> #define PCI_ROM_ADDRESS		0x30	/* Bits 31..11 are address, 10..1  
> reserved */
> #define  PCI_ROM_ADDRESS_ENABLE 0x01
> -#define PCI_ROM_ADDRESS_MASK	(~0x7ffUL)
> +#define PCI_ROM_ADDRESS_MASK	(~0x7ffULL)
>
> #define PCI_CAPABILITY_LIST	0x34	/* Offset of first capability list  
> entry */
>
> @@ -312,13 +312,21 @@
>
> #include <pci_ids.h>
>
> +#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_MEM		0x00000000	/* PCI memory space */
> @@ -330,9 +338,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,
> +				      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,9 +441,9 @@ 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);
> -extern unsigned long pci_hose_phys_to_bus(struct pci_controller*  
> hose,
> -					  phys_addr_t addr, unsigned long flags);
> +					pci_addr_t addr, unsigned long flags);

check whitespace here

>
> +extern pci_addr_t pci_hose_phys_to_bus(struct pci_controller* hose,
> +					phys_addr_t addr, unsigned long flags);
>
> #define pci_phys_to_bus(dev, addr, flags) \
> 	pci_hose_phys_to_bus(pci_bus_to_hose(PCI_BUS(dev)), (addr), (flags))
> @@ -483,8 +491,8 @@ extern int pci_hose_scan(struct pci_controller  
> *hose);
> extern int pci_hose_scan_bus(struct pci_controller *hose, int bus);
>
> extern void pciauto_region_init(struct pci_region* res);
> -extern void pciauto_region_align(struct pci_region *res, unsigned  
> long size);
> -extern int pciauto_region_allocate(struct pci_region* res, unsigned  
> int size, unsigned int *bar);
> +extern void pciauto_region_align(struct pci_region *res, pci_size_t  
> size);
> +extern int pciauto_region_allocate(struct pci_region* res,  
> pci_size_t size, pci_addr_t *bar);
> extern void pciauto_setup_device(struct pci_controller *hose,
> 				 pci_dev_t dev, int bars_num,
> 				 struct pci_region *mem,
> @@ -500,7 +508,7 @@ extern pci_dev_t pci_find_class(int  
> wanted_class, int wanted_sub_code,
> extern 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);
>
> #ifdef CONFIG_MPC824X
> -- 
> 1.5.5.1





More information about the U-Boot mailing list