> -----Original Message----- > From: Tony Lindgren [mailto:tony@xxxxxxxxxxx] > Sent: Thursday, December 08, 2011 5:11 AM > To: Hiremath, Vaibhav > Cc: linux-omap@xxxxxxxxxxxxxxx > Subject: Re: [RFC PATCH] arm:omap: cleanup & split > omap2/3/4_check_revision function > > * Hiremath, Vaibhav <hvaibhav@xxxxxx> [111122 21:56]: > > > > > > This patch doesn't change functionality or behavior of the code > > > execution; it barely cleans up the code and splits into SoC > > > specific implementation for ID and feature detection; makes > > > easier to add new SoC (especially for AM devices where we do not have > > > feature register). > ... > > > If there are not review comments, can this be merged? > > Sorry for the delay, the idea is good now that we don't need the > revision check super early any longer. Few comments below. > Thanks for the comment, response in-lined below - > > > --- a/arch/arm/mach-omap2/id.c > > > +++ b/arch/arm/mach-omap2/id.c > > > @@ -226,9 +226,9 @@ static void __init omap4_check_features(void) > > > } > > > } > > > > > > -static void __init ti816x_check_features(void) > > > +static void __init omap3_add_def_features(void) > > > { > > > - omap_features = OMAP3_HAS_NEON; > > > + omap_features = OMAP3_HAS_NEON | OMAP3_HAS_L2CACHE; > > > } > > > > > > static void __init omap3_check_revision(const char **cpu_rev) > > Can you please split this patch a bit? First a patch that does not > change the behaviour except adds the SoC specific revision checks. > > > > @@ -393,6 +394,7 @@ void __init omap2430_init_early(void) > > > void __init omap3_init_early(void) > > > { > > > omap2_set_globals_3xxx(); > > > + omap3xxx_check_revision(true); > > > omap_common_init_early(); > > > omap3xxx_voltagedomains_init(); > > > omap3xxx_powerdomains_init(); > > > @@ -425,6 +427,7 @@ void __init am35xx_init_early(void) > > > void __init ti816x_init_early(void) > > > { > > > omap2_set_globals_ti816x(); > > > + omap3xxx_check_revision(false); > > > omap_common_init_early(); > > > omap3xxx_voltagedomains_init(); > > > omap3xxx_powerdomains_init(); > > Maybe just call ti816x_check_features separately? > > omap3xxx_check_revision(); > omap3xxx_check_features(); > and > omap3xxx_check_revision(); > ti816x_check_features(); > The reason why I did not do this is, In case of omap3_check_revision, Void __init omap3xxx_check_revision() { const char *cpu_rev; omap3_check_revision(&cpu_rev); omap3_check_features(); omap3_cpuinfo(cpu_rev); } If I separate, omap3_check_revision() and omap3_check_features(), then I have to maintain cpu_rev in the parent function. Also, I can not break the sequence due to this dependency on cpu_rev. and I wanted to retain the void function calls in omap3_init_early(). So I came up with this optimized and clean way of using "has_feature_reg" Flag, and set to default features for AM family of devices. OR As you suggested, I have to maintain cpu_rev into function omap3_init_early() and call the above 3 function in the sequence (alone for omap3). I am ok with any approach... Thanks, Vaibhav > Regards, > > Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
