On 02/09/2017 04:22 PM, Bystricky, Juro wrote:
>
>
>> [...]
>>
>>>>> +static uint64_t altera_juart_read(void *opaque, hwaddr addr, unsigned
>>>> int size)
>>>>> +{
>>>>> + AlteraJUARTState *s = opaque;
>>>>> + uint32_t r;
>>>>> +
>>>>> + addr >>= 2;
>>>>
>>>> Hmmmmm, how will unaligned read from one of these registers be handled
>>>> on real HW ? ie. read from address 0x3 ? What about writes ?
>>>>
>>>
>>> there is no reading/writing going on via "addr".
>>> This just maps the hw address into register number, where registers are
>> at
>>> 4 bytes boundaries (so they are aligned as needed) but indexed as
>> 1,2,3....
>>> (Pretty common code in other drivers.)
>>> But will redo the code anyway so there are no shifts.
>>
>> This doesn't answer my question at all. How does real hardware behave if
>> you read from unaligned address in the register space , ie. offset 0x3 ?
>>
>
> Not sure I understand the question here. Which "real hardware" are we talking
> about?
By real hardware I mean real Nios2 system ...
> If "real hardware" contains MMU or MPU then an exception is generated on
> misalign access.
Is this handled here or not ?
>> [...]
>>
>>>>> +static void altera_juart_receive(void *opaque, const uint8_t *buf, int
>>>> size)
>>>>> +{
>>>>> + int i;
>>>>> + AlteraJUARTState *s = opaque;
>>>>> +
>>>>> + if (s->rx_fifo_len >= FIFO_LENGTH) {
>>>>> + fprintf(stderr, "WARNING: UART dropped char.\n");
>>>>
>>>> Can this ever happen ? can_receive will IMO prevent this case.
>>>>
>>>
>>> Fair question. I did not go through all QEMU code to see when/where
>>> in the bowels of QEMU can_receive is actually called. If it is always
>> called prior "receive", then this check may be redundant. Again, there are
>>> other serial drivers that do this check as well (etraxfs, Xilinx,...)
>>> But most don't, so I'll follow the majority and remove it.
>>
>> Maybe you can check the core code or someone else can jump in an answer
>> this.
>
> OK, will look deeper
>
> [...]
>
>>>>> +#ifndef ALTERA_JUART_H
>>>>> +#define ALTERA_JUART_H
>>>>> +
>>>>> +#include "hw/sysbus.h"
>>>>> +#include "sysemu/char.h"
>>>>> +
>>>>> +/*
>>>>> + * The read and write FIFO depths can be set from 8 to 32,768 bytes.
>>>>> + * Only powers of two are allowed. A depth of 64 is generally optimal
>>>> for
>>>>> + * performance, and larger values are rarely necessary.
>>>>> + */
>>>>> +
>>>>> +#define FIFO_LENGTH 64
>>>>
>>>> Should probably be a QOM property, no ?
>>>
>>> Did not want to mess with dynamic FIFO buffer allocation.
>>
>> You probably should, since this is configurable at the FPGA level (in
>> QSys), right ?
>>
>
> OK, will implement fifo size as a property
>
>
> Thanks
> Juro
>
--
Best regards,
Marek Vasut
