[v1] Support AST2700 A1

[PATCH v1 11/18] hw/misc/aspeed_scu: Add Support for AST2700/AST2750 A1 Silicon Revisions

Posted by Jamin Lin via 2 months, 2 weeks ago

Added new definitions for AST2700_A1_SILICON_REV and AST2750_A1_SILICON_REV to
identify the A1 silicon revisions.

Update "aspeed_ast2700_scu_reset" to set the silicon_rev field in the SCU
registers.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/misc/aspeed_scu.c         | 3 +++
 include/hw/misc/aspeed_scu.h | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index bac1441b06..f049a9fd96 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -559,6 +559,8 @@ static uint32_t aspeed_silicon_revs[] = {
     AST2700_A0_SILICON_REV,
     AST2720_A0_SILICON_REV,
     AST2750_A0_SILICON_REV,
+    AST2700_A1_SILICON_REV,
+    AST2750_A1_SILICON_REV,
 };
 
 bool is_supported_silicon_rev(uint32_t silicon_rev)
@@ -938,6 +940,7 @@ static void aspeed_ast2700_scu_reset(DeviceState *dev)
     AspeedSCUClass *asc = ASPEED_SCU_GET_CLASS(dev);
 
     memcpy(s->regs, asc->resets, asc->nr_regs * 4);
+    s->regs[AST2700_SILICON_REV] = s->silicon_rev;
 }
 
 static void aspeed_2700_scu_class_init(ObjectClass *klass, void *data)
diff --git a/include/hw/misc/aspeed_scu.h b/include/hw/misc/aspeed_scu.h
index 356be95e45..684b48b722 100644
--- a/include/hw/misc/aspeed_scu.h
+++ b/include/hw/misc/aspeed_scu.h
@@ -54,6 +54,8 @@ struct AspeedSCUState {
 #define AST2700_A0_SILICON_REV   0x06000103U
 #define AST2720_A0_SILICON_REV   0x06000203U
 #define AST2750_A0_SILICON_REV   0x06000003U
+#define AST2700_A1_SILICON_REV   0x06010103U
+#define AST2750_A1_SILICON_REV   0x06010003U
 
 #define ASPEED_IS_AST2500(si_rev)     ((((si_rev) >> 24) & 0xff) == 0x04)
 
-- 
2.34.1

Re: [PATCH v1 11/18] hw/misc/aspeed_scu: Add Support for AST2700/AST2750 A1 Silicon Revisions

Posted by Andrew Jeffery 2 months ago

On Tue, 2025-01-21 at 15:04 +0800, Jamin Lin wrote:
> Added new definitions for AST2700_A1_SILICON_REV and
> AST2750_A1_SILICON_REV to
> identify the A1 silicon revisions.
> 
> Update "aspeed_ast2700_scu_reset" to set the silicon_rev field in the
> SCU
> registers.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>  hw/misc/aspeed_scu.c         | 3 +++
>  include/hw/misc/aspeed_scu.h | 2 ++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> index bac1441b06..f049a9fd96 100644
> --- a/hw/misc/aspeed_scu.c
> +++ b/hw/misc/aspeed_scu.c
> @@ -559,6 +559,8 @@ static uint32_t aspeed_silicon_revs[] = {
>      AST2700_A0_SILICON_REV,
>      AST2720_A0_SILICON_REV,
>      AST2750_A0_SILICON_REV,
> +    AST2700_A1_SILICON_REV,
> +    AST2750_A1_SILICON_REV,
>  };
>  
>  bool is_supported_silicon_rev(uint32_t silicon_rev)
> @@ -938,6 +940,7 @@ static void aspeed_ast2700_scu_reset(DeviceState
> *dev)
>      AspeedSCUClass *asc = ASPEED_SCU_GET_CLASS(dev);
>  
>      memcpy(s->regs, asc->resets, asc->nr_regs * 4);
> +    s->regs[AST2700_SILICON_REV] = s->silicon_rev;

Has s->silicon_rev been set?

Should we now remove the AST2700_SILICON_REV entry from asc->resets?

This seems like a fix regardless. Perhaps separate it from the addition
of the new silicon IDs?

>  }
>  
>  static void aspeed_2700_scu_class_init(ObjectClass *klass, void
> *data)
> diff --git a/include/hw/misc/aspeed_scu.h
> b/include/hw/misc/aspeed_scu.h
> index 356be95e45..684b48b722 100644
> --- a/include/hw/misc/aspeed_scu.h
> +++ b/include/hw/misc/aspeed_scu.h
> @@ -54,6 +54,8 @@ struct AspeedSCUState {
>  #define AST2700_A0_SILICON_REV   0x06000103U
>  #define AST2720_A0_SILICON_REV   0x06000203U
>  #define AST2750_A0_SILICON_REV   0x06000003U
> +#define AST2700_A1_SILICON_REV   0x06010103U
> +#define AST2750_A1_SILICON_REV   0x06010003U

These look fine.

Andrew

>  
>  #define ASPEED_IS_AST2500(si_rev)     ((((si_rev) >> 24) & 0xff) ==
> 0x04)
>

RE: [PATCH v1 11/18] hw/misc/aspeed_scu: Add Support for AST2700/AST2750 A1 Silicon Revisions

Posted by Jamin Lin 2 months ago

Hi Andrew,

> From: Andrew Jeffery <andrew@codeconstruct.com.au>
> Sent: Thursday, January 30, 2025 12:05 PM
> To: Jamin Lin <jamin_lin@aspeedtech.com>; Cédric Le Goater <clg@kaod.org>;
> Peter Maydell <peter.maydell@linaro.org>; Steven Lee
> <steven_lee@aspeedtech.com>; Troy Lee <leetroy@gmail.com>; Joel Stanley
> <joel@jms.id.au>; open list:ASPEED BMCs <qemu-arm@nongnu.org>; open
> list:All patches CC here <qemu-devel@nongnu.org>
> Cc: Troy Lee <troy_lee@aspeedtech.com>; Yunlin Tang
> <yunlin.tang@aspeedtech.com>
> Subject: Re: [PATCH v1 11/18] hw/misc/aspeed_scu: Add Support for
> AST2700/AST2750 A1 Silicon Revisions
> 
> On Tue, 2025-01-21 at 15:04 +0800, Jamin Lin wrote:
> > Added new definitions for AST2700_A1_SILICON_REV and
> > AST2750_A1_SILICON_REV to identify the A1 silicon revisions.
> >
> > Update "aspeed_ast2700_scu_reset" to set the silicon_rev field in the
> > SCU registers.
> >
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > ---
> >  hw/misc/aspeed_scu.c         | 3 +++
> >  include/hw/misc/aspeed_scu.h | 2 ++
> >  2 files changed, 5 insertions(+)
> >
> > diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c index
> > bac1441b06..f049a9fd96 100644
> > --- a/hw/misc/aspeed_scu.c
> > +++ b/hw/misc/aspeed_scu.c
> > @@ -559,6 +559,8 @@ static uint32_t aspeed_silicon_revs[] = {
> >      AST2700_A0_SILICON_REV,
> >      AST2720_A0_SILICON_REV,
> >      AST2750_A0_SILICON_REV,
> > +    AST2700_A1_SILICON_REV,
> > +    AST2750_A1_SILICON_REV,
> >  };
> >
> >  bool is_supported_silicon_rev(uint32_t silicon_rev) @@ -938,6 +940,7
> > @@ static void aspeed_ast2700_scu_reset(DeviceState
> > *dev)
> >      AspeedSCUClass *asc = ASPEED_SCU_GET_CLASS(dev);
> >
> >      memcpy(s->regs, asc->resets, asc->nr_regs * 4);
> > +    s->regs[AST2700_SILICON_REV] = s->silicon_rev;
> 
> Has s->silicon_rev been set?
> 
> Should we now remove the AST2700_SILICON_REV entry from asc->resets?
> 
The value of s->silicon_rev is set at the SoC layer.
If we remove this line, the system will display " UnKnow-SOC".

> This seems like a fix regardless. Perhaps separate it from the addition of the
> new silicon IDs?
> 
Yes, I noticed that s->silicon_rev cannot be set at the SOC layer.
For example: I am unable to set silicon_rev for AST2700 A0 and A1 in hw/arm/aspeed_ast27x0.c.
I will split this into two separate patches.
One patch to fix silicon_rev and another patch for A1 silicon revision definition.

Thanks-Jamin
> >  }
> >
> >  static void aspeed_2700_scu_class_init(ObjectClass *klass, void
> > *data)
> > diff --git a/include/hw/misc/aspeed_scu.h
> > b/include/hw/misc/aspeed_scu.h index 356be95e45..684b48b722 100644
> > --- a/include/hw/misc/aspeed_scu.h
> > +++ b/include/hw/misc/aspeed_scu.h
> > @@ -54,6 +54,8 @@ struct AspeedSCUState {
> >  #define AST2700_A0_SILICON_REV   0x06000103U
> >  #define AST2720_A0_SILICON_REV   0x06000203U
> >  #define AST2750_A0_SILICON_REV   0x06000003U
> > +#define AST2700_A1_SILICON_REV   0x06010103U #define
> > +AST2750_A1_SILICON_REV   0x06010003U
> 
> These look fine.
> 
> Andrew
> 
> >
> >  #define ASPEED_IS_AST2500(si_rev)     ((((si_rev) >> 24) & 0xff) ==
> > 0x04)
> >

Re: [PATCH v1 11/18] hw/misc/aspeed_scu: Add Support for AST2700/AST2750 A1 Silicon Revisions

Posted by Cédric Le Goater 2 months ago

Hello Jamin,

>>> --- a/hw/misc/aspeed_scu.c
>>> +++ b/hw/misc/aspeed_scu.c
>>> @@ -559,6 +559,8 @@ static uint32_t aspeed_silicon_revs[] = {
>>>       AST2700_A0_SILICON_REV,
>>>       AST2720_A0_SILICON_REV,
>>>       AST2750_A0_SILICON_REV,
>>> +    AST2700_A1_SILICON_REV,
>>> +    AST2750_A1_SILICON_REV,
>>>   };
>>>
>>>   bool is_supported_silicon_rev(uint32_t silicon_rev) @@ -938,6 +940,7
>>> @@ static void aspeed_ast2700_scu_reset(DeviceState
>>> *dev)
>>>       AspeedSCUClass *asc = ASPEED_SCU_GET_CLASS(dev);
>>>
>>>       memcpy(s->regs, asc->resets, asc->nr_regs * 4);
>>> +    s->regs[AST2700_SILICON_REV] = s->silicon_rev;
>>
>> Has s->silicon_rev been set?
>>
>> Should we now remove the AST2700_SILICON_REV entry from asc->resets?
>>
> The value of s->silicon_rev is set at the SoC layer.
> If we remove this line, the system will display " UnKnow-SOC".
> 
>> This seems like a fix regardless. Perhaps separate it from the addition of the
>> new silicon IDs?
>>
> Yes, I noticed that s->silicon_rev cannot be set at the SOC layer.
> For example: I am unable to set silicon_rev for AST2700 A0 and A1 in hw/arm/aspeed_ast27x0.c.
> I will split this into two separate patches.
> One patch to fix silicon_rev and another patch for A1 silicon revision definition.

Is the AST2700 SoC different from the other SoCs AST 2[456]00 ?
If not please check the _scu_reset routines.

Thanks,

C.