[PATCH 0/9] Improve Synopsys DesignWare Root Complex driver code

[PATCH 0/9] Improve Synopsys DesignWare Root Complex driver code

[Gustavo Pimentel]

This patch series aims to improve Synopsys DesignWare Root Complex driver
code by:
 - removing unnecessary header files
 - enforcing variable naming coherency among similar functions
 - simplifing code readability of mask/unmask function operations
 - replacing bit rotation operation (1 << bit) by BIT(bit)
 - adding a pointer null validation on dw_pcie_irq_domain_free()

No functional changes are intended.

All patches are independent so they can be easily analyzed and accepted or
discarded.

This patches organization also allows to easily merge (squash) some patches,
if desired.

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: Jingoo Han <jingoohan1@gmail.com>

Gustavo Pimentel (9):
  PCI: dwc: Remove unnecessary header include (of_gpio.h)
  PCI: dwc: Remove unnecessary header include (signal.h)
  PCI: dwc: Replace variable name from data to d on
    dw_pci_bottom_mask/unmask()
  PCI: dwc: Replace variable name from data to d on
    dw_pci_setup_msi_msg()
  PCI: dwc: Replace variable name from data to d on
    dw_pci_msi_set_affinity()
  PCI: dwc: Replace variable name from data to d on
    dw_pcie_irq_domain_free()
  PCI: dwc: Improve code readability and simplifies mask/unmask
    operations
  PCI: dwc: Replace bit rotation operation (1 << bit) by BIT(bit)
  PCI: dwc: Add pcie port pointer validation

 drivers/pci/controller/dwc/pcie-designware-host.c | 57 ++++++++++++-----------
 drivers/pci/controller/dwc/pcie-designware-plat.c |  2 -
 drivers/pci/controller/dwc/pcie-designware.h      |  2 +-
 3 files changed, 32 insertions(+), 29 deletions(-)

-- 
2.7.4

[PATCH 1/9] PCI: dwc: Remove unnecessary header include (of_gpio.h)

[Gustavo Pimentel]

Remove unnecessary header include (of_gpio.h) since it doesn't provide
any needed symbols.

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: Jingoo Han <jingoohan1@gmail.com>
---
 drivers/pci/controller/dwc/pcie-designware-plat.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-plat.c b/drivers/pci/controller/dwc/pcie-designware-plat.c
index 99ad471..e889512 100644
--- a/drivers/pci/controller/dwc/pcie-designware-plat.c
+++ b/drivers/pci/controller/dwc/pcie-designware-plat.c
@@ -13,7 +13,6 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/of_device.h>
-#include <linux/of_gpio.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
 #include <linux/resource.h>
-- 
2.7.4

[PATCH 2/9] PCI: dwc: Remove unnecessary header include (signal.h)

[Gustavo Pimentel]

Remove unnecessary header include (signal.h) since it doesn't provide
any needed symbols.

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: Jingoo Han <jingoohan1@gmail.com>
---
 drivers/pci/controller/dwc/pcie-designware-plat.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-plat.c b/drivers/pci/controller/dwc/pcie-designware-plat.c
index e889512..4272c4c 100644
--- a/drivers/pci/controller/dwc/pcie-designware-plat.c
+++ b/drivers/pci/controller/dwc/pcie-designware-plat.c
@@ -16,7 +16,6 @@
 #include <linux/pci.h>
 #include <linux/platform_device.h>
 #include <linux/resource.h>
-#include <linux/signal.h>
 #include <linux/types.h>
 #include <linux/regmap.h>
 
-- 
2.7.4

[PATCH 3/9] PCI: dwc: Replace variable name from data to d on dw_pci_bottom_mask/unmask()

[Gustavo Pimentel]

Replace variable from data to d to maintain coherency between similar
functions, such as dw_msi_mask_irq() and dw_msi_unmask_irq().

