Change in coreboot[master]: sb/intel/common: Add the SOUTHBRIDGE_INTEL_COMMON_USBDEBUG Kconfig sy...

[Mike Banon (Code Review)]

Mike Banon has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36879 )

Change subject: sb/intel/common: Add the SOUTHBRIDGE_INTEL_COMMON_USBDEBUG Kconfig symbol
......................................................................

sb/intel/common: Add the SOUTHBRIDGE_INTEL_COMMON_USBDEBUG Kconfig symbol

This symbol is needed to guard Intel's usb_debug.c from the AMD boards.

Signed-off-by: Mike Banon 
Change-Id: I8b58aa4d9c678b125a5b239f41e97b5c4a9d12fe
---
M src/southbridge/intel/common/Kconfig
M src/southbridge/intel/common/Makefile.inc
2 files changed, 7 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/36879/1

diff --git a/src/southbridge/intel/common/Kconfig b/src/southbridge/intel/common/Kconfig
index 57c0dbe..fbaf791 100644
--- a/src/southbridge/intel/common/Kconfig
+++ b/src/southbridge/intel/common/Kconfig
@@ -12,6 +12,10 @@
 config SOUTHBRIDGE_INTEL_COMMON_PMBASE
    def_bool n

+config SOUTHBRIDGE_INTEL_COMMON_USBDEBUG
+	def_bool n
+	depends on USBDEBUG
+
 config SOUTHBRIDGE_INTEL_COMMON_GPIO
    def_bool n

diff --git a/src/southbridge/intel/common/Makefile.inc b/src/southbridge/intel/common/Makefile.inc
index 9ff0ebc..e882796 100644
--- a/src/southbridge/intel/common/Makefile.inc
+++ b/src/southbridge/intel/common/Makefile.inc
@@ -28,9 +28,9 @@
 all-$(CONFIG_SOUTHBRIDGE_INTEL_COMMON_PMBASE) += pmbase.c
 smm-$(CONFIG_SOUTHBRIDGE_INTEL_COMMON_PMBASE) += pmbase.c

-bootblock-$(CONFIG_USBDEBUG) += usb_debug.c
-romstage-$(CONFIG_USBDEBUG) += usb_debug.c
-ramstage-$(CONFIG_USBDEBUG) += usb_debug.c
+bootblock-$(CONFIG_SOUTHBRIDGE_INTEL_USBDEBUG) += usb_debug.c
+romstage-$(CONFIG_SOUTHBRIDGE_INTEL_USBDEBUG) += usb_debug.c
+ramstage-$(CONFIG_SOUTHBRIDGE_INTEL_USBDEBUG) += usb_debug.c

bootblock-$(CONFIG_SOUTHBRIDGE_INTEL_COMMON_GPIO) += gpio.c
 romstage-$(CONFIG_SOUTHBRIDGE_INTEL_COMMON_GPIO) += gpio.c

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36879 )

Change subject: sb/intel/common: Add the SOUTHBRIDGE_INTEL_COMMON_USBDEBUG Kconfig symbol
......................................................................

Patch Set 1:

This breaks EHCI debug on every Intel board.

[Mike Banon (Code Review)]

Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36879 )

Change subject: sb/intel/common: Add the SOUTHBRIDGE_INTEL_COMMON_USBDEBUG Kconfig symbol
......................................................................

Patch Set 1:
> Patch Set 1:
>
> This breaks EHCI debug on every Intel board.

Why? SOUTHBRIDGE_INTEL_COMMON_USBDEBUG will be automatically selected if USBDEBUG is enabled

[Mike Banon (Code Review)]

Hello Patrick Rudolph, Arthur Heymans, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/36879

to look at the new patch set (#2).

Change subject: sb/intel/common: Add the SOUTHBRIDGE_INTEL_COMMON_USBDEBUG Kconfig symbol
......................................................................

sb/intel/common: Add the SOUTHBRIDGE_INTEL_COMMON_USBDEBUG Kconfig symbol

This symbol is needed to guard Intel's usb_debug.c from the AMD boards.

Signed-off-by: Mike Banon 
Change-Id: I8b58aa4d9c678b125a5b239f41e97b5c4a9d12fe
---
M src/southbridge/intel/common/Kconfig
M src/southbridge/intel/common/Makefile.inc
2 files changed, 7 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/36879/2

[Mike Banon (Code Review)]

Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36879 )

Change subject: sb/intel/common: Add the SOUTHBRIDGE_INTEL_COMMON_USBDEBUG Kconfig symbol
......................................................................

Patch Set 2:
> Patch Set 1:
>
> This breaks EHCI debug on every Intel board.

Fixed a typo. Hope it is better now

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36879 )

Change subject: sb/intel/common: Add the SOUTHBRIDGE_INTEL_COMMON_USBDEBUG Kconfig symbol
......................................................................

Patch Set 2:
> Patch Set 2:
> > Patch Set 1:
> >
> > This breaks EHCI debug on every Intel board.
>
> Fixed a typo. Hope it is better now

I fail to so see how it improves the situation. Now you just replace one Kconfig symbol by another.

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36879 )

Change subject: sb/intel/common: Add the SOUTHBRIDGE_INTEL_COMMON_USBDEBUG Kconfig symbol
......................................................................

Patch Set 2:

(1 comment)
> > > This breaks EHCI debug on every Intel board.
> >
> > Fixed a typo. Hope it is better now
>
> I fail to so see how it improves the situation. Now you just replace one Kconfig symbol by another.

Mike, if you make it `default y` platforms not using it would have to
opt-out. If `default n`, platforms would have to opt-in to make use
of it. The latter seems correct and is what Arthur does.

I just tested it with the typo corrected, fails for G505s with the
same error as before.

https://review.coreboot.org/c/coreboot/+/36879/2/src/southbridge/intel/commo... 
File src/southbridge/intel/common/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/36879/2/src/southbridge/intel/commo... 
PS2, Line 31: CONFIG_SOUTHBRIDGE_INTEL_USBDEBUG
You named it differently in the Kconfig.

[Mike Banon (Code Review)]

Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36879 )

Change subject: sb/intel/common: Add the SOUTHBRIDGE_INTEL_COMMON_USBDEBUG Kconfig symbol
......................................................................

Patch Set 2:
> Patch Set 2:
>
> (1 comment)
> > > > This breaks EHCI debug on every Intel board.
> > >
> > > Fixed a typo. Hope it is better now
> >
> > I fail to so see how it improves the situation. Now you just replace one Kconfig symbol by another.
>
> Mike, if you make it `default y` platforms not using it would have to
> opt-out. If `default n`, platforms would have to opt-in to make use
> of it. The latter seems correct and is what Arthur does.
>
> I just tested it with the typo corrected, fails for G505s with the
> same error as before.

I see. Thank you very much for helping to figure out this Kconfig stuff

[Mike Banon (Code Review)]

Mike Banon has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/36879 )

Change subject: sb/intel/common: Add the SOUTHBRIDGE_INTEL_COMMON_USBDEBUG Kconfig symbol
......................................................................

Abandoned

CB:36878 is better