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

From: Felipe Balbi <balbi@ti.com>
To: Roger Quadros <rogerq@ti.com>
Cc: <balbi@ti.com>, <tony@atomide.com>, <Joao.Pinto@synopsys.com>,
	<sergei.shtylyov@cogentembedded.com>, <peter.chen@freescale.com>,
	<jun.li@freescale.com>, <linux-usb@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-omap@vger.kernel.org>
Subject: Re: [PATCH v4 4/9] usb: dwc3: core: Adapt to named interrupts
Date: Thu, 3 Sep 2015 10:48:49 -0500	[thread overview]
Message-ID: <20150903154849.GB4031@saruman.tx.rr.com> (raw)
In-Reply-To: <55E84133.9070704@ti.com>

[-- 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 --]