chrdev: implement __[un]register_chrdev()
PATCH 1/2 chrdev: implement __[un]register_chrdev()
[un]register_chrdev() assume minor range 0-255. This patch adds __ prefixed versions which take @minorbase and @count explicitly.
---
Hello,
These two patches make sound_core grabbing OSS device number optional. If there's no objection, I think it would be easiest to push this through Takashi's tree.
Thanks.
fs/char_dev.c | 63 +++++++++++++++++++++++++++++++++++++++++++---------- include/linux/fs.h | 11 ++++++--- 2 files changed, 60 insertions(+), 14 deletions(-)
fs/char_dev.c +52 -11
@@ -237,8 +237,10 @@ int alloc_chrdev_region(dev_t *dev, unsi }/** -* register_chrdev() - Register a major number for character devices. +* __register_chrdev() - create and register a cdev occupying a range of minors * @major: major device number or 0 for dynamic allocation +* @baseminor: first of the requested range of minor numbers +* @count: the number of minor numbers required * @name: name of this range of devices * @fops: file operations associated with this devices *
@@ -254,19 +256,17 @@ int alloc_chrdev_region(dev_t *dev, unsi* /dev. It only helps to keep track of the different owners of devices. If * your module name has only one type of devices it's ok to use e.g. the name * of the module here. -* -* This function registers a range of 256 minor numbers. The first minor number -* is 0. */ -int register_chrdev (unsigned int major,const char *name, -const struct file_operations *fops) +int __register_chrdev (unsigned int major,unsigned int baseminor, +unsigned int count,const char *name, +const struct file_operations *fops) {struct char_device_struct *cd;struct cdev *cdev;char *s;int err = -ENOMEM ; - cd =__register_chrdev_region (major,0 ,256 , name); + cd =__register_chrdev_region (major, baseminor, count, name);if (IS_ERR (cd))return PTR_ERR (cd);
@@ -280,7 +280,7 @@ int register_chrdev(unsigned int major,for (s =strchr (kobject_name (&cdev->kobj),'/' ); s; s =strchr (s,'/' )) *s ='!' ; - err =cdev_add (cdev,MKDEV (cd->major,0 ),256 ); + err =cdev_add (cdev,MKDEV (cd->major, baseminor), count);if (err) goto out;
@@ -290,11 +290,26 @@ int register_chrdev(unsigned int major, out:kobject_put (&cdev->kobj); out2: -kfree (__unregister_chrdev_region (cd->major,0 ,256 )); +kfree (__unregister_chrdev_region (cd->major, baseminor, count));return err; }/** +* register_chrdev() - create and register a cdev occupying a full major +* @major: major device number or 0 for dynamic allocation +* @name: name of this range of devices +* @fops: file operations associated with this devices +* +* Wrapper around __register_chrdev() which always allocates full +* minor range from 0 to 256. +*/ +int register_chrdev (unsigned int major,const char *name, +const struct file_operations *fops) +{ +return __register_chrdev (major,0 ,256 , name, fops); +} + +/** * unregister_chrdev_region() - return a range of device numbers * @from: the first in the range of numbers to unregister * @count: the number of device numbers to unregister
@@ -316,15 +331,41 @@ void unregister_chrdev_region(dev_t from } } -void unregister_chrdev (unsigned int major,const char *name) +/** +* __unregister_chrdev - unregister and destroy a cdev +* @major: major device number +* @baseminor: first of the range of minor numbers +* @count: the number of minor numbers this cdev is occupying +* @name: name of this range of devices +* +* Unregister and destroy the cdev occupying the region described by +* @major, @baseminor and @count. This function undoes what +* __register_chrdev() did. +*/ +void __unregister_chrdev (unsigned int major,unsigned int baseminor, +unsigned int count,const char *name) {struct char_device_struct *cd; - cd =__unregister_chrdev_region (major,0 ,256 ); + + cd =__unregister_chrdev_region (major, baseminor, count);if (cd && cd->cdev)cdev_del (cd->cdev);kfree (cd); } +/** +* unregister_chrdev - unregister and destroy a cdev +* @major: major device number +* @name: name of this range of devices +* +* Wrapper around __unregister_chrdev() which assumes full minor range +* from 0 to 256. +*/ +void unregister_chrdev (unsigned int major,const char *name) +{ +__unregister_chrdev (major,0 ,256 , name); +} +static DEFINE_SPINLOCK (cdev_lock);static struct kobject *cdev_get (struct cdev *p)
include/linux/fs.h +8 -3
@@ -1998,9 +1998,14 @@ extern void bd_release_from_disk(struct#define CHRDEV_MAJOR_HASH_SIZE 255extern int alloc_chrdev_region (dev_t *,unsigned ,unsigned ,const char *);extern int register_chrdev_region (dev_t ,unsigned ,const char *); -extern int register_chrdev (unsigned int ,const char *, -const struct file_operations *); -extern void unregister_chrdev (unsigned int ,const char *); +extern int __register_chrdev (unsigned int major,unsigned int baseminor, +unsigned int count,const char *name, +const struct file_operations *fops); +extern int register_chrdev (unsigned int major,const char *name, +const struct file_operations *fops); +extern void __unregister_chrdev (unsigned int major,unsigned int baseminor, +unsigned int count,const char *name); +extern void unregister_chrdev (unsigned int major,const char *name);extern void unregister_chrdev_region (dev_t ,unsigned );extern void chrdev_show (struct seq_file *,off_t );
PATCH 2/2 sound: make OSS device number claiming optional
If any OSS support is enabled, regardless of built-in or module, sound_core claims full OSS major number (that is, the old 0-255 region) to trap open attempts and request sound module automatically. This feature is redundant as chrdev already has such mechanism and no longer used by modern distros. This preemptive claiming prevents alternative OSS implementation.
This patch makes the preclaiming optional and adds a config option SOUND_OSS_CORE_PRECLAIM and kernel parameter soundcore.preclaim_oss to control whether preclaim is enabled or not. This allows distros and developers to try new things without rebuilding kernel.
---
Hello, Takashi.
This patch is to allow using ossp without needing to recompile the kernel. The affected code is mostly deprecated. chrdev already has exactly about the same mechanism although it would use different alises and AFAIK udev based modern distros just don't use it at all (SUSE doesn't at least). I thought about removing the code altogether but this part of code is hopefully in slow death so the least amount change should be the best.
Thanks.
sound/Kconfig | 20 +++++++++++++++ sound/sound_core.c | 68 ++++++++++++++++++++++++++++++++++++----------------- 2 files changed, 67 insertions(+), 21 deletions(-)
sound/Kconfig +20 -0
@@ -32,6 +32,26 @@ config SOUND_OSS_CORE bool default n +config SOUND_OSS_CORE_PRECLAIM + bool "Preclaim OSS device numbers" + depends on SOUND_OSS_CORE + default y + help + With this option enabled, the kernel will claim all OSS device + numbers if any OSS support (native or emulation) is enabled + whether the respective module is loaded or not and, if necessary, + try to load the appropriate module when one of the device numbers + is opened. With this option disabled, kernel will only claim + actually in-use device numbers and module loading is left to + userland. + + Disabling it allows alternative out-of-kernel OSS implementation. + + The behavior can be overridden using kernel parameter + soundcore.preclaim_oss. + + If unusre, say Y. + source "sound/oss/dmasound/Kconfig" if !M68K
sound/sound_core.c +47 -21
116 lines not on this page · raw message
Re: [PATCH 2/2] sound: make OSS device number claiming optional
Quoted 11 lines · On Wed, 05 Aug 2009 15:40:42 +0900 Tejun Heo <[email protected]> wrote:
If any OSS support is enabled, regardless of built-in or module,
sound_coreclaims full OSS major number (that is, the old 0-255 region) to trap open attempts and request sound module automatically. This feature is redundant as chrdev already has such mechanism and no longer used by modern distros. This preemptive claiming prevents alternative OSS implementation.This patch makes the preclaiming optional and adds a config option
SOUND_OSS_CORE_PRECLAIMand kernel parameter soundcore.preclaim_oss to control whether preclaim is enabled or not. This allows distros and developers to try new things without rebuilding kernel.
This looks like a random private devel hack sand I don't see why it's appropriate for mainstream, especially as these "new things" don't exist mainstream either.
Re: [PATCH 2/2] sound: make OSS device number claiming optional
Hello, Alan.
Quoted 1 line · Alan Cox
This looks like a random private devel hack sand
Why so?
Quoted 2 lines
I don't see why it's appropriate for mainstream, especially as these "new things" don't exist mainstream either.
The new things is CUSE and it's already in mainstream.
Thanks.
Signature · 1 line
tejun
Re: [PATCH 2/2] sound: make OSS device number claiming optional
Quoted 11 lines · On Wed, 05 Aug 2009 18:32:32 +0900 Tejun Heo <[email protected]> wrote:
Hello, Alan.
Quoted 1 line · Alan Cox
This looks like a random private devel hack sand
Why so?
Quoted 2 lines
I don't see why it's appropriate for mainstream, especially as these "new things" don't exist mainstream either.
The new things is CUSE and it's already in mainstream.
Then fix it to use the sound interfaces properly. We have a multiplexor for sound devices so they can be shared between OSS/ALSA/whatever. Use it. Ditto if you want CUSE to handle miscdevices use the miscdevice interface properly.
Re: [PATCH 2/2] sound: make OSS device number claiming optional
Hello, Alan.
Quoted 9 lines · Alan Cox
Quoted 6 lines · On Wed, 05 Aug 2009 18:32:32 +0900 Tejun Heo <[email protected]> wrote:
Hello, Alan.
Quoted 1 line · Alan Cox
This looks like a random private devel hack sand
Why so?
Quoted 8 lines
Quoted 3 lines
Quoted 2 lines
I don't see why it's appropriate for mainstream, especially as these "new things" don't exist mainstream either.
The new things is CUSE and it's already in mainstream.
Then fix it to use the sound interfaces properly. We have a multiplexor for sound devices so they can be shared between OSS/ALSA/whatever. Use it. Ditto if you want CUSE to handle miscdevices use the miscdevice interface properly.
The only problem here is the now obsolete hackish way OSS module auto-loading is handled. CUSE is being a regular chardev citizen. The OSS multiplexing mechanism itself isn't a problem and the posted patch doesn't distort it in any way but by preclaiming all OSS device numbers, OSS requires its own different rule for no good reason. The only actual difference here would be what module aliases are used.
Given the situation, I sure can add hacks to CUSE so that it doesn't do regular chardev thing but does OSS specific stuff if OSS major is detected. To me, this would be much painful exercise in spreading the hack.
The proper solution would be extending chardev such that OSS sound core can use its own custom module aliases for backward compatibility. But given that OSS is about the only user left for such facility and modern distros don't depend on the feature or can easily work around it from userland, that would be an overkill.
Another option is to rip the custom module requesting code altogether. It just doesn't matter at all anymore. modprobe is smart enough to work around those issues with a couple lines of configuration changes. We can add those alises to feature-removal-schedule.txt and then kill it in a year or so. If OSS is still alive and well, I would have chosen this path.
However, in-kernel OSS is largely dead or at least is dying a slow death. Native OSS drivers are no longer in development and the only left user is in-kernel emulation from ALSA, which I'm sure have its places but is getting more and more inadequate for general desktop usage due to lack of software multiplexing.
I don't see much reason to go through feature removal process for the now obsolete OSS-specific module requesting feature. Allowing distros to make the switch as they see fit and letting the code wither away is the least painful way.
So, in summary, the hack here is how OSS does custom module requesting and the proper solution would be removing the mis-feature according to our feature removal protocol or reimplementing it in a non-hackish way, both of which are unnecessarily painful and overkill at this point.
Thanks.
Signature · 1 line
tejun
Re: [PATCH 2/2] sound: make OSS device number claiming optional
Quoted 2 lines
The only problem here is the now obsolete hackish way OSS module auto-loading is handled. CUSE is being a regular chardev citizen.
"obsolete hackish". It's the same mechanism used by just about every other driver that multiplexes. Doesn't seem obsolete to me. You are expected to register subdrivers of most specialist interfaces through that interface. You use misc_register, video_register_device etc.
Providing you do so it all works nicely.
The OSS device major is owned by OSS (soundcore). If you want to use it your driver should use the interfaces provided by soundcore. Ditto any other similar interface.
If you want to completely pre-empt soundcore then use a different (probably dynamic) major number and tweak your udev rules to create different /dev files.
None of this requires ugly kernel hacks and weird user config options and module params.
Quoted 4 lines
Given the situation, I sure can add hacks to CUSE so that it doesn't do regular chardev thing but does OSS specific stuff if OSS major is detected. To me, this would be much painful exercise in spreading the hack.
And it can then be a good citizen and share the resource. CUSE needs to do the same for video/misc/framebuffer/tty/usb/etc so this is a general pattern it needs to handle. In short cuse needs to grow this ability anyway if it wants to be support these sorts of device.
Quoted 5 lines
However, in-kernel OSS is largely dead or at least is dying a slow death. Native OSS drivers are no longer in development and the only left user is in-kernel emulation from ALSA, which I'm sure have its places but is getting more and more inadequate for general desktop usage due to lack of software multiplexing.
The desktop uses pulseaudio for the most part (or jack in some setups). OSS uses are pretty rare now that flash has gone away. Unfortunately many of those existing users aren't terribly fixable
Quoted 4 lines
I don't see much reason to go through feature removal process for the now obsolete OSS-specific module requesting feature. Allowing distros to make the switch as they see fit and letting the code wither away is the least painful way.
You appear to be self contradictory here. Either
- OSS is irrelevant in which case we don't need the CUSE stuff either
- OSS is relevant in which case we shouldn't break it
A few old old games still use OSS, are proprietary apps and have never been re-released. Some of them also do irritiating to emulate things like use mmap. Flash at least appears to have finally gotten the message.
Alan
Re: [PATCH 2/2] sound: make OSS device number claiming optional
Hello, Alan.
Quoted 7 lines · Alan Cox
Quoted 2 lines
The only problem here is the now obsolete hackish way OSS module auto-loading is handled. CUSE is being a regular chardev citizen.
"obsolete hackish". It's the same mechanism used by just about every other driver that multiplexes. Doesn't seem obsolete to me. You are expected to register subdrivers of most specialist interfaces through that interface. You use
misc_register,video_register_deviceetc.
Subsystem registration isn't hackish or obsolete but claiming all minor devices to use custom module alises is at best obsolete. There just is no reason to do that.
Quoted 5 lines
Providing you do so it all works nicely.
The OSS device major is owned by OSS (soundcore). If you want to use it your driver should use the interfaces provided by soundcore. Ditto any other similar interface.
None of this would be a problem if OSS doesn't claim minors it doesn't have actual device for and as I said multiple times there is no benefit in doing that, not anymore and the only reason to keep that around is to maintain backward compatibility for a mostly unused and easy to work around feature.
Quoted 9 lines
Quoted 4 lines
Given the situation, I sure can add hacks to CUSE so that it doesn't do regular chardev thing but does OSS specific stuff if OSS major is detected. To me, this would be much painful exercise in spreading the hack.
And it can then be a good citizen and share the resource. CUSE needs to do the same for video/misc/framebuffer/tty/usb/etc so this is a general pattern it needs to handle. In short cuse needs to grow this ability anyway if it wants to be support these sorts of device.
The sharing and mixing and matching can happen very gracefully at chrdev level. Does it make sense to drag it down to each subsystem level just to use custom module alises? Not at all to me. The same goes for video or whatever. There is no reason to claim unused device numbers anymore and as long as that holds everyone is good citizen at the chrdev region. That's _much_ cleaner.
Quoted 9 lines
Quoted 4 lines
I don't see much reason to go through feature removal process for the now obsolete OSS-specific module requesting feature. Allowing distros to make the switch as they see fit and letting the code wither away is the least painful way.
You appear to be self contradictory here. Either
- OSS is irrelevant in which case we don't need the CUSE stuff either
- OSS is relevant in which case we shouldn't break it
What's broken? Module aliases? The whole point of the patch is to let people keep the module alises while allowing distros which wish to try or switch to different mechanism to do so.
Quoted 3 lines
A few old old games still use OSS, are proprietary apps and have never been re-released. Some of them also do irritiating to emulate things like use mmap. Flash at least appears to have finally gotten the message.
I like backward compatibility and think it's relevant but its relevance is much lower than current stuff and ever diminishing. Plus the current in-kernel emulation has annoying limitations which can be alleviated by userland implementation. So,
* OSS compatibility as seen from userland apps : relevant
* doing everything right by sound_core.c which basically is only used by in-kernel emulation : not so much
Thanks.
Signature · 1 line
tejun
Re: [PATCH 2/2] sound: make OSS device number claiming optional
Quoted 11 lines
Quoted 4 lines
And it can then be a good citizen and share the resource. CUSE needs to do the same for video/misc/framebuffer/tty/usb/etc so this is a general pattern it needs to handle. In short cuse needs to grow this ability anyway if it wants to be support these sorts of device.
The sharing and mixing and matching can happen very gracefully at chrdev level. Does it make sense to drag it down to each subsystem level just to use custom module alises? Not at all to me. The same goes for video or whatever. There is no reason to claim unused device numbers anymore and as long as that holds everyone is good citizen at the chrdev region. That's _much_ cleaner.
I would suggest you read the code for the others. OSS is probably the easy one to kill but the others create objects for platform wide sysfs files and the like (as indeed OSS once did for a proc file) so in the general case CUSE needs to work through them.
Quoted 4 lines
* OSS compatibility as seen from userland apps : relevant
* doing everything right by
sound_core.cwhich basically is only used by in-kernel emulation : not so much
So instead of weird config magic why not send both messages. Initially soundcore can generate both requests for sound-slot-foo and also fake up its own char requests. It's easy to do, it doesn't put any code into the kernel we'll be stuck with for years with weird config options. It doesn't require the user choose a magic kernel option - it just works.
It's not much code either - just another request call in soundcore_open - one line extra to write, a year to wait (4 releases ?) and then the old stuff can go away, ALSA can then avoid soundcore and soundcore can vanish.
Much cleaner, and I believe one line of code ?
Re: [PATCH 2/2] sound: make OSS device number claiming optional
Hello, Alan.
Quoted 4 lines · Alan Cox
I would suggest you read the code for the others. OSS is probably the easy one to kill but the others create objects for platform wide sysfs files and the like (as indeed OSS once did for a proc file) so in the general case CUSE needs to work through them.
I don't think it's wise for CUSE to try to emulate all those. CUSE provides emulation for a character device file and that's it. It's a building block not a complete solution. In many cases information provided via sysfs or proc doesn't matter to much for individual apps and there's no reason to try to emulate them. For other cases, those nodes would need to be emulated using other building blocks - tmpfs or fuse overlay mount maybe.
Quoted 6 lines
Quoted 4 lines
* OSS compatibility as seen from userland apps : relevant
* doing everything right by
sound_core.cwhich basically is only used by in-kernel emulation : not so muchSo instead of weird config magic why not send both messages.
It's weird but simple and the alternative behavior of only acquiring existent devices shouldn't hurt anyone who's using anything remotely modern. The only thing that changes is the module alias kernel requests after all. Oh well...
Quoted 11 lines
Initially soundcore can generate both requests for sound-slot-foo and also fake up its own char requests. It's easy to do, it doesn't put any code into the kernel we'll be stuck with for years with weird config options. It doesn't require the user choose a magic kernel option - it just works.
It's not much code either - just another request call in
soundcore_open- one line extra to write, a year to wait (4 releases ?) and then the old stuff can go away, ALSA can then avoid soundcore and soundcore can vanish.Much cleaner, and I believe one line of code ?
Hmm... the problem is that the old stuff claims the whole chrdev region. I was thinking about adding support for alternative module alises at the chrdev layer if this is that essential and changing sound_core to claim only the devices which actually exist. That way we can continue to use the same module aliases and allo OSS emulation and ossp to occupy the same kernel.
I don't see how I would be able to achieve the latter with one liner. Can you please elaborate a little bit?
Thanks.
Signature · 1 line
tejun
Re: [PATCH 2/2] sound: make OSS device number claiming optional
Quoted 2 lines
I don't see how I would be able to achieve the latter with one liner. Can you please elaborate a little bit?
It depends how much of a hurry you are in, but for the mainstream we can do this
1. Make soundcore also issue the request_module() as if the char
minor was unclaimed. Mark the old one as obsolete
2. Wait a year while people adjust their scripts to trigger on the
char event instead (and fix the ordering bits for ALSA if the soundcore docs are still valid on that)
3. Apply a patch which makes ALSA use char dev allocation directly
for its OSS devices and dumps out soundcore, remove soundcore and switch any other code using it.
Re: [PATCH 2/2] sound: make OSS device number claiming optional
Quoted 5 lines · On Wed, 5 Aug 2009 17:33:46 +0100 Alan Cox <[email protected]> wrote:
Quoted 2 lines
I don't see how I would be able to achieve the latter with one liner. Can you please elaborate a little bit?
It depends how much of a hurry you are in, but for the mainstream we can do this
and a PS to this
If we have a clear plan on obsoleting sound_, a proper documented entry for it going obsolete and a clear roadmap which ends up with soundcore being recycled[1] then adding a temporary option to soundcore to make it shut up is much less of a problem because the option also clearly goes obsolete with soundcore so no mess is left when the process is over - which would let you get your CUSE stuff up as a hack in the meantime.
Ala
[1] 0 bits in the green bag, 1 bits in the black bag please
Re: [PATCH 2/2] sound: make OSS device number claiming optional
In reply to Alan CoxQuoted 9 lines · Alan Cox
Quoted 2 lines
I don't see how I would be able to achieve the latter with one liner. Can you please elaborate a little bit?
It depends how much of a hurry you are in, but for the mainstream we can do this
1. Make soundcore also issue the
request_module()as if the char
minor was unclaimed. Mark the old one as obsolete
Quoted 7 lines
2. Wait a year while people adjust their scripts to trigger on the
char event instead (and fix the ordering bits for ALSA if the soundcore docs are still valid on that)3. Apply a patch which makes ALSA use char dev allocation directly
for its OSS devices and dumps out soundcore, remove soundcore and switch any other code using it.
It's quite difficult to think that those now mostly unused module alises are worth full year of waiting and careful coordination. I think we're chasing a non-issue here. If it really matters, I'll try to teach chrdev about alternative module alises.
If you think the above is a good solution, how about the following?
1. Merge the weird switch thing and the extra standard chrdev module alias patch
2. Add to feature-removal that snd-slot/service-* are going away in a year along with the weird switches. This allows people who wish to try or switch in the meantime to do so.
3. After a year, drop module loading related code from sound_core along with the weird config option and kernel parameter.
In the end, the only choice we have to make is whether to keep snd-slot/service-* aliases. If we're gonna (I don't see why tho), the cleanest way would be to teach chrdev about aliases. If not, the best way is to add a switch so that it can be phased out gradually.
Thanks.
Signature · 1 line
tejun
Re: [PATCH 2/2] sound: make OSS device number claiming optional
Quoted 2 lines
1. Merge the weird switch thing and the extra standard chrdev module alias patch
Might as well fake the normal aliases in soundcore if loaded rather than pollute chardev with it but otherwise I think we agree.
Quoted 3 lines
2. Add to feature-removal that snd-slot/service-* are going away in a year along with the weird switches. This allows people who wish to try or switch in the meantime to do so.
Yep
Quoted 2 lines
3. After a year, drop module loading related code from
sound_corealong with the weird config option and kernel parameter.
Do we need soundcore at all at that point ? It seems phase 3 is "move any needed logic to ALSA oss emulation and kill it off"
Quoted 2 lines
In the end, the only choice we have to make is whether to keep snd-slot/service-* aliases. If we're gonna (I don't see why tho), the
I think we need to for a year or so - and its trivial to do so.
Quoted 2 lines
cleanest way would be to teach chrdev about aliases. If not, the best way is to add a switch so that it can be phased out gradually.
In the mean time if you are grabbing just some minors sound_core.c:chains also needs keeping in sync with direct character range grabs because your CUSE based device might grab some minors before sound_core, and then it appears tears will result ?
Re: [PATCH 2/2] sound: make OSS device number claiming optional
Hello, Alan.
Quoted 5 lines · Alan Cox
Quoted 2 lines
1. Merge the weird switch thing and the extra standard chrdev module alias patch
Might as well fake the normal aliases in soundcore if loaded rather than pollute chardev with it but otherwise I think we agree.
Yeap, that was what I meant by the second part of the sentence.
Quoted 11 lines
Quoted 3 lines
2. Add to feature-removal that snd-slot/service-* are going away in a year along with the weird switches. This allows people who wish to try or switch in the meantime to do so.
Yep
Quoted 2 lines
3. After a year, drop module loading related code from
sound_corealong with the weird config option and kernel parameter.Do we need soundcore at all at that point ? It seems phase 3 is "move any needed logic to ALSA oss emulation and kill it off"
Yeah, sure. There's nothing preventing ALSA from directly registering chrdevs.
Quoted 5 lines
Quoted 2 lines
In the end, the only choice we have to make is whether to keep snd-slot/service-* aliases. If we're gonna (I don't see why tho), the
I think we need to for a year or so - and its trivial to do so.
Quoted 7 lines
Quoted 2 lines
cleanest way would be to teach chrdev about aliases. If not, the best way is to add a switch so that it can be phased out gradually.
In the mean time if you are grabbing just some minors sound_core.c:chains also needs keeping in sync with direct character range grabs because your CUSE based device might grab some minors before
sound_core, and then it appears tears will result ?
Hmmm... okay, will take a deeper look at that.
Thanks.
Signature · 1 line
tejun
Re: [PATCH 2/2] sound: make OSS device number claiming optional
In reply to Alan Cox'Twas brillig, and Alan Cox at 05/08/09 10:15 did gyre and gimble:
Quoted 10 lines
Quoted 7 lines · On Wed, 05 Aug 2009 15:40:42 +0900 Tejun Heo <[email protected]> wrote:
If any OSS support is enabled, regardless of built-in or module,
sound_coreclaims full OSS major number (that is, the old 0-255 region) to trap open attempts and request sound module automatically. This feature is redundant as chrdev already has such mechanism and no longer used by modern distros. This preemptive claiming prevents alternative OSS implementation.
Quoted 8 lines
Quoted 4 lines
This patch makes the preclaiming optional and adds a config option
SOUND_OSS_CORE_PRECLAIMand kernel parameter soundcore.preclaim_oss to control whether preclaim is enabled or not. This allows distros and developers to try new things without rebuilding kernel.This looks like a random private devel hack sand I don't see why it's appropriate for mainstream, especially as these "new things" don't exist mainstream either.
While I'm not familiar enough with the kernel itself to comment on *how* the patch works, the principle is quite important here.
As far as I know most distros enable snd-*-oss module loading via a modprobe trick (e.g. in user space) so the change here shouldn't affect this approach.
However, I certainly want to experiment with osspd and while I hope it's going to be a good solution generally, it'll only make sense if the user chooses to use pulseaudio.
If the user decided they want to use pure alsa, then they'll have to drop back to using snd-*-oss for that instead.
This is all something I'd like to enable without making the user change kernels based on a preference.
So if this patch is not accepted, I'd still like to see the kernel not claim this device number, and leave it up to userspace to load the modules.
Sorry if this is not all factually accurate, but I think i've got the gist right! :)
Signature · 10 lines
Colin Guthrie cguthrie(at)mandriva.org http://colin.guthr.ie/ Day Job: Tribalogic Limited [http://www.tribalogic.net/] Open Source: Mandriva Linux Contributor [http://www.mandriva.com/] PulseAudio Hacker [http://www.pulseaudio.org/] Trac Hacker [http://trac.edgewall.org/]
Alan Cox
NAKRe: [PATCH 2/2] sound: make OSS device number claiming optional
Quoted 11 lines
While I'm not familiar enough with the kernel itself to comment on *how* the patch works, the principle is quite important here.
As far as I know most distros enable snd-*-oss module loading via a modprobe trick (e.g. in user space) so the change here shouldn't affect this approach.
However, I certainly want to experiment with osspd and while I hope it's going to be a good solution generally, it'll only make sense if the user chooses to use pulseaudio.
Quoted 5 lines
If the user decided they want to use pure alsa, then they'll have to drop back to using snd-*-oss for that instead.
This is all something I'd like to enable without making the user change kernels based on a preference.
As you said "I'd like to experiment"
We don't put everyones random experiments in the kernel or we'd have a kernel that had a million unfinished project hacks.
Suppose this doesn't work out - how do we get rid of the new hack, someone might by then have decided to depend upon it. More crud, more APIs we can't get rid of
More importantly there is an API *in* soundcore for registering sound devices in the OSS category. You can use that.
Not only was that interface written precisely to allow the kind of sharing in question but it will also let you mix OSS, ALSA *and* your own experimental device work at the same time because it hands out minor numbers properly.
Given we have an existing proper interface for this I think this hack belongs in a private dev tree only. In fact I suspect it belongs in the bitbucket and experimental code should be using the proper provided and exported interfaces as it will need to if it is ever going to be accepted upstream.
NAK
Alan
Re: [PATCH 2/2] sound: make OSS device number claiming optional
At Wed, 5 Aug 2009 10:59:16 +0100,
Quoted 41 lines · Alan Cox
Quoted 12 lines
While I'm not familiar enough with the kernel itself to comment on *how* the patch works, the principle is quite important here.
As far as I know most distros enable snd-*-oss module loading via a modprobe trick (e.g. in user space) so the change here shouldn't affect this approach.
However, I certainly want to experiment with osspd and while I hope it's going to be a good solution generally, it'll only make sense if the user chooses to use pulseaudio.
Quoted 5 lines
If the user decided they want to use pure alsa, then they'll have to drop back to using snd-*-oss for that instead.
This is all something I'd like to enable without making the user change kernels based on a preference.
As you said "I'd like to experiment"
We don't put everyones random experiments in the kernel or we'd have a kernel that had a million unfinished project hacks.
Suppose this doesn't work out - how do we get rid of the new hack, someone might by then have decided to depend upon it. More crud, more APIs we can't get rid of
More importantly there is an API *in* soundcore for registering sound devices in the OSS category. You can use that.
Not only was that interface written precisely to allow the kind of sharing in question but it will also let you mix OSS, ALSA *and* your own experimental device work at the same time because it hands out minor numbers properly.
Given we have an existing proper interface for this I think this hack belongs in a private dev tree only. In fact I suspect it belongs in the bitbucket and experimental code should be using the proper provided and exported interfaces as it will need to if it is ever going to be accepted upstream.
Quoted 3 lines
NAK
Alan
As far as I understand, the current problem is that ALSA core becomes dependent when you build with CONFIG_SND_*_OSS config, even if you load no snd-*-oss modules and use only ALSA-native ones. It's because the OSS device registration is done through the ALSA snd module. Thus, loading ALSA modules always results in loading soundcore module, too.
So... instead of hacking around soundcore stuff, splitting off the OSS (soundcore) dependency from ALSA snd module could be another solution? Then soundcore doesn't have to be loaded unless you load snd-*-oss or any real OSS modules.
One drawback is that this splits to yet another module. Or, maybe it can be put into somewhere else...
Takashi
Re: [PATCH 2/2] sound: make OSS device number claiming optional
Quoted 6 lines
As far as I understand, the current problem is that ALSA core becomes dependent when you build with CONFIG_SND_*_OSS config, even if you load no snd-*-oss modules and use only ALSA-native ones. It's because the OSS device registration is done through the ALSA snd module. Thus, loading ALSA modules always results in loading soundcore module, too.
That isn't a bug, but a feature.
Quoted 4 lines
So... instead of hacking around soundcore stuff, splitting off the OSS (soundcore) dependency from ALSA snd module could be another solution? Then soundcore doesn't have to be loaded unless you load snd-*-oss or any real OSS modules.
This makes it worse.
Quoted 2 lines
One drawback is that this splits to yet another module. Or, maybe it can be put into somewhere else...
Currently I can load bits of OSS driver, ALSA drivers and something else at the same time and soundcore will properly share the major number between devices and do allocations (Just as miscdevice does with its major).
If people start hacking around the interface rather than using it properly then the end user is left with
- obscure magical configuration Kconfig items
- a weird boot parameter/module option
- no ability to mix and match
Exactly why is that a forward step compared with teaching CUSE to behave like a civilised kernel driver with regard to sound ?
If CUSE/Sound is fixed to use the sound APIs then lo and behold
- no weird options
- no weird boot parameters
- mix and match just works
no user intervention, no complexity to the end user, and more flexible all around.
(Its also better for debug as you can load two different versions of the CUSE/Sound stack at once ...)
Hence I still think this patch is an ugly pointless hack that is nothing but private debug and quick hack stuff. Teach CUSE to behave and everything works nicely. CUSE needs this sort of additional plug in support anyway in order to do anything with it involving the other multiplexed device interfaces (eg tty, video4linux, framebuffer, usb, ...)
Re: [PATCH 2/2] sound: make OSS device number claiming optional
At Wed, 5 Aug 2009 11:26:49 +0100,
Quoted 9 lines · Alan Cox
Quoted 7 lines
As far as I understand, the current problem is that ALSA core becomes dependent when you build with CONFIG_SND_*_OSS config, even if you load no snd-*-oss modules and use only ALSA-native ones. It's because the OSS device registration is done through the ALSA snd module. Thus, loading ALSA modules always results in loading soundcore module, too.
That isn't a bug, but a feature.
Not really :) The soundcore is only for OSS device files. As long as you use only ALSA-native APIs, you don't need it. But, currently it's loaded and initialized just because of the module dependency. This is no bug but also not an intended feature.
Quoted 6 lines
Quoted 4 lines
So... instead of hacking around soundcore stuff, splitting off the OSS (soundcore) dependency from ALSA snd module could be another solution? Then soundcore doesn't have to be loaded unless you load snd-*-oss or any real OSS modules.
This makes it worse.
Well, the only regression would be the case where you create static /dev/dsp (or else) devices and let auto-loading through sound-slot-* or sound-service-*-* aliases. Of course, this still works if you load soundcore in some way.
What I suggested in the above is to cut off an unneeded dependency between soundcore and ALSA-native stuff instead of hacking soundcore. It won't change anything else, so everything else can coexist as before.
Just my $0.02, though...
Takashi
Re: [PATCH 2/2] sound: make OSS device number claiming optional
Quoted 4 lines
Not really :) The soundcore is only for OSS device files. As long as you use only ALSA-native APIs, you don't need it. But, currently it's loaded and initialized just because of the module dependency. This is no bug but also not an intended feature.
Sure for the ALSA device node it makes sense because ALSA is the mux for it. Fortunately soundcore is tiny anyway.
Quoted 6 lines
Quoted 1 line
This makes it worse.
Well, the only regression would be the case where you create static
/dev/dsp(or else) devices and let auto-loading through sound-slot-* or sound-service-*-* aliases. Of course, this still works if you load soundcore in some way.
Unless some ugly cuse hack got there first.
Quoted 4 lines
What I suggested in the above is to cut off an unneeded dependency between soundcore and ALSA-native stuff instead of hacking soundcore. It won't change anything else, so everything else can coexist as before.
Agreed - but that is really a separate issue to having something break the soundcore by being rude.
Alan
Re: [PATCH 2/2] sound: make OSS device number claiming optional
Quoted 8 lines · Alan Cox
Quoted 5 lines
Quoted 1 line
This makes it worse.
Well, the only regression would be the case where you create static
/dev/dsp(or else) devices and let auto-loading through sound-slot-* or sound-service-*-* aliases. Of course, this still works if you load soundcore in some way.Unless some ugly cuse hack got there first.
Quoted 7 lines
Quoted 4 lines
What I suggested in the above is to cut off an unneeded dependency between soundcore and ALSA-native stuff instead of hacking soundcore. It won't change anything else, so everything else can coexist as before.
Agreed - but that is really a separate issue to having something break the soundcore by being rude.
Sigh, does the wording really have to be 'ugly' and 'rude'? sound_core.c itself is a strange hack at this point. :-(
Signature · 1 line
tejun
Re: [PATCH 2/2] sound: make OSS device number claiming optional
In reply to Takashi IwaiQuoted 4 lines · Takashi Iwai
Well, the only regression would be the case where you create static
/dev/dsp(or else) devices and let auto-loading through sound-slot-* or sound-service-*-* aliases. Of course, this still works if you load soundcore in some way.
If we're gonna do that, we might as well just rip off the pointless 'claim everything so we can use our own module alises' code from sound_core.c and be done with it. That really is the only problem.
Thanks.
Signature · 1 line
tejun
Re: [PATCH 2/2] sound: make OSS device number claiming optional
Quoted 3 lines
If we're gonna do that, we might as well just rip off the pointless 'claim everything so we can use our own module alises' code from
sound_core.cand be done with it. That really is the only problem.
The interface expected is the sound_register_* interface and the sharing is likewise expected so you could for example mix OSS. CUSE and ALSA oss emulation drivers on the same system at the same time.
Rewriting sound_register* in terms of minor number allocations using the fact modern kernels have better allocators for devices would also be preferable to the weird module hacks initially proposed. I guess a starting point would be to tweak soundcore to generate both message sets itself for a year and then pull the old stuff out.
Alan
Re: [PATCH 2/2] sound: make OSS device number claiming optional
Hello,
Quoted 7 lines · Alan Cox
Quoted 3 lines
If we're gonna do that, we might as well just rip off the pointless 'claim everything so we can use our own module alises' code from
sound_core.cand be done with it. That really is the only problem.The interface expected is the sound_register_* interface and the sharing is likewise expected so you could for example mix OSS. CUSE and ALSA oss emulation drivers on the same system at the same time.
If OSS behaves like a good chrdev and claims what it can really do, all that can be done so much easier at the chrdev level.
Quoted 5 lines
Rewriting sound_register* in terms of minor number allocations using the fact modern kernels have better allocators for devices would also be preferable to the weird module hacks initially proposed. I guess a starting point would be to tweak soundcore to generate both message sets itself for a year and then pull the old stuff out.
I don't disagree with you in that the above would be the perfect technical solution but I just don't think the balance sheet is right.
Thanks.
Signature · 1 line
tejun
At Wed, 05 Aug 2009 15:35:42 +0900,
Quoted 7 lines · Tejun Heo
[un]register_chrdev() assume minor range 0-255. This patch adds __ prefixed versions which take @minorbase and @count explicitly.
Signed-off-by Tejun Heo <[email protected]>Cc Al Viro <[email protected]>Cc Greg Kroah-Hartman <[email protected]>
Quoted 6 lines
---
Hello,These two patches make
sound_coregrabbing OSS device number optional. If there's no objection, I think it would be easiest to push this through Takashi's tree.
I'm fine to take this.
But, I think EXPORT_SYMBOL(__register_chrdev) and EXPORT_SYMBOL(__unregister_chrdev) are missing in this patch.
thanks,
Takashi
Quoted 11 lines · Takashi Iwai
At Wed, 05 Aug 2009 15:35:42 +0900,
Quoted 9 lines · Tejun Heo
[un]register_chrdev() assume minor range 0-255. This patch adds __ prefixed versions which take @minorbase and @count explicitly.
Signed-off-by Tejun Heo <[email protected]>Cc Al Viro <[email protected]>Cc Greg Kroah-Hartman <[email protected]>---
Hello,
Quoted 8 lines
Quoted 3 lines
These two patches make
sound_coregrabbing OSS device number optional. If there's no objection, I think it would be easiest to push this through Takashi's tree.I'm fine to take this.
But, I think
EXPORT_SYMBOL(__register_chrdev)andEXPORT_SYMBOL(__unregister_chrdev)are missing in this patch.
The only current users would be sound_core.c which is always compiled built-in. Adrian would be mighty unhappy about adding EXPORT_SYMBOL() there.
Thanks.
Signature · 1 line
tejun
At Wed, 05 Aug 2009 16:11:21 +0900,
Quoted 17 lines · Tejun Heo
Quoted 15 lines · Takashi Iwai
At Wed, 05 Aug 2009 15:35:42 +0900,
Quoted 12 lines · Tejun Heo
[un]register_chrdev() assume minor range 0-255. This patch adds __ prefixed versions which take @minorbase and @count explicitly.
Signed-off-by Tejun Heo <[email protected]>Cc Al Viro <[email protected]>Cc Greg Kroah-Hartman <[email protected]>---
Hello,These two patches make
sound_coregrabbing OSS device number optional. If there's no objection, I think it would be easiest to push this through Takashi's tree.
Quoted 7 lines
Quoted 4 lines
I'm fine to take this.
But, I think
EXPORT_SYMBOL(__register_chrdev)andEXPORT_SYMBOL(__unregister_chrdev)are missing in this patch.The only current users would be
sound_core.cwhich is always compiled built-in.
CONFIG_SOUND is tristate, so it can be a module.
Actually SUSE kernels have it as a module :)
Quoted 2 lines
Adrian would be mighty unhappy about adding
EXPORT_SYMBOL()there.
How about to replace the old *register_chrdev() with static inline, instead?
thanks,
Takashi
Hello,
Quoted 2 lines · Takashi Iwai
CONFIG_SOUNDis tristate, so it can be a module.
Actually SUSE kernels have it as a module :)
Ah... crap. I got confused. I somehow thought it was always included in kernel.
Quoted 5 lines
Quoted 2 lines
Adrian would be mighty unhappy about adding
EXPORT_SYMBOL()there.How about to replace the old
*register_chrdev()with static inline, instead?
Will do so.
Thanks.
Signature · 1 line
tejun
PATCH 1/2 chrdev: implement __[un]register_chrdev()
[un]register_chrdev() assume minor range 0-255. This patch adds __ prefixed versions which take @minorbase and @count explicitly. The original ones become inline wrappers around __ ones.
--- fs/char_dev.c | 39 ++++++++++++++++++++++++++------------- include/linux/fs.h | 19 ++++++++++++++++--- 2 files changed, 42 insertions(+), 16 deletions(-)
fs/char_dev.c +26 -13
@@ -237,8 +237,10 @@ int alloc_chrdev_region(dev_t *dev, unsi }/** -* register_chrdev() - Register a major number for character devices. +* __register_chrdev() - create and register a cdev occupying a range of minors * @major: major device number or 0 for dynamic allocation +* @baseminor: first of the requested range of minor numbers +* @count: the number of minor numbers required * @name: name of this range of devices * @fops: file operations associated with this devices *
@@ -254,19 +256,17 @@ int alloc_chrdev_region(dev_t *dev, unsi* /dev. It only helps to keep track of the different owners of devices. If * your module name has only one type of devices it's ok to use e.g. the name * of the module here. -* -* This function registers a range of 256 minor numbers. The first minor number -* is 0. */ -int register_chrdev (unsigned int major,const char *name, -const struct file_operations *fops) +int __register_chrdev (unsigned int major,unsigned int baseminor, +unsigned int count,const char *name, +const struct file_operations *fops) {struct char_device_struct *cd;struct cdev *cdev;char *s;int err = -ENOMEM ; - cd =__register_chrdev_region (major,0 ,256 , name); + cd =__register_chrdev_region (major, baseminor, count, name);if (IS_ERR (cd))return PTR_ERR (cd);
@@ -280,7 +280,7 @@ int register_chrdev(unsigned int major,for (s =strchr (kobject_name (&cdev->kobj),'/' ); s; s =strchr (s,'/' )) *s ='!' ; - err =cdev_add (cdev,MKDEV (cd->major,0 ),256 ); + err =cdev_add (cdev,MKDEV (cd->major, baseminor), count);if (err) goto out;
@@ -290,7 +290,7 @@ int register_chrdev(unsigned int major, out:kobject_put (&cdev->kobj); out2: -kfree (__unregister_chrdev_region (cd->major,0 ,256 )); +kfree (__unregister_chrdev_region (cd->major, baseminor, count));return err; }
@@ -316,10 +316,23 @@ void unregister_chrdev_region(dev_t from } } -void unregister_chrdev (unsigned int major,const char *name) +/** +* __unregister_chrdev - unregister and destroy a cdev +* @major: major device number +* @baseminor: first of the range of minor numbers +* @count: the number of minor numbers this cdev is occupying +* @name: name of this range of devices +* +* Unregister and destroy the cdev occupying the region described by +* @major, @baseminor and @count. This function undoes what +* __register_chrdev() did. +*/ +void __unregister_chrdev (unsigned int major,unsigned int baseminor, +unsigned int count,const char *name) {struct char_device_struct *cd; - cd =__unregister_chrdev_region (major,0 ,256 ); + + cd =__unregister_chrdev_region (major, baseminor, count);if (cd && cd->cdev)cdev_del (cd->cdev);kfree (cd);
@@ -568,6 +581,6 @@ EXPORT_SYMBOL(cdev_alloc);EXPORT_SYMBOL (cdev_del);EXPORT_SYMBOL (cdev_add);EXPORT_SYMBOL (cdev_index); -EXPORT_SYMBOL (register_chrdev); -EXPORT_SYMBOL (unregister_chrdev); +EXPORT_SYMBOL (__register_chrdev); +EXPORT_SYMBOL (__unregister_chrdev);EXPORT_SYMBOL (directly_mappable_cdev_bdi);
include/linux/fs.h +16 -3
@@ -1998,12 +1998,25 @@ extern void bd_release_from_disk(struct#define CHRDEV_MAJOR_HASH_SIZE 255extern int alloc_chrdev_region (dev_t *,unsigned ,unsigned ,const char *);extern int register_chrdev_region (dev_t ,unsigned ,const char *); -extern int register_chrdev (unsigned int ,const char *, -const struct file_operations *); -extern void unregister_chrdev (unsigned int ,const char *); +extern int __register_chrdev (unsigned int major,unsigned int baseminor, +unsigned int count,const char *name, +const struct file_operations *fops); +extern void __unregister_chrdev (unsigned int major,unsigned int baseminor, +unsigned int count,const char *name);extern void unregister_chrdev_region (dev_t ,unsigned );extern void chrdev_show (struct seq_file *,off_t ); +static inline int register_chrdev (unsigned int major,const char *name, +const struct file_operations *fops) +{ +return __register_chrdev (major,0 ,256 , name, fops); +} + +static inline void unregister_chrdev (unsigned int major,const char *name) +{ +__unregister_chrdev (major,0 ,256 , name); +} +/* fs/block_dev.c */ #define BDEVNAME_SIZE 32/* Largest string for a blockdev identifier */ #define BDEVT_SIZE 10/* Largest string for MAJ:MIN for blkdev */
Quoted 2 lines · Tejun Heo
[un]register_chrdev() assume minor range 0-255. This patch adds __ prefixed versions which take @minorbase and @count explicitly.
What's the difference between this and the existing register_chrdev_region() function? Why doesn't that work for you?
What am I missing here?
thanks,
greg k-h
Quoted 8 lines · Greg KH
Quoted 2 lines · Tejun Heo
[un]register_chrdev() assume minor range 0-255. This patch adds __ prefixed versions which take @minorbase and @count explicitly.
What's the difference between this and the existing
register_chrdev_region()function? Why doesn't that work for you?What am I missing here?
The function names in char_dev.c are quite confusing. register_chrdev_region() only registers chrdev region. register_chrdev() registers chrdev region and creates a cdev for the region. :-)
Signature · 1 line
tejun
Quoted 11 lines · Tejun Heo
Quoted 8 lines · Greg KH
Quoted 2 lines · Tejun Heo
[un]register_chrdev() assume minor range 0-255. This patch adds __ prefixed versions which take @minorbase and @count explicitly.
What's the difference between this and the existing
register_chrdev_region()function? Why doesn't that work for you?What am I missing here?
The function names in
char_dev.care quite confusing.
That's an understatement :)
Quoted 3 lines
register_chrdev_region()only registers chrdev region.register_chrdev()registers chrdev region and creates a cdev for the region. :-)
Yes, but you can use register_chrdev_region() and then call cdev_alloc() for the specific cdevs you need, right? It's not the prettiest, but it should work.
thanks,
greg k-h
Quoted 5 lines · Greg KH
Quoted 4 lines · Tejun Heo
Quoted 3 lines · Greg KH
Quoted 2 lines · Tejun Heo
[un]register_chrdev() assume minor range 0-255. This patch adds __ prefixed versions which take @minorbase and @count explicitly.
Quoted 7 lines
Quoted 5 lines
Quoted 4 lines
What's the difference between this and the existing
register_chrdev_region()function? Why doesn't that work for you?What am I missing here?
The function names in
char_dev.care quite confusing.That's an understatement :)
Yeah, it sure is. It should have used a different verb at the very least.
Quoted 7 lines
Quoted 3 lines
register_chrdev_region()only registers chrdev region.register_chrdev()registers chrdev region and creates a cdev for the region. :-)Yes, but you can use
register_chrdev_region()and then callcdev_alloc()for the specific cdevs you need, right? It's not the prettiest, but it should work.
Yeap, I can. That's basically what register_chrdev() is. If it had a better name, adding a new variant would have been much nicer. Having the helper makes sense tho. Names too confusing?
Thanks.
Signature · 1 line
tejun
Quoted 21 lines · Tejun Heo
Quoted 5 lines · Greg KH
Quoted 4 lines · Tejun Heo
Quoted 3 lines · Greg KH
Quoted 2 lines · Tejun Heo
[un]register_chrdev() assume minor range 0-255. This patch adds __ prefixed versions which take @minorbase and @count explicitly.
Quoted 7 lines
Quoted 5 lines
Quoted 4 lines
What's the difference between this and the existing
register_chrdev_region()function? Why doesn't that work for you?What am I missing here?
The function names in
char_dev.care quite confusing.That's an understatement :)
Yeah, it sure is. It should have used a different verb at the very least.
Quoted 4 lines
Quoted 3 lines
register_chrdev_region()only registers chrdev region.register_chrdev()registers chrdev region and creates a cdev for the region. :-)
Quoted 7 lines
Quoted 3 lines
Yes, but you can use
register_chrdev_region()and then callcdev_alloc()for the specific cdevs you need, right? It's not the prettiest, but it should work.Yeap, I can. That's basically what
register_chrdev()is. If it had a better name, adding a new variant would have been much nicer. Having the helper makes sense tho. Names too confusing?
Yes the names are confusing, the whole api needs a rework to make it easier to understand :(
thanks,
greg k-h
Hello,
Quoted 2 lines · Greg KH
Yes the names are confusing, the whole api needs a rework to make it easier to understand :(
For now, I'll open code it then but it would be really nice to clean the names up.
Thanks.
Signature · 1 line
tejun
Quoted 8 lines · Tejun Heo
Hello,
Quoted 2 lines · Greg KH
Yes the names are confusing, the whole api needs a rework to make it easier to understand :(
For now, I'll open code it then but it would be really nice to clean the names up.
Crap, there's a difference. For [un]register_chrdev() the chrdev layer keeps track of cdev while using cdev_alloc/add/del() directly requires the caller to keep track of the pointer. Also, it's a bit of pain to open code the whole thing.
For now, it looks like adding the __ prefixed versions seems to be the proper solution. It's strange to provide the helper interface only for full minor acquisitions anyway. Let's do naming cleanup later. How does that sound?
Thanks.
Signature · 1 line
tejun
Quoted 15 lines · Tejun Heo
Quoted 8 lines · Tejun Heo
Hello,
Quoted 2 lines · Greg KH
Yes the names are confusing, the whole api needs a rework to make it easier to understand :(
For now, I'll open code it then but it would be really nice to clean the names up.
Crap, there's a difference. For [un]register_chrdev() the chrdev layer keeps track of cdev while using cdev_alloc/add/del() directly requires the caller to keep track of the pointer. Also, it's a bit of pain to open code the whole thing.
Quoted 4 lines
For now, it looks like adding the __ prefixed versions seems to be the proper solution. It's strange to provide the helper interface only for full minor acquisitions anyway. Let's do naming cleanup later. How does that sound?
Well, what's the odds that I can get you to do cleanup now? :)
Sure, I'll take this, only if Takashi wants the other stuff that depends on it. Let me know.
thanks,
greg k-h
Hello, Greg.
Quoted 6 lines · Greg KH
Quoted 4 lines
For now, it looks like adding the __ prefixed versions seems to be the proper solution. It's strange to provide the helper interface only for full minor acquisitions anyway. Let's do naming cleanup later. How does that sound?
Well, what's the odds that I can get you to do cleanup now? :)
I'd be happy to but whether cleaning it up would be benefitial or not is a bit unclear. We have 214 users of [un]register_chrdev_region() and 118 users of [un]register_chrdev(). So, renaming the latter to, say, create/destroy_chrdev() and make it take @baseminor and @nr_minors should do it but I'm not quite sure whether it would worth touching all those files. Maybe it's better left as a mess like the page allocation functions?
Quoted 2 lines
Sure, I'll take this, only if Takashi wants the other stuff that depends on it. Let me know.
Thanks.
Signature · 1 line
tejun
Quoted 10 lines · Tejun Heo
Hello, Greg.
Quoted 6 lines · Greg KH
Quoted 4 lines
For now, it looks like adding the __ prefixed versions seems to be the proper solution. It's strange to provide the helper interface only for full minor acquisitions anyway. Let's do naming cleanup later. How does that sound?
Well, what's the odds that I can get you to do cleanup now? :)
Quoted 7 lines
I'd be happy to but whether cleaning it up would be benefitial or not is a bit unclear. We have 214 users of [un]register_chrdev_region() and 118 users of [un]register_chrdev(). So, renaming the latter to, say, create/destroy_chrdev() and make it take @baseminor and @nr_minors should do it but I'm not quite sure whether it would worth touching all those files. Maybe it's better left as a mess like the page allocation functions?
Yeah, I guess it's easier to leave it alone for now. If I ever get bored, I'll work on cleaning them up :)
thanks,
greg k-h