[RFC PATCH for AMD Seattle 0/4] Drivers for AMD-Seatlle to boot from ACPI

[RFC PATCH for AMD Seattle 0/4] Drivers for AMD-Seatlle to boot from ACPI

[suravee.suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

As a follow up of the email thread:

    [PATCH v3 00/17] Introduce ACPI for ARM64 based on ACPI 5.1
    https://lkml.org/lkml/2014/9/1/446

Besides Hanjun Guo's patches above, these are the additional patches required
to boot AMD Seattle platform with full ACPI support:

    [PATCH] efi/arm64: efistub: don't abort if base of DRAM is occupied
    http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/272274.html

    [PATCH v2] efi/arm64: fix fdt-related memory reservation
    https://lkml.org/lkml/2014/9/8/483

    [PATCH] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller
    https://lkml.org/lkml/2014/9/9/834

    [RFC PATCH for Juno 2/2] tty: SBSA compatible UART
    https://lkml.org/lkml/2014/9/1/448
    (NOTE: This is not meant to be submitted upstream, but required to enable
     ACPI support for UART SBSA driver.)

These patches are rebased from the linux-3.17-rc4 upstream kernel, and 
compatible with the AMD Seattle Firmware version ROD0070C.
The full kernel boot log can be found here:

    http://people.linaro.org/~al.stone/seattle-boot-acpi-dmesg.txt

Suravee / Al

Ard Biesheuvel (1):
  arm64/efi: efistub: don't abort if base of DRAM is occupied

Grame Gregory (1):
  [RFC PATCH for Juno 2/2] tty: SBSA compatible UART

Mark Salter (1):
  efi/arm64: fix fdt-related memory reservation

Suravee Suthikulpanit (1):
  ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller

 arch/arm64/kernel/efi-stub.c       |  16 +-
 arch/arm64/mm/init.c               |   3 +-
 drivers/ata/Kconfig                |   2 +-
 drivers/ata/ahci_platform.c        |  13 ++
 drivers/firmware/efi/libstub/fdt.c |  10 +-
 drivers/tty/Kconfig                |   6 +
 drivers/tty/Makefile               |   1 +
 drivers/tty/sbsauart.c             | 328 +++++++++++++++++++++++++++++++++++++
 8 files changed, 365 insertions(+), 14 deletions(-)
 create mode 100644 drivers/tty/sbsauart.c

-- 
1.9.3

[PATCH 1/4] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller

[suravee.suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

This patch adds ACPI match table in ahci_platform. The table includes
the acpi_device_id to match AMD Seattle SATA controller with following
asl structure in DSDT:

    Device (SATA0)
    {
      Name(_HID, "AMDI0600")	// Seattle AHSATA
      Name (_CCA, 1)		// Cache-coherent controller
      Name (_CRS, ResourceTemplate ()
      {
        Memory32Fixed (ReadWrite, 0xE0300000, 0x00010000)
        Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive,,,) { 387 }
      })
    }

Also, since ATA driver should not require PCI support for ATA_ACPI,
this patch removes dependency in the driver/ata/Kconfig.
---
 drivers/ata/Kconfig         |  2 +-
 drivers/ata/ahci_platform.c | 13 +++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index e1b9278..f2e6c9e 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -48,7 +48,7 @@ config ATA_VERBOSE_ERROR
 
 config ATA_ACPI
 	bool "ATA ACPI Support"
-	depends on ACPI && PCI
+	depends on ACPI
 	default y
 	help
 	  This option adds support for ATA-related ACPI objects.
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index f61ddb9..3499bab 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -20,6 +20,9 @@
 #include <linux/platform_device.h>
 #include <linux/libata.h>
 #include <linux/ahci_platform.h>
+#ifdef CONFIG_ATA_ACPI
+#include <linux/acpi.h>
+#endif
 #include "ahci.h"
 
 static const struct ata_port_info ahci_port_info = {
@@ -87,6 +90,13 @@ static const struct of_device_id ahci_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, ahci_of_match);
 
+#ifdef CONFIG_ATA_ACPI
+static const struct acpi_device_id ahci_acpi_match[] = {
+	{ "AMDI0600", 0 }, /* AMD Seattle AHCI */
+	{ },
+};
+#endif
+
 static struct platform_driver ahci_driver = {
 	.probe = ahci_probe,
 	.remove = ata_platform_remove_one,
@@ -94,6 +104,9 @@ static struct platform_driver ahci_driver = {
 		.name = "ahci",
 		.owner = THIS_MODULE,
 		.of_match_table = ahci_of_match,
+#ifdef CONFIG_ATA_ACPI
+		.acpi_match_table = ACPI_PTR(ahci_acpi_match),
+#endif
 		.pm = &ahci_pm_ops,
 	},
 };
-- 
1.9.3

[PATCH 2/4] arm64/efi: efistub: don't abort if base of DRAM is occupied

[suravee.suthikulpanit]

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

If we cannot relocate the kernel Image to its preferred offset of base of DRAM
plus TEXT_OFFSET, instead relocate it to the lowest available 2 MB boundary plus
TEXT_OFFSET. We may lose a bit of memory at the low end, but we can still
proceed normally otherwise.

Acked-by: Mark Salter <msalter@redhat.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Leif Lindholm <leif.lindholm@linaro.org>
Tested-by: Leif Lindholm <leif.lindholm@linaro.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/efi-stub.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/kernel/efi-stub.c b/arch/arm64/kernel/efi-stub.c
index 1317fef..d27dd98 100644
--- a/arch/arm64/kernel/efi-stub.c
+++ b/arch/arm64/kernel/efi-stub.c
@@ -28,20 +28,16 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table,
 	kernel_size = _edata - _text;
 	if (*image_addr != (dram_base + TEXT_OFFSET)) {
 		kernel_memsize = kernel_size + (_end - _edata);
-		status = efi_relocate_kernel(sys_table, image_addr,
-					     kernel_size, kernel_memsize,
-					     dram_base + TEXT_OFFSET,
-					     PAGE_SIZE);
+		status = efi_low_alloc(sys_table, kernel_memsize + TEXT_OFFSET,
+				       SZ_2M, reserve_addr);
 		if (status != EFI_SUCCESS) {
 			pr_efi_err(sys_table, "Failed to relocate kernel\n");
 			return status;
 		}
-		if (*image_addr != (dram_base + TEXT_OFFSET)) {
-			pr_efi_err(sys_table, "Failed to alloc kernel memory\n");
-			efi_free(sys_table, kernel_memsize, *image_addr);
-			return EFI_LOAD_ERROR;
-		}
-		*image_size = kernel_memsize;
+		memcpy((void *)*reserve_addr + TEXT_OFFSET, (void *)*image_addr,
+		       kernel_size);
+		*image_addr = *reserve_addr + TEXT_OFFSET;
+		*reserve_size = kernel_memsize + TEXT_OFFSET;
 	}
 
 
-- 
1.9.3

[PATCH 3/4] efi/arm64: fix fdt-related memory reservation

[suravee.suthikulpanit]

From: Mark Salter <msalter@redhat.com>

Commit 86c8b27a01cf:
 "arm64: ignore DT memreserve entries when booting in UEFI mode

prevents early_init_fdt_scan_reserved_mem() from being called for
arm64 kernels booting via UEFI. This was done because the kernel
will use the UEFI memory map to determine reserved memory regions.
That approach has problems in that early_init_fdt_scan_reserved_mem()
also reserves the FDT itself and any node-specific reserved memory.
By chance of some kernel configs, the FDT may be overwritten before
it can be unflattened and the kernel will fail to boot. More subtle
problems will result if the FDT has node specific reserved memory
which is not really reserved.

This patch has the UEFI stub remove the memory reserve map entries
from the FDT as it does with the memory nodes. This allows
early_init_fdt_scan_reserved_mem() to be called unconditionally
so that the other needed reservations are made.

Signed-off-by: Mark Salter <msalter@redhat.com>
---
 arch/arm64/mm/init.c               |  3 +--
 drivers/firmware/efi/libstub/fdt.c | 10 +++++++++-
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 5472c24..a83061f 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -149,8 +149,7 @@ void __init arm64_memblock_init(void)
 		memblock_reserve(__virt_to_phys(initrd_start), initrd_end - initrd_start);
 #endif
 
-	if (!efi_enabled(EFI_MEMMAP))
-		early_init_fdt_scan_reserved_mem();
+	early_init_fdt_scan_reserved_mem();
 
 	/* 4GB maximum for 32-bit only capable devices */
 	if (IS_ENABLED(CONFIG_ZONE_DMA))
diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index a56bb35..c846a96 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -22,7 +22,7 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
 			unsigned long map_size, unsigned long desc_size,
 			u32 desc_ver)
 {
-	int node, prev;
+	int node, prev, num_rsv;
 	int status;
 	u32 fdt_val32;
 	u64 fdt_val64;
@@ -73,6 +73,14 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
 		prev = node;
 	}
 
