[PATCH v2 3/6] TI816X: Update common OMAP machine specific sources

[PATCH v2 3/6] TI816X: Update common OMAP machine specific sources

[Hemant Pedanekar]

This patch updates the common machine spcific source files with support for
TI816X.

Note that the nr_irqs is overridden in INTC driver since the number of IRQs on
TI816X are different (128) comapared to other OMAPs (96).

Signed-off-by: Hemant Pedanekar <hemantp@ti.com>
---
 arch/arm/mach-omap2/clockdomain.c |   20 ++++++++++----------
 arch/arm/mach-omap2/id.c          |   25 +++++++++++++++++++++++++
 arch/arm/mach-omap2/io.c          |   21 ++++++++++++++++++++-
 arch/arm/mach-omap2/irq.c         |    4 ++++
 arch/arm/mach-omap2/serial.c      |    3 ++-
 5 files changed, 61 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index 5d80cb8..d7d539d 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -1,5 +1,5 @@
 /*
- * OMAP2/3/4 clockdomain framework functions
+ * OMAP2/3/4 and TI816X clockdomain framework functions
  *
  * Copyright (C) 2008-2010 Texas Instruments, Inc.
  * Copyright (C) 2008-2010 Nokia Corporation
@@ -240,7 +240,7 @@ static void _omap2_clkdm_set_hwsup(struct clockdomain *clkdm, int enable)
 			bits = OMAP24XX_CLKSTCTRL_ENABLE_AUTO;
 		else
 			bits = OMAP24XX_CLKSTCTRL_DISABLE_AUTO;
-	} else if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
+	} else if (cpu_is_omap34xx() || cpu_is_omap44xx() || cpu_is_ti816x()) {
 		if (enable)
 			bits = OMAP34XX_CLKSTCTRL_ENABLE_AUTO;
 		else
@@ -812,7 +812,7 @@ int omap2_clkdm_sleep(struct clockdomain *clkdm)
 		cm_set_mod_reg_bits(OMAP24XX_FORCESTATE_MASK,
 			    clkdm->pwrdm.ptr->prcm_offs, OMAP2_PM_PWSTCTRL);
 
-	} else if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
+	} else if (cpu_is_omap34xx() || cpu_is_omap44xx() || cpu_is_ti816x()) {
 
 		u32 bits = (OMAP34XX_CLKSTCTRL_FORCE_SLEEP <<
 			 __ffs(clkdm->clktrctrl_mask));
@@ -856,7 +856,7 @@ int omap2_clkdm_wakeup(struct clockdomain *clkdm)
 		cm_clear_mod_reg_bits(OMAP24XX_FORCESTATE_MASK,
 			      clkdm->pwrdm.ptr->prcm_offs, OMAP2_PM_PWSTCTRL);
 
-	} else if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
+	} else if (cpu_is_omap34xx() || cpu_is_omap44xx() || cpu_is_ti816x()) {
 
 		u32 bits = (OMAP34XX_CLKSTCTRL_FORCE_WAKEUP <<
 			 __ffs(clkdm->clktrctrl_mask));
@@ -899,10 +899,10 @@ void omap2_clkdm_allow_idle(struct clockdomain *clkdm)
 
 	/*
 	 * XXX This should be removed once TI adds wakeup/sleep
-	 * dependency code and data for OMAP4.
+	 * dependency code and data for OMAP4, TI816X
 	 */
-	if (cpu_is_omap44xx()) {
-		WARN_ONCE(1, "clockdomain: OMAP4 wakeup/sleep dependency "
+	if (cpu_is_omap44xx() || cpu_is_ti816x()) {
+		WARN_ONCE(1, "clockdomain: wakeup/sleep dependency "
 			  "support is not yet implemented\n");
 	} else {
 		if (atomic_read(&clkdm->usecount) > 0)
@@ -941,10 +941,10 @@ void omap2_clkdm_deny_idle(struct clockdomain *clkdm)
 
 	/*
 	 * XXX This should be removed once TI adds wakeup/sleep
-	 * dependency code and data for OMAP4.
+	 * dependency code and data for OMAP4, TI816X.
 	 */
-	if (cpu_is_omap44xx()) {
-		WARN_ONCE(1, "clockdomain: OMAP4 wakeup/sleep dependency "
+	if (cpu_is_omap44xx() || cpu_is_ti816x()) {
+		WARN_ONCE(1, "clockdomain: wakeup/sleep dependency "
 			  "support is not yet implemented\n");
 	} else {
 		if (atomic_read(&clkdm->usecount) > 0)
diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index 9a879f9..ca0779a 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -61,6 +61,8 @@ int omap_type(void)
 		val = omap_ctrl_readl(OMAP343X_CONTROL_STATUS);
 	} else if (cpu_is_omap44xx()) {
 		val = omap_ctrl_readl(OMAP44XX_CONTROL_STATUS);
+	} else if (cpu_is_ti816x()) {
+		val = omap_ctrl_readl(TI816X_CONTROL_STATUS);
 	} else {
 		pr_err("Cannot detect omap type!\n");
 		goto out;
@@ -319,6 +321,26 @@ static void __init omap4_check_revision(void)
 	pr_err("Unknown OMAP4 CPU id\n");
 }
 
+void __init ti816x_check_revision(void)
+{
+	u32 idcode;
+	u16 partnum;
+	u8 rev;
+
+	idcode = read_tap_reg(TI816X_CONTROL_DEVICE_ID);
+	partnum = (idcode >> 12) & 0xffff;
+	rev = (idcode >> 28) & 0xff;
+
+	if ((partnum == 0xb81e) && (rev == 0x0)) {
+		omap_revision = TI8168_REV_ES1_0;
+		omap_chip.oc |= CHIP_IS_TI816X;
+		pr_info("OMAP chip is TI8168\n");
+		return;
+	}
+
+	pr_err("Unknown TI816X CPU id\n");
+}
+
 #define OMAP3_SHOW_FEATURE(feat)		\
 	if (omap3_has_ ##feat())		\
 		printk(#feat" ");
@@ -419,6 +441,9 @@ void __init omap2_check_revision(void)
 	} else if (cpu_is_omap44xx()) {
 		omap4_check_revision();
 		return;
+	} else if (cpu_is_ti816x()) {
+		ti816x_check_revision();
+		return;
 	} else {
 		pr_err("OMAP revision unknown, please fix!\n");
 	}
diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
index b9ea70b..8675b57 100644
--- a/arch/arm/mach-omap2/io.c
+++ b/arch/arm/mach-omap2/io.c
@@ -229,6 +229,17 @@ static struct map_desc omap44xx_io_desc[] __initdata = {
 };
 #endif
 
+#ifdef CONFIG_ARCH_TI816X
+static struct map_desc ti816x_io_desc[] __initdata = {
+	{
+		.virtual	= L4_SLOW_TI816X_VIRT,
+		.pfn		= __phys_to_pfn(L4_SLOW_TI816X_PHYS),
+		.length		= L4_SLOW_TI816X_SIZE,
+		.type		= MT_DEVICE
+	},
+};
+#endif
+
 static void __init _omap2_map_common_io(void)
 {
 	/* Normally devicemaps_init() would flush caches and tlb after
@@ -276,6 +287,14 @@ void __init omap44xx_map_common_io(void)
 }
 #endif
 
+#ifdef CONFIG_ARCH_TI816X
+void __init ti816x_map_common_io()
+{
+	iotable_init(ti816x_io_desc, ARRAY_SIZE(ti816x_io_desc));
+	_omap2_map_common_io();
+}
+#endif
+
 /*
  * omap2_init_reprogram_sdrc - reprogram SDRC timing parameters
  *
@@ -342,7 +361,7 @@ void __init omap2_init_common_hw(struct omap_sdrc_params *sdrc_cs0,
 #ifndef CONFIG_PM_RUNTIME
 	skip_setup_idle = 1;
 #endif
-	if (cpu_is_omap24xx() || cpu_is_omap34xx())   /* FIXME: OMAP4 */
+	if (cpu_is_omap24xx() || cpu_is_omap34xx())   /* FIXME: OMAP4, TI8168 */
 		omap_hwmod_late_init(skip_setup_idle);
 
 	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
index 26aeef5..e440d7f 100644
--- a/arch/arm/mach-omap2/irq.c
+++ b/arch/arm/mach-omap2/irq.c
@@ -201,6 +201,10 @@ void __init omap_init_irq(void)
 			base = OMAP24XX_IC_BASE;
 		else if (cpu_is_omap34xx())
 			base = OMAP34XX_IC_BASE;
+		else if (cpu_is_ti816x()) {
+			base = TI816X_ARM_INTC_BASE;
+			bank->nr_irqs = 128;
+		}
 
 		BUG_ON(!base);
 
diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 566e991..4b654cf 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -798,10 +798,11 @@ void __init omap_serial_init_port(int port)
 
 	/*
 	 * omap44xx: Never read empty UART fifo
+	 * ti816x: Never read when UART fifo empty or write when full
 	 * omap3xxx: Never read empty UART fifo on UARTs
 	 * with IP rev >=0x52
 	 */
-	if (cpu_is_omap44xx())
+	if (cpu_is_omap44xx() || cpu_is_ti816x())
 		uart->errata |= UART_ERRATA_FIFO_FULL_ABORT;
 	else if ((serial_read_reg(uart->p, UART_OMAP_MVER) & 0xFF)
 			>= UART_OMAP_NO_EMPTY_FIFO_READ_IP_REV)
-- 
1.6.2.4

Re: [PATCH v2 3/6] TI816X: Update common OMAP machine specific sources

[Tony Lindgren]

* Hemant Pedanekar <hemantp@ti.com> [100811 10:03]:
> This patch updates the common machine spcific source files with support for
> TI816X.
> 
> Note that the nr_irqs is overridden in INTC driver since the number of IRQs on
> TI816X are different (128) comapared to other OMAPs (96).

<snip>

> --- a/arch/arm/mach-omap2/id.c
> +++ b/arch/arm/mach-omap2/id.c
> @@ -419,6 +441,9 @@ void __init omap2_check_revision(void)
>  	} else if (cpu_is_omap44xx()) {
>  		omap4_check_revision();
>  		return;
> +	} else if (cpu_is_ti816x()) {
> +		ti816x_check_revision();
> +		return;
>  	} else {
>  		pr_err("OMAP revision unknown, please fix!\n");
>  	}

This does not look right, at this point you should just know
the processor class set by the set_globals call.

Please take a look at plat-omap/common.c and add an entry for
omap2_set_globals_ti816x. This gets called very early from the board-*.c
file, so you can initialize things.

Note that by default we are building in support for omap2, 3 and 4
and support for all the boards too. This allows booting them
with the same kernel binary for developers and distros.

In general, you might want to see how we added support for omap36xx.
TI816X seems to be very close to omap3 with some extra registers, and
I'm not at all convinced that we even need to set it up as a separate
processor. Basically adding the set_globals + doing the more
accurate detection in id.c should do the trick.

Regards,

Tony

RE: [PATCH v2 3/6] TI816X: Update common OMAP machine specific sources

[Pedanekar, Hemant]

Tony Lindgren wrote on Friday, September 17, 2010 3:55 AM:

> * Hemant Pedanekar <hemantp@ti.com> [100811 10:03]:
>> This patch updates the common machine spcific source files with support
>> for TI816X. 
>> 
>> Note that the nr_irqs is overridden in INTC driver since the number of
>> IRQs on TI816X are different (128) comapared to other OMAPs (96).
> 
> <snip>
> 
>> --- a/arch/arm/mach-omap2/id.c
>> +++ b/arch/arm/mach-omap2/id.c
>> @@ -419,6 +441,9 @@ void __init omap2_check_revision(void)
>>  	} else if (cpu_is_omap44xx()) {
>>  		omap4_check_revision();
>>  		return;
>> +	} else if (cpu_is_ti816x()) {
>> +		ti816x_check_revision();
>> +		return;
>>  	} else {
>>  		pr_err("OMAP revision unknown, please fix!\n");
>>  	}
> 
> This does not look right, at this point you should just know
> the processor class set by the set_globals call.
> 
> Please take a look at plat-omap/common.c and add an entry for
> omap2_set_globals_ti816x. This gets called very early from
> the board-*.c
> file, so you can initialize things.

Tony,

Do you mean following changes in the patch I sent earlier?

In arch/arm/mach-omap2/common.c -->

#if defined(CONFIG_ARCH_TI816X)
static struct omap_globals ti816x_globals = {
-        .class  = TI816X_CLASS,
+        .class  = TI81XX_CLASS,

In arch/arm/mach-omap2/id.c -->
-void __init ti816x_check_revision(void)
+void __init ti81xx_check_revision(void)
 {
        u32 idcode;
        u16 partnum;
        u8 rev;

-       idcode = read_tap_reg(TI816X_CONTROL_DEVICE_ID);
+       idcode = read_tap_reg(TI81XX_CONTROL_DEVICE_ID);
        partnum = (idcode >> 12) & 0xffff;
        rev = (idcode >> 28) & 0xff;

@@ -490,8 +490,8 @@ void __init omap2_check_revision(void)
        } else if (cpu_is_omap44xx()) {
                omap4_check_revision();
                return;
-       } else if (cpu_is_ti816x()) {
-               ti816x_check_revision();
+       } else if (cpu_is_ti81xx()) {
+               ti81xx_check_revision();
                return;

> 
> Note that by default we are building in support for omap2, 3 and 4
> and support for all the boards too. This allows booting them
> with the same kernel binary for developers and distros.
> 
> In general, you might want to see how we added support for omap36xx.
> TI816X seems to be very close to omap3 with some extra registers, and
> I'm not at all convinced that we even need to set it up as a separate
> processor. Basically adding the set_globals + doing the more
> accurate detection in id.c should do the trick.
> 

The TI816X series, though similar to Omap architecture wise, has 
differences in PRCM, PLL etc w.r.t. Omap3.

Please refer following link for more details about the series:
http://focus.ti.com/general/docs/gencontent.tsp?contentId=77960

Thanks
-
Hemant

Re: [PATCH v2 3/6] TI816X: Update common OMAP machine specific sources

[Tony Lindgren]

* Pedanekar, Hemant <hemantp@ti.com> [101022 10:58]:
> Tony Lindgren wrote on Friday, September 17, 2010 3:55 AM:
> 
> > * Hemant Pedanekar <hemantp@ti.com> [100811 10:03]:
> >> This patch updates the common machine spcific source files with support
> >> for TI816X. 
> >> 
> >> Note that the nr_irqs is overridden in INTC driver since the number of
> >> IRQs on TI816X are different (128) comapared to other OMAPs (96).
> > 
> > <snip>
> > 
> >> --- a/arch/arm/mach-omap2/id.c
> >> +++ b/arch/arm/mach-omap2/id.c
> >> @@ -419,6 +441,9 @@ void __init omap2_check_revision(void)
> >>  	} else if (cpu_is_omap44xx()) {
> >>  		omap4_check_revision();
> >>  		return;
> >> +	} else if (cpu_is_ti816x()) {
> >> +		ti816x_check_revision();
> >> +		return;
> >>  	} else {
> >>  		pr_err("OMAP revision unknown, please fix!\n");
> >>  	}
> > 
> > This does not look right, at this point you should just know
> > the processor class set by the set_globals call.
> > 
> > Please take a look at plat-omap/common.c and add an entry for
> > omap2_set_globals_ti816x. This gets called very early from
> > the board-*.c
> > file, so you can initialize things.
> 
> Tony,
> 
> Do you mean following changes in the patch I sent earlier?

No, I'm mostly wondering how come you can't use cpu_is_omap34xx
for ti816x and then detect based on the values set in
omap2_set_globals. Then cpu_is_ti816x just becomes a set of
features.

In general, we should initialize more things in set_globals
if necessary rather than sprinkle cpu_is_omap tests all over
the place to add support for new omaps.

Regards,

Tony

RE: [PATCH v2 3/6] TI816X: Update common OMAP machine specific sources

[Pedanekar, Hemant]

Tony Lindgren wrote on Saturday, November 06, 2010 2:30 AM:

> * Pedanekar, Hemant <hemantp@ti.com> [101022 10:58]:
>> Tony Lindgren wrote on Friday, September 17, 2010 3:55 AM:
>> 
>>> * Hemant Pedanekar <hemantp@ti.com> [100811 10:03]:
>>>> This patch updates the common machine spcific source files with support
>>>> for TI816X. 
>>>> 
>>>> Note that the nr_irqs is overridden in INTC driver since the number of
>>>> IRQs on TI816X are different (128) comapared to other OMAPs (96).
>>> 
>>> <snip>
>>> 
>>>> --- a/arch/arm/mach-omap2/id.c
>>>> +++ b/arch/arm/mach-omap2/id.c
>>>> @@ -419,6 +441,9 @@ void __init omap2_check_revision(void)
>>>>  	} else if (cpu_is_omap44xx()) {
>>>>  		omap4_check_revision();
>>>>  		return;
>>>> +	} else if (cpu_is_ti816x()) {
>>>> +		ti816x_check_revision();
>>>> +		return;
>>>>  	} else {
>>>>  		pr_err("OMAP revision unknown, please fix!\n");
>>>>  	}
>>> 
>>> This does not look right, at this point you should just know
>>> the processor class set by the set_globals call.
>>> 
>>> Please take a look at plat-omap/common.c and add an entry for
>>> omap2_set_globals_ti816x. This gets called very early from the board-*.c
>>> file, so you can initialize things.
>> 
>> Tony,
>> 
>> Do you mean following changes in the patch I sent earlier?
> 
> No, I'm mostly wondering how come you can't use cpu_is_omap34xx
> for ti816x and then detect based on the values set in
> omap2_set_globals. Then cpu_is_ti816x just becomes a set of features.
> 
> In general, we should initialize more things in set_globals
> if necessary rather than sprinkle cpu_is_omap tests all over
> the place to add support for new omaps.
> 

Tony,

Though based on Cortex A8, TI816X series has differences in PRCM, PLL, clock
structure compared to OMAP3.

Many of the OMAP3 specific checks are not applicable for TI816X. For example, 
consider following:
File - arch/arm/mach-omap2/omap_hwmod.c 
Function - _wait_target_ready()

	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
		ret = omap2_cm_wait_module_ready(oh->prcm.omap2.module_offs,
				oh->prcm.omap2.idlest_reg_id,
				oh->prcm.omap2.idlest_idle_bit);

The above code inside cpu_is_omap34xx() check is not applicable for TI816X as
there are no CM_IDELST registers.

Thanks
-
Hemant

Re: [PATCH v2 3/6] TI816X: Update common OMAP machine specific sources

[Tony Lindgren]

* Pedanekar, Hemant <hemantp@ti.com> [101129 09:07]:
> Tony Lindgren wrote on Saturday, November 06, 2010 2:30 AM:
> 
> Though based on Cortex A8, TI816X series has differences in PRCM, PLL, clock
> structure compared to OMAP3.
> 
> Many of the OMAP3 specific checks are not applicable for TI816X. For example, 
> consider following:
> File - arch/arm/mach-omap2/omap_hwmod.c 
> Function - _wait_target_ready()
> 
> 	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
> 		ret = omap2_cm_wait_module_ready(oh->prcm.omap2.module_offs,
> 				oh->prcm.omap2.idlest_reg_id,
> 				oh->prcm.omap2.idlest_idle_bit);
> 
> The above code inside cpu_is_omap34xx() check is not applicable for TI816X as
> there are no CM_IDELST registers.

OK, so places like these will need different handling, and should
then be based on some idlest flag that gets set during the init
based on cpu_is_omap24xx() || cpu_is_omap3430() || cpu_is_omap3630().

Have you looked at defining cpu_is_ti816x() based on features?

Something like:

#define cpu_is_ti816x()		(cpu_is_omap34xx() && \
					(omap3_has_xyz()) 

Where xyz is based on some runtime detected flag, just like
we do for sgx and iva.

Regards,

Tony

RE: [PATCH v2 3/6] TI816X: Update common OMAP machine specific sources

[Pedanekar, Hemant]

Tony Lindgren wrote on Tuesday, November 30, 2010 12:59 AM:

> * Pedanekar, Hemant <hemantp@ti.com> [101129 09:07]:
>> Tony Lindgren wrote on Saturday, November 06, 2010 2:30 AM:
>> 
>> Though based on Cortex A8, TI816X series has differences in PRCM, PLL,
>> clock structure compared to OMAP3. 
>> 
>> Many of the OMAP3 specific checks are not applicable for TI816X. For
>> example, consider following: File - arch/arm/mach-omap2/omap_hwmod.c
>> Function - _wait_target_ready()
>> 
>> 	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
>> 		ret =
> omap2_cm_wait_module_ready(oh->prcm.omap2.module_offs,
>> 				oh->prcm.omap2.idlest_reg_id,
>> 				oh->prcm.omap2.idlest_idle_bit);
>> 
>> The above code inside cpu_is_omap34xx() check is not applicable for TI816X
>> as there are no CM_IDELST registers.
> 
> OK, so places like these will need different handling, and should
> then be based on some idlest flag that gets set during the init
> based on cpu_is_omap24xx() || cpu_is_omap3430() || cpu_is_omap3630().
>
Tony,

Thanks for clarifications. I have some concerns though:
1) I will eventually end up preceeding existing OMAP3 ckecks to have
cpu_is_34xx() fail. Fo example, in above case, since cpu_is_omap34xx will
be true for ti81xx (which we don't want), I need to update the code as:

	if (cpu_is_omap24xx() || (cpu_is_omap34xx() && !cpu_is_ti816x()) {
OR
	if (cpu_is_omap24xx() || (cpu_is_omap34xx() && omap_has_idlest()) {

Then proceed to have a TI816X specific handling in "else if" part with
cpu_is_ti816x() check.
 
2) Various module base addresses part of global data are different compared to
OMAP3/4 - e.g., .tap, .ctrl, .prm etc are different. This means I will still
need separate global data in arch/arm/mach-omap2/common.c as present for
OMAP3/4 and have it set up in omap2_set_globals_ti816x().

3) Differences in PRCM, PLL mean we need a separate clock data files such as
clock816x_data.c, clockdomains816x.h, powerdomains816x.h etc. In fact, future
SoCs in 816x (or rather, 81xx) series will share the same organization and we
will be adding to these files instead of growing omap3xxx_data.c etc. Of course,
I see some special handling done depending upon cpu_is_ and features in
omap3xxx_clk_init() - but will similar approach scale with TI816X which has
completely different clock data?

Similarly, we will also need to add TI816X specific hwmods.

4) TI816X series shares similarity with OMAP4 too - e.g., various IPs are same,
CM module is closer to OMAP4 than OMAP3. Thus, regaring (1) above, I could use
OMAP4 code instead of adding new "else if". Of course, again, there are above
mentioned differences too.

With all this, won't it be better if we add TI816X (or TI81XX) series to exist
on similar lines with OMAP3/4?

Please let me know your thoughts on the above. I can send v3 patch set
incorporating your suggestion of plugging TI816X into OMAP3.    

Also, the patch set submitted doesn't have complete set of files yet
(particularly PRCM/clock code), please have a look at code hosted on Arago.
Below is the link to mach-omap2 folder (2.6.34 at the moment)
http://arago-project.org/git/people/?p=sriram/ti-psp-omap.git;a=tree;f=arch/arm/mach-omap2;h=7f2c48b8cb3213b3850d0b6ea242c0c53fa5d6fa;hb=refs/heads/ti816x-master

Thanks
-
Hemant

RE: [PATCH v2 3/6] TI816X: Update common OMAP machine specific sources

[Pedanekar, Hemant]

Pedanekar, Hemant wrote on Wednesday, December 01, 2010 7:17 AM:

> Tony Lindgren wrote on Tuesday, November 30, 2010 12:59 AM:
> 
>> * Pedanekar, Hemant <hemantp@ti.com> [101129 09:07]:
>>> Tony Lindgren wrote on Saturday, November 06, 2010 2:30 AM:
>>> 
>>> Though based on Cortex A8, TI816X series has differences in PRCM, PLL,
>>> clock structure compared to OMAP3.
>>> 
>>> Many of the OMAP3 specific checks are not applicable for TI816X. For
>>> example, consider following: File -
> arch/arm/mach-omap2/omap_hwmod.c
>>> Function - _wait_target_ready()
>>> 
>>> 	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
>>> 		ret =
>> omap2_cm_wait_module_ready(oh->prcm.omap2.module_offs,
>>> 				oh->prcm.omap2.idlest_reg_id,
>>> 				oh->prcm.omap2.idlest_idle_bit);
>>> 
>>> The above code inside cpu_is_omap34xx() check is not applicable for TI816X
>>> as there are no CM_IDELST registers.
>> 
>> OK, so places like these will need different handling, and should
>> then be based on some idlest flag that gets set during the init
>> based on cpu_is_omap24xx() || cpu_is_omap3430() || cpu_is_omap3630().
>> 
> Tony,
> 
> Thanks for clarifications. I have some concerns though:
> 1) I will eventually end up preceeding existing OMAP3 ckecks to have
> cpu_is_34xx() fail. Fo example, in above case, since
> cpu_is_omap34xx will
> be true for ti81xx (which we don't want), I need to update
> the code as:
> 
> 	if (cpu_is_omap24xx() || (cpu_is_omap34xx() &&
> !cpu_is_ti816x()) {
> OR
> 	if (cpu_is_omap24xx() || (cpu_is_omap34xx() &&
> omap_has_idlest()) {
> 
> Then proceed to have a TI816X specific handling in "else if" part with
> cpu_is_ti816x() check. 
> 
> 2) Various module base addresses part of global data are
> different compared to
> OMAP3/4 - e.g., .tap, .ctrl, .prm etc are different. This
> means I will still
> need separate global data in arch/arm/mach-omap2/common.c as
> present for
> OMAP3/4 and have it set up in omap2_set_globals_ti816x().
> 
> 3) Differences in PRCM, PLL mean we need a separate clock
> data files such as
> clock816x_data.c, clockdomains816x.h, powerdomains816x.h etc.
> In fact, future
> SoCs in 816x (or rather, 81xx) series will share the same organization and
> we will be adding to these files instead of growing
> omap3xxx_data.c etc. Of course,
> I see some special handling done depending upon cpu_is_ and
> features in
> omap3xxx_clk_init() - but will similar approach scale with
> TI816X which has
> completely different clock data?
> 
> Similarly, we will also need to add TI816X specific hwmods.
> 
> 4) TI816X series shares similarity with OMAP4 too - e.g.,
> various IPs are same,
> CM module is closer to OMAP4 than OMAP3. Thus, regaring (1)
> above, I could use
> OMAP4 code instead of adding new "else if". Of course, again,
> there are above
> mentioned differences too.
> 
> With all this, won't it be better if we add TI816X (or
> TI81XX) series to exist
> on similar lines with OMAP3/4?
> 
> Please let me know your thoughts on the above. I can send v3 patch set
> incorporating your suggestion of plugging TI816X into OMAP3.
> 
> Also, the patch set submitted doesn't have complete set of files yet
> (particularly PRCM/clock code), please have a look at code
> hosted on Arago.
> Below is the link to mach-omap2 folder (2.6.34 at the moment)
> http://arago-project.org/git/people/?p=sriram/ti-psp-omap.git;
a=tree;f=arch/arm/mach-omap2;h=>
7f2c48b8cb3213b3850d0b6ea242c0c53fa5d6fa;hb=refs/heads/ti816x-master 
> 
Tony,

Could you please provide your comments on the above?

Thanks
-
Hemant

Re: [PATCH v2 3/6] TI816X: Update common OMAP machine specific sources

[Tony Lindgren]

Hi,

Sorry for the delay.

* Pedanekar, Hemant <hemantp@ti.com> [101130 17:36]:
> Tony Lindgren wrote on Tuesday, November 30, 2010 12:59 AM:
> 
> Thanks for clarifications. I have some concerns though:
> 1) I will eventually end up preceeding existing OMAP3 ckecks to have
> cpu_is_34xx() fail. Fo example, in above case, since cpu_is_omap34xx will
> be true for ti81xx (which we don't want), I need to update the code as:
> 
> 	if (cpu_is_omap24xx() || (cpu_is_omap34xx() && !cpu_is_ti816x()) {
> OR
> 	if (cpu_is_omap24xx() || (cpu_is_omap34xx() && omap_has_idlest()) {
> 
> Then proceed to have a TI816X specific handling in "else if" part with
> cpu_is_ti816x() check.

In places like that omap_has_idlest or similar should be the only test
needed to avoid having to patch all over the place to add new processors.
  
> 2) Various module base addresses part of global data are different compared to
> OMAP3/4 - e.g., .tap, .ctrl, .prm etc are different. This means I will still
> need separate global data in arch/arm/mach-omap2/common.c as present for
> OMAP3/4 and have it set up in omap2_set_globals_ti816x().

Yes that we're already handling.
 
> 3) Differences in PRCM, PLL mean we need a separate clock data files such as
> clock816x_data.c, clockdomains816x.h, powerdomains816x.h etc. In fact, future
> SoCs in 816x (or rather, 81xx) series will share the same organization and we
> will be adding to these files instead of growing omap3xxx_data.c etc. Of course,
> I see some special handling done depending upon cpu_is_ and features in
> omap3xxx_clk_init() - but will similar approach scale with TI816X which has
> completely different clock data?

Those patches should be separate patches on top of the minimal supoort
to boot to console. 
 
> Similarly, we will also need to add TI816X specific hwmods.
> 
> 4) TI816X series shares similarity with OMAP4 too - e.g., various IPs are same,
> CM module is closer to OMAP4 than OMAP3. Thus, regaring (1) above, I could use
> OMAP4 code instead of adding new "else if". Of course, again, there are above
> mentioned differences too.

We really want to use same code for the shared modules so using feature
based detection is the way to go.
 
> With all this, won't it be better if we add TI816X (or TI81XX) series to exist
> on similar lines with OMAP3/4?

It depends, to me it still sounds like it should be done based on the
features. If necessary we can initialize more things with set_globals.
 
> Please let me know your thoughts on the above. I can send v3 patch set
> incorporating your suggestion of plugging TI816X into OMAP3.    

Well let's start with the absolute minimal patches to get those
integrated.
 
> Also, the patch set submitted doesn't have complete set of files yet
> (particularly PRCM/clock code), please have a look at code hosted on Arago.
> Below is the link to mach-omap2 folder (2.6.34 at the moment)
> http://arago-project.org/git/people/?p=sriram/ti-psp-omap.git;a=tree;f=arch/arm/mach-omap2;h=7f2c48b8cb3213b3850d0b6ea242c0c53fa5d6fa;hb=refs/heads/ti816x-master

Those you might want to post also for people to look at even if
we don't know yet how we should deal with them.

Regards,

Tony

RE: [PATCH v2 3/6] TI816X: Update common OMAP machine specific sources

[Pedanekar, Hemant]

Tony Lindgren wrote on Monday, December 06, 2010 10:23 PM:

[...]
>> Similarly, we will also need to add TI816X specific hwmods.
>> 
>> 4) TI816X series shares similarity with OMAP4 too - e.g., various IPs are
>> same, CM module is closer to OMAP4 than OMAP3. Thus, regaring (1) above, I
>> could use OMAP4 code instead of adding new "else if". Of course, again,
>> there are above mentioned differences too.
> 
> We really want to use same code for the shared modules so
> using feature
> based detection is the way to go.
> 
Tony,

Just wanted to confirm: so you are suggesting we do not have any
cpu_is_ti81xx() check in the code - is that correct? 

Thanks
-
Hemant

Re: [PATCH v2 3/6] TI816X: Update common OMAP machine specific sources

[Tony Lindgren]

* Pedanekar, Hemant <hemantp@ti.com> [101210 16:58]:
> Tony Lindgren wrote on Monday, December 06, 2010 10:23 PM:
> 
> [...]
> >> Similarly, we will also need to add TI816X specific hwmods.
> >> 
> >> 4) TI816X series shares similarity with OMAP4 too - e.g., various IPs are
> >> same, CM module is closer to OMAP4 than OMAP3. Thus, regaring (1) above, I
> >> could use OMAP4 code instead of adding new "else if". Of course, again,
> >> there are above mentioned differences too.
> > 
> > We really want to use same code for the shared modules so
> > using feature
> > based detection is the way to go.
> > 
> Tony,
> 
> Just wanted to confirm: so you are suggesting we do not have any
> cpu_is_ti81xx() check in the code - is that correct? 

We can have that, but we should avoid it where possible.
And ideally cpu_is_ti81xx() would be just something like:

#define cpu_is_ti81xx()		\\
	cpu_is_omap34xx() && omap_has_aaa() && omap_has_bbb()

Regards,

Tony

RE: [PATCH v2 3/6] TI816X: Update common OMAP machine specific sources

[Pedanekar, Hemant]

Tony Lindgren wrote on Saturday, December 11, 2010 7:21 AM:

> * Pedanekar, Hemant <hemantp@ti.com> [101210 16:58]:
>> Tony Lindgren wrote on Monday, December 06, 2010 10:23 PM:
>> 
>> [...]
>>>> Similarly, we will also need to add TI816X specific hwmods.
>>>> 
>>>> 4) TI816X series shares similarity with OMAP4 too - e.g., various IPs are
>>>> same, CM module is closer to OMAP4 than OMAP3. Thus, regaring (1) above,
>>>> I could use OMAP4 code instead of adding new "else if". Of course, again,
>>>> there are above mentioned differences too.
>>> 
>>> We really want to use same code for the shared modules so using feature
>>> based detection is the way to go.
>>> 
>> Tony,
>> 
>> Just wanted to confirm: so you are suggesting we do not have any
>> cpu_is_ti81xx() check in the code - is that correct?
> 
> We can have that, but we should avoid it where possible.
> And ideally cpu_is_ti81xx() would be just something like:
> 
> #define cpu_is_ti81xx()		\\
> 	cpu_is_omap34xx() && omap_has_aaa() && omap_has_bbb()
> 
> Regards,
> 
> Tony

Thanks. I will send the patches early next week.

-
Hemant