Change in coreboot[master]: soc/amd/picasso: Add device operations for UART MMIO devices
Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42068 )
Change subject: soc/amd/picasso: Add device operations for UART MMIO devices ......................................................................
soc/amd/picasso: Add device operations for UART MMIO devices
This change adds device_operations for UART MMIO devices that provides following operations: 1. uart_acpi_name: Returns ACPI name of UART device. Generation of UART device node is not yet moved to SSDT, but will be done in follow-up CLs. 2. scan_bus: Uses scan_static_bus to scan devices added under the UART devices. This allows mainboard to add devices under the UART MMIO device.
Change-Id: I18abbe88952e7006668657eb1d0c177e53e95850 Signed-off-by: Furquan Shaikh furquan@google.com --- M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/uart.c 2 files changed, 31 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/42068/1
diff --git a/src/soc/amd/picasso/chip.c b/src/soc/amd/picasso/chip.c index ed39b53..503dc62 100644 --- a/src/soc/amd/picasso/chip.c +++ b/src/soc/amd/picasso/chip.c @@ -16,6 +16,8 @@
/* Supplied by i2c.c */ extern struct device_operations picasso_i2c_mmio_ops; +/* Supplied by uart.c */ +extern struct device_operations picasso_uart_mmio_ops;
struct device_operations cpu_bus_ops = { .read_resources = noop_read_resources, @@ -127,6 +129,12 @@ case APU_I2C4_BASE: dev->ops = &picasso_i2c_mmio_ops; break; + case APU_UART0_BASE: + case APU_UART1_BASE: + case APU_UART2_BASE: + case APU_UART3_BASE: + dev->ops = &picasso_uart_mmio_ops; + break; } }
diff --git a/src/soc/amd/picasso/uart.c b/src/soc/amd/picasso/uart.c index 6ae79b95..b1331ec 100644 --- a/src/soc/amd/picasso/uart.c +++ b/src/soc/amd/picasso/uart.c @@ -74,3 +74,26 @@ { return CONFIG(PICASSO_UART_48MZ) ? 48000000 : 115200 * 16; } + +static const char *uart_acpi_name(const struct device *dev) +{ + switch (dev->path.mmio.addr) { + case APU_UART0_BASE: + return "FUR0"; + case APU_UART1_BASE: + return "FUR1"; + case APU_UART2_BASE: + return "FUR2"; + case APU_UART3_BASE: + return "FUR3"; + default: + return NULL; + } +} + +struct device_operations picasso_uart_mmio_ops = { + .read_resources = noop_read_resources, + .set_resources = noop_set_resources, + .scan_bus = scan_static_bus, + .acpi_name = uart_acpi_name, +};
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42068 )
Change subject: soc/amd/picasso: Add device operations for UART MMIO devices ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/42068/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42068/1//COMMIT_MSG@13 PS1, Line 13: follow-up CLs Can you hold off until I get power resources added to the UART nodes? Or you can do it if you have time :)
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42068 )
Change subject: soc/amd/picasso: Add device operations for UART MMIO devices ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42068/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42068/1//COMMIT_MSG@13 PS1, Line 13: follow-up CLs
Can you hold off until I get power resources added to the UART nodes? Or you can do it if you have t […]
I am fine either ways. I just pushed this series to unblock Bhanu. Move to SSDT can be done at a lower priority. Is there a bug where you are tracking the power resource change?
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42068 )
Change subject: soc/amd/picasso: Add device operations for UART MMIO devices ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42068/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42068/1//COMMIT_MSG@13 PS1, Line 13: follow-up CLs
I am fine either ways. I just pushed this series to unblock Bhanu. […]
I was going to handle it as part of b/153001807. But we can raise a new bug.
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42068 )
Change subject: soc/amd/picasso: Add device operations for UART MMIO devices ......................................................................
soc/amd/picasso: Add device operations for UART MMIO devices
This change adds device_operations for UART MMIO devices that provides following operations: 1. uart_acpi_name: Returns ACPI name of UART device. Generation of UART device node is not yet moved to SSDT, but will be done in follow-up CLs. 2. scan_bus: Uses scan_static_bus to scan devices added under the UART devices. This allows mainboard to add devices under the UART MMIO device.
Change-Id: I18abbe88952e7006668657eb1d0c177e53e95850 Signed-off-by: Furquan Shaikh furquan@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/42068 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Raul Rangel rrangel@chromium.org --- M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/uart.c 2 files changed, 31 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved
diff --git a/src/soc/amd/picasso/chip.c b/src/soc/amd/picasso/chip.c index d364b9a..2e5fae5 100644 --- a/src/soc/amd/picasso/chip.c +++ b/src/soc/amd/picasso/chip.c @@ -15,6 +15,8 @@
/* Supplied by i2c.c */ extern struct device_operations picasso_i2c_mmio_ops; +/* Supplied by uart.c */ +extern struct device_operations picasso_uart_mmio_ops;
struct device_operations cpu_bus_ops = { .read_resources = noop_read_resources, @@ -126,6 +128,12 @@ case APU_I2C4_BASE: dev->ops = &picasso_i2c_mmio_ops; break; + case APU_UART0_BASE: + case APU_UART1_BASE: + case APU_UART2_BASE: + case APU_UART3_BASE: + dev->ops = &picasso_uart_mmio_ops; + break; } }
diff --git a/src/soc/amd/picasso/uart.c b/src/soc/amd/picasso/uart.c index 6ae79b95..b1331ec 100644 --- a/src/soc/amd/picasso/uart.c +++ b/src/soc/amd/picasso/uart.c @@ -74,3 +74,26 @@ { return CONFIG(PICASSO_UART_48MZ) ? 48000000 : 115200 * 16; } + +static const char *uart_acpi_name(const struct device *dev) +{ + switch (dev->path.mmio.addr) { + case APU_UART0_BASE: + return "FUR0"; + case APU_UART1_BASE: + return "FUR1"; + case APU_UART2_BASE: + return "FUR2"; + case APU_UART3_BASE: + return "FUR3"; + default: + return NULL; + } +} + +struct device_operations picasso_uart_mmio_ops = { + .read_resources = noop_read_resources, + .set_resources = noop_set_resources, + .scan_bus = scan_static_bus, + .acpi_name = uart_acpi_name, +};
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42068 )
Change subject: soc/amd/picasso: Add device operations for UART MMIO devices ......................................................................
Patch Set 2:
this breaks the build for Mandolin with serial on LPC debug card enabled, since CONFIG_PICASSO_UART isn't set there resulting in a missing symbol error
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42068 )
Change subject: soc/amd/picasso: Add device operations for UART MMIO devices ......................................................................
Patch Set 2:
Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/4986 "QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/4985 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/4984 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/4983
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Felix Held (Code Review)
-
Furquan Shaikh (Code Review)
-
Patrick Georgi (Code Review)
-
Raul Rangel (Code Review)