[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/7] nios2: Add periodic timer emulation
From: |
Marek Vasut |
Subject: |
Re: [Qemu-devel] [PATCH 5/7] nios2: Add periodic timer emulation |
Date: |
Fri, 19 Aug 2016 01:24:11 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.2.0 |
On 08/18/2016 11:49 AM, Dmitry Osipenko wrote:
> On 17.08.2016 23:19, Marek Vasut wrote:
>> On 08/16/2016 11:38 PM, Dmitry Osipenko wrote:
>>
>> [...]
>>
>>>>>> Well what is sane clock frequency for hardware which can have arbitrary
>>>>>> frequency configured in ?
>>>>>>
>>>>>
>>>>> You could set to the one that is used by "10M50 GHRD" patch for example.
>>>>
>>>> That doesn't sound right . I can set it to 1 (Hz) , that should make it
>>>> slow enough to signalize that something is broken while providing valid
>>>> number.
>>>>
>>>
>>> I'm not sure about it. Explicitly showing that something is wrong would be
>>> better.
>>>
>>>> +static void altera_timer_realize(DeviceState *dev, Error **errp)
>>>> +{
>>>> + AlteraTimer *t = ALTERA_TIMER(dev);
>>>> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>>> +
>>>> + assert(t->freq_hz != 0);
>>>
>>> If you would prefer to keep error'ing out, then I can suggest to add some
>>> verbose message instead of the assertion, like:
>>>
>>> if (!t->freq_hz) {
>>> error_setg(errp, "altera_timer: \"clock-frequency\" property " \
>>> "must be provided");
>>> return;
>>> }
>>
>> That's better, thanks.
>>
>> btw breaking strings is frowned upon in linux/u-boot as it makes it
>> impossible to git grep for error message. Does the same apply to qemu?
>>
>
> Actually, "altera_timer: " prefix isn't needed, as it should be already
> included
> in the error message produced by by the error_setg(), so that string could be
> squeezed into one line. And, of course, it could be rephrased more concisely.
Even better, prefix dropped. Thanks
So what about patches 1..5 ? Anything I should change there ?
--
Best regards,
Marek Vasut
- Re: [Qemu-devel] [PATCH 5/7] nios2: Add periodic timer emulation, (continued)
- Re: [Qemu-devel] [PATCH 5/7] nios2: Add periodic timer emulation, Marek Vasut, 2016/08/12
- Re: [Qemu-devel] [PATCH 5/7] nios2: Add periodic timer emulation, Dmitry Osipenko, 2016/08/15
- Re: [Qemu-devel] [PATCH 5/7] nios2: Add periodic timer emulation, Marek Vasut, 2016/08/16
- Re: [Qemu-devel] [PATCH 5/7] nios2: Add periodic timer emulation, Dmitry Osipenko, 2016/08/16
- Re: [Qemu-devel] [PATCH 5/7] nios2: Add periodic timer emulation, Marek Vasut, 2016/08/16
- Re: [Qemu-devel] [PATCH 5/7] nios2: Add periodic timer emulation, Dmitry Osipenko, 2016/08/16
- Re: [Qemu-devel] [PATCH 5/7] nios2: Add periodic timer emulation, Marek Vasut, 2016/08/17
- Re: [Qemu-devel] [PATCH 5/7] nios2: Add periodic timer emulation, Peter Maydell, 2016/08/17
- Re: [Qemu-devel] [PATCH 5/7] nios2: Add periodic timer emulation, Marek Vasut, 2016/08/17
- Re: [Qemu-devel] [PATCH 5/7] nios2: Add periodic timer emulation, Dmitry Osipenko, 2016/08/18
- Re: [Qemu-devel] [PATCH 5/7] nios2: Add periodic timer emulation,
Marek Vasut <=
- Re: [Qemu-devel] [PATCH 5/7] nios2: Add periodic timer emulation, Dmitry Osipenko, 2016/08/19
- Re: [Qemu-devel] [PATCH 5/7] nios2: Add periodic timer emulation, Marek Vasut, 2016/08/19