Re: [PATCH 2/2] Staging: comedi: comedi_fops: Fix "out of minor numbers for board device files"

From: Ian Abbott
Date: Wed Mar 08 2017 - 06:16:30 EST


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@xxxxxxxxx> )=-
-=( Web: http://www.mev.co.uk/ )=-