Am 21.02.2017 um 22:21 schrieb Neil Armstrong:
> On 02/21/2017 12:26 PM, Heiner Kallweit wrote:
>> Add handling of RNG0 clock to the driver.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
>> ---
>> v2:
>> - consider that clock is optional
>> - use managed call to clk_disable_unprepare to ensure that
>> cleaning up is done in the right order if driver is removed
>> ---
>> drivers/char/hw_random/meson-rng.c | 22 ++++++++++++++++++++++
>> 1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/char/hw_random/meson-rng.c b/drivers/char/hw_random/meson-rng.c
>> index 119d6984..2e23be80 100644
>> --- a/drivers/char/hw_random/meson-rng.c
>> +++ b/drivers/char/hw_random/meson-rng.c
>> @@ -62,6 +62,7 @@
>> #include <linux/slab.h>
>> #include <linux/types.h>
>> #include <linux/of.h>
>> +#include <linux/clk.h>
>>
>> #define RNG_DATA 0x00
>>
>> @@ -69,6 +70,7 @@ struct meson_rng_data {
>> void __iomem *base;
>> struct platform_device *pdev;
>> struct hwrng rng;
>> + struct clk *core_clk;
>> };
>>
>> static int meson_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
>> @@ -81,11 +83,17 @@ static int meson_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
>> return sizeof(u32);
>> }
>>
>> +static void meson_rng_clk_disable(void *clk)
>> +{
>> + clk_disable_unprepare(clk);
>> +}
>> +
>> static int meson_rng_probe(struct platform_device *pdev)
>> {
>> struct device *dev = &pdev->dev;
>> struct meson_rng_data *data;
>> struct resource *res;
>> + int ret;
>>
>> data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>> if (!data)
>> @@ -98,6 +106,20 @@ static int meson_rng_probe(struct platform_device *pdev)
>> if (IS_ERR(data->base))
>> return PTR_ERR(data->base);
>>
>> + data->core_clk = devm_clk_get(dev, "core");
>> + if (IS_ERR(data->core_clk))
>> + data->core_clk = NULL;
>> +
>> + if (data->core_clk) {
>
> This "if" is useless, if core_clk is NULL, this will work also.
>
Regarding clk_prepare_enable you're right. However we need this "if" for
the call to devm_add_action_or_reset because we can't provide a NULL
pointer as data argument.
And including clk_prepare_enable in the if clause creates no overhead
and is clearer IMHO.
>> + ret = clk_prepare_enable(data->core_clk);
>> + if (ret)
>> + return ret;
>> + ret = devm_add_action_or_reset(dev, meson_rng_clk_disable,
>> + data->core_clk);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> data->rng.name = pdev->name;
>> data->rng.read = meson_rng_read;
>>
>>
>
>
