Re: alloc_chrdev_region/unregister_chrdev_region - Issue

["Chetan Nanda" <chetannanda@xxxxxxxxx>]

On Mon, Dec 8, 2008 at 9:35 PM, Sandeep K Sinha <sandeepksinha@xxxxxxxxx> wrote:

On Mon, Dec 8, 2008 at 7:45 PM, Chetan Nanda <chetannanda@xxxxxxxxx> wrote:
> Hi,
>
> I am using 'alloc_chrdev_region' for registering char device range and then
> 'unregister_chrdev_region' for un-registering it.
> But on each insmod I am getting MAJOR number as one less then that of
> previous time.

 /* temporary */
 96         if (major == 0) {
 97                 for (i = ARRAY_SIZE(chrdevs)-1; i > 0; i--) {
 98                         if (chrdevs[i] == NULL)
 99                                 break;
100                 }
101
102                 if (i == 0) {
103                         ret = -EBUSY;
104                         goto out;
105                 }
106                 major = i;
107                 ret = major;
108         }
109

REF: http://lxr.oss.org.cn/source/fs/char_dev.c#L80

>
> On seeing the code (unregister_chrdev_region) it seems that it just kfree
> the chrdevs[] entry but not set it to NULL which cause for new
> entry in chrdevs[].
>

Thats right, it is like that.
Dont have much idea regarding the same. I think we should assign a
NULL to it for sure.

I have tried with setting chrdevs entry to NULL after kfree but it didn't help

void unregister_chrdev_region(dev_t from, unsigned count)
{
  dev_t to = from + count;
  dev_t n, next;

  for (n = from; n < to; n = next) {
    next = MKDEV(MAJOR(n)+1, 0);
    if (next > to)
      next = to;
    kfree(__unregister_chrdev_region(MAJOR(n), MINOR(n), next - n));
  }
  /*I do it*/
  chrdevs[MAJOR(n)] = NULL; //<--- setting it to null here
}
 
It seems that problem is in my test driver implementation.
Here are my init and clean-up function:

int __init
cmos_init(void)
{
    int i;

    /*Clear cmosdev_array struct*/
    memset((void *)&cmosdev_array, 0 , sizeof(struct cmos_dev*) * NUM_CMOS_BANKS);   

    /* Request dynamic allocation of a device major number */
    if (alloc_chrdev_region(&cmos_dev_number, 0, NUM_CMOS_BANKS, DEVICE_NAME) < 0)
    {
        printk(KERN_DEBUG "Can't register device\n"); return -1;
    }

    /* Populate sysfs entries */
    cmos_class = class_create(THIS_MODULE, DEVICE_NAME);

    for (i=0; i<NUM_CMOS_BANKS; i++)
    {
        /* Allocate memory for the per-device structure */
        cmos_devp = kmalloc(sizeof(struct cmos_dev), GFP_KERNEL);
        if (!cmos_devp)
        {
            printk("Bad Kmalloc\n"); return 1;
        }
        cmosdev_array[i] = cmos_devp;

        /* Request I/O region */
        sprintf(cmos_devp->name, "cmos%d", i);
        if (!(request_region(addrports[i], 2, cmos_devp->name)))
        {
            printk("cmos: I/O port 0x%x is not free.\n", addrports[i]);
            return -EIO;
        }

        /* Fill in the bank number to correlate this device
         *         with the corresponding CMOS bank */
        cmos_devp->bank_number = i;

        /* Connect the file operations with the cdev */
        cdev_init(&cmos_devp->cdev, &cmos_fops);
        cmos_devp->cdev.owner = THIS_MODULE;

        /* Connect the major/minor number to the cdev */
        if(cdev_add(&cmos_devp->cdev,(cmos_dev_number + i),1))
        {
            printk("Bad cdev\n");
            return 1;
        }

        /* Send uevents to udev, so it'll create /dev nodes */
        class_device_create(cmos_class, NULL, (cmos_dev_number + i),NULL, "cmos%d", i);
    }
    printk("CMOS Driver Initialized.\n");
    return 0;
}

//clean-up function
/* Driver Exit */
void __exit
cmos_cleanup(void)
{
    int i;
   
    /* Release the major number */
    unregister_chrdev_region(MAJOR(cmos_dev_number), NUM_CMOS_BANKS);
   
    /* Release I/O region */
    for (i=0; i<NUM_CMOS_BANKS; i++)
    {
        /* Remove the cdev */
        cdev_del(&(cmosdev_array[i]->cdev));   
        class_device_destroy(cmos_class, MKDEV(MAJOR(cmos_dev_number), i));
        release_region(addrports[i], 2);
        kfree(cmosdev_array[i]);
    }

    /* Destroy cmos_class */
    class_destroy(cmos_class);
    return;
}

Please provide me any feedback on this piece of code.

> Thanks,
> Chetan Nanda
>



--
Regards,
Sandeep.






"To learn is to change. Education is a process that changes the learner."