diff mbox

[1/3] gigaset: avoid registering CAPI driver more than once

Message ID 1268664986.25524@pingi
State Not Applicable, archived
Delegated to: David Miller
Headers show

Commit Message

Karsten Keil March 15, 2010, 2:48 p.m. UTC
Registering/unregistering the Gigaset CAPI driver when a device is
connected/disconnected causes an Oops when disconnecting two Gigaset
devices in a row, because the same capi_driver structure gets
unregistered twice. Fix by making driver registration/unregistration
a separate operation (empty in the ISDN4Linux case) called when the
main module is loaded/unloaded.

Impact: bugfix
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
Acked-by: Karsten Keil <keil@b1-systems.de>

---
Note to -stable: applies correctly to 2.6.33 with fuzz 2 in capi.c.

 drivers/isdn/gigaset/capi.c    |   44 ++++++++++++++++++++++-----------------
 drivers/isdn/gigaset/common.c  |    6 +++-
 drivers/isdn/gigaset/gigaset.h |    6 +++-
 drivers/isdn/gigaset/i4l.c     |   28 ++++++++++++++++++-------
 4 files changed, 53 insertions(+), 31 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

David Miller March 15, 2010, 9:20 p.m. UTC | #1
Karsten the way you reforward patches does not work.

And I've told you about this last time you submitted ISDN patches.

You don't add an extra "From: " line in the message body, so when the
patch gets applied the author gets set to _you_ instead of the person
who actually wrote the change.

All you're doing is ACK'ing this person's work, so simply reply to the
patch they posted and with your "Acked-by" line.  This way patchwork
and friends will figure out the rest when the patch gets integrated
into the net-2.6 tree.

By reposting the patches the way you are we're losing information.

Add to this the fact that you never have the time to properly take
care of ISDN patches, and when you do finally "get to it" you make all
kinds of submission errors.

This is not the first time either, it happens over and over again.
It's incredibly frustrating, especially for me.

Please seriously consider handing ISDN maintainership over to someone
who unlike you 1) has the time and the desire and 2) knows how to
submit patches properly.  Probably Tilman is the person who most meets
these criteria based upon what I've seen.

Meanwhile, I'm going to apply the original patches as posted by
Tilman so that the proper authorship gets set.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Karsten Keil March 16, 2010, 12:23 a.m. UTC | #2
On Montag, 15. März 2010 22:20:40 David Miller wrote:
> Karsten the way you reforward patches does not work.
> 
> And I've told you about this last time you submitted ISDN patches.
> 
> You don't add an extra "From: " line in the message body, so when the
> patch gets applied the author gets set to _you_ instead of the person
> who actually wrote the change.
>
Sorry about that.

> 
> All you're doing is ACK'ing this person's work, so simply reply to the
> patch they posted and with your "Acked-by" line.  This way patchwork
> and friends will figure out the rest when the patch gets integrated
> into the net-2.6 tree.

I tried to use my own patchwork which is feed  by the
ISDN list to learn more about it, unfortunately it does not help for such 
issues.

> 
> By reposting the patches the way you are we're losing information.
> 
> Add to this the fact that you never have the time to properly take
> care of ISDN patches, and when you do finally "get to it" you make all
> kinds of submission errors.
> 
> This is not the first time either, it happens over and over again.
> It's incredibly frustrating, especially for me.
I can understand that.
> Please seriously consider handing ISDN maintainership over to someone
> who unlike you 1) has the time and the desire and 2) knows how to
> submit patches properly.  Probably Tilman is the person who most meets
> these criteria based upon what I've seen.
> 
> Meanwhile, I'm going to apply the original patches as posted by
> Tilman so that the proper authorship gets set.

Fair enough, thanks.

Karsten
- who had really a very bad day

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/isdn/gigaset/capi.c b/drivers/isdn/gigaset/capi.c
index 6643d65..4a31962 100644
--- a/drivers/isdn/gigaset/capi.c
+++ b/drivers/isdn/gigaset/capi.c
@@ -2191,36 +2191,24 @@  static const struct file_operations gigaset_proc_fops = {
 	.release	= single_release,
 };
 
-static struct capi_driver capi_driver_gigaset = {
-	.name		= "gigaset",
-	.revision	= "1.0",
-};
-
 /**
- * gigaset_isdn_register() - register to LL
+ * gigaset_isdn_regdev() - register device to LL
  * @cs:		device descriptor structure.
  * @isdnid:	device name.
  *
- * Called by main module to register the device with the LL.
- *
  * Return value: 1 for success, 0 for failure
  */
-int gigaset_isdn_register(struct cardstate *cs, const char *isdnid)
+int gigaset_isdn_regdev(struct cardstate *cs, const char *isdnid)
 {
 	struct gigaset_capi_ctr *iif;
 	int rc;
 
-	pr_info("Kernel CAPI interface\n");
-
 	iif = kmalloc(sizeof(*iif), GFP_KERNEL);
 	if (!iif) {
 		pr_err("%s: out of memory\n", __func__);
 		return 0;
 	}
 
-	/* register driver with CAPI (ToDo: what for?) */
-	register_capi_driver(&capi_driver_gigaset);
-
 	/* prepare controller structure */
 	iif->ctr.owner         = THIS_MODULE;
 	iif->ctr.driverdata    = cs;
@@ -2241,7 +2229,6 @@  int gigaset_isdn_register(struct cardstate *cs, const char *isdnid)
 	rc = attach_capi_ctr(&iif->ctr);
 	if (rc) {
 		pr_err("attach_capi_ctr failed (%d)\n", rc);
-		unregister_capi_driver(&capi_driver_gigaset);
 		kfree(iif);
 		return 0;
 	}
@@ -2252,17 +2239,36 @@  int gigaset_isdn_register(struct cardstate *cs, const char *isdnid)
 }
 
 /**
- * gigaset_isdn_unregister() - unregister from LL
+ * gigaset_isdn_unregdev() - unregister device from LL
  * @cs:		device descriptor structure.
- *
- * Called by main module to unregister the device from the LL.
  */
-void gigaset_isdn_unregister(struct cardstate *cs)
+void gigaset_isdn_unregdev(struct cardstate *cs)
 {
 	struct gigaset_capi_ctr *iif = cs->iif;
 
 	detach_capi_ctr(&iif->ctr);
 	kfree(iif);
 	cs->iif = NULL;
+}
+
+static struct capi_driver capi_driver_gigaset = {
+	.name		= "gigaset",
+	.revision	= "1.0",
+};
+
+/**
+ * gigaset_isdn_regdrv() - register driver to LL
+ */
+void gigaset_isdn_regdrv(void)
+{
+	pr_info("Kernel CAPI interface\n");
+	register_capi_driver(&capi_driver_gigaset);
+}
+
+/**
+ * gigaset_isdn_unregdrv() - unregister driver from LL
+ */
+void gigaset_isdn_unregdrv(void)
+{
 	unregister_capi_driver(&capi_driver_gigaset);
 }
diff --git a/drivers/isdn/gigaset/common.c b/drivers/isdn/gigaset/common.c
index 85de339..bdc01cb 100644
--- a/drivers/isdn/gigaset/common.c
+++ b/drivers/isdn/gigaset/common.c
@@ -507,7 +507,7 @@  void gigaset_freecs(struct cardstate *cs)
 	case 2: /* error in initcshw */
 		/* Deregister from LL */
 		make_invalid(cs, VALID_ID);
-		gigaset_isdn_unregister(cs);
+		gigaset_isdn_unregdev(cs);
 
 		/* fall through */
 	case 1: /* error when registering to LL */
@@ -769,7 +769,7 @@  struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
 	cs->cmdbytes = 0;
 
 	gig_dbg(DEBUG_INIT, "setting up iif");
-	if (!gigaset_isdn_register(cs, modulename)) {
+	if (!gigaset_isdn_regdev(cs, modulename)) {
 		pr_err("error registering ISDN device\n");
 		goto error;
 	}
@@ -1205,11 +1205,13 @@  static int __init gigaset_init_module(void)
 		gigaset_debuglevel = DEBUG_DEFAULT;
 
 	pr_info(DRIVER_DESC DRIVER_DESC_DEBUG "\n");
+	gigaset_isdn_regdrv();
 	return 0;
 }
 
 static void __exit gigaset_exit_module(void)
 {
+	gigaset_isdn_unregdrv();
 }
 
 module_init(gigaset_init_module);
diff --git a/drivers/isdn/gigaset/gigaset.h b/drivers/isdn/gigaset/gigaset.h
index 1875ab8..cdd144e 100644
--- a/drivers/isdn/gigaset/gigaset.h
+++ b/drivers/isdn/gigaset/gigaset.h
@@ -675,8 +675,10 @@  int gigaset_isowbuf_getbytes(struct isowbuf_t *iwb, int size);
  */
 
 /* Called from common.c for setting up/shutting down with the ISDN subsystem */
-int gigaset_isdn_register(struct cardstate *cs, const char *isdnid);
-void gigaset_isdn_unregister(struct cardstate *cs);
+void gigaset_isdn_regdrv(void);
+void gigaset_isdn_unregdrv(void);
+int gigaset_isdn_regdev(struct cardstate *cs, const char *isdnid);
+void gigaset_isdn_unregdev(struct cardstate *cs);
 
 /* Called from hardware module to indicate completion of an skb */
 void gigaset_skb_sent(struct bc_state *bcs, struct sk_buff *skb);
diff --git a/drivers/isdn/gigaset/i4l.c b/drivers/isdn/gigaset/i4l.c
index f0acb9d..c22e5ac 100644
--- a/drivers/isdn/gigaset/i4l.c
+++ b/drivers/isdn/gigaset/i4l.c
@@ -592,15 +592,13 @@  void gigaset_isdn_stop(struct cardstate *cs)
 }
 
 /**
- * gigaset_isdn_register() - register to LL
+ * gigaset_isdn_regdev() - register to LL
  * @cs:		device descriptor structure.
  * @isdnid:	device name.
  *
- * Called by main module to register the device with the LL.
- *
  * Return value: 1 for success, 0 for failure
  */
-int gigaset_isdn_register(struct cardstate *cs, const char *isdnid)
+int gigaset_isdn_regdev(struct cardstate *cs, const char *isdnid)
 {
 	isdn_if *iif;
 
@@ -650,15 +648,29 @@  int gigaset_isdn_register(struct cardstate *cs, const char *isdnid)
 }
 
 /**
- * gigaset_isdn_unregister() - unregister from LL
+ * gigaset_isdn_unregdev() - unregister device from LL
  * @cs:		device descriptor structure.
- *
- * Called by main module to unregister the device from the LL.
  */
-void gigaset_isdn_unregister(struct cardstate *cs)
+void gigaset_isdn_unregdev(struct cardstate *cs)
 {
 	gig_dbg(DEBUG_CMD, "sending UNLOAD");
 	gigaset_i4l_cmd(cs, ISDN_STAT_UNLOAD);
 	kfree(cs->iif);
 	cs->iif = NULL;
 }
+
+/**
+ * gigaset_isdn_regdrv() - register driver to LL
+ */
+void gigaset_isdn_regdrv(void)
+{
+	/* nothing to do */
+}
+
+/**
+ * gigaset_isdn_unregdrv() - unregister driver from LL
+ */
+void gigaset_isdn_unregdrv(void)
+{
+	/* nothing to do */
+}