+	/*
+	 * Delete all memory reserve map entries. When booting via UEFI,
+	 * kernel will use the UEFI memory map to find reserved regions.
+	 */
+	num_rsv = fdt_num_mem_rsv(fdt);
+	while (num_rsv-- > 0)
+		fdt_del_mem_rsv(fdt, num_rsv);
+
 	node = fdt_subnode_offset(fdt, 0, "chosen");
 	if (node < 0) {
 		node = fdt_add_subnode(fdt, 0, "chosen");
-- 
1.9.3

[PATCH 4/4] [RFC PATCH for Juno 2/2] tty: SBSA compatible UART

[suravee.suthikulpanit]

From: Grame Gregory <graeme.gregory@linaro.org>

This is a subset of pl011 UART which does not supprt DMA or baud rate
changing.

It is specified in the Server Base System Architecture document from
ARM.

Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org>
---
 drivers/tty/Kconfig    |   6 +
 drivers/tty/Makefile   |   1 +
 drivers/tty/sbsauart.c | 328 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 335 insertions(+)
 create mode 100644 drivers/tty/sbsauart.c

diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
index b24aa01..50fe279 100644
--- a/drivers/tty/Kconfig
+++ b/drivers/tty/Kconfig
@@ -419,4 +419,10 @@ config DA_CONSOLE
 	help
 	  This enables a console on a Dash channel.
 
+config SBSAUART_TTY
+	tristate "SBSA UART TTY Driver"
+	help
+	  Console and system TTY driver for the SBSA UART which is defined
+	  in the Server Base System Architecure document for ARM64 servers.
+
 endif # TTY
diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile
index 58ad1c0..c3211c0 100644
--- a/drivers/tty/Makefile
+++ b/drivers/tty/Makefile
@@ -29,5 +29,6 @@ obj-$(CONFIG_SYNCLINK)		+= synclink.o
 obj-$(CONFIG_PPC_EPAPR_HV_BYTECHAN) += ehv_bytechan.o
 obj-$(CONFIG_GOLDFISH_TTY)	+= goldfish.o
 obj-$(CONFIG_DA_TTY)		+= metag_da.o
+obj-$(CONFIG_SBSAUART_TTY)	+= sbsauart.o
 
 obj-y += ipwireless/
diff --git a/drivers/tty/sbsauart.c b/drivers/tty/sbsauart.c
new file mode 100644
index 0000000..c9bf923
--- /dev/null
+++ b/drivers/tty/sbsauart.c
@@ -0,0 +1,328 @@
+/*
+ * SBSA (Server Base System Architecture) Compatible UART driver
+ *
+ * Copyright (C) 2014 Linaro Ltd
+ *
+ * Author: Graeme Gregory <graeme.gregory@linaro.org>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/acpi.h>
+#include <linux/amba/serial.h>
+#include <linux/console.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
+
+struct sbsa_tty {
+	struct tty_port port;
+	spinlock_t lock;
+	void __iomem *base;
+	u32 irq;
+	int opencount;
+	struct console console;
+};
+
+static struct tty_driver *sbsa_tty_driver;
+static struct sbsa_tty *sbsa_tty;
+
+#define SBSAUART_CHAR_MASK	0xFF
+
+static void sbsa_tty_do_write(const char *buf, unsigned count)
+{
+	unsigned long irq_flags;
+	struct sbsa_tty *qtty = sbsa_tty;
+	void __iomem *base = qtty->base;
+	unsigned n;
+
+	spin_lock_irqsave(&qtty->lock, irq_flags);
+	for (n = 0; n < count; n++) {
+		while (readw(base + UART01x_FR) & UART01x_FR_TXFF)
+			mdelay(10);
+		writew(buf[n], base + UART01x_DR);
+	}
+	spin_unlock_irqrestore(&qtty->lock, irq_flags);
+}
+
+static void sbsauart_fifo_to_tty(struct sbsa_tty *qtty)
+{
+	void __iomem *base = qtty->base;
+	unsigned int flag, max_count = 32;
+	u16 status, ch;
+
+	while (max_count--) {
+		status = readw(base + UART01x_FR);
+		if (status & UART01x_FR_RXFE)
+			break;
+
+		/* Take chars from the FIFO and update status */
+		ch = readw(base + UART01x_DR);
+		flag = TTY_NORMAL;
+
+		if (ch & UART011_DR_BE)
+			flag = TTY_BREAK;
+		else if (ch & UART011_DR_PE)
+			flag = TTY_PARITY;
+		else if (ch & UART011_DR_FE)
+			flag = TTY_FRAME;
+		else if (ch & UART011_DR_OE)
+			flag = TTY_OVERRUN;
+
+		ch &= SBSAUART_CHAR_MASK;
+
+		tty_insert_flip_char(&qtty->port, ch, flag);
+	}
+
+	tty_schedule_flip(&qtty->port);
+
+	/* Clear the RX IRQ */
+	writew(UART011_RXIC | UART011_RXIC, base + UART011_ICR);
+}
+
+static irqreturn_t sbsa_tty_interrupt(int irq, void *dev_id)
+{
+	struct sbsa_tty *qtty = sbsa_tty;
+	unsigned long irq_flags;
+
+	spin_lock_irqsave(&qtty->lock, irq_flags);
+	sbsauart_fifo_to_tty(qtty);
+	spin_unlock_irqrestore(&qtty->lock, irq_flags);
+
+	return IRQ_HANDLED;
+}
+
+static int sbsa_tty_open(struct tty_struct *tty, struct file *filp)
+{
+	struct sbsa_tty *qtty = sbsa_tty;
+
+	return tty_port_open(&qtty->port, tty, filp);
+}
+
+static void sbsa_tty_close(struct tty_struct *tty, struct file *filp)
+{
+	tty_port_close(tty->port, tty, filp);
+}
+
+static void sbsa_tty_hangup(struct tty_struct *tty)
+{
+	tty_port_hangup(tty->port);
+}
+
+static int sbsa_tty_write(struct tty_struct *tty, const unsigned char *buf,
+								int count)
+{
+	sbsa_tty_do_write(buf, count);
+	return count;
+}
+
+static int sbsa_tty_write_room(struct tty_struct *tty)
+{
+	return 32;
+}
+
+static void sbsa_tty_console_write(struct console *co, const char *b,
+								unsigned count)
+{
+	sbsa_tty_do_write(b, count);
+
+	if(b[count - 1] == '\n');
+		sbsa_tty_do_write("\r", 1);
+}
+
+static struct tty_driver *sbsa_tty_console_device(struct console *c,
+								int *index)
+{
+	*index = c->index;
+	return sbsa_tty_driver;
+}
+
+static int sbsa_tty_console_setup(struct console *co, char *options)
+{
+	if ((unsigned)co->index > 0)
+		return -ENODEV;
+	if (sbsa_tty->base == NULL)
+		return -ENODEV;
+	return 0;
+}
+
+static struct tty_port_operations sbsa_port_ops = {
+};
+
+static const struct tty_operations sbsa_tty_ops = {
+	.open = sbsa_tty_open,
+	.close = sbsa_tty_close,
+	.hangup = sbsa_tty_hangup,
+	.write = sbsa_tty_write,
+	.write_room = sbsa_tty_write_room,
+};
+
+static int sbsa_tty_create_driver(void)
+{
+	int ret;
+	struct tty_driver *tty;
+
+	sbsa_tty = kzalloc(sizeof(*sbsa_tty), GFP_KERNEL);
+	if (sbsa_tty == NULL) {
+		ret = -ENOMEM;
+		goto err_alloc_sbsa_tty_failed;
+	}
+	tty = alloc_tty_driver(1);
+	if (tty == NULL) {
+		ret = -ENOMEM;
+		goto err_alloc_tty_driver_failed;
+	}
+	tty->driver_name = "sbsauart";
+	tty->name = "ttySBSA";
+	tty->type = TTY_DRIVER_TYPE_SERIAL;
+	tty->subtype = SERIAL_TYPE_NORMAL;
+	tty->init_termios = tty_std_termios;
+	tty->flags = TTY_DRIVER_RESET_TERMIOS | TTY_DRIVER_REAL_RAW |
+						TTY_DRIVER_DYNAMIC_DEV;
+	tty_set_operations(tty, &sbsa_tty_ops);
+	ret = tty_register_driver(tty);
+	if (ret)
+		goto err_tty_register_driver_failed;
+
+	sbsa_tty_driver = tty;
+	return 0;
+
+err_tty_register_driver_failed:
+	put_tty_driver(tty);
+err_alloc_tty_driver_failed:
+	kfree(sbsa_tty);
+	sbsa_tty = NULL;
+err_alloc_sbsa_tty_failed:
+	return ret;
+}
+
+static void sbsa_tty_delete_driver(void)
+{
+	tty_unregister_driver(sbsa_tty_driver);
+	put_tty_driver(sbsa_tty_driver);
+	sbsa_tty_driver = NULL;
+	kfree(sbsa_tty);
+	sbsa_tty = NULL;
+}
+
+static int sbsa_tty_probe(struct platform_device *pdev)
+{
+	struct sbsa_tty *qtty;
+	int ret = -EINVAL;
+	int i;
+	struct resource *r;
+	struct device *ttydev;
+	void __iomem *base;
+	u32 irq;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (r == NULL)
+		return -EINVAL;
+
+	base = ioremap(r->start, r->end - r->start);
+	if (base == NULL)
+		pr_err("sbsa_tty: unable to remap base\n");
+
+	r = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (r == NULL)
+		goto err_unmap;
+
+	irq = r->start;
+
+	if (pdev->id > 0)
+		goto err_unmap;
+
+	ret = sbsa_tty_create_driver();
+	if (ret)
+		goto err_unmap;
+
+	qtty = sbsa_tty;
+	spin_lock_init(&qtty->lock);
+	tty_port_init(&qtty->port);
+	qtty->port.ops = &sbsa_port_ops;
+	qtty->base = base;
+	qtty->irq = irq;
+
+	/* Clear and Mask all IRQs */
+	writew(0, base + UART011_IMSC);
+	writew(0xFFFF, base + UART011_ICR);
+
+	ret = request_irq(irq, sbsa_tty_interrupt, IRQF_SHARED,
+						"sbsa_tty", pdev);
+	if (ret)
+		goto err_request_irq_failed;
+
+	/* Unmask the RX IRQ */
+	writew(UART011_RXIM | UART011_RTIM, base + UART011_IMSC);
+
+	ttydev = tty_port_register_device(&qtty->port, sbsa_tty_driver,
+							0, &pdev->dev);
+	if (IS_ERR(ttydev)) {
+		ret = PTR_ERR(ttydev);
+		goto err_tty_register_device_failed;
+	}
+
+	strcpy(qtty->console.name, "ttySBSA");
+	qtty->console.write = sbsa_tty_console_write;
+	qtty->console.device = sbsa_tty_console_device;
+	qtty->console.setup = sbsa_tty_console_setup;
+	qtty->console.flags = CON_PRINTBUFFER;
+	qtty->console.index = pdev->id;
+	register_console(&qtty->console);
+
+	return 0;
+
+	tty_unregister_device(sbsa_tty_driver, i);
+err_tty_register_device_failed:
+	free_irq(irq, pdev);
+err_request_irq_failed:
+	sbsa_tty_delete_driver();
+err_unmap:
+	iounmap(base);
+	return ret;
+}
+
+static int sbsa_tty_remove(struct platform_device *pdev)
+{
+	struct sbsa_tty *qtty;
+
+	qtty = sbsa_tty;
+	unregister_console(&qtty->console);
+	tty_unregister_device(sbsa_tty_driver, pdev->id);
+	iounmap(qtty->base);
+	qtty->base = 0;
+	free_irq(qtty->irq, pdev);
+	sbsa_tty_delete_driver();
+	return 0;
+}
+
+static const struct acpi_device_id sbsa_acpi_match[] = {
+	{ "ARMH0011", 0 },
+	{ }
+};
+
+static struct platform_driver sbsa_tty_platform_driver = {
+	.probe = sbsa_tty_probe,
+	.remove = sbsa_tty_remove,
+	.driver = {
+		.name = "sbsa_tty",
+		.acpi_match_table = ACPI_PTR(sbsa_acpi_match),
+	}
+};
+
+module_platform_driver(sbsa_tty_platform_driver);
+
+MODULE_LICENSE("GPL v2");
-- 
1.9.3

Re: [RFC PATCH for AMD Seattle 0/4] Drivers for AMD-Seatlle to boot from ACPI

[Suravee Suthikulpanit]

Note: Correcting the typo for linux-acpi mailing list in this reply.

Also, I would like to add the link to the "AMD Opteron™ A1100 Series 
Processor ACPI Porting Guide", which describes the current ACPI table 
for AMD Seattle platform here:

http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2012/10/Seattle_ACPI_Guide.pdf

Suravee

On 09/15/2014 07:47 PM, suravee.suthikulpanit@amd.com wrote:
> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>
> As a follow up of the email thread:
>
>      [PATCH v3 00/17] Introduce ACPI for ARM64 based on ACPI 5.1
>      https://lkml.org/lkml/2014/9/1/446
>
> Besides Hanjun Guo's patches above, these are the additional patches required
> to boot AMD Seattle platform with full ACPI support:
>
>      [PATCH] efi/arm64: efistub: don't abort if base of DRAM is occupied
>      http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/272274.html
>
>      [PATCH v2] efi/arm64: fix fdt-related memory reservation
>      https://lkml.org/lkml/2014/9/8/483
>
>      [PATCH] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller
>      https://lkml.org/lkml/2014/9/9/834
>
>      [RFC PATCH for Juno 2/2] tty: SBSA compatible UART
>      https://lkml.org/lkml/2014/9/1/448
>      (NOTE: This is not meant to be submitted upstream, but required to enable
>       ACPI support for UART SBSA driver.)
>
> These patches are rebased from the linux-3.17-rc4 upstream kernel, and
> compatible with the AMD Seattle Firmware version ROD0070C.
> The full kernel boot log can be found here:
>
>      http://people.linaro.org/~al.stone/seattle-boot-acpi-dmesg.txt
>
> Suravee / Al
>
> Ard Biesheuvel (1):
>    arm64/efi: efistub: don't abort if base of DRAM is occupied
>
> Grame Gregory (1):
>    [RFC PATCH for Juno 2/2] tty: SBSA compatible UART
>
> Mark Salter (1):
>    efi/arm64: fix fdt-related memory reservation
>
> Suravee Suthikulpanit (1):
>    ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller
>
>   arch/arm64/kernel/efi-stub.c       |  16 +-
>   arch/arm64/mm/init.c               |   3 +-
>   drivers/ata/Kconfig                |   2 +-
>   drivers/ata/ahci_platform.c        |  13 ++
>   drivers/firmware/efi/libstub/fdt.c |  10 +-
>   drivers/tty/Kconfig                |   6 +
>   drivers/tty/Makefile               |   1 +
>   drivers/tty/sbsauart.c             | 328 +++++++++++++++++++++++++++++++++++++
>   8 files changed, 365 insertions(+), 14 deletions(-)
>   create mode 100644 drivers/tty/sbsauart.c
>

Re: [RFC PATCH for AMD Seattle 0/4] Drivers for AMD-Seatlle to boot from ACPI

[Jon Masters]

On 09/16/2014 12:28 AM, Suravee Suthikulpanit wrote:
> Note: Correcting the typo for linux-acpi mailing list in this reply.
> 
> Also, I would like to add the link to the "AMD Opteron™ A1100 Series 
> Processor ACPI Porting Guide", which describes the current ACPI table 
> for AMD Seattle platform here:
> 
> http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2012/10/Seattle_ACPI_Guide.pdf

Thanks again for posting the patch series, and this document, which is
very useful. We have test systems running these patches right now, and
they work quite well. If anyone at Linaro Connect this week would like
to see hardware and get their own kernel built running these bits, let
myself, Suravee, or Al know and we'll help you make that happen.

Jon.

Re: [PATCH 1/4] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller

[Matthew Garrett]

On Mon, Sep 15, 2014 at 07:47:23PM -0500, suravee.suthikulpanit@amd.com wrote:
> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> 
> This patch adds ACPI match table in ahci_platform. The table includes
> the acpi_device_id to match AMD Seattle SATA controller with following
> asl structure in DSDT:
> 
>     Device (SATA0)
>     {
>       Name(_HID, "AMDI0600")	// Seattle AHSATA

There really ought to be a well-defined PNPID for AHCI, so you can _HID 
to AMD and _CID to something generic. That way we won't have:

> +#ifdef CONFIG_ATA_ACPI
> +static const struct acpi_device_id ahci_acpi_match[] = {
> +	{ "AMDI0600", 0 }, /* AMD Seattle AHCI */
> +	{ },
> +};

utter sadness here. Really, please don't end up in a situation where we 
need to add device-specific IDs to a generic driver.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 4/4] [RFC PATCH for Juno 2/2] tty: SBSA compatible UART

[One Thousand Gnomes]

Firstly provide some useful information about the hardware. It's no good
wavng your arms at a document that requires agreeing to a giant ARM T&C
to get access to. Most of don't work for ARM and we'd have to get our own
corporate legal to approve the legal garbage involved.

Secondly explain why you can't use PL011 given that it already supports
non DMA accesses. What would it take to tweak it further for this ?


> +static void sbsa_tty_do_write(const char *buf, unsigned count)
> +{
> +	unsigned long irq_flags;
> +	struct sbsa_tty *qtty = sbsa_tty;
> +	void __iomem *base = qtty->base;
> +	unsigned n;
> +
> +	spin_lock_irqsave(&qtty->lock, irq_flags);
> +	for (n = 0; n < count; n++) {
> +		while (readw(base + UART01x_FR) & UART01x_FR_TXFF)
> +			mdelay(10);
> +		writew(buf[n], base + UART01x_DR);

serious - you are going to sit and spin in kernel space with interrupts
off for an indefinite period ?

No. Even if your hardware is so completely brain dead and broken that it
hasn't got an interrupt for 'write room' that's not acceptable. You need
error handling and some kind of sensible timer based solution.

To put it simply. You have a queue (or you should - your driver is broken
in that respect too), you have a baud rate, you have a bit time. From
that you can compute sensible wakeup points to try and refill the
hardware FIFO. Assuming the hardware fifo is not tiny you don't even have
to be that good an aim.

It's acceptable for the printk console code to spin, if you think getting
the message out on a failure or error outweighs the pain. It's not
acceptable for the tty layer.

> +static void sbsauart_fifo_to_tty(struct sbsa_tty *qtty)
> +{
> +	void __iomem *base = qtty->base;
> +	unsigned int flag, max_count = 32;
> +	u16 status, ch;
> +
> +	while (max_count--) {
> +		status = readw(base + UART01x_FR);
> +		if (status & UART01x_FR_RXFE)
> +			break;
> +
> +		/* Take chars from the FIFO and update status */
> +		ch = readw(base + UART01x_DR);
> +		flag = TTY_NORMAL;
> +
> +		if (ch & UART011_DR_BE)
> +			flag = TTY_BREAK;
> +		else if (ch & UART011_DR_PE)
> +			flag = TTY_PARITY;
> +		else if (ch & UART011_DR_FE)
> +			flag = TTY_FRAME;
> +		else if (ch & UART011_DR_OE)
> +			flag = TTY_OVERRUN;
> +
> +		ch &= SBSAUART_CHAR_MASK;
> +
> +		tty_insert_flip_char(&qtty->port, ch, flag);

If its a console you ought to support the sysrq interfaces.

> +static int sbsa_tty_write_room(struct tty_struct *tty)
> +{
> +	return 32;
> +}

You can't do this. You need a proper queue and queueing mechanism or
you'll break in some situations (aside from sitting spinning in your
write code trashing your system performance entirely). We have a kfifo
object in the kernel which is really trivial to use and should do what
you need without any effort.

> +
> +static void sbsa_tty_console_write(struct console *co, const char *b,
> +								unsigned count)
> +{
> +	sbsa_tty_do_write(b, count);
> +
> +	if(b[count - 1] == '\n');
> +		sbsa_tty_do_write("\r", 1);

I would expect \r\n to be the order ?

> +static struct tty_port_operations sbsa_port_ops = {
> +};

No power management ?

> +
> +static const struct tty_operations sbsa_tty_ops = {
> +	.open = sbsa_tty_open,
> +	.close = sbsa_tty_close,
> +	.hangup = sbsa_tty_hangup,
> +	.write = sbsa_tty_write,
> +	.write_room = sbsa_tty_write_room,

No termios handling ?

> +static int sbsa_tty_probe(struct platform_device *pdev)
> +{
> +	struct sbsa_tty *qtty;
> +	int ret = -EINVAL;
> +	int i;
> +	struct resource *r;
> +	struct device *ttydev;
> +	void __iomem *base;
> +	u32 irq;
> +
> +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (r == NULL)
> +		return -EINVAL;
> +
> +	base = ioremap(r->start, r->end - r->start);
> +	if (base == NULL)
> +		pr_err("sbsa_tty: unable to remap base\n");

So you are then going to continue and randomly crash  ???

Also you've got a device so use dev_err() and friends on it

> +	if (pdev->id > 0)
> +		goto err_unmap;

Why not test this before you do all the mapping ??


It's clean code, it's easy to understand it just doesn't seem to be very
complete ?

Alan

Re: [PATCH 4/4] [RFC PATCH for Juno 2/2] tty: SBSA compatible UART

[Graeme Gregory]

On Wed, Sep 17, 2014 at 11:40:29AM +0100, One Thousand Gnomes wrote:
> 
> Firstly provide some useful information about the hardware. It's no good
> wavng your arms at a document that requires agreeing to a giant ARM T&C
> to get access to. Most of don't work for ARM and we'd have to get our own
> corporate legal to approve the legal garbage involved.
> 
> Secondly explain why you can't use PL011 given that it already supports
> non DMA accesses. What would it take to tweak it further for this ?
> 
> 

As the original author of this driver it is only meant for internal use
inside Linaro. It only ever reached the level of "good enough" for
internal testing.

There is a discussion on a more complete driver in this thread.

http://www.spinics.net/lists/arm-kernel/msg358083.html

Graeme

> > +static void sbsa_tty_do_write(const char *buf, unsigned count)
> > +{
> > +	unsigned long irq_flags;
> > +	struct sbsa_tty *qtty = sbsa_tty;
> > +	void __iomem *base = qtty->base;
> > +	unsigned n;
> > +
> > +	spin_lock_irqsave(&qtty->lock, irq_flags);
> > +	for (n = 0; n < count; n++) {
> > +		while (readw(base + UART01x_FR) & UART01x_FR_TXFF)
> > +			mdelay(10);
> > +		writew(buf[n], base + UART01x_DR);
> 
> serious - you are going to sit and spin in kernel space with interrupts
> off for an indefinite period ?
> 
> No. Even if your hardware is so completely brain dead and broken that it
> hasn't got an interrupt for 'write room' that's not acceptable. You need
> error handling and some kind of sensible timer based solution.
> 
> To put it simply. You have a queue (or you should - your driver is broken
> in that respect too), you have a baud rate, you have a bit time. From
> that you can compute sensible wakeup points to try and refill the
> hardware FIFO. Assuming the hardware fifo is not tiny you don't even have
> to be that good an aim.
> 
> It's acceptable for the printk console code to spin, if you think getting
> the message out on a failure or error outweighs the pain. It's not
> acceptable for the tty layer.
> 
> > +static void sbsauart_fifo_to_tty(struct sbsa_tty *qtty)
> > +{
> > +	void __iomem *base = qtty->base;
> > +	unsigned int flag, max_count = 32;
> > +	u16 status, ch;
> > +
> > +	while (max_count--) {
> > +		status = readw(base + UART01x_FR);
> > +		if (status & UART01x_FR_RXFE)
> > +			break;
> > +
> > +		/* Take chars from the FIFO and update status */
> > +		ch = readw(base + UART01x_DR);
> > +		flag = TTY_NORMAL;
> > +
> > +		if (ch & UART011_DR_BE)
> > +			flag = TTY_BREAK;
> > +		else if (ch & UART011_DR_PE)
> > +			flag = TTY_PARITY;
> > +		else if (ch & UART011_DR_FE)
> > +			flag = TTY_FRAME;
> > +		else if (ch & UART011_DR_OE)
> > +			flag = TTY_OVERRUN;
> > +
> > +		ch &= SBSAUART_CHAR_MASK;
> > +
> > +		tty_insert_flip_char(&qtty->port, ch, flag);
> 
> If its a console you ought to support the sysrq interfaces.
> 
> > +static int sbsa_tty_write_room(struct tty_struct *tty)
> > +{
> > +	return 32;
> > +}
> 
> You can't do this. You need a proper queue and queueing mechanism or
> you'll break in some situations (aside from sitting spinning in your
> write code trashing your system performance entirely). We have a kfifo
> object in the kernel which is really trivial to use and should do what
> you need without any effort.
> 
> > +
> > +static void sbsa_tty_console_write(struct console *co, const char *b,
> > +								unsigned count)
> > +{
> > +	sbsa_tty_do_write(b, count);
> > +
> > +	if(b[count - 1] == '\n');
> > +		sbsa_tty_do_write("\r", 1);
> 
> I would expect \r\n to be the order ?
> 
> > +static struct tty_port_operations sbsa_port_ops = {
> > +};
> 
> No power management ?
> 
> > +
> > +static const struct tty_operations sbsa_tty_ops = {
> > +	.open = sbsa_tty_open,
> > +	.close = sbsa_tty_close,
> > +	.hangup = sbsa_tty_hangup,
> > +	.write = sbsa_tty_write,
> > +	.write_room = sbsa_tty_write_room,
> 
> No termios handling ?
> 
> > +static int sbsa_tty_probe(struct platform_device *pdev)
> > +{
> > +	struct sbsa_tty *qtty;
> > +	int ret = -EINVAL;
> > +	int i;
> > +	struct resource *r;
> > +	struct device *ttydev;
> > +	void __iomem *base;
> > +	u32 irq;
> > +
> > +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	if (r == NULL)
> > +		return -EINVAL;
> > +
> > +	base = ioremap(r->start, r->end - r->start);
> > +	if (base == NULL)
> > +		pr_err("sbsa_tty: unable to remap base\n");
> 
> So you are then going to continue and randomly crash  ???
> 
> Also you've got a device so use dev_err() and friends on it
> 
> > +	if (pdev->id > 0)
> > +		goto err_unmap;
> 
> Why not test this before you do all the mapping ??
> 
> 
> It's clean code, it's easy to understand it just doesn't seem to be very
> complete ?
> 
> Alan

Re: [PATCH 1/4] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller

[Suravee Suthikulanit]

On 9/16/2014 8:26 PM, Matthew Garrett wrote:
> On Mon, Sep 15, 2014 at 07:47:23PM -0500, suravee.suthikulpanit@amd.com wrote:
>> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>>
>> This patch adds ACPI match table in ahci_platform. The table includes
>> the acpi_device_id to match AMD Seattle SATA controller with following
>> asl structure in DSDT:
>>
>>      Device (SATA0)
>>      {
>>        Name(_HID, "AMDI0600")	// Seattle AHSATA
>
> There really ought to be a well-defined PNPID for AHCI, so you can _HID
> to AMD and _CID to something generic. That way we won't have:
>
>> +#ifdef CONFIG_ATA_ACPI
>> +static const struct acpi_device_id ahci_acpi_match[] = {
>> +	{ "AMDI0600", 0 }, /* AMD Seattle AHCI */
>> +	{ },
>> +};
>
> utter sadness here. Really, please don't end up in a situation where we
> need to add device-specific IDs to a generic driver.
>
Matthew,

Currently, there is no _CID defined for generic AHCI. We will work on 
proposing one, and provide update patches for including the new ID.

Thanks,
Suravee

Re: [PATCH 1/4] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller

[Arnd Bergmann]

On Wednesday 01 October 2014 16:19:37 Suravee Suthikulanit wrote:
> On 9/16/2014 8:26 PM, Matthew Garrett wrote:
> > On Mon, Sep 15, 2014 at 07:47:23PM -0500, suravee.suthikulpanit@amd.com wrote:
> >> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> >>
> >> This patch adds ACPI match table in ahci_platform. The table includes
> >> the acpi_device_id to match AMD Seattle SATA controller with following
> >> asl structure in DSDT:
> >>
> >>      Device (SATA0)
> >>      {
> >>        Name(_HID, "AMDI0600")        // Seattle AHSATA
> >
> > There really ought to be a well-defined PNPID for AHCI, so you can _HID
> > to AMD and _CID to something generic. That way we won't have:
> >
> >> +#ifdef CONFIG_ATA_ACPI
> >> +static const struct acpi_device_id ahci_acpi_match[] = {
> >> +    { "AMDI0600", 0 }, /* AMD Seattle AHCI */
> >> +    { },
> >> +};
> >
> > utter sadness here. Really, please don't end up in a situation where we
> > need to add device-specific IDs to a generic driver.
> >
> Matthew,
> 
> Currently, there is no _CID defined for generic AHCI. We will work on 
> proposing one, and provide update patches for including the new ID.
> 

I think part of the problem is that there is no specification for what
an AHCI device should look like when it's not connected to a PCI
bus, the AHCI document published by Intel just states:

"AHCI is a PCI class device that acts as a data movement engine
between system memory and Serial ATA devices."

It also requires the PCI config space to have the PM capability
registers and (optionally) the MSI capability registers.
There are lots of chips we support in Linux with the ahci-platform
driver, but they are not actually compliant because they cannot
use the pmcap registers but instead typically rely on setting
external clock/phy/regulator/pinctrl registers.

The ARM SBSA document just requires any SATA controller to be AHCI
compliant but does not explain what that means in the case where
it's not a PCI device.

	Arnd

Re: [PATCH 1/4] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller

[Matthew Garrett]

On Thu, Oct 02, 2014 at 10:39:18AM +0200, Arnd Bergmann wrote:

> I think part of the problem is that there is no specification for what
> an AHCI device should look like when it's not connected to a PCI
> bus, the AHCI document published by Intel just states:

One thing that has come out of further discussion on this - ACPI defines 
a _CLS object, which allows ACPI devices to declare compatibility with a 
PCI class device. We don't have support for it at the moment, but it 
would be easy enough to add this to the kernel and then just expose the 
AHCI PCI class via _CLS. That would avoid us having problems with people 
doing similar things with USB hosts.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 1/4] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller

[Arnd Bergmann]

On Monday 06 October 2014 17:31:47 Matthew Garrett wrote:
> On Thu, Oct 02, 2014 at 10:39:18AM +0200, Arnd Bergmann wrote:
> 
> > I think part of the problem is that there is no specification for what
> > an AHCI device should look like when it's not connected to a PCI
> > bus, the AHCI document published by Intel just states:
> 
> One thing that has come out of further discussion on this - ACPI defines 
> a _CLS object, which allows ACPI devices to declare compatibility with a 
> PCI class device. We don't have support for it at the moment, but it 
> would be easy enough to add this to the kernel and then just expose the 
> AHCI PCI class via _CLS. That would avoid us having problems with people 
> doing similar things with USB hosts.

Interesting. Does this also define a way to get access to registers
that are normally in PCI config space, provided they are accessible at
all?

I'm guessing that it's not extremely important, given that so far all
platforms are doing ok without the power management registers or MSI,
but I guess it would be nice if we could support those.

	Arnd

Re: [PATCH 1/4] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller

[Matthew Garrett]

On Mon, Oct 06, 2014 at 08:19:37PM +0200, Arnd Bergmann wrote:

> Interesting. Does this also define a way to get access to registers
> that are normally in PCI config space, provided they are accessible at
> all?

Unfortunately not. I'd assume that PM registers are expected to be 
accessed via the _PS* methods instead. Does MSI make sense outside the 
context of PCI interrupts?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 1/4] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller

[Arnd Bergmann]

On Monday 06 October 2014 19:21:53 Matthew Garrett wrote:
> On Mon, Oct 06, 2014 at 08:19:37PM +0200, Arnd Bergmann wrote:
> 
> > Interesting. Does this also define a way to get access to registers
> > that are normally in PCI config space, provided they are accessible at
> > all?
> 
> Unfortunately not. I'd assume that PM registers are expected to be 
> accessed via the _PS* methods instead. Does MSI make sense outside the 
> context of PCI interrupts?

Yes, the ARM GIC has a weird sense of what MSI is used for, and
apparently some SoC vendors have started using MSI by default for
all on-chip peripherals.

A patch series to extend MSI to platform devices is currently
under review.

	Arnd

Re: [PATCH 1/4] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller

[Matthew Garrett]

On Mon, Oct 06, 2014 at 08:44:35PM +0200, Arnd Bergmann wrote:
> On Monday 06 October 2014 19:21:53 Matthew Garrett wrote:
> > Unfortunately not. I'd assume that PM registers are expected to be 
> > accessed via the _PS* methods instead. Does MSI make sense outside the 
> > context of PCI interrupts?
> 
> Yes, the ARM GIC has a weird sense of what MSI is used for, and
> apparently some SoC vendors have started using MSI by default for
> all on-chip peripherals.
> 
> A patch series to extend MSI to platform devices is currently
> under review.

Mm. Yeah, it doesn't seem like there's any ACPI-defined mechanism for 
MSI control. Let's chat about this next week?

-- 
Matthew Garrett | mjg59@srcf.ucam.org