Xen devel - Oct 2012 - [PATCH, v2] fix build with XEN and EARLY_PRINTK_DBGP enabled but USB_SUPPORT disabled

Since there''s no possible caller of dbgp_external_startup() and
dbgp_reset_prep() when !USB_EHCI_HCD, there''s no point in building and
exporting these functions in that case. This eliminates a build error
under the conditions listed in the subject, introduced with the merge
f1c6872e4980bc4078cfaead05f892b3d78dea64.

Reported-by: Randy Dunlap <rdunlap@xenotime.net>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
---
v2: Switch the dependency from USB_SUPPORT to USB_EHCI_HCD as requested
    by Alan (albeit I don''t really agree to that change).

---
 arch/x86/kernel/cpu/perf_event_p6.c |    2 +-
 drivers/usb/early/ehci-dbgp.c       |   15 +++++++++------
 2 files changed, 10 insertions(+), 7 deletions(-)

--- 3.7-rc3/drivers/usb/early/ehci-dbgp.c
+++ 3.7-rc3-ehci-dbgp-early-xen-no-usb/drivers/usb/early/ehci-dbgp.c
@@ -20,6 +20,7 @@
 #include <linux/usb/ehci_def.h>
 #include <linux/delay.h>
 #include <linux/serial_core.h>
+#include <linux/kconfig.h>
 #include <linux/kgdb.h>
 #include <linux/kthread.h>
 #include <asm/io.h>
@@ -614,12 +615,6 @@ err:
 	return -ENODEV;
 }
 
