Re: [PATCH] Make bootable RPi4B model

[Kambalin, Sergey]

Hi Phil!

I've put unit tests to a separate patch in order to not overwhelm you with code.

It is already a huge piece, and I agree that  3000+ lines are a sort of a challenge to a reviewer.

OK, I'll try to split it into a several patches - core functionality + a single patch for each device (gpio, trng, thermal sensors)

BR,

Sergey

---

От: Philippe Mathieu-Daudé <philmd@linaro.org>
Отправлено: 3 апреля 2023 г. 9:51:11
Кому: Sergey Kambalin; qemu-devel@nongnu.org
Копия: Kambalin, Sergey
Тема: Re: [PATCH] Make bootable RPi4B model

 

Hi Sergey,

On 2/4/23 18:53, Sergey Kambalin wrote:
> Signed-off-by: Sergey Kambalin <sergey.kambalin@auriga.com>
> ---
>   configs/devices/aarch64-softmmu/default.mak |   1 +
>   hw/arm/Kconfig                              |   7 +
>   hw/arm/bcm2835_peripherals.c                | 218 ++++++----
>   hw/arm/bcm2836.c                            | 129 +++---
>   hw/arm/bcm2838.c                            | 294 ++++++++++++++
>   hw/arm/bcm2838_pcie.c                       | 300 ++++++++++++++
>   hw/arm/bcm2838_peripherals.c                | 262 ++++++++++++
>   hw/arm/meson.build                          |   6 +
>   hw/arm/raspi.c                              | 128 +++---
>   hw/arm/raspi4b.c                            | 232 +++++++++++
>   hw/arm/trace-events                         |   6 +
>   hw/gpio/bcm2838_gpio.c                      | 395 ++++++++++++++++++
>   hw/gpio/meson.build                         |   1 +
>   hw/misc/bcm2835_property.c                  | 318 ++++++++++++---
>   hw/misc/bcm2838_rng200.c                    | 421 ++++++++++++++++++++
>   hw/misc/bcm2838_thermal.c                   |  97 +++++
>   hw/misc/meson.build                         |   4 +
>   hw/misc/trace-events                        |  10 +
>   include/hw/arm/bcm2835_peripherals.h        |  29 +-
>   include/hw/arm/bcm2836.h                    |  30 +-
>   include/hw/arm/bcm2838.h                    |  26 ++
>   include/hw/arm/bcm2838_pcie.h               |  67 ++++
>   include/hw/arm/bcm2838_peripherals.h        |  56 +++
>   include/hw/arm/raspi4b_platform.h           |  54 +++
>   include/hw/arm/raspi_platform.h             |  34 ++
>   include/hw/display/bcm2835_fb.h             |   2 +
>   include/hw/gpio/bcm2838_gpio.h              |  45 +++
>   include/hw/misc/bcm2838_rng200.h            |  77 ++++
>   include/hw/misc/bcm2838_thermal.h           |  24 ++
>   include/hw/misc/raspberrypi-fw-defs.h       | 169 ++++++++
>   30 files changed, 3175 insertions(+), 267 deletions(-)

The patch subject sounds promising! However 3175 lines of
code to review is a bit hardcore... Could you split your
patch per device (at least)?

Ideally one patch should contain a single logical change.

Also, can you provide how you tested your patch?

Thanks,

Phil.