On Thursday, February 06, 2014 4:21 AM, Paul Zimmerman wrote: > On Tuesday, February 04, 2014 10:14 PM, Dinh Nguyen wrote: > > On Wed, 2014-02-05 at 00:42 +0000, Paul Zimmerman wrote: > > > > From: dinguyen@xxxxxxxxxx [mailto:dinguyen@xxxxxxxxxx] > > > > Sent: Tuesday, February 04, 2014 1:46 PM > > > > > > > > From: Dinh Nguyen <dinguyen@xxxxxxxxxx> > > > > > > > > This means that the driver can be in host or peripheral mode when the appropriate > > > > connector is used. When an A-cable is plugged in, the driver behaves in host > > > > mode, and when a B-cable is used, the driver will be in peripheral mode. > > > > > > > > This commit: > > > > - Replaces in the defines used in s3c_hsotg.h with the defines used in the dwc2 > > > > hw.h defines. > > > > - Use the dw2_hsotg as the unified data structure for the host/gadget. > > > > - Uses the dwc2 IRQ handler for host/gadget. > > > > - A single spinlock. > > > > > > Hi Dinh, > > > > > > Putting all of these changes into a single patch makes them unreviewable > > > as far I am concerned. You need to break this into a series of smaller > > > patches. I would suggest something like this: > > > > > > 1 of n: Make the minimum changes to the dwc2 header files needed to > > > support s3c-hsotg as a standalone driver. > > > 2 of n: Make the spelling changes to s3c-hsotg.c needed to use the dwc2 > > > headers, and move it to the dwc2/ directory. Make the Kconfig > > > and Makefile changes needed for the move. Delete s3c-hsotg.h. > > > 3 of n: Move the struct defines etc. from s3c-hsotg.c to the dwc2 > > > header files. > > > .. of n: Make the changes required to combine the functionality of > > > both drivers into one. Preferably this would also be a series > > > of patches instead of one big one. > > > > > > At each step of the series, both drivers should still compile and work. > > > > I agree. My original thought was to also split this patch, but I just > > didn't know how to split it. This is why I designated as an RFC. I was > > really looking for feedback as this is the correct way to combine this > > driver. I was also looking for testing purpose to make sure I did not > > break anything for the s3c platform. > > The problem is, it's almost impossible to see what the functional > changes are because of all the noise of the other changes. Yes, right. I think so, too. > > > > > > > Also, please follow the patch style used on the linux lists. > > > 'git format-patch --cover-letter' should do most of this for you > > > automatically. > > > > I did use --cover-letter on this patch series. > > > > > > > > And you should probably trim the Cc list to something more reasonable. > > > > I looked through all the commits for the dwc2 driver for the cc list. I > > also CC a bunch of the Samsung people as I figured that the biggest > > impact of the work would affect the s3c folks. > > I would suggest just CCing the Samsung folks to start with, since s3c-hsotg > is a driver for their hardware. And the linux-usb and linux-samsung-soc > lists. Please keep CCing linux-samsung-soc. Some Samsung folks will test the patches with Samsung hardware. Thank you. Best regards, Jingoo Han -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html