Skip to contentSkip to conversation

gigaset: patches for 2.6.30

PATCH Superseded Tilman Schmidt 3 participants 12 messages Mar 7, 2009 – Mar 10, 2009

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover gigaset: patches for 2.6.30

Dave,

following are two patches for the Gigaset driver I'd like to see included in release 2.6.30. Would you please take them through your tree.

Thanks,
Tilman

#1 lore raw

PATCH 1/2 gigaset: return -ENOSYS for unimplemented functions

From: Paul Bolle <[email protected]>

A number of functions in the usb_gigaset module will return -EINVAL if CONFIG_GIGASET_UNDOCREQ is not set. Make these return -ENOSYS as it's more specific and it might make it easier to see (from userspace) why these functions actually fail.

Impact: some error return codes changed

---
 drivers/isdn/gigaset/usb-gigaset.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
drivers/isdn/gigaset/usb-gigaset.c +4 -4
index fba61f6..2bd6d9d 100644
@@ -278,17 +278,17 @@ static int gigaset_set_line_ctrl(struct cardstate *cs, unsigned cflag)
 static int gigaset_set_modem_ctrl(struct cardstate *cs, unsigned old_state,
 				  unsigned new_state)
 {
-	return -EINVAL;
+	return -ENOSYS;
 }
 
 static int gigaset_set_line_ctrl(struct cardstate *cs, unsigned cflag)
 {
-	return -EINVAL;
+	return -ENOSYS;
 }
 
 static int gigaset_baud_rate(struct cardstate *cs, unsigned cflag)
 {
-	return -EINVAL;
+	return -ENOSYS;
 }
 #endif
 
@@ -577,7 +577,7 @@ static int gigaset_brkchars(struct cardstate *cs, const unsigned char buf[6])
 	return usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x19, 0x41,
 			       0, 0, &buf, 6, 2000);
 #else
-	return -EINVAL;
+	return -ENOSYS;
 #endif
 }
 
Signature · 1 line
1.5.4.7.gd8534-dirty
#2 lore raw

Re: [PATCH 1/2] gigaset: return -ENOSYS for unimplemented functions

Quoted 8 lines · On Sat, 7 Mar 2009 23:10:57 +0100 (CET) Tilman Schmidt <[email protected]> wrote:

From: Paul Bolle <[email protected]>

A number of functions in the usb_gigaset module will return -EINVAL if CONFIG_GIGASET_UNDOCREQ is not set. Make these return -ENOSYS as it's more specific and it might make it easier to see (from userspace) why these functions actually fail.

Impact: some error return codes changed

ENODEV is what would be more appropriate.

ENOSYS shuoldn't be returned from drivers, only from unimplemented system calls!

Signature · 3 lines
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org
#3 lore raw

Re: [PATCH 1/2] gigaset: return -ENOSYS for unimplemented functions

Am 07.03.2009 23:26 schrieb Arjan van de Ven:

Quoted 5 lines
Quoted 2 lines · On Sat, 7 Mar 2009 23:10:57 +0100 (CET) Tilman Schmidt <[email protected]> wrote:

From: Paul Bolle <[email protected]>

Quoted 8 lines
Quoted 6 lines

A number of functions in the usb_gigaset module will return -EINVAL if CONFIG_GIGASET_UNDOCREQ is not set. Make these return -ENOSYS as it's more specific and it might make it easier to see (from userspace) why these functions actually fail.

Impact: some error return codes changed

ENODEV is what would be more appropriate.

Not at all. ENODEV means "no such device", which would be quite wrong. The device does exist and is in all probability working perfectly fine. It just doesn't implement that particular ioctl.

Quoted 2 lines

ENOSYS shuoldn't be returned from drivers, only from unimplemented system calls!

There's precedent for using ENOSYS for that case, for example in drivers/char/vt_ioctl.c. But I'm open for other suggestions.

Thanks,
Tilman

Signature · 4 lines
Tilman Schmidt                    E-Mail: [email protected]
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)
#4 lore raw

Re: [PATCH 1/2] gigaset: return -ENOSYS for unimplemented functions

Quoted 11 lines · On Sun, 08 Mar 2009 01:22:28 +0100 Tilman Schmidt <[email protected]> wrote:

Am 07.03.2009 23:26 schrieb Arjan van de Ven:

Quoted 10 lines
Quoted 7 lines · On Sat, 7 Mar 2009 23:10:57 +0100 (CET) Tilman Schmidt <[email protected]> wrote:

From: Paul Bolle <[email protected]>

A number of functions in the usb_gigaset module will return

