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

[Philippe Mathieu-Daudé]

On 9/18/19 9:43 AM, Thomas Huth wrote:
> 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?

I find SPDX tags clearer too, but last time I wanted to use them Peter said:

  I think we should not do that until/unless
  somebody (probably a corporate somebody) steps forward
  to make the argument for "this is why we should have them,
  we as a contributor to the project think they are worthwhile
  and a useful feature for us, and we will make the effort to
  add them, review that they are correct, update checkpatch to
  insist on tags for new files, etc". In other words, "if it
  ain't broke, don't fix it"; nobody is yet complaining that
  our current setup is broken.

https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg04151.html

At this time we had less:

$ git grep SPDX v3.1.0 | egrep -v linux-headers | wc -l
21

Since we few entered, so we have a mix:

$ git grep SPDX origin/master | egrep -v linux-headers | wc -l
79