On Tue, Dec 31, 2024 at 05:57:47PM +0100, Niklas Cassel wrote: > On Tue, Dec 31, 2024 at 06:43:39PM +0530, Manivannan Sadhasivam wrote: > > IOCTLs are supposed to return 0 for success and negative error codes for > > failure. Currently, this driver is returning 0 for failure and 1 for > > success, that's not correct. Hence, fix it! > > > > Reported-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > > Closes: https://lore.kernel.org/all/YvzNg5ROnxEApDgS@xxxxxxxxx > > Fixes: 2c156ac71c6b ("misc: Add host side PCI driver for PCI test function device") > > Reviewed-by: Damien Le Moal <dlemoal@xxxxxxxxxx> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx> > > --- > > drivers/misc/pci_endpoint_test.c | 250 +++++++++++++++---------------- > > tools/pci/pcitest.c | 51 +++---- > > 2 files changed, 148 insertions(+), 153 deletions(-) > > > > diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c > > index 5c99da952b7a..7d3f78b6f854 100644 > > --- a/drivers/misc/pci_endpoint_test.c > > +++ b/drivers/misc/pci_endpoint_test.c > > @@ -169,43 +169,47 @@ static void pci_endpoint_test_free_irq_vectors(struct pci_endpoint_test *test) > > test->irq_type = IRQ_TYPE_UNDEFINED; > > } > > > > -static bool pci_endpoint_test_alloc_irq_vectors(struct pci_endpoint_test *test, > > +static int pci_endpoint_test_alloc_irq_vectors(struct pci_endpoint_test *test, > > int type) > > { > > - int irq = -1; > > + int irq; > > struct pci_dev *pdev = test->pdev; > > struct device *dev = &pdev->dev; > > - bool res = true; > > > > switch (type) { > > case IRQ_TYPE_INTX: > > irq = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_INTX); > > - if (irq < 0) > > + if (irq < 0) { > > dev_err(dev, "Failed to get Legacy interrupt\n"); > > + return -ENOSPC; > > + } > > + > > break; > > case IRQ_TYPE_MSI: > > irq = pci_alloc_irq_vectors(pdev, 1, 32, PCI_IRQ_MSI); > > - if (irq < 0) > > + if (irq < 0) { > > dev_err(dev, "Failed to get MSI interrupts\n"); > > + return -ENOSPC; > > + } > > + > > break; > > case IRQ_TYPE_MSIX: > > irq = pci_alloc_irq_vectors(pdev, 1, 2048, PCI_IRQ_MSIX); > > - if (irq < 0) > > + if (irq < 0) { > > dev_err(dev, "Failed to get MSI-X interrupts\n"); > > + return -ENOSPC; > > From the pci_alloc_irq_vectors() kdoc: > * Return: number of allocated vectors (which might be smaller than > * @max_vecs), -ENOSPC if less than @min_vecs interrupt vectors are > * available, other errnos otherwise. > > So pci_alloc_irq_vectors() can return errors different than ENOSPC, > and in that case, you will overwrite that error. > Ack. > > > @@ -442,9 +444,12 @@ static bool pci_endpoint_test_msi_irq(struct pci_endpoint_test *test, > > val = wait_for_completion_timeout(&test->irq_raised, > > msecs_to_jiffies(1000)); > > if (!val) > > - return false; > > + return -ETIMEDOUT; > > > > - return pci_irq_vector(pdev, msi_num - 1) == test->last_irq; > > + if (!(pci_irq_vector(pdev, msi_num - 1) == test->last_irq)) > > if (pci_irq_vector(pdev, msi_num - 1) != test->last_irq) ? > > Or perhaps even: > > ret = pci_irq_vector(); > if (ret < 0) > return ret; > > if (ret != test->last_irq) > return -EIO; > Ack. > > Otherwise, this looks good to me: > Reviewed-by: Niklas Cassel <cassel@xxxxxxxxxx> Thanks! - Mani -- மணிவண்ணன் சதாசிவம்
