usb: gadget: dwc2: make driver run on a version 3.10a instance of DWC_OTG
Subsystems (2)
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
@@ -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
Quoted 37 lines
From: Neil Armstrong [mailto:[email protected]]
Sent: Wednesday, August 13, 2014 5:17 AMWe are trying to make a driver run on our PCD only instance of Synopsys
DWC_OTGIP 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 : 1844I 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 thathw_cfgmust be done before init call.Same for the
num_of_epsvalue, 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