Change in coreboot[master]: soc/amd/picasso: Include uart.c unconditionally

[Furquan Shaikh (Code Review)]

Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: Include uart.c unconditionally
......................................................................

soc/amd/picasso: Include uart.c unconditionally

This change includes uart.c in bootblock, romstage, ramstage and
verstage unconditionally because this file is handling more than just
the UART console configuration. This allows boards to take advantage
of picasso_uart_mmio_ops even if PICASSO_UART is not selected. If the
functions within the file are unused, they will be optimized out by
the linker.

BUG=b:158346697

Change-Id: If1173034b0d2ed32f77241768e1e8abb208aac3a
Signed-off-by: Furquan Shaikh 
---
M src/soc/amd/picasso/Makefile.inc
1 file changed, 4 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/42339/1

diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc
index 7fce124..d7d1082 100644
--- a/src/soc/amd/picasso/Makefile.inc
+++ b/src/soc/amd/picasso/Makefile.inc
@@ -14,7 +14,7 @@
 bootblock-y += bootblock/bootblock.c
 bootblock-y += southbridge.c
 bootblock-y += i2c.c
-bootblock-$(CONFIG_PICASSO_UART) += uart.c
+bootblock-y += uart.c
 bootblock-y += tsc_freq.c
 bootblock-y += gpio.c
 bootblock-y += smi_util.c
@@ -25,7 +25,7 @@
 romstage-y += gpio.c
 romstage-y += pmutil.c
 romstage-y += memmap.c
-romstage-$(CONFIG_PICASSO_UART) += uart.c
+romstage-y += uart.c
 romstage-y += tsc_freq.c
 romstage-y += southbridge.c
 romstage-$(CONFIG_HAVE_SMI_HANDLER) += smi_util.c
@@ -37,7 +37,7 @@
 verstage-y += i2c.c
 verstage-y += pmutil.c
 verstage-y += config.c
-verstage-$(CONFIG_PICASSO_UART) += uart.c
+verstage-y += uart.c
 verstage-y += tsc_freq.c

ramstage-y += i2c.c
@@ -56,7 +56,7 @@
 ramstage-y += memmap.c
 ramstage-$(CONFIG_HAVE_SMI_HANDLER) += smi.c
 ramstage-$(CONFIG_HAVE_SMI_HANDLER) += smi_util.c
-ramstage-$(CONFIG_PICASSO_UART) += uart.c
+ramstage-y += uart.c
 ramstage-y += usb.c
 ramstage-y += tsc_freq.c
 ramstage-y += finalize.c

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: Include uart.c unconditionally
......................................................................

Patch Set 1: Code-Review-1

already tried this quick-fix and it doesn't unbreak Mandolin; it just breaks it in a different way:

/home/felix/coreboot/util/crossgcc/xgcc/bin/i386-elf-ld.bfd: build/bootblock/soc/amd/picasso/uart.o: in function `uart_platform_base':
/home/felix/coreboot/src/soc/amd/picasso/uart.c:34: multiple definition of `uart_platform_base'; build/bootblock/drivers/uart/uart8250io.o:/home/felix/coreboot/src/drivers/uart/uart8250io.c:81: first defined here
/home/felix/coreboot/util/crossgcc/xgcc/bin/i386-elf-ld.bfd: build/bootblock/soc/amd/picasso/uart.o: in function `uart_platform_refclk':
/home/felix/coreboot/src/soc/amd/picasso/uart.c:76: multiple definition of `uart_platform_refclk'; build/bootblock/drivers/uart/util.o:/home/felix/coreboot/src/drivers/uart/util.c:44: first defined here
src/arch/x86/Makefile.inc:117: recipe for target 'build/cbfs/fallback/bootblock.debug' failed

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Felix Held (Code Review)]

