Linux Serial subsystem development
 help / color / mirror / Atom feed
* Re: [PATCH 1/3] OF: Add helper for matching against linux,stdout-path
From: Sascha Hauer @ 2012-11-15  8:04 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-serial, Alan Cox, Greg Kroah-Hartman, linux-kernel,
	devicetree-discuss, linux-arm-kernel, kernel
In-Reply-To: <CACxGe6uW+XN6RDnP+8QFXaCYqqPZO4dZM_aynR6AVMYZT=KuMg@mail.gmail.com>

On Wed, Nov 14, 2012 at 08:49:54PM +0000, Grant Likely wrote:
> > +int of_device_is_stdout_path(struct device_node *dn)
> > +{
> > +       const char *name;
> > +
> > +       name = of_get_property(of_chosen, "linux,stdout-path", NULL);
> > +       if (name == NULL)
> > +               return 0;
> > +
> > +       if (dn == of_find_node_by_path(name))
> 
> need to of_node_put() the return value of of_find_node_by_path()
> 
> > +               return 1;
> > +
> > +       return 0;
> > +}
> 
> Hi Sascha,
> 
> I'm fine with the helper, but there really is no need for a completely
> separate .c file. Just put it in drivers/of/base.c.

Agreed.

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

^ permalink raw reply

* [PATCH v2] serial: mfd: Add nmi_touch_watchdog() into the console write function
From: Feng Tang @ 2012-11-15  8:03 UTC (permalink / raw)
  To: linux-serial, gregkh, alan; +Cc: Feng Tang

This is following what 8250 driver is doing in console write function,
to avoid the hardware lockup case.

v2: incldudes the <linux/nmi.h>

Signed-off-by: Feng Tang <feng.tang@intel.com>
Acked-by: Alan Cox <alan@linux.intel.com>
---
 drivers/tty/serial/mfd.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/tty/serial/mfd.c b/drivers/tty/serial/mfd.c
index c4b50af..79fe59b 100644
--- a/drivers/tty/serial/mfd.c
+++ b/drivers/tty/serial/mfd.c
@@ -36,6 +36,7 @@
 #include <linux/serial_mfd.h>
 #include <linux/dma-mapping.h>
 #include <linux/pci.h>
+#include <linux/nmi.h>
 #include <linux/io.h>
 #include <linux/debugfs.h>
 #include <linux/pm_runtime.h>
@@ -1113,6 +1114,8 @@ serial_hsu_console_write(struct console *co, const char *s, unsigned int count)
 	unsigned int ier;
 	int locked = 1;
 
+	touch_nmi_watchdog();
+
 	local_irq_save(flags);
 	if (up->port.sysrq)
 		locked = 0;
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH v2 2/3] serial: mxs-auart: add the DMA support for mx28
From: Lauri Hintsala @ 2012-11-15  7:22 UTC (permalink / raw)
  To: Huang Shijie
  Cc: gregkh, alan, linux-serial, linux-arm-kernel, shawn.guo, linux,
	vinod.koul, Veli-Pekka Peltola, Fabio Estevam
In-Reply-To: <50A45F83.1010009@freescale.com>

Hi,

On 11/15/2012 05:20 AM, Huang Shijie wrote:
> 于 2012年11月13日 17:42, Lauri Hintsala 写道:
>> Hi Huang,
>>
>> DMA support doesn't work with latest stable v3.6.5 or development
>> 3.7-rc5 kernels. I get following error message when I open the serial
>> port /dev/ttyAPP0:
>>
>> [ 48.730000] mxs-auart 8006a000.serial: step 1 error
>> [ 48.750000] mxs-auart 8006a000.serial: We can not start up the DMA.
>>
> I tested this patch set in imx28-evk board Rev C with linux-next-20121114.
> it works fine.
>
> Maybe you can try the linux-next code.

I tested linux-next-20121114 on apx4devkit (imx28 based device) and I 
got the same error message:

# stty -F /dev/ttyAPP0 crtscts; microcom /dev/ttyAPP0 -s 115200
[  133.710000] mxs-auart 8006a000.serial: step 1 error
[  133.720000] mxs-auart 8006a000.serial: We can not start up the DMA.


> About the flow control:
> If we do not enable the HW flow control, the data may lost. And I do not
> know how to handle with the Xon/Xoff when the DMA is supported.

I do not have the answer but it is already implemented in Freescale's 
reference kernel 
(http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/serial/mxs-auart.c?h=imx_2.6.35_11.09.01). 
So I think it is possible to handle all data without HW flow control.


Best Regards,
Lauri Hintsala
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 2/3] serial: mxs-auart: add the DMA support for mx28
From: Huang Shijie @ 2012-11-15  3:20 UTC (permalink / raw)
  To: Lauri Hintsala
  Cc: gregkh, alan, linux-serial, linux-arm-kernel, shawn.guo, linux,
	vinod.koul, Veli-Pekka Peltola
In-Reply-To: <50A21613.6030709@bluegiga.com>

于 2012年11月13日 17:42, Lauri Hintsala 写道:
> Hi Huang,
>
> DMA support doesn't work with latest stable v3.6.5 or development 
> 3.7-rc5 kernels. I get following error message when I open the serial 
> port /dev/ttyAPP0:
>
> [ 48.730000] mxs-auart 8006a000.serial: step 1 error
> [ 48.750000] mxs-auart 8006a000.serial: We can not start up the DMA.
>
I tested this patch set in imx28-evk board Rev C with linux-next-20121114.
it works fine.

Maybe you can try the linux-next code.

About the flow control:
If we do not enable the HW flow control, the data may lost. And I do not 
know how to handle with the Xon/Xoff when the DMA is supported.

Best Regards
Huang Shijie

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: Comments requested: driver for Quatech PCI serial cards
From: Jonathan Woithe @ 2012-11-15  0:11 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-serial, jwoithe
In-Reply-To: <20121114235719.794a9bdc@pyramind.ukuu.org.uk>

On Wed, Nov 14, 2012 at 11:57:19PM +0000, Alan Cox wrote:
> > our upcoming system.  I'm guessing that the difficulties with the driver as
> > it's currently structured may have been the reason it was never pushed to
> > mainline by the original author in 2007.
> 
> There are not many difficulties I can see. You just add the identifiers
> to the 8250_pci tables and a couple of small bits of code and everything
> but the RS422/485 stuff just works.
> 
> If you need the RS422 stuff then yes its a bit more work.

Unfortunately we need the RS422 stuff - that's the reason for using the
DSC-200/300 card.

Thanks for the comments and feedback - I'll take another look at things in
the light of the information.  If the functionality we require requires a
significant amount of new code to be written, I suspect that the work will
be deemed too time consuming by those who call the shots.  We'll see.

Regards
  jonathan

^ permalink raw reply

* Re: Comments requested: driver for Quatech PCI serial cards
From: Alan Cox @ 2012-11-14 23:57 UTC (permalink / raw)
  To: Jonathan Woithe; +Cc: linux-serial
In-Reply-To: <20121114231054.GC4505@marvin.atrad.com.au>

On Thu, 15 Nov 2012 09:40:54 +1030
Jonathan Woithe <jwoithe@just42.net> wrote:

> On Wed, Nov 14, 2012 at 12:12:22PM +0000, Alan Cox wrote:
> > >  - the name of the module.  Anything with "qt" in it will confuse people.
> > >    I'm thinking along the lines of serial-quatech or quatech-serial:
> > >    comments welcome.
> > 
> > qt is fine in kernel - just be careful of the qt usb module.
> 
> Ok.
> 
> > I think in the same place I'd start by just adding the needed identifiers
> > to the 8250_pci driver along with any needed init helper.
> 
> Is this in relation to the proposed signal routing API?

No in relation to throwing that driver away and doing it right.

> supplied by Quatech (which are not opensource unfortunately) will not be
> compatible with the new driver, necessitating the writing of new replacement
> userspace utilities.

tragedy 8)

> > As far as I can see the issues are
> > 
> > 1.	Clock multiplier feature
> > 
> > Supportable by flags in 8250.c and worst case by providing a custom
> > set_termios. No API needed.
> > 
> > 2.	RS485/RS422 options
> > 
> > Supportable by adding TIOCRS485 handling/callout to 8250.c (needs doing
> > anyway)
> 
> Based on this, is it fair to say that the driver in its current basic form
> (clean-ups notwithstanding) cannot be considered for inclusion in mainline?

It is.

> our upcoming system.  I'm guessing that the difficulties with the driver as
> it's currently structured may have been the reason it was never pushed to
> mainline by the original author in 2007.

There are not many difficulties I can see. You just add the identifiers
to the 8250_pci tables and a couple of small bits of code and everything
but the RS422/485 stuff just works.

If you need the RS422 stuff then yes its a bit more work.

Alan

^ permalink raw reply

* Re: Comments requested: driver for Quatech PCI serial cards
From: Jonathan Woithe @ 2012-11-14 23:10 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-serial, jwoithe
In-Reply-To: <20121114121222.3e42860f@pyramind.ukuu.org.uk>

On Wed, Nov 14, 2012 at 12:12:22PM +0000, Alan Cox wrote:
> >  - the name of the module.  Anything with "qt" in it will confuse people.
> >    I'm thinking along the lines of serial-quatech or quatech-serial:
> >    comments welcome.
> 
> qt is fine in kernel - just be careful of the qt usb module.

Ok.

> I think in the same place I'd start by just adding the needed identifiers
> to the 8250_pci driver along with any needed init helper.

Is this in relation to the proposed signal routing API?

> For the ioctls a lot appear to be just exposing values which as you say
> would fit sysfs. The tty layer now (as of 3.7rc) usefully supports sysfs
> nodes on a tty itself too.

Do I take it from this that the sysfs route is the only way this driver is
likely to get merged?  If that's the case the configuration binaries
supplied by Quatech (which are not opensource unfortunately) will not be
compatible with the new driver, necessitating the writing of new replacement
userspace utilities.

> As far as I can see the issues are
> 
> 1.	Clock multiplier feature
> 
> Supportable by flags in 8250.c and worst case by providing a custom
> set_termios. No API needed.
> 
> 2.	RS485/RS422 options
> 
> Supportable by adding TIOCRS485 handling/callout to 8250.c (needs doing
> anyway)

Based on this, is it fair to say that the driver in its current basic form
(clean-ups notwithstanding) cannot be considered for inclusion in mainline?
While time allows me to do cleanups to the existing code, porting the
functionality to totally different mechanisms (ie: away from ioctl) is
unfortunately not something I would have the time for now (mostly because I
would first have to learn those other interfaces).  My employer would simply
say to find another card, unless there were existing examples of this sort
of usage which I can use as a template.

The DSC-200/300 card (which is what I have) was initially preferred for a
new design because we've used it in other non-Linux systems in the past.  We
don't want to be stuck with maintaining an out-of-tree driver to facilitate
this though, so mainlining it is the only feasible route to us using it in
our upcoming system.  I'm guessing that the difficulties with the driver as
it's currently structured may have been the reason it was never pushed to
mainline by the original author in 2007.

> The RS485 ioctl might need some extending but we've been gradually doing
> this as we hit chips with more features in hardware ...

As far as I know the Quatech cards in question do RS422 but not explicitly
RS485 (that is, their specification doesn't mention RS485).  But I take it
that "RS485 ioctl" is what would be used to control additional RS422
features as well.

Regards
  jonathan

^ permalink raw reply

* Re: [PATCH v2] serial/8250: Add support for Exar's XR17V35x family of multi-port PCIe UARTs
From: Matt Schulte @ 2012-11-14 23:05 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-serial, Greg KH
In-Reply-To: <20121114230036.2504063c@pyramind.ukuu.org.uk>

On Wed, Nov 14, 2012 at 5:00 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> +     /* Fixme: needs tidying up */
>> +     check = serial_in(up, UART_EXAR_DVID);
>> +     if ((check == 0x82) || (check == 0x84) || (check == 0x88))
>> +             p->handle_irq = exar_handle_irq;
>
> Can't you check the port type here rather than re-probing stuff ?
>
>
> Otherwise looks good

I tried that but in my tests port.type == 0 at this point.

Matt

^ permalink raw reply

* Re: [PATCH v2] serial/8250: Add support for Exar's XR17V35x family of multi-port PCIe UARTs
From: Alan Cox @ 2012-11-14 23:00 UTC (permalink / raw)
  To: Matt Schulte; +Cc: linux-serial, Greg KH
In-Reply-To: <CAJp1Oe7tdGRzBAfXq67kzehgfUnw2FeeJRUEiX7RGya4NWkw7g@mail.gmail.com>

> +	/* Fixme: needs tidying up */
> +	check = serial_in(up, UART_EXAR_DVID);
> +	if ((check == 0x82) || (check == 0x84) || (check == 0x88))
> +		p->handle_irq = exar_handle_irq;

Can't you check the port type here rather than re-probing stuff ?


Otherwise looks good

^ permalink raw reply

* Re: [PATCH 1/3] OF: Add helper for matching against linux,stdout-path
From: Grant Likely @ 2012-11-14 20:49 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: linux-serial, Alan Cox, Greg Kroah-Hartman, linux-kernel,
	devicetree-discuss, linux-arm-kernel, kernel
In-Reply-To: <1351849734-9836-2-git-send-email-s.hauer@pengutronix.de>

On Fri, Nov 2, 2012 at 9:48 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> devicetrees may have a linux,stdout-path property in the chosen
> node describing the console device. This adds a helper function
> to match a device against this property so a driver can call
> add_preferred_console for a matching device.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/of/Kconfig        |    3 +++
>  drivers/of/Makefile       |    1 +
>  drivers/of/of_stdout.c    |   27 +++++++++++++++++++++++++++
>  include/linux/of_stdout.h |   24 ++++++++++++++++++++++++
>  4 files changed, 55 insertions(+)
>  create mode 100644 drivers/of/of_stdout.c
>  create mode 100644 include/linux/of_stdout.h
>
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index dfba3e6..8574ebb 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -67,6 +67,9 @@ config OF_MDIO
>         help
>           OpenFirmware MDIO bus (Ethernet PHY) accessors
>
> +config OF_STDOUT
> +       def_bool y
> +
>  config OF_PCI
>         def_tristate PCI
>         depends on PCI
> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> index e027f44..c9f3f2f 100644
> --- a/drivers/of/Makefile
> +++ b/drivers/of/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_OF_I2C)    += of_i2c.o
>  obj-$(CONFIG_OF_NET)   += of_net.o
>  obj-$(CONFIG_OF_SELFTEST) += selftest.o
>  obj-$(CONFIG_OF_MDIO)  += of_mdio.o
> +obj-$(CONFIG_OF_STDOUT) += of_stdout.o
>  obj-$(CONFIG_OF_PCI)   += of_pci.o
>  obj-$(CONFIG_OF_PCI_IRQ)  += of_pci_irq.o
>  obj-$(CONFIG_OF_MTD)   += of_mtd.o
> diff --git a/drivers/of/of_stdout.c b/drivers/of/of_stdout.c
> new file mode 100644
> index 0000000..c9e370e
> --- /dev/null
> +++ b/drivers/of/of_stdout.c
> @@ -0,0 +1,27 @@
> +/*
> + * OF helper for linux,stdoutpath property.
> + *
> + * This file is released under the GPLv2
> + */
> +#include <linux/of_stdout.h>
> +
> +/**
> + * of_device_is_stdout_path - check if a device node matches the
> + *                            linux,stdout-path property
> + *
> + * Check if this device node matches the linux,stdout-path property
> + * in the chosen node. return true if yes, false otherwise.
> + */
> +int of_device_is_stdout_path(struct device_node *dn)
> +{
> +       const char *name;
> +
> +       name = of_get_property(of_chosen, "linux,stdout-path", NULL);
> +       if (name == NULL)
> +               return 0;
> +
> +       if (dn == of_find_node_by_path(name))

need to of_node_put() the return value of of_find_node_by_path()

> +               return 1;
> +
> +       return 0;
> +}

Hi Sascha,

I'm fine with the helper, but there really is no need for a completely
separate .c file. Just put it in drivers/of/base.c.

g.

^ permalink raw reply

* Re: [PATCH 1/3] OF: Add helper for matching against linux,stdout-path
From: Greg Kroah-Hartman @ 2012-11-14 20:43 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: linux-serial, Alan Cox, Grant Likely, linux-kernel,
	devicetree-discuss, linux-arm-kernel, kernel
In-Reply-To: <1351849734-9836-2-git-send-email-s.hauer@pengutronix.de>

On Fri, Nov 02, 2012 at 10:48:52AM +0100, Sascha Hauer wrote:
> devicetrees may have a linux,stdout-path property in the chosen
> node describing the console device. This adds a helper function
> to match a device against this property so a driver can call
> add_preferred_console for a matching device.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/of/Kconfig        |    3 +++
>  drivers/of/Makefile       |    1 +
>  drivers/of/of_stdout.c    |   27 +++++++++++++++++++++++++++
>  include/linux/of_stdout.h |   24 ++++++++++++++++++++++++
>  4 files changed, 55 insertions(+)
>  create mode 100644 drivers/of/of_stdout.c
>  create mode 100644 include/linux/of_stdout.h

I need an ACK from the OF maintainers before I can take this through the
tty tree.

thanks,

greg k-h

^ permalink raw reply

* [PATCH v2] serial/8250: Add support for Exar's XR17V35x family of multi-port PCIe UARTs
From: Matt Schulte @ 2012-11-14 18:06 UTC (permalink / raw)
  To: linux-serial; +Cc: Greg KH, Theodore Ts'o, Alan Cox

Add support for new devices: Exar's XR17V35x family of multi-port PCIe UARTs.

Built against tty-next 76cc43868c1e9d6344ad6c4992c4f6abd5204a8f

Signed-off-by: Matt Schulte <matts@commtech-fastcom.com>
---
v2: Moved extra interrupt handling to its own custom handle_irq

v1: Hopefully if this gets accepted I will be able to submit an additional
patch to add support for my company's PCIe cards that use these
UARTs.

 drivers/tty/serial/8250/8250.c     |   72 +++++++++++++++++++++++++++
 drivers/tty/serial/8250/8250_pci.c |   96 ++++++++++++++++++++++++++++++++++++
 include/linux/pci_ids.h            |    3 +
 include/uapi/linux/serial_core.h   |    3 +-
 include/uapi/linux/serial_reg.h    |    6 ++
 5 files changed, 179 insertions(+), 1 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index 5ccbd90..eb503da 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -282,6 +282,15 @@ static const struct serial8250_config uart_config[] = {
 		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
 		.flags		= UART_CAP_FIFO | UART_CAP_AFE | UART_CAP_EFR,
 	},
+	[PORT_XR17V35X] = {
+		.name		= "XR17V35X",
+		.fifo_size	= 256,
+		.tx_loadsz	= 256,
+		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_11 |
+				  UART_FCR_T_TRIG_11,
+		.flags		= UART_CAP_FIFO | UART_CAP_AFE | UART_CAP_EFR |
+				  UART_CAP_SLEEP,
+	},
 	[PORT_LPC3220] = {
 		.name		= "LPC3220",
 		.fifo_size	= 64,
@@ -455,11 +464,13 @@ static void io_serial_out(struct uart_port *p,
int offset, int value)
 }

 static int serial8250_default_handle_irq(struct uart_port *port);
+static int exar_handle_irq(struct uart_port *port);

 static void set_io_from_upio(struct uart_port *p)
 {
 	struct uart_8250_port *up =
 		container_of(p, struct uart_8250_port, port);
+	unsigned check;

 	up->dl_read = default_serial_dl_read;
 	up->dl_write = default_serial_dl_write;
@@ -506,6 +517,10 @@ static void set_io_from_upio(struct uart_port *p)
 	/* Remember loaded iotype */
 	up->cur_iotype = p->iotype;
 	p->handle_irq = serial8250_default_handle_irq;
+	/* Fixme: needs tidying up */
+	check = serial_in(up, UART_EXAR_DVID);
+	if ((check == 0x82) || (check == 0x84) || (check == 0x88))
+		p->handle_irq = exar_handle_irq;
 }

 static void
@@ -574,6 +589,18 @@ EXPORT_SYMBOL_GPL(serial8250_clear_and_reinit_fifos);
  */
 static void serial8250_set_sleep(struct uart_8250_port *p, int sleep)
 {
+	/*
+	 * Exar UARTs have a SLEEP register that enables or disables
+	 * each UART to enter sleep mode separately.  On the XR17V35x the
+	 * register is accessible to each UART at the UART_EXAR_SLEEP
+	 * offset but the UART channel may only write to the corresponding
+	 * bit.
+	 */
+	if (p->port.type == PORT_XR17V35X) {
+		serial_out(p, UART_EXAR_SLEEP, 0xff);
+		return;
+	}
+
 	if (p->capabilities & UART_CAP_SLEEP) {
 		if (p->capabilities & UART_CAP_EFR) {
 			serial_out(p, UART_LCR, UART_LCR_CONF_MODE_B);
@@ -882,6 +909,27 @@ static void autoconfig_16550a(struct uart_8250_port *up)
 	up->capabilities |= UART_CAP_FIFO;

 	/*
+	 * XR17V35x UARTs have an extra divisor register, DLD
+	 * that gets enabled with when DLAB is set which will
+	 * cause the device to incorrectly match and assign
+	 * port type to PORT_16650.  The EFR for this UART is
+	 * found at offset 0x09. Instead check the Deice ID (DVID)
+	 * register for a 2, 4 or 8 port UART.
+	 */
+	status1 = serial_in(up, UART_EXAR_DVID);
+	if (status1 == 0x82 || status1 == 0x84 || status1 == 0x88) {
+		if (up->port.flags & UPF_EXAR_EFR) {
+			DEBUG_AUTOCONF("Exar XR17V35x ");
+			up->port.type = PORT_XR17V35X;
+			up->capabilities |= UART_CAP_AFE | UART_CAP_EFR |
+						UART_CAP_SLEEP;
+
+			return;
+		}
+
+	}
+
+	/*
 	 * Check for presence of the EFR when DLAB is set.
 	 * Only ST16C650V1 UARTs pass this test.
 	 */
@@ -1516,6 +1564,30 @@ static int serial8250_default_handle_irq(struct
uart_port *port)
 }

 /*
+ * These Exar UARTs have an extra interrupt indicator that could
+ * fire for a few unimplemented interrupts.  One of which is a
+ * wakeup event when coming out of sleep.  Put this here just
+ * to be on the safe side that these interrupts don't go unhandled.
+ */
+static int exar_handle_irq(struct uart_port *port)
+{
+	unsigned char int0, int1, int2, int3;
+	unsigned int iir = serial_port_in(port, UART_IIR);
+	int ret;
+
+	ret = serial8250_handle_irq(port, iir);
+
+	if (port->type == PORT_XR17V35X) {
+		int0 = serial_port_in(port, 0x80);
+		int1 = serial_port_in(port, 0x81);
+		int2 = serial_port_in(port, 0x82);
+		int3 = serial_port_in(port, 0x83);
+	}
+
+	return ret;
+}
+
+/*
  * This is the serial driver's interrupt routine.
  *
  * Arjan thinks the old way was overly complex, so it got simplified.
diff --git a/drivers/tty/serial/8250/8250_pci.c
b/drivers/tty/serial/8250/8250_pci.c
index 508063b..0512906 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1165,6 +1165,39 @@ pci_xr17c154_setup(struct serial_private *priv,
 }

 static int
+pci_xr17v35x_setup(struct serial_private *priv,
+		  const struct pciserial_board *board,
+		  struct uart_8250_port *port, int idx)
+{
+	u8 __iomem *p;
+
+	p = pci_ioremap_bar(priv->dev, 0);
+
+	/*
+	 * Setup Multipurpose Input/Output pins.
+	 */
+
+	port->port.flags |= UPF_EXAR_EFR;
+	if (idx == 0) {
+		writeb(0x00, p + 0x8f); /*MPIOINT[7:0]*/
+		writeb(0x00, p + 0x90); /*MPIOLVL[7:0]*/
+		writeb(0x00, p + 0x91); /*MPIO3T[7:0]*/
+		writeb(0x00, p + 0x92); /*MPIOINV[7:0]*/
+		writeb(0x00, p + 0x93); /*MPIOSEL[7:0]*/
+		writeb(0x00, p + 0x94); /*MPIOOD[7:0]*/
+		writeb(0x00, p + 0x95); /*MPIOINT[15:8]*/
+		writeb(0x00, p + 0x96); /*MPIOLVL[15:8]*/
+		writeb(0x00, p + 0x97); /*MPIO3T[15:8]*/
+		writeb(0x00, p + 0x98); /*MPIOINV[15:8]*/
+		writeb(0x00, p + 0x99); /*MPIOSEL[15:8]*/
+		writeb(0x00, p + 0x9a); /*MPIOOD[15:8]*/
+	}
+	iounmap(p);
+
+	return pci_default_setup(priv, board, port, idx);
+}
+
+static int
 pci_wch_ch353_setup(struct serial_private *priv,
                     const struct pciserial_board *board,
                     struct uart_8250_port *port, int idx)
@@ -1622,6 +1655,27 @@ static struct pci_serial_quirk
pci_serial_quirks[] __refdata = {
 		.subdevice	= PCI_ANY_ID,
 		.setup		= pci_xr17c154_setup,
 	},
+	{
+		.vendor = PCI_VENDOR_ID_EXAR,
+		.device = PCI_DEVICE_ID_EXAR_XR17V352,
+		.subvendor	= PCI_ANY_ID,
+		.subdevice	= PCI_ANY_ID,
+		.setup		= pci_xr17v35x_setup,
+	},
+	{
+		.vendor = PCI_VENDOR_ID_EXAR,
+		.device = PCI_DEVICE_ID_EXAR_XR17V354,
+		.subvendor	= PCI_ANY_ID,
+		.subdevice	= PCI_ANY_ID,
+		.setup		= pci_xr17v35x_setup,
+	},
+	{
+		.vendor = PCI_VENDOR_ID_EXAR,
+		.device = PCI_DEVICE_ID_EXAR_XR17V358,
+		.subvendor	= PCI_ANY_ID,
+		.subdevice	= PCI_ANY_ID,
+		.setup		= pci_xr17v35x_setup,
+	},
 	/*
 	 * Xircom cards
 	 */
@@ -1962,6 +2016,9 @@ enum pci_board_num_t {
 	pbn_exar_XR17C152,
 	pbn_exar_XR17C154,
 	pbn_exar_XR17C158,
+	pbn_exar_XR17V352,
+	pbn_exar_XR17V354,
+	pbn_exar_XR17V358,
 	pbn_exar_ibm_saturn,
 	pbn_pasemi_1682M,
 	pbn_ni8430_2,
@@ -2580,6 +2637,30 @@ static struct pciserial_board pci_boards[]
__devinitdata = {
 		.base_baud	= 921600,
 		.uart_offset	= 0x200,
 	},
+	[pbn_exar_XR17V352] = {
+		.flags		= FL_BASE0,
+		.num_ports	= 2,
+		.base_baud	= 7812500,
+		.uart_offset	= 0x400,
+		.reg_shift	= 0,
+		.first_offset	= 0,
+	},
+	[pbn_exar_XR17V354] = {
+		.flags		= FL_BASE0,
+		.num_ports	= 4,
+		.base_baud	= 7812500,
+		.uart_offset	= 0x400,
+		.reg_shift	= 0,
+		.first_offset	= 0,
+	},
+	[pbn_exar_XR17V358] = {
+		.flags		= FL_BASE0,
+		.num_ports	= 8,
+		.base_baud	= 7812500,
+		.uart_offset	= 0x400,
+		.reg_shift	= 0,
+		.first_offset	= 0,
+	},
 	[pbn_exar_ibm_saturn] = {
 		.flags		= FL_BASE0,
 		.num_ports	= 1,
@@ -3826,6 +3907,21 @@ static struct pci_device_id serial_pci_tbl[] = {
 		PCI_ANY_ID, PCI_ANY_ID,
 		0,
 		0, pbn_exar_XR17C158 },
+	/*
+	 * Exar Corp. XR17V35[248] Dual/Quad/Octal PCIe UARTs
+	 */
+	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17V352,
+		PCI_ANY_ID, PCI_ANY_ID,
+		0,
+		0, pbn_exar_XR17V352 },
+	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17V354,
+		PCI_ANY_ID, PCI_ANY_ID,
+		0,
+		0, pbn_exar_XR17V354 },
+	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17V358,
+		PCI_ANY_ID, PCI_ANY_ID,
+		0,
+		0, pbn_exar_XR17V358 },

 	/*
 	 * Topic TP560 Data/Fax/Voice 56k modem (reported by Evan Clarke)
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 9d36b82..0199a7a 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -1985,6 +1985,9 @@
 #define PCI_DEVICE_ID_EXAR_XR17C152	0x0152
 #define PCI_DEVICE_ID_EXAR_XR17C154	0x0154
 #define PCI_DEVICE_ID_EXAR_XR17C158	0x0158
+#define PCI_DEVICE_ID_EXAR_XR17V352	0x0352
+#define PCI_DEVICE_ID_EXAR_XR17V354	0x0354
+#define PCI_DEVICE_ID_EXAR_XR17V358	0x0358

 #define PCI_VENDOR_ID_MICROGATE		0x13c0
 #define PCI_DEVICE_ID_MICROGATE_USC	0x0010
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index ebcc73f..78f99d9 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -49,7 +49,8 @@
 #define PORT_XR17D15X	21	/* Exar XR17D15x UART */
 #define PORT_LPC3220	22	/* NXP LPC32xx SoC "Standard" UART */
 #define PORT_8250_CIR	23	/* CIR infrared port, has its own driver */
-#define PORT_MAX_8250	23	/* max port ID */
+#define PORT_XR17V35X	24	/* Exar XR17V35x UARTs */
+#define PORT_MAX_8250	24	/* max port ID */

 /*
  * ARM specific type numbers.  These are not currently guaranteed
diff --git a/include/uapi/linux/serial_reg.h b/include/uapi/linux/serial_reg.h
index 5ed325e..d0b4760 100644
--- a/include/uapi/linux/serial_reg.h
+++ b/include/uapi/linux/serial_reg.h
@@ -367,5 +367,11 @@
 #define UART_OMAP_MDR1_CIR_MODE		0x06	/* CIR mode */
 #define UART_OMAP_MDR1_DISABLE		0x07	/* Disable (default state) */

+/*
+ * These are definitions for the XR17V35X and XR17D15X
+ */
+#define UART_EXAR_SLEEP		0x8b	/* Sleep mode */
+#define UART_EXAR_DVID		0x8d	/* Device identification */
+
 #endif /* _LINUX_SERIAL_REG_H */

-- 
1.7.2.5

^ permalink raw reply related

* Re: [PATCH] serial/8250: Add support for Exar's XR17V35x family of multi-port PCIe UARTs.
From: Alan Cox @ 2012-11-14 12:22 UTC (permalink / raw)
  To: Matt Schulte; +Cc: linux-serial, Greg KH, Theodore Ts'o
In-Reply-To: <CAJp1Oe73J848d++W_NLbp96VKmr6JrgkM4boLFyqUZ4WsdehVA@mail.gmail.com>

> I'm having trouble figuring out where would be the right place to override the
> 
>      uart_port.handle_irq = serial8250_default_handle_irq
> 
> with my
> 
>      uart_port.handle_irq = exar_handle_irq
> 
> Can you give me a tip as to where in 8250.c this should be done?


To get it going how about adding a hack for it into set_io_from_upio with
a fixme note that we will need to tidy it all up later once things are
done ?

Alan

^ permalink raw reply

* Re: 8250_early for big-endian
From: Alan Cox @ 2012-11-14 12:20 UTC (permalink / raw)
  To: Noam Camus; +Cc: linux-serial@vger.kernel.org, Vineet Gupta, Gilad Ben Yossef
In-Reply-To: <264C179F799EF24AB26D5319053335E80CC30499FF@ezexch.ezchip.com>

>From e2dda56ab679d81bb0a021e102519185028deded Mon Sep 17 00:00:00 2001
> From: Noam Camus <noamc@ezchip.com>
> Date: Sun, 11 Nov 2012 05:49:39 +0200
> Subject: [PATCH] tty/8250_early: Turn serial_in/serial_out into weak symbols.
> 
> Allows overriding default methods serial_in/serial_out.
> 
> In such platform specific replacement it is possible to use,
> other regshift, biased register offset, any other manipulation
> that is not covered with common default methods.
> 
> Overriding default methods may be useful for platforms which got
> serial peripheral with registers represented in big endian.
> In this situation and assuming that 32 bit operations / alignment
> is required then it may be useful to swab words before/after
> accessing the serial registers.


Looks ok to me.

Acked-by: Alan Cox <alan@linux.intel.com>

^ permalink raw reply

* Re: Comments requested: driver for Quatech PCI serial cards
From: Alan Cox @ 2012-11-14 12:12 UTC (permalink / raw)
  To: Jonathan Woithe; +Cc: linux-serial
