| Message ID | 20221218071229.484944-2-balbi@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/arm: Add support for STM32 H405 and fix STM32F405 memory layout | expand |
On Mon, Dec 19, 2022 at 1:24 AM Felipe Balbi <balbi@kernel.org> 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> Reviewed-by: Alistair Francis <alistair@alistair23.me> Alistair > --- > 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, > + &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) > > struct STM32F405State { > /*< private >*/ > @@ -65,6 +67,7 @@ struct STM32F405State { > STM32F2XXADCState adc[STM_NUM_ADCS]; > STM32F2XXSPIState spi[STM_NUM_SPIS]; > > + MemoryRegion ccm; > MemoryRegion sram; > MemoryRegion flash; > MemoryRegion flash_alias; > -- > 2.38.1 > >
Hi Felipe, 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'? > + &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 :) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
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.
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, + &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) struct STM32F405State { /*< private >*/ @@ -65,6 +67,7 @@ struct STM32F405State { STM32F2XXADCState adc[STM_NUM_ADCS]; STM32F2XXSPIState spi[STM_NUM_SPIS]; + MemoryRegion ccm; MemoryRegion sram; MemoryRegion flash; MemoryRegion flash_alias;
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(-)