Re: [PATCH v2 2/2] staging: ion: create one device entry per heap

[Greg KH]

On Tue, Sep 19, 2017 at 01:55:36PM +0200, Benjamin Gaignard wrote:
> >> +
> >>       spin_lock_init(&heap->free_lock);
> >>       heap->free_list_size = 0;
> >>
> >> @@ -595,13 +610,9 @@ static int ion_device_create(void)
> >>       if (!idev)
> >>               return -ENOMEM;
> >>
> >> -     idev->dev.minor = MISC_DYNAMIC_MINOR;
> >> -     idev->dev.name = "ion";
> >> -     idev->dev.fops = &ion_fops;
> >> -     idev->dev.parent = NULL;
> >> -     ret = misc_register(&idev->dev);
> >> +     ret = alloc_chrdev_region(&idev->devt, 0, ION_DEV_MAX, "ion");
> >
> > Did you just change the major number for the device node as well?
> >
> My understanding of alloc_chrdev_region() is that major number is chosen
> dynamically but I don't understand the link with device node, sorry.

Yes, the major is chosen dynamically if you ask for it (like you are
here), but previously you were using the misc major number.  That might
break userspace really badly if it had hard-coded /dev (like lots of
android devices do...)

> > Wow, that's a lot of userspace breakage (both major number, and name),
> > how did you test this?
> 
> I had to write a test by myself:
> https://git.linaro.org/people/benjamin.gaignard/ion_test_application.git/log/?h=one_device_per_heap
> 
> Laura have tried to push a test VGEM but I believe it hasn't be
> accepted yet (I will check)

A "test" program is a bit different than "boots and runs a working
system", right?  Please work with the correct graphics people to ensure
this change works with their code, before resending it.

thanks,

greg k-h