Hi Mark,
I regret the delay,
On Tue, 8 Jan 2013, Mark A. Greer wrote:
> On Sun, Dec 23, 2012 at 08:40:43AM +0000, Paul Walmsley wrote:
>
> > - The patch series causes AM3517/3505 to crash. I'd guess this is due to
> > the SHAM/AES modules being initialized on those chips, but they probably
> > don't exist there. Can you change the initialization for those on OMAP3
> > to only take place on OMAP34xx/36xx GP? I guess you'd need to create new
> > lists for those in the hwmod init.
>
> All am35xx GPs have the SHAM and AES modules except some very old ones.
> I've been told that there should be very few of the "old" ones around
> (I don't know how to differentiate them). We're likely safe since the
> SHAM & AES modules are not enabled in omap2plus_defconfig so nobody
> should be enabling them on an am35xx unless they know that they have
> the modules. Do you agree?
Those will presumably only enable or disable the device drivers. The
hwmod code will probably still try to write to those IP blocks if they are
listed as present in the hwmod data, during the initial reset-and-idle
phase.
What do you think about adding an am35xx_es11plus_hwmod_ocp_ifs[] array to
omap_hwmod_3xxx_data.c for these secure hwmods? That carries the implicit
and possibly wrong assumption that it's likely to be ES1.0 devices that
are missing the SHAM/AES, but it seems unlikely that TI would have
multiple silicon revs running around claiming to be ES1.1? Or maybe I'm
just being naïve.
> The issue that you're likely running into is that 'CK_AM35XX' needs to be
> added for aes2_ick & sha12_ick in cclock3xxx_data.c. The following
> patch should fix it (applies to my submitted/crypto/hwmod branch):
>
> diff --git a/arch/arm/mach-omap2/cclock3xxx_data.c b/arch/arm/mach-omap2/cclock3xxx_data.c
> index 582b055..aa5bdf6 100644
> --- a/arch/arm/mach-omap2/cclock3xxx_data.c
> +++ b/arch/arm/mach-omap2/cclock3xxx_data.c
> @@ -3332,10 +3332,10 @@ static struct omap_clk omap3xxx_clks[] = {
> CLK("omap_hsmmc.2", "ick", &mmchs3_ick, CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
> CLK(NULL, "mmchs3_ick", &mmchs3_ick, CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
> CLK(NULL, "icr_ick", &icr_ick, CK_34XX | CK_36XX),
> - CLK("omap-aes", "ick", &aes2_ick, CK_34XX | CK_36XX),
> - CLK(NULL, "aes2_ick", &aes2_ick, CK_34XX | CK_36XX),
> - CLK("omap-sham", "ick", &sha12_ick, CK_34XX | CK_36XX),
> - CLK(NULL, "sha12_ick", &sha12_ick, CK_34XX | CK_36XX),
> + CLK("omap-aes", "ick", &aes2_ick, CK_34XX | CK_AM35XX | CK_36XX),
> + CLK(NULL, "aes2_ick", &aes2_ick, CK_34XX | CK_AM35XX | CK_36XX),
> + CLK("omap-sham", "ick", &sha12_ick, CK_34XX | CK_AM35XX | CK_36XX),
> + CLK(NULL, "sha12_ick", &sha12_ick, CK_34XX | CK_AM35XX | CK_36XX),
> CLK(NULL, "des2_ick", &des2_ick, CK_34XX | CK_36XX),
> CLK("omap_hsmmc.1", "ick", &mmchs2_ick, CK_3XXX),
> CLK("omap_hsmmc.0", "ick", &mmchs1_ick, CK_3XXX),
>
>
> Please let me know if this patch works for you and, if it does, I'll respin
> my patches to add those changes.
If those clocks are referenced by the hwmods, that that patch makes sense
to me. Haven't had the chance to test it yet but maybe tomorrow. On the
other hand it looks 'obviously correct' so maybe just add that change to
your patches and repost that one?
- Paul
