Re: [PATCH 1/5] aspeed/smc: Restore default AHB window mapping at reset

[Joel Stanley]

On Thu, 14 Nov 2019 at 09:46, Cédric Le Goater <address@hidden> wrote:
>
> The current model only restores the Segment Register values but leaves
> the previous CS mapping behind. Introduce a helper setting the
> register value and mapping the region at the requested address. Use
> this helper when a Segment register is set and at reset.
>
> Signed-off-by: Cédric Le Goater <address@hidden>

Reviewed-by: Joel Stanley <address@hidden>

> ---
>  hw/ssi/aspeed_smc.c | 32 +++++++++++++++++++++-----------
>  1 file changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index f0c7bbbad302..955ec21852ac 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -475,10 +475,26 @@ static bool aspeed_smc_flash_overlap(const 
> AspeedSMCState *s,
>      return false;
>  }
>
> +static void aspeed_smc_flash_set_segment_region(AspeedSMCState *s, int cs,
> +                                                uint64_t regval)
> +{
> +    AspeedSMCFlash *fl = &s->flashes[cs];
> +    AspeedSegments seg;
> +
> +    s->ctrl->reg_to_segment(s, regval, &seg);
> +
> +    memory_region_transaction_begin();
> +    memory_region_set_size(&fl->mmio, seg.size);
> +    memory_region_set_address(&fl->mmio, seg.addr - 
> s->ctrl->flash_window_base);
> +    memory_region_set_enabled(&fl->mmio, true);
> +    memory_region_transaction_commit();
> +
> +    s->regs[R_SEG_ADDR0 + cs] = regval;
> +}
> +
>  static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs,
>                                           uint64_t new)
>  {
> -    AspeedSMCFlash *fl = &s->flashes[cs];
>      AspeedSegments seg;
>
>      s->ctrl->reg_to_segment(s, new, &seg);
> @@ -529,13 +545,7 @@ static void aspeed_smc_flash_set_segment(AspeedSMCState 
> *s, int cs,
>      aspeed_smc_flash_overlap(s, &seg, cs);
>
>      /* All should be fine now to move the region */
> -    memory_region_transaction_begin();
> -    memory_region_set_size(&fl->mmio, seg.size);
> -    memory_region_set_address(&fl->mmio, seg.addr - 
> s->ctrl->flash_window_base);
> -    memory_region_set_enabled(&fl->mmio, true);
> -    memory_region_transaction_commit();
> -
> -    s->regs[R_SEG_ADDR0 + cs] = new;
> +    aspeed_smc_flash_set_segment_region(s, cs, new);
>  }
>
>  static uint64_t aspeed_smc_flash_default_read(void *opaque, hwaddr addr,
> @@ -897,10 +907,10 @@ static void aspeed_smc_reset(DeviceState *d)
>          qemu_set_irq(s->cs_lines[i], true);
>      }
>
> -    /* setup default segment register values for all */
> +    /* setup the default segment register values and regions for all */
>      for (i = 0; i < s->ctrl->max_slaves; ++i) {
> -        s->regs[R_SEG_ADDR0 + i] =
> -            s->ctrl->segment_to_reg(s, &s->ctrl->segments[i]);
> +        aspeed_smc_flash_set_segment_region(s, i,
> +                    s->ctrl->segment_to_reg(s, &s->ctrl->segments[i]));
>      }
>
>      /* HW strapping flash type for the AST2600 controllers  */
> --
> 2.21.0
>