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

[Thomas Huth]

On 17/09/2019 12.03, Philippe Mathieu-Daudé wrote:
> 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/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);

Ok, fair. But vl.h had a slightly different copyright statement than
vl.c, so I think you should rather use the one from vl.h.
But IMHO you could at least drop the "THE SOFTWARE IS PROVIDED ..."
paragraph and add a SPDX tag instead?

 Thomas