usb: gadget: f_tcm: Enhance UASP driver
Subsystems (3)
v2 cover usb: gadget: f_tcm: Enhance UASP driver
The Linux UASP gadget driver is incomplete and remained broken for a long time. It was not implemented for performance either. This series adds some of the required features for the UASP driver to work. It also makes some changes to the target core.
This is tested against UASP CV and DWC_usb3x controller. It still needs some fixes in the target core, which will be separated from this series.
There are still more room for performance improvement and fixes. However, this series should be sufficient to bring up a working UASP device.
Changes in v2:
- Remove most target core changes from this series and only keep the must-have ones
- Split the task-management patch to smaller patches
- Don't send failure Task Management response to target core, reducing dependency
- Add UASP bringup script example in cover page
- Make various small updates according to feedbacks
Example script
==============
To test UASP, here's an example perl script snippet to bring it up.
Note: the script was cut down and quickly rewritten, so sorry if I make mistakes. :)
my $MY_UAS_VID = xxxx;
my $MY_UAS_PID = yyyy;
my $SERIAL = "1234";
my $VENDOR = "VENDOR";
my $MY_VER = "VER";
my $vendor_id = "my_vid";
my $product_id = "my_pid";
my $revision = "my_rev";
# Must update:
my $backing_storage = "/tmp/some_file";
my $backing_storage_size = 1024*1024*16;
my $use_ramdisk = 0;
my $g = "/sys/kernel/config/usb_gadget/g1";
system("modprobe libcomposite");
system("modprobe usb_f_tcm");
system("mkdir -p $g");
system("mkdir -p $g/configs/c.1");
system("mkdir -p $g/functions/tcm.0");
system("mkdir -p $g/strings/0x409");
system("mkdir -p $g/configs/c.1/strings/0x409");
my $tp = "/sys/kernel/config/target/usb_gadget/naa.0/tpgt_1";
my $tf;
my $ctrl;
if ($use_ramdisk) {
$tf = "/sys/kernel/config/target/core/rd_mcp_0/ramdisk";
$ctrl = 'rd_pages=524288';
} else {
$tf = "/sys/kernel/config/target/core/fileio_0/fileio";
$ctrl = 'fd_dev_name=$backing_storage,fd_dev_size=$backing_storage_size,fd_async_io=1';
}
system("mkdir -p /etc/target");
system("mkdir -p $tp");
system("mkdir -p $tf");
system("mkdir -p $tp/lun/lun_0");
system("echo naa.0 > $tp/nexus");
system("echo $ctrl > $tf/control");
system("echo 1 > $tf/attrib/emulate_ua_intlck_ctrl");
system("echo 123 > $tf/wwn/vpd_unit_serial");
system("echo $vendor_id > $tf/wwn/vendor_id");
system("echo $product_id > $tf/wwn/product_id");
system("echo $revision > $tf/wwn/revision");
system("echo 1 > $tf/enable");
system("ln -s $tf $tp/lun/lun_0/virtual_scsi_port");
system("echo 1 > $tp/enable");
system("echo $MY_UAS_PID > $g/idProduct");
system("ln -s $g/functions/tcm.0 $g/configs/c.1");
system("echo $MY_UAS_VID > $g/idVendor");
system("echo $SERIAL > $g/strings/0x409/serialnumber");
system("echo $VENDOR > $g/strings/0x409/manufacturer");
system("echo \"$MY_VER\" > $g/strings/0x409/product");
system("echo \"Conf 1\" > $g/configs/c.1/strings/0x409/configuration");
system("echo super-speed-plus > $g/max_speed");
# Make sure the UDC is available
system("echo $my_udc > $g/UDC");Thinh Nguyen (25):
target: Add overlapped response to tmrsp_table
target: Add common TMR enum
usb: gadget: f_tcm: Increase stream count
usb: gadget: f_tcm: Increase bMaxBurst
usb: gadget: f_tcm: Don't set static stream_id
usb: gadget: f_tcm: Allocate matching number of commands to streams
usb: gadget: f_tcm: Limit number of sessions
usb: gadget: f_tcm: Handle multiple commands in parallel
usb: gadget: f_tcm: Use extra number of commands
usb: gadget: f_tcm: Return ATA cmd direction
usb: gadget: f_tcm: Execute command on write completion
usb: gadget: f_tcm: Minor cleanup redundant code
usb: gadget: f_tcm: Don't free command immediately
usb: gadget: f_tcm: Translate error to sense
usb: gadget: f_tcm: Cleanup unused variable
usb: gadget: f_tcm: Update state on data write
usb: gadget: f_tcm: Handle abort command
usb: gadget: f_tcm: Cleanup requests on ep disable
usb: gadget: f_tcm: Decrement command ref count on cleanup
usb: gadget: f_tcm: Save CPU ID per command
usb: gadget: f_tcm: Get stream by tag
usb: gadget: f_tcm: Send sense on cancelled transfer
usb: gadget: f_tcm: Handle TASK_MANAGEMENT commands
usb: gadget: f_tcm: Check overlapped command
usb: gadget: f_tcm: Comply with UAS Task Management requirement
drivers/target/target_core_transport.c | 10 + drivers/usb/gadget/function/f_tcm.c | 536 +++++++++++++++++++------ drivers/usb/gadget/function/tcm.h | 20 +- include/target/target_core_base.h | 5 + 4 files changed, 436 insertions(+), 135 deletions(-)
base-commit: 88a15fbb47db483d06b12b1ae69f114b96361a96
Signature · 1 line
2.28.0
PATCH v2 1/25 target: Add overlapped response to tmrsp_table
Add TMR_OVERLAPPED_TAG_ATTEMPTED response to tmrsp_table.
---
Changes in v2:
- None
include/target/target_core_base.h | 1 + 1 file changed, 1 insertion(+)
include/target/target_core_base.h +1 -0
@@ -223,6 +223,7 @@ enum tcm_tmrsp_table {TMR_LUN_DOES_NOT_EXIST =3 ,TMR_TASK_MGMT_FUNCTION_NOT_SUPPORTED =4 ,TMR_FUNCTION_REJECTED =5 , + TMR_OVERLAPPED_TAG_ATTEMPTED = 6, };/*
Signature · 1 line
2.28.0
PATCH v2 2/25 target: Add common TMR enum
In reply to Thinh NguyenAdd the following common TMR enum:
* TMR_I_T_NEXUS_RESET
* TMR_QUERY_TASK
* TMR_QUERY_TASK_SET
* TMR_QUERY_ASYNC_EVENT
---
Changes in v2:
- Changed enum numbering to continue from 8, 9, 10, ...
drivers/target/target_core_transport.c | 10 ++++++++++ include/target/target_core_base.h | 4 ++++ 2 files changed, 14 insertions(+)
drivers/target/target_core_transport.c +10 -0
@@ -3094,6 +3094,10 @@ static const char *target_tmf_name(enum tcm_tmreq_table tmf)case TMR_TARGET_WARM_RESET :return "TARGET_WARM_RESET" ;case TMR_TARGET_COLD_RESET :return "TARGET_COLD_RESET" ;case TMR_LUN_RESET_PRO :return "LUN_RESET_PRO" ; +case TMR_I_T_NEXUS_RESET :return "I_T_NEXUS_RESET" ; +case TMR_QUERY_TASK :return "QUERY_TASK" ; +case TMR_QUERY_TASK_SET :return "QUERY_TASK_SET" ; +case TMR_QUERY_ASYNC_EVENT :return "QUERY_ASYNC_EVENT" ;case TMR_UNKNOWN :break ; }return "(?)" ;
@@ -3542,6 +3546,12 @@ static void target_tmr_work(struct work_struct *work)case TMR_TARGET_COLD_RESET : tmr->response =TMR_FUNCTION_REJECTED ;break ; +case TMR_I_T_NEXUS_RESET : +case TMR_QUERY_TASK : +case TMR_QUERY_TASK_SET : +case TMR_QUERY_ASYNC_EVENT : + tmr->response =TMR_FUNCTION_REJECTED ; +break ;default :pr_err ("Unknown TMR function: 0x%02x.\n" , tmr->function);
include/target/target_core_base.h +4 -0
@@ -211,6 +211,10 @@ enum tcm_tmreq_table {TMR_LUN_RESET =5 ,TMR_TARGET_WARM_RESET =6 ,TMR_TARGET_COLD_RESET =7 , +TMR_I_T_NEXUS_RESET =8 , +TMR_QUERY_TASK =9 , +TMR_QUERY_TASK_SET =10 , +TMR_QUERY_ASYNC_EVENT =11 ,TMR_LUN_RESET_PRO =0x80 , TMR_UNKNOWN = 0xff, };
Signature · 1 line
2.28.0
PATCH v2 3/25 usb: gadget: f_tcm: Increase stream count
In reply to Thinh NguyenSome old builds of Microsoft Windows 10 UASP class driver reject USAP device with stream count of 2^4. To keep compatibility with both Linux and Windows, let's increase the stream count to 2^5. Also, internal tests show that stream count of 2^5 increases performance slightly.
---
Changes in v2:
- None
drivers/usb/gadget/function/tcm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/gadget/function/tcm.h +1 -1
@@ -13,7 +13,7 @@#define USBG_NAMELEN 32#define fuas_to_gadget (f) (f->function.config->cdev->gadget) -#define UASP_SS_EP_COMP_LOG_STREAMS 4 +#define UASP_SS_EP_COMP_LOG_STREAMS 5#define UASP_SS_EP_COMP_NUM_STREAMS (1 << UASP_SS_EP_COMP_LOG_STREAMS)enum {
Signature · 1 line
2.28.0
Re: [PATCH v2 03/25] usb: gadget: f_tcm: Increase stream count
Hello!
Quoted 1 line · Thinh Nguyen
Some old builds of Microsoft Windows 10 UASP class driver reject USAP
UASP?
Quoted 5 lines
device with stream count of 2^4. To keep compatibility with both Linux and Windows, let's increase the stream count to 2^5. Also, internal tests show that stream count of 2^5 increases performance slightly.
Signed-off-by Thinh Nguyen <[email protected]>
[...]
MBR, Sergey
PATCH v2 4/25 usb: gadget: f_tcm: Increase bMaxBurst
In reply to Thinh NguyenCurrently the default bMaxBurst is 0. Set default bMaxBurst to 15 (i.e. 16 bursts) to Data IN and OUT endpoints to improve performance. It should be fine for a controller that supports less than 16 bursts. It should be able to negotiate properly with the host at packet level for the end of burst.
If the controller can't handle a burst of 16, and high performance isn't important, the user can use BOT protocol from mass_storage gadget driver instead.
---
Changes in v2:
- None
drivers/usb/gadget/function/f_tcm.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/usb/gadget/function/f_tcm.c +4 -2
@@ -1774,7 +1774,7 @@ static struct usb_endpoint_descriptor uasp_ss_bi_desc = {static struct usb_ss_ep_comp_descriptor uasp_bi_ep_comp_desc = { .bLength =sizeof (uasp_bi_ep_comp_desc), .bDescriptorType =USB_DT_SS_ENDPOINT_COMP , - .bMaxBurst =0 , + .bMaxBurst =15 , .bmAttributes =UASP_SS_EP_COMP_LOG_STREAMS , .wBytesPerInterval =0 , };
@@ -1782,7 +1782,7 @@ static struct usb_ss_ep_comp_descriptor uasp_bi_ep_comp_desc = {static struct usb_ss_ep_comp_descriptor bot_bi_ep_comp_desc = { .bLength =sizeof (bot_bi_ep_comp_desc), .bDescriptorType =USB_DT_SS_ENDPOINT_COMP , - .bMaxBurst =0 , + .bMaxBurst =15 , };static struct usb_endpoint_descriptor uasp_bo_desc = {
@@ -1817,12 +1817,14 @@ static struct usb_endpoint_descriptor uasp_ss_bo_desc = {static struct usb_ss_ep_comp_descriptor uasp_bo_ep_comp_desc = { .bLength =sizeof (uasp_bo_ep_comp_desc), .bDescriptorType =USB_DT_SS_ENDPOINT_COMP , + .bMaxBurst =15 , .bmAttributes =UASP_SS_EP_COMP_LOG_STREAMS , };static struct usb_ss_ep_comp_descriptor bot_bo_ep_comp_desc = { .bLength =sizeof (bot_bo_ep_comp_desc), .bDescriptorType =USB_DT_SS_ENDPOINT_COMP , + .bMaxBurst =15 , };static struct usb_endpoint_descriptor uasp_status_desc = {
Signature · 1 line
2.28.0
PATCH v2 5/25 usb: gadget: f_tcm: Don't set static stream_id
In reply to Thinh NguyenHost can assign stream ID value greater than number of streams allocated. The tcm function needs to keep track of which stream is available to assign the stream ID. This patch doesn't track that, but at least it makes sure that there's no Oops if the host send tag with a value greater than the number of supported streams.
---
Changes in v2:
- None
drivers/usb/gadget/function/f_tcm.c | 32 +++++------------------------ 1 file changed, 5 insertions(+), 27 deletions(-)
drivers/usb/gadget/function/f_tcm.c +5 -27
@@ -532,6 +532,7 @@ static int uasp_prepare_r_request(struct usbg_cmd *cmd) } stream->req_in->is_last =1 ; + stream->req_in->stream_id = cmd->tag; stream->req_in->complete = uasp_status_data_cmpl; stream->req_in->length = se_cmd->data_length; stream->req_in->context = cmd;
@@ -556,6 +557,7 @@ static void uasp_prepare_status(struct usbg_cmd *cmd) iu->len =cpu_to_be16 (se_cmd->scsi_sense_length); iu->status = se_cmd->scsi_status; stream->req_status->is_last =1 ; + stream->req_status->stream_id = cmd->tag; stream->req_status->context = cmd; stream->req_status->length = se_cmd->scsi_sense_length +16 ; stream->req_status->buf = iu;
@@ -786,19 +788,6 @@ static int uasp_alloc_cmd(struct f_uas *fu)return -ENOMEM ; } -static void uasp_setup_stream_res (struct f_uas *fu,int max_streams) -{ -int i; - -for (i =0 ; i < max_streams; i++) { -struct uas_stream *s = &fu->stream[i]; - - s->req_in->stream_id = i +1 ; - s->req_out->stream_id = i +1 ; - s->req_status->stream_id = i +1 ; - } -} -static int uasp_prepare_reqs (struct f_uas *fu) {int ret;
@@ -819,7 +808,6 @@ static int uasp_prepare_reqs(struct f_uas *fu) ret =uasp_alloc_cmd (fu);if (ret) goto err_free_stream; -uasp_setup_stream_res (fu, max_streams); ret =usb_ep_queue (fu->ep_cmd, fu->cmd.req,GFP_ATOMIC );if (ret)
@@ -995,6 +983,7 @@ static int usbg_prepare_w_request(struct usbg_cmd *cmd, struct usb_request *req) } req->is_last =1 ; + req->stream_id = cmd->tag; req->complete = usbg_data_write_cmpl; req->length = se_cmd->data_length; req->context = cmd;
@@ -1125,16 +1114,8 @@ static int usbg_submit_command(struct f_uas *fu, }memcpy (cmd->cmd_buf, cmd_iu->cdb, cmd_len); -if (fu->flags &USBG_USE_STREAMS ) { -if (cmd->tag >UASP_SS_EP_COMP_NUM_STREAMS ) - goto err; -if (!cmd->tag) - cmd->stream = &fu->stream[0 ]; -else - cmd->stream = &fu->stream[cmd->tag -1 ]; - }else { - cmd->stream = &fu->stream[0 ]; - } + cmd->stream = &fu->stream[cmd->tag % +UASP_SS_EP_COMP_NUM_STREAMS ];switch (cmd_iu->prio_attr &0x7 ) {case UAS_HEAD_TAG :
@@ -1161,9 +1142,6 @@ static int usbg_submit_command(struct f_uas *fu,queue_work (tpg->workqueue, &cmd->work);return 0 ; -err: -usbg_release_cmd (&cmd->se_cmd); -return -EINVAL ; }static void bot_cmd_work (struct work_struct *work)
Signature · 1 line
2.28.0
PATCH v2 6/25 usb: gadget: f_tcm: Allocate matching number of commands to streams
In reply to Thinh NguyenWe can handle multiple commands concurently. Each command services a stream id. At the moment, the driver will handle 32 outstanding streams, which is equivalent to 32 commands. Make sure to allocate a matching number of commands to the number of streams.
---
Changes in v2:
- None
drivers/usb/gadget/function/f_tcm.c | 110 ++++++++++++++-------------- drivers/usb/gadget/function/tcm.h | 8 +- 2 files changed, 61 insertions(+), 57 deletions(-)
drivers/usb/gadget/function/f_tcm.c +55 -55
@@ -50,7 +50,7 @@ static int bot_enqueue_cmd_cbw(struct f_uas *fu)if (fu->flags &USBG_BOT_CMD_PEND )return 0 ; - ret =usb_ep_queue (fu->ep_out, fu->cmd.req,GFP_ATOMIC ); + ret =usb_ep_queue (fu->ep_out, fu->cmd[0 ].req,GFP_ATOMIC );if (!ret) fu->flags |=USBG_BOT_CMD_PEND ;return ret;
@@ -136,7 +136,7 @@ static void bot_send_bad_status(struct usbg_cmd *cmd) } req->complete = bot_err_compl; req->context = cmd; - req->buf = fu->cmd.buf; + req->buf = fu->cmd[0 ].buf;usb_ep_queue (ep, req,GFP_KERNEL ); }else {bot_enqueue_sense_code (fu, cmd);
@@ -314,8 +314,8 @@ static int bot_prepare_reqs(struct f_uas *fu)if (!fu->bot_req_out) goto err_out; - fu->cmd.req =usb_ep_alloc_request (fu->ep_out,GFP_KERNEL ); -if (!fu->cmd.req) + fu->cmd[0 ].req =usb_ep_alloc_request (fu->ep_out,GFP_KERNEL ); +if (!fu->cmd[0 ].req) goto err_cmd; fu->bot_status.req =usb_ep_alloc_request (fu->ep_in,GFP_KERNEL );
@@ -327,27 +327,27 @@ static int bot_prepare_reqs(struct f_uas *fu) fu->bot_status.req->complete = bot_status_complete; fu->bot_status.csw.Signature =cpu_to_le32 (US_BULK_CS_SIGN ); - fu->cmd.buf =kmalloc (fu->ep_out->maxpacket,GFP_KERNEL ); -if (!fu->cmd.buf) + fu->cmd[0 ].buf =kmalloc (fu->ep_out->maxpacket,GFP_KERNEL ); +if (!fu->cmd[0 ].buf) goto err_buf; - fu->cmd.req->complete = bot_cmd_complete; - fu->cmd.req->buf = fu->cmd.buf; - fu->cmd.req->length = fu->ep_out->maxpacket; - fu->cmd.req->context = fu; + fu->cmd[0 ].req->complete = bot_cmd_complete; + fu->cmd[0 ].req->buf = fu->cmd[0 ].buf; + fu->cmd[0 ].req->length = fu->ep_out->maxpacket; + fu->cmd[0 ].req->context = fu; ret =bot_enqueue_cmd_cbw (fu);if (ret) goto err_queue;return 0 ; err_queue: -kfree (fu->cmd.buf); - fu->cmd.buf =NULL ; +kfree (fu->cmd[0 ].buf); + fu->cmd[0 ].buf =NULL ; err_buf:usb_ep_free_request (fu->ep_in, fu->bot_status.req); err_sts: -usb_ep_free_request (fu->ep_out, fu->cmd.req); - fu->cmd.req =NULL ; +usb_ep_free_request (fu->ep_out, fu->cmd[0 ].req); + fu->cmd[0 ].req =NULL ; err_cmd:usb_ep_free_request (fu->ep_out, fu->bot_req_out); fu->bot_req_out =NULL ;
@@ -372,16 +372,16 @@ static void bot_cleanup_old_alt(struct f_uas *fu)usb_ep_free_request (fu->ep_in, fu->bot_req_in);usb_ep_free_request (fu->ep_out, fu->bot_req_out); -usb_ep_free_request (fu->ep_out, fu->cmd.req); +usb_ep_free_request (fu->ep_out, fu->cmd[0 ].req);usb_ep_free_request (fu->ep_in, fu->bot_status.req); -kfree (fu->cmd.buf); +kfree (fu->cmd[0 ].buf); fu->bot_req_in =NULL ; fu->bot_req_out =NULL ; - fu->cmd.req =NULL ; + fu->cmd[0 ].req =NULL ; fu->bot_status.req =NULL ; - fu->cmd.buf =NULL ; + fu->cmd[0 ].buf =NULL ; }static void bot_set_alt (struct f_uas *fu)
@@ -482,10 +482,14 @@ static void uasp_cleanup_one_stream(struct f_uas *fu, struct uas_stream *stream)static void uasp_free_cmdreq (struct f_uas *fu) { -usb_ep_free_request (fu->ep_cmd, fu->cmd.req); -kfree (fu->cmd.buf); - fu->cmd.req =NULL ; - fu->cmd.buf =NULL ; +int i; + +for (i =0 ; i <USBG_NUM_CMDS ; i++) { +usb_ep_free_request (fu->ep_cmd, fu->cmd[i].req); +kfree (fu->cmd[i].buf); + fu->cmd[i].req =NULL ; + fu->cmd[i].buf =NULL ; + } }static void uasp_cleanup_old_alt (struct f_uas *fu)
@@ -500,7 +504,7 @@ static void uasp_cleanup_old_alt(struct f_uas *fu)usb_ep_disable (fu->ep_status);usb_ep_disable (fu->ep_cmd); -for (i =0 ; i <UASP_SS_EP_COMP_NUM_STREAMS ; i++) +for (i =0 ; i <USBG_NUM_CMDS ; i++)uasp_cleanup_one_stream (fu, &fu->stream[i]);uasp_free_cmdreq (fu); }
@@ -603,7 +607,8 @@ static void uasp_status_data_cmpl(struct usb_ep *ep, struct usb_request *req)case UASP_QUEUE_COMMAND :transport_generic_free_cmd (&cmd->se_cmd,0 ); -usb_ep_queue (fu->ep_cmd, fu->cmd.req,GFP_ATOMIC ); +usb_ep_queue (fu->ep_cmd, cmd->req,GFP_ATOMIC ); +break ;default :
@@ -718,7 +723,7 @@ static int uasp_send_write_request(struct usbg_cmd *cmd)return ret; } -static int usbg_submit_command (struct f_uas *,void *,unsigned int ); +static int usbg_submit_command (struct f_uas *,struct usb_request *);static void uasp_cmd_complete (struct usb_ep *ep,struct usb_request *req) {
@@ -728,7 +733,7 @@ static void uasp_cmd_complete(struct usb_ep *ep, struct usb_request *req)if (req->status <0 )return ; - ret =usbg_submit_command (fu, req->buf, req->actual); + ret =usbg_submit_command (fu, req);/* * Once we tune for performance enqueue the command req here again so * we can receive a second command while we processing this one. Pay
@@ -737,7 +742,7 @@ static void uasp_cmd_complete(struct usb_ep *ep, struct usb_request *req)*/ if (!ret)return ; -usb_ep_queue (fu->ep_cmd, fu->cmd.req,GFP_ATOMIC ); +usb_ep_queue (fu->ep_cmd, req,GFP_ATOMIC ); }static int uasp_alloc_stream_res (struct f_uas *fu,struct uas_stream *stream)
@@ -766,24 +771,24 @@ static int uasp_alloc_stream_res(struct f_uas *fu, struct uas_stream *stream)return -ENOMEM ; } -static int uasp_alloc_cmd (struct f_uas *fu) +static int uasp_alloc_cmd (struct f_uas *fu,int i) { - fu->cmd.req =usb_ep_alloc_request (fu->ep_cmd,GFP_KERNEL ); -if (!fu->cmd.req) + fu->cmd[i].req =usb_ep_alloc_request (fu->ep_cmd,GFP_KERNEL ); +if (!fu->cmd[i].req) goto err; - fu->cmd.buf =kmalloc (fu->ep_cmd->maxpacket,GFP_KERNEL ); -if (!fu->cmd.buf) + fu->cmd[i].buf =kmalloc (fu->ep_cmd->maxpacket,GFP_KERNEL ); +if (!fu->cmd[i].buf) goto err_buf; - fu->cmd.req->complete = uasp_cmd_complete; - fu->cmd.req->buf = fu->cmd.buf; - fu->cmd.req->length = fu->ep_cmd->maxpacket; - fu->cmd.req->context = fu; + fu->cmd[i].req->complete = uasp_cmd_complete; + fu->cmd[i].req->buf = fu->cmd[i].buf; + fu->cmd[i].req->length = fu->ep_cmd->maxpacket; + fu->cmd[i].req->context = fu;return 0 ; err_buf: -usb_ep_free_request (fu->ep_cmd, fu->cmd.req); +usb_ep_free_request (fu->ep_cmd, fu->cmd[i].req); err:return -ENOMEM ; }
@@ -792,26 +797,22 @@ static int uasp_prepare_reqs(struct f_uas *fu) {int ret;int i; -int max_streams; - -if (fu->flags &USBG_USE_STREAMS ) - max_streams =UASP_SS_EP_COMP_NUM_STREAMS ; -else - max_streams =1 ; -for (i =0 ; i < max_streams; i++) { +for (i =0 ; i <USBG_NUM_CMDS ; i++) { ret =uasp_alloc_stream_res (fu, &fu->stream[i]);if (ret) goto err_cleanup; } - ret =uasp_alloc_cmd (fu); -if (ret) - goto err_free_stream; +for (i =0 ; i <USBG_NUM_CMDS ; i++) { + ret =uasp_alloc_cmd (fu, i); +if (ret) + goto err_free_stream; - ret =usb_ep_queue (fu->ep_cmd, fu->cmd.req,GFP_ATOMIC ); -if (ret) - goto err_free_stream; + ret =usb_ep_queue (fu->ep_cmd, fu->cmd[i].req,GFP_ATOMIC ); +if (ret) + goto err_free_stream; + }return 0 ;
@@ -1081,10 +1082,9 @@ static struct usbg_cmd *usbg_get_cmd(struct f_uas *fu,static void usbg_release_cmd (struct se_cmd *); -static int usbg_submit_command (struct f_uas *fu, -void *cmdbuf,unsigned int len) +static int usbg_submit_command (struct f_uas *fu,struct usb_request *req) { -struct command_iu *cmd_iu = cmdbuf; +struct command_iu *cmd_iu = req->buf;struct usbg_cmd *cmd;struct usbg_tpg *tpg = fu->tpg;struct tcm_usbg_nexus *tv_nexus;
@@ -1114,8 +1114,7 @@ static int usbg_submit_command(struct f_uas *fu, }memcpy (cmd->cmd_buf, cmd_iu->cdb, cmd_len); - cmd->stream = &fu->stream[cmd->tag % -UASP_SS_EP_COMP_NUM_STREAMS ]; + cmd->stream = &fu->stream[cmd->tag %USBG_NUM_CMDS ];switch (cmd_iu->prio_attr &0x7 ) {case UAS_HEAD_TAG :
@@ -1137,6 +1136,7 @@ static int usbg_submit_command(struct f_uas *fu, } cmd->unpacked_lun =scsilun_to_int (&cmd_iu->lun); + cmd->req = req;INIT_WORK (&cmd->work, usbg_cmd_work);queue_work (tpg->workqueue, &cmd->work);
drivers/usb/gadget/function/tcm.h +6 -2
@@ -16,6 +16,8 @@#define UASP_SS_EP_COMP_LOG_STREAMS 5#define UASP_SS_EP_COMP_NUM_STREAMS (1 << UASP_SS_EP_COMP_LOG_STREAMS) +#define USBG_NUM_CMDS UASP_SS_EP_COMP_NUM_STREAMS +enum {USB_G_STR_INT_UAS =0 ,USB_G_STR_INT_BBB ,
@@ -75,6 +77,8 @@ struct usbg_cmd {struct completion write_complete;struct kref ref; +struct usb_request *req; +/* UAS only */ u16 tag;u16 prio_attr;
@@ -117,14 +121,14 @@ struct f_uas {#define USBG_IS_BOT (1 << 3)#define USBG_BOT_CMD_PEND (1 << 4) -struct usbg_cdb cmd; +struct usbg_cdb cmd[USBG_NUM_CMDS ];struct usb_ep *ep_in;struct usb_ep *ep_out;/* UAS */ struct usb_ep *ep_status;struct usb_ep *ep_cmd; -struct uas_stream stream[UASP_SS_EP_COMP_NUM_STREAMS ]; +struct uas_stream stream[USBG_NUM_CMDS ];/* BOT */ struct bot_status bot_status;
Signature · 1 line
2.28.0
PATCH v2 7/25 usb: gadget: f_tcm: Limit number of sessions
In reply to Thinh NguyenOnly allocate up to USBG_NUM_CMDS number of session tags. We should not be using more than USBG_NUM_CMDS of tags due to the number of commands limit we imposed. Each command uses a unique tag. Any more than that is unnecessary. By limitting it, we can detect an issue in our driver immediately should we run out of session tags.
---
Changes in v2:
- Add more comments in change log for the change reason.
drivers/usb/gadget/function/tcm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/gadget/function/tcm.h +1 -1
@@ -26,7 +26,7 @@ enum {#define USB_G_ALT_INT_BBB 0#define USB_G_ALT_INT_UAS 1 -#define USB_G_DEFAULT_SESSION_TAGS 128 +#define USB_G_DEFAULT_SESSION_TAGS USBG_NUM_CMDSstruct tcm_usbg_nexus {struct se_session *tvn_se_sess;
Signature · 1 line
2.28.0
PATCH v2 8/25 usb: gadget: f_tcm: Handle multiple commands in parallel
In reply to Thinh NguyenResubmit command on completion to fetch more commands and service them in parallel. Increase the number of work in a queue. Each work will be for each command allowing them to be processed concurrently. Also, set them to be unbounded by cpu to improve performance.
---
Changes in v2:
- None
drivers/usb/gadget/function/f_tcm.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-)
drivers/usb/gadget/function/f_tcm.c +8 -12
@@ -728,21 +728,16 @@ static int usbg_submit_command(struct f_uas *, struct usb_request *);static void uasp_cmd_complete (struct usb_ep *ep,struct usb_request *req) {struct f_uas *fu = req->context; -int ret; -if (req->status <0 ) +if (req->status == -ESHUTDOWN )return ; - ret =usbg_submit_command (fu, req); -/* -* Once we tune for performance enqueue the command req here again so -* we can receive a second command while we processing this one. Pay -* attention to properly sync STAUS endpoint with DATA IN + OUT so you -* don't break HS. -*/ -if (!ret) +if (req->status <0 ) { +usb_ep_queue (fu->ep_cmd, req,GFP_ATOMIC );return ; -usb_ep_queue (fu->ep_cmd, req,GFP_ATOMIC ); + } + +usbg_submit_command (fu, req); }static int uasp_alloc_stream_res (struct f_uas *fu,struct uas_stream *stream)
@@ -1357,7 +1352,8 @@ static struct se_portal_group *usbg_make_tpg(struct se_wwn *wwn, goto unref_dep;mutex_init (&tpg->tpg_mutex);atomic_set (&tpg->tpg_port_count,0 ); - tpg->workqueue =alloc_workqueue ("tcm_usb_gadget" ,0 ,1 ); + tpg->workqueue =alloc_workqueue ("tcm_usb_gadget" , +WQ_UNBOUND ,WQ_UNBOUND_MAX_ACTIVE );if (!tpg->workqueue) goto free_tpg;
Signature · 1 line
2.28.0
PATCH v2 9/25 usb: gadget: f_tcm: Use extra number of commands
In reply to Thinh NguyenTo properly respond to host sending more commands than the number of streams the device advertises, the device needs to be able to reject the command with a response. Allocate an extra request to handle 1 more command than the number of streams.
---
Changes in v2:
- None
drivers/usb/gadget/function/tcm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/gadget/function/tcm.h +1 -1
@@ -16,7 +16,7 @@#define UASP_SS_EP_COMP_LOG_STREAMS 5#define UASP_SS_EP_COMP_NUM_STREAMS (1 << UASP_SS_EP_COMP_LOG_STREAMS) -#define USBG_NUM_CMDS UASP_SS_EP_COMP_NUM_STREAMS +#define USBG_NUM_CMDS (UASP_SS_EP_COMP_NUM_STREAMS + 1)enum {USB_G_STR_INT_UAS =0 ,
Signature · 1 line
2.28.0
PATCH v2 10/25 usb: gadget: f_tcm: Return ATA cmd direction
In reply to Thinh NguyenCheck ATA Pass-Through for direction.
---
Changes in v2:
- None
drivers/usb/gadget/function/f_tcm.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/usb/gadget/function/f_tcm.c +2 -0
@@ -898,6 +898,8 @@ static int get_cmd_dir(const unsigned char *cdb)case READ_TOC :case READ_FORMAT_CAPACITIES :case REQUEST_SENSE : +case ATA_12 : +case ATA_16 : ret =DMA_FROM_DEVICE ;break ;
Signature · 1 line
2.28.0
PATCH v2 11/25 usb: gadget: f_tcm: Execute command on write completion
In reply to Thinh NguyenDon't just wait for the data write completion and execute the target command. We need to verify if the request completed successfully and not just sending invalid data. The verification is done in the write request completion routine, so we can just run target_execute_cmd() there. The wait for the data write is not needed.
---
Changes in v2:
- None
drivers/usb/gadget/function/f_tcm.c | 8 +------- drivers/usb/gadget/function/tcm.h | 1 - 2 files changed, 1 insertion(+), 8 deletions(-)
drivers/usb/gadget/function/f_tcm.c +1 -7
@@ -248,7 +248,6 @@ static int bot_send_write_request(struct usbg_cmd *cmd)struct usb_gadget *gadget =fuas_to_gadget (fu);int ret; -init_completion (&cmd->write_complete); cmd->fu = fu;if (!cmd->data_len) {
@@ -279,8 +278,6 @@ static int bot_send_write_request(struct usbg_cmd *cmd)if (ret)pr_err ("%s(%d)\n" , __func__, __LINE__); -wait_for_completion (&cmd->write_complete); -target_execute_cmd (se_cmd); cleanup:return ret; }
@@ -685,7 +682,6 @@ static int uasp_send_write_request(struct usbg_cmd *cmd)struct sense_iu *iu = &cmd->sense_iu;int ret; -init_completion (&cmd->write_complete); cmd->fu = fu; iu->tag =cpu_to_be16 (cmd->tag);
@@ -717,8 +713,6 @@ static int uasp_send_write_request(struct usbg_cmd *cmd)pr_err ("%s(%d)\n" , __func__, __LINE__); } -wait_for_completion (&cmd->write_complete); -target_execute_cmd (se_cmd); cleanup:return ret; }
@@ -955,7 +949,7 @@ static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req) se_cmd->data_length); } -complete (&cmd->write_complete); +target_execute_cmd (se_cmd);return ; cleanup:
drivers/usb/gadget/function/tcm.h +0 -1
@@ -74,7 +74,6 @@ struct usbg_cmd {struct se_cmd se_cmd;void *data_buf;/* used if no sg support available */ struct f_uas *fu; -struct completion write_complete;struct kref ref;struct usb_request *req;
Signature · 1 line
2.28.0
Re: [PATCH v2 11/25] usb: gadget: f_tcm: Execute command on write completion
drivers/usb/gadget/function/f_tcm.c static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req) · line 952
Quoted 9 lines · Thinh Nguyen
index
6fea80afe2d7..ec83f2f9a858100644drivers/usb/gadget/function/f_tcm.c +1
-1@@ -955,7 +949,7 @@ static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req) se_cmd->data_length); } -complete (&cmd->write_complete); +target_execute_cmd (se_cmd);
usbg_data_write_cmpl() is invoked from interrupt service routing which may run with disabled interrupts. From looking at target_execute_cmd():
| void target_execute_cmd(struct se_cmd *cmd)
| {
…
| spin_lock_irq(&cmd->t_state_lock);
…
| spin_unlock_irq(&cmd->t_state_lock);
…
| }
which means interrupts will remain open after leaving target_execute_cmd(). Now, why didn't the WARN_ONCE() in __handle_irq_event_percpu() trigger? Am I missing something?
drivers/usb/gadget/function/f_tcm.c static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req) · line 953
Quoted 1 line
drivers/usb/gadget/function/f_tcm.c +0
-0return ;
Sebastian
Re: [PATCH v2 11/25] usb: gadget: f_tcm: Execute command on write completion
Quoted 8 lines · Sebastian Andrzej Siewior
Quoted 7 lines · Thinh Nguyen
index
6fea80afe2d7..ec83f2f9a858100644drivers/usb/gadget/function/f_tcm.c +0
-0@@ -955,7 +949,7 @@ static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req) se_cmd->data_length); }
Quoted 5 lines
drivers/usb/gadget/function/f_tcm.c static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req) · line 952
Quoted 2 lines
drivers/usb/gadget/function/f_tcm.c +1
-1-complete (&cmd->write_complete); +target_execute_cmd (se_cmd);
usbg_data_write_cmpl()is invoked from interrupt service routing which may run with disabled interrupts. From looking attarget_execute_cmd():
It will always be called with interrupts disabled as documented in usb_request API.
Quoted 9 lines
| void
target_execute_cmd(struct se_cmd *cmd)
| {
…
|spin_lock_irq(&cmd->t_state_lock);
…
|spin_unlock_irq(&cmd->t_state_lock);
…
| }
Quoted 6 lines
which means interrupts will remain open after leaving
target_execute_cmd(). Now, why didn't theWARN_ONCE()in__handle_irq_event_percpu()trigger? Am I missing something?Quoted 1 line
drivers/usb/gadget/function/f_tcm.c +0
-0return ;
Since target_execute_cmd() is called in usbg_data_write_cmpl(), interrupts are still disabled.
Thanks,
Thinh
Re: [PATCH v2 11/25] usb: gadget: f_tcm: Execute command on write completion
Quoted 31 lines · Thinh Nguyen
Quoted 8 lines · Sebastian Andrzej Siewior
Quoted 7 lines · Thinh Nguyen
index
6fea80afe2d7..ec83f2f9a858100644drivers/usb/gadget/function/f_tcm.c +0
-0@@ -955,7 +949,7 @@ static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req) se_cmd->data_length); }Quoted 5 lines
drivers/usb/gadget/function/f_tcm.c static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req) · line 952
Quoted 2 lines
drivers/usb/gadget/function/f_tcm.c +1
-1-complete (&cmd->write_complete); +target_execute_cmd (se_cmd);
usbg_data_write_cmpl()is invoked from interrupt service routing which may run with disabled interrupts. From looking attarget_execute_cmd():It will always be called with interrupts disabled as documented in
usb_requestAPI.Quoted 13 lines
| void
target_execute_cmd(struct se_cmd *cmd)
| {
…
|spin_lock_irq(&cmd->t_state_lock);
…
|spin_unlock_irq(&cmd->t_state_lock);
…
| }which means interrupts will remain open after leaving
target_execute_cmd(). Now, why didn't theWARN_ONCE()in__handle_irq_event_percpu()trigger? Am I missing something?
Quoted 5 lines
Quoted 2 lines
drivers/usb/gadget/function/f_tcm.c static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req) · line 953
Quoted 1 line
drivers/usb/gadget/function/f_tcm.c +0
-0return ;Since
target_execute_cmd()is called inusbg_data_write_cmpl(), interrupts are still disabled.
but you do realize that target_execute_cmd() will leave with enabled interrupts and this is not desired? I _think_ this was the reason why I ended up with the wait+complete construct instead of invoking this function directly.
An _irqsave() in target_execute_cmd() would probably be all you need here.
Quoted 2 lines
Thanks,
Thinh
Sebastian
Re: [PATCH v2 11/25] usb: gadget: f_tcm: Execute command on write completion
Quoted 38 lines · Sebastian Andrzej Siewior
Quoted 31 lines · Thinh Nguyen
Quoted 8 lines · Sebastian Andrzej Siewior
Quoted 7 lines · Thinh Nguyen
index
6fea80afe2d7..ec83f2f9a858100644drivers/usb/gadget/function/f_tcm.c +0
-0@@ -955,7 +949,7 @@ static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req) se_cmd->data_length); }Quoted 5 lines
drivers/usb/gadget/function/f_tcm.c static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req) · line 952
Quoted 2 lines
drivers/usb/gadget/function/f_tcm.c +1
-1-complete (&cmd->write_complete); +target_execute_cmd (se_cmd);
usbg_data_write_cmpl()is invoked from interrupt service routing which may run with disabled interrupts. From looking attarget_execute_cmd():It will always be called with interrupts disabled as documented in
usb_requestAPI.Quoted 13 lines
| void
target_execute_cmd(struct se_cmd *cmd)
| {
…
|spin_lock_irq(&cmd->t_state_lock);
…
|spin_unlock_irq(&cmd->t_state_lock);
…
| }which means interrupts will remain open after leaving
target_execute_cmd(). Now, why didn't theWARN_ONCE()in__handle_irq_event_percpu()trigger? Am I missing something?Quoted 5 lines
Quoted 2 lines
drivers/usb/gadget/function/f_tcm.c static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req) · line 953
Quoted 1 line
drivers/usb/gadget/function/f_tcm.c +0
-0return ;Since
target_execute_cmd()is called inusbg_data_write_cmpl(), interrupts are still disabled.
Quoted 7 lines
but you do realize that
target_execute_cmd()will leave with enabled interrupts and this is not desired? I _think_ this was the reason why I ended up with the wait+complete construct instead of invoking this function directly.
An_irqsave()intarget_execute_cmd()would probably be all you need here.
Ok. Maybe we should make a change in the target_execute_cmd() then. It seems unreasonable to force the caller to workaround this such as the wait+complete construct you did (and I don't recall we have changes in place to know/guarantee that interrupts are enabled before executing target_execute_cmd() previously either).
For the dwc3, we masked the interrupt at this point, so interrupt won't be asserted here.
Thanks,
Thinh
Re: [PATCH v2 11/25] usb: gadget: f_tcm: Execute command on write completion
Quoted 5 lines · Thinh Nguyen
Ok. Maybe we should make a change in the
target_execute_cmd()then. It seems unreasonable to force the caller to workaround this such as the wait+complete construct you did (and I don't recall we have changes in place to know/guarantee that interrupts are enabled before executingtarget_execute_cmd()previously either).
Sounds reasonable. Back then I wasn't sure if I'm putting all the puzzle pieces correctly together so I preferred this over a target change I wasn't sure was really needed. Anyway.
Quoted 2 lines
For the dwc3, we masked the interrupt at this point, so interrupt won't be asserted here.
dwc3 has a irqrestore() after calling the routine so that will avoid the splat. But lockdep should yell here.
Anyway, other interrupts on that CPU (timer for instance) could trigger.
Quoted 2 lines
Thanks,
Thinh
Sebastian
PATCH v2 12/25 usb: gadget: f_tcm: Minor cleanup redundant code
In reply to Thinh NguyenThe status request preparation is done in uasp_prepare_status(). Remove duplicate code. No functional change here.
---
Changes in v2:
- None
drivers/usb/gadget/function/f_tcm.c | 2 -- 1 file changed, 2 deletions(-)
drivers/usb/gadget/function/f_tcm.c +0 -2
@@ -624,8 +624,6 @@ static int uasp_send_status_response(struct usbg_cmd *cmd)struct sense_iu *iu = &cmd->sense_iu; iu->tag =cpu_to_be16 (cmd->tag); - stream->req_status->complete = uasp_status_data_cmpl; - stream->req_status->context = cmd; cmd->fu = fu;uasp_prepare_status (cmd);return usb_ep_queue (fu->ep_status, stream->req_status,GFP_ATOMIC );
Signature · 1 line
2.28.0
PATCH v2 13/25 usb: gadget: f_tcm: Don't free command immediately
In reply to Thinh NguyenDon't prematurely free the command. Wait for the status completion of the sense status. It can be freed then. Otherwise we will double-free the command.
---
Changes in v2:
- None
drivers/usb/gadget/function/f_tcm.c | 1 - 1 file changed, 1 deletion(-)
drivers/usb/gadget/function/f_tcm.c +0 -1
@@ -1045,7 +1045,6 @@ static void usbg_cmd_work(struct work_struct *work) out:transport_send_check_condition_and_sense (se_cmd,TCM_UNSUPPORTED_SCSI_OPCODE ,1 ); -transport_generic_free_cmd (&cmd->se_cmd,0 ); } static struct usbg_cmd *usbg_get_cmd(struct f_uas *fu,
Signature · 1 line
2.28.0
PATCH v2 14/25 usb: gadget: f_tcm: Translate error to sense
In reply to Thinh NguyenWhen respond with check_condition error status, clear from_transport input so the target layer can translate the sense reason reported by f_tcm.
---
Changes in v2:
- None
drivers/usb/gadget/function/f_tcm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/usb/gadget/function/f_tcm.c +2 -2
@@ -1044,7 +1044,7 @@ static void usbg_cmd_work(struct work_struct *work) out:transport_send_check_condition_and_sense (se_cmd, -TCM_UNSUPPORTED_SCSI_OPCODE ,1 ); +TCM_UNSUPPORTED_SCSI_OPCODE ,0 ); } static struct usbg_cmd *usbg_get_cmd(struct f_uas *fu,
@@ -1160,7 +1160,7 @@ static void bot_cmd_work(struct work_struct *work) out:transport_send_check_condition_and_sense (se_cmd, -TCM_UNSUPPORTED_SCSI_OPCODE ,1 ); +TCM_UNSUPPORTED_SCSI_OPCODE ,0 );transport_generic_free_cmd (&cmd->se_cmd,0 ); }
Signature · 1 line
2.28.0
PATCH v2 15/25 usb: gadget: f_tcm: Cleanup unused variable
In reply to Thinh Nguyense_cmd is not used anywhere. Remove it.
---
Changes in v2:
- None
drivers/usb/gadget/function/f_tcm.c | 1 - 1 file changed, 1 deletion(-)
drivers/usb/gadget/function/f_tcm.c +0 -1
@@ -675,7 +675,6 @@ static int uasp_send_read_response(struct usbg_cmd *cmd)static int uasp_send_write_request (struct usbg_cmd *cmd) {struct f_uas *fu = cmd->fu; -struct se_cmd *se_cmd = &cmd->se_cmd;struct uas_stream *stream = cmd->stream;struct sense_iu *iu = &cmd->sense_iu;int ret;
Signature · 1 line
2.28.0
Re: [PATCH v2 15/25] usb: gadget: f_tcm: Cleanup unused variable
Quoted 1 line · Thinh Nguyen
se_cmdis not used anywhere. Remove it.
This should folded into the original commit where se_cmd has stopped been using. I believe it was when target_execute_cmd() has been moved.
Quoted 1 line
Signed-off-by Thinh Nguyen <[email protected]>
Sebastian
Re: [PATCH v2 15/25] usb: gadget: f_tcm: Cleanup unused variable
Quoted 5 lines · Sebastian Andrzej Siewior
Quoted 1 line · Thinh Nguyen
se_cmdis not used anywhere. Remove it.This should folded into the original commit where
se_cmdhas stopped been using. I believe it was whentarget_execute_cmd()has been moved.
Right, I missed that.
Thanks,
Thinh
Quoted 4 lines
Quoted 2 lines
Signed-off-by Thinh Nguyen <[email protected]>Sebastian
PATCH v2 16/25 usb: gadget: f_tcm: Update state on data write
In reply to Thinh NguyenWhen preparing request for data write state, the next state is UASP_SEND_STATUS. When data write completes, the next state is UASP_QUEUE_COMMAND. Without this change, the command will transition to the wrong state.
---
Changes in v2:
- Move a related change from TASK MANAGEMENT updating cmd state to UASP_QUEUE_COMMAND to here.
drivers/usb/gadget/function/f_tcm.c | 4 ++++ 1 file changed, 4 insertions(+)
drivers/usb/gadget/function/f_tcm.c +4 -0
@@ -934,6 +934,8 @@ static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req)struct usbg_cmd *cmd = req->context;struct se_cmd *se_cmd = &cmd->se_cmd; + cmd->state =UASP_QUEUE_COMMAND ; +if (req->status <0 ) {pr_err ("%s() state %d transfer failed\n" , __func__, cmd->state); goto cleanup;
@@ -976,6 +978,8 @@ static int usbg_prepare_w_request(struct usbg_cmd *cmd, struct usb_request *req) req->complete = usbg_data_write_cmpl; req->length = se_cmd->data_length; req->context = cmd; + + cmd->state =UASP_SEND_STATUS ;return 0 ; }
Signature · 1 line
2.28.0
Re: [PATCH v2 16/25] usb: gadget: f_tcm: Update state on data write
Quoted 4 lines · Thinh Nguyen
When preparing request for data write state, the next state is
UASP_SEND_STATUS. When data write completes, the next state isUASP_QUEUE_COMMAND. Without this change, the command will transition to the wrong state.
Why is this needed now, what is the outcome of not having it? My point is, was this always broken, worked by chance and broke over time while code was changed?
Quoted 23 lines
Signed-off-by Thinh Nguyen <[email protected]>---
Changes in v2:
- Move a related change from TASK MANAGEMENT updating cmd state toUASP_QUEUE_COMMANDto here.drivers/usb/gadget/function/f_tcm.c | 4 ++++ 1 file changed, 4 insertions(+)drivers/usb/gadget/function/f_tcm.c +2
-0@@ -934,6 +934,8 @@ static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req)struct usbg_cmd *cmd = req->context;struct se_cmd *se_cmd = &cmd->se_cmd; + cmd->state =UASP_QUEUE_COMMAND ; +if (req->status <0 ) {pr_err ("%s() state %d transfer failed\n" , __func__, cmd->state); goto cleanup;
drivers/usb/gadget/function/f_tcm.c static int usbg_prepare_w_request(struct usbg_cmd *cmd, struct usb_request *req) · line 985
Quoted 8 lines
drivers/usb/gadget/function/f_tcm.c +2
-0@@ -976,6 +978,8 @@ static int usbg_prepare_w_request(struct usbg_cmd *cmd, struct usb_request *req) req->complete = usbg_data_write_cmpl; req->length = se_cmd->data_length; req->context = cmd; + + cmd->state =UASP_SEND_STATUS ;return 0 ; }
Sebastian
Re: [PATCH v2 16/25] usb: gadget: f_tcm: Update state on data write
Quoted 9 lines · Sebastian Andrzej Siewior
Quoted 4 lines · Thinh Nguyen
When preparing request for data write state, the next state is
UASP_SEND_STATUS. When data write completes, the next state isUASP_QUEUE_COMMAND. Without this change, the command will transition to the wrong state.Why is this needed now, what is the outcome of not having it? My point is, was this always broken, worked by chance and broke over time while code was changed?
This patch should've been part of the patch "usb: gadget: f_tcm: Execute command on write completion", and maybe I should've been more clear in the commmit message.
Previously, the f_tcm doesn't take care of the case where the queued data could fail/aborted. It attempts to execute the command anyway.
This change here takes care of both scenarios. If the request fails, the next state is to send status, and if the request completes successfully, the next state is to queue a new command.
Thanks,
Thinh
Quoted 34 lines
Quoted 24 lines
Signed-off-by Thinh Nguyen <[email protected]>---
Changes in v2:
- Move a related change from TASK MANAGEMENT updating cmd state toUASP_QUEUE_COMMANDto here.drivers/usb/gadget/function/f_tcm.c | 4 ++++ 1 file changed, 4 insertions(+)drivers/usb/gadget/function/f_tcm.c +2
-0@@ -934,6 +934,8 @@ static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req)struct usbg_cmd *cmd = req->context;struct se_cmd *se_cmd = &cmd->se_cmd; + cmd->state =UASP_QUEUE_COMMAND ; +if (req->status <0 ) {pr_err ("%s() state %d transfer failed\n" , __func__, cmd->state); goto cleanup;drivers/usb/gadget/function/f_tcm.c static int usbg_prepare_w_request(struct usbg_cmd *cmd, struct usb_request *req) · line 985
Quoted 8 lines
drivers/usb/gadget/function/f_tcm.c +2
-0@@ -976,6 +978,8 @@ static int usbg_prepare_w_request(struct usbg_cmd *cmd, struct usb_request *req) req->complete = usbg_data_write_cmpl; req->length = se_cmd->data_length; req->context = cmd; + + cmd->state =UASP_SEND_STATUS ;return 0 ; }Sebastian
PATCH v2 17/25 usb: gadget: f_tcm: Handle abort command
In reply to Thinh NguyenImplemented aborted_task to cancel outstanding request.
---
Changes in v2:
- None
drivers/usb/gadget/function/f_tcm.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
drivers/usb/gadget/function/f_tcm.c +16 -0
@@ -1278,6 +1278,22 @@ static void usbg_queue_tm_rsp(struct se_cmd *se_cmd)static void usbg_aborted_task (struct se_cmd *se_cmd) { + struct usbg_cmd *cmd = container_of(se_cmd, struct usbg_cmd, se_cmd); +struct f_uas *fu = cmd->fu; +struct uas_stream *stream = cmd->stream; +int ret =0 ; + +if (stream->req_out->status == -EINPROGRESS ) + ret =usb_ep_dequeue (fu->ep_out, stream->req_out); +else if (stream->req_in->status == -EINPROGRESS ) + ret =usb_ep_dequeue (fu->ep_in, stream->req_in); +else if (stream->req_status->status == -EINPROGRESS ) + ret =usb_ep_dequeue (fu->ep_status, stream->req_status); + +if (ret) +pr_err ("Unable to dequeue se_cmd out %p\n" , se_cmd); + + cmd->state =UASP_QUEUE_COMMAND ; }static const char *usbg_check_wwn (const char *name)
Signature · 1 line
2.28.0
Re: [PATCH v2 17/25] usb: gadget: f_tcm: Handle abort command
Quoted 11 lines · Thinh Nguyen
drivers/usb/gadget/function/f_tcm.c +4
-0@@ -1278,6 +1278,22 @@ static void usbg_queue_tm_rsp(struct se_cmd *se_cmd)static void usbg_aborted_task (struct se_cmd *se_cmd) { + struct usbg_cmd *cmd = container_of(se_cmd, struct usbg_cmd, se_cmd); +struct f_uas *fu = cmd->fu; +struct uas_stream *stream = cmd->stream; +int ret =0 ;
drivers/usb/gadget/function/f_tcm.c static void usbg_queue_tm_rsp(struct se_cmd *se_cmd) · line 1294
Quoted 10 lines
drivers/usb/gadget/function/f_tcm.c +10
-0+ +if (stream->req_out->status == -EINPROGRESS ) + ret =usb_ep_dequeue (fu->ep_out, stream->req_out); +else if (stream->req_in->status == -EINPROGRESS ) + ret =usb_ep_dequeue (fu->ep_in, stream->req_in); +else if (stream->req_status->status == -EINPROGRESS ) + ret =usb_ep_dequeue (fu->ep_status, stream->req_status); + +if (ret) +pr_err ("Unable to dequeue se_cmd out %p\n" , se_cmd);
I know I wasn't the best example here. But if you continue to work on that one, if would be nice if you could replace all the pr_err() here with a dev_err() (or so) so it is bound to an actual device and the reader can actually pin point the message to the actually device/ subsystem. I'm not sure if we lack a device or I was extremely lazy. Again, not something you need change now.
drivers/usb/gadget/function/f_tcm.c static void usbg_queue_tm_rsp(struct se_cmd *se_cmd) · line 1299
Quoted 4 lines
drivers/usb/gadget/function/f_tcm.c +1
-0+ cmd->state =UASP_QUEUE_COMMAND ; }static const char *usbg_check_wwn (const char *name)
Sebastian
Re: [PATCH v2 17/25] usb: gadget: f_tcm: Handle abort command
Quoted 21 lines · Sebastian Andrzej Siewior
Quoted 20 lines · Thinh Nguyen
drivers/usb/gadget/function/f_tcm.c +13
-0@@ -1278,6 +1278,22 @@ static void usbg_queue_tm_rsp(struct se_cmd *se_cmd)static void usbg_aborted_task (struct se_cmd *se_cmd) { + struct usbg_cmd *cmd = container_of(se_cmd, struct usbg_cmd, se_cmd); +struct f_uas *fu = cmd->fu; +struct uas_stream *stream = cmd->stream; +int ret =0 ; + +if (stream->req_out->status == -EINPROGRESS ) + ret =usb_ep_dequeue (fu->ep_out, stream->req_out); +else if (stream->req_in->status == -EINPROGRESS ) + ret =usb_ep_dequeue (fu->ep_in, stream->req_in); +else if (stream->req_status->status == -EINPROGRESS ) + ret =usb_ep_dequeue (fu->ep_status, stream->req_status); + +if (ret)
Quoted 8 lines
drivers/usb/gadget/function/f_tcm.c static void usbg_queue_tm_rsp(struct se_cmd *se_cmd) · line 1294
Quoted 1 line
drivers/usb/gadget/function/f_tcm.c +1
-0+pr_err ("Unable to dequeue se_cmd out %p\n" , se_cmd);I know I wasn't the best example here. But if you continue to work on that one, if would be nice if you could replace all the
pr_err()here with adev_err()(or so) so it is bound to an actual device and the reader can actually pin point the message to the actually device/ subsystem. I'm not sure if we lack a device or I was extremely lazy. Again, not something you need change now.
Yes. I noticed that too. However, I think that can be done later as it doesn't seem critical. I tried to keep the code consistent for now.
Thanks,
Thinh
Quoted 7 lines
drivers/usb/gadget/function/f_tcm.c static void usbg_queue_tm_rsp(struct se_cmd *se_cmd) · line 1299
Quoted 5 lines
drivers/usb/gadget/function/f_tcm.c +1
-0+ cmd->state =UASP_QUEUE_COMMAND ; }static const char *usbg_check_wwn (const char *name)Sebastian
PATCH v2 18/25 usb: gadget: f_tcm: Cleanup requests on ep disable
In reply to Thinh NguyenIf the endpoint is disabled, then free the command. Normally we don't free a command until it's completed its UASP status (failure or not).
---
Changes in v2:
- Use goto cleanup for a cleaner look.
drivers/usb/gadget/function/f_tcm.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/usb/gadget/function/f_tcm.c +5 -2
@@ -572,7 +572,7 @@ static void uasp_status_data_cmpl(struct usb_ep *ep, struct usb_request *req)struct f_uas *fu = cmd->fu;int ret; -if (req->status <0 ) +if (req->status == -ESHUTDOWN ) goto cleanup;switch (cmd->state) {
@@ -936,7 +936,10 @@ static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req) cmd->state =UASP_QUEUE_COMMAND ; -if (req->status <0 ) { +if (req->status == -ESHUTDOWN ) + goto cleanup; + +if (req->status) {pr_err ("%s() state %d transfer failed\n" , __func__, cmd->state); goto cleanup; }
Signature · 1 line
2.28.0
PATCH v2 19/25 usb: gadget: f_tcm: Decrement command ref count on cleanup
In reply to Thinh NguyenWe submitted the command with TARGET_SCF_ACK_KREF, which requires acknowledgment of command completion. If the command fails, make sure to decrement the ref count.
---
Changes in v2:
- New patch
drivers/usb/gadget/function/f_tcm.c | 1 + 1 file changed, 1 insertion(+)
drivers/usb/gadget/function/f_tcm.c +1 -0
@@ -955,6 +955,7 @@ static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req)return ; cleanup: +target_put_sess_cmd (se_cmd);transport_generic_free_cmd (&cmd->se_cmd,0 ); }
Signature · 1 line
2.28.0
PATCH v2 20/25 usb: gadget: f_tcm: Save CPU ID per command
In reply to Thinh NguyenNormally we don't care about the CPU id, but if we ever use TARGET_SCF_USE_CPUID, then we need to save the cpuid.
---
Changes in v2:
- None
drivers/usb/gadget/function/f_tcm.c | 1 + 1 file changed, 1 insertion(+)
drivers/usb/gadget/function/f_tcm.c +1 -0
@@ -1069,6 +1069,7 @@ static struct usbg_cmd *usbg_get_cmd(struct f_uas *fu,memset (cmd,0 ,sizeof (*cmd)); cmd->se_cmd.map_tag = tag; cmd->se_cmd.map_cpu = cpu; + cmd->se_cmd.cpuid = cpu; cmd->se_cmd.tag = cmd->tag = scsi_tag; cmd->fu = fu;
Signature · 1 line
2.28.0
PATCH v2 21/25 usb: gadget: f_tcm: Get stream by tag
In reply to Thinh NguyenIn preparation for TASK MANAGEMENT handling of overlapped command, implement uasp_get_stream_by_tag() to find the active stream/command based on a given tag.
For simplicity, we use mod operation to quickly find an in-progress matching command tag to check for overlapped command. The assumption is that the UASP class driver will limit to using tag id from 1 to USBG_NUM_CMDS. This is based on observation from the Windows and Linux UASP storage class driver behavior. If an unusual UASP class driver uses a tag greater than USBG_NUM_CMDS, then this method may no longer work due to possible stream id collision. In that case, we need to use a proper algorithm to fetch the stream (or simply walk through all active streams to check for overlap).
---
Changes in v2:
- Splitted from TASK MANAGEMENT change
drivers/usb/gadget/function/f_tcm.c | 30 ++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-)
drivers/usb/gadget/function/f_tcm.c +23 -7
@@ -506,6 +506,22 @@ static void uasp_cleanup_old_alt(struct f_uas *fu)uasp_free_cmdreq (fu); } +static struct uas_stream *uasp_get_stream_by_tag (struct f_uas *fu,u16 tag) +{ +/* +* For simplicity, we use mod operation to quickly find an in-progress +* matching command tag to check for overlapped command. The assumption +* is that the UASP class driver will limit to using tag id from 1 to +* USBG_NUM_CMDS. This is based on observation from the Windows and +* Linux UASP storage class driver behavior. If an unusual UASP class +* driver uses a tag greater than USBG_NUM_CMDS, then this method may no +* longer work due to possible stream id collision. In that case, we +* need to use a proper algorithm to fetch the stream (or simply walk +* through all active streams to check for overlap). +*/ +return &fu->stream[tag %USBG_NUM_CMDS ]; +} +static void uasp_status_data_cmpl (struct usb_ep *ep,struct usb_request *req);static int uasp_prepare_r_request (struct usbg_cmd *cmd)
@@ -513,7 +529,7 @@ static int uasp_prepare_r_request(struct usbg_cmd *cmd)struct se_cmd *se_cmd = &cmd->se_cmd;struct f_uas *fu = cmd->fu;struct usb_gadget *gadget =fuas_to_gadget (fu); -struct uas_stream *stream = cmd->stream; +struct uas_stream *stream =uasp_get_stream_by_tag (fu, cmd->tag);if (!gadget->sg_supported) { cmd->data_buf =kmalloc (se_cmd->data_length,GFP_ATOMIC );
@@ -546,7 +562,7 @@ static void uasp_prepare_status(struct usbg_cmd *cmd) {struct se_cmd *se_cmd = &cmd->se_cmd;struct sense_iu *iu = &cmd->sense_iu; -struct uas_stream *stream = cmd->stream; +struct uas_stream *stream =uasp_get_stream_by_tag (cmd->fu, cmd->tag); cmd->state =UASP_QUEUE_COMMAND ; iu->iu_id =IU_ID_STATUS ;
@@ -568,8 +584,8 @@ static void uasp_prepare_status(struct usbg_cmd *cmd)static void uasp_status_data_cmpl (struct usb_ep *ep,struct usb_request *req) {struct usbg_cmd *cmd = req->context; -struct uas_stream *stream = cmd->stream;struct f_uas *fu = cmd->fu; +struct uas_stream *stream =uasp_get_stream_by_tag (fu, cmd->tag);int ret;if (req->status == -ESHUTDOWN )
@@ -620,7 +636,7 @@ static void uasp_status_data_cmpl(struct usb_ep *ep, struct usb_request *req)static int uasp_send_status_response (struct usbg_cmd *cmd) {struct f_uas *fu = cmd->fu; -struct uas_stream *stream = cmd->stream; +struct uas_stream *stream =uasp_get_stream_by_tag (fu, cmd->tag);struct sense_iu *iu = &cmd->sense_iu; iu->tag =cpu_to_be16 (cmd->tag);
@@ -632,7 +648,7 @@ static int uasp_send_status_response(struct usbg_cmd *cmd)static int uasp_send_read_response (struct usbg_cmd *cmd) {struct f_uas *fu = cmd->fu; -struct uas_stream *stream = cmd->stream; +struct uas_stream *stream =uasp_get_stream_by_tag (fu, cmd->tag);struct sense_iu *iu = &cmd->sense_iu;int ret;
@@ -675,7 +691,7 @@ static int uasp_send_read_response(struct usbg_cmd *cmd)static int uasp_send_write_request (struct usbg_cmd *cmd) {struct f_uas *fu = cmd->fu; -struct uas_stream *stream = cmd->stream; +struct uas_stream *stream =uasp_get_stream_by_tag (fu, cmd->tag);struct sense_iu *iu = &cmd->sense_iu;int ret;
@@ -1285,7 +1301,7 @@ static void usbg_aborted_task(struct se_cmd *se_cmd) { struct usbg_cmd *cmd = container_of(se_cmd, struct usbg_cmd, se_cmd);struct f_uas *fu = cmd->fu; -struct uas_stream *stream = cmd->stream; +struct uas_stream *stream =uasp_get_stream_by_tag (fu, cmd->tag);int ret =0 ;if (stream->req_out->status == -EINPROGRESS )
Signature · 1 line
2.28.0
Re: [PATCH v2 21/25] usb: gadget: f_tcm: Get stream by tag
Quoted 12 lines · Thinh Nguyen
drivers/usb/gadget/function/f_tcm.c +4
-0@@ -506,6 +506,22 @@ static void uasp_cleanup_old_alt(struct f_uas *fu)uasp_free_cmdreq (fu); } +static struct uas_stream *uasp_get_stream_by_tag (struct f_uas *fu,u16 tag) +{ +/* +* For simplicity, we use mod operation to quickly find an in-progress
drivers/usb/gadget/function/f_tcm.c static void uasp_cleanup_old_alt(struct f_uas *fu) · line 522
Quoted 10 lines
drivers/usb/gadget/function/f_tcm.c +10
-0+* matching command tag to check for overlapped command. The assumption +* is that the UASP class driver will limit to using tag id from 1 to +* USBG_NUM_CMDS. This is based on observation from the Windows and +* Linux UASP storage class driver behavior. If an unusual UASP class +* driver uses a tag greater than USBG_NUM_CMDS, then this method may no +* longer work due to possible stream id collision. In that case, we +* need to use a proper algorithm to fetch the stream (or simply walk +* through all active streams to check for overlap). +*/ +return &fu->stream[tag %USBG_NUM_CMDS ];
Could you please avoid the assumption what tag actually is? Please take a look at hashtable.h, hash_add(), hash_del(), hash_for_each_possible_safe() is probably all you need.
That % looks efficient but gcc will try and remove the div operation which is something the hash implementation (as of hash_min()) avoids. So the only additional costs here is the additional hashtable which worth the price given that you don't assume what tag can be.
Sebastian
Re: [PATCH v2 21/25] usb: gadget: f_tcm: Get stream by tag
Quoted 24 lines · Sebastian Andrzej Siewior
Quoted 12 lines · Thinh Nguyen
drivers/usb/gadget/function/f_tcm.c +4
-0@@ -506,6 +506,22 @@ static void uasp_cleanup_old_alt(struct f_uas *fu)uasp_free_cmdreq (fu); } +static struct uas_stream *uasp_get_stream_by_tag (struct f_uas *fu,u16 tag) +{ +/* +* For simplicity, we use mod operation to quickly find an in-progress drivers/usb/gadget/function/f_tcm.c static void uasp_cleanup_old_alt(struct f_uas *fu) · line 522
Quoted 10 lines
drivers/usb/gadget/function/f_tcm.c +10
-0+* matching command tag to check for overlapped command. The assumption +* is that the UASP class driver will limit to using tag id from 1 to +* USBG_NUM_CMDS. This is based on observation from the Windows and +* Linux UASP storage class driver behavior. If an unusual UASP class +* driver uses a tag greater than USBG_NUM_CMDS, then this method may no +* longer work due to possible stream id collision. In that case, we +* need to use a proper algorithm to fetch the stream (or simply walk +* through all active streams to check for overlap). +*/ +return &fu->stream[tag %USBG_NUM_CMDS ];
Quoted 8 lines
Could you please avoid the assumption what tag actually is? Please take a look at
hashtable.h,hash_add(),hash_del(),hash_for_each_possible_safe()is probably all you need.
That % looks efficient but gcc will try and remove the div operation which is something the hash implementation (as ofhash_min()) avoids. So the only additional costs here is the additional hashtable which worth the price given that you don't assume what tag can be.
Sure, I can look into it.
Thanks,
Thinh
PATCH v2 22/25 usb: gadget: f_tcm: Send sense on cancelled transfer
In reply to Thinh NguyenIf the transfer is cancelled due to a disconnect or driver tear down (error code -ESHUTDOWN), then just free the command. However, if it got cancelled due to other reasons, then send a sense CHECK CONDITION status with TCM_CHECK_CONDITION_ABORT_CMD status to host notifying the delivery failure. Note that this is separate from TASK MANAGEMENT function abort task command, which will require a separate response IU.
See UAS-r04 section 8.
---
Changes in v2:
- New patch
drivers/usb/gadget/function/f_tcm.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
drivers/usb/gadget/function/f_tcm.c +9 -3
@@ -949,11 +949,16 @@ static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req) {struct usbg_cmd *cmd = req->context;struct se_cmd *se_cmd = &cmd->se_cmd; +struct uas_stream *stream =uasp_get_stream_by_tag (cmd->fu, cmd->tag); cmd->state =UASP_QUEUE_COMMAND ; -if (req->status == -ESHUTDOWN ) - goto cleanup; +if (req->status == -ESHUTDOWN ) { + stream->cmd =NULL ; +target_put_sess_cmd (se_cmd); +transport_generic_free_cmd (&cmd->se_cmd,0 ); +return ; + }if (req->status) {pr_err ("%s() state %d transfer failed\n" , __func__, cmd->state);
@@ -972,7 +977,8 @@ static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req) cleanup:target_put_sess_cmd (se_cmd); -transport_generic_free_cmd (&cmd->se_cmd,0 ); +transport_send_check_condition_and_sense (se_cmd, +TCM_CHECK_CONDITION_ABORT_CMD ,0 ); }static int usbg_prepare_w_request (struct usbg_cmd *cmd,struct usb_request *req)
Signature · 1 line
2.28.0
PATCH v2 23/25 usb: gadget: f_tcm: Handle TASK_MANAGEMENT commands
In reply to Thinh NguyenHandle target_core_fabric_ops TASK MANAGEMENT functions and their response. If a TASK MANAGEMENT command is received, the driver will interpret the function TMF_*, translate to TMR_*, and fire off a command work executing target_submit_tmr(). On completion, it will handle the TASK MANAGEMENT response through uasp_send_tm_response().
---
Changes in v2:
- Splitted into smaller change. Remove handling of overlapped tag.
- Directly respond to host if f_tcm detected failure with response IU rather than reporting to target core.
drivers/usb/gadget/function/f_tcm.c | 206 +++++++++++++++++++++++++--- drivers/usb/gadget/function/tcm.h | 7 +- 2 files changed, 193 insertions(+), 20 deletions(-)
drivers/usb/gadget/function/f_tcm.c +187 -19
@@ -12,6 +12,7 @@#include <linux/string.h> #include <linux/configfs.h> #include <linux/ctype.h> +#include <linux/delay.h> #include <linux/usb/ch9.h> #include <linux/usb/composite.h> #include <linux/usb/gadget.h>
@@ -462,6 +463,51 @@ static int usbg_bot_setup(struct usb_function *f,/* Start uas.c code */ +static int tcm_to_uasp_response (enum tcm_tmrsp_table code) +{ +switch (code) { +case TMR_FUNCTION_FAILED : +return RC_TMF_FAILED ; +case TMR_FUNCTION_COMPLETE : +case TMR_TASK_DOES_NOT_EXIST : +return RC_TMF_COMPLETE ; +case TMR_LUN_DOES_NOT_EXIST : +return RC_INCORRECT_LUN ; +case TMR_OVERLAPPED_TAG_ATTEMPTED : +return RC_OVERLAPPED_TAG ; +case TMR_FUNCTION_REJECTED : +case TMR_TASK_MGMT_FUNCTION_NOT_SUPPORTED : +default : +return RC_TMF_NOT_SUPPORTED ; + } +} + +static unsigned char uasp_to_tcm_func (int code) +{ +switch (code) { +case TMF_ABORT_TASK : +return TMR_ABORT_TASK ; +case TMF_ABORT_TASK_SET : +return TMR_ABORT_TASK_SET ; +case TMF_CLEAR_TASK_SET : +return TMR_CLEAR_TASK_SET ; +case TMF_LOGICAL_UNIT_RESET : +return TMR_LUN_RESET ; +case TMF_I_T_NEXUS_RESET : +return TMR_I_T_NEXUS_RESET ; +case TMF_CLEAR_ACA : +return TMR_CLEAR_ACA ; +case TMF_QUERY_TASK : +return TMR_QUERY_TASK ; +case TMF_QUERY_TASK_SET : +return TMR_QUERY_TASK_SET ; +case TMF_QUERY_ASYNC_EVENT : +return TMR_QUERY_ASYNC_EVENT ; +default : +return TMR_UNKNOWN ; + } +} +static void uasp_cleanup_one_stream (struct f_uas *fu,struct uas_stream *stream) {/* We have either all three allocated or none */
@@ -581,6 +627,33 @@ static void uasp_prepare_status(struct usbg_cmd *cmd) stream->req_status->complete = uasp_status_data_cmpl; } +static void uasp_prepare_response (struct usbg_cmd *cmd) +{ +struct se_cmd *se_cmd = &cmd->se_cmd; +struct response_iu *rsp_iu = &cmd->response_iu; +struct uas_stream *stream =uasp_get_stream_by_tag (cmd->fu, cmd->tag); + + cmd->state =UASP_QUEUE_COMMAND ; + rsp_iu->iu_id =IU_ID_RESPONSE ; + rsp_iu->tag =cpu_to_be16 (cmd->tag); + +if (cmd->tmr_rsp !=RC_RESPONSE_UNKNOWN ) + rsp_iu->response_code = cmd->tmr_rsp; +else + rsp_iu->response_code = +tcm_to_uasp_response (se_cmd->se_tmr_req->response); + + stream->req_status->is_last =1 ; + stream->req_status->stream_id = cmd->tag; + stream->req_status->context = cmd; + stream->req_status->length =sizeof (struct response_iu ); + stream->req_status->buf = rsp_iu; + stream->req_status->complete = uasp_status_data_cmpl; +} + +static void usbg_release_cmd (struct se_cmd *se_cmd); +static int uasp_send_tm_response (struct usbg_cmd *cmd); +static void uasp_status_data_cmpl (struct usb_ep *ep,struct usb_request *req) {struct usbg_cmd *cmd = req->context;
@@ -619,9 +692,26 @@ static void uasp_status_data_cmpl(struct usb_ep *ep, struct usb_request *req)break ;case UASP_QUEUE_COMMAND : -transport_generic_free_cmd (&cmd->se_cmd,0 ); -usb_ep_queue (fu->ep_cmd, cmd->req,GFP_ATOMIC ); + stream->cmd =NULL ; + +/* +* If no command submitted to target core here, just free the +* bitmap index. This is for the cases where f_tcm handles +* status response instead of the target core. +*/ +if (cmd->tmr_rsp !=RC_RESPONSE_UNKNOWN ) { +struct se_session *se_sess; + + se_sess = fu->tpg->tpg_nexus->tvn_se_sess; +sbitmap_queue_clear (&se_sess->sess_tag_pool, + cmd->se_cmd.map_tag, + cmd->se_cmd.map_cpu); + }else { +transport_generic_free_cmd (&cmd->se_cmd,0 ); + } + +usb_ep_queue (fu->ep_cmd, cmd->req,GFP_ATOMIC );break ;default :
@@ -630,6 +720,7 @@ static void uasp_status_data_cmpl(struct usb_ep *ep, struct usb_request *req)return ; cleanup: + stream->cmd =NULL ;transport_generic_free_cmd (&cmd->se_cmd,0 ); }
@@ -645,6 +736,18 @@ static int uasp_send_status_response(struct usbg_cmd *cmd)return usb_ep_queue (fu->ep_status, stream->req_status,GFP_ATOMIC ); } +static int uasp_send_tm_response (struct usbg_cmd *cmd) +{ +struct f_uas *fu = cmd->fu; +struct uas_stream *stream =uasp_get_stream_by_tag (fu, cmd->tag); +struct response_iu *iu = &cmd->response_iu; + + iu->tag =cpu_to_be16 (cmd->tag); + cmd->fu = fu; +uasp_prepare_response (cmd); +return usb_ep_queue (fu->ep_status, stream->req_status,GFP_ATOMIC ); +} +static int uasp_send_read_response (struct usbg_cmd *cmd) {struct f_uas *fu = cmd->fu;
@@ -1045,9 +1148,24 @@ static int usbg_send_read_response(struct se_cmd *se_cmd)return uasp_send_read_response (cmd); } -static void usbg_cmd_work (struct work_struct *work) +static void usbg_submit_tmr (struct usbg_cmd *cmd) { struct usbg_cmd *cmd = container_of(work, struct usbg_cmd, work); +struct se_session *se_sess; +struct se_cmd *se_cmd; +int flags =TARGET_SCF_ACK_KREF ; + + se_cmd = &cmd->se_cmd; + se_sess = cmd->fu->tpg->tpg_nexus->tvn_se_sess; + +target_submit_tmr (se_cmd, se_sess, + cmd->response_iu.add_response_info, + cmd->unpacked_lun,NULL , cmd->tmr_func, +GFP_ATOMIC , cmd->tag, flags); +} + +static void usbg_submit_cmd (struct usbg_cmd *cmd) +{struct se_cmd *se_cmd;struct tcm_usbg_nexus *tv_nexus;struct usbg_tpg *tpg;
@@ -1076,6 +1194,29 @@ static void usbg_cmd_work(struct work_struct *work) TCM_UNSUPPORTED_SCSI_OPCODE, 0); } +static void usbg_cmd_work (struct work_struct *work) +{ + struct usbg_cmd *cmd = container_of(work, struct usbg_cmd, work); + +/* +* Failure is detected by f_tcm here. Skip submitting the command to the +* target core if we already know the failing response and send the usb +* response to the host directly. +*/ +if (cmd->tmr_rsp !=RC_RESPONSE_UNKNOWN ) + goto skip; + +if (cmd->tmr_func) +usbg_submit_tmr (cmd); +else +usbg_submit_cmd (cmd); + +return ; + +skip: +uasp_send_tm_response (cmd); +} +static struct usbg_cmd *usbg_get_cmd (struct f_uas *fu,struct tcm_usbg_nexus *tv_nexus,u32 scsi_tag) {
@@ -1102,37 +1243,63 @@ static void usbg_release_cmd(struct se_cmd *);static int usbg_submit_command (struct f_uas *fu,struct usb_request *req) { -struct command_iu *cmd_iu = req->buf; +struct iu *iu = req->buf;struct usbg_cmd *cmd;struct usbg_tpg *tpg = fu->tpg;struct tcm_usbg_nexus *tv_nexus; +struct uas_stream *stream; +struct command_iu *cmd_iu;u32 cmd_len;u16 scsi_tag; -if (cmd_iu->iu_id !=IU_ID_COMMAND ) { -pr_err ("Unsupported type %d\n" , cmd_iu->iu_id); -return -EINVAL ; - } - tv_nexus = tpg->tpg_nexus;if (!tv_nexus) {pr_err ("Missing nexus, ignoring command\n" );return -EINVAL ; } - cmd_len = (cmd_iu->len & ~0x3 ) +16 ; -if (cmd_len >USBG_MAX_CMD ) -return -EINVAL ; - - scsi_tag =be16_to_cpup (&cmd_iu->tag); + scsi_tag =be16_to_cpup (&iu->tag); cmd =usbg_get_cmd (fu, tv_nexus, scsi_tag);if (IS_ERR (cmd)) {pr_err ("usbg_get_cmd failed\n" );return -ENOMEM ; } -memcpy (cmd->cmd_buf, cmd_iu->cdb, cmd_len); - cmd->stream = &fu->stream[cmd->tag %USBG_NUM_CMDS ]; + cmd->req = req; + cmd->fu = fu; + cmd->tag = scsi_tag; + cmd->se_cmd.tag = scsi_tag; + cmd->tmr_func =0 ; + cmd->tmr_rsp =RC_RESPONSE_UNKNOWN ; + + cmd_iu = (struct command_iu *)iu; + +/* Command and Task Management IUs share the same LUN offset */ + cmd->unpacked_lun =scsilun_to_int (&cmd_iu->lun); + +if (iu->iu_id !=IU_ID_COMMAND && iu->iu_id !=IU_ID_TASK_MGMT ) { + cmd->tmr_rsp =RC_INVALID_INFO_UNIT ; + goto skip; + } + + stream->cmd = cmd; + +if (iu->iu_id ==IU_ID_TASK_MGMT ) { +struct task_mgmt_iu *tm_iu; + + tm_iu = (struct task_mgmt_iu *)iu; + cmd->tmr_func =uasp_to_tcm_func (tm_iu->function); + goto skip; + } + + cmd_len = (cmd_iu->len & ~0x3 ) +16 ; +if (cmd_len >USBG_MAX_CMD ) { +pr_err ("invalid len %d\n" , cmd_len); +target_free_tag (tv_nexus->tvn_se_sess, &cmd->se_cmd); + stream->cmd =NULL ; +return -EINVAL ; + } +memcpy (cmd->cmd_buf, cmd_iu->cdb, cmd_len);switch (cmd_iu->prio_attr &0x7 ) {case UAS_HEAD_TAG :
@@ -1153,9 +1320,7 @@ static int usbg_submit_command(struct f_uas *fu, struct usb_request *req)break ; } - cmd->unpacked_lun =scsilun_to_int (&cmd_iu->lun); - cmd->req = req; - +skip:INIT_WORK (&cmd->work, usbg_cmd_work);queue_work (tpg->workqueue, &cmd->work);
@@ -1301,6 +1466,9 @@ static int usbg_get_cmd_state(struct se_cmd *se_cmd)static void usbg_queue_tm_rsp (struct se_cmd *se_cmd) { + struct usbg_cmd *cmd = container_of(se_cmd, struct usbg_cmd, se_cmd); + +uasp_send_tm_response (cmd); }static void usbg_aborted_task (struct se_cmd *se_cmd)
drivers/usb/gadget/function/tcm.h +6 -1
@@ -82,8 +82,11 @@ struct usbg_cmd {u16 tag;u16 prio_attr;struct sense_iu sense_iu; +struct response_iu response_iu;enum uas_state state; -struct uas_stream *stream; +int tmr_func; +int tmr_rsp; +#define RC_RESPONSE_UNKNOWN 0xff/* BOT only */ __le32 bot_tag;
@@ -96,6 +99,8 @@ struct uas_stream {struct usb_request *req_in;struct usb_request *req_out;struct usb_request *req_status; + +struct usbg_cmd *cmd; };struct usbg_cdb {
Signature · 1 line
2.28.0
PATCH v2 24/25 usb: gadget: f_tcm: Check overlapped command
In reply to Thinh NguyenIf there's an overlapped command tag, cancel the command and respond with RC_OVERLAPPED_TAG to host.
---
Changes in v2:
- New patch. It was part of TASK MANAGEMENT change.
- Directly respond to host on overlapped tag instead of reporting to target core.
drivers/usb/gadget/function/f_tcm.c | 75 ++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 2 deletions(-)
drivers/usb/gadget/function/f_tcm.c +73 -2
@@ -692,6 +692,15 @@ static void uasp_status_data_cmpl(struct usb_ep *ep, struct usb_request *req)break ;case UASP_QUEUE_COMMAND : +/* +* Overlapped command detected and cancelled. +* So send overlapped attempted status. +*/ +if (cmd->tmr_rsp ==RC_OVERLAPPED_TAG && + req->status == -ECONNRESET ) { +uasp_send_tm_response (cmd); +return ; + } stream->cmd =NULL ;
@@ -700,7 +709,8 @@ static void uasp_status_data_cmpl(struct usb_ep *ep, struct usb_request *req)* bitmap index. This is for the cases where f_tcm handles * status response instead of the target core. */ -if (cmd->tmr_rsp !=RC_RESPONSE_UNKNOWN ) { +if (cmd->tmr_rsp !=RC_OVERLAPPED_TAG && + cmd->tmr_rsp !=RC_RESPONSE_UNKNOWN ) {struct se_session *se_sess; se_sess = fu->tpg->tpg_nexus->tvn_se_sess;
@@ -1080,6 +1090,14 @@ static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req) cleanup:target_put_sess_cmd (se_cmd); + +/* Command was aborted due to overlapped tag */ +if (cmd->state ==UASP_QUEUE_COMMAND && + cmd->tmr_rsp ==RC_OVERLAPPED_TAG ) { +uasp_send_tm_response (cmd); +return ; + } +transport_send_check_condition_and_sense (se_cmd,TCM_CHECK_CONDITION_ABORT_CMD ,0 ); }
@@ -1148,9 +1166,10 @@ static int usbg_send_read_response(struct se_cmd *se_cmd)return uasp_send_read_response (cmd); } +static void usbg_aborted_task (struct se_cmd *se_cmd); +static void usbg_submit_tmr (struct usbg_cmd *cmd) { - struct usbg_cmd *cmd = container_of(work, struct usbg_cmd, work);struct se_session *se_sess;struct se_cmd *se_cmd;int flags =TARGET_SCF_ACK_KREF ;
@@ -1214,6 +1233,51 @@ static void usbg_cmd_work(struct work_struct *work)return ; skip: +if (cmd->tmr_rsp ==RC_OVERLAPPED_TAG ) { +struct se_session *se_sess; +struct uas_stream *stream; + + se_sess = cmd->fu->tpg->tpg_nexus->tvn_se_sess; + stream =uasp_get_stream_by_tag (cmd->fu, cmd->tag); + +/* +* There's no guarantee of a matching completion order between +* different endpoints. i.e. The device may receive a new (CDB) +* command request completion of the command endpoint before it +* gets notified of the previous command status completion from +* a status endpoint. The driver still needs to detect +* misbehaving host and respond with an overlap command tag. To +* prevent false overlapped tag failure, give the active and +* matching stream id a short time (1ms) to complete before +* respond with overlapped command failure. +*/ +msleep (1 ); + +/* If the stream is completed, retry the command. */ +if (!stream->cmd) { +usbg_submit_command (cmd->fu, cmd->req); +return ; + } + +/* +* The command isn't submitted to the target core, so we're safe +* to remove the bitmap index from the session tag pool. +*/ +sbitmap_queue_clear (&se_sess->sess_tag_pool, + cmd->se_cmd.map_tag, + cmd->se_cmd.map_cpu); + +/* +* Overlap command tag detected. Cancel any pending transfer of +* the command submitted to target core. +*/ + stream->cmd->tmr_rsp =RC_OVERLAPPED_TAG ; +usbg_aborted_task (&stream->cmd->se_cmd); + +/* Send the response after the transfer is aborted. */ +return ; + } +uasp_send_tm_response (cmd); }
@@ -1282,6 +1346,13 @@ static int usbg_submit_command(struct f_uas *fu, struct usb_request *req) goto skip; } + stream =uasp_get_stream_by_tag (fu, scsi_tag); +if (stream->cmd) { +pr_err ("Command tag %d overlapped\n" , scsi_tag); + cmd->tmr_rsp =RC_OVERLAPPED_TAG ; + goto skip; + } + stream->cmd = cmd;if (iu->iu_id ==IU_ID_TASK_MGMT ) {
Signature · 1 line
2.28.0
Re: [PATCH v2 24/25] usb: gadget: f_tcm: Check overlapped command
Quoted 17 lines · Thinh Nguyen
drivers/usb/gadget/function/f_tcm.c +9
-0@@ -1214,6 +1233,51 @@ static void usbg_cmd_work(struct work_struct *work)return ; skip: +if (cmd->tmr_rsp ==RC_OVERLAPPED_TAG ) { +struct se_session *se_sess; +struct uas_stream *stream; + + se_sess = cmd->fu->tpg->tpg_nexus->tvn_se_sess; + stream =uasp_get_stream_by_tag (cmd->fu, cmd->tag); + +/* +* There's no guarantee of a matching completion order between
drivers/usb/gadget/function/f_tcm.c static void usbg_cmd_work(struct work_struct *work) · line 1254
Quoted 10 lines
drivers/usb/gadget/function/f_tcm.c +10
-0+* different endpoints. i.e. The device may receive a new (CDB) +* command request completion of the command endpoint before it +* gets notified of the previous command status completion from +* a status endpoint. The driver still needs to detect +* misbehaving host and respond with an overlap command tag. To +* prevent false overlapped tag failure, give the active and +* matching stream id a short time (1ms) to complete before +* respond with overlapped command failure. +*/ +msleep (1 );
How likely is it for this to happen? Is there maybe some synchronisation missing? If I see this right, in order to get here, you will already spill the message "Command tag %d overlapped" which does not look good. Why should the host re-use a tag which did not yet complete?
Sebastian
Re: [PATCH v2 24/25] usb: gadget: f_tcm: Check overlapped command
Quoted 25 lines · Sebastian Andrzej Siewior
Quoted 24 lines · Thinh Nguyen
drivers/usb/gadget/function/f_tcm.c +16
-0@@ -1214,6 +1233,51 @@ static void usbg_cmd_work(struct work_struct *work)return ; skip: +if (cmd->tmr_rsp ==RC_OVERLAPPED_TAG ) { +struct se_session *se_sess; +struct uas_stream *stream; + + se_sess = cmd->fu->tpg->tpg_nexus->tvn_se_sess; + stream =uasp_get_stream_by_tag (cmd->fu, cmd->tag); + +/* +* There's no guarantee of a matching completion order between +* different endpoints. i.e. The device may receive a new (CDB) +* command request completion of the command endpoint before it +* gets notified of the previous command status completion from +* a status endpoint. The driver still needs to detect +* misbehaving host and respond with an overlap command tag. To +* prevent false overlapped tag failure, give the active and +* matching stream id a short time (1ms) to complete before
Quoted 9 lines
drivers/usb/gadget/function/f_tcm.c static void usbg_cmd_work(struct work_struct *work) · line 1254
Quoted 3 lines
drivers/usb/gadget/function/f_tcm.c +3
-0+* respond with overlapped command failure. +*/ +msleep (1 );How likely is it for this to happen? Is there maybe some synchronisation missing? If I see this right, in order to get here, you will already spill the message "Command tag %d overlapped" which does not look good. Why should the host re-use a tag which did not yet complete?
Not often, but it can happen. For example, even though the device sent a status on the wire and the host received it. The host may send a new command with the same tag before the device is notified of the previous command completion. Since they operate in different endpoints, the device driver may get notification of the new command from the command endpoint before the status completion of the status endpoint.
This is an attempt to maintain synchronization and prevent false overlap check. It's a quick fix. I feel that we can handle this better. If you can think of a better way to handle this, let me know.
BR,
Thinh
PATCH v2 25/25 usb: gadget: f_tcm: Comply with UAS Task Management requirement
In reply to Thinh NguyenThe UASP driver must support all the task management functions listed in Table 20 of UAS-r04. However, not all of them are implemented in the Target core. To remain compliant while indicate that the TMR did not go through, report RC_TMF_FAILED instead of RC_TMF_NOT_SUPPORTED and print a warning to the user.
---
Changes in v2:
- None
drivers/usb/gadget/function/f_tcm.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
drivers/usb/gadget/function/f_tcm.c +24 -0
@@ -643,6 +643,30 @@ static void uasp_prepare_response(struct usbg_cmd *cmd) rsp_iu->response_code =tcm_to_uasp_response (se_cmd->se_tmr_req->response); +/* +* The UASP driver must support all the task management functions listed +* in Table 20 of UAS-r04. To remain compliant while indicate that the +* TMR did not go through, report RC_TMF_FAILED instead of +* RC_TMF_NOT_SUPPORTED and print a warning to the user. +*/ +switch (cmd->tmr_func) { +case TMR_ABORT_TASK : +case TMR_ABORT_TASK_SET : +case TMR_CLEAR_TASK_SET : +case TMR_LUN_RESET : +case TMR_I_T_NEXUS_RESET : +case TMR_CLEAR_ACA : +case TMR_QUERY_TASK : +case TMR_QUERY_TASK_SET : +case TMR_QUERY_ASYNC_EVENT : +if (rsp_iu->response_code ==RC_TMF_NOT_SUPPORTED ) { +pr_warn ("TMR function %d not supported\n" , + cmd->tmr_func); + rsp_iu->response_code =RC_TMF_FAILED ; + } +break ; + } + stream->req_status->is_last =1 ; stream->req_status->stream_id = cmd->tag; stream->req_status->context = cmd;
Signature · 1 line
2.28.0
Greg Kroah-Hartman
maintainerQuoted 12 lines · Thinh Nguyen
The Linux UASP gadget driver is incomplete and remained broken for a long time. It was not implemented for performance either. This series adds some of the required features for the UASP driver to work. It also makes some changes to the target core.
This is tested against UASP CV and
DWC_usb3xcontroller. It still needs some fixes in the target core, which will be separated from this series.There are still more room for performance improvement and fixes. However, this series should be sufficient to bring up a working UASP device.
Quoted 8 lines
Changes in v2:
- Remove most target core changes from this series and only keep the must-have ones
- Split the task-management patch to smaller patches
- Don't send failure Task Management response to target core, reducing dependency
- Add UASP bringup script example in cover page
- Make various small updates according to feedbacks
I would need a review by the target maintainers before being able to take any of the USB gadget changes into the USB tree...
thanks,
greg k-h
Hi Martin, Dmitry, and others,
Quoted 22 lines · Greg Kroah-Hartman
Quoted 12 lines · Thinh Nguyen
The Linux UASP gadget driver is incomplete and remained broken for a long time. It was not implemented for performance either. This series adds some of the required features for the UASP driver to work. It also makes some changes to the target core.
This is tested against UASP CV and
DWC_usb3xcontroller. It still needs some fixes in the target core, which will be separated from this series.There are still more room for performance improvement and fixes. However, this series should be sufficient to bring up a working UASP device.
Quoted 8 lines
Changes in v2:
- Remove most target core changes from this series and only keep the must-have ones
- Split the task-management patch to smaller patches
- Don't send failure Task Management response to target core, reducing dependency
- Add UASP bringup script example in cover page
- Make various small updates according to feedbacks
Quoted 3 lines
I would need a review by the target maintainers before being able to take any of the USB gadget changes into the USB tree...
Do you have any comment on this series?
Thanks,
Thinh
Quoted 25 lines · Thinh Nguyen
Hi Martin, Dmitry, and others,
Quoted 22 lines · Greg Kroah-Hartman
Quoted 20 lines · Thinh Nguyen
The Linux UASP gadget driver is incomplete and remained broken for a long time. It was not implemented for performance either. This series adds some of the required features for the UASP driver to work. It also makes some changes to the target core.
This is tested against UASP CV and
DWC_usb3xcontroller. It still needs some fixes in the target core, which will be separated from this series.There are still more room for performance improvement and fixes. However, this series should be sufficient to bring up a working UASP device.
Changes in v2:
- Remove most target core changes from this series and only keep the must-have ones
- Split the task-management patch to smaller patches
- Don't send failure Task Management response to target core, reducing dependency
- Add UASP bringup script example in cover page
- Make various small updates according to feedbacks
Quoted 6 lines
Quoted 3 lines
I would need a review by the target maintainers before being able to take any of the USB gadget changes into the USB tree...
Do you have any comment on this series?
Hi target maintainers,
Gentle ping...
BR,
Thinh
Quoted 4 lines · Thinh Nguyen
The Linux UASP gadget driver is incomplete and remained broken for a long time. It was not implemented for performance either. This series adds some of the required features for the UASP driver to work. It also makes some changes to the target core.
Some patches here have fixes: tags and are in the middle of the series. If they are indeed fixes which are needed for the driver function regardless of the other changes, which are part of the series, then they should be moved to the front of series _or_ submitted independently as in "lets first fix the broken things and then make it pretty".
All in all I am happy to see that somebody is looking into the target USB gadget.
Sebastian
Quoted 6 lines · Sebastian Andrzej Siewior
Quoted 4 lines · Thinh Nguyen
The Linux UASP gadget driver is incomplete and remained broken for a long time. It was not implemented for performance either. This series adds some of the required features for the UASP driver to work. It also makes some changes to the target core.
Quoted 9 lines
Some patches here have fixes: tags and are in the middle of the series. If they are indeed fixes which are needed for the driver function regardless of the other changes, which are part of the series, then they should be moved to the front of series _or_ submitted independently as in "lets first fix the broken things and then make it pretty".
All in all I am happy to see that somebody is looking into the target USB gadget.
Thanks for the reviews!
I can move the commits with fixes tag around. But for the driver to work properly against different OSes (and maybe even for Linux), most of the changes without the fixes tags are also needed and not just to make it "pretty".
There are still more work for f_tcm, but this series (plus the fixes series in target) is enough for the driver to work properly.
Thanks,
Thinh