Hi, On Tue, Feb 18, 2014 at 03:14:25PM -0600, Dinh Nguyen wrote: > On Fri, 2014-02-14 at 22:50 +0000, Paul Zimmerman wrote: > > > From: Jingoo Han [mailto:jg1.han@xxxxxxxxxxx] > > > Sent: Thursday, February 13, 2014 11:46 PM > > > > > > On Friday, February 14, 2014 2:27 PM, Peter Chen wrote: > > > > On Thu, Feb 13, 2014 at 03:10:40PM -0600, dinguyen@xxxxxxxxxx wrote: > > > > > From: Dinh Nguyen <dinguyen@xxxxxxxxxx> > > > > > > > > > > Hello, > > > > > > > > > > This patch series combines the dwc2 host driver and the s3c-hsotg peripheral > > > > > driver into a single dual-roler driver similar to the dwc3. > > > > > > > > Does s3c-hsotg use dwc usb2 ip too? If it is, the structure should > > > > like ip driver + glue layer. If not, why needs to put them > > > > together, I am a little puzzled here. > > > > > > Yes, 's3c-hsotg' also uses DWC USB2 IP. > > > As DWC3 USB driver (./drivers/usb/dwc3/), 'IP driver + glue layer' > > > looks good. > > > > > > Best regards, > > > Jingoo Han > > > > Well, DWC3 is a little different. It has a standards-based host API, > > xHCI, that is (almost) completely independent from its non-standards- > > based device API (I say 'almost' because of the optional OTG > > functionality). This means the two drivers have to be separate, to > > support other xHCI implementations that don't have the device mode > > functionality. But it also complicates some things, like the > > implementation of OTG support, which perhaps can be seen by the fact > > that there is almost no OTG support yet in the DWC3 driver (except for > > role switching). I don't think having separate drivers is undermining OTG support. It's just that nobody has put down the effort to implement it ;-) > > Whereas the DWC2 host and device modes both use a proprietary API, so > > there is no need to have separate drivers, because there are no other > > implementations of the core except the Synopsys one. Also, quite a few > > of the registers are shared between host and device modes, so keeping > > the two drivers separate would mean duplicating some code that could > > be shared between them. > > > > That said, I would have no objection to keeping the two drivers > > separate, as long as it doesn't create significant difficulties with > > the implementation. > > > > Dinh, what do you think? Did you consider the possibility of keeping the > > two drivers separate, perhaps sharing some code in a library module? For > > example, you could have dwc2-lib.ko, dwc2.ko, and s3c-hsotg.ko. > > dwc2-lib.ko would always be loaded, and dwc2.ko or s3c-hsotg.ko (or both) > > would be loaded depending on which mode is selected. Each driver would > > have its own interrupt handler, both of them sharing the common IRQ. > > They would both have their own probe methods, too. > > > > No, I didn't think of this possibility. This initial patch was to try to > remove some duplicating of code between dwc2/s3c-hsotg without breaking > them too badly. But I think I failed in this aspect from the initial > testing. > > I like your suggestion of keeping the drivers separate with a library > module for the role-switching. Do you see this driver being able to go > full OTG in the future? And if so, will one method be more adaptable > than the other to enable OTG? > > > > I haven't thought too deeply about this, so maybe it's a bad idea. But > > you are the one doing the work, so I think the final decision should be > > yours. > > > > So let try your approach. But does moving s3c-hostg into the dwc2 folder > and sharing 1 define file still valid? I would say combining those driver is the best thing you guys can do for several reasons. It will make sure everybody uses a single generic driver, it'll help you avoid code duplication (how many other dwc2 implementations have we seen on linux-usb alone ?), it'll focus efforts to stabilize a single dwc2 driver etc. In fact, I have been asking Samsung folks to cleanup s3c-hsotg for quite a while now exactly so we could have a single dwc2 driver for host and device. cheers -- balbi
Attachment:
signature.asc
Description: Digital signature
