[PATCH V5 2/9] Add Synopsys DesignWare HS USB OTG driver framework.

[Alan Cox]

> +	int retval = IRQ_NONE;
> +
> +	retval = dwc_otg_handle_common_intr(dwc_dev->core_if);
> +	return IRQ_RETVAL(retval);

Why assign retval first ?

> +}
> +
> +/**
> + * This function is the interrupt handler for the OverCurrent condition
> + * from the external charge pump (if enabled)
> + */
> +static irqreturn_t dwc_otg_externalchgpump_irq(int _irq, void *dev)
> +{
> +	struct dwc_otg_device *dwc_dev = dev;
> +
> +	if (dwc_otg_is_host_mode(dwc_dev->core_if)) {
> +		struct dwc_hcd *dwc_hcd;
> +		union hprt0_data hprt0 = {.d32 = 0};
> +
> +		dwc_hcd = dwc_dev->hcd;
> +		spin_lock(&dwc_hcd->lock);
> +		dwc_hcd->flags.b.port_over_current_change = 1;
> +
> +		hprt0.b.prtpwr = 0;
> +		dwc_write_reg32(dwc_dev->core_if->host_if->hprt0,
> +				hprt0.d32);
> +		spin_unlock(&dwc_hcd->lock);
> +	} else {
> +		/* Device mode - This int is n/a for device mode */
> +		printk(KERN_ERR "DeviceMode: OTG OverCurrent Detected\n");

Use dev_err



> +	if (of_address_to_resource(ofdev->dev.of_node, 0, &res)) {
> +		printk(KERN_ERR "%s: Can't get USB-OTG register address\n",
> +			__func__);

dev_err

> +		retval = -ENOMEM;
> +		goto fail;
> +	}
> +	dev_dbg(dev, "OTG - ioresource_mem start0x%08x: end:0x%08x\n",
> +			(u32)res.start, (u32)res.end);

Why not print these with the proper types ?


> +	 * Install the interrupt handler for the common interrupts before
> +	 * enabling common interrupts in core_init below.
> +	 */
> +	retval = request_irq(dwc_dev->irq, dwc_otg_common_irq,
> +			IRQF_SHARED, "dwc_otg", dwc_dev);

What if there is an IRQ pending - you've enabled the IRQ handler before
the otg core - is that safe ?



The module options also seems a bit excessive. Is there a reason this
can't just set by the various board platform devices ?