Re: [coreboot] [commit] r5976 - trunk/src/cpu/amd/car

[Scott Duplichan]

-----Original Message-----
From: [e-mail] [mailto:[e-mail]] On Behalf Of Warren Turkal
Sent: Wednesday, October 20, 2010 03:20 AM
To: [e-mail]
Subject: Re: [coreboot] [commit] r5976 - trunk/src/cpu/amd/car

]On Tue, Oct 19, 2010 at 2:08 PM, repository service  wrote:
]> +   #if (CONFIG_MMCONF_BASE_ADDRESS > 0xFFFFFFFF)
]> +   #error "MMCONF_BASE_ADDRESS too big"
]> +   #elif (CONFIG_MMCONF_BASE_ADDRESS & 0xFFFFF)
]> +   #error "MMCONF_BASE_ADDRESS not 1MB aligned"
]> +   #endif
]> +       movl    $0, %edx
]> +       movl    $((CONFIG_MMCONF_BASE_ADDRESS) | (1 << 0)), %eax
]> +   #if (CONFIG_MMCONF_BUS_NUMBER == 1)
]
]If you insist on doing it this way, it might be better to change the
]above 2 lines to:
]  movl $CONFIG_MMCONF_BASE_ADDRESS
]#if (CONFIG_MMCONF_BUS_NUMBER == 1)
]  orl $(1 << 0), %eax

Hello Warren, thanks for the suggestions. Bit zero is the enable bit,
so it must be set unconditionally. That is why I included it in the
movl. For the next question, I believe lack of readability is a 
factor. That is why so much effort went into attempts to improve it.
Bit positions 2-5 must be set to the base 2 logarithm of
CONFIG_MMCONF_BUS_NUMBER. I could not find a straightforward and
readable method of finding the base2 log at either assembly time or
at run time. What would have been nice is:
   movl $(log2(CONFIG_MMCONF_BUS_NUMBER) << 2), %eax
But gas has no base 2 log function and could not figure out how to
write a macro that returns a constant.

Thanks,
Scott

]
]> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 2)
]> +       orl     $(1 << 2), %eax
]
]However, why not just:
]  movl $CONFIG_MMCONF_BASE_ADDRESS
]  orl $CONFIG_MMCONF_BUS_NUMBER, %eax"
]and no CPP conditionals since all those shifted numbers appear to be
]the binary representations of the numbers you are testing for?
]
]> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 4)
]> +       orl     $(2 << 2), %eax
]> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 8)
]> +       orl     $(3 << 2), %eax
]> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 16)
]> +       orl     $(4 << 2), %eax
]> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 32)
]> +       orl     $(5 << 2), %eax
]> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 64)
]> +       orl     $(6 << 2), %eax
]> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 128)
]> +       orl     $(7 << 2), %eax
]> +   #elif (CONFIG_MMCONF_BUS_NUMBER == 256)
]> +       orl     $(8 << 2), %eax
]> +   #else
]> +       #error "bad MMCONF_BUS_NUMBER value"
]> +   #endif
]> +       movl    $(0xc0010058), %ecx
]
]Thanks,
]wt