Re: [PATCH 2/2] serial: 8250_ingenic: allow to be independent of SERIAL_8250_CONSOLE

From: Paul Gortmaker <paul.gortmaker@windriver.com>
To: Peter Hurley <peter@hurleysoftware.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>,
	linux-serial@vger.kernel.org,
	Paul Burton <paul.burton@imgtec.com>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Jiri Slaby <jslaby@suse.com>, Alan Cox <alan@linux.intel.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Joachim Eastwood <manabian@gmail.com>,
	Krzysztof Kozlowski <k.kozlowski@samsung.com>,
	John Crispin <blogic@openwrt.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Ralf Baechle <ralf@linux-mips.org>
Subject: Re: [PATCH 2/2] serial: 8250_ingenic: allow to be independent of SERIAL_8250_CONSOLE
Date: Mon, 21 Dec 2015 15:15:59 -0500	[thread overview]
Message-ID: <CAP=VYLqu8bkiUrARY3J2gxEpcTiQJSCOQXAkNPiDWgFXiW1E_Q@mail.gmail.com> (raw)
In-Reply-To: <562F8816.4030504@hurleysoftware.com>

On Tue, Oct 27, 2015 at 10:20 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
> On 10/23/2015 09:31 AM, Masahiro Yamada wrote:
>> This UART driver should not depend on the console.  They should be
>> orthogonal.
>>
>> Surround the earlycon code with CONFIG_SERIAL_EARLYCON conditional
>> and rip off "depends on SERIAL_8250_CONSOLE".

The recent merge of this patch to linux-next allows me to build this driver
for ARM, which in turn trips my local "bool Kconfig driver using modular
functions" test failure.

Any thoughts whether people care if this driver gets moved to tristate
vs. having the unused modular code ripped out?

Paul.
--

>
> Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
>
> PS - While reviewing this patch, I noticed another problem in the
> Ingenic earlycon; see below.
>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>> Currently, SERIAL_8250_FSL also depends on SERIAL_8250_CONSOLE,
>> but I doubt its necessity.
>>
>>
>>  drivers/tty/serial/8250/8250_ingenic.c | 2 ++
>>  drivers/tty/serial/8250/Kconfig        | 2 +-
>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250_ingenic.c b/drivers/tty/serial/8250/8250_ingenic.c
>> index 7c1e4be..44633ee 100644
>> --- a/drivers/tty/serial/8250/8250_ingenic.c
>> +++ b/drivers/tty/serial/8250/8250_ingenic.c
>> @@ -34,6 +34,7 @@ struct ingenic_uart_data {
>>
>>  #define UART_FCR_UME BIT(4)
>>
>> +#ifdef CONFIG_SERIAL_EARLYCON
>
>
>>  static struct earlycon_device *early_device;
>
> The earlycon setup should not be saving the earlycon_device *;
> see early_8250serial_write() for how to deref from console to port.
>
> Regards,
> Peter Hurley
>
>
>>  static uint8_t __init early_in(struct uart_port *port, int offset)
>> @@ -126,6 +127,7 @@ OF_EARLYCON_DECLARE(jz4775_uart, "ingenic,jz4775-uart",
>>  EARLYCON_DECLARE(jz4780_uart, ingenic_early_console_setup);
>>  OF_EARLYCON_DECLARE(jz4780_uart, "ingenic,jz4780-uart",
>>                   ingenic_early_console_setup);
>> +#endif /* CONFIG_SERIAL_EARLYCON */
>>
>>  static void ingenic_uart_serial_out(struct uart_port *p, int offset, int value)
>>  {
>> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
>> index 960da95..6f62934 100644
>> --- a/drivers/tty/serial/8250/Kconfig
>> +++ b/drivers/tty/serial/8250/Kconfig
>> @@ -360,7 +360,7 @@ config SERIAL_8250_UNIPHIER
>>
>>  config SERIAL_8250_INGENIC
>>       bool "Support for Ingenic SoC serial ports"
>> -     depends on SERIAL_8250_CONSOLE && OF_FLATTREE
>> +     depends on OF_FLATTREE
>>       select LIBFDT
>>       help
>>         If you have a system using an Ingenic SoC and wish to make use of
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-serial" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html