Felix Held has uploaded a new patch set (#2) to the change originally created by Furquan Shaikh. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

soc/amd/picasso: fix build if PICASSO_UART is unset

This change includes uart.c in bootblock, romstage, ramstage and
verstage unconditionally because this file is handling more than just
the UART console configuration. This allows boards to take advantage
of picasso_uart_mmio_ops even if PICASSO_UART is not selected.
uart_platform_base and uart_platform_refclk mustn't be provided if
PICASSO_UART is unset, so add an #if around those functions.

BUG=b:158346697

TEST=Mandolin builds again.

Change-Id: If1173034b0d2ed32f77241768e1e8abb208aac3a
Signed-off-by: Furquan Shaikh 
Signed-off-by: Felix Held 
---
M src/soc/amd/picasso/Makefile.inc
M src/soc/amd/picasso/uart.c
2 files changed, 13 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/42339/2

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Raul Rangel (Code Review)]

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

Patch Set 2: Code-Review+2

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

Patch Set 2:

smm also?  Actually, I'd be fine with consolidating into

all-y += uart.c

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

Patch Set 2:
> Patch Set 2:
>
> smm also?  Actually, I'd be fine with consolidating into
>
> all-y += uart.c

It's not needed in SMM and I'd avoid linking anything into SMM that's not needed. The linker will likely drop it when it's unused though; I'd still keep it in the current state

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c 
File src/soc/amd/picasso/uart.c:

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c@... 
PS2, Line 33: PICASSO_UART
Should this be renamed to PICASSO_CONSOLE_UART?

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c@... 
PS2, Line 59: 
There will have to be a check here as well:
if (!CONFIG(PICASSO_UART))
    return;

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c 
File src/soc/amd/picasso/uart.c:

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c@... 
PS2, Line 33: PICASSO_UART
> Should this be renamed to PICASSO_CONSOLE_UART?

good idea; will do, since this is about the coreboot console

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c@... 
PS2, Line 59:
> There will have to be a check here as well: […]

I don't think so; this function is about setting up the uart hardware and not to configure the console. Also the only call to this is in southbridge.c and it's guarded by a if (CONFIG(PICASSO_UART)).
Would also be good to further refactor the uart part, but fixing the newly introduced breakage around mandolin is currently much higher on my priority list.

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c 
File src/soc/amd/picasso/uart.c:

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c@... 
PS2, Line 33: PICASSO_UART
> good idea; will do, since this is about the coreboot console

hmm, in aoac.c are some things depending on PICASSO_UART, but those are about the actual uart and not about the console. can we keep this as it is at the moment and i'll look into it after i sorted out the apob issue?

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c 
File src/soc/amd/picasso/uart.c:

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c@... 
PS2, Line 33: PICASSO_UART
> hmm, in aoac. […]

ignore my last comment; it is about the uart for console

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c 
File src/soc/amd/picasso/uart.c:

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c@... 
PS2, Line 33: PICASSO_UART
> ignore my last comment; it is about the uart for console

addressed in 42429, since this is another logical change

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c 
File src/soc/amd/picasso/uart.c:

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c@... 
PS2, Line 48: #endif /* PICASSO_UART */
Why are we guarding this w/ preprocessor macros? Can't we leave the implementations and rely upon the generic Kconfigs to honor not accessing these functions?

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c 
File src/soc/amd/picasso/uart.c:

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c@... 
PS2, Line 48: #endif /* PICASSO_UART */
> Why are we guarding this w/ preprocessor macros? Can't we leave the implementations and rely upon th […]

Those two functions need to be provided by exactly one compilation unit that provides the functionality used by src/drivers/uart/ for the console UART. When an UART is used that is not the SoC's integrated one, these functions mustn't end up in the object file of this compilation unit.

Putting those two functions into a different compilation unit is definitely what should be done in the end, but since uart.c doesn't have a proper header file yet, it can't be pulled apart easily and I'm currently working on fixing things to get Mandolin working again. Basically every time I get anywhere close to have Mandolin in a state that could finally get submitted, some other change gets submitted creating significant breakage for Mandolin, which is rather frustrating for me, since Mandolin is the platform I do development. I'm planning to do that as a follow-up after fixing the APOB issue, but the further cleanup not a priority for me right now; getting this change landed however is a priority for me, since it is one of the patches required to unbreak non-zork/non-chromeos Picasso support on upstream.

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c 
File src/soc/amd/picasso/uart.c:

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c@... 
PS2, Line 48: #endif /* PICASSO_UART */
> Those two functions need to be provided by exactly one compilation unit that provides the functional […]

