Charles M. Hannum <abuse@spamalicious.com> wrote:
 
> So, I'm looking at this, and there are two obvious problems:
> 
>                                 bus_space_write_1(iot, ioh, com_ier,sc->sc_ier)
> ;
> -                               iir = IIR_NOPEND;
>                                 continue;
> 
> You can't do this.  In that case where that kluge fires (a bug in some
> Winbond chips), it will cause the go to go into a loop and wedge.
> This change *must* be removed.

Maybe I'm completely missing something here. As far as I know the continue
causes the code to go to the end of the do while loop where we get

> 
>                 }
> !       } while (ISSET((iir = bus_space_read_1(iot, ioh, com_iir)), IIR_RXRDY)
> !           || ((iir & IIR_IMASK) == 0));
> 
> This test is also wrong.  The IIR is *NOT* a bit mask.  Please read
> the documentation.

And here, iir is set again via the bus_space_read_1() and thus overwriting
the IIR_NOPEND from before the continue statement. So, if the
iir = IIR_NOPEND bit is meant as a no-op or delay to fix a winbond uart bug
it probably should have a comment as to why it is there.

The ((iir & IIR_IMASK) == 0) could/should be ((iir & IIR_IMASK) == IIR_MLSC).

Now, if only I had saved the email discussions I exchanged with Allen Briggs
about his broken uart and how to fix it, this respons would be somewhat
clearer.


Onno