On 5 October 2015 at 10:22, Daniel Thompson <daniel.thompson@linaro.org> wrote:
> On 4 October 2015 at 11:32, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Sat, Oct 3, 2015 at 10:35 PM, Daniel Thompson
>> <daniel.thompson@linaro.org> wrote:
>> Then this construct:
>>
>>> +static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
>>> +{
>> (...)
>>> + /* enable random number generation */
>>> + clk_enable(priv->clk);
>>> + cr = readl(priv->base + RNG_CR);
>>> + writel(cr | RNG_CR_RNGEN, priv->base + RNG_CR);
>> (...)
>>> + /* disable the generator */
>>> + writel(cr, priv->base + RNG_CR);
>>> + clk_disable(priv->clk);
>>
>> This is in the hotpath where megabytes of data may be read out
>> by consecutive reads. I think it's wise to move the clock enable/disable
>> and also the write to cr to enable/disable the block to runtime PM
>> hooks, so that you can e.g. set some hysteresis around the disablement
>> with pm_runtime_set_autosuspend_delay(&dev->dev, 50);
>> pm_runtime_use_autosuspend(&dev->dev);pm_runtime_put_autosuspend().
>> For a primecell check the usage in drivers/mmc/host/mmci.c
>> for a complex usecase or e.g. drivers/hwtracing/coresight/coresight-tpiu.c
>> for a simpler usecase.
>>
>> For the performance hints I guess you can even test whether
>> what I'm saying makes sense or not by reading some megabytes
>> of random and timing it.
>
> I'll have to benchmark this. clk_enable/disable have pretty simple
> implementations on STM32 so there might not be much in it.

Well... I was extremely wrong about that!

Switching the driver over to autosuspend yielded a very significant
performance boost: ~1.1MiB/s to ~1.5MiB/s .

Very pleased with that!


Daniel.