* [Query/Discussion]: IO translation with designware PCIe controller @ 2013-12-05 5:04 Pratyush Anand 2013-12-05 21:33 ` Arnd Bergmann 0 siblings, 1 reply; 17+ messages in thread From: Pratyush Anand @ 2013-12-05 5:04 UTC (permalink / raw) To: Arnd Bergmann, Jingoo Han Cc: Mohit Kumar, Marek Vasut, Richard Zhu, Kishon Vijay Abraham I, linux-pci, Tim Harvey Hi Arnd / Jingoo, Sorry for not able to follow earlier discussion you had on this topic while initial reviews of patches from Jingoo. I have few doubts that how will it actually work. As per my understanding under current implementation: 1. Physical IO addresses are in the range of 0x1000 to 0xfffff. 2. They are mapped to fixed virtual address 0xFEE00000 using pci_ioremap_io. 3. IO resource addresses to any PCIe device will be allocated in above range. 4. Driver for that PCIe device will write in the above range only for IO transaction to the device. 5. As per current IO translation programming, its one to one translation. It means PCIe translation unit will have both input and output address in the range of 0x1000 to 0xfffff. Did I miss something, or if above statements are correct, then what I do not understand is how can designware PCIe translation unit accept input address in the range of 0x1000 to 0xfffff? For example in SPEAr1340, physically RAM is mapped on above addresses. PCIe address translation unit can accept address only in the range of core addresses which are assigned to PCIe RC ie 0x80000000-0x8FFFFFFF. Regards Pratyush ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Query/Discussion]: IO translation with designware PCIe controller 2013-12-05 5:04 [Query/Discussion]: IO translation with designware PCIe controller Pratyush Anand @ 2013-12-05 21:33 ` Arnd Bergmann 2013-12-06 9:12 ` Pratyush Anand 0 siblings, 1 reply; 17+ messages in thread From: Arnd Bergmann @ 2013-12-05 21:33 UTC (permalink / raw) To: Pratyush Anand Cc: Jingoo Han, Mohit Kumar, Marek Vasut, Richard Zhu, Kishon Vijay Abraham I, linux-pci, Tim Harvey On Thursday 05 December 2013, Pratyush Anand wrote: > Hi Arnd / Jingoo, > > Sorry for not able to follow earlier discussion you had on this > topic while initial reviews of patches from Jingoo. > > I have few doubts that how will it actually work. > > As per my understanding under current implementation: > > 1. Physical IO addresses are in the range of 0x1000 to 0xfffff. > 2. They are mapped to fixed virtual address 0xFEE00000 using > pci_ioremap_io. > 3. IO resource addresses to any PCIe device will be allocated in above > range. > 4. Driver for that PCIe device will write in the above range only for > IO transaction to the device. > 5. As per current IO translation programming, its one to one > translation. It means PCIe translation unit will have both input and > output address in the range of 0x1000 to 0xfffff. > > Did I miss something, or if above statements are correct, then what I > do not understand is how can designware PCIe translation unit accept > input address in the range of 0x1000 to 0xfffff? Hi Pratyush, I think the part that you are missing is that there are four, not two address spaces involved: 1. The "bus" I/O port address, in the range between 0x1000 and 0xffff (65535). This is the address used in data frames in the actual bus transaction going over the PCIe wires. If you have more than one PCIe host bridge, each of them can normally have a range of up to 65536 addresses. The bus addresses get written into BAR registers of the device. 2. The Linux I/O port numbers, between 0x00000 and 0xfffff (1048575). This is a logical number space that aggregates all bus I/O port numbers. The first 4096 ports are normally reserved for ISA devices and are not available for PCIe resources. Each PCI or PCIe host bridge can have a 0x10000 byte naturally aligned range in here. The common case is that you have only one host bridge using the range 0x1000-0xffff. If you have more than one, there may be an offset between them, e.g. the second PCIe host may have io_offset set to 0x10000, which means that its bus range 0x1000-0xffff gets translated to Linux port number 0x11000-0x1ffff. Linux port numbers are visible e.g. in /proc/ioport and /dev/port 3. The CPU physical address space: Each PCIe host normally has its bus I/O port space mapped into PCU visible addresses, either hardwired or (in case of dw_pcie) set up using an outbound translation window. These addresses are what get passed to pci_ioremap_io. 4. The CPU virtual address space: This is an implementation detail of the kernel, which results in the Linux I/O port numbers 0x0-0xfffff to be mapped at 0xfee00000-0xfeefffff. > For example in SPEAr1340, physically RAM is mapped on above addresses. > PCIe address translation unit can accept address only in the range of > core addresses which are assigned to PCIe RC ie 0x80000000-0x8FFFFFFF. Since this is a physical address, it corresponds to address space 3 in my list above, and the address you pick here is what you pass to pci_ioremap_io. Arnd ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Query/Discussion]: IO translation with designware PCIe controller 2013-12-05 21:33 ` Arnd Bergmann @ 2013-12-06 9:12 ` Pratyush Anand 2013-12-06 14:46 ` Arnd Bergmann 0 siblings, 1 reply; 17+ messages in thread From: Pratyush Anand @ 2013-12-06 9:12 UTC (permalink / raw) To: Arnd Bergmann Cc: Jingoo Han, Mohit KUMAR DCG, Marek Vasut, Richard Zhu, Kishon Vijay Abraham I, linux-pci@vger.kernel.org, Tim Harvey Hi Arnd, Thanks a lot for detailed explanation. On Fri, Dec 06, 2013 at 05:33:12AM +0800, Arnd Bergmann wrote: > On Thursday 05 December 2013, Pratyush Anand wrote: > > Hi Arnd / Jingoo, > > > > Sorry for not able to follow earlier discussion you had on this > > topic while initial reviews of patches from Jingoo. > > > > I have few doubts that how will it actually work. > > > > As per my understanding under current implementation: > > > > 1. Physical IO addresses are in the range of 0x1000 to 0xfffff. > > 2. They are mapped to fixed virtual address 0xFEE00000 using > > pci_ioremap_io. > > 3. IO resource addresses to any PCIe device will be allocated in above > > range. > > 4. Driver for that PCIe device will write in the above range only for > > IO transaction to the device. > > 5. As per current IO translation programming, its one to one > > translation. It means PCIe translation unit will have both input and > > output address in the range of 0x1000 to 0xfffff. > > > > Did I miss something, or if above statements are correct, then what I > > do not understand is how can designware PCIe translation unit accept > > input address in the range of 0x1000 to 0xfffff? > > Hi Pratyush, > > I think the part that you are missing is that there are four, not two > address spaces involved: > > 1. The "bus" I/O port address, in the range between 0x1000 and 0xffff > (65535). This is the address used in data frames in the actual bus > transaction going over the PCIe wires. If you have more than one PCIe > host bridge, each of them can normally have a range of up to 65536 > addresses. The bus addresses get written into BAR registers of the > device. Correct. Got this point. So this address should be output of designware outbound address translation unit. > > 2. The Linux I/O port numbers, between 0x00000 and 0xfffff (1048575). > This is a logical number space that aggregates all bus I/O port numbers. > The first 4096 ports are normally reserved for ISA devices and are > not available for PCIe resources. Each PCI or PCIe host bridge can have > a 0x10000 byte naturally aligned range in here. The common case is that > you have only one host bridge using the range 0x1000-0xffff. If you have > more than one, there may be an offset between them, e.g. the second PCIe > host may have io_offset set to 0x10000, which means that its bus range > 0x1000-0xffff gets translated to Linux port number 0x11000-0x1ffff. > Linux port numbers are visible e.g. in /proc/ioport and /dev/port Ok, got it. To know more, which binding function maps this logical number space to address space in 4? > > 3. The CPU physical address space: Each PCIe host normally has its > bus I/O port space mapped into PCU visible addresses, either hardwired > or (in case of dw_pcie) set up using an outbound translation window. > These addresses are what get passed to pci_ioremap_io. OK. > > 4. The CPU virtual address space: This is an implementation detail > of the kernel, which results in the Linux I/O port numbers 0x0-0xfffff > to be mapped at 0xfee00000-0xfeefffff. > > > For example in SPEAr1340, physically RAM is mapped on above addresses. > > PCIe address translation unit can accept address only in the range of > > core addresses which are assigned to PCIe RC ie 0x80000000-0x8FFFFFFF. > > Since this is a physical address, it corresponds to address space 3 in > my list above, and the address you pick here is what you pass to > pci_ioremap_io. This is what I was expecting. But currently designware driver does not pass this address to pci_ioremap_io. OK.. We will fix it and will send patch. Regards Pratyush > > Arnd ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Query/Discussion]: IO translation with designware PCIe controller 2013-12-06 9:12 ` Pratyush Anand @ 2013-12-06 14:46 ` Arnd Bergmann 2013-12-09 7:12 ` Pratyush Anand 0 siblings, 1 reply; 17+ messages in thread From: Arnd Bergmann @ 2013-12-06 14:46 UTC (permalink / raw) To: Pratyush Anand Cc: Jingoo Han, Mohit KUMAR DCG, Marek Vasut, Richard Zhu, Kishon Vijay Abraham I, linux-pci@vger.kernel.org, Tim Harvey On Friday 06 December 2013, Pratyush Anand wrote: > > > > 1. The "bus" I/O port address, in the range between 0x1000 and 0xffff > > (65535). This is the address used in data frames in the actual bus > > transaction going over the PCIe wires. If you have more than one PCIe > > host bridge, each of them can normally have a range of up to 65536 > > addresses. The bus addresses get written into BAR registers of the > > device. > > Correct. Got this point. So this address should be output of > designware outbound address translation unit. Right. > > 2. The Linux I/O port numbers, between 0x00000 and 0xfffff (1048575). > > This is a logical number space that aggregates all bus I/O port numbers. > > The first 4096 ports are normally reserved for ISA devices and are > > not available for PCIe resources. Each PCI or PCIe host bridge can have > > a 0x10000 byte naturally aligned range in here. The common case is that > > you have only one host bridge using the range 0x1000-0xffff. If you have > > more than one, there may be an offset between them, e.g. the second PCIe > > host may have io_offset set to 0x10000, which means that its bus range > > 0x1000-0xffff gets translated to Linux port number 0x11000-0x1ffff. > > Linux port numbers are visible e.g. in /proc/ioport and /dev/port > > Ok, got it. To know more, which binding function maps this logical > number space to address space in 4? There is just a linear offset between the two: PCI_IO_VIRT_BASE. This offset is applied in the __io() macro when converting from a port number to a pointer in various places in the kernel (inb/outb as well as pci_iomap/ioport_map), and the offset is added in pci_ioremap_io() when creating the page table entries. > > 4. The CPU virtual address space: This is an implementation detail > > of the kernel, which results in the Linux I/O port numbers 0x0-0xfffff > > to be mapped at 0xfee00000-0xfeefffff. > > > > > For example in SPEAr1340, physically RAM is mapped on above addresses. > > > PCIe address translation unit can accept address only in the range of > > > core addresses which are assigned to PCIe RC ie 0x80000000-0x8FFFFFFF. > > > > Since this is a physical address, it corresponds to address space 3 in > > my list above, and the address you pick here is what you pass to > > pci_ioremap_io. > > This is what I was expecting. But currently designware driver does not > pass this address to pci_ioremap_io. > OK.. We will fix it and will send patch. I think it does handle this correctly, look at static int dw_pcie_setup(int nr, struct pci_sys_data *sys) { ... if (global_io_offset < SZ_1M && pp->config.io_size > 0) { sys->io_offset = global_io_offset - pp->config.io_bus_addr; pci_ioremap_io(sys->io_offset, pp->io.start); global_io_offset += SZ_64K; pci_add_resource_offset(&sys->resources, &pp->io, sys->io_offset); } ... } I believe this does the right thing, but you have to put the correct translation into the 'ranges' property of the host bridge node in DT. Arnd ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Query/Discussion]: IO translation with designware PCIe controller 2013-12-06 14:46 ` Arnd Bergmann @ 2013-12-09 7:12 ` Pratyush Anand 2013-12-09 16:09 ` Arnd Bergmann 2013-12-10 13:26 ` Marek Vasut 0 siblings, 2 replies; 17+ messages in thread From: Pratyush Anand @ 2013-12-09 7:12 UTC (permalink / raw) To: Arnd Bergmann Cc: Jingoo Han, Mohit KUMAR DCG, Marek Vasut, Richard Zhu, Kishon Vijay Abraham I, linux-pci@vger.kernel.org, Tim Harvey Hi Arnd, On Fri, Dec 06, 2013 at 10:46:23PM +0800, Arnd Bergmann wrote: > On Friday 06 December 2013, Pratyush Anand wrote: > > > > [...] > > > > For example in SPEAr1340, physically RAM is mapped on above addresses. > > > > PCIe address translation unit can accept address only in the range of > > > > core addresses which are assigned to PCIe RC ie 0x80000000-0x8FFFFFFF. > > > > > > Since this is a physical address, it corresponds to address space 3 in > > > my list above, and the address you pick here is what you pass to > > > pci_ioremap_io. > > > > This is what I was expecting. But currently designware driver does not > > pass this address to pci_ioremap_io. > > OK.. We will fix it and will send patch. > > I think it does handle this correctly, look at > > static int dw_pcie_setup(int nr, struct pci_sys_data *sys) > { > ... > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > pci_ioremap_io(sys->io_offset, pp->io.start); > global_io_offset += SZ_64K; > pci_add_resource_offset(&sys->resources, &pp->io, > sys->io_offset); > } > ... > } > > I believe this does the right thing, but you have to put the correct > translation into the 'ranges' property of the host bridge node in DT. May be not exactly. pp->io is the realio, and it is passed correctly to pci_add_resource_offset. But, as you had also said that pci_ioremap_io will receive cpu physical address space as input, therefore I think following modification will be needed to work io transaction properly. diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c index be6ce30..cf68632 100644 --- a/drivers/pci/host/pcie-designware.c +++ b/drivers/pci/host/pcie-designware.c @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp) + global_io_offset); pp->config.io_size = resource_size(&pp->io); pp->config.io_bus_addr = range.pci_addr; + pp->io_base = range.cpu_addr; } if (restype == IORESOURCE_MEM) { of_pci_range_to_resource(&range, np, &pp->mem); @@ -403,7 +404,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) pp->cfg0_base = pp->cfg.start; pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; - pp->io_base = pp->io.start; pp->mem_base = pp->mem.start; pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, @@ -667,7 +667,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data *sys) if (global_io_offset < SZ_1M && pp->config.io_size > 0) { sys->io_offset = global_io_offset - pp->config.io_bus_addr; - pci_ioremap_io(sys->io_offset, pp->io.start); + pci_ioremap_io(sys->io_offset, pp->io_base); global_io_offset += SZ_64K; pci_add_resource_offset(&sys->resources, &pp->io, sys->io_offset); Regards Pratyush > > Arnd ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Query/Discussion]: IO translation with designware PCIe controller 2013-12-09 7:12 ` Pratyush Anand @ 2013-12-09 16:09 ` Arnd Bergmann 2013-12-10 4:34 ` Pratyush Anand 2013-12-10 13:26 ` Marek Vasut 1 sibling, 1 reply; 17+ messages in thread From: Arnd Bergmann @ 2013-12-09 16:09 UTC (permalink / raw) To: Pratyush Anand Cc: Jingoo Han, Mohit KUMAR DCG, Marek Vasut, Richard Zhu, Kishon Vijay Abraham I, linux-pci@vger.kernel.org, Tim Harvey On Monday 09 December 2013, Pratyush Anand wrote: > > I think it does handle this correctly, look at > > > > static int dw_pcie_setup(int nr, struct pci_sys_data *sys) > > { > > ... > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > > pci_ioremap_io(sys->io_offset, pp->io.start); > > global_io_offset += SZ_64K; > > pci_add_resource_offset(&sys->resources, &pp->io, > > sys->io_offset); > > } > > ... > > } > > > > I believe this does the right thing, but you have to put the correct > > translation into the 'ranges' property of the host bridge node in DT. > > May be not exactly. pp->io is the realio, and it is passed correctly > to pci_add_resource_offset. But, as you had also > said that pci_ioremap_io will receive cpu physical address space as > input, therefore I think following modification will be needed to work > io transaction properly. I see. I think you are right. > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > index be6ce30..cf68632 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > + global_io_offset); > pp->config.io_size = resource_size(&pp->io); > pp->config.io_bus_addr = range.pci_addr; > + pp->io_base = range.cpu_addr; > } > if (restype == IORESOURCE_MEM) { > of_pci_range_to_resource(&range, np, &pp->mem); > @@ -403,7 +404,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > pp->cfg0_base = pp->cfg.start; > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; > - pp->io_base = pp->io.start; > pp->mem_base = pp->mem.start; > > pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, This looks correct to me and it seems to also fix a bug in dw_pcie_prog_viewport_io_outbound if I read this correctly. > @@ -667,7 +667,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data *sys) > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > - pci_ioremap_io(sys->io_offset, pp->io.start); > + pci_ioremap_io(sys->io_offset, pp->io_base); > global_io_offset += SZ_64K; > pci_add_resource_offset(&sys->resources, &pp->io, > sys->io_offset); I think there is still a related bug in here: we should pass global_io_offset rather than sys->io_offset to pci_ioremap_io, so we map the new window into the first available spot in the Linux view of the I/O space, rather than passing a number that might be zero for any bus, if the 'ranges' are set up to have an identity mapping between Linux I/O spaces and PCI I/O spaces. You should be able to verify this by setting the I/O range for the bus to a random 4KB multiple in DT and observe that Linux start allocating ports from 0x1000 but the raw BAR values would contain the value you have chosen. Arnd ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Query/Discussion]: IO translation with designware PCIe controller 2013-12-09 16:09 ` Arnd Bergmann @ 2013-12-10 4:34 ` Pratyush Anand 2013-12-10 5:25 ` Jingoo Han 0 siblings, 1 reply; 17+ messages in thread From: Pratyush Anand @ 2013-12-10 4:34 UTC (permalink / raw) To: Arnd Bergmann, Jingoo Han, Mohit KUMAR DCG Cc: Marek Vasut, Richard Zhu, Kishon Vijay Abraham I, linux-pci@vger.kernel.org, Tim Harvey On Tue, Dec 10, 2013 at 12:09:37AM +0800, Arnd Bergmann wrote: > On Monday 09 December 2013, Pratyush Anand wrote: > > > I think it does handle this correctly, look at > > > > > > static int dw_pcie_setup(int nr, struct pci_sys_data *sys) > > > { > > > ... > > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > > > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > > > pci_ioremap_io(sys->io_offset, pp->io.start); > > > global_io_offset += SZ_64K; > > > pci_add_resource_offset(&sys->resources, &pp->io, > > > sys->io_offset); > > > } > > > ... > > > } > > > > > > I believe this does the right thing, but you have to put the correct > > > translation into the 'ranges' property of the host bridge node in DT. > > > > May be not exactly. pp->io is the realio, and it is passed correctly > > to pci_add_resource_offset. But, as you had also > > said that pci_ioremap_io will receive cpu physical address space as > > input, therefore I think following modification will be needed to work > > io transaction properly. > > I see. I think you are right. > > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > > index be6ce30..cf68632 100644 > > --- a/drivers/pci/host/pcie-designware.c > > +++ b/drivers/pci/host/pcie-designware.c > > @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > + global_io_offset); > > pp->config.io_size = resource_size(&pp->io); > > pp->config.io_bus_addr = range.pci_addr; > > + pp->io_base = range.cpu_addr; > > } > > if (restype == IORESOURCE_MEM) { > > of_pci_range_to_resource(&range, np, &pp->mem); > > @@ -403,7 +404,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > > > pp->cfg0_base = pp->cfg.start; > > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; > > - pp->io_base = pp->io.start; > > pp->mem_base = pp->mem.start; > > > > pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, > > This looks correct to me and it seems to also fix a bug in > dw_pcie_prog_viewport_io_outbound if I read this correctly. Yes, now outbound viewport for IO translation will get correct input address. > > > @@ -667,7 +667,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data *sys) > > > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > > - pci_ioremap_io(sys->io_offset, pp->io.start); > > + pci_ioremap_io(sys->io_offset, pp->io_base); > > global_io_offset += SZ_64K; > > pci_add_resource_offset(&sys->resources, &pp->io, > > sys->io_offset); > > I think there is still a related bug in here: we should pass global_io_offset rather > than sys->io_offset to pci_ioremap_io, so we map the new window into the first > available spot in the Linux view of the I/O space, rather than passing a number > that might be zero for any bus, if the 'ranges' are set up to have an identity > mapping between Linux I/O spaces and PCI I/O spaces. You should be able to verify > this by setting the I/O range for the bus to a random 4KB multiple in DT and > observe that Linux start allocating ports from 0x1000 but the raw BAR values > would contain the value you have chosen. OK. @ Jingoo, Mohit Is it possible for you to test following patch. diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c index be6ce30..b83f5e8 100644 --- a/drivers/pci/host/pcie-designware.c +++ b/drivers/pci/host/pcie-designware.c @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp) + global_io_offset); pp->config.io_size = resource_size(&pp->io); pp->config.io_bus_addr = range.pci_addr; + pp->io_base = range.cpu_addr; } if (restype == IORESOURCE_MEM) { of_pci_range_to_resource(&range, np, &pp->mem); @@ -403,7 +404,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) pp->cfg0_base = pp->cfg.start; pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; - pp->io_base = pp->io.start; pp->mem_base = pp->mem.start; pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, @@ -667,7 +667,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data *sys) if (global_io_offset < SZ_1M && pp->config.io_size > 0) { sys->io_offset = global_io_offset - pp->config.io_bus_addr; - pci_ioremap_io(sys->io_offset, pp->io.start); + pci_ioremap_io(global_io_offset, pp->io_base); global_io_offset += SZ_64K; pci_add_resource_offset(&sys->resources, &pp->io, sys->io_offset); Regards Pratyush > > Arnd ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Query/Discussion]: IO translation with designware PCIe controller 2013-12-10 4:34 ` Pratyush Anand @ 2013-12-10 5:25 ` Jingoo Han 2013-12-10 6:31 ` Mohit KUMAR DCG 0 siblings, 1 reply; 17+ messages in thread From: Jingoo Han @ 2013-12-10 5:25 UTC (permalink / raw) To: 'Pratyush Anand', 'Arnd Bergmann', 'Mohit KUMAR DCG' Cc: 'Marek Vasut', 'Richard Zhu', 'Kishon Vijay Abraham I', linux-pci, 'Tim Harvey', 'Jingoo Han' On Tuesday, December 10, 2013 1:34 PM, Pratyush Anand wrote: > On Tue, Dec 10, 2013 at 12:09:37AM +0800, Arnd Bergmann wrote: > > On Monday 09 December 2013, Pratyush Anand wrote: > > > > I think it does handle this correctly, look at > > > > > > > > static int dw_pcie_setup(int nr, struct pci_sys_data *sys) > > > > { > > > > ... > > > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > > > > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > > > > pci_ioremap_io(sys->io_offset, pp->io.start); > > > > global_io_offset += SZ_64K; > > > > pci_add_resource_offset(&sys->resources, &pp->io, > > > > sys->io_offset); > > > > } > > > > ... > > > > } > > > > > > > > I believe this does the right thing, but you have to put the correct > > > > translation into the 'ranges' property of the host bridge node in DT. > > > > > > May be not exactly. pp->io is the realio, and it is passed correctly > > > to pci_add_resource_offset. But, as you had also > > > said that pci_ioremap_io will receive cpu physical address space as > > > input, therefore I think following modification will be needed to work > > > io transaction properly. > > > > I see. I think you are right. > > > > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > > > index be6ce30..cf68632 100644 > > > --- a/drivers/pci/host/pcie-designware.c > > > +++ b/drivers/pci/host/pcie-designware.c > > > @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > > + global_io_offset); > > > pp->config.io_size = resource_size(&pp->io); > > > pp->config.io_bus_addr = range.pci_addr; > > > + pp->io_base = range.cpu_addr; > > > } > > > if (restype == IORESOURCE_MEM) { > > > of_pci_range_to_resource(&range, np, &pp->mem); > > > @@ -403,7 +404,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > > > > > pp->cfg0_base = pp->cfg.start; > > > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; > > > - pp->io_base = pp->io.start; > > > pp->mem_base = pp->mem.start; > > > > > > pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, > > > > This looks correct to me and it seems to also fix a bug in > > dw_pcie_prog_viewport_io_outbound if I read this correctly. > > Yes, now outbound viewport for IO translation will get correct input > address. > > > > > > @@ -667,7 +667,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data *sys) > > > > > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > > > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > > > - pci_ioremap_io(sys->io_offset, pp->io.start); > > > + pci_ioremap_io(sys->io_offset, pp->io_base); > > > global_io_offset += SZ_64K; > > > pci_add_resource_offset(&sys->resources, &pp->io, > > > sys->io_offset); > > > > I think there is still a related bug in here: we should pass global_io_offset rather > > than sys->io_offset to pci_ioremap_io, so we map the new window into the first > > available spot in the Linux view of the I/O space, rather than passing a number > > that might be zero for any bus, if the 'ranges' are set up to have an identity > > mapping between Linux I/O spaces and PCI I/O spaces. You should be able to verify > > this by setting the I/O range for the bus to a random 4KB multiple in DT and > > observe that Linux start allocating ports from 0x1000 but the raw BAR values > > would contain the value you have chosen. > > OK. > > @ Jingoo, Mohit > > Is it possible for you to test following patch. Hi Pratyush Anand, This patch works properly on Exynos platform with two different NICs. Also, I am tracing this discussion. Thank you for sending your patch. Best regards, Jingoo Han > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > index be6ce30..b83f5e8 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > + global_io_offset); > pp->config.io_size = resource_size(&pp->io); > pp->config.io_bus_addr = range.pci_addr; > + pp->io_base = range.cpu_addr; > } > if (restype == IORESOURCE_MEM) { > of_pci_range_to_resource(&range, np, &pp->mem); > @@ -403,7 +404,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > pp->cfg0_base = pp->cfg.start; > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; > - pp->io_base = pp->io.start; > pp->mem_base = pp->mem.start; > > pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, > @@ -667,7 +667,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data *sys) > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > - pci_ioremap_io(sys->io_offset, pp->io.start); > + pci_ioremap_io(global_io_offset, pp->io_base); > global_io_offset += SZ_64K; > pci_add_resource_offset(&sys->resources, &pp->io, > sys->io_offset); ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [Query/Discussion]: IO translation with designware PCIe controller 2013-12-10 5:25 ` Jingoo Han @ 2013-12-10 6:31 ` Mohit KUMAR DCG 2013-12-10 6:57 ` Pratyush Anand 2013-12-10 7:02 ` Jingoo Han 0 siblings, 2 replies; 17+ messages in thread From: Mohit KUMAR DCG @ 2013-12-10 6:31 UTC (permalink / raw) To: Jingoo Han, Pratyush ANAND, 'Arnd Bergmann' Cc: 'Marek Vasut', 'Richard Zhu', 'Kishon Vijay Abraham I', linux-pci@vger.kernel.org, 'Tim Harvey' Hello Pratyush, > -----Original Message----- > From: Jingoo Han [mailto:jg1.han@samsung.com] > Sent: Tuesday, December 10, 2013 10:55 AM > To: Pratyush ANAND; 'Arnd Bergmann'; Mohit KUMAR DCG > Cc: 'Marek Vasut'; 'Richard Zhu'; 'Kishon Vijay Abraham I'; linux- > pci@vger.kernel.org; 'Tim Harvey'; 'Jingoo Han' > Subject: Re: [Query/Discussion]: IO translation with designware PCIe > controller > > On Tuesday, December 10, 2013 1:34 PM, Pratyush Anand wrote: > > On Tue, Dec 10, 2013 at 12:09:37AM +0800, Arnd Bergmann wrote: > > > On Monday 09 December 2013, Pratyush Anand wrote: > > > > > I think it does handle this correctly, look at > > > > > > > > > > static int dw_pcie_setup(int nr, struct pci_sys_data *sys) { > > > > > ... > > > > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > > > > > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > > > > > pci_ioremap_io(sys->io_offset, pp->io.start); > > > > > global_io_offset += SZ_64K; > > > > > pci_add_resource_offset(&sys->resources, &pp->io, > > > > > sys->io_offset); > > > > > } > > > > > ... > > > > > } > > > > > > > > > > I believe this does the right thing, but you have to put the > > > > > correct translation into the 'ranges' property of the host bridge node > in DT. > > > > > > > > May be not exactly. pp->io is the realio, and it is passed > > > > correctly to pci_add_resource_offset. But, as you had also said > > > > that pci_ioremap_io will receive cpu physical address space as > > > > input, therefore I think following modification will be needed to > > > > work io transaction properly. > > > > > > I see. I think you are right. > > > > > > > diff --git a/drivers/pci/host/pcie-designware.c > > > > b/drivers/pci/host/pcie-designware.c > > > > index be6ce30..cf68632 100644 > > > > --- a/drivers/pci/host/pcie-designware.c > > > > +++ b/drivers/pci/host/pcie-designware.c > > > > @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port > *pp) > > > > + global_io_offset); > > > > pp->config.io_size = resource_size(&pp->io); > > > > pp->config.io_bus_addr = range.pci_addr; > > > > + pp->io_base = range.cpu_addr; > > > > } > > > > if (restype == IORESOURCE_MEM) { > > > > of_pci_range_to_resource(&range, np, &pp->mem); > @@ -403,7 > > > > +404,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > > > > > > > pp->cfg0_base = pp->cfg.start; > > > > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; > > > > - pp->io_base = pp->io.start; > > > > pp->mem_base = pp->mem.start; > > > > > > > > pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, > > > > > > This looks correct to me and it seems to also fix a bug in > > > dw_pcie_prog_viewport_io_outbound if I read this correctly. > > > > Yes, now outbound viewport for IO translation will get correct input > > address. > > > > > > > > > @@ -667,7 +667,7 @@ static int dw_pcie_setup(int nr, struct > > > > pci_sys_data *sys) > > > > > > > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > > > > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > > > > - pci_ioremap_io(sys->io_offset, pp->io.start); > > > > + pci_ioremap_io(sys->io_offset, pp->io_base); > > > > global_io_offset += SZ_64K; > > > > pci_add_resource_offset(&sys->resources, &pp->io, > > > > sys->io_offset); > > > > > > I think there is still a related bug in here: we should pass > > > global_io_offset rather than sys->io_offset to pci_ioremap_io, so we > > > map the new window into the first available spot in the Linux view > > > of the I/O space, rather than passing a number that might be zero > > > for any bus, if the 'ranges' are set up to have an identity mapping > > > between Linux I/O spaces and PCI I/O spaces. You should be able to > > > verify this by setting the I/O range for the bus to a random 4KB > > > multiple in DT and observe that Linux start allocating ports from 0x1000 > but the raw BAR values would contain the value you have chosen. > > > > OK. > > > > @ Jingoo, Mohit - Now its working properly for SPEAr1310 tested with directly connected EP(xhc cards) as well as EP Connected through switch(Lecroy PTC in AIC mode). Without this patch my EP devices were not working when connected through Switch. Thanks Mohit > > > > Is it possible for you to test following patch. > > Hi Pratyush Anand, > > This patch works properly on Exynos platform with two different NICs. Also, I > am tracing this discussion. > Thank you for sending your patch. > > Best regards, > Jingoo Han > > > > > diff --git a/drivers/pci/host/pcie-designware.c > > b/drivers/pci/host/pcie-designware.c > > index be6ce30..b83f5e8 100644 > > --- a/drivers/pci/host/pcie-designware.c > > +++ b/drivers/pci/host/pcie-designware.c > > @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > + global_io_offset); > > pp->config.io_size = resource_size(&pp->io); > > pp->config.io_bus_addr = range.pci_addr; > > + pp->io_base = range.cpu_addr; > > } > > if (restype == IORESOURCE_MEM) { > > of_pci_range_to_resource(&range, np, &pp->mem); > @@ -403,7 +404,6 > > @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > > > pp->cfg0_base = pp->cfg.start; > > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; > > - pp->io_base = pp->io.start; > > pp->mem_base = pp->mem.start; > > > > pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, @@ - > 667,7 > > +667,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data *sys) > > > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > > - pci_ioremap_io(sys->io_offset, pp->io.start); > > + pci_ioremap_io(global_io_offset, pp->io_base); > > global_io_offset += SZ_64K; > > pci_add_resource_offset(&sys->resources, &pp->io, > > sys->io_offset); > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Query/Discussion]: IO translation with designware PCIe controller 2013-12-10 6:31 ` Mohit KUMAR DCG @ 2013-12-10 6:57 ` Pratyush Anand 2013-12-10 7:02 ` Jingoo Han 1 sibling, 0 replies; 17+ messages in thread From: Pratyush Anand @ 2013-12-10 6:57 UTC (permalink / raw) To: 'Tim Harvey' Cc: Jingoo Han, 'Arnd Bergmann', Mohit KUMAR DCG, 'Marek Vasut', 'Richard Zhu', 'Kishon Vijay Abraham I', linux-pci@vger.kernel.org Hi Tim, On Tue, Dec 10, 2013 at 02:31:08PM +0800, Mohit KUMAR DCG wrote: > Hello Pratyush, > > > -----Original Message----- > > From: Jingoo Han [mailto:jg1.han@samsung.com] > > Sent: Tuesday, December 10, 2013 10:55 AM > > To: Pratyush ANAND; 'Arnd Bergmann'; Mohit KUMAR DCG > > Cc: 'Marek Vasut'; 'Richard Zhu'; 'Kishon Vijay Abraham I'; linux- > > pci@vger.kernel.org; 'Tim Harvey'; 'Jingoo Han' > > Subject: Re: [Query/Discussion]: IO translation with designware PCIe > > controller > > > > On Tuesday, December 10, 2013 1:34 PM, Pratyush Anand wrote: > > > On Tue, Dec 10, 2013 at 12:09:37AM +0800, Arnd Bergmann wrote: > > > > On Monday 09 December 2013, Pratyush Anand wrote: > > > > > > I think it does handle this correctly, look at > > > > > > > > > > > > static int dw_pcie_setup(int nr, struct pci_sys_data *sys) { > > > > > > ... > > > > > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > > > > > > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > > > > > > pci_ioremap_io(sys->io_offset, pp->io.start); > > > > > > global_io_offset += SZ_64K; > > > > > > pci_add_resource_offset(&sys->resources, &pp->io, > > > > > > sys->io_offset); > > > > > > } > > > > > > ... > > > > > > } > > > > > > > > > > > > I believe this does the right thing, but you have to put the > > > > > > correct translation into the 'ranges' property of the host bridge node > > in DT. > > > > > > > > > > May be not exactly. pp->io is the realio, and it is passed > > > > > correctly to pci_add_resource_offset. But, as you had also said > > > > > that pci_ioremap_io will receive cpu physical address space as > > > > > input, therefore I think following modification will be needed to > > > > > work io transaction properly. > > > > > > > > I see. I think you are right. > > > > > > > > > diff --git a/drivers/pci/host/pcie-designware.c > > > > > b/drivers/pci/host/pcie-designware.c > > > > > index be6ce30..cf68632 100644 > > > > > --- a/drivers/pci/host/pcie-designware.c > > > > > +++ b/drivers/pci/host/pcie-designware.c > > > > > @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port > > *pp) > > > > > + global_io_offset); > > > > > pp->config.io_size = resource_size(&pp->io); > > > > > pp->config.io_bus_addr = range.pci_addr; > > > > > + pp->io_base = range.cpu_addr; > > > > > } > > > > > if (restype == IORESOURCE_MEM) { > > > > > of_pci_range_to_resource(&range, np, &pp->mem); > > @@ -403,7 > > > > > +404,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > > > > > > > > > pp->cfg0_base = pp->cfg.start; > > > > > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; > > > > > - pp->io_base = pp->io.start; > > > > > pp->mem_base = pp->mem.start; > > > > > > > > > > pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, > > > > > > > > This looks correct to me and it seems to also fix a bug in > > > > dw_pcie_prog_viewport_io_outbound if I read this correctly. > > > > > > Yes, now outbound viewport for IO translation will get correct input > > > address. > > > > > > > > > > > > @@ -667,7 +667,7 @@ static int dw_pcie_setup(int nr, struct > > > > > pci_sys_data *sys) > > > > > > > > > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > > > > > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > > > > > - pci_ioremap_io(sys->io_offset, pp->io.start); > > > > > + pci_ioremap_io(sys->io_offset, pp->io_base); > > > > > global_io_offset += SZ_64K; > > > > > pci_add_resource_offset(&sys->resources, &pp->io, > > > > > sys->io_offset); > > > > > > > > I think there is still a related bug in here: we should pass > > > > global_io_offset rather than sys->io_offset to pci_ioremap_io, so we > > > > map the new window into the first available spot in the Linux view > > > > of the I/O space, rather than passing a number that might be zero > > > > for any bus, if the 'ranges' are set up to have an identity mapping > > > > between Linux I/O spaces and PCI I/O spaces. You should be able to > > > > verify this by setting the I/O range for the bus to a random 4KB > > > > multiple in DT and observe that Linux start allocating ports from 0x1000 > > but the raw BAR values would contain the value you have chosen. > > > > > > OK. > > > > > > @ Jingoo, Mohit > > - Now its working properly for SPEAr1310 tested with directly connected EP(xhc cards) as well as EP > Connected through switch(Lecroy PTC in AIC mode). Without this patch my EP devices were not working when > connected through Switch. > Can you also check if this patch resolves your issue, and you are able to work with switch without commenting dw_pcie_prog_viewport_io_outbound. Regards Pratyush > Thanks > Mohit > > > > > > > > Is it possible for you to test following patch. > > > > Hi Pratyush Anand, > > > > This patch works properly on Exynos platform with two different NICs. Also, I > > am tracing this discussion. > > Thank you for sending your patch. > > > > Best regards, > > Jingoo Han > > > > > > > > diff --git a/drivers/pci/host/pcie-designware.c > > > b/drivers/pci/host/pcie-designware.c > > > index be6ce30..b83f5e8 100644 > > > --- a/drivers/pci/host/pcie-designware.c > > > +++ b/drivers/pci/host/pcie-designware.c > > > @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > > + global_io_offset); > > > pp->config.io_size = resource_size(&pp->io); > > > pp->config.io_bus_addr = range.pci_addr; > > > + pp->io_base = range.cpu_addr; > > > } > > > if (restype == IORESOURCE_MEM) { > > > of_pci_range_to_resource(&range, np, &pp->mem); > > @@ -403,7 +404,6 > > > @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > > > > > pp->cfg0_base = pp->cfg.start; > > > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; > > > - pp->io_base = pp->io.start; > > > pp->mem_base = pp->mem.start; > > > > > > pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, @@ - > > 667,7 > > > +667,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data *sys) > > > > > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > > > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > > > - pci_ioremap_io(sys->io_offset, pp->io.start); > > > + pci_ioremap_io(global_io_offset, pp->io_base); > > > global_io_offset += SZ_64K; > > > pci_add_resource_offset(&sys->resources, &pp->io, > > > sys->io_offset); > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Query/Discussion]: IO translation with designware PCIe controller 2013-12-10 6:31 ` Mohit KUMAR DCG 2013-12-10 6:57 ` Pratyush Anand @ 2013-12-10 7:02 ` Jingoo Han 2013-12-13 7:36 ` Hong-Xing.Zhu 1 sibling, 1 reply; 17+ messages in thread From: Jingoo Han @ 2013-12-10 7:02 UTC (permalink / raw) To: 'Marek Vasut', 'Richard Zhu' Cc: 'Mohit KUMAR DCG', 'Pratyush ANAND', 'Arnd Bergmann', 'Kishon Vijay Abraham I', linux-pci, 'Tim Harvey', 'Jingoo Han' On Tuesday, December 10, 2013 3:31 PM, Mohit KUMAR DCG wrote: > On Tuesday, December 10, 2013 10:55 AM, Jingoo Han wrote: > > On Tuesday, December 10, 2013 1:34 PM, Pratyush Anand wrote: > > > On Tue, Dec 10, 2013 at 12:09:37AM +0800, Arnd Bergmann wrote: > > > > On Monday 09 December 2013, Pratyush Anand wrote: > > > > > > I think it does handle this correctly, look at > > > > > > > > > > > > static int dw_pcie_setup(int nr, struct pci_sys_data *sys) { > > > > > > ... > > > > > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > > > > > > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > > > > > > pci_ioremap_io(sys->io_offset, pp->io.start); > > > > > > global_io_offset += SZ_64K; > > > > > > pci_add_resource_offset(&sys->resources, &pp->io, > > > > > > sys->io_offset); > > > > > > } > > > > > > ... > > > > > > } > > > > > > > > > > > > I believe this does the right thing, but you have to put the > > > > > > correct translation into the 'ranges' property of the host bridge node > > in DT. > > > > > > > > > > May be not exactly. pp->io is the realio, and it is passed > > > > > correctly to pci_add_resource_offset. But, as you had also said > > > > > that pci_ioremap_io will receive cpu physical address space as > > > > > input, therefore I think following modification will be needed to > > > > > work io transaction properly. > > > > > > > > I see. I think you are right. > > > > > > > > > diff --git a/drivers/pci/host/pcie-designware.c > > > > > b/drivers/pci/host/pcie-designware.c > > > > > index be6ce30..cf68632 100644 > > > > > --- a/drivers/pci/host/pcie-designware.c > > > > > +++ b/drivers/pci/host/pcie-designware.c > > > > > @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port > > *pp) > > > > > + global_io_offset); > > > > > pp->config.io_size = resource_size(&pp->io); > > > > > pp->config.io_bus_addr = range.pci_addr; > > > > > + pp->io_base = range.cpu_addr; > > > > > } > > > > > if (restype == IORESOURCE_MEM) { > > > > > of_pci_range_to_resource(&range, np, &pp->mem); > > @@ -403,7 > > > > > +404,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > > > > > > > > > pp->cfg0_base = pp->cfg.start; > > > > > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; > > > > > - pp->io_base = pp->io.start; > > > > > pp->mem_base = pp->mem.start; > > > > > > > > > > pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, > > > > > > > > This looks correct to me and it seems to also fix a bug in > > > > dw_pcie_prog_viewport_io_outbound if I read this correctly. > > > > > > Yes, now outbound viewport for IO translation will get correct input > > > address. > > > > > > > > > > > > @@ -667,7 +667,7 @@ static int dw_pcie_setup(int nr, struct > > > > > pci_sys_data *sys) > > > > > > > > > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > > > > > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > > > > > - pci_ioremap_io(sys->io_offset, pp->io.start); > > > > > + pci_ioremap_io(sys->io_offset, pp->io_base); > > > > > global_io_offset += SZ_64K; > > > > > pci_add_resource_offset(&sys->resources, &pp->io, > > > > > sys->io_offset); > > > > > > > > I think there is still a related bug in here: we should pass > > > > global_io_offset rather than sys->io_offset to pci_ioremap_io, so we > > > > map the new window into the first available spot in the Linux view > > > > of the I/O space, rather than passing a number that might be zero > > > > for any bus, if the 'ranges' are set up to have an identity mapping > > > > between Linux I/O spaces and PCI I/O spaces. You should be able to > > > > verify this by setting the I/O range for the bus to a random 4KB > > > > multiple in DT and observe that Linux start allocating ports from 0x1000 > > but the raw BAR values would contain the value you have chosen. > > > > > > OK. > > > > > > @ Jingoo, Mohit > > - Now its working properly for SPEAr1310 tested with directly connected EP(xhc cards) as well as EP > Connected through switch(Lecroy PTC in AIC mode). Without this patch my EP devices were not working > when > connected through Switch. Hi Marek, Richard, How about testing the following Pratyush's patch on imx6 platform with the Pericom PI7C9X2G303EL PCIe switch? As Mohit tested, it looks to resolve the i.MX6 problem when the PCIe switch is connected to the i.MX6 platform. Best regards, Jingoo Han [.....] diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c index be6ce30..b83f5e8 100644 --- a/drivers/pci/host/pcie-designware.c +++ b/drivers/pci/host/pcie-designware.c @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp) + global_io_offset); pp->config.io_size = resource_size(&pp->io); pp->config.io_bus_addr = range.pci_addr; + pp->io_base = range.cpu_addr; } if (restype == IORESOURCE_MEM) { of_pci_range_to_resource(&range, np, &pp->mem); @@ -403,7 +404,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) pp->cfg0_base = pp->cfg.start; pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; - pp->io_base = pp->io.start; pp->mem_base = pp->mem.start; pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, @@ -667,7 +667,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data *sys) if (global_io_offset < SZ_1M && pp->config.io_size > 0) { sys->io_offset = global_io_offset - pp->config.io_bus_addr; - pci_ioremap_io(sys->io_offset, pp->io.start); + pci_ioremap_io(global_io_offset, pp->io_base); global_io_offset += SZ_64K; pci_add_resource_offset(&sys->resources, &pp->io, sys->io_offset); ^ permalink raw reply related [flat|nested] 17+ messages in thread
* RE: [Query/Discussion]: IO translation with designware PCIe controller 2013-12-10 7:02 ` Jingoo Han @ 2013-12-13 7:36 ` Hong-Xing.Zhu 0 siblings, 0 replies; 17+ messages in thread From: Hong-Xing.Zhu @ 2013-12-13 7:36 UTC (permalink / raw) To: Jingoo Han, 'Marek Vasut' Cc: 'Mohit KUMAR DCG', 'Pratyush ANAND', 'Arnd Bergmann', 'Kishon Vijay Abraham I', linux-pci@vger.kernel.org, 'Tim Harvey' Hi Jinhoo: > -----Original Message----- > From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-owner@vger.kernel.org] > On Behalf Of Jingoo Han > Sent: Tuesday, December 10, 2013 3:03 PM > To: 'Marek Vasut'; Zhu Richard-R65037 > Cc: 'Mohit KUMAR DCG'; 'Pratyush ANAND'; 'Arnd Bergmann'; 'Kishon Vijay > Abraham I'; linux-pci@vger.kernel.org; 'Tim Harvey'; 'Jingoo Han' > Subject: Re: [Query/Discussion]: IO translation with designware PCIe > controller > > On Tuesday, December 10, 2013 3:31 PM, Mohit KUMAR DCG wrote: > > On Tuesday, December 10, 2013 10:55 AM, Jingoo Han wrote: > > > On Tuesday, December 10, 2013 1:34 PM, Pratyush Anand wrote: > > > > On Tue, Dec 10, 2013 at 12:09:37AM +0800, Arnd Bergmann wrote: > > > > > On Monday 09 December 2013, Pratyush Anand wrote: > > > > > > > I think it does handle this correctly, look at > > > > > > > > > > > > > > static int dw_pcie_setup(int nr, struct pci_sys_data *sys) { > > > > > > > ... > > > > > > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > > > > > > > sys->io_offset = global_io_offset - pp- > >config.io_bus_addr; > > > > > > > pci_ioremap_io(sys->io_offset, pp->io.start); > > > > > > > global_io_offset += SZ_64K; > > > > > > > pci_add_resource_offset(&sys->resources, &pp->io, > > > > > > > sys->io_offset); > > > > > > > } > > > > > > > ... > > > > > > > } > > > > > > > > > > > > > > I believe this does the right thing, but you have to put the > > > > > > > correct translation into the 'ranges' property of the host > > > > > > > bridge node > > > in DT. > > > > > > > > > > > > May be not exactly. pp->io is the realio, and it is passed > > > > > > correctly to pci_add_resource_offset. But, as you had also > > > > > > said that pci_ioremap_io will receive cpu physical address > > > > > > space as input, therefore I think following modification will > > > > > > be needed to work io transaction properly. > > > > > > > > > > I see. I think you are right. > > > > > > > > > > > diff --git a/drivers/pci/host/pcie-designware.c > > > > > > b/drivers/pci/host/pcie-designware.c > > > > > > index be6ce30..cf68632 100644 > > > > > > --- a/drivers/pci/host/pcie-designware.c > > > > > > +++ b/drivers/pci/host/pcie-designware.c > > > > > > @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct > > > > > > pcie_port > > > *pp) > > > > > > + global_io_offset); > > > > > > pp->config.io_size = resource_size(&pp->io); > > > > > > pp->config.io_bus_addr = range.pci_addr; > > > > > > + pp->io_base = range.cpu_addr; > > > > > > } > > > > > > if (restype == IORESOURCE_MEM) { > > > > > > of_pci_range_to_resource(&range, np, &pp->mem); > > > @@ -403,7 > > > > > > +404,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > > > > > > > > > > > pp->cfg0_base = pp->cfg.start; > > > > > > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; > > > > > > - pp->io_base = pp->io.start; > > > > > > pp->mem_base = pp->mem.start; > > > > > > > > > > > > pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, > > > > > > > > > > This looks correct to me and it seems to also fix a bug in > > > > > dw_pcie_prog_viewport_io_outbound if I read this correctly. > > > > > > > > Yes, now outbound viewport for IO translation will get correct > > > > input address. > > > > > > > > > > > > > > > @@ -667,7 +667,7 @@ static int dw_pcie_setup(int nr, struct > > > > > > pci_sys_data *sys) > > > > > > > > > > > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > > > > > > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > > > > > > - pci_ioremap_io(sys->io_offset, pp->io.start); > > > > > > + pci_ioremap_io(sys->io_offset, pp->io_base); > > > > > > global_io_offset += SZ_64K; > > > > > > pci_add_resource_offset(&sys->resources, &pp->io, > > > > > > sys->io_offset); > > > > > > > > > > I think there is still a related bug in here: we should pass > > > > > global_io_offset rather than sys->io_offset to pci_ioremap_io, > > > > > so we map the new window into the first available spot in the > > > > > Linux view of the I/O space, rather than passing a number that > > > > > might be zero for any bus, if the 'ranges' are set up to have an > > > > > identity mapping between Linux I/O spaces and PCI I/O spaces. > > > > > You should be able to verify this by setting the I/O range for > > > > > the bus to a random 4KB multiple in DT and observe that Linux > > > > > start allocating ports from 0x1000 > > > but the raw BAR values would contain the value you have chosen. > > > > > > > > OK. > > > > > > > > @ Jingoo, Mohit > > > > - Now its working properly for SPEAr1310 tested with directly > > connected EP(xhc cards) as well as EP Connected through switch(Lecroy > > PTC in AIC mode). Without this patch my EP devices were not working > > when connected through Switch. > > Hi Marek, Richard, > > How about testing the following Pratyush's patch on imx6 platform with the > Pericom PI7C9X2G303EL PCIe switch? > > As Mohit tested, it looks to resolve the i.MX6 problem when the PCIe switch is > connected to the i.MX6 platform. > > Best regards, > Jingoo Han > [Richard] Sorry to reply late, I went to hospital in the past two days. This patch is great, I tested at my side, it works, thanks a lot. > [.....] > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie- > designware.c > index be6ce30..b83f5e8 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > + global_io_offset); > pp->config.io_size = resource_size(&pp->io); > pp->config.io_bus_addr = range.pci_addr; > + pp->io_base = range.cpu_addr; > } > if (restype == IORESOURCE_MEM) { > of_pci_range_to_resource(&range, np, &pp->mem); @@ -403,7 > +404,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > pp->cfg0_base = pp->cfg.start; > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; > - pp->io_base = pp->io.start; > pp->mem_base = pp->mem.start; > > pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, @@ -667,7 +667,7 > @@ static int dw_pcie_setup(int nr, struct pci_sys_data *sys) > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > - pci_ioremap_io(sys->io_offset, pp->io.start); > + pci_ioremap_io(global_io_offset, pp->io_base); > global_io_offset += SZ_64K; > pci_add_resource_offset(&sys->resources, &pp->io, > sys->io_offset); > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in the > body of a message to majordomo@vger.kernel.org More majordomo info at > http://vger.kernel.org/majordomo-info.html > Best Regards Richard Zhu ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Query/Discussion]: IO translation with designware PCIe controller 2013-12-09 7:12 ` Pratyush Anand 2013-12-09 16:09 ` Arnd Bergmann @ 2013-12-10 13:26 ` Marek Vasut 2013-12-10 22:22 ` Jingoo Han 1 sibling, 1 reply; 17+ messages in thread From: Marek Vasut @ 2013-12-10 13:26 UTC (permalink / raw) To: Pratyush Anand Cc: Arnd Bergmann, Jingoo Han, Mohit KUMAR DCG, Richard Zhu, Kishon Vijay Abraham I, linux-pci@vger.kernel.org, Tim Harvey On Monday, December 09, 2013 at 08:12:42 AM, Pratyush Anand wrote: > Hi Arnd, > > On Fri, Dec 06, 2013 at 10:46:23PM +0800, Arnd Bergmann wrote: > > On Friday 06 December 2013, Pratyush Anand wrote: > [...] > > > > > > For example in SPEAr1340, physically RAM is mapped on above > > > > > addresses. PCIe address translation unit can accept address only > > > > > in the range of core addresses which are assigned to PCIe RC ie > > > > > 0x80000000-0x8FFFFFFF. > > > > > > > > Since this is a physical address, it corresponds to address space 3 > > > > in my list above, and the address you pick here is what you pass to > > > > pci_ioremap_io. > > > > > > This is what I was expecting. But currently designware driver does not > > > pass this address to pci_ioremap_io. > > > OK.. We will fix it and will send patch. > > > > I think it does handle this correctly, look at > > > > static int dw_pcie_setup(int nr, struct pci_sys_data *sys) > > { > > > > ... > > > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > > > > sys->io_offset = global_io_offset - > > pp->config.io_bus_addr; pci_ioremap_io(sys->io_offset, > > pp->io.start); > > global_io_offset += SZ_64K; > > pci_add_resource_offset(&sys->resources, &pp->io, > > > > sys->io_offset); > > > > } > > > > ... > > > > } > > > > I believe this does the right thing, but you have to put the correct > > translation into the 'ranges' property of the host bridge node in DT. > > May be not exactly. pp->io is the realio, and it is passed correctly > to pci_add_resource_offset. But, as you had also > said that pci_ioremap_io will receive cpu physical address space as > input, therefore I think following modification will be needed to work > io transaction properly. > > > diff --git a/drivers/pci/host/pcie-designware.c > b/drivers/pci/host/pcie-designware.c index be6ce30..cf68632 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > + global_io_offset); > pp->config.io_size = resource_size(&pp->io); > pp->config.io_bus_addr = range.pci_addr; > + pp->io_base = range.cpu_addr; > } > if (restype == IORESOURCE_MEM) { > of_pci_range_to_resource(&range, np, &pp->mem); > @@ -403,7 +404,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > pp->cfg0_base = pp->cfg.start; > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; > - pp->io_base = pp->io.start; > pp->mem_base = pp->mem.start; > > pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, > @@ -667,7 +667,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data > *sys) > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > - pci_ioremap_io(sys->io_offset, pp->io.start); > + pci_ioremap_io(sys->io_offset, pp->io_base); > global_io_offset += SZ_64K; > pci_add_resource_offset(&sys->resources, &pp->io, > sys->io_offset); Tim, can you test if this patch fixes your SKY2 IOspace problem please ? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Query/Discussion]: IO translation with designware PCIe controller 2013-12-10 13:26 ` Marek Vasut @ 2013-12-10 22:22 ` Jingoo Han 2013-12-10 23:23 ` Tim Harvey 0 siblings, 1 reply; 17+ messages in thread From: Jingoo Han @ 2013-12-10 22:22 UTC (permalink / raw) To: 'Marek Vasut', 'Tim Harvey' Cc: 'Pratyush Anand', 'Arnd Bergmann', 'Mohit KUMAR DCG', 'Richard Zhu', 'Kishon Vijay Abraham I', linux-pci, 'Jingoo Han' On Tuesday, December 10, 2013 10:27 PM, Marek Vasut wrote: > On Monday, December 09, 2013 at 08:12:42 AM, Pratyush Anand wrote: [.....] > > > > diff --git a/drivers/pci/host/pcie-designware.c > > b/drivers/pci/host/pcie-designware.c index be6ce30..cf68632 100644 > > --- a/drivers/pci/host/pcie-designware.c > > +++ b/drivers/pci/host/pcie-designware.c > > @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > + global_io_offset); > > pp->config.io_size = resource_size(&pp->io); > > pp->config.io_bus_addr = range.pci_addr; > > + pp->io_base = range.cpu_addr; > > } > > if (restype == IORESOURCE_MEM) { > > of_pci_range_to_resource(&range, np, &pp->mem); > > @@ -403,7 +404,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > > > pp->cfg0_base = pp->cfg.start; > > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; > > - pp->io_base = pp->io.start; > > pp->mem_base = pp->mem.start; > > > > pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, > > @@ -667,7 +667,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data > > *sys) > > > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > > - pci_ioremap_io(sys->io_offset, pp->io.start); > > + pci_ioremap_io(sys->io_offset, pp->io_base); > > global_io_offset += SZ_64K; > > pci_add_resource_offset(&sys->resources, &pp->io, > > sys->io_offset); > > Tim, can you test if this patch fixes your SKY2 IOspace problem please ? Above mentioned patch is NOT the latest patch. Pratyush Anand already submitted the next patch as below. Tim, Would you test the following patch on i.MX6 platform? diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c index be6ce30..b83f5e8 100644 --- a/drivers/pci/host/pcie-designware.c +++ b/drivers/pci/host/pcie-designware.c @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp) + global_io_offset); pp->config.io_size = resource_size(&pp->io); pp->config.io_bus_addr = range.pci_addr; + pp->io_base = range.cpu_addr; } if (restype == IORESOURCE_MEM) { of_pci_range_to_resource(&range, np, &pp->mem); @@ -403,7 +404,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) pp->cfg0_base = pp->cfg.start; pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; - pp->io_base = pp->io.start; pp->mem_base = pp->mem.start; pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, @@ -667,7 +667,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data *sys) if (global_io_offset < SZ_1M && pp->config.io_size > 0) { sys->io_offset = global_io_offset - pp->config.io_bus_addr; - pci_ioremap_io(sys->io_offset, pp->io.start); + pci_ioremap_io(global_io_offset, pp->io_base); global_io_offset += SZ_64K; pci_add_resource_offset(&sys->resources, &pp->io, sys->io_offset); Best regards, Jingoo Han ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Query/Discussion]: IO translation with designware PCIe controller 2013-12-10 22:22 ` Jingoo Han @ 2013-12-10 23:23 ` Tim Harvey 2013-12-10 23:25 ` Marek Vasut 2013-12-10 23:58 ` Jingoo Han 0 siblings, 2 replies; 17+ messages in thread From: Tim Harvey @ 2013-12-10 23:23 UTC (permalink / raw) To: Jingoo Han Cc: Marek Vasut, Pratyush Anand, Arnd Bergmann, Mohit KUMAR DCG, Richard Zhu, Kishon Vijay Abraham I, linux-pci@vger.kernel.org On Tue, Dec 10, 2013 at 2:22 PM, Jingoo Han <jg1.han@samsung.com> wrote: > On Tuesday, December 10, 2013 10:27 PM, Marek Vasut wrote: >> On Monday, December 09, 2013 at 08:12:42 AM, Pratyush Anand wrote: > > [.....] > >> > >> > diff --git a/drivers/pci/host/pcie-designware.c >> > b/drivers/pci/host/pcie-designware.c index be6ce30..cf68632 100644 >> > --- a/drivers/pci/host/pcie-designware.c >> > +++ b/drivers/pci/host/pcie-designware.c >> > @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp) >> > + global_io_offset); >> > pp->config.io_size = resource_size(&pp->io); >> > pp->config.io_bus_addr = range.pci_addr; >> > + pp->io_base = range.cpu_addr; >> > } >> > if (restype == IORESOURCE_MEM) { >> > of_pci_range_to_resource(&range, np, &pp->mem); >> > @@ -403,7 +404,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) >> > >> > pp->cfg0_base = pp->cfg.start; >> > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; >> > - pp->io_base = pp->io.start; >> > pp->mem_base = pp->mem.start; >> > >> > pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, >> > @@ -667,7 +667,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data >> > *sys) >> > >> > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { >> > sys->io_offset = global_io_offset - pp->config.io_bus_addr; >> > - pci_ioremap_io(sys->io_offset, pp->io.start); >> > + pci_ioremap_io(sys->io_offset, pp->io_base); >> > global_io_offset += SZ_64K; >> > pci_add_resource_offset(&sys->resources, &pp->io, >> > sys->io_offset); >> >> Tim, can you test if this patch fixes your SKY2 IOspace problem please ? > > Above mentioned patch is NOT the latest patch. > Pratyush Anand already submitted the next patch as below. > > Tim, > Would you test the following patch on i.MX6 platform? > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > index be6ce30..b83f5e8 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > + global_io_offset); > pp->config.io_size = resource_size(&pp->io); > pp->config.io_bus_addr = range.pci_addr; > + pp->io_base = range.cpu_addr; > } > if (restype == IORESOURCE_MEM) { > of_pci_range_to_resource(&range, np, &pp->mem); > @@ -403,7 +404,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > pp->cfg0_base = pp->cfg.start; > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; > - pp->io_base = pp->io.start; > pp->mem_base = pp->mem.start; > > pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, > @@ -667,7 +667,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data *sys) > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > - pci_ioremap_io(sys->io_offset, pp->io.start); > + pci_ioremap_io(global_io_offset, pp->io_base); > global_io_offset += SZ_64K; > pci_add_resource_offset(&sys->resources, &pp->io, > sys->io_offset); > > > Best regards, > Jingoo Han > Yes, this patch resolves the issue and now I can use devices with io resources behind a bridge attached to the IMX6. Thanks everyone for your efforts! Tim ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Query/Discussion]: IO translation with designware PCIe controller 2013-12-10 23:23 ` Tim Harvey @ 2013-12-10 23:25 ` Marek Vasut 2013-12-10 23:58 ` Jingoo Han 1 sibling, 0 replies; 17+ messages in thread From: Marek Vasut @ 2013-12-10 23:25 UTC (permalink / raw) To: Tim Harvey Cc: Jingoo Han, Pratyush Anand, Arnd Bergmann, Mohit KUMAR DCG, Richard Zhu, Kishon Vijay Abraham I, linux-pci@vger.kernel.org On Wednesday, December 11, 2013 at 12:23:47 AM, Tim Harvey wrote: > On Tue, Dec 10, 2013 at 2:22 PM, Jingoo Han <jg1.han@samsung.com> wrote: > > On Tuesday, December 10, 2013 10:27 PM, Marek Vasut wrote: > >> On Monday, December 09, 2013 at 08:12:42 AM, Pratyush Anand wrote: > > [.....] > > > >> > diff --git a/drivers/pci/host/pcie-designware.c > >> > b/drivers/pci/host/pcie-designware.c index be6ce30..cf68632 100644 > >> > --- a/drivers/pci/host/pcie-designware.c > >> > +++ b/drivers/pci/host/pcie-designware.c > >> > @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > >> > > >> > + global_io_offset); > >> > > >> > pp->config.io_size = resource_size(&pp->io); > >> > pp->config.io_bus_addr = range.pci_addr; > >> > > >> > + pp->io_base = range.cpu_addr; > >> > > >> > } > >> > if (restype == IORESOURCE_MEM) { > >> > > >> > of_pci_range_to_resource(&range, np, &pp->mem); > >> > > >> > @@ -403,7 +404,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > >> > > >> > pp->cfg0_base = pp->cfg.start; > >> > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; > >> > > >> > - pp->io_base = pp->io.start; > >> > > >> > pp->mem_base = pp->mem.start; > >> > > >> > pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, > >> > > >> > @@ -667,7 +667,7 @@ static int dw_pcie_setup(int nr, struct > >> > pci_sys_data *sys) > >> > > >> > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > >> > > >> > sys->io_offset = global_io_offset - > >> > pp->config.io_bus_addr; > >> > > >> > - pci_ioremap_io(sys->io_offset, pp->io.start); > >> > + pci_ioremap_io(sys->io_offset, pp->io_base); > >> > > >> > global_io_offset += SZ_64K; > >> > pci_add_resource_offset(&sys->resources, &pp->io, > >> > > >> > sys->io_offset); > >> > >> Tim, can you test if this patch fixes your SKY2 IOspace problem please ? > > > > Above mentioned patch is NOT the latest patch. > > Pratyush Anand already submitted the next patch as below. > > > > Tim, > > Would you test the following patch on i.MX6 platform? > > > > diff --git a/drivers/pci/host/pcie-designware.c > > b/drivers/pci/host/pcie-designware.c index be6ce30..b83f5e8 100644 > > --- a/drivers/pci/host/pcie-designware.c > > +++ b/drivers/pci/host/pcie-designware.c > > @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > > > + global_io_offset); > > > > pp->config.io_size = resource_size(&pp->io); > > pp->config.io_bus_addr = range.pci_addr; > > > > + pp->io_base = range.cpu_addr; > > > > } > > if (restype == IORESOURCE_MEM) { > > > > of_pci_range_to_resource(&range, np, &pp->mem); > > > > @@ -403,7 +404,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > > > pp->cfg0_base = pp->cfg.start; > > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; > > > > - pp->io_base = pp->io.start; > > > > pp->mem_base = pp->mem.start; > > > > pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, > > > > @@ -667,7 +667,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data > > *sys) > > > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > > > > sys->io_offset = global_io_offset - > > pp->config.io_bus_addr; > > > > - pci_ioremap_io(sys->io_offset, pp->io.start); > > + pci_ioremap_io(global_io_offset, pp->io_base); > > > > global_io_offset += SZ_64K; > > pci_add_resource_offset(&sys->resources, &pp->io, > > > > sys->io_offset); > > > > Best regards, > > Jingoo Han > > Yes, this patch resolves the issue and now I can use devices with io > resources behind a bridge attached to the IMX6. > > Thanks everyone for your efforts! This is awesome, thanks guys! ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Query/Discussion]: IO translation with designware PCIe controller 2013-12-10 23:23 ` Tim Harvey 2013-12-10 23:25 ` Marek Vasut @ 2013-12-10 23:58 ` Jingoo Han 1 sibling, 0 replies; 17+ messages in thread From: Jingoo Han @ 2013-12-10 23:58 UTC (permalink / raw) To: 'Tim Harvey', 'Pratyush Anand' Cc: 'Marek Vasut', 'Arnd Bergmann', 'Mohit KUMAR DCG', 'Richard Zhu', 'Kishon Vijay Abraham I', linux-pci, 'Jingoo Han' On Wednesday, December 11, 2013 8:24 AM, Tim Harvey wrote: > On Tue, Dec 10, 2013 at 2:22 PM, Jingoo Han <jg1.han@samsung.com> wrote: > > On Tuesday, December 10, 2013 10:27 PM, Marek Vasut wrote: > >> On Monday, December 09, 2013 at 08:12:42 AM, Pratyush Anand wrote: > > > > [.....] > > > >> > > >> > diff --git a/drivers/pci/host/pcie-designware.c > >> > b/drivers/pci/host/pcie-designware.c index be6ce30..cf68632 100644 > >> > --- a/drivers/pci/host/pcie-designware.c > >> > +++ b/drivers/pci/host/pcie-designware.c > >> > @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > >> > + global_io_offset); > >> > pp->config.io_size = resource_size(&pp->io); > >> > pp->config.io_bus_addr = range.pci_addr; > >> > + pp->io_base = range.cpu_addr; > >> > } > >> > if (restype == IORESOURCE_MEM) { > >> > of_pci_range_to_resource(&range, np, &pp->mem); > >> > @@ -403,7 +404,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > >> > > >> > pp->cfg0_base = pp->cfg.start; > >> > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; > >> > - pp->io_base = pp->io.start; > >> > pp->mem_base = pp->mem.start; > >> > > >> > pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, > >> > @@ -667,7 +667,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data > >> > *sys) > >> > > >> > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > >> > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > >> > - pci_ioremap_io(sys->io_offset, pp->io.start); > >> > + pci_ioremap_io(sys->io_offset, pp->io_base); > >> > global_io_offset += SZ_64K; > >> > pci_add_resource_offset(&sys->resources, &pp->io, > >> > sys->io_offset); > >> > >> Tim, can you test if this patch fixes your SKY2 IOspace problem please ? > > > > Above mentioned patch is NOT the latest patch. > > Pratyush Anand already submitted the next patch as below. > > > > Tim, > > Would you test the following patch on i.MX6 platform? > > > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > > index be6ce30..b83f5e8 100644 > > --- a/drivers/pci/host/pcie-designware.c > > +++ b/drivers/pci/host/pcie-designware.c > > @@ -378,6 +378,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > + global_io_offset); > > pp->config.io_size = resource_size(&pp->io); > > pp->config.io_bus_addr = range.pci_addr; > > + pp->io_base = range.cpu_addr; > > } > > if (restype == IORESOURCE_MEM) { > > of_pci_range_to_resource(&range, np, &pp->mem); > > @@ -403,7 +404,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > > > pp->cfg0_base = pp->cfg.start; > > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; > > - pp->io_base = pp->io.start; > > pp->mem_base = pp->mem.start; > > > > pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base, > > @@ -667,7 +667,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data *sys) > > > > if (global_io_offset < SZ_1M && pp->config.io_size > 0) { > > sys->io_offset = global_io_offset - pp->config.io_bus_addr; > > - pci_ioremap_io(sys->io_offset, pp->io.start); > > + pci_ioremap_io(global_io_offset, pp->io_base); > > global_io_offset += SZ_64K; > > pci_add_resource_offset(&sys->resources, &pp->io, > > sys->io_offset); > > > > > > Best regards, > > Jingoo Han > > > > Yes, this patch resolves the issue and now I can use devices with io > resources behind a bridge attached to the IMX6. > > Thanks everyone for your efforts! Wow, happy ending! :-) I was deeply moved. Pratyush Anand, Would you re-send this patch with commit message? You did a good job! I really appreciate your patch. Best regards, Jingoo Han ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2013-12-13 7:36 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-12-05 5:04 [Query/Discussion]: IO translation with designware PCIe controller Pratyush Anand 2013-12-05 21:33 ` Arnd Bergmann 2013-12-06 9:12 ` Pratyush Anand 2013-12-06 14:46 ` Arnd Bergmann 2013-12-09 7:12 ` Pratyush Anand 2013-12-09 16:09 ` Arnd Bergmann 2013-12-10 4:34 ` Pratyush Anand 2013-12-10 5:25 ` Jingoo Han 2013-12-10 6:31 ` Mohit KUMAR DCG 2013-12-10 6:57 ` Pratyush Anand 2013-12-10 7:02 ` Jingoo Han 2013-12-13 7:36 ` Hong-Xing.Zhu 2013-12-10 13:26 ` Marek Vasut 2013-12-10 22:22 ` Jingoo Han 2013-12-10 23:23 ` Tim Harvey 2013-12-10 23:25 ` Marek Vasut 2013-12-10 23:58 ` Jingoo Han
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).