In-Reply-To: <20121114060724.GN30156@marvin.atrad.com.au>

> The driver supports a wide range of Quatech DSC/ESC/QSC cards, including the
> custom functionality which allow various aspects of the signal routing to be
> altered under software control.

This has actually been kicked around with another couple of cards. We
have a draft but never test implemented API for binding gpio numbers on
a device to signals for the tty layer.

>  - the name of the module.  Anything with "qt" in it will confuse people.
>    I'm thinking along the lines of serial-quatech or quatech-serial:
>    comments welcome.

qt is fine in kernel - just be careful of the qt usb module.

I think in the same place I'd start by just adding the needed identifiers
to the 8250_pci driver along with any needed init helper.

For the ioctls a lot appear to be just exposing values which as you say
would fit sysfs. The tty layer now (as of 3.7rc) usefully supports sysfs
nodes on a tty itself too.

The basic ones are already handled by 8250_pci.c (QSC100, DSC100,
ESC100D, ESC100M).

As far as I can see the issues are

1.	Clock multiplier feature

Supportable by flags in 8250.c and worst case by providing a custom
set_termios. No API needed.

2.	RS485/RS422 options

Supportable by adding TIOCRS485 handling/callout to 8250.c (needs doing
anyway)

The RS485 ioctl might need some extending but we've been gradually doing
this as we hit chips with more features in hardware so that's not a
problem beyond synchronizing with other platforms who can use the extra
flags.

Alan

^ permalink raw reply

* [PATCH] tty/serial/ar933x_uart: fix baud rate calculation
From: Gabor Juhos @ 2012-11-14  9:38 UTC (permalink / raw)
  To: Alan Cox; +Cc: Greg Kroah-Hartman, linux-serial, Gabor Juhos

The UART of the AR933x SoC implements a fractional divisor
for generating the desired baud rate.

The current code uses a fixed value for the fractional
part of the divisor, and this leads to improperly
calculated baud rates:

   baud    scale   step  real baud         diff
     300   5207*   8192     17756     17456   5818.66%
     600   2603*   8192     35511     34911   5818.50%
    1200   1301*   8192     71023     69823   5818.58%
    2400    650*   8192     11241      8841    368.37%
    4800    324*   8192     22645     17845    371.77%
    9600    161    8192      9645        45      0.46%
   14400    107    8192     14468        68      0.47%
   19200     80    8192     19290        90      0.46%
   28800     53    8192     28935       135      0.46%
   38400     39    8192     39063       663      1.72%
   57600     26    8192     57870       270      0.46%
  115200     12    8192    120192      4992      4.33%
  230400      5    8192    260417     30017     13.02%
  460800      2    8192    520833     60033     13.02%
  921600      0    8192   1562500    640900     69.93%

After the patch, the integer and fractional parts of the
divisor will be calculated dynamically. This ensures that
the UART will use correct baud rates:

   baud    scale   step  real baud         diff
     300      6      11       300         0      0.00%
     600     54     173       600         0      0.00%
    1200     30     195      1200         0      0.00%
    2400     30     390      2400         0      0.00%
    4800     48    1233      4800         0      0.00%
    9600     78    3976      9600         0      0.00%
   14400     98    7474     14400         0      0.00%
   19200     55    5637     19200         0      0.00%
   28800    130   19780     28800         0      0.00%
   38400     36    7449     38400         0      0.00%
   57600     78   23857     57600         0      0.00%
  115200     43   26575    115200         0      0.00%
  230400     23   28991    230400         0      0.00%
  460800     11   28991    460800         0      0.00%
  921600      5   28991    921599        -1      0.00%

Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
---
 drivers/tty/serial/ar933x_uart.c |   90 +++++++++++++++++++++++++++++++++++---
 1 file changed, 85 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/ar933x_uart.c b/drivers/tty/serial/ar933x_uart.c
index e4f60e2b..0f5b28a 100644
--- a/drivers/tty/serial/ar933x_uart.c
+++ b/drivers/tty/serial/ar933x_uart.c
@@ -25,11 +25,19 @@
 #include <linux/io.h>
 #include <linux/irq.h>
 
+#include <asm/div64.h>
+
 #include <asm/mach-ath79/ar933x_uart.h>
 #include <asm/mach-ath79/ar933x_uart_platform.h>
 
 #define DRIVER_NAME "ar933x-uart"
 
+#define AR933X_UART_MAX_SCALE	0xff
+#define AR933X_UART_MAX_STEP	0xffff
+
+#define AR933X_UART_MIN_BAUD	300
+#define AR933X_UART_MAX_BAUD	3000000
+
 #define AR933X_DUMMY_STATUS_RD	0x01
 
 static struct uart_driver ar933x_uart_driver;
