[PATCH V6 1/3] PCI: dwc: Add API support to de-initialize host

[PATCH V6 1/3] PCI: dwc: Add API support to de-initialize host

[Vidya Sagar]

Add an API to group all the tasks to be done to de-initialize host which
can then be called by any DesignWare core based driver implementations
while adding .remove() support in their respective drivers.

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
---
Changes from v5:
* None

Changes from v4:
* None

Changes from v3:
* Added check if (pci_msi_enabled() && !pp->ops->msi_host_init) before calling
  dw_pcie_free_msi() API to mimic init path

Changes from v2:
* Rebased on top of linux-next top of the tree branch

Changes from v1:
* s/Designware/DesignWare

 drivers/pci/controller/dwc/pcie-designware-host.c | 8 ++++++++
 drivers/pci/controller/dwc/pcie-designware.h      | 5 +++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 77db32529319..d069e4290180 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -496,6 +496,14 @@ int dw_pcie_host_init(struct pcie_port *pp)
 	return ret;
 }
 
+void dw_pcie_host_deinit(struct pcie_port *pp)
+{
+	pci_stop_root_bus(pp->root_bus);
+	pci_remove_root_bus(pp->root_bus);
+	if (pci_msi_enabled() && !pp->ops->msi_host_init)
+		dw_pcie_free_msi(pp);
+}
+
 static int dw_pcie_access_other_conf(struct pcie_port *pp, struct pci_bus *bus,
 				     u32 devfn, int where, int size, u32 *val,
 				     bool write)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index b8993f2b78df..14762e262758 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -351,6 +351,7 @@ void dw_pcie_msi_init(struct pcie_port *pp);
 void dw_pcie_free_msi(struct pcie_port *pp);
 void dw_pcie_setup_rc(struct pcie_port *pp);
 int dw_pcie_host_init(struct pcie_port *pp);
+void dw_pcie_host_deinit(struct pcie_port *pp);
 int dw_pcie_allocate_domains(struct pcie_port *pp);
 #else
 static inline irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
@@ -375,6 +376,10 @@ static inline int dw_pcie_host_init(struct pcie_port *pp)
 	return 0;
 }
 
+static inline void dw_pcie_host_deinit(struct pcie_port *pp)
+{
+}
+
 static inline int dw_pcie_allocate_domains(struct pcie_port *pp)
 {
 	return 0;
-- 
2.17.1

[PATCH V6 2/3] PCI: dwc: Cleanup DBI read and write APIs

[Vidya Sagar]

Cleanup DBI read and write APIs by removing "__" (underscore) from their
names as there are no no-underscore versions and the underscore versions
are already doing what no-underscore versions typically do. It also removes
passing dbi/dbi2 base address as one of the arguments as the same can be
derived with in read and write APIs.

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
Changes from v5:
* Removed passing base address as one of the arguments as the same can be derived within
  the API itself.
* Modified ATU read/write APIs to call dw_pcie_{write/read}() API

Changes from v4:
* This is a new patch in this series

 drivers/pci/controller/dwc/pcie-designware.c | 28 ++++++-------
 drivers/pci/controller/dwc/pcie-designware.h | 43 ++++++++++++--------
 2 files changed, 37 insertions(+), 34 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 9d7c51c32b3b..0b383feb13de 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -52,64 +52,60 @@ int dw_pcie_write(void __iomem *addr, int size, u32 val)
 	return PCIBIOS_SUCCESSFUL;
 }
 
-u32 __dw_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base, u32 reg,
-		       size_t size)
+u32 dw_pcie_read_dbi(struct dw_pcie *pci, u32 reg, size_t size)
 {
 	int ret;
 	u32 val;
 
 	if (pci->ops->read_dbi)
-		return pci->ops->read_dbi(pci, base, reg, size);
+		return pci->ops->read_dbi(pci, pci->dbi_base, reg, size);
 
-	ret = dw_pcie_read(base + reg, size, &val);
+	ret = dw_pcie_read(pci->dbi_base + reg, size, &val);
 	if (ret)
 		dev_err(pci->dev, "Read DBI address failed\n");
 
 	return val;
 }
 
-void __dw_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base, u32 reg,
-			 size_t size, u32 val)
+void dw_pcie_write_dbi(struct dw_pcie *pci, u32 reg, size_t size, u32 val)
 {
 	int ret;
 
 	if (pci->ops->write_dbi) {
-		pci->ops->write_dbi(pci, base, reg, size, val);
+		pci->ops->write_dbi(pci, pci->dbi_base, reg, size, val);
 		return;
 	}
 
-	ret = dw_pcie_write(base + reg, size, val);
+	ret = dw_pcie_write(pci->dbi_base + reg, size, val);
 	if (ret)
 		dev_err(pci->dev, "Write DBI address failed\n");
 }
 
-u32 __dw_pcie_read_dbi2(struct dw_pcie *pci, void __iomem *base, u32 reg,
-			size_t size)
+u32 dw_pcie_read_dbi2(struct dw_pcie *pci, u32 reg, size_t size)
 {
 	int ret;
 	u32 val;
 
 	if (pci->ops->read_dbi2)
-		return pci->ops->read_dbi2(pci, base, reg, size);
+		return pci->ops->read_dbi2(pci, pci->dbi_base2, reg, size);
 
-	ret = dw_pcie_read(base + reg, size, &val);
+	ret = dw_pcie_read(pci->dbi_base2 + reg, size, &val);
 	if (ret)
 		dev_err(pci->dev, "read DBI address failed\n");
 
 	return val;
 }
 
-void __dw_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base, u32 reg,
-			  size_t size, u32 val)
+void dw_pcie_write_dbi2(struct dw_pcie *pci, u32 reg, size_t size, u32 val)
 {
 	int ret;
 
 	if (pci->ops->write_dbi2) {
-		pci->ops->write_dbi2(pci, base, reg, size, val);
+		pci->ops->write_dbi2(pci, pci->dbi_base2, reg, size, val);
 		return;
 	}
 
-	ret = dw_pcie_write(base + reg, size, val);
+	ret = dw_pcie_write(pci->dbi_base2 + reg, size, val);
 	if (ret)
 		dev_err(pci->dev, "write DBI address failed\n");
 }
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 14762e262758..88300b445a4d 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -254,14 +254,10 @@ struct dw_pcie {
 int dw_pcie_read(void __iomem *addr, int size, u32 *val);
 int dw_pcie_write(void __iomem *addr, int size, u32 val);
 
-u32 __dw_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base, u32 reg,
-		       size_t size);
-void __dw_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base, u32 reg,
-			 size_t size, u32 val);
-u32 __dw_pcie_read_dbi2(struct dw_pcie *pci, void __iomem *base, u32 reg,
-			size_t size);
-void __dw_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base, u32 reg,
-			  size_t size, u32 val);
+u32 dw_pcie_read_dbi(struct dw_pcie *pci, u32 reg, size_t size);
+void dw_pcie_write_dbi(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
+u32 dw_pcie_read_dbi2(struct dw_pcie *pci, u32 reg, size_t size);
+void dw_pcie_write_dbi2(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
 int dw_pcie_link_up(struct dw_pcie *pci);
 int dw_pcie_wait_for_link(struct dw_pcie *pci);
 void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
@@ -275,52 +271,63 @@ void dw_pcie_setup(struct dw_pcie *pci);
 
 static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val)
 {
-	__dw_pcie_write_dbi(pci, pci->dbi_base, reg, 0x4, val);
+	dw_pcie_write_dbi(pci, reg, 0x4, val);
 }
 
 static inline u32 dw_pcie_readl_dbi(struct dw_pcie *pci, u32 reg)
 {
-	return __dw_pcie_read_dbi(pci, pci->dbi_base, reg, 0x4);
+	return dw_pcie_read_dbi(pci, reg, 0x4);
 }
 
 static inline void dw_pcie_writew_dbi(struct dw_pcie *pci, u32 reg, u16 val)
 {
-	__dw_pcie_write_dbi(pci, pci->dbi_base, reg, 0x2, val);
+	dw_pcie_write_dbi(pci, reg, 0x2, val);
 }
 
 static inline u16 dw_pcie_readw_dbi(struct dw_pcie *pci, u32 reg)
 {
-	return __dw_pcie_read_dbi(pci, pci->dbi_base, reg, 0x2);
+	return dw_pcie_read_dbi(pci, reg, 0x2);
 }
 
 static inline void dw_pcie_writeb_dbi(struct dw_pcie *pci, u32 reg, u8 val)
 {
-	__dw_pcie_write_dbi(pci, pci->dbi_base, reg, 0x1, val);
+	dw_pcie_write_dbi(pci, reg, 0x1, val);
 }
 
 static inline u8 dw_pcie_readb_dbi(struct dw_pcie *pci, u32 reg)
 {
-	return __dw_pcie_read_dbi(pci, pci->dbi_base, reg, 0x1);
+	return dw_pcie_read_dbi(pci, reg, 0x1);
 }
 
 static inline void dw_pcie_writel_dbi2(struct dw_pcie *pci, u32 reg, u32 val)
 {
-	__dw_pcie_write_dbi2(pci, pci->dbi_base2, reg, 0x4, val);
+	dw_pcie_write_dbi2(pci, reg, 0x4, val);
 }
 
 static inline u32 dw_pcie_readl_dbi2(struct dw_pcie *pci, u32 reg)
 {
-	return __dw_pcie_read_dbi2(pci, pci->dbi_base2, reg, 0x4);
+	return dw_pcie_read_dbi2(pci, reg, 0x4);
 }
 
 static inline void dw_pcie_writel_atu(struct dw_pcie *pci, u32 reg, u32 val)
 {
-	__dw_pcie_write_dbi(pci, pci->atu_base, reg, 0x4, val);
+	int ret;
+
+	ret = dw_pcie_write(pci->atu_base + reg, 0x4, val);
+	if (ret)
+		dev_err(pci->dev, "write ATU address failed\n");
 }
 
 static inline u32 dw_pcie_readl_atu(struct dw_pcie *pci, u32 reg)
 {
-	return __dw_pcie_read_dbi(pci, pci->atu_base, reg, 0x4);
+	int ret;
+	u32 val;
+
+	ret = dw_pcie_read(pci->atu_base + reg, 0x4, &val);
+	if (ret)
+		dev_err(pci->dev, "Read ATU address failed\n");
+
+	return val;
 }
 
 static inline void dw_pcie_dbi_ro_wr_en(struct dw_pcie *pci)
-- 
2.17.1

[PATCH V6 3/3] PCI: dwc: Export APIs to support .remove() implementation

[Vidya Sagar]

Export all configuration space access APIs and also other APIs to
support host controller drivers of DesignWare core based implementations
while adding support for .remove() hook to build their respective drivers
as modules

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
---
Changes from v5:
* None

Changes from v4:
* Removed __ (underscore) from dw_pcie_{write/read}_dbi API names

Changes from v3:
* Exported only __dw_pcie_{read/write}_dbi() APIs instead of
  dw_pcie_read{l/w/b}_dbi & dw_pcie_write{l/w/b}_dbi APIs.

Changes from v2:
* Rebased on top of linux-next top of the tree branch

Changes from v1:
* s/Designware/DesignWare

 drivers/pci/controller/dwc/pcie-designware-host.c | 4 ++++
 drivers/pci/controller/dwc/pcie-designware.c      | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index d069e4290180..f93252d0da5b 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -311,6 +311,7 @@ void dw_pcie_msi_init(struct pcie_port *pp)
 	dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_HI, 4,
 			    upper_32_bits(msi_target));
 }
+EXPORT_SYMBOL_GPL(dw_pcie_msi_init);
 
 int dw_pcie_host_init(struct pcie_port *pp)
 {
@@ -495,6 +496,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 		dw_pcie_free_msi(pp);
 	return ret;
 }
+EXPORT_SYMBOL_GPL(dw_pcie_host_init);
 
 void dw_pcie_host_deinit(struct pcie_port *pp)
 {
@@ -503,6 +505,7 @@ void dw_pcie_host_deinit(struct pcie_port *pp)
 	if (pci_msi_enabled() && !pp->ops->msi_host_init)
 		dw_pcie_free_msi(pp);
 }
