Patchwork [RESEND] Serial: DOS Plus regression

login
register
mail settings
Submitter Roy Tam
Date March 25, 2010, 12:58 p.m.
Message ID <473191351003250558m8bb35c7t1c89ee94bd05ee61@mail.gmail.com>
Download mbox | patch
Permalink /patch/48515/
State New
Headers show

Comments

Roy Tam - March 25, 2010, 12:58 p.m.
The commit in r1049 (serial interrupt fix (Hampa Hug)) prevents
booting Digital Research DOSPlus. Following patch partially reverts
that commit and makes DOSPlus booting in QEMU again.

Sign-off-by: Roy Tam <roytam@gmail.com>

--
Aurelien Jarno - March 26, 2010, 10:16 p.m.
On Thu, Mar 25, 2010 at 08:58:13PM +0800, Roy Tam wrote:
> The commit in r1049 (serial interrupt fix (Hampa Hug)) prevents
> booting Digital Research DOSPlus. Following patch partially reverts
> that commit and makes DOSPlus booting in QEMU again.

I guess that this patch has been introduced to fix an issue. Are you
sure your partial revert is correct and is not going to create a 
regression? What the documentation says?

> Sign-off-by: Roy Tam <roytam@gmail.com>
> 
> --
> 
> diff --git a/hw/serial.c b/hw/serial.c
> index f3ec36a..2ae550e 100644
> --- a/hw/serial.c
> +++ b/hw/serial.c
> @@ -401,10 +401,6 @@ static void serial_ioport_write(void *opaque,
>                       s->poll_msl = 0;
>                  }
>              }
> -            if (s->lsr & UART_LSR_THRE) {
> -                s->thr_ipending = 1;
> -                serial_update_irq(s);
> -            }
>          }
>          break;
>      case 2:
> 
> 
>
Roy Tam - March 26, 2010, 11:38 p.m.
2010/3/27 Aurelien Jarno <aurelien@aurel32.net>:
> On Thu, Mar 25, 2010 at 08:58:13PM +0800, Roy Tam wrote:
>> The commit in r1049 (serial interrupt fix (Hampa Hug)) prevents
>> booting Digital Research DOSPlus. Following patch partially reverts
>> that commit and makes DOSPlus booting in QEMU again.
>
> I guess that this patch has been introduced to fix an issue. Are you
> sure your partial revert is correct and is not going to create a
> regression? What the documentation says?

In other words, I can't find other implementations (such as Bochs)
checks LSR_THRE in IER. And the log message in r1049 doesn't say any
regressions too. As a result I assume checking LSR_THRE in IER in
r1049 is wrong implementation. Partial reverting r1049 are tested and
no regression created(tested with DOS/Windows/*nix guests, if they
work correctly before applying this patch).

>
>> Sign-off-by: Roy Tam <roytam@gmail.com>
>>
>> --
>>
>> diff --git a/hw/serial.c b/hw/serial.c
>> index f3ec36a..2ae550e 100644
>> --- a/hw/serial.c
>> +++ b/hw/serial.c
>> @@ -401,10 +401,6 @@ static void serial_ioport_write(void *opaque,
>>                       s->poll_msl = 0;
>>                  }
>>              }
>> -            if (s->lsr & UART_LSR_THRE) {
>> -                s->thr_ipending = 1;
>> -                serial_update_irq(s);
>> -            }
>>          }
>>          break;
>>      case 2:
>>
>>
>>
>
> --
> Aurelien Jarno                          GPG: 1024D/F1BCDB73
> aurelien@aurel32.net                 http://www.aurel32.net
>

Patch

diff --git a/hw/serial.c b/hw/serial.c
index f3ec36a..2ae550e 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -401,10 +401,6 @@  static void serial_ioport_write(void *opaque,
                      s->poll_msl = 0;
                 }
             }
-            if (s->lsr & UART_LSR_THRE) {
-                s->thr_ipending = 1;
-                serial_update_irq(s);
-            }
         }
         break;
     case 2: