From: "Alex Bennée" <alex.bennee@linaro.org>
To: Alistair Francis <alistair.francis@xilinx.com>
Cc: edgar.iglesias@xilinx.com, peter.maydell@linaro.org,
qemu-devel@nongnu.org, crosthwaitepeter@gmail.com,
edgar.iglesias@gmail.com, afaerber@suse.de,
fred.konrad@greensocs.com
Subject: Re: [Qemu-devel] [PATCH v3 00/16] data-driven device registers
Date: Tue, 09 Feb 2016 17:22:13 +0000 [thread overview]
Message-ID: <8760xxx0hm.fsf@linaro.org> (raw)
In-Reply-To: <cover.1454115217.git.alistair.francis@xilinx.com>
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.
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.
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.
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 ;-)
Cheers,
--
Alex Bennée
next prev parent reply other threads:[~2016-02-09 17:22 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-30 1:00 [Qemu-devel] [PATCH v3 00/16] data-driven device registers Alistair Francis
2016-01-30 1:00 ` [Qemu-devel] [PATCH v3 01/16] memory: Allow subregions to not be printed by info mtree Alistair Francis
2016-01-30 1:00 ` [Qemu-devel] [PATCH v3 02/16] register: Add Register API Alistair Francis
2016-02-09 16:06 ` Alex Bennée
2016-02-09 19:35 ` Alistair Francis
2016-02-09 22:29 ` Peter Crosthwaite
2016-01-30 1:00 ` [Qemu-devel] [PATCH v3 03/16] register: Add Memory API glue Alistair Francis
2016-01-30 1:00 ` [Qemu-devel] [PATCH v3 04/16] register: Add support for decoding information Alistair Francis
2016-01-30 1:00 ` [Qemu-devel] [PATCH v3 05/16] register: Define REG and FIELD macros Alistair Francis
2016-01-30 1:00 ` [Qemu-devel] [PATCH v3 06/16] register: QOMify Alistair Francis
2016-02-09 11:49 ` Alex Bennée
2016-02-09 18:09 ` Alistair Francis
2016-01-30 1:00 ` [Qemu-devel] [PATCH v3 07/16] register: Add block initialise helper Alistair Francis
2016-02-09 16:12 ` Alex Bennée
2016-02-09 19:50 ` Alistair Francis
2016-01-30 1:01 ` [Qemu-devel] [PATCH v3 08/16] bitops: Add ONES macro Alistair Francis
2016-01-30 1:01 ` [Qemu-devel] [PATCH v3 09/16] dma: Add Xilinx Zynq devcfg device model Alistair Francis
2016-02-09 17:08 ` Alex Bennée
2016-02-09 21:47 ` Alistair Francis
2016-01-30 1:01 ` [Qemu-devel] [PATCH v3 10/16] xilinx_zynq: add devcfg to machine model Alistair Francis
2016-01-30 1:01 ` [Qemu-devel] [PATCH v3 11/16] qdev: Define qdev_get_gpio_out Alistair Francis
2016-01-30 1:01 ` [Qemu-devel] [PATCH v3 12/16] qdev: Add qdev_pass_all_gpios API Alistair Francis
2016-01-30 1:01 ` [Qemu-devel] [PATCH v3 13/16] irq: Add opaque setter routine Alistair Francis
2016-01-30 1:01 ` [Qemu-devel] [PATCH v3 14/16] register: Add GPIO API Alistair Francis
2016-01-30 1:01 ` [Qemu-devel] [PATCH v3 15/16] misc: Introduce ZynqMP IOU SLCR Alistair Francis
2016-02-09 17:22 ` Alex Bennée [this message]
2016-02-09 21:56 ` [Qemu-devel] [PATCH v3 00/16] data-driven device registers Alistair Francis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8760xxx0hm.fsf@linaro.org \
--to=alex.bennee@linaro.org \
--cc=afaerber@suse.de \
--cc=alistair.francis@xilinx.com \
--cc=crosthwaitepeter@gmail.com \
--cc=edgar.iglesias@gmail.com \
--cc=edgar.iglesias@xilinx.com \
--cc=fred.konrad@greensocs.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).