On Wed, Jun 08, 2022 at 06:50:18AM +0200, Anton Lindqvist wrote:
> On Sun, May 01, 2022 at 04:17:34PM +0000, Visa Hankala wrote:
> > On Sat, Apr 30, 2022 at 09:40:24AM +0200, Anton Lindqvist wrote:
> > > On Sun, Mar 13, 2022 at 04:17:07PM +0100, Mark Kettenis wrote:
> > > > > Date: Fri, 11 Mar 2022 07:53:13 +0100
> > > > > From: Anton Lindqvist <[email protected]>
> > > > >
> > > > > On Tue, Mar 08, 2022 at 01:44:47PM +0000, Visa Hankala wrote:
> > > > > > On Tue, Mar 08, 2022 at 08:04:36AM +0100, Anton Lindqvist wrote:
> > > > > > > On Mon, Mar 07, 2022 at 07:36:35AM +0000, Visa Hankala wrote:
> > > > > > > > I still think that checking TXFF and using the same code for
> > > > > > > > both
> > > > > > > > SBSA and true PL011 UARTs would be the best choice. This would
> > > > > > > > avoid
> > > > > > > > fragmenting the code and improve robustness by relying on
> > > > > > > > functionality
> > > > > > > > that is common to the different controller variants.
> > > > > > >
> > > > > > > Fair enough, new diff.
> > > > > >
> > > > > > Maybe the comments should omit the FIFO space description and just
> > > > > > mention the lack of the level control register in the SBSA UART
> > > > > > register interface.
> > > > >
> > > > > I ended up tweaking the comments before committing. Thanks for all the
> > > > > feedback.
> > > > >
> > > >
> > > > Hi Anton,
> > > >
> > > > This diff seems to break things. When I boot my rpi4 it now prints:
> > > >
> > > > pluart0 at simplebus0: rev 0, 16 byte fifo
> > > > pluart0: console
> > > > bcmbsc0 at simplebus0
> > > > iic0 at bcmbsc0
> > > >
> > > > so it appears that a carriage return character is lost here.
> > > >
> > > > Later on output stops at:
> > > >
> > > > reordering libraries: done.
> > > >
> > > > and only when I reboot the machine the login prompt appears, but with
> > > > some wierd respawning:
> > > >
> > > > OpenBSD/arm64 (rutter.sibelius.xs4all.nl) (console)
> > > >
> > > > login: init: getty repeating too quickly on port /dev/console,
> > > > sleeping
> > > > init: getty repeating too quickly on port /dev/console, sleeping
> > > >
> > > > OpenBSD/arm64 (rutter.sibelius.xs4all.nl) (console)
> > > >
> > > > login:
> > > > OpenBSD/arm64 (rutter.sibelius.xs4all.nl) (console)
> > > >
> > > > login:
> > > >
> > > > If you don't have a quick fix for this, may I suggest reverting the
> > > > commit? We're heading towards release and we don't want the serial
> > > > console on the rpi4 to be broken.
> > >
> > > Circling back to this: what happens is that no tx interrupt is raised
> > > when sending less data than the configured interrupt fifo level, causing
> > > the tty to end up in a forever busy state. Clearing the busy flag after
> > > a successful transmission of all queued data solves the problem.
> >
> > Are you sure about the behaviour of the interrupt?
> >
> > One possible problem is that pluart_intr() uses the raw, unmasked,
> > interrupt status to clear interrupts. Your previous patch always
> > disabled the Tx interrupt whenever the raw status indicated a Tx FIFO
> > level event.
> >
> > This new patch might very well be correct. However, it feels strange
> > if the hardware raises the Tx interrupt only at one specific level of
> > FIFO state change.
> >
> > It would be nice to know if a comstart()-style arrangement of interrupt
> > masking worked in pluart_start().
>
> What did work was to not clear the tx interrupt in pluart_intr().
> Updated diff with some additional changes:
>
> * Flush any pending transmission before configuring the device during
> attachment. Prevents the next dmesg line from being mangled.
>
> * Make pluart_start() mimic comstart() as proposed by visa@.
>
> * While entering ddb (i.e. poll mode), disable interrupts.
<snip>
> @@ -792,4 +870,13 @@ pluartcnputc(dev_t dev, int c)
> void
> pluartcnpollc(dev_t dev, int on)
> {
> + int s;
> +
> + s = splhigh();
> + if (on)
> + bus_space_write_4(pluartconsiot, pluartconsioh, UART_IMSC, 0);
> + else
> + bus_space_write_4(pluartconsiot, pluartconsioh, UART_IMSC,
> + UART_IMSC_RXIM | UART_IMSC_RTIM);
> + splx(s);
> }
Does this fix an actual issue? If not, I would leave it out. ddb entry
can happen in unexpected places. There is a risk that the mask gets
messed up in particular when leaving the debugger.
Otherwise, OK visa@
