Re: [PATCH v1 1/1] hw: aspeed_scu: Add AST2600 hpll calculation function

[Cédric Le Goater]

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.