[PATCH v3 0/2] Early 8250 fixlets

[PATCH v3 0/2] Early 8250 fixlets

[Vineet Gupta]

Hi,

This series addresses some of the issues seen on early 8250 uart when
working with ARC SDP platform.

Changes since v2 [2]
	-Patch 2/3 dropped (see next bullet)
	-Remove the weak binding scheme and make BASE_BAUD simply call arch
	 function					[Peter Hurley]
	-Verified that IER optimization 1/3 is per line, not char - hence
	 Changelog updated				[Peter Hurley]

Changes since v1 [1]
	-Rebased to linux-next of 20150105
	-Patch 3/3 SNAFU fixed (arch over-ride was not correct due to mix up
	 of orig work based off 3.13 vs. 3.19+ based linux next submission
	 (different core earlycon code)
Thx,
-Vineet

[2] http://www.spinics.net/linux/lists/kernel/msg1896349.html
[1] http://www.spinics.net/lists/kernel/msg1893299.html

Vineet Gupta (2):
  serial: 8250_early: optimize early 8250 uart
  ARC: Dynamically determine BASE_BAUD from DeviceTree

 arch/arc/include/asm/serial.h        | 23 +++++------------------
 arch/arc/kernel/devtree.c            | 24 ++++++++++++++++++++++++
 drivers/tty/serial/8250/8250_early.c |  7 +++++--
 3 files changed, 34 insertions(+), 20 deletions(-)

-- 
1.9.1

[PATCH v3 1/2] serial: 8250_early: optimize early 8250 uart

[Vineet Gupta]

In early 8250, IER is already zero so no point in writing this - twice
per line

This helped improve the SystemC model based ARC OSCI platform

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: linux-serial@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Rob Herring <robh@kernel.org>
Cc: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
 drivers/tty/serial/8250/8250_early.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index 4858b8a99d3b..ce2a8abd1f54 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -95,13 +95,16 @@ static void __init early_serial8250_write(struct console *console,
 
 	/* Save the IER and disable interrupts */
 	ier = serial8250_early_in(port, UART_IER);
-	serial8250_early_out(port, UART_IER, 0);
+	if (ier)
+		serial8250_early_out(port, UART_IER, 0);
 
 	uart_console_write(port, s, count, serial_putc);
 
 	/* Wait for transmitter to become empty and restore the IER */
 	wait_for_xmitr(port);
-	serial8250_early_out(port, UART_IER, ier);
+
+	if (ier)
+		serial8250_early_out(port, UART_IER, ier);
 }
 
 static unsigned int __init probe_baud(struct uart_port *port)
-- 
1.9.1

[PATCH v3 2/2] ARC: Dynamically determine BASE_BAUD from DeviceTree

[Vineet Gupta]

This resolves the BASE_BAUD issue in Multiplatform images

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: linux-serial@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Rob Herring <robh@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
 arch/arc/include/asm/serial.h | 23 +++++------------------
 arch/arc/kernel/devtree.c     | 24 ++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/arch/arc/include/asm/serial.h b/arch/arc/include/asm/serial.h
index 602b0970a764..744a6ae15754 100644
--- a/arch/arc/include/asm/serial.h
+++ b/arch/arc/include/asm/serial.h
@@ -10,26 +10,13 @@
 #define _ASM_ARC_SERIAL_H
 
 /*
- * early-8250 requires BASE_BAUD to be defined and includes this header.
- * We put in a typical value:
- * 	(core clk / 16) - i.e. UART samples 16 times per sec.
- * Athough in multi-platform-image this might not work, specially if the
- * clk driving the UART is different.
- * We can't use DeviceTree as this is typically for early serial.
+ * early 8250 (now earlycon) requires BASE_BAUD to be defined in this header.
+ * However to still determine it dynamically (for multi-platform images)
+ * we do this in a helper by parsing the FDT early
  */
 
-#include <asm/clk.h>
+extern unsigned int __init arc_early_base_baud(void);
 
-#define BASE_BAUD	(arc_get_core_freq() / 16)
-
-/*
- * This is definitely going to break early 8250 consoles on multi-platform
- * images but hey, it won't add any code complexity for a debug feature of
- * one broken driver.
- */
-#ifdef CONFIG_ARC_PLAT_TB10X
-#undef BASE_BAUD
-#define BASE_BAUD	(arc_get_core_freq() / 16 / 3)
-#endif
+#define BASE_BAUD	arc_early_base_baud()
 
 #endif /* _ASM_ARC_SERIAL_H */
diff --git a/arch/arc/kernel/devtree.c b/arch/arc/kernel/devtree.c
index fffdb5e41b20..69a790cd9b6e 100644
--- a/arch/arc/kernel/devtree.c
+++ b/arch/arc/kernel/devtree.c
@@ -17,6 +17,28 @@
 #include <asm/clk.h>
 #include <asm/mach_desc.h>
 
+#ifdef CONFIG_SERIAL_8250_CONSOLE
+
+static unsigned int arc_base_baud;
+
+unsigned int __init arc_early_base_baud(void)
+{
+	return arc_base_baud/16;
+}
+
+static void __init arc_set_early_base_baud(unsigned long dt_root)
+{
+	unsigned int core_clk = arc_get_core_freq();
+
+	if (of_flat_dt_is_compatible(dt_root, "abilis,arc-tb10x"))
+		arc_base_baud = core_clk/3;
+	else
+		arc_base_baud = core_clk;
+}
+#else
+#define arc_set_early_base_baud(dt_root)
+#endif
+
 static const void * __init arch_get_next_mach(const char *const **match)
 {
 	static const struct machine_desc *mdesc = __arch_info_begin;
@@ -56,5 +78,7 @@ const struct machine_desc * __init setup_machine_fdt(void *dt)
 	if (clk)
 		arc_set_core_freq(of_read_ulong(clk, len/4));
 
+	arc_set_early_base_baud(dt_root);
+
 	return mdesc;
 }
-- 
1.9.1

Re: [PATCH v3 2/2] ARC: Dynamically determine BASE_BAUD from DeviceTree

[Peter Hurley]

On 01/06/2015 06:29 AM, Vineet Gupta wrote:
> This resolves the BASE_BAUD issue in Multiplatform images

This commit log could be improved. Something like:

8250 earlycon is broken on multi-platform ARC because the UART clk
value (BASE_BAUD) is fixed at build time.

Instead, determine the appropriate UART clk at runtime; parse the
devicetree early for platforms requiring alternate UART clk values
(currently only the TB10X platform).

Regards,
Peter Hurley

> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jiri Slaby <jslaby@suse.cz>
> Cc: linux-serial@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Rob Herring <robh@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Peter Hurley <peter@hurleysoftware.com>
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> ---
>  arch/arc/include/asm/serial.h | 23 +++++------------------
>  arch/arc/kernel/devtree.c     | 24 ++++++++++++++++++++++++
>  2 files changed, 29 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/arc/include/asm/serial.h b/arch/arc/include/asm/serial.h
> index 602b0970a764..744a6ae15754 100644
> --- a/arch/arc/include/asm/serial.h
> +++ b/arch/arc/include/asm/serial.h
> @@ -10,26 +10,13 @@
>  #define _ASM_ARC_SERIAL_H
>  
>  /*
> - * early-8250 requires BASE_BAUD to be defined and includes this header.
> - * We put in a typical value:
> - * 	(core clk / 16) - i.e. UART samples 16 times per sec.
> - * Athough in multi-platform-image this might not work, specially if the
> - * clk driving the UART is different.
> - * We can't use DeviceTree as this is typically for early serial.
> + * early 8250 (now earlycon) requires BASE_BAUD to be defined in this header.
> + * However to still determine it dynamically (for multi-platform images)
> + * we do this in a helper by parsing the FDT early
>   */
>  
> -#include <asm/clk.h>
> +extern unsigned int __init arc_early_base_baud(void);
>  
> -#define BASE_BAUD	(arc_get_core_freq() / 16)
> -
> -/*
> - * This is definitely going to break early 8250 consoles on multi-platform
> - * images but hey, it won't add any code complexity for a debug feature of
> - * one broken driver.
> - */
> -#ifdef CONFIG_ARC_PLAT_TB10X
> -#undef BASE_BAUD
> -#define BASE_BAUD	(arc_get_core_freq() / 16 / 3)
> -#endif
> +#define BASE_BAUD	arc_early_base_baud()
>  
>  #endif /* _ASM_ARC_SERIAL_H */
> diff --git a/arch/arc/kernel/devtree.c b/arch/arc/kernel/devtree.c
> index fffdb5e41b20..69a790cd9b6e 100644
> --- a/arch/arc/kernel/devtree.c
> +++ b/arch/arc/kernel/devtree.c
> @@ -17,6 +17,28 @@
>  #include <asm/clk.h>
>  #include <asm/mach_desc.h>
>  
> +#ifdef CONFIG_SERIAL_8250_CONSOLE
> +
> +static unsigned int arc_base_baud;
> +
> +unsigned int __init arc_early_base_baud(void)
> +{
> +	return arc_base_baud/16;
> +}
> +
> +static void __init arc_set_early_base_baud(unsigned long dt_root)
> +{
> +	unsigned int core_clk = arc_get_core_freq();
> +
> +	if (of_flat_dt_is_compatible(dt_root, "abilis,arc-tb10x"))
> +		arc_base_baud = core_clk/3;
> +	else
> +		arc_base_baud = core_clk;
> +}
> +#else
> +#define arc_set_early_base_baud(dt_root)
> +#endif
> +
>  static const void * __init arch_get_next_mach(const char *const **match)
>  {
>  	static const struct machine_desc *mdesc = __arch_info_begin;
> @@ -56,5 +78,7 @@ const struct machine_desc * __init setup_machine_fdt(void *dt)
>  	if (clk)
>  		arc_set_core_freq(of_read_ulong(clk, len/4));
>  
> +	arc_set_early_base_baud(dt_root);
> +
>  	return mdesc;
>  }
> 

[Patch v4] ARC: Dynamically determine BASE_BAUD from DeviceTree

[Vineet Gupta]

8250 earlycon is broken on multi-platform ARC because the UART clk
value (BASE_BAUD) is fixed at build time.

Instead, determine the appropriate UART clk at runtime; parse the
devicetree early for platforms requiring alternate UART clk values
(currently only the TB10X platform).

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: linux-serial@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Rob Herring <robh@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
 arch/arc/include/asm/serial.h | 23 +++++------------------
 arch/arc/kernel/devtree.c     | 24 ++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/arch/arc/include/asm/serial.h b/arch/arc/include/asm/serial.h
index 602b0970a764..744a6ae15754 100644
--- a/arch/arc/include/asm/serial.h
+++ b/arch/arc/include/asm/serial.h
@@ -10,26 +10,13 @@
 #define _ASM_ARC_SERIAL_H
 
 /*
- * early-8250 requires BASE_BAUD to be defined and includes this header.
- * We put in a typical value:
- * 	(core clk / 16) - i.e. UART samples 16 times per sec.
- * Athough in multi-platform-image this might not work, specially if the
- * clk driving the UART is different.
- * We can't use DeviceTree as this is typically for early serial.
+ * early 8250 (now earlycon) requires BASE_BAUD to be defined in this header.
+ * However to still determine it dynamically (for multi-platform images)
+ * we do this in a helper by parsing the FDT early
  */
 
-#include <asm/clk.h>
+extern unsigned int __init arc_early_base_baud(void);
 
-#define BASE_BAUD	(arc_get_core_freq() / 16)
-
-/*
- * This is definitely going to break early 8250 consoles on multi-platform
- * images but hey, it won't add any code complexity for a debug feature of
- * one broken driver.
- */
-#ifdef CONFIG_ARC_PLAT_TB10X
-#undef BASE_BAUD
-#define BASE_BAUD	(arc_get_core_freq() / 16 / 3)
-#endif
+#define BASE_BAUD	arc_early_base_baud()
 
 #endif /* _ASM_ARC_SERIAL_H */
diff --git a/arch/arc/kernel/devtree.c b/arch/arc/kernel/devtree.c
index fffdb5e41b20..69a790cd9b6e 100644
--- a/arch/arc/kernel/devtree.c
+++ b/arch/arc/kernel/devtree.c
@@ -17,6 +17,28 @@
 #include <asm/clk.h>
 #include <asm/mach_desc.h>
 
+#ifdef CONFIG_SERIAL_8250_CONSOLE
+
+static unsigned int arc_base_baud;
+
+unsigned int __init arc_early_base_baud(void)
+{
+	return arc_base_baud/16;
+}
+
+static void __init arc_set_early_base_baud(unsigned long dt_root)
+{
+	unsigned int core_clk = arc_get_core_freq();
+
+	if (of_flat_dt_is_compatible(dt_root, "abilis,arc-tb10x"))
+		arc_base_baud = core_clk/3;
+	else
+		arc_base_baud = core_clk;
+}
+#else
+#define arc_set_early_base_baud(dt_root)
+#endif
+
 static const void * __init arch_get_next_mach(const char *const **match)
 {
 	static const struct machine_desc *mdesc = __arch_info_begin;
@@ -56,5 +78,7 @@ const struct machine_desc * __init setup_machine_fdt(void *dt)
 	if (clk)
 		arc_set_core_freq(of_read_ulong(clk, len/4));
 
+	arc_set_early_base_baud(dt_root);
+
 	return mdesc;
 }
-- 
1.9.1

Re: [Patch v4] ARC: Dynamically determine BASE_BAUD from DeviceTree

[Peter Hurley]

On 01/06/2015 08:59 AM, Vineet Gupta wrote:
> 8250 earlycon is broken on multi-platform ARC because the UART clk
> value (BASE_BAUD) is fixed at build time.
> 
> Instead, determine the appropriate UART clk at runtime; parse the
> devicetree early for platforms requiring alternate UART clk values
> (currently only the TB10X platform).

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

Re: [Patch v4] ARC: Dynamically determine BASE_BAUD from DeviceTree

[Rob Herring]

On Tue, Jan 6, 2015 at 7:59 AM, Vineet Gupta <Vineet.Gupta1@synopsys.com> wrote:
> 8250 earlycon is broken on multi-platform ARC because the UART clk
> value (BASE_BAUD) is fixed at build time.

Note that it should only be broken if you rely on the kernel to init
the uart. It should work if the boot loader configured the UART and
you don't specify the baudrate.

> Instead, determine the appropriate UART clk at runtime; parse the
> devicetree early for platforms requiring alternate UART clk values
> (currently only the TB10X platform).
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jiri Slaby <jslaby@suse.cz>
> Cc: linux-serial@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Rob Herring <robh@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Peter Hurley <peter@hurleysoftware.com>
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> ---
>  arch/arc/include/asm/serial.h | 23 +++++------------------
>  arch/arc/kernel/devtree.c     | 24 ++++++++++++++++++++++++
>  2 files changed, 29 insertions(+), 18 deletions(-)
>
> diff --git a/arch/arc/include/asm/serial.h b/arch/arc/include/asm/serial.h
> index 602b0970a764..744a6ae15754 100644
> --- a/arch/arc/include/asm/serial.h
> +++ b/arch/arc/include/asm/serial.h
> @@ -10,26 +10,13 @@
>  #define _ASM_ARC_SERIAL_H
>
>  /*
> - * early-8250 requires BASE_BAUD to be defined and includes this header.
> - * We put in a typical value:
> - *     (core clk / 16) - i.e. UART samples 16 times per sec.
> - * Athough in multi-platform-image this might not work, specially if the
> - * clk driving the UART is different.
> - * We can't use DeviceTree as this is typically for early serial.
> + * early 8250 (now earlycon) requires BASE_BAUD to be defined in this header.
> + * However to still determine it dynamically (for multi-platform images)
> + * we do this in a helper by parsing the FDT early
>   */
>
> -#include <asm/clk.h>
> +extern unsigned int __init arc_early_base_baud(void);
>
> -#define BASE_BAUD      (arc_get_core_freq() / 16)
> -
> -/*
> - * This is definitely going to break early 8250 consoles on multi-platform
> - * images but hey, it won't add any code complexity for a debug feature of
> - * one broken driver.
> - */
> -#ifdef CONFIG_ARC_PLAT_TB10X
> -#undef BASE_BAUD
> -#define BASE_BAUD      (arc_get_core_freq() / 16 / 3)
> -#endif
> +#define BASE_BAUD      arc_early_base_baud()
>
>  #endif /* _ASM_ARC_SERIAL_H */
> diff --git a/arch/arc/kernel/devtree.c b/arch/arc/kernel/devtree.c
> index fffdb5e41b20..69a790cd9b6e 100644
> --- a/arch/arc/kernel/devtree.c
> +++ b/arch/arc/kernel/devtree.c
> @@ -17,6 +17,28 @@
>  #include <asm/clk.h>
>  #include <asm/mach_desc.h>
>
> +#ifdef CONFIG_SERIAL_8250_CONSOLE
> +
> +static unsigned int arc_base_baud;

This can be initdata.

> +unsigned int __init arc_early_base_baud(void)
> +{
> +       return arc_base_baud/16;
> +}
> +
> +static void __init arc_set_early_base_baud(unsigned long dt_root)
> +{
> +       unsigned int core_clk = arc_get_core_freq();
> +
> +       if (of_flat_dt_is_compatible(dt_root, "abilis,arc-tb10x"))
> +               arc_base_baud = core_clk/3;

How many platforms do you expect this to be? This scales to maybe 10,
but not to 100 platforms. It certainly would not scale for ARM. If it
is a lot, then we need to find a generic way to describe this in DT.
For example, perhaps we require the uart node to have a
clock-frequency property or add a chosen property. You could make this
part of the machine descriptor instead, but that wouldn't be my first
choice.

Rob

Re: [Patch v4] ARC: Dynamically determine BASE_BAUD from DeviceTree

[Vineet Gupta]

On Tuesday 06 January 2015 08:08 PM, Rob Herring wrote:
> On Tue, Jan 6, 2015 at 7:59 AM, Vineet Gupta <Vineet.Gupta1@synopsys.com> wrote:
>> 8250 earlycon is broken on multi-platform ARC because the UART clk
>> value (BASE_BAUD) is fixed at build time.
> Note that it should only be broken if you rely on the kernel to init
> the uart. It should work if the boot loader configured the UART and
> you don't specify the baudrate.

But even if uboot set it up right - when the early 8250 is enabled in kernel, it
will try to apply BASE_BAUD to re init it again.
So if that doesn't match platform expectations, early prints will be garbled. Am I
missing something here?

>>  #include <asm/mach_desc.h>
>>
>> +#ifdef CONFIG_SERIAL_8250_CONSOLE
>> +
>> +static unsigned int arc_base_baud;
> This can be initdata.

OK !

>
>> +unsigned int __init arc_early_base_baud(void)
>> +{
>> +       return arc_base_baud/16;
>> +}
>> +
>> +static void __init arc_set_early_base_baud(unsigned long dt_root)
>> +{
>> +       unsigned int core_clk = arc_get_core_freq();
>> +
>> +       if (of_flat_dt_is_compatible(dt_root, "abilis,arc-tb10x"))
>> +               arc_base_baud = core_clk/3;
> How many platforms do you expect this to be? This scales to maybe 10,
> but not to 100 platforms. It certainly would not scale for ARM.

The need for this came from our internal development of 2 new platform based on
new ARCv2 ISA.
They are slated to hit mainline sometime this year. Hence this is in a sense prep
patch and converts the
only existing upstream user of BASE_BAUD for ARC (tb10x). For ARC atleast I don't
expect the scalability issue - yet :-)
ARM doesn't seem define asm/serial.h (BASE_BAUD) and it probably works OK with the
stub value in asm-generic ?


> If it
> is a lot, then we need to find a generic way to describe this in DT.
> For example, perhaps we require the uart node to have a
> clock-frequency property or add a chosen property. 

Yeah that would indeed be cleanest way. But I think we shd be ok for now.

> You could make this
> part of the machine descriptor instead, but that wouldn't be my first
> choice.

Me neither !

Thx,
-Vineet

>
> Rob
>

Re: [Patch v4] ARC: Dynamically determine BASE_BAUD from DeviceTree

[Rob Herring]

On Wed, Jan 7, 2015 at 12:36 AM, Vineet Gupta
<Vineet.Gupta1@synopsys.com> wrote:
> On Tuesday 06 January 2015 08:08 PM, Rob Herring wrote:
>> On Tue, Jan 6, 2015 at 7:59 AM, Vineet Gupta <Vineet.Gupta1@synopsys.com> wrote:
>>> 8250 earlycon is broken on multi-platform ARC because the UART clk
>>> value (BASE_BAUD) is fixed at build time.
>> Note that it should only be broken if you rely on the kernel to init
>> the uart. It should work if the boot loader configured the UART and
>> you don't specify the baudrate.
>
> But even if uboot set it up right - when the early 8250 is enabled in kernel, it
> will try to apply BASE_BAUD to re init it again.
> So if that doesn't match platform expectations, early prints will be garbled. Am I
> missing something here?

I could be wrong as I've only tested under QEMU which is why I'm
curious if you find it doesn't work for you. If you do not specify the
baud rate, the code will detect it by reading the divider registers.
Of course it will get a crap baud rate if BASE_BAUD is wrong, but it
just ends up setting the divider to the same value. I suppose there
could be a problem later on when the full driver loads if the wrong
baud rate is used then with a correct clock rate.

>>>  #include <asm/mach_desc.h>
>>>
>>> +#ifdef CONFIG_SERIAL_8250_CONSOLE
>>> +
>>> +static unsigned int arc_base_baud;
>> This can be initdata.
>
> OK !
>
>>
>>> +unsigned int __init arc_early_base_baud(void)
>>> +{
>>> +       return arc_base_baud/16;
>>> +}
>>> +
>>> +static void __init arc_set_early_base_baud(unsigned long dt_root)
>>> +{
>>> +       unsigned int core_clk = arc_get_core_freq();
>>> +
>>> +       if (of_flat_dt_is_compatible(dt_root, "abilis,arc-tb10x"))
>>> +               arc_base_baud = core_clk/3;
>> How many platforms do you expect this to be? This scales to maybe 10,
>> but not to 100 platforms. It certainly would not scale for ARM.
>
> The need for this came from our internal development of 2 new platform based on
> new ARCv2 ISA.
> They are slated to hit mainline sometime this year. Hence this is in a sense prep
> patch and converts the
> only existing upstream user of BASE_BAUD for ARC (tb10x). For ARC atleast I don't
> expect the scalability issue - yet :-)
> ARM doesn't seem define asm/serial.h (BASE_BAUD) and it probably works OK with the
> stub value in asm-generic ?

ARM does not yet have the necessary early fixmap support in mainline
to enable this feature yet.

Rob

>> If it
>> is a lot, then we need to find a generic way to describe this in DT.
>> For example, perhaps we require the uart node to have a
>> clock-frequency property or add a chosen property.
>
> Yeah that would indeed be cleanest way. But I think we shd be ok for now.
>
>> You could make this
>> part of the machine descriptor instead, but that wouldn't be my first
>> choice.
>
> Me neither !
>
> Thx,
> -Vineet
>
>>
>> Rob
>>
>

[PATCH v5] ARC: Dynamically determine BASE_BAUD from DeviceTree

[Vineet Gupta]

8250 earlycon is broken on multi-platform ARC because the UART clk
value (BASE_BAUD) is fixed at build time.

Instead, determine the appropriate UART clk at runtime; parse the
devicetree early for platforms requiring alternate UART clk values
(currently only the TB10X platform).

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: linux-serial@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Rob Herring <robh@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
 arch/arc/include/asm/serial.h | 23 +++++------------------
 arch/arc/kernel/devtree.c     | 24 ++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/arch/arc/include/asm/serial.h b/arch/arc/include/asm/serial.h
index 602b0970a764..744a6ae15754 100644
--- a/arch/arc/include/asm/serial.h
+++ b/arch/arc/include/asm/serial.h
@@ -10,26 +10,13 @@
 #define _ASM_ARC_SERIAL_H
 
 /*
- * early-8250 requires BASE_BAUD to be defined and includes this header.
- * We put in a typical value:
- * 	(core clk / 16) - i.e. UART samples 16 times per sec.
- * Athough in multi-platform-image this might not work, specially if the
- * clk driving the UART is different.
- * We can't use DeviceTree as this is typically for early serial.
+ * early 8250 (now earlycon) requires BASE_BAUD to be defined in this header.
+ * However to still determine it dynamically (for multi-platform images)
+ * we do this in a helper by parsing the FDT early
  */
 
-#include <asm/clk.h>
+extern unsigned int __init arc_early_base_baud(void);
 
-#define BASE_BAUD	(arc_get_core_freq() / 16)
-
-/*
- * This is definitely going to break early 8250 consoles on multi-platform
- * images but hey, it won't add any code complexity for a debug feature of
- * one broken driver.
- */
-#ifdef CONFIG_ARC_PLAT_TB10X
-#undef BASE_BAUD
-#define BASE_BAUD	(arc_get_core_freq() / 16 / 3)
-#endif
+#define BASE_BAUD	arc_early_base_baud()
 
 #endif /* _ASM_ARC_SERIAL_H */
diff --git a/arch/arc/kernel/devtree.c b/arch/arc/kernel/devtree.c
index fffdb5e41b20..5036d4c06996 100644
--- a/arch/arc/kernel/devtree.c
+++ b/arch/arc/kernel/devtree.c
@@ -17,6 +17,28 @@
 #include <asm/clk.h>
 #include <asm/mach_desc.h>
 
+#ifdef CONFIG_SERIAL_8250_CONSOLE
+
+static unsigned int __initdata arc_base_baud;
+
+unsigned int __init arc_early_base_baud(void)
+{
+	return arc_base_baud/16;
+}
+
+static void __init arc_set_early_base_baud(unsigned long dt_root)
+{
+	unsigned int core_clk = arc_get_core_freq();
+
+	if (of_flat_dt_is_compatible(dt_root, "abilis,arc-tb10x"))
+		arc_base_baud = core_clk/3;
+	else
+		arc_base_baud = core_clk;
+}
+#else
+#define arc_set_early_base_baud(dt_root)
+#endif
+
 static const void * __init arch_get_next_mach(const char *const **match)
 {
 	static const struct machine_desc *mdesc = __arch_info_begin;
@@ -56,5 +78,7 @@ const struct machine_desc * __init setup_machine_fdt(void *dt)
 	if (clk)
 		arc_set_core_freq(of_read_ulong(clk, len/4));
 
+	arc_set_early_base_baud(dt_root);
+
 	return mdesc;
 }
-- 
1.9.1

Re: [PATCH v5] ARC: Dynamically determine BASE_BAUD from DeviceTree

[Vineet Gupta]

Hi Greg,

On Saturday 10 January 2015 04:56 PM, Vineet Gupta wrote:
> 8250 earlycon is broken on multi-platform ARC because the UART clk
> value (BASE_BAUD) is fixed at build time.
> 
> Instead, determine the appropriate UART clk at runtime; parse the
> devicetree early for platforms requiring alternate UART clk values
> (currently only the TB10X platform).
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jiri Slaby <jslaby@suse.cz>
> Cc: linux-serial@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Rob Herring <robh@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> ---
>  arch/arc/include/asm/serial.h | 23 +++++------------------
>  arch/arc/kernel/devtree.c     | 24 ++++++++++++++++++++++++

This being all in arch code, do you want me to take this in via ARC tree ?

Thx,
-Vineet

Re: [PATCH v5] ARC: Dynamically determine BASE_BAUD from DeviceTree

[Greg KH]

On Thu, Jan 15, 2015 at 09:49:12AM +0530, Vineet Gupta wrote:
> Hi Greg,
> 
> On Saturday 10 January 2015 04:56 PM, Vineet Gupta wrote:
> > 8250 earlycon is broken on multi-platform ARC because the UART clk
> > value (BASE_BAUD) is fixed at build time.
> > 
> > Instead, determine the appropriate UART clk at runtime; parse the
> > devicetree early for platforms requiring alternate UART clk values
> > (currently only the TB10X platform).
> > 
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Jiri Slaby <jslaby@suse.cz>
> > Cc: linux-serial@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
> > Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> > ---
> >  arch/arc/include/asm/serial.h | 23 +++++------------------
> >  arch/arc/kernel/devtree.c     | 24 ++++++++++++++++++++++++
> 
> This being all in arch code, do you want me to take this in via ARC tree ?

Sure, that's fine with me:

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>