* [U-Boot] [RFC PATCH v1 0/2] edison: Enable ACPI
@ 2017-08-30 15:04 Andy Shevchenko
2017-08-30 15:04 ` [U-Boot] [PATCH v1 1/2] x86: tangier: Enable ACPI support for Intel Tangier Andy Shevchenko
2017-08-30 15:04 ` [U-Boot] [PATCH v1 2/2] x86: edison: Bring ACPI minimal support to the board Andy Shevchenko
0 siblings, 2 replies; 6+ messages in thread
From: Andy Shevchenko @ 2017-08-30 15:04 UTC (permalink / raw)
To: u-boot
This RFC series based on discussion [1] to show how we may unleash the
powerfulness of ACPI on the board, which initially didn't support it,
via U-Boot.
The series is highly experimental, though I'm using it on daily basis
since my main work on ACPI pin control glue layer.
More information is available on [2].
Note, I dunno the split of DSL pieces is fully correct since we need to
understand the difference between SoC, platform, and board terms. Even
though some stuff here declared under 'tangier' folder it's related to
Intel Merrifield platform (like Wi-Fi chip connected to SDIO port of
Intel Tangier). Because of some restrictions of ACPI language /
interpreter I may not split some of those parts. Thus, it needs to be
revisited.
P.S. It would be nice if someone who possess the board can check and
test this independently.
[1]: https://lists.denx.de/pipermail/u-boot/2017-August/303997.html
[2]: https://edison.internet-share.com/wiki/ACPI
Andy Shevchenko (2):
x86: tangier: Enable ACPI support for Intel Tangier
x86: edison: Bring ACPI minimal support to the board
arch/x86/cpu/tangier/Makefile | 1 +
arch/x86/cpu/tangier/acpi.c | 86 ++++++
.../include/asm/arch-tangier/acpi/global_nvs.asl | 16 ++
.../x86/include/asm/arch-tangier/acpi/platform.asl | 31 +++
.../include/asm/arch-tangier/acpi/southcluster.asl | 306 +++++++++++++++++++++
arch/x86/include/asm/arch-tangier/global_nvs.h | 22 ++
board/intel/edison/.gitignore | 3 +
board/intel/edison/Kconfig | 6 +
board/intel/edison/Makefile | 1 +
board/intel/edison/dsdt.asl | 13 +
include/configs/edison.h | 3 +
11 files changed, 488 insertions(+)
create mode 100644 arch/x86/cpu/tangier/acpi.c
create mode 100644 arch/x86/include/asm/arch-tangier/acpi/global_nvs.asl
create mode 100644 arch/x86/include/asm/arch-tangier/acpi/platform.asl
create mode 100644 arch/x86/include/asm/arch-tangier/acpi/southcluster.asl
create mode 100644 arch/x86/include/asm/arch-tangier/global_nvs.h
create mode 100644 board/intel/edison/.gitignore
create mode 100644 board/intel/edison/dsdt.asl
--
2.14.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v1 1/2] x86: tangier: Enable ACPI support for Intel Tangier
2017-08-30 15:04 [U-Boot] [RFC PATCH v1 0/2] edison: Enable ACPI Andy Shevchenko
@ 2017-08-30 15:04 ` Andy Shevchenko
2017-09-03 9:46 ` Bin Meng
2017-08-30 15:04 ` [U-Boot] [PATCH v1 2/2] x86: edison: Bring ACPI minimal support to the board Andy Shevchenko
1 sibling, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2017-08-30 15:04 UTC (permalink / raw)
To: u-boot
Intel Tangier SoC is a part of Intel Merrifield platform which doesn't
utilize ACPI by default. Here is an attempt to unleash ACPI flexibility
power on Intel Merrifield based platforms.
The change brings minimum support of the devices that found on
Intel Merrifield based end user device.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
arch/x86/cpu/tangier/Makefile | 1 +
arch/x86/cpu/tangier/acpi.c | 86 ++++++
.../include/asm/arch-tangier/acpi/global_nvs.asl | 16 ++
.../x86/include/asm/arch-tangier/acpi/platform.asl | 31 +++
.../include/asm/arch-tangier/acpi/southcluster.asl | 306 +++++++++++++++++++++
arch/x86/include/asm/arch-tangier/global_nvs.h | 22 ++
6 files changed, 462 insertions(+)
create mode 100644 arch/x86/cpu/tangier/acpi.c
create mode 100644 arch/x86/include/asm/arch-tangier/acpi/global_nvs.asl
create mode 100644 arch/x86/include/asm/arch-tangier/acpi/platform.asl
create mode 100644 arch/x86/include/asm/arch-tangier/acpi/southcluster.asl
create mode 100644 arch/x86/include/asm/arch-tangier/global_nvs.h
diff --git a/arch/x86/cpu/tangier/Makefile b/arch/x86/cpu/tangier/Makefile
index d146b3f5c2..92cfa555ed 100644
--- a/arch/x86/cpu/tangier/Makefile
+++ b/arch/x86/cpu/tangier/Makefile
@@ -5,3 +5,4 @@
#
obj-y += car.o tangier.o sdram.o
+obj-$(CONFIG_GENERATE_ACPI_TABLE) += acpi.o
diff --git a/arch/x86/cpu/tangier/acpi.c b/arch/x86/cpu/tangier/acpi.c
new file mode 100644
index 0000000000..fb15ce40ad
--- /dev/null
+++ b/arch/x86/cpu/tangier/acpi.c
@@ -0,0 +1,86 @@
+/*
+ * Copyright (c) 2017 Intel Corporation
+ *
+ * Partially based on acpi.c for other x86 platforms
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <common.h>
+#include <cpu.h>
+#include <dm.h>
+#include <dm/uclass-internal.h>
+#include <asm/acpi_table.h>
+#include <asm/ioapic.h>
+#include <asm/mpspec.h>
+#include <asm/tables.h>
+#include <asm/arch/global_nvs.h>
+
+void acpi_create_fadt(struct acpi_fadt *fadt, struct acpi_facs *facs,
+ void *dsdt)
+{
+ struct acpi_table_header *header = &(fadt->header);
+
+ memset((void *)fadt, 0, sizeof(struct acpi_fadt));
+
+ acpi_fill_header(header, "FACP");
+ header->length = sizeof(struct acpi_fadt);
+ header->revision = 6;
+
+ fadt->firmware_ctrl = (u32)facs;
+ fadt->dsdt = (u32)dsdt;
+ fadt->preferred_pm_profile = ACPI_PM_UNSPECIFIED;
+
+ fadt->iapc_boot_arch = ACPI_FADT_VGA_NOT_PRESENT |
+ ACPI_FADT_NO_PCIE_ASPM_CONTROL;
+ fadt->flags =
+ ACPI_FADT_WBINVD |
+ ACPI_FADT_POWER_BUTTON | ACPI_FADT_SLEEP_BUTTON |
+ ACPI_FADT_SEALED_CASE | ACPI_FADT_HEADLESS |
+ ACPI_FADT_HW_REDUCED_ACPI;
+
+ fadt->minor_revision = 2;
+
+ fadt->x_firmware_ctl_l = (u32)facs;
+ fadt->x_firmware_ctl_h = 0;
+ fadt->x_dsdt_l = (u32)dsdt;
+ fadt->x_dsdt_h = 0;
+
+ header->checksum = table_compute_checksum(fadt, header->length);
+}
+
+u32 acpi_fill_madt(u32 current)
+{
+ current += acpi_create_madt_lapics(current);
+
+ current += acpi_create_madt_ioapic((struct acpi_madt_ioapic *)current,
+ io_apic_read(IO_APIC_ID) >> 24, IO_APIC_ADDR, 0);
+
+ return current;
+}
+
+u32 acpi_fill_mcfg(u32 current)
+{
+ current += acpi_create_mcfg_mmconfig
+ ((struct acpi_mcfg_mmconfig *)current,
+ 0x3f500000, 0x0, 0x0, 0x0);
+
+ return current;
+}
+
+void acpi_create_gnvs(struct acpi_global_nvs *gnvs)
+{
+ struct udevice *dev;
+ int ret;
+
+ /* at least we have one processor */
+ gnvs->pcnt = 1;
+
+ /* override the processor count with actual number */
+ ret = uclass_find_first_device(UCLASS_CPU, &dev);
+ if (ret == 0 && dev != NULL) {
+ ret = cpu_get_count(dev);
+ if (ret > 0)
+ gnvs->pcnt = ret;
+ }
+}
diff --git a/arch/x86/include/asm/arch-tangier/acpi/global_nvs.asl b/arch/x86/include/asm/arch-tangier/acpi/global_nvs.asl
new file mode 100644
index 0000000000..84fffbe140
--- /dev/null
+++ b/arch/x86/include/asm/arch-tangier/acpi/global_nvs.asl
@@ -0,0 +1,16 @@
+/*
+ * Copyright (c) 2017 Intel Corporation
+ *
+ * Partially based on global_nvs.asl for other x86 platforms
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <asm/acpi/global_nvs.h>
+
+OperationRegion(GNVS, SystemMemory, ACPI_GNVS_ADDR, ACPI_GNVS_SIZE)
+Field(GNVS, ByteAcc, NoLock, Preserve)
+{
+ Offset (0x00),
+ PCNT, 2, /* processor count */
+}
diff --git a/arch/x86/include/asm/arch-tangier/acpi/platform.asl b/arch/x86/include/asm/arch-tangier/acpi/platform.asl
new file mode 100644
index 0000000000..a57b7cb319
--- /dev/null
+++ b/arch/x86/include/asm/arch-tangier/acpi/platform.asl
@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2017 Intel Corporation
+ *
+ * Partially based on platform.asl for other x86 platforms
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <asm/acpi/statdef.asl>
+
+/*
+ * The _PTS method (Prepare To Sleep) is called before the OS is
+ * entering a sleep state. The sleep state number is passed in Arg0.
+ */
+Method(_PTS, 1)
+{
+}
+
+/* The _WAK method is called on system wakeup */
+Method(_WAK, 1)
+{
+ Return (Package() {0, 0})
+}
+
+/* ACPI global NVS */
+#include "global_nvs.asl"
+
+Scope (\_SB)
+{
+ #include "southcluster.asl"
+}
diff --git a/arch/x86/include/asm/arch-tangier/acpi/southcluster.asl b/arch/x86/include/asm/arch-tangier/acpi/southcluster.asl
new file mode 100644
index 0000000000..d3a9b114cb
--- /dev/null
+++ b/arch/x86/include/asm/arch-tangier/acpi/southcluster.asl
@@ -0,0 +1,306 @@
+/*
+ * Copyright (c) 2017 Intel Corporation
+ *
+ * Partially based on southcluster.asl for other x86 platforms
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+Device (PCI0)
+{
+ Name (_HID, EISAID("PNP0A08")) /* PCIe */
+ Name (_CID, EISAID("PNP0A03")) /* PCI */
+
+ Name (_ADR, 0)
+ Name (_BBN, 0)
+
+ Name (MCRS, ResourceTemplate()
+ {
+ /* Bus Numbers */
+ WordBusNumber(ResourceProducer, MinFixed, MaxFixed, PosDecode,
+ 0x0000, 0x0000, 0x00ff, 0x0000, 0x0100, , , PB00)
+
+ /* IO Region 0 */
+ WordIO(ResourceProducer, MinFixed, MaxFixed, PosDecode, EntireRange,
+ 0x0000, 0x0000, 0x0cf7, 0x0000, 0x0cf8, , , PI00)
+
+ /* PCI Config Space */
+ IO(Decode16, 0x0cf8, 0x0cf8, 0x0001, 0x0008)
+
+ /* IO Region 1 */
+ WordIO(ResourceProducer, MinFixed, MaxFixed, PosDecode, EntireRange,
+ 0x0000, 0x0d00, 0xffff, 0x0000, 0xf300, , , PI01)
+
+ /* GPIO Low Memory Region */
+ DWordMemory(ResourceProducer, PosDecode, MinFixed, MaxFixed,
+ Cacheable, ReadWrite,
+ 0x00000000, 0x000ddcc0, 0x000ddccf, 0x00000000,
+ 0x00000010, , , GP00)
+
+ /* PSH Memory Region 0 */
+ DWordMemory(ResourceProducer, PosDecode, MinFixed, MaxFixed,
+ Cacheable, ReadWrite,
+ 0x00000000, 0x04819000, 0x04898fff, 0x00000000,
+ 0x00080000, , , PSH0)
+
+ /* PSH Memory Region 1 */
+ DWordMemory(ResourceProducer, PosDecode, MinFixed, MaxFixed,
+ Cacheable, ReadWrite,
+ 0x00000000, 0x04919000, 0x04920fff, 0x00000000,
+ 0x00008000, , , PSH1)
+
+ /* SST Memory Region */
+ DWordMemory(ResourceProducer, PosDecode, MinFixed, MaxFixed,
+ Cacheable, ReadWrite,
+ 0x00000000, 0x05e00000, 0x05ffffff, 0x00000000,
+ 0x00200000, , , SST0)
+
+ /* PCI Memory Region */
+ DWordMemory(ResourceProducer, PosDecode, MinFixed, MaxFixed,
+ Cacheable, ReadWrite,
+ 0x00000000, 0x80000000, 0xffffffff, 0x00000000,
+ 0x80000000, , , PMEM)
+ })
+
+ Method (_CRS, 0, Serialized)
+ {
+ Return (MCRS)
+ }
+
+ Method (_OSC, 4)
+ {
+ /* Check for proper GUID */
+ If (LEqual(Arg0, ToUUID("33db4d5b-1ff7-401c-9657-7441c03dd766"))) {
+ /* Let OS control everything */
+ Return (Arg3)
+ } Else {
+ /* Unrecognized UUID */
+ CreateDWordField(Arg3, 0, CDW1)
+ Or(CDW1, 4, CDW1)
+ Return (Arg3)
+ }
+ }
+
+ Device (SDHC)
+ {
+ Name (_ADR, 0x00010003)
+ Name (_DEP, Package (0x01)
+ {
+ GPIO
+ })
+ Name (PSTS, Zero)
+
+ Method (_STA)
+ {
+ Return (STA_VISIBLE)
+ }
+
+ Method (_PS3, 0, NotSerialized)
+ {
+ }
+
+ Method (_PS0, 0, NotSerialized)
+ {
+ If (PSTS == Zero)
+ {
+ If (^^GPIO.AVBL == One)
+ {
+ ^^GPIO.WFD3 = One
+ PSTS = One
+ }
+ }
+ }
+
+ /* BCM43340 */
+ Device (BRC1)
+ {
+ Name (_ADR, One)
+ Name (_DEP, Package (0x01)
+ {
+ GPIO
+ })
+
+ Method (_STA)
+ {
+ Return (STA_VISIBLE)
+ }
+
+ Method (_RMV, 0, NotSerialized)
+ {
+ Return (Zero)
+ }
+
+ Method (_PS3, 0, NotSerialized)
+ {
+ If (^^^GPIO.AVBL == One)
+ {
+ ^^^GPIO.WFD3 = Zero
+ PSTS = Zero
+ }
+ }
+
+ Method (_PS0, 0, NotSerialized)
+ {
+ If (PSTS == Zero)
+ {
+ If (^^^GPIO.AVBL == One)
+ {
+ ^^^GPIO.WFD3 = One
+ PSTS = One
+ }
+ }
+ }
+ }
+
+ Device (BRC2)
+ {
+ Name (_ADR, 0x02)
+ Method (_STA, 0, NotSerialized)
+ {
+ Return (STA_VISIBLE)
+ }
+
+ Method (_RMV, 0, NotSerialized)
+ {
+ Return (Zero)
+ }
+ }
+ }
+
+ Device (SPI5)
+ {
+ Name (_ADR, 0x00070001)
+ Name (_DEP, Package (0x01)
+ {
+ GPIO
+ })
+ Name (RBUF, ResourceTemplate()
+ {
+ GpioIo(Exclusive, PullUp, 0, 0, IoRestrictionOutputOnly,
+ "\\_SB.PCI0.GPIO", 0, ResourceConsumer, , ) { 91 }
+ GpioIo(Exclusive, PullUp, 0, 0, IoRestrictionOutputOnly,
+ "\\_SB.PCI0.GPIO", 0, ResourceConsumer, , ) { 92 }
+ GpioIo(Exclusive, PullUp, 0, 0, IoRestrictionOutputOnly,
+ "\\_SB.PCI0.GPIO", 0, ResourceConsumer, , ) { 93 }
+ GpioIo(Exclusive, PullUp, 0, 0, IoRestrictionOutputOnly,
+ "\\_SB.PCI0.GPIO", 0, ResourceConsumer, , ) { 94 }
+ })
+
+ Method (_CRS, 0, NotSerialized)
+ {
+ Return (RBUF)
+ }
+
+ /*
+ * See
+ * http://www.kernel.org/doc/Documentation/acpi/gpio-properties.txt
+ * for more information about GPIO bindings.
+ */
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package () {
+ "cs-gpios", Package () {
+ ^SPI5, 0, 0, 0,
+ ^SPI5, 1, 0, 0,
+ ^SPI5, 2, 0, 0,
+ ^SPI5, 3, 0, 0,
+ },
+ },
+ }
+ })
+
+ Method (_STA, 0, NotSerialized)
+ {
+ Return (STA_VISIBLE)
+ }
+ }
+
+ Device (I2C1)
+ {
+ Name (_ADR, 0x00080000)
+
+ Method (_STA, 0, NotSerialized)
+ {
+ Return (STA_VISIBLE)
+ }
+ }
+
+ Device (GPIO)
+ {
+ Name (_ADR, 0x000c0000)
+ Name (_DEP, Package (0x01)
+ {
+ \_SB.FLIS
+ })
+
+ Method (_STA)
+ {
+ Return (STA_VISIBLE)
+ }
+
+ Name (AVBL, Zero)
+ Method (_REG, 2, NotSerialized)
+ {
+ If (Arg0 == 0x08)
+ {
+ AVBL = Arg1
+ }
+ }
+
+ OperationRegion (GPOP, GeneralPurposeIo, Zero, 0x04)
+ Field (GPOP, ByteAcc, NoLock, Preserve)
+ {
+ Connection (
+ GpioIo(Exclusive, PullDefault, 0, 0, IoRestrictionOutputOnly,
+ "\\_SB.PCI0.GPIO", 0, ResourceConsumer, , ) { 56 }
+ ),
+ WFD3, 1,
+ }
+ }
+
+ Device (PWM0)
+ {
+ Name (_ADR, 0x00170000)
+
+ Method (_STA, 0, NotSerialized)
+ {
+ Return (STA_VISIBLE)
+ }
+ }
+}
+
+Device (FLIS)
+{
+ Name (_HID, "PRP0001")
+ Name (_DDN, "Intel Merrifield Family-Level Interface Shim")
+ Name (RBUF, ResourceTemplate()
+ {
+ Memory32Fixed(ReadWrite, 0xFF0C0000, 0x00008000, )
+ PinGroup("spi5", ResourceProducer, ) { 90, 91, 92, 93, 94, 95, 96 }
+ PinGroup("uart0", ResourceProducer, ) { 115, 116, 117, 118 }
+ PinGroup("uart1", ResourceProducer, ) { 119, 120, 121, 122 }
+ PinGroup("uart2", ResourceProducer, ) { 123, 124, 125, 126 }
+ PinGroup("pwm0", ResourceProducer, ) { 144 }
+ PinGroup("pwm1", ResourceProducer, ) { 145 }
+ PinGroup("pwm2", ResourceProducer, ) { 132 }
+ PinGroup("pwm3", ResourceProducer, ) { 133 }
+ })
+
+ Method (_CRS, 0, NotSerialized)
+ {
+ Return (RBUF)
+ }
+
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package () {"compatible", Package () {"intel,merrifield-pinctrl"}},
+ }
+ })
+
+ Method (_STA, 0, NotSerialized)
+ {
+ Return (STA_VISIBLE)
+ }
+}
diff --git a/arch/x86/include/asm/arch-tangier/global_nvs.h b/arch/x86/include/asm/arch-tangier/global_nvs.h
new file mode 100644
index 0000000000..8ab5cf2aa2
--- /dev/null
+++ b/arch/x86/include/asm/arch-tangier/global_nvs.h
@@ -0,0 +1,22 @@
+/*
+ * Copyright (c) 2017 Intel Corporation
+ *
+ * Partially based on global_nvs.h for other x86 platforms
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#ifndef _GLOBAL_NVS_H_
+#define _GLOBAL_NVS_H_
+
+struct __packed acpi_global_nvs {
+ u8 pcnt; /* processor count */
+
+ /*
+ * Add padding so sizeof(struct acpi_global_nvs) == 0x100.
+ * This must match the size defined in the global_nvs.asl.
+ */
+ u8 rsvd[255];
+};
+
+#endif /* _GLOBAL_NVS_H_ */
--
2.14.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v1 2/2] x86: edison: Bring ACPI minimal support to the board
2017-08-30 15:04 [U-Boot] [RFC PATCH v1 0/2] edison: Enable ACPI Andy Shevchenko
2017-08-30 15:04 ` [U-Boot] [PATCH v1 1/2] x86: tangier: Enable ACPI support for Intel Tangier Andy Shevchenko
@ 2017-08-30 15:04 ` Andy Shevchenko
2017-09-03 9:46 ` Bin Meng
1 sibling, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2017-08-30 15:04 UTC (permalink / raw)
To: u-boot
This board is based on Intel Tangier SoC (Intel Merrifield platform)
and may utilize ACPI powerfulness.
Bring minimum support by appending initial DSDT table for it.
Note, the addresses for generated tables are carefully chosen to avoid
any conflicts with existing shadowed BIOS data. The user have somewhat
like ~31 kB available for compiled ACPI tables that ought to be enough.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
board/intel/edison/.gitignore | 3 +++
board/intel/edison/Kconfig | 6 ++++++
board/intel/edison/Makefile | 1 +
board/intel/edison/dsdt.asl | 13 +++++++++++++
include/configs/edison.h | 3 +++
5 files changed, 26 insertions(+)
create mode 100644 board/intel/edison/.gitignore
create mode 100644 board/intel/edison/dsdt.asl
diff --git a/board/intel/edison/.gitignore b/board/intel/edison/.gitignore
new file mode 100644
index 0000000000..6eb8a5481a
--- /dev/null
+++ b/board/intel/edison/.gitignore
@@ -0,0 +1,3 @@
+dsdt.aml
+dsdt.asl.tmp
+dsdt.c
diff --git a/board/intel/edison/Kconfig b/board/intel/edison/Kconfig
index 4ff9d5adec..ef9b14aa2b 100644
--- a/board/intel/edison/Kconfig
+++ b/board/intel/edison/Kconfig
@@ -15,6 +15,12 @@ config SYS_CONFIG_NAME
config SYS_TEXT_BASE
default 0x01101000
+config ROM_TABLE_ADDR
+ default 0x0e4500
+
+config ROM_TABLE_SIZE
+ default 0x007b00
+
config BOARD_SPECIFIC_OPTIONS # dummy
def_bool y
select X86_LOAD_FROM_32_BIT
diff --git a/board/intel/edison/Makefile b/board/intel/edison/Makefile
index dde159435b..eed8d65eb6 100644
--- a/board/intel/edison/Makefile
+++ b/board/intel/edison/Makefile
@@ -5,3 +5,4 @@
#
obj-y += start.o edison.o
+obj-$(CONFIG_GENERATE_ACPI_TABLE) += dsdt.o
diff --git a/board/intel/edison/dsdt.asl b/board/intel/edison/dsdt.asl
new file mode 100644
index 0000000000..d2e04730c9
--- /dev/null
+++ b/board/intel/edison/dsdt.asl
@@ -0,0 +1,13 @@
+/*
+ * Copyright (c) 2017 Intel Corporation
+ *
+ * Partially based on dsdt.asl for other x86 boards
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+DefinitionBlock("dsdt.aml", "DSDT", 2, "U-BOOT", "U-BOOTBL", 0x00010000)
+{
+ /* platform specific */
+ #include <asm/arch/acpi/platform.asl>
+}
diff --git a/include/configs/edison.h b/include/configs/edison.h
index 399fbc7d9c..d62c0c783b 100644
--- a/include/configs/edison.h
+++ b/include/configs/edison.h
@@ -9,6 +9,9 @@
#include <asm/ibmpc.h>
+/* ACPI */
+#define CONFIG_LAST_STAGE_INIT
+
/* Boot */
#define CONFIG_BOOTCOMMAND "run bootcmd"
--
2.14.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v1 1/2] x86: tangier: Enable ACPI support for Intel Tangier
2017-08-30 15:04 ` [U-Boot] [PATCH v1 1/2] x86: tangier: Enable ACPI support for Intel Tangier Andy Shevchenko
@ 2017-09-03 9:46 ` Bin Meng
2017-10-02 11:26 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Bin Meng @ 2017-09-03 9:46 UTC (permalink / raw)
To: u-boot
Hi Andy,
On Wed, Aug 30, 2017 at 11:04 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> Intel Tangier SoC is a part of Intel Merrifield platform which doesn't
> utilize ACPI by default. Here is an attempt to unleash ACPI flexibility
> power on Intel Merrifield based platforms.
>
> The change brings minimum support of the devices that found on
> Intel Merrifield based end user device.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> arch/x86/cpu/tangier/Makefile | 1 +
> arch/x86/cpu/tangier/acpi.c | 86 ++++++
> .../include/asm/arch-tangier/acpi/global_nvs.asl | 16 ++
> .../x86/include/asm/arch-tangier/acpi/platform.asl | 31 +++
> .../include/asm/arch-tangier/acpi/southcluster.asl | 306 +++++++++++++++++++++
> arch/x86/include/asm/arch-tangier/global_nvs.h | 22 ++
> 6 files changed, 462 insertions(+)
> create mode 100644 arch/x86/cpu/tangier/acpi.c
> create mode 100644 arch/x86/include/asm/arch-tangier/acpi/global_nvs.asl
> create mode 100644 arch/x86/include/asm/arch-tangier/acpi/platform.asl
> create mode 100644 arch/x86/include/asm/arch-tangier/acpi/southcluster.asl
> create mode 100644 arch/x86/include/asm/arch-tangier/global_nvs.h
>
Generally it looks good. Some comments below.
> diff --git a/arch/x86/cpu/tangier/Makefile b/arch/x86/cpu/tangier/Makefile
> index d146b3f5c2..92cfa555ed 100644
> --- a/arch/x86/cpu/tangier/Makefile
> +++ b/arch/x86/cpu/tangier/Makefile
> @@ -5,3 +5,4 @@
> #
>
> obj-y += car.o tangier.o sdram.o
> +obj-$(CONFIG_GENERATE_ACPI_TABLE) += acpi.o
> diff --git a/arch/x86/cpu/tangier/acpi.c b/arch/x86/cpu/tangier/acpi.c
> new file mode 100644
> index 0000000000..fb15ce40ad
> --- /dev/null
> +++ b/arch/x86/cpu/tangier/acpi.c
> @@ -0,0 +1,86 @@
> +/*
> + * Copyright (c) 2017 Intel Corporation
> + *
> + * Partially based on acpi.c for other x86 platforms
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <cpu.h>
> +#include <dm.h>
> +#include <dm/uclass-internal.h>
> +#include <asm/acpi_table.h>
> +#include <asm/ioapic.h>
> +#include <asm/mpspec.h>
> +#include <asm/tables.h>
> +#include <asm/arch/global_nvs.h>
> +
> +void acpi_create_fadt(struct acpi_fadt *fadt, struct acpi_facs *facs,
> + void *dsdt)
> +{
> + struct acpi_table_header *header = &(fadt->header);
> +
> + memset((void *)fadt, 0, sizeof(struct acpi_fadt));
> +
> + acpi_fill_header(header, "FACP");
> + header->length = sizeof(struct acpi_fadt);
> + header->revision = 6;
> +
> + fadt->firmware_ctrl = (u32)facs;
> + fadt->dsdt = (u32)dsdt;
> + fadt->preferred_pm_profile = ACPI_PM_UNSPECIFIED;
> +
> + fadt->iapc_boot_arch = ACPI_FADT_VGA_NOT_PRESENT |
> + ACPI_FADT_NO_PCIE_ASPM_CONTROL;
> + fadt->flags =
> + ACPI_FADT_WBINVD |
> + ACPI_FADT_POWER_BUTTON | ACPI_FADT_SLEEP_BUTTON |
> + ACPI_FADT_SEALED_CASE | ACPI_FADT_HEADLESS |
> + ACPI_FADT_HW_REDUCED_ACPI;
> +
> + fadt->minor_revision = 2;
> +
> + fadt->x_firmware_ctl_l = (u32)facs;
> + fadt->x_firmware_ctl_h = 0;
> + fadt->x_dsdt_l = (u32)dsdt;
> + fadt->x_dsdt_h = 0;
> +
> + header->checksum = table_compute_checksum(fadt, header->length);
> +}
> +
> +u32 acpi_fill_madt(u32 current)
> +{
> + current += acpi_create_madt_lapics(current);
> +
> + current += acpi_create_madt_ioapic((struct acpi_madt_ioapic *)current,
> + io_apic_read(IO_APIC_ID) >> 24, IO_APIC_ADDR, 0);
> +
> + return current;
> +}
> +
> +u32 acpi_fill_mcfg(u32 current)
> +{
> + current += acpi_create_mcfg_mmconfig
> + ((struct acpi_mcfg_mmconfig *)current,
> + 0x3f500000, 0x0, 0x0, 0x0);
What is 0x3f500000? Can we define something in asm/arch/iomap.h (like
Baytrail) and use it here? And I see the end_bus_number is set to zero
here, why? Is this table a faked one? How about completely drop this
table? Does Linux boot without this table?
> +
> + return current;
> +}
> +
> +void acpi_create_gnvs(struct acpi_global_nvs *gnvs)
> +{
> + struct udevice *dev;
> + int ret;
> +
> + /* at least we have one processor */
> + gnvs->pcnt = 1;
> +
> + /* override the processor count with actual number */
> + ret = uclass_find_first_device(UCLASS_CPU, &dev);
> + if (ret == 0 && dev != NULL) {
> + ret = cpu_get_count(dev);
> + if (ret > 0)
> + gnvs->pcnt = ret;
> + }
> +}
> diff --git a/arch/x86/include/asm/arch-tangier/acpi/global_nvs.asl b/arch/x86/include/asm/arch-tangier/acpi/global_nvs.asl
> new file mode 100644
> index 0000000000..84fffbe140
> --- /dev/null
> +++ b/arch/x86/include/asm/arch-tangier/acpi/global_nvs.asl
> @@ -0,0 +1,16 @@
> +/*
> + * Copyright (c) 2017 Intel Corporation
> + *
> + * Partially based on global_nvs.asl for other x86 platforms
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <asm/acpi/global_nvs.h>
> +
> +OperationRegion(GNVS, SystemMemory, ACPI_GNVS_ADDR, ACPI_GNVS_SIZE)
> +Field(GNVS, ByteAcc, NoLock, Preserve)
> +{
> + Offset (0x00),
> + PCNT, 2, /* processor count */
2 is weird here. Why only two bits? I believe it should be 8 per your
global_nvs.h defines.
> +}
> diff --git a/arch/x86/include/asm/arch-tangier/acpi/platform.asl b/arch/x86/include/asm/arch-tangier/acpi/platform.asl
> new file mode 100644
> index 0000000000..a57b7cb319
> --- /dev/null
> +++ b/arch/x86/include/asm/arch-tangier/acpi/platform.asl
> @@ -0,0 +1,31 @@
> +/*
> + * Copyright (c) 2017 Intel Corporation
> + *
> + * Partially based on platform.asl for other x86 platforms
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <asm/acpi/statdef.asl>
> +
> +/*
> + * The _PTS method (Prepare To Sleep) is called before the OS is
> + * entering a sleep state. The sleep state number is passed in Arg0.
> + */
> +Method(_PTS, 1)
> +{
> +}
> +
> +/* The _WAK method is called on system wakeup */
> +Method(_WAK, 1)
> +{
> + Return (Package() {0, 0})
> +}
> +
> +/* ACPI global NVS */
> +#include "global_nvs.asl"
> +
> +Scope (\_SB)
> +{
> + #include "southcluster.asl"
> +}
> diff --git a/arch/x86/include/asm/arch-tangier/acpi/southcluster.asl b/arch/x86/include/asm/arch-tangier/acpi/southcluster.asl
> new file mode 100644
> index 0000000000..d3a9b114cb
> --- /dev/null
> +++ b/arch/x86/include/asm/arch-tangier/acpi/southcluster.asl
> @@ -0,0 +1,306 @@
> +/*
> + * Copyright (c) 2017 Intel Corporation
> + *
> + * Partially based on southcluster.asl for other x86 platforms
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +Device (PCI0)
> +{
> + Name (_HID, EISAID("PNP0A08")) /* PCIe */
> + Name (_CID, EISAID("PNP0A03")) /* PCI */
> +
> + Name (_ADR, 0)
> + Name (_BBN, 0)
> +
> + Name (MCRS, ResourceTemplate()
> + {
> + /* Bus Numbers */
> + WordBusNumber(ResourceProducer, MinFixed, MaxFixed, PosDecode,
> + 0x0000, 0x0000, 0x00ff, 0x0000, 0x0100, , , PB00)
> +
> + /* IO Region 0 */
> + WordIO(ResourceProducer, MinFixed, MaxFixed, PosDecode, EntireRange,
> + 0x0000, 0x0000, 0x0cf7, 0x0000, 0x0cf8, , , PI00)
> +
> + /* PCI Config Space */
> + IO(Decode16, 0x0cf8, 0x0cf8, 0x0001, 0x0008)
> +
> + /* IO Region 1 */
> + WordIO(ResourceProducer, MinFixed, MaxFixed, PosDecode, EntireRange,
> + 0x0000, 0x0d00, 0xffff, 0x0000, 0xf300, , , PI01)
> +
> + /* GPIO Low Memory Region */
> + DWordMemory(ResourceProducer, PosDecode, MinFixed, MaxFixed,
> + Cacheable, ReadWrite,
> + 0x00000000, 0x000ddcc0, 0x000ddccf, 0x00000000,
> + 0x00000010, , , GP00)
> +
> + /* PSH Memory Region 0 */
> + DWordMemory(ResourceProducer, PosDecode, MinFixed, MaxFixed,
> + Cacheable, ReadWrite,
> + 0x00000000, 0x04819000, 0x04898fff, 0x00000000,
> + 0x00080000, , , PSH0)
> +
> + /* PSH Memory Region 1 */
> + DWordMemory(ResourceProducer, PosDecode, MinFixed, MaxFixed,
> + Cacheable, ReadWrite,
> + 0x00000000, 0x04919000, 0x04920fff, 0x00000000,
> + 0x00008000, , , PSH1)
> +
> + /* SST Memory Region */
> + DWordMemory(ResourceProducer, PosDecode, MinFixed, MaxFixed,
> + Cacheable, ReadWrite,
> + 0x00000000, 0x05e00000, 0x05ffffff, 0x00000000,
> + 0x00200000, , , SST0)
> +
> + /* PCI Memory Region */
> + DWordMemory(ResourceProducer, PosDecode, MinFixed, MaxFixed,
> + Cacheable, ReadWrite,
> + 0x00000000, 0x80000000, 0xffffffff, 0x00000000,
> + 0x80000000, , , PMEM)
> + })
> +
> + Method (_CRS, 0, Serialized)
> + {
> + Return (MCRS)
> + }
> +
> + Method (_OSC, 4)
> + {
> + /* Check for proper GUID */
> + If (LEqual(Arg0, ToUUID("33db4d5b-1ff7-401c-9657-7441c03dd766"))) {
> + /* Let OS control everything */
> + Return (Arg3)
> + } Else {
> + /* Unrecognized UUID */
> + CreateDWordField(Arg3, 0, CDW1)
> + Or(CDW1, 4, CDW1)
> + Return (Arg3)
> + }
> + }
> +
> + Device (SDHC)
> + {
> + Name (_ADR, 0x00010003)
> + Name (_DEP, Package (0x01)
> + {
> + GPIO
> + })
> + Name (PSTS, Zero)
> +
> + Method (_STA)
> + {
> + Return (STA_VISIBLE)
> + }
> +
> + Method (_PS3, 0, NotSerialized)
> + {
> + }
> +
> + Method (_PS0, 0, NotSerialized)
> + {
> + If (PSTS == Zero)
> + {
nits: can we do something similar to U-Boot coding style with these If/Else?
> + If (^^GPIO.AVBL == One)
> + {
> + ^^GPIO.WFD3 = One
> + PSTS = One
> + }
> + }
> + }
> +
> + /* BCM43340 */
> + Device (BRC1)
> + {
> + Name (_ADR, One)
nits: since it represents an address, I would use 0x01 instead of One
> + Name (_DEP, Package (0x01)
> + {
> + GPIO
> + })
> +
> + Method (_STA)
> + {
> + Return (STA_VISIBLE)
> + }
> +
> + Method (_RMV, 0, NotSerialized)
> + {
> + Return (Zero)
> + }
> +
> + Method (_PS3, 0, NotSerialized)
> + {
> + If (^^^GPIO.AVBL == One)
> + {
> + ^^^GPIO.WFD3 = Zero
> + PSTS = Zero
> + }
> + }
> +
> + Method (_PS0, 0, NotSerialized)
> + {
> + If (PSTS == Zero)
> + {
> + If (^^^GPIO.AVBL == One)
> + {
> + ^^^GPIO.WFD3 = One
> + PSTS = One
> + }
> + }
> + }
> + }
> +
> + Device (BRC2)
> + {
> + Name (_ADR, 0x02)
> + Method (_STA, 0, NotSerialized)
> + {
> + Return (STA_VISIBLE)
> + }
> +
> + Method (_RMV, 0, NotSerialized)
> + {
> + Return (Zero)
> + }
> + }
> + }
> +
> + Device (SPI5)
> + {
> + Name (_ADR, 0x00070001)
> + Name (_DEP, Package (0x01)
> + {
> + GPIO
> + })
> + Name (RBUF, ResourceTemplate()
> + {
> + GpioIo(Exclusive, PullUp, 0, 0, IoRestrictionOutputOnly,
> + "\\_SB.PCI0.GPIO", 0, ResourceConsumer, , ) { 91 }
nits: can we use relative path here, like "GPIO"?
> + GpioIo(Exclusive, PullUp, 0, 0, IoRestrictionOutputOnly,
> + "\\_SB.PCI0.GPIO", 0, ResourceConsumer, , ) { 92 }
> + GpioIo(Exclusive, PullUp, 0, 0, IoRestrictionOutputOnly,
> + "\\_SB.PCI0.GPIO", 0, ResourceConsumer, , ) { 93 }
> + GpioIo(Exclusive, PullUp, 0, 0, IoRestrictionOutputOnly,
> + "\\_SB.PCI0.GPIO", 0, ResourceConsumer, , ) { 94 }
> + })
> +
> + Method (_CRS, 0, NotSerialized)
> + {
> + Return (RBUF)
> + }
> +
> + /*
> + * See
> + * http://www.kernel.org/doc/Documentation/acpi/gpio-properties.txt
> + * for more information about GPIO bindings.
> + */
> + Name (_DSD, Package () {
> + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> + Package () {
> + Package () {
> + "cs-gpios", Package () {
> + ^SPI5, 0, 0, 0,
> + ^SPI5, 1, 0, 0,
> + ^SPI5, 2, 0, 0,
> + ^SPI5, 3, 0, 0,
> + },
> + },
> + }
> + })
> +
> + Method (_STA, 0, NotSerialized)
> + {
> + Return (STA_VISIBLE)
> + }
> + }
> +
> + Device (I2C1)
> + {
> + Name (_ADR, 0x00080000)
> +
> + Method (_STA, 0, NotSerialized)
> + {
> + Return (STA_VISIBLE)
> + }
> + }
> +
> + Device (GPIO)
> + {
> + Name (_ADR, 0x000c0000)
> + Name (_DEP, Package (0x01)
> + {
> + \_SB.FLIS
> + })
Looks _DEP method is supported only for Operation Regions as I read
from ACPI spec? Does it work here?
> +
> + Method (_STA)
> + {
> + Return (STA_VISIBLE)
> + }
> +
> + Name (AVBL, Zero)
> + Method (_REG, 2, NotSerialized)
> + {
> + If (Arg0 == 0x08)
I assume 0x08 here means GeneralPurposeIO which is one of Operation
Region Address Space Identifiers Values? If yes, could we add
something in arch/x86/include/asm/acpi/ and use macro here?
> + {
> + AVBL = Arg1
> + }
> + }
> +
> + OperationRegion (GPOP, GeneralPurposeIo, Zero, 0x04)
> + Field (GPOP, ByteAcc, NoLock, Preserve)
> + {
> + Connection (
> + GpioIo(Exclusive, PullDefault, 0, 0, IoRestrictionOutputOnly,
> + "\\_SB.PCI0.GPIO", 0, ResourceConsumer, , ) { 56 }
> + ),
> + WFD3, 1,
> + }
> + }
> +
> + Device (PWM0)
> + {
> + Name (_ADR, 0x00170000)
> +
> + Method (_STA, 0, NotSerialized)
> + {
> + Return (STA_VISIBLE)
> + }
> + }
> +}
> +
> +Device (FLIS)
> +{
> + Name (_HID, "PRP0001")
> + Name (_DDN, "Intel Merrifield Family-Level Interface Shim")
> + Name (RBUF, ResourceTemplate()
> + {
> + Memory32Fixed(ReadWrite, 0xFF0C0000, 0x00008000, )
> + PinGroup("spi5", ResourceProducer, ) { 90, 91, 92, 93, 94, 95, 96 }
> + PinGroup("uart0", ResourceProducer, ) { 115, 116, 117, 118 }
> + PinGroup("uart1", ResourceProducer, ) { 119, 120, 121, 122 }
> + PinGroup("uart2", ResourceProducer, ) { 123, 124, 125, 126 }
> + PinGroup("pwm0", ResourceProducer, ) { 144 }
> + PinGroup("pwm1", ResourceProducer, ) { 145 }
> + PinGroup("pwm2", ResourceProducer, ) { 132 }
> + PinGroup("pwm3", ResourceProducer, ) { 133 }
> + })
> +
> + Method (_CRS, 0, NotSerialized)
> + {
> + Return (RBUF)
> + }
> +
> + Name (_DSD, Package () {
> + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Is this new UUID supposed to describe pinctrl? I don't see it in
include/acpi/acuuid.h in the kernel tree.
> + Package () {
> + Package () {"compatible", Package () {"intel,merrifield-pinctrl"}},
Is the 2nd Package() necessary? Like just below, is it OK?
Package () {"compatible", "intel,merrifield-pinctrl"},
I believe the intel,merrifield-pinctrl driver is an OF driver now, but
I don't see such string in current kernel tree. ACPI v6.2 has native
support of pinctrl, but kernel is not ready for it, so this is a
temporary placeholder?
> + }
> + })
> +
> + Method (_STA, 0, NotSerialized)
> + {
> + Return (STA_VISIBLE)
> + }
> +}
> diff --git a/arch/x86/include/asm/arch-tangier/global_nvs.h b/arch/x86/include/asm/arch-tangier/global_nvs.h
> new file mode 100644
> index 0000000000..8ab5cf2aa2
> --- /dev/null
> +++ b/arch/x86/include/asm/arch-tangier/global_nvs.h
> @@ -0,0 +1,22 @@
> +/*
> + * Copyright (c) 2017 Intel Corporation
> + *
> + * Partially based on global_nvs.h for other x86 platforms
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#ifndef _GLOBAL_NVS_H_
> +#define _GLOBAL_NVS_H_
> +
> +struct __packed acpi_global_nvs {
> + u8 pcnt; /* processor count */
> +
> + /*
> + * Add padding so sizeof(struct acpi_global_nvs) == 0x100.
> + * This must match the size defined in the global_nvs.asl.
> + */
> + u8 rsvd[255];
> +};
> +
> +#endif /* _GLOBAL_NVS_H_ */
> --
Regards,
Bin
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v1 2/2] x86: edison: Bring ACPI minimal support to the board
2017-08-30 15:04 ` [U-Boot] [PATCH v1 2/2] x86: edison: Bring ACPI minimal support to the board Andy Shevchenko
@ 2017-09-03 9:46 ` Bin Meng
0 siblings, 0 replies; 6+ messages in thread
From: Bin Meng @ 2017-09-03 9:46 UTC (permalink / raw)
To: u-boot
On Wed, Aug 30, 2017 at 11:04 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> This board is based on Intel Tangier SoC (Intel Merrifield platform)
> and may utilize ACPI powerfulness.
>
> Bring minimum support by appending initial DSDT table for it.
>
> Note, the addresses for generated tables are carefully chosen to avoid
> any conflicts with existing shadowed BIOS data. The user have somewhat
> like ~31 kB available for compiled ACPI tables that ought to be enough.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> board/intel/edison/.gitignore | 3 +++
> board/intel/edison/Kconfig | 6 ++++++
> board/intel/edison/Makefile | 1 +
> board/intel/edison/dsdt.asl | 13 +++++++++++++
> include/configs/edison.h | 3 +++
> 5 files changed, 26 insertions(+)
> create mode 100644 board/intel/edison/.gitignore
> create mode 100644 board/intel/edison/dsdt.asl
>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v1 1/2] x86: tangier: Enable ACPI support for Intel Tangier
2017-09-03 9:46 ` Bin Meng
@ 2017-10-02 11:26 ` Andy Shevchenko
0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2017-10-02 11:26 UTC (permalink / raw)
To: u-boot
On Sun, 2017-09-03 at 17:46 +0800, Bin Meng wrote:
> Hi Andy,
>
> On Wed, Aug 30, 2017 at 11:04 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > Intel Tangier SoC is a part of Intel Merrifield platform which
> > doesn't
> > utilize ACPI by default. Here is an attempt to unleash ACPI
> > flexibility
> > power on Intel Merrifield based platforms.
> >
> > The change brings minimum support of the devices that found on
> > Intel Merrifield based end user device.
> >
> Generally it looks good. Some comments below.
Thanks for review, my answers below.
> > +u32 acpi_fill_mcfg(u32 current)
> > +{
> > + current += acpi_create_mcfg_mmconfig
> > + ((struct acpi_mcfg_mmconfig *)current,
> > + 0x3f500000, 0x0, 0x0, 0x0);
>
> What is 0x3f500000?
Nice question (I have in my local branch an additional commit with FIXME
near to this line)!
The root cause is a hardware design here. There are two (okay, on those
platforms up to four) *real* PCI devices. The rest have PCI
*programming* interface but being not PCI at the same time. This is a
magic address of so called PCI Shim for those (non-PCI) devices which
filed by firmware.
> Can we define something in asm/arch/iomap.h (like
> Baytrail) and use it here?
It comes from SFI, so, the best approach is to parse SFI for that.
I would not do this right now (will take a lot of time for not much
benefit), though can put as TODO.
> And I see the end_bus_number is set to zero
> here, why?
See above.
> Is this table a faked one?
It's based on SFI.
> How about completely drop this
> table? Does Linux boot without this table?
It might start booting (as initial part), but it will lose an ability to
enumerate almost all devices in SoC. Basically user will not get even
console.
> > +
> > + return current;
> > +}
> >
> > +OperationRegion(GNVS, SystemMemory, ACPI_GNVS_ADDR, ACPI_GNVS_SIZE)
> > +Field(GNVS, ByteAcc, NoLock, Preserve)
> > +{
> > + Offset (0x00),
> > + PCNT, 2, /* processor count */
>
> 2 is weird here. Why only two bits? I believe it should be 8 per your
> global_nvs.h defines.
Ah, I misread what that number means. Will fix.
> > + Method (_PS0, 0, NotSerialized)
> > + {
> > + If (PSTS == Zero)
> > + {
>
> nits: can we do something similar to U-Boot coding style with these
> If/Else?
I would rather follow iasl style for ASL.
Though if it's obliged to use U-Boot style over all files, I can switch.
>
> > + If (^^GPIO.AVBL == One)
> > + {
> > + ^^GPIO.WFD3 = One
> > + PSTS = One
> > + }
> > + }
> > + }
> > +
> > + /* BCM43340 */
> > + Device (BRC1)
> > + {
> > + Name (_ADR, One)
>
> nits: since it represents an address, I would use 0x01 instead of One
I have no strong opinion here, ACPI spec says
"The use of this operator can reduce AML code size, since it is
represented by a one-byte AML opcode."
Same for Zero.
Though to be consistent with the rest, I will change it.
> + Device (BRC2)
> > + {
> > + Name (_ADR, 0x02)
> >
> > + Name (RBUF, ResourceTemplate()
> > + {
> > + GpioIo(Exclusive, PullUp, 0, 0,
> > IoRestrictionOutputOnly,
> > + "\\_SB.PCI0.GPIO", 0, ResourceConsumer, , ) { 91 }
>
> nits: can we use relative path here, like "GPIO"?
While spec says:
"ResourceSource can be a fully-qualified name, a relative name or a name
segment that utilizes the namespace search rules."
I would rather follow common practice, i.e. to use fully-qualified name
for GPIO and Pin control resources.
>
> > + GpioIo(Exclusive, PullUp, 0, 0,
> > IoRestrictionOutputOnly,
> > + "\\_SB.PCI0.GPIO", 0, ResourceConsumer, , ) { 92 }
> > + GpioIo(Exclusive, PullUp, 0, 0,
> > IoRestrictionOutputOnly,
> > + "\\_SB.PCI0.GPIO", 0, ResourceConsumer, , ) { 93 }
> > + GpioIo(Exclusive, PullUp, 0, 0,
> > IoRestrictionOutputOnly,
> > + "\\_SB.PCI0.GPIO", 0, ResourceConsumer, , ) { 94 }
> >
> > + Device (GPIO)
> > + {
> > + Name (_ADR, 0x000c0000)
> > + Name (_DEP, Package (0x01)
> > + {
> > + \_SB.FLIS
> > + })
>
> Looks _DEP method is supported only for Operation Regions as I read
> from ACPI spec? Does it work here?
No, it doesn't (yet?). I would move it out to my debugging stuff.
> > +
> > + Method (_STA)
> > + {
> > + Return (STA_VISIBLE)
> > + }
> > +
> > + Name (AVBL, Zero)
> > + Method (_REG, 2, NotSerialized)
> > + {
> > + If (Arg0 == 0x08)
>
> I assume 0x08 here means GeneralPurposeIO which is one of Operation
> Region Address Space Identifiers Values? If yes, could we add
> something in arch/x86/include/asm/acpi/ and use macro here?
This part I got based on disassemble from iasl. Moreover, there are two
scopes for this value: inside OperationRegion() and outside. Since iasl
does not replace it with such (existing) value, I would follow it rather
than creating a potential collision with names and scopes.
> > + Name (_DSD, Package () {
> > + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>
> Is this new UUID supposed to describe pinctrl? I don't see it in
> include/acpi/acuuid.h in the kernel tree.
Nope, this UUID is supposed to describe device properties.
Please, refer to 6.2.5 _DSD (Device Specific Data) in the spec.
> > + Package () {
> > + Package () {"compatible", Package ()
> > {"intel,merrifield-pinctrl"}},
>
> Is the 2nd Package() necessary? Like just below, is it OK?
> Package () {"compatible", "intel,merrifield-pinctrl"},
In this case yes, we have only one value for the "compatible" key.
I will change it.
> I believe the intel,merrifield-pinctrl driver is an OF driver now,
No.
The only way for now to get it enumerated via ACPI is to apply
compatible string.
> but I don't see such string in current kernel tree.
I sent recently a v2 which adds such binding to the kernel sources.
> ACPI v6.2 has native
> support of pinctrl, but kernel is not ready for it, so this is a
> temporary placeholder?
The first part of sentence has nothing to do with the question at the
second.
Kernel is not ready for pin control glue layer (ACPICA already handles
that), but it is orthogonal to compatible strings.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-10-02 11:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-30 15:04 [U-Boot] [RFC PATCH v1 0/2] edison: Enable ACPI Andy Shevchenko
2017-08-30 15:04 ` [U-Boot] [PATCH v1 1/2] x86: tangier: Enable ACPI support for Intel Tangier Andy Shevchenko
2017-09-03 9:46 ` Bin Meng
2017-10-02 11:26 ` Andy Shevchenko
2017-08-30 15:04 ` [U-Boot] [PATCH v1 2/2] x86: edison: Bring ACPI minimal support to the board Andy Shevchenko
2017-09-03 9:46 ` Bin Meng
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.