Re: MO: opening for write in cdrom.c

Re: MO: opening for write in cdrom.c

[Pascal Schmidt]

[...]
> I'll make that change everywhere and then send an updated version.

Here's an updated version, both patches as one.

This solves the remaining problems (512 byte sector discs) and
rough edges (write-protect detection) with ATAPI MO support via
ide-cd/cdrom for me.


--- linux-2.6.2-rc1/include/linux/cdrom.h.orig	Sun Jan 25 23:21:19 2004
+++ linux-2.6.2-rc1/include/linux/cdrom.h	Mon Jan 26 18:46:54 2004
@@ -496,6 +496,7 @@ struct cdrom_generic_command
 #define GPCMD_GET_MEDIA_STATUS		    0xda
 
 /* Mode page codes for mode sense/set */
+#define GPMODE_VENDOR_PAGE		0x00
 #define GPMODE_R_W_ERROR_PAGE		0x01
 #define GPMODE_WRITE_PARMS_PAGE		0x05
 #define GPMODE_AUDIO_CTL_PAGE		0x0e
--- linux-2.6.2-rc1/drivers/cdrom/cdrom.c.orig	Sat Jan 24 01:23:30 2004
+++ linux-2.6.2-rc1/drivers/cdrom/cdrom.c	Tue Jan 27 18:49:02 2004
@@ -696,6 +696,35 @@ static int cdrom_mrw_open_write(struct c
 	return ret;
 }
 
+static int mo_open_write(struct cdrom_device_info *cdi)
+{
+	struct cdrom_generic_command cgc;
+	char buffer[255];
+	int ret;
+	
+	init_cdrom_command(&cgc, &buffer, 4, CGC_DATA_READ);
+	cgc.quiet = 1;
+
+	/*
+	 * obtain write protect information as per
+	 * drivers/scsi/sd.c:sd_read_write_protect_flag
+	 */
+
+	ret = cdrom_mode_sense(cdi, &cgc, GPMODE_ALL_PAGES, 0);
+	if (ret)
+		ret = cdrom_mode_sense(cdi, &cgc, GPMODE_VENDOR_PAGE, 0);
+	if (ret) {
+		cgc.buflen = 255;
+		ret = cdrom_mode_sense(cdi, &cgc, GPMODE_ALL_PAGES, 0);
+	}
+
+	/* drive gave us no info, let the user go ahead */
+	if (ret)
+		return 0;
+	
+	return buffer[3] & 0x80;
+}
+
 /*
  * returns 0 for ok to open write, non-0 to disallow
  */
@@ -707,11 +736,8 @@ static int cdrom_open_write(struct cdrom
 		ret = cdrom_mrw_open_write(cdi);
 	else if (CDROM_CAN(CDC_DVD_RAM))
 		ret = cdrom_dvdram_open_write(cdi);
-	/*
-	 * needs to really check whether media is writeable
-	 */
 	else if (CDROM_CAN(CDC_MO_DRIVE))
-		ret = 0;
+		ret = mo_open_write(cdi);
 
 	return ret;
 }
--- linux-2.6.2-rc1/drivers/ide/ide-cd.c.orig	Sat Jan 24 01:24:03 2004
+++ linux-2.6.2-rc1/drivers/ide/ide-cd.c	Tue Jan 27 18:53:09 2004
@@ -294,10 +294,12 @@
  * 4.60  Dec 17, 2003	- Add mt rainier support
  *			- Bump timeout for packet commands, matches sr
  *			- Odd stuff
+ * 4.61  Jan 22, 2004	- support hardware sector sizes other than 2kB,
+ *			  Pascal Schmidt <der.eremit@email.de>
  *
  *************************************************************************/
  
-#define IDECD_VERSION "4.60"
+#define IDECD_VERSION "4.61"
 
 #include <linux/config.h>
 #include <linux/module.h>
@@ -1211,6 +1213,9 @@ static int cdrom_read_from_buffer (ide_d
 {
 	struct cdrom_info *info = drive->driver_data;
 	struct request *rq = HWGROUP(drive)->rq;
+	unsigned short sectors_per_frame;
+
+	sectors_per_frame = queue_hardsect_size(drive->queue) >> SECTOR_BITS;
 
 	/* Can't do anything if there's no buffer. */
 	if (info->buffer == NULL) return 0;
@@ -1249,7 +1254,7 @@ static int cdrom_read_from_buffer (ide_d
 	   will fail.  I think that this will never happen, but let's be
 	   paranoid and check. */
 	if (rq->current_nr_sectors < bio_cur_sectors(rq->bio) &&
-	    (rq->sector % SECTORS_PER_FRAME) != 0) {
+	    (rq->sector % sectors_per_frame) != 0) {
 		printk("%s: cdrom_read_from_buffer: buffer botch (%ld)\n",
 			drive->name, (long)rq->sector);
 		cdrom_end_request(drive, 0);
@@ -1268,13 +1273,10 @@ static int cdrom_read_from_buffer (ide_d
 static ide_startstop_t cdrom_start_read_continuation (ide_drive_t *drive)
 {
 	struct request *rq = HWGROUP(drive)->rq;
-	int nsect, sector, nframes, frame, nskip;
-
-	/* Number of sectors to transfer. */
-	nsect = rq->nr_sectors;
+	unsigned short sectors_per_frame;
+	int nskip;
 
-	/* Starting sector. */
-	sector = rq->sector;
+	sectors_per_frame = queue_hardsect_size(drive->queue) >> SECTOR_BITS;
 
 	/* If the requested sector doesn't start on a cdrom block boundary,
 	   we must adjust the start of the transfer so that it does,
@@ -1283,31 +1285,19 @@ static ide_startstop_t cdrom_start_read_
 	   of the buffer, it will mean that we're to skip a number
 	   of sectors equal to the amount by which CURRENT_NR_SECTORS
 	   is larger than the buffer size. */
-	nskip = (sector % SECTORS_PER_FRAME);
+	nskip = (rq->sector % sectors_per_frame);
 	if (nskip > 0) {
 		/* Sanity check... */
 		if (rq->current_nr_sectors != bio_cur_sectors(rq->bio) &&
-			(rq->sector % CD_FRAMESIZE != 0)) {
+			(rq->sector % sectors_per_frame != 0)) {
 			printk ("%s: cdrom_start_read_continuation: buffer botch (%u)\n",
 				drive->name, rq->current_nr_sectors);
 			cdrom_end_request(drive, 0);
 			return ide_stopped;
 		}
-		sector -= nskip;
-		nsect += nskip;
 		rq->current_nr_sectors += nskip;
 	}
 
-	/* Convert from sectors to cdrom blocks, rounding up the transfer
-	   length if needed. */
-	nframes = (nsect + SECTORS_PER_FRAME-1) / SECTORS_PER_FRAME;
-	frame = sector / SECTORS_PER_FRAME;
-
-	/* Largest number of frames was can transfer at once is 64k-1. For
-	   some drives we need to limit this even more. */
-	nframes = MIN (nframes, (CDROM_CONFIG_FLAGS (drive)->limit_nframes) ?
-		(65534 / CD_FRAMESIZE) : 65535);
-
 	/* Set up the command */
 	rq->timeout = ATAPI_WAIT_PC;
 
@@ -1346,13 +1336,11 @@ static ide_startstop_t cdrom_seek_intr (
 static ide_startstop_t cdrom_start_seek_continuation (ide_drive_t *drive)
 {
 	struct request *rq = HWGROUP(drive)->rq;
-	int sector, frame, nskip;
+	unsigned short sectors_per_frame;
+	int frame;
 
-	sector = rq->sector;
-	nskip = (sector % SECTORS_PER_FRAME);
-	if (nskip > 0)
-		sector -= nskip;
-	frame = sector / SECTORS_PER_FRAME;
+	sectors_per_frame = queue_hardsect_size(drive->queue) >> SECTOR_BITS;
+	frame = rq->sector / sectors_per_frame;
 
 	memset(rq->cmd, 0, sizeof(rq->cmd));
 	rq->cmd[0] = GPCMD_SEEK;
@@ -1396,6 +1384,9 @@ static ide_startstop_t cdrom_start_read 
 {
 	struct cdrom_info *info = drive->driver_data;
 	struct request *rq = HWGROUP(drive)->rq;
+	unsigned short sectors_per_frame;
+
+	sectors_per_frame = queue_hardsect_size(drive->queue) >> SECTOR_BITS;
 
 	/* We may be retrying this request after an error.  Fix up
 	   any weirdness which might be present in the request packet. */
@@ -1411,8 +1402,8 @@ static ide_startstop_t cdrom_start_read 
 	info->nsectors_buffered = 0;
 
 	/* use dma, if possible. */
-	if (drive->using_dma && (rq->sector % SECTORS_PER_FRAME == 0) &&
-				(rq->nr_sectors % SECTORS_PER_FRAME == 0))
+	if (drive->using_dma && (rq->sector % sectors_per_frame == 0) &&
+				(rq->nr_sectors % sectors_per_frame == 0))
 		info->dma = 1;
 	else
 		info->dma = 0;
@@ -1950,11 +1941,15 @@ static ide_startstop_t cdrom_start_write
 static ide_startstop_t cdrom_start_write(ide_drive_t *drive, struct request *rq)
 {
 	struct cdrom_info *info = drive->driver_data;
+	unsigned short sectors_per_frame;
+
+	sectors_per_frame = queue_hardsect_size(drive->queue) >> SECTOR_BITS;
 
 	/*
-	 * writes *must* be 2kB frame aligned
+	 * writes *must* be hardware frame aligned
 	 */
-	if ((rq->nr_sectors & 3) || (rq->sector & 3)) {
+	if ((rq->nr_sectors & (sectors_per_frame - 1)) ||
+	    (rq->sector & (sectors_per_frame - 1))) {
 		cdrom_end_request(drive, 0);
 		return ide_stopped;
 	}
@@ -1969,12 +1964,12 @@ static ide_startstop_t cdrom_start_write
 
 	info->nsectors_buffered = 0;
 
-        /* use dma, if possible. we don't need to check more, since we
-	 * know that the transfer is always (at least!) 2KB aligned */
+	/* use dma, if possible. we don't need to check more, since we
+	 * know that the transfer is always (at least!) frame aligned */
 	info->dma = drive->using_dma ? 1 : 0;
 	info->cmd = WRITE;
 
-	/* Start sending the read request to the drive. */
+	/* Start sending the write request to the drive. */
 	return cdrom_start_packet_command(drive, 32768, cdrom_start_write_cont);
 }
 
@@ -2209,6 +2204,7 @@ static int cdrom_eject(ide_drive_t *driv
 }
 
 static int cdrom_read_capacity(ide_drive_t *drive, unsigned long *capacity,
+			       unsigned long *sectors_per_frame,
 			       struct request_sense *sense)
 {
 	struct {
@@ -2227,8 +2223,11 @@ static int cdrom_read_capacity(ide_drive
 	req.data_len = sizeof(capbuf);
 
 	stat = cdrom_queue_packet_command(drive, &req);
-	if (stat == 0)
+	if (stat == 0) {
 		*capacity = 1 + be32_to_cpu(capbuf.lba);
+		*sectors_per_frame =
+			be32_to_cpu(capbuf.blocklen) >> SECTOR_BITS;
+	}
 
 	return stat;
 }
@@ -2270,6 +2269,7 @@ static int cdrom_read_toc(ide_drive_t *d
 		struct atapi_toc_entry  ent;
 	} ms_tmp;
 	long last_written;
+	unsigned long sectors_per_frame = SECTORS_PER_FRAME;
 
 	if (toc == NULL) {
 		/* Try to allocate space. */
@@ -2289,12 +2289,15 @@ static int cdrom_read_toc(ide_drive_t *d
 	if (CDROM_STATE_FLAGS(drive)->toc_valid)
 		return 0;
 
-	/* Try to get the total cdrom capacity. */
-	stat = cdrom_read_capacity(drive, &toc->capacity, sense);
+	/* Try to get the total cdrom capacity and sector size. */
+	stat = cdrom_read_capacity(drive, &toc->capacity, &sectors_per_frame,
+				   sense);
 	if (stat)
 		toc->capacity = 0x1fffff;
 
-	set_capacity(drive->disk, toc->capacity * SECTORS_PER_FRAME);
+	set_capacity(drive->disk, toc->capacity * sectors_per_frame);
+	blk_queue_hardsect_size(drive->queue,
+				sectors_per_frame << SECTOR_BITS);
 
 	/* First read just the header, so we know how long the TOC is. */
 	stat = cdrom_read_tocentry(drive, 0, 1, 0, (char *) &toc->hdr,
@@ -2406,7 +2409,7 @@ static int cdrom_read_toc(ide_drive_t *d
 	stat = cdrom_get_last_written(cdi, &last_written);
 	if (!stat && last_written) {
 		toc->capacity = last_written;
-		set_capacity(drive->disk, toc->capacity * SECTORS_PER_FRAME);
+		set_capacity(drive->disk, toc->capacity * sectors_per_frame);
 	}
 
 	/* Remember that we've read this stuff. */
@@ -3306,12 +3309,12 @@ int ide_cdrom_setup (ide_drive_t *drive)
 static
 sector_t ide_cdrom_capacity (ide_drive_t *drive)
 {
-	unsigned long capacity;
+	unsigned long capacity, sectors_per_frame;
 
-	if (cdrom_read_capacity(drive, &capacity, NULL))
+	if (cdrom_read_capacity(drive, &capacity, &sectors_per_frame, NULL))
 		return 0;
 
-	return capacity * SECTORS_PER_FRAME;
+	return capacity * sectors_per_frame;
 }
 
 static


-- 
Ciao,
Pascal

Re: MO: opening for write in cdrom.c

[Jens Axboe]

On Tue, Jan 27 2004, Pascal Schmidt wrote:
> 
> [...]
> > I'll make that change everywhere and then send an updated version.
> 
> Here's an updated version, both patches as one.
> 
> This solves the remaining problems (512 byte sector discs) and
> rough edges (write-protect detection) with ATAPI MO support via
> ide-cd/cdrom for me.

Alright, this is your version plus write protect io error handling.
Could you check if this works for you?

--- include/linux/cdrom.h~	2004-01-28 00:59:18.918633482 +0100
+++ include/linux/cdrom.h	2004-01-28 00:57:11.684379155 +0100
@@ -496,6 +496,7 @@
 #define GPCMD_GET_MEDIA_STATUS		    0xda
 
 /* Mode page codes for mode sense/set */
+#define GPMODE_VENDOR_PAGE		0x00
 #define GPMODE_R_W_ERROR_PAGE		0x01
 #define GPMODE_WRITE_PARMS_PAGE		0x05
 #define GPMODE_AUDIO_CTL_PAGE		0x0e
--- drivers/cdrom/cdrom.c~	2004-01-28 00:59:07.878826161 +0100
+++ drivers/cdrom/cdrom.c	2004-01-28 00:57:11.690378507 +0100
@@ -696,6 +696,35 @@
 	return ret;
 }
 
+static int mo_open_write(struct cdrom_device_info *cdi)
+{
+	struct cdrom_generic_command cgc;
+	char buffer[255];
+	int ret;
+	
+	init_cdrom_command(&cgc, &buffer, 4, CGC_DATA_READ);
+	cgc.quiet = 1;
+
+	/*
+	 * obtain write protect information as per
+	 * drivers/scsi/sd.c:sd_read_write_protect_flag
+	 */
+
+	ret = cdrom_mode_sense(cdi, &cgc, GPMODE_ALL_PAGES, 0);
+	if (ret)
+		ret = cdrom_mode_sense(cdi, &cgc, GPMODE_VENDOR_PAGE, 0);
+	if (ret) {
+		cgc.buflen = 255;
+		ret = cdrom_mode_sense(cdi, &cgc, GPMODE_ALL_PAGES, 0);
+	}
+
+	/* drive gave us no info, let the user go ahead */
+	if (ret)
+		return 0;
+	
+	return buffer[3] & 0x80;
+}
+
 /*
  * returns 0 for ok to open write, non-0 to disallow
  */
@@ -707,11 +736,8 @@
 		ret = cdrom_mrw_open_write(cdi);
 	else if (CDROM_CAN(CDC_DVD_RAM))
 		ret = cdrom_dvdram_open_write(cdi);
-	/*
-	 * needs to really check whether media is writeable
-	 */
 	else if (CDROM_CAN(CDC_MO_DRIVE))
-		ret = 0;
+		ret = mo_open_write(cdi);
 
 	return ret;
 }
--- drivers/ide/ide-cd.c~	2004-01-28 00:59:29.188523981 +0100
+++ drivers/ide/ide-cd.c	2004-01-28 01:00:41.412721285 +0100
@@ -294,10 +294,12 @@
  * 4.60  Dec 17, 2003	- Add mt rainier support
  *			- Bump timeout for packet commands, matches sr
  *			- Odd stuff
+ * 4.61  Jan 22, 2004	- support hardware sector sizes other than 2kB,
+ *			  Pascal Schmidt <der.eremit@email.de>
  *
  *************************************************************************/
  
-#define IDECD_VERSION "4.60"
+#define IDECD_VERSION "4.61"
 
 #include <linux/config.h>
 #include <linux/module.h>
@@ -818,6 +820,14 @@
 				do_end_request = 1;
 		} else if (sense_key == ILLEGAL_REQUEST ||
 			   sense_key == DATA_PROTECT) {
+			/*
+			 * check if this was a write protected media
+			 */
+			if (rq_data_dir(rq) == WRITE) {
+				printk("ide-cd: media marked write protected\n");
+				set_disk_ro(drive->disk, 1);
+			}
+				
 			/* No point in retrying after an illegal
 			   request or data protect error.*/
 			ide_dump_status (drive, "command error", stat);
@@ -1211,6 +1221,9 @@
 {
 	struct cdrom_info *info = drive->driver_data;
 	struct request *rq = HWGROUP(drive)->rq;
+	unsigned short sectors_per_frame;
+
+	sectors_per_frame = queue_hardsect_size(drive->queue) >> SECTOR_BITS;
 
 	/* Can't do anything if there's no buffer. */
 	if (info->buffer == NULL) return 0;
@@ -1249,7 +1262,7 @@
 	   will fail.  I think that this will never happen, but let's be
 	   paranoid and check. */
 	if (rq->current_nr_sectors < bio_cur_sectors(rq->bio) &&
-	    (rq->sector % SECTORS_PER_FRAME) != 0) {
+	    (rq->sector % sectors_per_frame) != 0) {
 		printk("%s: cdrom_read_from_buffer: buffer botch (%ld)\n",
 			drive->name, (long)rq->sector);
 		cdrom_end_request(drive, 0);
@@ -1268,13 +1281,10 @@
 static ide_startstop_t cdrom_start_read_continuation (ide_drive_t *drive)
 {
 	struct request *rq = HWGROUP(drive)->rq;
-	int nsect, sector, nframes, frame, nskip;
-
-	/* Number of sectors to transfer. */
-	nsect = rq->nr_sectors;
+	unsigned short sectors_per_frame;
+	int nskip;
 
-	/* Starting sector. */
-	sector = rq->sector;
+	sectors_per_frame = queue_hardsect_size(drive->queue) >> SECTOR_BITS;
 
 	/* If the requested sector doesn't start on a cdrom block boundary,
 	   we must adjust the start of the transfer so that it does,
@@ -1283,31 +1293,19 @@
 	   of the buffer, it will mean that we're to skip a number
 	   of sectors equal to the amount by which CURRENT_NR_SECTORS
 	   is larger than the buffer size. */
-	nskip = (sector % SECTORS_PER_FRAME);
+	nskip = (rq->sector % sectors_per_frame);
 	if (nskip > 0) {
 		/* Sanity check... */
 		if (rq->current_nr_sectors != bio_cur_sectors(rq->bio) &&
-			(rq->sector % CD_FRAMESIZE != 0)) {
+			(rq->sector % sectors_per_frame != 0)) {
 			printk ("%s: cdrom_start_read_continuation: buffer botch (%u)\n",
 				drive->name, rq->current_nr_sectors);
 			cdrom_end_request(drive, 0);
 			return ide_stopped;
 		}
-		sector -= nskip;
-		nsect += nskip;
 		rq->current_nr_sectors += nskip;
 	}
 
-	/* Convert from sectors to cdrom blocks, rounding up the transfer
-	   length if needed. */
-	nframes = (nsect + SECTORS_PER_FRAME-1) / SECTORS_PER_FRAME;
-	frame = sector / SECTORS_PER_FRAME;
-
-	/* Largest number of frames was can transfer at once is 64k-1. For
-	   some drives we need to limit this even more. */
-	nframes = MIN (nframes, (CDROM_CONFIG_FLAGS (drive)->limit_nframes) ?
-		(65534 / CD_FRAMESIZE) : 65535);
-
 	/* Set up the command */
 	rq->timeout = ATAPI_WAIT_PC;
 
@@ -1346,13 +1344,11 @@
 static ide_startstop_t cdrom_start_seek_continuation (ide_drive_t *drive)
 {
 	struct request *rq = HWGROUP(drive)->rq;
-	int sector, frame, nskip;
+	unsigned short sectors_per_frame;
+	int frame;
 
-	sector = rq->sector;
-	nskip = (sector % SECTORS_PER_FRAME);
-	if (nskip > 0)
-		sector -= nskip;
-	frame = sector / SECTORS_PER_FRAME;
+	sectors_per_frame = queue_hardsect_size(drive->queue) >> SECTOR_BITS;
+	frame = rq->sector / sectors_per_frame;
 
 	memset(rq->cmd, 0, sizeof(rq->cmd));
 	rq->cmd[0] = GPCMD_SEEK;
@@ -1396,6 +1392,9 @@
 {
 	struct cdrom_info *info = drive->driver_data;
 	struct request *rq = HWGROUP(drive)->rq;
+	unsigned short sectors_per_frame;
+
+	sectors_per_frame = queue_hardsect_size(drive->queue) >> SECTOR_BITS;
 
 	/* We may be retrying this request after an error.  Fix up
 	   any weirdness which might be present in the request packet. */
@@ -1411,8 +1410,8 @@
 	info->nsectors_buffered = 0;
 
 	/* use dma, if possible. */
-	if (drive->using_dma && (rq->sector % SECTORS_PER_FRAME == 0) &&
-				(rq->nr_sectors % SECTORS_PER_FRAME == 0))
+	if (drive->using_dma && (rq->sector % sectors_per_frame == 0) &&
+				(rq->nr_sectors % sectors_per_frame == 0))
 		info->dma = 1;
 	else
 		info->dma = 0;
@@ -1950,11 +1949,22 @@
 static ide_startstop_t cdrom_start_write(ide_drive_t *drive, struct request *rq)
 {
 	struct cdrom_info *info = drive->driver_data;
+	struct gendisk *g = drive->disk;
+	unsigned short sectors_per_frame = queue_hardsect_size(drive->queue) >> SECTOR_BITS;
 
 	/*
-	 * writes *must* be 2kB frame aligned
+	 * writes *must* be hardware frame aligned
 	 */
-	if ((rq->nr_sectors & 3) || (rq->sector & 3)) {
+	if ((rq->nr_sectors & (sectors_per_frame - 1)) ||
+	    (rq->sector & (sectors_per_frame - 1))) {
+		cdrom_end_request(drive, 0);
+		return ide_stopped;
+	}
+
+	/*
+	 * disk has become write protected
+	 */
+	if (g->policy) {
 		cdrom_end_request(drive, 0);
 		return ide_stopped;
 	}
@@ -1969,12 +1979,12 @@
 
 	info->nsectors_buffered = 0;
 
-        /* use dma, if possible. we don't need to check more, since we
-	 * know that the transfer is always (at least!) 2KB aligned */
+	/* use dma, if possible. we don't need to check more, since we
+	 * know that the transfer is always (at least!) frame aligned */
 	info->dma = drive->using_dma ? 1 : 0;
 	info->cmd = WRITE;
 
-	/* Start sending the read request to the drive. */
+	/* Start sending the write request to the drive. */
 	return cdrom_start_packet_command(drive, 32768, cdrom_start_write_cont);
 }
 
@@ -2209,6 +2219,7 @@
 }
 
 static int cdrom_read_capacity(ide_drive_t *drive, unsigned long *capacity,
+			       unsigned long *sectors_per_frame,
 			       struct request_sense *sense)
 {
 	struct {
@@ -2227,8 +2238,11 @@
 	req.data_len = sizeof(capbuf);
 
 	stat = cdrom_queue_packet_command(drive, &req);
-	if (stat == 0)
+	if (stat == 0) {
 		*capacity = 1 + be32_to_cpu(capbuf.lba);
+		*sectors_per_frame =
+			be32_to_cpu(capbuf.blocklen) >> SECTOR_BITS;
+	}
 
 	return stat;
 }
@@ -2270,6 +2284,7 @@
 		struct atapi_toc_entry  ent;
 	} ms_tmp;
 	long last_written;
+	unsigned long sectors_per_frame = SECTORS_PER_FRAME;
 
 	if (toc == NULL) {
 		/* Try to allocate space. */
@@ -2289,12 +2304,15 @@
 	if (CDROM_STATE_FLAGS(drive)->toc_valid)
 		return 0;
 
-	/* Try to get the total cdrom capacity. */
-	stat = cdrom_read_capacity(drive, &toc->capacity, sense);
+	/* Try to get the total cdrom capacity and sector size. */
+	stat = cdrom_read_capacity(drive, &toc->capacity, &sectors_per_frame,
+				   sense);
 	if (stat)
 		toc->capacity = 0x1fffff;
 
-	set_capacity(drive->disk, toc->capacity * SECTORS_PER_FRAME);
+	set_capacity(drive->disk, toc->capacity * sectors_per_frame);
+	blk_queue_hardsect_size(drive->queue,
+				sectors_per_frame << SECTOR_BITS);
 
 	/* First read just the header, so we know how long the TOC is. */
 	stat = cdrom_read_tocentry(drive, 0, 1, 0, (char *) &toc->hdr,
@@ -2406,7 +2424,7 @@
 	stat = cdrom_get_last_written(cdi, &last_written);
 	if (!stat && last_written) {
 		toc->capacity = last_written;
-		set_capacity(drive->disk, toc->capacity * SECTORS_PER_FRAME);
+		set_capacity(drive->disk, toc->capacity * sectors_per_frame);
 	}
 
 	/* Remember that we've read this stuff. */
@@ -3306,12 +3324,12 @@
 static
 sector_t ide_cdrom_capacity (ide_drive_t *drive)
 {
-	unsigned long capacity;
+	unsigned long capacity, sectors_per_frame;
 
-	if (cdrom_read_capacity(drive, &capacity, NULL))
+	if (cdrom_read_capacity(drive, &capacity, &sectors_per_frame, NULL))
 		return 0;
 
-	return capacity * SECTORS_PER_FRAME;
+	return capacity * sectors_per_frame;
 }
 
 static

-- 
Jens Axboe

Re: MO: opening for write in cdrom.c

[Pascal Schmidt]

On Wed, 28 Jan 2004, Jens Axboe wrote:

> Alright, this is your version plus write protect io error handling.
> Could you check if this works for you?

Works for me. I've tested the fallback by making mo_open_write always 
succeed and then inserting a write-protected disc.

I now only get one error report from the drive, the rest of the
writes are correctly denied before hitting the drive.

-- 
Ciao,
Pascal

Re: MO: opening for write in cdrom.c

[Jens Axboe]

On Wed, Jan 28 2004, Pascal Schmidt wrote:
> On Wed, 28 Jan 2004, Jens Axboe wrote:
> 
> > Alright, this is your version plus write protect io error handling.
> > Could you check if this works for you?
> 
> Works for me. I've tested the fallback by making mo_open_write always 
> succeed and then inserting a write-protected disc.
> 
> I now only get one error report from the drive, the rest of the
> writes are correctly denied before hitting the drive.

Excellent, thanks a lot for the work and testing.

-- 
Jens Axboe

Re: MO: opening for write in cdrom.c

[Pascal Schmidt]

On Wed, 28 Jan 2004, Jens Axboe wrote:

> Excellent, thanks a lot for the work and testing.

No problem. It was me, after all, who started to push all this MO
support stuff for the cdrom drivers. ;)

-- 
Ciao,
Pascal

Re: MO: opening for write in cdrom.c

[Pascal Schmidt]

On Tue, 27 Jan 2004 17:40:45 +0100, you wrote in linux.kernel:

> I'm surprised the sense messages don't show that it's a write to a write
> protected disc (xx/27/zz, where xx == 0x07 or 0x05).

Yep, I wasn't precise, that shows up before the error=0x70 line.

> However, it's even
> more annoying to _not_ be able to write to a media because the kernel
> thinks it knows better. In your fsck case you sort of get what you ask
> for, by shooting yourself in the foot :)

Agreed. It just bit me extra hard because I was thinking my sector
size patches were screwing things up. ;)
 
>> It's fine with me either way. Do you want me to resend with the
>> default fallback changed?
> Yes please.

Ok, I'll do that.

>> +	unsigned short sectors_per_frame = drive->queue->hardsect_size >> 9;
> 
> Nitpick: sectors_per_frame = queue_hardsect_size(q) >> 9;

Yes, I agree, better to use the existing abstraction. I'll split it
then and initialize it as the first line of the function(s), otherwise
it doesn't fit into 80 columns.

> That's about, the rest looks fine.

I'll make that change everywhere and then send an updated version.

-- 
Ciao,
Pascal

Re: MO: opening for write in cdrom.c

[Jens Axboe]

On Tue, Jan 27 2004, Pascal Schmidt wrote:
> On Tue, 27 Jan 2004 17:40:45 +0100, you wrote in linux.kernel:
> 
> > I'm surprised the sense messages don't show that it's a write to a write
> > protected disc (xx/27/zz, where xx == 0x07 or 0x05).
> 
> Yep, I wasn't precise, that shows up before the error=0x70 line.

Good, so it's catchable easily even for your drive. So mark the disc
write protected when you see this error on write, and print a message
saying so.

-- 
Jens Axboe

Re: MO: opening for write in cdrom.c

[Pascal Schmidt]

On Mon, 26 Jan 2004, Pascal Schmidt wrote:

[short summary for l-k: this is about finding out whether an MO is 
write-protected or not, code that is yet missing from cdrom.c]

> I'll try to implement the fallback that sd.c uses next. That code
> tries several mode senses with different page and length.

Okay, I got it working with the exact method that sd.c uses. I've put
a few printk's in to see where it fails a mode sense. It's actually
inconsistent:

	a) insert a writable disc first after boot, method 1 works
	b) then insert a non-writable disc once - suddenly method 1
	   stops working, even on writable discs, instead method 2
	   works
	c) insert a non-writable disc first after boot, method 1
	   never works, but method 2 does

There's a third method in sd.c. I've also left that in since I suspect
it might be necessary under some circumstances, too.

>From my testing, I get the impression that this Fujitsu drive only
has mode page 0, meaning that only 0x00 and 0x3F make sense, and that
mode page 0x00 also only contains very few bytes of information -
because asking for 16 bytes from 0x3F didn't work, but 4 bytes does.
What's weird is that asking for all pages can also stop suddenly, after
which only page 0x00 is accessible. And when that happens, we only get
a meaningless request sense of 00/00/00 back.

Oh well, strange hardware indeed.

Here's the patch that works for me, please consider applying and
pushing to Linus/Andrew:


--- linux-2.6.2-rc1/include/linux/cdrom.h.orig	Sun Jan 25 23:21:19 2004
+++ linux-2.6.2-rc1/include/linux/cdrom.h	Mon Jan 26 18:46:54 2004
@@ -496,6 +496,7 @@ struct cdrom_generic_command
 #define GPCMD_GET_MEDIA_STATUS		    0xda
 
 /* Mode page codes for mode sense/set */
+#define GPMODE_VENDOR_PAGE		0x00
 #define GPMODE_R_W_ERROR_PAGE		0x01
 #define GPMODE_WRITE_PARMS_PAGE		0x05
 #define GPMODE_AUDIO_CTL_PAGE		0x0e
--- linux-2.6.2-rc1/drivers/cdrom/cdrom.c.orig	Sat Jan 24 01:23:30 2004
+++ linux-2.6.2-rc1/drivers/cdrom/cdrom.c	Mon Jan 26 18:52:00 2004
@@ -696,6 +696,34 @@ static int cdrom_mrw_open_write(struct c
 	return ret;
 }
 
+static int mo_open_write(struct cdrom_device_info *cdi)
+{
+	struct cdrom_generic_command cgc;
+	char buffer[255];
+	int ret;
+	
+	init_cdrom_command(&cgc, &buffer, 4, CGC_DATA_READ);
+	cgc.quiet = 1;
+
+	/*
+	 * obtain write protect information as per
+	 * drivers/scsi/sd.c:sd_read_write_protect_flag
+	 */
+
+	ret = cdrom_mode_sense(cdi, &cgc, GPMODE_ALL_PAGES, 0);
+	if (ret)
+		ret = cdrom_mode_sense(cdi, &cgc, GPMODE_VENDOR_PAGE, 0);
+	if (ret) {
+		cgc.buflen = 255;
+		ret = cdrom_mode_sense(cdi, &cgc, GPMODE_ALL_PAGES, 0);
+	}
+
+	if (ret)
+		return 1;
+	
+	return (buffer[3] & 0x80) != 0;
+}
+
 /*
  * returns 0 for ok to open write, non-0 to disallow
  */
@@ -707,11 +735,8 @@ static int cdrom_open_write(struct cdrom
 		ret = cdrom_mrw_open_write(cdi);
 	else if (CDROM_CAN(CDC_DVD_RAM))
 		ret = cdrom_dvdram_open_write(cdi);
-	/*
-	 * needs to really check whether media is writeable
-	 */
 	else if (CDROM_CAN(CDC_MO_DRIVE))
-		ret = 0;
+		ret = mo_open_write(cdi);
 
 	return ret;
 }


-- 
Ciao,
Pascal

Re: MO: opening for write in cdrom.c

[Jens Axboe]

On Mon, Jan 26 2004, Pascal Schmidt wrote:
> On Mon, 26 Jan 2004, Pascal Schmidt wrote:
> 
> [short summary for l-k: this is about finding out whether an MO is 
> write-protected or not, code that is yet missing from cdrom.c]
> 
> > I'll try to implement the fallback that sd.c uses next. That code
> > tries several mode senses with different page and length.
> 
> Okay, I got it working with the exact method that sd.c uses. I've put
> a few printk's in to see where it fails a mode sense. It's actually
> inconsistent:
> 
> 	a) insert a writable disc first after boot, method 1 works
> 	b) then insert a non-writable disc once - suddenly method 1
> 	   stops working, even on writable discs, instead method 2
> 	   works
> 	c) insert a non-writable disc first after boot, method 1
> 	   never works, but method 2 does

Sounds pretty shaky...

> There's a third method in sd.c. I've also left that in since I suspect
> it might be necessary under some circumstances, too.

Probably a good idea.

> >From my testing, I get the impression that this Fujitsu drive only
> has mode page 0, meaning that only 0x00 and 0x3F make sense, and that
> mode page 0x00 also only contains very few bytes of information -
> because asking for 16 bytes from 0x3F didn't work, but 4 bytes does.
> What's weird is that asking for all pages can also stop suddenly, after
> which only page 0x00 is accessible. And when that happens, we only get
> a meaningless request sense of 00/00/00 back.
> 
> Oh well, strange hardware indeed.
> 
> Here's the patch that works for me, please consider applying and
> pushing to Linus/Andrew:

Hmm, looks a bit strange. You want write protect to be set _if_
detection works, not otherwise. If it fails, just assume that you can
write to the drive and let the normal drive rejection work fail those
(maybe even catch them and write protect then). Seeing as the method is
unreliable, we cannot solely rely on that.

-- 
Jens Axboe

Re: MO: opening for write in cdrom.c

[Pascal Schmidt]

On Tue, 27 Jan 2004, Jens Axboe wrote:

> Sounds pretty shaky...

This drive is a bit strange, yeah.

> Hmm, looks a bit strange. You want write protect to be set _if_
> detection works, not otherwise. If it fails, just assume that you can
> write to the drive and let the normal drive rejection work fail those
> (maybe even catch them and write protect then).

Before I had this worked out, I accidentally tried fscking a
write-protected disc. Yes, it errored out at the end of the fsck, but
the error messages are quite unintuitive. Lot's of I/O error in the
kernel log and an error=0x70 from the drive. I sure didn't realize
at first what the problem really was.

> Seeing as the method is
> unreliable, we cannot solely rely on that.

I've not seen all three variants fail, yet. I was following sd.c's
example, that also sets write protect by default if it cannot get
info from the drive.

It's fine with me either way. Do you want me to resend with the
default fallback changed?

Oh, BTW, while I have your attention ;), have you looked at my
latest variable blocksize support patch for ide-cd? I've tried to
incorporate yours and Bart's suggestions.

Here it is again in case you missed it in the thread.


--- linux-2.6.2-rc1/drivers/ide/ide-cd.c.orig	Sat Jan 24 01:24:03 2004
+++ linux-2.6.2-rc1/drivers/ide/ide-cd.c	Sat Jan 24 01:39:40 2004
@@ -294,10 +294,12 @@
  * 4.60  Dec 17, 2003	- Add mt rainier support
  *			- Bump timeout for packet commands, matches sr
  *			- Odd stuff
+ * 4.61  Jan 22, 2004	- support hardware sector sizes other than 2kB,
+ *			  Pascal Schmidt <der.eremit@email.de>
  *
  *************************************************************************/
  
-#define IDECD_VERSION "4.60"
+#define IDECD_VERSION "4.61"
 
 #include <linux/config.h>
 #include <linux/module.h>
@@ -1211,6 +1213,7 @@ static int cdrom_read_from_buffer (ide_d
 {
 	struct cdrom_info *info = drive->driver_data;
 	struct request *rq = HWGROUP(drive)->rq;
+	unsigned short sectors_per_frame = drive->queue->hardsect_size >> 9;
 
 	/* Can't do anything if there's no buffer. */
 	if (info->buffer == NULL) return 0;
@@ -1249,7 +1252,7 @@ static int cdrom_read_from_buffer (ide_d
 	   will fail.  I think that this will never happen, but let's be
 	   paranoid and check. */
 	if (rq->current_nr_sectors < bio_cur_sectors(rq->bio) &&
-	    (rq->sector % SECTORS_PER_FRAME) != 0) {
+	    (rq->sector % sectors_per_frame) != 0) {
 		printk("%s: cdrom_read_from_buffer: buffer botch (%ld)\n",
 			drive->name, (long)rq->sector);
 		cdrom_end_request(drive, 0);
@@ -1268,13 +1271,8 @@ static int cdrom_read_from_buffer (ide_d
 static ide_startstop_t cdrom_start_read_continuation (ide_drive_t *drive)
 {
 	struct request *rq = HWGROUP(drive)->rq;
-	int nsect, sector, nframes, frame, nskip;
-
-	/* Number of sectors to transfer. */
-	nsect = rq->nr_sectors;
-
-	/* Starting sector. */
-	sector = rq->sector;
+	unsigned short sectors_per_frame = drive->queue->hardsect_size >> 9;
+	int nskip;
 
 	/* If the requested sector doesn't start on a cdrom block boundary,
 	   we must adjust the start of the transfer so that it does,
@@ -1283,31 +1281,19 @@ static ide_startstop_t cdrom_start_read_
 	   of the buffer, it will mean that we're to skip a number
 	   of sectors equal to the amount by which CURRENT_NR_SECTORS
 	   is larger than the buffer size. */
-	nskip = (sector % SECTORS_PER_FRAME);
+	nskip = (rq->sector % sectors_per_frame);
 	if (nskip > 0) {
 		/* Sanity check... */
 		if (rq->current_nr_sectors != bio_cur_sectors(rq->bio) &&
-			(rq->sector % CD_FRAMESIZE != 0)) {
+			(rq->sector % sectors_per_frame != 0)) {
 			printk ("%s: cdrom_start_read_continuation: buffer botch (%u)\n",
 				drive->name, rq->current_nr_sectors);
 			cdrom_end_request(drive, 0);
 			return ide_stopped;
 		}
-		sector -= nskip;
-		nsect += nskip;
 		rq->current_nr_sectors += nskip;
 	}
 
-	/* Convert from sectors to cdrom blocks, rounding up the transfer
-	   length if needed. */
-	nframes = (nsect + SECTORS_PER_FRAME-1) / SECTORS_PER_FRAME;
-	frame = sector / SECTORS_PER_FRAME;
-
-	/* Largest number of frames was can transfer at once is 64k-1. For
-	   some drives we need to limit this even more. */
-	nframes = MIN (nframes, (CDROM_CONFIG_FLAGS (drive)->limit_nframes) ?
-		(65534 / CD_FRAMESIZE) : 65535);
-
 	/* Set up the command */
 	rq->timeout = ATAPI_WAIT_PC;
 
@@ -1346,13 +1332,10 @@ static ide_startstop_t cdrom_seek_intr (
 static ide_startstop_t cdrom_start_seek_continuation (ide_drive_t *drive)
 {
 	struct request *rq = HWGROUP(drive)->rq;
-	int sector, frame, nskip;
+	unsigned short sectors_per_frame = drive->queue->hardsect_size >> 9;
+	int frame;
 
-	sector = rq->sector;
-	nskip = (sector % SECTORS_PER_FRAME);
-	if (nskip > 0)
-		sector -= nskip;
-	frame = sector / SECTORS_PER_FRAME;
+	frame = rq->sector / sectors_per_frame;
 
 	memset(rq->cmd, 0, sizeof(rq->cmd));
 	rq->cmd[0] = GPCMD_SEEK;
@@ -1396,6 +1379,7 @@ static ide_startstop_t cdrom_start_read 
 {
 	struct cdrom_info *info = drive->driver_data;
 	struct request *rq = HWGROUP(drive)->rq;
+	unsigned short sectors_per_frame = drive->queue->hardsect_size >> 9;
 
 	/* We may be retrying this request after an error.  Fix up
 	   any weirdness which might be present in the request packet. */
@@ -1411,8 +1395,8 @@ static ide_startstop_t cdrom_start_read 
 	info->nsectors_buffered = 0;
 
 	/* use dma, if possible. */
-	if (drive->using_dma && (rq->sector % SECTORS_PER_FRAME == 0) &&
-				(rq->nr_sectors % SECTORS_PER_FRAME == 0))
+	if (drive->using_dma && (rq->sector % sectors_per_frame == 0) &&
+				(rq->nr_sectors % sectors_per_frame == 0))
 		info->dma = 1;
 	else
 		info->dma = 0;
@@ -1950,11 +1934,13 @@ static ide_startstop_t cdrom_start_write
 static ide_startstop_t cdrom_start_write(ide_drive_t *drive, struct request *rq)
 {
 	struct cdrom_info *info = drive->driver_data;
+	unsigned short sectors_per_frame = drive->queue->hardsect_size >> 9;
 
 	/*
-	 * writes *must* be 2kB frame aligned
+	 * writes *must* be hardware frame aligned
 	 */
-	if ((rq->nr_sectors & 3) || (rq->sector & 3)) {
+	if ((rq->nr_sectors & (sectors_per_frame - 1)) ||
+	    (rq->sector & (sectors_per_frame - 1))) {
 		cdrom_end_request(drive, 0);
 		return ide_stopped;
 	}
@@ -1969,12 +1955,12 @@ static ide_startstop_t cdrom_start_write
 
 	info->nsectors_buffered = 0;
 
-        /* use dma, if possible. we don't need to check more, since we
-	 * know that the transfer is always (at least!) 2KB aligned */
+	/* use dma, if possible. we don't need to check more, since we
+	 * know that the transfer is always (at least!) frame aligned */
 	info->dma = drive->using_dma ? 1 : 0;
 	info->cmd = WRITE;
 
-	/* Start sending the read request to the drive. */
+	/* Start sending the write request to the drive. */
 	return cdrom_start_packet_command(drive, 32768, cdrom_start_write_cont);
 }
 
@@ -2209,6 +2195,7 @@ static int cdrom_eject(ide_drive_t *driv
 }
 
 static int cdrom_read_capacity(ide_drive_t *drive, unsigned long *capacity,
+			       unsigned long *sectors_per_frame,
 			       struct request_sense *sense)
 {
 	struct {
@@ -2227,8 +2214,10 @@ static int cdrom_read_capacity(ide_drive
 	req.data_len = sizeof(capbuf);
 
 	stat = cdrom_queue_packet_command(drive, &req);
-	if (stat == 0)
+	if (stat == 0) {
 		*capacity = 1 + be32_to_cpu(capbuf.lba);
+		*sectors_per_frame = be32_to_cpu(capbuf.blocklen) >> 9;
+	}
 
 	return stat;
 }
@@ -2270,6 +2259,7 @@ static int cdrom_read_toc(ide_drive_t *d
 		struct atapi_toc_entry  ent;
 	} ms_tmp;
 	long last_written;
+	unsigned long sectors_per_frame = SECTORS_PER_FRAME;
 
 	if (toc == NULL) {
 		/* Try to allocate space. */
@@ -2289,12 +2279,14 @@ static int cdrom_read_toc(ide_drive_t *d
 	if (CDROM_STATE_FLAGS(drive)->toc_valid)
 		return 0;
 
-	/* Try to get the total cdrom capacity. */
-	stat = cdrom_read_capacity(drive, &toc->capacity, sense);
+	/* Try to get the total cdrom capacity and sector size. */
+	stat = cdrom_read_capacity(drive, &toc->capacity, &sectors_per_frame,
+				   sense);
 	if (stat)
 		toc->capacity = 0x1fffff;
 
-	set_capacity(drive->disk, toc->capacity * SECTORS_PER_FRAME);
+	set_capacity(drive->disk, toc->capacity * sectors_per_frame);
+	blk_queue_hardsect_size(drive->queue, sectors_per_frame << 9);
 
 	/* First read just the header, so we know how long the TOC is. */
 	stat = cdrom_read_tocentry(drive, 0, 1, 0, (char *) &toc->hdr,
@@ -2406,7 +2398,7 @@ static int cdrom_read_toc(ide_drive_t *d
 	stat = cdrom_get_last_written(cdi, &last_written);
 	if (!stat && last_written) {
 		toc->capacity = last_written;
-		set_capacity(drive->disk, toc->capacity * SECTORS_PER_FRAME);
+		set_capacity(drive->disk, toc->capacity * sectors_per_frame);
 	}
 
 	/* Remember that we've read this stuff. */
@@ -3306,12 +3298,12 @@ int ide_cdrom_setup (ide_drive_t *drive)
 static
 sector_t ide_cdrom_capacity (ide_drive_t *drive)
 {
-	unsigned long capacity;
+	unsigned long capacity, sectors_per_frame;
 
-	if (cdrom_read_capacity(drive, &capacity, NULL))
+	if (cdrom_read_capacity(drive, &capacity, &sectors_per_frame, NULL))
 		return 0;
 
-	return capacity * SECTORS_PER_FRAME;
+	return capacity * sectors_per_frame;
 }
 
 static


-- 
Ciao,
Pascal

Re: MO: opening for write in cdrom.c

[Jens Axboe]

On Tue, Jan 27 2004, Pascal Schmidt wrote:
> > Hmm, looks a bit strange. You want write protect to be set _if_
> > detection works, not otherwise. If it fails, just assume that you can
> > write to the drive and let the normal drive rejection work fail those
> > (maybe even catch them and write protect then).
> 
> Before I had this worked out, I accidentally tried fscking a
> write-protected disc. Yes, it errored out at the end of the fsck, but
> the error messages are quite unintuitive. Lot's of I/O error in the
> kernel log and an error=0x70 from the drive. I sure didn't realize
> at first what the problem really was.

I'm surprised the sense messages don't show that it's a write to a write
protected disc (xx/27/zz, where xx == 0x07 or 0x05). However, it's even
more annoying to _not_ be able to write to a media because the kernel
thinks it knows better. In your fsck case you sort of get what you ask
for, by shooting yourself in the foot :)

> > Seeing as the method is
> > unreliable, we cannot solely rely on that.
> 
> I've not seen all three variants fail, yet. I was following sd.c's
> example, that also sets write protect by default if it cannot get
> info from the drive.
> 
> It's fine with me either way. Do you want me to resend with the
> default fallback changed?

Yes please.

> Oh, BTW, while I have your attention ;), have you looked at my
> latest variable blocksize support patch for ide-cd? I've tried to
> incorporate yours and Bart's suggestions.
> 
> Here it is again in case you missed it in the thread.
> 
> 
> --- linux-2.6.2-rc1/drivers/ide/ide-cd.c.orig	Sat Jan 24 01:24:03 2004
> +++ linux-2.6.2-rc1/drivers/ide/ide-cd.c	Sat Jan 24 01:39:40 2004
> @@ -294,10 +294,12 @@
>   * 4.60  Dec 17, 2003	- Add mt rainier support
>   *			- Bump timeout for packet commands, matches sr
>   *			- Odd stuff
> + * 4.61  Jan 22, 2004	- support hardware sector sizes other than 2kB,
> + *			  Pascal Schmidt <der.eremit@email.de>
>   *
>   *************************************************************************/
>   
> -#define IDECD_VERSION "4.60"
> +#define IDECD_VERSION "4.61"
>  
>  #include <linux/config.h>
>  #include <linux/module.h>
> @@ -1211,6 +1213,7 @@ static int cdrom_read_from_buffer (ide_d
>  {
>  	struct cdrom_info *info = drive->driver_data;
>  	struct request *rq = HWGROUP(drive)->rq;
> +	unsigned short sectors_per_frame = drive->queue->hardsect_size >> 9;

Nitpick: sectors_per_frame = queue_hardsect_size(q) >> 9;

That's about, the rest looks fine.

-- 
Jens Axboe