[Qemu-devel] [PATCH] i.MX: Support serial RS-232 break properly

Trent Piepho posted 1 patch 7 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180320013657.25038-1-tpiepho@impinj.com
Test checkpatch failed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test docker-quick@centos6 passed
Test s390x passed
hw/char/imx_serial.c         | 5 ++++-
include/hw/char/imx_serial.h | 1 +
2 files changed, 5 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH] i.MX: Support serial RS-232 break properly
Posted by Trent Piepho 7 years, 10 months ago
Linux does not detect a break from this IMX serial driver as a magic
sysrq.  Nor does it note a break in the port error counts.

The former is because the Linux driver uses the BRCD bit in the USR2
register to trigger the RS-232 break handler in the kernel, which is
where sysrq hooks in.  The emulated UART was not setting this status
bit.

The latter is because the Linux driver expects, in addition to the BRK
bit, that the ERR bit is set when a break is read in the FIFO.  A break
should also count as a frame error, so add that bit too.

Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
Signed-off-by: Trent Piepho <tpiepho@impinj.com>
---
 hw/char/imx_serial.c         | 5 ++++-
 include/hw/char/imx_serial.h | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c
index 70405ccf8b..4ca821d74c 100644
--- a/hw/char/imx_serial.c
+++ b/hw/char/imx_serial.c
@@ -286,6 +286,9 @@ static void imx_put_data(void *opaque, uint32_t value)
     s->usr2 |= USR2_RDR;
     s->uts1 &= ~UTS1_RXEMPTY;
     s->readbuff = value;
+    if (value & URXD_BRK) {
+        s->usr2 |= USR2_BRCD;
+    }
     imx_update(s);
 }
 
@@ -297,7 +300,7 @@ static void imx_receive(void *opaque, const uint8_t *buf, int size)
 static void imx_event(void *opaque, int event)
 {
     if (event == CHR_EVENT_BREAK) {
-        imx_put_data(opaque, URXD_BRK);
+        imx_put_data(opaque, URXD_BRK | URXD_FRMERR | URXD_ERR);
     }
 }
 
diff --git a/include/hw/char/imx_serial.h b/include/hw/char/imx_serial.h
index baeec3183f..f8575ae6db 100644
--- a/include/hw/char/imx_serial.h
+++ b/include/hw/char/imx_serial.h
@@ -26,6 +26,7 @@
 
 #define URXD_CHARRDY    (1<<15)   /* character read is valid */
 #define URXD_ERR        (1<<14)   /* Character has error */
+#define URXD_FRMERR     (1<<12)   /* Character has frame error */
 #define URXD_BRK        (1<<11)   /* Break received */
 
 #define USR1_PARTYER    (1<<15)   /* Parity Error */
-- 
2.14.3


Re: [Qemu-devel] [PATCH] i.MX: Support serial RS-232 break properly
Posted by no-reply@patchew.org 7 years, 10 months ago
Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180320013657.25038-1-tpiepho@impinj.com
Subject: [Qemu-devel] [PATCH] i.MX: Support serial RS-232 break properly

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]            patchew/1521468046-10400-1-git-send-email-thuth@redhat.com -> patchew/1521468046-10400-1-git-send-email-thuth@redhat.com
 t [tag update]            patchew/20180319031545.29359-1-richard.henderson@linaro.org -> patchew/20180319031545.29359-1-richard.henderson@linaro.org
 t [tag update]            patchew/20180319110215.16755-1-peter.maydell@linaro.org -> patchew/20180319110215.16755-1-peter.maydell@linaro.org
 t [tag update]            patchew/20180319113544.704-1-laurent@vivier.eu -> patchew/20180319113544.704-1-laurent@vivier.eu
 t [tag update]            patchew/20180319115846.9662-1-kbastian@mail.uni-paderborn.de -> patchew/20180319115846.9662-1-kbastian@mail.uni-paderborn.de
 t [tag update]            patchew/20180319131743.3885-1-peter.maydell@linaro.org -> patchew/20180319131743.3885-1-peter.maydell@linaro.org
 t [tag update]            patchew/20180319213101.6100-1-wsa+renesas@sang-engineering.com -> patchew/20180319213101.6100-1-wsa+renesas@sang-engineering.com
 * [new tag]               patchew/20180320013657.25038-1-tpiepho@impinj.com -> patchew/20180320013657.25038-1-tpiepho@impinj.com
Switched to a new branch 'test'
10224115a7 i.MX: Support serial RS-232 break properly

=== OUTPUT BEGIN ===
Checking PATCH 1/1: i.MX: Support serial RS-232 break properly...
ERROR: spaces required around that '<<' (ctx:VxV)
#53: FILE: include/hw/char/imx_serial.h:29:
+#define URXD_FRMERR     (1<<12)   /* Character has frame error */
                           ^

total: 1 errors, 0 warnings, 24 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
Re: [Qemu-devel] [PATCH] i.MX: Support serial RS-232 break properly
Posted by Peter Maydell 7 years, 10 months ago
On 20 March 2018 at 01:36, Trent Piepho <tpiepho@impinj.com> wrote:
> Linux does not detect a break from this IMX serial driver as a magic
> sysrq.  Nor does it note a break in the port error counts.
>
> The former is because the Linux driver uses the BRCD bit in the USR2
> register to trigger the RS-232 break handler in the kernel, which is
> where sysrq hooks in.  The emulated UART was not setting this status
> bit.
>
> The latter is because the Linux driver expects, in addition to the BRK
> bit, that the ERR bit is set when a break is read in the FIFO.  A break
> should also count as a frame error, so add that bit too.
>
> Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
> Signed-off-by: Trent Piepho <tpiepho@impinj.com>

Yep, the data sheet is nice and clear here about the requirements.

Applied to target-arm.next, thanks.

-- PMM