Andy Stewart wrote:

Jeff Garzik wrote:

quoted

By hardcoding so much of the inquiry data, this patch -overwrites- valid
inquiry data provided by the device, with generic data.  This patch
makes generic the probe data that the SCSI layer -depends on to be
different-.

The SCSI inquiry command does not work on this device for reasons
unknown to me.  I saw in the code where the SCSI inquiry command was
"emulated", or handled in software, for ATA devices.  I simply copied
that method for ATAPI devices.  At least that was my intent.  I cloned
one function, modified it slightly, and (I thought) called it in a
reasonable place.

All of SCSI is emulated for ATA; for ATAPI, 99% of SCSI is passed 
through to the underlying device.  The two must be treated very differently.

The SCSI layer needs to see the per-device data returned by passing the 
INQUIRY command to the device via the ATA PACKET command.

quoted

Effectively you made one CD-ROM device work, killed all the others, and
enabled an oops generator.

I fail to see how other devices would have been killed by this patch.

The SCSI layer discovers, and interprets devices based on the data 
returned by the INQUIRY command.  Your patch causes the kernel to act as 
if all ATAPI devices behave just like your Plextor, which is very very 
wrong.

It's a good thing nobody tried to use an ATAPI tape drive with your 
patch, for example.  The kernel would have thought it was a CD-ROM, and 
tried to talk to it as such.

I tested this patch on my system with many different reads, mounts, and
unmounts and never generated an oops.  Would you tell me what you did
that caused an oops?  That would help me to improve my testing before
attempting to submit future patches.

Any use of ATAPI in certain drivers (like AHCI) will cause an oops, 
because those drivers are not yet fully ATAPI aware.

quoted

Good show.

Aw, come on, Jeff.  I gave it a shot, I'm trying to give back to the
community rather than simply complain.  OK, so my work isn't perfect,
and you've pointed ont valid technical reasons why.  At least *I tried*
to contribute code rather than just offering complaints, and I'm willing
to admit that I'll need to try harder in the future.

There's nothing wrong with contributing to ATAPI debugging and development!

We just don't need to be merging such a broken patch into -mm, where it 
will cause more headaches than it will solve.

Thou Shalt Not Turn On ATAPI Before Its Time.  It's still in the realm 
of debugging patches.

	Jeff