On Tue, Feb 28, 2012 at 05:28:47PM +0200, Felipe Balbi wrote:
> There's really no point in having hcd->irq as a
> signed integer when we consider the fact that
> IRQ 0 means NO_IRQ. In order to avoid confusion,
> make hcd->irq unsigned and fix users who were
> passing -1 as the IRQ number to usb_add_hcd.
Comments below.
> Signed-off-by: Felipe Balbi <balbi@xxxxxx>
> ---
> drivers/usb/core/hcd.c | 6 +++---
> drivers/usb/host/xhci.c | 4 ++--
> drivers/usb/musb/musb_core.c | 2 +-
> drivers/usb/musb/musb_gadget.c | 2 +-
> include/linux/usb/hcd.h | 2 +-
> 5 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index e128232..9d7fc9a 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2352,7 +2352,7 @@ static int usb_hcd_request_irqs(struct usb_hcd *hcd,
> "io mem" : "io base",
> (unsigned long long)hcd->rsrc_start);
> } else {
> - hcd->irq = -1;
> + hcd->irq = 0;
> if (hcd->rsrc_start)
> dev_info(hcd->self.controller, "%s 0x%08llx\n",
> (hcd->driver->flags & HCD_MEMORY) ?
> @@ -2508,7 +2508,7 @@ err_register_root_hub:
> clear_bit(HCD_FLAG_POLL_RH, &hcd->flags);
> del_timer_sync(&hcd->rh_timer);
> err_hcd_driver_start:
> - if (usb_hcd_is_primary_hcd(hcd) && hcd->irq >= 0)
> + if (usb_hcd_is_primary_hcd(hcd) && hcd->irq > 0)
> free_irq(irqnum, hcd);
> err_request_irq:
> err_hcd_driver_setup:
> @@ -2573,7 +2573,7 @@ void usb_remove_hcd(struct usb_hcd *hcd)
> del_timer_sync(&hcd->rh_timer);
>
> if (usb_hcd_is_primary_hcd(hcd)) {
> - if (hcd->irq >= 0)
> + if (hcd->irq > 0)
> free_irq(hcd->irq, hcd);
> }
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index c939f5f..7c2d3f5 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -224,13 +224,13 @@ static void xhci_free_irq(struct xhci_hcd *xhci)
> int ret;
>
> /* return if using legacy interrupt */
> - if (xhci_to_hcd(xhci)->irq >= 0)
> + if (xhci_to_hcd(xhci)->irq > 0)
> return;
>
> ret = xhci_free_msi(xhci);
> if (!ret)
> return;
> - if (pdev->irq >= 0)
> + if (pdev->irq > 0)
> free_irq(pdev->irq, xhci_to_hcd(xhci));
I think you may have missed a couple spots in the xHCI driver, in
xhci_try_enable_msi():
/* unregister the legacy interrupt */
if (hcd->irq)
free_irq(hcd->irq, hcd);
hcd->irq = -1;
ret = xhci_setup_msix(xhci);
if (ret)
/* fall back to msi*/
ret = xhci_setup_msi(xhci);
if (!ret)
/* hcd->irq is -1, we have MSI */
return 0;
You probably want to change both the set to -1 and the comment above the
return on MSI enabled.
Alex, you'll need to rebase your MSI enabling patchset against Felipe's
when it gets into Greg's usb-next branch.
Otherwise, I think the patch is fine. How will this effect PCI devices
where the BIOS doesn't provide a legacy IRQ line definition? I think it
will be ok, because hcd->irq is separate from pci_dev->irq (which will
be negative if there's no legacy IRQ line definition). Felipe, do you
think this case will still be fine under this change?
Sarah Sharp
> return;
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 3d11cf64..2954517 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -1987,7 +1987,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
> musb->xceiv->default_a = 1;
> musb->xceiv->state = OTG_STATE_A_IDLE;
>
> - status = usb_add_hcd(musb_to_hcd(musb), -1, 0);
> + status = usb_add_hcd(musb_to_hcd(musb), 0, 0);
>
> hcd->self.uses_pio_for_control = 1;
> dev_dbg(musb->controller, "%s mode, status %d, devctl %02x %c\n",
> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> index ac3d2ee..e9a750a 100644
> --- a/drivers/usb/musb/musb_gadget.c
> +++ b/drivers/usb/musb/musb_gadget.c
> @@ -1938,7 +1938,7 @@ static int musb_gadget_start(struct usb_gadget *g,
> * handles power budgeting ... this way also
> * ensures HdrcStart is indirectly called.
> */
> - retval = usb_add_hcd(musb_to_hcd(musb), -1, 0);
> + retval = usb_add_hcd(musb_to_hcd(musb), 0, 0);
> if (retval < 0) {
> dev_dbg(musb->controller, "add_hcd failed, %d\n", retval);
> goto err2;
> diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
> index b2f62f3..60f6feb 100644
> --- a/include/linux/usb/hcd.h
> +++ b/include/linux/usb/hcd.h
> @@ -127,7 +127,7 @@ struct usb_hcd {
> unsigned authorized_default:1;
> unsigned has_tt:1; /* Integrated TT in root hub */
>
> - int irq; /* irq allocated */
> + unsigned int irq; /* irq allocated */
> void __iomem *regs; /* device memory/io */
> u64 rsrc_start; /* memory/io resource start */
> u64 rsrc_len; /* memory/io resource length */
> --
> 1.7.9.2
>
> --
> 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
--
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
