Re: [PATCH v2] Use named constants in BCM props

["Kambalin, Sergey" <sergey.kambalin@auriga.com>]

[-- Attachment #1: Type: text/plain, Size: 2153 bytes --]




________________________________
От: Philippe Mathieu-Daudé <philmd@linaro.org>
Отправлено: 13 июня 2023 г. 1:30
Кому: Sergey Kambalin; qemu-arm@nongnu.org
Копия: qemu-devel@nongnu.org; Kambalin, Sergey
Тема: Re: [PATCH v2] Use named constants in BCM props

On 12/6/23 13:59, Sergey Kambalin wrote:
> ping
>
> - PI_FIRMWARE_*_RATE constsnts were moved to raspberrypi-fw-defs.h
>    (seems more suitable place for them)
> - inclusion of "qemu/osdep.h" has been removed
> - year in copyright header has been updated
>
> Signed-off-by: Sergey Kambalin <sergey.kambalin@auriga.com>
> ---
>   hw/misc/bcm2835_property.c            | 120 ++++++++++---------
>   include/hw/arm/raspi_platform.h       |   6 +
>   include/hw/misc/raspberrypi-fw-defs.h | 163 ++++++++++++++++++++++++++
>   3 files changed, 236 insertions(+), 53 deletions(-)
>   create mode 100644 include/hw/misc/raspberrypi-fw-defs.h


> diff --git a/include/hw/arm/raspi_platform.h b/include/hw/arm/raspi_platform.h
> index 4a56dd4b89..92a317950a 100644
> --- a/include/hw/arm/raspi_platform.h
> +++ b/include/hw/arm/raspi_platform.h
> @@ -170,4 +170,10 @@
>   #define INTERRUPT_ILLEGAL_TYPE0        6
>   #define INTERRUPT_ILLEGAL_TYPE1        7
>
> +/* Clock rates */
> +#define RPI_FIRMWARE_EMMC_CLK_RATE    50000000

OK.

> +#define RPI_FIRMWARE_UART_CLK_RATE    3000000

OK.

> +#define RPI_FIRMWARE_CORE_CLK_RATE    350000000

Seems VC4 frequency range, adapted for raspi3 (BCM2837).

IIUC the VC6 (raspi4) is clocked at 500MHz.

S.K.: You're right! There should be 250 MHz as these are bcm2835 values, not bcm2711(rpi4)

> +#define RPI_FIRMWARE_DEFAULT_CLK_RATE 700000000

Seems VC4 freq for raspi1 (BCM2835)

Likely we don't want to use a default, but the correct per-soc
value...

S.K.:  Per-soc values are not implemented yet, and I'm not 100% sure if it's needed.
          So I decided to return BCM2835 core clock as a default value.
          May be leaving LOG_UNIMP message would have been a better option?

Should I fix it there or pull your split patch and continue it as v4?

[-- Attachment #2: Type: text/html, Size: 4695 bytes --]