Re: [PATCH] usb: typec: Group all TCPCI/TCPM code together
HI, On 07-09-18 18:19, Guenter Roeck wrote: On Fri, Sep 07, 2018 at 05:06:12PM +0300, Heikki Krogerus wrote: On Fri, Sep 07, 2018 at 06:35:12AM -0700, Guenter Roeck wrote: On 09/07/2018 05:56 AM, Heikki Krogerus wrote: Moving all the drivers that depend on the Port Controller Manager under a new a new directory drivers/usb/typec/tcpci/ and making Guenter Roeck as the designated reviewer of that code. Signed-off-by: Heikki Krogerus --- Hi guys, This should be fairly trivial change. There is no functional effect. Even menuconfig looks the same. The only interesting thing is that I'm proposing that Guenter is marked as the reviewer/maintainer of all TCPCI/TCPM drivers. So Guenter, I guess the only question is that are you okay with this? NP. I am not sure about the directory name, though. fusb and wcove don't really support tcpci; only rt1711h does. The common denominator is really the port manager (tcpm), not the port controller interface (tcpci). As such, "typec port controller drivers" (without "interface") may be a bit more appropriate. Makes sense. Not sure about the directory name, though. Maybe a simple "port" or "controller" would do ? Or anything else that doesn't look like the abbreviation for one of the supported protocols. "port" maybe, but not controller. tps6598x.c is also a controller driver, but it does not belong to that directory. How about "phy"? Maybe just use 'port'. Seems to me that 'phy' would not really be a good match for the port manager (tcpm). 'phy' would still be better than tcpci, though, so I am ok with it if others think it should be used. I'm not a fan of phy, that makes me expect actual ethernet/sata/usb phy drivers, which these or not. Why not just use tcpm ? As Guenter said that is the common denominator. Anyways this change is fine with me regardless of the name. Regards, Hans
Re: [PATCH] usb: typec: Group all TCPCI/TCPM code together
On 09/08/2018 04:12 AM, Hans de Goede wrote: HI, On 07-09-18 18:19, Guenter Roeck wrote: On Fri, Sep 07, 2018 at 05:06:12PM +0300, Heikki Krogerus wrote: On Fri, Sep 07, 2018 at 06:35:12AM -0700, Guenter Roeck wrote: On 09/07/2018 05:56 AM, Heikki Krogerus wrote: Moving all the drivers that depend on the Port Controller Manager under a new a new directory drivers/usb/typec/tcpci/ and making Guenter Roeck as the designated reviewer of that code. Signed-off-by: Heikki Krogerus --- Hi guys, This should be fairly trivial change. There is no functional effect. Even menuconfig looks the same. The only interesting thing is that I'm proposing that Guenter is marked as the reviewer/maintainer of all TCPCI/TCPM drivers. So Guenter, I guess the only question is that are you okay with this? NP. I am not sure about the directory name, though. fusb and wcove don't really support tcpci; only rt1711h does. The common denominator is really the port manager (tcpm), not the port controller interface (tcpci). As such, "typec port controller drivers" (without "interface") may be a bit more appropriate. Makes sense. Not sure about the directory name, though. Maybe a simple "port" or "controller" would do ? Or anything else that doesn't look like the abbreviation for one of the supported protocols. "port" maybe, but not controller. tps6598x.c is also a controller driver, but it does not belong to that directory. How about "phy"? Maybe just use 'port'. Seems to me that 'phy' would not really be a good match for the port manager (tcpm). 'phy' would still be better than tcpci, though, so I am ok with it if others think it should be used. I'm not a fan of phy, that makes me expect actual ethernet/sata/usb phy drivers, which these or not. Why not just use tcpm ? As Guenter said that is the common denominator. Thinking about it, that makes sense. Guenter
Re: [PATCH v9 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
Hi Peter, . >SMBUS has nothing to do with the >problem, that was just an example. An I2C >>client driver can issue such I2C xfers all by >itself without going through >emulation, so >just dropping the _EMUL flag is not the >answer. And I'd be >surprised if the >hardware doesn't support single message >reads. >There is no quirk flag for this abnormality, >so you will have to open code >the check in >your master_xfer if you can't make such >xfers work, but the >best fix is certainly to >just make them work... I have fixed that in v10. Please check >Cheers, >Peter — nvpublic —
Re: [PATCH v9 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
On 2018-09-08 16:13, Ajay Gupta wrote: > > Hi Peter, > > . > >> SMBUS has nothing to do with the >problem, that was just an example. An I2C >> >client driver can issue such I2C xfers all by >itself without going through >> emulation, so >just dropping the _EMUL flag is not the >answer. And I'd be >> surprised if the >hardware doesn't support single message >reads. > >> There is no quirk flag for this abnormality, >so you will have to open code >> the check in >your master_xfer if you can't make such >xfers work, but the >> best fix is certainly to >just make them work... > > I have fixed that in v10. Please check Ah, I didn't even bother to look earlier. I have now taken a peek, and there are still issues. I'll comment inline for the v10 1/2 patch as usual, but I'm short on time at the moment so it will probably be a few hours. It's an option to hold on to v11 until that happens... Cheers, Peter
Re: [PATCH v10 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
Andy, there's a question for you below. On 2018-09-08 02:09, Ajay Gupta wrote: > Latest NVIDIA GPU card has USB Type-C interface. There is a > Type-C controller which can be accessed over I2C. > > This driver adds I2C bus driver to communicate with Type-C controller. > I2C client driver will be part of USB Type-C UCSI driver. > > Signed-off-by: Ajay Gupta > Reviewed-by: Andy Shevchenko > Reviewed-by: Heikki Krogerus > --- > Changes from v1 -> v2 > None > Changes from v2 -> v3 > Fixed review comments from Andy and Thierry > Rename i2c-gpu.c -> i2c-nvidia-gpu.c > Changes from v3 -> v4 > Fixed review comments from Andy > Changes from v4 -> v5 > Fixed review comments from Andy > Changes from v5 -> v6 > None > Changes from v6 -> v7 -> v8 > Fixed review comments from Peter > - Add implicit STOP for last write message > - Add i2c_adapter_quirks with max_read_len and > I2C_AQ_COMB flags > Changes from v8 -> v9 > Fixed review comments from Peter > - Drop do_start flag > - Use i2c_8bit_addr_from_msg() > Changes from v9 -> v10 > Fixed review comments from Peter > - Dropped I2C_FUNC_SMBUS_EMUL > - Dropped local mutex > > Documentation/i2c/busses/i2c-nvidia-gpu | 18 ++ > MAINTAINERS | 7 + > drivers/i2c/busses/Kconfig | 9 + > drivers/i2c/busses/Makefile | 1 + > drivers/i2c/busses/i2c-nvidia-gpu.c | 372 > > 5 files changed, 407 insertions(+) > create mode 100644 Documentation/i2c/busses/i2c-nvidia-gpu > create mode 100644 drivers/i2c/busses/i2c-nvidia-gpu.c > > diff --git a/Documentation/i2c/busses/i2c-nvidia-gpu > b/Documentation/i2c/busses/i2c-nvidia-gpu > new file mode 100644 > index 000..31884d2 > --- /dev/null > +++ b/Documentation/i2c/busses/i2c-nvidia-gpu > @@ -0,0 +1,18 @@ > +Kernel driver i2c-nvidia-gpu > + > +Datasheet: not publicly available. > + > +Authors: > + Ajay Gupta > + > +Description > +--- > + > +i2c-nvidia-gpu is a driver for I2C controller included in NVIDIA Turing > +and later GPUs and it is used to communicate with Type-C controller on GPUs. > + > +If your 'lspci -v' listing shows something like the following, > + > +01:00.3 Serial bus controller [0c80]: NVIDIA Corporation Device 1ad9 (rev a1) > + > +then this driver should support the I2C controller of your GPU. > diff --git a/MAINTAINERS b/MAINTAINERS > index 9ad052a..2d1c5a1 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6797,6 +6797,13 @@ L: linux-a...@vger.kernel.org > S: Maintained > F: drivers/i2c/i2c-core-acpi.c > > +I2C CONTROLLER DRIVER FOR NVIDIA GPU > +M: Ajay Gupta > +L: linux-...@vger.kernel.org > +S: Maintained > +F: Documentation/i2c/busses/i2c-nvidia-gpu > +F: drivers/i2c/busses/i2c-nvidia-gpu.c > + > I2C MUXES > M: Peter Rosin > L: linux-...@vger.kernel.org > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index 451d4ae..eed827b 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -224,6 +224,15 @@ config I2C_NFORCE2_S4985 > This driver can also be built as a module. If so, the module > will be called i2c-nforce2-s4985. > > +config I2C_NVIDIA_GPU > + tristate "NVIDIA GPU I2C controller" > + depends on PCI > + help > + If you say yes to this option, support will be included for the > + NVIDIA GPU I2C controller which is used to communicate with the GPU's > + Type-C controller. This driver can also be built as a module called > + i2c-nvidia-gpu. > + > config I2C_SIS5595 > tristate "SiS 5595" > depends on PCI > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile > index 18b26af..d499813 100644 > --- a/drivers/i2c/busses/Makefile > +++ b/drivers/i2c/busses/Makefile > @@ -140,5 +140,6 @@ obj-$(CONFIG_I2C_SIBYTE) += i2c-sibyte.o > obj-$(CONFIG_I2C_XGENE_SLIMPRO) += i2c-xgene-slimpro.o > obj-$(CONFIG_SCx200_ACB) += scx200_acb.o > obj-$(CONFIG_I2C_FSI)+= i2c-fsi.o > +obj-$(CONFIG_I2C_NVIDIA_GPU) += i2c-nvidia-gpu.o > > ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG > diff --git a/drivers/i2c/busses/i2c-nvidia-gpu.c > b/drivers/i2c/busses/i2c-nvidia-gpu.c > new file mode 100644 > index 000..c231121 > --- /dev/null > +++ b/drivers/i2c/busses/i2c-nvidia-gpu.c > @@ -0,0 +1,372 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Nvidia GPU I2C controller Driver > + * > + * Copyright (C) 2018 NVIDIA Corporation. All rights reserved. > + * Author: Ajay Gupta > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +/* I2C definitions */ > +#define I2C_MST_CNTL 0x00 > +#define I2C_MST_CNTL_GEN_START BIT(0) > +#define I2C_MST_CNTL_GEN_STOPBIT(1) > +#define I2C_MST_C
[PATCH] option: Improve Quectel EP06 detection
The Quectel EP06 (and EM06/EG06) LTE modem supports updating the USB
configuration, without the VID/PID or configuration number changing.
When the configuration is updated and interfaces are added/removed, the
interface numbers are updated. This causes our current code for matching
EP06 not to work as intended, as the assumption about reserved
interfaces no longer holds. If for example the diagnostic (first)
interface is removed, option will (try to) bind to the QMI interface.
This patch improves EP06 detection by replacing the current match with
two matches, and those matches check class, subclass and protocol as
well as VID and PID. The diag interface exports class, subclass and
protocol as 0xff. For the other serial interfaces, class is 0xff and
subclass and protocol are both 0x0.
The modem can export the following devices and always in this order:
diag, nmea, at, ppp. qmi and adb. This means that diag can only ever be
interface 0, and interface numbers 1-5 should be marked as reserved. The
three other serial devices can have interface numbers 0-3, but I have
not marked any interfaces as reserved. The reason is that the serial
devices are the only interfaces exported by the device where subclass
and protocol is 0x0.
QMI exports the same class, subclass and protocol values as the diag
interface. However, the two interfaces have different number of
endpoints, QMI has three and diag two. I have added a check for number
of interfaces if VID/PID matches the EP06, and we ignore the device if
number of interfaces equals three (and subclass is set).
Signed-off-by: Kristian Evensen
---
drivers/usb/serial/option.c | 17 +++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 0215b70c4efc..835dcd2875a7 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -1081,8 +1081,9 @@ static const struct usb_device_id option_ids[] = {
.driver_info = RSVD(4) },
{ USB_DEVICE(QUECTEL_VENDOR_ID, QUECTEL_PRODUCT_BG96),
.driver_info = RSVD(4) },
- { USB_DEVICE(QUECTEL_VENDOR_ID, QUECTEL_PRODUCT_EP06),
- .driver_info = RSVD(4) | RSVD(5) },
+ { USB_DEVICE_AND_INTERFACE_INFO(QUECTEL_VENDOR_ID,
QUECTEL_PRODUCT_EP06, 0xff, 0xff, 0xff),
+ .driver_info = RSVD(1) | RSVD(2) | RSVD(3) | RSVD(4) | RSVD(5) },
+ { USB_DEVICE_AND_INTERFACE_INFO(QUECTEL_VENDOR_ID,
QUECTEL_PRODUCT_EP06, 0xff, 0, 0) },
{ USB_DEVICE(CMOTECH_VENDOR_ID, CMOTECH_PRODUCT_6001) },
{ USB_DEVICE(CMOTECH_VENDOR_ID, CMOTECH_PRODUCT_CMU_300) },
{ USB_DEVICE(CMOTECH_VENDOR_ID, CMOTECH_PRODUCT_6003),
@@ -1985,6 +1986,7 @@ static int option_probe(struct usb_serial *serial,
{
struct usb_interface_descriptor *iface_desc =
&serial->interface->cur_altsetting->desc;
+ struct usb_device_descriptor *dev_desc = &serial->dev->descriptor;
unsigned long device_flags = id->driver_info;
/* Never bind to the CD-Rom emulation interface */
@@ -1999,6 +2001,17 @@ static int option_probe(struct usb_serial *serial,
if (device_flags & RSVD(iface_desc->bInterfaceNumber))
return -ENODEV;
+ /*
+* Don't bind to the QMI device of the Quectel EP06/EG06/EM06. Class,
+* subclass and protocol is 0xff for both the diagnostic port and the
+* QMI interface, but the diagnostic port only has two endpoints (QMI
+* has three).
+*/
+ if (dev_desc->idVendor == cpu_to_le16(QUECTEL_VENDOR_ID) &&
+ dev_desc->idProduct == cpu_to_le16(QUECTEL_PRODUCT_EP06) &&
+ iface_desc->bInterfaceSubClass && iface_desc->bNumEndpoints == 3)
+ return -ENODEV;
+
/* Store the device flags so we can use them during attach. */
usb_set_serial_data(serial, (void *)device_flags);
--
2.14.1
[PATCH v4 6/8] usb: dwc3: check for requests in started list for stream capable endpoints
For stream capable endpoints, uas layer can queue mulpile requests on
single ep with different stream ids. So, there can be multiple pending
requests waiting to be transferred. This patch changes the code to check
for any pending requests waiting to be transferred on ep started_list and
calls __dwc3_gadget_kick_transfer() if any.
Signed-off-by: Anurag Kumar Vulisha
Reviewed-by: Thinh Nguyen
---
Changes in v4:
1. None
Changes in v3:
1. None
Changes in v2:
1. None
---
drivers/usb/dwc3/gadget.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 97bfdf0..c50cad8 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2433,6 +2433,9 @@ static void
dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep,
dwc3_gadget_ep_cleanup_completed_requests(dep, event, status);
+ if (dep->stream_capable && !list_empty(&dep->started_list))
+ __dwc3_gadget_kick_transfer(dep);
+
if (stop) {
dwc3_stop_active_transfer(dep, true);
dep->flags = DWC3_EP_ENABLED;
--
2.1.1
[PATCH v4 2/8] usb: dwc3: update stream id in depcmd
For stream capable endpoints, stream id related information
needs to be updated into DEPCMD while issuing START TRANSFER.
This patch does the same.
Signed-off-by: Anurag Kumar Vulisha
Reviewed-by: Thinh Nguyen
---
Changes in v4:
1. None
Changes in v3:
1. None
Changes in v2:
1. None
---
drivers/usb/dwc3/gadget.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 8a1622b..43d63a8 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1224,6 +1224,9 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep
*dep)
params.param1 = lower_32_bits(req->trb_dma);
cmd = DWC3_DEPCMD_STARTTRANSFER;
+ if (dep->stream_capable)
+ cmd |= DWC3_DEPCMD_PARAM(req->request.stream_id);
+
if (usb_endpoint_xfer_isoc(dep->endpoint.desc))
cmd |= DWC3_DEPCMD_PARAM(dep->frame_number);
} else {
--
2.1.1
[PATCH v4 5/8] usb: dwc3: don't issue no-op trb for stream capable endpoints
The stream capable endpoints require stream id to be given
when issuing START TRANSFER. While issuing no-op trb the
stream id is not yet known, so don't issue no-op trb's on
stream capable endpoints.
Signed-off-by: Anurag Kumar Vulisha
Reviewed-by: Thinh Nguyen
---
Changes in v4:
1. None
Changes in v3:
1. None
Changes in v2:
1. None
---
drivers/usb/dwc3/gadget.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 306d4c5..97bfdf0 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -677,7 +677,7 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
unsigned int action)
* Issue StartTransfer here with no-op TRB so we can always rely on No
* Response Update Transfer command.
*/
- if (usb_endpoint_xfer_bulk(desc) ||
+ if ((usb_endpoint_xfer_bulk(desc) && !dep->stream_capable) ||
usb_endpoint_xfer_int(desc)) {
struct dwc3_gadget_ep_cmd_params params;
struct dwc3_trb *trb;
--
2.1.1
[PATCH v4 8/8] usb: dwc3: Check MISSED ISOC bit only for ISOC endpoints
When streaming is enabled on BULK endpoints and LST bit is set
observed MISSED ISOC bit set in event->status for BULK ep. Since
this bit is only valid for isocronous endpoints, changed the code
to check for isocrnous endpoints when MISSED ISOC bit is set.
Signed-off-by: Anurag Kumar Vulisha
Reviewed-by: Thinh Nguyen
---
Changes in v4:
1. None
Changes in v3:
1. None
Changes in v2:
1. None
---
drivers/usb/dwc3/gadget.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 6b6bdd2..2c9e3ab 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2422,7 +2422,8 @@ static void
dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep,
if (event->status & DEPEVT_STATUS_BUSERR)
status = -ECONNRESET;
- if (event->status & DEPEVT_STATUS_MISSED_ISOC) {
+ if ((event->status & DEPEVT_STATUS_MISSED_ISOC) &&
+ usb_endpoint_xfer_isoc(dep->endpoint.desc)) {
status = -EXDEV;
if (list_empty(&dep->started_list))
--
2.1.1
[PATCH v4 7/8] usb: dwc3: Check for IOC/LST bit in both event->status and TRB->ctrl fields
The present code in dwc3_gadget_ep_reclaim_completed_trb() will check for IOC/LST bit in the event->status and returns if IOC/LST bit is set. This logic doesn't work if multiple TRBs are queued per request and the IOC/LST bit is set on the last TRB of that request. Consider an example where a queued request has multiple queued TRBs and IOC/LST bit is set only for the last TRB. In this case, the Core generates XferComplete/XferInProgress events only for the last TRB (since IOC/LST are set only for the last TRB). As per the logic in dwc3_gadget_ep_reclaim_completed_trb() event->status is checked for IOC/LST bit and returns on the first TRB. This makes the remaining TRBs left unhandled. To aviod this, changed the code to check for IOC/LST bits in both event->status & TRB->ctrl. This patch does the same. Signed-off-by: Anurag Kumar Vulisha Reviewed-by: Thinh Nguyen --- Changes in v4: 1. None Changes in v3: 1. None Changes in v2: 1. None --- drivers/usb/dwc3/gadget.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index c50cad8..6b6bdd2 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2299,7 +2299,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; + + if ((event->status & DEPEVT_STATUS_LST) && + (trb->ctrl & DWC3_TRB_CTRL_LST)) return 1; return 0; -- 2.1.1
[PATCH v4 1/8] usb: dwc3: Correct the logic for checking TRB full in __dwc3_prepare_one_trb()
Availability of TRB's is calculated using dwc3_calc_trbs_left(), which determines total available TRB's based on the HWO bit set in a TRB. In the present code, __dwc3_prepare_one_trb() is called with a TRB which needs to be prepared for transfer. This __dwc3_prepare_one_trb() calls dwc3_calc_trbs_left() to determine total available TRBs and set IOC bit if the total available TRBs are zero. Since the present working TRB (which is passed as an argument to __dwc3_prepare_one_trb() ) doesn't yet have the HWO bit set before calling dwc3_calc_trbs_left(), there are chances that dwc3_calc_trbs_left() wrongly calculates this present working TRB as free(since the HWO bit is not yet set) and returns the total available TRBs as greater than zero (including the present working TRB). This could be a problem. This patch corrects the above mentioned problem in __dwc3_prepare_one_trb() by increementing the dep->trb_enqueue at the last (after preparing the TRB) instead of increementing at the start and setting the IOC bit only if the total available TRBs returned by dwc3_calc_trbs_left() is 1 . Since we are increementing the dep->trb_enqueue at the last, the present working TRB is also considered as available by dwc3_calc_trbs_left() and non zero value is returned . So, according to the modified logic, when the total available TRBs is equal to 1 that means the total available TRBs in the pool are 0. Signed-off-by: Anurag Kumar Vulisha Reviewed-by: Thinh Nguyen --- Changes in v4: 1. Corrected the commit message as suggested by "Thinh Nguyen" Changes in v3: 1. Corrected the logic for setting HWO bit as suggested by "Thinh Nguyen" Changes in v2: 1. Changed the commit message --- drivers/usb/dwc3/gadget.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 032ea7d..8a1622b 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -911,8 +911,6 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, struct dwc3_trb *trb, struct usb_gadget *gadget = &dwc->gadget; enum usb_device_speed speed = gadget->speed; - dwc3_ep_inc_enq(dep); - trb->size = DWC3_TRB_SIZE_LENGTH(length); trb->bpl = lower_32_bits(dma); trb->bph = upper_32_bits(dma); @@ -991,7 +989,7 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, struct dwc3_trb *trb, } if ((!no_interrupt && !chain) || - (dwc3_calc_trbs_left(dep) == 0)) + (dwc3_calc_trbs_left(dep) == 1)) trb->ctrl |= DWC3_TRB_CTRL_IOC; if (chain) @@ -1002,6 +1000,8 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, struct dwc3_trb *trb, trb->ctrl |= DWC3_TRB_CTRL_HWO; + dwc3_ep_inc_enq(dep); + trace_dwc3_prepare_trb(dep, trb); } -- 2.1.1
[PATCH v4 3/8] usb: dwc3: make controller clear transfer resources after complete
To start transfer with another stream id, controller needs to free
previously allocated transfer resource. This will be automatically
done by the controller at the time of XferComplete Event. This
patch updates the code to issue XferComplete event once all transfers
are done by setting LST bit in the ctrl field of the last TRB.
Signed-off-by: Anurag Kumar Vulisha
Reviewed-by: Thinh Nguyen
---
Changes in v4:
1. None
Changes in v3:
1. Added the changes suggested by "Thinh Nguyen"
Changes in v2:
1. None
---
drivers/usb/dwc3/gadget.c | 20 +---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 43d63a8..13ea282 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -571,7 +571,8 @@ static int dwc3_gadget_set_ep_config(struct dwc3_ep *dep,
unsigned int action)
if (usb_ss_max_streams(comp_desc) && usb_endpoint_xfer_bulk(desc)) {
params.param1 |= DWC3_DEPCFG_STREAM_CAPABLE
- | DWC3_DEPCFG_STREAM_EVENT_EN;
+ | DWC3_DEPCFG_STREAM_EVENT_EN
+ | DWC3_DEPCFG_XFER_COMPLETE_EN;
dep->stream_capable = true;
}
@@ -995,6 +996,15 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep,
struct dwc3_trb *trb,
if (chain)
trb->ctrl |= DWC3_TRB_CTRL_CHN;
+ /*
+* To issue start transfer on another stream, controller need to free
+* previously acquired transfer resource. Setting the LST bit in
+* last TRB makes the controller clear transfer resource for that
+* endpoint, allowing to start another stream on that endpoint.
+*/
+ else if (dep->stream_capable)
+ trb->ctrl |= DWC3_TRB_CTRL_LST;
+
if (usb_endpoint_xfer_bulk(dep->endpoint.desc) && dep->stream_capable)
trb->ctrl |= DWC3_TRB_CTRL_SID_SOFN(stream_id);
@@ -2268,7 +2278,7 @@ 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)
+ if (event->status & (DEPEVT_STATUS_IOC | DEPEVT_STATUS_LST))
return 1;
return 0;
@@ -2457,6 +2467,11 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
}
switch (event->endpoint_event) {
+ case DWC3_DEPEVT_XFERCOMPLETE:
+ if (!dep->stream_capable)
+ break;
+ dep->flags &= ~DWC3_EP_TRANSFER_STARTED;
+ /* Fall Through */
case DWC3_DEPEVT_XFERINPROGRESS:
dwc3_gadget_endpoint_transfer_in_progress(dep, event);
break;
@@ -2472,7 +2487,6 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
}
break;
case DWC3_DEPEVT_STREAMEVT:
- case DWC3_DEPEVT_XFERCOMPLETE:
case DWC3_DEPEVT_RXTXFIFOEVT:
break;
}
--
2.1.1
[PATCH v4 4/8] usb: dwc3: implement stream transfer timeout
According to dwc3 databook when streams are used, it may be possible
for the host and device become out of sync, where device may wait for
host to issue prime transcation and host may wait for device to issue
erdy. To avoid such deadlock, timeout needs to be implemented. After
timeout occurs, device will first stop transfer and restart the transfer
again. This patch does the same.
Signed-off-by: Anurag Kumar Vulisha
Reviewed-by: Thinh Nguyen
---
Chnages in v4:
1. Added description for stream timeout timer as suggested by
"Thinh Nguyen"
Changes in v3:
1. Added the changes suggested by "Thinh Nguyen"
Changes in v2:
1. Changed STREAM_TIMEOUT to STREAM_TIMEOUT_MS as suggested by
"Andy Shevchenko"
---
drivers/usb/dwc3/core.h | 7 +++
drivers/usb/dwc3/gadget.c | 45 +
2 files changed, 52 insertions(+)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 5bfb625..f62e8c4 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -633,6 +633,11 @@ struct dwc3_event_buffer {
#define DWC3_TRB_NUM 256
+/*
+ * Timeout value in msecs used by stream_timeout_timer when streams are enabled
+ */
+#define STREAM_TIMEOUT_MS 50
+
/**
* struct dwc3_ep - device side endpoint representation
* @endpoint: usb endpoint
@@ -656,6 +661,7 @@ struct dwc3_event_buffer {
* @name: a human readable name e.g. ep1out-bulk
* @direction: true for TX, false for RX
* @stream_capable: true when streams are enabled
+ * @stream_timeout_timer: timeout timer used by bulk streams
*/
struct dwc3_ep {
struct usb_ep endpoint;
@@ -705,6 +711,7 @@ struct dwc3_ep {
unsigneddirection:1;
unsignedstream_capable:1;
+ struct timer_list stream_timeout_timer;
};
enum dwc3_phy {
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 13ea282..306d4c5 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -254,6 +254,7 @@ int dwc3_send_gadget_generic_command(struct dwc3 *dwc,
unsigned cmd, u32 param)
}
static int __dwc3_gadget_wakeup(struct dwc3 *dwc);
+static void stream_timeout_function(struct timer_list *arg);
/**
* dwc3_send_gadget_ep_cmd - issue an endpoint command
@@ -574,6 +575,17 @@ static int dwc3_gadget_set_ep_config(struct dwc3_ep *dep,
unsigned int action)
| DWC3_DEPCFG_STREAM_EVENT_EN
| DWC3_DEPCFG_XFER_COMPLETE_EN;
dep->stream_capable = true;
+
+ /*
+* When BULK streams are enabled it may be possible for the host
+* and device become out of sync, where device may wait for host
+* to issue prime transcation and host may wait for device to
+* issue ERDY. To avoid such deadlock, timeout needs to be
+* implemented. After timeout occurs, device will first stop
+* transfer and restart the transfer again.
+*/
+ timer_setup(&dep->stream_timeout_timer,
+ stream_timeout_function, 0);
}
if (!usb_endpoint_xfer_control(desc))
@@ -730,6 +742,9 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep)
trace_dwc3_gadget_ep_disable(dep);
+ if (dep->stream_capable)
+ del_timer(&dep->stream_timeout_timer);
+
dwc3_remove_requests(dwc, dep);
/* make sure HW endpoint isn't stalled */
@@ -1257,6 +1272,12 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep
*dep)
return ret;
}
+ if (starting && dep->stream_capable) {
+ dep->stream_timeout_timer.expires = jiffies +
+ msecs_to_jiffies(STREAM_TIMEOUT_MS);
+ add_timer(&dep->stream_timeout_timer);
+ }
+
return 0;
}
@@ -2403,6 +2424,13 @@ static void
dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep,
stop = true;
}
+ /*
+* Delete the timer that was started in __dwc3_gadget_kick_transfer()
+* for stream capable endpoints.
+*/
+ if (dep->stream_capable)
+ del_timer(&dep->stream_timeout_timer);
+
dwc3_gadget_ep_cleanup_completed_requests(dep, event, status);
if (stop) {
@@ -2487,6 +2515,11 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
}
break;
case DWC3_DEPEVT_STREAMEVT:
+ if (event->status == DEPEVT_STREAMEVT_FOUND)
+ del_timer(&dep->stream_timeout_timer);
+ else
+ dev_dbg(dwc->dev, "unable to find suitable stream");
+ break;
case DWC3_DEPEVT_RXTXFIFOEVT:
break;
}
@@ -2588,6 +2621,18 @@ static void dwc3_stop_active_transfer(struct dwc3_ep
*dep, bool
[PATCH v4 0/8] usb: dwc3: Fix broken BULK stream support to dwc3 gadget driver
These patch series fixes the broken BULK streaming support in dwc3 gadget driver. Changes in v4: 1. Corrected the commit messgae and stream timeout description as suggested by "Thinh Nguyen" Changes in v3: 1. Added the changes suggested by "Thinh Nguyen" Changes in v2: 1. Added "usb: dwc3:" in subject heading Anurag Kumar Vulisha (8): usb: dwc3: Correct the logic for checking TRB full in __dwc3_prepare_one_trb() usb: dwc3: update stream id in depcmd usb: dwc3: make controller clear transfer resources after complete usb: dwc3: implement stream transfer timeout usb: dwc3: don't issue no-op trb for stream capable endpoints usb: dwc3: check for requests in started list for stream capable endpoints usb: dwc3: Check for IOC/LST bit in both event->status and TRB->ctrl fields usb: dwc3: Check MISSED ISOC bit only for ISOC endpoints drivers/usb/dwc3/core.h | 7 drivers/usb/dwc3/gadget.c | 87 ++- 2 files changed, 86 insertions(+), 8 deletions(-) -- 2.1.1
