Re: [PATCH-V3 2/4] arm:omap:am33xx: Update common OMAP machine specific sources

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



"Premi, Sanjeev" <premi@xxxxxx> writes:

>> -----Original Message-----
>> From: linux-omap-owner@xxxxxxxxxxxxxxx 
>> [mailto:linux-omap-owner@xxxxxxxxxxxxxxx] On Behalf Of Hilman, Kevin
>> Sent: Tuesday, September 27, 2011 12:16 AM
>> To: Hiremath, Vaibhav
>> Cc: linux-omap@xxxxxxxxxxxxxxx; paul@xxxxxxxxx; 
>> tony@xxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; 
>> Mohammed, Afzal
>> Subject: Re: [PATCH-V3 2/4] arm:omap:am33xx: Update common 
>> OMAP machine specific sources
>> 
>> <hvaibhav@xxxxxx> writes:
>> 
>> > From: Afzal Mohammed <afzal@xxxxxx>
>> >
>> > 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 <afzal@xxxxxx>
>> > Signed-off-by: Vaibhav Hiremath <hvaibhav@xxxxxx>
>> 
>> [...]
>> 
>> > @@ -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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux