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