(quoted diff) +0 -2
-EINVAL if CONFIG_GIGASET_UNDOCREQ is not set. Make these return
-ENOSYS as it's more specific and it might make it easier to see

(from userspace) why these functions actually fail.

Quoted 7 lines
Quoted 3 lines
Quoted 1 line

Impact: some error return codes changed

ENODEV is what would be more appropriate.

Not at all. ENODEV means "no such device", which would be quite wrong. The device does exist and is in all probability working perfectly fine. It just doesn't implement that particular ioctl.

then -ENOTTY is the right answer

Signature · 3 lines
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org
#5 lore raw

Re: [PATCH 1/2] gigaset: return -ENOSYS for unimplemented functions

Am 08.03.2009 01:35 schrieb Arjan van de Ven:

Quoted 4 lines
Quoted 1 line · On Sun, 08 Mar 2009 01:22:28 +0100 Tilman Schmidt <[email protected]> wrote:

Am 07.03.2009 23:26 schrieb Arjan van de Ven:

[...]

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

ENODEV is what would be more appropriate.

Not at all. ENODEV means "no such device", which would be quite wrong. The device does exist and is in all probability working perfectly fine. It just doesn't implement that particular ioctl.

then -ENOTTY is the right answer

Interesting, though slightly surprising proposition. "Not a typewriter" is certainly correct. :-)

"Not a tty device", however, which I take is the customary interpretation, much less clearly so. The device most certainly is a tty device. It just happens to know a few additional ioctl commands which may or may not be implemented, depending on the kernel config.

Not to question your authority, but I would really like a second opinion on that issue before I adopt your proposition, simply to minimize the risk of getting another objection from someone else who feels that ENOTTY is inappropriate in that situation.

Thanks,
Tilman

Signature · 4 lines
Tilman Schmidt                    E-Mail: [email protected]
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)
#6 lore raw

Re: [PATCH 1/2] gigaset: return -ENOSYS for unimplemented functions

Quoted 22 lines · On Sun, 08 Mar 2009 01:55:06 +0100 Tilman Schmidt <[email protected]> wrote:

Am 08.03.2009 01:35 schrieb Arjan van de Ven:

Quoted 4 lines
Quoted 1 line · On Sun, 08 Mar 2009 01:22:28 +0100 Tilman Schmidt <[email protected]> wrote:

Am 07.03.2009 23:26 schrieb Arjan van de Ven:

[...]

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

ENODEV is what would be more appropriate.

Not at all. ENODEV means "no such device", which would be quite wrong. The device does exist and is in all probability working perfectly fine. It just doesn't implement that particular ioctl.

then -ENOTTY is the right answer

Interesting, though slightly surprising proposition. "Not a typewriter" is certainly correct. :-)

"Not a tty device", however, which I take is the customary interpretation, much less clearly so. The device most certainly is a tty device. It just happens to know a few additional ioctl commands which may or may not be implemented, depending on the kernel config.

Quoted 4 lines

Not to question your authority, but I would really like a second opinion on that issue before I adopt your proposition, simply to minimize the risk of getting another objection from someone else who feels that ENOTTY is inappropriate in that situation.

from the ioctl manpage:

ERRORS
[snip]

ENOTTY The specified request does not apply to the kind of object that the descriptor d references.

Signature · 3 lines
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org
#7 lore raw

Re: [PATCH 1/2] gigaset: return -ENOSYS for unimplemented functions

Am 08.03.2009 02:38 schrieb Arjan van de Ven:

Quoted 7 lines

from the ioctl manpage:

ERRORS
[snip]

ENOTTY The specified request does not apply to the kind of object that the descriptor d references.

Ok, I'm convinced. I'll redo the patch with ENOTTY instead of ENOSYS.

Thanks,
Tilman

Signature · 4 lines
Tilman Schmidt                    E-Mail: [email protected]
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)
#8 lore raw

PATCH v2 1/2 gigaset: return -ENOTTY for unimplemented functions

In reply to Tilman Schmidt

From: Paul Bolle <[email protected]>

A number of functions in the usb_gigaset module will return -EINVAL if CONFIG_GIGASET_UNDOCREQ is not set. Make these return -ENOTTY as it's more specific and it might make it easier to see (from userspace) why these functions actually fail.

Impact: some error return codes changed

---
 drivers/isdn/gigaset/usb-gigaset.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
