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

[Philippe Mathieu-Daudé]

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

I guess you misread the patch context, pasting it again:

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

Pasting with more context and lines numbered:

 283 RENESAS RX CPUs
 284 M: Yoshinori Sato <ysato@users.sourceforge.jp>
 285 S: Maintained
 286 F: target/rx/
 287
 288 S390 TCG CPUs
 289 M: Richard Henderson <rth@twiddle.net>
 290 M: David Hildenbrand <david@redhat.com>
 291 S: Maintained
 292 F: target/s390x/
 293 F: hw/s390x/
 294 F: disas/s390.c
 295 F: tests/tcg/s390x/
 296 L: qemu-s390x@nongnu.org
 297
 298 SH4 TCG CPUs
 299 M: Aurelien Jarno <aurelien@aurel32.net>
 300 S: Odd Fixes
 301 F: target/sh4/
 302 F: disas/sh4.c

^ This part is the TCG frontend.

 303
 304 SPARC TCG CPUs
 305 M: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
 306 M: Artyom Tarasenko <atar4qemu@gmail.com>
 307 S: Maintained
 308 F: target/sparc/
 309 F: hw/sparc/
 310 F: hw/sparc64/
 311 F: include/hw/sparc/sparc64.h
 312 F: disas/sparc.c
 313
 314 UniCore32 TCG CPUs
 315 M: Guan Xuetao <gxt@mprc.pku.edu.cn>
 316 S: Maintained
 317 F: target/unicore32/
 318 F: hw/unicore32/
 319 F: include/hw/unicore32/
 ...

1240 RISC-V Machines
1241 ---------------
1242 OpenTitan
1243 M: Alistair Francis <Alistair.Francis@wdc.com>
1244 L: qemu-riscv@nongnu.org
1245 S: Supported
1246 F: hw/riscv/opentitan.c
1247 F: include/hw/riscv/opentitan.h
1248
1249 SH4 Machines
1250 ------------
1251 R2D
1252 M: Magnus Damm <magnus.damm@gmail.com>
1253 S: Maintained
1254 F: hw/sh4/r2d.c
1255 F: hw/intc/sh_intc.c
1256 F: hw/timer/sh_timer.c

^ a SH4-based machine

1257
1258 Shix
1259 M: Magnus Damm <magnus.damm@gmail.com>
1260 S: Odd Fixes
1261 F: hw/sh4/shix.c
1262
1263 SPARC Machines
1264 --------------
1265 Sun4m
1266 M: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
1267 S: Maintained
1268 F: hw/sparc/sun4m.c
1269 F: hw/sparc/sun4m_iommu.c
....

1455 Devices
1456 -------
1457 EDU
1458 M: Jiri Slaby <jslaby@suse.cz>
1459 S: Maintained
1460 F: hw/misc/edu.c
1461
1462 IDE
1463 M: John Snow <jsnow@redhat.com>
1464 L: qemu-block@nongnu.org
1465 S: Supported
1466 F: include/hw/ide.h
1467 F: include/hw/ide/
1468 F: hw/ide/
....

1919 PIIX4 South Bridge (i82371AB)
1920 M: Hervé Poussineau <hpoussin@reactos.org>
1921 M: Philippe Mathieu-Daudé <f4bug@amsat.org>
1922 S: Maintained
1923 F: hw/isa/piix4.c
1924 F: include/hw/southbridge/piix.h
1925
1926 Firmware configuration (fw_cfg)
1927 M: Philippe Mathieu-Daudé <philmd@redhat.com>
1928 R: Laszlo Ersek <lersek@redhat.com>
1929 R: Gerd Hoffmann <kraxel@redhat.com>
1930 S: Supported
1931 F: docs/specs/fw_cfg.txt
1932 F: hw/nvram/fw_cfg.c
1933 F: stubs/fw_cfg.c
1934 F: include/hw/nvram/fw_cfg.h
1935 F: include/standard-headers/linux/qemu_fw_cfg.h
1936 F: tests/qtest/libqos/fw_cfg.c
1937 F: tests/qtest/fw_cfg-test.c
1938 T: git https://github.com/philmd/qemu.git fw_cfg-next
1939
1940 XIVE
1941 M: David Gibson <david@gibson.dropbear.id.au>
1942 M: Cédric Le Goater <clg@kaod.org>
1943 L: qemu-ppc@nongnu.org
1944 S: Supported
1945 F: hw/*/*xive*
1946 F: include/hw/*/*xive*
1947 F: docs/*/*xive*
1948
1949 SH4 Hardware
1950 S: Orphan
1951 F: hw/sh4/
1952 F: hw/char/sh_serial.c
1953 F: hw/intc/sh_intc.c
1954 F: hw/timer/sh_timer.c
1955 F: include/hw/sh4/

^ The section is placed under the "Devices" category.

1956
1957 Subsystems
1958 ----------
1959 Audio
1960 M: Gerd Hoffmann <kraxel@redhat.com>
1961 S: Maintained
1962 F: audio/
....

Where do you think this entry should be placed?

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