[U-Boot] [PATCH V2] ppc4xx: Fix UART baudrate setup by FDT

[Stefan Roese]

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 at 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 at denx.de
=====================================================================