[patchset 0/3 -2.6.18-rc1] pc8736x_gpio: fix re-modprobe errors

[Jim Cromie]

Linus Torvalds wrote:
> Ok,
>  the merge window for 2.6.18 is closed, and -rc1 is out there (git trees 
>   


The new pc8736x_gpio driver has at least 1 bug, which manifests when
module is rmmod'd then re-modprobe'd.  This 3 part patchset addresses it 
as follows:

1 - add constant defines - preparatory patch

- adds #define CONSTs  for max-pin,  gpio-addr-range (for reserving region)
- fix wrong max-pin check in gpio_open()
- add 'Winbond' to module description:  NSC sold the product, Winbond 
has us / lm-sensors

       pc-const-decl      - define several symbolic constants
       pc-open-const    - use reduced max-pin to what hardware supports
       pc-req-region-const        - can wait, but why
       pc-mod-desc-add-winbond    -

since this is just preparatory work, it still exhibits the bug:

soekris:~# rmmod pc8736x_gpio
soekris:~# modprobe pc8736x_gpio
[  498.970735] kobject_add failed for pc8736x_gpio.0 with -EEXIST, don't 
try to register things with the same name in the same directory.
[  498.985568]  [<c0102fbf>] show_trace_log_lvl+0x54/0xfd
[  498.991398]  [<c0103fba>] show_trace+0xd/0x10
[  498.996629]  [<c0103fd4>] dump_stack+0x17/0x1b
[  499.001759]  [<c01d1c6a>] kobject_add+0x17d/0x19c
[  499.014898]  [<c0203362>] device_add+0x7a/0x2cc
[  499.029917]  [<c020616d>] platform_device_add+0xdc/0x10c
[  499.045902]  [<c8820028>] pc8736x_gpio_init+0x28/0x258 [pc8736x_gpio]
[  499.053212]  [<c012a67a>] sys_init_module+0x1381/0x1463
[  499.061327]  [<c0102657>] syscall_call+0x7/0xb
FATAL: Error inserting pc8736x_gpio 
(/lib/modules/2.6.18-rc1-jimc-sk/kernel/drivers/char/pc8736x_gpio.ko): 
No such device


2 - pc-init-fix-undo-region-rollup

this patch fixes module-init-func by repairing usage of 
platform_device_del/put in module-exit-func,
ie it imitates Ingo's 'mishaps' patch, which fixed the 
module-init-func's undo handling.
Also fixes lack of release_region to undo the earlier registration.

Also included:
- add include <linux/cdev.h> - needed by next patches
and cuz theyre within the diff-context-window:
- remove include <linux/config.h>      everyone's doing it
- copyright updates - current date is 'wrong'

the patch fixes the above bug, reducing it to a different one; (which is 
fixed by 3rd patch)

soekris:~# rmmod pc8736x_gpio
soekris:~# modprobe pc8736x_gpio
[  283.618997] platform pc8736x_gpio.0: NatSemi pc8736x GPIO Driver 
Initializing
[  283.626534] platform pc8736x_gpio.0: GPIO ioport 6600 reserved
[  283.633319] platform pc8736x_gpio.0: register-chrdev failed: -16
FATAL: Error inserting pc8736x_gpio 
(/lib/modules/2.6.18-rc1-jimc-sk/kernel/drivers/char/pc8736x_gpio.ko): 
Device or resource busy


3 - pc-init-fix-chrdev-region-rollup

- Switch from register_chrdev() to   (register|alloc)_chrdev_region().

- use a cdev.   This was intended for original patchset, but was overlooked.
We use a single cdev for all pins (minor device-numbers), as gleaned 
from cs5535_gpio,
and in contrast to whats currently done in scx200_gpio (which I'll fix soon)

By some reasoning, this patch should have been combined with patch 2; 
the fixes
are to the same 2 mod-init/exit functions, and both touch cdevs, 
regions, etc.
Despite this, I separated them (incompletely).  I'll combine them if 
thats better..


Given that this is a brand new driver, can I assume a bit more latitude to
clean up and normalize the code ?     For example, s/DEVNAME/DRVNAME/
would improve consistency with other drivers.  The scx200_gpio driver 
could also
stand some improvements; a single cdev, and same DRVNAME normalization.

Whats the right way to mark this as (NEW/EXPERIMENTAL) ?
Is either appropriate for a reworked / existing driver (scx200_gpio)

cc'g  AKPM since (I think) he pushed this driver to mainline.

thanks
Jim Cromie
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/