Skip to contentSkip to conversation

Gigaset driver bugfixes and cleanups

PATCH In review Tilman Schmidt 2 participants 10 messages May 13, 2009 – May 18, 2009

Subsystems (3)
lore
Share & tools
lore
#1 lore raw

PATCH 1/8 gigaset: documentation update

In reply to Tilman Schmidt

Mention handling of unregisteted DECT wireless datasets in README.gigaset.

---
 Documentation/isdn/README.gigaset |   23 ++++++++++++++++++++++-
 1 files changed, 22 insertions(+), 1 deletions(-)
Documentation/isdn/README.gigaset +22 -1
index 02c0e93..f6e9eaa 100644
@@ -192,7 +192,6 @@ GigaSet 307x Device Driver
 
 2.6. M105 Undocumented USB Requests
      ------------------------------
-
      The Gigaset M105 USB data box understands a couple of useful, but
      undocumented USB commands. These requests are not used in normal
      operation (for wireless access to the base), but are needed for access
@@ -204,6 +203,20 @@ GigaSet 307x Device Driver
      M105, try setting that option to "y" via 'make {x,menu}config' and
      recompiling the driver.
 
+2.7. Unregistered Wireless Devices (M101/M105)
+     -----------------------------------------
+     The main purpose of the ser_gigaset and usb_gigaset drivers is to allow
+     the M101 and M105 wireless devices to be used as ISDN devices for ISDN
+     connections through a Gigaset base. Therefore they assume that the device
+     is registered to a DECT base.
+
+     If the M101/M105 device is not registered to a base, initialization of
+     the device fails, and a corresponding error message is logged by the
+     driver. In that situation, a restricted set of functions is available
+     which includes, in particular, those necessary for registering the device
+     to a base or for switching it between Fixed Part and Portable Part
+     modes. For the M105, these commands require the "Support for undocumented
+     USB requests" configuration option (see section 2.6.) to be enabled.
 
 3.   Troubleshooting
      ---------------
@@ -240,6 +253,14 @@ GigaSet 307x Device Driver
         Recompile the usb_gigaset driver with the kernel configuration option
         CONFIG_GIGASET_UNDOCREQ set to 'y'. (see section 2.6.)
 
+     Problem:
+	Messages like this:
+	    usb_gigaset 3-2:1.0: Could not initialize the device.
+	appear in your syslog.
+     Solution:
+	Check whether your M10x wireless device is correctly registered to the
+	Gigaset base. (see section 2.7.)
+
 3.2. Telling the driver to provide more information
      ----------------------------------------------
      Building the driver with the "Gigaset debugging" kernel configuration
Signature · 1 line
1.6.2.1.214.ge986c
#2 lore raw

PATCH 2/8 gigaset: fix possible oops in error handling

In reply to Tilman Schmidt

Use pr_warning() / pr_err() instead of dev_warn() / dev_err() in two places where the dev pointer isn't guaranteed to be valid.

Impact: error handling bugfix

---
 drivers/isdn/gigaset/interface.c |    3 +--
 drivers/isdn/gigaset/proc.c      |    2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)
drivers/isdn/gigaset/interface.c +1 -2
index 820a309..5a8e259 100644
@@ -599,8 +599,7 @@ void gigaset_if_init(struct cardstate *cs)
 	if (!IS_ERR(cs->tty_dev))
 		dev_set_drvdata(cs->tty_dev, cs);
 	else {
-		dev_warn(cs->dev,
-			 "could not register device to the tty subsystem\n");
+		pr_warning("could not register device to the tty subsystem\n");
 		cs->tty_dev = NULL;
 	}
 	mutex_unlock(&cs->mutex);
drivers/isdn/gigaset/proc.c +1 -1
index da6f3ac..9715aad 100644
@@ -79,5 +79,5 @@ void gigaset_init_dev_sysfs(struct cardstate *cs)
 
 	gig_dbg(DEBUG_INIT, "setting up sysfs");
 	if (device_create_file(cs->tty_dev, &dev_attr_cidmode))
-		dev_err(cs->dev, "could not create sysfs attribute\n");
+		pr_err("could not create sysfs attribute\n");
 }
Signature · 1 line
1.6.2.1.214.ge986c
#5 lore raw

PATCH 5/8 gigaset: remove obsolete references to m10x state table

In reply to Tilman Schmidt

The separation of state tables for base and M10x has long been removed. Clean up remaining traces of it.

Impact: cleanup

---
 drivers/isdn/gigaset/common.c   |   10 ++--------
 drivers/isdn/gigaset/ev-layer.c |    4 ++--
 drivers/isdn/gigaset/gigaset.h  |    4 ++--
 3 files changed, 6 insertions(+), 12 deletions(-)
drivers/isdn/gigaset/common.c +2 -8
index 0048ce9..79c9b0f 100644
@@ -672,14 +672,8 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
 	cs->tty = NULL;
 	cs->tty_dev = NULL;
 	cs->cidmode = cidmode != 0;
-
-	//if(onechannel) { //FIXME
-		cs->tabnocid = gigaset_tab_nocid_m10x;
-		cs->tabcid = gigaset_tab_cid_m10x;
-	//} else {
-	//	cs->tabnocid = gigaset_tab_nocid;
-	//	cs->tabcid = gigaset_tab_cid;
-	//}
+	cs->tabnocid = gigaset_tab_nocid;
+	cs->tabcid = gigaset_tab_cid;
 
 	init_waitqueue_head(&cs->waitqueue);
 	cs->waiting = 0;
drivers/isdn/gigaset/ev-layer.c +2 -2
index e582a48..ec51696 100644
@@ -160,7 +160,7 @@
 
 
 // 100: init, 200: dle0, 250:dle1, 300: get cid (dial), 350: "hup" (no cid), 400: hup, 500: reset, 600: dial, 700: ring
-struct reply_t gigaset_tab_nocid_m10x[]= /* with dle mode */
+struct reply_t gigaset_tab_nocid[] =
 {
 	/* resp_code, min_ConState, max_ConState, parameter, new_ConState, timeout, action, command */
 
@@ -280,7 +280,7 @@ struct reply_t gigaset_tab_nocid_m10x[]= /* with dle mode */
 };
 
 // 600: start dialing, 650: dial in progress, 800: connection is up, 700: ring, 400: hup, 750: accepted icall
-struct reply_t gigaset_tab_cid_m10x[] = /* for M10x */
+struct reply_t gigaset_tab_cid[] =
 {
 	/* resp_code, min_ConState, max_ConState, parameter, new_ConState, timeout, action, command */
 
drivers/isdn/gigaset/gigaset.h +2 -2
index 747178f..d5f0110 100644
@@ -282,8 +282,8 @@ struct reply_t {
 	char	*command;	/* NULL==none */
 };
 
-extern struct reply_t gigaset_tab_cid_m10x[];
-extern struct reply_t gigaset_tab_nocid_m10x[];
+extern struct reply_t gigaset_tab_cid[];
+extern struct reply_t gigaset_tab_nocid[];
 
 struct inbuf_t {
 	unsigned char		*rcvbuf;	/* usb-gigaset receive buffer */
Signature · 1 line
1.6.2.1.214.ge986c
#6 lore raw

PATCH 6/8 gigaset: remove unused structure member rcvbytes

In reply to Tilman Schmidt

The B channel data structure member rcvbytes was never set to anything else but zero, so drop it.

Impact: cleanup

---
 drivers/isdn/gigaset/asyncdata.c |    5 ++---
 drivers/isdn/gigaset/common.c    |    2 --
 drivers/isdn/gigaset/gigaset.h   |    1 -
 3 files changed, 2 insertions(+), 6 deletions(-)
drivers/isdn/gigaset/asyncdata.c +2 -3
index 2a4ce96..234cc5d 100644
@@ -174,9 +174,8 @@ byte_stuff:
 
 				if (unlikely(fcs != PPP_GOODFCS)) {
 					dev_err(cs->dev,
-					    "Packet checksum at %lu failed, "
-					    "packet is corrupted (%u bytes)!\n",
-					    bcs->rcvbytes, skb->len);
+				"Checksum failed, %u bytes corrupted!\n",
+						skb->len);
 					compskb = NULL;
 					gigaset_rcv_error(compskb, cs, bcs);
 					error = 1;
drivers/isdn/gigaset/common.c +0 -2
index 79c9b0f..e4141bf 100644
@@ -565,8 +565,6 @@ static struct bc_state *gigaset_initbcs(struct bc_state *bcs,
 	gig_dbg(DEBUG_INIT, "setting up bcs[%d]->at_state", channel);
 	gigaset_at_init(&bcs->at_state, bcs, cs, -1);
 
-	bcs->rcvbytes = 0;
-
 #ifdef CONFIG_GIGASET_DEBUG
 	bcs->emptycount = 0;
 #endif
drivers/isdn/gigaset/gigaset.h +0 -1
index d5f0110..a2f6125 100644
@@ -384,7 +384,6 @@ struct bc_state {
 	int trans_up;			/* Counter of packages (upstream) */
 
 	struct at_state_t at_state;
-	unsigned long rcvbytes;
 
 	__u16 fcs;
 	struct sk_buff *skb;
Signature · 1 line
1.6.2.1.214.ge986c
#7 lore raw

PATCH 7/8 gigaset: remove UNDOCREQ config option

In reply to Tilman Schmidt

Drop the kernel config option GIGASET_UNDOCREQ, permanently activating the code it controlled, as there have been no reports of problems caused by its activation but many problems caused by it being disabled.
Also fix a few bad comments while we're at it.

Impact: cleanup

---
 Documentation/isdn/README.gigaset  |   33 +++----------------
 drivers/isdn/gigaset/Kconfig       |    7 ----
 drivers/isdn/gigaset/usb-gigaset.c |   62 +++++++++++++----------------------
 3 files changed, 28 insertions(+), 74 deletions(-)
Documentation/isdn/README.gigaset +5 -28
index f6e9eaa..f996310 100644
@@ -149,10 +149,8 @@ GigaSet 307x Device Driver
      configuration files and chat scripts in the gigaset-VERSION/ppp directory
      in the driver packages from http://sourceforge.net/projects/gigaset307x/.
      Please note that the USB drivers are not able to change the state of the
-     control lines (the M105 driver can be configured to use some undocumented
-     control requests, if you really need the control lines, though). This means
-     you must use "Stupid Mode" if you are using wvdial or you should use the
-     nocrtscts option of pppd.
+     control lines. This means you must use "Stupid Mode" if you are using
+     wvdial or you should use the nocrtscts option of pppd.
      You must also assure that the ppp_async module is loaded with the parameter
      flag_time=0. You can do this e.g. by adding a line like
 
@@ -190,20 +188,7 @@ GigaSet 307x Device Driver
      You can also use /sys/class/tty/ttyGxy/cidmode for changing the CID mode
      setting (ttyGxy is ttyGU0 or ttyGB0).
 
-2.6. M105 Undocumented USB Requests
-     ------------------------------
-     The Gigaset M105 USB data box understands a couple of useful, but
-     undocumented USB commands. These requests are not used in normal
-     operation (for wireless access to the base), but are needed for access
-     to the M105's own configuration mode (registration to the base, baudrate
-     and line format settings, device status queries) via the gigacontr
-     utility. Their use is controlled by the kernel configuration option
-     "Support for undocumented USB requests" (CONFIG_GIGASET_UNDOCREQ). If you
-     encounter error code -ENOTTY when trying to use some features of the
-     M105, try setting that option to "y" via 'make {x,menu}config' and
-     recompiling the driver.
-
-2.7. Unregistered Wireless Devices (M101/M105)
+2.6. Unregistered Wireless Devices (M101/M105)
      -----------------------------------------
      The main purpose of the ser_gigaset and usb_gigaset drivers is to allow
      the M101 and M105 wireless devices to be used as ISDN devices for ISDN
@@ -215,8 +200,7 @@ GigaSet 307x Device Driver
      driver. In that situation, a restricted set of functions is available
      which includes, in particular, those necessary for registering the device
      to a base or for switching it between Fixed Part and Portable Part
-     modes. For the M105, these commands require the "Support for undocumented
-     USB requests" configuration option (see section 2.6.) to be enabled.
+     modes.
 
 3.   Troubleshooting
      ---------------
@@ -247,19 +231,12 @@ GigaSet 307x Device Driver
         Select Unimodem mode for all DECT data adapters. (see section 2.4.)
 
      Problem:
-        You want to configure your USB DECT data adapter (M105) but gigacontr
-        reports an error: "/dev/ttyGU0: Inappropriate ioctl for device".
-     Solution:
-        Recompile the usb_gigaset driver with the kernel configuration option
-        CONFIG_GIGASET_UNDOCREQ set to 'y'. (see section 2.6.)
-
-     Problem:
 	Messages like this:
 	    usb_gigaset 3-2:1.0: Could not initialize the device.
 	appear in your syslog.
      Solution:
 	Check whether your M10x wireless device is correctly registered to the
-	Gigaset base. (see section 2.7.)
+	Gigaset base. (see section 2.6.)
 
 3.2. Telling the driver to provide more information
      ----------------------------------------------
drivers/isdn/gigaset/Kconfig +0 -7
index 9ca889a..65ca305 100644
@@ -42,11 +42,4 @@ config GIGASET_DEBUG
 	  This enables debugging code in the Gigaset drivers.
 	  If in doubt, say yes.
 
-config GIGASET_UNDOCREQ
-	bool "Support for undocumented USB requests"
-	help
-	  This enables support for USB requests we only know from
-	  reverse engineering (currently M105 only). If you need
-	  features like configuration mode of M105, say yes.
-
 endif # ISDN_DRV_GIGASET
drivers/isdn/gigaset/usb-gigaset.c +23 -39

136 lines not on this page · raw message

Signature · 1 line
1.6.2.1.214.ge986c
#8 lore raw

PATCH 8/8 gigaset: move up Kconfig inclusion point

In reply to Tilman Schmidt

In preparation for porting to kernel CAPI subsystem, include the Gigaset driver's Kconfig directly from ISDN's instead of I4L's.

Impact: Kconfig reorganisation, no functional change

---
 drivers/isdn/Kconfig         |    2 ++
 drivers/isdn/gigaset/Kconfig |    1 +
 drivers/isdn/i4l/Kconfig     |    2 --
 3 files changed, 3 insertions(+), 2 deletions(-)
drivers/isdn/Kconfig +2 -0
index 3d113c6..02bdca6 100644
@@ -61,4 +61,6 @@ source "drivers/isdn/hardware/Kconfig"
 
 endif # ISDN_CAPI
 
+source "drivers/isdn/gigaset/Kconfig"
+
 endif # ISDN
drivers/isdn/gigaset/Kconfig +1 -0
index 65ca305..18ab865 100644
@@ -1,5 +1,6 @@
 menuconfig ISDN_DRV_GIGASET
 	tristate "Siemens Gigaset support"
+	depends on ISDN_I4L
 	select CRC_CCITT
 	select BITREVERSE
 	help
drivers/isdn/i4l/Kconfig +0 -2
index 36778b2..ed3510f 100644
@@ -135,5 +135,3 @@ source "drivers/isdn/act2000/Kconfig"
 source "drivers/isdn/hysdn/Kconfig"
 
 endmenu
-
-source "drivers/isdn/gigaset/Kconfig"
Signature · 1 line
1.6.2.1.214.ge986c
#0 lore raw

cover Gigaset driver bugfixes and cleanups

Dave,

following are 8 small patches for the Gigaset driver I'd like to get out from under my feet. Nothing urgent, so if you feel they are inappropriate for merging that late in the 2.6.30 cycle they can as well wait for 2.6.31.

Thanks,
Tilman

#3 lore raw

PATCH 3/8 gigaset: fix error return code

gigaset_register_to_LL() is expected to print a message and return 0 on failure. Make it do so consistently.

Impact: error handling bugfix

---
 drivers/isdn/gigaset/i4l.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)
drivers/isdn/gigaset/i4l.c +7 -5
index 69a702f..9b22f9c 100644
@@ -544,11 +544,11 @@ int gigaset_register_to_LL(struct cardstate *cs, const char *isdnid)
 
 	gig_dbg(DEBUG_ANY, "Register driver capabilities to LL");
 
-	//iif->id[sizeof(iif->id) - 1]=0;
-	//strncpy(iif->id, isdnid, sizeof(iif->id) - 1);
 	if (snprintf(iif->id, sizeof iif->id, "%s_%u", isdnid, cs->minor_index)
-	    >= sizeof iif->id)
-		return -ENOMEM; //FIXME EINVAL/...??
+	    >= sizeof iif->id) {
+		pr_err("ID too long: %s\n", isdnid);
+		return 0;
+	}
 
 	iif->owner = THIS_MODULE;
 	iif->channels = cs->channels;
@@ -568,8 +568,10 @@ int gigaset_register_to_LL(struct cardstate *cs, const char *isdnid)
 	iif->rcvcallb_skb = NULL;		/* Will be set by LL */
 	iif->statcallb = NULL;			/* Will be set by LL */
 
-	if (!register_isdn(iif))
+	if (!register_isdn(iif)) {
+		pr_err("register_isdn failed\n");
 		return 0;
+	}
 
 	cs->myid = iif->channels;		/* Set my device id */
 	return 1;
Signature · 1 line
1.6.2.1.214.ge986c
#4 lore raw

PATCH 4/8 gigaset: skip unnecessary hex formatting

In reply to Tilman Schmidt

Don't generate the hex representation of the payload data if it isn't actually used afterwards.

Impact: optimization

---
 drivers/isdn/gigaset/isocdata.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
drivers/isdn/gigaset/isocdata.c +4 -0
index b171e75..09202a5 100644
@@ -246,6 +246,10 @@ static inline void dump_bytes(enum debuglevel level, const char *tag,
 	unsigned char c;
 	static char dbgline[3 * 32 + 1];
 	int i = 0;
+
+	if (!(gigaset_debuglevel & level))
+		return;
+
 	while (count-- > 0) {
 		if (i > sizeof(dbgline) - 4) {
 			dbgline[i] = '\0';
Signature · 1 line
1.6.2.1.214.ge986c
maintainer
#9 lore raw
In reply to Tilman Schmidt

From: Tilman Schmidt <[email protected]>
Date: Thu, 14 May 2009 00:44:17 +0200 (CEST)

Quoted 4 lines

following are 8 small patches for the Gigaset driver I'd like to get out from under my feet. Nothing urgent, so if you feel they are inappropriate for merging that late in the 2.6.30 cycle they can as well wait for 2.6.31.

All applied to net-next-2.6, thanks!