On 08/07/2018 09:57 AM, Joel Stanley wrote:
> This fixes the intended protection of read-only values in the
> configuration register. They were being always set to zero by mistake.
yes :/
> The read-only fields depend on the configured memory size of the system,
> so they cannot be fixed at compile time. The most straight forward
> option was to store them in the state structure.
We could also use an array of init values for registers, like SCU does, but
this is fine for now.
> Signed-off-by: Joel Stanley <j...@jms.id.au>
Reviewed-by: Cédric Le Goater <c...@kaod.org>
Thanks,
C.
> ---
> hw/misc/aspeed_sdmc.c | 27 ++++++++-------------------
> include/hw/misc/aspeed_sdmc.h | 1 +
> 2 files changed, 9 insertions(+), 19 deletions(-)
>
> diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c
> index 0df008e52a18..24fd4aee2d82 100644
> --- a/hw/misc/aspeed_sdmc.c
> +++ b/hw/misc/aspeed_sdmc.c
> @@ -126,10 +126,12 @@ static void aspeed_sdmc_write(void *opaque, hwaddr
> addr, uint64_t data,
> case AST2400_A0_SILICON_REV:
> case AST2400_A1_SILICON_REV:
> data &= ~ASPEED_SDMC_READONLY_MASK;
> + data |= s->fixed_conf;
> break;
> case AST2500_A0_SILICON_REV:
> case AST2500_A1_SILICON_REV:
> data &= ~ASPEED_SDMC_AST2500_READONLY_MASK;
> + data |= s->fixed_conf;
> break;
> default:
> g_assert_not_reached();
> @@ -198,25 +200,7 @@ static void aspeed_sdmc_reset(DeviceState *dev)
> memset(s->regs, 0, sizeof(s->regs));
>
> /* Set ram size bit and defaults values */
> - switch (s->silicon_rev) {
> - case AST2400_A0_SILICON_REV:
> - case AST2400_A1_SILICON_REV:
> - s->regs[R_CONF] |=
> - ASPEED_SDMC_VGA_COMPAT |
> - ASPEED_SDMC_DRAM_SIZE(s->ram_bits);
> - break;
> -
> - case AST2500_A0_SILICON_REV:
> - case AST2500_A1_SILICON_REV:
> - s->regs[R_CONF] |=
> - ASPEED_SDMC_HW_VERSION(1) |
> - ASPEED_SDMC_VGA_APERTURE(ASPEED_SDMC_VGA_64MB) |
> - ASPEED_SDMC_DRAM_SIZE(s->ram_bits);
> - break;
> -
> - default:
> - g_assert_not_reached();
> - }
> + s->regs[R_CONF] = s->fixed_conf;
> }
>
> static void aspeed_sdmc_realize(DeviceState *dev, Error **errp)
> @@ -234,10 +218,15 @@ static void aspeed_sdmc_realize(DeviceState *dev, Error
> **errp)
> case AST2400_A0_SILICON_REV:
> case AST2400_A1_SILICON_REV:
> s->ram_bits = ast2400_rambits(s);
> + s->fixed_conf = ASPEED_SDMC_VGA_COMPAT |
> + ASPEED_SDMC_DRAM_SIZE(s->ram_bits);
> break;
> case AST2500_A0_SILICON_REV:
> case AST2500_A1_SILICON_REV:
> s->ram_bits = ast2500_rambits(s);
> + s->fixed_conf = ASPEED_SDMC_HW_VERSION(1) |
> + ASPEED_SDMC_VGA_APERTURE(ASPEED_SDMC_VGA_64MB) |
> + ASPEED_SDMC_DRAM_SIZE(s->ram_bits);
> break;
> default:
> g_assert_not_reached();
> diff --git a/include/hw/misc/aspeed_sdmc.h b/include/hw/misc/aspeed_sdmc.h
> index 682f0f5d56dc..e079c66a7d73 100644
> --- a/include/hw/misc/aspeed_sdmc.h
> +++ b/include/hw/misc/aspeed_sdmc.h
> @@ -27,6 +27,7 @@ typedef struct AspeedSDMCState {
> uint32_t silicon_rev;
> uint32_t ram_bits;
> uint64_t ram_size;
> + uint32_t fixed_conf;
>
> } AspeedSDMCState;
>
>
