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? >>>>>> For the IIC, I wonder what that'd look like -- probably >>>>>> qemu_set_irq(parent, 0); ? >>>>>> >>>>> >>>>> No, IRQ's would be "deasserted" by resetting CPUClass state, of which >>>>> QEMU takes >>>>> care itself. No action needed by the IIC. >>>> >>>> I see, thanks :) >>>> >>>> btw any chance someone can look at the other patches too ? >>>> >>>> >>> >>> >> >> > > -- Best regards, Marek Vasut