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

[Frederic Konrad <fred.konrad@greensocs.com>]

On 02/09/2017 09:53 PM, Bystricky, Juro wrote:
> 
> 
>> 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 <juro.bystricky@intel.com>
>>> ---
>>
>> Changelog is missing ...
> 
> I modelled this patch based on other nios2 patches, none of them contained 
> Changelog. So please elaborate.

Things after "---" disappear when commited in the git.

Thanks,
Fred

> 
> 
>>> +    /*
>>> +     * 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
> 
>