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

[Alistair Francis]

On Tue, Sep 17, 2019 at 3:28 AM Philippe Mathieu-Daudé
<address@hidden> wrote:
>
> Hi Alistair,
>
> On 9/16/19 11:44 PM, Alistair Francis wrote:
> > On Mon, Sep 16, 2019 at 8:56 AM Philippe Mathieu-Daudé
> > <address@hidden> wrote:
> >>
> >> Move RTC devices under the hw/rtc/ subdirectory.
> >>
> >> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> >> ---
> >>  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 <address@hidden>
> >> + * 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:

Yeah, I assume it bounces now.

>
> $ git show c22e580c2ad
> commit c22e580c2ad1cccef582e1490e732f254d4ac064 (tag:
> pull-target-arm-20180301)
> Author: Alistair Francis <address@hidden>
> 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).

I always though that the email address indicated that "Alistair
Francis" wrote this, but while being funded by "Xilinx". It gave both
parties credit for the work. The email address is also unique (at
least somewhat) which helps tie things together if you have a common
name.

You are right though that the author and copyright tags do the same thing.

As for contact to ask questions that is where the maintainers file
comes in, so it probably doesn't matter too much.

>
> What do you prefer?

I'm not fussed, was just curious why you thought the email should be
removed. It's fine as it is.

Alistair

>
> >
> > Otherwise:
> >
> > Reviewed-by: Alistair Francis <address@hidden>
>
> 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
> >>
> >>