max3100 driver
Subsystems (3)
PATCH RESEND 1/1 max3100 driver
This patch adds support for the MAX3100 SPI UART.
Generated on 20080920 against v2.6.27-rc6
--- Documentation/devices.txt | 3 + drivers/serial/Kconfig | 6 +- drivers/serial/Makefile | 1 + drivers/serial/max3100.c | 953 ++++++++++++++++++++++++++++++++++++++++ include/linux/serial_core.h | 3 + include/linux/serial_max3100.h | 31 ++ 6 files changed, 996 insertions(+), 1 deletions(-)
Documentation/devices.txt +3 -0
@@ -2807,6 +2807,9 @@ Your cooperation is appreciated. ... 190 = /dev/ttyUL3 Xilinx uartlite - port 3 191 = /dev/xvc0 Xen virtual console - port 0 + 192 = /dev/ttyMAX0 first MAX3100 UART + ... + 195 = /dev/ttyMAX3 fourth MAX3100 UART 205 char Low-density serial ports (alternate device) 0 = /dev/culu0 Callout device for ttyLU0
drivers/serial/Kconfig +5 -1
@@ -509,7 +509,11 @@ config SERIAL_S3C2440 help Serial port support for the Samsung S3C2440 and S3C2442 SoC - +config SERIAL_MAX3100 + tristate "MAX3100 support" + depends on SPI + help + MAX3100 chip support config SERIAL_DZ bool "DECstation DZ serial driver"
drivers/serial/Makefile +1 -0
@@ -31,6 +31,7 @@ obj-$(CONFIG_SERIAL_S3C2400) += s3c2400.o obj-$(CONFIG_SERIAL_S3C2410) += s3c2410.o obj-$(CONFIG_SERIAL_S3C2412) += s3c2412.o obj-$(CONFIG_SERIAL_S3C2440) += s3c2440.o +obj-$(CONFIG_SERIAL_MAX3100) += max3100.o obj-$(CONFIG_SERIAL_SUNCORE) += suncore.o obj-$(CONFIG_SERIAL_SUNHV) += sunhv.o obj-$(CONFIG_SERIAL_SUNZILOG) += sunzilog.o
drivers/serial/max3100.c new file +953 -0
953 lines not on this page · raw message
include/linux/serial_core.h +3 -0
@@ -155,6 +155,9 @@#define PORT_SC26XX 82 +/* MAX3100 */ +#define PORT_MAX3100 83 +#ifdef __KERNEL__#include <linux/compiler.h>
include/linux/serial_max3100.h new file +31 -0
@@ -0,0 +1,31 @@ + +/* +* +* Copyright (C) 2007 Christian Pellegrin +* +* 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. +*/ + + +#ifndef _LINUX_SERIAL_MAX3100_H +#define _LINUX_SERIAL_MAX3100_H 1 + +struct plat_max3100 { +/* force MAX3100 in loopback */ +int loopback; +/* 0 for 3.6864 Mhz, 1 for 1.8432 */ +int crystal; +/* for archs like PXA with only edge irqs */ +int only_edge_irq; +/* MAX3100 has a shutdown pin. This is a hook +called on suspend and resume to activate it.*/ +void (*max3100_hw_suspend) (int suspend); +/* poll time for ctr signals in ms, 0 disables (so no hw flow +* ctrl is possible) */ +int poll_time; +}; + +#endif
--
1.4.4.4
You got this working ... congrats! :)
Small suggestion: next time you resend this, make sure that $SUBJECT mentions it's a UART driver. Maybe even that it's a SPI UART driver. That should help get more comments.
Quoted 4 lines · Christian Pellegrin
include/linux/serial_max3100.h new file +4
-0+struct plat_max3100 { +/* force MAX3100 in loopback */ + int loopback; +/* 0 for 3.6864 Mhz, 1 for 1.8432 */
include/linux/serial_max3100.h line 29
Quoted 10 lines
include/linux/serial_max3100.h new file +10
-0+ int crystal; +/* for archs like PXA with only edge irqs */ + int only_edge_irq; +/* MAX3100 has a shutdown pin. This is a hook +called on suspend and resume to activate it.*/ + void (*max3100_hw_suspend) (int suspend); +/* poll time for ctr signals in ms, 0 disables (so no hw flow +* ctrl is possible) */ + int poll_time; +};
This is a bit picky, but it's the first thing I noticed when scanning the patch ... wierd comment layout! Either indent those all, or (better) convert to kerneldoc style.
Potentially less picky: probe() doesn't lock max3100s[], neither does remove(), and in fact there seems to be no lock for that table. Which suggests trouble in cases like concurrent I/O (including open) and driver remove(). You should probably just allocate a mutex to help control that table, like most other drivers.
And is that workqueue single threaded?
I just skimmed the rest of the driver. Seems to be fairly straightforward -- at least the non-TTY bits.
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
Quoted 5 lines · David Brownell
Small suggestion: next time you resend this, make sure that $SUBJECT mentions it's a UART driver. Maybe even that it's a SPI UART driver. That should help get more comments.
OK!
Quoted 4 lines
This is a bit picky, but it's the first thing I noticed when scanning the patch ... wierd comment layout! Either indent those all, or (better) convert to kerneldoc style.
I will have a look at kerneldoc and do the change. I'm waiting for a minor number in "Low-density serial ports" before resending the modifications asked by Andrew and Alan, so I will do this too. I just trusted M-q in emacs for the comment layout.
Quoted 4 lines
Potentially less picky:
probe()doesn't lock max3100s[], neither doesremove(), and in fact there seems to be no lock for that table. Which suggests trouble in cases like
I (wrongly!) was assuming that probing of devices is serialized. I will add the lock.
Quoted 3 lines
And is that workqueue single threaded?
it's freezeabe and (looking at include/linux/workqueue.) it implies that it's single-threaded. This is important because I don't do locking since I presume all the I/O to the MAX3100 is done in just one workqueue. When I do I/O in other places (suspend for example) I assume that the worqueue is friezed so not running.
Best regards and thanks for the review,
Signature · 11 lines
Christian Pellegrin, see http://www.evolware.org/chri/ "Real Programmers don't play tennis, or any other sport which requires you to change clothes. Mountain climbing is OK, and Real Programmers wear their climbing boots to work in case a mountain should suddenly spring up in the middle of the computer room." ------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/