Re: [PATCH 2/3] hw/char: cadence_uart: Disable transmit when input clock is disabled

[Edgar E. Iglesias]

On Mon, Aug 23, 2021 at 02:43:26PM +1000, Alistair Francis wrote:
> On Mon, Aug 23, 2021 at 12:11 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > At present when input clock is disabled, any character transmitted
> > to tx fifo can still show on the serial line, which is wrong.
> >
> > Fixes: b636db306e06 ("hw/char/cadence_uart: add clock support")
> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > ---
> >
> >  hw/char/cadence_uart.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
> > index b4b5e8a3ee..154be34992 100644
> > --- a/hw/char/cadence_uart.c
> > +++ b/hw/char/cadence_uart.c
> > @@ -327,6 +327,11 @@ static gboolean cadence_uart_xmit(void *do_not_use, 
> > GIOCondition cond,
> >  static void uart_write_tx_fifo(CadenceUARTState *s, const uint8_t *buf,
> >                                 int size)
> >  {
> > +    /* ignore characters when unclocked or in reset */
> > +    if (!clock_is_enabled(s->refclk) || device_is_in_reset(DEVICE(s))) {
> > +        return;
> > +    }
> 
> Should we log a guest error here?
> 


I think it's a good idea to log a guest error on the TX path.

It's not uncommon for bare-metal applications to rely on FSBL/Bootloaders
setting up clocks/resets. On QEMU, it's convenient to run applications
directly without boot-loaders, particularly FSBL since it requires Xilinx tools
to generate it. It's going to be hard for users to figure out what's going on
with no console output and no guest error.

I also wonder how Linux handles this when CCF is not active?
Do we need to use TYPE_ARM_LINUX_BOOT_IF?

Best regards,
Edgar