sym53c8xx: fix setflag user command to control disconnects

This patch fixes the sym53c8xx "setflag" user command to control disconnect privilege, which has been broken for a long time.

---

NOTE regarding the following change:

can_disconnect = (cp->tag != NO_TAG) ||
- (lp && (lp->curr_flags & SYM_DISC_ENABLED));
+ (lp && (tp->usrflags & SYM_DISC_ENABLED));

In 2.4 kernels, lp == NULL when scanning for devices, and allowing disconnect when lp == NULL would confuse the code that handles reselection. So with 2.4 kernels, the check for lp != NULL had to be left in even if lp wasn't being dereferenced. In 2.6 kernels, lp != NULL when scanning for devices. In fact, I didn't encounter any cases where lp == NULL during my testing with 2.6 kernels, so the check for lp != NULL may be superfluous now. However, the same check is performed in other places in the same function, so I left it in to be safe.

diff -urpN linux-2.6.24-rc2/drivers/scsi/sym53c8xx_2/sym_glue.c linux-2.6.24-rc2-sym2/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -785,11 +785,6 @@ static int sym53c8xx_slave_configure(str
 	int reqtags, depth_to_use;
 
 	/*
-	 *  Get user flags.
-	 */
-	lp->curr_flags = lp->user_flags;
-
-	/*
 	 *  Select queue depth from driver setup.
 	 *  Donnot use more than configured by user.
 	 *  Use at least 2.

@@ -937,7 +932,9 @@ static void sym_exec_user_command (struc
 				OUTB(np, nc_istat, SIGP|SEM);
 				break;
 			case UC_SETFLAG:
-				tp->usrflags = uc->data;
+				tp->usrflags =
+					(tp->usrflags & ~SYM_DISC_ENABLED) |
+					uc->data;
 				break;
 			}
 		}

@@ -1098,6 +1095,7 @@ printk("sym_user_command: data=%ld\n", u
 		break;
 #endif /* SYM_LINUX_DEBUG_CONTROL_SUPPORT */
 	case UC_SETFLAG:
+		uc->data = SYM_DISC_ENABLED;
 		while (len > 0) {
 			SKIP_SPACES(ptr, len);
 			if	((arg_len = is_keyword(ptr, len, "no_disc")))

diff -urpN linux-2.6.24-rc2/drivers/scsi/sym53c8xx_2/sym_hipd.c linux-2.6.24-rc2-sym2/drivers/scsi/sym53c8xx_2/sym_hipd.c
@@ -4984,11 +4984,6 @@ struct sym_lcb *sym_alloc_lcb (struct sy
 	 */
 	lp->head.resel_sa = cpu_to_scr(SCRIPTB_BA(np, resel_bad_lun));
 
-	/*
-	 *  Set user capabilities.
-	 */
-	lp->user_flags = tp->usrflags & (SYM_DISC_ENABLED | SYM_TAGS_ENABLED);
-
 #ifdef SYM_OPT_HANDLE_DEVICE_QUEUEING
 	/*
 	 *  Initialize device queueing.

@@ -5077,7 +5072,7 @@ int sym_queue_scsiio(struct sym_hcb *np,
 	lp = sym_lp(tp, sdev->lun);
 
 	can_disconnect = (cp->tag != NO_TAG) ||
-		(lp && (lp->curr_flags & SYM_DISC_ENABLED));
+		(lp && (tp->usrflags & SYM_DISC_ENABLED));
 
 	msgptr = cp->scsi_smsg;
 	msglen = 0;

diff -urpN linux-2.6.24-rc2/drivers/scsi/sym53c8xx_2/sym_hipd.h linux-2.6.24-rc2-sym2/drivers/scsi/sym53c8xx_2/sym_hipd.h
@@ -536,12 +536,6 @@ struct sym_lcb {
 	 *  Set when we want to clear all tasks.
 	 */
 	u_char to_clear;
-
-	/*
-	 *  Capabilities.
-	 */
-	u_char	user_flags;
-	u_char	curr_flags;
 };
 
 /*

The first observation is that the sym specific setflags interface is being replaced by the parallel transport specific sysfs interface. However, as you've probably noticed, the SPI transport has no flag for disconnect permission, so how important actually is this? If it's important, then we can put it in scsi_transport_spi ... if not, then we should probably just eliminate it from sym2.

James

Most people would probably just use the HBA's BIOS utility to configure the NVRAM to disallow disconnects if needed. However, in my case, the HBA is inside an embedded device sold by my company, and customers can attach their own SCSI devices to it. There is one type of SCSI device that we support which doesn't work with disconnect privilege enabled, so my code running in the embedded device needs to be able to enable/disable disconnect privilege dynamically based on whether or not the customer has attached this specific broken device.

It would be nice to have a generic way to control it. That would save me the bother of sending my patch to control disconnect privilege for LSI MPT Fusion Ultra320 HBAs via mptctl. ;-)

Tony

Yes, the embedded space is a good enough reason to implement it in the SPI transport class. And I think we should take that as an opportunity to kill the sym2 /proc support or at least the write support.