LinuxLists.cc - Re: [PATCH] OMAP3 ROM Random Number Generator support

2013-03-27 21:09:28

Subject: Re: [PATCH] OMAP3 ROM Random Number Generator support

* Pali Rohár <[email protected]> [130228 10:09]:
> --- /dev/null
> +++ b/drivers/char/hw_random/omap3-rom-asm.S
> @@ -0,0 +1,27 @@
> +/*
> + * Copyright (C) 2009 Nokia Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/linkage.h>
> +#include <asm/assembler.h>
> +
> +ENTRY(omap3_rng_call_rom_asm)
> + .arch_extension sec
> + stmfd sp!, {r4-r12, lr}
> + stmfd sp!, {r0-r3}
> + bl v7_flush_dcache_all
> + ldmfd sp!, {r0-r3}
> + mov r6, #0xff
> + mov r12, r0
> + smc #1
> + mov r12, r0
> + bl v7_flush_dcache_all
> + mov r0, #0
> + mcr p15, 0, r0, c7, c5, 0
> + mov r0, r12
> + ldmfd sp!, {r4-r12, pc}

This part should be in arch/arm/mach-omap2 because of the
direct calls to v7_flush_dcache_all. Then you can pass a pointer
to this function in platform_data for now, and set it up based
on the compatible flag with device tree from driver later on.

> --- /dev/null
> +++ b/drivers/char/hw_random/omap3-rom-drv.c
> @@ -0,0 +1,167 @@
> +/*
> + * omap3-rom-drv.c - RNG driver for TI OMAP3 CPU family
> + *
> + * Copyright (C) 2009 Nokia Corporation
> + * Author: Juha Yrjola <[email protected]>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/random.h>
> +#include <linux/hw_random.h>
> +#include <linux/timer.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +
> +#include "../../../arch/arm/mach-omap2/soc.h"

NAK for this, this must be private to arch/arm/mach-omap2.

Instead, just create a minimal include/linux/platform_data/omap3-rom.h,
and pass the function pointer in the pdata if passed.

> +static int omap3_rom_rng_probe(struct platform_device *pdev)
> +{
> + printk(KERN_INFO "%s: initializing\n", omap3_rom_rng_name);
> + if (!cpu_is_omap34xx()) {
> + printk(KERN_ERR "%s: currently supports only OMAP34xx CPUs\n",
> + omap3_rom_rng_name);
> + return -ENODEV;
> + }
> + if (omap_type() == OMAP2_DEVICE_TYPE_GP) {
> + printk(KERN_ERR "%s: GP OMAPs not supported\n",
> + omap3_rom_rng_name);
> + return -ENODEV;
> + }
> +
> + 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);
> + }
> +
> + /* Leave the RNG in reset state. */
> + clk_prepare_enable(rng_clk);
> + omap3_rom_idle_rng(0);
> +
> + return hwrng_register(&omap3_rom_rng_ops);
> +}

You should replace the cpu_is_omap34xx() and omap_type() with
some flags that you pass in the pdata. These too can be then
deciphered based on the DT compatible flag later on. And then
you don't need to include soc.h.

Regards,

Tony

2013-03-27 22:03:50

On Thursday 28 March 2013 10:52:28 Russell King - ARM Linux
wrote:
> On Wed, Mar 27, 2013 at 02:09:18PM -0700, Tony Lindgren wrote:
> > > + setup_timer(&idle_timer, omap3_rom_idle_rng, 0);
> > > + rng_clk = clk_get_sys("omap_rng", "ick");
>
> Why is this using clk_get_sys when it should be using
> clk_get() with the platform device?

Funcion clk_get() needs struct device* instead clk device name.
And I do not know how to get that pointer to device. With
clk_get_sys driver working.

--
Pali Rohár
[email protected]

Attachments:

signature.asc (198.00 B)
This is a digitally signed message part.

2013-03-28 17:54:09

On Friday 20 September 2013 18:23:43 Tony Lindgren wrote:
> * Pali Rohár <[email protected]> [130920 06:33]:
> > Name omap_rng is already used by omap_rng driver which is
> > for omap2 and omap4. For omap3 HS devices there is new
> > driver omap3-rom-rng which needs that clk. And because
> > there cannot be two drivers with same name, rename omap3
> > clk dev id.
> >
> > Signed-off-by: Pali Rohár <[email protected]>
> > ---
> >
> > arch/arm/mach-omap2/cclock3xxx_data.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/mach-omap2/cclock3xxx_data.c
> > b/arch/arm/mach-omap2/cclock3xxx_data.c index
> > 334b767..5059338 100644
> > --- a/arch/arm/mach-omap2/cclock3xxx_data.c
> > +++ b/arch/arm/mach-omap2/cclock3xxx_data.c
> > @@ -3274,7 +3274,7 @@ static struct omap_clk omap36xx_clks[]
> > = {
> >
> > */
> >
> > static struct omap_clk omap34xx_omap36xx_clks[] = {
> >
> > CLK(NULL, "aes1_ick", &aes1_ick),
> >
> > - CLK("omap_rng", "ick", &rng_ick),
> > + CLK("omap3-rom-rng", "ick", &rng_ick),
> >
> > CLK(NULL, "sha11_ick", &sha11_ick),
> > CLK(NULL, "des1_ick", &des1_ick),
> > CLK(NULL, "cam_mclk", &cam_mclk),
>
> Hmm are you sure the omap_rng is not available on
> any of the omap3 GP versions? If so, the omap_rng
> alias should be kept as well.
>
> Regards,
>
> Tony

I really do not know, but driver char/hw_random/omap-rng.c
support only omap2 and omap4 platforms. So "omap_rng" dev id for
omap3 is not used.

In email [1] you suggested to use clk_get instead clk_get_sys. If
I understand correctly clk_get calling clk_get_sys with driver
name. So dev in in CLK macro must be same as driver name. And
because in cclock3xxx_data.c is dev id "omap_rng" and there is
already driver with name "omap_rng", I need to rename dev in
cclock3xxx_data.c to get new omap3-rom-rng driver working.

Or if you know better way how to do it, let me know.

[1] - https://lkml.org/lkml/2013/4/1/278

--
Pali Rohár
[email protected]

Attachments:

signature.asc (198.00 B)
This is a digitally signed message part.

2013-09-20 17:46:52

by Tony Lindgren

[permalink] [raw]

Subject: Re: [PATCH v2 1/3] omap2: Change clk dev id for rng to omap3-rom-rng

* Pali Rohár <[email protected]> [130920 09:49]:
> On Friday 20 September 2013 18:23:43 Tony Lindgren wrote:
> > * Pali Rohár <[email protected]> [130920 06:33]:
> > > Name omap_rng is already used by omap_rng driver which is
> > > for omap2 and omap4. For omap3 HS devices there is new
> > > driver omap3-rom-rng which needs that clk. And because
> > > there cannot be two drivers with same name, rename omap3
> > > clk dev id.
> > >
> > > Signed-off-by: Pali Rohár <[email protected]>
> > > ---
> > >
> > > arch/arm/mach-omap2/cclock3xxx_data.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm/mach-omap2/cclock3xxx_data.c
> > > b/arch/arm/mach-omap2/cclock3xxx_data.c index
> > > 334b767..5059338 100644
> > > --- a/arch/arm/mach-omap2/cclock3xxx_data.c
> > > +++ b/arch/arm/mach-omap2/cclock3xxx_data.c
> > > @@ -3274,7 +3274,7 @@ static struct omap_clk omap36xx_clks[]
> > > = {
> > >
> > > */
> > >
> > > static struct omap_clk omap34xx_omap36xx_clks[] = {
> > >
> > > CLK(NULL, "aes1_ick", &aes1_ick),
> > >
> > > - CLK("omap_rng", "ick", &rng_ick),
> > > + CLK("omap3-rom-rng", "ick", &rng_ick),
> > >
> > > CLK(NULL, "sha11_ick", &sha11_ick),
> > > CLK(NULL, "des1_ick", &des1_ick),
> > > CLK(NULL, "cam_mclk", &cam_mclk),
> >
> > Hmm are you sure the omap_rng is not available on
> > any of the omap3 GP versions? If so, the omap_rng
> > alias should be kept as well.
> >
> > Regards,
> >
> > Tony
>
> I really do not know, but driver char/hw_random/omap-rng.c
> support only omap2 and omap4 platforms. So "omap_rng" dev id for
> omap3 is not used.
>
> In email [1] you suggested to use clk_get instead clk_get_sys. If
> I understand correctly clk_get calling clk_get_sys with driver
> name. So dev in in CLK macro must be same as driver name. And
> because in cclock3xxx_data.c is dev id "omap_rng" and there is
> already driver with name "omap_rng", I need to rename dev in
> cclock3xxx_data.c to get new omap3-rom-rng driver working.
>
> Or if you know better way how to do it, let me know.

I think it's safest to keep the existing alias also, and
add a comment saying that it may not be available on omap3
processors.

Regards,

Tony

> [1] - https://lkml.org/lkml/2013/4/1/278
>
> --
> Pali Rohár
> [email protected]

2013-09-20 18:24:57

On Tue, Oct 08, 2013 at 12:04:09PM -0700, Tony Lindgren wrote:
> * Pali Roh?r <[email protected]> [130920 06:33]:
> > This driver provides kernel-side support for the Random Number
> > Generator hardware found on OMAP34xx processors.
> >
> > This driver comes from Maemo 2.6.28 kernel and was tested on Nokia RX-51.
> > It is platform device because it needs board specific function for smc calls.
>
> This one is should be merged via the hw_random patches seprately:
>
> Acked-by: Tony Lindgren <[email protected]>

Patch applied. Thanks!
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt