lkml.org 
[lkml]   [1997]   [Apr]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subjectpatch for uniform cdrom code
Date
Hi All,

I've been trying to submit this patch the old way during the past
months, without luck. I made three attempts, to two different
addressess. If there is anyone who can get this in the next
development release, i'd be much obliged.

The patch is rather harmless, libc-5.4.24+ already has support for the
changes.

---david

For 2.1, i've improved cdrom behaviour and removed some bugs in the
generic cdrom driver.

We needed some extra, removable-medium-specific errors. I coded them
in the errno.h's for i386, m68k and ppc. I don't know if the linux
cdrom drivers work on other architectures than i386.

With this patch, you can get decent error messages when you try to
mount an audio disc or have no disc in the drive, etc.

First the patch, then some advocation proza.


diff -u --recursive 2.1-linux/drivers/cdrom/cdrom.c 2.1-devel/drivers/cdrom/cdrom.c
--- 2.1-linux/drivers/cdrom/cdrom.c Sat Feb 1 14:34:34 1997
+++ 2.1-devel/drivers/cdrom/cdrom.c Thu Feb 27 23:14:27 1997
@@ -1,5 +1,5 @@
/* cdrom.c. Common ioctl and open routines for various Linux cdrom drivers. -*- linux-c -*-
- Copyright (c) 1996 David van Leeuwen.
+ Copyright (c) 1996, 1997 David A. van Leeuwen.

The routines in the file should provide an interface between
software accessing cdroms and the various drivers that implement
@@ -21,9 +21,29 @@
#include <linux/cdrom.h>
#include <linux/ucdrom.h>

+/* define CHECKTYPE if you want to check for audio/data cdrom. You
+ must use cd player programs that respect the O_NONBLOCK option for
+ opening the cdrom-device for ioctl-commanding only. (See
+ Documentation/cdrom/cdrom-standard.tex). Patches for a number of cd
+ players (CDplayer-2.0, cd-console-1.1, workbone-2.3, cdtool-1.0,
+ cdp-0.33, cdplayer-0.4, cdthing-1.4, lyn0.8a, playcd-1.0) can be
+ found in directory
+
+ ftp://ftp.gwdg.de/pub/linux/cdrom/drivers/cm206/
+
+ A runtime configuration program "checktype.c" to allow for cdrom
+ medium type checking can be found at the same location.
+
+ In order to be able to get neat system errors like "No medium
+ found" or "Wrong medium type" upon attempting to mount/play an
+ empty slot, mount an audio disc or play a data disc, you need to
+ have a new libc.5.so. */
+#undef CHECKTYPE
+
#define FM_WRITE 0x2 /* file mode write bit */

-#define VERSION "Generic CD-ROM driver, v 1.21 1996/11/08 03:24:49"
+#define VERSION "$Id: cdrom.c,v 1.7 1997/02/27 22:14:26 david Exp $"
+#define REVISION "$Revision: 1.7 $"

/* Not-exported routines. */
static int cdrom_open(struct inode *ip, struct file *fp);
@@ -86,6 +106,9 @@
cdi->next = cdromdevs[major];
cdromdevs[major] = cdi;
cdi->options = CDO_AUTO_CLOSE | CDO_USE_FFLAGS | CDO_LOCK;
+#ifdef CHECKTYPE
+ cdi->options |= CDO_CHECK_TYPE;
+#endif
cdi->mc_flags = 0;
return 0;
}
@@ -172,19 +195,19 @@
if (cdo->tray_move(cdi,0))
return -EIO;
} else
- return -ENXIO; /* can't close: too bad */
+ return -ENOMEDIUM; /* can't close: too bad */
ds = cdo->drive_status(cdi, CDSL_CURRENT);
if (ds == CDS_NO_DISC)
- return -ENXIO;
+ return -ENOMEDIUM;
}
}
if (cdo->disc_status != NULL) {
int ds = cdo->disc_status(cdi);
if (ds == CDS_NO_DISC)
- return -ENXIO;
+ return -ENOMEDIUM;
if (cdi->options & CDO_CHECK_TYPE &&
ds != CDS_DATA_1)
- return -ENODATA;
+ return -EMEDIUMTYPE;
}
/* all is well, we can open the device */
ret = cdo->open(cdi, 0); /* open for data */
@@ -201,11 +224,15 @@
kdev_t dev = ip->i_rdev;
struct cdrom_device_info *cdi = cdrom_find_device (dev);
struct cdrom_device_ops *cdo;
+ int opened_for_data;

