gmane.linux.serial

[PATCH RESEND] max3100 driver

This patch adds support for the MAX3100 SPI UART.

Generated on  20080920  against v2.6.27-rc6

Signed-off-by: Christian Pellegrin <chripell <at> fsfe.org>
---
 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(-)

diff --git a/Documentation/devices.txt b/Documentation/devices.txt
index 05c8064..c228887 100644
--- a/Documentation/devices.txt
+++ b/Documentation/devices.txt
@@ -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
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 77cb342..de3193d 100644

(Continue reading)

headers

David Brownell | 25 Sep 06:56

Re: [PATCH RESEND] max3100 driver

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.

On Saturday 20 September 2008, Christian Pellegrin wrote:
> +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;
> +};

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

(Continue reading)

headers

chri | 25 Sep 09:07

Re: [PATCH RESEND] max3100 driver

On Thu, Sep 25, 2008 at 6:56 AM, David Brownell <david-b@...> wrote:
>
> 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!

>
> 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.

>
> 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

I (wrongly!) was assuming that probing of devices is serialized. I
will add the lock.

>
> And is that workqueue single threaded?
>

(Continue reading)