The flash mmio region is exposed as an AddressSpace.
AddressSpaces must not be sysbus-mapped, therefore map
the region using an alias.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/ssi/aspeed_smc.h | 1 +
hw/ssi/aspeed_smc.c | 4 +++-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
index 16c03fe64f3..e3c96cecbd8 100644
--- a/include/hw/ssi/aspeed_smc.h
+++ b/include/hw/ssi/aspeed_smc.h
@@ -84,6 +84,7 @@ struct AspeedSMCState {
MemoryRegion mmio;
MemoryRegion mmio_flash;
+ MemoryRegion mmio_flash_alias;
qemu_irq irq;
int irqline;
diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 16addee4dc8..aa26578bdac 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -1386,7 +1386,9 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
memory_region_init_io(&s->mmio_flash, OBJECT(s),
&aspeed_smc_flash_default_ops, s, name,
s->ctrl->flash_window_size);
- sysbus_init_mmio(sbd, &s->mmio_flash);
+ memory_region_init_alias(&s->mmio_flash_alias, OBJECT(s), name,
+ &s->mmio_flash, 0, s->ctrl->flash_window_size);
+ sysbus_init_mmio(sbd, &s->mmio_flash_alias);
s->flashes = g_new0(AspeedSMCFlash, s->ctrl->max_peripherals);
--
2.26.2
Incorrect subject prefix, should be "hw/ssi/aspeed_smc"
On 3/12/21 7:28 PM, Philippe Mathieu-Daudé wrote:
> The flash mmio region is exposed as an AddressSpace.
> AddressSpaces must not be sysbus-mapped, therefore map
> the region using an alias.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> include/hw/ssi/aspeed_smc.h | 1 +
> hw/ssi/aspeed_smc.c | 4 +++-
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
> index 16c03fe64f3..e3c96cecbd8 100644
> --- a/include/hw/ssi/aspeed_smc.h
> +++ b/include/hw/ssi/aspeed_smc.h
> @@ -84,6 +84,7 @@ struct AspeedSMCState {
>
> MemoryRegion mmio;
> MemoryRegion mmio_flash;
> + MemoryRegion mmio_flash_alias;
>
> qemu_irq irq;
> int irqline;
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index 16addee4dc8..aa26578bdac 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -1386,7 +1386,9 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
> memory_region_init_io(&s->mmio_flash, OBJECT(s),
> &aspeed_smc_flash_default_ops, s, name,
> s->ctrl->flash_window_size);
> - sysbus_init_mmio(sbd, &s->mmio_flash);
> + memory_region_init_alias(&s->mmio_flash_alias, OBJECT(s), name,
> + &s->mmio_flash, 0, s->ctrl->flash_window_size);
> + sysbus_init_mmio(sbd, &s->mmio_flash_alias);
>
> s->flashes = g_new0(AspeedSMCFlash, s->ctrl->max_peripherals);
On 3/13/21 1:05 PM, Philippe Mathieu-Daudé wrote:
> Incorrect subject prefix, should be "hw/ssi/aspeed_smc"
Is this just good practice or something that was agreed upon ?
We should update checkpatch if so.
Thanks,
C.
>
> On 3/12/21 7:28 PM, Philippe Mathieu-Daudé wrote:
>> The flash mmio region is exposed as an AddressSpace.
>> AddressSpaces must not be sysbus-mapped, therefore map
>> the region using an alias.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> include/hw/ssi/aspeed_smc.h | 1 +
>> hw/ssi/aspeed_smc.c | 4 +++-
>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
>> index 16c03fe64f3..e3c96cecbd8 100644
>> --- a/include/hw/ssi/aspeed_smc.h
>> +++ b/include/hw/ssi/aspeed_smc.h
>> @@ -84,6 +84,7 @@ struct AspeedSMCState {
>>
>> MemoryRegion mmio;
>> MemoryRegion mmio_flash;
>> + MemoryRegion mmio_flash_alias;
>>
>> qemu_irq irq;
>> int irqline;
>> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
>> index 16addee4dc8..aa26578bdac 100644
>> --- a/hw/ssi/aspeed_smc.c
>> +++ b/hw/ssi/aspeed_smc.c
>> @@ -1386,7 +1386,9 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
>> memory_region_init_io(&s->mmio_flash, OBJECT(s),
>> &aspeed_smc_flash_default_ops, s, name,
>> s->ctrl->flash_window_size);
>> - sysbus_init_mmio(sbd, &s->mmio_flash);
>> + memory_region_init_alias(&s->mmio_flash_alias, OBJECT(s), name,
>> + &s->mmio_flash, 0, s->ctrl->flash_window_size);
>> + sysbus_init_mmio(sbd, &s->mmio_flash_alias);
>>
>> s->flashes = g_new0(AspeedSMCFlash, s->ctrl->max_peripherals);
On 3/17/21 7:46 PM, Cédric Le Goater wrote: > On 3/13/21 1:05 PM, Philippe Mathieu-Daudé wrote: >> Incorrect subject prefix, should be "hw/ssi/aspeed_smc" > > Is this just good practice or something that was agreed upon ? Not sure, maybe "good practice"? Anyway here I only meant to correct my own patch without respining the series just for this. > We should update checkpatch if so. Certainly a waste of time =) > > Thanks, > > C.
On 3/12/21 7:28 PM, Philippe Mathieu-Daudé wrote:
> The flash mmio region is exposed as an AddressSpace.
> AddressSpaces must not be sysbus-mapped, therefore map
> the region using an alias.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
That does the trick but you need an extra change in the model.
The fixes are in my aspeed-6.0 branch on GH and they survive the last
patch of your series :
[PATCH 5/5] memory: Make sure root MR won't be added as subregion
I will upstream for 6.1.
Thanks,
C.
> ---
> include/hw/ssi/aspeed_smc.h | 1 +
> hw/ssi/aspeed_smc.c | 4 +++-
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
> index 16c03fe64f3..e3c96cecbd8 100644
> --- a/include/hw/ssi/aspeed_smc.h
> +++ b/include/hw/ssi/aspeed_smc.h
> @@ -84,6 +84,7 @@ struct AspeedSMCState {
>
> MemoryRegion mmio;
> MemoryRegion mmio_flash;
> + MemoryRegion mmio_flash_alias;
>
> qemu_irq irq;
> int irqline;
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index 16addee4dc8..aa26578bdac 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -1386,7 +1386,9 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
> memory_region_init_io(&s->mmio_flash, OBJECT(s),
> &aspeed_smc_flash_default_ops, s, name,
> s->ctrl->flash_window_size);
> - sysbus_init_mmio(sbd, &s->mmio_flash);
> + memory_region_init_alias(&s->mmio_flash_alias, OBJECT(s), name,
> + &s->mmio_flash, 0, s->ctrl->flash_window_size);
> + sysbus_init_mmio(sbd, &s->mmio_flash_alias);
>
> s->flashes = g_new0(AspeedSMCFlash, s->ctrl->max_peripherals);
>
>
On 3/17/21 7:30 PM, Cédric Le Goater wrote: > On 3/12/21 7:28 PM, Philippe Mathieu-Daudé wrote: >> The flash mmio region is exposed as an AddressSpace. >> AddressSpaces must not be sysbus-mapped, therefore map >> the region using an alias. >> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > > That does the trick but you need an extra change in the model. > > The fixes are in my aspeed-6.0 branch on GH and they survive the last > patch of your series : > > [PATCH 5/5] memory: Make sure root MR won't be added as subregion I wondered about changing DMA_FLASH_ADDR() wasn't sure the tests would use the flash. > I will upstream for 6.1. Thanks! Phil.
On 3/17/21 8:00 PM, Philippe Mathieu-Daudé wrote: > On 3/17/21 7:30 PM, Cédric Le Goater wrote: >> On 3/12/21 7:28 PM, Philippe Mathieu-Daudé wrote: >>> The flash mmio region is exposed as an AddressSpace. >>> AddressSpaces must not be sysbus-mapped, therefore map >>> the region using an alias. >>> >>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> >> That does the trick but you need an extra change in the model. >> >> The fixes are in my aspeed-6.0 branch on GH and they survive the last >> patch of your series : >> >> [PATCH 5/5] memory: Make sure root MR won't be added as subregion > > I wondered about changing DMA_FLASH_ADDR() wasn't sure the tests > would use the flash. The acceptance tests (not merged yet) download firmware images in which u-boot does DMA accesses to calibrate the reads on the flash device. C. > >> I will upstream for 6.1. > > Thanks! > > Phil. >
© 2016 - 2026 Red Hat, Inc.