confused by char dev registration in a gpio driver

[Jim Cromie]

hi Robert,

On Tue, May 3, 2011 at 3:21 PM, Robert P. J. Day <rpjday at crashcourse.ca> wrote:
>
>  i'm sure i'm going to embarrass myself here but i was perusing the
> char drivers for nice examples, and i ran across this excerpt in
> pc8736x_gpio.c:

Thanks for taking a look, and questioning what looks wrong.

>
> ===== begin =====
>
>        if (major) {
>                devid = MKDEV(major, 0);
>                rc = register_chrdev_region(devid, PC8736X_GPIO_CT, DEVNAME);
>        } else {
>                rc = alloc_chrdev_region(&devid, 0, PC8736X_GPIO_CT, DEVNAME);
>                major = MAJOR(devid);
>        }
>

so major is set here, either before the if, or as result of the else.

>        if (rc < 0) {
>                dev_err(&pdev->dev, "register-chrdev failed: %d\n", rc);
>                goto undo_request_region;
>        }

so when we get here, rc==0, and major is set
FWIW, it looks like I poached the above code from LDD3,
which explains why it looks right up to here..

Id agree that theres something wrong/unclear below.
major = rc looks kinda rotten.

Given that the only purpose was to debug-print,
and the "dynamic" fact is wrong, I think I'll prep a patch to yank that crap.
Unless you beat me to it :-)

>        if (!major) {
>                major = rc;
>                dev_dbg(&pdev->dev, "got dynamic major %d\n", major);
>        }
>
> ===== end =====
>
>  i'm good with most of that -- if the (parameter) major is explicit,
> then a dev_t of "devid" is created and register_chrdev_region() is
> used.
>
>  on the other hand, if major is zero, then alloc_chrdev_region() is
> used for *dynamic* allocation of the major number.  in both cases, the
> return code "rc" is saved and, if it's < 0, we have an error.  and
> that's where the confusion comes in.
>
>  i always thought both of those routines returned a simple zero to
> indicate success.  but look at those last few lines -- that return
> code is assigned to "major", at which point it's *that* value that's
> printed.  wouldn't that just be zero all the time?
>
>  and wouldn't it also print that this was a "dynamic" major even if
> the user specified an explicit major number at load time?  this second
> point is more nitpicky, but what about that first point?  wouldn't a
> successful registration always print an allocated major number of
> zero?
>
> rday
>

again, thanks.
~jimc