public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jim Cromie <jim.cromie@gmail.com>
To: Linus Torvalds <torvalds@osdl.org>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@osdl.org>
Subject: [patchset 0/3 -2.6.18-rc1]  pc8736x_gpio:  fix re-modprobe errors
Date: Sat, 08 Jul 2006 08:09:57 -0600	[thread overview]
Message-ID: <44AFBCB5.4040409@gmail.com> (raw)
In-Reply-To: <Pine.LNX.4.64.0607052115210.12404@g5.osdl.org>

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

  parent reply	other threads:[~2006-07-08 14:09 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-07-06  4:26 Linux v2.6.18-rc1 Linus Torvalds
2006-07-06  9:44 ` Matt Keenan
2006-07-06 12:49   ` Jonathan Corbet
2006-07-06 16:22   ` Linus Torvalds
2006-07-06 12:34 ` Alistair John Strachan
2006-07-07  0:11   ` Andrew Morton
2006-07-06 12:44 ` Nigel Cunningham
2006-07-06 19:44 ` David R
2006-07-06 22:17   ` Greg KH
2006-07-07 21:11     ` David R
2006-07-08  8:14       ` Borislav Petkov
2006-07-08 14:19       ` Kay Sievers
2006-07-08 14:44         ` Andi Kleen
2006-07-08 14:44     ` Andi Kleen
2006-07-08 16:02       ` Greg KH
2006-07-09 23:17         ` Andi Kleen
2006-07-09 23:36           ` Greg KH
2006-07-07  1:05   ` Hervé Fache
2006-07-07 15:41 ` Steve Fox
2006-07-09 10:34   ` Benjamin Herrenschmidt
2006-07-10 13:21     ` Will Schmidt
2006-07-10 14:40       ` Alan Cox
2006-07-10 21:17         ` Benjamin Herrenschmidt
2006-07-11 17:00           ` Steve Fox
2006-07-10 21:16       ` Benjamin Herrenschmidt
2006-07-10 16:38     ` Steve Fox
2006-07-10 18:30       ` Steve Fox
2006-07-07 17:52 ` lost cpufreq (Re: Linux v2.6.18-rc1) Tomasz Torcz
2006-07-07 19:07   ` Dave Jones
2006-07-07 20:27     ` Tomasz Torcz
2006-07-07 20:35       ` Dave Jones
2006-07-10 10:03   ` Takashi Iwai
2006-07-08 14:09 ` Jim Cromie [this message]
2006-07-08 14:16   ` [patchset 1/3 -2.6.18-rc1] pc8736x_gpio: fix re-modprobe errors - define and use constants Jim Cromie
2006-07-08 14:29   ` [patchset 2/3 -2.6.18-rc1] pc8736x_gpio: fix re-modprobe errors - undo region reservation Jim Cromie
2006-07-08 14:34   ` [patchset 3/3 -2.6.18-rc1] pc8736x_gpio: fix re-modprobe errors - fix/finish cdev-init Jim Cromie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=44AFBCB5.4040409@gmail.com \
    --to=jim.cromie@gmail.com \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@osdl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox