Re: [PATCH] ARM: OMAP4: execute initcall to reserve SRAM for I688 only on OMAP4

[Lucas Stach <l.stach@pengutronix.de>]

Am Montag, den 30.11.2015, 20:27 +0200 schrieb Grygorii Strashko:
> On 11/30/2015 07:27 PM, Lucas Stach wrote:
> > Am Montag, den 16.11.2015, 14:24 +0200 schrieb Grygorii Strashko:
> >> On 11/16/2015 01:25 PM, Lucas Stach wrote:
> >>> omap_interconnect_sync() is the only user of the SRAM scratch area
> >>> allocated in the omap4_sram_init initcall. The interconnect sync is
> >>> used exclusively in the OMAP4 specific WFI implementation, so there
> >>> is no point in allocating the SRAM scratch on other SoC types.
> >>>
> >>> Bail out of the initcall if the kernel is not running on OMAP4 to
> >>> avoid a confusing warning about being unable to allocate the SRAM
> >>> needed for I688 handling.
> >>>
> >>> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> >>> Tested-by: Bastian Stender <bst@pengutronix.de>
> >>> ---
> >>>    arch/arm/mach-omap2/omap4-common.c | 3 +++
> >>>    1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/arch/arm/mach-omap2/omap4-common.c b/arch/arm/mach-omap2/omap4-common.c
> >>> index 949696b6f17b..6db393a30a28 100644
> >>> --- a/arch/arm/mach-omap2/omap4-common.c
> >>> +++ b/arch/arm/mach-omap2/omap4-common.c
> >>> @@ -131,6 +131,9 @@ static int __init omap4_sram_init(void)
> >>>    	struct device_node *np;
> >>>    	struct gen_pool *sram_pool;
> >>>    
> >>> +	if (!cpu_is_omap44xx())
> >>> +		return 0;
> >>
> >> This one affects on am43xx also
> >>
> > So you are saying this erratum is also present on AM43xx? I wasn't able
> > to deduce this from the information provided by Richard Woodruff.
> > 
> 
> "..SOCs using similar chassis components of OMAP4430 time are impacted..."
> "..But AM335x should be immune from this particular issue..."
> 
> Advisory 11 Asynchronous Bridge Corruption
> http://www.ti.com/lit/er/sprz408b/sprz408b.pdf
> 
> 
Thanks for the link, it makes things a lot more clear.
> 
> >>
> >>> +
> >>>    	np = of_find_compatible_node(NULL, NULL, "ti,omap4-mpu");
> >>>    	if (!np)
> >>>    		pr_warn("%s:Unable to allocate sram needed to handle errata I688\n",
> >>
> >> Since all OMAP4+ platforms are now DT based why can't we just return from here silently?
> >>
> > If we are unable to allocate the SRAM needed to work around I688 this is
> > a real error on platforms that expose this erratum, so silently bailing
> > out at this point may obscure a real issue.
> > 
> 
> SRAM is not allocated here - It's just check to understand do we need it or not
> in case of multiplatform build where CONFIG_OMAP_INTERCONNECT_BARRIER will be selected most
> probably.
> 
> And if "ti,omap4-mpu" was not found - it just means that this, particular, platform
> is not affected by i688 errata.
> If someone misses corresponding node in DT - we can't do nothing :)
> 
Okay, so the above document says that AM43xx is affected by the erratum,
but the am4372.dtsi doesn't contain a "ti,omap4-mpu" node, so the
workaround will not be applied. If we silence the warning, we now have a
system that will be prone to data corruption without ever warning the
user about it. This is surely not what anyone wants.

Regards,
Lucas
-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |