| Message ID | eba21fed997cdf148bc03cb459c12d1a32a77a8f.1547631485.git.gustavo.pimentel@synopsys.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | Improve Synopsys DesignWare Root Complex driver code | expand |
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 >
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 >>
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); };
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(-)