| Message ID | 20200724203407.16972-10-james.quinlan@broadcom.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: brcmstb: enable PCIe for STB chips | expand |
Hi Jim, On Fri, 2020-07-24 at 16:33 -0400, Jim Quinlan wrote: > The Raspberry Pi (RPI) is currently the only chip using this driver > (pcie-brcmstb.c). There, only one memory controller is used, without an > extension region, and the SCB0 viewport size is set to the size of the > first and only dma-range region. Other BrcmSTB SOCs have more complicated > memory configurations that require setting additional viewport sizes. > > BrcmSTB PCIe controllers are intimately connected to the memory > controller(s) on the SOC. The SOC may have one to three memory > controllers; they are indicated by the term SCBi. Each controller has a > base region and an optional extension region. In physical memory, the base > and extension regions of a controller are not adjacent, but in PCIe-space > they are. > > There is a "viewport" for each memory controller that allows DMA from > endpoint devices. Each viewport's size must be set to a power of two, and > that size must be equal to or larger than the amount of memory each > controller supports which is the sum of base region and its optional > extension. Further, the 1-3 viewports are also adjacent in PCIe-space. > > Unfortunately the viewport sizes cannot be ascertained from the > "dma-ranges" property so they have their own property, "brcm,scb-sizes". > This is because dma-range information does not indicate what memory > controller it is associated. For example, consider the following case > where the size of one dma-range is 2GB and the second dma-range is 1GB: > > /* Case 1: SCB0 size set to 4GB */ > dma-range0: 2GB (from memc0-base) > dma-range1: 1GB (from memc0-extension) > > /* Case 2: SCB0 size set to 2GB, SCB1 size set to 1GB */ > dma-range0: 2GB (from memc0-base) > dma-range1: 1GB (from memc0-extension) > > By just looking at the dma-ranges information, one cannot tell which > situation applies. That is why an additional property is needed. Its > length indicates the number of memory controllers being used and each value > indicates the viewport size. > > Note that the RPI DT does not have a "brcm,scb-sizes" property value, > as it is assumed that it only requires one memory controller and no > extension. So the optional use of "brcm,scb-sizes" will be backwards > compatible. > > One last layer of complexity exists: all of the viewports sizes must be > added and rounded up to a power of two to determine what the "BAR" size is. > Further, an offset must be given that indicates the base PCIe address of > this "BAR". The use of the term BAR is typically associated with endpoint > devices, and the term is used here because the PCIe HW may be used as an RC > or an EP. In the former case, all of the system memory appears in a single > "BAR" region in PCIe memory. As it turns out, BrcmSTB PCIe HW is rarely > used in the EP role and its system of mapping memory is an artifact that > requires multiple dma-ranges regions. > > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com> > Acked-by: Florian Fainelli <f.fainelli@gmail.com> > --- > drivers/pci/controller/pcie-brcmstb.c | 68 ++++++++++++++++++++------- > 1 file changed, 50 insertions(+), 18 deletions(-) > > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c > index 8dacb9d3b7b6..3ef2d37cc43b 100644 > --- a/drivers/pci/controller/pcie-brcmstb.c > +++ b/drivers/pci/controller/pcie-brcmstb.c > @@ -715,22 +720,44 @@ static inline int brcm_pcie_get_rc_bar2_size_and_offset(struct brcm_pcie *pcie, > u64 *rc_bar2_offset) > { > struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie); > - struct device *dev = pcie->dev; > struct resource_entry *entry; > + struct device *dev = pcie->dev; > + u64 lowest_pcie_addr = ~(u64)0; > + int ret, i = 0; > + u64 size = 0; > > - entry = resource_list_first_type(&bridge->dma_ranges, IORESOURCE_MEM); > - if (!entry) > - return -ENODEV; > + resource_list_for_each_entry(entry, &bridge->dma_ranges) { > + u64 pcie_beg = entry->res->start - entry->offset; > > + size += entry->res->end - entry->res->start + 1; > + if (pcie_beg < lowest_pcie_addr) > + lowest_pcie_addr = pcie_beg; > + } > > - /* > - * The controller expects the inbound window offset to be calculated as > - * the difference between PCIe's address space and CPU's. The offset > - * provided by the firmware is calculated the opposite way, so we > - * negate it. > - */ > - *rc_bar2_offset = -entry->offset; > - *rc_bar2_size = 1ULL << fls64(entry->res->end - entry->res->start); > + if (lowest_pcie_addr == ~(u64)0) { > + dev_err(dev, "DT node has no dma-ranges\n"); > + return -EINVAL; > + } > + > + ret = of_property_read_variable_u64_array(pcie->np, "brcm,scb-sizes", pcie->memc_size, 1, > + PCIE_BRCM_MAX_MEMC); > + > + if (ret <= 0) { > + /* Make an educated guess */ > + pcie->num_memc = 1; > + pcie->memc_size[0] = 1 << fls64(size - 1); You need to 1ULL here. Regards, Nicolas > + } else { > + pcie->num_memc = ret; > + } > + > + /* Each memc is viewed through a "port" that is a power of 2 */ > + for (i = 0, size = 0; i < pcie->num_memc; i++) > + size += pcie->memc_size[i]; > + > + /* System memory starts at this address in PCIe-space */ > + *rc_bar2_offset = lowest_pcie_addr; > + /* The sum of all memc views must also be a power of 2 */ > + *rc_bar2_size = 1ULL << fls64(size - 1); > > /* > * We validate the inbound memory view even though we should trust
fls64 On Sat, Aug 1, 2020 at 1:39 PM Nicolas Saenz Julienne <nsaenzjulienne@suse.de> wrote: > > Hi Jim, > > On Fri, 2020-07-24 at 16:33 -0400, Jim Quinlan wrote: > > The Raspberry Pi (RPI) is currently the only chip using this driver > > (pcie-brcmstb.c). There, only one memory controller is used, without an > > extension region, and the SCB0 viewport size is set to the size of the > > first and only dma-range region. Other BrcmSTB SOCs have more complicated > > memory configurations that require setting additional viewport sizes. > > > > BrcmSTB PCIe controllers are intimately connected to the memory > > controller(s) on the SOC. The SOC may have one to three memory > > controllers; they are indicated by the term SCBi. Each controller has a > > base region and an optional extension region. In physical memory, the base > > and extension regions of a controller are not adjacent, but in PCIe-space > > they are. > > > > There is a "viewport" for each memory controller that allows DMA from > > endpoint devices. Each viewport's size must be set to a power of two, and > > that size must be equal to or larger than the amount of memory each > > controller supports which is the sum of base region and its optional > > extension. Further, the 1-3 viewports are also adjacent in PCIe-space. > > > > Unfortunately the viewport sizes cannot be ascertained from the > > "dma-ranges" property so they have their own property, "brcm,scb-sizes". > > This is because dma-range information does not indicate what memory > > controller it is associated. For example, consider the following case > > where the size of one dma-range is 2GB and the second dma-range is 1GB: > > > > /* Case 1: SCB0 size set to 4GB */ > > dma-range0: 2GB (from memc0-base) > > dma-range1: 1GB (from memc0-extension) > > > > /* Case 2: SCB0 size set to 2GB, SCB1 size set to 1GB */ > > dma-range0: 2GB (from memc0-base) > > dma-range1: 1GB (from memc0-extension) > > > > By just looking at the dma-ranges information, one cannot tell which > > situation applies. That is why an additional property is needed. Its > > length indicates the number of memory controllers being used and each value > > indicates the viewport size. > > > > Note that the RPI DT does not have a "brcm,scb-sizes" property value, > > as it is assumed that it only requires one memory controller and no > > extension. So the optional use of "brcm,scb-sizes" will be backwards > > compatible. > > > > One last layer of complexity exists: all of the viewports sizes must be > > added and rounded up to a power of two to determine what the "BAR" size is. > > Further, an offset must be given that indicates the base PCIe address of > > this "BAR". The use of the term BAR is typically associated with endpoint > > devices, and the term is used here because the PCIe HW may be used as an RC > > or an EP. In the former case, all of the system memory appears in a single > > "BAR" region in PCIe memory. As it turns out, BrcmSTB PCIe HW is rarely > > used in the EP role and its system of mapping memory is an artifact that > > requires multiple dma-ranges regions. > > > > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com> > > Acked-by: Florian Fainelli <f.fainelli@gmail.com> > > --- > > drivers/pci/controller/pcie-brcmstb.c | 68 ++++++++++++++++++++------- > > 1 file changed, 50 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c > > index 8dacb9d3b7b6..3ef2d37cc43b 100644 > > --- a/drivers/pci/controller/pcie-brcmstb.c > > +++ b/drivers/pci/controller/pcie-brcmstb.c > > @@ -715,22 +720,44 @@ static inline int brcm_pcie_get_rc_bar2_size_and_offset(struct brcm_pcie *pcie, > > u64 *rc_bar2_offset) > > { > > struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie); > > - struct device *dev = pcie->dev; > > struct resource_entry *entry; > > + struct device *dev = pcie->dev; > > + u64 lowest_pcie_addr = ~(u64)0; > > + int ret, i = 0; > > + u64 size = 0; > > > > - entry = resource_list_first_type(&bridge->dma_ranges, IORESOURCE_MEM); > > - if (!entry) > > - return -ENODEV; > > + resource_list_for_each_entry(entry, &bridge->dma_ranges) { > > + u64 pcie_beg = entry->res->start - entry->offset; > > > > + size += entry->res->end - entry->res->start + 1; > > + if (pcie_beg < lowest_pcie_addr) > > + lowest_pcie_addr = pcie_beg; > > + } > > > > - /* > > - * The controller expects the inbound window offset to be calculated as > > - * the difference between PCIe's address space and CPU's. The offset > > - * provided by the firmware is calculated the opposite way, so we > > - * negate it. > > - */ > > - *rc_bar2_offset = -entry->offset; > > - *rc_bar2_size = 1ULL << fls64(entry->res->end - entry->res->start); > > + if (lowest_pcie_addr == ~(u64)0) { > > + dev_err(dev, "DT node has no dma-ranges\n"); > > + return -EINVAL; > > + } > > + > > + ret = of_property_read_variable_u64_array(pcie->np, "brcm,scb-sizes", pcie->memc_size, 1, > > + PCIE_BRCM_MAX_MEMC); > > + > > + if (ret <= 0) { > > + /* Make an educated guess */ > > + pcie->num_memc = 1; > > + pcie->memc_size[0] = 1 << fls64(size - 1); > > You need to 1ULL here. Got it. Thanks, Jim > > Regards, > Nicolas > > > + } else { > > + pcie->num_memc = ret; > > + } > > + > > + /* Each memc is viewed through a "port" that is a power of 2 */ > > + for (i = 0, size = 0; i < pcie->num_memc; i++) > > + size += pcie->memc_size[i]; > > + > > + /* System memory starts at this address in PCIe-space */ > > + *rc_bar2_offset = lowest_pcie_addr; > > + /* The sum of all memc views must also be a power of 2 */ > > + *rc_bar2_size = 1ULL << fls64(size - 1); > > > > /* > > * We validate the inbound memory view even though we should trust >
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c index 8dacb9d3b7b6..3ef2d37cc43b 100644 --- a/drivers/pci/controller/pcie-brcmstb.c +++ b/drivers/pci/controller/pcie-brcmstb.c @@ -57,6 +57,8 @@ #define PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK 0x300000 #define PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_128 0x0 #define PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK 0xf8000000 +#define PCIE_MISC_MISC_CTRL_SCB1_SIZE_MASK 0x07c00000 +#define PCIE_MISC_MISC_CTRL_SCB2_SIZE_MASK 0x0000001f #define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO 0x400c #define PCIE_MEM_WIN0_LO(win) \ @@ -154,6 +156,7 @@ #define SSC_STATUS_OFFSET 0x1 #define SSC_STATUS_SSC_MASK 0x400 #define SSC_STATUS_PLL_LOCK_MASK 0x800 +#define PCIE_BRCM_MAX_MEMC 3 #define IDX_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_INDEX]) #define DATA_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_DATA]) @@ -260,6 +263,8 @@ struct brcm_pcie { const int *reg_field_info; enum pcie_type type; struct reset_control *rescal; + int num_memc; + u64 memc_size[PCIE_BRCM_MAX_MEMC]; }; /* @@ -715,22 +720,44 @@ static inline int brcm_pcie_get_rc_bar2_size_and_offset(struct brcm_pcie *pcie, u64 *rc_bar2_offset) { struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie); - struct device *dev = pcie->dev; struct resource_entry *entry; + struct device *dev = pcie->dev; + u64 lowest_pcie_addr = ~(u64)0; + int ret, i = 0; + u64 size = 0; - entry = resource_list_first_type(&bridge->dma_ranges, IORESOURCE_MEM); - if (!entry) - return -ENODEV; + resource_list_for_each_entry(entry, &bridge->dma_ranges) { + u64 pcie_beg = entry->res->start - entry->offset; + size += entry->res->end - entry->res->start + 1; + if (pcie_beg < lowest_pcie_addr) + lowest_pcie_addr = pcie_beg; + } - /* - * The controller expects the inbound window offset to be calculated as - * the difference between PCIe's address space and CPU's. The offset - * provided by the firmware is calculated the opposite way, so we - * negate it. - */ - *rc_bar2_offset = -entry->offset; - *rc_bar2_size = 1ULL << fls64(entry->res->end - entry->res->start); + if (lowest_pcie_addr == ~(u64)0) { + dev_err(dev, "DT node has no dma-ranges\n"); + return -EINVAL; + } + + ret = of_property_read_variable_u64_array(pcie->np, "brcm,scb-sizes", pcie->memc_size, 1, + PCIE_BRCM_MAX_MEMC); + + if (ret <= 0) { + /* Make an educated guess */ + pcie->num_memc = 1; + pcie->memc_size[0] = 1 << fls64(size - 1); + } else { + pcie->num_memc = ret; + } + + /* Each memc is viewed through a "port" that is a power of 2 */ + for (i = 0, size = 0; i < pcie->num_memc; i++) + size += pcie->memc_size[i]; + + /* System memory starts at this address in PCIe-space */ + *rc_bar2_offset = lowest_pcie_addr; + /* The sum of all memc views must also be a power of 2 */ + *rc_bar2_size = 1ULL << fls64(size - 1); /* * We validate the inbound memory view even though we should trust @@ -782,12 +809,11 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie) void __iomem *base = pcie->base; struct device *dev = pcie->dev; struct resource_entry *entry; - unsigned int scb_size_val; bool ssc_good = false; struct resource *res; int num_out_wins = 0; u16 nlw, cls, lnksta; - int i, ret; + int i, ret, memc; u32 tmp, aspm_support; /* Reset the bridge */ @@ -827,11 +853,17 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie) writel(upper_32_bits(rc_bar2_offset), base + PCIE_MISC_RC_BAR2_CONFIG_HI); - scb_size_val = rc_bar2_size ? - ilog2(rc_bar2_size) - 15 : 0xf; /* 0xf is 1GB */ tmp = readl(base + PCIE_MISC_MISC_CTRL); - u32p_replace_bits(&tmp, scb_size_val, - PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK); + for (memc = 0; memc < pcie->num_memc; memc++) { + u32 scb_size_val = ilog2(pcie->memc_size[memc]) - 15; + + if (memc == 0) + u32p_replace_bits(&tmp, scb_size_val, PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK); + else if (memc == 1) + u32p_replace_bits(&tmp, scb_size_val, PCIE_MISC_MISC_CTRL_SCB1_SIZE_MASK); + else if (memc == 2) + u32p_replace_bits(&tmp, scb_size_val, PCIE_MISC_MISC_CTRL_SCB2_SIZE_MASK); + } writel(tmp, base + PCIE_MISC_MISC_CTRL); /*
The Raspberry Pi (RPI) is currently the only chip using this driver (pcie-brcmstb.c). There, only one memory controller is used, without an extension region, and the SCB0 viewport size is set to the size of the first and only dma-range region. Other BrcmSTB SOCs have more complicated memory configurations that require setting additional viewport sizes. BrcmSTB PCIe controllers are intimately connected to the memory controller(s) on the SOC. The SOC may have one to three memory controllers; they are indicated by the term SCBi. Each controller has a base region and an optional extension region. In physical memory, the base and extension regions of a controller are not adjacent, but in PCIe-space they are. There is a "viewport" for each memory controller that allows DMA from endpoint devices. Each viewport's size must be set to a power of two, and that size must be equal to or larger than the amount of memory each controller supports which is the sum of base region and its optional extension. Further, the 1-3 viewports are also adjacent in PCIe-space. Unfortunately the viewport sizes cannot be ascertained from the "dma-ranges" property so they have their own property, "brcm,scb-sizes". This is because dma-range information does not indicate what memory controller it is associated. For example, consider the following case where the size of one dma-range is 2GB and the second dma-range is 1GB: /* Case 1: SCB0 size set to 4GB */ dma-range0: 2GB (from memc0-base) dma-range1: 1GB (from memc0-extension) /* Case 2: SCB0 size set to 2GB, SCB1 size set to 1GB */ dma-range0: 2GB (from memc0-base) dma-range1: 1GB (from memc0-extension) By just looking at the dma-ranges information, one cannot tell which situation applies. That is why an additional property is needed. Its length indicates the number of memory controllers being used and each value indicates the viewport size. Note that the RPI DT does not have a "brcm,scb-sizes" property value, as it is assumed that it only requires one memory controller and no extension. So the optional use of "brcm,scb-sizes" will be backwards compatible. One last layer of complexity exists: all of the viewports sizes must be added and rounded up to a power of two to determine what the "BAR" size is. Further, an offset must be given that indicates the base PCIe address of this "BAR". The use of the term BAR is typically associated with endpoint devices, and the term is used here because the PCIe HW may be used as an RC or an EP. In the former case, all of the system memory appears in a single "BAR" region in PCIe memory. As it turns out, BrcmSTB PCIe HW is rarely used in the EP role and its system of mapping memory is an artifact that requires multiple dma-ranges regions. Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com> Acked-by: Florian Fainelli <f.fainelli@gmail.com> --- drivers/pci/controller/pcie-brcmstb.c | 68 ++++++++++++++++++++------- 1 file changed, 50 insertions(+), 18 deletions(-)