On Sat, Aug 10, 2019 at 12:58:19PM +0200, Christoph Hellwig wrote:
> 
> > +int kvmppc_devm_init(void)
> > +{
> > +   int ret = 0;
> > +   unsigned long size;
> > +   struct resource *res;
> > +   void *addr;
> > +
> > +   size = kvmppc_get_secmem_size();
> > +   if (!size) {
> > +           ret = -ENODEV;
> > +           goto out;
> > +   }
> > +
> > +   ret = alloc_chrdev_region(&kvmppc_devm.devt, 0, 1,
> > +                           "kvmppc-devm");
> > +   if (ret)
> > +           goto out;
> > +
> > +   dev_set_name(&kvmppc_devm.dev, "kvmppc_devm_device%d", 0);
> > +   kvmppc_devm.dev.release = kvmppc_devm_release;
> > +   device_initialize(&kvmppc_devm.dev);
> > +   res = devm_request_free_mem_region(&kvmppc_devm.dev,
> > +           &iomem_resource, size);
> > +   if (IS_ERR(res)) {
> > +           ret = PTR_ERR(res);
> > +           goto out_unregister;
> > +   }
> > +
> > +   kvmppc_devm.pagemap.type = MEMORY_DEVICE_PRIVATE;
> > +   kvmppc_devm.pagemap.res = *res;
> > +   kvmppc_devm.pagemap.ops = &kvmppc_devm_ops;
> > +   addr = devm_memremap_pages(&kvmppc_devm.dev, &kvmppc_devm.pagemap);
> > +   if (IS_ERR(addr)) {
> > +           ret = PTR_ERR(addr);
> > +           goto out_unregister;
> > +   }
> 
> It seems a little silly to allocate a struct device just so that we can
> pass it to devm_request_free_mem_region and devm_memremap_pages.  I think
> we should just create non-dev_ versions of those as well.

There is no reason for us to create a device really. If non-dev versions
of the above two routines are present, I can switch.

I will take care of the rest of your comments. Thanks for the review.

Regards,
Bharata.

Reply via email to