confused by char dev registration in a gpio driver
Greg KH
greg at kroah.com
Tue May 10 14:40:05 EDT 2011
On Tue, May 10, 2011 at 11:26:59AM -0600, Jim Cromie wrote:
> lets start with a list of grumbles about current api ?
>
> 1 - register_chrdev_region() VS alloc_chrdev_region()
> ISTM that these could be combined into 1 function:
>
> dev_t devid = MKNOD(major, minor);
> register_chrdev_numbers(&devid, count, name);
>
> ie make &devid an in/out parameter, where in-value
> distinguishes which alloc/register behavior is desired.
>
> this looks like a minor variation on alloc_chrdev_region()
> one downside is need to include kdev_t.h
>
> 2 - EXPORTs in drivers/char/char_dev.c
> original version (~2.6.12-rc2) had no __double-under exports,
> now have:
> EXPORT_SYMBOL(__register_chrdev);
> EXPORT_SYMBOL(__unregister_chrdev);
>
> __register_chrdev() looks to be a higher level function;
> it 1st calls __register_chrdev_region()
> then does cdev, kobject stuff too.
>
> Or, why does it have __ in the name ?
> __ typically means not for external use, but its exported.
> is it because it does the cdev_add "too early"
> ie before the caller is really ready to handle service requests ?
> theres no caveat mentioned in the /** comments
>
> Are the insanities you alluded to of a different sort,
> ie internal suboptimalities ?
No, those you have listed are all things I was talking about.
And yes, there are also some internal issues as well (see the kobject
mapping stuff).
> - struct inefficiencies ?
> struct cdev *cdev; /* will die */
> why die ? not needed, or available in another struct ?
>
> - in char_dev.c, there is
>
> static struct char_device_struct {
> ...} *chrdevs[CHRDEV_MAJOR_HASH_SIZE];
> ie 255 elements.
>
> This is == to legacy MAJOR number space,
> and is scanned 255..0 to find unused MAJOR number.
> It will need new const if MAJOR range increases,
> but doesnt waste much memory currently
>
> My 32bit laptop uses 48 devices, so ~200 major slots are
> empty/wasted (800 bytes) and ISTM like over-engineering
> to fix this preemptively if >255 is unneeded.
> OTOH, a hash or rb-tree could use only whats needed.
> is there a 'library' hash implementation in the kernel ?
The kobject map stuff should handle most of this, but really, 800 bytes
isn't that much overhead for the speed you get, right? Changing the
code to use something else would take more than 800 bytes from what I
can see.
Let's get the api fixed up first, that's the most important thing as
it's what people use all the time.
I'm going to have some long-distance flights soon, so I'll try to work
on this then, thanks for the great summary above, I appreciate it.
thanks,
greg k-h
More information about the Kernelnewbies
mailing list