Re: [Qemu-devel] [PATCH v3 00/16] data-driven device registers

[Alistair Francis <alistair.francis@xilinx.com>]

On Tue, Feb 9, 2016 at 9:22 AM, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Alistair Francis <alistair.francis@xilinx.com> writes:
>
>> This patch series is based on Peter C's original register API. His
>> original cover letter is below.
>
> OK that's my first pass review. I seem to be missing 16/16 in my inbox
> though.

Thanks for going through it. I really appreciate it. I have problems
with all of my emails going through, you should get it this time.

>
> My initial thoughts are it seems pretty useful from a validation point
> of view. I'm a little uncomfortable with so much of the validation being
> hidden behind debug statements though. I wonder if they should always be
> checked but only optionally printed. Otherwise stuff is liable to
> bit-rot until you turn on debug.

I have addressed some of those in V4, let me know if you see any other
problem areas.

>
> Also I'm not super crazy about the macro stuff. I can see what you are
> trying to get to but I wonder if there is a neater way of defining this.
> Obviously with C being what it is it could well be it is the least ugly
> solution.

I see what you mean and the macro magic does take some getting used
to, but I don't have a better solution. If you do have one I can do it
that way instead.

>
> The case for this may be improved if in addition to new devices and
> existing device could be converted to the data driven style. That would
> give a nicer old-style <-> new-style comparison.
>
> Once you've had a chance to go through the comments and fix up the
> compile please CC me on future patches and I'll give the tyres a runtime
> kicking ;-)

Sending out V4 now, doesn't do us any good having something that
doesn't compile on list.

Thanks,

Alistair

>
> Cheers,
>
> --
> Alex Bennée
>