| Message ID | 1596795922-705-6-git-send-email-hayashi.kunihiko@socionext.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v6,1/6] PCI: portdrv: Add pcie_port_service_get_irq() function | expand |
On Fri, Aug 7, 2020 at 4:25 AM Kunihiko Hayashi <hayashi.kunihiko@socionext.com> wrote: > > This gets iATU register area from reg property. In Synopsys DWC version > 4.80 or later, since iATU register area is separated from core register > area, this area is necessary to get from DT independently. > > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> > --- > drivers/pci/controller/dwc/pcie-uniphier.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c > index 55a7166..93ef608 100644 > --- a/drivers/pci/controller/dwc/pcie-uniphier.c > +++ b/drivers/pci/controller/dwc/pcie-uniphier.c > @@ -471,6 +471,11 @@ static int uniphier_pcie_probe(struct platform_device *pdev) > if (IS_ERR(priv->pci.dbi_base)) > return PTR_ERR(priv->pci.dbi_base); > > + priv->pci.atu_base = > + devm_platform_ioremap_resource_byname(pdev, "atu"); > + if (IS_ERR(priv->pci.atu_base)) > + priv->pci.atu_base = NULL; Keystone has the same 'atu' resource setup. Please move its code to the DW core and use that. Rob
On 2020/08/18 1:48, Rob Herring wrote: > On Fri, Aug 7, 2020 at 4:25 AM Kunihiko Hayashi > <hayashi.kunihiko@socionext.com> wrote: >> >> This gets iATU register area from reg property. In Synopsys DWC version >> 4.80 or later, since iATU register area is separated from core register >> area, this area is necessary to get from DT independently. >> >> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> >> --- >> drivers/pci/controller/dwc/pcie-uniphier.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c >> index 55a7166..93ef608 100644 >> --- a/drivers/pci/controller/dwc/pcie-uniphier.c >> +++ b/drivers/pci/controller/dwc/pcie-uniphier.c >> @@ -471,6 +471,11 @@ static int uniphier_pcie_probe(struct platform_device *pdev) >> if (IS_ERR(priv->pci.dbi_base)) >> return PTR_ERR(priv->pci.dbi_base); >> >> + priv->pci.atu_base = >> + devm_platform_ioremap_resource_byname(pdev, "atu"); >> + if (IS_ERR(priv->pci.atu_base)) >> + priv->pci.atu_base = NULL; > > Keystone has the same 'atu' resource setup. Please move its code to > the DW core and use that. There are some platforms that pci.atu_base is set by other way. The 'atu' code shouldn't be conflicted with the following existing code. drivers/pci/controller/dwc/pci-keystone.c: atu_base = devm_platform_ioremap_resource_byname(pdev, "atu"); drivers/pci/controller/dwc/pci-keystone.c: pci->atu_base = atu_base; drivers/pci/controller/dwc/pcie-designware.c: pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET; drivers/pci/controller/dwc/pcie-intel-gw.c: pci->atu_base = pci->dbi_base + data->pcie_atu_offset; drivers/pci/controller/dwc/pcie-tegra194.c: pci->atu_base = devm_ioremap_resource(dev, atu_dma_res); So I'm not sure where to move the code in the DW core. Is there any idea? Thank you, --- Best Regards Kunihiko Hayashi
On Fri, Aug 21, 2020 at 1:05 AM Kunihiko Hayashi <hayashi.kunihiko@socionext.com> wrote: > > On 2020/08/18 1:48, Rob Herring wrote: > > On Fri, Aug 7, 2020 at 4:25 AM Kunihiko Hayashi > > <hayashi.kunihiko@socionext.com> wrote: > >> > >> This gets iATU register area from reg property. In Synopsys DWC version > >> 4.80 or later, since iATU register area is separated from core register > >> area, this area is necessary to get from DT independently. > >> > >> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> > >> --- > >> drivers/pci/controller/dwc/pcie-uniphier.c | 5 +++++ > >> 1 file changed, 5 insertions(+) > >> > >> diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c > >> index 55a7166..93ef608 100644 > >> --- a/drivers/pci/controller/dwc/pcie-uniphier.c > >> +++ b/drivers/pci/controller/dwc/pcie-uniphier.c > >> @@ -471,6 +471,11 @@ static int uniphier_pcie_probe(struct platform_device *pdev) > >> if (IS_ERR(priv->pci.dbi_base)) > >> return PTR_ERR(priv->pci.dbi_base); > >> > >> + priv->pci.atu_base = > >> + devm_platform_ioremap_resource_byname(pdev, "atu"); > >> + if (IS_ERR(priv->pci.atu_base)) > >> + priv->pci.atu_base = NULL; > > > > Keystone has the same 'atu' resource setup. Please move its code to > > the DW core and use that. > > There are some platforms that pci.atu_base is set by other way. > The 'atu' code shouldn't be conflicted with the following existing code. No, it's not a conflict but needless duplication. > drivers/pci/controller/dwc/pci-keystone.c: atu_base = devm_platform_ioremap_resource_byname(pdev, "atu"); > drivers/pci/controller/dwc/pci-keystone.c: pci->atu_base = atu_base; > drivers/pci/controller/dwc/pcie-designware.c: pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET; > drivers/pci/controller/dwc/pcie-intel-gw.c: pci->atu_base = pci->dbi_base + data->pcie_atu_offset; This one should have had an 'atu' region in DT. > drivers/pci/controller/dwc/pcie-tegra194.c: pci->atu_base = devm_ioremap_resource(dev, atu_dma_res); Unfortunately, a different name was used. That is the mess which is the DW PCI controller. > > So I'm not sure where to move the code in the DW core. > Is there any idea? You just need this and then remove the keystone code: diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c index b723e0cc41fb..680084467447 100644 --- a/drivers/pci/controller/dwc/pcie-designware.c +++ b/drivers/pci/controller/dwc/pcie-designware.c @@ -556,6 +556,8 @@ void dw_pcie_setup(struct dw_pcie *pci) dw_pcie_iatu_unroll_enabled(pci))) { pci->iatu_unroll_enabled = true; if (!pci->atu_base) + pci->atu_base = devm_platform_ioremap_resource_byname(pdev, "atu"); + if (IS_ERR(pci->atu_base)) pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET; } dev_dbg(pci->dev, "iATU unroll: %s\n", pci->iatu_unroll_enabled ?
Hi Rob, On 2020/09/04 7:12, Rob Herring wrote: > On Fri, Aug 21, 2020 at 1:05 AM Kunihiko Hayashi > <hayashi.kunihiko@socionext.com> wrote: >> >> On 2020/08/18 1:48, Rob Herring wrote: >>> On Fri, Aug 7, 2020 at 4:25 AM Kunihiko Hayashi >>> <hayashi.kunihiko@socionext.com> wrote: >>>> >>>> This gets iATU register area from reg property. In Synopsys DWC version >>>> 4.80 or later, since iATU register area is separated from core register >>>> area, this area is necessary to get from DT independently. >>>> >>>> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> >>>> --- >>>> drivers/pci/controller/dwc/pcie-uniphier.c | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>>> >>>> diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c >>>> index 55a7166..93ef608 100644 >>>> --- a/drivers/pci/controller/dwc/pcie-uniphier.c >>>> +++ b/drivers/pci/controller/dwc/pcie-uniphier.c >>>> @@ -471,6 +471,11 @@ static int uniphier_pcie_probe(struct platform_device *pdev) >>>> if (IS_ERR(priv->pci.dbi_base)) >>>> return PTR_ERR(priv->pci.dbi_base); >>>> >>>> + priv->pci.atu_base = >>>> + devm_platform_ioremap_resource_byname(pdev, "atu"); >>>> + if (IS_ERR(priv->pci.atu_base)) >>>> + priv->pci.atu_base = NULL; >>> >>> Keystone has the same 'atu' resource setup. Please move its code to >>> the DW core and use that. >> >> There are some platforms that pci.atu_base is set by other way. >> The 'atu' code shouldn't be conflicted with the following existing code. > > No, it's not a conflict but needless duplication. I see. >> drivers/pci/controller/dwc/pci-keystone.c: atu_base = devm_platform_ioremap_resource_byname(pdev, "atu"); >> drivers/pci/controller/dwc/pci-keystone.c: pci->atu_base = atu_base; >> drivers/pci/controller/dwc/pcie-designware.c: pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET; >> drivers/pci/controller/dwc/pcie-intel-gw.c: pci->atu_base = pci->dbi_base + data->pcie_atu_offset; > > This one should have had an 'atu' region in DT. > >> drivers/pci/controller/dwc/pcie-tegra194.c: pci->atu_base = devm_ioremap_resource(dev, atu_dma_res); > > Unfortunately, a different name was used. That is the mess which is > the DW PCI controller. Okay, this has already set atu_base, so ignore it for this patch. >> >> So I'm not sure where to move the code in the DW core. >> Is there any idea? > > You just need this and then remove the keystone code: > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c > b/drivers/pci/controller/dwc/pcie-designware.c > index b723e0cc41fb..680084467447 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.c > +++ b/drivers/pci/controller/dwc/pcie-designware.c > @@ -556,6 +556,8 @@ void dw_pcie_setup(struct dw_pcie *pci) > dw_pcie_iatu_unroll_enabled(pci))) { > pci->iatu_unroll_enabled = true; > if (!pci->atu_base) > + pci->atu_base = > devm_platform_ioremap_resource_byname(pdev, "atu"); > + if (IS_ERR(pci->atu_base)) > pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET; > } > dev_dbg(pci->dev, "iATU unroll: %s\n", pci->iatu_unroll_enabled ? > I've got it. I think I need to add a bit of code to this though, I'll try to apply this and remove the duplicate code. I'll separate this from other PER/AER patches and send new series. Thank you, --- Best Regards Kunihiko Hayashi
diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c index 55a7166..93ef608 100644 --- a/drivers/pci/controller/dwc/pcie-uniphier.c +++ b/drivers/pci/controller/dwc/pcie-uniphier.c @@ -471,6 +471,11 @@ static int uniphier_pcie_probe(struct platform_device *pdev) if (IS_ERR(priv->pci.dbi_base)) return PTR_ERR(priv->pci.dbi_base); + priv->pci.atu_base = + devm_platform_ioremap_resource_byname(pdev, "atu"); + if (IS_ERR(priv->pci.atu_base)) + priv->pci.atu_base = NULL; + priv->base = devm_platform_ioremap_resource_byname(pdev, "link"); if (IS_ERR(priv->base)) return PTR_ERR(priv->base);
This gets iATU register area from reg property. In Synopsys DWC version 4.80 or later, since iATU register area is separated from core register area, this area is necessary to get from DT independently. Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> --- drivers/pci/controller/dwc/pcie-uniphier.c | 5 +++++ 1 file changed, 5 insertions(+)