public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] [USB] UASP: USB Attached SCSI (UAS) protocol driver
@ 2010-12-06 17:05 Luben Tuikov
  2010-12-06 17:09 ` Greg KH
  0 siblings, 1 reply; 23+ messages in thread
From: Luben Tuikov @ 2010-12-06 17:05 UTC (permalink / raw)
  To: Greg KH, linux-kernel, linux-usb

This driver allows you to connect to UAS devices
and use them as SCSI devices.

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
---
 * The name "uas" was taken, so I used "uasp".
 * The idea is to have this in Greg's tree, everyone submits
 patches to this CC list, it gets ACKed and goes right into
 Greg's tree.
 * Before submitting patches, please test _both_ High-Speed and
 SuperSpeed mode.
 * Vendors whose solution is still in development wanting to
 utilize this driver, can use dynamic ids to test their
 implementation, although there is no reason to not have the
 correct descriptors.
 * Vendors: I don't know /when/ this will make it into the kernel.
 CC this thread when asking.
 drivers/usb/storage/Kconfig  |   19 +
 drivers/usb/storage/Makefile |    5 +
 drivers/usb/storage/uasp.c   | 1117 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1141 insertions(+), 0 deletions(-)
 create mode 100644 drivers/usb/storage/uasp.c

diff --git a/drivers/usb/storage/Kconfig b/drivers/usb/storage/Kconfig
index eb9f6af..1f58cd0 100644
--- a/drivers/usb/storage/Kconfig
+++ b/drivers/usb/storage/Kconfig
@@ -195,6 +195,25 @@ config USB_UAS_DEBUG
 
 	 If unsure, say 'N'.
 
+config USB_UASP
+       tristate "USB Attached SCSI (UAS) protocol driver (uasp)"
+       depends on USB && SCSI
+       help
+	USB Attached SCSI (UAS) protocol driver. This driver allows you
+	to connect to UAS devices and use them as SCSI devices.
+
+	If unusure say 'Y' or 'M'.
+
+	If compiled as a module, this driver will be named uasp.
+
+config USB_UASP_DEBUG
+       bool "Compile the uasp driver in debug mode"
+       depends on USB_UASP
+       help
+	Compiles the uasp driver in debug mode.
+
+	If unsure say 'N'.
+
 config USB_LIBUSUAL
 	bool "The shared table of common (or usual) storage devices"
 	depends on USB
diff --git a/drivers/usb/storage/Makefile b/drivers/usb/storage/Makefile
index 16715ab..d148bdc 100644
--- a/drivers/usb/storage/Makefile
+++ b/drivers/usb/storage/Makefile
@@ -8,12 +8,17 @@
 ccflags-y := -Idrivers/scsi
 
 obj-$(CONFIG_USB_UAS)		+= uas.o
+obj-$(CONFIG_USB_UASP)		+= uasp.o
 obj-$(CONFIG_USB_STORAGE)	+= usb-storage.o
 
 ifeq ($(CONFIG_USB_UAS_DEBUG),y)
 	EXTRA_CFLAGS += -DUAS_DEBUG
 endif
 
+ifeq ($(CONFIG_USB_UASP_DEBUG),y)
+	EXTRA_CFLAGS += -DUASP_DEBUG
+endif
+
 usb-storage-y := scsiglue.o protocol.o transport.o usb.o
 usb-storage-y += initializers.o sierra_ms.o option_ms.o
 
diff --git a/drivers/usb/storage/uasp.c b/drivers/usb/storage/uasp.c
new file mode 100644
index 0000000..1d2b2e3
--- /dev/null
+++ b/drivers/usb/storage/uasp.c
@@ -0,0 +1,1117 @@
+/*
+ * USB Attached SCSI (UAS) protocol driver
+ * Copyright Luben Tuikov, 2010
+ *
+ * This driver allows you to connect to UAS devices and use them as
+ * SCSI devices.
+ *
+ * Distributed under the terms of the GNU GPL version 2.
+ */
+
+#ifdef UASP_DEBUG
+#if !defined(DEBUG) && !defined(CONFIG_DYNAMIC_DEBUG)
+#define DEBUG
+#endif
+#endif /* UASP_DEBUG */
+
+#define DRIVER_NAME "uasp"
+
+#include <linux/blkdev.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/usb.h>
+#include <linux/usb/storage.h>
+#include <linux/completion.h>
+
+#include <scsi/scsi.h>
+#include <scsi/scsi_dbg.h>
+#include <scsi/scsi_cmnd.h>
+#include <scsi/scsi_device.h>
+#include <scsi/scsi_host.h>
+#include <scsi/scsi_tcq.h>
+
+/* Information unit types
+ */
+#define IU_CMD   1
+#define IU_SENSE 3
+#define IU_RESP  4
+#define IU_TMF   5
+#define IU_RRDY  6
+#define IU_WRDY  7
+
+#define IU_CMD_LEN	32
+#define IU_SENSE_LEN	16
+#define IU_RESP_LEN	8
+#define IU_TMF_LEN	16
+#define IU_RRDY_LEN	4
+#define IU_WRDY_LEN	4
+
+#define MAX_SENSE_DATA_LEN 252
+
+#define STAT_IU_LEN	(IU_SENSE_LEN + MAX_SENSE_DATA_LEN)
+
+#define GET_IU_ID(__Ptr)	((__Ptr)[0])
+#define GET_IU_TAG(__Ptr)	((__Ptr)[2] << 8 | (__Ptr)[3])
+
+/* SENSE IU
+ */
+#define GET_IU_STATQUAL(__Ptr)	((__Ptr)[4] << 8 | (__Ptr)[5])
+#define GET_IU_STATUS(__Ptr)	((__Ptr)[6])
+#define GET_IU_LENGTH(__Ptr)	((__Ptr)[14] << 8 | (__Ptr)[15])
+
+/* RESPONSE IU
+ */
+#define GET_IU_RESPONSE(__Ptr)	((__Ptr)[4] << 24 | (__Ptr)[5] << 16 | \
+				 (__Ptr)[6] << 8 | (__Ptr)[7])
+
+/* Task management
+ */
+#define TMF_ABORT_TASK		1
+#define TMF_ABORT_TASK_SET	2
+#define TMF_CLEAR_TASK_SET	4
+#define TMF_LU_RESET		8
+#define TMF_IT_NEXUS_RESET	0x10
+#define TMF_CLEAR_ACA		0x40
+#define TMF_QUERY_TASK		0x80
+#define TMF_QUERY_TASK_SET	0x81
+#define TMF_QUERY_ASYNC_EVENT   0x82
+
+#define TMR_RESPONSE_CODE_MASK	0xFF
+#define TMR_RESPONSE_CODE_SHIFT 0
+#define TMR_RESPONSE_INFO_MASK  0xFFFFFF00
+#define TMR_RESPONSE_INFO_SHIFT 8
+
+#define TMR_RESPONSE_CODE(__Val)					\
+	(((__Val) & TMR_RESPONSE_CODE_MASK) >> TMR_RESPONSE_CODE_SHIFT)
+
+#define TMR_RESPONSE_INFO(__Val)					\
+	(((__Val) & TMR_RESPONSE_INFO_MASK) >> TMR_RESPONSE_INFO_SHIFT)
+
+#define TMR_COMPLETE 0
+#define TMR_IIU      2
+#define TMR_UNSUPP   4
+#define TMR_FAILED   5
+#define TMR_SUCC     8
+#define TMR_ILUN     9
+#define TMR_OLAP     0xA
+
+/* Pipe types
+ */
+#define CMD_PIPE_ID     1
+#define STAT_PIPE_ID    2
+#define DATAIN_PIPE_ID  3
+#define DATAOUT_PIPE_ID 4
+
+/* Task attribute
+ */
+#define UASP_TASK_SIMPLE  0
+#define UASP_TASK_HOQ     1
+#define UASP_TASK_ORDERED 2
+#define UASP_TASK_ACA     4
+
+/* Target device port information
+ */
+struct uasp_tport_info {
+	struct usb_interface *iface;
+	struct usb_device *udev;
+	unsigned cmd_pipe, status_pipe, datain_pipe, dataout_pipe;
+	struct usb_host_endpoint *eps[4];
+	unsigned use_streams:1;
+	int max_streams;	  /* streams supported */
+	int num_streams;	  /* usable streams [1, num_streams] */
+	int max_cmds;		  /* max cmds we can queue */
+};
+
+/* Current task management information
+ */
+struct uasp_lu_info {
+	struct completion tmf_completion;
+	unsigned int tmf_resp;
+};
+
+/* Command information
+ */
+struct uasp_cmd_info {
+	int tag;
+	struct urb *cmd_urb;
+	struct urb *status_urb;
+	struct urb *datain_urb;
+	struct urb *dataout_urb;
+};
+
+#define UASP_CMD_INFO(__Scmd) ((struct uasp_cmd_info *)(&(__Scmd)->SCp))
+#define UASP_TPORT_INFO(__Scmd) \
+	((struct uasp_tport_info *)((__Scmd)->device->host->hostdata[0]))
+#define SDEV_TPORT_INFO(__Sdev)  \
+	((struct uasp_tport_info *)((__Sdev)->host->hostdata[0]))
+#define SDEV_LU_INFO(__Sdev) ((struct uasp_lu_info *)((__Sdev)->hostdata))
+
+/* ---------- IU processors ---------- */
+
+static void uasp_sense(struct urb *urb, struct scsi_cmnd *cmd, u16 tag)
+{
+	unsigned char *iu = urb->transfer_buffer;
+	struct scsi_device *sdev = cmd->device;
+
+	cmd->result = GET_IU_STATUS(iu);
+
+	if (urb->actual_length > IU_SENSE_LEN) {
+		unsigned slen = GET_IU_LENGTH(iu);
+
+		if (urb->actual_length >= IU_SENSE_LEN + slen) {
+			slen = min(slen, (unsigned) SCSI_SENSE_BUFFERSIZE);
+		} else {
+			unsigned dlen = slen;
+
+			slen = min(urb->actual_length - IU_SENSE_LEN,
+				   (unsigned) SCSI_SENSE_BUFFERSIZE);
+
+			dev_err(&SDEV_TPORT_INFO(sdev)->udev->dev,
+				"%s: short SENSE IU by %d bytes, "
+				"using %d/%d bytes of sense data\n",
+				__func__,
+				IU_SENSE_LEN + slen - urb->actual_length,
+				slen, dlen);
+		}
+		memcpy(cmd->sense_buffer, iu + IU_SENSE_LEN, slen);
+	}
+
+	if (tag == 1)
+		sdev->current_cmnd = NULL;
+	cmd->scsi_done(cmd);
+	usb_free_urb(urb);
+}
+
+/* High-Speed devices only
+ */
+static void uasp_xfer_data(struct urb *urb, struct scsi_cmnd *cmd,
+			   struct uasp_tport_info *tpinfo,
+			   enum dma_data_direction dir, int tag)
+{
+	struct uasp_cmd_info *cmdinfo = UASP_CMD_INFO(cmd);
+	int res = 0;
+
+	res = usb_submit_urb(urb, GFP_ATOMIC);
+
+	if (res == 0) {
+		if (dir == DMA_FROM_DEVICE)
+			res = usb_submit_urb(cmdinfo->datain_urb, GFP_ATOMIC);
+		else
+			res = usb_submit_urb(cmdinfo->dataout_urb,GFP_ATOMIC);
+	}
+
+	dev_dbg(&tpinfo->udev->dev, "%s: cmd:%p tag:%d res:%d\n",
+		__func__, cmd, cmdinfo->tag, res);
+}
+
+#ifdef UASP_DEBUG
+
+static const char *id_to_str[] = {
+	[0 ... 0xFF] = "Unknown",
+	[1] = "Command",
+	[3] = "Sense",
+	[4] = "Response",
+	[5] = "Task Management",
+	[6] = "RRDY",
+	[7] = "WRDY",
+};
+
+#endif /* UASP_DEBUG */
+
+/**
+ * uasp_stat_cmplt -- Status pipe urb completion
+ * @urb: the URB that completed
+ *
+ * Anything we expect to come back on the status pipe
+ * comes here.
+ */
+static void uasp_stat_done(struct urb *urb)
+{
+	unsigned char *iu = urb->transfer_buffer;
+	struct scsi_device *sdev = urb->context;
+	struct uasp_tport_info *tpinfo = SDEV_TPORT_INFO(sdev);
+	struct scsi_cmnd *cmd = NULL;
+	int id = GET_IU_ID(iu);
+	int tag =  GET_IU_TAG(iu);
+
+	dev_dbg(&urb->dev->dev, "%s: %s IU (0x%02x) tag:%d urb:%p\n",
+		__func__, id_to_str[id], id, tag, urb);
+
+	if (urb->status) {
+		dev_err(&urb->dev->dev, "%s: URB BAD STATUS %d\n",
+			__func__, urb->status);
+		usb_free_urb(urb);
+		return;
+	}
+
+	if (id == IU_RESP) {
+		struct uasp_lu_info *luinfo = SDEV_LU_INFO(sdev);
+		unsigned char *riu = urb->transfer_buffer;
+
+		luinfo->tmf_resp = GET_IU_RESPONSE(riu);
+		complete(&luinfo->tmf_completion);
+		usb_free_urb(urb);
+		return;
+	}
+
+	if (tag == 1)
+		cmd = sdev->current_cmnd;
+	else if (tag > 1)
+		cmd = scsi_find_tag(sdev, tag-2);
+
+	if (cmd == NULL)
+		goto Out_no_cmd;
+	
+	switch (id) {
+	case IU_SENSE:
+		uasp_sense(urb, cmd, tag);
+		break;
+	case IU_RRDY:
+		uasp_xfer_data(urb, cmd, tpinfo, DMA_FROM_DEVICE, tag);
+		break;
+	case IU_WRDY:
+		uasp_xfer_data(urb, cmd, tpinfo, DMA_TO_DEVICE, tag);
+		break;
+	default:
+		usb_free_urb(urb);
+		break;
+	}
+
+	return;
+
+ Out_no_cmd:
+	dev_dbg(&urb->dev->dev, "%s: No command!?\n", __func__);
+	usb_free_urb(urb);
+}
+
+static void uasp_data_done(struct urb *urb)
+{
+	struct scsi_data_buffer *sdb = urb->context;
+
+	dev_dbg(&urb->dev->dev, "%s: urb:%p\n", __func__, urb);
+
+	if (urb->status == 0)
+		sdb->resid = sdb->length - urb->actual_length;
+	else
+		dev_err(&urb->dev->dev, "%s: URB BAD STATUS %d\n",
+			__func__, urb->status);
+
+	usb_free_urb(urb);
+}
+
+/* ---------- URB allocators and submission ---------- */
+
+/**
+ * uasp_fill_cmdp_urb -- Fill in a command pipe urb
+ * @urb: the urb
+ * @tpinfo: the UAS device info struct
+ * @transfer_buffer: the IU
+ * @buffer_length: the size of the IU
+ *
+ * A unified place to initialize a command pipe urb so that
+ * all command pipe urbs are freed in the same manner.
+ */
+static void uasp_fill_cmdp_urb(struct urb *urb, struct uasp_tport_info *tpinfo,
+			       void *transfer_buffer, int buffer_length)
+{
+	usb_fill_bulk_urb(urb, tpinfo->udev, tpinfo->cmd_pipe,
+			  transfer_buffer, buffer_length,
+			  usb_free_urb, NULL);
+	urb->transfer_flags |= URB_FREE_BUFFER;
+}
+
+/**
+ * uasp_fill_statp_urb -- Fill in a status pipe urb
+ * @urb: the urb
+ * @dev: the scsi device
+ * @transfer_buffer: the transfer buffer of size STAT_IU_LEN
+ * @tag: the tag
+ *
+ * The callback is responsible to free/recycle the urb and the
+ * transfer buffer.
+ */
+static void uasp_fill_statp_urb(struct urb *urb, struct scsi_device *sdev,
+				void *transfer_buffer, int tag)
+{
+	struct uasp_tport_info *tpinfo = SDEV_TPORT_INFO(sdev);
+
+	usb_fill_bulk_urb(urb, tpinfo->udev, tpinfo->status_pipe,
+			  transfer_buffer, STAT_IU_LEN,
+			  uasp_stat_done, sdev);
+	urb->transfer_flags |= URB_FREE_BUFFER;
+	if (tpinfo->use_streams)
+		urb->stream_id = tag;
+}
+
+static struct urb *uasp_alloc_data_urb(struct uasp_tport_info *tpinfo,
+				       gfp_t gfp, unsigned int data_pipe,
+				       u16 tag, struct scsi_data_buffer *sdb)
+{
+	struct usb_device *udev = tpinfo->udev;
+	struct urb *urb;
+
+	urb = usb_alloc_urb(0, gfp);
+	if (urb == NULL)
+		goto Out;
+
+	usb_fill_bulk_urb(urb, udev, data_pipe, NULL, sdb->length,
+			  uasp_data_done, sdb);
+	if (tpinfo->use_streams)
+		urb->stream_id = tag;
+	urb->num_sgs = udev->bus->sg_tablesize ? sdb->table.nents : 0;
+	urb->sg = sdb->table.sgl;
+ Out:
+	return urb;
+}
+
+static struct urb *uasp_alloc_status_urb(struct scsi_cmnd *cmd, gfp_t gfp)
+{
+	unsigned char *siu;
+	struct urb *urb;
+
+	urb = usb_alloc_urb(0, gfp);
+	if (urb == NULL)
+		return NULL;
+
+	siu = kzalloc(STAT_IU_LEN, gfp);
+	if (siu == NULL)
+		goto Out_free;
+
+	uasp_fill_statp_urb(urb, cmd->device, siu, UASP_CMD_INFO(cmd)->tag);
+
+	return urb;
+ Out_free:
+	usb_free_urb(urb);
+	return NULL;
+}
+
+static struct urb *uasp_alloc_cmd_urb(struct scsi_cmnd *cmd, gfp_t gfp)
+{
+	struct uasp_cmd_info *cmdinfo = UASP_CMD_INFO(cmd);
+	struct uasp_tport_info *tpinfo = UASP_TPORT_INFO(cmd);
+	struct usb_device *udev = tpinfo->udev;
+	struct scsi_device *sdev = cmd->device;
+	struct urb *urb;
+	struct scsi_lun slun;
+	unsigned char *ciu;
+	int len;
+
+	urb = usb_alloc_urb(0, gfp);
+	if (urb == NULL)
+		return NULL;
+
+	len = cmd->cmd_len + 16;
+	if (len < IU_CMD_LEN)
+		len = IU_CMD_LEN;
+	else if (len > IU_CMD_LEN)
+		len = ALIGN(len, 4);
+
+	ciu = kzalloc(len, gfp);
+	if (ciu == NULL)
+		goto Free;
+
+	ciu[0] = IU_CMD;
+	ciu[2] = cmdinfo->tag >> 8;
+	ciu[3] = cmdinfo->tag;
+	if (sdev->ordered_tags && cmd->request->cmd_flags & REQ_HARDBARRIER)
+		ciu[4] = UASP_TASK_ORDERED;
+	ciu[6] = len - IU_CMD_LEN;
+	int_to_scsilun(sdev->lun, &slun);
+	memcpy(&ciu[8], slun.scsi_lun, 8);
+	memcpy(&ciu[16], cmd->cmnd, cmd->cmd_len);
+
+	usb_fill_bulk_urb(urb, udev, tpinfo->cmd_pipe, ciu, len, usb_free_urb,
+			  NULL);
+	urb->transfer_flags |= URB_FREE_BUFFER;
+	return urb;
+ Free:
+	usb_free_urb(urb);
+	return NULL;
+}
+
+static int uasp_alloc_urbs(struct scsi_cmnd *cmd, gfp_t gfp)
+{
+	struct uasp_cmd_info *cmdinfo = UASP_CMD_INFO(cmd);
+	struct uasp_tport_info *tpinfo = UASP_TPORT_INFO(cmd);
+
+	cmdinfo->status_urb = NULL;
+	cmdinfo->datain_urb = NULL;
+	cmdinfo->dataout_urb = NULL;
+	cmdinfo->cmd_urb = NULL;
+
+	cmdinfo->status_urb = uasp_alloc_status_urb(cmd, gfp);
+	cmdinfo->cmd_urb = uasp_alloc_cmd_urb(cmd, gfp);
+	if (cmdinfo->cmd_urb == NULL || cmdinfo->status_urb == NULL)
+		goto Out_err1;
+
+	switch (cmd->sc_data_direction) {
+	case DMA_BIDIRECTIONAL:
+	case DMA_TO_DEVICE:
+		cmdinfo->dataout_urb =
+			uasp_alloc_data_urb(tpinfo, gfp,
+					    tpinfo->dataout_pipe,
+					    cmdinfo->tag,
+					    scsi_out(cmd));
+		if (cmdinfo->dataout_urb == NULL)
+			goto Out_err2;
+		if (cmd->sc_data_direction != DMA_BIDIRECTIONAL)
+			break;
+		else {
+	case DMA_FROM_DEVICE:
+		cmdinfo->datain_urb =
+			uasp_alloc_data_urb(tpinfo, gfp,
+					    tpinfo->datain_pipe,
+					    cmdinfo->tag,
+					    scsi_in(cmd));
+		if (cmdinfo->datain_urb == NULL)
+			goto Out_err2;
+		}
+		break;
+	case DMA_NONE:
+		break;
+	}
+
+	return 0;
+	
+ Out_err2:
+	usb_free_urb(cmdinfo->datain_urb);
+	usb_free_urb(cmdinfo->dataout_urb);
+ Out_err1:
+	usb_free_urb(cmdinfo->cmd_urb);
+	usb_free_urb(cmdinfo->status_urb);
+	return -ENOMEM;
+}
+
+static int uasp_submit_urbs(struct scsi_cmnd *cmd, gfp_t gfp)
+{
+	struct uasp_cmd_info *cmdinfo = UASP_CMD_INFO(cmd);
+	struct uasp_tport_info *tpinfo = UASP_TPORT_INFO(cmd);
+	int res;
+
+	dev_dbg(&tpinfo->udev->dev, "%s: cmd:%p (0x%02x) tag:%d\n",
+		__func__, cmd, cmd->cmnd[0], cmdinfo->tag);
+
+	res = usb_submit_urb(cmdinfo->status_urb, gfp);
+	if (res) {
+		dev_err(&tpinfo->udev->dev,
+			"error submitting status urb (%d)\n", res);
+		return res;
+	}
+
+	if (cmdinfo->datain_urb && tpinfo->use_streams) {
+		res = usb_submit_urb(cmdinfo->datain_urb, gfp);
+		if (res) {
+			dev_err(&tpinfo->udev->dev,
+				"error submitting datain urb (%d)\n", res);
+			return res;
+		}
+	}
+
+	if (cmdinfo->dataout_urb && tpinfo->use_streams) {
+		res = usb_submit_urb(cmdinfo->dataout_urb, gfp);
+		if (res) {
+			dev_err(&tpinfo->udev->dev,
+				"error submitting dataout urb (%d)\n", res);
+			return res;
+		}
+	}
+
+	res = usb_submit_urb(cmdinfo->cmd_urb, gfp);
+	if (res) {
+		dev_err(&tpinfo->udev->dev,
+			"error submitting cmd urb (%d)\n", res);
+		return res;
+	}
+
+	return 0;
+}
+
+static void uasp_free_urbs(struct scsi_cmnd *cmd)
+{
+	struct uasp_cmd_info *cmdinfo = UASP_CMD_INFO(cmd);
+	int res;
+
+	if (cmdinfo->cmd_urb) {
+		res = usb_unlink_urb(cmdinfo->cmd_urb);
+		if (res != -EINPROGRESS)
+			usb_free_urb(cmdinfo->cmd_urb);
+	}
+	if (cmdinfo->status_urb) {
+		res = usb_unlink_urb(cmdinfo->status_urb);
+		if (res != -EINPROGRESS)
+			usb_free_urb(cmdinfo->status_urb);
+	}
+	if (cmdinfo->datain_urb) {
+		res = usb_unlink_urb(cmdinfo->datain_urb);
+		if (res != -EINPROGRESS)
+			usb_free_urb(cmdinfo->datain_urb);
+	}
+	if (cmdinfo->dataout_urb) {
+		res = usb_unlink_urb(cmdinfo->dataout_urb);
+		if (res != -EINPROGRESS)
+			usb_free_urb(cmdinfo->dataout_urb);
+	}
+}
+
+static int uasp_queuecommand(struct scsi_cmnd *cmd,
+			    void (*done)(struct scsi_cmnd *))
+{
+	struct scsi_device *sdev = cmd->device;
+	struct uasp_cmd_info *cmdinfo = UASP_CMD_INFO(cmd);
+	int res;
+
+	BUILD_BUG_ON(sizeof(struct uasp_cmd_info) > sizeof(struct scsi_pointer));
+
+	/* If LLDDs are NOT to maintain their own tags, (but the block
+	 * layer would), THEN ANY AND ALL scsi_cmnds passed to the
+	 * queuecommand entry of a LLDD MUST HAVE A VALID,
+	 * REVERSE-MAPPABLE tag, REGARDLESS of where the command came
+	 * from, regardless of whether the device supports tags, and
+	 * regardless of how many tags it supports.
+	 */
+	if (blk_rq_tagged(cmd->request)) {
+		cmdinfo->tag = cmd->request->tag + 2;
+	} else if (sdev->current_cmnd == NULL) {
+		sdev->current_cmnd = cmd;
+		cmdinfo->tag = 1;
+	} else {
+		cmd->result = DID_ABORT << 16;
+		goto Out_err;
+	}
+
+	cmd->scsi_done = done;
+
+	res = uasp_alloc_urbs(cmd, GFP_ATOMIC);
+	if (res) {
+		cmd->result = DID_ABORT << 16;
+		goto Out_err_null;
+	}
+
+	res = uasp_submit_urbs(cmd, GFP_ATOMIC);
+	if (res) {
+		cmd->result = DID_NO_CONNECT << 16;
+		uasp_free_urbs(cmd);
+		goto Out_err_null;
+	}
+	
+	dev_dbg(&UASP_TPORT_INFO(cmd)->udev->dev,
+		"%s: cmd:%p (0x%02x) tag:%d lun:%d res:%d\n",
+		__func__, cmd, cmd->cmnd[0], cmdinfo->tag,
+		cmd->device->lun, res);
+
+	return 0;
+ Out_err_null:
+	if (sdev->current_cmnd == cmd)
+		sdev->current_cmnd = NULL;
+ Out_err:
+	done(cmd);
+	return 0;
+}
+
+/* ---------- Error Recovery ---------- */
+
+static int uasp_alloc_tmf_urb(struct urb **urb, struct uasp_tport_info *tpinfo,
+			      u8 tmf, u16 ttbm, unsigned char *lun)
+{
+	unsigned char *tiu;
+	int tag;
+	
+	*urb = usb_alloc_urb(0, GFP_KERNEL);
+	if (*urb == NULL)
+		return -ENOMEM;
+
+	tiu = kzalloc(IU_TMF_LEN, GFP_KERNEL);
+	if (tiu == NULL)
+		goto Out_err;
+
+	/* If LLDDs are NOT to maintain their own tags, (but the block
+	 * layer would), THEN LLDDs must be able to call a function of
+	 * some sort and reserve a tag from the same pool and obtain
+	 * it for their own use, as well as being able to free it back
+	 * later. See the comment in uasp_set_max_cmds().
+	 */
+	tag = tpinfo->max_cmds + 2;
+
+	tiu[0] = IU_TMF;
+	tiu[2] = tag >> 8;
+	tiu[3] = tag;
+	tiu[4] = tmf;
+	tiu[6] = ttbm >> 8;
+	tiu[7] = ttbm;
+	memcpy(&tiu[8], lun, 8);
+
+	uasp_fill_cmdp_urb(*urb, tpinfo, tiu, IU_TMF_LEN);
+
+	return 0;
+ Out_err:
+	usb_free_urb(*urb);
+	return -ENOMEM;
+}
+
+static int uasp_alloc_resp_urb(struct urb **urb, struct scsi_device *sdev,
+			       int tag)
+{
+	unsigned char *resp;
+
+	*urb = usb_alloc_urb(0, GFP_KERNEL);
+	if (*urb == NULL)
+		return -ENOMEM;
+
+	resp = kzalloc(STAT_IU_LEN, GFP_KERNEL);
+	if (resp == NULL)
+		goto Out_free;
+
+	uasp_fill_statp_urb(*urb, sdev, resp, tag);
+
+	return 0;
+ Out_free:
+	usb_free_urb(*urb);
+	return -ENOMEM;
+}
+
+#define UASP_TMF_TIMEOUT	(5*HZ)
+
+/**
+ * uasp_do_tmf -- Execute the desired TMF
+ * @sdev: the device to which we should send the TMF
+ * @tmf:  the task management function to execute
+ * @ttbm: the tag of task to be managed
+ *
+ * This function returns a negative value on error (-ENOMEM, etc), or
+ * an integer with bytes 3, 2 and 1 being the high to low byte of the
+ * Additional Response Information field and byte 0 being the Response
+ * code of the Response IU.
+ * 
+ * If the response code is 0xFF, then the TMF timed out.
+ */
+static int uasp_do_tmf(struct scsi_cmnd *cmd, u8 tmf, u8 ttbm)
+{
+	struct scsi_device *sdev = cmd->device;
+	struct uasp_tport_info *tpinfo = SDEV_TPORT_INFO(sdev);
+	struct uasp_lu_info *luinfo = SDEV_LU_INFO(sdev);
+	struct urb *tmf_urb = NULL;
+	struct urb *resp_urb = NULL;
+	unsigned char *tiu;
+	struct scsi_lun slun;
+	int    res;
+
+	/* scsi_dev should contain u8[8] for a LUN, not an unsigned int!
+	 */
+	int_to_scsilun(sdev->lun, &slun);
+	res = uasp_alloc_tmf_urb(&tmf_urb, tpinfo, tmf, ttbm, slun.scsi_lun);
+	if (res)
+		return -ENOMEM;
+
+	tiu = tmf_urb->transfer_buffer;
+	res = uasp_alloc_resp_urb(&resp_urb, sdev, GET_IU_TAG(tiu));
+	if (res) {
+		usb_free_urb(tmf_urb);
+		return -ENOMEM;
+	}
+
+	init_completion(&luinfo->tmf_completion);
+	luinfo->tmf_resp = 0xFFFFFFFF;
+
+	res = usb_submit_urb(resp_urb, GFP_KERNEL);
+	if (res) {
+		complete(&luinfo->tmf_completion);
+		usb_free_urb(tmf_urb);
+		res = usb_unlink_urb(resp_urb);
+		if (res != -EINPROGRESS)
+			usb_free_urb(resp_urb);
+		return res;
+	}
+
+	res = usb_submit_urb(tmf_urb, GFP_KERNEL);
+	if (res) {
+		complete(&luinfo->tmf_completion);
+		res = usb_unlink_urb(tmf_urb);
+		if (res != -EINPROGRESS)
+			usb_free_urb(tmf_urb);
+		res = usb_unlink_urb(resp_urb);
+		if (res != -EINPROGRESS)
+			usb_free_urb(resp_urb);
+		return res;
+	}
+
+	wait_for_completion_timeout(&luinfo->tmf_completion, UASP_TMF_TIMEOUT);
+
+	if (luinfo->tmf_resp != 0xFFFFFFFF) {
+		res = luinfo->tmf_resp;
+	} else {
+		res = 0xFF;
+	}
+
+	return res;
+}
+
+static int uasp_er_tmf(struct scsi_cmnd *cmd, u8 tmf)
+{
+	struct scsi_device *sdev = cmd->device;
+	int tag;
+	int res;
+
+	if (sdev->current_cmnd == cmd)
+		tag = 1;
+	else
+		tag = cmd->request->tag + 2;
+
+	res = uasp_do_tmf(cmd, tmf, tag);
+
+	dev_dbg(&SDEV_TPORT_INFO(sdev)->udev->dev,
+		"%s: cmd:%p (0x%02x) tag:%d tmf:0x%02x resp:0x%08x\n",
+		__func__, cmd, cmd->cmnd[0], tag, tmf, res);
+
+	switch (TMR_RESPONSE_CODE(res)) {
+	case TMR_COMPLETE:
+	case TMR_SUCC:
+		return SUCCESS;
+	default:
+		return FAILED;
+	}
+}
+
+static int uasp_abort_cmd(struct scsi_cmnd *cmd)
+{
+	return uasp_er_tmf(cmd, TMF_ABORT_TASK);
+}
+
+static int uasp_device_reset(struct scsi_cmnd *cmd)
+{
+	return uasp_er_tmf(cmd, TMF_LU_RESET);
+}
+
+static int uasp_target_reset(struct scsi_cmnd *cmd)
+{
+	return uasp_er_tmf(cmd, TMF_IT_NEXUS_RESET);
+}
+
+static int uasp_bus_reset(struct scsi_cmnd *cmd)
+{
+	struct scsi_device *sdev = cmd->device;
+	struct uasp_tport_info *tpinfo = SDEV_TPORT_INFO(sdev);
+	struct usb_device *udev = tpinfo->udev;
+	int res;
+
+	res = usb_lock_device_for_reset(udev, tpinfo->iface);
+	if (res == 0) {
+		res = usb_reset_device(udev);
+	} else {
+		dev_err(&udev->dev, "%s: cmd:%p (0x%02x) failed to "
+			"lock dev (%d)\n",
+			__func__, cmd, cmd->cmnd[0], res);
+		return FAILED;
+	}
+
+	dev_dbg(&udev->dev, "%s: cmd:%p (0x%02x) (%d)\n",
+		__func__, cmd, cmd->cmnd[0], res);
+
+	if (res == 0)
+		return SUCCESS;
+
+	return FAILED;
+}
+
+/* ---------- SCSI Host support ---------- */
+
+static int uasp_slave_alloc(struct scsi_device *sdev)
+{
+	sdev->hostdata = kzalloc(sizeof(struct uasp_lu_info), GFP_KERNEL);
+	if (sdev->hostdata == NULL)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static int uasp_slave_configure(struct scsi_device *sdev)
+{
+	struct uasp_tport_info *tpinfo = SDEV_TPORT_INFO(sdev);
+
+	scsi_set_tag_type(sdev, MSG_ORDERED_TAG);
+	scsi_activate_tcq(sdev, tpinfo->max_cmds);
+
+	return 0;
+}
+
+static void uasp_slave_destroy(struct scsi_device *sdev)
+{
+	kfree(sdev->hostdata);
+}
+
+static struct scsi_host_template uasp_host_template = {
+	.module = THIS_MODULE,
+	.name = DRIVER_NAME,
+	.queuecommand = uasp_queuecommand,
+	.slave_alloc = uasp_slave_alloc,
+	.slave_configure = uasp_slave_configure,
+	.slave_destroy = uasp_slave_destroy,
+	.eh_abort_handler = uasp_abort_cmd,
+	.eh_device_reset_handler = uasp_device_reset,
+	.eh_target_reset_handler = uasp_target_reset,
+	.eh_bus_reset_handler = uasp_bus_reset,
+	.can_queue = 1,
+	.cmd_per_lun = 1,
+	.this_id = -1,
+	.sg_tablesize = SG_NONE,
+	.max_sectors = SCSI_DEFAULT_MAX_SECTORS,
+	.use_clustering = ENABLE_CLUSTERING,
+	.skip_settle_delay = 1,
+	.ordered_tag = 1,
+};
+
+/* ---------- USB related ---------- */
+
+static const struct usb_device_id uasp_usb_ids[] = {
+	{ USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, USB_SC_SCSI, USB_PR_BULK)},
+	{ USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, USB_SC_SCSI, USB_PR_UAS) },
+	{ }
+};
+MODULE_DEVICE_TABLE(usb, uasp_usb_ids);
+
+/* We don't have to do this here if Linux allowed tag usage for
+ * anything other than commands, i.e. TMFs which also use tags.
+ */
+static int uasp_set_max_cmds(struct uasp_tport_info *tpinfo)
+{
+	int mc;
+
+	/* The range of tags generated by the block layer would be
+	 * [0, max_cmds-1], which is [0, num_streams-3]. Now reserve
+	 * stream 1 for untagged commands submitted to us and the last
+	 * usable stream id for a TMF to get the following stream id
+	 * assignment:
+	 * [1:untagged, [2, num_streams-1]:tagged, num_streams:TMF].
+	 */
+	mc = tpinfo->num_streams - 2;
+	if (mc <= 0) {
+		/* Pathological case--perhaps fail discovery?
+		 */
+		dev_notice(&tpinfo->udev->dev,
+			   "device supports too few streams (%d)\n",
+			   tpinfo->num_streams);
+		mc = max(1, tpinfo->num_streams - 1);
+	}
+
+	tpinfo->max_cmds = mc;
+
+	return 0;
+}
+
+static int uasp_ep_conf(struct uasp_tport_info *tpinfo)
+{
+	struct usb_device *udev = tpinfo->udev;
+	struct usb_interface *iface = tpinfo->iface;
+	struct usb_host_endpoint *epa = iface->cur_altsetting->endpoint;
+	int numep = iface->cur_altsetting->desc.bNumEndpoints;
+	int i;
+
+	for (i = 0; i < numep; i++) {
+		unsigned char *desc = epa[i].extra;
+		int len = epa[i].extralen;
+
+		for ( ; len > 1; len -= desc[0], desc += desc[0]) {
+			unsigned pid = desc[2];
+
+			if (desc[0] != 4 || desc[1] != USB_DT_PIPE_USAGE)
+				continue;
+			else if (CMD_PIPE_ID <= pid && pid <= DATAOUT_PIPE_ID) {
+				tpinfo->eps[pid - 1] = &epa[i];
+				break;
+			}
+		}
+	}
+
+	if (tpinfo->eps[0] == NULL || tpinfo->eps[1] == NULL ||
+	    tpinfo->eps[2] == NULL || tpinfo->eps[3] == NULL) {
+		dev_err(&udev->dev, "%s: one or more endpoints are missing\n",
+			__func__);
+		return -1;
+	}
+
+	tpinfo->cmd_pipe = usb_sndbulkpipe(udev,
+					 tpinfo->eps[0]->desc.bEndpointAddress);
+	tpinfo->status_pipe = usb_rcvbulkpipe(udev,
+					 tpinfo->eps[1]->desc.bEndpointAddress);
+	tpinfo->datain_pipe = usb_rcvbulkpipe(udev,
+					 tpinfo->eps[2]->desc.bEndpointAddress);
+	tpinfo->dataout_pipe = usb_sndbulkpipe(udev,
+					 tpinfo->eps[3]->desc.bEndpointAddress);
+
+	if (udev->speed == USB_SPEED_SUPER) {
+		for (i = 1; i < 4; i++) {
+			if (tpinfo->max_streams == 0)
+				tpinfo->max_streams = USB_SS_MAX_STREAMS(tpinfo->eps[i]->ss_ep_comp.bmAttributes);
+			else
+				tpinfo->max_streams = min(tpinfo->max_streams,
+		   USB_SS_MAX_STREAMS(tpinfo->eps[i]->ss_ep_comp.bmAttributes));
+		}
+		
+		if (tpinfo->max_streams <= 1) {
+			dev_err(&udev->dev, "%s: no streams\n", __func__);
+			return -1;
+		}
+		
+		tpinfo->use_streams = 1;
+		tpinfo->num_streams = usb_alloc_streams(iface,
+							&tpinfo->eps[1], 3,
+							tpinfo->max_streams,
+							GFP_KERNEL);
+		if (tpinfo->num_streams <= 0) {
+			dev_err(&udev->dev,
+				"%s: Couldn't allocate %d streams (%d)\n",
+				__func__, tpinfo->max_streams,
+				tpinfo->num_streams);
+			return -1;
+		}
+
+		uasp_set_max_cmds(tpinfo);
+		dev_info(&udev->dev, "streams:%d allocated streams:%d\n",
+			 tpinfo->max_streams, tpinfo->num_streams);
+	} else {
+		tpinfo->use_streams = 0;
+		tpinfo->max_cmds = 32; /* Be conservative */
+	}
+
+	return 0;
+}
+
+static int uasp_set_alternate(struct usb_interface *iface)
+{
+	struct usb_device *udev = interface_to_usbdev(iface);
+	int i, res;
+
+	for (i = 0; i < iface->num_altsetting; i++) {
+		struct usb_host_interface *hi = &iface->altsetting[i];
+
+		if (hi->desc.bInterfaceProtocol == USB_PR_UAS) {
+			int ifnum = iface->cur_altsetting->
+				desc.bInterfaceNumber;
+			int alt = hi->desc.bAlternateSetting;
+
+			res = usb_set_interface(udev, ifnum, alt);
+			if (res) {
+				dev_err(&udev->dev, "%s: Couldn't "
+					"set alternate %d iface (%d)\n",
+					__func__, alt, res);
+				return -ENODEV;
+			}
+			return 0;
+		}
+	}
+	dev_err(&udev->dev, "%s: Match, but no suitable alt "
+		"iface setting\n", __func__);
+	return -ENODEV;
+}
+
+/* TODO: We should ideally register a SCSI Target port with the SCSI
+ * subsystem and let the SCSI Core perform, in that order, REPORT LUN,
+ * INQUIRY, TUR, etc, for each LU, and register each LU as a SCSI
+ * Device.
+ *
+ * We should modify/change/remove the struct scsi_host concept, as we
+ * now see SCSI over a myriad of other protocols, where the host
+ * controller is NOT a SCSI controller at all.  This however deserves
+ * it's own patch.
+ */
+static int uasp_probe(struct usb_interface *iface,
+		      const struct usb_device_id *id)
+{
+	int res;
+	struct Scsi_Host *shost = NULL;
+	struct uasp_tport_info *tpinfo;
+	struct usb_device *udev = interface_to_usbdev(iface);
+
+	if (id->bInterfaceProtocol == USB_PR_BULK) {
+		res = uasp_set_alternate(iface);
+		if (res)
+			return res;
+	}
+
+	tpinfo = kzalloc(sizeof(struct uasp_tport_info), GFP_KERNEL);
+	if (tpinfo == NULL)
+		return -ENOMEM;
+
+	tpinfo->iface = iface;
+	tpinfo->udev = udev;
+
+	res = uasp_ep_conf(tpinfo);
+	if (res)
+		goto Free;
+
+	res = -ENOMEM;
+	shost = scsi_host_alloc(&uasp_host_template, sizeof(void *));
+	if (shost == NULL)
+		goto Free;
+
+	/* The protocol supports XCDB, but that won't fit unsigned short,
+	 * so claim variable length CDB.
+	 */
+	shost->max_cmd_len = 260;
+	shost->max_id = 1;
+	shost->sg_tablesize = udev->bus->sg_tablesize;
+	shost->can_queue = tpinfo->max_cmds;
+	shost->cmd_per_lun = tpinfo->max_cmds;
+
+	dev_info(&shost->shost_gendev, "max commands: %d\n", tpinfo->max_cmds);
+
+	res = scsi_add_host(shost, &iface->dev);
+	if (res)
+		goto Free;
+
+	shost->hostdata[0] = (unsigned long)tpinfo;
+	scsi_scan_host(shost);
+	usb_set_intfdata(iface, shost);
+
+	return res;
+ Free:
+	kfree(tpinfo);
+	if (shost)
+		scsi_host_put(shost);
+	return res;
+}
+
+static int uasp_pre_reset(struct usb_interface *iface)
+{
+	return 0;
+}
+
+static int uasp_post_reset(struct usb_interface *iface)
+{
+	return 0;
+}
+
+static void uasp_disconnect(struct usb_interface *iface)
+{
+	struct Scsi_Host *shost = usb_get_intfdata(iface);
+	struct uasp_tport_info *tpinfo = (void *)shost->hostdata[0];
+
+	scsi_remove_host(shost);
+	usb_free_streams(iface, &tpinfo->eps[1], 3, GFP_KERNEL);
+
+	kfree(tpinfo);
+}
+
+static struct usb_driver uasp_driver = {
+	.name = DRIVER_NAME,
+	.probe = uasp_probe,
+	.disconnect = uasp_disconnect,
+	.pre_reset = uasp_pre_reset,
+	.post_reset = uasp_post_reset,
+	.id_table = uasp_usb_ids,
+};
+
+static int uasp_init(void)
+{
+	return usb_register(&uasp_driver);
+}
+
+static void uasp_exit(void)
+{
+	usb_deregister(&uasp_driver);
+}
+
+module_init(uasp_init);
+module_exit(uasp_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Luben Tuikov");
-- 
1.7.2.2.165.gbc382



^ permalink raw reply related	[flat|nested] 23+ messages in thread
* Re: [PATCH] [USB] UASP: USB Attached SCSI (UAS) protocol driver
@ 2010-12-09  0:16 Luben Tuikov
  2010-12-09  1:52 ` Greg KH
  0 siblings, 1 reply; 23+ messages in thread
From: Luben Tuikov @ 2010-12-09  0:16 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, linux-usb

--- On Mon, 12/6/10, Greg KH <greg@kroah.com> wrote:
>
> We already have this support in the kernel tree,

Greg, this isn't an effort for "linux kernel code turf".  This is a well-implemented driver according to the respective specifications (SAM, SPC, USB 2/3). It reflects correct implementation, naming, etc. It shows understanding of the abstraction layers and underlying technology. It's what you'd want to have in /YOUR/ kernel.

> why do you want and need another driver that does the same exact thing?

Nothing can be further from the truth. The current uas.c driver doesn't do the "exact same thing", far from it.

You responded 4 minutes after my post. Did you have to time to verify that the driver I posted does "the exact same thing" as the one in the kernel?

"The exact same thing", let's see:
1) uasp.c doesn't assume the alternate setting of the interface and discovers its setting. uas.c assumes it's 1.

2) uas.c doesn't do any error checking of the endpoints, and goes ahead
and registers a scsi host first, and then "configures" the endpoints, while end point configuration/discovery may fail or be incorrect.

3) uasp.c correctly parses the pipe usage endpoint descriptors and
saves them in a target port abstraction. If any were not present, discovery of the device fails. 

4) uasp.c discovers the maximum number of streams we can use of the streaming endpoints (their minimum) of the interface, uasp blindly requests 256.

