serial driver fixes/improvements

Hi,

this patch improves the behaviour of xeno_16550A on RTSER_RTIOC_EVENT_WAIT. In case it is invoked from non-RT, the driver tries to trigger an automatic switch-back to RT via returning ENOSYS. The patch also fixes another remaining issue about the right context when calling RTSER_RTIOC_SET_CONFIG (forgot the corner case of releasing the RX history buffer in RT).

Note that I intentionally did not apply the EPERM->ENOSYS scheme on RTSER_RTIOC_SET_CONFIG. Here a switch to non-RT might be required if the device was opened in non-RT and a reconfigure request regarding the RX history buffer is now issued in RT. I think it's better to let the user decide via a preceding explicit switch to non-RT if leaving RT is really correct.

The patch furthermore contains a README describing the device setup. I hope I addressed potential questions and problems sufficiently, feedback is welcome.

Jan

------------------------------------------------------------------------

Index: drivers/16550A/Kconfig
===================================================================

16550A/Kconfig +2 -1

@@ -3,4 +3,5 @@
 	bool "16550A UART driver"
 	default n
 	help
-	Real-time UART driver for 16550A compatible controllers.
+	Real-time UART driver for 16550A compatible controllers. See
+	drivers/16550A/README for more details.

Index: drivers/16550A/16550A.c
===================================================================

16550A/16550A.c +19 -13

@@ -556,18 +556,24 @@
                 config = &config_buf;
             }
 
-            if (testbits(config->config_mask, RTSER_SET_TIMESTAMP_HISTORY) &&
-                testbits(config->timestamp_history,
-                         RTSER_RX_TIMESTAMP_HISTORY)) {
-                if (test_bit(RTDM_CREATED_IN_NRT, &context->context_flags)) {
-                    if (rtdm_in_rt_context())
-                        return -EPERM;
+            if (testbits(config->config_mask, RTSER_SET_TIMESTAMP_HISTORY)) {
+                if (test_bit(RTDM_CREATED_IN_NRT, &context->context_flags) &&
+                    rtdm_in_rt_context()) {
+                    /* already fail if we MAY allocate or release a non-RT
+                     * buffer in RT context */
+                    return -EPERM;
+                }
 
-                    hist_buf = kmalloc(IN_BUFFER_SIZE * sizeof(__u64),
-                                       GFP_KERNEL);
-                } else
-                    hist_buf =
-                        rtdm_malloc(IN_BUFFER_SIZE * sizeof(__u64));
+                if (testbits(config->timestamp_history,
+                             RTSER_RX_TIMESTAMP_HISTORY)) {
+                    if (test_bit(RTDM_CREATED_IN_NRT,
+                                 &context->context_flags))
+                        hist_buf = kmalloc(IN_BUFFER_SIZE * sizeof(__u64),
+                                           GFP_KERNEL);
+                    else
+                        hist_buf =
+                            rtdm_malloc(IN_BUFFER_SIZE * sizeof(__u64));
+                }
 
                 if (!hist_buf)
                     return -ENOMEM;

@@ -643,7 +649,7 @@
             rtdm_toseq_t        timeout_seq;
 
             if (!rtdm_in_rt_context())
-                return -EPERM;
+                return -ENOSYS;
 
             /* only one waiter allowed, stop any further attempts here */
             if (test_and_set_bit(0, &ctx->ioc_event_lock))

@@ -1005,7 +1011,7 @@
     device_class:       RTDM_CLASS_SERIAL,
     device_sub_class:   RTDM_SUBCLASS_16550A,
     driver_name:        "rt_16550A",
-    driver_version:     RTDM_DRIVER_VER(1, 1, 1),
+    driver_version:     RTDM_DRIVER_VER(1, 1, 2),
     peripheral_name:    "UART 16550A",
     provider_name:      "Jan Kiszka",
 };

Index: drivers/16550A/README
===================================================================

16550A/README +34 -1

@@ -0,0 +1,34 @@
+Real-Time Serial Driver for 16550A-Compatible Devices
+=====================================================
+
+Preparation
+-----------
+ - decide which serial ports are to be managed by the real-time driver
+
+ - identify their I/O addresses and IRQ numbers:
+
+   setserial /dev/ttyS<N>
+
+ - disable the Linux driver for all these devices:
+
+   setserial /dev/ttyS<N> uart none
+
+
+Invocation
+----------
+
+modprobe xeno_16550A ioaddr=<io1>[,<io2>...] irq=<irq1>[,<irq2>...]
+                     [tx_fifo=<len1>[,<len2>...]] [start_index=<index>]
+
+Arguments:
+    ioaddr<N>   - I/O address of device <N> (e.g. 0x3f8 for ttyS0)
+    irq<N>      - interrupt number of device <N> (e.g. 4 for ttyS0)
+    tx_fifo<N>  - Transmitter FIFO size in bytes of device <N>, default is 16
+    start_index - First device instance number to be used, default is 0
+
+
+Usage
+-----
+
+The API is described in the API documentation under Modules -> Real-Time
+Driver Model -> Device Profiles -> Serial Devices.
 
 
------------------------------------------------------------------------

_______________________________________________
Xenomai-core mailing list
[email protected]
https://mail.gna.org/listinfo/xenomai-core

Hi Jan et al.!

Jan Kiszka wrote:
[...]

Quoted 9 lines

16550A/README +7 -0

+modprobe xeno_16550A ioaddr=<io1>[,<io2>...] irq=<irq1>[,<irq2>...]
+                     [tx_fifo=<len1>[,<len2>...]] [start_index=<index>]
+
+Arguments:
+    ioaddr<N>   - I/O address of device <N> (e.g. 0x3f8 for ttyS0)
+    irq<N>      - interrupt number of device <N> (e.g. 4 for ttyS0)
+    tx_fifo<N>  - Transmitter FIFO size in bytes of device <N>,

default is 16
+ start_index - First device instance number to be used, default is 0

If I may throw in my 2 eurocents:
I think the Arguments-section is a bit misleading.
One doesn't specify ioaddr1, ioaddr2 etc. on the command line, as suggested by ioaddr<N>, but ioaddr=[1.dev],[2.dev]...
Same for irq and tx_fifo.

What about this:

Arguments:

    ioaddr      - I/O addresses of devices (e.g. 0x3f8 for ttyS0),

separated by comma
irq - interrupt numbers of devices (e.g. 4 for ttyS0),
separated by comma
tx_fifo - Transmitter FIFO sizes in bytes of devices, default is
16, separated by comma
start_index - First device instance number to be used, default is 0
=> rtser0

Just trying to make it as clear as possible :-)

Quoted 35 lines · Hannes Mayer

Hi Jan et al.!

Jan Kiszka wrote:
[...]

Quoted 9 lines

16550A/README +7 -0

+modprobe xeno_16550A ioaddr=<io1>[,<io2>...] irq=<irq1>[,<irq2>...]
+                     [tx_fifo=<len1>[,<len2>...]] [start_index=<index>]
+
+Arguments:
+    ioaddr<N>   - I/O address of device <N> (e.g. 0x3f8 for ttyS0)
+    irq<N>      - interrupt number of device <N> (e.g. 4 for ttyS0)
+    tx_fifo<N>  - Transmitter FIFO size in bytes of device <N>,

default is 16
+ start_index - First device instance number to be used, default is 0

If I may throw in my 2 eurocents:
I think the Arguments-section is a bit misleading.
One doesn't specify ioaddr1, ioaddr2 etc. on the command line, as suggested by ioaddr<N>, but ioaddr=[1.dev],[2.dev]...
Same for irq and tx_fifo.

What about this:

Arguments:

   ioaddr      - I/O addresses of devices (e.g. 0x3f8 for ttyS0),

separated by comma
irq - interrupt numbers of devices (e.g. 4 for ttyS0),
separated by comma
tx_fifo - Transmitter FIFO sizes in bytes of devices, default is
16, separated by comma
start_index - First device instance number to be used, default is 0
=> rtser0

Quoted 2 lines

Just trying to make it as clear as possible :-)

Indeed, my version is misleading, your version makes more sense. Just the parameter for ttyS0 are not necessarily the one you listed, only

typically. Will post a patch.

Oops, actually I originally sorted ttyS0 to 0x3f8, not you. As I said:
don't trust anything I produce between 3 and 5 am...