No functional change is intended.

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: Jingoo Han <jingoohan1@gmail.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 721d60a..36e284c 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -149,20 +149,20 @@ static int dw_pci_msi_set_affinity(struct irq_data *irq_data,
 	return -EINVAL;
 }
 
-static void dw_pci_bottom_mask(struct irq_data *data)
+static void dw_pci_bottom_mask(struct irq_data *d)
 {
-	struct pcie_port *pp = irq_data_get_irq_chip_data(data);
+	struct pcie_port *pp = irq_data_get_irq_chip_data(d);
 	unsigned int res, bit, ctrl;
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&pp->lock, flags);
 
 	if (pp->ops->msi_clear_irq) {
-		pp->ops->msi_clear_irq(pp, data->hwirq);
+		pp->ops->msi_clear_irq(pp, d->hwirq);
 	} else {
-		ctrl = data->hwirq / MAX_MSI_IRQS_PER_CTRL;
+		ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
 		res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
-		bit = data->hwirq % MAX_MSI_IRQS_PER_CTRL;
+		bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
 
 		pp->irq_status[ctrl] &= ~(1 << bit);
 		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4,
@@ -172,20 +172,20 @@ static void dw_pci_bottom_mask(struct irq_data *data)
 	raw_spin_unlock_irqrestore(&pp->lock, flags);
 }
 
-static void dw_pci_bottom_unmask(struct irq_data *data)
+static void dw_pci_bottom_unmask(struct irq_data *d)
 {
-	struct pcie_port *pp = irq_data_get_irq_chip_data(data);
+	struct pcie_port *pp = irq_data_get_irq_chip_data(d);
 	unsigned int res, bit, ctrl;
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&pp->lock, flags);
 
 	if (pp->ops->msi_set_irq) {
-		pp->ops->msi_set_irq(pp, data->hwirq);
+		pp->ops->msi_set_irq(pp, d->hwirq);
 	} else {
-		ctrl = data->hwirq / MAX_MSI_IRQS_PER_CTRL;
+		ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
 		res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
-		bit = data->hwirq % MAX_MSI_IRQS_PER_CTRL;
+		bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
 
 		pp->irq_status[ctrl] |= 1 << bit;
 		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4,
-- 
2.7.4

[PATCH 4/9] PCI: dwc: Replace variable name from data to d on dw_pci_setup_msi_msg()

[Gustavo Pimentel]

Replace variable from data to d to maintain coherency between similar
functions, such as dw_pci_setup_msi_msg().

No functional change is intended.

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: Jingoo Han <jingoohan1@gmail.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 36e284c..0c6e19a 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -120,9 +120,9 @@ static void dw_chained_msi_isr(struct irq_desc *desc)
 	chained_irq_exit(chip, desc);
 }
 
-static void dw_pci_setup_msi_msg(struct irq_data *data, struct msi_msg *msg)
+static void dw_pci_setup_msi_msg(struct irq_data *d, struct msi_msg *msg)
 {
-	struct pcie_port *pp = irq_data_get_irq_chip_data(data);
+	struct pcie_port *pp = irq_data_get_irq_chip_data(d);
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 	u64 msi_target;
 
@@ -135,12 +135,12 @@ static void dw_pci_setup_msi_msg(struct irq_data *data, struct msi_msg *msg)
 	msg->address_hi = upper_32_bits(msi_target);
 
 	if (pp->ops->get_msi_data)
-		msg->data = pp->ops->get_msi_data(pp, data->hwirq);
+		msg->data = pp->ops->get_msi_data(pp, d->hwirq);
 	else
-		msg->data = data->hwirq;
+		msg->data = d->hwirq;
 
 	dev_dbg(pci->dev, "msi#%d address_hi %#x address_lo %#x\n",
-		(int)data->hwirq, msg->address_hi, msg->address_lo);
+		(int)d->hwirq, msg->address_hi, msg->address_lo);
 }
 
 static int dw_pci_msi_set_affinity(struct irq_data *irq_data,
-- 
2.7.4

[PATCH 5/9] PCI: dwc: Replace variable name from data to d on dw_pci_msi_set_affinity()

[Gustavo Pimentel]

Replace variable from data to d to maintain coherency between similar
functions.

No functional change is intended.

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: Jingoo Han <jingoohan1@gmail.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 0c6e19a..a59fc17e 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -143,7 +143,7 @@ static void dw_pci_setup_msi_msg(struct irq_data *d, struct msi_msg *msg)
 		(int)d->hwirq, msg->address_hi, msg->address_lo);
 }
 
-static int dw_pci_msi_set_affinity(struct irq_data *irq_data,
+static int dw_pci_msi_set_affinity(struct irq_data *d,
 				   const struct cpumask *mask, bool force)
 {
 	return -EINVAL;
-- 
2.7.4

[PATCH 6/9] PCI: dwc: Replace variable name from data to d on dw_pcie_irq_domain_free()

[Gustavo Pimentel]

Replace variable from data to d to maintain coherency between similar
functions.

No functional change is intended.

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: Jingoo Han <jingoohan1@gmail.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index a59fc17e..768e16a 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -255,13 +255,13 @@ static int dw_pcie_irq_domain_alloc(struct irq_domain *domain,
 static void dw_pcie_irq_domain_free(struct irq_domain *domain,
 				    unsigned int virq, unsigned int nr_irqs)
 {
-	struct irq_data *data = irq_domain_get_irq_data(domain, virq);
-	struct pcie_port *pp = irq_data_get_irq_chip_data(data);
+	struct irq_data *d = irq_domain_get_irq_data(domain, virq);
+	struct pcie_port *pp = irq_data_get_irq_chip_data(d);
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&pp->lock, flags);
 
-	bitmap_release_region(pp->msi_irq_in_use, data->hwirq,
+	bitmap_release_region(pp->msi_irq_in_use, d->hwirq,
 			      order_base_2(nr_irqs));
 
 	raw_spin_unlock_irqrestore(&pp->lock, flags);
-- 
2.7.4

[PATCH 7/9] PCI: dwc: Improve code readability and simplifies mask/unmask operations

[Gustavo Pimentel]

Improve code readability and simplifies mask/unmask operations by
inverting the applied logic (no functional change is intended).

Replace variable name from irq_status to irq_mask, since its goal is to
keep track of which interuptions are mask or not.

Replace bit rotation operation (1 << bit) by BIT(bit), which simplifies
code reading.

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: Jingoo Han <jingoohan1@gmail.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 12 ++++++------
 drivers/pci/controller/dwc/pcie-designware.h      |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 768e16a..d53e6f7 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -164,9 +164,9 @@ static void dw_pci_bottom_mask(struct irq_data *d)
 		res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
 		bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
 
-		pp->irq_status[ctrl] &= ~(1 << bit);
+		pp->irq_mask[ctrl] |= BIT(bit);
 		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4,
-				    ~pp->irq_status[ctrl]);
+				    pp->irq_mask[ctrl]);
 	}
 
 	raw_spin_unlock_irqrestore(&pp->lock, flags);
@@ -187,9 +187,9 @@ static void dw_pci_bottom_unmask(struct irq_data *d)
 		res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
 		bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
 
-		pp->irq_status[ctrl] |= 1 << bit;
+		pp->irq_mask[ctrl] &= ~BIT(bit);
 		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4,
-				    ~pp->irq_status[ctrl]);
+				    pp->irq_mask[ctrl]);
 	}
 
 	raw_spin_unlock_irqrestore(&pp->lock, flags);
@@ -665,13 +665,13 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
 
 	/* Initialize IRQ Status array */
 	for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
+		pp->irq_mask[ctrl] = ~0;
 		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK +
 					(ctrl * MSI_REG_CTRL_BLOCK_SIZE),
