On Thu, Mar 30, 2006 at 12:53:10PM +0200, Ingo Flaschberger wrote:
> * out_of_space hacks, D. Gilbert (dpg) 990608
> + *
> + * Added Compaq RA4x00 Fiber Channel Array support, 29.03.2006
> + * crossip communications gmbh - Ingo Flaschberger <if@xxxxxx>
> + *
> */
I think you can drop the changelog entry here -- it's not been updated
in many years. Plus, it's not really accurate.
> {"CNSi", "G8324", NULL, BLIST_SPARSELUN}, /* Chaparral G8324 RAID */
> + {"COMPAQ", "ARRAY CONTROLLER", "2.60", BLIST_SPARSELUN | BLIST_LARGELUN |
> + BLIST_MAX_512K}, /* Compaq RA4x00 */
> + {"COMPAQ", "LOGICAL VOLUME", "2.60", BLIST_MAX_512K}, /* Compaq RA4x00 */
> {"COMPAQ", "LOGICAL VOLUME", NULL, BLIST_FORCELUN},
I wonder if you should just be adding BLIST_MAX_512K to the current
entry for LOGICAL VOLUME instead?
> * or a LUN is seen that cannot have a device attached to it.
> + *
> + * Modification history:
> + * - Added Compaq RA4x00 Fiber Channel Array support, 29.03.2006
> + * crossip communications gmbh - Ingo Flaschberger <if@xxxxxx>
> + *
> */
Definitely don't start a changelog. That's what SCMs are for.
> @@ -715,6 +721,14 @@
> if (*bflags & BLIST_SELECT_NO_ATN)
> sdev->select_no_atn = 1;
>
> + /*
> + * Maximum 512K cdb transfer length
> + * broken RA4x00 Compaq Disk Array
> + */
> + if (*bflags & BLIST_MAX_512K) {
> + sdev->max_512k = 1;
> + }
> +
Indent with tabs, not spaces. Also, sdev->max_512k is a bit ...
specific. How about adding an unsigned short to struct scsi_device
(right above queue_depth, so it fits in the padding) called something
like max_xfer_len? Then you can do something like:
sdev->max_xfer_len = 0xffff;
if (*bflags & BLIST_MAX_512K)
sdev->max_xfer_len = 0x200;
and later:
> @@ -344,8 +346,14 @@
> SCpnt->cmnd[14] = SCpnt->cmnd[15] = 0;
> } else if ((this_count > 0xff) || (block > 0x1fffff) ||
> SCpnt->device->use_10_for_rw) {
> - if (this_count > 0xffff)
> - this_count = 0xffff;
> + if (SCpnt->device->max_512k) {
> + if (this_count > 0x200) {
> + this_count = 0x200;
> + }
> + } else {
> + if (this_count > 0xffff)
> + this_count = 0xffff;
> + }
>
> SCpnt->cmnd[0] += READ_10 - READ_6;
> SCpnt->cmnd[1] |= blk_fua_rq(rq) ? 0x8 : 0;
simplifies to:
if (this_count > SCpnt->device->max_xfer_len)
this_count = SCpnt->device->max_xfer_len;
> @@ -1089,9 +1103,12 @@
> * support more than 8 LUNs.
> */
> if ((bflags & BLIST_NOREPORTLUN) ||
> - starget->scsi_level < SCSI_2 ||
> - (starget->scsi_level < SCSI_3 &&
> - (!(bflags & BLIST_REPORTLUN2) || shost->max_lun <= 8)) )
> + ((starget->scsi_level < SCSI_2) &&
> + (starget->scsi_level != SCSI_UNKNOWN)) ||
> + (starget->scsi_level < SCSI_3 &&
> + ((!(bflags & BLIST_REPORTLUN2) &&
> + (starget->scsi_level != SCSI_UNKNOWN)) ||
> + shost->max_lun <= 8)) )
> return 1;
That conditional makes my brain hurt. Can we split it up?
if ((bflags & BLIST_NOREPORTLUN)
return 1;
if ((starget->scsi_level < SCSI_2) &&
(starget->scsi_level != SCSI_UNKNOWN))
return 1;
if ((starget->scsi_level < SCSI_3) &&
((!(bflags & BLIST_REPORTLUN2) &&
(starget->scsi_level != SCSI_UNKNOWN) ||
shost->max_lun <= 8)))
return 1;
I'm not sure that's what you meant to do, even. How about more simply:
/*
* Only support SCSI-3 and up devices if BLIST_NOREPORTLUN is not set.
* Also allow SCSI-2 and unknown devices if BLIST_REPORTLUN2 is set
* and host adapter supports more than 8 LUNs.
*/
if ((bflags & BLIST_NOREPORTLUN)
return 1;
if ((starget->scsi_level < SCSI_2) &&
(starget->scsi_level != SCSI_UNKNOWN))
return 1;
if ((starget->scsi_level < SCSI_3) &&
(!(bflags & BLIST_REPORTLUN2) || shost->max_lun <= 8))
return 1;
-
: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