if (cdi == NULL)
return;
+ opened_for_data = !(cdi->options & CDO_USE_FFLAGS) ||
+ !(fp && fp->f_flags & O_NONBLOCK);
cdo = cdi->ops;
if (cdi->use_count == 1 && /* last process that closes dev*/
+ opened_for_data &&
cdi->options & CDO_LOCK &&
cdo->capability & ~cdi->mask & CDC_LOCK)
cdo->lock_door(cdi, 0);
@@ -214,7 +241,8 @@
if (cdi->use_count == 0) { /* last process that closes dev*/
sync_dev(dev);
invalidate_buffers(dev);
- if (cdi->options & CDO_AUTO_EJECT &&
+ if (opened_for_data &&
+ cdi->options & CDO_AUTO_EJECT &&
cdo->capability & ~cdi->mask & CDC_OPEN_TRAY)
cdo->tray_move(cdi, 1);
}
@@ -317,6 +345,10 @@
* memory-verification is performed for these ioctls.
*/
static
+int check_for_audio_disc(struct cdrom_device_info * cdi,
+ struct cdrom_device_ops * cdo);
+
+static
int cdrom_ioctl(struct inode *ip, struct file *fp,
unsigned int cmd, unsigned long arg)
{
@@ -373,17 +405,15 @@

case CDROM_MEDIA_CHANGED:
if (cdo->capability & ~cdi->mask & CDC_MEDIA_CHANGED) {
- if (arg == CDSL_CURRENT)
+ if (!(cdo->capability & ~cdi->mask & CDC_SELECT_DISC)
+ || arg == CDSL_CURRENT)
+ /* cannot select disc or select current disc */
return media_changed(cdi, 1);
- else if ((int)arg < cdi->capacity &&
- cdo->capability & ~cdi->mask
- &CDC_SELECT_DISC)
+ if ((unsigned int)arg < cdi->capacity)
return cdo->media_changed (cdi, arg);
- else
- return -EINVAL;
- }
- else
- return -EINVAL;
+ return -EINVAL;
+ } else
+ return -EINVAL;

case CDROM_SET_OPTIONS:
cdi->options |= (int) arg;
@@ -429,8 +459,8 @@
if ((arg == CDSL_CURRENT) || (arg == CDSL_NONE))
return cdo->drive_status(cdi, arg);
if (cdo->drive_status == NULL ||
- ! (cdo->capability & ~cdi->mask & CDC_SELECT_DISC
- && (int)arg < cdi->capacity))
+ ((cdo->capability & ~cdi->mask & CDC_SELECT_DISC)
+ && (int)arg >= cdi->capacity))
return -EINVAL;
else
return cdo->drive_status(cdi, arg);
@@ -466,7 +496,10 @@
/* Now all the audio-ioctls follow, they are all routed through the
same call audio_ioctl(). */

