Hi Pali,
Yep, the code looks almost identical, I guess with some tweaks all SMC code from that patch could be removed and instead used the one from SMC PPA API.
What I don't get, is why one needs to disable/enable fiqs/irqs:
+static int call_sec_rom(u32 appl_id, u32 proc_id, u32 flag, ...)
+{
+ va_list ap;
+ u32 ret;
+ u32 val;
+
+ va_start(ap, flag);
+ val = *(u32 *) ≈
+ local_irq_disable();
+ local_fiq_disable();
+ ret = omap3_rom_rng_call(appl_id, proc_id, flag,
+ (u32) virt_to_phys((void *) val));
+ local_fiq_enable();
+ local_irq_enable();
+ va_end(ap);
+
+ return ret;
+}
Do you have any idea why is that needed? Any other code I've ever seen to call SM, does not disable fiqs/irqs, is RNG SMC somehow special? I know this is Nokia's code, but still, if we can get some understanding...
Regards,
Ivo
>-------- Оригинално писмо --------
>От: Pali Rohár
>Относно: Re: [PATCH] OMAP3 ROM Random Number Generator support
>До: Tony Lindgren
>Изпратено на: Неделя, 2013, Март 31 12:28:12 EEST
>
>
>On Thursday 28 March 2013 22:44:11 Tony Lindgren wrote:
>> * Pali Rohár [130328 10:58]:
>> > Here is new version of patch:
>> >
>> > --- a/arch/arm/mach-omap2/devices.c
>> > +++ b/arch/arm/mach-omap2/devices.c
>> > @@ -486,6 +486,23 @@ static void omap_init_mcspi(void)
>> >
>> > static inline void omap_init_mcspi(void) {}
>> > #endif
>> >
>> > +extern u32 *omap3_rom_rng_call(u32 id, u32 proc, u32 flags,
>> > u32 va_ptr); +
>> > +static struct platform_device omap3_rom_rng_device = {
>> > + .name = "omap3-rom-rng",
>> > + .id = -1,
>> > + .dev = {
>> > + .platform_data = omap3_rom_rng_call,
>> > + },
>> > +};
>> > +
>> > +static void omap_init_rom_rng(void)
>> > +{
>> > + if (!cpu_is_omap34xx() || omap_type() ==
>> > OMAP2_DEVICE_TYPE_GP) + return;
>> > + platform_device_register(&omap3_rom_rng_device);
>> > +}
>> > +
>> >
>> > /**
>> >
>> > * omap_init_rng - bind the RNG hwmod to the RNG
>> > omap_device *
>>
>> This driver probably only works on Nokia boards because of the
>> different SMC call numbering. Until it's been verified on some
>> other HS omap34xx, I'd probably register this only from the
>> Nokia board-*.c file.
>>
>
>Freemangordon, can you look at this smc and errara 430973 code if
>they could be merged? Really omap3_rom_rng_call function looks
>like n900 specific.
>
>Link: https://lkml.org/lkml/2013/3/28/398
>
>> > --- /dev/null
>> > +++ b/drivers/char/hw_random/omap3-rom-rng.c
>> > +static int omap3_rom_rng_probe(struct platform_device
>> > *pdev) +{
>> > + printk(KERN_INFO "%s: initializing\n",
>> > omap3_rom_rng_name); +
>> > + omap3_rom_rng_call = pdev->dev.platform_data;
>> > + if (!omap3_rom_rng_call) {
>> > + printk(KERN_ERR "%s: omap3_rom_rng_call is NULL\n",
>> > + omap3_rom_rng_name);
>> > + return -EINVAL;
>> > + }
>> > +
>> > + setup_timer(&idle_timer, omap3_rom_idle_rng, 0);
>> > + rng_clk = clk_get_sys("omap_rng", "ick");
>> > + if (IS_ERR(rng_clk)) {
>> > + printk(KERN_ERR "%s: unable to get RNG clock\n",
>> > + omap3_rom_rng_name);
>> > + return IS_ERR(rng_clk);
>> > + }
>>
>> You can use regular clk_get if you add the alias to
>> struct omap_clk omap3xxx_clks table.
>>
>> Regards,
>>
>> Tony
>
>Tony, can you show me how?
>
>--
>Pali Rohár
>[email protected]
>
On Sunday 31 March 2013 14:05:42 Ивайло Димитров wrote:
> Hi Pali,
>
> Yep, the code looks almost identical, I guess with some tweaks
> all SMC code from that patch could be removed and instead
> used the one from SMC PPA API.
>
> What I don't get, is why one needs to disable/enable
> fiqs/irqs:
>
> +static int call_sec_rom(u32 appl_id, u32 proc_id, u32 flag,
> ...) +{
> + va_list ap;
> + u32 ret;
> + u32 val;
> +
> + va_start(ap, flag);
> + val = *(u32 *) ≈
> + local_irq_disable();
> + local_fiq_disable();
> + ret = omap3_rom_rng_call(appl_id, proc_id, flag,
> + (u32) virt_to_phys((void *) val));
> + local_fiq_enable();
> + local_irq_enable();
> + va_end(ap);
> +
> + return ret;
> +}
>
> Do you have any idea why is that needed? Any other code I've
> ever seen to call SM, does not disable fiqs/irqs, is RNG SMC
> somehow special? I know this is Nokia's code, but still, if
> we can get some understanding...
>
> Regards,
> Ivo
This driver is written by Juha Yrjola and is part of maemo 2.6.28 kernel.
Juha Yrjola, are you still around?
--
Pali Rohár
[email protected]
* Ивайло Димитров <[email protected]> [130331 05:10]:
>
> Hi Pali,
>
> Yep, the code looks almost identical, I guess with some tweaks all SMC code from that patch could be removed and instead used the one from SMC PPA API.
>
> What I don't get, is why one needs to disable/enable fiqs/irqs:
>
> +static int call_sec_rom(u32 appl_id, u32 proc_id, u32 flag, ...)
> +{
> + va_list ap;
> + u32 ret;
> + u32 val;
> +
> + va_start(ap, flag);
> + val = *(u32 *) ≈
> + local_irq_disable();
> + local_fiq_disable();
> + ret = omap3_rom_rng_call(appl_id, proc_id, flag,
> + (u32) virt_to_phys((void *) val));
> + local_fiq_enable();
> + local_irq_enable();
> + va_end(ap);
> +
> + return ret;
> +}
>
> Do you have any idea why is that needed? Any other code I've ever seen to call SM, does not disable fiqs/irqs, is RNG SMC somehow special? I know this is Nokia's code, but still, if we can get some understanding...
Sounds like the fiq handling can be dropped based on what
the TI guys posted recently about FIQs only be available in
the secure mode starting with omap2.
Regards,
Tony
Hi,
On Sun, Mar 31, 2013 at 03:05:42PM +0300, Ивайло Димитров wrote:
> Yep, the code looks almost identical, I guess with some tweaks all SMC
> code from that patch could be removed and instead used the one from SMC
> PPA API.
>
> What I don't get, is why one needs to disable/enable fiqs/irqs:
[..]
> Do you have any idea why is that needed? Any other code I've ever seen
> to call SM, does not disable fiqs/irqs, is RNG SMC somehow special? I
> know this is Nokia's code, but still, if we can get some understanding...
I think without disabling irqs you may suffer some strange crashes when
heavily stressing the RNG... that's why you also have cache flushes before
and after the SMC. With proper test suite you could probably verify this.
How are you testing this driver?
A.