Re: [PATCH 3/4] staging: dwc2: add platform device bindings

[Felipe Balbi]

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 <matth...@stdin.nl>
> ---
>  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.

> +- 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(&params, -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, &params);
> +     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 <matth...@stdin.nl>") more
appropriate ?

-- 
balbi

signature.asc
Description: Digital signature