Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 26 Aug 2022 09:22:37 +0200
From:   Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To:     Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Cc:     Felipe Balbi <balbi@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
        Dmitry Bogdanov <d.bogdanov@yadro.com>,
        linux-scsi@vger.kernel.org, target-devel@vger.kernel.org
Subject: Re: [PATCH v2 16/25] usb: gadget: f_tcm: Update state on data write
Message-ID: <Ywh0vQkRLTrSeExk@linutronix.de>
References: <cover.1658192351.git.Thinh.Nguyen@synopsys.com>
 <dd9069e0527f2da04b6567fd17b19545646f4348.1658192351.git.Thinh.Nguyen@synopsys.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <dd9069e0527f2da04b6567fd17b19545646f4348.1658192351.git.Thinh.Nguyen@synopsys.com>
Precedence: bulk

On 2022-07-18 18:27:45 [-0700], Thinh Nguyen wrote:
> When 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.

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?

> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> ---
>  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(+)
> 
> diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
> index 1e7d29f8aecb..d7318c84af98 100644
> --- a/drivers/usb/gadget/function/f_tcm.c
> +++ b/drivers/usb/gadget/function/f_tcm.c
> @@ -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;
>  }
>  

Sebastian