On Wed, Sep 20, 2017 at 04:21:44PM +0200, Niklas Cassel wrote:
> On 09/19/2017 09:25 PM, Bjorn Helgaas wrote:
> > On Wed, Sep 06, 2017 at 02:55:24PM +0100, John Keeping wrote:
> >> pci_epf_test_raise_irq() reads the interrupt to use for the response
> >> from reg->command, but this has been cleared at the beginning of the
> >> command handler so the value is always zero at this point.
> >>
> >> Instead, extract the interrupt index before handling the command and
> >> then pass the requested interrupt into pci_epf_test_raise_irq(). This
> >> allows us to remove the specific code to extract the interrupt for
> >> COMMAND_RAISE_MSI_IRQ since it is now handled in common code.
> >>
> >> Signed-off-by: John Keeping <john@xxxxxxxxxxxx>
> >
> > Applied with Kishon's ack to pci/endpoint for v4.15, thanks!
>
> Hello PCI hackers!
>
> I actually think that this patch should be applied to v4.14,
> since it actually fixes a regression introduced by commit
> 3ecf3232c54c ("PCI: endpoint: Do not reset *command* inadvertently")
> which is included in v4.14-rc1.
>
> It would have been nice if this patch actually had a fixes-tag.
Agreed. I added it and moved the patch to for-linus for v4.14.
Thanks a lot, Niklas!
> (I cannot do any read/write/copy tests if I include 3ecf3232c54c,
> but if I include 3ecf3232c54c + this patch, all tests work again.)
>
> Regards,
> Niklas
>
> >
> >> ---
> >> drivers/pci/endpoint/functions/pci-epf-test.c | 13 ++++++-------
> >> 1 file changed, 6 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> >> index 4ddc6e8f9fe7..f9308c2f22e6 100644
> >> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> >> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> >> @@ -251,9 +251,8 @@ static int pci_epf_test_write(struct pci_epf_test *epf_test)
> >> return ret;
> >> }
> >>
> >> -static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test)
> >> +static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test, u8 irq)
> >> {
> >> - u8 irq;
> >> u8 msi_count;
> >> struct pci_epf *epf = epf_test->epf;
> >> struct pci_epc *epc = epf->epc;
> >> @@ -262,7 +261,6 @@ static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test)
> >>
> >> reg->status |= STATUS_IRQ_RAISED;
> >> msi_count = pci_epc_get_msi(epc);
> >> - irq = (reg->command & MSI_NUMBER_MASK) >> MSI_NUMBER_SHIFT;
> >> if (irq > msi_count || msi_count <= 0)
> >> pci_epc_raise_irq(epc, PCI_EPC_IRQ_LEGACY, 0);
> >> else
> >> @@ -289,6 +287,8 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
> >> reg->command = 0;
> >> reg->status = 0;
> >>
> >> + irq = (command & MSI_NUMBER_MASK) >> MSI_NUMBER_SHIFT;
> >> +
> >> if (command & COMMAND_RAISE_LEGACY_IRQ) {
> >> reg->status = STATUS_IRQ_RAISED;
> >> pci_epc_raise_irq(epc, PCI_EPC_IRQ_LEGACY, 0);
> >> @@ -301,7 +301,7 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
> >> reg->status |= STATUS_WRITE_FAIL;
> >> else
> >> reg->status |= STATUS_WRITE_SUCCESS;
> >> - pci_epf_test_raise_irq(epf_test);
> >> + pci_epf_test_raise_irq(epf_test, irq);
> >> goto reset_handler;
> >> }
> >>
> >> @@ -311,7 +311,7 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
> >> reg->status |= STATUS_READ_SUCCESS;
> >> else
> >> reg->status |= STATUS_READ_FAIL;
> >> - pci_epf_test_raise_irq(epf_test);
> >> + pci_epf_test_raise_irq(epf_test, irq);
> >> goto reset_handler;
> >> }
> >>
> >> @@ -321,13 +321,12 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
> >> reg->status |= STATUS_COPY_SUCCESS;
> >> else
> >> reg->status |= STATUS_COPY_FAIL;
> >> - pci_epf_test_raise_irq(epf_test);
> >> + pci_epf_test_raise_irq(epf_test, irq);
> >> goto reset_handler;
> >> }
> >>
> >> if (command & COMMAND_RAISE_MSI_IRQ) {
> >> msi_count = pci_epc_get_msi(epc);
> >> - irq = (command & MSI_NUMBER_MASK) >> MSI_NUMBER_SHIFT;
> >> if (irq > msi_count || msi_count <= 0)
> >> goto reset_handler;
> >> reg->status = STATUS_IRQ_RAISED;
> >> --
> >> 2.14.1
> >>
