Re: [Qemu-devel] [RFC PATCH v1 2/4] m25p80: initial verion

[Andreas Färber]

Am 05.06.2012 02:47, schrieb Peter Crosthwaite:
> On Wed, Apr 4, 2012 at 10:53 PM, Andreas Färber <address@hidden> wrote:
>> Am 30.03.2012 08:37, schrieb Peter A. G. Crosthwaite:
>>> Added device model for m25p80 SPI flash
>>>
>>> Signed-off-by: Peter A. G. Crosthwaite <address@hidden>
>>> ---
>>>  Makefile.target |    1 +
>>>  hw/m25p80.c     |  495 
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 496 insertions(+), 0 deletions(-)
>>>  create mode 100644 hw/m25p80.c
>>>
>>> diff --git a/Makefile.target b/Makefile.target
>>> index 8fd3718..fcccf1b 100644
>>> --- a/Makefile.target
>>> +++ b/Makefile.target
>>> @@ -321,6 +321,7 @@ obj-microblaze-y = petalogix_s3adsp1800_mmu.o
>>>  obj-microblaze-y += petalogix_ml605_mmu.o
>>>  obj-microblaze-y += microblaze_boot.o
>>>  obj-microblaze-y += spi.o
>>> +obj-microblaze-y += m25p80.o
>>>
>>>  obj-microblaze-y += microblaze_pic_cpu.o
>>>  obj-microblaze-y += xilinx_intc.o
>>> diff --git a/hw/m25p80.c b/hw/m25p80.c
>>> new file mode 100644
>>> index 0000000..2b67375
>>> --- /dev/null
>>> +++ b/hw/m25p80.c
>>> @@ -0,0 +1,495 @@
>>> +/*
>>> + * ST M25P80 emulator.
>> [snip]
>>
>> A device by ST does not sound microblaze-specific and in that case,
>> similar to the recent Atmel maxtouch device, should go into
>> hw-obj-$(CONFIG_M25P80) instead so that it's compiled only once for
>> microblaze-softmmu and microblazeel-softmmu.
>>
> 
> Hi Andreas,
> 
> I have regenerated this series, but have not actioned this just yet. I
> think adding a config switch is probably not the right solution, as
> its just a device model. If every device model has a config switch
> then isnt creating everyone config process going to be excessively
> tedious? perhaps it can just live next to obj-y=ssi.o? I.E. if you
> have SSI, then you have m25p80. I cant think of a case where you would
> want SSI but need to exclude m25p80 from the build.

I think you're missing the point here. Whenever I touch, e.g.,
include/qemu/object.h, it takes an awfully long time to rebuild all
targets because, among others, you are unneccessarily duplicating device
model objects between microblaze and microblazeel. Devices by definition
do not depend on target endianness (they specify their endianness in
code) and should be built in libhw32/64. The mechanism to do so is
defining flags in default-configs/microblaze[el]-softmmu.mak. For
example, I had locally started a CONFIG_XILINX iirc, to share devices
between microblaze and ppc4xx, then there might be CONFIG_XILINX_PPC and
CONFIG_XILINX_MICROBLAZE for target-specific ones. My suggestion here
was to use CONFIG_M25P80 but if you have a better grouping that would be
fine, too. CONFIG_SSI maybe? Or if the amount of such devices is large
and to be shared across targets you might consider your own Makefile
like default-configs/pci.mak that can be included from multiple
*-softmmu.mak files.

Note however that Paolo has posted a series on the list refactoring the
whole Makefile system with which any such changes / additions clash.
Cc'ing Paolo so he can comment on the intended timing.

Andreas

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