On Sat, Oct 20, 2012 at 05:50:58PM +0400, Alexander Shiyan wrote:
> On Tue, 16 Oct 2012 23:39:56 +0200
> Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> wrote:
>
> ...
> > > > We already had this. This SoC has a very strange register layout. The
> > > > registers for the UARTs are spread around the SoC and the registers
> > > > are not even exclusively used for the UARTs. They have bits which are
> > > > also used for the timer and other stuff.
> > > use the device id to select the register is wrong
> > >
> > > we need to pass the register via resource (without request) or platform data
> > > >
> > > > > > +static int clps711x_tstc(struct console_device *cdev)
> > > > > > +{
> > > > > > + return !(readl(SYSFLG(cdev->dev->id)) & SYSFLG_URXFE);
> > > > > factorise those pull status as
> > > > >
> > > > > status int xxx(data, mask) {}
> > > >
> > > > I don'r understand this. This is a single register read, there's not
> > > > much to factorize and the code is easy to read.
> > > he use the code at a least 2 place
> >
> > Testing for a bit like above is done a thousand times all around the
> > kernel and barebox. There is nothing to factorize. The way Alexander did
> > it is perfectly fine.
> Please look on new version below. If it looks OK, I will make complete v2 patch.
> Thanks!
>
> /*
> * Simple CLPS711X serial driver
> *
> * (C) Copyright 2012 Alexander Shiyan <shc_work@xxxxxxx>
> *
> * This program is free software; you can redistribute it and/or
> * modify it under the terms of the GNU General Public License as
> * published by the Free Software Foundation; either version 2 of
> * the License, or (at your option) any later version.
> */
>
> #include <common.h>
> #include <malloc.h>
> #include <init.h>
> #include <io.h>
> #include <linux/clk.h>
>
> #include <mach/clps711x.h>
>
> struct clps711x_uart {
> void __iomem *UBRLCR;
> void __iomem *SYSCON;
> void __iomem *SYSFLG;
> void __iomem *UARTDR;
> struct clk *uart_clk;
> struct console_device cdev;
> };
>
> static int clps711x_setbaudrate(struct console_device *cdev, int baudrate)
> {
> struct clps711x_uart *s = cdev->dev->priv;
> int divisor;
> u32 tmp;
>
> divisor = (clk_get_rate(s->uart_clk) / 16) / baudrate;
>
> tmp = readl(s->UBRLCR) & ~UBRLCR_BAUD_MASK;
> tmp |= divisor - 1;
> writel(tmp, s->UBRLCR);
>
> return 0;
> }
>
> static void clps711x_init_port(struct console_device *cdev)
> {
> struct clps711x_uart *s = cdev->dev->priv;
> u32 tmp;
>
> /* Disable the UART */
> writel(readl(s->SYSCON) & ~SYSCON_UARTEN, s->SYSCON);
>
> /* Setup Line Control Register */
> tmp = readl(s->UBRLCR) & UBRLCR_BAUD_MASK;
> tmp |= UBRLCR_FIFOEN | UBRLCR_WRDLEN8; /* FIFO on, 8N1 mode */
> writel(tmp, s->UBRLCR);
>
> /* Set default baudrate on initialization */
> clps711x_setbaudrate(cdev, CONFIG_BAUDRATE);
>
> /* Enable the UART */
> writel(readl(s->SYSCON) | SYSCON_UARTEN, s->SYSCON);
> }
>
> static void clps711x_putc(struct console_device *cdev, char c)
> {
> struct clps711x_uart *s = cdev->dev->priv;
>
> /* Wait until there is space in the FIFO */
> while (readl(s->SYSFLG) & SYSFLG_UTXFF)
> barrier();
>
> /* Send the character */
> writew(c, s->UARTDR);
> }
>
> static int clps711x_getc(struct console_device *cdev)
> {
> struct clps711x_uart *s = cdev->dev->priv;
> u16 data;
>
> /* Wait until there is data in the FIFO */
> while (readl(s->SYSFLG) & SYSFLG_URXFE)
> barrier();
>
> data = readw(s->UARTDR);
>
> /* Check for an error flag */
> if (data & (UARTDR_FRMERR | UARTDR_PARERR | UARTDR_OVERR))
> return -1;
>
> return (int)data;
> }
>
> static int clps711x_tstc(struct console_device *cdev)
> {
> struct clps711x_uart *s = cdev->dev->priv;
>
> return !(readl(s->SYSFLG) & SYSFLG_URXFE);
> }
>
> static void clps711x_flush(struct console_device *cdev)
> {
> struct clps711x_uart *s = cdev->dev->priv;
>
> while (readl(s->SYSFLG) & SYSFLG_UBUSY)
> barrier();
> }
>
> static int clps711x_probe(struct device_d *dev)
> {
> struct clps711x_uart *s;
>
> BUG_ON(dev->num_resources != 4);
>
> s = xzalloc(sizeof(struct clps711x_uart));
> s->uart_clk = clk_get(dev, NULL);
> BUG_ON(!s->uart_clk);
clk_get returns error pointers, you have to use IS_ERR() to test for
errors.
>
> s->UBRLCR = dev_get_mem_region(dev, 0);
> s->SYSCON = dev_get_mem_region(dev, 1);
> s->SYSFLG = dev_get_mem_region(dev, 2);
> s->UARTDR = dev_get_mem_region(dev, 3);
>From my point of view this looks ok.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
