On 05/15/2013 04:27 AM, Joseph Lo wrote: > There are some Tegra SoC ID checking code around the low level assembly > code. Adding a marco to replace them. For the single image to support all > the Tegra series, we may also need the marco in other common code. So we > make it become a marco for the usage.
I'm not sure this patch doesn't obfuscate the code too much. The big issue I see is: > @@ -115,13 +112,9 @@ ENTRY(__tegra_cpu_reset_handler) > > cpsid aif, 0x13 @ SVC mode, interrupts disabled > > - mov32 r6, TEGRA_APB_MISC_BASE > - ldr r6, [r6, #APB_MISC_GP_HIDREV] > - and r6, r6, #0xff00 > -#ifdef CONFIG_ARCH_TEGRA_2x_SOC > -t20_check: > - cmp r6, #(0x20 << 8) > + tegra_check_soc_id TEGRA20, TEGRA_APB_MISC_BASE, r6, r5 Here, we replace all the code with the new macro, ... > #ifdef CONFIG_ARCH_TEGRA_2x_SOC > /* Are we on Tegra20? */ > - cmp r6, #(0x20 << 8) > + cmp r6, #(TEGRA20 << 8) But here we still have to write out the cmp instructions. It may be reasonable to create a macro to read/mask/shift the SoC ID, similar to the existing cpu_id macro, i.e. roughly: /* Macro to check Tegra revision */ +#define APB_MISC_GP_HIDREV 0x804 +.macro tegra_soc_id base, tmp1, tmp2 + mov32 \tmp2, \base + ldr \tmp1, [\tmp2, #APB_MISC_GP_HIDREV] + and \tmp1, \tmp1, #0xff00 +.endm Also, I wonder: (a) why the need to pass TEGRA_APB_MISC_BASE into the macro; can't the macro just hard-code that value since it's always the same? (b) Why need two registers passed to the macro. At least one of the code segments you've replaced with the macro uses a single register instead: - mov32 r6, TEGRA_APB_MISC_BASE - ldr r6, [r6, #APB_MISC_GP_HIDREV] - and r6, r6, #0xff00 Wouldn't that be a better implementation of the macro? I don't think relying on \tmp2 containing "base" after the macro invocation would be a good idea, since that's rather like looking inside the black box. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