5) uasp.c fails discovery if no streaming is supported (bad configuration) for a SuperSpeed device. uas.c doesn't do any checking and doesn't discern between SS and HS (it relies on usb_alloc_streams() to fail, thus it cannot discover the correct/valid number of streams of the target interface).

6) uasp.c correctly sets the number of commands the block/SCSI layer can enqueue, reserving tags for TMF and untagged commands. uas.c has no concept of TMFs and doesn't do this.

7) uas.c incorrectly sets some members of shost. uasp.c does this correctly.

8) uas.c is filled with incorrect comments showing the unfamiliarity of the author(s) with SAM, SPC, USB, etc. For example,

SCSI:
> .can_queue = 65536,    /* Is there a limit on the _host_ ? */

Unfamiliarity with USB or SAM or both:
> /* XXX: Can we reset just the one USB interface?
>  * Would calling usb_set_interface() have the right effect?
>  */
and
> /* XXX: Need to return 1 if it's not our device in error handling */

Unfamiliarity with UAS/SS (scary commend follows, read at your own risk):
> /*
>  * Why should I request the Status IU before sending the Command IU? Spec
>  * says to, but also says the device may receive them in any order.  Seems
> * daft to me.
> */

Unfamiliarity with SAM, SCSI, etc: the driver trying to re-queue
failed Execute Command procedure with the transport. The driver abusing SCSI_MLQUEUE_DEVICE_BUSY and its usage of a work queue to both requeue URBs and data (wrong in transport protocol driver).  These result in infinite loops in SCSI/block layer when the device goes out to lunch (I can test quite a few possibilities), or errors out, or errors out in the middle of a HighSpeed transaction requiring RRDY and WRDY.

