linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] serial: 8250_ingenic: delete redundant "select SERIAL_EARLYCON"
@ 2015-10-23 13:31 Masahiro Yamada
  2015-10-23 13:31 ` [PATCH 2/2] serial: 8250_ingenic: allow to be independent of SERIAL_8250_CONSOLE Masahiro Yamada
  2015-10-27 14:15 ` [PATCH 1/2] serial: 8250_ingenic: delete redundant "select SERIAL_EARLYCON" Peter Hurley
  0 siblings, 2 replies; 5+ messages in thread
From: Masahiro Yamada @ 2015-10-23 13:31 UTC (permalink / raw)
  To: linux-serial
  Cc: Paul Burton, Peter Hurley, Masahiro Yamada,
	Sebastian Andrzej Siewior, Jiri Slaby, linux-kernel,
	Andy Shevchenko, Joachim Eastwood, John Crispin,
	Greg Kroah-Hartman, Ralf Baechle

SERIAL_8250_INGENIC depends on SERIAL_8250_CONSOLE, which already
selects SERIAL_EARLYCON.

This line is redundant.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/tty/serial/8250/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index e1de118..960da95 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -362,7 +362,6 @@ config SERIAL_8250_INGENIC
 	bool "Support for Ingenic SoC serial ports"
 	depends on SERIAL_8250_CONSOLE && OF_FLATTREE
 	select LIBFDT
-	select SERIAL_EARLYCON
 	help
 	  If you have a system using an Ingenic SoC and wish to make use of
 	  its UARTs, say Y to this option. If unsure, say N.
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] serial: 8250_ingenic: allow to be independent of SERIAL_8250_CONSOLE
  2015-10-23 13:31 [PATCH 1/2] serial: 8250_ingenic: delete redundant "select SERIAL_EARLYCON" Masahiro Yamada
@ 2015-10-23 13:31 ` Masahiro Yamada
  2015-10-27 14:20   ` Peter Hurley
  2015-10-27 14:15 ` [PATCH 1/2] serial: 8250_ingenic: delete redundant "select SERIAL_EARLYCON" Peter Hurley
  1 sibling, 1 reply; 5+ messages in thread
From: Masahiro Yamada @ 2015-10-23 13:31 UTC (permalink / raw)
  To: linux-serial
  Cc: Paul Burton, Peter Hurley, Masahiro Yamada,
	Sebastian Andrzej Siewior, Jiri Slaby, Alan Cox, linux-kernel,
	Andy Shevchenko, Joachim Eastwood, Krzysztof Kozlowski,
	John Crispin, Greg Kroah-Hartman, Ralf Baechle

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".

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;
 
 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
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] serial: 8250_ingenic: delete redundant "select SERIAL_EARLYCON"
  2015-10-23 13:31 [PATCH 1/2] serial: 8250_ingenic: delete redundant "select SERIAL_EARLYCON" Masahiro Yamada
  2015-10-23 13:31 ` [PATCH 2/2] serial: 8250_ingenic: allow to be independent of SERIAL_8250_CONSOLE Masahiro Yamada
@ 2015-10-27 14:15 ` Peter Hurley
  1 sibling, 0 replies; 5+ messages in thread
From: Peter Hurley @ 2015-10-27 14:15 UTC (permalink / raw)
  To: Masahiro Yamada, linux-serial
  Cc: Paul Burton, Sebastian Andrzej Siewior, Jiri Slaby, linux-kernel,
	Andy Shevchenko, Joachim Eastwood, John Crispin,
	Greg Kroah-Hartman, Ralf Baechle

On 10/23/2015 09:31 AM, Masahiro Yamada wrote:
> SERIAL_8250_INGENIC depends on SERIAL_8250_CONSOLE, which already
> selects SERIAL_EARLYCON.
> 
> This line is redundant.

Reviewed-by: Peter Hurley <peter@hurleysoftware.com>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] serial: 8250_ingenic: allow to be independent of SERIAL_8250_CONSOLE
  2015-10-23 13:31 ` [PATCH 2/2] serial: 8250_ingenic: allow to be independent of SERIAL_8250_CONSOLE Masahiro Yamada
@ 2015-10-27 14:20   ` Peter Hurley
  2015-12-21 20:15     ` Paul Gortmaker
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Hurley @ 2015-10-27 14:20 UTC (permalink / raw)
  To: Masahiro Yamada, linux-serial
  Cc: Paul Burton, Sebastian Andrzej Siewior, Jiri Slaby, Alan Cox,
	linux-kernel, Andy Shevchenko, Joachim Eastwood,
	Krzysztof Kozlowski, John Crispin, Greg Kroah-Hartman,
	Ralf Baechle

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".

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
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] serial: 8250_ingenic: allow to be independent of SERIAL_8250_CONSOLE
  2015-10-27 14:20   ` Peter Hurley
@ 2015-12-21 20:15     ` Paul Gortmaker
  0 siblings, 0 replies; 5+ messages in thread
From: Paul Gortmaker @ 2015-12-21 20:15 UTC (permalink / raw)
  To: Peter Hurley
  Cc: Masahiro Yamada, linux-serial, Paul Burton,
	Sebastian Andrzej Siewior, Jiri Slaby, Alan Cox, LKML,
	Andy Shevchenko, Joachim Eastwood, Krzysztof Kozlowski,
	John Crispin, Greg Kroah-Hartman, Ralf Baechle

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-12-21 20:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-23 13:31 [PATCH 1/2] serial: 8250_ingenic: delete redundant "select SERIAL_EARLYCON" Masahiro Yamada
2015-10-23 13:31 ` [PATCH 2/2] serial: 8250_ingenic: allow to be independent of SERIAL_8250_CONSOLE Masahiro Yamada
2015-10-27 14:20   ` Peter Hurley
2015-12-21 20:15     ` Paul Gortmaker
2015-10-27 14:15 ` [PATCH 1/2] serial: 8250_ingenic: delete redundant "select SERIAL_EARLYCON" Peter Hurley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).