linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] PCI: dwc: Support remove
@ 2019-03-01  5:02 Jisheng Zhang
  2019-03-01  5:03 ` [PATCH v2 1/5] PCI: dwc: Fix dw_pcie_free_msi() if msi_irq is invalid Jisheng Zhang
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Jisheng Zhang @ 2019-03-01  5:02 UTC (permalink / raw)
  To: Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi, Bjorn Helgaas
  Cc: linux-pci, linux-kernel, linux-arm-kernel

Currently, the PCI dwc host users don't support the remove, but nothing
prevent us from supporting it. To achieve this goal, we need to ensure
we can do necessary clean up work.

Changes since v1:
  - address Bjorn's comments, I.E Capitalize, s/irq/IRQ/, s/msi/MSI/

Jisheng Zhang (5):
  PCI: dwc: Fix dw_pcie_free_msi() if msi_irq is invalid
  PCI: dwc: Free the page for MSI IRQ in dw_pcie_free_msi()
  PCI: dwc: Free MSI in the error code path of dw_pcie_host_init()
  PCI: dwc: Use devm_pci_alloc_host_bridge() to simplify the code
  PCI: dwc: Save root bus for driver remove

 .../pci/controller/dwc/pcie-designware-host.c | 38 ++++++++++---------
 drivers/pci/controller/dwc/pcie-designware.h  |  2 +
 2 files changed, 23 insertions(+), 17 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/5] PCI: dwc: Fix dw_pcie_free_msi() if msi_irq is invalid
  2019-03-01  5:02 [PATCH v2 0/5] PCI: dwc: Support remove Jisheng Zhang
@ 2019-03-01  5:03 ` Jisheng Zhang
  2019-03-13 10:05   ` Gustavo Pimentel
  2019-03-01  5:04 ` [PATCH v2 2/5] PCI: dwc: Free the page for MSI IRQ in dw_pcie_free_msi() Jisheng Zhang
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Jisheng Zhang @ 2019-03-01  5:03 UTC (permalink / raw)
  To: Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi, Bjorn Helgaas
  Cc: linux-pci, linux-kernel, linux-arm-kernel

We should check msi_irq before calling irq_set_chained_handler() and
irq_set_handler_data().

Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 0c18ab63811f..a94d3530b694 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -298,8 +298,10 @@ int dw_pcie_allocate_domains(struct pcie_port *pp)
 
 void dw_pcie_free_msi(struct pcie_port *pp)
 {
-	irq_set_chained_handler(pp->msi_irq, NULL);
-	irq_set_handler_data(pp->msi_irq, NULL);
+	if (pp->msi_irq) {
+		irq_set_chained_handler(pp->msi_irq, NULL);
+		irq_set_handler_data(pp->msi_irq, NULL);
+	}
 
 	irq_domain_remove(pp->msi_domain);
 	irq_domain_remove(pp->irq_domain);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/5] PCI: dwc: Free the page for MSI IRQ in dw_pcie_free_msi()
  2019-03-01  5:02 [PATCH v2 0/5] PCI: dwc: Support remove Jisheng Zhang
  2019-03-01  5:03 ` [PATCH v2 1/5] PCI: dwc: Fix dw_pcie_free_msi() if msi_irq is invalid Jisheng Zhang
@ 2019-03-01  5:04 ` Jisheng Zhang
  2019-03-13 10:12   ` Gustavo Pimentel
  2019-03-01  5:05 ` [PATCH v2 3/5] PCI: dwc: Free MSI in the error code path of dw_pcie_host_init() Jisheng Zhang
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Jisheng Zhang @ 2019-03-01  5:04 UTC (permalink / raw)
  To: Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi, Bjorn Helgaas
  Cc: linux-pci, linux-kernel, linux-arm-kernel

To avoid memory leak, we need to free the page for MSI in
dw_pcie_free_msi().

Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 3 +++
 drivers/pci/controller/dwc/pcie-designware.h      | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index a94d3530b694..abe3ff5f0867 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -305,6 +305,8 @@ void dw_pcie_free_msi(struct pcie_port *pp)
 
 	irq_domain_remove(pp->msi_domain);
 	irq_domain_remove(pp->irq_domain);
+
+	__free_page(pp->msi_page);
 }
 
 void dw_pcie_msi_init(struct pcie_port *pp)
@@ -322,6 +324,7 @@ void dw_pcie_msi_init(struct pcie_port *pp)
 		return;
 	}
 	msi_target = (u64)pp->msi_data;
+	pp->msi_page = page;
 
 	/* Program the msi_data */
 	dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_LO, 4,
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index f6fb65a40f10..3be7ca9f1fc3 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -179,6 +179,7 @@ struct pcie_port {
 	struct irq_domain	*irq_domain;
 	struct irq_domain	*msi_domain;
 	dma_addr_t		msi_data;
+	struct page		*msi_page;
 	u32			num_vectors;
 	u32			irq_mask[MAX_MSI_CTRLS];
 	raw_spinlock_t		lock;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/5] PCI: dwc: Free MSI in the error code path of dw_pcie_host_init()
  2019-03-01  5:02 [PATCH v2 0/5] PCI: dwc: Support remove Jisheng Zhang
  2019-03-01  5:03 ` [PATCH v2 1/5] PCI: dwc: Fix dw_pcie_free_msi() if msi_irq is invalid Jisheng Zhang
  2019-03-01  5:04 ` [PATCH v2 2/5] PCI: dwc: Free the page for MSI IRQ in dw_pcie_free_msi() Jisheng Zhang
@ 2019-03-01  5:05 ` Jisheng Zhang
  2019-03-13 10:24   ` Gustavo Pimentel
  2019-03-01  5:06 ` [PATCH v2 4/5] PCI: dwc: Use devm_pci_alloc_host_bridge() to simplify the code Jisheng Zhang
  2019-03-01  5:06 ` [PATCH v2 5/5] PCI: dwc: Save root bus for driver remove Jisheng Zhang
  4 siblings, 1 reply; 11+ messages in thread
From: Jisheng Zhang @ 2019-03-01  5:05 UTC (permalink / raw)
  To: Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi, Bjorn Helgaas
  Cc: linux-pci, linux-kernel, linux-arm-kernel

If we ever did some msi related initializations, we need to call
dw_pcie_free_msi() in the error code path.

Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index abe3ff5f0867..66569d0f3ab9 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -482,7 +482,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 	if (pp->ops->host_init) {
 		ret = pp->ops->host_init(pp);
 		if (ret)
-			goto error;
+			goto err_free_msi;
 	}
 
 	pp->root_bus_nr = pp->busn->start;
@@ -496,7 +496,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 
 	ret = pci_scan_root_bus_bridge(bridge);
 	if (ret)
-		goto error;
+		goto err_free_msi;
 
 	bus = bridge->bus;
 
@@ -512,6 +512,9 @@ int dw_pcie_host_init(struct pcie_port *pp)
 	pci_bus_add_devices(bus);
 	return 0;
 
+err_free_msi:
+	if (IS_ENABLED(CONFIG_PCI_MSI) && !pp->ops->msi_host_init)
+		dw_pcie_free_msi(pp);
 error:
 	pci_free_host_bridge(bridge);
 	return ret;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 4/5] PCI: dwc: Use devm_pci_alloc_host_bridge() to simplify the code
  2019-03-01  5:02 [PATCH v2 0/5] PCI: dwc: Support remove Jisheng Zhang
                   ` (2 preceding siblings ...)
  2019-03-01  5:05 ` [PATCH v2 3/5] PCI: dwc: Free MSI in the error code path of dw_pcie_host_init() Jisheng Zhang
