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

[Becky Bruce]

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 <galak at kernel.crashing.org>
> ---
>
> 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 <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,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