| Message ID | 20210125044803.4310-1-Zhiqiang.Hou@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: dwc: Move forward the iATU detection process | expand |
Hi, This looks to me the same fix as my posted patch[1]. Is this more effective than mine? Thank you, [1] https://www.spinics.net/lists/linux-pci/msg103889.html On 2021/01/25 13:48, Zhiqiang Hou wrote: > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > In the dw_pcie_ep_init(), it depends on the detected iATU region > numbers to allocate the in/outbound window management bit map. > It fails after the commit 281f1f99cf3a ("PCI: dwc: Detect number > of iATU windows"). > > So this patch move the iATU region detection into a new function, > move forward the detection to the very beginning of functions > dw_pcie_host_init() and dw_pcie_ep_init(). And also remove it > from the dw_pcie_setup(), since it's more like a software > perspective initialization step than hardware setup. > > Fixes: 281f1f99cf3a ("PCI: dwc: Detect number of iATU windows") > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > --- > drivers/pci/controller/dwc/pcie-designware-ep.c | 2 ++ > drivers/pci/controller/dwc/pcie-designware-host.c | 2 ++ > drivers/pci/controller/dwc/pcie-designware.c | 11 ++++++++--- > drivers/pci/controller/dwc/pcie-designware.h | 1 + > 4 files changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index bcd1cd9ba8c8..fcf935bf6f5e 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -707,6 +707,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > } > } > > + dw_pcie_iatu_detect(pci); > + > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space"); > if (!res) > return -EINVAL; > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index 8a84c005f32b..8eae817c138d 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -316,6 +316,8 @@ int dw_pcie_host_init(struct pcie_port *pp) > return PTR_ERR(pci->dbi_base); > } > > + dw_pcie_iatu_detect(pci); > + > bridge = devm_pci_alloc_host_bridge(dev, 0); > if (!bridge) > return -ENOMEM; > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c > index 5b72a5448d2e..5b9bf02d918b 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.c > +++ b/drivers/pci/controller/dwc/pcie-designware.c > @@ -654,11 +654,9 @@ static void dw_pcie_iatu_detect_regions(struct dw_pcie *pci) > pci->num_ob_windows = ob; > } > > -void dw_pcie_setup(struct dw_pcie *pci) > +void dw_pcie_iatu_detect(struct dw_pcie *pci) > { > - u32 val; > struct device *dev = pci->dev; > - struct device_node *np = dev->of_node; > struct platform_device *pdev = to_platform_device(dev); > > if (pci->version >= 0x480A || (!pci->version && > @@ -687,6 +685,13 @@ void dw_pcie_setup(struct dw_pcie *pci) > > dev_info(pci->dev, "Detected iATU regions: %u outbound, %u inbound", > pci->num_ob_windows, pci->num_ib_windows); > +} > + > +void dw_pcie_setup(struct dw_pcie *pci) > +{ > + u32 val; > + struct device *dev = pci->dev; > + struct device_node *np = dev->of_node; > > if (pci->link_gen > 0) > dw_pcie_link_set_max_speed(pci, pci->link_gen); > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index 5d979953800d..867369d4c4f7 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -305,6 +305,7 @@ int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no, int index, > void dw_pcie_disable_atu(struct dw_pcie *pci, int index, > enum dw_pcie_region_type type); > void dw_pcie_setup(struct dw_pcie *pci); > +void dw_pcie_iatu_detect(struct dw_pcie *pci); > > static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val) > { > --- Best Regards Kunihiko Hayashi
Hi, Yes, they are fix the same issue. Rob and other contributors sent so many patches to refine the drivers and make the code brief and more readable, so I don't think we should just focus on the fixes of this issue. I don't think it is a good choice that your patch move some of the software perspective initializations into hardware ones. Thanks Zhiqiang > -----Original Message----- > From: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> > Sent: 2021年1月26日 13:26 > To: Z.q. Hou <zhiqiang.hou@nxp.com> > Cc: linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; > lorenzo.pieralisi@arm.com; robh@kernel.org; bhelgaas@google.com; > gustavo.pimentel@synopsys.com; jingoohan1@gmail.com; > jaswinder.singh@linaro.org; masami.hiramatsu@linaro.org > Subject: Re: [PATCH] PCI: dwc: Move forward the iATU detection process > > Hi, > > This looks to me the same fix as my posted patch[1]. > Is this more effective than mine? > > Thank you, > > [1] > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww > .spinics.net%2Flists%2Flinux-pci%2Fmsg103889.html&data=04%7C01% > 7CZhiqiang.Hou%40nxp.com%7Cd9fa58aac4774c9dd61b08d8c1bad128%7C > 686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637472355412202563 > %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiL > CJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Mt3B4jQ1Q1fu%2 > FAz9s4Y4eieHv7nYorvvT2pKlqFLE9k%3D&reserved=0 > > On 2021/01/25 13:48, Zhiqiang Hou wrote: > > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > > > In the dw_pcie_ep_init(), it depends on the detected iATU region > > numbers to allocate the in/outbound window management bit map. > > It fails after the commit 281f1f99cf3a ("PCI: dwc: Detect number of > > iATU windows"). > > > > So this patch move the iATU region detection into a new function, move > > forward the detection to the very beginning of functions > > dw_pcie_host_init() and dw_pcie_ep_init(). And also remove it from the > > dw_pcie_setup(), since it's more like a software perspective > > initialization step than hardware setup. > > > > Fixes: 281f1f99cf3a ("PCI: dwc: Detect number of iATU windows") > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > --- > > drivers/pci/controller/dwc/pcie-designware-ep.c | 2 ++ > > drivers/pci/controller/dwc/pcie-designware-host.c | 2 ++ > > drivers/pci/controller/dwc/pcie-designware.c | 11 ++++++++--- > > drivers/pci/controller/dwc/pcie-designware.h | 1 + > > 4 files changed, 13 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c > > b/drivers/pci/controller/dwc/pcie-designware-ep.c > > index bcd1cd9ba8c8..fcf935bf6f5e 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > > @@ -707,6 +707,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > > } > > } > > > > + dw_pcie_iatu_detect(pci); > > + > > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, > "addr_space"); > > if (!res) > > return -EINVAL; > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c > > b/drivers/pci/controller/dwc/pcie-designware-host.c > > index 8a84c005f32b..8eae817c138d 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > > @@ -316,6 +316,8 @@ int dw_pcie_host_init(struct pcie_port *pp) > > return PTR_ERR(pci->dbi_base); > > } > > > > + dw_pcie_iatu_detect(pci); > > + > > bridge = devm_pci_alloc_host_bridge(dev, 0); > > if (!bridge) > > return -ENOMEM; > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c > > b/drivers/pci/controller/dwc/pcie-designware.c > > index 5b72a5448d2e..5b9bf02d918b 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware.c > > +++ b/drivers/pci/controller/dwc/pcie-designware.c > > @@ -654,11 +654,9 @@ static void dw_pcie_iatu_detect_regions(struct > dw_pcie *pci) > > pci->num_ob_windows = ob; > > } > > > > -void dw_pcie_setup(struct dw_pcie *pci) > > +void dw_pcie_iatu_detect(struct dw_pcie *pci) > > { > > - u32 val; > > struct device *dev = pci->dev; > > - struct device_node *np = dev->of_node; > > struct platform_device *pdev = to_platform_device(dev); > > > > if (pci->version >= 0x480A || (!pci->version && @@ -687,6 +685,13 > > @@ void dw_pcie_setup(struct dw_pcie *pci) > > > > dev_info(pci->dev, "Detected iATU regions: %u outbound, %u > inbound", > > pci->num_ob_windows, pci->num_ib_windows); > > +} > > + > > +void dw_pcie_setup(struct dw_pcie *pci) { > > + u32 val; > > + struct device *dev = pci->dev; > > + struct device_node *np = dev->of_node; > > > > if (pci->link_gen > 0) > > dw_pcie_link_set_max_speed(pci, pci->link_gen); diff --git > > a/drivers/pci/controller/dwc/pcie-designware.h > > b/drivers/pci/controller/dwc/pcie-designware.h > > index 5d979953800d..867369d4c4f7 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware.h > > +++ b/drivers/pci/controller/dwc/pcie-designware.h > > @@ -305,6 +305,7 @@ int dw_pcie_prog_inbound_atu(struct dw_pcie > *pci, u8 func_no, int index, > > void dw_pcie_disable_atu(struct dw_pcie *pci, int index, > > enum dw_pcie_region_type type); > > void dw_pcie_setup(struct dw_pcie *pci); > > +void dw_pcie_iatu_detect(struct dw_pcie *pci); > > > > static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 > val) > > { > > > > --- > Best Regards > Kunihiko Hayashi
On 2021/01/27 15:27, Z.q. Hou wrote: > Hi, > > Yes, they are fix the same issue. > Rob and other contributors sent so many patches to refine the drivers and make the code brief and more readable, so I don't think we should just focus on the fixes of this issue. I don't think it is a good choice that your patch move some of the software perspective initializations into hardware ones. Ok, I checked your patch fixed this issue on my board with or without my patch. I'll follow the maintainers for handling my patch. Tested-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> Thank you, > > Thanks > Zhiqiang > >> -----Original Message----- >> From: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> >> Sent: 2021年1月26日 13:26 >> To: Z.q. Hou <zhiqiang.hou@nxp.com> >> Cc: linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; >> lorenzo.pieralisi@arm.com; robh@kernel.org; bhelgaas@google.com; >> gustavo.pimentel@synopsys.com; jingoohan1@gmail.com; >> jaswinder.singh@linaro.org; masami.hiramatsu@linaro.org >> Subject: Re: [PATCH] PCI: dwc: Move forward the iATU detection process >> >> Hi, >> >> This looks to me the same fix as my posted patch[1]. >> Is this more effective than mine? >> >> Thank you, >> >> [1] >> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww >> .spinics.net%2Flists%2Flinux-pci%2Fmsg103889.html&data=04%7C01% >> 7CZhiqiang.Hou%40nxp.com%7Cd9fa58aac4774c9dd61b08d8c1bad128%7C >> 686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637472355412202563 >> %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiL >> CJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Mt3B4jQ1Q1fu%2 >> FAz9s4Y4eieHv7nYorvvT2pKlqFLE9k%3D&reserved=0 >> >> On 2021/01/25 13:48, Zhiqiang Hou wrote: >>> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> >>> >>> In the dw_pcie_ep_init(), it depends on the detected iATU region >>> numbers to allocate the in/outbound window management bit map. >>> It fails after the commit 281f1f99cf3a ("PCI: dwc: Detect number of >>> iATU windows"). >>> >>> So this patch move the iATU region detection into a new function, move >>> forward the detection to the very beginning of functions >>> dw_pcie_host_init() and dw_pcie_ep_init(). And also remove it from the >>> dw_pcie_setup(), since it's more like a software perspective >>> initialization step than hardware setup. >>> >>> Fixes: 281f1f99cf3a ("PCI: dwc: Detect number of iATU windows") >>> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> >>> --- >>> drivers/pci/controller/dwc/pcie-designware-ep.c | 2 ++ >>> drivers/pci/controller/dwc/pcie-designware-host.c | 2 ++ >>> drivers/pci/controller/dwc/pcie-designware.c | 11 ++++++++--- >>> drivers/pci/controller/dwc/pcie-designware.h | 1 + >>> 4 files changed, 13 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c >>> b/drivers/pci/controller/dwc/pcie-designware-ep.c >>> index bcd1cd9ba8c8..fcf935bf6f5e 100644 >>> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c >>> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c >>> @@ -707,6 +707,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) >>> } >>> } >>> >>> + dw_pcie_iatu_detect(pci); >>> + >>> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, >> "addr_space"); >>> if (!res) >>> return -EINVAL; >>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c >>> b/drivers/pci/controller/dwc/pcie-designware-host.c >>> index 8a84c005f32b..8eae817c138d 100644 >>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c >>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c >>> @@ -316,6 +316,8 @@ int dw_pcie_host_init(struct pcie_port *pp) >>> return PTR_ERR(pci->dbi_base); >>> } >>> >>> + dw_pcie_iatu_detect(pci); >>> + >>> bridge = devm_pci_alloc_host_bridge(dev, 0); >>> if (!bridge) >>> return -ENOMEM; >>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c >>> b/drivers/pci/controller/dwc/pcie-designware.c >>> index 5b72a5448d2e..5b9bf02d918b 100644 >>> --- a/drivers/pci/controller/dwc/pcie-designware.c >>> +++ b/drivers/pci/controller/dwc/pcie-designware.c >>> @@ -654,11 +654,9 @@ static void dw_pcie_iatu_detect_regions(struct >> dw_pcie *pci) >>> pci->num_ob_windows = ob; >>> } >>> >>> -void dw_pcie_setup(struct dw_pcie *pci) >>> +void dw_pcie_iatu_detect(struct dw_pcie *pci) >>> { >>> - u32 val; >>> struct device *dev = pci->dev; >>> - struct device_node *np = dev->of_node; >>> struct platform_device *pdev = to_platform_device(dev); >>> >>> if (pci->version >= 0x480A || (!pci->version && @@ -687,6 +685,13 >>> @@ void dw_pcie_setup(struct dw_pcie *pci) >>> >>> dev_info(pci->dev, "Detected iATU regions: %u outbound, %u >> inbound", >>> pci->num_ob_windows, pci->num_ib_windows); >>> +} >>> + >>> +void dw_pcie_setup(struct dw_pcie *pci) { >>> + u32 val; >>> + struct device *dev = pci->dev; >>> + struct device_node *np = dev->of_node; >>> >>> if (pci->link_gen > 0) >>> dw_pcie_link_set_max_speed(pci, pci->link_gen); diff --git >>> a/drivers/pci/controller/dwc/pcie-designware.h >>> b/drivers/pci/controller/dwc/pcie-designware.h >>> index 5d979953800d..867369d4c4f7 100644 >>> --- a/drivers/pci/controller/dwc/pcie-designware.h >>> +++ b/drivers/pci/controller/dwc/pcie-designware.h >>> @@ -305,6 +305,7 @@ int dw_pcie_prog_inbound_atu(struct dw_pcie >> *pci, u8 func_no, int index, >>> void dw_pcie_disable_atu(struct dw_pcie *pci, int index, >>> enum dw_pcie_region_type type); >>> void dw_pcie_setup(struct dw_pcie *pci); >>> +void dw_pcie_iatu_detect(struct dw_pcie *pci); >>> >>> static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 >> val) >>> { >>> >> >> --- >> Best Regards >> Kunihiko Hayashi --- Best Regards Kunihiko Hayashi
On Sun, Jan 24, 2021 at 10:39 PM Zhiqiang Hou <Zhiqiang.Hou@nxp.com> wrote: > > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > In the dw_pcie_ep_init(), it depends on the detected iATU region > numbers to allocate the in/outbound window management bit map. > It fails after the commit 281f1f99cf3a ("PCI: dwc: Detect number > of iATU windows"). > > So this patch move the iATU region detection into a new function, > move forward the detection to the very beginning of functions > dw_pcie_host_init() and dw_pcie_ep_init(). And also remove it > from the dw_pcie_setup(), since it's more like a software > perspective initialization step than hardware setup. > > Fixes: 281f1f99cf3a ("PCI: dwc: Detect number of iATU windows") > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > --- > drivers/pci/controller/dwc/pcie-designware-ep.c | 2 ++ > drivers/pci/controller/dwc/pcie-designware-host.c | 2 ++ > drivers/pci/controller/dwc/pcie-designware.c | 11 ++++++++--- > drivers/pci/controller/dwc/pcie-designware.h | 1 + > 4 files changed, 13 insertions(+), 3 deletions(-) Reviewed-by: Rob Herring <robh@kernel.org> Bjorn, can you pick this up for 5.12. Lorenzo is on holiday. Rob
On Mon, 25 Jan 2021 12:48:03 +0800, Zhiqiang Hou wrote: > In the dw_pcie_ep_init(), it depends on the detected iATU region > numbers to allocate the in/outbound window management bit map. > It fails after the commit 281f1f99cf3a ("PCI: dwc: Detect number > of iATU windows"). > > So this patch move the iATU region detection into a new function, > move forward the detection to the very beginning of functions > dw_pcie_host_init() and dw_pcie_ep_init(). And also remove it > from the dw_pcie_setup(), since it's more like a software > perspective initialization step than hardware setup. Applied to pci/dwc, thanks! [1/1] PCI: dwc: Move forward the iATU detection process https://git.kernel.org/lpieralisi/pci/c/5ff8ca16f8 Thanks, Lorenzo
On Mon, Mar 22, 2021 at 06:03:57PM +0000, Lorenzo Pieralisi wrote: > On Mon, 25 Jan 2021 12:48:03 +0800, Zhiqiang Hou wrote: > > In the dw_pcie_ep_init(), it depends on the detected iATU region > > numbers to allocate the in/outbound window management bit map. > > It fails after the commit 281f1f99cf3a ("PCI: dwc: Detect number > > of iATU windows"). > > > > So this patch move the iATU region detection into a new function, > > move forward the detection to the very beginning of functions > > dw_pcie_host_init() and dw_pcie_ep_init(). And also remove it > > from the dw_pcie_setup(), since it's more like a software > > perspective initialization step than hardware setup. > > Applied to pci/dwc, thanks! > > [1/1] PCI: dwc: Move forward the iATU detection process > https://git.kernel.org/lpieralisi/pci/c/5ff8ca16f8 Bjorn will queue it for v5.12, so I have dropped it, ignore this notification. Thanks, Lorenzo
On Mon, Jan 25, 2021 at 12:48:03PM +0800, Zhiqiang Hou wrote: > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > In the dw_pcie_ep_init(), it depends on the detected iATU region > numbers to allocate the in/outbound window management bit map. > It fails after the commit 281f1f99cf3a ("PCI: dwc: Detect number > of iATU windows"). > > So this patch move the iATU region detection into a new function, > move forward the detection to the very beginning of functions > dw_pcie_host_init() and dw_pcie_ep_init(). And also remove it > from the dw_pcie_setup(), since it's more like a software > perspective initialization step than hardware setup. > > Fixes: 281f1f99cf3a ("PCI: dwc: Detect number of iATU windows") > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> Applied to for-linus for v5.12, with stable tag for v5.11, thanks! > --- > drivers/pci/controller/dwc/pcie-designware-ep.c | 2 ++ > drivers/pci/controller/dwc/pcie-designware-host.c | 2 ++ > drivers/pci/controller/dwc/pcie-designware.c | 11 ++++++++--- > drivers/pci/controller/dwc/pcie-designware.h | 1 + > 4 files changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index bcd1cd9ba8c8..fcf935bf6f5e 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -707,6 +707,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > } > } > > + dw_pcie_iatu_detect(pci); > + > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space"); > if (!res) > return -EINVAL; > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index 8a84c005f32b..8eae817c138d 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -316,6 +316,8 @@ int dw_pcie_host_init(struct pcie_port *pp) > return PTR_ERR(pci->dbi_base); > } > > + dw_pcie_iatu_detect(pci); > + > bridge = devm_pci_alloc_host_bridge(dev, 0); > if (!bridge) > return -ENOMEM; > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c > index 5b72a5448d2e..5b9bf02d918b 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.c > +++ b/drivers/pci/controller/dwc/pcie-designware.c > @@ -654,11 +654,9 @@ static void dw_pcie_iatu_detect_regions(struct dw_pcie *pci) > pci->num_ob_windows = ob; > } > > -void dw_pcie_setup(struct dw_pcie *pci) > +void dw_pcie_iatu_detect(struct dw_pcie *pci) > { > - u32 val; > struct device *dev = pci->dev; > - struct device_node *np = dev->of_node; > struct platform_device *pdev = to_platform_device(dev); > > if (pci->version >= 0x480A || (!pci->version && > @@ -687,6 +685,13 @@ void dw_pcie_setup(struct dw_pcie *pci) > > dev_info(pci->dev, "Detected iATU regions: %u outbound, %u inbound", > pci->num_ob_windows, pci->num_ib_windows); > +} > + > +void dw_pcie_setup(struct dw_pcie *pci) > +{ > + u32 val; > + struct device *dev = pci->dev; > + struct device_node *np = dev->of_node; > > if (pci->link_gen > 0) > dw_pcie_link_set_max_speed(pci, pci->link_gen); > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index 5d979953800d..867369d4c4f7 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -305,6 +305,7 @@ int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no, int index, > void dw_pcie_disable_atu(struct dw_pcie *pci, int index, > enum dw_pcie_region_type type); > void dw_pcie_setup(struct dw_pcie *pci); > +void dw_pcie_iatu_detect(struct dw_pcie *pci); > > static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val) > { > -- > 2.17.1 >
Hi, On 25.01.2021 05:48, Zhiqiang Hou wrote: > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > In the dw_pcie_ep_init(), it depends on the detected iATU region > numbers to allocate the in/outbound window management bit map. > It fails after the commit 281f1f99cf3a ("PCI: dwc: Detect number > of iATU windows"). > > So this patch move the iATU region detection into a new function, > move forward the detection to the very beginning of functions > dw_pcie_host_init() and dw_pcie_ep_init(). And also remove it > from the dw_pcie_setup(), since it's more like a software > perspective initialization step than hardware setup. > > Fixes: 281f1f99cf3a ("PCI: dwc: Detect number of iATU windows") > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> This patch causes exynos-pcie to hang during the initialization. It looks that some resources are not enabled yet, so calling dw_pcie_iatu_detect() much earlier causes a hang. When I have some time, I will try to identify what is needed to call it properly. > --- > drivers/pci/controller/dwc/pcie-designware-ep.c | 2 ++ > drivers/pci/controller/dwc/pcie-designware-host.c | 2 ++ > drivers/pci/controller/dwc/pcie-designware.c | 11 ++++++++--- > drivers/pci/controller/dwc/pcie-designware.h | 1 + > 4 files changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index bcd1cd9ba8c8..fcf935bf6f5e 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -707,6 +707,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > } > } > > + dw_pcie_iatu_detect(pci); > + > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space"); > if (!res) > return -EINVAL; > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index 8a84c005f32b..8eae817c138d 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -316,6 +316,8 @@ int dw_pcie_host_init(struct pcie_port *pp) > return PTR_ERR(pci->dbi_base); > } > > + dw_pcie_iatu_detect(pci); > + > bridge = devm_pci_alloc_host_bridge(dev, 0); > if (!bridge) > return -ENOMEM; > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c > index 5b72a5448d2e..5b9bf02d918b 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.c > +++ b/drivers/pci/controller/dwc/pcie-designware.c > @@ -654,11 +654,9 @@ static void dw_pcie_iatu_detect_regions(struct dw_pcie *pci) > pci->num_ob_windows = ob; > } > > -void dw_pcie_setup(struct dw_pcie *pci) > +void dw_pcie_iatu_detect(struct dw_pcie *pci) > { > - u32 val; > struct device *dev = pci->dev; > - struct device_node *np = dev->of_node; > struct platform_device *pdev = to_platform_device(dev); > > if (pci->version >= 0x480A || (!pci->version && > @@ -687,6 +685,13 @@ void dw_pcie_setup(struct dw_pcie *pci) > > dev_info(pci->dev, "Detected iATU regions: %u outbound, %u inbound", > pci->num_ob_windows, pci->num_ib_windows); > +} > + > +void dw_pcie_setup(struct dw_pcie *pci) > +{ > + u32 val; > + struct device *dev = pci->dev; > + struct device_node *np = dev->of_node; > > if (pci->link_gen > 0) > dw_pcie_link_set_max_speed(pci, pci->link_gen); > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index 5d979953800d..867369d4c4f7 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -305,6 +305,7 @@ int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no, int index, > void dw_pcie_disable_atu(struct dw_pcie *pci, int index, > enum dw_pcie_region_type type); > void dw_pcie_setup(struct dw_pcie *pci); > +void dw_pcie_iatu_detect(struct dw_pcie *pci); > > static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val) > { Best regards
On Thu, Mar 25, 2021 at 10:24:28AM +0100, Marek Szyprowski wrote: > On 25.01.2021 05:48, Zhiqiang Hou wrote: > > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > > > In the dw_pcie_ep_init(), it depends on the detected iATU region > > numbers to allocate the in/outbound window management bit map. > > It fails after the commit 281f1f99cf3a ("PCI: dwc: Detect number > > of iATU windows"). > > > > So this patch move the iATU region detection into a new function, > > move forward the detection to the very beginning of functions > > dw_pcie_host_init() and dw_pcie_ep_init(). And also remove it > > from the dw_pcie_setup(), since it's more like a software > > perspective initialization step than hardware setup. > > > > Fixes: 281f1f99cf3a ("PCI: dwc: Detect number of iATU windows") > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > This patch causes exynos-pcie to hang during the initialization. It > looks that some resources are not enabled yet, so calling > dw_pcie_iatu_detect() much earlier causes a hang. When I have some time, > I will try to identify what is needed to call it properly. Thanks, I dropped it for now. We can add it back after we figure out what the exynos issue is. For reference, here's the patch I dropped (I had made some minor corrections to the commit log): commit fd4162f05194 ("PCI: dwc: Move iATU detection earlier") Author: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> Date: Mon Jan 25 12:48:03 2021 +0800 PCI: dwc: Move iATU detection earlier dw_pcie_ep_init() depends on the detected iATU region numbers to allocate the in/outbound window management bitmap. It fails after 281f1f99cf3a ("PCI: dwc: Detect number of iATU windows"). Move the iATU region detection into a new function, move the detection to the very beginning of dw_pcie_host_init() and dw_pcie_ep_init(). Also remove it from the dw_pcie_setup(), since it's more like a software initialization step than hardware setup. Fixes: 281f1f99cf3a ("PCI: dwc: Detect number of iATU windows") Link: https://lore.kernel.org/r/20210125044803.4310-1-Zhiqiang.Hou@nxp.com Tested-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> Reviewed-by: Rob Herring <robh@kernel.org> Cc: stable@vger.kernel.org # v5.11+ diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c index 1c25d8337151..8d028a88b375 100644 --- a/drivers/pci/controller/dwc/pcie-designware-ep.c +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c @@ -705,6 +705,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) } } + dw_pcie_iatu_detect(pci); + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space"); if (!res) return -EINVAL; diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c index 7e55b2b66182..52f6887179cd 100644 --- a/drivers/pci/controller/dwc/pcie-designware-host.c +++ b/drivers/pci/controller/dwc/pcie-designware-host.c @@ -319,6 +319,8 @@ int dw_pcie_host_init(struct pcie_port *pp) return PTR_ERR(pci->dbi_base); } + dw_pcie_iatu_detect(pci); + bridge = devm_pci_alloc_host_bridge(dev, 0); if (!bridge) return -ENOMEM; diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c index 004cb860e266..a945f0c0e73d 100644 --- a/drivers/pci/controller/dwc/pcie-designware.c +++ b/drivers/pci/controller/dwc/pcie-designware.c @@ -660,11 +660,9 @@ static void dw_pcie_iatu_detect_regions(struct dw_pcie *pci) pci->num_ob_windows = ob; } -void dw_pcie_setup(struct dw_pcie *pci) +void dw_pcie_iatu_detect(struct dw_pcie *pci) { - u32 val; struct device *dev = pci->dev; - struct device_node *np = dev->of_node; struct platform_device *pdev = to_platform_device(dev); if (pci->version >= 0x480A || (!pci->version && @@ -693,6 +691,13 @@ void dw_pcie_setup(struct dw_pcie *pci) dev_info(pci->dev, "Detected iATU regions: %u outbound, %u inbound", pci->num_ob_windows, pci->num_ib_windows); +} + +void dw_pcie_setup(struct dw_pcie *pci) +{ + u32 val; + struct device *dev = pci->dev; + struct device_node *np = dev->of_node; if (pci->link_gen > 0) dw_pcie_link_set_max_speed(pci, pci->link_gen); diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h index 7247c8b01f04..7d6e9b7576be 100644 --- a/drivers/pci/controller/dwc/pcie-designware.h +++ b/drivers/pci/controller/dwc/pcie-designware.h @@ -306,6 +306,7 @@ int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no, int index, void dw_pcie_disable_atu(struct dw_pcie *pci, int index, enum dw_pcie_region_type type); void dw_pcie_setup(struct dw_pcie *pci); +void dw_pcie_iatu_detect(struct dw_pcie *pci); static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val) {
On 25.03.2021 21:19, Bjorn Helgaas wrote: > On Thu, Mar 25, 2021 at 10:24:28AM +0100, Marek Szyprowski wrote: >> On 25.01.2021 05:48, Zhiqiang Hou wrote: >>> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> >>> >>> In the dw_pcie_ep_init(), it depends on the detected iATU region >>> numbers to allocate the in/outbound window management bit map. >>> It fails after the commit 281f1f99cf3a ("PCI: dwc: Detect number >>> of iATU windows"). >>> >>> So this patch move the iATU region detection into a new function, >>> move forward the detection to the very beginning of functions >>> dw_pcie_host_init() and dw_pcie_ep_init(). And also remove it >>> from the dw_pcie_setup(), since it's more like a software >>> perspective initialization step than hardware setup. >>> >>> Fixes: 281f1f99cf3a ("PCI: dwc: Detect number of iATU windows") >>> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> >> This patch causes exynos-pcie to hang during the initialization. It >> looks that some resources are not enabled yet, so calling >> dw_pcie_iatu_detect() much earlier causes a hang. When I have some time, >> I will try to identify what is needed to call it properly. > Thanks, I dropped it for now. We can add it back after we figure out > what the exynos issue is. Thanks, I will try to identify at which point of initialization it is safe to call iATU region detection. > For reference, here's the patch I dropped (I had made some minor > corrections to the commit log): > > commit fd4162f05194 ("PCI: dwc: Move iATU detection earlier") > Author: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > Date: Mon Jan 25 12:48:03 2021 +0800 > > PCI: dwc: Move iATU detection earlier > > dw_pcie_ep_init() depends on the detected iATU region numbers to allocate > the in/outbound window management bitmap. It fails after 281f1f99cf3a > ("PCI: dwc: Detect number of iATU windows"). > > Move the iATU region detection into a new function, move the detection to > the very beginning of dw_pcie_host_init() and dw_pcie_ep_init(). Also > remove it from the dw_pcie_setup(), since it's more like a software > initialization step than hardware setup. > > Fixes: 281f1f99cf3a ("PCI: dwc: Detect number of iATU windows") > Link: https://lore.kernel.org/r/20210125044803.4310-1-Zhiqiang.Hou@nxp.com > Tested-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > Reviewed-by: Rob Herring <robh@kernel.org> > Cc: stable@vger.kernel.org # v5.11+ > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index 1c25d8337151..8d028a88b375 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -705,6 +705,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > } > } > > + dw_pcie_iatu_detect(pci); > + > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space"); > if (!res) > return -EINVAL; > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index 7e55b2b66182..52f6887179cd 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -319,6 +319,8 @@ int dw_pcie_host_init(struct pcie_port *pp) > return PTR_ERR(pci->dbi_base); > } > > + dw_pcie_iatu_detect(pci); > + > bridge = devm_pci_alloc_host_bridge(dev, 0); > if (!bridge) > return -ENOMEM; > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c > index 004cb860e266..a945f0c0e73d 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.c > +++ b/drivers/pci/controller/dwc/pcie-designware.c > @@ -660,11 +660,9 @@ static void dw_pcie_iatu_detect_regions(struct dw_pcie *pci) > pci->num_ob_windows = ob; > } > > -void dw_pcie_setup(struct dw_pcie *pci) > +void dw_pcie_iatu_detect(struct dw_pcie *pci) > { > - u32 val; > struct device *dev = pci->dev; > - struct device_node *np = dev->of_node; > struct platform_device *pdev = to_platform_device(dev); > > if (pci->version >= 0x480A || (!pci->version && > @@ -693,6 +691,13 @@ void dw_pcie_setup(struct dw_pcie *pci) > > dev_info(pci->dev, "Detected iATU regions: %u outbound, %u inbound", > pci->num_ob_windows, pci->num_ib_windows); > +} > + > +void dw_pcie_setup(struct dw_pcie *pci) > +{ > + u32 val; > + struct device *dev = pci->dev; > + struct device_node *np = dev->of_node; > > if (pci->link_gen > 0) > dw_pcie_link_set_max_speed(pci, pci->link_gen); > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index 7247c8b01f04..7d6e9b7576be 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -306,6 +306,7 @@ int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no, int index, > void dw_pcie_disable_atu(struct dw_pcie *pci, int index, > enum dw_pcie_region_type type); > void dw_pcie_setup(struct dw_pcie *pci); > +void dw_pcie_iatu_detect(struct dw_pcie *pci); > > static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val) > { > Best regards
On 26.03.2021 18:05, Marek Szyprowski wrote: > On 25.03.2021 21:19, Bjorn Helgaas wrote: >> On Thu, Mar 25, 2021 at 10:24:28AM +0100, Marek Szyprowski wrote: >>> On 25.01.2021 05:48, Zhiqiang Hou wrote: >>>> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> >>>> >>>> In the dw_pcie_ep_init(), it depends on the detected iATU region >>>> numbers to allocate the in/outbound window management bit map. >>>> It fails after the commit 281f1f99cf3a ("PCI: dwc: Detect number >>>> of iATU windows"). >>>> >>>> So this patch move the iATU region detection into a new function, >>>> move forward the detection to the very beginning of functions >>>> dw_pcie_host_init() and dw_pcie_ep_init(). And also remove it >>>> from the dw_pcie_setup(), since it's more like a software >>>> perspective initialization step than hardware setup. >>>> >>>> Fixes: 281f1f99cf3a ("PCI: dwc: Detect number of iATU windows") >>>> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> >>> This patch causes exynos-pcie to hang during the initialization. It >>> looks that some resources are not enabled yet, so calling >>> dw_pcie_iatu_detect() much earlier causes a hang. When I have some >>> time, >>> I will try to identify what is needed to call it properly. >> Thanks, I dropped it for now. We can add it back after we figure out >> what the exynos issue is. > Thanks, I will try to identify at which point of initialization it is > safe to call iATU region detection. I've just checked and it is enough to move the dw_pcie_iatu_detect(pci); after pp->ops->host_init(pp); in dw_pcie_host_init() to fix driver operation on Exynos SoCs with the $subject patch applied. Best regards
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c index bcd1cd9ba8c8..fcf935bf6f5e 100644 --- a/drivers/pci/controller/dwc/pcie-designware-ep.c +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c @@ -707,6 +707,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) } } + dw_pcie_iatu_detect(pci); + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space"); if (!res) return -EINVAL; diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c index 8a84c005f32b..8eae817c138d 100644 --- a/drivers/pci/controller/dwc/pcie-designware-host.c +++ b/drivers/pci/controller/dwc/pcie-designware-host.c @@ -316,6 +316,8 @@ int dw_pcie_host_init(struct pcie_port *pp) return PTR_ERR(pci->dbi_base); } + dw_pcie_iatu_detect(pci); + bridge = devm_pci_alloc_host_bridge(dev, 0); if (!bridge) return -ENOMEM; diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c index 5b72a5448d2e..5b9bf02d918b 100644 --- a/drivers/pci/controller/dwc/pcie-designware.c +++ b/drivers/pci/controller/dwc/pcie-designware.c @@ -654,11 +654,9 @@ static void dw_pcie_iatu_detect_regions(struct dw_pcie *pci) pci->num_ob_windows = ob; } -void dw_pcie_setup(struct dw_pcie *pci) +void dw_pcie_iatu_detect(struct dw_pcie *pci) { - u32 val; struct device *dev = pci->dev; - struct device_node *np = dev->of_node; struct platform_device *pdev = to_platform_device(dev); if (pci->version >= 0x480A || (!pci->version && @@ -687,6 +685,13 @@ void dw_pcie_setup(struct dw_pcie *pci) dev_info(pci->dev, "Detected iATU regions: %u outbound, %u inbound", pci->num_ob_windows, pci->num_ib_windows); +} + +void dw_pcie_setup(struct dw_pcie *pci) +{ + u32 val; + struct device *dev = pci->dev; + struct device_node *np = dev->of_node; if (pci->link_gen > 0) dw_pcie_link_set_max_speed(pci, pci->link_gen); diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h index 5d979953800d..867369d4c4f7 100644 --- a/drivers/pci/controller/dwc/pcie-designware.h +++ b/drivers/pci/controller/dwc/pcie-designware.h @@ -305,6 +305,7 @@ int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no, int index, void dw_pcie_disable_atu(struct dw_pcie *pci, int index, enum dw_pcie_region_type type); void dw_pcie_setup(struct dw_pcie *pci); +void dw_pcie_iatu_detect(struct dw_pcie *pci); static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val) {