linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] ISDN patches for net-next
@ 2012-04-25 23:02 Tilman Schmidt
  2012-04-25 23:02 ` [PATCH 1/8] isdn/gigaset: ratelimit CAPI message dumps Tilman Schmidt
                   ` (8 more replies)
  0 siblings, 9 replies; 15+ messages in thread
From: Tilman Schmidt @ 2012-04-25 23:02 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel

Karsten, David,

here's a series of eight patches to the Kernel CAPI subsystem and
the Gigaset driver. Please consider for application to net-next.
The first three should also go into -stable and are tagged accordingly.

Thanks,
Tilman

Tilman Schmidt (8):
      isdn/gigaset: ratelimit CAPI message dumps
      isdn/gigaset: fix CAPI disconnect B3 handling
      isdn/gigaset: improve error handling querying firmware version
      isdn/gigaset: fix readability damage 
      isdn/gigaset: internal function name cleanup
      isdn/gigaset: unify function return values
      isdn/capi: fix readability damage
      isdn/capi: elliminate capincci_find() in non-middleware case

 drivers/isdn/capi/capi.c           |   50 ++---
 drivers/isdn/capi/capidrv.c        |    8 
 drivers/isdn/gigaset/bas-gigaset.c |   44 ++---
 drivers/isdn/gigaset/capi.c        |  118 ++++++-------
 drivers/isdn/gigaset/common.c      |   59 ++----
 drivers/isdn/gigaset/dummyll.c     |    2 
 drivers/isdn/gigaset/ev-layer.c    |  321 ++++++++++++++++++-------------------
 drivers/isdn/gigaset/gigaset.h     |   30 +--
 drivers/isdn/gigaset/i4l.c         |   12 -
 drivers/isdn/gigaset/isocdata.c    |   12 -
 drivers/isdn/gigaset/ser-gigaset.c |   21 +-
 drivers/isdn/gigaset/usb-gigaset.c |   19 +-
 12 files changed, 341 insertions(+), 355 deletions(-)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/8] isdn/gigaset: ratelimit CAPI message dumps
  2012-04-25 23:02 [PATCH 0/8] ISDN patches for net-next Tilman Schmidt
@ 2012-04-25 23:02 ` Tilman Schmidt
  2012-04-26  6:39   ` Karsten Keil
  2012-04-25 23:02 ` [PATCH 5/8] isdn/gigaset: internal function name cleanup Tilman Schmidt
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Tilman Schmidt @ 2012-04-25 23:02 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel

Introduce a global ratelimit for CAPI message dumps to protect
against possible log flood.
Drop the ratelimit for ignored messages which is now covered by the
global one.

Signed-off-by: Tilman Schmidt <tilman@imap.cc>
CC: stable <stable@kernel.org>
---
 drivers/isdn/gigaset/capi.c |   22 +++++++++-------------
 1 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/isdn/gigaset/capi.c b/drivers/isdn/gigaset/capi.c
index 343b5c8..292ca2f 100644
--- a/drivers/isdn/gigaset/capi.c
+++ b/drivers/isdn/gigaset/capi.c
@@ -14,6 +14,7 @@
 #include "gigaset.h"
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
+#include <linux/ratelimit.h>
 #include <linux/isdn/capilli.h>
 #include <linux/isdn/capicmd.h>
 #include <linux/isdn/capiutil.h>
@@ -223,10 +224,14 @@ get_appl(struct gigaset_capi_ctr *iif, u16 appl)
 static inline void dump_cmsg(enum debuglevel level, const char *tag, _cmsg *p)
 {
 #ifdef CONFIG_GIGASET_DEBUG
+	/* dump at most 20 messages in 20 secs */
+	static DEFINE_RATELIMIT_STATE(msg_dump_ratelimit, 20 * HZ, 20);
 	_cdebbuf *cdb;
 
 	if (!(gigaset_debuglevel & level))
 		return;
+	if (!___ratelimit(&msg_dump_ratelimit, tag))
+		return;
 
 	cdb = capi_cmsg2str(p);
 	if (cdb) {
@@ -2059,12 +2064,6 @@ static void do_reset_b3_req(struct gigaset_capi_ctr *iif,
 }
 
 /*
- * dump unsupported/ignored messages at most twice per minute,
- * some apps send those very frequently
- */
-static unsigned long ignored_msg_dump_time;
-
-/*
  * unsupported CAPI message handler
  */
 static void do_unsupported(struct gigaset_capi_ctr *iif,
@@ -2073,8 +2072,7 @@ static void do_unsupported(struct gigaset_capi_ctr *iif,
 {
 	/* decode message */
 	capi_message2cmsg(&iif->acmsg, skb->data);
-	if (printk_timed_ratelimit(&ignored_msg_dump_time, 30 * 1000))
-		dump_cmsg(DEBUG_CMD, __func__, &iif->acmsg);
+	dump_cmsg(DEBUG_CMD, __func__, &iif->acmsg);
 	send_conf(iif, ap, skb, CapiMessageNotSupportedInCurrentState);
 }
 
@@ -2085,11 +2083,9 @@ static void do_nothing(struct gigaset_capi_ctr *iif,
 		       struct gigaset_capi_appl *ap,
 		       struct sk_buff *skb)
 {
-	if (printk_timed_ratelimit(&ignored_msg_dump_time, 30 * 1000)) {
-		/* decode message */
-		capi_message2cmsg(&iif->acmsg, skb->data);
-		dump_cmsg(DEBUG_CMD, __func__, &iif->acmsg);
-	}
+	/* decode message */
+	capi_message2cmsg(&iif->acmsg, skb->data);
+	dump_cmsg(DEBUG_CMD, __func__, &iif->acmsg);
 	dev_kfree_skb_any(skb);
 }
 
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/8] isdn/gigaset: fix CAPI disconnect B3 handling
  2012-04-25 23:02 [PATCH 0/8] ISDN patches for net-next Tilman Schmidt
  2012-04-25 23:02 ` [PATCH 1/8] isdn/gigaset: ratelimit CAPI message dumps Tilman Schmidt
  2012-04-25 23:02 ` [PATCH 5/8] isdn/gigaset: internal function name cleanup Tilman Schmidt
@ 2012-04-25 23:02 ` Tilman Schmidt
  2012-04-25 23:02 ` [PATCH 7/8] isdn/capi: fix readability damage Tilman Schmidt
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Tilman Schmidt @ 2012-04-25 23:02 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel

If DISCONNECT_B3_IND was synthesized because of a DISCONNECT_REQ
with existing logical connections, the connection state wasn't
updated accordingly. Also the emitted DISCONNECT_B3_IND message
wasn't included in the debug log as requested.
This patch fixes both of these issues.

Signed-off-by: Tilman Schmidt <tilman@imap.cc>
CC: stable <stable@kernel.org>
---
 drivers/isdn/gigaset/capi.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/isdn/gigaset/capi.c b/drivers/isdn/gigaset/capi.c
index 292ca2f..579aa02 100644
--- a/drivers/isdn/gigaset/capi.c
+++ b/drivers/isdn/gigaset/capi.c
@@ -1887,6 +1887,9 @@ static void do_disconnect_req(struct gigaset_capi_ctr *iif,
 
 	/* check for active logical connection */
 	if (bcs->apconnstate >= APCONN_ACTIVE) {
+		/* clear it */
+		bcs->apconnstate = APCONN_SETUP;
+
 		/*
 		 * emit DISCONNECT_B3_IND with cause 0x3301
 		 * use separate cmsg structure, as the content of iif->acmsg
@@ -1911,6 +1914,7 @@ static void do_disconnect_req(struct gigaset_capi_ctr *iif,
 		}
 		capi_cmsg2message(b3cmsg,
 				  __skb_put(b3skb, CAPI_DISCONNECT_B3_IND_BASELEN));
+		dump_cmsg(DEBUG_CMD, __func__, b3cmsg);
 		kfree(b3cmsg);
 		capi_ctr_handle_message(&iif->ctr, ap->id, b3skb);
 	}
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/8] isdn/gigaset: improve error handling querying firmware version
  2012-04-25 23:02 [PATCH 0/8] ISDN patches for net-next Tilman Schmidt
                   ` (4 preceding siblings ...)
  2012-04-25 23:02 ` [PATCH 4/8] isdn/gigaset: " Tilman Schmidt
@ 2012-04-25 23:02 ` Tilman Schmidt
  2012-04-25 23:02 ` [PATCH 8/8] isdn/capi: elliminate capincci_find() in non-middleware case Tilman Schmidt
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Tilman Schmidt @ 2012-04-25 23:02 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel

An out-of-place "OK" response to the "AT+GMR" (get firmware version)
command turns out to be, more often than not, a delayed response to
a previous command rather than an actual error, so continue waiting
for the version number in that case.

Signed-off-by: Tilman Schmidt <tilman@imap.cc>
CC: stable <stable@kernel.org>
---
 drivers/isdn/gigaset/ev-layer.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/isdn/gigaset/ev-layer.c b/drivers/isdn/gigaset/ev-layer.c
index 624a825..685638a 100644
--- a/drivers/isdn/gigaset/ev-layer.c
+++ b/drivers/isdn/gigaset/ev-layer.c
@@ -190,6 +190,7 @@ struct reply_t gigaset_tab_nocid[] =
 								  ACT_INIT} },
 	{RSP_OK,	121, 121, -1,			  0,  0, {ACT_GOTVER,
 								  ACT_INIT} },
+	{RSP_NONE,	121, 121, -1,			120,  0, {ACT_GETSTRING} },
 
 /* leave dle mode */
 	{RSP_INIT,	  0,   0, SEQ_DLE0,		201,  5, {0},	"^SDLE=0\r"},
@@ -1314,8 +1315,9 @@ static void do_action(int action, struct cardstate *cs,
 		s = ev->ptr;
 
 		if (!strcmp(s, "OK")) {
+			/* OK without version string: assume old response */
 			*p_genresp = 1;
-			*p_resp_code = RSP_ERROR;
+			*p_resp_code = RSP_NONE;
 			break;
 		}
 
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 8/8] isdn/capi: elliminate capincci_find() in non-middleware case
  2012-04-25 23:02 [PATCH 0/8] ISDN patches for net-next Tilman Schmidt
                   ` (5 preceding siblings ...)
  2012-04-25 23:02 ` [PATCH 3/8] isdn/gigaset: improve error handling querying firmware version Tilman Schmidt
@ 2012-04-25 23:02 ` Tilman Schmidt
  2012-04-25 23:02 ` [PATCH 6/8] isdn/gigaset: unify function return values Tilman Schmidt
  2012-05-08  0:24 ` [PATCH 0/8] ISDN patches for net-next Tilman Schmidt
  8 siblings, 0 replies; 15+ messages in thread
From: Tilman Schmidt @ 2012-04-25 23:02 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel

If Kernel CAPI is compiled without CONFIG_ISDN_CAPI_MIDDLEWARE,
the structure retrieved via capincci_find() is never actually
used, so don't compile that function in that case.

Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 drivers/isdn/capi/capi.c |   27 +++++++++++++--------------
 1 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 9ee3991..38c4bd8 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -336,11 +336,6 @@ static inline void
 capincci_alloc_minor(struct capidev *cdev, struct capincci *np) { }
 static inline void capincci_free_minor(struct capincci *np) { }
 
-static inline unsigned int capincci_minor_opencount(struct capincci *np)
-{
-	return 0;
-}
-
 #endif /* !CONFIG_ISDN_CAPI_MIDDLEWARE */
 
 static struct capincci *capincci_alloc(struct capidev *cdev, u32 ncci)
@@ -372,6 +367,7 @@ static void capincci_free(struct capidev *cdev, u32 ncci)
 		}
 }
 
+#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
 static struct capincci *capincci_find(struct capidev *cdev, u32 ncci)
 {
 	struct capincci *np;
@@ -382,7 +378,6 @@ static struct capincci *capincci_find(struct capidev *cdev, u32 ncci)
 	return NULL;
 }
 
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
 /* -------- handle data queue --------------------------------------- */
 
 static struct sk_buff *
@@ -578,8 +573,8 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
 	struct tty_struct *tty;
 	struct capiminor *mp;
 	u16 datahandle;
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
 	struct capincci *np;
+#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
 
 	mutex_lock(&cdev->lock);
 
@@ -597,6 +592,12 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
 		goto unlock_out;
 	}
 
+#ifndef CONFIG_ISDN_CAPI_MIDDLEWARE
+	skb_queue_tail(&cdev->recvqueue, skb);
+	wake_up_interruptible(&cdev->recvwait);
+
+#else /* CONFIG_ISDN_CAPI_MIDDLEWARE */
+
 	np = capincci_find(cdev, CAPIMSG_CONTROL(skb->data));
 	if (!np) {
 		printk(KERN_ERR "BUG: capi_signal: ncci not found\n");
@@ -605,12 +606,6 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
 		goto unlock_out;
 	}
 
-#ifndef CONFIG_ISDN_CAPI_MIDDLEWARE
-	skb_queue_tail(&cdev->recvqueue, skb);
-	wake_up_interruptible(&cdev->recvwait);
-
-#else /* CONFIG_ISDN_CAPI_MIDDLEWARE */
-
 	mp = np->minorp;
 	if (!mp) {
 		skb_queue_tail(&cdev->recvqueue, skb);
@@ -893,6 +888,11 @@ register_out:
 			return -EFAULT;
 		return 0;
 
+#ifndef CONFIG_ISDN_CAPI_MIDDLEWARE
+	case CAPI_NCCI_OPENCOUNT:
+		return 0;
+
+#else /* CONFIG_ISDN_CAPI_MIDDLEWARE */
 	case CAPI_NCCI_OPENCOUNT: {
 		struct capincci *nccip;
 		unsigned ncci;
@@ -909,7 +909,6 @@ register_out:
 		return count;
 	}
 
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
 	case CAPI_NCCI_GETUNIT: {
 		struct capincci *nccip;
 		struct capiminor *mp;
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 7/8] isdn/capi: fix readability damage
  2012-04-25 23:02 [PATCH 0/8] ISDN patches for net-next Tilman Schmidt
                   ` (2 preceding siblings ...)
  2012-04-25 23:02 ` [PATCH 2/8] isdn/gigaset: fix CAPI disconnect B3 handling Tilman Schmidt
@ 2012-04-25 23:02 ` Tilman Schmidt
  2012-04-25 23:02 ` [PATCH 4/8] isdn/gigaset: " Tilman Schmidt
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Tilman Schmidt @ 2012-04-25 23:02 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel

Fix up some of the readibility deterioration caused by the recent
whitespace coding style cleanup.

Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 drivers/isdn/capi/capi.c    |   23 +++++++----------------
 drivers/isdn/capi/capidrv.c |    8 +++++---
 2 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index b902794..9ee3991 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -786,7 +786,6 @@ register_out:
 		return retval;
 
 	case CAPI_GET_VERSION:
-	{
 		if (copy_from_user(&data.contr, argp,
 				   sizeof(data.contr)))
 			return -EFAULT;
@@ -796,11 +795,9 @@ register_out:
 		if (copy_to_user(argp, &data.version,
 				 sizeof(data.version)))
 			return -EFAULT;
-	}
-	return 0;
+		return 0;
 
 	case CAPI_GET_SERIAL:
-	{
 		if (copy_from_user(&data.contr, argp,
 				   sizeof(data.contr)))
 			return -EFAULT;
@@ -810,10 +807,9 @@ register_out:
 		if (copy_to_user(argp, data.serial,
 				 sizeof(data.serial)))
 			return -EFAULT;
-	}
-	return 0;
+		return 0;
+
 	case CAPI_GET_PROFILE:
-	{
 		if (copy_from_user(&data.contr, argp,
 				   sizeof(data.contr)))
 			return -EFAULT;
@@ -837,11 +833,9 @@ register_out:
 		}
 		if (retval)
 			return -EFAULT;
-	}
-	return 0;
+		return 0;
 
 	case CAPI_GET_MANUFACTURER:
-	{
 		if (copy_from_user(&data.contr, argp,
 				   sizeof(data.contr)))
 			return -EFAULT;
@@ -853,8 +847,8 @@ register_out:
 				 sizeof(data.manufacturer)))
 			return -EFAULT;
 
-	}
-	return 0;
+		return 0;
+
 	case CAPI_GET_ERRCODE:
 		data.errcode = cdev->errcode;
 		cdev->errcode = CAPI_NOERROR;
@@ -870,8 +864,7 @@ register_out:
 			return 0;
 		return -ENXIO;
 
-	case CAPI_MANUFACTURER_CMD:
-	{
+	case CAPI_MANUFACTURER_CMD: {
 		struct capi_manufacturer_cmd mcmd;
 		if (!capable(CAP_SYS_ADMIN))
 			return -EPERM;
@@ -879,8 +872,6 @@ register_out:
 			return -EFAULT;
 		return capi20_manufacturer(mcmd.cmd, mcmd.data);
 	}
-	return 0;
-
 	case CAPI_SET_FLAGS:
 	case CAPI_CLR_FLAGS: {
 		unsigned userflags;
diff --git a/drivers/isdn/capi/capidrv.c b/drivers/isdn/capi/capidrv.c
index 6f5016b..832bc80 100644
--- a/drivers/isdn/capi/capidrv.c
+++ b/drivers/isdn/capi/capidrv.c
@@ -1593,7 +1593,7 @@ static int capidrv_command(isdn_ctrl *c, capidrv_contr *card)
 		return capidrv_ioctl(c, card);
 
 	switch (c->command) {
-	case ISDN_CMD_DIAL:{
+	case ISDN_CMD_DIAL: {
 		u8 calling[ISDN_MSNLEN + 3];
 		u8 called[ISDN_MSNLEN + 2];
 
@@ -2072,7 +2072,8 @@ static int capidrv_addcontr(u16 contr, struct capi_profile *profp)
 	card->interface.writebuf_skb = if_sendbuf;
 	card->interface.writecmd = NULL;
 	card->interface.readstat = if_readstat;
-	card->interface.features = ISDN_FEATURE_L2_HDLC |
+	card->interface.features =
+		ISDN_FEATURE_L2_HDLC |
 		ISDN_FEATURE_L2_TRANS |
 		ISDN_FEATURE_L3_TRANS |
 		ISDN_FEATURE_P_UNKNOWN |
@@ -2080,7 +2081,8 @@ static int capidrv_addcontr(u16 contr, struct capi_profile *profp)
 		ISDN_FEATURE_L2_X75UI |
 		ISDN_FEATURE_L2_X75BUI;
 	if (profp->support1 & (1 << 2))
-		card->interface.features |= ISDN_FEATURE_L2_V11096 |
+		card->interface.features |=
+			ISDN_FEATURE_L2_V11096 |
 			ISDN_FEATURE_L2_V11019 |
 			ISDN_FEATURE_L2_V11038;
 	if (profp->support1 & (1 << 8))
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/8] isdn/gigaset: internal function name cleanup
  2012-04-25 23:02 [PATCH 0/8] ISDN patches for net-next Tilman Schmidt
  2012-04-25 23:02 ` [PATCH 1/8] isdn/gigaset: ratelimit CAPI message dumps Tilman Schmidt
@ 2012-04-25 23:02 ` Tilman Schmidt
  2012-04-25 23:02 ` [PATCH 2/8] isdn/gigaset: fix CAPI disconnect B3 handling Tilman Schmidt
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Tilman Schmidt @ 2012-04-25 23:02 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel

Functions clear_at_state and free_strings did the same thing;
drop one of them, keeping the more descriptive name.
Drop a redundant call.
Rename function dealloc_at_states to dealloc_temp_at_states
to clarify its purpose.

Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 drivers/isdn/gigaset/common.c |   16 +++++-----------
 1 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/isdn/gigaset/common.c b/drivers/isdn/gigaset/common.c
index 7679270..6c306d4 100644
--- a/drivers/isdn/gigaset/common.c
+++ b/drivers/isdn/gigaset/common.c
@@ -362,7 +362,7 @@ struct event_t *gigaset_add_event(struct cardstate *cs,
 }
 EXPORT_SYMBOL_GPL(gigaset_add_event);
 
-static void free_strings(struct at_state_t *at_state)
+static void clear_at_state(struct at_state_t *at_state)
 {
 	int i;
 
@@ -372,18 +372,13 @@ static void free_strings(struct at_state_t *at_state)
 	}
 }
 
-static void clear_at_state(struct at_state_t *at_state)
-{
-	free_strings(at_state);
-}
-
-static void dealloc_at_states(struct cardstate *cs)
+static void dealloc_temp_at_states(struct cardstate *cs)
 {
 	struct at_state_t *cur, *next;
 
 	list_for_each_entry_safe(cur, next, &cs->temp_at_states, list) {
 		list_del(&cur->list);
-		free_strings(cur);
+		clear_at_state(cur);
 		kfree(cur);
 	}
 }
@@ -512,7 +507,7 @@ void gigaset_freecs(struct cardstate *cs)
 	case 1: /* error when registering to LL */
 		gig_dbg(DEBUG_INIT, "clearing at_state");
 		clear_at_state(&cs->at_state);
-		dealloc_at_states(cs);
+		dealloc_temp_at_states(cs);
 
 		/* fall through */
 	case 0:	/* error in basic setup */
@@ -848,8 +843,7 @@ static void cleanup_cs(struct cardstate *cs)
 	cs->mstate = MS_UNINITIALIZED;
 
 	clear_at_state(&cs->at_state);
-	dealloc_at_states(cs);
-	free_strings(&cs->at_state);
+	dealloc_temp_at_states(cs);
 	gigaset_at_init(&cs->at_state, NULL, cs, 0);
 
 	cs->inbuf->inputstate = INS_command;
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 6/8] isdn/gigaset: unify function return values
  2012-04-25 23:02 [PATCH 0/8] ISDN patches for net-next Tilman Schmidt
                   ` (6 preceding siblings ...)
  2012-04-25 23:02 ` [PATCH 8/8] isdn/capi: elliminate capincci_find() in non-middleware case Tilman Schmidt
@ 2012-04-25 23:02 ` Tilman Schmidt
  2012-05-08  0:24 ` [PATCH 0/8] ISDN patches for net-next Tilman Schmidt
  8 siblings, 0 replies; 15+ messages in thread
From: Tilman Schmidt @ 2012-04-25 23:02 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel

Various functions in the Gigaset driver were using different
conventions for the meaning of their int return values.
Align them to the usual negative error numbers convention.

Inspired-by: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 drivers/isdn/gigaset/bas-gigaset.c |   33 ++++++++++++++++-----------
 drivers/isdn/gigaset/capi.c        |    8 +++---
 drivers/isdn/gigaset/common.c      |   43 ++++++++++++++++-------------------
 drivers/isdn/gigaset/dummyll.c     |    2 +-
 drivers/isdn/gigaset/ev-layer.c    |   16 ++++++------
 drivers/isdn/gigaset/gigaset.h     |    2 +-
 drivers/isdn/gigaset/i4l.c         |   12 +++++-----
 drivers/isdn/gigaset/isocdata.c    |   12 +++++-----
 drivers/isdn/gigaset/ser-gigaset.c |   21 ++++++++++-------
 drivers/isdn/gigaset/usb-gigaset.c |   19 ++++++++-------
 10 files changed, 87 insertions(+), 81 deletions(-)

diff --git a/drivers/isdn/gigaset/bas-gigaset.c b/drivers/isdn/gigaset/bas-gigaset.c
index d651523..3b9278b 100644
--- a/drivers/isdn/gigaset/bas-gigaset.c
+++ b/drivers/isdn/gigaset/bas-gigaset.c
@@ -2077,16 +2077,14 @@ static int gigaset_brkchars(struct cardstate *cs, const unsigned char buf[6])
 /* Free hardware dependent part of the B channel structure
  * parameter:
  *	bcs	B channel structure
- * return value:
- *	!=0 on success
  */
-static int gigaset_freebcshw(struct bc_state *bcs)
+static void gigaset_freebcshw(struct bc_state *bcs)
 {
 	struct bas_bc_state *ubc = bcs->hw.bas;
 	int i;
 
 	if (!ubc)
-		return 0;
+		return;
 
 	/* kill URBs and tasklets before freeing - better safe than sorry */
 	ubc->running = 0;
@@ -2104,14 +2102,13 @@ static int gigaset_freebcshw(struct bc_state *bcs)
 	kfree(ubc->isooutbuf);
 	kfree(ubc);
 	bcs->hw.bas = NULL;
-	return 1;
 }
 
 /* Initialize hardware dependent part of the B channel structure
  * parameter:
  *	bcs	B channel structure
  * return value:
- *	!=0 on success
+ *	0 on success, error code < 0 on failure
  */
 static int gigaset_initbcshw(struct bc_state *bcs)
 {
@@ -2121,7 +2118,7 @@ static int gigaset_initbcshw(struct bc_state *bcs)
 	bcs->hw.bas = ubc = kmalloc(sizeof(struct bas_bc_state), GFP_KERNEL);
 	if (!ubc) {
 		pr_err("out of memory\n");
-		return 0;
+		return -ENOMEM;
 	}
 
 	ubc->running = 0;
@@ -2138,7 +2135,7 @@ static int gigaset_initbcshw(struct bc_state *bcs)
 		pr_err("out of memory\n");
 		kfree(ubc);
 		bcs->hw.bas = NULL;
-		return 0;
+		return -ENOMEM;
 	}
 	tasklet_init(&ubc->sent_tasklet,
 		     write_iso_tasklet, (unsigned long) bcs);
@@ -2163,7 +2160,7 @@ static int gigaset_initbcshw(struct bc_state *bcs)
 	ubc->stolen0s = 0;
 	tasklet_init(&ubc->rcvd_tasklet,
 		     read_iso_tasklet, (unsigned long) bcs);
-	return 1;
+	return 0;
 }
 
 static void gigaset_reinitbcshw(struct bc_state *bcs)
@@ -2186,6 +2183,12 @@ static void gigaset_freecshw(struct cardstate *cs)
 	cs->hw.bas = NULL;
 }
 
+/* Initialize hardware dependent part of the cardstate structure
+ * parameter:
+ *	cs	cardstate structure
+ * return value:
+ *	0 on success, error code < 0 on failure
+ */
 static int gigaset_initcshw(struct cardstate *cs)
 {
 	struct bas_cardstate *ucs;
@@ -2193,13 +2196,13 @@ static int gigaset_initcshw(struct cardstate *cs)
 	cs->hw.bas = ucs = kmalloc(sizeof *ucs, GFP_KERNEL);
 	if (!ucs) {
 		pr_err("out of memory\n");
-		return 0;
+		return -ENOMEM;
 	}
 	ucs->int_in_buf = kmalloc(IP_MSGSIZE, GFP_KERNEL);
 	if (!ucs->int_in_buf) {
 		kfree(ucs);
 		pr_err("out of memory\n");
-		return 0;
+		return -ENOMEM;
 	}
 
 	ucs->urb_cmd_in = NULL;
@@ -2218,7 +2221,7 @@ static int gigaset_initcshw(struct cardstate *cs)
 	init_waitqueue_head(&ucs->waitqueue);
 	INIT_WORK(&ucs->int_in_wq, int_in_work);
 
-	return 1;
+	return 0;
 }
 
 /* freeurbs
@@ -2378,18 +2381,20 @@ static int gigaset_probe(struct usb_interface *interface,
 	/* save address of controller structure */
 	usb_set_intfdata(interface, cs);
 
-	if (!gigaset_start(cs))
+	rc = gigaset_start(cs);
+	if (rc < 0)
 		goto error;
 
 	return 0;
 
 allocerr:
 	dev_err(cs->dev, "could not allocate URBs\n");
+	rc = -ENOMEM;
 error:
 	freeurbs(cs);
 	usb_set_intfdata(interface, NULL);
 	gigaset_freecs(cs);
-	return -ENODEV;
+	return rc;
 }
 
 /* gigaset_disconnect
diff --git a/drivers/isdn/gigaset/capi.c b/drivers/isdn/gigaset/capi.c
index 86cee65..27e4a3e 100644
--- a/drivers/isdn/gigaset/capi.c
+++ b/drivers/isdn/gigaset/capi.c
@@ -2346,7 +2346,7 @@ static const struct file_operations gigaset_proc_fops = {
  * @cs:		device descriptor structure.
  * @isdnid:	device name.
  *
- * Return value: 1 for success, 0 for failure
+ * Return value: 0 on success, error code < 0 on failure
  */
 int gigaset_isdn_regdev(struct cardstate *cs, const char *isdnid)
 {
@@ -2356,7 +2356,7 @@ int gigaset_isdn_regdev(struct cardstate *cs, const char *isdnid)
 	iif = kmalloc(sizeof(*iif), GFP_KERNEL);
 	if (!iif) {
 		pr_err("%s: out of memory\n", __func__);
-		return 0;
+		return -ENOMEM;
 	}
 
 	/* prepare controller structure */
@@ -2380,12 +2380,12 @@ int gigaset_isdn_regdev(struct cardstate *cs, const char *isdnid)
 	if (rc) {
 		pr_err("attach_capi_ctr failed (%d)\n", rc);
 		kfree(iif);
-		return 0;
+		return rc;
 	}
 
 	cs->iif = iif;
 	cs->hw_hdr_len = CAPI_DATA_B3_REQ_LEN;
-	return 1;
+	return 0;
 }
 
 /**
diff --git a/drivers/isdn/gigaset/common.c b/drivers/isdn/gigaset/common.c
index 6c306d4..aa41485 100644
--- a/drivers/isdn/gigaset/common.c
+++ b/drivers/isdn/gigaset/common.c
@@ -194,13 +194,13 @@ int gigaset_get_channel(struct bc_state *bcs)
 		gig_dbg(DEBUG_CHANNEL, "could not allocate channel %d",
 			bcs->channel);
 		spin_unlock_irqrestore(&bcs->cs->lock, flags);
-		return 0;
+		return -EBUSY;
 	}
 	++bcs->use_count;
 	bcs->busy = 1;
 	gig_dbg(DEBUG_CHANNEL, "allocated channel %d", bcs->channel);
 	spin_unlock_irqrestore(&bcs->cs->lock, flags);
-	return 1;
+	return 0;
 }
 
 struct bc_state *gigaset_get_free_channel(struct cardstate *cs)
@@ -258,7 +258,7 @@ int gigaset_get_channels(struct cardstate *cs)
 			spin_unlock_irqrestore(&cs->lock, flags);
 			gig_dbg(DEBUG_CHANNEL,
 				"could not allocate all channels");
-			return 0;
+			return -EBUSY;
 		}
 	for (i = 0; i < cs->channels; ++i)
 		++cs->bcs[i].use_count;
@@ -266,7 +266,7 @@ int gigaset_get_channels(struct cardstate *cs)
 
 	gig_dbg(DEBUG_CHANNEL, "allocated all channels");
 
-	return 1;
+	return 0;
 }
 
 void gigaset_free_channels(struct cardstate *cs)
@@ -388,8 +388,7 @@ static void gigaset_freebcs(struct bc_state *bcs)
 	int i;
 
 	gig_dbg(DEBUG_INIT, "freeing bcs[%d]->hw", bcs->channel);
-	if (!bcs->cs->ops->freebcshw(bcs))
-		gig_dbg(DEBUG_INIT, "failed");
+	bcs->cs->ops->freebcshw(bcs);
 
 	gig_dbg(DEBUG_INIT, "clearing bcs[%d]->at_state", bcs->channel);
 	clear_at_state(&bcs->at_state);
@@ -566,6 +565,8 @@ static void gigaset_inbuf_init(struct inbuf_t *inbuf, struct cardstate *cs)
  * @inbuf:	buffer structure.
  * @src:	received data.
  * @numbytes:	number of bytes received.
+ *
+ * Return value: !=0 if some data was appended
  */
 int gigaset_fill_inbuf(struct inbuf_t *inbuf, const unsigned char *src,
 		       unsigned numbytes)
@@ -609,8 +610,8 @@ int gigaset_fill_inbuf(struct inbuf_t *inbuf, const unsigned char *src,
 EXPORT_SYMBOL_GPL(gigaset_fill_inbuf);
 
 /* Initialize the b-channel structure */
-static struct bc_state *gigaset_initbcs(struct bc_state *bcs,
-					struct cardstate *cs, int channel)
+static int gigaset_initbcs(struct bc_state *bcs, struct cardstate *cs,
+			   int channel)
 {
 	int i;
 
@@ -649,11 +650,7 @@ static struct bc_state *gigaset_initbcs(struct bc_state *bcs,
 	bcs->apconnstate = 0;
 
 	gig_dbg(DEBUG_INIT, "  setting up bcs[%d]->hw", channel);
-	if (cs->ops->initbcshw(bcs))
-		return bcs;
-
-	gig_dbg(DEBUG_INIT, "  failed");
-	return NULL;
+	return cs->ops->initbcshw(bcs);
 }
 
 /**
@@ -752,7 +749,7 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
 	cs->cmdbytes = 0;
 
 	gig_dbg(DEBUG_INIT, "setting up iif");
-	if (!gigaset_isdn_regdev(cs, modulename)) {
+	if (gigaset_isdn_regdev(cs, modulename) < 0) {
 		pr_err("error registering ISDN device\n");
 		goto error;
 	}
@@ -760,7 +757,7 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
 	make_valid(cs, VALID_ID);
 	++cs->cs_init;
 	gig_dbg(DEBUG_INIT, "setting up hw");
-	if (!cs->ops->initcshw(cs))
+	if (cs->ops->initcshw(cs) < 0)
 		goto error;
 
 	++cs->cs_init;
@@ -774,7 +771,7 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
 	/* set up channel data structures */
 	for (i = 0; i < channels; ++i) {
 		gig_dbg(DEBUG_INIT, "setting up bcs[%d]", i);
-		if (!gigaset_initbcs(cs->bcs + i, cs, i)) {
+		if (gigaset_initbcs(cs->bcs + i, cs, i) < 0) {
 			pr_err("could not allocate channel %d data\n", i);
 			goto error;
 		}
@@ -869,7 +866,7 @@ static void cleanup_cs(struct cardstate *cs)
 
 	for (i = 0; i < cs->channels; ++i) {
 		gigaset_freebcs(cs->bcs + i);
-		if (!gigaset_initbcs(cs->bcs + i, cs, i))
+		if (gigaset_initbcs(cs->bcs + i, cs, i) < 0)
 			pr_err("could not allocate channel %d data\n", i);
 	}
 
@@ -890,14 +887,14 @@ static void cleanup_cs(struct cardstate *cs)
  * waiting for completion of the initialization.
  *
  * Return value:
- *	1 - success, 0 - error
+ *	0 on success, error code < 0 on failure
  */
 int gigaset_start(struct cardstate *cs)
 {
 	unsigned long flags;
 
 	if (mutex_lock_interruptible(&cs->mutex))
-		return 0;
+		return -EBUSY;
 
 	spin_lock_irqsave(&cs->lock, flags);
 	cs->connected = 1;
@@ -921,11 +918,11 @@ int gigaset_start(struct cardstate *cs)
 	wait_event(cs->waitqueue, !cs->waiting);
 
 	mutex_unlock(&cs->mutex);
-	return 1;
+	return 0;
 
 error:
 	mutex_unlock(&cs->mutex);
-	return 0;
+	return -ENOMEM;
 }
 EXPORT_SYMBOL_GPL(gigaset_start);
 
@@ -937,7 +934,7 @@ EXPORT_SYMBOL_GPL(gigaset_start);
  * waiting for completion of the shutdown.
  *
  * Return value:
- *	0 - success, -1 - error (no device associated)
+ *	0 - success, -ENODEV - error (no device associated)
  */
 int gigaset_shutdown(struct cardstate *cs)
 {
@@ -945,7 +942,7 @@ int gigaset_shutdown(struct cardstate *cs)
 
 	if (!(cs->flags & VALID_MINOR)) {
 		mutex_unlock(&cs->mutex);
-		return -1;
+		return -ENODEV;
 	}
 
 	cs->waiting = 1;
diff --git a/drivers/isdn/gigaset/dummyll.c b/drivers/isdn/gigaset/dummyll.c
index 19b1c77..570c2d5 100644
--- a/drivers/isdn/gigaset/dummyll.c
+++ b/drivers/isdn/gigaset/dummyll.c
@@ -60,7 +60,7 @@ void gigaset_isdn_stop(struct cardstate *cs)
 
 int gigaset_isdn_regdev(struct cardstate *cs, const char *isdnid)
 {
-	return 1;
+	return 0;
 }
 
 void gigaset_isdn_unregdev(struct cardstate *cs)
diff --git a/drivers/isdn/gigaset/ev-layer.c b/drivers/isdn/gigaset/ev-layer.c
index 8391e09..2e6963d 100644
--- a/drivers/isdn/gigaset/ev-layer.c
+++ b/drivers/isdn/gigaset/ev-layer.c
@@ -658,7 +658,7 @@ static inline struct at_state_t *get_free_channel(struct cardstate *cs,
 	struct at_state_t *ret;
 
 	for (i = 0; i < cs->channels; ++i)
-		if (gigaset_get_channel(cs->bcs + i)) {
+		if (gigaset_get_channel(cs->bcs + i) >= 0) {
 			ret = &cs->bcs[i].at_state;
 			ret->cid = cid;
 			return ret;
@@ -923,18 +923,18 @@ static void do_stop(struct cardstate *cs)
  * channel >= 0: getting cid for the channel failed
  * channel < 0:  entering cid mode failed
  *
- * returns 0 on failure
+ * returns 0 on success, <0 on failure
  */
 static int reinit_and_retry(struct cardstate *cs, int channel)
 {
 	int i;
 
 	if (--cs->retry_count <= 0)
-		return 0;
+		return -EFAULT;
 
 	for (i = 0; i < cs->channels; ++i)
 		if (cs->bcs[i].at_state.cid > 0)
-			return 0;
+			return -EBUSY;
 
 	if (channel < 0)
 		dev_warn(cs->dev,
@@ -945,7 +945,7 @@ static int reinit_and_retry(struct cardstate *cs, int channel)
 		cs->bcs[channel].at_state.pending_commands |= PC_CID;
 	}
 	schedule_init(cs, MS_INIT);
-	return 1;
+	return 0;
 }
 
 static int at_state_invalid(struct cardstate *cs,
@@ -1016,7 +1016,7 @@ static int do_lock(struct cardstate *cs)
 			if (cs->bcs[i].at_state.pending_commands)
 				return -EBUSY;
 
-		if (!gigaset_get_channels(cs))
+		if (gigaset_get_channels(cs) < 0)
 			return -EBUSY;
 
 		break;
@@ -1125,7 +1125,7 @@ static void do_action(int action, struct cardstate *cs,
 			init_failed(cs, M_UNKNOWN);
 			break;
 		}
-		if (!reinit_and_retry(cs, -1))
+		if (reinit_and_retry(cs, -1) < 0)
 			schedule_init(cs, MS_RECOVER);
 		break;
 	case ACT_FAILUMODE:
@@ -1268,7 +1268,7 @@ static void do_action(int action, struct cardstate *cs,
 	case ACT_FAILCID:
 		cs->cur_at_seq = SEQ_NONE;
 		channel = cs->curchannel;
-		if (!reinit_and_retry(cs, channel)) {
+		if (reinit_and_retry(cs, channel) < 0) {
 			dev_warn(cs->dev,
 				 "Could not get a call ID. Cannot dial.\n");
 			at_state2 = &cs->bcs[channel].at_state;
diff --git a/drivers/isdn/gigaset/gigaset.h b/drivers/isdn/gigaset/gigaset.h
index eae7351..8e2fc8f 100644
--- a/drivers/isdn/gigaset/gigaset.h
+++ b/drivers/isdn/gigaset/gigaset.h
@@ -583,7 +583,7 @@ struct gigaset_ops {
 	int (*initbcshw)(struct bc_state *bcs);
 
 	/* Called by gigaset_freecs() for freeing bcs->hw.xxx */
-	int (*freebcshw)(struct bc_state *bcs);
+	void (*freebcshw)(struct bc_state *bcs);
 
 	/* Called by gigaset_bchannel_down() for resetting bcs->hw.xxx */
 	void (*reinitbcshw)(struct bc_state *bcs);
diff --git a/drivers/isdn/gigaset/i4l.c b/drivers/isdn/gigaset/i4l.c
index 0f13eb1..2d75329 100644
--- a/drivers/isdn/gigaset/i4l.c
+++ b/drivers/isdn/gigaset/i4l.c
@@ -229,7 +229,7 @@ static int command_from_LL(isdn_ctrl *cntrl)
 			return -EINVAL;
 		}
 		bcs = cs->bcs + ch;
-		if (!gigaset_get_channel(bcs)) {
+		if (gigaset_get_channel(bcs) < 0) {
 			dev_err(cs->dev, "ISDN_CMD_DIAL: channel not free\n");
 			return -EBUSY;
 		}
@@ -618,7 +618,7 @@ void gigaset_isdn_stop(struct cardstate *cs)
  * @cs:		device descriptor structure.
  * @isdnid:	device name.
  *
- * Return value: 1 for success, 0 for failure
+ * Return value: 0 on success, error code < 0 on failure
  */
 int gigaset_isdn_regdev(struct cardstate *cs, const char *isdnid)
 {
@@ -627,14 +627,14 @@ int gigaset_isdn_regdev(struct cardstate *cs, const char *isdnid)
 	iif = kmalloc(sizeof *iif, GFP_KERNEL);
 	if (!iif) {
 		pr_err("out of memory\n");
-		return 0;
+		return -ENOMEM;
 	}
 
 	if (snprintf(iif->id, sizeof iif->id, "%s_%u", isdnid, cs->minor_index)
 	    >= sizeof iif->id) {
 		pr_err("ID too long: %s\n", isdnid);
 		kfree(iif);
-		return 0;
+		return -EINVAL;
 	}
 
 	iif->owner = THIS_MODULE;
@@ -656,13 +656,13 @@ int gigaset_isdn_regdev(struct cardstate *cs, const char *isdnid)
 	if (!register_isdn(iif)) {
 		pr_err("register_isdn failed\n");
 		kfree(iif);
-		return 0;
+		return -EINVAL;
 	}
 
 	cs->iif = iif;
 	cs->myid = iif->channels;		/* Set my device id */
 	cs->hw_hdr_len = HW_HDR_LEN;
-	return 1;
+	return 0;
 }
 
 /**
diff --git a/drivers/isdn/gigaset/isocdata.c b/drivers/isdn/gigaset/isocdata.c
index a351c16..bc29f1d 100644
--- a/drivers/isdn/gigaset/isocdata.c
+++ b/drivers/isdn/gigaset/isocdata.c
@@ -56,7 +56,7 @@ static inline int isowbuf_freebytes(struct isowbuf_t *iwb)
 
 /* start writing
  * acquire the write semaphore
- * return true if acquired, false if busy
+ * return 0 if acquired, <0 if busy
  */
 static inline int isowbuf_startwrite(struct isowbuf_t *iwb)
 {
@@ -64,12 +64,12 @@ static inline int isowbuf_startwrite(struct isowbuf_t *iwb)
 		atomic_inc(&iwb->writesem);
 		gig_dbg(DEBUG_ISO, "%s: couldn't acquire iso write semaphore",
 			__func__);
-		return 0;
+		return -EBUSY;
 	}
 	gig_dbg(DEBUG_ISO,
 		"%s: acquired iso write semaphore, data[write]=%02x, nbits=%d",
 		__func__, iwb->data[iwb->write], iwb->wbits);
-	return 1;
+	return 0;
 }
 
 /* finish writing
@@ -158,7 +158,7 @@ int gigaset_isowbuf_getbytes(struct isowbuf_t *iwb, int size)
 		/* no wraparound in valid data */
 		if (limit >= write) {
 			/* append idle frame */
-			if (!isowbuf_startwrite(iwb))
+			if (isowbuf_startwrite(iwb) < 0)
 				return -EBUSY;
 			/* write position could have changed */
 			write = iwb->write;
@@ -403,7 +403,7 @@ static inline int hdlc_buildframe(struct isowbuf_t *iwb,
 	unsigned char c;
 
 	if (isowbuf_freebytes(iwb) < count + count / 5 + 6 ||
-	    !isowbuf_startwrite(iwb)) {
+	    isowbuf_startwrite(iwb) < 0) {
 		gig_dbg(DEBUG_ISO, "%s: %d bytes free -> -EAGAIN",
 			__func__, isowbuf_freebytes(iwb));
 		return -EAGAIN;
@@ -457,7 +457,7 @@ static inline int trans_buildframe(struct isowbuf_t *iwb,
 		return iwb->write;
 
 	if (isowbuf_freebytes(iwb) < count ||
-	    !isowbuf_startwrite(iwb)) {
+	    isowbuf_startwrite(iwb) < 0) {
 		gig_dbg(DEBUG_ISO, "can't put %d bytes", count);
 		return -EAGAIN;
 	}
diff --git a/drivers/isdn/gigaset/ser-gigaset.c b/drivers/isdn/gigaset/ser-gigaset.c
index 6f3fd4c..8c91fd5 100644
--- a/drivers/isdn/gigaset/ser-gigaset.c
+++ b/drivers/isdn/gigaset/ser-gigaset.c
@@ -340,17 +340,16 @@ static int gigaset_initbcshw(struct bc_state *bcs)
 {
 	/* unused */
 	bcs->hw.ser = NULL;
-	return 1;
+	return 0;
 }
 
 /*
  * Free B channel structure
  * Called by "gigaset_freebcs" in common.c
  */
-static int gigaset_freebcshw(struct bc_state *bcs)
+static void gigaset_freebcshw(struct bc_state *bcs)
 {
 	/* unused */
-	return 1;
 }
 
 /*
@@ -398,7 +397,7 @@ static int gigaset_initcshw(struct cardstate *cs)
 	scs = kzalloc(sizeof(struct ser_cardstate), GFP_KERNEL);
 	if (!scs) {
 		pr_err("out of memory\n");
-		return 0;
+		return -ENOMEM;
 	}
 	cs->hw.ser = scs;
 
@@ -410,13 +409,13 @@ static int gigaset_initcshw(struct cardstate *cs)
 		pr_err("error %d registering platform device\n", rc);
 		kfree(cs->hw.ser);
 		cs->hw.ser = NULL;
-		return 0;
+		return rc;
 	}
 	dev_set_drvdata(&cs->hw.ser->dev.dev, cs);
 
 	tasklet_init(&cs->write_tasklet,
 		     gigaset_modem_fill, (unsigned long) cs);
-	return 1;
+	return 0;
 }
 
 /*
@@ -503,6 +502,7 @@ static int
 gigaset_tty_open(struct tty_struct *tty)
 {
 	struct cardstate *cs;
+	int rc;
 
 	gig_dbg(DEBUG_INIT, "Starting HLL for Gigaset M101");
 
@@ -515,8 +515,10 @@ gigaset_tty_open(struct tty_struct *tty)
 
 	/* allocate memory for our device state and initialize it */
 	cs = gigaset_initcs(driver, 1, 1, 0, cidmode, GIGASET_MODULENAME);
-	if (!cs)
+	if (!cs) {
+		rc = -ENODEV;
 		goto error;
+	}
 
 	cs->dev = &cs->hw.ser->dev.dev;
 	cs->hw.ser->tty = tty;
@@ -530,7 +532,8 @@ gigaset_tty_open(struct tty_struct *tty)
 	 */
 	if (startmode == SM_LOCKED)
 		cs->mstate = MS_LOCKED;
-	if (!gigaset_start(cs)) {
+	rc = gigaset_start(cs);
+	if (rc < 0) {
 		tasklet_kill(&cs->write_tasklet);
 		goto error;
 	}
@@ -542,7 +545,7 @@ error:
 	gig_dbg(DEBUG_INIT, "Startup of HLL failed");
 	tty->disc_data = NULL;
 	gigaset_freecs(cs);
-	return -ENODEV;
+	return rc;
 }
 
 /*
diff --git a/drivers/isdn/gigaset/usb-gigaset.c b/drivers/isdn/gigaset/usb-gigaset.c
index 049da67..bb12d80 100644
--- a/drivers/isdn/gigaset/usb-gigaset.c
+++ b/drivers/isdn/gigaset/usb-gigaset.c
@@ -549,10 +549,9 @@ static int gigaset_brkchars(struct cardstate *cs, const unsigned char buf[6])
 			       0, 0, &buf, 6, 2000);
 }
 
-static int gigaset_freebcshw(struct bc_state *bcs)
+static void gigaset_freebcshw(struct bc_state *bcs)
 {
 	/* unused */
-	return 1;
 }
 
 /* Initialize the b-channel structure */
@@ -560,7 +559,7 @@ static int gigaset_initbcshw(struct bc_state *bcs)
 {
 	/* unused */
 	bcs->hw.usb = NULL;
-	return 1;
+	return 0;
 }
 
 static void gigaset_reinitbcshw(struct bc_state *bcs)
@@ -582,7 +581,7 @@ static int gigaset_initcshw(struct cardstate *cs)
 		kmalloc(sizeof(struct usb_cardstate), GFP_KERNEL);
 	if (!ucs) {
 		pr_err("out of memory\n");
-		return 0;
+		return -ENOMEM;
 	}
 
 	ucs->bchars[0] = 0;
@@ -597,7 +596,7 @@ static int gigaset_initcshw(struct cardstate *cs)
 	tasklet_init(&cs->write_tasklet,
 		     gigaset_modem_fill, (unsigned long) cs);
 
-	return 1;
+	return 0;
 }
 
 /* Send data from current skb to the device. */
@@ -766,9 +765,9 @@ static int gigaset_probe(struct usb_interface *interface,
 	if (startmode == SM_LOCKED)
 		cs->mstate = MS_LOCKED;
 
-	if (!gigaset_start(cs)) {
+	retval = gigaset_start(cs);
+	if (retval < 0) {
 		tasklet_kill(&cs->write_tasklet);
-		retval = -ENODEV;
 		goto error;
 	}
 	return 0;
@@ -898,8 +897,10 @@ static int __init usb_gigaset_init(void)
 	driver = gigaset_initdriver(GIGASET_MINOR, GIGASET_MINORS,
 				    GIGASET_MODULENAME, GIGASET_DEVNAME,
 				    &ops, THIS_MODULE);
-	if (driver == NULL)
+	if (driver == NULL) {
+		result = -ENOMEM;
 		goto error;
+	}
 
 	/* register this driver with the USB subsystem */
 	result = usb_register(&gigaset_usb_driver);
@@ -915,7 +916,7 @@ error:
 	if (driver)
 		gigaset_freedriver(driver);
 	driver = NULL;
-	return -1;
+	return result;
 }
 
 /*
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/8] isdn/gigaset: fix readability damage
  2012-04-25 23:02 [PATCH 0/8] ISDN patches for net-next Tilman Schmidt
                   ` (3 preceding siblings ...)
  2012-04-25 23:02 ` [PATCH 7/8] isdn/capi: fix readability damage Tilman Schmidt
@ 2012-04-25 23:02 ` Tilman Schmidt
  2012-04-25 23:02 ` [PATCH 3/8] isdn/gigaset: improve error handling querying firmware version Tilman Schmidt
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Tilman Schmidt @ 2012-04-25 23:02 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel

Fix up some of the readibility deterioration caused by the recent
whitespace coding style cleanup.

Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 drivers/isdn/gigaset/bas-gigaset.c |   11 +-
 drivers/isdn/gigaset/capi.c        |   84 +++++------
 drivers/isdn/gigaset/ev-layer.c    |  301 ++++++++++++++++++------------------
 drivers/isdn/gigaset/gigaset.h     |   28 ++--
 4 files changed, 208 insertions(+), 216 deletions(-)

diff --git a/drivers/isdn/gigaset/bas-gigaset.c b/drivers/isdn/gigaset/bas-gigaset.c
index afa0802..d651523 100644
--- a/drivers/isdn/gigaset/bas-gigaset.c
+++ b/drivers/isdn/gigaset/bas-gigaset.c
@@ -410,10 +410,10 @@ static void check_pending(struct bas_cardstate *ucs)
 		if (!(ucs->basstate & BS_RESETTING))
 			ucs->pending = 0;
 		break;
-		/*
-		 * HD_READ_ATMESSAGE and HD_WRITE_ATMESSAGE are handled separately
-		 * and should never end up here
-		 */
+	/*
+	 * HD_READ_ATMESSAGE and HD_WRITE_ATMESSAGE are handled separately
+	 * and should never end up here
+	 */
 	default:
 		dev_warn(&ucs->interface->dev,
 			 "unknown pending request 0x%02x cleared\n",
@@ -877,8 +877,7 @@ static void read_iso_callback(struct urb *urb)
 		for (i = 0; i < BAS_NUMFRAMES; i++) {
 			ubc->isoinlost += urb->iso_frame_desc[i].actual_length;
 			if (unlikely(urb->iso_frame_desc[i].status != 0 &&
-				     urb->iso_frame_desc[i].status !=
-				     -EINPROGRESS))
+				     urb->iso_frame_desc[i].status != -EINPROGRESS))
 				ubc->loststatus = urb->iso_frame_desc[i].status;
 			urb->iso_frame_desc[i].status = 0;
 			urb->iso_frame_desc[i].actual_length = 0;
diff --git a/drivers/isdn/gigaset/capi.c b/drivers/isdn/gigaset/capi.c
index 579aa02..86cee65 100644
--- a/drivers/isdn/gigaset/capi.c
+++ b/drivers/isdn/gigaset/capi.c
@@ -109,51 +109,35 @@ static struct {
 	u8 *bc;
 	u8 *hlc;
 } cip2bchlc[] = {
-	[1] = { "8090A3", NULL },
-	/* Speech (A-law) */
-	[2] = { "8890", NULL },
-	/* Unrestricted digital information */
-	[3] = { "8990", NULL },
-	/* Restricted digital information */
-	[4] = { "9090A3", NULL },
-	/* 3,1 kHz audio (A-law) */
-	[5] = { "9190", NULL },
-	/* 7 kHz audio */
-	[6] = { "9890", NULL },
-	/* Video */
-	[7] = { "88C0C6E6", NULL },
-	/* Packet mode */
-	[8] = { "8890218F", NULL },
-	/* 56 kbit/s rate adaptation */
-	[9] = { "9190A5", NULL },
-	/* Unrestricted digital information with tones/announcements */
-	[16] = { "8090A3", "9181" },
-	/* Telephony */
-	[17] = { "9090A3", "9184" },
-	/* Group 2/3 facsimile */
-	[18] = { "8890", "91A1" },
-	/* Group 4 facsimile Class 1 */
-	[19] = { "8890", "91A4" },
-	/* Teletex service basic and mixed mode
-	   and Group 4 facsimile service Classes II and III */
-	[20] = { "8890", "91A8" },
-	/* Teletex service basic and processable mode */
-	[21] = { "8890", "91B1" },
-	/* Teletex service basic mode */
-	[22] = { "8890", "91B2" },
-	/* International interworking for Videotex */
-	[23] = { "8890", "91B5" },
-	/* Telex */
-	[24] = { "8890", "91B8" },
-	/* Message Handling Systems in accordance with X.400 */
-	[25] = { "8890", "91C1" },
-	/* OSI application in accordance with X.200 */
-	[26] = { "9190A5", "9181" },
-	/* 7 kHz telephony */
-	[27] = { "9190A5", "916001" },
-	/* Video telephony, first connection */
-	[28] = { "8890", "916002" },
-	/* Video telephony, second connection */
+	[1] = { "8090A3", NULL },	/* Speech (A-law) */
+	[2] = { "8890", NULL },		/* Unrestricted digital information */
+	[3] = { "8990", NULL },		/* Restricted digital information */
+	[4] = { "9090A3", NULL },	/* 3,1 kHz audio (A-law) */
+	[5] = { "9190", NULL },		/* 7 kHz audio */
+	[6] = { "9890", NULL },		/* Video */
+	[7] = { "88C0C6E6", NULL },	/* Packet mode */
+	[8] = { "8890218F", NULL },	/* 56 kbit/s rate adaptation */
+	[9] = { "9190A5", NULL },	/* Unrestricted digital information
+					 * with tones/announcements */
+	[16] = { "8090A3", "9181" },	/* Telephony */
+	[17] = { "9090A3", "9184" },	/* Group 2/3 facsimile */
+	[18] = { "8890", "91A1" },	/* Group 4 facsimile Class 1 */
+	[19] = { "8890", "91A4" },	/* Teletex service basic and mixed mode
+					 * and Group 4 facsimile service
+					 * Classes II and III */
+	[20] = { "8890", "91A8" },	/* Teletex service basic and
+					 * processable mode */
+	[21] = { "8890", "91B1" },	/* Teletex service basic mode */
+	[22] = { "8890", "91B2" },	/* International interworking for
+					 * Videotex */
+	[23] = { "8890", "91B5" },	/* Telex */
+	[24] = { "8890", "91B8" },	/* Message Handling Systems
+					 * in accordance with X.400 */
+	[25] = { "8890", "91C1" },	/* OSI application
+					 * in accordance with X.200 */
+	[26] = { "9190A5", "9181" },	/* 7 kHz telephony */
+	[27] = { "9190A5", "916001" },	/* Video telephony, first connection */
+	[28] = { "8890", "916002" },	/* Video telephony, second connection */
 };
 
 /*
@@ -1197,7 +1181,9 @@ static void do_facility_req(struct gigaset_capi_ctr *iif,
 			confparam[3] = 2;	/* length */
 			capimsg_setu16(confparam, 4, CapiSuccess);
 			break;
-			/* ToDo: add supported services */
+
+		/* ToDo: add supported services */
+
 		default:
 			dev_notice(cs->dev,
 				   "%s: unsupported supplementary service function 0x%04x\n",
@@ -1771,7 +1757,8 @@ static void do_connect_b3_req(struct gigaset_capi_ctr *iif,
 
 	/* NCPI parameter: not applicable for B3 Transparent */
 	ignore_cstruct_param(cs, cmsg->NCPI, "CONNECT_B3_REQ", "NCPI");
-	send_conf(iif, ap, skb, (cmsg->NCPI && cmsg->NCPI[0]) ?
+	send_conf(iif, ap, skb,
+		  (cmsg->NCPI && cmsg->NCPI[0]) ?
 		  CapiNcpiNotSupportedByProtocol : CapiSuccess);
 }
 
@@ -1975,7 +1962,8 @@ static void do_disconnect_b3_req(struct gigaset_capi_ctr *iif,
 	/* NCPI parameter: not applicable for B3 Transparent */
 	ignore_cstruct_param(cs, cmsg->NCPI,
 			     "DISCONNECT_B3_REQ", "NCPI");
-	send_conf(iif, ap, skb, (cmsg->NCPI && cmsg->NCPI[0]) ?
+	send_conf(iif, ap, skb,
+		  (cmsg->NCPI && cmsg->NCPI[0]) ?
 		  CapiNcpiNotSupportedByProtocol : CapiSuccess);
 }
 
diff --git a/drivers/isdn/gigaset/ev-layer.c b/drivers/isdn/gigaset/ev-layer.c
index 685638a..8391e09 100644
--- a/drivers/isdn/gigaset/ev-layer.c
+++ b/drivers/isdn/gigaset/ev-layer.c
@@ -153,104 +153,104 @@ struct reply_t gigaset_tab_nocid[] =
  * action, command */
 
 /* initialize device, set cid mode if possible */
-	{RSP_INIT,	 -1,  -1, SEQ_INIT,		100,  1, {ACT_TIMEOUT} },
+	{RSP_INIT,	 -1,  -1, SEQ_INIT,	100,  1, {ACT_TIMEOUT} },
 
-	{EV_TIMEOUT,	100, 100, -1,			101,  3, {0},	"Z\r"},
-	{RSP_OK,	101, 103, -1,			120,  5, {ACT_GETSTRING},
-	 "+GMR\r"},
+	{EV_TIMEOUT,	100, 100, -1,		101,  3, {0},	"Z\r"},
+	{RSP_OK,	101, 103, -1,		120,  5, {ACT_GETSTRING},
+								"+GMR\r"},
 
-	{EV_TIMEOUT,	101, 101, -1,			102,  5, {0},	"Z\r"},
-	{RSP_ERROR,	101, 101, -1,			102,  5, {0},	"Z\r"},
+	{EV_TIMEOUT,	101, 101, -1,		102,  5, {0},	"Z\r"},
+	{RSP_ERROR,	101, 101, -1,		102,  5, {0},	"Z\r"},
 
-	{EV_TIMEOUT,	102, 102, -1,			108,  5, {ACT_SETDLE1},
-	 "^SDLE=0\r"},
-	{RSP_OK,	108, 108, -1,			104, -1},
-	{RSP_ZDLE,	104, 104,  0,			103,  5, {0},	"Z\r"},
-	{EV_TIMEOUT,	104, 104, -1,			  0,  0, {ACT_FAILINIT} },
-	{RSP_ERROR,	108, 108, -1,			  0,  0, {ACT_FAILINIT} },
+	{EV_TIMEOUT,	102, 102, -1,		108,  5, {ACT_SETDLE1},
+								"^SDLE=0\r"},
+	{RSP_OK,	108, 108, -1,		104, -1},
+	{RSP_ZDLE,	104, 104,  0,		103,  5, {0},	"Z\r"},
+	{EV_TIMEOUT,	104, 104, -1,		  0,  0, {ACT_FAILINIT} },
+	{RSP_ERROR,	108, 108, -1,		  0,  0, {ACT_FAILINIT} },
 
-	{EV_TIMEOUT,	108, 108, -1,			105,  2, {ACT_SETDLE0,
-								  ACT_HUPMODEM,
-								  ACT_TIMEOUT} },
-	{EV_TIMEOUT,	105, 105, -1,			103,  5, {0},	"Z\r"},
+	{EV_TIMEOUT,	108, 108, -1,		105,  2, {ACT_SETDLE0,
+							  ACT_HUPMODEM,
+							  ACT_TIMEOUT} },
+	{EV_TIMEOUT,	105, 105, -1,		103,  5, {0},	"Z\r"},
 
-	{RSP_ERROR,	102, 102, -1,			107,  5, {0},	"^GETPRE\r"},
-	{RSP_OK,	107, 107, -1,			  0,  0, {ACT_CONFIGMODE} },
-	{RSP_ERROR,	107, 107, -1,			  0,  0, {ACT_FAILINIT} },
-	{EV_TIMEOUT,	107, 107, -1,			  0,  0, {ACT_FAILINIT} },
+	{RSP_ERROR,	102, 102, -1,		107,  5, {0},	"^GETPRE\r"},
+	{RSP_OK,	107, 107, -1,		  0,  0, {ACT_CONFIGMODE} },
+	{RSP_ERROR,	107, 107, -1,		  0,  0, {ACT_FAILINIT} },
+	{EV_TIMEOUT,	107, 107, -1,		  0,  0, {ACT_FAILINIT} },
 
-	{RSP_ERROR,	103, 103, -1,			  0,  0, {ACT_FAILINIT} },
-	{EV_TIMEOUT,	103, 103, -1,			  0,  0, {ACT_FAILINIT} },
+	{RSP_ERROR,	103, 103, -1,		  0,  0, {ACT_FAILINIT} },
+	{EV_TIMEOUT,	103, 103, -1,		  0,  0, {ACT_FAILINIT} },
 
-	{RSP_STRING,	120, 120, -1,			121, -1, {ACT_SETVER} },
+	{RSP_STRING,	120, 120, -1,		121, -1, {ACT_SETVER} },
 
-	{EV_TIMEOUT,	120, 121, -1,			  0,  0, {ACT_FAILVER,
-								  ACT_INIT} },
-	{RSP_ERROR,	120, 121, -1,			  0,  0, {ACT_FAILVER,
-								  ACT_INIT} },
-	{RSP_OK,	121, 121, -1,			  0,  0, {ACT_GOTVER,
-								  ACT_INIT} },
-	{RSP_NONE,	121, 121, -1,			120,  0, {ACT_GETSTRING} },
+	{EV_TIMEOUT,	120, 121, -1,		  0,  0, {ACT_FAILVER,
+							  ACT_INIT} },
+	{RSP_ERROR,	120, 121, -1,		  0,  0, {ACT_FAILVER,
+							  ACT_INIT} },
+	{RSP_OK,	121, 121, -1,		  0,  0, {ACT_GOTVER,
+							  ACT_INIT} },
+	{RSP_NONE,	121, 121, -1,		120,  0, {ACT_GETSTRING} },
 
 /* leave dle mode */
-	{RSP_INIT,	  0,   0, SEQ_DLE0,		201,  5, {0},	"^SDLE=0\r"},
-	{RSP_OK,	201, 201, -1,			202, -1},
-	{RSP_ZDLE,	202, 202,  0,			  0,  0, {ACT_DLE0} },
-	{RSP_NODEV,	200, 249, -1,			  0,  0, {ACT_FAKEDLE0} },
-	{RSP_ERROR,	200, 249, -1,			  0,  0, {ACT_FAILDLE0} },
-	{EV_TIMEOUT,	200, 249, -1,			  0,  0, {ACT_FAILDLE0} },
+	{RSP_INIT,	  0,   0, SEQ_DLE0,	201,  5, {0},	"^SDLE=0\r"},
+	{RSP_OK,	201, 201, -1,		202, -1},
+	{RSP_ZDLE,	202, 202,  0,		  0,  0, {ACT_DLE0} },
+	{RSP_NODEV,	200, 249, -1,		  0,  0, {ACT_FAKEDLE0} },
+	{RSP_ERROR,	200, 249, -1,		  0,  0, {ACT_FAILDLE0} },
+	{EV_TIMEOUT,	200, 249, -1,		  0,  0, {ACT_FAILDLE0} },
 
 /* enter dle mode */
-	{RSP_INIT,	  0,   0, SEQ_DLE1,		251,  5, {0},	"^SDLE=1\r"},
-	{RSP_OK,	251, 251, -1,			252, -1},
-	{RSP_ZDLE,	252, 252,  1,			  0,  0, {ACT_DLE1} },
-	{RSP_ERROR,	250, 299, -1,			  0,  0, {ACT_FAILDLE1} },
-	{EV_TIMEOUT,	250, 299, -1,			  0,  0, {ACT_FAILDLE1} },
+	{RSP_INIT,	  0,   0, SEQ_DLE1,	251,  5, {0},	"^SDLE=1\r"},
+	{RSP_OK,	251, 251, -1,		252, -1},
+	{RSP_ZDLE,	252, 252,  1,		  0,  0, {ACT_DLE1} },
+	{RSP_ERROR,	250, 299, -1,		  0,  0, {ACT_FAILDLE1} },
+	{EV_TIMEOUT,	250, 299, -1,		  0,  0, {ACT_FAILDLE1} },
 
 /* incoming call */
-	{RSP_RING,	 -1,  -1, -1,			 -1, -1, {ACT_RING} },
+	{RSP_RING,	 -1,  -1, -1,		 -1, -1, {ACT_RING} },
 
 /* get cid */
-	{RSP_INIT,	  0,   0, SEQ_CID,		301,  5, {0},	"^SGCI?\r"},
-	{RSP_OK,	301, 301, -1,			302, -1},
-	{RSP_ZGCI,	302, 302, -1,			  0,  0, {ACT_CID} },
-	{RSP_ERROR,	301, 349, -1,			  0,  0, {ACT_FAILCID} },
-	{EV_TIMEOUT,	301, 349, -1,			  0,  0, {ACT_FAILCID} },
+	{RSP_INIT,	  0,   0, SEQ_CID,	301,  5, {0},	"^SGCI?\r"},
+	{RSP_OK,	301, 301, -1,		302, -1},
+	{RSP_ZGCI,	302, 302, -1,		  0,  0, {ACT_CID} },
+	{RSP_ERROR,	301, 349, -1,		  0,  0, {ACT_FAILCID} },
+	{EV_TIMEOUT,	301, 349, -1,		  0,  0, {ACT_FAILCID} },
 
 /* enter cid mode */
-	{RSP_INIT,	  0,   0, SEQ_CIDMODE,		150,  5, {0},	"^SGCI=1\r"},
-	{RSP_OK,	150, 150, -1,			  0,  0, {ACT_CMODESET} },
-	{RSP_ERROR,	150, 150, -1,			  0,  0, {ACT_FAILCMODE} },
-	{EV_TIMEOUT,	150, 150, -1,			  0,  0, {ACT_FAILCMODE} },
+	{RSP_INIT,	  0,   0, SEQ_CIDMODE,	150,  5, {0},	"^SGCI=1\r"},
+	{RSP_OK,	150, 150, -1,		  0,  0, {ACT_CMODESET} },
+	{RSP_ERROR,	150, 150, -1,		  0,  0, {ACT_FAILCMODE} },
+	{EV_TIMEOUT,	150, 150, -1,		  0,  0, {ACT_FAILCMODE} },
 
 /* leave cid mode */
-	{RSP_INIT,	  0,   0, SEQ_UMMODE,		160,  5, {0},	"Z\r"},
-	{RSP_OK,	160, 160, -1,			  0,  0, {ACT_UMODESET} },
-	{RSP_ERROR,	160, 160, -1,			  0,  0, {ACT_FAILUMODE} },
-	{EV_TIMEOUT,	160, 160, -1,			  0,  0, {ACT_FAILUMODE} },
+	{RSP_INIT,	  0,   0, SEQ_UMMODE,	160,  5, {0},	"Z\r"},
+	{RSP_OK,	160, 160, -1,		  0,  0, {ACT_UMODESET} },
+	{RSP_ERROR,	160, 160, -1,		  0,  0, {ACT_FAILUMODE} },
+	{EV_TIMEOUT,	160, 160, -1,		  0,  0, {ACT_FAILUMODE} },
 
 /* abort getting cid */
-	{RSP_INIT,	  0,   0, SEQ_NOCID,		  0,  0, {ACT_ABORTCID} },
+	{RSP_INIT,	  0,   0, SEQ_NOCID,	  0,  0, {ACT_ABORTCID} },
 
 /* reset */
-	{RSP_INIT,	  0,   0, SEQ_SHUTDOWN,		504,  5, {0},	"Z\r"},
-	{RSP_OK,	504, 504, -1,			  0,  0, {ACT_SDOWN} },
-	{RSP_ERROR,	501, 599, -1,			  0,  0, {ACT_FAILSDOWN} },
-	{EV_TIMEOUT,	501, 599, -1,			  0,  0, {ACT_FAILSDOWN} },
-	{RSP_NODEV,	501, 599, -1,			  0,  0, {ACT_FAKESDOWN} },
-
-	{EV_PROC_CIDMODE, -1, -1, -1,			 -1, -1, {ACT_PROC_CIDMODE} },
-	{EV_IF_LOCK,	 -1,  -1, -1,			 -1, -1, {ACT_IF_LOCK} },
-	{EV_IF_VER,	 -1,  -1, -1,			 -1, -1, {ACT_IF_VER} },
-	{EV_START,	 -1,  -1, -1,			 -1, -1, {ACT_START} },
-	{EV_STOP,	 -1,  -1, -1,			 -1, -1, {ACT_STOP} },
-	{EV_SHUTDOWN,	 -1,  -1, -1,			 -1, -1, {ACT_SHUTDOWN} },
+	{RSP_INIT,	  0,   0, SEQ_SHUTDOWN,	504,  5, {0},	"Z\r"},
+	{RSP_OK,	504, 504, -1,		  0,  0, {ACT_SDOWN} },
+	{RSP_ERROR,	501, 599, -1,		  0,  0, {ACT_FAILSDOWN} },
+	{EV_TIMEOUT,	501, 599, -1,		  0,  0, {ACT_FAILSDOWN} },
+	{RSP_NODEV,	501, 599, -1,		  0,  0, {ACT_FAKESDOWN} },
+
+	{EV_PROC_CIDMODE, -1, -1, -1,		 -1, -1, {ACT_PROC_CIDMODE} },
+	{EV_IF_LOCK,	 -1,  -1, -1,		 -1, -1, {ACT_IF_LOCK} },
+	{EV_IF_VER,	 -1,  -1, -1,		 -1, -1, {ACT_IF_VER} },
+	{EV_START,	 -1,  -1, -1,		 -1, -1, {ACT_START} },
+	{EV_STOP,	 -1,  -1, -1,		 -1, -1, {ACT_STOP} },
+	{EV_SHUTDOWN,	 -1,  -1, -1,		 -1, -1, {ACT_SHUTDOWN} },
 
 /* misc. */
-	{RSP_ERROR,	 -1,  -1, -1,			 -1, -1, {ACT_ERROR} },
-	{RSP_ZCAU,	 -1,  -1, -1,			 -1, -1, {ACT_ZCAU} },
-	{RSP_NONE,	 -1,  -1, -1,			 -1, -1, {ACT_DEBUG} },
-	{RSP_ANY,	 -1,  -1, -1,			 -1, -1, {ACT_WARN} },
+	{RSP_ERROR,	 -1,  -1, -1,		 -1, -1, {ACT_ERROR} },
+	{RSP_ZCAU,	 -1,  -1, -1,		 -1, -1, {ACT_ZCAU} },
+	{RSP_NONE,	 -1,  -1, -1,		 -1, -1, {ACT_DEBUG} },
+	{RSP_ANY,	 -1,  -1, -1,		 -1, -1, {ACT_WARN} },
 	{RSP_LAST}
 };
 
@@ -262,90 +262,90 @@ struct reply_t gigaset_tab_cid[] =
  * action, command */
 
 /* dial */
-	{EV_DIAL,	 -1,  -1, -1,			 -1, -1, {ACT_DIAL} },
-	{RSP_INIT,	  0,   0, SEQ_DIAL,		601,  5, {ACT_CMD + AT_BC} },
-	{RSP_OK,	601, 601, -1,			603,  5, {ACT_CMD + AT_PROTO} },
-	{RSP_OK,	603, 603, -1,			604,  5, {ACT_CMD + AT_TYPE} },
-	{RSP_OK,	604, 604, -1,			605,  5, {ACT_CMD + AT_MSN} },
-	{RSP_NULL,	605, 605, -1,			606,  5, {ACT_CMD + AT_CLIP} },
-	{RSP_OK,	605, 605, -1,			606,  5, {ACT_CMD + AT_CLIP} },
-	{RSP_NULL,	606, 606, -1,			607,  5, {ACT_CMD + AT_ISO} },
-	{RSP_OK,	606, 606, -1,			607,  5, {ACT_CMD + AT_ISO} },
-	{RSP_OK,	607, 607, -1,			608,  5, {0},	"+VLS=17\r"},
-	{RSP_OK,	608, 608, -1,			609, -1},
-	{RSP_ZSAU,	609, 609, ZSAU_PROCEEDING,	610,  5, {ACT_CMD + AT_DIAL} },
-	{RSP_OK,	610, 610, -1,			650,  0, {ACT_DIALING} },
-
-	{RSP_ERROR,	601, 610, -1,			  0,  0, {ACT_ABORTDIAL} },
-	{EV_TIMEOUT,	601, 610, -1,			  0,  0, {ACT_ABORTDIAL} },
+	{EV_DIAL,	 -1,  -1, -1,		 -1, -1, {ACT_DIAL} },
+	{RSP_INIT,	  0,   0, SEQ_DIAL,	601,  5, {ACT_CMD + AT_BC} },
+	{RSP_OK,	601, 601, -1,		603,  5, {ACT_CMD + AT_PROTO} },
+	{RSP_OK,	603, 603, -1,		604,  5, {ACT_CMD + AT_TYPE} },
+	{RSP_OK,	604, 604, -1,		605,  5, {ACT_CMD + AT_MSN} },
+	{RSP_NULL,	605, 605, -1,		606,  5, {ACT_CMD + AT_CLIP} },
+	{RSP_OK,	605, 605, -1,		606,  5, {ACT_CMD + AT_CLIP} },
+	{RSP_NULL,	606, 606, -1,		607,  5, {ACT_CMD + AT_ISO} },
+	{RSP_OK,	606, 606, -1,		607,  5, {ACT_CMD + AT_ISO} },
+	{RSP_OK,	607, 607, -1,		608,  5, {0},	"+VLS=17\r"},
+	{RSP_OK,	608, 608, -1,		609, -1},
+	{RSP_ZSAU,	609, 609, ZSAU_PROCEEDING, 610, 5, {ACT_CMD + AT_DIAL} },
+	{RSP_OK,	610, 610, -1,		650,  0, {ACT_DIALING} },
+
+	{RSP_ERROR,	601, 610, -1,		  0,  0, {ACT_ABORTDIAL} },
+	{EV_TIMEOUT,	601, 610, -1,		  0,  0, {ACT_ABORTDIAL} },
 
 /* optional dialing responses */
-	{EV_BC_OPEN,	650, 650, -1,			651, -1},
-	{RSP_ZVLS,	609, 651, 17,			 -1, -1, {ACT_DEBUG} },
-	{RSP_ZCTP,	610, 651, -1,			 -1, -1, {ACT_DEBUG} },
-	{RSP_ZCPN,	610, 651, -1,			 -1, -1, {ACT_DEBUG} },
-	{RSP_ZSAU,	650, 651, ZSAU_CALL_DELIVERED,	 -1, -1, {ACT_DEBUG} },
+	{EV_BC_OPEN,	650, 650, -1,		651, -1},
+	{RSP_ZVLS,	609, 651, 17,		 -1, -1, {ACT_DEBUG} },
+	{RSP_ZCTP,	610, 651, -1,		 -1, -1, {ACT_DEBUG} },
+	{RSP_ZCPN,	610, 651, -1,		 -1, -1, {ACT_DEBUG} },
+	{RSP_ZSAU,	650, 651, ZSAU_CALL_DELIVERED, -1, -1, {ACT_DEBUG} },
 
 /* connect */
-	{RSP_ZSAU,	650, 650, ZSAU_ACTIVE,		800, -1, {ACT_CONNECT} },
-	{RSP_ZSAU,	651, 651, ZSAU_ACTIVE,		800, -1, {ACT_CONNECT,
-								  ACT_NOTIFY_BC_UP} },
-	{RSP_ZSAU,	750, 750, ZSAU_ACTIVE,		800, -1, {ACT_CONNECT} },
-	{RSP_ZSAU,	751, 751, ZSAU_ACTIVE,		800, -1, {ACT_CONNECT,
-								  ACT_NOTIFY_BC_UP} },
-	{EV_BC_OPEN,	800, 800, -1,			800, -1, {ACT_NOTIFY_BC_UP} },
+	{RSP_ZSAU,	650, 650, ZSAU_ACTIVE,	800, -1, {ACT_CONNECT} },
+	{RSP_ZSAU,	651, 651, ZSAU_ACTIVE,	800, -1, {ACT_CONNECT,
+							  ACT_NOTIFY_BC_UP} },
+	{RSP_ZSAU,	750, 750, ZSAU_ACTIVE,	800, -1, {ACT_CONNECT} },
+	{RSP_ZSAU,	751, 751, ZSAU_ACTIVE,	800, -1, {ACT_CONNECT,
+							  ACT_NOTIFY_BC_UP} },
+	{EV_BC_OPEN,	800, 800, -1,		800, -1, {ACT_NOTIFY_BC_UP} },
 
 /* remote hangup */
-	{RSP_ZSAU,	650, 651, ZSAU_DISCONNECT_IND,	  0,  0, {ACT_REMOTEREJECT} },
-	{RSP_ZSAU,	750, 751, ZSAU_DISCONNECT_IND,	  0,  0, {ACT_REMOTEHUP} },
-	{RSP_ZSAU,	800, 800, ZSAU_DISCONNECT_IND,	  0,  0, {ACT_REMOTEHUP} },
+	{RSP_ZSAU,	650, 651, ZSAU_DISCONNECT_IND, 0, 0, {ACT_REMOTEREJECT} },
+	{RSP_ZSAU,	750, 751, ZSAU_DISCONNECT_IND, 0, 0, {ACT_REMOTEHUP} },
+	{RSP_ZSAU,	800, 800, ZSAU_DISCONNECT_IND, 0, 0, {ACT_REMOTEHUP} },
 
 /* hangup */
-	{EV_HUP,	 -1,  -1, -1,			 -1, -1, {ACT_HUP} },
-	{RSP_INIT,	 -1,  -1, SEQ_HUP,		401,  5, {0},	"+VLS=0\r"},
-	{RSP_OK,	401, 401, -1,			402,  5},
-	{RSP_ZVLS,	402, 402,  0,			403,  5},
-	{RSP_ZSAU,	403, 403, ZSAU_DISCONNECT_REQ,	 -1, -1, {ACT_DEBUG} },
-	{RSP_ZSAU,	403, 403, ZSAU_NULL,		  0,  0, {ACT_DISCONNECT} },
-	{RSP_NODEV,	401, 403, -1,			  0,  0, {ACT_FAKEHUP} },
-	{RSP_ERROR,	401, 401, -1,			  0,  0, {ACT_ABORTHUP} },
-	{EV_TIMEOUT,	401, 403, -1,			  0,  0, {ACT_ABORTHUP} },
-
-	{EV_BC_CLOSED,	  0,   0, -1,			  0, -1, {ACT_NOTIFY_BC_DOWN} },
+	{EV_HUP,	 -1,  -1, -1,		 -1, -1, {ACT_HUP} },
+	{RSP_INIT,	 -1,  -1, SEQ_HUP,	401,  5, {0},	"+VLS=0\r"},
+	{RSP_OK,	401, 401, -1,		402,  5},
+	{RSP_ZVLS,	402, 402,  0,		403,  5},
+	{RSP_ZSAU,	403, 403, ZSAU_DISCONNECT_REQ, -1, -1, {ACT_DEBUG} },
+	{RSP_ZSAU,	403, 403, ZSAU_NULL,	  0,  0, {ACT_DISCONNECT} },
+	{RSP_NODEV,	401, 403, -1,		  0,  0, {ACT_FAKEHUP} },
+	{RSP_ERROR,	401, 401, -1,		  0,  0, {ACT_ABORTHUP} },
+	{EV_TIMEOUT,	401, 403, -1,		  0,  0, {ACT_ABORTHUP} },
+
+	{EV_BC_CLOSED,	  0,   0, -1,		  0, -1, {ACT_NOTIFY_BC_DOWN} },
 
 /* ring */
-	{RSP_ZBC,	700, 700, -1,			 -1, -1, {0} },
-	{RSP_ZHLC,	700, 700, -1,			 -1, -1, {0} },
-	{RSP_NMBR,	700, 700, -1,			 -1, -1, {0} },
-	{RSP_ZCPN,	700, 700, -1,			 -1, -1, {0} },
-	{RSP_ZCTP,	700, 700, -1,			 -1, -1, {0} },
-	{EV_TIMEOUT,	700, 700, -1,			720, 720, {ACT_ICALL} },
-	{EV_BC_CLOSED,	720, 720, -1,			  0, -1, {ACT_NOTIFY_BC_DOWN} },
+	{RSP_ZBC,	700, 700, -1,		 -1, -1, {0} },
+	{RSP_ZHLC,	700, 700, -1,		 -1, -1, {0} },
+	{RSP_NMBR,	700, 700, -1,		 -1, -1, {0} },
+	{RSP_ZCPN,	700, 700, -1,		 -1, -1, {0} },
+	{RSP_ZCTP,	700, 700, -1,		 -1, -1, {0} },
+	{EV_TIMEOUT,	700, 700, -1,		720, 720, {ACT_ICALL} },
+	{EV_BC_CLOSED,	720, 720, -1,		  0, -1, {ACT_NOTIFY_BC_DOWN} },
 
 /*accept icall*/
-	{EV_ACCEPT,	 -1,  -1, -1,			 -1, -1, {ACT_ACCEPT} },
-	{RSP_INIT,	720, 720, SEQ_ACCEPT,		721,  5, {ACT_CMD + AT_PROTO} },
-	{RSP_OK,	721, 721, -1,			722,  5, {ACT_CMD + AT_ISO} },
-	{RSP_OK,	722, 722, -1,			723,  5, {0},	"+VLS=17\r"},
-	{RSP_OK,	723, 723, -1,			724,  5, {0} },
-	{RSP_ZVLS,	724, 724, 17,			750, 50, {ACT_ACCEPTED} },
-	{RSP_ERROR,	721, 729, -1,			  0,  0, {ACT_ABORTACCEPT} },
-	{EV_TIMEOUT,	721, 729, -1,			  0,  0, {ACT_ABORTACCEPT} },
-	{RSP_ZSAU,	700, 729, ZSAU_NULL,		  0,  0, {ACT_ABORTACCEPT} },
-	{RSP_ZSAU,	700, 729, ZSAU_ACTIVE,		  0,  0, {ACT_ABORTACCEPT} },
-	{RSP_ZSAU,	700, 729, ZSAU_DISCONNECT_IND,	  0,  0, {ACT_ABORTACCEPT} },
-
-	{EV_BC_OPEN,	750, 750, -1,			751, -1},
-	{EV_TIMEOUT,	750, 751, -1,			  0,  0, {ACT_CONNTIMEOUT} },
+	{EV_ACCEPT,	 -1,  -1, -1,		 -1, -1, {ACT_ACCEPT} },
+	{RSP_INIT,	720, 720, SEQ_ACCEPT,	721,  5, {ACT_CMD + AT_PROTO} },
+	{RSP_OK,	721, 721, -1,		722,  5, {ACT_CMD + AT_ISO} },
+	{RSP_OK,	722, 722, -1,		723,  5, {0},	"+VLS=17\r"},
+	{RSP_OK,	723, 723, -1,		724,  5, {0} },
+	{RSP_ZVLS,	724, 724, 17,		750, 50, {ACT_ACCEPTED} },
+	{RSP_ERROR,	721, 729, -1,		  0,  0, {ACT_ABORTACCEPT} },
+	{EV_TIMEOUT,	721, 729, -1,		  0,  0, {ACT_ABORTACCEPT} },
+	{RSP_ZSAU,	700, 729, ZSAU_NULL,	  0,  0, {ACT_ABORTACCEPT} },
+	{RSP_ZSAU,	700, 729, ZSAU_ACTIVE,	  0,  0, {ACT_ABORTACCEPT} },
+	{RSP_ZSAU,	700, 729, ZSAU_DISCONNECT_IND, 0, 0, {ACT_ABORTACCEPT} },
+
+	{EV_BC_OPEN,	750, 750, -1,		751, -1},
+	{EV_TIMEOUT,	750, 751, -1,		  0,  0, {ACT_CONNTIMEOUT} },
 
 /* B channel closed (general case) */
-	{EV_BC_CLOSED,	 -1,  -1, -1,			 -1, -1, {ACT_NOTIFY_BC_DOWN} },
+	{EV_BC_CLOSED,	 -1,  -1, -1,		 -1, -1, {ACT_NOTIFY_BC_DOWN} },
 
 /* misc. */
-	{RSP_ZCON,	 -1,  -1, -1,			 -1, -1, {ACT_DEBUG} },
-	{RSP_ZCAU,	 -1,  -1, -1,			 -1, -1, {ACT_ZCAU} },
-	{RSP_NONE,	 -1,  -1, -1,			 -1, -1, {ACT_DEBUG} },
-	{RSP_ANY,	 -1,  -1, -1,			 -1, -1, {ACT_WARN} },
+	{RSP_ZCON,	 -1,  -1, -1,		 -1, -1, {ACT_DEBUG} },
+	{RSP_ZCAU,	 -1,  -1, -1,		 -1, -1, {ACT_ZCAU} },
+	{RSP_NONE,	 -1,  -1, -1,		 -1, -1, {ACT_DEBUG} },
+	{RSP_ANY,	 -1,  -1, -1,		 -1, -1, {ACT_WARN} },
 	{RSP_LAST}
 };
 
@@ -649,9 +649,9 @@ static void disconnect(struct at_state_t **at_state_p)
 static inline struct at_state_t *get_free_channel(struct cardstate *cs,
 						  int cid)
 /* cids: >0: siemens-cid
-   0: without cid
-   -1: no cid assigned yet
-*/
+ *        0: without cid
+ *       -1: no cid assigned yet
+ */
 {
 	unsigned long flags;
 	int i;
@@ -1374,7 +1374,8 @@ static void do_action(int action, struct cardstate *cs,
 			 ev->parameter, at_state->ConState);
 		break;
 
-		/* events from the LL */
+	/* events from the LL */
+
 	case ACT_DIAL:
 		start_dial(at_state, ev->ptr, ev->parameter);
 		break;
@@ -1387,7 +1388,8 @@ static void do_action(int action, struct cardstate *cs,
 		cs->commands_pending = 1;
 		break;
 
-		/* hotplug events */
+	/* hotplug events */
+
 	case ACT_STOP:
 		do_stop(cs);
 		break;
@@ -1395,7 +1397,8 @@ static void do_action(int action, struct cardstate *cs,
 		do_start(cs);
 		break;
 
-		/* events from the interface */
+	/* events from the interface */
+
 	case ACT_IF_LOCK:
 		cs->cmd_result = ev->parameter ? do_lock(cs) : do_unlock(cs);
 		cs->waiting = 0;
@@ -1414,7 +1417,8 @@ static void do_action(int action, struct cardstate *cs,
 		wake_up(&cs->waitqueue);
 		break;
 
-		/* events from the proc file system */
+	/* events from the proc file system */
+
 	case ACT_PROC_CIDMODE:
 		spin_lock_irqsave(&cs->lock, flags);
 		if (ev->parameter != cs->cidmode) {
@@ -1433,7 +1437,8 @@ static void do_action(int action, struct cardstate *cs,
 		wake_up(&cs->waitqueue);
 		break;
 
-		/* events from the hardware drivers */
+	/* events from the hardware drivers */
+
 	case ACT_NOTIFY_BC_DOWN:
 		bchannel_down(bcs);
 		break;
diff --git a/drivers/isdn/gigaset/gigaset.h b/drivers/isdn/gigaset/gigaset.h
index 1dc2513..eae7351 100644
--- a/drivers/isdn/gigaset/gigaset.h
+++ b/drivers/isdn/gigaset/gigaset.h
@@ -163,8 +163,8 @@ void gigaset_dbg_buffer(enum debuglevel level, const unsigned char *msg,
 #define BAS_LOWFRAME	5	/* "    "    with negative flow control */
 #define BAS_CORRFRAMES	4	/* flow control multiplicator */
 
-#define BAS_INBUFSIZE	(BAS_MAXFRAME * BAS_NUMFRAMES)
-/* size of isoc in buf per URB */
+#define BAS_INBUFSIZE	(BAS_MAXFRAME * BAS_NUMFRAMES)	/* size of isoc in buf
+							 * per URB */
 #define BAS_OUTBUFSIZE	4096		/* size of common isoc out buffer */
 #define BAS_OUTBUFPAD	BAS_MAXFRAME	/* size of pad area for isoc out buf */
 
@@ -471,18 +471,18 @@ struct cardstate {
 					   for */
 	int commands_pending;		/* flag(s) in xxx.commands_pending have
 					   been set */
-	struct tasklet_struct event_tasklet;
-	/* tasklet for serializing AT commands.
-	 * Scheduled
-	 *   -> for modem reponses (and
-	 *      incoming data for M10x)
-	 *   -> on timeout
-	 *   -> after setting bits in
-	 *      xxx.at_state.pending_command
-	 *      (e.g. command from LL) */
-	struct tasklet_struct write_tasklet;
-	/* tasklet for serial output
-	 * (not used in base driver) */
+	struct tasklet_struct
+		event_tasklet;		/* tasklet for serializing AT commands.
+					 * Scheduled
+					 *   -> for modem reponses (and
+					 *      incoming data for M10x)
+					 *   -> on timeout
+					 *   -> after setting bits in
+					 *      xxx.at_state.pending_command
+					 *      (e.g. command from LL) */
+	struct tasklet_struct
+		write_tasklet;		/* tasklet for serial output
+					 * (not used in base driver) */
 
 	/* event queue */
 	struct event_t events[MAX_EVENTS];
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/8] isdn/gigaset: ratelimit CAPI message dumps
  2012-04-25 23:02 ` [PATCH 1/8] isdn/gigaset: ratelimit CAPI message dumps Tilman Schmidt
@ 2012-04-26  6:39   ` Karsten Keil
  2012-04-27 10:29     ` Tilman Schmidt
  0 siblings, 1 reply; 15+ messages in thread
From: Karsten Keil @ 2012-04-26  6:39 UTC (permalink / raw)
  To: Tilman Schmidt
  Cc: Karsten Keil, David Miller, Hansjoerg Lipp, i4ldeveloper, netdev,
	linux-kernel

Am 26.04.2012 01:02, schrieb Tilman Schmidt:
> Introduce a global ratelimit for CAPI message dumps to protect
> against possible log flood.
> Drop the ratelimit for ignored messages which is now covered by the
> global one.
> 

Hmm, I think the only CAPI messages which would need a ratelimit are
related to the DATA_B3 messages. If you need CAPI debug messages in most
cases you do not need all of the DATA_B3, but you do not want to miss
any other message related to the call control. With a general rate limit
you do not have the control, which messages are logged and which are not.
And here maybe some cases, when even the DATA_B3 are important (e.g.
searching bugs in flow control), so I would make it still conditional
to allow to print all messages.
And I'm not sure, if this is really something for stable.

> Signed-off-by: Tilman Schmidt <tilman@imap.cc>
> CC: stable <stable@kernel.org>
> ---
>  drivers/isdn/gigaset/capi.c |   22 +++++++++-------------
>  1 files changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/isdn/gigaset/capi.c b/drivers/isdn/gigaset/capi.c
> index 343b5c8..292ca2f 100644
> --- a/drivers/isdn/gigaset/capi.c
> +++ b/drivers/isdn/gigaset/capi.c
> @@ -14,6 +14,7 @@
>  #include "gigaset.h"
>  #include <linux/proc_fs.h>
>  #include <linux/seq_file.h>
> +#include <linux/ratelimit.h>
>  #include <linux/isdn/capilli.h>
>  #include <linux/isdn/capicmd.h>
>  #include <linux/isdn/capiutil.h>
> @@ -223,10 +224,14 @@ get_appl(struct gigaset_capi_ctr *iif, u16 appl)
>  static inline void dump_cmsg(enum debuglevel level, const char *tag, _cmsg *p)
>  {
>  #ifdef CONFIG_GIGASET_DEBUG
> +	/* dump at most 20 messages in 20 secs */
> +	static DEFINE_RATELIMIT_STATE(msg_dump_ratelimit, 20 * HZ, 20);
>  	_cdebbuf *cdb;
>  
>  	if (!(gigaset_debuglevel & level))
>  		return;
> +	if (!___ratelimit(&msg_dump_ratelimit, tag))
> +		return;
>  
>  	cdb = capi_cmsg2str(p);
>  	if (cdb) {
> @@ -2059,12 +2064,6 @@ static void do_reset_b3_req(struct gigaset_capi_ctr *iif,
>  }
>  
>  /*
> - * dump unsupported/ignored messages at most twice per minute,
> - * some apps send those very frequently
> - */
> -static unsigned long ignored_msg_dump_time;
> -
> -/*
>   * unsupported CAPI message handler
>   */
>  static void do_unsupported(struct gigaset_capi_ctr *iif,
> @@ -2073,8 +2072,7 @@ static void do_unsupported(struct gigaset_capi_ctr *iif,
>  {
>  	/* decode message */
>  	capi_message2cmsg(&iif->acmsg, skb->data);
> -	if (printk_timed_ratelimit(&ignored_msg_dump_time, 30 * 1000))
> -		dump_cmsg(DEBUG_CMD, __func__, &iif->acmsg);
> +	dump_cmsg(DEBUG_CMD, __func__, &iif->acmsg);
>  	send_conf(iif, ap, skb, CapiMessageNotSupportedInCurrentState);
>  }
>  
> @@ -2085,11 +2083,9 @@ static void do_nothing(struct gigaset_capi_ctr *iif,
>  		       struct gigaset_capi_appl *ap,
>  		       struct sk_buff *skb)
>  {
> -	if (printk_timed_ratelimit(&ignored_msg_dump_time, 30 * 1000)) {
> -		/* decode message */
> -		capi_message2cmsg(&iif->acmsg, skb->data);
> -		dump_cmsg(DEBUG_CMD, __func__, &iif->acmsg);
> -	}
> +	/* decode message */
> +	capi_message2cmsg(&iif->acmsg, skb->data);
> +	dump_cmsg(DEBUG_CMD, __func__, &iif->acmsg);
>  	dev_kfree_skb_any(skb);
>  }
>  


-- 
Karsten Keil
Linux Kernel Development
Tel: +49 175 7249132
Mail: keil@b1-systems.de

B1 Systems GmbH
Osterfeldstraße 7 / 85088 Vohburg / http://www.b1-systems.de
GF: Ralph Dehner / Unternehmenssitz: Vohburg / AG: Ingolstadt,HRB 3537

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/8] isdn/gigaset: ratelimit CAPI message dumps
  2012-04-26  6:39   ` Karsten Keil
@ 2012-04-27 10:29     ` Tilman Schmidt
  2012-04-28  9:29       ` Karsten Keil
  0 siblings, 1 reply; 15+ messages in thread
From: Tilman Schmidt @ 2012-04-27 10:29 UTC (permalink / raw)
  To: Karsten Keil
  Cc: Karsten Keil, David Miller, Hansjoerg Lipp, i4ldeveloper, netdev,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2320 bytes --]

Am 26.04.2012 08:39, schrieb Karsten Keil:
> Am 26.04.2012 01:02, schrieb Tilman Schmidt:
>> Introduce a global ratelimit for CAPI message dumps to protect
>> against possible log flood.
>> Drop the ratelimit for ignored messages which is now covered by the
>> global one.
> 
> Hmm, I think the only CAPI messages which would need a ratelimit are
> related to the DATA_B3 messages. If you need CAPI debug messages in most
> cases you do not need all of the DATA_B3, but you do not want to miss
> any other message related to the call control. With a general rate limit
> you do not have the control, which messages are logged and which are not.

The ratelimit introduced by this patch only applies to messages
other than DATA_B3. Logging DATA_B3 messages is not done via
dump_cmsg().

I'd like to ratelimit specifically non-DATA_B3 messages because I
saw a (possibly buggy) CAPI application flooding the log with
FACILITY messages. Equally important, I'd like to make the
ratelimit in do_nothing() / do_unsupported() bursty because I had
a case where I needed to see several ignored/unhandled CAPI
messages in quick succession. So this patch is killing two birds
with one stone for me.

The burst limit of 20 messages in 20 seconds is chosen to allow a
complete call setup sequence to be logged, while limiting to one
message per second in the long run.

> And here maybe some cases, when even the DATA_B3 are important (e.g.
> searching bugs in flow control), so I would make it still conditional
> to allow to print all messages.

DATA_B3 dumps produce an enormous amount of log data and are
therefore controlled separately by the DEBUG_MCMD flag.
Someone who enables that should know what she or he does.
But if you need them, you need them all. A ratelimit doesn't
make sense there in my experience.

> And I'm not sure, if this is really something for stable.

It's pretty simple and localized, a net simplification, and only
affects generation of debugging messages, so I think it's safe.
But if you see a problem there I can drop the "CC: stable" line.

Thanks,
Tilman

-- 
Tilman Schmidt                    E-Mail: tilman@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/8] isdn/gigaset: ratelimit CAPI message dumps
  2012-04-27 10:29     ` Tilman Schmidt
@ 2012-04-28  9:29       ` Karsten Keil
  0 siblings, 0 replies; 15+ messages in thread
From: Karsten Keil @ 2012-04-28  9:29 UTC (permalink / raw)
  To: Tilman Schmidt
  Cc: Karsten Keil, David Miller, Hansjoerg Lipp, i4ldeveloper, netdev,
	linux-kernel

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 27.04.2012 12:29, schrieb Tilman Schmidt:
> Am 26.04.2012 08:39, schrieb Karsten Keil:
>> Am 26.04.2012 01:02, schrieb Tilman Schmidt:
>>> Introduce a global ratelimit for CAPI message dumps to protect
>>>  against possible log flood. Drop the ratelimit for ignored 
>>> messages which is now covered by the global one.
>> 
>> Hmm, I think the only CAPI messages which would need a ratelimit 
>> are related to the DATA_B3 messages. If you need CAPI debug 
>> messages in most cases you do not need all of the DATA_B3, but 
>> you do not want to miss any other message related to the call 
>> control. With a general rate limit you do not have the control, 
>> which messages are logged and which are not.
> 
> The ratelimit introduced by this patch only applies to messages 
> other than DATA_B3. Logging DATA_B3 messages is not done via 
> dump_cmsg().
> 

Thanks for the clarification, forget about my objection.
I ack this patch now.

> I'd like to ratelimit specifically non-DATA_B3 messages because I 
> saw a (possibly buggy) CAPI application flooding the log with 
> FACILITY messages. Equally important, I'd like to make the 
> ratelimit in do_nothing() / do_unsupported() bursty because I had
> a case where I needed to see several ignored/unhandled CAPI
> messages in quick succession. So this patch is killing two birds
> with one stone for me.
> 
> The burst limit of 20 messages in 20 seconds is chosen to allow a 
> complete call setup sequence to be logged, while limiting to one 
> message per second in the long run.
> 
>> And here maybe some cases, when even the DATA_B3 are important 
>> (e.g. searching bugs in flow control), so I would make it still 
>> conditional to allow to print all messages.
> 
> DATA_B3 dumps produce an enormous amount of log data and are 
> therefore controlled separately by the DEBUG_MCMD flag. Someone
> who enables that should know what she or he does. But if you need
> them, you need them all. A ratelimit doesn't make sense there in
> my experience.
> 
>> And I'm not sure, if this is really something for stable.
> 
> It's pretty simple and localized, a net simplification, and only 
> affects generation of debugging messages, so I think it's safe.
> But if you see a problem there I can drop the "CC: stable" line.
> 

I let the decision about it to you and the stable maintainers.

- -- 
Karsten Keil
Linux Kernel Development
Tel: +49 175 7249132
Mail: keil@b1-systems.de

B1 Systems GmbH
Osterfeldstraße 7 / 85088 Vohburg / http://www.b1-systems.de
GF: Ralph Dehner / Unternehmenssitz: Vohburg / AG: Ingolstadt,HRB 3537
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.16 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk+buHwACgkQo5VVC52CNcRoygCfWwPlWZ+A48OwEkr/MtK6PeNG
0UEAnipdxPSZDKa4s99LlGYwvggWIIAJ
=CLr6
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/8] ISDN patches for net-next
  2012-04-25 23:02 [PATCH 0/8] ISDN patches for net-next Tilman Schmidt
                   ` (7 preceding siblings ...)
  2012-04-25 23:02 ` [PATCH 6/8] isdn/gigaset: unify function return values Tilman Schmidt
@ 2012-05-08  0:24 ` Tilman Schmidt
  2012-05-08  2:29   ` David Miller
  8 siblings, 1 reply; 15+ messages in thread
From: Tilman Schmidt @ 2012-05-08  0:24 UTC (permalink / raw)
  To: Karsten Keil, David Miller, netdev
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1030 bytes --]

On 26.04.2012 01:02, /me wrote:
> here's a series of eight patches to the Kernel CAPI subsystem and
> the Gigaset driver. Please consider for application to net-next.
> The first three should also go into -stable and are tagged accordingly.

Is there anything missing for these to be accepted into net-next?
Or should I just be patient?

Thanks,
Tilman

> Tilman Schmidt (8):
>       isdn/gigaset: ratelimit CAPI message dumps
>       isdn/gigaset: fix CAPI disconnect B3 handling
>       isdn/gigaset: improve error handling querying firmware version
>       isdn/gigaset: fix readability damage 
>       isdn/gigaset: internal function name cleanup
>       isdn/gigaset: unify function return values
>       isdn/capi: fix readability damage
>       isdn/capi: elliminate capincci_find() in non-middleware case

-- 
Tilman Schmidt                    E-Mail: tilman@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/8] ISDN patches for net-next
  2012-05-08  0:24 ` [PATCH 0/8] ISDN patches for net-next Tilman Schmidt
@ 2012-05-08  2:29   ` David Miller
  2012-05-08  2:42     ` David Miller
  0 siblings, 1 reply; 15+ messages in thread
From: David Miller @ 2012-05-08  2:29 UTC (permalink / raw)
  To: tilman; +Cc: isdn, netdev, hjlipp, keil, i4ldeveloper, linux-kernel

From: Tilman Schmidt <tilman@imap.cc>
Date: Tue, 08 May 2012 02:24:17 +0200

> On 26.04.2012 01:02, /me wrote:
>> here's a series of eight patches to the Kernel CAPI subsystem and
>> the Gigaset driver. Please consider for application to net-next.
>> The first three should also go into -stable and are tagged accordingly.
> 
> Is there anything missing for these to be accepted into net-next?
> Or should I just be patient?

I'll take care of it, I just put them back into "under review" state
in patchwork.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/8] ISDN patches for net-next
  2012-05-08  2:29   ` David Miller
@ 2012-05-08  2:42     ` David Miller
  0 siblings, 0 replies; 15+ messages in thread
From: David Miller @ 2012-05-08  2:42 UTC (permalink / raw)
  To: tilman; +Cc: isdn, netdev, hjlipp, keil, i4ldeveloper, linux-kernel

From: David Miller <davem@davemloft.net>
Date: Mon, 07 May 2012 22:29:12 -0400 (EDT)

> From: Tilman Schmidt <tilman@imap.cc>
> Date: Tue, 08 May 2012 02:24:17 +0200
> 
>> On 26.04.2012 01:02, /me wrote:
>>> here's a series of eight patches to the Kernel CAPI subsystem and
>>> the Gigaset driver. Please consider for application to net-next.
>>> The first three should also go into -stable and are tagged accordingly.
>> 
>> Is there anything missing for these to be accepted into net-next?
>> Or should I just be patient?
> 
> I'll take care of it, I just put them back into "under review" state
> in patchwork.

All applied and pushed out to net-next, thanks Tilman.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2012-05-08  2:42 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-25 23:02 [PATCH 0/8] ISDN patches for net-next Tilman Schmidt
2012-04-25 23:02 ` [PATCH 1/8] isdn/gigaset: ratelimit CAPI message dumps Tilman Schmidt
2012-04-26  6:39   ` Karsten Keil
2012-04-27 10:29     ` Tilman Schmidt
2012-04-28  9:29       ` Karsten Keil
2012-04-25 23:02 ` [PATCH 5/8] isdn/gigaset: internal function name cleanup Tilman Schmidt
2012-04-25 23:02 ` [PATCH 2/8] isdn/gigaset: fix CAPI disconnect B3 handling Tilman Schmidt
2012-04-25 23:02 ` [PATCH 7/8] isdn/capi: fix readability damage Tilman Schmidt
2012-04-25 23:02 ` [PATCH 4/8] isdn/gigaset: " Tilman Schmidt
2012-04-25 23:02 ` [PATCH 3/8] isdn/gigaset: improve error handling querying firmware version Tilman Schmidt
2012-04-25 23:02 ` [PATCH 8/8] isdn/capi: elliminate capincci_find() in non-middleware case Tilman Schmidt
2012-04-25 23:02 ` [PATCH 6/8] isdn/gigaset: unify function return values Tilman Schmidt
2012-05-08  0:24 ` [PATCH 0/8] ISDN patches for net-next Tilman Schmidt
2012-05-08  2:29   ` David Miller
2012-05-08  2:42     ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).