Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1345756 > unrolled thread
| Started by | Aleksey Makarov <aleksey.makarov@linaro.org> |
|---|---|
| First post | 2016-02-29 13:50 +0100 |
| Last post | 2016-03-04 18:50 +0100 |
| Articles | 20 on this page of 31 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH v3 0/7] ACPI: parse the DBG2 table Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-02-29 13:50 +0100
[PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-02-29 13:50 +0100
Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port Peter Hurley <peter@hurleysoftware.com> - 2016-03-01 17:00 +0100
Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-03-01 18:00 +0100
Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port Peter Hurley <peter@hurleysoftware.com> - 2016-03-03 18:50 +0100
Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port Peter Hurley <peter@hurleysoftware.com> - 2016-03-03 20:40 +0100
Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-03-04 14:10 +0100
Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port Peter Hurley <peter@hurleysoftware.com> - 2016-03-04 16:50 +0100
Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port Peter Hurley <peter@hurleysoftware.com> - 2016-03-04 17:00 +0100
[PATCH v3 1/7] of/serial: move earlycon early_param handling to serial Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-02-29 13:50 +0100
Re: [PATCH v3 1/7] of/serial: move earlycon early_param handling to serial Peter Hurley <peter@hurleysoftware.com> - 2016-03-01 16:00 +0100
Re: [PATCH v3 1/7] of/serial: move earlycon early_param handling to serial Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-03-01 17:40 +0100
Re: [PATCH v3 1/7] of/serial: move earlycon early_param handling to serial Peter Hurley <peter@hurleysoftware.com> - 2016-03-01 18:30 +0100
Re: [PATCH v3 1/7] of/serial: move earlycon early_param handling to serial Aleksey Makarov <amakarov.linux@gmail.com> - 2016-03-01 19:00 +0100
Re: [PATCH v3 1/7] of/serial: move earlycon early_param handling to serial Peter Hurley <peter@hurleysoftware.com> - 2016-03-01 19:30 +0100
[PATCH v3 7/7] serial: pl011: add ACPI DBG2 serial port Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-02-29 13:50 +0100
[PATCH v3 6/7] ACPI: enable ACPI_DBG2_TABLE on ARM64 Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-02-29 13:50 +0100
Re: [PATCH v3 6/7] ACPI: enable ACPI_DBG2_TABLE on ARM64 Peter Hurley <peter@hurleysoftware.com> - 2016-03-01 16:00 +0100
Re: [PATCH v3 6/7] ACPI: enable ACPI_DBG2_TABLE on ARM64 Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-03-01 18:10 +0100
Re: [PATCH v3 6/7] ACPI: enable ACPI_DBG2_TABLE on ARM64 Peter Hurley <peter@hurleysoftware.com> - 2016-03-01 18:30 +0100
Re: [PATCH v3 6/7] ACPI: enable ACPI_DBG2_TABLE on ARM64 Aleksey Makarov <amakarov.linux@gmail.com> - 2016-03-03 12:50 +0100
Re: [PATCH v3 6/7] ACPI: enable ACPI_DBG2_TABLE on ARM64 Peter Hurley <peter@hurleysoftware.com> - 2016-03-03 17:00 +0100
[PATCH v3 2/7] ACPI: add definitions of DBG2 subtypes Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-02-29 13:50 +0100
[PATCH v3 4/7] ACPI: parse DBG2 table Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-02-29 13:50 +0100
Re: [PATCH v3 4/7] ACPI: parse DBG2 table Peter Hurley <peter@hurleysoftware.com> - 2016-03-01 15:50 +0100
Re: [PATCH v3 4/7] ACPI: parse DBG2 table Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-03-01 17:30 +0100
Re: [PATCH v3 4/7] ACPI: parse DBG2 table Peter Hurley <peter@hurleysoftware.com> - 2016-03-01 18:30 +0100
Re: [PATCH v3 4/7] ACPI: parse DBG2 table Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-03-01 19:30 +0100
Re: [PATCH v3 4/7] ACPI: parse DBG2 table Peter Hurley <peter@hurleysoftware.com> - 2016-03-03 17:50 +0100
Re: [PATCH v3 4/7] ACPI: parse DBG2 table Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-03-04 13:30 +0100
Re: [PATCH v3 4/7] ACPI: parse DBG2 table Peter Hurley <peter@hurleysoftware.com> - 2016-03-04 18:50 +0100
Page 1 of 2 [1] 2 Next page →
| From | Aleksey Makarov <aleksey.makarov@linaro.org> |
|---|---|
| Date | 2016-02-29 13:50 +0100 |
| Subject | [PATCH v3 0/7] ACPI: parse the DBG2 table |
| Message-ID | <r7vqV-345-5@gated-at.bofh.it> |
"ARM Server Base Boot Requirements" [1] mentions DBG2 (Microsoft Debug Port Table 2) [2] as a mandatory ACPI table that specifies debug ports. - Move earlycon early_param handling to serial to parse earlycon option once - Add definition of DBG2 subtypes. It's the same patch as in the SPCR series [3] - Refactor ACPI linker tables code to enable iterating over subtables other than subtables having acpi_subtype header (such as DBG2) - Implement macros ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) that defines a handler for the port of the given type and subtype. - For each port defined by that macros that is also described in the ACPI DBG2 table call provided callback. - Implement a helper macros that can be used to define early serial console - Enable DBG2 on ARM64 - Define early console for pl011 serial Based on the work by Leif Lindholm <leif.lindholm@linaro.org> [4] Should be applied to next-20160229. Tested on last QEMU with patches that introduce DBG2 and fix a QEMU bug: https://git.linaro.org/people/aleksey.makarov/qemu.git amakarov/gdb2.03 v3: - Drop "arm64: move acpi/dt decision earlier in boot process". It is incorrect. (Matthias Brugger) - Fix the message at port discovery. v2: https://lkml.kernel.org/g/1456333819-13482-1-git-send-email-aleksey.makarov@linaro.org - Fix parse_acpi() (Matthias Brugger) - Add a reference to QEMU DBG2 patches and to original work - Rebase to the last linux-next v1: https://lkml.kernel.org/g/1456148818-26257-1-git-send-email-aleksey.makarov@linaro.org [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html [2] http://go.microsoft.com/fwlink/p/?LinkId=234837 [3] https://lkml.kernel.org/g/1455559532-8305-1-git-send-email-aleksey.makarov@linaro.org [4] https://lkml.kernel.org/g/1441716217-23786-1-git-send-email-leif.lindholm@linaro.org Aleksey Makarov (6): ACPI: add definitions of DBG2 subtypes ACPI: genaralize iterating over subtables in ACPI_PROBE_TABLE() ACPI: parse DBG2 table ACPI: serial: implement earlycon on ACPI DBG2 port ACPI: enable ACPI_DBG2_TABLE on ARM64 serial: pl011: add ACPI DBG2 serial port Leif Lindholm (1): of/serial: move earlycon early_param handling to serial Documentation/kernel-parameters.txt | 3 + arch/arm64/Kconfig | 1 + arch/arm64/kernel/acpi.c | 2 + drivers/acpi/Kconfig | 3 + drivers/acpi/Makefile | 1 + drivers/acpi/dbg2.c | 88 +++++++++++++++++++++++++++++ drivers/acpi/scan.c | 44 ++++++++++----- drivers/clocksource/arm_arch_timer.c | 3 +- drivers/irqchip/irq-gic.c | 4 +- drivers/of/fdt.c | 11 +--- drivers/tty/serial/amba-pl011.c | 3 + drivers/tty/serial/earlycon.c | 61 ++++++++++++++++++++ include/acpi/actbl2.h | 5 ++ include/asm-generic/vmlinux.lds.h | 1 + include/linux/acpi.h | 104 ++++++++++++++++++++++++----------- include/linux/acpi_dbg2.h | 68 +++++++++++++++++++++++ include/linux/clocksource.h | 2 +- include/linux/irqchip.h | 5 +- include/linux/of_fdt.h | 2 + 19 files changed, 348 insertions(+), 63 deletions(-) create mode 100644 drivers/acpi/dbg2.c create mode 100644 include/linux/acpi_dbg2.h -- 2.7.1
[toc] | [next] | [standalone]
| From | Aleksey Makarov <aleksey.makarov@linaro.org> |
|---|---|
| Date | 2016-02-29 13:50 +0100 |
| Subject | [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port |
| Message-ID | <r7vqV-345-9@gated-at.bofh.it> |
| In reply to | #1345756 |
Add ACPI_DBG2_EARLYCON_DECLARE() macros that declares
an earlycon on the serial port specified in the DBG2 ACPI table.
Pass the string "earlycon=acpi_dbg2" to the kernel to activate it.
Callbacks for EARLYCON_DECLARE() and OF_EARLYCON_DECLARE()
can also be used for this macros.
Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
---
Documentation/kernel-parameters.txt | 3 ++
drivers/tty/serial/earlycon.c | 60 +++++++++++++++++++++++++++++++++++++
include/linux/acpi_dbg2.h | 20 +++++++++++++
3 files changed, 83 insertions(+)
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index e0a21e4..19b947b 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1072,6 +1072,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
A valid base address must be provided, and the serial
port must already be setup and configured.
+ acpi_dbg2
+ Use serial port specified by the DBG2 ACPI table.
+
earlyprintk= [X86,SH,BLACKFIN,ARM,M68k]
earlyprintk=vga
earlyprintk=efi
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index d217366..9ba3a04 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -22,6 +22,7 @@
#include <linux/sizes.h>
#include <linux/of.h>
#include <linux/of_fdt.h>
+#include <linux/acpi.h>
#ifdef CONFIG_FIX_EARLYCON_MEM
#include <asm/fixmap.h>
@@ -200,6 +201,8 @@ int __init setup_earlycon(char *buf)
return -ENOENT;
}
+static bool setup_dbg2_earlycon;
+
/* early_param wrapper for setup_earlycon() */
static int __init param_setup_earlycon(char *buf)
{
@@ -212,6 +215,11 @@ static int __init param_setup_earlycon(char *buf)
if (!buf || !buf[0])
return early_init_dt_scan_chosen_serial();
+ if (!strcmp(buf, "acpi_dbg2")) {
+ setup_dbg2_earlycon = true;
+ return 0;
+ }
+
err = setup_earlycon(buf);
if (err == -ENOENT || err == -EALREADY)
return 0;
@@ -286,3 +294,55 @@ int __init of_setup_earlycon(const struct earlycon_id *match,
}
#endif /* CONFIG_OF_EARLY_FLATTREE */
+
+#ifdef CONFIG_ACPI_DBG2_TABLE
+
+int __init acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d)
+{
+ int err;
+ struct uart_port *port = &early_console_dev.port;
+ int (*setup)(struct earlycon_device *, const char *) = d;
+ struct acpi_generic_address *reg;
+
+ if (!setup_dbg2_earlycon)
+ return -ENODEV;
+
+ if (device->register_count < 1)
+ return -ENODEV;
+
+ if (device->base_address_offset >= device->length)
+ return -EINVAL;
+
+ reg = (void *)device + device->base_address_offset;
+
+ if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY &&
+ reg->space_id != ACPI_ADR_SPACE_SYSTEM_IO)
+ return -EINVAL;
+
+ spin_lock_init(&port->lock);
+ port->uartclk = BASE_BAUD * 16;
+
+ if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
+ if (device->port_type == ACPI_DBG2_ARM_SBSA_32BIT)
+ port->iotype = UPIO_MEM32;
+ else
+ port->iotype = UPIO_MEM;
+ port->mapbase = reg->address;
+ port->membase = earlycon_map(reg->address, SZ_4K);
+ } else {
+ port->iotype = UPIO_PORT;
+ port->iobase = reg->address;
+ }
+
+ early_console_dev.con->data = &early_console_dev;
+ err = setup(&early_console_dev, NULL);
+ if (err < 0)
+ return err;
+ if (!early_console_dev.con->write)
+ return -ENODEV;
+
+ register_console(early_console_dev.con);
+ return 0;
+}
+
+#endif /* CONFIG_ACPI_DBG2_TABLE */
diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
index 125ae7e..b653752 100644
--- a/include/linux/acpi_dbg2.h
+++ b/include/linux/acpi_dbg2.h
@@ -37,12 +37,32 @@ int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
acpi_dbg2_setup, &__acpi_dbg2_data_##name)
+int acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d);
+
+/**
+ * ACPI_DBG2_EARLYCON_DECLARE() - Define handler for ACPI GDB2 serial port
+ * @name: Identifier to compose name of table data
+ * @subtype: Subtype of the port
+ * @console_setup: Function to be called to setup the port
+ *
+ * Type of the console_setup() callback is
+ * int (*setup)(struct earlycon_device *, const char *)
+ * It's the type of callback of of_setup_earlycon().
+ */
+#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
+ ACPI_DBG2_DECLARE(name, ACPI_DBG2_SERIAL_PORT, subtype, \
+ acpi_setup_earlycon, console_setup)
+
#else
#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
static const void *__acpi_dbg_data_##name[] \
__used __initdata = { (void *)setup_fn, (void *)data_ptr }
+#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
+ static const void *__acpi_dbg_data_serial_##name[] \
+ __used __initdata = { (void *)console_setup }
+
#endif
#endif
--
2.7.1
[toc] | [prev] | [next] | [standalone]
| From | Peter Hurley <peter@hurleysoftware.com> |
|---|---|
| Date | 2016-03-01 17:00 +0100 |
| Subject | Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port |
| Message-ID | <r7USm-2ET-25@gated-at.bofh.it> |
| In reply to | #1345758 |
On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
> Add ACPI_DBG2_EARLYCON_DECLARE() macros that declares
> an earlycon on the serial port specified in the DBG2 ACPI table.
>
> Pass the string "earlycon=acpi_dbg2" to the kernel to activate it.
>
> Callbacks for EARLYCON_DECLARE() and OF_EARLYCON_DECLARE()
> can also be used for this macros.
>
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> ---
> Documentation/kernel-parameters.txt | 3 ++
> drivers/tty/serial/earlycon.c | 60 +++++++++++++++++++++++++++++++++++++
> include/linux/acpi_dbg2.h | 20 +++++++++++++
> 3 files changed, 83 insertions(+)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index e0a21e4..19b947b 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1072,6 +1072,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> A valid base address must be provided, and the serial
> port must already be setup and configured.
>
> + acpi_dbg2
> + Use serial port specified by the DBG2 ACPI table.
> +
> earlyprintk= [X86,SH,BLACKFIN,ARM,M68k]
> earlyprintk=vga
> earlyprintk=efi
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index d217366..9ba3a04 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -22,6 +22,7 @@
> #include <linux/sizes.h>
> #include <linux/of.h>
> #include <linux/of_fdt.h>
> +#include <linux/acpi.h>
>
> #ifdef CONFIG_FIX_EARLYCON_MEM
> #include <asm/fixmap.h>
> @@ -200,6 +201,8 @@ int __init setup_earlycon(char *buf)
> return -ENOENT;
> }
>
> +static bool setup_dbg2_earlycon;
> +
> /* early_param wrapper for setup_earlycon() */
> static int __init param_setup_earlycon(char *buf)
> {
> @@ -212,6 +215,11 @@ static int __init param_setup_earlycon(char *buf)
> if (!buf || !buf[0])
> return early_init_dt_scan_chosen_serial();
>
> + if (!strcmp(buf, "acpi_dbg2")) {
> + setup_dbg2_earlycon = true;
> + return 0;
> + }
So this series doesn't start an ACPI earlycon at early_param time?
That doesn't seem very useful.
When does the ACPI earlycon actually start?
And don't say "when the DBG2 table is probed"; that much is obvious.
> +
> err = setup_earlycon(buf);
> if (err == -ENOENT || err == -EALREADY)
> return 0;
> @@ -286,3 +294,55 @@ int __init of_setup_earlycon(const struct earlycon_id *match,
> }
>
> #endif /* CONFIG_OF_EARLY_FLATTREE */
> +
> +#ifdef CONFIG_ACPI_DBG2_TABLE
> +
> +int __init acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d)
> +{
> + int err;
> + struct uart_port *port = &early_console_dev.port;
> + int (*setup)(struct earlycon_device *, const char *) = d;
> + struct acpi_generic_address *reg;
> +
> + if (!setup_dbg2_earlycon)
> + return -ENODEV;
> +
> + if (device->register_count < 1)
> + return -ENODEV;
> +
> + if (device->base_address_offset >= device->length)
> + return -EINVAL;
> +
> + reg = (void *)device + device->base_address_offset;
> +
> + if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY &&
> + reg->space_id != ACPI_ADR_SPACE_SYSTEM_IO)
> + return -EINVAL;
> +
> + spin_lock_init(&port->lock);
> + port->uartclk = BASE_BAUD * 16;
> +
> + if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> + if (device->port_type == ACPI_DBG2_ARM_SBSA_32BIT)
> + port->iotype = UPIO_MEM32;
> + else
> + port->iotype = UPIO_MEM;
> + port->mapbase = reg->address;
> + port->membase = earlycon_map(reg->address, SZ_4K);
> + } else {
> + port->iotype = UPIO_PORT;
> + port->iobase = reg->address;
> + }
> +
> + early_console_dev.con->data = &early_console_dev;
> + err = setup(&early_console_dev, NULL);
> + if (err < 0)
> + return err;
> + if (!early_console_dev.con->write)
> + return -ENODEV;
> +
> + register_console(early_console_dev.con);
> + return 0;
> +}
> +
> +#endif /* CONFIG_ACPI_DBG2_TABLE */
> diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
> index 125ae7e..b653752 100644
> --- a/include/linux/acpi_dbg2.h
> +++ b/include/linux/acpi_dbg2.h
> @@ -37,12 +37,32 @@ int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
> ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
> acpi_dbg2_setup, &__acpi_dbg2_data_##name)
>
> +int acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d);
> +
> +/**
> + * ACPI_DBG2_EARLYCON_DECLARE() - Define handler for ACPI GDB2 serial port
> + * @name: Identifier to compose name of table data
> + * @subtype: Subtype of the port
> + * @console_setup: Function to be called to setup the port
> + *
> + * Type of the console_setup() callback is
> + * int (*setup)(struct earlycon_device *, const char *)
> + * It's the type of callback of of_setup_earlycon().
> + */
> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
> + ACPI_DBG2_DECLARE(name, ACPI_DBG2_SERIAL_PORT, subtype, \
> + acpi_setup_earlycon, console_setup)
> +
> #else
>
> #define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
> static const void *__acpi_dbg_data_##name[] \
> __used __initdata = { (void *)setup_fn, (void *)data_ptr }
>
> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
> + static const void *__acpi_dbg_data_serial_##name[] \
> + __used __initdata = { (void *)console_setup }
> +
> #endif
>
> #endif
>
[toc] | [prev] | [next] | [standalone]
| From | Aleksey Makarov <aleksey.makarov@linaro.org> |
|---|---|
| Date | 2016-03-01 18:00 +0100 |
| Subject | Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port |
| Message-ID | <r7VOr-3gD-23@gated-at.bofh.it> |
| In reply to | #1346714 |
On 03/01/2016 06:53 PM, Peter Hurley wrote:
> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>> Add ACPI_DBG2_EARLYCON_DECLARE() macros that declares
>> an earlycon on the serial port specified in the DBG2 ACPI table.
>>
>> Pass the string "earlycon=acpi_dbg2" to the kernel to activate it.
>>
>> Callbacks for EARLYCON_DECLARE() and OF_EARLYCON_DECLARE()
>> can also be used for this macros.
>>
>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>> ---
>> Documentation/kernel-parameters.txt | 3 ++
>> drivers/tty/serial/earlycon.c | 60 +++++++++++++++++++++++++++++++++++++
>> include/linux/acpi_dbg2.h | 20 +++++++++++++
>> 3 files changed, 83 insertions(+)
>>
>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>> index e0a21e4..19b947b 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -1072,6 +1072,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>> A valid base address must be provided, and the serial
>> port must already be setup and configured.
>>
>> + acpi_dbg2
>> + Use serial port specified by the DBG2 ACPI table.
>> +
>> earlyprintk= [X86,SH,BLACKFIN,ARM,M68k]
>> earlyprintk=vga
>> earlyprintk=efi
>> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
>> index d217366..9ba3a04 100644
>> --- a/drivers/tty/serial/earlycon.c
>> +++ b/drivers/tty/serial/earlycon.c
>> @@ -22,6 +22,7 @@
>> #include <linux/sizes.h>
>> #include <linux/of.h>
>> #include <linux/of_fdt.h>
>> +#include <linux/acpi.h>
>>
>> #ifdef CONFIG_FIX_EARLYCON_MEM
>> #include <asm/fixmap.h>
>> @@ -200,6 +201,8 @@ int __init setup_earlycon(char *buf)
>> return -ENOENT;
>> }
>>
>> +static bool setup_dbg2_earlycon;
>> +
>> /* early_param wrapper for setup_earlycon() */
>> static int __init param_setup_earlycon(char *buf)
>> {
>> @@ -212,6 +215,11 @@ static int __init param_setup_earlycon(char *buf)
>> if (!buf || !buf[0])
>> return early_init_dt_scan_chosen_serial();
>>
>> + if (!strcmp(buf, "acpi_dbg2")) {
>> + setup_dbg2_earlycon = true;
>> + return 0;
>> + }
>
> So this series doesn't start an ACPI earlycon at early_param time?
> That doesn't seem very useful.
>
> When does the ACPI earlycon actually start?
> And don't say "when the DBG2 table is probed"; that much is obvious.
ACPI earlycon starts as soon as ACPI tables become accessible (setup_arch()).
I think that is still quite early.
>> +
>> err = setup_earlycon(buf);
>> if (err == -ENOENT || err == -EALREADY)
>> return 0;
>> @@ -286,3 +294,55 @@ int __init of_setup_earlycon(const struct earlycon_id *match,
>> }
>>
>> #endif /* CONFIG_OF_EARLY_FLATTREE */
>> +
>> +#ifdef CONFIG_ACPI_DBG2_TABLE
>> +
>> +int __init acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d)
>> +{
>> + int err;
>> + struct uart_port *port = &early_console_dev.port;
>> + int (*setup)(struct earlycon_device *, const char *) = d;
>> + struct acpi_generic_address *reg;
>> +
>> + if (!setup_dbg2_earlycon)
>> + return -ENODEV;
>> +
>> + if (device->register_count < 1)
>> + return -ENODEV;
>> +
>> + if (device->base_address_offset >= device->length)
>> + return -EINVAL;
>> +
>> + reg = (void *)device + device->base_address_offset;
>> +
>> + if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY &&
>> + reg->space_id != ACPI_ADR_SPACE_SYSTEM_IO)
>> + return -EINVAL;
>> +
>> + spin_lock_init(&port->lock);
>> + port->uartclk = BASE_BAUD * 16;
>> +
>> + if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
>> + if (device->port_type == ACPI_DBG2_ARM_SBSA_32BIT)
>> + port->iotype = UPIO_MEM32;
>> + else
>> + port->iotype = UPIO_MEM;
>> + port->mapbase = reg->address;
>> + port->membase = earlycon_map(reg->address, SZ_4K);
>> + } else {
>> + port->iotype = UPIO_PORT;
>> + port->iobase = reg->address;
>> + }
>> +
>> + early_console_dev.con->data = &early_console_dev;
>> + err = setup(&early_console_dev, NULL);
>> + if (err < 0)
>> + return err;
>> + if (!early_console_dev.con->write)
>> + return -ENODEV;
>> +
>> + register_console(early_console_dev.con);
>> + return 0;
>> +}
>> +
>> +#endif /* CONFIG_ACPI_DBG2_TABLE */
>> diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
>> index 125ae7e..b653752 100644
>> --- a/include/linux/acpi_dbg2.h
>> +++ b/include/linux/acpi_dbg2.h
>> @@ -37,12 +37,32 @@ int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
>> ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
>> acpi_dbg2_setup, &__acpi_dbg2_data_##name)
>>
>> +int acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d);
>> +
>> +/**
>> + * ACPI_DBG2_EARLYCON_DECLARE() - Define handler for ACPI GDB2 serial port
>> + * @name: Identifier to compose name of table data
>> + * @subtype: Subtype of the port
>> + * @console_setup: Function to be called to setup the port
>> + *
>> + * Type of the console_setup() callback is
>> + * int (*setup)(struct earlycon_device *, const char *)
>> + * It's the type of callback of of_setup_earlycon().
>> + */
>> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
>> + ACPI_DBG2_DECLARE(name, ACPI_DBG2_SERIAL_PORT, subtype, \
>> + acpi_setup_earlycon, console_setup)
>> +
>> #else
>>
>> #define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>> static const void *__acpi_dbg_data_##name[] \
>> __used __initdata = { (void *)setup_fn, (void *)data_ptr }
>>
>> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
>> + static const void *__acpi_dbg_data_serial_##name[] \
>> + __used __initdata = { (void *)console_setup }
>> +
>> #endif
>>
>> #endif
>>
>
[toc] | [prev] | [next] | [standalone]
| From | Peter Hurley <peter@hurleysoftware.com> |
|---|---|
| Date | 2016-03-03 18:50 +0100 |
| Subject | Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port |
| Message-ID | <r8FxV-29v-23@gated-at.bofh.it> |
| In reply to | #1346761 |
On 03/01/2016 08:57 AM, Aleksey Makarov wrote:
>
>
> On 03/01/2016 06:53 PM, Peter Hurley wrote:
>> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>>> Add ACPI_DBG2_EARLYCON_DECLARE() macros that declares
>>> an earlycon on the serial port specified in the DBG2 ACPI table.
>>>
>>> Pass the string "earlycon=acpi_dbg2" to the kernel to activate it.
>>>
>>> Callbacks for EARLYCON_DECLARE() and OF_EARLYCON_DECLARE()
>>> can also be used for this macros.
>>>
>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>> ---
>>> Documentation/kernel-parameters.txt | 3 ++
>>> drivers/tty/serial/earlycon.c | 60 +++++++++++++++++++++++++++++++++++++
>>> include/linux/acpi_dbg2.h | 20 +++++++++++++
>>> 3 files changed, 83 insertions(+)
>>>
>>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>>> index e0a21e4..19b947b 100644
>>> --- a/Documentation/kernel-parameters.txt
>>> +++ b/Documentation/kernel-parameters.txt
>>> @@ -1072,6 +1072,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>> A valid base address must be provided, and the serial
>>> port must already be setup and configured.
>>>
>>> + acpi_dbg2
>>> + Use serial port specified by the DBG2 ACPI table.
>>> +
>>> earlyprintk= [X86,SH,BLACKFIN,ARM,M68k]
>>> earlyprintk=vga
>>> earlyprintk=efi
>>> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
>>> index d217366..9ba3a04 100644
>>> --- a/drivers/tty/serial/earlycon.c
>>> +++ b/drivers/tty/serial/earlycon.c
>>> @@ -22,6 +22,7 @@
>>> #include <linux/sizes.h>
>>> #include <linux/of.h>
>>> #include <linux/of_fdt.h>
>>> +#include <linux/acpi.h>
>>>
>>> #ifdef CONFIG_FIX_EARLYCON_MEM
>>> #include <asm/fixmap.h>
>>> @@ -200,6 +201,8 @@ int __init setup_earlycon(char *buf)
>>> return -ENOENT;
>>> }
>>>
>>> +static bool setup_dbg2_earlycon;
>>> +
>>> /* early_param wrapper for setup_earlycon() */
>>> static int __init param_setup_earlycon(char *buf)
>>> {
>>> @@ -212,6 +215,11 @@ static int __init param_setup_earlycon(char *buf)
>>> if (!buf || !buf[0])
>>> return early_init_dt_scan_chosen_serial();
>>>
>>> + if (!strcmp(buf, "acpi_dbg2")) {
>>> + setup_dbg2_earlycon = true;
>>> + return 0;
>>> + }
>>
>> So this series doesn't start an ACPI earlycon at early_param time?
>> That doesn't seem very useful.
>>
>> When does the ACPI earlycon actually start?
>> And don't say "when the DBG2 table is probed"; that much is obvious.
>
> ACPI earlycon starts as soon as ACPI tables become accessible (setup_arch()).
> I think that is still quite early.
I see now; the probe is in patch 6/7.
setup_arch()
acpi_boot_table_init()
acpi_probe_device_table()
...
acpi_dbg2_setup()
->setup()
acpi_setup_earlycon()
>>> +
>>> err = setup_earlycon(buf);
>>> if (err == -ENOENT || err == -EALREADY)
>>> return 0;
>>> @@ -286,3 +294,55 @@ int __init of_setup_earlycon(const struct earlycon_id *match,
>>> }
>>>
>>> #endif /* CONFIG_OF_EARLY_FLATTREE */
>>> +
>>> +#ifdef CONFIG_ACPI_DBG2_TABLE
>>> +
>>> +int __init acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d)
>>> +{
>>> + int err;
>>> + struct uart_port *port = &early_console_dev.port;
>>> + int (*setup)(struct earlycon_device *, const char *) = d;
>>> + struct acpi_generic_address *reg;
>>> +
>>> + if (!setup_dbg2_earlycon)
>>> + return -ENODEV;
>>> +
>>> + if (device->register_count < 1)
>>> + return -ENODEV;
>>> +
>>> + if (device->base_address_offset >= device->length)
>>> + return -EINVAL;
>>> +
>>> + reg = (void *)device + device->base_address_offset;
>>> +
>>> + if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY &&
>>> + reg->space_id != ACPI_ADR_SPACE_SYSTEM_IO)
>>> + return -EINVAL;
>>> +
>>> + spin_lock_init(&port->lock);
>>> + port->uartclk = BASE_BAUD * 16;
>>> +
>>> + if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
>>> + if (device->port_type == ACPI_DBG2_ARM_SBSA_32BIT)
>>> + port->iotype = UPIO_MEM32;
>>> + else
>>> + port->iotype = UPIO_MEM;
>>> + port->mapbase = reg->address;
>>> + port->membase = earlycon_map(reg->address, SZ_4K);
>>> + } else {
>>> + port->iotype = UPIO_PORT;
>>> + port->iobase = reg->address;
>>> + }
>>> +
>>> + early_console_dev.con->data = &early_console_dev;
>>> + err = setup(&early_console_dev, NULL);
>>> + if (err < 0)
>>> + return err;
>>> + if (!early_console_dev.con->write)
>>> + return -ENODEV;
>>> +
>>> + register_console(early_console_dev.con);
>>> + return 0;
>>> +}
>>> +
>>> +#endif /* CONFIG_ACPI_DBG2_TABLE */
>>> diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
>>> index 125ae7e..b653752 100644
>>> --- a/include/linux/acpi_dbg2.h
>>> +++ b/include/linux/acpi_dbg2.h
>>> @@ -37,12 +37,32 @@ int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
>>> ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
>>> acpi_dbg2_setup, &__acpi_dbg2_data_##name)
>>>
>>> +int acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d);
>>> +
>>> +/**
>>> + * ACPI_DBG2_EARLYCON_DECLARE() - Define handler for ACPI GDB2 serial port
>>> + * @name: Identifier to compose name of table data
>>> + * @subtype: Subtype of the port
>>> + * @console_setup: Function to be called to setup the port
>>> + *
>>> + * Type of the console_setup() callback is
>>> + * int (*setup)(struct earlycon_device *, const char *)
>>> + * It's the type of callback of of_setup_earlycon().
>>> + */
>>> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
>>> + ACPI_DBG2_DECLARE(name, ACPI_DBG2_SERIAL_PORT, subtype, \
>>> + acpi_setup_earlycon, console_setup)
>>> +
>>> #else
>>>
>>> #define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>>> static const void *__acpi_dbg_data_##name[] \
>>> __used __initdata = { (void *)setup_fn, (void *)data_ptr }
>>>
>>> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
>>> + static const void *__acpi_dbg_data_serial_##name[] \
>>> + __used __initdata = { (void *)console_setup }
console_setup is a terrible macro argument name; console_setup() is an
actual kernel function (although file-scope).
Please change it to something short and generic.
Honestly, I'd just prefer you skip all this apparatus that makes
ACPI earlycon appear to be like OF earlycon code-wise, but without any of
the real underpinning or flexibility.
This would be trivial to parse the ACPI table and invoke
setup_earlycon() with a string specifier instead.
For example,
int __init acpi_earlycon_setup(struct acpi_dbg2_device *dbg2)
{
char opts[64];
struct acpi_generic_addr *addr = (void*)dbg2 + dbg2->base_address_offset;
int mmio = addr->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY;
if (dbg2->port_type != ACPI_DBG2_SERIAL_PORT)
return 0;
switch (dbg2->port_subtype) {
case ACPI_DBG2_ARM_PL011:
case ACPI_DBG2_ARM_SBSA_GENERIC:
case ACPI_DBG2_BCM2835:
sprintf(opts, "pl011,%s,0x%llx", mmio, addr->address);
break;
case ACPI_DBG2_ARM_SBSA_32BIT:
sprintf(opts, "pl011,mmio32,0x%llx", addr->address);
break;
case ACPI_DBG2_16550_COMPATIBLE:
case ACPI_DBG2_16550_SUBSET:
sprintf(opts, "uart,%s,0x%llx", mmio, addr->address);
break;
default:
return 0;
}
return setup_earlycon(opts);
}
This supports every earlycon ACPI DBG2 declares, not just the ARM_PL011
subtype of your series.
>>> +
>>> #endif
>>>
>>> #endif
>>>
>>
[toc] | [prev] | [next] | [standalone]
| From | Peter Hurley <peter@hurleysoftware.com> |
|---|---|
| Date | 2016-03-03 20:40 +0100 |
| Subject | Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port |
| Message-ID | <r8Hgn-3iI-23@gated-at.bofh.it> |
| In reply to | #1349451 |
On 03/03/2016 09:48 AM, Peter Hurley wrote:
> On 03/01/2016 08:57 AM, Aleksey Makarov wrote:
>>
>>
>> On 03/01/2016 06:53 PM, Peter Hurley wrote:
>>> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>>>> Add ACPI_DBG2_EARLYCON_DECLARE() macros that declares
>>>> an earlycon on the serial port specified in the DBG2 ACPI table.
>>>>
>>>> Pass the string "earlycon=acpi_dbg2" to the kernel to activate it.
>>>>
>>>> Callbacks for EARLYCON_DECLARE() and OF_EARLYCON_DECLARE()
>>>> can also be used for this macros.
>>>>
>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>>> ---
>>>> Documentation/kernel-parameters.txt | 3 ++
>>>> drivers/tty/serial/earlycon.c | 60 +++++++++++++++++++++++++++++++++++++
>>>> include/linux/acpi_dbg2.h | 20 +++++++++++++
>>>> 3 files changed, 83 insertions(+)
>>>>
>>>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>>>> index e0a21e4..19b947b 100644
>>>> --- a/Documentation/kernel-parameters.txt
>>>> +++ b/Documentation/kernel-parameters.txt
>>>> @@ -1072,6 +1072,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>>> A valid base address must be provided, and the serial
>>>> port must already be setup and configured.
>>>>
>>>> + acpi_dbg2
>>>> + Use serial port specified by the DBG2 ACPI table.
>>>> +
>>>> earlyprintk= [X86,SH,BLACKFIN,ARM,M68k]
>>>> earlyprintk=vga
>>>> earlyprintk=efi
>>>> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
>>>> index d217366..9ba3a04 100644
>>>> --- a/drivers/tty/serial/earlycon.c
>>>> +++ b/drivers/tty/serial/earlycon.c
>>>> @@ -22,6 +22,7 @@
>>>> #include <linux/sizes.h>
>>>> #include <linux/of.h>
>>>> #include <linux/of_fdt.h>
>>>> +#include <linux/acpi.h>
>>>>
>>>> #ifdef CONFIG_FIX_EARLYCON_MEM
>>>> #include <asm/fixmap.h>
>>>> @@ -200,6 +201,8 @@ int __init setup_earlycon(char *buf)
>>>> return -ENOENT;
>>>> }
>>>>
>>>> +static bool setup_dbg2_earlycon;
>>>> +
>>>> /* early_param wrapper for setup_earlycon() */
>>>> static int __init param_setup_earlycon(char *buf)
>>>> {
>>>> @@ -212,6 +215,11 @@ static int __init param_setup_earlycon(char *buf)
>>>> if (!buf || !buf[0])
>>>> return early_init_dt_scan_chosen_serial();
>>>>
>>>> + if (!strcmp(buf, "acpi_dbg2")) {
>>>> + setup_dbg2_earlycon = true;
>>>> + return 0;
>>>> + }
>>>
>>> So this series doesn't start an ACPI earlycon at early_param time?
>>> That doesn't seem very useful.
>>>
>>> When does the ACPI earlycon actually start?
>>> And don't say "when the DBG2 table is probed"; that much is obvious.
>>
>> ACPI earlycon starts as soon as ACPI tables become accessible (setup_arch()).
>> I think that is still quite early.
>
> I see now; the probe is in patch 6/7.
>
> setup_arch()
> acpi_boot_table_init()
> acpi_probe_device_table()
> ...
> acpi_dbg2_setup()
> ->setup()
> acpi_setup_earlycon()
>
>
>>>> +
>>>> err = setup_earlycon(buf);
>>>> if (err == -ENOENT || err == -EALREADY)
>>>> return 0;
>>>> @@ -286,3 +294,55 @@ int __init of_setup_earlycon(const struct earlycon_id *match,
>>>> }
>>>>
>>>> #endif /* CONFIG_OF_EARLY_FLATTREE */
>>>> +
>>>> +#ifdef CONFIG_ACPI_DBG2_TABLE
>>>> +
>>>> +int __init acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d)
>>>> +{
>>>> + int err;
>>>> + struct uart_port *port = &early_console_dev.port;
>>>> + int (*setup)(struct earlycon_device *, const char *) = d;
>>>> + struct acpi_generic_address *reg;
>>>> +
>>>> + if (!setup_dbg2_earlycon)
>>>> + return -ENODEV;
>>>> +
>>>> + if (device->register_count < 1)
>>>> + return -ENODEV;
>>>> +
>>>> + if (device->base_address_offset >= device->length)
>>>> + return -EINVAL;
>>>> +
>>>> + reg = (void *)device + device->base_address_offset;
>>>> +
>>>> + if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY &&
>>>> + reg->space_id != ACPI_ADR_SPACE_SYSTEM_IO)
>>>> + return -EINVAL;
>>>> +
>>>> + spin_lock_init(&port->lock);
>>>> + port->uartclk = BASE_BAUD * 16;
>>>> +
>>>> + if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
>>>> + if (device->port_type == ACPI_DBG2_ARM_SBSA_32BIT)
>>>> + port->iotype = UPIO_MEM32;
>>>> + else
>>>> + port->iotype = UPIO_MEM;
>>>> + port->mapbase = reg->address;
>>>> + port->membase = earlycon_map(reg->address, SZ_4K);
>>>> + } else {
>>>> + port->iotype = UPIO_PORT;
>>>> + port->iobase = reg->address;
>>>> + }
>>>> +
>>>> + early_console_dev.con->data = &early_console_dev;
>>>> + err = setup(&early_console_dev, NULL);
>>>> + if (err < 0)
>>>> + return err;
>>>> + if (!early_console_dev.con->write)
>>>> + return -ENODEV;
>>>> +
>>>> + register_console(early_console_dev.con);
>>>> + return 0;
>>>> +}
>>>> +
>>>> +#endif /* CONFIG_ACPI_DBG2_TABLE */
>>>> diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
>>>> index 125ae7e..b653752 100644
>>>> --- a/include/linux/acpi_dbg2.h
>>>> +++ b/include/linux/acpi_dbg2.h
>>>> @@ -37,12 +37,32 @@ int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
>>>> ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
>>>> acpi_dbg2_setup, &__acpi_dbg2_data_##name)
>>>>
>>>> +int acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d);
>>>> +
>>>> +/**
>>>> + * ACPI_DBG2_EARLYCON_DECLARE() - Define handler for ACPI GDB2 serial port
>>>> + * @name: Identifier to compose name of table data
>>>> + * @subtype: Subtype of the port
>>>> + * @console_setup: Function to be called to setup the port
>>>> + *
>>>> + * Type of the console_setup() callback is
>>>> + * int (*setup)(struct earlycon_device *, const char *)
>>>> + * It's the type of callback of of_setup_earlycon().
>>>> + */
>>>> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
>>>> + ACPI_DBG2_DECLARE(name, ACPI_DBG2_SERIAL_PORT, subtype, \
>>>> + acpi_setup_earlycon, console_setup)
>>>> +
>>>> #else
>>>>
>>>> #define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>>>> static const void *__acpi_dbg_data_##name[] \
>>>> __used __initdata = { (void *)setup_fn, (void *)data_ptr }
>>>>
>>>> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
>>>> + static const void *__acpi_dbg_data_serial_##name[] \
>>>> + __used __initdata = { (void *)console_setup }
>
> console_setup is a terrible macro argument name; console_setup() is an
> actual kernel function (although file-scope).
> Please change it to something short and generic.
>
> Honestly, I'd just prefer you skip all this apparatus that makes
> ACPI earlycon appear to be like OF earlycon code-wise, but without any of
> the real underpinning or flexibility.
>
> This would be trivial to parse the ACPI table and invoke
> setup_earlycon() with a string specifier instead.
>
> For example,
>
> int __init acpi_earlycon_setup(struct acpi_dbg2_device *dbg2)
> {
> char opts[64];
> struct acpi_generic_addr *addr = (void*)dbg2 + dbg2->base_address_offset;
> int mmio = addr->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY;
This should be
char *mmio = (addr->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) ? "mmio" : "io";
>
> if (dbg2->port_type != ACPI_DBG2_SERIAL_PORT)
> return 0;
>
> switch (dbg2->port_subtype) {
> case ACPI_DBG2_ARM_PL011:
> case ACPI_DBG2_ARM_SBSA_GENERIC:
> case ACPI_DBG2_BCM2835:
> sprintf(opts, "pl011,%s,0x%llx", mmio, addr->address);
> break;
> case ACPI_DBG2_ARM_SBSA_32BIT:
> sprintf(opts, "pl011,mmio32,0x%llx", addr->address);
> break;
> case ACPI_DBG2_16550_COMPATIBLE:
> case ACPI_DBG2_16550_SUBSET:
> sprintf(opts, "uart,%s,0x%llx", mmio, addr->address);
> break;
> default:
> return 0;
> }
>
> return setup_earlycon(opts);
> }
>
> This supports every earlycon ACPI DBG2 declares, not just the ARM_PL011
> subtype of your series.
>
>
>
>>>> +
>>>> #endif
>>>>
>>>> #endif
>>>>
>>>
>
[toc] | [prev] | [next] | [standalone]
| From | Aleksey Makarov <aleksey.makarov@linaro.org> |
|---|---|
| Date | 2016-03-04 14:10 +0100 |
| Subject | Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port |
| Message-ID | <r8XEv-7hh-21@gated-at.bofh.it> |
| In reply to | #1349451 |
On 03/03/2016 08:48 PM, Peter Hurley wrote:
> On 03/01/2016 08:57 AM, Aleksey Makarov wrote:
>>
>>
>> On 03/01/2016 06:53 PM, Peter Hurley wrote:
>>> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>>>> Add ACPI_DBG2_EARLYCON_DECLARE() macros that declares
>>>> an earlycon on the serial port specified in the DBG2 ACPI table.
>>>>
>>>> Pass the string "earlycon=acpi_dbg2" to the kernel to activate it.
>>>>
>>>> Callbacks for EARLYCON_DECLARE() and OF_EARLYCON_DECLARE()
>>>> can also be used for this macros.
>>>>
>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>>> ---
>>>> Documentation/kernel-parameters.txt | 3 ++
>>>> drivers/tty/serial/earlycon.c | 60 +++++++++++++++++++++++++++++++++++++
>>>> include/linux/acpi_dbg2.h | 20 +++++++++++++
>>>> 3 files changed, 83 insertions(+)
>>>>
>>>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>>>> index e0a21e4..19b947b 100644
>>>> --- a/Documentation/kernel-parameters.txt
>>>> +++ b/Documentation/kernel-parameters.txt
>>>> @@ -1072,6 +1072,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>>> A valid base address must be provided, and the serial
>>>> port must already be setup and configured.
>>>>
>>>> + acpi_dbg2
>>>> + Use serial port specified by the DBG2 ACPI table.
>>>> +
>>>> earlyprintk= [X86,SH,BLACKFIN,ARM,M68k]
>>>> earlyprintk=vga
>>>> earlyprintk=efi
>>>> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
>>>> index d217366..9ba3a04 100644
>>>> --- a/drivers/tty/serial/earlycon.c
>>>> +++ b/drivers/tty/serial/earlycon.c
>>>> @@ -22,6 +22,7 @@
>>>> #include <linux/sizes.h>
>>>> #include <linux/of.h>
>>>> #include <linux/of_fdt.h>
>>>> +#include <linux/acpi.h>
>>>>
>>>> #ifdef CONFIG_FIX_EARLYCON_MEM
>>>> #include <asm/fixmap.h>
>>>> @@ -200,6 +201,8 @@ int __init setup_earlycon(char *buf)
>>>> return -ENOENT;
>>>> }
>>>>
>>>> +static bool setup_dbg2_earlycon;
>>>> +
>>>> /* early_param wrapper for setup_earlycon() */
>>>> static int __init param_setup_earlycon(char *buf)
>>>> {
>>>> @@ -212,6 +215,11 @@ static int __init param_setup_earlycon(char *buf)
>>>> if (!buf || !buf[0])
>>>> return early_init_dt_scan_chosen_serial();
>>>>
>>>> + if (!strcmp(buf, "acpi_dbg2")) {
>>>> + setup_dbg2_earlycon = true;
>>>> + return 0;
>>>> + }
>>>
>>> So this series doesn't start an ACPI earlycon at early_param time?
>>> That doesn't seem very useful.
>>>
>>> When does the ACPI earlycon actually start?
>>> And don't say "when the DBG2 table is probed"; that much is obvious.
>>
>> ACPI earlycon starts as soon as ACPI tables become accessible (setup_arch()).
>> I think that is still quite early.
>
> I see now; the probe is in patch 6/7.
>
> setup_arch()
> acpi_boot_table_init()
> acpi_probe_device_table()
> ...
> acpi_dbg2_setup()
> ->setup()
> acpi_setup_earlycon()
>
>
>>>> +
>>>> err = setup_earlycon(buf);
>>>> if (err == -ENOENT || err == -EALREADY)
>>>> return 0;
>>>> @@ -286,3 +294,55 @@ int __init of_setup_earlycon(const struct earlycon_id *match,
>>>> }
>>>>
>>>> #endif /* CONFIG_OF_EARLY_FLATTREE */
>>>> +
>>>> +#ifdef CONFIG_ACPI_DBG2_TABLE
>>>> +
>>>> +int __init acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d)
>>>> +{
>>>> + int err;
>>>> + struct uart_port *port = &early_console_dev.port;
>>>> + int (*setup)(struct earlycon_device *, const char *) = d;
>>>> + struct acpi_generic_address *reg;
>>>> +
>>>> + if (!setup_dbg2_earlycon)
>>>> + return -ENODEV;
>>>> +
>>>> + if (device->register_count < 1)
>>>> + return -ENODEV;
>>>> +
>>>> + if (device->base_address_offset >= device->length)
>>>> + return -EINVAL;
>>>> +
>>>> + reg = (void *)device + device->base_address_offset;
>>>> +
>>>> + if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY &&
>>>> + reg->space_id != ACPI_ADR_SPACE_SYSTEM_IO)
>>>> + return -EINVAL;
>>>> +
>>>> + spin_lock_init(&port->lock);
>>>> + port->uartclk = BASE_BAUD * 16;
>>>> +
>>>> + if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
>>>> + if (device->port_type == ACPI_DBG2_ARM_SBSA_32BIT)
>>>> + port->iotype = UPIO_MEM32;
>>>> + else
>>>> + port->iotype = UPIO_MEM;
>>>> + port->mapbase = reg->address;
>>>> + port->membase = earlycon_map(reg->address, SZ_4K);
>>>> + } else {
>>>> + port->iotype = UPIO_PORT;
>>>> + port->iobase = reg->address;
>>>> + }
>>>> +
>>>> + early_console_dev.con->data = &early_console_dev;
>>>> + err = setup(&early_console_dev, NULL);
>>>> + if (err < 0)
>>>> + return err;
>>>> + if (!early_console_dev.con->write)
>>>> + return -ENODEV;
>>>> +
>>>> + register_console(early_console_dev.con);
>>>> + return 0;
>>>> +}
>>>> +
>>>> +#endif /* CONFIG_ACPI_DBG2_TABLE */
>>>> diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
>>>> index 125ae7e..b653752 100644
>>>> --- a/include/linux/acpi_dbg2.h
>>>> +++ b/include/linux/acpi_dbg2.h
>>>> @@ -37,12 +37,32 @@ int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
>>>> ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
>>>> acpi_dbg2_setup, &__acpi_dbg2_data_##name)
>>>>
>>>> +int acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d);
>>>> +
>>>> +/**
>>>> + * ACPI_DBG2_EARLYCON_DECLARE() - Define handler for ACPI GDB2 serial port
>>>> + * @name: Identifier to compose name of table data
>>>> + * @subtype: Subtype of the port
>>>> + * @console_setup: Function to be called to setup the port
>>>> + *
>>>> + * Type of the console_setup() callback is
>>>> + * int (*setup)(struct earlycon_device *, const char *)
>>>> + * It's the type of callback of of_setup_earlycon().
>>>> + */
>>>> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
>>>> + ACPI_DBG2_DECLARE(name, ACPI_DBG2_SERIAL_PORT, subtype, \
>>>> + acpi_setup_earlycon, console_setup)
>>>> +
>>>> #else
>>>>
>>>> #define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>>>> static const void *__acpi_dbg_data_##name[] \
>>>> __used __initdata = { (void *)setup_fn, (void *)data_ptr }
>>>>
>>>> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
>>>> + static const void *__acpi_dbg_data_serial_##name[] \
>>>> + __used __initdata = { (void *)console_setup }
>
> console_setup is a terrible macro argument name; console_setup() is an
> actual kernel function (although file-scope).
> Please change it to something short and generic.
Is 'setup_fn' ok?
> Honestly, I'd just prefer you skip all this apparatus that makes
> ACPI earlycon appear to be like OF earlycon code-wise, but without any of
> the real underpinning or flexibility.
Actually it was Mark Salter who asked to introduce such macros.
https://lkml.kernel.org/g/1441730339.5459.8.camel@redhat.com
I think reusing the OF functions is a good decision.
Your "but without any of the real underpinning or flexibility" is unfounded.
> This would be trivial to parse the ACPI table and invoke
> setup_earlycon() with a string specifier instead.
>
> For example,
>
> int __init acpi_earlycon_setup(struct acpi_dbg2_device *dbg2)
> {
> char opts[64];
> struct acpi_generic_addr *addr = (void*)dbg2 + dbg2->base_address_offset;
> int mmio = addr->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY;
>
> if (dbg2->port_type != ACPI_DBG2_SERIAL_PORT)
> return 0;
>
> switch (dbg2->port_subtype) {
> case ACPI_DBG2_ARM_PL011:
> case ACPI_DBG2_ARM_SBSA_GENERIC:
> case ACPI_DBG2_BCM2835:
> sprintf(opts, "pl011,%s,0x%llx", mmio, addr->address);
> break;
> case ACPI_DBG2_ARM_SBSA_32BIT:
> sprintf(opts, "pl011,mmio32,0x%llx", addr->address);
> break;
> case ACPI_DBG2_16550_COMPATIBLE:
> case ACPI_DBG2_16550_SUBSET:
> sprintf(opts, "uart,%s,0x%llx", mmio, addr->address);
> break;
> default:
> return 0;
> }
>
> return setup_earlycon(opts);
> }
- Note that this decision forces setting earlycon on GDB2 debug port.
DBG2 does not specify that it should be exactly earlycon.
- You missed ACPI_DBG2_ARM_DCC. And actually I think the list of
debug ports is open. You will have to make up names like "uart" "pl011"
each time a new port is introduced into the specs.
- Most important thing, this way you disclose the internals of serial ports
to the generic earlycon.c Such info as access mode should stay
in the respective drivers.
- I would not like printing address and then parsing it back.
> This supports every earlycon ACPI DBG2 declares, not just the ARM_PL011
> subtype of your series.
To support earlycon on other types of debug port just add literally one
string of code (as in pl011).
>
>
>
>>>> +
>>>> #endif
>>>>
>>>> #endif
>>>>
>>>
>
[toc] | [prev] | [next] | [standalone]
| From | Peter Hurley <peter@hurleysoftware.com> |
|---|---|
| Date | 2016-03-04 16:50 +0100 |
| Subject | Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port |
| Message-ID | <r909j-mc-3@gated-at.bofh.it> |
| In reply to | #1350216 |
On 03/04/2016 05:03 AM, Aleksey Makarov wrote:
>
>
> On 03/03/2016 08:48 PM, Peter Hurley wrote:
>> On 03/01/2016 08:57 AM, Aleksey Makarov wrote:
>>>
>>>
>>> On 03/01/2016 06:53 PM, Peter Hurley wrote:
>>>> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>>>>> Add ACPI_DBG2_EARLYCON_DECLARE() macros that declares
>>>>> an earlycon on the serial port specified in the DBG2 ACPI table.
>>>>>
>>>>> Pass the string "earlycon=acpi_dbg2" to the kernel to activate it.
>>>>>
>>>>> Callbacks for EARLYCON_DECLARE() and OF_EARLYCON_DECLARE()
>>>>> can also be used for this macros.
>>>>>
>>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>>>> ---
>>>>> Documentation/kernel-parameters.txt | 3 ++
>>>>> drivers/tty/serial/earlycon.c | 60 +++++++++++++++++++++++++++++++++++++
>>>>> include/linux/acpi_dbg2.h | 20 +++++++++++++
>>>>> 3 files changed, 83 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>>>>> index e0a21e4..19b947b 100644
>>>>> --- a/Documentation/kernel-parameters.txt
>>>>> +++ b/Documentation/kernel-parameters.txt
>>>>> @@ -1072,6 +1072,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>>>> A valid base address must be provided, and the serial
>>>>> port must already be setup and configured.
>>>>>
>>>>> + acpi_dbg2
>>>>> + Use serial port specified by the DBG2 ACPI table.
>>>>> +
>>>>> earlyprintk= [X86,SH,BLACKFIN,ARM,M68k]
>>>>> earlyprintk=vga
>>>>> earlyprintk=efi
>>>>> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
>>>>> index d217366..9ba3a04 100644
>>>>> --- a/drivers/tty/serial/earlycon.c
>>>>> +++ b/drivers/tty/serial/earlycon.c
>>>>> @@ -22,6 +22,7 @@
>>>>> #include <linux/sizes.h>
>>>>> #include <linux/of.h>
>>>>> #include <linux/of_fdt.h>
>>>>> +#include <linux/acpi.h>
>>>>>
>>>>> #ifdef CONFIG_FIX_EARLYCON_MEM
>>>>> #include <asm/fixmap.h>
>>>>> @@ -200,6 +201,8 @@ int __init setup_earlycon(char *buf)
>>>>> return -ENOENT;
>>>>> }
>>>>>
>>>>> +static bool setup_dbg2_earlycon;
>>>>> +
>>>>> /* early_param wrapper for setup_earlycon() */
>>>>> static int __init param_setup_earlycon(char *buf)
>>>>> {
>>>>> @@ -212,6 +215,11 @@ static int __init param_setup_earlycon(char *buf)
>>>>> if (!buf || !buf[0])
>>>>> return early_init_dt_scan_chosen_serial();
>>>>>
>>>>> + if (!strcmp(buf, "acpi_dbg2")) {
>>>>> + setup_dbg2_earlycon = true;
>>>>> + return 0;
>>>>> + }
>>>>
>>>> So this series doesn't start an ACPI earlycon at early_param time?
>>>> That doesn't seem very useful.
>>>>
>>>> When does the ACPI earlycon actually start?
>>>> And don't say "when the DBG2 table is probed"; that much is obvious.
>>>
>>> ACPI earlycon starts as soon as ACPI tables become accessible (setup_arch()).
>>> I think that is still quite early.
>>
>> I see now; the probe is in patch 6/7.
>>
>> setup_arch()
>> acpi_boot_table_init()
>> acpi_probe_device_table()
>> ...
>> acpi_dbg2_setup()
>> ->setup()
>> acpi_setup_earlycon()
>>
>>
>>>>> +
>>>>> err = setup_earlycon(buf);
>>>>> if (err == -ENOENT || err == -EALREADY)
>>>>> return 0;
>>>>> @@ -286,3 +294,55 @@ int __init of_setup_earlycon(const struct earlycon_id *match,
>>>>> }
>>>>>
>>>>> #endif /* CONFIG_OF_EARLY_FLATTREE */
>>>>> +
>>>>> +#ifdef CONFIG_ACPI_DBG2_TABLE
>>>>> +
>>>>> +int __init acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d)
>>>>> +{
>>>>> + int err;
>>>>> + struct uart_port *port = &early_console_dev.port;
>>>>> + int (*setup)(struct earlycon_device *, const char *) = d;
>>>>> + struct acpi_generic_address *reg;
>>>>> +
>>>>> + if (!setup_dbg2_earlycon)
>>>>> + return -ENODEV;
>>>>> +
>>>>> + if (device->register_count < 1)
>>>>> + return -ENODEV;
>>>>> +
>>>>> + if (device->base_address_offset >= device->length)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + reg = (void *)device + device->base_address_offset;
>>>>> +
>>>>> + if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY &&
>>>>> + reg->space_id != ACPI_ADR_SPACE_SYSTEM_IO)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + spin_lock_init(&port->lock);
>>>>> + port->uartclk = BASE_BAUD * 16;
>>>>> +
>>>>> + if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
>>>>> + if (device->port_type == ACPI_DBG2_ARM_SBSA_32BIT)
>>>>> + port->iotype = UPIO_MEM32;
>>>>> + else
>>>>> + port->iotype = UPIO_MEM;
>>>>> + port->mapbase = reg->address;
>>>>> + port->membase = earlycon_map(reg->address, SZ_4K);
>>>>> + } else {
>>>>> + port->iotype = UPIO_PORT;
>>>>> + port->iobase = reg->address;
>>>>> + }
>>>>> +
>>>>> + early_console_dev.con->data = &early_console_dev;
>>>>> + err = setup(&early_console_dev, NULL);
>>>>> + if (err < 0)
>>>>> + return err;
>>>>> + if (!early_console_dev.con->write)
>>>>> + return -ENODEV;
>>>>> +
>>>>> + register_console(early_console_dev.con);
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +#endif /* CONFIG_ACPI_DBG2_TABLE */
>>>>> diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
>>>>> index 125ae7e..b653752 100644
>>>>> --- a/include/linux/acpi_dbg2.h
>>>>> +++ b/include/linux/acpi_dbg2.h
>>>>> @@ -37,12 +37,32 @@ int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
>>>>> ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
>>>>> acpi_dbg2_setup, &__acpi_dbg2_data_##name)
>>>>>
>>>>> +int acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d);
>>>>> +
>>>>> +/**
>>>>> + * ACPI_DBG2_EARLYCON_DECLARE() - Define handler for ACPI GDB2 serial port
>>>>> + * @name: Identifier to compose name of table data
>>>>> + * @subtype: Subtype of the port
>>>>> + * @console_setup: Function to be called to setup the port
>>>>> + *
>>>>> + * Type of the console_setup() callback is
>>>>> + * int (*setup)(struct earlycon_device *, const char *)
>>>>> + * It's the type of callback of of_setup_earlycon().
>>>>> + */
>>>>> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
>>>>> + ACPI_DBG2_DECLARE(name, ACPI_DBG2_SERIAL_PORT, subtype, \
>>>>> + acpi_setup_earlycon, console_setup)
>>>>> +
>>>>> #else
>>>>>
>>>>> #define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>>>>> static const void *__acpi_dbg_data_##name[] \
>>>>> __used __initdata = { (void *)setup_fn, (void *)data_ptr }
>>>>>
>>>>> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
>>>>> + static const void *__acpi_dbg_data_serial_##name[] \
>>>>> + __used __initdata = { (void *)console_setup }
>>
>> console_setup is a terrible macro argument name; console_setup() is an
>> actual kernel function (although file-scope).
>> Please change it to something short and generic.
>
> Is 'setup_fn' ok?
>
>> Honestly, I'd just prefer you skip all this apparatus that makes
>> ACPI earlycon appear to be like OF earlycon code-wise, but without any of
>> the real underpinning or flexibility.
>
> Actually it was Mark Salter who asked to introduce such macros.
>
> https://lkml.kernel.org/g/1441730339.5459.8.camel@redhat.com
>
> I think reusing the OF functions is a good decision.
>
> Your "but without any of the real underpinning or flexibility" is unfounded.
1. Lack of real underpinning.
Can't start an earlycon at early_param time to debug arch issues.
As a result, everyone will continue using command line for earlycon which
makes this series useless.
2. Lack of flexibility.
OF earlycon supports any new hardware simply by string matching w/o
requiring any approvals. I can add earlycon support for anything in
10 minutes.
ACPI earlycon for any new hardware requires approvals and spec changes.
2-3 months.
Founded.
>> This would be trivial to parse the ACPI table and invoke
>> setup_earlycon() with a string specifier instead.
>>
>> For example,
>>
>> int __init acpi_earlycon_setup(struct acpi_dbg2_device *dbg2)
>> {
>> char opts[64];
>> struct acpi_generic_addr *addr = (void*)dbg2 + dbg2->base_address_offset;
>> int mmio = addr->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY;
>>
>> if (dbg2->port_type != ACPI_DBG2_SERIAL_PORT)
>> return 0;
>>
>> switch (dbg2->port_subtype) {
>> case ACPI_DBG2_ARM_PL011:
>> case ACPI_DBG2_ARM_SBSA_GENERIC:
>> case ACPI_DBG2_BCM2835:
>> sprintf(opts, "pl011,%s,0x%llx", mmio, addr->address);
>> break;
>> case ACPI_DBG2_ARM_SBSA_32BIT:
>> sprintf(opts, "pl011,mmio32,0x%llx", addr->address);
>> break;
>> case ACPI_DBG2_16550_COMPATIBLE:
>> case ACPI_DBG2_16550_SUBSET:
>> sprintf(opts, "uart,%s,0x%llx", mmio, addr->address);
>> break;
>> default:
>> return 0;
>> }
>>
>> return setup_earlycon(opts);
>> }
>
> - Note that this decision forces setting earlycon on GDB2 debug port.
> DBG2 does not specify that it should be exactly earlycon.
Obviously my example was simplified to point out how easy it is
to start an earlycon with a string specifier.
I assumed you would understand that
if (!setup_dbg2_earlycon)
return 0;
was omitted for clarity (or handled at a higher level).
> - You missed ACPI_DBG2_ARM_DCC.
What is this?
Your series _only_ adds ACPI_DBG2_ARM_PL011 and none of the others.
If you add ACPI_DBG2_ARM_DCC support to your series, I'll add it
to my example.
> And actually I think the list of
> debug ports is open. You will have to make up names like "uart" "pl011"
> each time a new port is introduced into the specs.
5 new ports have been added in 1 decade. I think we can keep up with
that rate of change.
And you keep going on about "make up names". _For the last time_,
these "make up names" are the documented and defined console names for
earlycons. They will *never* change, because these same string
specifiers are used on the command line.
> - Most important thing, this way you disclose the internals of serial ports
> to the generic earlycon.c Such info as access mode should stay
> in the respective drivers.
??
My example function above doesn't go in "generic earlycon.c"
You leave it in ACPI where it belongs. setup_earlycon() is already global
scope, because like I've said repeatedly, it's already used by firmware to
start earlycons.
And I don't know what you mean by "access mode".
If you're referring to ["io","mmio","mmio32"], this is how earlycon is
specified and has been since the first patch. Besides your own patch decodes
and sets the iotype (UPIO_IO, UPIO_MEM, UPIO_MEM32) so I don't get what
you're objecting to here.
And if anything, the DBG2 table is under-specified.
Such things as endianness, register stride and i/o width are missing.
Not to mention line settings like baud rate, parity, stop bits, and flow
control.
> - I would not like printing address and then parsing it back.
The "parsing it back" is already implemented: that's how command line
earlycon works. That will never change.
And this method is already used by firmware other than OF.
>> This supports every earlycon ACPI DBG2 declares, not just the ARM_PL011
>> subtype of your series.
>
> To support earlycon on other types of debug port just add literally one
> string of code (as in pl011).
And as I've already shown, so does my way.
In 1/2 as much code, without macros or all the ACPI linker table changes.
>>>>> +
>>>>> #endif
>>>>>
>>>>> #endif
>>>>>
>>>>
>>
[toc] | [prev] | [next] | [standalone]
| From | Peter Hurley <peter@hurleysoftware.com> |
|---|---|
| Date | 2016-03-04 17:00 +0100 |
| Subject | Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port |
| Message-ID | <r90iZ-qK-7@gated-at.bofh.it> |
| In reply to | #1350216 |
On 03/04/2016 05:03 AM, Aleksey Makarov wrote: > Actually it was Mark Salter who asked to introduce such macros. > > https://lkml.kernel.org/g/1441730339.5459.8.camel@redhat.com I wasn't copied on that series, sorry. > I think reusing the OF functions is a good decision. But you're not reusing the OF functions; you're duplicating them.
[toc] | [prev] | [next] | [standalone]
| From | Aleksey Makarov <aleksey.makarov@linaro.org> |
|---|---|
| Date | 2016-02-29 13:50 +0100 |
| Subject | [PATCH v3 1/7] of/serial: move earlycon early_param handling to serial |
| Message-ID | <r7vqV-345-13@gated-at.bofh.it> |
| In reply to | #1345756 |
From: Leif Lindholm <leif.lindholm@linaro.org>
We have multiple "earlycon" early_param handlers - merge the DT one into
the main earlycon one. This means the earlycon early_param handler does
not just return success if no options are specified.
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
---
drivers/of/fdt.c | 11 +----------
drivers/tty/serial/earlycon.c | 3 ++-
include/linux/of_fdt.h | 2 ++
3 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 3349d2a..0547256 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -805,7 +805,7 @@ static inline void early_init_dt_check_for_initrd(unsigned long node)
#ifdef CONFIG_SERIAL_EARLYCON
-static int __init early_init_dt_scan_chosen_serial(void)
+int __init early_init_dt_scan_chosen_serial(void)
{
int offset;
const char *p, *q, *options = NULL;
@@ -849,15 +849,6 @@ static int __init early_init_dt_scan_chosen_serial(void)
}
return -ENODEV;
}
-
-static int __init setup_of_earlycon(char *buf)
-{
- if (buf)
- return 0;
-
- return early_init_dt_scan_chosen_serial();
-}
-early_param("earlycon", setup_of_earlycon);
#endif
/**
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index 067783f..d217366 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -17,6 +17,7 @@
#include <linux/kernel.h>
#include <linux/init.h>
#include <linux/io.h>
+#include <linux/of_fdt.h>
#include <linux/serial_core.h>
#include <linux/sizes.h>
#include <linux/of.h>
@@ -209,7 +210,7 @@ static int __init param_setup_earlycon(char *buf)
* don't generate a warning from parse_early_params() in that case
*/
if (!buf || !buf[0])
- return 0;
+ return early_init_dt_scan_chosen_serial();
err = setup_earlycon(buf);
if (err == -ENOENT || err == -EALREADY)
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index 2fbe868..56b2a43 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -63,6 +63,7 @@ extern int early_init_dt_scan_chosen(unsigned long node, const char *uname,
int depth, void *data);
extern int early_init_dt_scan_memory(unsigned long node, const char *uname,
int depth, void *data);
+extern int early_init_dt_scan_chosen_serial(void);
extern void early_init_fdt_scan_reserved_mem(void);
extern void early_init_fdt_reserve_self(void);
extern void early_init_dt_add_memory_arch(u64 base, u64 size);
@@ -91,6 +92,7 @@ extern void early_get_first_memblock_info(void *, phys_addr_t *);
extern u64 of_flat_dt_translate_address(unsigned long node);
extern void of_fdt_limit_memory(int limit);
#else /* CONFIG_OF_FLATTREE */
+static inline int early_init_dt_scan_chosen_serial(void) { return -ENODEV; }
static inline void early_init_fdt_scan_reserved_mem(void) {}
static inline void early_init_fdt_reserve_self(void) {}
static inline const char *of_flat_dt_get_machine_name(void) { return NULL; }
--
2.7.1
[toc] | [prev] | [next] | [standalone]
| From | Peter Hurley <peter@hurleysoftware.com> |
|---|---|
| Date | 2016-03-01 16:00 +0100 |
| Subject | Re: [PATCH v3 1/7] of/serial: move earlycon early_param handling to serial |
| Message-ID | <r7TWh-216-13@gated-at.bofh.it> |
| In reply to | #1345759 |
On 02/29/2016 04:41 AM, Aleksey Makarov wrote:
> From: Leif Lindholm <leif.lindholm@linaro.org>
>
> We have multiple "earlycon" early_param handlers - merge the DT one into
> the main earlycon one. This means the earlycon early_param handler does
> not just return success if no options are specified.
Why is this patch necessary?
Is this cleanup or required for some reason in other parts of this
series?
> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> ---
> drivers/of/fdt.c | 11 +----------
> drivers/tty/serial/earlycon.c | 3 ++-
> include/linux/of_fdt.h | 2 ++
> 3 files changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 3349d2a..0547256 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -805,7 +805,7 @@ static inline void early_init_dt_check_for_initrd(unsigned long node)
>
> #ifdef CONFIG_SERIAL_EARLYCON
>
> -static int __init early_init_dt_scan_chosen_serial(void)
> +int __init early_init_dt_scan_chosen_serial(void)
> {
> int offset;
> const char *p, *q, *options = NULL;
> @@ -849,15 +849,6 @@ static int __init early_init_dt_scan_chosen_serial(void)
> }
> return -ENODEV;
> }
> -
> -static int __init setup_of_earlycon(char *buf)
> -{
> - if (buf)
> - return 0;
> -
> - return early_init_dt_scan_chosen_serial();
> -}
> -early_param("earlycon", setup_of_earlycon);
> #endif
>
> /**
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index 067783f..d217366 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -17,6 +17,7 @@
> #include <linux/kernel.h>
> #include <linux/init.h>
> #include <linux/io.h>
> +#include <linux/of_fdt.h>
> #include <linux/serial_core.h>
> #include <linux/sizes.h>
> #include <linux/of.h>
> @@ -209,7 +210,7 @@ static int __init param_setup_earlycon(char *buf)
> * don't generate a warning from parse_early_params() in that case
> */
> if (!buf || !buf[0])
> - return 0;
> + return early_init_dt_scan_chosen_serial();
>
> err = setup_earlycon(buf);
> if (err == -ENOENT || err == -EALREADY)
> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
> index 2fbe868..56b2a43 100644
> --- a/include/linux/of_fdt.h
> +++ b/include/linux/of_fdt.h
> @@ -63,6 +63,7 @@ extern int early_init_dt_scan_chosen(unsigned long node, const char *uname,
> int depth, void *data);
> extern int early_init_dt_scan_memory(unsigned long node, const char *uname,
> int depth, void *data);
> +extern int early_init_dt_scan_chosen_serial(void);
> extern void early_init_fdt_scan_reserved_mem(void);
> extern void early_init_fdt_reserve_self(void);
> extern void early_init_dt_add_memory_arch(u64 base, u64 size);
> @@ -91,6 +92,7 @@ extern void early_get_first_memblock_info(void *, phys_addr_t *);
> extern u64 of_flat_dt_translate_address(unsigned long node);
> extern void of_fdt_limit_memory(int limit);
> #else /* CONFIG_OF_FLATTREE */
> +static inline int early_init_dt_scan_chosen_serial(void) { return -ENODEV; }
> static inline void early_init_fdt_scan_reserved_mem(void) {}
> static inline void early_init_fdt_reserve_self(void) {}
> static inline const char *of_flat_dt_get_machine_name(void) { return NULL; }
>
[toc] | [prev] | [next] | [standalone]
| From | Aleksey Makarov <aleksey.makarov@linaro.org> |
|---|---|
| Date | 2016-03-01 17:40 +0100 |
| Subject | Re: [PATCH v3 1/7] of/serial: move earlycon early_param handling to serial |
| Message-ID | <r7Vv4-39C-11@gated-at.bofh.it> |
| In reply to | #1346672 |
On 03/01/2016 05:50 PM, Peter Hurley wrote:
> On 02/29/2016 04:41 AM, Aleksey Makarov wrote:
>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>
>> We have multiple "earlycon" early_param handlers - merge the DT one into
>> the main earlycon one. This means the earlycon early_param handler does
>> not just return success if no options are specified.
>
> Why is this patch necessary?
>
> Is this cleanup or required for some reason in other parts of this
> series?
It's a cleanup.
Also, without this, the series has to introduce a new (third) handler for "earlycon=".
>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>> ---
>> drivers/of/fdt.c | 11 +----------
>> drivers/tty/serial/earlycon.c | 3 ++-
>> include/linux/of_fdt.h | 2 ++
>> 3 files changed, 5 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>> index 3349d2a..0547256 100644
>> --- a/drivers/of/fdt.c
>> +++ b/drivers/of/fdt.c
>> @@ -805,7 +805,7 @@ static inline void early_init_dt_check_for_initrd(unsigned long node)
>>
>> #ifdef CONFIG_SERIAL_EARLYCON
>>
>> -static int __init early_init_dt_scan_chosen_serial(void)
>> +int __init early_init_dt_scan_chosen_serial(void)
>> {
>> int offset;
>> const char *p, *q, *options = NULL;
>> @@ -849,15 +849,6 @@ static int __init early_init_dt_scan_chosen_serial(void)
>> }
>> return -ENODEV;
>> }
>> -
>> -static int __init setup_of_earlycon(char *buf)
>> -{
>> - if (buf)
>> - return 0;
>> -
>> - return early_init_dt_scan_chosen_serial();
>> -}
>> -early_param("earlycon", setup_of_earlycon);
>> #endif
>>
>> /**
>> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
>> index 067783f..d217366 100644
>> --- a/drivers/tty/serial/earlycon.c
>> +++ b/drivers/tty/serial/earlycon.c
>> @@ -17,6 +17,7 @@
>> #include <linux/kernel.h>
>> #include <linux/init.h>
>> #include <linux/io.h>
>> +#include <linux/of_fdt.h>
>> #include <linux/serial_core.h>
>> #include <linux/sizes.h>
>> #include <linux/of.h>
>> @@ -209,7 +210,7 @@ static int __init param_setup_earlycon(char *buf)
>> * don't generate a warning from parse_early_params() in that case
>> */
>> if (!buf || !buf[0])
>> - return 0;
>> + return early_init_dt_scan_chosen_serial();
>>
>> err = setup_earlycon(buf);
>> if (err == -ENOENT || err == -EALREADY)
>> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
>> index 2fbe868..56b2a43 100644
>> --- a/include/linux/of_fdt.h
>> +++ b/include/linux/of_fdt.h
>> @@ -63,6 +63,7 @@ extern int early_init_dt_scan_chosen(unsigned long node, const char *uname,
>> int depth, void *data);
>> extern int early_init_dt_scan_memory(unsigned long node, const char *uname,
>> int depth, void *data);
>> +extern int early_init_dt_scan_chosen_serial(void);
>> extern void early_init_fdt_scan_reserved_mem(void);
>> extern void early_init_fdt_reserve_self(void);
>> extern void early_init_dt_add_memory_arch(u64 base, u64 size);
>> @@ -91,6 +92,7 @@ extern void early_get_first_memblock_info(void *, phys_addr_t *);
>> extern u64 of_flat_dt_translate_address(unsigned long node);
>> extern void of_fdt_limit_memory(int limit);
>> #else /* CONFIG_OF_FLATTREE */
>> +static inline int early_init_dt_scan_chosen_serial(void) { return -ENODEV; }
>> static inline void early_init_fdt_scan_reserved_mem(void) {}
>> static inline void early_init_fdt_reserve_self(void) {}
>> static inline const char *of_flat_dt_get_machine_name(void) { return NULL; }
>>
>
[toc] | [prev] | [next] | [standalone]
| From | Peter Hurley <peter@hurleysoftware.com> |
|---|---|
| Date | 2016-03-01 18:30 +0100 |
| Subject | Re: [PATCH v3 1/7] of/serial: move earlycon early_param handling to serial |
| Message-ID | <r7Whr-3Ha-1@gated-at.bofh.it> |
| In reply to | #1346739 |
On 03/01/2016 08:31 AM, Aleksey Makarov wrote:
>
>
> On 03/01/2016 05:50 PM, Peter Hurley wrote:
>> On 02/29/2016 04:41 AM, Aleksey Makarov wrote:
>>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>>
>>> We have multiple "earlycon" early_param handlers - merge the DT one into
>>> the main earlycon one. This means the earlycon early_param handler does
>>> not just return success if no options are specified.
>>
>> Why is this patch necessary?
>>
>> Is this cleanup or required for some reason in other parts of this
>> series?
>
> It's a cleanup.
>
> Also, without this, the series has to introduce a new (third) handler for "earlycon=".
Not if it were opt-in like OF is via the same blank
"earlycon" parameter.
>>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>> ---
>>> drivers/of/fdt.c | 11 +----------
>>> drivers/tty/serial/earlycon.c | 3 ++-
>>> include/linux/of_fdt.h | 2 ++
>>> 3 files changed, 5 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>>> index 3349d2a..0547256 100644
>>> --- a/drivers/of/fdt.c
>>> +++ b/drivers/of/fdt.c
>>> @@ -805,7 +805,7 @@ static inline void early_init_dt_check_for_initrd(unsigned long node)
>>>
>>> #ifdef CONFIG_SERIAL_EARLYCON
>>>
>>> -static int __init early_init_dt_scan_chosen_serial(void)
>>> +int __init early_init_dt_scan_chosen_serial(void)
>>> {
>>> int offset;
>>> const char *p, *q, *options = NULL;
>>> @@ -849,15 +849,6 @@ static int __init early_init_dt_scan_chosen_serial(void)
>>> }
>>> return -ENODEV;
>>> }
>>> -
>>> -static int __init setup_of_earlycon(char *buf)
>>> -{
>>> - if (buf)
>>> - return 0;
>>> -
>>> - return early_init_dt_scan_chosen_serial();
>>> -}
>>> -early_param("earlycon", setup_of_earlycon);
>>> #endif
>>>
>>> /**
>>> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
>>> index 067783f..d217366 100644
>>> --- a/drivers/tty/serial/earlycon.c
>>> +++ b/drivers/tty/serial/earlycon.c
>>> @@ -17,6 +17,7 @@
>>> #include <linux/kernel.h>
>>> #include <linux/init.h>
>>> #include <linux/io.h>
>>> +#include <linux/of_fdt.h>
>>> #include <linux/serial_core.h>
>>> #include <linux/sizes.h>
>>> #include <linux/of.h>
>>> @@ -209,7 +210,7 @@ static int __init param_setup_earlycon(char *buf)
>>> * don't generate a warning from parse_early_params() in that case
>>> */
>>> if (!buf || !buf[0])
>>> - return 0;
>>> + return early_init_dt_scan_chosen_serial();
>>>
>>> err = setup_earlycon(buf);
>>> if (err == -ENOENT || err == -EALREADY)
>>> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
>>> index 2fbe868..56b2a43 100644
>>> --- a/include/linux/of_fdt.h
>>> +++ b/include/linux/of_fdt.h
>>> @@ -63,6 +63,7 @@ extern int early_init_dt_scan_chosen(unsigned long node, const char *uname,
>>> int depth, void *data);
>>> extern int early_init_dt_scan_memory(unsigned long node, const char *uname,
>>> int depth, void *data);
>>> +extern int early_init_dt_scan_chosen_serial(void);
>>> extern void early_init_fdt_scan_reserved_mem(void);
>>> extern void early_init_fdt_reserve_self(void);
>>> extern void early_init_dt_add_memory_arch(u64 base, u64 size);
>>> @@ -91,6 +92,7 @@ extern void early_get_first_memblock_info(void *, phys_addr_t *);
>>> extern u64 of_flat_dt_translate_address(unsigned long node);
>>> extern void of_fdt_limit_memory(int limit);
>>> #else /* CONFIG_OF_FLATTREE */
>>> +static inline int early_init_dt_scan_chosen_serial(void) { return -ENODEV; }
>>> static inline void early_init_fdt_scan_reserved_mem(void) {}
>>> static inline void early_init_fdt_reserve_self(void) {}
>>> static inline const char *of_flat_dt_get_machine_name(void) { return NULL; }
>>>
>>
[toc] | [prev] | [next] | [standalone]
| From | Aleksey Makarov <amakarov.linux@gmail.com> |
|---|---|
| Date | 2016-03-01 19:00 +0100 |
| Subject | Re: [PATCH v3 1/7] of/serial: move earlycon early_param handling to serial |
| Message-ID | <r7WKu-3T3-11@gated-at.bofh.it> |
| In reply to | #1346786 |
On 03/01/2016 08:24 PM, Peter Hurley wrote:
> On 03/01/2016 08:31 AM, Aleksey Makarov wrote:
>>
>>
>> On 03/01/2016 05:50 PM, Peter Hurley wrote:
>>> On 02/29/2016 04:41 AM, Aleksey Makarov wrote:
>>>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>>>
>>>> We have multiple "earlycon" early_param handlers - merge the DT one into
>>>> the main earlycon one. This means the earlycon early_param handler does
>>>> not just return success if no options are specified.
>>>
>>> Why is this patch necessary?
>>>
>>> Is this cleanup or required for some reason in other parts of this
>>> series?
>>
>> It's a cleanup.
>>
>> Also, without this, the series has to introduce a new (third) handler for "earlycon=".
>
> Not if it were opt-in like OF is via the same blank
> "earlycon" parameter.
So you are suggesting to enable ACPI earlycon with just "earlyon" without
any arguments to this paramener, right? That's good idea as OF and ACPI
can not be used simultaneously to specify earlycon. Actually that is
how it worked in the original submussion. I will change this in the next version.
Still I think that having this parameter parsed in one place is a good idea.
Or would you prefer to drop this patch?
>>>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>>> ---
>>>> drivers/of/fdt.c | 11 +----------
>>>> drivers/tty/serial/earlycon.c | 3 ++-
>>>> include/linux/of_fdt.h | 2 ++
>>>> 3 files changed, 5 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>>>> index 3349d2a..0547256 100644
>>>> --- a/drivers/of/fdt.c
>>>> +++ b/drivers/of/fdt.c
>>>> @@ -805,7 +805,7 @@ static inline void early_init_dt_check_for_initrd(unsigned long node)
>>>>
>>>> #ifdef CONFIG_SERIAL_EARLYCON
>>>>
>>>> -static int __init early_init_dt_scan_chosen_serial(void)
>>>> +int __init early_init_dt_scan_chosen_serial(void)
>>>> {
>>>> int offset;
>>>> const char *p, *q, *options = NULL;
>>>> @@ -849,15 +849,6 @@ static int __init early_init_dt_scan_chosen_serial(void)
>>>> }
>>>> return -ENODEV;
>>>> }
>>>> -
>>>> -static int __init setup_of_earlycon(char *buf)
>>>> -{
>>>> - if (buf)
>>>> - return 0;
>>>> -
>>>> - return early_init_dt_scan_chosen_serial();
>>>> -}
>>>> -early_param("earlycon", setup_of_earlycon);
>>>> #endif
>>>>
>>>> /**
>>>> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
>>>> index 067783f..d217366 100644
>>>> --- a/drivers/tty/serial/earlycon.c
>>>> +++ b/drivers/tty/serial/earlycon.c
>>>> @@ -17,6 +17,7 @@
>>>> #include <linux/kernel.h>
>>>> #include <linux/init.h>
>>>> #include <linux/io.h>
>>>> +#include <linux/of_fdt.h>
>>>> #include <linux/serial_core.h>
>>>> #include <linux/sizes.h>
>>>> #include <linux/of.h>
>>>> @@ -209,7 +210,7 @@ static int __init param_setup_earlycon(char *buf)
>>>> * don't generate a warning from parse_early_params() in that case
>>>> */
>>>> if (!buf || !buf[0])
>>>> - return 0;
>>>> + return early_init_dt_scan_chosen_serial();
>>>>
>>>> err = setup_earlycon(buf);
>>>> if (err == -ENOENT || err == -EALREADY)
>>>> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
>>>> index 2fbe868..56b2a43 100644
>>>> --- a/include/linux/of_fdt.h
>>>> +++ b/include/linux/of_fdt.h
>>>> @@ -63,6 +63,7 @@ extern int early_init_dt_scan_chosen(unsigned long node, const char *uname,
>>>> int depth, void *data);
>>>> extern int early_init_dt_scan_memory(unsigned long node, const char *uname,
>>>> int depth, void *data);
>>>> +extern int early_init_dt_scan_chosen_serial(void);
>>>> extern void early_init_fdt_scan_reserved_mem(void);
>>>> extern void early_init_fdt_reserve_self(void);
>>>> extern void early_init_dt_add_memory_arch(u64 base, u64 size);
>>>> @@ -91,6 +92,7 @@ extern void early_get_first_memblock_info(void *, phys_addr_t *);
>>>> extern u64 of_flat_dt_translate_address(unsigned long node);
>>>> extern void of_fdt_limit_memory(int limit);
>>>> #else /* CONFIG_OF_FLATTREE */
>>>> +static inline int early_init_dt_scan_chosen_serial(void) { return -ENODEV; }
>>>> static inline void early_init_fdt_scan_reserved_mem(void) {}
>>>> static inline void early_init_fdt_reserve_self(void) {}
>>>> static inline const char *of_flat_dt_get_machine_name(void) { return NULL; }
>>>>
>>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
[toc] | [prev] | [next] | [standalone]
| From | Peter Hurley <peter@hurleysoftware.com> |
|---|---|
| Date | 2016-03-01 19:30 +0100 |
| Subject | Re: [PATCH v3 1/7] of/serial: move earlycon early_param handling to serial |
| Message-ID | <r7Xdw-4kR-3@gated-at.bofh.it> |
| In reply to | #1346819 |
On 03/01/2016 09:52 AM, Aleksey Makarov wrote:
>
>
> On 03/01/2016 08:24 PM, Peter Hurley wrote:
>> On 03/01/2016 08:31 AM, Aleksey Makarov wrote:
>>>
>>>
>>> On 03/01/2016 05:50 PM, Peter Hurley wrote:
>>>> On 02/29/2016 04:41 AM, Aleksey Makarov wrote:
>>>>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>>>>
>>>>> We have multiple "earlycon" early_param handlers - merge the DT one into
>>>>> the main earlycon one. This means the earlycon early_param handler does
>>>>> not just return success if no options are specified.
>>>>
>>>> Why is this patch necessary?
>>>>
>>>> Is this cleanup or required for some reason in other parts of this
>>>> series?
>>>
>>> It's a cleanup.
>>>
>>> Also, without this, the series has to introduce a new (third) handler for "earlycon=".
>>
>> Not if it were opt-in like OF is via the same blank
>> "earlycon" parameter.
>
> So you are suggesting to enable ACPI earlycon with just "earlyon" without
> any arguments to this paramener, right? That's good idea as OF and ACPI
> can not be used simultaneously to specify earlycon. Actually that is
> how it worked in the original submussion. I will change this in the next version.
>
> Still I think that having this parameter parsed in one place is a good idea.
> Or would you prefer to drop this patch?
No need to drop it, imo; the cleanup is fine with me.
But it really helps if either the commit message or the cover letter
describes the purpose and not just the effect (ie., the why and not just
the what).
Regards,
Peter Hurley
>>>>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
>>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>>>> ---
>>>>> drivers/of/fdt.c | 11 +----------
>>>>> drivers/tty/serial/earlycon.c | 3 ++-
>>>>> include/linux/of_fdt.h | 2 ++
>>>>> 3 files changed, 5 insertions(+), 11 deletions(-)
>>>>>
>>>>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>>>>> index 3349d2a..0547256 100644
>>>>> --- a/drivers/of/fdt.c
>>>>> +++ b/drivers/of/fdt.c
>>>>> @@ -805,7 +805,7 @@ static inline void early_init_dt_check_for_initrd(unsigned long node)
>>>>>
>>>>> #ifdef CONFIG_SERIAL_EARLYCON
>>>>>
>>>>> -static int __init early_init_dt_scan_chosen_serial(void)
>>>>> +int __init early_init_dt_scan_chosen_serial(void)
>>>>> {
>>>>> int offset;
>>>>> const char *p, *q, *options = NULL;
>>>>> @@ -849,15 +849,6 @@ static int __init early_init_dt_scan_chosen_serial(void)
>>>>> }
>>>>> return -ENODEV;
>>>>> }
>>>>> -
>>>>> -static int __init setup_of_earlycon(char *buf)
>>>>> -{
>>>>> - if (buf)
>>>>> - return 0;
>>>>> -
>>>>> - return early_init_dt_scan_chosen_serial();
>>>>> -}
>>>>> -early_param("earlycon", setup_of_earlycon);
>>>>> #endif
>>>>>
>>>>> /**
>>>>> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
>>>>> index 067783f..d217366 100644
>>>>> --- a/drivers/tty/serial/earlycon.c
>>>>> +++ b/drivers/tty/serial/earlycon.c
>>>>> @@ -17,6 +17,7 @@
>>>>> #include <linux/kernel.h>
>>>>> #include <linux/init.h>
>>>>> #include <linux/io.h>
>>>>> +#include <linux/of_fdt.h>
>>>>> #include <linux/serial_core.h>
>>>>> #include <linux/sizes.h>
>>>>> #include <linux/of.h>
>>>>> @@ -209,7 +210,7 @@ static int __init param_setup_earlycon(char *buf)
>>>>> * don't generate a warning from parse_early_params() in that case
>>>>> */
>>>>> if (!buf || !buf[0])
>>>>> - return 0;
>>>>> + return early_init_dt_scan_chosen_serial();
>>>>>
>>>>> err = setup_earlycon(buf);
>>>>> if (err == -ENOENT || err == -EALREADY)
>>>>> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
>>>>> index 2fbe868..56b2a43 100644
>>>>> --- a/include/linux/of_fdt.h
>>>>> +++ b/include/linux/of_fdt.h
>>>>> @@ -63,6 +63,7 @@ extern int early_init_dt_scan_chosen(unsigned long node, const char *uname,
>>>>> int depth, void *data);
>>>>> extern int early_init_dt_scan_memory(unsigned long node, const char *uname,
>>>>> int depth, void *data);
>>>>> +extern int early_init_dt_scan_chosen_serial(void);
>>>>> extern void early_init_fdt_scan_reserved_mem(void);
>>>>> extern void early_init_fdt_reserve_self(void);
>>>>> extern void early_init_dt_add_memory_arch(u64 base, u64 size);
>>>>> @@ -91,6 +92,7 @@ extern void early_get_first_memblock_info(void *, phys_addr_t *);
>>>>> extern u64 of_flat_dt_translate_address(unsigned long node);
>>>>> extern void of_fdt_limit_memory(int limit);
>>>>> #else /* CONFIG_OF_FLATTREE */
>>>>> +static inline int early_init_dt_scan_chosen_serial(void) { return -ENODEV; }
>>>>> static inline void early_init_fdt_scan_reserved_mem(void) {}
>>>>> static inline void early_init_fdt_reserve_self(void) {}
>>>>> static inline const char *of_flat_dt_get_machine_name(void) { return NULL; }
>>>>>
>>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
[toc] | [prev] | [next] | [standalone]
| From | Aleksey Makarov <aleksey.makarov@linaro.org> |
|---|---|
| Date | 2016-02-29 13:50 +0100 |
| Subject | [PATCH v3 7/7] serial: pl011: add ACPI DBG2 serial port |
| Message-ID | <r7vqW-345-15@gated-at.bofh.it> |
| In reply to | #1345756 |
Add a handler for ACPI DBG2 serial port of type ACPI_DBG2_ARM_PL011 that sets up an earlycon on it. Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org> --- drivers/tty/serial/amba-pl011.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c index 500232a..d38da86 100644 --- a/drivers/tty/serial/amba-pl011.c +++ b/drivers/tty/serial/amba-pl011.c @@ -59,6 +59,7 @@ #include <linux/sizes.h> #include <linux/io.h> #include <linux/acpi.h> +#include <linux/acpi_dbg2.h> #include "amba-pl011.h" @@ -2327,6 +2328,8 @@ static int __init pl011_early_console_setup(struct earlycon_device *device, return 0; } OF_EARLYCON_DECLARE(pl011, "arm,pl011", pl011_early_console_setup); +ACPI_DBG2_EARLYCON_DECLARE(pl011, ACPI_DBG2_ARM_PL011, + pl011_early_console_setup); #else #define AMBA_CONSOLE NULL -- 2.7.1
[toc] | [prev] | [next] | [standalone]
| From | Aleksey Makarov <aleksey.makarov@linaro.org> |
|---|---|
| Date | 2016-02-29 13:50 +0100 |
| Subject | [PATCH v3 6/7] ACPI: enable ACPI_DBG2_TABLE on ARM64 |
| Message-ID | <r7vqW-345-27@gated-at.bofh.it> |
| In reply to | #1345756 |
SBBR mentions DBG2 as a mandatory ACPI table.
So enable it for ARM64
Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
---
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/acpi.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 9dc5209..0b614d4 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -3,6 +3,7 @@ config ARM64
select ACPI_CCA_REQUIRED if ACPI
select ACPI_GENERIC_GSI if ACPI
select ACPI_REDUCED_HARDWARE_ONLY if ACPI
+ select ACPI_DBG2_TABLE if ACPI
select ARCH_HAS_DEVMEM_IS_ALLOWED
select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
select ARCH_HAS_ELF_RANDOMIZE
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index d1ce8e2..3370046 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -208,6 +208,8 @@ void __init acpi_boot_table_init(void)
pr_err("Failed to init ACPI tables\n");
if (!param_acpi_force)
disable_acpi();
+ } else {
+ acpi_probe_device_table(dbg2);
}
}
--
2.7.1
[toc] | [prev] | [next] | [standalone]
| From | Peter Hurley <peter@hurleysoftware.com> |
|---|---|
| Date | 2016-03-01 16:00 +0100 |
| Subject | Re: [PATCH v3 6/7] ACPI: enable ACPI_DBG2_TABLE on ARM64 |
| Message-ID | <r7TWi-216-19@gated-at.bofh.it> |
| In reply to | #1345764 |
On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
> SBBR mentions DBG2 as a mandatory ACPI table.
> So enable it for ARM64
Why does this have to be opted-in per-arch?
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> ---
> arch/arm64/Kconfig | 1 +
> arch/arm64/kernel/acpi.c | 2 ++
> 2 files changed, 3 insertions(+)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 9dc5209..0b614d4 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -3,6 +3,7 @@ config ARM64
> select ACPI_CCA_REQUIRED if ACPI
> select ACPI_GENERIC_GSI if ACPI
> select ACPI_REDUCED_HARDWARE_ONLY if ACPI
> + select ACPI_DBG2_TABLE if ACPI
> select ARCH_HAS_DEVMEM_IS_ALLOWED
> select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
> select ARCH_HAS_ELF_RANDOMIZE
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index d1ce8e2..3370046 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -208,6 +208,8 @@ void __init acpi_boot_table_init(void)
> pr_err("Failed to init ACPI tables\n");
> if (!param_acpi_force)
> disable_acpi();
> + } else {
> + acpi_probe_device_table(dbg2);
> }
> }
>
>
[toc] | [prev] | [next] | [standalone]
| From | Aleksey Makarov <aleksey.makarov@linaro.org> |
|---|---|
| Date | 2016-03-01 18:10 +0100 |
| Subject | Re: [PATCH v3 6/7] ACPI: enable ACPI_DBG2_TABLE on ARM64 |
| Message-ID | <r7VY6-3zV-37@gated-at.bofh.it> |
| In reply to | #1346677 |
On 03/01/2016 05:52 PM, Peter Hurley wrote:
> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>> SBBR mentions DBG2 as a mandatory ACPI table.
>> So enable it for ARM64
>
> Why does this have to be opted-in per-arch?
DBG2 can specify earlycons. They should start as soon as possible.
On ARM64 it is just after boot tables become available (in arch_init()).
So it has to be per-arch unless we decide not to start earlycons on
DBG2 ports.
>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>> ---
>> arch/arm64/Kconfig | 1 +
>> arch/arm64/kernel/acpi.c | 2 ++
>> 2 files changed, 3 insertions(+)
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 9dc5209..0b614d4 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -3,6 +3,7 @@ config ARM64
>> select ACPI_CCA_REQUIRED if ACPI
>> select ACPI_GENERIC_GSI if ACPI
>> select ACPI_REDUCED_HARDWARE_ONLY if ACPI
>> + select ACPI_DBG2_TABLE if ACPI
>> select ARCH_HAS_DEVMEM_IS_ALLOWED
>> select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
>> select ARCH_HAS_ELF_RANDOMIZE
>> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
>> index d1ce8e2..3370046 100644
>> --- a/arch/arm64/kernel/acpi.c
>> +++ b/arch/arm64/kernel/acpi.c
>> @@ -208,6 +208,8 @@ void __init acpi_boot_table_init(void)
>> pr_err("Failed to init ACPI tables\n");
>> if (!param_acpi_force)
>> disable_acpi();
>> + } else {
>> + acpi_probe_device_table(dbg2);
>> }
>> }
>>
>>
>
[toc] | [prev] | [next] | [standalone]
| From | Peter Hurley <peter@hurleysoftware.com> |
|---|---|
| Date | 2016-03-01 18:30 +0100 |
| Subject | Re: [PATCH v3 6/7] ACPI: enable ACPI_DBG2_TABLE on ARM64 |
| Message-ID | <r7Whs-3Ha-19@gated-at.bofh.it> |
| In reply to | #1346770 |
On 03/01/2016 09:02 AM, Aleksey Makarov wrote:
>
>
> On 03/01/2016 05:52 PM, Peter Hurley wrote:
>> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>>> SBBR mentions DBG2 as a mandatory ACPI table.
>>> So enable it for ARM64
>>
>> Why does this have to be opted-in per-arch?
>
> DBG2 can specify earlycons. They should start as soon as possible.
> On ARM64 it is just after boot tables become available (in arch_init()).
> So it has to be per-arch unless we decide not to start earlycons on
> DBG2 ports.
OF does all that already but the earlycon opt-in is not per-arch.
>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>> ---
>>> arch/arm64/Kconfig | 1 +
>>> arch/arm64/kernel/acpi.c | 2 ++
>>> 2 files changed, 3 insertions(+)
>>>
>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>> index 9dc5209..0b614d4 100644
>>> --- a/arch/arm64/Kconfig
>>> +++ b/arch/arm64/Kconfig
>>> @@ -3,6 +3,7 @@ config ARM64
>>> select ACPI_CCA_REQUIRED if ACPI
>>> select ACPI_GENERIC_GSI if ACPI
>>> select ACPI_REDUCED_HARDWARE_ONLY if ACPI
>>> + select ACPI_DBG2_TABLE if ACPI
>>> select ARCH_HAS_DEVMEM_IS_ALLOWED
>>> select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
>>> select ARCH_HAS_ELF_RANDOMIZE
>>> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
>>> index d1ce8e2..3370046 100644
>>> --- a/arch/arm64/kernel/acpi.c
>>> +++ b/arch/arm64/kernel/acpi.c
>>> @@ -208,6 +208,8 @@ void __init acpi_boot_table_init(void)
>>> pr_err("Failed to init ACPI tables\n");
>>> if (!param_acpi_force)
>>> disable_acpi();
>>> + } else {
>>> + acpi_probe_device_table(dbg2);
>>> }
>>> }
>>>
>>>
>>
[toc] | [prev] | [next] | [standalone]
Page 1 of 2 [1] 2 Next page →
Back to top | Article view | linux.kernel
csiph-web