[PATCH] Fix serial port detection on systems with Blizzard Chrome EC

[Paul Menzel]

From: "Ronald G. Minnich" 
Date: Thu, 24 May 2012 11:27:26 -0700

Serial port detection in SeaBIOS tests for bit 2 in the IER. That bit is
never set, and SeaBIOS mistakenly reports 0 serial ports. As it happens,
just testing for IER to be not-all-ones is good enough.

This happens on all systems where the AP serial port uses the Blizzard
EC (Ivy Bridge, Haswell). Does not affect Bay Trail, since we use the
native Intel serial port.

It’s a hardware limitation of which registers the Blizzard COMx port
implementation provides. Nothing we can do in EC software.

BUG=chrome-os-partner:9946
TEST=build SeaBIOS with serial port enabled, and note that SeaBIOS
says
     init serial
     Found 1 serial ports
BRANCH=none

Signed-off-by: Ronald G. Minnich

Change-Id: I6bee73788ca6533091d85e54d05b37a564f910a7
Reviewed-on: https://chromium-review.googlesource.com/184421
Reviewed-by: Stefan Reinauer 
Commit-Queue: Stefan Reinauer 
Tested-by: Stefan Reinauer 
Reviewed-on: https://chromium-review.googlesource.com/227500
Signed-off-by: Paul Menzel 
---
  src/serial.c | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/serial.c b/src/serial.c
index 88349c8..e67ac77 100644
--- a/src/serial.c
+++ b/src/serial.c
@@ -26,10 +26,12 @@ detect_serial(u16 port, u8 timeout, u8 count)
          return 0;
      outb(0x02, port+SEROFF_IER);
      u8 ier = inb(port+SEROFF_IER);
-    if (ier != 0x02)
+    dprintf(8, "IER is 0x%02x (Expected 0x02 or 0x00)\n", ier);
+    if (ier != 0x02 && ier != 0)
          return 0;
      u8 iir = inb(port+SEROFF_IIR);
-    if ((iir & 0x3f) != 0x02)
+    dprintf(8, "IIR is 0x%02x (Expected != 0x3f)\n", iir & 0x3f);
+    if ((iir & 0x3f) == 0x3f)
          return 0;

outb(0x00, port+SEROFF_IER);

[Gerd Hoffmann]

On Mon, Mar 16, 2020 at 05:39:45PM +0100, Paul Menzel wrote:
> From: "Ronald G. Minnich" 
> Date: Thu, 24 May 2012 11:27:26 -0700
>
> Serial port detection in SeaBIOS tests for bit 2 in the IER. That bit is
> never set, and SeaBIOS mistakenly reports 0 serial ports. As it happens,
> just testing for IER to be not-all-ones is good enough.
>
> This happens on all systems where the AP serial port uses the Blizzard
> EC (Ivy Bridge, Haswell). Does not affect Bay Trail, since we use the
> native Intel serial port.

What is "AP serial port"?
> if (ier != 0x02)
> dprintf(8, "IER is 0x%02x (Expected 0x02 or 0x00)\n", ier);
> if (ier != 0x02 && ier != 0)
>     return 0;
> u8 iir = inb(port+SEROFF_IIR);
> if ((iir & 0x3f) != 0x02)
> dprintf(8, "IIR is 0x%02x (Expected != 0x3f)\n", iir & 0x3f);
> if ((iir & 0x3f) == 0x3f)

Looks reasonable to me.  Patch doesn't apply too.

cheers,
  Gerd

[Kevin O'Connor]

On Mon, Mar 16, 2020 at 05:39:45PM +0100, Paul Menzel wrote:
> From: "Ronald G. Minnich" 
> Date: Thu, 24 May 2012 11:27:26 -0700
>
> Serial port detection in SeaBIOS tests for bit 2 in the IER. That bit is
> never set, and SeaBIOS mistakenly reports 0 serial ports. As it happens,
> just testing for IER to be not-all-ones is good enough.
>
> This happens on all systems where the AP serial port uses the Blizzard
> EC (Ivy Bridge, Haswell). Does not affect Bay Trail, since we use the
> native Intel serial port.
>
> It’s a hardware limitation of which registers the Blizzard COMx port
> implementation provides. Nothing we can do in EC software.

Thanks.  At a high-level, the change looks fine to me.  However, I
don't think we should add dprintf() statements here - I fear they'll
just flood the log with uninteresting information when trying to debug
other issues.

-Kevin
> BUG=chrome-os-partner:9946
> TEST=build SeaBIOS with serial port enabled, and note that SeaBIOS
> says
>     init serial
>     Found 1 serial ports
> BRANCH=none
>
> Signed-off-by: Ronald G. Minnich
>
> Change-Id: I6bee73788ca6533091d85e54d05b37a564f910a7
> Reviewed-on: https://chromium-review.googlesource.com/184421
> Reviewed-by: Stefan Reinauer 
> Commit-Queue: Stefan Reinauer 
> Tested-by: Stefan Reinauer 
> Reviewed-on: https://chromium-review.googlesource.com/227500
> Signed-off-by: Paul Menzel
>
> src/serial.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/serial.c b/src/serial.c
> index 88349c8..e67ac77 100644
> --- a/src/serial.c
> +++ b/src/serial.c
> @@ -26,10 +26,12 @@ detect_serial(u16 port, u8 timeout, u8 count)
>          return 0;
>      outb(0x02, port+SEROFF_IER);
>      u8 ier = inb(port+SEROFF_IER);
> if (ier != 0x02)
> dprintf(8, "IER is 0x%02x (Expected 0x02 or 0x00)\n", ier);
> if (ier != 0x02 && ier != 0)
>     return 0;
> u8 iir = inb(port+SEROFF_IIR);
> if ((iir & 0x3f) != 0x02)
>
> dprintf(8, "IIR is 0x%02x (Expected != 0x3f)\n", iir & 0x3f);
>
> if ((iir & 0x3f) == 0x3f)
>     return 0;
>
> outb(0x00, port+SEROFF_IER);