[PATCH 4.19 regression fix 1/2] staging: vboxvideo: Fix IRQs no longer working

[Nicholas Mc Guire]

On Tue, Sep 11, 2018 at 08:53:35AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 11-09-18 08:48, Nicholas Mc Guire wrote:
> >On Mon, Sep 10, 2018 at 08:30:38PM +0200, Hans de Goede wrote:
> >>Commit 1daddbc8dec5 ("staging: vboxvideo: Update driver to use
> >>drm_dev_register.") replaced the obsolere drm_get_pci_dev() with
> >>normal pci probe and remove functions.
> >>
> >>But the new vbox_pci_probe() is missing a pci_enable_device() call,
> >>causing interrupts to not be delivered. This causes resizes of the
> >>vm window to not get seen by the drm/kms code.
> >>
> >>This commit adds the missing pci_enable_device() call, fixing this.
> >
> >pci_enable_device is the wrapper to pci_enable_device_flags() the later
> >return < 0 on error - so while the check for if(ret) will do the right
> >think here I think it would be prefereable to explicidly use if (ret < 0)
> >as all error values pci_enable_device_flags() returns are negative.
> 
> The use of "if (ret)" checks for functions which return 0 on success
> negative value on error is a standard pattern in the kernel, so I would
> prefer to keep this as is.
>

Well as noted it does the right thing - just checking the use of 
pci_enable_device() in the existing drivers it seems that it is roughly
balanced between checking < 0 and !0. The rational for < 0 would be that
the negative return values mandate a signed type, whilc !0 does not and
if someone then uses and unsigned type the error case would return as
success while the < 0 would be detected at compile time (or other static
code checkers). 

thx!
hofrat

> >>Fixes: 1daddbc8dec5 ("staging: vboxvideo: Update driver to use ...")
> >>Cc: Fabio Rafael da Rosa <fdr at pid42.net>
> >>Cc: Nicholas Mc Guire <der.herr at hofr.at>
> >>Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> >Reviewed-by: Nicholas Mc Guire <der.herr at hofr.at>
> 
> Thanks.
> 
> Regards,
> 
> Hans
> 
> 
> >
> >>---
> >>  drivers/staging/vboxvideo/vbox_drv.c | 7 +++++++
> >>  1 file changed, 7 insertions(+)
> >>
> >>diff --git a/drivers/staging/vboxvideo/vbox_drv.c b/drivers/staging/vboxvideo/vbox_drv.c
> >>index da92c493f157..69cc508af1bc 100644
> >>--- a/drivers/staging/vboxvideo/vbox_drv.c
> >>+++ b/drivers/staging/vboxvideo/vbox_drv.c
> >>@@ -59,6 +59,11 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >>  		ret = PTR_ERR(dev);
> >>  		goto err_drv_alloc;
> >>  	}
> >>+
> >>+	ret = pci_enable_device(pdev);
> >>+	if (ret)
> >>+		goto err_pci_enable;
> >>+
> >>  	dev->pdev = pdev;
> >>  	pci_set_drvdata(pdev, dev);
> >>@@ -75,6 +80,8 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >>   err_drv_dev_register:
> >>  	vbox_driver_unload(dev);
> >>   err_vbox_driver_load:
> >>+	pci_disable_device(pdev);
> >>+ err_pci_enable:
> >>  	drm_dev_put(dev);
> >>   err_drv_alloc:
> >>  	return ret;
> >>-- 
> >>2.19.0.rc0
> >>