+EXPORT_SYMBOL_GPL(dw_pcie_host_deinit);
 
 static int dw_pcie_access_other_conf(struct pcie_port *pp, struct pci_bus *bus,
 				     u32 devfn, int where, int size, u32 *val,
@@ -695,3 +698,4 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
 	val |= PORT_LOGIC_SPEED_CHANGE;
 	dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val);
 }
+EXPORT_SYMBOL_GPL(dw_pcie_setup_rc);
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 0b383feb13de..dc9cdcd72ffc 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -34,6 +34,7 @@ int dw_pcie_read(void __iomem *addr, int size, u32 *val)
 
 	return PCIBIOS_SUCCESSFUL;
 }
+EXPORT_SYMBOL_GPL(dw_pcie_read);
 
 int dw_pcie_write(void __iomem *addr, int size, u32 val)
 {
@@ -51,6 +52,7 @@ int dw_pcie_write(void __iomem *addr, int size, u32 val)
 
 	return PCIBIOS_SUCCESSFUL;
 }
+EXPORT_SYMBOL_GPL(dw_pcie_write);
 
 u32 dw_pcie_read_dbi(struct dw_pcie *pci, u32 reg, size_t size)
 {
@@ -66,6 +68,7 @@ u32 dw_pcie_read_dbi(struct dw_pcie *pci, u32 reg, size_t size)
 
 	return val;
 }
+EXPORT_SYMBOL_GPL(dw_pcie_read_dbi);
 
 void dw_pcie_write_dbi(struct dw_pcie *pci, u32 reg, size_t size, u32 val)
 {
@@ -80,6 +83,7 @@ void dw_pcie_write_dbi(struct dw_pcie *pci, u32 reg, size_t size, u32 val)
 	if (ret)
 		dev_err(pci->dev, "Write DBI address failed\n");
 }
+EXPORT_SYMBOL_GPL(dw_pcie_write_dbi);
 
 u32 dw_pcie_read_dbi2(struct dw_pcie *pci, u32 reg, size_t size)
 {
-- 
2.17.1

Re: [PATCH V6 2/3] PCI: dwc: Cleanup DBI read and write APIs

[Lorenzo Pieralisi]

On Fri, Jun 21, 2019 at 04:39:59PM +0530, Vidya Sagar wrote:
> Cleanup DBI read and write APIs by removing "__" (underscore) from their
> names as there are no no-underscore versions and the underscore versions
> are already doing what no-underscore versions typically do. It also removes
> passing dbi/dbi2 base address as one of the arguments as the same can be
> derived with in read and write APIs.
> 
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
> Changes from v5:
> * Removed passing base address as one of the arguments as the same can be derived within
>   the API itself.
> * Modified ATU read/write APIs to call dw_pcie_{write/read}() API
> 
> Changes from v4:
> * This is a new patch in this series
> 
>  drivers/pci/controller/dwc/pcie-designware.c | 28 ++++++-------
>  drivers/pci/controller/dwc/pcie-designware.h | 43 ++++++++++++--------
>  2 files changed, 37 insertions(+), 34 deletions(-)

Gustavo, Jingoo,

please ACK this patch if you are OK with it so that I can merge
the series, thanks.

Lorenzo

> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 9d7c51c32b3b..0b383feb13de 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -52,64 +52,60 @@ int dw_pcie_write(void __iomem *addr, int size, u32 val)
>  	return PCIBIOS_SUCCESSFUL;
>  }
>  
> -u32 __dw_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base, u32 reg,
> -		       size_t size)
> +u32 dw_pcie_read_dbi(struct dw_pcie *pci, u32 reg, size_t size)
>  {
>  	int ret;
>  	u32 val;
>  
>  	if (pci->ops->read_dbi)
> -		return pci->ops->read_dbi(pci, base, reg, size);
> +		return pci->ops->read_dbi(pci, pci->dbi_base, reg, size);
>  
> -	ret = dw_pcie_read(base + reg, size, &val);
> +	ret = dw_pcie_read(pci->dbi_base + reg, size, &val);
>  	if (ret)
>  		dev_err(pci->dev, "Read DBI address failed\n");
>  
>  	return val;
>  }
>  
> -void __dw_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base, u32 reg,
> -			 size_t size, u32 val)
> +void dw_pcie_write_dbi(struct dw_pcie *pci, u32 reg, size_t size, u32 val)
>  {
>  	int ret;
>  
>  	if (pci->ops->write_dbi) {
> -		pci->ops->write_dbi(pci, base, reg, size, val);
> +		pci->ops->write_dbi(pci, pci->dbi_base, reg, size, val);
>  		return;
>  	}
>  
> -	ret = dw_pcie_write(base + reg, size, val);
> +	ret = dw_pcie_write(pci->dbi_base + reg, size, val);
>  	if (ret)
>  		dev_err(pci->dev, "Write DBI address failed\n");
>  }
>  
> -u32 __dw_pcie_read_dbi2(struct dw_pcie *pci, void __iomem *base, u32 reg,
> -			size_t size)
> +u32 dw_pcie_read_dbi2(struct dw_pcie *pci, u32 reg, size_t size)
>  {
>  	int ret;
>  	u32 val;
>  
>  	if (pci->ops->read_dbi2)
> -		return pci->ops->read_dbi2(pci, base, reg, size);
> +		return pci->ops->read_dbi2(pci, pci->dbi_base2, reg, size);
>  
> -	ret = dw_pcie_read(base + reg, size, &val);
> +	ret = dw_pcie_read(pci->dbi_base2 + reg, size, &val);
>  	if (ret)
>  		dev_err(pci->dev, "read DBI address failed\n");
>  
>  	return val;
>  }
>  
> -void __dw_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base, u32 reg,
> -			  size_t size, u32 val)
> +void dw_pcie_write_dbi2(struct dw_pcie *pci, u32 reg, size_t size, u32 val)
>  {
>  	int ret;
>  
>  	if (pci->ops->write_dbi2) {
> -		pci->ops->write_dbi2(pci, base, reg, size, val);
> +		pci->ops->write_dbi2(pci, pci->dbi_base2, reg, size, val);
>  		return;
>  	}
>  
> -	ret = dw_pcie_write(base + reg, size, val);
> +	ret = dw_pcie_write(pci->dbi_base2 + reg, size, val);
>  	if (ret)
>  		dev_err(pci->dev, "write DBI address failed\n");
>  }
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 14762e262758..88300b445a4d 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -254,14 +254,10 @@ struct dw_pcie {
>  int dw_pcie_read(void __iomem *addr, int size, u32 *val);
>  int dw_pcie_write(void __iomem *addr, int size, u32 val);
>  
> -u32 __dw_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base, u32 reg,
> -		       size_t size);
> -void __dw_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base, u32 reg,
> -			 size_t size, u32 val);
> -u32 __dw_pcie_read_dbi2(struct dw_pcie *pci, void __iomem *base, u32 reg,
> -			size_t size);
> -void __dw_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base, u32 reg,
> -			  size_t size, u32 val);
> +u32 dw_pcie_read_dbi(struct dw_pcie *pci, u32 reg, size_t size);
> +void dw_pcie_write_dbi(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
> +u32 dw_pcie_read_dbi2(struct dw_pcie *pci, u32 reg, size_t size);
> +void dw_pcie_write_dbi2(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
>  int dw_pcie_link_up(struct dw_pcie *pci);
>  int dw_pcie_wait_for_link(struct dw_pcie *pci);
>  void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
> @@ -275,52 +271,63 @@ void dw_pcie_setup(struct dw_pcie *pci);
>  
>  static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val)
>  {
> -	__dw_pcie_write_dbi(pci, pci->dbi_base, reg, 0x4, val);
> +	dw_pcie_write_dbi(pci, reg, 0x4, val);
>  }
>  
>  static inline u32 dw_pcie_readl_dbi(struct dw_pcie *pci, u32 reg)
>  {
> -	return __dw_pcie_read_dbi(pci, pci->dbi_base, reg, 0x4);
> +	return dw_pcie_read_dbi(pci, reg, 0x4);
>  }
>  
>  static inline void dw_pcie_writew_dbi(struct dw_pcie *pci, u32 reg, u16 val)
>  {
> -	__dw_pcie_write_dbi(pci, pci->dbi_base, reg, 0x2, val);
> +	dw_pcie_write_dbi(pci, reg, 0x2, val);
>  }
>  
>  static inline u16 dw_pcie_readw_dbi(struct dw_pcie *pci, u32 reg)
>  {
> -	return __dw_pcie_read_dbi(pci, pci->dbi_base, reg, 0x2);
> +	return dw_pcie_read_dbi(pci, reg, 0x2);
>  }
>  
>  static inline void dw_pcie_writeb_dbi(struct dw_pcie *pci, u32 reg, u8 val)
>  {
> -	__dw_pcie_write_dbi(pci, pci->dbi_base, reg, 0x1, val);
> +	dw_pcie_write_dbi(pci, reg, 0x1, val);
>  }
>  
>  static inline u8 dw_pcie_readb_dbi(struct dw_pcie *pci, u32 reg)
>  {
> -	return __dw_pcie_read_dbi(pci, pci->dbi_base, reg, 0x1);
> +	return dw_pcie_read_dbi(pci, reg, 0x1);
>  }
>  
>  static inline void dw_pcie_writel_dbi2(struct dw_pcie *pci, u32 reg, u32 val)
>  {
> -	__dw_pcie_write_dbi2(pci, pci->dbi_base2, reg, 0x4, val);
> +	dw_pcie_write_dbi2(pci, reg, 0x4, val);
>  }
>  
>  static inline u32 dw_pcie_readl_dbi2(struct dw_pcie *pci, u32 reg)
>  {
> -	return __dw_pcie_read_dbi2(pci, pci->dbi_base2, reg, 0x4);
> +	return dw_pcie_read_dbi2(pci, reg, 0x4);
>  }
>  
>  static inline void dw_pcie_writel_atu(struct dw_pcie *pci, u32 reg, u32 val)
>  {
> -	__dw_pcie_write_dbi(pci, pci->atu_base, reg, 0x4, val);
> +	int ret;
> +
> +	ret = dw_pcie_write(pci->atu_base + reg, 0x4, val);
> +	if (ret)
> +		dev_err(pci->dev, "write ATU address failed\n");
>  }
>  
>  static inline u32 dw_pcie_readl_atu(struct dw_pcie *pci, u32 reg)
>  {
> -	return __dw_pcie_read_dbi(pci, pci->atu_base, reg, 0x4);
> +	int ret;
> +	u32 val;
> +
> +	ret = dw_pcie_read(pci->atu_base + reg, 0x4, &val);
> +	if (ret)
> +		dev_err(pci->dev, "Read ATU address failed\n");
> +
> +	return val;
>  }
>  
>  static inline void dw_pcie_dbi_ro_wr_en(struct dw_pcie *pci)
> -- 
> 2.17.1
> 

Re: [PATCH V6 2/3] PCI: dwc: Cleanup DBI read and write APIs

[Jingoo Han]

On 6/21/19, 8:10 PM, Vidya Sagar wrote:
> 
> Cleanup DBI read and write APIs by removing "__" (underscore) from their
> names as there are no no-underscore versions and the underscore versions
> are already doing what no-underscore versions typically do. It also removes
> passing dbi/dbi2 base address as one of the arguments as the same can be
> derived with in read and write APIs.
>
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
> Changes from v5:
> * Removed passing base address as one of the arguments as the same can be derived within
>   the API itself.
> * Modified ATU read/write APIs to call dw_pcie_{write/read}() API

Unlike previous patches (v1~v5), you modified ATU read/write APIs from v6.
Why do you change ATU read/write APIs to call dw_pcie_{write/read}() API???
It is not clean-up, but function change. Please add the reason to the commit message.

Best regards,
Jingoo Han

>
> Changes from v4:
> * This is a new patch in this series
>
>  drivers/pci/controller/dwc/pcie-designware.c | 28 ++++++-------
>  drivers/pci/controller/dwc/pcie-designware.h | 43 ++++++++++++--------
>  2 files changed, 37 insertions(+), 34 deletions(-)

.....

>  static inline void dw_pcie_writel_atu(struct dw_pcie *pci, u32 reg, u32 val)
>  {
> -	__dw_pcie_write_dbi(pci, pci->atu_base, reg, 0x4, val);
> +	int ret;
> +
> +	ret = dw_pcie_write(pci->atu_base + reg, 0x4, val);
> +	if (ret)
> +		dev_err(pci->dev, "write ATU address failed\n");
>  }
>  
>  static inline u32 dw_pcie_readl_atu(struct dw_pcie *pci, u32 reg)
>  {
> -	return __dw_pcie_read_dbi(pci, pci->atu_base, reg, 0x4);
> +	int ret;
> +	u32 val;
> +
> +	ret = dw_pcie_read(pci->atu_base + reg, 0x4, &val);
> +	if (ret)
> +		dev_err(pci->dev, "Read ATU address failed\n");
> +
> +	return val;
>  }
>  
>  static inline void dw_pcie_dbi_ro_wr_en(struct dw_pcie *pci)
> -- 
> 2.17.1

Re: [PATCH V6 2/3] PCI: dwc: Cleanup DBI read and write APIs

[Vidya Sagar]

On 6/22/2019 10:56 AM, Jingoo Han wrote:
> On 6/21/19, 8:10 PM, Vidya Sagar wrote:
>>
>> Cleanup DBI read and write APIs by removing "__" (underscore) from their
>> names as there are no no-underscore versions and the underscore versions
>> are already doing what no-underscore versions typically do. It also removes
>> passing dbi/dbi2 base address as one of the arguments as the same can be
>> derived with in read and write APIs.
>>
>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>> ---
>> Changes from v5:
>> * Removed passing base address as one of the arguments as the same can be derived within
>>    the API itself.
>> * Modified ATU read/write APIs to call dw_pcie_{write/read}() API
> 
> Unlike previous patches (v1~v5), you modified ATU read/write APIs from v6.
> Why do you change ATU read/write APIs to call dw_pcie_{write/read}() API???
> It is not clean-up, but function change. Please add the reason to the commit message.
> 
> Best regards,
> Jingoo Han
Reason is that, now dbi/dbi2 APIs don't take base address offset as one of the input arguments
instead, those APIs derive that inside the API itself. So, for ATU read/write DBI read/write
APIs can be used directly and hence I called dw_pcie_{write/read}() API directly. But, taking a
second look at it, it may work for Tegra, but, may not for other implementations where accessing
DBI space is not a simple read/write to DBI base + offset. Let me address address that too in
next patch.

Thanks,
Vidya Sagar

> 
>>
>> Changes from v4:
>> * This is a new patch in this series
>>
>>   drivers/pci/controller/dwc/pcie-designware.c | 28 ++++++-------
>>   drivers/pci/controller/dwc/pcie-designware.h | 43 ++++++++++++--------
>>   2 files changed, 37 insertions(+), 34 deletions(-)
> 
> .....
> 
>>   static inline void dw_pcie_writel_atu(struct dw_pcie *pci, u32 reg, u32 val)
>>   {
>> -	__dw_pcie_write_dbi(pci, pci->atu_base, reg, 0x4, val);
>> +	int ret;
>> +
>> +	ret = dw_pcie_write(pci->atu_base + reg, 0x4, val);
>> +	if (ret)
>> +		dev_err(pci->dev, "write ATU address failed\n");
>>   }
>>   
>>   static inline u32 dw_pcie_readl_atu(struct dw_pcie *pci, u32 reg)
>>   {
>> -	return __dw_pcie_read_dbi(pci, pci->atu_base, reg, 0x4);
>> +	int ret;
>> +	u32 val;
>> +
>> +	ret = dw_pcie_read(pci->atu_base + reg, 0x4, &val);
>> +	if (ret)
>> +		dev_err(pci->dev, "Read ATU address failed\n");
>> +
>> +	return val;
>>   }
>>   
>>   static inline void dw_pcie_dbi_ro_wr_en(struct dw_pcie *pci)
>> -- 
>> 2.17.1
>