[U-Boot] [PATCH v5] nios2: convert altera_jtag_uart to driver model
Simon Glass
sjg at chromium.org
Sat Sep 19 17:46:12 CEST 2015
Hi Thomas,
On 18 September 2015 at 00:23, Thomas Chou <thomas at wytron.com.tw> wrote:
>
> Convert altera_jtag_uart to driver model.
>
> Signed-off-by: Thomas Chou <thomas at wytron.com.tw>
> ---
>
> v2
> add ioremap.
> make the change to dts compatible with linux.
>
> v3
> use fdt address translation patch from Stefan Roese.
> fix watchdog and loop as Marek suggested.
>
> v4
> add clear AC flag to probe().
> remove polling loops and watchdog reset because they are
> done in the serial-uclass.c.
>
> v5
> fix coding style as Marek suggested to altera_uart.
>
> arch/nios2/dts/3c120_devboard.dts | 5 ++
> configs/nios2-generic_defconfig | 3 +
> drivers/serial/Kconfig | 13 ++++
> drivers/serial/altera_jtag_uart.c | 145 ++++++++++++++++++++++----------------
> include/configs/nios2-generic.h | 3 -
> 5 files changed, 106 insertions(+), 63 deletions(-)
This looks right to me but I have some comments below.
Reviewed-by: Simon Glass <sjg at chromium.org>
>
> diff --git a/arch/nios2/dts/3c120_devboard.dts b/arch/nios2/dts/3c120_devboard.dts
> index 02524ab..7f76328 100644
> --- a/arch/nios2/dts/3c120_devboard.dts
> +++ b/arch/nios2/dts/3c120_devboard.dts
> @@ -93,6 +93,7 @@
> reg = <0x00004d50 0x00000008>;
> interrupt-parent = <&cpu>;
> interrupts = <1>;
> + u-boot,dm-pre-reloc;
> };
>
> tse_mac: ethernet at 0x4000 {
> @@ -147,6 +148,10 @@
> };
> };
>
> + aliases {
> + console = &jtag_uart;
The normal way is to put this into the 'chosen' node, with the
property name stdout-path.
> + };
> +
> chosen {
> bootargs = "debug console=ttyJ0,115200";
> };
> diff --git a/configs/nios2-generic_defconfig b/configs/nios2-generic_defconfig
> index 9c1bec5..9dc6a72 100644
> --- a/configs/nios2-generic_defconfig
> +++ b/configs/nios2-generic_defconfig
> @@ -1,4 +1,5 @@
> CONFIG_NIOS2=y
> +CONFIG_DM_SERIAL=y
> CONFIG_TARGET_NIOS2_GENERIC=y
> CONFIG_DEFAULT_DEVICE_TREE="3c120_devboard"
> CONFIG_HUSH_PARSER=y
> @@ -14,3 +15,5 @@ CONFIG_CMD_PING=y
> CONFIG_OF_CONTROL=y
> CONFIG_NET_RANDOM_ETHADDR=y
> CONFIG_DM=y
> +CONFIG_ALTERA_JTAG_UART=y
> +CONFIG_ALTERA_JTAG_UART_BYPASS=y
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index ccb80d2..5a8cb3a 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -109,6 +109,19 @@ config DEBUG_UART_SHIFT
> value. Use this value to specify the shift to use, where 0=byte
> registers, 2=32-bit word registers, etc.
>
> +config ALTERA_JTAG_UART
> + bool "Altera JTAG UART support"
> + depends on NIOS2 && DM_SERIAL
> + help
> + Select this to enable an JTAG UART for Altera devices.
a JTAG UART
Is there anything else to say about it? Perhaps how it works? A link
to instructions on how to set it up?
> +
> +config ALTERA_JTAG_UART_BYPASS
> + bool "Bypass output when no connection"
> + depends on ALTERA_JTAG_UART
> + help
> + Bypass console output and keep going even if there is no
> + JTAG terminal connection with the host.
> +
> config ROCKCHIP_SERIAL
> bool "Rockchip on-chip UART support"
> depends on ARCH_UNIPHIER && DM_SERIAL
> diff --git a/drivers/serial/altera_jtag_uart.c b/drivers/serial/altera_jtag_uart.c
> index 9a81402..2c025bc 100644
> --- a/drivers/serial/altera_jtag_uart.c
> +++ b/drivers/serial/altera_jtag_uart.c
> @@ -6,98 +6,123 @@
> */
>
> #include <common.h>
> -#include <watchdog.h>
> +#include <dm.h>
> +#include <errno.h>
> #include <asm/io.h>
> #include <linux/compiler.h>
> #include <serial.h>
>
> -typedef volatile struct {
> - unsigned data; /* Data register */
> - unsigned control; /* Control register */
> -} nios_jtag_t;
> +struct altera_jtaguart_regs {
> + u32 data; /* Data register */
> + u32 control; /* Control register */
> +};
> +
> +struct altera_jtaguart_platdata {
> + struct altera_jtaguart_regs *reg;
> +};
>
> /* data register */
> #define NIOS_JTAG_RVALID (1<<15) /* Read valid */
> -#define NIOS_JTAG_DATA(d) ((d)&0x0ff) /* Read data */
> -#define NIOS_JTAG_RAVAIL(d) ((d)>>16) /* Read space avail */
>
> /* control register */
> -#define NIOS_JTAG_RE (1 << 0) /* read intr enable */
> -#define NIOS_JTAG_WE (1 << 1) /* write intr enable */
> -#define NIOS_JTAG_RI (1 << 8) /* read intr pending */
> -#define NIOS_JTAG_WI (1 << 9) /* write intr pending*/
> #define NIOS_JTAG_AC (1 << 10) /* activity indicator */
> #define NIOS_JTAG_RRDY (1 << 12) /* read available */
> #define NIOS_JTAG_WSPACE(d) ((d)>>16) /* Write space avail */
> +/* Write fifo size. FIXME: this should be extracted with sopc2dts */
> +#define NIOS_JTAG_WRITE_DEPTH 64
>
> DECLARE_GLOBAL_DATA_PTR;
>
> -/*------------------------------------------------------------------
> - * JTAG acts as the serial port
> - *-----------------------------------------------------------------*/
> -static nios_jtag_t *jtag = (nios_jtag_t *)CONFIG_SYS_NIOS_CONSOLE;
> -
> -static void altera_jtag_serial_setbrg(void)
> -{
> -}
> -
> -static int altera_jtag_serial_init(void)
> +static int altera_jtaguart_setbrg(struct udevice *dev, int baudrate)
> {
> return 0;
> }
>
> -static void altera_jtag_serial_putc(char c)
> +static int altera_jtaguart_putc(struct udevice *dev, const char c)
While you are here, how abouve 'ch' instead of 'c' as it is easier to spot.
> {
> - while (1) {
> - unsigned st = readl(&jtag->control);
> - if (NIOS_JTAG_WSPACE(st))
> - break;
> + struct altera_jtaguart_platdata *plat = dev->platdata;
> + struct altera_jtaguart_regs *const regs = plat->reg;
> + u32 st = readl(®s->control);
> +
> #ifdef CONFIG_ALTERA_JTAG_UART_BYPASS
It seems odd that you return this error when writing a character.
Don't you know whether the connection is valid in the probe() method?
Or are you trying to avoid an error at that stage?
> - if (!(st & NIOS_JTAG_AC)) /* no connection */
> - return;
> + if (!(st & NIOS_JTAG_AC)) /* no connection */
> + return -ENETUNREACH;
> #endif
> - WATCHDOG_RESET();
> - }
> - writel ((unsigned char)c, &jtag->data);
> +
> + if (NIOS_JTAG_WSPACE(st) == 0)
> + return -EAGAIN;
> +
> + writel(c, ®s->data);
> +
> + return 0;
> }
>
> -static int altera_jtag_serial_tstc(void)
> +static int altera_jtaguart_pending(struct udevice *dev, bool input)
> {
> - return ( readl (&jtag->control) & NIOS_JTAG_RRDY);
> + struct altera_jtaguart_platdata *plat = dev->platdata;
> + struct altera_jtaguart_regs *const regs = plat->reg;
> + u32 st = readl(®s->control);
> +
> + if (input)
> + return st & NIOS_JTAG_RRDY;
I think this should be:
return st & NIOS_JTAG_RRDY ? 1 : 0;
> + else
> + return !(NIOS_JTAG_WSPACE(st) == NIOS_JTAG_WRITE_DEPTH);
> }
>
> -static int altera_jtag_serial_getc(void)
> +static int altera_jtaguart_getc(struct udevice *dev)
> {
> - int c;
> - unsigned val;
> -
> - while (1) {
> - WATCHDOG_RESET ();
> - val = readl (&jtag->data);
> - if (val & NIOS_JTAG_RVALID)
> - break;
> - }
> - c = val & 0x0ff;
> - return (c);
> -}
> + struct altera_jtaguart_platdata *plat = dev->platdata;
> + struct altera_jtaguart_regs *const regs = plat->reg;
> + u32 val;
>
> -static struct serial_device altera_jtag_serial_drv = {
> - .name = "altera_jtag_uart",
> - .start = altera_jtag_serial_init,
> - .stop = NULL,
> - .setbrg = altera_jtag_serial_setbrg,
> - .putc = altera_jtag_serial_putc,
> - .puts = default_serial_puts,
> - .getc = altera_jtag_serial_getc,
> - .tstc = altera_jtag_serial_tstc,
> -};
> + val = readl(®s->data);
> +
> + if (!(val & NIOS_JTAG_RVALID))
> + return -EAGAIN;
>
> -void altera_jtag_serial_initialize(void)
> + return val & 0xff;
> +}
> +
> +static int altera_jtaguart_probe(struct udevice *dev)
> {
> - serial_register(&altera_jtag_serial_drv);
> + struct altera_jtaguart_platdata *plat = dev->platdata;
> + struct altera_jtaguart_regs *const regs = plat->reg;
> +
> +#ifdef CONFIG_ALTERA_JTAG_UART_BYPASS
> + writel(NIOS_JTAG_AC, ®s->control); /* clear AC flag */
> +#endif
> + return 0;
> }
>
> -__weak struct serial_device *default_serial_console(void)
> +static int altera_jtaguart_ofdata_to_platdata(struct udevice *dev)
> {
> - return &altera_jtag_serial_drv;
> + struct altera_jtaguart_platdata *plat = dev_get_platdata(dev);
> +
> + plat->reg = ioremap(dev_get_addr(dev),
> + sizeof(struct altera_jtaguart_regs));
What is the ioremap() for? Is this because we don't support ranges
properly yet? If so, perhaps we can assume that will be fixed before
this is applied?
> +
> + return 0;
> }
> +
> +static const struct dm_serial_ops altera_jtaguart_ops = {
> + .putc = altera_jtaguart_putc,
> + .pending = altera_jtaguart_pending,
> + .getc = altera_jtaguart_getc,
> + .setbrg = altera_jtaguart_setbrg,
> +};
> +
> +static const struct udevice_id altera_jtaguart_ids[] = {
> + { .compatible = "altr,juart-1.0", },
> + { }
> +};
> +
> +U_BOOT_DRIVER(altera_jtaguart) = {
> + .name = "altera_jtaguart",
> + .id = UCLASS_SERIAL,
> + .of_match = altera_jtaguart_ids,
> + .ofdata_to_platdata = altera_jtaguart_ofdata_to_platdata,
> + .platdata_auto_alloc_size = sizeof(struct altera_jtaguart_platdata),
> + .probe = altera_jtaguart_probe,
> + .ops = &altera_jtaguart_ops,
> + .flags = DM_FLAG_PRE_RELOC,
> +};
> diff --git a/include/configs/nios2-generic.h b/include/configs/nios2-generic.h
> index 66ad2f0..bd6d45c 100644
> --- a/include/configs/nios2-generic.h
> +++ b/include/configs/nios2-generic.h
> @@ -23,14 +23,11 @@
> /*
> * SERIAL
> */
> -#define CONFIG_ALTERA_JTAG_UART
> #if defined(CONFIG_ALTERA_JTAG_UART)
> -# define CONFIG_SYS_NIOS_CONSOLE CONFIG_SYS_JTAG_UART_BASE
> #else
> # define CONFIG_SYS_NIOS_CONSOLE CONFIG_SYS_UART_BASE
> #endif
>
> -#define CONFIG_ALTERA_JTAG_UART_BYPASS
> #define CONFIG_SYS_NIOS_FIXEDBAUD
> #define CONFIG_BAUDRATE CONFIG_SYS_UART_BAUD
> #define CONFIG_SYS_BAUDRATE_TABLE {CONFIG_BAUDRATE}
> --
> 2.1.4
>
Regards,
Simon
More information about the U-Boot
mailing list