Message ID | 20190426145617.2342824-1-arnd@arndb.de |
---|---|
State | Accepted |
Commit | 6d6165603e162e3d391f35853a4ab232ab0db064 |
Headers | show |
Series | amba: tegra-ahb: mark PM functions as __maybe_unused | expand |
On Fri, Apr 26, 2019 at 04:56:03PM +0200, Arnd Bergmann wrote: > clang warns about an unused variable when CONFIG_PM is disabled, > since it is only referenced from an #ifdef: > > drivers/amba/tegra-ahb.c:97:18: error: variable 'tegra_ahb_gizmo' is not needed and will not be emitted [-Werror,-Wunneeded-internal-declaration] > > Rather than trying to get the #ifdef right, remove it and > use __maybe_unused here, which is less error prone. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/amba/tegra-ahb.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) Shouldn't tegra_ahb_gizmo have the same annotation, then? I do see that it's "used" in tegra_ahb_probe() as part of an ARRAY_SIZE() expression, but that technically doesn't mean that it would need to be emitted, but it might be enough to shut up clang? Or does the __maybe_unused not get propagated? Looking at the code, I guess we could save a bit of memory if we didn't allocate memory for the zero-length "ctx" array for !PM since that's no longer needed. But then again, we've recently changed 32-bit Tegra to forcefully enable PM, just like we did for 64-bit Tegra, so that's moot anyway. Do you want me to pick this up, or would you rather stash it into ARM SoC directly? If the latter: Acked-by: Thierry Reding <treding@nvidia.com> > diff --git a/drivers/amba/tegra-ahb.c b/drivers/amba/tegra-ahb.c > index b0b688c481e8..3751d811be39 100644 > --- a/drivers/amba/tegra-ahb.c > +++ b/drivers/amba/tegra-ahb.c > @@ -170,8 +170,7 @@ int tegra_ahb_enable_smmu(struct device_node *dn) > EXPORT_SYMBOL(tegra_ahb_enable_smmu); > #endif > > -#ifdef CONFIG_PM > -static int tegra_ahb_suspend(struct device *dev) > +static int __maybe_unused tegra_ahb_suspend(struct device *dev) > { > int i; > struct tegra_ahb *ahb = dev_get_drvdata(dev); > @@ -181,7 +180,7 @@ static int tegra_ahb_suspend(struct device *dev) > return 0; > } > > -static int tegra_ahb_resume(struct device *dev) > +static int __maybe_unused tegra_ahb_resume(struct device *dev) > { > int i; > struct tegra_ahb *ahb = dev_get_drvdata(dev); > @@ -190,7 +189,6 @@ static int tegra_ahb_resume(struct device *dev) > gizmo_writel(ahb, ahb->ctx[i], tegra_ahb_gizmo[i]); > return 0; > } > -#endif > > static UNIVERSAL_DEV_PM_OPS(tegra_ahb_pm, > tegra_ahb_suspend, > -- > 2.20.0 >
On Fri, Apr 26, 2019 at 5:20 PM Thierry Reding <thierry.reding@gmail.com> wrote: > > On Fri, Apr 26, 2019 at 04:56:03PM +0200, Arnd Bergmann wrote: > > clang warns about an unused variable when CONFIG_PM is disabled, > > since it is only referenced from an #ifdef: > > > > drivers/amba/tegra-ahb.c:97:18: error: variable 'tegra_ahb_gizmo' is not needed and will not be emitted [-Werror,-Wunneeded-internal-declaration] > > > > Rather than trying to get the #ifdef right, remove it and > > use __maybe_unused here, which is less error prone. > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > --- > > drivers/amba/tegra-ahb.c | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > Shouldn't tegra_ahb_gizmo have the same annotation, then? I do see that > it's "used" in tegra_ahb_probe() as part of an ARRAY_SIZE() expression, > but that technically doesn't mean that it would need to be emitted, but > it might be enough to shut up clang? It would be enough to annotated tegra_ahb_gizmo as __maybe_unused, but I don't like the idea of using both __maybe_unused and #ifdef to deal with the CONFIG_PM issue, so I was try to make this more consistent. > Or does the __maybe_unused not get propagated? > > Looking at the code, I guess we could save a bit of memory if we didn't > allocate memory for the zero-length "ctx" array for !PM since that's no > longer needed. But then again, we've recently changed 32-bit Tegra to > forcefully enable PM, just like we did for 64-bit Tegra, so that's moot > anyway. Right > Do you want me to pick this up, or would you rather stash it into ARM > SoC directly? If the latter: Olof is currently doing the merges and I forgot to Cc him, so I'd prefer if you could forward the patch to him or include it in a later pull request. Arnd
diff --git a/drivers/amba/tegra-ahb.c b/drivers/amba/tegra-ahb.c index b0b688c481e8..3751d811be39 100644 --- a/drivers/amba/tegra-ahb.c +++ b/drivers/amba/tegra-ahb.c @@ -170,8 +170,7 @@ int tegra_ahb_enable_smmu(struct device_node *dn) EXPORT_SYMBOL(tegra_ahb_enable_smmu); #endif -#ifdef CONFIG_PM -static int tegra_ahb_suspend(struct device *dev) +static int __maybe_unused tegra_ahb_suspend(struct device *dev) { int i; struct tegra_ahb *ahb = dev_get_drvdata(dev); @@ -181,7 +180,7 @@ static int tegra_ahb_suspend(struct device *dev) return 0; } -static int tegra_ahb_resume(struct device *dev) +static int __maybe_unused tegra_ahb_resume(struct device *dev) { int i; struct tegra_ahb *ahb = dev_get_drvdata(dev); @@ -190,7 +189,6 @@ static int tegra_ahb_resume(struct device *dev) gizmo_writel(ahb, ahb->ctx[i], tegra_ahb_gizmo[i]); return 0; } -#endif static UNIVERSAL_DEV_PM_OPS(tegra_ahb_pm, tegra_ahb_suspend,
clang warns about an unused variable when CONFIG_PM is disabled, since it is only referenced from an #ifdef: drivers/amba/tegra-ahb.c:97:18: error: variable 'tegra_ahb_gizmo' is not needed and will not be emitted [-Werror,-Wunneeded-internal-declaration] Rather than trying to get the #ifdef right, remove it and use __maybe_unused here, which is less error prone. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/amba/tegra-ahb.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) -- 2.20.0