Subject: Re: MPC8349ea Random Device Generator driver

From: Arnd Bergmann <[email-masked]>
Date: Wed, 6 Jun 2007 16:15:10 +0200

On Tuesday 05 June 2007, Philippe Lachenal wrote:
> I've made a driver for the MPC8349ea Random Device Generator, and I 
> therefore submit it to your impartial judgment.. ;)

Looks good from the point of integration into the driver framework.
I can't judge the hwrng specific parts, but don't see anything
fundamentally wrong there.

> +static int __init mpc834x_itx_declare_of_platform_devices(void)
> +{
> +       if (!machine_is(mpc834x_itx)){
> +               printk("__init mpc834x_itx_declare_of_platform_devices error\n");
> +               return 0;
> +       }

I would think that it's not strictly an error to be running on some other
hardware than yours ;-)

Just remove the printk here.

> +       err = of_address_to_resource(rng_np, 0, &res);
> +       if (err)
> +               return -ENODEV; 
> +       rng_regs = ioremap(res.start, (res.end - res.start));

This can be done in one step with the new 'of_iomap' function.

> +       printk(KERN_INFO "Registering Talitos RNG\n");
> +
> +       err = hwrng_register(&talitos_rng);
> +       if (err){
> +               printk(".............. failure\n");
> +               iounmap(rng_regs);
> +       }

This printk should have a KERN_ERROR or similar level.
Ideally, replace all instances of printk in your driver with 'dev_err',
'dev_info' or similar calls from <linux/device.h>.

	Arnd <><