Skip to contentSkip to conversation

usb: gadget: dwc2: make driver run on a version 3.10a instance of DWC_OTG

RFC PATCH In review Neil Armstrong 2 participants 2 messages Aug 13, 2014

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 usb: gadget: dwc2: make driver run on a version 3.10a instance of DWC_OTG

We are trying to make a driver run on our PCD only instance of Synopsys DWC_OTG IP with the following parameters :
- Dedicated Fifos : Yes
- Descriptor DMA : Yes
- PHY : 8bit UTMI+ (may need also support for ULPI)
- Endpoints : 6 (7 with ep0)
- Periodic IN Endpoints : 0
- IN Endpoints : 3 (4 with ep0)
- EP repartition : INOUT, IN, OUT, IN , OUT, IN, OUT
- DFifo Depth : 1844

I do not know what is the original s3c hsotg IP config, but to correctly support the Dedicated Fifos you need to attribute a FIFO index for each IN endpoints, and on the 3.10a release we only have 4 TX FIFOS, so we just cannot give the EP number as fifo index.

It impacts the FIFO repartition, it is not perfect since we do not take into account the Isochronous needs, so it needs some rework.

To managed these FIFOs, I load from the registers the EP repartition, HW fifo depth and HW revision.
It also impacts that hw_cfg must be done before init call.

Same for the num_of_eps value, in the Synopsys documentation, it specifies it is the count of non-ep0 endpoints, but the driver for loops where like :
for(i = 1 ; i < hsotg->num_of_eps ; ++i)
which is absolutely invalid and ignores the last EP.

We never managed to make the driver work in FIFO mode, but in DMA mode it works like a charm, and to support all the gadget drivers, we implemented a silly Bounce Buffer mode.

The PHY configuration was also rewritten according to the documentation.

Fo the descripto DMA mode, I tried to add missing parts, but I get stuck with weird HW beheviours....

---
 drivers/usb/dwc2/core.h   |   14 +++
 drivers/usb/dwc2/gadget.c |  267 +++++++++++++++++++++++++++++++++++----------
 2 files changed, 221 insertions(+), 60 deletions(-)

This is mainly an RFC in order to make this driver run with recent HW. This is why the patch is not 100% clean.

drivers/usb/dwc2/core.h +14 -0
index 1efd10c..21c136e 100644
@@ -102,6 +102,7 @@ struct s3c_hsotg_req;
  * @dir_in: Set to true if this endpoint is of the IN direction, which
  *          means that it is sending data to the Host.
  * @index: The index for the endpoint registers.
+ * @txfnum: The TX fifo index.
  * @mc: Multi Count - number of transactions per microframe
  * @interval - Interval for periodic endpoints
  * @name: The name array passed to the USB core.
@@ -142,6 +143,7 @@ struct s3c_hsotg_ep {
 
     unsigned char           dir_in;
     unsigned char           index;
+    unsigned int            txfnum;
     unsigned char           mc;
     unsigned char           interval;
 
@@ -167,6 +169,10 @@ struct s3c_hsotg_ep {
  * @phyif: PHY interface width
  * @dedicated_fifos: Set if the hardware has dedicated IN-EP fifos.
  * @num_of_eps: Number of available EPs (excluding EP0)
+ * @dfifo_depth: depth of HW DFIFO
+ * @dir_of_eps: direction of each endpoint
+ * @hw_major: Hardware Major number
+ * @hw_minor: Hardware minor ID
  * @debug_root: root directrory for debugfs.
  * @debug_file: main status file for debugfs.
  * @debug_fifo: FIFO status file for debugfs.
@@ -195,7 +201,12 @@ struct s3c_hsotg {
 
     u32                     phyif;
     unsigned int            dedicated_fifos:1;
+    unsigned int            dfifo_depth;
     unsigned char           num_of_eps;
+    unsigned int            dma_enable:1;
+    unsigned char           dir_of_eps[16];
+    unsigned int            hw_major;
+    unsigned int            hw_minor[3];
 
     struct dentry           *debug_root;
     struct dentry           *debug_file;
@@ -224,6 +235,9 @@ struct s3c_hsotg_req {
     struct list_head        queue;
     unsigned char           in_progress;
     unsigned char           mapped;
+    unsigned            bounced_dma;
+    dma_addr_t          bounce_phys;
+    void                * bounce;
 };
 
 #define call_gadget(_hs, _entry) \
drivers/usb/dwc2/gadget.c +207 -60

471 lines not on this page · raw message

Signature · 1 line
1.7.0.4
#1 lore raw
Quoted 37 lines

From: Neil Armstrong [mailto:[email protected]]
Sent: Wednesday, August 13, 2014 5:17 AM

We are trying to make a driver run on our PCD only instance of Synopsys DWC_OTG IP with the following parameters :
- Dedicated Fifos : Yes
- Descriptor DMA : Yes
- PHY : 8bit UTMI+ (may need also support for ULPI)
- Endpoints : 6 (7 with ep0)
- Periodic IN Endpoints : 0
- IN Endpoints : 3 (4 with ep0)
- EP repartition : INOUT, IN, OUT, IN , OUT, IN, OUT
- DFifo Depth : 1844

I do not know what is the original s3c hsotg IP config, but to correctly support the Dedicated Fifos you need to attribute a FIFO index for each IN endpoints, and on the 3.10a release we only have 4 TX FIFOS, so we just cannot give the EP number as fifo index.

It impacts the FIFO repartition, it is not perfect since we do not take into account the Isochronous needs, so it needs some rework.

To managed these FIFOs, I load from the registers the EP repartition, HW fifo depth and HW revision.
It also impacts that hw_cfg must be done before init call.

Same for the num_of_eps value, in the Synopsys documentation, it specifies it is the count of non-ep0 endpoints, but the driver for loops where like :
for(i = 1 ; i < hsotg->num_of_eps ; ++i)
which is absolutely invalid and ignores the last EP.

We never managed to make the driver work in FIFO mode, but in DMA mode it works like a charm, and to support all the gadget drivers, we implemented a silly Bounce Buffer mode.

Quoted 5 lines

The PHY configuration was also rewritten according to the documentation.

Fo the descripto DMA mode, I tried to add missing parts, but I get stuck with weird HW beheviours....

Hi Neil,

Thanks for your work on this.

First off, please always submit patches, even RFCs, with the proper kernel coding style. scripts/checkpatch.pl will help you there. That way other developers are more likely to take your patches seriously.

Second, you should base your work on top of Robert Baldyga's patch series "usb: dwc2/gadget: fix series". That should be going into the kernel when the merge window opens up next week. That series also touches the FIFO configuration code. You can find it at
http://marc.info/?l=linux-kernel&m=140568383216978.

Third, it looks like we may need some way to customize the FIFO configuration depending on the platform. I don't think it's possible to configure the FIFOs properly for all platforms otherwise. Maybe you and Robert could work together on that?

Fourth, you should split out the fixes (like the off-by-one in the endpoint loops) into separate patches. Those could be applied right away.

Other than that, this looks OK to me. I especially like that you are providing a way to enable DMA support, since I will need that myself in the near future ;)

Signature · 1 line
Paul