Subject: Re: [PATCH 2/2] Staging: comedi: comedi_fops: Fix "out of minor
 numbers for board device files"
To: Cheah Kok Cheong <thrust73@gmail.com>,
        Dan Carpenter <dan.carpenter@oracle.com>
References: <cd5f3e9f6320ac0063e61f0d0457e5400091390c.1488654618.git.thrust73@gmail.com>
 <b96cfe8f8b63ff70d8bc3c34bff24ea0fbfc191d.1488654618.git.thrust73@gmail.com>
 <20170307195149.GA10722@mwanda>
 <20170308100809.GB2496@linux-Precision-WorkStation-T5500>
Cc: hsweeten@visionengravers.com, gregkh@linuxfoundation.org,
        devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org
From: Ian Abbott <abbotti@mev.co.uk>
Message-ID: <ac316a4c-3e12-7ab4-116c-fed5804014a9@mev.co.uk>
Date: Wed, 8 Mar 2017 11:13:49 +0000
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Icedove/45.6.0
MIME-Version: 1.0
In-Reply-To: <20170308100809.GB2496@linux-Precision-WorkStation-T5500>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2546
Lines: 68

On 08/03/17 10:08, Cheah Kok Cheong wrote:
> Dear Dan,
>   Thanks for reviewing this patch.
>
> On Wed, Mar 08, 2017 at 08:54:47AM +0300, Dan Carpenter wrote:
>> On Sun, Mar 05, 2017 at 03:22:33AM +0800, Cheah Kok Cheong wrote:
>>> If comedi module is loaded with the following max allowed parameter
>>> [comedi_num_legacy_minors=48], subsequent loading of an auto-configured
>>> device will fail.
>>
>> Don't set comedi_num_legacy_minors=48, then?
>>
>> This doesn't seem like the right fix at all.  Why only allow one auto
>> configured board?  Why not 5 or 10?
>>
>
> Let me explain, the original intended behaviour is to allow user to
> reserve up to 48 minor numbers for legacy devices.
>
> Therefore [sudo modprobe comedi comedi_num_legacy_minors=3]
> will allocate minor number 0, 1, 2 for legacy devices.
> Subsequent loading of an auto-configured device will use minor number 3.
> And the next one number 4 so on and so forth.
>
> Now for the corner case of [comedi_num_legacy_minors=48] which
> is supposed to reserve minor number 0 till 47 for legacy devices,
> and is supposed to allocate number 48 and so on for auto-configured
> devices, does not allocate number 48 anymore after commit
> 38b9722a4414.

Both legacy and auto-configured devices will have minor numbers less 
than 48, which is the total number of devices currently supported by 
Comedi.  Minor numbers 48 and above have been reserved for dynamic 
allocation to those Comedi subdevices that support asynchronous commands.

> This is due to the changes in comedi_alloc_board_minor().
>
> As to why I chose to limit [comedi_num_legacy_minors=47], is given
> in the commit log.
>
> This will allow user to allocate 0 till 46 for legacy devices and
> subsequent auto-configured devices will start from 47 and so forth.
>
> I don't think anybody will miss one less number for legacy devices
> otherwise there'll be complains earlier on.

As Dan implies above, if you want auto-configured devices to work, set 
comedi_num_legacy_minors to a value less than 48.  Further limiting 
comedi_num_legacy_minors to 47 seems a bit arbitrary.

The comedi_test module mentioned in your commit can still be used when 
comedi_num_legacy_minors=48 by configuring a "legacy" comedi_test 
device.  Most of the other Comedi drivers support "legacy" devices 
exclusive-or auto-configured devices.

> Thanks.
>
> Brgds,
> CheahKC
>
>> regards,
>> dan carpenter
>>


-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-