| Message ID | 20181022145750.25127-4-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | staging: vboxvideo: Address various review remarks | expand |
On Mon, 22 Oct 2018 at 15:58, Hans de Goede <hdegoede@redhat.com> wrote: > > Add #ifdef CONFIG_PM_SLEEP around the suspend/hibernate functions. > > Remove unnecessary #ifdef CONFIG_COMPAT, the .compat_ioctl member is > always available and if CONFIG_COMPAT is not set then drm_compat_ioctl > is defined to NULL. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/staging/vboxvideo/vbox_drv.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/vboxvideo/vbox_drv.c b/drivers/staging/vboxvideo/vbox_drv.c > index f9f4c6c2a4e9..b6e6530aa4be 100644 > --- a/drivers/staging/vboxvideo/vbox_drv.c > +++ b/drivers/staging/vboxvideo/vbox_drv.c > @@ -117,6 +117,7 @@ static void vbox_pci_remove(struct pci_dev *pdev) > drm_dev_put(&vbox->ddev); > } > > +#ifdef CONFIG_PM_SLEEP > static int vbox_pm_suspend(struct device *dev) AFAICT Arnd has been annotating these as __maybe_unused -Emil
Hi, On 22-10-18 17:42, Emil Velikov wrote: > On Mon, 22 Oct 2018 at 15:58, Hans de Goede <hdegoede@redhat.com> wrote: >> >> Add #ifdef CONFIG_PM_SLEEP around the suspend/hibernate functions. >> >> Remove unnecessary #ifdef CONFIG_COMPAT, the .compat_ioctl member is >> always available and if CONFIG_COMPAT is not set then drm_compat_ioctl >> is defined to NULL. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/staging/vboxvideo/vbox_drv.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/staging/vboxvideo/vbox_drv.c b/drivers/staging/vboxvideo/vbox_drv.c >> index f9f4c6c2a4e9..b6e6530aa4be 100644 >> --- a/drivers/staging/vboxvideo/vbox_drv.c >> +++ b/drivers/staging/vboxvideo/vbox_drv.c >> @@ -117,6 +117,7 @@ static void vbox_pci_remove(struct pci_dev *pdev) >> drm_dev_put(&vbox->ddev); >> } >> >> +#ifdef CONFIG_PM_SLEEP >> static int vbox_pm_suspend(struct device *dev) > > AFAICT Arnd has been annotating these as __maybe_unused That only works in combinations with a macro like SET_SYSTEM_SLEEP_PM_OPS(), which makes sure that we do not actually set the .suspend, etc. members of dev_pm_ops when CONFIG_PM_SLEEP is not set, so that the code actually gets compiled out. We cannot use SET_SYSTEM_SLEEP_PM_OPS here, since the suspend and hibernate paths differ. Regards, Hans
diff --git a/drivers/staging/vboxvideo/vbox_drv.c b/drivers/staging/vboxvideo/vbox_drv.c index f9f4c6c2a4e9..b6e6530aa4be 100644 --- a/drivers/staging/vboxvideo/vbox_drv.c +++ b/drivers/staging/vboxvideo/vbox_drv.c @@ -117,6 +117,7 @@ static void vbox_pci_remove(struct pci_dev *pdev) drm_dev_put(&vbox->ddev); } +#ifdef CONFIG_PM_SLEEP static int vbox_pm_suspend(struct device *dev) { struct vbox_private *vbox = dev_get_drvdata(dev); @@ -172,13 +173,16 @@ static const struct dev_pm_ops vbox_pm_ops = { .poweroff = vbox_pm_poweroff, .restore = vbox_pm_resume, }; +#endif static struct pci_driver vbox_pci_driver = { .name = DRIVER_NAME, .id_table = pciidlist, .probe = vbox_pci_probe, .remove = vbox_pci_remove, +#ifdef CONFIG_PM_SLEEP .driver.pm = &vbox_pm_ops, +#endif }; static const struct file_operations vbox_fops = { @@ -186,11 +190,9 @@ static const struct file_operations vbox_fops = { .open = drm_open, .release = drm_release, .unlocked_ioctl = drm_ioctl, + .compat_ioctl = drm_compat_ioctl, .mmap = vbox_mmap, .poll = drm_poll, -#ifdef CONFIG_COMPAT - .compat_ioctl = drm_compat_ioctl, -#endif .read = drm_read, };
Add #ifdef CONFIG_PM_SLEEP around the suspend/hibernate functions. Remove unnecessary #ifdef CONFIG_COMPAT, the .compat_ioctl member is always available and if CONFIG_COMPAT is not set then drm_compat_ioctl is defined to NULL. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/staging/vboxvideo/vbox_drv.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)