On a device that negotiates asynchronous, this patch causes a WDTR exchange on every command (not just inquiry/request sense), even on devices that don't advertise wide support in the inquiry data. This is unnecessary overhead, and may (in theory) cause problems for old devices that don't handle WDTR correctly. The original code avoided this by setting nego = 0 in sym_prepare_nego() if the current agreement matched the goal. However, it was pointed out that the original code was buggy because the sym_sir_bad_scsi_status()/S_CHECK_COND branch does not start a new negotiation (to me it looks like the original code actually does work if the negotiation will be PPR but not if the negotiation will be WDTR/SDTR). This patch forces either WDTR or PPR on every inquiry and request sense. I think it would be better if the negotiation was skipped if the current agreement and the goal are both narrow/asynchronous; that way we don't try to negotiate at all for devices that don't support wide or sync. If the current agreement is 16-bit wide with a nonzero sync offset, and only the sync parameters need to be renegotiated (e.g. during DV), the original code will use just SDTR without WDTR, and this patch will use WDTR followed by SDTR. I agree that this is probably a good change. However, if the current agreement is 8-bit narrow and only the sync parameters need to be renegotiated, then I think it would be better to use SDTR only. That way we don't rely on WDTR working in order to use SDTR successfully. To summarize, I think the following would be the best policy: on every inquiry and request sense: 1) if both the current and goal are narrow/async, then don't negotiate 2) if the current and goal are both 8-bit narrow but with nonzero offset, then use SDTR only 3) if either the current or goal are 16-bit wide, then use WDTR+SDTR or PPR on commands other than inquiry and request sense: 4) if the current and goal are the same, then don't negotiate 5) if the current and goal are both 8-bit narrow but with nonzero offset, then use SDTR only 6) if either the current or goal are 16-bit wide, then use WDTR+SDTR or PPR If you tell sym_prepare_nego() whether or not the command is inquiry or request sense, then it should be fairly straightforward to implement this policy. Tony -- To unsubscribe from this list: 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