@ 2019-03-01  5:06 ` Jisheng Zhang
  2019-03-13 10:37   ` Gustavo Pimentel
  2019-03-01  5:06 ` [PATCH v2 5/5] PCI: dwc: Save root bus for driver remove Jisheng Zhang
  4 siblings, 1 reply; 11+ messages in thread
From: Jisheng Zhang @ 2019-03-01  5:06 UTC (permalink / raw)
  To: Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi, Bjorn Helgaas
  Cc: linux-pci, linux-kernel, linux-arm-kernel

Use devm_pci_alloc_host_bridge() to simplify the error code path.

Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
 .../pci/controller/dwc/pcie-designware-host.c | 21 +++++++------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 66569d0f3ab9..4831c12fee93 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -357,7 +357,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 		dev_err(dev, "Missing *config* reg space\n");
 	}
 
-	bridge = pci_alloc_host_bridge(0);
+	bridge = devm_pci_alloc_host_bridge(dev, 0);
 	if (!bridge)
 		return -ENOMEM;
 
@@ -368,7 +368,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 
 	ret = devm_request_pci_bus_resources(dev, &bridge->windows);
 	if (ret)
-		goto error;
+		return ret;
 
 	/* Get the I/O and memory ranges from DT */
 	resource_list_for_each_entry_safe(win, tmp, &bridge->windows) {
@@ -412,8 +412,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 						resource_size(pp->cfg));
 		if (!pci->dbi_base) {
 			dev_err(dev, "Error with ioremap\n");
-			ret = -ENOMEM;
-			goto error;
+			return -ENOMEM;
 		}
 	}
 
@@ -424,8 +423,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 					pp->cfg0_base, pp->cfg0_size);
 		if (!pp->va_cfg0_base) {
 			dev_err(dev, "Error with ioremap in function\n");
-			ret = -ENOMEM;
-			goto error;
+			return -ENOMEM;
 		}
 	}
 
@@ -435,8 +433,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 						pp->cfg1_size);
 		if (!pp->va_cfg1_base) {
 			dev_err(dev, "Error with ioremap\n");
-			ret = -ENOMEM;
-			goto error;
+			return -ENOMEM;
 		}
 	}
 
@@ -459,14 +456,14 @@ int dw_pcie_host_init(struct pcie_port *pp)
 			    pp->num_vectors == 0) {
 				dev_err(dev,
 					"Invalid number of vectors\n");
-				goto error;
+				return -EINVAL;
 			}
 		}
 
 		if (!pp->ops->msi_host_init) {
 			ret = dw_pcie_allocate_domains(pp);
 			if (ret)
-				goto error;
+				return ret;
 
 			if (pp->msi_irq)
 				irq_set_chained_handler_and_data(pp->msi_irq,
@@ -475,7 +472,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 		} else {
 			ret = pp->ops->msi_host_init(pp);
 			if (ret < 0)
-				goto error;
+				return ret;
 		}
 	}
 
@@ -515,8 +512,6 @@ int dw_pcie_host_init(struct pcie_port *pp)
 err_free_msi:
 	if (IS_ENABLED(CONFIG_PCI_MSI) && !pp->ops->msi_host_init)
 		dw_pcie_free_msi(pp);
-error:
-	pci_free_host_bridge(bridge);
 	return ret;
 }
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 5/5] PCI: dwc: Save root bus for driver remove
  2019-03-01  5:02 [PATCH v2 0/5] PCI: dwc: Support remove Jisheng Zhang
                   ` (3 preceding siblings ...)
  2019-03-01  5:06 ` [PATCH v2 4/5] PCI: dwc: Use devm_pci_alloc_host_bridge() to simplify the code Jisheng Zhang
@ 2019-03-01  5:06 ` Jisheng Zhang
  2019-03-13 10:33   ` Gustavo Pimentel
  4 siblings, 1 reply; 11+ messages in thread
From: Jisheng Zhang @ 2019-03-01  5:06 UTC (permalink / raw)
  To: Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi, Bjorn Helgaas
  Cc: linux-pci, linux-kernel, linux-arm-kernel

Currently dwc host doesn't support the remove, but nothing prevent us
from supporting it. Save the root bus for clean up work in driver
remove code path.

After this patch, the dwc host users could implement its remove as:

static int foo_pcie_remove(struct platform_device *pdev)
{
	...
	pci_stop_root_bus(pp->root_bus);
	pci_remove_root_bus(pp->root_bus);
	dw_pcie_free_msi(pp);
	...
}

Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 1 +
 drivers/pci/controller/dwc/pcie-designware.h      | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 4831c12fee93..ca45a4471ca0 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -496,6 +496,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 		goto err_free_msi;
 
 	bus = bridge->bus;
+	pp->root_bus = bus;
 
 	if (pp->ops->scan_bus)
 		pp->ops->scan_bus(pp);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 3be7ca9f1fc3..cd92ded606c6 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -182,6 +182,7 @@ struct pcie_port {
 	struct page		*msi_page;
 	u32			num_vectors;
 	u32			irq_mask[MAX_MSI_CTRLS];
+	struct pci_bus		*root_bus;
 	raw_spinlock_t		lock;
 	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
 };
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/5] PCI: dwc: Fix dw_pcie_free_msi() if msi_irq is invalid
  2019-03-01  5:03 ` [PATCH v2 1/5] PCI: dwc: Fix dw_pcie_free_msi() if msi_irq is invalid Jisheng Zhang