Please leave a comment as to why we're guarding things like this then that provides the context. It's not clear why we'd be removing these functions based on the diff itself.

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c 
File src/soc/amd/picasso/uart.c:

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c@... 
PS2, Line 48: #endif /* PICASSO_UART */
> Please leave a comment as to why we're guarding things like this then that provides the context. […]

ah, yeah, that is a good idea for the meantime. will push a new revision later

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Felix Held (Code Review)]

Felix Held has uploaded a new patch set (#3) to the change originally created by Furquan Shaikh. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

soc/amd/picasso: fix build if PICASSO_UART is unset

This change includes uart.c in bootblock, romstage, ramstage and
verstage unconditionally because this file is handling more than just
the UART console configuration. This allows boards to take advantage
of picasso_uart_mmio_ops even if PICASSO_UART is not selected.
uart_platform_base and uart_platform_refclk mustn't be provided if
PICASSO_UART is unset, so add an #if around those functions.

BUG=b:158346697

TEST=Mandolin builds again.

Change-Id: If1173034b0d2ed32f77241768e1e8abb208aac3a
Signed-off-by: Furquan Shaikh 
Signed-off-by: Felix Held 
---
M src/soc/amd/picasso/Makefile.inc
M src/soc/amd/picasso/uart.c
2 files changed, 21 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/42339/3

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c 
File src/soc/amd/picasso/uart.c:

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c@... 
PS2, Line 48: #endif /* PICASSO_UART */
> ah, yeah, that is a good idea for the meantime. […]

Done

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

Patch Set 3: Code-Review+2

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

Patch Set 3: Code-Review+2

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

Patch Set 3:
> Patch Set 2:
> > Patch Set 2:
> >
> > smm also?  Actually, I'd be fine with consolidating into
> >
> > all-y += uart.c
>
> It's not needed in SMM and I'd avoid linking anything into SMM that's not needed. The linker will likely drop it when it's unused though; I'd still keep it in the current state

Well, how about at least

smm-$(CONFIG_DEBUG_SMI) += uart.c

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

Patch Set 3:
> Well, how about at least
> smm-$(CONFIG_DEBUG_SMI) += uart.c

That's already in there

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c 
File src/soc/amd/picasso/uart.c:

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c@... 
PS2, Line 59:
> I don't think so; this function is about setting up the uart hardware and not to configure the conso […]

SG.

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c 
File src/soc/amd/picasso/uart.c:

https://review.coreboot.org/c/coreboot/+/42339/2/src/soc/amd/picasso/uart.c@... 
PS2, Line 59:
> SG.

Done

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Raul Rangel (Code Review)]

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

Patch Set 3: Code-Review+2

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

Patch Set 4: Code-Review+2
> Patch Set 3:
> > Well, how about at least
> > smm-$(CONFIG_DEBUG_SMI) += uart.c
>
> That's already in there

Oh sorry.  I thought I'd already changed smm-$() to CONFIG_PICASSO_UART here.  But you're right, it's correct as it is now.

Change in coreboot[master]: soc/amd/picasso: fix build if PICASSO_UART is unset

[Felix Held (Code Review)]

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42339 )

Change subject: soc/amd/picasso: fix build if PICASSO_UART is unset
......................................................................

soc/amd/picasso: fix build if PICASSO_UART is unset

This change includes uart.c in bootblock, romstage, ramstage and
verstage unconditionally because this file is handling more than just
the UART console configuration. This allows boards to take advantage
of picasso_uart_mmio_ops even if PICASSO_UART is not selected.
uart_platform_base and uart_platform_refclk mustn't be provided if
PICASSO_UART is unset, so add an #if around those functions.

BUG=b:158346697

TEST=Mandolin builds again.

Change-Id: If1173034b0d2ed32f77241768e1e8abb208aac3a
Signed-off-by: Furquan Shaikh 
Signed-off-by: Felix Held 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/42339
Reviewed-by: Marshall Dawson 
Reviewed-by: Aaron Durbin 
Reviewed-by: Raul Rangel 
Tested-by: build bot (Jenkins) 
---
M src/soc/amd/picasso/Makefile.inc
M src/soc/amd/picasso/uart.c
2 files changed, 21 insertions(+), 9 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Aaron Durbin: Looks good to me, approved
  Furquan Shaikh: Looks good to me, approved
  Marshall Dawson: Looks good to me, approved
  Raul Rangel: Looks good to me, approved

diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc
index 03ce272..9894877 100644
--- a/src/soc/amd/picasso/Makefile.inc
+++ b/src/soc/amd/picasso/Makefile.inc
@@ -15,7 +15,7 @@
 bootblock-y += aoac.c
 bootblock-y += southbridge.c
 bootblock-y += i2c.c
-bootblock-$(CONFIG_PICASSO_UART) += uart.c
+bootblock-y += uart.c
 bootblock-y += tsc_freq.c
 bootblock-y += gpio.c
 bootblock-y += smi_util.c
@@ -26,7 +26,7 @@
 romstage-y += gpio.c
 romstage-y += pmutil.c
 romstage-y += memmap.c
-romstage-$(CONFIG_PICASSO_UART) += uart.c
+romstage-y += uart.c
 romstage-y += tsc_freq.c
 romstage-y += aoac.c
 romstage-y += southbridge.c
@@ -40,7 +40,7 @@
 verstage-y += pmutil.c
 verstage-y += config.c
 verstage-y += aoac.c
-verstage-$(CONFIG_PICASSO_UART) += uart.c
+verstage-y += uart.c
 verstage-y += tsc_freq.c

ramstage-y += i2c.c
@@ -60,7 +60,7 @@
 ramstage-y += memmap.c
 ramstage-$(CONFIG_HAVE_SMI_HANDLER) += smi.c
 ramstage-$(CONFIG_HAVE_SMI_HANDLER) += smi_util.c
-ramstage-$(CONFIG_PICASSO_UART) += uart.c
+ramstage-y += uart.c
 ramstage-y += usb.c
 ramstage-y += tsc_freq.c
 ramstage-y += finalize.c
diff --git a/src/soc/amd/picasso/uart.c b/src/soc/amd/picasso/uart.c
index 6439efb..f523bce 100644
--- a/src/soc/amd/picasso/uart.c
+++ b/src/soc/amd/picasso/uart.c
@@ -30,6 +30,16 @@
    } },
 };

+/*
+ * Don't provide uart_platform_base and uart_platform_refclk functions if PICASSO_UART
+ * isn't selected. Those two functions are used by the console UART driver and need to be
+ * provided exactly once and only by the UART that is used for console.
+ *
+ * TODO: Replace the #if block by factoring out the two functions into a different compilation
+ *       unit.
+ */
+#if CONFIG(PICASSO_UART)
+
 uintptr_t uart_platform_base(int idx)
 {
    if (idx < 0 || idx >= ARRAY_SIZE(uart_info))
@@ -38,6 +48,13 @@
    return uart_info[idx].base;
 }

+unsigned int uart_platform_refclk(void)
+{
+	return CONFIG(PICASSO_UART_48MZ) ? 48000000 : 115200 * 16;
+}
+
+#endif /* PICASSO_UART */
+
 void clear_uart_legacy_config(void)
 {
    write16((void *)FCH_UART_LEGACY_DECODE, 0);
@@ -75,11 +92,6 @@
    }
 }

-unsigned int uart_platform_refclk(void)
-{
-	return CONFIG(PICASSO_UART_48MZ) ? 48000000 : 115200 * 16;
-}
-
 static const char *uart_acpi_name(const struct device *dev)
 {
    switch (dev->path.mmio.addr) {