qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4] nios2: Add Altera JTAG UART emulation


From: Bystricky, Juro
Subject: Re: [Qemu-devel] [PATCH v4] nios2: Add Altera JTAG UART emulation
Date: Thu, 9 Feb 2017 20:53:04 +0000


> On 02/09/2017 07:52 PM, Juro Bystricky wrote:
> > JTAG UART core eliminates the need for a separate RS-232 serial
> > connection to a host PC for character I/O.
> 
> And how does this describe the content of this patch ? This patch adds a
> model of JTAG UART , it doesn't eliminate anything ...
> 

The commit message explicitly says:
"Add Altera JTAG UART emulation."

But I will amend the message with more details.

> > Hardware emulation based on:
> > https://www.altera.com/en_US/pdfs/literature/ug/ug_embedded_ip.pdf
> > (Please see "Register Map" on page 65)
> >
> > Signed-off-by: Juro Bystricky <address@hidden>
> > ---
> 
> Changelog is missing ...

I modelled this patch based on other nios2 patches, none of them contained 
Changelog. So please elaborate.


> > +    /*
> > +     * FIFO size can be set from 8 to 32,768 bytes.
> > +     * Only powers of two are allowed.
> > +     */
> > +    fifo_size_ok = false;
> > +    for (size = 8; size <= 32768; size *= 2) {
> > +        if (size == fifo_size) {
> > +            fifo_size_ok = true;
> > +            break;
> > +        }
> > +    }
> 
> Isn't that like
> 
> if (size < 8 || size > 32768 || (size & ~(1 << ffs(size))))
>   error()
> 

possibly, however ffs is rejected by checkpatch as non-portable
libc call. 


> > +#define DEFAULT_FIFO_SIZE 64
> 
> This is still not QOM property , why ?
> 

Not sure what you mean, the code has:

DEFINE_PROP_UINT32("fifo-size", AlteraJUARTState, rx_fifo_size, 
DEFAULT_FIFO_SIZE),


Thanks

Juro



reply via email to

[Prev in Thread] Current Thread [Next in Thread]