@ 2019-03-13 10:05   ` Gustavo Pimentel
  0 siblings, 0 replies; 11+ messages in thread
From: Gustavo Pimentel @ 2019-03-13 10:05 UTC (permalink / raw)
  To: Jisheng Zhang, Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi,
	Bjorn Helgaas
  Cc: linux-pci, linux-kernel, linux-arm-kernel

On 01/03/2019 05:03, Jisheng Zhang wrote:
> We should check msi_irq before calling irq_set_chained_handler() and
> irq_set_handler_data().
> 
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 0c18ab63811f..a94d3530b694 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -298,8 +298,10 @@ int dw_pcie_allocate_domains(struct pcie_port *pp)
>  
>  void dw_pcie_free_msi(struct pcie_port *pp)
>  {
> -	irq_set_chained_handler(pp->msi_irq, NULL);
> -	irq_set_handler_data(pp->msi_irq, NULL);
> +	if (pp->msi_irq) {
> +		irq_set_chained_handler(pp->msi_irq, NULL);
> +		irq_set_handler_data(pp->msi_irq, NULL);
> +	}
>  
>  	irq_domain_remove(pp->msi_domain);
>  	irq_domain_remove(pp->irq_domain);
> 

Sounds good.

Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/5] PCI: dwc: Free the page for MSI IRQ in dw_pcie_free_msi()
  2019-03-01  5:04 ` [PATCH v2 2/5] PCI: dwc: Free the page for MSI IRQ in dw_pcie_free_msi() Jisheng Zhang
@ 2019-03-13 10:12   ` Gustavo Pimentel
  0 siblings, 0 replies; 11+ messages in thread
From: Gustavo Pimentel @ 2019-03-13 10:12 UTC (permalink / raw)
  To: Jisheng Zhang, Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi,
	Bjorn Helgaas
  Cc: linux-pci, linux-kernel, linux-arm-kernel

Hi,

On 01/03/2019 05:04, Jisheng Zhang wrote:
> To avoid memory leak, we need to free the page for MSI in
> dw_pcie_free_msi().
> 
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 3 +++
>  drivers/pci/controller/dwc/pcie-designware.h      | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index a94d3530b694..abe3ff5f0867 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -305,6 +305,8 @@ void dw_pcie_free_msi(struct pcie_port *pp)
>  
>  	irq_domain_remove(pp->msi_domain);
>  	irq_domain_remove(pp->irq_domain);
> +
> +	__free_page(pp->msi_page);

Perhaps you could test is msi_page is null or not before the free.

>  }
>  
>  void dw_pcie_msi_init(struct pcie_port *pp)
> @@ -322,6 +324,7 @@ void dw_pcie_msi_init(struct pcie_port *pp)
>  		return;
>  	}
>  	msi_target = (u64)pp->msi_data;
> +	pp->msi_page = page;

And maybe use the pp->msi_page variable from the beginning of the function, that
way is one less variable.

Regards,
Gustavo

>  
>  	/* Program the msi_data */
>  	dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_LO, 4,
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index f6fb65a40f10..3be7ca9f1fc3 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -179,6 +179,7 @@ struct pcie_port {
>  	struct irq_domain	*irq_domain;
>  	struct irq_domain	*msi_domain;
>  	dma_addr_t		msi_data;
> +	struct page		*msi_page;
>  	u32			num_vectors;
>  	u32			irq_mask[MAX_MSI_CTRLS];
>  	raw_spinlock_t		lock;
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/5] PCI: dwc: Free MSI in the error code path of dw_pcie_host_init()
  2019-03-01  5:05 ` [PATCH v2 3/5] PCI: dwc: Free MSI in the error code path of dw_pcie_host_init() Jisheng Zhang
@ 2019-03-13 10:24   ` Gustavo Pimentel
  0 siblings, 0 replies; 11+ messages in thread
From: Gustavo Pimentel @ 2019-03-13 10:24 UTC (permalink / raw)
  To: Jisheng Zhang, Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi,
	Bjorn Helgaas
  Cc: linux-pci, linux-kernel, linux-arm-kernel

Hi,


On 01/03/2019 05:05, Jisheng Zhang wrote:
> If we ever did some msi related initializations, we need to call
> dw_pcie_free_msi() in the error code path.
> 
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index abe3ff5f0867..66569d0f3ab9 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -482,7 +482,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  	if (pp->ops->host_init) {
>  		ret = pp->ops->host_init(pp);
>  		if (ret)
> -			goto error;
> +			goto err_free_msi;
>  	}
>  
>  	pp->root_bus_nr = pp->busn->start;
> @@ -496,7 +496,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  
>  	ret = pci_scan_root_bus_bridge(bridge);
>  	if (ret)
> -		goto error;
> +		goto err_free_msi;
>  
>  	bus = bridge->bus;
>  
> @@ -512,6 +512,9 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  	pci_bus_add_devices(bus);
>  	return 0;
>  
> +err_free_msi:
> +	if (IS_ENABLED(CONFIG_PCI_MSI) && !pp->ops->msi_host_init)

Look to Lucas Stach patch 3afc8299f39a ("PCI: dwc: skip MSI init if MSIs have
been explicitly disabled")

You need to change this to:

if (IS_ENABLED(CONFIG_PCI_MSI) && pci_msi_enabled() && !pp->ops->msi_host_init)


> +		dw_pcie_free_msi(pp);
>  error:
>  	pci_free_host_bridge(bridge);
>  	return ret;
> 

Sounds good, thanks.

Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 5/5] PCI: dwc: Save root bus for driver remove
  2019-03-01  5:06 ` [PATCH v2 5/5] PCI: dwc: Save root bus for driver remove Jisheng Zhang
@ 2019-03-13 10:33   ` Gustavo Pimentel
  0 siblings, 0 replies; 11+ messages in thread
From: Gustavo Pimentel @ 2019-03-13 10:33 UTC (permalink / raw)
  To: Jisheng Zhang, Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi,
	Bjorn Helgaas
  Cc: linux-pci, linux-kernel, linux-arm-kernel

Hi,

On 01/03/2019 05:06, Jisheng Zhang wrote:
> Currently dwc host doesn't support the remove, but nothing prevent us
> from supporting it. Save the root bus for clean up work in driver
> remove code path.
> 
> After this patch, the dwc host users could implement its remove as:
> 
> static int foo_pcie_remove(struct platform_device *pdev)
> {
> 	...
> 	pci_stop_root_bus(pp->root_bus);
> 	pci_remove_root_bus(pp->root_bus);
> 	dw_pcie_free_msi(pp);
> 	...
> }
> 
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 1 +
>  drivers/pci/controller/dwc/pcie-designware.h      | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 4831c12fee93..ca45a4471ca0 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -496,6 +496,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  		goto err_free_msi;
>  
>  	bus = bridge->bus;
> +	pp->root_bus = bus;

Why you don't use from the begging the pp->root_bus variable instead of bus
variable? That way we can remove the bus variable.

Regards,
Gustavo

>  
>  	if (pp->ops->scan_bus)
>  		pp->ops->scan_bus(pp);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 3be7ca9f1fc3..cd92ded606c6 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -182,6 +182,7 @@ struct pcie_port {
>  	struct page		*msi_page;
>  	u32			num_vectors;
>  	u32			irq_mask[MAX_MSI_CTRLS];
> +	struct pci_bus		*root_bus;
>  	raw_spinlock_t		lock;
>  	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
>  };
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 4/5] PCI: dwc: Use devm_pci_alloc_host_bridge() to simplify the code
  2019-03-01  5:06 ` [PATCH v2 4/5] PCI: dwc: Use devm_pci_alloc_host_bridge() to simplify the code Jisheng Zhang
