* [PATCH v1 0/1] hw: aspeed_scu: Add AST2600 hpll calculation function
@ 2022-03-14 9:54 Steven Lee
2022-03-14 9:54 ` [PATCH v1 1/1] " Steven Lee
0 siblings, 1 reply; 5+ messages in thread
From: Steven Lee @ 2022-03-14 9:54 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell, Andrew Jeffery,
Joel Stanley, open list:ASPEED BMCs,
open list:All patches CC here
Cc: jamin_lin, troy_lee, steven_lee
AST2600's HPLL register offset and bit definition are different from AST2500.
The patch series adds a hpll calculation function for ast2600 and modify apb frequency
calculation function based on SCU200 register description and note in ast2600v11.pdf.
Please help to review.
Thanks,
Steven
Steven Lee (1):
hw: aspeed_scu: Add AST2600 hpll calculation function
hw/misc/aspeed_scu.c | 43 ++++++++++++++++++++++++++++++++----
include/hw/misc/aspeed_scu.h | 17 ++++++++++++++
2 files changed, 56 insertions(+), 4 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v1 1/1] hw: aspeed_scu: Add AST2600 hpll calculation function
2022-03-14 9:54 [PATCH v1 0/1] hw: aspeed_scu: Add AST2600 hpll calculation function Steven Lee
@ 2022-03-14 9:54 ` Steven Lee
2022-03-14 12:21 ` Cédric Le Goater
0 siblings, 1 reply; 5+ messages in thread
From: Steven Lee @ 2022-03-14 9:54 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell, Andrew Jeffery,
Joel Stanley, open list:ASPEED BMCs,
open list:All patches CC here
Cc: jamin_lin, troy_lee, steven_lee
AST2600's HPLL register offset and bit definition are different from
AST2500. Add a hpll calculation function for ast2600 and modify apb frequency
calculation function based on SCU200 register description in ast2600v11.pdf.
Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
---
hw/misc/aspeed_scu.c | 43 ++++++++++++++++++++++++++++++++----
include/hw/misc/aspeed_scu.h | 17 ++++++++++++++
2 files changed, 56 insertions(+), 4 deletions(-)
diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index d06e179a6e..3b11e98d66 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -205,6 +205,8 @@ static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
[BMC_DEV_ID] = 0x00002402U
};
+static uint32_t aspeed_2600_scu_calc_hpll(AspeedSCUState *s, uint32_t hpll_reg);
+
static uint32_t aspeed_scu_get_random(void)
{
uint32_t num;
@@ -215,9 +217,19 @@ static uint32_t aspeed_scu_get_random(void)
uint32_t aspeed_scu_get_apb_freq(AspeedSCUState *s)
{
AspeedSCUClass *asc = ASPEED_SCU_GET_CLASS(s);
- uint32_t hpll = asc->calc_hpll(s, s->regs[HPLL_PARAM]);
+ uint32_t hpll, hpll_reg, clk_sel_reg;
+
+ if (asc->calc_hpll == aspeed_2600_scu_calc_hpll) {
+ hpll_reg = s->regs[AST2600_HPLL_PARAM];
+ clk_sel_reg = s->regs[AST2600_CLK_SEL];
+ } else {
+ hpll_reg = s->regs[HPLL_PARAM];
+ clk_sel_reg = s->regs[CLK_SEL];
+ }
+
+ hpll = asc->calc_hpll(s, hpll_reg);
- return hpll / (SCU_CLK_GET_PCLK_DIV(s->regs[CLK_SEL]) + 1)
+ return hpll / (SCU_CLK_GET_PCLK_DIV(clk_sel_reg) + 1)
/ asc->apb_divider;
}
@@ -357,7 +369,10 @@ static const MemoryRegionOps aspeed_ast2500_scu_ops = {
static uint32_t aspeed_scu_get_clkin(AspeedSCUState *s)
{
- if (s->hw_strap1 & SCU_HW_STRAP_CLK_25M_IN) {
+ AspeedSCUClass *asc = ASPEED_SCU_GET_CLASS(s);
+
+ if (s->hw_strap1 & SCU_HW_STRAP_CLK_25M_IN ||
+ asc->calc_hpll == aspeed_2600_scu_calc_hpll) {
return 25000000;
} else if (s->hw_strap1 & SCU_HW_STRAP_CLK_48M_IN) {
return 48000000;
@@ -426,6 +441,26 @@ static uint32_t aspeed_2500_scu_calc_hpll(AspeedSCUState *s, uint32_t hpll_reg)
return clkin * multiplier;
}
+static uint32_t aspeed_2600_scu_calc_hpll(AspeedSCUState *s, uint32_t hpll_reg)
+{
+ uint32_t multiplier = 1;
+ uint32_t clkin = aspeed_scu_get_clkin(s);
+
+ if (hpll_reg & SCU_AST2600_H_PLL_OFF) {
+ return 0;
+ }
+
+ if (!(hpll_reg & SCU_H_PLL_BYPASS_EN)) {
+ uint32_t p = (hpll_reg >> 19) & 0xf;
+ uint32_t n = (hpll_reg >> 13) & 0x3f;
+ uint32_t m = hpll_reg & 0x1fff;
+
+ multiplier = ((m + 1) / (n + 1)) / (p + 1);
+ }
+
+ return clkin * multiplier;
+}
+
static void aspeed_scu_reset(DeviceState *dev)
{
AspeedSCUState *s = ASPEED_SCU(dev);
@@ -716,7 +751,7 @@ static void aspeed_2600_scu_class_init(ObjectClass *klass, void *data)
dc->desc = "ASPEED 2600 System Control Unit";
dc->reset = aspeed_ast2600_scu_reset;
asc->resets = ast2600_a3_resets;
- asc->calc_hpll = aspeed_2500_scu_calc_hpll; /* No change since AST2500 */
+ asc->calc_hpll = aspeed_2600_scu_calc_hpll;
asc->apb_divider = 4;
asc->nr_regs = ASPEED_AST2600_SCU_NR_REGS;
asc->ops = &aspeed_ast2600_scu_ops;
diff --git a/include/hw/misc/aspeed_scu.h b/include/hw/misc/aspeed_scu.h
index c14aff2bcb..91c500c5bc 100644
--- a/include/hw/misc/aspeed_scu.h
+++ b/include/hw/misc/aspeed_scu.h
@@ -316,4 +316,21 @@ uint32_t aspeed_scu_get_apb_freq(AspeedSCUState *s);
SCU_HW_STRAP_VGA_SIZE_SET(VGA_16M_DRAM) | \
SCU_AST2500_HW_STRAP_RESERVED1)
+/* SCU200 H-PLL Parameter Register (for Aspeed AST2600 SOC)
+ *
+ * 28:26 H-PLL Parameters
+ * 25 Enable H-PLL reset
+ * 24 Enable H-PLL bypass mode
+ * 23 Turn off H-PLL
+ * 22:19 H-PLL Post Divider (P)
+ * 18:13 H-PLL Numerator (M)
+ * 12:0 H-PLL Denumerator (N)
+ *
+ * (Output frequency) = CLKIN(25MHz) * [(M+1) / (N+1)] / (P+1)
+ *
+ * The default frequency is 1200Mhz when CLKIN = 25MHz
+ */
+#define SCU_AST2600_H_PLL_BYPASS_EN (0x1 << 24)
+#define SCU_AST2600_H_PLL_OFF (0x1 << 23)
+
#endif /* ASPEED_SCU_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/1] hw: aspeed_scu: Add AST2600 hpll calculation function
2022-03-14 9:54 ` [PATCH v1 1/1] " Steven Lee
@ 2022-03-14 12:21 ` Cédric Le Goater
2022-03-15 2:42 ` Steven Lee
0 siblings, 1 reply; 5+ messages in thread
From: Cédric Le Goater @ 2022-03-14 12:21 UTC (permalink / raw)
To: Steven Lee, Peter Maydell, Andrew Jeffery, Joel Stanley,
open list:ASPEED BMCs, open list:All patches CC here
Cc: troy_lee, jamin_lin
Hello Steven,
On 3/14/22 10:54, Steven Lee wrote:
> AST2600's HPLL register offset and bit definition are different from
> AST2500. Add a hpll calculation function for ast2600 and modify apb frequency
> calculation function based on SCU200 register description in ast2600v11.pdf.
It looks good. A few minor comments on the modeling.
> Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
> ---
> hw/misc/aspeed_scu.c | 43 ++++++++++++++++++++++++++++++++----
> include/hw/misc/aspeed_scu.h | 17 ++++++++++++++
> 2 files changed, 56 insertions(+), 4 deletions(-)
>
> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> index d06e179a6e..3b11e98d66 100644
> --- a/hw/misc/aspeed_scu.c
> +++ b/hw/misc/aspeed_scu.c
> @@ -205,6 +205,8 @@ static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
> [BMC_DEV_ID] = 0x00002402U
> };
>
> +static uint32_t aspeed_2600_scu_calc_hpll(AspeedSCUState *s, uint32_t hpll_reg);
> +
> static uint32_t aspeed_scu_get_random(void)
> {
> uint32_t num;
> @@ -215,9 +217,19 @@ static uint32_t aspeed_scu_get_random(void)
> uint32_t aspeed_scu_get_apb_freq(AspeedSCUState *s)
> {
> AspeedSCUClass *asc = ASPEED_SCU_GET_CLASS(s);
> - uint32_t hpll = asc->calc_hpll(s, s->regs[HPLL_PARAM]);
> + uint32_t hpll, hpll_reg, clk_sel_reg;
> +
> + if (asc->calc_hpll == aspeed_2600_scu_calc_hpll) {
That's indeed one way to distinguish the AST2600 from the previous SoCs.
I would prefer to introduce a new APB freq class handler to deal with
the differences in the AST2600. aspeed_scu_get_apb_freq() would become :
uint32_t aspeed_scu_get_apb_freq(AspeedSCUState *s)
{
return ASPEED_SCU_GET_CLASS(s)->get_apb(s);
}
The current aspeed_scu_get_apb_freq() would become the AST2400 and AST2500
handler and you would have to introduce a new one for the AST2600.
> + hpll_reg = s->regs[AST2600_HPLL_PARAM];
> + clk_sel_reg = s->regs[AST2600_CLK_SEL];
> + } else {
> + hpll_reg = s->regs[HPLL_PARAM];
> + clk_sel_reg = s->regs[CLK_SEL];
> + }
> +
> + hpll = asc->calc_hpll(s, hpll_reg);
>
> - return hpll / (SCU_CLK_GET_PCLK_DIV(s->regs[CLK_SEL]) + 1)
> + return hpll / (SCU_CLK_GET_PCLK_DIV(clk_sel_reg) + 1)
> / asc->apb_divider;> }
>
> @@ -357,7 +369,10 @@ static const MemoryRegionOps aspeed_ast2500_scu_ops = {
>
> static uint32_t aspeed_scu_get_clkin(AspeedSCUState *s)
> {
> - if (s->hw_strap1 & SCU_HW_STRAP_CLK_25M_IN) {
> + AspeedSCUClass *asc = ASPEED_SCU_GET_CLASS(s);
> +
> + if (s->hw_strap1 & SCU_HW_STRAP_CLK_25M_IN ||
> + asc->calc_hpll == aspeed_2600_scu_calc_hpll) {
Indeed, the AST2600 CLKIN is always 25Mhz. Instead of testing ->calc_hpll,
I would introduce a class attribute, something like 'bool is_25Mhz'.
This change should be in a second patch though.
Thanks,
C.
> return 25000000;
> } else if (s->hw_strap1 & SCU_HW_STRAP_CLK_48M_IN) {
> return 48000000;
> @@ -426,6 +441,26 @@ static uint32_t aspeed_2500_scu_calc_hpll(AspeedSCUState *s, uint32_t hpll_reg)
> return clkin * multiplier;
> }
>
> +static uint32_t aspeed_2600_scu_calc_hpll(AspeedSCUState *s, uint32_t hpll_reg)
> +{
> + uint32_t multiplier = 1;
> + uint32_t clkin = aspeed_scu_get_clkin(s);
> +
> + if (hpll_reg & SCU_AST2600_H_PLL_OFF) {
> + return 0;
> + }
> +
> + if (!(hpll_reg & SCU_H_PLL_BYPASS_EN)) {
> + uint32_t p = (hpll_reg >> 19) & 0xf;
> + uint32_t n = (hpll_reg >> 13) & 0x3f;
> + uint32_t m = hpll_reg & 0x1fff;
> +
> + multiplier = ((m + 1) / (n + 1)) / (p + 1);
> + }
> +
> + return clkin * multiplier;
> +}
> +
> static void aspeed_scu_reset(DeviceState *dev)
> {
> AspeedSCUState *s = ASPEED_SCU(dev);
> @@ -716,7 +751,7 @@ static void aspeed_2600_scu_class_init(ObjectClass *klass, void *data)
> dc->desc = "ASPEED 2600 System Control Unit";
> dc->reset = aspeed_ast2600_scu_reset;
> asc->resets = ast2600_a3_resets;
> - asc->calc_hpll = aspeed_2500_scu_calc_hpll; /* No change since AST2500 */
> + asc->calc_hpll = aspeed_2600_scu_calc_hpll;
> asc->apb_divider = 4;
> asc->nr_regs = ASPEED_AST2600_SCU_NR_REGS;
> asc->ops = &aspeed_ast2600_scu_ops;
> diff --git a/include/hw/misc/aspeed_scu.h b/include/hw/misc/aspeed_scu.h
> index c14aff2bcb..91c500c5bc 100644
> --- a/include/hw/misc/aspeed_scu.h
> +++ b/include/hw/misc/aspeed_scu.h
> @@ -316,4 +316,21 @@ uint32_t aspeed_scu_get_apb_freq(AspeedSCUState *s);
> SCU_HW_STRAP_VGA_SIZE_SET(VGA_16M_DRAM) | \
> SCU_AST2500_HW_STRAP_RESERVED1)
>
> +/* SCU200 H-PLL Parameter Register (for Aspeed AST2600 SOC)
> + *
> + * 28:26 H-PLL Parameters
> + * 25 Enable H-PLL reset
> + * 24 Enable H-PLL bypass mode
> + * 23 Turn off H-PLL
> + * 22:19 H-PLL Post Divider (P)
> + * 18:13 H-PLL Numerator (M)
> + * 12:0 H-PLL Denumerator (N)
> + *
> + * (Output frequency) = CLKIN(25MHz) * [(M+1) / (N+1)] / (P+1)
> + *
> + * The default frequency is 1200Mhz when CLKIN = 25MHz
> + */
> +#define SCU_AST2600_H_PLL_BYPASS_EN (0x1 << 24)
> +#define SCU_AST2600_H_PLL_OFF (0x1 << 23)
> +
> #endif /* ASPEED_SCU_H */
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/1] hw: aspeed_scu: Add AST2600 hpll calculation function
2022-03-14 12:21 ` Cédric Le Goater
@ 2022-03-15 2:42 ` Steven Lee
2022-03-15 6:29 ` Cédric Le Goater
0 siblings, 1 reply; 5+ messages in thread
From: Steven Lee @ 2022-03-15 2:42 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Peter Maydell, Jamin Lin, Andrew Jeffery, Troy Lee,
open list:All patches CC here, open list:ASPEED BMCs,
Joel Stanley
The 03/14/2022 20:21, Cédric Le Goater wrote:
> Hello Steven,
>
> On 3/14/22 10:54, Steven Lee wrote:
> > AST2600's HPLL register offset and bit definition are different from
> > AST2500. Add a hpll calculation function for ast2600 and modify apb frequency
> > calculation function based on SCU200 register description in ast2600v11.pdf.
>
> It looks good. A few minor comments on the modeling.
>
> > Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
> > ---
> > hw/misc/aspeed_scu.c | 43 ++++++++++++++++++++++++++++++++----
> > include/hw/misc/aspeed_scu.h | 17 ++++++++++++++
> > 2 files changed, 56 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> > index d06e179a6e..3b11e98d66 100644
> > --- a/hw/misc/aspeed_scu.c
> > +++ b/hw/misc/aspeed_scu.c
> > @@ -205,6 +205,8 @@ static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
> > [BMC_DEV_ID] = 0x00002402U
> > };
> >
> > +static uint32_t aspeed_2600_scu_calc_hpll(AspeedSCUState *s, uint32_t hpll_reg);
> > +
> > static uint32_t aspeed_scu_get_random(void)
> > {
> > uint32_t num;
> > @@ -215,9 +217,19 @@ static uint32_t aspeed_scu_get_random(void)
> > uint32_t aspeed_scu_get_apb_freq(AspeedSCUState *s)
> > {
> > AspeedSCUClass *asc = ASPEED_SCU_GET_CLASS(s);
> > - uint32_t hpll = asc->calc_hpll(s, s->regs[HPLL_PARAM]);
> > + uint32_t hpll, hpll_reg, clk_sel_reg;
> > +
> > + if (asc->calc_hpll == aspeed_2600_scu_calc_hpll) {
>
> That's indeed one way to distinguish the AST2600 from the previous SoCs.
> I would prefer to introduce a new APB freq class handler to deal with
> the differences in the AST2600. aspeed_scu_get_apb_freq() would become :
>
> uint32_t aspeed_scu_get_apb_freq(AspeedSCUState *s)
> {
> return ASPEED_SCU_GET_CLASS(s)->get_apb(s);
> }
>
> The current aspeed_scu_get_apb_freq() would become the AST2400 and AST2500
> handler and you would have to introduce a new one for the AST2600.
>
Hi Cédric,
Thanks for the review.
I was wondering if the following implementation is good to you.
1 Modify aspeed_scu_get_apb_freq() as below
uint32_t aspeed_scu_get_apb_freq(AspeedSCUState *s)
{
return ASPEED_SCU_GET_CLASS(s)->get_apb(s);
}
2. Introduce 2 APB class handlers: aspeed_2400_scu_get_apb_freq() and aspeed_2600_scu_get_apb_freq()
3. Add new attribute get_apb in AspeedSCUClass.
4. In aspeed_2400_scu_class_init() and aspeed_2500_scu_class_init()
asc->get_apb = aspeed_2400_scu_get_apb_freq;
In aspeed_2600_scu_class_init()
asc->get_apb = aspeed_2600_scu_get_apb_freq;
> > + hpll_reg = s->regs[AST2600_HPLL_PARAM];
> > + clk_sel_reg = s->regs[AST2600_CLK_SEL];
> > + } else {
> > + hpll_reg = s->regs[HPLL_PARAM];
> > + clk_sel_reg = s->regs[CLK_SEL];
> > + }
> > +
> > + hpll = asc->calc_hpll(s, hpll_reg);
> >
> > - return hpll / (SCU_CLK_GET_PCLK_DIV(s->regs[CLK_SEL]) + 1)
> > + return hpll / (SCU_CLK_GET_PCLK_DIV(clk_sel_reg) + 1)
> > / asc->apb_divider;> }
> >
> > @@ -357,7 +369,10 @@ static const MemoryRegionOps aspeed_ast2500_scu_ops = {
> >
> > static uint32_t aspeed_scu_get_clkin(AspeedSCUState *s)
> > {
> > - if (s->hw_strap1 & SCU_HW_STRAP_CLK_25M_IN) {
> > + AspeedSCUClass *asc = ASPEED_SCU_GET_CLASS(s);
> > +
> > + if (s->hw_strap1 & SCU_HW_STRAP_CLK_25M_IN ||
> > + asc->calc_hpll == aspeed_2600_scu_calc_hpll) {
>
> Indeed, the AST2600 CLKIN is always 25Mhz. Instead of testing ->calc_hpll,
> I would introduce a class attribute, something like 'bool is_25Mhz'.
>
> This change should be in a second patch though.
>
will add a new attribute for clkin in the second patch.
Thanks,
Steven
> Thanks,
>
> C.
>
> > return 25000000;
> > } else if (s->hw_strap1 & SCU_HW_STRAP_CLK_48M_IN) {
> > return 48000000;
> > @@ -426,6 +441,26 @@ static uint32_t aspeed_2500_scu_calc_hpll(AspeedSCUState *s, uint32_t hpll_reg)
> > return clkin * multiplier;
> > }
> >
> > +static uint32_t aspeed_2600_scu_calc_hpll(AspeedSCUState *s, uint32_t hpll_reg)
> > +{
> > + uint32_t multiplier = 1;
> > + uint32_t clkin = aspeed_scu_get_clkin(s);
> > +
> > + if (hpll_reg & SCU_AST2600_H_PLL_OFF) {
> > + return 0;
> > + }
> > +
> > + if (!(hpll_reg & SCU_H_PLL_BYPASS_EN)) {
> > + uint32_t p = (hpll_reg >> 19) & 0xf;
> > + uint32_t n = (hpll_reg >> 13) & 0x3f;
> > + uint32_t m = hpll_reg & 0x1fff;
> > +
> > + multiplier = ((m + 1) / (n + 1)) / (p + 1);
> > + }
> > +
> > + return clkin * multiplier;
> > +}
> > +
> > static void aspeed_scu_reset(DeviceState *dev)
> > {
> > AspeedSCUState *s = ASPEED_SCU(dev);
> > @@ -716,7 +751,7 @@ static void aspeed_2600_scu_class_init(ObjectClass *klass, void *data)
> > dc->desc = "ASPEED 2600 System Control Unit";
> > dc->reset = aspeed_ast2600_scu_reset;
> > asc->resets = ast2600_a3_resets;
> > - asc->calc_hpll = aspeed_2500_scu_calc_hpll; /* No change since AST2500 */
> > + asc->calc_hpll = aspeed_2600_scu_calc_hpll;
> > asc->apb_divider = 4;
> > asc->nr_regs = ASPEED_AST2600_SCU_NR_REGS;
> > asc->ops = &aspeed_ast2600_scu_ops;
> > diff --git a/include/hw/misc/aspeed_scu.h b/include/hw/misc/aspeed_scu.h
> > index c14aff2bcb..91c500c5bc 100644
> > --- a/include/hw/misc/aspeed_scu.h
> > +++ b/include/hw/misc/aspeed_scu.h
> > @@ -316,4 +316,21 @@ uint32_t aspeed_scu_get_apb_freq(AspeedSCUState *s);
> > SCU_HW_STRAP_VGA_SIZE_SET(VGA_16M_DRAM) | \
> > SCU_AST2500_HW_STRAP_RESERVED1)
> >
> > +/* SCU200 H-PLL Parameter Register (for Aspeed AST2600 SOC)
> > + *
> > + * 28:26 H-PLL Parameters
> > + * 25 Enable H-PLL reset
> > + * 24 Enable H-PLL bypass mode
> > + * 23 Turn off H-PLL
> > + * 22:19 H-PLL Post Divider (P)
> > + * 18:13 H-PLL Numerator (M)
> > + * 12:0 H-PLL Denumerator (N)
> > + *
> > + * (Output frequency) = CLKIN(25MHz) * [(M+1) / (N+1)] / (P+1)
> > + *
> > + * The default frequency is 1200Mhz when CLKIN = 25MHz
> > + */
> > +#define SCU_AST2600_H_PLL_BYPASS_EN (0x1 << 24)
> > +#define SCU_AST2600_H_PLL_OFF (0x1 << 23)
> > +
> > #endif /* ASPEED_SCU_H */
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/1] hw: aspeed_scu: Add AST2600 hpll calculation function
2022-03-15 2:42 ` Steven Lee
@ 2022-03-15 6:29 ` Cédric Le Goater
0 siblings, 0 replies; 5+ messages in thread
From: Cédric Le Goater @ 2022-03-15 6:29 UTC (permalink / raw)
To: Steven Lee
Cc: Peter Maydell, Jamin Lin, Andrew Jeffery, Troy Lee,
open list:All patches CC here, open list:ASPEED BMCs,
Joel Stanley
Hello Steven,
[ ... ]
> I was wondering if the following implementation is good to you.
>
> 1 Modify aspeed_scu_get_apb_freq() as below
> uint32_t aspeed_scu_get_apb_freq(AspeedSCUState *s)
> {
> return ASPEED_SCU_GET_CLASS(s)->get_apb(s);
> }
>
> 2. Introduce 2 APB class handlers: aspeed_2400_scu_get_apb_freq() and aspeed_2600_scu_get_apb_freq()
>
> 3. Add new attribute get_apb in AspeedSCUClass.
>
> 4. In aspeed_2400_scu_class_init() and aspeed_2500_scu_class_init()
> asc->get_apb = aspeed_2400_scu_get_apb_freq;
>
> In aspeed_2600_scu_class_init()
> asc->get_apb = aspeed_2600_scu_get_apb_freq;
yes. that's the idea.
[ ... ]
>>> static uint32_t aspeed_scu_get_clkin(AspeedSCUState *s)
>>> {
>>> - if (s->hw_strap1 & SCU_HW_STRAP_CLK_25M_IN) {
>>> + AspeedSCUClass *asc = ASPEED_SCU_GET_CLASS(s);
>>> +
>>> + if (s->hw_strap1 & SCU_HW_STRAP_CLK_25M_IN ||
>>> + asc->calc_hpll == aspeed_2600_scu_calc_hpll) {
>>
>> Indeed, the AST2600 CLKIN is always 25Mhz. Instead of testing ->calc_hpll,
>> I would introduce a class attribute, something like 'bool is_25Mhz'.
>>
>> This change should be in a second patch though.
>>
>
> will add a new attribute for clkin in the second patch.
yes. 'clkin_25Mhz' may be.
Thanks,
C.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-03-15 6:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-03-14 9:54 [PATCH v1 0/1] hw: aspeed_scu: Add AST2600 hpll calculation function Steven Lee
2022-03-14 9:54 ` [PATCH v1 1/1] " Steven Lee
2022-03-14 12:21 ` Cédric Le Goater
2022-03-15 2:42 ` Steven Lee
2022-03-15 6:29 ` Cédric Le Goater
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).