9) uasp.c provides correct (extensible) abstraction of underlying (represented) objects (Target port, LU and command, a la I_T_L_Q nexus). uas.c lacks this.

10) uasp.c implements TMF for both SS and HS modes of operation (directly related to 9 above.) uas.c cannot do this as it has no representation of the I_T_L_Q nexus.

11) uasp.c provides correct naming of structs, constants, etc. It is what you want in a protocol driver. uas.c lacks this.

12) uasp.c does NOT implement packed structs to represent the information units, nor does it use be_to_cpu[p]() and cpu_to_be() to read/write values from said units. It is a portability nightmare (alignment, flexibility, etc). Instead uasp.c uses byte access, shift and or logical operations to extract values from the information units.  uas.c does implement packed structs, and does use be_to_cpu[p]() and cpu_to_be(), again a portability nightmare.

13) uas.c uses infantile non-value comments like:
> /* I hate forward declarations, but I actually have a loop */

14) uasp.c correctly uses dev_dbg()/dev_err()/dev_info()/etc, to report interesting information at each level of debugging. Thus it supports dynamic debugging.

15) uasp.c correctly implements the protocol and doesn't allow for non-conforming devices to be used (which are by default NON-UAS), such as devices that don't implement the required pipes, or early (out of protocol) information units. Such devices should never see the light of day, as they will cause interoperability problems. Dynamic ids are also supported by uasp.c.

