Re: [PATCH 2/2] aspeed/scu: Implement chip ID register

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 1/21/20 2:33 AM, Joel Stanley wrote:
> This returns a fixed but non-zero value for the chip id.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>   hw/misc/aspeed_scu.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> index 7108cad8c6a7..19d1780a40da 100644
> --- a/hw/misc/aspeed_scu.c
> +++ b/hw/misc/aspeed_scu.c
> @@ -77,6 +77,8 @@
>   #define CPU2_BASE_SEG4       TO_REG(0x110)
>   #define CPU2_BASE_SEG5       TO_REG(0x114)
>   #define CPU2_CACHE_CTRL      TO_REG(0x118)
> +#define CHIP_ID0             TO_REG(0x150)
> +#define CHIP_ID1             TO_REG(0x154)
>   #define UART_HPLL_CLK        TO_REG(0x160)
>   #define PCIE_CTRL            TO_REG(0x180)
>   #define BMC_MMIO_CTRL        TO_REG(0x184)
> @@ -115,6 +117,8 @@
>   #define AST2600_HW_STRAP2_PROT    TO_REG(0x518)
>   #define AST2600_RNG_CTRL          TO_REG(0x524)
>   #define AST2600_RNG_DATA          TO_REG(0x540)
> +#define AST2600_CHIP_ID0          TO_REG(0x5B0)
> +#define AST2600_CHIP_ID1          TO_REG(0x5B4)
>   
>   #define AST2600_CLK TO_REG(0x40)
>   
> @@ -182,6 +186,8 @@ static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
>        [CPU2_BASE_SEG1]  = 0x80000000U,
>        [CPU2_BASE_SEG4]  = 0x1E600000U,
>        [CPU2_BASE_SEG5]  = 0xC0000000U,
> +     [CHIP_ID0]        = 0x1234ABCDU,
> +     [CHIP_ID1]        = 0x88884444U,
>        [UART_HPLL_CLK]   = 0x00001903U,
>        [PCIE_CTRL]       = 0x0000007BU,
>        [BMC_DEV_ID]      = 0x00002402U
> @@ -307,6 +313,8 @@ static void aspeed_ast2500_scu_write(void *opaque, hwaddr offset,
>       case RNG_DATA:
>       case FREE_CNTR4:
>       case FREE_CNTR4_EXT:
> +    case CHIP_ID0:
> +    case CHIP_ID1:
>           qemu_log_mask(LOG_GUEST_ERROR,
>                         "%s: Write to read-only offset 0x%" HWADDR_PRIx "\n",
>                         __func__, offset);
> @@ -620,6 +628,8 @@ static void aspeed_ast2600_scu_write(void *opaque, hwaddr offset,
>       case AST2600_RNG_DATA:
>       case AST2600_SILICON_REV:
>       case AST2600_SILICON_REV2:
> +    case AST2600_CHIP_ID0:
> +    case AST2600_CHIP_ID1:
>           /* Add read only registers here */
>           qemu_log_mask(LOG_GUEST_ERROR,
>                         "%s: Write to read-only offset 0x%" HWADDR_PRIx "\n",
> @@ -648,6 +658,9 @@ static const uint32_t ast2600_a0_resets[ASPEED_AST2600_SCU_NR_REGS] = {
>       [AST2600_CLK_STOP_CTRL2]    = 0xFFF0FFF0,
>       [AST2600_SDRAM_HANDSHAKE]   = 0x00000040,  /* SoC completed DRAM init */
>       [AST2600_HPLL_PARAM]        = 0x1000405F,
> +    [AST2600_CHIP_ID0]          = 0x1234ABCD,
> +    [AST2600_CHIP_ID1]          = 0x88884444,

Since this doesn't match the datasheet, can you add a comment /* 
Arbitrary non-zero value */?

With it:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +
>   };
>   
>   static void aspeed_ast2600_scu_reset(DeviceState *dev)
>