"Premi, Sanjeev" <[email protected]> writes:

>> -----Original Message-----
>> From: [email protected] 
>> [mailto:[email protected]] On Behalf Of Hilman, Kevin
>> Sent: Tuesday, September 27, 2011 12:16 AM
>> To: Hiremath, Vaibhav
>> Cc: [email protected]; [email protected]; 
>> [email protected]; [email protected]; 
>> Mohammed, Afzal
>> Subject: Re: [PATCH-V3 2/4] arm:omap:am33xx: Update common 
>> OMAP machine specific sources
>> 
>> <[email protected]> writes:
>> 
>> > From: Afzal Mohammed <[email protected]>
>> >
>> > This patch updates the common machine specific source files for
>> > support for AM33XX/AM335x with cpu type, macros for 
>> identification of
>> > AM33XX/AM335X device.
>> >
>> > Signed-off-by: Afzal Mohammed <[email protected]>
>> > Signed-off-by: Vaibhav Hiremath <[email protected]>
>> 
>> [...]
>> 
>> > @@ -3576,7 +3579,8 @@ int __init omap3xxx_clk_init(void)
>> >     * Lock DPLL5 -- here only until other device init code can
>> >     * handle this
>> >     */
>> > -  if (!cpu_is_ti816x() && (omap_rev() >= OMAP3430_REV_ES2_0))
>> > +  if (!cpu_is_ti816x() && !cpu_is_am33xx() &&
>> > +                  (omap_rev() >= OMAP3430_REV_ES2_0))
>> >            omap3_clk_lock_dpll5();
>> 
>> This is getting ugly.  
>> 
>> Instead of continuing to expand this if-list, I think it's time for a
>> new feature-flag for whether or not an SoC has DPLL5 instead.
>
> I agree that the code is really getting ugly here. But, isn't
> feature-flag going to be over-used with this and similar features?
>
> Just thinking ahead, for these possible cases:
> 1) An soc adds DPLL6.
> 2) An soc uses DPLL5, but mechanism to lock is different.

You're right.

> Wouldn't it be better to have a scheme like this:
> 1) Define a simple structure for DPLLs.
> 2) Initialize the unused DPLLs to be null/ -1 early
>    in arch/soc specific init.
> 3) The DPLL functions check for corresponding flag on
>    entry.

Actually, looking at this closer, I think the infrastructure is already
there to handle this cleanly.

Basically, dpll5 should not even be registered for SoCs where it doesn't
exist.  Then, any attempts to use DPLL5 would know it doesn't exist
because the call to clk_get() in omap3_clk_lock_dpll5() would fail.

I think the clock3xxx_data.c needs a bit more cleanup so that only
clocks that exist for a given SoC are registered.

Paul already did a similar cleanup for the powerdomain data files by
creating separate lists for common ones and unique ones.  Looks like we
need the same for the clock data.

Patches welcome.

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to