[U-Boot] [PATCH] ppc4xx: Fix UART baudrate setup by FDT
On ppc4xx platforms __ft_board_setup updates clock-frequency properties of all ns16550 compatible UARTs. This is not a good idea when those UARTs are external discrete UARTs that are not clocked by some internal clocks. So any external clock value in the DTB is overwritten and those UARTs will not be setup correctly by the Linux kernel.
This patch uses the approach from fdt_fixup_ethernet(). Only UART nodes that have a serial* alias are updated. --- common/fdt_support.c | 20 ++++++++++++++++++++ cpu/ppc4xx/fdt.c | 3 ++- include/fdt_support.h | 1 + 3 files changed, 23 insertions(+), 1 deletions(-)
diff --git a/common/fdt_support.c b/common/fdt_support.c index 5a83bca..99d4c3f 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -454,6 +454,26 @@ void fdt_fixup_ethernet(void *fdt) } }
+void fdt_fixup_serial(void *fdt) +{ + int node, i; + char serial[10]; + const char *path; + + node = fdt_path_offset(fdt, "/aliases"); + if (node < 0) + return; + + i = 0; + while (1) { + sprintf(serial, "serial%d", i++); + path = fdt_getprop(fdt, node, serial, NULL); + if (path == NULL) + break; + do_fixup_by_path_u32(fdt, path, "clock-frequency", gd->uart_clk, 1); + } +} + #ifdef CONFIG_HAS_FSL_DR_USB void fdt_fixup_dr_usb(void *blob, bd_t *bd) { diff --git a/cpu/ppc4xx/fdt.c b/cpu/ppc4xx/fdt.c index c55e1cf..5b267d1 100644 --- a/cpu/ppc4xx/fdt.c +++ b/cpu/ppc4xx/fdt.c @@ -134,8 +134,9 @@ void ft_cpu_setup(void *blob, bd_t *bd)
/* * Setup all baudrates for the UARTs + * Note: aliases in the dts are required for this */ - do_fixup_by_compat_u32(blob, "ns16550", "clock-frequency", gd->uart_clk, 1); + fdt_fixup_serial(blob);
/* * Fixup all ethernet nodes diff --git a/include/fdt_support.h b/include/fdt_support.h index 6062df9..bf3df60 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -49,6 +49,7 @@ void do_fixup_by_compat_u32(void *fdt, const char *compat, const char *prop, u32 val, int create); int fdt_fixup_memory(void *blob, u64 start, u64 size); void fdt_fixup_ethernet(void *fdt); +void fdt_fixup_serial(void *fdt); int fdt_find_and_setprop(void *fdt, const char *node, const char *prop, const void *val, int len, int create); void fdt_fixup_qe_firmware(void *fdt);
On ppc4xx platforms __ft_board_setup updates clock-frequency properties of all ns16550 compatible UARTs. This is not a good idea when those UARTs are external discrete UARTs that are not clocked by some internal clocks. So any external clock value in the DTB is overwritten and those UARTs will not be setup correctly by the Linux kernel.
This patch uses the approach from fdt_fixup_ethernet(). Only UART nodes that have a serial* alias are updated.
Signed-off-by: Matthias Fuchs matthias.fuchs@esd-electronics.com --- common/fdt_support.c | 20 ++++++++++++++++++++ cpu/ppc4xx/fdt.c | 3 ++- include/fdt_support.h | 1 + 3 files changed, 23 insertions(+), 1 deletions(-)
diff --git a/common/fdt_support.c b/common/fdt_support.c index 5a83bca..99d4c3f 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -454,6 +454,26 @@ void fdt_fixup_ethernet(void *fdt) } }
+void fdt_fixup_serial(void *fdt) +{ + int node, i; + char serial[10]; + const char *path; + + node = fdt_path_offset(fdt, "/aliases"); + if (node < 0) + return; + + i = 0; + while (1) { + sprintf(serial, "serial%d", i++); + path = fdt_getprop(fdt, node, serial, NULL); + if (path == NULL) + break; + do_fixup_by_path_u32(fdt, path, "clock-frequency", gd->uart_clk, 1); + } +} + #ifdef CONFIG_HAS_FSL_DR_USB void fdt_fixup_dr_usb(void *blob, bd_t *bd) { diff --git a/cpu/ppc4xx/fdt.c b/cpu/ppc4xx/fdt.c index c55e1cf..5b267d1 100644 --- a/cpu/ppc4xx/fdt.c +++ b/cpu/ppc4xx/fdt.c @@ -134,8 +134,9 @@ void ft_cpu_setup(void *blob, bd_t *bd)
/* * Setup all baudrates for the UARTs + * Note: aliases in the dts are required for this */ - do_fixup_by_compat_u32(blob, "ns16550", "clock-frequency", gd->uart_clk, 1); + fdt_fixup_serial(blob);
/* * Fixup all ethernet nodes diff --git a/include/fdt_support.h b/include/fdt_support.h index 6062df9..bf3df60 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -49,6 +49,7 @@ void do_fixup_by_compat_u32(void *fdt, const char *compat, const char *prop, u32 val, int create); int fdt_fixup_memory(void *blob, u64 start, u64 size); void fdt_fixup_ethernet(void *fdt); +void fdt_fixup_serial(void *fdt); int fdt_find_and_setprop(void *fdt, const char *node, const char *prop, const void *val, int len, int create); void fdt_fixup_qe_firmware(void *fdt);
Hi Matthias,
On Friday 02 January 2009, Matthias Fuchs wrote:
On ppc4xx platforms __ft_board_setup updates clock-frequency properties of all ns16550 compatible UARTs. This is not a good idea when those UARTs are external discrete UARTs that are not clocked by some internal clocks. So any external clock value in the DTB is overwritten and those UARTs will not be setup correctly by the Linux kernel.
This patch uses the approach from fdt_fixup_ethernet(). Only UART nodes that have a serial* alias are updated.
Wouldn't it be "better" to check if an external UART clock is configured via CONFIG_SYS_EXT_SERIAL_CLOCK and just use it instead of the calculated internal clock value in this case?
BTW: This patch version is not ppc4xx specific as it touches the common fdt_support.c. So you would need to split this patch if we decide to go your way.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
Hi Matthias,
On Friday 02 January 2009, Matthias Fuchs wrote:
On ppc4xx platforms __ft_board_setup updates clock-frequency properties of all ns16550 compatible UARTs. This is not a good idea when those UARTs are external discrete UARTs that are not clocked by some internal clocks. So any external clock value in the DTB is overwritten and those UARTs will not be setup correctly by the Linux kernel.
This patch uses the approach from fdt_fixup_ethernet(). Only UART nodes that have a serial* alias are updated.
Wouldn't it be "better" to check if an external UART clock is configured via CONFIG_SYS_EXT_SERIAL_CLOCK and just use it instead of the calculated internal clock value in this case?
CONFIG_SYS_EXT_SERIAL_CLOCK is already used to tell U-Boot, that the internal UARTs are clocked by some externally provides clock. In my special case, I have external UART chips connected to a 405EP. Even defining CONFIG_SYS_EXT_SERIAL_CLOCK will bring up an error - the 405EP does not support external UART clock. So this special macro should not be used. On our board (PLU405) there is no reason to touch the external UARTs from U-Boot. So I think the best way would be to leave their fdt description untouched. And the correct clock comes from the device tree.
So which UART's clock would you prefer to be setup to CONFIG_SYS_EXT_SERIAL_CLOCK? All non-alias'd?
(In reality my UARTs are ns16850 compatible. When I put that into the DT, U-Boot does not touch them, but the Linux kernel's drivers/serial/of_serial.c does not handle them. Independant from this I posted a patch to extend of_serial.c for ns16850 to the linux-serial mailing list.)
So the very best way would be to let U-Boot detect internal UARTs. We could do that by adding an additional compatible value to the internal UART nodes:
UART0: serial@ef600300 { device_type = "serial"; compatible = "ns16550", "ibm,uart"; ...
Any other ideas?
BTW: This patch version is not ppc4xx specific as it touches the common fdt_support.c. So you would need to split this patch if we decide to go your way.
I will do so when we decided which way to go.
Matthias
Best regards, Stefan
On Monday 05 January 2009, Matthias Fuchs wrote:
On ppc4xx platforms __ft_board_setup updates clock-frequency properties of all ns16550 compatible UARTs. This is not a good idea when those UARTs are external discrete UARTs that are not clocked by some internal clocks. So any external clock value in the DTB is overwritten and those UARTs will not be setup correctly by the Linux kernel.
This patch uses the approach from fdt_fixup_ethernet(). Only UART nodes that have a serial* alias are updated.
Wouldn't it be "better" to check if an external UART clock is configured via CONFIG_SYS_EXT_SERIAL_CLOCK and just use it instead of the calculated internal clock value in this case?
CONFIG_SYS_EXT_SERIAL_CLOCK is already used to tell U-Boot, that the internal UARTs are clocked by some externally provides clock. In my special case, I have external UART chips connected to a 405EP.
Ups. I misread your original mail. I thought you were talking about external clocks provided to the internal UARTs instad of using external UARTs. Sorry.
Even defining CONFIG_SYS_EXT_SERIAL_CLOCK will bring up an error - the 405EP does not support external UART clock. So this special macro should not be used. On our board (PLU405) there is no reason to touch the external UARTs from U-Boot. So I think the best way would be to leave their fdt description untouched. And the correct clock comes from the device tree.
OK, makes sense to me.
So which UART's clock would you prefer to be setup to CONFIG_SYS_EXT_SERIAL_CLOCK? All non-alias'd?
(In reality my UARTs are ns16850 compatible. When I put that into the DT, U-Boot does not touch them, but the Linux kernel's drivers/serial/of_serial.c does not handle them. Independant from this I posted a patch to extend of_serial.c for ns16850 to the linux-serial mailing list.)
So the very best way would be to let U-Boot detect internal UARTs. We could do that by adding an additional compatible value to the internal UART nodes:
UART0: serial@ef600300 { device_type = "serial"; compatible = "ns16550", "ibm,uart"; ...
That could be done as well. Perhaps it's the "better" solution. You might want to ask on the linuxppc-dev list if such a patch is welcome. If yes, then we should go this way.
Thanks.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
Hi Stefan,
Even defining CONFIG_SYS_EXT_SERIAL_CLOCK will bring up an error - the 405EP does not support external UART clock. So this special macro should not be used. On our board (PLU405) there is no reason to touch the external UARTs from U-Boot. So I think the best way would be to leave their fdt description untouched. And the correct clock comes from the device tree.
OK, makes sense to me.
So the very best way would be to let U-Boot detect internal UARTs. We could do that by adding an additional compatible value to the internal UART nodes:
UART0: serial@ef600300 { device_type = "serial"; compatible = "ns16550", "ibm,uart"; ...That could be done as well. Perhaps it's the "better" solution. You might want to ask on the linuxppc-dev list if such a patch is welcome. If yes, then we should go this way.
Changing U-Boot to check for something different than ns16550 will break all board that have ns16550 in their dt.
The Linux kernel only checks for ns16550 compatible nodes under specific parents (see arch/powerpc/kernel/legacy_serial.c). For 4xx this is only "opb" and not even opb/ebc.
So I think we should do it also this way. In this case we could keep the modification in U-Boots' cpu/ppc4xx/fdt.c.
When you are with me I will try to prepare a patch.
Matthias
Thanks.
Best regards, Stefan
Hi Matthias,
On Tuesday 06 January 2009, Matthias Fuchs wrote:
UART0: serial@ef600300 { device_type = "serial"; compatible = "ns16550", "ibm,uart"; ...That could be done as well. Perhaps it's the "better" solution. You might want to ask on the linuxppc-dev list if such a patch is welcome. If yes, then we should go this way.
Changing U-Boot to check for something different than ns16550 will break all board that have ns16550 in their dt.
Ah, your talking about compatibility problems with older dts files. Understood.
The Linux kernel only checks for ns16550 compatible nodes under specific parents (see arch/powerpc/kernel/legacy_serial.c). For 4xx this is only "opb" and not even opb/ebc.
So I think we should do it also this way. In this case we could keep the modification in U-Boots' cpu/ppc4xx/fdt.c.
When you are with me I will try to prepare a patch.
Yes, this sounds like a good idea. Please go ahead.
Thanks.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
participants (3)
-
Matthias Fuchs -
Matthias Fuchs -
Stefan Roese