On Thu, Mar 20, 2008 at 8:43 AM, John Linn <[EMAIL PROTECTED]> 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.
More comments below.
>
> Signed-off-by: Stephen Neuendorffer <[EMAIL PROTECTED]>
> Signed-off-by: John Linn <[EMAIL PROTECTED]>
> ---
> drivers/serial/of_serial.c | 47
> ++++++++++++++++++++++++++++++-------------
> 1 files changed, 33 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/serial/of_serial.c b/drivers/serial/of_serial.c
> index 2efb892..910c94f 100644
> --- a/drivers/serial/of_serial.c
> +++ b/drivers/serial/of_serial.c
> @@ -20,13 +20,15 @@
> struct of_serial_info {
> int type;
> int line;
> + int regshift;
> + int regoffset;
> };
>
> /*
> * Fill a struct uart_port for a given device node
> */
> static int __devinit of_platform_serial_setup(struct of_device *ofdev,
> - int type, struct uart_port *port)
> + struct of_serial_info *info, struct
> uart_port *port)
> {
> struct resource resource;
> struct device_node *np = ofdev->node;
> @@ -48,17 +50,16 @@ static int __devinit of_platform_serial_setup(struct
> of_device *ofdev,
> }
>
> spin_lock_init(&port->lock);
> - port->mapbase = resource.start;
> + port->mapbase = resource.start + info->regoffset;
> port->irq = irq_of_parse_and_map(np, 0);
> port->iotype = UPIO_MEM;
> - port->type = type;
> + port->type = info->type;
> + port->regshift = info->regshift;
> port->uartclk = *clk;
> port->flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF | UPF_IOREMAP
> | UPF_FIXED_PORT;
> port->dev = &ofdev->dev;
> - /* If current-speed was set, then try not to change it. */
> - if (spd)
> - port->custom_divisor = *clk / (16 * (*spd));
> + port->custom_divisor = *clk / (16 * (*spd));
Oops, looks like you're undoing your previous patch here.
>
> return 0;
> }
> @@ -81,8 +82,9 @@ static int __devinit of_platform_serial_probe(struct
> of_device *ofdev,
> if (info == NULL)
> return -ENOMEM;
>
> - port_type = (unsigned long)id->data;
> - ret = of_platform_serial_setup(ofdev, port_type, &port);
> + memcpy(info, id->data, sizeof(struct of_serial_info));
> + port_type = info->type;
> + ret = of_platform_serial_setup(ofdev, info, &port);
> if (ret)
> goto out;
>
> @@ -100,7 +102,6 @@ static int __devinit of_platform_serial_probe(struct
> of_device *ofdev,
> if (ret < 0)
> goto out;
>
> - info->type = port_type;
> info->line = ret;
> ofdev->dev.driver_data = info;
> return 0;
> @@ -128,15 +129,33 @@ static int of_platform_serial_remove(struct of_device
> *ofdev)
> return 0;
> }
>
> +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. :-)
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