-				    4, ~0);
+				    4, pp->irq_mask[ctrl]);
 		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE +
 					(ctrl * MSI_REG_CTRL_BLOCK_SIZE),
 				    4, ~0);
-		pp->irq_status[ctrl] = 0;
 	}
 
 	/* Setup RC BARs */
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 9943d8c..2790002 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -177,7 +177,7 @@ struct pcie_port {
 	struct irq_domain	*msi_domain;
 	dma_addr_t		msi_data;
 	u32			num_vectors;
-	u32			irq_status[MAX_MSI_CTRLS];
+	u32			irq_mask[MAX_MSI_CTRLS];
 	raw_spinlock_t		lock;
 	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
 };
-- 
2.7.4

[PATCH 8/9] PCI: dwc: Replace bit rotation operation (1 << bit) by BIT(bit)

[Gustavo Pimentel]

Replace bit rotation operation (1 << bit) by BIT(bit), which simplifies
code reading.

No functional change is intended.

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: Jingoo Han <jingoohan1@gmail.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index d53e6f7..45ff5e4 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -207,7 +207,7 @@ static void dw_pci_bottom_ack(struct irq_data *d)
 
 	raw_spin_lock_irqsave(&pp->lock, flags);
 
-	dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS + res, 4, 1 << bit);
+	dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS + res, 4, BIT(bit));
 
 	if (pp->ops->msi_irq_ack)
 		pp->ops->msi_irq_ack(d->hwirq, pp);
-- 
2.7.4

[PATCH 9/9] PCI: dwc: Add pcie port pointer validation

[Gustavo Pimentel]

Add pcie port pointer validation to avoid a potencial crash accessing a
null pointer reported by coverity scan.

