libata vs ATAPI

libata vs ATAPI

[Bartlomiej Zolnierkiewicz]

Hi Jeff,

Is libata currently supposed to work with ATAPI
(I know about lack of REQUEST_SENSE support)?

With ATA_ENABLE_ATAPI defined INQUIRY seems to succeed for my
TOSHIBA ODD-DVD SD-R6372 but empty info is printed by SCSI layer.

Then REPORT_LUNS errors out (dev_stat = 0x51)...

I've noticed that DMA is used by default for all packet commands
which doesn't seem to be wise thing to do so I disabled it.
Unfortunately it doesn't help because atapi_pio_sector() assumes
transfer length to be % SECTOR_SIZE and transfer length for INQUIRY
is mere 36 bytes so this fails.

This comment in atapi_pio_sector():

	/* make sure byte count is multiple of sector size; not
	* required by standard (warning! warning!), but IDE driver
	* does this to simplify things a bit.  We are lazy, and
	* follow suit.
	*/
	if (bytes & (ATA_SECT_SIZE - 1))
		goto err_out;

looks very suspicious because ide-cd and ide-scsi seem
to be doing this but only for READ/WRITE commands.

Also ATAPI-SCSI emulation is still missing in libata-scsi.c.

I'm sure you know more about what needs to be done.
Could you make some nice, detailed TODO? :-)

Thanks,
Bartlomiej

Re: libata vs ATAPI

[Jeff Garzik]

Bartlomiej Zolnierkiewicz wrote:
> Hi Jeff,
> 
> Is libata currently supposed to work with ATAPI
> (I know about lack of REQUEST_SENSE support)?
> 
> With ATA_ENABLE_ATAPI defined INQUIRY seems to succeed for my
> TOSHIBA ODD-DVD SD-R6372 but empty info is printed by SCSI layer.

I get this too


> Then REPORT_LUNS errors out (dev_stat = 0x51)...

This doesn't surprise me, I bet that many ATAPI devices might not like 
REPORT LUNS


> I've noticed that DMA is used by default for all packet commands
> which doesn't seem to be wise thing to do so I disabled it.

DMA is _never_ used for packet commands, only for packet data.

libata enables DMA at the maximum level supported by both the device and 
host controller.  I definitely want to do this by default.


> Unfortunately it doesn't help because atapi_pio_sector() assumes
> transfer length to be % SECTOR_SIZE and transfer length for INQUIRY
> is mere 36 bytes so this fails.

Yes, you are welcome to fix this assumption.  :)

> Also ATAPI-SCSI emulation is still missing in libata-scsi.c.

I wish to minimize the emulation.  That means no read/write emulation, 
just modify INQUIRY to report MMC, and do auto-sense (as it is presented 
to the SCSI layer).


> I'm sure you know more about what needs to be done.
> Could you make some nice, detailed TODO? :-)

Pat LaVarre had IOmega ATAPI devices working, once he hand-hacked 
REQUEST SENSE.  As for to-do list, I think this email covers it.  Here's 
a summary...

* possibly avoid REPORT LUNS for ATAPI devices
* when check condition occurs, automatically request sense inside 
driver, and put that data into the sense buffer
* make INQUIRY report SCSI version MMC-3 (or later...)
* support strange sector sizes

	Jeff

Re: libata vs ATAPI

[Bartlomiej Zolnierkiewicz]

On Tue, 12 Oct 2004 16:50:20 -0400, Jeff Garzik <jgarzik@pobox.com> wrote:
> Bartlomiej Zolnierkiewicz wrote:
> > Hi Jeff,
> >
> > Is libata currently supposed to work with ATAPI
> > (I know about lack of REQUEST_SENSE support)?
> >
> > With ATA_ENABLE_ATAPI defined INQUIRY seems to succeed for my
> > TOSHIBA ODD-DVD SD-R6372 but empty info is printed by SCSI layer.
> 
> I get this too

OTOH ide-scsi gets correct INQUIRY output

> > Then REPORT_LUNS errors out (dev_stat = 0x51)...
> 
> This doesn't surprise me, I bet that many ATAPI devices might not like
> REPORT LUNS

Yes, it is not defined in original ATAPI spec.

> > I've noticed that DMA is used by default for all packet commands
> > which doesn't seem to be wise thing to do so I disabled it.
> 
> DMA is _never_ used for packet commands, only for packet data.

Yep, I know.

> libata enables DMA at the maximum level supported by both the device and
> host controller.  I definitely want to do this by default.

Me too but I suppose that there are many devices which don't like it
(otherwise restrictions in ide-cd and ide-scsi make no sense).

Jens?

> > Unfortunately it doesn't help because atapi_pio_sector() assumes
> > transfer length to be % SECTOR_SIZE and transfer length for INQUIRY
> > is mere 36 bytes so this fails.
> 
> Yes, you are welcome to fix this assumption.  :)

Another bug in this area: bcount is set to 64 * 1024 (0xFFFF) but
ATA/ATAPI spec defines max bcount as 0xFFFE and there must
be some reason why ide-scsi limits it to 63 * 1024 and ide-cd to
32 * 1024.

> > Also ATAPI-SCSI emulation is still missing in libata-scsi.c.
> 
> I wish to minimize the emulation.  That means no read/write emulation,
> just modify INQUIRY to report MMC, and do auto-sense (as it is presented
> to the SCSI layer).
> 
> 
> > I'm sure you know more about what needs to be done.
> > Could you make some nice, detailed TODO? :-)
> 
> Pat LaVarre had IOmega ATAPI devices working, once he hand-hacked
> REQUEST SENSE.  As for to-do list, I think this email covers it.  Here's
> a summary...
> 
> * possibly avoid REPORT LUNS for ATAPI devices
> * when check condition occurs, automatically request sense inside
> driver, and put that data into the sense buffer
> * make INQUIRY report SCSI version MMC-3 (or later...)
> * support strange sector sizes
> 
>         Jeff
> 

Thanks,
Bartlomiej

Re: libata vs ATAPI

[Jeff Garzik]

On Tue, Oct 12, 2004 at 11:17:38PM +0200, Bartlomiej Zolnierkiewicz wrote:
> On Tue, 12 Oct 2004 16:50:20 -0400, Jeff Garzik <jgarzik@pobox.com> wrote:
> > Bartlomiej Zolnierkiewicz wrote:
> > > Hi Jeff,
> > >
> > > Is libata currently supposed to work with ATAPI
> > > (I know about lack of REQUEST_SENSE support)?
> > >
> > > With ATA_ENABLE_ATAPI defined INQUIRY seems to succeed for my
> > > TOSHIBA ODD-DVD SD-R6372 but empty info is printed by SCSI layer.
> > 
> > I get this too
> 
> OTOH ide-scsi gets correct INQUIRY output

I spoke incorrect, I meant to say the exact opposite...!

I get correct INQUIRY output.


> > libata enables DMA at the maximum level supported by both the device and
> > host controller.  I definitely want to do this by default.
> 
> Me too but I suppose that there are many devices which don't like it
> (otherwise restrictions in ide-cd and ide-scsi make no sense).
> 
> Jens?

The IDE layer currently figures that all my test CD-ROMs support DMA, so
I would like libata to do that too ;-)


> > > Unfortunately it doesn't help because atapi_pio_sector() assumes
> > > transfer length to be % SECTOR_SIZE and transfer length for INQUIRY
> > > is mere 36 bytes so this fails.
> > 
> > Yes, you are welcome to fix this assumption.  :)
> 
> Another bug in this area: bcount is set to 64 * 1024 (0xFFFF) but
> ATA/ATAPI spec defines max bcount as 0xFFFE and there must
> be some reason why ide-scsi limits it to 63 * 1024 and ide-cd to
> 32 * 1024.

bcount should be set to 8K (one SATA FIS) for PIO,
and 0xFFFF for DMA.  It's not used for DMA.

Where is this code in libata?

	Jeff

Re: libata vs ATAPI

[Bartlomiej Zolnierkiewicz]

On Tue, 12 Oct 2004 17:22:25 -0400, Jeff Garzik <jgarzik@pobox.com> wrote:
> On Tue, Oct 12, 2004 at 11:17:38PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > On Tue, 12 Oct 2004 16:50:20 -0400, Jeff Garzik <jgarzik@pobox.com> wrote:
> > > Bartlomiej Zolnierkiewicz wrote:
> > > > Hi Jeff,
> > > >
> > > > Is libata currently supposed to work with ATAPI
> > > > (I know about lack of REQUEST_SENSE support)?
> > > >
> > > > With ATA_ENABLE_ATAPI defined INQUIRY seems to succeed for my
> > > > TOSHIBA ODD-DVD SD-R6372 but empty info is printed by SCSI layer.
> > >
> > > I get this too
> >
> > OTOH ide-scsi gets correct INQUIRY output
> 
> I spoke incorrect, I meant to say the exact opposite...!
> 
> I get correct INQUIRY output.

damn ;-)

> > > libata enables DMA at the maximum level supported by both the device and
> > > host controller.  I definitely want to do this by default.
> >
> > Me too but I suppose that there are many devices which don't like it
> > (otherwise restrictions in ide-cd and ide-scsi make no sense).
> >
> > Jens?
> 
> The IDE layer currently figures that all my test CD-ROMs support DMA, so
> I would like libata to do that too ;-)

INQUIRY is done in PIO in ide-scsi and it works for this device

> > > > Unfortunately it doesn't help because atapi_pio_sector() assumes
> > > > transfer length to be % SECTOR_SIZE and transfer length for INQUIRY
> > > > is mere 36 bytes so this fails.
> > >
> > > Yes, you are welcome to fix this assumption.  :)
> >
> > Another bug in this area: bcount is set to 64 * 1024 (0xFFFF) but
> > ATA/ATAPI spec defines max bcount as 0xFFFE and there must
> > be some reason why ide-scsi limits it to 63 * 1024 and ide-cd to
> > 32 * 1024.
> 
> bcount should be set to 8K (one SATA FIS) for PIO,
> and 0xFFFF for DMA.  It's not used for DMA.

According to ATA/ATAPI-5 spec bcount is ignored for
non-PIO transfers.

It sets bcount also for no-data which seems wrong.

> Where is this code in libata?

atapi_xlat() in libata-scsi.c

Bartlomiej

Re: libata vs ATAPI

[Jeff Garzik]

Bartlomiej Zolnierkiewicz wrote:
> INQUIRY is done in PIO in ide-scsi and it works for this device

I wouldn't mind doing non-read/write commands with PIO.  As long as the 
"fast path" commands are done via DMA...


>>>>>Unfortunately it doesn't help because atapi_pio_sector() assumes
>>>>>transfer length to be % SECTOR_SIZE and transfer length for INQUIRY
>>>>>is mere 36 bytes so this fails.
>>>>
>>>>Yes, you are welcome to fix this assumption.  :)
>>>
>>>Another bug in this area: bcount is set to 64 * 1024 (0xFFFF) but
>>>ATA/ATAPI spec defines max bcount as 0xFFFE and there must
>>>be some reason why ide-scsi limits it to 63 * 1024 and ide-cd to
>>>32 * 1024.
>>
>>bcount should be set to 8K (one SATA FIS) for PIO,
>>and 0xFFFF for DMA.  It's not used for DMA.
> 
> 
> According to ATA/ATAPI-5 spec bcount is ignored for
> non-PIO transfers.

Right.  In practice for some host controllers it should be set to 
0xffff.  It looks like I should change atapi_xlat() to do that.


> It sets bcount also for no-data which seems wrong.

well some "non-data" commands could still return sense, so I preferred 
to err on the side of caution.


>>Where is this code in libata?
> 
> atapi_xlat() in libata-scsi.c

I do not see code in atapi_xlat() that sets bcount to 0xffff?

	Jeff

Re: libata vs ATAPI

[Bartlomiej Zolnierkiewicz]

On Tue, 12 Oct 2004 17:45:55 -0400, Jeff Garzik <jgarzik@pobox.com> wrote:
> Bartlomiej Zolnierkiewicz wrote:
> > INQUIRY is done in PIO in ide-scsi and it works for this device
> 
> I wouldn't mind doing non-read/write commands with PIO.  As long as the
> "fast path" commands are done via DMA...

Exactly my POV. 

> >>>>>Unfortunately it doesn't help because atapi_pio_sector() assumes
> >>>>>transfer length to be % SECTOR_SIZE and transfer length for INQUIRY
> >>>>>is mere 36 bytes so this fails.
> >>>>
> >>>>Yes, you are welcome to fix this assumption.  :)
> >>>
> >>>Another bug in this area: bcount is set to 64 * 1024 (0xFFFF) but
> >>>ATA/ATAPI spec defines max bcount as 0xFFFE and there must
> >>>be some reason why ide-scsi limits it to 63 * 1024 and ide-cd to
> >>>32 * 1024.
> >>
> >>bcount should be set to 8K (one SATA FIS) for PIO,
> >>and 0xFFFF for DMA.  It's not used for DMA.
> >
> >
> > According to ATA/ATAPI-5 spec bcount is ignored for
> > non-PIO transfers.
> 
> Right.  In practice for some host controllers it should be set to
> 0xffff.  It looks like I should change atapi_xlat() to do that.
> 
> 
> > It sets bcount also for no-data which seems wrong.
> 
> well some "non-data" commands could still return sense, so I preferred
> to err on the side of caution.
> 
> 
> >>Where is this code in libata?
> >
> > atapi_xlat() in libata-scsi.c
> 
> I do not see code in atapi_xlat() that sets bcount to 0xffff?

