Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port

[Peter Hurley <peter@hurleysoftware.com>]

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
>