Messages in this thread | | | Date | Fri, 22 Jun 2012 10:39:45 +0900 | From | jonghwa3.lee@samsung ... | Subject | Re: [PATCH] Exynos : Add support for Exynos random number generator |
| |
On 2012년 06월 21일 12:12, Stephen Boyd wrote:
> On 06/20/12 19:39, jonghwa3.lee@samsung.com wrote: >> >>>> +static int exynos_init(struct hwrng *rng) >>>> +{ >>>> + struct exynos_rng *exynos_rng = container_of(rng, >>>> + struct exynos_rng, rng); >>>> + int i; >>>> + int ret = 0; >>>> + u32 PRND_SEED[5]; >>>> + >>>> + pm_runtime_put_noidle(exynos_rng->dev); >>>> + pm_runtime_get_sync(exynos_rng->dev); >>> This looks very odd. Why are you calling pm_runtime_put_noidle()? >>> >> >> When this callback function is called, the status of power state is >> 'suspended' and use_count is 1. To perform pm_runtime_get_sync() >> correctly, it requires to have 'suspended' status and use_count is 0. >> Thus i force to decrease use_count only with using >> pm_runtime_put_noidle. I know it looks odd, but i couldn't find better >> way. Otherwise it can use clk_enable() directly, but i think that it >> isn't good neither. > > Is the device suspended initially at probe? If so can you set the state > of the device to suspended? >
Yes, it suspended already at the initial of probing. But i can't get your point, you want to me to make device suspended?
>> >>>> +{ >>>> + int ret; >>>> + struct exynos_rng *exynos_rng; >>>> + struct resource *res; >>>> + >>>> + exynos_rng = devm_kzalloc(&pdev->dev, sizeof(struct exynos_rng), >>>> + GFP_KERNEL); >>>> + if (!exynos_rng) >>>> + return -ENOMEM; >>>> + >>>> + exynos_rng->dev = &pdev->dev; >>>> + exynos_rng->rng.name = "exynos"; >>>> + exynos_rng->rng.init = exynos_init; >>>> + exynos_rng->rng.read = exynos_read; >>>> + exynos_rng->clk = clk_get(NULL, "secss"); >>> Can you please pass &pdev->dev to clk_get()? >>> >> >> But, this clock is not only used in PRNG. Should i put it in? > > Why would that matter? I'm not familiar with the samsung clock > implementation but I would assume that something like clkdev is being > used. Ideally you would have one lookup entry per struct device that > uses this clock and then tie each lookup entry to the same clock. This > allows you to support multiple instances of the same device in a generic > way (e.g. two rng devices that use different clocks but from the driver > perspective they have the same name 'secss'). >
Okay, I see. I'll apply it. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |