On Mon, 26 Nov 2018 15:52:42 +0000, Trent Piepho <tpiepho@xxxxxxxxxx> wrote: > > On Thu, 2018-11-22 at 12:03 +0000, Gustavo Pimentel wrote: > > > > > I just realised (at 1am!) that the first patch is awfully buggy: > > > - ENABLE and MASK have opposite polarities > > > - There is nothing initialising the ENABLE and MASK registers > > > > > > I've stashed the following fix-up on top of the existing series: > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c > > > b/drivers/pci/controller/dwc/pcie-designware-host.c > > > index f06e67c60593..0fa9e8fdce66 100644 > > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > > > @@ -166,7 +166,7 @@ static void dw_pci_bottom_mask(struct irq_data > > > *data) > > > > (snip) > > > > I used your patch and made it more perceptible in my opinion, (basically I > > transformed the variable irq_mask (previous irq_status) into a mirror of MASK > > registers, which removed the need for the *NOT* operation and forced the mask > > operation swap in the callbacks) > > This would be the patch that enables all MSI interrupts on driver > initialization? > > I don't think that's a good idea. I was under the impression Marc > thought that as well. It would be better to enable them when they are > enabled, via enable and disable methods. What gain does this bring? Frankly, I see exactly zero advantage of doing so. It may look cosmetically appealing in the sense that it makes use of of a register that the IP offers, but for Linux the advantage is basically null. Thanks, M. -- Jazz is not dead, it just smell funny.
