Re: Problems with Toshiba SD-W2002 DVD-RAM drive (IDE)

Jens Axboe
2001-04-18T12:40:34Z

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

On Wed, Apr 18 2001, Jens Axboe wrote:
> On Tue, Apr 17 2001, Stefan Jaschke wrote:
> > Judging from the thread started Jan 1, 2001, by Andre Hedrick,
> > I thought IDE DVD-RAM just works out of the box and got a
> > Toshiba SD-W2002.
> >
> > Problem: /dev/hdc cannot be read or written to when the drive contains
> > DVD-RAM media. The behavior is the same for the stock 2.4.3 kernel
> > and the SuSE-2.4.0 kernel. Strangely enough, the disk can be read,
> > but not written to, with the 2.2.18 kernel.
>
> It should work, note that I recently spotted some quite severe bugs in
> the pio write handling for ATAPI which I've almost fixed here now. It
> seems you drive is in DMA mode though, so it shouldn't be affecting you.

Attached patch for 2.4.4-pre4 which fixes all known DVD-RAM ATAPI bugs.
Both pio and dma mode work fine here, using ext2, on a 9.4gb HITACHI
DVD-RAM GF-2000 drive.

--
Jens Axboe

[-- Attachment #2: cd-244p4-1 --]
[-- Type: text/plain, Size: 14843 bytes --]

diff -ur --exclude-from /home/axboe/exclude /opt/kernel/linux-2.4.4-pre4/drivers/cdrom/cdrom.c linux/drivers/cdrom/cdrom.c
--- /opt/kernel/linux-2.4.4-pre4/drivers/cdrom/cdrom.c Thu Mar 29 21:56:07 2001
+++ linux/drivers/cdrom/cdrom.c Wed Apr 18 13:27:36 2001
@@ -279,6 +279,9 @@
static int lockdoor = 1;
/* will we ever get to use this... sigh. */
static int check_media_type;
+static unsigned long *cdrom_numbers;
+static DECLARE_MUTEX(cdrom_sem);
+
MODULE_PARM(debug, "i");
MODULE_PARM(autoclose, "i");
MODULE_PARM(autoeject, "i");
@@ -340,6 +343,38 @@
check_media_change: cdrom_media_changed,
};

+/*
+ * get or clear a new cdrom number, run under cdrom_sem
+ */
+static int cdrom_get_entry(void)
+{
+ int i, nr, foo;
+
+ nr = 0;
+ foo = -1;
+ for (i = 0; i < CDROM_MAX_CDROMS / (sizeof(unsigned long) * 8); i++) {
+ if (cdrom_numbers[i] == ~0UL) {
+ nr += sizeof(unsigned long) * 8;
+ continue;
+ }
+ foo = ffz(cdrom_numbers[i]);
+ set_bit(foo, &cdrom_numbers[i]);
+ nr += foo;
+ break;
+ }
+
+ return foo == -1 ? foo : nr;
+}
+
+static void cdrom_clear_entry(struct cdrom_device_info *cdi)
+{
+ int bit_nr = cdi->nr & ~(sizeof(unsigned long) * 8);
+ int cd_index = cdi->nr / (sizeof(unsigned long) * 8);
+
+ clear_bit(bit_nr, &cdrom_numbers[cd_index]);
+}
+
+
/* This macro makes sure we don't have to check on cdrom_device_ops
* existence in the run-time routines below. Change_capability is a
* hack to have the capability flags defined const, while we can still
@@ -354,7 +389,6 @@
struct cdrom_device_ops *cdo = cdi->ops;
int *change_capability = (int *)&cdo->capability; /* hack */
char vname[16];
- static unsigned int cdrom_counter;

cdinfo(CD_OPEN, "entering register_cdrom\n");

@@ -395,7 +429,17 @@

if (!devfs_handle)
devfs_handle = devfs_mk_dir (NULL, "cdroms", NULL);
- sprintf (vname, "cdrom%u", cdrom_counter++);
+
+ /*
+ * get new cdrom number
+ */
+ down(&cdrom_sem);
+ cdi->nr = cdrom_get_entry();
+ up(&cdrom_sem);
+ if (cdi->nr == -1)
+ return -ENOMEM;
+
+ sprintf(vname, "cdrom%u", cdi->nr);
if (cdi->de) {
int pos;
devfs_handle_t slave;
@@ -418,9 +462,13 @@
S_IFBLK | S_IRUGO | S_IWUGO,
&cdrom_fops, NULL);
}
- cdinfo(CD_REG_UNREG, "drive \"/dev/%s\" registered\n", cdi->name);
+
+ down(&cdrom_sem);
cdi->next = topCdromPtr;
topCdromPtr = cdi;
+ up(&cdrom_sem);
+
+ cdinfo(CD_REG_UNREG, "drive \"/dev/%s\" registered\n", cdi->name);
return 0;
}
#undef ENSURE
@@ -429,12 +477,14 @@
{
struct cdrom_device_info *cdi, *prev;
int major = MAJOR(unreg->dev);
+ int bit_nr, cd_index;

cdinfo(CD_OPEN, "entering unregister_cdrom\n");

if (major < 0 || major >= MAX_BLKDEV)
return -1;

+ down(&cdrom_sem);
prev = NULL;
cdi = topCdromPtr;
while (cdi != NULL && cdi->dev != unreg->dev) {
@@ -442,14 +492,20 @@
cdi = cdi->next;
}

- if (cdi == NULL)
+ if (cdi == NULL) {
+ up(&cdrom_sem);
return -2;
+ }
+
+ cdrom_clear_entry(cdi);
+
if (prev)
prev->next = cdi->next;
else
topCdromPtr = cdi->next;
+ up(&cdrom_sem);
cdi->ops->n_minors--;
- devfs_unregister (cdi->de);
+ devfs_unregister(cdi->de);
cdinfo(CD_REG_UNREG, "drive \"/dev/%s\" unregistered\n", cdi->name);
return 0;
}
@@ -458,10 +514,14 @@
{
struct cdrom_device_info *cdi;

+ down(&cdrom_sem);
+
cdi = topCdromPtr;
while (cdi != NULL && cdi->dev != dev)
cdi = cdi->next;

+ up(&cdrom_sem);
+
return cdi;
}

@@ -2418,6 +2478,8 @@
}

pos = sprintf(info, "CD-ROM information, " VERSION "\n");
+
+ down(&cdrom_sem);

pos += sprintf(info+pos, "\ndrive name:\t");
for (cdi=topCdromPtr;cdi!=NULL;cdi=cdi->next)
@@ -2487,6 +2549,8 @@
for (cdi=topCdromPtr;cdi!=NULL;cdi=cdi->next)
pos += sprintf(info+pos, "\t%d", CDROM_CAN(CDC_DVD_RAM) != 0);

+ up(&cdrom_sem);
+
strcpy(info+pos,"\n\n");

return proc_dostring(ctl, write, filp, buffer, lenp);
@@ -2633,6 +2697,10 @@

static int __init cdrom_init(void)
{
+ int n_entries = CDROM_MAX_CDROMS / (sizeof(unsigned long) * 8);
+
+ cdrom_numbers = kmalloc(n_entries * sizeof(unsigned long), GFP_KERNEL);
+
#ifdef CONFIG_SYSCTL
cdrom_sysctl_register();
#endif
@@ -2643,6 +2711,7 @@
static void __exit cdrom_exit(void)
{
printk(KERN_INFO "Uniform CD-ROM driver unloaded\n");
+ kfree(cdrom_numbers);
#ifdef CONFIG_SYSCTL
cdrom_sysctl_unregister();
#endif
diff -ur --exclude-from /home/axboe/exclude /opt/kernel/linux-2.4.4-pre4/drivers/ide/ide-cd.c linux/drivers/ide/ide-cd.c
--- /opt/kernel/linux-2.4.4-pre4/drivers/ide/ide-cd.c Fri Feb 9 20:30:23 2001
+++ linux/drivers/ide/ide-cd.c Wed Apr 18 14:28:30 2001
@@ -977,8 +977,7 @@

/* If we've filled the present buffer but there's another
chained buffer after it, move on. */
- if (rq->current_nr_sectors == 0 &&
- rq->nr_sectors > 0)
+ if (rq->current_nr_sectors == 0 && rq->nr_sectors)
cdrom_end_request (1, drive);

/* If the buffers are full, cache the rest of the data in our
@@ -1192,6 +1191,55 @@
return cdrom_start_packet_command (drive, 0, cdrom_start_seek_continuation);
}

+static inline int cdrom_merge_requests(struct request *rq, struct request *nxt)
+{
+ int ret = 1;
+
+ /*
+ * partitions not really working, but better check anyway...
+ */
+ if (rq->cmd == nxt->cmd && rq->rq_dev == nxt->rq_dev) {
+ rq->nr_sectors += nxt->nr_sectors;
+ rq->hard_nr_sectors += nxt->nr_sectors;
+ rq->bhtail->b_reqnext = nxt->bh;
+ rq->bhtail = nxt->bhtail;
+ list_del(&nxt->queue);
+ blkdev_release_request(nxt);
+ ret = 0;
+ }
+
+ return ret;
+}
+
+/*
+ * the current request will always be the first one on the list
+ */
+static void cdrom_attempt_remerge(ide_drive_t *drive, struct request *rq)
+{
+ struct list_head *entry;
+ struct request *nxt;
+ unsigned long flags;
+
+ spin_lock_irqsave(&io_request_lock, flags);
+
+ while (1) {
+ entry = rq->queue.next;
+ if (entry == &drive->queue.queue_head)
+ break;
+
+ nxt = blkdev_entry_to_request(entry);
+ if (rq->sector + rq->nr_sectors != nxt->sector)
+ break;
+ else if (rq->nr_sectors + nxt->nr_sectors > SECTORS_MAX)
+ break;
+
+ if (cdrom_merge_requests(rq, nxt))
+ break;
+ }
+
+ spin_unlock_irqrestore(&io_request_lock, flags);
+}
+
/* Fix up a possibly partially-processed request so that we can
start it over entirely, or even put it back on the request queue. */
static void restore_request (struct request *rq)
@@ -1203,6 +1251,8 @@
rq->sector -= n;
}
rq->current_nr_sectors = rq->bh->b_size >> SECTOR_BITS;
+ rq->hard_nr_sectors = rq->nr_sectors;
+ rq->hard_sector = rq->sector;
}

/*
@@ -1216,20 +1266,22 @@

/* If the request is relative to a partition, fix it up to refer to the
absolute address. */
- if ((minor & PARTN_MASK) != 0) {
+ if (minor & PARTN_MASK) {
rq->sector = block;
minor &= ~PARTN_MASK;
- rq->rq_dev = MKDEV (MAJOR(rq->rq_dev), minor);
+ rq->rq_dev = MKDEV(MAJOR(rq->rq_dev), minor);
}

/* We may be retrying this request after an error. Fix up
any weirdness which might be present in the request packet. */
- restore_request (rq);
+ restore_request(rq);

/* Satisfy whatever we can of this request from our cached sector. */
if (cdrom_read_from_buffer(drive))
return ide_stopped;

+ cdrom_attempt_remerge(drive, rq);
+
/* Clear the local sector buffer. */
info->nsectors_buffered = 0;

@@ -1477,7 +1529,7 @@

static ide_startstop_t cdrom_write_intr(ide_drive_t *drive)
{
- int stat, ireason, len, sectors_to_transfer;
+ int stat, ireason, len, sectors_to_transfer, uptodate;
struct cdrom_info *info = drive->driver_data;
int i, dma_error = 0, dma = info->dma;
ide_startstop_t startstop;
@@ -1498,6 +1550,9 @@
return startstop;
}

+ /*
+ * using dma, transfer is complete now
+ */
if (dma) {
if (dma_error)
return ide_error(drive, "dma error", stat);
@@ -1519,12 +1574,13 @@
/* If we're not done writing, complain.
* Otherwise, complete the command normally.
*/
+ uptodate = 1;
if (rq->current_nr_sectors > 0) {
printk("%s: write_intr: data underrun (%ld blocks)\n",
- drive->name, rq->current_nr_sectors);
- cdrom_end_request(0, drive);
- } else
- cdrom_end_request(1, drive);
+ drive->name, rq->current_nr_sectors);
+ uptodate = 0;
+ }
+ cdrom_end_request(uptodate, drive);
return ide_stopped;
}

@@ -1533,26 +1589,42 @@
if (cdrom_write_check_ireason(drive, len, ireason))
return ide_stopped;

- /* The number of sectors we need to read from the drive. */
sectors_to_transfer = len / SECTOR_SIZE;

- /* Now loop while we still have data to read from the drive. DMA
- * transfers will already have been complete
+ /*
+ * now loop and write out the data
*/
while (sectors_to_transfer > 0) {
- /* If we've filled the present buffer but there's another
- chained buffer after it, move on. */
- if (rq->current_nr_sectors == 0 && rq->nr_sectors > 0)
- cdrom_end_request(1, drive);
+ int this_transfer;
+
+ if (!rq->current_nr_sectors) {
+ printk("ide-cd: write_intr: oops\n");
+ break;
+ }
+
+ /*
+ * Figure out how many sectors we can transfer
+ */
+ this_transfer = MIN(sectors_to_transfer,rq->current_nr_sectors);
+
+ while (this_transfer > 0) {
+ atapi_output_bytes(drive, rq->buffer, SECTOR_SIZE);
+ rq->buffer += SECTOR_SIZE;
+ --rq->nr_sectors;
+ --rq->current_nr_sectors;
+ ++rq->sector;
+ --this_transfer;
+ --sectors_to_transfer;
+ }

- atapi_output_bytes(drive, rq->buffer, rq->current_nr_sectors);
- rq->nr_sectors -= rq->current_nr_sectors;
- rq->current_nr_sectors = 0;
- rq->sector += rq->current_nr_sectors;
- sectors_to_transfer -= rq->current_nr_sectors;
+ /*
+ * current buffer complete, move on
+ */
+ if (rq->current_nr_sectors == 0 && rq->nr_sectors)
+ cdrom_end_request (1, drive);
}

- /* arm handler */
+ /* re-arm handler */
ide_set_handler(drive, &cdrom_write_intr, 5 * WAIT_CMD, NULL);
return ide_started;
}
@@ -1583,10 +1655,26 @@
return cdrom_transfer_packet_command(drive, &pc, cdrom_write_intr);
}

-static ide_startstop_t cdrom_start_write(ide_drive_t *drive)
+static ide_startstop_t cdrom_start_write(ide_drive_t *drive, struct request *rq)
{
struct cdrom_info *info = drive->driver_data;

+ /*
+ * writes *must* be 2kB frame aligned
+ */
+ if ((rq->nr_sectors & 3) || (rq->sector & 3)) {
+ cdrom_end_request(0, drive);
+ return ide_stopped;
+ }
+
+ /*
+ * for dvd-ram and such media, it's a really big deal to get
+ * big writes all the time. so scour the queue and attempt to
+ * remerge requests, often the plugging will not have had time
+ * to do this properly
+ */
+ cdrom_attempt_remerge(drive, rq);
+
info->nsectors_buffered = 0;

/* use dma, if possible. we don't need to check more, since we
@@ -1629,7 +1717,7 @@
if (rq->cmd == READ)
action = cdrom_start_read(drive, block);
else
- action = cdrom_start_write(drive);
+ action = cdrom_start_write(drive, rq);
}
info->last_block = block;
return action;
@@ -1832,6 +1920,7 @@

pc.buffer = buf;
pc.buflen = buflen;
+ pc.quiet = 1;
pc.c[0] = GPCMD_READ_TOC_PMA_ATIP;
pc.c[6] = trackno;
pc.c[7] = (buflen >> 8);
@@ -2826,7 +2915,12 @@
drive->part[0].nr_sects = toc->capacity * SECTORS_PER_FRAME;
HWIF(drive)->gd->sizes[minor] = toc->capacity * BLOCKS_PER_FRAME;

+ /*
+ * reset block size, ide_revalidate_disk incorrectly sets it to
+ * 1024 even for CDROM's
+ */
blk_size[HWIF(drive)->major] = HWIF(drive)->gd->sizes;
+ set_blocksize(MKDEV(HWIF(drive)->major, minor), CD_FRAMESIZE);
}

static
diff -ur --exclude-from /home/axboe/exclude /opt/kernel/linux-2.4.4-pre4/drivers/ide/ide-cd.h linux/drivers/ide/ide-cd.h
--- /opt/kernel/linux-2.4.4-pre4/drivers/ide/ide-cd.h Tue Mar 27 01:49:15 2001
+++ linux/drivers/ide/ide-cd.h Wed Apr 18 13:09:13 2001
@@ -37,11 +37,12 @@

/************************************************************************/

-#define SECTOR_SIZE 512
#define SECTOR_BITS 9
-#define SECTORS_PER_FRAME (CD_FRAMESIZE / SECTOR_SIZE)
+#define SECTOR_SIZE (1 << SECTOR_BITS)
+#define SECTORS_PER_FRAME (CD_FRAMESIZE >> SECTOR_BITS)
#define SECTOR_BUFFER_SIZE (CD_FRAMESIZE * 32)
-#define SECTORS_BUFFER (SECTOR_BUFFER_SIZE / SECTOR_SIZE)
+#define SECTORS_BUFFER (SECTOR_BUFFER_SIZE >> SECTOR_BITS)
+#define SECTORS_MAX (131072 >> SECTOR_BITS)

#define BLOCKS_PER_FRAME (CD_FRAMESIZE / BLOCK_SIZE)

diff -ur --exclude-from /home/axboe/exclude /opt/kernel/linux-2.4.4-pre4/drivers/scsi/sr.c linux/drivers/scsi/sr.c
--- /opt/kernel/linux-2.4.4-pre4/drivers/scsi/sr.c Mon Feb 19 19:25:17 2001
+++ linux/drivers/scsi/sr.c Wed Apr 18 13:00:32 2001
@@ -262,7 +262,7 @@
static int sr_scatter_pad(Scsi_Cmnd *SCpnt, int s_size)
{
struct scatterlist *sg, *old_sg = NULL;
- int i, fsize, bsize, sg_ent;
+ int i, fsize, bsize, sg_ent, sg_count;
char *front, *back;

back = front = NULL;
@@ -290,17 +290,24 @@
/*
* extend or allocate new scatter-gather table
*/
- if (SCpnt->use_sg)
+ sg_count = SCpnt->use_sg;
+ if (sg_count)
old_sg = (struct scatterlist *) SCpnt->request_buffer;
else {
- SCpnt->use_sg = 1;
+ sg_count = 1;
sg_ent++;
}

- SCpnt->sglist_len = ((sg_ent * sizeof(struct scatterlist)) + 511) & ~511;
- if ((sg = scsi_malloc(SCpnt->sglist_len)) == NULL)
+ i = ((sg_ent * sizeof(struct scatterlist)) + 511) & ~511;
+ if ((sg = scsi_malloc(i)) == NULL)
goto no_mem;

+ /*
+ * no more failing memory allocs possible, we can safely assign
+ * SCpnt values now
+ */
+ SCpnt->sglist_len = i;
+ SCpnt->use_sg = sg_count;
memset(sg, 0, SCpnt->sglist_len);

i = 0;
diff -ur --exclude-from /home/axboe/exclude /opt/kernel/linux-2.4.4-pre4/drivers/scsi/sr_ioctl.c linux/drivers/scsi/sr_ioctl.c
--- /opt/kernel/linux-2.4.4-pre4/drivers/scsi/sr_ioctl.c Fri Dec 29 23:07:22 2000
+++ linux/drivers/scsi/sr_ioctl.c Wed Apr 18 13:00:32 2001
@@ -530,6 +530,8 @@
target = MINOR(cdi->dev);

switch (cmd) {
+ case BLKGETSIZE:
+ return put_user(scsi_CDs[target].capacity >> 1, (long *) arg);
case BLKROSET:
case BLKROGET:
case BLKRASET:
diff -ur --exclude-from /home/axboe/exclude /opt/kernel/linux-2.4.4-pre4/include/linux/cdrom.h linux/include/linux/cdrom.h
--- /opt/kernel/linux-2.4.4-pre4/include/linux/cdrom.h Wed Apr 18 14:37:43 2001
+++ linux/include/linux/cdrom.h Wed Apr 18 13:02:10 2001
@@ -577,6 +577,8 @@
struct dvd_manufact manufact;
} dvd_struct;

+#define CDROM_MAX_CDROMS 256
+
/*
* DVD authentication ioctl
*/
@@ -732,6 +734,7 @@
devfs_handle_t de; /* real driver creates this */
/* specifications */
kdev_t dev; /* device number */
+ int nr; /* cdrom entry */
int mask; /* mask of capability: disables them */
int speed; /* maximum speed for reading data */
int capacity; /* number of discs in jukebox */