[PATCH V5 2/9] Add Synopsys DesignWare HS USB OTG driver framework.
Alan Cox
alan at lxorguk.ukuu.org.uk
Thu Oct 21 20:12:03 EST 2010
> + 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 ?
More information about the Linuxppc-dev
mailing list