* [PATCH tty-next] x86: ce4100: allow second UART usage
From: Florian Fainelli @ 2012-10-19 8:45 UTC (permalink / raw)
To: alan; +Cc: linux-serial, gregkh, tglx, linux-kernel, mbizon,
Florian Fainelli
From: Maxime Bizon <mbizon@freebox.fr>
The current CE4100 and 8250_pci code have both a limitation preventing the
registration and usage of CE4100's second UART. This patch changes the
platform code fixing up the UART port to work on a relative UART port
base address, as well as the 8250_pci code to make it register 2 UART ports
for CE4100 and pass the port index down to all consumers.
Signed-off-by: Florian Fainelli <ffainelli@freebox.fr>
---
arch/x86/platform/ce4100/ce4100.c | 3 +++
drivers/tty/serial/8250/8250_pci.c | 6 +++---
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/x86/platform/ce4100/ce4100.c b/arch/x86/platform/ce4100/ce4100.c
index 4c61b52..0dcc30e 100644
--- a/arch/x86/platform/ce4100/ce4100.c
+++ b/arch/x86/platform/ce4100/ce4100.c
@@ -92,8 +92,11 @@ static void ce4100_serial_fixup(int port, struct uart_port *up,
up->membase =
(void __iomem *)__fix_to_virt(FIX_EARLYCON_MEM_BASE);
up->membase += up->mapbase & ~PAGE_MASK;
+ up->mapbase += port * 0x100;
+ up->membase += port * 0x100;
up->iotype = UPIO_MEM32;
up->regshift = 2;
+ up->irq = 4;
}
#endif
up->iobase = 0;
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 17b7d26..cec8852 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1068,7 +1068,7 @@ ce4100_serial_setup(struct serial_private *priv,
{
int ret;
- ret = setup_port(priv, port, 0, 0, board->reg_shift);
+ ret = setup_port(priv, port, idx, 0, board->reg_shift);
port->port.iotype = UPIO_MEM32;
port->port.type = PORT_XSCALE;
port->port.flags = (port->port.flags | UPF_FIXED_PORT | UPF_FIXED_TYPE);
@@ -2658,8 +2658,8 @@ static struct pciserial_board pci_boards[] __devinitdata = {
.first_offset = 0x1000,
},
[pbn_ce4100_1_115200] = {
- .flags = FL_BASE0,
- .num_ports = 1,
+ .flags = FL_BASE_BARS,
+ .num_ports = 2,
.base_baud = 921600,
.reg_shift = 2,
},
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] serial: ifx6x60: del_timer_sync must not be called in interrupt context.
From: Alan Cox @ 2012-10-19 9:54 UTC (permalink / raw)
To: Jun Chen; +Cc: alan, linux-serial, russ.gorby, chuansheng.liu, chao.bi
In-Reply-To: <1350654690.4332.12.camel@chenjun-workstation>
On Fri, 19 Oct 2012 09:51:30 -0400
Jun Chen <jun.d.chen@intel.com> wrote:
>
> This patch make use of del_timer instead of del_timer_sync in the
> interrupt context.
> The spi_timer function don't use any resources that may release after
> running del_timer,
> so using the del_timer is also safe and enough in this context.
>
> Signed-off-by: Chen Jun <jun.d.chen@intel.com>
Acked-by: Alan Cox <alan@linux.intel.com>
^ permalink raw reply
* Re: [PATCH tty-next] x86: ce4100: allow second UART usage
From: Alan Cox @ 2012-10-19 9:56 UTC (permalink / raw)
To: Florian Fainelli; +Cc: alan, linux-serial, gregkh, tglx, linux-kernel, mbizon
In-Reply-To: <1350636307-23476-1-git-send-email-ffainelli@freebox.fr>
On Fri, 19 Oct 2012 10:45:07 +0200
Florian Fainelli <ffainelli@freebox.fr> wrote:
> From: Maxime Bizon <mbizon@freebox.fr>
>
> The current CE4100 and 8250_pci code have both a limitation preventing the
> registration and usage of CE4100's second UART. This patch changes the
> platform code fixing up the UART port to work on a relative UART port
> base address, as well as the 8250_pci code to make it register 2 UART ports
> for CE4100 and pass the port index down to all consumers.
>
> Signed-off-by: Florian Fainelli <ffainelli@freebox.fr>
Acked-by: Alan Cox <alan@linux.intel.com>
^ permalink raw reply
* [GIT PATCH] TTY patches for 3.7-rc2
From: Greg KH @ 2012-10-19 18:26 UTC (permalink / raw)
To: Linus Torvalds, Alan Cox, Jiri Slaby
Cc: Andrew Morton, linux-kernel, linux-serial
The following changes since commit ddffeb8c4d0331609ef2581d84de4d763607bd37:
Linux 3.7-rc1 (2012-10-14 14:41:04 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/ tags/tty-3.7-rc1
for you to fetch changes up to 178e485a0ebbfdb7165b4363d8fea2a07d650c0b:
staging: dgrp: check return value of alloc_tty_driver (2012-10-17 14:10:10 -0700)
----------------------------------------------------------------
TTY fixes for 3.7-rc2
Here are some tty and serial driver fixes for your 3.7-rc1 tree.
Again, the UABI header file fixes, and a number of build and runtime serial
driver bugfixes that solve problems people have been reporting (the staging
driver is a tty driver, hence the fixes coming in through this tree.)
All of these have been in the linux-next tree for a while.
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
----------------------------------------------------------------
Alexander Shiyan (1):
serial: sccnxp: Allows the driver to be compiled as a module
Bill Pemberton (2):
staging: dgrp: check for NULL pointer in (un)register_proc_table
staging: dgrp: check return value of alloc_tty_driver
David Howells (1):
UAPI: (Scripted) Disintegrate include/linux/hsi
Geert Uytterhoeven (2):
staging: serial: dgrp: Add missing #include <linux/uaccess.h>
serial/8250_hp300: Missing 8250 register interface conversion bits
Greg Kroah-Hartman (3):
tty: serial: sccnxp: Fix bug with unterminated platform_id list
Merge 3.7-rc1 into tty-linus
Merge tag 'disintegrate-tty-20121009' of git://git.infradead.org/users/dhowells/linux-headers into tty-linus
Markus Trippelsdorf (1):
tty: Fix bogus "callbacks suppressed" messages
Sasha Levin (1):
net, TTY: initialize tty->driver_data before usage
drivers/staging/dgrp/dgrp_mon_ops.c | 1 +
drivers/staging/dgrp/dgrp_specproc.c | 7 +++++++
drivers/staging/dgrp/dgrp_tty.c | 10 ++++++++++
drivers/tty/serial/8250/8250_hp300.c | 20 ++++++++++----------
drivers/tty/serial/Kconfig | 4 ++--
drivers/tty/serial/sccnxp.c | 1 +
include/linux/hsi/Kbuild | 1 -
include/linux/ratelimit.h | 27 +++++++++------------------
include/uapi/linux/hsi/Kbuild | 1 +
include/{ => uapi}/linux/hsi/hsi_char.h | 0
net/irda/ircomm/ircomm_tty.c | 2 ++
11 files changed, 43 insertions(+), 31 deletions(-)
rename include/{ => uapi}/linux/hsi/hsi_char.h (100%)
^ permalink raw reply
* Re: New serial card development
From: Theodore Ts'o @ 2012-10-19 21:21 UTC (permalink / raw)
To: Matt Schulte; +Cc: Alan Cox, linux-serial
In-Reply-To: <CAJp1Oe4n-TtxDM8D82bFQxjBtx8rSAX8mWrFk0bSM6uGL9X=Eg@mail.gmail.com>
On Wed, Oct 17, 2012 at 03:24:06PM -0500, Matt Schulte wrote:
>
> What I would need to have happen is for a flag to be set that says
> "enable 9-bit mode transmit" or something. When set, the first byte
> of data of a given write will go out with mark parity, then the rest
> of the bytes will have space parity. The one byte with mark is the
> address byte.
Why not just have a userspace library routine which simply uses
tcgetattr/tcsetattr to toggle a bit (probably a new bit in c_cflag) in
struct termios, issue the single byte write, and then toggle the bit
back?
If you really want to implement a special case "the first byte should
treated differently from the rest", that gets problemtic for a number
of reasons. First of all, it's just a lot more kernel code; you'd
probably need to implement a new line displine, and secondly, if
userspace sends too much data so that it can't fit in the kernel
buffer, you might get a partial write. In which case, when you send a
second write call to send the rest of the data, you'd have to go
through extra work to make sure the first byte of that second,
follow-up byte doesn't have the high 9th bit set.
Of course, you could hack in even more changes to make the write
system call behave entirely differently in your magic "9th bit mode",
where the write system call becomes a "all or nothing" sort of thing,
but that's even more evil hackery that almost certainly would be
rejected with extreme prejudice by the kernel maintainers.
In Linux, system calls are fast, so using some extra ioctl's to toggle
the termios structure is probably the way to go. It's also much more
general since it doesn't presume a very specific protocol (i.e., your
magic multi-drop protocol).
> In the receive direction I would need to optionally enable a station
> address which the chip will use to verify all incoming data and will
> reject anything that is not of the appropriate address.
For the receive direction, my suggestion is to encode it as an
extension for how PARMRK does things, and then leave the parsing of
the address bits to userspace. Otherwise you'll end up needing to
make the kernel know what are "appropriate" addresses, which again is
a lot of very protocol specific knowledge that you would have to push
into the kernel.
Alan's advice to get your card working as a basic serial card is very
good one. Get basic functionality working, and then you can add the
support for the extra bits later....
- Ted
^ permalink raw reply
* [PATCH] tty: serial: KGDB support for PXA
From: Haojian Zhuang @ 2012-10-21 4:00 UTC (permalink / raw)
To: gregkh, linux-serial; +Cc: Denis V. Lunev, Marko Katic, Eric Miao, Russell King
From: "Denis V. Lunev" <den@openvz.org>
Actually, in order to support KGDB over serial console one must
implement two callbacks for character polling. Clone them from
8250 driver with a bit of tuning.
Signed-off-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Marko Katic <dromede@gmail.com>
CC: Eric Miao <eric.y.miao@gmail.com>
CC: Russell King <linux@arm.linux.org.uk>
Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>
drivers/tty/serial/pxa.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 55 insertions(+), 0 deletions(-)
---
drivers/tty/serial/pxa.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/drivers/tty/serial/pxa.c b/drivers/tty/serial/pxa.c
index 9033fc6..2764828 100644
--- a/drivers/tty/serial/pxa.c
+++ b/drivers/tty/serial/pxa.c
@@ -705,6 +705,57 @@ serial_pxa_console_write(struct console *co, const char *s, unsigned int count)
clk_disable_unprepare(up->clk);
}
+#ifdef CONFIG_CONSOLE_POLL
+/*
+ * Console polling routines for writing and reading from the uart while
+ * in an interrupt or debug context.
+ */
+
+static int serial_pxa_get_poll_char(struct uart_port *port)
+{
+ struct uart_pxa_port *up = (struct uart_pxa_port *)port;
+ unsigned char lsr = serial_in(up, UART_LSR);
+
+ while (!(lsr & UART_LSR_DR))
+ lsr = serial_in(up, UART_LSR);
+
+ return serial_in(up, UART_RX);
+}
+
+
+static void serial_pxa_put_poll_char(struct uart_port *port,
+ unsigned char c)
+{
+ unsigned int ier;
+ struct uart_pxa_port *up = (struct uart_pxa_port *)port;
+
+ /*
+ * First save the IER then disable the interrupts
+ */
+ ier = serial_in(up, UART_IER);
+ serial_out(up, UART_IER, UART_IER_UUE);
+
+ wait_for_xmitr(up);
+ /*
+ * Send the character out.
+ * If a LF, also do CR...
+ */
+ serial_out(up, UART_TX, c);
+ if (c == 10) {
+ wait_for_xmitr(up);
+ serial_out(up, UART_TX, 13);
+ }
+
+ /*
+ * Finally, wait for transmitter to become empty
+ * and restore the IER
+ */
+ wait_for_xmitr(up);
+ serial_out(up, UART_IER, ier);
+}
+
+#endif /* CONFIG_CONSOLE_POLL */
+
static int __init
serial_pxa_console_setup(struct console *co, char *options)
{
@@ -759,6 +810,10 @@ struct uart_ops serial_pxa_pops = {
.request_port = serial_pxa_request_port,
.config_port = serial_pxa_config_port,
.verify_port = serial_pxa_verify_port,
+#ifdef CONFIG_CONSOLE_POLL
+ .poll_get_char = serial_pxa_get_poll_char,
+ .poll_put_char = serial_pxa_put_poll_char,
+#endif
};
static struct uart_driver serial_pxa_reg = {
--
1.7.9.5
^ permalink raw reply related
* [PATCH] serial: ifx6x60: add_timer is not safe in the mrdy_assert function
From: Jun Chen @ 2012-10-22 14:23 UTC (permalink / raw)
To: alan; +Cc: linux-serial, russ.gorby, chuansheng.liu, chao.bi
This patch make use of mod_timer instead of add_timer in the mrdy_assert function.
Because the srdy interrupter can go high when we are running function mrdy_assert and mrdy_assert
can be called by multi-entry. In our medfield platform, spi stress test can encounter this
error logs triggered by the BUG_ON of add_timer function.This patch had been tested on
our medfield platform.
the scenario:
CPU0 CPU1
mrdy_assert
set_bit(IFX_SPI_STATE_TIMER_PENDING)
ifx_spi_handle_srdy
...
clear_bit(IFX_SPI_STATE_TIMER_PENDING)
...
mrdy_assert
set_bit(IFX_SPI_STATE_TIMER_PENDING)
...
add_timer
...
add_timer
cc:liu chuansheng <chuansheng.liu@intel.com>
cc:Bi Chao <chao.bi@intel.com>
Signed-off-by: Chen Jun <jun.d.chen@intel.com>
---
drivers/tty/serial/ifx6x60.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/ifx6x60.c b/drivers/tty/serial/ifx6x60.c
index 769396a..e56ed33 100644
--- a/drivers/tty/serial/ifx6x60.c
+++ b/drivers/tty/serial/ifx6x60.c
@@ -190,9 +190,7 @@ static void mrdy_assert(struct ifx_spi_device *ifx_dev)
if (!val) {
if (!test_and_set_bit(IFX_SPI_STATE_TIMER_PENDING,
&ifx_dev->flags)) {
- ifx_dev->spi_timer.expires =
- jiffies + IFX_SPI_TIMEOUT_SEC*HZ;
- add_timer(&ifx_dev->spi_timer);
+ mod_timer(&ifx_dev->spi_timer,jiffies + IFX_SPI_TIMEOUT_SEC*HZ);
}
}
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH] serial: ifx6x60: add_timer is not safe in the mrdy_assert function
From: Alan Cox @ 2012-10-22 9:55 UTC (permalink / raw)
To: Jun Chen; +Cc: alan, linux-serial, russ.gorby, chuansheng.liu, chao.bi
In-Reply-To: <1350915787.4332.27.camel@chenjun-workstation>
On Mon, 22 Oct 2012 10:23:07 -0400
Jun Chen <jun.d.chen@intel.com> wrote:
>
> This patch make use of mod_timer instead of add_timer in the mrdy_assert function.
> Because the srdy interrupter can go high when we are running function mrdy_assert and mrdy_assert
> can be called by multi-entry. In our medfield platform, spi stress test can encounter this
> error logs triggered by the BUG_ON of add_timer function.This patch had been tested on
> our medfield platform.
>
> the scenario:
> CPU0 CPU1
> mrdy_assert
> set_bit(IFX_SPI_STATE_TIMER_PENDING)
> ifx_spi_handle_srdy
> ...
> clear_bit(IFX_SPI_STATE_TIMER_PENDING)
> ...
> mrdy_assert
> set_bit(IFX_SPI_STATE_TIMER_PENDING)
> ...
> add_timer
> ...
> add_timer
>
> cc:liu chuansheng <chuansheng.liu@intel.com>
> cc:Bi Chao <chao.bi@intel.com>
> Signed-off-by: Chen Jun <jun.d.chen@intel.com>
Acked-by: Alan Cox <alan@linux.intel.com>
^ permalink raw reply
* [RESEND PATCH 1/2] of serial port driver - add clk_get_rate() support
From: Murali Karicheri @ 2012-10-22 15:58 UTC (permalink / raw)
To: alan, gregkh, linux-serial, linux-kernel; +Cc: Murali Karicheri
Currently this driver expects the clock-frequency attribute. This
patch allows getting clock-frequency through clk driver API
clk_get_rate() if clock-frequency attribute is not defined.
So in the device bindings for serial device, one can add clocks
phandle to refer to the clk device to get the rate.
Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
diff --git a/drivers/tty/serial/of_serial.c b/drivers/tty/serial/of_serial.c
index 34e7187..6f64f02 100644
--- a/drivers/tty/serial/of_serial.c
+++ b/drivers/tty/serial/of_serial.c
@@ -21,8 +21,10 @@
#include <linux/of_serial.h>
#include <linux/of_platform.h>
#include <linux/nwpserial.h>
+#include <linux/clk.h>
struct of_serial_info {
+ struct clk *clk;
int type;
int line;
};
@@ -51,7 +53,8 @@ EXPORT_SYMBOL_GPL(tegra_serial_handle_break);
* Fill a struct uart_port for a given device node
*/
static int __devinit of_platform_serial_setup(struct platform_device *ofdev,
- int type, struct uart_port *port)
+ int type, struct uart_port *port,
+ struct of_serial_info *info)
{
struct resource resource;
struct device_node *np = ofdev->dev.of_node;
@@ -60,8 +63,17 @@ static int __devinit of_platform_serial_setup(struct platform_device *ofdev,
memset(port, 0, sizeof *port);
if (of_property_read_u32(np, "clock-frequency", &clk)) {
- dev_warn(&ofdev->dev, "no clock-frequency property set\n");
- return -ENODEV;
+
+ /* Get clk rate through clk driver if present */
+ info->clk = clk_get(&ofdev->dev, NULL);
+ if (info->clk == NULL) {
+ dev_warn(&ofdev->dev,
+ "clk or clock-frequency not defined\n");
+ return -ENODEV;
+ }
+
+ clk_prepare_enable(info->clk);
+ clk = clk_get_rate(info->clk);
}
/* If current-speed was set, then try not to change it. */
if (of_property_read_u32(np, "current-speed", &spd) == 0)
@@ -70,7 +82,7 @@ static int __devinit of_platform_serial_setup(struct platform_device *ofdev,
ret = of_address_to_resource(np, 0, &resource);
if (ret) {
dev_warn(&ofdev->dev, "invalid address\n");
- return ret;
+ goto out;
}
spin_lock_init(&port->lock);
@@ -97,7 +109,8 @@ static int __devinit of_platform_serial_setup(struct platform_device *ofdev,
default:
dev_warn(&ofdev->dev, "unsupported reg-io-width (%d)\n",
prop);
- return -EINVAL;
+ ret = -EINVAL;
+ goto out;
}
}
@@ -111,6 +124,10 @@ static int __devinit of_platform_serial_setup(struct platform_device *ofdev,
port->handle_break = tegra_serial_handle_break;
return 0;
+out:
+ if (info->clk)
+ clk_disable_unprepare(info->clk);
+ return ret;
}
/*
@@ -137,7 +154,7 @@ static int __devinit of_platform_serial_probe(struct platform_device *ofdev)
return -ENOMEM;
port_type = (unsigned long)match->data;
- ret = of_platform_serial_setup(ofdev, port_type, &port);
+ ret = of_platform_serial_setup(ofdev, port_type, &port, info);
if (ret)
goto out;
@@ -193,6 +210,9 @@ static int of_platform_serial_remove(struct platform_device *ofdev)
/* need to add code for these */
break;
}
+
+ if (info->clk)
+ clk_disable_unprepare(info->clk);
kfree(info);
return 0;
}
--
1.7.9.5
^ permalink raw reply related
* [RESEND PATCH 2/2] Documentation: of-serial.txt - update for clocks phandle for clk
From: Murali Karicheri @ 2012-10-22 15:58 UTC (permalink / raw)
To: alan, gregkh, linux-serial, linux-kernel; +Cc: Murali Karicheri
In-Reply-To: <1350921482-22554-1-git-send-email-m-karicheri2@ti.com>
Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
diff --git a/Documentation/devicetree/bindings/tty/serial/of-serial.txt b/Documentation/devicetree/bindings/tty/serial/of-serial.txt
index 0847fde..423b7ff 100644
--- a/Documentation/devicetree/bindings/tty/serial/of-serial.txt
+++ b/Documentation/devicetree/bindings/tty/serial/of-serial.txt
@@ -14,7 +14,10 @@ Required properties:
- "serial" if the port type is unknown.
- reg : offset and length of the register set for the device.
- interrupts : should contain uart interrupt.
-- clock-frequency : the input clock frequency for the UART.
+- clock-frequency : the input clock frequency for the UART
+ or
+ clocks phandle to refer to the clk used as per Documentation/devicetree
+ /bindings/clock/clock-bindings.txt
Optional properties:
- current-speed : the current active speed of the UART.
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH-v3] tty: prevent unnecessary work queue lock checking on flip buffer copy
From: Greg KH @ 2012-10-22 23:47 UTC (permalink / raw)
To: Ivo Sieben; +Cc: Alan Cox, linux-serial, RT
In-Reply-To: <1348747325-13539-1-git-send-email-meltedpianoman@gmail.com>
On Thu, Sep 27, 2012 at 02:02:05PM +0200, Ivo Sieben wrote:
> When low_latency flag is set the TTY receive flip buffer is copied to the
> line discipline directly instead of using a work queue in the background.
> Therefor only in case a workqueue is actually used for copying data to the
> line discipline we'll have to flush the workqueue.
>
> This prevents unnecessary spin lock/unlock on the workqueue spin lock that
> can cause additional scheduling overhead on a PREEMPT_RT system. On a 200
> MHz AT91SAM9261 processor setup this fixes about 100us of scheduling
> overhead on the TTY read call.
>
> Signed-off-by: Ivo Sieben <meltedpianoman@gmail.com>
Note, I took out the WARN_ON() in this patch, as what is that really
going to help here? It also will conflict with a patch from Jiri I'll
ba applying after this one, so if you think it's needed, care to send a
follow-on patch based on linux-next in a few days?
thanks,
greg k-h
^ permalink raw reply
* [PATCH] serial: 8250 check iir rdi in interrupt
From: Min Zhang @ 2012-10-23 0:19 UTC (permalink / raw)
To: gregkh, linux-serial; +Cc: linux-kernel
The patch works around two UART interrupt bugs when the serial console is
flooded with inputs:
1. syslog shows "serial8250: too much works for irq"
2. serial console stops responding to key stroke
serial8250_handle_irq() checks UART_IIR_RDI before reading receive fifo
and clears bogus interrupt UART_IIR_RDI without accompanying UART_LSR_DR,
otherwise RDI interrupt could freeze or too many unhandled RDI interrupts.
Added module parameter skip_rdi_check to opt out this workaround.
Tested on Radisys ATCA 46XX which uses FPGA 16550-compatible and
other generic 16550 UART. It takes from an hour to days to reproduce by
pumping inputs to serial console continously using TeraTerm script:
---
drivers/tty/serial/8250/8250.c | 55 ++++++++++++++++++++++++++++++++++++++++
1 files changed, 55 insertions(+), 0 deletions(-)
diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index 3ba4234..838dd22 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -64,6 +64,7 @@ static int serial_index(struct uart_port *port)
}
static unsigned int skip_txen_test; /* force skip of txen test at init time */
+static unsigned int skip_rdi_check; /* skip of IIR RDI check in interrupt */
/*
* Debugging.
@@ -1479,6 +1480,51 @@ unsigned int serial8250_modem_status(struct uart_8250_port *up)
EXPORT_SYMBOL_GPL(serial8250_modem_status);
/*
+ * Check if status UART_LSR_RD accompanies with interrupt UART_IIR_RDI.
+ * If they are mismatch, massage the status or interupt cause accordingly:
+ *
+ * Return a cleared UART_LSR_RD status if there is no accompanying
+ * UART_IIR_RDI. Hopefully the new status is used by interrupt handler
+ * to skip reading receive FIFO. Otherwise some UART controller stops
+ * generating RDI interrupt after this unnotified FIFO read, until other
+ * interrupts maybe transmit interrupt reads UART_LSR again.
+ *
+ * Or clear interrupt cause UART_IIR_RDI without UART_LSR_RD. The UART sets
+ * UART_IIR_RDI *even* if the received data has been read out from the FIFO
+ * before the timeout occurs. To clear UART_IIR_RDI, read receive buffer
+ * register. Reading it also clears timeout interrupt for 16550+. Otherwise
+ * the uncleared UART_IIR_RDI will keep triggering IRQ but interrupt
+ * handler finds nothing to do.
+ *
+ * Skip this workaround if interrupt is not expected, such as backup timer,
+ * so that handler can still solely rely on original status register.
+ */
+static unsigned char serial8250_iir_rdi_check(struct uart_8250_port *up,
+ unsigned char status,
+ unsigned int iir)
+{
+ unsigned int ier, rdi_stat, rdi_intr;
+
+ /* skip for handler without interrupt */
+ if (!up->port.irq)
+ return status;
+
+ /* skip for polling handler such as backup timer */
+ ier = serial_in(up, UART_IER);
+ if (!(ier & UART_IER_RDI))
+ return status;
+
+ rdi_stat = status & UART_LSR_DR;
+ rdi_intr = iir & UART_IIR_RDI;
+
+ if (rdi_stat && !rdi_intr)
+ status &= ~UART_LSR_DR;
+ else if (!rdi_stat && rdi_intr)
+ serial_in(up, UART_RX);
+ return status;
+}
+
+/*
* This handles the interrupt from one port.
*/
int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
@@ -1497,6 +1543,12 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
DEBUG_INTR("status = %x...", status);
+ /* Some UART controller has mismatched UART_IIR_RDI and UART_LSR_DR,
+ which causes either too many interrupts or interrupt freeze
+ */
+ if (!skip_rdi_check)
+ status = serial8250_iir_rdi_check(up, status, iir);
+
if (status & (UART_LSR_DR | UART_LSR_BI))
status = serial8250_rx_chars(up, status);
serial8250_modem_status(up);
@@ -3338,6 +3390,9 @@ MODULE_PARM_DESC(nr_uarts, "Maximum number of UARTs supported. (1-" __MODULE_STR
module_param(skip_txen_test, uint, 0644);
MODULE_PARM_DESC(skip_txen_test, "Skip checking for the TXEN bug at init time");
+module_param(skip_rdi_check, uint, 0644);
+MODULE_PARM_DESC(skip_rdi_check, "Skip checking IIR RDI bug in interrupt");
+
#ifdef CONFIG_SERIAL_8250_RSA
module_param_array(probe_rsa, ulong, &probe_rsa_count, 0444);
MODULE_PARM_DESC(probe_rsa, "Probe I/O ports for RSA");
--
1.7.7.4
^ permalink raw reply related
* [tty:tty-next 15/23] drivers/staging/dgrp/dgrp_net_ops.c:216:27: error: 'struct tty_struct' has no member named 'real_raw'
From: Fengguang Wu @ 2012-10-23 2:25 UTC (permalink / raw)
To: Jiri Slaby; +Cc: Yuanhan Liu, Greg Kroah-Hartman, linux-serial
Hi Jiri,
FYI, kernel build failed on
tree: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-next
head: ecbbfd44a08fa80e0d664814efd4c187721b85f6
commit: 53c5ee2cfb4dadc4f5c24fe671e2fbfc034c875e [15/23] TTY: move ldisc data from tty_struct: simple members
config: x86_64-allmodconfig # make ARCH=x86_64 allmodconfig
All error/warnings:
drivers/staging/dgrp/dgrp_net_ops.c: In function 'dgrp_input':
drivers/staging/dgrp/dgrp_net_ops.c:216:27: error: 'struct tty_struct' has no member named 'real_raw'
drivers/staging/dgrp/dgrp_net_ops.c:261:30: error: 'struct tty_struct' has no member named 'real_raw'
drivers/staging/dgrp/dgrp_net_ops.c:276:28: error: 'struct tty_struct' has no member named 'real_raw'
vim +216 drivers/staging/dgrp/dgrp_net_ops.c
0b52b749 Bill Pemberton 2012-09-20 215 /* Decide how much data we can send into the tty layer */
0b52b749 Bill Pemberton 2012-09-20 @216 if (dgrp_rawreadok && tty->real_raw)
0b52b749 Bill Pemberton 2012-09-20 217 flip_len = MYFLIPLEN;
0b52b749 Bill Pemberton 2012-09-20 218 else
0b52b749 Bill Pemberton 2012-09-20 219 flip_len = TTY_FLIPBUF_SIZE;
---
0-DAY kernel build testing backend Open Source Technology Center
Fengguang Wu, Yuanhan Liu Intel Corporation
^ permalink raw reply
* [tty:tty-next 17/23] drivers/staging/dgrp/dgrp_net_ops.c:229:8: error: 'struct tty_struct' has no member named 'read_cnt'
From: Fengguang Wu @ 2012-10-23 2:30 UTC (permalink / raw)
To: Jiri Slaby; +Cc: Yuanhan Liu, Greg Kroah-Hartman, linux-serial
Hi Jiri,
FYI, kernel build failed on
tree: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-next
head: ecbbfd44a08fa80e0d664814efd4c187721b85f6
commit: ba2e68ac6157004ee4922fb39ebd9459bbae883e [17/23] TTY: move ldisc data from tty_struct: read_* and echo_* and canon_* stuff
config: x86_64-allmodconfig # make ARCH=x86_64 allmodconfig
All error/warnings:
drivers/staging/dgrp/dgrp_net_ops.c: In function 'dgrp_input':
drivers/staging/dgrp/dgrp_net_ops.c:216:27: error: 'struct tty_struct' has no member named 'real_raw'
drivers/staging/dgrp/dgrp_net_ops.c:229:8: error: 'struct tty_struct' has no member named 'read_cnt'
drivers/staging/dgrp/dgrp_net_ops.c:229:8: error: 'struct tty_struct' has no member named 'read_cnt'
drivers/staging/dgrp/dgrp_net_ops.c:261:30: error: 'struct tty_struct' has no member named 'real_raw'
drivers/staging/dgrp/dgrp_net_ops.c:276:28: error: 'struct tty_struct' has no member named 'real_raw'
vim +229 drivers/staging/dgrp/dgrp_net_ops.c
0b52b749 Bill Pemberton 2012-09-20 228 /* take into consideration length of ldisc */
0b52b749 Bill Pemberton 2012-09-20 @229 len = min(len, (N_TTY_BUF_SIZE - 1) - tty->read_cnt);
---
0-DAY kernel build testing backend Open Source Technology Center
Fengguang Wu, Yuanhan Liu Intel Corporation
^ permalink raw reply
* Re: [tty:tty-next 15/23] drivers/staging/dgrp/dgrp_net_ops.c:216:27: error: 'struct tty_struct' has no member named 'real_raw'
From: Greg Kroah-Hartman @ 2012-10-23 3:05 UTC (permalink / raw)
To: Fengguang Wu; +Cc: Jiri Slaby, Yuanhan Liu, linux-serial
In-Reply-To: <20121023022545.GG6830@localhost>
On Tue, Oct 23, 2012 at 10:25:45AM +0800, Fengguang Wu wrote:
> Hi Jiri,
>
> FYI, kernel build failed on
>
> tree: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-next
> head: ecbbfd44a08fa80e0d664814efd4c187721b85f6
> commit: 53c5ee2cfb4dadc4f5c24fe671e2fbfc034c875e [15/23] TTY: move ldisc data from tty_struct: simple members
> config: x86_64-allmodconfig # make ARCH=x86_64 allmodconfig
>
> All error/warnings:
>
> drivers/staging/dgrp/dgrp_net_ops.c: In function 'dgrp_input':
> drivers/staging/dgrp/dgrp_net_ops.c:216:27: error: 'struct tty_struct' has no member named 'real_raw'
> drivers/staging/dgrp/dgrp_net_ops.c:261:30: error: 'struct tty_struct' has no member named 'real_raw'
> drivers/staging/dgrp/dgrp_net_ops.c:276:28: error: 'struct tty_struct' has no member named 'real_raw'
>
> vim +216 drivers/staging/dgrp/dgrp_net_ops.c
>
> 0b52b749 Bill Pemberton 2012-09-20 215 /* Decide how much data we can send into the tty layer */
> 0b52b749 Bill Pemberton 2012-09-20 @216 if (dgrp_rawreadok && tty->real_raw)
> 0b52b749 Bill Pemberton 2012-09-20 217 flip_len = MYFLIPLEN;
> 0b52b749 Bill Pemberton 2012-09-20 218 else
> 0b52b749 Bill Pemberton 2012-09-20 219 flip_len = TTY_FLIPBUF_SIZE;
Thanks for this, and the other report, I already pointed them out to
Jiri and hopefully will get a patch from him when he returns from the
openSUSE conference.
thanks,
greg k-h
^ permalink raw reply
* Re: [tty:tty-next 15/23] drivers/staging/dgrp/dgrp_net_ops.c:216:27: error: 'struct tty_struct' has no member named 'real_raw'
From: Fengguang Wu @ 2012-10-23 3:54 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jiri Slaby, Yuanhan Liu, linux-serial
In-Reply-To: <20121023030543.GB22018@kroah.com>
On Mon, Oct 22, 2012 at 08:05:43PM -0700, Greg KH wrote:
> On Tue, Oct 23, 2012 at 10:25:45AM +0800, Fengguang Wu wrote:
> > Hi Jiri,
> >
> > FYI, kernel build failed on
> >
> > tree: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-next
> > head: ecbbfd44a08fa80e0d664814efd4c187721b85f6
> > commit: 53c5ee2cfb4dadc4f5c24fe671e2fbfc034c875e [15/23] TTY: move ldisc data from tty_struct: simple members
> > config: x86_64-allmodconfig # make ARCH=x86_64 allmodconfig
> >
> > All error/warnings:
> >
> > drivers/staging/dgrp/dgrp_net_ops.c: In function 'dgrp_input':
> > drivers/staging/dgrp/dgrp_net_ops.c:216:27: error: 'struct tty_struct' has no member named 'real_raw'
> > drivers/staging/dgrp/dgrp_net_ops.c:261:30: error: 'struct tty_struct' has no member named 'real_raw'
> > drivers/staging/dgrp/dgrp_net_ops.c:276:28: error: 'struct tty_struct' has no member named 'real_raw'
> >
> > vim +216 drivers/staging/dgrp/dgrp_net_ops.c
> >
> > 0b52b749 Bill Pemberton 2012-09-20 215 /* Decide how much data we can send into the tty layer */
> > 0b52b749 Bill Pemberton 2012-09-20 @216 if (dgrp_rawreadok && tty->real_raw)
> > 0b52b749 Bill Pemberton 2012-09-20 217 flip_len = MYFLIPLEN;
> > 0b52b749 Bill Pemberton 2012-09-20 218 else
> > 0b52b749 Bill Pemberton 2012-09-20 219 flip_len = TTY_FLIPBUF_SIZE;
>
>
> Thanks for this, and the other report, I already pointed them out to
> Jiri and hopefully will get a patch from him when he returns from the
> openSUSE conference.
OK, thanks for the info. BTW, expect another related error:
arch/um/drivers/chan_kern.c:89:42: error: 'struct tty_struct' has no member named 'raw'
Thanks,
Fengguang
^ permalink raw reply
* Subject: [PATCH] serial:ifx6x60:add swap_buf_32 function in case SPI word width is 32 bits
From: chao bi @ 2012-10-23 8:32 UTC (permalink / raw)
To: alan; +Cc: linux-serial, richardx.r.gorby, jun.d.chen, chuansheng.liu
SPI protocol driver only provide one function (swap_buf()) to swap SPI data
into big endian format, which is only available when SPI controller's word width
is 16 bits. But per our experiment and validation on medfield platform, 16 bit
word width cannot cover all scenarioes, 32 bits word width is a better choice
to avoid SPI FIFO overrun. Therefore, SPI controller is likely to configure its
word width as either 16 bits or 32 bits. This patch is to implement 2 functions
(swap_buf_16() and swap_buf_32()) to adapt the two configurations.
cc: liu chuansheng <chuansheng.liu@intel.com>
cc: Chen Jun <jun.d.chen@intel.com>
Signed-off-by: channing <chao.bi@intel.com>
---
drivers/tty/serial/ifx6x60.c | 51 +++++++++++++++++++++++++++++++++--------
1 files changed, 41 insertions(+), 10 deletions(-)
diff --git a/drivers/tty/serial/ifx6x60.c b/drivers/tty/serial/ifx6x60.c
index 5b9bc19..ad718ec 100644
--- a/drivers/tty/serial/ifx6x60.c
+++ b/drivers/tty/serial/ifx6x60.c
@@ -151,29 +151,60 @@ ifx_spi_power_state_clear(struct ifx_spi_device *ifx_dev, unsigned char val)
spin_unlock_irqrestore(&ifx_dev->power_lock, flags);
}
+#ifdef SPI_CONTROLLER_USE_32_BITS_PER_WORD
+#define SWAP_BUF swap_buf_32
/**
- * swap_buf
+ * swap_buf_32
* @buf: our buffer
* @len : number of bytes (not words) in the buffer
* @end: end of buffer
*
* Swap the contents of a buffer into big endian format
*/
-static inline void swap_buf(u16 *buf, int len, void *end)
+static inline void swap_buf_32(unsigned char *buf, int len, void *end)
{
int n;
+ u32 *buf_32 = (u32 *)buf;
+ len = (0 == (len&0x03)) ? (len >> 2) : ((len >> 2) + 1);
+
+ if ((void *)&buf_32[len] > end) {
+ pr_err("swap_buf_32: swap exceeds boundary (%p > %p)!\n",
+ &buf_32[len], end);
+ return;
+ }
+ for (n = 0; n < len; n++) {
+ *buf_32 = cpu_to_be32(*buf_32);
+ buf_32++;
+ }
+}
+#else
+#define SWAP_BUF swap_buf_16
+/**
+ * swap_buf_16
+ * @buf: our buffer
+ * @len : number of bytes (not words) in the buffer
+ * @end: end of buffer
+ *
+ * Swap the contents of a buffer into big endian format
+ */
+static inline void swap_buf_16(unsigned char *buf, int len, void *end)
+{
+ int n;
+
+ u16 *buf_16 = (u16 *)buf;
len = ((len + 1) >> 1);
- if ((void *)&buf[len] > end) {
- pr_err("swap_buf: swap exceeds boundary (%p > %p)!",
- &buf[len], end);
+ if ((void *)&buf_16[len] > end) {
+ pr_err("swap_buf_16: swap exceeds boundary (%p > %p)!",
+ &buf_16[len], end);
return;
}
for (n = 0; n < len; n++) {
- *buf = cpu_to_be16(*buf);
- buf++;
+ *buf_16 = cpu_to_be16(*buf_16);
+ buf_16++;
}
}
+#endif
/**
* mrdy_assert - assert MRDY line
@@ -449,7 +480,7 @@ static int ifx_spi_prepare_tx_buffer(struct ifx_spi_device *ifx_dev)
tx_count-IFX_SPI_HEADER_OVERHEAD,
ifx_dev->spi_more);
/* swap actual data in the buffer */
- swap_buf((u16 *)(ifx_dev->tx_buffer), tx_count,
+ SWAP_BUF((ifx_dev->tx_buffer), tx_count,
&ifx_dev->tx_buffer[IFX_SPI_TRANSFER_SIZE]);
return tx_count;
}
@@ -617,7 +648,7 @@ static void ifx_spi_complete(void *ctx)
if (!ifx_dev->spi_msg.status) {
/* check header validity, get comm flags */
- swap_buf((u16 *)ifx_dev->rx_buffer, IFX_SPI_HEADER_OVERHEAD,
+ SWAP_BUF(ifx_dev->rx_buffer, IFX_SPI_HEADER_OVERHEAD,
&ifx_dev->rx_buffer[IFX_SPI_HEADER_OVERHEAD]);
decode_result = ifx_spi_decode_spi_header(ifx_dev->rx_buffer,
&length, &more, &cts);
@@ -636,7 +667,7 @@ static void ifx_spi_complete(void *ctx)
actual_length = min((unsigned int)length,
ifx_dev->spi_msg.actual_length);
- swap_buf((u16 *)(ifx_dev->rx_buffer + IFX_SPI_HEADER_OVERHEAD),
+ SWAP_BUF((ifx_dev->rx_buffer + IFX_SPI_HEADER_OVERHEAD),
actual_length,
&ifx_dev->rx_buffer[IFX_SPI_TRANSFER_SIZE]);
ifx_spi_insert_flip_string(
--
1.7.1
^ permalink raw reply related
* Re: Subject: [PATCH] serial:ifx6x60:add swap_buf_32 function in case SPI word width is 32 bits
From: Alan Cox @ 2012-10-23 9:39 UTC (permalink / raw)
To: chao bi; +Cc: alan, linux-serial, richardx.r.gorby, jun.d.chen, chuansheng.liu
In-Reply-To: <1350981170.18855.83.camel@bichao>
> is 16 bits. But per our experiment and validation on medfield platform, 16 bit
> word width cannot cover all scenarioes, 32 bits word width is a better choice
> to avoid SPI FIFO overrun. Therefore, SPI controller is likely to configure its
> word width as either 16 bits or 32 bits. This patch is to implement 2 functions
> (swap_buf_16() and swap_buf_32()) to adapt the two configurations.
And how will you get both to work with the same kernel, which is the way
Linux wants to work (and avoids the nasty mess some other embedded
platforms have gotten themselves into).
What you probably want to do is is put a 16/32bit flag into the platform
data and then in ifx_spi_spi_probe() just do
if (pl_data->spi_32)
ifx_dev->swap = swap_buf_32;
else
ifx_dev->swap = swap_buf;
then call
ifx_dev->swap_buf()
Alan
^ permalink raw reply
* Re: [tty:tty-next 17/23] drivers/staging/dgrp/dgrp_net_ops.c:229:8: error: 'struct tty_struct' has no member named 'read_cnt'
From: Alan Cox @ 2012-10-23 9:53 UTC (permalink / raw)
To: Fengguang Wu; +Cc: Jiri Slaby, Yuanhan Liu, Greg Kroah-Hartman, linux-serial
In-Reply-To: <20121023023054.GI6830@localhost>
On Tue, 23 Oct 2012 10:30:54 +0800
Fengguang Wu <fengguang.wu@intel.com> wrote:
> Hi Jiri,
>
> FYI, kernel build failed on
>
> tree: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-next
> head: ecbbfd44a08fa80e0d664814efd4c187721b85f6
> commit: ba2e68ac6157004ee4922fb39ebd9459bbae883e [17/23] TTY: move ldisc data from tty_struct: read_* and echo_* and canon_* stuff
> config: x86_64-allmodconfig # make ARCH=x86_64 allmodconfig
>
> All error/warnings:
>
> drivers/staging/dgrp/dgrp_net_ops.c: In function 'dgrp_input':
> drivers/staging/dgrp/dgrp_net_ops.c:216:27: error: 'struct tty_struct' has no member named 'real_raw'
> drivers/staging/dgrp/dgrp_net_ops.c:229:8: error: 'struct tty_struct' has no member named 'read_cnt'
> drivers/staging/dgrp/dgrp_net_ops.c:229:8: error: 'struct tty_struct' has no member named 'read_cnt'
> drivers/staging/dgrp/dgrp_net_ops.c:261:30: error: 'struct tty_struct' has no member named 'real_raw'
> drivers/staging/dgrp/dgrp_net_ops.c:276:28: error: 'struct tty_struct' has no member named 'real_raw'
>
> vim +229 drivers/staging/dgrp/dgrp_net_ops.c
>
> 0b52b749 Bill Pemberton 2012-09-20 228 /* take into consideration length of ldisc */
> 0b52b749 Bill Pemberton 2012-09-20 @229 len = min(len, (N_TTY_BUF_SIZE - 1) - tty->read_cnt);
This is broken and unsafe. It's always been broken and unsafe. Probably
the report wants directing to whoever signed up to fix it all in staging.
Alan
^ permalink raw reply
* Re: [PATCH] serial: 8250 check iir rdi in interrupt
From: Alan Cox @ 2012-10-23 10:01 UTC (permalink / raw)
To: Min Zhang; +Cc: gregkh, linux-serial, linux-kernel
In-Reply-To: <alpine.LFD.2.02.1210221633450.28146@nightowl>
> Added module parameter skip_rdi_check to opt out this workaround.
NAK. Anything like this should be runtime.
> Tested on Radisys ATCA 46XX which uses FPGA 16550-compatible and
> other generic 16550 UART. It takes from an hour to days to reproduce by
> pumping inputs to serial console continously using TeraTerm script:
You turn this on by default but it's a nasty IRQ latency penalty
on a lot of x86 platforms with the uarts on the lpc bus.
What I am not clear on from this is
- do you see it on both the ports (the bug that is)
- if you do see it on both are you sure its not in reality a symptom of
some other console/irq handling race ?
Alan
^ permalink raw reply
* Re: [tty:tty-next 17/23] drivers/staging/dgrp/dgrp_net_ops.c:229:8: error: 'struct tty_struct' has no member named 'read_cnt'
From: Fengguang Wu @ 2012-10-23 10:00 UTC (permalink / raw)
To: Alan Cox
Cc: Bill Pemberton, Jiri Slaby, Yuanhan Liu, Greg Kroah-Hartman,
linux-serial
In-Reply-To: <20121023105330.13d31938@pyramind.ukuu.org.uk>
On Tue, Oct 23, 2012 at 10:53:30AM +0100, Alan Cox wrote:
> On Tue, 23 Oct 2012 10:30:54 +0800
> Fengguang Wu <fengguang.wu@intel.com> wrote:
>
> > Hi Jiri,
> >
> > FYI, kernel build failed on
> >
> > tree: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-next
> > head: ecbbfd44a08fa80e0d664814efd4c187721b85f6
> > commit: ba2e68ac6157004ee4922fb39ebd9459bbae883e [17/23] TTY: move ldisc data from tty_struct: read_* and echo_* and canon_* stuff
> > config: x86_64-allmodconfig # make ARCH=x86_64 allmodconfig
> >
> > All error/warnings:
> >
> > drivers/staging/dgrp/dgrp_net_ops.c: In function 'dgrp_input':
> > drivers/staging/dgrp/dgrp_net_ops.c:216:27: error: 'struct tty_struct' has no member named 'real_raw'
> > drivers/staging/dgrp/dgrp_net_ops.c:229:8: error: 'struct tty_struct' has no member named 'read_cnt'
> > drivers/staging/dgrp/dgrp_net_ops.c:229:8: error: 'struct tty_struct' has no member named 'read_cnt'
> > drivers/staging/dgrp/dgrp_net_ops.c:261:30: error: 'struct tty_struct' has no member named 'real_raw'
> > drivers/staging/dgrp/dgrp_net_ops.c:276:28: error: 'struct tty_struct' has no member named 'real_raw'
> >
> > vim +229 drivers/staging/dgrp/dgrp_net_ops.c
> >
> > 0b52b749 Bill Pemberton 2012-09-20 228 /* take into consideration length of ldisc */
> > 0b52b749 Bill Pemberton 2012-09-20 @229 len = min(len, (N_TTY_BUF_SIZE - 1) - tty->read_cnt);
>
> This is broken and unsafe. It's always been broken and unsafe. Probably
> the report wants directing to whoever signed up to fix it all in staging.
Add CC to Bill. Alan, thanks for the suggestion!
As for the build system, it perhaps can hardly be more smart than
notifying the contacts for the commit that *directly* triggered the
build errors (bisected first bad commit is not necessary the root
cause). It relies on the CCed developers to figure out the real
problem and if necessary, add more CC list.
Thanks,
Fengguang
^ permalink raw reply
* Re: [PATCH-v3] tty: prevent unnecessary work queue lock checking on flip buffer copy
From: Alan Cox @ 2012-10-23 10:16 UTC (permalink / raw)
To: Greg KH; +Cc: Ivo Sieben, linux-serial, RT
In-Reply-To: <20121022234746.GA12743@kroah.com>
> Note, I took out the WARN_ON() in this patch, as what is that really
> going to help here? It also will conflict with a patch from Jiri I'll
> ba applying after this one, so if you think it's needed, care to send
> a follow-on patch based on linux-next in a few days?
It catches anyone flipping low_latency wrongly or in other places. It's
fairly important as WARN_ON() goes !
^ permalink raw reply
* Re: New serial card development
From: Matt Schulte @ 2012-10-23 16:27 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Alan Cox, linux-serial
In-Reply-To: <20121019212158.GB4721@thunk.org>
On Fri, Oct 19, 2012 at 4:21 PM, Theodore Ts'o <tytso@mit.edu> wrote:
> On Wed, Oct 17, 2012 at 03:24:06PM -0500, Matt Schulte wrote:
>>
>> What I would need to have happen is for a flag to be set that says
>> "enable 9-bit mode transmit" or something. When set, the first byte
>> of data of a given write will go out with mark parity, then the rest
>> of the bytes will have space parity. The one byte with mark is the
>> address byte.
>
> Why not just have a userspace library routine which simply uses
> tcgetattr/tcsetattr to toggle a bit (probably a new bit in c_cflag) in
> struct termios, issue the single byte write, and then toggle the bit
> back?
> In Linux, system calls are fast, so using some extra ioctl's to toggle
> the termios structure is probably the way to go. It's also much more
> general since it doesn't presume a very specific protocol (i.e., your
> magic multi-drop protocol).
The only reason that I could come up with as to why not to do that is
the probably delay between the first byte and the second byte.
Perhaps the system calls will be so quick that this delay won't be an
issue. Other than that I think that a new bit in c_cflag would be a
pretty decent solution.
Also this isn't a magic protocol, it is a very old protocol that is
apparently still in use today because I have several customers still
asking for it.
>> In the receive direction I would need to optionally enable a station
>> address which the chip will use to verify all incoming data and will
>> reject anything that is not of the appropriate address.
>
> For the receive direction, my suggestion is to encode it as an
> extension for how PARMRK does things, and then leave the parsing of
> the address bits to userspace. Otherwise you'll end up needing to
> make the kernel know what are "appropriate" addresses, which again is
> a lot of very protocol specific knowledge that you would have to push
> into the kernel.
The address is something that is decided by the user, it could
literally be anything. All I need is a way to write a byte to the
appropriate UART register and a bit to turn it on and off.
> Alan's advice to get your card working as a basic serial card is very
> good one. Get basic functionality working, and then you can add the
> support for the extra bits later....
I can see the logic in getting it working as a basic serial card
first. I think at minimum I would still need to implement the extra
divisor calculations to get accurate bit rates.
So when it works as a basic serial card, I assume you would want me to
use the default PCI IDs to keep it more generic. Then would I add my
own PCI IDs and refer them back to the generic port?
Matt
^ permalink raw reply
* Re: [PATCH 0/1] staging: Add firewire-serial driver
From: Peter Hurley @ 2012-10-23 16:30 UTC (permalink / raw)
To: Alan Cox
Cc: Greg Kroah-Hartman, Stefan Richter, devel, linux1394-devel,
linux-kernel, linux-serial
In-Reply-To: <20121023105140.5996c3a5@pyramind.ukuu.org.uk>
[cc'ing linux-serial]
On Tue, 2012-10-23 at 10:51 +0100, Alan Cox wrote:
> > 1. The ldisc drops the contents of tty_buffer on hangup (rather than
> > waiting for completion). Maybe for other devices this isn't so
> > noticeable because the ldisc can mostly keep up with the device, but on
> > firewire the ldisc lags well behind. Right now, this driver works around
> > this by holding off the hangup until the ldisc empties the tty_buffer.
> > The driver determines how much data is still left in the tty_buffer by
> > walking the flip buffers.
>
> The driver should not be trying to look at this, so it's good that it
> broke now. If you need it to process the data on then hangup then the
> core code wants fixing to allow this to occur.
Sorry. I assumed not waiting for ldisc completion on hangup was
architectural.
tty_ldisc_hangup() has this comment from '09:
/*
* FIXME: Once we trust the LDISC code better we can wait here for
* ldisc completion and fix the driver call race
*/
and then several lines below specifically stops emptying the tty_buffer:
cancel_work_sync(&tty->buf.work);
But even earlier in tty_ldisc_hangup(), the ldisc has been flushed
(which the n_tty ldisc interprets as a reset).
> > 2. Because this driver can fill the entire tty_buffer (64K +) before the
> > ldisc even runs once, this driver has to self-throttle when feeding the
> > tty_buffer.
>
> That wants investigating properly. We do multiple megabits quite happily
> via the tty layer with 3G modems over USB. What is your data rate ?
The _slowest_ Firewire does 125 Mbits/sec using only the portion of the
bus cycle assigned for async tx (which is what this driver uses). That's
2Kbytes every 125us. Real world can be pretty close to that because
Firewire uses autonomous DMA and most controllers allow spillover into
the portion of bus cycle assigned for sync tx (which is another 4Kbytes
per 125us). (Technical note: the actual total max for combined async
and sync tx is 6144 bytes per 125us clock)
2Kbytes/125us = 16Kbytes/1ms = 160Kbytes/jiffy (for CONFIG_HZ_100)
Some retail Firewire controllers do twice that, controllers in
early-production do 4x that, and reportedly at least one prototype does
8x that (although I haven't seen it or tested it).
'cat /dev/fwtty0' using this driver gets considerably less that, but
still enough to find this race.
Ignoring for a moment the data rates, the throttling logic is racy
because the ldisc sends the throttle from non-atomic context (so that
termios can be locked and accessed) but the driver fills the tty_buffer
from atomic context. In other words, the condition for which the ldisc
will send a throttle (receive_room < TTY_THRESHOLD_THROTTLE) hasn't yet
happened, but it will the next time receive_buf() runs.
So, the best that tty_insert_flip_string_* can do is return an error
that tty_buffer_alloc() refuses to allocate more than 64k. But the
problem is this driver has data that __must__ go somewhere at that
particular point in time. This driver could introduce another layer of
buffering in front of this but that would defeat the purpose of even
having tty_buffer in the first place.
Checking how much space might be avail for future rx and throttling if
too low seems like the natural solution here. Of course, directly
peeking at tty_buffer is the wrong way to go about doing it. But what if
that was properly exposed by tty_buffer, such as (adjusted for Jiri's
recent changes):
int tty_buffer_space_avail(struct tty_struct *tty)
{
struct tty_port *port = tty->port;
struct tty_bufhead *buf = &port->buf;
unsigned long flags;
int avail;
spin_lock_irqsave(&buf->lock, flags);
avail = 65536 - buf.memory_used; /* threshold used by tty_buffer_alloc() */
spin_unlock_irqrestore(&buf->lock, flags);
return avail;
}
Regards,
Peter Hurley
^ permalink raw reply
* Re: New serial card development
From: Matt Schulte @ 2012-10-23 16:31 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Alan Cox, linux-serial
In-Reply-To: <CAJp1Oe6ekkaZ29iwJ91VXwM_CSL6Z0_KEaqVQTa1XMzTB5niEQ@mail.gmail.com>
On Tue, Oct 23, 2012 at 11:27 AM, Matt Schulte
<matts@commtech-fastcom.com> wrote:
> On Fri, Oct 19, 2012 at 4:21 PM, Theodore Ts'o <tytso@mit.edu> wrote:
>> Alan's advice to get your card working as a basic serial card is very
>> good one. Get basic functionality working, and then you can add the
>> support for the extra bits later....
>
> I can see the logic in getting it working as a basic serial card
> first. I think at minimum I would still need to implement the extra
> divisor calculations to get accurate bit rates.
>
> So when it works as a basic serial card, I assume you would want me to
> use the default PCI IDs to keep it more generic. Then would I add my
> own PCI IDs and refer them back to the generic port?
>
As more of a procedural question, when I go to make the patch(es) to
submit which, kernel repo do I start with? Do I start with Greg KH's
tty repo and then generate the patches and submit them here?
Matt
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox