Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751887AbdISMi4 (ORCPT ); Tue, 19 Sep 2017 08:38:56 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:37876 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750972AbdISMiy (ORCPT ); Tue, 19 Sep 2017 08:38:54 -0400 Date: Tue, 19 Sep 2017 14:39:05 +0200 From: Greg KH To: Benjamin Gaignard Cc: driverdevel , Arve =?iso-8859-1?B?SGr4bm5lduVn?= , Linux Kernel Mailing List , Riley Andrews , Mark Brown , Sumit Semwal , Dan Carpenter Subject: Re: [PATCH v2 2/2] staging: ion: create one device entry per heap Message-ID: <20170919123905.GA17917@kroah.com> References: <1505816738-30017-1-git-send-email-benjamin.gaignard@linaro.org> <1505816738-30017-3-git-send-email-benjamin.gaignard@linaro.org> <20170919110254.GB4511@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.0 (2017-09-02) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1576 Lines: 42 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