@@ -37,6 +45,8 @@ static struct uart_driver ar933x_uart_driver;
 struct ar933x_uart_port {
 	struct uart_port	port;
 	unsigned int		ier;	/* shadow Interrupt Enable Register */
+	unsigned int		min_baud;
+	unsigned int		max_baud;
 };
 
 static inline unsigned int ar933x_uart_read(struct ar933x_uart_port *up,
@@ -162,6 +172,57 @@ static void ar933x_uart_enable_ms(struct uart_port *port)
 {
 }
 
+/*
+ * baudrate = (clk / (scale + 1)) * (step * (1 / 2^17))
+ */
+static unsigned long ar933x_uart_get_baud(unsigned int clk,
+					  unsigned int scale,
+					  unsigned int step)
+{
+	u64 t;
+	u32 div;
+
+	div = (2 << 16) * (scale + 1);
+	t = clk;
+	t *= step;
+	t += (div / 2);
+	do_div(t, div);
+
+	return t;
+}
+
+static void ar933x_uart_get_scale_step(unsigned int clk,
+				       unsigned int baud,
+				       unsigned int *scale,
+				       unsigned int *step)
+{
+	unsigned int tscale;
+	long min_diff;
+
+	*scale = 0;
+	*step = 0;
+
+	min_diff = baud;
+	for (tscale = 0; tscale < AR933X_UART_MAX_SCALE; tscale++) {
+		u64 tstep;
+		int diff;
+
+		tstep = baud * (tscale + 1);
+		tstep *= (2 << 16);
+		do_div(tstep, clk);
+
+		if (tstep > AR933X_UART_MAX_STEP)
+			break;
+
+		diff = abs(ar933x_uart_get_baud(clk, tscale, tstep) - baud);
+		if (diff < min_diff) {
+			min_diff = diff;
+			*scale = tscale;
+			*step = tstep;
+		}
+	}
+}
+
 static void ar933x_uart_set_termios(struct uart_port *port,
 				    struct ktermios *new,
 				    struct ktermios *old)
@@ -169,7 +230,7 @@ static void ar933x_uart_set_termios(struct uart_port *port,
 	struct ar933x_uart_port *up = (struct ar933x_uart_port *) port;
 	unsigned int cs;
 	unsigned long flags;
-	unsigned int baud, scale;
+	unsigned int baud, scale, step;
 
 	/* Only CS8 is supported */
 	new->c_cflag &= ~CSIZE;
@@ -191,8 +252,8 @@ static void ar933x_uart_set_termios(struct uart_port *port,
 	/* Mark/space parity is not supported */
 	new->c_cflag &= ~CMSPAR;
 
-	baud = uart_get_baud_rate(port, new, old, 0, port->uartclk / 16);
-	scale = (port->uartclk / (16 * baud)) - 1;
+	baud = uart_get_baud_rate(port, new, old, up->min_baud, up->max_baud);
+	ar933x_uart_get_scale_step(port->uartclk, baud, &scale, &step);
 
 	/*
 	 * Ok, we're now changing the port state. Do it with
@@ -200,6 +261,10 @@ static void ar933x_uart_set_termios(struct uart_port *port,
 	 */
 	spin_lock_irqsave(&up->port.lock, flags);
 
+	/* disable the UART */
+	ar933x_uart_rmw_clear(up, AR933X_UART_CS_REG,
+		      AR933X_UART_CS_IF_MODE_M << AR933X_UART_CS_IF_MODE_S);
+
 	/* Update the per-port timeout. */
 	uart_update_timeout(port, new->c_cflag, baud);
 
@@ -210,7 +275,7 @@ static void ar933x_uart_set_termios(struct uart_port *port,
 		up->port.ignore_status_mask |= AR933X_DUMMY_STATUS_RD;
 
 	ar933x_uart_write(up, AR933X_UART_CLOCK_REG,
-			  scale << AR933X_UART_CLOCK_SCALE_S | 8192);
+			  scale << AR933X_UART_CLOCK_SCALE_S | step);
 
 	/* setup configuration register */
 	ar933x_uart_rmw(up, AR933X_UART_CS_REG, AR933X_UART_CS_PARITY_M, cs);
@@ -219,6 +284,11 @@ static void ar933x_uart_set_termios(struct uart_port *port,
 	ar933x_uart_rmw_set(up, AR933X_UART_CS_REG,
 			    AR933X_UART_CS_HOST_INT_EN);
 
+	/* reenable the UART */
+	ar933x_uart_rmw(up, AR933X_UART_CS_REG,
+			AR933X_UART_CS_IF_MODE_M << AR933X_UART_CS_IF_MODE_S,
+			AR933X_UART_CS_IF_MODE_DCE << AR933X_UART_CS_IF_MODE_S);
+
 	spin_unlock_irqrestore(&up->port.lock, flags);
 
 	if (tty_termios_baud_rate(new))
@@ -401,6 +471,8 @@ static void ar933x_uart_config_port(struct uart_port *port, int flags)
 static int ar933x_uart_verify_port(struct uart_port *port,
 				   struct serial_struct *ser)
 {
+	struct ar933x_uart_port *up = (struct ar933x_uart_port *) port;
+
 	if (ser->type != PORT_UNKNOWN &&
 	    ser->type != PORT_AR933X)
 		return -EINVAL;
@@ -408,7 +480,8 @@ static int ar933x_uart_verify_port(struct uart_port *port,
 	if (ser->irq < 0 || ser->irq >= NR_IRQS)
 		return -EINVAL;
 
-	if (ser->baud_base < 28800)
+	if (ser->baud_base < up->min_baud ||
+	    ser->baud_base > up->max_baud)
 		return -EINVAL;
 
 	return 0;
@@ -561,6 +634,7 @@ static int __devinit ar933x_uart_probe(struct platform_device *pdev)
 	struct uart_port *port;
 	struct resource *mem_res;
 	struct resource *irq_res;
+	unsigned int baud;
 	int id;
 	int ret;
 
@@ -611,6 +685,12 @@ static int __devinit ar933x_uart_probe(struct platform_device *pdev)
 	port->fifosize = AR933X_UART_FIFO_SIZE;
 	port->ops = &ar933x_uart_ops;
 
+	baud = ar933x_uart_get_baud(port->uartclk, AR933X_UART_MAX_SCALE, 1);
+	up->min_baud = max_t(unsigned int, baud, AR933X_UART_MIN_BAUD);
+
+	baud = ar933x_uart_get_baud(port->uartclk, 0, AR933X_UART_MAX_STEP);
+	up->max_baud = min_t(unsigned int, baud, AR933X_UART_MAX_BAUD);
+
 	ar933x_uart_add_console_port(up);
 
 	ret = uart_add_one_port(&ar933x_uart_driver, &up->port);
-- 
1.7.10


^ permalink raw reply related

* Comments requested: driver for Quatech PCI serial cards
From: Jonathan Woithe @ 2012-11-14  6:07 UTC (permalink / raw)
  To: linux-serial; +Cc: jwoithe

Quatech (now B&B Electronics) make a range of multiport PCI serial cards for
RS232 and RS422 connectivity.  A GPL Linux driver is available on the
Quatech web site (http://www.quatech.com/support/drivers_pci.php) but as far
as I can tell no attempt has been made to mainline it.  The driver's
maintenance by Quatech appears to have ceased in 2007 and as a result, the
driver they provide no longer compiles against mainline.

The driver supports a wide range of Quatech DSC/ESC/QSC cards, including the
custom functionality which allow various aspects of the signal routing to be
altered under software control.

I have ported the driver to compile again (most recent kernel tested is
currently 3.4.4).  I've also cleaned up some formatting issues, removed the
most obvious redundant code and fixed a few bugs I discovered along the way. 
The result consists of two files which I have included at the end of this
message.

I am fully aware that there are still a bunch of things which need to be
done before this can be considered for mainline: things like the
module/filename naming convention for starters.  What I would like to
determine is how much extra work would be required before this module could
be considered for mainline.  While I do not have the time or resources to
embark on a full rewrite, I am very willing to address stylistic issues or
minor quibbles with the code.

Things I know will need addressing:
 - debug / printk usage
 - indenting convention
 - naming conventions, especially of internal structures
 - the name of the module.  Anything with "qt" in it will confuse people.
   I'm thinking along the lines of serial-quatech or quatech-serial:
   comments welcome.

Things I've currently only guessed (and which therefore could be completely
off-beam):
 - the compat_ioctl stuff
 - locking requirements within the ioctl handler

One issue which might raise eyebrows is the use of some new ioctls.  I'm
guessing that ideally this sort of thing would be done either via sysfs or
procfs now.  The problem is that Quatech supply some binary applications to
manipulate various non-standard features of these cards, and they expect
ioctls.  While all the information required to write replacements which use
a more acceptable interface is available in the driver source, I would
prefer to defer this until later if at all possible.

I would appreciate comments in relation to this driver so I can determine
whether or not it's feasible for me to get it into shape for formal
mainline submission.

Regards
  jonathan

----

SerQT_PCI.c

/*
 * Driver for Quatech PCI serial cards
 *
 * Copyright (C) 2004, 2005, 2007 Tim Gobeli <tgobeli@quatech.com>
 * Copyright (C) 2012 Jonathan Woithe <jwoithe@just42.net>
 *
 * 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 <linux/version.h>
#include <linux/module.h>
#include "serialqt_pci.h"

// Uncomment for debugging
//#define DEBUG_ON

#ifdef DEBUG_ON
    #define  dbgprint(const...)    printk(const)

#else
    #define  dbgprint(const...)
#endif

MODULE_AUTHOR("Tim Gobeli <tgobeli@quatech.com>");
MODULE_DESCRIPTION("Driver for PCI Quatech serial port adapters, v1.28");
MODULE_LICENSE("GPL");

static struct pci_dev_link_t *pPci_portlist = NULL;
static int major_number;

static long int ioctl_serial_pci (struct file *filp, unsigned int cmd,
                                     unsigned long arg);

#ifdef CONFIG_COMPAT
static long int compat_ioctl_serial_pci (struct file *filp, unsigned int cmd,
                                       unsigned long arg);
#endif

static void multi_config(struct pci_dev *pDev, multi_id_t multi_info, int index);

static unsigned SerialReadQOPR_Reg(unsigned Base_Address);

static void SerialWriteQOPR_Reg(unsigned Base_Address, bool internal, unsigned Arg);

static void SerialWriteQMCR_Reg(unsigned Base_Address, unsigned Arg);

static unsigned SerialReadQMCR_Reg(unsigned Base_Address);

static int SerialDoes422_Exist(unsigned Base_Address);

static int SerialDoesQMCR_Exist(unsigned Base_Address);

static bool SerialLockOutOverride(unsigned Base_Address);

static bool SerialClkCtrlOverride(unsigned Base_Address, unsigned long *pClock_Rate);

struct file_operations serialqt_pci_fops = {
    unlocked_ioctl:  ioctl_serial_pci,
#ifdef CONFIG_COMPAT
    compat_ioctl:    compat_ioctl_serial_pci,
#endif

};


long int ioctl_serial_pci (struct file *filp, unsigned int cmd, 
                              unsigned long arg)
{
    unsigned long irqflags;
    unsigned ud_QT_Baseaddress, err;
    unsigned ucOPR_OrgValue, ucOPR_NewValue, uc_Value;
    int *p_Num_of_adapters, counts, index, *p_QMCR_Value;
    struct pci_dev *dev = NULL;
    Identity_struct *p_Identity_of;
    Identity_struct  Identity_of;

    struct pci_dev_link_t *pTemp_dev_link ;
    dbgprint(KERN_DEBUG"ioctl_serial_pci  cmd =\n");
     if (_IOC_TYPE(cmd) != SERIALQT_PCI_IOC_MAGIC)
        return -ENOTTY;
    if (_IOC_NR(cmd) > SERIALQT_IOC_MAXNR)
        return -ENOTTY;
    dbgprint(KERN_DEBUG"ioctl_serial_pci  cmd = 0x%x\n", cmd);
        err = 0;

    switch (cmd)
    {
    case SERIALQT_WRITE_QOPR: 
        pTemp_dev_link = pPci_portlist;
        index = arg >> 16;
        counts = 0;
        // Go through the whole list and find the  adapter
        while (pTemp_dev_link)
        {
            dbgprint(KERN_DEBUG"ioctl_serial_pci  arg = 0x%lx\n", arg);
            // Setting one port QOPR on multiport adapter sets them all  
            if (pTemp_dev_link->Adapter_Index == index)
            {
                // We've found the adapter we want, lets change the QOPR time 
                // setting accordingly
                spin_lock_irqsave(pTemp_dev_link->lock, irqflags);
                ud_QT_Baseaddress = pTemp_dev_link->ulPortStart;
                ucOPR_OrgValue =  SerialReadQOPR_Reg(ud_QT_Baseaddress);
                // Just mask off the last two bits of the QOPR register 
                // because thats all that needs changed there for clock 
                // rate control
                ucOPR_NewValue = ucOPR_OrgValue & 0xfc;
                ucOPR_NewValue = ucOPR_NewValue | arg;
                SerialWriteQOPR_Reg(ud_QT_Baseaddress, FALSE, ucOPR_NewValue);
                // We're done
                spin_unlock_irqrestore(pTemp_dev_link->lock, irqflags);
                break;
            } else
            {
                pTemp_dev_link = pTemp_dev_link->next;
            }

        }  // end while
        break;

    case SERIALQT_WRITE_QMCR:
        // initialize as error so if we don't find this one we give
        err = -ENOTTY; 
        pTemp_dev_link = pPci_portlist;
        index = arg >> 16;

        // Go through the whole list and find the  adapter
        while (pTemp_dev_link != NULL)
        {
            dbgprint(KERN_DEBUG"ioctl_serial_pci  arg = 0x%lx\n", arg);
            if (pTemp_dev_link->Adapter_Index == index)
            {
                // must set each ports QMCR individually
                while ((pTemp_dev_link != NULL) && 
                       (pTemp_dev_link->Adapter_Index == index))
                {
                    spin_lock_irqsave(pTemp_dev_link->lock, irqflags);
                    // We've found the adapter we want, lets change the 
                    // QMCR time setting accordingly
                    ud_QT_Baseaddress = pTemp_dev_link->ulPortStart;
                    ucOPR_NewValue = arg;

                    dbgprint(KERN_DEBUG"ioctl SERIALQT_WRITE_QMCR:  Base Addrss = 0x%x,  value = 0x%x\n", ud_QT_Baseaddress, ucOPR_NewValue);
                    SerialWriteQMCR_Reg(ud_QT_Baseaddress, ucOPR_NewValue );
                    err = 0;
                    spin_unlock_irqrestore(pTemp_dev_link->lock, irqflags);
                    pTemp_dev_link = pTemp_dev_link->next;
                }
                // We're done
                break;
            } else
            {
                pTemp_dev_link = pTemp_dev_link->next;
            }
        }  // end while
        break;

    case SERIALQT_READ_QMCR:
        // initialize as error so if we don't find this one we give an error.
        err = -ENOTTY;
        p_QMCR_Value =  (int *)arg;
        pTemp_dev_link = pPci_portlist;
        index = arg >> 16;
        counts = 0;
        while (pTemp_dev_link != NULL)
        {
            if (pTemp_dev_link->Adapter_Index == index)
            {
                spin_lock_irqsave(pTemp_dev_link->lock, irqflags);
                ud_QT_Baseaddress = pTemp_dev_link->ulPortStart;
                uc_Value =  SerialReadQMCR_Reg(ud_QT_Baseaddress);
                err = put_user(uc_Value, p_QMCR_Value);
                spin_unlock_irqrestore(pTemp_dev_link->lock, irqflags);
                break;   // We're done
            } else
            {
                pTemp_dev_link = pTemp_dev_link->next;
            }
        }
        break;

    case SERIALQT_GET_NUMOF_UNITS: 
        p_Num_of_adapters = (int *)arg;
        counts = 0;
        dbgprint(KERN_DEBUG"SERIALQT_GET_NUMOF_UNITS \n");

        for (index = 0; index < (sizeof(multi_id)/sizeof( multi_id_t)); index++)
        {
            dev = pci_get_subsys(QUATECH_VID, multi_id[index].prodid, 
                                    PCI_ANY_ID, PCI_ANY_ID, NULL);
            while (dev != NULL) // find multiple devices of same signature
            {
                counts++;
                dev = pci_get_subsys(QUATECH_VID, multi_id[index].prodid, 
                                        PCI_ANY_ID, PCI_ANY_ID, dev);
            }
        }
        dbgprint(KERN_DEBUG"ioctl_serial_pci writting counts = %d", counts);
        err = put_user(counts, p_Num_of_adapters);
        break;

    case SERIALQT_GET_THIS_UNIT:
        counts = 0;
        p_Identity_of = (Identity_struct *)arg;
        // copy user structure to local variable

        get_user(Identity_of.index, &p_Identity_of->index);
        dbgprint(KERN_DEBUG"SERIALQT_GET_THIS_UNIT Identity_of.index= 0x%x\n", Identity_of.index);

        // initialize as error so if we don't find this one we give an error.
        err = -ENOTTY; 

        pTemp_dev_link = pPci_portlist;
        while (pTemp_dev_link != NULL)
        {
            if (pTemp_dev_link->Adapter_Index == Identity_of.index)
            {
                err = put_user(pTemp_dev_link->prodid, &p_Identity_of->n_identity);
                break;   // We're done

            } else
            {
                pTemp_dev_link = pTemp_dev_link->next;
            }
        }
        break;

    case SERIALQT_IS422_EXTENDED:
        err = -ENOTTY; // initialize as error so if we don't find this one we give
        dbgprint(KERN_DEBUG"SERIALQT_IS422_EXTENDED \n");
        pTemp_dev_link = pPci_portlist;
        index = arg >> 16;

        while (pTemp_dev_link != NULL)
        {
            if (pTemp_dev_link->Adapter_Index == index)
            {
                dbgprint(KERN_DEBUG"Found Adapter \n");

                ud_QT_Baseaddress = pTemp_dev_link->ulPortStart;
                if (SerialDoesQMCR_Exist(ud_QT_Baseaddress))
                {
                    err = 1;
                    break;   // no MCR so return -ENOTTY;
                }
                if (SerialDoes422_Exist(ud_QT_Baseaddress))
                {
                    err = 2;
                    break;
                }
                dbgprint(KERN_DEBUG"SERIALQT_IS422_EXTENDED err = 0\n");

                // If we got this far then it is a 422/485 extended feature 
                // device
                err = 0; 
                break; 
            } else
            {
                pTemp_dev_link = pTemp_dev_link->next;
            }
        }
        break;

    default:
        err = -ENOTTY;

    } // End switch

    return err;
}

#ifdef CONFIG_COMPAT
long int compat_ioctl_serial_pci (struct file *filp, unsigned int cmd,
                                     unsigned long arg)
{
    long int err = 0;
    unsigned long compat_arg = 0;
    switch (cmd)
    {
    case SERIALQT_WRITE_QOPR: 
    case SERIALQT_WRITE_QMCR:
    case SERIALQT_IS422_EXTENDED:
        /* arg is an integer value */
        compat_arg = arg;
        break;
    case SERIALQT_READ_QMCR:
    case SERIALQT_GET_NUMOF_UNITS:
    case SERIALQT_GET_THIS_UNIT:
        /* arg is a pointer */
        compat_arg = (unsigned long)compat_ptr(arg);
        break;
    default:
        err = -ENOTTY;
    }
    if (err == 0)
        err = ioctl_serial_pci(filp, cmd, compat_arg);

    return err;
}
#endif

void SerialWriteQOPR_Reg(unsigned Base_Address, bool internal, unsigned Arg)
{
    unsigned    ulQT_ORaddress, ulQT_LCRaddress;
    unsigned    ucLCR_OrgValue, ucOPR_OrgValue, ucOPR_NewValue;

    // We only allow the last two bits to change on the QOR register
    ulQT_ORaddress = Base_Address + 7;
    ulQT_LCRaddress = Base_Address + 3;
    // Set DLAB to access options register
    ucLCR_OrgValue = inb(ulQT_LCRaddress);
    dbgprint(KERN_DEBUG"Read LCR at = 0x%x value = 0x%x\n", ulQT_LCRaddress, ucLCR_OrgValue);

    outb(0xbf, ulQT_LCRaddress);
    dbgprint(KERN_DEBUG" Wrote LCR at = 0x%x value = 0x%x\n", ulQT_LCRaddress, 0xbf);

    ucOPR_OrgValue = inb(ulQT_ORaddress);
    // If this comes from user -Just mask off the last two bits of the QOPR register cause thats all 
    // that needs changed there for clock rate control
    if(internal == FALSE)
    {
         ucOPR_NewValue = ucOPR_OrgValue & 0xfc;
         ucOPR_NewValue = ucOPR_NewValue | Arg; 
         dbgprint(KERN_DEBUG"Read qopr at = 0x%x value = 0x%x\n", ulQT_ORaddress, ucOPR_OrgValue);
    }
    else
        ucOPR_NewValue = Arg;

    //Write new value to options register
    outb(ucOPR_NewValue, ulQT_ORaddress );

    dbgprint(KERN_DEBUG"SerialWriteQOPR_Reg: Wrote qopr at = 0x%x value = 0x%x\n", ulQT_ORaddress, ucOPR_NewValue);

    // Read the value back for testing purposes
    // ucLCR_OrgValue = 0;
    // ucLCR_OrgValue = inb(ulQT_ORaddress);
    // dbgprint(KERN_DEBUG"Read qopr at = 0x%x value = 0x%x", ulQT_ORaddress, ucLCR_OrgValue);

    // Restore origninal lCR value
    outb(ucLCR_OrgValue, ulQT_LCRaddress ); 
}

unsigned SerialReadQOPR_Reg(unsigned Base_Address)
{
    unsigned    ulQT_ORaddress, ulQT_LCRaddress;
    unsigned    ucLCR_OrgValue, ucOPR_Value;

    //We only allow the last two bits to change on the QOR register
    ulQT_ORaddress = Base_Address + 7;
    ulQT_LCRaddress = Base_Address + 3;
    // Set DLAB to access options register
    ucLCR_OrgValue = inb(ulQT_LCRaddress);

    outb(0xbf, ulQT_LCRaddress);
     ucOPR_Value = inb(ulQT_ORaddress);
    // Restore origninal lCR value
    outb(ucLCR_OrgValue, ulQT_LCRaddress ); 
    dbgprint(KERN_DEBUG" SerialReadQOPR_Reg:Read QOR at Base = 0x%x value = 0x%x\n", Base_Address, ucOPR_Value);

    return ucOPR_Value;  
}

void SerialWriteQMCR_Reg(unsigned Base_Address, unsigned Arg)
{
    unsigned    ulQT_ORaddress, ulQT_LCRaddress, ulQT_MCRaddress;
    unsigned    ucLCR_OrgValue, ucOPR_OrgValue;

    // We only allow the last two bits to change on the QOR register
    ulQT_ORaddress = Base_Address + 7;
    ulQT_LCRaddress = Base_Address + 3;
    ulQT_MCRaddress = Base_Address + 4;
    // Set DLAB to access options register
    ucLCR_OrgValue = inb(ulQT_LCRaddress);

    outb(0xbf, ulQT_LCRaddress);

    ucOPR_OrgValue = inb(ulQT_ORaddress);
    // Set options register QLAB bit to access QMCR register
    outb(ucOPR_OrgValue | 0x10, ulQT_ORaddress);
    // Now write to the QMCR register
    outb(Arg, ulQT_MCRaddress);

    // Restore original contents of QOPR register
    outb(ucLCR_OrgValue, ulQT_LCRaddress );

    // Restore origninal lCR value
    outb(ucLCR_OrgValue, ulQT_LCRaddress ); 
}

unsigned SerialReadQMCR_Reg(unsigned Base_Address){

    unsigned    ulQT_ORaddress, ulQT_LCRaddress, ulQT_MCRaddress;
    unsigned    ucLCR_OrgValue, ucOPR_OrgValue, ucMCR_Value;

    // We only allow the last two bits to change on the QOR register
    ulQT_ORaddress = Base_Address + 7;
    ulQT_LCRaddress = Base_Address + 3;
    ulQT_MCRaddress = Base_Address + 4;
    // Set DLAB to access options register
    ucLCR_OrgValue = inb(ulQT_LCRaddress);

    outb(0xbf, ulQT_LCRaddress);

    ucOPR_OrgValue = inb(ulQT_ORaddress);
    // Set options register QLAB bit to access QMCR register
    outb(ucOPR_OrgValue | 0x10, ulQT_ORaddress);
    // Now read the QMCR register
    ucMCR_Value = inb(ulQT_MCRaddress);

    // Restore original contents of QOPR register
    outb(ucOPR_OrgValue, ulQT_ORaddress );

    // Restore origninal lCR value
    outb(ucLCR_OrgValue, ulQT_LCRaddress ); 

    return ucMCR_Value;
}

// SerialDoesQMCR_Exist tests for existence of QMCR register and returns 0
// valueif it does and nonzero if it doesn't 
int SerialDoesQMCR_Exist(unsigned Base_Address)
{
    unsigned    ulQT_ORaddress, ulQT_LCRaddress, ulQT_MCRaddress;
    unsigned    ucLCR_OrgValue, uc_Value;
    int        Status = 0;

    // We only allow the last two bits to change on the QOR register
    ulQT_ORaddress = Base_Address + 7;
    ulQT_LCRaddress = Base_Address + 3;
    ulQT_MCRaddress = Base_Address + 4;
    // Set DLAB to access options register
    ucLCR_OrgValue = inb(ulQT_LCRaddress);
    // Writting a 0xbf to lcr will make bet 5 of QOR go high if QOR and QMCR exist
    outb(0xbf, ulQT_LCRaddress);
    uc_Value = inb(ulQT_ORaddress);
    if (!(uc_Value & 0x20))//Is bit 5 (IDX) set
    {
        dbgprint(KERN_DEBUG"QMCR bit b5 not set \n");
        Status = -ENOTTY;
    }

    if (Status == 0)
    {
        // So far so good now write something other than 0xbf to the LCR to set IDX to 0
        // and test
        outb(0x80, ulQT_LCRaddress);
        // IDX should be clear
        uc_Value = inb(ulQT_ORaddress);
        if (uc_Value & 0x20)
            Status = -ENOTTY;

    }
    outb(ucLCR_OrgValue, ulQT_LCRaddress); // Restore original value
    return Status;
}

int SerialDoes422_Exist(unsigned Base_Address)
{
    int Status;
    unsigned ucMCR_OrgValue, uc_Value;
    ucMCR_OrgValue = SerialReadQMCR_Reg(Base_Address);
    // Essentially we're ging to write a value of 0xff to the QMCR register and 
    // if we read back a nonzero value, then its a 422 device otherwize its 422
    uc_Value = 0xff;
    SerialWriteQMCR_Reg(Base_Address, uc_Value);
    uc_Value = 0;
    uc_Value = SerialReadQMCR_Reg(Base_Address);
    if (uc_Value)
        Status = 0;
    else
        Status = -ENOTTY;

    SerialWriteQMCR_Reg(Base_Address, ucMCR_OrgValue);

    return Status;
}

int init_serial_pci(void){

    struct pci_dev *dev;

    int index;
    int status = 0;
    int Adapter_Index = 0;

    dev = NULL;

    dbgprint(KERN_DEBUG"Entering SerialQT_PCI \n\n");

    // Find the given adapters and register them accordingly with the serial 
    // core
    for (index = 0; index < (sizeof(multi_id)/sizeof( multi_id_t)); index++)
    {
        dev = pci_get_subsys(QUATECH_VID, multi_id[index].prodid, PCI_ANY_ID, 
                                PCI_ANY_ID, NULL);
        if (dev != NULL)
        {
            while (dev != NULL) // find multiple devices of same signature
            {
                status++;
                status = pci_enable_device(dev);
                if(status < 0)
                {
                    dbgprint(KERN_DEBUG"SerialQT_PCI - Unable to enable device \n\n");
                    return -EBUSY;
                }
                multi_config(dev, multi_id[index], Adapter_Index);
                Adapter_Index++; // index for next one
                dev = pci_get_subsys(QUATECH_VID, multi_id[index].prodid, 
                                        PCI_ANY_ID, PCI_ANY_ID, dev);
            }
        }
    }

    if (Adapter_Index == 0)
        dbgprint(KERN_DEBUG"No devices found \n\n");

    status = 0; // Dynamic assignment of major number
    major_number = register_chrdev(status, "SerialQT_PCI", &serialqt_pci_fops);
    if (major_number < 0)
    {
        dbgprint(KERN_DEBUG"No devices found \n\n");
        return -EBUSY;
    } else
       dbgprint(KERN_DEBUG"SerQT_PCI major number assignment = %d \n\n", major_number);
    return 0;
}

// multi_config()
// Inputs:
//  * Struct pci_dev * passed from successful pci_get_subsys()
//  * multi_id_t structure of device found
//
// Description:
//   Determines resources used by an adapter and uses them to register the
//   ports.  It then saves pertinent information for each port in a linked 
//   list for later use to configure the adapter per users request.

static void multi_config(struct pci_dev *pDev, multi_id_t multi_info, int Adapter_Index){

    int index;
    unsigned int ulAddressStart, ulPortAddress;
    unsigned int Irq;
    int nFound = 0;
    struct pci_dev_link_t *pTemp_dev_link;
    #if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,9)
    struct uart_port serial;
    #else
    struct serial_struct serial;
    #endif
    int line;
    int nNum_of_Ports = multi_info.multi;
    unsigned long Clock_Rate;
    u32 tmp;
   
    Irq = pDev->irq;
    dbgprint(KERN_DEBUG"Device = 0x%x, Irq = %d\n", pDev->device, Irq);

    // Set byte in the Amcc dchiep in the extremly unlikely event that the
    // system bios didn't run the ROM bios extension on our pci card
    if(multi_info.AMCC == TRUE)
    {
        ulPortAddress = pci_resource_start(pDev, 0);
        ulAddressStart = ulPortAddress + 0x38;
        tmp = inl(ulAddressStart);
        tmp = tmp | 0x00002000;
        dbgprint(KERN_DEBUG"Bios work around, writing value = 0x%x Device = 0x%x, at 0x%x\n",tmp, pDev->device, ulAddressStart);

        outl(tmp, ulAddressStart);

        ulAddressStart = ulPortAddress + 0x3c;
		
        tmp = inl(ulAddressStart);
        tmp = tmp |  0x01000000;
        outl(tmp, ulAddressStart);
        tmp &= ~0x01000000;
        outl(tmp, ulAddressStart);
    }

    ulAddressStart = pci_resource_start(pDev, multi_info.IO_Region);

    dbgprint(KERN_DEBUG"Device = 0x%x, Address = 0x%x", pDev->device, ulAddressStart);

    // SerialClkCtrlOverride will set the adapter clock rate to highest
    // available rate and return that rate in the varible
    SerialClkCtrlOverride((unsigned)ulAddressStart, &Clock_Rate);

    if (ulAddressStart != 0)
        nFound = 1;
    else
        nFound = 0;

    if (nFound)
    {
        for (index = 0; index < nNum_of_Ports; index++)
        {
            ulPortAddress = ulAddressStart + (index * 8);
            memset(&serial, 0, sizeof(serial));
            serial.irq = Irq;
            #if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,9)
            serial.iobase = ulPortAddress;
            serial.flags = UPF_BOOT_AUTOCONF | UPF_SKIP_TEST | UPF_SHARE_IRQ;
            serial.uartclk = Clock_Rate;
            line = serial8250_register_port(&serial);
            #else
            serial.port = ulPortAddress;
            serial.flags = ASYNC_SKIP_TEST | ASYNC_SHARE_IRQ;
            serial.baud_base = Clock_Rate;
            line = register_serial(&serial);
            #endif
                       
            if (line < 0)
            {
                dbgprint(KERN_DEBUG"<1>SerQT_PCI:  register_serial() at 0x%04lx,irq %d failed\n",(u_long)ulPortAddress, serial.irq); 
                return;
            } else
            {
                dbgprint(KERN_DEBUG"SerQT_PCI:  register_serial() at 0x%04lx,irq %d, ttyS%d succeded\n",(u_long)ulPortAddress, serial.irq, line); 
            }
            pTemp_dev_link = kmalloc(sizeof(struct pci_dev_link_t), GFP_KERNEL);
            if (!pTemp_dev_link)
            {
                dbgprint(KERN_DEBUG"IN %s insufficient resources\n", __FUNCTION__);
                #if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,9)
                serial8250_unregister_port(line);
                #else
                unregister_serial(line);
                #endif
                
                return;
            } else
            {
                memset(pTemp_dev_link, 0, sizeof(struct pci_dev_link_t));
                serial.line = line;
            }
            // Create a list of ports with information we'll use to custom 
            // configure it
            pTemp_dev_link->line = serial.line;
            pTemp_dev_link->prodid =  multi_info.prodid;
            pTemp_dev_link->type = multi_info.type;
            pTemp_dev_link->Port_Number = index;
            pTemp_dev_link->ulPortStart = ulPortAddress;
            pTemp_dev_link->Adapter_Index = Adapter_Index;
            pTemp_dev_link->lock = &serial.lock;
            pTemp_dev_link->next = (void *)pPci_portlist;
            pPci_portlist = pTemp_dev_link;
        }   // end for
    } else
    {
        dbgprint(KERN_DEBUG"Couldn't find device\n");
    }// end if
}

// SerialClkCtrlOverride()
//
// Called from muli_config() to to determine if overrides to the Clock mode
// of the hardware feature have been set via jumper configurations
// also determines max clock multiplier and sets the adapter accordingly
// (some cards have a max multiplier of X4 some of X8
//
// Inputs: 
//  * Base_Address     Adapter base address
//
// Outputs: None
//
// Returns:
//  * True if no QOPR or locked out, also appropriate clocke rate is
//    returned in the pClock_Rate parameter

bool SerialClkCtrlOverride(unsigned Base_Address, unsigned long *pClock_Rate)
{
    unsigned QOPR_OrgValue, QOPR_Value, TestOutValue, Auto_QOPR_Setting;
    bool status;

    status =  SerialLockOutOverride(Base_Address);
    // If no QOR we disable all these selections
    if (status == TRUE)
    {
        dbgprint(KERN_DEBUG"SerialClkCrtlOverride, no QOR\n"); 

        *pClock_Rate =  CLOCK_X1; // No Qor so use standard rate and return
        return status;
    }

    QOPR_OrgValue = SerialReadQOPR_Reg(Base_Address);
    QOPR_Value = QOPR_OrgValue & (~QOPR_CLOCK_X8); // Clear clk rate bits

    SerialWriteQOPR_Reg(Base_Address, FALSE, QOPR_Value);

    TestOutValue = SerialReadQOPR_Reg(Base_Address);
    // Those bits shoud be 0, if not we're overridden
    if (TestOutValue &= QOPR_CLOCK_X8)
    {
        *pClock_Rate =  CLOCK_X1; // Set to standard rate if overridden
        status =  TRUE;
    }
    else
    {
        // Now try setting them to "1" and see if any of it sticks
        // some cards only go X4 so only hi bit sticks
        QOPR_Value = QOPR_Value | QOPR_CLOCK_X8; // Set clk rate bits


        SerialWriteQOPR_Reg(Base_Address, TRUE, QOPR_Value);

        TestOutValue = SerialReadQOPR_Reg(Base_Address);
        if (TestOutValue &=QOPR_CLOCK_X8)
            status =  FALSE;  // It stuck, we're not overridden
        else
        {
            *pClock_Rate =  CLOCK_X1; // Set to standard rate if overridden
            status =  TRUE; // It didn't stick, we are overridden
        }

    }

    // Ok were not forced but the max rate is not the same for all boards
    // lets find out what the max is and  
    if (status == FALSE)
    {
        TestOutValue = SerialReadQOPR_Reg(Base_Address);
        TestOutValue &=QOPR_CLOCK_X8;
        switch (TestOutValue)
        {
        case QOPR_CLOCK_X2:
            *pClock_Rate =  CLOCK_X2;
            Auto_QOPR_Setting = QOPR_CLOCK_X2;
            break;

        case QOPR_CLOCK_X4:
            *pClock_Rate =  CLOCK_X4;
            Auto_QOPR_Setting = QOPR_CLOCK_X4;
            break;

        case QOPR_CLOCK_X8:
            *pClock_Rate =  CLOCK_X8;
            Auto_QOPR_Setting = QOPR_CLOCK_X8;
            break;
            break;
        default:
            *pClock_Rate =  CLOCK_X1;
             Auto_QOPR_Setting = QOPR_CLOCK_X1;
        }

        QOPR_OrgValue = QOPR_OrgValue & (~QOPR_CLOCK_MASK); // clear clock bits
        QOPR_OrgValue = QOPR_OrgValue | Auto_QOPR_Setting; // Set clock bits
        // set adapter clock
        SerialWriteQOPR_Reg(Base_Address, TRUE,  QOPR_OrgValue);  
    }
    // set adapter clock
    SerialWriteQOPR_Reg(Base_Address, TRUE, QOPR_OrgValue);

    dbgprint(KERN_DEBUG"<1>SerialClkCtrlOverride:  Writing to QOPR value =  0x%04lx\n",(u_long)QOPR_OrgValue); 

    return status;
}

// SerialLockOutOverride()
//
// Check on the presences of QOR and return true if we don't get the 
// desired test pattern resuls
//
// Inputs: base address
//
// Outputs: none
//
// Returns:
//  * False if QOPR exist
//  * TRUE if jumper overridden on Qopr isn't there

bool SerialLockOutOverride(unsigned Base_Address)
{
    unsigned QOPR_OrgValue, QOPR_Value, TestOutValue;

    QOPR_OrgValue =  SerialReadQOPR_Reg( Base_Address);

    QOPR_Value = QOPR_OrgValue & QPCR_TEST_FOR1; // Clear ID0, ID1 bits
    SerialWriteQOPR_Reg(Base_Address, TRUE, QOPR_Value);


    TestOutValue = SerialReadQOPR_Reg(Base_Address);
    TestOutValue &= 0xc0;  // Mask ID0 & ID1
    if (TestOutValue != QPCR_TEST_GET1)
        return TRUE;

    QOPR_Value = QOPR_OrgValue & QPCR_TEST_FOR1; // Clear ID0, ID1 bits
    QOPR_Value = QOPR_Value | QPCR_TEST_FOR2; // set ID0, ID1 bits

    SerialWriteQOPR_Reg(Base_Address, TRUE, QOPR_Value);
    TestOutValue = SerialReadQOPR_Reg(Base_Address);

    TestOutValue &= 0xc0;    // mask off ID0 and ID1 bits

    if (TestOutValue != QPCR_TEST_GET2)
        return TRUE;

    QOPR_Value = QOPR_OrgValue & QPCR_TEST_FOR1; // Clear ID0, ID1 bits
    QOPR_Value = QOPR_Value | QPCR_TEST_FOR3; // set ID0, ID1 bits

    SerialWriteQOPR_Reg(Base_Address, TRUE, QOPR_Value);
    TestOutValue = SerialReadQOPR_Reg(Base_Address);

    TestOutValue &= 0xc0;    // mask off ID0 and ID1 bits

    if (TestOutValue != QPCR_TEST_GET3)
        return TRUE;

    QOPR_Value = QOPR_OrgValue & QPCR_TEST_FOR1; // Clear ID0, ID1 bits
    QOPR_Value = QOPR_Value | QPCR_TEST_FOR4; // set ID0, ID1 bits

    SerialWriteQOPR_Reg(Base_Address, TRUE, QOPR_Value);
    TestOutValue = SerialReadQOPR_Reg(Base_Address);

    TestOutValue &= 0xc0;    // mask off ID0 and ID1 bits

    SerialWriteQOPR_Reg(Base_Address, TRUE, QOPR_OrgValue); // Restore value


    if (TestOutValue != QPCR_TEST_GET4)
        return TRUE;

    return FALSE;
}

void exit_serial_pci(void){
    struct pci_dev_link_t *pTemp_dev_link;
    while (pPci_portlist != NULL)
    {              
        #if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,9)
        serial8250_unregister_port(pPci_portlist->line);
        #else
        unregister_serial(pPci_portlist->line);
        #endif
        
        pTemp_dev_link = pPci_portlist->next;
        kfree(pPci_portlist);
        pPci_portlist = pTemp_dev_link;
    }

    unregister_chrdev(major_number, "SerialQT_PCI");
}

module_init(init_serial_pci);
module_exit(exit_serial_pci);

----

serialqt_pci.h

/*
 * serialqt_pci.h
 *
 * Copyright (C) 2004, 2005, 2007 Tim Gobeli <tgobeli@quatech.com>
 * Copyright (C) 2012 Jonathan Woithe <jwoithe@just42.net>
 *
 * 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
 */

#ifndef __KERNEL__
#define __KERNEL__
#endif

#ifndef MODULE
#define MODULE
#endif

#ifndef EXPORT_SYMTAB
#define EXPORT_SYMTAB
#endif

#include <config/modules.h>

#include <linux/errno.h>
#include <asm/io.h>
#include <linux/pci.h>

#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,9)
#include <linux/serial_8250.h>
#else
#include <linux/serial.h>
#endif

#include <asm/uaccess.h>
#include <linux/init.h>

#define TRUE  1
#define FALSE 0
#define bool  int

#define QSC100          0x0010
#define DSC100          0x0020	
#define DSC100E         0x0181
#define DSC200          0x0030
#define DSC200E         0x01B1
#define QSC200          0x0040
#define ESC100          0x0050
#define ESC100_1        0x0060
#define QSCP100         0x0120
#define DSCP100         0x0130
#define QSCP200         0x0140
#define DSCP200         0x0150
#define QSCLP100        0x0170
#define DSCLP100        0x0180
#define SSCLP100        0x0190
#define QSCLP200        0x01A0
#define DSCLP200        0x01B0
#define SSCLP200        0x01C0
#define ESCLP100        0x01E0

#define QUATECH_VID     0x135C

#define RS232 0
#define RS422 1

#define BUFFER_LENGTH   50

typedef struct
{
    int     prodid;
    int     multi;           /* 1 = multifunction, > 1 = # ports */
    int     extended;        /* 1 = extended feature, 1 = no extended feature */
    int     type;            /* RS232 or RS422 */
    int     IO_Region;       /* I/O region of funcitonal I/O range*/ 
    bool    AMCC;
} multi_id_t;

// This is used to create a linked list of ports and to associate them with 
// a given adapter
struct pci_dev_link_t
{
    struct          pci_dev_link_t *next;
    int             prodid;
    unsigned int    ulPortStart;
    int             type;
    int             line;
    int             Port_Number;
    int             Adapter_Index;
    spinlock_t      *lock;    /* Pointer to the mutex used by serial core */
};

static multi_id_t multi_id[] =
{  
    {QSC100,    4,  1,  RS232,  1, TRUE}, 
    {DSC100,    2,  1,  RS232,  1, TRUE},
    {DSC100E,   2,  1,  RS232,  2, FALSE},
    {DSC200,    2,  1,  RS422,  1, TRUE},
    {DSC200E,    2,  1,  RS422,  2, FALSE},
    {QSC200,    4,  1,  RS422,  1, TRUE},
    {ESC100,    8,  1,  RS232,  1, TRUE},
    {ESC100_1,  8,  1,  RS232,  1, TRUE},
    {QSCP100,   4,  1,  RS232,  1, TRUE},
    {DSCP100,   2,  1,  RS232,  1, TRUE},
    {QSCP200,   4,  1,  RS422,  1, TRUE},
    {DSCP200,   2,  1,  RS422,  1, TRUE},
    {ESCLP100,  8,  1,  RS232,  0, FALSE},
    {QSCLP100,  4,  1,  RS232,  2, FALSE},
    {DSCLP100,  2,  1,  RS232,  2, FALSE},
    {SSCLP100,  1,  1,  RS232,  2, FALSE},
    {QSCLP200,  4,  1,  RS422,  2, FALSE},
    {DSCLP200,  2,  1,  RS422,  2, FALSE}, 
    {SSCLP200,  1,  1,  RS422,  2, FALSE},
};

typedef struct
{
    int     index;
    int     n_identity;           
            
} Identity_struct;

#define SERIALQT_PCI_IOC_MAGIC 'k'
#define SERIALQT_WRITE_QOPR _IOW(SERIALQT_PCI_IOC_MAGIC, 0, int)
#define SERIALQT_WRITE_QMCR _IOW(SERIALQT_PCI_IOC_MAGIC, 1, int)
#define SERIALQT_GET_NUMOF_UNITS _IOR(SERIALQT_PCI_IOC_MAGIC, 2, void *)
#define SERIALQT_GET_THIS_UNIT _IOR(SERIALQT_PCI_IOC_MAGIC, 3, void *)
#define SERIALQT_READ_QOPR _IOR(SERIALQT_PCI_IOC_MAGIC, 4, int)
#define SERIALQT_READ_QMCR _IOR(SERIALQT_PCI_IOC_MAGIC, 5, int)
#define SERIALQT_IS422_EXTENDED _IOR(SERIALQT_PCI_IOC_MAGIC, 6, int)    //returns successful if 422 extended

#define SERIALQT_IOC_MAXNR 6

// Constants to Check for presence for QOR register
#define QPCR_TEST_FOR1    0x3f  // 
#define QPCR_TEST_GET1    0x00  // (0 0 x x x x x x) in gives (0 0 x x x x x x) out
#define QPCR_TEST_FOR2    0x40  // 
#define QPCR_TEST_GET2    0x040  // (0 1 x x x x x x) in gives (0 1 x x x x x x) out
#define QPCR_TEST_FOR3    0x80  // 
#define QPCR_TEST_GET3    0x40  // (1 0 x x x x x x) in gives (0 1 x x x x x x) out
#define QPCR_TEST_FOR4    0xc0  // 
#define QPCR_TEST_GET4    0x80  // (1 1 x x x x x x) in gives (1 0 x x x x x x) out

// Quatech Options Register DWORD "QOPR" (Hardware Register)
// Lower 8 bits defined by hardware registers, upper 8 bits custom flags
#define QOPR_CLOCK_AUTO    0x0100  // (bit 8) custom flag for "Auto" data rate multiplier mode
#define QOPR_CLOCK_X1      0x0000  // (x x x x x x 0 0) force X1 clock mode   
#define QOPR_CLOCK_X2      0x0001  // (x x x x x x 0 1) force X2 clock mode
#define QOPR_CLOCK_X4      0x0002  // (x x x x x x 1 0) force X4 clock mode
#define QOPR_CLOCK_X8      0x0003  // (x x x x x x 1 1) force X8 clock mode
#define QOPR_CLOCK_MASK    0x0103  // (1)(x x x x x x 1 1) used by software to mask off bits
#define QOPR_CLOCK_RATE_MASK    0x0003  // (1)(x x x x x x 1 1) used by software to mask off bits

// Quatech Hardware Options Enable Register DWORD "HWOR" (Software Register)
// low word, low byte in HWOR defines clock multiplier options available 
#define HWOR_CLOCK_X1      0x00000001  // (x x x x x x x 1) X1 clock mode available
#define HWOR_CLOCK_X2      0x00000002  // (x x x x x x 1 x) X2 clock mode available
#define HWOR_CLOCK_X4      0x00000004  // (x x x x x 1 x x) X4 clock mode available
#define HWOR_CLOCK_X8      0x00000008  // (x x x x 1 x x x) X8 clock mode available
#define HWOR_CLOCK_AUTO    0x00000080  // (1 x x x x x x x) auto clock mode available
#define HWOR_CLOCK_CLR     0xffffff00  // (0 0 0 0 0 0 0 0) auto clock mode available
#define HWOR_CLOCK_MASK    0x000000FF  // Bit mask used to determine whether to display Advanced property page

// low word, low byte in HWOR defines clock multiplier options available 
#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,9)
    #define CLOCK_X1           1843200  // X1 clock RATE
    #define CLOCK_X2           3685400  // X2 clock RATE
    #define CLOCK_X4           7372800  // X4 clock RATE
    #define CLOCK_X8           14745600 // (X8 clock RATE
#else
    #define CLOCK_X1           115200  // X1 clock RATE
    #define CLOCK_X2           230400  // X2 clock RATE
    #define CLOCK_X4           460800  // X4 clock RATE
    #define CLOCK_X8           921600  // (X8 clock RATE
#endif

// low word, high byte in HWOR defines RS-485 transmit and receive control 
// options available
#define HWOR_DUPLEX_FULL   0x00000100  // (x x x x x x x 1) transmitters always on available
#define HWOR_DUPLEX_DTR    0x00000200  // (x x x x x x 1 x) transmitters controlled by DTR available
#define HWOR_DUPLEX_RTS    0x00000400  // (x x x x x 1 x x) transmitters controlled by RTS available
#define HWOR_DUPLEX_AUTO   0x00000800  // (x x x x 1 x x x) Auto Toggle mode available
#define HWOR_RECEIVE_NXMT  0x00008000  // (1 x x x x x x x) receivers enabled when not transmit enabled feature available
#define HWOR_XMTRVCTRL_CLR 0xfffff00ff  //(0 0 0 0 0 0 0 0) used to clear all transmit and receive control options available 

// high word, low byte in HWOR defines RS-485 signal line configuration 
// options available
#define HWOR_SIGNAL_MODEM  0x00010000  // (x x x x x x x 1) RTS to AUXOUT - CTS from AUXIN - RCLK loopback to XCLK
#define HWOR_SIGNAL_CLOCK  0x00020000  // (x x x x x x 1 x) XCLK to AUXOUT - RCLK from AUXIN - RTS loopback to CTS
#define HWOR_SIGNAL_LOOP   0x00040000  // (x x x x x 1 x x) RTS loopback to CTS - RCLK loopback to XCLK - AUXIN loopback to AUXOUT
#define HWOR_SIGNAL_CLR    0xfff0ffff  // (0 0 0 0 0 0 0 0) used to clear all transmit and receive control options available 

#define HWOR_RS485_MASK    0x000FFF00  // Bit mask used to determine whether to display RS-422/485 property page

#define HWOR_PORT_MASK    0x00100000  // Bit mask used to determine whether to Port property page

^ permalink raw reply

* Re: [PATCH v2 1/1] staging: fwserial: Add TTY-over-Firewire serial driver
From: Stefan Richter @ 2012-11-14  1:25 UTC (permalink / raw)
  To: Peter Hurley
  Cc: Greg Kroah-Hartman, Alan Cox, linux-kernel, devel,
	linux1394-devel, linux-serial
In-Reply-To: <1352834072.16401.100.camel@thor>

On Nov 13 Peter Hurley wrote:
> On Tue, 2012-11-13 at 00:33 +0100, Stefan Richter wrote:
> > On Nov 02 Peter Hurley wrote:
> > > +2. MAX_ASYNC_PAYLOAD needs to be publicly exposed by core/ohci
> > > +   - otherwise how will this driver know the max size of address window to
> > > +     open for one packet write?
> > 
> > Hmm, don't firewire-sbp2 and firewire-net deal with this very problem
> > already on their own?  Firewire-sbp2 tells the target what maximum payload
> > the local node is ready to accept, and firewire-net figures out whether it
> > needs to fragment the datagrams in unicast TX depending on the remote
> > node's capabilities.
> 
> I wasn't as clear as I should have been here. This is only about how big
> to make the address handler window.

Isn't fw_card.max_receive what you were looking for?  But since the
current firewire-core API requires you to allocate one handler address and
size for all cards --- even for cards which will be hot-added later while
your driver is already running ---, you should surely just allocate
4096 bytes; see below.

> Just like firewire-net, this driver
> communicates with remotes at S100 speeds (and packet sizes) to
> query the largest packet size that the remote supports.

(RFC 2734 additionally keeps things sane for itself by requiring all 2734
compliant nodes to support at least max packet = 512 bytes.)

Could it be easier to take this information from fw_device.max_speed and
the max_rec in fw_device.config_rom?  RFC 2734 was designed to work
without a local node management layer which is capable to gather such
information, but we have this information on Linux (at the time when
core_device.c finished its node discovery).

> Currently, firewire-net sets an arbitrary address handler length of
> 4096. This works because the largest AR packet size the current
> firewire-ohci driver handles is 4096 (value of MAX_ASYNC_PAYLOAD) +
> header/trailer. Note that firewire-ohci does not limit card->max_receive
> to this value.
> 
> So if the ohci driver changes to handle 8K+ AR packets and the hardware
> supports it, these address handler windows will be too small.

While the IEEE 1394:2008 link layer specification (section 6) provides for
asynchronous packet payloads of up to 16384 bytes (table 6-4), the IEEE
1394 beta mode port specification (section 13) only allows up to 4096
bytes (table 16-18).  And alpha mode is of course limited to 2048 bytes.

So, asynchronous packet payloads greater than 4096 bytes are out of scope
of the current revision of IEEE 1394.

> > > +3. Maybe device_max_receive() and link_speed_to_max_payload() should be
> > > +     taken up by the firewire core?
> > 
> > Sounds like an extension of item 2, and is easier resolved after the
> > driver moves out of staging.
> 
> Ok.
> 
> > > +4. To avoid dropping rx data while still limiting the maximum buffering,
> > > +     the size of the AR context must be known. How to expose this to drivers?
> > 
> > I don't see a requirement to know the local or remote node's size of AR
> > DMA buffer.  Rather, keep the traffic throttled such that too frequent
> > ack-busy are avoided.
[...]
> I do need to implement retries but that will be of limited benefit to
> this particular problem.
> 
> Data is written from the remote as a posted write into the AR buffer.
[...]

Oh, I forgot about posted writes.  However, you only lose data with posted
writes if the OHCI somehow fails to access the host bus.  Merely getting
to the end of the DMA program before all of the PCI posting FIFO could be
emptied is not a reason to drop data.  See OHCI-1394 section 3.3 which
speaks about the possibility to drop selfID/ IR/ AR-resp data if out of
buffer, but not AR-req data.

Well, maybe there are bad OHCI implementations which drop posted AR-req
data...?  I don't know.

firewire-net uses posted writes too.  But with IP-over-1394, delivery
guarantee is handled further above if needed, not by the 1394
encapsulation.  firewire-sbp2 also uses posted writes, in two ways:  With
physical DMA to SCSI READ command data buffers, and for the SBP status
FIFO.  But physical DMA does not have a backing DMA program, and data loss
during a status FIFO write would likely be covered by a SCSI transaction
retry.

[...]
> Besides the inefficiency of re-sending data that has already been
> received, retrying may not be possible. Consider when a console driver
> (work-in-progress) writes to the remote using the same interface. It
> could be one of the last things the remote does. And the most crucial
> information is that last write.

As mentioned, I have no idea how TTY works or is supposed to work.  But
even if the sending application is allowed to quit before its last
outbound datagram was delivered, fwserial could still be made to deliver
this last datagram at its own pace, with as many software retries as
prudent.
-- 
Stefan Richter
-=====-===-- =-== -===-
http://arcgraph.de/sr/

^ permalink raw reply

* Re: [PATCH v2 1/1] staging: fwserial: Add TTY-over-Firewire serial driver
From: Greg Kroah-Hartman @ 2012-11-13 19:47 UTC (permalink / raw)
  To: Peter Hurley
  Cc: Stefan Richter, devel, linux1394-devel, linux-kernel, Alan Cox,
	linux-serial
In-Reply-To: <1352835444.16401.107.camel@thor>

On Tue, Nov 13, 2012 at 02:37:24PM -0500, Peter Hurley wrote:
> On Mon, 2012-11-12 at 15:51 -0800, Greg Kroah-Hartman wrote:
> > On Tue, Nov 13, 2012 at 12:33:38AM +0100, Stefan Richter wrote:
> > > On Nov 02 Peter Hurley wrote:
> > > > This patch provides the kernel driver for high-speed TTY
> > > > communication over the IEEE 1394 bus.
> > > > 
> > > > Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> > > 
> > > Hi Greg,
> > > 
> > > you asked for an Ack.  Here it is:  I am OK with this getting added to the
> > > staging tree.
> > 
> > Thanks, I'll queue it up later this week when I return from my travels.
> 
> Thanks, Greg.
> 
> Except for your and Alan's earlier concerns, which I think I addressed,
> I didn't get any other feedback on the TTY side of things. Are you
> personally reviewing this submission?

I will poke at it, odds are it will need some changes as there has been
some tty changes in the tty.git tree that might need to also be done
here.  Give me a bit of time to catch up...

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH v2 1/1] staging: fwserial: Add TTY-over-Firewire serial driver
From: Peter Hurley @ 2012-11-13 19:37 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Stefan Richter, devel, linux1394-devel, linux-kernel, Alan Cox,
	linux-serial
In-Reply-To: <20121112235154.GA14017@kroah.com>

On Mon, 2012-11-12 at 15:51 -0800, Greg Kroah-Hartman wrote:
> On Tue, Nov 13, 2012 at 12:33:38AM +0100, Stefan Richter wrote:
> > On Nov 02 Peter Hurley wrote:
> > > This patch provides the kernel driver for high-speed TTY
> > > communication over the IEEE 1394 bus.
> > > 
> > > Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> > 
> > Hi Greg,
> > 
> > you asked for an Ack.  Here it is:  I am OK with this getting added to the
> > staging tree.
> 
> Thanks, I'll queue it up later this week when I return from my travels.

Thanks, Greg.

Except for your and Alan's earlier concerns, which I think I addressed,
I didn't get any other feedback on the TTY side of things. Are you
personally reviewing this submission?

Regards,
Peter Hurley



^ permalink raw reply

* Re: [PATCH] serial/8250: Add support for Exar's XR17V35x family of multi-port PCIe UARTs.
From: Matt Schulte @ 2012-11-13 19:26 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-serial, Greg KH, Theodore Ts'o
In-Reply-To: <20121110232444.7914f76f@pyramind.ukuu.org.uk>

On Sat, Nov 10, 2012 at 5:24 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> On Sat, 10 Nov 2012 11:52:32 -0600
> Matt Schulte <matts@commtech-fastcom.com> wrote:
>
>> On Fri, Nov 9, 2012 at 5:58 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> >> @@ -1484,6 +1526,7 @@ EXPORT_SYMBOL_GPL(serial8250_modem_status);
>> >>  int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
>> >>  {
>> >>       unsigned char status;
>> >> +     unsigned char int0, int1, int2, int3;
>> >>       unsigned long flags;
>> >>       struct uart_8250_port *up =
>> >>               container_of(port, struct uart_8250_port, port);
>> >> @@ -1503,6 +1546,19 @@ int serial8250_handle_irq(struct uart_port
>> >> *port, unsigned int iir)
>> >>       if (status & UART_LSR_THRE)
>> >>               serial8250_tx_chars(up);
>> >>
>> >> +     /*
>> >> +      * These Exar UARTs have an extra interrupt indicator that could
>> >> +      * fire for a few unimplemented interrupts.  Put this here just
>> >> +      * to be on the safe side that these interrupts don't go unhandled.
>> >> +      */
>> >> +
>> >> +     if (up->port.type == PORT_XR17V35X && status == 0) {
>> >> +             int0 = serial_port_in(port, 0x80);
>> >> +             int1 = serial_port_in(port, 0x81);
>> >> +             int2 = serial_port_in(port, 0x82);
>> >> +             int3 = serial_port_in(port, 0x83);
>> >> +     }
>> >> +
>> >>       spin_unlock_irqrestore(&port->lock, flags);
>> >>       return 1;
>> >
>> > Provide your own handle_irq method rather than adding more special cases
>> > to the default one (you can just wrap the default one and then perform
>> > the extra actions). Just a case of trying to keep the fast paths clean and
>> > maintainable.
>> >
>> > Otherwise looks good.
>> >
>> > Alan
>>
>> Thanks Alan, would the new Exar_handle_irq method be allowed to be
>> inside 8250.c?
>
> If its probed generically then it probably should - we can move it later
> anyway
>

I'm having trouble figuring out where would be the right place to override the

     uart_port.handle_irq = serial8250_default_handle_irq

with my

     uart_port.handle_irq = exar_handle_irq

Can you give me a tip as to where in 8250.c this should be done?

Thank you,
Matt Schulte

^ permalink raw reply

* Re: [PATCH v2 1/1] staging: fwserial: Add TTY-over-Firewire serial driver
From: Peter Hurley @ 2012-11-13 19:14 UTC (permalink / raw)
  To: Stefan Richter
  Cc: Greg Kroah-Hartman, Alan Cox, linux-kernel, devel,
	linux1394-devel, linux-serial
In-Reply-To: <20121113003338.6aafd7c8@stein>

On Tue, 2012-11-13 at 00:33 +0100, Stefan Richter wrote:
> On Nov 02 Peter Hurley wrote:
> > This patch provides the kernel driver for high-speed TTY
> > communication over the IEEE 1394 bus.
> > 
> > Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> 
> Hi Greg,
> 
> you asked for an Ack.  Here it is:  I am OK with this getting added to the
> staging tree.
> 
> (Sorry for the delay, I was ill and at the same time buried in work at the
> day job.)  Note, I only quickly scrolled through v1 and v2.  I will do an
> actual review when somebody asks to move this from staging into a proper
> mainline place.

Hi Stefan,

Thanks very much for looking over this submission. (I hope you're
feeling better.)

> Peter,
> below are some high-level comments to your TODOs.

> > --- /dev/null
> > +++ b/drivers/staging/fwserial/Kconfig
> > @@ -0,0 +1,9 @@
> > +config FIREWIRE_SERIAL
> > +       tristate "TTY over Firewire"
> > +       depends on FIREWIRE
> > +       help
> > +          This enables TTY over IEEE 1394, providing high-speed serial
> > +	  connectivity to cabled peers.
> > +
> > +	  To compile this driver as a module, say M here:  the module will
> > +	  be called firewire-serial.
> 
> This should mention that this is a Linux-only affair at this time:  It uses
> an ad-hoc defined protocol which no other platform is known to support for
> the time being.

Good point. I'll make this change.

> > --- /dev/null
> > +++ b/drivers/staging/fwserial/TODO
> > @@ -0,0 +1,37 @@
> > +TODOs
> > +-----
> > +1. Implement retries for RCODE_BUSY, RCODE_NO_ACK and RCODE_SEND_ERROR
> > +   - I/O is handled asynchronously which presents some issues when error
> > +     conditions occur.
> > +2. Implement _robust_ console on top of this. The existing prototype console
> > +   driver is not ready for the big leagues yet.
> > +3. Expose means of controlling attach/detach of peers via sysfs. Include
> > +   GUID-to-port matching/whitelist/blacklist.
> > +
> > +-- Issues with firewire stack --
> > +1. This driver uses the same unregistered vendor id that the firewire core does
> > +     (0xd00d1e). Perhaps this could be exposed as a define in
> > +     firewire-constants.h?
> 
> I guess there is no better OUI for this purpose.
> 
> The constant could be added to linux/firewire.h, but IMO rather not to
> uapi/linux/firewire-constants.h.  We don't want this definition to leak
> out to userland; userland can hardwire its own hacks itself. :-)
> 
> This trivial move to a header should be deferred until the driver moves
> out of staging.

You're right. I'll update the TODO.

And I should have noted in the TODO file itself that the issues noted
are for moving the driver out of staging.

> > +2. MAX_ASYNC_PAYLOAD needs to be publicly exposed by core/ohci
> > +   - otherwise how will this driver know the max size of address window to
> > +     open for one packet write?
> 
> Hmm, don't firewire-sbp2 and firewire-net deal with this very problem
> already on their own?  Firewire-sbp2 tells the target what maximum payload
> the local node is ready to accept, and firewire-net figures out whether it
> needs to fragment the datagrams in unicast TX depending on the remote
> node's capabilities.

I wasn't as clear as I should have been here. This is only about how big
to make the address handler window. Just like firewire-net, this driver
communicates with remotes at S100 speeds (and packet sizes) to
query the largest packet size that the remote supports.

Currently, firewire-net sets an arbitrary address handler length of
4096. This works because the largest AR packet size the current
firewire-ohci driver handles is 4096 (value of MAX_ASYNC_PAYLOAD) +
header/trailer. Note that firewire-ohci does not limit card->max_receive
to this value.

So if the ohci driver changes to handle 8K+ AR packets and the hardware
supports it, these address handler windows will be too small.

> > +3. Maybe device_max_receive() and link_speed_to_max_payload() should be
> > +     taken up by the firewire core?
> 
> Sounds like an extension of item 2, and is easier resolved after the
> driver moves out of staging.

Ok.

> > +4. To avoid dropping rx data while still limiting the maximum buffering,
> > +     the size of the AR context must be known. How to expose this to drivers?
> 
> I don't see a requirement to know the local or remote node's size of AR
> DMA buffer.  Rather, keep the traffic throttled such that too frequent
> ack-busy are avoided.  As far as I have learned from firewire-net, just
> limiting the number or in-flight transactions should work reasonably.  Of
> course, hardware ack-busy retries which the OHCI performs on its own won't
> be noticed by software but take away from overall bus bandwidth.  IOW the
> optimum queue depth is by a certain margin less than the depth at which
> software starts to see ack-busy transaction terminations.
> 
> Furthermore:  In order to avoid dropping data, you don't need to know the
> buffer size at all.  You need to implement proper software retries.  I.e.
> either those which you noted yourself at the top of this TODO file, or you
> let the userland on top of the TTY handle it if this is possible.  (I
> don't know how the TTY subsystem and TTY based applications work.)

I do need to implement retries but that will be of limited benefit to
this particular problem.

Data is written from the remote as a posted write into the AR buffer.
The remote receives hardware ack and continues writing more. Meanwhile,
the local node schedules the AR tasklet. The AR tasklet runs,
dispatching the packets to each appropriate handler. So, by the time
this driver's handler is delivered the first packet, many packets could
already have been acked and sitting in the AR buffer. At this point,
throttling the remote will only prevent the remote from sending even
more, and can't help with the data which has already been received.

The driver already limits the max in-flight transactions to 8. But since
they're unified transactions, that doesn't do much. Under heavy loads,
there may be many (small) completed packets in the AR buffer (NB: the
driver already does write aggregation but this isn't appropriate in the
console driver).

Besides the inefficiency of re-sending data that has already been
received, retrying may not be possible. Consider when a console driver
(work-in-progress) writes to the remote using the same interface. It
could be one of the last things the remote does. And the most crucial
information is that last write.

Despite my lengthy description, I agree that knowing the size of the AR
context is not a requirement, and we can drop this as an issue. As you
suggest, I can determine the appropriate buffer size experimentally (and
implement retries).

> > +5. Explore if bigger AR context will reduce RCODE_BUSY responses
> > +   (or auto-grow to certain max size -- but this would require major surgery
> > +    as the current AR is contiguously mapped)
> 
> For the particular application "firewire-net", the present AR-Req DMA
> buffer size has apparently been determined as suitable.  Of course if you
> find a different optimum, that could certainly be changed.
> 
> I haven't looked closely, but I suppose your bandwidth requirement
> concerns AR-Request DMA, not so much AR-Response DMA, right?

Exactly. And I was just thinking to myself there which unfortunately
slipped into a public document :)

Thanks again for your time, Stefan.

Regards,
Peter Hurley



^ permalink raw reply

* Re: [PATCH v2 2/3] serial: mxs-auart: add the DMA support for mx28
From: Lauri Hintsala @ 2012-11-13  9:42 UTC (permalink / raw)
  To: Huang Shijie
  Cc: gregkh, alan, linux-serial, linux-arm-kernel, shawn.guo, linux,
	vinod.koul, Veli-Pekka Peltola
In-Reply-To: <1351074456-25863-3-git-send-email-b32955@freescale.com>

Hi Huang,

DMA support doesn't work with latest stable v3.6.5 or development 
3.7-rc5 kernels. I get following error message when I open the serial 
port /dev/ttyAPP0:

[   48.730000] mxs-auart 8006a000.serial: step 1 error
[   48.750000] mxs-auart 8006a000.serial: We can not start up the DMA.


On 10/24/2012 01:27 PM, Huang Shijie wrote:
> Only we meet the following conditions, we can enable the DMA support for
> auart:
>
>    (1) We enable the DMA support in the dts file, such as
>        arch/arm/boot/dts/imx28.dtsi.
>
>    (2) We enable the hardware flow control.

Why HW flow control is required?

We need high speed auart without flow control. I have tested kernel from 
Freescale's BSP and the performance was good without HW flow control.

Best Regards,
Lauri Hintsala


>    (3) We use the mx28, not the mx23. Due to hardware bug(see errata: 2836),
>        we can not add the DMA support to mx23.
>
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>   .../bindings/tty/serial/fsl-mxs-auart.txt          |    8 +
>   drivers/tty/serial/mxs-auart.c                     |  318 +++++++++++++++++++-
>   2 files changed, 321 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt
> index 2ee903f..273a8d5 100644
> --- a/Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt
> +++ b/Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt
> @@ -6,11 +6,19 @@ Required properties:
>   - reg : Address and length of the register set for the device
>   - interrupts : Should contain the auart interrupt numbers
>
> +Optional properties:
> +- fsl,auart-dma-channel : The DMA channels, the first is for RX, the other
> +		is for TX. If you add this property, it also means that you
> +		will enable the DMA support for the auart.
> +		Note: due to the hardware bug in imx23(see errata : 2836),
> +		only the imx28 can enable the DMA support for the auart.
> +
>   Example:
>   auart0: serial@8006a000 {
>   	compatible = "fsl,imx28-auart", "fsl,imx23-auart";
>   	reg = <0x8006a000 0x2000>;
>   	interrupts = <112 70 71>;
> +	fsl,auart-dma-channel = <8 9>;
>   };
>
>   Note: Each auart port should have an alias correctly numbered in "aliases"
> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index 06d7271..d593e0a 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c
> @@ -34,6 +34,8 @@
>   #include <linux/io.h>
>   #include <linux/pinctrl/consumer.h>
>   #include <linux/of_device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/fsl/mxs-dma.h>
>
>   #include <asm/cacheflush.h>
>
> @@ -71,6 +73,15 @@
>
>   #define AUART_CTRL0_SFTRST			(1 << 31)
>   #define AUART_CTRL0_CLKGATE			(1 << 30)
> +#define AUART_CTRL0_RXTO_ENABLE			(1 << 27)
> +#define AUART_CTRL0_RXTIMEOUT(v)		(((v) & 0x7ff) << 16)
> +#define AUART_CTRL0_XFER_COUNT(v)		((v) & 0xffff)
> +
> +#define AUART_CTRL1_XFER_COUNT(v)		((v) & 0xffff)
> +
> +#define AUART_CTRL2_DMAONERR			(1 << 26)
> +#define AUART_CTRL2_TXDMAE			(1 << 25)
> +#define AUART_CTRL2_RXDMAE			(1 << 24)
>
>   #define AUART_CTRL2_CTSEN			(1 << 15)
>   #define AUART_CTRL2_RTSEN			(1 << 14)
> @@ -111,6 +122,7 @@
>   #define AUART_STAT_BERR				(1 << 18)
>   #define AUART_STAT_PERR				(1 << 17)
>   #define AUART_STAT_FERR				(1 << 16)
> +#define AUART_STAT_RXCOUNT_MASK			0xffff
>
>   static struct uart_driver auart_driver;
>
> @@ -122,7 +134,10 @@ enum mxs_auart_type {
>   struct mxs_auart_port {
>   	struct uart_port port;
>
> -	unsigned int flags;
> +#define MXS_AUART_DMA_CONFIG	0x1
> +#define MXS_AUART_DMA_ENABLED	0x2
> +#define MXS_AUART_DMA_TX_SYNC	2  /* bit 2 */
> +	unsigned long flags;
>   	unsigned int ctrl;
>   	enum mxs_auart_type devtype;
>
> @@ -130,6 +145,20 @@ struct mxs_auart_port {
>
>   	struct clk *clk;
>   	struct device *dev;
> +
> +	/* for DMA */
> +	struct mxs_dma_data dma_data;
> +	int dma_channel_rx, dma_channel_tx;
> +	int dma_irq_rx, dma_irq_tx;
> +	int dma_channel;
> +
> +	struct scatterlist tx_sgl;
> +	struct dma_chan	*tx_dma_chan;
> +	void *tx_dma_buf;
> +
> +	struct scatterlist rx_sgl;
> +	struct dma_chan	*rx_dma_chan;
> +	void *rx_dma_buf;
>   };
>
>   static struct platform_device_id mxs_auart_devtype[] = {
> @@ -155,14 +184,107 @@ static inline int is_imx28_auart(struct mxs_auart_port *s)
>   	return s->devtype == IMX28_AUART;
>   }
>
> +static inline bool auart_dma_enabled(struct mxs_auart_port *s)
> +{
> +	return s->flags & MXS_AUART_DMA_ENABLED;
> +}
> +
>   static void mxs_auart_stop_tx(struct uart_port *u);
>
>   #define to_auart_port(u) container_of(u, struct mxs_auart_port, port)
>
> -static inline void mxs_auart_tx_chars(struct mxs_auart_port *s)
> +static void mxs_auart_tx_chars(struct mxs_auart_port *s);
> +
> +static void dma_tx_callback(void *param)
>   {
> +	struct mxs_auart_port *s = param;
>   	struct circ_buf *xmit = &s->port.state->xmit;
>
> +	dma_unmap_sg(s->dev, &s->tx_sgl, 1, DMA_TO_DEVICE);
> +
> +	/* clear the bit used to serialize the DMA tx. */
> +	clear_bit(MXS_AUART_DMA_TX_SYNC, &s->flags);
> +	smp_mb__after_clear_bit();
> +
> +	/* wake up the possible processes. */
> +	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
> +		uart_write_wakeup(&s->port);
> +
> +	mxs_auart_tx_chars(s);
> +}
> +
> +static int mxs_auart_dma_tx(struct mxs_auart_port *s, int size)
> +{
> +	struct dma_async_tx_descriptor *desc;
> +	struct scatterlist *sgl = &s->tx_sgl;
> +	struct dma_chan *channel = s->tx_dma_chan;
> +	u32 pio;
> +
> +	/* [1] : send PIO. Note, the first pio word is CTRL1. */
> +	pio = AUART_CTRL1_XFER_COUNT(size);
> +	desc = dmaengine_prep_slave_sg(channel, (struct scatterlist *)&pio,
> +					1, DMA_TRANS_NONE, 0);
> +	if (!desc) {
> +		dev_err(s->dev, "step 1 error\n");
> +		return -EINVAL;
> +	}
> +
> +	/* [2] : set DMA buffer. */
> +	sg_init_one(sgl, s->tx_dma_buf, size);
> +	dma_map_sg(s->dev, sgl, 1, DMA_TO_DEVICE);
> +	desc = dmaengine_prep_slave_sg(channel, sgl,
> +			1, DMA_MEM_TO_DEV, DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> +	if (!desc) {
> +		dev_err(s->dev, "step 2 error\n");
> +		return -EINVAL;
> +	}
> +
> +	/* [3] : submit the DMA */
> +	desc->callback = dma_tx_callback;
> +	desc->callback_param = s;
> +	dmaengine_submit(desc);
> +	dma_async_issue_pending(channel);
> +	return 0;
> +}
> +
> +static void mxs_auart_tx_chars(struct mxs_auart_port *s)
> +{
> +	struct circ_buf *xmit = &s->port.state->xmit;
> +
> +	if (auart_dma_enabled(s)) {
> +		int i = 0;
> +		int size;
> +		void *buffer = s->tx_dma_buf;
> +
> +		if (test_and_set_bit(MXS_AUART_DMA_TX_SYNC, &s->flags))
> +			return;
> +
> +		while (!uart_circ_empty(xmit) && !uart_tx_stopped(&s->port)) {
> +			size = min_t(u32, UART_XMIT_SIZE - i,
> +				     CIRC_CNT_TO_END(xmit->head,
> +						     xmit->tail,
> +						     UART_XMIT_SIZE));
> +			memcpy(buffer + i, xmit->buf + xmit->tail, size);
> +			xmit->tail = (xmit->tail + size) & (UART_XMIT_SIZE - 1);
> +
> +			i += size;
> +			if (i >= UART_XMIT_SIZE)
> +				break;
> +		}
> +
> +		if (uart_tx_stopped(&s->port))
> +			mxs_auart_stop_tx(&s->port);
> +
> +		if (i) {
> +			mxs_auart_dma_tx(s, i);
> +		} else {
> +			clear_bit(MXS_AUART_DMA_TX_SYNC, &s->flags);
> +			smp_mb__after_clear_bit();
> +		}
> +		return;
> +	}
> +
> +
>   	while (!(readl(s->port.membase + AUART_STAT) &
>   		 AUART_STAT_TXFF)) {
>   		if (s->port.x_char) {
> @@ -316,10 +438,155 @@ static u32 mxs_auart_get_mctrl(struct uart_port *u)
>   	return mctrl;
>   }
>
> +static bool mxs_auart_dma_filter(struct dma_chan *chan, void *param)
> +{
> +	struct mxs_auart_port *s = param;
> +
> +	if (!mxs_dma_is_apbx(chan))
> +		return false;
> +
> +	if (s->dma_channel == chan->chan_id) {
> +		chan->private = &s->dma_data;
> +		return true;
> +	}
> +	return false;
> +}
> +
> +static int mxs_auart_dma_prep_rx(struct mxs_auart_port *s);
> +static void dma_rx_callback(void *arg)
> +{
> +	struct mxs_auart_port *s = (struct mxs_auart_port *) arg;
> +	struct tty_struct *tty = s->port.state->port.tty;
> +	int count;
> +	u32 stat;
> +
> +	stat = readl(s->port.membase + AUART_STAT);
> +	stat &= ~(AUART_STAT_OERR | AUART_STAT_BERR |
> +			AUART_STAT_PERR | AUART_STAT_FERR);
> +
> +	count = stat & AUART_STAT_RXCOUNT_MASK;
> +	tty_insert_flip_string(tty, s->rx_dma_buf, count);
> +
> +	writel(stat, s->port.membase + AUART_STAT);
> +	tty_flip_buffer_push(tty);
> +
> +	/* start the next DMA for RX. */
> +	mxs_auart_dma_prep_rx(s);
> +}
> +
> +static int mxs_auart_dma_prep_rx(struct mxs_auart_port *s)
> +{
> +	struct dma_async_tx_descriptor *desc;
> +	struct scatterlist *sgl = &s->rx_sgl;
> +	struct dma_chan *channel = s->rx_dma_chan;
> +	u32 pio[1];
> +
> +	/* [1] : send PIO */
> +	pio[0] = AUART_CTRL0_RXTO_ENABLE
> +		| AUART_CTRL0_RXTIMEOUT(0x80)
> +		| AUART_CTRL0_XFER_COUNT(UART_XMIT_SIZE);
> +	desc = dmaengine_prep_slave_sg(channel, (struct scatterlist *)pio,
> +					1, DMA_TRANS_NONE, 0);
> +	if (!desc) {
> +		dev_err(s->dev, "step 1 error\n");
> +		return -EINVAL;
> +	}
> +
> +	/* [2] : send DMA request */
> +	sg_init_one(sgl, s->rx_dma_buf, UART_XMIT_SIZE);
> +	dma_map_sg(s->dev, sgl, 1, DMA_FROM_DEVICE);
> +	desc = dmaengine_prep_slave_sg(channel, sgl, 1, DMA_DEV_TO_MEM,
> +					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> +	if (!desc) {
> +		dev_err(s->dev, "step 2 error\n");
> +		return -1;
> +	}
> +
> +	/* [3] : submit the DMA, but do not issue it. */
> +	desc->callback = dma_rx_callback;
> +	desc->callback_param = s;
> +	dmaengine_submit(desc);
> +	dma_async_issue_pending(channel);
> +	return 0;
> +}
> +
> +static void mxs_auart_dma_exit_channel(struct mxs_auart_port *s)
> +{
> +	if (s->tx_dma_chan) {
> +		dma_release_channel(s->tx_dma_chan);
> +		s->tx_dma_chan = NULL;
> +	}
> +	if (s->rx_dma_chan) {
> +		dma_release_channel(s->rx_dma_chan);
> +		s->rx_dma_chan = NULL;
> +	}
> +
> +	kfree(s->tx_dma_buf);
> +	kfree(s->rx_dma_buf);
> +	s->tx_dma_buf = NULL;
> +	s->rx_dma_buf = NULL;
> +}
> +
> +static void mxs_auart_dma_exit(struct mxs_auart_port *s)
> +{
> +
> +	writel(AUART_CTRL2_TXDMAE | AUART_CTRL2_RXDMAE | AUART_CTRL2_DMAONERR,
> +		s->port.membase + AUART_CTRL2_CLR);
> +
> +	mxs_auart_dma_exit_channel(s);
> +	s->flags &= ~MXS_AUART_DMA_ENABLED;
> +}
> +
> +static int mxs_auart_dma_init(struct mxs_auart_port *s)
> +{
> +	dma_cap_mask_t mask;
> +
> +	if (auart_dma_enabled(s))
> +		return 0;
> +
> +	/* We do not get the right DMA channels. */
> +	if (s->dma_channel_rx == -1 || s->dma_channel_rx == -1)
> +		return -EINVAL;
> +
> +	/* init for RX */
> +	dma_cap_zero(mask);
> +	dma_cap_set(DMA_SLAVE, mask);
> +	s->dma_channel = s->dma_channel_rx;
> +	s->dma_data.chan_irq = s->dma_irq_rx;
> +	s->rx_dma_chan = dma_request_channel(mask, mxs_auart_dma_filter, s);
> +	if (!s->rx_dma_chan)
> +		goto err_out;
> +	s->rx_dma_buf = kzalloc(UART_XMIT_SIZE, GFP_KERNEL | GFP_DMA);
> +	if (!s->rx_dma_buf)
> +		goto err_out;
> +
> +	/* init for TX */
> +	s->dma_channel = s->dma_channel_tx;
> +	s->dma_data.chan_irq = s->dma_irq_tx;
> +	s->tx_dma_chan = dma_request_channel(mask, mxs_auart_dma_filter, s);
> +	if (!s->tx_dma_chan)
> +		goto err_out;
> +	s->tx_dma_buf = kzalloc(UART_XMIT_SIZE, GFP_KERNEL | GFP_DMA);
> +	if (!s->tx_dma_buf)
> +		goto err_out;
> +
> +	/* set the flags */
> +	s->flags |= MXS_AUART_DMA_ENABLED;
> +	dev_dbg(s->dev, "enabled the DMA support.");
> +
> +	return 0;
> +
> +err_out:
> +	mxs_auart_dma_exit_channel(s);
> +	return -EINVAL;
> +
> +}
> +
>   static void mxs_auart_settermios(struct uart_port *u,
>   				 struct ktermios *termios,
>   				 struct ktermios *old)
>   {
> +	struct mxs_auart_port *s = to_auart_port(u);
>   	u32 bm, ctrl, ctrl2, div;
>   	unsigned int cflag, baud;
>
> @@ -391,10 +658,23 @@ static void mxs_auart_settermios(struct uart_port *u,
>   		ctrl |= AUART_LINECTRL_STP2;
>
>   	/* figure out the hardware flow control settings */
> -	if (cflag & CRTSCTS)
> +	if (cflag & CRTSCTS) {
> +		/*
> +		 * The DMA has a bug(see errata:2836) in mx23.
> +		 * So we can not implement the DMA for auart in mx23,
> +		 * we can only implement the DMA support for auart
> +		 * in mx28.
> +		 */
> +		if (is_imx28_auart(s) && (s->flags & MXS_AUART_DMA_CONFIG)) {
> +			if (!mxs_auart_dma_init(s))
> +				/* enable DMA tranfer */
> +				ctrl2 |= AUART_CTRL2_TXDMAE | AUART_CTRL2_RXDMAE
> +				       | AUART_CTRL2_DMAONERR;
> +		}
>   		ctrl2 |= AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN;
> -	else
> +	} else {
>   		ctrl2 &= ~(AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN);
> +	}
>
>   	/* set baud rate */
>   	baud = uart_get_baud_rate(u, termios, old, 0, u->uartclk);
> @@ -406,6 +686,17 @@ static void mxs_auart_settermios(struct uart_port *u,
>   	writel(ctrl2, u->membase + AUART_CTRL2);
>
>   	uart_update_timeout(u, termios->c_cflag, baud);
> +
> +	/* prepare for the DMA RX. */
> +	if (auart_dma_enabled(s)) {
> +		if (!mxs_auart_dma_prep_rx(s)) {
> +			/* Disable the normal RX interrupt. */
> +			writel(AUART_INTR_RXIEN, u->membase + AUART_INTR_CLR);
> +		} else {
> +			mxs_auart_dma_exit(s);
> +			dev_err(s->dev, "We can not start up the DMA.\n");
> +		}
> +	}
>   }
>
>   static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
> @@ -484,6 +775,9 @@ static void mxs_auart_shutdown(struct uart_port *u)
>   {
>   	struct mxs_auart_port *s = to_auart_port(u);
>
> +	if (auart_dma_enabled(s))
> +		mxs_auart_dma_exit(s);
> +
>   	writel(AUART_CTRL2_UARTEN, u->membase + AUART_CTRL2_CLR);
>
>   	writel(AUART_INTR_RXIEN | AUART_INTR_RTIEN | AUART_INTR_CTSMIEN,
> @@ -717,6 +1011,7 @@ static int serial_mxs_probe_dt(struct mxs_auart_port *s,
>   		struct platform_device *pdev)
>   {
>   	struct device_node *np = pdev->dev.of_node;
> +	u32 dma_channel[2];
>   	int ret;
>
>   	if (!np)
> @@ -730,6 +1025,20 @@ static int serial_mxs_probe_dt(struct mxs_auart_port *s,
>   	}
>   	s->port.line = ret;
>
> +	s->dma_irq_rx = platform_get_irq(pdev, 1);
> +	s->dma_irq_tx = platform_get_irq(pdev, 2);
> +
> +	ret = of_property_read_u32_array(np, "fsl,auart-dma-channel",
> +					dma_channel, 2);
> +	if (ret == 0) {
> +		s->dma_channel_rx = dma_channel[0];
> +		s->dma_channel_tx = dma_channel[1];
> +
> +		s->flags |= MXS_AUART_DMA_CONFIG;
> +	} else {
> +		s->dma_channel_rx = -1;
> +		s->dma_channel_tx = -1;
> +	}
>   	return 0;
>   }
>
> @@ -787,7 +1096,6 @@ static int __devinit mxs_auart_probe(struct platform_device *pdev)
>   	s->port.type = PORT_IMX;
>   	s->port.dev = s->dev = get_device(&pdev->dev);
>
> -	s->flags = 0;
>   	s->ctrl = 0;
>
>   	s->irq = platform_get_irq(pdev, 0);
>

^ permalink raw reply

* Re: [PATCH v2 1/1] staging: fwserial: Add TTY-over-Firewire serial driver
From: Greg Kroah-Hartman @ 2012-11-12 23:51 UTC (permalink / raw)
  To: Stefan Richter
  Cc: Peter Hurley, devel, linux1394-devel, linux-kernel, Alan Cox,
	linux-serial
In-Reply-To: <20121113003338.6aafd7c8@stein>

On Tue, Nov 13, 2012 at 12:33:38AM +0100, Stefan Richter wrote:
> On Nov 02 Peter Hurley wrote:
> > This patch provides the kernel driver for high-speed TTY
> > communication over the IEEE 1394 bus.
> > 
> > Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> 
> Hi Greg,
> 
> you asked for an Ack.  Here it is:  I am OK with this getting added to the
> staging tree.

Thanks, I'll queue it up later this week when I return from my travels.

greg k-h

^ permalink raw reply

* Re: [PATCH v2 1/1] staging: fwserial: Add TTY-over-Firewire serial driver
From: Stefan Richter @ 2012-11-12 23:33 UTC (permalink / raw)
  To: Peter Hurley, Greg Kroah-Hartman
  Cc: Alan Cox, linux-kernel, devel, linux1394-devel, linux-serial
In-Reply-To: <55547e779e65e6865f18d537ef1a42191a4b7e46.1351817601.git.peter@hurleysoftware.com>

On Nov 02 Peter Hurley wrote:
> This patch provides the kernel driver for high-speed TTY
> communication over the IEEE 1394 bus.
> 
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>

Hi Greg,

you asked for an Ack.  Here it is:  I am OK with this getting added to the
staging tree.

(Sorry for the delay, I was ill and at the same time buried in work at the
day job.)  Note, I only quickly scrolled through v1 and v2.  I will do an
actual review when somebody asks to move this from staging into a proper
mainline place.

Peter,
below are some high-level comments to your TODOs.

> ---
> v2
[...]
> ---
>  drivers/staging/Kconfig             |    2 +
>  drivers/staging/Makefile            |    1 +
>  drivers/staging/fwserial/Kconfig    |    9 +
>  drivers/staging/fwserial/Makefile   |    2 +
>  drivers/staging/fwserial/TODO       |   37 +
>  drivers/staging/fwserial/dma_fifo.c |  310 ++++
>  drivers/staging/fwserial/dma_fifo.h |  130 ++
>  drivers/staging/fwserial/fwserial.c | 2946 +++++++++++++++++++++++++++++++++++
>  drivers/staging/fwserial/fwserial.h |  387 +++++
>  9 files changed, 3824 insertions(+)
>  create mode 100644 drivers/staging/fwserial/Kconfig
>  create mode 100644 drivers/staging/fwserial/Makefile
>  create mode 100644 drivers/staging/fwserial/TODO
>  create mode 100644 drivers/staging/fwserial/dma_fifo.c
>  create mode 100644 drivers/staging/fwserial/dma_fifo.h
>  create mode 100644 drivers/staging/fwserial/fwserial.c
>  create mode 100644 drivers/staging/fwserial/fwserial.h
[...]
> --- /dev/null
> +++ b/drivers/staging/fwserial/Kconfig
> @@ -0,0 +1,9 @@
> +config FIREWIRE_SERIAL
> +       tristate "TTY over Firewire"
> +       depends on FIREWIRE
> +       help
> +          This enables TTY over IEEE 1394, providing high-speed serial
> +	  connectivity to cabled peers.
> +
> +	  To compile this driver as a module, say M here:  the module will
> +	  be called firewire-serial.

This should mention that this is a Linux-only affair at this time:  It uses
an ad-hoc defined protocol which no other platform is known to support for
the time being.

[...]
> --- /dev/null
> +++ b/drivers/staging/fwserial/TODO
> @@ -0,0 +1,37 @@
> +TODOs
> +-----
> +1. Implement retries for RCODE_BUSY, RCODE_NO_ACK and RCODE_SEND_ERROR
> +   - I/O is handled asynchronously which presents some issues when error
> +     conditions occur.
> +2. Implement _robust_ console on top of this. The existing prototype console
> +   driver is not ready for the big leagues yet.
> +3. Expose means of controlling attach/detach of peers via sysfs. Include
> +   GUID-to-port matching/whitelist/blacklist.
> +
> +-- Issues with firewire stack --
> +1. This driver uses the same unregistered vendor id that the firewire core does
> +     (0xd00d1e). Perhaps this could be exposed as a define in
> +     firewire-constants.h?

I guess there is no better OUI for this purpose.

The constant could be added to linux/firewire.h, but IMO rather not to
uapi/linux/firewire-constants.h.  We don't want this definition to leak
out to userland; userland can hardwire its own hacks itself. :-)

This trivial move to a header should be deferred until the driver moves
out of staging.

> +2. MAX_ASYNC_PAYLOAD needs to be publicly exposed by core/ohci
> +   - otherwise how will this driver know the max size of address window to
> +     open for one packet write?

Hmm, don't firewire-sbp2 and firewire-net deal with this very problem
already on their own?  Firewire-sbp2 tells the target what maximum payload
the local node is ready to accept, and firewire-net figures out whether it
needs to fragment the datagrams in unicast TX depending on the remote
node's capabilities.

> +3. Maybe device_max_receive() and link_speed_to_max_payload() should be
> +     taken up by the firewire core?

Sounds like an extension of item 2, and is easier resolved after the
driver moves out of staging.

> +4. To avoid dropping rx data while still limiting the maximum buffering,
> +     the size of the AR context must be known. How to expose this to drivers?

I don't see a requirement to know the local or remote node's size of AR
DMA buffer.  Rather, keep the traffic throttled such that too frequent
ack-busy are avoided.  As far as I have learned from firewire-net, just
limiting the number or in-flight transactions should work reasonably.  Of
course, hardware ack-busy retries which the OHCI performs on its own won't
be noticed by software but take away from overall bus bandwidth.  IOW the
optimum queue depth is by a certain margin less than the depth at which
software starts to see ack-busy transaction terminations.

Furthermore:  In order to avoid dropping data, you don't need to know the
buffer size at all.  You need to implement proper software retries.  I.e.
either those which you noted yourself at the top of this TODO file, or you
let the userland on top of the TTY handle it if this is possible.  (I
don't know how the TTY subsystem and TTY based applications work.)

> +5. Explore if bigger AR context will reduce RCODE_BUSY responses
> +   (or auto-grow to certain max size -- but this would require major surgery
> +    as the current AR is contiguously mapped)

For the particular application "firewire-net", the present AR-Req DMA
buffer size has apparently been determined as suitable.  Of course if you
find a different optimum, that could certainly be changed.

I haven't looked closely, but I suppose your bandwidth requirement
concerns AR-Request DMA, not so much AR-Response DMA, right?
-- 
Stefan Richter
-=====-===-- =-== -==--
http://arcgraph.de/sr/

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox