LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Maarten Bressers <mbres@gentoo.org>
To: James.Bottomley@SteelEye.com
Cc: linux-kernel@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
Cc: axboe@kernel.dk
Cc: tasio@hierro.tasio.net
Cc: dsd@gentoo.org
Subject: Re: [PATCH RESEND] SCSI not showing tray status correctly
Date: Sat, 27 Oct 2007 22:58:30 +0000 (UTC)	[thread overview]
Message-ID: <20071027225830.585C8657DB@smtp.gentoo.org> (raw)

> This patch is too simplistic.  ide-cd.c:ide_cdrom_drive_status() looks
> to be a reasonable implementation.  However, the worry is that
> GET_EVENT_NOTIFICATION is a MMC command; devices not conforming to MMC
> won't support it.  In theory, they should just return ILLEGAL_REQUEST,
> but USB devices have been known to crash when given commands they don't
> understand.  How widely tested has this been (if it's been in Gentoo
> since 2004, then it's probably widely tested enough)?
>
> James
>
This patch hasn't been tested at all, I'm sorry if I gave you that
impression, it isn't in Gentoo's patches, it's just been brought to our
attention in the bug I linked too.

I created another patch, based on your recommendations, and with a lot of
help from Daniel Drake (dsd@gentoo.org), that's included below. Some
changes are made to test_unit_ready() to be able to pass the sense data
to sr_drive_status(). Currently, sr_do_ioctl() swallows this data and
makes its own interpretations of sense codes, which isn't what we want
here.

Is this what you had in mind? Do you think the possible problems with
USB drives that you mentioned will prevent this patch from going in? 
The patch has only been compile tested right now, we can do some real
testing later, for now I'd just like to get your feedback on it.

Maarten

---

--- a/drivers/scsi/sr_ioctl.c	2007-10-26 22:40:41.000000000 +0200
+++ b/drivers/scsi/sr_ioctl.c	2007-10-27 23:56:16.000000000 +0200
@@ -275,16 +275,21 @@ int sr_do_ioctl(Scsi_CD *cd, struct pack
 /* ---------------------------------------------------------------------- */
 /* interface to cdrom.c                                                   */
 
-static int test_unit_ready(Scsi_CD *cd)
+static int test_unit_ready(Scsi_CD *cd, struct request_sense *sense)
 {
-	struct packet_command cgc;
+	struct scsi_device *SDev = cd->device;
+	unsigned char cmd[CDROM_PACKET_SIZE] = { GPCMD_TEST_UNIT_READY };
+	int result;
 
-	memset(&cgc, 0, sizeof(struct packet_command));
-	cgc.cmd[0] = GPCMD_TEST_UNIT_READY;
-	cgc.quiet = 1;
-	cgc.data_direction = DMA_NONE;
-	cgc.timeout = IOCTL_TIMEOUT;
-	return sr_do_ioctl(cd, &cgc);
+	if (!scsi_block_when_processing_errors(SDev))
+		return -ENODEV;
+
+	memset(sense, 0, sizeof(*sense));
+	result = scsi_execute(SDev, cmd, DMA_NONE,
+			      NULL, 0, (char *)sense,
+			      IOCTL_TIMEOUT, IOCTL_RETRIES, 0);
+
+	return driver_byte(result);
 }
 
 int sr_tray_move(struct cdrom_device_info *cdi, int pos)
@@ -310,14 +315,41 @@ int sr_lock_door(struct cdrom_device_inf
 
 int sr_drive_status(struct cdrom_device_info *cdi, int slot)
 {
+	struct request_sense sense;
+	struct media_event_desc med;
+
 	if (CDSL_CURRENT != slot) {
 		/* we have no changer support */
 		return -EINVAL;
 	}
-	if (0 == test_unit_ready(cdi->handle))
+	if ((0 == test_unit_ready(cdi->handle, &sense)) || 
+	    sense.sense_key == UNIT_ATTENTION)
 		return CDS_DISC_OK;
 
-	return CDS_TRAY_OPEN;
+	if (!cdrom_get_media_event(cdi, &med)) {
+		if (med.media_present)
+			return CDS_DISC_OK;
+		else if (med.door_open)
+			return CDS_TRAY_OPEN;
+		else
+			return CDS_NO_DISC;
+	}
+
+	if (sense.sense_key == NOT_READY && sense.asc == 0x04 && sense.ascq == 0x04)
+		return CDS_DISC_OK;
+
+	/*
+	 * If not using Mt Fuji extended media tray reports,
+	 * just return TRAY_OPEN since ATAPI doesn't provide
+	 * any other way to detect this...
+	 */
+	if (sense.sense_key == NOT_READY) {
+		if (sense.asc == 0x3a && sense.ascq == 1)
+			return CDS_NO_DISC;
+		else
+			return CDS_TRAY_OPEN;
+	}
+	return CDS_DRIVE_NOT_READY;
 }
 
 int sr_disk_status(struct cdrom_device_info *cdi)

             reply	other threads:[~2007-10-27 22:58 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-10-27 22:58 Maarten Bressers [this message]
2007-10-31  3:32 ` James Bottomley
2008-01-05 16:39   ` [PATCH] sr: update to follow " James Bottomley
2008-02-06 17:09     ` Daniel Drake
2008-02-06 19:01       ` James Bottomley
2008-02-06 20:54         ` Daniel Drake
  -- strict thread matches above, loose matches on Subject: below --
2007-10-25 22:30 [PATCH RESEND] SCSI not showing " Maarten Bressers
2007-10-25 23:24 ` James Bottomley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20071027225830.585C8657DB@smtp.gentoo.org \
    --to=mbres@gentoo.org \
    --cc=James.Bottomley@SteelEye.com \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: [PATCH RESEND] SCSI not showing tray status correctly' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).