linux-kernel - Re: [PATCH v5 10/15] serial: stm32-usart: Add STM32 USART Driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 7 Apr 2015 18:05:47 +0200
From:	Maxime Coquelin <mcoquelin.stm32@...il.com>
To:	Andy Shevchenko <andy.shevchenko@...il.com>
Cc:	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>,
	Andreas Färber <afaerber@...e.de>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	Rob Herring <robh+dt@...nel.org>,
	Philipp Zabel <p.zabel@...gutronix.de>,
	Linus Walleij <linus.walleij@...aro.org>,
	Arnd Bergmann <arnd@...db.de>, Stefan Agner <stefan@...er.ch>,
	Peter Meerwald <pmeerw@...erw.net>,
	Paul Bolle <pebolle@...cali.nl>,
	Peter Hurley <peter@...leysoftware.com>,
	Chanwoo Choi <cw00.choi@...sung.com>,
	Russell King <linux@....linux.org.uk>,
	Daniel Lezcano <daniel.lezcano@...aro.org>,
	Jonathan Corbet <corbet@....net>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jslaby@...e.cz>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"David S. Miller" <davem@...emloft.net>,
	Mauro Carvalho Chehab <mchehab@....samsung.com>,
	Joe Perches <joe@...ches.com>, Antti Palosaari <crope@....fi>,
	Tejun Heo <tj@...nel.org>, Will Deacon <will.deacon@....com>,
	Nikolay Borisov <Nikolay.Borisov@....com>,
	Rusty Russell <rusty@...tcorp.com.au>,
	Kees Cook <keescook@...omium.org>,
	Michal Marek <mmarek@...e.cz>,
	Linux Documentation List <linux-doc@...r.kernel.org>,
	linux-arm Mailing List <linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	devicetree <devicetree@...r.kernel.org>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
	Linux-Arch <linux-arch@...r.kernel.org>,
	"linux-api@...r.kernel.org" <linux-api@...r.kernel.org>
Subject: Re: [PATCH v5 10/15] serial: stm32-usart: Add STM32 USART Driver

2015-04-04 0:04 GMT+02:00 Andy Shevchenko <andy.shevchenko@...il.com>:
> On Fri, Apr 3, 2015 at 8:01 PM, Maxime Coquelin
> <mcoquelin.stm32@...il.com> wrote:
>> This drivers adds support to the STM32 USART controller, which is a
>> standard serial driver.
>>
>
> Few minor comments.
>
>> Tested-by: Chanwoo Choi <cw00.choi@...sung.com>
>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@...il.com>
>> ---
>>  drivers/tty/serial/Kconfig       |  17 +
>>  drivers/tty/serial/Makefile      |   1 +
>>  drivers/tty/serial/stm32-usart.c | 736 +++++++++++++++++++++++++++++++++++++++
>>  include/uapi/linux/serial_core.h |   3 +
>>  4 files changed, 757 insertions(+)
>>  create mode 100644 drivers/tty/serial/stm32-usart.c
>>

>> diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
>> new file mode 100644
>> index 0000000..61e0e19
>> --- /dev/null
>> +++ b/drivers/tty/serial/stm32-usart.c
>> @@ -0,0 +1,736 @@
...
>> +
>> +static unsigned int stm32_get_mctrl(struct uart_port *port)
>> +{
>> +       /* This routine is used to get signals of: DCD, DSR, RI, and CTS */
>> +       return TIOCM_CAR | TIOCM_DSR | TIOCM_CTS;
>> +}
>> +
>> +/* Transmit stop */
>> +static void stm32_stop_tx(struct uart_port *port)
>> +{
>> +       stm32_clr_bits(port, USART_CR1, USART_CR1_TXEIE);
>> +}
>> +
>> +
>
> Unneeded empty line.
Right, will be removed.

>
...
>> +
>> +static int stm32_startup(struct uart_port *port)
>> +{
>> +       const char *name = to_platform_device(port->dev)->name;
>> +       u32 val;
>> +
>> +       if (request_irq(port->irq, stm32_interrupt, IRQF_NO_SUSPEND,
>> +                               name, port)) {
>
> I think you have to propogate returned value, thus
> ret = request_irq();
> if (ret < 0)
>  return ret;
Exact, I will fix this.

>
...
>> +
>> +static const char *stm32_type(struct uart_port *port)
>> +{
>> +       return (port->type == PORT_STM32) ? DRIVER_NAME : NULL;
>> +}
>> +
>> +static void stm32_release_port(struct uart_port *port)
>> +{
>> +}
>
> Doesn't core handle NULL-functions? If it does, remove such from the driver.
No it does not.

>
>> +
>> +static int stm32_request_port(struct uart_port *port)
>> +{
>> +       return 0;
>> +}
>
> Ditto.
Ditto.

Thanks for the review,
Maxime

>
>
>
> --
> With Best Regards,
> Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/