On 29/10/13 14:32, Peter Maydell wrote:
On 29 October 2013 07:57, Markus Armbruster wrote:
sun4m.c and sun4u.c both define a qdev named TYPE_OPENPROM. As far as I
can tell, they differ only in the name of their memory region.
If they ever get linked into the same executable,
type_register_internal() will reject the second one. But they aren't,
so this isn't a bug. Is it bad style?
Yes, in several ways. Board models shouldn't include qdev devices
in their source files -- the devices should have their own source files
in the appropriate section of hw/. We shouldn't have multiple devices
of the same TYPE like that. And the device should probably be built
once rather than per-target.
I also suspect this device is jumping through hoops to deal with the
fact that our image-loading API doesn't have any "load this image
into this memory region" function (compare discussion recently
on how the sbus graphics card patches handle loading their prom
image). Fixing that might also simplify these devices.
From my time spent in the sun4m code recently, there are really just
two devices at the moment: one represents the RAM within the 36-bit
address space and the other represents the PROM located above the RAM
region.
As Peter rightly points out, the TYPE_OPENPROM stuff mainly appears to
be a hack to allow the ROM to be loaded into a MemoryRegion which is
then usable by the sysbus API. So if someone could come up with suitable
"load this image into this memory region" routine to replace
load_image_targphys() then this particular problem would go away.
ATB,
Mark.