My mistake, it is setting bcount to 8K (SATA FIS).  Sorry for noise.

>         Jeff
> 
>

Re: libata vs ATAPI

[Jens Axboe]

On Tue, Oct 12 2004, Jeff Garzik wrote:
> Bartlomiej Zolnierkiewicz wrote:
> >INQUIRY is done in PIO in ide-scsi and it works for this device
> 
> I wouldn't mind doing non-read/write commands with PIO.  As long as the 
> "fast path" commands are done via DMA...

If by non-read/write you mean non-READ_X/WRITE_X, then I disagree very
much. You just made cd ripping unbearable. SG_IO will use dma on ide-cd
whenever possible right now.

-- 
Jens Axboe

Re: libata vs ATAPI

[Jeff Garzik]

Jens Axboe wrote:
> On Tue, Oct 12 2004, Jeff Garzik wrote:
> 
>>Bartlomiej Zolnierkiewicz wrote:
>>
>>>INQUIRY is done in PIO in ide-scsi and it works for this device
>>
>>I wouldn't mind doing non-read/write commands with PIO.  As long as the 
>>"fast path" commands are done via DMA...
> 
> 
> If by non-read/write you mean non-READ_X/WRITE_X, then I disagree very
> much. You just made cd ripping unbearable. SG_IO will use dma on ide-cd
> whenever possible right now.


Bart fixed that, awaiting patch resend...

	Jeff

Re: libata vs ATAPI

[Jeff Garzik]

Another for the todo list:

ATAPI PIO data xfer should _read_ bcount, to determine how many bytes to 
read.  I forget if I do this, but I don't think so...

	Jeff

Re: libata vs ATAPI

[Bartlomiej Zolnierkiewicz]

On Tue, 12 Oct 2004 17:30:06 -0400, Jeff Garzik <jgarzik@pobox.com> wrote:
> Another for the todo list:
> 
> ATAPI PIO data xfer should _read_ bcount, to determine how many bytes to
> read.  I forget if I do this, but I don't think so...

atapi_pio_sector() does it...

Re: libata vs ATAPI

[Jeff Garzik]

Bartlomiej Zolnierkiewicz wrote:
> On Tue, 12 Oct 2004 17:30:06 -0400, Jeff Garzik <jgarzik@pobox.com> wrote:
> 
>>Another for the todo list:
>>
>>ATAPI PIO data xfer should _read_ bcount, to determine how many bytes to
>>read.  I forget if I do this, but I don't think so...
> 
> 
> atapi_pio_sector() does it...


Ah yes.  Well, don't add that to the todo list then :)

	Jeff

Re: libata vs ATAPI

[Jens Axboe]

On Tue, Oct 12 2004, Bartlomiej Zolnierkiewicz wrote:
> > libata enables DMA at the maximum level supported by both the device and
> > host controller.  I definitely want to do this by default.
> 
> Me too but I suppose that there are many devices which don't like it
> (otherwise restrictions in ide-cd and ide-scsi make no sense).
> 
> Jens?

There's no precedens for doing > 128KiB on ATAPI in Linux. Recently Pat
at Iomega tested 512/1024KiB requests and it worked fine. So if the
device works, we should be golden...

-- 
Jens Axboe

Re: libata vs ATAPI

[Bartlomiej Zolnierkiewicz]

On Thu, 14 Oct 2004 09:13:13 +0200, Jens Axboe <axboe@suse.de> wrote:
> On Tue, Oct 12 2004, Bartlomiej Zolnierkiewicz wrote:
> > > libata enables DMA at the maximum level supported by both the device and
> > > host controller.  I definitely want to do this by default.
> >
> > Me too but I suppose that there are many devices which don't like it
> > (otherwise restrictions in ide-cd and ide-scsi make no sense).
> >
> > Jens?
> 
> There's no precedens for doing > 128KiB on ATAPI in Linux. Recently Pat
> at Iomega tested 512/1024KiB requests and it worked fine. So if the
> device works, we should be golden...

