| Message ID | 20171215125205.2969211-1-arnd@arndb.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 15.12.2017 15:51, Arnd Bergmann wrote: > The newly introduced driver has optional suspend/resume functions, > causing a warning when CONFIG_PM is disabled: > > drivers/gpu/drm/tegra/hub.c:749:12: error: 'tegra_display_hub_resume' defined but not used [-Werror=unused-function] > drivers/gpu/drm/tegra/hub.c:733:12: error: 'tegra_display_hub_suspend' defined but not used [-Werror=unused-function] > > This marks them __maybe_unused to shut up the warnings. > > Fixes: c4755fb9064f ("drm/tegra: Add Tegra186 display hub support") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/gpu/drm/tegra/hub.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c > index cccd44711d68..f4911feda9ff 100644 > --- a/drivers/gpu/drm/tegra/hub.c > +++ b/drivers/gpu/drm/tegra/hub.c > @@ -730,7 +730,7 @@ static int tegra_display_hub_remove(struct platform_device *pdev) > return err; > } > > -static int tegra_display_hub_suspend(struct device *dev) > +static int __maybe_unused tegra_display_hub_suspend(struct device *dev) > { > struct tegra_display_hub *hub = dev_get_drvdata(dev); > int err; > @@ -746,7 +746,7 @@ static int tegra_display_hub_suspend(struct device *dev) > return 0; > } > > -static int tegra_display_hub_resume(struct device *dev) > +static int __maybe_unused tegra_display_hub_resume(struct device *dev) > { > struct tegra_display_hub *hub = dev_get_drvdata(dev); > int err; > Probably would be better to put '#ifdef CONFIG_PM' around these functions for the consistency with the other drm/tegra files.
On Fri, Dec 15, 2017 at 2:10 PM, Dmitry Osipenko <digetx@gmail.com> wrote: > On 15.12.2017 15:51, Arnd Bergmann wrote: >> --- a/drivers/gpu/drm/tegra/hub.c >> +++ b/drivers/gpu/drm/tegra/hub.c >> @@ -730,7 +730,7 @@ static int tegra_display_hub_remove(struct platform_device *pdev) >> return err; >> } >> >> -static int tegra_display_hub_suspend(struct device *dev) >> +static int __maybe_unused tegra_display_hub_suspend(struct device *dev) >> { >> struct tegra_display_hub *hub = dev_get_drvdata(dev); >> int err; >> @@ -746,7 +746,7 @@ static int tegra_display_hub_suspend(struct device *dev) >> return 0; >> } >> >> -static int tegra_display_hub_resume(struct device *dev) >> +static int __maybe_unused tegra_display_hub_resume(struct device *dev) >> { >> struct tegra_display_hub *hub = dev_get_drvdata(dev); >> int err; >> > > Probably would be better to put '#ifdef CONFIG_PM' around these functions for > the consistency with the other drm/tegra files. I usually use __maybe_unused because it is less error-prone, the #ifdefs are often wrong, either they are around the wrong functions or they use CONFIG_PM CONFIG_PM_SLEEP interchangeably, which they are not. If you want to add a patch with the correct #ifdef instead of my patch, that's fine with me (add a Reported-by tag then), but I won't send a patch to do that myself. Arnd
On Fri, Dec 15, 2017 at 01:51:52PM +0100, Arnd Bergmann wrote: > The newly introduced driver has optional suspend/resume functions, > causing a warning when CONFIG_PM is disabled: > > drivers/gpu/drm/tegra/hub.c:749:12: error: 'tegra_display_hub_resume' defined but not used [-Werror=unused-function] > drivers/gpu/drm/tegra/hub.c:733:12: error: 'tegra_display_hub_suspend' defined but not used [-Werror=unused-function] > > This marks them __maybe_unused to shut up the warnings. > > Fixes: c4755fb9064f ("drm/tegra: Add Tegra186 display hub support") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/gpu/drm/tegra/hub.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) We had touched on this topic in a different thread. The Tegra DRM driver currently relies on runtime PM to work properly. I don't see a reason to not make that official by adding a select PM to menuconfig ARCH_TEGRA on 32-bit ARM just like we already do on 64-bit ARM. I've gone and applied this patch as an interim solution. Once we've made the switch to select PM on all generations of Tegra we can remove all the #ifdef and __maybe_unused. Thanks, Thierry
On Fri, Dec 15, 2017 at 2:33 PM, Thierry Reding <thierry.reding@gmail.com> wrote: > On Fri, Dec 15, 2017 at 01:51:52PM +0100, Arnd Bergmann wrote: >> The newly introduced driver has optional suspend/resume functions, >> causing a warning when CONFIG_PM is disabled: >> >> drivers/gpu/drm/tegra/hub.c:749:12: error: 'tegra_display_hub_resume' defined but not used [-Werror=unused-function] >> drivers/gpu/drm/tegra/hub.c:733:12: error: 'tegra_display_hub_suspend' defined but not used [-Werror=unused-function] >> >> This marks them __maybe_unused to shut up the warnings. >> >> Fixes: c4755fb9064f ("drm/tegra: Add Tegra186 display hub support") >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >> --- >> drivers/gpu/drm/tegra/hub.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) > > We had touched on this topic in a different thread. The Tegra DRM driver > currently relies on runtime PM to work properly. I don't see a reason to > not make that official by adding a select PM to menuconfig ARCH_TEGRA on > 32-bit ARM just like we already do on 64-bit ARM. > > I've gone and applied this patch as an interim solution. Once we've made > the switch to select PM on all generations of Tegra we can remove all > the #ifdef and __maybe_unused. Sounds good, thanks! Arnd
On 15.12.2017 16:33, Thierry Reding wrote: > On Fri, Dec 15, 2017 at 01:51:52PM +0100, Arnd Bergmann wrote: >> The newly introduced driver has optional suspend/resume functions, >> causing a warning when CONFIG_PM is disabled: >> >> drivers/gpu/drm/tegra/hub.c:749:12: error: 'tegra_display_hub_resume' defined but not used [-Werror=unused-function] >> drivers/gpu/drm/tegra/hub.c:733:12: error: 'tegra_display_hub_suspend' defined but not used [-Werror=unused-function] >> >> This marks them __maybe_unused to shut up the warnings. >> >> Fixes: c4755fb9064f ("drm/tegra: Add Tegra186 display hub support") >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >> --- >> drivers/gpu/drm/tegra/hub.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) > > We had touched on this topic in a different thread. The Tegra DRM driver > currently relies on runtime PM to work properly. I don't see a reason to > not make that official by adding a select PM to menuconfig ARCH_TEGRA on > 32-bit ARM just like we already do on 64-bit ARM. Since you've repeated the third time that you 'don't see a reason', I'll take it as NAK for my patch that made drm/tegra to work with the disabled PM and drop it. Next time please state it clearly if you oppose to something strongly. Thanks.
diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c index cccd44711d68..f4911feda9ff 100644 --- a/drivers/gpu/drm/tegra/hub.c +++ b/drivers/gpu/drm/tegra/hub.c @@ -730,7 +730,7 @@ static int tegra_display_hub_remove(struct platform_device *pdev) return err; } -static int tegra_display_hub_suspend(struct device *dev) +static int __maybe_unused tegra_display_hub_suspend(struct device *dev) { struct tegra_display_hub *hub = dev_get_drvdata(dev); int err; @@ -746,7 +746,7 @@ static int tegra_display_hub_suspend(struct device *dev) return 0; } -static int tegra_display_hub_resume(struct device *dev) +static int __maybe_unused tegra_display_hub_resume(struct device *dev) { struct tegra_display_hub *hub = dev_get_drvdata(dev); int err;
The newly introduced driver has optional suspend/resume functions, causing a warning when CONFIG_PM is disabled: drivers/gpu/drm/tegra/hub.c:749:12: error: 'tegra_display_hub_resume' defined but not used [-Werror=unused-function] drivers/gpu/drm/tegra/hub.c:733:12: error: 'tegra_display_hub_suspend' defined but not used [-Werror=unused-function] This marks them __maybe_unused to shut up the warnings. Fixes: c4755fb9064f ("drm/tegra: Add Tegra186 display hub support") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/gpu/drm/tegra/hub.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)