Re: [PATCH 1/7] ARM: OMAP2xxx: hwmod: Convert SHAM crypto device data to hwmod

On Sat, Oct 20, 2012 at 07:40:19PM +0000, Paul Walmsley wrote:

Hi

Hi Paul.

a few comments:

On Fri, 19 Oct 2012, Mark A. Greer wrote:

quoted

From: "Mark A. Greer" <mgreer@animalcreek.com>

Convert the device data for the OMAP2 SHAM crypto IP from
explicit platform_data to hwmod.  When bit 1 (OMAP24XX_ST_SHA_MASK)
of the CM_IDLEST4_CORE register is set, the SHA IP is present.

CC: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Mark A. Greer <mgreer@animalcreek.com>

...

quoted

diff --git a/arch/arm/mach-omap2/clock2420_data.c b/arch/arm/mach-omap2/clock2420_data.c
index c3cde1a..a09603c 100644
--- a/arch/arm/mach-omap2/clock2420_data.c
+++ b/arch/arm/mach-omap2/clock2420_data.c

@@ -1905,6 +1905,7 @@ static struct omap_clk omap2420_clks[] = {
 	CLK(NULL,	"vlynq_ick",	&vlynq_ick,	CK_242X),
 	CLK(NULL,	"vlynq_fck",	&vlynq_fck,	CK_242X),
 	CLK(NULL,	"des_ick",	&des_ick,	CK_242X),
+	CLK(NULL,	"sha_ick",	&sha_ick,	CK_242X),

This isn't needed; this alias already exists two lines below.

Umm, yeah.  Oops.  :)

quoted

 	CLK("omap-sham",	"ick",	&sha_ick,	CK_242X),
 	CLK(NULL,	"sha_ick",	&sha_ick,	CK_242X),
 	CLK("omap_rng",	"ick",		&rng_ick,	CK_242X),

quoted

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index c8c2117..5b613fa 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c

quoted

@@ -500,17 +484,27 @@ static struct platform_device sham_device = {
 
 static void omap_init_sham(void)
 {
-	if (cpu_is_omap24xx()) {
-		sham_device.resource = omap2_sham_resources;
-		sham_device.num_resources = omap2_sham_resources_sz;
+	if (cpu_is_omap24xx() &&
+	    (omap2_cm_read_mod_reg(CORE_MOD, OMAP24XX_CM_IDLEST4) &
+						 OMAP24XX_ST_SHA_MASK)) {

Hmm.  Not sure I understand the purpose of this CM read.  Don't we want to 
initialize this device unconditionally?

No, the device doesn't exist on all parts.  This is the only way
to tell if its there (AFAIK).

Also we're trying to get rid of all of the direct CM/PRM accesses outside 
of the prm*.c/cm*.c code.  If we need something like this, we should add 
some support for it in hwmod & CM code, since that's where the data lives.

Okay.

Mark
--