Hi Bjorn, Lorenzo,
Sorry to bother you, but would you mind taking a look at this patch?
Appreciate any suggestions.
Thanks.
On Thu, 2023-11-23 at 01:27 +0000, Jianjun Wang (王建军) wrote:
> 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;
