Re: [PATCH v2 3/4] virtio: rng: delay hwrng_register() till driver is ready

[Herbert Xu]

On Mon, Jul 21, 2014 at 08:11:16AM -0400, Jason Cooper wrote:
>
> > @@ -136,15 +137,6 @@ static int probe_common(struct virtio_device *vdev)
> >  		return err;
> >  	}
> >  
> > -	err = hwrng_register(&vi->hwrng);
> > -	if (err) {
> > -		vdev->config->del_vqs(vdev);
> > -		vi->vq = NULL;
> > -		kfree(vi);
> > -		ida_simple_remove(&rng_index_ida, index);
> > -		return err;
> > -	}
> > -
> 
> This needs to stay.  register, and failure to do so, should occur in the
> probe routine.

Why? Probing involves finding out whether the hardware is present.
While hwrng_register is about adding an entry in the hwrng system
which may only be one aspect of the given hardware.

For example, the same hardware may support other features that are
used outside of the hwrng system, e.g., crypto.

So there's nothing special about probe that requires us to have the
hwrng_register call there.  In practice, the only difference between
having it in probe vs. scan is that you can return errors.  The only
error that can be returned is ENOMEM which isn't of much interest to
the caller of probe anyway.

On the other hand, if you are calling hwrng_register you better be
damn sure that your hardware is ready to answer requests from the
hwrng system.  Please don't add silly flags to work around this.

Cheers,
-- 
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/