Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Anurag Kumar Vulisha <anuragku@xilinx.com>
To:     Felipe Balbi <balbi@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Shuah Khan <shuah@kernel.org>,
        Alan Stern <stern@rowland.harvard.edu>,
        Johan Hovold <johan@kernel.org>,
        Jaejoong Kim <climbbb.kim@gmail.com>,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        Roger Quadros <rogerq@ti.com>,
        Manu Gautam <mgautam@codeaurora.org>,
        "martin.petersen@oracle.com" <martin.petersen@oracle.com>,
        Bart Van Assche <bvanassche@acm.org>,
        Mike Christie <mchristi@redhat.com>,
        Matthew Wilcox <willy@infradead.org>,
        Colin Ian King <colin.king@canonical.com>
CC:     "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "v.anuragkumar@gmail.com" <v.anuragkumar@gmail.com>,
        Thinh Nguyen <thinhn@synopsys.com>,
        Tejas Joglekar <tejas.joglekar@synopsys.com>,
        Ajay Yugalkishore Pandey <APANDEY@xilinx.com>
Subject: RE: [PATCH v7 09/10] usb: dwc3: Check for IOC/LST bit in both
 event->status and TRB->ctrl fields
Thread-Topic: [PATCH v7 09/10] usb: dwc3: Check for IOC/LST bit in both
 event->status and TRB->ctrl fields
Thread-Index: AQHUiWbqDI9WfI74TEmPyd+V2NlqDqVv4SkAgACgrWCAAlLUgIACXYXwgAJlVICAABFagA==
Date:   Mon, 10 Dec 2018 08:56:43 +0000
Message-ID: <BL0PR02MB56338A2EA4515527AA0EECF5A7A50@BL0PR02MB5633.namprd02.prod.outlook.com>
References: <1543662811-5194-1-git-send-email-anurag.kumar.vulisha@xilinx.com>
 <1543662811-5194-10-git-send-email-anurag.kumar.vulisha@xilinx.com>
 <875zw82vfj.fsf@linux.intel.com>
 <BL0PR02MB563352F83A80E3B92347A7A4A7A80@BL0PR02MB5633.namprd02.prod.outlook.com>
 <874lbpx3vg.fsf@linux.intel.com>
 <BL0PR02MB56331B2ECFFEB96A99936C32A7AB0@BL0PR02MB5633.namprd02.prod.outlook.com>
 <87pnu927oq.fsf@linux.intel.com>
In-Reply-To: <87pnu927oq.fsf@linux.intel.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: xilinx.com does not designate
 permitted sender hosts)
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 6a5333d7-4545-49e7-5640-08d65e7d6872
X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Dec 2018 08:56:43.3769
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 657af505-d5df-48d0-8300-c31994686c5c
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL0PR02MB4579
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Felipe,

