Re: [PATCH v2 1/8] MAINTAINERS: Mark SH4 hardware orphan

[Aleksandar Markovic]

сре, 10. јун 2020. у 14:06 Philippe Mathieu-Daudé <f4bug@amsat.org> је
написао/ла:
>
> On 6/10/20 1:08 PM, Aleksandar Markovic wrote:
> > пон, 8. јун 2020. у 11:05 Philippe Mathieu-Daudé <f4bug@amsat.org> је
> > написао/ла:
> >>
> >> Aurelien Jarno expressed his desire to orphan the SH4 hardware [*]:
> >>
> >>   I don't mind being [...] removed from there.
> >>   I do not really have time to work on that.
> >>
> >> Mark the SH4 emulated hardware orphan.
> >>
> >> Many thanks to Aurelien for his substantial contributions to QEMU,
> >> and for maintaining the SH4 hardware for various years!
> >>
> >> [*] https://www.mail-archive.com/qemu-devel@nongnu.org/msg708400.html
> >>
> >> Message-Id: <20200601214125.GA1924990@aurel32.net>
> >> Acked-by: Aurelien Jarno <aurelien@aurel32.net>
> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> >> ---
> >
> > The basic idea of the patch (as read from the title and the commit
> > message) is good and positive.
> >
> > The problem is that the patch does something different than the commit
> > message says - pretending that it just orphans something. Which is not
> > good. Actually, very clumsy and bad.
>
> I can improve my English (actually I am trying, I started to take
> lessons 2 years ago). Remember it is not easy for non native English
> speaker to elaborate non-technical topics.
> As a side note, it is also hard sometimes to understand contributors who
> uses a very advanced English level with fancy words, as we have to take
> the time to translate to understand. Even if we learn new word, this is
> not often the best use of our time.
>
> > It creates a whole new subsection in MAINTAINERS file (not said in the
> > commit message), without any consistency with the current organization
> > in the file. That new subsection looks completely misplaced, living
> > with "TCG CPUs" neighbours. On top of that, it creates a new
> > precedent, leaving many unanswered questions, like: Should other
> > targets follow the same pattern?
>
> If you look at git-log history, you'll see at the beginning there
> were not much separations in directories. Everything was altogether.
> Adding a new machine meant add the TCG emulation code and the hardware.
> The contributor adding TCG emulation was doing the harder work,
> usually doing it for a particular machine (hardware). Then other
> contributors could add other machines, and were maintaining only
> their machines. See commit b6f97c14f59 importing the short
> MAINTAINERS from SVN. It was already split in 3 broad categories:
> - TCG cores
> - Machines / Hardware
> - Subsystems (the rest)
>
> In October 2010 (fd5d5c566af0) Anthony Liguori rewrote MAINTAINERS
> using Linux style tags.
>
> In January 2011 (42f5a7e9367) Aurelien Jarno clarified the
> difference between the TCG host part (backend, under the tcg/
> directory) VS the target part (frontend, under target/).
>
> Then the project grew, and eventually in May 2012 Paolo Bonzini
> started to clean, by moving files into "subsystem" directories
> (see 5e8861a0361..c353f261946). In March 2013 he followed by
> splitting various hardware files from the hw/ directory
> (see 0d09e41a51a..47b43a1f414).
>
> In November 2013 (f05b328c9d8) Stefan Hajnoczi clarified the
> importance of the 'block' subsystem (later completed in
> commit e7c6e631b1 in April 2015).
>
> 2015 still, Paolo keeps cleaning (c17652ee409 cover the
> TCG backend disassembler, 062710000db introduce Peter
> Maydell for the ARM hardware).
>
> In October 2018 Thomas Huth added the target/ folder
> (commit fcf5ef2ab).
>
> Recently last year (6347e1f1cc7) Markus Armbruster tried
> to better describe the TCG frontend ("Overall TCG CPUs")
> and backend ("TCG target") differences.
>
>
> So while the difference between TCG emulation (or other
> accelerators, KVM, ...) versus the hardware emulation is
> not perfect, there is a separation (and the community
> failed at clarifying it).
>
> You can see when you were introduced as maintainer for the
> MIPS target (commit c92023bfd) you were also added as
> maintainer of the MIPSSIM and Fuloong 2E machines.
>
> In commit 485cd9820 you volunteered to maintain the Malta
> machine, so I believe you have a good understanding about
> the difference between target code and hardware code.
>
> >
> > I personally think that creating a new subsection is just a code
> > churn, waste of everybody's time on unimportant things.
>
> Eh... I read that after spending 1h writing the previous
> paragraph. Yes, you are right.
>
> >
> > Wouldn't it be simpler that you just changed statuses of all Aurelien
> > sh4 sections to "Orphaned", as he already said he approves, and leave
> > sh4 sections reorganization to a future maintainer?
> >
> > If you really want to reorganize sh4 sections, these changes should be
> > in a separate patch. "Orphaning" patch should contain only changes of
> > statuses.
>
> Good idea! Maybe if you send a patch to show me how to do it, it
> will be easier to understand what you mean.
>
> Thanks for also caring about the SH4 hardware, the code was
> prone to bitrot and needed maintainer to look after it.
>

Thank you very much for the analysis above, Philippe.

I think the whole thing is blown out of proportions, possibly to a
great degree by my fault.

Again, I think it is this simple: Yoshimori has the best conditions
among us for maintaining rx and sh4, he needs it, he wants it - and we
just give it to him.

Nothing more and nothing less.

Regards,
Aleksandar

> Regards,
>
> Phil.
>
> >
> > Regards,
> > Aleksandar
> >
> >>  MAINTAINERS | 10 ++++++++--
> >>  1 file changed, 8 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index 6e7890ce82..49d90c70de 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -299,9 +299,7 @@ SH4 TCG CPUs
> >>  M: Aurelien Jarno <aurelien@aurel32.net>
> >>  S: Odd Fixes
> >>  F: target/sh4/
> >> -F: hw/sh4/
> >>  F: disas/sh4.c
> >> -F: include/hw/sh4/
> >>
> >>  SPARC TCG CPUs
> >>  M: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> >> @@ -1948,6 +1946,14 @@ F: hw/*/*xive*
> >>  F: include/hw/*/*xive*
> >>  F: docs/*/*xive*
> >>
> >> +SH4 Hardware
> >> +S: Orphan
> >> +F: hw/sh4/
> >> +F: hw/char/sh_serial.c
> >> +F: hw/intc/sh_intc.c
> >> +F: hw/timer/sh_timer.c
> >> +F: include/hw/sh4/
> >> +
> >>  Subsystems
> >>  ----------
> >>  Audio
> >> --
> >> 2.21.3
> >>
> >>
> >