Hi Niek,
On 1/13/20 8:18 PM, Niek Linnenbank wrote:
> Hi,
>
> 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.
Yes, true indeed. I'll be patient and wait for more feedback when it comes.
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.
OK good idea. I'll add Beniamino in CC for v4!
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).
Yes the SDRAM controller is something that needs a good read to understand it,
especially also in the U-Boot sources which is using it. No hurry there, I very much
appreciate all the time you already took for reviewing this series. It encourages me
to continue with it as well. I'm also doing this work in my hobby time.
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.
Thanks for testing that Philippe! Yes the SD/MMC host device is indeed
relatively large compared to the other patches.
Anyway, don't forget this comment from the New Contribution page:
https://wiki.qemu.org/Contribute/SubmitAPatch#Return_the_favor
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 :)
Thanks for reminding about that. You are right, indeed I should start reviewing
other patches as well to return the favor and help others.
Regards,
Niek
That said, your series is almost there!
Regards,
Phil.
>
> Regards,
> Niek