| Message ID | 574E92E8.5080201@ti.com |
|---|---|
| State | New |
| Headers | show |
Felipe, On 01/06/16 11:06, Felipe Balbi wrote: > > Hi, > > Roger Quadros <rogerq@ti.com> writes: >> Implementations might use different IRQs for >> host, gadget and OTG so use named interrupt resources >> to allow Device tree to specify the 3 interrupts. >> >> Following are the interrupt names >> >> Peripheral Interrupt - peripheral >> HOST Interrupt - host >> OTG Interrupt - otg >> >> We still maintain backward compatibility for a single named >> interrupt for all 3 interrupts (e.g. for dwc3-pci) and >> single unnamed interrupt for all 3 interrupts (e.g. old DT). >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> v9: rebased on top of balbi/testing/next > > breaks dwc3: > > [ 222.776504] dwc3 dwc3.0.auto: failed to request irq #-6 --> -22 > > please test > I couldn't reproduce the failure at my end. Could it be specific to your setup? cheers, -roger
On 02/06/16 14:52, Grygorii Strashko wrote: > On 06/01/2016 10:46 AM, Roger Quadros wrote: >> Implementations might use different IRQs for >> host, gadget and OTG so use named interrupt resources >> to allow Device tree to specify the 3 interrupts. >> >> Following are the interrupt names >> >> Peripheral Interrupt - peripheral >> HOST Interrupt - host >> OTG Interrupt - otg > > or "dwc_usb3"?? That is for backward compatibility only. I could explicitly mention it in the next line. > >> >> We still maintain backward compatibility for a single named >> interrupt for all 3 interrupts (e.g. for dwc3-pci) and >> single unnamed interrupt for all 3 interrupts (e.g. old DT). > > bindings OK. > >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> v9: rebased on top of balbi/testing/next >> >> drivers/usb/dwc3/core.c | 10 ---------- >> drivers/usb/dwc3/gadget.c | 20 ++++++++++++++++++-- >> drivers/usb/dwc3/host.c | 19 +++++++++++++++++++ >> 3 files changed, 37 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >> index 9c4e1d8d..5cedf3d 100644 >> --- a/drivers/usb/dwc3/core.c >> +++ b/drivers/usb/dwc3/core.c >> @@ -843,16 +843,6 @@ static int dwc3_probe(struct platform_device *pdev) >> dwc->mem = mem; >> dwc->dev = dev; >> >> - res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); >> - if (!res) { >> - dev_err(dev, "missing IRQ\n"); >> - return -ENODEV; >> - } >> - dwc->xhci_resources[1].start = res->start; >> - dwc->xhci_resources[1].end = res->end; >> - dwc->xhci_resources[1].flags = res->flags; >> - dwc->xhci_resources[1].name = res->name; >> - >> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> if (!res) { >> dev_err(dev, "missing memory resource\n"); >> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >> index c37168d..c18c72f 100644 >> --- a/drivers/usb/dwc3/gadget.c >> +++ b/drivers/usb/dwc3/gadget.c >> @@ -1726,7 +1726,7 @@ static int dwc3_gadget_start(struct usb_gadget *g, >> int ret = 0; >> int irq; >> >> - irq = platform_get_irq(to_platform_device(dwc->dev), 0); >> + irq = dwc->irq_gadget; >> ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt, >> IRQF_SHARED, "dwc3", dwc->ev_buf); >> if (ret) { >> @@ -1734,7 +1734,6 @@ static int dwc3_gadget_start(struct usb_gadget *g, >> irq, ret); >> goto err0; >> } >> - dwc->irq_gadget = irq; >> >> spin_lock_irqsave(&dwc->lock, flags); >> if (dwc->gadget_driver) { >> @@ -2853,6 +2852,23 @@ static irqreturn_t dwc3_interrupt(int irq, void *_evt) >> int dwc3_gadget_init(struct dwc3 *dwc) >> { >> int ret; >> + struct resource *res; >> + struct platform_device *dwc3_pdev = to_platform_device(dwc->dev); >> + >> + dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev, "peripheral"); >> + if (dwc->irq_gadget <= 0) { > > Is it expected to get -EPROBE_DEFER here? Probably not as we don't have any chance of deferring probe here. We've already probed successfully and are just turning on the gadget mode here. > >> + dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev, >> + "dwc_usb3"); >> + if (dwc->irq_gadget <= 0) { >> + res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ, >> + 0); > > It's better to use platform_get_irq(). OK. > >> + if (!res) { >> + dev_err(dwc->dev, "missing peripheral IRQ\n"); >> + return -ENODEV; >> + } >> + dwc->irq_gadget = res->start; >> + } >> + } >> >> dwc->ctrl_req = dma_alloc_coherent(dwc->dev, sizeof(*dwc->ctrl_req), >> &dwc->ctrl_req_addr, GFP_KERNEL); >> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c >> index c679f63..f2b60a4 100644 >> --- a/drivers/usb/dwc3/host.c >> +++ b/drivers/usb/dwc3/host.c >> @@ -25,6 +25,25 @@ int dwc3_host_init(struct dwc3 *dwc) >> struct platform_device *xhci; >> struct usb_xhci_pdata pdata; >> int ret; >> + struct resource *res; >> + struct platform_device *dwc3_pdev = to_platform_device(dwc->dev); >> + >> + res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ, "host"); >> + if (!res) { >> + res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ, >> + "dwc_usb3"); >> + if (!res) { >> + res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ, >> + 0); >> + if (!res) > >> + return -ENOMEM; >> + } >> + } > > Is it expected to have more than one IRQ here? No. > > if not - it will better to use platform_get_irq[_byname](). OK. > > >> + >> + dwc->xhci_resources[1].start = res->start; >> + dwc->xhci_resources[1].end = res->end; >> + dwc->xhci_resources[1].flags = res->flags; >> + dwc->xhci_resources[1].name = res->name; >> >> xhci = platform_device_alloc("xhci-hcd", PLATFORM_DEVID_AUTO); >> if (!xhci) { >> > > cheers, -roger
On 07/06/16 14:49, Grygorii Strashko wrote: > On 06/07/2016 12:34 PM, Roger Quadros wrote: >> On 02/06/16 14:52, Grygorii Strashko wrote: >>> On 06/01/2016 10:46 AM, Roger Quadros wrote: >>>> Implementations might use different IRQs for >>>> host, gadget and OTG so use named interrupt resources >>>> to allow Device tree to specify the 3 interrupts. >>>> >>>> Following are the interrupt names >>>> >>>> Peripheral Interrupt - peripheral >>>> HOST Interrupt - host >>>> OTG Interrupt - otg >>> >>> or "dwc_usb3"?? >> >> That is for backward compatibility only. I could explicitly >> mention it in the next line. > > yes pls, this confuses. > Also I don't see how "otg" irq name is used in code. > OK. I'll remove it from the commit message. >> >>> >>>> >>>> We still maintain backward compatibility for a single named >>>> interrupt for all 3 interrupts (e.g. for dwc3-pci) and >>>> single unnamed interrupt for all 3 interrupts (e.g. old DT). >>> >>> bindings >> >> OK. >>> >>>> >>>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>>> --- >>>> v9: rebased on top of balbi/testing/next >>>> >>>> drivers/usb/dwc3/core.c | 10 ---------- >>>> drivers/usb/dwc3/gadget.c | 20 ++++++++++++++++++-- >>>> drivers/usb/dwc3/host.c | 19 +++++++++++++++++++ >>>> 3 files changed, 37 insertions(+), 12 deletions(-) >>>> >>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>>> index 9c4e1d8d..5cedf3d 100644 >>>> --- a/drivers/usb/dwc3/core.c >>>> +++ b/drivers/usb/dwc3/core.c >>>> @@ -843,16 +843,6 @@ static int dwc3_probe(struct platform_device *pdev) >>>> dwc->mem = mem; >>>> dwc->dev = dev; >>>> >>>> - res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); >>>> - if (!res) { >>>> - dev_err(dev, "missing IRQ\n"); >>>> - return -ENODEV; >>>> - } >>>> - dwc->xhci_resources[1].start = res->start; >>>> - dwc->xhci_resources[1].end = res->end; >>>> - dwc->xhci_resources[1].flags = res->flags; >>>> - dwc->xhci_resources[1].name = res->name; >>>> - >>>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>>> if (!res) { >>>> dev_err(dev, "missing memory resource\n"); >>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >>>> index c37168d..c18c72f 100644 >>>> --- a/drivers/usb/dwc3/gadget.c >>>> +++ b/drivers/usb/dwc3/gadget.c >>>> @@ -1726,7 +1726,7 @@ static int dwc3_gadget_start(struct usb_gadget *g, >>>> int ret = 0; >>>> int irq; >>>> >>>> - irq = platform_get_irq(to_platform_device(dwc->dev), 0); >>>> + irq = dwc->irq_gadget; >>>> ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt, >>>> IRQF_SHARED, "dwc3", dwc->ev_buf); >>>> if (ret) { >>>> @@ -1734,7 +1734,6 @@ static int dwc3_gadget_start(struct usb_gadget *g, >>>> irq, ret); >>>> goto err0; >>>> } >>>> - dwc->irq_gadget = irq; >>>> >>>> spin_lock_irqsave(&dwc->lock, flags); >>>> if (dwc->gadget_driver) { >>>> @@ -2853,6 +2852,23 @@ static irqreturn_t dwc3_interrupt(int irq, void *_evt) >>>> int dwc3_gadget_init(struct dwc3 *dwc) >>>> { >>>> int ret; >>>> + struct resource *res; >>>> + struct platform_device *dwc3_pdev = to_platform_device(dwc->dev); >>>> + >>>> + dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev, "peripheral"); >>>> + if (dwc->irq_gadget <= 0) { >>> >>> Is it expected to get -EPROBE_DEFER here? >> >> Probably not as we don't have any chance of deferring probe here. We've already >> probed successfully and are just turning on the gadget mode here. > > In general, you can't say that you've been probed successfully if not all resources are ready, > and irq is a resource :) > It's expected that all resources will be requested in probe, but here you are trying to get > resource outside of probe. As result, it will be perfectly possible to get -EPROBE_DEFER here > if on some HW GPIO IRQ will be used as peripheral, or host or otg irq (for example), because > GPIO IRQ controller might not be ready at the moment when IRQ resource is requested. I agree with you. Felipe, are you ok with moving the IRQ resource obtaining code to probe? -- cheers, -roger
On 07/06/16 14:49, Grygorii Strashko wrote: > On 06/07/2016 12:34 PM, Roger Quadros wrote: >> On 02/06/16 14:52, Grygorii Strashko wrote: >>> On 06/01/2016 10:46 AM, Roger Quadros wrote: >>>> Implementations might use different IRQs for >>>> host, gadget and OTG so use named interrupt resources >>>> to allow Device tree to specify the 3 interrupts. >>>> >>>> Following are the interrupt names >>>> >>>> Peripheral Interrupt - peripheral >>>> HOST Interrupt - host >>>> OTG Interrupt - otg >>> >>> or "dwc_usb3"?? >> >> That is for backward compatibility only. I could explicitly >> mention it in the next line. > > yes pls, this confuses. > Also I don't see how "otg" irq name is used in code. > >> >>> >>>> >>>> We still maintain backward compatibility for a single named >>>> interrupt for all 3 interrupts (e.g. for dwc3-pci) and >>>> single unnamed interrupt for all 3 interrupts (e.g. old DT). >>> >>> bindings >> >> OK. >>> >>>> >>>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>>> --- >>>> v9: rebased on top of balbi/testing/next >>>> >>>> drivers/usb/dwc3/core.c | 10 ---------- >>>> drivers/usb/dwc3/gadget.c | 20 ++++++++++++++++++-- >>>> drivers/usb/dwc3/host.c | 19 +++++++++++++++++++ >>>> 3 files changed, 37 insertions(+), 12 deletions(-) >>>> >>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>>> index 9c4e1d8d..5cedf3d 100644 >>>> --- a/drivers/usb/dwc3/core.c >>>> +++ b/drivers/usb/dwc3/core.c >>>> @@ -843,16 +843,6 @@ static int dwc3_probe(struct platform_device *pdev) >>>> dwc->mem = mem; >>>> dwc->dev = dev; >>>> >>>> - res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); >>>> - if (!res) { >>>> - dev_err(dev, "missing IRQ\n"); >>>> - return -ENODEV; >>>> - } >>>> - dwc->xhci_resources[1].start = res->start; >>>> - dwc->xhci_resources[1].end = res->end; >>>> - dwc->xhci_resources[1].flags = res->flags; >>>> - dwc->xhci_resources[1].name = res->name; >>>> - >>>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>>> if (!res) { >>>> dev_err(dev, "missing memory resource\n"); >>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >>>> index c37168d..c18c72f 100644 >>>> --- a/drivers/usb/dwc3/gadget.c >>>> +++ b/drivers/usb/dwc3/gadget.c >>>> @@ -1726,7 +1726,7 @@ static int dwc3_gadget_start(struct usb_gadget *g, >>>> int ret = 0; >>>> int irq; >>>> >>>> - irq = platform_get_irq(to_platform_device(dwc->dev), 0); >>>> + irq = dwc->irq_gadget; >>>> ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt, >>>> IRQF_SHARED, "dwc3", dwc->ev_buf); >>>> if (ret) { >>>> @@ -1734,7 +1734,6 @@ static int dwc3_gadget_start(struct usb_gadget *g, >>>> irq, ret); >>>> goto err0; >>>> } >>>> - dwc->irq_gadget = irq; >>>> >>>> spin_lock_irqsave(&dwc->lock, flags); >>>> if (dwc->gadget_driver) { >>>> @@ -2853,6 +2852,23 @@ static irqreturn_t dwc3_interrupt(int irq, void *_evt) >>>> int dwc3_gadget_init(struct dwc3 *dwc) >>>> { >>>> int ret; >>>> + struct resource *res; >>>> + struct platform_device *dwc3_pdev = to_platform_device(dwc->dev); >>>> + >>>> + dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev, "peripheral"); >>>> + if (dwc->irq_gadget <= 0) { >>> >>> Is it expected to get -EPROBE_DEFER here? >> >> Probably not as we don't have any chance of deferring probe here. We've already >> probed successfully and are just turning on the gadget mode here. > I was mistaken here. dwc3_gadget_init() and dwc3_host_init() get called during dwc3_core_init_mode() which is in fact called during probe(). So I'll add take care -EPROBE_DEFER in the next revision. > In general, you can't say that you've been probed successfully if not all resources are ready, > and irq is a resource :) > It's expected that all resources will be requested in probe, but here you are trying to get > resource outside of probe. As result, it will be perfectly possible to get -EPROBE_DEFER here > if on some HW GPIO IRQ will be used as peripheral, or host or otg irq (for example), because > GPIO IRQ controller might not be ready at the moment when IRQ resource is requested. > -- cheers, -roger
Grygorii, On 02/06/16 14:52, Grygorii Strashko wrote: > On 06/01/2016 10:46 AM, Roger Quadros wrote: >> Implementations might use different IRQs for >> host, gadget and OTG so use named interrupt resources >> to allow Device tree to specify the 3 interrupts. >> >> Following are the interrupt names >> >> Peripheral Interrupt - peripheral >> HOST Interrupt - host >> OTG Interrupt - otg > > or "dwc_usb3"?? > >> >> We still maintain backward compatibility for a single named >> interrupt for all 3 interrupts (e.g. for dwc3-pci) and >> single unnamed interrupt for all 3 interrupts (e.g. old DT). > > bindings > >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> v9: rebased on top of balbi/testing/next >> >> drivers/usb/dwc3/core.c | 10 ---------- >> drivers/usb/dwc3/gadget.c | 20 ++++++++++++++++++-- >> drivers/usb/dwc3/host.c | 19 +++++++++++++++++++ >> 3 files changed, 37 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >> index 9c4e1d8d..5cedf3d 100644 >> --- a/drivers/usb/dwc3/core.c >> +++ b/drivers/usb/dwc3/core.c >> @@ -843,16 +843,6 @@ static int dwc3_probe(struct platform_device *pdev) >> dwc->mem = mem; >> dwc->dev = dev; >> >> - res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); >> - if (!res) { >> - dev_err(dev, "missing IRQ\n"); >> - return -ENODEV; >> - } >> - dwc->xhci_resources[1].start = res->start; >> - dwc->xhci_resources[1].end = res->end; >> - dwc->xhci_resources[1].flags = res->flags; >> - dwc->xhci_resources[1].name = res->name; >> - >> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> if (!res) { >> dev_err(dev, "missing memory resource\n"); >> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >> index c37168d..c18c72f 100644 >> --- a/drivers/usb/dwc3/gadget.c >> +++ b/drivers/usb/dwc3/gadget.c >> @@ -1726,7 +1726,7 @@ static int dwc3_gadget_start(struct usb_gadget *g, >> int ret = 0; >> int irq; >> >> - irq = platform_get_irq(to_platform_device(dwc->dev), 0); >> + irq = dwc->irq_gadget; >> ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt, >> IRQF_SHARED, "dwc3", dwc->ev_buf); >> if (ret) { >> @@ -1734,7 +1734,6 @@ static int dwc3_gadget_start(struct usb_gadget *g, >> irq, ret); >> goto err0; >> } >> - dwc->irq_gadget = irq; >> >> spin_lock_irqsave(&dwc->lock, flags); >> if (dwc->gadget_driver) { >> @@ -2853,6 +2852,23 @@ static irqreturn_t dwc3_interrupt(int irq, void *_evt) >> int dwc3_gadget_init(struct dwc3 *dwc) >> { >> int ret; >> + struct resource *res; >> + struct platform_device *dwc3_pdev = to_platform_device(dwc->dev); >> + >> + dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev, "peripheral"); >> + if (dwc->irq_gadget <= 0) { > > Is it expected to get -EPROBE_DEFER here? > >> + dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev, >> + "dwc_usb3"); >> + if (dwc->irq_gadget <= 0) { >> + res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ, >> + 0); > > It's better to use platform_get_irq(). > >> + if (!res) { >> + dev_err(dwc->dev, "missing peripheral IRQ\n"); >> + return -ENODEV; >> + } >> + dwc->irq_gadget = res->start; >> + } >> + } >> >> dwc->ctrl_req = dma_alloc_coherent(dwc->dev, sizeof(*dwc->ctrl_req), >> &dwc->ctrl_req_addr, GFP_KERNEL); >> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c >> index c679f63..f2b60a4 100644 >> --- a/drivers/usb/dwc3/host.c >> +++ b/drivers/usb/dwc3/host.c >> @@ -25,6 +25,25 @@ int dwc3_host_init(struct dwc3 *dwc) >> struct platform_device *xhci; >> struct usb_xhci_pdata pdata; >> int ret; >> + struct resource *res; >> + struct platform_device *dwc3_pdev = to_platform_device(dwc->dev); >> + >> + res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ, "host"); >> + if (!res) { >> + res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ, >> + "dwc_usb3"); >> + if (!res) { >> + res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ, >> + 0); >> + if (!res) > >> + return -ENOMEM; >> + } >> + } > > Is it expected to have more than one IRQ here? > > if not - it will better to use platform_get_irq[_byname](). > The reason I used platform_get_resource variant is that i'm passing the resource directly to the XHCI platform device below. > >> + >> + dwc->xhci_resources[1].start = res->start; >> + dwc->xhci_resources[1].end = res->end; >> + dwc->xhci_resources[1].flags = res->flags; >> + dwc->xhci_resources[1].name = res->name; This could just change to dwc->xhci_resource[1] = *res; >> >> xhci = platform_device_alloc("xhci-hcd", PLATFORM_DEVID_AUTO); >> if (!xhci) { >> > > cheers, -roger
On 10/06/16 11:02, Roger Quadros wrote: > Grygorii, > > On 02/06/16 14:52, Grygorii Strashko wrote: >> On 06/01/2016 10:46 AM, Roger Quadros wrote: >>> Implementations might use different IRQs for >>> host, gadget and OTG so use named interrupt resources >>> to allow Device tree to specify the 3 interrupts. >>> >>> Following are the interrupt names >>> >>> Peripheral Interrupt - peripheral >>> HOST Interrupt - host >>> OTG Interrupt - otg >> >> or "dwc_usb3"?? >> >>> >>> We still maintain backward compatibility for a single named >>> interrupt for all 3 interrupts (e.g. for dwc3-pci) and >>> single unnamed interrupt for all 3 interrupts (e.g. old DT). >> >> bindings >> >>> >>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>> --- >>> v9: rebased on top of balbi/testing/next >>> >>> drivers/usb/dwc3/core.c | 10 ---------- >>> drivers/usb/dwc3/gadget.c | 20 ++++++++++++++++++-- >>> drivers/usb/dwc3/host.c | 19 +++++++++++++++++++ >>> 3 files changed, 37 insertions(+), 12 deletions(-) >>> >>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>> index 9c4e1d8d..5cedf3d 100644 >>> --- a/drivers/usb/dwc3/core.c >>> +++ b/drivers/usb/dwc3/core.c >>> @@ -843,16 +843,6 @@ static int dwc3_probe(struct platform_device *pdev) >>> dwc->mem = mem; >>> dwc->dev = dev; >>> >>> - res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); >>> - if (!res) { >>> - dev_err(dev, "missing IRQ\n"); >>> - return -ENODEV; >>> - } >>> - dwc->xhci_resources[1].start = res->start; >>> - dwc->xhci_resources[1].end = res->end; >>> - dwc->xhci_resources[1].flags = res->flags; >>> - dwc->xhci_resources[1].name = res->name; >>> - >>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>> if (!res) { >>> dev_err(dev, "missing memory resource\n"); >>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >>> index c37168d..c18c72f 100644 >>> --- a/drivers/usb/dwc3/gadget.c >>> +++ b/drivers/usb/dwc3/gadget.c >>> @@ -1726,7 +1726,7 @@ static int dwc3_gadget_start(struct usb_gadget *g, >>> int ret = 0; >>> int irq; >>> >>> - irq = platform_get_irq(to_platform_device(dwc->dev), 0); >>> + irq = dwc->irq_gadget; >>> ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt, >>> IRQF_SHARED, "dwc3", dwc->ev_buf); >>> if (ret) { >>> @@ -1734,7 +1734,6 @@ static int dwc3_gadget_start(struct usb_gadget *g, >>> irq, ret); >>> goto err0; >>> } >>> - dwc->irq_gadget = irq; >>> >>> spin_lock_irqsave(&dwc->lock, flags); >>> if (dwc->gadget_driver) { >>> @@ -2853,6 +2852,23 @@ static irqreturn_t dwc3_interrupt(int irq, void *_evt) >>> int dwc3_gadget_init(struct dwc3 *dwc) >>> { >>> int ret; >>> + struct resource *res; >>> + struct platform_device *dwc3_pdev = to_platform_device(dwc->dev); >>> + >>> + dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev, "peripheral"); >>> + if (dwc->irq_gadget <= 0) { >> >> Is it expected to get -EPROBE_DEFER here? >> >>> + dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev, >>> + "dwc_usb3"); >>> + if (dwc->irq_gadget <= 0) { >>> + res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ, >>> + 0); >> >> It's better to use platform_get_irq(). >> >>> + if (!res) { >>> + dev_err(dwc->dev, "missing peripheral IRQ\n"); >>> + return -ENODEV; >>> + } >>> + dwc->irq_gadget = res->start; >>> + } >>> + } >>> >>> dwc->ctrl_req = dma_alloc_coherent(dwc->dev, sizeof(*dwc->ctrl_req), >>> &dwc->ctrl_req_addr, GFP_KERNEL); >>> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c >>> index c679f63..f2b60a4 100644 >>> --- a/drivers/usb/dwc3/host.c >>> +++ b/drivers/usb/dwc3/host.c >>> @@ -25,6 +25,25 @@ int dwc3_host_init(struct dwc3 *dwc) >>> struct platform_device *xhci; >>> struct usb_xhci_pdata pdata; >>> int ret; >>> + struct resource *res; >>> + struct platform_device *dwc3_pdev = to_platform_device(dwc->dev); >>> + >>> + res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ, "host"); >>> + if (!res) { >>> + res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ, >>> + "dwc_usb3"); >>> + if (!res) { >>> + res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ, >>> + 0); >>> + if (!res) >> >>> + return -ENOMEM; >>> + } >>> + } >> >> Is it expected to have more than one IRQ here? >> >> if not - it will better to use platform_get_irq[_byname](). >> > > The reason I used platform_get_resource variant is that i'm passing the > resource directly to the XHCI platform device below. >> >>> + >>> + dwc->xhci_resources[1].start = res->start; >>> + dwc->xhci_resources[1].end = res->end; >>> + dwc->xhci_resources[1].flags = res->flags; >>> + dwc->xhci_resources[1].name = res->name; > > This could just change to > > dwc->xhci_resource[1] = *res; Probably not as we don't want to change parent/child members. > >>> >>> xhci = platform_device_alloc("xhci-hcd", PLATFORM_DEVID_AUTO); >>> if (!xhci) { >>> >> >> > -- cheers, -roger
On 10/06/16 11:18, Felipe Balbi wrote: > > Hi, > > Roger Quadros <rogerq@ti.com> writes: >>> dwc->xhci_resource[1] = *res; >> >> Probably not as we don't want to change parent/child members. > > oh, you had already replied. Sorry. This is correct > np :). So what i'll do is get the irq via platform_get_irq() and friends and if it was a success use platform_get_resource() and friends to get struct resource and just edit the relevant parts for the XHCI irq resource. Sounds OK? something like this. + int ret, irq; + struct resource *res; + struct platform_device *dwc3_pdev = to_platform_device(dwc->dev); + + irq = platform_get_irq_byname(dwc3_pdev, "host"); + if (irq == -EPROBE_DEFER) + return irq; + + if (irq <= 0) { + irq = platform_get_irq_byname(dwc3_pdev, "dwc_usb3"); + if (irq == -EPROBE_DEFER) + return irq; + + if (irq <= 0) { + irq = platform_get_irq(dwc3_pdev, 0); + if (irq <= 0) { + if (irq != -EPROBE_DEFER) { + dev_err(dwc->dev, + "missing host IRQ\n"); + } + return irq; + } else { + res = platform_get_resource(dwc3_pdev, + IORESOURCE_IRQ, 0); + } + } else { + res = platform_get_resource_byname(dwc3_pdev, + IORESOURCE_IRQ, + "dwc_usb3"); + } + + } else { + res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ, + "host"); + } + + dwc->xhci_resources[1].start = irq; + dwc->xhci_resources[1].end = irq; + dwc->xhci_resources[1].flags = res->flags; + dwc->xhci_resources[1].name = res->name; -- cheers, -roger
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 9c4e1d8d..5cedf3d 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -843,16 +843,6 @@ static int dwc3_probe(struct platform_device *pdev) dwc->mem = mem; dwc->dev = dev; - res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); - if (!res) { - dev_err(dev, "missing IRQ\n"); - return -ENODEV; - } - dwc->xhci_resources[1].start = res->start; - dwc->xhci_resources[1].end = res->end; - dwc->xhci_resources[1].flags = res->flags; - dwc->xhci_resources[1].name = res->name; - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!res) { dev_err(dev, "missing memory resource\n"); diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index c37168d..c18c72f 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1726,7 +1726,7 @@ static int dwc3_gadget_start(struct usb_gadget *g, int ret = 0; int irq; - irq = platform_get_irq(to_platform_device(dwc->dev), 0); + irq = dwc->irq_gadget; ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt, IRQF_SHARED, "dwc3", dwc->ev_buf); if (ret) { @@ -1734,7 +1734,6 @@ static int dwc3_gadget_start(struct usb_gadget *g, irq, ret); goto err0; } - dwc->irq_gadget = irq; spin_lock_irqsave(&dwc->lock, flags); if (dwc->gadget_driver) { @@ -2853,6 +2852,23 @@ static irqreturn_t dwc3_interrupt(int irq, void *_evt) int dwc3_gadget_init(struct dwc3 *dwc) { int ret; + struct resource *res; + struct platform_device *dwc3_pdev = to_platform_device(dwc->dev); + + dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev, "peripheral"); + if (dwc->irq_gadget <= 0) { + dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev, + "dwc_usb3"); + if (dwc->irq_gadget <= 0) { + res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ, + 0); + if (!res) { + dev_err(dwc->dev, "missing peripheral IRQ\n"); + return -ENODEV; + } + dwc->irq_gadget = res->start; + } + } dwc->ctrl_req = dma_alloc_coherent(dwc->dev, sizeof(*dwc->ctrl_req), &dwc->ctrl_req_addr, GFP_KERNEL); diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c index c679f63..f2b60a4 100644 --- a/drivers/usb/dwc3/host.c +++ b/drivers/usb/dwc3/host.c @@ -25,6 +25,25 @@ int dwc3_host_init(struct dwc3 *dwc) struct platform_device *xhci; struct usb_xhci_pdata pdata; int ret; + struct resource *res; + struct platform_device *dwc3_pdev = to_platform_device(dwc->dev); + + res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ, "host"); + if (!res) { + res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ, + "dwc_usb3"); + if (!res) { + res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ, + 0); + if (!res) + return -ENOMEM; + } + } + + dwc->xhci_resources[1].start = res->start; + dwc->xhci_resources[1].end = res->end; + dwc->xhci_resources[1].flags = res->flags; + dwc->xhci_resources[1].name = res->name; xhci = platform_device_alloc("xhci-hcd", PLATFORM_DEVID_AUTO); if (!xhci) {
Implementations might use different IRQs for host, gadget and OTG so use named interrupt resources to allow Device tree to specify the 3 interrupts. Following are the interrupt names Peripheral Interrupt - peripheral HOST Interrupt - host OTG Interrupt - otg We still maintain backward compatibility for a single named interrupt for all 3 interrupts (e.g. for dwc3-pci) and single unnamed interrupt for all 3 interrupts (e.g. old DT). Signed-off-by: Roger Quadros <rogerq@ti.com> --- v9: rebased on top of balbi/testing/next drivers/usb/dwc3/core.c | 10 ---------- drivers/usb/dwc3/gadget.c | 20 ++++++++++++++++++-- drivers/usb/dwc3/host.c | 19 +++++++++++++++++++ 3 files changed, 37 insertions(+), 12 deletions(-) -- 2.7.4