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

[Hans de Goede <hdegoede@redhat.com>]

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.

>> 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
>>