Re: [PATCH v3 07/17] hw/misc: Add support for NPCM8XX GCR

[Philippe Mathieu-Daudé]

Hi Hao,

On 6/2/25 02:30, Hao Wu wrote:
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Hao Wu <wuhaotsh@google.com>
> ---
>   hw/misc/npcm_gcr.c         | 131 ++++++++++++++++++++++++++++++++++++-
>   include/hw/misc/npcm_gcr.h |   6 +-
>   2 files changed, 134 insertions(+), 3 deletions(-)


> +    NPCM8XX_GCR_WD0RCRBLK,
> +    NPCM8XX_GCR_WD1RCRBLK,
> +    NPCM8XX_GCR_WD2RCRBLK,
> +    NPCM8XX_GCR_SWRSTC1BLK,
> +    NPCM8XX_GCR_SWRSTC2BLK,
> +    NPCM8XX_GCR_SWRSTC3BLK,
> +    NPCM8XX_GCR_TIPRSTCBLK,
> +    NPCM8XX_GCR_CORSTCBLK,
> +    /* 64 scratch pad registers start here. 0xe00 ~ 0xefc */
> +    NPCM8XX_GCR_SCRPAD_00       = 0xe00 / sizeof(uint32_t),
> +    /* 32 semaphore registers start here. 0xf00 ~ 0xf7c */
> +    NPCM8XX_GCR_GP_SEMFR_00     = 0xf00 / sizeof(uint32_t),

Alternatively:

       NPCM8XX_GCR_GP_SEMFR_31     = 0xf7c ...

> +    NPCM8XX_GCR_REGS_END        = 0xf80 / sizeof(uint32_t),

Then no need for NPCM8XX_GCR_REGS_END, we have NPCM8XX_GCR_NR_REGS.

> +};
> +
> +static const uint32_t npcm8xx_cold_reset_values[NPCM8XX_GCR_NR_REGS] = {
> +    [NPCM8XX_GCR_PDID]          = 0x04a35850,   /* Arbel A1 */
> +    [NPCM8XX_GCR_MISCPE]        = 0x0000ffff,
> +    [NPCM8XX_GCR_A35_MODE]      = 0xfff4ff30,
> +    [NPCM8XX_GCR_SPSWC]         = 0x00000003,
> +    [NPCM8XX_GCR_INTCR]         = 0x0010035e,
> +    [NPCM8XX_GCR_HIFCR]         = 0x0000004e,
> +    [NPCM8XX_GCR_SD2SUR1]       = 0xfdc80000,
> +    [NPCM8XX_GCR_SD2SUR2]       = 0x5200b130,
> +    [NPCM8XX_GCR_INTCR2]        = (1U << 19),   /* DDR initialized */
> +    [NPCM8XX_GCR_RESSR]         = 0x80000000,
> +    [NPCM8XX_GCR_DAVCLVLR]      = 0x5a00f3cf,
> +    [NPCM8XX_GCR_INTCR3]        = 0x5e001002,
> +    [NPCM8XX_GCR_VSRCR]         = 0x00004800,
> +    [NPCM8XX_GCR_SCRPAD]        = 0x00000008,
> +    [NPCM8XX_GCR_USB1PHYCTL]    = 0x034730e4,
> +    [NPCM8XX_GCR_USB2PHYCTL]    = 0x034730e4,
> +    [NPCM8XX_GCR_USB3PHYCTL]    = 0x034730e4,
> +    /* All 32 semaphores should be initialized to 1. */
> +    [NPCM8XX_GCR_GP_SEMFR_00...NPCM8XX_GCR_REGS_END - 1] = 0x00000001,

       [NPCM8XX_GCR_GP_SEMFR_00 ...
        NPCM8XX_GCR_GP_SEMFR_31] = 1;

> +};
> +
>   static uint64_t npcm_gcr_read(void *opaque, hwaddr offset, unsigned size)
>   {
>       uint32_t reg = offset / sizeof(uint32_t);
> @@ -263,6 +375,18 @@ static void npcm7xx_gcr_class_init(ObjectClass *klass, 
> void *data)
>       c->cold_reset_values = npcm7xx_cold_reset_values;
>   }
>   
> +static void npcm8xx_gcr_class_init(ObjectClass *klass, void *data)
> +{
> +    NPCMGCRClass *c = NPCM_GCR_CLASS(klass);
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    QEMU_BUILD_BUG_ON(NPCM8XX_GCR_REGS_END > NPCM_GCR_MAX_NR_REGS);
> +    QEMU_BUILD_BUG_ON(NPCM8XX_GCR_REGS_END != NPCM8XX_GCR_NR_REGS);

Not sure these checks are useful, but as you prefer.

> +    dc->desc = "NPCM8xx System Global Control Registers";
> +    c->nr_regs = NPCM8XX_GCR_NR_REGS;
> +    c->cold_reset_values = npcm8xx_cold_reset_values;
> +}


> @@ -54,8 +54,9 @@
>    * Number of registers in our device state structure. Don't change this 
> without
>    * incrementing the version_id in the vmstate.
>    */
> -#define NPCM_GCR_MAX_NR_REGS NPCM7XX_GCR_NR_REGS
> +#define NPCM_GCR_MAX_NR_REGS NPCM8XX_GCR_NR_REGS
>   #define NPCM7XX_GCR_NR_REGS (0x148 / sizeof(uint32_t))
> +#define NPCM8XX_GCR_NR_REGS (0xf80 / sizeof(uint32_t))
>   
>   typedef struct NPCMGCRState {
>       SysBusDevice parent;
> @@ -78,6 +79,7 @@ typedef struct NPCMGCRClass {
>   
>   #define TYPE_NPCM_GCR "npcm-gcr"
>   #define TYPE_NPCM7XX_GCR "npcm7xx-gcr"
> +#define TYPE_NPCM8XX_GCR "npcm8xx-gcr"
>   OBJECT_DECLARE_TYPE(NPCMGCRState, NPCMGCRClass, NPCM_GCR)
>   
>   #endif /* NPCM_GCR_H */