[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 03/17] hw/arm/allwinner-h3: add Clock Control Unit

From: Philippe Mathieu-Daudé
Subject: Re: [PATCH v3 03/17] hw/arm/allwinner-h3: add Clock Control Unit
Date: Sat, 18 Jan 2020 16:37:02 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2

Hi Niek,

On 1/13/20 8:18 PM, Niek Linnenbank wrote:

Just a friendly reminder for review of this patch and the others in this series
that don't yet have a reviewed-by tag :-)

You are right to ping the list after a week.

Cc'ing Damien for this particular patch, he might have good advises.

Looking at the stats from your cover:

 include/hw/arm/allwinner-h3.h          | 164 +++++
 include/hw/misc/allwinner-cpucfg.h     |  54 ++
 include/hw/misc/allwinner-h3-ccu.h     |  67 ++
 include/hw/misc/allwinner-h3-dramc.h   | 107 +++
 include/hw/misc/allwinner-h3-sysctrl.h |  68 ++
 include/hw/net/allwinner-sun8i-emac.h  | 103 +++
 include/hw/rtc/allwinner-rtc.h         | 129 ++++
 include/hw/sd/allwinner-sdhost.h       | 136 ++++
 hw/arm/allwinner-h3.c                  | 477 ++++++++++++++
 hw/arm/orangepi.c                      | 125 ++++
 hw/misc/allwinner-cpucfg.c             | 282 ++++++++
 hw/misc/allwinner-h3-ccu.c             | 243 +++++++
 hw/misc/allwinner-h3-dramc.c           | 358 ++++++++++
 hw/misc/allwinner-h3-sysctrl.c         | 140 ++++
 hw/misc/allwinner-sid.c                | 170 +++++
 hw/net/allwinner-sun8i-emac.c          | 871 +++++++++++++++++++++++++
 hw/rtc/allwinner-rtc.c                 | 386 +++++++++++
 hw/sd/allwinner-sdhost.c               | 848 ++++++++++++++++++++++++

 39 files changed, 5267 insertions(+), 2 deletions(-)

This is a LOT of code to process, keep in mind your series touches different subsystems with different maintainers. It is hard to know all of them in details.

Since your SoC is in the same family than the A10, I've Cc'ed Beniamino Galvani. You should Cc him in your v4, hopefully he can help reviewing.

Regarding the System Control Unit and SDRAM Controller, as I don't know this SoC so I have to digest the whole datasheet, so it takes me time, bare with me (I'm using my hobby time to review your work).

The last patch I plan to review in your series is the SD/MMC one:
 10 files changed, 1053 insertions(+), 2 deletions(-)

It is 1/5th of your series in a single patch, each time I try to look at it I get scared. Anyway today I could test NetBSD booting from a SD card so I am more confident.

Anyway, don't forget this comment from the New Contribution page:

  Peer review only works if everyone chips in a bit of review time.
  If everyone submitted more patches than they reviewed, we would
  have a patch backlog. A good goal is to try to review at least
  as many patches from others as what you submit.

With the quality of your patches, even if this is your first contribution, it is obvious you now understand various part of QEMU. Don't be shy to look at other patches on the list and help the community, as the reviewed authors might review you back :)

That said, your series is almost there!




reply via email to

[Prev in Thread] Current Thread [Next in Thread]