PATCH: scsi-sd-last-sector-bug-flag.patch

Hi all,

This patch adds a new scsi_device flag for devices which contain a bug where the device crashes when the last sector is read in a larger then 1 sector read.

This is for example the case with sdcards in the HP PSC1350 printer cardreader and in the HP PSC1610 printer cardreader.

Regards,

Hans

This patch adds a new scsi_device flag for devices which contain a bug where the device crashes when the last sector is read in a larger then 1 sector read.

This is for example the case with sdcards in the HP PSC1350 printer cardreader and in the HP PSC1610 printer cardreader.

diff -up vanilla-2.6.24-rc7/include/scsi/scsi_device.h.psc1350 vanilla-2.6.24-rc7/include/scsi/scsi_device.h
@@ -142,6 +142,7 @@ struct scsi_device {
 	unsigned fix_capacity:1;	/* READ_CAPACITY is too high by 1 */
 	unsigned guess_capacity:1;	/* READ_CAPACITY might be too high by 1 */
 	unsigned retry_hwerror:1;	/* Retry HARDWARE_ERROR */
+	unsigned last_sector_bug:1;	/* Always read last sector in a 1 sector read */
 
 	DECLARE_BITMAP(supported_events, SDEV_EVT_MAXBITS); /* supported events */
 	struct list_head event_list;	/* asserted events */

diff -up vanilla-2.6.24-rc7/drivers/scsi/sd.c.psc1350 vanilla-2.6.24-rc7/drivers/scsi/sd.c
@@ -395,6 +395,16 @@ static int sd_prep_fn(struct request_que
 		goto out;
 	}
 
+	/* 
+	 * Some devices (some sdcards for one) don't like it if the last sector
+	 * gets read in a larger then 1 sector read.
+	 */
+	if (unlikely(sdp->last_sector_bug && 
+	    rq->nr_sectors > sdp->sector_size / 512 &&
+ 	    block + this_count == get_capacity(disk))) {
+		this_count -= sdp->sector_size / 512;
+	}
+
 	SCSI_LOG_HLQUEUE(2, scmd_printk(KERN_INFO, SCpnt, "block=%llu\n",
 					(unsigned long long)block));
 

This could have done with running through checkpatch.pl:

ERROR: trailing whitespace
#28: FILE: drivers/scsi/sd.c:398:
+^I/* $

ERROR: trailing whitespace
#32: FILE: drivers/scsi/sd.c:402:
+^Iif (unlikely(sdp->last_sector_bug && $

WARNING: braces {} are not necessary for single statement blocks #32: FILE: drivers/scsi/sd.c:402:
+ if (unlikely(sdp->last_sector_bug &&
+ rq->nr_sectors > sdp->sector_size / 512 &&
+ block + this_count == get_capacity(disk))) {
+ this_count -= sdp->sector_size / 512;
+ }

ERROR: use tabs not spaces
#34: FILE: drivers/scsi/sd.c:404:
+ ^I block + this_count == get_capacity(disk))) {$

WARNING: line over 80 characters
#49: FILE: include/scsi/scsi_device.h:142:
+ unsigned last_sector_bug:1; /* Always read last sector in a
1 sector read */

total: 3 errors, 2 warnings, 23 lines checked

I've fixed all of these up.

James

Wait, we already handle this in the usb-storage driver, why are you putting this in the scsi core now?

confused,

greg k-h

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

No we don't, I've send patches for this to the usb-storage driver but they were refused as they modified scsi commands in flight, the consensus was that this should be done in the scsi layer, hence this patch.

I've noticed, esp. with regards to your second mail to which I will reply next. There has been a lot of discussion about this ending in this (perfectly fine) patch, which is much better then my original hack if I may add that.

Regards,

Hans

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel