| Message ID | 20220624143947.8991-3-Sergey.Semin@baikalelectronics.ru (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 36286b4ec2daf7f405b344dc3cb2f8a4e351b117 |
| Headers | show |
| Series | PCI: dwc: Add hw version and dma-ranges support | expand |
On Fri, Jun 24, 2022 at 05:39:34PM +0300, Serge Semin wrote: > The iATU detection procedure was introduced in the commit 281f1f99cf3a > ("PCI: dwc: Detect number of iATU windows"). A bit later the procedure > execution was moved to Host/EP-specific methods in the framework of commit > 8bcca2658558 ("PCI: dwc: Move iATU detection earlier"). The later > modification wasn't done in the most optimal way since the "addr_space" > CSR region resource doesn't depend on anything detected in the > dw_pcie_iatu_detect() method. Thus the detection can be postponed to be > executed after the resource request which can fail and make the detection > pointless. It will be also helpful for the dw_pcie_ep_init() method > readability since we are about to add the IP-core version and eDMA module > (a bit later) detection procedures. The patch is fine, but IMO the commit log is overkill. It's enough to say that we do some work (dw_pcie_iatu_detect()) earlier than necessary, so we can move it later. All the history makes it seem like this is more complicated than it really is. Something like this is enough: Previously, dw_pcie_ep_init() did: dw_pcie_iatu_detect(pci); res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space"); if (!res) return -EINVAL; The platform_get_resource_byname() can fail, and dw_pcie_iatu_detect() doesn't depend on the "addr_space" resource, so delay it until afterwards, i.e., platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space"); dw_pcie_iatu_detect(pci); No need to repost for this. > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > Reviewed-by: Rob Herring <robh@kernel.org> > > --- > > Changelog v2: > - This is a new patch added on v2 iteration of the series. > --- > drivers/pci/controller/dwc/pcie-designware-ep.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index 15b8059544e3..1e35542d6f72 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -704,8 +704,6 @@ 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; > @@ -713,6 +711,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > ep->phys_base = res->start; > ep->addr_size = resource_size(res); > > + dw_pcie_iatu_detect(pci); > + > ep->ib_window_map = devm_kcalloc(dev, > BITS_TO_LONGS(pci->num_ib_windows), > sizeof(long), > -- > 2.35.1 >
On Fri, Jun 24, 2022 at 05:39:34PM +0300, Serge Semin wrote: > The iATU detection procedure was introduced in the commit 281f1f99cf3a > ("PCI: dwc: Detect number of iATU windows"). A bit later the procedure > execution was moved to Host/EP-specific methods in the framework of commit > 8bcca2658558 ("PCI: dwc: Move iATU detection earlier"). The later > modification wasn't done in the most optimal way since the "addr_space" > CSR region resource doesn't depend on anything detected in the > dw_pcie_iatu_detect() method. Thus the detection can be postponed to be > executed after the resource request which can fail and make the detection > pointless. It will be also helpful for the dw_pcie_ep_init() method > readability since we are about to add the IP-core version and eDMA module > (a bit later) detection procedures. > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Thanks, Mani > Reviewed-by: Rob Herring <robh@kernel.org> > > --- > > Changelog v2: > - This is a new patch added on v2 iteration of the series. > --- > drivers/pci/controller/dwc/pcie-designware-ep.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index 15b8059544e3..1e35542d6f72 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -704,8 +704,6 @@ 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; > @@ -713,6 +711,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > ep->phys_base = res->start; > ep->addr_size = resource_size(res); > > + dw_pcie_iatu_detect(pci); > + > ep->ib_window_map = devm_kcalloc(dev, > BITS_TO_LONGS(pci->num_ib_windows), > sizeof(long), > -- > 2.35.1 >
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c index 15b8059544e3..1e35542d6f72 100644 --- a/drivers/pci/controller/dwc/pcie-designware-ep.c +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c @@ -704,8 +704,6 @@ 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; @@ -713,6 +711,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) ep->phys_base = res->start; ep->addr_size = resource_size(res); + dw_pcie_iatu_detect(pci); + ep->ib_window_map = devm_kcalloc(dev, BITS_TO_LONGS(pci->num_ib_windows), sizeof(long),
The iATU detection procedure was introduced in the commit 281f1f99cf3a ("PCI: dwc: Detect number of iATU windows"). A bit later the procedure execution was moved to Host/EP-specific methods in the framework of commit 8bcca2658558 ("PCI: dwc: Move iATU detection earlier"). The later modification wasn't done in the most optimal way since the "addr_space" CSR region resource doesn't depend on anything detected in the dw_pcie_iatu_detect() method. Thus the detection can be postponed to be executed after the resource request which can fail and make the detection pointless. It will be also helpful for the dw_pcie_ep_init() method readability since we are about to add the IP-core version and eDMA module (a bit later) detection procedures. Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> Reviewed-by: Rob Herring <robh@kernel.org> --- Changelog v2: - This is a new patch added on v2 iteration of the series. --- drivers/pci/controller/dwc/pcie-designware-ep.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)