qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Gerd Hoffmann <kraxel@redhat.com>
To: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Cc: Blue Swirl <blauwirbel@gmail.com>,
	Paul Brook <paul@codesourcery.com>,
	qemu-devel@nongnu.org, Markus Armbruster <armbru@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 1/6] Move function definitions out of xilinx.h
Date: Mon, 14 Sep 2009 10:57:25 +0200	[thread overview]
Message-ID: <4AAE0575.8060607@redhat.com> (raw)
In-Reply-To: <20090912101009.GC16110@laped.iglesias.mooo.com>

On 09/12/09 12:10, Edgar E. Iglesias wrote:
> On Sat, Sep 12, 2009 at 09:04:17AM +0300, Blue Swirl wrote:
>> On Fri, Sep 11, 2009 at 11:19 PM, Markus Armbruster<armbru@redhat.com>  wrote:
>>> xilinx.h defines a couple of static inline functions for creating
>>> devices.  While that's a fair technique for hot functions, device
>>> initialization is about as cold as it gets.  Define them in the device
>>> source files instead, and keep only declarations in the header.
>>
>> If I understood the qdev plan correctly, this is going to wrong
>> direction. These functions should reside near the instantiation, not
>> in the device code. The current approach looks OK if there are going
>> to be more users of the devices.

The functions should go away ;)

Some day the information carried by those code snippeds should come from 
a machine description file, then we'll don't need them any more.

> I agree that they shouldn't be in the device source.
> The reason they ended up in a header and not with the petalogix board
> was that in my tree there are multiple boards using these functions
> to easy instantiate devices.

They have to be somewhere.  Having them in a header file is unclean. 
Having them in the board-specific code isn't practical when multiple 
boards share the code.  I'd stick them to the device source code as 
well.  Also note that this is common practice elsewhere in the tree.

cheers
   Gerd

  reply	other threads:[~2009-09-14  8:57 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-11 20:19 [Qemu-devel] [PATCH 0/6] Clean up use of qdev_init() Markus Armbruster
2009-09-11 20:19 ` [Qemu-devel] [PATCH 1/6] Move function definitions out of xilinx.h Markus Armbruster
2009-09-12  6:04   ` Blue Swirl
2009-09-12 10:10     ` Edgar E. Iglesias
2009-09-14  8:57       ` Gerd Hoffmann [this message]
2009-09-14  9:15         ` Edgar E. Iglesias
2009-09-14 16:18           ` Blue Swirl
2009-09-14 18:59             ` Markus Armbruster
2009-09-14 19:24               ` Blue Swirl
2009-09-11 20:19 ` [Qemu-devel] [PATCH 2/6] Make qdev_init() destroy the device on failure Markus Armbruster
2009-09-11 20:19 ` [Qemu-devel] [PATCH 3/6] Check return value of qdev_init() Markus Armbruster
2009-09-11 20:19 ` [Qemu-devel] [PATCH 4/6] New qdev_init_nofail() Markus Armbruster
2009-09-11 20:19 ` [Qemu-devel] [PATCH 5/6] Make isa_create() terminate program on failure Markus Armbruster
2009-09-11 20:19 ` [Qemu-devel] [PATCH 6/6] Warn if value of qdev_init() isn't checked Markus Armbruster
2009-09-14  8:43 ` [Qemu-devel] [PATCH 0/6] Clean up use of qdev_init() Gerd Hoffmann

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=4AAE0575.8060607@redhat.com \
    --to=kraxel@redhat.com \
    --cc=armbru@redhat.com \
    --cc=blauwirbel@gmail.com \
    --cc=edgar.iglesias@gmail.com \
    --cc=paul@codesourcery.com \
    --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).