Hi Felipe,
thanks for your comments.
On Thu, Apr 11, 2013 at 09:36:23PM +0300, Felipe Balbi wrote:
> 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>
[]
> > +Required properties:
> > +- compatible : "snps,dwc2"
>
> please use the company's full name -> synopsys.
I've done as documented:
$ cat Documentation/devicetree/bindings/vendor-prefixes.txt | grep Synopsys
snps Synopsys, Inc.
Further grepping shows that only dwc3 uses synopsys, all others use
snps.
> > +- 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'
This is what the mips folks have in their upcoming version in the
arch/mips/ralink/dts/rt3050.dtsi file, but I'm happy to change this for
just the example.
> > +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.
[]
> > + dev_dbg(&dev->dev, "%s(%p)\n", __func__, dev);
>
> neither this one.
>
[]
> > + dev_dbg(&dev->dev, "hsotg=%p\n", hsotg);
>
> or this.
These are just copied from pci.c, to make these as similar as possible.
I can drop them and send a patch to drop them from pci.c as well, since
I agree they're rather pointless.
> > +MODULE_AUTHOR("Synopsys, Inc.");
>
> isn't MODULE_AUTHOR("Matthijs Kooijman <matthijs@xxxxxxxx>") more
> appropriate ?
It started out as a copy of pci.c, which is why I left this in. Changing
it would probably make sense now, yes.
I'll wait for Paul to have a look and then send an updated version.
Gr.
Matthijs
--
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
