Hi Maintainers,
Gentle ping for this patch, if there is anything I need to do, please
let me know.
Thanks.
On Mon, 2023-10-23 at 16:14 +0800, Jianjun Wang wrote:
> The size of translation table should be a power of 2, using fls()
> cannot get the proper value when the size is not a power of 2. For
> example, fls(0x3e00000) - 1 = 25, hence the PCIe translation window
> size will be set to 0x2000000 instead of the expected size 0x3e00000.
>
> Fix translation window by splitting the MMIO space to multiple tables
> if
> its size is not a power of 2.
>
> Fixes: d3bf75b579b9 ("PCI: mediatek-gen3: Add MediaTek Gen3 driver
> for MT8192")
> Signed-off-by: Jianjun Wang <jianjun.wang@xxxxxxxxxxxx>
>
> ---
> Changes in v2:
> Only print warning message when reach the maximum translation table
> number, the MEM space still works but will be smaller than expected.
>
> Bootup logs on MT8195 Platform:
>
> > Before this patch:
>
> mtk-pcie-gen3 112f0000.pcie: Parsing ranges property...
> mtk-pcie-gen3 112f0000.pcie: IO 0x0020000000..0x00201fffff ->
> 0x0020000000
> mtk-pcie-gen3 112f0000.pcie: MEM 0x0020200000..0x0023ffffff ->
> 0x0020200000
> mtk-pcie-gen3 112f0000.pcie: set IO trans window[0]: cpu_addr =
> 0x20000000, pci_addr = 0x20000000, size = 0x200000
> mtk-pcie-gen3 112f0000.pcie: set MEM trans window[1]: cpu_addr =
> 0x20200000, pci_addr = 0x20200000, size = 0x3e00000
>
> > We expect the MEM trans window size to be 0x3e00000, but its actual
> > available size is 0x2000000.
> > After applying this patch:
>
> mtk-pcie-gen3 112f0000.pcie: Parsing ranges property...
> mtk-pcie-gen3 112f0000.pcie: IO 0x0020000000..0x00201fffff ->
> 0x0020000000
> mtk-pcie-gen3 112f0000.pcie: MEM 0x0020200000..0x0023ffffff ->
> 0x0020200000
> mtk-pcie-gen3 112f0000.pcie: set IO trans window[0]: cpu_addr =
> 0x20000000, pci_addr = 0x20000000, size = 0x200000
> mtk-pcie-gen3 112f0000.pcie: set MEM trans window[1]: cpu_addr =
> 0x20200000, pci_addr = 0x20200000, size = 0x200000
> mtk-pcie-gen3 112f0000.pcie: set MEM trans window[2]: cpu_addr =
> 0x20400000, pci_addr = 0x20400000, size = 0x400000
> mtk-pcie-gen3 112f0000.pcie: set MEM trans window[3]: cpu_addr =
> 0x20800000, pci_addr = 0x20800000, size = 0x800000
> mtk-pcie-gen3 112f0000.pcie: set MEM trans window[4]: cpu_addr =
> 0x21000000, pci_addr = 0x21000000, size = 0x1000000
> mtk-pcie-gen3 112f0000.pcie: set MEM trans window[5]: cpu_addr =
> 0x22000000, pci_addr = 0x22000000, size = 0x2000000
>
> > Total available size for MEM trans window is 0x3e00000.
>
> ---
> ---
> drivers/pci/controller/pcie-mediatek-gen3.c | 85 ++++++++++++-------
> --
> 1 file changed, 50 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c
> b/drivers/pci/controller/pcie-mediatek-gen3.c
> index e0e27645fdf4..975b3024fb08 100644
> --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
> @@ -245,35 +245,60 @@ static int mtk_pcie_set_trans_table(struct
> mtk_gen3_pcie *pcie,
> resource_size_t cpu_addr,
> resource_size_t pci_addr,
> resource_size_t size,
> - unsigned long type, int num)
> + unsigned long type, int *num)
> {
> + resource_size_t remaining = size;
> + resource_size_t table_size;
> + resource_size_t addr_align;
> + const char *range_type;
> void __iomem *table;
> u32 val;
>
> - if (num >= PCIE_MAX_TRANS_TABLES) {
> - dev_err(pcie->dev, "not enough translate table for
> addr: %#llx, limited to [%d]\n",
> - (unsigned long long)cpu_addr,
> PCIE_MAX_TRANS_TABLES);
> - return -ENODEV;
> - }
> + while (remaining && (*num < PCIE_MAX_TRANS_TABLES)) {
> + /* Table size needs to be a power of 2 */
> + table_size = BIT(fls(remaining) - 1);
> +
> + if (cpu_addr > 0) {
> + addr_align = BIT(ffs(cpu_addr) - 1);
> + table_size = min(table_size, addr_align);
> + }
> +
> + /* Minimum size of translate table is 4KiB */
> + if (table_size < 0x1000) {
> + dev_err(pcie->dev, "illegal table size
> %#llx\n",
> + (unsigned long long)table_size);
> + return -EINVAL;
> + }
>
> - table = pcie->base + PCIE_TRANS_TABLE_BASE_REG +
> - num * PCIE_ATR_TLB_SET_OFFSET;
> + table = pcie->base + PCIE_TRANS_TABLE_BASE_REG + *num *
> PCIE_ATR_TLB_SET_OFFSET;
> + writel_relaxed(lower_32_bits(cpu_addr) |
> PCIE_ATR_SIZE(fls(table_size) - 1), table);
> + writel_relaxed(upper_32_bits(cpu_addr), table +
> PCIE_ATR_SRC_ADDR_MSB_OFFSET);
> + writel_relaxed(lower_32_bits(pci_addr), table +
> PCIE_ATR_TRSL_ADDR_LSB_OFFSET);
> + writel_relaxed(upper_32_bits(pci_addr), table +
> PCIE_ATR_TRSL_ADDR_MSB_OFFSET);
>
> - writel_relaxed(lower_32_bits(cpu_addr) |
> PCIE_ATR_SIZE(fls(size) - 1),
> - table);
> - writel_relaxed(upper_32_bits(cpu_addr),
> - table + PCIE_ATR_SRC_ADDR_MSB_OFFSET);
> - writel_relaxed(lower_32_bits(pci_addr),
> - table + PCIE_ATR_TRSL_ADDR_LSB_OFFSET);
> - writel_relaxed(upper_32_bits(pci_addr),
> - table + PCIE_ATR_TRSL_ADDR_MSB_OFFSET);
> + if (type == IORESOURCE_IO) {
> + val = PCIE_ATR_TYPE_IO | PCIE_ATR_TLP_TYPE_IO;
> + range_type = "IO";
> + } else {
> + val = PCIE_ATR_TYPE_MEM |
> PCIE_ATR_TLP_TYPE_MEM;
> + range_type = "MEM";
> + }
>
> - if (type == IORESOURCE_IO)
> - val = PCIE_ATR_TYPE_IO | PCIE_ATR_TLP_TYPE_IO;
> - else
> - val = PCIE_ATR_TYPE_MEM | PCIE_ATR_TLP_TYPE_MEM;
> + writel_relaxed(val, table +
> PCIE_ATR_TRSL_PARAM_OFFSET);
>
> - writel_relaxed(val, table + PCIE_ATR_TRSL_PARAM_OFFSET);
> + dev_dbg(pcie->dev, "set %s trans window[%d]: cpu_addr =
> %#llx, pci_addr = %#llx, size = %#llx\n",
> + range_type, *num, (unsigned long long)cpu_addr,
> + (unsigned long long)pci_addr, (unsigned long
> long)table_size);
> +
> + cpu_addr += table_size;
> + pci_addr += table_size;
> + remaining -= table_size;
> + (*num)++;
> + }
> +
> + if (remaining)
> + dev_warn(pcie->dev, "not enough translate table for
> addr: %#llx, limited to [%d]\n",
> + (unsigned long long)cpu_addr,
> PCIE_MAX_TRANS_TABLES);
>
> return 0;
> }
> @@ -380,30 +405,20 @@ static int mtk_pcie_startup_port(struct
> mtk_gen3_pcie *pcie)
> resource_size_t cpu_addr;
> resource_size_t pci_addr;
> resource_size_t size;
> - const char *range_type;
>
> - if (type == IORESOURCE_IO) {
> + if (type == IORESOURCE_IO)
> cpu_addr = pci_pio_to_address(res->start);
> - range_type = "IO";
> - } else if (type == IORESOURCE_MEM) {
> + else if (type == IORESOURCE_MEM)
> cpu_addr = res->start;
> - range_type = "MEM";
> - } else {
> + else
> continue;
> - }
>
> pci_addr = res->start - entry->offset;
> size = resource_size(res);
> err = mtk_pcie_set_trans_table(pcie, cpu_addr,
> pci_addr, size,
> - type, table_index);
> + type, &table_index);
> if (err)
> return err;
> -
> - dev_dbg(pcie->dev, "set %s trans window[%d]: cpu_addr =
> %#llx, pci_addr = %#llx, size = %#llx\n",
> - range_type, table_index, (unsigned long
> long)cpu_addr,
> - (unsigned long long)pci_addr, (unsigned long
> long)size);
> -
> - table_index++;
> }
>
> return 0;
