Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190611135842.8396-1-narmstrong@baylibre.com>
 <CAFBinCAkwjf9oDV6AGPi2PzzQ2KNTXXDHW6FTfN3kXpDT6cFpg@mail.gmail.com> <8950adde-0942-5f04-0ce6-922a9886c440@baylibre.com>
In-Reply-To: <8950adde-0942-5f04-0ce6-922a9886c440@baylibre.com>
From:   Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Date:   Wed, 12 Jun 2019 21:12:16 +0200
Message-ID: <CAFBinCBaBBv1buJ=U022GyHN+UJdfA+DC0t7wkSEt5aqvjdG-g@mail.gmail.com>
Subject: Re: [PATCH] usb: dwc3: meson-g12a: Add support for IRQ based OTG switching
To:     Neil Armstrong <narmstrong@baylibre.com>
Cc:     balbi@kernel.org, linux-amlogic@lists.infradead.org,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Neil,

On Wed, Jun 12, 2019 at 5:13 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
[...]
> >> @@ -436,6 +452,19 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev)
> >>         /* Get dr_mode */
> >>         priv->otg_mode = usb_get_dr_mode(dev);
> >>
> >> +       if (priv->otg_mode == USB_DR_MODE_OTG) {
> >> +               /* Ack irq before registering */
> >> +               regmap_update_bits(priv->regmap, USB_R5,
> >> +                                  USB_R5_ID_DIG_IRQ, 0);
> > I assume that either the IRQ line is:
> > - always enabled
> > - enabled when (USB_R5_ID_DIG_EN_0 | USB_R5_ID_DIG_EN_1 |
> > USB_R5_ID_DIG_TH_MASK) are set (which we already do in
> > dwc3_meson_g12a_usb_init)
>
> Can't say... I suspect the (USB_R5_ID_DIG_EN_0 | USB_R5_ID_DIG_EN_1 |
> > USB_R5_ID_DIG_TH_MASK) enables the detection.
> The regmap_update_bits(USB_R5_ID_DIG_IRQ) is only here to make sure the "current"
> irq event is masked, whatever the previous init.
>
> Or I misunderstood question ?
that perfectly answers my question - thank you!

> >
> >> +               irq = platform_get_irq(pdev, 0);
> > do we need to check the IRQ before trying to request it?
> > drivers/gpu/drm/meson/meson_dw_hdmi.c and drivers/usb/dwc3/host.c for
> > example error out if irq number is lower than 0
>
> No, devm_request_threaded_irq() will fail if -1 is given, I've using this scheme
> for a while now !
OK, it wasn't obvious to me when I looked at devm_request_threaded_irq.
thank you for clarifying this


Martin