Re: MPC8349ea Random Device Generator driver

[Arnd Bergmann <arnd@arndb.de>]

On Tuesday 05 June 2007, Philippe Lachenal wrote:
> I've made a driver for the MPC8349ea Random Device Generator, and I=20
> 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)
> +{
> +=A0=A0=A0=A0=A0=A0=A0if (!machine_is(mpc834x_itx)){
> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0printk("__init mpc834x_itx_=
declare_of_platform_devices error\n");
> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0return 0;
> +=A0=A0=A0=A0=A0=A0=A0}

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

Just remove the printk here.

> +=A0=A0=A0=A0=A0=A0=A0err =3D of_address_to_resource(rng_np, 0, &res);
> +=A0=A0=A0=A0=A0=A0=A0if (err)
> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0return -ENODEV;=A0
> +=A0=A0=A0=A0=A0=A0=A0rng_regs =3D ioremap(res.start, (res.end - res.star=
t));

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

> +=A0=A0=A0=A0=A0=A0=A0printk(KERN_INFO "Registering Talitos RNG\n");
> +
> +=A0=A0=A0=A0=A0=A0=A0err =3D hwrng_register(&talitos_rng);
> +=A0=A0=A0=A0=A0=A0=A0if (err){
> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0printk(".............. fail=
ure\n");
> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0iounmap(rng_regs);
> +=A0=A0=A0=A0=A0=A0=A0}

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 <><