[v2,04/14] ARM: OMAP5: Add minimal support for OMAP5430 SOC

Commit Message

Tony Lindgren July 9, 2012, 1:11 p.m. UTC

* Vaibhav Hiremath <[e-mail]> [120709 01:55]:
> On 7/6/2012 2:51 PM, Santosh Shilimkar wrote:
> > --- a/arch/arm/plat-omap/include/plat/clkdev_omap.h
> > +++ b/arch/arm/plat-omap/include/plat/clkdev_omap.h
> > @@ -39,6 +39,7 @@ struct omap_clk {
> >  #define CK_443X		(1 << 11)
> >  #define CK_TI816X	(1 << 12)
> >  #define CK_446X		(1 << 13)
> > +#define CK_54XX		(1 << 14)
> 
> This is conflicting with AM33XX, you may want to rebase it again, since
> AM33xx clock tree is already pushed and available in
> linux-omap/devel-am33xx-part2.

Heh these CK_XXXX defines are now running out of the u16 cpu_mask.

They really should be replaced with SoC specific lists of clocks
rather than bloating the cpu_mask and repeating it for every clock
that's compiled in for 800+ times.

Below (untested) is what could be done in the short term.

I wonder if we could #define CK_OMAP_DUMMY 0 that's always set
for non-shared clocks if they only get set in some *_data.c
file in a unique way?

Paul got any better ideas?

Regards,

Tony

Comments

Tony Lindgren July 9, 2012, 1:25 p.m. UTC | #1

* Tony Lindgren <[e-mail]> [120709 06:17]:
> * Vaibhav Hiremath <[e-mail]> [120709 01:55]:
> > On 7/6/2012 2:51 PM, Santosh Shilimkar wrote:
> > > --- a/arch/arm/plat-omap/include/plat/clkdev_omap.h
> > > +++ b/arch/arm/plat-omap/include/plat/clkdev_omap.h
> > > @@ -39,6 +39,7 @@ struct omap_clk {
> > >  #define CK_443X		(1 << 11)
> > >  #define CK_TI816X	(1 << 12)
> > >  #define CK_446X		(1 << 13)
> > > +#define CK_54XX		(1 << 14)
> > 
> > This is conflicting with AM33XX, you may want to rebase it again, since
> > AM33xx clock tree is already pushed and available in
> > linux-omap/devel-am33xx-part2.
> 
> Heh these CK_XXXX defines are now running out of the u16 cpu_mask.
> 
> They really should be replaced with SoC specific lists of clocks
> rather than bloating the cpu_mask and repeating it for every clock
> that's compiled in for 800+ times.
> 
> Below (untested) is what could be done in the short term.
> 
> I wonder if we could #define CK_OMAP_DUMMY 0 that's always set
> for non-shared clocks if they only get set in some *_data.c
> file in a unique way?
> 
> Paul got any better ideas?

Santosh, I suggest you just drop the CK_54XX change from your patches
as the clock fwk support will need further patching and is not used
yet.
 
> Regards,
> 
> Tony
> 
> 
> --- a/arch/arm/plat-omap/include/plat/clkdev_omap.h
> +++ b/arch/arm/plat-omap/include/plat/clkdev_omap.h
> @@ -26,26 +26,29 @@ struct omap_clk {
>  	}
>  
>  /* Platform flags for the clkdev-OMAP integration code */
> +
> +#ifdef CONFIG_ARCH_OMAP1
>  #define CK_310		(1 << 0)
>  #define CK_7XX		(1 << 1)	/* 7xx, 850 */
>  #define CK_1510		(1 << 2)
>  #define CK_16XX		(1 << 3)	/* 16xx, 17xx, 5912 */
> -#define CK_242X		(1 << 4)
> -#define CK_243X		(1 << 5)	/* 243x, 253x */
> -#define CK_3430ES1	(1 << 6)	/* 34xxES1 only */
> -#define CK_3430ES2PLUS	(1 << 7)	/* 34xxES2, ES3, non-Sitara 35xx only */
> -#define CK_AM35XX	(1 << 9)	/* Sitara AM35xx */
> -#define CK_36XX		(1 << 10)	/* 36xx/37xx-specific clocks */
> -#define CK_443X		(1 << 11)
> -#define CK_TI816X	(1 << 12)
> -#define CK_446X		(1 << 13)
> -#define CK_AM33XX	(1 << 14)	/* AM33xx specific clocks */
> -#define CK_1710		(1 << 15)	/* 1710 extra for rate selection */
> -
> +#define CK_1710		(1 << 4)	/* 1710 extra for rate selection */
> +#endif
>  
> +#ifdef CONFIG_ARCH_OMAP2PLUS
> +#define CK_242X		(1 << 0)
> +#define CK_243X		(1 << 1)	/* 243x, 253x */
> +#define CK_3430ES1	(1 << 2)	/* 34xxES1 only */
> +#define CK_3430ES2PLUS	(1 << 3)	/* 34xxES2, ES3, non-Sitara 35xx only */
> +#define CK_AM35XX	(1 << 4)	/* Sitara AM35xx */
> +#define CK_36XX		(1 << 5)	/* 36xx/37xx-specific clocks */
> +#define CK_443X		(1 << 6)
> +#define CK_TI816X	(1 << 7)
> +#define CK_446X		(1 << 8)
> +#define CK_AM33XX	(1 << 9)	/* AM33xx specific clocks */
>  #define CK_34XX		(CK_3430ES1 | CK_3430ES2PLUS)
>  #define CK_3XXX		(CK_34XX | CK_AM35XX | CK_36XX)
> -
> +#endif
>  
>  #endif
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to [e-mail]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Santosh Shilimkar July 9, 2012, 1:26 p.m. UTC | #2

On Mon, Jul 9, 2012 at 6:55 PM, Tony Lindgren <[e-mail]> wrote:
> * Tony Lindgren <[e-mail]> [120709 06:17]:
>> * Vaibhav Hiremath <[e-mail]> [120709 01:55]:
>> > On 7/6/2012 2:51 PM, Santosh Shilimkar wrote:
>> > > --- a/arch/arm/plat-omap/include/plat/clkdev_omap.h
>> > > +++ b/arch/arm/plat-omap/include/plat/clkdev_omap.h
>> > > @@ -39,6 +39,7 @@ struct omap_clk {
>> > >  #define CK_443X          (1 << 11)
>> > >  #define CK_TI816X        (1 << 12)
>> > >  #define CK_446X          (1 << 13)
>> > > +#define CK_54XX          (1 << 14)
>> >
>> > This is conflicting with AM33XX, you may want to rebase it again, since
>> > AM33xx clock tree is already pushed and available in
>> > linux-omap/devel-am33xx-part2.
>>
>> Heh these CK_XXXX defines are now running out of the u16 cpu_mask.
>>
>> They really should be replaced with SoC specific lists of clocks
>> rather than bloating the cpu_mask and repeating it for every clock
>> that's compiled in for 800+ times.
>>
>> Below (untested) is what could be done in the short term.
>>
>> I wonder if we could #define CK_OMAP_DUMMY 0 that's always set
>> for non-shared clocks if they only get set in some *_data.c
>> file in a unique way?
>>
>> Paul got any better ideas?
>
> Santosh, I suggest you just drop the CK_54XX change from your patches
> as the clock fwk support will need further patching and is not used
> yet.
>
Good idea. Will have a look at it.

Regards
Santosh

Vaibhav Hiremath July 10, 2012, 6:25 a.m. UTC | #3

On Mon, Jul 09, 2012 at 18:41:58, Tony Lindgren wrote:
> * Vaibhav Hiremath <[e-mail]> [120709 01:55]:
> > On 7/6/2012 2:51 PM, Santosh Shilimkar wrote:
> > > --- a/arch/arm/plat-omap/include/plat/clkdev_omap.h
> > > +++ b/arch/arm/plat-omap/include/plat/clkdev_omap.h
> > > @@ -39,6 +39,7 @@ struct omap_clk {
> > >  #define CK_443X		(1 << 11)
> > >  #define CK_TI816X	(1 << 12)
> > >  #define CK_446X		(1 << 13)
> > > +#define CK_54XX		(1 << 14)
> > 
> > This is conflicting with AM33XX, you may want to rebase it again, since
> > AM33xx clock tree is already pushed and available in
> > linux-omap/devel-am33xx-part2.
> 
> Heh these CK_XXXX defines are now running out of the u16 cpu_mask.
> 
> They really should be replaced with SoC specific lists of clocks
> rather than bloating the cpu_mask and repeating it for every clock
> that's compiled in for 800+ times.
> 
> Below (untested) is what could be done in the short term.
> 
> I wonder if we could #define CK_OMAP_DUMMY 0 that's always set
> for non-shared clocks if they only get set in some *_data.c
> file in a unique way?
> 
> Paul got any better ideas?
> 
> Regards,
> 
> Tony
> 
> 
> --- a/arch/arm/plat-omap/include/plat/clkdev_omap.h
> +++ b/arch/arm/plat-omap/include/plat/clkdev_omap.h
> @@ -26,26 +26,29 @@ struct omap_clk {
>  	}
>  
>  /* Platform flags for the clkdev-OMAP integration code */
> +
> +#ifdef CONFIG_ARCH_OMAP1
>  #define CK_310		(1 << 0)
>  #define CK_7XX		(1 << 1)	/* 7xx, 850 */
>  #define CK_1510		(1 << 2)
>  #define CK_16XX		(1 << 3)	/* 16xx, 17xx, 5912 */
> -#define CK_242X		(1 << 4)
> -#define CK_243X		(1 << 5)	/* 243x, 253x */
> -#define CK_3430ES1	(1 << 6)	/* 34xxES1 only */
> -#define CK_3430ES2PLUS	(1 << 7)	/* 34xxES2, ES3, non-Sitara 35xx only */
> -#define CK_AM35XX	(1 << 9)	/* Sitara AM35xx */
> -#define CK_36XX		(1 << 10)	/* 36xx/37xx-specific clocks */
> -#define CK_443X		(1 << 11)
> -#define CK_TI816X	(1 << 12)
> -#define CK_446X		(1 << 13)
> -#define CK_AM33XX	(1 << 14)	/* AM33xx specific clocks */
> -#define CK_1710		(1 << 15)	/* 1710 extra for rate selection */
> -
> +#define CK_1710		(1 << 4)	/* 1710 extra for rate selection */
> +#endif
>  
> +#ifdef CONFIG_ARCH_OMAP2PLUS
> +#define CK_242X		(1 << 0)
> +#define CK_243X		(1 << 1)	/* 243x, 253x */
> +#define CK_3430ES1	(1 << 2)	/* 34xxES1 only */
> +#define CK_3430ES2PLUS	(1 << 3)	/* 34xxES2, ES3, non-Sitara 35xx only */
> +#define CK_AM35XX	(1 << 4)	/* Sitara AM35xx */
> +#define CK_36XX		(1 << 5)	/* 36xx/37xx-specific clocks */
> +#define CK_443X		(1 << 6)
> +#define CK_TI816X	(1 << 7)
> +#define CK_446X		(1 << 8)
> +#define CK_AM33XX	(1 << 9)	/* AM33xx specific clocks */
>  #define CK_34XX		(CK_3430ES1 | CK_3430ES2PLUS)
>  #define CK_3XXX		(CK_34XX | CK_AM35XX | CK_36XX)
> -
> +#endif
>  
>  #endif

This also will not scale up in the future and will end up again in the same 
situation.

Just a quick thought, may work here,

I looked at the usage of cpu_mask and rates.flag and I believe we can 
restrict both to given SoC, something like,

OMAP34XX ->
          ES1
          ES2PLUS
          36XX
          AM35XX
          ...

OMAP4 ->
          443X
          446X

AM33XX ->
          AM335X
          TI816X
          TI814X
          ...

XYZ...  ->
          ...


The proposal would be,

To make cpu_mask and rate.flags 32 bit wide and divide it in 16-16 bits -

Lower 16 bits => describe SoC it is applicable to
Upper 16 bit => describes silicon versions or families

Thanks,
Vaibhav

Tony Lindgren July 10, 2012, 8:18 a.m. UTC | #4

* Hiremath, Vaibhav <[e-mail]> [120709 23:30]:
> On Mon, Jul 09, 2012 at 18:41:58, Tony Lindgren wrote:
> > * Vaibhav Hiremath <[e-mail]> [120709 01:55]:
> > > On 7/6/2012 2:51 PM, Santosh Shilimkar wrote:
> > > > --- a/arch/arm/plat-omap/include/plat/clkdev_omap.h
> > > > +++ b/arch/arm/plat-omap/include/plat/clkdev_omap.h
> > > > @@ -39,6 +39,7 @@ struct omap_clk {
> > > >  #define CK_443X		(1 << 11)
> > > >  #define CK_TI816X	(1 << 12)
> > > >  #define CK_446X		(1 << 13)
> > > > +#define CK_54XX		(1 << 14)
> > > 
> > > This is conflicting with AM33XX, you may want to rebase it again, since
> > > AM33xx clock tree is already pushed and available in
> > > linux-omap/devel-am33xx-part2.
> > 
> > Heh these CK_XXXX defines are now running out of the u16 cpu_mask.
> > 
> > They really should be replaced with SoC specific lists of clocks
> > rather than bloating the cpu_mask and repeating it for every clock
> > that's compiled in for 800+ times.
> > 
> > Below (untested) is what could be done in the short term.
> > 
> > I wonder if we could #define CK_OMAP_DUMMY 0 that's always set
> > for non-shared clocks if they only get set in some *_data.c
> > file in a unique way?
> > 
> > Paul got any better ideas?
...

> This also will not scale up in the future and will end up again in the same 
> situation.

Right that's why we want to get rid of it.
 
> Just a quick thought, may work here,
> 
> I looked at the usage of cpu_mask and rates.flag and I believe we can 
> restrict both to given SoC, something like,
> 
> OMAP34XX ->
>           ES1
>           ES2PLUS
>           36XX
>           AM35XX
>           ...
> 
> OMAP4 ->
>           443X
>           446X
> 
> AM33XX ->
>           AM335X
>           TI816X
>           TI814X
>           ...
> 
> XYZ...  ->
>           ...
> 
> 
> The proposal would be,
> 
> To make cpu_mask and rate.flags 32 bit wide and divide it in 16-16 bits -
> 
> Lower 16 bits => describe SoC it is applicable to
> Upper 16 bit => describes silicon versions or families

No thanks.. We don't want to make it 32 bit and bloat all the compiled in
clock even further. 

Regards,

Tony

Vaibhav Hiremath July 10, 2012, 8:30 a.m. UTC | #5

On Tue, Jul 10, 2012 at 13:48:52, Tony Lindgren wrote:
> * Hiremath, Vaibhav <[e-mail]> [120709 23:30]:
> > On Mon, Jul 09, 2012 at 18:41:58, Tony Lindgren wrote:
> > > * Vaibhav Hiremath <[e-mail]> [120709 01:55]:
> > > > On 7/6/2012 2:51 PM, Santosh Shilimkar wrote:
> > > > > --- a/arch/arm/plat-omap/include/plat/clkdev_omap.h
> > > > > +++ b/arch/arm/plat-omap/include/plat/clkdev_omap.h
> > > > > @@ -39,6 +39,7 @@ struct omap_clk {
> > > > >  #define CK_443X		(1 << 11)
> > > > >  #define CK_TI816X	(1 << 12)
> > > > >  #define CK_446X		(1 << 13)
> > > > > +#define CK_54XX		(1 << 14)
> > > > 
> > > > This is conflicting with AM33XX, you may want to rebase it again, since
> > > > AM33xx clock tree is already pushed and available in
> > > > linux-omap/devel-am33xx-part2.
> > > 
> > > Heh these CK_XXXX defines are now running out of the u16 cpu_mask.
> > > 
> > > They really should be replaced with SoC specific lists of clocks
> > > rather than bloating the cpu_mask and repeating it for every clock
> > > that's compiled in for 800+ times.
> > > 
> > > Below (untested) is what could be done in the short term.
> > > 
> > > I wonder if we could #define CK_OMAP_DUMMY 0 that's always set
> > > for non-shared clocks if they only get set in some *_data.c
> > > file in a unique way?
> > > 
> > > Paul got any better ideas?
> ...
> 
> > This also will not scale up in the future and will end up again in the same 
> > situation.
> 
> Right that's why we want to get rid of it.
>  
> > Just a quick thought, may work here,
> > 
> > I looked at the usage of cpu_mask and rates.flag and I believe we can 
> > restrict both to given SoC, something like,
> > 
> > OMAP34XX ->
> >           ES1
> >           ES2PLUS
> >           36XX
> >           AM35XX
> >           ...
> > 
> > OMAP4 ->
> >           443X
> >           446X
> > 
> > AM33XX ->
> >           AM335X
> >           TI816X
> >           TI814X
> >           ...
> > 
> > XYZ...  ->
> >           ...
> > 
> > 
> > The proposal would be,
> > 
> > To make cpu_mask and rate.flags 32 bit wide and divide it in 16-16 bits -
> > 
> > Lower 16 bits => describe SoC it is applicable to
> > Upper 16 bit => describes silicon versions or families
> 
> No thanks.. We don't want to make it 32 bit and bloat all the compiled in
> clock even further. 
> 

In that case, how about just get rid of cpu_mask completely and trust the 
data passed by clock-tree for clksel dividers?
Let clock-tree data handle this, even if in some cases we end up in 
duplicating data for some clocks??

Thanks,
Vaibhav

Tony Lindgren July 10, 2012, 8:37 a.m. UTC | #6

* Hiremath, Vaibhav <[e-mail]> [120710 01:35]:
> On Tue, Jul 10, 2012 at 13:48:52, Tony Lindgren wrote:
> > 
> > No thanks.. We don't want to make it 32 bit and bloat all the compiled in
> > clock even further. 
> > 
> 
> In that case, how about just get rid of cpu_mask completely and trust the 
> data passed by clock-tree for clksel dividers?
> Let clock-tree data handle this, even if in some cases we end up in 
> duplicating data for some clocks??

Yes something like that. We already know which clocks need to
be registered, so whatever we still use CK_XXXX for should be also
initialized in omapxxxx_clk_init() functions.

Regards,

Tony

Paul Walmsley Aug. 15, 2012, 10:26 p.m. UTC | #7

Hi

On Mon, 9 Jul 2012, Tony Lindgren wrote:

> Below (untested) is what could be done in the short term.

That's fine with me.  Do you want to queue it or do you want me to queue 
it?

> Heh these CK_XXXX defines are now running out of the u16 cpu_mask.
> 
> They really should be replaced with SoC specific lists of clocks
> rather than bloating the cpu_mask and repeating it for every clock
> that's compiled in for 800+ times.

Frankly, an extra 1.6KB -- uncompressed -- is pretty low on my list of 
bloat concerns for multi-OMAP kernels.  If it were up to me, I'd just 
change it to a u32 and be done with the problem for the foreseeable 
future.

> I wonder if we could #define CK_OMAP_DUMMY 0 that's always set
> for non-shared clocks if they only get set in some *_data.c
> file in a unique way?
> 
> Paul got any better ideas?

Aside from using u32?  Not really.  As we've discussed in the past, at 
some point we should convert the clock initialization to using some kind 
of per-SoC list.  But it doesn't seem worth spending too much time on that 
while the common clock framework conversion is higher priority.


- Paul

Tony Lindgren Aug. 16, 2012, 8:39 a.m. UTC | #8

* Paul Walmsley <[e-mail]> [120815 15:27]:
> Hi
> 
> On Mon, 9 Jul 2012, Tony Lindgren wrote:
> 
> > Below (untested) is what could be done in the short term.
> 
> That's fine with me.  Do you want to queue it or do you want me to queue 
> it?

Probably best for you to take it along with other related patches.
 
> > Heh these CK_XXXX defines are now running out of the u16 cpu_mask.
> > 
> > They really should be replaced with SoC specific lists of clocks
> > rather than bloating the cpu_mask and repeating it for every clock
> > that's compiled in for 800+ times.
> 
> Frankly, an extra 1.6KB -- uncompressed -- is pretty low on my list of 
> bloat concerns for multi-OMAP kernels.  If it were up to me, I'd just 
> change it to a u32 and be done with the problem for the foreseeable 
> future.

And then we're wasting that 1.6KB..
 
> > I wonder if we could #define CK_OMAP_DUMMY 0 that's always set
> > for non-shared clocks if they only get set in some *_data.c
> > file in a unique way?
> > 
> > Paul got any better ideas?
> 
> Aside from using u32?  Not really.  As we've discussed in the past, at 
> some point we should convert the clock initialization to using some kind 
> of per-SoC list.  But it doesn't seem worth spending too much time on that 
> while the common clock framework conversion is higher priority.

Right, let's do the ifdef else thing then.

Regards,

Tony

Vaibhav Hiremath Aug. 16, 2012, 9:36 a.m. UTC | #9

On Thu, Aug 16, 2012 at 03:56:42, Paul Walmsley wrote:
> Hi
> 
> On Mon, 9 Jul 2012, Tony Lindgren wrote:
> 
> > Below (untested) is what could be done in the short term.
> 
> That's fine with me.  Do you want to queue it or do you want me to queue 
> it?
> 
> > Heh these CK_XXXX defines are now running out of the u16 cpu_mask.
> > 
> > They really should be replaced with SoC specific lists of clocks
> > rather than bloating the cpu_mask and repeating it for every clock
> > that's compiled in for 800+ times.
> 
> Frankly, an extra 1.6KB -- uncompressed -- is pretty low on my list of 
> bloat concerns for multi-OMAP kernels.  If it were up to me, I'd just 
> change it to a u32 and be done with the problem for the foreseeable 
> future.
> 
> > I wonder if we could #define CK_OMAP_DUMMY 0 that's always set
> > for non-shared clocks if they only get set in some *_data.c
> > file in a unique way?
> > 
> > Paul got any better ideas?
> 
> Aside from using u32?  Not really.  As we've discussed in the past, at 
> some point we should convert the clock initialization to using some kind 
> of per-SoC list.  But it doesn't seem worth spending too much time on that 
> while the common clock framework conversion is higher priority.
> 
> 

This reminds me for AM33xx clock-tree migration to common-clock framework,
so just wanted to update on this, I have already converted the clock-tree to 
common-clock fw, on top of Rajendra's repository.

Now waiting on Rajendra for his next series...

Thanks,
Vaibhav

Patch

--- a/arch/arm/plat-omap/include/plat/clkdev_omap.h
+++ b/arch/arm/plat-omap/include/plat/clkdev_omap.h
@@ -26,26 +26,29 @@  struct omap_clk {
 	}
 
 /* Platform flags for the clkdev-OMAP integration code */
+
+#ifdef CONFIG_ARCH_OMAP1
 #define CK_310		(1 << 0)
 #define CK_7XX		(1 << 1)	/* 7xx, 850 */
 #define CK_1510		(1 << 2)
 #define CK_16XX		(1 << 3)	/* 16xx, 17xx, 5912 */
-#define CK_242X		(1 << 4)
-#define CK_243X		(1 << 5)	/* 243x, 253x */
-#define CK_3430ES1	(1 << 6)	/* 34xxES1 only */
-#define CK_3430ES2PLUS	(1 << 7)	/* 34xxES2, ES3, non-Sitara 35xx only */
-#define CK_AM35XX	(1 << 9)	/* Sitara AM35xx */
-#define CK_36XX		(1 << 10)	/* 36xx/37xx-specific clocks */
-#define CK_443X		(1 << 11)
-#define CK_TI816X	(1 << 12)
-#define CK_446X		(1 << 13)
-#define CK_AM33XX	(1 << 14)	/* AM33xx specific clocks */
-#define CK_1710		(1 << 15)	/* 1710 extra for rate selection */
-
+#define CK_1710		(1 << 4)	/* 1710 extra for rate selection */
+#endif
 
+#ifdef CONFIG_ARCH_OMAP2PLUS
+#define CK_242X		(1 << 0)
+#define CK_243X		(1 << 1)	/* 243x, 253x */
+#define CK_3430ES1	(1 << 2)	/* 34xxES1 only */
+#define CK_3430ES2PLUS	(1 << 3)	/* 34xxES2, ES3, non-Sitara 35xx only */
+#define CK_AM35XX	(1 << 4)	/* Sitara AM35xx */
+#define CK_36XX		(1 << 5)	/* 36xx/37xx-specific clocks */
+#define CK_443X		(1 << 6)
+#define CK_TI816X	(1 << 7)
+#define CK_446X		(1 << 8)
+#define CK_AM33XX	(1 << 9)	/* AM33xx specific clocks */
 #define CK_34XX		(CK_3430ES1 | CK_3430ES2PLUS)
 #define CK_3XXX		(CK_34XX | CK_AM35XX | CK_36XX)
-
+#endif
 
 #endif