Re: [RFC 2/8] serial: add driver for Canon DIGIC UART

[Jason Cooper <jason@lakedaemon.net>]

On Mon, Aug 26, 2013 at 08:57:11AM +0400, Antony Pavlov wrote:
> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> ---
>  drivers/serial/Kconfig        |   4 ++
>  drivers/serial/Makefile       |   1 +
>  drivers/serial/serial_digic.c | 131 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 136 insertions(+)
>  create mode 100644 drivers/serial/serial_digic.c
> 
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index 11fc155..541350d 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -122,4 +122,8 @@ config DRIVER_SERIAL_CADENCE
>  	help
>  	  Say Y here if you have a Cadence serial IP core.
>  
> +config DRIVER_SERIAL_DIGIC
> +	bool "Canon DIGIC serial driver"
> +	depends on ARCH_DIGIC
> +
>  endmenu
> diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
> index 93790b5..485a3ef 100644
> --- a/drivers/serial/Makefile
> +++ b/drivers/serial/Makefile
> @@ -17,3 +17,4 @@ obj-$(CONFIG_DRIVER_SERIAL_PXA)			+= serial_pxa.o
>  obj-$(CONFIG_DRIVER_SERIAL_OMAP4_USBBOOT)	+= serial_omap4_usbboot.o
>  obj-$(CONFIG_DRIVER_SERIAL_AUART)		+= serial_auart.o
>  obj-$(CONFIG_DRIVER_SERIAL_CADENCE)		+= serial_cadence.o
> +obj-$(CONFIG_DRIVER_SERIAL_DIGIC)		+= serial_digic.o
> diff --git a/drivers/serial/serial_digic.c b/drivers/serial/serial_digic.c
> new file mode 100644
> index 0000000..938172f
> --- /dev/null
> +++ b/drivers/serial/serial_digic.c
> @@ -0,0 +1,131 @@
> +/*
> + * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
> + *
> + * This file is part of barebox.
> + * See file CREDITS for list of people who contributed to this project.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <common.h>
> +#include <init.h>
> +#include <malloc.h>
> +#include <io.h>
> +
> +/* Serial interface registers offsets */
> +#define UART_TX	0x0
> +#define UART_RX	0x4
> +#define UART_ST	0x14
> + #define UART_ST_RX_RDY	1
> + #define UART_ST_TX_RDY	2

leading whitespace, BIT().

> +
> +/*
> + * This driver is based on the "Serial terminal" docs here:
> + *  http://magiclantern.wikia.com/wiki/Register_Map#Misc_Registers
> + *
> + * See also disassembler output for Canon A1100IS firmware
> + * (version a1100_100c):
> + *   * a outc-like function can be found at address 0xffff18f0;
> + *   * a getc-like function can be found at address 0xffff192c.
> + */
> +
> +static inline uint32_t digic_serial_readl(struct console_device *cdev, uint32_t off)
> +{
> +	void *digic = cdev->dev->priv;
> +
> +	return readl(digic + off);
> +}
> +
> +static inline void digic_serial_writel(struct console_device *cdev, uint32_t val,
> +			  uint32_t off)
> +{
> +	void *digic = cdev->dev->priv;
> +
> +	writel(val, digic + off);
> +}
> +

> +static int digic_serial_setbaudrate(struct console_device *cdev, int baudrate)
> +{
> +	/* FIXME: empty */
> +
> +	return 0;
> +}

You don't need this, see below.

> +
> +static void digic_serial_putc(struct console_device *cdev, char c)
> +{
> +	while (!(digic_serial_readl(cdev, UART_ST) & UART_ST_TX_RDY))
> +		; /* noop */
> +
> +	digic_serial_writel(cdev, 0x06, UART_ST);
> +	digic_serial_writel(cdev, c, UART_TX);
> +}
> +
> +static int digic_serial_getc(struct console_device *cdev)
> +{
> +	while (!(digic_serial_readl(cdev, UART_ST) & UART_ST_RX_RDY))
> +		; /* noop */
> +
> +	digic_serial_writel(cdev, 0x01, UART_ST);
> +
> +	return digic_serial_readl(cdev, UART_RX);
> +}
> +
> +static int digic_serial_tstc(struct console_device *cdev)
> +{
> +	return ((digic_serial_readl(cdev, UART_ST) & UART_ST_RX_RDY) != 0);
> +
> +	/*
> +	 * Canon folks use additional check, something like this:
> +	 *
> +	 *		if (digic_serial_readl(cdev, UART_ST) & 0x38) {
> +	 *			digic_serial_writel(cdev, 0x38, UART_ST);
> +	 *			return 0;
> +	 *		}
> +	 *
> +	 * But I know nothing about these magic bits in the status regster...
> +	 *
> +	*/
> +}
> +
> +static int digic_serial_probe(struct device_d *dev)
> +{
> +	struct console_device *cdev;
> +
> +	cdev = xzalloc(sizeof(struct console_device));
> +	dev->priv = dev_request_mem_region(dev, 0);
> +	cdev->dev = dev;
> +	cdev->f_caps = CONSOLE_STDIN | CONSOLE_STDOUT | CONSOLE_STDERR;
> +	cdev->tstc = &digic_serial_tstc;
> +	cdev->putc = &digic_serial_putc;
> +	cdev->getc = &digic_serial_getc;


> +	cdev->setbrg = &digic_serial_setbaudrate;

It's probably better to set this to NULL, since you don't know how
change the baud rate (and remove digic_serial_setbaudrate(), above).

thx,

Jason.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox