From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:41135)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <xbing6@gmail.com>) id 1WKnw8-0001SF-8d
	for qemu-devel@nongnu.org; Tue, 04 Mar 2014 06:55:12 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <xbing6@gmail.com>) id 1WKnvz-0005GT-Rf
	for qemu-devel@nongnu.org; Tue, 04 Mar 2014 06:55:04 -0500
Received: from mail-pa0-x22b.google.com ([2607:f8b0:400e:c03::22b]:54031)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <xbing6@gmail.com>) id 1WKnvz-0005GL-KJ
	for qemu-devel@nongnu.org; Tue, 04 Mar 2014 06:54:55 -0500
Received: by mail-pa0-f43.google.com with SMTP id bj1so5100920pad.2
	for <qemu-devel@nongnu.org>; Tue, 04 Mar 2014 03:54:54 -0800 (PST)
Message-ID: <5315BF05.2060500@gmail.com>
Date: Tue, 04 Mar 2014 19:54:45 +0800
From: Xuebing wang <xbing6@gmail.com>
MIME-Version: 1.0
References: <1393901250-3922-1-git-send-email-xbing6@gmail.com>
	<1393901250-3922-3-git-send-email-xbing6@gmail.com>
	<5315A89F.40700@redhat.com>
In-Reply-To: <5315A89F.40700@redhat.com>
Content-Type: text/plain; charset=ISO-8859-15; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [Discussion 02/10] NEED_CPU_H: remove '#include
 "cpu.h"' from include/qemu-common.h
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org
Cc: afaerber@suse.de, stefanha@redhat.com

Hi Paolo, thanks for reviewing.


On 03/04/2014 06:19 PM, Paolo Bonzini wrote:
> Hi,
>
> in general I agree with this patch.  I have a few comments, and I 
> suggest that you split it in multiple patches so that it's easier to 
> get it in when each part is ready.

I spent some time trying to reduce it. I may be wrong, but this is the 
smallest patch I can get to get all targets (linuxuser vs softmmu, kvm 
enable/disable) build.

>
>> diff --git a/hw/dma/soc_dma.c b/hw/dma/soc_dma.c
>> index c06aabb..950f3ec 100644
>> --- a/hw/dma/soc_dma.c
>> +++ b/hw/dma/soc_dma.c
>> @@ -21,6 +21,11 @@
>>  #include "qemu/timer.h"
>>  #include "hw/arm/soc_dma.h"
>>
>> +#ifndef NEED_CPU_H
>> +#error target-xxx/cpu.h must be included because target-specific are 
>> required
>> +#endif
>
> Not needed; include/exec/cpu-defs.h already has a similar #error.

I guess this is my personal preference.

My original idea is to emphasize that this *whole* file is 
"target-specific", as "target-independent" vs "target-specific" is 
important for qemu framework (everything except target-xxx, tcg backend 
and hw/arch*/). Ideally all qemu framework is designed to be 
target-independent, right?

In my docs/api-hierarchy.txt, I am trying my best to illustrate what are 
"target-independent", and what are "target-specific".

cpu-defs.h is NOT included in this file.

The idea is the same for the rest "#ifndef NEED_CPU_H"

     Why use "#ifndef NEED_CPU_H, then #error ...", is because 
"target-xxx/cpu.h"
     is better included for the whole file.

     If "target-xxx/cpu.h" is only needed for part of the file, use
     "#ifdef NEED_CPU_H, then #include "cpu.h"".


Thanks.