Re: enable-reads-on-plextor-712-sa-on-26115.patch added to -mm tree

[Andy Stewart <andystewart@comcast.net>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jeff Garzik wrote:
> akpm@osdl.org wrote:
>> The patch titled
>>
>>      Enable reads on Plextor 712-SA on 2.6.11.5
>>
>> has been added to the -mm tree.  Its filename is
>>
>>      enable-reads-on-plextor-712-sa-on-26115.patch
>>
>> Patches currently in -mm which might be from andystewart@comcast.net are
>>
>> enable-reads-on-plextor-712-sa-on-26115.patch
> 
> Andrew -- The use of the word 'hack' didn't trigger any response??

HI Jeff et al,

If you are referencing the commented out debug messages, I apologize,
and will endeavor to omit those in the future.

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

> 
> 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 inquiry data were simply moved from one data structure to another.

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.

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

> 
> Even if this patch worked, you still need to fix the following:
> 
> * Patch INQUIRY data -slightly- to fool the SCSI layer into working
> correctly.  This is what Andy's patch [poorly] attempts to address.
> * Handling DRQ interrupts (early patch exists)
> * Padding DMA data (50% patch exists)
> * Fix error handling (patch exists)
> * Fix all FIS-based drivers so that an error doesn't cause an oops
> * Implement non-polled REQUEST SENSE error handling for FIS-based drivers

As with any patch, it is completely at the discretion of the kernel
developers as to whether it gets included.  If you believe this patch is
inappropriate, then please ask Andrew Morton to remove it, and I'll
respect the decision.

Thanks for looking at the code and for offering your comments.

Andy

- --
Andy Stewart, Founder
Worcester Linux Users' Group
Worcester, MA, USA
http://www.wlug.org

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCknQ8Hl0iXDssISsRAtvpAKCBwfatiK8bLCSTz6EztI+C50KP2QCeM7vd
TCIX1cYtKEdsQK8zRTNr0Do=
=ZZKj
-----END PGP SIGNATURE-----
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/