[PATCH] AM35xx: disable checking for reserved feature bits

[PATCH] AM35xx: disable checking for reserved feature bits

[Ilya Yanok]

Bits corresponding to the IVA and ISP features in OMAP_STATUS register
are reserved on AM35xx and checking for them results in wrong results.
So we don't want to check for this features on AM35xx.

Signed-off-by: Ilya Yanok <yanok@emcraft.com>
---
 arch/arm/mach-omap2/id.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index 7f47092..40bab99 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -181,10 +181,8 @@ static void __init omap3_check_features(void)
 	status = omap_ctrl_readl(OMAP3_CONTROL_OMAP_STATUS);
 
 	OMAP3_CHECK_FEATURE(status, L2CACHE);
-	OMAP3_CHECK_FEATURE(status, IVA);
 	OMAP3_CHECK_FEATURE(status, SGX);
 	OMAP3_CHECK_FEATURE(status, NEON);
-	OMAP3_CHECK_FEATURE(status, ISP);
 	if (cpu_is_omap3630())
 		omap_features |= OMAP3_HAS_192MHZ_CLK;
 	if (cpu_is_omap3430() || cpu_is_omap3630())
@@ -192,6 +190,10 @@ static void __init omap3_check_features(void)
 	if (cpu_is_omap3630() || omap_rev() == OMAP3430_REV_ES3_1 ||
 	    omap_rev() == OMAP3430_REV_ES3_1_2)
 		omap_features |= OMAP3_HAS_IO_CHAIN_CTRL;
+	if (!cpu_is_omap3505() && !cpu_is_omap3517()) {
+		OMAP3_CHECK_FEATURE(status, IVA);
+		OMAP3_CHECK_FEATURE(status, ISP);
+	}
 
 	omap_features |= OMAP3_HAS_SDRC;
 
-- 
1.7.6.4

Re: [PATCH] AM35xx: disable checking for reserved feature bits

[Kevin Hilman]

Ilya Yanok <yanok@emcraft.com> writes:

> Bits corresponding to the IVA and ISP features in OMAP_STATUS register
> are reserved on AM35xx and checking for them results in wrong results.

Ouch.

> So we don't want to check for this features on AM35xx.
>
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>

This "feature" selection mechanism is clearly not scaling to newer SoCs.
While this patch works around the problem, IMO, we need a more scalable
solution.

For features like IVA and ISP (and SGX) which are acutally IP blocks on
the SoC, not "features"  per-se, what we really need to be doing is
checking for the presence of the IP block, not checking a bit in a
register that's not consistent across various SoCs.

We already have all the knowledge about whether the IP blocks are
present in the SoC-specific hwmod data.  So checking for the "feature"
of a specific IP block should instead be done using an
omap_hwmod_lookup().

However, there's a bit of a snag because this "feature" detection is
currently done before the hwmods are registered.

As a quick-and-dirty proof of concept, the patch/hack below moves the
feature checking after the hwmod init (omap3 only currently) and uses
omap_hwmod_lookup() to check whether a given IP block exists.

I only did a quick test on one OMAP3 platform (3430/n900) and it seems
to work.    The init order changes need some more thought, as I didn't
fully validate whether the feature detection can be safely moved later
for all platforms.

This is just to show the direction we should be taking this SoC
detection for newer SoCs.

Kevin


diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index 7f47092..54eaf23 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -23,6 +23,7 @@
 
 #include <plat/common.h>
 #include <plat/cpu.h>
+#include <plat/omap_hwmod.h>
 
 #include <mach/id.h>
 
@@ -172,6 +173,18 @@ static void __init omap24xx_check_revision(void)
 		omap_features |= OMAP3_HAS_ ##feat;			\
 	}
 
