On Wed, May 09, 2012 at 04:20:06, Hilman, Kevin wrote:
> Vaibhav Hiremath <hvaibhav@xxxxxx> writes:
>
> > This patch cleans up dpll_data structure, making structure
> > fields definition based on feature availability for given SoC,
> > managed using Kconfig rules.
> >
> > SOC_HAS_OMAP3_DPLL_IDLE: idle state
> > SOC_HAS_OMAP3_DPLL_RECAL: recalibration capability
> > SOC_HAS_OMAP3_DPLL_DCO_SEL: dco selection
> > SOC_HAS_OMAP3_DPLL_SDDIV: sigma-delta div factor
> > SOC_HAS_OMAP3_DPLL_FREQSEL: frequency selection
> >
> > Signed-off-by: Vaibhav Hiremath <hvaibhav@xxxxxx>
> > Cc: Tony Lindgren <tony@xxxxxxxxxxx>
> > Cc: Kevin Hilman <khilman@xxxxxx>
> > Cc: Paul Walmsley <paul@xxxxxxxxx>
>
> Paul is the one to make the call here, but personally I'd much prefer to
> just see the existing ifdefs go away[1] instead of adding a bunch of new
> ones.
>
> Yes, doing so would add a few fields to a struct that may be unused, but
> IMO, the improvement in readabilitly and maintainability is improved.
>
> Note that the users of these fields are not changed and are still based
> on Kconfig/Makefile values as before (e.g. CONFIG_ARCH_OMAP3), so to me
> this creates another layer of obfuscation.
>
This will bring in difference on omap2 alone build OR omap4, am33xx alone
builds.
But again, how much it makes sense, and how much benefits we bring-in by
adding config options for every bit-fields (like this) needs to be looked at.
And that's the reason, I submitted first version as a RFC.
>
> [1]
> diff --git a/arch/arm/plat-omap/include/plat/clock.h b/arch/arm/plat-omap/include/plat/clock.h
> index d0ef57c..656b986 100644
> --- a/arch/arm/plat-omap/include/plat/clock.h
> +++ b/arch/arm/plat-omap/include/plat/clock.h
> @@ -156,7 +156,6 @@ struct dpll_data {
> u8 min_divider;
> u16 max_divider;
> u8 modes;
> -#if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_ARCH_OMAP4)
> void __iomem *autoidle_reg;
> void __iomem *idlest_reg;
> u32 autoidle_mask;
> @@ -167,7 +166,6 @@ struct dpll_data {
> u8 auto_recal_bit;
> u8 recal_en_bit;
> u8 recal_st_bit;
> -# endif
> u8 flags;
> };
>
Honestly, I wanted to propose this first.
I am OK, as long as we all agree on this.
Paul, can you conform on this?
Thanks,
Vaibhav
--
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
