> From: Felipe Balbi [mailto:balbi@xxxxxx] > Sent: Thursday, April 11, 2013 11:36 AM > > On Thu, Apr 11, 2013 at 06:43:48PM +0200, Matthijs Kooijman wrote: > > This adds a dwc_platform.ko module that can be loaded by using > > compatible = "snps,dwc2" in a device tree. > > > > Signed-off-by: Matthijs Kooijman <matthijs@xxxxxxxx> > > --- > > Documentation/devicetree/bindings/staging/dwc2.txt | 15 +++ > > drivers/staging/dwc2/Kconfig | 6 +- > > drivers/staging/dwc2/Makefile | 2 + > > drivers/staging/dwc2/platform.c | 150 +++++++++++++++++++++ > > 4 files changed, 171 insertions(+), 2 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/staging/dwc2.txt > > create mode 100644 drivers/staging/dwc2/platform.c > > > > diff --git a/Documentation/devicetree/bindings/staging/dwc2.txt b/Documentation/devicetree/bindings/staging/dwc2.txt > > new file mode 100644 > > index 0000000..3649c88 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/staging/dwc2.txt > > @@ -0,0 +1,15 @@ > > +Platform DesignWare HS OTG USB 2.0 controller > > +----------------------------------------------------- > > + > > +Required properties: > > +- compatible : "snps,dwc2" > > please use the company's full name -> synopsys. Actually, if you grep the tree for '"snps' vs. '"synopsys', you will see that 'snps' is much more common in dt bindings. So I would suggest that this is actually more correct, and the few outliers using 'synopsys' should be converted. > > +- reg : Should contain 1 register range (address and length) > > +- interrupts : Should contain 1 interrupt > > + > > +Example: > > + > > + otg@101c0000 { > > this should probably be 'usb' instead of 'otg' > > > +static int dwc2_driver_remove(struct platform_device *dev) > > +{ > > + struct dwc2_hsotg *hsotg = platform_get_drvdata(dev); > > + > > + dev_dbg(&dev->dev, "%s(%p)\n", __func__, dev); > > I don't think you need this dev_dbg() line. > > > +static int dwc2_driver_probe(struct platform_device *dev) > > +{ > > + struct dwc2_hsotg *hsotg; > > + struct resource *res; > > + int retval; > > + int irq; > > + struct dwc2_core_params params; > > + > > + /* Default all params to autodetect */ > > + dwc2_set_all_params(¶ms, -1); > > + > > + dev_dbg(&dev->dev, "%s(%p)\n", __func__, dev); > > neither this one. > > > + hsotg = devm_kzalloc(&dev->dev, sizeof(*hsotg), GFP_KERNEL); > > + if (!hsotg) > > + return -ENOMEM; > > + > > + hsotg->dev = &dev->dev; > > + > > + irq = platform_get_irq(dev, 0); > > + if (irq < 0) { > > + dev_err(&dev->dev, "missing IRQ resource\n"); > > + return -EINVAL; > > + } > > + > > + res = platform_get_resource(dev, IORESOURCE_MEM, 0); > > + if (!res) { > > + dev_err(&dev->dev, "missing memory base resource\n"); > > + return -EINVAL; > > + } > > + > > + hsotg->regs = devm_ioremap_resource(&dev->dev, res); > > + if (IS_ERR(hsotg->regs)) > > + return PTR_ERR(hsotg->regs); > > + > > + dev_dbg(&dev->dev, "mapped PA %08lx to VA %p\n", > > + (unsigned long)res->start, hsotg->regs); > > + > > + retval = dwc2_hcd_init(hsotg, irq, ¶ms); > > + if (retval) > > + return retval; > > + > > + platform_set_drvdata(dev, hsotg); > > + dev_dbg(&dev->dev, "hsotg=%p\n", hsotg); > > or this. > > > +MODULE_AUTHOR("Synopsys, Inc."); > > isn't MODULE_AUTHOR("Matthijs Kooijman <matthijs@xxxxxxxx>") more > appropriate ? Yes, I agree. -- 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