Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UART2 on PCH-H ......................................................................
soc/intel/cannonlake: Add support for UART2 on PCH-H
On PCH-H the UART2 can't be used as PCI device as the parent multi function device isn't available.
As workaround add an ACPI driver for PCH-H UART2 that generates ACPI code for the Intel LPSS ACPI driver.
Tested on Linux with Sunrise Point ACPI ID for UART2.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/cannonlake/Makefile.inc M src/soc/intel/cannonlake/acpi/serialio.asl A src/soc/intel/cannonlake/pch_h.c M src/soc/intel/common/block/include/intelblocks/uart.h M src/soc/intel/common/block/uart/uart.c 5 files changed, 98 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40405/1
diff --git a/src/soc/intel/cannonlake/Makefile.inc b/src/soc/intel/cannonlake/Makefile.inc index c744e99..e263264 100644 --- a/src/soc/intel/cannonlake/Makefile.inc +++ b/src/soc/intel/cannonlake/Makefile.inc @@ -54,6 +54,11 @@ ramstage-y += vr_config.c ramstage-y += sd.c ramstage-y += xhci.c +ifeq ($(CONFIG_HAVE_ACPI_TABLES),y) +ifeq ($(CONFIG_SOC_INTEL_COMMON_BLOCK_LPSS),y) +ramstage-$(CONFIG_SOC_INTEL_CANNONLAKE_PCH_H) += pch_h.c +endif +endif
smm-y += elog.c smm-y += p2sb.c diff --git a/src/soc/intel/cannonlake/acpi/serialio.asl b/src/soc/intel/cannonlake/acpi/serialio.asl index 2785e3d..5e28b98 100644 --- a/src/soc/intel/cannonlake/acpi/serialio.asl +++ b/src/soc/intel/cannonlake/acpi/serialio.asl @@ -69,8 +69,11 @@ Name (_DDN, "Serial IO UART Controller 1") }
+ +#if !CONFIG(SOC_INTEL_CANNONLAKE_PCH_H) Device (UAR2) { Name (_ADR, 0x00190002) Name (_DDN, "Serial IO UART Controller 2") } +#endif diff --git a/src/soc/intel/cannonlake/pch_h.c b/src/soc/intel/cannonlake/pch_h.c new file mode 100644 index 0000000..84a75e8 --- /dev/null +++ b/src/soc/intel/cannonlake/pch_h.c @@ -0,0 +1,77 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* This file is part of the coreboot project. */ +#include <arch/acpi.h> +#include <arch/acpigen.h> +#include <device/device.h> +#include <device/pci.h> +#include <device/pci_def.h> +#include <device/pci_ids.h> +#include <device/pci_ops.h> +#include <soc/irq.h> +#include <intelblocks/uart.h> +#include <console/console.h> + +/* + * On PCH-H the I2C4 0:19.0 device isn't usable and thus 0:19.2 isn't detected + * using standard PCI probing. + * Generate an ACPI entry if the device is enabled in devicetree for the ACPI + * LPSS driver. + */ +static void pch_h_uart2_fill_ssdt(struct device *dev) +{ + struct acpi_irq irq = ACPI_IRQ_LEVEL_LOW(LPSS_UART2_IRQ); + const char *scope = acpi_device_scope(dev); + printk(BIOS_ERR, "ACPIGEN: %s\n", acpi_device_name(dev)); + struct resource *res; + if (!scope) + return; + + res = find_resource(dev, PCI_BASE_ADDRESS_0); + if (!res) + return; + + /* Device */ + acpigen_write_scope(scope); + acpigen_write_device(acpi_device_name(dev)); + acpigen_write_name_string("_HID", acpi_device_hid(dev)); + /* + * INT344A is the Sunrise Point HID, as the Linux kernel doesn't support + * CannonPoint yet... + */ + acpigen_write_name_string("_CID", "INT344A"); + acpi_device_write_uid(dev); + acpigen_write_name_string("_DDN", "LPSS ACPI UART"); + acpigen_write_STA(acpi_device_status(dev)); + + /* Resources */ + acpigen_write_name("_CRS"); + acpigen_write_resourcetemplate_header(); + + acpi_device_write_interrupt(&irq); + acpigen_write_mem32fixed(1, res->base, res->size); + + acpigen_write_resourcetemplate_footer(); + + acpigen_pop_len(); /* Device */ + acpigen_pop_len(); /* Scope */ +} + +static const char *pch_h_uart2_acpi_hid(const struct device *dev) +{ + return "INT34BA"; +} + +static struct device_operations device_ops_cnp_h = { + .read_resources = uart_common_read_resources, + .set_resources = pci_dev_set_resources, + .enable_resources = uart_common_enable_resources, + .ops_pci = &pci_dev_ops_pci, + .acpi_hid = pch_h_uart2_acpi_hid, + .acpi_fill_ssdt_generator = pch_h_uart2_fill_ssdt, +}; + +static const struct pci_driver pch_h_uart __pci_driver = { + .ops = &device_ops_cnp_h, + .vendor = PCI_VENDOR_ID_INTEL, + .device = PCI_DEVICE_ID_INTEL_CNP_H_UART2, +}; diff --git a/src/soc/intel/common/block/include/intelblocks/uart.h b/src/soc/intel/common/block/include/intelblocks/uart.h index cc27f0e..a51a739 100644 --- a/src/soc/intel/common/block/include/intelblocks/uart.h +++ b/src/soc/intel/common/block/include/intelblocks/uart.h @@ -31,6 +31,16 @@ void uart_common_init(const struct device *dev, uintptr_t baseaddr);
/* + * Common routine to enable UART resources in PCI config space. + */ +void uart_common_enable_resources(struct device *dev); + +/* + * Common routine to read UART resources in PCI config space. + */ +void uart_common_read_resources(struct device *dev); + +/* * Check if UART debug controller is initialized * Returns: * true = If debug controller PCI config space is initialized and device is diff --git a/src/soc/intel/common/block/uart/uart.c b/src/soc/intel/common/block/uart/uart.c index 7d75bdd..3464571 100644 --- a/src/soc/intel/common/block/uart/uart.c +++ b/src/soc/intel/common/block/uart/uart.c @@ -143,7 +143,7 @@
#if ENV_RAMSTAGE
-static void uart_read_resources(struct device *dev) +void uart_common_read_resources(struct device *dev) { pci_dev_read_resources(dev);
@@ -213,7 +213,7 @@ return pch_uart_init_debug_controller_on_resume(); }
-static void uart_common_enable_resources(struct device *dev) +void uart_common_enable_resources(struct device *dev) { pci_dev_enable_resources(dev);
@@ -227,7 +227,7 @@ }
static struct device_operations device_ops = { - .read_resources = uart_read_resources, + .read_resources = uart_common_read_resources, .set_resources = pci_dev_set_resources, .enable_resources = uart_common_enable_resources, .ops_pci = &pci_dev_ops_pci, @@ -256,7 +256,6 @@ PCI_DEVICE_ID_INTEL_GLK_UART3, PCI_DEVICE_ID_INTEL_CNP_H_UART0, PCI_DEVICE_ID_INTEL_CNP_H_UART1, - PCI_DEVICE_ID_INTEL_CNP_H_UART2, PCI_DEVICE_ID_INTEL_ICP_UART0, PCI_DEVICE_ID_INTEL_ICP_UART1, PCI_DEVICE_ID_INTEL_ICP_UART2,
Hello Philipp Deppenwiese, wouter.eckhardt@prodrive-technologies.com, build bot (Jenkins), Justin van Son, Patrick Georgi, Martin Roth, Christian Walter, Marcello Sylvester Bauer, Patrick Rudolph, Stef van Os,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40405
to look at the new patch set (#2).
Change subject: [WIP]soc/intel/cannonlake: Add support for UART2 on PCH-H ......................................................................
[WIP]soc/intel/cannonlake: Add support for UART2 on PCH-H
On PCH-H the UART2 can't be used as PCI device as the parent multi function device isn't available.
As workaround add an ACPI driver for PCH-H UART2 that generates ACPI code for the Intel LPSS ACPI driver.
Tested on Linux with Sunrise Point ACPI ID for UART2.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/cannonlake/Makefile.inc M src/soc/intel/cannonlake/acpi/serialio.asl A src/soc/intel/cannonlake/pch_h.c M src/soc/intel/common/block/include/intelblocks/uart.h M src/soc/intel/common/block/uart/uart.c 5 files changed, 98 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40405/2
Stef van Os has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: [WIP]soc/intel/cannonlake: Add support for UART2 on PCH-H ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40405/2/src/soc/intel/cannonlake/ac... File src/soc/intel/cannonlake/acpi/serialio.asl:
https://review.coreboot.org/c/coreboot/+/40405/2/src/soc/intel/cannonlake/ac... PS2, Line 72: double newline
https://review.coreboot.org/c/coreboot/+/40405/2/src/soc/intel/cannonlake/pc... File src/soc/intel/cannonlake/pch_h.c:
https://review.coreboot.org/c/coreboot/+/40405/2/src/soc/intel/cannonlake/pc... PS2, Line 24: printk(BIOS_ERR, "ACPIGEN: %s\n", acpi_device_name(dev)); should this be BIOS_ERR?
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: [WIP]soc/intel/cannonlake: Add support for UART2 on PCH-H ......................................................................
Patch Set 2: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/40405/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40405/2//COMMIT_MSG@15 PS2, Line 15: Linux What version?
Hello Philipp Deppenwiese, wouter.eckhardt@prodrive-technologies.com, build bot (Jenkins), Justin van Son, Patrick Georgi, Martin Roth, Paul Menzel, Christian Walter, Marcello Sylvester Bauer, Patrick Rudolph, Stef van Os,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40405
to look at the new patch set (#3).
Change subject: soc/intel/cannonlake: Add support for UART2 on PCH-H ......................................................................
soc/intel/cannonlake: Add support for UART2 on PCH-H
On PCH-H the UART2 can't be used as PCI device as the parent multi function device (0:19.0) isn't available.
As workaround add an ACPI driver for PCH-H UART2 that generates ACPI code for the Intel LPSS ACPI driver.
Tested on Linux 5.6 with Sunrise Point ACPI ID for UART2.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/cannonlake/Makefile.inc M src/soc/intel/cannonlake/acpi/serialio.asl A src/soc/intel/cannonlake/pch_h.c M src/soc/intel/common/block/include/intelblocks/uart.h M src/soc/intel/common/block/uart/uart.c 5 files changed, 99 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40405/3
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UART2 on PCH-H ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/40405/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40405/2//COMMIT_MSG@15 PS2, Line 15: Linux
What version?
Done
https://review.coreboot.org/c/coreboot/+/40405/2/src/soc/intel/cannonlake/ac... File src/soc/intel/cannonlake/acpi/serialio.asl:
https://review.coreboot.org/c/coreboot/+/40405/2/src/soc/intel/cannonlake/ac... PS2, Line 72:
double newline
Done
https://review.coreboot.org/c/coreboot/+/40405/2/src/soc/intel/cannonlake/pc... File src/soc/intel/cannonlake/pch_h.c:
https://review.coreboot.org/c/coreboot/+/40405/2/src/soc/intel/cannonlake/pc... PS2, Line 24: printk(BIOS_ERR, "ACPIGEN: %s\n", acpi_device_name(dev));
should this be BIOS_ERR?
Removed
Stef van Os has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UART2 on PCH-H ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40405/2/src/soc/intel/cannonlake/ac... File src/soc/intel/cannonlake/acpi/serialio.asl:
https://review.coreboot.org/c/coreboot/+/40405/2/src/soc/intel/cannonlake/ac... PS2, Line 72:
Done
Done
https://review.coreboot.org/c/coreboot/+/40405/2/src/soc/intel/cannonlake/pc... File src/soc/intel/cannonlake/pch_h.c:
https://review.coreboot.org/c/coreboot/+/40405/2/src/soc/intel/cannonlake/pc... PS2, Line 24: printk(BIOS_ERR, "ACPIGEN: %s\n", acpi_device_name(dev));
Removed
Done
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UART2 on PCH-H ......................................................................
Patch Set 4: Code-Review+1
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UART2 on PCH-H ......................................................................
Patch Set 4: -Code-Review
(2 comments)
https://review.coreboot.org/c/coreboot/+/40405/4/src/soc/intel/cannonlake/pc... File src/soc/intel/cannonlake/pch_h.c:
https://review.coreboot.org/c/coreboot/+/40405/4/src/soc/intel/cannonlake/pc... PS4, Line 3: #include <arch/acpi.h> fix includes
https://review.coreboot.org/c/coreboot/+/40405/4/src/soc/intel/cannonlake/pc... PS4, Line 4: arch acpi/acpigen.h
Justin van Son has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UART2 on PCH-H ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40405/4/src/soc/intel/cannonlake/pc... File src/soc/intel/cannonlake/pch_h.c:
https://review.coreboot.org/c/coreboot/+/40405/4/src/soc/intel/cannonlake/pc... PS4, Line 20: static void pch_h_uart2_fill_ssdt(struct device *dev) Shouldn't this be const struct device?
Christian Walter has uploaded a new patch set (#5) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
soc/intel/cannonlake: Add support for UARTs on PCH-H
On PCH-H all UARTs can't be used under Windows.
As workaround add an ACPI driver for PCH-H UART2 that generates ACPI code for the Intel LPSS ACPI driver.
Tested on Linux 5.6 with Sunrise Point ACPI ID for UART2. Tested on Windows for all other UARTs.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/cannonlake/Makefile.inc M src/soc/intel/cannonlake/acpi/serialio.asl A src/soc/intel/cannonlake/pch_h.c M src/soc/intel/common/block/include/intelblocks/uart.h M src/soc/intel/common/block/uart/uart.c 5 files changed, 148 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40405/5
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40405/5/src/soc/intel/cannonlake/pc... File src/soc/intel/cannonlake/pch_h.c:
https://review.coreboot.org/c/coreboot/+/40405/5/src/soc/intel/cannonlake/pc... PS5, Line 18: switch (dev->path.pci.devfn) that open brace { should be on the previous line
https://review.coreboot.org/c/coreboot/+/40405/5/src/soc/intel/cannonlake/pc... PS5, Line 92: switch (dev->path.pci.devfn) that open brace { should be on the previous line
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40405/6/src/soc/intel/cannonlake/pc... File src/soc/intel/cannonlake/pch_h.c:
https://review.coreboot.org/c/coreboot/+/40405/6/src/soc/intel/cannonlake/pc... PS6, Line 18: switch (dev->path.pci.devfn) that open brace { should be on the previous line
https://review.coreboot.org/c/coreboot/+/40405/6/src/soc/intel/cannonlake/pc... PS6, Line 92: switch (dev->path.pci.devfn) that open brace { should be on the previous line
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
Patch Set 6:
(3 comments)
https://review.coreboot.org/c/coreboot/+/40405/4/src/soc/intel/cannonlake/pc... File src/soc/intel/cannonlake/pch_h.c:
https://review.coreboot.org/c/coreboot/+/40405/4/src/soc/intel/cannonlake/pc... PS4, Line 3: #include <arch/acpi.h>
fix includes
Ack
https://review.coreboot.org/c/coreboot/+/40405/4/src/soc/intel/cannonlake/pc... PS4, Line 4: arch
acpi/acpigen. […]
Ack
https://review.coreboot.org/c/coreboot/+/40405/4/src/soc/intel/cannonlake/pc... PS4, Line 20: static void pch_h_uart2_fill_ssdt(struct device *dev)
Shouldn't this be const struct device?
Ack
Christian Walter has uploaded a new patch set (#7) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
soc/intel/cannonlake: Add support for UARTs on PCH-H
On PCH-H all UARTs can't be used under Windows.
As workaround add an ACPI driver for PCH-H UART2 that generates ACPI code for the Intel LPSS ACPI driver.
Tested on Linux 5.6 with Sunrise Point ACPI ID for UART2. Tested on Windows for all other UARTs.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Signed-off-by: Christian Walter christian.walter@9elements.com --- M src/soc/intel/cannonlake/Makefile.inc M src/soc/intel/cannonlake/acpi/serialio.asl A src/soc/intel/cannonlake/pch_h.c M src/soc/intel/common/block/include/intelblocks/uart.h M src/soc/intel/common/block/uart/uart.c 5 files changed, 146 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40405/7
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40405/7/src/soc/intel/cannonlake/pc... File src/soc/intel/cannonlake/pch_h.c:
https://review.coreboot.org/c/coreboot/+/40405/7/src/soc/intel/cannonlake/pc... PS7, Line 23: break; one for all
Christian Walter has uploaded a new patch set (#8) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
soc/intel/cannonlake: Add support for UARTs on PCH-H
On PCH-H all UARTs can't be used under Windows.
As workaround add an ACPI driver for PCH-H UART2 that generates ACPI code for the Intel LPSS ACPI driver.
Tested on Linux 5.6 with Sunrise Point ACPI ID for UART2. Tested on Windows for all other UARTs.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Signed-off-by: Christian Walter christian.walter@9elements.com --- M src/soc/intel/cannonlake/Makefile.inc M src/soc/intel/cannonlake/acpi/serialio.asl A src/soc/intel/cannonlake/pch_h.c M src/soc/intel/common/block/include/intelblocks/uart.h M src/soc/intel/common/block/uart/uart.c 5 files changed, 146 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40405/8
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40405/7/src/soc/intel/cannonlake/pc... File src/soc/intel/cannonlake/pch_h.c:
https://review.coreboot.org/c/coreboot/+/40405/7/src/soc/intel/cannonlake/pc... PS7, Line 23: break;
one for all
Done
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
Patch Set 8: Code-Review+2
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
Patch Set 8:
(3 comments)
https://review.coreboot.org/c/coreboot/+/40405/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40405/8//COMMIT_MSG@9 PS8, Line 9: all There's no difference between pch-h and pch-lp for uart0 and uart1. So pch-lp is likely also affected.
https://review.coreboot.org/c/coreboot/+/40405/8//COMMIT_MSG@11 PS8, Line 11: PCH Not accurate any more.
https://review.coreboot.org/c/coreboot/+/40405/8/src/soc/intel/cannonlake/ac... File src/soc/intel/cannonlake/acpi/serialio.asl:
https://review.coreboot.org/c/coreboot/+/40405/8/src/soc/intel/cannonlake/ac... PS8, Line 61: UAR0 Those entries conflict with the ssdt. You advertise the same device twice.
Christian Walter has uploaded a new patch set (#9) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
soc/intel/cannonlake: Add support for UARTs on PCH-H
On PCH-H all UARTs can't be used under Windows.
As workaround add an ACPI driver for PCH-H UART2 that generates ACPI code for the Intel LPSS ACPI driver.
Tested on Linux 5.6 with Sunrise Point ACPI ID for UART2. Tested on Windows for all other UARTs.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/cannonlake/Makefile.inc M src/soc/intel/cannonlake/acpi/serialio.asl A src/soc/intel/cannonlake/pch_h.c M src/soc/intel/common/block/include/intelblocks/uart.h M src/soc/intel/common/block/uart/uart.c 5 files changed, 148 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40405/9
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
Patch Set 9:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40405/9/src/soc/intel/cannonlake/pc... File src/soc/intel/cannonlake/pch_h.c:
https://review.coreboot.org/c/coreboot/+/40405/9/src/soc/intel/cannonlake/pc... PS9, Line 18: switch (dev->path.pci.devfn) that open brace { should be on the previous line
https://review.coreboot.org/c/coreboot/+/40405/9/src/soc/intel/cannonlake/pc... PS9, Line 92: switch (dev->path.pci.devfn) that open brace { should be on the previous line
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
Patch Set 9:
(3 comments)
https://review.coreboot.org/c/coreboot/+/40405/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40405/9//COMMIT_MSG@9 PS9, Line 9: On PCH-H all UARTs can't be used under Windows. You probably mean "not every UART can be used"?
https://review.coreboot.org/c/coreboot/+/40405/9//COMMIT_MSG@12 PS9, Line 12: ACPI code for the Intel LPSS ACPI driver. This is a separate topic and should be in its own change. For the above, you remove false advertisement. For this you add support code.
I also see no reason to call it a work around? Isn't it just standard ACPI code that Windows wants to see?
https://review.coreboot.org/c/coreboot/+/40405/9/src/soc/intel/cannonlake/pc... File src/soc/intel/cannonlake/pch_h.c:
PS9: I generally prefer per-platform drivers if there is anything platform specific. But I don't see it in this case. Can you elaborate, why this can't be common code?
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
Patch Set 9:
(4 comments)
https://review.coreboot.org/c/coreboot/+/40405/9/src/soc/intel/cannonlake/pc... File src/soc/intel/cannonlake/pch_h.c:
PS9:
I generally prefer per-platform drivers if there is anything platform […]
it evolved into something generic. besides _HID and _CID there's nothing special about it.
https://review.coreboot.org/c/coreboot/+/40405/9/src/soc/intel/cannonlake/pc... PS9, Line 28: struct acpi_irq irq = ACPI_IRQ_LEVEL_LOW(LPSS_UART0_IRQ); LPSS_UART1_IRQ
https://review.coreboot.org/c/coreboot/+/40405/9/src/soc/intel/cannonlake/pc... PS9, Line 34: LPSS_UART0_IRQ LPSS_UART2_IRQ
https://review.coreboot.org/c/coreboot/+/40405/9/src/soc/intel/cannonlake/pc... PS9, Line 66: ", according to the ACPI spec 6.1 Chapter 6.1 "For devices on an enumerable type of bus, such as a PCI bus, ... ACPI system firmware must supply an _ADR ..." and "A device object must contain either an _HID object or an _ADR object, but should not contain both."
Should we limit this driver to only generate code when the LPSS Uart works in "ACPI mode" and be a noop when running in "PCI mode"?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40405/9/src/soc/intel/cannonlake/pc... File src/soc/intel/cannonlake/pch_h.c:
https://review.coreboot.org/c/coreboot/+/40405/9/src/soc/intel/cannonlake/pc... PS9, Line 66: ",
according to the ACPI spec 6.1 Chapter 6.1 […]
That would be hard. For ACPI mode, the hardcoded _ADR should be removed. So the generally encouraged way is to always have a runtime driver and let it either emit _ADR or _HID depending on the mode.
For the rest of the driver, it depends on each object. For instance, _CRS should only be necessary in ACPI mode (unless the PCI device is not fully PCI compliant).
Christian Walter has uploaded a new patch set (#10) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
soc/intel/cannonlake: Add support for UARTs on PCH-H
On PCH-H all UARTs can't be used under Windows.
As workaround add an ACPI driver for PCH-H UART2 that generates ACPI code for the Intel LPSS ACPI driver.
Tested on Linux 5.6 with Sunrise Point ACPI ID for UART2. Tested on Windows for all other UARTs.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/cannonlake/Makefile.inc M src/soc/intel/cannonlake/acpi/serialio.asl A src/soc/intel/cannonlake/pch_h.c M src/soc/intel/common/block/include/intelblocks/uart.h M src/soc/intel/common/block/uart/uart.c 5 files changed, 148 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40405/10
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
Patch Set 10:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40405/10/src/soc/intel/cannonlake/p... File src/soc/intel/cannonlake/pch_h.c:
https://review.coreboot.org/c/coreboot/+/40405/10/src/soc/intel/cannonlake/p... PS10, Line 18: switch (dev->path.pci.devfn) that open brace { should be on the previous line
https://review.coreboot.org/c/coreboot/+/40405/10/src/soc/intel/cannonlake/p... PS10, Line 92: switch (dev->path.pci.devfn) that open brace { should be on the previous line
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
Patch Set 10:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40405/9/src/soc/intel/cannonlake/pc... File src/soc/intel/cannonlake/pch_h.c:
https://review.coreboot.org/c/coreboot/+/40405/9/src/soc/intel/cannonlake/pc... PS9, Line 28: struct acpi_irq irq = ACPI_IRQ_LEVEL_LOW(LPSS_UART0_IRQ);
LPSS_UART1_IRQ
Ack
https://review.coreboot.org/c/coreboot/+/40405/9/src/soc/intel/cannonlake/pc... PS9, Line 34: LPSS_UART0_IRQ
LPSS_UART2_IRQ
Ack
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
Patch Set 11:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40405/11/src/soc/intel/cannonlake/p... File src/soc/intel/cannonlake/pch_h.c:
https://review.coreboot.org/c/coreboot/+/40405/11/src/soc/intel/cannonlake/p... PS11, Line 18: switch (dev->path.pci.devfn) that open brace { should be on the previous line
https://review.coreboot.org/c/coreboot/+/40405/11/src/soc/intel/cannonlake/p... PS11, Line 92: switch (dev->path.pci.devfn) that open brace { should be on the previous line
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
Patch Set 12:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... File src/soc/intel/cannonlake/pch_h.c:
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 18: switch (dev->path.pci.devfn) that open brace { should be on the previous line
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 92: switch (dev->path.pci.devfn) that open brace { should be on the previous line
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
Patch Set 12: Code-Review+1
(11 comments)
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... File src/soc/intel/cannonlake/pch_h.c:
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 2: /* This file is part of the coreboot project. */ drop this comment, but leave a blank line between the SPDX identifier and the #includes
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 15: double blank line
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 18: switch (dev->path.pci.devfn)
that open brace { should be on the previous line
agreed.
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 20: // UART0 Such useful comments! I don't think we need them, do we? 😄
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 22: struct acpi_irq irq maybe declare this outside of the switch? that way, no braces are needed.
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 43: Another double blank line
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 52: spurious blank line
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 92: switch (dev->path.pci.devfn)
that open brace { should be on the previous line
Would be good to fix
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 94: // UART0 We can live without these comments
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 112: double empty line
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 119: .acpi_fill_ssdt = pch_h_uart_fill_ssdt, We can do away with one tab less
Angel Pons has uploaded a new patch set (#13) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
soc/intel/cannonlake: Add support for UARTs on PCH-H
On PCH-H all UARTs can't be used under Windows.
As workaround add an ACPI driver for PCH-H UART2 that generates ACPI code for the Intel LPSS ACPI driver.
Tested on Linux 5.6 with Sunrise Point ACPI ID for UART2. Tested on Windows for all other UARTs.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/cannonlake/Makefile.inc M src/soc/intel/cannonlake/acpi/serialio.asl A src/soc/intel/cannonlake/pch_h.c M src/soc/intel/common/block/include/intelblocks/uart.h M src/soc/intel/common/block/uart/uart.c 5 files changed, 137 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40405/13
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
Patch Set 13:
(12 comments)
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... File src/soc/intel/cannonlake/pch_h.c:
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 2: /* This file is part of the coreboot project. */
drop this comment, but leave a blank line between the SPDX identifier and the #includes
Done, and sorted the includes
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 15:
double blank line
Done
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 18: switch (dev->path.pci.devfn)
agreed.
Done
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 20: // UART0
Such useful comments! I don't think we need them, do we? 😄
Done
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 22: struct acpi_irq irq
maybe declare this outside of the switch? that way, no braces are needed.
Done
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 43:
Another double blank line
Done
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 52:
spurious blank line
Done
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 92: switch (dev->path.pci.devfn)
Would be good to fix
Done
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 94: // UART0
We can live without these comments
Done
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 112:
double empty line
Done
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 119: .acpi_fill_ssdt = pch_h_uart_fill_ssdt,
We can do away with one tab less
Done
https://review.coreboot.org/c/coreboot/+/40405/13/src/soc/intel/cannonlake/p... File src/soc/intel/cannonlake/pch_h.c:
https://review.coreboot.org/c/coreboot/+/40405/13/src/soc/intel/cannonlake/p... PS13, Line 56: AAAAA
Angel Pons has uploaded a new patch set (#14) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
soc/intel/cannonlake: Add support for UARTs on PCH-H
On PCH-H all UARTs can't be used under Windows.
As workaround add an ACPI driver for PCH-H UART2 that generates ACPI code for the Intel LPSS ACPI driver.
Tested on Linux 5.6 with Sunrise Point ACPI ID for UART2. Tested on Windows for all other UARTs.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/cannonlake/Makefile.inc M src/soc/intel/cannonlake/acpi/serialio.asl A src/soc/intel/cannonlake/pch_h.c M src/soc/intel/common/block/include/intelblocks/uart.h M src/soc/intel/common/block/uart/uart.c 5 files changed, 137 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40405/14
Hello Philipp Deppenwiese, wouter.eckhardt@prodrive-technologies.com, build bot (Jenkins), Justin van Son, Patrick Georgi, Martin Roth, Paul Menzel, Christian Walter, Angel Pons, Marcello Sylvester Bauer, Patrick Rudolph, Stef van Os,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40405
to look at the new patch set (#15).
Change subject: soc/intel/cannonlake|soc/intel/common: Add support for UARTs on PCH-H ......................................................................
soc/intel/cannonlake|soc/intel/common: Add support for UARTs on PCH-H
On Cannonpoint PCH-H all UARTs can't be used under Windows.
As workaround add an ACPI driver for Sunrisepoint and Cannonpoint that generates ACPI code for the Intel LPSS ACPI driver, when the device is running in "ACPI mode".
Tested on Linux 5.6 with Sunrise Point ACPI ID for UART2. Tested on Windows for all other UARTs.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/cannonlake/acpi/serialio.asl M src/soc/intel/common/block/uart/uart.c 2 files changed, 105 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40405/15
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake|soc/intel/common: Add support for UARTs on PCH-H ......................................................................
Patch Set 14:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40405/9/src/soc/intel/cannonlake/pc... File src/soc/intel/cannonlake/pch_h.c:
PS9:
it evolved into something generic. besides _HID and _CID there's nothing special about it.
Move to common code.
https://review.coreboot.org/c/coreboot/+/40405/9/src/soc/intel/cannonlake/pc... PS9, Line 66: ",
That would be hard. For ACPI mode, the hardcoded _ADR should be removed. […]
Added detection for ACPI mode and only emmit the _HID when running in such.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake|soc/intel/common: Add support for UARTs on PCH-H ......................................................................
Patch Set 15: Code-Review+1
(3 comments)
https://review.coreboot.org/c/coreboot/+/40405/15/src/soc/intel/common/block... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/40405/15/src/soc/intel/common/block... PS15, Line 230: Double newline
https://review.coreboot.org/c/coreboot/+/40405/15/src/soc/intel/common/block... PS15, Line 273: fffff there's five `f`
https://review.coreboot.org/c/coreboot/+/40405/15/src/soc/intel/common/block... PS15, Line 306: switch (dev->device) { Not all cases are handled here. Is this intentional?
Hello Philipp Deppenwiese, wouter.eckhardt@prodrive-technologies.com, build bot (Jenkins), Justin van Son, Patrick Georgi, Martin Roth, Paul Menzel, Christian Walter, Angel Pons, Marcello Sylvester Bauer, Patrick Rudolph, Stef van Os,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40405
to look at the new patch set (#16).
Change subject: soc/intel/cannonlake|soc/intel/common: Add support for UARTs on PCH-H ......................................................................
soc/intel/cannonlake|soc/intel/common: Add support for UARTs on PCH-H
On Cannonpoint PCH-H all UARTs can't be used under Windows.
As workaround add an ACPI driver for Sunrisepoint and Cannonpoint that generates ACPI code for the Intel LPSS ACPI driver, when the device is running in "ACPI mode".
Tested on Linux 5.6 with Sunrise Point ACPI ID for UART2. Tested on Windows for all other UARTs.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/cannonlake/acpi/serialio.asl M src/soc/intel/common/block/uart/uart.c 2 files changed, 107 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40405/16
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake|soc/intel/common: Add support for UARTs on PCH-H ......................................................................
Patch Set 16:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40405/15/src/soc/intel/common/block... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/40405/15/src/soc/intel/common/block... PS15, Line 273: fffff
there's five `f`
Done
https://review.coreboot.org/c/coreboot/+/40405/15/src/soc/intel/common/block... PS15, Line 306: switch (dev->device) {
Not all cases are handled here. […]
APL doesn't seem to support LPSS uart. The other IDs are secret and not part of the Linux kernel code.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake|soc/intel/common: Add support for UARTs on PCH-H ......................................................................
Patch Set 16:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40405/15/src/soc/intel/common/block... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/40405/15/src/soc/intel/common/block... PS15, Line 306: switch (dev->device) {
APL doesn't seem to support LPSS uart. […]
So in such cases, we don't generate ACPI? Hrm, I see.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake|soc/intel/common: Add support for UARTs on PCH-H ......................................................................
Patch Set 16: Code-Review+1
Hello Philipp Deppenwiese, wouter.eckhardt@prodrive-technologies.com, build bot (Jenkins), Justin van Son, Patrick Georgi, Martin Roth, Paul Menzel, Christian Walter, Angel Pons, Marcello Sylvester Bauer, Patrick Rudolph, Stef van Os,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40405
to look at the new patch set (#17).
Change subject: soc/intel/cannonlake|soc/intel/common: Add support for UARTs on PCH-H ......................................................................
soc/intel/cannonlake|soc/intel/common: Add support for UARTs on PCH-H
Add support for LPSS UART in ACPI mode.
Emit ACPI code for LPSS UARTs operating in ACPI mode. In this mode the device vendor ID reads as 0xffff, the PCI devices is still operating as usual.
Add ACPI devices IDs for APL, GLK, SPT, SPT_H and CNP_H.
Tested on Linux 5.6 with Sunrise Point ACPI ID for UART2. Tested on Windows for all other UARTs.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/common/block/uart/uart.c 1 file changed, 122 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40405/17
Hello Philipp Deppenwiese, wouter.eckhardt@prodrive-technologies.com, build bot (Jenkins), Justin van Son, Patrick Georgi, Martin Roth, Paul Menzel, Christian Walter, Angel Pons, Marcello Sylvester Bauer, Patrick Rudolph, Stef van Os,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40405
to look at the new patch set (#18).
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
soc/intel/common: Add support for LPSS UART in ACPI mode
Add support for LPSS UART in ACPI mode.
Emit ACPI code for LPSS UARTs operating in ACPI mode. In this mode the device vendor ID reads as 0xffff, the PCI devices is still operating as usual.
Add ACPI devices IDs for APL, GLK, SPT, SPT_H and CNP_H.
Tested on Linux 5.6 with Sunrise Point ACPI ID for UART2. Tested on Windows for all other UARTs.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/common/block/uart/uart.c 1 file changed, 122 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40405/18
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
Patch Set 18: Code-Review+1
(4 comments)
a
https://review.coreboot.org/c/coreboot/+/40405/18//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40405/18//COMMIT_MSG@9 PS18, Line 9: Add support for LPSS UART in ACPI mode. Since this is the same as the commit summary, I'd drop it
https://review.coreboot.org/c/coreboot/+/40405/18//COMMIT_MSG@12 PS18, Line 12: the PCI devices is still operating but the PCI devices still operate
https://review.coreboot.org/c/coreboot/+/40405/18//COMMIT_MSG@15 PS18, Line 15: devices IDs device IDs
https://review.coreboot.org/c/coreboot/+/40405/18/src/soc/intel/common/block... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/40405/18/src/soc/intel/common/block... PS18, Line 230: !CONFIG(SOC_INTEL_TIGERLAKE) maybe add a new Kconfig symbol? SOC_INTEL_COMMON_BLOCK_UART_SSDTGEN
Hello Philipp Deppenwiese, wouter.eckhardt@prodrive-technologies.com, build bot (Jenkins), Justin van Son, Patrick Georgi, Martin Roth, Paul Menzel, Christian Walter, Angel Pons, Marcello Sylvester Bauer, Patrick Rudolph, Stef van Os,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40405
to look at the new patch set (#19).
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
soc/intel/common: Add support for LPSS UART in ACPI mode
Emit ACPI code for LPSS UARTs operating in ACPI mode. In this mode the device vendor ID reads as 0xffff, the PCI devices is still operate.
Add ACPI device IDs for APL, GLK, SPT, SPT_H and CNP_H.
The mainboard's devicetree needs to be adapted to include the chip driver and the PCI ID when it wouldn't been have hidden.
Example: chip soc/intel/common/block/uart device pci 19.2 hidden register "devid" = "PCI_DEVICE_ID_INTEL_CNP_H_UART2" end # UART #2 end
Tested on Linux 5.6 with Sunrise Point ACPI ID for UART2. Tested on Windows for all other UARTs.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- A src/soc/intel/common/block/uart/chip.h M src/soc/intel/common/block/uart/uart.c 2 files changed, 174 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40405/19
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
Patch Set 19:
(4 comments)
https://review.coreboot.org/c/coreboot/+/40405/18//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40405/18//COMMIT_MSG@9 PS18, Line 9: Add support for LPSS UART in ACPI mode.
Since this is the same as the commit summary, I'd drop it
Done
https://review.coreboot.org/c/coreboot/+/40405/18//COMMIT_MSG@12 PS18, Line 12: the PCI devices is still operating
but the PCI devices still operate
Done
https://review.coreboot.org/c/coreboot/+/40405/18//COMMIT_MSG@15 PS18, Line 15: devices IDs
device IDs
Done
https://review.coreboot.org/c/coreboot/+/40405/18/src/soc/intel/common/block... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/40405/18/src/soc/intel/common/block... PS18, Line 230: !CONFIG(SOC_INTEL_TIGERLAKE)
maybe add a new Kconfig symbol? SOC_INTEL_COMMON_BLOCK_UART_SSDTGEN
Dropped
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
Patch Set 19:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40405/19/src/soc/intel/common/block... File src/soc/intel/common/block/uart/chip.h:
https://review.coreboot.org/c/coreboot/+/40405/19/src/soc/intel/common/block... PS19, Line 9: u16 devid; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/40405/19/src/soc/intel/common/block... PS19, Line 9: u16 devid; please, no spaces at the start of a line
Hello Philipp Deppenwiese, wouter.eckhardt@prodrive-technologies.com, build bot (Jenkins), Justin van Son, Patrick Georgi, Martin Roth, Paul Menzel, Christian Walter, Angel Pons, Marcello Sylvester Bauer, Patrick Rudolph, Stef van Os,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40405
to look at the new patch set (#20).
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
soc/intel/common: Add support for LPSS UART in ACPI mode
Emit ACPI code for LPSS UARTs operating in ACPI mode. In this mode the device vendor ID reads as 0xffff, the PCI devices is still operate.
Add ACPI device IDs for APL, GLK, SPT, SPT_H and CNP_H.
The mainboard's devicetree needs to be adapted to include the chip driver and the PCI ID when it wouldn't been have hidden.
Example: chip soc/intel/common/block/uart device pci 19.2 hidden register "devid" = "PCI_DEVICE_ID_INTEL_CNP_H_UART2" end # UART #2 end
Tested on Linux 5.6 with Sunrise Point ACPI ID for UART2. Tested on Windows for all other UARTs.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- A src/soc/intel/common/block/uart/chip.h M src/soc/intel/common/block/uart/uart.c 2 files changed, 174 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40405/20
Hello Philipp Deppenwiese, wouter.eckhardt@prodrive-technologies.com, build bot (Jenkins), Justin van Son, Patrick Georgi, Martin Roth, Paul Menzel, Christian Walter, Angel Pons, Marcello Sylvester Bauer, Patrick Rudolph, Stef van Os,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40405
to look at the new patch set (#21).
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
soc/intel/common: Add support for LPSS UART in ACPI mode
Emit ACPI code for LPSS UARTs operating in ACPI mode. In this mode the device vendor ID reads as 0xffff, the PCI devices is still operate.
Add ACPI device IDs for APL, GLK, SPT, SPT_H and CNP_H.
The mainboard's devicetree needs to be adapted to include the chip driver and the PCI ID when it wouldn't been have hidden.
Example: chip soc/intel/common/block/uart device pci 19.2 hidden register "devid" = "PCI_DEVICE_ID_INTEL_CNP_H_UART2" end # UART #2 end
Tested on Linux 5.6 with Sunrise Point ACPI ID for UART2. Tested on Windows for all other UARTs.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- A src/soc/intel/common/block/uart/chip.h M src/soc/intel/common/block/uart/uart.c 2 files changed, 174 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40405/21
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
Patch Set 21:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40405/21/src/soc/intel/common/block... File src/soc/intel/common/block/uart/chip.h:
https://review.coreboot.org/c/coreboot/+/40405/21/src/soc/intel/common/block... PS21, Line 1: SPDX-License-Identifier: GPL-2.0-only */ : #include <stdint.h> maybe an empty line in between?
https://review.coreboot.org/c/coreboot/+/40405/21/src/soc/intel/common/block... PS21, Line 3: device/pci_ids.h not used
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
Patch Set 21:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40405/21/src/soc/intel/common/block... File src/soc/intel/common/block/uart/chip.h:
https://review.coreboot.org/c/coreboot/+/40405/21/src/soc/intel/common/block... PS21, Line 1: SPDX-License-Identifier: GPL-2.0-only */ : #include <stdint.h>
maybe an empty line in between?
Jenkins seems not to care.
https://review.coreboot.org/c/coreboot/+/40405/21/src/soc/intel/common/block... PS21, Line 3: device/pci_ids.h
not used
It is. See commit message.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
Patch Set 21: Code-Review+1
(5 comments)
https://review.coreboot.org/c/coreboot/+/40405/21//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40405/21//COMMIT_MSG@14 PS21, Line 14: driver nit: this should go to the next line
https://review.coreboot.org/c/coreboot/+/40405/21//COMMIT_MSG@15 PS21, Line 15: wouldn't been have hidden wouldn't have been hidden
https://review.coreboot.org/c/coreboot/+/40405/21/src/soc/intel/common/block... File src/soc/intel/common/block/uart/chip.h:
https://review.coreboot.org/c/coreboot/+/40405/21/src/soc/intel/common/block... PS21, Line 1: SPDX-License-Identifier: GPL-2.0-only */ : #include <stdint.h>
Jenkins seems not to care.
It's just for aesthetics
https://review.coreboot.org/c/coreboot/+/40405/21/src/soc/intel/common/block... PS21, Line 3: device/pci_ids.h
It is. See commit message.
Maybe add a comment to let people know this is so that mainboards can use these definitions in the devicetree?
https://review.coreboot.org/c/coreboot/+/40405/21/src/soc/intel/common/block... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/40405/21/src/soc/intel/common/block... PS21, Line 284: strncmp strcmp, maybe?
IMHO another switch-case like that of `uart_acpi_hid` would be cleaner
Hello Philipp Deppenwiese, wouter.eckhardt@prodrive-technologies.com, build bot (Jenkins), Justin van Son, Patrick Georgi, Martin Roth, Paul Menzel, Christian Walter, Angel Pons, Marcello Sylvester Bauer, Patrick Rudolph, Stef van Os,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40405
to look at the new patch set (#22).
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
soc/intel/common: Add support for LPSS UART in ACPI mode
Emit ACPI code for LPSS UARTs operating in ACPI mode. In this mode the device vendor ID reads as 0xffff, the PCI devices is still operate.
Add ACPI device IDs for APL, GLK, SPT, SPT_H and CNP_H.
The mainboard's devicetree needs to be adapted to include the chip driver and the PCI ID when it wouldn't been have hidden.
Example: chip soc/intel/common/block/uart device pci 19.2 hidden register "devid" = "PCI_DEVICE_ID_INTEL_CNP_H_UART2" end # UART #2 end
Tested on Linux 5.6 with Sunrise Point ACPI ID for UART2. Tested on Windows for all other UARTs.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- A src/soc/intel/common/block/uart/chip.h M src/soc/intel/common/block/uart/uart.c 2 files changed, 176 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40405/22
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
Patch Set 21:
(9 comments)
https://review.coreboot.org/c/coreboot/+/40405/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40405/8//COMMIT_MSG@9 PS8, Line 9: all
There's no difference between pch-h and pch-lp for uart0 and uart1. […]
Done
https://review.coreboot.org/c/coreboot/+/40405/8//COMMIT_MSG@11 PS8, Line 11: PCH
Not accurate any more.
Done
https://review.coreboot.org/c/coreboot/+/40405/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40405/9//COMMIT_MSG@9 PS9, Line 9: On PCH-H all UARTs can't be used under Windows.
You probably mean "not every UART can be used"?
Done
https://review.coreboot.org/c/coreboot/+/40405/9//COMMIT_MSG@12 PS9, Line 12: ACPI code for the Intel LPSS ACPI driver.
This is a separate topic and should be in its own change. For the above, […]
Done
https://review.coreboot.org/c/coreboot/+/40405/8/src/soc/intel/cannonlake/ac... File src/soc/intel/cannonlake/acpi/serialio.asl:
https://review.coreboot.org/c/coreboot/+/40405/8/src/soc/intel/cannonlake/ac... PS8, Line 61: UAR0
Those entries conflict with the ssdt. You advertise the same device twice.
Done
https://review.coreboot.org/c/coreboot/+/40405/21/src/soc/intel/common/block... File src/soc/intel/common/block/uart/chip.h:
https://review.coreboot.org/c/coreboot/+/40405/21/src/soc/intel/common/block... PS21, Line 1: SPDX-License-Identifier: GPL-2.0-only */ : #include <stdint.h>
It's just for aesthetics
Done
https://review.coreboot.org/c/coreboot/+/40405/21/src/soc/intel/common/block... PS21, Line 3: device/pci_ids.h
Maybe add a comment to let people know this is so that mainboards can use these definitions in the d […]
Done
https://review.coreboot.org/c/coreboot/+/40405/15/src/soc/intel/common/block... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/40405/15/src/soc/intel/common/block... PS15, Line 230:
Double newline
Done
https://review.coreboot.org/c/coreboot/+/40405/21/src/soc/intel/common/block... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/40405/21/src/soc/intel/common/block... PS21, Line 284: strncmp
strcmp, maybe? […]
Done
Hello Philipp Deppenwiese, wouter.eckhardt@prodrive-technologies.com, build bot (Jenkins), Justin van Son, Patrick Georgi, Martin Roth, Paul Menzel, Christian Walter, Angel Pons, Marcello Sylvester Bauer, Patrick Rudolph, Stef van Os,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40405
to look at the new patch set (#23).
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
soc/intel/common: Add support for LPSS UART in ACPI mode
Emit ACPI code for LPSS UARTs operating in ACPI mode. In this mode the device vendor ID reads as 0xffff, the PCI devices is still operate.
Add ACPI device IDs for APL, GLK, SPT, SPT_H and CNP_H.
The mainboard's devicetree needs to be adapted to include the chip driver and the PCI ID when it wouldn't have been hidden.
Example: chip soc/intel/common/block/uart device pci 19.2 hidden register "devid" = "PCI_DEVICE_ID_INTEL_CNP_H_UART2" end # UART #2 end
Tested on Linux 5.6 with Sunrise Point ACPI ID for UART2. Tested on Windows for all other UARTs.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- A src/soc/intel/common/block/uart/chip.h M src/soc/intel/common/block/uart/uart.c 2 files changed, 174 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40405/23
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
Patch Set 22:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40405/21//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40405/21//COMMIT_MSG@14 PS21, Line 14: driver
nit: this should go to the next line
Done
https://review.coreboot.org/c/coreboot/+/40405/21//COMMIT_MSG@15 PS21, Line 15: wouldn't been have hidden
wouldn't have been hidden
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
Patch Set 23:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40405/23/src/soc/intel/common/block... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/40405/23/src/soc/intel/common/block... PS23, Line 284: if (strncmp(hid, "INT34B8", strlen("INT34B8")) == 0) This was undone
Hello Philipp Deppenwiese, wouter.eckhardt@prodrive-technologies.com, build bot (Jenkins), Justin van Son, Patrick Georgi, Martin Roth, Paul Menzel, Christian Walter, Angel Pons, Marcello Sylvester Bauer, Patrick Rudolph, Stef van Os,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40405
to look at the new patch set (#24).
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
soc/intel/common: Add support for LPSS UART in ACPI mode
Emit ACPI code for LPSS UARTs operating in ACPI mode. In this mode the device vendor ID reads as 0xffff, the PCI devices is still operate.
Add ACPI device IDs for APL, GLK, SPT, SPT_H and CNP_H.
The mainboard's devicetree needs to be adapted to include the chip driver and the PCI ID when it wouldn't have been hidden.
Example: chip soc/intel/common/block/uart device pci 19.2 hidden register "devid" = "PCI_DEVICE_ID_INTEL_CNP_H_UART2" end # UART #2 end
Tested on Linux 5.6 with Sunrise Point ACPI ID for UART2. Tested on Windows for all other UARTs.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- A src/soc/intel/common/block/uart/chip.h M src/soc/intel/common/block/uart/uart.c 2 files changed, 174 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40405/24
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
Patch Set 23:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40405/23/src/soc/intel/common/block... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/40405/23/src/soc/intel/common/block... PS23, Line 284: if (strncmp(hid, "INT34B8", strlen("INT34B8")) == 0)
This was undone
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
Patch Set 24:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40405/22/src/soc/intel/common/block... File src/soc/intel/common/block/uart/chip.h:
https://review.coreboot.org/c/coreboot/+/40405/22/src/soc/intel/common/block... PS22, Line 2: This blank line and comment went missing too
Hello Philipp Deppenwiese, wouter.eckhardt@prodrive-technologies.com, build bot (Jenkins), Justin van Son, Patrick Georgi, Martin Roth, Paul Menzel, Christian Walter, Angel Pons, Marcello Sylvester Bauer, Patrick Rudolph, Stef van Os,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40405
to look at the new patch set (#25).
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
soc/intel/common: Add support for LPSS UART in ACPI mode
Emit ACPI code for LPSS UARTs operating in ACPI mode. In this mode the device vendor ID reads as 0xffff, the PCI devices is still operate.
Add ACPI device IDs for APL, GLK, SPT, SPT_H and CNP_H.
The mainboard's devicetree needs to be adapted to include the chip driver and the PCI ID when it wouldn't have been hidden.
Example: chip soc/intel/common/block/uart device pci 19.2 hidden register "devid" = "PCI_DEVICE_ID_INTEL_CNP_H_UART2" end # UART #2 end
Tested on Linux 5.6 with Sunrise Point ACPI ID for UART2. Tested on Windows for all other UARTs.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- A src/soc/intel/common/block/uart/chip.h M src/soc/intel/common/block/uart/uart.c 2 files changed, 176 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40405/25
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
Patch Set 24:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40405/22/src/soc/intel/common/block... File src/soc/intel/common/block/uart/chip.h:
https://review.coreboot.org/c/coreboot/+/40405/22/src/soc/intel/common/block... PS22, Line 2:
This blank line and comment went missing too
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
Patch Set 25: Code-Review+2
Patrick Rudolph has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
soc/intel/common: Add support for LPSS UART in ACPI mode
Emit ACPI code for LPSS UARTs operating in ACPI mode. In this mode the device vendor ID reads as 0xffff, the PCI devices is still operate.
Add ACPI device IDs for APL, GLK, SPT, SPT_H and CNP_H.
The mainboard's devicetree needs to be adapted to include the chip driver and the PCI ID when it wouldn't have been hidden.
Example: chip soc/intel/common/block/uart device pci 19.2 hidden register "devid" = "PCI_DEVICE_ID_INTEL_CNP_H_UART2" end # UART #2 end
Tested on Linux 5.6 with Sunrise Point ACPI ID for UART2. Tested on Windows for all other UARTs.
Change-Id: I838d16322be38f5421c1f63b457a0af552e0ed96 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/40405 Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- A src/soc/intel/common/block/uart/chip.h M src/soc/intel/common/block/uart/uart.c 2 files changed, 176 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/soc/intel/common/block/uart/chip.h b/src/soc/intel/common/block/uart/chip.h new file mode 100644 index 0000000..5981126 --- /dev/null +++ b/src/soc/intel/common/block/uart/chip.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <stdint.h> +/* Indirect include for static.c: */ +#include <device/pci_ids.h> + +#ifndef _SOC_INTEL_COMMON_BLOCK_UART_CHIP_H_ +#define _SOC_INTEL_COMMON_BLOCK_UART_CHIP_H_ + +struct soc_intel_common_block_uart_config { + /* The Device ID read from config space at offset[2:4] when not hidden */ + u16 devid; +}; + +#endif /* _SOC_INTEL_COMMON_BLOCK_UART_CHIP_H_ */ diff --git a/src/soc/intel/common/block/uart/uart.c b/src/soc/intel/common/block/uart/uart.c index ed4f9c6..5507663 100644 --- a/src/soc/intel/common/block/uart/uart.c +++ b/src/soc/intel/common/block/uart/uart.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */
#include <acpi/acpi.h> +#include <acpi/acpigen.h> #include <acpi/acpi_gnvs.h> #include <console/uart.h> #include <device/device.h> @@ -12,7 +13,9 @@ #include <intelblocks/uart.h> #include <soc/pci_devs.h> #include <soc/iomap.h> +#include <soc/irq.h> #include <soc/nvs.h> +#include "chip.h"
#define UART_PCI_ENABLE (PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER) #define UART_CONSOLE_INVALID_INDEX 0xFF @@ -225,11 +228,156 @@ } }
+static void uart_acpi_write_irq(const struct device *dev) +{ + struct acpi_irq irq; + + switch (dev->path.pci.devfn) { + case PCH_DEVFN_UART0: + irq = (struct acpi_irq)ACPI_IRQ_LEVEL_LOW(LPSS_UART0_IRQ); + break; + case PCH_DEVFN_UART1: + irq = (struct acpi_irq)ACPI_IRQ_LEVEL_LOW(LPSS_UART1_IRQ); + break; + case PCH_DEVFN_UART2: + irq = (struct acpi_irq)ACPI_IRQ_LEVEL_LOW(LPSS_UART2_IRQ); + break; + default: + return; + } + + acpi_device_write_interrupt(&irq); +} + +/* + * Generate an ACPI entry if the device is enabled in devicetree for the ACPI + * LPSS driver. In this mode the device and vendor ID reads as 0xffff, but the + * PCI device is still there. + */ +static void uart_fill_ssdt(const struct device *dev) +{ + const char *scope = acpi_device_scope(dev); + const char *hid = acpi_device_hid(dev); + struct resource *res; + + /* In ACPI mode the device is "invisible" */ + if (!dev->hidden) + return; + + if (!scope || !hid) + return; + + res = probe_resource(dev, PCI_BASE_ADDRESS_0); + if (!res) + return; + + /* Scope */ + acpigen_write_scope(scope); + + /* Device */ + acpigen_write_device(acpi_device_name(dev)); + acpigen_write_name_string("_HID", hid); + /* + * Advertise compatibility to Sunrise Point, as the Linux kernel doesn't support + * CannonPoint yet... + */ + if (strcmp(hid, "INT34B8") == 0) + acpigen_write_name_string("_CID", "INT3448"); + else if (strcmp(hid, "INT34B9") == 0) + acpigen_write_name_string("_CID", "INT3449"); + else if (strcmp(hid, "INT34BA") == 0) + acpigen_write_name_string("_CID", "INT344A"); + + acpi_device_write_uid(dev); + acpigen_write_name_string("_DDN", "LPSS ACPI UART"); + acpigen_write_STA(acpi_device_status(dev)); + + /* Resources */ + acpigen_write_name("_CRS"); + acpigen_write_resourcetemplate_header(); + + uart_acpi_write_irq(dev); + acpigen_write_mem32fixed(1, res->base, res->size); + + acpigen_write_resourcetemplate_footer(); + + acpigen_pop_len(); /* Device */ + acpigen_pop_len(); /* Scope */ +} + +static const char *uart_acpi_hid(const struct device *dev) +{ + switch (dev->device) { + case PCI_DEVICE_ID_INTEL_APL_UART0: + return "80865abc"; + case PCI_DEVICE_ID_INTEL_APL_UART1: + return "80865abe"; + case PCI_DEVICE_ID_INTEL_APL_UART2: + return "80865ac0"; + case PCI_DEVICE_ID_INTEL_GLK_UART0: + return "808631bc"; + case PCI_DEVICE_ID_INTEL_GLK_UART1: + return "808631be"; + case PCI_DEVICE_ID_INTEL_GLK_UART2: + return "808631c0"; + case PCI_DEVICE_ID_INTEL_GLK_UART3: + return "808631ee"; + case PCI_DEVICE_ID_INTEL_SPT_UART0: + case PCI_DEVICE_ID_INTEL_SPT_H_UART0: + return "INT3448"; + case PCI_DEVICE_ID_INTEL_SPT_UART1: + case PCI_DEVICE_ID_INTEL_SPT_H_UART1: + return "INT3449"; + case PCI_DEVICE_ID_INTEL_SPT_UART2: + case PCI_DEVICE_ID_INTEL_SPT_H_UART2: + return "INT344A"; + case PCI_DEVICE_ID_INTEL_CNP_H_UART0: + return "INT34B8"; + case PCI_DEVICE_ID_INTEL_CNP_H_UART1: + return "INT34B9"; + case PCI_DEVICE_ID_INTEL_CNP_H_UART2: + return "INT34BA"; + default: + return NULL; + } +} + +static const char *uart_acpi_name(const struct device *dev) +{ + switch (dev->device) { + case PCI_DEVICE_ID_INTEL_APL_UART0: + case PCI_DEVICE_ID_INTEL_GLK_UART0: + case PCI_DEVICE_ID_INTEL_SPT_UART0: + case PCI_DEVICE_ID_INTEL_SPT_H_UART0: + case PCI_DEVICE_ID_INTEL_CNP_H_UART0: + return "UAR0"; + case PCI_DEVICE_ID_INTEL_APL_UART1: + case PCI_DEVICE_ID_INTEL_GLK_UART1: + case PCI_DEVICE_ID_INTEL_SPT_UART1: + case PCI_DEVICE_ID_INTEL_SPT_H_UART1: + case PCI_DEVICE_ID_INTEL_CNP_H_UART1: + return "UAR1"; + case PCI_DEVICE_ID_INTEL_APL_UART2: + case PCI_DEVICE_ID_INTEL_GLK_UART2: + case PCI_DEVICE_ID_INTEL_SPT_UART2: + case PCI_DEVICE_ID_INTEL_SPT_H_UART2: + case PCI_DEVICE_ID_INTEL_CNP_H_UART2: + return "UAR2"; + case PCI_DEVICE_ID_INTEL_GLK_UART3: + return "UAR3"; + default: + return NULL; + } +} + static struct device_operations device_ops = { .read_resources = uart_read_resources, .set_resources = pci_dev_set_resources, .enable_resources = uart_common_enable_resources, .ops_pci = &pci_dev_ops_pci, + .acpi_fill_ssdt = uart_fill_ssdt, + .acpi_hid = uart_acpi_hid, + .acpi_name = uart_acpi_name, };
static const unsigned short pci_device_ids[] = { @@ -296,4 +444,17 @@ .vendor = PCI_VENDOR_ID_INTEL, .devices = pci_device_ids, }; + +static void uart_enable(struct device *dev) +{ + struct soc_intel_common_block_uart_config *conf = dev->chip_info; + dev->ops = &device_ops; + dev->device = conf ? conf->devid : 0; +} + +struct chip_operations soc_intel_common_block_uart_ops = { + CHIP_NAME("LPSS UART in ACPI mode") + .enable_dev = uart_enable +}; + #endif /* ENV_RAMSTAGE */
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
Christian Walter (Code Review)
-
HAOUAS Elyes (Code Review)
-
Justin van Son (Code Review)
-
Nico Huber (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)
-
Philipp Deppenwiese (Code Review)
-
Stef van Os (Code Review)