16) uasp.c implement a module parameter limiting the number of streams to request from XHCI HCD to allocate. This is due to the fact that some HC misreport the number of streams they support by a factor of two since they filled in the HCCPARAMS field MaxPSASize. That is, they solved 2^v = streams, instead of 2^(v+1) = streams. Of course when the device tries to send back status, the host says ACK(NumP=0) and the device goes into flow control... etc. This module parameter allows to limit the streams allocated.

17) uasp.c implements correct tag assignment for TMFs such that should we get into 16 above, a TMF status _can_ be returned to the host. uas.c has no concept of TMFs to begin with.

18) uasp.c correctly checks the device configuration and fails discovery as it correctly reports that to USB.

There is perhaps more.

   Luben


^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH] [USB] UASP: USB Attached SCSI (UAS) protocol driver
@ 2010-12-10 23:15 Luben Tuikov
  2010-12-10 23:42 ` Alan Cox
  0 siblings, 1 reply; 23+ messages in thread
From: Luben Tuikov @ 2010-12-10 23:15 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, linux-usb

Before I begin, I'd like everyone to note that Greg has completely ignored (and removed from the quotation) the 18 points of technical matter in my email and instead decided to write this, to which I'm responding here:

--- On Wed, 12/8/10, Greg KH <greg@kroah.com> wrote:
> 
> What I don't want in _anyones_ kernel is multiple drivers
> wanting to
> attach to the same device.
> 
> By "same thing" I mean "binds to the same device."

Thanks for clarifying, Greg. For a moment everyone thought that by "the exact same thing" you meant "same functionality, same quality, identical, etc".

Now, if you don't want the still-born uas.c to bind to the same device, please simply remove the ids from that driver.

> I have lived through the hell of this in the past and I
> will not let
> that happen again, sorry.

So, according to your statement above "binding to the same device" is all it takes. The solution is very simple--see above.

So to expose the obvious, hypothetically, one of the resident "linux kernel engineers/maintainers" could've submitted AN EMPTY DRIVER that "binds to" the UAS id, and from then on, any other improvements would have to go through them, even if they don't work with the technology and are not very (at all?) knowledgeable about the technologies/standards/methodologies/termninology involved.

> I'm not saying that the in-kernel driver is perfect at all,

I think you did say "the exact same thing" ;-) in your previous email in this thread saying it "*does* the exact same thing".

So I guess, you didn't clarify what you meant by "does" when you said "does the exact same thing". Thanks for clarifying that by "does" you mean "binds to the same ids".

> and it's
> author is getting annoying by refusing to answer my emails,
> but still,
> please work with him to get the in-kernel driver to work up
> to your
> needs.

Why are you pushing him to this when he's getting annoying to you? When he's not responding to your emails? Is he your friend? (perhaps more?--none of my business) Do you guys have beers at LinuxCon or other Linux conferences? Is he in the technology sector? Or is he just a "linux kernel engineer"? A guru in USB? Or maybe SCSI? Is his employer, Intel, pressuring you? Because when you removed all possibilities, however improbable, must be the truth. (after all they did write the XHCI HCD) All I see is a lot of nepotism, an inferior coding, inferior terminology used, inferior naming convention, incompetence in USB and SCSI protocols, etc, etc, etc.

Were you not part of the same thread where YOU RESPONDED addressing its author about his cowardly and lacking-professional-integrity ways of changing my patches including changing the commit messages without reporting what's been changed, how it's been changed and where it's been committed to? To refresh everyone's memory, here:
http://marc.info/?l=linux-usb&m=129133499714636&w=2
I guess you had to do that to be released of some kind of binding?

Do you not see HOW DIFFERENT the two drivers are? Do you not see the difference in quality, presentation, etc, etc.

Linux is truly open source, but closed community. You have shown in this thread that it's not about the code or the technology, but the turf. After I listed the technical differences between the two drivers in this thread (18 and counting), after you've seen the code and leading on with this thread despite the problems you're having with him and his not responding to your emails.  Why you are protecting his interests so blatantly despite the problems you're having with him is beyond me. (other than, again, if his employers is involved and pressuring you)

"...whatever is left, however improbable, must be the truth."

> If you have problems with the maintainer, please let me
> know and I'll
> see what I can do.

What? Were you not on the CC list of the patches I posted, did you not state in those threads you're having problems with him too? You yourself admitted above that you also have problems with him in those very threads. You are protecting a person, not code.
Here are the threads:
http://marc.info/?t=128821112700001&r=1&w=2
http://marc.info/?t=128829652800022&r=1&w=2
http://marc.info/?t=128839891500022&r=1&w=2
http://marc.info/?t=128901886000001&r=1&w=2

> But again, this driver is not acceptable as-is, sorry.

"as-is"? You are giving me a clue, Greg! (Since you're NOT specifying any technical reasons.)

Greg, (to expose the obvious agenda and interests being protected) how about if I added his name as an author to my driver? Will you accept it then? Or perhaps I can send in a patch ripping out everything from the still-born uas.c other than the copyright and MODULE_AUTHOR() and substituting in the contents of my uasp.c (removing my name altogether)?

After all, people and the industry are learning a lot from this thread.  They are learning that the open source Linux kernel has been institutionalized between a few chosen people, regardless of capability, thus creating a closed community whose interests are highly protected. It's not about the code or the technology any more.

I think people need to know.

     Luben



^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH] [USB] UASP: USB Attached SCSI (UAS) protocol driver
@ 2010-12-13 21:46 Luben Tuikov
  2010-12-13 22:37 ` Matthew Dharm
  0 siblings, 1 reply; 23+ messages in thread
