Re: [Qemu-devel] [PATCH 1/6] Move function definitions out of xilinx.h

[Blue Swirl]

On Mon, Sep 14, 2009 at 12:15 PM, Edgar E. Iglesias
<address@hidden> wrote:
> On Mon, Sep 14, 2009 at 10:57:25AM +0200, Gerd Hoffmann wrote:
>> 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<address@hidden>
>>>> 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.
>
>>
>>> 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.
>
> I disagree.
>
> But if ppl feel very strongly about this, I can remove them and deal with
> the code duplication in my tree. Afterall, it's unlikely that upsteam
> qemu gets more xilinx boards before some kind of device tree driven
> board support is there.

I also disagree. Why would the functions in a header file be unclean?
The common practice is wrong.