[PATCH 0/2] gigaset: patches for 2.6.30

[PATCH 0/2] gigaset: patches for 2.6.30

[Tilman Schmidt]

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

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

[Tilman Schmidt]

From: Paul Bolle <pebolle@tiscali.nl>

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

Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 drivers/isdn/gigaset/usb-gigaset.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/isdn/gigaset/usb-gigaset.c b/drivers/isdn/gigaset/usb-gigaset.c
index fba61f6..2bd6d9d 100644
--- a/drivers/isdn/gigaset/usb-gigaset.c
+++ b/drivers/isdn/gigaset/usb-gigaset.c
@@ -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
 }
 
-- 
1.5.4.7.gd8534-dirty

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

[Arjan van de Ven]

On Sat,  7 Mar 2009 23:10:57 +0100 (CET)
Tilman Schmidt <tilman@imap.cc> wrote:

> From: Paul Bolle <pebolle@tiscali.nl>
> 
> 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!


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

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

[Tilman Schmidt]

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

Am 07.03.2009 23:26 schrieb Arjan van de Ven:
> On Sat,  7 Mar 2009 23:10:57 +0100 (CET)
> Tilman Schmidt <tilman@imap.cc> wrote:
> 
>> From: Paul Bolle <pebolle@tiscali.nl>
>>
>> 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.

> 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

-- 
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: 254 bytes --]

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

[Arjan van de Ven]

On Sun, 08 Mar 2009 01:22:28 +0100
Tilman Schmidt <tilman@imap.cc> wrote:

> Am 07.03.2009 23:26 schrieb Arjan van de Ven:
> > On Sat,  7 Mar 2009 23:10:57 +0100 (CET)
> > Tilman Schmidt <tilman@imap.cc> wrote:
> > 
> >> From: Paul Bolle <pebolle@tiscali.nl>
> >>
> >> 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.

then -ENOTTY is the right answer
-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

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

[Tilman Schmidt]

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

Am 08.03.2009 01:35 schrieb Arjan van de Ven:
> On Sun, 08 Mar 2009 01:22:28 +0100
> Tilman Schmidt <tilman@imap.cc> wrote:
> 
>> Am 07.03.2009 23:26 schrieb Arjan van de Ven:
[...]
>>> 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

-- 
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: 254 bytes --]

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

[Arjan van de Ven]

On Sun, 08 Mar 2009 01:55:06 +0100
Tilman Schmidt <tilman@imap.cc> wrote:

> Am 08.03.2009 01:35 schrieb Arjan van de Ven:
> > On Sun, 08 Mar 2009 01:22:28 +0100
> > Tilman Schmidt <tilman@imap.cc> wrote:
> > 
> >> Am 07.03.2009 23:26 schrieb Arjan van de Ven:
> [...]
> >>> 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.


from the ioctl manpage:

ERRORS
	[snip]

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



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

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

[Tilman Schmidt]

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

Am 08.03.2009 02:38 schrieb Arjan van de Ven:
> 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

-- 
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: 254 bytes --]

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

[Tilman Schmidt]

From: Paul Bolle <pebolle@tiscali.nl>

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

Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 drivers/isdn/gigaset/usb-gigaset.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/isdn/gigaset/usb-gigaset.c b/drivers/isdn/gigaset/usb-gigaset.c
index fba61f6..d783851 100644
--- a/drivers/isdn/gigaset/usb-gigaset.c
+++ b/drivers/isdn/gigaset/usb-gigaset.c
@@ -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
 }
 
-- 
1.5.4.7.gd8534-dirty

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

[David Miller]

From: Tilman Schmidt <tilman@imap.cc>
Date: Sun,  8 Mar 2009 16:23:13 +0100 (CET)

> From: Paul Bolle <pebolle@tiscali.nl>
> 
> 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
> 
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
> Signed-off-by: Tilman Schmidt <tilman@imap.cc>

Applied.

[PATCH 2/2] gigaset: Kconfig cleanup

[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

Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 drivers/isdn/gigaset/Kconfig |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/isdn/gigaset/Kconfig b/drivers/isdn/gigaset/Kconfig
index 0017e50..9ca889a 100644
--- a/drivers/isdn/gigaset/Kconfig
+++ b/drivers/isdn/gigaset/Kconfig
@@ -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
-- 
1.5.4.7.gd8534-dirty

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

[David Miller]

From: Tilman Schmidt <tilman@imap.cc>
Date: Sat,  7 Mar 2009 23:11:02 +0100 (CET)

> 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
> 
> Signed-off-by: Tilman Schmidt <tilman@imap.cc>

Also applied, thanks.