From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcin Slusarz Subject: Re: [PATCH] usb: Fix linker errors with CONFIG_PM=n Date: Sat, 16 Oct 2010 13:35:21 +0200 Message-ID: <20101016113521.GA2786@joi.lan> References: <20101015205756.GA1757@xanatos> <20101015215915.GA25942@xanatos> <20101015222301.GA4080@joi.lan> <20101015232629.GA26031@xanatos> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Received: from mail-bw0-f46.google.com ([209.85.214.46]:43593 "EHLO mail-bw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753295Ab0JPLfg (ORCPT ); Sat, 16 Oct 2010 07:35:36 -0400 Content-Disposition: inline In-Reply-To: <20101015232629.GA26031@xanatos> Sender: linux-next-owner@vger.kernel.org List-ID: To: Sarah Sharp Cc: Greg KH , Randy Dunlap , Stephen Rothwell , linux-next@vger.kernel.org, LKML , lud , Andiry Xu , Dong Nguyen , Libin Yang On Fri, Oct 15, 2010 at 04:26:29PM -0700, Sarah Sharp wrote: > On Sat, Oct 16, 2010 at 12:23:01AM +0200, Marcin Slusarz wrote: > > On Fri, Oct 15, 2010 at 02:59:15PM -0700, Sarah Sharp wrote: > > > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h > > > index c08928a..93d3bf4 100644 > > > --- a/drivers/usb/host/xhci.h > > > +++ b/drivers/usb/host/xhci.h > > > @@ -1405,8 +1405,15 @@ int xhci_init(struct usb_hcd *hcd); > > > int xhci_run(struct usb_hcd *hcd); > > > void xhci_stop(struct usb_hcd *hcd); > > > void xhci_shutdown(struct usb_hcd *hcd); > > > + > > > +#ifdef CONFIG_PM > > > int xhci_suspend(struct xhci_hcd *xhci); > > > int xhci_resume(struct xhci_hcd *xhci, bool hibernated); > > > +#else > > > +#define xhci_suspend NULL > > > +#define xhci_resume NULL > > > +#endif > > > + > >=20 > > "static inline int xhci_suspend(struct xhci_hcd *) {}" > > has the same effect but saves types >=20 > That doesn't have the same effect. Since those functions are only us= ed > as function pointers, the original patch compiles to less code when > CONFIG_PM=3Dn. =20 xhci_suspend is not called through function pointer. xhci_bus_suspend i= s. > Also, the original version will cause an oops if those > functions are ever called when CONFIG_PM=3Dn, which would indicate a = bug > in the callee (something we would rather catch anyway). This is the > style that other USB host controller drivers use, like EHCI. >=20 > I don't understand what you mean by "saves types". Is there somethin= g > I've missed? Depending on CONFIG_PM xhci_suspend is a function or a constant. If someone in the future would decide to call this function directly, w= ithout CONFIG_PM, he would fail with "error: called object =E2=80=980u=E2=80=99= is not a function". So, strictly speaking, this patch is not losing type informations. It's= just wrong. Sometimes people are tempted to do something like this: #define xhci_suspend(X) (-EINVAL) or when "function" does not return anything: #define void_fun(X) do {} while (0) or even worse: #define void fun(X) {} And this loses type information. Without CONFIG_PM sloppy developer could call this "function" like this= : struct whatever *p =3D xhci_suspend("foo bar"); and it would compile fine. Defining it as static inline int xhci_suspend(struct xhci_hcd *) { return -EINVAL; } would prevent it. But in this particular case I think the cleanest solutions is to just i= fdef everything out, without fallbacks. There's only one caller of xhci_susp= end and it's wrongly guarded by CONFIG_PM. Let's just fix. --- =46rom: Marcin Slusarz Subject: [PATCH] usb/xhci: fix !CONFIG_PM compile/link errors drivers/usb/host/xhci.c:675: error: implicit declaration of function 'u= sb_root_hub_lost_power' ERROR: "xhci_bus_resume" [drivers/usb/host/xhci-hcd.ko] undefined! ERROR: "xhci_bus_suspend" [drivers/usb/host/xhci-hcd.ko] undefined! Signed-off-by: Marcin Slusarz --- drivers/usb/host/xhci-hub.c | 5 ----- drivers/usb/host/xhci-pci.c | 2 ++ drivers/usb/host/xhci.c | 2 ++ drivers/usb/host/xhci.h | 4 ++++ 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 7f2f63c..ac63141 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -742,9 +742,4 @@ int xhci_bus_resume(struct usb_hcd *hcd) return 0; } =20 -#else - -#define xhci_bus_suspend NULL -#define xhci_bus_resume NULL - #endif diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index bb668a8..d10b45d 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -189,8 +189,10 @@ static const struct hc_driver xhci_pci_hc_driver =3D= { /* Root hub support */ .hub_control =3D xhci_hub_control, .hub_status_data =3D xhci_hub_status_data, +#ifdef CONFIG_PM .bus_suspend =3D xhci_bus_suspend, .bus_resume =3D xhci_bus_resume, +#endif }; =20 /*--------------------------------------------------------------------= -----*/ diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 33d0034..c9fc85a 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -551,6 +551,7 @@ void xhci_shutdown(struct usb_hcd *hcd) xhci_readl(xhci, &xhci->op_regs->status)); } =20 +#ifdef CONFIG_PM static void xhci_save_registers(struct xhci_hcd *xhci) { xhci->s3.command =3D xhci_readl(xhci, &xhci->op_regs->command); @@ -760,6 +761,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hiberna= ted) spin_unlock_irq(&xhci->lock); return 0; } +#endif =20 /*--------------------------------------------------------------------= -----*/ =20 diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index c08928a..6f6ee54 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1405,8 +1405,10 @@ int xhci_init(struct usb_hcd *hcd); int xhci_run(struct usb_hcd *hcd); void xhci_stop(struct usb_hcd *hcd); void xhci_shutdown(struct usb_hcd *hcd); +#ifdef CONFIG_PM int xhci_suspend(struct xhci_hcd *xhci); int xhci_resume(struct xhci_hcd *xhci, bool hibernated); +#endif int xhci_get_frame(struct usb_hcd *hcd); irqreturn_t xhci_irq(struct usb_hcd *hcd); irqreturn_t xhci_msi_irq(int irq, struct usb_hcd *hcd); @@ -1481,8 +1483,10 @@ void xhci_ring_ep_doorbell(struct xhci_hcd *xhci= , unsigned int slot_id, int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, u16= wIndex, char *buf, u16 wLength); int xhci_hub_status_data(struct usb_hcd *hcd, char *buf); +#ifdef CONFIG_PM int xhci_bus_suspend(struct usb_hcd *hcd); int xhci_bus_resume(struct usb_hcd *hcd); +#endif u32 xhci_port_state_to_neutral(u32 state); int xhci_find_slot_id_by_port(struct xhci_hcd *xhci, u16 port); void xhci_ring_device(struct xhci_hcd *xhci, int slot_id); --=20 1.7.3