[PATCH 2/3] [POWERPC] Xilinx: of_serial support for Xilinx uart16550.

[Stephen Neuendorffer]

> -----Original Message-----
> From: linuxppc-dev-bounces+stephen.neuendorffer=xilinx.com at ozlabs.org
[mailto:linuxppc-dev-
> bounces+stephen.neuendorffer=xilinx.com at ozlabs.org] On Behalf Of Grant
Likely
> Sent: Thursday, March 20, 2008 5:19 PM
> To: John Linn
> Cc: linuxppc-dev at ozlabs.org
> Subject: Re: [PATCH 2/3] [POWERPC] Xilinx: of_serial support for
Xilinx uart16550.
> 
> On Thu, Mar 20, 2008 at 8:43 AM, John Linn <john.linn at 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 introduces new bindings, which pass the correct
> >  register base and regshift properties to the uart driver to enable
> >  this core to be used.  Doing this cleanly required some refactoring
of
> >  the existing code.
> 
> Personally, I'm not fond of this approach.  There is already some
> traction to using the reg-shift property to specify spacing, and I
> think it would be appropriate to also define a reg-offset property to
> handle the +3 offset and then let the xilinx 16550 nodes use those.

Since the ePAPR ns16550 defines reg-shift, I don't think it makes sense
to have a separate binding for sparse16550, or for the xilinx16550
types.  Personally, I like having reg-offset better than
adding three to the reg space, but I leave that up to someone who is
much more involved in ePAPR.

> >  +static struct of_serial_info __devinitdata ns8250_info = { .type =
PORT_8250 };
> >  +static struct of_serial_info __devinitdata ns16450_info = { .type
= PORT_16450 };
> >  +static struct of_serial_info __devinitdata ns16550_info = { .type
= PORT_16550 };
> >  +static struct of_serial_info __devinitdata ns16750_info = { .type
= PORT_16750 };
> >  +static struct of_serial_info __devinitdata xilinx_16550_info = {
> >  +       .type = PORT_16550,
> >  +       .regshift = 2,
> >  +       .regoffset = 3,
> >  +};
> >  +static struct of_serial_info __devinitdata unknown_info = { .type
= PORT_UNKNOWN };
> 
> In support of my argument; the fact that you need a table of data says
> to me that this data should really be encoded in the device tree.  :-)

The table of data was always there to bind the UART types, I just added
some more info to it.

Steve