From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-ew0-f49.google.com ([209.85.215.49]) by canuck.infradead.org with esmtps (Exim 4.72 #1 (Red Hat Linux)) id 1Q6OXe-0005EN-4K for barebox@lists.infradead.org; Sun, 03 Apr 2011 14:44:43 +0000 Received: by ewy3 with SMTP id 3so1582626ewy.36 for ; Sun, 03 Apr 2011 07:44:36 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1301826331-18875-1-git-send-email-franck.jullien@gmail.com> <4D98680C.9060904@pengutronix.de> Date: Sun, 3 Apr 2011 16:44:34 +0200 Message-ID: From: Franck JULLIEN List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: multipart/mixed; boundary="===============0845465393==" Sender: barebox-bounces@lists.infradead.org Errors-To: barebox-bounces+u.kleine-koenig=pengutronix.de@lists.infradead.org Subject: Re: [PATCH] Nios2: Add Altera UART driver To: Marc Kleine-Budde Cc: barebox --===============0845465393== Content-Type: multipart/alternative; boundary=001517447908830ee204a004ae9f --001517447908830ee204a004ae9f Content-Type: text/plain; charset=ISO-8859-1 2011/4/3 Franck JULLIEN > > > 2011/4/3 Franck JULLIEN > >> Hello Marc, >> >> 2011/4/3 Marc Kleine-Budde >> >>> On 04/03/2011 12:25 PM, franck.jullien@gmail.com wrote: >>> > From: Franck JULLIEN >>> >>> Please add your S-o-b to the driver. See Documentation/SubmittingPatches >>> in a Linux-kernel tree. >>> >>> >> Ok I will. >> >> >> >>> See comments inline. >>> > >>> > --- >>> > drivers/serial/Kconfig | 5 ++ >>> > drivers/serial/Makefile | 25 +++++----- >>> > drivers/serial/serial_altera.c | 97 >>> ++++++++++++++++++++++++++++++++++++++++ >>> > 3 files changed, 115 insertions(+), 12 deletions(-) >>> > create mode 100644 drivers/serial/serial_altera.c >>> > >>> > diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig >>> > index ffd877a..9e05f4b 100644 >>> > --- a/drivers/serial/Kconfig >>> > +++ b/drivers/serial/Kconfig >>> > @@ -43,6 +43,11 @@ config DRIVER_SERIAL_BLACKFIN >>> > default y >>> > bool "Blackfin serial driver" >>> > >>> > +config DRIVER_SERIAL_ALTERA >>> > + depends on NIOS2 >>> > + default y >>> > + bool "Altera serial driver" >>> > + >>> > config DRIVER_SERIAL_NS16550 >>> > default n >>> > bool "NS16550 serial driver" >>> > diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile >>> > index 9f0e12b..8067b74 100644 >>> > --- a/drivers/serial/Makefile >>> > +++ b/drivers/serial/Makefile >>> > @@ -4,15 +4,16 @@ >>> > # serial_max3100.o >>> > # serial_pl010.o >>> > # serial_xuartlite.o >>> > -obj-$(CONFIG_DRIVER_SERIAL_ARM_DCC) += arm_dcc.o >>> > -obj-$(CONFIG_SERIAL_AMBA_PL011) += amba-pl011.o >>> > -obj-$(CONFIG_DRIVER_SERIAL_IMX) += serial_imx.o >>> > -obj-$(CONFIG_DRIVER_SERIAL_STM378X) += stm-serial.o >>> > -obj-$(CONFIG_DRIVER_SERIAL_ATMEL) += atmel.o >>> > -obj-$(CONFIG_DRIVER_SERIAL_NETX) += serial_netx.o >>> > -obj-$(CONFIG_DRIVER_SERIAL_LINUX_COMSOLE) += linux_console.o >>> > -obj-$(CONFIG_DRIVER_SERIAL_MPC5XXX) += serial_mpc5xxx.o >>> > -obj-$(CONFIG_DRIVER_SERIAL_BLACKFIN) += serial_blackfin.o >>> > -obj-$(CONFIG_DRIVER_SERIAL_NS16550) += serial_ns16550.o >>> > -obj-$(CONFIG_DRIVER_SERIAL_PL010) += serial_pl010.o >>> > -obj-$(CONFIG_DRIVER_SERIAL_S3C24X0) += serial_s3c24x0.o >>> > +obj-$(CONFIG_DRIVER_SERIAL_ARM_DCC) += arm_dcc.o >>> > +obj-$(CONFIG_SERIAL_AMBA_PL011) += amba-pl011.o >>> > +obj-$(CONFIG_DRIVER_SERIAL_IMX) += serial_imx.o >>> > +obj-$(CONFIG_DRIVER_SERIAL_STM378X) += stm-serial.o >>> > +obj-$(CONFIG_DRIVER_SERIAL_ATMEL) += atmel.o >>> > +obj-$(CONFIG_DRIVER_SERIAL_NETX) += serial_netx.o >>> > +obj-$(CONFIG_DRIVER_SERIAL_LINUX_COMSOLE) += linux_console.o >>> > +obj-$(CONFIG_DRIVER_SERIAL_MPC5XXX) += serial_mpc5xxx.o >>> > +obj-$(CONFIG_DRIVER_SERIAL_BLACKFIN) += serial_blackfin.o >>> > +obj-$(CONFIG_DRIVER_SERIAL_NS16550) += serial_ns16550.o >>> > +obj-$(CONFIG_DRIVER_SERIAL_PL010) += serial_pl010.o >>> > +obj-$(CONFIG_DRIVER_SERIAL_S3C24X0) += serial_s3c24x0.o >>> > +obj-$(CONFIG_DRIVER_SERIAL_ALTERA) += serial_altera.o >>> >>> Why do you reformat the whole file? Please don't do that. >>> >> >> >> Because there is a mix between tabs and spaces before the "+= ....". I >> think it's better to have spaces here in order >> to maintain alignment no matter what the tab size is. Tell me if you're >> agree, if not, I'll just put my new line at the bottom..... :) >> >> >>> >>> > diff --git a/drivers/serial/serial_altera.c >>> b/drivers/serial/serial_altera.c >>> > new file mode 100644 >>> > index 0000000..03e48d1 >>> > --- /dev/null >>> > +++ b/drivers/serial/serial_altera.c >>> > @@ -0,0 +1,97 @@ >>> > +/* >>> > + * (C) Copyright 2011, Franck JULLIEN, >>> > + * >>> > + * 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 as >>> > + * published by the Free Software Foundation; either version 2 of >>> > + * the License, or (at your option) any later version. >>> > + * >>> > + * 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. >>> > + * >>> > + * You should have received a copy of the GNU General Public License >>> > + * along with this program; if not, write to the Free Software >>> > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, >>> > + * MA 02111-1307 USA >>> > + */ >>> > + >>> > +#include >>> > +#include >>> > +#include >>> > +#include >>> > +#include >>> > +#include >>> > + >>> > +static int altera_serial_setbaudrate(struct console_device *cdev, int >>> baudrate) >>> > +{ >>> > + volatile struct nios_uart * uart = (struct nios_uart >>> *)cdev->dev->map_base; >>> ^ >>> >>> please remove this space. >>> >>> >> Damn, I used the checkpatch script though, should have seen it.... >> >> >>> no volatiles please. >>> >>> >> >> Yeah I know but last time I removed those volatile, the driver didn't work >> anymore ?!? I've done a test after I read your reply and it >> works without so I'll remove them. >> >> > In Documentation/volatile-considered-harmful.txt, one can read: > > There are still a few rare situations where volatile makes sense in the > kernel: > > - The above-mentioned accessor functions might use volatile on > architectures where direct I/O memory access does work. > Essentially, > each accessor call becomes a little critical section on its own > and > ensures that the access happens as expected by the programmer. > > (......) > > - Pointers to data structures in coherent memory which might be > modified > by I/O devices can, sometimes, legitimately be volatile. A ring > buffer > used by a network adapter, where that adapter changes pointers to > indicate which descriptors have been processed, is an example of > this > type of situation. > > So is it that bad to use volatile ? > > Ok, never mind, if we use readw, readl,...... the volatile is implicit. > > > >> >> >>> > + unsigned div; >>> >>> > + >>> > + div = (CPU_FREQ / baudrate) - 1; >>> > + writel(div, &uart->divisor); >>> > + >>> > + return 0; >>> > +} >>> > + >>> > +static void altera_serial_putc(struct console_device *cdev, char c) >>> > +{ >>> > + volatile struct nios_uart *uart = (struct nios_uart >>> *)cdev->dev->map_base; >>> > + >>> > + if (c == '\n') >>> > + altera_serial_putc(cdev, '\r'); >>> >>> Why this? Only the at91rm9200 driver does this and I don't know why.... >>> >>> >> Because I copied it from U-Boot :) Removed. >> >> >> >>> > + >>> > + while ((readl(&uart->status) & NIOS_UART_TRDY) == 0); >>> >>> > + writel(c, &uart->txdata); >>> > +} >>> > + >>> > +static int altera_serial_tstc(struct console_device *cdev) >>> > +{ >>> > + volatile struct nios_uart *uart = (struct nios_uart >>> *)cdev->dev->map_base; >>> >>> No volatiles >>> >>> > + >>> > + return readl(&uart->status) & NIOS_UART_RRDY; >>> > +} >>> > + >>> > +static int altera_serial_getc(struct console_device *cdev) >>> > +{ >>> > + volatile struct nios_uart *uart = (struct nios_uart >>> *)cdev->dev->map_base; >>> dito >>> > + >>> > + while (altera_serial_tstc(cdev) == 0); >>> please add a comment, or at least an empty line after while >>> >>> >> Do you guys put the semi colon on the next line also ? >> >> >> >>> > + return readl(&uart->rxdata) & 0x000000FF; >>> >>> > +} >>> > + >>> > +static int altera_serial_probe(struct device_d *dev) >>> > +{ >>> > + struct console_device *cdev; >>> > + >>> > + cdev = malloc(sizeof(struct console_device)); >>> >>> malloc can fail, use xmalloc instead >>> >>> >> Done. >> >> >>> > + dev->type_data = cdev; >>> >>> > + cdev->dev = dev; >>> > + cdev->f_caps = CONSOLE_STDIN | CONSOLE_STDOUT | CONSOLE_STDERR; >>> > + cdev->tstc = altera_serial_tstc; >>> > + cdev->putc = altera_serial_putc; >>> > + cdev->getc = altera_serial_getc; >>> > + cdev->setbrg = altera_serial_setbaudrate; >>> > + >>> > + console_register(cdev); >>> > + >>> > + return 0; >>> > +} >>> > + >>> > +static struct driver_d altera_serial_driver = { >>> > + .name = "altera_serial", >>> ^^ >>> one space is enough >>> >>> >> Copied from blackfin :) Patch it ? >> >> >> >>> > + .probe = altera_serial_probe, >>> >>> > +}; >>> > + >>> > +static int altera_serial_init(void) >>> > +{ >>> > + register_driver(&altera_serial_driver); >>> > + return 0; >>> >>> I think return register_driver(); is preferred. >>> >>> >> >> Copied from blackfin :) Patch it ? >> >> >>> > +} >>> > + >>> > +console_initcall(altera_serial_init); >>> > + >>> >>> cheers, Marc >>> >>> -- >>> Pengutronix e.K. | Marc Kleine-Budde | >>> Industrial Linux Solutions | Phone: +49-231-2826-924 | >>> Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | >>> Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | >>> >>> >> Do you want me to patch files each time I see a wrong coding style in the >> code ? Because I'm so focused >> on that since I'm working on the nios2 port (and since I display tabs and >> white spaces in my favorite editor :)) >> that I can see coding style mistakes in a lot of files :) (for >> example, leading spaces in serial_imx line 222, 228, >> 245, 313,.... checkpatch gives 9 errors and 44 warnings on this file). >> >> Regards, >> Franck. >> > > --001517447908830ee204a004ae9f Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable

2011/4/3 Franck JULLIEN <franck.jullien@gmail.com<= /a>>


2011/4= /3 Franck JULLIEN <franck.jullien@gmail.com>
Hello Marc,

2011/4/3 Marc Kleine-Bud= de <mkl@pengutronix.de>
On 04/03/2011 12:25 PM, franck.jullien@gmail.com wrote:
> From: Franck JULLIEN <franck.jullien@gmail.com>

Please add your S-o-b to the driver. See Documentation/SubmittingPatc= hes
in a Linux-kernel tree.


Ok I will.= =A0

=A0
See comments inline.
>
> ---
> =A0drivers/serial/Kconfig =A0 =A0 =A0 =A0 | =A0 =A05 ++
> =A0drivers/serial/Makefile =A0 =A0 =A0 =A0| =A0 25 +++++-----
> =A0drivers/serial/serial_altera.c | =A0 97 +++++++++++++++++++++++++++= +++++++++++++
> =A03 files changed, 115 insertions(+), 12 deletions(-)
> =A0create mode 100644 drivers/serial/serial_altera.c
>
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index ffd877a..9e05f4b 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -43,6 +43,11 @@ config DRIVER_SERIAL_BLACKFIN
> =A0 =A0 =A0 default y
> =A0 =A0 =A0 bool "Blackfin serial driver"
>
> +config DRIVER_SERIAL_ALTERA
> + =A0 =A0 depends on NIOS2
> + =A0 =A0 default y
> + =A0 =A0 bool "Altera serial driver"
> +
> =A0config DRIVER_SERIAL_NS16550
> =A0 =A0 =A0 default n
> =A0 =A0 =A0 bool "NS16550 serial driver"
> diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
> index 9f0e12b..8067b74 100644
> --- a/drivers/serial/Makefile
> +++ b/drivers/serial/Makefile
> @@ -4,15 +4,16 @@
> =A0# serial_max3100.o
> =A0# serial_pl010.o
> =A0# serial_xuartlite.o
> -obj-$(CONFIG_DRIVER_SERIAL_ARM_DCC) =A0 =A0 =A0 =A0 =A0+=3D arm_dcc.o=
> -obj-$(CONFIG_SERIAL_AMBA_PL011) +=3D amba-pl011.o
> -obj-$(CONFIG_DRIVER_SERIAL_IMX) =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0+=3D serial_imx.o
> -obj-$(CONFIG_DRIVER_SERIAL_STM378X) =A0 =A0 =A0 =A0 =A0+=3D stm-seria= l.o
> -obj-$(CONFIG_DRIVER_SERIAL_ATMEL) =A0 =A0 =A0 =A0 =A0 =A0+=3D atmel.o=
> -obj-$(CONFIG_DRIVER_SERIAL_NETX) =A0 =A0 =A0 =A0 =A0 =A0 +=3D serial_= netx.o
> -obj-$(CONFIG_DRIVER_SERIAL_LINUX_COMSOLE) =A0 =A0+=3D linux_console.o=
> -obj-$(CONFIG_DRIVER_SERIAL_MPC5XXX) =A0 =A0 =A0 =A0 =A0+=3D serial_mp= c5xxx.o
> -obj-$(CONFIG_DRIVER_SERIAL_BLACKFIN) =A0 =A0 =A0 =A0 +=3D serial_blac= kfin.o
> -obj-$(CONFIG_DRIVER_SERIAL_NS16550) =A0 =A0 =A0 =A0 =A0+=3D serial_ns= 16550.o
> -obj-$(CONFIG_DRIVER_SERIAL_PL010) =A0 =A0 =A0 =A0 =A0 =A0+=3D serial_= pl010.o
> -obj-$(CONFIG_DRIVER_SERIAL_S3C24X0) =A0 =A0 =A0 =A0 =A0+=3D serial_s3= c24x0.o
> +obj-$(CONFIG_DRIVER_SERIAL_ARM_DCC) =A0 =A0 =A0 =A0 =A0 =A0 +=3D arm_= dcc.o
> +obj-$(CONFIG_SERIAL_AMBA_PL011) =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 +=3D = amba-pl011.o
> +obj-$(CONFIG_DRIVER_SERIAL_IMX) =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 +=3D = serial_imx.o
> +obj-$(CONFIG_DRIVER_SERIAL_STM378X) =A0 =A0 =A0 =A0 =A0 =A0 +=3D stm-= serial.o
> +obj-$(CONFIG_DRIVER_SERIAL_ATMEL) =A0 =A0 =A0 =A0 =A0 =A0 =A0 +=3D at= mel.o
> +obj-$(CONFIG_DRIVER_SERIAL_NETX) =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0+=3D = serial_netx.o
> +obj-$(CONFIG_DRIVER_SERIAL_LINUX_COMSOLE) =A0 =A0 =A0 +=3D linux_cons= ole.o
> +obj-$(CONFIG_DRIVER_SERIAL_MPC5XXX) =A0 =A0 =A0 =A0 =A0 =A0 +=3D seri= al_mpc5xxx.o
> +obj-$(CONFIG_DRIVER_SERIAL_BLACKFIN) =A0 =A0 =A0 =A0 =A0 =A0+=3D seri= al_blackfin.o
> +obj-$(CONFIG_DRIVER_SERIAL_NS16550) =A0 =A0 =A0 =A0 =A0 =A0 +=3D seri= al_ns16550.o
> +obj-$(CONFIG_DRIVER_SERIAL_PL010) =A0 =A0 =A0 =A0 =A0 =A0 =A0 +=3D se= rial_pl010.o
> +obj-$(CONFIG_DRIVER_SERIAL_S3C24X0) =A0 =A0 =A0 =A0 =A0 =A0 +=3D seri= al_s3c24x0.o
> +obj-$(CONFIG_DRIVER_SERIAL_ALTERA) =A0 =A0 =A0 =A0 =A0 =A0 =A0+=3D se= rial_altera.o

Why do you reformat the whole file? Please don't do that.


Because there= is a mix between tabs and spaces before the "+=3D ....". I think= it's better to have spaces here in order
to maintain alignment no matter what the tab size is. Tell me if you&#= 39;re agree, if not, I'll just put my new line at the bottom..... :)
=A0

> diff --git a/drivers/serial/serial_altera.c b/drivers/serial/serial_al= tera.c
> new file mode 100644
> index 0000000..03e48d1
> --- /dev/null
> +++ b/drivers/serial/serial_altera.c
> @@ -0,0 +1,97 @@
> +/*
> + * (C) Copyright 2011, Franck JULLIEN, <elec4fun@gmail.com>
> + *
> + * 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 as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * 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. =A0See the > + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License<= br> > + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#include <common.h>
> +#include <driver.h>
> +#include <init.h>
> +#include <malloc.h>
> +#include <asm/io.h>
> +#include <asm/nios2-io.h>
> +
> +static int altera_serial_setbaudrate(struct console_device *cdev, int= baudrate)
> +{
> + =A0 =A0 volatile struct nios_uart * uart =3D (struct nios_uart *)cde= v->dev->map_base;
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 ^

please remove this space.


Damn, I used the checkpatc= h script though, should have seen it....
=A0
no volatiles please.



Yeah I know = but last time I removed those volatile, the driver didn't work anymore = ?!? I've done a test after I read your reply and it
works without so I'll remove them.


In=A0Documentation/volatile-conside= red-harmful.txt, one can read:

There are= still a few rare situations where volatile makes sense in the kernel:

=A0 =A0 =A0 =A0- The above-mentioned accessor functions= might use volatile on
=A0 =A0 =A0 =A0 =A0architectures where dir= ect I/O memory access does work. =A0Essentially,
=A0 =A0 =A0 =A0 = =A0each accessor call becomes a little critical section on its own and
=A0 =A0 =A0 =A0 =A0ensures that the access happens as expected by the = programmer.

=A0 =A0 =A0 (......)

=A0 =A0 =A0 =A0- Pointers to data structures in coherent memory whi= ch might be modified
=A0 =A0 =A0 =A0 =A0by I/O devices can, sometimes, legitimately be vola= tile. =A0A ring buffer
=A0 =A0 =A0 =A0 =A0used by a network adapt= er, where that adapter changes pointers to
=A0 =A0 =A0 =A0 =A0ind= icate which descriptors have been processed, is an example of this
=A0 =A0 =A0 =A0 =A0type of situation.

=
So is it that bad to use volatile ?


Ok, = never mind, if we use readw, readl,...... the volatile is=A0implicit.=A0

=A0

=A0
=A0
> + =A0 =A0 unsigned div;

> +
> + =A0 =A0 div =3D (CPU_FREQ / baudrate) - 1;
> + =A0 =A0 writel(div, &uart->divisor);
> +
> + =A0 =A0 return 0;
> +}
> +
> +static void altera_serial_putc(struct console_device *cdev, char c) > +{
> + =A0 =A0 volatile struct nios_uart *uart =3D (struct nios_uart *)cdev= ->dev->map_base;
> +
> + =A0 =A0 if (c =3D=3D '\n')
> + =A0 =A0 =A0 =A0 =A0 =A0 altera_serial_putc(cdev, '\r');

Why this? Only the at91rm9200 driver does this and I don&#= 39;t know why....


Because I copied it f= rom U-Boot :) Removed.

=A0
> +
> + =A0 =A0 while ((readl(&uart->status) & NIOS_UART_TRDY) = =3D=3D 0);

> + =A0 =A0 writel(c, &uart->txdata);
> +}
> +
> +static int altera_serial_tstc(struct console_device *cdev)
> +{
> + =A0 =A0 volatile struct nios_uart *uart =3D (struct nios_uart *)cdev= ->dev->map_base;

No volatiles

> +
> + =A0 =A0 return readl(&uart->status) & NIOS_UART_RRDY;
> +}
> +
> +static int altera_serial_getc(struct console_device *cdev)
> +{
> + =A0 =A0 volatile struct nios_uart *uart =3D (struct nios_uart *)cdev= ->dev->map_base;
dito
> +
> + =A0 =A0 while (altera_serial_tstc(cdev) =3D=3D 0);
please add a comment, or at least an empty line after while


Do you guys put the s= emi colon on the next line also ?

=A0
> + =A0 =A0 return readl(&uart->rxdata) & 0x000000FF;
> +}
> +
> +static int altera_serial_probe(struct device_d *dev)
> +{
> + =A0 =A0 struct console_device *cdev;
> +
> + =A0 =A0 cdev =3D malloc(sizeof(struct console_device));

malloc can fail, use xmalloc instead


Done.
=A0
> + =A0 =A0 dev->type_data =3D cdev;

> + =A0 =A0 cdev->dev =3D dev;
> + =A0 =A0 cdev->f_caps =3D CONSOLE_STDIN | CONSOLE_STDOUT | CONSOLE= _STDERR;
> + =A0 =A0 cdev->tstc =3D altera_serial_tstc;
> + =A0 =A0 cdev->putc =3D altera_serial_putc;
> + =A0 =A0 cdev->getc =3D altera_serial_getc;
> + =A0 =A0 cdev->setbrg =3D altera_serial_setbaudrate;
> +
> + =A0 =A0 console_register(cdev);
> +
> + =A0 =A0 return 0;
> +}
> +
> +static struct driver_d altera_serial_driver =3D {
> + =A0 =A0 .name =A0=3D "altera_serial",
=A0 =A0 =A0 =A0 =A0 =A0 ^^
one space is enough


Copied from blackfin :) Pat= ch it ?

=A0
> + =A0 =A0 .probe =3D altera_serial_probe,

> +};
> +
> +static int altera_serial_init(void)
> +{
> + =A0 =A0 register_driver(&altera_serial_driver);
> + =A0 =A0 return 0;

I think return register_driver(); is preferred.



Copied= from blackfin :)=A0Patch it ?
=A0
> +}
> +
> +console_initcall(altera_serial_init);
> +

cheers, Marc

--
Pengutronix e.K. =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0| Marc Kleine-Budde =A0= =A0 =A0 =A0 =A0 |
Industrial Linux Solutions =A0 =A0 =A0 =A0| Phone: +49-231-2826-924 =A0 =A0= |
Vertretung West/Dortmund =A0 =A0 =A0 =A0 =A0| Fax: =A0 +49-5121-206917-5555= |
Amtsgericht Hildesheim, HRA 2686 =A0| http://www.pengutronix.de =A0 |


Do you want me to patch files each= time I see a wrong coding style in the code ? Because I'm so focused
on that since I'm working on the nios2 port (and since I displ= ay tabs and white spaces in my favorite editor :))
that I can see coding style mistakes in a lot of files :) (for example= ,=A0leading spaces in serial_imx line 222, 228,=A0
245, 313,.... = checkpatch gives 9 errors and 44 warnings on this file).

Regards,
Franck.


--001517447908830ee204a004ae9f-- --===============0845465393== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox --===============0845465393==--