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
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
Signed-off-by: Paul Bolle <[email protected]>
Signed-off-by: Tilman Schmidt <[email protected]>
---
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
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 <[email protected]>
---
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
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!
--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org
Am 07.03.2009 23:26 schrieb Arjan van de Ven:
> 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.
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: [email protected]
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Unge?ffnet mindestens haltbar bis: (siehe R?ckseite)
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:
> > 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.
>
> 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
Am 08.03.2009 01:35 schrieb Arjan van de Ven:
> 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:
[...]
>>> 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: [email protected]
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Unge?ffnet mindestens haltbar bis: (siehe R?ckseite)
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:
> > 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:
> [...]
> >>> 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
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: [email protected]
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Unge?ffnet mindestens haltbar bis: (siehe R?ckseite)
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
Signed-off-by: Paul Bolle <[email protected]>
Signed-off-by: Tilman Schmidt <[email protected]>
---
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
From: Tilman Schmidt <[email protected]>
Date: Sun, 8 Mar 2009 16:23:13 +0100 (CET)
> 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
>
> Signed-off-by: Paul Bolle <[email protected]>
> Signed-off-by: Tilman Schmidt <[email protected]>
Applied.
From: Tilman Schmidt <[email protected]>
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 <[email protected]>
Also applied, thanks.