Message ID | 1403719007-9352-3-git-send-email-kishon@ti.com (mailing list archive) |
---|---|
State | New, archived |
Delegated to: | Bjorn Helgaas |
Headers | show |
Hi Kishon, Few things, if you can help me to understand: On Wed, Jun 25, 2014 at 11:26 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: > In DRA7, the cpu sees 32bit address, but the pcie controller can see only 28bit > address. So whenever the cpu issues a read/write request, the 4 most > significant bits are used by L3 to determine the target controller. > For example, the cpu reserves 0x2000_0000 - 0x2FFF_FFFF for PCIe controller but > the PCIe controller will see only (0x000_0000 - 0xFFF_FFF). So for programming > the outbound translation window the *base* should be programmed as 0x000_0000. > Whenever we try to write to say 0x2000_0000, it will be translated to whatever > we have programmed in the translation window with base as 0x000_0000. > > This is needed when the dt node is modelled something like below > axi { > compatible = "simple-bus"; > #size-cells = <1>; > #address-cells = <1>; > ranges = <0x0 0x20000000 0x10000000 // 28-bit bus > 0x51000000 0x51000000 0x3000>; > pcie@51000000 { > reg = <0x1000 0x2000>, <0x51002000 0x14c>, <0x51000000 0x2000>; > reg-names = "config", "ti_conf", "rc_dbics"; So for DRA7, config base which will be coming from reg property should be 0x1000 and size 0x2000, no? > #address-cells = <3>; > #size-cells = <2>; > ranges = <0x81000000 0 0 0x03000 0 0x00010000 range type 0x81000000 tells that it is IO > 0x82000000 0 0x20013000 0x13000 0 0xffed000>; range type 0x81000000 tells that it is mem > }; > }; > > Here the CPU address for configuration space is 0x20013000 and the controller > address for configuration space is 0x13000. The controller address should be If above understanding is correct then: Aren't these addresses(0x20013000 and 0x13000) from mem space instead of configuration space. If yes, then how can you get two addresses (CPU and Controller address) from reg property for configuration space? ~Pratyush -- 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
Hi Pratyush, On Thursday 26 June 2014 11:07 AM, Pratyush Anand wrote: > Hi Kishon, > > Few things, if you can help me to understand: > > On Wed, Jun 25, 2014 at 11:26 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: >> In DRA7, the cpu sees 32bit address, but the pcie controller can see only 28bit >> address. So whenever the cpu issues a read/write request, the 4 most >> significant bits are used by L3 to determine the target controller. >> For example, the cpu reserves 0x2000_0000 - 0x2FFF_FFFF for PCIe controller but >> the PCIe controller will see only (0x000_0000 - 0xFFF_FFF). So for programming >> the outbound translation window the *base* should be programmed as 0x000_0000. >> Whenever we try to write to say 0x2000_0000, it will be translated to whatever >> we have programmed in the translation window with base as 0x000_0000. >> >> This is needed when the dt node is modelled something like below >> axi { >> compatible = "simple-bus"; >> #size-cells = <1>; >> #address-cells = <1>; >> ranges = <0x0 0x20000000 0x10000000 // 28-bit bus >> 0x51000000 0x51000000 0x3000>; >> pcie@51000000 { >> reg = <0x1000 0x2000>, <0x51002000 0x14c>, <0x51000000 0x2000>; >> reg-names = "config", "ti_conf", "rc_dbics"; > > So for DRA7, config base which will be coming from reg property should > be 0x1000 and size 0x2000, no? right. The first element in 'reg' and 'reg-names' specify exactly that. > >> #address-cells = <3>; >> #size-cells = <2>; >> ranges = <0x81000000 0 0 0x03000 0 0x00010000 > > range type 0x81000000 tells that it is IO > >> 0x82000000 0 0x20013000 0x13000 0 0xffed000>; > > range type 0x81000000 tells that it is mem > > >> }; >> }; >> >> Here the CPU address for configuration space is 0x20013000 and the controller >> address for configuration space is 0x13000. The controller address should > be > > If above understanding is correct then: > > Aren't these addresses(0x20013000 and 0x13000) from mem space > instead of configuration space. Sorry. I didn't get you. Configuration space is different from mem space and IO space. We specify only the configuration space in "reg", the IO space and memory space should be specified in ranges. In my case configuration space range: 0x20001000 - 0x20002fff IO space range: 0x20003000 - 0x20012fff Mem space range: 0x20013000 - 0x2fffffff Here only the configuration space is obtained from 'reg' and 'IO' and 'MEM' space is obtained from ranges. > > If yes, then how can you get two addresses (CPU and Controller address) > from reg property for configuration space? I used platform_get_resource_byname() to get CPU address and of_get_address() to get the untranslated controller address. Thanks Kishon -- 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
Hi Kishon, On Thu, Jun 26, 2014 at 02:10:02PM +0800, Kishon Vijay Abraham I wrote: > Hi Pratyush, > > On Thursday 26 June 2014 11:07 AM, Pratyush Anand wrote: > > Hi Kishon, > > > > Few things, if you can help me to understand: > > > > On Wed, Jun 25, 2014 at 11:26 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: > >> In DRA7, the cpu sees 32bit address, but the pcie controller can see only 28bit > >> address. So whenever the cpu issues a read/write request, the 4 most > >> significant bits are used by L3 to determine the target controller. > >> For example, the cpu reserves 0x2000_0000 - 0x2FFF_FFFF for PCIe controller but > >> the PCIe controller will see only (0x000_0000 - 0xFFF_FFF). So for programming > >> the outbound translation window the *base* should be programmed as 0x000_0000. > >> Whenever we try to write to say 0x2000_0000, it will be translated to whatever > >> we have programmed in the translation window with base as 0x000_0000. > >> > >> This is needed when the dt node is modelled something like below > >> axi { > >> compatible = "simple-bus"; > >> #size-cells = <1>; > >> #address-cells = <1>; > >> ranges = <0x0 0x20000000 0x10000000 // 28-bit bus > >> 0x51000000 0x51000000 0x3000>; > >> pcie@51000000 { > >> reg = <0x1000 0x2000>, <0x51002000 0x14c>, <0x51000000 0x2000>; > >> reg-names = "config", "ti_conf", "rc_dbics"; > > > > So for DRA7, config base which will be coming from reg property should > > be 0x1000 and size 0x2000, no? > > right. The first element in 'reg' and 'reg-names' specify exactly that. > > > >> #address-cells = <3>; > >> #size-cells = <2>; > >> ranges = <0x81000000 0 0 0x03000 0 0x00010000 > > > > range type 0x81000000 tells that it is IO > > > >> 0x82000000 0 0x20013000 0x13000 0 0xffed000>; > > > > range type 0x81000000 tells that it is mem > > > > > >> }; > >> }; > >> > >> Here the CPU address for configuration space is 0x20013000 and the controller > >> address for configuration space is 0x13000. The controller address should > > be > > > > If above understanding is correct then: > > > > Aren't these addresses(0x20013000 and 0x13000) from mem space > > instead of configuration space. > > Sorry. I didn't get you. Configuration space is different from mem space and IO > space. We specify only the configuration space in "reg", the IO space and > memory space should be specified in ranges. > > In my case > configuration space range: 0x20001000 - 0x20002fff > IO space range: 0x20003000 - 0x20012fff > Mem space range: 0x20013000 - 0x2fffffff > > Here only the configuration space is obtained from 'reg' and 'IO' and 'MEM' > space is obtained from ranges. > > > > If yes, then how can you get two addresses (CPU and Controller address) > > from reg property for configuration space? > > I used platform_get_resource_byname() to get CPU address and of_get_address() > to get the untranslated controller address. This is what I am not able to understand that how does platform_get_resource_byname gives correct CPU address from reg = <0x1000 0x2000>? cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config"); how cfg_res->start is now 0x20001000? Shouldn't cfg_res->start be 0x1000? What am I missing? Thanks for explaining. ~Pratyush > Thanks > Kishon -- 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
Hi Pratyush, On Thursday 26 June 2014 12:03 PM, Pratyush Anand wrote: > Hi Kishon, > > On Thu, Jun 26, 2014 at 02:10:02PM +0800, Kishon Vijay Abraham I wrote: >> Hi Pratyush, >> >> On Thursday 26 June 2014 11:07 AM, Pratyush Anand wrote: >>> Hi Kishon, >>> >>> Few things, if you can help me to understand: >>> >>> On Wed, Jun 25, 2014 at 11:26 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: >>>> In DRA7, the cpu sees 32bit address, but the pcie controller can see only 28bit >>>> address. So whenever the cpu issues a read/write request, the 4 most >>>> significant bits are used by L3 to determine the target controller. >>>> For example, the cpu reserves 0x2000_0000 - 0x2FFF_FFFF for PCIe controller but >>>> the PCIe controller will see only (0x000_0000 - 0xFFF_FFF). So for programming >>>> the outbound translation window the *base* should be programmed as 0x000_0000. >>>> Whenever we try to write to say 0x2000_0000, it will be translated to whatever >>>> we have programmed in the translation window with base as 0x000_0000. >>>> >>>> This is needed when the dt node is modelled something like below >>>> axi { >>>> compatible = "simple-bus"; >>>> #size-cells = <1>; >>>> #address-cells = <1>; >>>> ranges = <0x0 0x20000000 0x10000000 // 28-bit bus >>>> 0x51000000 0x51000000 0x3000>; >>>> pcie@51000000 { >>>> reg = <0x1000 0x2000>, <0x51002000 0x14c>, <0x51000000 0x2000>; >>>> reg-names = "config", "ti_conf", "rc_dbics"; >>> >>> So for DRA7, config base which will be coming from reg property should >>> be 0x1000 and size 0x2000, no? >> >> right. The first element in 'reg' and 'reg-names' specify exactly that. >>> >>>> #address-cells = <3>; >>>> #size-cells = <2>; >>>> ranges = <0x81000000 0 0 0x03000 0 0x00010000 >>> >>> range type 0x81000000 tells that it is IO >>> >>>> 0x82000000 0 0x20013000 0x13000 0 0xffed000>; >>> >>> range type 0x81000000 tells that it is mem >>> >>> >>>> }; >>>> }; >>>> >>>> Here the CPU address for configuration space is 0x20013000 and the controller >>>> address for configuration space is 0x13000. The controller address should >>> be >>> >>> If above understanding is correct then: >>> >>> Aren't these addresses(0x20013000 and 0x13000) from mem space >>> instead of configuration space. >> >> Sorry. I didn't get you. Configuration space is different from mem space and IO >> space. We specify only the configuration space in "reg", the IO space and >> memory space should be specified in ranges. >> >> In my case >> configuration space range: 0x20001000 - 0x20002fff >> IO space range: 0x20003000 - 0x20012fff >> Mem space range: 0x20013000 - 0x2fffffff >> >> Here only the configuration space is obtained from 'reg' and 'IO' and 'MEM' >> space is obtained from ranges. >>> >>> If yes, then how can you get two addresses (CPU and Controller address) >>> from reg property for configuration space? >> >> I used platform_get_resource_byname() to get CPU address and of_get_address() >> to get the untranslated controller address. > > This is what I am not able to understand that how does > platform_get_resource_byname gives correct CPU address from reg = > <0x1000 0x2000>? > > cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config"); > > how cfg_res->start is now 0x20001000? Shouldn't cfg_res->start be > 0x1000? > > What am I missing? IIUC, converting to the cpu address happens long back when the platform device is created from dt node. See if the following makes sense to you. starting from drivers/of/platform.c of_device_alloc() -> of_address_to_resource() -> __of_address_to_resource() -> of_translate_address() -> __of_translate_address() -> of_translate_one() -> of_bus_default_translate() So the 'resource' in 'struct platform_device' already holds the translated address. platform_get_resource_byname() just returns the resource from this platform device. Cheers Kishon -- 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
Hi Mohit, Jingoo, On Wednesday 25 June 2014 11:26 PM, Kishon Vijay Abraham I wrote: > In DRA7, the cpu sees 32bit address, but the pcie controller can see only 28bit > address. So whenever the cpu issues a read/write request, the 4 most > significant bits are used by L3 to determine the target controller. > For example, the cpu reserves 0x2000_0000 - 0x2FFF_FFFF for PCIe controller but > the PCIe controller will see only (0x000_0000 - 0xFFF_FFF). So for programming > the outbound translation window the *base* should be programmed as 0x000_0000. > Whenever we try to write to say 0x2000_0000, it will be translated to whatever > we have programmed in the translation window with base as 0x000_0000. > > This is needed when the dt node is modelled something like below > axi { > compatible = "simple-bus"; > #size-cells = <1>; > #address-cells = <1>; > ranges = <0x0 0x20000000 0x10000000 // 28-bit bus > 0x51000000 0x51000000 0x3000>; > pcie@51000000 { > reg = <0x1000 0x2000>, <0x51002000 0x14c>, <0x51000000 0x2000>; > reg-names = "config", "ti_conf", "rc_dbics"; > #address-cells = <3>; > #size-cells = <2>; > ranges = <0x81000000 0 0 0x03000 0 0x00010000 > 0x82000000 0 0x20013000 0x13000 0 0xffed000>; > }; > }; > > Here the CPU address for configuration space is 0x20013000 and the controller > address for configuration space is 0x13000. The controller address should be > used while programming the ATU (in order for translation to happen properly in > DRA7xx). Are you okay with this patch? Can you give your Reviewed-by/Acked-by? Cheers Kishon > > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Mohit Kumar <mohit.kumar@st.com> > Cc: Jingoo Han <jg1.han@samsung.com> > Cc: Marek Vasut <marex@denx.de> > Cc: Arnd Bergmann <arnd@arndb.de> > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > drivers/pci/host/pcie-designware.c | 49 ++++++++++++++++++++++++++++-------- > drivers/pci/host/pcie-designware.h | 4 +++ > 2 files changed, 42 insertions(+), 11 deletions(-) > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > index 0b7b455..11aba6d 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -401,8 +401,15 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > struct of_pci_range range; > struct of_pci_range_parser parser; > struct resource *cfg_res; > - u32 val; > - int i; > + u32 val, na, ns; > + const __be32 *addrp; > + int i, index; > + > + /* Find the address cell size and the number of cells in order to get > + * the untranslated address. > + */ > + of_property_read_u32(np, "#address-cells", &na); > + ns = of_n_size_cells(np); > > cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config"); > if (cfg_res) { > @@ -410,6 +417,12 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > pp->config.cfg1_size = resource_size(cfg_res)/2; > pp->cfg0_base = cfg_res->start; > pp->cfg1_base = cfg_res->start + pp->config.cfg0_size; > + > + /* Find the untranslated configuration space address */ > + index = of_property_match_string(np, "reg-names", "config"); > + addrp = of_get_address(np, index, false, false); > + pp->cfg0_mod_addr = of_read_number(addrp, ns); > + pp->cfg1_mod_addr = pp->cfg0_mod_addr + pp->config.cfg0_size; > } else { > dev_err(pp->dev, "missing *config* reg space\n"); > } > @@ -435,12 +448,20 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > pp->config.io_size = resource_size(&pp->io); > pp->config.io_bus_addr = range.pci_addr; > pp->io_base = range.cpu_addr; > + > + /* Find the untranslated IO space address */ > + pp->io_mod_addr = of_read_number(parser.range - > + parser.np + na, ns); > } > if (restype == IORESOURCE_MEM) { > of_pci_range_to_resource(&range, np, &pp->mem); > pp->mem.name = "MEM"; > pp->config.mem_size = resource_size(&pp->mem); > pp->config.mem_bus_addr = range.pci_addr; > + > + /* Find the untranslated MEM space address */ > + pp->mem_mod_addr = of_read_number(parser.range - > + parser.np + na, ns); > } > if (restype == 0) { > of_pci_range_to_resource(&range, np, &pp->cfg); > @@ -448,6 +469,12 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > pp->config.cfg1_size = resource_size(&pp->cfg)/2; > pp->cfg0_base = pp->cfg.start; > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; > + > + /* Find the untranslated configuration space address */ > + pp->cfg0_mod_addr = of_read_number(parser.range - > + parser.np + na, ns); > + pp->cfg1_mod_addr = pp->cfg0_mod_addr + > + pp->config.cfg0_size; > } > } > > @@ -522,9 +549,9 @@ static void dw_pcie_prog_viewport_cfg0(struct pcie_port *pp, u32 busdev) > /* Program viewport 0 : OUTBOUND : CFG0 */ > dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX0, > PCIE_ATU_VIEWPORT); > - dw_pcie_writel_rc(pp, pp->cfg0_base, PCIE_ATU_LOWER_BASE); > - dw_pcie_writel_rc(pp, (pp->cfg0_base >> 32), PCIE_ATU_UPPER_BASE); > - dw_pcie_writel_rc(pp, pp->cfg0_base + pp->config.cfg0_size - 1, > + dw_pcie_writel_rc(pp, pp->cfg0_mod_addr, PCIE_ATU_LOWER_BASE); > + dw_pcie_writel_rc(pp, (pp->cfg0_mod_addr >> 32), PCIE_ATU_UPPER_BASE); > + dw_pcie_writel_rc(pp, pp->cfg0_mod_addr + pp->config.cfg0_size - 1, > PCIE_ATU_LIMIT); > dw_pcie_writel_rc(pp, busdev, PCIE_ATU_LOWER_TARGET); > dw_pcie_writel_rc(pp, 0, PCIE_ATU_UPPER_TARGET); > @@ -538,9 +565,9 @@ static void dw_pcie_prog_viewport_cfg1(struct pcie_port *pp, u32 busdev) > dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX1, > PCIE_ATU_VIEWPORT); > dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_CFG1, PCIE_ATU_CR1); > - dw_pcie_writel_rc(pp, pp->cfg1_base, PCIE_ATU_LOWER_BASE); > - dw_pcie_writel_rc(pp, (pp->cfg1_base >> 32), PCIE_ATU_UPPER_BASE); > - dw_pcie_writel_rc(pp, pp->cfg1_base + pp->config.cfg1_size - 1, > + dw_pcie_writel_rc(pp, pp->cfg1_mod_addr, PCIE_ATU_LOWER_BASE); > + dw_pcie_writel_rc(pp, (pp->cfg1_mod_addr >> 32), PCIE_ATU_UPPER_BASE); > + dw_pcie_writel_rc(pp, pp->cfg1_mod_addr + pp->config.cfg1_size - 1, > PCIE_ATU_LIMIT); > dw_pcie_writel_rc(pp, busdev, PCIE_ATU_LOWER_TARGET); > dw_pcie_writel_rc(pp, 0, PCIE_ATU_UPPER_TARGET); > @@ -553,9 +580,9 @@ static void dw_pcie_prog_viewport_mem_outbound(struct pcie_port *pp) > dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX0, > PCIE_ATU_VIEWPORT); > dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_MEM, PCIE_ATU_CR1); > - dw_pcie_writel_rc(pp, pp->mem_base, PCIE_ATU_LOWER_BASE); > - dw_pcie_writel_rc(pp, (pp->mem_base >> 32), PCIE_ATU_UPPER_BASE); > - dw_pcie_writel_rc(pp, pp->mem_base + pp->config.mem_size - 1, > + dw_pcie_writel_rc(pp, pp->mem_mod_addr, PCIE_ATU_LOWER_BASE); > + dw_pcie_writel_rc(pp, (pp->mem_mod_addr >> 32), PCIE_ATU_UPPER_BASE); > + dw_pcie_writel_rc(pp, pp->mem_mod_addr + pp->config.mem_size - 1, > PCIE_ATU_LIMIT); > dw_pcie_writel_rc(pp, pp->config.mem_bus_addr, PCIE_ATU_LOWER_TARGET); > dw_pcie_writel_rc(pp, upper_32_bits(pp->config.mem_bus_addr), > diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h > index 77f592f..13828f8 100644 > --- a/drivers/pci/host/pcie-designware.h > +++ b/drivers/pci/host/pcie-designware.h > @@ -36,11 +36,15 @@ struct pcie_port { > u8 root_bus_nr; > void __iomem *dbi_base; > u64 cfg0_base; > + u64 cfg0_mod_addr; > void __iomem *va_cfg0_base; > u64 cfg1_base; > + u64 cfg1_mod_addr; > void __iomem *va_cfg1_base; > u64 io_base; > + u64 io_mod_addr; > u64 mem_base; > + u64 mem_mod_addr; > struct resource cfg; > struct resource io; > struct resource mem; > -- 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
On Tuesday, July 08, 2014 9:31 PM, Kishon Vijay Abraham I wrote: > > Hi Mohit, Jingoo, > > On Wednesday 25 June 2014 11:26 PM, Kishon Vijay Abraham I wrote: > > In DRA7, the cpu sees 32bit address, but the pcie controller can see only 28bit > > address. So whenever the cpu issues a read/write request, the 4 most > > significant bits are used by L3 to determine the target controller. > > For example, the cpu reserves 0x2000_0000 - 0x2FFF_FFFF for PCIe controller but > > the PCIe controller will see only (0x000_0000 - 0xFFF_FFF). So for programming > > the outbound translation window the *base* should be programmed as 0x000_0000. > > Whenever we try to write to say 0x2000_0000, it will be translated to whatever > > we have programmed in the translation window with base as 0x000_0000. > > > > This is needed when the dt node is modelled something like below > > axi { > > compatible = "simple-bus"; > > #size-cells = <1>; > > #address-cells = <1>; > > ranges = <0x0 0x20000000 0x10000000 // 28-bit bus > > 0x51000000 0x51000000 0x3000>; > > pcie@51000000 { > > reg = <0x1000 0x2000>, <0x51002000 0x14c>, <0x51000000 0x2000>; > > reg-names = "config", "ti_conf", "rc_dbics"; > > #address-cells = <3>; > > #size-cells = <2>; > > ranges = <0x81000000 0 0 0x03000 0 0x00010000 > > 0x82000000 0 0x20013000 0x13000 0 0xffed000>; > > }; > > }; > > > > Here the CPU address for configuration space is 0x20013000 and the controller > > address for configuration space is 0x13000. The controller address should be > > used while programming the ATU (in order for translation to happen properly in > > DRA7xx). > > Are you okay with this patch? Can you give your Reviewed-by/Acked-by? Sorry, I cannot understand fully what this patch does. :-( However, I have some questions as below. 1. io_mod_addr The 'io_mod_addr' variable is not used. Then, how about removing 'io_mod_addr' from this patch? 2. ranges In the case of Exynos PCIe, there is no limitation such as 28bit for access. Then, when there is no 'ranges' property in DT file, is there no side effect? I mean the compatibility with other SoC's PCIe drivers. 3. _addr post fix How about renaming 'cfg0_mod_addr', 'mem_mod_addr' as below? u64 cfg0_base; + u64 cfg0_mod_base; void __iomem *va_cfg0_base; u64 cfg1_base; + u64 cfg1_mod_base; void __iomem *va_cfg1_base; u64 io_base; + u64 io_mod_base; u64 mem_base; + u64 mem_mod_base; Best regards, Jingoo Han > > Cheers > Kishon > > > > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > Cc: Bjorn Helgaas <bhelgaas@google.com> > > Cc: Mohit Kumar <mohit.kumar@st.com> > > Cc: Jingoo Han <jg1.han@samsung.com> > > Cc: Marek Vasut <marex@denx.de> > > Cc: Arnd Bergmann <arnd@arndb.de> > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > > --- > > drivers/pci/host/pcie-designware.c | 49 ++++++++++++++++++++++++++++-------- > > drivers/pci/host/pcie-designware.h | 4 +++ > > 2 files changed, 42 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > > index 0b7b455..11aba6d 100644 > > --- a/drivers/pci/host/pcie-designware.c > > +++ b/drivers/pci/host/pcie-designware.c > > @@ -401,8 +401,15 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > struct of_pci_range range; > > struct of_pci_range_parser parser; > > struct resource *cfg_res; > > - u32 val; > > - int i; > > + u32 val, na, ns; > > + const __be32 *addrp; > > + int i, index; > > + > > + /* Find the address cell size and the number of cells in order to get > > + * the untranslated address. > > + */ > > + of_property_read_u32(np, "#address-cells", &na); > > + ns = of_n_size_cells(np); > > > > cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config"); > > if (cfg_res) { > > @@ -410,6 +417,12 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > pp->config.cfg1_size = resource_size(cfg_res)/2; > > pp->cfg0_base = cfg_res->start; > > pp->cfg1_base = cfg_res->start + pp->config.cfg0_size; > > + > > + /* Find the untranslated configuration space address */ > > + index = of_property_match_string(np, "reg-names", "config"); > > + addrp = of_get_address(np, index, false, false); > > + pp->cfg0_mod_addr = of_read_number(addrp, ns); > > + pp->cfg1_mod_addr = pp->cfg0_mod_addr + pp->config.cfg0_size; > > } else { > > dev_err(pp->dev, "missing *config* reg space\n"); > > } > > @@ -435,12 +448,20 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > pp->config.io_size = resource_size(&pp->io); > > pp->config.io_bus_addr = range.pci_addr; > > pp->io_base = range.cpu_addr; > > + > > + /* Find the untranslated IO space address */ > > + pp->io_mod_addr = of_read_number(parser.range - > > + parser.np + na, ns); > > } > > if (restype == IORESOURCE_MEM) { > > of_pci_range_to_resource(&range, np, &pp->mem); > > pp->mem.name = "MEM"; > > pp->config.mem_size = resource_size(&pp->mem); > > pp->config.mem_bus_addr = range.pci_addr; > > + > > + /* Find the untranslated MEM space address */ > > + pp->mem_mod_addr = of_read_number(parser.range - > > + parser.np + na, ns); > > } > > if (restype == 0) { > > of_pci_range_to_resource(&range, np, &pp->cfg); > > @@ -448,6 +469,12 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > pp->config.cfg1_size = resource_size(&pp->cfg)/2; > > pp->cfg0_base = pp->cfg.start; > > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; > > + > > + /* Find the untranslated configuration space address */ > > + pp->cfg0_mod_addr = of_read_number(parser.range - > > + parser.np + na, ns); > > + pp->cfg1_mod_addr = pp->cfg0_mod_addr + > > + pp->config.cfg0_size; > > } > > } > > > > @@ -522,9 +549,9 @@ static void dw_pcie_prog_viewport_cfg0(struct pcie_port *pp, u32 busdev) > > /* Program viewport 0 : OUTBOUND : CFG0 */ > > dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX0, > > PCIE_ATU_VIEWPORT); > > - dw_pcie_writel_rc(pp, pp->cfg0_base, PCIE_ATU_LOWER_BASE); > > - dw_pcie_writel_rc(pp, (pp->cfg0_base >> 32), PCIE_ATU_UPPER_BASE); > > - dw_pcie_writel_rc(pp, pp->cfg0_base + pp->config.cfg0_size - 1, > > + dw_pcie_writel_rc(pp, pp->cfg0_mod_addr, PCIE_ATU_LOWER_BASE); > > + dw_pcie_writel_rc(pp, (pp->cfg0_mod_addr >> 32), PCIE_ATU_UPPER_BASE); > > + dw_pcie_writel_rc(pp, pp->cfg0_mod_addr + pp->config.cfg0_size - 1, > > PCIE_ATU_LIMIT); > > dw_pcie_writel_rc(pp, busdev, PCIE_ATU_LOWER_TARGET); > > dw_pcie_writel_rc(pp, 0, PCIE_ATU_UPPER_TARGET); > > @@ -538,9 +565,9 @@ static void dw_pcie_prog_viewport_cfg1(struct pcie_port *pp, u32 busdev) > > dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX1, > > PCIE_ATU_VIEWPORT); > > dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_CFG1, PCIE_ATU_CR1); > > - dw_pcie_writel_rc(pp, pp->cfg1_base, PCIE_ATU_LOWER_BASE); > > - dw_pcie_writel_rc(pp, (pp->cfg1_base >> 32), PCIE_ATU_UPPER_BASE); > > - dw_pcie_writel_rc(pp, pp->cfg1_base + pp->config.cfg1_size - 1, > > + dw_pcie_writel_rc(pp, pp->cfg1_mod_addr, PCIE_ATU_LOWER_BASE); > > + dw_pcie_writel_rc(pp, (pp->cfg1_mod_addr >> 32), PCIE_ATU_UPPER_BASE); > > + dw_pcie_writel_rc(pp, pp->cfg1_mod_addr + pp->config.cfg1_size - 1, > > PCIE_ATU_LIMIT); > > dw_pcie_writel_rc(pp, busdev, PCIE_ATU_LOWER_TARGET); > > dw_pcie_writel_rc(pp, 0, PCIE_ATU_UPPER_TARGET); > > @@ -553,9 +580,9 @@ static void dw_pcie_prog_viewport_mem_outbound(struct pcie_port *pp) > > dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX0, > > PCIE_ATU_VIEWPORT); > > dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_MEM, PCIE_ATU_CR1); > > - dw_pcie_writel_rc(pp, pp->mem_base, PCIE_ATU_LOWER_BASE); > > - dw_pcie_writel_rc(pp, (pp->mem_base >> 32), PCIE_ATU_UPPER_BASE); > > - dw_pcie_writel_rc(pp, pp->mem_base + pp->config.mem_size - 1, > > + dw_pcie_writel_rc(pp, pp->mem_mod_addr, PCIE_ATU_LOWER_BASE); > > + dw_pcie_writel_rc(pp, (pp->mem_mod_addr >> 32), PCIE_ATU_UPPER_BASE); > > + dw_pcie_writel_rc(pp, pp->mem_mod_addr + pp->config.mem_size - 1, > > PCIE_ATU_LIMIT); > > dw_pcie_writel_rc(pp, pp->config.mem_bus_addr, PCIE_ATU_LOWER_TARGET); > > dw_pcie_writel_rc(pp, upper_32_bits(pp->config.mem_bus_addr), > > diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h > > index 77f592f..13828f8 100644 > > --- a/drivers/pci/host/pcie-designware.h > > +++ b/drivers/pci/host/pcie-designware.h > > @@ -36,11 +36,15 @@ struct pcie_port { > > u8 root_bus_nr; > > void __iomem *dbi_base; > > u64 cfg0_base; > > + u64 cfg0_mod_addr; > > void __iomem *va_cfg0_base; > > u64 cfg1_base; > > + u64 cfg1_mod_addr; > > void __iomem *va_cfg1_base; > > u64 io_base; > > + u64 io_mod_addr; > > u64 mem_base; > > + u64 mem_mod_addr; > > struct resource cfg; > > struct resource io; > > struct resource mem; > > -- 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
Hello Kishon, > -----Original Message----- > From: Kishon Vijay Abraham I [mailto:kishon@ti.com] > Sent: Tuesday, July 08, 2014 6:01 PM > To: devicetree@vger.kernel.org; linux-doc@vger.kernel.org; linux- > pci@vger.kernel.org; jg1.han@samsung.com; bhelgaas@google.com; Mohit > KUMAR DCG; linux-kernel@vger.kernel.org > Cc: grant.likely@linaro.org; Jason Gunthorpe; Marek Vasut; Arnd Bergmann > Subject: Re: [PATCH 2/3] PCI: designware: use untranslated address while > programming ATU > > Hi Mohit, Jingoo, > > On Wednesday 25 June 2014 11:26 PM, Kishon Vijay Abraham I wrote: > > In DRA7, the cpu sees 32bit address, but the pcie controller can see > > only 28bit address. So whenever the cpu issues a read/write request, > > the 4 most significant bits are used by L3 to determine the target controller. > > For example, the cpu reserves 0x2000_0000 - 0x2FFF_FFFF for PCIe > > controller but the PCIe controller will see only (0x000_0000 - > > 0xFFF_FFF). So for programming the outbound translation window the > *base* should be programmed as 0x000_0000. > > Whenever we try to write to say 0x2000_0000, it will be translated to > > whatever we have programmed in the translation window with base as > 0x000_0000. > > > > This is needed when the dt node is modelled something like below axi { > > compatible = "simple-bus"; > > #size-cells = <1>; > > #address-cells = <1>; > > ranges = <0x0 0x20000000 0x10000000 // 28-bit bus > > 0x51000000 0x51000000 0x3000>; > > pcie@51000000 { > > reg = <0x1000 0x2000>, <0x51002000 0x14c>, <0x51000000 > 0x2000>; > > reg-names = "config", "ti_conf", "rc_dbics"; > > #address-cells = <3>; > > #size-cells = <2>; > > ranges = <0x81000000 0 0 0x03000 0 0x00010000 > > 0x82000000 0 0x20013000 0x13000 0 0xffed000>; > > }; > > }; > > > > Here the CPU address for configuration space is 0x20013000 and the > > controller address for configuration space is 0x13000. The controller > > address should be used while programming the ATU (in order for > > translation to happen properly in DRA7xx). > > Are you okay with this patch? Can you give your Reviewed-by/Acked-by? > - looks fine to me. Reviewed-by: Mohit Kumar<mohit.kumar@st.com> Thanks Mohit > Cheers > Kishon > > > > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > Cc: Bjorn Helgaas <bhelgaas@google.com> > > Cc: Mohit Kumar <mohit.kumar@st.com> > > Cc: Jingoo Han <jg1.han@samsung.com> > > Cc: Marek Vasut <marex@denx.de> > > Cc: Arnd Bergmann <arnd@arndb.de> > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > > --- > > drivers/pci/host/pcie-designware.c | 49 > ++++++++++++++++++++++++++++-------- > > drivers/pci/host/pcie-designware.h | 4 +++ > > 2 files changed, 42 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/pci/host/pcie-designware.c > > b/drivers/pci/host/pcie-designware.c > > index 0b7b455..11aba6d 100644 > > --- a/drivers/pci/host/pcie-designware.c > > +++ b/drivers/pci/host/pcie-designware.c > > @@ -401,8 +401,15 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > struct of_pci_range range; > > struct of_pci_range_parser parser; > > struct resource *cfg_res; > > - u32 val; > > - int i; > > + u32 val, na, ns; > > + const __be32 *addrp; > > + int i, index; > > + > > + /* Find the address cell size and the number of cells in order to get > > + * the untranslated address. > > + */ > > + of_property_read_u32(np, "#address-cells", &na); > > + ns = of_n_size_cells(np); > > > > cfg_res = platform_get_resource_byname(pdev, > IORESOURCE_MEM, "config"); > > if (cfg_res) { > > @@ -410,6 +417,12 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > pp->config.cfg1_size = resource_size(cfg_res)/2; > > pp->cfg0_base = cfg_res->start; > > pp->cfg1_base = cfg_res->start + pp->config.cfg0_size; > > + > > + /* Find the untranslated configuration space address */ > > + index = of_property_match_string(np, "reg-names", > "config"); > > + addrp = of_get_address(np, index, false, false); > > + pp->cfg0_mod_addr = of_read_number(addrp, ns); > > + pp->cfg1_mod_addr = pp->cfg0_mod_addr + pp- > >config.cfg0_size; > > } else { > > dev_err(pp->dev, "missing *config* reg space\n"); > > } > > @@ -435,12 +448,20 @@ int __init dw_pcie_host_init(struct pcie_port > *pp) > > pp->config.io_size = resource_size(&pp->io); > > pp->config.io_bus_addr = range.pci_addr; > > pp->io_base = range.cpu_addr; > > + > > + /* Find the untranslated IO space address */ > > + pp->io_mod_addr = of_read_number(parser.range - > > + parser.np + na, ns); > > } > > if (restype == IORESOURCE_MEM) { > > of_pci_range_to_resource(&range, np, &pp->mem); > > pp->mem.name = "MEM"; > > pp->config.mem_size = resource_size(&pp->mem); > > pp->config.mem_bus_addr = range.pci_addr; > > + > > + /* Find the untranslated MEM space address */ > > + pp->mem_mod_addr = > of_read_number(parser.range - > > + parser.np + na, ns); > > } > > if (restype == 0) { > > of_pci_range_to_resource(&range, np, &pp->cfg); > @@ -448,6 +469,12 > > @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > pp->config.cfg1_size = resource_size(&pp->cfg)/2; > > pp->cfg0_base = pp->cfg.start; > > pp->cfg1_base = pp->cfg.start + pp- > >config.cfg0_size; > > + > > + /* Find the untranslated configuration space address > */ > > + pp->cfg0_mod_addr = > of_read_number(parser.range - > > + parser.np + na, ns); > > + pp->cfg1_mod_addr = pp->cfg0_mod_addr + > > + pp->config.cfg0_size; > > } > > } > > > > @@ -522,9 +549,9 @@ static void dw_pcie_prog_viewport_cfg0(struct > pcie_port *pp, u32 busdev) > > /* Program viewport 0 : OUTBOUND : CFG0 */ > > dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | > PCIE_ATU_REGION_INDEX0, > > PCIE_ATU_VIEWPORT); > > - dw_pcie_writel_rc(pp, pp->cfg0_base, PCIE_ATU_LOWER_BASE); > > - dw_pcie_writel_rc(pp, (pp->cfg0_base >> 32), > PCIE_ATU_UPPER_BASE); > > - dw_pcie_writel_rc(pp, pp->cfg0_base + pp->config.cfg0_size - 1, > > + dw_pcie_writel_rc(pp, pp->cfg0_mod_addr, > PCIE_ATU_LOWER_BASE); > > + dw_pcie_writel_rc(pp, (pp->cfg0_mod_addr >> 32), > PCIE_ATU_UPPER_BASE); > > + dw_pcie_writel_rc(pp, pp->cfg0_mod_addr + pp->config.cfg0_size - > 1, > > PCIE_ATU_LIMIT); > > dw_pcie_writel_rc(pp, busdev, PCIE_ATU_LOWER_TARGET); > > dw_pcie_writel_rc(pp, 0, PCIE_ATU_UPPER_TARGET); @@ -538,9 > +565,9 @@ > > static void dw_pcie_prog_viewport_cfg1(struct pcie_port *pp, u32 busdev) > > dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | > PCIE_ATU_REGION_INDEX1, > > PCIE_ATU_VIEWPORT); > > dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_CFG1, PCIE_ATU_CR1); > > - dw_pcie_writel_rc(pp, pp->cfg1_base, PCIE_ATU_LOWER_BASE); > > - dw_pcie_writel_rc(pp, (pp->cfg1_base >> 32), > PCIE_ATU_UPPER_BASE); > > - dw_pcie_writel_rc(pp, pp->cfg1_base + pp->config.cfg1_size - 1, > > + dw_pcie_writel_rc(pp, pp->cfg1_mod_addr, > PCIE_ATU_LOWER_BASE); > > + dw_pcie_writel_rc(pp, (pp->cfg1_mod_addr >> 32), > PCIE_ATU_UPPER_BASE); > > + dw_pcie_writel_rc(pp, pp->cfg1_mod_addr + pp->config.cfg1_size - > 1, > > PCIE_ATU_LIMIT); > > dw_pcie_writel_rc(pp, busdev, PCIE_ATU_LOWER_TARGET); > > dw_pcie_writel_rc(pp, 0, PCIE_ATU_UPPER_TARGET); @@ -553,9 > +580,9 @@ > > static void dw_pcie_prog_viewport_mem_outbound(struct pcie_port *pp) > > dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | > PCIE_ATU_REGION_INDEX0, > > PCIE_ATU_VIEWPORT); > > dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_MEM, PCIE_ATU_CR1); > > - dw_pcie_writel_rc(pp, pp->mem_base, PCIE_ATU_LOWER_BASE); > > - dw_pcie_writel_rc(pp, (pp->mem_base >> 32), > PCIE_ATU_UPPER_BASE); > > - dw_pcie_writel_rc(pp, pp->mem_base + pp->config.mem_size - 1, > > + dw_pcie_writel_rc(pp, pp->mem_mod_addr, > PCIE_ATU_LOWER_BASE); > > + dw_pcie_writel_rc(pp, (pp->mem_mod_addr >> 32), > PCIE_ATU_UPPER_BASE); > > + dw_pcie_writel_rc(pp, pp->mem_mod_addr + pp->config.mem_size > - 1, > > PCIE_ATU_LIMIT); > > dw_pcie_writel_rc(pp, pp->config.mem_bus_addr, > PCIE_ATU_LOWER_TARGET); > > dw_pcie_writel_rc(pp, upper_32_bits(pp->config.mem_bus_addr), > > diff --git a/drivers/pci/host/pcie-designware.h > > b/drivers/pci/host/pcie-designware.h > > index 77f592f..13828f8 100644 > > --- a/drivers/pci/host/pcie-designware.h > > +++ b/drivers/pci/host/pcie-designware.h > > @@ -36,11 +36,15 @@ struct pcie_port { > > u8 root_bus_nr; > > void __iomem *dbi_base; > > u64 cfg0_base; > > + u64 cfg0_mod_addr; > > void __iomem *va_cfg0_base; > > u64 cfg1_base; > > + u64 cfg1_mod_addr; > > void __iomem *va_cfg1_base; > > u64 io_base; > > + u64 io_mod_addr; > > u64 mem_base; > > + u64 mem_mod_addr; > > struct resource cfg; > > struct resource io; > > struct resource mem; > > -- 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
Hi Jingoo, On Tuesday 08 July 2014 06:47 PM, Jingoo Han wrote: > On Tuesday, July 08, 2014 9:31 PM, Kishon Vijay Abraham I wrote: >> >> Hi Mohit, Jingoo, >> >> On Wednesday 25 June 2014 11:26 PM, Kishon Vijay Abraham I wrote: >>> In DRA7, the cpu sees 32bit address, but the pcie controller can see only 28bit >>> address. So whenever the cpu issues a read/write request, the 4 most >>> significant bits are used by L3 to determine the target controller. >>> For example, the cpu reserves 0x2000_0000 - 0x2FFF_FFFF for PCIe controller but >>> the PCIe controller will see only (0x000_0000 - 0xFFF_FFF). So for programming >>> the outbound translation window the *base* should be programmed as 0x000_0000. >>> Whenever we try to write to say 0x2000_0000, it will be translated to whatever >>> we have programmed in the translation window with base as 0x000_0000. >>> >>> This is needed when the dt node is modelled something like below >>> axi { >>> compatible = "simple-bus"; >>> #size-cells = <1>; >>> #address-cells = <1>; >>> ranges = <0x0 0x20000000 0x10000000 // 28-bit bus >>> 0x51000000 0x51000000 0x3000>; >>> pcie@51000000 { >>> reg = <0x1000 0x2000>, <0x51002000 0x14c>, <0x51000000 0x2000>; >>> reg-names = "config", "ti_conf", "rc_dbics"; >>> #address-cells = <3>; >>> #size-cells = <2>; >>> ranges = <0x81000000 0 0 0x03000 0 0x00010000 >>> 0x82000000 0 0x20013000 0x13000 0 0xffed000>; >>> }; >>> }; >>> >>> Here the CPU address for configuration space is 0x20013000 and the controller >>> address for configuration space is 0x13000. The controller address should be >>> used while programming the ATU (in order for translation to happen properly in >>> DRA7xx). >> >> Are you okay with this patch? Can you give your Reviewed-by/Acked-by? > > Sorry, I cannot understand fully what this patch does. :-( > However, I have some questions as below. > > 1. io_mod_addr > The 'io_mod_addr' variable is not used. Then, how about removing > 'io_mod_addr' from this patch? Actually I missed using it in io_outbound(). Will fix it. > > 2. ranges > In the case of Exynos PCIe, there is no limitation such as 28bit for > access. Then, when there is no 'ranges' property in DT file, is there > no side effect? I mean the compatibility with other SoC's PCIe drivers. It shouldn't break anything. The ranges is used to resolve cpu address while adding resources to the device. So in other SoCs it'll use use the parent 'ranges' which is usually ocp/simple-bus to resolve cpu address. In my case it has to use the axi ranges. So other SoCs shouldn't be impacted. > > 3. _addr post fix > > How about renaming 'cfg0_mod_addr', 'mem_mod_addr' as below? sure. Thanks Kishon -- 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
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c index 0b7b455..11aba6d 100644 --- a/drivers/pci/host/pcie-designware.c +++ b/drivers/pci/host/pcie-designware.c @@ -401,8 +401,15 @@ int __init dw_pcie_host_init(struct pcie_port *pp) struct of_pci_range range; struct of_pci_range_parser parser; struct resource *cfg_res; - u32 val; - int i; + u32 val, na, ns; + const __be32 *addrp; + int i, index; + + /* Find the address cell size and the number of cells in order to get + * the untranslated address. + */ + of_property_read_u32(np, "#address-cells", &na); + ns = of_n_size_cells(np); cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config"); if (cfg_res) { @@ -410,6 +417,12 @@ int __init dw_pcie_host_init(struct pcie_port *pp) pp->config.cfg1_size = resource_size(cfg_res)/2; pp->cfg0_base = cfg_res->start; pp->cfg1_base = cfg_res->start + pp->config.cfg0_size; + + /* Find the untranslated configuration space address */ + index = of_property_match_string(np, "reg-names", "config"); + addrp = of_get_address(np, index, false, false); + pp->cfg0_mod_addr = of_read_number(addrp, ns); + pp->cfg1_mod_addr = pp->cfg0_mod_addr + pp->config.cfg0_size; } else { dev_err(pp->dev, "missing *config* reg space\n"); } @@ -435,12 +448,20 @@ int __init dw_pcie_host_init(struct pcie_port *pp) pp->config.io_size = resource_size(&pp->io); pp->config.io_bus_addr = range.pci_addr; pp->io_base = range.cpu_addr; + + /* Find the untranslated IO space address */ + pp->io_mod_addr = of_read_number(parser.range - + parser.np + na, ns); } if (restype == IORESOURCE_MEM) { of_pci_range_to_resource(&range, np, &pp->mem); pp->mem.name = "MEM"; pp->config.mem_size = resource_size(&pp->mem); pp->config.mem_bus_addr = range.pci_addr; + + /* Find the untranslated MEM space address */ + pp->mem_mod_addr = of_read_number(parser.range - + parser.np + na, ns); } if (restype == 0) { of_pci_range_to_resource(&range, np, &pp->cfg); @@ -448,6 +469,12 @@ int __init dw_pcie_host_init(struct pcie_port *pp) pp->config.cfg1_size = resource_size(&pp->cfg)/2; pp->cfg0_base = pp->cfg.start; pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size; + + /* Find the untranslated configuration space address */ + pp->cfg0_mod_addr = of_read_number(parser.range - + parser.np + na, ns); + pp->cfg1_mod_addr = pp->cfg0_mod_addr + + pp->config.cfg0_size; } } @@ -522,9 +549,9 @@ static void dw_pcie_prog_viewport_cfg0(struct pcie_port *pp, u32 busdev) /* Program viewport 0 : OUTBOUND : CFG0 */ dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX0, PCIE_ATU_VIEWPORT); - dw_pcie_writel_rc(pp, pp->cfg0_base, PCIE_ATU_LOWER_BASE); - dw_pcie_writel_rc(pp, (pp->cfg0_base >> 32), PCIE_ATU_UPPER_BASE); - dw_pcie_writel_rc(pp, pp->cfg0_base + pp->config.cfg0_size - 1, + dw_pcie_writel_rc(pp, pp->cfg0_mod_addr, PCIE_ATU_LOWER_BASE); + dw_pcie_writel_rc(pp, (pp->cfg0_mod_addr >> 32), PCIE_ATU_UPPER_BASE); + dw_pcie_writel_rc(pp, pp->cfg0_mod_addr + pp->config.cfg0_size - 1, PCIE_ATU_LIMIT); dw_pcie_writel_rc(pp, busdev, PCIE_ATU_LOWER_TARGET); dw_pcie_writel_rc(pp, 0, PCIE_ATU_UPPER_TARGET); @@ -538,9 +565,9 @@ static void dw_pcie_prog_viewport_cfg1(struct pcie_port *pp, u32 busdev) dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX1, PCIE_ATU_VIEWPORT); dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_CFG1, PCIE_ATU_CR1); - dw_pcie_writel_rc(pp, pp->cfg1_base, PCIE_ATU_LOWER_BASE); - dw_pcie_writel_rc(pp, (pp->cfg1_base >> 32), PCIE_ATU_UPPER_BASE); - dw_pcie_writel_rc(pp, pp->cfg1_base + pp->config.cfg1_size - 1, + dw_pcie_writel_rc(pp, pp->cfg1_mod_addr, PCIE_ATU_LOWER_BASE); + dw_pcie_writel_rc(pp, (pp->cfg1_mod_addr >> 32), PCIE_ATU_UPPER_BASE); + dw_pcie_writel_rc(pp, pp->cfg1_mod_addr + pp->config.cfg1_size - 1, PCIE_ATU_LIMIT); dw_pcie_writel_rc(pp, busdev, PCIE_ATU_LOWER_TARGET); dw_pcie_writel_rc(pp, 0, PCIE_ATU_UPPER_TARGET); @@ -553,9 +580,9 @@ static void dw_pcie_prog_viewport_mem_outbound(struct pcie_port *pp) dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX0, PCIE_ATU_VIEWPORT); dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_MEM, PCIE_ATU_CR1); - dw_pcie_writel_rc(pp, pp->mem_base, PCIE_ATU_LOWER_BASE); - dw_pcie_writel_rc(pp, (pp->mem_base >> 32), PCIE_ATU_UPPER_BASE); - dw_pcie_writel_rc(pp, pp->mem_base + pp->config.mem_size - 1, + dw_pcie_writel_rc(pp, pp->mem_mod_addr, PCIE_ATU_LOWER_BASE); + dw_pcie_writel_rc(pp, (pp->mem_mod_addr >> 32), PCIE_ATU_UPPER_BASE); + dw_pcie_writel_rc(pp, pp->mem_mod_addr + pp->config.mem_size - 1, PCIE_ATU_LIMIT); dw_pcie_writel_rc(pp, pp->config.mem_bus_addr, PCIE_ATU_LOWER_TARGET); dw_pcie_writel_rc(pp, upper_32_bits(pp->config.mem_bus_addr), diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h index 77f592f..13828f8 100644 --- a/drivers/pci/host/pcie-designware.h +++ b/drivers/pci/host/pcie-designware.h @@ -36,11 +36,15 @@ struct pcie_port { u8 root_bus_nr; void __iomem *dbi_base; u64 cfg0_base; + u64 cfg0_mod_addr; void __iomem *va_cfg0_base; u64 cfg1_base; + u64 cfg1_mod_addr; void __iomem *va_cfg1_base; u64 io_base; + u64 io_mod_addr; u64 mem_base; + u64 mem_mod_addr; struct resource cfg; struct resource io; struct resource mem;
In DRA7, the cpu sees 32bit address, but the pcie controller can see only 28bit address. So whenever the cpu issues a read/write request, the 4 most significant bits are used by L3 to determine the target controller. For example, the cpu reserves 0x2000_0000 - 0x2FFF_FFFF for PCIe controller but the PCIe controller will see only (0x000_0000 - 0xFFF_FFF). So for programming the outbound translation window the *base* should be programmed as 0x000_0000. Whenever we try to write to say 0x2000_0000, it will be translated to whatever we have programmed in the translation window with base as 0x000_0000. This is needed when the dt node is modelled something like below axi { compatible = "simple-bus"; #size-cells = <1>; #address-cells = <1>; ranges = <0x0 0x20000000 0x10000000 // 28-bit bus 0x51000000 0x51000000 0x3000>; pcie@51000000 { reg = <0x1000 0x2000>, <0x51002000 0x14c>, <0x51000000 0x2000>; reg-names = "config", "ti_conf", "rc_dbics"; #address-cells = <3>; #size-cells = <2>; ranges = <0x81000000 0 0 0x03000 0 0x00010000 0x82000000 0 0x20013000 0x13000 0 0xffed000>; }; }; Here the CPU address for configuration space is 0x20013000 and the controller address for configuration space is 0x13000. The controller address should be used while programming the ATU (in order for translation to happen properly in DRA7xx). Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Mohit Kumar <mohit.kumar@st.com> Cc: Jingoo Han <jg1.han@samsung.com> Cc: Marek Vasut <marex@denx.de> Cc: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> --- drivers/pci/host/pcie-designware.c | 49 ++++++++++++++++++++++++++++-------- drivers/pci/host/pcie-designware.h | 4 +++ 2 files changed, 42 insertions(+), 11 deletions(-)