Secondary UART for the 8018x

[cocus]

Hi, continuing with the integration of my SBC (which is a step-up above a "base 8018x"), I'm trying to enable the secondary uart. In order for this uart to work fine, I had to route a specific pin from the UART (named SINT, serial interrupt) into one of the INTx pins (INT2 in my case). I've created a new IRQ number (3 in this case). I've also added the new element to the console-serial-8018x.c. The new uart should be attached to tty[1]. But I think this driver is NOT a "serial driver" but rather the console driver. So, I think it shouldn't suffice for adding a new UART. Here's the diff:

diff --git a/elks/arch/i86/drivers/char/console-serial-8018x.c b/elks/arch/i86/drivers/char/console-serial-8018x.c
index 021c6992..fd03aa04 100644
--- a/elks/arch/i86/drivers/char/console-serial-8018x.c
+++ b/elks/arch/i86/drivers/char/console-serial-8018x.c
@@ -42,9 +42,10 @@ struct serial_info {
     struct tty *tty;
 };

-static struct serial_info ports[1] = {
+static struct serial_info ports[2] = {
     /* Serial 0 */ { PCB_B0CMP, PCB_S0CON, PCB_S0STS, PCB_R0BUF, PCB_T0BUF, UART0_IRQ_RX, UART0_IRQ_TX, 0, &ttys[0] },
-    /* TODO: Serial 1 not available { PCB_B1CMP, PCB_S1CON, PCB_S1STS, PCB_R1BUF, PCB_T1BUF, 0, 0, 0, NULL} */
+    { PCB_B1CMP, PCB_S1CON, PCB_S1STS, PCB_R1BUF, PCB_T1BUF, UART1_IRQ_SINT, UART1_IRQ_SINT, 0, &ttys[1] }
+    /* TODO: Serial 1 not available */
 };

 /* UART clock baudrate_compares per baud rate */
@@ -96,6 +97,29 @@ void irq_tx(int irq, struct pt_regs *regs)
 {
 }

+/* serial receive interrupt routine, reads and queues received character */
+void irq_rx_secondary(int irq, struct pt_regs *regs)
+{
+    struct serial_info *sp = &ports[1];
+
+    /* Read UART status */
+    unsigned int status = inw(sp->io_sts);
+
+    if (status & 0x94) {
+        /* discard parity, framing and overrun errors */
+        return;
+    }
+
+    if (status & 64) {
+        /* Read received data */
+        unsigned char c = inb(sp->io_rxbuf);
+
+        if (!tty_intcheck(sp->tty, c)) {
+            chq_addch(&sp->tty->inq, c);
+        }
+    }
+}
+
 /* busy-loop and write a character to the UART */
 static void serial_putc(const struct serial_info *sp, byte_t c)
 {
@@ -160,6 +184,17 @@ void console_init(void)
     update_port(sp);

     printk("console_init: 8018X UART\n");
+
+
+    sp = &ports[1];
+    /* 0x0001 = Mode 1, Asynchronous, 8 data bits, 1 start, 1 stop, no parity, no handshake */
+    outw(0x0001, sp->io_ccon);
+
+    /* setup the UART 1 IRQ */
+    request_irq(sp->irq_rx, irq_rx_secondary, INT_GENERIC);
+
+    /* Set the baudrate, and enable the UART receiver */
+    update_port(sp); 
 }

 void bell(void)
@@ -180,6 +215,9 @@ static void sercon_conout(dev_t dev, char Ch)
 static int sercon_ioctl(struct tty *tty, int cmd, char *arg)
 {
     struct serial_info *port = &ports[0]; /* TODO: add support for Serial 1 */
+    if (ports[1].tty == tty) {
+        port = &ports[1];
+    }

     switch (cmd) {
     case TCSETS:
@@ -198,6 +236,10 @@ static int sercon_ioctl(struct tty *tty, int cmd, char *arg)
 static int sercon_write(register struct tty *tty)
 {
     struct serial_info *port = &ports[0]; /* TODO: add support for Serial 1 */
+    if (ports[1].tty == tty) {
+        port = &ports[1];
+    }
+
     int cnt = 0;

     while (tty->outq.len > 0) {
diff --git a/elks/include/arch/8018x.h b/elks/include/arch/8018x.h
index 88708e8a..d94004ca 100644
--- a/elks/include/arch/8018x.h
+++ b/elks/include/arch/8018x.h
@@ -4,6 +4,8 @@
 #define UART0_IRQ_RX  1 /* maps to interrupt type 20 */
 #define UART0_IRQ_TX  2 /* maps to interrupt type 21 */

+#define UART1_IRQ_SINT 3 /* this is an ephemeral IRQ, and requires the user to connect the SINT pin to an INTx pin */
+
 #define CPU_VEC_DIVIDE_ERROR 0
 #define CPU_VEC_SINGLE_STEP 1
 #define CPU_VEC_NMI 2

What do you suggest @ghaerr ?

Thanks!

[ghaerr]

Well, things are getting complicated because your console driver talks to a serial port, but it looks like a console. So that means that before this mod, your console should be /dev/console and /dev/tty1. After this, if we keep the same approach, then the second console should be /dev/tty2.

I don't quite understand what your problem is, but if it is not working, perhaps an easy way to debug this would be to split the development into two: first comment out the portions of the driver that try to deal with two serial ports, and instead just default the console to the "second" UART. This could be done by changing all the code that looks like this:

struct serial_info *port = &ports[0];

to

#define DEFAULT_PORT 1
struct serial_info *port = &ports[DEFAULT_PORT];

That is, after these mods, the kernel would think its running on the same /dev/tty1 and /dev/console, but all I/O would be on UART 2.

Yes, you would have to keep the following type code:

    /* setup the UART 1 IRQ */
    request_irq(sp->irq_rx, irq_rx_secondary, INT_GENERIC);

The below function is unnecessary, should probably use the original function and use the irq paramater to select the proper ports[]. You could pass the original irq_rx to to request_irq for the secondary port as well. That is, for step one you would call request_irq() exactly as you do for the first serial port, using irq_rx as the 2nd parameter.

Unnecessary function:

void irq_rx_secondary(int irq, struct pt_regs *regs)

After that gets working, comment back in the code that deals with port = ports + 1 etc.

Does that help?

[cocus]

Ok, I've got it. I think my issue was an unconnected line on the hardware :) I've reworked the code, since the irq_rx function can work for both UARTs, as stated.

Now I've everything running on the secondary UART.

Question is, how to make both show up? I understand that if I look for tty[0] or tty[1] on the tty_ops functions it should be enough, but... is it actually okay to do it like this? I thought I needed to move this entire source to another file so the kernel would understand these are "serial ports" and not a generic console.

[ghaerr]

I thought I needed to move this entire source to another file so the kernel would understand these are "serial ports" and not a generic console.

No, that's going to be a lot of work. For now, best to treat the two serial ports as consoles, since you have effectively written a console driver, not a serial driver. (Yes, the console driver happens to talk to a UART).

Question is, how to make both show up?

In include/linuxmt/ntty.h, you'll have increase NR_CONSOLES to 2:

#if defined(CONFIG_CONSOLE_DIRECT) || defined(CONFIG_CONSOLE_BIOS)
#define NR_CONSOLES MAX_CONSOLES
#else
#define NR_CONSOLES 1   /* headless*/
#endif

Then, look at elks/arch/i86/char/ntty.c. You will see it automatically adds the consoles in to the tty array.

When this all works, you'll have /dev/tty1 and /dev/tty2. You can run logins on both of them by editting /etc/inittab. /dev/tty1 will be the "kernel" console though.

[cocus]

Then, look at elks/arch/i86/char/ntty.c. You will see it automatically adds the consoles in to the tty array.

When this all works, you'll have /dev/tty1 and /dev/tty2. You can run logins on both of them by editting /etc/inittab. /dev/tty1 will be the "kernel" console though.

I've done this, but I can't even echo to tty2. getty also fails.

# /bin/getty /dev/tty2
/bin/getty: cannot open terminal /dev/tty2
# echo 1 > /dev/tty2
cannot create /dev/tty2: error 19

[ghaerr]

cannot create /dev/tty2: error 19

That's ENODEV, which is tty_open saying there isn't a second tty.

Are you sure NR_CONSOLES is 2?

If so, then ntty.c is probably failing the open. Did you have this working in the two step plan I outlined, or are you trying to get this all working at once? We need confirmation that the second open is succeeding. You might put printk in ntty.c during the open routines to see. You can also turn on DEBUG_TTY and DEBUG_CLOSE to trace what is happening.

[cocus]

Are you sure NR_CONSOLES is 2?

Yes.

Seems like changing that DEBUG_TTY rebuild stuff that actually made a difference. I can echo to tty2 and run getty on it. Interestingly enough it doesn't run getty by default, like it's not getting into the multiuser. Not sure if this is by design, but hey, it's an easy fix if needed.

Next question would be how to change the baudrate. The good ol' command with stty doesn't seem to work: stty -F /dev/tty2 19200. Is this a side-effect of these devices being a ttys but not serial ports?

[cocus]

Offtopic, I need to get this written, but we're missing kill on the romfs :) I think I need to add it to the Applications file for the big :192k tag of Romfs.

[ghaerr]

Seems like changing that DEBUG_TTY rebuild stuff that actually made a difference.

The kernel Makefiles aren't setup to automatically rebuild what is needed when you make changes to, say, ntty.h. Sounds like you just changed it and expected it to work. Nope. Use make kclean to save time rebuilding only the kernel on any top-level include file changes.

The good ol' command with stty doesn't seem to work:

Did it ever work for port 0? Look at static int sercon_ioctl(struct tty tty, int cmd, char arg). See if that is being called on the stty. Also, I don't think stty -F option is supported; what is that supposed to do?

Is this a side-effect of these devices being a ttys but not serial ports?

No, not if you have the ioctl implementedf for TCSETA etc, which it looks like you do.

I need to get this written, but we're missing kill on the romfs :) I think I need to add it to the Applications file for the big :192k tag of Romfs

Yes, I think we just ran out of same at :128k for kill. Go ahead and the :192k tag on sys_utils/kill.

[ghaerr]

like it's not getting into the multiuser. Not sure if this is by design,

You have to run init 3 to get multiuser, or change the initdefault line in /etc/inittab to 3. We desktop users have been essentially accomplishing both by specifying init 3 in /bootopts, but that's not available in CONFIG_ROMCODE.

[cocus]

The kernel Makefiles aren't setup to automatically rebuild what is needed when you make changes to, say, ntty.h. Sounds like you just changed it and expected it to work. Nope. Use make kclean to save time rebuilding only the kernel on any top-level include file changes.

Makes sense. I forgot to invoke make kclean (or even clean).

Did it ever work for port 0? Look at static int sercon_ioctl(struct tty tty, int cmd, char arg). See if that is being called on the stty. Also, I don't think stty -F option is supported; what is that supposed to do?

Nope, because it's not taking the -F argument. It's supposed to be used to indicate which device you're trying to change an argument to. And by the looks of it, if you don't specify any "command", passing the baud rate should suffice. I've always used it like that.

You have to run init 3 to get multiuser, or change the initdefault line in /etc/inittab to 3. We desktop users have been essentially accomplishing both by specifying init 3 in /bootopts, but that's not available in CONFIG_ROMCODE.

I think that'll be too much for my use case, but it's good to know. Also, why is the bootopts not being recognized but shipped on the fs?

[ghaerr]

it's not taking the -F argument. It's supposed to be used to indicate which device you're trying to change an argument to.

You'll have to use something like stty 19200 < /dev/tty2 for now.

why is the bootopts not being recognized but shipped on the fs?

Just added that to my list to remove. It's not as easy as putting it in elkscmd/Applications, since /bootopts has to be in the first directory sector and (used to be) required to be right next to /linux. Both are treated specially due to extremely tight space in the boot loader(s).

or change the initdefault line in /etc/inittab to 3 I think that'll be too much for my use case

You'll still probably need to edit /etc/inittab to get multiuser at startup. Let me know how it goes.

[cocus]

You'll have to use something like stty 19200 < /dev/tty2 for now.

I'll try it!

Just added that to my list to remove. It's not as easy as putting it in elkscmd/Applications, since /bootopts has to be in the first directory sector and (used to be) required to be right next to /linux. Both are treated specially due to extremely tight space in the boot loader(s).

Yeah, about the cleanups and optimizations for this platform, I was thinking that I don't have the BDA which it's located at 0400:0 on IBM PCs, so that could potentially be reclaimed as long as there's no one using that section of RAM. In short, I have a flat, and ready to use RAM section from 0 to 0x7FFFF. I've checked and the BDA is the only thing different on my setup and an IBM PC.

You'll still probably need to edit /etc/inittab to get multiuser at startup. Let me know how it goes.

I'll try anyway and let you know!

[ghaerr]

about the cleanups and optimizations for this platform, I was thinking that I don't have the BDA which it's located at 0400:0 on IBM PCs, so that could potentially be reclaimed as long as there's no one using that section of RAM. In short, I have a flat, and ready to use RAM section from 0 to 0x7FFFF.

Good point, and very doable (almost)! If you look at your section of include/config.h, You'll still need the 1K bytes for interrupt vectors 0x0000 - 0x03FF, right? Disk block devices may use DMASEG (a single 1K sector), except perhaps SSD (you'll have to check. I think it uses its own static buffer). If no disk and no track caching, then you have KERNEL_SETUP_DATA which I think for your system is likely set to 0x60. /bootopts is at 0x50 (unused), and BDA at 0x40 (unused).

So, you might have: 0x00 interrupt vectors (too large at 1k?) 0x40 wasted space? 0x60 KERNEL_SETUP_DATA

It looks like there might be an issue under CONFIG_ROMCODE for your system in that DMASEG is not defined (correct, if no disk driver) but DMASEGSZ is set to 0x40. I think if DMASEG is not defined, DMASEGSZ can be ignored. Thus, you might be able to set KERNEL_SETUP_DATA to 0x40 (or lower, if your interrupt vector space is lower).

[cocus]

You'll still need the 1K bytes for interrupt vectors 0x0000 - 0x03FF, right?

Yes!

Disk block devices may use DMASEG (a single 1K sector), except perhaps SSD (you'll have to check. I think it uses its own static buffer).

My SD driver uses the buffers provided by the ssd layer, I don't allocate anything in there (except maybe 16 bytes in the stack of a function).

So, you might have: 0x00 interrupt vectors (too large at 1k?) 0x40 wasted space? 0x60 KERNEL_SETUP_DATA

I don't have that many interrupt vectors, but I don't mind having the same reserved size as on an IBM PC. Not much gain there. Yes, the 0x40 (BDA) is completely wasted.

It looks like there might be an issue under CONFIG_ROMCODE for your system in that DMASEG is not defined (correct, if no disk driver) but DMASEGSZ is set to 0x40. I think if DMASEG is not defined, DMASEGSZ can be ignored. Thus, you might be able to set KERNEL_SETUP_DATA to 0x40 (or lower, if your interrupt vector space is lower).

Yes, I think it's quite doable to move the KERNEL_SETUP_DATA to 0x40 for this platform, so we have a "flatter" available RAM after it.

[ghaerr]

You'll still need the 1K bytes for interrupt vectors 0x0000 - 0x03FF, right? Yes! I don't have that many interrupt vectors, but I don't mind having the same reserved size as on an IBM PC.

I thought you're saying that you do need 256 interrupt vectors (=1k bytes) from segment 0x00 until 0x40. ?

Are you saying that your system doesn't use those in hardware vectors? I'm pretty sure the INT instruction takes a byte parameter, which could then vector to any one of the 256 low core locations...

[cocus]

Are you saying that your system doesn't use those in hardware vectors? I'm pretty sure the INT instruction takes a byte parameter, which could then vector to any one of the 256 low core locations...

I don't use that many on hardware, but the idea is to keep those (1k bytes) as on the IBM PC so you can use the INT instruction to address any of the 256 handlers, as you've said. I don't want to modify this, even if there's no real need to have all of them.

[cocus]

Sorry for the delay, you know I usually have my "lapsus" in general :). So I've tried the code, with the command you've provided before to change the baudrate, and of course it works! Now, by jumping the SINT pin into one of the INTx pins, the secondary uart is available and works a treat! However, this is a serial driver used as a console. My question is, should I move this into its own serial-xxxx.c or whatever? Or I could commit these changes into the console-serial-8018x.c for now, and afterwards, rename that particular file as serial-8018x.c. Thanks!

[ghaerr]

Or I could commit these changes into the console-serial-8018x.c

Go ahead and send over a PR with the changes in the existing file consiole-serial-8081x.c, and we can determine how/when to convert it to a serial driver afterwards, if needed.