Re: [PATCH 3/3] [POWERPC] Xilinx: boot support for Xilinx uart 16550.

["Grant Likely" <grant.likely@secretlab.ca>]

On Thu, Mar 20, 2008 at 8:43 AM, John Linn <john.linn@xilinx.com> wrote:
> The Xilinx 16550 uart core is not a standard 16550, because it uses
>  word-based addressing rather than byte-based addressing.  As a result,
>  it is not compatible with the open firmware 'ns16550' compatible
>  binding.
>
>  This code adds the Xilinx uart 16550 to the serial console
>  of the boot. A new initialization function for the Xilinx 16550 uart
>  is added to the ns16550 driver. It sets up the correct register base
>  and regshift properties specific to the Xilinx 16550.
>
>  Signed-off-by: John Linn <john.linn@xilinx.com>
>  --- a/arch/powerpc/boot/ops.h
>  +++ b/arch/powerpc/boot/ops.h
>  @@ -86,6 +86,7 @@ int mpsc_console_init(void *devp, struct serial_console_data *scdp);
>   int cpm_console_init(void *devp, struct serial_console_data *scdp);
>   int mpc5200_psc_console_init(void *devp, struct serial_console_data *scdp);
>   int uartlite_console_init(void *devp, struct serial_console_data *scdp);
>  +int xilinx16550_console_init(void *devp, struct serial_console_data *scdp);
>   void *simple_alloc_init(char *base, unsigned long heap_size,
>                         unsigned long granularity, unsigned long max_allocs);
>   extern void flush_cache(void *, unsigned long);
>  diff --git a/arch/powerpc/boot/serial.c b/arch/powerpc/boot/serial.c
>  index 7fa5078..fb5c91e 100644
>  --- a/arch/powerpc/boot/serial.c
>  +++ b/arch/powerpc/boot/serial.c
>  @@ -131,6 +131,14 @@ int serial_console_init(void)
>         else if (dt_is_compatible(devp, "xlnx,opb-uartlite-1.00.b") ||
>                   dt_is_compatible(devp, "xlnx,xps-uartlite-1.00.a"))
>                 rc = uartlite_console_init(devp, &serial_cd);
>  +       else if (dt_is_compatible(devp, "xlnx,opb-uart16550-1.00.c") ||
>  +                dt_is_compatible(devp, "xlnx,opb-uart16550-1.00.d") ||
>  +                dt_is_compatible(devp, "xlnx,opb-uart16550-1.00.e") ||
>  +                dt_is_compatible(devp, "xlnx,plb-uart16550-1.00.c") ||
>  +                dt_is_compatible(devp, "xlnx,xps-uart16550-1.00.a") ||
>  +                dt_is_compatible(devp, "xlnx,xps-uart16550-2.00.a") ||
>  +                dt_is_compatible(devp, "xlnx,xps-uart16550-2.00.b"))
>  +               rc = xilinx16550_console_init(devp, &serial_cd);

I'll review this whole patch set today, but I need to make this
comment now.  Adding to this ever increasing list of xilinx device
versions is not sustainable.  A common base compatible value needs to
be chosen instead.  Way back in the mists of time something line
"sparse16550" was suggested and I think it is a good one.  xilinx uart
nodes should claim compatibility with their specific version *and*
"sparse16550".  Then all this code should only match with
"sparse16550" and documentation should be added to
Documentation/powerpc/booting-without-of.txt to describe what
sparse16550 means.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.