2013-03-27 05:27:51

by Tony Prisk

[permalink] [raw]
Subject: [PATCHv2] usb: ehci: unlink_empty_async_suspended() only used with CONFIG_PM

Compiling with !CONFIG_PM generates an unused function warning on
unlink_empty_async_suspended().

Enclose the function in a #ifdef CONFIG_PM

Signed-off-by: Tony Prisk <[email protected]>
Acked-by: Alan Stern <[email protected]>
---
v2:
Tidy up blank line as requested by Alan Stern
drivers/usb/host/ehci-q.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index 23d1369..1ea4de1 100644
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -1316,6 +1316,7 @@ static void unlink_empty_async(struct ehci_hcd *ehci)
}
}

+#ifdef CONFIG_PM
/* The root hub is suspended; unlink all the async QHs */
static void unlink_empty_async_suspended(struct ehci_hcd *ehci)
{
@@ -1328,6 +1329,7 @@ static void unlink_empty_async_suspended(struct ehci_hcd *ehci)
}
start_iaa_cycle(ehci, false);
}
+#endif

/* makes sure the async qh will become idle */
/* caller must own ehci->lock */
--
1.7.9.5


2013-03-28 21:10:07

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH] usb: ehci: mark unlink_empty_async_suspended() as __maybe_unused

Patch 4d053fdac3 "usb: ehci: unlink_empty_async_suspended() only used
with CONFIG_PM" tried to hide the unlink_empty_async_suspended function
inside of an #ifdef to work around an unused function warning.

Unfortunately that had the effect of introducing a new warning:

drivers/usb/host/ehci-q.c:1297:13: warning: 'unlink_empty_async_suspended'
declared 'static' but never defined [-Wunused-function]

While we could add another #ifdef around the function declaration to avoid
this, a nicer solution is to mark it as __maybe_unused, which will let
gcc silently drop the function definition when it is not needed.

Signed-off-by: Arnd Bergmann <[email protected]>
---
diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index 7562d76..d34b399 100644
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -1293,9 +1293,8 @@ static void unlink_empty_async(struct ehci_hcd *ehci)
}
}

-#ifdef CONFIG_PM
/* The root hub is suspended; unlink all the async QHs */
-static void unlink_empty_async_suspended(struct ehci_hcd *ehci)
+static void __maybe_unused unlink_empty_async_suspended(struct ehci_hcd *ehci)
{
struct ehci_qh *qh;

@@ -1306,7 +1305,6 @@ static void unlink_empty_async_suspended(struct ehci_hcd *ehci)
}
start_iaa_cycle(ehci);
}
-#endif

/* makes sure the async qh will become idle */
/* caller must own ehci->lock */

2013-03-28 21:16:41

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] usb: ehci: mark unlink_empty_async_suspended() as __maybe_unused

On Thursday 28 March 2013, Arnd Bergmann wrote:
> Patch 4d053fdac3 "usb: ehci: unlink_empty_async_suspended() only used
> with CONFIG_PM" tried to hide the unlink_empty_async_suspended function
> inside of an #ifdef to work around an unused function warning.

Hi Greg,

Apparently the warning is now also in 3.8.5, so you might want to backport
this fix as well after you send it upstream.

Arnd

2013-03-28 21:20:28

by Tony Prisk

[permalink] [raw]
Subject: Re: [PATCH] usb: ehci: mark unlink_empty_async_suspended() as __maybe_unused

On 29/03/13 10:16, Arnd Bergmann wrote:
> On Thursday 28 March 2013, Arnd Bergmann wrote:
>> Patch 4d053fdac3 "usb: ehci: unlink_empty_async_suspended() only used
>> with CONFIG_PM" tried to hide the unlink_empty_async_suspended function
>> inside of an #ifdef to work around an unused function warning.
> Hi Greg,
>
> Apparently the warning is now also in 3.8.5, so you might want to backport
> this fix as well after you send it upstream.
>
> Arnd
Grr, my bad - I originally wrote the patch with the forward decl
#ifdef'd as well, but Alan pointed out that it didn't need to be. I
thought I recompiled it after the change, but obviously not.

Thanks Arnd,

Regards
Tony P

2013-03-29 14:05:07

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH] usb: ehci: mark unlink_empty_async_suspended() as __maybe_unused

On Thu, 28 Mar 2013, Arnd Bergmann wrote:

> Patch 4d053fdac3 "usb: ehci: unlink_empty_async_suspended() only used
> with CONFIG_PM" tried to hide the unlink_empty_async_suspended function
> inside of an #ifdef to work around an unused function warning.
>
> Unfortunately that had the effect of introducing a new warning:
>
> drivers/usb/host/ehci-q.c:1297:13: warning: 'unlink_empty_async_suspended'
> declared 'static' but never defined [-Wunused-function]
>
> While we could add another #ifdef around the function declaration to avoid
> this, a nicer solution is to mark it as __maybe_unused, which will let
> gcc silently drop the function definition when it is not needed.

IMO the compiler is being stupid. -Wunused-function should warn
about functions that are defined but not called, not about functions
that are declared but not defined. Grumble...

Anyway yes, this is a good fix.

Acked-by: Alan Stern <[email protected]>

2013-03-29 14:05:56

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH] usb: ehci: mark unlink_empty_async_suspended() as __maybe_unused

On Fri, 29 Mar 2013, Tony Prisk wrote:

> On 29/03/13 10:16, Arnd Bergmann wrote:
> > On Thursday 28 March 2013, Arnd Bergmann wrote:
> >> Patch 4d053fdac3 "usb: ehci: unlink_empty_async_suspended() only used
> >> with CONFIG_PM" tried to hide the unlink_empty_async_suspended function
> >> inside of an #ifdef to work around an unused function warning.
> > Hi Greg,
> >
> > Apparently the warning is now also in 3.8.5, so you might want to backport
> > this fix as well after you send it upstream.
> >
> > Arnd
> Grr, my bad - I originally wrote the patch with the forward decl
> #ifdef'd as well, but Alan pointed out that it didn't need to be. I
> thought I recompiled it after the change, but obviously not.

It's my fault too. I didn't realize the compiler would issue a warning
about a static declaration with no definition.

Alan Stern