>-----Original Message-----
>From: Felipe Balbi [mailto:balbi@kernel.org]
>Sent: Monday, December 10, 2018 12:24 PM
>To: Anurag Kumar Vulisha <anuragku@xilinx.com>; Greg Kroah-Hartman
><gregkh@linuxfoundation.org>; Shuah Khan <shuah@kernel.org>; Alan Stern
><stern@rowland.harvard.edu>; Johan Hovold <johan@kernel.org>; Jaejoong Kim
><climbbb.kim@gmail.com>; Benjamin Herrenschmidt <benh@kernel.crashing.org>=
;
>Roger Quadros <rogerq@ti.com>; Manu Gautam <mgautam@codeaurora.org>;
>martin.petersen@oracle.com; Bart Van Assche <bvanassche@acm.org>; Mike
>Christie <mchristi@redhat.com>; Matthew Wilcox <willy@infradead.org>; Coli=
n Ian
>King <colin.king@canonical.com>
>Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org;
>v.anuragkumar@gmail.com; Thinh Nguyen <thinhn@synopsys.com>; Tejas Jogleka=
r
><tejas.joglekar@synopsys.com>; Ajay Yugalkishore Pandey <APANDEY@xilinx.co=
m>
>Subject: RE: [PATCH v7 09/10] usb: dwc3: Check for IOC/LST bit in both eve=
nt->status
>and TRB->ctrl fields
>
>
>Hi,
>
>Anurag Kumar Vulisha <anuragku@xilinx.com> writes:
>> HI Felipe,
>>
>>>-----Original Message-----
>>>From: Felipe Balbi [mailto:balbi@kernel.org]
>>>Sent: Friday, December 07, 2018 11:42 AM
>>>To: Anurag Kumar Vulisha <anuragku@xilinx.com>; Greg Kroah-Hartman
>>><gregkh@linuxfoundation.org>; Shuah Khan <shuah@kernel.org>; Alan Stern
>>><stern@rowland.harvard.edu>; Johan Hovold <johan@kernel.org>; Jaejoong K=
im
>>><climbbb.kim@gmail.com>; Benjamin Herrenschmidt <benh@kernel.crashing.or=
g>;
>>>Roger Quadros <rogerq@ti.com>; Manu Gautam <mgautam@codeaurora.org>;
>>>martin.petersen@oracle.com; Bart Van Assche <bvanassche@acm.org>; Mike
>>>Christie <mchristi@redhat.com>; Matthew Wilcox <willy@infradead.org>; Co=
lin
>Ian
>>>King <colin.king@canonical.com>
>>>Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org;
>>>v.anuragkumar@gmail.com; Thinh Nguyen <thinhn@synopsys.com>; Tejas
>Joglekar
>>><tejas.joglekar@synopsys.com>; Ajay Yugalkishore Pandey
><APANDEY@xilinx.com>
>>>Subject: RE: [PATCH v7 09/10] usb: dwc3: Check for IOC/LST bit in both e=
vent-
>>status
>>>and TRB->ctrl fields
>>>
>>>
>>>Hi,
>>>
>>>Anurag Kumar Vulisha <anuragku@xilinx.com> writes:
>>>>>> @@ -2286,7 +2286,12 @@ static int
>>>>>dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
>>>>>>  	if (event->status & DEPEVT_STATUS_SHORT && !chain)
>>>>>>  		return 1;
>>>>>>
>>>>>> -	if (event->status & (DEPEVT_STATUS_IOC | DEPEVT_STATUS_LST))
>>>>>> +	if ((event->status & DEPEVT_STATUS_IOC) &&
>>>>>> +	    (trb->ctrl & DWC3_TRB_CTRL_IOC))
>>>>>> +		return 1;
>>>>>
>>>>>this shouldn't be necessary. According to databook, event->status
>>>>>contains the bits from the completed TRB.  Which means that
>>>>>event->status & IOC will always be equal to trb->ctrl & IOC.
>>>>>
>>>> Thanks for reviewing this patch. Lets consider an example where a
>>>> request has num_sgs > 0 and each sg is mapped to a TRB and the last
>>>> TRB has the IOC bit set. Once the controller is done with the
>>>> transfer, it  generates XferInProgress for the last TRB (since IOC bit
>>>> is set). As a part of trb reclaim process
>>>> dwc3_gadget_ep_reclaim_trb_sg() calls
>>>> dwc3_gadget_ep_reclaim_completed_trb() for req->num_sgs times. Since
>>>> the event already has the IOC bit set, the loop is exited from the
>>>> loop at the very first TRB and the remaining TRBs (mapped to the sglis=
t) are left
>>>unhandled.
>>>> To avoid this we modified the code to exit only if both TRB & event
>>>> has the IOC bit set.
>>>
>>>Seems like IOC case should just test for chain flag as well:
>>>
>>
>> Okay. Along with this logic the code for updating chain bit should also =
be modified I
>guess.
>
>not really
>
>> Since the IOC bit is also set when there are not enough TRBs available, =
the code
>should be
>> modified to not set DWC3_TRB_CTRL_CHN bit when the IOC bit is set. I wil=
l update
>below
>> changes along with your suggestions and resend the patches.
>
>no. Actually I don't think we're allowed to split a scatter/gather like
>that. I did that quite a while ago, but I don't think we're allowed to
>do so. What we should do, in that case, is not even queue that request
>until we have enough for all members of the scatter/gather. But that's a
>separate patch, anyway.
>

Okay. I have a doubt here, not pushing the request until all sgs are mapped=
 to enough TRBs
might remove the driver complexity but reduce the performance (since we are=
 waiting
until enough TRBs are available). Are we okay with that? =20

Thanks,
Anurag Kumar Vulisha =20