From: Luben Tuikov @ 2010-12-13 21:46 UTC (permalink / raw)
  To: Alan Cox; +Cc: Greg KH, linux-kernel, linux-usb

--- On Sat, 12/11/10, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> 
> > Alan, don't you work for the same company as do the
> authors listed in "uas.c"?
> 
> If you've got nothing better to do than insinuate further
> unpleasant
> things when people try and help then don't expect any help
> from anyone.
> 
> *plonk*

Alan,

When in doubt, follow the money.

The problem isn't in that you're trying to do what is convenient for the corporation which has hired you and pays your salary. The problem is that you're still pretending that you have the interests of _free_ open source *as you once had*.

Did you review my driver? Did anyone else in this thread? No, of course not. You didn't even do it the second time I called your bluff in this thread. Why not? Simple, it's not what you're getting paid to do now.

Can you imagine Greg pulling out a defunct driver written by a co-worker of yours, (working for the same large corporation as you are), in favor of some no-name working driver? I mean imagine the sponsorships, the free equipment, etc your company provides to you, Greg and the "Linux community"? Your company is the single largest sponsor of Linux conferences and anything Linux. 

After all, who do you think got Greg a UAS device?  (not me ;-) )

    Luben


^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH] [USB] UASP: USB Attached SCSI (UAS) protocol driver
@ 2010-12-14 17:25 Luben Tuikov
  2010-12-14 18:12 ` Ben Gamari
  0 siblings, 1 reply; 23+ messages in thread
From: Luben Tuikov @ 2010-12-14 17:25 UTC (permalink / raw)
  To: Matthew Dharm; +Cc: Alan Cox, Greg KH, linux-kernel, linux-usb

--- On Mon, 12/13/10, Matthew Dharm <mdharm-kernel@one-eyed-alien.net> wrote:
> Your insinuations against the character of many
> long-standing Linux
> developers, based on their refusal to accept your
> submission,

No, not submission. The fact is that no one bothered to review the code. It was simply flat out NAK-ed it in just four minutes, fronting one of their co-workers with the defunct driver already in.

It would've been a different story if Greg, Alan and you and anyone else willing, had provided a technical review, and /then/ reject it--the rejection reason would've been clearer to everyone, than taking all this thread to get to.

Greg didn't review it and rejected it 4 minutes after posting. Alan didn't review it and after I posted the 18 technical reasons (my 2nd post in this thread replying to Greg's) said this:
> Your driver may be the best on the planet - who knows - 
Obviously he didn't review it.
You, after I posted the driver and the 18 technical reasons, said this:
> If, in fact, Luben's driver is somehow "better", ...
and
> ... several things would have to be true.  I do not know if they are true here, ...
and
> That said, unless the current driver is really bad, and Luben's is tremendously better,

These are from this very thread.

> are simply
> beyond the pale.  I could just as easily make every
> one of these
> accusations against you -- who pays your salary, for
> example?  What are
> their interests?  You make claims about your
> motivations, but how am I to
> verify any of those?

I've already stated that there had never been an agenda about uasp.c and Linux. The uasp.c driver was written over most of a Saturday (when I had time and uas.c showed itself to be defunct).  Since uasp.c is a working driver and correctly implemented in terms of UAS, USB, SCSI, the kernel and it's layers, I thought it would be *helpful* and *useful* to people--that's all.

The driver was rejected flat out only after 4 minutes after submission claiming that there is a driver in the kernel that "does the exact same thing".  I then provided 18 technical reasons as to why uas.c does NOT in fact do the exact same thing to which Greg responded with:

> But again, this driver is not acceptable as-is, sorry.

Why would he have to apologize? Anyway, to which you responded with not knowing which driver is good.

> The linux community works because it is exactly that -- a
> community.  We
> have, just like any other community, standards of
> decorum.  You have
> violated those standards, repeatedly, despite several
> people politely
> telling you the proper manner in which to behave.

When people read "the community" responses in these mailing lists, they think "Wow what a benevolent bunch of people" and "look, they just wanna help". But what this mailing list doesn't show is that this is a very close knit of people who work for the same company (or two) and who go to Linux conferences religiously and are very close buddies. Obviously uasp.c didn't stand a chance _and_ was rejected in just 4 minutes, without a technical review. As you can see in this thread the three of you kept fronting the uas.c driver despite the 18 technical reasons I posted and the fact that it's badly written, and needs to be replaced with something that works, as people will find out when UAS devices make it to market. It's all in the beginning of this thread, start reading here:
http://marc.info/?t=129165521000011&r=2&w=2

So after such a blatant refusal to even give a technical review of the driver, it had me thinking: why would that be?

> > Did you review my driver? Did anyone else in this
> thread? No, of course
> > not. You didn't even do it the second time I called
> your bluff in this
> > thread. Why not? Simple, it's not what you're getting
> paid to do now.
> 
> *bzzt*  Wrong.  I did review your driver.  I
> found it interesting, but upon
> first submission I found no compelling reason to prefer it
> over the old.
> Your initial submission gave no such reasons.

If you reviewed it, then why did you write that you had no idea if the driver were good or not? Here is what you wrote in this very thread:

> If, in fact, Luben's driver is somehow "better", ...
and
> ... several things would have to be true.  I do not know if they are true here, ...
and
> That said, unless the current driver is really bad, and Luben's is tremendously better,

Your full message can be found here: http://marc.info/?l=linux-kernel&m=129186163519728&w=2.

But now you say that you had reviewed it!? Now you say you found the driver "interesting" but "no compelling reason to prefer it over the old". How about that it works? How about the 18 technical reasons I posted *before* you posted your response quoted above (which reasons you had read)?  You didn't address any of them.

> It was only until, after your submission was refused,

It was flat out refused only 4 minutes after I submitted it.

> that
> you addressed
> the technical merits of your driver over the existing
> one.

The reason I posted the 18 technical reasons, is because Greg claimed that uas.c "does the exact same thing". His response is here: http://marc.info/?l=linux-usb&m=129165544600441&w=2, he posted this only 4 minutes after I posted the driver. I therefore decided to clarify that uas.c does NOT in fact do the "exact same thing", and backed that up with the 18 technical points.

> I will admit,
> you have some valid technical points.  But, they are
> completely overridden
> by your inability to work within the standards of behavior
> of the community.

What does this actually mean? I just posted my uasp.c driver in this very thread and it got rejected 4 minutes afterwards without technical review. When I followed up with a technical review you say "your inability to work within the standards of behavior of the community".

Does providing no technical review and flat out rejecting a driver fall into your "inability to work within the standards of behavior of the community" definition?

> If, at any time, you had decided to stop using ad-hominem
> attacks and
> instead made a calm, composed, and well-reasoned statement
> about why the
> work to fixup the existing driver would be worse than doing

1. The whole thing should be ripped out. Any good work would change 100% of the driver as uasp.c shows (essentially getting a new driver in).
2. I did make that "statement" in the 18 technical reasons of why uasp.c was better than uas.c, other than "it works" of course.

> a drop-in
> replacement, we would have listened.

I did. In the 18 technical reasons I listed in this very thread. Here is the link: http://marc.info/?l=linux-kernel&m=129185378712222&w=2

> You came pretty
> close to doing this,
> but seemed completely unable to resist personal attacks
> against members of
> this community.

And those are? I think you're priming up here what people should think.

> And you never really addressed,
> beyond a 1-line statement
> stating that to fixup the existing driver you would need to
> replace it
> wholesale, why working to improve the existing driver is a
> bad choice.

I said that in the 18 technical reasons I listed, link above.  The existing driver is badly written, the infrastructure is just off.

> We look to submitters to maintain their work.  Yes,
> there are apparently
> some difficulties getting Matt W. to push the ball forward
> on his UAS
> driver.

It's simple, rip out uas.c and put in uasp.c and give people something working and let everyone contribute to something with good foundation.

> However, dealing with you is comparable to
> drinking napalm; I
> would much rather deal with someone who is
> less-than-responsive rather than
> someone who is abrasive.

Actually, it's very easy to deal with me. When people come to me, I say "What can I do to help you?" and then I provide them a solution and always go the extra mile, and give them something constructive they can use, and I say, "here it is, and I did this extra thing and let me know what else I can do for you."  Most of the time I don't wait for people to come to ask me, but proactively contribute to the end goal, surprising people with "it's already done" when they ask for it.

   Luben


^ permalink raw reply	[flat|nested] 23+ messages in thread
end of thread, other threads:[~2010-12-16 12:11 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-06 17:05 [PATCH] [USB] UASP: USB Attached SCSI (UAS) protocol driver Luben Tuikov
2010-12-06 17:09 ` Greg KH
  -- strict thread matches above, loose matches on Subject: below --
2010-12-09  0:16 Luben Tuikov
2010-12-09  1:52 ` Greg KH
2010-12-09  2:26   ` Matthew Dharm
2010-12-10 23:15 Luben Tuikov
2010-12-10 23:42 ` Alan Cox
2010-12-11  2:26   ` Luben Tuikov
2010-12-13 18:40     ` Mark Brown
2010-12-13 19:48       ` Matthias Schniedermeyer
2010-12-13 21:53         ` Luben Tuikov
2010-12-13 21:53         ` Andreas Mohr
2010-12-14  0:42           ` Sarah Sharp
2010-12-14  8:30             ` Luben Tuikov
2010-12-13 21:34       ` Luben Tuikov
2010-12-13 21:46 Luben Tuikov
2010-12-13 22:37 ` Matthew Dharm
2010-12-14 17:25 Luben Tuikov
2010-12-14 18:12 ` Ben Gamari
2010-12-15 22:17   ` Luben Tuikov
2010-12-16  6:29   ` Andreas Mohr
2010-12-16  9:46     ` Luben Tuikov
2010-12-16 12:11       ` Andreas Mohr

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox