On 4/28/25 6:10 PM, BALATON Zoltan wrote:
On Mon, 28 Apr 2025, Pierrick Bouvier wrote:
On 4/28/25 11:44 AM, BALATON Zoltan wrote:
On Mon, 28 Apr 2025, Pierrick Bouvier wrote:
On 4/28/25 3:31 AM, BALATON Zoltan wrote:
Since you are touching the lines using DEFINE_MACHINE it's a good
opportunity to change the macro to be more general to be able to keep
using it instead of replacing it with the boiler plate it's supposed to
hide. Adding one or two more parameters to the macro is not a big change
so I don't see why you don't want to do it. This could be addressed later
to revert to use the macro again but in practice it will not be addressed
because everybody will be busy doing other things and doing that now
would
prevent some churn. I too, don't like doing unrelated clean up which is
not the main goal, but if it's not much more work then it's not
unreasonable to do it. I only oppose to that if it's a lot of work so I
would not ask such change but what I asked is not unrelated and quite
simple change.
That said, I can't stop you so if you still don't want to do it now then
you can move on. I don't care that much as long as you stay within
hw/arm,
but will raise my concern again when you submit a similar patch that
touches parts I care more about. If others don't think it's a problem and
not bothered by the boiler plate code then it's not so important but
otherwise I think I have a valid point. I remember when I started to get
to know QEMU it was quite difficult to wade through all the QOM boiler
plate just to see what is related to the actual functionality. These
macros help to make code more readable and accessible for new people.
Having been through that recently, I agree with you that it can be hard to
follow at first. Luckily, we have perfect compiler based completion for
all
editors those days (I sincerely hope everyone spent 2 hours configuring
this
on their own favorite one), and it's easy to see where things are defined
and
used, even when code is cryptic.
It's not about typing but reading it. The verbose struct definitions are
hard to follow and makes board code look more complex than it should be.
That said, pushing to someone adding a new field the responsibility of
cleaning up the whole thing is not a fair request. You can't expect your
friends to clean your shared house because they brought a cake for dinner.
I tend to get such requests to clean up unrelated things whenever I try to
change anything in PPC Mac emulation which I also complain about and think
is not reasonable to ask. But I did not ask for unrelated cleanup here and
changing the patch so you don't do this:
-DEFINE_MACHINE("none", machine_none_machine_init)
+static const TypeInfo null_machine_types[] = {
+ {
+ .name = MACHINE_TYPE_NAME("none"),
+ .parent = TYPE_MACHINE,
+ .class_init = null_machine_class_init,
+ },
+};
+
+DEFINE_TYPES(null_machine_types)
but instead add the .interfaces field to a variant of DEFINE_MACHINE once
and keep the one line definition is not something unreasonable to ask. I
think you can ask your friends to not make a mess in the shared house
while having a party or at least clean up after that. Adding one more
parameter to the macro is also simple to do so I don't get why you're so
opposed to this.
Maybe there is a misunderstanding on my side, but it seems that what you
asked is exactly patch 7, which introduce DEFINE_MACHINE_WITH_INTERFACES.
Almost but not quite. I don't know why I can't get this through to you. If
you compare patch 7 to how DO_OBJECT_DEFINE_TYPE_EXTENDED is defined do
you notice the difference in how .interfaces is set? With the same way as
in DO_OBJECT_DEFINE_TYPE_EXTENDED you don't need separate InterfaceInfo
arm_aarch64_machine_interfaces[] definitions or different macros in the
next patch just list the needed interfaces in the machine definitions.
I'm sorry, I don't understand what you want exactly, despite asking
several times.
I think it would be more clear if you could apply this series on your
side, write a small patch showing *exactly* what you expect, and
applying this to one of the board concerned. Then, we can do the change
you request.
Regards,
BALATON Zoltan
