Re: [Qemu-devel] [PATCH 8/9] unicore32-softmmu: add config and makefile support

["Andreas Färber" <afaerber@suse.de>]

Am 28.05.2012 12:08, schrieb guanxuetao@mprc.pku.edu.cn:
>> Am 25.05.2012 13:29, schrieb Guan Xuetao:
>>> This patch adds configure and makefile support for unicore32-softmmu.
>>> All puv3-soc devices are put into hw/pkunity directory, so this dir
>>> will be added when unicore32-softmmu is selected.
>>>
>>> Signed-off-by: Guan Xuetao <gxt@mprc.pku.edu.cn>
>>> ---
>>>  Makefile.target                       |    5 +++++
>>>  arch_init.c                           |    2 ++
>>>  arch_init.h                           |    1 +
>>>  configure                             |    4 ++++
>>>  default-configs/unicore32-softmmu.mak |    4 ++++
>>>  5 files changed, 16 insertions(+), 0 deletions(-)
>>>  create mode 100644 default-configs/unicore32-softmmu.mak
>>>
>>> diff --git a/Makefile.target b/Makefile.target
>>> index 1582904..2f850d3 100644
>>> --- a/Makefile.target
>>> +++ b/Makefile.target
>>> @@ -387,6 +387,11 @@ obj-xtensa-y += core-dc232b.o
>>>  obj-xtensa-y += core-dc233c.o
>>>  obj-xtensa-y += core-fsf.o
>>>
>>> +obj-unicore32-y += uc32_softmmu.o
>>> +obj-unicore32-y += pkunity/puv3.o
>>> +obj-unicore32-y += pkunity/puv3_intc.o pkunity/puv3_ost.o
>>> pkunity/puv3_gpio.o
>>> +obj-unicore32-y += pkunity/puv3_pm.o pkunity/puv3_dma.o
>> [snip]
>>
>> You need to put the Makefile/configure changes into the patches that
>> introduce the files please, otherwise they cannot be checked for
>> compiler warnings/errors.
>>
> I think the patch series is considered as a whole, and only
> compiling/building one device sim-code doesn't make sense. In fact, when
> unicore32-softmmu not enabled, the device sim-code isn't going to be
> compiled at all.

Well, we expect each patch in a series to build warning-free for
bisectability (even if applied in one PULL), and only compiling things
in the final patch does not help. The series should be ordered so that
we can either manually enable it with --target-list=unicore32-softmmu
until it finally gets enabled by default, or like the openrisc target
enables itself by default with some stubs and refines itself over the
next patches.

The other aspect is to make it easy for humans to review your patches
before they can get applied, and personally I find it easier to review
small patches. But opinions are divided on that. The criteria for
acceptance is not just whether your kernel works in the end [*], my
concern is more about how its design aligns with upstream trends like
QOM awareness.

Another thing: It is advisable to place SoC devices (as opposed to the
machine) into Makefile.objs (hw-unicore32-y) so that they get compiled
into libhw32 rather than into the target's directory. That avoids
duplicates when a second endianness or a 64-bit version is introduced.
(Yes, some existing targets violate that principle. I am working towards
fixing it.)

Andreas

[*] It would be helpful if you could share linux-user and softmmu
binaries on the Wiki for us to avoid regressions.
http://wiki.qemu.org/Testing

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg