On Tue, Mar 17, 2020 at 9:45 AM Horia Geantă <horia.geanta@xxxxxxx> wrote: > > On 3/16/2020 5:01 PM, Andrey Smirnov wrote: > > diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c > > index 69a02ac5de54..753625f2b2c0 100644 > > --- a/drivers/crypto/caam/caamrng.c > > +++ b/drivers/crypto/caam/caamrng.c > > @@ -70,6 +70,7 @@ struct buf_data { > > > > /* rng per-device context */ > > struct caam_rng_ctx { > > + struct hwrng rng; > > struct device *jrdev; > > dma_addr_t sh_desc_dma; > > u32 sh_desc[DESC_RNG_LEN]; > > @@ -78,13 +79,10 @@ struct caam_rng_ctx { > > struct buf_data bufs[2]; > > }; > [...] > > +static struct caam_rng_ctx *to_caam_rng_ctx(struct hwrng *r) > > +{ > > + return (struct caam_rng_ctx *)r->priv; > > +} > [...] > > -static struct hwrng caam_rng = { > > - .name = "rng-caam", > > - .init = caam_init, > > - .cleanup = caam_cleanup, > > - .read = caam_read, > > -}; > I would keep this statically allocated, see below. > > > @@ -342,18 +332,27 @@ int caam_rng_init(struct device *ctrldev) > > if (!rng_inst) > > return 0; > > > > - rng_ctx = kmalloc(sizeof(*rng_ctx), GFP_DMA | GFP_KERNEL); > > - if (!rng_ctx) > > + if (!devres_open_group(ctrldev, caam_rng_init, GFP_KERNEL)) > > + return -ENOMEM; > > + > > + ctx = devm_kzalloc(ctrldev, sizeof(*ctx), GFP_DMA | GFP_KERNEL); > > + if (!ctx) > > return -ENOMEM; > > > > + ctx->rng.name = "rng-caam"; > > + ctx->rng.init = caam_init; > > + ctx->rng.cleanup = caam_cleanup; > > + ctx->rng.read = caam_read; > > + ctx->rng.priv = (unsigned long)ctx; > > + > > dev_info(ctrldev, "registering rng-caam\n"); > > > > - err = hwrng_register(&caam_rng); > > - if (!err) { > > - init_done = true; > > - return err; > > + ret = devm_hwrng_register(ctrldev, &ctx->rng); > Now that hwrng.priv is used to keep driver's private data / caam_rng_ctx, > and thus container_of() is no longer needed to get from hwrng struct > to caam_rng_ctx, it's no longer needed to embed struct hwrng > into caam_rng_ctx. > Why do we want this change though? It doesn't allow us to constify "struct hwrng", don't seem to give any other benefits (maybe I am missing something?) at the same time creating a new implicit global variable. I'd rather not do this. Thanks, Andrey Smirnov