Re: [Qemu-devel] [PATCH 09/13] hw: Move Xilinx ZynqMP RTC from hw/timer/ to hw/rtc/ subdirectory

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

Hi Alistair,

On 9/16/19 11:44 PM, Alistair Francis wrote:
> On Mon, Sep 16, 2019 at 8:56 AM Philippe Mathieu-Daudé
> <philmd@redhat.com> wrote:
>>
>> Move RTC devices under the hw/rtc/ subdirectory.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  hw/rtc/Makefile.objs                        | 1 +
>>  hw/rtc/trace-events                         | 3 +++
>>  hw/{timer => rtc}/xlnx-zynqmp-rtc.c         | 2 +-
>>  hw/timer/Makefile.objs                      | 1 -
>>  hw/timer/trace-events                       | 3 ---
>>  include/hw/arm/xlnx-zynqmp.h                | 2 +-
>>  include/hw/{timer => rtc}/xlnx-zynqmp-rtc.h | 6 +++---
>>  7 files changed, 9 insertions(+), 9 deletions(-)
>>  rename hw/{timer => rtc}/xlnx-zynqmp-rtc.c (99%)
>>  rename include/hw/{timer => rtc}/xlnx-zynqmp-rtc.h (95%)
>>
>> diff --git a/hw/rtc/Makefile.objs b/hw/rtc/Makefile.objs
>> index b195863291..543a550a0f 100644
>> --- a/hw/rtc/Makefile.objs
>> +++ b/hw/rtc/Makefile.objs
>> @@ -6,5 +6,6 @@ common-obj-$(CONFIG_M48T59) += m48t59-isa.o
>>  endif
>>  common-obj-$(CONFIG_PL031) += pl031.o
>>  common-obj-$(CONFIG_TWL92230) += twl92230.o
>> +common-obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp-rtc.o
>>  obj-$(CONFIG_MC146818RTC) += mc146818rtc.o
>>  common-obj-$(CONFIG_SUN4V_RTC) += sun4v-rtc.o
>> diff --git a/hw/rtc/trace-events b/hw/rtc/trace-events
>> index ac9e0e0fba..7f1945ad4c 100644
>> --- a/hw/rtc/trace-events
>> +++ b/hw/rtc/trace-events
>> @@ -4,6 +4,9 @@
>>  sun4v_rtc_read(uint64_t addr, uint64_t value) "read: addr 0x%" PRIx64 " value 0x%" PRIx64
>>  sun4v_rtc_write(uint64_t addr, uint64_t value) "write: addr 0x%" PRIx64 " value 0x%" PRIx64
>>
>> +# xlnx-zynqmp-rtc.c
>> +xlnx_zynqmp_rtc_gettime(int year, int month, int day, int hour, int min, int sec) "Get time from host: %d-%d-%d %2d:%02d:%02d"
>> +
>>  # pl031.c
>>  pl031_irq_state(int level) "irq state %d"
>>  pl031_read(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
>> diff --git a/hw/timer/xlnx-zynqmp-rtc.c b/hw/rtc/xlnx-zynqmp-rtc.c
>> similarity index 99%
>> rename from hw/timer/xlnx-zynqmp-rtc.c
>> rename to hw/rtc/xlnx-zynqmp-rtc.c
>> index 5692db98c2..f9f09b7296 100644
>> --- a/hw/timer/xlnx-zynqmp-rtc.c
>> +++ b/hw/rtc/xlnx-zynqmp-rtc.c
>> @@ -36,7 +36,7 @@
>>  #include "qemu/cutils.h"
>>  #include "sysemu/sysemu.h"
>>  #include "trace.h"
>> -#include "hw/timer/xlnx-zynqmp-rtc.h"
>> +#include "hw/rtc/xlnx-zynqmp-rtc.h"
>>  #include "migration/vmstate.h"
>>
>>  #ifndef XLNX_ZYNQMP_RTC_ERR_DEBUG
>> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
>> index 70b61b69c7..294465ef47 100644
>> --- a/hw/timer/Makefile.objs
>> +++ b/hw/timer/Makefile.objs
>> @@ -14,7 +14,6 @@ common-obj-$(CONFIG_IMX) += imx_epit.o
>>  common-obj-$(CONFIG_IMX) += imx_gpt.o
>>  common-obj-$(CONFIG_LM32) += lm32_timer.o
>>  common-obj-$(CONFIG_MILKYMIST) += milkymist-sysctl.o
>> -common-obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp-rtc.o
>>  common-obj-$(CONFIG_NRF51_SOC) += nrf51_timer.o
>>
>>  common-obj-$(CONFIG_ALTERA_TIMER) += altera_timer.o
>> diff --git a/hw/timer/trace-events b/hw/timer/trace-events
>> index ce34b967db..1459d07237 100644
>> --- a/hw/timer/trace-events
>> +++ b/hw/timer/trace-events
>> @@ -70,9 +70,6 @@ cmsdk_apb_dualtimer_reset(void) "CMSDK APB dualtimer: reset"
>>  aspeed_rtc_read(uint64_t addr, uint64_t value) "addr 0x%02" PRIx64 " value 0x%08" PRIx64
>>  aspeed_rtc_write(uint64_t addr, uint64_t value) "addr 0x%02" PRIx64 " value 0x%08" PRIx64
>>
>> -# xlnx-zynqmp-rtc.c
>> -xlnx_zynqmp_rtc_gettime(int year, int month, int day, int hour, int min, int sec) "Get time from host: %d-%d-%d %2d:%02d:%02d"
>> -
>>  # nrf51_timer.c
>>  nrf51_timer_read(uint64_t addr, uint32_t value, unsigned size) "read addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
>>  nrf51_timer_write(uint64_t addr, uint32_t value, unsigned size) "write addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
>> diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
>> index d7483c3b42..53076fa29a 100644
>> --- a/include/hw/arm/xlnx-zynqmp.h
>> +++ b/include/hw/arm/xlnx-zynqmp.h
>> @@ -29,7 +29,7 @@
>>  #include "hw/dma/xlnx-zdma.h"
>>  #include "hw/display/xlnx_dp.h"
>>  #include "hw/intc/xlnx-zynqmp-ipi.h"
>> -#include "hw/timer/xlnx-zynqmp-rtc.h"
>> +#include "hw/rtc/xlnx-zynqmp-rtc.h"
>>  #include "hw/cpu/cluster.h"
>>  #include "target/arm/cpu.h"
>>
>> diff --git a/include/hw/timer/xlnx-zynqmp-rtc.h b/include/hw/rtc/xlnx-zynqmp-rtc.h
>> similarity index 95%
>> rename from include/hw/timer/xlnx-zynqmp-rtc.h
>> rename to include/hw/rtc/xlnx-zynqmp-rtc.h
>> index 97e32322ed..6fa1cb2f43 100644
>> --- a/include/hw/timer/xlnx-zynqmp-rtc.h
>> +++ b/include/hw/rtc/xlnx-zynqmp-rtc.h
>> @@ -3,7 +3,7 @@
>>   *
>>   * Copyright (c) 2017 Xilinx Inc.
>>   *
>> - * Written-by: Alistair Francis <alistair.francis@xilinx.com>
>> + * Written-by: Alistair Francis
> 
> Why remove the email address?

In https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg00759.html
Thomas suggested to remove invalid email addresses:

  I don't think that this e-mail address is still valid since that
  company has been bought up > 10 years ago... so it likely does not
  make sense to mention it in new files anymore. So just mention the
  name?

In your case Xilinx still exists, but you warned us it might bounce:

$ git show c22e580c2ad
commit c22e580c2ad1cccef582e1490e732f254d4ac064 (tag:
pull-target-arm-20180301)
Author: Alistair Francis <alistair.francis@xilinx.com>
Date:   Thu Mar 1 11:05:58 2018 +0000

    MAINTAINERS: Update my email address

    I am leaving Xilinx, so to avoid having an email address that bounces
    update my maintainer address to point to my personal email address.

So I stripped your obsolete email.

Since the line is not "(c) Name <email>" but simply "Written by" and the
(C) remains to Xilinx, we could update the email by your personal one,
that seems fair (so if someone want to contact you to ask you question
about your code, he still can).

What do you prefer?

> 
> Otherwise:
> 
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Thanks!

> 
> Alistair
> 
>>   *
>>   * Permission is hereby granted, free of charge, to any person obtaining a copy
>>   * of this software and associated documentation files (the "Software"), to deal
>> @@ -24,8 +24,8 @@
>>   * THE SOFTWARE.
>>   */
>>
>> -#ifndef HW_TIMER_XLNX_ZYNQMP_RTC_H
>> -#define HW_TIMER_XLNX_ZYNQMP_RTC_H
>> +#ifndef HW_RTC_XLNX_ZYNQMP_H
>> +#define HW_RTC_XLNX_ZYNQMP_H
>>
>>  #include "hw/register.h"
>>  #include "hw/sysbus.h"
>> --
>> 2.20.1
>>
>>