Re: [PATCH v4 4/9] usb: dwc3: core: Adapt to named interrupts

Felipe Balbi
2015-09-03T15:49:04Z

[-- Attachment #1: Type: text/plain, Size: 3950 bytes --]

Hi,

On Thu, Sep 03, 2015 at 03:46:43PM +0300, Roger Quadros wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> On 02/09/15 17:34, Felipe Balbi wrote:
> > On Wed, Sep 02, 2015 at 05:24:19PM +0300, Roger Quadros wrote:
> >> From: Felipe Balbi <balbi@ti.com>
> >>
> >> Add support to use interrupt names,
> >>
> >> Following are the interrupt names
> >>
> >> Peripheral Interrupt - peripheral
> >> HOST Interrupt - host
> >> OTG Interrupt - otg
> >>
> >> [Roger Q]
> >> - If any of these are missing we use the
> >> first available IRQ resource so that we don't
> >> break with older DTBs.
> >
> > this is what original commit did:
> >
> > commit ecd5f71cfd663bcd4efd2f29824acd8b2ba9715d
> > Author: Felipe Balbi <balbi@ti.com>
> > Date: Fri Jan 3 13:49:38 2014 -0600
> >
> > usb: dwc3: cleanup IRQ resources
> >
> > In order to make it easier for the driver to
> > figure out which modes of operation it has,
> > and because some dwc3 integrations have rather
> > fuzzy IRQ routing, we now require three different
> > IRQ numbers (peripheral, host, otg).
> >
> > In order to do that and maintain backwards compatibility,
> > we still maintain support for the old IRQ resource name,
> > but if you *really* want to have proper peripheral/host/otg
> > support, you should make sure your resources are correct.
> >
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
>
> This is better since we request the resource only if needed and bail out
> if it is not present.
>
> >
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index 60580a01cdd2..1f01031873b7 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -556,10 +556,22 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
> > static int dwc3_core_init_mode(struct dwc3 *dwc)
> > {
> > struct device *dev = dwc->dev;
> > + struct platform_device *pdev = to_platform_device(dev);
> > int ret;
> >
> > switch (dwc->dr_mode) {
> > case USB_DR_MODE_PERIPHERAL:
> > + dwc->gadget_irq = platform_get_irq_byname(pdev, "dwc3_peripheral");
>
> Shall we just name it just "peripheral"?

sure, why not :-)

> > + if (dwc->gadget_irq < 0) {
> > + dwc->gadget_irq = platform_get_irq_byname(pdev, "dwc_usb3");
>
> How will this work? Currently we don't have a name for the IRQ in the DT.

we can just add interrupt-names, right ? Or, the fallback could be what
is already done today: just fetch it by index.

>
> > + if (dwc->gadget_irq < 0) {
> > + dev_err(dev, "missing IRQ\n");
> > + return dwc->gadget_irq;
> > + } else {
> > + dev_warn(dev, "dwc_usb3 resource is deprecated\n");
>
> Do we want to warn about legacy nodes?

Sure :-)

Now, do you want me to update it, or will you do it ? BTW, if you decide
to do it, we need to patch all users :-)

> > @@ -576,6 +604,28 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
> > }
> > break;
> > case USB_DR_MODE_OTG:
> > + dwc->gadget_irq = platform_get_irq_byname(pdev, "dwc3_peripheral");
> > + if (dwc->gadget_irq < 0) {
> > + dwc->gadget_irq = platform_get_irq_byname(pdev, "dwc_usb3");
> > + if (dwc->gadget_irq < 0) {
> > + dev_err(dev, "missing IRQ\n");
> > + return dwc->gadget_irq;
> > + } else {
> > + dev_warn(dev, "dwc_usb3 resource is deprecated\n");
> > + }
> > +
> > + dwc->xhci_irq = dwc->gadget_irq;
> > + dwc->otg_irq = dwc->gadget_irq;
> > + } else {
> > + dwc->xhci_irq = platform_get_irq_byname(pdev, "dwc3_host");
> > + if (dwc->xhci_irq < 0) {
> > + dev_err(dev, "missing Host IRQ\n");
> > + return dwc->xhci_irq;
> > + }
> > +
> > + dwc->otg_irq = platform_get_irq_byname(pdev, "dwc3_otg");
>
> need to check if error?

right

> > + }
>
> Need to setup xhci_resource[1]?

isn't it done above ?

--
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]