Re: [PATCH 08/10] aspeed: Emulate the AST2600A3

[Cédric Le Goater]

On 8/9/21 5:55 PM, Philippe Mathieu-Daudé wrote:
> On 8/9/21 3:15 PM, Cédric Le Goater wrote:
>> From: Joel Stanley <joel@jms.id.au>
>>
>> This is the latest revision of the ASPEED 2600 SoC. As there is no
>> need to model multiple revisions of the same SoC for the moment,
>> update the SCU AST2600 to model the A3 revision instead of the A1 and
>> adapt the AST2600 SoC and machines.
>>
>> Reset values are taken from v8 of the datasheet.
>>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> [ clg: - Introduced an Aspeed "ast2600-a3" SoC class
>>        - Commit log update ]
>> Message-Id: <20210407171637.777743-21-clg@kaod.org>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Message-Id: <20210629142336.750058-3-clg@kaod.org>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  include/hw/misc/aspeed_scu.h |  2 ++
>>  hw/arm/aspeed.c              |  6 +++---
>>  hw/arm/aspeed_ast2600.c      |  6 +++---
>>  hw/misc/aspeed_scu.c         | 36 +++++++++++++++++++++++++++++-------
>>  4 files changed, 37 insertions(+), 13 deletions(-)
> 
>> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
>> index 40a38ebd8549..05edebedeb46 100644
>> --- a/hw/misc/aspeed_scu.c
>> +++ b/hw/misc/aspeed_scu.c
>> @@ -101,14 +101,24 @@
>>  #define AST2600_CLK_STOP_CTRL_CLR TO_REG(0x84)
>>  #define AST2600_CLK_STOP_CTRL2     TO_REG(0x90)
>>  #define AST2600_CLK_STOP_CTRL2_CLR TO_REG(0x94)
>> +#define AST2600_DEBUG_CTRL        TO_REG(0xC8)
>> +#define AST2600_DEBUG_CTRL2       TO_REG(0xD8)
>>  #define AST2600_SDRAM_HANDSHAKE   TO_REG(0x100)
>>  #define AST2600_HPLL_PARAM        TO_REG(0x200)
>>  #define AST2600_HPLL_EXT          TO_REG(0x204)
>> +#define AST2600_APLL_PARAM        TO_REG(0x210)
>> +#define AST2600_APLL_EXT          TO_REG(0x214)
>> +#define AST2600_MPLL_PARAM        TO_REG(0x220)
>>  #define AST2600_MPLL_EXT          TO_REG(0x224)
>> +#define AST2600_EPLL_PARAM        TO_REG(0x240)
>>  #define AST2600_EPLL_EXT          TO_REG(0x244)
>> +#define AST2600_DPLL_PARAM        TO_REG(0x260)
>> +#define AST2600_DPLL_EXT          TO_REG(0x264)
>>  #define AST2600_CLK_SEL           TO_REG(0x300)
>>  #define AST2600_CLK_SEL2          TO_REG(0x304)
>> -#define AST2600_CLK_SEL3          TO_REG(0x310)
>> +#define AST2600_CLK_SEL3          TO_REG(0x308)
> 
> Is it a bugfix? Otherwise this is annoying.

This is a bug in the model. These registers have the same layout
on the A1.

Thanks,

C.

> 
> Maybe:
> 
>  #define AST2600A1_CLK_SEL3          TO_REG(0x310)
>  #define AST2600A3_CLK_SEL3          TO_REG(0x308)
> 
> and...
> 
>> +#define AST2600_CLK_SEL4          TO_REG(0x310)
>> +#define AST2600_CLK_SEL5          TO_REG(0x314)
>>  #define AST2600_HW_STRAP1         TO_REG(0x500)
>>  #define AST2600_HW_STRAP1_CLR     TO_REG(0x504)
>>  #define AST2600_HW_STRAP1_PROT    TO_REG(0x508)
>> @@ -433,6 +443,8 @@ static uint32_t aspeed_silicon_revs[] = {
>>      AST2500_A1_SILICON_REV,
>>      AST2600_A0_SILICON_REV,
>>      AST2600_A1_SILICON_REV,
>> +    AST2600_A2_SILICON_REV,
>> +    AST2600_A3_SILICON_REV,
>>  };
>>  
>>  bool is_supported_silicon_rev(uint32_t silicon_rev)
>> @@ -651,16 +663,26 @@ static const MemoryRegionOps aspeed_ast2600_scu_ops = {
>>      .valid.unaligned = false,
>>  };
>>  
>> -static const uint32_t ast2600_a1_resets[ASPEED_AST2600_SCU_NR_REGS] = {
>> +static const uint32_t ast2600_a3_resets[ASPEED_AST2600_SCU_NR_REGS] = {
>>      [AST2600_SYS_RST_CTRL]      = 0xF7C3FED8,
>> -    [AST2600_SYS_RST_CTRL2]     = 0xFFFFFFFC,
>> +    [AST2600_SYS_RST_CTRL2]     = 0x0DFFFFFC,
>>      [AST2600_CLK_STOP_CTRL]     = 0xFFFF7F8A,
>>      [AST2600_CLK_STOP_CTRL2]    = 0xFFF0FFF0,
>> +    [AST2600_DEBUG_CTRL]        = 0x00000FFF,
>> +    [AST2600_DEBUG_CTRL2]       = 0x000000FF,
>>      [AST2600_SDRAM_HANDSHAKE]   = 0x00000000,
>> -    [AST2600_HPLL_PARAM]        = 0x1000405F,
>> +    [AST2600_HPLL_PARAM]        = 0x1000408F,
>> +    [AST2600_APLL_PARAM]        = 0x1000405F,
>> +    [AST2600_MPLL_PARAM]        = 0x1008405F,
>> +    [AST2600_EPLL_PARAM]        = 0x1004077F,
>> +    [AST2600_DPLL_PARAM]        = 0x1078405F,
>> +    [AST2600_CLK_SEL]           = 0xF3940000,
>> +    [AST2600_CLK_SEL2]          = 0x00700000,
>> +    [AST2600_CLK_SEL3]          = 0x00000000,
> 
> ... use AST2600A3_CLK_SEL3 here?
> 
> So someone wanting the emulate the A1 doesn't get
> the nasty bug of having CLK_SEL3 misplaced.
> 
>> +    [AST2600_CLK_SEL4]          = 0xF3F40000,
>> +    [AST2600_CLK_SEL5]          = 0x30000000,
>>      [AST2600_CHIP_ID0]          = 0x1234ABCD,
>>      [AST2600_CHIP_ID1]          = 0x88884444,
>> -
>>  };