2004-01-22 19:23:58

by Pascal Schmidt

[permalink] [raw]
Subject: [PATCH] make ide-cd handle non-2kB sector sizes


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.


--- linux-2.6.2-rc1/drivers/ide/ide-cd.h.orig Thu Jan 22 18:05:04 2004
+++ linux-2.6.2-rc1/drivers/ide/ide-cd.h Thu Jan 22 18:07:14 2004
@@ -109,6 +109,7 @@ struct ide_cd_state_flags {
__u8 door_locked : 1; /* We think that the drive door is locked. */
__u8 writing : 1; /* the drive is currently writing */
__u8 reserved : 4;
+ byte sectors_per_frame; /* Current sectors per hw frame */
byte current_speed; /* Current speed of the drive */
};

--- linux-2.6.2-rc1/drivers/ide/ide-cd.c.orig Thu Jan 22 18:04:59 2004
+++ linux-2.6.2-rc1/drivers/ide/ide-cd.c Thu Jan 22 20:07:47 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 <[email protected]>
*
*************************************************************************/

-#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;
+ byte sectors_per_frame = CDROM_STATE_FLAGS(drive)->sectors_per_frame;

/* 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;
+ byte sectors_per_frame = CDROM_STATE_FLAGS(drive)->sectors_per_frame;
+ 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,14 @@ 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;
+ byte sectors_per_frame = CDROM_STATE_FLAGS(drive)->sectors_per_frame;
int sector, frame, nskip;

sector = rq->sector;
- nskip = (sector % SECTORS_PER_FRAME);
+ nskip = (sector % sectors_per_frame);
if (nskip > 0)
sector -= nskip;
- frame = sector / SECTORS_PER_FRAME;
+ frame = sector / sectors_per_frame;

memset(rq->cmd, 0, sizeof(rq->cmd));
rq->cmd[0] = GPCMD_SEEK;
@@ -1396,6 +1383,7 @@ static ide_startstop_t cdrom_start_read
{
struct cdrom_info *info = drive->driver_data;
struct request *rq = HWGROUP(drive)->rq;
+ byte sectors_per_frame = CDROM_STATE_FLAGS(drive)->sectors_per_frame;

/* We may be retrying this request after an error. Fix up
any weirdness which might be present in the request packet. */
@@ -1411,8 +1399,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,14 +1938,16 @@ 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;
+ 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;
+ }

/*
* for dvd-ram and such media, it's a really big deal to get
@@ -1969,9 +1959,13 @@ 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 */
- info->dma = drive->using_dma ? 1 : 0;
+ /* use dma, if possible */
+ if (drive->using_dma && (rq->sector % sectors_per_frame == 0) &&
+ (rq->nr_sectors % sectors_per_frame == 0))
+ info->dma = 1;
+ else
+ info->dma = 0;
+
info->cmd = WRITE;

/* Start sending the read request to the drive. */
@@ -2209,6 +2203,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 +2222,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 +2267,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 +2287,20 @@ 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);
+
+ 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);
+
+ CDROM_STATE_FLAGS(drive)->sectors_per_frame = 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 +2412,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. */
@@ -3184,6 +3190,7 @@ int ide_cdrom_setup (ide_drive_t *drive)
CDROM_STATE_FLAGS(drive)->media_changed = 1;
CDROM_STATE_FLAGS(drive)->toc_valid = 0;
CDROM_STATE_FLAGS(drive)->door_locked = 0;
+ CDROM_STATE_FLAGS(drive)->sectors_per_frame = SECTORS_PER_FRAME;

#if NO_DOOR_LOCKING
CDROM_CONFIG_FLAGS(drive)->no_doorlock = 1;
@@ -3306,12 +3313,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


2004-01-23 09:35:43

by Jens Axboe

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

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

2004-01-23 14:02:05

by Pascal Schmidt

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

On Fri, 23 Jan 2004, Jens Axboe wrote:

> 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.

Hmmm. I'm doing

blk_queue_hardsect_size(drive->queue, sectors_per_frame << 9);

inside of cdrom_read_toc, is that not enough? Or do you mean that
I should only store it in the queue and not also in cdrom_state_flags?

> 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

I'll change that.

> > - 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.

This happens on disc change, no other info is normally printed. But this
printk can probably go away, I just put it there for debugging.

--
Ciao,
Pascal

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

On Friday 23 of January 2004 15:01, Pascal Schmidt wrote:
> On Fri, 23 Jan 2004, Jens Axboe wrote:
> > 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.
>
> Hmmm. I'm doing
>
> blk_queue_hardsect_size(drive->queue, sectors_per_frame << 9);
>
> inside of cdrom_read_toc, is that not enough? Or do you mean that
> I should only store it in the queue and not also in cdrom_state_flags?

I think Jens means storing it only in q->hardsect_size.
This way you can just use rq->q->hardsect_size << 9 to get sectors_per_frame.

> > 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
>
> I'll change that.

If you do that please remember to revert this chunk
(except comment fix of course):

@@ -1969,9 +1959,13 @@ 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 */
- info->dma = drive->using_dma ? 1 : 0;
+ /* use dma, if possible */
+ if (drive->using_dma && (rq->sector % sectors_per_frame == 0) &&
+ (rq->nr_sectors % sectors_per_frame == 0))
+ info->dma = 1;
+ else
+ info->dma = 0;
+

> > > - 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.
>
> This happens on disc change, no other info is normally printed. But this
> printk can probably go away, I just put it there for debugging.

Good.

Cheers,
--bart

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


Hi Pascal!

> --- linux-2.6.2-rc1/drivers/ide/ide-cd.h.orig Thu Jan 22 18:05:04 2004
> +++ linux-2.6.2-rc1/drivers/ide/ide-cd.h Thu Jan 22 18:07:14 2004
> @@ -109,6 +109,7 @@ struct ide_cd_state_flags {
> __u8 door_locked : 1; /* We think that the drive door is locked. */
> __u8 writing : 1; /* the drive is currently writing */
> __u8 reserved : 4;
> + byte sectors_per_frame; /* Current sectors per hw frame */
> byte current_speed; /* Current speed of the drive */
> };

Please don't use 'byte' type in your patch, use 'u8' instead.

> @@ -1346,13 +1332,14 @@ 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;
> + byte sectors_per_frame = CDROM_STATE_FLAGS(drive)->sectors_per_frame;
> int sector, frame, nskip;
>
> sector = rq->sector;
> - nskip = (sector % SECTORS_PER_FRAME);
> + nskip = (sector % sectors_per_frame);
> if (nskip > 0)
> sector -= nskip;
> - frame = sector / SECTORS_PER_FRAME;
> + frame = sector / sectors_per_frame;
>
> memset(rq->cmd, 0, sizeof(rq->cmd));
> rq->cmd[0] = GPCMD_SEEK;

You can as well clean this up while at it.
We don't need 'nskip' for calculating 'frame',

frame = rq->sector / sectors_per_frame;

should be enough.

--bart

2004-01-23 16:18:53

by Jens Axboe

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

On Fri, Jan 23 2004, Bartlomiej Zolnierkiewicz wrote:
> On Friday 23 of January 2004 15:01, Pascal Schmidt wrote:
> > On Fri, 23 Jan 2004, Jens Axboe wrote:
> > > 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.
> >
> > Hmmm. I'm doing
> >
> > blk_queue_hardsect_size(drive->queue, sectors_per_frame << 9);
> >
> > inside of cdrom_read_toc, is that not enough? Or do you mean that
> > I should only store it in the queue and not also in cdrom_state_flags?
>
> I think Jens means storing it only in q->hardsect_size. This way you
> can just use rq->q->hardsect_size << 9 to get sectors_per_frame.

Yes that's precisely what I mean, there's no need to keep the same info
in several places.

--
Jens Axboe

2004-01-23 17:37:32

by Pascal Schmidt

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

On Fri, 23 Jan 2004, Jens Axboe wrote:

> > I think Jens means storing it only in q->hardsect_size. This way you
> > can just use rq->q->hardsect_size << 9 to get sectors_per_frame.
>
> Yes that's precisely what I mean, there's no need to keep the same info
> in several places.

Yes, agreed. I'll do a new version based on your and Bart's comments.

--
Ciao,
Pascal

2004-01-23 17:36:15

by Pascal Schmidt

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

On Fri, 23 Jan 2004, Bartlomiej Zolnierkiewicz wrote:

> I think Jens means storing it only in q->hardsect_size.
> This way you can just use rq->q->hardsect_size << 9 to get
> sectors_per_frame.

Agreed (though it will have to be >> 9 ;).

> If you do that please remember to revert this chunk
> (except comment fix of course):

[dma alignment]

Agreed, thanks.

--
Ciao,
Pascal

2004-01-23 18:50:39

by Pascal Schmidt

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

On Fri, 23 Jan 2004, Jens Axboe wrote:

> Yes that's precisely what I mean, there's no need to keep the same info
> in several places.

Here's an updated patch. It doesn't work yet, though, and I can't figure
out why.

With this patch applied, I can successfully use a 512 byte sector disc.
However, then inserting a 2048 byte sector disk and trying to fsck it,
I get a dozen of:

hde: command error: status=0x51 { DriveReady SeekComplete Error }
hde: command error: error=0x70
end_request: I/O error, dev hde, sector 196608
Buffer I/O error on device hde, logical block 24576
lost page write due to I/O error on hde

Notice how the sector and logical sector numbers are different by a
factor of 8. Shouldn't this be a factor of 4?

I don't see why this behaves differently than my previous patch, which
shows no such problem.

Comments?


--- linux-2.6.2-rc1/drivers/ide/ide-cd.c.orig Thu Jan 22 18:04:59 2004
+++ linux-2.6.2-rc1/drivers/ide/ide-cd.c Fri Jan 23 19:34:02 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 <[email protected]>
*
*************************************************************************/

-#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 = queue_hardsect_size(rq->q) >> 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 = queue_hardsect_size(rq->q) >> 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 = queue_hardsect_size(rq->q) >> 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 = queue_hardsect_size(rq->q) >> 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 = queue_hardsect_size(rq->q) >> 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 != 0 ||
+ rq->sector % sectors_per_frame != 0) {
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

2004-01-23 19:04:19

by Pascal Schmidt

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

On Fri, 23 Jan 2004, Maciej W. Rozycki wrote:

> Hmm, given MO is a removable direct access device, I'd suppose ide-floppy
> would be used as the handling driver similarly to the Zip drive, wouldn't
> it?

No, ide-floppy refuses to use media with larger sectors than 512 bytes.

--
Ciao,
Pascal


2004-01-23 18:58:42

by Maciej W. Rozycki

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

On Thu, 22 Jan 2004, Pascal Schmidt wrote:

> 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.

Hmm, given MO is a removable direct access device, I'd suppose ide-floppy
would be used as the handling driver similarly to the Zip drive, wouldn't
it?

--
+ Maciej W. Rozycki, Technical University of Gdansk, Poland +
+--------------------------------------------------------------+
+ e-mail: [email protected], PGP key available +

2004-01-23 19:19:21

by Maciej W. Rozycki

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

On Fri, 23 Jan 2004, Pascal Schmidt wrote:

> > Hmm, given MO is a removable direct access device, I'd suppose ide-floppy
> > would be used as the handling driver similarly to the Zip drive, wouldn't
> > it?
>
> No, ide-floppy refuses to use media with larger sectors than 512 bytes.

So that's just opposite to what ide-cd does, but I think ide-cd should be
limited to CD-like devices with their all properties (oddities).
Specifically you can do random writes to an MO disk, perhaps even format
it, which is usually not the case with CDs.

BTW, what does ide-scsi say of the device type for the MO: is it "CD-ROM"
or "Direct-Access" or anything else? I used an MO drive (a SCSI one --
nobody was crazy enough to think of an ATAPI interface for that kinds of
devices at that time) for a short while under Linux once and it used to be
the latter, with sd, not sr being the appropriate driver.

--
+ Maciej W. Rozycki, Technical University of Gdansk, Poland +
+--------------------------------------------------------------+
+ e-mail: [email protected], PGP key available +

2004-01-23 19:56:39

by Pascal Schmidt

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

On Fri, 23 Jan 2004, Maciej W. Rozycki wrote:

> So that's just opposite to what ide-cd does, but I think ide-cd should be
> limited to CD-like devices with their all properties (oddities).

When I brought up the issue a few months back, the consensus was to
use ide-cd, not ide-floppy.

> Specifically you can do random writes to an MO disk, perhaps even format
> it, which is usually not the case with CDs.

ide-cd also handles DVD-RAM, which can also handle random writes.

> BTW, what does ide-scsi say of the device type for the MO: is it "CD-ROM"
> or "Direct-Access" or anything else? I used an MO drive (a SCSI one --
> nobody was crazy enough to think of an ATAPI interface for that kinds of
> devices at that time) for a short while under Linux once and it used to be
> the latter, with sd, not sr being the appropriate driver.

On 2.4:

scsi0 : SCSI host adapter emulation for IDE ATAPI devices
Vendor: FUJITSU Model: M25-MCC3064AP Rev: 0051
Type: Optical Device ANSI SCSI revision: 02

And yes, this uses the sd driver.

--
Ciao,
Pascal

2004-01-23 22:10:10

by Maciej W. Rozycki

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

On Fri, 23 Jan 2004, Pascal Schmidt wrote:

> > So that's just opposite to what ide-cd does, but I think ide-cd should be
> > limited to CD-like devices with their all properties (oddities).
>
> When I brought up the issue a few months back, the consensus was to
> use ide-cd, not ide-floppy.

Interesting. I would consider ide-floppy (despite its somewhat
inadequate name) the driver for all ATAPI disks as opposed to ATA disks
that use ide-disk. CD-like devices are much different, supporting such
alien to disk devices entities like tracks or audio reading or playing.

BTW, does ide-cd support partition tables yet? You typically want them
for MO disks if you want to transport data to/from other OSes or simply
because their space is big enough to create separate filesystems for
certain applications. Or perhaps swap space even. ;-)

> > Specifically you can do random writes to an MO disk, perhaps even format
> > it, which is usually not the case with CDs.
>
> ide-cd also handles DVD-RAM, which can also handle random writes.

