On 22/11/2018 16:26, Lorenzo Pieralisi wrote:
> On Thu, Nov 22, 2018 at 12:03:25PM +0000, Gustavo Pimentel wrote:
[...]
>> Just a couple of suggestions Lorenzo, maybe you could exchange the *designware*
>> by *dwc* on all patch series titles and on eca44651920c("PCI: designware: Move
>> interrupt acking into the proper callback") replace *acking* by *ACKing* like
>> previous patch has.
>>
>> Marc thanks for this patch fix! :)
>>
>> Tested-by: Gustavo Pimentel <gustavo.pimentel@xxxxxxxxxxxx>
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c
>> b/drivers/pci/controller/dwc/pcie-designware-host.c
>> index 0fa9e8f..a5132b3 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 *data)
>> res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
>> bit = data->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,30 +187,30 @@ static void dw_pci_bottom_unmask(struct irq_data *data)
>> res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
>> bit = data->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);
>> }
>>
>> -static void dw_pci_bottom_ack(struct irq_data *d)
>> +static void dw_pci_bottom_ack(struct irq_data *data)
>> {
>> - struct pcie_port *pp = irq_data_get_irq_chip_data(d);
>> + struct pcie_port *pp = irq_data_get_irq_chip_data(data);
>> unsigned int res, bit, ctrl;
>> unsigned long flags;
>>
>> - ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
>> + ctrl = data->hwirq / MAX_MSI_IRQS_PER_CTRL;
>> res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
>> - bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
>> + bit = data->hwirq % MAX_MSI_IRQS_PER_CTRL;
>>
>> 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);
>> + pp->ops->msi_irq_ack(data->hwirq, pp);
>
> Changes in this hunk are unrelated, I won't squash them in.
To add to Lorenzo's comment, we're trying hard to have a *minimal* fix
that can be easily backported. Changing variable and field names as well
as flipping the semantic of other bits of the driver makes it harder to
review, and certainly doesn't help getting things backported to stable
(see the stable kernel rules).
I'd suggest this kind of repainting is better kept as a separate patch
and merged separately.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