@ 2019-03-13 10:37   ` Gustavo Pimentel
  0 siblings, 0 replies; 11+ messages in thread
From: Gustavo Pimentel @ 2019-03-13 10:37 UTC (permalink / raw)
  To: Jisheng Zhang, Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi,
	Bjorn Helgaas
  Cc: linux-pci, linux-kernel, linux-arm-kernel

Hi,

On 01/03/2019 05:06, Jisheng Zhang wrote:
> Use devm_pci_alloc_host_bridge() to simplify the error code path.
> 
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> ---
>  .../pci/controller/dwc/pcie-designware-host.c | 21 +++++++------------
>  1 file changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 66569d0f3ab9..4831c12fee93 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -357,7 +357,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  		dev_err(dev, "Missing *config* reg space\n");
>  	}
>  
> -	bridge = pci_alloc_host_bridge(0);
> +	bridge = devm_pci_alloc_host_bridge(dev, 0);
>  	if (!bridge)
>  		return -ENOMEM;
>  
> @@ -368,7 +368,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  
>  	ret = devm_request_pci_bus_resources(dev, &bridge->windows);
>  	if (ret)
> -		goto error;
> +		return ret;
>  
>  	/* Get the I/O and memory ranges from DT */
>  	resource_list_for_each_entry_safe(win, tmp, &bridge->windows) {
> @@ -412,8 +412,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  						resource_size(pp->cfg));
>  		if (!pci->dbi_base) {
>  			dev_err(dev, "Error with ioremap\n");
> -			ret = -ENOMEM;
> -			goto error;
> +			return -ENOMEM;
>  		}
>  	}
>  
> @@ -424,8 +423,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  					pp->cfg0_base, pp->cfg0_size);
>  		if (!pp->va_cfg0_base) {
>  			dev_err(dev, "Error with ioremap in function\n");
> -			ret = -ENOMEM;
> -			goto error;
> +			return -ENOMEM;
>  		}
>  	}
>  
> @@ -435,8 +433,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  						pp->cfg1_size);
>  		if (!pp->va_cfg1_base) {
>  			dev_err(dev, "Error with ioremap\n");
> -			ret = -ENOMEM;
> -			goto error;
> +			return -ENOMEM;
>  		}
>  	}
>  
> @@ -459,14 +456,14 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  			    pp->num_vectors == 0) {
>  				dev_err(dev,
>  					"Invalid number of vectors\n");
> -				goto error;
> +				return -EINVAL;
>  			}
>  		}
>  
>  		if (!pp->ops->msi_host_init) {
>  			ret = dw_pcie_allocate_domains(pp);
>  			if (ret)
> -				goto error;
> +				return ret;
>  
>  			if (pp->msi_irq)
>  				irq_set_chained_handler_and_data(pp->msi_irq,
> @@ -475,7 +472,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  		} else {
>  			ret = pp->ops->msi_host_init(pp);
>  			if (ret < 0)
> -				goto error;
> +				return ret;
>  		}
>  	}
>  
> @@ -515,8 +512,6 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  err_free_msi:
>  	if (IS_ENABLED(CONFIG_PCI_MSI) && !pp->ops->msi_host_init)
>  		dw_pcie_free_msi(pp);
> -error:
> -	pci_free_host_bridge(bridge);
>  	return ret;
>  }
>  
> 

Nice!

Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-03-13 10:38 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-01  5:02 [PATCH v2 0/5] PCI: dwc: Support remove Jisheng Zhang
2019-03-01  5:03 ` [PATCH v2 1/5] PCI: dwc: Fix dw_pcie_free_msi() if msi_irq is invalid Jisheng Zhang
2019-03-13 10:05   ` Gustavo Pimentel
2019-03-01  5:04 ` [PATCH v2 2/5] PCI: dwc: Free the page for MSI IRQ in dw_pcie_free_msi() Jisheng Zhang
2019-03-13 10:12   ` Gustavo Pimentel
2019-03-01  5:05 ` [PATCH v2 3/5] PCI: dwc: Free MSI in the error code path of dw_pcie_host_init() Jisheng Zhang
2019-03-13 10:24   ` Gustavo Pimentel
2019-03-01  5:06 ` [PATCH v2 4/5] PCI: dwc: Use devm_pci_alloc_host_bridge() to simplify the code Jisheng Zhang
2019-03-13 10:37   ` Gustavo Pimentel
2019-03-01  5:06 ` [PATCH v2 5/5] PCI: dwc: Save root bus for driver remove Jisheng Zhang
2019-03-13 10:33   ` Gustavo Pimentel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).