Well, an exception rather than a rule. ;-)

> > BTW, what does ide-scsi say of the device type for the MO: is it "CD-ROM"
> > or "Direct-Access" or anything else? I used an MO drive (a SCSI one --
> > nobody was crazy enough to think of an ATAPI interface for that kinds of
> > devices at that time) for a short while under Linux once and it used to be
> > the latter, with sd, not sr being the appropriate driver.
>
> On 2.4:
>
> scsi0 : SCSI host adapter emulation for IDE ATAPI devices
> Vendor: FUJITSU Model: M25-MCC3064AP Rev: 0051
> Type: Optical Device ANSI SCSI revision: 02
>
> And yes, this uses the sd driver.

I see -- that's reasonable. And I can't understand the proposed
inconsistency with drivers -- why it should be a CD when being an ATAPI
device and a disk when being a SCSI one? After all SCSI has a separate
driver for CDs as well...

Maciej

--
+ Maciej W. Rozycki, Technical University of Gdansk, Poland +
+--------------------------------------------------------------+
+ e-mail: [email protected], PGP key available +

2004-01-23 23:29:37

by Pascal Schmidt

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

On Fri, 23 Jan 2004, Pascal Schmidt wrote:

> With this patch applied, I can successfully use a 512 byte sector disc.
> However, then inserting a 2048 byte sector disk and trying to fsck it,
> I get a dozen of:
>
> hde: command error: status=0x51 { DriveReady SeekComplete Error }
> hde: command error: error=0x70
> end_request: I/O error, dev hde, sector 196608
> Buffer I/O error on device hde, logical block 24576
> lost page write due to I/O error on hde
>
> Notice how the sector and logical sector numbers are different by a
> factor of 8. Shouldn't this be a factor of 4?
>
> I don't see why this behaves differently than my previous patch, which
> shows no such problem.

Slightly better patch, but I still don't get why this doesn't work while
my first patch does.

--- linux-2.6.2-rc1/drivers/ide/ide-cd.c.orig Thu Jan 22 18:04:59 2004
+++ linux-2.6.2-rc1/drivers/ide/ide-cd.c Sat Jan 24 00:22:25 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 <[email protected]>
*
*************************************************************************/

-#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 = queue_hardsect_size(drive->queue) >> 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 = queue_hardsect_size(drive->queue) >> 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 = queue_hardsect_size(drive->queue) >> 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 = queue_hardsect_size(drive->queue) >> 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 sector_size = queue_hardsect_size(drive->queue);

/*
- * writes *must* be 2kB frame aligned
+ * writes *must* be hardware frame aligned
*/
- if ((rq->nr_sectors & 3) || (rq->sector & 3)) {
+ if ((rq->nr_sectors << 9) & (sector_size - 1) ||
+ (rq->sector & ((sector_size >> 9) - 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

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

On Saturday 24 of January 2004 00:29, Pascal Schmidt wrote:
> On Fri, 23 Jan 2004, Pascal Schmidt wrote:
> > With this patch applied, I can successfully use a 512 byte sector disc.
> > However, then inserting a 2048 byte sector disk and trying to fsck it,
> > I get a dozen of:
> >
> > hde: command error: status=0x51 { DriveReady SeekComplete Error }
> > hde: command error: error=0x70
> > end_request: I/O error, dev hde, sector 196608
> > Buffer I/O error on device hde, logical block 24576
> > lost page write due to I/O error on hde
> >
> > Notice how the sector and logical sector numbers are different by a
> > factor of 8. Shouldn't this be a factor of 4?
> >
> > I don't see why this behaves differently than my previous patch, which
> > shows no such problem.
>
> Slightly better patch, but I still don't get why this doesn't work while
> my first patch does.

Maybe you've tested them differently?
You can start from your first patch then incrementally:
apply some changes, check, repeat.

--bart

2004-01-24 00:06:17

by Pascal Schmidt

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

On Sat, 24 Jan 2004, Bartlomiej Zolnierkiewicz wrote:

> Maybe you've tested them differently?
> You can start from your first patch then incrementally:
> apply some changes, check, repeat.

Looks like I have to. This is getting tiring after half a dozen
reboots already.

Since ide-scsi seems to work okay with the latest fixes in
Linus' tree, maybe this whole MO support stuff should dropped
from ide-cd?

--
Ciao,
Pascal

2004-01-24 00:58:48

by Pascal Schmidt

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

On Sat, 24 Jan 2004, Bartlomiej Zolnierkiewicz wrote:

> Maybe you've tested them differently?

Well, duh. Hand me the brown paper bag, please. I was accidentally
testing with a write-protected disk.

So here's a working patch, with the additional benefit of keeping
the logic in cdrom_start_write as close to the original as
possible:


--- 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 <[email protected]>
*
*************************************************************************/

-#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