* [PATCH v2] tty/serial: atmel: use port->name as name in request_irq()
From: Sebastian Andrzej Siewior @ 2018-05-04 8:14 UTC (permalink / raw)
To: Richard Genoud
Cc: Rob Herring, Alexandre Belloni, Peter Hurley, Greg Kroah-Hartman,
linux-serial, Jiri Slaby, tglx, linux-arm-kernel
In-Reply-To: <80203f75-f38e-6214-e9b6-8512c5941647@sorico.fr>
I was puzzled while looking at /proc/interrupts and random things showed
up between reboots. This occurred more often but I realised it later. The
"correct" output should be:
|38: 11861 atmel-aic5 2 Level ttyS0
but I saw sometimes
|38: 6426 atmel-aic5 2 Level tty1
and accounted it wrongly as correct. This is use after free and the
former example randomly got the "old" pointer which pointed to the same
content. With SLAB_FREELIST_RANDOM and HARDENED I even got
|38: 7067 atmel-aic5 2 Level E=Started User Manager for UID 0
or other nonsense.
As it turns out the tty, pointer that is accessed in atmel_startup(), is
freed() before atmel_shutdown(). It seems to happen quite often that the
tty for ttyS0 is allocated and freed while ->shutdown is not invoked. I
don't do anything special - just a systemd boot :)
Use port->name as the IRQ name for request_irq(). This exists as long as
the driver is loaded so no use-after-free here.
For backports before v4.12 I suggest to use `"atmel_serial"' instead
`port->name' (that member was introduced in f7048b15900f ("tty: serial_core:
Add name field to uart_port struct").
Cc: stable@vger.kernel.org
Fixes: 761ed4a94582 ("tty: serial_core: convert uart_close to use tty_port_close")
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
v1…v2: - Bisected and added a Fixes tag
- added a note for backporters to v4.9 … v4.12 (pointed out by
Richard Genoud)
drivers/tty/serial/atmel_serial.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index e287fe8f10fc..d3189816740e 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -1757,7 +1757,6 @@ static int atmel_startup(struct uart_port *port)
{
struct platform_device *pdev = to_platform_device(port->dev);
struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
- struct tty_struct *tty = port->state->port.tty;
int retval;
/*
@@ -1772,8 +1771,7 @@ static int atmel_startup(struct uart_port *port)
* Allocate the IRQ
*/
retval = request_irq(port->irq, atmel_interrupt,
- IRQF_SHARED | IRQF_COND_SUSPEND,
- tty ? tty->name : "atmel_serial", port);
+ IRQF_SHARED | IRQF_COND_SUSPEND, port->name, port);
if (retval) {
dev_err(port->dev, "atmel_startup - Can't get irq\n");
return retval;
--
2.17.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH] tty/serial: atmel: use port->name as name in request_irq()
From: Richard Genoud @ 2018-05-04 6:35 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Rob Herring, Alexandre Belloni, Peter Hurley, Greg Kroah-Hartman,
linux-serial, Jiri Slaby, tglx, linux-arm-kernel
In-Reply-To: <921729c6-7c3f-11cd-8634-044a5c52f810@sorico.fr>
On 03/05/2018 15:34, Richard Genoud wrote:
> On 03/05/2018 14:44, Sebastian Andrzej Siewior wrote:
>> On 2018-05-03 14:36:03 [+0200], Richard Genoud wrote:
>>> Could resend your patch with Fixes: in the commit message ?
>>
>> will do shortly.
Actually, this fix can only be applied on stable kernels 4.14+, because
uart_port->name was introduced in 4.12 by commit f7048b15900f ("tty:
serial_core: Add name field to uart_port struct")
>>
>>> BTW, I didn't manage to reproduce the behavior you describe, could you
>>> give me your .config and describe a little more how you manage to
>>> trigger this bug ?
>> .config sent offlist.
>> Did you not reproduce this even with the second/debug patch I've sent?
> Nope. (I tried on a sam9g35 with buildroot/busybox)
>
> I have a sama5d3 with a debian as a home server, I'll give it a try.
>
>>
>>> (do you use the console on serial debug ? which board ? )
>> up to date debian sid booting with systemd. After boot completed I do
>> "cat /proc/interrupts" and check for the "gone" string.
>> This is
>> [ 0.000000] OF: fdt:Machine model: SAMA5D3 Xplained
>> [ 0.000000] Kernel command line: earlyprintk=ttyS0,115200 console=ttyS0,115200 root=/dev/mmcblk0p2 rootwait
>> [ 0.520000] AT91: Detected SoC family: sama5d3
>> [ 0.520000] AT91: Detected SoC: sama5d36, revision 2
>>
>> at91-sama5d3_xplained.dtb
>>
>> Is this enough?
>
> Great,
>
> Thanks !
>
^ permalink raw reply
* Re: serial: start_tx & buffer handling
From: Muni Sekhar @ 2018-05-04 4:31 UTC (permalink / raw)
To: Greg KH; +Cc: linux-serial, linux-kernel, kernelnewbies
In-Reply-To: <20180503183408.GA12152@kroah.com>
On Fri, May 4, 2018 at 12:04 AM, Greg KH <greg@kroah.com> wrote:
> On Thu, May 03, 2018 at 08:08:48PM +0530, Muni Sekhar wrote:
>> Hi All,
>>
>> I’m trying to understand how user mode buffer is written to low level
>> serial hardware registers.
>>
>> For this I read the kernel code and I came to know that from user mode
>> write() API lands into kernel’s tty_write() ("drivers/tty/tty_io.c")
>> and then it calls a uart_write() ("drivers/tty/serial/serial_core.c").
>>
>> In uart_write(), the buffer is copied to circ_buf and then it calls
>> low level serial hardware driver’s start_tx() (struct uart_ops
>> .start_tx). But here I could not find how the buffer kept in circ_buf
>> is copied to serial port’s TX_FIFO registers?
>>
>> Can someone take a moment to explain me on this?
>
> It all depends on which specific UART driver you are looking at, they
> all do it a bit different depending on the hardware.
>
> Which one are you looking at? Look at what the start_tx callback does
> for that specific driver, that should give you a hint as to how data
> starts flowing. Usually an interrupt is enabled that is used to flush
> the buffer out to the hardware.
>
I’m looking for any existing sample code which does DMA transfers of
UART transmitted data. I looked at the bcm63xx_uart.c, it looks it
does not handle DMA transfers. Even copying the Tx buffer (from
circ_buf) to UART_FIFO_REG happening in ISR.
> thanks,
>
> greg k-h
--
Thanks,
Sekhar
^ permalink raw reply
* Re: [PATCH] serial: imx: enable IMX21_UCR3_RXDMUXSEL for non-dte-mode
From: Chris Ruehl @ 2018-05-04 3:59 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Uwe Kleine-König, Jiri Slaby, linux-serial, linux-kernel
In-Reply-To: <20180503201317.GA19401@kroah.com>
>>
>> The IMX serial driver is broken in the 4.9.x tree since 4.9.89.
>> Uwe's patch
>>
>> 6df765dca378bddf994cfd2044acafa501bd800f
>>
>> please apply to clear the situation.
>>
>> I know you very busy! -
>
> It's best to email stable@vger.kernel.org for stuff like this, as I
> almost missed it...
>
> Now queued up.
>
> greg k-h
Thanks!
will follow up and cc the stable@ next time.
Chris
^ permalink raw reply
* Re: [PATCH v2] tty: implement led triggers
From: kbuild test robot @ 2018-05-04 0:29 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: kbuild-all, linux-serial, One Thousand Gnomes, Florian Fainelli,
Pavel Machek, Mathieu Poirier, Greg Kroah-Hartman, linux-kernel,
kernel, linux-arm-kernel, Robin Murphy
In-Reply-To: <20180503201952.16592-1-u.kleine-koenig@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 4156 bytes --]
Hi Uwe,
I love your patch! Yet something to improve:
[auto build test ERROR on tty/tty-testing]
[also build test ERROR on v4.17-rc3 next-20180503]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Uwe-Kleine-K-nig/tty-implement-led-triggers/20180504-075232
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
config: i386-randconfig-x014-201817 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/tty/tty_io.c: In function 'do_tty_write':
>> drivers/tty/tty_io.c:962:38: error: 'struct tty_port' has no member named 'led_trigger_tx'
led_trigger_blink_oneshot(tty->port->led_trigger_tx, &delay, &delay, 0);
^~
--
drivers/tty/tty_buffer.c: In function 'flush_to_ldisc':
>> drivers/tty/tty_buffer.c:526:33: error: 'struct tty_port' has no member named 'led_trigger_rx'
led_trigger_blink_oneshot(port->led_trigger_rx, &delay, &delay, 0);
^~
vim +962 drivers/tty/tty_io.c
893
894 /*
895 * Split writes up in sane blocksizes to avoid
896 * denial-of-service type attacks
897 */
898 static inline ssize_t do_tty_write(
899 ssize_t (*write)(struct tty_struct *, struct file *, const unsigned char *, size_t),
900 struct tty_struct *tty,
901 struct file *file,
902 const char __user *buf,
903 size_t count)
904 {
905 ssize_t ret, written = 0;
906 unsigned int chunk;
907
908 ret = tty_write_lock(tty, file->f_flags & O_NDELAY);
909 if (ret < 0)
910 return ret;
911
912 /*
913 * We chunk up writes into a temporary buffer. This
914 * simplifies low-level drivers immensely, since they
915 * don't have locking issues and user mode accesses.
916 *
917 * But if TTY_NO_WRITE_SPLIT is set, we should use a
918 * big chunk-size..
919 *
920 * The default chunk-size is 2kB, because the NTTY
921 * layer has problems with bigger chunks. It will
922 * claim to be able to handle more characters than
923 * it actually does.
924 *
925 * FIXME: This can probably go away now except that 64K chunks
926 * are too likely to fail unless switched to vmalloc...
927 */
928 chunk = 2048;
929 if (test_bit(TTY_NO_WRITE_SPLIT, &tty->flags))
930 chunk = 65536;
931 if (count < chunk)
932 chunk = count;
933
934 /* write_buf/write_cnt is protected by the atomic_write_lock mutex */
935 if (tty->write_cnt < chunk) {
936 unsigned char *buf_chunk;
937
938 if (chunk < 1024)
939 chunk = 1024;
940
941 buf_chunk = kmalloc(chunk, GFP_KERNEL);
942 if (!buf_chunk) {
943 ret = -ENOMEM;
944 goto out;
945 }
946 kfree(tty->write_buf);
947 tty->write_cnt = chunk;
948 tty->write_buf = buf_chunk;
949 }
950
951 /* Do the write .. */
952 for (;;) {
953 size_t size = count;
954 unsigned long delay = 50 /* ms */;
955
956 if (size > chunk)
957 size = chunk;
958 ret = -EFAULT;
959 if (copy_from_user(tty->write_buf, buf, size))
960 break;
961
> 962 led_trigger_blink_oneshot(tty->port->led_trigger_tx, &delay, &delay, 0);
963
964 ret = write(tty, file, tty->write_buf, size);
965 if (ret <= 0)
966 break;
967 written += ret;
968 buf += ret;
969 count -= ret;
970 if (!count)
971 break;
972 ret = -ERESTARTSYS;
973 if (signal_pending(current))
974 break;
975 cond_resched();
976 }
977 if (written) {
978 tty_update_time(&file_inode(file)->i_mtime);
979 ret = written;
980 }
981 out:
982 tty_write_unlock(tty);
983 return ret;
984 }
985
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28533 bytes --]
^ permalink raw reply
* [PATCH v2] tty: implement led triggers
From: Uwe Kleine-König @ 2018-05-03 20:19 UTC (permalink / raw)
To: linux-serial
Cc: One Thousand Gnomes, Florian Fainelli, Pavel Machek,
Mathieu Poirier, Greg Kroah-Hartman, linux-kernel, kernel,
linux-arm-kernel, Robin Murphy
In-Reply-To: <0c1bb915-bd92-4433-61ec-78fdba453396@arm.com>
The rx trigger fires when data is pushed to the ldisc. This is a bit later
than the actual receiving of data but has the nice benefit that it
doesn't need adaption for each driver and isn't in the hot path.
Similarily the tx trigger fires when data taken from the ldisc.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Changes since v1, sent with Message-Id:
20180503100448.1350-1-u.kleine-koenig@pengutronix.de:
- implement tx trigger;
- introduce Kconfig symbol for conditional compilation;
- set trigger to NULL if allocating the name failed to not free random
pointers in case the port struct wasn't zeroed;
- use if/else instead of goto
drivers/tty/Kconfig | 7 +++++++
drivers/tty/tty_buffer.c | 4 ++++
drivers/tty/tty_io.c | 6 ++++++
drivers/tty/tty_port.c | 32 ++++++++++++++++++++++++++++++++
include/linux/tty.h | 7 +++++++
5 files changed, 56 insertions(+)
diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
index 0840d27381ea..07a2fb05439f 100644
--- a/drivers/tty/Kconfig
+++ b/drivers/tty/Kconfig
@@ -41,6 +41,13 @@ config VT
If unsure, say Y, or else you won't be able to do much with your new
shiny Linux system :-)
+config TTY_LEDS_TRIGGER
+ bool "Enable support for TTY actions making LEDs blink"
+ depends on LEDS_TRIGGERS
+ ---help---
+ This enable support for tty triggers. It provides two LED triggers
+ (rx and tx) for each TTY.
+
config CONSOLE_TRANSLATIONS
depends on VT
default y
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index c996b6859c5e..4d364b77b1a7 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -17,6 +17,7 @@
#include <linux/delay.h>
#include <linux/module.h>
#include <linux/ratelimit.h>
+#include <linux/leds.h>
#define MIN_TTYB_SIZE 256
@@ -499,6 +500,7 @@ static void flush_to_ldisc(struct work_struct *work)
struct tty_buffer *head = buf->head;
struct tty_buffer *next;
int count;
+ unsigned long delay = 50 /* ms */;
/* Ldisc or user is trying to gain exclusive access */
if (atomic_read(&buf->priority))
@@ -521,6 +523,8 @@ static void flush_to_ldisc(struct work_struct *work)
continue;
}
+ led_trigger_blink_oneshot(port->led_trigger_rx, &delay, &delay, 0);
+
count = receive_buf(port, head, count);
if (!count)
break;
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 7c838b90a31d..2c840f1e1e82 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -83,6 +83,7 @@
#include <linux/timer.h>
#include <linux/ctype.h>
#include <linux/kd.h>
+#include <linux/leds.h>
#include <linux/mm.h>
#include <linux/string.h>
#include <linux/slab.h>
@@ -950,11 +951,16 @@ static inline ssize_t do_tty_write(
/* Do the write .. */
for (;;) {
size_t size = count;
+ unsigned long delay = 50 /* ms */;
+
if (size > chunk)
size = chunk;
ret = -EFAULT;
if (copy_from_user(tty->write_buf, buf, size))
break;
+
+ led_trigger_blink_oneshot(tty->port->led_trigger_tx, &delay, &delay, 0);
+
ret = write(tty, file, tty->write_buf, size);
if (ret <= 0)
break;
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index 25d736880013..f042879a597c 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -16,6 +16,7 @@
#include <linux/wait.h>
#include <linux/bitops.h>
#include <linux/delay.h>
+#include <linux/leds.h>
#include <linux/module.h>
#include <linux/serdev.h>
@@ -157,6 +158,30 @@ struct device *tty_port_register_device_attr_serdev(struct tty_port *port,
tty_port_link_device(port, driver, index);
+#ifdef CONFIG_TTY_LEDS_TRIGGER
+ port->led_trigger_rx_name = kasprintf(GFP_KERNEL, "%s%d-rx",
+ driver->name, index);
+ if (port->led_trigger_rx_name) {
+ led_trigger_register_simple(port->led_trigger_rx_name,
+ &port->led_trigger_rx);
+ } else {
+ port->led_trigger_rx = NULL;
+ pr_err("Failed to allocate trigger name for %s%d\n",
+ driver->name, index);
+ }
+
+ port->led_trigger_tx_name = kasprintf(GFP_KERNEL, "%s%d-tx",
+ driver->name, index);
+ if (port->led_trigger_tx_name) {
+ led_trigger_register_simple(port->led_trigger_tx_name,
+ &port->led_trigger_tx);
+ } else {
+ port->led_trigger_tx = NULL;
+ pr_err("Failed to allocate trigger name for %s%d\n",
+ driver->name, index);
+ }
+#endif
+
dev = serdev_tty_port_register(port, device, driver, index);
if (PTR_ERR(dev) != -ENODEV) {
/* Skip creating cdev if we registered a serdev device */
@@ -206,6 +231,13 @@ void tty_port_unregister_device(struct tty_port *port,
if (ret == 0)
return;
+#ifdef CONFIG_TTY_LEDS_TRIGGER
+ led_trigger_unregister_simple(port->led_trigger_rx);
+ kfree(port->led_trigger_rx_name);
+ led_trigger_unregister_simple(port->led_trigger_tx);
+ kfree(port->led_trigger_tx_name);
+#endif
+
tty_unregister_device(driver, index);
}
EXPORT_SYMBOL_GPL(tty_port_unregister_device);
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 1dd587ba6d88..b7dc957365b6 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -249,6 +249,13 @@ struct tty_port {
set to size of fifo */
struct kref kref; /* Ref counter */
void *client_data;
+
+#ifdef CONFIG_TTY_LEDS_TRIGGER
+ struct led_trigger *led_trigger_rx;
+ char *led_trigger_rx_name;
+ struct led_trigger *led_trigger_tx;
+ char *led_trigger_tx_name;
+#endif
};
/* tty_port::iflags bits -- use atomic bit ops */
--
2.17.0
^ permalink raw reply related
* Re: [PATCH] serial: imx: enable IMX21_UCR3_RXDMUXSEL for non-dte-mode
From: Greg Kroah-Hartman @ 2018-05-03 20:13 UTC (permalink / raw)
To: Chris Ruehl; +Cc: Uwe Kleine-König, Jiri Slaby, linux-serial, linux-kernel
In-Reply-To: <ca240b4a-aa42-b747-9bab-49b18290ca49@gtsys.com.hk>
On Thu, May 03, 2018 at 02:13:53PM +0800, Chris Ruehl wrote:
> Hi Greg,
>
> On Friday, April 20, 2018 02:13 PM, Uwe Kleine-König wrote:
> > Hello Chris,
> >
> > On Fri, Apr 20, 2018 at 09:07:59AM +0800, Chris Ruehl wrote:
> > > Fix a problem introduced with
> > > commit e61c38d85b73 ("serial: imx: setup DCEDTE early and ensure DCD and RI irqs to be off")
> > > result in non dte-mode imx-uart fail receive data.
> > > By add back IMX21_UCR3_RXDMUXSEL the serial port works as expected.
> > >
> > > Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
> > > ---
> > > drivers/tty/serial/imx.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> > > index 91f3a1a..3d09933 100644
> > > --- a/drivers/tty/serial/imx.c
> > > +++ b/drivers/tty/serial/imx.c
> > > @@ -1391,7 +1391,7 @@ static int imx_uart_startup(struct uart_port *port)
> > > ucr3 = imx_uart_readl(sport, UCR3);
> > > - ucr3 |= UCR3_DTRDEN | UCR3_RI | UCR3_DCD;
> > > + ucr3 |= IMX21_UCR3_RXDMUXSEL | UCR3_DTRDEN | UCR3_RI | UCR3_DCD;
> > > if (sport->dte_mode)
> > > /* disable broken interrupts */
> >
> > Doesn't 6df765dca378bddf994cfd2044acafa501bd800f fix this for you?
> >
> > Best regards
> > Uwe
> >
>
> The IMX serial driver is broken in the 4.9.x tree since 4.9.89.
> Uwe's patch
>
> 6df765dca378bddf994cfd2044acafa501bd800f
>
> please apply to clear the situation.
>
> I know you very busy! -
It's best to email stable@vger.kernel.org for stuff like this, as I
almost missed it...
Now queued up.
greg k-h
^ permalink raw reply
* Re: serial: start_tx & buffer handling
From: Greg KH @ 2018-05-03 18:34 UTC (permalink / raw)
To: Muni Sekhar; +Cc: linux-serial, linux-kernel, kernelnewbies
In-Reply-To: <CAHhAz+iPbNjeHH72Omi7VCUVL_cOUnQjrqbyJKQstUSL6J1hCw@mail.gmail.com>
On Thu, May 03, 2018 at 08:08:48PM +0530, Muni Sekhar wrote:
> Hi All,
>
> I’m trying to understand how user mode buffer is written to low level
> serial hardware registers.
>
> For this I read the kernel code and I came to know that from user mode
> write() API lands into kernel’s tty_write() ("drivers/tty/tty_io.c")
> and then it calls a uart_write() ("drivers/tty/serial/serial_core.c").
>
> In uart_write(), the buffer is copied to circ_buf and then it calls
> low level serial hardware driver’s start_tx() (struct uart_ops
> .start_tx). But here I could not find how the buffer kept in circ_buf
> is copied to serial port’s TX_FIFO registers?
>
> Can someone take a moment to explain me on this?
It all depends on which specific UART driver you are looking at, they
all do it a bit different depending on the hardware.
Which one are you looking at? Look at what the start_tx callback does
for that specific driver, that should give you a hint as to how data
starts flowing. Usually an interrupt is enabled that is used to flush
the buffer out to the hardware.
thanks,
greg k-h
^ permalink raw reply
* Re: serial: custom baud rate
From: Grant Edwards @ 2018-05-03 18:27 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-serial
In-Reply-To: <CAHhAz+hrmLiRJ77cM=CYj+iyH8aUJ64R6FtAwGqqB2pOS0n0aQ@mail.gmail.com>
On 2018-05-03, Muni Sekhar <munisekharrms@gmail.com> wrote:
> If I need to set a custom baud rates(e.g. 14400, 128000, 256000), does
> Linux serial framework has any supporting method?
Sure, use the termios2 structure instead of the termios structure:
#include <linux/termios.h>
struct termios2 t;
ioctl(fd, TCGETS2, &t)
t.c_cflag &= ~CBAUD;
t.c_cflag |= BOTHER;
t.c_ispeed = baud;
t.c_ospeed = baud;
ioctl(fd, TCSETS2, &t)
[Not all devices/drivers support termios2]
--
Grant Edwards grant.b.edwards Yow! Are we live or on
at tape?
gmail.com
^ permalink raw reply
* Re: serial: custom baud rate
From: Theodore Y. Ts'o @ 2018-05-03 17:54 UTC (permalink / raw)
To: Muni Sekhar; +Cc: linux-serial, linux-kernel
In-Reply-To: <CAHhAz+hrmLiRJ77cM=CYj+iyH8aUJ64R6FtAwGqqB2pOS0n0aQ@mail.gmail.com>
On Thu, May 03, 2018 at 06:09:13PM +0530, Muni Sekhar wrote:
> Hi All,
>
> From include/asm-generic/termbits.h , I see baudrate can be one of the
> standard values: 50, 75, 110, 134, 150, 200, 300, 600, 1200, 1800,
> 2400, 4800, 9600, 19200, 38400, 57600, 115200, 230400, 460800, 500000,
> 576000, 921600, 1000000, 1152000, 1500000, 2000000, 2500000, 3000000,
> 3500000, 4000000.
>
> If I need to set a custom baud rates(e.g. 14400, 128000, 256000), does
> Linux serial framework has any supporting method?
See the setserial man page:t
https://linux.die.net/man/8/setserial
Not all serial devices support the spd_cust and divisor, however. In
general, only devices where the kernel directly programs the
8250/16450/16550 UART directly will support this feature.
- Ted
^ permalink raw reply
* Re: [PATCH] tty/serial: atmel: use port->name as name in request_irq()
From: Richard Genoud @ 2018-05-03 15:12 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Rob Herring, Alexandre Belloni, Peter Hurley, Greg Kroah-Hartman,
linux-serial, Jiri Slaby, tglx, linux-arm-kernel
In-Reply-To: <921729c6-7c3f-11cd-8634-044a5c52f810@sorico.fr>
On 03/05/2018 15:34, Richard Genoud wrote:
> On 03/05/2018 14:44, Sebastian Andrzej Siewior wrote:
>> On 2018-05-03 14:36:03 [+0200], Richard Genoud wrote:
>>> Could resend your patch with Fixes: in the commit message ?
>>
>> will do shortly.
>>
>>> BTW, I didn't manage to reproduce the behavior you describe, could you
>>> give me your .config and describe a little more how you manage to
>>> trigger this bug ?
>> .config sent offlist.
>> Did you not reproduce this even with the second/debug patch I've sent?
> Nope. (I tried on a sam9g35 with buildroot/busybox)
>
> I have a sama5d3 with a debian as a home server, I'll give it a try.
Ok, I reproduced the use-after-free on a SAMA5D2 and on a SAMA5D3, both
with debian (sid / stretch).
After the patch, no mode funny things in /proc/interrupts.
>
>>
>>> (do you use the console on serial debug ? which board ? )
>> up to date debian sid booting with systemd. After boot completed I do
>> "cat /proc/interrupts" and check for the "gone" string.
>> This is
>> [ 0.000000] OF: fdt:Machine model: SAMA5D3 Xplained
>> [ 0.000000] Kernel command line: earlyprintk=ttyS0,115200 console=ttyS0,115200 root=/dev/mmcblk0p2 rootwait
>> [ 0.520000] AT91: Detected SoC family: sama5d3
>> [ 0.520000] AT91: Detected SoC: sama5d36, revision 2
>>
>> at91-sama5d3_xplained.dtb
>>
>> Is this enough?
>
> Great,
>
> Thanks !
>
^ permalink raw reply
* serial: start_tx & buffer handling
From: Muni Sekhar @ 2018-05-03 14:38 UTC (permalink / raw)
To: linux-serial, linux-kernel, kernelnewbies
Hi All,
I’m trying to understand how user mode buffer is written to low level
serial hardware registers.
For this I read the kernel code and I came to know that from user mode
write() API lands into kernel’s tty_write() ("drivers/tty/tty_io.c")
and then it calls a uart_write() ("drivers/tty/serial/serial_core.c").
In uart_write(), the buffer is copied to circ_buf and then it calls
low level serial hardware driver’s start_tx() (struct uart_ops
.start_tx). But here I could not find how the buffer kept in circ_buf
is copied to serial port’s TX_FIFO registers?
Can someone take a moment to explain me on this?
--
Thanks,
Sekhar
^ permalink raw reply
* Re: [PATCH] serial: 8250: omap: Fix idling of clocks for unused uarts
From: Tony Lindgren @ 2018-05-03 14:31 UTC (permalink / raw)
To: Vignesh R
Cc: Peter Hurley, Greg Kroah-Hartman, Peter Ujfalusi,
Sebastian Andrzej Siewior, linux-serial, linux-omap, linux-kernel,
Keerthy, Matthijs van Duin, Sekhar Nori, Tero Kristo
In-Reply-To: <58bcba76-d63b-212e-4baf-d40a28c549d3@ti.com>
* Vignesh R <vigneshr@ti.com> [180503 10:20]:
>
>
> On Wednesday 02 May 2018 10:45 PM, Tony Lindgren wrote:
> > I noticed that unused UARTs won't necessarily idle properly always
> > unless at least one byte tx transfer is done first.
> >
> > After some debugging I narrowed down the problem to the scr register
> > dma configuration bits that need to be set before softreset for the
> > clocks to idle. Unless we do this, the module clkctrl idlest bits
> > may be set to 1 instead of 3 meaning the clock will never idle and
> > is blocking deeper idle states for the whole domain.
> >
> > This might be related to the configuration done by the bootloader
> > or kexec booting where certain configurations cause the 8250 or
> > the clkctrl clock to jam in a way where setting of the scr bits
> > and reset is needed to clear it. I've tried diffing the 8250
> > registers for the various modes, but did not see anything specific.
> > So far I've only seen this on omap4 but I'm suspecting this might
> > also happen on the other clkctrl using SoCs considering they
> > already have a quirk enabled for UART_ERRATA_CLOCK_DISABLE.
> >
>
> That's interesting! We do have AM437x suspend/resume working without
> this workaround (UARTs on AM437x does not use DMA) and UART IPs clkctrl
> do go to idle state. Seems like a OMAP4 specific issue.
Yeah seems to be omap4 specific. I did not see this yesterday
on my am437x-idk or beagle-x15 after clearing status = "disabled"
for all ports and idling them.
> Comment in omap8250_update_scr() warns not to set these two bits in a
> single register write because this may lead to malfunction. I would
> recommend to split this into two writes.
OK I'll split the write into two parts.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH] tty/serial: atmel: use port->name as name in request_irq()
From: Richard Genoud @ 2018-05-03 13:34 UTC (permalink / raw)
To: Sebastian Andrzej Siewior, Richard Genoud
Cc: Rob Herring, Alexandre Belloni, Peter Hurley, Greg Kroah-Hartman,
linux-serial, Jiri Slaby, tglx, linux-arm-kernel
In-Reply-To: <20180503124410.n6ze2ngwv6ekjhdr@linutronix.de>
On 03/05/2018 14:44, Sebastian Andrzej Siewior wrote:
> On 2018-05-03 14:36:03 [+0200], Richard Genoud wrote:
>> Could resend your patch with Fixes: in the commit message ?
>
> will do shortly.
>
>> BTW, I didn't manage to reproduce the behavior you describe, could you
>> give me your .config and describe a little more how you manage to
>> trigger this bug ?
> .config sent offlist.
> Did you not reproduce this even with the second/debug patch I've sent?
Nope. (I tried on a sam9g35 with buildroot/busybox)
I have a sama5d3 with a debian as a home server, I'll give it a try.
>
>> (do you use the console on serial debug ? which board ? )
> up to date debian sid booting with systemd. After boot completed I do
> "cat /proc/interrupts" and check for the "gone" string.
> This is
> [ 0.000000] OF: fdt:Machine model: SAMA5D3 Xplained
> [ 0.000000] Kernel command line: earlyprintk=ttyS0,115200 console=ttyS0,115200 root=/dev/mmcblk0p2 rootwait
> [ 0.520000] AT91: Detected SoC family: sama5d3
> [ 0.520000] AT91: Detected SoC: sama5d36, revision 2
>
> at91-sama5d3_xplained.dtb
>
> Is this enough?
Great,
Thanks !
^ permalink raw reply
* Re: [PATCH v3 09/20] iommu: Remove depends on HAS_DMA in case of platform dependency
From: Joerg Roedel @ 2018-05-03 13:02 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Ulf Hansson, Wolfram Sang, linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-fpga-u79uwXL29TY76Z2rM5mHXA,
linux-remoteproc-u79uwXL29TY76Z2rM5mHXA,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Bjorn Andersson, Eric Anholt,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux1394-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
Christoph Hellwig, Stefan Wahren, Boris Brezillon, Herbert Xu,
Richard Weinberger, Jassi Brar, Marek Vasut,
linux-serial-u79uwXL29TY76Z2rM5mHXA, Matias Bjorling,
linux-media-u79uwXL29TY76Z2rM5mHXA, Ohad Ben-Cohen,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b, Alan Tull
In-Reply-To: <1523987360-18760-10-git-send-email-geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
On Tue, Apr 17, 2018 at 07:49:09PM +0200, Geert Uytterhoeven wrote:
> Remove dependencies on HAS_DMA where a Kconfig symbol depends on another
> symbol that implies HAS_DMA, and, optionally, on "|| COMPILE_TEST".
> In most cases this other symbol is an architecture or platform specific
> symbol, or PCI.
>
> Generic symbols and drivers without platform dependencies keep their
> dependencies on HAS_DMA, to prevent compiling subsystems or drivers that
> cannot work anyway.
>
> This simplifies the dependencies, and allows to improve compile-testing.
>
> Signed-off-by: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
> Reviewed-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Acked-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
> Acked-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] tty/serial: atmel: use port->name as name in request_irq()
From: Sebastian Andrzej Siewior @ 2018-05-03 12:44 UTC (permalink / raw)
To: Richard Genoud
Cc: Rob Herring, Alexandre Belloni, Peter Hurley, Greg Kroah-Hartman,
linux-serial, Jiri Slaby, tglx, linux-arm-kernel
In-Reply-To: <4f3f6e7c-2806-1749-1523-72d5c8eb8229@sorico.fr>
On 2018-05-03 14:36:03 [+0200], Richard Genoud wrote:
> Could resend your patch with Fixes: in the commit message ?
will do shortly.
> BTW, I didn't manage to reproduce the behavior you describe, could you
> give me your .config and describe a little more how you manage to
> trigger this bug ?
.config sent offlist.
Did you not reproduce this even with the second/debug patch I've sent?
> (do you use the console on serial debug ? which board ? )
up to date debian sid booting with systemd. After boot completed I do
"cat /proc/interrupts" and check for the "gone" string.
This is
[ 0.000000] OF: fdt:Machine model: SAMA5D3 Xplained
[ 0.000000] Kernel command line: earlyprintk=ttyS0,115200 console=ttyS0,115200 root=/dev/mmcblk0p2 rootwait
[ 0.520000] AT91: Detected SoC family: sama5d3
[ 0.520000] AT91: Detected SoC: sama5d36, revision 2
at91-sama5d3_xplained.dtb
Is this enough?
> Thanks !
Sebastian
^ permalink raw reply
* serial: custom baud rate
From: Muni Sekhar @ 2018-05-03 12:39 UTC (permalink / raw)
To: linux-serial, linux-kernel
Hi All,
>From include/asm-generic/termbits.h , I see baudrate can be one of the
standard values: 50, 75, 110, 134, 150, 200, 300, 600, 1200, 1800,
2400, 4800, 9600, 19200, 38400, 57600, 115200, 230400, 460800, 500000,
576000, 921600, 1000000, 1152000, 1500000, 2000000, 2500000, 3000000,
3500000, 4000000.
If I need to set a custom baud rates(e.g. 14400, 128000, 256000), does
Linux serial framework has any supporting method?
--
Thanks,
Sekhar
^ permalink raw reply
* Re: [PATCH] tty/serial: atmel: use port->name as name in request_irq()
From: Richard Genoud @ 2018-05-03 12:36 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Rob Herring, Alexandre Belloni, Peter Hurley, Greg Kroah-Hartman,
linux-serial, Jiri Slaby, tglx, linux-arm-kernel
In-Reply-To: <20180502191632.tgda4g5v3rhncx7c@linutronix.de>
On 02/05/2018 21:16, Sebastian Andrzej Siewior wrote:
> On 2018-04-27 12:31:52 [+0200], Richard Genoud wrote:
>> Hi Sebastian,
> Hi,
>
>>> If this is unknown and a bisect is requested, please let me know.
>> Indeed, this will be appreciated.
>> I'm quite curious to find the commit that led to this.
>
> commit 761ed4a94582ab291aa24dcbea4e01e8936488c8
> Author: Rob Herring <robh@kernel.org>
> Date: Mon Aug 22 17:39:10 2016 -0500
>
> tty: serial_core: convert uart_close to use tty_port_close
>
> tty_port_close handles much of the common parts of tty close. Convert
> uart_close to use it and move the serial_core specific parts into
> tty_port.shutdown function. This will be needed to use tty_port functions
> directly from in kernel clients.
>
> This change causes ops->stop_rx() to be called after uart_wait_until_sent()
> is called which I think should be fine. Otherwise, the sequence of the
> close should be the same.
>
> Cc: Peter Hurley <peter@hurleysoftware.com>
> Signed-off-by: Rob Herring <robh@kernel.org>
> Acked-by: Alan Cox <alan@linux.intel.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> The thread starts at
> https://www.spinics.net/lists/linux-serial/msg30070.html
> http://lkml.kernel.org/r/20180426150625.q5tqcb7fzchvkb5d@linutronix.de
Thanks for this hunting session !
Could resend your patch with Fixes: in the commit message ?
BTW, I didn't manage to reproduce the behavior you describe, could you
give me your .config and describe a little more how you manage to
trigger this bug ?
(do you use the console on serial debug ? which board ? )
Thanks !
^ permalink raw reply
* Re: [PATCH] tty: implement a rx led trigger
From: Robin Murphy @ 2018-05-03 12:33 UTC (permalink / raw)
To: Uwe Kleine-König, linux-serial
Cc: One Thousand Gnomes, Florian Fainelli, Pavel Machek,
Mathieu Poirier, Greg Kroah-Hartman, linux-kernel, kernel,
linux-arm-kernel
In-Reply-To: <20180503100448.1350-1-u.kleine-koenig@pengutronix.de>
On 03/05/18 11:04, Uwe Kleine-König wrote:
[...]
> @@ -157,6 +158,18 @@ struct device *tty_port_register_device_attr_serdev(struct tty_port *port,
>
> tty_port_link_device(port, driver, index);
>
> + port->led_trigger_rx_name = kasprintf(GFP_KERNEL, "%s%d-rx",
> + driver->name, index);
> + if (!port->led_trigger_rx_name) {
> + pr_err("Failed to allocate trigger name for %s%d\n",
> + driver->name, index);
> + goto skip_trigger;
Hmm, isn't that a rather awkward way to spell "else"? ;)
Robin.
> + }
> +
> + led_trigger_register_simple(port->led_trigger_rx_name,
> + &port->led_trigger_rx);
> +
> +skip_trigger:
> dev = serdev_tty_port_register(port, device, driver, index);
> if (PTR_ERR(dev) != -ENODEV) {
> /* Skip creating cdev if we registered a serdev device */
^ permalink raw reply
* Re: [PATCH] tty: implement a rx led trigger
From: Uwe Kleine-König @ 2018-05-03 11:52 UTC (permalink / raw)
To: Pavel Machek
Cc: One Thousand Gnomes, Florian Fainelli, linux-serial,
Mathieu Poirier, Greg Kroah-Hartman, linux-kernel, kernel,
linux-arm-kernel
In-Reply-To: <20180503101047.GF32180@amd>
On Thu, May 03, 2018 at 12:10:47PM +0200, Pavel Machek wrote:
> Hi!
>
> > The trigger fires when data is pushed to the ldisc. This is a bit later
> > than the actual receiving of data but has the nice benefit that it
> > doesn't need adaption for each driver and isn't in the hot path.
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>
> Well, why not, but there certainly should be config option? Because I
> really don't see many people wanting to use this trigger.
Yeah, can be done. I'll implement this for v2.
> > diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
> > index c996b6859c5e..4d364b77b1a7 100644
> > --- a/drivers/tty/tty_buffer.c
> > +++ b/drivers/tty/tty_buffer.c
> > @@ -17,6 +17,7 @@
> > #include <linux/delay.h>
> > #include <linux/module.h>
> > #include <linux/ratelimit.h>
> > +#include <linux/leds.h>
> >
> >
> > #define MIN_TTYB_SIZE 256
> > @@ -499,6 +500,7 @@ static void flush_to_ldisc(struct work_struct *work)
> > struct tty_buffer *head = buf->head;
> > struct tty_buffer *next;
> > int count;
> > + unsigned long delay = 50 /* ms */;
> >
> > /* Ldisc or user is trying to gain exclusive access */
> > if (atomic_read(&buf->priority))
> > @@ -521,6 +523,8 @@ static void flush_to_ldisc(struct work_struct *work)
> > continue;
> > }
> >
> > + led_trigger_blink_oneshot(port->led_trigger_rx, &delay, &delay, 0);
> > +
> > count = receive_buf(port, head, count);
> > if (!count)
> > break;
> > diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
> > index 25d736880013..51b78a585417 100644
> > --- a/drivers/tty/tty_port.c
> > +++ b/drivers/tty/tty_port.c
> > @@ -16,6 +16,7 @@
> > #include <linux/wait.h>
> > #include <linux/bitops.h>
> > #include <linux/delay.h>
> > +#include <linux/leds.h>
> > #include <linux/module.h>
> > #include <linux/serdev.h>
> >
> > @@ -157,6 +158,18 @@ struct device *tty_port_register_device_attr_serdev(struct tty_port *port,
> >
> > tty_port_link_device(port, driver, index);
> >
> > + port->led_trigger_rx_name = kasprintf(GFP_KERNEL, "%s%d-rx",
> > + driver->name, index);
> > + if (!port->led_trigger_rx_name) {
> > + pr_err("Failed to allocate trigger name for %s%d\n",
> > + driver->name, index);
> > + goto skip_trigger;
> > + }
> > +
> > + led_trigger_register_simple(port->led_trigger_rx_name,
> > + &port->led_trigger_rx);
> > +
> > +skip_trigger:
> > dev = serdev_tty_port_register(port, device, driver, index);
> > if (PTR_ERR(dev) != -ENODEV) {
> > /* Skip creating cdev if we registered a serdev device */
> > @@ -206,6 +219,9 @@ void tty_port_unregister_device(struct tty_port *port,
> > if (ret == 0)
> > return;
> >
> > + led_trigger_unregister_simple(port->led_trigger_rx);
>
> Is it ok to unregister if it was not registered in the first place?
led_trigger_unregister_simple() looks as follows:
if (trig)
led_trigger_unregister(trig);
kfree(trig);
So yes, it does the right thing when it wasn't registered. Assuming it
is NULL then, which doesn't seem to be guaranteed. Probably I'd need
port->led_trigger_rx = NULL
if kasprintf failed.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH] serial: 8250: omap: Fix idling of clocks for unused uarts
From: Vignesh R @ 2018-05-03 10:18 UTC (permalink / raw)
To: Tony Lindgren, Peter Hurley, Greg Kroah-Hartman
Cc: Peter Ujfalusi, Sebastian Andrzej Siewior, linux-serial,
linux-omap, linux-kernel, Keerthy, Matthijs van Duin, Sekhar Nori,
Tero Kristo
In-Reply-To: <20180502171500.60462-1-tony@atomide.com>
On Wednesday 02 May 2018 10:45 PM, Tony Lindgren wrote:
> I noticed that unused UARTs won't necessarily idle properly always
> unless at least one byte tx transfer is done first.
>
> After some debugging I narrowed down the problem to the scr register
> dma configuration bits that need to be set before softreset for the
> clocks to idle. Unless we do this, the module clkctrl idlest bits
> may be set to 1 instead of 3 meaning the clock will never idle and
> is blocking deeper idle states for the whole domain.
>
> This might be related to the configuration done by the bootloader
> or kexec booting where certain configurations cause the 8250 or
> the clkctrl clock to jam in a way where setting of the scr bits
> and reset is needed to clear it. I've tried diffing the 8250
> registers for the various modes, but did not see anything specific.
> So far I've only seen this on omap4 but I'm suspecting this might
> also happen on the other clkctrl using SoCs considering they
> already have a quirk enabled for UART_ERRATA_CLOCK_DISABLE.
>
That's interesting! We do have AM437x suspend/resume working without
this workaround (UARTs on AM437x does not use DMA) and UART IPs clkctrl
do go to idle state. Seems like a OMAP4 specific issue.
> Let's fix the issue by configuring scr before reset for basic dma
> even if we don't use it. The scr register will be reset when we do
> softreset few lines after, and we restore scr on resume. We should
> do this for all the SoCs with UART_ERRATA_CLOCK_DISABLE quirk flag
> set since the ones with UART_ERRATA_CLOCK_DISABLE are all based
> using clkctrl similar to omap4.
>
> Looks like both OMAP_UART_SCR_DMAMODE_1 | OMAP_UART_SCR_DMAMODE_CTL
> bits are needed for the clkctrl to idle after a softreset.
>
> And we need to add omap4 to also use the UART_ERRATA_CLOCK_DISABLE
> for the related workaround to be enabled. This same compatible
> value will also be used for omap5.
>
> Fixes: cdb929e4452a ("serial: 8250_omap: workaround errata around
> idling UART after using DMA")
> Cc: Keerthy <j-keerthy@ti.com>
> Cc: Matthijs van Duin <matthijsvanduin@gmail.com>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> drivers/tty/serial/8250/8250_omap.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -1110,13 +1110,14 @@ static int omap8250_no_handle_irq(struct uart_port *port)
> return 0;
> }
>
> +static const u8 omap4_habit = UART_ERRATA_CLOCK_DISABLE;
> static const u8 am3352_habit = OMAP_DMA_TX_KICK | UART_ERRATA_CLOCK_DISABLE;
> static const u8 dra742_habit = UART_ERRATA_CLOCK_DISABLE;
>
> static const struct of_device_id omap8250_dt_ids[] = {
> { .compatible = "ti,omap2-uart" },
> { .compatible = "ti,omap3-uart" },
> - { .compatible = "ti,omap4-uart" },
> + { .compatible = "ti,omap4-uart", .data = &omap4_habit, },
> { .compatible = "ti,am3352-uart", .data = &am3352_habit, },
> { .compatible = "ti,am4372-uart", .data = &am3352_habit, },
> { .compatible = "ti,dra742-uart", .data = &dra742_habit, },
> @@ -1362,6 +1363,16 @@ static int omap8250_soft_reset(struct device *dev)
> int sysc;
> int syss;
>
> + /*
> + * At least on omap4, unused uarts may not idle after reset without
> + * a basic scr dma configuration even with no dma in use. The
> + * module clkctrl status bit will stay set blocking idle for the
> + * whole clockdomain. The softreset below will clear scr, and we
> + * restore it on resume so this is safe to do on all SoCs needing
> + * omap8250_soft_reset() quirk.
> + */
> + serial_out(up, UART_OMAP_SCR,
> + OMAP_UART_SCR_DMAMODE_1 | OMAP_UART_SCR_DMAMODE_CTL);
Comment in omap8250_update_scr() warns not to set these two bits in a
single register write because this may lead to malfunction. I would
recommend to split this into two writes.
> sysc = serial_in(up, UART_OMAP_SYSC);
>
> /* softreset the UART */
>
--
Regards
Vignesh
^ permalink raw reply
* Re: [PATCH] tty: implement a rx led trigger
From: Pavel Machek @ 2018-05-03 10:10 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: linux-serial, kernel, Greg Kroah-Hartman, linux-kernel,
linux-arm-kernel, One Thousand Gnomes, Florian Fainelli,
Mathieu Poirier
In-Reply-To: <20180503100448.1350-1-u.kleine-koenig@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 2835 bytes --]
Hi!
> The trigger fires when data is pushed to the ldisc. This is a bit later
> than the actual receiving of data but has the nice benefit that it
> doesn't need adaption for each driver and isn't in the hot path.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Well, why not, but there certainly should be config option? Because I
really don't see many people wanting to use this trigger.
> diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
> index c996b6859c5e..4d364b77b1a7 100644
> --- a/drivers/tty/tty_buffer.c
> +++ b/drivers/tty/tty_buffer.c
> @@ -17,6 +17,7 @@
> #include <linux/delay.h>
> #include <linux/module.h>
> #include <linux/ratelimit.h>
> +#include <linux/leds.h>
>
>
> #define MIN_TTYB_SIZE 256
> @@ -499,6 +500,7 @@ static void flush_to_ldisc(struct work_struct *work)
> struct tty_buffer *head = buf->head;
> struct tty_buffer *next;
> int count;
> + unsigned long delay = 50 /* ms */;
>
> /* Ldisc or user is trying to gain exclusive access */
> if (atomic_read(&buf->priority))
> @@ -521,6 +523,8 @@ static void flush_to_ldisc(struct work_struct *work)
> continue;
> }
>
> + led_trigger_blink_oneshot(port->led_trigger_rx, &delay, &delay, 0);
> +
> count = receive_buf(port, head, count);
> if (!count)
> break;
> diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
> index 25d736880013..51b78a585417 100644
> --- a/drivers/tty/tty_port.c
> +++ b/drivers/tty/tty_port.c
> @@ -16,6 +16,7 @@
> #include <linux/wait.h>
> #include <linux/bitops.h>
> #include <linux/delay.h>
> +#include <linux/leds.h>
> #include <linux/module.h>
> #include <linux/serdev.h>
>
> @@ -157,6 +158,18 @@ struct device *tty_port_register_device_attr_serdev(struct tty_port *port,
>
> tty_port_link_device(port, driver, index);
>
> + port->led_trigger_rx_name = kasprintf(GFP_KERNEL, "%s%d-rx",
> + driver->name, index);
> + if (!port->led_trigger_rx_name) {
> + pr_err("Failed to allocate trigger name for %s%d\n",
> + driver->name, index);
> + goto skip_trigger;
> + }
> +
> + led_trigger_register_simple(port->led_trigger_rx_name,
> + &port->led_trigger_rx);
> +
> +skip_trigger:
> dev = serdev_tty_port_register(port, device, driver, index);
> if (PTR_ERR(dev) != -ENODEV) {
> /* Skip creating cdev if we registered a serdev device */
> @@ -206,6 +219,9 @@ void tty_port_unregister_device(struct tty_port *port,
> if (ret == 0)
> return;
>
> + led_trigger_unregister_simple(port->led_trigger_rx);
Is it ok to unregister if it was not registered in the first place?
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* [PATCH] tty: implement a rx led trigger
From: Uwe Kleine-König @ 2018-05-03 10:04 UTC (permalink / raw)
To: linux-serial
Cc: kernel, Greg Kroah-Hartman, linux-kernel, linux-arm-kernel,
One Thousand Gnomes, Florian Fainelli, Pavel Machek,
Mathieu Poirier
The trigger fires when data is pushed to the ldisc. This is a bit later
than the actual receiving of data but has the nice benefit that it
doesn't need adaption for each driver and isn't in the hot path.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,
this is actually v2 of the series "serial: Add LED trigger support" that
Sascha sent in November 2016.
For now this is only the RX part however that I'd like to use as a base
to get feedback. Once we got to an acceptable change the TX part should
be easy (once the right place to place the trigger is clear).
Other than that the trigger moved from a dedicated function in
serial_core that each driver is supposed to call on reception of data to
flush_to_ldisc().
Best regards
Uwe
drivers/tty/tty_buffer.c | 4 ++++
drivers/tty/tty_port.c | 16 ++++++++++++++++
include/linux/tty.h | 3 +++
3 files changed, 23 insertions(+)
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index c996b6859c5e..4d364b77b1a7 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -17,6 +17,7 @@
#include <linux/delay.h>
#include <linux/module.h>
#include <linux/ratelimit.h>
+#include <linux/leds.h>
#define MIN_TTYB_SIZE 256
@@ -499,6 +500,7 @@ static void flush_to_ldisc(struct work_struct *work)
struct tty_buffer *head = buf->head;
struct tty_buffer *next;
int count;
+ unsigned long delay = 50 /* ms */;
/* Ldisc or user is trying to gain exclusive access */
if (atomic_read(&buf->priority))
@@ -521,6 +523,8 @@ static void flush_to_ldisc(struct work_struct *work)
continue;
}
+ led_trigger_blink_oneshot(port->led_trigger_rx, &delay, &delay, 0);
+
count = receive_buf(port, head, count);
if (!count)
break;
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index 25d736880013..51b78a585417 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -16,6 +16,7 @@
#include <linux/wait.h>
#include <linux/bitops.h>
#include <linux/delay.h>
+#include <linux/leds.h>
#include <linux/module.h>
#include <linux/serdev.h>
@@ -157,6 +158,18 @@ struct device *tty_port_register_device_attr_serdev(struct tty_port *port,
tty_port_link_device(port, driver, index);
+ port->led_trigger_rx_name = kasprintf(GFP_KERNEL, "%s%d-rx",
+ driver->name, index);
+ if (!port->led_trigger_rx_name) {
+ pr_err("Failed to allocate trigger name for %s%d\n",
+ driver->name, index);
+ goto skip_trigger;
+ }
+
+ led_trigger_register_simple(port->led_trigger_rx_name,
+ &port->led_trigger_rx);
+
+skip_trigger:
dev = serdev_tty_port_register(port, device, driver, index);
if (PTR_ERR(dev) != -ENODEV) {
/* Skip creating cdev if we registered a serdev device */
@@ -206,6 +219,9 @@ void tty_port_unregister_device(struct tty_port *port,
if (ret == 0)
return;
+ led_trigger_unregister_simple(port->led_trigger_rx);
+ kfree(port->led_trigger_rx_name);
+
tty_unregister_device(driver, index);
}
EXPORT_SYMBOL_GPL(tty_port_unregister_device);
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 1dd587ba6d88..7e36ee1d2056 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -249,6 +249,9 @@ struct tty_port {
set to size of fifo */
struct kref kref; /* Ref counter */
void *client_data;
+
+ struct led_trigger *led_trigger_rx;
+ char *led_trigger_rx_name;
};
/* tty_port::iflags bits -- use atomic bit ops */
--
2.17.0
^ permalink raw reply related
* Re: [PATCH] serial: sh-sci: Use spin_{try}lock_irqsave instead of open coding version
From: Geert Uytterhoeven @ 2018-05-03 7:43 UTC (permalink / raw)
To: Daniel Wagner
Cc: Linux Kernel Mailing List, linux-rt-users,
open list:SERIAL DRIVERS, Greg Kroah-Hartman, Linux-sh list,
Daniel Wagner, Shinya Kuribayashi
In-Reply-To: <20180430080918.16763-1-wagi@monom.org>
Hi Daniel,
On Mon, Apr 30, 2018 at 10:09 AM, Daniel Wagner <wagi@monom.org> wrote:
> From: Daniel Wagner <daniel.wagner@siemens.com>
>
> Commit 40f70c03e33a ("serial: sh-sci: add locking to console write
> function to avoid SMP lockup") copied the strategy to avoid locking
> problems in conjuncture with the console from the UART8250
> driver. Instead using directly spin_{try}lock_irqsave(),
> local_irq_save() followed by spin_{try}lock() was used. While this is
> correct on mainline, for -rt it is a problem. spin_{try}lock() will
> check if it is running in a valid context. Since the local_irq_save()
> has already been executed, the context has changed and
> spin_{try}lock() will complain. The reason why spin_{try}lock()
> complains is that on -rt the spin locks are turned into mutexes and
> therefore can sleep. Sleeping with interrupts disabled is not valid.
> Cc: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
> Signed-off-by: Daniel Wagner <daniel.wagner@siemens.com>
Thanks for your patch!
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -2516,13 +2516,12 @@ static void serial_console_write(struct console *co, const char *s,
> unsigned long flags;
> int locked = 1;
>
> - local_irq_save(flags);
Hence the below now runs with local interrupts enabled.
For checking port->sysrq or oops_in_progress that probably isn't an issue.
If oops_in_progress is set, you have other problems, and the race condition
between checking the flag and calling spin_lock{,_irqsave}() existed before,
and is hard to avoid.
For actual console printing, I think you want to keep interrupts disabled.
> if (port->sysrq)
> locked = 0;
> else if (oops_in_progress)
> - locked = spin_trylock(&port->lock);
> + locked = spin_trylock_irqsave(&port->lock, flags);
> else
> - spin_lock(&port->lock);
> + spin_lock_irqsave(&port->lock, flags);
Add
if (!locked
local_irq_save(flags)
here?
> /* first save the SCSCR then disable the interrupts */
> ctrl = serial_port_in(port, SCSCR);
> @@ -2539,8 +2538,7 @@ static void serial_console_write(struct console *co, const char *s,
> serial_port_out(port, SCSCR, ctrl);
>
> if (locked)
> - spin_unlock(&port->lock);
> - local_irq_restore(flags);
> + spin_unlock_irqrestore(&port->lock, flags);
else
local_irq_restore(flags);
> }
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] serial: imx: enable IMX21_UCR3_RXDMUXSEL for non-dte-mode
From: Chris Ruehl @ 2018-05-03 6:13 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Uwe Kleine-König, Jiri Slaby, linux-serial, linux-kernel
In-Reply-To: <20180420061316.sakoupltqxn2qhfr@pengutronix.de>
Hi Greg,
On Friday, April 20, 2018 02:13 PM, Uwe Kleine-König wrote:
> Hello Chris,
>
> On Fri, Apr 20, 2018 at 09:07:59AM +0800, Chris Ruehl wrote:
>> Fix a problem introduced with
>> commit e61c38d85b73 ("serial: imx: setup DCEDTE early and ensure DCD and RI irqs to be off")
>> result in non dte-mode imx-uart fail receive data.
>> By add back IMX21_UCR3_RXDMUXSEL the serial port works as expected.
>>
>> Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
>> ---
>> drivers/tty/serial/imx.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
>> index 91f3a1a..3d09933 100644
>> --- a/drivers/tty/serial/imx.c
>> +++ b/drivers/tty/serial/imx.c
>> @@ -1391,7 +1391,7 @@ static int imx_uart_startup(struct uart_port *port)
>>
>> ucr3 = imx_uart_readl(sport, UCR3);
>>
>> - ucr3 |= UCR3_DTRDEN | UCR3_RI | UCR3_DCD;
>> + ucr3 |= IMX21_UCR3_RXDMUXSEL | UCR3_DTRDEN | UCR3_RI | UCR3_DCD;
>>
>> if (sport->dte_mode)
>> /* disable broken interrupts */
>
> Doesn't 6df765dca378bddf994cfd2044acafa501bd800f fix this for you?
>
> Best regards
> Uwe
>
The IMX serial driver is broken in the 4.9.x tree since 4.9.89.
Uwe's patch
6df765dca378bddf994cfd2044acafa501bd800f
please apply to clear the situation.
I know you very busy! -
Chris
^ 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