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@pid42.net Cc: Nicholas Mc Guire der.herr@hofr.at Signed-off-by: Hans de Goede hdegoede@redhat.com
Reviewed-by: Nicholas Mc Guire der.herr@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