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: Fri, 10 Feb 2017 18:35:24 +0000

Hello Konrad, 
I agree with all your comments.

[...]

> > +static void altera_juart_init(Object *obj)
> > +{
> > +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> > +    AlteraJUARTState *s = ALTERA_JUART(obj);
> > +
> > +    memory_region_init_io(&s->mmio,  OBJECT(s), &juart_ops, s,
> > +                          TYPE_ALTERA_JUART, 2 * 4);
> 
> What's this 2 * 4?
> I'd suggest you use a #define for that.

yes, not very clear it is the memory size, will redo

[...]

> > +    chr = serial_hds[channel];
> > +    if (!chr) {
> > +        snprintf(label, ARRAY_SIZE(label), "%s%d", chr_name, channel);
> > +        chr = qemu_chr_new(label, "null");
> > +        if (!chr) {
> > +            error_report("Can't assign serial port to altera juart%d",
> channel);
> 
> Can't you reuse label?
> 

Will rework the message to use "label"

[...]


> > +#define DEFAULT_FIFO_SIZE 64
> 
> The name here might conflict with other thing.
> I'd change that by ALTERA_JUART_DEFAULT_FIFO_SZ or something.
> 
> 

I agree, will rename

[...]

> 
> The rest seems ok to me.
> 


Thanks for reviewing the code.
Juro





reply via email to

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