drivers/isdn/gigaset/usb-gigaset.c +4 -4
index fba61f6..d783851 100644
@@ -278,17 +278,17 @@ static int gigaset_set_line_ctrl(struct cardstate *cs, unsigned cflag)
 static int gigaset_set_modem_ctrl(struct cardstate *cs, unsigned old_state,
 				  unsigned new_state)
 {
-	return -EINVAL;
+	return -ENOTTY;
 }
 
 static int gigaset_set_line_ctrl(struct cardstate *cs, unsigned cflag)
 {
-	return -EINVAL;
+	return -ENOTTY;
 }
 
 static int gigaset_baud_rate(struct cardstate *cs, unsigned cflag)
 {
-	return -EINVAL;
+	return -ENOTTY;
 }
 #endif
 
@@ -577,7 +577,7 @@ static int gigaset_brkchars(struct cardstate *cs, const unsigned char buf[6])
 	return usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x19, 0x41,
 			       0, 0, &buf, 6, 2000);
 #else
-	return -EINVAL;
+	return -ENOTTY;
 #endif
 }
 
Signature · 1 line
1.5.4.7.gd8534-dirty
maintainer Applied
#9 lore raw

Re: [PATCH 1/2 v2] gigaset: return -ENOTTY for unimplemented functions

From: Tilman Schmidt <[email protected]>
Date: Sun, 8 Mar 2009 16:23:13 +0100 (CET)

Quoted 11 lines

From: Paul Bolle <[email protected]>

A number of functions in the usb_gigaset module will return -EINVAL if CONFIG_GIGASET_UNDOCREQ is not set. Make these return -ENOTTY as it's more specific and it might make it easier to see (from userspace) why these functions actually fail.

Impact: some error return codes changed

Applied.

#10 lore raw

PATCH 2/2 gigaset: Kconfig cleanup

In reply to Tilman Schmidt

Streamline dependencies and remove some obsolete or redundant comments in the Gigaset ISDN driver's Kconfig file. In particular, remove the strong warning against the GIGASET_UNDOCREQ option, as in seven years of existence, the code in question has never been reported to cause any harm.

Impact: Kconfig cleanup, no functional change

---
 drivers/isdn/gigaset/Kconfig |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)
drivers/isdn/gigaset/Kconfig +6 -8
index 0017e50..9ca889a 100644
@@ -1,5 +1,5 @@
 menuconfig ISDN_DRV_GIGASET
-	tristate "Siemens Gigaset support (isdn)"
+	tristate "Siemens Gigaset support"
 	select CRC_CCITT
 	select BITREVERSE
 	help
@@ -11,11 +11,11 @@ menuconfig ISDN_DRV_GIGASET
 	  one of the connection specific parts that follow.
 	  This will build a module called "gigaset".
 
-if ISDN_DRV_GIGASET!=n
+if ISDN_DRV_GIGASET
 
 config GIGASET_BASE
 	tristate "Gigaset base station support"
-	depends on ISDN_DRV_GIGASET && USB
+	depends on USB
 	help
 	  Say M here if you want to use the USB interface of the Gigaset
 	  base for connection to your system.
@@ -23,7 +23,7 @@ config GIGASET_BASE
 
 config GIGASET_M105
 	tristate "Gigaset M105 support"
-	depends on ISDN_DRV_GIGASET && USB
+	depends on USB
 	help
 	  Say M here if you want to connect to the Gigaset base via DECT
 	  using a Gigaset M105 (Sinus 45 Data 2) USB DECT device.
@@ -31,7 +31,6 @@ config GIGASET_M105
 
 config GIGASET_M101
 	tristate "Gigaset M101 support"
-	depends on ISDN_DRV_GIGASET
 	help
 	  Say M here if you want to connect to the Gigaset base via DECT
 	  using a Gigaset M101 (Sinus 45 Data 1) RS232 DECT device.
@@ -48,7 +47,6 @@ config GIGASET_UNDOCREQ
 	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. If you
-	  care about your device, say no.
+	  features like configuration mode of M105, say yes.
 
-endif # ISDN_DRV_GIGASET != n
+endif # ISDN_DRV_GIGASET
Signature · 1 line
1.5.4.7.gd8534-dirty
maintainer
#11 lore raw

Re: [PATCH 2/2] gigaset: Kconfig cleanup

From: Tilman Schmidt <[email protected]>
Date: Sat, 7 Mar 2009 23:11:02 +0100 (CET)

Quoted 9 lines

Streamline dependencies and remove some obsolete or redundant comments in the Gigaset ISDN driver's Kconfig file. In particular, remove the strong warning against the GIGASET_UNDOCREQ option, as in seven years of existence, the code in question has never been reported to cause any harm.

Impact: Kconfig cleanup, no functional change

Also applied, thanks.