Warning generated by coverity: "Dereferencing a pointer that might be
null "data" when calling "irq_data_get_irq_chip_data".

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: Jingoo Han <jingoohan1@gmail.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 45ff5e4..419a56b 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -256,9 +256,14 @@ static void dw_pcie_irq_domain_free(struct irq_domain *domain,
 				    unsigned int virq, unsigned int nr_irqs)
 {
 	struct irq_data *d = irq_domain_get_irq_data(domain, virq);
-	struct pcie_port *pp = irq_data_get_irq_chip_data(d);
+	struct pcie_port *pp;
 	unsigned long flags;
 
+	if (!d)
+		return;
+
+	pp = irq_data_get_irq_chip_data(d);
+
 	raw_spin_lock_irqsave(&pp->lock, flags);
 
 	bitmap_release_region(pp->msi_irq_in_use, d->hwirq,
-- 
2.7.4

Re: [PATCH 3/9] PCI: dwc: Replace variable name from data to d on dw_pci_bottom_mask/unmask()

[Lorenzo Pieralisi]

On Wed, Jan 16, 2019 at 11:14:16AM +0100, Gustavo Pimentel wrote:
> Replace variable from data to d to maintain coherency between similar

s/Replace/Rename
s/coherency/consistency

s/similar/driver

These comments are valid for other patches in this series too.

> functions, such as dw_msi_mask_irq() and dw_msi_unmask_irq().
> 
> No functional change is intended.
> 
> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Joao Pinto <jpinto@synopsys.com>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 721d60a..36e284c 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -149,20 +149,20 @@ static int dw_pci_msi_set_affinity(struct irq_data *irq_data,
>  	return -EINVAL;
>  }
>  
> -static void dw_pci_bottom_mask(struct irq_data *data)
> +static void dw_pci_bottom_mask(struct irq_data *d)
>  {
> -	struct pcie_port *pp = irq_data_get_irq_chip_data(data);
> +	struct pcie_port *pp = irq_data_get_irq_chip_data(d);
>  	unsigned int res, bit, ctrl;
>  	unsigned long flags;
>  
>  	raw_spin_lock_irqsave(&pp->lock, flags);
>  
>  	if (pp->ops->msi_clear_irq) {
> -		pp->ops->msi_clear_irq(pp, data->hwirq);
> +		pp->ops->msi_clear_irq(pp, d->hwirq);
>  	} else {
> -		ctrl = data->hwirq / MAX_MSI_IRQS_PER_CTRL;
> +		ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
>  		res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
> -		bit = data->hwirq % MAX_MSI_IRQS_PER_CTRL;
> +		bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
>  
>  		pp->irq_status[ctrl] &= ~(1 << bit);
>  		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4,
> @@ -172,20 +172,20 @@ static void dw_pci_bottom_mask(struct irq_data *data)
>  	raw_spin_unlock_irqrestore(&pp->lock, flags);
>  }
>  
> -static void dw_pci_bottom_unmask(struct irq_data *data)
> +static void dw_pci_bottom_unmask(struct irq_data *d)
>  {
> -	struct pcie_port *pp = irq_data_get_irq_chip_data(data);
> +	struct pcie_port *pp = irq_data_get_irq_chip_data(d);
>  	unsigned int res, bit, ctrl;
>  	unsigned long flags;
>  
>  	raw_spin_lock_irqsave(&pp->lock, flags);
>  
>  	if (pp->ops->msi_set_irq) {
> -		pp->ops->msi_set_irq(pp, data->hwirq);
> +		pp->ops->msi_set_irq(pp, d->hwirq);
>  	} else {
> -		ctrl = data->hwirq / MAX_MSI_IRQS_PER_CTRL;
> +		ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
>  		res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
> -		bit = data->hwirq % MAX_MSI_IRQS_PER_CTRL;
> +		bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
>  
>  		pp->irq_status[ctrl] |= 1 << bit;
>  		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4,
> -- 
> 2.7.4
> 

Re: [PATCH 7/9] PCI: dwc: Improve code readability and simplifies mask/unmask operations

[Lorenzo Pieralisi]

On Wed, Jan 16, 2019 at 11:14:20AM +0100, Gustavo Pimentel wrote:
> Improve code readability and simplifies mask/unmask operations by
> inverting the applied logic (no functional change is intended).
> 
> Replace variable name from irq_status to irq_mask, since its goal is to
> keep track of which interuptions are mask or not.
> 
> Replace bit rotation operation (1 << bit) by BIT(bit), which simplifies
> code reading.

Two changes, two patches, I know it is tempting to squash trivial
changes in one patch but logically that's not correct.

Lorenzo

> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Joao Pinto <jpinto@synopsys.com>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 12 ++++++------
>  drivers/pci/controller/dwc/pcie-designware.h      |  2 +-
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 768e16a..d53e6f7 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -164,9 +164,9 @@ static void dw_pci_bottom_mask(struct irq_data *d)
>  		res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
>  		bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
>  
> -		pp->irq_status[ctrl] &= ~(1 << bit);
> +		pp->irq_mask[ctrl] |= BIT(bit);
>  		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4,
> -				    ~pp->irq_status[ctrl]);
> +				    pp->irq_mask[ctrl]);
>  	}
>  
>  	raw_spin_unlock_irqrestore(&pp->lock, flags);
> @@ -187,9 +187,9 @@ static void dw_pci_bottom_unmask(struct irq_data *d)
>  		res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
>  		bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
>  
> -		pp->irq_status[ctrl] |= 1 << bit;
> +		pp->irq_mask[ctrl] &= ~BIT(bit);
>  		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4,
> -				    ~pp->irq_status[ctrl]);
> +				    pp->irq_mask[ctrl]);
>  	}
>  
>  	raw_spin_unlock_irqrestore(&pp->lock, flags);
> @@ -665,13 +665,13 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
>  
>  	/* Initialize IRQ Status array */
>  	for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
> +		pp->irq_mask[ctrl] = ~0;
>  		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK +
>  					(ctrl * MSI_REG_CTRL_BLOCK_SIZE),
> -				    4, ~0);
> +				    4, pp->irq_mask[ctrl]);
>  		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE +
>  					(ctrl * MSI_REG_CTRL_BLOCK_SIZE),
>  				    4, ~0);
> -		pp->irq_status[ctrl] = 0;
>  	}
>  
>  	/* Setup RC BARs */
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 9943d8c..2790002 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -177,7 +177,7 @@ struct pcie_port {
>  	struct irq_domain	*msi_domain;
>  	dma_addr_t		msi_data;
>  	u32			num_vectors;
> -	u32			irq_status[MAX_MSI_CTRLS];
> +	u32			irq_mask[MAX_MSI_CTRLS];
>  	raw_spinlock_t		lock;
>  	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
>  };
> -- 
> 2.7.4
> 

Re: [PATCH 9/9] PCI: dwc: Add pcie port pointer validation

[Lorenzo Pieralisi]

[+Marc, for his information]

On Wed, Jan 16, 2019 at 11:14:22AM +0100, Gustavo Pimentel wrote:
> Add pcie port pointer validation to avoid a potencial crash accessing a
> null pointer reported by coverity scan.

s/potencial/potential
s/null/NULL

> Warning generated by coverity: "Dereferencing a pointer that might be
> null "data" when calling "irq_data_get_irq_chip_data".

s/null/NULL

> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Joao Pinto <jpinto@synopsys.com>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 45ff5e4..419a56b 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -256,9 +256,14 @@ static void dw_pcie_irq_domain_free(struct irq_domain *domain,
>  				    unsigned int virq, unsigned int nr_irqs)
>  {
>  	struct irq_data *d = irq_domain_get_irq_data(domain, virq);
> -	struct pcie_port *pp = irq_data_get_irq_chip_data(d);
> +	struct pcie_port *pp;
>  	unsigned long flags;
>  
> +	if (!d)
> +		return;
> +
> +	pp = irq_data_get_irq_chip_data(d);

Well, we do not validate pp either and I wonder whether coverity trips
over this code path but that's just a false positive.

If we have to actually patch it there are other code paths in the kernel
to be updated too.

Lorenzo

> +
>  	raw_spin_lock_irqsave(&pp->lock, flags);
>  
>  	bitmap_release_region(pp->msi_irq_in_use, d->hwirq,
> -- 
> 2.7.4
> 

Re: [PATCH 9/9] PCI: dwc: Add pcie port pointer validation

[Gustavo Pimentel]

On 31/01/2019 16:51, Lorenzo Pieralisi wrote:
> [+Marc, for his information]
> 
> On Wed, Jan 16, 2019 at 11:14:22AM +0100, Gustavo Pimentel wrote:
>> Add pcie port pointer validation to avoid a potencial crash accessing a
>> null pointer reported by coverity scan.
> 
> s/potencial/potential
> s/null/NULL
> 
>> Warning generated by coverity: "Dereferencing a pointer that might be
>> null "data" when calling "irq_data_get_irq_chip_data".
> 
> s/null/NULL
> 
>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Cc: Joao Pinto <jpinto@synopsys.com>
>> Cc: Jingoo Han <jingoohan1@gmail.com>
>> ---
>>  drivers/pci/controller/dwc/pcie-designware-host.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
>> index 45ff5e4..419a56b 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
>> @@ -256,9 +256,14 @@ static void dw_pcie_irq_domain_free(struct irq_domain *domain,
>>  				    unsigned int virq, unsigned int nr_irqs)
>>  {
>>  	struct irq_data *d = irq_domain_get_irq_data(domain, virq);
>> -	struct pcie_port *pp = irq_data_get_irq_chip_data(d);
>> +	struct pcie_port *pp;
>>  	unsigned long flags;
>>  
>> +	if (!d)
>> +		return;
>> +
>> +	pp = irq_data_get_irq_chip_data(d);
> 
> Well, we do not validate pp either and I wonder whether coverity trips
> over this code path but that's just a false positive.
> 
> If we have to actually patch it there are other code paths in the kernel
> to be updated too.

Ok, I can drop this patch.

Gustavo

> 
> Lorenzo
> 
>> +
>>  	raw_spin_lock_irqsave(&pp->lock, flags);
>>  
>>  	bitmap_release_region(pp->msi_irq_in_use, d->hwirq,
>> -- 
>> 2.7.4
>>

Re: [PATCH 7/9] PCI: dwc: Improve code readability and simplifies mask/unmask operations

[Gustavo Pimentel]

On 31/01/2019 16:22, Lorenzo Pieralisi wrote:
> On Wed, Jan 16, 2019 at 11:14:20AM +0100, Gustavo Pimentel wrote:
>> Improve code readability and simplifies mask/unmask operations by
>> inverting the applied logic (no functional change is intended).
>>
>> Replace variable name from irq_status to irq_mask, since its goal is to
>> keep track of which interuptions are mask or not.
>>
>> Replace bit rotation operation (1 << bit) by BIT(bit), which simplifies
>> code reading.
> 
> Two changes, two patches, I know it is tempting to squash trivial
> changes in one patch but logically that's not correct.

Ok, I'll move 2 replacement bit operation to the next patch since its goal is
only that.
Thanks

Gustavo

> 
> Lorenzo
> 
>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Cc: Joao Pinto <jpinto@synopsys.com>
>> Cc: Jingoo Han <jingoohan1@gmail.com>
>> ---
>>  drivers/pci/controller/dwc/pcie-designware-host.c | 12 ++++++------
>>  drivers/pci/controller/dwc/pcie-designware.h      |  2 +-
>>  2 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
>> index 768e16a..d53e6f7 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
>> @@ -164,9 +164,9 @@ static void dw_pci_bottom_mask(struct irq_data *d)
>>  		res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
>>  		bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
>>  
>> -		pp->irq_status[ctrl] &= ~(1 << bit);
>> +		pp->irq_mask[ctrl] |= BIT(bit);
>>  		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4,
>> -				    ~pp->irq_status[ctrl]);
>> +				    pp->irq_mask[ctrl]);
>>  	}
>>  
>>  	raw_spin_unlock_irqrestore(&pp->lock, flags);
>> @@ -187,9 +187,9 @@ static void dw_pci_bottom_unmask(struct irq_data *d)
>>  		res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
>>  		bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
>>  
>> -		pp->irq_status[ctrl] |= 1 << bit;
>> +		pp->irq_mask[ctrl] &= ~BIT(bit);
>>  		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4,
>> -				    ~pp->irq_status[ctrl]);
>> +				    pp->irq_mask[ctrl]);
>>  	}
>>  
>>  	raw_spin_unlock_irqrestore(&pp->lock, flags);
>> @@ -665,13 +665,13 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
>>  
>>  	/* Initialize IRQ Status array */
>>  	for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
>> +		pp->irq_mask[ctrl] = ~0;
>>  		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK +
>>  					(ctrl * MSI_REG_CTRL_BLOCK_SIZE),
>> -				    4, ~0);
>> +				    4, pp->irq_mask[ctrl]);
>>  		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE +
>>  					(ctrl * MSI_REG_CTRL_BLOCK_SIZE),
>>  				    4, ~0);
>> -		pp->irq_status[ctrl] = 0;
>>  	}
>>  
>>  	/* Setup RC BARs */
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
>> index 9943d8c..2790002 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.h
>> +++ b/drivers/pci/controller/dwc/pcie-designware.h
>> @@ -177,7 +177,7 @@ struct pcie_port {
>>  	struct irq_domain	*msi_domain;
>>  	dma_addr_t		msi_data;
>>  	u32			num_vectors;
>> -	u32			irq_status[MAX_MSI_CTRLS];
>> +	u32			irq_mask[MAX_MSI_CTRLS];
>>  	raw_spinlock_t		lock;
>>  	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
>>  };
>> -- 
>> 2.7.4
>>