Re: [PATCH] make ide-cd handle non-2kB sector sizes

[Jens Axboe]

On Thu, Jan 22 2004, Pascal Schmidt wrote:
> 
> Hello Jens,
> 
> as I suspected, ide-cd doesn't want to play with my 512 byte sector
> MO discs. You asked me whether I could cook up a patch to support
> different hardware sector sizes, and here it is.
> 
> I've tested it with a 230 MB MO disc, which uses 512 byte sectors.
> I filled the whole disk, then ejected - reinsert - fsck - read and
> compare. Everything worked without problems. Then I inserted a
> 640 MB MO disc, which uses 2048 byte sectors, and went through the
> same procedure. No problems either, so switching between different
> sector sizes appears to work.
> 
> I've also tested with DVDs and CD-ROMs, which continue to work like
> before the patch.
> 
> Without this patch, I only get tons of I/O errors when trying to read
> or write the 512 byte sector disc.
> 
> Please check the logic of my changes.

It's a good first start, thanks for doing this. You really want to be
storing this info in the queue, though, there's a hardsector size just
for this very purpose. That way other layers know about the hardware
sector size as well, not just ide-cd. And you get other things right for
free as well, for instance ide_cdrom_prep_fs() needs a correct hardware
block size or it will build wrong cdbs.

>  static ide_startstop_t cdrom_start_write(ide_drive_t *drive, struct request *rq)
>  {
>  	struct cdrom_info *info = drive->driver_data;
> +	byte sectors_per_frame = CDROM_STATE_FLAGS(drive)->sectors_per_frame;
>  
>  	/*
> -	 * writes *must* be 2kB frame aligned
> +	 * writes *must* be 2kB frame aligned if not MO
>  	 */
> -	if ((rq->nr_sectors & 3) || (rq->sector & 3)) {
> -		cdrom_end_request(drive, 0);
> -		return ide_stopped;
> -	}
> +	if (!CDROM_CONFIG_FLAGS(drive)->mo_drive)
> +		if ((rq->nr_sectors & 3) || (rq->sector & 3)) {
> +			cdrom_end_request(drive, 0);
> +			return ide_stopped;
> +		}

Hmm, you made it a bit more confusing. It should read that writes must
be hardware sector aligned. Something ala

	if ((rq->nr_sectors << 9) & (sector_size - 1) ||
	    (rq->sector & ((sector_size >> 9) - 1)))
		problem

> -	set_capacity(drive->disk, toc->capacity * SECTORS_PER_FRAME);
> +	set_capacity(drive->disk, toc->capacity * sectors_per_frame);
> +
> +	if (CDROM_STATE_FLAGS(drive)->sectors_per_frame != sectors_per_frame)
> +		printk(KERN_INFO "%s: new hardware sector size %lu\n",
> +			drive->name, sectors_per_frame << 9);

if you feel you must print this, then do it in the same line as the
other cdrom info printed.

-- 
Jens Axboe

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/