From: Felipe Balbi <balbi@kernel.org>
To: Anurag Kumar Vulisha <anuragku@xilinx.com>,
Alan Stern <stern@rowland.harvard.edu>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Shuah Khan <shuah@kernel.org>, 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>,
"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: [v7,01/10] usb: gadget: udc: Add timer support for usb requests
Date: Fri, 07 Dec 2018 08:05:39 +0200 [thread overview]
Message-ID: <877eglx45o.fsf@linux.intel.com> (raw)
hi,
Anurag Kumar Vulisha <anuragku@xilinx.com> writes:
>>Does the data book suggest a value for the timeout?
>>
>
> No, the databook doesn't mention about the timeout value
>
>>> >At this point, it seems that the generic approach will be messier than having every
>>> >controller driver implement its own fix. At least, that's how it appears to me.
Why, if the UDC implementation will, anyway, be a timer?
>>(Especially if dwc3 is the only driver affected.)
>
> As discussed above, the issue may happen with other gadgets too. As I got divide opinions
> on this implementation and both the implementations looks fine to me, I am little confused
> on which should be implemented.
>
> @Felipe: Do you agree with Alan's implementation? Please let us know your suggestion
> on this.
I still think a generic timer is a better solution since it has other uses.
>>> >Ideally it would not be necessary to rely on a timeout at all.
>>> >
>>> >Also, maintainers dislike module parameters. It would be better not to add one.
>>>
>>> Okay. I would be happy if any alternative for this issue is present but unfortunately
>>> I am not able to figure out any alternative other than timers. If not
>>module_params()
>>> we can add an configfs entry in stream gadget to update the timeout. Please
>>provide
>>> your opinion on this approach.
>>
>>Since the purpose of the timeout is to detect a deadlock caused by a
>>hardware bug, I suggest a fixed and relatively short timeout value such
>>as one second. Cancelling and requeuing a few requests at 1-second
>>intervals shouldn't add very much overhead.
I wouldn't call this a HW bug though. This is just how the UDC
behaves. There are N streams and host can move data in any stream at any
time. This means that host & gadget _can_ disagree on what stream to
start next.
One way to avoid this would be to never pre-start any streams and always
rely on XferNotReady, but that would mean greatly reduced throughput for
streams.
WARNING: multiple messages have this Message-ID (diff)
From: Felipe Balbi <balbi@kernel.org>
To: Anurag Kumar Vulisha <anuragku@xilinx.com>,
Alan Stern <stern@rowland.harvard.edu>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Shuah Khan <shuah@kernel.org>, 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>,
"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 01/10] usb: gadget: udc: Add timer support for usb requests
Date: Fri, 07 Dec 2018 08:05:39 +0200 [thread overview]
Message-ID: <877eglx45o.fsf@linux.intel.com> (raw)
In-Reply-To: <BL0PR02MB563386B88BDC306AF44EE736A7A80@BL0PR02MB5633.namprd02.prod.outlook.com>
hi,
Anurag Kumar Vulisha <anuragku@xilinx.com> writes:
>>Does the data book suggest a value for the timeout?
>>
>
> No, the databook doesn't mention about the timeout value
>
>>> >At this point, it seems that the generic approach will be messier than having every
>>> >controller driver implement its own fix. At least, that's how it appears to me.
Why, if the UDC implementation will, anyway, be a timer?
>>(Especially if dwc3 is the only driver affected.)
>
> As discussed above, the issue may happen with other gadgets too. As I got divide opinions
> on this implementation and both the implementations looks fine to me, I am little confused
> on which should be implemented.
>
> @Felipe: Do you agree with Alan's implementation? Please let us know your suggestion
> on this.
I still think a generic timer is a better solution since it has other uses.
>>> >Ideally it would not be necessary to rely on a timeout at all.
>>> >
>>> >Also, maintainers dislike module parameters. It would be better not to add one.
>>>
>>> Okay. I would be happy if any alternative for this issue is present but unfortunately
>>> I am not able to figure out any alternative other than timers. If not
>>module_params()
>>> we can add an configfs entry in stream gadget to update the timeout. Please
>>provide
>>> your opinion on this approach.
>>
>>Since the purpose of the timeout is to detect a deadlock caused by a
>>hardware bug, I suggest a fixed and relatively short timeout value such
>>as one second. Cancelling and requeuing a few requests at 1-second
>>intervals shouldn't add very much overhead.
I wouldn't call this a HW bug though. This is just how the UDC
behaves. There are N streams and host can move data in any stream at any
time. This means that host & gadget _can_ disagree on what stream to
start next.
One way to avoid this would be to never pre-start any streams and always
rely on XferNotReady, but that would mean greatly reduced throughput for
streams.
--
balbi
next reply other threads:[~2018-12-07 6:05 UTC|newest]
Thread overview: 67+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-07 6:05 Felipe Balbi [this message]
2018-12-07 6:05 ` [PATCH v7 01/10] usb: gadget: udc: Add timer support for usb requests Felipe Balbi
-- strict thread matches above, loose matches on Subject: below --
2019-01-04 14:17 [v7,01/10] " Anurag Kumar Vulisha
2019-01-04 14:17 ` [PATCH v7 01/10] " Anurag Kumar Vulisha
2018-12-12 15:11 [v7,01/10] " Anurag Kumar Vulisha
2018-12-12 15:11 ` [PATCH v7 01/10] " Anurag Kumar Vulisha
2018-12-10 9:03 [v7,09/10] usb: dwc3: Check for IOC/LST bit in both event->status and TRB->ctrl fields Felipe Balbi
2018-12-10 9:03 ` [PATCH v7 09/10] " Felipe Balbi
2018-12-10 8:56 [v7,09/10] " Anurag Kumar Vulisha
2018-12-10 8:56 ` [PATCH v7 09/10] " Anurag Kumar Vulisha
2018-12-10 6:54 [v7,09/10] " Felipe Balbi
2018-12-10 6:54 ` [PATCH v7 09/10] " Felipe Balbi
2018-12-08 19:03 [v7,09/10] " Anurag Kumar Vulisha
2018-12-08 19:03 ` [PATCH v7 09/10] " Anurag Kumar Vulisha
2018-12-07 17:09 [v7,01/10] usb: gadget: udc: Add timer support for usb requests Alan Stern
2018-12-07 17:09 ` [PATCH v7 01/10] " Alan Stern
2018-12-07 6:11 [v7,09/10] usb: dwc3: Check for IOC/LST bit in both event->status and TRB->ctrl fields Felipe Balbi
2018-12-07 6:11 ` [PATCH v7 09/10] " Felipe Balbi
2018-12-05 19:05 [v7,05/10] usb: dwc3: make controller clear transfer resources after complete Anurag Kumar Vulisha
2018-12-05 19:05 ` [PATCH v7 05/10] " Anurag Kumar Vulisha
2018-12-05 19:01 [v7,09/10] usb: dwc3: Check for IOC/LST bit in both event->status and TRB->ctrl fields Anurag Kumar Vulisha
2018-12-05 19:01 ` [PATCH v7 09/10] " Anurag Kumar Vulisha
2018-12-05 15:43 [v7,01/10] usb: gadget: udc: Add timer support for usb requests Anurag Kumar Vulisha
2018-12-05 15:43 ` [PATCH v7 01/10] " Anurag Kumar Vulisha
2018-12-05 9:07 [v7,09/10] usb: dwc3: Check for IOC/LST bit in both event->status and TRB->ctrl fields Felipe Balbi
2018-12-05 9:07 ` [PATCH v7 09/10] " Felipe Balbi
2018-12-05 9:01 [v7,05/10] usb: dwc3: make controller clear transfer resources after complete Felipe Balbi
2018-12-05 9:01 ` [PATCH v7 05/10] " Felipe Balbi
2018-12-04 19:28 [v7,01/10] usb: gadget: udc: Add timer support for usb requests Alan Stern
2018-12-04 19:28 ` [PATCH v7 01/10] " Alan Stern
2018-12-04 19:07 [v7,01/10] " Anurag Kumar Vulisha
2018-12-04 19:07 ` [PATCH v7 01/10] " Anurag Kumar Vulisha
2018-12-04 16:46 [v7,01/10] " Alan Stern
2018-12-04 16:46 ` [PATCH v7 01/10] " Alan Stern
2018-12-04 16:18 [v7,01/10] " Anurag Kumar Vulisha
2018-12-04 16:18 ` [PATCH v7 01/10] " Anurag Kumar Vulisha
2018-12-03 23:08 [v7,01/10] " Alan Stern
2018-12-03 23:08 ` [PATCH v7 01/10] " Alan Stern
2018-12-03 16:05 [v7,01/10] " Anurag Kumar Vulisha
2018-12-03 16:05 ` [PATCH v7 01/10] " Anurag Kumar Vulisha
2018-12-03 14:51 [v7,01/10] " Alan Stern
2018-12-03 14:51 ` [PATCH v7 01/10] " Alan Stern
2018-12-03 10:23 [v7,01/10] " Anurag Kumar Vulisha
2018-12-03 10:23 ` [PATCH v7 01/10] " Anurag Kumar Vulisha
2018-12-02 16:36 [v7,01/10] " Alan Stern
2018-12-02 16:36 ` [PATCH v7 01/10] " Alan Stern
2018-12-01 11:13 [v7,10/10] usb: dwc3: Check MISSED ISOC bit only for ISOC endpoints Anurag Kumar Vulisha
2018-12-01 11:13 ` [PATCH v7 10/10] " Anurag Kumar Vulisha
2018-12-01 11:13 [v7,09/10] usb: dwc3: Check for IOC/LST bit in both event->status and TRB->ctrl fields Anurag Kumar Vulisha
2018-12-01 11:13 ` [PATCH v7 09/10] " Anurag Kumar Vulisha
2018-12-01 11:13 [v7,08/10] usb: dwc3: Correct the logic for checking TRB full in __dwc3_prepare_one_trb() Anurag Kumar Vulisha
2018-12-01 11:13 ` [PATCH v7 08/10] " Anurag Kumar Vulisha
2018-12-01 11:13 [v7,07/10] usb: dwc3: check for requests in started list for stream capable endpoints Anurag Kumar Vulisha
2018-12-01 11:13 ` [PATCH v7 07/10] " Anurag Kumar Vulisha
2018-12-01 11:13 [v7,06/10] usb: dwc3: don't issue no-op trb " Anurag Kumar Vulisha
2018-12-01 11:13 ` [PATCH v7 06/10] " Anurag Kumar Vulisha
2018-12-01 11:13 [v7,05/10] usb: dwc3: make controller clear transfer resources after complete Anurag Kumar Vulisha
2018-12-01 11:13 ` [PATCH v7 05/10] " Anurag Kumar Vulisha
2018-12-01 11:13 [v7,04/10] usb: dwc3: update stream id in depcmd Anurag Kumar Vulisha
2018-12-01 11:13 ` [PATCH v7 04/10] " Anurag Kumar Vulisha
2018-12-01 11:13 [v7,03/10] usb: dwc3: gadget: handle stream events Anurag Kumar Vulisha
2018-12-01 11:13 ` [PATCH v7 03/10] " Anurag Kumar Vulisha
2018-12-01 11:13 [v7,02/10] usb: gadget: function: tcm: Add timeout for stream capable endpoints Anurag Kumar Vulisha
2018-12-01 11:13 ` [PATCH v7 02/10] " Anurag Kumar Vulisha
2018-12-01 11:13 [v7,01/10] usb: gadget: udc: Add timer support for usb requests Anurag Kumar Vulisha
2018-12-01 11:13 ` [PATCH v7 01/10] " Anurag Kumar Vulisha
2018-12-01 11:13 [PATCH v7 00/10] usb: dwc3: Fix broken BULK stream support to dwc3 gadget driver Anurag Kumar Vulisha
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=877eglx45o.fsf@linux.intel.com \
--to=balbi@kernel.org \
--cc=APANDEY@xilinx.com \
--cc=anuragku@xilinx.com \
--cc=benh@kernel.crashing.org \
--cc=bvanassche@acm.org \
--cc=climbbb.kim@gmail.com \
--cc=colin.king@canonical.com \
--cc=gregkh@linuxfoundation.org \
--cc=johan@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=mchristi@redhat.com \
--cc=mgautam@codeaurora.org \
--cc=rogerq@ti.com \
--cc=shuah@kernel.org \
--cc=stern@rowland.harvard.edu \
--cc=tejas.joglekar@synopsys.com \
--cc=thinhn@synopsys.com \
--cc=v.anuragkumar@gmail.com \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.