+static void omap_check_ip(char *name, u32 feat)	
+{
+	struct omap_hwmod *oh;
+
+	oh = omap_hwmod_lookup(name);
+	printk("%s: checking for %s\n", __func__, name);
+	if (oh) {
+		printk("%s: %s present\n", __func__, name);
+		omap_features |= feat;
+	}
+}
+
 static void __init omap3_check_features(void)
 {
 	u32 status;
@@ -180,11 +193,12 @@ static void __init omap3_check_features(void)
 
 	status = omap_ctrl_readl(OMAP3_CONTROL_OMAP_STATUS);
 
+	omap_check_ip("iva", OMAP3_HAS_IVA);
+	omap_check_ip("isp", OMAP3_HAS_ISP);
+
 	OMAP3_CHECK_FEATURE(status, L2CACHE);
-	OMAP3_CHECK_FEATURE(status, IVA);
 	OMAP3_CHECK_FEATURE(status, SGX);
 	OMAP3_CHECK_FEATURE(status, NEON);
-	OMAP3_CHECK_FEATURE(status, ISP);
 	if (cpu_is_omap3630())
 		omap_features |= OMAP3_HAS_192MHZ_CLK;
 	if (cpu_is_omap3430() || cpu_is_omap3630())
diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
index 25d20ce..c3faa7e 100644
--- a/arch/arm/mach-omap2/io.c
+++ b/arch/arm/mach-omap2/io.c
@@ -393,12 +393,12 @@ void __init omap2430_init_early(void)
 void __init omap3_init_early(void)
 {
 	omap2_set_globals_3xxx();
-	omap_common_init_early();
 	omap3xxx_voltagedomains_init();
 	omap3xxx_powerdomains_init();
 	omap3xxx_clockdomains_init();
 	omap3xxx_hwmod_init();
 	omap_hwmod_init_postsetup();
+	omap_common_init_early();
 	omap3xxx_clk_init();
 }
 

Re: [PATCH] AM35xx: disable checking for reserved feature bits

[Tony Lindgren]

Hi,

* Kevin Hilman <khilman@ti.com> [111108 16:23]:
> Ilya Yanok <yanok@emcraft.com> writes:
> 
> > Bits corresponding to the IVA and ISP features in OMAP_STATUS register
> > are reserved on AM35xx and checking for them results in wrong results.
> 
> Ouch.
> 
> > So we don't want to check for this features on AM35xx.
> >
> > Signed-off-by: Ilya Yanok <yanok@emcraft.com>
> 
> This "feature" selection mechanism is clearly not scaling to newer SoCs.
> While this patch works around the problem, IMO, we need a more scalable
> solution.

Agreed.
 
> For features like IVA and ISP (and SGX) which are acutally IP blocks on
> the SoC, not "features"  per-se, what we really need to be doing is
> checking for the presence of the IP block, not checking a bit in a
> register that's not consistent across various SoCs.
> 
> We already have all the knowledge about whether the IP blocks are
> present in the SoC-specific hwmod data.  So checking for the "feature"
> of a specific IP block should instead be done using an
> omap_hwmod_lookup().
> 
> However, there's a bit of a snag because this "feature" detection is
> currently done before the hwmods are registered.
> 
> As a quick-and-dirty proof of concept, the patch/hack below moves the
> feature checking after the hwmod init (omap3 only currently) and uses
> omap_hwmod_lookup() to check whether a given IP block exists.
> 
> I only did a quick test on one OMAP3 platform (3430/n900) and it seems
> to work.    The init order changes need some more thought, as I didn't
> fully validate whether the feature detection can be safely moved later
> for all platforms.
> 
> This is just to show the direction we should be taking this SoC
> detection for newer SoCs.

This should be coordinated with the splitting of feature detection
as posted by Vaibhave in thread "[RFC PATCH] arm:omap: cleanup & split
omap2/3/4_check_revision function" thread.

We no longer need the SoC detection super early, so I suggest we make
the feature detection separate from SoC detection.

The SoC specific init can then call:

	omap3_check_revision();
	ti816x_check_features();

Regards,

Tony

Re: [PATCH] AM35xx: disable checking for reserved feature bits

[Kevin Hilman]

Tony Lindgren <tony@atomide.com> writes:

[...]

>> This "feature" selection mechanism is clearly not scaling to newer SoCs.
>> While this patch works around the problem, IMO, we need a more scalable
>> solution.
>
> Agreed.
>  
>> For features like IVA and ISP (and SGX) which are acutally IP blocks on
>> the SoC, not "features"  per-se, what we really need to be doing is
>> checking for the presence of the IP block, not checking a bit in a
>> register that's not consistent across various SoCs.
>> 
>> We already have all the knowledge about whether the IP blocks are
>> present in the SoC-specific hwmod data.  So checking for the "feature"
>> of a specific IP block should instead be done using an
>> omap_hwmod_lookup().
>> 
>> However, there's a bit of a snag because this "feature" detection is
>> currently done before the hwmods are registered.
>> 
>> As a quick-and-dirty proof of concept, the patch/hack below moves the
>> feature checking after the hwmod init (omap3 only currently) and uses
>> omap_hwmod_lookup() to check whether a given IP block exists.
>> 
>> I only did a quick test on one OMAP3 platform (3430/n900) and it seems
>> to work.    The init order changes need some more thought, as I didn't
>> fully validate whether the feature detection can be safely moved later
>> for all platforms.
>> 
>> This is just to show the direction we should be taking this SoC
>> detection for newer SoCs.
>
> This should be coordinated with the splitting of feature detection
> as posted by Vaibhave in thread "[RFC PATCH] arm:omap: cleanup & split
> omap2/3/4_check_revision function" thread.

Vaibhav,

Feel free to take my proposed patch and develop it further and include
it in your rework of the SoC/feature detection.

Kevin

RE: [PATCH] AM35xx: disable checking for reserved feature bits

[Hiremath, Vaibhav]

> -----Original Message-----
> From: Hilman, Kevin
> Sent: Saturday, December 10, 2011 6:51 AM
> To: Tony Lindgren
> Cc: Ilya Yanok; linux-omap@vger.kernel.org; wd@denx.de; dzu@denx.de;
> sasha_d@emcraft.com; Hiremath, Vaibhav
> Subject: Re: [PATCH] AM35xx: disable checking for reserved feature bits
> 
> Tony Lindgren <tony@atomide.com> writes:
> 
> [...]
> 
> >> This "feature" selection mechanism is clearly not scaling to newer SoCs.
> >> While this patch works around the problem, IMO, we need a more scalable
> >> solution.
> >
> > Agreed.
> >
> >> For features like IVA and ISP (and SGX) which are acutally IP blocks on
> >> the SoC, not "features"  per-se, what we really need to be doing is
> >> checking for the presence of the IP block, not checking a bit in a
> >> register that's not consistent across various SoCs.
> >>
> >> We already have all the knowledge about whether the IP blocks are
> >> present in the SoC-specific hwmod data.  So checking for the "feature"
> >> of a specific IP block should instead be done using an
> >> omap_hwmod_lookup().
> >>
> >> However, there's a bit of a snag because this "feature" detection is
> >> currently done before the hwmods are registered.
> >>
> >> As a quick-and-dirty proof of concept, the patch/hack below moves the
> >> feature checking after the hwmod init (omap3 only currently) and uses
> >> omap_hwmod_lookup() to check whether a given IP block exists.
> >>
> >> I only did a quick test on one OMAP3 platform (3430/n900) and it seems
> >> to work.    The init order changes need some more thought, as I didn't
> >> fully validate whether the feature detection can be safely moved later
> >> for all platforms.
> >>
> >> This is just to show the direction we should be taking this SoC
> >> detection for newer SoCs.
> >
> > This should be coordinated with the splitting of feature detection
> > as posted by Vaibhave in thread "[RFC PATCH] arm:omap: cleanup & split
> > omap2/3/4_check_revision function" thread.
> 
> Vaibhav,
> 
> Feel free to take my proposed patch and develop it further and include
> it in your rework of the SoC/feature detection.
> 
Sorry for delayed response,

This looks good approach to me, the problem is, currently in the HWMOD database
we don't have all the entries, like isp or gfx/sgx.

So please note that I have to first add the dummy entry for the same. Submitting RFC patch shortly.


Having said that, my original patch still required, so I will submit the
first patch splitting xxx_check_revision and xxx_check_features and then on
top of this I will merge hwmod related patch.

Thanks,
Vaibhav

> Kevin

RE: [PATCH] AM35xx: disable checking for reserved feature bits

[Hiremath, Vaibhav]

> -----Original Message-----
> From: Hilman, Kevin
> Sent: Saturday, December 10, 2011 6:51 AM
> To: Tony Lindgren
> Cc: Ilya Yanok; linux-omap@vger.kernel.org; wd@denx.de; dzu@denx.de;
> sasha_d@emcraft.com; Hiremath, Vaibhav
> Subject: Re: [PATCH] AM35xx: disable checking for reserved feature bits
> 
> Tony Lindgren <tony@atomide.com> writes:
> 
> [...]
> 
> >> This "feature" selection mechanism is clearly not scaling to newer SoCs.
> >> While this patch works around the problem, IMO, we need a more scalable
> >> solution.
> >
> > Agreed.
> >
<snip>
> >
> > This should be coordinated with the splitting of feature detection
> > as posted by Vaibhave in thread "[RFC PATCH] arm:omap: cleanup & split
> > omap2/3/4_check_revision function" thread.
> 
> Vaibhav,
> 
> Feel free to take my proposed patch and develop it further and include
> it in your rework of the SoC/feature detection.
> 
Kevin,

I spend some time on this, and I think it is not possible to use HWMOD 
Entries for feature check. Reason being,

	- The whole revision story is built upon howkeye and silicon rev.
        And both remains same for different devices in same family, 
        For example,
          omap3430, omap3503 and omap3515 (for that matter all AM37x) all
          will have same howkeye and silicon revision no.

        Also, in the kernel we have something like.

# define cpu_is_omap3515()              (cpu_is_omap3430() &&           \
                                                 (!omap3_has_iva()) &&   \
                                                 (omap3_has_sgx()))
# define cpu_is_omap3525()              (cpu_is_omap3430() &&           \
                                                 (!omap3_has_sgx()) &&   \
                                                 (omap3_has_iva()))

        Which means, you can not do IP detection before check_feature
        function.

	- The current omap_hwmod_xxxx_data.c uses silicon version alone
        and only differentiate between silicon versions. We do not 
        differentiate between different family of devices while registering
        hwmod data.

So the conclusion is, we have to stick to check_feature function.

Thanks,
Vaibhav


> Kevin

Re: [PATCH] AM35xx: disable checking for reserved feature bits

[Kevin Hilman]

"Hiremath, Vaibhav" <hvaibhav@ti.com> writes:

>> -----Original Message-----
>> From: Hilman, Kevin
>> Sent: Saturday, December 10, 2011 6:51 AM
>> To: Tony Lindgren
>> Cc: Ilya Yanok; linux-omap@vger.kernel.org; wd@denx.de; dzu@denx.de;
>> sasha_d@emcraft.com; Hiremath, Vaibhav
>> Subject: Re: [PATCH] AM35xx: disable checking for reserved feature bits
>> 
>> Tony Lindgren <tony@atomide.com> writes:
>> 
>> [...]
>> 
>> >> This "feature" selection mechanism is clearly not scaling to newer SoCs.
>> >> While this patch works around the problem, IMO, we need a more scalable
>> >> solution.
>> >
>> > Agreed.
>> >
> <snip>
>> >
>> > This should be coordinated with the splitting of feature detection
>> > as posted by Vaibhave in thread "[RFC PATCH] arm:omap: cleanup & split
>> > omap2/3/4_check_revision function" thread.
>> 
>> Vaibhav,
>> 
>> Feel free to take my proposed patch and develop it further and include
>> it in your rework of the SoC/feature detection.
>> 
> Kevin,
>
> I spend some time on this, and I think it is not possible to use HWMOD 
> Entries for feature check. Reason being,
>
> 	- The whole revision story is built upon howkeye and silicon rev.
>         And both remains same for different devices in same family, 
>         For example,
>           omap3430, omap3503 and omap3515 (for that matter all AM37x) all
>           will have same howkeye and silicon revision no.

I see now.

>         Also, in the kernel we have something like.
>
> # define cpu_is_omap3515()              (cpu_is_omap3430() &&           \
>                                                  (!omap3_has_iva()) &&   \
>                                                  (omap3_has_sgx()))
> # define cpu_is_omap3525()              (cpu_is_omap3430() &&           \
>                                                  (!omap3_has_sgx()) &&   \
>                                                  (omap3_has_iva()))
>
>         Which means, you can not do IP detection before check_feature
>         function.
> 	- The current omap_hwmod_xxxx_data.c uses silicon version alone
>         and only differentiate between silicon versions. We do not 
>         differentiate between different family of devices while registering
>         hwmod data.
>
> So the conclusion is, we have to stick to check_feature function.

I don't fully agree.

I still think we need to rework how we're using cpu_is_* checks.

Specifially, we need to start separating the checking for SoC family
(based on die ID) and specific IP blocks.

I spent some time on this today and just posted a series for RFC to
hopefully move in that direction.   Let me know what you think.

Kevin