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

[Aleksandar Markovic]

пон, 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.

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?

I personally think that creating a new subsection is just a code
churn, waste of everybody's time on unimportant things.

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.

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
>
>