-int dbgp_external_startup(struct usb_hcd *hcd)
-{
-	return xen_dbgp_external_startup(hcd) ?: _dbgp_external_startup();
-}
-EXPORT_SYMBOL_GPL(dbgp_external_startup);
-
 static int ehci_reset_port(int port)
 {
 	u32 portsc;
@@ -979,6 +974,7 @@ struct console early_dbgp_console = {
 	.index =	-1,
 };
 
+#if IS_ENABLED(CONFIG_USB_EHCI_HCD)
 int dbgp_reset_prep(struct usb_hcd *hcd)
 {
 	int ret = xen_dbgp_reset_prep(hcd);
@@ -1007,6 +1003,13 @@ int dbgp_reset_prep(struct usb_hcd *hcd)
 }
 EXPORT_SYMBOL_GPL(dbgp_reset_prep);
 
+int dbgp_external_startup(struct usb_hcd *hcd)
+{
+	return xen_dbgp_external_startup(hcd) ?: _dbgp_external_startup();
+}
+EXPORT_SYMBOL_GPL(dbgp_external_startup);
+#endif /* USB_EHCI_HCD */
+
 #ifdef CONFIG_KGDB
 
 static char kgdbdbgp_buf[DBGP_MAX_PACKET];

On Mon, Oct 29, 2012 at 04:45:54PM +0000, Jan Beulich
wrote:
> Since there''s no possible caller of dbgp_external_startup() and
> dbgp_reset_prep() when !USB_EHCI_HCD, there''s no point in building
and
> exporting these functions in that case. This eliminates a build error
> under the conditions listed in the subject, introduced with the merge
> f1c6872e4980bc4078cfaead05f892b3d78dea64.
That''s a merge commit, is that where the problem happened because Linus
did the merge resolution incorrectly, or is it really due to some other
patch that added this feature incorrectly?

If the latter, please provide that git commit id.

thanks,

greg k-h

Hello.

On 10/29/2012 07:45 PM, Jan Beulich wrote:
> Since there''s no possible caller of dbgp_external_startup() and
> dbgp_reset_prep() when !USB_EHCI_HCD, there''s no point in building
and
> exporting these functions in that case. This eliminates a build error
> under the conditions listed in the subject, introduced with the merge
> f1c6872e4980bc4078cfaead05f892b3d78dea64.
   Summary of that merge needs to be quoted I think, as well as in the case it
was a normal commit...
> Reported-by: Randy Dunlap <rdunlap@xenotime.net>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Alan Stern <stern@rowland.harvard.edu>
WBR, Sergei

On Mon, Oct 29, 2012 at 09:50:47AM -0700, Greg KH wrote:
> On Mon, Oct 29, 2012 at 04:45:54PM +0000, Jan Beulich wrote:
> > Since there''s no possible caller of dbgp_external_startup()
and
> > dbgp_reset_prep() when !USB_EHCI_HCD, there''s no point in
building and
> > exporting these functions in that case. This eliminates a build error
> > under the conditions listed in the subject, introduced with the merge
> > f1c6872e4980bc4078cfaead05f892b3d78dea64.
> 
> That''s a merge commit, is that where the problem happened because
Linus
> did the merge resolution incorrectly, or is it really due to some other
No. He did it based on what we (Stefano and me) thought was the correct
way that address the ARM and generic patches merge.

While it did work - Randy found via his random config generator
cronjob that it did not address all of the EHCI/EARLY/USB/PCI
combinations.
> patch that added this feature incorrectly?
> 
> If the latter, please provide that git commit id.
> 
> thanks,
> 
> greg k-h

On Mon, 29 Oct 2012, Jan Beulich wrote:
> Since there''s no possible caller of dbgp_external_startup() and
> dbgp_reset_prep() when !USB_EHCI_HCD, there''s no point in building
and
> exporting these functions in that case. This eliminates a build error
> under the conditions listed in the subject, introduced with the merge
> f1c6872e4980bc4078cfaead05f892b3d78dea64.
> 
> Reported-by: Randy Dunlap <rdunlap@xenotime.net>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> ---
> v2: Switch the dependency from USB_SUPPORT to USB_EHCI_HCD as requested
>     by Alan (albeit I don''t really agree to that change).
Considering that the rest of ehci-dbgp.c is specific to EHCI hardware,
using USB_EHCI_HCD seems to me like a reasonable thing to do.
> ---
>  arch/x86/kernel/cpu/perf_event_p6.c |    2 +-
What''s up with this?
>  drivers/usb/early/ehci-dbgp.c       |   15 +++++++++------
>  2 files changed, 10 insertions(+), 7 deletions(-)
Acked-by: Alan Stern <stern@rowland.harvard.edu>

>>> On 29.10.12 at 23:17, Alan Stern <stern@rowland.harvard.edu>
wrote:
> On Mon, 29 Oct 2012, Jan Beulich wrote:
> 
>> Since there''s no possible caller of dbgp_external_startup()
and
>> dbgp_reset_prep() when !USB_EHCI_HCD, there''s no point in
building and
>> exporting these functions in that case. This eliminates a build error
>> under the conditions listed in the subject, introduced with the merge
>> f1c6872e4980bc4078cfaead05f892b3d78dea64.
>> 
>> Reported-by: Randy Dunlap <rdunlap@xenotime.net>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> Cc: Alan Stern <stern@rowland.harvard.edu>
>> ---
>> v2: Switch the dependency from USB_SUPPORT to USB_EHCI_HCD as requested
>>     by Alan (albeit I don''t really agree to that change).
> 
> Considering that the rest of ehci-dbgp.c is specific to EHCI hardware,
> using USB_EHCI_HCD seems to me like a reasonable thing to do.
> 
>> ---
>>  arch/x86/kernel/cpu/perf_event_p6.c |    2 +-
> 
> What''s up with this?
I had to fix an unrelated build issue, and while I removed the
resulting change from the patch, I forgot to update the diffstat
(I had lumped that change into the same patch). So please
ignore this wrong piece of information.

Jan
>>  drivers/usb/early/ehci-dbgp.c       |   15 +++++++++------
>>  2 files changed, 10 insertions(+), 7 deletions(-)
> 
> Acked-by: Alan Stern <stern@rowland.harvard.edu>

>>> On 29.10.12 at 17:50, Greg KH <gregkh@linuxfoundation.org>
wrote:
> On Mon, Oct 29, 2012 at 04:45:54PM +0000, Jan Beulich wrote:
>> Since there''s no possible caller of dbgp_external_startup()
and
>> dbgp_reset_prep() when !USB_EHCI_HCD, there''s no point in
building and
>> exporting these functions in that case. This eliminates a build error
>> under the conditions listed in the subject, introduced with the merge
>> f1c6872e4980bc4078cfaead05f892b3d78dea64.
> 
> That''s a merge commit, is that where the problem happened because
Linus
> did the merge resolution incorrectly, or is it really due to some other
> patch that added this feature incorrectly?
It was indeed the merge that broke things (Linus just followed
the recommendation from Stefano, which weren''t consistent
with the rest of the code).

Jan
> If the latter, please provide that git commit id.
> 
> thanks,
> 
> greg k-h