- if (cdo->capability & CDC_PLAY_AUDIO)
+#define CHECKAUDIO if ((ret=check_for_audio_disc(cdi, cdo))) return ret
+
+ if (cdo->capability & CDC_PLAY_AUDIO) {
+ int ret;
switch (cmd) {
case CDROMSUBCHNL:
{
@@ -489,7 +522,8 @@
case CDROMREADTOCHDR: {
struct cdrom_tochdr header;
GETARG(struct cdrom_tochdr, header);
- if (!cdo->audio_ioctl(cdi, cmd, &header)) {
+ CHECKAUDIO;
+ if (!cdo->audio_ioctl(cdi, cmd, &header)) {
PUTARG(struct cdrom_tochdr, header);
return 0;
} else
@@ -513,11 +547,13 @@
case CDROMPLAYMSF: {
struct cdrom_msf msf;
GETARG(struct cdrom_msf, msf);
+ CHECKAUDIO;
return cdo->audio_ioctl(cdi, cmd, &msf);
}
case CDROMPLAYTRKIND: {
struct cdrom_ti track_index;
GETARG(struct cdrom_ti, track_index);
+ CHECKAUDIO;
return cdo->audio_ioctl(cdi, cmd, &track_index);
}
case CDROMVOLCTRL: {
@@ -534,21 +570,57 @@
return -EINVAL;
}
case CDROMSTART:
+ CHECKAUDIO;
case CDROMSTOP:
case CDROMPAUSE:
case CDROMRESUME:
return cdo->audio_ioctl(cdi, cmd, NULL);
} /* switch */
-
+ }
if (cdo->dev_ioctl != NULL) /* device specific ioctls? */
return cdo->dev_ioctl(cdi, cmd, arg);
return -EINVAL;
}

+/* This code is similar to that in open_for_data. The routine is called
+ in case a audio play operation is requested. It doesn't make much sense
+ do do this on a data disc.
+ */
+int check_for_audio_disc(struct cdrom_device_info * cdi,
+ struct cdrom_device_ops * cdo)
+{
+ if (!(cdi->options & CDO_CHECK_TYPE))
+ return 0;
+ if (cdo->drive_status != NULL) {
+ int ds = cdo->drive_status(cdi, CDSL_CURRENT);
+ if (ds == CDS_TRAY_OPEN) {
+ /* can/may i close it? */
+ if (cdo->capability & ~cdi->mask & CDC_CLOSE_TRAY &&
+ cdi->options & CDO_AUTO_CLOSE) {
+ if (cdo->tray_move(cdi,0))
+ return -EIO;
+ } else
+ return -ENOMEDIUM; /* can't close: too bad */
+ ds = cdo->drive_status(cdi, CDSL_CURRENT);
+ if (ds == CDS_NO_DISC)
+ return -ENOMEDIUM;
+ }
+ if (cdo->disc_status != NULL) {
+ ds = cdo->disc_status(cdi);
+ if (ds == CDS_NO_DISC)
+ return -ENOMEDIUM;
+ if (ds != CDS_AUDIO)
+ return -EMEDIUMTYPE;
+ }
+ }
+ return 0;
+}
+
+
#ifdef MODULE
int init_module(void)
{
- printk(KERN_INFO "Module inserted: " VERSION "\n");
+ printk(KERN_INFO "Module cdrom: Generic CDROM driver " REVISION "\n");
return 0;
}

@@ -563,6 +635,6 @@
/*
* Local variables:
* comment-column: 40
- * compile-command: "gcc -DMODULE -D__KERNEL__ -I. -I/usr/src/linux-obj/include -Wall -Wstrict-prototypes -O2 -m486 -c cdrom.c -o cdrom.o"
+ * compile-command: "gcc -D__KERNEL__ -I/usr/src/linux/include -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer -pipe -fno-strength-reduce -m486 -DCPU=486 -DMODULE -DMODVERSIONS -include /usr/src/linux/include/linux/modversions.h -c -o cdrom.o cdrom.c"
* End:
*/
diff -u --recursive 2.1-linux/include/asm-i386/errno.h 2.1-devel/include/asm-i386/errno.h
--- 2.1-linux/include/asm-i386/errno.h Thu Jul 4 06:28:57 1996
+++ 2.1-devel/include/asm-i386/errno.h Mon Feb 17 20:55:14 1997
@@ -126,4 +126,7 @@
#define EREMOTEIO 121 /* Remote I/O error */
#define EDQUOT 122 /* Quota exceeded */

+#define ENOMEDIUM 123 /* No medium found */
+#define EMEDIUMTYPE 124 /* Wrong medium type */
+
#endif
diff -u --recursive 2.1-linux/include/asm-m68k/errno.h 2.1-devel/include/asm-m68k/errno.h
--- 2.1-linux/include/asm-m68k/errno.h Wed Sep 25 09:47:41 1996
+++ 2.1-devel/include/asm-m68k/errno.h Mon Feb 17 20:54:56 1997
@@ -126,4 +126,7 @@
#define EREMOTEIO 121 /* Remote I/O error */
#define EDQUOT 122 /* Quota exceeded */

+#define ENOMEDIUM 123 /* No medium found */
+#define EMEDIUMTYPE 124 /* Wrong medium type */
+
#endif /* _M68K_ERRNO_H */
diff -u --recursive 2.1-linux/include/asm-ppc/errno.h 2.1-devel/include/asm-ppc/errno.h
--- 2.1-linux/include/asm-ppc/errno.h Mon May 27 11:00:59 1996
+++ 2.1-devel/include/asm-ppc/errno.h Mon Feb 17 20:55:12 1997
@@ -124,6 +124,9 @@
#define EREMOTEIO 121 /* Remote I/O error */
#define EDQUOT 122 /* Quota exceeded */

+#define ENOMEDIUM 123 /* No medium found */
+#define EMEDIUMTYPE 124 /* Wrong medium type */
+
/* Should never be seen by user programs */
#define ERESTARTSYS 512
#define ERESTARTNOINTR 513
--

This piece of text advocates the use of new error messages for
removable media devices, such as CDROM, in Linux 2.1 and Libc.

I am the maintainer of "linux/drivers/cdrom.c", and i propose to add a
few new error messages to "linux/include/asm/errno.h" and the
accompanying array sys_errlist[] in libc. These are (for asm-i386)

#define ENOMEDIUM 123 /* No medium found */
#define EMEDIUMTYPE 124 /* Wrong medium type */
The point is the following. CDROM drives are nowadays more modern than
the old IBM-PC floppy disc drive. Apparently, for this drive it was
not possible to detect if a floppy is inside the drive or not,
resulting weird error messages if one tried to access the drive.
For CDROM drives detection of a disc, and the type of disc, is not a
problem at all. However, error messages connected to CDROM drives
are still weird.

If we take the most modern version of Linux, for instance, we get an
error `No such device' if one tries to mount a cdrom when there is
nothing is the drive. We get the same error if either
- There is no physical CDROM drive,
- the correct device driver isn't loaded,
and maybe also in other circumstances.

In "cdrom.c" i took great care to separate different sources of error,
but i found that there was no error defined in "errno.h" that is
really applicable to CDROMs. As i didn't want to be intrusive in other
parts of the kernel, i defined

#define ENOMEDIUM EAGAIN

so that during read operations, one would at least not be mislead by
the `No such device' error in case no CDROM is loaded. To my surprise,
some people replaced all `return -ENOMEDIUM' statements in cdrom.c
(naturally without notifying me) by `return -ENODEV', instead of
changing the definition of ENOMEDIUM above. I believe that the
argument was that `this error is returned by the floppy driver as
well.' Please. We live in 1997. Does Linux has to suffer for all
design mistakes that the hardware manufacturers made over 15 years
ago?

Currently we _can_ detect if a CDROM is in the drive, and we _can_
find out its type (audio/data/...) without trying to bluntly read data
or play tracks, and wait for what is going to happen. The drivers have
even implemented calls to detect such errors. Looking at the list of
errors defined in "asm/errno.h", it appears that removable media are
ignored. Well, they are an important part of computer systems, and
they _are_ different from hard discs (not removable) and tapes (no
random access).

So Linux kernel developers and Linux libc developers, _please_ apply
the patches i've proposed. It would make the behaviour of CDROM
devices a lot more sensible.

David van Leeuwen, <david@tm.tno.nl>






\
 
 \ /
  Last update: 2005-03-22 13:39    [W:0.046 / U:0.628 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site