On Mon, 26 Feb 2024, Philippe Mathieu-Daudé wrote:
On 26/2/24 14:01, BALATON Zoltan wrote:
On Mon, 26 Feb 2024, Philippe Mathieu-Daudé wrote:
Expose TYPE_ICH_DMI_PCI_BRIDGE to the new
"hw/pci-bridge/ich9_dmi.h" header.
Since this is effectively an empty object (that's not even instantiated by
default) I still think that instead of adding even more files for it all
this could just be moved to hw/isa/lpc_ich9.c and define there as an
internal object and drop the OBJECT_DECLARE_SIMPLE_TYPE(I82801b11Bridge,
ICH_DMI_PCI_BRIDGE) and just use the size of the superclass as it's
instance size. That just adds the realize function and a type definition
and gets rid of boilerplate scattered around the source tree which just
adds complexity for no reason. But I don't care too much about it, just
wanted to say again that if something can be kept simple I'd prefer that
over making it more complex and for this device it looks already too
complex for what it does or used for.
My understanding was project coherency and style is preferred over
simplifications / optimizations, so we prefer explicit TYPE_FOO
and corresponding OBJECT_DECLARE_XXX() in a public include/hw/foo
header -- because it will end up copy/pasted --, but I might be
wrong.
For classes that are actually used that may be true but this class isn't
used anywhere just defined for the user to be instantiated from command
line or config. So you won't even need a type name in code currently. It's
also an internal part of ICH southbridge so no need to be exposed outside
of that as it's only useful within that chip. Finally it has no
functionality, just an empty boilerplate so that the device appears in
lspci output but it does nothing. Therefore, I think it's simpler to just
move all of it within the same file where the southbridge is implemented
and define as empty object there like the via-mc97 device in
hw/audio/via-ac97.c which serves the same purpose to show the part to the
guest but its empty device without function. That would save half of all
this boilerplate that just adds complexity now. If sometimes in the future
this device gains some functionality then it can be split off and add all
the defines and stuff around it but that's not needed yet so why not keep
it simple?
Regards,
BALATON Zoltan