diff mbox

[GIT,PULL] io.h clean-up for PCI

Message ID 201207191412.00540.arnd@arndb.de (mailing list archive)
State New, archived
Headers show

Commit Message

Arnd Bergmann July 19, 2012, 2:12 p.m. UTC
On Monday 16 July 2012, Rob Herring wrote:
> Arnd,
> 
> Please pull io.h PCI clean-up series. As you suggested, lets get it in
> next for test and decide later if to apply it for 3.6 or wait.
> 
> BTW, I'll have sporadic email access over the next 2 weeks.

I think we need this one:

iop13xx ended up with two conflicting definitions for
IOP13XX_PCIE_LOWER_IO_BA, where one of them is used to set up
the hardware window and should be zero, while the other one is
used to set the location of the second mapping into the virtual
address space.

This kills the second definition and hardcodes the 64KB offset
that is already hardcoded in the bios32.c file now.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

index 9278b8c..cc9e058 100644

Comments

Rob Herring July 24, 2012, 12:33 p.m. UTC | #1
On 07/19/2012 09:12 AM, Arnd Bergmann wrote:
> On Monday 16 July 2012, Rob Herring wrote:
>> Arnd,
>>
>> Please pull io.h PCI clean-up series. As you suggested, lets get it in
>> next for test and decide later if to apply it for 3.6 or wait.
>>
>> BTW, I'll have sporadic email access over the next 2 weeks.
> 
> I think we need this one:
> 
> iop13xx ended up with two conflicting definitions for
> IOP13XX_PCIE_LOWER_IO_BA, where one of them is used to set up
> the hardware window and should be zero, while the other one is
> used to set the location of the second mapping into the virtual
> address space.
> 
> This kills the second definition and hardcodes the 64KB offset
> that is already hardcoded in the bios32.c file now.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> index 9278b8c..cc9e058 100644
> --- a/arch/arm/mach-iop13xx/include/mach/iop13xx.h
> +++ b/arch/arm/mach-iop13xx/include/mach/iop13xx.h
> @@ -95,7 +95,6 @@ extern unsigned long get_iop_tick_rate(void);
>  /* PCI-E ranges */
>  #define IOP13XX_PCIE_LOWER_IO_PA      	 0xfffd0000UL
>  #define IOP13XX_PCIE_LOWER_IO_BA      	 0x0UL  /* OIOTVR */
> -#define IOP13XX_PCIE_LOWER_IO_BA      	 0x10000UL

This means we have PCIE and PCIX buses both using i/o bus addresses
starting at 0x0. We can't have that, right?

The requested resource won't match either as the resource start address
is bus_nr * 64K.

Rob

>  
>  #define IOP13XX_PCIE_MEM_PHYS_OFFSET  	 0x200000000ULL
>  #define IOP13XX_PCIE_MEM_WINDOW_SIZE  	 0x3a000000UL
> diff --git a/arch/arm/mach-iop13xx/pci.c b/arch/arm/mach-iop13xx/pci.c
> index 56a4b41..9cad41b 100644
> --- a/arch/arm/mach-iop13xx/pci.c
> +++ b/arch/arm/mach-iop13xx/pci.c
> @@ -1058,7 +1058,7 @@ int iop13xx_pci_setup(int nr, struct pci_sys_data *sys)
>  
>  		__raw_writel(pcsr, IOP13XX_ATUE_PCSR);
>  
> -		pci_ioremap_io(IOP13XX_PCIE_LOWER_IO_BA, IOP13XX_PCIE_LOWER_IO_PA);
> +		pci_ioremap_io(SZ_64K, IOP13XX_PCIE_LOWER_IO_PA);
>  
>  		res->start = IOP13XX_PCIE_LOWER_MEM_RA;
>  		res->end   = IOP13XX_PCIE_UPPER_MEM_RA;
> 
>
Arnd Bergmann July 24, 2012, 12:43 p.m. UTC | #2
On Tuesday 24 July 2012, Rob Herring wrote:
> > --- a/arch/arm/mach-iop13xx/include/mach/iop13xx.h
> > +++ b/arch/arm/mach-iop13xx/include/mach/iop13xx.h
> > @@ -95,7 +95,6 @@ extern unsigned long get_iop_tick_rate(void);
> >  /* PCI-E ranges */
> >  #define IOP13XX_PCIE_LOWER_IO_PA              0xfffd0000UL
> >  #define IOP13XX_PCIE_LOWER_IO_BA              0x0UL  /* OIOTVR */
> > -#define IOP13XX_PCIE_LOWER_IO_BA              0x10000UL
> 
> This means we have PCIE and PCIX buses both using i/o bus addresses
> starting at 0x0. We can't have that, right?
> 
> The requested resource won't match either as the resource start address
> is bus_nr * 64K.

Well, this is the number that gets written into the outbound
translation window register, which has to be zero AFAICT.

The PCI device you plug into the bus will always see its io
ports as being between zero and 65536 -- the part that
stays at 0x10000UL is the offset address that we use in Linux
to give it a unique address in the virtual address space
window we use to cover all the io port ranges.

The io_offset still gets set to 0x10000, so this number always
gets added and subtracted when converting between Linux port
numbers and bus-specific port numbers.

	Arnd
Rob Herring July 24, 2012, 1:25 p.m. UTC | #3
On 07/24/2012 07:43 AM, Arnd Bergmann wrote:
> On Tuesday 24 July 2012, Rob Herring wrote:
>>> --- a/arch/arm/mach-iop13xx/include/mach/iop13xx.h
>>> +++ b/arch/arm/mach-iop13xx/include/mach/iop13xx.h
>>> @@ -95,7 +95,6 @@ extern unsigned long get_iop_tick_rate(void);
>>>  /* PCI-E ranges */
>>>  #define IOP13XX_PCIE_LOWER_IO_PA              0xfffd0000UL
>>>  #define IOP13XX_PCIE_LOWER_IO_BA              0x0UL  /* OIOTVR */
>>> -#define IOP13XX_PCIE_LOWER_IO_BA              0x10000UL
>>
>> This means we have PCIE and PCIX buses both using i/o bus addresses
>> starting at 0x0. We can't have that, right?
>>
>> The requested resource won't match either as the resource start address
>> is bus_nr * 64K.
> 
> Well, this is the number that gets written into the outbound
> translation window register, which has to be zero AFAICT.
> 
> The PCI device you plug into the bus will always see its io
> ports as being between zero and 65536 -- the part that
> stays at 0x10000UL is the offset address that we use in Linux
> to give it a unique address in the virtual address space
> window we use to cover all the io port ranges.
> 
> The io_offset still gets set to 0x10000, so this number always
> gets added and subtracted when converting between Linux port
> numbers and bus-specific port numbers.

Okay, then this is probably something I need to fix.

How is mem_offset supposed to be set? Generally memory is setup as cpu
base paddr = pci mem addr, so mem_offset should be 0? But integrator is
set to the cpu paddr, and it works.

Rob

> 
> 	Arnd
>
Arnd Bergmann July 24, 2012, 2:18 p.m. UTC | #4
On Tuesday 24 July 2012, Rob Herring wrote:
> How is mem_offset supposed to be set? Generally memory is setup as cpu
> base paddr = pci mem addr, so mem_offset should be 0? But integrator is
> set to the cpu paddr, and it works.

I don't really know. I would assume that setting mem_offset to something
other than 0 means your bus address space is not the same as the cpu
address space. The window that is usable by PCI should be specified
in the memory resource, not using the mem_offset, but I could be
interpreting that incorrectly.

	Arnd
Rob Herring July 25, 2012, 2:41 p.m. UTC | #5
On 07/24/2012 07:43 AM, Arnd Bergmann wrote:
> On Tuesday 24 July 2012, Rob Herring wrote:
>>> --- a/arch/arm/mach-iop13xx/include/mach/iop13xx.h
>>> +++ b/arch/arm/mach-iop13xx/include/mach/iop13xx.h
>>> @@ -95,7 +95,6 @@ extern unsigned long get_iop_tick_rate(void);
>>>  /* PCI-E ranges */
>>>  #define IOP13XX_PCIE_LOWER_IO_PA              0xfffd0000UL
>>>  #define IOP13XX_PCIE_LOWER_IO_BA              0x0UL  /* OIOTVR */
>>> -#define IOP13XX_PCIE_LOWER_IO_BA              0x10000UL
>>
>> This means we have PCIE and PCIX buses both using i/o bus addresses
>> starting at 0x0. We can't have that, right?
>>
>> The requested resource won't match either as the resource start address
>> is bus_nr * 64K.
> 
> Well, this is the number that gets written into the outbound
> translation window register, which has to be zero AFAICT.
> 
> The PCI device you plug into the bus will always see its io
> ports as being between zero and 65536 -- the part that
> stays at 0x10000UL is the offset address that we use in Linux
> to give it a unique address in the virtual address space
> window we use to cover all the io port ranges.
> 
> The io_offset still gets set to 0x10000, so this number always
> gets added and subtracted when converting between Linux port
> numbers and bus-specific port numbers.

I don't think this is going to work right with the orion family. They
set the 2nd+ buses to some non-zero bus value.

	{ 0, KIRKWOOD_PCIE_IO_PHYS_BASE, KIRKWOOD_PCIE_IO_SIZE,
	  TARGET_PCIE, ATTR_PCIE_IO, KIRKWOOD_PCIE_IO_BUS_BASE

This is 0.

	},
	{ 1, KIRKWOOD_PCIE_MEM_PHYS_BASE, KIRKWOOD_PCIE_MEM_SIZE,
	  TARGET_PCIE, ATTR_PCIE_MEM, KIRKWOOD_PCIE_MEM_BUS_BASE
	},
	{ 2, KIRKWOOD_PCIE1_IO_PHYS_BASE, KIRKWOOD_PCIE1_IO_SIZE,
	  TARGET_PCIE, ATTR_PCIE1_IO, KIRKWOOD_PCIE1_IO_BUS_BASE

This was 0x100000 and is now 0x10000.

	},
	{ 3, KIRKWOOD_PCIE1_MEM_PHYS_BASE, KIRKWOOD_PCIE1_MEM_SIZE,
	  TARGET_PCIE, ATTR_PCIE1_MEM, KIRKWOOD_PCIE1_MEM_BUS_BASE
	},

If we change all these to 0, then I need to go back thru all the platforms.

Rob
diff mbox

Patch

--- a/arch/arm/mach-iop13xx/include/mach/iop13xx.h
+++ b/arch/arm/mach-iop13xx/include/mach/iop13xx.h
@@ -95,7 +95,6 @@  extern unsigned long get_iop_tick_rate(void);
 /* PCI-E ranges */
 #define IOP13XX_PCIE_LOWER_IO_PA      	 0xfffd0000UL
 #define IOP13XX_PCIE_LOWER_IO_BA      	 0x0UL  /* OIOTVR */
-#define IOP13XX_PCIE_LOWER_IO_BA      	 0x10000UL
 
 #define IOP13XX_PCIE_MEM_PHYS_OFFSET  	 0x200000000ULL
 #define IOP13XX_PCIE_MEM_WINDOW_SIZE  	 0x3a000000UL
diff --git a/arch/arm/mach-iop13xx/pci.c b/arch/arm/mach-iop13xx/pci.c
index 56a4b41..9cad41b 100644
--- a/arch/arm/mach-iop13xx/pci.c
+++ b/arch/arm/mach-iop13xx/pci.c
@@ -1058,7 +1058,7 @@  int iop13xx_pci_setup(int nr, struct pci_sys_data *sys)
 
 		__raw_writel(pcsr, IOP13XX_ATUE_PCSR);
 
-		pci_ioremap_io(IOP13XX_PCIE_LOWER_IO_BA, IOP13XX_PCIE_LOWER_IO_PA);
+		pci_ioremap_io(SZ_64K, IOP13XX_PCIE_LOWER_IO_PA);
 
 		res->start = IOP13XX_PCIE_LOWER_MEM_RA;
 		res->end   = IOP13XX_PCIE_UPPER_MEM_RA;