| Message ID | 20230313124016.17102-9-enachman@marvell.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | PCI: dwc: Add support for Marvell AC5 SoC | expand |
[+cc Serge, who has done most of the recent work in this file] On Mon, Mar 13, 2023 at 02:40:16PM +0200, Elad Nachman wrote: > From: Elad Nachman <enachman@marvell.com> > > Allow dts override of region limit for SOCs with older Synopsis > Designware PCIe IP but with greater than 32-bit address range support, > such as the Armada 7020/7040/8040 family of SOCs by Marvell, > when the DT file places the PCIe window above the 4GB region. > The Synopsis Designware PCIe IP in these SOCs is too old to specify the > highest memory location supported by the PCIe, but practically supports > such locations. Allow these locations to be specified in the DT file. > DT property is called num-regionmask , and can range between 33 and 64. s/Synopsis/Synopsys/ (several occurrences) s/Designware/DesignWare/ (several occurrences) Remove space before comma. > Signed-off-by: Elad Nachman <enachman@marvell.com> > --- > v4: > 1) Fix blank lines removal / addition > > 2) Remove usage of variable with same name as dt binding property > > drivers/pci/controller/dwc/pcie-designware.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c > index 53a16b8b6ac2..9773c110c733 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.c > +++ b/drivers/pci/controller/dwc/pcie-designware.c > @@ -735,8 +735,10 @@ static void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen) > void dw_pcie_iatu_detect(struct dw_pcie *pci) > { > int max_region, ob, ib; > - u32 val, min, dir; > + u32 val, min, dir, ret; > u64 max; > + struct device *dev = pci->dev; > + struct device_node *np = dev->of_node; > > val = dw_pcie_readl_dbi(pci, PCIE_ATU_VIEWPORT); > if (val == 0xFFFFFFFF) { > @@ -781,7 +783,13 @@ void dw_pcie_iatu_detect(struct dw_pcie *pci) > dw_pcie_writel_atu(pci, dir, 0, PCIE_ATU_UPPER_LIMIT, 0xFFFFFFFF); > max = dw_pcie_readl_atu(pci, dir, 0, PCIE_ATU_UPPER_LIMIT); > } else { > - max = 0; > + /* Allow dts override of region limit for older IP with above 32-bit support: */ Reflow comment to fit in 80 columns. > + ret = of_property_read_u32(np, "num-regionmask", &val); > + if (!ret && val > 32) { > + max = GENMASK(val - 33, 0); > + dev_info(pci->dev, "Overriding region limit to %u bits\n", val); > + } else > + max = 0; > } > > pci->num_ob_windows = ob; > -- > 2.17.1 >
Hi Bjorn On Mon, Mar 13, 2023 at 02:48:02PM -0500, Bjorn Helgaas wrote: > [+cc Serge, who has done most of the recent work in this file] > Thanks for sending copy to me. I'll have a look at the series on this week. -Serge(y) > On Mon, Mar 13, 2023 at 02:40:16PM +0200, Elad Nachman wrote: > > From: Elad Nachman <enachman@marvell.com> > > > > Allow dts override of region limit for SOCs with older Synopsis > > Designware PCIe IP but with greater than 32-bit address range support, > > such as the Armada 7020/7040/8040 family of SOCs by Marvell, > > when the DT file places the PCIe window above the 4GB region. > > The Synopsis Designware PCIe IP in these SOCs is too old to specify the > > highest memory location supported by the PCIe, but practically supports > > such locations. Allow these locations to be specified in the DT file. > > DT property is called num-regionmask , and can range between 33 and 64. > > s/Synopsis/Synopsys/ (several occurrences) > > s/Designware/DesignWare/ (several occurrences) > > Remove space before comma. > > > Signed-off-by: Elad Nachman <enachman@marvell.com> > > --- > > v4: > > 1) Fix blank lines removal / addition > > > > 2) Remove usage of variable with same name as dt binding property > > > > drivers/pci/controller/dwc/pcie-designware.c | 12 ++++++++++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c > > index 53a16b8b6ac2..9773c110c733 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware.c > > +++ b/drivers/pci/controller/dwc/pcie-designware.c > > @@ -735,8 +735,10 @@ static void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen) > > void dw_pcie_iatu_detect(struct dw_pcie *pci) > > { > > int max_region, ob, ib; > > - u32 val, min, dir; > > + u32 val, min, dir, ret; > > u64 max; > > + struct device *dev = pci->dev; > > + struct device_node *np = dev->of_node; > > > > val = dw_pcie_readl_dbi(pci, PCIE_ATU_VIEWPORT); > > if (val == 0xFFFFFFFF) { > > @@ -781,7 +783,13 @@ void dw_pcie_iatu_detect(struct dw_pcie *pci) > > dw_pcie_writel_atu(pci, dir, 0, PCIE_ATU_UPPER_LIMIT, 0xFFFFFFFF); > > max = dw_pcie_readl_atu(pci, dir, 0, PCIE_ATU_UPPER_LIMIT); > > } else { > > - max = 0; > > + /* Allow dts override of region limit for older IP with above 32-bit support: */ > > Reflow comment to fit in 80 columns. > > > + ret = of_property_read_u32(np, "num-regionmask", &val); > > + if (!ret && val > 32) { > > + max = GENMASK(val - 33, 0); > > + dev_info(pci->dev, "Overriding region limit to %u bits\n", val); > > + } else > > + max = 0; > > } > > > > pci->num_ob_windows = ob; > > -- > > 2.17.1 > > >
On Mon, Mar 13, 2023 at 02:40:16PM +0200, Elad Nachman wrote: > From: Elad Nachman <enachman@marvell.com> > > Allow dts override of region limit for SOCs with older Synopsis > Designware PCIe IP but with greater than 32-bit address range support, > such as the Armada 7020/7040/8040 family of SOCs by Marvell, > when the DT file places the PCIe window above the 4GB region. > The Synopsis Designware PCIe IP in these SOCs is too old to specify the > highest memory location supported by the PCIe, but practically supports > such locations. Allow these locations to be specified in the DT file. > DT property is called num-regionmask , and can range between 33 and 64. The implemented algorithm doesn't prevents you from specifying the outbound MW base above 4GB. It prevents you from overflowing the limit address which is of the 32-bits width only for the chips older v4.60a and if the INCREASE_REGION_SIZE IP-core synthesize parameter is set to zero. In other words you must make sure that dma-ranges/ranges entries size when combined with the source address doesn't cause the limit address overflow (4GB boundary cross in your case). For instance, you want to map 0x1F0000000 CPU-address region of 512MB size to 0x0 PCIe address. In that case you'd specify the ranges property like this: < ranges = <0x82000000 0 0 0x1 0xf0000000 0 0x20000000>; The CPU-base address is ok since iATU always supports 64-bit base addresses. But after you add 0x20000000 to 0x1f0000000 you'll get the 4GB boundary overflow (0x210000000) which can't be described with the 32-bit limit address CSR. In this particular case the maximum range size you can specify is 0x10000000 (256MB). Anyway judging by the patch content you do nothing but hacking the ranges entries sanity check procedure which I don't think is a good thing to do. -Serge(y) > > Signed-off-by: Elad Nachman <enachman@marvell.com> > --- > v4: > 1) Fix blank lines removal / addition > > 2) Remove usage of variable with same name as dt binding property > > drivers/pci/controller/dwc/pcie-designware.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c > index 53a16b8b6ac2..9773c110c733 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.c > +++ b/drivers/pci/controller/dwc/pcie-designware.c > @@ -735,8 +735,10 @@ static void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen) > void dw_pcie_iatu_detect(struct dw_pcie *pci) > { > int max_region, ob, ib; > - u32 val, min, dir; > + u32 val, min, dir, ret; > u64 max; > + struct device *dev = pci->dev; > + struct device_node *np = dev->of_node; > > val = dw_pcie_readl_dbi(pci, PCIE_ATU_VIEWPORT); > if (val == 0xFFFFFFFF) { > @@ -781,7 +783,13 @@ void dw_pcie_iatu_detect(struct dw_pcie *pci) > dw_pcie_writel_atu(pci, dir, 0, PCIE_ATU_UPPER_LIMIT, 0xFFFFFFFF); > max = dw_pcie_readl_atu(pci, dir, 0, PCIE_ATU_UPPER_LIMIT); > } else { > - max = 0; > + /* Allow dts override of region limit for older IP with above 32-bit support: */ > + ret = of_property_read_u32(np, "num-regionmask", &val); > + if (!ret && val > 32) { > + max = GENMASK(val - 33, 0); > + dev_info(pci->dev, "Overriding region limit to %u bits\n", val); > + } else > + max = 0; > } > > pci->num_ob_windows = ob; > -- > 2.17.1 > >
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c index 53a16b8b6ac2..9773c110c733 100644 --- a/drivers/pci/controller/dwc/pcie-designware.c +++ b/drivers/pci/controller/dwc/pcie-designware.c @@ -735,8 +735,10 @@ static void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen) void dw_pcie_iatu_detect(struct dw_pcie *pci) { int max_region, ob, ib; - u32 val, min, dir; + u32 val, min, dir, ret; u64 max; + struct device *dev = pci->dev; + struct device_node *np = dev->of_node; val = dw_pcie_readl_dbi(pci, PCIE_ATU_VIEWPORT); if (val == 0xFFFFFFFF) { @@ -781,7 +783,13 @@ void dw_pcie_iatu_detect(struct dw_pcie *pci) dw_pcie_writel_atu(pci, dir, 0, PCIE_ATU_UPPER_LIMIT, 0xFFFFFFFF); max = dw_pcie_readl_atu(pci, dir, 0, PCIE_ATU_UPPER_LIMIT); } else { - max = 0; + /* Allow dts override of region limit for older IP with above 32-bit support: */ + ret = of_property_read_u32(np, "num-regionmask", &val); + if (!ret && val > 32) { + max = GENMASK(val - 33, 0); + dev_info(pci->dev, "Overriding region limit to %u bits\n", val); + } else + max = 0; } pci->num_ob_windows = ob;