| Message ID | 1350683640-15044-2-git-send-email-mgreer@animalcreek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi a few comments: On Fri, 19 Oct 2012, Mark A. Greer wrote: > 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> ... > > 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. > CLK("omap-sham", "ick", &sha_ick, CK_242X), > CLK(NULL, "sha_ick", &sha_ick, CK_242X), > CLK("omap_rng", "ick", &rng_ick, CK_242X), > diff --git a/arch/arm/mach-omap2/clock2430_data.c b/arch/arm/mach-omap2/clock2430_data.c > index 22404fe..654e314 100644 > --- a/arch/arm/mach-omap2/clock2430_data.c > +++ b/arch/arm/mach-omap2/clock2430_data.c > @@ -1992,6 +1992,7 @@ static struct omap_clk omap2430_clks[] = { > CLK(NULL, "sdma_ick", &sdma_ick, CK_243X), > CLK(NULL, "sdrc_ick", &sdrc_ick, CK_243X), > CLK(NULL, "des_ick", &des_ick, CK_243X), > + CLK(NULL, "sha_ick", &sha_ick, CK_242X), > CLK("omap-sham", "ick", &sha_ick, CK_243X), > CLK("omap_rng", "ick", &rng_ick, CK_243X), > CLK(NULL, "rng_ick", &rng_ick, CK_243X), > 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 > @@ -34,6 +34,8 @@ > #include "mux.h" > #include "control.h" > #include "devices.h" > +#include "cm2xxx_3xxx.h" > +#include "cm-regbits-24xx.h" > > #define L3_MODULES_MAX_LEN 12 > #define L3_MODULES 3 > @@ -453,24 +455,6 @@ static void omap_init_rng(void) > > #if defined(CONFIG_CRYPTO_DEV_OMAP_SHAM) || defined(CONFIG_CRYPTO_DEV_OMAP_SHAM_MODULE) > > -#ifdef CONFIG_ARCH_OMAP2 > -static struct resource omap2_sham_resources[] = { > - { > - .start = OMAP24XX_SEC_SHA1MD5_BASE, > - .end = OMAP24XX_SEC_SHA1MD5_BASE + 0x64, > - .flags = IORESOURCE_MEM, > - }, > - { > - .start = 51 + OMAP_INTC_START, > - .flags = IORESOURCE_IRQ, > - } > -}; > -static int omap2_sham_resources_sz = ARRAY_SIZE(omap2_sham_resources); > -#else > -#define omap2_sham_resources NULL > -#define omap2_sham_resources_sz 0 > -#endif > - > #ifdef CONFIG_ARCH_OMAP3 > static struct resource omap3_sham_resources[] = { > { > @@ -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? 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. - Paul
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: > > > 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> > > ... > > > > > 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. :) > > CLK("omap-sham", "ick", &sha_ick, CK_242X), > > CLK(NULL, "sha_ick", &sha_ick, CK_242X), > > CLK("omap_rng", "ick", &rng_ick, CK_242X), > > 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 > > @@ -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 --
On Mon, 22 Oct 2012, Mark A. Greer wrote: > On Sat, Oct 20, 2012 at 07:40:19PM +0000, Paul Walmsley wrote: > > > > 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. It should exist on all OMAP2xxx, AFAIK. (Whether some bootloader firewalled it off is another matter, of course.) > This is the only way to tell if its there (AFAIK). Hmm. I don't think the CM_IDLEST bits work that way, in the general case. They just indicate whether the PRCM considers that IP block to be currently accessible. So for example if the clocks are disabled to an IP block, the CM_IDLEST bit would indicate that it's not accessible. The software could then enable the clocks, and the CM_IDLEST bit would indicate that it is accessible. We use this in the clock framework and hwmod code after enabling clocks to wait until the system considers the IP block accessible. See for example http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/mach-omap2/clock.c;h=961ac8f7e13d8c84a1cbb4587255ea685520bd18;hb=HEAD#l80 ... Now it's possible that on some TI chips, the CM_IDLEST bit is tied to the SIdleAck signal originating from the IP block. (I've been told that on other OMAPs, CM_IDLEST is actually tied to the SIdleReq signal originating from the PRCM, which is not terribly useful...) So if it's tied to the SIdleAck signal, and the PRCM clocks are enabled, then it might provide an indication of whether the IP block exists on that chip. But ultimately the IP block might still be firewalled off even if it appears to exist to the PRCM. So for 3xxx, I'd suggest just adding it to the appropriate GP-specific hwmod init lists, such as omap3xxx_gp_hwmod_ocp_ifs. - Paul
On Mon, Oct 22, 2012 at 07:49:47PM +0000, Paul Walmsley wrote: > On Mon, 22 Oct 2012, Mark A. Greer wrote: > > > On Sat, Oct 20, 2012 at 07:40:19PM +0000, Paul Walmsley wrote: > > > > > > 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. > > It should exist on all OMAP2xxx, AFAIK. (Whether some bootloader > firewalled it off is another matter, of course.) > > > This is the only way to tell if its there (AFAIK). > > Hmm. I don't think the CM_IDLEST bits work that way, in the general case. > They just indicate whether the PRCM considers that IP block to be > currently accessible. So for example if the clocks are disabled to an IP > block, the CM_IDLEST bit would indicate that it's not accessible. The > software could then enable the clocks, and the CM_IDLEST bit would > indicate that it is accessible. We use this in the clock framework and > hwmod code after enabling clocks to wait until the system considers the IP > block accessible. See for example > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/mach-omap2/clock.c;h=961ac8f7e13d8c84a1cbb4587255ea685520bd18;hb=HEAD#l80 > > ... > > Now it's possible that on some TI chips, the CM_IDLEST bit is tied to the > SIdleAck signal originating from the IP block. (I've been told that on > other OMAPs, CM_IDLEST is actually tied to the SIdleReq signal originating > from the PRCM, which is not terribly useful...) So if it's tied to the > SIdleAck signal, and the PRCM clocks are enabled, then it might provide an > indication of whether the IP block exists on that chip. But ultimately > the IP block might still be firewalled off even if it appears to exist to > the PRCM. So for 3xxx, I'd suggest just adding it to the appropriate > GP-specific hwmod init lists, such as omap3xxx_gp_hwmod_ocp_ifs. Paul, thank you for the clear explanation. I've talked to some hardware guys and I have come to the following conclusions: a) What you say above about CM_IDLEST is correct. b) There seems to be no good way, in general, to tell if the SHA (or AES or RNG or ...) module exists. c) The best thing to do is what you suggest and add the SHA hwmod data for all omap2's and omap3 GP's. I will respin my patches with everyone's comments addressed later today. Thanks everyone. Mark --
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), CLK("omap-sham", "ick", &sha_ick, CK_242X), CLK(NULL, "sha_ick", &sha_ick, CK_242X), CLK("omap_rng", "ick", &rng_ick, CK_242X), diff --git a/arch/arm/mach-omap2/clock2430_data.c b/arch/arm/mach-omap2/clock2430_data.c index 22404fe..654e314 100644 --- a/arch/arm/mach-omap2/clock2430_data.c +++ b/arch/arm/mach-omap2/clock2430_data.c @@ -1992,6 +1992,7 @@ static struct omap_clk omap2430_clks[] = { CLK(NULL, "sdma_ick", &sdma_ick, CK_243X), CLK(NULL, "sdrc_ick", &sdrc_ick, CK_243X), CLK(NULL, "des_ick", &des_ick, CK_243X), + CLK(NULL, "sha_ick", &sha_ick, CK_242X), CLK("omap-sham", "ick", &sha_ick, CK_243X), CLK("omap_rng", "ick", &rng_ick, CK_243X), CLK(NULL, "rng_ick", &rng_ick, CK_243X), 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 @@ -34,6 +34,8 @@ #include "mux.h" #include "control.h" #include "devices.h" +#include "cm2xxx_3xxx.h" +#include "cm-regbits-24xx.h" #define L3_MODULES_MAX_LEN 12 #define L3_MODULES 3 @@ -453,24 +455,6 @@ static void omap_init_rng(void) #if defined(CONFIG_CRYPTO_DEV_OMAP_SHAM) || defined(CONFIG_CRYPTO_DEV_OMAP_SHAM_MODULE) -#ifdef CONFIG_ARCH_OMAP2 -static struct resource omap2_sham_resources[] = { - { - .start = OMAP24XX_SEC_SHA1MD5_BASE, - .end = OMAP24XX_SEC_SHA1MD5_BASE + 0x64, - .flags = IORESOURCE_MEM, - }, - { - .start = 51 + OMAP_INTC_START, - .flags = IORESOURCE_IRQ, - } -}; -static int omap2_sham_resources_sz = ARRAY_SIZE(omap2_sham_resources); -#else -#define omap2_sham_resources NULL -#define omap2_sham_resources_sz 0 -#endif - #ifdef CONFIG_ARCH_OMAP3 static struct resource omap3_sham_resources[] = { { @@ -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)) { + struct omap_hwmod *oh; + struct platform_device *pdev; + + oh = omap_hwmod_lookup("sham"); + if (!oh) + return; + + pdev = omap_device_build("omap-sham", -1, oh, NULL, 0, NULL, + 0, 0); + WARN(IS_ERR(pdev), "Can't build omap_device for omap-sham\n"); } else if (cpu_is_omap34xx()) { sham_device.resource = omap3_sham_resources; sham_device.num_resources = omap3_sham_resources_sz; + platform_device_register(&sham_device); } else { pr_err("%s: platform not supported\n", __func__); return; } - platform_device_register(&sham_device); } #else static inline void omap_init_sham(void) { } diff --git a/arch/arm/mach-omap2/omap_hwmod_2420_data.c b/arch/arm/mach-omap2/omap_hwmod_2420_data.c index b5db600..b102a53 100644 --- a/arch/arm/mach-omap2/omap_hwmod_2420_data.c +++ b/arch/arm/mach-omap2/omap_hwmod_2420_data.c @@ -603,6 +603,7 @@ static struct omap_hwmod_ocp_if *omap2420_hwmod_ocp_ifs[] __initdata = { &omap2420_l4_core__mcbsp2, &omap2420_l4_core__msdi1, &omap2xxx_l4_core__rng, + &omap2xxx_l4_core__sham, &omap2420_l4_core__hdq1w, &omap2420_l4_wkup__counter_32k, &omap2420_l3__gpmc, diff --git a/arch/arm/mach-omap2/omap_hwmod_2430_data.c b/arch/arm/mach-omap2/omap_hwmod_2430_data.c index c455e41..b1ce7b0 100644 --- a/arch/arm/mach-omap2/omap_hwmod_2430_data.c +++ b/arch/arm/mach-omap2/omap_hwmod_2430_data.c @@ -963,6 +963,7 @@ static struct omap_hwmod_ocp_if *omap2430_hwmod_ocp_ifs[] __initdata = { &omap2430_l4_core__mcbsp5, &omap2430_l4_core__hdq1w, &omap2xxx_l4_core__rng, + &omap2xxx_l4_core__sham, &omap2430_l4_wkup__counter_32k, &omap2430_l3__gpmc, NULL, diff --git a/arch/arm/mach-omap2/omap_hwmod_2xxx_interconnect_data.c b/arch/arm/mach-omap2/omap_hwmod_2xxx_interconnect_data.c index 1a1287d..bb314c5 100644 --- a/arch/arm/mach-omap2/omap_hwmod_2xxx_interconnect_data.c +++ b/arch/arm/mach-omap2/omap_hwmod_2xxx_interconnect_data.c @@ -138,6 +138,15 @@ static struct omap_hwmod_addr_space omap2_rng_addr_space[] = { { } }; +struct omap_hwmod_addr_space omap2xxx_sham_addrs[] = { + { + .pa_start = 0x480a4000, + .pa_end = 0x480a4000 + 0x64 - 1, + .flags = ADDR_TYPE_RT + }, + { } +}; + /* * Common interconnect data */ @@ -389,3 +398,12 @@ struct omap_hwmod_ocp_if omap2xxx_l4_core__rng = { .addr = omap2_rng_addr_space, .user = OCP_USER_MPU | OCP_USER_SDMA, }; + +/* l4 core -> sham interface */ +struct omap_hwmod_ocp_if omap2xxx_l4_core__sham = { + .master = &omap2xxx_l4_core_hwmod, + .slave = &omap2xxx_sham_hwmod, + .clk = "sha_ick", + .addr = omap2xxx_sham_addrs, + .user = OCP_USER_MPU, +}; diff --git a/arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c b/arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c index bd9220e..a041670 100644 --- a/arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c +++ b/arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c @@ -851,3 +851,40 @@ struct omap_hwmod omap2xxx_rng_hwmod = { .flags = HWMOD_INIT_NO_RESET, .class = &omap2_rng_hwmod_class, }; + +/* SHAM */ + +static struct omap_hwmod_class_sysconfig omap2_sham_sysc = { + .rev_offs = 0x5c, + .sysc_offs = 0x60, + .syss_offs = 0x64, + .sysc_flags = (SYSC_HAS_SOFTRESET | SYSC_HAS_AUTOIDLE | + SYSS_HAS_RESET_STATUS), + .sysc_fields = &omap_hwmod_sysc_type1, +}; + +static struct omap_hwmod_class omap2xxx_sham_class = { + .name = "sham", + .sysc = &omap2_sham_sysc, +}; + +struct omap_hwmod_irq_info omap2_sham_mpu_irqs[] = { + { .irq = 51 + OMAP_INTC_START, }, + { .irq = -1 } +}; + +struct omap_hwmod omap2xxx_sham_hwmod = { + .name = "sham", + .mpu_irqs = omap2_sham_mpu_irqs, + .main_clk = "l4_ck", + .prcm = { + .omap2 = { + .module_offs = CORE_MOD, + .prcm_reg_id = 4, + .module_bit = OMAP24XX_EN_SHA_SHIFT, + .idlest_reg_id = 4, + .idlest_idle_bit = OMAP24XX_ST_SHA_SHIFT, + }, + }, + .class = &omap2xxx_sham_class, +}; diff --git a/arch/arm/mach-omap2/omap_hwmod_common_data.h b/arch/arm/mach-omap2/omap_hwmod_common_data.h index 2bc8f17..74a7b7a 100644 --- a/arch/arm/mach-omap2/omap_hwmod_common_data.h +++ b/arch/arm/mach-omap2/omap_hwmod_common_data.h @@ -78,6 +78,7 @@ extern struct omap_hwmod omap2xxx_mcspi2_hwmod; extern struct omap_hwmod omap2xxx_counter_32k_hwmod; extern struct omap_hwmod omap2xxx_gpmc_hwmod; extern struct omap_hwmod omap2xxx_rng_hwmod; +extern struct omap_hwmod omap2xxx_sham_hwmod; /* Common interface data across OMAP2xxx */ extern struct omap_hwmod_ocp_if omap2xxx_l3_main__l4_core; @@ -105,6 +106,7 @@ extern struct omap_hwmod_ocp_if omap2xxx_l4_core__dss_dispc; extern struct omap_hwmod_ocp_if omap2xxx_l4_core__dss_rfbi; extern struct omap_hwmod_ocp_if omap2xxx_l4_core__dss_venc; extern struct omap_hwmod_ocp_if omap2xxx_l4_core__rng; +extern struct omap_hwmod_ocp_if omap2xxx_l4_core__sham; /* Common IP block data */ extern struct omap_hwmod_dma_info omap2_uart1_sdma_reqs[];