Re: [PATCH 0/3] PCI: designware: Fixing MSI handling flow

[Trent Piepho <tpiepho@impinj.com>]

On Wed, 2018-11-14 at 09:54 +0000, Marc Zyngier wrote:
>         /* Initialize IRQ Status array */
> -       for (ctrl = 0; ctrl < num_ctrls; ctrl++)
> -               dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE +
> +       for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
> +               dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK +
>                                         (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
> -                                   4, &pp->irq_status[ctrl]);
> +                                   4, ~0);
> +               dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE +
> +                                       (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
> +                                   4, ~0);
> +               pp->irq_status[ctrl] = 0;
> +       }
>  

I tested yesterday before this patch was sent and fixed this issue
another way.  I pretty sure this would work as well, though it's not
clear to me it's more correct.

It looks to me that the previous code was using whatever MSIs were
already enabled when the driver is initialized as the set to leave
enabled.

This looks like it changing that behavior, and instead enabling all
MSIs on initialization.

I would think the default value for a MSI should be disabled until
something enables it.

I speculated that the previous behavior was trying to work with an MSI
enabled by the bootloader, ACPI firmware, etc. that should be left
alone.  Or perhaps there was no good reason not to disable everything
on initialization and that code just got copied from somewhere else and
no one thought about it.  There's certainly evidence of that in this
driver.