Re: [Qemu-arm] [PATCH 03/13] hw: Move MC146818 device from hw/timer/ to hw/rtc/ subdirectory

[Philippe Mathieu-Daudé]

On 9/17/19 7:07 AM, Thomas Huth wrote:
> On 16/09/2019 17.48, Philippe Mathieu-Daudé wrote:
>> The MC146818 is a Real Time Clock, not a timer.
>> Move it under the hw/rtc/ subdirectory.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> [...]
>> diff --git a/include/hw/rtc/mc146818rtc.h b/include/hw/rtc/mc146818rtc.h
>> new file mode 100644
>> index 0000000000..888e04f9ab
>> --- /dev/null
>> +++ b/include/hw/rtc/mc146818rtc.h
>> @@ -0,0 +1,38 @@
>> +/*
>> + * QEMU MC146818 RTC emulation
>> + *
>> + * Copyright (c) 2003-2004 Fabrice Bellard
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a 
>> copy
>> + * of this software and associated documentation files (the "Software"), to 
>> deal
>> + * in the Software without restriction, including without limitation the 
>> rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included 
>> in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
>> OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
>> FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + */
> 
> If you run "git blame" on the old header file, it does not seem like
> Fabrice wrote this header, so I'm not sure whether it makes sense to add
> his (c) statement here?
> 
> Maybe rather use a one-line "SPDX-License-Identifier: GPL-2.0-or-later"
> here?

It was first added by Fabrice here:

$ git show 80cabfad163
[...]
> diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
new file mode 100644
index 0000000000..cab76cfab2
--- /dev/null
+++ b/hw/mc146818rtc.c
@@ -0,0 +1,203 @@
+/*
+ * QEMU MC146818 RTC emulation
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person
obtaining a copy
+ * of this software and associated documentation files (the
"Software"), to deal
+ * in the Software without restriction, including without limitation
the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or
sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be
included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
DEALINGS IN
+ * THE SOFTWARE.
+ */[...]
diff --git a/vl.h b/vl.h
index 35962d1985..026a5dee5a 100644
--- a/vl.h
+++ b/vl.h
+/* mc146818rtc.c */
+
+typedef struct RTCState {
+    uint8_t cmos_data[128];
+    uint8_t cmos_index;
+    int irq;
+} RTCState;
+
+extern RTCState rtc_state;
+
+void rtc_init(int base, int irq);
+void rtc_timer(void);

Then moved from vl.h to pc.h:

$ git show 87ecb68bdf8
commit 87ecb68bdf8a3e40ef885ddbb7ca1797dca40ebf
Author: pbrook <pbrook@c046a42c-6fe2-441c-8c8c-71466251a162>
Date:   Sat Nov 17 17:14:51 2007 +0000

    Break up vl.h.

Then moved from pc.h to mc146818rtc.h without adding the (c):

$ git show e1460e4707c
commit e1460e4707cd80982add597f5cb421289db84e4e
Author: Isaku Yamahata <address@hidden>
Date:   Fri May 14 16:29:16 2010 +0900

    pc: move rtc declarations from pc.h into a dedicated header file.

    Move rtc_xxx declarations from pc.h into mc146818rtc.h.

    Signed-off-by: Isaku Yamahata <address@hidden>
    Acked-by: Gerd Hoffmann <address@hidden>
    Signed-off-by: Blue Swirl <address@hidden>

diff --git a/hw/mc146818rtc.h b/hw/mc146818rtc.h
new file mode 100644
index 0000000000..7211dae37e
--- /dev/null
+++ b/hw/mc146818rtc.h
@@ -0,0 +1,10 @@
+#ifndef MC146818RTC_H
+#define MC146818RTC_H
+
+typedef struct RTCState RTCState;
+
+RTCState *rtc_init(int base_year);
+void rtc_set_memory(RTCState *s, int addr, int val);
+void rtc_set_date(RTCState *s, const struct tm *tm);
+
+#endif /* !MC146818RTC_H */
diff --git a/hw/pc.h b/hw/pc.h
index 8cfec6c0dc..e3cc0a58bd 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -5,6 +5,7 @@
 #include "ioport.h"
 #include "isa.h"
 #include "fdc.h"
+#include "mc146818rtc.h"

 /* PC-style peripherals (also used by other machines).  */

@@ -76,14 +77,6 @@ void i8042_mm_init(qemu_irq kbd_irq, qemu_irq mouse_irq,
                    target_phys_addr_t base, ram_addr_t size,
                    target_phys_addr_t mask);

-/* mc146818rtc.c */
-
-typedef struct RTCState RTCState;
-
-RTCState *rtc_init(int base_year);
-void rtc_set_memory(RTCState *s, int addr, int val);
-void rtc_set_date(RTCState *s, const struct tm *tm);

So the (c) looks correct to me. Do you still disagree?