Nikolai Vyssotski has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47545 )
Change subject: Support SIO1036 with addresses other than 0x4e/0x4d ......................................................................
Support SIO1036 with addresses other than 0x4e/0x4d
Newer LPC Mandolin debug boards hard strap SIO address to be at 0x164e/0x164d vs 0x4e/0x4d as before. Add support for configurable SIO address (SUPERIO_ADR_RANGE) to support these cards.
Change-Id: I103c61f21f13970dfa3b9a788b29964e478fb84c Signed-off-by: Nikolai Vyssotski nikolai.vyssotski@amd.corp-partner.google.com --- M src/mainboard/amd/mandolin/bootblock.c M src/superio/smsc/sio1036/Kconfig M src/superio/smsc/sio1036/sio1036_early_init.c 3 files changed, 14 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/47545/1
diff --git a/src/mainboard/amd/mandolin/bootblock.c b/src/mainboard/amd/mandolin/bootblock.c index 06da379..dc3ce34 100644 --- a/src/mainboard/amd/mandolin/bootblock.c +++ b/src/mainboard/amd/mandolin/bootblock.c @@ -5,14 +5,19 @@ #include <superio/smsc/sio1036/sio1036.h> #include "gpio.h"
-#define SERIAL_DEV PNP_DEV(0x4e, SIO1036_SP1) +#define SERIAL_DEV PNP_DEV(CONFIG_SUPERIO_ADR_RANGE, SIO1036_SP1)
void bootblock_mainboard_early_init(void) { mainboard_program_early_gpios();
if (CONFIG(SUPERIO_SMSC_SIO1036)) { - lpc_enable_sio_decode(LPC_SELECT_SIO_4E4F); + if ((CONFIG_SUPERIO_ADR_RANGE == 0x4e) || (CONFIG_SUPERIO_ADR_RANGE == 0x2e)) { + lpc_enable_sio_decode(LPC_WIDEIO0_ENABLE); + } else { + // set up 16 byte wide I/O range window for the super IO + lpc_set_wideio_range(CONFIG_SUPERIO_ADR_RANGE & ~0xF, 16); + } lpc_enable_decode(DECODE_ENABLE_SERIAL_PORT0 << CONFIG_UART_FOR_CONSOLE); sio1036_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE); } diff --git a/src/superio/smsc/sio1036/Kconfig b/src/superio/smsc/sio1036/Kconfig index 94899ff..fb13d73 100644 --- a/src/superio/smsc/sio1036/Kconfig +++ b/src/superio/smsc/sio1036/Kconfig @@ -2,3 +2,8 @@
config SUPERIO_SMSC_SIO1036 bool + +config SUPERIO_ADR_RANGE + depends on SUPERIO_SMSC_SIO1036 + hex "I/O address for SMSC1036 SIO" + default 0x4e diff --git a/src/superio/smsc/sio1036/sio1036_early_init.c b/src/superio/smsc/sio1036/sio1036_early_init.c index a65f672..c4a580a 100644 --- a/src/superio/smsc/sio1036/sio1036_early_init.c +++ b/src/superio/smsc/sio1036/sio1036_early_init.c @@ -10,13 +10,13 @@
static inline void sio1036_enter_conf_state(pnp_devfn_t dev) { - u8 port = dev >> 8; + u16 port = dev >> 8; outb(0x55, port); }
static inline void sio1036_exit_conf_state(pnp_devfn_t dev) { - u8 port = dev >> 8; + u16 port = dev >> 8; outb(0xaa, port); }
Nikolai Vyssotski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47545 )
Change subject: mb/amd/mandolin: Add decode range for LPC debug card ......................................................................
Patch Set 2:
(6 comments)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/47545/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47545/1//COMMIT_MSG@7 PS1, Line 7: Support SIO1036 with addresses other than 0x4e/0x4d
Make this something like […]
Done
https://review.coreboot.org/c/coreboot/+/47545/1//COMMIT_MSG@9 PS1, Line 9: Mandolin
I'd remove mention of Mandolin since it's really the debug card with the strap.
Done
https://review.coreboot.org/c/coreboot/+/47545/1/src/mainboard/amd/mandolin/... File src/mainboard/amd/mandolin/bootblock.c:
https://review.coreboot.org/c/coreboot/+/47545/1/src/mainboard/amd/mandolin/... PS1, Line 8: CONFIG_SUPERIO_ADR_RANGE
If this is going to be used here, the Kconfig should also be declared in mandolin/Kconfig.
Done
https://review.coreboot.org/c/coreboot/+/47545/1/src/mainboard/amd/mandolin/... PS1, Line 16: LPC_WIDEIO0_ENABLE
Isn't this still LPC_SELECT_SIO_4E4F or 2E2F?
Thank you. You are correct. I missed this one. It should have stayed the same.
https://review.coreboot.org/c/coreboot/+/47545/1/src/superio/smsc/sio1036/Kc... File src/superio/smsc/sio1036/Kconfig:
https://review.coreboot.org/c/coreboot/+/47545/1/src/superio/smsc/sio1036/Kc... PS1, Line 9: default 0x4e
I think we can turn this into a choice. […]
Done
https://review.coreboot.org/c/coreboot/+/47545/1/src/superio/smsc/sio1036/si... File src/superio/smsc/sio1036/sio1036_early_init.c:
https://review.coreboot.org/c/coreboot/+/47545/1/src/superio/smsc/sio1036/si... PS1, Line 16:
I would split this file into a separate patch. It should be unrelated to a mainboard change.
Done
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47545 )
Change subject: mb/amd/mandolin: Add decode range for LPC debug card ......................................................................
Patch Set 2:
(4 comments)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/47545/2/src/mainboard/amd/mandolin/... File src/mainboard/amd/mandolin/Kconfig:
https://review.coreboot.org/c/coreboot/+/47545/2/src/mainboard/amd/mandolin/... PS2, Line 31: base Maybe config address?
https://review.coreboot.org/c/coreboot/+/47545/2/src/mainboard/amd/mandolin/... PS2, Line 45: default 0x2E if SMSC_SIO1036_BASE_2E Ah, this line needs to come out now
https://review.coreboot.org/c/coreboot/+/47545/2/src/mainboard/amd/mandolin/... PS2, Line 46: 0x4E nit: lower case here and on the next line
https://review.coreboot.org/c/coreboot/+/47545/2/src/mainboard/amd/mandolin/... File src/mainboard/amd/mandolin/bootblock.c:
https://review.coreboot.org/c/coreboot/+/47545/2/src/mainboard/amd/mandolin/... PS2, Line 15: || (CONFIG_SUPERIO_ADDR_RANGE == 0x2e) You can remove this now
Hello build bot (Jenkins), Marshall Dawson, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47545
to look at the new patch set (#3).
Change subject: mb/amd/mandolin: Add decode range for LPC debug card ......................................................................
mb/amd/mandolin: Add decode range for LPC debug card
Newer LPC debug boards hard strap SIO address to be at 0x164e/0x164d vs 0x4e/0x4d as before. Add support for configurable SIO address to support these cards.
Change-Id: I103c61f21f13970dfa3b9a788b29964e478fb84c Signed-off-by: Nikolai Vyssotski nikolai.vyssotski@amd.corp-partner.google.com --- M src/mainboard/amd/mandolin/Kconfig M src/mainboard/amd/mandolin/bootblock.c 2 files changed, 26 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/47545/3
Nikolai Vyssotski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47545 )
Change subject: mb/amd/mandolin: Add decode range for LPC debug card ......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/c/coreboot/+/47545/2/src/mainboard/amd/mandolin/... File src/mainboard/amd/mandolin/Kconfig:
https://review.coreboot.org/c/coreboot/+/47545/2/src/mainboard/amd/mandolin/... PS2, Line 31: base
Maybe config address?
Done
https://review.coreboot.org/c/coreboot/+/47545/2/src/mainboard/amd/mandolin/... PS2, Line 45: default 0x2E if SMSC_SIO1036_BASE_2E
Ah, this line needs to come out now
Done
https://review.coreboot.org/c/coreboot/+/47545/2/src/mainboard/amd/mandolin/... PS2, Line 46: 0x4E
nit: lower case here and on the next line
Done
https://review.coreboot.org/c/coreboot/+/47545/2/src/mainboard/amd/mandolin/... File src/mainboard/amd/mandolin/bootblock.c:
https://review.coreboot.org/c/coreboot/+/47545/2/src/mainboard/amd/mandolin/... PS2, Line 15: || (CONFIG_SUPERIO_ADDR_RANGE == 0x2e)
You can remove this now
Done
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47545 )
Change subject: mb/amd/mandolin: Add decode range for LPC debug card ......................................................................
Patch Set 3:
(2 comments)
this one needs a manual rebase, since I've fixed the comment on AMD_LPC_DEBUG_CARD in the meantime and I think that we shold still mention that this option is mutually exclusive with PICASSO_CONSOLE_UART
https://review.coreboot.org/c/coreboot/+/47545/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47545/3//COMMIT_MSG@9 PS3, Line 9: Newer i'm not sure if this is only for the newer ones or if that's for all amd lpc serial port cards. for debugging a system without needing to change other things, it does make sense that it uses a non-standard address, so it won't collide with the main super i/o in the system
https://review.coreboot.org/c/coreboot/+/47545/3/src/mainboard/amd/mandolin/... File src/mainboard/amd/mandolin/bootblock.c:
https://review.coreboot.org/c/coreboot/+/47545/3/src/mainboard/amd/mandolin/... PS3, Line 15: RANGE i'd call it _BASE instead of _RANGE
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47545 )
Change subject: mb/amd/mandolin: Add decode range for LPC debug card ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47545/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47545/3//COMMIT_MSG@9 PS3, Line 9: Newer
i'm not sure if this is only for the newer ones or if that's for all amd lpc serial port cards. […]
Mine is 4E and I can't see any signs of it having been reworked. I've probably had it for about 4 years.
FWIW, the commit message doesn't need to cite any chronology. It could simply state that the cards have two addresses they can be strapped to.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47545 )
Change subject: mb/amd/mandolin: Add decode range for LPC debug card ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47545/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47545/3//COMMIT_MSG@9 PS3, Line 9: Newer
Mine is 4E and I can't see any signs of it having been reworked. […]
oh. that does explain why the image I built some months ago had working serial out via the LPC card on your system while it didn't work on my system
Hello build bot (Jenkins), Marshall Dawson, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47545
to look at the new patch set (#4).
Change subject: mb/amd/mandolin: Add decode range for LPC debug card ......................................................................
mb/amd/mandolin: Add decode range for LPC debug card
Some LPC debug boards hard strap SIO address to be at 0x164e/0x164d vs 0x4e/0x4d as before. Add support for configurable SIO address to support these cards.
Change-Id: I103c61f21f13970dfa3b9a788b29964e478fb84c Signed-off-by: Nikolai Vyssotski nikolai.vyssotski@amd.corp-partner.google.com --- M src/mainboard/amd/mandolin/Kconfig M src/mainboard/amd/mandolin/bootblock.c 2 files changed, 26 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/47545/4
Hello build bot (Jenkins), Marshall Dawson, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47545
to look at the new patch set (#5).
Change subject: mb/amd/mandolin: Add decode range for LPC debug card ......................................................................
mb/amd/mandolin: Add decode range for LPC debug card
Some LPC debug boards hard strap SIO address to be at 0x164e/0x164d vs 0x4e/0x4d. Add support for configurable SIO address to support these cards.
Change-Id: I103c61f21f13970dfa3b9a788b29964e478fb84c Signed-off-by: Nikolai Vyssotski nikolai.vyssotski@amd.corp-partner.google.com --- M src/mainboard/amd/mandolin/bootblock.c 1 file changed, 7 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/47545/5
Hello build bot (Jenkins), Marshall Dawson, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47545
to look at the new patch set (#6).
Change subject: mb/amd/mandolin: Add decode range for LPC debug card ......................................................................
mb/amd/mandolin: Add decode range for LPC debug card
Some LPC debug boards hard strap SIO address to be at 0x164e/0x164d vs 0x4e/0x4d. Add support for configurable SIO address to support these cards.
Change-Id: I103c61f21f13970dfa3b9a788b29964e478fb84c Signed-off-by: Nikolai Vyssotski nikolai.vyssotski@amd.corp-partner.google.com --- M src/mainboard/amd/mandolin/Kconfig M src/mainboard/amd/mandolin/bootblock.c 2 files changed, 25 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/47545/6
Nikolai Vyssotski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47545 )
Change subject: mb/amd/mandolin: Add decode range for LPC debug card ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47545/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47545/3//COMMIT_MSG@9 PS3, Line 9: Newer
oh. […]
Done
https://review.coreboot.org/c/coreboot/+/47545/3/src/mainboard/amd/mandolin/... File src/mainboard/amd/mandolin/bootblock.c:
https://review.coreboot.org/c/coreboot/+/47545/3/src/mainboard/amd/mandolin/... PS3, Line 15: RANGE
i'd call it _BASE instead of _RANGE
Done
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47545 )
Change subject: mb/amd/mandolin: Add decode range for LPC debug card ......................................................................
Patch Set 6: Code-Review+2
this fixes the coreboot console over the LPC serial port for me. thank you!
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47545 )
Change subject: mb/amd/mandolin: Add decode range for LPC debug card ......................................................................
Patch Set 6:
Patch Set 6: Code-Review+2
this fixes the coreboot console over the LPC serial port for me. thank you!
oh, it would be useful if you add the bug ID to the commit message: BUG=b:159933344
Hello build bot (Jenkins), Marshall Dawson, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47545
to look at the new patch set (#7).
Change subject: mb/amd/mandolin: Add decode range for LPC debug card ......................................................................
mb/amd/mandolin: Add decode range for LPC debug card
Some LPC debug boards hard strap SIO address to be at 0x164e/0x164d vs 0x4e/0x4d. Add support for configurable SIO address to support these cards.
BUG=b:159933344 TEST=boot with LPC debug card, verify serial output
Change-Id: I103c61f21f13970dfa3b9a788b29964e478fb84c Signed-off-by: Nikolai Vyssotski nikolai.vyssotski@amd.corp-partner.google.com --- M src/mainboard/amd/mandolin/Kconfig M src/mainboard/amd/mandolin/bootblock.c 2 files changed, 25 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/47545/7
Nikolai Vyssotski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47545 )
Change subject: mb/amd/mandolin: Add decode range for LPC debug card ......................................................................
Patch Set 7:
Patch Set 6:
Patch Set 6: Code-Review+2
this fixes the coreboot console over the LPC serial port for me. thank you!
oh, it would be useful if you add the bug ID to the commit message: BUG=b:159933344
done. thanks!
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47545 )
Change subject: mb/amd/mandolin: Add decode range for LPC debug card ......................................................................
mb/amd/mandolin: Add decode range for LPC debug card
Some LPC debug boards hard strap SIO address to be at 0x164e/0x164d vs 0x4e/0x4d. Add support for configurable SIO address to support these cards.
BUG=b:159933344 TEST=boot with LPC debug card, verify serial output
Change-Id: I103c61f21f13970dfa3b9a788b29964e478fb84c Signed-off-by: Nikolai Vyssotski nikolai.vyssotski@amd.corp-partner.google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/47545 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Felix Held felix-coreboot@felixheld.de --- M src/mainboard/amd/mandolin/Kconfig M src/mainboard/amd/mandolin/bootblock.c 2 files changed, 25 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved
diff --git a/src/mainboard/amd/mandolin/Kconfig b/src/mainboard/amd/mandolin/Kconfig index dde7ac8..b779630 100644 --- a/src/mainboard/amd/mandolin/Kconfig +++ b/src/mainboard/amd/mandolin/Kconfig @@ -29,6 +29,24 @@ PICASSO_CONSOLE_UART which selects the SoC's integrated memory-mapped UART for coreboot console output.
+choice + prompt "SMSC/Microchip 1036 SuperIO config address" + depends on SUPERIO_SMSC_SIO1036 + default SMSC_SIO1036_BASE_164E + +config SMSC_SIO1036_BASE_4E + bool "0x4e/0x4d base address" + +config SMSC_SIO1036_BASE_164E + bool "0x164e/0x164d base address" + +endchoice + +config SUPERIO_ADDR_BASE + hex + default 0x4e if SMSC_SIO1036_BASE_4E + default 0x164e if SMSC_SIO1036_BASE_164E + config CBFS_SIZE hex default 0x7cf000 if BOARD_AMD_MANDOLIN # Maximum size for the Mandolin FMAP diff --git a/src/mainboard/amd/mandolin/bootblock.c b/src/mainboard/amd/mandolin/bootblock.c index 06da379..94a1329 100644 --- a/src/mainboard/amd/mandolin/bootblock.c +++ b/src/mainboard/amd/mandolin/bootblock.c @@ -5,14 +5,19 @@ #include <superio/smsc/sio1036/sio1036.h> #include "gpio.h"
-#define SERIAL_DEV PNP_DEV(0x4e, SIO1036_SP1) +#define SERIAL_DEV PNP_DEV(CONFIG_SUPERIO_ADDR_BASE, SIO1036_SP1)
void bootblock_mainboard_early_init(void) { mainboard_program_early_gpios();
if (CONFIG(SUPERIO_SMSC_SIO1036)) { - lpc_enable_sio_decode(LPC_SELECT_SIO_4E4F); + if (CONFIG_SUPERIO_ADDR_BASE == 0x4e) { + lpc_enable_sio_decode(LPC_SELECT_SIO_4E4F); + } else { + // set up 16 byte wide I/O range window for the super IO + lpc_set_wideio_range(CONFIG_SUPERIO_ADDR_BASE & ~0xF, 16); + } lpc_enable_decode(DECODE_ENABLE_SERIAL_PORT0 << CONFIG_UART_FOR_CONSOLE); sio1036_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE); }
-
Felix Held (Code Review) -
Marshall Dawson (Code Review)
-
Nikolai Vyssotski (Code Review)
-
Patrick Georgi (Code Review)