Cool...

Jeff, I added arbitrary size ATAPI PIO support and it didn't help :(
(still empty info from SCSI layer)

Patch below:

diff -Nru a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
--- a/drivers/scsi/libata-core.c	2004-10-14 21:18:29 +02:00
+++ b/drivers/scsi/libata-core.c	2004-10-14 21:18:29 +02:00
@@ -2223,11 +2223,48 @@
 	kunmap(page);
 }
 
-static void atapi_pio_sector(struct ata_queued_cmd *qc)
+static void __atapi_pio_bytes(struct ata_queued_cmd *qc, unsigned int bytes)
+{
+	int do_write = (qc->tf.flags & ATA_TFLAG_WRITE);
+	struct scatterlist *sg = qc->sg;
+	struct ata_port *ap = qc->ap;
+	struct page *page;
+	unsigned char *buf;
+	unsigned int count;
+
+	if (qc->curbytes == qc->nbytes - bytes)
+		ap->pio_task_state = PIO_ST_LAST;
+
+next_sg:
+	sg = &sg[qc->cursg];
+	count = min(sg_dma_len(sg) - qc->cursg_ofs, bytes);
+	buf = kmap(sg->page) + sg->offset + qc->cursg_ofs;
+
+	bytes -= count;
+	qc->curbytes += count;
+	qc->cursg_ofs += count;
+
+	if (qc->cursg_ofs == sg_dma_len(sg)) {
+		qc->cursg++;
+		qc->cursg_ofs = 0;
+	}
+
+	DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
+
+	/* do the actual data transfer */
+	ata_data_xfer(ap, buf, count, do_write);
+
+	kunmap(page);
+
+	if (bytes)
+		goto next_sg;
+}
+
+static void atapi_pio_bytes(struct ata_queued_cmd *qc)
 {
 	struct ata_port *ap = qc->ap;
 	struct ata_device *dev = qc->dev;
-	unsigned int i, ireason, bc_lo, bc_hi, bytes;
+	unsigned int ireason, bc_lo, bc_hi, bytes;
 	int i_write, do_write = (qc->tf.flags & ATA_TFLAG_WRITE) ? 1 : 0;
 
 	ap->ops->tf_read(ap, &qc->tf);
@@ -2245,16 +2282,7 @@
 	if (do_write != i_write)
 		goto err_out;
 
-	/* make sure byte count is multiple of sector size; not
-	* required by standard (warning! warning!), but IDE driver
-	* does this to simplify things a bit.  We are lazy, and
-	* follow suit.
-	*/
-	if (bytes & (ATA_SECT_SIZE - 1))
-		goto err_out;
-
-	for (i = 0; i < (bytes >> 9); i++)
-		ata_pio_sector(qc);
+	__atapi_pio_bytes(qc, bytes);
 
 	return;
 
@@ -2305,7 +2333,7 @@
 	assert(qc != NULL);
 
 	if (is_atapi_taskfile(&qc->tf))
-		atapi_pio_sector(qc);
+		atapi_pio_bytes(qc);
 	else
 		ata_pio_sector(qc);
 }
@@ -2512,6 +2540,7 @@
 		qc->dev = dev;
 		qc->cursect = qc->cursg = qc->cursg_ofs = 0;
 		qc->nsect = 0;
+		qc->nbytes = qc->nbytes = 0;
 
 		ata_tf_init(ap, &qc->tf, dev->devno);
 
diff -Nru a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
--- a/drivers/scsi/libata-scsi.c	2004-10-14 21:18:29 +02:00
+++ b/drivers/scsi/libata-scsi.c	2004-10-14 21:18:29 +02:00
@@ -1464,6 +1464,9 @@
 
 	qc->tf.command = ATA_CMD_PACKET;
 
+	if (scsicmd[0] == INQUIRY)
+		using_pio = 1;
+
 	/* no data, or PIO data xfer */
 	if (using_pio || nodata) {
 		if (nodata)
@@ -1485,6 +1488,8 @@
 			qc->tf.feature |= ATAPI_DMADIR;
 #endif
 	}
+
+	qc->nbytes = cmd->bufflen;
 
 	return 0;
 }
diff -Nru a/include/linux/libata.h b/include/linux/libata.h
--- a/include/linux/libata.h	2004-10-14 21:18:29 +02:00
+++ b/include/linux/libata.h	2004-10-14 21:18:29 +02:00
@@ -231,6 +231,10 @@
 
 	unsigned int		nsect;
 	unsigned int		cursect;
+
+	unsigned int		nbytes;
+	unsigned int		curbytes;
+
 	unsigned int		cursg;
 	unsigned int		cursg_ofs;

Re: libata vs ATAPI

[Bartlomiej Zolnierkiewicz]

On Thu, 14 Oct 2004 21:19:25 +0200, Bartlomiej Zolnierkiewicz
<bzolnier@gmail.com> wrote:
> 
> 
> On Thu, 14 Oct 2004 09:13:13 +0200, Jens Axboe <axboe@suse.de> wrote:
> > On Tue, Oct 12 2004, Bartlomiej Zolnierkiewicz wrote:
> > > > libata enables DMA at the maximum level supported by both the device and
> > > > host controller.  I definitely want to do this by default.
> > >
> > > Me too but I suppose that there are many devices which don't like it
> > > (otherwise restrictions in ide-cd and ide-scsi make no sense).
> > >
> > > Jens?
> >
> > There's no precedens for doing > 128KiB on ATAPI in Linux. Recently Pat
> > at Iomega tested 512/1024KiB requests and it worked fine. So if the
> > device works, we should be golden...
> 
> Cool...
> 
> Jeff, I added arbitrary size ATAPI PIO support and it didn't help :(
> (still empty info from SCSI layer)
> 
> Patch below:

It has a small bug...
 
> diff -Nru a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
> --- a/drivers/scsi/libata-core.c        2004-10-14 21:18:29 +02:00
> +++ b/drivers/scsi/libata-core.c        2004-10-14 21:18:29 +02:00
> @@ -2223,11 +2223,48 @@
>         kunmap(page);
>  }
> 
> -static void atapi_pio_sector(struct ata_queued_cmd *qc)
> +static void __atapi_pio_bytes(struct ata_queued_cmd *qc, unsigned int bytes)
> +{
> +       int do_write = (qc->tf.flags & ATA_TFLAG_WRITE);
> +       struct scatterlist *sg = qc->sg;
> +       struct ata_port *ap = qc->ap;
> +       struct page *page;
> +       unsigned char *buf;
> +       unsigned int count;
> +
> +       if (qc->curbytes == qc->nbytes - bytes)
> +               ap->pio_task_state = PIO_ST_LAST;
> +
> +next_sg:
> +       sg = &sg[qc->cursg];
> +       count = min(sg_dma_len(sg) - qc->cursg_ofs, bytes);

page = sg->page;

should be here

I also found the real bug (feature?): ata_scsi_rbuf_get() clears
INQUIRY buffer for ATAPI when called from atapi_qc_complete().

With the patch below INQUIRY w/ DMA works just fine for me
("Vendor" and "Model" are correctly reported).

Now time for REPORT_LUNS...


[libata] fix INQUIRY command handling for ATAPI

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

diff -Nru a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
--- a/drivers/scsi/libata-scsi.c	2004-10-14 23:04:45 +02:00
+++ b/drivers/scsi/libata-scsi.c	2004-10-14 23:04:45 +02:00
@@ -895,7 +895,6 @@
 		buflen = cmd->request_bufflen;
 	}
 
-	memset(buf, 0, buflen);
 	*buf_out = buf;
 	return buflen;
 }
@@ -944,6 +943,7 @@
 	struct scsi_cmnd *cmd = args->cmd;
 
 	buflen = ata_scsi_rbuf_get(cmd, &rbuf);
+	memset(rbuf, 0, buflen);
 	rc = actor(args, rbuf, buflen);
 	ata_scsi_rbuf_put(cmd);

Re: libata vs ATAPI

[Jeff Garzik]

Bartlomiej Zolnierkiewicz wrote:
> [libata] fix INQUIRY command handling for ATAPI
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

applied to libata-dev-2.6.


> page = sg->page;
> 
> should be here
> 
> I also found the real bug (feature?): ata_scsi_rbuf_get() clears
> INQUIRY buffer for ATAPI when called from atapi_qc_complete().
> 
> With the patch below INQUIRY w/ DMA works just fine for me
> ("Vendor" and "Model" are correctly reported).

Can you please resend your first patch,

1) assuming that this "fix INQUIRY command handling" patch is applied

2) removing the following piece of code:

+	if (scsicmd[0] == INQUIRY)
+		using_pio = 1;

Thanks,

	Jeff