[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] hw/arm/stm32f405: correctly describe the memory layout
From: |
Felipe Balbi |
Subject: |
Re: [PATCH 1/2] hw/arm/stm32f405: correctly describe the memory layout |
Date: |
Mon, 19 Dec 2022 12:31:30 +0200 |
Hi,
Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> On 18/12/22 08:12, Felipe Balbi wrote:
>> STM32F405 has 128K of SRAM and another 64K of CCM (Core-coupled
>> Memory) at a different base address. Correctly describe the memory
>> layout to give existing FW images have a chance to run unmodified.
>>
>> Signed-off-by: Felipe Balbi <balbi@kernel.org>
>> ---
>> hw/arm/stm32f405_soc.c | 8 ++++++++
>> include/hw/arm/stm32f405_soc.h | 5 ++++-
>> 2 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/arm/stm32f405_soc.c b/hw/arm/stm32f405_soc.c
>> index c07947d9f8b1..cef23d7ee41a 100644
>> --- a/hw/arm/stm32f405_soc.c
>> +++ b/hw/arm/stm32f405_soc.c
>> @@ -139,6 +139,14 @@ static void stm32f405_soc_realize(DeviceState *dev_soc,
>> Error **errp)
>> }
>> memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS,
>> &s->sram);
>>
>> + memory_region_init_ram(&s->ccm, NULL, "STM32F405.ccm", CCM_SIZE,
>
> Including the machine name in the memory description seems a bad
> habit from old days. What do you think about renaming as
> 'core-coupled-memory'?
I don't oppose it, but I was merely following the model from
`netduino2plus'.
>> + &err);
>> + if (err != NULL) {
>> + error_propagate(errp, err);
>> + return;
>> + }
>> + memory_region_add_subregion(system_memory, CCM_BASE_ADDRESS, &s->ccm);
>> +
>> armv7m = DEVICE(&s->armv7m);
>> qdev_prop_set_uint32(armv7m, "num-irq", 96);
>> qdev_prop_set_string(armv7m, "cpu-type", s->cpu_type);
>> diff --git a/include/hw/arm/stm32f405_soc.h b/include/hw/arm/stm32f405_soc.h
>> index 5bb0c8d56979..249ab5434ec7 100644
>> --- a/include/hw/arm/stm32f405_soc.h
>> +++ b/include/hw/arm/stm32f405_soc.h
>> @@ -46,7 +46,9 @@ OBJECT_DECLARE_SIMPLE_TYPE(STM32F405State, STM32F405_SOC)
>> #define FLASH_BASE_ADDRESS 0x08000000
>> #define FLASH_SIZE (1024 * 1024)
>> #define SRAM_BASE_ADDRESS 0x20000000
>> -#define SRAM_SIZE (192 * 1024)
>> +#define SRAM_SIZE (128 * 1024)
>> +#define CCM_BASE_ADDRESS 0x10000000
>> +#define CCM_SIZE (64 * 1024)
>
> Since the CCM_SIZE won't be used elsewhere, we can simply use '64 * KiB'
> in the memory_region_init_ram() in the source file. Up to the maintainer
> :)
Also not opposed. I'll wait a couple days before respinning the patch to
give the rest of the community time to reply.
--
balbi
signature.asc
Description: PGP signature