> From: dinguyen@xxxxxxxxxx [mailto:dinguyen@xxxxxxxxxx] > Sent: Thursday, February 13, 2014 1:11 PM > > The dwc2_hsotg will be the main data structure for both the dwc2 and > s3c-hsotg drivers. This patch adds the appropriate data structures into the > dwc2_hsotg data structure in order for it to support both host and gadget > functionality. > > This commit still keeps the dwc2 host and s3c_hsotg gadget driver separate. > The only commonality is a single data structure, clock, register base, and irq. > > The majority of changes in this commit are edits to the s3c-hsotg.c file. The > edits are dominated by changes in how the s3c_hsotg data structure are accessed > directly that now have to be accessed through a pointer in the dwc2_hsotg > structure. > > The gadget data structure is now also part of the dwc2_hsotg structure. The > dwc2_hsotg data structure now has a pointer to the clk structure. The "otg" > clock that was needed for the s3c_hsotg driver is now needed for both the > dwc2 and s3c_hsotg drivers. < snip > > diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h > index 1efd10c..1dae260 100644 > --- a/drivers/usb/dwc2/core.h > +++ b/drivers/usb/dwc2/core.h > @@ -84,6 +84,8 @@ static const char * const s3c_hsotg_supply_names[] = { > */ > #define EP0_MPS_LIMIT 64 > > +struct dwc2_hsotg; > +struct dwc2_host_chan; > struct s3c_hsotg; > struct s3c_hsotg_req; > > @@ -130,7 +132,7 @@ struct s3c_hsotg_req; > struct s3c_hsotg_ep { > struct usb_ep ep; > struct list_head queue; > - struct s3c_hsotg *parent; > + struct dwc2_hsotg *parent; Whitespace damage here. In fact, it looks like a lot (all?) of the s3c-hsotg code here got whitespace damaged when you moved it to this file in earlier patches. That all needs to be fixed (redo the earlier patches to fix the damage). > struct s3c_hsotg_req *req; > struct dentry *debugfs; > > @@ -162,7 +164,6 @@ struct s3c_hsotg_ep { > * @plat: The platform specific configuration data. This can be removed once > * all SoCs support usb transceiver. > * @regs: The memory area mapped for accessing registers. > - * @irq: The IRQ number we are using > * @supplies: Definition of USB power supplies > * @phyif: PHY interface width > * @dedicated_fifos: Set if the hardware has dedicated IN-EP fifos. > @@ -179,18 +180,10 @@ struct s3c_hsotg_ep { > * @eps: The endpoints being supplied to the gadget framework > */ > struct s3c_hsotg { > - struct device *dev; > - struct usb_gadget_driver *driver; > struct phy *phy; > struct usb_phy *uphy; > struct s3c_hsotg_plat *plat; > > - spinlock_t lock; > - > - void __iomem *regs; > - int irq; > - struct clk *clk; > - > struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)]; > > u32 phyif; > @@ -206,10 +199,8 @@ struct s3c_hsotg { > u8 ep0_buff[8]; > u8 ctrl_buff[8]; > > - struct usb_gadget gadget; > unsigned int setup; > unsigned long last_rst; > - struct s3c_hsotg_ep *eps; > }; > > /** > @@ -236,9 +227,6 @@ do { \ > } \ > } while (0) > > -struct dwc2_hsotg; > -struct dwc2_host_chan; > - > /* Device States */ > enum dwc2_lx_state { > DWC2_L0, /* On state */ > @@ -587,6 +575,8 @@ struct dwc2_hw_params { > struct dwc2_hsotg { > struct device *dev; > void __iomem *regs; > + struct clk *clk; > + Please keep the formatting consistent when modifying existing code. > /** Params detected from hardware */ > struct dwc2_hw_params hw_params; > /** Params to actually use */ > @@ -601,6 +591,11 @@ struct dwc2_hsotg { > struct timer_list wkp_timer; > enum dwc2_lx_state lx_state; > > + struct s3c_hsotg *s3c_hsotg; I don't think it's a good idea to include a pointer to the s3c_hsotg struct inside of the dwc2_hsotg struct. Then you get things like dwc2->s3c_hsotg->dedicated_fifos later on in this patch. How about just moving all the members of s3c_hsotg into dwc2_hsotg? > + struct usb_gadget gadget; > + struct usb_gadget_driver *driver; > + struct s3c_hsotg_ep *eps; > + Formatting here is a mess. < snip > > diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c > index d01d0d3..c6fb4f2 100644 > --- a/drivers/usb/dwc2/platform.c > +++ b/drivers/usb/dwc2/platform.c > @@ -34,6 +34,7 @@ > * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > */ > > +#include <linux/clk.h> > #include <linux/kernel.h> > #include <linux/module.h> > #include <linux/slab.h> > @@ -90,12 +91,16 @@ static int dwc2_driver_remove(struct platform_device *dev) > { > struct dwc2_hsotg *hsotg = platform_get_drvdata(dev); > > - dwc2_hcd_remove(hsotg); > + if (IS_ENABLED(CONFIG_USB_DWC2_HOST)) > + dwc2_hcd_remove(hsotg); > + else /* s3c-hsotg gadget */ > + s3c_hsotg_remove(hsotg); > > return 0; > } > > static const struct of_device_id dwc2_of_match_table[] = { > + { .compatible = "samsung,s3c6400-hsotg", }, > { .compatible = "brcm,bcm2835-usb", .data = ¶ms_bcm2835 }, > { .compatible = "snps,dwc2", .data = NULL }, > {}, > @@ -129,7 +134,8 @@ static int dwc2_driver_probe(struct platform_device *dev) > params = match->data; > } else { > /* Default all params to autodetect */ > - dwc2_set_all_params(&defparams, -1); > + if (IS_ENABLED(CONFIG_USB_DWC2_HOST)) > + dwc2_set_all_params(&defparams, -1); > params = &defparams; > } > > @@ -162,9 +168,23 @@ static int dwc2_driver_probe(struct platform_device *dev) > dev_dbg(&dev->dev, "mapped PA %08lx to VA %p\n", > (unsigned long)res->start, hsotg->regs); > > - retval = dwc2_hcd_init(hsotg, irq, params); > - if (retval) > - return retval; > + /* Get clock */ > + hsotg->clk = devm_clk_get(hsotg->dev, "otg"); > + if (IS_ERR(hsotg->clk)) { > + dev_err(hsotg->dev, "cannot get otg clock\n"); > + return PTR_ERR(hsotg->clk); > + } > + clk_prepare_enable(hsotg->clk); Doesn't the addition of this get clock code break any existing dwc2 platforms that don't have a clock named "otg"? The rest of the patch looks like mostly mechanical replacement of struct s3c_hsotg with struct dwc2_hsotg, which should be fine, modulo moving the members of s3c_hsotg into dwc2_hsotg like I recommended. -- Paul -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
